Re: [PATCH net-next v10 01/11] net: stmmac: move XPCS lifetime management to platform drivers

[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] Commit message audit claim: "every DTS that pairs a dwmac node
  with a PCS goes through a glue that sets pcs_init() (Intel, Renesas,…
- [Low] Commit message framing: the removal of the unconditional
  `priv->hw->xpcs = xpcs;` in stmmac_pcs_setup() is described as fixing…

--

net: stmmac: move XPCS lifetime management to platform drivers

This moves XPCS creation and destruction out of the common
stmmac_pcs_setup() and stmmac_pcs_clean(), which now only dispatch to
plat->pcs_init() and plat->pcs_exit(). The Intel mGbE glue gains its own
pcs_init()/pcs_exit() pair, and the renesas-gbeth, rzn1 and socfpga
pcs_exit() callbacks clear priv->hw->phylink_pcs after destroying the PCS.

No Link: tags are present in the commit header.

I looked at the two places in stmmac_pcs_setup() that seemed worth double
checking and did not find anything to report.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
> index afe98ff5bdcb0..7396b68899c66 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
> @@ -426,35 +426,14 @@ int stmmac_mdio_reset(struct mii_bus *bus)
>  int stmmac_pcs_setup(struct net_device *ndev)
>  {
>  	struct stmmac_priv *priv = netdev_priv(ndev);
> -	struct fwnode_handle *devnode, *pcsnode;
> -	struct dw_xpcs *xpcs = NULL;
> -	int addr, ret;
> -
> -	devnode = dev_fwnode(priv->device);
> -
> -	if (priv->plat->pcs_init) {
> -		ret = priv->plat->pcs_init(priv);
> -	} else if (fwnode_property_present(devnode, "pcs-handle")) {
> -		pcsnode = fwnode_find_reference(devnode, "pcs-handle", 0);
> -		xpcs = xpcs_create_fwnode(pcsnode);
> -		fwnode_handle_put(pcsnode);
> -		ret = PTR_ERR_OR_ZERO(xpcs);
> -	} else if (priv->plat->mdio_bus_data &&
> -		   priv->plat->mdio_bus_data->pcs_mask) {
> -		addr = ffs(priv->plat->mdio_bus_data->pcs_mask) - 1;
> -		xpcs = xpcs_create_mdiodev(priv->mii, addr);
> -		ret = PTR_ERR_OR_ZERO(xpcs);
> -	} else {
> +	int ret;
> +
> +	if (!priv->plat->pcs_init)
>  		return 0;
> -	}

Dropping the generic "pcs-handle" and pcs_mask parsing here matches the
in-tree users I could find: the only dwmac nodes carrying pcs-handle go
through dwmac-rzn1 (r9a06g032.dtsi) and dwmac-renesas-gbeth, both of which
set plat->pcs_init(), and dwmac-intel gains pcs_init() in this same patch.

>  
> +	ret = priv->plat->pcs_init(priv);
>  	if (ret)
> -		return dev_err_probe(priv->device, ret, "No xPCS found\n");
> -
> -	if (xpcs)
> -		xpcs_config_eee_mult_fact(xpcs, priv->plat->mult_fact_100ns);
> -
> -	priv->hw->xpcs = xpcs;
> +		return dev_err_probe(priv->device, ret, "Failed to initialize PCS\n");
>  
>  	return 0;
>  }

Removing the trailing priv->hw->xpcs assignment also looks right. None of
the pcs_init() implementations present before this patch touch
priv->hw->xpcs (renesas_gmac_pcs_init(), rzn1_dwmac_pcs_init() and
socfpga_dwmac_pcs_init() all set priv->hw->phylink_pcs), so no existing
platform changes behaviour, and a glue that does set priv->hw->xpcs from
pcs_init() would previously have had it clobbered.

> @@ -463,14 +442,10 @@ void stmmac_pcs_clean(struct net_device *ndev)
>  {
>  	struct stmmac_priv *priv = netdev_priv(ndev);
>  
> -	if (priv->plat->pcs_exit)
> -		priv->plat->pcs_exit(priv);
> -
> -	if (!priv->hw->xpcs)
> +	if (!priv->plat->pcs_exit)
>  		return;
>  
> -	xpcs_destroy(priv->hw->xpcs);
> -	priv->hw->xpcs = NULL;
> +	priv->plat->pcs_exit(priv);
>  }

The Intel side pairs this with intel_mgbe_pcs_exit(), which calls
xpcs_destroy() and clears priv->hw->xpcs, so the destroy path is preserved
for the only platform that was relying on the common code.

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