[PATCH] w5100: restore GPIO-based link detection

[PATCH] w5100: restore GPIO-based link detection

[Arthur Crépin Leblond]

Commit dacf281771a9 ("w5100: remove unused gpio link detection")
dropped the link_gpio/link_irq handling on the grounds that no
devicetree user passed a "link" GPIO at the time.

Signed-off-by: Arthur Crépin Leblond <arthur@marmottus.net>
---
Hi,

I am using the W5500 on a Raspberry Pi and rely on the link GPIO for
the carrier detection.
If we could bring it back, I would greatly appreciate.

Thank you!

Arthur Crépin Leblond
---
 .../devicetree/bindings/net/wiznet,w5x00.txt       |  8 ++-
 drivers/net/ethernet/wiznet/w5100.c                | 84 ++++++++++++++++++++++
 2 files changed, 89 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/wiznet,w5x00.txt b/Documentation/devicetree/bindings/net/wiznet,w5x00.txt
index e9665798c4be..e97ce3cb9183 100644
--- a/Documentation/devicetree/bindings/net/wiznet,w5x00.txt
+++ b/Documentation/devicetree/bindings/net/wiznet,w5x00.txt
@@ -25,6 +25,7 @@ Optional properties:
   According to the w5500 datasheet, the chip allows a maximum of 80 MHz, however,
   board designs may need to limit this value.
 - local-mac-address: See ethernet.txt in the same directory.
+- link-gpios: a GPIO line used for the link detection interrupt
 
 
 Example (for Raspberry Pi with pin control stuff for GPIO irq):
@@ -38,13 +39,14 @@ Example (for Raspberry Pi with pin control stuff for GPIO irq):
 		interrupt-parent = <&gpio>;
 		interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
 		spi-max-frequency = <30000000>;
+		link-gpios = <&gpio 4 GPIO_ACTIVE_HIGH>;
 	};
 };
 
 &gpio {
 	eth1_pins: eth1_pins {
-		brcm,pins = <25>;
-		brcm,function = <0>; /* in */
-		brcm,pull = <0>; /* none */
+		brcm,pins = <25 4>;
+		brcm,function = <0 0>; /* in */
+		brcm,pull = <0 0>; /* none */
 	};
 };
diff --git a/drivers/net/ethernet/wiznet/w5100.c b/drivers/net/ethernet/wiznet/w5100.c
index 53d8dc642fbd..c9d1891f37b9 100644
--- a/drivers/net/ethernet/wiznet/w5100.c
+++ b/drivers/net/ethernet/wiznet/w5100.c
@@ -22,6 +22,7 @@
 #include <linux/ioport.h>
 #include <linux/interrupt.h>
 #include <linux/irq.h>
+#include <linux/gpio/consumer.h>
 
 #include "w5100.h"
 
@@ -154,6 +155,8 @@ struct w5100_priv {
 	u16 s0_rx_buf_size;
 
 	int irq;
+	int link_irq;
+	struct gpio_desc *link_gpio;
 
 	struct napi_struct napi;
 	struct net_device *ndev;
@@ -414,6 +417,16 @@ static void w5100_get_drvinfo(struct net_device *ndev,
 		sizeof(info->bus_info));
 }
 
+static u32 w5100_get_link(struct net_device *ndev)
+{
+	struct w5100_priv *priv = netdev_priv(ndev);
+
+	if (priv->link_gpio)
+		return !!gpiod_get_value_cansleep(priv->link_gpio);
+
+	return 1;
+}
+
 static u32 w5100_get_msglevel(struct net_device *ndev)
 {
 	struct w5100_priv *priv = netdev_priv(ndev);
@@ -616,6 +629,24 @@ static irqreturn_t w5100_interrupt(int irq, void *ndev_instance)
 	return IRQ_HANDLED;
 }
 
+static irqreturn_t w5100_detect_link(int irq, void *ndev_instance)
+{
+	struct net_device *ndev = ndev_instance;
+	struct w5100_priv *priv = netdev_priv(ndev);
+
+	if (netif_running(ndev)) {
+		if (gpiod_get_value(priv->link_gpio) != 0) {
+			netif_info(priv, link, ndev, "link is up\n");
+			netif_carrier_on(ndev);
+		} else {
+			netif_info(priv, link, ndev, "link is down\n");
+			netif_carrier_off(ndev);
+		}
+	}
+
+	return IRQ_HANDLED;
+}
+
 static void w5100_setrx_work(struct work_struct *work)
 {
 	struct w5100_priv *priv = container_of(work, struct w5100_priv,
@@ -659,6 +690,14 @@ static int w5100_open(struct net_device *ndev)
 	w5100_hw_start(priv);
 	napi_enable(&priv->napi);
 	netif_start_queue(ndev);
+
+	if (priv->link_gpio) {
+		if (gpiod_get_value_cansleep(priv->link_gpio) != 0)
+			netif_carrier_on(ndev);
+		else
+			netif_carrier_off(ndev);
+	}
+
 	return 0;
 }
 
@@ -678,6 +717,7 @@ static const struct ethtool_ops w5100_ethtool_ops = {
 	.get_drvinfo		= w5100_get_drvinfo,
 	.get_msglevel		= w5100_get_msglevel,
 	.set_msglevel		= w5100_set_msglevel,
+	.get_link		= w5100_get_link,
 	.get_regs_len		= w5100_get_regs_len,
 	.get_regs		= w5100_get_regs,
 };
@@ -751,6 +791,13 @@ int w5100_probe(struct device *dev, const struct w5100_ops *ops,
 	priv->ndev = ndev;
 	priv->ops = ops;
 	priv->irq = irq;
+	priv->link_gpio = gpiod_get_optional(dev, "link", GPIOD_IN);
+	if (IS_ERR(priv->link_gpio)) {
+		err = dev_err_probe(dev, PTR_ERR(priv->link_gpio),
+				    "failed to get link GPIO\n");
+		priv->link_gpio = NULL;
+		goto err_register;
+	}
 
 	ndev->netdev_ops = &w5100_netdev_ops;
 	ndev->ethtool_ops = &w5100_ethtool_ops;
@@ -803,13 +850,40 @@ int w5100_probe(struct device *dev, const struct w5100_ops *ops,
 	if (err)
 		goto err_hw;
 
+	if (priv->link_gpio) {
+		char *link_name = devm_kasprintf(dev, GFP_KERNEL, "%s-link",
+						 dev_name(dev));
+		if (!link_name) {
+			err = -ENOMEM;
+			goto err_gpio;
+		}
+
+		priv->link_irq = gpiod_to_irq(priv->link_gpio);
+		if (priv->link_irq < 0) {
+			err = priv->link_irq;
+			goto err_gpio;
+		}
+
+		err = request_any_context_irq(priv->link_irq, w5100_detect_link,
+					      IRQF_TRIGGER_RISING |
+						      IRQF_TRIGGER_FALLING,
+					      link_name, priv->ndev);
+		if (err < 0)
+			goto err_gpio;
+	}
+
 	return 0;
 
+err_gpio:
+	free_irq(priv->irq, ndev);
 err_hw:
 	destroy_workqueue(priv->xfer_wq);
 err_wq:
 	unregister_netdev(ndev);
 err_register:
+	if (priv->link_gpio)
+		gpiod_put(priv->link_gpio);
+
 	free_netdev(ndev);
 	return err;
 }
@@ -822,6 +896,10 @@ void w5100_remove(struct device *dev)
 
 	w5100_hw_reset(priv);
 	free_irq(priv->irq, ndev);
+	if (priv->link_gpio) {
+		free_irq(priv->link_irq, ndev);
+		gpiod_put(priv->link_gpio);
+	}
 
 	flush_work(&priv->setrx_work);
 	flush_work(&priv->restart_work);
@@ -840,6 +918,7 @@ static int w5100_suspend(struct device *dev)
 
 	if (netif_running(ndev)) {
 		netif_carrier_off(ndev);
+
 		netif_device_detach(ndev);
 
 		w5100_hw_close(priv);
@@ -857,7 +936,12 @@ static int w5100_resume(struct device *dev)
 		w5100_hw_start(priv);
 
 		netif_device_attach(ndev);
+
+		if (!priv->link_gpio ||
+		    gpiod_get_value_cansleep(priv->link_gpio) != 0)
+			netif_carrier_on(ndev);
 	}
+
 	return 0;
 }
 #endif /* CONFIG_PM_SLEEP */

Re: [PATCH] w5100: restore GPIO-based link detection

[Arnd Bergmann]

On Tue, Aug 4, 2026, at 16:38, Arthur Crépin Leblond wrote:
> Commit dacf281771a9 ("w5100: remove unused gpio link detection")
> dropped the link_gpio/link_irq handling on the grounds that no
> devicetree user passed a "link" GPIO at the time.
>
> Signed-off-by: Arthur Crépin Leblond <arthur@marmottus.net>

Hi Arthur,

The patch description could use some more explanation here, and
a clarification that you don't just bring back the original
broken code but add devicetree support for it.

>  .../devicetree/bindings/net/wiznet,w5x00.txt       |  8 ++-
>  drivers/net/ethernet/wiznet/w5100.c                | 84 ++++++++++++++++++++++
>  2 files changed, 89 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/wiznet,w5x00.txt 
> b/Documentation/devicetree/bindings/net/wiznet,w5x00.txt
> index e9665798c4be..e97ce3cb9183 100644
> --- a/Documentation/devicetree/bindings/net/wiznet,w5x00.txt
> +++ b/Documentation/devicetree/bindings/net/wiznet,w5x00.txt
> @@ -25,6 +25,7 @@ Optional properties:
>    According to the w5500 datasheet, the chip allows a maximum of 80 
> MHz, however,
>    board designs may need to limit this value.
>  - local-mac-address: See ethernet.txt in the same directory.
> +- link-gpios: a GPIO line used for the link detection interrupt
> 
> 
>  Example (for Raspberry Pi with pin control stuff for GPIO irq):
> @@ -38,13 +39,14 @@ Example (for Raspberry Pi with pin control stuff 
> for GPIO irq):
>  		interrupt-parent = <&gpio>;
>  		interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
>  		spi-max-frequency = <30000000>;
> +		link-gpios = <&gpio 4 GPIO_ACTIVE_HIGH>;
>  	};
>  };

Ok, so you are using the binding I suggested originally,
which I think is fine here, but note that Rob asked for
the binding to be converted to yaml format in
https://lore.kernel.org/all/20260427145010.GA2502144-robh@kernel.org/

I avoiding touching it by just removing the broken implementation,
but it would be good if you could do this now.

> +
> +		priv->link_irq = gpiod_to_irq(priv->link_gpio);
> +		if (priv->link_irq < 0) {
> +			err = priv->link_irq;
> +			goto err_gpio;
> +		}
> +
> +		err = request_any_context_irq(priv->link_irq, w5100_detect_link,
> +					      IRQF_TRIGGER_RISING |
> +						      IRQF_TRIGGER_FALLING,
> +					      link_name, priv->ndev);

I think you can just use a hardcoded link name here, and
an open-coded gpiod_to_irq(priv->link_gpio) for simplicity. My
previous version kept this from the original code, but if you
reintroduce it, you can improve it further (as you did elsewhere
already)

I would probably also use devm_request_threaded_irq()

> @@ -840,6 +918,7 @@ static int w5100_suspend(struct device *dev)
> 
>  	if (netif_running(ndev)) {
>  		netif_carrier_off(ndev);
> +
>  		netif_device_detach(ndev);
...
>  		w5100_hw_close(priv);
>  	}
> +
>  	return 0;

The whitespace changes should not be part of the patch.

     Arnd

Re: [PATCH] w5100: restore GPIO-based link detection

[Arthur Crépin Leblond]

On Tue, Aug 04, 2026 at 05:02:48PM +0200, Arnd Bergmann wrote:
>On Tue, Aug 4, 2026, at 16:38, Arthur Crépin Leblond wrote:
>> Commit dacf281771a9 ("w5100: remove unused gpio link detection")
>> dropped the link_gpio/link_irq handling on the grounds that no
>> devicetree user passed a "link" GPIO at the time.
>>
>> Signed-off-by: Arthur Crépin Leblond <arthur@marmottus.net>
>
>Hi Arthur,

Hi Arnd,

Thanks for the reply and the review!

>
>The patch description could use some more explanation here, and
>a clarification that you don't just bring back the original
>broken code but add devicetree support for it.
>
>>  .../devicetree/bindings/net/wiznet,w5x00.txt       |  8 ++-
>>  drivers/net/ethernet/wiznet/w5100.c                | 84 ++++++++++++++++++++++
>>  2 files changed, 89 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/net/wiznet,w5x00.txt
>> b/Documentation/devicetree/bindings/net/wiznet,w5x00.txt
>> index e9665798c4be..e97ce3cb9183 100644
>> --- a/Documentation/devicetree/bindings/net/wiznet,w5x00.txt
>> +++ b/Documentation/devicetree/bindings/net/wiznet,w5x00.txt
>> @@ -25,6 +25,7 @@ Optional properties:
>>    According to the w5500 datasheet, the chip allows a maximum of 80
>> MHz, however,
>>    board designs may need to limit this value.
>>  - local-mac-address: See ethernet.txt in the same directory.
>> +- link-gpios: a GPIO line used for the link detection interrupt
>>
>>
>>  Example (for Raspberry Pi with pin control stuff for GPIO irq):
>> @@ -38,13 +39,14 @@ Example (for Raspberry Pi with pin control stuff
>> for GPIO irq):
>>  		interrupt-parent = <&gpio>;
>>  		interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
>>  		spi-max-frequency = <30000000>;
>> +		link-gpios = <&gpio 4 GPIO_ACTIVE_HIGH>;
>>  	};
>>  };
>
>Ok, so you are using the binding I suggested originally,
>which I think is fine here, but note that Rob asked for
>the binding to be converted to yaml format in
>https://lore.kernel.org/all/20260427145010.GA2502144-robh@kernel.org/
>
>I avoiding touching it by just removing the broken implementation,
>but it would be good if you could do this now.

Yes, I started to reintroduce the driver link GPIO code from the 6.18 tree
and converted to gpiod_ and noticed that you already had a patch
(20230127095839.3266452-1-arnd@kernel.org) so I reused most of your code.

>
>> +
>> +		priv->link_irq = gpiod_to_irq(priv->link_gpio);
>> +		if (priv->link_irq < 0) {
>> +			err = priv->link_irq;
>> +			goto err_gpio;
>> +		}
>> +
>> +		err = request_any_context_irq(priv->link_irq, w5100_detect_link,
>> +					      IRQF_TRIGGER_RISING |
>> +						      IRQF_TRIGGER_FALLING,
>> +					      link_name, priv->ndev);
>
>I think you can just use a hardcoded link name here, and
>an open-coded gpiod_to_irq(priv->link_gpio) for simplicity. My
>previous version kept this from the original code, but if you
>reintroduce it, you can improve it further (as you did elsewhere
>already)
>
>I would probably also use devm_request_threaded_irq()
>
>> @@ -840,6 +918,7 @@ static int w5100_suspend(struct device *dev)
>>
>>  	if (netif_running(ndev)) {
>>  		netif_carrier_off(ndev);
>> +
>>  		netif_device_detach(ndev);
>...
>>  		w5100_hw_close(priv);
>>  	}
>> +
>>  	return 0;
>
>The whitespace changes should not be part of the patch.
>
>     Arnd

I took your changes into account and made a v2.

Thanks!

-- 
Arthur Crépin Leblond

Re: [PATCH] w5100: restore GPIO-based link detection

[Andrew Lunn]

On Tue, Aug 04, 2026 at 04:38:35PM +0200, Arthur Crépin Leblond wrote:
> Commit dacf281771a9 ("w5100: remove unused gpio link detection")
> dropped the link_gpio/link_irq handling on the grounds that no
> devicetree user passed a "link" GPIO at the time.
> 
> Signed-off-by: Arthur Crépin Leblond <arthur@marmottus.net>

Hi Arthur

Do you understand the architecture of this device? What exactly is on
the other end of this GPIO?

https://wiznet.io/products/ethernet-chips/w5100

suggests it has an integrated PHY. So why is a GPIO needed to report
link?

Thanks
	Andrew

Re: [PATCH] w5100: restore GPIO-based link detection

[Arthur Crépin Leblond]

On Tue, Aug 04, 2026 at 07:54:29PM +0200, Andrew Lunn wrote:
>On Tue, Aug 04, 2026 at 04:38:35PM +0200, Arthur Crépin Leblond wrote:
>> Commit dacf281771a9 ("w5100: remove unused gpio link detection")
>> dropped the link_gpio/link_irq handling on the grounds that no
>> devicetree user passed a "link" GPIO at the time.
>>
>> Signed-off-by: Arthur Crépin Leblond <arthur@marmottus.net>
>
>Hi Arthur
>
>Do you understand the architecture of this device? What exactly is on
>the other end of this GPIO?
>
>https://wiznet.io/products/ethernet-chips/w5100
>
>suggests it has an integrated PHY. So why is a GPIO needed to report
>link?
>
>Thanks
>	Andrew

Hi Andrew,

the W5100/W5500 exposes directly a LINKLED pin for the carrier status.
On my board (RPi), that pin is wired to a GPIO to detect changes on the host
directly via an interrupt.

Arthur

Re: [PATCH] w5100: restore GPIO-based link detection

[Arnd Bergmann]

On Wed, Aug 5, 2026, at 10:25, Arthur Crépin Leblond wrote:
> On Tue, Aug 04, 2026 at 07:54:29PM +0200, Andrew Lunn wrote:
>>
>>https://wiznet.io/products/ethernet-chips/w5100
>>
>>suggests it has an integrated PHY. So why is a GPIO needed to report
>>link?
>
> the W5100/W5500 exposes directly a LINKLED pin for the carrier status.
> On my board (RPi), that pin is wired to a GPIO to detect changes on the host
> directly via an interrupt.

The datasheet says

   LINKLED O 66 Link LED
      Active low in link state indicates a good status for
      10/100M.
      It is always ON when the link is OK and it flashes
      while in a TX or RX state.

which sounds like this is not a great way to do it, since any
data transfer would drop the link status. Are you sure the
gpio line as you connect it actually only reports link status
and not RX/TX? Which chip/revision specifically are you using?

With the W5300 driver (now removed) that was trying to use the
link gpio, the LINKLED description in the datasheet is different
and does not mention flashing, so on that one, the gpio link
interrupt was more likely to actually work.

      Arnd

Re: [PATCH] w5100: restore GPIO-based link detection

[Arthur Crépin Leblond]

On Wed, Aug 05, 2026 at 11:11:53AM +0200, Arnd Bergmann wrote:
>On Wed, Aug 5, 2026, at 10:25, Arthur Crépin Leblond wrote:
>> On Tue, Aug 04, 2026 at 07:54:29PM +0200, Andrew Lunn wrote:
>>>
>>>https://wiznet.io/products/ethernet-chips/w5100
>>>
>>>suggests it has an integrated PHY. So why is a GPIO needed to report
>>>link?
>>
>> the W5100/W5500 exposes directly a LINKLED pin for the carrier status.
>> On my board (RPi), that pin is wired to a GPIO to detect changes on the host
>> directly via an interrupt.
>
>The datasheet says
>
>   LINKLED O 66 Link LED
>      Active low in link state indicates a good status for
>      10/100M.
>      It is always ON when the link is OK and it flashes
>      while in a TX or RX state.
>
>which sounds like this is not a great way to do it, since any
>data transfer would drop the link status. Are you sure the
>gpio line as you connect it actually only reports link status
>and not RX/TX? Which chip/revision specifically are you using?
>
>With the W5300 driver (now removed) that was trying to use the
>link gpio, the LINKLED description in the datasheet is different
>and does not mention flashing, so on that one, the gpio link
>interrupt was more likely to actually work.
>
>      Arnd

You're right for the W5100 that would not make any sense during data
transfers it would trigger the interrupt. That's unreliable.

I am using the W5500, and in the datasheet it says

   Link LED
   This shows the Link status.
   Low: Link is established
   High: Link is not established

I can confirm it is what is happening, I don't see any changes
of state during TX/RX.

Arthur

Re: [PATCH] w5100: restore GPIO-based link detection

[Arnd Bergmann]

On Wed, Aug 5, 2026, at 11:44, Arthur Crépin Leblond wrote:
> On Wed, Aug 05, 2026 at 11:11:53AM +0200, Arnd Bergmann wrote:
> You're right for the W5100 that would not make any sense during data
> transfers it would trigger the interrupt. That's unreliable.
>
> I am using the W5500, and in the datasheet it says
>
>    Link LED
>    This shows the Link status.
>    Low: Link is established
>    High: Link is not established
>
> I can confirm it is what is happening, I don't see any changes
> of state during TX/RX.

Ok, good. On the other hand, the W5500 also has a PHYCFG register
that should tell you the link status without looking at the
GPIO line, though it's not clear if the CON/DISCON interrupt
fires on link state change in MACRAW mode.

It probably makes sense to wire up link w5100_get_link() to
the phy register for w5500 either way, as that works without
connecting a GPIO. Then you can just describe the LINKLED
signal as an optional interrupt in the DT binding to trigger
checking the link state in that register.

     Arnd

Re: [PATCH] w5100: restore GPIO-based link detection

[Arthur Crépin Leblond]

On Wed, Aug 05, 2026 at 12:46:14PM +0200, Arnd Bergmann wrote:
>On Wed, Aug 5, 2026, at 11:44, Arthur Crépin Leblond wrote:
>> On Wed, Aug 05, 2026 at 11:11:53AM +0200, Arnd Bergmann wrote:
>> You're right for the W5100 that would not make any sense during data
>> transfers it would trigger the interrupt. That's unreliable.
>>
>> I am using the W5500, and in the datasheet it says
>>
>>    Link LED
>>    This shows the Link status.
>>    Low: Link is established
>>    High: Link is not established
>>
>> I can confirm it is what is happening, I don't see any changes
>> of state during TX/RX.
>
>Ok, good. On the other hand, the W5500 also has a PHYCFG register
>that should tell you the link status without looking at the
>GPIO line, though it's not clear if the CON/DISCON interrupt
>fires on link state change in MACRAW mode.
>
>It probably makes sense to wire up link w5100_get_link() to
>the phy register for w5500 either way, as that works without
>connecting a GPIO. Then you can just describe the LINKLED
>signal as an optional interrupt in the DT binding to trigger
>checking the link state in that register.
>
>     Arnd

Seems to work, the PHYCFG[0] bit gets updated on link change.

Arthur