[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-Id: <20250812105928.2124169-1-vladimir.oltean@nxp.com>
Date: Tue, 12 Aug 2025 13:59:28 +0300
From: Vladimir Oltean <vladimir.oltean@....com>
To: netdev@...r.kernel.org
Cc: Andrew Lunn <andrew@...n.ch>,
Heiner Kallweit <hkallweit1@...il.com>,
Russell King <linux@...linux.org.uk>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>,
Stas Sergeev <stsp@...t.ru>,
linux-kernel@...r.kernel.org
Subject: [PATCH net] net: explicitly check in of_phy_is_fixed_link() for managed = "in-band-status"
In phylib-based systems, there are at least two ways of handling a link
to a PHY whose registers are unavailable over MDIO. The better known
implementation is phylink, the lesser known one was added by Stas
Sergeev two years prior, in commit 898b2970e2c9 ("mvneta: implement
SGMII-based in-band link state signaling") and its various follow-ups.
There are two sub-cases of the MDIO-less PHY to consider. First is the
case where the PHY would at least emit in-band autoneg code groups.
The firmware description of this case looks like below (a):
mac {
managed = "in-band-status";
phy-mode = "sgmii";
};
And the other sub-case is when the MDIO-less PHY is also silent on the
in-band autoneg front. In that case, the firmware description would look
like this (b):
mac {
phy-mode = "sgmii";
fixed-link {
speed = <10000>;
full-duplex;
};
};
(side note: phylink would probably have something to object against the
PHY not reporting its state in any way, and would consider the setup
invalid, even if in some cases it would work. This is because its
configuration may not be fixed, and there would be no way to be notified
of updates)
Concentrating on sub-case (a), Stas Sergeev's mvneta implementation
differs from the later phylink implementation which also took over in
mvneta.
In the well known phylink model, the phylib PHY is completely optional,
and the pl->cfg_link_an_mode will be placed in MLO_AN_INBAND.
Whereas Stas Sergeev admittedly took "the path of least resistance" and
worked with what was available, i.e. the fixed PHY software emulation:
https://lore.kernel.org/lkml/55156730.5030807@list.ru/
Commit 4cba5c210365 ("of_mdio: add new DT property 'managed' to specify
the PHY management type") made of_phy_is_fixed_link() return true for
sub-case (a), so that the fixed PHY driver would handle it. From
forensic evidence, I believe that was done to have unified phylib driver
handling with sub-case (b).
We want to preserve that behavior, but if other values for the "managed"
property have to be introduced, it means of_phy_is_fixed_link() will
automatically return true for them. As a general rule, that doesn't make
any sense. For example, managed = "c73" may be added to mean that the
operating interface of a port is selected through IEEE 802.3 clause 73
(backplane) auto-negotiation.
So, we need to be explicit about the check.
Documentation/devicetree/bindings/net/ethernet-controller.yaml makes it
clear that the 2 allowed values for "managed" are "auto" and
"in-band-status". So, given the current binding, strcmp(managed, "auto") != 0
should be exactly equivalent with strcmp(managed, "in-band-status") == 0.
The difference is made for new additions.
The Fixes: tag and backport to stable is justified by the fact that new
device trees need to do something reasonable with old kernels.
Fixes: 4cba5c210365 ("of_mdio: add new DT property 'managed' to specify the PHY management type")
Signed-off-by: Vladimir Oltean <vladimir.oltean@....com>
---
drivers/net/mdio/of_mdio.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/mdio/of_mdio.c b/drivers/net/mdio/of_mdio.c
index 98f667b121f7..8e8a34293a8b 100644
--- a/drivers/net/mdio/of_mdio.c
+++ b/drivers/net/mdio/of_mdio.c
@@ -401,7 +401,7 @@ bool of_phy_is_fixed_link(struct device_node *np)
}
err = of_property_read_string(np, "managed", &managed);
- if (err == 0 && strcmp(managed, "auto") != 0)
+ if (err == 0 && strcmp(managed, "in-band-status") == 0)
return true;
/* Old binding */
--
2.34.1
Powered by blists - more mailing lists