linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] i2c: stm32f7: several fixes in error cases
@ 2021-06-30 14:11 Alain Volmat
  2021-06-30 14:11 ` [PATCH 1/3] i2c: stm32f7: recover the bus on access timeout Alain Volmat
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Alain Volmat @ 2021-06-30 14:11 UTC (permalink / raw)
  To: wsa, pierre-yves.mordret
  Cc: alexandre.torgue, linux-i2c, linux-stm32, linux-arm-kernel,
	linux-kernel, fabrice.gasnier, amelie.delaunay, 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

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

* [PATCH 1/3] i2c: stm32f7: recover the bus on access timeout
  2021-06-30 14:11 [PATCH 0/3] i2c: stm32f7: several fixes in error cases Alain Volmat
@ 2021-06-30 14:11 ` Alain Volmat
  2021-06-30 14:11 ` [PATCH 2/3] i2c: stm32f7: flush TX FIFO upon transfer errors Alain Volmat
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Alain Volmat @ 2021-06-30 14:11 UTC (permalink / raw)
  To: wsa, pierre-yves.mordret
  Cc: alexandre.torgue, linux-i2c, linux-stm32, linux-arm-kernel,
	linux-kernel, fabrice.gasnier, amelie.delaunay, 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

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

* [PATCH 2/3] i2c: stm32f7: flush TX FIFO upon transfer errors
  2021-06-30 14:11 [PATCH 0/3] i2c: stm32f7: several fixes in error cases Alain Volmat
  2021-06-30 14:11 ` [PATCH 1/3] i2c: stm32f7: recover the bus on access timeout Alain Volmat
@ 2021-06-30 14:11 ` Alain Volmat
  2021-06-30 14:11 ` [PATCH 3/3] i2c: stm32f7: prevent calling slave handling if no slave running Alain Volmat
  2021-07-01  8:51 ` [PATCH 0/3] i2c: stm32f7: several fixes in error cases Alain Volmat
  3 siblings, 0 replies; 7+ messages in thread
From: Alain Volmat @ 2021-06-30 14:11 UTC (permalink / raw)
  To: wsa, pierre-yves.mordret
  Cc: alexandre.torgue, linux-i2c, linux-stm32, linux-arm-kernel,
	linux-kernel, fabrice.gasnier, amelie.delaunay, 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

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

* [PATCH 3/3] i2c: stm32f7: prevent calling slave handling if no slave running
  2021-06-30 14:11 [PATCH 0/3] i2c: stm32f7: several fixes in error cases Alain Volmat
  2021-06-30 14:11 ` [PATCH 1/3] i2c: stm32f7: recover the bus on access timeout Alain Volmat
  2021-06-30 14:11 ` [PATCH 2/3] i2c: stm32f7: flush TX FIFO upon transfer errors Alain Volmat
@ 2021-06-30 14:11 ` Alain Volmat
  2021-11-29 12:21   ` Wolfram Sang
  2021-07-01  8:51 ` [PATCH 0/3] i2c: stm32f7: several fixes in error cases Alain Volmat
  3 siblings, 1 reply; 7+ messages in thread
From: Alain Volmat @ 2021-06-30 14:11 UTC (permalink / raw)
  To: wsa, pierre-yves.mordret
  Cc: alexandre.torgue, linux-i2c, linux-stm32, linux-arm-kernel,
	linux-kernel, fabrice.gasnier, amelie.delaunay, 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

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

* Re: [PATCH 0/3] i2c: stm32f7: several fixes in error cases
  2021-06-30 14:11 [PATCH 0/3] i2c: stm32f7: several fixes in error cases Alain Volmat
                   ` (2 preceding siblings ...)
  2021-06-30 14:11 ` [PATCH 3/3] i2c: stm32f7: prevent calling slave handling if no slave running Alain Volmat
@ 2021-07-01  8:51 ` Alain Volmat
  3 siblings, 0 replies; 7+ messages in thread
From: Alain Volmat @ 2021-07-01  8:51 UTC (permalink / raw)
  To: wsa, pierre-yves.mordret
  Cc: alexandre.torgue, linux-i2c, linux-stm32, linux-arm-kernel,
	linux-kernel, fabrice.gasnier, amelie.delaunay

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

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

* Re: [PATCH 3/3] i2c: stm32f7: prevent calling slave handling if no slave running
  2021-06-30 14:11 ` [PATCH 3/3] i2c: stm32f7: prevent calling slave handling if no slave running Alain Volmat
@ 2021-11-29 12:21   ` Wolfram Sang
  2021-11-29 12:25     ` Alain Volmat
  0 siblings, 1 reply; 7+ messages in thread
From: Wolfram Sang @ 2021-11-29 12:21 UTC (permalink / raw)
  To: Alain Volmat
  Cc: pierre-yves.mordret, alexandre.torgue, linux-i2c, linux-stm32,
	linux-arm-kernel, linux-kernel, fabrice.gasnier, amelie.delaunay


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

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

* Re: [PATCH 3/3] i2c: stm32f7: prevent calling slave handling if no slave running
  2021-11-29 12:21   ` Wolfram Sang
@ 2021-11-29 12:25     ` Alain Volmat
  0 siblings, 0 replies; 7+ messages in thread
From: Alain Volmat @ 2021-11-29 12:25 UTC (permalink / raw)
  To: Wolfram Sang, pierre-yves.mordret, alexandre.torgue, linux-i2c,
	linux-stm32, linux-arm-kernel, linux-kernel, fabrice.gasnier,
	amelie.delaunay

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

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

end of thread, other threads:[~2021-11-29 12:28 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-30 14:11 [PATCH 0/3] i2c: stm32f7: several fixes in error cases Alain Volmat
2021-06-30 14:11 ` [PATCH 1/3] i2c: stm32f7: recover the bus on access timeout Alain Volmat
2021-06-30 14:11 ` [PATCH 2/3] i2c: stm32f7: flush TX FIFO upon transfer errors Alain Volmat
2021-06-30 14:11 ` [PATCH 3/3] i2c: stm32f7: prevent calling slave handling if no slave running Alain Volmat
2021-11-29 12:21   ` Wolfram Sang
2021-11-29 12:25     ` Alain Volmat
2021-07-01  8:51 ` [PATCH 0/3] i2c: stm32f7: several fixes in error cases Alain Volmat

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