[U-Boot] [PATCH] sh: i2c: Add support I2C controller of SH7734

[Nobuhiro Iwamatsu]

Hi,

Thanks for your comment.

2012/2/6 Heiko Schocher <hs at denx.de>:
> Hello Nobuhiro,
>
> Nobuhiro Iwamatsu wrote:
>> Renesas SH7734 has two I2C interfaceis.
>> This supports these I2C.
>>
>> Signed-off-by: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj at renesas.com>
>> ---
>>  drivers/i2c/Makefile        |    1 +
>>  drivers/i2c/sh_sh7734_i2c.c |  468 +++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 469 insertions(+), 0 deletions(-)
>>  create mode 100644 drivers/i2c/sh_sh7734_i2c.c
>
> [...]
>> diff --git a/drivers/i2c/sh_sh7734_i2c.c b/drivers/i2c/sh_sh7734_i2c.c
>> new file mode 100644
>> index 0000000..6c514b4
>> --- /dev/null
>> +++ b/drivers/i2c/sh_sh7734_i2c.c
>> @@ -0,0 +1,468 @@
>
> [...]
>> +static void sh_i2c_send_stop(struct sh_i2c *base)
>> +{
>> +     writeb(readb(&base->iccr2) & ~(SH_I2C_ICCR2_BBSY | SH_I2C_ICCR2_SCP),
>> +             &base->iccr2);
>
> Please use clrbits_*

OK, but clrbits_* functions is not implement to SuperH and other.
I will implement these functions to SuperH.

>
> [...]
>> +static int check_tend(struct sh_i2c *base, int stop)
>> +{
>> +     int i, ret = 1;
>> +
>> +     for (i = 0 ; i < IRQ_WAIT ; i++) {
>> +             if (SH_I2C_ICSR_TEND & readb(&base->icsr)) {
>> +                     ret = 0;
>> +                     break;
>> +             }
>> +             udelay(10);
>> +     }
>> +
>> +     if (stop) {
>> +             u8 data;
>> +
>> +             writeb(readb(&base->icsr) & ~SH_I2C_ICSR_STOP, &base->icsr);
>> +
>> +             sh_i2c_send_stop(base);
>> +     }
>> +
>> +     writeb(readb(&base->icsr) & ~SH_I2C_ICSR_TEND, &base->icsr);
>
> here too... please check globally.

OK. I will changed from writeb to clr|set|clrsetbits_*.

>
> [...]
>> +#ifdef CONFIG_I2C_MULTI_BUS
>> +static unsigned int current_bus;
>> +
>> +/**
>
> wrong comment style.
>

OK.
> [...]
>> +/**
>
> here too.

OK.
>
>> + * i2c_get_bus_num - returns index of active I2C bus
>> + */
>> +unsigned int i2c_get_bus_num(void)
> [...]
>> +int i2c_read(u8 chip, u32 addr, int alen, u8 *buffer, int len)
>> +{
>> +     int i = 0;
>
> new line please.
>

OK.

>> +     for (i = 0 ; i < len ; i++)
>> +             buffer[i] = i2c_raw_read(base, chip, addr + i);
> [...]
>
> Beside of this minor comments patch looks good, thanks!
>

Thanks!

Nobuhiro
-- 
Nobuhiro Iwamatsu