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 Nakamura<hisashi.nakamura...@renesas.com>
Signed-off-by: Nobuhiro Iwamatsu<nobuhiro.iwamatsu...@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to