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

[Heiko Schocher]

Hello Jorge,

On 09.09.22 10:30, Jorge Ramirez-Ortiz, Foundries wrote:
> On 08/09/22, Patrick DELAUNAY wrote:
>> Hi,
>>
>> 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).
>>>
> 
> Cmon no need to thank me, that is kind of excessive :)
> Just the sign-off or codevelop tags for reference
> 
> 
> if you can wait before merging I will test the series before monday

I would love to see a test before we merge this.

@Patrick: feel free to merge it through stm32 repo.

Thanks!

bye,
Heiko
> 
> thanks
> Jorge
> 
>>> 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 at foundries.io>
>>> Signed-off-by: Jorge Ramirez-Ortiz <jorge at foundries.io>
>>> Signed-off-by: Alain Volmat <alain.volmat at 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);
>>>   }
>>
>>
>> Reviewed-by: Patrick Delaunay <patrick.delaunay at foss.st.com>
>>
>> Thanks
>> Patrick
>>

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs at denx.de