Re: [PATCH net-next v7 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 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] Concern that the newly documented `rockchip,sgmii-mac-sel`
  property has no in-tree consumer, so a DT setting `rockchip,sgmii-mac-…

--

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

This adds an optional rockchip,sgmii-mac-sel property to the naneng-combphy
binding so that a device tree can select whether the RK3568 SGMII lane is
routed to GMAC0 or GMAC1 via the pipe_sgmii_mac_sel GRF bit.  The property
is restricted to the values 0 and 1, with a documented default of 1, and is
allowed only for the compatible handled by the surrounding if block.

> 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 for the record in case anyone looks at this
commit in isolation.

At this commit alone there is no consumer of rockchip,sgmii-mac-sel in
drivers/phy/rockchip/phy-rockchip-naneng-combphy.c, so a device tree with
rockchip,sgmii-mac-sel = <0> would pass dt_binding_check and probe while the
SGMII lane stays routed to GMAC1.

The following patch in the series, "phy: rockchip: naneng-combphy: add SGMII
MAC selection for RK3568", adds the consumer:

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

  rk3568_combphy_cfg() {
      ...
      case PHY_TYPE_SGMII:
          rockchip_combphy_param_write(priv->pipe_grf, &cfg->pipe_sgmii_mac_sel,
                                       priv->sgmii_mac_sel > 0);
      ...
  }

The 0/1 encoding and the default of 1 in the schema match that driver code,
and bindings-before-driver is the expected submission order, so nothing needs
changing here.

> +
>    "#phy-cells":
>      const: 1
>  

[ ... ]

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