Re: [PATCH net-next v10 6/6] net: stmmac: qcom-ethqos: add support for sa8255p
From: Simon Horman <horms@kernel.org>
Date: 2026-03-25 16:54:15
Also in:
imx, linux-amlogic, linux-arm-msm, linux-devicetree, linux-mips, linux-renesas-soc, linux-riscv, linux-rockchip, linux-sunxi, lkml
This is an AI-generated review of your patch. The human sending this email has considered the AI review valid, or at least plausible. For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html --- net: stmmac: qcom-ethqos: add support for sa8255p Extend the driver to support a new model - sa8255p. Unlike the previously supported variants, this one's power management is done in the firmware using SCMI. This is modeled in linux using power domains so add support for them.
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c index f32ff0d9ce51..09ce80b446cb 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -710,6 +785,45 @@ static int ethqos_mac_finish_serdes(struct net_device *ndev, void *priv, return ret; } +static int ethqos_mac_finish_serdes_pd(struct net_device *ndev, void *priv, + unsigned int mode, + phy_interface_t interface) +{ + struct qcom_ethqos *ethqos = priv; + struct device *dev = ethqos->pd.pd_list->pd_devs[ETHQOS_PD_SERDES]; + int ret = 0; + + qcom_ethqos_set_sgmii_loopback(ethqos, false); + + if (interface == PHY_INTERFACE_MODE_SGMII || + interface == PHY_INTERFACE_MODE_2500BASEX) + ret = dev_pm_opp_set_level(dev, ethqos->pd.serdes_level); + + return ret; +} + +static int qcom_ethqos_pd_serdes_powerup(struct net_device *ndev, void *priv) +{ + struct qcom_ethqos *ethqos = priv; + struct device *dev = ethqos->pd.pd_list->pd_devs[ETHQOS_PD_SERDES]; + int ret; + + ret = qcom_ethqos_domain_on(ethqos, ETHQOS_PD_SERDES); + if (ret < 0) + return ret; + + return dev_pm_opp_set_level(dev, ethqos->pd.serdes_level); +}
Does this leak the PM runtime reference if dev_pm_opp_set_level() fails?
qcom_ethqos_pd_serdes_powerup() calls qcom_ethqos_domain_on() which
acquires a PM runtime reference via pm_runtime_resume_and_get():
qcom_ethqos_pd_serdes_powerup()
qcom_ethqos_domain_on()
pm_runtime_resume_and_get(dev)
If the subsequent dev_pm_opp_set_level() fails, the function returns the
error without calling qcom_ethqos_domain_off() to release the reference.
The caller stmmac_legacy_serdes_power_up() in stmmac_main.c does not set
legacy_serdes_is_powered=true on error, so stmmac_legacy_serdes_power_down()
in the cleanup path skips the powerdown call.
In the existing non-PD code path, qcom_ethqos_serdes_powerup() correctly
handles this case by calling phy_exit() when phy_power_on() fails:
drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c:qcom_ethqos_serdes_powerup() {
ret = phy_init(ethqos->pm.serdes_phy);
if (ret)
return ret;
ret = phy_power_on(ethqos->pm.serdes_phy);
if (ret) {
phy_exit(ethqos->pm.serdes_phy);
return ret;
}
...
}
Should qcom_ethqos_pd_serdes_powerup() call qcom_ethqos_domain_off()
before returning the error from dev_pm_opp_set_level()?
+
+static void qcom_ethqos_pd_serdes_powerdown(struct net_device *ndev, void *priv)
+{
+ struct qcom_ethqos *ethqos = priv;
+ struct device *dev = ethqos->pd.pd_list->pd_devs[ETHQOS_PD_SERDES];
+
+ dev_pm_opp_set_level(dev, 0);
+ qcom_ethqos_domain_off(ethqos, ETHQOS_PD_SERDES);
+}[ ... ]