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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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
>
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 <[email protected]>
>
> Is this one still of interest? You resent patches 1 and 2 but not this
> one?
No you can ignore it. Thanks.
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 <[email protected]>
Is this one still of interest? You resent patches 1 and 2 but not this
one?