Hi, Mike.
Thank you for your review.
2012/2/22 Mike Frysinger <vap...@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
