public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next v1 0/3] net: dsa: microchip: add single-led-mode support
@ 2026-01-28 13:38 Heinrich Toews
  2026-01-28 13:38 ` [PATCH v1 1/3] dt-bindings: net: dsa: microchip: add microchip,single-led-mode flag Heinrich Toews
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Heinrich Toews @ 2026-01-28 13:38 UTC (permalink / raw)
  To: netdev; +Cc: andrew, f.fainelli, olteanv, robh+dt, krzysztof.kozlowski+dt

This series adds support for single LED mode to KSZ9477 switches.

The first patch adds the YAML device tree binding documentation for the
new 'microchip,single-led-mode' property.

The second patch implements the mode configuration in the ksz_common
driver, including a necessary hardware workaround for Errata Module 19
(DS80000754F), which requires an additional write to a debug register to
ensure proper LED activity.

Heinrich Toews (3):
  dt-bindings: net: dsa: microchip: add microchip,single-led-mode flag
  net: phy: micrel: add flag for KSZ9477 LED erratum
  net: dsa: microchip: add single-led-mode support for KSZ9477

 .../bindings/net/dsa/microchip,ksz.yaml       |  6 ++++
 drivers/net/dsa/microchip/ksz_common.c        | 34 +++++++++++++++++++
 drivers/net/dsa/microchip/ksz_common.h        |  4 +++
 include/linux/micrel_phy.h                    |  1 +
 4 files changed, 45 insertions(+)

-- 
2.52.0


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

* [PATCH v1 1/3] dt-bindings: net: dsa: microchip: add microchip,single-led-mode flag
  2026-01-28 13:38 [PATCH net-next v1 0/3] net: dsa: microchip: add single-led-mode support Heinrich Toews
@ 2026-01-28 13:38 ` Heinrich Toews
  2026-01-28 13:59   ` Andrew Lunn
  2026-01-28 18:06   ` Rob Herring
  2026-01-28 13:38 ` [PATCH v1 2/3] net: phy: micrel: add flag for KSZ9477 LED erratum Heinrich Toews
  2026-01-28 13:38 ` [PATCH v1 3/3] net: dsa: microchip: add single-led-mode support for KSZ9477 Heinrich Toews
  2 siblings, 2 replies; 14+ messages in thread
From: Heinrich Toews @ 2026-01-28 13:38 UTC (permalink / raw)
  To: netdev; +Cc: andrew, f.fainelli, olteanv, robh+dt, krzysztof.kozlowski+dt

Enable single LED mode globally for all non-CPU ports to indicate
link and activity via a single LED per port.

Signed-off-by: Heinrich Toews <ht@twx-software.de>
---
 .../devicetree/bindings/net/dsa/microchip,ksz.yaml          | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml b/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml
index 8d4a3a9a33fcc..35abe242a7741 100644
--- a/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml
@@ -65,6 +65,12 @@ properties:
     description:
       Indicates if the PME pin polarity is active-high.
 
+  microchip,single-led-mode:
+    type: boolean
+    description:
+      Enable single LED mode for all non-CPU ports. In this mode, a single LED
+      indicates both link and activity.
+
   microchip,io-drive-strength-microamp:
     description:
       IO Pad Drive Strength
-- 
2.52.0


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

* [PATCH v1 2/3] net: phy: micrel: add flag for KSZ9477 LED erratum
  2026-01-28 13:38 [PATCH net-next v1 0/3] net: dsa: microchip: add single-led-mode support Heinrich Toews
  2026-01-28 13:38 ` [PATCH v1 1/3] dt-bindings: net: dsa: microchip: add microchip,single-led-mode flag Heinrich Toews
@ 2026-01-28 13:38 ` Heinrich Toews
  2026-01-28 13:38 ` [PATCH v1 3/3] net: dsa: microchip: add single-led-mode support for KSZ9477 Heinrich Toews
  2 siblings, 0 replies; 14+ messages in thread
From: Heinrich Toews @ 2026-01-28 13:38 UTC (permalink / raw)
  To: netdev; +Cc: andrew, f.fainelli, olteanv, robh+dt, krzysztof.kozlowski+dt

Add a new flag MICREL_KSZ9_LED_ERRATA to indicate support for a specific
LED erratum workaround on Microchip KSZ9477 switches. This flag
will be used by the DSA driver to trigger a hardware-specific register
write required for correct LED activity in single-LED mode.

Signed-off-by: Heinrich Toews <ht@twx-software.de>
---
 include/linux/micrel_phy.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/micrel_phy.h b/include/linux/micrel_phy.h
index ca691641788b8..1dcbb0c93d9e1 100644
--- a/include/linux/micrel_phy.h
+++ b/include/linux/micrel_phy.h
@@ -45,6 +45,7 @@
 #define MICREL_PHY_50MHZ_CLK	BIT(0)
 #define MICREL_PHY_FXEN		BIT(1)
 #define MICREL_KSZ8_P1_ERRATA	BIT(2)
+#define MICREL_KSZ9_LED_ERRATA	BIT(3)
 
 #define MICREL_KSZ9021_EXTREG_CTRL	0xB
 #define MICREL_KSZ9021_EXTREG_DATA_WRITE	0xC
-- 
2.52.0


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

* [PATCH v1 3/3] net: dsa: microchip: add single-led-mode support for KSZ9477
  2026-01-28 13:38 [PATCH net-next v1 0/3] net: dsa: microchip: add single-led-mode support Heinrich Toews
  2026-01-28 13:38 ` [PATCH v1 1/3] dt-bindings: net: dsa: microchip: add microchip,single-led-mode flag Heinrich Toews
  2026-01-28 13:38 ` [PATCH v1 2/3] net: phy: micrel: add flag for KSZ9477 LED erratum Heinrich Toews
@ 2026-01-28 13:38 ` Heinrich Toews
  2026-01-29  2:25   ` [v1,3/3] " Jakub Kicinski
  2 siblings, 1 reply; 14+ messages in thread
From: Heinrich Toews @ 2026-01-28 13:38 UTC (permalink / raw)
  To: netdev; +Cc: andrew, f.fainelli, olteanv, robh+dt, krzysztof.kozlowski+dt

Add support for single LED mode via the "microchip,single-led-mode"
device tree property which will apply the configuration globally on all
non-CPU ports.

The implementation includes a workaround for Errata Module 19,
requiring a debug register write to ensure proper LED activity.
The mode is configured during the MAC link-up phase for non-CPU ports.

Signed-off-by: Heinrich Toews <ht@twx-software.de>
---
 drivers/net/dsa/microchip/ksz_common.c | 34 ++++++++++++++++++++++++++
 drivers/net/dsa/microchip/ksz_common.h |  4 +++
 2 files changed, 38 insertions(+)

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index e5fa1f5fc09b3..bef8ec51d9483 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -815,6 +815,9 @@ static const u16 ksz9477_regs[] = {
 	[PTP_RTC_SEC]			= 0x0508,
 	[PTP_SUBNANOSEC_RATE]		= 0x050C,
 	[PTP_MSG_CONF1]			= 0x0514,
+	[P_PHY_MMD_SETUP]		= 0x011A,
+	[P_PHY_MMD_DATA]		= 0x011C,
+	[P_PHY_DIGITAL_DEBUG_3]		= 0x013C,
 };
 
 static const u32 ksz9477_masks[] = {
@@ -3241,6 +3244,14 @@ static u32 ksz_get_phy_flags(struct dsa_switch *ds, int port)
 		if (!port)
 			return MICREL_KSZ8_P1_ERRATA;
 		break;
+	case KSZ9477_CHIP_ID:
+		/* KSZ9477S Errata DS80000754F
+		 *
+		 * Module 19: Single-LED Mode Setting Requires Two Register Writes
+		 *   The PHY Port LEDx_0 pin does not go low in the presence of link
+		 *   activity when Single-LED Mode is selected in the MMD LED Mode Register.
+		 */
+		return MICREL_KSZ9_LED_ERRATA;
 	}
 
 	return 0;
@@ -3909,6 +3920,24 @@ static void ksz_set_gbit(struct ksz_device *dev, int port, bool gbit)
 	ksz_pwrite8(dev, port, regs[P_XMII_CTRL_1], data8);
 }
 
+static void ksz_enable_single_led_mode(struct ksz_device *dev, int port, struct phy_device *phydev)
+{
+	const u16 *regs = dev->info->regs;
+
+	ksz_pwrite16(dev, port, regs[P_PHY_MMD_SETUP], 0x0002); /* Set up register address for MMD */
+	ksz_pwrite16(dev, port,  regs[P_PHY_MMD_DATA], 0x0000); /* Select Register 00h of MMD */
+	ksz_pwrite16(dev, port, regs[P_PHY_MMD_SETUP], 0x4002); /* Select register data for MMD */
+	ksz_pwrite16(dev, port,  regs[P_PHY_MMD_DATA], 0x0011); /* Write value 0011h to MMD */
+
+	/* According to KSZ9477S Errata DS80000754F (Module 19) 0xfa00 has to
+	 * be written to the Debug Register 3 to enable Single-LED Mode.
+	 */
+	if (phydev->dev_flags & MICREL_KSZ9_LED_ERRATA)
+		ksz_pwrite16(dev, port, regs[P_PHY_DIGITAL_DEBUG_3], 0xfa00);
+
+	dev_info(dev->dev, "port-%d: single-led mode enabled.\n", port);
+}
+
 static void ksz_set_100_10mbit(struct ksz_device *dev, int port, int speed)
 {
 	const u8 *bitval = dev->info->xmii_ctrl0;
@@ -3977,6 +4006,9 @@ static void ksz9477_phylink_mac_link_up(struct phylink_config *config,
 	int port = dp->index;
 	struct ksz_port *p;
 
+	if (dev->single_led_mode && port != dev->cpu_port)
+		ksz_enable_single_led_mode(dev, port, phydev);
+
 	p = &dev->ports[port];
 
 	/* Internal PHYs */
@@ -5526,6 +5558,8 @@ int ksz_switch_register(struct ksz_device *dev)
 							   "wakeup-source");
 		dev->pme_active_high = of_property_read_bool(dev->dev->of_node,
 							     "microchip,pme-active-high");
+		dev->single_led_mode = of_property_read_bool(dev->dev->of_node,
+							     "microchip,single-led-mode");
 	}
 
 	ret = dsa_register_switch(dev->ds);
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index 929aff4c55de5..9900e9dd91810 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -187,6 +187,7 @@ struct ksz_device {
 	bool synclko_disable;
 	bool wakeup_source;
 	bool pme_active_high;
+	bool single_led_mode;		/* Enable Single LED Mode */
 
 	struct vlan_table *vlan_cache;
 
@@ -277,6 +278,9 @@ enum ksz_regs {
 	PTP_RTC_SUB_NANOSEC,
 	PTP_SUBNANOSEC_RATE,
 	PTP_MSG_CONF1,
+	P_PHY_MMD_SETUP,
+	P_PHY_MMD_DATA,
+	P_PHY_DIGITAL_DEBUG_3,
 };
 
 enum ksz_masks {
-- 
2.52.0


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

* Re: [PATCH v1 1/3] dt-bindings: net: dsa: microchip: add microchip,single-led-mode flag
  2026-01-28 13:38 ` [PATCH v1 1/3] dt-bindings: net: dsa: microchip: add microchip,single-led-mode flag Heinrich Toews
@ 2026-01-28 13:59   ` Andrew Lunn
  2026-01-28 15:10     ` Heinrich Töws (TWx)
  2026-01-28 18:06   ` Rob Herring
  1 sibling, 1 reply; 14+ messages in thread
From: Andrew Lunn @ 2026-01-28 13:59 UTC (permalink / raw)
  To: Heinrich Toews
  Cc: netdev, f.fainelli, olteanv, robh+dt, krzysztof.kozlowski+dt

On Wed, Jan 28, 2026 at 02:38:31PM +0100, Heinrich Toews wrote:
> Enable single LED mode globally for all non-CPU ports to indicate
> link and activity via a single LED per port.

Are there other blinking modes? If the LED is capable of more, you
should be using /sys/class/leds and the netdev trigger. If the only
thing it can indicate is link and activity, this approach is O.K.

> +  microchip,single-led-mode:
> +    type: boolean
> +    description:
> +      Enable single LED mode for all non-CPU ports. In this mode, a single LED
> +      indicates both link and activity.

Is there a dual LED mode? Triple LED mode? What happens if this
property is not found?

	Andrew

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

* Re: [PATCH v1 1/3] dt-bindings: net: dsa: microchip: add microchip,single-led-mode flag
  2026-01-28 13:59   ` Andrew Lunn
@ 2026-01-28 15:10     ` Heinrich Töws (TWx)
  2026-01-28 15:22       ` Andrew Lunn
  0 siblings, 1 reply; 14+ messages in thread
From: Heinrich Töws (TWx) @ 2026-01-28 15:10 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: netdev, f.fainelli, olteanv, robh+dt, krzysztof.kozlowski+dt

Hi Andrew,

Am 28.01.26 um 14:59 schrieb Andrew Lunn:
> Are there other blinking modes? If the LED is capable of more, you
> should be using /sys/class/leds and the netdev trigger. If the only
> thing it can indicate is link and activity, this approach is O.K.

I re-checked the datasheet (DS00002392). The KSZ9477 PHY ports indeed 
have two pins, but they are not general-purpose. The term "programmable" 
in the datasheet refers only to switching between two fixed 
hardware-mapped modes via MMD:

1. Tri-Color Dual-LED Mode (Default): The pins LEDx_1 and LEDx_0 work in 
conjunction to indicate link speed (10/100/1000) and activity. For 
example, 10BASE-T status is indicated by both pins working together.

2. Single-LED Mode: Here, the logic is simplified so that LEDx_1 
indicates Link and LEDx_0 indicates Activity.

There is no "Manual Mode" or "GPIO Mode" for these pins. They are driven 
by a fixed internal state machine. Since we cannot control the pins' 
brightness or trigger events independently from the PHY's hardcoded 
logic, the netdev trigger or the LED class cannot be used to emulate 
this behavior.

> Is there a dual LED mode? Triple LED mode? What happens if this
> property is not found?

If the property is not found, the switch remains in its default 
"Tri-Color Dual-LED Mode".

I will clarify this in the description for v2.

Thanks,Heinrich.

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

* Re: [PATCH v1 1/3] dt-bindings: net: dsa: microchip: add microchip,single-led-mode flag
  2026-01-28 15:10     ` Heinrich Töws (TWx)
@ 2026-01-28 15:22       ` Andrew Lunn
  2026-01-28 15:53         ` Heinrich Töws (TWx)
  0 siblings, 1 reply; 14+ messages in thread
From: Andrew Lunn @ 2026-01-28 15:22 UTC (permalink / raw)
  To: Heinrich Töws (TWx)
  Cc: netdev, f.fainelli, olteanv, robh+dt, krzysztof.kozlowski+dt

> There is no "Manual Mode" or "GPIO Mode" for these pins. They are driven by
> a fixed internal state machine. Since we cannot control the pins' brightness
> or trigger events independently from the PHY's hardcoded logic, the netdev
> trigger or the LED class cannot be used to emulate this behavior.

Thanks for checking.

> > Is there a dual LED mode? Triple LED mode? What happens if this
> > property is not found?
> 
> If the property is not found, the switch remains in its default "Tri-Color
> Dual-LED Mode".

Is that really true? Generally, what actually happens is the
configuration is left alone. So if the boot loader, for example, has
set the configuration, that configuration is left unchanged. So it
could actually be in single LED mode, not tri-colour.

      Andrew

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

* Re: [PATCH v1 1/3] dt-bindings: net: dsa: microchip: add microchip,single-led-mode flag
  2026-01-28 15:22       ` Andrew Lunn
@ 2026-01-28 15:53         ` Heinrich Töws (TWx)
  0 siblings, 0 replies; 14+ messages in thread
From: Heinrich Töws (TWx) @ 2026-01-28 15:53 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: netdev, f.fainelli, olteanv, robh+dt, krzysztof.kozlowski+dt

Hi Andrew,

Am 28.01.26 um 16:22 schrieb Andrew Lunn:
> Is that really true? Generally, what actually happens is the
> configuration is left alone. So if the boot loader, for example, has
> set the configuration, that configuration is left unchanged. So it
> could actually be in single LED mode, not tri-colour.

Oh, you're right. We cannot assume a hardware reset state here.
Since the driver currently does not touch these registers, the 
configuration is indeed "inherited" from the bootloader or the
hardware strapping.

Thanks,

Heinrich.

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

* Re: [PATCH v1 1/3] dt-bindings: net: dsa: microchip: add microchip,single-led-mode flag
  2026-01-28 13:38 ` [PATCH v1 1/3] dt-bindings: net: dsa: microchip: add microchip,single-led-mode flag Heinrich Toews
  2026-01-28 13:59   ` Andrew Lunn
@ 2026-01-28 18:06   ` Rob Herring
  2026-02-03 10:22     ` Heinrich Toews
  1 sibling, 1 reply; 14+ messages in thread
From: Rob Herring @ 2026-01-28 18:06 UTC (permalink / raw)
  To: Heinrich Toews
  Cc: netdev, andrew, f.fainelli, olteanv, krzysztof.kozlowski+dt

On Wed, Jan 28, 2026 at 7:39 AM Heinrich Toews <ht@twx-software.de> wrote:
>
> Enable single LED mode globally for all non-CPU ports to indicate
> link and activity via a single LED per port.

Please use get_maintainers.pl and send to the correct lists if you
want this reviewed.

>
> Signed-off-by: Heinrich Toews <ht@twx-software.de>
> ---
>  .../devicetree/bindings/net/dsa/microchip,ksz.yaml          | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml b/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml
> index 8d4a3a9a33fcc..35abe242a7741 100644
> --- a/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml
> +++ b/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml
> @@ -65,6 +65,12 @@ properties:
>      description:
>        Indicates if the PME pin polarity is active-high.
>
> +  microchip,single-led-mode:
> +    type: boolean
> +    description:
> +      Enable single LED mode for all non-CPU ports. In this mode, a single LED
> +      indicates both link and activity.
> +
>    microchip,io-drive-strength-microamp:
>      description:
>        IO Pad Drive Strength
> --
> 2.52.0
>

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

* Re: [v1,3/3] net: dsa: microchip: add single-led-mode support for KSZ9477
  2026-01-28 13:38 ` [PATCH v1 3/3] net: dsa: microchip: add single-led-mode support for KSZ9477 Heinrich Toews
@ 2026-01-29  2:25   ` Jakub Kicinski
  2026-02-03 11:13     ` Heinrich Toews
  0 siblings, 1 reply; 14+ messages in thread
From: Jakub Kicinski @ 2026-01-29  2:25 UTC (permalink / raw)
  To: ht
  Cc: Jakub Kicinski, f.fainelli, robh+dt, andrew, olteanv, netdev,
	krzysztof.kozlowski+dt

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
net: dsa: microchip: add single-led-mode support for KSZ9477

This patch adds single LED mode support for KSZ9477 via a device tree
property. The mode is configured during MAC link-up for non-CPU ports
and includes a workaround for Errata Module 19.

> diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
> index e5fa1f5fc09b..bef8ec51d948 100644
> --- a/drivers/net/dsa/microchip/ksz_common.c
> +++ b/drivers/net/dsa/microchip/ksz_common.c

[ ... ]

> @@ -3909,6 +3920,24 @@ static void ksz_set_gbit(struct ksz_device *dev, int port, bool gbit)
>  	ksz_pwrite8(dev, port, regs[P_XMII_CTRL_1], data8);
>  }
>
> +static void ksz_enable_single_led_mode(struct ksz_device *dev, int port, struct phy_device *phydev)
> +{
> +	const u16 *regs = dev->info->regs;
> +
> +	ksz_pwrite16(dev, port, regs[P_PHY_MMD_SETUP], 0x0002); /* Set up register address for MMD */
> +	ksz_pwrite16(dev, port,  regs[P_PHY_MMD_DATA], 0x0000); /* Select Register 00h of MMD */
> +	ksz_pwrite16(dev, port, regs[P_PHY_MMD_SETUP], 0x4002); /* Select register data for MMD */
> +	ksz_pwrite16(dev, port,  regs[P_PHY_MMD_DATA], 0x0011); /* Write value 0011h to MMD */
> +
> +	/* According to KSZ9477S Errata DS80000754F (Module 19) 0xfa00 has to
> +	 * be written to the Debug Register 3 to enable Single-LED Mode.
> +	 */
> +	if (phydev->dev_flags & MICREL_KSZ9_LED_ERRATA)
              ^^^^^^

Can phydev be NULL here? The phylink framework passes phydev as NULL when
using fixed-link configurations (MLO_AN_FIXED mode). For example, if a
non-CPU port on KSZ9477 is configured with a fixed-link in device tree
instead of a real PHY, phylink_fwnode_phy_connect() returns early without
attaching a PHY, leaving pl->phydev as NULL.

When link comes up on such a port, phylink_link_up() calls the mac_link_up
callback with this NULL phydev, which then gets passed to this function.

> +		ksz_pwrite16(dev, port, regs[P_PHY_DIGITAL_DEBUG_3], 0xfa00);
> +
> +	dev_info(dev->dev, "port-%d: single-led mode enabled.\n", port);
> +}

[ ... ]

> @@ -3977,6 +4006,9 @@ static void ksz9477_phylink_mac_link_up(struct phylink_config *config,
>  	int port = dp->index;
>  	struct ksz_port *p;
>
> +	if (dev->single_led_mode && port != dev->cpu_port)
> +		ksz_enable_single_led_mode(dev, port, phydev);

The guard here checks single_led_mode and that port is not the CPU port,
but it does not verify that phydev is valid. On KSZ9477, ports 5 and 6
do not have internal PHYs (internal_phy[] is false), and if one of these
external ports is configured with fixed-link while cpu_port is the other
external port, this path will be taken with phydev=NULL.

[ ... ]
-- 
pw-bot: cr

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

* Re: [PATCH v1 1/3] dt-bindings: net: dsa: microchip: add microchip,single-led-mode flag
@ 2026-01-30 19:19 Tim Harvey
  2026-02-03 13:44 ` Heinrich Toews
  0 siblings, 1 reply; 14+ messages in thread
From: Tim Harvey @ 2026-01-30 19:19 UTC (permalink / raw)
  To: netdev, Heinrich Toews, Andrew Lunn
  Cc: Florian Fainelli, Vladimir Oltean, Rob Herring,
	Krzysztof Kozlowski

> Hi Andrew,
>
> > Am 28.01.26 um 16:22 schrieb Andrew Lunn:
> > Is that really true? Generally, what actually happens is the
> > configuration is left alone. So if the boot loader, for example, has
> > set the configuration, that configuration is left unchanged. So it
> > could actually be in single LED mode, not tri-colour.
> >
> Oh, you're right. We cannot assume a hardware reset state here.
> Since the driver currently does not touch these registers, the configuration is indeed
> "inherited" from the bootloader or the
> hardware strapping.

Hi Heinrich,

The setting the SW_RESET bit of REG_SW_OPERATION in
ksz9477_reset_switch() resets all switch registers so even when the
mode/errata are taken care of in boot firmware that gets clobbered
when Linux starts. It is not inherited.

I have this switch with single-led mode on two of the boards I support
(imx8mp-venice-gw74xx.dts and imx8mm-venice-gw7901.dts) and am
interested in seeing your v2 of this series. Please add me to the cc
if you would not mind.

Best Regards,

Tim

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

* Re: [PATCH v1 1/3] dt-bindings: net: dsa: microchip: add microchip,single-led-mode flag
  2026-01-28 18:06   ` Rob Herring
@ 2026-02-03 10:22     ` Heinrich Toews
  0 siblings, 0 replies; 14+ messages in thread
From: Heinrich Toews @ 2026-02-03 10:22 UTC (permalink / raw)
  To: Rob Herring; +Cc: netdev, andrew, f.fainelli, olteanv, krzysztof.kozlowski+dt

Hi Rob,

On Wed, Jan 28, 2026 at 12:06:10PM -0600, Rob Herring wrote:
> On Wed, Jan 28, 2026 at 7:39 AM Heinrich Toews <ht@twx-software.de> wrote:
> >
> > Enable single LED mode globally for all non-CPU ports to indicate
> > link and activity via a single LED per port.
> 
> Please use get_maintainers.pl and send to the correct lists if you
> want this reviewed.

Thanks for pointing that out. I will re-run the script and fix the recipient list in v2.

Best regards,
Heinrich.

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

* Re: [v1,3/3] net: dsa: microchip: add single-led-mode support for KSZ9477
  2026-01-29  2:25   ` [v1,3/3] " Jakub Kicinski
@ 2026-02-03 11:13     ` Heinrich Toews
  0 siblings, 0 replies; 14+ messages in thread
From: Heinrich Toews @ 2026-02-03 11:13 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: f.fainelli, robh+dt, andrew, olteanv, netdev,
	krzysztof.kozlowski+dt

Hello Jakub,

On Wed, Jan 28, 2026 at 06:25:48PM -0800, Jakub Kicinski wrote:
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> 
> For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
> ---
> net: dsa: microchip: add single-led-mode support for KSZ9477
> 
> This patch adds single LED mode support for KSZ9477 via a device tree
> property. The mode is configured during MAC link-up for non-CPU ports
> and includes a workaround for Errata Module 19.
> 
> > diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
> > index e5fa1f5fc09b..bef8ec51d948 100644
> > --- a/drivers/net/dsa/microchip/ksz_common.c
> > +++ b/drivers/net/dsa/microchip/ksz_common.c
> 
> [ ... ]
> 
> > @@ -3909,6 +3920,24 @@ static void ksz_set_gbit(struct ksz_device *dev, int port, bool gbit)
> >  	ksz_pwrite8(dev, port, regs[P_XMII_CTRL_1], data8);
> >  }
> >
> > +static void ksz_enable_single_led_mode(struct ksz_device *dev, int port, struct phy_device *phydev)
> > +{
> > +	const u16 *regs = dev->info->regs;
> > +
> > +	ksz_pwrite16(dev, port, regs[P_PHY_MMD_SETUP], 0x0002); /* Set up register address for MMD */
> > +	ksz_pwrite16(dev, port,  regs[P_PHY_MMD_DATA], 0x0000); /* Select Register 00h of MMD */
> > +	ksz_pwrite16(dev, port, regs[P_PHY_MMD_SETUP], 0x4002); /* Select register data for MMD */
> > +	ksz_pwrite16(dev, port,  regs[P_PHY_MMD_DATA], 0x0011); /* Write value 0011h to MMD */
> > +
> > +	/* According to KSZ9477S Errata DS80000754F (Module 19) 0xfa00 has to
> > +	 * be written to the Debug Register 3 to enable Single-LED Mode.
> > +	 */
> > +	if (phydev->dev_flags & MICREL_KSZ9_LED_ERRATA)
>               ^^^^^^
> 
> Can phydev be NULL here? The phylink framework passes phydev as NULL when
> using fixed-link configurations (MLO_AN_FIXED mode). For example, if a
> non-CPU port on KSZ9477 is configured with a fixed-link in device tree
> instead of a real PHY, phylink_fwnode_phy_connect() returns early without
> attaching a PHY, leaving pl->phydev as NULL.
> 
> When link comes up on such a port, phylink_link_up() calls the mac_link_up
> callback with this NULL phydev, which then gets passed to this function.

Thanks, I see the problem here. Indeed, in a fixed-link configuration, phydev
will be NULL here.

Since the LED mode configuration is specific to the PHY, I will add a 
NULL check to ensure we only proceed if a PHY is actually attached to 
the port.

> > +		ksz_pwrite16(dev, port, regs[P_PHY_DIGITAL_DEBUG_3], 0xfa00);
> > +
> > +	dev_info(dev->dev, "port-%d: single-led mode enabled.\n", port);
> > +}
> 
> [ ... ]
> 
> > @@ -3977,6 +4006,9 @@ static void ksz9477_phylink_mac_link_up(struct phylink_config *config,
> >  	int port = dp->index;
> >  	struct ksz_port *p;
> >
> > +	if (dev->single_led_mode && port != dev->cpu_port)
> > +		ksz_enable_single_led_mode(dev, port, phydev);
> 
> The guard here checks single_led_mode and that port is not the CPU port,
> but it does not verify that phydev is valid. On KSZ9477, ports 5 and 6
> do not have internal PHYs (internal_phy[] is false), and if one of these
> external ports is configured with fixed-link while cpu_port is the other
> external port, this path will be taken with phydev=NULL.

I re-checked the datasheet (DS00002392C) and indeed the switch has seven 
ports, of which only five have integrated PHYs. The two external MAC 
ports (5 and 6) can be connected to a host processor, another switch, 
or an external PHY.

As you pointed out, if these are used as fixed-links, phydev will be 
NULL. I will add a check to ensure we only proceed if a PHY is 
attached to the port.

I will fix this in v2.

Regards,
Heinrich

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

* Re: [PATCH v1 1/3] dt-bindings: net: dsa: microchip: add microchip,single-led-mode flag
  2026-01-30 19:19 [PATCH v1 1/3] dt-bindings: net: dsa: microchip: add microchip,single-led-mode flag Tim Harvey
@ 2026-02-03 13:44 ` Heinrich Toews
  0 siblings, 0 replies; 14+ messages in thread
From: Heinrich Toews @ 2026-02-03 13:44 UTC (permalink / raw)
  To: Tim Harvey
  Cc: netdev, Andrew Lunn, Florian Fainelli, Vladimir Oltean,
	Rob Herring, Krzysztof Kozlowski

Hi Tim,

On Fri, Jan 30, 2026 at 11:19:30AM -0800, Tim Harvey wrote:
> > Hi Andrew,
> >
> > > Am 28.01.26 um 16:22 schrieb Andrew Lunn:
> > > Is that really true? Generally, what actually happens is the
> > > configuration is left alone. So if the boot loader, for example, has
> > > set the configuration, that configuration is left unchanged. So it
> > > could actually be in single LED mode, not tri-colour.
> > >
> > Oh, you're right. We cannot assume a hardware reset state here.
> > Since the driver currently does not touch these registers, the configuration is indeed
> > "inherited" from the bootloader or the
> > hardware strapping.
> 
> Hi Heinrich,
> 
> The setting the SW_RESET bit of REG_SW_OPERATION in
> ksz9477_reset_switch() resets all switch registers so even when the
> mode/errata are taken care of in boot firmware that gets clobbered
> when Linux starts. It is not inherited.

You're right. I see that ksz_setup() always triggers a switch-reset and
there is no device tree property which could change this behaviour.

According to the datasheet (DS00002392C, p. 86), setting Bit 1 in 
REG_SW_OPERATION (0x0300) resets all internal registers, including 
PHY registers. So the switch indeed remains its default
"Tri-Color Dual-LED Mode" if microchip,single-led-mode property is not set.

> I have this switch with single-led mode on two of the boards I support
> (imx8mp-venice-gw74xx.dts and imx8mm-venice-gw7901.dts) and am
> interested in seeing your v2 of this series. Please add me to the cc
> if you would not mind.

Sure. I will add you to the CC for the v2 series. Thanks for the feedback!

Regards,
Heinrich.

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

end of thread, other threads:[~2026-02-03 13:44 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-28 13:38 [PATCH net-next v1 0/3] net: dsa: microchip: add single-led-mode support Heinrich Toews
2026-01-28 13:38 ` [PATCH v1 1/3] dt-bindings: net: dsa: microchip: add microchip,single-led-mode flag Heinrich Toews
2026-01-28 13:59   ` Andrew Lunn
2026-01-28 15:10     ` Heinrich Töws (TWx)
2026-01-28 15:22       ` Andrew Lunn
2026-01-28 15:53         ` Heinrich Töws (TWx)
2026-01-28 18:06   ` Rob Herring
2026-02-03 10:22     ` Heinrich Toews
2026-01-28 13:38 ` [PATCH v1 2/3] net: phy: micrel: add flag for KSZ9477 LED erratum Heinrich Toews
2026-01-28 13:38 ` [PATCH v1 3/3] net: dsa: microchip: add single-led-mode support for KSZ9477 Heinrich Toews
2026-01-29  2:25   ` [v1,3/3] " Jakub Kicinski
2026-02-03 11:13     ` Heinrich Toews
  -- strict thread matches above, loose matches on Subject: below --
2026-01-30 19:19 [PATCH v1 1/3] dt-bindings: net: dsa: microchip: add microchip,single-led-mode flag Tim Harvey
2026-02-03 13:44 ` Heinrich Toews

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