* [PATCH] net: stmmac: platform: Fix misleading interrupt error msg
@ 2020-03-06 16:38 Markus Fuchs
2020-03-12 6:04 ` David Miller
0 siblings, 1 reply; 5+ messages in thread
From: Markus Fuchs @ 2020-03-06 16:38 UTC (permalink / raw)
To: mklntf
Cc: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
David S. Miller, Maxime Coquelin, netdev, linux-stm32,
linux-arm-kernel, linux-kernel
Not every stmmac based platform makes use of the eth_wake_irq or eth_lpi
interrupts. Use the platform_get_irq_byname_optional variant for these
interrupts, so no error message is displayed, if they can't be found.
Rather print an information to hint something might be wrong to assist
debugging on platforms which use these interrupts.
Signed-off-by: Markus Fuchs <mklntf@gmail.com>
---
On my cyclone V socfpga platform I get error messages after updating to
Linux Kernel 5.4.24
Starting kernel ...
Deasserting all peripheral resets
[ 1.206363] socfpga-dwmac ff700000.ethernet: IRQ eth_wake_irq not found
[ 1.213023] socfpga-dwmac ff700000.ethernet: IRQ eth_lpi not found
These interrupts don't matter for my platform and many other stmmac based
ones, as we can see by grepping for 'macirq'.
socfpga.dtsi: interrupt-names = "macirq";
socfpga.dtsi: interrupt-names = "macirq";
sun7i-a20.dtsi: interrupt-names = "macirq";
spear600.dtsi: interrupt-names = "macirq", "eth_wake_irq";
artpec6.dtsi: interrupt-names = "macirq", "eth_lpi";
rk322x.dtsi: interrupt-names = "macirq";
sun9i-a80.dtsi: interrupt-names = "macirq";
spear1310.dtsi: interrupt-names = "macirq";
spear1310.dtsi: interrupt-names = "macirq";
spear1310.dtsi: interrupt-names = "macirq";
spear1310.dtsi: interrupt-names = "macirq";
stih407-family.dtsi: interrupt-names = "macirq", "eth_wake_irq";
stm32f429.dtsi: interrupt-names = "macirq";
sun6i-a31.dtsi: interrupt-names = "macirq";
meson.dtsi: interrupt-names = "macirq";
rk3288.dtsi: interrupt-names = "macirq", "eth_wake_irq";
sun8i-r40.dtsi: interrupt-names = "macirq";
sunxi-h3-h5.dtsi: interrupt-names = "macirq";
spear3xx.dtsi: interrupt-names = "macirq", "eth_wake_irq";
lpc18xx.dtsi: interrupt-names = "macirq";
stm32h743.dtsi: interrupt-names = "macirq";
socfpga_arria10.dtsi: interrupt-names = "macirq";
socfpga_arria10.dtsi: interrupt-names = "macirq";
socfpga_arria10.dtsi: interrupt-names = "macirq";
rv1108.dtsi: interrupt-names = "macirq", "eth_wake_irq";
spear13xx.dtsi: interrupt-names = "macirq", "eth_wake_irq";
stm32mp151.dtsi: interrupt-names = "macirq";
ox820.dtsi: interrupt-names = "macirq", "eth_wake_irq";
sun8i-a83t.dtsi: interrupt-names = "macirq";
So, in my opinion, the error messages are missleading. I believe
the right way to handle this would require more changes though. Some
kind of configuration information, telling which interrupts are required
by the platform and than conditionally call platform_get_irq_byname().
This would print an error message, if something is wrong, on the right
platforms and nothing at all on the others.
.../net/ethernet/stmicro/stmmac/stmmac_platform.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index d10ac54bf385..13fafd905db8 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -663,16 +663,22 @@ int stmmac_get_platform_resources(struct platform_device *pdev,
* In case the wake up interrupt is not passed from the platform
* so the driver will continue to use the mac irq (ndev->irq)
*/
- stmmac_res->wol_irq = platform_get_irq_byname(pdev, "eth_wake_irq");
+ stmmac_res->wol_irq =
+ platform_get_irq_byname_optional(pdev, "eth_wake_irq");
if (stmmac_res->wol_irq < 0) {
if (stmmac_res->wol_irq == -EPROBE_DEFER)
return -EPROBE_DEFER;
+ dev_info(&pdev->dev, "IRQ eth_wake_irq not found\n");
stmmac_res->wol_irq = stmmac_res->irq;
}
- stmmac_res->lpi_irq = platform_get_irq_byname(pdev, "eth_lpi");
- if (stmmac_res->lpi_irq == -EPROBE_DEFER)
- return -EPROBE_DEFER;
+ stmmac_res->lpi_irq =
+ platform_get_irq_byname_optional(pdev, "eth_lpi");
+ if (stmmac_res->lpi_irq < 0) {
+ if (stmmac_res->lpi_irq == -EPROBE_DEFER)
+ return -EPROBE_DEFER;
+ dev_info(&pdev->dev, "IRQ eth_lpi not found\n");
+ }
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
stmmac_res->addr = devm_ioremap_resource(&pdev->dev, res);
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] net: stmmac: platform: Fix misleading interrupt error msg
2020-03-06 16:38 [PATCH] net: stmmac: platform: Fix misleading interrupt error msg Markus Fuchs
@ 2020-03-12 6:04 ` David Miller
2020-03-12 16:37 ` Markus Fuchs
2020-03-12 19:46 ` Markus Fuchs
0 siblings, 2 replies; 5+ messages in thread
From: David Miller @ 2020-03-12 6:04 UTC (permalink / raw)
To: mklntf
Cc: peppe.cavallaro, alexandre.torgue, joabreu, mcoquelin.stm32,
netdev, linux-stm32, linux-arm-kernel, linux-kernel
From: Markus Fuchs <mklntf@gmail.com>
Date: Fri, 6 Mar 2020 17:38:48 +0100
> Not every stmmac based platform makes use of the eth_wake_irq or eth_lpi
> interrupts. Use the platform_get_irq_byname_optional variant for these
> interrupts, so no error message is displayed, if they can't be found.
> Rather print an information to hint something might be wrong to assist
> debugging on platforms which use these interrupts.
>
> Signed-off-by: Markus Fuchs <mklntf@gmail.com>
What do you mean the error message is misleading right now?
It isn't printing anything out at the moment in this situation.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] net: stmmac: platform: Fix misleading interrupt error msg
2020-03-12 6:04 ` David Miller
@ 2020-03-12 16:37 ` Markus Fuchs
2020-03-12 19:46 ` Markus Fuchs
1 sibling, 0 replies; 5+ messages in thread
From: Markus Fuchs @ 2020-03-12 16:37 UTC (permalink / raw)
To: David Miller
Cc: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
Maxime Coquelin, netdev, linux-stm32, linux-arm-kernel,
linux-kernel
On Thu, 12 Mar 2020 at 07:04, David Miller <davem@davemloft.net> wrote:
>
> From: Markus Fuchs <mklntf@gmail.com>
> Date: Fri, 6 Mar 2020 17:38:48 +0100
>
> > Not every stmmac based platform makes use of the eth_wake_irq or eth_lpi
> > interrupts. Use the platform_get_irq_byname_optional variant for these
> > interrupts, so no error message is displayed, if they can't be found.
> > Rather print an information to hint something might be wrong to assist
> > debugging on platforms which use these interrupts.
> >
> > Signed-off-by: Markus Fuchs <mklntf@gmail.com>
>
> What do you mean the error message is misleading right now?
>
> It isn't printing anything out at the moment in this situation.
Hello,
the error messages are
[ 1.206363] socfpga-dwmac ff700000.ethernet: IRQ eth_wake_irq not found
[ 1.213023] socfpga-dwmac ff700000.ethernet: IRQ eth_lpi not found
I tried to explain this in my original post between the --- lines of the patch.
Maybe this was wrong, so I repost it.
On my cyclone V socfpga platform I get error messages after updating to
Linux Kernel 5.4.24
Starting kernel ...
Deasserting all peripheral resets
[ 1.206363] socfpga-dwmac ff700000.ethernet: IRQ eth_wake_irq not found
[ 1.213023] socfpga-dwmac ff700000.ethernet: IRQ eth_lpi not found
These interrupts don't matter for my platform and many other stmmac based
ones, as we can see by grepping for 'macirq'.
socfpga.dtsi: interrupt-names = "macirq";
socfpga.dtsi: interrupt-names = "macirq";
sun7i-a20.dtsi: interrupt-names = "macirq";
spear600.dtsi: interrupt-names = "macirq", "eth_wake_irq";
artpec6.dtsi: interrupt-names = "macirq", "eth_lpi";
rk322x.dtsi: interrupt-names = "macirq";
sun9i-a80.dtsi: interrupt-names = "macirq";
spear1310.dtsi: interrupt-names = "macirq";
spear1310.dtsi: interrupt-names = "macirq";
spear1310.dtsi: interrupt-names = "macirq";
spear1310.dtsi: interrupt-names = "macirq";
stih407-family.dtsi: interrupt-names = "macirq", "eth_wake_irq";
stm32f429.dtsi: interrupt-names = "macirq";
sun6i-a31.dtsi: interrupt-names = "macirq";
meson.dtsi: interrupt-names = "macirq";
rk3288.dtsi: interrupt-names = "macirq", "eth_wake_irq";
sun8i-r40.dtsi: interrupt-names = "macirq";
sunxi-h3-h5.dtsi: interrupt-names = "macirq";
spear3xx.dtsi: interrupt-names = "macirq", "eth_wake_irq";
lpc18xx.dtsi: interrupt-names = "macirq";
stm32h743.dtsi: interrupt-names = "macirq";
socfpga_arria10.dtsi: interrupt-names = "macirq";
socfpga_arria10.dtsi: interrupt-names = "macirq";
socfpga_arria10.dtsi: interrupt-names = "macirq";
rv1108.dtsi: interrupt-names = "macirq", "eth_wake_irq";
spear13xx.dtsi: interrupt-names = "macirq", "eth_wake_irq";
stm32mp151.dtsi: interrupt-names = "macirq";
ox820.dtsi: interrupt-names = "macirq", "eth_wake_irq";
sun8i-a83t.dtsi: interrupt-names = "macirq";
So, in my opinion, the error messages are missleading. I believe
the right way to handle this would require more changes though. Some
kind of configuration information, telling which interrupts are required
by the platform and than conditionally call platform_get_irq_byname().
This would print an error message, if something is wrong, on the right
platforms and nothing at all on the others.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] net: stmmac: platform: Fix misleading interrupt error msg
2020-03-12 6:04 ` David Miller
2020-03-12 16:37 ` Markus Fuchs
@ 2020-03-12 19:46 ` Markus Fuchs
2020-03-15 4:00 ` David Miller
1 sibling, 1 reply; 5+ messages in thread
From: Markus Fuchs @ 2020-03-12 19:46 UTC (permalink / raw)
To: David Miller
Cc: peppe.cavallaro, alexandre.torgue, joabreu, mcoquelin.stm32,
netdev, linux-stm32, linux-arm-kernel, linux-kernel
On Wed, Mar 11, 2020 at 11:04:02PM -0700, David Miller wrote:
> From: Markus Fuchs <mklntf@gmail.com>
> Date: Fri, 6 Mar 2020 17:38:48 +0100
>
> > Not every stmmac based platform makes use of the eth_wake_irq or eth_lpi
> > interrupts. Use the platform_get_irq_byname_optional variant for these
> > interrupts, so no error message is displayed, if they can't be found.
> > Rather print an information to hint something might be wrong to assist
> > debugging on platforms which use these interrupts.
> >
> > Signed-off-by: Markus Fuchs <mklntf@gmail.com>
>
> What do you mean the error message is misleading right now?
>
> It isn't printing anything out at the moment in this situation.
Commit 7723f4c5ecdb driver core: platform: Add an error message to
platform_get_irq*()
The above commit added a generic dev_err() output to the platform_get_irq_byname
function.
My patch uses the platform_get_irq_byname_optional function, which
doesn't print anything and adds the original dev_err output as dev_info output
to the driver.
Otherwise there would be no output at all even for platforms in need of these
irqs.
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] net: stmmac: platform: Fix misleading interrupt error msg
2020-03-12 19:46 ` Markus Fuchs
@ 2020-03-15 4:00 ` David Miller
0 siblings, 0 replies; 5+ messages in thread
From: David Miller @ 2020-03-15 4:00 UTC (permalink / raw)
To: mklntf
Cc: peppe.cavallaro, alexandre.torgue, joabreu, mcoquelin.stm32,
netdev, linux-stm32, linux-arm-kernel, linux-kernel
From: Markus Fuchs <mklntf@gmail.com>
Date: Thu, 12 Mar 2020 20:46:25 +0100
> On Wed, Mar 11, 2020 at 11:04:02PM -0700, David Miller wrote:
>> From: Markus Fuchs <mklntf@gmail.com>
>> Date: Fri, 6 Mar 2020 17:38:48 +0100
>>
>> > Not every stmmac based platform makes use of the eth_wake_irq or eth_lpi
>> > interrupts. Use the platform_get_irq_byname_optional variant for these
>> > interrupts, so no error message is displayed, if they can't be found.
>> > Rather print an information to hint something might be wrong to assist
>> > debugging on platforms which use these interrupts.
>> >
>> > Signed-off-by: Markus Fuchs <mklntf@gmail.com>
>>
>> What do you mean the error message is misleading right now?
>>
>> It isn't printing anything out at the moment in this situation.
>
> Commit 7723f4c5ecdb driver core: platform: Add an error message to
> platform_get_irq*()
>
> The above commit added a generic dev_err() output to the platform_get_irq_byname
> function.
>
> My patch uses the platform_get_irq_byname_optional function, which
> doesn't print anything and adds the original dev_err output as dev_info output
> to the driver.
> Otherwise there would be no output at all even for platforms in need of these
> irqs.
Aha, now I get it, thanks for explaining.
Applied, thank you.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-03-15 4:00 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-06 16:38 [PATCH] net: stmmac: platform: Fix misleading interrupt error msg Markus Fuchs
2020-03-12 6:04 ` David Miller
2020-03-12 16:37 ` Markus Fuchs
2020-03-12 19:46 ` Markus Fuchs
2020-03-15 4:00 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).