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

[Jorge Ramirez-Ortiz, Foundries]

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>

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?


> 
> 
> Thanks
> Patrick
> 
>