[PATCH v3 0/1 resend] ARM: shmobile: r8a7790: add I2C support

[PATCH v3 0/1 resend] ARM: shmobile: r8a7790: add I2C support

[Nguyen Viet Dung]

Hi Wolfram
CC Morimoto

Please consider the following patch for the r8a7790 Soc.
This patch modify I2C driver of rcar-H1 to usable on both rcar-H1 and rcar-H2.
It was developed base on the renesas-devel-20130722 branch and
have tested on the Lager board.

V3: Using the ID tables to resolve the difference between H1 and H2.

Thanks,
Nguyen viet Dung

Nguyen Viet Dung (1):
  i2c: rcar: modify I2C driver

 drivers/i2c/busses/i2c-rcar.c |   37 +++++++++++++++++++++++++++++++++++--
 1 file changed, 35 insertions(+), 2 deletions(-)

-- 
1.7.9.5

[PATCH v3 1/1 resend] i2c: rcar: modify I2C driver

[Nguyen Viet Dung]

This patch modify I2C driver of rcar-H1 to usable on both rcar-H1 and rcar-H2.

Signed-off-by: Nguyen Viet Dung <nv-dung@jinso.co.jp>
---
 drivers/i2c/busses/i2c-rcar.c |   37 +++++++++++++++++++++++++++++++++++--
 1 file changed, 35 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index 0fc5858..e08ef28 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -101,6 +101,11 @@ enum {
 #define ID_ARBLOST	(1 << 3)
 #define ID_NACK		(1 << 4)
 
+enum rcar_i2c_type {
+	I2C_RCAR_H1,
+	I2C_RCAR_H2,
+};
+
 struct rcar_i2c_priv {
 	void __iomem *io;
 	struct i2c_adapter adap;
@@ -113,6 +118,7 @@ struct rcar_i2c_priv {
 	int irq;
 	u32 icccr;
 	u32 flags;
+	enum rcar_i2c_type	devtype;
 };
 
 #define rcar_i2c_priv_to_dev(p)		((p)->adap.dev.parent)
@@ -224,12 +230,23 @@ static int rcar_i2c_clock_calculate(struct rcar_i2c_priv *priv,
 	u32 scgd, cdf;
 	u32 round, ick;
 	u32 scl;
+	u32 cdf_width;
 
 	if (!clkp) {
 		dev_err(dev, "there is no peripheral_clk\n");
 		return -EIO;
 	}
 
+	switch (priv->devtype) {
+	default:
+	case I2C_RCAR_H1:
+		cdf_width = 2;
+		break;
+	case I2C_RCAR_H2:
+		cdf_width = 3;
+		break;
+	}
+
 	/*
 	 * calculate SCL clock
 	 * see
@@ -245,7 +262,7 @@ static int rcar_i2c_clock_calculate(struct rcar_i2c_priv *priv,
 	 * clkp : peripheral_clk
 	 * F[]  : integer up-valuation
 	 */
-	for (cdf = 0; cdf < 4; cdf++) {
+	for (cdf = 0; cdf < (1 << cdf_width); cdf++) {
 		ick = clk_get_rate(clkp) / (1 + cdf);
 		if (ick < 20000000)
 			goto ick_find;
@@ -287,7 +304,7 @@ scgd_find:
 	/*
 	 * keep icccr value
 	 */
-	priv->icccr = (scgd << 2 | cdf);
+	priv->icccr = (scgd << (cdf_width) | cdf);
 
 	return 0;
 }
@@ -632,6 +649,13 @@ static int rcar_i2c_probe(struct platform_device *pdev)
 	bus_speed = 100000; /* default 100 kHz */
 	if (pdata && pdata->bus_speed)
 		bus_speed = pdata->bus_speed;
+
+	if (!pdev->id_entry) {
+		dev_err(&pdev->dev, "no entry\n");
+		return -ENODEV;
+	}
+	priv->devtype = pdev->id_entry->driver_data;
+
 	ret = rcar_i2c_clock_calculate(priv, bus_speed, dev);
 	if (ret < 0)
 		return ret;
@@ -686,6 +710,14 @@ static int rcar_i2c_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static struct platform_device_id rcar_i2c_id_table[] = {
+	{ "i2c-rcar",		I2C_RCAR_H1 },
+	{ "i2c-rcar_h1",	I2C_RCAR_H1 },
+	{ "i2c-rcar_h2",	I2C_RCAR_H2 },
+	{},
+};
+MODULE_DEVICE_TABLE(platform, rcar_i2c_id_table);
+
 static struct platform_driver rcar_i2c_driver = {
 	.driver	= {
 		.name	= "i2c-rcar",
@@ -693,6 +725,7 @@ static struct platform_driver rcar_i2c_driver = {
 	},
 	.probe		= rcar_i2c_probe,
 	.remove		= rcar_i2c_remove,
+	.id_table	= rcar_i2c_id_table,
 };
 
 module_platform_driver(rcar_i2c_driver);
-- 
1.7.9.5

Re: [PATCH v3 1/1 resend] i2c: rcar: modify I2C driver

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1134 bytes --]

On Tue, Aug 27, 2013 at 04:55:22PM +0900, Nguyen Viet Dung wrote:
> This patch modify I2C driver of rcar-H1 to usable on both rcar-H1 and rcar-H2.
> 
> Signed-off-by: Nguyen Viet Dung <nv-dung@jinso.co.jp>

Yes, this is much better. Only minor things...

> +	switch (priv->devtype) {
> +	default:
> +	case I2C_RCAR_H1:
> +		cdf_width = 2;
> +		break;
> +	case I2C_RCAR_H2:
> +		cdf_width = 3;
> +		break;
> +	}
> +

Please put the default case block at the end.

> @@ -632,6 +649,13 @@ static int rcar_i2c_probe(struct platform_device *pdev)
>  	bus_speed = 100000; /* default 100 kHz */
>  	if (pdata && pdata->bus_speed)
>  		bus_speed = pdata->bus_speed;
> +
> +	if (!pdev->id_entry) {
> +		dev_err(&pdev->dev, "no entry\n");
> +		return -ENODEV;
> +	}

This cannot happen. Since you have 'i2c-rcar' in the id_table, the
driver core will always match against the id_table and there is always a
driver_data defined.


> +	priv->devtype = pdev->id_entry->driver_data;

Basically the same, but please use

	platform_get_device_id(pdev)->driver_data;

Otherwise good!

Thanks,

   Wolfram

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v3 1/1 resend] i2c: rcar: modify I2C driver

[Nguyen Viet Dung]

Hi Wolfram,

Thank your for the guide.

I have understood your guide.
And I have released version 4 of I2C patch.

Please check it for me.

Thanks,
Nguyen Viet Dung

On 08/28/2013 06:02 PM, Wolfram Sang wrote:
> On Tue, Aug 27, 2013 at 04:55:22PM +0900, Nguyen Viet Dung wrote:
>> This patch modify I2C driver of rcar-H1 to usable on both rcar-H1 and rcar-H2.
>>
>> Signed-off-by: Nguyen Viet Dung <nv-dung@jinso.co.jp>
> Yes, this is much better. Only minor things...
>
>> +	switch (priv->devtype) {
>> +	default:
>> +	case I2C_RCAR_H1:
>> +		cdf_width = 2;
>> +		break;
>> +	case I2C_RCAR_H2:
>> +		cdf_width = 3;
>> +		break;
>> +	}
>> +
> Please put the default case block at the end.
>
>> @@ -632,6 +649,13 @@ static int rcar_i2c_probe(struct platform_device *pdev)
>>   	bus_speed = 100000; /* default 100 kHz */
>>   	if (pdata && pdata->bus_speed)
>>   		bus_speed = pdata->bus_speed;
>> +
>> +	if (!pdev->id_entry) {
>> +		dev_err(&pdev->dev, "no entry\n");
>> +		return -ENODEV;
>> +	}
> This cannot happen. Since you have 'i2c-rcar' in the id_table, the
> driver core will always match against the id_table and there is always a
> driver_data defined.
>
>
>> +	priv->devtype = pdev->id_entry->driver_data;
> Basically the same, but please use
>
> 	platform_get_device_id(pdev)->driver_data;
>
> Otherwise good!
>
> Thanks,
>
>     Wolfram