[PATCH net-next] net: phy: intel-xway: workaround stale LEDs before link-up

[PATCH net-next] net: phy: intel-xway: workaround stale LEDs before link-up

[Daniel Golle]

Due to a bug in some PHY internal firmware, manual control as well as
polarity configuration of the PHY LEDs has no effect until a link has
been detected at least once after reset. Apparently the LED control
thread is not started until then.

As a workaround, clear the BMCR_ANENABLE bit for 100ms to force the
firmware to start the LED thread, allowing manual LED control and
respecting LED polarity before the first link comes up.

In case the legacy default LED configuration is used the bug isn't
visible, so only apply the workaround in case LED configuration is
present in the device tree.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 drivers/net/phy/intel-xway.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/intel-xway.c b/drivers/net/phy/intel-xway.c
index 9766dd99afaa0..37150115c8edb 100644
--- a/drivers/net/phy/intel-xway.c
+++ b/drivers/net/phy/intel-xway.c
@@ -4,6 +4,7 @@
  * Copyright (C) 2016 Hauke Mehrtens <hauke@hauke-m.de>
  */
 
+#include <linux/delay.h>
 #include <linux/mdio.h>
 #include <linux/module.h>
 #include <linux/phy.h>
@@ -286,8 +287,33 @@ static int xway_gphy_config_init(struct phy_device *phydev)
 		return err;
 
 	/* Use default LED configuration if 'leds' node isn't defined */
-	if (!of_get_child_by_name(np, "leds"))
+	if (!of_get_child_by_name(np, "leds")) {
 		xway_gphy_init_leds(phydev);
+	} else {
+		/* Due to a bug in some PHY internal firmware, manual control
+		 * as well as polarity configuration of the PHY LEDs has no
+		 * effect until a link has been detected at least once after
+		 * reset. Apparently the LED control thread is not started
+		 * until then.
+		 *
+		 * Workaround: clear the BMCR_ANENABLE bit to force the firmware
+		 * to start the LED thread, allowing manual LED control and
+		 * respecting LED polarity before the first link comes up.
+		 *
+		 * In case the default LED configuration is used the bug isn't
+		 * visible, so only apply the workaround in case LED
+		 * configuration is present in the device tree.
+		 */
+		phy_clear_bits(phydev, MII_BMCR, BMCR_ANENABLE);
+		/* 100ms was found to be required by experimentation.
+		 * A shorter delay causes other (serious) bugs...
+		 */
+		msleep(100);
+		/* It turns out BMCR_ANENABLE *has* to be set when removing
+		 * BMCR_PDOWN for not facing yet different bugs
+		 */
+		phy_set_bits(phydev, MII_BMCR, BMCR_ANENABLE);
+	}
 
 	/* Clear all pending interrupts */
 	phy_read(phydev, XWAY_MDIO_ISTAT);
-- 
2.52.0

Re: [PATCH net-next] net: phy: intel-xway: workaround stale LEDs before link-up

[Andrew Lunn]

On Thu, Jan 15, 2026 at 11:40:38PM +0000, Daniel Golle wrote:
> Due to a bug in some PHY internal firmware, manual control as well as
> polarity configuration of the PHY LEDs has no effect until a link has
> been detected at least once after reset. Apparently the LED control
> thread is not started until then.
> 
> As a workaround, clear the BMCR_ANENABLE bit for 100ms to force the
> firmware to start the LED thread, allowing manual LED control and
> respecting LED polarity before the first link comes up.
> 
> In case the legacy default LED configuration is used the bug isn't
> visible, so only apply the workaround in case LED configuration is
> present in the device tree.

You should consider the case of forced links, where autoneg is
disabled. Under such conditions, you should not leave autoneg enabled.

	  Andrew

Re: [PATCH net-next] net: phy: intel-xway: workaround stale LEDs before link-up

[Daniel Golle]

On Fri, Jan 16, 2026 at 02:23:18AM +0100, Andrew Lunn wrote:
> On Thu, Jan 15, 2026 at 11:40:38PM +0000, Daniel Golle wrote:
> > Due to a bug in some PHY internal firmware, manual control as well as
> > polarity configuration of the PHY LEDs has no effect until a link has
> > been detected at least once after reset. Apparently the LED control
> > thread is not started until then.
> > 
> > As a workaround, clear the BMCR_ANENABLE bit for 100ms to force the
> > firmware to start the LED thread, allowing manual LED control and
> > respecting LED polarity before the first link comes up.
> > 
> > In case the legacy default LED configuration is used the bug isn't
> > visible, so only apply the workaround in case LED configuration is
> > present in the device tree.
> 
> You should consider the case of forced links, where autoneg is
> disabled. Under such conditions, you should not leave autoneg enabled.

If BMCR_ANENABLE has already been disabled after a reset we can skip
this workaround entirely, as doing that once for more than 100ms is all
needed for the LEDs to work properly.

However, I'm not aware of .config_init ever being run again after the
intial attachment of the PHY and call to phy_init_hw().

All user-defined configuration happens after that, and would then remove
the BMCR_ANENABLE bit just like it would do it if it was set by the
hardware after reset.
(note that BMCR_ANENABLE is set as part of the reset value of BMCR on
this PHY)

But maybe I'm getting something wrong here?

Re: [net-next] net: phy: intel-xway: workaround stale LEDs before link-up

[Jakub Kicinski]

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: phy: intel-xway: workaround stale LEDs before link-up

This patch works around a PHY firmware bug where LED manual control and
polarity configuration have no effect until a link is detected. The fix
temporarily clears BMCR_ANENABLE for 100ms to force the LED thread to start.

> diff --git a/drivers/net/phy/intel-xway.c b/drivers/net/phy/intel-xway.c
> index 9766dd99afaa..e06ed7ab0ff3 100644
> --- a/drivers/net/phy/intel-xway.c
> +++ b/drivers/net/phy/intel-xway.c

[ ... ]

> @@ -286,8 +287,33 @@ static int xway_gphy_config_init(struct phy_device *phydev)
>  		return err;
>
>  	/* Use default LED configuration if 'leds' node isn't defined */
> -	if (!of_get_child_by_name(np, "leds"))
> +	if (!of_get_child_by_name(np, "leds")) {
>  		xway_gphy_init_leds(phydev);
> +	} else {

Does this leak the device_node reference returned by of_get_child_by_name()?

The API documentation states that of_get_child_by_name() returns a node
pointer with refcount incremented, requiring of_node_put() when done.
When the "leds" node exists, the returned pointer is never captured and
of_node_put() is never called.

Other PHY drivers handle this correctly, for example vsc85xx_probe_common()
in drivers/net/phy/mscc/mscc_main.c:

    np = of_get_child_by_name(phydev->mdio.dev.of_node, "leds");
    if (np) {
        of_node_put(np);
        ...
    }

And of_phy_leds() in drivers/net/phy/phy_device.c also properly calls
of_node_put(leds) on all exit paths.

Note that this leak also existed in the original code before this patch,
but since this patch is touching this exact code path, it would be a good
opportunity to fix it.
-- 
pw-bot: cr

Re: [net-next] net: phy: intel-xway: workaround stale LEDs before link-up

[Jakub Kicinski]

On Sat, 17 Jan 2026 18:29:07 -0800 Jakub Kicinski wrote:
> > @@ -286,8 +287,33 @@ static int xway_gphy_config_init(struct phy_device *phydev)
> >  		return err;
> >
> >  	/* Use default LED configuration if 'leds' node isn't defined */
> > -	if (!of_get_child_by_name(np, "leds"))
> > +	if (!of_get_child_by_name(np, "leds")) {
> >  		xway_gphy_init_leds(phydev);
> > +	} else {  
> 
> Does this leak the device_node reference returned by of_get_child_by_name()?

Of course this is a pre-existing issue but could you fix it first
in net then proceed with this submission? Otherwise we'll have a
conflict.

Re: [net-next] net: phy: intel-xway: workaround stale LEDs before link-up

[Daniel Golle]

On Sat, Jan 17, 2026 at 06:31:45PM -0800, Jakub Kicinski wrote:
> On Sat, 17 Jan 2026 18:29:07 -0800 Jakub Kicinski wrote:
> > > @@ -286,8 +287,33 @@ static int xway_gphy_config_init(struct phy_device *phydev)
> > >  		return err;
> > >
> > >  	/* Use default LED configuration if 'leds' node isn't defined */
> > > -	if (!of_get_child_by_name(np, "leds"))
> > > +	if (!of_get_child_by_name(np, "leds")) {
> > >  		xway_gphy_init_leds(phydev);
> > > +	} else {  
> > 
> > Does this leak the device_node reference returned by of_get_child_by_name()?
> 
> Of course this is a pre-existing issue but could you fix it first
> in net then proceed with this submission? Otherwise we'll have a
> conflict.

Ack. I'll send the fix for this to net first, then resend this patch
after the merge of net-next and net.