Hi Andi, >-----Original Message----- >From: Andi Shyti <andi.shyti@xxxxxxxxxx> >Sent: Wednesday, January 17, 2024 6:50 PM >To: Boddu, Sai Pavan <sai.pavan.boddu@xxxxxxx> >Cc: linux-kernel@xxxxxxxxxxxxxxx; linux-i2c@xxxxxxxxxxxxxxx; linux-arm- >kernel@xxxxxxxxxxxxxxxxxxx; Simek, Michal <michal.simek@xxxxxxx>; Lars- >Peter Clausen <lars@xxxxxxxxxx>; Wolfram Sang <wsa@xxxxxxxxxx> >Subject: Re: [PATCH] i2c: cadence: Avoid fifo clear after start > >Hi Sai, > >sorry, but I'm not really understanding the issue here. >On Fri, Jan 05, 2024 at 06:22:58PM +0530, Sai Pavan Boddu wrote: >> Driver unintentionally programs ctrl reg to clear fifo which is >> happening after start of transaction > >what does it mean "unintentionally"? [Boddu, Sai Pavan] I mean, the previous patch which introduced the issue, was unintentional. > >> this was not the case previously >> as it was read-modified-write. This issue breaks i2c reads on QEMU as >> i2c-read is done before guest starts programming ctrl register. > >this log can be improved. How about something like > >The driver unintentionally programs the control register to clear the FIFO, >which occurs after the start of the transaction. >Previously, this was not an issue as it involved read-modify-write operations. >However, this current issue disrupts I2C reads on QEMU, as the I2C read is >executed before the guest starts programming the control register. [Boddu, Sai Pavan] Looks good. I will mention as above. >> Fixes: ff0cf7bca6309 ("i2c: cadence: Remove unnecessary register >> reads") >> Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xxxxxxx> >> --- >> drivers/i2c/busses/i2c-cadence.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/drivers/i2c/busses/i2c-cadence.c >> b/drivers/i2c/busses/i2c-cadence.c >> index de3f58b60dce..6f7d753a8197 100644 >> --- a/drivers/i2c/busses/i2c-cadence.c >> +++ b/drivers/i2c/busses/i2c-cadence.c >> @@ -633,6 +633,7 @@ static void cdns_i2c_mrecv(struct cdns_i2c *id) >> >> if (hold_clear) { >> ctrl_reg &= ~CDNS_I2C_CR_HOLD; >> + ctrl_reg &= ~CDNS_I2C_CR_CLR_FIFO; > >I'm wondering whether the whole ctrl_reg should be reset after the first write. [Boddu, Sai Pavan] previous implementation of read-modify-write was good then ? Regards, Sai Pavan > >Andi > >> /* >> * In case of Xilinx Zynq SOC, clear the HOLD bit before transfer >size >> * register reaches '0'. This is an IP bug which causes transfer >> size >> -- >> 2.25.1 >>