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

Premi, Sanjeev
2011-01-19T10:29:10Z

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