On Wed, Feb 01, 2012 at 23:03:56, Hilman, Kevin wrote: > "Hiremath, Vaibhav" <hvaibhav@xxxxxx> writes: > > > On Tue, Jan 24, 2012 at 04:05:32, Hilman, Kevin wrote: > >> "Hiremath, Vaibhav" <hvaibhav@xxxxxx> writes: > >> > >> > On Wed, Jan 11, 2012 at 21:48:25, Hiremath, Vaibhav wrote: > >> >> On Tue, Jan 10, 2012 at 23:39:22, Hilman, Kevin wrote: > >> >> > Vaibhav Hiremath <hvaibhav@xxxxxx> writes: > >> >> > > >> >> > > AM33XX PRM module (L4_WK domain) will be treated as another seperate > >> >> > > partition in _prm_bases[] table. > >> >> > > > >> >> > > Also, since cpu_is_omap34xx check is true for am33xx family of > >> >> > > devices, we must check cpu_is_am33xx fisrt, in order to follow > >> >> > > omap4 execution path. > >> >> > > >> >> > Can you remind me why cpu_is_omap34xx() is true for AM33xx family? > >> >> > >> >> Yeah sure... > >> >> > >> >> Kevin, > >> >> As mentioned before, the main idea behind bringing am33xx under omap34xx > >> >> was mainly due to "cortex-A8 family of devices". > >> >> > >> >> It has been discussed and aligned long time back, so > >> >> please refer to the thread - > >> >> > >> >> http://www.spinics.net/lists/linux-omap/msg41046.html > >> >> Multiple versions of - > >> >> http://www.spinics.net/lists/linux-omap/msg45505.html > >> >> > >> >> Thanks, > >> >> Vaibhav > >> >> > >> >> > These AM3xxx devices make my brain hurt. > >> >> > > >> >> > > Signed-off-by: Vaibhav Hiremath <hvaibhav@xxxxxx> > >> >> > > Cc: Kevin Hilman <khilman@xxxxxx> > >> >> > > Cc: Rajendra Nayak <rnayak@xxxxxx> > >> >> > > >> >> > [...] > >> >> > > >> >> > > diff --git a/arch/arm/mach-omap2/prminst44xx.c b/arch/arm/mach-omap2/prminst44xx.c > >> >> > > index 3d9894f..fcc4123 100644 > >> >> > > --- a/arch/arm/mach-omap2/prminst44xx.c > >> >> > > +++ b/arch/arm/mach-omap2/prminst44xx.c > >> >> > > @@ -19,6 +19,7 @@ > >> >> > > #include "common.h" > >> >> > > > >> >> > > #include "prm44xx.h" > >> >> > > +#include "prm33xx.h" > >> >> > > #include "prminst44xx.h" > >> >> > > #include "prm-regbits-44xx.h" > >> >> > > #include "prcm44xx.h" > >> >> > > @@ -31,6 +32,7 @@ static u32 _prm_bases[OMAP4_MAX_PRCM_PARTITIONS] = { > >> >> > > [OMAP4430_CM2_PARTITION] = 0, > >> >> > > [OMAP4430_SCRM_PARTITION] = 0, > >> >> > > [OMAP4430_PRCM_MPU_PARTITION] = OMAP2_L4_IO_ADDRESS(OMAP4430_PRCM_MPU_BASE), > >> >> > > + [AM33XX_PRM_PARTITION] = AM33XX_L4_WK_IO_ADDRESS(AM33XX_PRM_BASE), > >> >> > > }; > >> >> > > >> >> > I'm not crazy about just extending the "normal" OMAP4 table. > >> >> > >> >> If it is required then yes (with proper comment). > >> >> > >> >> > That would > >> >> > imply that with each OMAP4 derivatve we keep extending this table. > >> >> > > >> >> > >> >> I would say anyway we will end up adding > >> >> Cpu_is_xxx everywhere as we add new table for derivatives. > >> >> > >> >> > Instead, how about rename this to one to omap44xx_prm_bases[], then > >> >> > create a new one called am33xx_prm_bases[]. Then, at init time, assing > >> >> > _prm_bases to the right one based on cpu_is_. > >> >> > > >> >> > >> >> Just wanted to avoid cpu_is_xxxx check here. Will specific comment wouldn't > >> >> help here (I have clearly mentioned in patch description), may be in c file > >> >> it is required? > >> >> OR > >> >> you want to be clearly separate table for code readability. > >> >> > >> > > >> > Kevin, > >> > > >> > Any comments on this? Should I stick to what is implemented now? > >> > > >> > >> cpu_is_* checks are acceptable at init time, and we use them often to > >> initialize SoC-dependent tables/arrays etc. > >> > > Kevin, > > > > Sorry for delayed response, > > > > Here is the ugly part, which I was referring to - > > > > 1) "_prm_bases" variable is static variable to the prminst44xx.c > > > > 2) prminst44xx.c file doesn't contain any boot time __init function, > > So I have to either add exported __init function OR extern __prm_bases > > variable and initialize somewhere outside this file. > > > > 3) Even if I create 2 separate variables, for example, > > > > static u32 omap44xx_prm_bases[OMAP4_MAX_PRCM_PARTITIONS] = { > > ... > > }; > > > > static u32 omap33xx_prm_bases[AM33XX_MAX_PRCM_PARTITIONS] = { > > ... > > }; > > > > Makes it difficult and messy to handle inside below code, > > BUG_ON doesn't make sense from AM335x perspective. > > Then you can change the BUG_ON. > > static u32 omap44xx_max_partitions = ARRAY_SIZE(omap44xx_prm_bases) > static u32 am33xx_max_partitions = ARRAY_SIZE(am33xx_prm_bases) > static u32 max_partitions. > > At init time, assign max_partitions when you assign prm_bases, then > change the BUG_ON() to be something like: > > BUG_ON(part >= max_partitions || part == INVALID_PARTITION) > Kevin, Sorry for delayed response on this, was into volleyball match whole day... As I mentioned, I attempted to do this and below is output of git-diff. Also, I have boot tested it on AM335xEVM board. NOTE: Diff is created on top of my earlier submitted patches. ===============git diff============= diff --git a/arch/arm/mach-omap2/io.c b/arch/arm/mach-omap2/io.c index b3da178..f387857 100644 --- a/arch/arm/mach-omap2/io.c +++ b/arch/arm/mach-omap2/io.c @@ -39,6 +39,7 @@ #include <plat/omap-pm.h> #include "voltage.h" #include "powerdomain.h" +#include "prminst44xx.h" #include "clockdomain.h" #include <plat/omap_hwmod.h> @@ -475,6 +476,7 @@ void __init am33xx_init_early(void) omap2_set_globals_am33xx(); omap_common_init_early(); am33xx_voltagedomains_init(); + omap44xx_prminst_init(); am33xx_powerdomains_init(); am33xx_clockdomains_init(); am33xx_hwmod_init(); @@ -491,6 +493,7 @@ void __init omap4430_init_early(void) omap4xxx_check_features(); omap_common_init_early(); omap44xx_voltagedomains_init(); + omap44xx_prminst_init(); omap44xx_powerdomains_init(); omap44xx_clockdomains_init(); omap44xx_hwmod_init(); diff --git a/arch/arm/mach-omap2/prcm44xx.h b/arch/arm/mach-omap2/prcm44xx.h index d193f91..c539eb0 100644 --- a/arch/arm/mach-omap2/prcm44xx.h +++ b/arch/arm/mach-omap2/prcm44xx.h @@ -32,7 +32,7 @@ #define OMAP4430_SCRM_PARTITION 4 #define OMAP4430_PRCM_MPU_PARTITION 5 /* AM33XX PRCM is closer to OMAP4, so try to reuse all API's */ -#define AM33XX_PRM_PARTITION 6 +#define AM33XX_PRM_PARTITION 1 /* * OMAP4_MAX_PRCM_PARTITIONS: set to the highest value of the PRCM partition diff --git a/arch/arm/mach-omap2/prminst44xx.c b/arch/arm/mach-omap2/prminst44xx.c index fcc4123..d2748b7 100644 --- a/arch/arm/mach-omap2/prminst44xx.c +++ b/arch/arm/mach-omap2/prminst44xx.c @@ -25,20 +25,26 @@ #include "prcm44xx.h" #include "prcm_mpu44xx.h" -static u32 _prm_bases[OMAP4_MAX_PRCM_PARTITIONS] = { +static u32 *_prm_bases; +static u32 max_prm_partitions; + +static u32 omap44xx_prm_bases[] = { [OMAP4430_INVALID_PRCM_PARTITION] = 0, [OMAP4430_PRM_PARTITION] = OMAP2_L4_IO_ADDRESS(OMAP4430_PRM_BASE), [OMAP4430_CM1_PARTITION] = 0, [OMAP4430_CM2_PARTITION] = 0, [OMAP4430_SCRM_PARTITION] = 0, [OMAP4430_PRCM_MPU_PARTITION] = OMAP2_L4_IO_ADDRESS(OMAP4430_PRCM_MPU_BASE), - [AM33XX_PRM_PARTITION] = AM33XX_L4_WK_IO_ADDRESS(AM33XX_PRM_BASE), }; +static u32 am33xx_prm_bases[] = { + [OMAP4430_INVALID_PRCM_PARTITION] = 0, + [AM33XX_PRM_PARTITION] = AM33XX_L4_WK_IO_ADDRESS(AM33XX_PRM_BASE), +}; /* Read a register in a PRM instance */ u32 omap4_prminst_read_inst_reg(u8 part, s16 inst, u16 idx) { - BUG_ON(part >= OMAP4_MAX_PRCM_PARTITIONS || + BUG_ON(part >= max_prm_partitions || part == OMAP4430_INVALID_PRCM_PARTITION || !_prm_bases[part]); return __raw_readl(_prm_bases[part] + inst + idx); @@ -47,7 +53,7 @@ u32 omap4_prminst_read_inst_reg(u8 part, s16 inst, u16 idx) /* Write into a register in a PRM instance */ void omap4_prminst_write_inst_reg(u32 val, u8 part, s16 inst, u16 idx) { - BUG_ON(part >= OMAP4_MAX_PRCM_PARTITIONS || + BUG_ON(part >= max_prm_partitions || part == OMAP4430_INVALID_PRCM_PARTITION || !_prm_bases[part]); __raw_writel(val, _prm_bases[part] + inst + idx); @@ -175,3 +181,14 @@ void omap4_prminst_global_warm_sw_reset(void) OMAP4430_PRM_DEVICE_INST, OMAP4_PRM_RSTCTRL_OFFSET); } + +void __init omap44xx_prminst_init(void) +{ + if (cpu_is_omap44xx()) { + _prm_bases = omap44xx_prm_bases; + max_prm_partitions = ARRAY_SIZE(omap44xx_prm_bases); + } else if (cpu_is_am33xx()) { + _prm_bases = am33xx_prm_bases; + max_prm_partitions = ARRAY_SIZE(am33xx_prm_bases); + } +} diff --git a/arch/arm/mach-omap2/prminst44xx.h b/arch/arm/mach-omap2/prminst44xx.h index 46f2efb..9a44c68 100644 --- a/arch/arm/mach-omap2/prminst44xx.h +++ b/arch/arm/mach-omap2/prminst44xx.h @@ -29,5 +29,5 @@ extern int omap4_prminst_assert_hardreset(u8 shift, u8 part, s16 inst, u16 rstctrl_offs); extern int omap4_prminst_deassert_hardreset(u8 shift, u8 part, s16 inst, u16 rstctrl_offs); - +extern void __init omap44xx_prminst_init(void); #endif -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
