Messages in this thread | | | Date | Fri, 20 Feb 2015 07:40:38 -0800 | From | Greg Kroah-Hartman <> | Subject | Re: [PATCH] drivers: platform: parse IRQ flags from resources |
| |
On Wed, Feb 18, 2015 at 05:12:18PM +0100, Linus Walleij wrote: > This fixes a regression from the net subsystem: > After commit d52fdbb735c36a209f36a628d40ca9185b349ba7 > "smc91x: retrieve IRQ and trigger flags in a modern way" > a regression would appear on some legacy platforms such > as the ARM PXA Zylonite that specify IRQ resources like > this: > > static struct resource r = { > .start = X, > .end = X, > .flags = IORESOURCE_IRQ | IORESOURCE_IRQ_HIGHEDGE, > }; > > The previous code would retrieve the resource and parse > the high edge setting in the SMC91x driver, a use pattern > that means every driver specifying an IRQ flag from a > static resource need to parse resource flags and apply > them at runtime. > > As we switched the code to use IRQ descriptors to retrieve > the the trigger type like this: > > irqd_get_trigger_type(irq_get_irq_data(...)); > > the code would work for new platforms using e.g. device > tree as the backing irq descriptor would have its flags > properly set, whereas this kind of oldstyle static > resources at no point assign the trigger flags to the > corresponding IRQ descriptor. > > To make the behaviour identical on modern device tree > and legacy static platform data platforms, modify > platform_get_irq() to assign the trigger flags to the > irq descriptor when a client looks up an IRQ from static > resources. > > Fixes: d52fdbb735c3 ("smc91x: retrieve IRQ and trigger flags in a modern way") > Tested-by: Robert Jarzmik <robert.jarzmik@free.fr> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- > Greg/Grant: I'm a bit uncertain here. It's kind of > unintuitive that the platform_get_irq() function go around > setting trigger flags on IRQ descriptors, but it is *also* > unintuitive that the descriptor has all the right flags > from a device tree but not from an identical static > resource. And it is bloated to have resource parsing in > each and every driver. The alternative is to revert > the offending patch and live with some resource parsing > all over the place. (Such a patch is posted.) > --- > drivers/base/platform.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/drivers/base/platform.c b/drivers/base/platform.c > index 9421fed40905..e68ab79df28b 100644 > --- a/drivers/base/platform.c > +++ b/drivers/base/platform.c > @@ -101,6 +101,15 @@ int platform_get_irq(struct platform_device *dev, unsigned int num) > } > > r = platform_get_resource(dev, IORESOURCE_IRQ, num); > + /* > + * The resources may pass trigger flags to the irqs that need > + * to be set up. It so happens that the trigger flags for > + * IORESOURCE_BITS correspond 1-to-1 to the IRQF_TRIGGER* > + * settings. > + */ > + if (r && r->flags & IORESOURCE_BITS) > + irqd_set_trigger_type(irq_get_irq_data(r->start), > + r->flags & IORESOURCE_BITS);
Ah, found this now.
I'll defer to Grant as to what to do here, as I have no idea...
| |