| Message ID | 20240105125258.2470397-1-sai.pavan.boddu@amd.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | i2c: cadence: Avoid fifo clear after start | expand |
On 1/5/24 13:52, Sai Pavan Boddu wrote: > Driver unintentionally programs ctrl reg to clear fifo which is > happening after start of transaction, 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. > > Fixes: ff0cf7bca6309 ("i2c: cadence: Remove unnecessary register reads") > Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@amd.com> > --- > 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; > /* > * 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 Acked-by: Michal Simek <michal.simek@amd.com> Thanks, Michal
Hi Andi, Sorry, I did not close on this one. Anyway I will re-spin fixing the commit message issues. More comments inline below. >-----Original Message----- >From: Andi Shyti <andi.shyti@kernel.org> >Sent: Thursday, January 18, 2024 2:36 AM >To: Boddu, Sai Pavan <sai.pavan.boddu@amd.com> >Cc: linux-kernel@vger.kernel.org; linux-i2c@vger.kernel.org; linux-arm- >kernel@lists.infradead.org; Simek, Michal <michal.simek@amd.com>; Lars- >Peter Clausen <lars@metafoo.de>; Wolfram Sang <wsa@kernel.org> >Subject: Re: [PATCH] i2c: cadence: Avoid fifo clear after start > >Hi, > >> >> 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 ? > >I don't know, I'm just asking... because rather than read-modify-write, this is >read-modify-write-modify-write :-) > >I'm just wondering if after the first write ctrl_reg is still holding a valid value. [Boddu, Sai Pavan] Yes, all bits in ctrl_reg stay as configured except CLR_FIFO which is self-clearing. None of the other bits would change state. CLR_FIFO post start of transactions should not be allowed; this patch address the same. Regards, Sai Pavan > >Andi
Hi Sai Pavan, > Sorry, I did not close on this one. > Anyway I will re-spin fixing the commit message issues. More comments inline below. Thanks! :-) Andi
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; /* * 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
Driver unintentionally programs ctrl reg to clear fifo which is happening after start of transaction, 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. Fixes: ff0cf7bca6309 ("i2c: cadence: Remove unnecessary register reads") Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@amd.com> --- drivers/i2c/busses/i2c-cadence.c | 1 + 1 file changed, 1 insertion(+)