archive mirror
 help / color / mirror / Atom feed
From: Daniel Mack <>
To: Maarten Brock <>
Subject: Re: [PATCH 4/4] sc16is7xx: Use threaded IRQ
Date: Mon, 18 May 2020 18:57:50 +0200	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

Hi Maarten,

On 5/18/20 1:14 PM, Maarten Brock wrote:
> On 2020-05-17 22:44, Daniel Mack wrote:

>>> Therefor I suggest to change IRQF_TRIGGER_FALLING to
>>> way the thread will be retriggered after IRQ_HANDLED is returned.
>> This doesn't work in my setup unfortunately, as the interrupt controller
>> is incapable of handling level IRQs.
> That sounds like a lousy interrupt controller to me. 

While that is true, there are many such controllers around.

> Summerizing:
> - After switching to a threaded IRQ, the trigger could be switched to
> IRQF_TRIGGER_LOW and with that interrupt sharing can be enabled for
> this device with IRQF_SHARED.

Yes, but we don't need that. As discussed, the UART driver can cope with
edge IRQs just fine.

> - Some (your) interrupt controllers do not support IRQF_TRIGGER_LOW.
> For those only IRQF_TRIGGER_FALLING can be used for this device and
> thus IRQF_SHARED cannot be used.

True. Interrupts cannot be shared for this device then. That's a fair
limitation, and it has always been like that.

> - The driver for your interrupt controller should be improved to support
> level IRQs.

It's a controller that sits behind another hardware bus itself, so
polling is expensive. If the controller would need to check for level
IRQs it would need to poll, and then we could as well just poll the UART
directly, that's just as good :)

But again - the UART driver works perfectly fine with edge IRQs as long
as the interrupt is not shared.

> This makes me wonder if it would be better to let the device tree specify
> the interrupt configuration.

There can be flags in the 2nd cell of the node, but their meaning is
specific to the controller. Hence the SPI/I2C layers don't pass that
information up.

What many drivers do is try with one setting, and if that fails because
the interrupt controller returns an error, they fall back to something
else. We could do the same here of course, but it'd be another patch on
top, as it's unrelated to the concrete change the patch we're commenting
on is bringing in.

So what I can add is logic that first tries with IRQF_LOW|IRQF_SHARED,
and if that fails, we fall back to IRQF_FALLING and retry. WDYT?


  reply	other threads:[~2020-05-18 16:57 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-08 14:37 [PATCH 0/4] sc16is7xx: Add IrDA mode and threaded IRQ Daniel Mack
2020-05-08 14:37 ` [PATCH 1/4] dt-bindings: sc16is7xx: Add flag to activate IrDA mode Daniel Mack
2020-05-18 18:08   ` Rob Herring
2020-05-18 18:41     ` Daniel Mack
2020-05-19 12:01       ` Maarten Brock
2020-05-08 14:37 ` [PATCH 2/4] " Daniel Mack
2020-05-08 14:37 ` [PATCH 3/4] sc16is7xx: Always use falling edge IRQ Daniel Mack
2020-05-09 12:41   ` Maarten Brock
2020-05-08 14:37 ` [PATCH 4/4] sc16is7xx: Use threaded IRQ Daniel Mack
2020-05-09 12:55   ` Maarten Brock
2020-05-17 20:44     ` Daniel Mack
2020-05-18 11:14       ` Maarten Brock
2020-05-18 16:57         ` Daniel Mack [this message]
2020-05-19 16:32           ` Maarten Brock
2020-05-19 17:37             ` Daniel Mack

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:

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

  git send-email \ \ \ \ \ \ \ \ \ \ \

* 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).