public inbox for linux-omap@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] OMAP3: PM: Removing redundant and potentially dangerous PRCM configration
@ 2009-10-15 23:14 Sripathy, Vishwanath
  2009-10-16 11:16 ` Gopinath, Thara
  2009-10-16 15:56 ` [PATCHV2] " Sripathy, Vishwanath
  0 siblings, 2 replies; 7+ messages in thread
From: Sripathy, Vishwanath @ 2009-10-15 23:14 UTC (permalink / raw)
  To: Sripathy, Vishwanath, linux-omap@vger.kernel.org, Kevin Hilman

As part of Core domain context restoration while coming out of off mode
there are some registers being restored which are not required to be restored.
ROM code should have restored them already. Overwriting some of them can have
potential side effect. Eg: CM_CLKEN_PLL register should not be written while dpll is locked.

Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
---

diff --git a/arch/arm/mach-omap2/prcm.c b/arch/arm/mach-omap2/prcm.c
index fb864cc..da16804 100644
--- a/arch/arm/mach-omap2/prcm.c
+++ b/arch/arm/mach-omap2/prcm.c
@@ -279,7 +279,6 @@ void omap3_prcm_save_context(void)
 	prcm_context.cm_sysconfig = __raw_readl(OMAP3430_CM_SYSCONFIG);
 	prcm_context.sgx_cm_clksel =
 			 cm_read_mod_reg(OMAP3430ES2_SGX_MOD, CM_CLKSEL);
-	prcm_context.wkup_cm_clksel = cm_read_mod_reg(WKUP_MOD, CM_CLKSEL);
 	prcm_context.dss_cm_clksel =
 			 cm_read_mod_reg(OMAP3430_DSS_MOD, CM_CLKSEL);
 	prcm_context.cam_cm_clksel =
@@ -296,8 +295,6 @@ void omap3_prcm_save_context(void)
 			cm_read_mod_reg(PLL_MOD, OMAP3430ES2_CM_CLKSEL4);
 	prcm_context.pll_cm_clksel5 =
 			 cm_read_mod_reg(PLL_MOD, OMAP3430ES2_CM_CLKSEL5);
-	prcm_context.pll_cm_clken =
-			cm_read_mod_reg(PLL_MOD, CM_CLKEN);
 	prcm_context.pll_cm_clken2 =
 			cm_read_mod_reg(PLL_MOD, OMAP3430ES2_CM_CLKEN2);
 	prcm_context.cm_polctrl = __raw_readl(OMAP3430_CM_POLCTRL);
@@ -343,8 +340,6 @@ void omap3_prcm_save_context(void)
 			 cm_read_mod_reg(OMAP3430_IVA2_MOD, CM_AUTOIDLE2);
 	prcm_context.mpu_cm_autoidle2 =
 			 cm_read_mod_reg(MPU_MOD, CM_AUTOIDLE2);
-	prcm_context.pll_cm_autoidle =
-			 cm_read_mod_reg(PLL_MOD, CM_AUTOIDLE);
 	prcm_context.iva2_cm_clkstctrl =
 			 cm_read_mod_reg(OMAP3430_IVA2_MOD, CM_CLKSTCTRL);
 	prcm_context.mpu_cm_clkstctrl =
@@ -436,7 +431,6 @@ void omap3_prcm_restore_context(void)
 	__raw_writel(prcm_context.cm_sysconfig, OMAP3430_CM_SYSCONFIG);
 	cm_write_mod_reg(prcm_context.sgx_cm_clksel, OMAP3430ES2_SGX_MOD,
 					 CM_CLKSEL);
-	cm_write_mod_reg(prcm_context.wkup_cm_clksel, WKUP_MOD, CM_CLKSEL);
 	cm_write_mod_reg(prcm_context.dss_cm_clksel, OMAP3430_DSS_MOD,
 					 CM_CLKSEL);
 	cm_write_mod_reg(prcm_context.cam_cm_clksel, OMAP3430_CAM_MOD,
@@ -453,7 +447,6 @@ void omap3_prcm_restore_context(void)
 					OMAP3430ES2_CM_CLKSEL4);
 	cm_write_mod_reg(prcm_context.pll_cm_clksel5, PLL_MOD,
 					 OMAP3430ES2_CM_CLKSEL5);
-	cm_write_mod_reg(prcm_context.pll_cm_clken, PLL_MOD, CM_CLKEN);
 	cm_write_mod_reg(prcm_context.pll_cm_clken2, PLL_MOD,
 					OMAP3430ES2_CM_CLKEN2);
 	__raw_writel(prcm_context.cm_polctrl, OMAP3430_CM_POLCTRL);
@@ -492,7 +485,6 @@ void omap3_prcm_restore_context(void)
 	cm_write_mod_reg(prcm_context.iva2_cm_autiidle2, OMAP3430_IVA2_MOD,
 					CM_AUTOIDLE2);
 	cm_write_mod_reg(prcm_context.mpu_cm_autoidle2, MPU_MOD, CM_AUTOIDLE2);
-	cm_write_mod_reg(prcm_context.pll_cm_autoidle, PLL_MOD, CM_AUTOIDLE);
 	cm_write_mod_reg(prcm_context.iva2_cm_clkstctrl, OMAP3430_IVA2_MOD,
 					CM_CLKSTCTRL);
 	cm_write_mod_reg(prcm_context.mpu_cm_clkstctrl, MPU_MOD, CM_CLKSTCTRL);

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* RE: [PATCH] OMAP3: PM: Removing redundant and potentially dangerous PRCM configration
  2009-10-15 23:14 [PATCH] OMAP3: PM: Removing redundant and potentially dangerous PRCM configration Sripathy, Vishwanath
@ 2009-10-16 11:16 ` Gopinath, Thara
  2009-10-16 12:58   ` Sripathy, Vishwanath
  2009-10-16 15:56 ` [PATCHV2] " Sripathy, Vishwanath
  1 sibling, 1 reply; 7+ messages in thread
From: Gopinath, Thara @ 2009-10-16 11:16 UTC (permalink / raw)
  To: Sripathy, Vishwanath, linux-omap@vger.kernel.org, Kevin Hilman



>>-----Original Message-----
>>From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of
>>Sripathy, Vishwanath
>>Sent: Friday, October 16, 2009 4:45 AM
>>To: Sripathy, Vishwanath; linux-omap@vger.kernel.org; Kevin Hilman
>>Subject: [PATCH] OMAP3: PM: Removing redundant and potentially dangerous PRCM configration
>>
>>As part of Core domain context restoration while coming out of off mode
>>there are some registers being restored which are not required to be restored.
>>ROM code should have restored them already. Overwriting some of them can have
>>potential side effect. Eg: CM_CLKEN_PLL register should not be written while dpll is locked.
>>
>>Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
>>---
>>
>>diff --git a/arch/arm/mach-omap2/prcm.c b/arch/arm/mach-omap2/prcm.c
>>index fb864cc..da16804 100644
>>--- a/arch/arm/mach-omap2/prcm.c
>>+++ b/arch/arm/mach-omap2/prcm.c
>>@@ -279,7 +279,6 @@ void omap3_prcm_save_context(void)
>> 	prcm_context.cm_sysconfig = __raw_readl(OMAP3430_CM_SYSCONFIG);
>> 	prcm_context.sgx_cm_clksel =
>> 			 cm_read_mod_reg(OMAP3430ES2_SGX_MOD, CM_CLKSEL);
>>-	prcm_context.wkup_cm_clksel = cm_read_mod_reg(WKUP_MOD, CM_CLKSEL);
>> 	prcm_context.dss_cm_clksel =
>> 			 cm_read_mod_reg(OMAP3430_DSS_MOD, CM_CLKSEL);
>> 	prcm_context.cam_cm_clksel =
>>@@ -296,8 +295,6 @@ void omap3_prcm_save_context(void)
>> 			cm_read_mod_reg(PLL_MOD, OMAP3430ES2_CM_CLKSEL4);
>> 	prcm_context.pll_cm_clksel5 =
>> 			 cm_read_mod_reg(PLL_MOD, OMAP3430ES2_CM_CLKSEL5);
>>-	prcm_context.pll_cm_clken =
>>-			cm_read_mod_reg(PLL_MOD, CM_CLKEN);
>> 	prcm_context.pll_cm_clken2 =
>> 			cm_read_mod_reg(PLL_MOD, OMAP3430ES2_CM_CLKEN2);
>> 	prcm_context.cm_polctrl = __raw_readl(OMAP3430_CM_POLCTRL);
>>@@ -343,8 +340,6 @@ void omap3_prcm_save_context(void)
>> 			 cm_read_mod_reg(OMAP3430_IVA2_MOD, CM_AUTOIDLE2);
>> 	prcm_context.mpu_cm_autoidle2 =
>> 			 cm_read_mod_reg(MPU_MOD, CM_AUTOIDLE2);
>>-	prcm_context.pll_cm_autoidle =
>>-			 cm_read_mod_reg(PLL_MOD, CM_AUTOIDLE);
>> 	prcm_context.iva2_cm_clkstctrl =
>> 			 cm_read_mod_reg(OMAP3430_IVA2_MOD, CM_CLKSTCTRL);
>> 	prcm_context.mpu_cm_clkstctrl =
>>@@ -436,7 +431,6 @@ void omap3_prcm_restore_context(void)
>> 	__raw_writel(prcm_context.cm_sysconfig, OMAP3430_CM_SYSCONFIG);
>> 	cm_write_mod_reg(prcm_context.sgx_cm_clksel, OMAP3430ES2_SGX_MOD,
>> 					 CM_CLKSEL);
>>-	cm_write_mod_reg(prcm_context.wkup_cm_clksel, WKUP_MOD, CM_CLKSEL);
>> 	cm_write_mod_reg(prcm_context.dss_cm_clksel, OMAP3430_DSS_MOD,
>> 					 CM_CLKSEL);
>> 	cm_write_mod_reg(prcm_context.cam_cm_clksel, OMAP3430_CAM_MOD,
>>@@ -453,7 +447,6 @@ void omap3_prcm_restore_context(void)
>> 					OMAP3430ES2_CM_CLKSEL4);
>> 	cm_write_mod_reg(prcm_context.pll_cm_clksel5, PLL_MOD,
>> 					 OMAP3430ES2_CM_CLKSEL5);
>>-	cm_write_mod_reg(prcm_context.pll_cm_clken, PLL_MOD, CM_CLKEN);
>> 	cm_write_mod_reg(prcm_context.pll_cm_clken2, PLL_MOD,
>> 					OMAP3430ES2_CM_CLKEN2);
>> 	__raw_writel(prcm_context.cm_polctrl, OMAP3430_CM_POLCTRL);
>>@@ -492,7 +485,6 @@ void omap3_prcm_restore_context(void)
>> 	cm_write_mod_reg(prcm_context.iva2_cm_autiidle2, OMAP3430_IVA2_MOD,
>> 					CM_AUTOIDLE2);
>> 	cm_write_mod_reg(prcm_context.mpu_cm_autoidle2, MPU_MOD, CM_AUTOIDLE2);
>>-	cm_write_mod_reg(prcm_context.pll_cm_autoidle, PLL_MOD, CM_AUTOIDLE);
>> 	cm_write_mod_reg(prcm_context.iva2_cm_clkstctrl, OMAP3430_IVA2_MOD,
>> 					CM_CLKSTCTRL);
>> 	cm_write_mod_reg(prcm_context.mpu_cm_clkstctrl, MPU_MOD, CM_CLKSTCTRL);

Why not remove these unwanted variables wkup_cm_clksel, pll_cm_clken, pll_cm_autoidle from the structure omap3_prcm_regs also?

Regards
Thara

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH] OMAP3: PM: Removing redundant and potentially dangerous PRCM configration
  2009-10-16 11:16 ` Gopinath, Thara
@ 2009-10-16 12:58   ` Sripathy, Vishwanath
  0 siblings, 0 replies; 7+ messages in thread
From: Sripathy, Vishwanath @ 2009-10-16 12:58 UTC (permalink / raw)
  To: Gopinath, Thara, linux-omap@vger.kernel.org, Kevin Hilman

Yes it can be removed. I will repost the patch with them removed.

Vishwa

-----Original Message-----
From: Gopinath, Thara 
Sent: Friday, October 16, 2009 6:16 AM
To: Sripathy, Vishwanath; linux-omap@vger.kernel.org; Kevin Hilman
Subject: RE: [PATCH] OMAP3: PM: Removing redundant and potentially dangerous PRCM configration



>>-----Original Message-----
>>From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of
>>Sripathy, Vishwanath
>>Sent: Friday, October 16, 2009 4:45 AM
>>To: Sripathy, Vishwanath; linux-omap@vger.kernel.org; Kevin Hilman
>>Subject: [PATCH] OMAP3: PM: Removing redundant and potentially dangerous PRCM configration
>>
>>As part of Core domain context restoration while coming out of off mode
>>there are some registers being restored which are not required to be restored.
>>ROM code should have restored them already. Overwriting some of them can have
>>potential side effect. Eg: CM_CLKEN_PLL register should not be written while dpll is locked.
>>
>>Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
>>---
>>
>>diff --git a/arch/arm/mach-omap2/prcm.c b/arch/arm/mach-omap2/prcm.c
>>index fb864cc..da16804 100644
>>--- a/arch/arm/mach-omap2/prcm.c
>>+++ b/arch/arm/mach-omap2/prcm.c
>>@@ -279,7 +279,6 @@ void omap3_prcm_save_context(void)
>> 	prcm_context.cm_sysconfig = __raw_readl(OMAP3430_CM_SYSCONFIG);
>> 	prcm_context.sgx_cm_clksel =
>> 			 cm_read_mod_reg(OMAP3430ES2_SGX_MOD, CM_CLKSEL);
>>-	prcm_context.wkup_cm_clksel = cm_read_mod_reg(WKUP_MOD, CM_CLKSEL);
>> 	prcm_context.dss_cm_clksel =
>> 			 cm_read_mod_reg(OMAP3430_DSS_MOD, CM_CLKSEL);
>> 	prcm_context.cam_cm_clksel =
>>@@ -296,8 +295,6 @@ void omap3_prcm_save_context(void)
>> 			cm_read_mod_reg(PLL_MOD, OMAP3430ES2_CM_CLKSEL4);
>> 	prcm_context.pll_cm_clksel5 =
>> 			 cm_read_mod_reg(PLL_MOD, OMAP3430ES2_CM_CLKSEL5);
>>-	prcm_context.pll_cm_clken =
>>-			cm_read_mod_reg(PLL_MOD, CM_CLKEN);
>> 	prcm_context.pll_cm_clken2 =
>> 			cm_read_mod_reg(PLL_MOD, OMAP3430ES2_CM_CLKEN2);
>> 	prcm_context.cm_polctrl = __raw_readl(OMAP3430_CM_POLCTRL);
>>@@ -343,8 +340,6 @@ void omap3_prcm_save_context(void)
>> 			 cm_read_mod_reg(OMAP3430_IVA2_MOD, CM_AUTOIDLE2);
>> 	prcm_context.mpu_cm_autoidle2 =
>> 			 cm_read_mod_reg(MPU_MOD, CM_AUTOIDLE2);
>>-	prcm_context.pll_cm_autoidle =
>>-			 cm_read_mod_reg(PLL_MOD, CM_AUTOIDLE);
>> 	prcm_context.iva2_cm_clkstctrl =
>> 			 cm_read_mod_reg(OMAP3430_IVA2_MOD, CM_CLKSTCTRL);
>> 	prcm_context.mpu_cm_clkstctrl =
>>@@ -436,7 +431,6 @@ void omap3_prcm_restore_context(void)
>> 	__raw_writel(prcm_context.cm_sysconfig, OMAP3430_CM_SYSCONFIG);
>> 	cm_write_mod_reg(prcm_context.sgx_cm_clksel, OMAP3430ES2_SGX_MOD,
>> 					 CM_CLKSEL);
>>-	cm_write_mod_reg(prcm_context.wkup_cm_clksel, WKUP_MOD, CM_CLKSEL);
>> 	cm_write_mod_reg(prcm_context.dss_cm_clksel, OMAP3430_DSS_MOD,
>> 					 CM_CLKSEL);
>> 	cm_write_mod_reg(prcm_context.cam_cm_clksel, OMAP3430_CAM_MOD,
>>@@ -453,7 +447,6 @@ void omap3_prcm_restore_context(void)
>> 					OMAP3430ES2_CM_CLKSEL4);
>> 	cm_write_mod_reg(prcm_context.pll_cm_clksel5, PLL_MOD,
>> 					 OMAP3430ES2_CM_CLKSEL5);
>>-	cm_write_mod_reg(prcm_context.pll_cm_clken, PLL_MOD, CM_CLKEN);
>> 	cm_write_mod_reg(prcm_context.pll_cm_clken2, PLL_MOD,
>> 					OMAP3430ES2_CM_CLKEN2);
>> 	__raw_writel(prcm_context.cm_polctrl, OMAP3430_CM_POLCTRL);
>>@@ -492,7 +485,6 @@ void omap3_prcm_restore_context(void)
>> 	cm_write_mod_reg(prcm_context.iva2_cm_autiidle2, OMAP3430_IVA2_MOD,
>> 					CM_AUTOIDLE2);
>> 	cm_write_mod_reg(prcm_context.mpu_cm_autoidle2, MPU_MOD, CM_AUTOIDLE2);
>>-	cm_write_mod_reg(prcm_context.pll_cm_autoidle, PLL_MOD, CM_AUTOIDLE);
>> 	cm_write_mod_reg(prcm_context.iva2_cm_clkstctrl, OMAP3430_IVA2_MOD,
>> 					CM_CLKSTCTRL);
>> 	cm_write_mod_reg(prcm_context.mpu_cm_clkstctrl, MPU_MOD, CM_CLKSTCTRL);

Why not remove these unwanted variables wkup_cm_clksel, pll_cm_clken, pll_cm_autoidle from the structure omap3_prcm_regs also?

Regards
Thara

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCHV2] OMAP3: PM: Removing redundant and potentially dangerous PRCM configration
  2009-10-15 23:14 [PATCH] OMAP3: PM: Removing redundant and potentially dangerous PRCM configration Sripathy, Vishwanath
  2009-10-16 11:16 ` Gopinath, Thara
@ 2009-10-16 15:56 ` Sripathy, Vishwanath
  2009-10-20 19:35   ` Kevin Hilman
  2009-11-16 11:11   ` [PATCHV3] " Sripathy, Vishwanath
  1 sibling, 2 replies; 7+ messages in thread
From: Sripathy, Vishwanath @ 2009-10-16 15:56 UTC (permalink / raw)
  To: Sripathy, Vishwanath, linux-omap@vger.kernel.org, Kevin Hilman

As part of Core domain context restoration while coming out of off mode
there are some registers being restored which are not required to be restored.
ROM code should have restored them already. Overwriting some of them can have
potential side effect. Eg: CM_CLKEN_PLL register should not be written while dpll is locked.

Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
---

diff --git a/arch/arm/mach-omap2/prcm.c b/arch/arm/mach-omap2/prcm.c
index fb864cc..88182a8 100644
--- a/arch/arm/mach-omap2/prcm.c
+++ b/arch/arm/mach-omap2/prcm.c
@@ -43,7 +43,6 @@ struct omap3_prcm_regs {
 	u32 iva2_cm_clksel2;
 	u32 cm_sysconfig;
 	u32 sgx_cm_clksel;
-	u32 wkup_cm_clksel;
 	u32 dss_cm_clksel;
 	u32 cam_cm_clksel;
 	u32 per_cm_clksel;
@@ -52,7 +51,6 @@ struct omap3_prcm_regs {
 	u32 pll_cm_autoidle2;
 	u32 pll_cm_clksel4;
 	u32 pll_cm_clksel5;
-	u32 pll_cm_clken;
 	u32 pll_cm_clken2;
 	u32 cm_polctrl;
 	u32 iva2_cm_fclken;
@@ -76,7 +74,6 @@ struct omap3_prcm_regs {
 	u32 usbhost_cm_iclken;
 	u32 iva2_cm_autiidle2;
 	u32 mpu_cm_autoidle2;
-	u32 pll_cm_autoidle;
 	u32 iva2_cm_clkstctrl;
 	u32 mpu_cm_clkstctrl;
 	u32 core_cm_clkstctrl;
@@ -279,7 +276,6 @@ void omap3_prcm_save_context(void)
 	prcm_context.cm_sysconfig = __raw_readl(OMAP3430_CM_SYSCONFIG);
 	prcm_context.sgx_cm_clksel =
 			 cm_read_mod_reg(OMAP3430ES2_SGX_MOD, CM_CLKSEL);
-	prcm_context.wkup_cm_clksel = cm_read_mod_reg(WKUP_MOD, CM_CLKSEL);
 	prcm_context.dss_cm_clksel =
 			 cm_read_mod_reg(OMAP3430_DSS_MOD, CM_CLKSEL);
 	prcm_context.cam_cm_clksel =
@@ -296,8 +292,6 @@ void omap3_prcm_save_context(void)
 			cm_read_mod_reg(PLL_MOD, OMAP3430ES2_CM_CLKSEL4);
 	prcm_context.pll_cm_clksel5 =
 			 cm_read_mod_reg(PLL_MOD, OMAP3430ES2_CM_CLKSEL5);
-	prcm_context.pll_cm_clken =
-			cm_read_mod_reg(PLL_MOD, CM_CLKEN);
 	prcm_context.pll_cm_clken2 =
 			cm_read_mod_reg(PLL_MOD, OMAP3430ES2_CM_CLKEN2);
 	prcm_context.cm_polctrl = __raw_readl(OMAP3430_CM_POLCTRL);
@@ -343,8 +337,6 @@ void omap3_prcm_save_context(void)
 			 cm_read_mod_reg(OMAP3430_IVA2_MOD, CM_AUTOIDLE2);
 	prcm_context.mpu_cm_autoidle2 =
 			 cm_read_mod_reg(MPU_MOD, CM_AUTOIDLE2);
-	prcm_context.pll_cm_autoidle =
-			 cm_read_mod_reg(PLL_MOD, CM_AUTOIDLE);
 	prcm_context.iva2_cm_clkstctrl =
 			 cm_read_mod_reg(OMAP3430_IVA2_MOD, CM_CLKSTCTRL);
 	prcm_context.mpu_cm_clkstctrl =
@@ -436,7 +428,6 @@ void omap3_prcm_restore_context(void)
 	__raw_writel(prcm_context.cm_sysconfig, OMAP3430_CM_SYSCONFIG);
 	cm_write_mod_reg(prcm_context.sgx_cm_clksel, OMAP3430ES2_SGX_MOD,
 					 CM_CLKSEL);
-	cm_write_mod_reg(prcm_context.wkup_cm_clksel, WKUP_MOD, CM_CLKSEL);
 	cm_write_mod_reg(prcm_context.dss_cm_clksel, OMAP3430_DSS_MOD,
 					 CM_CLKSEL);
 	cm_write_mod_reg(prcm_context.cam_cm_clksel, OMAP3430_CAM_MOD,
@@ -453,7 +444,6 @@ void omap3_prcm_restore_context(void)
 					OMAP3430ES2_CM_CLKSEL4);
 	cm_write_mod_reg(prcm_context.pll_cm_clksel5, PLL_MOD,
 					 OMAP3430ES2_CM_CLKSEL5);
-	cm_write_mod_reg(prcm_context.pll_cm_clken, PLL_MOD, CM_CLKEN);
 	cm_write_mod_reg(prcm_context.pll_cm_clken2, PLL_MOD,
 					OMAP3430ES2_CM_CLKEN2);
 	__raw_writel(prcm_context.cm_polctrl, OMAP3430_CM_POLCTRL);
@@ -492,7 +482,6 @@ void omap3_prcm_restore_context(void)
 	cm_write_mod_reg(prcm_context.iva2_cm_autiidle2, OMAP3430_IVA2_MOD,
 					CM_AUTOIDLE2);
 	cm_write_mod_reg(prcm_context.mpu_cm_autoidle2, MPU_MOD, CM_AUTOIDLE2);
-	cm_write_mod_reg(prcm_context.pll_cm_autoidle, PLL_MOD, CM_AUTOIDLE);
 	cm_write_mod_reg(prcm_context.iva2_cm_clkstctrl, OMAP3430_IVA2_MOD,
 					CM_CLKSTCTRL);
 	cm_write_mod_reg(prcm_context.mpu_cm_clkstctrl, MPU_MOD, CM_CLKSTCTRL);

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCHV2] OMAP3: PM: Removing redundant and potentially dangerous PRCM configration
  2009-10-16 15:56 ` [PATCHV2] " Sripathy, Vishwanath
@ 2009-10-20 19:35   ` Kevin Hilman
  2009-11-16 11:11   ` [PATCHV3] " Sripathy, Vishwanath
  1 sibling, 0 replies; 7+ messages in thread
From: Kevin Hilman @ 2009-10-20 19:35 UTC (permalink / raw)
  To: Sripathy, Vishwanath; +Cc: linux-omap@vger.kernel.org

"Sripathy, Vishwanath" <vishwanath.bs@ti.com> writes:

> As part of Core domain context restoration while coming out of off mode
> there are some registers being restored which are not required to be restored.
> ROM code should have restored them already. 

Saying "should have restored" is not very convincing.  :)

I'm hoping that you mean ROM code *has* restored them already.  In
which case, the changelog should read something like

...not required to be restored because ROM code will have already
restored them.

> Overwriting some of them can have potential side effect. Eg:
> CM_CLKEN_PLL register should not be written while dpll is locked.
>
> Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>

While updating the changelog, can you report how OFF-mode was tested
(suspend, off-while-idle, etc.)  and on which platforms.

Otherwise, patch looks fine.

Thanks,

Kevin

> ---
>
> diff --git a/arch/arm/mach-omap2/prcm.c b/arch/arm/mach-omap2/prcm.c
> index fb864cc..88182a8 100644
> --- a/arch/arm/mach-omap2/prcm.c
> +++ b/arch/arm/mach-omap2/prcm.c
> @@ -43,7 +43,6 @@ struct omap3_prcm_regs {
>  	u32 iva2_cm_clksel2;
>  	u32 cm_sysconfig;
>  	u32 sgx_cm_clksel;
> -	u32 wkup_cm_clksel;
>  	u32 dss_cm_clksel;
>  	u32 cam_cm_clksel;
>  	u32 per_cm_clksel;
> @@ -52,7 +51,6 @@ struct omap3_prcm_regs {
>  	u32 pll_cm_autoidle2;
>  	u32 pll_cm_clksel4;
>  	u32 pll_cm_clksel5;
> -	u32 pll_cm_clken;
>  	u32 pll_cm_clken2;
>  	u32 cm_polctrl;
>  	u32 iva2_cm_fclken;
> @@ -76,7 +74,6 @@ struct omap3_prcm_regs {
>  	u32 usbhost_cm_iclken;
>  	u32 iva2_cm_autiidle2;
>  	u32 mpu_cm_autoidle2;
> -	u32 pll_cm_autoidle;
>  	u32 iva2_cm_clkstctrl;
>  	u32 mpu_cm_clkstctrl;
>  	u32 core_cm_clkstctrl;
> @@ -279,7 +276,6 @@ void omap3_prcm_save_context(void)
>  	prcm_context.cm_sysconfig = __raw_readl(OMAP3430_CM_SYSCONFIG);
>  	prcm_context.sgx_cm_clksel =
>  			 cm_read_mod_reg(OMAP3430ES2_SGX_MOD, CM_CLKSEL);
> -	prcm_context.wkup_cm_clksel = cm_read_mod_reg(WKUP_MOD, CM_CLKSEL);
>  	prcm_context.dss_cm_clksel =
>  			 cm_read_mod_reg(OMAP3430_DSS_MOD, CM_CLKSEL);
>  	prcm_context.cam_cm_clksel =
> @@ -296,8 +292,6 @@ void omap3_prcm_save_context(void)
>  			cm_read_mod_reg(PLL_MOD, OMAP3430ES2_CM_CLKSEL4);
>  	prcm_context.pll_cm_clksel5 =
>  			 cm_read_mod_reg(PLL_MOD, OMAP3430ES2_CM_CLKSEL5);
> -	prcm_context.pll_cm_clken =
> -			cm_read_mod_reg(PLL_MOD, CM_CLKEN);
>  	prcm_context.pll_cm_clken2 =
>  			cm_read_mod_reg(PLL_MOD, OMAP3430ES2_CM_CLKEN2);
>  	prcm_context.cm_polctrl = __raw_readl(OMAP3430_CM_POLCTRL);
> @@ -343,8 +337,6 @@ void omap3_prcm_save_context(void)
>  			 cm_read_mod_reg(OMAP3430_IVA2_MOD, CM_AUTOIDLE2);
>  	prcm_context.mpu_cm_autoidle2 =
>  			 cm_read_mod_reg(MPU_MOD, CM_AUTOIDLE2);
> -	prcm_context.pll_cm_autoidle =
> -			 cm_read_mod_reg(PLL_MOD, CM_AUTOIDLE);
>  	prcm_context.iva2_cm_clkstctrl =
>  			 cm_read_mod_reg(OMAP3430_IVA2_MOD, CM_CLKSTCTRL);
>  	prcm_context.mpu_cm_clkstctrl =
> @@ -436,7 +428,6 @@ void omap3_prcm_restore_context(void)
>  	__raw_writel(prcm_context.cm_sysconfig, OMAP3430_CM_SYSCONFIG);
>  	cm_write_mod_reg(prcm_context.sgx_cm_clksel, OMAP3430ES2_SGX_MOD,
>  					 CM_CLKSEL);
> -	cm_write_mod_reg(prcm_context.wkup_cm_clksel, WKUP_MOD, CM_CLKSEL);
>  	cm_write_mod_reg(prcm_context.dss_cm_clksel, OMAP3430_DSS_MOD,
>  					 CM_CLKSEL);
>  	cm_write_mod_reg(prcm_context.cam_cm_clksel, OMAP3430_CAM_MOD,
> @@ -453,7 +444,6 @@ void omap3_prcm_restore_context(void)
>  					OMAP3430ES2_CM_CLKSEL4);
>  	cm_write_mod_reg(prcm_context.pll_cm_clksel5, PLL_MOD,
>  					 OMAP3430ES2_CM_CLKSEL5);
> -	cm_write_mod_reg(prcm_context.pll_cm_clken, PLL_MOD, CM_CLKEN);
>  	cm_write_mod_reg(prcm_context.pll_cm_clken2, PLL_MOD,
>  					OMAP3430ES2_CM_CLKEN2);
>  	__raw_writel(prcm_context.cm_polctrl, OMAP3430_CM_POLCTRL);
> @@ -492,7 +482,6 @@ void omap3_prcm_restore_context(void)
>  	cm_write_mod_reg(prcm_context.iva2_cm_autiidle2, OMAP3430_IVA2_MOD,
>  					CM_AUTOIDLE2);
>  	cm_write_mod_reg(prcm_context.mpu_cm_autoidle2, MPU_MOD, CM_AUTOIDLE2);
> -	cm_write_mod_reg(prcm_context.pll_cm_autoidle, PLL_MOD, CM_AUTOIDLE);
>  	cm_write_mod_reg(prcm_context.iva2_cm_clkstctrl, OMAP3430_IVA2_MOD,
>  					CM_CLKSTCTRL);
>  	cm_write_mod_reg(prcm_context.mpu_cm_clkstctrl, MPU_MOD, CM_CLKSTCTRL);

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCHV3] OMAP3: PM: Removing redundant and potentially dangerous PRCM configration
  2009-10-16 15:56 ` [PATCHV2] " Sripathy, Vishwanath
  2009-10-20 19:35   ` Kevin Hilman
@ 2009-11-16 11:11   ` Sripathy, Vishwanath
  2009-11-16 16:08     ` Kevin Hilman
  1 sibling, 1 reply; 7+ messages in thread
From: Sripathy, Vishwanath @ 2009-11-16 11:11 UTC (permalink / raw)
  To: Sripathy, Vishwanath, linux-omap@vger.kernel.org, Kevin Hilman

As part of Core domain context restoration while coming out of off mode
there are some registers being restored which are not required to be restored.
ROM code will have restored them already. Overwriting some of them can have
potential side effect. Eg: CM_CLKEN_PLL register should not be written while dpll is locked.
Tested on OMAP 3430 SDP for suspend/resume and off mode with sleep_while_idle enabled.

Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
---

diff --git a/arch/arm/mach-omap2/prcm.c b/arch/arm/mach-omap2/prcm.c
index fb864cc..88182a8 100644
--- a/arch/arm/mach-omap2/prcm.c
+++ b/arch/arm/mach-omap2/prcm.c
@@ -43,7 +43,6 @@ struct omap3_prcm_regs {
 	u32 iva2_cm_clksel2;
 	u32 cm_sysconfig;
 	u32 sgx_cm_clksel;
-	u32 wkup_cm_clksel;
 	u32 dss_cm_clksel;
 	u32 cam_cm_clksel;
 	u32 per_cm_clksel;
@@ -52,7 +51,6 @@ struct omap3_prcm_regs {
 	u32 pll_cm_autoidle2;
 	u32 pll_cm_clksel4;
 	u32 pll_cm_clksel5;
-	u32 pll_cm_clken;
 	u32 pll_cm_clken2;
 	u32 cm_polctrl;
 	u32 iva2_cm_fclken;
@@ -76,7 +74,6 @@ struct omap3_prcm_regs {
 	u32 usbhost_cm_iclken;
 	u32 iva2_cm_autiidle2;
 	u32 mpu_cm_autoidle2;
-	u32 pll_cm_autoidle;
 	u32 iva2_cm_clkstctrl;
 	u32 mpu_cm_clkstctrl;
 	u32 core_cm_clkstctrl;
@@ -279,7 +276,6 @@ void omap3_prcm_save_context(void)
 	prcm_context.cm_sysconfig = __raw_readl(OMAP3430_CM_SYSCONFIG);
 	prcm_context.sgx_cm_clksel =
 			 cm_read_mod_reg(OMAP3430ES2_SGX_MOD, CM_CLKSEL);
-	prcm_context.wkup_cm_clksel = cm_read_mod_reg(WKUP_MOD, CM_CLKSEL);
 	prcm_context.dss_cm_clksel =
 			 cm_read_mod_reg(OMAP3430_DSS_MOD, CM_CLKSEL);
 	prcm_context.cam_cm_clksel =
@@ -296,8 +292,6 @@ void omap3_prcm_save_context(void)
 			cm_read_mod_reg(PLL_MOD, OMAP3430ES2_CM_CLKSEL4);
 	prcm_context.pll_cm_clksel5 =
 			 cm_read_mod_reg(PLL_MOD, OMAP3430ES2_CM_CLKSEL5);
-	prcm_context.pll_cm_clken =
-			cm_read_mod_reg(PLL_MOD, CM_CLKEN);
 	prcm_context.pll_cm_clken2 =
 			cm_read_mod_reg(PLL_MOD, OMAP3430ES2_CM_CLKEN2);
 	prcm_context.cm_polctrl = __raw_readl(OMAP3430_CM_POLCTRL);
@@ -343,8 +337,6 @@ void omap3_prcm_save_context(void)
 			 cm_read_mod_reg(OMAP3430_IVA2_MOD, CM_AUTOIDLE2);
 	prcm_context.mpu_cm_autoidle2 =
 			 cm_read_mod_reg(MPU_MOD, CM_AUTOIDLE2);
-	prcm_context.pll_cm_autoidle =
-			 cm_read_mod_reg(PLL_MOD, CM_AUTOIDLE);
 	prcm_context.iva2_cm_clkstctrl =
 			 cm_read_mod_reg(OMAP3430_IVA2_MOD, CM_CLKSTCTRL);
 	prcm_context.mpu_cm_clkstctrl =
@@ -436,7 +428,6 @@ void omap3_prcm_restore_context(void)
 	__raw_writel(prcm_context.cm_sysconfig, OMAP3430_CM_SYSCONFIG);
 	cm_write_mod_reg(prcm_context.sgx_cm_clksel, OMAP3430ES2_SGX_MOD,
 					 CM_CLKSEL);
-	cm_write_mod_reg(prcm_context.wkup_cm_clksel, WKUP_MOD, CM_CLKSEL);
 	cm_write_mod_reg(prcm_context.dss_cm_clksel, OMAP3430_DSS_MOD,
 					 CM_CLKSEL);
 	cm_write_mod_reg(prcm_context.cam_cm_clksel, OMAP3430_CAM_MOD,
@@ -453,7 +444,6 @@ void omap3_prcm_restore_context(void)
 					OMAP3430ES2_CM_CLKSEL4);
 	cm_write_mod_reg(prcm_context.pll_cm_clksel5, PLL_MOD,
 					 OMAP3430ES2_CM_CLKSEL5);
-	cm_write_mod_reg(prcm_context.pll_cm_clken, PLL_MOD, CM_CLKEN);
 	cm_write_mod_reg(prcm_context.pll_cm_clken2, PLL_MOD,
 					OMAP3430ES2_CM_CLKEN2);
 	__raw_writel(prcm_context.cm_polctrl, OMAP3430_CM_POLCTRL);
@@ -492,7 +482,6 @@ void omap3_prcm_restore_context(void)
 	cm_write_mod_reg(prcm_context.iva2_cm_autiidle2, OMAP3430_IVA2_MOD,
 					CM_AUTOIDLE2);
 	cm_write_mod_reg(prcm_context.mpu_cm_autoidle2, MPU_MOD, CM_AUTOIDLE2);
-	cm_write_mod_reg(prcm_context.pll_cm_autoidle, PLL_MOD, CM_AUTOIDLE);
 	cm_write_mod_reg(prcm_context.iva2_cm_clkstctrl, OMAP3430_IVA2_MOD,
 					CM_CLKSTCTRL);
 	cm_write_mod_reg(prcm_context.mpu_cm_clkstctrl, MPU_MOD, CM_CLKSTCTRL);

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCHV3] OMAP3: PM: Removing redundant and potentially dangerous PRCM configration
  2009-11-16 11:11   ` [PATCHV3] " Sripathy, Vishwanath
@ 2009-11-16 16:08     ` Kevin Hilman
  0 siblings, 0 replies; 7+ messages in thread
From: Kevin Hilman @ 2009-11-16 16:08 UTC (permalink / raw)
  To: Sripathy, Vishwanath; +Cc: linux-omap@vger.kernel.org

"Sripathy, Vishwanath" <vishwanath.bs@ti.com> writes:

> As part of Core domain context restoration while coming out of off mode
> there are some registers being restored which are not required to be restored.
> ROM code will have restored them already. Overwriting some of them can have
> potential side effect. Eg: CM_CLKEN_PLL register should not be written while dpll is locked.
> Tested on OMAP 3430 SDP for suspend/resume and off mode with sleep_while_idle enabled.
>
> Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>

Thanks, applying to PM branch.

Kevin

> ---
>
> diff --git a/arch/arm/mach-omap2/prcm.c b/arch/arm/mach-omap2/prcm.c
> index fb864cc..88182a8 100644
> --- a/arch/arm/mach-omap2/prcm.c
> +++ b/arch/arm/mach-omap2/prcm.c
> @@ -43,7 +43,6 @@ struct omap3_prcm_regs {
>  	u32 iva2_cm_clksel2;
>  	u32 cm_sysconfig;
>  	u32 sgx_cm_clksel;
> -	u32 wkup_cm_clksel;
>  	u32 dss_cm_clksel;
>  	u32 cam_cm_clksel;
>  	u32 per_cm_clksel;
> @@ -52,7 +51,6 @@ struct omap3_prcm_regs {
>  	u32 pll_cm_autoidle2;
>  	u32 pll_cm_clksel4;
>  	u32 pll_cm_clksel5;
> -	u32 pll_cm_clken;
>  	u32 pll_cm_clken2;
>  	u32 cm_polctrl;
>  	u32 iva2_cm_fclken;
> @@ -76,7 +74,6 @@ struct omap3_prcm_regs {
>  	u32 usbhost_cm_iclken;
>  	u32 iva2_cm_autiidle2;
>  	u32 mpu_cm_autoidle2;
> -	u32 pll_cm_autoidle;
>  	u32 iva2_cm_clkstctrl;
>  	u32 mpu_cm_clkstctrl;
>  	u32 core_cm_clkstctrl;
> @@ -279,7 +276,6 @@ void omap3_prcm_save_context(void)
>  	prcm_context.cm_sysconfig = __raw_readl(OMAP3430_CM_SYSCONFIG);
>  	prcm_context.sgx_cm_clksel =
>  			 cm_read_mod_reg(OMAP3430ES2_SGX_MOD, CM_CLKSEL);
> -	prcm_context.wkup_cm_clksel = cm_read_mod_reg(WKUP_MOD, CM_CLKSEL);
>  	prcm_context.dss_cm_clksel =
>  			 cm_read_mod_reg(OMAP3430_DSS_MOD, CM_CLKSEL);
>  	prcm_context.cam_cm_clksel =
> @@ -296,8 +292,6 @@ void omap3_prcm_save_context(void)
>  			cm_read_mod_reg(PLL_MOD, OMAP3430ES2_CM_CLKSEL4);
>  	prcm_context.pll_cm_clksel5 =
>  			 cm_read_mod_reg(PLL_MOD, OMAP3430ES2_CM_CLKSEL5);
> -	prcm_context.pll_cm_clken =
> -			cm_read_mod_reg(PLL_MOD, CM_CLKEN);
>  	prcm_context.pll_cm_clken2 =
>  			cm_read_mod_reg(PLL_MOD, OMAP3430ES2_CM_CLKEN2);
>  	prcm_context.cm_polctrl = __raw_readl(OMAP3430_CM_POLCTRL);
> @@ -343,8 +337,6 @@ void omap3_prcm_save_context(void)
>  			 cm_read_mod_reg(OMAP3430_IVA2_MOD, CM_AUTOIDLE2);
>  	prcm_context.mpu_cm_autoidle2 =
>  			 cm_read_mod_reg(MPU_MOD, CM_AUTOIDLE2);
> -	prcm_context.pll_cm_autoidle =
> -			 cm_read_mod_reg(PLL_MOD, CM_AUTOIDLE);
>  	prcm_context.iva2_cm_clkstctrl =
>  			 cm_read_mod_reg(OMAP3430_IVA2_MOD, CM_CLKSTCTRL);
>  	prcm_context.mpu_cm_clkstctrl =
> @@ -436,7 +428,6 @@ void omap3_prcm_restore_context(void)
>  	__raw_writel(prcm_context.cm_sysconfig, OMAP3430_CM_SYSCONFIG);
>  	cm_write_mod_reg(prcm_context.sgx_cm_clksel, OMAP3430ES2_SGX_MOD,
>  					 CM_CLKSEL);
> -	cm_write_mod_reg(prcm_context.wkup_cm_clksel, WKUP_MOD, CM_CLKSEL);
>  	cm_write_mod_reg(prcm_context.dss_cm_clksel, OMAP3430_DSS_MOD,
>  					 CM_CLKSEL);
>  	cm_write_mod_reg(prcm_context.cam_cm_clksel, OMAP3430_CAM_MOD,
> @@ -453,7 +444,6 @@ void omap3_prcm_restore_context(void)
>  					OMAP3430ES2_CM_CLKSEL4);
>  	cm_write_mod_reg(prcm_context.pll_cm_clksel5, PLL_MOD,
>  					 OMAP3430ES2_CM_CLKSEL5);
> -	cm_write_mod_reg(prcm_context.pll_cm_clken, PLL_MOD, CM_CLKEN);
>  	cm_write_mod_reg(prcm_context.pll_cm_clken2, PLL_MOD,
>  					OMAP3430ES2_CM_CLKEN2);
>  	__raw_writel(prcm_context.cm_polctrl, OMAP3430_CM_POLCTRL);
> @@ -492,7 +482,6 @@ void omap3_prcm_restore_context(void)
>  	cm_write_mod_reg(prcm_context.iva2_cm_autiidle2, OMAP3430_IVA2_MOD,
>  					CM_AUTOIDLE2);
>  	cm_write_mod_reg(prcm_context.mpu_cm_autoidle2, MPU_MOD, CM_AUTOIDLE2);
> -	cm_write_mod_reg(prcm_context.pll_cm_autoidle, PLL_MOD, CM_AUTOIDLE);
>  	cm_write_mod_reg(prcm_context.iva2_cm_clkstctrl, OMAP3430_IVA2_MOD,
>  					CM_CLKSTCTRL);
>  	cm_write_mod_reg(prcm_context.mpu_cm_clkstctrl, MPU_MOD, CM_CLKSTCTRL);

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2009-11-16 16:08 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-10-15 23:14 [PATCH] OMAP3: PM: Removing redundant and potentially dangerous PRCM configration Sripathy, Vishwanath
2009-10-16 11:16 ` Gopinath, Thara
2009-10-16 12:58   ` Sripathy, Vishwanath
2009-10-16 15:56 ` [PATCHV2] " Sripathy, Vishwanath
2009-10-20 19:35   ` Kevin Hilman
2009-11-16 11:11   ` [PATCHV3] " Sripathy, Vishwanath
2009-11-16 16:08     ` Kevin Hilman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox