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

[Mohd Ayaan Anwar]

On Fri, Sep 11, 2026 at 11:25:16AM +0000, netdev-bot+sashiko@xxxxxxxxxx wrote:
> 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.

I will make this clearer in v3 that all non RGMII-ID modes are being
deprecated.

> 
> [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:
> 

The plan is to convert the current DTs to use "rgmii-id" after this
patch series.

	Ayaan