[PATCH] i2c-mxs: fixed error message in pio transfer

[PATCH] i2c-mxs: fixed error message in pio transfer

[Janusz Uzycki]

If I2C_M_RD flag is set SELECT command is sent
and afterward READ command.
The patch fixes READ command to return READ failure error message
instead of SELECT failure error message.

Signed-off-by: Janusz Uzycki <j.uzycki-9tnw74Q4ehaHKKo6LODCOg@public.gmane.org>
---
 drivers/i2c/busses/i2c-mxs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-mxs.c b/drivers/i2c/busses/i2c-mxs.c
index 0cde4e6..87ee72d 100644
--- a/drivers/i2c/busses/i2c-mxs.c
+++ b/drivers/i2c/busses/i2c-mxs.c
@@ -429,7 +429,7 @@ static int mxs_i2c_pio_setup_xfer(struct i2c_adapter *adap,
 		ret = mxs_i2c_pio_wait_xfer_end(i2c);
 		if (ret) {
 			dev_err(i2c->dev,
-				"PIO: Failed to send SELECT command!\n");
+				"PIO: Failed to send READ command!\n");
 			goto cleanup;
 		}
 
-- 
1.7.11.3

[PATCH] i2c-mxs: fixed PIO NACK error instead of timeout (1000ms)

[Janusz Uzycki]

i2cdetect scanned i2c bus very slow if address was not occupied by any device.

Signed-off-by: Janusz Uzycki <j.uzycki-9tnw74Q4ehaHKKo6LODCOg@public.gmane.org>
---
 drivers/i2c/busses/i2c-mxs.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/i2c/busses/i2c-mxs.c b/drivers/i2c/busses/i2c-mxs.c
index 87ee72d..35ae448 100644
--- a/drivers/i2c/busses/i2c-mxs.c
+++ b/drivers/i2c/busses/i2c-mxs.c
@@ -307,6 +307,10 @@ static int mxs_i2c_pio_wait_xfer_end(struct mxs_i2c_dev *i2c)
 	unsigned long timeout = jiffies + msecs_to_jiffies(1000);
 
 	while (readl(i2c->regs + MXS_I2C_CTRL0) & MXS_I2C_CTRL0_RUN) {
+		if (readl(i2c->regs + MXS_I2C_CTRL1) &
+				MXS_I2C_CTRL1_NO_SLAVE_ACK_IRQ)
+			return -ENXIO;
 		if (time_after(jiffies, timeout))
 			return -ETIMEDOUT;
 		cond_resched();
-- 
1.7.11.3

Re: [PATCH] i2c-mxs: fixed PIO NACK error instead of timeout (1000ms)

[Marek Vasut]

On Tuesday, September 09, 2014 at 06:19:31 PM, Janusz Uzycki wrote:
> i2cdetect scanned i2c bus very slow if address was not occupied by any
> device.

This still fails to explain how the patch fixes the issue. You see, I don't want 
to abuse you unnecessarily, but the commit message serves mostly for the purpose 
of explaining the issue (which you did) and how the change you implemented fixes 
the problem.

So in this case, you should explain that you can use this NO_SLAVE_ACK bit to 
figure out if the peripheral is present. You should also explain that even 
though the mxs_i2c_pio_wait_xfer_end() is called from multiple places, adding 
this particular check will not impact all those other places.

I hope this makes sense. Thank you for the effort though, I really appreciate 
your contributions!

> Signed-off-by: Janusz Uzycki <j.uzycki-9tnw74Q4ehaHKKo6LODCOg@public.gmane.org>
> ---
>  drivers/i2c/busses/i2c-mxs.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/i2c/busses/i2c-mxs.c b/drivers/i2c/busses/i2c-mxs.c
> index 87ee72d..35ae448 100644
> --- a/drivers/i2c/busses/i2c-mxs.c
> +++ b/drivers/i2c/busses/i2c-mxs.c
> @@ -307,6 +307,10 @@ static int mxs_i2c_pio_wait_xfer_end(struct
> mxs_i2c_dev *i2c) unsigned long timeout = jiffies +
> msecs_to_jiffies(1000);
> 
>  	while (readl(i2c->regs + MXS_I2C_CTRL0) & MXS_I2C_CTRL0_RUN) {
> +		if (readl(i2c->regs + MXS_I2C_CTRL1) &
> +				MXS_I2C_CTRL1_NO_SLAVE_ACK_IRQ)
> +			return -ENXIO;
>  		if (time_after(jiffies, timeout))
>  			return -ETIMEDOUT;
>  		cond_resched();

Best regards,
Marek Vasut

Re: [PATCH] i2c-mxs: fixed PIO NACK error instead of timeout (1000ms)

[Wolfram Sang]

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

On Tue, Sep 09, 2014 at 06:19:31PM +0200, Janusz Uzycki wrote:
> i2cdetect scanned i2c bus very slow if address was not occupied by any device.
> 
> Signed-off-by: Janusz Uzycki <j.uzycki-9tnw74Q4ehaHKKo6LODCOg@public.gmane.org>

I would apply this patch and produce a fitting commit message out of the
text you already sent, so you get an idea what is apropriate. However, I
can't apply the patch. How did you create it?

> ---
>  drivers/i2c/busses/i2c-mxs.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/i2c/busses/i2c-mxs.c b/drivers/i2c/busses/i2c-mxs.c
> index 87ee72d..35ae448 100644
> --- a/drivers/i2c/busses/i2c-mxs.c
> +++ b/drivers/i2c/busses/i2c-mxs.c
> @@ -307,6 +307,10 @@ static int mxs_i2c_pio_wait_xfer_end(struct mxs_i2c_dev *i2c)

... the 307,10 here should be 307,9. Did you manually edit it?

>  	unsigned long timeout = jiffies + msecs_to_jiffies(1000);
>  
>  	while (readl(i2c->regs + MXS_I2C_CTRL0) & MXS_I2C_CTRL0_RUN) {
> +		if (readl(i2c->regs + MXS_I2C_CTRL1) &
> +				MXS_I2C_CTRL1_NO_SLAVE_ACK_IRQ)
> +			return -ENXIO;
>  		if (time_after(jiffies, timeout))
>  			return -ETIMEDOUT;
>  		cond_resched();
> -- 
> 1.7.11.3
> 

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

Re: [PATCH] i2c-mxs: fixed PIO NACK error instead of timeout (1000ms)

[Marek Vasut]

On Saturday, September 20, 2014 at 02:49:36 PM, Wolfram Sang wrote:
> On Tue, Sep 09, 2014 at 06:19:31PM +0200, Janusz Uzycki wrote:
> > i2cdetect scanned i2c bus very slow if address was not occupied by any
> > device.
> > 
> > Signed-off-by: Janusz Uzycki <j.uzycki-9tnw74Q4ehaHKKo6LODCOg@public.gmane.org>
> 
> I would apply this patch and produce a fitting commit message out of the
> text you already sent, so you get an idea what is apropriate. However, I
> can't apply the patch. How did you create it?
> 

I'd say it's based on Linux 3.14 . Btw. you could have pasted the commit message 
here too ;-)

Best regards,
Marek Vasut

Re: [PATCH] i2c-mxs: fixed PIO NACK error instead of timeout (1000ms)

[Wolfram Sang]

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

On Sat, Sep 20, 2014 at 03:22:00PM +0200, Marek Vasut wrote:
> On Saturday, September 20, 2014 at 02:49:36 PM, Wolfram Sang wrote:
> > On Tue, Sep 09, 2014 at 06:19:31PM +0200, Janusz Uzycki wrote:
> > > i2cdetect scanned i2c bus very slow if address was not occupied by any
> > > device.
> > > 
> > > Signed-off-by: Janusz Uzycki <j.uzycki-9tnw74Q4ehaHKKo6LODCOg@public.gmane.org>
> > 
> > I would apply this patch and produce a fitting commit message out of the
> > text you already sent, so you get an idea what is apropriate. However, I
> > can't apply the patch. How did you create it?
> > 
> 
> I'd say it's based on Linux 3.14 . Btw. you could have pasted the commit message 
> here too ;-)

I can't apply the patch, it needs to be resent. And it is not wrong
context IMO, the numbers say 4 lines will be added while the patch only
contains 3. I could fix it manually, but I rather have a proper patch.


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

Re: [PATCH] i2c-mxs: fixed error message in pio transfer

[Wolfram Sang]

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

On Tue, Sep 09, 2014 at 06:19:30PM +0200, Janusz Uzycki wrote:
> If I2C_M_RD flag is set SELECT command is sent
> and afterward READ command.
> The patch fixes READ command to return READ failure error message
> instead of SELECT failure error message.
> 
> Signed-off-by: Janusz Uzycki <j.uzycki-9tnw74Q4ehaHKKo6LODCOg@public.gmane.org>

Applied to for-next with minor updates to the commit message, thanks!

Marek, I added your ack since you said you are fine with the change
itself. Thanks for reviewing the Freescale drivers BTW, much
appreciated!


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

Re: [PATCH] i2c-mxs: fixed error message in pio transfer

[Marek Vasut]

On Saturday, September 20, 2014 at 02:39:53 PM, Wolfram Sang wrote:
> On Tue, Sep 09, 2014 at 06:19:30PM +0200, Janusz Uzycki wrote:
> > If I2C_M_RD flag is set SELECT command is sent
> > and afterward READ command.
> > The patch fixes READ command to return READ failure error message
> > instead of SELECT failure error message.
> > 
> > Signed-off-by: Janusz Uzycki <j.uzycki-9tnw74Q4ehaHKKo6LODCOg@public.gmane.org>
> 
> Applied to for-next with minor updates to the commit message, thanks!
> 
> Marek, I added your ack since you said you are fine with the change
> itself.

Ah yes, sure.

> Thanks for reviewing the Freescale drivers BTW, much
> appreciated!

Well you're welcome, though I don't do much, really.

Best regards,
Marek Vasut