[PATCH net-next v2 1/5] dt-bindings: net: maxlinear: add MxL86211C VDDP supply

[PATCH net-next v2 1/5] dt-bindings: net: maxlinear: add MxL86211C VDDP supply

[Robert Marko]

Describe the VDDP supply used by the MxL86211C PHY. Its voltage
determines whether the MDIO, MDC, and MDINT pads use 1.8 V or 3.3 V.

Select this binding for the MxL86211C PHY ID so the supply requirement
is validated.

Signed-off-by: Robert Marko <robert.marko@sartura.hr>
---
Changes in v2:
- Model VDDP as a supply instead of a regulator provider.
- Select the schema for the MxL86211C PHY ID.

 .../bindings/net/maxlinear,gpy2xx.yaml        | 49 +++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml b/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml
index 8a3713abd1ca9..a0dfb6192d81b 100644
--- a/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml
+++ b/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml
@@ -10,10 +10,38 @@ maintainers:
   - Andrew Lunn <andrew@lunn.ch>
   - Michael Walle <michael@walle.cc>
 
+select:
+  properties:
+    compatible:
+      contains:
+        const: ethernet-phy-idc133.5400
+  required:
+    - compatible
+
 allOf:
   - $ref: ethernet-phy.yaml#
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: ethernet-phy-idc133.5400
+      required:
+        - compatible
+
+    then:
+      required:
+        - vddp-supply
+
+    else:
+      properties:
+        vddp-supply: false
 
 properties:
+  vddp-supply:
+    description: |
+      Regulator supplying the VDDP pin. Its voltage configures the MDIO,
+      MDC, and MDINT pad voltage and must be either 1.8 V or 3.3 V.
+
   maxlinear,use-broken-interrupts:
     description: |
       Interrupts are broken on some GPY2xx PHYs in that they keep the
@@ -45,4 +73,25 @@ examples:
         };
     };
 
+  - |
+    vddp: regulator-vddp {
+        compatible = "regulator-fixed";
+        regulator-name = "vddp";
+        regulator-min-microvolt = <1800000>;
+        regulator-max-microvolt = <1800000>;
+    };
+
+    ethernet {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        ethernet-phy@0 {
+            compatible = "ethernet-phy-idc133.5400",
+                         "ethernet-phy-ieee802.3-c45";
+            reg = <0>;
+            vddp-supply = <&vddp>;
+
+        };
+    };
+
 ...
-- 
2.56.0

[PATCH net-next v2 2/5] net: phy: mxl-gpy: add MxL86211C VDDP supply

[Robert Marko]

Read the VDDP supply voltage and configure the MxL86211C MDIO, MDC,
and MDINT pads for the matching 1.8 V or 3.3 V level.

Keep the shared GPY driver buildable without regulator support; only
the MxL86211C probe requires the supply.

Signed-off-by: Robert Marko <robert.marko@sartura.hr>
---
Changes in v2:
- Replace the MDIO regulator provider with a VDDP supply consumer.
- Set the pad voltage from the supply's reported voltage.
- Keep the shared driver buildable with CONFIG_REGULATOR=n.

 drivers/net/phy/mxl-gpy.c | 63 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 62 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/mxl-gpy.c b/drivers/net/phy/mxl-gpy.c
index 089a1a9e5bc61..fbac7c49b4759 100644
--- a/drivers/net/phy/mxl-gpy.c
+++ b/drivers/net/phy/mxl-gpy.c
@@ -14,6 +14,7 @@
 #include <linux/polynomial.h>
 #include <linux/property.h>
 #include <linux/netdevice.h>
+#include <linux/regulator/consumer.h>
 
 /* PHY ID */
 #define PHY_ID_GPYx15B_MASK	0xFFFFFFFC
@@ -110,6 +111,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)
@@ -140,6 +145,7 @@ struct gpy_priv {
 	u8 fw_minor;
 	u32 wolopts;
 	u64 rx_errors;
+	struct regulator *vddp;
 
 	/* It takes 3 seconds to fully switch out of loopback mode before
 	 * it can safely re-enter loopback mode. Record the time when
@@ -390,6 +396,61 @@ static int gpy_probe(struct phy_device *phydev)
 	return 0;
 }
 
+static int mxl86211c_configure_mdio_voltage(struct phy_device *phydev,
+					    struct regulator *vddp)
+{
+	int voltage;
+	int val;
+
+	voltage = regulator_get_voltage(vddp);
+	if (voltage < 0)
+		return voltage;
+
+	switch (voltage) {
+	case 1800000:
+		val = VSPEC1_PM_CTRL_MDIO_VOL;
+		break;
+	case 3300000:
+		val = 0;
+		break;
+	default:
+		phydev_err(phydev, "unsupported VDDP voltage: %d\n", voltage);
+		return -EINVAL;
+	}
+
+	return phy_modify_mmd(phydev, MDIO_MMD_VEND1, VSPEC1_PM_CTRL,
+			      VSPEC1_PM_CTRL_MDIO_VOL, val);
+}
+
+static int mxl86211c_probe(struct phy_device *phydev)
+{
+	struct device *dev = &phydev->mdio.dev;
+	struct gpy_priv *priv;
+	struct regulator *vddp;
+	int ret;
+
+	vddp = devm_regulator_get(dev, "vddp");
+	if (IS_ERR(vddp))
+		return dev_err_probe(dev, PTR_ERR(vddp),
+				     "failed to get VDDP supply\n");
+	if (!vddp)
+		return dev_err_probe(dev, -EOPNOTSUPP,
+				     "VDDP supply requires regulator support\n");
+
+	ret = mxl86211c_configure_mdio_voltage(phydev, vddp);
+	if (ret)
+		return ret;
+
+	ret = gpy_probe(phydev);
+	if (ret)
+		return ret;
+
+	priv = phydev->priv;
+	priv->vddp = vddp;
+
+	return 0;
+}
+
 static bool gpy_sgmii_need_reaneg(struct phy_device *phydev)
 {
 	struct gpy_priv *priv = phydev->priv;
@@ -1406,7 +1467,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.56.0

[PATCH net-next v2 3/5] net: phy: mxl-gpy: release MxL86211C Super Isolate

[Robert Marko]

The MxL86211C can be strapped into Super Isolate mode, leaving the PHY
halted after reset until software releases it.

Reapply the MDIO pad voltage and release Super Isolate in config_init,
so both settings are restored after each hardware reset.

Signed-off-by: Robert Marko <robert.marko@sartura.hr>
---
Changes in v2:
- Move Super Isolate release from probe to config_init for resets.
- Reapply the MDIO pad voltage before releasing Super Isolate.

 drivers/net/phy/mxl-gpy.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/mxl-gpy.c b/drivers/net/phy/mxl-gpy.c
index fbac7c49b4759..8bc119655f104 100644
--- a/drivers/net/phy/mxl-gpy.c
+++ b/drivers/net/phy/mxl-gpy.c
@@ -114,6 +114,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
@@ -422,6 +423,23 @@ static int mxl86211c_configure_mdio_voltage(struct phy_device *phydev,
 			      VSPEC1_PM_CTRL_MDIO_VOL, val);
 }
 
+static int mxl86211c_config_init(struct phy_device *phydev)
+{
+	struct gpy_priv *priv = phydev->priv;
+	int ret;
+
+	ret = mxl86211c_configure_mdio_voltage(phydev, priv->vddp);
+	if (ret)
+		return ret;
+
+	ret = phy_modify_mmd(phydev, MDIO_MMD_VEND1, VSPEC1_PM_CTRL,
+			     VSPEC1_PM_CTRL_SI, 0);
+	if (ret)
+		return ret;
+
+	return gpy21x_config_init(phydev);
+}
+
 static int mxl86211c_probe(struct phy_device *phydev)
 {
 	struct device *dev = &phydev->mdio.dev;
@@ -1466,7 +1484,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.56.0

[PATCH net-next v2 4/5] dt-bindings: net: maxlinear: add MxL86211C lane mapping support

[Robert Marko]

Describe the optional board-level mapping from physical TPI lanes to the
MxL86211C analog front ends (ASPs). This allows DTS to describe wiring
that differs from the hardware reset mapping.

Signed-off-by: Robert Marko <robert.marko@sartura.hr>
---
Changes in v2:
- Define the lane-map property at the top level for type checking.
- Describe ASPs as analog front ends.

 .../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 a0dfb6192d81b..4323be300c721 100644
--- a/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml
+++ b/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml
@@ -35,6 +35,7 @@ allOf:
     else:
       properties:
         vddp-supply: false
+        maxlinear,lane-asp-map: false
 
 properties:
   vddp-supply:
@@ -42,6 +43,21 @@ properties:
       Regulator supplying the VDDP pin. Its voltage configures the MDIO,
       MDC, and MDINT pad voltage and must be either 1.8 V or 3.3 V.
 
+  maxlinear,lane-asp-map:
+    description: |
+      Mapping of physical TPI lanes A through D to analog front ends (ASPs)
+      A through D. The array index identifies the physical TPI lane, while
+      its value identifies the analog front end, encoded as 0 through 3.
+      Each analog front end 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
+
   maxlinear,use-broken-interrupts:
     description: |
       Interrupts are broken on some GPY2xx PHYs in that they keep the
@@ -91,6 +107,9 @@ examples:
             reg = <0>;
             vddp-supply = <&vddp>;
 
+            /* Swap physical TPI lanes C and D. */
+            maxlinear,lane-asp-map = <0 1 3 2>;
+
         };
     };
 
-- 
2.56.0

[PATCH net-next v2 5/5] net: phy: mxl-gpy: support MxL86211C lane mapping

[Robert Marko]

Configure the MxL86211C mapping from physical TPI lanes to analog
front ends (ASPs) when specified in DTS.

Leave the hardware reset mapping untouched when the optional property
is absent.

Signed-off-by: Robert Marko <robert.marko@sartura.hr>
---
Changes in v2:
- Preserve unrelated register bits when updating the lane map.

 drivers/net/phy/mxl-gpy.c | 47 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/drivers/net/phy/mxl-gpy.c b/drivers/net/phy/mxl-gpy.c
index 8bc119655f104..acd5ad01a285d 100644
--- a/drivers/net/phy/mxl-gpy.c
+++ b/drivers/net/phy/mxl-gpy.c
@@ -116,6 +116,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)
@@ -423,6 +430,42 @@ static int mxl86211c_configure_mdio_voltage(struct phy_device *phydev,
 			      VSPEC1_PM_CTRL_MDIO_VOL, val);
 }
 
+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_modify_mmd(phydev, MDIO_MMD_VEND1, VSPEC1_LANE_ASP_MAP,
+			      VSPEC1_LANE_ASP_MAP_A |
+			      VSPEC1_LANE_ASP_MAP_B |
+			      VSPEC1_LANE_ASP_MAP_C |
+			      VSPEC1_LANE_ASP_MAP_D, val);
+}
+
 static int mxl86211c_config_init(struct phy_device *phydev)
 {
 	struct gpy_priv *priv = phydev->priv;
@@ -432,6 +475,10 @@ static int mxl86211c_config_init(struct phy_device *phydev)
 	if (ret)
 		return ret;
 
+	ret = mxl86211c_configure_lane_asp_map(phydev);
+	if (ret)
+		return ret;
+
 	ret = phy_modify_mmd(phydev, MDIO_MMD_VEND1, VSPEC1_PM_CTRL,
 			     VSPEC1_PM_CTRL_SI, 0);
 	if (ret)
-- 
2.56.0

Re: [PATCH net-next v2 2/5] net: phy: mxl-gpy: add MxL86211C VDDP supply

[Andrew Lunn]

> +static int mxl86211c_probe(struct phy_device *phydev)
> +{
> +	struct device *dev = &phydev->mdio.dev;
> +	struct gpy_priv *priv;
> +	struct regulator *vddp;
> +	int ret;
> +
> +	vddp = devm_regulator_get(dev, "vddp");
> +	if (IS_ERR(vddp))
> +		return dev_err_probe(dev, PTR_ERR(vddp),
> +				     "failed to get VDDP supply\n");
> +	if (!vddp)
> +		return dev_err_probe(dev, -EOPNOTSUPP,
> +				     "VDDP supply requires regulator support\n");
> +

>  static bool gpy_sgmii_need_reaneg(struct phy_device *phydev)
>  {
>  	struct gpy_priv *priv = phydev->priv;
> @@ -1406,7 +1467,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,

This is an existing device. You cannot add required new DT parameters,
that would break backwards compatibility. Anything you add news to be
optional.

    Andrew

---
pw-bot: cr

Re: [PATCH net-next v2 5/5] net: phy: mxl-gpy: support MxL86211C lane mapping

[Andrew Lunn]

On Fri, Oct 16, 2026 at 01:19:36PM +0200, Robert Marko wrote:
> Configure the MxL86211C mapping from physical TPI lanes to analog
> front ends (ASPs) when specified in DTS.
> 
> Leave the hardware reset mapping untouched when the optional property
> is absent.
> 
> Signed-off-by: Robert Marko <robert.marko@sartura.hr>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net-next v2 1/5] dt-bindings: net: maxlinear: add MxL86211C VDDP supply

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] dt-bindings: overly restrictive select block breaks validation for other PHYs
--

commit 695546f38e461889fde9ccbc3738543cebc3766c
Author: Robert Marko <robert.marko@sartura.hr>

dt-bindings: net: maxlinear: add MxL86211C VDDP supply

This commit describes the VDDP supply used by the MxL86211C PHY.
It adds a new schema section to validate that the supply requirement is met
when the MxL86211C PHY ID is present.

> diff --git a/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml b/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml
> index 8a3713abd1ca9..a0dfb6192d81b 100644
> --- a/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml
> +++ b/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml
> @@ -10,10 +10,38 @@ maintainers:
>    - Andrew Lunn <andrew@lunn.ch>
>    - Michael Walle <michael@walle.cc>
>  
> +select:
> +  properties:
> +    compatible:
> +      contains:
> +        const: ethernet-phy-idc133.5400
> +  required:
> +    - compatible

[Severity: Medium]
Does this top-level select block restrict the entire schema to only match this
single PHY ID? By acting as a gatekeeper for the whole schema, it appears this
breaks validation for all other supported GPY2xx PHYs.

For example, the maxlinear,use-broken-interrupts property intended for
GPY215B/C might no longer be validated, and even pre-existing examples in the
schema might fail to match.

> +
>  allOf:
>    - $ref: ethernet-phy.yaml#
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: ethernet-phy-idc133.5400
> +      required:
> +        - compatible
> +
> +    then:
> +      required:
> +        - vddp-supply
> +
> +    else:
> +      properties:
> +        vddp-supply: false

[Severity: Medium]
Since the top-level select block already guarantees that the compatible string
contains ethernet-phy-idc133.5400, does this else branch become dead code? 

The if condition will always evaluate to true when this file is applied, so
this branch won't be able to enforce the vddp-supply restriction on other
PHYs.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016112033.949026-1-robert.marko@sartura.hr?part=1

Re: [PATCH net-next v2 2/5] net: phy: mxl-gpy: add MxL86211C VDDP supply

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] mxl-gpy: missing regulator_enable() for VDDP supply
- [High] phy: mxl-gpy: probe regression on missing vddp-supply
--

commit a90a687bf14a0ab1f83d612c5413b01b74244ef5
Author: Robert Marko <robert.marko@sartura.hr>

net: phy: mxl-gpy: add MxL86211C VDDP supply

This commit introduces VDDP supply handling for the MxL86211C PHY. It
fetches the supply voltage to properly configure the MDIO, MDC, and MDINT
pads for 1.8V or 3.3V operation.

> diff --git a/drivers/net/phy/mxl-gpy.c b/drivers/net/phy/mxl-gpy.c
> index 089a1a9e5bc61..fbac7c49b4759 100644
> --- a/drivers/net/phy/mxl-gpy.c
> +++ b/drivers/net/phy/mxl-gpy.c
[ ... ]
> @@ -390,6 +396,61 @@ static int gpy_probe(struct phy_device *phydev)
>  	return 0;
>  }
>  
> +static int mxl86211c_configure_mdio_voltage(struct phy_device *phydev,
> +					    struct regulator *vddp)
> +{
> +	int voltage;
> +	int val;
> +
> +	voltage = regulator_get_voltage(vddp);
> +	if (voltage < 0)
> +		return voltage;

[Severity: High]
If the device tree does not provide a vddp-supply, devm_regulator_get()
will return a dummy regulator. Calling regulator_get_voltage() on this dummy
regulator returns -EINVAL, which propagates up and aborts the probe. Will
this break backward compatibility on existing boards that do not define a
vddp-supply?

> +
> +	switch (voltage) {
> +	case 1800000:
> +		val = VSPEC1_PM_CTRL_MDIO_VOL;
> +		break;
> +	case 3300000:
> +		val = 0;
> +		break;
> +	default:
> +		phydev_err(phydev, "unsupported VDDP voltage: %d\n", voltage);
> +		return -EINVAL;
> +	}
> +
> +	return phy_modify_mmd(phydev, MDIO_MMD_VEND1, VSPEC1_PM_CTRL,
> +			      VSPEC1_PM_CTRL_MDIO_VOL, val);
> +}
> +
> +static int mxl86211c_probe(struct phy_device *phydev)
> +{
> +	struct device *dev = &phydev->mdio.dev;
> +	struct gpy_priv *priv;
> +	struct regulator *vddp;
> +	int ret;
> +
> +	vddp = devm_regulator_get(dev, "vddp");
> +	if (IS_ERR(vddp))
> +		return dev_err_probe(dev, PTR_ERR(vddp),
> +				     "failed to get VDDP supply\n");

[Severity: High]
The VDDP supply is acquired here, but it appears regulator_enable() is never
called. Because the regulator's use-count remains at zero, won't the
regulator core disable the VDDP supply during late init (via
regulator_disable_unused()) unless it is marked as regulator-always-on? 

Disabling this supply would cut power to the PHY's MDIO pads and render the
network interface unusable.

> +	if (!vddp)
> +		return dev_err_probe(dev, -EOPNOTSUPP,
> +				     "VDDP supply requires regulator support\n");
> +
> +	ret = mxl86211c_configure_mdio_voltage(phydev, vddp);
> +	if (ret)
> +		return ret;
> +
> +	ret = gpy_probe(phydev);
> +	if (ret)
> +		return ret;
> +
> +	priv = phydev->priv;
> +	priv->vddp = vddp;
> +
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016112033.949026-1-robert.marko@sartura.hr?part=2

Re: [PATCH net-next v2 3/5] net: phy: mxl-gpy: release MxL86211C Super Isolate

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] net: phy: mxl-gpy: firmware version read while PHY is halted
--

commit 2628bcaa4964186af3313096275fe989a084332d
Author: Robert Marko <robert.marko@sartura.hr>

net: phy: mxl-gpy: release MxL86211C Super Isolate

This commit modifies the driver to release the MxL86211C Super Isolate
mode in config_init rather than probe. This ensures both MDIO pad voltage
and Super Isolate settings are restored after each hardware reset.

> diff --git a/drivers/net/phy/mxl-gpy.c b/drivers/net/phy/mxl-gpy.c
> index fbac7c49b4759..8bc119655f104 100644
> --- a/drivers/net/phy/mxl-gpy.c
> +++ b/drivers/net/phy/mxl-gpy.c
[ ... ]
> @@ -422,6 +423,23 @@ static int mxl86211c_configure_mdio_voltage(struct phy_device *phydev,
>  			      VSPEC1_PM_CTRL_MDIO_VOL, val);
>  }
>  
> +static int mxl86211c_config_init(struct phy_device *phydev)
> +{
> +	struct gpy_priv *priv = phydev->priv;
> +	int ret;
> +
> +	ret = mxl86211c_configure_mdio_voltage(phydev, priv->vddp);
> +	if (ret)
> +		return ret;
> +
> +	ret = phy_modify_mmd(phydev, MDIO_MMD_VEND1, VSPEC1_PM_CTRL,
> +			     VSPEC1_PM_CTRL_SI, 0);

[Severity: Medium]
Will this delay in releasing Super Isolate cause gpy_probe() to read an
incorrect firmware version?

If the MxL86211C PHY is strapped into Super Isolate mode, it remains halted
after reset. During device registration, mxl86211c_probe() executes and
invokes gpy_probe() which attempts to read the firmware version:

drivers/net/phy/mxl-gpy.c:gpy_probe() {
    ...
    fw_version = phy_read(phydev, PHY_FWV);
    if (fw_version < 0)
        return fw_version;
    priv->fw_major = FIELD_GET(PHY_FWV_MAJOR_MASK, fw_version);
    priv->fw_minor = FIELD_GET(PHY_FWV_MINOR_MASK, fw_version);
    ...
}

Because the release of the Super Isolate bit (VSPEC1_PM_CTRL_SI) is
deferred here to mxl86211c_config_init(), the PHY microcontroller is
still halted during the probe path.

Could this cause the driver to cache an incorrect firmware version
(likely 0.0), potentially breaking firmware-specific logic like the
workarounds in gpy_sgmii_need_reaneg() and gpy115_loopback()?

> +	if (ret)
> +		return ret;
> +
> +	return gpy21x_config_init(phydev);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016112033.949026-1-robert.marko@sartura.hr?part=3

Re: [PATCH net-next v2 4/5] dt-bindings: net: maxlinear: add MxL86211C lane mapping support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] dt-bindings: net: maxlinear: dead code in else block due to strict select
--

commit 414c3b989bc8453c4971568d53b96638314826bb
Author: Robert Marko <robert.marko@sartura.hr>

dt-bindings: net: maxlinear: add MxL86211C lane mapping support

Describe the optional board-level mapping from physical TPI lanes to the
MxL86211C analog front ends (ASPs). This allows DTS to describe wiring
that differs from the hardware reset mapping.

> diff --git a/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml b/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml
> index a0dfb6192d81b..4323be300c721 100644
> --- a/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml
> +++ b/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml
> @@ -35,6 +35,7 @@ allOf:
>      else:
>        properties:
>          vddp-supply: false
> +        maxlinear,lane-asp-map: false

[Severity: Medium]
Will this else block ever be reached?

In Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml, the schema
contains a restricted select block:

select:
  properties:
    compatible:
      contains:
        const: ethernet-phy-idc133.5400

Since the allOf / if block checks for the exact same condition, it appears
guaranteed to always evaluate to true, as the schema only evaluates nodes
that already passed the select filter.

This seems to make the else block dead code, meaning maxlinear,lane-asp-map: false
is never actually enforced for non-idc133.5400 nodes.

Additionally, does this strict select block prevent the schema from
evaluating older PHYs like GPY215B/C? If the schema restricts evaluation
exclusively to ethernet-phy-idc133.5400, properties like
maxlinear,use-broken-interrupts might skip validation on older PHYs and be
flagged as undocumented during dtbs_check validations.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016112033.949026-1-robert.marko@sartura.hr?part=4

Re: [PATCH net-next v2 1/5] dt-bindings: net: maxlinear: add MxL86211C VDDP supply

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 1

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 binding defines `vddp-supply` as "Regulator supplying the
  VDDP pin", which powers the MDIO/MDC/MDINT pads, and makes it…
- [Low] In Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml,
  the `else: properties: vddp-supply: false` branch (lines 35-37) can…

--

dt-bindings: net: maxlinear: add MxL86211C VDDP supply

This commit adds a vddp-supply property to the MaxLinear GPY2xx binding.
The voltage of that supply selects 1.8 V or 3.3 V for the MDIO, MDC and
MDINT pads. A top-level select restricts the schema to the MxL86211C PHY
ID, and an if/then/else makes the supply required for that PHY.

> diff --git a/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml b/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml
> index 8a3713abd1ca9..a0dfb6192d81b 100644
> --- a/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml
> +++ b/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml
> @@ -10,10 +10,38 @@ maintainers:
>    - Andrew Lunn <andrew@lunn.ch>
>    - Michael Walle <michael@walle.cc>
>  
> +select:
> +  properties:
> +    compatible:
> +      contains:
> +        const: ethernet-phy-idc133.5400
> +  required:
> +    - compatible
> +
>  allOf:
>    - $ref: ethernet-phy.yaml#
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: ethernet-phy-idc133.5400
> +      required:
> +        - compatible
> +
> +    then:
> +      required:
> +        - vddp-supply
> +
> +    else:
> +      properties:
> +        vddp-supply: false

[Severity: Low]
Can this else branch ever be evaluated?

The new top-level select applies the schema only to nodes whose
compatible contains ethernet-phy-idc133.5400. The if above tests the
same predicate, so whenever the schema runs, only the then branch runs.

No other binding references this schema. Other GPY2xx nodes are not
selected at all, so "vddp-supply: false" is never enforced for them.

The later patch in the series, "dt-bindings: net: maxlinear: add
MxL86211C lane mapping support", adds "maxlinear,lane-asp-map: false" to
this same else branch. That constraint would not be enforced either.

>  
>  properties:
> +  vddp-supply:
> +    description: |
> +      Regulator supplying the VDDP pin. Its voltage configures the MDIO,
> +      MDC, and MDINT pad voltage and must be either 1.8 V or 3.3 V.
> +

[Severity: Medium]
This makes vddp-supply mandatory for the MxL86211C and describes it as
the supply for the MDIO, MDC and MDINT pads. Does anything in the series
enable this supply?

The later driver patch, "net: phy: mxl-gpy: add MxL86211C VDDP supply",
does only two things with the regulator: it gets it, and it reads its
voltage.

drivers/net/phy/mxl-gpy.c:mxl86211c_probe() {
    ...
	vddp = devm_regulator_get(dev, "vddp");
    ...
}

drivers/net/phy/mxl-gpy.c:mxl86211c_configure_mdio_voltage() {
    ...
	voltage = regulator_get_voltage(vddp);
    ...
}

Neither function calls regulator_enable() or devm_regulator_get_enable(),
so rdev->use_count stays 0 for this rail. Take a board where VDDP can be
switched and is not marked regulator-always-on. Could
regulator_late_cleanup() turn the rail off there?

drivers/regulator/core.c:regulator_late_cleanup() {
    ...
	if (rdev->use_count)
		goto unlock;
    ...
}

On DT systems have_full_constraints() is true, so the rail would be
disabled and the PHY would lose power to its MDIO pads.
regulator-boot-on does not prevent this, because late cleanup only checks
always_on.

If the rail is off at boot, nothing ever turns it on. The MDIO accesses
in mxl86211c_probe() and gpy_probe() would then fail.

Other PHY drivers handle this by enabling the supply. For example,
qcom/at803x.c calls devm_regulator_get_enable_optional(..., "vddio").
Should the MxL86211C driver do the same?

The binding example uses a regulator-fixed with no GPIO, which cannot be
switched off, so the example does not show this problem.

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261016112033.949026-1-robert.marko%40sartura.hr

Re: [PATCH net-next v2 2/5] net: phy: mxl-gpy: add MxL86211C VDDP supply

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 4 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 1 · 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).

- [High] Probe regression for existing MxL86211C users.
- [Medium] mxl86211c_probe() gets the VDDP supply with
  devm_regulator_get(), reads its voltage, programs the MDIO/MDC/MDINT…
- [Low] Baseline decision: invalid (kept only because
  requires_validation=true).
- [Low] Baseline decision: invalid for the final state of the series
  (kept only because requires_validation=true).

--

net: phy: mxl-gpy: add MxL86211C VDDP supply

This commit adds mxl86211c_probe(). It gets the VDDP supply, reads its
voltage and sets the MDIO/MDC/MDINT pad level in VSPEC1_PM_CTRL to 1.8 V
or 3.3 V. The MxL86211C entry in gpy_drivers[] now uses this probe
instead of gpy_probe().

> diff --git a/drivers/net/phy/mxl-gpy.c b/drivers/net/phy/mxl-gpy.c
> index 089a1a9e5bc61..fbac7c49b4759 100644
> --- a/drivers/net/phy/mxl-gpy.c
> +++ b/drivers/net/phy/mxl-gpy.c

[ ... ]

> +static int mxl86211c_probe(struct phy_device *phydev)
> +{
> +	struct device *dev = &phydev->mdio.dev;
> +	struct gpy_priv *priv;
> +	struct regulator *vddp;
> +	int ret;
> +
> +	vddp = devm_regulator_get(dev, "vddp");
> +	if (IS_ERR(vddp))
> +		return dev_err_probe(dev, PTR_ERR(vddp),
> +				     "failed to get VDDP supply\n");
> +	if (!vddp)
> +		return dev_err_probe(dev, -EOPNOTSUPP,
> +				     "VDDP supply requires regulator support\n");
> +
> +	ret = mxl86211c_configure_mdio_voltage(phydev, vddp);
> +	if (ret)
> +		return ret;

[Severity: High]
Will this break probe for existing MxL86211C users whose firmware has no
vddp-supply property? gpy_probe() has handled the MxL86211C since commit
9d844da693d6. DT nodes written before this series won't have the
property, and neither will PHYs matched only by MDIO ID.

With CONFIG_REGULATOR=y and full constraints, devm_regulator_get() falls
back to the dummy regulator. That regulator has empty ops, no fixed_uV
and no supply, so regulator_get_voltage() reaches the last branch of
regulator_get_voltage_rdev():

drivers/regulator/core.c:regulator_get_voltage_rdev() {
    ...
	} else if (rdev->supply_name) {
		return -EPROBE_DEFER;
	} else {
		return -EINVAL;
	}
    ...
}

mxl86211c_configure_mdio_voltage() passes that -EINVAL back, and
mxl86211c_probe() fails without printing any message.

Without full constraints, devm_regulator_get() returns ERR_PTR(-ENODEV).
With CONFIG_REGULATOR=n, the stub returns NULL, which becomes -EOPNOTSUPP
here. So probe fails in all three cases.

When probe fails, the PHY no longer binds to mxl-gpy and phylib falls
back to genphy/genphy_c45. The PHY then loses the SGMII/2500BASE-X
possible_interfaces setup, in-band AN handling, WoL, LEDs, hwmon and the
SGMII re-AN workarounds.

No in-tree DTS sets vddp-supply for this PHY. Could this use
devm_regulator_get_optional() instead, and leave the hardware/strap
default in place on -ENODEV?

The requirement is still there at the end of the series, because the
later mxl86211c_config_init() also fails when the voltage can't be read.

[Severity: Low]
The earlier binding patch in this series ("dt-bindings: net: maxlinear:
add MxL86211C VDDP supply") documents vddp-supply and makes it required
in maxlinear,gpy2xx.yaml. However, the schema's select only matches nodes
that carry the explicit compatible:

select:
  properties:
    compatible:
      contains:
        const: ethernet-phy-idc133.5400

A PHY node matched only by MDIO ID is not checked against this schema,
but mxl86211c_probe() still requires the supply for it. Should the
binding and the driver agree on which nodes need vddp-supply?

> +
> +	ret = gpy_probe(phydev);
> +	if (ret)
> +		return ret;
> +
> +	priv = phydev->priv;
> +	priv->vddp = vddp;
> +
> +	return 0;
> +}

[Severity: Medium]
Should the VDDP supply be enabled here? mxl86211c_probe() stores the
handle in priv->vddp but never calls regulator_enable() or uses
devm_regulator_get_enable(). As a result, rdev->use_count stays at 0 for
this consumer. No later patch in the series enables it either.

Suppose VDDP is a switchable regulator that is on at boot but not marked
regulator-always-on. Then regulator_late_cleanup() will turn it off:

drivers/regulator/core.c:regulator_late_cleanup() {
    ...
	if (rdev->use_count)
		goto unlock;
    ...
	if (have_full_constraints()) {
		...
		rdev_info(rdev, "disabling\n");
		ret = _regulator_do_disable(rdev);
    ...
}

On a shared rail, another consumer could also turn it off by dropping its
last enable reference. In either case the MDIO/MDC/MDINT pads lose power
while the driver is still bound.

regulator_get_voltage() also works on a disabled regulator, so the read
in mxl86211c_configure_mdio_voltage() does not show that the pads are
powered.

[ ... ]

> @@ -1406,7 +1467,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,

[Severity: Low]
At this commit, only mxl86211c_probe() sets the pad voltage. .config_init
is still gpy21x_config_init, which doesn't touch VSPEC1_PM_CTRL.

phy_detach() can assert a hardware reset, and phy_init_hw() releases it.
After that only .config_init runs again. Wouldn't VSPEC1_PM_CTRL_MDIO_VOL
then go back to its hardware default?

The next patch in the series, "net: phy: mxl-gpy: release MxL86211C
Super Isolate", adds mxl86211c_config_init(), which calls
mxl86211c_configure_mdio_voltage() again. So this only matters for
bisection.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261016112033.949026-1-robert.marko%40sartura.hr

Re: [PATCH net-next v2 3/5] net: phy: mxl-gpy: release MxL86211C Super Isolate

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 2 · 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] This series breaks existing MxL86211C setups, and this patch
  makes the new Super Isolate (SI) release depend on the same supply.
- [Medium] The VDDP (MDIO/MDC/MDINT pad) supply is acquired with
  devm_regulator_get() in mxl86211c_probe() but never enabled.
- [Medium] net: phy: mxl-gpy: firmware version read while PHY is halted

--

net: phy: mxl-gpy: release MxL86211C Super Isolate

This adds an MxL86211C config_init callback. It reapplies the MDIO pad
voltage from the VDDP supply, then clears the Super Isolate bit in
VSPEC1_PM_CTRL, then calls gpy21x_config_init(). A PHY strapped into
Super Isolate is then released after each hardware reset.

> diff --git a/drivers/net/phy/mxl-gpy.c b/drivers/net/phy/mxl-gpy.c
> index fbac7c49b4759..8bc119655f104 100644
> --- a/drivers/net/phy/mxl-gpy.c
> +++ b/drivers/net/phy/mxl-gpy.c

[ ... ]

> @@ -422,6 +423,23 @@ static int mxl86211c_configure_mdio_voltage(struct phy_device *phydev,
>  			      VSPEC1_PM_CTRL_MDIO_VOL, val);
>  }
>  
> +static int mxl86211c_config_init(struct phy_device *phydev)
> +{
> +	struct gpy_priv *priv = phydev->priv;
> +	int ret;
> +
> +	ret = mxl86211c_configure_mdio_voltage(phydev, priv->vddp);
> +	if (ret)
> +		return ret;

[Severity: High]
Does this make releasing Super Isolate depend on the VDDP supply
reporting exactly 1.8V or 3.3V?

If mxl86211c_configure_mdio_voltage() fails, mxl86211c_config_init()
returns before VSPEC1_PM_CTRL_SI is cleared. Clearing the SI bit has
nothing to do with the pad voltage.

This adds to a problem from the earlier patch in this series ("net: phy:
mxl-gpy: add MxL86211C VDDP supply"), where mxl86211c_probe() now
requires the supply:

drivers/net/phy/mxl-gpy.c:mxl86211c_probe() {
    ...
	vddp = devm_regulator_get(dev, "vddp");
	if (IS_ERR(vddp))
		return dev_err_probe(dev, PTR_ERR(vddp),
				     "failed to get VDDP supply\n");
	if (!vddp)
		return dev_err_probe(dev, -EOPNOTSUPP,
				     "VDDP supply requires regulator support\n");

	ret = mxl86211c_configure_mdio_voltage(phydev, vddp);
	if (ret)
		return ret;
    ...
}

MxL86211C support is older than this series and has never used a supply,
so no existing DT has a vddp-supply property. Without that property,
devm_regulator_get() returns the dummy regulator. The dummy has no ops,
no fixed_uV, no supply and no supply_name, so
regulator_get_voltage_rdev() returns -EINVAL for it:

drivers/regulator/core.c:regulator_get_voltage_rdev() {
    ...
	} else if (rdev->supply_name) {
		return -EPROBE_DEFER;
	} else {
		return -EINVAL;
	}
    ...
}

With CONFIG_REGULATOR=n, the stub returns NULL and probe fails with
-EOPNOTSUPP instead. Non-DT setups and PHYs found by MDIO scan go down
the same paths.

In every case mxl86211c_probe() fails and the mxl-gpy driver no longer
binds to the PHY. That loses SGMII/2500BASE-X handling, LEDs, WoL and
stats.

The binding select only matches nodes with the ethernet-phy-idc133.5400
compatible, and no in-tree DTS uses that compatible. So dtbs_check won't
flag existing nodes that lack the supply.

Neither commit message says existing setups stop working. The last patch
in the series ("net: phy: mxl-gpy: support MxL86211C lane mapping") keeps
the same probe and config_init ordering, so the problem is still there at
the end of the series.

Could the supply be made optional, with the pad voltage setup skipped
when no supply is described and the SI bit cleared either way?

> +
> +	ret = phy_modify_mmd(phydev, MDIO_MMD_VEND1, VSPEC1_PM_CTRL,
> +			     VSPEC1_PM_CTRL_SI, 0);
> +	if (ret)
> +		return ret;

[Severity: Medium]
Does gpy_probe() still read the firmware version while the PHY is
halted?

mxl86211c_probe() calls gpy_probe(), which reads PHY_FWV once and
caches priv->fw_major and priv->fw_minor. With this version, Super
Isolate is only cleared from mxl86211c_config_init(). That runs from
phy_init_hw() on attach and resume, well after probe:

mxl86211c_probe()
  gpy_probe()
    phy_read(phydev, PHY_FWV)
...
phy_attach_direct()
  phy_init_hw()
    mxl86211c_config_init()
      phy_modify_mmd(..., VSPEC1_PM_CTRL, VSPEC1_PM_CTRL_SI, 0)

The commit message says a strapped PHY stays halted after reset until
software releases it. Suppose PHY_FWV is filled in by the PHY
firmware. Then on a strapped board, probe caches whatever the register
holds before the firmware has run, most likely 0. Nothing re-reads it
after SI is cleared, so the bogus value sticks for the life of the
device.

That value is printed as the firmware version in dmesg. It is also
used by gpy_sgmii_need_reaneg(), which compares it against
ver_need_sgmii_reaneg to decide whether gpy_config_aneg() applies the
SGMII re-ANEG workaround. A zeroed version would not match any of
those entries, so the workaround would be skipped on firmware that
needs it.

v1 cleared SI in probe. Is PHY_FWV valid while the PHY is in Super
Isolate? If not, should SI also be released in mxl86211c_probe()
before gpy_probe() reads the version? Or should config_init read the
version again after clearing the bit?

[Severity: Medium]
Is the VDDP supply ever enabled?

mxl86211c_probe() gets it with devm_regulator_get(). There is no
regulator_enable() or devm_regulator_get_enable() call, and the only use
of the supply is regulator_get_voltage() in
mxl86211c_configure_mdio_voltage(). So the consumer use_count stays at 0.

Suppose the VDDP rail can be switched and is not always-on. Examples are
a GPIO-controlled regulator-fixed, or a PMIC LDO marked only
regulator-boot-on. In that case regulator_late_cleanup() turns the rail
off after boot:

drivers/regulator/core.c:regulator_late_cleanup() {
    ...
	if (rdev->use_count)
		goto unlock;
    ...
	if (have_full_constraints()) {
		rdev_info(rdev, "disabling\n");
		ret = _regulator_do_disable(rdev);
    ...
}

If the rail is off at boot, nothing turns it on.

phy_init_hw() calls mxl86211c_config_init() on attach and on resume,
often after late cleanup has run. It then does read-modify-writes on
VSPEC1_PM_CTRL over MDIO pads that may have no power:

phy_attach_direct() / mdio_bus_phy_resume()
  phy_init_hw()
    mxl86211c_config_init()
      mxl86211c_configure_mdio_voltage()
        phy_modify_mmd(..., VSPEC1_PM_CTRL, VSPEC1_PM_CTRL_MDIO_VOL, val)
      phy_modify_mmd(..., VSPEC1_PM_CTRL, VSPEC1_PM_CTRL_SI, 0)

Can these accesses fail, or read all-ones and write back bad values,
leaving the PHY in Super Isolate?

The binding does not require the rail to be always-on. The missing
enable comes from the earlier "net: phy: mxl-gpy: add MxL86211C VDDP
supply" patch in this series.

Should probe keep the supply enabled, for example by using
devm_regulator_get_enable()?

> +
> +	return gpy21x_config_init(phydev);
> +}

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261016112033.949026-1-robert.marko%40sartura.hr

Re: [PATCH net-next v2 4/5] dt-bindings: net: maxlinear: add MxL86211C lane mapping support

[netdev-bot+sashiko]

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] REFUTED (baseline decision false; kept only because
  requires_validation=true).
- [Low] The new line `else: properties: maxlinear,lane-asp-map: false` in
  maxlinear,gpy2xx.yaml can never take effect.

--

dt-bindings: net: maxlinear: add MxL86211C lane mapping support

This adds an optional maxlinear,lane-asp-map property to the GPY2xx PHY
binding. The property describes how the physical TPI lanes map to the
MxL86211C analog front ends (ASPs). The patch also extends the existing
else branch so the property is rejected on PHYs other than the MxL86211C.

> diff --git a/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml b/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml
> index a0dfb6192d81b..4323be300c721 100644
> --- a/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml
> +++ b/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml
> @@ -35,6 +35,7 @@ allOf:
>      else:
>        properties:
>          vddp-supply: false
> +        maxlinear,lane-asp-map: false

[Severity: Low]
Can this else branch ever be evaluated? The top-level select of this schema
only matches nodes that carry the MxL86211C PHY ID compatible:

select:
  properties:
    compatible:
      contains:
        const: ethernet-phy-idc133.5400
  required:
    - compatible

The if inside allOf tests the same condition:

  - if:
      properties:
        compatible:
          contains:
            const: ethernet-phy-idc133.5400
      required:
        - compatible

So every node this schema applies to takes the then branch. No other binding
references maxlinear,gpy2xx.yaml, so the else branch has no other way to be
reached.

Other PHY nodes are checked only by ethernet-phy.yaml, which selects by
$nodename and has additionalProperties: true. That covers other GPY2xx PHYs,
and also MxL86211C PHYs that leave out the optional PHY ID compatible because
they are auto-probed.

On those nodes, maxlinear,lane-asp-map: false is never enforced. The
minItems/maxItems, minimum/maximum and uniqueItems constraints are never
checked either, so a value like <0 0> would pass dtbs_check. The problem
would then only show up at boot, when the driver's config_init rejects the
value.

The vddp-supply: false line added earlier in this series has the same
problem. This patch adds a second restriction that depends on it.

Would it make sense to change the select so it matches every node that could
use this binding, or to drop the else branch? Another option is to require
the PHY ID compatible whenever maxlinear,lane-asp-map is present.

> @@ -42,6 +43,21 @@ properties:
>        Regulator supplying the VDDP pin. Its voltage configures the MDIO,
>        MDC, and MDINT pad voltage and must be either 1.8 V or 3.3 V.
>  
> +  maxlinear,lane-asp-map:
> +    description: |
> +      Mapping of physical TPI lanes A through D to analog front ends (ASPs)
> +      A through D. The array index identifies the physical TPI lane, while
> +      its value identifies the analog front end, encoded as 0 through 3.
> +      Each analog front end 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

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261016112033.949026-1-robert.marko%40sartura.hr

Re: [PATCH net-next v2 5/5] net: phy: mxl-gpy: support MxL86211C lane mapping

[netdev-bot+sashiko]

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 binding and the driver disagree about which nodes
  `maxlinear,lane-asp-map` is valid on and how long it may be.

--

net: phy: mxl-gpy: support MxL86211C lane mapping

This commit lets the MxL86211C take an optional maxlinear,lane-asp-map DT
property. The property programs the mapping from physical TPI lanes to
analog front ends in VSPEC1_LANE_ASP_MAP. When the property is absent, the
hardware reset mapping is left alone.

> diff --git a/drivers/net/phy/mxl-gpy.c b/drivers/net/phy/mxl-gpy.c
> index 8bc119655f104..acd5ad01a285d 100644
> --- a/drivers/net/phy/mxl-gpy.c
> +++ b/drivers/net/phy/mxl-gpy.c

[ ... ]

> @@ -423,6 +430,42 @@ static int mxl86211c_configure_mdio_voltage(struct phy_device *phydev,
>  			      VSPEC1_PM_CTRL_MDIO_VOL, val);
>  }
>  
> +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));

[Severity: Low]
Should a maxlinear,lane-asp-map with more than 4 cells be rejected here?

With the OF backend, device_property_read_u32_array() ends up in
of_property_read_variable_u32_array() with sz_max set to 0.
of_find_property_value_of_size() treats that as "no upper bound":

drivers/of/property.c:of_find_property_value_of_size() {
    ...
	if (max && prop->length > max)
		return ERR_PTR(-EOVERFLOW);
    ...
}

So a value like <0 1 3 2 1> is accepted. Only the first 4 cells are
used, and nothing is logged. The binding says the property has exactly
4 items.

The range and duplicate checks below still keep bad values out of
VSPEC1_LANE_ASP_MAP. The problem is that the driver and the binding
accept different things.

The schema may not catch this case either. The driver binds by PHY ID
through PHY_ID_MATCH_MODEL(PHY_ID_MXL86211C), whatever the node's
compatible is. However, maxlinear,gpy2xx.yaml (from the earlier
"dt-bindings: net: maxlinear: add MxL86211C VDDP supply" patch) only
selects nodes that carry this exact compatible:

select:
  properties:
    compatible:
      contains:
        const: ethernet-phy-idc133.5400
  required:
    - compatible

An ethernet-phy@N node without that compatible is only checked against
ethernet-phy.yaml, and that schema allows additional properties. So a
malformed maxlinear,lane-asp-map on such a node would pass dtbs_check,
and the driver would still read it.

There is a related issue in "dt-bindings: net: maxlinear: add MxL86211C
lane mapping support". The allOf if/then/else tests the same compatible
that select already requires:

    else:
      properties:
        vddp-supply: false
        maxlinear,lane-asp-map: false

Can this else branch ever be evaluated? No other schema references this
one, so the ban on these properties for other GPY PHYs never seems to
take effect.

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261016112033.949026-1-robert.marko%40sartura.hr