ARM: OMAP4: Cleanup in cminst code

Similar to PRM code cleanup done recently to reuse prminst api's for AM33xx device, so in order to reuse the existing OMAP4 cminst code for AM33xx this patch adds,

- Boot time __init function, to initialize _cm_bases based on cpu_is_xxx
- Instead of maintaining phy addr for CM partition in _cm_bases[] table and then changing it to virt addr, directly maintain respective virt addr.

And also hook-up support for AM33xx device to existing cminst code.

Patch series is boot tested on AM335x EVM and AM37xEVM.

NOTE: This patch series is dependent on earlier submitted PRM
cleanup patch-series -
http://www.spinics.net/lists/linux-omap/msg62351.html

Vaibhav Hiremath (2):
ARM: OMAP4: cminst: Add boot time __init function for cminst
ARM: OMAP: am33xx: Hook-up am33xx support to existing cm code

 arch/arm/mach-omap2/cminst44xx.c |   38 +++++++++++++++++++++++++++++---------
 arch/arm/mach-omap2/cminst44xx.h |    1 +
 arch/arm/mach-omap2/io.c         |    3 +++
 arch/arm/mach-omap2/omap_hwmod.c |   23 +++++++++++++++--------
 4 files changed, 48 insertions(+), 17 deletions(-)

AM33xx CM module is closer to OMAP4 CM module, and in order to reuse cminst api's we have to address some of the differences like, base addresses and partitions. Unlike OMAP4 CM, AM33xx doesn't have any partitions and maintains only single partition.

So, in order to reuse the existing OMAP4 cminst code for AM33xx this patch adds,

- Boot time __init function, to initialize _cm_bases based on cpu_is_xxx
- Instead of maintaining phy addr for CM partition in _cm_bases[] table and then changing it to virt addr, directly maintain respective virt addr.

---
 arch/arm/mach-omap2/cminst44xx.c |   29 ++++++++++++++++++++---------
 arch/arm/mach-omap2/cminst44xx.h |    1 +
 arch/arm/mach-omap2/io.c         |    2 ++
 3 files changed, 23 insertions(+), 9 deletions(-)

index 6204dea..f709708 100644
@@ -49,13 +49,16 @@
 #define CLKCTRL_IDLEST_INTERFACE_IDLE		0x2
 #define CLKCTRL_IDLEST_DISABLED			0x3
 
-static u32 _cm_bases[OMAP4_MAX_PRCM_PARTITIONS] = {
+static u32 **_cm_bases;
+static u32 max_cm_partitions;
+
+static u32 *omap44xx_cm_bases[] = {
 	[OMAP4430_INVALID_PRCM_PARTITION]	= 0,
-	[OMAP4430_PRM_PARTITION]		= OMAP4430_PRM_BASE,
-	[OMAP4430_CM1_PARTITION]		= OMAP4430_CM1_BASE,
-	[OMAP4430_CM2_PARTITION]		= OMAP4430_CM2_BASE,
+	[OMAP4430_PRM_PARTITION]		= OMAP2_L4_IO_ADDRESS(OMAP4430_PRM_BASE),
+	[OMAP4430_CM1_PARTITION]		= OMAP2_L4_IO_ADDRESS(OMAP4430_CM1_BASE),
+	[OMAP4430_CM2_PARTITION]		= OMAP2_L4_IO_ADDRESS(OMAP4430_CM2_BASE),
 	[OMAP4430_SCRM_PARTITION]		= 0,
-	[OMAP4430_PRCM_MPU_PARTITION]		= OMAP4430_PRCM_MPU_BASE,
+	[OMAP4430_PRCM_MPU_PARTITION]		= OMAP2_L4_IO_ADDRESS(OMAP4430_PRCM_MPU_BASE),
 };
 
 /* Private functions */

@@ -103,19 +106,19 @@ static bool _is_module_ready(u8 part, u16 inst, s16 cdoffs, u16 clkctrl_offs)
 /* Read a register in a CM instance */
 u32 omap4_cminst_read_inst_reg(u8 part, s16 inst, u16 idx)
 {
-	BUG_ON(part >= OMAP4_MAX_PRCM_PARTITIONS ||
+	BUG_ON(part >= max_cm_partitions ||
 	       part == OMAP4430_INVALID_PRCM_PARTITION ||
 	       !_cm_bases[part]);
-	return __raw_readl(OMAP2_L4_IO_ADDRESS(_cm_bases[part] + inst + idx));
+	return __raw_readl(_cm_bases[part] + ((inst + idx)/sizeof(u32)));
 }
 
 /* Write into a register in a CM instance */
 void omap4_cminst_write_inst_reg(u32 val, u8 part, s16 inst, u16 idx)
 {
-	BUG_ON(part >= OMAP4_MAX_PRCM_PARTITIONS ||
+	BUG_ON(part >= max_cm_partitions ||
 	       part == OMAP4430_INVALID_PRCM_PARTITION ||
 	       !_cm_bases[part]);
-	__raw_writel(val, OMAP2_L4_IO_ADDRESS(_cm_bases[part] + inst + idx));
+	__raw_writel(val, _cm_bases[part] + ((inst + idx)/sizeof(u32)));
 }
 
 /* Read-modify-write a register in CM1. Caller must lock */

@@ -349,3 +352,11 @@ void omap4_cminst_module_disable(u8 part, u16 inst, s16 cdoffs,
 	v &= ~OMAP4430_MODULEMODE_MASK;
 	omap4_cminst_write_inst_reg(v, part, inst, clkctrl_offs);
 }
+
+void __init omap44xx_cminst_init(void)
+{
+	if (cpu_is_omap44xx()) {
+		_cm_bases = omap44xx_cm_bases;
+		max_cm_partitions = ARRAY_SIZE(omap44xx_cm_bases);
+	}
+}

index a018a73..3d1bde1 100644
@@ -63,4 +63,5 @@ extern u32 omap4_cminst_clear_inst_reg_bits(u32 bits, u8 part, s16 inst,
 extern u32 omap4_cminst_read_inst_reg_bits(u8 part, u16 inst, s16 idx,
 					   u32 mask);
 
+extern void __init omap44xx_cminst_init(void);
 #endif

index 50cf914..ebcc242 100644
@@ -40,6 +40,7 @@
 #include "voltage.h"
 #include "powerdomain.h"
 #include "prminst44xx.h"
+#include "cminst44xx.h"
 
 #include "clockdomain.h"
 #include <plat/omap_hwmod.h>

@@ -492,6 +493,7 @@ void __init omap4430_init_early(void)
 	omap44xx_voltagedomains_init();
 	omap44xx_prminst_init();
 	omap44xx_powerdomains_init();
+	omap44xx_cminst_init();
 	omap44xx_clockdomains_init();
 	omap44xx_hwmod_init();
 	omap_hwmod_init_postsetup();

--
1.7.0.4

Vaibhav> AM33xx CM module is closer to OMAP4 CM module, and Vaibhav> in order to reuse cminst api's we have to address Vaibhav> some of the differences like, base addresses and partitions. Vaibhav> Unlike OMAP4 CM, AM33xx doesn't have any partitions and Vaibhav> maintains only single partition.

Vaibhav> So, in order to reuse the existing OMAP4 cminst code Vaibhav> for AM33xx this patch adds,

Vaibhav> - Boot time __init function, to initialize _cm_bases
Vaibhav> based on cpu_is_xxx
Vaibhav> - Instead of maintaining phy addr for CM partition
Vaibhav> in _cm_bases[] table and then changing it to virt addr,
Vaibhav> directly maintain respective virt addr.

Vaibhav> @@ -103,19 +106,19 @@ static bool _is_module_ready(u8 part, u16 inst, s16 cdoffs, u16 clkctrl_offs)

 Vaibhav>  /* Read a register in a CM instance */
 Vaibhav>  u32 omap4_cminst_read_inst_reg(u8 part, s16 inst, u16 idx)
 Vaibhav>  {

Vaibhav> - BUG_ON(part >= OMAP4_MAX_PRCM_PARTITIONS ||
Vaibhav> + BUG_ON(part >= max_cm_partitions ||
Vaibhav> part == OMAP4430_INVALID_PRCM_PARTITION ||

How about dropping the OMAP4430_ prefix here as it's no longer 4430 specific?

Bye, Peter Korsgaard

Peter,

You could be right here, from this patch perspective, this change doesn't make sense.

But at the first place I am quite not sure, why this has been named as OMAP4430_? Vs some macros are named as OMAP4_.

So first we need to understand why it has been named this way, may be "Paul Walmsley" can comment on this. He is original author of this file.

Thanks,
Vaibhav

Hi,

Vaibhav> - BUG_ON(part >= OMAP4_MAX_PRCM_PARTITIONS ||
Vaibhav> + BUG_ON(part >= max_cm_partitions ||
Vaibhav> part == OMAP4430_INVALID_PRCM_PARTITION ||

Vaibhav> Peter,

Vaibhav> You could be right here, from this patch perspective, this Vaibhav> change doesn't make sense.

Vaibhav> But at the first place I am quite not sure, why this has been Vaibhav> named as OMAP4430_? Vs some macros are named as OMAP4_.

Vaibhav> So first we need to understand why it has been named this way, Vaibhav> may be "Paul Walmsley" can comment on this. He is original Vaibhav> author of this file.

As I read it, it doesn't have anything to do with any specific hardware, it's just to catch bugs when people forget to initialize the structure field:

arch/arm/mach-omap2/prcm44xx.h:
/*
* OMAP4 PRCM partition IDs
*
* The numbers and order are arbitrary, but 0 is reserved for the
* 'invalid' partition in case someone forgets to add a
* .prcm_partition field.
*/
#define OMAP4430_INVALID_PRCM_PARTITION 0

Bye, Peter Korsgaard

Reuse existing omap4 cminst code for am33xx device, add separate cm base table for am33xx device and initialize it during __init for future use.

Also, since cpu_is_omap34xx() check is true for am33xx family of devices, we must change the order of cpu_is_xxxx check, so first check for cpu_is_am33xx() to follow right execution path.

---
 arch/arm/mach-omap2/cminst44xx.c |    9 +++++++++
 arch/arm/mach-omap2/io.c         |    1 +
 arch/arm/mach-omap2/omap_hwmod.c |   23 +++++++++++++++--------
 3 files changed, 25 insertions(+), 8 deletions(-)

index f709708..402d35a 100644
@@ -31,6 +31,7 @@
 #include "cm-regbits-44xx.h"
 #include "prcm44xx.h"
 #include "prm44xx.h"
+#include "prm33xx.h"
 #include "prcm_mpu44xx.h"
 
 /*

@@ -61,6 +62,11 @@ static u32 *omap44xx_cm_bases[] = {
 	[OMAP4430_PRCM_MPU_PARTITION]		= OMAP2_L4_IO_ADDRESS(OMAP4430_PRCM_MPU_BASE),
 };
 
+static u32 *am33xx_cm_bases[] = {
+	[OMAP4430_INVALID_PRCM_PARTITION]	= 0,
+	[AM33XX_PRM_PARTITION]			= AM33XX_L4_WK_IO_ADDRESS(AM33XX_PRM_BASE),
+};
+
 /* Private functions */
 
 /**

@@ -358,5 +364,8 @@ void __init omap44xx_cminst_init(void)
 	if (cpu_is_omap44xx()) {
 		_cm_bases = omap44xx_cm_bases;
 		max_cm_partitions = ARRAY_SIZE(omap44xx_cm_bases);
+	} else if (cpu_is_am33xx()) {
+		_cm_bases =am33xx_cm_bases;
+		max_cm_partitions = ARRAY_SIZE(am33xx_cm_bases);
 	}
 }

index ebcc242..b318b44 100644
@@ -479,6 +479,7 @@ void __init am33xx_init_early(void)
 	am33xx_voltagedomains_init();
 	omap44xx_prminst_init();
 	am33xx_powerdomains_init();
+	omap44xx_cminst_init();
 	omap3xxx_clk_init();
 }
 #endif

index 612701e..e4ae811 100644
@@ -737,7 +737,7 @@ static void _disable_optional_clocks(struct omap_hwmod *oh)
 static void _enable_module(struct omap_hwmod *oh)
 {
 	/* The module mode does not exist prior OMAP4 */
-	if (cpu_is_omap24xx() || cpu_is_omap34xx())
+	if (!cpu_is_omap44xx() && !cpu_is_am33xx())
 		return;
 
 	if (!oh->clkdm || !oh->prcm.omap4.modulemode)

@@ -764,7 +764,7 @@ static void _enable_module(struct omap_hwmod *oh)
  */
 static int _omap4_wait_target_disable(struct omap_hwmod *oh)
 {
-	if (!cpu_is_omap44xx())
+	if (!cpu_is_omap44xx() && !cpu_is_am33xx())
 		return 0;
 
 	if (!oh)

@@ -794,7 +794,7 @@ static int _omap4_disable_module(struct omap_hwmod *oh)
 	int v;
 
 	/* The module mode does not exist prior OMAP4 */
-	if (!cpu_is_omap44xx())
+	if (!cpu_is_omap44xx() && !cpu_is_am33xx())
 		return -EINVAL;
 
 	if (!oh->clkdm || !oh->prcm.omap4.modulemode)

@@ -1219,11 +1219,13 @@ static int _wait_target_ready(struct omap_hwmod *oh)
 
 	/* XXX check clock enable states */
 
-	if (cpu_is_omap24xx() || cpu_is_omap34xx()) {
-		ret = omap2_cm_wait_module_ready(oh->prcm.omap2.module_offs,
-						 oh->prcm.omap2.idlest_reg_id,
-						 oh->prcm.omap2.idlest_idle_bit);
-	} else if (cpu_is_omap44xx()) {
+	/*
+	 * In order to use omap4 cminst 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 (!oh->clkdm)
 			return -EINVAL;
 

@@ -1231,6 +1233,11 @@ static int _wait_target_ready(struct omap_hwmod *oh)
 						     oh->clkdm->cm_inst,
 						     oh->clkdm->clkdm_offs,
 						     oh->prcm.omap4.clkctrl_offs);
+	} else if (cpu_is_omap24xx() || cpu_is_omap34xx()) {
+			ret = omap2_cm_wait_module_ready(
+						oh->prcm.omap2.module_offs,
+						oh->prcm.omap2.idlest_reg_id,
+						oh->prcm.omap2.idlest_idle_bit);
 	} else {
 		BUG();
 	};

1.7.0.4

Vaibhav> Reuse existing omap4 cminst code for am33xx device, Vaibhav> add separate cm base table for am33xx device and initialize Vaibhav> it during __init for future use.

Vaibhav> Also, since cpu_is_omap34xx() check is true for am33xx family of Vaibhav> devices, we must change the order of cpu_is_xxxx check, so first Vaibhav> check for cpu_is_am33xx() to follow right execution path.

 Vaibhav>  	if (cpu_is_omap44xx()) {
 Vaibhav>  		_cm_bases = omap44xx_cm_bases;
 Vaibhav>  		max_cm_partitions = ARRAY_SIZE(omap44xx_cm_bases);

Vaibhav> + } else if (cpu_is_am33xx()) {
Vaibhav> + _cm_bases =am33xx_cm_bases;

Space after '='.

Vaibhav> +++ b/arch/arm/mach-omap2/omap_hwmod.c
Vaibhav> @@ -737,7 +737,7 @@ static void _disable_optional_clocks(struct omap_hwmod *oh)

 Vaibhav>  static void _enable_module(struct omap_hwmod *oh)
 Vaibhav>  {
 Vaibhav>  	/* The module mode does not exist prior OMAP4 */

Vaibhav> - if (cpu_is_omap24xx() || cpu_is_omap34xx())
Vaibhav> + if (!cpu_is_omap44xx() && !cpu_is_am33xx())

Maybe update the comment as well?

Vaibhav> /* The module mode does not exist prior OMAP4 */
Vaibhav> - if (!cpu_is_omap44xx())
Vaibhav> + if (!cpu_is_omap44xx() && !cpu_is_am33xx())
Vaibhav> return -EINVAL;

Here as well.

Otherwise it looks good.

Bye, Peter Korsgaard

Oops...Will fix it.

Will update accordingly and submit the next version.

Thanks
Vaibhav