Hi, Heiko.
Thank you for your review.
(2013/09/27 12:37), Heiko Schocher wrote:
Hello Nobuhiro,
Am 27.09.2013 01:21, schrieb Nobuhiro Iwamatsu:
This supports i2c controller for Renesas rcar.
Signed-off-by: Hisashi Nakamurahisashi.nakamura.ak@renesas.com Signed-off-by: Nobuhiro Iwamatsunobuhiro.iwamatsu.yj@renesas.com
drivers/i2c/Makefile | 1 + drivers/i2c/rcar_i2c.c | 289 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 290 insertions(+) create mode 100644 drivers/i2c/rcar_i2c.c
Thanks! Patch looks good to me, just some nitpicking comments:
diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile index 37ccbd1..f7cbd62 100644 --- a/drivers/i2c/Makefile +++ b/drivers/i2c/Makefile @@ -26,6 +26,7 @@ COBJS-$(CONFIG_TSI108_I2C) += tsi108_i2c.o COBJS-$(CONFIG_U8500_I2C) += u8500_i2c.o COBJS-$(CONFIG_SH_I2C) += sh_i2c.o COBJS-$(CONFIG_SH_SH7734_I2C) += sh_sh7734_i2c.o +COBJS-$(CONFIG_SYS_I2C_RCAR) += rcar_i2c.o
Please keep this list sorted ...
OK, I will fix.
COBJS-$(CONFIG_SYS_I2C) += i2c_core.o COBJS-$(CONFIG_SYS_I2C_FSL) += fsl_i2c.o COBJS-$(CONFIG_SYS_I2C_FTI2C010) += fti2c010.o diff --git a/drivers/i2c/rcar_i2c.c b/drivers/i2c/rcar_i2c.c new file mode 100644 index 0000000..92f0700 --- /dev/null +++ b/drivers/i2c/rcar_i2c.c @@ -0,0 +1,289 @@
[...]
+static u8 +rcar_i2c_raw_read(struct rcar_i2c *dev, u8 chip, uint addr) +{
- u8 ret;
- rcar_i2c_raw_rw_common(dev, chip, addr);
- /* set slave address, receive */
- writel((chip<< 1) | 1,&dev->icmar);
^ ^ space please, please fix globally
Hmm.. checkpatch says for your patch:
total: 0 errors, 0 warnings, 0 checks, 296 lines checked
NOTE: Ignored message types: COMPLEX_MACRO CONSIDER_KSTRTO MINMAX MULTISTATEMENT_MACRO_USE_DO_WHILE NETWORKING_BLOCK_COMMENT_STYLE USLEEP_RANGE
mbox has no obvious style problems and is ready for submission.
Seems checpatch did not check this ...
this is my mistake. I will fix and recheck.
- /* start master receive */
- writel(MCR_MDBS | MCR_MIE | MCR_ESG,&dev->icmcr);
- while ((readl(&dev->icmsr)& (MSR_MAT | MSR_MDE))
- != (MSR_MAT | MSR_MDE))
- udelay(10);
- /* clear ESG */
- writel(MCR_MDBS | MCR_MIE,&dev->icmcr);
- /* prepare stop condition */
- writel(MCR_MDBS | MCR_MIE | MCR_FSB,&dev->icmcr);
- /* start SCLclk */
- writel(~(MSR_MAT | MSR_MDR),&dev->icmsr);
- while (!(readl(&dev->icmsr)& MSR_MDR))
- udelay(10);
- /* get receive data */
- ret = (u8)readl(&dev->icrxdtxd);
- /* start SCLclk */
- writel(~MSR_MDR,&dev->icmsr);
- rcar_i2c_raw_rw_finish(dev);
- return ret;
+}
Please only one empty line.
[...]
OK, I will remove this line.
bye, Heiko
Best regards, Nobuhiro