* [PATCH] dm9000: Fix irq trigger type setup on non-dt platforms
@ 2016-08-09 16:00 Sylwester Nawrocki
2016-08-09 17:20 ` Robert Jarzmik
0 siblings, 1 reply; 3+ messages in thread
From: Sylwester Nawrocki @ 2016-08-09 16:00 UTC (permalink / raw)
To: davem
Cc: netdev, robert.jarzmik, linus.walleij, tglx, b.zolnierkie,
linux-kernel, linux-samsung-soc, Sylwester Nawrocki
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(-)
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
--
1.9.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] dm9000: Fix irq trigger type setup on non-dt platforms
2016-08-09 16:00 [PATCH] dm9000: Fix irq trigger type setup on non-dt platforms Sylwester Nawrocki
@ 2016-08-09 17:20 ` Robert Jarzmik
2016-08-09 22:08 ` David Miller
0 siblings, 1 reply; 3+ messages in thread
From: Robert Jarzmik @ 2016-08-09 17:20 UTC (permalink / raw)
To: Sylwester Nawrocki
Cc: davem, netdev, linus.walleij, tglx, b.zolnierkie, linux-kernel,
linux-samsung-soc
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.
--
Robert
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] dm9000: Fix irq trigger type setup on non-dt platforms
2016-08-09 17:20 ` Robert Jarzmik
@ 2016-08-09 22:08 ` David Miller
0 siblings, 0 replies; 3+ messages in thread
From: David Miller @ 2016-08-09 22:08 UTC (permalink / raw)
To: robert.jarzmik
Cc: s.nawrocki, netdev, linus.walleij, tglx, b.zolnierkie,
linux-kernel, linux-samsung-soc
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.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-08-09 22:08 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-09 16:00 [PATCH] dm9000: Fix irq trigger type setup on non-dt platforms Sylwester Nawrocki
2016-08-09 17:20 ` Robert Jarzmik
2016-08-09 22:08 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.