On Fri, Mar 02, 2018 at 02:58:54PM +0100, Wolfram Sang wrote:
It needs platform maintainers to be motivated to fix it, and one way to provide that motivation is for subsystem maintainers to say no to patches like this. If patches like this get accepted, then the "problem" gets solved, and there is very little motivation to fix the platform itself.
Yes, I can see this. I will drop / revert the patch.
TBH, I can't find the threads from November so I feel a bit lost and there is no documentation for platform_get_irq().
regards, dan carpenter
On Fri, Mar 02, 2018 at 05:09:01PM +0300, Dan Carpenter wrote:
On Fri, Mar 02, 2018 at 02:58:54PM +0100, Wolfram Sang wrote:
It needs platform maintainers to be motivated to fix it, and one way to provide that motivation is for subsystem maintainers to say no to patches like this. If patches like this get accepted, then the "problem" gets solved, and there is very little motivation to fix the platform itself.
Yes, I can see this. I will drop / revert the patch.
TBH, I can't find the threads from November so I feel a bit lost and there is no documentation for platform_get_irq().
Start from here:
http://archive.armlinux.org.uk/lurker/search/20380101.000000.00000000@ml:lin...
With the right list archiving software with a built-in search facility, it becomes much easier to find stuff! There's quite a number of messages there though, as there were multiple patch series posted.
Some specific messages:
http://archive.armlinux.org.uk/lurker/message/20171204.182556.775e16ab.en.ht... http://archive.armlinux.org.uk/lurker/message/20171120.164840.87002f9c.en.ht... http://archive.armlinux.org.uk/lurker/message/20171118.182704.3e1a5553.en.ht...
The reason it hasn't be trivially done (just by changing platform_get_irq() now) is that doing so will cause a bunch of regressions, precisely because people _are_ using IRQ 0 with some platform drivers.
The patch series above has died a death, so yet again the IRQ0/NO_IRQ issue has disappeared off people's radars and there's no reason to fix the situation. So, we're yet again back to the status quo of almost nothing happening.
How do we break this status quo and finally solve the IRQ 0 and NO_IRQ issue?
I believe that we have to bite the bullet and start by saying no to these trivial patches which try to preserve the current situation. That at least provides some motivation for things to get fixed in the right way.
Another possibility would be to change platform_get_irq() and suffer the regressions that will cause, telling people that fixing their platform IRQ numbering is the only solution (but this requires breaking our ideals about regressions.)
The alternative is everyone (including Linus) stops whinging about NO_IRQ and IRQ0 and put up with the fact that some platforms treat IRQ0 as a valid interrupt - which, I think we can all agree, isn't going to happen.
On Fri, Mar 02, 2018 at 03:32:22PM +0000, Russell King - ARM Linux wrote:
How do we break this status quo and finally solve the IRQ 0 and NO_IRQ issue?
Another possibility would be to change platform_get_irq() and suffer the regressions that will cause, telling people that fixing their platform IRQ numbering is the only solution (but this requires breaking our ideals about regressions.)
How about we start with a warning? That'll be visible, but shouldn't result in broken systems while we wait for people to fix things up.
e.g. something like the below.
Mark.
---->8---- diff --git a/drivers/base/platform.c b/drivers/base/platform.c index f1bf7b38d91c..bd42eeffd2aa 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -126,7 +126,12 @@ int platform_get_irq(struct platform_device *dev, unsigned int num) irqd_set_trigger_type(irqd, r->flags & IORESOURCE_BITS); }
- return r ? r->start : -ENXIO; + if (!r) + return -ENXIO; + + WARN_ONCE(!r->start, "Platform uses zero as a valid IRQ."); + + return r->start; #endif } EXPORT_SYMBOL_GPL(platform_get_irq);
On Fri, Mar 2, 2018 at 6:28 PM, Mark Rutland mark.rutland@arm.com wrote:
On Fri, Mar 02, 2018 at 03:32:22PM +0000, Russell King - ARM Linux wrote:
How do we break this status quo and finally solve the IRQ 0 and NO_IRQ issue?
Guys, the question: Wouldn't be request_irq() failed when it gets a wrong number on input?
On Sat, Mar 03, 2018 at 06:25:17PM +0200, Andy Shevchenko wrote:
On Fri, Mar 2, 2018 at 6:28 PM, Mark Rutland mark.rutland@arm.com wrote:
On Fri, Mar 02, 2018 at 03:32:22PM +0000, Russell King - ARM Linux wrote:
How do we break this status quo and finally solve the IRQ 0 and NO_IRQ issue?
Guys, the question: Wouldn't be request_irq() failed when it gets a wrong number on input?
Unfortunately not - IRQ 0 is kind of valid on x86 (it's the i8253 PIT) and an exception is made for x86 arch code with regard to this. It gets setup using setup_irq() rather than request_irq() (see arch/x86/kernel/time.c::setup_default_timer_irq()).
request_irq() doesn't deny IRQ 0 - it denies IRQ_NOTCONNECTED and anything that irq_to_desc() returns NULL for, which can be a radix tree lookup or simply any unsigned IRQ number less than NR_IRQS for legacy platforms.
If you're on a DT platform, then the IRQ subsystem avoids allocating IRQ0 for any DT IRQ controller, so DT platforms should be fine. It's just the legacy platforms that continue to be an ongoing issue wrt the IRQ 0 / NO_IRQ business, and those will generally be using the non-radix tree version of irq_to_desc().
linux-stable-mirror@lists.linaro.org