[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-i2c
Subject:    Re: [PATCH] i2c: tegra: Retry transfer when no_ack status is detected
From:       Thierry Reding <thierry.reding () avionic-design ! de>
Date:       2013-04-22 7:02:45
Message-ID: 20130422070245.GA17002 () avionic-0098 ! mockup ! avionic-design ! de
[Download RAW message or body]

On Fri, Apr 19, 2013 at 01:43:24PM +0200, Hendrik Lippek wrote:
> In such conditions return -EAGAIN from driver.
> This will cause the i2c-core to retry
> the transmission as per the retry count and time-out specified by the
> platform data of the adapter.
> 
> This fix is adapted from chromeos
> commit:	16d971dd7ee9571746ff1d352fa3c0092a36478d
> 
> Signed-off-by: Hendrik Lippek <hendrik.lippek@avionic-design.de>
> ---
>  drivers/i2c/busses/i2c-tegra.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Patches for I2C driver should go to the respective maintainers, not only
the Tegra mailing list. You can use scripts/get_maintainer.pl to obtain
a list of people and mailing lists to Cc (though you may want to apply
common sense and strip that list down a bit). In this case at least
Wolfram Sang and the linux-i2c mailing lists should be Cc'ed. I've added
them now.

Also it would be useful if you could describe the error case that this
patch fixes in more detail. That way people know the context and can
more easily judge whether the fix is appropriate or suggest alternative
solutions to the problem.

> diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
> index c1bd68d..81a6170 100644
> --- a/drivers/i2c/busses/i2c-tegra.c
> +++ b/drivers/i2c/busses/i2c-tegra.c
> @@ -756,7 +756,7 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_bus *i2c_bus,
>  	if (i2c_dev->msg_err == I2C_ERR_NO_ACK) {
>  		if (msg->flags & I2C_M_IGNORE_NAK)
>  			return 0;
> -		return -EREMOTEIO;
> +		return -EAGAIN;
>  	}
>  
>  	if (i2c_dev->msg_err & I2C_ERR_UNEXPECTED_STATUS)

I've looked a bit at what other drivers do when receiving a NAK, and
returning -EREMOTEIO is by far the most common way to deal with it.
Furthermore I'm not sure if repeating a transfer when no ACK is received
is the right thing to do. After all there's usually a good reason for a
missing ACK (e.g. slave not present). That also means if this patch is
applied, every access to a slave that isn't present will be retried
several times before failing. It also means that if the transfer finally
fails because the retry count reaches 0, the I2C core will actually
propagate -EAGAIN and hide the original cause of the error.

Perhaps a better alternative would be to use the I2C_M_IGNORE_NAK flag.
There is some useful information in Documentation/i2c/i2c-protocol,
section "Modified transactions". It seems like this isn't a general
problem with the Tegra I2C controller but rather with the specific chip
on this hardware. Can you try if adding the I2C_M_IGNORE_NAK flag to the
transfers makes the problem disappear as well?

Thierry

[Attachment #3 (application/pgp-signature)]
--
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

[prev in list] [next in list] [prev in thread] [next in thread]