| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 31 Mar 2016 12:28:16 +0200
From: Jan Glauber <jglauber@...ium.com>
To: Wolfram Sang <wsa@...-dreams.de>
Cc: linux-kernel@...r.kernel.org, linux-i2c@...r.kernel.org,
David Daney <ddaney@...iumnetworks.com>,
Peter Swain <pswain@...ium.com>,
Jan Glauber <jglauber@...ium.com>
Subject: [PATCH v5 03/14] i2c: octeon: Improve error handling
From: Peter Swain <pswain@...ium.com>
Consider more status codes and improve error handling.
Distinguish handling for first and last part of a message.
TODO: Convert to use the i2c recovery framework.
Signed-off-by: Peter Swain <pswain@...ium.com>
Signed-off-by: Jan Glauber <jglauber@...ium.com>
---
drivers/i2c/busses/i2c-octeon.c | 244 +++++++++++++++++++++++++++++-----------
1 file changed, 178 insertions(+), 66 deletions(-)
diff --git a/drivers/i2c/busses/i2c-octeon.c b/drivers/i2c/busses/i2c-octeon.c
index f647667..a037245 100644
--- a/drivers/i2c/busses/i2c-octeon.c
+++ b/drivers/i2c/busses/i2c-octeon.c
@@ -56,12 +56,34 @@
#define TWSI_CTL_AAK 0x04 /* Assert ACK */
/* Some status values */
+#define STAT_ERROR 0x00
#define STAT_START 0x08
#define STAT_RSTART 0x10
#define STAT_TXADDR_ACK 0x18
+#define STAT_TXADDR_NAK 0x20
#define STAT_TXDATA_ACK 0x28
+#define STAT_TXDATA_NAK 0x30
+#define STAT_LOST_ARB_38 0x38
#define STAT_RXADDR_ACK 0x40
+#define STAT_RXADDR_NAK 0x48
#define STAT_RXDATA_ACK 0x50
+#define STAT_RXDATA_NAK 0x58
+#define STAT_SLAVE_60 0x60
+#define STAT_LOST_ARB_68 0x68
+#define STAT_SLAVE_70 0x70
+#define STAT_LOST_ARB_78 0x78
+#define STAT_SLAVE_80 0x80
+#define STAT_SLAVE_88 0x88
+#define STAT_GENDATA_ACK 0x90
+#define STAT_GENDATA_NAK 0x98
+#define STAT_SLAVE_A0 0xA0
+#define STAT_SLAVE_A8 0xA8
+#define STAT_LOST_ARB_B0 0xB0
+#define STAT_SLAVE_LOST 0xB8
+#define STAT_SLAVE_NAK 0xC0
+#define STAT_SLAVE_ACK 0xC8
+#define STAT_AD2W_ACK 0xD0
+#define STAT_AD2W_NAK 0xD8
#define STAT_IDLE 0xF8
/* TWSI_INT values */
@@ -79,6 +101,8 @@ struct octeon_i2c {
struct device *dev;
};
+static int reset_how;
+
/**
* octeon_i2c_write_sw - write an I2C core register
* @i2c: The struct octeon_i2c
@@ -186,7 +210,6 @@ static irqreturn_t octeon_i2c_isr(int irq, void *dev_id)
return IRQ_HANDLED;
}
-
static int octeon_i2c_test_iflg(struct octeon_i2c *i2c)
{
return (octeon_i2c_read_sw(i2c, SW_TWSI_EOP_TWSI_CTL) & TWSI_CTL_IFLG) != 0;
@@ -214,6 +237,66 @@ static int octeon_i2c_wait(struct octeon_i2c *i2c)
return 0;
}
+static int octeon_i2c_lost_arb(u8 code, int final_read)
+{
+ switch (code) {
+ /* Arbitration lost */
+ case STAT_LOST_ARB_38:
+ case STAT_LOST_ARB_68:
+ case STAT_LOST_ARB_78:
+ case STAT_LOST_ARB_B0:
+ return -EAGAIN;
+
+ /* Being addressed as slave, should back off & listen */
+ case STAT_SLAVE_60:
+ case STAT_SLAVE_70:
+ case STAT_GENDATA_ACK:
+ case STAT_GENDATA_NAK:
+ return -EIO;
+
+ /* Core busy as slave */
+ case STAT_SLAVE_80:
+ case STAT_SLAVE_88:
+ case STAT_SLAVE_A0:
+ case STAT_SLAVE_A8:
+ case STAT_SLAVE_LOST:
+ case STAT_SLAVE_NAK:
+ case STAT_SLAVE_ACK:
+ return -EIO;
+
+ /* ACK allowed on pre-terminal bytes only */
+ case STAT_RXDATA_ACK:
+ if (!final_read)
+ return 0;
+ return -EAGAIN;
+
+ /* NAK allowed on terminal byte only */
+ case STAT_RXDATA_NAK:
+ if (final_read)
+ return 0;
+ return -EAGAIN;
+ case STAT_TXDATA_NAK:
+ case STAT_TXADDR_NAK:
+ case STAT_RXADDR_NAK:
+ case STAT_AD2W_NAK:
+ return -EAGAIN;
+ }
+ return 0;
+}
+
+static int check_arb(struct octeon_i2c *i2c, int final_read)
+{
+ return octeon_i2c_lost_arb(octeon_i2c_read_sw(i2c,
+ SW_TWSI_EOP_TWSI_STAT), final_read);
+}
+
+/* send STOP to the bus */
+static void octeon_i2c_stop(struct octeon_i2c *i2c)
+{
+ octeon_i2c_write_sw(i2c, SW_TWSI_EOP_TWSI_CTL,
+ TWSI_CTL_ENAB | TWSI_CTL_STP);
+}
+
/* calculate and set clock divisors */
static void octeon_i2c_set_clock(struct octeon_i2c *i2c)
{
@@ -277,71 +360,103 @@ static int octeon_i2c_init_lowlevel(struct octeon_i2c *i2c)
return -EIO;
}
+/*
+ * TWSI state seems stuck. Not sure if it's TWSI-engine state or something
+ * else on bus. The initial _stop() is always harmless, it just resets state
+ * machine, does not _transmit_ STOP unless engine was active.
+ */
+static int start_unstick(struct octeon_i2c *i2c)
+{
+ octeon_i2c_stop(i2c);
+
+ /*
+ * Response is escalated over successive calls,
+ * as EAGAIN provokes retries from i2c/core.
+ */
+ switch (reset_how++ % 4) {
+ case 0:
+ /* just the stop above */
+ break;
+ case 1:
+ /*
+ * Controller refused to send start flag. May be a
+ * client is holding SDA low? Let's try to free it.
+ */
+ octeon_i2c_unblock(i2c);
+ break;
+ case 2:
+ /* re-init our TWSI hardware */
+ octeon_i2c_init_lowlevel(i2c);
+ break;
+ default:
+ /* retry in caller */
+ reset_how = 0;
+ return -EAGAIN;
+ }
+ return 0;
+}
+
/**
* octeon_i2c_start - send START to the bus
* @i2c: The struct octeon_i2c
+ * @first: first msg in combined operation?
*
* Returns 0 on success, otherwise a negative errno.
*/
-static int octeon_i2c_start(struct octeon_i2c *i2c)
+static int octeon_i2c_start(struct octeon_i2c *i2c, int first)
{
int result;
u8 data;
- octeon_i2c_write_sw(i2c, SW_TWSI_EOP_TWSI_CTL,
- TWSI_CTL_ENAB | TWSI_CTL_STA);
+ while (1) {
+ octeon_i2c_write_sw(i2c, SW_TWSI_EOP_TWSI_CTL,
+ TWSI_CTL_ENAB | TWSI_CTL_STA);
- result = octeon_i2c_wait(i2c);
- if (result) {
- if (octeon_i2c_read_sw(i2c, SW_TWSI_EOP_TWSI_STAT) == STAT_IDLE) {
- /*
- * Controller refused to send start flag May
- * be a client is holding SDA low - let's try
- * to free it.
- */
- octeon_i2c_unblock(i2c);
- octeon_i2c_write_sw(i2c, SW_TWSI_EOP_TWSI_CTL,
- TWSI_CTL_ENAB | TWSI_CTL_STA);
- result = octeon_i2c_wait(i2c);
+ result = octeon_i2c_wait(i2c);
+ data = octeon_i2c_read_sw(i2c, SW_TWSI_EOP_TWSI_STAT);
+
+ switch (data) {
+ case STAT_START:
+ case STAT_RSTART:
+ if (!first)
+ return -EAGAIN;
+ reset_how = 0;
+ return 0;
+ case STAT_RXADDR_ACK:
+ if (first)
+ return -EAGAIN;
+ return start_unstick(i2c);
+ /*
+ * case STAT_IDLE:
+ * case STAT_ERROR:
+ */
+ default:
+ if (!first)
+ return -EAGAIN;
+ start_unstick(i2c);
}
- if (result)
- return result;
}
-
- data = octeon_i2c_read_sw(i2c, SW_TWSI_EOP_TWSI_STAT);
- if ((data != STAT_START) && (data != STAT_RSTART)) {
- dev_err(i2c->dev, "%s: bad status (0x%x)\n", __func__, data);
- return -EIO;
- }
-
return 0;
}
-/* send STOP to the bus */
-static void octeon_i2c_stop(struct octeon_i2c *i2c)
-{
- octeon_i2c_write_sw(i2c, SW_TWSI_EOP_TWSI_CTL,
- TWSI_CTL_ENAB | TWSI_CTL_STP);
-}
-
/**
* octeon_i2c_write - send data to the bus via low-level controller
* @i2c: The struct octeon_i2c
* @target: Target address
* @data: Pointer to the data to be sent
* @length: Length of the data
+ * @last: is last msg in combined operation?
*
* The address is sent over the bus, then the data.
*
* Returns 0 on success, otherwise a negative errno.
*/
static int octeon_i2c_write(struct octeon_i2c *i2c, int target,
- const u8 *data, int length)
+ const u8 *data, int length, int first, int last)
{
int i, result;
- u8 tmp;
- result = octeon_i2c_start(i2c);
+ result = octeon_i2c_start(i2c, first);
if (result)
return result;
@@ -353,14 +468,9 @@ static int octeon_i2c_write(struct octeon_i2c *i2c, int target,
return result;
for (i = 0; i < length; i++) {
- tmp = octeon_i2c_read_sw(i2c, SW_TWSI_EOP_TWSI_STAT);
-
- if ((tmp != STAT_TXADDR_ACK) && (tmp != STAT_TXDATA_ACK)) {
- dev_err(i2c->dev,
- "%s: bad status before write (0x%x)\n",
- __func__, tmp);
- return -EIO;
- }
+ result = check_arb(i2c, false);
+ if (result)
+ return result;
octeon_i2c_write_sw(i2c, SW_TWSI_EOP_TWSI_DATA, data[i]);
octeon_i2c_write_sw(i2c, SW_TWSI_EOP_TWSI_CTL, TWSI_CTL_ENAB);
@@ -368,6 +478,9 @@ static int octeon_i2c_write(struct octeon_i2c *i2c, int target,
result = octeon_i2c_wait(i2c);
if (result)
return result;
+ result = check_arb(i2c, false);
+ if (result)
+ return result;
}
return 0;
@@ -379,54 +492,51 @@ static int octeon_i2c_write(struct octeon_i2c *i2c, int target,
* @target: Target address
* @data: Pointer to the location to store the data
* @rlength: Length of the data
+ * @phase: which phase of a combined operation.
* @recv_len: flag for length byte
*
* The address is sent over the bus, then the data is read.
*
* Returns 0 on success, otherwise a negative errno.
*/
-static int octeon_i2c_read(struct octeon_i2c *i2c, int target,
- u8 *data, u16 *rlength, bool recv_len)
+static int octeon_i2c_read(struct octeon_i2c *i2c, int target, u8 *data,
+ u16 *rlength, bool first, bool last, bool recv_len)
{
+ u8 ctl = TWSI_CTL_ENAB | TWSI_CTL_AAK;
int i, result, length = *rlength;
u8 tmp;
if (length < 1)
return -EINVAL;
- result = octeon_i2c_start(i2c);
+ result = octeon_i2c_start(i2c, first);
if (result)
return result;
octeon_i2c_write_sw(i2c, SW_TWSI_EOP_TWSI_DATA, (target << 1) | 1);
- octeon_i2c_write_sw(i2c, SW_TWSI_EOP_TWSI_CTL, TWSI_CTL_ENAB);
-
- result = octeon_i2c_wait(i2c);
- if (result)
- return result;
- for (i = 0; i < length; i++) {
+ for (i = 0; i < length; ) {
tmp = octeon_i2c_read_sw(i2c, SW_TWSI_EOP_TWSI_STAT);
+ result = octeon_i2c_lost_arb(tmp, !(ctl & TWSI_CTL_AAK));
+ if (result)
+ return result;
- if ((tmp != STAT_RXDATA_ACK) && (tmp != STAT_RXADDR_ACK)) {
- dev_err(i2c->dev,
- "%s: bad status before read (0x%x)\n",
- __func__, tmp);
- return -EIO;
+ switch (tmp) {
+ case STAT_RXDATA_ACK:
+ case STAT_RXDATA_NAK:
+ data[i++] = octeon_i2c_read_sw(i2c, SW_TWSI_EOP_TWSI_DATA);
}
- if (i + 1 < length)
- octeon_i2c_write_sw(i2c, SW_TWSI_EOP_TWSI_CTL,
- TWSI_CTL_ENAB | TWSI_CTL_AAK);
- else
- octeon_i2c_write_sw(i2c, SW_TWSI_EOP_TWSI_CTL,
- TWSI_CTL_ENAB);
+ /* NAK last recv'd byte, as a no-more-please */
+ if (last && i == length - 1)
+ ctl &= ~TWSI_CTL_AAK;
+ /* clr iflg to allow next event */
+ octeon_i2c_write_sw(i2c, SW_TWSI_EOP_TWSI_CTL, ctl);
result = octeon_i2c_wait(i2c);
if (result)
return result;
- data[i] = octeon_i2c_read_sw(i2c, SW_TWSI_EOP_TWSI_DATA);
if (recv_len && i == 0) {
if (data[i] > I2C_SMBUS_BLOCK_MAX + 1) {
dev_err(i2c->dev,
@@ -457,6 +567,7 @@ static int octeon_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
for (i = 0; ret == 0 && i < num; i++) {
struct i2c_msg *pmsg = &msgs[i];
+ bool last = (i == (num - 1));
dev_dbg(i2c->dev,
"Doing %s %d byte(s) to/from 0x%02x - %d of %d messages\n",
@@ -464,10 +575,11 @@ static int octeon_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
pmsg->len, pmsg->addr, i + 1, num);
if (pmsg->flags & I2C_M_RD)
ret = octeon_i2c_read(i2c, pmsg->addr, pmsg->buf,
- &pmsg->len, pmsg->flags & I2C_M_RECV_LEN);
+ &pmsg->len, !i, last,
+ pmsg->flags & I2C_M_RECV_LEN);
else
ret = octeon_i2c_write(i2c, pmsg->addr, pmsg->buf,
- pmsg->len);
+ pmsg->len, !i, last);
}
octeon_i2c_stop(i2c);
--
1.9.1
Powered by blists - more mailing lists