| Message ID | 20231121180855.1278717-1-robert.hancock@calian.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Xilinx I2C driver fixes | expand |
On Tue, Nov 21, 2023 at 11:42 PM Robert Hancock <robert.hancock@calian.com> wrote: > > A couple of fixes for the Xilinx I2C driver. Thanks for the fix is there a way i can reproduce the issue reported here. > > Changed since v1: > -Fixed an issue in first patch where an additional message could still have > been written to the TX FIFO without waiting for it to empty. > > Robert Hancock (2): > i2c: xiic: Wait for TX empty to avoid missed TX NAKs > i2c: xiic: Try re-initialization on bus busy timeout > > drivers/i2c/busses/i2c-xiic.c | 61 +++++++++++++++++++++-------------- > 1 file changed, 36 insertions(+), 25 deletions(-) > > -- > 2.42.0 > >
On Thu, 2023-11-23 at 11:23 +0530, Shubhrajyoti Datta wrote: > CAUTION: This email originated from outside of the organization. Do > not click links or open attachments unless you recognize the sender > and know the content is safe. > > On Tue, Nov 21, 2023 at 11:42 PM Robert Hancock > <robert.hancock@calian.com> wrote: > > > > A couple of fixes for the Xilinx I2C driver. > > Thanks for the fix is there a way i can reproduce the issue reported > here. For the missed TX NAK problem, we were seeing that with a Xilinx MPSoC PL I2C controller and an Infineon IRPS5401 PMIC, using the irps5401 kernel driver. However in this application the PMIC is located on a separate board and the I2C communication is over a cable using TI P82B96DR cable extender devices. In some cases when the driver would try to read a register that's not supported by the device during probing, the device would NAK a register address write but the controller would go ahead with reading the value anyway. This caused the PMIC to flag a "CML" fault which the driver initialization falsely interpreted as meaning a subsequent register read was unsupported. The visible symptom was that some of the configured min/max thresholds on the PMIC would randomly be missing from the output of the "sensors" command. We haven't seen similar symptoms on more typical setups with the FPGA and PMIC on the same board. It's possible that the presence of the cable drivers changes the I2C timing sufficiently to make the issue manifest. I think the root issue is that the behavior of the IP isn't really documented/defined in the case where a write command receives a NAK but additional dynamic start/stop commands have already been queued in the TX FIFO, so it is better to avoid that case. For the second issue with the bus busy timeout, it can be likely reproduced by manually pulling the SDA and SCL lines on the I2C bus to ground and then initiating a request. Depending on the order in which this is done, the controller can see a "start" operation with no "stop" which causes it to think the bus is busy even though SDA and SCL lines are currently high. > > > > > Changed since v1: > > -Fixed an issue in first patch where an additional message could > > still have > > been written to the TX FIFO without waiting for it to empty. > > > > Robert Hancock (2): > > i2c: xiic: Wait for TX empty to avoid missed TX NAKs > > i2c: xiic: Try re-initialization on bus busy timeout > > > > drivers/i2c/busses/i2c-xiic.c | 61 +++++++++++++++++++++---------- > > ---- > > 1 file changed, 36 insertions(+), 25 deletions(-) > > > > -- > > 2.42.0 > > > >
Hi, On Tue, Nov 21, 2023 at 06:10:52PM +0000, Robert Hancock wrote: > A couple of fixes for the Xilinx I2C driver. > > Changed since v1: > -Fixed an issue in first patch where an additional message could still have > been written to the TX FIFO without waiting for it to empty. > > Robert Hancock (2): > i2c: xiic: Wait for TX empty to avoid missed TX NAKs > i2c: xiic: Try re-initialization on bus busy timeout Michal, any chance to take a look here? Andi
> -----Original Message----- > From: Robert Hancock <robert.hancock@calian.com> > Sent: Tuesday, November 21, 2023 11:41 PM > To: michal.simek@amd.com; ben-linux@fluff.org; andi.shyti@kernel.org > Cc: linux-arm-kernel@lists.infradead.org; linux-i2c@vger.kernel.org; Robert > Hancock <robert.hancock@calian.com> > Subject: [PATCH v2 0/2] Xilinx I2C driver fixes > > A couple of fixes for the Xilinx I2C driver. > > Changed since v1: > -Fixed an issue in first patch where an additional message could still have > been written to the TX FIFO without waiting for it to empty. > > Robert Hancock (2): > i2c: xiic: Wait for TX empty to avoid missed TX NAKs > i2c: xiic: Try re-initialization on bus busy timeout > > drivers/i2c/busses/i2c-xiic.c | 61 +++++++++++++++++++++-------------- > 1 file changed, 36 insertions(+), 25 deletions(-) > > -- > 2.42.0 > Reviewed-by: Manikanta Guntupalli <manikanta.guntupalli@amd.com> Thanks, Manikanta.
On 3/1/24 10:19, Guntupalli, Manikanta wrote: > >> -----Original Message----- >> From: Robert Hancock <robert.hancock@calian.com> >> Sent: Tuesday, November 21, 2023 11:41 PM >> To: michal.simek@amd.com; ben-linux@fluff.org; andi.shyti@kernel.org >> Cc: linux-arm-kernel@lists.infradead.org; linux-i2c@vger.kernel.org; Robert >> Hancock <robert.hancock@calian.com> >> Subject: [PATCH v2 0/2] Xilinx I2C driver fixes >> >> A couple of fixes for the Xilinx I2C driver. >> >> Changed since v1: >> -Fixed an issue in first patch where an additional message could still have >> been written to the TX FIFO without waiting for it to empty. >> >> Robert Hancock (2): >> i2c: xiic: Wait for TX empty to avoid missed TX NAKs >> i2c: xiic: Try re-initialization on bus busy timeout >> >> drivers/i2c/busses/i2c-xiic.c | 61 +++++++++++++++++++++-------------- >> 1 file changed, 36 insertions(+), 25 deletions(-) >> >> -- >> 2.42.0 >> > Reviewed-by: Manikanta Guntupalli <manikanta.guntupalli@amd.com> Acked-by: Michal Simek <michal.simek@amd.com> Thanks, Michal
Hi Andi, On 11/21/23 19:10, Robert Hancock wrote: > A couple of fixes for the Xilinx I2C driver. > > Changed since v1: > -Fixed an issue in first patch where an additional message could still have > been written to the TX FIFO without waiting for it to empty. > > Robert Hancock (2): > i2c: xiic: Wait for TX empty to avoid missed TX NAKs > i2c: xiic: Try re-initialization on bus busy timeout > > drivers/i2c/busses/i2c-xiic.c | 61 +++++++++++++++++++++-------------- > 1 file changed, 36 insertions(+), 25 deletions(-) > Can you please take a look at these two patches? It looks like they have been missed. Thanks, Michal
Hi Andi, Could you please review the patch series that has been awaiting feedback for some time? we believe it deserves your attention. Thank you for your time and consideration. > -----Original Message----- > From: Simek, Michal <michal.simek@amd.com> > Sent: Friday, March 1, 2024 2:53 PM > To: Guntupalli, Manikanta <manikanta.guntupalli@amd.com>; Robert Hancock > <robert.hancock@calian.com>; ben-linux@fluff.org; andi.shyti@kernel.org > Cc: linux-arm-kernel@lists.infradead.org; linux-i2c@vger.kernel.org > Subject: Re: [PATCH v2 0/2] Xilinx I2C driver fixes > > > > On 3/1/24 10:19, Guntupalli, Manikanta wrote: > > > >> -----Original Message----- > >> From: Robert Hancock <robert.hancock@calian.com> > >> Sent: Tuesday, November 21, 2023 11:41 PM > >> To: michal.simek@amd.com; ben-linux@fluff.org; andi.shyti@kernel.org > >> Cc: linux-arm-kernel@lists.infradead.org; linux-i2c@vger.kernel.org; > >> Robert Hancock <robert.hancock@calian.com> > >> Subject: [PATCH v2 0/2] Xilinx I2C driver fixes > >> > >> A couple of fixes for the Xilinx I2C driver. > >> > >> Changed since v1: > >> -Fixed an issue in first patch where an additional message could > >> still have been written to the TX FIFO without waiting for it to empty. > >> > >> Robert Hancock (2): > >> i2c: xiic: Wait for TX empty to avoid missed TX NAKs > >> i2c: xiic: Try re-initialization on bus busy timeout > >> > >> drivers/i2c/busses/i2c-xiic.c | 61 +++++++++++++++++++++-------------- > >> 1 file changed, 36 insertions(+), 25 deletions(-) > >> > >> -- > >> 2.42.0 > >> > > Reviewed-by: Manikanta Guntupalli <manikanta.guntupalli@amd.com> > > Acked-by: Michal Simek <michal.simek@amd.com> > Thanks, Manikanta.
Hi Manikanta, On Wed, Sep 11, 2024 at 11:42:09AM GMT, Guntupalli, Manikanta wrote: > Could you please review the patch series that has been awaiting feedback for some time? we believe it deserves your attention. yes, I'm sorry this skipped from my review list. Thanks for bringing it up, I will check it. > Thank you for your time and consideration. Thank you! Andi
Hi, On Tue, Nov 21, 2023 at 06:10:52PM GMT, Robert Hancock wrote: > A couple of fixes for the Xilinx I2C driver. > > Changed since v1: > -Fixed an issue in first patch where an additional message could still have > been written to the TX FIFO without waiting for it to empty. > > Robert Hancock (2): > i2c: xiic: Wait for TX empty to avoid missed TX NAKs > i2c: xiic: Try re-initialization on bus busy timeout merged to i2c/i2c-host-fixes. I just added in the tag section: Cc: <stable@vger.kernel.org> # v2.6.34+ As requested by the stable kernel rules. Thanks Manikanta for the reminder. Andi