| Message ID | 20210823214145.295104-1-marex@denx.de |
|---|---|
| Headers | show |
| Series | i2c: xiic: Fix broken locking | expand |
+ravi On 8/23/21 11:41 PM, Marek Vasut wrote: > Booting ZynqMP with XIIC I2C driver shows multitude of race conditions > in the XIIC driver. This is because locking is completely missing from > the driver, and there are odd corner cases where the hardware behaves > strangely. > > Most of these races could be triggered easily when booting on SMP > machines, like the ZynqMP which has up to 4 cores. It is sufficient > for the interrupt handler to run on another core than xiic_start_xfer > and the driver fails completely. > > This does not add support for long transfers, this only fixes the > driver to be usable at all instead of being completely broken. > > The V2 fixes a few remaining details which cropped up in deployment > over the last year or so, so I believe the result should be reasonably > well tested. > > Marek Vasut (6): > i2c: xiic: Fix broken locking on tx_msg > i2c: xiic: Drop broken interrupt handler > i2c: xiic: Defer xiic_wakeup() and __xiic_start_xfer() in > xiic_process() > i2c: xiic: Switch from waitqueue to completion > i2c: xiic: Only ever transfer single message > i2c: xiic: Fix RX IRQ busy check > > drivers/i2c/busses/i2c-xiic.c | 161 +++++++++++++++------------------- > 1 file changed, 69 insertions(+), 92 deletions(-) > > Cc: Michal Simek <michal.simek@xilinx.com> > Cc: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com> > Cc: Wolfram Sang <wsa@kernel.org> >
> -----Original Message----- > From: Michal Simek <michal.simek@xilinx.com> > Sent: Tuesday, August 24, 2021 12:29 PM > To: Marek Vasut <marex@denx.de>; linux-i2c@vger.kernel.org; Raviteja > Narayanam <rna@xilinx.com> > Cc: Michal Simek <michals@xilinx.com>; Shubhrajyoti Datta > <shubhraj@xilinx.com>; Wolfram Sang <wsa@kernel.org> > Subject: Re: [PATCH v2 0/6] i2c: xiic: Fix broken locking > > +ravi > > On 8/23/21 11:41 PM, Marek Vasut wrote: > > Booting ZynqMP with XIIC I2C driver shows multitude of race conditions > > in the XIIC driver. This is because locking is completely missing from > > the driver, and there are odd corner cases where the hardware behaves > > strangely. > > > > Most of these races could be triggered easily when booting on SMP > > machines, like the ZynqMP which has up to 4 cores. It is sufficient > > for the interrupt handler to run on another core than xiic_start_xfer > > and the driver fails completely. > > > > This does not add support for long transfers, this only fixes the > > driver to be usable at all instead of being completely broken. > > > > The V2 fixes a few remaining details which cropped up in deployment > > over the last year or so, so I believe the result should be reasonably > > well tested. Thanks a lot for the patches, Marek. I have tested these on our boards and they are working fine. I will rebase my patch series on top of this and send after rc1. > > > > Marek Vasut (6): > > i2c: xiic: Fix broken locking on tx_msg > > i2c: xiic: Drop broken interrupt handler > > i2c: xiic: Defer xiic_wakeup() and __xiic_start_xfer() in > > xiic_process() > > i2c: xiic: Switch from waitqueue to completion > > i2c: xiic: Only ever transfer single message > > i2c: xiic: Fix RX IRQ busy check > > > > drivers/i2c/busses/i2c-xiic.c | 161 > > +++++++++++++++------------------- > > 1 file changed, 69 insertions(+), 92 deletions(-) > > > > Cc: Michal Simek <michal.simek@xilinx.com> > > Cc: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com> > > Cc: Wolfram Sang <wsa@kernel.org> > >
On 8/27/21 10:31 AM, Raviteja Narayanam wrote: > > >> -----Original Message----- >> From: Michal Simek <michal.simek@xilinx.com> >> Sent: Tuesday, August 24, 2021 12:29 PM >> To: Marek Vasut <marex@denx.de>; linux-i2c@vger.kernel.org; Raviteja >> Narayanam <rna@xilinx.com> >> Cc: Michal Simek <michals@xilinx.com>; Shubhrajyoti Datta >> <shubhraj@xilinx.com>; Wolfram Sang <wsa@kernel.org> >> Subject: Re: [PATCH v2 0/6] i2c: xiic: Fix broken locking >> >> +ravi >> >> On 8/23/21 11:41 PM, Marek Vasut wrote: >>> Booting ZynqMP with XIIC I2C driver shows multitude of race conditions >>> in the XIIC driver. This is because locking is completely missing from >>> the driver, and there are odd corner cases where the hardware behaves >>> strangely. >>> >>> Most of these races could be triggered easily when booting on SMP >>> machines, like the ZynqMP which has up to 4 cores. It is sufficient >>> for the interrupt handler to run on another core than xiic_start_xfer >>> and the driver fails completely. >>> >>> This does not add support for long transfers, this only fixes the >>> driver to be usable at all instead of being completely broken. >>> >>> The V2 fixes a few remaining details which cropped up in deployment >>> over the last year or so, so I believe the result should be reasonably >>> well tested. > > Thanks a lot for the patches, Marek. > I have tested these on our boards and they are working fine. > > I will rebase my patch series on top of this and send after rc1. Wolfram: Can you please merge this series? Ravi's series will come on the top of this one. Acked-by: Michal Simek <michal.simek@xilinx.com> Thanks, Michal
On Mon, Aug 23, 2021 at 11:41:39PM +0200, Marek Vasut wrote: > Booting ZynqMP with XIIC I2C driver shows multitude of race conditions > in the XIIC driver. This is because locking is completely missing from > the driver, and there are odd corner cases where the hardware behaves > strangely. > > Most of these races could be triggered easily when booting on SMP > machines, like the ZynqMP which has up to 4 cores. It is sufficient > for the interrupt handler to run on another core than xiic_start_xfer > and the driver fails completely. > > This does not add support for long transfers, this only fixes the > driver to be usable at all instead of being completely broken. > > The V2 fixes a few remaining details which cropped up in deployment > over the last year or so, so I believe the result should be reasonably > well tested. > > Marek Vasut (6): > i2c: xiic: Fix broken locking on tx_msg > i2c: xiic: Drop broken interrupt handler > i2c: xiic: Defer xiic_wakeup() and __xiic_start_xfer() in > xiic_process() > i2c: xiic: Switch from waitqueue to completion > i2c: xiic: Only ever transfer single message > i2c: xiic: Fix RX IRQ busy check > Applied to for-next, thanks everyone!
On 9/14/21 12:29 PM, Wolfram Sang wrote: > On Mon, Aug 23, 2021 at 11:41:39PM +0200, Marek Vasut wrote: >> Booting ZynqMP with XIIC I2C driver shows multitude of race conditions >> in the XIIC driver. This is because locking is completely missing from >> the driver, and there are odd corner cases where the hardware behaves >> strangely. >> >> Most of these races could be triggered easily when booting on SMP >> machines, like the ZynqMP which has up to 4 cores. It is sufficient >> for the interrupt handler to run on another core than xiic_start_xfer >> and the driver fails completely. >> >> This does not add support for long transfers, this only fixes the >> driver to be usable at all instead of being completely broken. >> >> The V2 fixes a few remaining details which cropped up in deployment >> over the last year or so, so I believe the result should be reasonably >> well tested. >> >> Marek Vasut (6): >> i2c: xiic: Fix broken locking on tx_msg >> i2c: xiic: Drop broken interrupt handler >> i2c: xiic: Defer xiic_wakeup() and __xiic_start_xfer() in >> xiic_process() >> i2c: xiic: Switch from waitqueue to completion >> i2c: xiic: Only ever transfer single message >> i2c: xiic: Fix RX IRQ busy check >> > > Applied to for-next, thanks everyone! > Thx. FYI: Ravi will be sending his series on the top of this one. Thanks, Michal
Booting ZynqMP with XIIC I2C driver shows multitude of race conditions in the XIIC driver. This is because locking is completely missing from the driver, and there are odd corner cases where the hardware behaves strangely. Most of these races could be triggered easily when booting on SMP machines, like the ZynqMP which has up to 4 cores. It is sufficient for the interrupt handler to run on another core than xiic_start_xfer and the driver fails completely. This does not add support for long transfers, this only fixes the driver to be usable at all instead of being completely broken. The V2 fixes a few remaining details which cropped up in deployment over the last year or so, so I believe the result should be reasonably well tested. Marek Vasut (6): i2c: xiic: Fix broken locking on tx_msg i2c: xiic: Drop broken interrupt handler i2c: xiic: Defer xiic_wakeup() and __xiic_start_xfer() in xiic_process() i2c: xiic: Switch from waitqueue to completion i2c: xiic: Only ever transfer single message i2c: xiic: Fix RX IRQ busy check drivers/i2c/busses/i2c-xiic.c | 161 +++++++++++++++------------------- 1 file changed, 69 insertions(+), 92 deletions(-) Cc: Michal Simek <michal.simek@xilinx.com> Cc: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com> Cc: Wolfram Sang <wsa@kernel.org>