[PATCH 0/4] i2c: stm32: various fixes & dmaengine updates

[PATCH 0/4] i2c: stm32: various fixes & dmaengine updates

[Alain Volmat]

This serie contains 3 fixes for I²C error handling cases.
It also includes a patch to get rid of the deprecated
dmaengine_terminate_all calls.

Alain Volmat (4):
  i2c: stm32f7: flush TX FIFO upon transfer errors
  i2c: stm32f7: recover the bus on access timeout
  i2c: stm32f7: stop dma transfer in case of NACK
  i2c: stm32f7: use proper DMAENGINE API for termination

 drivers/i2c/busses/i2c-stm32f7.c | 45 +++++++++++++++++++++++++++-----
 1 file changed, 38 insertions(+), 7 deletions(-)

-- 
2.25.1

[PATCH 1/4] i2c: stm32f7: flush TX FIFO upon transfer errors

[Alain Volmat]

While handling an error during transfer (ex: NACK), it could
happen that the driver has already written data into TXDR
before the transfer get stopped.
This commit add TXDR Flush after end of transfer in case of error to
avoid sending a wrong data on any other slave upon next transfer.

Fixes: aeb068c57214 ("i2c: i2c-stm32f7: add driver")

Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>
---
 drivers/i2c/busses/i2c-stm32f7.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-stm32f7.c b/drivers/i2c/busses/i2c-stm32f7.c
index b9b19a2a2ffa..ed977b6f7ab6 100644
--- a/drivers/i2c/busses/i2c-stm32f7.c
+++ b/drivers/i2c/busses/i2c-stm32f7.c
@@ -1696,6 +1696,16 @@ static int stm32f7_i2c_xfer(struct i2c_adapter *i2c_adap,
 	time_left = wait_for_completion_timeout(&i2c_dev->complete,
 						i2c_dev->adap.timeout);
 	ret = f7_msg->result;
+	if (ret) {
+		/*
+		 * It is possible that some unsent data have already been
+		 * written into TXDR. To avoid sending old data in a
+		 * further transfer, flush TXDR in case of any error
+		 */
+		writel_relaxed(STM32F7_I2C_ISR_TXE,
+			       i2c_dev->base + STM32F7_I2C_ISR);
+		goto pm_free;
+	}
 
 	if (!time_left) {
 		dev_dbg(i2c_dev->dev, "Access to slave 0x%x timed out\n",
@@ -1744,8 +1754,16 @@ static int stm32f7_i2c_smbus_xfer(struct i2c_adapter *adapter, u16 addr,
 	timeout = wait_for_completion_timeout(&i2c_dev->complete,
 					      i2c_dev->adap.timeout);
 	ret = f7_msg->result;
-	if (ret)
+	if (ret) {
+		/*
+		 * It is possible that some unsent data have already been
+		 * written into TXDR. To avoid sending old data in a
+		 * further transfer, flush TXDR in case of any error
+		 */
+		writel_relaxed(STM32F7_I2C_ISR_TXE,
+			       i2c_dev->base + STM32F7_I2C_ISR);
 		goto pm_free;
+	}
 
 	if (!timeout) {
 		dev_dbg(dev, "Access to slave 0x%x timed out\n", f7_msg->addr);
-- 
2.25.1

[PATCH 2/4] i2c: stm32f7: recover the bus on access timeout

[Alain Volmat]

When getting an access timeout, ensure that the bus is in a proper
state prior to returning the error.

Fixes: aeb068c57214 ("i2c: i2c-stm32f7: add driver")

Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>
---
 drivers/i2c/busses/i2c-stm32f7.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/i2c/busses/i2c-stm32f7.c b/drivers/i2c/busses/i2c-stm32f7.c
index ed977b6f7ab6..ad3459a3bc5e 100644
--- a/drivers/i2c/busses/i2c-stm32f7.c
+++ b/drivers/i2c/busses/i2c-stm32f7.c
@@ -1712,6 +1712,7 @@ static int stm32f7_i2c_xfer(struct i2c_adapter *i2c_adap,
 			i2c_dev->msg->addr);
 		if (i2c_dev->use_dma)
 			dmaengine_terminate_all(dma->chan_using);
+		stm32f7_i2c_wait_free_bus(i2c_dev);
 		ret = -ETIMEDOUT;
 	}
 
@@ -1769,6 +1770,7 @@ static int stm32f7_i2c_smbus_xfer(struct i2c_adapter *adapter, u16 addr,
 		dev_dbg(dev, "Access to slave 0x%x timed out\n", f7_msg->addr);
 		if (i2c_dev->use_dma)
 			dmaengine_terminate_all(dma->chan_using);
+		stm32f7_i2c_wait_free_bus(i2c_dev);
 		ret = -ETIMEDOUT;
 		goto pm_free;
 	}
-- 
2.25.1

[PATCH 3/4] i2c: stm32f7: stop dma transfer in case of NACK

[Alain Volmat]

In case of receiving a NACK, the dma transfer should be stopped
to avoid feeding data into the FIFO.
Also ensure to properly return the proper error code and avoid
waiting for the end of the dma completion in case of
error happening during the transmission.

Fixes: 7ecc8cfde553 ("i2c: i2c-stm32f7: Add DMA support")

Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>
---
 drivers/i2c/busses/i2c-stm32f7.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-stm32f7.c b/drivers/i2c/busses/i2c-stm32f7.c
index ad3459a3bc5e..50d5ae81d227 100644
--- a/drivers/i2c/busses/i2c-stm32f7.c
+++ b/drivers/i2c/busses/i2c-stm32f7.c
@@ -1493,6 +1493,7 @@ static irqreturn_t stm32f7_i2c_isr_event(int irq, void *data)
 {
 	struct stm32f7_i2c_dev *i2c_dev = data;
 	struct stm32f7_i2c_msg *f7_msg = &i2c_dev->f7_msg;
+	struct stm32_i2c_dma *dma = i2c_dev->dma;
 	void __iomem *base = i2c_dev->base;
 	u32 status, mask;
 	int ret = IRQ_HANDLED;
@@ -1518,6 +1519,10 @@ static irqreturn_t stm32f7_i2c_isr_event(int irq, void *data)
 		dev_dbg(i2c_dev->dev, "<%s>: Receive NACK (addr %x)\n",
 			__func__, f7_msg->addr);
 		writel_relaxed(STM32F7_I2C_ICR_NACKCF, base + STM32F7_I2C_ICR);
+		if (i2c_dev->use_dma) {
+			stm32f7_i2c_disable_dma_req(i2c_dev);
+			dmaengine_terminate_all(dma->chan_using);
+		}
 		f7_msg->result = -ENXIO;
 	}
 
@@ -1533,7 +1538,7 @@ static irqreturn_t stm32f7_i2c_isr_event(int irq, void *data)
 		/* Clear STOP flag */
 		writel_relaxed(STM32F7_I2C_ICR_STOPCF, base + STM32F7_I2C_ICR);
 
-		if (i2c_dev->use_dma) {
+		if (i2c_dev->use_dma && !f7_msg->result) {
 			ret = IRQ_WAKE_THREAD;
 		} else {
 			i2c_dev->master_mode = false;
@@ -1546,7 +1551,7 @@ static irqreturn_t stm32f7_i2c_isr_event(int irq, void *data)
 		if (f7_msg->stop) {
 			mask = STM32F7_I2C_CR2_STOP;
 			stm32f7_i2c_set_bits(base + STM32F7_I2C_CR2, mask);
-		} else if (i2c_dev->use_dma) {
+		} else if (i2c_dev->use_dma && !f7_msg->result) {
 			ret = IRQ_WAKE_THREAD;
 		} else if (f7_msg->smbus) {
 			stm32f7_i2c_smbus_rep_start(i2c_dev);
-- 
2.25.1

[PATCH 4/4] i2c: stm32f7: use proper DMAENGINE API for termination

[Alain Volmat]

dmaengine_terminate_all() is deprecated in favor of explicitly saying if
it should be sync or async.  Here, we use dmaengine_terminate_sync in
i2c_xfer and i2c_smbus_xfer handlers and rely on
dmaengine_terminate_async within interrupt handlers
(transmission error cases).
dmaengine_synchronize is added within i2c_xfer and i2c_smbus_xfer handler
to finalize terminate started in interrupt handlers.

Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>
---
 drivers/i2c/busses/i2c-stm32f7.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/busses/i2c-stm32f7.c b/drivers/i2c/busses/i2c-stm32f7.c
index 50d5ae81d227..66145d2b9b55 100644
--- a/drivers/i2c/busses/i2c-stm32f7.c
+++ b/drivers/i2c/busses/i2c-stm32f7.c
@@ -1521,7 +1521,7 @@ static irqreturn_t stm32f7_i2c_isr_event(int irq, void *data)
 		writel_relaxed(STM32F7_I2C_ICR_NACKCF, base + STM32F7_I2C_ICR);
 		if (i2c_dev->use_dma) {
 			stm32f7_i2c_disable_dma_req(i2c_dev);
-			dmaengine_terminate_all(dma->chan_using);
+			dmaengine_terminate_async(dma->chan_using);
 		}
 		f7_msg->result = -ENXIO;
 	}
@@ -1588,7 +1588,7 @@ static irqreturn_t stm32f7_i2c_isr_event_thread(int irq, void *data)
 	if (!ret) {
 		dev_dbg(i2c_dev->dev, "<%s>: Timed out\n", __func__);
 		stm32f7_i2c_disable_dma_req(i2c_dev);
-		dmaengine_terminate_all(dma->chan_using);
+		dmaengine_terminate_async(dma->chan_using);
 		f7_msg->result = -ETIMEDOUT;
 	}
 
@@ -1665,7 +1665,7 @@ static irqreturn_t stm32f7_i2c_isr_error(int irq, void *data)
 	/* Disable dma */
 	if (i2c_dev->use_dma) {
 		stm32f7_i2c_disable_dma_req(i2c_dev);
-		dmaengine_terminate_all(dma->chan_using);
+		dmaengine_terminate_async(dma->chan_using);
 	}
 
 	i2c_dev->master_mode = false;
@@ -1702,6 +1702,9 @@ static int stm32f7_i2c_xfer(struct i2c_adapter *i2c_adap,
 						i2c_dev->adap.timeout);
 	ret = f7_msg->result;
 	if (ret) {
+		if (i2c_dev->use_dma)
+			dmaengine_synchronize(dma->chan_using);
+
 		/*
 		 * It is possible that some unsent data have already been
 		 * written into TXDR. To avoid sending old data in a
@@ -1716,7 +1719,7 @@ static int stm32f7_i2c_xfer(struct i2c_adapter *i2c_adap,
 		dev_dbg(i2c_dev->dev, "Access to slave 0x%x timed out\n",
 			i2c_dev->msg->addr);
 		if (i2c_dev->use_dma)
-			dmaengine_terminate_all(dma->chan_using);
+			dmaengine_terminate_sync(dma->chan_using);
 		stm32f7_i2c_wait_free_bus(i2c_dev);
 		ret = -ETIMEDOUT;
 	}
@@ -1761,6 +1764,9 @@ static int stm32f7_i2c_smbus_xfer(struct i2c_adapter *adapter, u16 addr,
 					      i2c_dev->adap.timeout);
 	ret = f7_msg->result;
 	if (ret) {
+		if (i2c_dev->use_dma)
+			dmaengine_synchronize(dma->chan_using);
+
 		/*
 		 * It is possible that some unsent data have already been
 		 * written into TXDR. To avoid sending old data in a
@@ -1774,7 +1780,7 @@ static int stm32f7_i2c_smbus_xfer(struct i2c_adapter *adapter, u16 addr,
 	if (!timeout) {
 		dev_dbg(dev, "Access to slave 0x%x timed out\n", f7_msg->addr);
 		if (i2c_dev->use_dma)
-			dmaengine_terminate_all(dma->chan_using);
+			dmaengine_terminate_sync(dma->chan_using);
 		stm32f7_i2c_wait_free_bus(i2c_dev);
 		ret = -ETIMEDOUT;
 		goto pm_free;
-- 
2.25.1

Re: [PATCH 1/4] i2c: stm32f7: flush TX FIFO upon transfer errors

[Pierre Yves MORDRET]

Hi Alain

Reviewed-by: Pierre-Yves MORDRET <pierre-yves.mordret@foss.st.com>

Regards

On 9/20/21 5:21 PM, Alain Volmat wrote:
> While handling an error during transfer (ex: NACK), it could
> happen that the driver has already written data into TXDR
> before the transfer get stopped.
> This commit add TXDR Flush after end of transfer in case of error to
> avoid sending a wrong data on any other slave upon next transfer.
> 
> Fixes: aeb068c57214 ("i2c: i2c-stm32f7: add driver")
> 
> Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>
> ---
>  drivers/i2c/busses/i2c-stm32f7.c | 20 +++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/busses/i2c-stm32f7.c b/drivers/i2c/busses/i2c-stm32f7.c
> index b9b19a2a2ffa..ed977b6f7ab6 100644
> --- a/drivers/i2c/busses/i2c-stm32f7.c
> +++ b/drivers/i2c/busses/i2c-stm32f7.c
> @@ -1696,6 +1696,16 @@ static int stm32f7_i2c_xfer(struct i2c_adapter *i2c_adap,
>  	time_left = wait_for_completion_timeout(&i2c_dev->complete,
>  						i2c_dev->adap.timeout);
>  	ret = f7_msg->result;
> +	if (ret) {
> +		/*
> +		 * It is possible that some unsent data have already been
> +		 * written into TXDR. To avoid sending old data in a
> +		 * further transfer, flush TXDR in case of any error
> +		 */
> +		writel_relaxed(STM32F7_I2C_ISR_TXE,
> +			       i2c_dev->base + STM32F7_I2C_ISR);
> +		goto pm_free;
> +	}
>  
>  	if (!time_left) {
>  		dev_dbg(i2c_dev->dev, "Access to slave 0x%x timed out\n",
> @@ -1744,8 +1754,16 @@ static int stm32f7_i2c_smbus_xfer(struct i2c_adapter *adapter, u16 addr,
>  	timeout = wait_for_completion_timeout(&i2c_dev->complete,
>  					      i2c_dev->adap.timeout);
>  	ret = f7_msg->result;
> -	if (ret)
> +	if (ret) {
> +		/*
> +		 * It is possible that some unsent data have already been
> +		 * written into TXDR. To avoid sending old data in a
> +		 * further transfer, flush TXDR in case of any error
> +		 */
> +		writel_relaxed(STM32F7_I2C_ISR_TXE,
> +			       i2c_dev->base + STM32F7_I2C_ISR);
>  		goto pm_free;
> +	}
>  
>  	if (!timeout) {
>  		dev_dbg(dev, "Access to slave 0x%x timed out\n", f7_msg->addr);
> 

-- 
--
~ Py MORDRET
--

Re: [PATCH 2/4] i2c: stm32f7: recover the bus on access timeout

[Pierre Yves MORDRET]

Hi Alain

Look good to me

Reviewed-by: Pierre-Yves MORDRET <pierre-yves.mordret@foss.st.com>

Regards

On 9/20/21 5:21 PM, Alain Volmat wrote:
> When getting an access timeout, ensure that the bus is in a proper
> state prior to returning the error.
> 
> Fixes: aeb068c57214 ("i2c: i2c-stm32f7: add driver")
> 
> Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>
> ---
>  drivers/i2c/busses/i2c-stm32f7.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/i2c/busses/i2c-stm32f7.c b/drivers/i2c/busses/i2c-stm32f7.c
> index ed977b6f7ab6..ad3459a3bc5e 100644
> --- a/drivers/i2c/busses/i2c-stm32f7.c
> +++ b/drivers/i2c/busses/i2c-stm32f7.c
> @@ -1712,6 +1712,7 @@ static int stm32f7_i2c_xfer(struct i2c_adapter *i2c_adap,
>  			i2c_dev->msg->addr);
>  		if (i2c_dev->use_dma)
>  			dmaengine_terminate_all(dma->chan_using);
> +		stm32f7_i2c_wait_free_bus(i2c_dev);
>  		ret = -ETIMEDOUT;
>  	}
>  
> @@ -1769,6 +1770,7 @@ static int stm32f7_i2c_smbus_xfer(struct i2c_adapter *adapter, u16 addr,
>  		dev_dbg(dev, "Access to slave 0x%x timed out\n", f7_msg->addr);
>  		if (i2c_dev->use_dma)
>  			dmaengine_terminate_all(dma->chan_using);
> +		stm32f7_i2c_wait_free_bus(i2c_dev);
>  		ret = -ETIMEDOUT;
>  		goto pm_free;
>  	}
> 

-- 
--
~ Py MORDRET
--

Re: [PATCH 3/4] i2c: stm32f7: stop dma transfer in case of NACK

[Pierre Yves MORDRET]

Hi Alain

Look good to me

Reviewed-by: Pierre-Yves MORDRET <pierre-yves.mordret@foss.st.com>

Regards

On 9/20/21 5:21 PM, Alain Volmat wrote:
> In case of receiving a NACK, the dma transfer should be stopped
> to avoid feeding data into the FIFO.
> Also ensure to properly return the proper error code and avoid
> waiting for the end of the dma completion in case of
> error happening during the transmission.
> 
> Fixes: 7ecc8cfde553 ("i2c: i2c-stm32f7: Add DMA support")
> 
> Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>
> ---
>  drivers/i2c/busses/i2c-stm32f7.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-stm32f7.c b/drivers/i2c/busses/i2c-stm32f7.c
> index ad3459a3bc5e..50d5ae81d227 100644
> --- a/drivers/i2c/busses/i2c-stm32f7.c
> +++ b/drivers/i2c/busses/i2c-stm32f7.c
> @@ -1493,6 +1493,7 @@ static irqreturn_t stm32f7_i2c_isr_event(int irq, void *data)
>  {
>  	struct stm32f7_i2c_dev *i2c_dev = data;
>  	struct stm32f7_i2c_msg *f7_msg = &i2c_dev->f7_msg;
> +	struct stm32_i2c_dma *dma = i2c_dev->dma;
>  	void __iomem *base = i2c_dev->base;
>  	u32 status, mask;
>  	int ret = IRQ_HANDLED;
> @@ -1518,6 +1519,10 @@ static irqreturn_t stm32f7_i2c_isr_event(int irq, void *data)
>  		dev_dbg(i2c_dev->dev, "<%s>: Receive NACK (addr %x)\n",
>  			__func__, f7_msg->addr);
>  		writel_relaxed(STM32F7_I2C_ICR_NACKCF, base + STM32F7_I2C_ICR);
> +		if (i2c_dev->use_dma) {
> +			stm32f7_i2c_disable_dma_req(i2c_dev);
> +			dmaengine_terminate_all(dma->chan_using);
> +		}
>  		f7_msg->result = -ENXIO;
>  	}
>  
> @@ -1533,7 +1538,7 @@ static irqreturn_t stm32f7_i2c_isr_event(int irq, void *data)
>  		/* Clear STOP flag */
>  		writel_relaxed(STM32F7_I2C_ICR_STOPCF, base + STM32F7_I2C_ICR);
>  
> -		if (i2c_dev->use_dma) {
> +		if (i2c_dev->use_dma && !f7_msg->result) {
>  			ret = IRQ_WAKE_THREAD;
>  		} else {
>  			i2c_dev->master_mode = false;
> @@ -1546,7 +1551,7 @@ static irqreturn_t stm32f7_i2c_isr_event(int irq, void *data)
>  		if (f7_msg->stop) {
>  			mask = STM32F7_I2C_CR2_STOP;
>  			stm32f7_i2c_set_bits(base + STM32F7_I2C_CR2, mask);
> -		} else if (i2c_dev->use_dma) {
> +		} else if (i2c_dev->use_dma && !f7_msg->result) {
>  			ret = IRQ_WAKE_THREAD;
>  		} else if (f7_msg->smbus) {
>  			stm32f7_i2c_smbus_rep_start(i2c_dev);
> 

-- 
--
~ Py MORDRET
--

Re: [PATCH 4/4] i2c: stm32f7: use proper DMAENGINE API for termination

[Pierre Yves MORDRET]

Hi Alain

Look good to me

Reviewed-by: Pierre-Yves MORDRET <pierre-yves.mordret@foss.st.com>

Regards

On 9/20/21 5:21 PM, Alain Volmat wrote:
> dmaengine_terminate_all() is deprecated in favor of explicitly saying if
> it should be sync or async.  Here, we use dmaengine_terminate_sync in
> i2c_xfer and i2c_smbus_xfer handlers and rely on
> dmaengine_terminate_async within interrupt handlers
> (transmission error cases).
> dmaengine_synchronize is added within i2c_xfer and i2c_smbus_xfer handler
> to finalize terminate started in interrupt handlers.
> 
> Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>
> ---
>  drivers/i2c/busses/i2c-stm32f7.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-stm32f7.c b/drivers/i2c/busses/i2c-stm32f7.c
> index 50d5ae81d227..66145d2b9b55 100644
> --- a/drivers/i2c/busses/i2c-stm32f7.c
> +++ b/drivers/i2c/busses/i2c-stm32f7.c
> @@ -1521,7 +1521,7 @@ static irqreturn_t stm32f7_i2c_isr_event(int irq, void *data)
>  		writel_relaxed(STM32F7_I2C_ICR_NACKCF, base + STM32F7_I2C_ICR);
>  		if (i2c_dev->use_dma) {
>  			stm32f7_i2c_disable_dma_req(i2c_dev);
> -			dmaengine_terminate_all(dma->chan_using);
> +			dmaengine_terminate_async(dma->chan_using);
>  		}
>  		f7_msg->result = -ENXIO;
>  	}
> @@ -1588,7 +1588,7 @@ static irqreturn_t stm32f7_i2c_isr_event_thread(int irq, void *data)
>  	if (!ret) {
>  		dev_dbg(i2c_dev->dev, "<%s>: Timed out\n", __func__);
>  		stm32f7_i2c_disable_dma_req(i2c_dev);
> -		dmaengine_terminate_all(dma->chan_using);
> +		dmaengine_terminate_async(dma->chan_using);
>  		f7_msg->result = -ETIMEDOUT;
>  	}
>  
> @@ -1665,7 +1665,7 @@ static irqreturn_t stm32f7_i2c_isr_error(int irq, void *data)
>  	/* Disable dma */
>  	if (i2c_dev->use_dma) {
>  		stm32f7_i2c_disable_dma_req(i2c_dev);
> -		dmaengine_terminate_all(dma->chan_using);
> +		dmaengine_terminate_async(dma->chan_using);
>  	}
>  
>  	i2c_dev->master_mode = false;
> @@ -1702,6 +1702,9 @@ static int stm32f7_i2c_xfer(struct i2c_adapter *i2c_adap,
>  						i2c_dev->adap.timeout);
>  	ret = f7_msg->result;
>  	if (ret) {
> +		if (i2c_dev->use_dma)
> +			dmaengine_synchronize(dma->chan_using);
> +
>  		/*
>  		 * It is possible that some unsent data have already been
>  		 * written into TXDR. To avoid sending old data in a
> @@ -1716,7 +1719,7 @@ static int stm32f7_i2c_xfer(struct i2c_adapter *i2c_adap,
>  		dev_dbg(i2c_dev->dev, "Access to slave 0x%x timed out\n",
>  			i2c_dev->msg->addr);
>  		if (i2c_dev->use_dma)
> -			dmaengine_terminate_all(dma->chan_using);
> +			dmaengine_terminate_sync(dma->chan_using);
>  		stm32f7_i2c_wait_free_bus(i2c_dev);
>  		ret = -ETIMEDOUT;
>  	}
> @@ -1761,6 +1764,9 @@ static int stm32f7_i2c_smbus_xfer(struct i2c_adapter *adapter, u16 addr,
>  					      i2c_dev->adap.timeout);
>  	ret = f7_msg->result;
>  	if (ret) {
> +		if (i2c_dev->use_dma)
> +			dmaengine_synchronize(dma->chan_using);
> +
>  		/*
>  		 * It is possible that some unsent data have already been
>  		 * written into TXDR. To avoid sending old data in a
> @@ -1774,7 +1780,7 @@ static int stm32f7_i2c_smbus_xfer(struct i2c_adapter *adapter, u16 addr,
>  	if (!timeout) {
>  		dev_dbg(dev, "Access to slave 0x%x timed out\n", f7_msg->addr);
>  		if (i2c_dev->use_dma)
> -			dmaengine_terminate_all(dma->chan_using);
> +			dmaengine_terminate_sync(dma->chan_using);
>  		stm32f7_i2c_wait_free_bus(i2c_dev);
>  		ret = -ETIMEDOUT;
>  		goto pm_free;
> 

-- 
--
~ Py MORDRET
--

Re: [PATCH 1/4] i2c: stm32f7: flush TX FIFO upon transfer errors

[Wolfram Sang]

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

On Mon, Sep 20, 2021 at 05:21:29PM +0200, Alain Volmat wrote:
> While handling an error during transfer (ex: NACK), it could
> happen that the driver has already written data into TXDR
> before the transfer get stopped.
> This commit add TXDR Flush after end of transfer in case of error to
> avoid sending a wrong data on any other slave upon next transfer.
> 
> Fixes: aeb068c57214 ("i2c: i2c-stm32f7: add driver")
> 
> Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>

Applied to for-current, thanks!


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

Re: [PATCH 2/4] i2c: stm32f7: recover the bus on access timeout

[Wolfram Sang]

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


> +		stm32f7_i2c_wait_free_bus(i2c_dev);

This does only a controller reset, not a bus recovery with 9 toggling
pulses, or?


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

Re: [PATCH 2/4] i2c: stm32f7: recover the bus on access timeout

[Alain Volmat]

Hi Wolfram

On Mon, Nov 29, 2021 at 01:17:21PM +0100, Wolfram Sang wrote:
> 
> > +		stm32f7_i2c_wait_free_bus(i2c_dev);
> 
> This does only a controller reset, not a bus recovery with 9 toggling
> pulses, or?

indeed. I might better rework this and at the same time introduce the
bus recovery mechanism via the bus recovery callback in this driver.
Please don't merge this patch and I will rework that.

Re: [PATCH 2/4] i2c: stm32f7: recover the bus on access timeout

[Wolfram Sang]

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

Hi Alain,

> > > +		stm32f7_i2c_wait_free_bus(i2c_dev);
> > 
> > This does only a controller reset, not a bus recovery with 9 toggling
> > pulses, or?
> 
> indeed. I might better rework this and at the same time introduce the
> bus recovery mechanism via the bus recovery callback in this driver.
> Please don't merge this patch and I will rework that.

Wait a sec. Resetting a controller at the end of a failed transfer might
make sense if the controller is otherwise in an confused state.

Full bus recovery (9 pulses) should be done at the beginning of a
transfer when SDA is low, though.

So, I'd actually suggest to apply this patch and add full bus recovery
based on SDA low at the beginning of a transfer seperately.

What do you think?

All the best,

   Wolfram


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

Re: [PATCH 2/4] i2c: stm32f7: recover the bus on access timeout

[Alain Volmat]

Hi Alain,

On Mon, Nov 29, 2021 at 01:52:30PM +0100, Wolfram Sang wrote:
> Hi Alain,
> 
> > > > +		stm32f7_i2c_wait_free_bus(i2c_dev);
> > > 
> > > This does only a controller reset, not a bus recovery with 9 toggling
> > > pulses, or?
> > 
> > indeed. I might better rework this and at the same time introduce the
> > bus recovery mechanism via the bus recovery callback in this driver.
> > Please don't merge this patch and I will rework that.
> 
> Wait a sec. Resetting a controller at the end of a failed transfer might
> make sense if the controller is otherwise in an confused state.
> 
> Full bus recovery (9 pulses) should be done at the beginning of a
> transfer when SDA is low, though.
> 
> So, I'd actually suggest to apply this patch and add full bus recovery
> based on SDA low at the beginning of a transfer seperately.
> 
> What doo you think?

I just checked again.  Indeed, this patch is here to handle cases when
communication went bad with a device leading to controller being left in
a confused state.  This is done to put it back in a working state.

I agree with you on the fact to decouple this with the 9 pulses bus
recovery and first apply this one first.

Thanks.
Alain

> 
> All the best,
> 
>    Wolfram
> 

Re: [PATCH 2/4] i2c: stm32f7: recover the bus on access timeout

[Wolfram Sang]

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


> I agree with you on the fact to decouple this with the 9 pulses bus
> recovery and first apply this one first.

Good. I noticed something in your driver on the way. Will send an RFC in
some minutes.


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

Re: [PATCH 2/4] i2c: stm32f7: recover the bus on access timeout

[Wolfram Sang]

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

On Mon, Sep 20, 2021 at 05:21:30PM +0200, Alain Volmat wrote:
> When getting an access timeout, ensure that the bus is in a proper
> state prior to returning the error.
> 
> Fixes: aeb068c57214 ("i2c: i2c-stm32f7: add driver")
> 
> Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>

Applied to for-current, thanks!


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

Re: [PATCH 3/4] i2c: stm32f7: stop dma transfer in case of NACK

[Wolfram Sang]

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

On Mon, Sep 20, 2021 at 05:21:31PM +0200, Alain Volmat wrote:
> In case of receiving a NACK, the dma transfer should be stopped
> to avoid feeding data into the FIFO.
> Also ensure to properly return the proper error code and avoid
> waiting for the end of the dma completion in case of
> error happening during the transmission.
> 
> Fixes: 7ecc8cfde553 ("i2c: i2c-stm32f7: Add DMA support")
> 
> Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>

Applied to for-current, thanks!


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

Re: [PATCH 4/4] i2c: stm32f7: use proper DMAENGINE API for termination

[Wolfram Sang]

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

On Mon, Sep 20, 2021 at 05:21:32PM +0200, Alain Volmat wrote:
> dmaengine_terminate_all() is deprecated in favor of explicitly saying if
> it should be sync or async.  Here, we use dmaengine_terminate_sync in
> i2c_xfer and i2c_smbus_xfer handlers and rely on
> dmaengine_terminate_async within interrupt handlers
> (transmission error cases).
> dmaengine_synchronize is added within i2c_xfer and i2c_smbus_xfer handler
> to finalize terminate started in interrupt handlers.
> 
> Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>

Applied to for-current, thanks!


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