Re: [PATCH net-next v6 02/11] dt-bindings: phy: rockchip: naneng-combphy: add rockchip,sgmii-mac-sel property

[netdev-bot+sashiko@kernel.org]

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

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

- [Low] Concern (verified FALSE POSITIVE, retained only per validation
  policy): the binding was said to document `rockchip,sgmii-mac-sel`…

--

dt-bindings: phy: rockchip: naneng-combphy: add rockchip,sgmii-mac-sel property

This adds an optional uint32 property rockchip,sgmii-mac-sel to the
naneng-combphy binding, gated so it is only accepted for
rockchip,rk3568-naneng-combphy.  The value selects whether SGMII is routed
to GMAC0 (0) or GMAC1 (1), with 1 documented as the hardware reset value.

> diff --git a/Documentation/devicetree/bindings/phy/phy-rockchip-naneng-combphy.yaml b/Documentation/devicetree/bindings/phy/phy-rockchip-naneng-combphy.yaml
> index 379b08bd9e97a..8e898bce9af73 100644
> --- a/Documentation/devicetree/bindings/phy/phy-rockchip-naneng-combphy.yaml
> +++ b/Documentation/devicetree/bindings/phy/phy-rockchip-naneng-combphy.yaml
> @@ -80,6 +80,15 @@ properties:
>      description:
>        Some additional pipe settings are accessed through GRF regs.
>  
> +  rockchip,sgmii-mac-sel:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [0, 1]
> +    default: 1
> +    description:
> +      Select gmac0 or gmac1 to be used as SGMII controller.
> +      The hardware reset value is GMAC1 (1). Set this to 0 to route
> +      SGMII to GMAC0.
> +

[Severity: Low]
This isn't a bug, just a note on patch ordering for anyone reading this
patch on its own: at this commit there is no in-tree consumer of
rockchip,sgmii-mac-sel yet, so the new ABI looks inert here.

Checking the rest of the series, the consumer does arrive immediately
after in "phy: rockchip: naneng-combphy: add SGMII MAC selection for
RK3568", which parses the property in rockchip_combphy_parse_dt():

    priv->sgmii_mac_sel = 1;
    device_property_read_u32(dev, "rockchip,sgmii-mac-sel", &priv->sgmii_mac_sel);

and applies it in the PHY_TYPE_SGMII branch of rk3568_combphy_cfg():

    rockchip_combphy_param_write(priv->pipe_grf, &cfg->pipe_sgmii_mac_sel,
                                 priv->sgmii_mac_sel > 0);

with the RK3568 descriptor .pipe_sgmii_mac_sel = { 0x0040, 1, 1, 0x00, 0x01 },
and the Photonicat DT patch then sets rockchip,sgmii-mac-sel = <0> on
&combphy2.  So the GMAC0 routing does work at the end of the series and
there is nothing to fix; would it be worth mentioning the driver patch in
this commit message so the dependency is obvious when the binding is
reviewed in isolation?

> [ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260915123802.1561724-1-coiaprant%40gmail.com