Re: [PATCH v2 3/3] i2c: stm32: only send a STOP upon transfer completion

Hi Jorge

On Sun, Sep 11, 2022 at 08:57:17PM +0200, Jorge Ramirez-Ortiz, Foundries wrote:

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/20220815145211.31342-2-jorge@foundries.io/

Reported-by: Jorge Ramirez-Ortiz, Foundries jorge@foundries.io Signed-off-by: Jorge Ramirez-Ortiz jorge@foundries.io 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(&regs->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(&regs->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.

Sure, just added it in my tree and will put the serie v4 in few seconds. Thanks a lot for the patch.

Alain

many thanks for steping up for these fixes

Jorge

Author: Jorge Ramirez-Ortiz jorge@foundries.io 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 <jorge@foundries.io>

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