Re: [PATCH net-next v7 08/11] dt-bindings: net: rockchip-dwmac: document pcs-handle

[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 (INVALIDATED): the commit message claims `pcs-handle`
  "will be parsed by rk_pcs_init(), and a missing phandle fails the…

--

dt-bindings: net: rockchip-dwmac: document pcs-handle

This adds a pcs-handle property to the Rockchip GMAC binding and makes it
required when phy-mode is "sgmii" on rockchip,rk3568-gmac.  The property
describes the PCS device used by the SGMII support added by a later patch
in the same series.

No regressions to report on this patch.

> diff --git a/Documentation/devicetree/bindings/net/rockchip-dwmac.yaml b/Documentation/devicetree/bindings/net/rockchip-dwmac.yaml
> index 80c252845349c..bb7540e838033 100644
> --- a/Documentation/devicetree/bindings/net/rockchip-dwmac.yaml
> +++ b/Documentation/devicetree/bindings/net/rockchip-dwmac.yaml
> @@ -120,6 +120,12 @@ properties:
>      maximum: 0x7F
>      default: 0x10
>  
> +  pcs-handle:
> +    description:
> +      Specifies a reference to a node representing the PCS device
> +      connected to this GMAC. Required when phy-mode is "sgmii".
> +    maxItems: 1
> +
>    phy-supply:
>      description: PHY regulator
>  

[ ... ]

> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: rockchip,rk3568-gmac
> +        phy-mode:
> +          contains:
> +            const: sgmii
> +    then:
> +      required:
> +        - pcs-handle
> +

One concern was raised during review of the hunks above and then dropped
after checking the rest of the series: the claim that the commit message
references a rk_pcs_init() that does not exist in tree, leaving a required
property with no consumer.

Looking at the end of the series, drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
does gain rk_pcs_init(), which parses the new property:

  drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c:rk_pcs_init() {
      ...
      pcs_node = of_parse_phandle(np, "pcs-handle", 0);
      if (!pcs_node)
              return -ENODEV;

      xpcs = xpcs_rk_create(priv->device, pcs_node);
      ...
  }

and rk_gmac_probe() installs it for PHY_INTERFACE_MODE_SGMII:

  drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c:rk_gmac_probe() {
      ...
      if (plat_dat->phy_interface == PHY_INTERFACE_MODE_SGMII) {
              /* SGMII clock always runs at 125 MHz */
              plat_dat->set_clk_tx_rate = NULL;

              /* SGMII requires a PCS */
              plat_dat->default_an_inband = true;
              plat_dat->pcs_init = rk_pcs_init;
      ...
  }

So the documented property does have an in-tree consumer and the commit
message matches the code.

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