Re: [PATCH net-next v2 6/9] net: stmmac: qcom-ethqos: warn about legacy RGMII PHY modes

[netdev-bot+sashiko]

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

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

- [Medium] Unactionable boot-time warning on every in-tree qcom ethqos
  platform: the new dev_warn() in qcom_ethqos_probe() (dwmac-qcom-…
- [Low] Commit message / code mismatch: the changelog of "net: stmmac:
  qcom-ethqos: warn about legacy RGMII PHY modes" states that "existing…

--

net: stmmac: qcom-ethqos: warn about legacy RGMII PHY modes

This adds a probe-time dev_warn() in qcom_ethqos_probe() when the ethernet
node uses one of the older RGMII phy-mode values, suggesting a move to
"rgmii-id" with PHY-provided delays.  PHY_INTERFACE_MODE_RGMII_ID is split
out of the shared case group so that it does not warn.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
> index 53fb9de4d92c7..a92bf4a6a2dbd 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
> @@ -764,9 +764,11 @@ static int qcom_ethqos_probe(struct platform_device *pdev)
>  	ethqos->phy_mode = plat_dat->phy_interface;
>  	switch (ethqos->phy_mode) {
>  	case PHY_INTERFACE_MODE_RGMII:
> -	case PHY_INTERFACE_MODE_RGMII_ID:
>  	case PHY_INTERFACE_MODE_RGMII_RXID:
>  	case PHY_INTERFACE_MODE_RGMII_TXID:
> +		dev_warn(dev, "legacy RGMII phy-mode detected; consider upgrading to a newer DTB\n");

[Severity: Low]
The changelog says:

    However, existing DTBs use "rgmii" and "rgmii-txid" and must remain
    supported for backwards compatibility.

    Warn when either of these legacy PHY modes is used [...]

but the warning in qcom_ethqos_probe() is attached to a group of three case
labels, so PHY_INTERFACE_MODE_RGMII_RXID also warns.

Is "rgmii-rxid" intended to be declared legacy as well?  The driver does
treat these modes differently, for example in ethqos_rgmii_macro_init():

	if (ethqos->phy_mode == PHY_INTERFACE_MODE_RGMII_TXID)
		phase_shift = 0;
	else
		phase_shift = RGMII_CONFIG2_TX_CLK_PHASE_SHIFT_EN;

so which phy-modes are being deprecated seems worth stating exactly in the
commit message.

[Severity: Medium]
Is there an upstream DTB that this warning can point people at?  No in-tree
device tree for this driver uses "rgmii-id", and this series does not touch
arch/ at all, so every in-tree board hits the new warning on each probe:

  arch/arm64/boot/dts/qcom/qcs404-evb-4000.dts	phy-mode = "rgmii";
  arch/arm64/boot/dts/qcom/sa8155p-adp.dts	phy-mode = "rgmii";
  arch/arm64/boot/dts/qcom/sa8540p-ride.dts	phy-mode = "rgmii-txid";  (both MACs)

The binding example in Documentation/devicetree/bindings/net/qcom,ethqos.yaml
also still uses:

      phy-mode = "rgmii";

and the binding carries no phy-mode deprecation note, so the documented ABI
still recommends what the driver now calls legacy.

Converting those device trees is also the only way the advocated
PHY-provided-delay path gets exercised, since ethqos_set_clk_tx_rate() stops
doubling the link clock only for PHY_INTERFACE_MODE_RGMII_ID.  Would it make
sense to convert the in-tree DTs (and the binding example/text) in the same
series, or to hold off on the warning until a migration target exists?

> +		fallthrough;
> +	case PHY_INTERFACE_MODE_RGMII_ID:
>  		plat_dat->fix_mac_speed = ethqos_fix_mac_speed_rgmii;
>  		break;
>  	case PHY_INTERFACE_MODE_2500BASEX:

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260908-shikra_ethernet-v2-0-bbe3389d0652%40oss.qualcomm.com