Skip to contentSkip to conversation

i2c: davinci: Avoid sending to own address

PATCH Superseded Alexander Sverdlin 3 participants 5 messages May 13, 2015 – Jun 10, 2015 2 messages not in the archive

Subsystems (2)
I2C subsystem host drivers TI DaVinci machine support
lore
Share & tools
lore

Alexander Sverdlin

author
#0 lore raw

PATCH v2 1/1 i2c: davinci: Avoid sending to own address

Sending a message to own address locks the controller up in very bizarre state, it behaves as slave even if MDR register clearly states master. The controller remains in this state until reset. To avoid unnecessary timeouts simply avoid sending to own address. The controller cannot do this any way. Also, do not enable AAS IRQ, as the slave mode is not supported by the driver and the only possibility to trigger this IRQ is to send to own address.

Signed-off-by Alexander Sverdlin <[email protected]>

---
Changes in v2:
- rebased on 110bc76729d4 of Linus's tree;

 drivers/i2c/busses/i2c-davinci.c |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)
drivers/i2c/busses/i2c-davinci.c +6 -3
index 4788a32..df96ab6 100644
@@ -41,8 +41,8 @@
 
 #define DAVINCI_I2C_TIMEOUT	(1*HZ)
 #define DAVINCI_I2C_MAX_TRIES	2
-#define I2C_DAVINCI_INTR_ALL    (DAVINCI_I2C_IMR_AAS | \
-				 DAVINCI_I2C_IMR_SCD | \
+#define DAVINCI_I2C_OWN_ADDRESS	0x08
+#define I2C_DAVINCI_INTR_ALL    (DAVINCI_I2C_IMR_SCD | \
 				 DAVINCI_I2C_IMR_ARDY | \
 				 DAVINCI_I2C_IMR_NACK | \
 				 DAVINCI_I2C_IMR_AL)

@@ -233,7 +233,7 @@ static int i2c_davinci_init(struct davinci_i2c_dev *dev)
 	/* Respond at reserved "SMBus Host" slave address" (and zero);
 	 * we seem to have no option to not respond...
 	 */
-	davinci_i2c_write_reg(dev, DAVINCI_I2C_OAR_REG, 0x08);
+	davinci_i2c_write_reg(dev, DAVINCI_I2C_OAR_REG, DAVINCI_I2C_OWN_ADDRESS);
 
 	dev_dbg(dev->dev, "PSC  = %d\n",
 		davinci_i2c_read_reg(dev, DAVINCI_I2C_PSC_REG));

@@ -390,6 +390,9 @@ i2c_davinci_xfer_msg(struct i2c_adapter *adap, struct i2c_msg *msg, int stop)
 	u16 w;
 	unsigned long time_left;
 
+	if (msg->addr == DAVINCI_I2C_OWN_ADDRESS)
+		return -EIO;
+
 	/* Introduce a delay, required for some boards (e.g Davinci EVM) */
 	if (pdata->bus_delay)
 		udelay(pdata->bus_delay);

A message that is not in the archive (sent off-list or not yet received): [email protected]

Reviewed-by
#2 lore raw
Quoted 7 lines · Alexander Sverdlin

Sending a message to own address locks the controller up in very bizarre state, it behaves as slave even if MDR register clearly states master. The controller remains in this state until reset. To avoid unnecessary timeouts simply avoid sending to own address. The controller cannot do this any way. Also, do not enable AAS IRQ, as the slave mode is not supported by the driver and the only possibility to trigger this IRQ is to send to own address.

Reviewed-by Grygorii Strashko <[email protected]>
Quoted 43 lines
Signed-off-by Alexander Sverdlin <[email protected]>

---
Changes in v2:
- rebased on 110bc76729d4 of Linus's tree;

  drivers/i2c/busses/i2c-davinci.c |    9 ++++++---
  1 files changed, 6 insertions(+), 3 deletions(-)
drivers/i2c/busses/i2c-davinci.c +6 -3
index 4788a32..df96ab6 100644
@@ -41,8 +41,8 @@
 
  #define DAVINCI_I2C_TIMEOUT	(1*HZ)
  #define DAVINCI_I2C_MAX_TRIES	2
-#define I2C_DAVINCI_INTR_ALL    (DAVINCI_I2C_IMR_AAS | \
-				 DAVINCI_I2C_IMR_SCD | \
+#define DAVINCI_I2C_OWN_ADDRESS	0x08
+#define I2C_DAVINCI_INTR_ALL    (DAVINCI_I2C_IMR_SCD | \
  				 DAVINCI_I2C_IMR_ARDY | \
  				 DAVINCI_I2C_IMR_NACK | \
  				 DAVINCI_I2C_IMR_AL)

@@ -233,7 +233,7 @@ static int i2c_davinci_init(struct davinci_i2c_dev *dev)
  	/* Respond at reserved "SMBus Host" slave address" (and zero);
  	 * we seem to have no option to not respond...
  	 */
-	davinci_i2c_write_reg(dev, DAVINCI_I2C_OAR_REG, 0x08);
+	davinci_i2c_write_reg(dev, DAVINCI_I2C_OAR_REG, DAVINCI_I2C_OWN_ADDRESS);
 
  	dev_dbg(dev->dev, "PSC  = %d\n",
  		davinci_i2c_read_reg(dev, DAVINCI_I2C_PSC_REG));

@@ -390,6 +390,9 @@ i2c_davinci_xfer_msg(struct i2c_adapter *adap, struct i2c_msg *msg, int stop)
  	u16 w;
  	unsigned long time_left;
 
+	if (msg->addr == DAVINCI_I2C_OWN_ADDRESS)
+		return -EIO;
+
  	/* Introduce a delay, required for some boards (e.g Davinci EVM) */
  	if (pdata->bus_delay)
  		udelay(pdata->bus_delay);
Signature · 2 lines
regards,
-grygorii
#3 lore raw
In reply to a message not in the archive
Quoted 2 lines · Alexander Sverdlin

Sending a message to own address locks the controller up in very bizarre state, it behaves as slave even if MDR register clearly states master. The controller

Eeks...

drivers/i2c/busses/i2c-davinci.c i2c_davinci_xfer_msg(struct i2c_adapter *adap, struct i2c_msg *msg, int stop) · line 394

Quoted 3 lines
drivers/i2c/busses/i2c-davinci.c +2 -0
+	if (msg->addr == DAVINCI_I2C_OWN_ADDRESS)
+		return -EIO;

I think we should return at least -EADDRNOTAVAIL here. Otherwise the user might wonder if the slave is broken giving always IO errors. Maybe we should also add a warning message, I leave that decision to you.

author
#4 lore raw

PATCH v3 1/1 i2c: davinci: Avoid sending to own address

In reply to a message not in the archive

Sending a message to own address locks the controller up in very bizarre state, it behaves as slave even if MDR register clearly states master. The controller remains in this state until reset. To avoid unnecessary timeouts simply avoid sending to own address. The controller cannot do this any way. Also, do not enable AAS IRQ, as the slave mode is not supported by the driver and the only possibility to trigger this IRQ is to send to own address.

Signed-off-by Alexander Sverdlin <[email protected]>

---
Changes in v3:
- return -EADDRNOTAVAIL instead of -EIO;
- emit a warning
Changes in v2:
- rebased on 110bc76729d4 of Linus's tree;

 drivers/i2c/busses/i2c-davinci.c |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)
drivers/i2c/busses/i2c-davinci.c +8 -3
index ab341aa..3d3ae52 100644
@@ -41,8 +41,8 @@
 
 #define DAVINCI_I2C_TIMEOUT	(1*HZ)
 #define DAVINCI_I2C_MAX_TRIES	2
-#define I2C_DAVINCI_INTR_ALL    (DAVINCI_I2C_IMR_AAS | \
-				 DAVINCI_I2C_IMR_SCD | \
+#define DAVINCI_I2C_OWN_ADDRESS	0x08
+#define I2C_DAVINCI_INTR_ALL    (DAVINCI_I2C_IMR_SCD | \
 				 DAVINCI_I2C_IMR_ARDY | \
 				 DAVINCI_I2C_IMR_NACK | \
 				 DAVINCI_I2C_IMR_AL)

@@ -233,7 +233,7 @@ static int i2c_davinci_init(struct davinci_i2c_dev *dev)
 	/* Respond at reserved "SMBus Host" slave address" (and zero);
 	 * we seem to have no option to not respond...
 	 */
-	davinci_i2c_write_reg(dev, DAVINCI_I2C_OAR_REG, 0x08);
+	davinci_i2c_write_reg(dev, DAVINCI_I2C_OAR_REG, DAVINCI_I2C_OWN_ADDRESS);
 
 	dev_dbg(dev->dev, "PSC  = %d\n",
 		davinci_i2c_read_reg(dev, DAVINCI_I2C_PSC_REG));

@@ -386,6 +386,11 @@ i2c_davinci_xfer_msg(struct i2c_adapter *adap, struct i2c_msg *msg, int stop)
 	u16 w;
 	unsigned long time_left;
 
+	if (msg->addr == DAVINCI_I2C_OWN_ADDRESS) {
+		dev_warn(dev->dev, "transfer to own address aborted\n");
+		return -EADDRNOTAVAIL;
+	}
+
 	/* Introduce a delay, required for some boards (e.g Davinci EVM) */
 	if (pdata->bus_delay)
 		udelay(pdata->bus_delay);

A message that is not in the archive (sent off-list or not yet received): [email protected]

Applied
#6 lore raw
Quoted 8 lines · Alexander Sverdlin

Sending a message to own address locks the controller up in very bizarre state, it behaves as slave even if MDR register clearly states master. The controller remains in this state until reset. To avoid unnecessary timeouts simply avoid sending to own address. The controller cannot do this any way. Also, do not enable AAS IRQ, as the slave mode is not supported by the driver and the only possibility to trigger this IRQ is to send to own address.

Signed-off-by Alexander Sverdlin <[email protected]>

Applied to for-next, thanks!

Please don't send new patch versions as replies to old versions!