Re: [PATCH net-next v13 06/10] net: stmmac: qcom-ethqos: update phy_mode to the resolved interface in mac_finish()

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Updating `ethqos->phy_mode` before clearing the SGMII loopback causes the loopback to remain erroneously enabled when switching away from 2500BASE-X.
--

--- Patch [6]: [PATCH net-next v13 06/10] net: stmmac: qcom-ethqos: update phy_mode to the resolved interface in mac_finish() ---
commit 3dfd6663f759c10c24659b5275b88ff5f976b420
Author: Bartosz Golaszewski <[email protected]>

net: stmmac: qcom-ethqos: update phy_mode to the resolved interface in mac_finish()

This commit updates the driver to overwrite ethqos->phy_mode with the
resolved phylink interface during the mac_finish() callback. This ensures
the stored mode matches runtime changes instead of being permanently stuck
at the device tree default.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
> index 47b70b5e706f2..9d19203ae4bd7 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
> @@ -632,6 +632,12 @@ static int ethqos_mac_finish_serdes(struct net_device *ndev, void *priv,
>  	struct qcom_ethqos *ethqos = priv;
>  	int ret = 0;
>  
> +	/* The interface mode is only known once phylink has resolved it and
> +	 * may change at runtime, so keep it in sync for later
> +	 * serdes_powerup()/serdes_powerdown() calls.
> +	 */
> +	ethqos->phy_mode = interface;
> +
>  	qcom_ethqos_set_sgmii_loopback(ethqos, false);

[Severity: High]
Does updating ethqos->phy_mode here prevent the loopback from being disabled?

If the link negotiates from 2500BASE-X down to SGMII, this update changes
ethqos->phy_mode to SGMII before the loopback is disabled.

Looking at qcom_ethqos_set_sgmii_loopback(), it checks the current mode to
determine if it needs to act:

qcom_ethqos_set_sgmii_loopback() {
    if (!ethqos->needs_sgmii_loopback ||
        ethqos->phy_mode != PHY_INTERFACE_MODE_2500BASEX)
            return;
    ...
}

Will this cause the function to return early without clearing the loopback
when switching away from 2500BASE-X, leaving outgoing packets reflected
internally instead of transmitting to the wire?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6

_______________________________________________
linux-amlogic mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-amlogic