| Message ID | 20220908080627.1762034-4-alain.volmat@foss.st.com |
|---|---|
| State | Superseded |
| Delegated to: | Patrick Delaunay |
| Headers | show |
| Series | i2c: stm32: cleanup & stop handling fix | expand |
Hi Jorge, On Thu, Sep 08, 2022 at 10:36:22AM +0200, Jorge Ramirez-Ortiz, Foundries wrote: > On 08/09/22, Alain Volmat wrote: > > Current function stm32_i2c_message_xfer is sending a STOP > > whatever the result of the transaction is. This can cause issues > > such as making the bus busy since the controller itself is already > > sending automatically a STOP when a NACK is generated. This can > > be especially seen when the processing get slower (ex: enabling lots > > of debug messages), ending up send 2 STOP (one automatically by the > > controller and a 2nd one at the end of the stm32_i2c_message_xfer > > function). > > um I debugged this - took me a couple of days - and I proposed a fix that has just > been massaged a little in this PR. > > IMO the best thing to do is either adding me - or to be fair yourself since you > invested almost zero time on it - as a co-author. The hard bit was to find the > issue in the first place. > > what do you think? I'm very sorry, it wasn't my intention at all but indeed I should have taken more care to highlight your work. Sorry about that. I'm going to push a v2 for the serie, highlighting that and putting your Signed-of first. > > I'll try to find some time to test this. > > > > > Signed-off-by: Alain Volmat <alain.volmat@foss.st.com> > > --- > > drivers/i2c/stm32f7_i2c.c | 8 ++++---- > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/i2c/stm32f7_i2c.c b/drivers/i2c/stm32f7_i2c.c > > index 0ec67b5c12..8803979d3e 100644 > > --- a/drivers/i2c/stm32f7_i2c.c > > +++ b/drivers/i2c/stm32f7_i2c.c > > @@ -477,16 +477,16 @@ static int stm32_i2c_message_xfer(struct stm32_i2c_priv *i2c_priv, > > if (ret) > > break; > > > > + /* End of transfer, send stop condition */ > > + mask = STM32_I2C_CR2_STOP; > > + setbits_le32(®s->cr2, mask); > > + > > if (!stop) > > /* Message sent, new message has to be sent */ > > return 0; > > } > > } > > > > - /* End of transfer, send stop condition */ > > - mask = STM32_I2C_CR2_STOP; > > - setbits_le32(®s->cr2, mask); > > - > > return stm32_i2c_check_end_of_message(i2c_priv); > > } > > > > -- > > 2.25.1 > >
diff --git a/drivers/i2c/stm32f7_i2c.c b/drivers/i2c/stm32f7_i2c.c index 0ec67b5c12..8803979d3e 100644 --- a/drivers/i2c/stm32f7_i2c.c +++ b/drivers/i2c/stm32f7_i2c.c @@ -477,16 +477,16 @@ static int stm32_i2c_message_xfer(struct stm32_i2c_priv *i2c_priv, if (ret) break; + /* End of transfer, send stop condition */ + mask = STM32_I2C_CR2_STOP; + setbits_le32(®s->cr2, mask); + if (!stop) /* Message sent, new message has to be sent */ return 0; } } - /* End of transfer, send stop condition */ - mask = STM32_I2C_CR2_STOP; - setbits_le32(®s->cr2, mask); - return stm32_i2c_check_end_of_message(i2c_priv); }
Current function stm32_i2c_message_xfer is sending a STOP whatever the result of the transaction is. This can cause issues such as making the bus busy since the controller itself is already sending automatically a STOP when a NACK is generated. This can be especially seen when the processing get slower (ex: enabling lots of debug messages), ending up send 2 STOP (one automatically by the controller and a 2nd one at the end of the stm32_i2c_message_xfer function). Signed-off-by: Alain Volmat <alain.volmat@foss.st.com> --- drivers/i2c/stm32f7_i2c.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)