Hi, Mike. 2012/3/1 Mike Frysinger <vap...@gentoo.org>: > On Wednesday 29 February 2012 21:58:42 Nobuhiro Iwamatsu wrote: >> --- /dev/null >> +++ b/drivers/i2c/sh_sh7734_i2c.c >> >> +#include <common.h> >> +#include <asm/io.h> > > should include i2c.h too so you know your funcs match the prototypes everyone > else uses
OK. > >> +static int check_stop(struct sh_i2c *base) >> +{ >> + int i, ret = 1; >> + >> + for (i = 0; i < IRQ_WAIT; i++) { >> + if (SH_I2C_ICSR_STOP & readb(&base->icsr)) { >> + ret = 0; >> + break; >> + } >> + udelay(10); >> + } >> + >> + clrbits_le8(&base->icsr, SH_I2C_ICSR_STOP); >> + >> + return ret; >> +} >> + >> +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; >> + >> + clrbits_le8(&base->icsr, SH_I2C_ICSR_STOP); >> + >> + sh_i2c_send_stop(base); >> + } >> + >> + clrbits_le8(&base->icsr, SH_I2C_ICSR_TEND); >> + >> + return ret; >> +} >> + >> +static int check_tdre(struct sh_i2c *base) >> +{ >> + int i; >> + >> + for (i = 0; i < IRQ_WAIT; i++) { >> + if (SH_I2C_ICSR_TDRE & readb(&base->icsr)) >> + return 0; >> + udelay(10); >> + } >> + >> + return 1; >> +} >> + >> +static int check_rdrf(struct sh_i2c *base) >> +{ >> + int i; >> + >> + for (i = 0; i < IRQ_WAIT; i++) { >> + if (SH_I2C_ICSR_RDRF & readb(&base->icsr)) >> + return 0; >> + udelay(10); >> + } >> + >> + return 1; >> +} > > you've got the same icsr bit polling logic here. probably better to put it > into a dedicated func which takes the bit flag to check and the rest will call > that. > static int check_icsr_bit(struct sh_i2c *base, uint bit) > { > int i; > > for (i = 0; i < IRQ_WAIT; i++) { > if (bit & readb(&base->icsr)) > return 0; > udelay(10); > } > > return 1; > } > > static int check_tdre(struct sh_i2c *base) > { > return check_icsr_bit(base, SH_I2C_ICSR_TDRE); > } OK, I fixed. > >> +static u8 i2c_raw_read(struct sh_i2c *base, u8 id, u8 reg) >> +{ >> ... >> + writeb(id << 1 | 1, &base->icdrt); > > shifting and bit ops can lead to unexpected behavior ... might be better: > writeb((id << 1) | 1, &base->icdrt); > OK, I fixed. >> +/* >> + * i2c_probe: - Test if a chip answers for a given i2c address >> + * >> + * @chip: address of the chip which is searched for >> + * @return: 0 if a chip was found, -1 otherwhise >> + */ >> +int i2c_probe(u8 chip) >> +{ >> + return 0; >> +} > > should implement this. look at the Blackfin twi driver for an easy one. Thanks for your infomation. I implemented this. Thanks, Nobuhiro -- Nobuhiro Iwamatsu _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot