i2c: s3c2410: Properly handle interrupts of number 0

Commit Message

Mark Rutland March 2, 2018, 4:28 p.m. UTC

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----

Comments

Andy Shevchenko March 3, 2018, 4:25 p.m. UTC | #1

On Fri, Mar 2, 2018 at 6:28 PM, Mark Rutland <[e-mail]> 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?

Russell King (Oracle) March 3, 2018, 6:36 p.m. UTC | #2

On Sat, Mar 03, 2018 at 06:25:17PM +0200, Andy Shevchenko wrote:
> On Fri, Mar 2, 2018 at 6:28 PM, Mark Rutland <[e-mail]> 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().

Patch

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);