linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 1/4] i2c: at91: Send bus clear command if SDA is down
@ 2020-02-25 15:50 Codrin Ciubotariu
  2020-02-25 15:50 ` [PATCH v4 2/4] ARM: at91/dt: sama5d3: add i2c gpio pinctrl Codrin Ciubotariu
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Codrin Ciubotariu @ 2020-02-25 15:50 UTC (permalink / raw)
  To: linux-i2c, linux-arm-kernel, linux-kernel, devicetree
  Cc: ludovic.desroches, nicolas.ferre, alexandre.belloni, robh+dt,
	peda, linux, wsa, Codrin Ciubotariu

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>
---

Changes in v4:
 - moved the HW bus clear command in a recover_bus() function, to be able
   to just call i2c_recover_bus() if a transfer fails;
 - recovery gpios are no longer taken if HW bus clear command is
   supported;  

Changes in v3:
 - removed unnecessary condition from info print;
 - removed unneded declarations;

Changes in v2:
 - called i2c_recover_bus() after an error occurs, if SDA is down;
 - release gpios if recovery information is incomplete;

 drivers/i2c/busses/i2c-at91-core.c   |  2 ++
 drivers/i2c/busses/i2c-at91-master.c | 49 ++++++++++++++++++++++++----
 drivers/i2c/busses/i2c-at91.h        |  7 +++-
 3 files changed, 50 insertions(+), 8 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..776e95962ab6 100644
--- a/drivers/i2c/busses/i2c-at91-master.c
+++ b/drivers/i2c/busses/i2c-at91-master.c
@@ -480,7 +480,6 @@ 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;
 
 	/*
 	 * WARNING: the TXCOMP bit in the Status Register is NOT a clear on
@@ -641,11 +640,12 @@ 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
+	 */
+	i2c_recover_bus(&dev->adapter);
 
 	return ret;
 }
@@ -830,7 +830,7 @@ static void at91_unprepare_twi_recovery(struct i2c_adapter *adap)
 	pinctrl_select_state(dev->pinctrl, dev->pinctrl_pins_default);
 }
 
-static int at91_init_twi_recovery_info(struct platform_device *pdev,
+static int at91_init_twi_recovery_gpio(struct platform_device *pdev,
 				       struct at91_twi_dev *dev)
 {
 	struct i2c_bus_recovery_info *rinfo = &dev->rinfo;
@@ -880,6 +880,41 @@ static int at91_init_twi_recovery_info(struct platform_device *pdev,
 	return 0;
 }
 
+static int at91_twi_recover_bus_cmd(struct i2c_adapter *adap)
+{
+	struct at91_twi_dev *dev = i2c_get_adapdata(adap);
+
+	dev->transfer_status |= at91_twi_read(dev, AT91_TWI_SR);
+	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);
+	}
+
+	return 0;
+}
+
+static int at91_init_twi_recovery_info(struct platform_device *pdev,
+				       struct at91_twi_dev *dev)
+{
+	struct i2c_bus_recovery_info *rinfo = &dev->rinfo;
+	bool has_clear_cmd = dev->pdata->has_clear_cmd;
+
+	if (!has_clear_cmd)
+		return at91_init_twi_recovery_gpio(pdev, dev);
+
+	rinfo->recover_bus = at91_twi_recover_bus_cmd;
+	dev->adapter.bus_recovery_info = rinfo;
+
+	return 0;
+}
+
 int at91_twi_probe_master(struct platform_device *pdev,
 			  u32 phy_addr, struct at91_twi_dev *dev)
 {
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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v4 2/4] ARM: at91/dt: sama5d3: add i2c gpio pinctrl
  2020-02-25 15:50 [PATCH v4 1/4] i2c: at91: Send bus clear command if SDA is down Codrin Ciubotariu
@ 2020-02-25 15:50 ` Codrin Ciubotariu
  2020-03-09 21:20   ` Alexandre Belloni
  2020-02-25 15:50 ` [PATCH v4 3/4] ARM: at91/dt: sama5d4: " Codrin Ciubotariu
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Codrin Ciubotariu @ 2020-02-25 15:50 UTC (permalink / raw)
  To: linux-i2c, linux-arm-kernel, linux-kernel, devicetree
  Cc: ludovic.desroches, nicolas.ferre, alexandre.belloni, robh+dt,
	peda, linux, wsa, Kamel Bouhara, Codrin Ciubotariu

From: Kamel Bouhara <kamel.bouhara@bootlin.com>

Add the i2c gpio pinctrls to support the i2c bus recovery

Signed-off-by: Kamel Bouhara <kamel.bouhara@bootlin.com>
[codrin.ciubotariu@microchip.com: removed gpio pull-ups]
Signed-off-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
Acked-by: Ludovic Desroches <ludovic.desroches@microchip.com>
---

Changes in v4:
 - added Acked-by tag from Ludovic;

Changes in v3:
 - removed gpio pull-ups;

Changes in v2:
  - none;

 arch/arm/boot/dts/sama5d3.dtsi | 33 ++++++++++++++++++++++++++++++---
 1 file changed, 30 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/sama5d3.dtsi b/arch/arm/boot/dts/sama5d3.dtsi
index 203d40be70a5..f11b018e9173 100644
--- a/arch/arm/boot/dts/sama5d3.dtsi
+++ b/arch/arm/boot/dts/sama5d3.dtsi
@@ -159,8 +159,11 @@
 				dmas = <&dma0 2 AT91_DMA_CFG_PER_ID(7)>,
 				       <&dma0 2 AT91_DMA_CFG_PER_ID(8)>;
 				dma-names = "tx", "rx";
-				pinctrl-names = "default";
+				pinctrl-names = "default", "gpio";
 				pinctrl-0 = <&pinctrl_i2c0>;
+				pinctrl-1 = <&pinctrl_i2c0_gpio>;
+				sda-gpios = <&pioA 30 GPIO_ACTIVE_HIGH>;
+				scl-gpios = <&pioA 31 GPIO_ACTIVE_HIGH>;
 				#address-cells = <1>;
 				#size-cells = <0>;
 				clocks = <&twi0_clk>;
@@ -174,8 +177,11 @@
 				dmas = <&dma0 2 AT91_DMA_CFG_PER_ID(9)>,
 				       <&dma0 2 AT91_DMA_CFG_PER_ID(10)>;
 				dma-names = "tx", "rx";
-				pinctrl-names = "default";
+				pinctrl-names = "default", "gpio";
 				pinctrl-0 = <&pinctrl_i2c1>;
+				pinctrl-1 = <&pinctrl_i2c1_gpio>;
+				sda-gpios = <&pioC 26 GPIO_ACTIVE_HIGH>;
+				scl-gpios = <&pioC 27 GPIO_ACTIVE_HIGH>;
 				#address-cells = <1>;
 				#size-cells = <0>;
 				clocks = <&twi1_clk>;
@@ -357,8 +363,11 @@
 				dmas = <&dma1 2 AT91_DMA_CFG_PER_ID(11)>,
 				       <&dma1 2 AT91_DMA_CFG_PER_ID(12)>;
 				dma-names = "tx", "rx";
-				pinctrl-names = "default";
+				pinctrl-names = "default", "gpio";
 				pinctrl-0 = <&pinctrl_i2c2>;
+				pinctrl-1 = <&pinctrl_i2c2_gpio>;
+				sda-gpios = <&pioA 18 GPIO_ACTIVE_HIGH>;
+				scl-gpios = <&pioA 19 GPIO_ACTIVE_HIGH>;
 				#address-cells = <1>;
 				#size-cells = <0>;
 				clocks = <&twi2_clk>;
@@ -639,6 +648,12 @@
 							<AT91_PIOA 30 AT91_PERIPH_A AT91_PINCTRL_NONE	/* PA30 periph A TWD0 pin, conflicts with URXD1, ISI_VSYNC */
 							 AT91_PIOA 31 AT91_PERIPH_A AT91_PINCTRL_NONE>;	/* PA31 periph A TWCK0 pin, conflicts with UTXD1, ISI_HSYNC */
 					};
+
+					pinctrl_i2c0_gpio: i2c0-gpio {
+						atmel,pins =
+							<AT91_PIOA 30 AT91_PERIPH_GPIO AT91_PINCTRL_NONE
+							 AT91_PIOA 31 AT91_PERIPH_GPIO AT91_PINCTRL_NONE>;
+					};
 				};
 
 				i2c1 {
@@ -647,6 +662,12 @@
 							<AT91_PIOC 26 AT91_PERIPH_B AT91_PINCTRL_NONE	/* PC26 periph B TWD1 pin, conflicts with SPI1_NPCS1, ISI_D11 */
 							 AT91_PIOC 27 AT91_PERIPH_B AT91_PINCTRL_NONE>;	/* PC27 periph B TWCK1 pin, conflicts with SPI1_NPCS2, ISI_D10 */
 					};
+
+					pinctrl_i2c1_gpio: i2c1-gpio {
+						atmel,pins =
+							<AT91_PIOC 26 AT91_PERIPH_GPIO AT91_PINCTRL_NONE
+							 AT91_PIOC 27 AT91_PERIPH_GPIO AT91_PINCTRL_NONE>;
+					};
 				};
 
 				i2c2 {
@@ -655,6 +676,12 @@
 							<AT91_PIOA 18 AT91_PERIPH_B AT91_PINCTRL_NONE	/* TWD2 pin, conflicts with LCDDAT18, ISI_D2 */
 							 AT91_PIOA 19 AT91_PERIPH_B AT91_PINCTRL_NONE>; /* TWCK2 pin, conflicts with LCDDAT19, ISI_D3 */
 					};
+
+					pinctrl_i2c2_gpio: i2c2-gpio {
+						atmel,pins =
+							<AT91_PIOA 18 AT91_PERIPH_GPIO AT91_PINCTRL_NONE
+							 AT91_PIOA 19 AT91_PERIPH_GPIO AT91_PINCTRL_NONE>;
+					};
 				};
 
 				isi {
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v4 3/4] ARM: at91/dt: sama5d4: add i2c gpio pinctrl
  2020-02-25 15:50 [PATCH v4 1/4] i2c: at91: Send bus clear command if SDA is down Codrin Ciubotariu
  2020-02-25 15:50 ` [PATCH v4 2/4] ARM: at91/dt: sama5d3: add i2c gpio pinctrl Codrin Ciubotariu
@ 2020-02-25 15:50 ` Codrin Ciubotariu
  2020-03-09 21:20   ` Alexandre Belloni
  2020-02-25 15:50 ` [PATCH v4 4/4] ARM: at91/dt: sama5d2: " Codrin Ciubotariu
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Codrin Ciubotariu @ 2020-02-25 15:50 UTC (permalink / raw)
  To: linux-i2c, linux-arm-kernel, linux-kernel, devicetree
  Cc: ludovic.desroches, nicolas.ferre, alexandre.belloni, robh+dt,
	peda, linux, wsa, Kamel Bouhara, Codrin Ciubotariu

From: Kamel Bouhara <kamel.bouhara@bootlin.com>

Add the i2c gpio pinctrls so the i2c bus recovery option can be enabled

Signed-off-by: Kamel Bouhara <kamel.bouhara@bootlin.com>
[codrin.ciubotariu@microchip.com: removed gpio pull-ups]
Signed-off-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
Acked-by: Ludovic Desroches <ludovic.desroches@microchip.com>
---

Changes in v4:
 - added Acked-by tag from Ludovic;

Changes in v3:
 - removed gpio pull-ups;

Changes in v2:
  - none;

 arch/arm/boot/dts/sama5d4.dtsi | 33 ++++++++++++++++++++++++++++++---
 1 file changed, 30 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/sama5d4.dtsi b/arch/arm/boot/dts/sama5d4.dtsi
index 6ab27a7b388d..26ce868096c2 100644
--- a/arch/arm/boot/dts/sama5d4.dtsi
+++ b/arch/arm/boot/dts/sama5d4.dtsi
@@ -458,8 +458,11 @@
 					(AT91_XDMAC_DT_MEM_IF(0) | AT91_XDMAC_DT_PER_IF(1)
 					| AT91_XDMAC_DT_PERID(3))>;
 				dma-names = "tx", "rx";
-				pinctrl-names = "default";
+				pinctrl-names = "default", "gpio";
 				pinctrl-0 = <&pinctrl_i2c0>;
+				pinctrl-1 = <&pinctrl_i2c0_gpio>;
+				sda-gpios = <&pioA 30 GPIO_ACTIVE_HIGH>;
+				scl-gpios = <&pioA 31 GPIO_ACTIVE_HIGH>;
 				#address-cells = <1>;
 				#size-cells = <0>;
 				clocks = <&pmc PMC_TYPE_PERIPHERAL 32>;
@@ -477,8 +480,11 @@
 					(AT91_XDMAC_DT_MEM_IF(0) | AT91_XDMAC_DT_PER_IF(1)
 					| AT91_XDMAC_DT_PERID(5))>;
 				dma-names = "tx", "rx";
-				pinctrl-names = "default";
+				pinctrl-names = "default", "gpio";
 				pinctrl-0 = <&pinctrl_i2c1>;
+				pinctrl-1 = <&pinctrl_i2c1_gpio>;
+				sda-gpios = <&pioE 29 GPIO_ACTIVE_HIGH>;
+				scl-gpios = <&pioE 30 GPIO_ACTIVE_HIGH>;
 				#address-cells = <1>;
 				#size-cells = <0>;
 				clocks = <&pmc PMC_TYPE_PERIPHERAL 33>;
@@ -519,8 +525,11 @@
 					(AT91_XDMAC_DT_MEM_IF(0) | AT91_XDMAC_DT_PER_IF(1)
 					| AT91_XDMAC_DT_PERID(7))>;
 				dma-names = "tx", "rx";
-				pinctrl-names = "default";
+				pinctrl-names = "default", "gpio";
 				pinctrl-0 = <&pinctrl_i2c2>;
+				pinctrl-1 = <&pinctrl_i2c2_gpio>;
+				sda-gpios = <&pioB 29 GPIO_ACTIVE_HIGH>;
+				scl-gpios = <&pioB 30 GPIO_ACTIVE_HIGH>;
 				#address-cells = <1>;
 				#size-cells = <0>;
 				clocks = <&pmc PMC_TYPE_PERIPHERAL 34>;
@@ -1122,6 +1131,12 @@
 							<AT91_PIOA 30 AT91_PERIPH_A AT91_PINCTRL_NONE
 							 AT91_PIOA 31 AT91_PERIPH_A AT91_PINCTRL_NONE>;
 					};
+
+					pinctrl_i2c0_gpio: i2c0-gpio {
+						atmel,pins =
+							<AT91_PIOA 30 AT91_PERIPH_GPIO AT91_PINCTRL_NONE
+							 AT91_PIOA 31 AT91_PERIPH_GPIO AT91_PINCTRL_NONE>;
+					};
 				};
 
 				i2c1 {
@@ -1130,6 +1145,12 @@
 							<AT91_PIOE 29 AT91_PERIPH_C AT91_PINCTRL_NONE	/* TWD1, conflicts with UART0 RX and DIBP */
 							 AT91_PIOE 30 AT91_PERIPH_C AT91_PINCTRL_NONE>;	/* TWCK1, conflicts with UART0 TX and DIBN */
 					};
+
+					pinctrl_i2c1_gpio: i2c1-gpio {
+						atmel,pins =
+							<AT91_PIOE 29 AT91_PERIPH_GPIO AT91_PINCTRL_NONE
+							 AT91_PIOE 30 AT91_PERIPH_GPIO AT91_PINCTRL_NONE>;
+					};
 				};
 
 				i2c2 {
@@ -1138,6 +1159,12 @@
 							<AT91_PIOB 29 AT91_PERIPH_A AT91_PINCTRL_NONE	/* TWD2, conflicts with RD0 and PWML1 */
 							 AT91_PIOB 30 AT91_PERIPH_A AT91_PINCTRL_NONE>; /* TWCK2, conflicts with RF0 */
 					};
+
+					pinctrl_i2c2_gpio: i2c2-gpio {
+						atmel,pins =
+							<AT91_PIOB 29 AT91_PERIPH_GPIO AT91_PINCTRL_NONE
+							 AT91_PIOB 30 AT91_PERIPH_GPIO AT91_PINCTRL_NONE>;
+					};
 				};
 
 				isi {
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v4 4/4] ARM: at91/dt: sama5d2: add i2c gpio pinctrl
  2020-02-25 15:50 [PATCH v4 1/4] i2c: at91: Send bus clear command if SDA is down Codrin Ciubotariu
  2020-02-25 15:50 ` [PATCH v4 2/4] ARM: at91/dt: sama5d3: add i2c gpio pinctrl Codrin Ciubotariu
  2020-02-25 15:50 ` [PATCH v4 3/4] ARM: at91/dt: sama5d4: " Codrin Ciubotariu
@ 2020-02-25 15:50 ` Codrin Ciubotariu
  2020-03-09 21:20   ` Alexandre Belloni
  2020-03-29 21:13 ` [PATCH v4 1/4] i2c: at91: Send bus clear command if SDA is down Ludovic.Desroches
  2020-05-05 14:37 ` Wolfram Sang
  4 siblings, 1 reply; 10+ messages in thread
From: Codrin Ciubotariu @ 2020-02-25 15:50 UTC (permalink / raw)
  To: linux-i2c, linux-arm-kernel, linux-kernel, devicetree
  Cc: ludovic.desroches, nicolas.ferre, alexandre.belloni, robh+dt,
	peda, linux, wsa, Kamel Bouhara, Codrin Ciubotariu

From: Kamel Bouhara <kamel.bouhara@bootlin.com>

Add the i2c gpio pinctrls to support the i2c bus recovery

Signed-off-by: Kamel Bouhara <kamel.bouhara@bootlin.com>
[codrin.ciubotariu@microchip.com: removed gpio pull-ups]
Signed-off-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
Acked-by: Ludovic Desroches <ludovic.desroches@microchip.com>
---

Changes in v4:
 - added Acked-by tag from Ludovic;

Changes in v3:
 - removed gpio pull-ups;

Changes in v2:
 - new patch;

 arch/arm/boot/dts/at91-sama5d2_ptc_ek.dts   | 33 +++++++++++++++++++--
 arch/arm/boot/dts/at91-sama5d2_xplained.dts | 33 +++++++++++++++++++--
 2 files changed, 60 insertions(+), 6 deletions(-)

diff --git a/arch/arm/boot/dts/at91-sama5d2_ptc_ek.dts b/arch/arm/boot/dts/at91-sama5d2_ptc_ek.dts
index ba7f3e646c26..1c24ac8019ba 100644
--- a/arch/arm/boot/dts/at91-sama5d2_ptc_ek.dts
+++ b/arch/arm/boot/dts/at91-sama5d2_ptc_ek.dts
@@ -180,8 +180,11 @@
 
 			i2c0: i2c@f8028000 {
 				dmas = <0>, <0>;
-				pinctrl-names = "default";
+				pinctrl-names = "default", "gpio";
 				pinctrl-0 = <&pinctrl_i2c0_default>;
+				pinctrl-1 = <&pinctrl_i2c0_gpio>;
+				sda-gpios = <&pioA PIN_PD21 GPIO_ACTIVE_HIGH>;
+				scl-gpios = <&pioA PIN_PD22 GPIO_ACTIVE_HIGH>;
 				status = "okay";
 			};
 
@@ -198,8 +201,11 @@
 					#address-cells = <1>;
 					#size-cells = <0>;
 					clocks = <&pmc PMC_TYPE_PERIPHERAL 19>;
-					pinctrl-names = "default";
+					pinctrl-names = "default", "gpio";
 					pinctrl-0 = <&pinctrl_flx0_default>;
+					pinctrl-1 = <&pinctrl_flx0_gpio>;
+					sda-gpios = <&pioA PIN_PB28 GPIO_ACTIVE_HIGH>;
+					scl-gpios = <&pioA PIN_PB29 GPIO_ACTIVE_HIGH>;
 					atmel,fifo-size = <16>;
 					status = "okay";
 				};
@@ -226,8 +232,11 @@
 
 			i2c1: i2c@fc028000 {
 				dmas = <0>, <0>;
-				pinctrl-names = "default";
+				pinctrl-names = "default", "gpio";
 				pinctrl-0 = <&pinctrl_i2c1_default>;
+				pinctrl-1 = <&pinctrl_i2c1_gpio>;
+				sda-gpios = <&pioA PIN_PC6 GPIO_ACTIVE_HIGH>;
+				scl-gpios = <&pioA PIN_PC7 GPIO_ACTIVE_HIGH>;
 				status = "okay";
 
 				at24@50 {
@@ -244,18 +253,36 @@
 					bias-disable;
 				};
 
+				pinctrl_flx0_gpio: flx0_gpio {
+					pinmux = <PIN_PB28__GPIO>,
+						 <PIN_PB29__GPIO>;
+					bias-disable;
+				};
+
 				pinctrl_i2c0_default: i2c0_default {
 					pinmux = <PIN_PD21__TWD0>,
 						 <PIN_PD22__TWCK0>;
 					bias-disable;
 				};
 
+				pinctrl_i2c0_gpio: i2c0_gpio {
+					pinmux = <PIN_PD21__GPIO>,
+						 <PIN_PD22__GPIO>;
+					bias-disable;
+				};
+
 				pinctrl_i2c1_default: i2c1_default {
 					pinmux = <PIN_PC6__TWD1>,
 						 <PIN_PC7__TWCK1>;
 					bias-disable;
 				};
 
+				pinctrl_i2c1_gpio: i2c1_gpio {
+					pinmux = <PIN_PC6__GPIO>,
+						 <PIN_PC7__GPIO>;
+					bias-disable;
+				};
+
 				pinctrl_key_gpio_default: key_gpio_default {
 					pinmux = <PIN_PA10__GPIO>;
 					bias-pull-up;
diff --git a/arch/arm/boot/dts/at91-sama5d2_xplained.dts b/arch/arm/boot/dts/at91-sama5d2_xplained.dts
index 9d0a7fbea725..055ee53e4773 100644
--- a/arch/arm/boot/dts/at91-sama5d2_xplained.dts
+++ b/arch/arm/boot/dts/at91-sama5d2_xplained.dts
@@ -129,8 +129,11 @@
 
 			i2c0: i2c@f8028000 {
 				dmas = <0>, <0>;
-				pinctrl-names = "default";
+				pinctrl-names = "default", "gpio";
 				pinctrl-0 = <&pinctrl_i2c0_default>;
+				pinctrl-1 = <&pinctrl_i2c0_gpio>;
+				sda-gpios = <&pioA PIN_PD21 GPIO_ACTIVE_HIGH>;
+				scl-gpios = <&pioA PIN_PD22 GPIO_ACTIVE_HIGH>;
 				i2c-sda-hold-time-ns = <350>;
 				status = "okay";
 
@@ -331,8 +334,11 @@
 					#address-cells = <1>;
 					#size-cells = <0>;
 					clocks = <&pmc PMC_TYPE_PERIPHERAL 23>;
-					pinctrl-names = "default";
+					pinctrl-names = "default", "gpio";
 					pinctrl-0 = <&pinctrl_flx4_default>;
+					pinctrl-1 = <&pinctrl_flx4_gpio>;
+					sda-gpios = <&pioA PIN_PD12 GPIO_ACTIVE_HIGH>;
+					scl-gpios = <&pioA PIN_PD13 GPIO_ACTIVE_HIGH>;
 					atmel,fifo-size = <16>;
 					i2c-analog-filter;
 					i2c-digital-filter;
@@ -343,11 +349,14 @@
 
 			i2c1: i2c@fc028000 {
 				dmas = <0>, <0>;
-				pinctrl-names = "default";
+				pinctrl-names = "default", "gpio";
 				pinctrl-0 = <&pinctrl_i2c1_default>;
 				i2c-analog-filter;
 				i2c-digital-filter;
 				i2c-digital-filter-width-ns = <35>;
+				pinctrl-1 = <&pinctrl_i2c1_gpio>;
+				sda-gpios = <&pioA PIN_PD4 GPIO_ACTIVE_HIGH>;
+				scl-gpios = <&pioA PIN_PD5 GPIO_ACTIVE_HIGH>;
 				status = "okay";
 
 				at24@54 {
@@ -441,18 +450,36 @@
 					bias-disable;
 				};
 
+				pinctrl_flx4_gpio: flx4_gpio {
+					pinmux = <PIN_PD12__GPIO>,
+						 <PIN_PD13__GPIO>;
+					bias-disable;
+				};
+
 				pinctrl_i2c0_default: i2c0_default {
 					pinmux = <PIN_PD21__TWD0>,
 						 <PIN_PD22__TWCK0>;
 					bias-disable;
 				};
 
+				pinctrl_i2c0_gpio: i2c0_gpio {
+					pinmux = <PIN_PD21__GPIO>,
+						 <PIN_PD22__GPIO>;
+					bias-disable;
+				};
+
 				pinctrl_i2c1_default: i2c1_default {
 					pinmux = <PIN_PD4__TWD1>,
 						 <PIN_PD5__TWCK1>;
 					bias-disable;
 				};
 
+				pinctrl_i2c1_gpio: i2c1_gpio {
+					pinmux = <PIN_PD4__GPIO>,
+						 <PIN_PD5__GPIO>;
+					bias-disable;
+				};
+
 				pinctrl_i2s0_default: i2s0_default {
 					pinmux = <PIN_PC1__I2SC0_CK>,
 						 <PIN_PC2__I2SC0_MCK>,
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 2/4] ARM: at91/dt: sama5d3: add i2c gpio pinctrl
  2020-02-25 15:50 ` [PATCH v4 2/4] ARM: at91/dt: sama5d3: add i2c gpio pinctrl Codrin Ciubotariu
@ 2020-03-09 21:20   ` Alexandre Belloni
  0 siblings, 0 replies; 10+ messages in thread
From: Alexandre Belloni @ 2020-03-09 21:20 UTC (permalink / raw)
  To: Codrin Ciubotariu
  Cc: linux-i2c, linux-arm-kernel, linux-kernel, devicetree,
	ludovic.desroches, nicolas.ferre, robh+dt, peda, linux, wsa,
	Kamel Bouhara

On 25/02/2020 17:50:10+0200, Codrin Ciubotariu wrote:
> From: Kamel Bouhara <kamel.bouhara@bootlin.com>
> 
> Add the i2c gpio pinctrls to support the i2c bus recovery
> 
> Signed-off-by: Kamel Bouhara <kamel.bouhara@bootlin.com>
> [codrin.ciubotariu@microchip.com: removed gpio pull-ups]
> Signed-off-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
> Acked-by: Ludovic Desroches <ludovic.desroches@microchip.com>
> ---
> 
> Changes in v4:
>  - added Acked-by tag from Ludovic;
> 
> Changes in v3:
>  - removed gpio pull-ups;
> 
> Changes in v2:
>   - none;
> 
>  arch/arm/boot/dts/sama5d3.dtsi | 33 ++++++++++++++++++++++++++++++---
>  1 file changed, 30 insertions(+), 3 deletions(-)
> 
Applied after modifying the subject, thanks.

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 3/4] ARM: at91/dt: sama5d4: add i2c gpio pinctrl
  2020-02-25 15:50 ` [PATCH v4 3/4] ARM: at91/dt: sama5d4: " Codrin Ciubotariu
@ 2020-03-09 21:20   ` Alexandre Belloni
  0 siblings, 0 replies; 10+ messages in thread
From: Alexandre Belloni @ 2020-03-09 21:20 UTC (permalink / raw)
  To: Codrin Ciubotariu
  Cc: linux-i2c, linux-arm-kernel, linux-kernel, devicetree,
	ludovic.desroches, nicolas.ferre, robh+dt, peda, linux, wsa,
	Kamel Bouhara

On 25/02/2020 17:50:11+0200, Codrin Ciubotariu wrote:
> From: Kamel Bouhara <kamel.bouhara@bootlin.com>
> 
> Add the i2c gpio pinctrls so the i2c bus recovery option can be enabled
> 
> Signed-off-by: Kamel Bouhara <kamel.bouhara@bootlin.com>
> [codrin.ciubotariu@microchip.com: removed gpio pull-ups]
> Signed-off-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
> Acked-by: Ludovic Desroches <ludovic.desroches@microchip.com>
> ---
> 
> Changes in v4:
>  - added Acked-by tag from Ludovic;
> 
> Changes in v3:
>  - removed gpio pull-ups;
> 
> Changes in v2:
>   - none;
> 
>  arch/arm/boot/dts/sama5d4.dtsi | 33 ++++++++++++++++++++++++++++++---
>  1 file changed, 30 insertions(+), 3 deletions(-)
> 
Applied, thanks.

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 4/4] ARM: at91/dt: sama5d2: add i2c gpio pinctrl
  2020-02-25 15:50 ` [PATCH v4 4/4] ARM: at91/dt: sama5d2: " Codrin Ciubotariu
@ 2020-03-09 21:20   ` Alexandre Belloni
  0 siblings, 0 replies; 10+ messages in thread
From: Alexandre Belloni @ 2020-03-09 21:20 UTC (permalink / raw)
  To: Codrin Ciubotariu
  Cc: linux-i2c, linux-arm-kernel, linux-kernel, devicetree,
	ludovic.desroches, nicolas.ferre, robh+dt, peda, linux, wsa,
	Kamel Bouhara

On 25/02/2020 17:50:12+0200, Codrin Ciubotariu wrote:
> From: Kamel Bouhara <kamel.bouhara@bootlin.com>
> 
> Add the i2c gpio pinctrls to support the i2c bus recovery
> 
> Signed-off-by: Kamel Bouhara <kamel.bouhara@bootlin.com>
> [codrin.ciubotariu@microchip.com: removed gpio pull-ups]
> Signed-off-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
> Acked-by: Ludovic Desroches <ludovic.desroches@microchip.com>
> ---
> 
> Changes in v4:
>  - added Acked-by tag from Ludovic;
> 
> Changes in v3:
>  - removed gpio pull-ups;
> 
> Changes in v2:
>  - new patch;
> 
>  arch/arm/boot/dts/at91-sama5d2_ptc_ek.dts   | 33 +++++++++++++++++++--
>  arch/arm/boot/dts/at91-sama5d2_xplained.dts | 33 +++++++++++++++++++--
>  2 files changed, 60 insertions(+), 6 deletions(-)
> 
Applied, thanks.

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 1/4] i2c: at91: Send bus clear command if SDA is down
  2020-02-25 15:50 [PATCH v4 1/4] i2c: at91: Send bus clear command if SDA is down Codrin Ciubotariu
                   ` (2 preceding siblings ...)
  2020-02-25 15:50 ` [PATCH v4 4/4] ARM: at91/dt: sama5d2: " Codrin Ciubotariu
@ 2020-03-29 21:13 ` Ludovic.Desroches
  2020-04-21  7:51   ` Codrin.Ciubotariu
  2020-05-05 14:37 ` Wolfram Sang
  4 siblings, 1 reply; 10+ messages in thread
From: Ludovic.Desroches @ 2020-03-29 21:13 UTC (permalink / raw)
  To: Codrin.Ciubotariu, linux-i2c, linux-arm-kernel, linux-kernel, devicetree
  Cc: Nicolas.Ferre, alexandre.belloni, robh+dt, peda, linux, wsa

On 2/25/2020 4:50 PM, Codrin Ciubotariu wrote:
> 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>

Sounds good so
Acked-by: Ludovic Desroches <ludovic.desroches@microchip.com>

Wolfram, you have accepted the v3 of the recovery support and requested 
these changes. It has been sent as a v4, but, in fact, this patch goes 
on top on v3. It could, should, have been a new patch.

Regards

Ludovic

> ---
> 
> Changes in v4:
>   - moved the HW bus clear command in a recover_bus() function, to be able
>     to just call i2c_recover_bus() if a transfer fails;
>   - recovery gpios are no longer taken if HW bus clear command is
>     supported;
> 
> Changes in v3:
>   - removed unnecessary condition from info print;
>   - removed unneded declarations;
> 
> Changes in v2:
>   - called i2c_recover_bus() after an error occurs, if SDA is down;
>   - release gpios if recovery information is incomplete;
> 
>   drivers/i2c/busses/i2c-at91-core.c   |  2 ++
>   drivers/i2c/busses/i2c-at91-master.c | 49 ++++++++++++++++++++++++----
>   drivers/i2c/busses/i2c-at91.h        |  7 +++-
>   3 files changed, 50 insertions(+), 8 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..776e95962ab6 100644
> --- a/drivers/i2c/busses/i2c-at91-master.c
> +++ b/drivers/i2c/busses/i2c-at91-master.c
> @@ -480,7 +480,6 @@ 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;
>   
>   	/*
>   	 * WARNING: the TXCOMP bit in the Status Register is NOT a clear on
> @@ -641,11 +640,12 @@ 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
> +	 */
> +	i2c_recover_bus(&dev->adapter);
>   
>   	return ret;
>   }
> @@ -830,7 +830,7 @@ static void at91_unprepare_twi_recovery(struct i2c_adapter *adap)
>   	pinctrl_select_state(dev->pinctrl, dev->pinctrl_pins_default);
>   }
>   
> -static int at91_init_twi_recovery_info(struct platform_device *pdev,
> +static int at91_init_twi_recovery_gpio(struct platform_device *pdev,
>   				       struct at91_twi_dev *dev)
>   {
>   	struct i2c_bus_recovery_info *rinfo = &dev->rinfo;
> @@ -880,6 +880,41 @@ static int at91_init_twi_recovery_info(struct platform_device *pdev,
>   	return 0;
>   }
>   
> +static int at91_twi_recover_bus_cmd(struct i2c_adapter *adap)
> +{
> +	struct at91_twi_dev *dev = i2c_get_adapdata(adap);
> +
> +	dev->transfer_status |= at91_twi_read(dev, AT91_TWI_SR);
> +	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);
> +	}
> +
> +	return 0;
> +}
> +
> +static int at91_init_twi_recovery_info(struct platform_device *pdev,
> +				       struct at91_twi_dev *dev)
> +{
> +	struct i2c_bus_recovery_info *rinfo = &dev->rinfo;
> +	bool has_clear_cmd = dev->pdata->has_clear_cmd;
> +
> +	if (!has_clear_cmd)
> +		return at91_init_twi_recovery_gpio(pdev, dev);
> +
> +	rinfo->recover_bus = at91_twi_recover_bus_cmd;
> +	dev->adapter.bus_recovery_info = rinfo;
> +
> +	return 0;
> +}
> +
>   int at91_twi_probe_master(struct platform_device *pdev,
>   			  u32 phy_addr, struct at91_twi_dev *dev)
>   {
> 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;
>   };
>   
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 1/4] i2c: at91: Send bus clear command if SDA is down
  2020-03-29 21:13 ` [PATCH v4 1/4] i2c: at91: Send bus clear command if SDA is down Ludovic.Desroches
@ 2020-04-21  7:51   ` Codrin.Ciubotariu
  0 siblings, 0 replies; 10+ messages in thread
From: Codrin.Ciubotariu @ 2020-04-21  7:51 UTC (permalink / raw)
  To: Ludovic.Desroches, linux-i2c, linux-arm-kernel, linux-kernel, devicetree
  Cc: Nicolas.Ferre, alexandre.belloni, robh+dt, peda, linux, wsa

On 30.03.2020 00:13, Ludovic Desroches - M43218 wrote:
> On 2/25/2020 4:50 PM, Codrin Ciubotariu wrote:
>> 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>
> 
> Sounds good so
> Acked-by: Ludovic Desroches <ludovic.desroches@microchip.com>
> 
> Wolfram, you have accepted the v3 of the recovery support and requested
> these changes. It has been sent as a v4, but, in fact, this patch goes
> on top on v3. It could, should, have been a new patch.
> 
> Regards
> 
> Ludovic

Hello, any updates on this patch? Do you want me to change anything?

Best regards,
Codrin

> 
>> ---
>>
>> Changes in v4:
>>    - moved the HW bus clear command in a recover_bus() function, to be able
>>      to just call i2c_recover_bus() if a transfer fails;
>>    - recovery gpios are no longer taken if HW bus clear command is
>>      supported;
>>
>> Changes in v3:
>>    - removed unnecessary condition from info print;
>>    - removed unneded declarations;
>>
>> Changes in v2:
>>    - called i2c_recover_bus() after an error occurs, if SDA is down;
>>    - release gpios if recovery information is incomplete;
>>
>>    drivers/i2c/busses/i2c-at91-core.c   |  2 ++
>>    drivers/i2c/busses/i2c-at91-master.c | 49 ++++++++++++++++++++++++----
>>    drivers/i2c/busses/i2c-at91.h        |  7 +++-
>>    3 files changed, 50 insertions(+), 8 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..776e95962ab6 100644
>> --- a/drivers/i2c/busses/i2c-at91-master.c
>> +++ b/drivers/i2c/busses/i2c-at91-master.c
>> @@ -480,7 +480,6 @@ 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;
>>    
>>    	/*
>>    	 * WARNING: the TXCOMP bit in the Status Register is NOT a clear on
>> @@ -641,11 +640,12 @@ 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
>> +	 */
>> +	i2c_recover_bus(&dev->adapter);
>>    
>>    	return ret;
>>    }
>> @@ -830,7 +830,7 @@ static void at91_unprepare_twi_recovery(struct i2c_adapter *adap)
>>    	pinctrl_select_state(dev->pinctrl, dev->pinctrl_pins_default);
>>    }
>>    
>> -static int at91_init_twi_recovery_info(struct platform_device *pdev,
>> +static int at91_init_twi_recovery_gpio(struct platform_device *pdev,
>>    				       struct at91_twi_dev *dev)
>>    {
>>    	struct i2c_bus_recovery_info *rinfo = &dev->rinfo;
>> @@ -880,6 +880,41 @@ static int at91_init_twi_recovery_info(struct platform_device *pdev,
>>    	return 0;
>>    }
>>    
>> +static int at91_twi_recover_bus_cmd(struct i2c_adapter *adap)
>> +{
>> +	struct at91_twi_dev *dev = i2c_get_adapdata(adap);
>> +
>> +	dev->transfer_status |= at91_twi_read(dev, AT91_TWI_SR);
>> +	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);
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int at91_init_twi_recovery_info(struct platform_device *pdev,
>> +				       struct at91_twi_dev *dev)
>> +{
>> +	struct i2c_bus_recovery_info *rinfo = &dev->rinfo;
>> +	bool has_clear_cmd = dev->pdata->has_clear_cmd;
>> +
>> +	if (!has_clear_cmd)
>> +		return at91_init_twi_recovery_gpio(pdev, dev);
>> +
>> +	rinfo->recover_bus = at91_twi_recover_bus_cmd;
>> +	dev->adapter.bus_recovery_info = rinfo;
>> +
>> +	return 0;
>> +}
>> +
>>    int at91_twi_probe_master(struct platform_device *pdev,
>>    			  u32 phy_addr, struct at91_twi_dev *dev)
>>    {
>> 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;
>>    };
>>    
>>
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 1/4] i2c: at91: Send bus clear command if SDA is down
  2020-02-25 15:50 [PATCH v4 1/4] i2c: at91: Send bus clear command if SDA is down Codrin Ciubotariu
                   ` (3 preceding siblings ...)
  2020-03-29 21:13 ` [PATCH v4 1/4] i2c: at91: Send bus clear command if SDA is down Ludovic.Desroches
@ 2020-05-05 14:37 ` Wolfram Sang
  4 siblings, 0 replies; 10+ messages in thread
From: Wolfram Sang @ 2020-05-05 14:37 UTC (permalink / raw)
  To: Codrin Ciubotariu
  Cc: linux-i2c, linux-arm-kernel, linux-kernel, devicetree,
	ludovic.desroches, nicolas.ferre, alexandre.belloni, robh+dt,
	peda, linux

[-- Attachment #1: Type: text/plain, Size: 457 bytes --]

On Tue, Feb 25, 2020 at 05:50:09PM +0200, Codrin Ciubotariu wrote:
> 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>

Applied to for-next, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-05-05 14:37 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-25 15:50 [PATCH v4 1/4] i2c: at91: Send bus clear command if SDA is down Codrin Ciubotariu
2020-02-25 15:50 ` [PATCH v4 2/4] ARM: at91/dt: sama5d3: add i2c gpio pinctrl Codrin Ciubotariu
2020-03-09 21:20   ` Alexandre Belloni
2020-02-25 15:50 ` [PATCH v4 3/4] ARM: at91/dt: sama5d4: " Codrin Ciubotariu
2020-03-09 21:20   ` Alexandre Belloni
2020-02-25 15:50 ` [PATCH v4 4/4] ARM: at91/dt: sama5d2: " Codrin Ciubotariu
2020-03-09 21:20   ` Alexandre Belloni
2020-03-29 21:13 ` [PATCH v4 1/4] i2c: at91: Send bus clear command if SDA is down Ludovic.Desroches
2020-04-21  7:51   ` Codrin.Ciubotariu
2020-05-05 14:37 ` Wolfram Sang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).