The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH net-next v4 0/3] w5100: restore GPIO-based link detection
@ 2026-08-11 12:34 Arthur Crépin Leblond
  2026-08-11 12:34 ` [PATCH net-next v4 1/3] dt-bindings: net: wiznet,w5100: convert to DT schema Arthur Crépin Leblond
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Arthur Crépin Leblond @ 2026-08-11 12:34 UTC (permalink / raw)
  To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: Arnd Bergmann, netdev, devicetree, linux-kernel,
	Arthur Crépin Leblond, Krzysztof Kozlowski

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 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 Crépin 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      | 72 ++++++++++++++++++++++
 .../devicetree/bindings/net/wiznet,w5x00.txt       | 50 ---------------
 drivers/net/ethernet/wiznet/w5100.c                | 68 ++++++++++++++++++++
 3 files changed, 140 insertions(+), 50 deletions(-)


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH net-next v4 1/3] dt-bindings: net: wiznet,w5100: convert to DT schema
  2026-08-11 12:34 [PATCH net-next v4 0/3] w5100: restore GPIO-based link detection Arthur Crépin Leblond
@ 2026-08-11 12:34 ` Arthur Crépin Leblond
  2026-08-11 12:57   ` Arnd Bergmann
  2026-08-11 12:34 ` [PATCH net-next v4 2/3] dt-bindings: net: wiznet,w5100: add link status interrupt Arthur Crépin Leblond
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ messages in thread
From: Arthur Crépin Leblond @ 2026-08-11 12:34 UTC (permalink / raw)
  To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: Arnd Bergmann, netdev, devicetree, linux-kernel,
	Arthur Crépin Leblond, Krzysztof Kozlowski

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

Signed-off-by: Arthur Crépin Leblond <arthur@marmottus.net>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
---
 .../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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH net-next v4 2/3] dt-bindings: net: wiznet,w5100: add link status interrupt
  2026-08-11 12:34 [PATCH net-next v4 0/3] w5100: restore GPIO-based link detection Arthur Crépin Leblond
  2026-08-11 12:34 ` [PATCH net-next v4 1/3] dt-bindings: net: wiznet,w5100: convert to DT schema Arthur Crépin Leblond
@ 2026-08-11 12:34 ` Arthur Crépin Leblond
  2026-08-11 13:33   ` Rob Herring (Arm)
  2026-08-11 13:36   ` Krzysztof Kozlowski
  2026-08-11 12:34 ` [PATCH net-next v4 3/3] w5100: detect carrier state using link status bit and optional interrupt Arthur Crépin Leblond
  2026-08-11 13:01 ` [PATCH net-next v4 0/3] w5100: restore GPIO-based link detection Arnd Bergmann
  3 siblings, 2 replies; 12+ messages in thread
From: Arthur Crépin Leblond @ 2026-08-11 12:34 UTC (permalink / raw)
  To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: Arnd Bergmann, netdev, devicetree, linux-kernel,
	Arthur Crépin 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 Crépin Leblond <arthur@marmottus.net>
---
 Documentation/devicetree/bindings/net/wiznet,w5100.yaml | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/wiznet,w5100.yaml b/Documentation/devicetree/bindings/net/wiznet,w5100.yaml
index 641c9ddbde9e..30f013f40a77 100644
--- a/Documentation/devicetree/bindings/net/wiznet,w5100.yaml
+++ b/Documentation/devicetree/bindings/net/wiznet,w5100.yaml
@@ -24,7 +24,11 @@ properties:
       - wiznet,w5500
 
   interrupts:
-    maxItems: 1
+    maxItems: 2
+    items:
+      - description: Main interrupt (INT pin on the device).
+      - description:
+          Optional interrupt for link status changes (LINKLED pin on w5500).
 
   reg:
     maxItems: 1
@@ -59,7 +63,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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH net-next v4 3/3] w5100: detect carrier state using link status bit and optional interrupt
  2026-08-11 12:34 [PATCH net-next v4 0/3] w5100: restore GPIO-based link detection Arthur Crépin Leblond
  2026-08-11 12:34 ` [PATCH net-next v4 1/3] dt-bindings: net: wiznet,w5100: convert to DT schema Arthur Crépin Leblond
  2026-08-11 12:34 ` [PATCH net-next v4 2/3] dt-bindings: net: wiznet,w5100: add link status interrupt Arthur Crépin Leblond
@ 2026-08-11 12:34 ` Arthur Crépin Leblond
  2026-08-11 13:01 ` [PATCH net-next v4 0/3] w5100: restore GPIO-based link detection Arnd Bergmann
  3 siblings, 0 replies; 12+ messages in thread
From: Arthur Crépin Leblond @ 2026-08-11 12:34 UTC (permalink / raw)
  To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: Arnd Bergmann, netdev, devicetree, linux-kernel,
	Arthur Crépin 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.

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 Crépin Leblond <arthur@marmottus.net>
---
 drivers/net/ethernet/wiznet/w5100.c | 68 +++++++++++++++++++++++++++++++++++++
 1 file changed, 68 insertions(+)

diff --git a/drivers/net/ethernet/wiznet/w5100.c b/drivers/net/ethernet/wiznet/w5100.c
index 53d8dc642fbd..ca5054c20993 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;
@@ -414,6 +418,20 @@ 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) {
+		int ret = w5100_read(priv, W5500_PHYCFGR);
+
+		if (ret >= 0)
+			return ret & PHYCFGR_LNK;
+	}
+
+	return 1;
+}
+
 static u32 w5100_get_msglevel(struct net_device *ndev)
 {
 	struct w5100_priv *priv = netdev_priv(ndev);
@@ -616,6 +634,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 +695,10 @@ 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 || w5100_get_link(ndev))
+		netif_carrier_on(ndev);
+
 	return 0;
 }
 
@@ -678,6 +718,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 +792,8 @@ int w5100_probe(struct device *dev, const struct w5100_ops *ops,
 	priv->ndev = ndev;
 	priv->ops = ops;
 	priv->irq = irq;
+	priv->link_irq = ops->chip_id == W5500 ?
+			 fwnode_irq_get(dev_fwnode(dev), 1) : -ENODEV;
 
 	ndev->netdev_ops = &w5100_netdev_ops;
 	ndev->ethtool_ops = &w5100_ethtool_ops;
@@ -803,8 +846,21 @@ int w5100_probe(struct device *dev, const struct w5100_ops *ops,
 	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,
+					   "w5100-link", priv->ndev);
+		if (err < 0)
+			goto err_link_irq;
+	}
+
 	return 0;
 
+err_link_irq:
+	free_irq(priv->irq, ndev);
 err_hw:
 	destroy_workqueue(priv->xfer_wq);
 err_wq:
@@ -820,6 +876,9 @@ void w5100_remove(struct device *dev)
 	struct net_device *ndev = dev_get_drvdata(dev);
 	struct w5100_priv *priv = netdev_priv(ndev);
 
+	if (priv->link_irq >= 0)
+		free_irq(priv->link_irq, ndev);
+
 	w5100_hw_reset(priv);
 	free_irq(priv->irq, ndev);
 
@@ -842,6 +901,9 @@ static int w5100_suspend(struct device *dev)
 		netif_carrier_off(ndev);
 		netif_device_detach(ndev);
 
+		if (priv->link_irq >= 0)
+			disable_irq(priv->link_irq);
+
 		w5100_hw_close(priv);
 	}
 	return 0;
@@ -856,7 +918,13 @@ static int w5100_resume(struct device *dev)
 		w5100_hw_reset(priv);
 		w5100_hw_start(priv);
 
+		if (priv->link_irq >= 0)
+			enable_irq(priv->link_irq);
+
 		netif_device_attach(ndev);
+
+		if (priv->link_irq < 0 || w5100_get_link(ndev))
+			netif_carrier_on(ndev);
 	}
 	return 0;
 }

-- 
2.55.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v4 1/3] dt-bindings: net: wiznet,w5100: convert to DT schema
  2026-08-11 12:34 ` [PATCH net-next v4 1/3] dt-bindings: net: wiznet,w5100: convert to DT schema Arthur Crépin Leblond
@ 2026-08-11 12:57   ` Arnd Bergmann
  2026-08-11 14:14     ` Arthur Crépin Leblond
  0 siblings, 1 reply; 12+ messages in thread
From: Arnd Bergmann @ 2026-08-11 12:57 UTC (permalink / raw)
  To: Arthur Crépin Leblond, Andrew Lunn, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: Netdev, devicetree, linux-kernel, Krzysztof Kozlowski

On Tue, Aug 11, 2026, at 14:34, Arthur Crépin Leblond wrote:
> +        ethernet@0 {
> +            compatible = "wiznet,w5500";
> +            reg = <0>;
> +            pinctrl-names = "default";
> +            pinctrl-0 = <&eth1_pins>;
> +            interrupt-parent = <&gpio>;
> +            interrupts = <25 IRQ_TYPE_EDGE_FALLING>;

I think this needs to be IRQ_TYPE_EDGE_BOTH instead of IRQ_TYPE_EDGE_FALLING
to make sense.

      Arnd

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v4 0/3] w5100: restore GPIO-based link detection
  2026-08-11 12:34 [PATCH net-next v4 0/3] w5100: restore GPIO-based link detection Arthur Crépin Leblond
                   ` (2 preceding siblings ...)
  2026-08-11 12:34 ` [PATCH net-next v4 3/3] w5100: detect carrier state using link status bit and optional interrupt Arthur Crépin Leblond
@ 2026-08-11 13:01 ` Arnd Bergmann
  2026-08-11 13:37   ` Krzysztof Kozlowski
  3 siblings, 1 reply; 12+ messages in thread
From: Arnd Bergmann @ 2026-08-11 13:01 UTC (permalink / raw)
  To: Arthur Crépin Leblond, Andrew Lunn, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: Netdev, devicetree, linux-kernel, Krzysztof Kozlowski

On Tue, Aug 11, 2026, at 14:34, Arthur Crépin Leblond wrote:
>
> 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 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

Hi Arthur,

This version looks good to me,

Reviewed-by: Arnd Bergmann <arnd@arndb.de>

Krzystof said that he preferred these to be described as GPIOs
instead, but I think there were some other open questions with
your previous version. If you can come up with a version that
Krzystof likes better, I'm fine with that as well.

To me, the main benefit here is that describing it as a
interrupt instead of gpio means it can be connected to an
interrupt pin of a chip that is not also a gpio pin, but
this is another rare corner case, and the code will already
work just fine without the optional interrupt if someone
has a system with interrupt pins that are not also gpios.

      Arnd

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v4 2/3] dt-bindings: net: wiznet,w5100: add link status interrupt
  2026-08-11 12:34 ` [PATCH net-next v4 2/3] dt-bindings: net: wiznet,w5100: add link status interrupt Arthur Crépin Leblond
@ 2026-08-11 13:33   ` Rob Herring (Arm)
  2026-08-11 13:36   ` Krzysztof Kozlowski
  1 sibling, 0 replies; 12+ messages in thread
From: Rob Herring (Arm) @ 2026-08-11 13:33 UTC (permalink / raw)
  To: Arthur Crépin Leblond
  Cc: Paolo Abeni, Jakub Kicinski, Conor Dooley, devicetree,
	Krzysztof Kozlowski, David S. Miller, netdev, Andrew Lunn,
	Eric Dumazet, linux-kernel, Arnd Bergmann


On Tue, 11 Aug 2026 14:34:34 +0200, Arthur Crépin 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 Crépin Leblond <arthur@marmottus.net>
> ---
>  Documentation/devicetree/bindings/net/wiznet,w5100.yaml | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/wiznet,w5100.yaml: properties:interrupts: {'maxItems': 2, 'items': [{'description': 'Main interrupt (INT pin on the device).'}, {'description': 'Optional interrupt for link status changes (LINKLED pin on w5500).'}]} should not be valid under {'required': ['maxItems']}
	hint: "maxItems" is not needed with an "items" list
	from schema $id: http://devicetree.org/meta-schemas/items.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.kernel.org/project/devicetree/patch/20260811-wiznet-link-gpio-v4-2-4d5f7da885a8@marmottus.net

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v4 2/3] dt-bindings: net: wiznet,w5100: add link status interrupt
  2026-08-11 12:34 ` [PATCH net-next v4 2/3] dt-bindings: net: wiznet,w5100: add link status interrupt Arthur Crépin Leblond
  2026-08-11 13:33   ` Rob Herring (Arm)
@ 2026-08-11 13:36   ` Krzysztof Kozlowski
  2026-08-11 14:02     ` Arthur Crépin Leblond
  1 sibling, 1 reply; 12+ messages in thread
From: Krzysztof Kozlowski @ 2026-08-11 13:36 UTC (permalink / raw)
  To: Arthur Crépin Leblond, Andrew Lunn, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: Arnd Bergmann, netdev, devicetree, linux-kernel

On 11/08/2026 14:34, Arthur Crépin 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 Crépin Leblond <arthur@marmottus.net>
> ---
>  Documentation/devicetree/bindings/net/wiznet,w5100.yaml | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/wiznet,w5100.yaml b/Documentation/devicetree/bindings/net/wiznet,w5100.yaml
> index 641c9ddbde9e..30f013f40a77 100644
> --- a/Documentation/devicetree/bindings/net/wiznet,w5100.yaml
> +++ b/Documentation/devicetree/bindings/net/wiznet,w5100.yaml
> @@ -24,7 +24,11 @@ properties:
>        - wiznet,w5500
>  
>    interrupts:
> -    maxItems: 1
> +    maxItems: 2

This is redundant, instead you need "minItems: 1"


Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v4 0/3] w5100: restore GPIO-based link detection
  2026-08-11 13:01 ` [PATCH net-next v4 0/3] w5100: restore GPIO-based link detection Arnd Bergmann
@ 2026-08-11 13:37   ` Krzysztof Kozlowski
  0 siblings, 0 replies; 12+ messages in thread
From: Krzysztof Kozlowski @ 2026-08-11 13:37 UTC (permalink / raw)
  To: Arnd Bergmann, Arthur Crépin Leblond, Andrew Lunn,
	David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: Netdev, devicetree, linux-kernel, Krzysztof Kozlowski

On 11/08/2026 15:01, Arnd Bergmann wrote:
> On Tue, Aug 11, 2026, at 14:34, Arthur Crépin Leblond wrote:
>>
>> 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 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
> 
> Hi Arthur,
> 
> This version looks good to me,
> 
> Reviewed-by: Arnd Bergmann <arnd@arndb.de>
> 
> Krzystof said that he preferred these to be described as GPIOs
> instead, but I think there were some other open questions with
> your previous version. If you can come up with a version that
> Krzystof likes better, I'm fine with that as well.

No, interrupts are fine.

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v4 2/3] dt-bindings: net: wiznet,w5100: add link status interrupt
  2026-08-11 13:36   ` Krzysztof Kozlowski
@ 2026-08-11 14:02     ` Arthur Crépin Leblond
  0 siblings, 0 replies; 12+ messages in thread
From: Arthur Crépin Leblond @ 2026-08-11 14:02 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Arnd Bergmann, netdev, devicetree, linux-kernel

On Tue, Aug 11, 2026 at 03:36:43PM +0200, Krzysztof Kozlowski wrote:
>On 11/08/2026 14:34, Arthur Crépin 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 Crépin Leblond <arthur@marmottus.net>
>> ---
>>  Documentation/devicetree/bindings/net/wiznet,w5100.yaml | 9 +++++++--
>>  1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/net/wiznet,w5100.yaml b/Documentation/devicetree/bindings/net/wiznet,w5100.yaml
>> index 641c9ddbde9e..30f013f40a77 100644
>> --- a/Documentation/devicetree/bindings/net/wiznet,w5100.yaml
>> +++ b/Documentation/devicetree/bindings/net/wiznet,w5100.yaml
>> @@ -24,7 +24,11 @@ properties:
>>        - wiznet,w5500
>>
>>    interrupts:
>> -    maxItems: 1
>> +    maxItems: 2
>
>This is redundant, instead you need "minItems: 1"
>
>
>Best regards,
>Krzysztof

Thank you, I'll update it in the next patch.
I will also remove the unused gpio/gpio.h include.

Arthur

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v4 1/3] dt-bindings: net: wiznet,w5100: convert to DT schema
  2026-08-11 12:57   ` Arnd Bergmann
@ 2026-08-11 14:14     ` Arthur Crépin Leblond
  2026-08-11 14:25       ` Arnd Bergmann
  0 siblings, 1 reply; 12+ messages in thread
From: Arthur Crépin Leblond @ 2026-08-11 14:14 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Netdev, devicetree, linux-kernel, Krzysztof Kozlowski

On Tue, Aug 11, 2026 at 02:57:40PM +0200, Arnd Bergmann wrote:
>On Tue, Aug 11, 2026, at 14:34, Arthur Crépin Leblond wrote:
>> +        ethernet@0 {
>> +            compatible = "wiznet,w5500";
>> +            reg = <0>;
>> +            pinctrl-names = "default";
>> +            pinctrl-0 = <&eth1_pins>;
>> +            interrupt-parent = <&gpio>;
>> +            interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
>
>I think this needs to be IRQ_TYPE_EDGE_BOTH instead of IRQ_TYPE_EDGE_FALLING
>to make sense.
>
>      Arnd

This interrupt is for the main one which is active low, we don't need
to catch the rising one.
Only The link interrupt needs to be both edges.

Arthur

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v4 1/3] dt-bindings: net: wiznet,w5100: convert to DT schema
  2026-08-11 14:14     ` Arthur Crépin Leblond
@ 2026-08-11 14:25       ` Arnd Bergmann
  0 siblings, 0 replies; 12+ messages in thread
From: Arnd Bergmann @ 2026-08-11 14:25 UTC (permalink / raw)
  To: Arthur Crépin Leblond
  Cc: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Netdev, devicetree, linux-kernel, Krzysztof Kozlowski

On Tue, Aug 11, 2026, at 16:14, Arthur Crépin Leblond wrote:
> On Tue, Aug 11, 2026 at 02:57:40PM +0200, Arnd Bergmann wrote:
>>
>>I think this needs to be IRQ_TYPE_EDGE_BOTH instead of IRQ_TYPE_EDGE_FALLING
>>to make sense.
>
> This interrupt is for the main one which is active low, we don't need
> to catch the rising one.
> Only The link interrupt needs to be both edges.

Ah of course, sorry for the noise.

      Arnd

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2026-08-11 14:25 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-11 12:34 [PATCH net-next v4 0/3] w5100: restore GPIO-based link detection Arthur Crépin Leblond
2026-08-11 12:34 ` [PATCH net-next v4 1/3] dt-bindings: net: wiznet,w5100: convert to DT schema Arthur Crépin Leblond
2026-08-11 12:57   ` Arnd Bergmann
2026-08-11 14:14     ` Arthur Crépin Leblond
2026-08-11 14:25       ` Arnd Bergmann
2026-08-11 12:34 ` [PATCH net-next v4 2/3] dt-bindings: net: wiznet,w5100: add link status interrupt Arthur Crépin Leblond
2026-08-11 13:33   ` Rob Herring (Arm)
2026-08-11 13:36   ` Krzysztof Kozlowski
2026-08-11 14:02     ` Arthur Crépin Leblond
2026-08-11 12:34 ` [PATCH net-next v4 3/3] w5100: detect carrier state using link status bit and optional interrupt Arthur Crépin Leblond
2026-08-11 13:01 ` [PATCH net-next v4 0/3] w5100: restore GPIO-based link detection Arnd Bergmann
2026-08-11 13:37   ` Krzysztof Kozlowski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox