diff mbox

[v2,2/5] i2c: davinci: query STP always when NACK is received

Message ID 1417010393-30598-3-git-send-email-grygorii.strashko@ti.com
State Superseded
Headers show

Commit Message

Grygorii Strashko Nov. 26, 2014, 1:59 p.m. UTC
According to I2C specification the NACK should be handled as folows:
"When SDA remains HIGH during this ninth clock pulse, this is defined as the Not
Acknowledge signal. The master can then generate either a STOP condition to
abort the transfer, or a repeated START condition to start a new transfer."
[I2C spec Rev. 6, 3.1.6: http://www.nxp.com/documents/user_manual/UM10204.pdf]

Currently the Davinci i2c driver interrupts the transfer on receipt of a
NACK but fails to send a STOP in some situations and so makes the bus
stuck until next I2C IP reset (idle/enable).

For example, the issue will happen during SMBus read transfer which
consists from two i2c messages write command/address and read data:

S Slave Address Wr A Command Code A Sr Slave Address Rd A D1..Dn A P
<--- write -----------------------> <--- read --------------------->

The I2C client device will send NACK if it can't recognize "Command Code"
and it's expected from I2C master to query STP in this case.
But now, Davinci i2C driver will just exit with -EREMOTEIO and STP will
not be queried.

Hence, fix it by querying Stop condition (STP) always when NACK is received.

This patch fixes Davinci I2C in the same way it was done for OMAP I2C
commit cda2109a26eb ("i2c: omap: query STP always when NACK is received").

CC: Sekhar Nori <nsekhar@ti.com>
CC: Kevin Hilman <khilman@deeprootsystems.com>
CC: Santosh Shilimkar <ssantosh@kernel.org>
CC: Murali Karicheri <m-karicheri2@ti.com>
Reported-by: Hein Tibosch <hein_tibosch@yahoo.es>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 drivers/i2c/busses/i2c-davinci.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

Comments

Uwe Kleine-König Nov. 26, 2014, 3:57 p.m. UTC | #1
Hello,

I don't understand your use of "query" in the subject and later in the
commit log. Do you mean "send"?

On Wed, Nov 26, 2014 at 03:59:50PM +0200, Grygorii Strashko wrote:
> According to I2C specification the NACK should be handled as folows:
s/folows/follows/

Best regards
Uwe
Grygorii Strashko Nov. 26, 2014, 4:31 p.m. UTC | #2
On 11/26/2014 05:57 PM, Uwe Kleine-König wrote:
> Hello,
>
> I don't understand your use of "query" in the subject and later in the
> commit log. Do you mean "send"?

will change to "generate". Ok?

>
> On Wed, Nov 26, 2014 at 03:59:50PM +0200, Grygorii Strashko wrote:
>> According to I2C specification the NACK should be handled as folows:
> s/folows/follows/
ok.

Can I assume that you are ok with this patch in general?

regards,
-grygorii


--
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
Uwe Kleine-König Nov. 26, 2014, 5:15 p.m. UTC | #3
Hello Grygorii,

On Wed, Nov 26, 2014 at 06:31:22PM +0200, Grygorii Strashko wrote:
> On 11/26/2014 05:57 PM, Uwe Kleine-König wrote:
> >I don't understand your use of "query" in the subject and later in the
> >commit log. Do you mean "send"?
> 
> will change to "generate". Ok?
yes that would be better understandable

> Can I assume that you are ok with this patch in general?
Yeah, your argument that being able to send a Sr is not sensible made
sense to me. If Wolfram is happy with your patch so am I.

Best regards
Uwe
diff mbox

Patch

diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
index 7f54903..1990ae8 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -411,11 +411,9 @@  i2c_davinci_xfer_msg(struct i2c_adapter *adap, struct i2c_msg *msg, int stop)
 	if (dev->cmd_err & DAVINCI_I2C_STR_NACK) {
 		if (msg->flags & I2C_M_IGNORE_NAK)
 			return msg->len;
-		if (stop) {
-			w = davinci_i2c_read_reg(dev, DAVINCI_I2C_MDR_REG);
-			w |= DAVINCI_I2C_MDR_STP;
-			davinci_i2c_write_reg(dev, DAVINCI_I2C_MDR_REG, w);
-		}
+		w = davinci_i2c_read_reg(dev, DAVINCI_I2C_MDR_REG);
+		w |= DAVINCI_I2C_MDR_STP;
+		davinci_i2c_write_reg(dev, DAVINCI_I2C_MDR_REG, w);
 		return -EREMOTEIO;
 	}
 	return -EIO;