* [PATCH 2/5] net: phy: mxl-gpy: add MxL86211C MDIO I/O regulator
2026-09-30 12:22 [PATCH 1/5] dt-bindings: net: maxlinear: add MxL86211C MDIO I/O regulator Robert Marko
@ 2026-09-30 12:22 ` Robert Marko
2026-10-04 13:17 ` netdev-bot+sashiko
2026-09-30 12:22 ` [PATCH 3/5] net: phy: mxl-gpy: release MxL86211C Super Isolate Robert Marko
` (3 subsequent siblings)
4 siblings, 1 reply; 16+ messages in thread
From: Robert Marko @ 2026-09-30 12:22 UTC (permalink / raw)
To: andrew+netdev, davem, edumazet, kuba, pabeni, robh, krzk+dt,
conor+dt, hkallweit1, lxu, michael, netdev, devicetree,
linux-kernel
Cc: luka.perkov, Robert Marko
The MxL86211C can configure its MDIO, MDC, and MDINT pads for
1.8 V or 3.3 V signaling through VSPEC1_PM_CTRL.MDIO_VOL.
MxL86211C is a bit weird in the sense that it power the MDIO pins via
the VDPP pad, so you must provide it with 1.8V or 3.3V there.
However, it does not automatically configure the VSPEC1_PM_CTRL.MDIO_VOL
bit and defaults to 3.3V, so if you are using 1.8V in order to have
reliable MDIO communication you must set that bit.
So, model the MDIO I/O voltage setting as a regulator.
Signed-off-by: Robert Marko <robert.marko@sartura.hr>
---
drivers/net/phy/Kconfig | 1 +
drivers/net/phy/mxl-gpy.c | 74 ++++++++++++++++++++++++++++++++++++++-
2 files changed, 74 insertions(+), 1 deletion(-)
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index d3835597e3795..bdaee0cb4829d 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -302,6 +302,7 @@ config MAXLINEAR_GPHY
tristate "Maxlinear Ethernet PHYs"
select POLYNOMIAL if HWMON
depends on HWMON || HWMON=n
+ depends on REGULATOR
help
Support for the Maxlinear GPY115, GPY211, GPY212, GPY215,
GPY241, GPY245 PHYs.
diff --git a/drivers/net/phy/mxl-gpy.c b/drivers/net/phy/mxl-gpy.c
index 089a1a9e5bc61..4cbbfae046317 100644
--- a/drivers/net/phy/mxl-gpy.c
+++ b/drivers/net/phy/mxl-gpy.c
@@ -14,6 +14,8 @@
#include <linux/polynomial.h>
#include <linux/property.h>
#include <linux/netdevice.h>
+#include <linux/regulator/driver.h>
+#include <linux/of.h>
/* PHY ID */
#define PHY_ID_GPYx15B_MASK 0xFFFFFFFC
@@ -110,6 +112,10 @@
#define VSPEC1_SGMII_ANEN_ANRS (VSPEC1_SGMII_CTRL_ANEN | \
VSPEC1_SGMII_CTRL_ANRS)
+/* Packet Manager Control */
+#define VSPEC1_PM_CTRL 0x0c
+#define VSPEC1_PM_CTRL_MDIO_VOL BIT(14)
+
/* Temperature sensor */
#define VSPEC1_TEMP_STA 0x0E
#define VSPEC1_TEMP_STA_DATA GENMASK(9, 0)
@@ -390,6 +396,72 @@ static int gpy_probe(struct phy_device *phydev)
return 0;
}
+static int mxl86211c_mdio_reg_set_voltage_sel(struct regulator_dev *rdev,
+ unsigned int selector)
+{
+ struct phy_device *phydev = rdev_get_drvdata(rdev);
+
+ return phy_modify_mmd(phydev, MDIO_MMD_VEND1, VSPEC1_PM_CTRL,
+ VSPEC1_PM_CTRL_MDIO_VOL,
+ selector ? 0 : VSPEC1_PM_CTRL_MDIO_VOL);
+}
+
+static int mxl86211c_mdio_reg_get_voltage_sel(struct regulator_dev *rdev)
+{
+ struct phy_device *phydev = rdev_get_drvdata(rdev);
+ int val;
+
+ val = phy_read_mmd(phydev, MDIO_MMD_VEND1, VSPEC1_PM_CTRL);
+ if (val < 0)
+ return val;
+
+ return (val & VSPEC1_PM_CTRL_MDIO_VOL) ? 0 : 1;
+}
+
+static const struct regulator_ops mxl86211c_mdio_regulator_ops = {
+ .list_voltage = regulator_list_voltage_table,
+ .set_voltage_sel = mxl86211c_mdio_reg_set_voltage_sel,
+ .get_voltage_sel = mxl86211c_mdio_reg_get_voltage_sel,
+};
+
+static const unsigned int mxl86211c_mdio_voltage_table[] = {
+ 1800000,
+ 3300000,
+};
+
+static const struct regulator_desc mxl86211c_mdio_desc = {
+ .name = "mdio",
+ .of_match = of_match_ptr("mdio-regulator"),
+ .n_voltages = ARRAY_SIZE(mxl86211c_mdio_voltage_table),
+ .volt_table = mxl86211c_mdio_voltage_table,
+ .ops = &mxl86211c_mdio_regulator_ops,
+ .type = REGULATOR_VOLTAGE,
+ .owner = THIS_MODULE,
+};
+
+static int mxl86211c_probe(struct phy_device *phydev)
+{
+ struct device *dev = &phydev->mdio.dev;
+ struct regulator_config config = { };
+ struct regulator_dev *rdev;
+ int ret;
+
+ ret = gpy_probe(phydev);
+ if (ret)
+ return ret;
+
+ config.dev = dev;
+ config.driver_data = phydev;
+
+ rdev = devm_regulator_register(dev, &mxl86211c_mdio_desc, &config);
+ if (IS_ERR(rdev)) {
+ phydev_err(phydev, "failed to register MDIO regulator\n");
+ return PTR_ERR(rdev);
+ }
+
+ return 0;
+}
+
static bool gpy_sgmii_need_reaneg(struct phy_device *phydev)
{
struct gpy_priv *priv = phydev->priv;
@@ -1406,7 +1478,7 @@ static struct phy_driver gpy_drivers[] = {
.name = "Maxlinear Ethernet MxL86211C",
.get_features = genphy_c45_pma_read_abilities,
.config_init = gpy21x_config_init,
- .probe = gpy_probe,
+ .probe = mxl86211c_probe,
.inband_caps = gpy_inband_caps,
.config_inband = gpy_config_inband,
.suspend = genphy_suspend,
--
2.55.0
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 2/5] net: phy: mxl-gpy: add MxL86211C MDIO I/O regulator
2026-09-30 12:22 ` [PATCH 2/5] net: phy: mxl-gpy: " Robert Marko
@ 2026-10-04 13:17 ` netdev-bot+sashiko
0 siblings, 0 replies; 16+ messages in thread
From: netdev-bot+sashiko @ 2026-10-04 13:17 UTC (permalink / raw)
To: robert.marko
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, robh, krzk+dt,
conor+dt, hkallweit1, lxu, michael, netdev, devicetree,
linux-kernel, luka.perkov
Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.
Critical: 0 · High: 1 · Medium: 1 · Low: 0
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [High] The driver programs VSPEC1_PM_CTRL.MDIO_VOL only as a side
effect of devm_regulator_register() in mxl86211c_probe(), via…
- [Medium] The patch adds an unconditional `depends on REGULATOR` to
MAXLINEAR_GPHY (drivers/net/phy/Kconfig:305), so the whole mxl-gpy…
--
net: phy: mxl-gpy: add MxL86211C MDIO I/O regulator
This commit models the MxL86211C VSPEC1_PM_CTRL.MDIO_VOL bit as a
regulator. The bit selects 1.8V or 3.3V signaling for the MDIO, MDC and
MDINT pads. The regulator is registered from a new mxl86211c_probe(), and
MAXLINEAR_GPHY now depends on REGULATOR.
> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> index d3835597e3795..bdaee0cb4829d 100644
> --- a/drivers/net/phy/Kconfig
> +++ b/drivers/net/phy/Kconfig
> @@ -302,6 +302,7 @@ config MAXLINEAR_GPHY
> tristate "Maxlinear Ethernet PHYs"
> select POLYNOMIAL if HWMON
> depends on HWMON || HWMON=n
> + depends on REGULATOR
[Severity: Medium]
Does this unconditional dependency hide the whole mxl-gpy driver when
REGULATOR=n?
MAXLINEAR_GPHY covers GPY2xx, GPY115B/C, GPY211B/C, GPY212B/C, GPY215B/C,
GPY241B/BM, GPY245B, MxL86211C, MxL86252 and MxL86282. Only the MxL86211C
probe path uses the regulator API.
Take an existing .config with REGULATOR=n and MAXLINEAR_GPHY=y/m. After
olddefconfig, MAXLINEAR_GPHY is silently dropped and Kconfig gives no
warning. Those PHYs then fall back to genphy and lose SGMII/2500BASE-X
handling, inband AN, LEDs, WoL and hwmon.
include/linux/regulator/driver.h has no !CONFIG_REGULATOR stubs for
devm_regulator_register() or rdev_get_drvdata(). That is probably why the
dependency covers the whole driver.
Could the regulator code in mxl86211c_probe() be guarded with
IS_ENABLED(CONFIG_REGULATOR) instead, so the rest of the PHY family does
not need the regulator framework?
The commit message does not mention this dependency change. The last patch
in the series still has the unconditional dependency.
> help
> Support for the Maxlinear GPY115, GPY211, GPY212, GPY215,
> GPY241, GPY245 PHYs.
> diff --git a/drivers/net/phy/mxl-gpy.c b/drivers/net/phy/mxl-gpy.c
> index 089a1a9e5bc61..4cbbfae046317 100644
> --- a/drivers/net/phy/mxl-gpy.c
> +++ b/drivers/net/phy/mxl-gpy.c
[ ... ]
> @@ -390,6 +396,72 @@ static int gpy_probe(struct phy_device *phydev)
> return 0;
> }
>
> +static int mxl86211c_mdio_reg_set_voltage_sel(struct regulator_dev *rdev,
> + unsigned int selector)
> +{
> + struct phy_device *phydev = rdev_get_drvdata(rdev);
> +
> + return phy_modify_mmd(phydev, MDIO_MMD_VEND1, VSPEC1_PM_CTRL,
> + VSPEC1_PM_CTRL_MDIO_VOL,
> + selector ? 0 : VSPEC1_PM_CTRL_MDIO_VOL);
> +}
[ ... ]
> +static int mxl86211c_probe(struct phy_device *phydev)
> +{
> + struct device *dev = &phydev->mdio.dev;
> + struct regulator_config config = { };
> + struct regulator_dev *rdev;
> + int ret;
> +
> + ret = gpy_probe(phydev);
> + if (ret)
> + return ret;
> +
> + config.dev = dev;
> + config.driver_data = phydev;
> +
> + rdev = devm_regulator_register(dev, &mxl86211c_mdio_desc, &config);
[Severity: High]
Is MDIO_VOL restored after a PHY hardware reset?
The bit is written only in two cases. One is devm_regulator_register(),
when the regulator core applies the apply_uV constraints once at
registration. The other is a consumer calling set_voltage. Nothing writes
it again after that.
phylib asserts the PHY reset in phy_detach(). It releases the reset in
phy_init_hw(), which then calls only drv->config_init, not probe:
drivers/net/phy/phy_device.c:phy_detach() {
...
/* Assert the reset signal */
phy_device_reset(phydev, 1);
...
}
drivers/net/phy/phy_device.c:phy_init_hw() {
...
/* Deassert the reset signal */
phy_device_reset(phydev, 0);
...
}
Consider a board with 1.8V VDDP and reset-gpios in DT, behind a MAC that
disconnects the PHY in ndo_stop. An ifdown/ifup cycle would go through:
phy_detach()->phy_device_reset(phydev, 1)
phy_attach_direct()->phy_init_hw()->phy_device_reset(phydev, 0)
phy_init_hw()->gpy21x_config_init()
The MxL86211C entry has no soft_reset. gpy21x_config_init() never touches
VSPEC1_PM_CTRL.
The commit message says the bit defaults to 3.3V. After such a reset,
would the pads go back to 3.3V signaling and bring back the unreliable
MDIO that this patch is meant to fix? The registered regulator would then
also report 3.3V, which no longer matches the board. Losing the supply
across suspend would have the same effect.
Would it work to cache the requested selector and re-apply it from
config_init? This is still the case at the end of the series, where
mxl86211c_config_init() only adds the lane ASP mapping.
> + if (IS_ERR(rdev)) {
> + phydev_err(phydev, "failed to register MDIO regulator\n");
> + return PTR_ERR(rdev);
> + }
> +
> + return 0;
> +}
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260930122305.293686-1-robert.marko%40sartura.hr
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 3/5] net: phy: mxl-gpy: release MxL86211C Super Isolate
2026-09-30 12:22 [PATCH 1/5] dt-bindings: net: maxlinear: add MxL86211C MDIO I/O regulator Robert Marko
2026-09-30 12:22 ` [PATCH 2/5] net: phy: mxl-gpy: " Robert Marko
@ 2026-09-30 12:22 ` Robert Marko
2026-10-04 13:17 ` netdev-bot+sashiko
2026-09-30 12:22 ` [PATCH 4/5] dt-bindings: net: maxlinear: add MxL86211C lane mapping support Robert Marko
` (2 subsequent siblings)
4 siblings, 1 reply; 16+ messages in thread
From: Robert Marko @ 2026-09-30 12:22 UTC (permalink / raw)
To: andrew+netdev, davem, edumazet, kuba, pabeni, robh, krzk+dt,
conor+dt, hkallweit1, lxu, michael, netdev, devicetree,
linux-kernel
Cc: luka.perkov, Robert Marko
The MxL86211C can be strapped into Super Isolate mode, leaving
the PHY halted after reset until software releases it.
Clear VSPEC1_PM_CTRL.SI after registering the MDIO voltage regulator
and before the generic GPY initialization accesses the PHY.
Signed-off-by: Robert Marko <robert.marko@sartura.hr>
---
drivers/net/phy/mxl-gpy.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/net/phy/mxl-gpy.c b/drivers/net/phy/mxl-gpy.c
index 4cbbfae046317..790075632fc3f 100644
--- a/drivers/net/phy/mxl-gpy.c
+++ b/drivers/net/phy/mxl-gpy.c
@@ -115,6 +115,7 @@
/* Packet Manager Control */
#define VSPEC1_PM_CTRL 0x0c
#define VSPEC1_PM_CTRL_MDIO_VOL BIT(14)
+#define VSPEC1_PM_CTRL_SI BIT(12) /* Super Isolate */
/* Temperature sensor */
#define VSPEC1_TEMP_STA 0x0E
@@ -446,10 +447,6 @@ static int mxl86211c_probe(struct phy_device *phydev)
struct regulator_dev *rdev;
int ret;
- ret = gpy_probe(phydev);
- if (ret)
- return ret;
-
config.dev = dev;
config.driver_data = phydev;
@@ -459,7 +456,15 @@ static int mxl86211c_probe(struct phy_device *phydev)
return PTR_ERR(rdev);
}
- return 0;
+ ret = phy_modify_mmd(phydev, MDIO_MMD_VEND1, VSPEC1_PM_CTRL,
+ VSPEC1_PM_CTRL_SI,
+ 0);
+ if (ret)
+ return ret;
+
+ ret = gpy_probe(phydev);
+
+ return ret;
}
static bool gpy_sgmii_need_reaneg(struct phy_device *phydev)
--
2.55.0
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 3/5] net: phy: mxl-gpy: release MxL86211C Super Isolate
2026-09-30 12:22 ` [PATCH 3/5] net: phy: mxl-gpy: release MxL86211C Super Isolate Robert Marko
@ 2026-10-04 13:17 ` netdev-bot+sashiko
0 siblings, 0 replies; 16+ messages in thread
From: netdev-bot+sashiko @ 2026-10-04 13:17 UTC (permalink / raw)
To: robert.marko
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, robh, krzk+dt,
conor+dt, hkallweit1, lxu, michael, netdev, devicetree,
linux-kernel, luka.perkov
Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.
Critical: 0 · High: 1 · Medium: 0 · Low: 0
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [High] The patch clears Super Isolate only once, in mxl86211c_probe()
(drivers/net/phy/mxl-gpy.c:459-463).
--
net: phy: mxl-gpy: release MxL86211C Super Isolate
This commit clears VSPEC1_PM_CTRL.SI in mxl86211c_probe() so that a
MxL86211C strapped into Super Isolate mode gets released. The clear
happens after the MDIO voltage regulator is registered, and gpy_probe()
now runs after it.
> diff --git a/drivers/net/phy/mxl-gpy.c b/drivers/net/phy/mxl-gpy.c
> index 4cbbfae046317..790075632fc3f 100644
> --- a/drivers/net/phy/mxl-gpy.c
> +++ b/drivers/net/phy/mxl-gpy.c
[ ... ]
> @@ -459,7 +456,15 @@ static int mxl86211c_probe(struct phy_device *phydev)
> return PTR_ERR(rdev);
> }
>
> - return 0;
> + ret = phy_modify_mmd(phydev, MDIO_MMD_VEND1, VSPEC1_PM_CTRL,
> + VSPEC1_PM_CTRL_SI,
> + 0);
[Severity: High]
Can the PHY end up back in Super Isolate after a later hardware reset?
The commit message says the strap leaves "the PHY halted after reset until
software releases it". However, SI is only cleared here in probe, and
probe runs once per driver bind.
On boards that have a PHY reset-gpio or reset controller, phylib can
pulse the reset line after probe. phy_detach_internal() asserts it:
drivers/net/phy/phy_device.c:phy_detach_internal() {
...
/* Assert the reset signal */
phy_device_reset(phydev, 1);
...
}
On the next attach or resume, phy_init_hw() releases reset. After that it
only calls soft_reset (this PHY has none) and config_init:
drivers/net/phy/phy_device.c:phy_init_hw() {
/* Deassert the reset signal */
phy_device_reset(phydev, 0);
...
if (phydev->drv->config_init) {
ret = phydev->drv->config_init(phydev);
...
}
The MxL86211C entry in genphy_driver[] uses gpy21x_config_init() as its
.config_init. That function only sets possible_interfaces and writes
PHY_ERRCNT, and nothing else in the driver clears VSPEC1_PM_CTRL_SI.
Take a MAC driver that connects the PHY in ndo_open and disconnects it in
ndo_stop. Wouldn't an "ip link set dev X down; ip link set dev X up"
cycle bring the PHY back isolated, with no link until the driver is
rebound?
The same could happen on a resume that power-cycles the PHY, because
mdio_bus_phy_resume() also goes through phy_init_hw().
Later in the series, "net: phy: mxl-gpy: support MxL86211C lane mapping"
adds mxl86211c_config_init(). It only calls
mxl86211c_configure_lane_asp_map() and gpy21x_config_init(), so the SI
clear is still only done in probe at the end of the series.
Would it be better to clear SI in a MxL86211C-specific config_init, so
that it runs after every phy_init_hw()?
> + if (ret)
> + return ret;
> +
> + ret = gpy_probe(phydev);
> +
> + return ret;
> }
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260930122305.293686-1-robert.marko%40sartura.hr
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 4/5] dt-bindings: net: maxlinear: add MxL86211C lane mapping support
2026-09-30 12:22 [PATCH 1/5] dt-bindings: net: maxlinear: add MxL86211C MDIO I/O regulator Robert Marko
2026-09-30 12:22 ` [PATCH 2/5] net: phy: mxl-gpy: " Robert Marko
2026-09-30 12:22 ` [PATCH 3/5] net: phy: mxl-gpy: release MxL86211C Super Isolate Robert Marko
@ 2026-09-30 12:22 ` Robert Marko
2026-10-01 5:55 ` Krzysztof Kozlowski
` (2 more replies)
2026-09-30 12:22 ` [PATCH 5/5] net: phy: mxl-gpy: support MxL86211C lane mapping Robert Marko
2026-10-04 13:17 ` [PATCH 1/5] dt-bindings: net: maxlinear: add MxL86211C MDIO I/O regulator netdev-bot+sashiko
4 siblings, 3 replies; 16+ messages in thread
From: Robert Marko @ 2026-09-30 12:22 UTC (permalink / raw)
To: andrew+netdev, davem, edumazet, kuba, pabeni, robh, krzk+dt,
conor+dt, hkallweit1, lxu, michael, netdev, devicetree,
linux-kernel
Cc: luka.perkov, Robert Marko
Describe the optional lane-to-ASP mapping for the MxL86211C PHY.
The mapping accounts for board-level swaps between its physical TPI
lanes and analog signal processing lanes.
MxL86211C supports per lane mapping.
Signed-off-by: Robert Marko <robert.marko@sartura.hr>
---
.../bindings/net/maxlinear,gpy2xx.yaml | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml b/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml
index 0645e885f1747..b98cb3c3e6d49 100644
--- a/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml
+++ b/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml
@@ -22,6 +22,21 @@ allOf:
then:
properties:
+ maxlinear,lane-asp-map:
+ description: |
+ Mapping of the physical TPI lanes A through D to the PHY's
+ analog signal processing lanes (ASPs). The array index identifies
+ physical lane A, B, C, or D, while its value identifies ASP A, B,
+ C, or D, encoded as 0 through 3. Each ASP must be mapped exactly
+ once. Omit the property to retain the hardware reset mapping.
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ minItems: 4
+ maxItems: 4
+ uniqueItems: true
+ items:
+ minimum: 0
+ maximum: 3
+
mdio-regulator:
type: object
description: |
@@ -34,6 +49,7 @@ allOf:
else:
properties:
+ maxlinear,lane-asp-map: false
mdio-regulator: false
properties:
@@ -78,6 +94,9 @@ examples:
"ethernet-phy-ieee802.3-c45";
reg = <0>;
+ /* Swap physical TPI lanes C and D. */
+ maxlinear,lane-asp-map = <0 1 3 2>;
+
mdio: mdio-regulator {
regulator-min-microvolt = <1800000>;
regulator-max-microvolt = <1800000>;
--
2.55.0
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 4/5] dt-bindings: net: maxlinear: add MxL86211C lane mapping support
2026-09-30 12:22 ` [PATCH 4/5] dt-bindings: net: maxlinear: add MxL86211C lane mapping support Robert Marko
@ 2026-10-01 5:55 ` Krzysztof Kozlowski
2026-10-01 12:14 ` Robert Marko
2026-10-01 16:25 ` Rob Herring (Arm)
2026-10-04 13:17 ` netdev-bot+sashiko
2 siblings, 1 reply; 16+ messages in thread
From: Krzysztof Kozlowski @ 2026-10-01 5:55 UTC (permalink / raw)
To: Robert Marko
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, robh, krzk+dt,
conor+dt, hkallweit1, lxu, michael, netdev, devicetree,
linux-kernel, luka.perkov
On Wed, Sep 30, 2026 at 02:22:16PM +0200, Robert Marko wrote:
> Describe the optional lane-to-ASP mapping for the MxL86211C PHY.
>
> The mapping accounts for board-level swaps between its physical TPI
> lanes and analog signal processing lanes.
>
> MxL86211C supports per lane mapping.
>
> Signed-off-by: Robert Marko <robert.marko@sartura.hr>
> ---
> .../bindings/net/maxlinear,gpy2xx.yaml | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml b/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml
> index 0645e885f1747..b98cb3c3e6d49 100644
> --- a/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml
> +++ b/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml
> @@ -22,6 +22,21 @@ allOf:
>
> then:
> properties:
> + maxlinear,lane-asp-map:
> + description: |
> + Mapping of the physical TPI lanes A through D to the PHY's
> + analog signal processing lanes (ASPs). The array index identifies
Are there other lanes as well? Not ASP? What sort? data-lanes does not
fit here?
> + physical lane A, B, C, or D, while its value identifies ASP A, B,
> + C, or D, encoded as 0 through 3. Each ASP must be mapped exactly
> + once. Omit the property to retain the hardware reset mapping.
> + $ref: /schemas/types.yaml#/definitions/uint32-array
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 4/5] dt-bindings: net: maxlinear: add MxL86211C lane mapping support
2026-10-01 5:55 ` Krzysztof Kozlowski
@ 2026-10-01 12:14 ` Robert Marko
2026-10-01 12:33 ` Andrew Lunn
0 siblings, 1 reply; 16+ messages in thread
From: Robert Marko @ 2026-10-01 12:14 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, robh, krzk+dt,
conor+dt, hkallweit1, lxu, michael, netdev, devicetree,
linux-kernel, luka.perkov
On Thu, Oct 1, 2026 at 7:55 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On Wed, Sep 30, 2026 at 02:22:16PM +0200, Robert Marko wrote:
> > Describe the optional lane-to-ASP mapping for the MxL86211C PHY.
> >
> > The mapping accounts for board-level swaps between its physical TPI
> > lanes and analog signal processing lanes.
> >
> > MxL86211C supports per lane mapping.
> >
> > Signed-off-by: Robert Marko <robert.marko@sartura.hr>
> > ---
> > .../bindings/net/maxlinear,gpy2xx.yaml | 19 +++++++++++++++++++
> > 1 file changed, 19 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml b/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml
> > index 0645e885f1747..b98cb3c3e6d49 100644
> > --- a/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml
> > +++ b/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml
> > @@ -22,6 +22,21 @@ allOf:
> >
> > then:
> > properties:
> > + maxlinear,lane-asp-map:
> > + description: |
> > + Mapping of the physical TPI lanes A through D to the PHY's
> > + analog signal processing lanes (ASPs). The array index identifies
>
> Are there other lanes as well? Not ASP? What sort? data-lanes does not
> fit here?
Hi,
I tried to keep it similar to the datasheet, but looking back, a more
appropriate
name would be ASP instances instead of lanes.
I am not sure if data-lanes describe what it is fully but I am open to ideas.
Regards,
Robert
>
> > + physical lane A, B, C, or D, while its value identifies ASP A, B,
> > + C, or D, encoded as 0 through 3. Each ASP must be mapped exactly
> > + once. Omit the property to retain the hardware reset mapping.
> > + $ref: /schemas/types.yaml#/definitions/uint32-array
>
> Best regards,
> Krzysztof
>
--
Robert Marko
Staff Embedded Linux Engineer
Sartura d.d.
Lendavska ulica 16a
10000 Zagreb, Croatia
Email: robert.marko@sartura.hr
Web: www.sartura.hr
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 4/5] dt-bindings: net: maxlinear: add MxL86211C lane mapping support
2026-10-01 12:14 ` Robert Marko
@ 2026-10-01 12:33 ` Andrew Lunn
0 siblings, 0 replies; 16+ messages in thread
From: Andrew Lunn @ 2026-10-01 12:33 UTC (permalink / raw)
To: Robert Marko
Cc: Krzysztof Kozlowski, andrew+netdev, davem, edumazet, kuba,
pabeni, robh, krzk+dt, conor+dt, hkallweit1, lxu, michael,
netdev, devicetree, linux-kernel, luka.perkov
> > > +++ b/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml
> > > @@ -22,6 +22,21 @@ allOf:
> > >
> > > then:
> > > properties:
> > > + maxlinear,lane-asp-map:
> > > + description: |
> > > + Mapping of the physical TPI lanes A through D to the PHY's
> > > + analog signal processing lanes (ASPs). The array index identifies
> >
> > Are there other lanes as well? Not ASP? What sort? data-lanes does not
> > fit here?
>
> Hi,
> I tried to keep it similar to the datasheet, but looking back, a more
> appropriate
> name would be ASP instances instead of lanes.
>
> I am not sure if data-lanes describe what it is fully but I am open to ideas.
Analogue front end ports? Or even just analogue from ends?
I do get what you mean with your current description, but i have more
domain knowledge than your average DT writer.
Andrew
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 4/5] dt-bindings: net: maxlinear: add MxL86211C lane mapping support
2026-09-30 12:22 ` [PATCH 4/5] dt-bindings: net: maxlinear: add MxL86211C lane mapping support Robert Marko
2026-10-01 5:55 ` Krzysztof Kozlowski
@ 2026-10-01 16:25 ` Rob Herring (Arm)
2026-10-04 13:17 ` netdev-bot+sashiko
2 siblings, 0 replies; 16+ messages in thread
From: Rob Herring (Arm) @ 2026-10-01 16:25 UTC (permalink / raw)
To: Robert Marko
Cc: luka.perkov, conor+dt, krzk+dt, hkallweit1, lxu, devicetree,
davem, andrew+netdev, michael, netdev, linux-kernel, edumazet,
pabeni, kuba
On Wed, 30 Sep 2026 14:22:16 +0200, Robert Marko wrote:
> Describe the optional lane-to-ASP mapping for the MxL86211C PHY.
>
> The mapping accounts for board-level swaps between its physical TPI
> lanes and analog signal processing lanes.
>
> MxL86211C supports per lane mapping.
>
> Signed-off-by: Robert Marko <robert.marko@sartura.hr>
> ---
> .../bindings/net/maxlinear,gpy2xx.yaml | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
My bot found errors running 'make dt_binding_check' on your patch:
yamllint warnings/errors:
dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/net/maxlinear,gpy2xx.example.dtb: ethernet-phy@0 (ethernet-phy-idc133.5400): maxlinear,lane-asp-map: b'\x00\x00\x00\x00\x00\x00\x00\x01\x00\x00\x00\x03\x00\x00\x00\x02' is not of type 'object', 'integer', 'array', 'boolean', 'null'
from schema $id: http://devicetree.org/schemas/dt-core.yaml
doc reference errors (make refcheckdocs):
See https://patchwork.kernel.org/project/devicetree/patch/20260930122305.293686-4-robert.marko@sartura.hr
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] 16+ messages in thread
* Re: [PATCH 4/5] dt-bindings: net: maxlinear: add MxL86211C lane mapping support
2026-09-30 12:22 ` [PATCH 4/5] dt-bindings: net: maxlinear: add MxL86211C lane mapping support Robert Marko
2026-10-01 5:55 ` Krzysztof Kozlowski
2026-10-01 16:25 ` Rob Herring (Arm)
@ 2026-10-04 13:17 ` netdev-bot+sashiko
2 siblings, 0 replies; 16+ messages in thread
From: netdev-bot+sashiko @ 2026-10-04 13:17 UTC (permalink / raw)
To: robert.marko
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, robh, krzk+dt,
conor+dt, hkallweit1, lxu, michael, netdev, devicetree,
linux-kernel, luka.perkov
Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.
Low priority / nit picks for your consideration:
- [Low] The concern says that nothing reads the `maxlinear,lane-asp-map`
property added by this binding, so DTs using it would be silently…
--
dt-bindings: net: maxlinear: add MxL86211C lane mapping support
This commit adds an optional maxlinear,lane-asp-map property to the
maxlinear,gpy2xx binding. The property describes how the MxL86211C
physical TPI lanes map to its analog signal processing lanes. It is
allowed only for the MxL86211C compatible.
> diff --git a/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml b/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml
> index 0645e885f1747..b98cb3c3e6d49 100644
> --- a/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml
> +++ b/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml
> @@ -22,6 +22,21 @@ allOf:
>
> then:
> properties:
> + maxlinear,lane-asp-map:
> + description: |
> + Mapping of the physical TPI lanes A through D to the PHY's
> + analog signal processing lanes (ASPs). The array index identifies
> + physical lane A, B, C, or D, while its value identifies ASP A, B,
> + C, or D, encoded as 0 through 3. Each ASP must be mapped exactly
> + once. Omit the property to retain the hardware reset mapping.
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> + minItems: 4
> + maxItems: 4
> + uniqueItems: true
> + items:
> + minimum: 0
> + maximum: 3
> +
[Severity: Low]
Does anything read maxlinear,lane-asp-map at this point in the series? In
drivers/net/phy/mxl-gpy.c at this commit, mxl86211c_probe() does not
handle a lane map. So a device tree that sets this property would have it
silently ignored.
The concern goes away later in the series. The next patch, "net: phy:
mxl-gpy: support MxL86211C lane mapping", adds
mxl86211c_configure_lane_asp_map(), and mxl86211c_config_init() calls it:
mxl86211c_config_init()
-> mxl86211c_configure_lane_asp_map()
device_property_present(dev, "maxlinear,lane-asp-map")
device_property_read_u32_array(dev, "maxlinear,lane-asp-map", ...)
phy_write_mmd(..., VSPEC1_LANE_ASP_MAP, ...)
That function checks that the four values are a permutation of 0..3
before it writes the register. Once the whole series is applied, the
property has a consumer. Keeping the binding and driver changes in
separate patches is normal for dt-bindings, so this looks fine as posted.
> +
> mdio-regulator:
> type: object
> description: |
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260930122305.293686-1-robert.marko%40sartura.hr
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 5/5] net: phy: mxl-gpy: support MxL86211C lane mapping
2026-09-30 12:22 [PATCH 1/5] dt-bindings: net: maxlinear: add MxL86211C MDIO I/O regulator Robert Marko
` (2 preceding siblings ...)
2026-09-30 12:22 ` [PATCH 4/5] dt-bindings: net: maxlinear: add MxL86211C lane mapping support Robert Marko
@ 2026-09-30 12:22 ` Robert Marko
2026-09-30 15:35 ` Andrew Lunn
2026-10-04 13:17 ` netdev-bot+sashiko
2026-10-04 13:17 ` [PATCH 1/5] dt-bindings: net: maxlinear: add MxL86211C MDIO I/O regulator netdev-bot+sashiko
4 siblings, 2 replies; 16+ messages in thread
From: Robert Marko @ 2026-09-30 12:22 UTC (permalink / raw)
To: andrew+netdev, davem, edumazet, kuba, pabeni, robh, krzk+dt,
conor+dt, hkallweit1, lxu, michael, netdev, devicetree,
linux-kernel
Cc: luka.perkov, Robert Marko
Configure the MxL86211C physical TPI lane to ASP mapping when it is
described in DTS.
MxL86211C supports per lane configuration.
Leave the hardware reset configuration untouched when the optional
property is absent.
Signed-off-by: Robert Marko <robert.marko@sartura.hr>
---
drivers/net/phy/mxl-gpy.c | 53 ++++++++++++++++++++++++++++++++++++++-
1 file changed, 52 insertions(+), 1 deletion(-)
diff --git a/drivers/net/phy/mxl-gpy.c b/drivers/net/phy/mxl-gpy.c
index 790075632fc3f..f716da0ffe890 100644
--- a/drivers/net/phy/mxl-gpy.c
+++ b/drivers/net/phy/mxl-gpy.c
@@ -117,6 +117,13 @@
#define VSPEC1_PM_CTRL_MDIO_VOL BIT(14)
#define VSPEC1_PM_CTRL_SI BIT(12) /* Super Isolate */
+/* TPI lane to ASP map */
+#define VSPEC1_LANE_ASP_MAP 0x14
+#define VSPEC1_LANE_ASP_MAP_A GENMASK(1, 0)
+#define VSPEC1_LANE_ASP_MAP_B GENMASK(3, 2)
+#define VSPEC1_LANE_ASP_MAP_C GENMASK(5, 4)
+#define VSPEC1_LANE_ASP_MAP_D GENMASK(7, 6)
+
/* Temperature sensor */
#define VSPEC1_TEMP_STA 0x0E
#define VSPEC1_TEMP_STA_DATA GENMASK(9, 0)
@@ -440,6 +447,50 @@ static const struct regulator_desc mxl86211c_mdio_desc = {
.owner = THIS_MODULE,
};
+static int mxl86211c_configure_lane_asp_map(struct phy_device *phydev)
+{
+ struct device *dev = &phydev->mdio.dev;
+ u32 lane_asp_map[4];
+ u16 val;
+ unsigned int seen = 0;
+ int i, ret;
+
+ if (!device_property_present(dev, "maxlinear,lane-asp-map"))
+ return 0;
+
+ ret = device_property_read_u32_array(dev, "maxlinear,lane-asp-map",
+ lane_asp_map,
+ ARRAY_SIZE(lane_asp_map));
+ if (ret)
+ return ret;
+
+ for (i = 0; i < ARRAY_SIZE(lane_asp_map); i++) {
+ if (lane_asp_map[i] > 3 || seen & BIT(lane_asp_map[i]))
+ return -EINVAL;
+
+ seen |= BIT(lane_asp_map[i]);
+ }
+
+ val = FIELD_PREP(VSPEC1_LANE_ASP_MAP_A, lane_asp_map[0]) |
+ FIELD_PREP(VSPEC1_LANE_ASP_MAP_B, lane_asp_map[1]) |
+ FIELD_PREP(VSPEC1_LANE_ASP_MAP_C, lane_asp_map[2]) |
+ FIELD_PREP(VSPEC1_LANE_ASP_MAP_D, lane_asp_map[3]);
+
+ return phy_write_mmd(phydev, MDIO_MMD_VEND1, VSPEC1_LANE_ASP_MAP,
+ val);
+}
+
+static int mxl86211c_config_init(struct phy_device *phydev)
+{
+ int ret;
+
+ ret = mxl86211c_configure_lane_asp_map(phydev);
+ if (ret)
+ return ret;
+
+ return gpy21x_config_init(phydev);
+}
+
static int mxl86211c_probe(struct phy_device *phydev)
{
struct device *dev = &phydev->mdio.dev;
@@ -1482,7 +1533,7 @@ static struct phy_driver gpy_drivers[] = {
PHY_ID_MATCH_MODEL(PHY_ID_MXL86211C),
.name = "Maxlinear Ethernet MxL86211C",
.get_features = genphy_c45_pma_read_abilities,
- .config_init = gpy21x_config_init,
+ .config_init = mxl86211c_config_init,
.probe = mxl86211c_probe,
.inband_caps = gpy_inband_caps,
.config_inband = gpy_config_inband,
--
2.55.0
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 5/5] net: phy: mxl-gpy: support MxL86211C lane mapping
2026-09-30 12:22 ` [PATCH 5/5] net: phy: mxl-gpy: support MxL86211C lane mapping Robert Marko
@ 2026-09-30 15:35 ` Andrew Lunn
2026-09-30 16:58 ` Robert Marko
2026-10-04 13:17 ` netdev-bot+sashiko
1 sibling, 1 reply; 16+ messages in thread
From: Andrew Lunn @ 2026-09-30 15:35 UTC (permalink / raw)
To: Robert Marko
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, robh, krzk+dt,
conor+dt, hkallweit1, lxu, michael, netdev, devicetree,
linux-kernel, luka.perkov
On Wed, Sep 30, 2026 at 02:22:17PM +0200, Robert Marko wrote:
> Configure the MxL86211C physical TPI lane to ASP mapping when it is
> described in DTS.
>
> MxL86211C supports per lane configuration.
I assume you have a board which needs these features? Is the .dts file
ready for posting?
Andrew
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 5/5] net: phy: mxl-gpy: support MxL86211C lane mapping
2026-09-30 15:35 ` Andrew Lunn
@ 2026-09-30 16:58 ` Robert Marko
0 siblings, 0 replies; 16+ messages in thread
From: Robert Marko @ 2026-09-30 16:58 UTC (permalink / raw)
To: Andrew Lunn
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, robh, krzk+dt,
conor+dt, hkallweit1, lxu, michael, netdev, devicetree,
linux-kernel, luka.perkov
On Wed, Sep 30, 2026 at 5:35 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Wed, Sep 30, 2026 at 02:22:17PM +0200, Robert Marko wrote:
> > Configure the MxL86211C physical TPI lane to ASP mapping when it is
> > described in DTS.
> >
> > MxL86211C supports per lane configuration.
>
> I assume you have a board which needs these features? Is the .dts file
> ready for posting?
Hi Andrew,
Yes, I have a board that requires it, as pairs are reversed intentionally.
DTS is not yet ready, as there will be another revision of the board
before it is commercially available.
But I wanted to get the ball rolling on the missing stuff in the MXL PHY driver.
Regards,
Robert
>
> Andrew
--
Robert Marko
Staff Embedded Linux Engineer
Sartura d.d.
Lendavska ulica 16a
10000 Zagreb, Croatia
Email: robert.marko@sartura.hr
Web: www.sartura.hr
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 5/5] net: phy: mxl-gpy: support MxL86211C lane mapping
2026-09-30 12:22 ` [PATCH 5/5] net: phy: mxl-gpy: support MxL86211C lane mapping Robert Marko
2026-09-30 15:35 ` Andrew Lunn
@ 2026-10-04 13:17 ` netdev-bot+sashiko
1 sibling, 0 replies; 16+ messages in thread
From: netdev-bot+sashiko @ 2026-10-04 13:17 UTC (permalink / raw)
To: robert.marko
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, robh, krzk+dt,
conor+dt, hkallweit1, lxu, michael, netdev, devicetree,
linux-kernel, luka.perkov
Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.
Critical: 0 · High: 0 · Medium: 1 · Low: 0
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [Medium] net: phy: mxl-gpy: potential overwrite of upper bits in
VSPEC1_LANE_ASP_MAP
--
> diff --git a/drivers/net/phy/mxl-gpy.c b/drivers/net/phy/mxl-gpy.c
> --- a/drivers/net/phy/mxl-gpy.c
> +++ b/drivers/net/phy/mxl-gpy.c
> @@ -440,6 +447,50 @@ static const struct regulator_desc mxl86211c_mdio_desc = {
> .owner = THIS_MODULE,
> };
>
> +static int mxl86211c_configure_lane_asp_map(struct phy_device *phydev)
> +{
> + struct device *dev = &phydev->mdio.dev;
> + u32 lane_asp_map[4];
> + u16 val;
> + unsigned int seen = 0;
> + int i, ret;
> +
> + if (!device_property_present(dev, "maxlinear,lane-asp-map"))
> + return 0;
> +
> + ret = device_property_read_u32_array(dev, "maxlinear,lane-asp-map",
> + lane_asp_map,
> + ARRAY_SIZE(lane_asp_map));
> + if (ret)
> + return ret;
> +
> + for (i = 0; i < ARRAY_SIZE(lane_asp_map); i++) {
> + if (lane_asp_map[i] > 3 || seen & BIT(lane_asp_map[i]))
> + return -EINVAL;
> +
> + seen |= BIT(lane_asp_map[i]);
> + }
> +
> + val = FIELD_PREP(VSPEC1_LANE_ASP_MAP_A, lane_asp_map[0]) |
> + FIELD_PREP(VSPEC1_LANE_ASP_MAP_B, lane_asp_map[1]) |
> + FIELD_PREP(VSPEC1_LANE_ASP_MAP_C, lane_asp_map[2]) |
> + FIELD_PREP(VSPEC1_LANE_ASP_MAP_D, lane_asp_map[3]);
> +
> + return phy_write_mmd(phydev, MDIO_MMD_VEND1, VSPEC1_LANE_ASP_MAP,
> + val);
> +}
> +
> +static int mxl86211c_config_init(struct phy_device *phydev)
> +{
> + int ret;
> +
> + ret = mxl86211c_configure_lane_asp_map(phydev);
> + if (ret)
> + return ret;
> +
> + return gpy21x_config_init(phydev);
> +}
> +
> static int mxl86211c_probe(struct phy_device *phydev)
> {
> struct device *dev = &phydev->mdio.dev;
[Severity: Medium]
mxl86211c_configure_lane_asp_map() writes VSPEC1_LANE_ASP_MAP with
a plain phy_write_mmd(). That means bits 15:8 of the register are
always written as zero, even though the patch only defines the four
2-bit lane fields in bits 7:0.
Those upper bits might hold something: reserved bits with a non-zero
reset value, or an unrelated control field. If so, they get cleared
whenever maxlinear,lane-asp-map is present in DT. The commit message
says the reset configuration is left alone when the property is
absent. When the property is present, this write also changes
whatever else lives in that register, not just the lane mapping.
Does the datasheet say bits 15:8 of VSPEC1 0x14 are unused and safe
to write as zero? If not, would phy_modify_mmd() with a mask built
from VSPEC1_LANE_ASP_MAP_A..D be safer? mxl86211c_probe() already
uses phy_modify_mmd() for VSPEC1_PM_CTRL, so that would also match
the rest of the driver.
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260930122305.293686-1-robert.marko%40sartura.hr
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/5] dt-bindings: net: maxlinear: add MxL86211C MDIO I/O regulator
2026-09-30 12:22 [PATCH 1/5] dt-bindings: net: maxlinear: add MxL86211C MDIO I/O regulator Robert Marko
` (3 preceding siblings ...)
2026-09-30 12:22 ` [PATCH 5/5] net: phy: mxl-gpy: support MxL86211C lane mapping Robert Marko
@ 2026-10-04 13:17 ` netdev-bot+sashiko
4 siblings, 0 replies; 16+ messages in thread
From: netdev-bot+sashiko @ 2026-10-04 13:17 UTC (permalink / raw)
To: robert.marko
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, robh, krzk+dt,
conor+dt, hkallweit1, lxu, michael, netdev, devicetree,
linux-kernel, luka.perkov
Thank you for your contribution! Sashiko AI review found 4 potential
issue(s) to consider.
Critical: 0 · High: 0 · Medium: 2 · Low: 2
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [Medium] The commit message says the patch will "Restrict the regulator
subnode to the MxL86211C PHY ID".
- [Medium] The binding models the MxL86211C MDIO/MDC/MDINT pad I/O-level
selection (the VSPEC1_PM_CTRL.MDIO_VOL bit) as a regulator provider…
- [Low] In Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml,
the `mdio-regulator` subnode is defined only as `type: object`, `$ref:…
- [Low] dt-bindings: net: maxlinear: schema property 'mdio-regulator'
lacks top-level definition
--
dt-bindings: net: maxlinear: add MxL86211C MDIO I/O regulator
This commit adds an mdio-regulator child node for the MxL86211C PHY
(ethernet-phy-idc133.5400) to the gpy2xx binding. The node references
regulator.yaml and selects a 1.8 V or 3.3 V level for the MDIO, MDC and
MDINT pads. An allOf if/then/else is meant to forbid the node on other PHY
IDs, and a new example is added.
> diff --git a/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml b/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml
> index 8a3713abd1ca9..0645e885f1747 100644
> --- a/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml
> +++ b/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml
> @@ -12,6 +12,29 @@ maintainers:
>
> allOf:
> - $ref: ethernet-phy.yaml#
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: ethernet-phy-idc133.5400
> + required:
> + - compatible
> +
> + then:
> + properties:
> + mdio-regulator:
> + type: object
> + description: |
> + Initial data for the MxL86211C MDIO I/O voltage regulator. Set
> + this to 1.8 V or 3.3 V to configure the MDIO, MDC, and MDINT
> + pad voltage. The board must supply the VDDP pin at the selected
> + voltage.
> + $ref: /schemas/regulator/regulator.yaml#
> + unevaluatedProperties: false
[Severity: Low]
The mdio-regulator node is defined only inside the then: branch of
this conditional.
The usual dt-schema convention, as shown in example-schema.yaml, is to
define every property once in the top-level properties: block. The
if/then/else blocks are then used only to narrow a property, or to set
it to false for the compatibles that do not have it.
As written, the properties: section of maxlinear,gpy2xx.yaml does not
mention mdio-regulator at all. Its type, its description and the $ref
to regulator.yaml sit inside a conditional. Readers and documentation
tooling looking at the top-level properties will not find it.
Could the full definition (type: object, the description,
$ref: /schemas/regulator/regulator.yaml# and
unevaluatedProperties: false) move under the top-level properties:,
next to maxlinear,use-broken-interrupts?
The allOf entry would then only have to disallow the node for other
PHY IDs. That could be done by keeping the current else: and dropping
the then:. Another way is an if: that negates the
ethernet-phy-idc133.5400 match, with then: mdio-regulator: false.
If maxlinear,lane-asp-map in the later lane mapping patch is defined
the same way, it would need the same change.
[Severity: Low]
The description says the pads can only be set to 1.8 V or 3.3 V. Should
the schema enforce that?
regulator.yaml puts no bounds on regulator-min-microvolt or
regulator-max-microvolt. So this node accepts values like 2500000. It also
accepts a range such as min = 1800000 / max = 3300000, which does not fix
the pad level.
The driver added later in the series supports only two levels:
drivers/net/phy/mxl-gpy.c:
static const unsigned int mxl86211c_mdio_voltage_table[] = {
1800000,
3300000,
};
With an unsupported fixed value, mxl86211c_probe() would presumably fail
in devm_regulator_register(), even though the DT passes the schema.
With a 1.8-3.3 V range, no voltage would be applied at registration. The
pads would stay at the 3.3 V reset default, even on a board where VDDP is
1.8 V.
Could regulator-min-microvolt and regulator-max-microvolt be limited to
enum: [1800000, 3300000], and possibly required to be equal?
The commit message also says:
The regulator configures the MDIO, MDC, and MDINT pad voltage
between 1.8 V and 3.3 V.
That reads like a continuous range, but the binding text says "1.8 V or
3.3 V". Should the commit message use the same wording as the binding?
> +
> + else:
> + properties:
> + mdio-regulator: false
>
> properties:
> maxlinear,use-broken-interrupts:
[Severity: Medium]
The commit message says this will "Restrict the regulator subnode to the
MxL86211C PHY ID". Is this conditional ever applied to a real device tree
during make dtbs_check?
This schema has no select: block. Its top-level properties contain only
maxlinear,use-broken-interrupts, with no compatible or $nodename.
dt-schema builds a default select only from compatible or $nodename, so
maxlinear,gpy2xx.yaml ends up with select: false. The $ref to
ethernet-phy.yaml does not carry over that schema's select.
That leaves real PHY nodes matched only by ethernet-phy.yaml:
Documentation/devicetree/bindings/net/ethernet-phy.yaml:
select:
properties:
$nodename:
pattern: "^ethernet-phy(@[a-f0-9]+)?$"
...
additionalProperties: true
So an mdio-regulator child under any PHY node (a GPY211, a Marvell part,
and so on) would pass. The node's contents would also never be checked
against regulator.yaml. The if/then/else seems to run only against the
examples in this file.
Other PHY bindings with per-PHY-ID constraints, such as
marvell,aquantia.yaml, add an explicit select: on the
ethernet-phy-idXXXX.XXXX compatibles. Is something similar needed here?
The maxlinear,lane-asp-map restriction added later in the series, by
"dt-bindings: net: maxlinear: add MxL86211C lane mapping support", looks
like it has the same gap.
[ ... ]
> @@ -45,4 +68,21 @@ examples:
> };
> };
>
> + - |
> + ethernet {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + ethernet-phy@0 {
> + compatible = "ethernet-phy-idc133.5400",
> + "ethernet-phy-ieee802.3-c45";
> + reg = <0>;
> +
> + mdio: mdio-regulator {
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + };
[Severity: Medium]
Is a regulator provider the right model for this hardware?
The binding description says "The board must supply the VDDP pin at the
selected voltage". The driver commit message says the PHY powers the MDIO
pins from the external VDDP, and the MDIO_VOL bit in VSPEC1_PM_CTRL only
has to match it. So the PHY does not supply any power itself.
The example gives the node a phandle label:
mdio: mdio-regulator {
That suggests other nodes could use it as a *-supply. Any such consumer
would get a regulator that supplies nothing.
The real dependency, the board's VDDP rail, is described only in prose.
Without something like a vddp-supply consumer property, neither dt-schema
nor the driver can check that the programmed pad level matches the actual
VDDP voltage. A DT that sets 1.8 V while VDDP is 3.3 V, or the reverse,
would not be flagged.
Would a vddp-supply property be a better fit, with the driver reading its
voltage to program MDIO_VOL? A plain vendor property is another option.
Once DTs ship with the mdio-regulator node, it becomes DT ABI.
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260930122305.293686-1-robert.marko%40sartura.hr
^ permalink raw reply [flat|nested] 16+ messages in thread