linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Marc Zyngier <maz@kernel.org>
To: Maxime Ripard <maxime@cerno.tech>
Cc: "Marc Dorval" <marc.dorval@silabs.com>,
	"Chen-Yu Tsai" <wens@csie.org>,
	"Thomas Gleixner" <tglx@linutronix.de>,
	"Jérôme Pouiller" <jerome.pouiller@silabs.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: Possible race while masking IRQ on Allwinner A20
Date: Thu, 21 May 2020 09:02:48 +0100	[thread overview]
Message-ID: <7b8772cbdb9ed907981b18a0ffbc7762@kernel.org> (raw)
In-Reply-To: <20200521072634.6ig7jcuy5tmvmojf@gilmour.lan>

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
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2020-05-21  8:02 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-19  8:59 Possible race while masking IRQ on Allwinner A20 Jérôme Pouiller
2020-05-21  7:26 ` Maxime Ripard
2020-05-21  8:02   ` Marc Zyngier [this message]
2020-05-21 13:28     ` Jérôme Pouiller
2020-05-21 13:39       ` Marc Zyngier
2020-05-21 14:08         ` Jérôme Pouiller
2020-05-21 13:12   ` Jérôme Pouiller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7b8772cbdb9ed907981b18a0ffbc7762@kernel.org \
    --to=maz@kernel.org \
    --cc=jerome.pouiller@silabs.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=marc.dorval@silabs.com \
    --cc=maxime@cerno.tech \
    --cc=tglx@linutronix.de \
    --cc=wens@csie.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).