[PATCH v3 3/3] i2c: stm32: do not set the STOP condition on error

[Patrick DELAUNAY]

Hi Jorge,

On 9/12/22 10:35, Jorge Ramirez-Ortiz, Foundries wrote:
> On 12/09/22, Patrick DELAUNAY wrote:
>> Hi Alain,
>>
>> On 9/9/22 18:06, 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.
>>>
>>> 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 | 6 +++---
>>>    1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/i2c/stm32f7_i2c.c b/drivers/i2c/stm32f7_i2c.c
>>> index 0ec67b5c12..2db7f44d44 100644
>>> --- a/drivers/i2c/stm32f7_i2c.c
>>> +++ b/drivers/i2c/stm32f7_i2c.c
>>> @@ -483,9 +483,9 @@ static int stm32_i2c_message_xfer(struct stm32_i2c_priv *i2c_priv,
>>>    		}
>>>    	}
>>> -	/* End of transfer, send stop condition */
>>> -	mask = STM32_I2C_CR2_STOP;
>>> -	setbits_le32(&regs->cr2, mask);
>>> +	/* End of transfer, send stop condition if appropriate */
>>> +	if (!ret && !(status & (STM32_I2C_ISR_NACKF | STM32_I2C_ISR_ERRORS)))
>>> +		setbits_le32(&regs->cr2, STM32_I2C_CR2_STOP);
>>>    	return stm32_i2c_check_end_of_message(i2c_priv);
>>>    }
>>
>>
>> Reviewed-by: Patrick Delaunay <patrick.delaunay at foss.st.com>
>> Tested-by: Patrick Delaunay <patrick.delaunay at foss.st.com> [stm32mp157c-dk2]
>>
>> No regression detection on ST Microelectonics board.
>>
>> - No error trace on boot
>>
>> - I2C probe command is OK
>>
>>    STM32MP> i2c probe
>>    Valid chip addresses: 28 33
>>
>> - And other tests done with the 2 I2C devices STPMIC1 & STUSB1600 are ok:
>>    regulalor command
>>
>>    pmic status command
>>
>>    USB type C connection/deconnection
>>
>>
>> @Jorge: can you test also for your use-case, thanks
> yes I did test a few hours ago and it is good on my end.
> can add my tested tag if needed
>
> Tested-by: Jorge Ramirez-Ortiz <jorge at foundries.io>


Thanks for the test

but sorry I don't see your message when I made my pull request for 
v2022.10-rc5

so I don't add your tag "Tested-by" when I merge this commit in serie V4.


> btw I also sent a patch to fix the way some dts properties are handled.
> shall I submit separately or will it be includeed in this set?

Yes pushed by Alain in V4 = 
http://patchwork.ozlabs.org/project/uboot/list/?series=317940&state=*

See[v4,4/4] i2c: stm32: fix usage of rise/fall device tree properties

http://patchwork.ozlabs.org/project/uboot/patch/20220912084201.1826979-5-alain.volmat@foss.st.com/

it is part of my last pull request = u-boot-stm32-20220915

it is now merged in master branch and part of the next v2022.10-rc5

Thanks
Patrick