Skip to contentSkip to conversation

OMAP3: PM: Set/clear T2 bit for Smartreflex on TWL

PATCH Superseded Shweta Gulati 4 participants 11 messages Feb 15, 2011 – Mar 3, 2011

Subsystems (5)
Arm (32-bit) Arm sub-architectures Multifunction devices (MFD) OMAP2+ support +1 more subsystems
lore
Share & tools
lore

Shweta Gulati

author
#0 lore raw

PATCH v4 1/1 OMAP3: PM: Set/clear T2 bit for Smartreflex on TWL

From: Thara Gopinath <[email protected]>

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 <[email protected]>
Signed-off-by Thara Gopinath <[email protected]>
Signed-off-by Shweta Gulati <[email protected]>
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(-)
arch/arm/mach-omap2/omap_twl.c +60 -0
index 00e1d2b..b341c36 100644
@@ -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;
+}
arch/arm/mach-omap2/pm.h +1 -0
index 39580e6..797bfd1 100644
@@ -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)
 {
Signature · 1 line
1.7.0.4
#1 lore raw

PATCH v4 1/1 OMAP3: PM: Set/clear T2 bit for Smartreflex on TWL

Quoted 4 lines · On Tue, 15 Feb 2011 13:28:58 +0530 Shweta Gulati <[email protected]> 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.

Quoted 3 lines
arch/arm/mach-omap2/omap_twl.c +0 -0
@@ -269,6 +276,18 @@ int __init omap3_twl_init(void)
 		omap3_core_volt_info.vp_vddmax = OMAP3630_VP2_VLIMITTO_VDDMAX;
 	}

...

Quoted 2 lines
arch/arm/mach-omap2/omap_twl.c +2 -0
+	if (!twl_sr_enable_autoinit)
+		omap3_twl_set_sr_bit(true);

...

Quoted 9 lines
arch/arm/mach-omap2/omap_twl.c +9 -0
+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);

arch/arm/mach-omap2/omap_twl.c int __init omap3_twl_init(void) · line 331

Quoted 10 lines
arch/arm/mach-omap2/omap_twl.c +10 -0
+	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.

arch/arm/mach-omap2/omap_twl.c int __init omap3_twl_init(void) · line 334

Quoted 3 lines
arch/arm/mach-omap2/omap_twl.c +3 -0
+	if (!ret) {
+		twl_sr_enable_autoinit = true;
+		return 0;

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

Signature · 1 line
Jarkko
#2 lore raw

PATCH v4 1/1 OMAP3: PM: Set/clear T2 bit for Smartreflex on TWL

Quoted 4 lines · On Tue, 15 Feb 2011 17:16:52 +0200 Jarkko Nikula <[email protected]> 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/.

Signature · 1 line
Jarkko
author
#3 lore raw

PATCH v4 1/1 OMAP3: PM: Set/clear T2 bit for Smartreflex on TWL

Quoted 10 lines · Jarkko Nikula
Quoted 4 lines · On Tue, 15 Feb 2011 17:16:52 +0200 Jarkko Nikula <[email protected]> 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.

Quoted 3 lines

--
Jarkko

Signature · 3 lines
Thanks,
Regards,
Shweta
#4 lore raw

PATCH v4 1/1 OMAP3: PM: Set/clear T2 bit for Smartreflex on TWL

Quoted 6 lines · Shweta
Quoted 3 lines · Jarkko Nikula

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.

Signature · 1 line
Jarkko
drivers/mfd/twl4030-power.c +27 -1
index 16422de0..e767b0f 100644
@@ -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:
include/linux/i2c/twl.h +1 -0
index 61b9609..1f64e3e 100644
@@ -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)
 };
author
#5 lore raw

PATCH v4 1/1 OMAP3: PM: Set/clear T2 bit for Smartreflex on TWL

Jarkko,

Quoted 15 lines · Jarkko Nikula
Quoted 6 lines · Shweta
Quoted 3 lines · Jarkko Nikula

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.

Quoted 4 lines

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.

Quoted 71 lines

--
Jarkko

drivers/mfd/twl4030-power.c +0 -0
index 16422de0..e767b0f 100644
@@ -92,6 +92,9 @@ static u8 twl4030_start_script_address = 0x2b;

?#define OFF_STATE_SHIFT ? ? ? ? ? ? ? ?4 ?#define OFF_STATE_MASK ? ? ? ? (0xf << OFF_STATE_SHIFT)

drivers/mfd/twl4030-power.c +3 -0
+#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,

drivers/mfd/twl4030-power.c +24 -1
@@ -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:

include/linux/i2c/twl.h +1 -0
index 61b9609..1f64e3e 100644
@@ -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)
 ?};
Signature · 3 lines
Thanks,
Regards,
Shweta
#6 lore raw

PATCH v4 1/1 OMAP3: PM: Set/clear T2 bit for Smartreflex on TWL

Quoted 10 lines · Shweta
Quoted 4 lines

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.

Signature · 1 line
Jarkko
author
#7 lore raw

PATCH v4 1/1 OMAP3: PM: Set/clear T2 bit for Smartreflex on TWL

Hi,

Quoted 14 lines · Jarkko Nikula
Quoted 10 lines · Shweta
Quoted 4 lines

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.

Quoted 8 lines

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.

Quoted 3 lines

--
Jarkko

Signature · 3 lines
Thanks,
Regards,
Shweta
#8 lore raw

PATCH v4 1/1 OMAP3: PM: Set/clear T2 bit for Smartreflex on TWL

In reply to Jarkko Nikula
Quoted 44 lines

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

Quoted 4 lines · On Tue, 15 Feb 2011 13:28:58 +0530 Shweta Gulati <[email protected]> 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.

Review int __init omap3_twl_init(void) · line 276

Quoted 2 lines
(quoted diff) +0 -0
@@ -269,6 +276,18 @@ int __init omap3_twl_init(void)
 		omap3_core_volt_info.vp_vddmax =

OMAP3630_VP2_VLIMITTO_VDDMAX;

Quoted 1 line
(quoted diff) +0 -0
 	}

...

Quoted 2 lines
arch/arm/mach-omap2/omap_twl.c +2 -0
+	if (!twl_sr_enable_autoinit)
+		omap3_twl_set_sr_bit(true);

...

Quoted 15 lines
arch/arm/mach-omap2/omap_twl.c +15 -0
+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
Quoted 8 lines

arch/arm/mach-omap2/omap_twl.c int __init omap3_twl_init(void) · line 331

Quoted 4 lines
arch/arm/mach-omap2/omap_twl.c +4 -0
+		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

Quoted 15 lines

arch/arm/mach-omap2/omap_twl.c int __init omap3_twl_init(void) · line 334

Quoted 4 lines
arch/arm/mach-omap2/omap_twl.c +3 -0
+	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

author
#9 lore raw

PATCH v4 1/1 OMAP3: PM: Set/clear T2 bit for Smartreflex on TWL

Hi,

Quoted 52 lines · Vishwanath Sripathy
Quoted 44 lines

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

Quoted 4 lines · On Tue, 15 Feb 2011 13:28:58 +0530 Shweta Gulati <[email protected]> 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.

Quoted 2 lines
(quoted diff) +0 -0
@@ -269,6 +276,18 @@ int __init omap3_twl_init(void)

? ? ? ? ? ? omap3_core_volt_info.vp_vddmax =

OMAP3630_VP2_VLIMITTO_VDDMAX;

Quoted 1 line

? ? }

...

Quoted 2 lines

+ ? if (!twl_sr_enable_autoinit)
+ ? ? ? ? ? omap3_twl_set_sr_bit(true);

...

Quoted 15 lines
(quoted diff) +15 -0
+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
Quoted 8 lines
Quoted 4 lines

+ ? ? ? ? ? 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.

Quoted 4 lines

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.

Quoted 17 lines

vishwa

Quoted 15 lines
Quoted 4 lines

+ ? 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

Signature · 3 lines
Thanks,
Regards,
Shweta
#10 lore raw

PATCH v4 1/1 OMAP3: PM: Set/clear T2 bit for Smartreflex on TWL

In reply to Shweta Gulati
Quoted 10 lines · Shweta Gulati

From: Thara Gopinath <[email protected]>

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.

Quoted 4 lines
Signed-off-by Nishanth Menon <[email protected]>
Signed-off-by Thara Gopinath <[email protected]>
Signed-off-by Shweta Gulati <[email protected]>
Cc linux-arm-kernel at lists.infradead.org

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

Kevin

Quoted 105 lines

---
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(-)
arch/arm/mach-omap2/omap_twl.c +60 -0
index 00e1d2b..b341c36 100644
@@ -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;
+}
arch/arm/mach-omap2/pm.h +1 -0
index 39580e6..797bfd1 100644
@@ -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)
 {