On 09/09/22, Alain Volmat wrote: > Hi Patrick > > On Fri, Sep 09, 2022 at 02:53:23PM +0200, Patrick DELAUNAY wrote: > > Hi Alain > > > > On 9/8/22 12:59, 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). > > > > > > Thanks to Jorge Ramirez-Ortiz for diagnosing and proposing a first > > > fix for this. [1] > > > > > > [1] > > > https://lore.kernel.org/u-boot/[email protected]/ > > > > > > Reported-by: Jorge Ramirez-Ortiz, Foundries <[email protected]> > > > Signed-off-by: Jorge Ramirez-Ortiz <[email protected]> > > > Signed-off-by: Alain Volmat <[email protected]> > > > --- > > > 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); > > > } > > > > > > Boot on DK2 failed with the traces: > > Ouch, I am very sorry about that. I think I might have made a mistake > during testing / removing debug traces, leading to this mistake ;-( > Very sorry about that, thanks a lot Patrick for the test. >
Just did a quick verification on my end and at least for my use case it is all good. My use case is enabling SCP03 on the NXP SE05x using the U-boot i2c driver from OP-TEE via the trampoline. Also, xould you mind also including the fix below in your series Alain? I think it is better if you send them all together. many thanks for steping up for these fixes Jorge Author: Jorge Ramirez-Ortiz <[email protected]> Date: 3 minutes ago i2c: stm32: fix usage of rise/fall device tree properties These two device tree properties were not being applied. Signed-off-by: Jorge Ramirez-Ortiz <[email protected]> diff --git a/drivers/i2c/stm32f7_i2c.c b/drivers/i2c/stm32f7_i2c.c index 505d27afe8..5231055be0 100644 --- a/drivers/i2c/stm32f7_i2c.c +++ b/drivers/i2c/stm32f7_i2c.c @@ -910,17 +910,18 @@ static int stm32_of_to_plat(struct udevice *dev) { const struct stm32_i2c_data *data; struct stm32_i2c_priv *i2c_priv = dev_get_priv(dev); - u32 rise_time, fall_time; int ret; data = (const struct stm32_i2c_data *)dev_get_driver_data(dev); if (!data) return -EINVAL; - rise_time = dev_read_u32_default(dev, "i2c-scl-rising-time-ns", + i2c_priv->setup.rise_time = dev_read_u32_default(dev, + "i2c-scl-rising-time-ns", STM32_I2C_RISE_TIME_DEFAULT); - fall_time = dev_read_u32_default(dev, "i2c-scl-falling-time-ns", + i2c_priv->setup.fall_time = dev_read_u32_default(dev, + "i2c-scl-falling-time-ns", STM32_I2C_FALL_TIME_DEFAULT); i2c_priv->dnf_dt = dev_read_u32_default(dev, "i2c-digital-filter-width-ns", 0);
