Re: [PATCH 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] 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