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

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

[Arthur Crépin 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 Crépin Leblond

Signed-off-by: Arthur Crépin Leblond <arthur@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 Crépin Leblond (3):
      dt-bindings: net: wiznet,w5100: convert to DT schema
      dt-bindings: net: wiznet,w5100: add link-gpios
      w5100: detect carrier state using link status bit

 .../devicetree/bindings/net/wiznet,w5100.yaml      | 74 ++++++++++++++++++++++
 .../devicetree/bindings/net/wiznet,w5x00.txt       | 50 ---------------
 drivers/net/ethernet/wiznet/w5100.c                | 70 ++++++++++++++++++++
 3 files changed, 144 insertions(+), 50 deletions(-)

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

[Arthur Crépin Leblond]

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

Signed-off-by: Arthur Crépin 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..641c9ddbde9e
--- /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:
+    maxItems: 1
+
+  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/gpio/gpio.h>
+    #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 v3 1/3] dt-bindings: net: wiznet,w5100: convert to DT schema

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Preexisting module removal race condition: `unregister_netdev()` is called after `destroy_workqueue()`, leading to potential use-after-free.
- [High] Preexisting probe race condition: `register_netdev()` is called too early, exposing the device to userspace before hardware and structures are initialized.
--

--- Patch [1]: [PATCH net-next v3 1/3] dt-bindings: net: wiznet,w5100: convert to DT schema ---
commit 6b1d871923a8b7c229c0db289868ab706e3e28d1
Author: Arthur Crépin 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..641c9ddbde9e7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/wiznet,w5100.yaml

[ ... ]

While reviewing this dt-bindings change, I noticed a couple of pre-existing
issues in the related driver code (drivers/net/ethernet/wiznet/w5100.c)
that might be worth addressing.

drivers/net/ethernet/wiznet/w5100.c:w5100_probe() {
    ...
	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));
    ...
}

[Severity: High]
This isn't a bug introduced by this patch, but is it safe to call
register_netdev() before fully initializing the device?

If userspace automation brings the interface up immediately after
registration, it could trigger a packet transmission. This would call
w5100_start_tx(), which attempts to queue work to priv->xfer_wq.

Since the workqueue and other hardware structures are initialized after
registration, could this result in a NULL pointer dereference?


drivers/net/ethernet/wiznet/w5100.c:w5100_remove() {
    ...
	flush_work(&priv->setrx_work);
	flush_work(&priv->restart_work);
	destroy_workqueue(priv->xfer_wq);

	unregister_netdev(ndev);
    ...
}

[Severity: High]
This is a pre-existing issue, but does destroying the workqueue before
calling unregister_netdev() create a race condition during module removal?

The unregister_netdev() function is responsible for safely stopping the
transmission queue and tearing down device visibility. If the workqueue is
destroyed first, there is a window where the network stack can concurrently
call ndo_start_xmit (w5100_start_tx()).

Can this lead to a use-after-free when w5100_start_tx() attempts to queue
work to the destroyed workqueue?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260806-wiznet-link-gpio-v3-0-532d4a143805@marmottus.net?part=1

[PATCH net-next v3 2/3] dt-bindings: net: wiznet,w5100: add link-gpios

[Arthur Crépin Leblond]

Add the link-gpios property which describes an optional GPIO line that
can be wired to the LINKLED pin of the w5500 as an interrupt to detect
link status changes.

Signed-off-by: Arthur Crépin Leblond <arthur@marmottus.net>
---
 Documentation/devicetree/bindings/net/wiznet,w5100.yaml | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/wiznet,w5100.yaml b/Documentation/devicetree/bindings/net/wiznet,w5100.yaml
index 641c9ddbde9e..cebf437ad3c4 100644
--- a/Documentation/devicetree/bindings/net/wiznet,w5100.yaml
+++ b/Documentation/devicetree/bindings/net/wiznet,w5100.yaml
@@ -35,6 +35,12 @@ properties:
       however, board designs may need to limit this value.
     maximum: 80000000
 
+  link-gpios:
+    description:
+      An optional GPIO line that can be wired to the LINKLED pin of the w5500
+      as an interrupt to detect link status changes.
+    maxItems: 1
+
 required:
   - compatible
   - reg
@@ -60,6 +66,7 @@ examples:
             pinctrl-0 = <&eth1_pins>;
             interrupt-parent = <&gpio>;
             interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
+            link-gpios = <&gpio 24 GPIO_ACTIVE_LOW>;
             spi-max-frequency = <30000000>;
             local-mac-address = [ a0 b0 c0 d0 e0 f0 ];
         };

-- 
2.55.0

[PATCH net-next v3 3/3] w5100: detect carrier state using link status bit

[Arthur Crépin Leblond]

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

Also add an optional GPIO line that can be wired to the LINKLED pin of
the w5500 as an interrupt to detect link status changes and read the
register.

On chip variants other than the w5500, which lack this register, the
link is always reported as up.

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. It uses the new managed GPIO
descriptors so that the resources are released automatically on driver
removal or probe failure.

Signed-off-by: Arthur Crépin Leblond <arthur@marmottus.net>
---
 drivers/net/ethernet/wiznet/w5100.c | 70 +++++++++++++++++++++++++++++++++++++
 1 file changed, 70 insertions(+)

diff --git a/drivers/net/ethernet/wiznet/w5100.c b/drivers/net/ethernet/wiznet/w5100.c
index 53d8dc642fbd..6707a6f27395 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"
 
@@ -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;
+	struct gpio_desc *link_gpio;
 
 	struct napi_struct napi;
 	struct net_device *ndev;
@@ -414,6 +418,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 w5100_read(priv, W5500_PHYCFGR) & PHYCFGR_LNK;
+
+	return 1;
+}
+
 static u32 w5100_get_msglevel(struct net_device *ndev)
 {
 	struct w5100_priv *priv = netdev_priv(ndev);
@@ -616,6 +630,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 +691,12 @@ static int w5100_open(struct net_device *ndev)
 	w5100_hw_start(priv);
 	napi_enable(&priv->napi);
 	netif_start_queue(ndev);
+
+	if (w5100_get_link(ndev))
+		netif_carrier_on(ndev);
+	else
+		netif_carrier_off(ndev);
+
 	return 0;
 }
 
@@ -678,6 +716,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 +790,13 @@ int w5100_probe(struct device *dev, const struct w5100_ops *ops,
 	priv->ndev = ndev;
 	priv->ops = ops;
 	priv->irq = irq;
+	priv->link_gpio = devm_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,8 +849,29 @@ int w5100_probe(struct device *dev, const struct w5100_ops *ops,
 	if (err)
 		goto err_hw;
 
+	if (priv->link_gpio) {
+		int link_irq = gpiod_to_irq(priv->link_gpio);
+
+		if (link_irq < 0) {
+			err = dev_err_probe(dev, link_irq,
+					    "No corresponding irq for link gpio\n");
+			goto err_gpio;
+		}
+
+		err = devm_request_threaded_irq(dev, link_irq, NULL,
+						w5100_detect_link,
+						IRQF_TRIGGER_RISING |
+						IRQF_TRIGGER_FALLING |
+						IRQF_ONESHOT,
+						"w5100-link", 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:
@@ -857,6 +924,9 @@ static int w5100_resume(struct device *dev)
 		w5100_hw_start(priv);
 
 		netif_device_attach(ndev);
+
+		if (w5100_get_link(ndev))
+			netif_carrier_on(ndev);
 	}
 	return 0;
 }

-- 
2.55.0

Re: [PATCH net-next v3 3/3] w5100: detect carrier state using link status bit

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:

New issues:
- [High] Using `devm_request_threaded_irq` for the link interrupt causes a use-after-free on module removal because the network device is manually freed before devres cleanup.
- [High] W5500 interface becomes permanently stuck with the carrier off if the cable is plugged in late and no optional link GPIO is configured.
- [High] Link detection is broken for W5100 and W5200 hardware when a link GPIO is used, as the driver always incorrectly reports the link as 'up' on every interrupt edge.
- [Medium] `w5100_get_link()` does not handle negative error codes from `w5100_read()`, causing SPI read failures to be misinterpreted as a valid link state.

Pre-existing issues:
- [High] Early device registration in `w5100_probe()` exposes uninitialized driver state and hardware to userspace.
- [High] `unregister_netdev()` is called after destroying the workqueue and freeing interrupts, allowing active network operations to use destroyed resources.
- [High] The probe error path unregisters and frees the netdev without canceling pending asynchronous works, leading to a Use-After-Free.
--

--- Patch [3]: [PATCH net-next v3 3/3] w5100: detect carrier state using link status bit ---
commit 6726aad63717b1d64239168658010e6e77a6636c
Author: Arthur Crépin Leblond <arthur@marmottus.net>

w5100: detect carrier state using link status bit

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

Also add an optional GPIO line that can be wired to the LINKLED pin of
the w5500 as an interrupt to detect link status changes and read the
register.

On chip variants other than the w5500, which lack this register, the
link is always reported as up.

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. It uses the new managed GPIO
descriptors so that the resources are released automatically on driver
removal or probe failure.

> diff --git a/drivers/net/ethernet/wiznet/w5100.c b/drivers/net/ethernet/wiznet/w5100.c
> index 53d8dc642fbd6..6707a6f273954 100644
> --- a/drivers/net/ethernet/wiznet/w5100.c
> +++ b/drivers/net/ethernet/wiznet/w5100.c

[ ... ]

> @@ -414,6 +418,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 w5100_read(priv, W5500_PHYCFGR) & PHYCFGR_LNK;

[Severity: Medium]
Does this correctly handle negative error codes from w5100_read()?

w5100_read() delegates to SPI read functions which can return negative error
codes (e.g., -EIO or -ENOMEM) upon transfer failure. Because negative numbers
are represented in two's complement, an error code like -EIO (-5, which is
0xFFFFFFFB) will have the lowest bit set.

Could this cause the driver to incorrectly interpret an SPI read failure as a
valid "link up" state?

> +
> +	return 1;
> +}
> +

[ ... ]

> @@ -616,6 +630,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)) {

[Severity: High]
For non-W5500 chips (like W5100 or W5200), w5100_get_link() unconditionally
returns 1.

Since the new link GPIO interrupt is configured to trigger on both rising
and falling edges, does this mean any link state change (even a cable unplug)
will be incorrectly interpreted as "link is up" on older chips because the
actual GPIO value is never read?

> +			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;
> +}
> +

[ ... ]

> @@ -659,6 +691,12 @@ static int w5100_open(struct net_device *ndev)
>  	w5100_hw_start(priv);
>  	napi_enable(&priv->napi);
>  	netif_start_queue(ndev);
> +
> +	if (w5100_get_link(ndev))
> +		netif_carrier_on(ndev);
> +	else
> +		netif_carrier_off(ndev);

[Severity: High]
If the cable is disconnected at open time, this explicitly turns the carrier
off.

If a board does not define the optional link GPIO in its devicetree, the
driver skips setting up the link IRQ. Since there is no fallback polling
timer implemented when the interrupt is unavailable, is there any mechanism
to detect when a cable is plugged in later, or does the interface remain
permanently stuck with the carrier off?

> +
>  	return 0;
>  }
>  

[ ... ]

> @@ -751,6 +790,13 @@ int w5100_probe(struct device *dev, const struct w5100_ops *ops,
>  	priv->ndev = ndev;
>  	priv->ops = ops;
>  	priv->irq = irq;
> +	priv->link_gpio = devm_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;
> +	}

[Severity: High]
This is a pre-existing issue, but does registering the netdev this early
expose uninitialized state to userspace?

Later in w5100_probe(), register_netdev() is called before critical
initialization steps like allocating priv->xfer_wq, executing INIT_WORK(),
and performing a hardware reset. 

If userspace attempts to use the device immediately after registration (e.g.,
via udev bringing the interface UP), could ndo_open() and transmit paths
execute on uninitialized hardware or attempt to queue work to a NULL
priv->xfer_wq?

[ ... ]

> @@ -803,8 +849,29 @@ int w5100_probe(struct device *dev, const struct w5100_ops *ops,
>  	if (err)
>  		goto err_hw;
>  
> +	if (priv->link_gpio) {
> +		int link_irq = gpiod_to_irq(priv->link_gpio);
> +
> +		if (link_irq < 0) {
> +			err = dev_err_probe(dev, link_irq,
> +					    "No corresponding irq for link gpio\n");
> +			goto err_gpio;
> +		}
> +
> +		err = devm_request_threaded_irq(dev, link_irq, NULL,
> +						w5100_detect_link,
> +						IRQF_TRIGGER_RISING |
> +						IRQF_TRIGGER_FALLING |
> +						IRQF_ONESHOT,
> +						"w5100-link", priv->ndev);

[Severity: High]
Can this devm-managed interrupt trigger a use-after-free on module removal?

The devm cleanup runs after the driver's w5100_remove() callback returns.
However, w5100_remove() calls free_netdev(ndev) to synchronously free the
network device before returning. 

If a link change triggers the interrupt during this window, wouldn't the
active threaded IRQ handler w5100_detect_link() execute and dereference the
already-freed ndev pointer?

> +		if (err < 0)
> +			goto err_gpio;
> +	}
> +
>  	return 0;
>  
> +err_gpio:
> +	free_irq(priv->irq, ndev);
>  err_hw:
>  	destroy_workqueue(priv->xfer_wq);

[Severity: High]
This is a pre-existing issue, but is it safe to destroy the workqueue before
unregistering the netdev?

By calling destroy_workqueue() before unregister_netdev(), the network
device is still registered and active while its workqueue is being destroyed.
Could a concurrent packet transmission attempt to queue work to the destroyed
workqueue?

>  err_wq:
>  	unregister_netdev(ndev);

[Severity: High]
This is a pre-existing issue, but does this error path leak pending
asynchronous work or cause a use-after-free?

If an error occurs after register_netdev() succeeds, the error path
unregisters and frees the netdev. Because the device was visible to userspace,
actions like ndo_set_rx_mode might have already scheduled priv->setrx_work
onto the global system workqueue.

Since cancel_work_sync() is never called for these global works before
free_netdev() is invoked, could the pending work execute later and
dereference freed memory?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260806-wiznet-link-gpio-v3-0-532d4a143805@marmottus.net?part=3