[PATCH V4] OMAP3: PM: Set/clear T2 bit for Smartreflex on TWL

[PATCH V4] OMAP3: PM: Set/clear T2 bit for Smartreflex on TWL

[Shweta Gulati]

From: Thara Gopinath <thara@ti.com>

Voltage control on TWL can be done using VMODE/I2C1/I2C_SR.
Since almost all platforms use I2C_SR on omap3, omap3_twl_init by
default expects that OMAP's I2C_SR is plugged in to TWL's I2C
and calls omap3_twl_set_sr_bit. On platforms where I2C_SR is not connected,
the board files are expected to call omap3_twl_set_sr_bit(false) to
ensure that I2C_SR path is not set for voltage control and prevent
the default behavior of omap3_twl_init.

Signed-off-by: Nishanth Menon <nm@ti.com>
Signed-off-by: Thara Gopinath <thara@ti.com>
Signed-off-by: Shweta Gulati <shweta.gulati@ti.com>
Cc: linux-arm-kernel at lists.infradead.org
---
This patch is based on LO PM Branch and Smartreflex has been
tested on OMAP3430 SDP, OMAP3630 SDP and boot tested on
OMAP2430 SDP.

 arch/arm/mach-omap2/omap_twl.c |   60 ++++++++++++++++++++++++++++++++++++++++
 arch/arm/mach-omap2/pm.h       |    1 +
 2 files changed, 61 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_twl.c b/arch/arm/mach-omap2/omap_twl.c
index 00e1d2b..b341c36 100644
--- a/arch/arm/mach-omap2/omap_twl.c
+++ b/arch/arm/mach-omap2/omap_twl.c
@@ -59,8 +59,15 @@
 
 static bool is_offset_valid;
 static u8 smps_offset;
+/*
+ * Flag to ensure Smartreflex bit in TWL
+ * being cleared in board file is not overwritten.
+ */
+static bool __initdata twl_sr_enable_autoinit;
 
+#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 +276,18 @@ int __init omap3_twl_init(void)
 		omap3_core_volt_info.vp_vddmax = OMAP3630_VP2_VLIMITTO_VDDMAX;
 	}
 
+	/*
+	 * The smartreflex bit on twl4030 specifies if the setting of voltage
+	 * is done over the I2C_SR path. Since this setting is independent of
+	 * the actual usage of smartreflex AVS module, we enable TWL SR bit
+	 * by default irrespective of whether smartreflex AVS module is enabled
+	 * on the OMAP side or not. This is because without this bit enabled,
+	 * the voltage scaling through vp forceupdate/bypass mechanism of
+	 * voltage scaling will not function on TWL over I2C_SR.
+	 */
+	if (!twl_sr_enable_autoinit)
+		omap3_twl_set_sr_bit(true);
+
 	voltdm = omap_voltage_domain_lookup("mpu");
 	omap_voltage_register_pmic(voltdm, &omap3_mpu_volt_info);
 
@@ -277,3 +296,44 @@ int __init omap3_twl_init(void)
 
 	return 0;
 }
+
+/**
+ * omap3_twl_set_sr_bit() - Set/Clear SR bit on TWL
+ * @enable: enable SR mode in twl or not
+ *
+ * If 'enable' is true, enables Smartreflex bit on TWL 4030 to make sure
+ * voltage scaling through OMAP SR works. Else, the smartreflex bit
+ * on twl4030 is cleared as 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,
+ * in those scenarios this bit is to be cleared (enable = false).
+ *
+ * Returns 0 on sucess, error is returned if I2C read/write fails.
+ */
+int __init omap3_twl_set_sr_bit(bool enable)
+{
+	u8 temp;
+	int ret;
+	if (twl_sr_enable_autoinit)
+		pr_warning("%s: unexpected multiple calls\n", __func__);
+
+	ret = twl_i2c_read_u8(TWL4030_MODULE_PM_RECEIVER, &temp,
+					TWL4030_DCDC_GLOBAL_CFG);
+	if (ret)
+		goto err;
+
+	if (enable)
+		temp |= SMARTREFLEX_ENABLE;
+	else
+		temp &= ~SMARTREFLEX_ENABLE;
+
+	ret = twl_i2c_write_u8(TWL4030_MODULE_PM_RECEIVER, temp,
+				TWL4030_DCDC_GLOBAL_CFG);
+	if (!ret) {
+		twl_sr_enable_autoinit = true;
+		return 0;
+	}
+err:
+	pr_err("%s: Error access to TWL4030 (%d)\n", __func__, ret);
+	return ret;
+}
diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
index 39580e6..797bfd1 100644
--- a/arch/arm/mach-omap2/pm.h
+++ b/arch/arm/mach-omap2/pm.h
@@ -127,6 +127,7 @@ 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 int omap3_twl_set_sr_bit(bool enable);
 #else
 static inline int omap3_twl_init(void)
 {
-- 
1.7.0.4

[PATCH V4] OMAP3: PM: Set/clear T2 bit for Smartreflex on TWL

[Jarkko Nikula]

On Tue, 15 Feb 2011 13:28:58 +0530
Shweta Gulati <shweta.gulati@ti.com> wrote:

> This patch is based on LO PM Branch and Smartreflex has been
> tested on OMAP3430 SDP, OMAP3630 SDP and boot tested on
> OMAP2430 SDP.
> 
I saw this was working on N900 (kind of special instrumentation
setup) after enabling /sys/kernel/debug/voltage/[vdd_core |
vdd_mpu]/smartreflex/autocomp. Few comments below.

> @@ -269,6 +276,18 @@ int __init omap3_twl_init(void)
>  		omap3_core_volt_info.vp_vddmax = OMAP3630_VP2_VLIMITTO_VDDMAX;
>  	}
...
> +	if (!twl_sr_enable_autoinit)
> +		omap3_twl_set_sr_bit(true);
...
> +int __init omap3_twl_set_sr_bit(bool enable)
> +{
> +	u8 temp;
> +	int ret;
> +	if (twl_sr_enable_autoinit)
> +		pr_warning("%s: unexpected multiple calls\n", __func__);
> +
> +	ret = twl_i2c_read_u8(TWL4030_MODULE_PM_RECEIVER, &temp,
> +					TWL4030_DCDC_GLOBAL_CFG);
> +	if (ret)
> +		goto err;
> +
> +	if (enable)
> +		temp |= SMARTREFLEX_ENABLE;
> +	else
> +		temp &= ~SMARTREFLEX_ENABLE;
> +
> +	ret = twl_i2c_write_u8(TWL4030_MODULE_PM_RECEIVER, temp,
> +				TWL4030_DCDC_GLOBAL_CFG);

Would it make more sense to set only the flag here and do the register
writes when omap3_twl_init is executing? Then it's not so strict when
the board code calls this function.

> +	if (!ret) {
> +		twl_sr_enable_autoinit = true;
> +		return 0;

Should this be twl_sr_enable_autoinit = enable (if going to do
register write here)?

-- 
Jarkko

[PATCH V4] OMAP3: PM: Set/clear T2 bit for Smartreflex on TWL

[Jarkko Nikula]

On Tue, 15 Feb 2011 17:16:52 +0200
Jarkko Nikula <jhnikula@gmail.com> wrote:

> Would it make more sense to set only the flag here and do the register
> writes when omap3_twl_init is executing? Then it's not so strict when
> the board code calls this function.
> 
Probably discussed earlier but would it make more sense to have flag in
struct twl4030_platform_data and to do registers writes in twl-core?
Looks suspicious to have i2c_writes under arch/arm/.

-- 
Jarkko

[PATCH V4] OMAP3: PM: Set/clear T2 bit for Smartreflex on TWL

[Gulati, Shweta]

On Tue, Feb 15, 2011 at 8:59 PM, Jarkko Nikula <jhnikula@gmail.com> wrote:
> On Tue, 15 Feb 2011 17:16:52 +0200
> Jarkko Nikula <jhnikula@gmail.com> wrote:
>
>> Would it make more sense to set only the flag here and do the register
>> writes when omap3_twl_init is executing? Then it's not so strict when
>> the board code calls this function.
>>
> Probably discussed earlier but would it make more sense to have flag in
> struct twl4030_platform_data and to do registers writes in twl-core?
> Looks suspicious to have i2c_writes under arch/arm/.
twl_i2c_read/write APIs are used from arch/arm in many board files,
so I think it should not cause any issue.
> --
> Jarkko
>



-- 
Thanks,
Regards,
Shweta

[PATCH V4] OMAP3: PM: Set/clear T2 bit for Smartreflex on TWL

[Jarkko Nikula]

On Wed, 16 Feb 2011 11:22:09 +0530
"Gulati, Shweta" <shweta.gulati@ti.com> wrote:

> On Tue, Feb 15, 2011 at 8:59 PM, Jarkko Nikula <jhnikula@gmail.com> wrote:
> > Probably discussed earlier but would it make more sense to have flag in
> > struct twl4030_platform_data and to do registers writes in twl-core?
> > Looks suspicious to have i2c_writes under arch/arm/.
> twl_i2c_read/write APIs are used from arch/arm in many board files,
> so I think it should not cause any issue.

Not good either e.g. in modularization point of view. I was thinking
something like below. I played safe and let the SR to be enabled only if
the twl4030_power_data.sr_enable is set. I read that Kevin had problems
earlier with 2430SDP if SR was enabled.

Note proof of concept patch only. I omitted the comments and don't do
explicit SR disable and I'd clean up the error paths in twl4030_power_init
a bit before this (e.g. printing error codes). Not sure either is the
twl4030-power.c right place for this or core.

-- 
Jarkko

diff --git a/drivers/mfd/twl4030-power.c b/drivers/mfd/twl4030-power.c
index 16422de0..e767b0f 100644
--- a/drivers/mfd/twl4030-power.c
+++ b/drivers/mfd/twl4030-power.c
@@ -92,6 +92,9 @@ static u8 twl4030_start_script_address = 0x2b;
 #define OFF_STATE_SHIFT		4
 #define OFF_STATE_MASK		(0xf << OFF_STATE_SHIFT)
 
+#define TWL4030_DCDC_GLOBAL_CFG	PHY_TO_OFF_PM_RECEIVER(0x61)
+#define SMARTREFLEX_ENABLE	BIT(3)
+
 static u8 res_config_addrs[] = {
 	[RES_VAUX1]	= 0x17,
 	[RES_VAUX2]	= 0x1b,
@@ -510,6 +513,22 @@ int twl4030_remove_script(u8 flags)
 	return err;
 }
 
+int __init twl4030_sr_enable(void)
+{
+	u8 temp;
+	int ret;
+
+	ret = twl_i2c_read_u8(TWL4030_MODULE_PM_RECEIVER, &temp,
+			      TWL4030_DCDC_GLOBAL_CFG);
+	if (ret)
+		return ret;
+
+	temp |= SMARTREFLEX_ENABLE;
+
+	return twl_i2c_write_u8(TWL4030_MODULE_PM_RECEIVER, temp,
+				TWL4030_DCDC_GLOBAL_CFG);
+}
+
 void __init twl4030_power_init(struct twl4030_power_data *twl4030_scripts)
 {
 	int err = 0;
@@ -549,8 +568,15 @@ void __init twl4030_power_init(struct twl4030_power_data *twl4030_scripts)
 
 	err = twl_i2c_write_u8(TWL4030_MODULE_PM_MASTER, 0,
 			TWL4030_PM_MASTER_PROTECT_KEY);
-	if (err)
+	if (err) {
 		pr_err("TWL4030 Unable to relock registers\n");
+		return;
+	}
+
+	if (twl4030_scripts->sr_enable)
+		err = twl4030_sr_enable();
+	if (err)
+		pr_err("TWL4030 Unable to set smartreflex. %d\n", err);
 	return;
 
 unlock:
diff --git a/include/linux/i2c/twl.h b/include/linux/i2c/twl.h
index 61b9609..1f64e3e 100644
--- a/include/linux/i2c/twl.h
+++ b/include/linux/i2c/twl.h
@@ -630,6 +630,7 @@ struct twl4030_power_data {
 	struct twl4030_script **scripts;
 	unsigned num;
 	struct twl4030_resconfig *resource_config;
+	bool sr_enable;	/* Smartreflex enable state */
 #define TWL4030_RESCONFIG_UNDEF	((u8)-1)
 };
 

[PATCH V4] OMAP3: PM: Set/clear T2 bit for Smartreflex on TWL

[Gulati, Shweta]

Jarkko,

On Wed, Feb 16, 2011 at 5:15 PM, Jarkko Nikula <jhnikula@gmail.com> wrote:
> On Wed, 16 Feb 2011 11:22:09 +0530
> "Gulati, Shweta" <shweta.gulati@ti.com> wrote:
>
>> On Tue, Feb 15, 2011 at 8:59 PM, Jarkko Nikula <jhnikula@gmail.com> wrote:
>> > Probably discussed earlier but would it make more sense to have flag in
>> > struct twl4030_platform_data and to do registers writes in twl-core?
>> > Looks suspicious to have i2c_writes under arch/arm/.
>> twl_i2c_read/write APIs are used from arch/arm in many board files,
>> so I think it should not cause any issue.
>
> Not good either e.g. in modularization point of view. I was thinking
> something like below. I played safe and let the SR to be enabled only if
> the twl4030_power_data.sr_enable is set. I read that Kevin had problems
> earlier with 2430SDP if SR was enabled.
>
> Note proof of concept patch only. I omitted the comments and don't do
> explicit SR disable and I'd clean up the error paths in twl4030_power_init
> a bit before this (e.g. printing error codes). Not sure either is the
> twl4030-power.c right place for this or core.
You missed commit log which says that "T2 bit is required to enable I2C_SR
path of voltage control" it is not at all enabling SR, voltage scale
APIs VPforceupdate/ VCbypass
needs this path to be enabled.
And calling APIs twl_i2c_read/write in driver codebase does n't ensure correct
ordering of flag changes and twl_read/write.
> --
> Jarkko
>
> diff --git a/drivers/mfd/twl4030-power.c b/drivers/mfd/twl4030-power.c
> index 16422de0..e767b0f 100644
> --- a/drivers/mfd/twl4030-power.c
> +++ b/drivers/mfd/twl4030-power.c
> @@ -92,6 +92,9 @@ static u8 twl4030_start_script_address = 0x2b;
> ?#define OFF_STATE_SHIFT ? ? ? ? ? ? ? ?4
> ?#define OFF_STATE_MASK ? ? ? ? (0xf << OFF_STATE_SHIFT)
>
> +#define TWL4030_DCDC_GLOBAL_CFG ? ? ? ?PHY_TO_OFF_PM_RECEIVER(0x61)
> +#define SMARTREFLEX_ENABLE ? ? BIT(3)
> +
> ?static u8 res_config_addrs[] = {
> ? ? ? ?[RES_VAUX1] ? ? = 0x17,
> ? ? ? ?[RES_VAUX2] ? ? = 0x1b,
> @@ -510,6 +513,22 @@ int twl4030_remove_script(u8 flags)
> ? ? ? ?return err;
> ?}
>
> +int __init twl4030_sr_enable(void)
> +{
> + ? ? ? u8 temp;
> + ? ? ? int ret;
> +
> + ? ? ? ret = twl_i2c_read_u8(TWL4030_MODULE_PM_RECEIVER, &temp,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? TWL4030_DCDC_GLOBAL_CFG);
> + ? ? ? if (ret)
> + ? ? ? ? ? ? ? return ret;
> +
> + ? ? ? temp |= SMARTREFLEX_ENABLE;
> +
> + ? ? ? return twl_i2c_write_u8(TWL4030_MODULE_PM_RECEIVER, temp,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? TWL4030_DCDC_GLOBAL_CFG);
> +}
> +
> ?void __init twl4030_power_init(struct twl4030_power_data *twl4030_scripts)
> ?{
> ? ? ? ?int err = 0;
> @@ -549,8 +568,15 @@ void __init twl4030_power_init(struct twl4030_power_data *twl4030_scripts)
>
> ? ? ? ?err = twl_i2c_write_u8(TWL4030_MODULE_PM_MASTER, 0,
> ? ? ? ? ? ? ? ? ? ? ? ?TWL4030_PM_MASTER_PROTECT_KEY);
> - ? ? ? if (err)
> + ? ? ? if (err) {
> ? ? ? ? ? ? ? ?pr_err("TWL4030 Unable to relock registers\n");
> + ? ? ? ? ? ? ? return;
> + ? ? ? }
> +
> + ? ? ? if (twl4030_scripts->sr_enable)
> + ? ? ? ? ? ? ? err = twl4030_sr_enable();
> + ? ? ? if (err)
> + ? ? ? ? ? ? ? pr_err("TWL4030 Unable to set smartreflex. %d\n", err);
> ? ? ? ?return;
>
> ?unlock:
> diff --git a/include/linux/i2c/twl.h b/include/linux/i2c/twl.h
> index 61b9609..1f64e3e 100644
> --- a/include/linux/i2c/twl.h
> +++ b/include/linux/i2c/twl.h
> @@ -630,6 +630,7 @@ struct twl4030_power_data {
> ? ? ? ?struct twl4030_script **scripts;
> ? ? ? ?unsigned num;
> ? ? ? ?struct twl4030_resconfig *resource_config;
> + ? ? ? bool sr_enable; /* Smartreflex enable state */
> ?#define TWL4030_RESCONFIG_UNDEF ? ? ? ?((u8)-1)
> ?};
>
>
>

-- 
Thanks,
Regards,
Shweta

[PATCH V4] OMAP3: PM: Set/clear T2 bit for Smartreflex on TWL

[Jarkko Nikula]

On Wed, 16 Feb 2011 17:24:30 +0530
"Gulati, Shweta" <shweta.gulati@ti.com> wrote:

> > Note proof of concept patch only. I omitted the comments and don't do
> > explicit SR disable and I'd clean up the error paths in twl4030_power_init
> > a bit before this (e.g. printing error codes). Not sure either is the
> > twl4030-power.c right place for this or core.
> You missed commit log which says that "T2 bit is required to enable I2C_SR
> path of voltage control" it is not at all enabling SR, voltage scale
> APIs VPforceupdate/ VCbypass
> needs this path to be enabled.
> And calling APIs twl_i2c_read/write in driver codebase does n't ensure correct
> ordering of flag changes and twl_read/write.

Ah, yeah. I forgot to comment that I tried shortly also to run this
enable code from workqueue ~60 s after bootup and indeed SR didn't work
if those autocomp sysfs nodes were set before setting the TWL SR bit and
I believe same holds for voltage scaling too as you say.

What I'm thinking is there actually need for some higher level
control for these that quarantees the right order independently from
when each module are initialized.

-- 
Jarkko

[PATCH V4] OMAP3: PM: Set/clear T2 bit for Smartreflex on TWL

[Gulati, Shweta]

Hi,

On Wed, Feb 16, 2011 at 6:01 PM, Jarkko Nikula <jhnikula@gmail.com> wrote:
> On Wed, 16 Feb 2011 17:24:30 +0530
> "Gulati, Shweta" <shweta.gulati@ti.com> wrote:
>
>> > Note proof of concept patch only. I omitted the comments and don't do
>> > explicit SR disable and I'd clean up the error paths in twl4030_power_init
>> > a bit before this (e.g. printing error codes). Not sure either is the
>> > twl4030-power.c right place for this or core.
>> You missed commit log which says that "T2 bit is required to enable I2C_SR
>> path of voltage control" it is not at all enabling SR, voltage scale
>> APIs VPforceupdate/ VCbypass
>> needs this path to be enabled.
>> And calling APIs twl_i2c_read/write in driver codebase does n't ensure correct
>> ordering of flag changes and twl_read/write.
>
> Ah, yeah. I forgot to comment that I tried shortly also to run this
> enable code from workqueue ~60 s after bootup and indeed SR didn't work
> if those autocomp sysfs nodes were set before setting the TWL SR bit and
> I believe same holds for voltage scaling too as you say.
>
> What I'm thinking is there actually need for some higher level
> control for these that quarantees the right order independently from
> when each module are initialized.
Yes there is a need.
Voltage control for TWL uses different ways VMODE/I2C1/I2C_SR
Most of the OMAP3+ boards uses I2C_SR, for this to function properly
SR_BIT on TWL side needs to be enabled but there could be some boards
that use VMODE  method of scaling, for those boards this bit has to be
disabled which can be done from board file that is why a flag is set to make
sure that API 'omap3_twl_set_sr_bit()' is called once and all initializations
are done in order so SR bit is set/cleared aptly.
> --
> Jarkko
>



-- 
Thanks,
Regards,
Shweta

[PATCH V4] OMAP3: PM: Set/clear T2 bit for Smartreflex on TWL

[Vishwanath Sripathy]

> -----Original Message-----
> From: linux-arm-kernel-bounces at lists.infradead.org [mailto:linux-arm-
> kernel-bounces at lists.infradead.org] On Behalf Of Jarkko Nikula
> Sent: Tuesday, February 15, 2011 8:47 PM
> To: Shweta Gulati
> Cc: Nishanth Menon; Thara Gopinath; linux-omap at vger.kernel.org;
> linux-arm-kernel at lists.infradead.org
> Subject: Re: [PATCH V4] OMAP3: PM: Set/clear T2 bit for Smartreflex on
> TWL
>
> On Tue, 15 Feb 2011 13:28:58 +0530
> Shweta Gulati <shweta.gulati@ti.com> wrote:
>
> > This patch is based on LO PM Branch and Smartreflex has been
> > tested on OMAP3430 SDP, OMAP3630 SDP and boot tested on
> > OMAP2430 SDP.
> >
> I saw this was working on N900 (kind of special instrumentation
> setup) after enabling /sys/kernel/debug/voltage/[vdd_core |
> vdd_mpu]/smartreflex/autocomp. Few comments below.
>
> > @@ -269,6 +276,18 @@ int __init omap3_twl_init(void)
> >  		omap3_core_volt_info.vp_vddmax =
> OMAP3630_VP2_VLIMITTO_VDDMAX;
> >  	}
> ...
> > +	if (!twl_sr_enable_autoinit)
> > +		omap3_twl_set_sr_bit(true);
> ...
> > +int __init omap3_twl_set_sr_bit(bool enable)
> > +{
> > +	u8 temp;
> > +	int ret;
> > +	if (twl_sr_enable_autoinit)
> > +		pr_warning("%s: unexpected multiple calls\n", __func__);
> > +
> > +	ret = twl_i2c_read_u8(TWL4030_MODULE_PM_RECEIVER, &temp,
> > +					TWL4030_DCDC_GLOBAL_CFG);
> > +	if (ret)
> > +		goto err;
> > +
> > +	if (enable)
> > +		temp |= SMARTREFLEX_ENABLE;
> > +	else
> > +		temp &= ~SMARTREFLEX_ENABLE;
> > +
> > +	ret = twl_i2c_write_u8(TWL4030_MODULE_PM_RECEIVER, temp,
> > +				TWL4030_DCDC_GLOBAL_CFG);
>
> Would it make more sense to set only the flag here and do the register
> writes when omap3_twl_init is executing? Then it's not so strict when
> the board code calls this function.
What if board code calls this function after twl_init is executed? Then
you will not clear the bit right?
Intention of this function is to make sure the bit is set/cleared when it
is called.

vishwa
>
> > +	if (!ret) {
> > +		twl_sr_enable_autoinit = true;
> > +		return 0;
>
> Should this be twl_sr_enable_autoinit = enable (if going to do
> register write here)?
>
> --
> Jarkko
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V4] OMAP3: PM: Set/clear T2 bit for Smartreflex on TWL

[Gulati, Shweta]

Hi,

On Tue, Feb 15, 2011 at 10:00 PM, Vishwanath Sripathy
<vishwanath.bs@ti.com> wrote:
>> -----Original Message-----
>> From: linux-arm-kernel-bounces at lists.infradead.org [mailto:linux-arm-
>> kernel-bounces at lists.infradead.org] On Behalf Of Jarkko Nikula
>> Sent: Tuesday, February 15, 2011 8:47 PM
>> To: Shweta Gulati
>> Cc: Nishanth Menon; Thara Gopinath; linux-omap at vger.kernel.org;
>> linux-arm-kernel at lists.infradead.org
>> Subject: Re: [PATCH V4] OMAP3: PM: Set/clear T2 bit for Smartreflex on
>> TWL
>>
>> On Tue, 15 Feb 2011 13:28:58 +0530
>> Shweta Gulati <shweta.gulati@ti.com> wrote:
>>
>> > This patch is based on LO PM Branch and Smartreflex has been
>> > tested on OMAP3430 SDP, OMAP3630 SDP and boot tested on
>> > OMAP2430 SDP.
>> >
>> I saw this was working on N900 (kind of special instrumentation
>> setup) after enabling /sys/kernel/debug/voltage/[vdd_core |
>> vdd_mpu]/smartreflex/autocomp. Few comments below.
>>
>> > @@ -269,6 +276,18 @@ int __init omap3_twl_init(void)
>> > ? ? ? ? ? ? omap3_core_volt_info.vp_vddmax =
>> OMAP3630_VP2_VLIMITTO_VDDMAX;
>> > ? ? }
>> ...
>> > + ? if (!twl_sr_enable_autoinit)
>> > + ? ? ? ? ? omap3_twl_set_sr_bit(true);
>> ...
>> > +int __init omap3_twl_set_sr_bit(bool enable)
>> > +{
>> > + ? u8 temp;
>> > + ? int ret;
>> > + ? if (twl_sr_enable_autoinit)
>> > + ? ? ? ? ? pr_warning("%s: unexpected multiple calls\n", __func__);
>> > +
>> > + ? ret = twl_i2c_read_u8(TWL4030_MODULE_PM_RECEIVER, &temp,
>> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? TWL4030_DCDC_GLOBAL_CFG);
>> > + ? if (ret)
>> > + ? ? ? ? ? goto err;
>> > +
>> > + ? if (enable)
>> > + ? ? ? ? ? temp |= SMARTREFLEX_ENABLE;
>> > + ? else
>> > + ? ? ? ? ? temp &= ~SMARTREFLEX_ENABLE;
>> > +
>> > + ? ret = twl_i2c_write_u8(TWL4030_MODULE_PM_RECEIVER, temp,
>> > + ? ? ? ? ? ? ? ? ? ? ? ? ? TWL4030_DCDC_GLOBAL_CFG);
>>
>> Would it make more sense to set only the flag here and do the register
>> writes when omap3_twl_init is executing? Then it's not so strict when
>> the board code calls this function.
> What if board code calls this function after twl_init is executed? Then
> you will not clear the bit right?
> Intention of this function is to make sure the bit is set/cleared when it
> is called.
ompa3_twl_init is called from 'omap2_common_pm_late_init' which is
late_initcall.
So it would be called after board specific initializations are
completed in 'init' process.
> vishwa
>>
>> > + ? if (!ret) {
>> > + ? ? ? ? ? twl_sr_enable_autoinit = true;
>> > + ? ? ? ? ? return 0;
>>
>> Should this be twl_sr_enable_autoinit = enable (if going to do
>> register write here)?
>>
>> --
>> Jarkko
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>



-- 
Thanks,
Regards,
Shweta

[PATCH V4] OMAP3: PM: Set/clear T2 bit for Smartreflex on TWL

[Kevin Hilman]

Shweta Gulati <shweta.gulati@ti.com> writes:

> From: Thara Gopinath <thara@ti.com>
>
> Voltage control on TWL can be done using VMODE/I2C1/I2C_SR.
> Since almost all platforms use I2C_SR on omap3, omap3_twl_init by
> default expects that OMAP's I2C_SR is plugged in to TWL's I2C
> and calls omap3_twl_set_sr_bit. On platforms where I2C_SR is not connected,
> the board files are expected to call omap3_twl_set_sr_bit(false) to
> ensure that I2C_SR path is not set for voltage control and prevent
> the default behavior of omap3_twl_init.
>
> Signed-off-by: Nishanth Menon <nm@ti.com>
> Signed-off-by: Thara Gopinath <thara@ti.com>
> Signed-off-by: Shweta Gulati <shweta.gulati@ti.com>
> Cc: linux-arm-kernel at lists.infradead.org

queueing this one for 2.6.39 (branch: for_2.6.39/pm-misc)

Kevin

> ---
> This patch is based on LO PM Branch and Smartreflex has been
> tested on OMAP3430 SDP, OMAP3630 SDP and boot tested on
> OMAP2430 SDP.
>
>  arch/arm/mach-omap2/omap_twl.c |   60 ++++++++++++++++++++++++++++++++++++++++
>  arch/arm/mach-omap2/pm.h       |    1 +
>  2 files changed, 61 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/omap_twl.c b/arch/arm/mach-omap2/omap_twl.c
> index 00e1d2b..b341c36 100644
> --- a/arch/arm/mach-omap2/omap_twl.c
> +++ b/arch/arm/mach-omap2/omap_twl.c
> @@ -59,8 +59,15 @@
>  
>  static bool is_offset_valid;
>  static u8 smps_offset;
> +/*
> + * Flag to ensure Smartreflex bit in TWL
> + * being cleared in board file is not overwritten.
> + */
> +static bool __initdata twl_sr_enable_autoinit;
>  
> +#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 +276,18 @@ int __init omap3_twl_init(void)
>  		omap3_core_volt_info.vp_vddmax = OMAP3630_VP2_VLIMITTO_VDDMAX;
>  	}
>  
> +	/*
> +	 * The smartreflex bit on twl4030 specifies if the setting of voltage
> +	 * is done over the I2C_SR path. Since this setting is independent of
> +	 * the actual usage of smartreflex AVS module, we enable TWL SR bit
> +	 * by default irrespective of whether smartreflex AVS module is enabled
> +	 * on the OMAP side or not. This is because without this bit enabled,
> +	 * the voltage scaling through vp forceupdate/bypass mechanism of
> +	 * voltage scaling will not function on TWL over I2C_SR.
> +	 */
> +	if (!twl_sr_enable_autoinit)
> +		omap3_twl_set_sr_bit(true);
> +
>  	voltdm = omap_voltage_domain_lookup("mpu");
>  	omap_voltage_register_pmic(voltdm, &omap3_mpu_volt_info);
>  
> @@ -277,3 +296,44 @@ int __init omap3_twl_init(void)
>  
>  	return 0;
>  }
> +
> +/**
> + * omap3_twl_set_sr_bit() - Set/Clear SR bit on TWL
> + * @enable: enable SR mode in twl or not
> + *
> + * If 'enable' is true, enables Smartreflex bit on TWL 4030 to make sure
> + * voltage scaling through OMAP SR works. Else, the smartreflex bit
> + * on twl4030 is cleared as 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,
> + * in those scenarios this bit is to be cleared (enable = false).
> + *
> + * Returns 0 on sucess, error is returned if I2C read/write fails.
> + */
> +int __init omap3_twl_set_sr_bit(bool enable)
> +{
> +	u8 temp;
> +	int ret;
> +	if (twl_sr_enable_autoinit)
> +		pr_warning("%s: unexpected multiple calls\n", __func__);
> +
> +	ret = twl_i2c_read_u8(TWL4030_MODULE_PM_RECEIVER, &temp,
> +					TWL4030_DCDC_GLOBAL_CFG);
> +	if (ret)
> +		goto err;
> +
> +	if (enable)
> +		temp |= SMARTREFLEX_ENABLE;
> +	else
> +		temp &= ~SMARTREFLEX_ENABLE;
> +
> +	ret = twl_i2c_write_u8(TWL4030_MODULE_PM_RECEIVER, temp,
> +				TWL4030_DCDC_GLOBAL_CFG);
> +	if (!ret) {
> +		twl_sr_enable_autoinit = true;
> +		return 0;
> +	}
> +err:
> +	pr_err("%s: Error access to TWL4030 (%d)\n", __func__, ret);
> +	return ret;
> +}
> diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
> index 39580e6..797bfd1 100644
> --- a/arch/arm/mach-omap2/pm.h
> +++ b/arch/arm/mach-omap2/pm.h
> @@ -127,6 +127,7 @@ 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 int omap3_twl_set_sr_bit(bool enable);
>  #else
>  static inline int omap3_twl_init(void)
>  {