Re: [PATCH] i2c: omap: fix draining irq handling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Mon, Jan 21, 2013 at 08:01:59PM +0200, Aaro Koskinen wrote:
> Hi,
> 
> On Mon, Jan 21, 2013 at 10:44:31AM +0200, Felipe Balbi wrote:
> > On Sun, Jan 20, 2013 at 08:37:02PM +0200, Aaro Koskinen wrote:
> > > Commit 0bdfe0cb803dce699ff337c35d8e97ac355fa417 (i2c: omap: sanitize
> > > exit path) changed the interrupt handler to exit early and complete
> > > the transfer after the draining IRQ is handled. As a result, the ARDY
> > > may not be cleared properly, and it may cause all future I2C transfers
> > > to timeout with "timeout waiting for bus ready". This is reproducible
> > > at least with N900 when twl4030_gpio makes a long write (> FIFO size)
> > > during the probe (http://marc.info/?l=linux-omap&m=135818882610432&w=2).
> > > 
> > > The fix is to continue until we get ARDY interrupt that completes the
> > > transfer. Tested with 3.8-rc4 + N900: 20 boots in a row without errors;
> > > without the patch the problem triggers after few reboots.
> > 
> > fair enough, but ...
> > 
> > > Signed-off-by: Aaro Koskinen <aaro.koskinen@xxxxxx>
> > > ---
> > >  drivers/i2c/busses/i2c-omap.c |    4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
> > > index 832f16e..4cc2f05 100644
> > > --- a/drivers/i2c/busses/i2c-omap.c
> > > +++ b/drivers/i2c/busses/i2c-omap.c
> > > @@ -963,7 +963,7 @@ omap_i2c_isr_thread(int this_irq, void *dev_id)
> > >  				i2c_omap_errata_i207(dev, stat);
> > >  
> > >  			omap_i2c_ack_stat(dev, OMAP_I2C_STAT_RDR);
> > > -			break;
> > > +			continue;
> > 
> > ... when we get draining Interrupt, it means we're receiving the last
> > few bytes, meaning that by the time we handle it there's nothing pending
> > to be transferred.
> > 
> > I would rather wait for ARDY before exiting omap_i2c_xfer_msg() rather
> > than here. Specially since that was the programming model suggested by
> > our IP engineers. Such handling is part of my other patchset and can be
> > easily ported.
> > 
> > OTOH, we might decide to go with this during this -rc and fix it all up
> > for v3.9 on my other patchset.
> 
> Completing through ARDY interrupt has been known to work reliably for
> years. So for 3.8-rc I would prefer to just restore the old behaviour.

fair enough, I'll rebase my patchset on top of $SUBJECT

-- 
balbi

Attachment: signature.asc
Description: Digital signature


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux