| Message ID | 1470758408-14248-1-git-send-email-s.nawrocki@samsung.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
Sylwester Nawrocki <s.nawrocki@samsung.com> writes: > Commit b5a099c67a1c36b "net: ethernet: davicom: fix devicetree irq > resource" causes an interrupt storm after the ethernet interface > is activated on S3C24XX platform (ARM non-dt), due to the interrupt > trigger type not being set properly. > > It seems, after adding parsing of IRQ flags in commit 7085a7401ba54e92b > "drivers: platform: parse IRQ flags from resources", there is no path > for non-dt platforms where irq_set_type callback could be invoked when > we don't pass the trigger type flags to the request_irq() call. > > In case of a board where the regression is seen the interrupt trigger > type flags are passed through a platform device's resource and it is > not currently handled properly without passing the irq trigger type > flags to the request_irq() call. In case of OF an of_irq_get() call > within platform_get_irq() function seems to be ensuring required irq_chip > setup, but there is no equivalent code for non OF/ACPI platforms. > > This patch mostly restores irq trigger type setting code which has been > removed in commit ("net: ethernet: davicom: fix devicetree irq resource"). > > Fixes: b5a099c67a1c36b913 ("net: ethernet: davicom: fix devicetree irq resource") > > Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> > --- > > Perhaps instead the core could be configuring the irqchip automatically as it > is done for OF/ACPI cases. I had doubts though if trying to make such changes > for a bug fix patch was the right thing to do. Hi Sylvester, You're right, and I came to the same conclusion a bit earlier, in [1], but I didn't notice my FAI didn't actually send the mail. Your analysis of the core in non-OF/ACPI case is the reason I didn't post a patch for dm9000 ... I was overconfident in finding a reason in irq core code within a couple of days. Therefore: Acked-by: Robert Jarzmik <robert.jarzmik@free.fr> And I can make a test for you on my cm-x300 board, even if your patch is very alike the draft I had in my internal tree since then. Cheers. -- Robert [1] Non-delivered mail, shame on me From: Robert Jarzmik <robert.jarzmik@free.fr> To: Linus Walleij <linus.walleij@linaro.org> Cc: linux-arm-kernel@lists.infradead.org, Thomas Gleixner <tglx@linutronix.de>, linux-kernel@vger.kernel.org "David S. Miller" <davem@davemloft.net> Subject: platform_get_irq and trigger types X-URL: http://belgarath.falguerolles.org/ Date: Sat, 21 May 2016 11:16:09 +0200 Message-ID: <87y473hiue.fsf@belgarion.home> User-Agent: Gnus/5.130008 (Ma Gnus v0.8) Emacs/24.4 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain Hi Linus, I was bitten again by the rising/falling flags of interrupt flags. The commit which triggered this "regression" (the wording regression is rather incorrect so please don't take this as an incentive to revert) is : b5a099c67a1c ("net: ethernet: davicom: fix devicetree irq resource") The exact context is that for platform type builds, the irq rising edge flag is not activated in the irqchip, ie. in the gpio-pxa.c pxa_gpio_irq_type() is not called. The board used for this test is arch/arm/mach-pxa/cm-x300.c (line 200). Now I've started to add printks here and there, and from a first glance : - platform_get_irq() is correctly calling irqd_set_trigger_type() - but upon the request_irq() in drivers/net/ethernet/davicom/dm9000.c:1319, these flags are not taken into account => this is where commit b5a099c67a1c comes into play => re-adding irq_get_trigger_type(dev->irq) to the passed flags does solve the issue I tried to ponder whether my commit was wrong, or if it's the gpio-pxa.c which is wrong, or something else. My inner feeling is that dm9000.c code is now correct, and that something else is happening that I don't understand. I'm bringing this to your attention if you have an idea before I begin to dig deeper, add printk() and go down to the problem. Cheers.
From: Robert Jarzmik <robert.jarzmik@free.fr> Date: Tue, 09 Aug 2016 19:20:44 +0200 > Sylwester Nawrocki <s.nawrocki@samsung.com> writes: > >> Commit b5a099c67a1c36b "net: ethernet: davicom: fix devicetree irq >> resource" causes an interrupt storm after the ethernet interface >> is activated on S3C24XX platform (ARM non-dt), due to the interrupt >> trigger type not being set properly. >> >> It seems, after adding parsing of IRQ flags in commit 7085a7401ba54e92b >> "drivers: platform: parse IRQ flags from resources", there is no path >> for non-dt platforms where irq_set_type callback could be invoked when >> we don't pass the trigger type flags to the request_irq() call. >> >> In case of a board where the regression is seen the interrupt trigger >> type flags are passed through a platform device's resource and it is >> not currently handled properly without passing the irq trigger type >> flags to the request_irq() call. In case of OF an of_irq_get() call >> within platform_get_irq() function seems to be ensuring required irq_chip >> setup, but there is no equivalent code for non OF/ACPI platforms. >> >> This patch mostly restores irq trigger type setting code which has been >> removed in commit ("net: ethernet: davicom: fix devicetree irq resource"). >> >> Fixes: b5a099c67a1c36b913 ("net: ethernet: davicom: fix devicetree irq resource") >> >> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> >> --- >> >> Perhaps instead the core could be configuring the irqchip automatically as it >> is done for OF/ACPI cases. I had doubts though if trying to make such changes >> for a bug fix patch was the right thing to do. > Hi Sylvester, > > You're right, and I came to the same conclusion a bit earlier, in [1], but I > didn't notice my FAI didn't actually send the mail. Your analysis of the core in > non-OF/ACPI case is the reason I didn't post a patch for dm9000 ... I was > overconfident in finding a reason in irq core code within a couple of days. > > Therefore: > Acked-by: Robert Jarzmik <robert.jarzmik@free.fr> Applied. -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/ethernet/davicom/dm9000.c b/drivers/net/ethernet/davicom/dm9000.c index 1471e16..f45385f 100644 --- a/drivers/net/ethernet/davicom/dm9000.c +++ b/drivers/net/ethernet/davicom/dm9000.c @@ -1299,6 +1299,7 @@ static int dm9000_open(struct net_device *dev) { struct board_info *db = netdev_priv(dev); + unsigned int irq_flags = irq_get_trigger_type(dev->irq); if (netif_msg_ifup(db)) dev_dbg(db->dev, "enabling %s\n", dev->name); @@ -1306,9 +1307,11 @@ dm9000_open(struct net_device *dev) /* If there is no IRQ type specified, tell the user that this is a * problem */ - if (irq_get_trigger_type(dev->irq) == IRQF_TRIGGER_NONE) + if (irq_flags == IRQF_TRIGGER_NONE) dev_warn(db->dev, "WARNING: no IRQ resource flags set.\n"); + irq_flags |= IRQF_SHARED; + /* GPIO0 on pre-activate PHY, Reg 1F is not set by reset */ iow(db, DM9000_GPR, 0); /* REG_1F bit0 activate phyxcer */ mdelay(1); /* delay needs by DM9000B */ @@ -1316,8 +1319,7 @@ dm9000_open(struct net_device *dev) /* Initialize DM9000 board */ dm9000_init_dm9000(dev); - if (request_irq(dev->irq, dm9000_interrupt, IRQF_SHARED, - dev->name, dev)) + if (request_irq(dev->irq, dm9000_interrupt, irq_flags, dev->name, dev)) return -EAGAIN; /* Now that we have an interrupt handler hooked up we can unmask * our interrupts
Commit b5a099c67a1c36b "net: ethernet: davicom: fix devicetree irq resource" causes an interrupt storm after the ethernet interface is activated on S3C24XX platform (ARM non-dt), due to the interrupt trigger type not being set properly. It seems, after adding parsing of IRQ flags in commit 7085a7401ba54e92b "drivers: platform: parse IRQ flags from resources", there is no path for non-dt platforms where irq_set_type callback could be invoked when we don't pass the trigger type flags to the request_irq() call. In case of a board where the regression is seen the interrupt trigger type flags are passed through a platform device's resource and it is not currently handled properly without passing the irq trigger type flags to the request_irq() call. In case of OF an of_irq_get() call within platform_get_irq() function seems to be ensuring required irq_chip setup, but there is no equivalent code for non OF/ACPI platforms. This patch mostly restores irq trigger type setting code which has been removed in commit ("net: ethernet: davicom: fix devicetree irq resource"). Fixes: b5a099c67a1c36b913 ("net: ethernet: davicom: fix devicetree irq resource") Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> --- Perhaps instead the core could be configuring the irqchip automatically as it is done for OF/ACPI cases. I had doubts though if trying to make such changes for a bug fix patch was the right thing to do. --- drivers/net/ethernet/davicom/dm9000.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)