[PATCH net-next v5 0/3] w5100: restore GPIO-based link detection

[PATCH net-next v5 0/3] w5100: restore GPIO-based link detection

[Arthur Crepin Leblond]

Hi,

In the previous version of the w5100 driver, the LINKLED pin could be
wired to a GPIO to detect the link status changes via an interrupt.
This only works on w5500 as it is the only one documenting its LINKLED
pin to hold the link status (on w5100 the LINKLED pin is changing with
RX/TX activity).

This series of patches is bringing back the link status detection on
w5500 using the Link Status bit of the PHY Configuration Register.
Additionally, the LINKLED pin can also be wired to detect link status
changes and read the register in the interrupt handler.

Arthur Crepin Leblond

Signed-off-by: Arthur Crepin Leblond <arthur@marmottus.net>
---
Changes in v5:
- Remove gpio/gpio.h include from DT example
- Make the DT interrupts property dynamic from the compatible one
- Fix DT bindings errors
- Read the link status register only on w5500 instead of checking link_irq
- Address sashiko review
 - call netif_carrier_off on w5500 link status off in open
 - enable/disable the link irq in open/stop
 - enable/disable the link irq in resume/suspend
 - handle link irq probe defer error
 - register the netdev last
- Link to v4: https://patch.msgid.link/20260811-wiznet-link-gpio-v4-0-4d5f7da885a8@marmottus.net

Changes in v4:
- Use directly an interrupt line instead of gpio -> irq
- Update interrupts in DT bindings
- Address sashiko reviews
 - drop devm_ on request_threaded_irq to avoid use after free
 - disable/enable the link_irq in the suspend/resume
 - only call netif_carrier_on|off if the link interrupt is present
- Link to v3: https://patch.msgid.link/20260806-wiznet-link-gpio-v3-0-532d4a143805@marmottus.net

Changes in v3:
- Change cover letter
- Fix DT binding errors
- Use the Link Status bit of the PHY Configuration Register
- Use the LINKLED gpio binding for change detection only
- Link to v2: https://patch.msgid.link/20260804-wiznet-link-gpio-v2-0-3b1d0c870f35@marmottus.net

Changes in v2:
- Convert device tree binding to YAML
- Use devm_request_threaded_irq instead of request_any_context_irq
- Use devm_gpiod_get_optional instead of gpiod_get_optional
- Call dev_err_probe on gpiod_to_irq failure
- Remove link_irq from priv
- Use a fixed string for the IRQ name
- Remove empty new lines
- Link to v1: https://patch.msgid.link/20260804-wiznet-link-gpio-v1-1-b626fd4f7ccb@marmottus.net
---
Arthur Crepin Leblond (3):
      dt-bindings: net: wiznet,w5100: convert to DT schema
      dt-bindings: net: wiznet,w5100: add link status interrupt
      w5100: detect carrier state using link status bit and optional interrupt

 .../devicetree/bindings/net/wiznet,w5100.yaml      |  91 ++++++++++++++++
 .../devicetree/bindings/net/wiznet,w5x00.txt       |  50 ---------
 drivers/net/ethernet/wiznet/w5100.c                | 119 +++++++++++++++++++--
 3 files changed, 199 insertions(+), 61 deletions(-)

[PATCH net-next v5 1/3] dt-bindings: net: wiznet,w5100: convert to DT schema

[Arthur Crepin Leblond]

Convert the Wiznet w5x00 SPI Ethernet controller binding from
plain text to DT schema.

Signed-off-by: Arthur Crepin Leblond <arthur@marmottus.net>
---
 .../devicetree/bindings/net/wiznet,w5100.yaml      | 67 ++++++++++++++++++++++
 .../devicetree/bindings/net/wiznet,w5x00.txt       | 50 ----------------
 2 files changed, 67 insertions(+), 50 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/wiznet,w5100.yaml b/Documentation/devicetree/bindings/net/wiznet,w5100.yaml
new file mode 100644
index 000000000000..217f301db182
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/wiznet,w5100.yaml
@@ -0,0 +1,67 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/wiznet,w5100.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Wiznet w5100/w5200/w5500 SPI Ethernet Controller
+
+maintainers:
+  - Arthur Crépin Leblond <arthur@marmottus.net>
+
+description: |
+  This is a standalone 10/100 MBit Ethernet controller with SPI interface.
+
+allOf:
+  - $ref: /schemas/net/ethernet-controller.yaml#
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+properties:
+  compatible:
+    enum:
+      - wiznet,w5100
+      - wiznet,w5200
+      - wiznet,w5500
+
+  interrupts:
+    items:
+      - description: Main interrupt (INT pin on the device).
+
+  reg:
+    maxItems: 1
+
+  spi-max-frequency:
+    description:
+      According to the w5500 datasheet, the chip allows a maximum of 80 MHz,
+      however, board designs may need to limit this value.
+    maximum: 80000000
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    /* Example (for Raspberry Pi with pin control stuff for GPIO irq) */
+
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        ethernet@0 {
+            compatible = "wiznet,w5500";
+            reg = <0>;
+            pinctrl-names = "default";
+            pinctrl-0 = <&eth1_pins>;
+            interrupt-parent = <&gpio>;
+            interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
+            spi-max-frequency = <30000000>;
+            local-mac-address = [ a0 b0 c0 d0 e0 f0 ];
+        };
+    };
+...
diff --git a/Documentation/devicetree/bindings/net/wiznet,w5x00.txt b/Documentation/devicetree/bindings/net/wiznet,w5x00.txt
deleted file mode 100644
index e9665798c4be..000000000000
--- a/Documentation/devicetree/bindings/net/wiznet,w5x00.txt
+++ /dev/null
@@ -1,50 +0,0 @@
-* Wiznet w5x00
-
-This is a standalone 10/100 MBit Ethernet controller with SPI interface.
-
-For each device connected to a SPI bus, define a child node within
-the SPI master node.
-
-Required properties:
-- compatible: Should be one of the following strings:
-	      "wiznet,w5100"
-	      "wiznet,w5200"
-	      "wiznet,w5500"
-- reg: Specify the SPI chip select the chip is wired to.
-- interrupts: Specify the interrupt index within the interrupt controller (referred
-              to above in interrupt-parent) and interrupt type. w5x00 natively
-              generates falling edge interrupts, however, additional board logic
-              might invert the signal.
-- pinctrl-names: List of assigned state names, see pinctrl binding documentation.
-- pinctrl-0: List of phandles to configure the GPIO pin used as interrupt line,
-             see also generic and your platform specific pinctrl binding
-             documentation.
-
-Optional properties:
-- spi-max-frequency: Maximum frequency of the SPI bus when accessing the w5500.
-  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.
-
-
-Example (for Raspberry Pi with pin control stuff for GPIO irq):
-
-&spi {
-	ethernet@0: w5500@0 {
-		compatible = "wiznet,w5500";
-		reg = <0>;
-		pinctrl-names = "default";
-		pinctrl-0 = <&eth1_pins>;
-		interrupt-parent = <&gpio>;
-		interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
-		spi-max-frequency = <30000000>;
-	};
-};
-
-&gpio {
-	eth1_pins: eth1_pins {
-		brcm,pins = <25>;
-		brcm,function = <0>; /* in */
-		brcm,pull = <0>; /* none */
-	};
-};

-- 
2.55.0

Re: [PATCH net-next v5 1/3] dt-bindings: net: wiznet,w5100: convert to DT schema

[Krzysztof Kozlowski]

On Thu, Aug 13, 2026 at 10:58:14AM +0200, Arthur Crepin Leblond wrote:
> Convert the Wiznet w5x00 SPI Ethernet controller binding from
> plain text to DT schema.
> 
> Signed-off-by: Arthur Crepin Leblond <arthur@marmottus.net>
> ---
>  .../devicetree/bindings/net/wiznet,w5100.yaml      | 67 ++++++++++++++++++++++
>  .../devicetree/bindings/net/wiznet,w5x00.txt       | 50 ----------------
>  2 files changed, 67 insertions(+), 50 deletions(-)

Where did you explain reasons of dropping tags? Why would I need to do
the work twice?

<form letter>
This is a friendly reminder during the review process.

It looks like you received a tag and forgot to add it.

If you do not know the process, here is a short explanation:
Please add Acked-by/Reviewed-by/Tested-by tags when posting new
versions of patchset, under or above your Signed-off-by tag, unless
patch changed significantly (e.g. new properties added to the DT
bindings). Tag is "received", when provided in a message replied to you
on the mailing list. Tools like b4 can help here. However, there's no
need to repost patches *only* to add the tags. The upstream maintainer
will do that for tags received on the version they apply.

Please read:
https://elixir.bootlin.com/linux/v6.12-rc3/source/Documentation/process/submitting-patches.rst#L577

If a tag was not added on purpose, please state in the patch changelog
or cover letter why and what changed.
</form letter>

Best regards,
Krzysztof

Re: [PATCH net-next v5 1/3] dt-bindings: net: wiznet,w5100: convert to DT schema

[Arthur Crepin Leblond]

On 14 August 2026 09:15:27 CEST, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>On Thu, Aug 13, 2026 at 10:58:14AM +0200, Arthur Crepin Leblond wrote:
>> Convert the Wiznet w5x00 SPI Ethernet controller binding from
>> plain text to DT schema.
>> 
>> Signed-off-by: Arthur Crepin Leblond <arthur@marmottus.net>
>> ---
>>  .../devicetree/bindings/net/wiznet,w5100.yaml      | 67 ++++++++++++++++++++++
>>  .../devicetree/bindings/net/wiznet,w5x00.txt       | 50 ----------------
>>  2 files changed, 67 insertions(+), 50 deletions(-)
>
>Where did you explain reasons of dropping tags? Why would I need to do
>the work twice?
>
Hi,

I did remove the tag as I made a small change by removing the unused
gpio/gpio.h include and commented in the cover letter changelog.

Apologies, I found it fair to request a new review not to go behind
your back but it was indeed a very insignificant change and I did not
give enough context in the changelog.

Arthur

Re: [PATCH net-next v5 1/3] dt-bindings: net: wiznet,w5100: convert to DT schema

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The YAML device tree example will fail compilation during `make dt_binding_check` due to unresolved phandles (`<&eth1_pins>` and `<&gpio>`).
--

--- Patch [1]: [PATCH net-next v5 1/3] dt-bindings: net: wiznet,w5100: convert to DT schema ---
commit fcc3d575c269eac63488595e1b5b27fd36f552f7
Author: Arthur Crepin Leblond <arthur@marmottus.net>

dt-bindings: net: wiznet,w5100: convert to DT schema

Convert the Wiznet w5x00 SPI Ethernet controller binding from
plain text to DT schema.

> diff --git a/Documentation/devicetree/bindings/net/wiznet,w5100.yaml b/Documentation/devicetree/bindings/net/wiznet,w5100.yaml
> new file mode 100644
> index 0000000000000..217f301db182b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/wiznet,w5100.yaml

[ ... ]

> +examples:
> +  - |
> +    /* Example (for Raspberry Pi with pin control stuff for GPIO irq) */
> +
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        ethernet@0 {
> +            compatible = "wiznet,w5500";
> +            reg = <0>;
> +            pinctrl-names = "default";
> +            pinctrl-0 = <&eth1_pins>;
> +            interrupt-parent = <&gpio>;

[Severity: Low]
Will this example fail compilation during make dt_binding_check due to
unresolved phandles? It looks like &eth1_pins and &gpio are referenced
here, but their node definitions were not carried over from the original
text binding.

> +            interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
> +            spi-max-frequency = <30000000>;
> +            local-mac-address = [ a0 b0 c0 d0 e0 f0 ];
> +        };
> +    };
> +...

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260813-wiznet-link-gpio-v5-0-ac8046f24413@marmottus.net?part=1

[PATCH net-next v5 2/3] dt-bindings: net: wiznet,w5100: add link status interrupt

[Arthur Crepin Leblond]

Add an optional link status interrupt which can be connected to the
LINKLED pin on the device to detect link status changes.

Signed-off-by: Arthur Crepin Leblond <arthur@marmottus.net>
---
 .../devicetree/bindings/net/wiznet,w5100.yaml      | 34 ++++++++++++++++++----
 1 file changed, 29 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/wiznet,w5100.yaml b/Documentation/devicetree/bindings/net/wiznet,w5100.yaml
index 217f301db182..81f4382eb96c 100644
--- a/Documentation/devicetree/bindings/net/wiznet,w5100.yaml
+++ b/Documentation/devicetree/bindings/net/wiznet,w5100.yaml
@@ -15,6 +15,33 @@ description: |
 allOf:
   - $ref: /schemas/net/ethernet-controller.yaml#
   - $ref: /schemas/spi/spi-peripheral-props.yaml#
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - wiznet,w5500
+    then:
+      properties:
+        interrupts:
+          minItems: 1
+          items:
+            - description: Main interrupt (INT pin on the device).
+            - description:
+                Optional interrupt for link status changes (LINKLED pin on
+                w5500).
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - wiznet,w5100
+              - wiznet,w5200
+    then:
+      properties:
+        interrupts:
+          items:
+            - description: Main interrupt (INT pin on the device).
 
 properties:
   compatible:
@@ -23,10 +50,6 @@ properties:
       - wiznet,w5200
       - wiznet,w5500
 
-  interrupts:
-    items:
-      - description: Main interrupt (INT pin on the device).
-
   reg:
     maxItems: 1
 
@@ -59,7 +82,8 @@ examples:
             pinctrl-names = "default";
             pinctrl-0 = <&eth1_pins>;
             interrupt-parent = <&gpio>;
-            interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
+            interrupts = <25 IRQ_TYPE_EDGE_FALLING>,
+                         <24 IRQ_TYPE_EDGE_BOTH>;
             spi-max-frequency = <30000000>;
             local-mac-address = [ a0 b0 c0 d0 e0 f0 ];
         };

-- 
2.55.0

Re: [PATCH net-next v5 2/3] dt-bindings: net: wiznet,w5100: add link status interrupt

[Krzysztof Kozlowski]

On Thu, Aug 13, 2026 at 10:58:15AM +0200, Arthur Crepin Leblond wrote:
> Add an optional link status interrupt which can be connected to the
> LINKLED pin on the device to detect link status changes.
> 
> Signed-off-by: Arthur Crepin Leblond <arthur@marmottus.net>
> ---
>  .../devicetree/bindings/net/wiznet,w5100.yaml      | 34 ++++++++++++++++++----
>  1 file changed, 29 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/wiznet,w5100.yaml b/Documentation/devicetree/bindings/net/wiznet,w5100.yaml
> index 217f301db182..81f4382eb96c 100644
> --- a/Documentation/devicetree/bindings/net/wiznet,w5100.yaml
> +++ b/Documentation/devicetree/bindings/net/wiznet,w5100.yaml
> @@ -15,6 +15,33 @@ description: |
>  allOf:
>    - $ref: /schemas/net/ethernet-controller.yaml#
>    - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - wiznet,w5500
> +    then:
> +      properties:
> +        interrupts:
> +          minItems: 1
> +          items:
> +            - description: Main interrupt (INT pin on the device).
> +            - description:
> +                Optional interrupt for link status changes (LINKLED pin on
> +                w5500).
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - wiznet,w5100
> +              - wiznet,w5200
> +    then:
> +      properties:
> +        interrupts:
> +          items:
> +            - description: Main interrupt (INT pin on the device).

allOf goes under required: block.

>  
>  properties:
>    compatible:
> @@ -23,10 +50,6 @@ properties:
>        - wiznet,w5200
>        - wiznet,w5500
>  
> -  interrupts:
> -    items:
> -      - description: Main interrupt (INT pin on the device).
> -

Properties are always defined in top-level

I do not understand why you made such changes. I did not ask for them.
Read again WHERE exactly my comment appears.

Best regards,
Krzysztof

Re: [PATCH net-next v5 2/3] dt-bindings: net: wiznet,w5100: add link status interrupt

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Removing the `interrupts` property from the top-level `properties` block without explicitly specifying `maxItems` in the conditional branches removes the upper bound on the array length, allowing invalid configurations to pass validation.
--

--- Patch [2]: [PATCH net-next v5 2/3] dt-bindings: net: wiznet,w5100: add link status interrupt ---
commit 27905addd6c59c9b61baecb1400ea1f5c8b0242f
Author: Arthur Crepin Leblond <arthur@marmottus.net>

dt-bindings: net: wiznet,w5100: add link status interrupt

Add an optional link status interrupt which can be connected to the
LINKLED pin on the device to detect link status changes.

> diff --git a/Documentation/devicetree/bindings/net/wiznet,w5100.yaml b/Documentation/devicetree/bindings/net/wiznet,w5100.yaml
> index 217f301db182b..81f4382eb96c5 100644
> --- a/Documentation/devicetree/bindings/net/wiznet,w5100.yaml
> +++ b/Documentation/devicetree/bindings/net/wiznet,w5100.yaml
> @@ -15,6 +15,33 @@ description: |
>  allOf:
>    - $ref: /schemas/net/ethernet-controller.yaml#
>    - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - wiznet,w5500
> +    then:
> +      properties:
> +        interrupts:
> +          minItems: 1
> +          items:
> +            - description: Main interrupt (INT pin on the device).
> +            - description:
> +                Optional interrupt for link status changes (LINKLED pin on
> +                w5500).

[Severity: Medium]
Does this leave the array length unbounded for the wiznet,w5500 conditional
block?

Because the items keyword in standard JSON Schema only applies constraints to
the specified indices without capping the array size, it appears maxItems: 2
is missing here. 

This could allow device tree configurations with 3 or more extraneous
interrupts elements to silently pass schema validation.

> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - wiznet,w5100
> +              - wiznet,w5200
> +    then:
> +      properties:
> +        interrupts:
> +          items:
> +            - description: Main interrupt (INT pin on the device).

[Severity: Medium]
Does this branch for wiznet,w5100 and wiznet,w5200 also require an explicit
maxItems: 1 constraint?

Without an explicit cap, invalid device tree configurations specifying 2 or
more interrupts for these devices could similarly go undetected.

>  properties:
>    compatible:
> @@ -23,10 +50,6 @@ properties:
>        - wiznet,w5200
>        - wiznet,w5500
>  
> -  interrupts:
> -    items:
> -      - description: Main interrupt (INT pin on the device).
> -

[Severity: Medium]
Does removing the interrupts property from this top-level properties block
prevent dt-schema from automatically inferring and injecting a maxItems
constraint?

Leaving maxItems unspecified in the allOf conditional branches while removing
it from the top-level block removes the upper bound on the array length,
allowing invalid hardware descriptions to go undetected during dtbs_check.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260813-wiznet-link-gpio-v5-0-ac8046f24413@marmottus.net?part=2

[PATCH net-next v5 3/3] w5100: detect carrier state using link status bit and optional interrupt

[Arthur Crepin Leblond]

Detect the link status on the w5500 using the Link Status bit from the
PHY Configuration register (PHYCFGR).

On w5100/w5200, which lack this register, the link is always reported
as up.

Also add an optional interrupt, wired to the LINKLED pin of
the w5500, to detect link status changes and read the PHYCFGR register.

Make sure that the netdev is registered last in the probe function so
no operation can be performed before the probing is finished.

The w5500 interrupt is only enabled when the netdev is opened and
disabled on stop. Same behavior for suspend/resume.

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 and that it used the
old gpio interface.
This isn't a plain revert of that removal: link detection is now done
using a second interrupt rather than a gpio with a documented DT binding.

Signed-off-by: Arthur Crepin Leblond <arthur@marmottus.net>
---
 drivers/net/ethernet/wiznet/w5100.c | 119 ++++++++++++++++++++++++++++++++----
 1 file changed, 108 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/wiznet/w5100.c b/drivers/net/ethernet/wiznet/w5100.c
index 53d8dc642fbd..b5976d5e6c60 100644
--- a/drivers/net/ethernet/wiznet/w5100.c
+++ b/drivers/net/ethernet/wiznet/w5100.c
@@ -21,6 +21,7 @@
 #include <linux/io.h>
 #include <linux/ioport.h>
 #include <linux/interrupt.h>
+#include <linux/property.h>
 #include <linux/irq.h>
 
 #include "w5100.h"
@@ -124,6 +125,8 @@ MODULE_LICENSE("GPL");
  */
 #define W5500_SIMR		0x0018 /* Socket Interrupt Mask Register */
 #define W5500_RTR		0x0019 /* Retry Time-value Register */
+#define W5500_PHYCFGR		0x002e /* PHY Configuration Register */
+#define   PHYCFGR_LNK		  0x01 /* Link status */
 
 #define W5500_S0_REGS		0x10000
 
@@ -154,6 +157,7 @@ struct w5100_priv {
 	u16 s0_rx_buf_size;
 
 	int irq;
+	int link_irq;
 
 	struct napi_struct napi;
 	struct net_device *ndev;
@@ -345,6 +349,20 @@ static void w5500_memory_configure(struct w5100_priv *priv)
 	}
 }
 
+static int w5500_get_phycfgr_lnk(struct net_device *ndev)
+{
+	struct w5100_priv *priv = netdev_priv(ndev);
+	int ret = w5100_read(priv, W5500_PHYCFGR);
+
+	if (ret < 0) {
+		netif_err(priv, link, ndev,
+			  "failed to read link status: %d\n", ret);
+		return 0;
+	}
+
+	return ret & PHYCFGR_LNK;
+}
+
 static int w5100_hw_reset(struct w5100_priv *priv)
 {
 	u32 rtr;
@@ -414,6 +432,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->ops->chip_id == W5500)
+		return w5500_get_phycfgr_lnk(ndev);
+
+	return 1;
+}
+
 static u32 w5100_get_msglevel(struct net_device *ndev)
 {
 	struct w5100_priv *priv = netdev_priv(ndev);
@@ -616,6 +644,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 (w5100_get_link(ndev)) {
+			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 +705,15 @@ static int w5100_open(struct net_device *ndev)
 	w5100_hw_start(priv);
 	napi_enable(&priv->napi);
 	netif_start_queue(ndev);
+
+	if (priv->link_irq >= 0)
+		enable_irq(priv->link_irq);
+
+	if (priv->ops->chip_id != W5500 || w5500_get_phycfgr_lnk(ndev))
+		netif_carrier_on(ndev);
+	else
+		netif_carrier_off(ndev);
+
 	return 0;
 }
 
@@ -667,6 +722,10 @@ static int w5100_stop(struct net_device *ndev)
 	struct w5100_priv *priv = netdev_priv(ndev);
 
 	netif_info(priv, ifdown, ndev, "shutting down\n");
+
+	if (priv->link_irq >= 0)
+		disable_irq(priv->link_irq);
+
 	w5100_hw_close(priv);
 	netif_carrier_off(ndev);
 	netif_stop_queue(ndev);
@@ -678,6 +737,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,
 };
@@ -745,13 +805,21 @@ int w5100_probe(struct device *dev, const struct w5100_ops *ops,
 		break;
 	default:
 		err = -EINVAL;
-		goto err_register;
+		goto err_wq;
 	}
 
 	priv->ndev = ndev;
 	priv->ops = ops;
 	priv->irq = irq;
 
+	priv->link_irq = ops->chip_id == W5500 ?
+			 fwnode_irq_get(dev_fwnode(dev), 1) : -ENODEV;
+	if (priv->link_irq == -EPROBE_DEFER) {
+		err = dev_err_probe(dev, priv->link_irq,
+				    "failed to get link irq\n");
+		goto err_wq;
+	}
+
 	ndev->netdev_ops = &w5100_netdev_ops;
 	ndev->ethtool_ops = &w5100_ethtool_ops;
 	netif_napi_add_weight(ndev, &priv->napi, w5100_napi_poll, 16);
@@ -761,12 +829,8 @@ int w5100_probe(struct device *dev, const struct w5100_ops *ops,
 	 */
 	ndev->features |= NETIF_F_VLAN_CHALLENGED;
 
-	err = register_netdev(ndev);
-	if (err < 0)
-		goto err_register;
-
 	priv->xfer_wq = alloc_workqueue("%s", WQ_MEM_RECLAIM | WQ_PERCPU, 0,
-					netdev_name(ndev));
+					dev_name(dev));
 	if (!priv->xfer_wq) {
 		err = -ENOMEM;
 		goto err_wq;
@@ -795,21 +859,39 @@ int w5100_probe(struct device *dev, const struct w5100_ops *ops,
 	if (ops->may_sleep) {
 		err = request_threaded_irq(priv->irq, NULL, w5100_interrupt,
 					   IRQF_TRIGGER_LOW | IRQF_ONESHOT,
-					   netdev_name(ndev), ndev);
+					   dev_name(dev), ndev);
 	} else {
 		err = request_irq(priv->irq, w5100_interrupt,
-				  IRQF_TRIGGER_LOW, netdev_name(ndev), ndev);
+				  IRQF_TRIGGER_LOW, dev_name(dev), ndev);
 	}
 	if (err)
 		goto err_hw;
 
+	if (priv->link_irq >= 0) {
+		err = request_threaded_irq(priv->link_irq, NULL,
+					   w5100_detect_link,
+					   IRQF_TRIGGER_RISING |
+					   IRQF_TRIGGER_FALLING |
+					   IRQF_ONESHOT | IRQF_NO_AUTOEN,
+					   "w5100-link", priv->ndev);
+		if (err < 0)
+			goto err_link_irq;
+	}
+
+	err = register_netdev(ndev);
+	if (err < 0)
+		goto err_register;
+
 	return 0;
 
+err_register:
+	if (priv->link_irq >= 0)
+		free_irq(priv->link_irq, ndev);
+err_link_irq:
+	free_irq(priv->irq, ndev);
 err_hw:
 	destroy_workqueue(priv->xfer_wq);
 err_wq:
-	unregister_netdev(ndev);
-err_register:
 	free_netdev(ndev);
 	return err;
 }
@@ -820,6 +902,13 @@ void w5100_remove(struct device *dev)
 	struct net_device *ndev = dev_get_drvdata(dev);
 	struct w5100_priv *priv = netdev_priv(ndev);
 
+	unregister_netdev(ndev);
+
+	if (priv->link_irq >= 0) {
+		free_irq(priv->link_irq, ndev);
+		priv->link_irq = -ENODEV;
+	}
+
 	w5100_hw_reset(priv);
 	free_irq(priv->irq, ndev);
 
@@ -827,7 +916,6 @@ void w5100_remove(struct device *dev)
 	flush_work(&priv->restart_work);
 	destroy_workqueue(priv->xfer_wq);
 
-	unregister_netdev(ndev);
 	free_netdev(ndev);
 }
 EXPORT_SYMBOL_GPL(w5100_remove);
@@ -839,6 +927,9 @@ static int w5100_suspend(struct device *dev)
 	struct w5100_priv *priv = netdev_priv(ndev);
 
 	if (netif_running(ndev)) {
+		if (priv->link_irq >= 0)
+			disable_irq(priv->link_irq);
+
 		netif_carrier_off(ndev);
 		netif_device_detach(ndev);
 
@@ -857,6 +948,12 @@ static int w5100_resume(struct device *dev)
 		w5100_hw_start(priv);
 
 		netif_device_attach(ndev);
+
+		if (priv->link_irq >= 0)
+			enable_irq(priv->link_irq);
+
+		if (priv->ops->chip_id != W5500 || w5500_get_phycfgr_lnk(ndev))
+			netif_carrier_on(ndev);
 	}
 	return 0;
 }

-- 
2.55.0

Re: [PATCH net-next v5 3/3] w5100: detect carrier state using link status bit and optional interrupt

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] W5500 devices without the optional link interrupt can become permanently stuck with the carrier off if initialized while disconnected.
- [High] Reordering unregister_netdev() before flush_work() leaves the hardware active and unmasked after driver removal.
- [Medium] A Time-Of-Check to Time-Of-Use (TOCTOU) race condition in link state initialization.
--

--- Patch [3]: [PATCH net-next v5 3/3] w5100: detect carrier state using link status bit and optional interrupt ---
commit d746a0d78f3b21d66ca77ce59de3867ee0ba026e
Author: Arthur Crepin Leblond <arthur@marmottus.net>

w5100: detect carrier state using link status bit and optional interrupt
    
Detect the link status on the w5500 using the Link Status bit from the
PHY Configuration register. Add an optional interrupt to detect link
status changes and read the PHYCFGR register. Make sure the netdev is
registered last in the probe function.

> diff --git a/drivers/net/ethernet/wiznet/w5100.c b/drivers/net/ethernet/wiznet/w5100.c
> index 53d8dc642fbd6..b5976d5e6c60c 100644
> --- a/drivers/net/ethernet/wiznet/w5100.c
> +++ b/drivers/net/ethernet/wiznet/w5100.c
[ ... ]
> @@ -659,6 +705,15 @@ static int w5100_open(struct net_device *ndev)
>  	w5100_hw_start(priv);
>  	napi_enable(&priv->napi);
>  	netif_start_queue(ndev);
> +
> +	if (priv->link_irq >= 0)
> +		enable_irq(priv->link_irq);
> +
> +	if (priv->ops->chip_id != W5500 || w5500_get_phycfgr_lnk(ndev))
> +		netif_carrier_on(ndev);
> +	else
> +		netif_carrier_off(ndev);
> +

[Severity: Medium]
Could this introduce a race condition during link state initialization?

If the physical link changes state right after enable_irq() is called but
before the manual PHY state check:

CPU1                                      CPU2
w5100_open()
   enable_irq()
                                          w5100_detect_link()
                                             w5100_get_link() -> link up
                                             netif_carrier_on()
   w5500_get_phycfgr_lnk() -> link down
   netif_carrier_off()

Will this leave the interface permanently stuck in a no-carrier state despite
having a valid physical link?

[Severity: High]
Does this code permanently break connectivity for setups without a link
interrupt?

If the device tree does not provide the optional link_irq, and the driver is
initialized while the physical cable is disconnected, netif_carrier_off() is
called here.

Since there is no polling mechanism to detect a subsequent link-up event, how
will the carrier ever be turned back on once the cable is plugged in?

>  	return 0;
>  }
[ ... ]
> @@ -820,6 +902,13 @@ void w5100_remove(struct device *dev)
>  	struct net_device *ndev = dev_get_drvdata(dev);
>  	struct w5100_priv *priv = netdev_priv(ndev);
>  
> +	unregister_netdev(ndev);
> +
> +	if (priv->link_irq >= 0) {
> +		free_irq(priv->link_irq, ndev);
> +		priv->link_irq = -ENODEV;
> +	}
> +
>  	w5100_hw_reset(priv);
>  	free_irq(priv->irq, ndev);
>  
> @@ -827,7 +916,6 @@ void w5100_remove(struct device *dev)
>  	flush_work(&priv->restart_work);
>  	destroy_workqueue(priv->xfer_wq);
>  
> -	unregister_netdev(ndev);
>  	free_netdev(ndev);
>  }

[Severity: High]
Can this reordering leave the hardware active after the driver is removed?

unregister_netdev() was moved to the top of w5100_remove(), which gracefully
stops the hardware early via ndo_stop(). 

However, flush_work() is called later in the teardown process:

w5100_remove()
   unregister_netdev()
      ...
         w5100_stop()
            w5100_hw_close()
   ...
   flush_work(&priv->setrx_work)
      w5100_setrx_work()
         w5100_hw_start()

If setrx_work or restart_work are pending in the workqueue, they will execute
and call w5100_hw_start(), which re-enables hardware interrupts and opens the
socket after the OS interrupt handler has been freed. Is it safe to execute
flush_work() after unregister_netdev()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260813-wiznet-link-gpio-v5-0-ac8046f24413@marmottus.net?part=3

Re: [PATCH net-next v5 3/3] w5100: detect carrier state using link status bit and optional interrupt

[Arthur Crepin Leblond]

On 13 August 2026 10:58:16 CEST, Arthur Crepin Leblond <arthur@marmottus.net> wrote:
>Detect the link status on the w5500 using the Link Status bit from the
>PHY Configuration register (PHYCFGR).
>
Ignore this patch, I went for some changes based on sashiko review but introduced some extra issue when on w5500 but without the link irq set.