Thread (16 messages) 16 messages, 3 authors, 2020-02-22

Re: [PATCH v3 3/6] i2c: at91: Send bus clear command if SDA is down

From: Ludovic Desroches <ludovic.desroches@microchip.com>
Date: 2020-01-27 08:55:06
Also in: linux-devicetree, linux-i2c, lkml

On Wed, Jan 15, 2020 at 01:54:19PM +0200, Codrin Ciubotariu wrote:
EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

After a transfer timeout, some faulty I2C slave devices might hold down
the SDA pin. We can generate a bus clear command, hoping that the slave
might release the pins.
If the CLEAR command is not supported, we will use gpio recovery, if
available, to reset the bus.

Signed-off-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
I still have issue to apply it even if the context seems correct.
Sometimes git am is picky... so

Acked-by: Ludovic Desroches <ludovic.desroches@microchip.com>
quoted hunk ↗ jump to hunk
---

Changes in v3:
 - rebased on top of i2c/for-next;
  - remove unnecessary initializations with false;
 - replaced SCL with SDA in title and commit message;
 - updated commit message;

Changes in v2:
 - use CLEAR command only if SDA is down; update patch subject to
   reflect this;
 - CLEAR command is no longer used for sama5d2, only sam9x60;

 drivers/i2c/busses/i2c-at91-core.c   |  2 ++
 drivers/i2c/busses/i2c-at91-master.c | 32 +++++++++++++++++++++++-----
 drivers/i2c/busses/i2c-at91.h        |  7 +++++-
 3 files changed, 35 insertions(+), 6 deletions(-)
diff --git a/drivers/i2c/busses/i2c-at91-core.c b/drivers/i2c/busses/i2c-at91-core.c
index 3da1a8acecb5..e14edd236108 100644
--- a/drivers/i2c/busses/i2c-at91-core.c
+++ b/drivers/i2c/busses/i2c-at91-core.c
@@ -131,6 +131,7 @@ static struct at91_twi_pdata sama5d2_config = {
        .has_dig_filtr = true,
        .has_adv_dig_filtr = true,
        .has_ana_filtr = true,
+       .has_clear_cmd = false, /* due to errata, CLEAR cmd is not working */
 };

 static struct at91_twi_pdata sam9x60_config = {
@@ -142,6 +143,7 @@ static struct at91_twi_pdata sam9x60_config = {
        .has_dig_filtr = true,
        .has_adv_dig_filtr = true,
        .has_ana_filtr = true,
+       .has_clear_cmd = true,
 };

 static const struct of_device_id atmel_twi_dt_ids[] = {
diff --git a/drivers/i2c/busses/i2c-at91-master.c b/drivers/i2c/busses/i2c-at91-master.c
index 0aba51a7df32..bcc05a8fe826 100644
--- a/drivers/i2c/busses/i2c-at91-master.c
+++ b/drivers/i2c/busses/i2c-at91-master.c
@@ -480,7 +480,7 @@ static int at91_do_twi_transfer(struct at91_twi_dev *dev)
        unsigned long time_left;
        bool has_unre_flag = dev->pdata->has_unre_flag;
        bool has_alt_cmd = dev->pdata->has_alt_cmd;
-       struct i2c_bus_recovery_info *rinfo = &dev->rinfo;
+       bool has_clear_cmd = dev->pdata->has_clear_cmd;

        /*
         * WARNING: the TXCOMP bit in the Status Register is NOT a clear on
@@ -641,10 +641,32 @@ static int at91_do_twi_transfer(struct at91_twi_dev *dev)
                               AT91_TWI_THRCLR | AT91_TWI_LOCKCLR);
        }

-       if (rinfo->get_sda && !(rinfo->get_sda(&dev->adapter))) {
-               dev_dbg(dev->dev,
-                       "SDA is down; clear bus using gpio\n");
-               i2c_recover_bus(&dev->adapter);
+       /*
+        * some faulty I2C slave devices might hold SDA down;
+        * we can send a bus clear command, hoping that the pins will be
+        * released
+        */
+       if (has_clear_cmd) {
+               if (!(dev->transfer_status & AT91_TWI_SDA)) {
+                       dev_dbg(dev->dev,
+                               "SDA is down; sending bus clear command\n");
+                       if (dev->use_alt_cmd) {
+                               unsigned int acr;
+
+                               acr = at91_twi_read(dev, AT91_TWI_ACR);
+                               acr &= ~AT91_TWI_ACR_DATAL_MASK;
+                               at91_twi_write(dev, AT91_TWI_ACR, acr);
+                       }
+                       at91_twi_write(dev, AT91_TWI_CR, AT91_TWI_CLEAR);
+               }
+       } else {
+               struct i2c_bus_recovery_info *rinfo = &dev->rinfo;
+
+               if (rinfo->get_sda && !(rinfo->get_sda(&dev->adapter))) {
+                       dev_dbg(dev->dev,
+                               "SDA is down; clear bus using gpio\n");
+                       i2c_recover_bus(&dev->adapter);
+               }
        }

        return ret;
diff --git a/drivers/i2c/busses/i2c-at91.h b/drivers/i2c/busses/i2c-at91.h
index f57a6cab96b4..7e7b4955ca7f 100644
--- a/drivers/i2c/busses/i2c-at91.h
+++ b/drivers/i2c/busses/i2c-at91.h
@@ -36,6 +36,7 @@
 #define        AT91_TWI_SVDIS          BIT(5)  /* Slave Transfer Disable */
 #define        AT91_TWI_QUICK          BIT(6)  /* SMBus quick command */
 #define        AT91_TWI_SWRST          BIT(7)  /* Software Reset */
+#define        AT91_TWI_CLEAR          BIT(15) /* Bus clear command */
 #define        AT91_TWI_ACMEN          BIT(16) /* Alternative Command Mode Enable */
 #define        AT91_TWI_ACMDIS         BIT(17) /* Alternative Command Mode Disable */
 #define        AT91_TWI_THRCLR         BIT(24) /* Transmit Holding Register Clear */
@@ -69,6 +70,8 @@
 #define        AT91_TWI_NACK           BIT(8)  /* Not Acknowledged */
 #define        AT91_TWI_EOSACC         BIT(11) /* End Of Slave Access */
 #define        AT91_TWI_LOCK           BIT(23) /* TWI Lock due to Frame Errors */
+#define        AT91_TWI_SCL            BIT(24) /* TWI SCL status */
+#define        AT91_TWI_SDA            BIT(25) /* TWI SDA status */

 #define        AT91_TWI_INT_MASK \
        (AT91_TWI_TXCOMP | AT91_TWI_RXRDY | AT91_TWI_TXRDY | AT91_TWI_NACK \
@@ -81,7 +84,8 @@
 #define        AT91_TWI_THR            0x0034  /* Transmit Holding Register */

 #define        AT91_TWI_ACR            0x0040  /* Alternative Command Register */
-#define        AT91_TWI_ACR_DATAL(len) ((len) & 0xff)
+#define        AT91_TWI_ACR_DATAL_MASK GENMASK(15, 0)
+#define        AT91_TWI_ACR_DATAL(len) ((len) & AT91_TWI_ACR_DATAL_MASK)
 #define        AT91_TWI_ACR_DIR        BIT(8)

 #define AT91_TWI_FILTR         0x0044
@@ -118,6 +122,7 @@ struct at91_twi_pdata {
        bool has_dig_filtr;
        bool has_adv_dig_filtr;
        bool has_ana_filtr;
+       bool has_clear_cmd;
        struct at_dma_slave dma_slave;
 };

--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help