Hi David,
On Sat, Feb 19, 2011 at 5:21 AM, David Cohen <dacohen@xxxxxxxxx> wrote:
> Hi,
>
> On Fri, Feb 18, 2011 at 7:08 PM, Lesly A M <leslyam@xxxxxx> wrote:
>> Added api to get the TWL5030 Si version from the IDCODE register.
>> It is used for enabling the workaround for TWL errata 27.
>>
>> Signed-off-by: Lesly A M <leslyam@xxxxxx>
>> Cc: Nishanth Menon <nm@xxxxxx>
>> Cc: David Derrick <dderrick@xxxxxx>
>> Cc: Samuel Ortiz <sameo@xxxxxxxxxxxxxxx>
>> ---
>> drivers/mfd/twl-core.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++
>> include/linux/i2c/twl.h | 10 +++++++++
>> 2 files changed, 60 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
>> index a35fa7d..211c2cc 100644
>> --- a/drivers/mfd/twl-core.c
>> +++ b/drivers/mfd/twl-core.c
>> @@ -487,6 +487,56 @@ EXPORT_SYMBOL(twl_i2c_read_u8);
>>
>> /*----------------------------------------------------------------------*/
>>
>> +/**
>> + * twl_read_idcode_register - api to read the IDCODE register.
>> + * @value: returns 32 bit IDCODE register value.
>> + *
>> + * Unlocks the IDCODE register and read the 32 bit value.
>> + */
>> +int twl_read_idcode_register(u32 *value)
>> +{
>> + int err = 0;
>
> No need to initialize 'ret'.
Ok
>> +
>> + err = twl_i2c_write_u8(TWL4030_MODULE_INTBR, TWL_EEPROM_R_UNLOCK,
>> + R_UNLOCK_TEST_REG);
>> + if (err)
>> + pr_err("TWL4030 Unable to unlock IDCODE registers\n");
>> +
>> + err = twl_i2c_read(TWL4030_MODULE_INTBR, (u8 *)(value),
>> + 0x0, 4);
>
> Variable 'err' is being overwritten here.
>
>> + if (err)
>> + pr_err("TWL4030: unable to read IDCODE-%d\n", err);
>> +
>> + err = twl_i2c_write_u8(TWL4030_MODULE_INTBR, 0x0, R_UNLOCK_TEST_REG);
>
> Here again.
>
>> + if (err)
>> + pr_err("TWL4030 Unable to relock IDCODE registers\n");
>> +
>> + return err;
>
> And then you're returning 'err'. Unless you care only about the last
> 'err' assignment, something is wrong here, isn't it? :)
Ok, I will use a label and goto that label if there is some err on any
i2c_read/write
>
>> +}
>> +
>> +/**
>> + * twl5030_get_si_ver - api to get TWL5030 Si version.
>> + * @value: returns Si version from IDCODE.
>> + *
>> + * Api to get the TWL5030 Si version from IDCODE value.
>> + */
>> +int twl5030_get_si_ver(u32 *value)
>> +{
>> + int ret = 0;
>
> No need to initialize 'ret'.
Ok
>> + static u32 twl_idcode;
>> +
>> + if (twl_idcode == 0)
>> + ret = twl_read_idcode_register(&twl_idcode);
>> + if (ret)
>> + pr_err("TWL4030 Unable to check Si version\n");
>
> Shouldn't you return error here?
Yes,
>> +
>> + if (TWL_SIL_TYPE(twl_idcode) == TWL_SIL_5030)
>> + *value = TWL_SIL_REV(twl_idcode);
>
> Otherwise, what happens here if twl_read_idcode_register() fails?
>
>> +
>> + return ret;
>
> return 0 if the code reaches this point.
Ok. I will do the changes
Regards,
Lesly A M
>> +}
>> +EXPORT_SYMBOL(twl5030_get_si_ver);
>> +
>> static struct device *
>> add_numbered_child(unsigned chip, const char *name, int num,
>> void *pdata, unsigned pdata_len,
>> diff --git a/include/linux/i2c/twl.h b/include/linux/i2c/twl.h
>> index f4bd475..5d3f2bf 100644
>> --- a/include/linux/i2c/twl.h
>> +++ b/include/linux/i2c/twl.h
>> @@ -150,7 +150,15 @@
>> #define MMC_PU (0x1 << 3)
>> #define MMC_PD (0x1 << 2)
>>
>> +#define R_UNLOCK_TEST_REG 0x12
>> +#define TWL_EEPROM_R_UNLOCK 0x49
>>
>> +#define TWL_SIL_TYPE(rev) ((rev) & 0x00FFFFFF)
>> +#define TWL_SIL_REV(rev) ((rev) >> 24)
>> +#define TWL_SIL_5030 0x09002F
>> +#define TWL5030_REV_1_0 0x00
>> +#define TWL5030_REV_1_1 0x10
>> +#define TWL5030_REV_1_2 0x30
>>
>> #define TWL4030_CLASS_ID 0x4030
>> #define TWL6030_CLASS_ID 0x6030
>> @@ -180,6 +188,8 @@ int twl_i2c_read_u8(u8 mod_no, u8 *val, u8 reg);
>> int twl_i2c_write(u8 mod_no, u8 *value, u8 reg, unsigned num_bytes);
>> int twl_i2c_read(u8 mod_no, u8 *value, u8 reg, unsigned num_bytes);
>>
>> +int twl5030_get_si_ver(u32 *value);
>> +
>> int twl6030_interrupt_unmask(u8 bit_mask, u8 offset);
>> int twl6030_interrupt_mask(u8 bit_mask, u8 offset);
>>
>> --
>> 1.7.1
>>
>> --
>> 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
>>
>
--
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
