[PATCH 3/4] i2c: rcar: do not print error if device nacks transfer

[PATCH 3/4] i2c: rcar: do not print error if device nacks transfer

[Ben Dooks]

The i2c-rcar driver currently prints an error message if the master_xfer
callback fails. However if the bus is being probed then lots of NAKs
will be generated, causing the output of a number of errors printed.

To solve this, disable the print if the error is not -EREMOTEIO.

An example of running i2cdetect:

10: i2c-rcar e6530000.i2c: error -121 : 15
-- i2c-rcar e6530000.i2c: error -121 : 15
-- 12 i2c-rcar e6530000.i2c: error -121 : 15
-- i2c-rcar e6530000.i2c: error -121 : 15
-- i2c-rcar e6530000.i2c: error -121 : 15
-- i2c-rcar e6530000.i2c: error -121 : 15
-- i2c-rcar e6530000.i2c: error -121 : 15
-- i2c-rcar e6530000.i2c: error -121 : 15

Cc: Simon Horman <horms+renesas@verge.net.au>
Cc: Wolfram Sang <wsa@the-dreams.de>
Cc: linux-i2c@vger.kernel.org
Cc: linux-sh@vger.kernel.org
Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
 drivers/i2c/busses/i2c-rcar.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index 2c2fd7c..0d25104 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -623,7 +623,7 @@ static int rcar_i2c_master_xfer(struct i2c_adapter *adap,
 
 	pm_runtime_put(dev);
 
-	if (ret < 0)
+	if (ret < 0 && ret != -EREMOTEIO)
 		dev_err(dev, "error %d : %x\n", ret, priv->flags);
 
 	return ret;
-- 
1.8.5.2

[PATCH 4/4] i2c: rcar: use devm_clk_get to ensure clock is properly ref-counted

[Ben Dooks]

The current i2c-rcar driver does clk_get() without a corresponding
clk_put(). Add the clk to the driver private data and then get it
with the devm functions so that it is released when the driver is
unbound.

Note, we do not call clk_prepare_enable() at this point due to the
very possible magic that is being done by the pm_runtime system
underneath the driver.

Cc: Simon Horman <horms+renesas@verge.net.au>
Cc: Wolfram Sang <wsa@the-dreams.de>
Cc: linux-i2c@vger.kernel.org
Cc: linux-sh@vger.kernel.org
Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
 drivers/i2c/busses/i2c-rcar.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index 0d25104..6fd716e 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -111,6 +111,7 @@ struct rcar_i2c_priv {
 	void __iomem *io;
 	struct i2c_adapter adap;
 	struct i2c_msg	*msg;
+	struct clk *clk;
 
 	spinlock_t lock;
 	wait_queue_head_t wait;
@@ -227,18 +228,12 @@ static int rcar_i2c_clock_calculate(struct rcar_i2c_priv *priv,
 				    u32 bus_speed,
 				    struct device *dev)
 {
-	struct clk *clkp = clk_get(dev, NULL);
 	u32 scgd, cdf;
 	u32 round, ick;
 	u32 scl;
 	u32 cdf_width;
 	unsigned long rate;
 
-	if (IS_ERR(clkp)) {
-		dev_err(dev, "couldn't get clock\n");
-		return PTR_ERR(clkp);
-	}
-
 	switch (priv->devtype) {
 	case I2C_RCAR_GEN1:
 		cdf_width = 2;
@@ -266,7 +261,7 @@ static int rcar_i2c_clock_calculate(struct rcar_i2c_priv *priv,
 	 * clkp : peripheral_clk
 	 * F[]  : integer up-valuation
 	 */
-	rate = clk_get_rate(clkp);
+	rate = clk_get_rate(priv->clk);
 	cdf = rate / 20000000;
 	if (cdf >= 1 << cdf_width) {
 		dev_err(dev, "Input clock %lu too high\n", rate);
@@ -308,7 +303,7 @@ static int rcar_i2c_clock_calculate(struct rcar_i2c_priv *priv,
 
 scgd_find:
 	dev_dbg(dev, "clk %d/%d(%lu), round %u, CDF:0x%x, SCGD: 0x%x\n",
-		scl, bus_speed, clk_get_rate(clkp), round, cdf, scgd);
+		scl, bus_speed, clk_get_rate(priv->clk), round, cdf, scgd);
 
 	/*
 	 * keep icccr value
@@ -664,6 +659,12 @@ static int rcar_i2c_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
+	priv->clk = devm_clk_get(dev, NULL);
+	if (IS_ERR(priv->clk)) {
+		dev_err(dev, "cannot get clock\n");
+		return -ENOENT;
+	}
+
 	bus_speed = 100000; /* default 100 kHz */
 	ret = of_property_read_u32(dev->of_node, "clock-frequency", &bus_speed);
 	if (ret < 0 && pdata && pdata->bus_speed)
-- 
1.8.5.2

Re: [PATCH 3/4] i2c: rcar: do not print error if device nacks transfer

[Wolfram Sang]

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

On Fri, Jan 17, 2014 at 06:11:37PM +0000, Ben Dooks wrote:
> The i2c-rcar driver currently prints an error message if the master_xfer
> callback fails. However if the bus is being probed then lots of NAKs
> will be generated, causing the output of a number of errors printed.
> 
> To solve this, disable the print if the error is not -EREMOTEIO.

Basically OK. Yet, according to 'fault-codes" it should be -ENXIO. May
be a good time to fix this as well.

Also, you might want to print for -EIO only? Or do you want to see
-EAGAIN, too?


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

Re: [PATCH 4/4] i2c: rcar: use devm_clk_get to ensure clock is properly ref-counted

[Wolfram Sang]

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

On Fri, Jan 17, 2014 at 06:11:38PM +0000, Ben Dooks wrote:
> The current i2c-rcar driver does clk_get() without a corresponding
> clk_put(). Add the clk to the driver private data and then get it
> with the devm functions so that it is released when the driver is
> unbound.
> 

>  
> +	priv->clk = devm_clk_get(dev, NULL);
> +	if (IS_ERR(priv->clk)) {
> +		dev_err(dev, "cannot get clock\n");
> +		return -ENOENT;
> +	}

Return the error here?


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

Re: [PATCH 3/4] i2c: rcar: do not print error if device nacks transfer

[Ben Dooks]

On 24/01/14 17:11, Wolfram Sang wrote:
> On Fri, Jan 17, 2014 at 06:11:37PM +0000, Ben Dooks wrote:
>> The i2c-rcar driver currently prints an error message if the master_xfer
>> callback fails. However if the bus is being probed then lots of NAKs
>> will be generated, causing the output of a number of errors printed.
>>
>> To solve this, disable the print if the error is not -EREMOTEIO.
>
> Basically OK. Yet, according to 'fault-codes" it should be -ENXIO. May
> be a good time to fix this as well.
>
> Also, you might want to print for -EIO only? Or do you want to see
> -EAGAIN, too?

Ok, although the change to the error code should probably be a separate
patch instead of modifying this one.

-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

Re: [PATCH 4/4] i2c: rcar: use devm_clk_get to ensure clock is properly ref-counted

[Ben Dooks]

On 24/01/14 17:12, Wolfram Sang wrote:
> On Fri, Jan 17, 2014 at 06:11:38PM +0000, Ben Dooks wrote:
>> The current i2c-rcar driver does clk_get() without a corresponding
>> clk_put(). Add the clk to the driver private data and then get it
>> with the devm functions so that it is released when the driver is
>> unbound.
>>
>
>>
>> +	priv->clk = devm_clk_get(dev, NULL);
>> +	if (IS_ERR(priv->clk)) {
>> +		dev_err(dev, "cannot get clock\n");
>> +		return -ENOENT;
>> +	}
>
> Return the error here?

Yes, will change this to be PTR_ERR(priv->clk) and re-send

-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

Re: [PATCH 3/4] i2c: rcar: do not print error if device nacks transfer

[Ben Dooks]

On 24/01/14 17:11, Wolfram Sang wrote:
> On Fri, Jan 17, 2014 at 06:11:37PM +0000, Ben Dooks wrote:
>> The i2c-rcar driver currently prints an error message if the master_xfer
>> callback fails. However if the bus is being probed then lots of NAKs
>> will be generated, causing the output of a number of errors printed.
>>
>> To solve this, disable the print if the error is not -EREMOTEIO.
>
> Basically OK. Yet, according to 'fault-codes" it should be -ENXIO. May
> be a good time to fix this as well.
>
> Also, you might want to print for -EIO only? Or do you want to see
> -EAGAIN, too.

I'm not sure, I've not seen any EAGAIN replies from the driver.
I will leave this for a future patch.


-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius