Jarkko,
On Wed, Feb 16, 2011 at 5:15 PM, Jarkko Nikula <jhnikula@xxxxxxxxx> wrote:
> On Wed, 16 Feb 2011 11:22:09 +0530
> "Gulati, Shweta" <shweta.gulati@xxxxxx> wrote:
>
>> On Tue, Feb 15, 2011 at 8:59 PM, Jarkko Nikula <jhnikula@xxxxxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
