diff mbox

Revert "smc91x: retrieve IRQ and trigger flags in a modern way"

Message ID CACRpkdZGze_2=hWe414Eum_9hpNXgog5vmu6Y4R_zhg0x2z0SQ@mail.gmail.com
State New
Headers show

Commit Message

Linus Walleij Feb. 13, 2015, 2:16 a.m. UTC 
On Fri, Feb 13, 2015 at 12:59 AM, Robert Jarzmik <robert.jarzmik@free.fr> wrote:

> The commit breaks the legacy platforms, ie. these not using device-tree,
> and setting up the interrupt resources with a flag to activate edge
> detection. The issue was found on the zylonite platform.
>
> The reason is that zylonite uses platform resources to pass the interrupt number
> and the irq flags (here IORESOURCE_IRQ_HIGHEDGE). It expects the driver to
> request the irq with these flags, which in turn setups the irq as high edge
> triggered.
>
> After the patch, this was supposed to be taken care of with :
>   irq_resflags = irqd_get_trigger_type(irq_get_irq_data(ndev->irq));
>
> But irq_resflags is 0 for legacy platforms, while for example in
> arch/arm/mach-pxa/zylonite.c, in struct resource smc91x_resources[] the
> irq flag is specified. This breaks zylonite because the interrupt is not
> setup as triggered, and hardware doesn't provide interrupts.
>
> Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>

But isn't the real problem that in the device tree case,
irq_get_irq_data(ndev->irq) will work becaus parsing an interrupt
from the device tree populates it correctly in platform_get_irq()
whereas for the legacy lookup it just fetches the number.

So to me it seems like a weakness in the platform_get_irq()
helper altogether.

Does the following work? (I can send as a separate patch for
testing if you like).

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Comments

Linus Walleij Feb. 20, 2015, 9:37 a.m. UTC | #1 
On Thu, Feb 19, 2015 at 9:48 PM, Robert Jarzmik <robert.jarzmik@free.fr> wrote:
> David Miller <davem@davemloft.net> writes:
>
>>> Now if you can make it in -rc2 or -rc3, this revert should be forgotten. But if
>>> you can't make it for 3.20, I'll push for the revert.
>>>
>>> So I think it's up to you now, and let's see what Gregh says about it.
>>
>> What is the current status of this?  I'd like to see this move forward so we
>> can get this fixed ASAP.
> Hi David,
>
> Linus has submitted the patch [1]. I'll be watching carefully until -rc4 that
> this is applied. If it's not, I'll reping you to apply this revert. Until then,
> you can forget about it, I'll do the follow-up.
> [1] https://lkml.org/lkml/2015/2/18/310

Looping in Greg and Grant so they know we are waiting for their verdict
on that patch...


Yours,
Liunus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Linus Walleij Feb. 23, 2015, 3:14 p.m. UTC | #2 
On Fri, Feb 20, 2015 at 4:33 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Fri, Feb 20, 2015 at 10:37:59AM +0100, Linus Walleij wrote:
>> On Thu, Feb 19, 2015 at 9:48 PM, Robert Jarzmik <robert.jarzmik@free.fr> wrote:
>> > David Miller <davem@davemloft.net> writes:
>> >
>> >>> Now if you can make it in -rc2 or -rc3, this revert should be forgotten. But if
>> >>> you can't make it for 3.20, I'll push for the revert.
>> >>>
>> >>> So I think it's up to you now, and let's see what Gregh says about it.
>> >>
>> >> What is the current status of this?  I'd like to see this move forward so we
>> >> can get this fixed ASAP.
>> > Hi David,
>> >
>> > Linus has submitted the patch [1]. I'll be watching carefully until -rc4 that
>> > this is applied. If it's not, I'll reping you to apply this revert. Until then,
>> > you can forget about it, I'll do the follow-up.
>> > [1] https://lkml.org/lkml/2015/2/18/310
>>
>> Looping in Greg and Grant so they know we are waiting for their verdict
>> on that patch...
>
> I have no idea what you are talking about here :(

It is a patch to drivers/base, which I'm waiting for you to look at...
The link to the patch was right above, here it is again :)

https://lkml.org/lkml/2015/2/18/310

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
diff mbox

Patch

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 9421fed40905..301f4b9ae908 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->flags & IORESOURCE_BITS)
+        irqd_set_trigger_type(irq_get_irq_data(r->start),
+                      r->flags & IORESOURCE_BITS);