On 13.01.25 14:37, Alexander Shiyan wrote: > Hello. > > As far as I understand, AT91_TWI_TXCOMP is set at the same time as > one of the AT91_TWI_RXRDY or AT91_TWI_TXRDY signals, so we need > to check these bits first to perform the read or write operation of > the next byte. > If it was the last byte (when the STOP bit is transmitted) we exit. If the case AT91_TWI_TXCOMP on its own can't happen, then this is fine. Acked-by: Ahmad Fatoum <a.fatoum@xxxxxxxxxxxxxx> Thanks, Ahmad > > Thanks! > > пн, 13 янв. 2025 г. в 16:17, Ahmad Fatoum <a.fatoum@xxxxxxxxxxxxxx>: >> >> On 13.01.25 08:02, Alexander Shiyan wrote: >>> Most bits of the AT91 SR status register are cleared after reading, >>> so we may lose the status at the end of the wait loop. >>> Let's use the value already readed to check for "completed" status. >>> >>> Signed-off-by: Alexander Shiyan <eagle.alexander923@xxxxxxxxx> >>> --- >>> drivers/i2c/busses/i2c-at91.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c >>> index dbc5aa4ddf..8929dbaede 100644 >>> --- a/drivers/i2c/busses/i2c-at91.c >>> +++ b/drivers/i2c/busses/i2c-at91.c >>> @@ -276,7 +276,7 @@ static int at91_twi_wait_completion(struct at91_twi_dev *dev) >>> >>> dev->transfer_status |= status; >>> >>> - } while (!(at91_twi_read(dev, AT91_TWI_SR) & AT91_TWI_TXCOMP)); >>> + } while (!(status & AT91_TWI_TXCOMP)); >> >> This makes the function harder to reason about, because the status read >> is done at the start, but only acted upon at the very end. >> >> Would it make sense to change this to a while loop or move it into the >> loop directly after the other status read and early exit if it's true? >> >>> >>> at91_disable_twi_interrupts(dev); >>> >> >> >> -- >> Pengutronix e.K. | | >> Steuerwalder Str. 21 | http://www.pengutronix.de/ | >> 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | >> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | > -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |