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

[Nobuhiro Iwamatsu]

Hi, Mike.

Thank you for your review.

2012/2/22 Mike Frysinger <vapier at gentoo.org>:
> On Tuesday 21 February 2012 20:13:47 Nobuhiro Iwamatsu wrote:
>> --- /dev/null
>> +++ b/drivers/i2c/sh_sh7734_i2c.c
>>
>> +#if DEBUG
>> +static void sh_i2c_dump_reg(struct sh_i2c *base)
>> +{
>> +     printf("iccr1 : %02X\n", readb(&base->iccr1));
>> +     printf("iccr2 : %02X\n", readb(&base->iccr2));
>> +     printf("icmr  : %02X\n", readb(&base->icmr));
>> +     printf("icier : %02X\n", readb(&base->icier));
>> +     printf("icsr  : %02X\n", readb(&base->icsr));
>> +     printf("sar   : %02X\n", readb(&base->sar));
>> +     printf("icdrt : %02X\n", readb(&base->icdrt));
>> +     printf("icdrr : %02X\n", readb(&base->icdrr));
>> +     printf("nf2cyc: %02X\n", readb(&base->nf2cyc));
>> +}
>> +#endif
>
> if you used debug(), you wouldn't need the DEBUG check

OK, but sh_i2c_dump_reg function is not referred from anywhere, I remove.

>
>> +static int
>> +i2c_raw_write(struct sh_i2c *base, u8 id, u8 reg, u8 *val, int size)
>> +{
>> +     int i;
>> +     u8 data;
>> +
>> +     if (i2c_set_addr(base, id, reg)) {
>> +             printf("Fail set slave address\n");
>
> should use puts() when there's no fmt

OK.

>
>> +     for (i = 0 ; i < size ; i++) {
>
> no space before the semi-colon
>
>> +int i2c_set_bus_num(unsigned int bus)
>> +{
>> +     if ((bus < 0) || (bus >= CONFIG_SYS_MAX_I2C_BUS)) {
>> +             printf("Bad bus: %d\n", bus);
>> +             return -1;
>> +     }
>> +
>> +     switch (bus) {
>> +     case 0:
>> +             base = (void *)CONFIG_SH_I2C_BASE0;
>> +             break;
>> +     case 1:
>> +             base = (void *)CONFIG_SH_I2C_BASE1;
>> +             break;
>> +     default:
>> +             return -1;
>> +     }
>
> do you need the if() check if you have default here ?

int i2c_set_bus_num(unsigned int bus)
{
    switch (bus) {
    case 0:
        base = (void *)CONFIG_SH_I2C_BASE0;
        break;
    case 1:
        base = (void *)CONFIG_SH_I2C_BASE1;
        break;
    default:
        printf("Bad bus: %d\n", bus);
        return -1;


Bestr regards,
  Nobuhiro
-- 
Nobuhiro Iwamatsu