Messages in this thread | | | From | Dave Stevenson <> | Date | Tue, 31 Oct 2023 15:14:41 +0000 | Subject | Re: [PATCH 1/2] [i2c-bcm2835] Fully clean up hardware state machine after a timeout |
| |
Hi Mike & Andi
On Tue, 31 Oct 2023 at 11:44, Andi Shyti <andi.shyti@kernel.org> wrote: > > Hi Mike, > > > When the driver detects a timeout, there's no guarantee that the ISR > > would have fired. Thus after a timeout, it's the foreground that > > becomes responsible to reset the hardware state machine. The change > > here just duplicates what is already implemented in the ISR. > > Is this a fix? What failing here? > > Can we have a feedback from Florian, Ray or Scott here? > > ... > > > if (!time_left) { > > + /* Since we can't trust the ISR to have cleaned up, do the > > + * full cleanup here... */ > > Please use the > > /* > * comment > * comment > */ > > format > > > bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_C, > > BCM2835_I2C_C_CLEAR); > > + bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_S, BCM2835_I2C_S_CLKT | > > + BCM2835_I2C_S_ERR | BCM2835_I2C_S_DONE); > > I'm not sure this is really making any difference though. How > have you tested this? > > Have you tried reading those registers before and understand what > went wrong?
I guess we may have a race hazard here. The completion has timed out. The write to I2C_C will flush the FIFOs and mask the interrupts out, so if the transaction can complete at that point then the ISR won't handle clearing the status flags. As the flags are "write 1 to reset", they could still be set for the next transfer. It'd be down to the exact timing of hitting I2C_C_CLEAR (to clear the FIFOs and disable the interrupts) and when that terminates the transfer. Ensuring the status bits are cleared will do no harm, but I'm not convinced that there is an issue in the first place that needs fixing. Can you give any more detail of when you've seen this fail?
Dave
> Andi > > > _______________________________________________ > linux-rpi-kernel mailing list > linux-rpi-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-rpi-kernel
| |