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

[Marek Vasut <marek.vasut@gmail.com>]

> 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

But this puts lower amount of strain on the CPP. It doesn't have to check if 
DEBUG is defined 10 times, but only once ;-) Putting this into one big debug() 
call is also possible btw., though I don't like it.

> 
> > +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
> 
> > +	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 ?

CONFIG_SYS_MAX_I2C_BUS may be different in some obscure case?

M

> -mike