[U-Boot] [PATCH v3 6/8] fsl_pmic: add I2C interface support

[Jason Liu]

Hi, Stefano,

2010/12/31 Stefano Babic <sbabic at denx.de>:
> On 12/29/2010 01:38 PM, Jason Liu wrote:
>> This patch add I2C interface for fsl_pmic driver support
>>
>> Signed-off-by: Jason Liu <r64343 at freescale.com>
>>
>> ---
>> Changes for v2:
>> - Address the comments from Stefano,
>>   - factor out the param_check in pmic_reg for both spi/i2c
>> ---
>>  drivers/misc/fsl_pmic.c |   52 ++++++++++++++++++++++++++++++++++++++++++----
>>  1 files changed, 47 insertions(+), 5 deletions(-)
>>
>
> Hi Jason,
>
>> +
>> +     if (init_done == 0) {
>> +             i2c_init(CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE);
>> +             init_done = 1;
>> +     }
>
> As I can see, i2c_init is called during initialization in
> arch/arm/lib/board.c. Why do we need to call it again ?

Yes, I think, I can remove it.

>
>> +     if (write) {
>> +             buf[0] = (val >> 16) & 0xff;
>> +             buf[1] = (val >> 8) & 0xff;
>> +             buf[2] = (val) & 0xff;
>> +             if (i2c_write(CONFIG_SYS_FSL_PMIC_I2C_ADDR, reg, 1, buf, 3))
>> +                     return -1;
>> +     } else {
>> +             if (i2c_read(CONFIG_SYS_FSL_PMIC_I2C_ADDR, reg, 1, buf, 3)) {
>> +                     return -1;
>> +             ret_val = buf[0] << 16 | buf[1] << 8 | buf[2];
>> +             }
>
> I am wondering if it works. The line with ret_val is never reached. Do
> you tested it ?

I have tested before. This patch of code is definitely wrong. I will fix it.

Thanks for review.
Happy New Year!

>
> Best regards,
> Stefano Babic
>
> --
> =====================================================================
> DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
> =====================================================================
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>