[PATCH-V2 0/3] arm:omap:omap4:Remove hardcoded reg-offs for PWRSTCTRL & PWRSTST

[PATCH-V2 0/3] arm:omap:omap4:Remove hardcoded reg-offs for PWRSTCTRL & PWRSTST

[Vaibhav Hiremath]

This patch series removes the existing hard-coded way of providing
offset to omap4_prminst_xxx API's and instead use offsets
provided in powerdomainsxxxx_data.
Also, hook up AM33XX device support to existing omap4 PRM code.

Background:
==========
PRM module in AM33XX is closer to OMAP4 PRM module, so it complete
sense to reuse all the code from existing OMAP4 implementation.
Having said that, ther is a catch here with respect to AM33XX device,

The register offset in PRM module is not consistent
across (crazy IP integration), for example,

PRM_XXX         PWRSTCTRL PWRSTST RSTCTRL RSTST
===============================================
PRM_PER_MOD:    0x0C,     0x08,   0x00,   0x04
PRM_WKUP_MOD:   0x04,     0x08,   0x00,   0x0C
PRM_MPU_MOD:    0x00,     0x04,   0x08,   NA
PRM_DEVICE_MOD: NA,       NA,     0x00,   0x08

So in order to reuse the existing OMAP4 code, we have to add
seperate entry for register offsets, especially
PWRSTCTRL & PWRSTST.

This patch series is dependent on recently submitted
voltage and powerdomain data patch for AM33xx device -

http://www.mail-archive.com/linux-omap@vger.kernel.org/msg60719.html

Changes from V1:
	- As per Kevin's comment, patch is split into logical
	  commits for ease of review.
	- Added specific comment for cpu_is_xxx check order change.


Vaibhav Hiremath (3):
  arm:omap:omap4: Remove hardcoded reg-offs for PWRSTCTRL & PWRSTST
  arm:omap:omap4: Maintain virtual addr in in _prm_bases table
  arm:omap:omap4: Hook-up am33xx support to existing prm code

 arch/arm/mach-omap2/omap_hwmod.c            |   44 +++++++++++++++++++--------
 arch/arm/mach-omap2/powerdomain.h           |    4 ++
 arch/arm/mach-omap2/powerdomain44xx.c       |   24 +++++++-------
 arch/arm/mach-omap2/powerdomains44xx_data.c |    8 +++++
 arch/arm/mach-omap2/prcm44xx.h              |    4 ++-
 arch/arm/mach-omap2/prminst44xx.c           |   11 ++++---
 6 files changed, 64 insertions(+), 31 deletions(-)

[PATCH-V2 1/3] arm:omap:omap4: Remove hardcoded reg-offs for PWRSTCTRL & PWRSTST

[Vaibhav Hiremath]

This patch removes the existing hard-coded way of providing
offset to omap4_prminst_xxx API's and instead use offsets
provided in powerdomainsxxxx_data.

Very much required for the new device AM33XX, where,

PRM module in AM33XX is closer to OMAP4 PRM module, so it makes
complete sense to reuse all the code from existing OMAP4 implementation.
Having said that, ther is a catch here with respect to AM33XX device,

The register offset in PRM module is not consistent
across (crazy IP integration), for example,

PRM_XXX         PWRSTCTRL PWRSTST RSTCTRL RSTST
===============================================
PRM_PER_MOD:    0x0C,     0x08,   0x00,   0x04
PRM_WKUP_MOD:   0x04,     0x08,   0x00,   0x0C
PRM_MPU_MOD:    0x00,     0x04,   0x08,   NA
PRM_DEVICE_MOD: NA,       NA,     0x00,   0x08

So in order to reuse the existing OMAP4 code, we have to add
seperate entry for register offsets, especially
PWRSTCTRL & PWRSTST.

Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com>
Cc: Kevin Hilman <khilman@ti.com>
Cc: Rajendra Nayak <rnayak@ti.com>
---
 arch/arm/mach-omap2/powerdomain.h           |    4 ++++
 arch/arm/mach-omap2/powerdomain44xx.c       |   24 ++++++++++++------------
 arch/arm/mach-omap2/powerdomains44xx_data.c |    8 ++++++++
 3 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/arch/arm/mach-omap2/powerdomain.h b/arch/arm/mach-omap2/powerdomain.h
index 0d72a8a..9ebb872 100644
--- a/arch/arm/mach-omap2/powerdomain.h
+++ b/arch/arm/mach-omap2/powerdomain.h
@@ -92,6 +92,8 @@ struct powerdomain;
  * @pwrdm_clkdms: Clockdomains in this powerdomain
  * @node: list_head linking all powerdomains
  * @voltdm_node: list_head linking all powerdomains in a voltagedomain
+ * @pwrstctrl_offs: XXX_PWRSTCTRL reg offset from prcm_offs
+ * @pwrstst_offs: XXX_PWRSTST reg offset from prcm_offs
  * @state:
  * @state_counter:
  * @timer:
@@ -121,6 +123,8 @@ struct powerdomain {
 	unsigned ret_logic_off_counter;
 	unsigned ret_mem_off_counter[PWRDM_MAX_MEM_BANKS];

+	u8 pwrstctrl_offs;
+	u8 pwrstst_offs;
 #ifdef CONFIG_PM_DEBUG
 	s64 timer;
 	s64 state_timer[PWRDM_MAX_PWRSTS];
diff --git a/arch/arm/mach-omap2/powerdomain44xx.c b/arch/arm/mach-omap2/powerdomain44xx.c
index a7880af..b088540 100644
--- a/arch/arm/mach-omap2/powerdomain44xx.c
+++ b/arch/arm/mach-omap2/powerdomain44xx.c
@@ -28,7 +28,7 @@ static int omap4_pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst)
 	omap4_prminst_rmw_inst_reg_bits(OMAP_POWERSTATE_MASK,
 					(pwrst << OMAP_POWERSTATE_SHIFT),
 					pwrdm->prcm_partition,
-					pwrdm->prcm_offs, OMAP4_PM_PWSTCTRL);
+					pwrdm->prcm_offs, pwrdm->pwrstctrl_offs);
 	return 0;
 }

@@ -37,7 +37,7 @@ static int omap4_pwrdm_read_next_pwrst(struct powerdomain *pwrdm)
 	u32 v;

 	v = omap4_prminst_read_inst_reg(pwrdm->prcm_partition, pwrdm->prcm_offs,
-					OMAP4_PM_PWSTCTRL);
+					pwrdm->pwrstctrl_offs);
 	v &= OMAP_POWERSTATE_MASK;
 	v >>= OMAP_POWERSTATE_SHIFT;

@@ -49,7 +49,7 @@ static int omap4_pwrdm_read_pwrst(struct powerdomain *pwrdm)
 	u32 v;

 	v = omap4_prminst_read_inst_reg(pwrdm->prcm_partition, pwrdm->prcm_offs,
-					OMAP4_PM_PWSTST);
+					pwrdm->pwrstst_offs);
 	v &= OMAP_POWERSTATEST_MASK;
 	v >>= OMAP_POWERSTATEST_SHIFT;

@@ -61,7 +61,7 @@ static int omap4_pwrdm_read_prev_pwrst(struct powerdomain *pwrdm)
 	u32 v;

 	v = omap4_prminst_read_inst_reg(pwrdm->prcm_partition, pwrdm->prcm_offs,
-					OMAP4_PM_PWSTST);
+					pwrdm->pwrstst_offs);
 	v &= OMAP4430_LASTPOWERSTATEENTERED_MASK;
 	v >>= OMAP4430_LASTPOWERSTATEENTERED_SHIFT;

@@ -73,7 +73,7 @@ static int omap4_pwrdm_set_lowpwrstchange(struct powerdomain *pwrdm)
 	omap4_prminst_rmw_inst_reg_bits(OMAP4430_LOWPOWERSTATECHANGE_MASK,
 					(1 << OMAP4430_LOWPOWERSTATECHANGE_SHIFT),
 					pwrdm->prcm_partition,
-					pwrdm->prcm_offs, OMAP4_PM_PWSTCTRL);
+					pwrdm->prcm_offs, pwrdm->pwrstctrl_offs);
 	return 0;
 }

@@ -82,7 +82,7 @@ static int omap4_pwrdm_clear_all_prev_pwrst(struct powerdomain *pwrdm)
 	omap4_prminst_rmw_inst_reg_bits(OMAP4430_LASTPOWERSTATEENTERED_MASK,
 					OMAP4430_LASTPOWERSTATEENTERED_MASK,
 					pwrdm->prcm_partition,
-					pwrdm->prcm_offs, OMAP4_PM_PWSTST);
+					pwrdm->prcm_offs, pwrdm->pwrstst_offs);
 	return 0;
 }

@@ -93,7 +93,7 @@ static int omap4_pwrdm_set_logic_retst(struct powerdomain *pwrdm, u8 pwrst)
 	v = pwrst << __ffs(OMAP4430_LOGICRETSTATE_MASK);
 	omap4_prminst_rmw_inst_reg_bits(OMAP4430_LOGICRETSTATE_MASK, v,
 					pwrdm->prcm_partition, pwrdm->prcm_offs,
-					OMAP4_PM_PWSTCTRL);
+					pwrdm->pwrstctrl_offs);

 	return 0;
 }
@@ -107,7 +107,7 @@ static int omap4_pwrdm_set_mem_onst(struct powerdomain *pwrdm, u8 bank,

 	omap4_prminst_rmw_inst_reg_bits(m, (pwrst << __ffs(m)),
 					pwrdm->prcm_partition, pwrdm->prcm_offs,
-					OMAP4_PM_PWSTCTRL);
+					pwrdm->pwrstctrl_offs);

 	return 0;
 }
@@ -131,7 +131,7 @@ static int omap4_pwrdm_read_logic_pwrst(struct powerdomain *pwrdm)
 	u32 v;

 	v = omap4_prminst_read_inst_reg(pwrdm->prcm_partition, pwrdm->prcm_offs,
-					OMAP4_PM_PWSTST);
+					pwrdm->pwrstst_offs);
 	v &= OMAP4430_LOGICSTATEST_MASK;
 	v >>= OMAP4430_LOGICSTATEST_SHIFT;

@@ -157,7 +157,7 @@ static int omap4_pwrdm_read_mem_pwrst(struct powerdomain *pwrdm, u8 bank)
 	m = omap2_pwrdm_get_mem_bank_stst_mask(bank);

 	v = omap4_prminst_read_inst_reg(pwrdm->prcm_partition, pwrdm->prcm_offs,
-					OMAP4_PM_PWSTST);
+					pwrdm->pwrstst_offs);
 	v &= m;
 	v >>= __ffs(m);

@@ -171,7 +171,7 @@ static int omap4_pwrdm_read_mem_retst(struct powerdomain *pwrdm, u8 bank)
 	m = omap2_pwrdm_get_mem_bank_retst_mask(bank);

 	v = omap4_prminst_read_inst_reg(pwrdm->prcm_partition, pwrdm->prcm_offs,
-					OMAP4_PM_PWSTCTRL);
+					pwrdm->pwrstctrl_offs);
 	v &= m;
 	v >>= __ffs(m);

@@ -191,7 +191,7 @@ static int omap4_pwrdm_wait_transition(struct powerdomain *pwrdm)
 	/* XXX Is this udelay() value meaningful? */
 	while ((omap4_prminst_read_inst_reg(pwrdm->prcm_partition,
 					    pwrdm->prcm_offs,
-					    OMAP4_PM_PWSTST) &
+					    pwrdm->pwrstst_offs) &
 		OMAP_INTRANSITION_MASK) &&
 	       (c++ < PWRDM_TRANSITION_BAILOUT))
 		udelay(1);
diff --git a/arch/arm/mach-omap2/powerdomains44xx_data.c b/arch/arm/mach-omap2/powerdomains44xx_data.c
index 704664c..e1f545b 100644
--- a/arch/arm/mach-omap2/powerdomains44xx_data.c
+++ b/arch/arm/mach-omap2/powerdomains44xx_data.c
@@ -352,7 +352,15 @@ static struct powerdomain *powerdomains_omap44xx[] __initdata = {

 void __init omap44xx_powerdomains_init(void)
 {
+	int i;
+
 	pwrdm_register_platform_funcs(&omap4_pwrdm_operations);
+	/* Initialise PRM reg offs to default value */
+	for (i = 0; powerdomains_omap44xx[i] != NULL; i++) {
+		struct powerdomain *pwrdm = powerdomains_omap44xx[i];
+		pwrdm->pwrstctrl_offs = OMAP4_PM_PWSTCTRL;
+		pwrdm->pwrstst_offs = OMAP4_PM_PWSTST;
+	}
 	pwrdm_register_pwrdms(powerdomains_omap44xx);
 	pwrdm_complete_init();
 }
--
1.7.0.4

[PATCH-V2 2/3] arm:omap:omap4: Maintain virtual addr in in _prm_bases table

[Vaibhav Hiremath]

Since, now omap4 prm code is reused for AM33XX device where,
PRM module fall under different domain than omap4.
So instead of maintaining phy addr for PRM partition
in _prm_bases[] table and then changing it to virt addr for r/w,
just directly maintain respective virt addr.

Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com>
Cc: Kevin Hilman <khilman@ti.com>
Cc: Rajendra Nayak <rnayak@ti.com>
---
 arch/arm/mach-omap2/prminst44xx.c |    9 ++++-----
 1 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-omap2/prminst44xx.c b/arch/arm/mach-omap2/prminst44xx.c
index f6de5bc..3d9894f 100644
--- a/arch/arm/mach-omap2/prminst44xx.c
+++ b/arch/arm/mach-omap2/prminst44xx.c
@@ -26,11 +26,11 @@

 static u32 _prm_bases[OMAP4_MAX_PRCM_PARTITIONS] = {
 	[OMAP4430_INVALID_PRCM_PARTITION]	= 0,
-	[OMAP4430_PRM_PARTITION]		= OMAP4430_PRM_BASE,
+	[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]		= OMAP4430_PRCM_MPU_BASE,
+	[OMAP4430_PRCM_MPU_PARTITION]		= OMAP2_L4_IO_ADDRESS(OMAP4430_PRCM_MPU_BASE),
 };

 /* Read a register in a PRM instance */
@@ -39,8 +39,7 @@ u32 omap4_prminst_read_inst_reg(u8 part, s16 inst, u16 idx)
 	BUG_ON(part >= OMAP4_MAX_PRCM_PARTITIONS ||
 	       part == OMAP4430_INVALID_PRCM_PARTITION ||
 	       !_prm_bases[part]);
-	return __raw_readl(OMAP2_L4_IO_ADDRESS(_prm_bases[part] + inst +
-					       idx));
+	return __raw_readl(_prm_bases[part] + inst + idx);
 }

 /* Write into a register in a PRM instance */
@@ -49,7 +48,7 @@ void omap4_prminst_write_inst_reg(u32 val, u8 part, s16 inst, u16 idx)
 	BUG_ON(part >= OMAP4_MAX_PRCM_PARTITIONS ||
 	       part == OMAP4430_INVALID_PRCM_PARTITION ||
 	       !_prm_bases[part]);
-	__raw_writel(val, OMAP2_L4_IO_ADDRESS(_prm_bases[part] + inst + idx));
+	__raw_writel(val, _prm_bases[part] + inst + idx);
 }

 /* Read-modify-write a register in PRM. Caller must lock */
--
1.7.0.4

[PATCH-V2 3/3] arm:omap:omap4: Hook-up am33xx support to existing prm code

[Vaibhav Hiremath]

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.

Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com>
Cc: Kevin Hilman <khilman@ti.com>
Cc: Rajendra Nayak <rnayak@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod.c  |   44 ++++++++++++++++++++++++++-----------
 arch/arm/mach-omap2/prcm44xx.h    |    4 ++-
 arch/arm/mach-omap2/prminst44xx.c |    2 +
 3 files changed, 36 insertions(+), 14 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 5192cab..3639f80 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -1288,14 +1288,20 @@ static int _assert_hardreset(struct omap_hwmod *oh, const char *name)
 	if (IS_ERR_VALUE(ret))
 		return ret;

-	if (cpu_is_omap24xx() || cpu_is_omap34xx())
-		return omap2_prm_assert_hardreset(oh->prcm.omap2.module_offs,
-						  ohri.rst_shift);
-	else if (cpu_is_omap44xx())
+	/*
+	 * In order to use omap4 prm code for am33xx family of devices,
+	 * first check cpu_is_am33xx here.
+	 *
+	 * Note: cpu_is_omap34xx is true for am33xx device as well.
+	 */
+	if (cpu_is_omap44xx() || cpu_is_am33xx())
 		return omap4_prminst_assert_hardreset(ohri.rst_shift,
 				  oh->clkdm->pwrdm.ptr->prcm_partition,
 				  oh->clkdm->pwrdm.ptr->prcm_offs,
 				  oh->prcm.omap4.rstctrl_offs);
+	else if (cpu_is_omap24xx() || cpu_is_omap34xx())
+		return omap2_prm_assert_hardreset(oh->prcm.omap2.module_offs,
+						  ohri.rst_shift);
 	else
 		return -EINVAL;
 }
@@ -1322,11 +1328,13 @@ static int _deassert_hardreset(struct omap_hwmod *oh, const char *name)
 	if (IS_ERR_VALUE(ret))
 		return ret;

-	if (cpu_is_omap24xx() || cpu_is_omap34xx()) {
-		ret = omap2_prm_deassert_hardreset(oh->prcm.omap2.module_offs,
-						   ohri.rst_shift,
-						   ohri.st_shift);
-	} else if (cpu_is_omap44xx()) {
+	/*
+	 * In order to use omap4 prm code for am33xx family of devices,
+	 * first check cpu_is_am33xx here.
+	 *
+	 * Note: cpu_is_omap34xx is true for am33xx device as well.
+	 */
+	if (cpu_is_omap44xx() || cpu_is_am33xx()) {
 		if (ohri.st_shift)
 			pr_err("omap_hwmod: %s: %s: hwmod data error: OMAP4 does not support st_shift\n",
 			       oh->name, name);
@@ -1334,6 +1342,10 @@ static int _deassert_hardreset(struct omap_hwmod *oh, const char *name)
 				  oh->clkdm->pwrdm.ptr->prcm_partition,
 				  oh->clkdm->pwrdm.ptr->prcm_offs,
 				  oh->prcm.omap4.rstctrl_offs);
+	} else if (cpu_is_omap24xx() || cpu_is_omap34xx()) {
+		ret = omap2_prm_deassert_hardreset(oh->prcm.omap2.module_offs,
+						   ohri.rst_shift,
+						   ohri.st_shift);
 	} else {
 		return -EINVAL;
 	}
@@ -1364,14 +1376,20 @@ static int _read_hardreset(struct omap_hwmod *oh, const char *name)
 	if (IS_ERR_VALUE(ret))
 		return ret;

-	if (cpu_is_omap24xx() || cpu_is_omap34xx()) {
-		return omap2_prm_is_hardreset_asserted(oh->prcm.omap2.module_offs,
-						       ohri.st_shift);
-	} else if (cpu_is_omap44xx()) {
+	/*
+	 * In order to use omap4 prm code for am33xx family of devices,
+	 * first check cpu_is_am33xx here.
+	 *
+	 * Note: cpu_is_omap34xx is true for am33xx device as well.
+	 */
+	if (cpu_is_omap44xx() || cpu_is_am33xx()) {
 		return omap4_prminst_is_hardreset_asserted(ohri.rst_shift,
 				  oh->clkdm->pwrdm.ptr->prcm_partition,
 				  oh->clkdm->pwrdm.ptr->prcm_offs,
 				  oh->prcm.omap4.rstctrl_offs);
+	} else if (cpu_is_omap24xx() || cpu_is_omap34xx()) {
+		return omap2_prm_is_hardreset_asserted(oh->prcm.omap2.module_offs,
+						       ohri.st_shift);
 	} else {
 		return -EINVAL;
 	}
diff --git a/arch/arm/mach-omap2/prcm44xx.h b/arch/arm/mach-omap2/prcm44xx.h
index 7334ffb..d193f91 100644
--- a/arch/arm/mach-omap2/prcm44xx.h
+++ b/arch/arm/mach-omap2/prcm44xx.h
@@ -31,12 +31,14 @@
 #define OMAP4430_CM2_PARTITION			3
 #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

 /*
  * OMAP4_MAX_PRCM_PARTITIONS: set to the highest value of the PRCM partition
  * IDs, plus one
  */
-#define OMAP4_MAX_PRCM_PARTITIONS		6
+#define OMAP4_MAX_PRCM_PARTITIONS		7


 #endif
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),
 };

 /* Read a register in a PRM instance */
--
1.7.0.4

Re: [PATCH-V2 3/3] arm:omap:omap4: Hook-up am33xx support to existing prm code

[Kevin Hilman]

Vaibhav Hiremath <hvaibhav@ti.com> 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?
These AM3xxx devices make my brain hurt.

> Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com>
> Cc: Kevin Hilman <khilman@ti.com>
> Cc: Rajendra Nayak <rnayak@ti.com>

[...]

> 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.  That would
imply that with each OMAP4 derivatve we keep extending this table.

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_.

Kevin

Re: [PATCH-V2 1/3] arm:omap:omap4: Remove hardcoded reg-offs for PWRSTCTRL & PWRSTST

[Kevin Hilman]

Vaibhav Hiremath <hvaibhav@ti.com> writes:

> This patch removes the existing hard-coded way of providing
> offset to omap4_prminst_xxx API's and instead use offsets
> provided in powerdomainsxxxx_data.
>
> Very much required for the new device AM33XX, where,
>
> PRM module in AM33XX is closer to OMAP4 PRM module, so it makes
> complete sense to reuse all the code from existing OMAP4 implementation.
> Having said that, ther is a catch here with respect to AM33XX device,
>
> The register offset in PRM module is not consistent
> across (crazy IP integration), for example,
>
> PRM_XXX         PWRSTCTRL PWRSTST RSTCTRL RSTST
> ===============================================
> PRM_PER_MOD:    0x0C,     0x08,   0x00,   0x04
> PRM_WKUP_MOD:   0x04,     0x08,   0x00,   0x0C
> PRM_MPU_MOD:    0x00,     0x04,   0x08,   NA
> PRM_DEVICE_MOD: NA,       NA,     0x00,   0x08
>
> So in order to reuse the existing OMAP4 code, we have to add
> seperate entry for register offsets, especially
> PWRSTCTRL & PWRSTST.
>
> Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com>
> Cc: Kevin Hilman <khilman@ti.com>
> Cc: Rajendra Nayak <rnayak@ti.com>

Reviewed-by: Kevin Hilman <khilman@ti.com>

Minor nit below, but after that, looks OK to me, but this should be
reviewed/merged by Paul/Benoit also.

[...]

>  void __init omap44xx_powerdomains_init(void)
>  {
> +	int i;
> +
>  	pwrdm_register_platform_funcs(&omap4_pwrdm_operations);
> +	/* Initialise PRM reg offs to default value */
> +	for (i = 0; powerdomains_omap44xx[i] != NULL; i++) {
> +		struct powerdomain *pwrdm = powerdomains_omap44xx[i];

insert blank line here

> +		pwrdm->pwrstctrl_offs = OMAP4_PM_PWSTCTRL;
> +		pwrdm->pwrstst_offs = OMAP4_PM_PWSTST;
> +	}
>  	pwrdm_register_pwrdms(powerdomains_omap44xx);
>  	pwrdm_complete_init();
>  }

Kevin

RE: [PATCH-V2 3/3] arm:omap:omap4: Hook-up am33xx support to existing prm code

[Hiremath, Vaibhav]

On Tue, Jan 10, 2012 at 23:39:22, Hilman, Kevin wrote:
> Vaibhav Hiremath <hvaibhav@ti.com> 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@ti.com>
> > Cc: Kevin Hilman <khilman@ti.com>
> > Cc: Rajendra Nayak <rnayak@ti.com>
> 
> [...]
> 
> > 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.

Thanks,
Vaibhav

> Kevin
> 
> 

RE: [PATCH-V2 1/3] arm:omap:omap4: Remove hardcoded reg-offs for PWRSTCTRL & PWRSTST

[Hiremath, Vaibhav]

On Tue, Jan 10, 2012 at 23:39:56, Hilman, Kevin wrote:
> Vaibhav Hiremath <hvaibhav@ti.com> writes:
> 
> > This patch removes the existing hard-coded way of providing
> > offset to omap4_prminst_xxx API's and instead use offsets
> > provided in powerdomainsxxxx_data.
> >
> > Very much required for the new device AM33XX, where,
> >
> > PRM module in AM33XX is closer to OMAP4 PRM module, so it makes
> > complete sense to reuse all the code from existing OMAP4 implementation.
> > Having said that, ther is a catch here with respect to AM33XX device,
> >
> > The register offset in PRM module is not consistent
> > across (crazy IP integration), for example,
> >
> > PRM_XXX         PWRSTCTRL PWRSTST RSTCTRL RSTST
> > ===============================================
> > PRM_PER_MOD:    0x0C,     0x08,   0x00,   0x04
> > PRM_WKUP_MOD:   0x04,     0x08,   0x00,   0x0C
> > PRM_MPU_MOD:    0x00,     0x04,   0x08,   NA
> > PRM_DEVICE_MOD: NA,       NA,     0x00,   0x08
> >
> > So in order to reuse the existing OMAP4 code, we have to add
> > seperate entry for register offsets, especially
> > PWRSTCTRL & PWRSTST.
> >
> > Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com>
> > Cc: Kevin Hilman <khilman@ti.com>
> > Cc: Rajendra Nayak <rnayak@ti.com>
> 
> Reviewed-by: Kevin Hilman <khilman@ti.com>
> 
Thanks Kevin,

> Minor nit below, but after that, looks OK to me, but this should be
> reviewed/merged by Paul/Benoit also.
> 
Paul/Benoit,

Can you also please review the patch series and let me know your comments?

Thanks,
Vaibhav
> [...]
> 
> >  void __init omap44xx_powerdomains_init(void)
> >  {
> > +	int i;
> > +
> >  	pwrdm_register_platform_funcs(&omap4_pwrdm_operations);
> > +	/* Initialise PRM reg offs to default value */
> > +	for (i = 0; powerdomains_omap44xx[i] != NULL; i++) {
> > +		struct powerdomain *pwrdm = powerdomains_omap44xx[i];
> 
> insert blank line here
> 

I will wait for Paul and Benoit comments and will fix it in next series.

Thanks,
Vaibhav

> > +		pwrdm->pwrstctrl_offs = OMAP4_PM_PWSTCTRL;
> > +		pwrdm->pwrstst_offs = OMAP4_PM_PWSTST;
> > +	}
> >  	pwrdm_register_pwrdms(powerdomains_omap44xx);
> >  	pwrdm_complete_init();
> >  }
> 
> Kevin
> 

RE: [PATCH-V2 1/3] arm:omap:omap4: Remove hardcoded reg-offs for PWRSTCTRL & PWRSTST

[Hiremath, Vaibhav]

On Tue, Jan 10, 2012 at 23:39:56, Hilman, Kevin wrote:
> Vaibhav Hiremath <hvaibhav@ti.com> writes:
> 
> > This patch removes the existing hard-coded way of providing
> > offset to omap4_prminst_xxx API's and instead use offsets
> > provided in powerdomainsxxxx_data.
> >
> > Very much required for the new device AM33XX, where,
> >
> > PRM module in AM33XX is closer to OMAP4 PRM module, so it makes
> > complete sense to reuse all the code from existing OMAP4 implementation.
> > Having said that, ther is a catch here with respect to AM33XX device,
> >
> > The register offset in PRM module is not consistent
> > across (crazy IP integration), for example,
> >
> > PRM_XXX         PWRSTCTRL PWRSTST RSTCTRL RSTST
> > ===============================================
> > PRM_PER_MOD:    0x0C,     0x08,   0x00,   0x04
> > PRM_WKUP_MOD:   0x04,     0x08,   0x00,   0x0C
> > PRM_MPU_MOD:    0x00,     0x04,   0x08,   NA
> > PRM_DEVICE_MOD: NA,       NA,     0x00,   0x08
> >
> > So in order to reuse the existing OMAP4 code, we have to add
> > seperate entry for register offsets, especially
> > PWRSTCTRL & PWRSTST.
> >
> > Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com>
> > Cc: Kevin Hilman <khilman@ti.com>
> > Cc: Rajendra Nayak <rnayak@ti.com>
> 
> Reviewed-by: Kevin Hilman <khilman@ti.com>
> 
> Minor nit below, but after that, looks OK to me, but this should be
> reviewed/merged by Paul/Benoit also.
> 

Paul and Benoit,

Can you please review this patch series and let me know the 
comments (if any)...

Thanks,
Vaibhav

> [...]
> 
> >  void __init omap44xx_powerdomains_init(void)
> >  {
> > +	int i;
> > +
> >  	pwrdm_register_platform_funcs(&omap4_pwrdm_operations);
> > +	/* Initialise PRM reg offs to default value */
> > +	for (i = 0; powerdomains_omap44xx[i] != NULL; i++) {
> > +		struct powerdomain *pwrdm = powerdomains_omap44xx[i];
> 
> insert blank line here
> 
> > +		pwrdm->pwrstctrl_offs = OMAP4_PM_PWSTCTRL;
> > +		pwrdm->pwrstst_offs = OMAP4_PM_PWSTST;
> > +	}
> >  	pwrdm_register_pwrdms(powerdomains_omap44xx);
> >  	pwrdm_complete_init();
> >  }
> 
> Kevin
> 

RE: [PATCH-V2 3/3] arm:omap:omap4: Hook-up am33xx support to existing prm code

[Hiremath, Vaibhav]

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@ti.com> 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@ti.com>
> > > Cc: Kevin Hilman <khilman@ti.com>
> > > Cc: Rajendra Nayak <rnayak@ti.com>
> > 
> > [...]
> > 
> > > 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?

Thanks,
Vaibhav


> Thanks,
> Vaibhav
> 
> > Kevin
> > 
> > 
> 
> 

Re: [PATCH-V2 3/3] arm:omap:omap4: Hook-up am33xx support to existing prm code

[Kevin Hilman]

"Hiremath, Vaibhav" <hvaibhav@ti.com> 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@ti.com> 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@ti.com>
>> > > Cc: Kevin Hilman <khilman@ti.com>
>> > > Cc: Rajendra Nayak <rnayak@ti.com>
>> > 
>> > [...]
>> > 
>> > > 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

RE: [PATCH-V2 3/3] arm:omap:omap4: Hook-up am33xx support to existing prm code

[Hiremath, Vaibhav]

On Tue, Jan 24, 2012 at 04:05:32, Hilman, Kevin wrote:
> "Hiremath, Vaibhav" <hvaibhav@ti.com> 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@ti.com> 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@ti.com>
> >> > > Cc: Kevin Hilman <khilman@ti.com>
> >> > > Cc: Rajendra Nayak <rnayak@ti.com>
> >> > 
> >> > [...]
> >> > 
> >> > > 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.

u32 omap4_prminst_read_inst_reg(u8 part, s16 inst, u16 idx)
{
	BUG_ON(part >= OMAP4_MAX_PRCM_PARTITIONS ||
		part == OMAP4430_INVALID_PRCM_PARTITION ||
		!_prm_bases[part]);
		return __raw_readl(_prm_bases[part] + inst + 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 ||
			part == OMAP4430_INVALID_PRCM_PARTITION ||
			!_prm_bases[part]);
		__raw_writel(val, _prm_bases[part] + inst + idx);
}



This was the reason I had extended existing omap3 __prm_bases to use it
for AM33xx.

Thanks,
Vaibhav

> Kevin
> 

Re: [PATCH-V2 3/3] arm:omap:omap4: Hook-up am33xx support to existing prm code

[Kevin Hilman]

"Hiremath, Vaibhav" <hvaibhav@ti.com> writes:

> On Tue, Jan 24, 2012 at 04:05:32, Hilman, Kevin wrote:
>> "Hiremath, Vaibhav" <hvaibhav@ti.com> 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@ti.com> 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@ti.com>
>> >> > > Cc: Kevin Hilman <khilman@ti.com>
>> >> > > Cc: Rajendra Nayak <rnayak@ti.com>
>> >> > 
>> >> > [...]
>> >> > 
>> >> > > 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

RE: [PATCH-V2 3/3] arm:omap:omap4: Hook-up am33xx support to existing prm code

[Hiremath, Vaibhav]

On Wed, Feb 01, 2012 at 23:03:56, Hilman, Kevin wrote:
> "Hiremath, Vaibhav" <hvaibhav@ti.com> writes:
> 
> > On Tue, Jan 24, 2012 at 04:05:32, Hilman, Kevin wrote:
> >> "Hiremath, Vaibhav" <hvaibhav@ti.com> 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@ti.com> 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@ti.com>
> >> >> > > Cc: Kevin Hilman <khilman@ti.com>
> >> >> > > Cc: Rajendra Nayak <rnayak@ti.com>
> >> >> > 
> >> >> > [...]
> >> >> > 
> >> >> > > 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,

Getting rid of BUG_ON was the least and trivial one, the issue is 1 & 2.

Let me atleast attempt to implement something on this, will update you.

Thanks,
Vaibhav


> Kevin
> 

Re: [PATCH-V2 3/3] arm:omap:omap4: Hook-up am33xx support to existing prm code

[Kevin Hilman]

"Hiremath, Vaibhav" <hvaibhav@ti.com> writes:

> On Wed, Feb 01, 2012 at 23:03:56, Hilman, Kevin wrote:
>> "Hiremath, Vaibhav" <hvaibhav@ti.com> writes:
>> 
>> > On Tue, Jan 24, 2012 at 04:05:32, Hilman, Kevin wrote:
>> >> "Hiremath, Vaibhav" <hvaibhav@ti.com> 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@ti.com> 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@ti.com>
>> >> >> > > Cc: Kevin Hilman <khilman@ti.com>
>> >> >> > > Cc: Rajendra Nayak <rnayak@ti.com>
>> >> >> > 
>> >> >> > [...]
>> >> >> > 
>> >> >> > > 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,
>
> Getting rid of BUG_ON was the least and trivial one, the issue is 1 & 2.

Oh, I didn't think those two were a problem since we do similiar things
throughout the kernel.  Consider using an initcall instead of calling it
from somwhere else, unless there are specific dependencies.

Kevin

> Let me atleast attempt to implement something on this, will update you.
>
> Thanks,
> Vaibhav
>
>
>> Kevin
>> 
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH-V2 3/3] arm:omap:omap4: Hook-up am33xx support to existing prm code

[Hiremath, Vaibhav]

On Wed, Feb 01, 2012 at 23:03:56, Hilman, Kevin wrote:
> "Hiremath, Vaibhav" <hvaibhav@ti.com> writes:
> 
> > On Tue, Jan 24, 2012 at 04:05:32, Hilman, Kevin wrote:
> >> "Hiremath, Vaibhav" <hvaibhav@ti.com> 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@ti.com> 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@ti.com>
> >> >> > > Cc: Kevin Hilman <khilman@ti.com>
> >> >> > > Cc: Rajendra Nayak <rnayak@ti.com>
> >> >> > 
> >> >> > [...]
> >> >> > 
> >> >> > > 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