From mboxrd@z Thu Jan  1 00:00:00 1970
From: Heiko Schocher <hs@denx.de>
Date: Fri, 27 Sep 2013 05:37:55 +0200
Subject: [U-Boot] [PATCH] i2c: Add support for Renesas rcar
In-Reply-To: <1380237685-1741-1-git-send-email-nobuhiro.iwamatsu.yj@renesas.com>
References: <1380237685-1741-1-git-send-email-nobuhiro.iwamatsu.yj@renesas.com>
Message-ID: <5244FD93.2060709@denx.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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.ak@renesas.com>
> Signed-off-by: Nobuhiro Iwamatsu<nobuhiro.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 ...

>   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 ...

> +	/* 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.

[...]

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany