diff mbox

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

Message ID 1410279571-4552-2-git-send-email-j.uzycki@elproma.com.pl
State Superseded
Headers show

Commit Message

Janusz Użycki Sept. 9, 2014, 4:19 p.m. UTC
i2cdetect scanned i2c bus very slow if address was not occupied by any device.

Signed-off-by: Janusz Uzycki <j.uzycki@elproma.com.pl>
---
 drivers/i2c/busses/i2c-mxs.c | 4 ++++
 1 file changed, 4 insertions(+)


--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Marek Vasut Sept. 9, 2014, 6:56 p.m. UTC | #1
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@elproma.com.pl>
> ---
>  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
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wolfram Sang Sept. 20, 2014, 12:49 p.m. UTC | #2
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@elproma.com.pl>

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
>
Marek Vasut Sept. 20, 2014, 1:22 p.m. UTC | #3
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@elproma.com.pl>
> 
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wolfram Sang Sept. 22, 2014, 3:56 p.m. UTC | #4
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@elproma.com.pl>
> > 
> > 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.
diff mbox

Patch

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