[PATCH] OMAP3: PM: Adding T2 enabling of smartreflex

[PATCH] OMAP3: PM: Adding T2 enabling of smartreflex

[Thara Gopinath]

The smartreflex bit on twl4030 needs to be enabled by default irrespective
of whether smartreflex module is enabled on the OMAP side or not.
This is because without this bit enabled the voltage scaling through
vp forceupdate does not function properly on OMAP3.

Signed-off-by: Thara Gopinath <thara@ti.com>
---
This patch is against LO master and has been
tested on OMAP3430 SDP and OMAP2430 SDP.

 arch/arm/mach-omap2/omap_twl.c |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_twl.c b/arch/arm/mach-omap2/omap_twl.c
index 15f8c6c..a59f36b 100644
--- a/arch/arm/mach-omap2/omap_twl.c
+++ b/arch/arm/mach-omap2/omap_twl.c
@@ -58,7 +58,9 @@
 static bool is_offset_valid;
 static u8 smps_offset;
 
+#define TWL4030_DCDC_GLOBAL_CFG	0x06
 #define REG_SMPS_OFFSET         0xE0
+#define SMARTREFLEX_ENABLE	BIT(3)
 
 unsigned long twl4030_vsel_to_uv(const u8 vsel)
 {
@@ -256,6 +258,7 @@ int __init omap4_twl_init(void)
 int __init omap3_twl_init(void)
 {
 	struct voltagedomain *voltdm;
+	u8 temp;
 
 	if (!cpu_is_omap34xx())
 		return -ENODEV;
@@ -267,6 +270,19 @@ int __init omap3_twl_init(void)
 		omap3_core_volt_info.vp_vddmax = OMAP3630_VP2_VLIMITTO_VDDMAX;
 	}
 
+	/*
+	 * The smartreflex bit on twl4030 needs to be enabled by
+	 * default irrespective of whether smartreflex module is
+	 * enabled on the OMAP side or not. This is because without
+	 * this bit enabled the voltage scaling through
+	 * vp forceupdate does not function properly on OMAP3.
+	 */
+	twl_i2c_read_u8(TWL4030_MODULE_PM_RECEIVER, &temp,
+			TWL4030_DCDC_GLOBAL_CFG);
+	temp |= SMARTREFLEX_ENABLE;
+	twl_i2c_write_u8(TWL4030_MODULE_PM_RECEIVER, temp,
+			TWL4030_DCDC_GLOBAL_CFG);
+
 	voltdm = omap_voltage_domain_lookup("mpu");
 	omap_voltage_register_pmic(voltdm, &omap3_mpu_volt_info);
 
-- 
1.7.0.4

Re: [PATCH] OMAP3: PM: Adding T2 enabling of smartreflex

[Nishanth Menon]

Thara Gopinath had written, on 12/31/2010 02:07 AM, the following:
> The smartreflex bit on twl4030 needs to be enabled by default irrespective
> of whether smartreflex module is enabled on the OMAP side or not.
> This is because without this bit enabled the voltage scaling through
> vp forceupdate does not function properly on OMAP3.
s/does not function properly/ does not function ;)
SR I2C is used for forceupdate/vc bypass modes - so neither will 
function without switching t2 mode.

T2 voltages could be set in quiet a few methods:
a) Synchronized Scaling Hardware Strategy (ENABLE_VMODE = 1) (for OMAP2 
and I had worked on one OMAP3430 product which used this as well) using 
VDD1_VFLOOR and VDD1_VROOF
b) Direct Strategy Software Scaling Mode (ENABLE_VMODE = 0) (Smart 
reflex disabled) - VDD1_VSEL
c) using smart reflex - as done below - allows OMAP with smart reflex 
hardware wired to the twl to use that functionality.

Blindly setting it to smartreflex mode is not correct IMHO. It might 
work on SDP and few TI and non-TI platforms, but not all.

> 
> Signed-off-by: Thara Gopinath <thara@ti.com>
> ---
> This patch is against LO master and has been
> tested on OMAP3430 SDP and OMAP2430 SDP.
> 
>  arch/arm/mach-omap2/omap_twl.c |   16 ++++++++++++++++
>  1 files changed, 16 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/omap_twl.c b/arch/arm/mach-omap2/omap_twl.c
> index 15f8c6c..a59f36b 100644
> --- a/arch/arm/mach-omap2/omap_twl.c
> +++ b/arch/arm/mach-omap2/omap_twl.c
> @@ -58,7 +58,9 @@
>  static bool is_offset_valid;
>  static u8 smps_offset;
>  
> +#define TWL4030_DCDC_GLOBAL_CFG	0x06
>  #define REG_SMPS_OFFSET         0xE0
> +#define SMARTREFLEX_ENABLE	BIT(3)
>  
>  unsigned long twl4030_vsel_to_uv(const u8 vsel)
>  {
> @@ -256,6 +258,7 @@ int __init omap4_twl_init(void)
>  int __init omap3_twl_init(void)
>  {
>  	struct voltagedomain *voltdm;
> +	u8 temp;
>  
>  	if (!cpu_is_omap34xx())
>  		return -ENODEV;
> @@ -267,6 +270,19 @@ int __init omap3_twl_init(void)
>  		omap3_core_volt_info.vp_vddmax = OMAP3630_VP2_VLIMITTO_VDDMAX;
>  	}
>  
> +	/*
> +	 * The smartreflex bit on twl4030 needs to be enabled by
> +	 * default irrespective of whether smartreflex module is
> +	 * enabled on the OMAP side or not. This is because without
> +	 * this bit enabled the voltage scaling through
> +	 * vp forceupdate does not function properly on OMAP3.
> +	 */
> +	twl_i2c_read_u8(TWL4030_MODULE_PM_RECEIVER, &temp,
> +			TWL4030_DCDC_GLOBAL_CFG);
> +	temp |= SMARTREFLEX_ENABLE;
> +	twl_i2c_write_u8(TWL4030_MODULE_PM_RECEIVER, temp,
> +			TWL4030_DCDC_GLOBAL_CFG);
> +
>  	voltdm = omap_voltage_domain_lookup("mpu");
>  	omap_voltage_register_pmic(voltdm, &omap3_mpu_volt_info);
>  


-- 
Regards,
Nishanth Menon

RE: [PATCH] OMAP3: PM: Adding T2 enabling of smartreflex

[Gopinath, Thara]

>>-----Original Message-----
>>From: Menon, Nishanth
>>Sent: Monday, January 03, 2011 9:22 PM
>>To: Gopinath, Thara
>>Cc: linux-omap@vger.kernel.org; khilman@deeprootsystems.com;
>>paul@pwsan.com; Cousson, Benoit; Sripathy, Vishwanath; Sawant, Anand
>>Subject: Re: [PATCH] OMAP3: PM: Adding T2 enabling of smartreflex
>>
>>Thara Gopinath had written, on 12/31/2010 02:07 AM, the following:
>>> The smartreflex bit on twl4030 needs to be enabled by default
>>irrespective
>>> of whether smartreflex module is enabled on the OMAP side or not.
>>> This is because without this bit enabled the voltage scaling through
>>> vp forceupdate does not function properly on OMAP3.
>>s/does not function properly/ does not function ;)
>>SR I2C is used for forceupdate/vc bypass modes - so neither will
>>function without switching t2 mode.
>>
>>T2 voltages could be set in quiet a few methods:
>>a) Synchronized Scaling Hardware Strategy (ENABLE_VMODE = 1) (for OMAP2
>>and I had worked on one OMAP3430 product which used this as well) using
>>VDD1_VFLOOR and VDD1_VROOF
>>b) Direct Strategy Software Scaling Mode (ENABLE_VMODE = 0) (Smart
>>reflex disabled) - VDD1_VSEL
>>c) using smart reflex - as done below - allows OMAP with smart reflex
>>hardware wired to the twl to use that functionality.
>>
>>Blindly setting it to smartreflex mode is not correct IMHO. It might
>>work on SDP and few TI and non-TI platforms, but not all.

Yes you are right. The SR I2C needs to be enabled for vp force update
and vc bypass method of voltage scaling as well as for smartreflex
feature. Are you telling that there are platforms out there using OMAP3
and T2 and still using VMODE or VSEL. As far as I am aware our h/w folks themselves do not recommend these methods. 

Regards
Thara

>>
>>>
>>> Signed-off-by: Thara Gopinath <thara@ti.com>
>>> ---
>>> This patch is against LO master and has been
>>> tested on OMAP3430 SDP and OMAP2430 SDP.
>>>
>>>  arch/arm/mach-omap2/omap_twl.c |   16 ++++++++++++++++
>>>  1 files changed, 16 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-omap2/omap_twl.c b/arch/arm/mach-
>>omap2/omap_twl.c
>>> index 15f8c6c..a59f36b 100644
>>> --- a/arch/arm/mach-omap2/omap_twl.c
>>> +++ b/arch/arm/mach-omap2/omap_twl.c
>>> @@ -58,7 +58,9 @@
>>>  static bool is_offset_valid;
>>>  static u8 smps_offset;
>>>
>>> +#define TWL4030_DCDC_GLOBAL_CFG	0x06
>>>  #define REG_SMPS_OFFSET         0xE0
>>> +#define SMARTREFLEX_ENABLE	BIT(3)
>>>
>>>  unsigned long twl4030_vsel_to_uv(const u8 vsel)
>>>  {
>>> @@ -256,6 +258,7 @@ int __init omap4_twl_init(void)
>>>  int __init omap3_twl_init(void)
>>>  {
>>>  	struct voltagedomain *voltdm;
>>> +	u8 temp;
>>>
>>>  	if (!cpu_is_omap34xx())
>>>  		return -ENODEV;
>>> @@ -267,6 +270,19 @@ int __init omap3_twl_init(void)
>>>  		omap3_core_volt_info.vp_vddmax = OMAP3630_VP2_VLIMITTO_VDDMAX;
>>>  	}
>>>
>>> +	/*
>>> +	 * The smartreflex bit on twl4030 needs to be enabled by
>>> +	 * default irrespective of whether smartreflex module is
>>> +	 * enabled on the OMAP side or not. This is because without
>>> +	 * this bit enabled the voltage scaling through
>>> +	 * vp forceupdate does not function properly on OMAP3.
>>> +	 */
>>> +	twl_i2c_read_u8(TWL4030_MODULE_PM_RECEIVER, &temp,
>>> +			TWL4030_DCDC_GLOBAL_CFG);
>>> +	temp |= SMARTREFLEX_ENABLE;
>>> +	twl_i2c_write_u8(TWL4030_MODULE_PM_RECEIVER, temp,
>>> +			TWL4030_DCDC_GLOBAL_CFG);
>>> +
>>>  	voltdm = omap_voltage_domain_lookup("mpu");
>>>  	omap_voltage_register_pmic(voltdm, &omap3_mpu_volt_info);
>>>
>>
>>
>>--
>>Regards,
>>Nishanth Menon

Re: [PATCH] OMAP3: PM: Adding T2 enabling of smartreflex

[Nishanth Menon]

Gopinath, Thara wrote, on 01/05/2011 04:46 AM:
[..]
> Yes you are right. The SR I2C needs to be enabled for vp force update
> and vc bypass method of voltage scaling as well as for smartreflex
> feature. Are you telling that there are platforms out there using OMAP3
> and T2 and still using VMODE or VSEL. As far as I am aware our h/w folks themselves do not recommend these methods.

;) not every one likes to follow recommendations rt? but yeah, there are 
folks who use vmode for 3430, vsel is possible even for 3630 - using 
class2 implementation if desired or forced to if the SR lines are not 
hooked up on the board.

-- 
Regards,
Nishanth Menon

Re: [PATCH] OMAP3: PM: Adding T2 enabling of smartreflex

[Kevin Hilman]

Thara Gopinath <thara@ti.com> writes:

> The smartreflex bit on twl4030 needs to be enabled by default irrespective
> of whether smartreflex module is enabled on the OMAP side or not.
> This is because without this bit enabled the voltage scaling through
> vp forceupdate does not function properly on OMAP3.

Based on Nishanth's comments, the abofe statements need a little more
justification.

What is probably needed is some default setting (possibly this one) but
with the possibility of board code to disable this if needed.

Kevin

>
> Signed-off-by: Thara Gopinath <thara@ti.com>
> ---
> This patch is against LO master and has been
> tested on OMAP3430 SDP and OMAP2430 SDP.
>
>  arch/arm/mach-omap2/omap_twl.c |   16 ++++++++++++++++
>  1 files changed, 16 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/omap_twl.c b/arch/arm/mach-omap2/omap_twl.c
> index 15f8c6c..a59f36b 100644
> --- a/arch/arm/mach-omap2/omap_twl.c
> +++ b/arch/arm/mach-omap2/omap_twl.c
> @@ -58,7 +58,9 @@
>  static bool is_offset_valid;
>  static u8 smps_offset;
>  
> +#define TWL4030_DCDC_GLOBAL_CFG	0x06
>  #define REG_SMPS_OFFSET         0xE0
> +#define SMARTREFLEX_ENABLE	BIT(3)
>  
>  unsigned long twl4030_vsel_to_uv(const u8 vsel)
>  {
> @@ -256,6 +258,7 @@ int __init omap4_twl_init(void)
>  int __init omap3_twl_init(void)
>  {
>  	struct voltagedomain *voltdm;
> +	u8 temp;
>  
>  	if (!cpu_is_omap34xx())
>  		return -ENODEV;
> @@ -267,6 +270,19 @@ int __init omap3_twl_init(void)
>  		omap3_core_volt_info.vp_vddmax = OMAP3630_VP2_VLIMITTO_VDDMAX;
>  	}
>  
> +	/*
> +	 * The smartreflex bit on twl4030 needs to be enabled by
> +	 * default irrespective of whether smartreflex module is
> +	 * enabled on the OMAP side or not. This is because without
> +	 * this bit enabled the voltage scaling through
> +	 * vp forceupdate does not function properly on OMAP3.
> +	 */
> +	twl_i2c_read_u8(TWL4030_MODULE_PM_RECEIVER, &temp,
> +			TWL4030_DCDC_GLOBAL_CFG);
> +	temp |= SMARTREFLEX_ENABLE;
> +	twl_i2c_write_u8(TWL4030_MODULE_PM_RECEIVER, temp,
> +			TWL4030_DCDC_GLOBAL_CFG);
> +
>  	voltdm = omap_voltage_domain_lookup("mpu");
>  	omap_voltage_register_pmic(voltdm, &omap3_mpu_volt_info);

RE: [PATCH] OMAP3: PM: Adding T2 enabling of smartreflex

[Gopinath, Thara]

>>-----Original Message-----
>>From: Hilman, Kevin
>>Sent: Wednesday, January 05, 2011 4:18 AM
>>To: Gopinath, Thara
>>Cc: linux-omap@vger.kernel.org; paul@pwsan.com; Cousson, Benoit; Sripathy,
>>Vishwanath; Sawant, Anand; Menon, Nishanth
>>Subject: Re: [PATCH] OMAP3: PM: Adding T2 enabling of smartreflex
>>
>>Thara Gopinath <thara@ti.com> writes:
>>
>>> The smartreflex bit on twl4030 needs to be enabled by default
>>irrespective
>>> of whether smartreflex module is enabled on the OMAP side or not.
>>> This is because without this bit enabled the voltage scaling through
>>> vp forceupdate does not function properly on OMAP3.
>>
>>Based on Nishanth's comments, the abofe statements need a little more
>>justification.
>>
>>What is probably needed is some default setting (possibly this one) but
>>with the possibility of board code to disable this if needed.

Yes. If we need to support the other means of voltage scaling, we definitely need to override this bit from board files. I am not so convinced that we need to support them though. IMHO, this patch can still go in with change in the comment and if needed, there can be another patch with an API in omap_twl.c allowing overriding/ enabling-disabling of sr - i2c bit on
the T2 side.

Regards
Thara

>>
>>Kevin
>>
>>>
>>> Signed-off-by: Thara Gopinath <thara@ti.com>
>>> ---
>>> This patch is against LO master and has been
>>> tested on OMAP3430 SDP and OMAP2430 SDP.
>>>
>>>  arch/arm/mach-omap2/omap_twl.c |   16 ++++++++++++++++
>>>  1 files changed, 16 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-omap2/omap_twl.c b/arch/arm/mach-
>>omap2/omap_twl.c
>>> index 15f8c6c..a59f36b 100644
>>> --- a/arch/arm/mach-omap2/omap_twl.c
>>> +++ b/arch/arm/mach-omap2/omap_twl.c
>>> @@ -58,7 +58,9 @@
>>>  static bool is_offset_valid;
>>>  static u8 smps_offset;
>>>
>>> +#define TWL4030_DCDC_GLOBAL_CFG	0x06
>>>  #define REG_SMPS_OFFSET         0xE0
>>> +#define SMARTREFLEX_ENABLE	BIT(3)
>>>
>>>  unsigned long twl4030_vsel_to_uv(const u8 vsel)
>>>  {
>>> @@ -256,6 +258,7 @@ int __init omap4_twl_init(void)
>>>  int __init omap3_twl_init(void)
>>>  {
>>>  	struct voltagedomain *voltdm;
>>> +	u8 temp;
>>>
>>>  	if (!cpu_is_omap34xx())
>>>  		return -ENODEV;
>>> @@ -267,6 +270,19 @@ int __init omap3_twl_init(void)
>>>  		omap3_core_volt_info.vp_vddmax = OMAP3630_VP2_VLIMITTO_VDDMAX;
>>>  	}
>>>
>>> +	/*
>>> +	 * The smartreflex bit on twl4030 needs to be enabled by
>>> +	 * default irrespective of whether smartreflex module is
>>> +	 * enabled on the OMAP side or not. This is because without
>>> +	 * this bit enabled the voltage scaling through
>>> +	 * vp forceupdate does not function properly on OMAP3.
>>> +	 */
>>> +	twl_i2c_read_u8(TWL4030_MODULE_PM_RECEIVER, &temp,
>>> +			TWL4030_DCDC_GLOBAL_CFG);
>>> +	temp |= SMARTREFLEX_ENABLE;
>>> +	twl_i2c_write_u8(TWL4030_MODULE_PM_RECEIVER, temp,
>>> +			TWL4030_DCDC_GLOBAL_CFG);
>>> +
>>>  	voltdm = omap_voltage_domain_lookup("mpu");
>>>  	omap_voltage_register_pmic(voltdm, &omap3_mpu_volt_info);

Re: [PATCH] OMAP3: PM: Adding T2 enabling of smartreflex

[Kevin Hilman]

"Gopinath, Thara" <thara@ti.com> writes:

>>>-----Original Message-----
>>>From: Hilman, Kevin
>>>Sent: Wednesday, January 05, 2011 4:18 AM
>>>To: Gopinath, Thara
>>>Cc: linux-omap@vger.kernel.org; paul@pwsan.com; Cousson, Benoit; Sripathy,
>>>Vishwanath; Sawant, Anand; Menon, Nishanth
>>>Subject: Re: [PATCH] OMAP3: PM: Adding T2 enabling of smartreflex
>>>
>>>Thara Gopinath <thara@ti.com> writes:
>>>
>>>> The smartreflex bit on twl4030 needs to be enabled by default
>>>irrespective
>>>> of whether smartreflex module is enabled on the OMAP side or not.
>>>> This is because without this bit enabled the voltage scaling through
>>>> vp forceupdate does not function properly on OMAP3.
>>>
>>>Based on Nishanth's comments, the abofe statements need a little more
>>>justification.
>>>
>>>What is probably needed is some default setting (possibly this one) but
>>>with the possibility of board code to disable this if needed.
>
> Yes. If we need to support the other means of voltage scaling, we
> definitely need to override this bit from board files. I am not so
> convinced that we need to support them though. IMHO, this patch can
> still go in with change in the comment and if needed, there can be
> another patch with an API in omap_twl.c allowing overriding/
> enabling-disabling of sr - i2c bit on the T2 side.

I'm OK with this.

Kevin

[PATCH] OMAP3: PM: Adding T2 enabling of smartreflex

[shweta gulati]

From: Thara Gopinath <thara@ti.com>

The smartreflex bit on twl4030 needs to be enabled by default irrespective
of whether smartreflex module is enabled on the OMAP side or not.
This is because without this bit enabled the voltage scaling through
vp forceupdate does not function properly on OMAP3. There are two
APIs being added 'omap3_twl_enable_sr' to set SR bit and other API
'omap3_twl_disable_sr' to disable SR bit for platforms where voltage
is not scaled using vpforceupdate or vc_bypass Method.

This patch is based on LO PM Branch and Smartreflex has been
tested on OMAP3430 SDP, OMAP3630 SDP and boot tested on
OMAP2430 SDP.

Signed-off-by: Shweta Gulati <shweta.gulati@ti.com>
Signed-off-by: Thara Gopinath <thara@ti.com>
---
 arch/arm/mach-omap2/omap_twl.c |   46 ++++++++++++++++++++++++++++++++++++++++
 arch/arm/mach-omap2/pm.h       |    2 +
 2 files changed, 48 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_twl.c b/arch/arm/mach-omap2/omap_twl.c
index 00e1d2b..5b3ca56 100644
--- a/arch/arm/mach-omap2/omap_twl.c
+++ b/arch/arm/mach-omap2/omap_twl.c
@@ -60,7 +60,9 @@
 static bool is_offset_valid;
 static u8 smps_offset;
 
+#define TWL4030_DCDC_GLOBAL_CFG        0x06
 #define REG_SMPS_OFFSET         0xE0
+#define SMARTREFLEX_ENABLE     BIT(3)
 
 static unsigned long twl4030_vsel_to_uv(const u8 vsel)
 {
@@ -269,6 +271,15 @@ int __init omap3_twl_init(void)
 		omap3_core_volt_info.vp_vddmax = OMAP3630_VP2_VLIMITTO_VDDMAX;
 	}
 
+	/*
+	 * The smartreflex bit on twl4030 needs to be enabled by
+	 * default irrespective of whether smartreflex module is
+	 * enabled on the OMAP side or not. This is because without
+	 * this bit enabled the voltage scaling through
+	 * vp forceupdate does not function properly on OMAP3.
+	 */
+	omap3_twl_enable_sr();
+
 	voltdm = omap_voltage_domain_lookup("mpu");
 	omap_voltage_register_pmic(voltdm, &omap3_mpu_volt_info);
 
@@ -277,3 +288,38 @@ int __init omap3_twl_init(void)
 
 	return 0;
 }
+
+/*
+ * The smartreflex bit on twl4030 is enabled in twl_init
+ * but there are platforms which use OMAP3 and T2 but use
+ * Synchronized Scaling Hardware Strategy (ENABLE_VMODE=1) and
+ * Direct Strategy Software Scaling Mode (ENABLE_VMODE=0).
+ * for setting the voltages of T2, in those scenarios this bit
+ * is to be cleared.
+ */
+void omap3_twl_disable_sr()
+{
+	u8 temp;
+
+	twl_i2c_read_u8(TWL4030_MODULE_PM_RECEIVER, &temp,
+			TWL4030_DCDC_GLOBAL_CFG);
+	temp &= ~SMARTREFLEX_ENABLE;
+	twl_i2c_write_u8(TWL4030_MODULE_PM_RECEIVER, temp,
+			TWL4030_DCDC_GLOBAL_CFG);
+}
+
+/*
+ * To enable Smartreflex bit on TWl 4030 to make sure
+ * voltage scaling through Vp forceupdate works.
+ */
+
+void omap3_twl_enable_sr()
+{
+	u8 temp;
+
+	twl_i2c_read_u8(TWL4030_MODULE_PM_RECEIVER, &temp,
+			TWL4030_DCDC_GLOBAL_CFG);
+	temp |= SMARTREFLEX_ENABLE;
+	twl_i2c_write_u8(TWL4030_MODULE_PM_RECEIVER, temp,
+			TWL4030_DCDC_GLOBAL_CFG);
+}
diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
index 704766b..8500356 100644
--- a/arch/arm/mach-omap2/pm.h
+++ b/arch/arm/mach-omap2/pm.h
@@ -127,6 +127,8 @@ static inline void omap_enable_smartreflex_on_init(void) {}
 #ifdef CONFIG_TWL4030_CORE
 extern int omap3_twl_init(void);
 extern int omap4_twl_init(void);
+extern void omap3_twl_disable_sr(void);
+extern void omap3_twl_enable_sr(void);
 #else
 static inline int omap3_twl_init(void)
 {
-- 
1.7.0.4

Re: [PATCH] OMAP3: PM: Adding T2 enabling of smartreflex

[Nishanth Menon]

shweta gulati wrote, on 01/19/2011 11:07 AM:

could we improve $subject?

> From: Thara Gopinath<thara@ti.com>
>
> The smartreflex bit on twl4030 needs to be enabled by default irrespective
> of whether smartreflex module is enabled on the OMAP side or not.
> This is because without this bit enabled the voltage scaling through
> vp forceupdate does not function properly on OMAP3. There are two
> APIs being added 'omap3_twl_enable_sr' to set SR bit and other API
> 'omap3_twl_disable_sr' to disable SR bit for platforms where voltage
> is not scaled using vpforceupdate or vc_bypass Method.
I cant see how disable_sr is usable, further as discussed in 
http://marc.info/?l=linux-omap&m=129424774929498&w=2 we decided to 
introduce api *if needed*. does any one need it?

>
> This patch is based on LO PM Branch and Smartreflex has been
> tested on OMAP3430 SDP, OMAP3630 SDP and boot tested on
> OMAP2430 SDP.
>
> Signed-off-by: Shweta Gulati<shweta.gulati@ti.com>
> Signed-off-by: Thara Gopinath<thara@ti.com>
> ---
>   arch/arm/mach-omap2/omap_twl.c |   46 ++++++++++++++++++++++++++++++++++++++++
>   arch/arm/mach-omap2/pm.h       |    2 +
>   2 files changed, 48 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/omap_twl.c b/arch/arm/mach-omap2/omap_twl.c
> index 00e1d2b..5b3ca56 100644
> --- a/arch/arm/mach-omap2/omap_twl.c
> +++ b/arch/arm/mach-omap2/omap_twl.c
> @@ -60,7 +60,9 @@
>   static bool is_offset_valid;
>   static u8 smps_offset;
>
> +#define TWL4030_DCDC_GLOBAL_CFG        0x06
>   #define REG_SMPS_OFFSET         0xE0
> +#define SMARTREFLEX_ENABLE     BIT(3)
>
>   static unsigned long twl4030_vsel_to_uv(const u8 vsel)
>   {
> @@ -269,6 +271,15 @@ int __init omap3_twl_init(void)
>   		omap3_core_volt_info.vp_vddmax = OMAP3630_VP2_VLIMITTO_VDDMAX;
>   	}
>
> +	/*
> +	 * The smartreflex bit on twl4030 needs to be enabled by
> +	 * default irrespective of whether smartreflex module is
> +	 * enabled on the OMAP side or not. This is because without
> +	 * this bit enabled the voltage scaling through
> +	 * vp forceupdate does not function properly on OMAP3.
> +	 */
> +	omap3_twl_enable_sr();

I am a bit lost:omap3_twl_init is called from pm.c 
omap2_common_pm_late_init right? as per
http://marc.info/?l=linux-omap&m=129424774929498&w=2
we could optionally override this bit from board file using some api - 
with this being done in pm_late_init, what chance does the board file 
have to do something else?


> +
>   	voltdm = omap_voltage_domain_lookup("mpu");
>   	omap_voltage_register_pmic(voltdm,&omap3_mpu_volt_info);
>
> @@ -277,3 +288,38 @@ int __init omap3_twl_init(void)
>
>   	return 0;
>   }
> +
> +/*
> + * The smartreflex bit on twl4030 is enabled in twl_init
> + * but there are platforms which use OMAP3 and T2 but use
> + * Synchronized Scaling Hardware Strategy (ENABLE_VMODE=1) and
> + * Direct Strategy Software Scaling Mode (ENABLE_VMODE=0).
> + * for setting the voltages of T2, in those scenarios this bit
> + * is to be cleared.
> + */
please read Documentation/kernel-doc-nano-HOWTO.txt line 54 to see how 
to use function comment header

> +void omap3_twl_disable_sr()
a) why is this not returning int? i2c reads and writes can fail.
b) there is no usage of this function - why have it?
> +{
> +	u8 temp;
> +
> +	twl_i2c_read_u8(TWL4030_MODULE_PM_RECEIVER,&temp,
> +			TWL4030_DCDC_GLOBAL_CFG);
> +	temp&= ~SMARTREFLEX_ENABLE;
> +	twl_i2c_write_u8(TWL4030_MODULE_PM_RECEIVER, temp,
> +			TWL4030_DCDC_GLOBAL_CFG);
> +}
> +
> +/*
> + * To enable Smartreflex bit on TWl 4030 to make sure
> + * voltage scaling through Vp forceupdate works.
> + */
> +
and here.
> +void omap3_twl_enable_sr()
a) __init?
b) why is this not returning int? i2c reads and writes can fail.

> +{
> +	u8 temp;
> +
> +	twl_i2c_read_u8(TWL4030_MODULE_PM_RECEIVER,&temp,
> +			TWL4030_DCDC_GLOBAL_CFG);
> +	temp |= SMARTREFLEX_ENABLE;
> +	twl_i2c_write_u8(TWL4030_MODULE_PM_RECEIVER, temp,
> +			TWL4030_DCDC_GLOBAL_CFG);
> +}
> diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
> index 704766b..8500356 100644
> --- a/arch/arm/mach-omap2/pm.h
> +++ b/arch/arm/mach-omap2/pm.h
> @@ -127,6 +127,8 @@ static inline void omap_enable_smartreflex_on_init(void) {}
>   #ifdef CONFIG_TWL4030_CORE
>   extern int omap3_twl_init(void);
>   extern int omap4_twl_init(void);
> +extern void omap3_twl_disable_sr(void);
> +extern void omap3_twl_enable_sr(void);
>   #else
>   static inline int omap3_twl_init(void)
>   {


-- 
Regards,
Nishanth Menon

Re: [PATCH] OMAP3: PM: Adding T2 enabling of smartreflex

[Nishanth Menon]

Gulati, Shweta wrote, on 01/19/2011 02:10 PM:
>      >I cant see how disable_sr is usable, further as discussed in
>     http://marc.info/?l=linux->omap&m=129424774929498&w=2
>     <http://marc.info/?l=linux-omap&m=129424774929498&w=2> we decided to
>     introduce api *if needed*. does any one need it?
>
>     There might be some OMAP3xxx chips where for DVFS VMODE or VSEL
>     method is used, for those boards this disable API could be called
>     from Board File,  to make sure the sequence of execution  of
>
>      late_init_call of twl_init from omap2_common_pm_late_init does not
> hamper clearing of this bit I would add a global variable to keep track
> if Resetting is required or not, will submit V2 with changes.
Apologies, but a Dumb question:

late_initcall(omap2_common_pm_late_init)
board files(arch/arm/include/asm/mach/arch.h machine_desc) tend to have 
the following hooks:

void                    (*fixup)(struct machine_desc *,
                                  struct tag *, char **,
                                  struct meminfo *);
void                    (*reserve)(void);/* reserve mem blocks  */
void                    (*map_io)(void);/* IO mapping function  */
void                    (*init_early)(void);
void                    (*init_irq)(void);
struct sys_timer        *timer;         /* system tick timer    */
void                    (*init_machine (void);
void                    (*handle_irq)(struct pt_regs *);

So if i wanted to disable the SR setting, where should I do it?

-- 
Regards,
Nishanth Menon

PS: could you please fix your mailer when replying on l-o for 
netetiquette reasons?

Re: [PATCH] OMAP3: PM: Adding T2 enabling of smartreflex

[Gulati, Shweta]

On Wed, Jan 19, 2011 at 5:54 PM, Nishanth Menon <nm@ti.com> wrote:
> Gulati, Shweta wrote, on 01/19/2011 02:10 PM:
>>
>>     >I cant see how disable_sr is usable, further as discussed in
>>    http://marc.info/?l=linux->omap&m=129424774929498&w=2
>>    <http://marc.info/?l=linux-omap&m=129424774929498&w=2> we decided to
>>    introduce api *if needed*. does any one need it?
>>
>>    There might be some OMAP3xxx chips where for DVFS VMODE or VSEL
>>    method is used, for those boards this disable API could be called
>>    from Board File,  to make sure the sequence of execution  of
>>
>>     late_init_call of twl_init from omap2_common_pm_late_init does not
>> hamper clearing of this bit I would add a global variable to keep track
>> if Resetting is required or not, will submit V2 with changes.
>
> Apologies, but a Dumb question:
>
> late_initcall(omap2_common_pm_late_init)
> board files(arch/arm/include/asm/mach/arch.h machine_desc) tend to have the
> following hooks:
>
> void                    (*fixup)(struct machine_desc *,
>                                 struct tag *, char **,
>                                 struct meminfo *);
> void                    (*reserve)(void);/* reserve mem blocks  */
> void                    (*map_io)(void);/* IO mapping function  */
> void                    (*init_early)(void);
> void                    (*init_irq)(void);
> struct sys_timer        *timer;         /* system tick timer    */
> void                    (*init_machine (void);
> void                    (*handle_irq)(struct pt_regs *);
>
> So if i wanted to disable the SR setting, where should I do it?
>
> --
> Regards,
> Nishanth Menon

Nishanth,

I think i could not make my point clear in my previous reply.

There are two APIs added 'omap3_twl_disable_sr' and
'omap3_twl_enable_sr'. omap3_twl_enable_sr sets SR bit in TWL4030
registers to make sure VPforceupdate method of voltage scaling works
properly but there could be some OMAP3xxx chips where VMODE or VSEL
method of voltage scaling is used (though today these voltage scaling
methods are not recommended by HW folks). For the boards where
Vpforceupdate method is used this  'omap3_twl_disable_sr' would not be
called.  For possible case of platforms where VMODE or VSEL methods
are used the API 'omap3_twl_disable_sr' will reset this bit.
The API 'omap3_twl_disable_sr' can be called from board file, to make
sure the resetting of this bit is not
overwritten in late_init call of omap3_twl_init(where
'omap3_twl_enable_sr' is called) I suggested adding a
global flag which will keep track whether this bit is cleared from
board file or not.
Would make these changes in V2.

Regards,
Shweta
> PS: could you please fix your mailer when replying on l-o for netetiquette
> reasons?
>



--
Thanks,
Regards,
Shweta
--
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] OMAP3: PM: Adding T2 enabling of smartreflex

[Premi, Sanjeev]

> -----Original Message-----
> From: linux-omap-owner@vger.kernel.org 
> [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of shweta gulati
> Sent: Wednesday, January 19, 2011 2:38 PM
> To: linux-omap@vger.kernel.org
> Cc: Gopinath, Thara; Gulati, Shweta
> Subject: [PATCH] OMAP3: PM: Adding T2 enabling of smartreflex

[sp] Few questions interleaved:

> 
> From: Thara Gopinath <thara@ti.com>
> 
> The smartreflex bit on twl4030 needs to be enabled by default 
> irrespective
> of whether smartreflex module is enabled on the OMAP side or not.

[sp] Then shouldn't we set this bit at init - one time only?

> This is because without this bit enabled the voltage scaling through
> vp forceupdate does not function properly on OMAP3. There are two
> APIs being added 'omap3_twl_enable_sr' to set SR bit and other API
> 'omap3_twl_disable_sr' to disable SR bit for platforms where voltage
> is not scaled using vpforceupdate or vc_bypass Method.

[sp] If the intent of the patch is to set the bit "irrespective of
     smartreflex; why would we need API to clear the bit.

     You may also want to consider if  set/reset is better suited
     in the api as compared to enable/disable().

     If there is need to set/clear the bit, I suggest one fuction for
     the purpose e.g. omap3_twl_set_sr(u8 flag) - though suffix "_sr"
     as used in my example still seems to be ambiguous.

~sanjeev

> 
> This patch is based on LO PM Branch and Smartreflex has been
> tested on OMAP3430 SDP, OMAP3630 SDP and boot tested on
> OMAP2430 SDP.
> 
> Signed-off-by: Shweta Gulati <shweta.gulati@ti.com>
> Signed-off-by: Thara Gopinath <thara@ti.com>
> ---
>  arch/arm/mach-omap2/omap_twl.c |   46 
> ++++++++++++++++++++++++++++++++++++++++
>  arch/arm/mach-omap2/pm.h       |    2 +
>  2 files changed, 48 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/omap_twl.c 
> b/arch/arm/mach-omap2/omap_twl.c
> index 00e1d2b..5b3ca56 100644
> --- a/arch/arm/mach-omap2/omap_twl.c
> +++ b/arch/arm/mach-omap2/omap_twl.c
> @@ -60,7 +60,9 @@
>  static bool is_offset_valid;
>  static u8 smps_offset;
>  
> +#define TWL4030_DCDC_GLOBAL_CFG        0x06
>  #define REG_SMPS_OFFSET         0xE0
> +#define SMARTREFLEX_ENABLE     BIT(3)
>  
>  static unsigned long twl4030_vsel_to_uv(const u8 vsel)
>  {
> @@ -269,6 +271,15 @@ int __init omap3_twl_init(void)
>  		omap3_core_volt_info.vp_vddmax = 
> OMAP3630_VP2_VLIMITTO_VDDMAX;
>  	}
>  
> +	/*
> +	 * The smartreflex bit on twl4030 needs to be enabled by
> +	 * default irrespective of whether smartreflex module is
> +	 * enabled on the OMAP side or not. This is because without
> +	 * this bit enabled the voltage scaling through
> +	 * vp forceupdate does not function properly on OMAP3.
> +	 */
> +	omap3_twl_enable_sr();
> +
>  	voltdm = omap_voltage_domain_lookup("mpu");
>  	omap_voltage_register_pmic(voltdm, &omap3_mpu_volt_info);
>  
> @@ -277,3 +288,38 @@ int __init omap3_twl_init(void)
>  
>  	return 0;
>  }
> +
> +/*
> + * The smartreflex bit on twl4030 is enabled in twl_init
> + * but there are platforms which use OMAP3 and T2 but use
> + * Synchronized Scaling Hardware Strategy (ENABLE_VMODE=1) and
> + * Direct Strategy Software Scaling Mode (ENABLE_VMODE=0).
> + * for setting the voltages of T2, in those scenarios this bit
> + * is to be cleared.
> + */
> +void omap3_twl_disable_sr()
> +{
> +	u8 temp;
> +
> +	twl_i2c_read_u8(TWL4030_MODULE_PM_RECEIVER, &temp,
> +			TWL4030_DCDC_GLOBAL_CFG);
> +	temp &= ~SMARTREFLEX_ENABLE;
> +	twl_i2c_write_u8(TWL4030_MODULE_PM_RECEIVER, temp,
> +			TWL4030_DCDC_GLOBAL_CFG);
> +}
> +
> +/*
> + * To enable Smartreflex bit on TWl 4030 to make sure
> + * voltage scaling through Vp forceupdate works.
> + */
> +
> +void omap3_twl_enable_sr()
> +{
> +	u8 temp;
> +
> +	twl_i2c_read_u8(TWL4030_MODULE_PM_RECEIVER, &temp,
> +			TWL4030_DCDC_GLOBAL_CFG);
> +	temp |= SMARTREFLEX_ENABLE;
> +	twl_i2c_write_u8(TWL4030_MODULE_PM_RECEIVER, temp,
> +			TWL4030_DCDC_GLOBAL_CFG);
> +}
> diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
> index 704766b..8500356 100644
> --- a/arch/arm/mach-omap2/pm.h
> +++ b/arch/arm/mach-omap2/pm.h
> @@ -127,6 +127,8 @@ static inline void 
> omap_enable_smartreflex_on_init(void) {}
>  #ifdef CONFIG_TWL4030_CORE
>  extern int omap3_twl_init(void);
>  extern int omap4_twl_init(void);
> +extern void omap3_twl_disable_sr(void);
> +extern void omap3_twl_enable_sr(void);
>  #else
>  static inline int omap3_twl_init(void)
>  {
> -- 
> 1.7.0.4
> 
> --
> 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
>