* [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; 16+ 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] 16+ 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; 16+ 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] 16+ 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; 16+ 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] 16+ 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; 16+ 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] 16+ 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; 16+ 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] 16+ 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; 16+ 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] 16+ 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; 16+ 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] 16+ 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; 16+ 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] 16+ 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; 16+ 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] 16+ 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; 16+ 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] 16+ 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; 16+ 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] 16+ 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; 16+ 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] 16+ messages in thread
* Re: [v1,3/3] net: dsa: microchip: add single-led-mode support for KSZ9477
@ 2026-01-30 21:46 Tim Harvey
2026-02-03 16:58 ` Heinrich Toews
0 siblings, 1 reply; 16+ messages in thread
From: Tim Harvey @ 2026-01-30 21:46 UTC (permalink / raw)
To: Heinrich Toews, netdev
Cc: Andrew Lunn, Florian Fainelli, Vladimir Oltean, Rob Herring,
Krzysztof Kozlowski
>
> 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,
> };
>
> @@ -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;
> }
>
Hi Heinrich,
Thanks for doing this. I have two boards I work with that have a
KSZ9897S switch and use single-led mode (imx8mp-venice-gw74xx.dts and
imx8mm-venice-gw7901.dts) that suffer from this issue. In my scenario
I take care of this in the bootloader but when ksz9477_reset_switch()
is called and sets the SW_RESET bit of REG_SW_OPERATION it resets all
registers and reverts to tri-color dual-led mode thus we really do
need a dt prop and handling for this.
You can add the errata for KSZ9897 as well here:
+ case KSZ9897_CHIP_ID:
+ /* KSZ9897S Errata DS80000759F: Module 18 */
+ return MICREL_KSZ9_LED_ERRATA;
And because this errata applies to multiple chips it's probably best
to describe the workaround once and refer to the errata # in the cases
statements.
> +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 */
These are wrong: The MMD_SETUP and MMD_DATA regs you are using here
0x011a and 0x11c are actually 0xN11a 0x 0xN11c (where N is the port)
so they port specific so the above only would set them for port0.
Because these registers correspond to standardPHY register 0x0d and
0x0e which are defined in mii.h you can just use those.
What you want here is to use ksz_phy_write16 on the port which will
call the chip-specific w_phy which will handle writing to the port
specific register:
ksz_phy_write16(dev, port, MII_MMD_CTRL, 0x02);
ksz_phy_write16(dev, port, MII_MMD_DATA, 0x00);
ksz_phy_write16(dev, port, MII_MMD_CTRL, MII_MMD_CTRL_NOINCR | 0x02);
ksz_phy_write16(dev, port, MII_MMD_DATA, 0x10);
Note on that last one, the errata says to write 0x11 but that's a typo
as if you read closely it says 'setting bit 4' and looking at the
datasheet bit0 is reserved, so the 0x11 should be 0x10.
> +
> + /* 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);
Similarly the errata specifies this as PHY register 0x1E (which is a
vendor specific register so not defined in mii.h):
ksz_phy_write16(dev, port, 0x1e, 0xfa00);
Your original patch doesn't work in my case (maybe it worked for you
because you were trying to address port0?). With the above changes the
LED's are properly configured on my boards.
Best Regards,
Tim
> +
> + 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 {
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [v1,3/3] net: dsa: microchip: add single-led-mode support for KSZ9477
2026-01-30 21:46 Tim Harvey
@ 2026-02-03 16:58 ` Heinrich Toews
2026-02-04 0:56 ` Tim Harvey
0 siblings, 1 reply; 16+ messages in thread
From: Heinrich Toews @ 2026-02-03 16:58 UTC (permalink / raw)
To: Tim Harvey
Cc: netdev, Andrew Lunn, Florian Fainelli, Vladimir Oltean,
Rob Herring
Hi Rob,
On Fri, Jan 30, 2026 at 01:46:52PM -0800, Tim Harvey wrote:
> >
> > 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,
> > };
> >
> > @@ -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;
> > }
> >
>
> Hi Heinrich,
>
> Thanks for doing this. I have two boards I work with that have a
> KSZ9897S switch and use single-led mode (imx8mp-venice-gw74xx.dts and
> imx8mm-venice-gw7901.dts) that suffer from this issue. In my scenario
> I take care of this in the bootloader but when ksz9477_reset_switch()
> is called and sets the SW_RESET bit of REG_SW_OPERATION it resets all
> registers and reverts to tri-color dual-led mode thus we really do
> need a dt prop and handling for this.
I agree. To support these use cases, a specific Device Tree property
would be the right thing here.
> You can add the errata for KSZ9897 as well here:
> + case KSZ9897_CHIP_ID:
> + /* KSZ9897S Errata DS80000759F: Module 18 */
> + return MICREL_KSZ9_LED_ERRATA;
>
> And because this errata applies to multiple chips it's probably best
> to describe the workaround once and refer to the errata # in the cases
> statements.
I did some research on this and it seems that nearly all members of the
KSZ9xxx series are affected, including:
KSZ9477 / KSZ9897 / KSZ9896
KSZ9567 / KSZ8567 / KSZ9893
KSZ9563 / KSZ8563 / KSZ8565
So MICREL_KSZ9_LED_ERRATA should be enabled for these CHIP_IDs.
> > +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 */
>
> These are wrong: The MMD_SETUP and MMD_DATA regs you are using here
> 0x011a and 0x11c are actually 0xN11a 0x 0xN11c (where N is the port)
> so they port specific so the above only would set them for port0.
> Because these registers correspond to standardPHY register 0x0d and
> 0x0e which are defined in mii.h you can just use those.
>
> What you want here is to use ksz_phy_write16 on the port which will
> call the chip-specific w_phy which will handle writing to the port
> specific register:
> ksz_phy_write16(dev, port, MII_MMD_CTRL, 0x02);
> ksz_phy_write16(dev, port, MII_MMD_DATA, 0x00);
> ksz_phy_write16(dev, port, MII_MMD_CTRL, MII_MMD_CTRL_NOINCR | 0x02);
> ksz_phy_write16(dev, port, MII_MMD_DATA, 0x10);
I added debug output and tested my setup on all 4 external ports and it
works:
ksz-switch spi1.0: ksz_pwrite16: port: 1, offset: 0x011a, data: 0x0002
ksz-switch spi1.0: ksz_write16: reg: 0x0000211a, value: 0x0002
^^^^
The port index
is set
correctly.
[...]
ksz-switch spi1.0: ksz_pwrite16: port: 2, offset: 0x011a, data: 0x0002
ksz-switch spi1.0: ksz_write16: reg: 0x0000311a, value: 0x0002
^^^^
The same here
on port 2.
Is see activity on the leds connected to LEDx_0.
ksz9477_get_port_addr() is adding the port index by PORT_CTRL_ADDR(port, offset).
But since I'm accessing PHY registers here, I'll switch to ksz_phy_write16() in v2.
> Note on that last one, the errata says to write 0x11 but that's a typo
> as if you read closely it says 'setting bit 4' and looking at the
> datasheet bit0 is reserved, so the 0x11 should be 0x10.
Yes, you're right. I remember seeing it before. Thanks.
> Similarly the errata specifies this as PHY register 0x1E (which is a
> vendor specific register so not defined in mii.h):
>
> ksz_phy_write16(dev, port, 0x1e, 0xfa00);
May be I should add a local macro define for it.
> Your original patch doesn't work in my case (maybe it worked for you
> because you were trying to address port0?). With the above changes the
> LED's are properly configured on my boards.
Okay. But I don't really understand why its not working in your
case.
Thanks for you're feedback.
Regards,
Heinrich.
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [v1,3/3] net: dsa: microchip: add single-led-mode support for KSZ9477
2026-02-03 16:58 ` Heinrich Toews
@ 2026-02-04 0:56 ` Tim Harvey
2026-02-04 10:22 ` Heinrich Toews
0 siblings, 1 reply; 16+ messages in thread
From: Tim Harvey @ 2026-02-04 0:56 UTC (permalink / raw)
To: Heinrich Toews
Cc: netdev, Andrew Lunn, Florian Fainelli, Vladimir Oltean,
Rob Herring
On Tue, Feb 3, 2026 at 8:58 AM Heinrich Toews <ht@twx-software.de> wrote:
>
> Hi Rob,
>
> On Fri, Jan 30, 2026 at 01:46:52PM -0800, Tim Harvey wrote:
> > >
> > > 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,
> > > };
> > >
> > > @@ -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;
> > > }
> > >
> >
> > Hi Heinrich,
> >
> > Thanks for doing this. I have two boards I work with that have a
> > KSZ9897S switch and use single-led mode (imx8mp-venice-gw74xx.dts and
> > imx8mm-venice-gw7901.dts) that suffer from this issue. In my scenario
> > I take care of this in the bootloader but when ksz9477_reset_switch()
> > is called and sets the SW_RESET bit of REG_SW_OPERATION it resets all
> > registers and reverts to tri-color dual-led mode thus we really do
> > need a dt prop and handling for this.
>
> I agree. To support these use cases, a specific Device Tree property
> would be the right thing here.
>
> > You can add the errata for KSZ9897 as well here:
> > + case KSZ9897_CHIP_ID:
> > + /* KSZ9897S Errata DS80000759F: Module 18 */
> > + return MICREL_KSZ9_LED_ERRATA;
> >
> > And because this errata applies to multiple chips it's probably best
> > to describe the workaround once and refer to the errata # in the cases
> > statements.
>
> I did some research on this and it seems that nearly all members of the
> KSZ9xxx series are affected, including:
>
> KSZ9477 / KSZ9897 / KSZ9896
> KSZ9567 / KSZ8567 / KSZ9893
> KSZ9563 / KSZ8563 / KSZ8565
>
> So MICREL_KSZ9_LED_ERRATA should be enabled for these CHIP_IDs.
>
> > > +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 */
> >
> > These are wrong: The MMD_SETUP and MMD_DATA regs you are using here
> > 0x011a and 0x11c are actually 0xN11a 0x 0xN11c (where N is the port)
> > so they port specific so the above only would set them for port0.
> > Because these registers correspond to standardPHY register 0x0d and
> > 0x0e which are defined in mii.h you can just use those.
> >
> > What you want here is to use ksz_phy_write16 on the port which will
> > call the chip-specific w_phy which will handle writing to the port
> > specific register:
> > ksz_phy_write16(dev, port, MII_MMD_CTRL, 0x02);
> > ksz_phy_write16(dev, port, MII_MMD_DATA, 0x00);
> > ksz_phy_write16(dev, port, MII_MMD_CTRL, MII_MMD_CTRL_NOINCR | 0x02);
> > ksz_phy_write16(dev, port, MII_MMD_DATA, 0x10);
>
> I added debug output and tested my setup on all 4 external ports and it
> works:
>
> ksz-switch spi1.0: ksz_pwrite16: port: 1, offset: 0x011a, data: 0x0002
> ksz-switch spi1.0: ksz_write16: reg: 0x0000211a, value: 0x0002
> ^^^^
> The port index
> is set
> correctly.
>
> [...]
>
> ksz-switch spi1.0: ksz_pwrite16: port: 2, offset: 0x011a, data: 0x0002
> ksz-switch spi1.0: ksz_write16: reg: 0x0000311a, value: 0x0002
> ^^^^
> The same here
> on port 2.
>
> Is see activity on the leds connected to LEDx_0.
>
Hi Heinrich,
My apologies - you are correct. I accidentally missed the hunk where
you set your registers in ksz9477_regs causing it to not work for me.
> ksz9477_get_port_addr() is adding the port index by PORT_CTRL_ADDR(port, offset).
>
> But since I'm accessing PHY registers here, I'll switch to ksz_phy_write16() in v2.
Yes, that would be best as you would no longer need to add custom registers.
I have:
ksz_phy_write16(dev->ds, port, MII_MMD_CTRL, 0x0002);
ksz_phy_write16(dev->ds, port, MII_MMD_DATA, 0x0000);
ksz_phy_write16(dev->ds, port, MII_MMD_CTRL,
MII_MMD_CTRL_NOINCR | 0x02);
ksz_phy_write16(dev->ds, port, MII_MMD_DATA, 0x0010); /* set bit 4 */
/* LED_ERRATA requires writing 0xfa00 to Debug Register 3 (PHY
register 0x1e) */
if (phydev->dev_flags & MICREL_KSZ9_LED_ERRATA)
ksz_phy_write16(dev->ds, port, 0x1e, 0xfa00);
I look forward to seeing your next submission.
Best Regards,
Tim
>
> > Note on that last one, the errata says to write 0x11 but that's a typo
> > as if you read closely it says 'setting bit 4' and looking at the
> > datasheet bit0 is reserved, so the 0x11 should be 0x10.
>
> Yes, you're right. I remember seeing it before. Thanks.
>
> > Similarly the errata specifies this as PHY register 0x1E (which is a
> > vendor specific register so not defined in mii.h):
> >
> > ksz_phy_write16(dev, port, 0x1e, 0xfa00);
>
> May be I should add a local macro define for it.
>
> > Your original patch doesn't work in my case (maybe it worked for you
> > because you were trying to address port0?). With the above changes the
> > LED's are properly configured on my boards.
>
> Okay. But I don't really understand why its not working in your
> case.
>
> Thanks for you're feedback.
>
> Regards,
> Heinrich.
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [v1,3/3] net: dsa: microchip: add single-led-mode support for KSZ9477
2026-02-04 0:56 ` Tim Harvey
@ 2026-02-04 10:22 ` Heinrich Toews
0 siblings, 0 replies; 16+ messages in thread
From: Heinrich Toews @ 2026-02-04 10:22 UTC (permalink / raw)
To: Tim Harvey
Cc: netdev, Andrew Lunn, Florian Fainelli, Vladimir Oltean,
Rob Herring
Hi Tim,
On Tue, Feb 03, 2026 at 04:56:30PM -0800, Tim Harvey wrote:
> On Tue, Feb 3, 2026 at 8:58 AM Heinrich Toews <ht@twx-software.de> wrote:
> >
> > Hi Rob,
Oh Sorry, I see that I was accidentally using the wrong name in my last message.
[...]
> > I added debug output and tested my setup on all 4 external ports and it
> > works:
> >
> > ksz-switch spi1.0: ksz_pwrite16: port: 1, offset: 0x011a, data: 0x0002
> > ksz-switch spi1.0: ksz_write16: reg: 0x0000211a, value: 0x0002
> > ^^^^
> > The port index
> > is set
> > correctly.
> >
> > [...]
> >
> > ksz-switch spi1.0: ksz_pwrite16: port: 2, offset: 0x011a, data: 0x0002
> > ksz-switch spi1.0: ksz_write16: reg: 0x0000311a, value: 0x0002
> > ^^^^
> > The same here
> > on port 2.
> >
> > Is see activity on the leds connected to LEDx_0.
> >
>
> Hi Heinrich,
>
> My apologies - you are correct. I accidentally missed the hunk where
> you set your registers in ksz9477_regs causing it to not work for me.
Ahh okay. Now I understand.
> > ksz9477_get_port_addr() is adding the port index by PORT_CTRL_ADDR(port, offset).
> >
> > But since I'm accessing PHY registers here, I'll switch to ksz_phy_write16() in v2.
>
> Yes, that would be best as you would no longer need to add custom registers.
Right, this is also a goog reason to switch.
> I have:
>
> ksz_phy_write16(dev->ds, port, MII_MMD_CTRL, 0x0002);
> ksz_phy_write16(dev->ds, port, MII_MMD_DATA, 0x0000);
> ksz_phy_write16(dev->ds, port, MII_MMD_CTRL,
> MII_MMD_CTRL_NOINCR | 0x02);
> ksz_phy_write16(dev->ds, port, MII_MMD_DATA, 0x0010); /* set bit 4 */
> /* LED_ERRATA requires writing 0xfa00 to Debug Register 3 (PHY
> register 0x1e) */
> if (phydev->dev_flags & MICREL_KSZ9_LED_ERRATA)
> ksz_phy_write16(dev->ds, port, 0x1e, 0xfa00);
Thanks. But I'm thinking to add a local '#define MII_PHY_DIGITAL_DEBUG_3 0x1e' here.
> I look forward to seeing your next submission.
I hopefully will get it ready this week.
Regards,
Heinrich.
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2026-02-04 10:22 UTC | newest]
Thread overview: 16+ 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 21:46 Tim Harvey
2026-02-03 16:58 ` Heinrich Toews
2026-02-04 0:56 ` Tim Harvey
2026-02-04 10:22 ` Heinrich Toews
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox