Subject: Re: Possible race while masking IRQ on Allwinner A20

From: Marc Zyngier <[email-masked]>
Date: Thu, 21 May 2020 09:02:48 +0100

Hi Maxime, Jérôme,

On 2020-05-21 08:26, Maxime Ripard wrote:
> Hi!
> 
> Adding Thomas and Marc in Cc.
> 
> On Tue, May 19, 2020 at 10:59:26AM +0200, Jérôme Pouiller wrote:
>> I have some trouble with integration of the wfx driver[1] on Allwinner
>> A20 platform.
>> 
>> The chip WF200 is connected to the SDIO bus. At the beginning, I tried
>> to use the IRQ provided by the SDIO bus. However, I have noticed I
>> received some IRQs twice. Since the IRQ line is multiplexed with the
>> data line, it is not very clear if it is a bug, or if the SDIO device
>> has to support that.
>> 
>> The chip WF200 allows to use a dedicated line for the IRQ (aka
>> "Out-Of-Band" IRQ). So I have enabled this feature with a edge 
>> triggered
>> IRQ. However, I missed some IRQs. Indeed, it seems that Allwinner use 
>> a
>> 32KHz clock to sample the IRQs. It is not fast enough for us. I think 
>> it
>> explains why we miss some IRQs (using the attribute 
>> "input-debounce"[2],
>> I tried to enable the 24Mhz clock, but without success).
> 
> Without success as in you couldn't make it use the 24MHz clock, or 
> using it
> didn't change anything?
> 
> But yeah, missing an edge interrupt is bound to happen at some point, 
> and a
> level interrupt is going to be more reliable (especially if you can't 
> recover
> from a missed interrupt).
> 
>> Nevermind, I tried to use a level triggered IRQ (and my request is on
>> this part). As you can see in the wfx driver (in  bus_sdio.c and 
>> bh.c),
>> I use a threaded IRQ for that. Unfortunately, I receive some IRQs 
>> twice.
>> I traced the problem, I get:
>> 
>>  QSGRenderThread-981   [000] d.h.   247.485524: irq_handler_entry: 
>> irq=80 name=wfx
>>  QSGRenderThread-981   [000] d.h.   247.485547: irq_handler_exit: 
>> irq=80 ret=handled
>>  QSGRenderThread-981   [000] d.h.   247.485600: irq_handler_entry: 
>> irq=80 name=wfx
>>  QSGRenderThread-981   [000] d.h.   247.485606: irq_handler_exit: 
>> irq=80 ret=handled
>>       irq/80-wfx-260   [001] ....   247.485828: io_read32: CONTROL: 
>> 0000f046
>>       irq/80-wfx-260   [001] ....   247.486072: io_read32: CONTROL: 
>> 0000f046
>>     kworker/1:1H-116   [001] ....   247.486214: io_read: QUEUE: 8b 00 
>> 84 18 00 00 00 00 01 00 15 82 2b 48 01 1e 88 42 30 00 08 6b d7 c3 53 
>> e0 28 80 88 67 32 af ... (192 bytes)
>>     kworker/1:1H-116   [001] ....   247.493097: io_read: QUEUE: 00 00 
>> 00 00 00 00 00 00 06 06 00 6a 3f 95 00 60 00 00 00 00 08 62 00 00 01 
>> 00 5e 00 00 07 28 80 ... (192 bytes)
>>     [...]
>> 
>> On this trace, we can see:
>>   - the hard IRQ handler
>>   - the IRQ acknowledge from the thread irq/80-wfx-260
>>   - the access to the data from kworker/1:1H-116
>> 
>> As far as I understand, the first call to the IRQ handler (at
>> 247.485524) should mask the IRQ 80. So, the second IRQ (at 247.485600)
>> should not happen and the thread irq/80 should be triggered only once.
>> 
>> Do you have any idea of what is going wrong with this IRQ?
> 
> That's pretty weird indeed. My first guess was that you weren't using
> IRQF_ONESHOT, but it looks like you are. My next lead would be to see 
> if the
> mask / unmask hooks in the pinctrl driver are properly called (and 
> actually do
> what they are supposed to do). I'm not sure we have any in-tree user of 
> a
> threaded IRQ attached to the pinctrl driver, so it might have been 
> broken for
> quite some time.

What is certainly puzzling is that this driver doesn't seem to use
threaded IRQs at all. Instead, it uses its own workqueue that seems
to bypass the core IRQ subsystem altogether. So any guarantee we'd
expect goes at of the window.

It is also pretty unclear to me how whether the HW supports switch
from edge to level signalling. The request_irq() call definitely asks
for edge, and I don't know how you'd instruct the HW to change its
signalling method (in general, it isn't possible).

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...

_______________________________________________
linux-arm-kernel mailing list
[email-masked]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel