KSZ swithes used interrupts for detecting the phy link up and down.
During registering the interrupt handler, it used IRQF_TRIGGER_FALLING
flag. But this flag has to be retrieved from device tree instead of hard
coding in the driver, so removing the flag.
Fixes: ff319a644829 ("net: dsa: microchip: move interrupt handling logic from lan937x to ksz_common")
Reported-by: Christian Eggers <[email protected]>
Signed-off-by: Arun Ramadoss <[email protected]>
---
drivers/net/dsa/microchip/ksz_common.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index d612181b3226..c68f48cd1ec0 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -1883,8 +1883,7 @@ static int ksz_irq_common_setup(struct ksz_device *dev, struct ksz_irq *kirq)
irq_create_mapping(kirq->domain, n);
ret = request_threaded_irq(kirq->irq_num, NULL, ksz_irq_thread_fn,
- IRQF_ONESHOT | IRQF_TRIGGER_FALLING,
- kirq->name, kirq);
+ IRQF_ONESHOT, kirq->name, kirq);
if (ret)
goto out;
base-commit: e095493091e850d5292ad01d8fbf5cde1d89ac53
--
2.36.1
On Tue, 2022-12-13 at 15:44 +0530, Arun Ramadoss wrote:
> KSZ swithes used interrupts for detecting the phy link up and down.
> During registering the interrupt handler, it used IRQF_TRIGGER_FALLING
> flag. But this flag has to be retrieved from device tree instead of hard
> coding in the driver,
Out of sheer ignorance, why?
> so removing the flag.
It looks like the device tree currently lack such item, so this is
effecivelly breaking phy linkup/linkdown?
What am I missing?
Thanks!
Paolo
Hi Paolo,
On Thursday, 15 December 2022, 12:29:22 CET, Paolo Abeni wrote:
> On Tue, 2022-12-13 at 15:44 +0530, Arun Ramadoss wrote:
> > KSZ swithes used interrupts for detecting the phy link up and down.
> > During registering the interrupt handler, it used IRQF_TRIGGER_FALLING
> > flag. But this flag has to be retrieved from device tree instead of hard
> > coding in the driver,
>
> Out of sheer ignorance, why?
As far as I know, some IRQF_ flags should be set through the firmware
(e.g. device tree) instead hard coding them into the driver. On my
platform, I have to use IRQF_TRIGGER_LOW instead of IRQF_TRIGGER_FALLING.
If the value is hard coded into the driver, the value from the driver
will have precedence.
See also: https://stackoverflow.com/a/40051191
> > so removing the flag.
>
> It looks like the device tree currently lack such item, so this is
> effecivelly breaking phy linkup/linkdown?
What is "the" device tree. Do you mean the device tree for your specific
board, or the example under
Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml?
The latter doesn't mention the irq at all.
BTW: In my kernel log I get the following messages:
> ksz9477-switch 0-005f: configuring for fixed/rmii link mode
> ksz9477-switch 0-005f lan0 (uninitialized): PHY [dsa-0.0:00] driver [Microchip KSZ9477] (irq=POLL)
> ksz9477-switch 0-005f: Link is Up - 100Mbps/Full - flow control off
> ksz9477-switch 0-005f lan1 (uninitialized): PHY [dsa-0.0:01] driver [Microchip KSZ9477] (irq=POLL)
Should I see something different than "irq=POLL" when an
irq line is provided in the device tree?
regards
Christian
Hi Christian,
On Thu, 2022-12-15 at 14:50 +0100, Christian Eggers wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
>
> Hi Paolo,
>
> On Thursday, 15 December 2022, 12:29:22 CET, Paolo Abeni wrote:
> > On Tue, 2022-12-13 at 15:44 +0530, Arun Ramadoss wrote:
> > > KSZ swithes used interrupts for detecting the phy link up and
> > > down.
> > > During registering the interrupt handler, it used
> > > IRQF_TRIGGER_FALLING
> > > flag. But this flag has to be retrieved from device tree instead
> > > of hard
> > > coding in the driver,
> >
> > Out of sheer ignorance, why?
>
> As far as I know, some IRQF_ flags should be set through the firmware
> (e.g. device tree) instead hard coding them into the driver. On my
> platform, I have to use IRQF_TRIGGER_LOW instead of
> IRQF_TRIGGER_FALLING.
> If the value is hard coded into the driver, the value from the driver
> will have precedence.
>
> See also: https://stackoverflow.com/a/40051191
>
> > > so removing the flag.
> >
> > It looks like the device tree currently lack such item, so this is
> > effecivelly breaking phy linkup/linkdown?
>
> What is "the" device tree. Do you mean the device tree for your
> specific
> board, or the example under
> Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml?
> The latter doesn't mention the irq at all.
>
> BTW: In my kernel log I get the following messages:
>
> > ksz9477-switch 0-005f: configuring for fixed/rmii link mode
> > ksz9477-switch 0-005f lan0 (uninitialized): PHY [dsa-0.0:00] driver
> > [Microchip KSZ9477] (irq=POLL)
> > ksz9477-switch 0-005f: Link is Up - 100Mbps/Full - flow control off
> > ksz9477-switch 0-005f lan1 (uninitialized): PHY [dsa-0.0:01] driver
> > [Microchip KSZ9477] (irq=POLL)
>
> Should I see something different than "irq=POLL" when an
> irq line is provided in the device tree?
If the device tree is provided *interrupt controller and interrupt
cells*, the kernel message should print the irq number like (irq=67)
instead of POLL. (67 is random number).
Following is the device tree snippet,
ksz9563: switch@0 {
compatible = "microchip,ksz9563";
reg = <0>;
spi-max-frequency = <44000000>;
interrupt-parent = <&pioB>;
interrupts = <28 IRQ_TYPE_LEVEL_LOW>;
interrupt-controller;
#interrupt-cells = <2>;
resetb-gpios = <&pioD 18 GPIO_ACTIVE_LOW>;
pinctrl-0 = <&pinctrl_spi_ksz_rst>;
status = "okay";
....
}
>
> regards
> Christian
>
>
>
Hello:
This patch was applied to netdev/net.git (master)
by Jakub Kicinski <[email protected]>:
On Tue, 13 Dec 2022 15:44:40 +0530 you wrote:
> KSZ swithes used interrupts for detecting the phy link up and down.
> During registering the interrupt handler, it used IRQF_TRIGGER_FALLING
> flag. But this flag has to be retrieved from device tree instead of hard
> coding in the driver, so removing the flag.
>
> Fixes: ff319a644829 ("net: dsa: microchip: move interrupt handling logic from lan937x to ksz_common")
> Reported-by: Christian Eggers <[email protected]>
> Signed-off-by: Arun Ramadoss <[email protected]>
>
> [...]
Here is the summary with links:
- [net] net: dsa: microchip: remove IRQF_TRIGGER_FALLING in request_threaded_irq
https://git.kernel.org/netdev/net/c/62e027fb0e52
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html