Messages in this thread | | | Date | Sun, 14 Jun 2020 17:12:47 +0200 | From | Krzysztof Kozlowski <> | Subject | Re: [PATCH 2/2] spi: spi-fsl-dspi: Initialize completion before possible interrupt |
| |
On Sun, Jun 14, 2020 at 04:43:28PM +0300, Vladimir Oltean wrote: > On Sun, 14 Jun 2020 at 16:39, Vladimir Oltean <olteanv@gmail.com> wrote: > > > > On Sun, 14 Jun 2020 at 14:18, Krzysztof Kozlowski <krzk@kernel.org> wrote: > > > > > > On Sun, Jun 14, 2020 at 02:14:15PM +0300, Vladimir Oltean wrote: > > > > On Sun, 14 Jun 2020 at 13:56, Krzysztof Kozlowski <krzk@kernel.org> wrote: > > > > > > > > > > If interrupt fires early, the dspi_interrupt() could complete > > > > > (dspi->xfer_done) before its initialization happens. > > > > > > > > > > Fixes: 4f5ee75ea171 ("spi: spi-fsl-dspi: Replace interruptible wait queue with a simple completion") > > Also please note that this patch merely replaced an > init_waitqueue_head with init_completion. But the "bug" (if we can > call it that) originates from even before.
Yeah, I know, the Fixes is not accurate. Backport to earlier kernels would be manual so I am not sure if accurate Fixes matter.
> > > > > > Cc: <stable@vger.kernel.org> > > > > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> > > > > > --- > > > > > > > > Why would an interrupt fire before spi_register_controller, therefore > > > > before dspi_transfer_one_message could get called? > > > > Is this master or slave mode? > > > > > > I guess practically it won't fire. It's more of a matter of logical > > > order and: > > > 1. Someone might fix the CONFIG_DEBUG_SHIRQ_FIXME one day, > > > > And what if CONFIG_DEBUG_SHIRQ_FIXME gets fixed? I uncommented it, and > > still no issues. dspi_interrupt checks the status bit of the hw, sees > > there's nothing to do, and returns IRQ_NONE.
Indeed, still the logical way of initializing is to do it before any possible use.
> > > > > 2. The hardware is actually initialized before and someone could attach > > > to SPI bus some weird device. > > > > > > > Some weird device that does what?
You never know what people will connect to a SoM :).
Wolfram made actually much better point - bootloaders are known to initialize some things and leaving them in whatever state, assuming that Linux kernel will redo any initialization properly.
Best regards, Krzysztof
| |