[PATCH 0/3] i2c: stm32f7: several fixes in error cases

[PATCH 0/3] i2c: stm32f7: several fixes in error cases

[Alain Volmat]

This serie provides several fixes needed for cases when communication
when a device is not behaving properly.

Alain Volmat (3):
  i2c: stm32f7: recover the bus on access timeout
  i2c: stm32f7: flush TX FIFO upon transfer errors
  i2c: stm32f7: prevent calling slave handling if no slave running

 drivers/i2c/busses/i2c-stm32f7.c | 31 +++++++++++++++++++++++++++----
 1 file changed, 27 insertions(+), 4 deletions(-)

-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/3] 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 b9b19a2a2ffa..d596733136c3 100644
--- a/drivers/i2c/busses/i2c-stm32f7.c
+++ b/drivers/i2c/busses/i2c-stm32f7.c
@@ -1702,6 +1702,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;
 	}
 
@@ -1751,6 +1752,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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/3] 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 the TX Fifo
before the transfer get stopped.
This commit add FIFO 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 | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-stm32f7.c b/drivers/i2c/busses/i2c-stm32f7.c
index d596733136c3..0d99c075deb2 100644
--- a/drivers/i2c/busses/i2c-stm32f7.c
+++ b/drivers/i2c/busses/i2c-stm32f7.c
@@ -1696,6 +1696,15 @@ 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 in the TX Fifo. To avoid sending old data in a
+		 * further transfer, flush FIFO in case of any error
+		 */
+		writel_relaxed(STM32F7_I2C_ISR_TXE,
+			       i2c_dev->base + STM32F7_I2C_ISR);
+	}
 
 	if (!time_left) {
 		dev_dbg(i2c_dev->dev, "Access to slave 0x%x timed out\n",
@@ -1745,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 in the TX Fifo. To avoid sending old data in a
+		 * further transfer, flush FIFO 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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/3] i2c: stm32f7: prevent calling slave handling if no slave running

[Alain Volmat]

Slave interrupt handler should only be called if there is actually
a slave registered and running to avoid accessing an invalid pointer.

Without this commit, an OOPS can be generated due to a NULL ptr dereference
while receiving an IT when there is no master transfer and no slave
running:
  - stm32f7_i2c_isr_event
  - no master_mode hence calling stm32f7_i2c_slave_isr_event
  - access to i2c_dev->slave_running leading to oops due to
slave_running being NULL.

Fixes: 60d609f30de2 ("i2c: i2c-stm32f7: Add slave support")

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

diff --git a/drivers/i2c/busses/i2c-stm32f7.c b/drivers/i2c/busses/i2c-stm32f7.c
index 0d99c075deb2..2cc9bb0f6d7f 100644
--- a/drivers/i2c/busses/i2c-stm32f7.c
+++ b/drivers/i2c/busses/i2c-stm32f7.c
@@ -1497,10 +1497,14 @@ static irqreturn_t stm32f7_i2c_isr_event(int irq, void *data)
 	u32 status, mask;
 	int ret = IRQ_HANDLED;
 
-	/* Check if the interrupt if for a slave device */
+	/* Check if the interrupt is for a slave device */
 	if (!i2c_dev->master_mode) {
-		ret = stm32f7_i2c_slave_isr_event(i2c_dev);
-		return ret;
+		if (i2c_dev->slave_running)
+			return stm32f7_i2c_slave_isr_event(i2c_dev);
+
+		dev_warn_ratelimited(i2c_dev->dev,
+				"Unexpected IT received: ISR:0x%x\n",
+				readl_relaxed(i2c_dev->base + STM32F7_I2C_ISR));
 	}
 
 	status = readl_relaxed(i2c_dev->base + STM32F7_I2C_ISR);
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/3] i2c: stm32f7: prevent calling slave handling if no slave running

[Wolfram Sang]

[-- Attachment #1.1: Type: text/plain, Size: 768 bytes --]

On Wed, Jun 30, 2021 at 04:11:43PM +0200, Alain Volmat wrote:
> Slave interrupt handler should only be called if there is actually
> a slave registered and running to avoid accessing an invalid pointer.
> 
> Without this commit, an OOPS can be generated due to a NULL ptr dereference
> while receiving an IT when there is no master transfer and no slave
> running:
>   - stm32f7_i2c_isr_event
>   - no master_mode hence calling stm32f7_i2c_slave_isr_event
>   - access to i2c_dev->slave_running leading to oops due to
> slave_running being NULL.
> 
> Fixes: 60d609f30de2 ("i2c: i2c-stm32f7: Add slave support")
> 
> Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>

Is this one still of interest? You resent patches 1 and 2 but not this
one?


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

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/3] i2c: stm32f7: prevent calling slave handling if no slave running

[Alain Volmat]

Hi Wolfram,

On Mon, Nov 29, 2021 at 01:21:05PM +0100, Wolfram Sang wrote:
> On Wed, Jun 30, 2021 at 04:11:43PM +0200, Alain Volmat wrote:
> > Slave interrupt handler should only be called if there is actually
> > a slave registered and running to avoid accessing an invalid pointer.
> > 
> > Without this commit, an OOPS can be generated due to a NULL ptr dereference
> > while receiving an IT when there is no master transfer and no slave
> > running:
> >   - stm32f7_i2c_isr_event
> >   - no master_mode hence calling stm32f7_i2c_slave_isr_event
> >   - access to i2c_dev->slave_running leading to oops due to
> > slave_running being NULL.
> > 
> > Fixes: 60d609f30de2 ("i2c: i2c-stm32f7: Add slave support")
> > 
> > Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>
> 
> Is this one still of interest? You resent patches 1 and 2 but not this
> one?

No you can ignore it. Thanks.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/3] i2c: stm32f7: several fixes in error cases

[Alain Volmat]

Hi,

please put this on hold for the moment as I might have found an issue
with one patch.
Sorry for the noise.

Regards,
Alain

On Wed, Jun 30, 2021 at 04:11:40PM +0200, Alain Volmat wrote:
> This serie provides several fixes needed for cases when communication
> when a device is not behaving properly.
> 
> Alain Volmat (3):
>   i2c: stm32f7: recover the bus on access timeout
>   i2c: stm32f7: flush TX FIFO upon transfer errors
>   i2c: stm32f7: prevent calling slave handling if no slave running
> 
>  drivers/i2c/busses/i2c-stm32f7.c | 31 +++++++++++++++++++++++++++----
>  1 file changed, 27 insertions(+), 4 deletions(-)
> 
> -- 
> 2.25.1
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel