From 49d8bbc21244f0c2d2ebf5ee03fb7352ff019ecc Mon Sep 17 00:00:00 2001 From: Coia Prant Date: Fri, 23 Oct 2026 15:52:12 +0800 Subject: net: pcs: xpcs-plat: fix runtime PM initialization The driver calls pm_runtime_set_active() before runtime PM is enabled and before the CSR clock is prepared and enabled. If the device is later suspended (e.g. during unbind), the runtime suspend callback tries to disable a clock that was never enabled, producing: clk_csr already disabled clk_csr already unprepared Set the initial runtime PM status to suspended instead, and register a devm action that force-suspends the device on teardown. Without that action the CSR clock would stay enabled if the driver is removed while runtime-active, since nothing triggers a runtime suspend on unbind. pm_runtime_force_suspend() is not checked because the runtime suspend callback cannot fail. The clock is enabled on the first runtime resume (e.g. during MDIO access). No upstream platform currently enables this driver with a real CSR clock dependency, so the issue is not reachable in mainline today. It was found while using this driver as a reference for the Rockchip XPCS glue driver. Signed-off-by: Coia Prant Link: https://patch.msgid.link/20261023075214.1793922-3-coiaprant@gmail.com Signed-off-by: Jakub Kicinski --- drivers/net/pcs/pcs-xpcs-plat.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) (limited to 'drivers') diff --git a/drivers/net/pcs/pcs-xpcs-plat.c b/drivers/net/pcs/pcs-xpcs-plat.c index e669a7b248ac..11c4239bfd74 100644 --- a/drivers/net/pcs/pcs-xpcs-plat.c +++ b/drivers/net/pcs/pcs-xpcs-plat.c @@ -275,6 +275,14 @@ static int xpcs_plat_init_res(struct dw_xpcs_plat *pxpcs) return 0; } +static void xpcs_plat_exit_clk(void *data) +{ + struct dw_xpcs_plat *pxpcs = data; + struct device *dev = &pxpcs->pdev->dev; + + pm_runtime_force_suspend(dev); +} + static int xpcs_plat_init_clk(struct dw_xpcs_plat *pxpcs) { struct device *dev = &pxpcs->pdev->dev; @@ -285,10 +293,12 @@ static int xpcs_plat_init_clk(struct dw_xpcs_plat *pxpcs) return dev_err_probe(dev, PTR_ERR(pxpcs->cclk), "Failed to get CSR clock\n"); - pm_runtime_set_active(dev); - ret = devm_pm_runtime_enable(dev); + pm_runtime_set_suspended(dev); + pm_runtime_enable(dev); + + ret = devm_add_action_or_reset(dev, xpcs_plat_exit_clk, pxpcs); if (ret) { - dev_err(dev, "Failed to enable runtime-PM\n"); + dev_err(dev, "Failed to register devm for xpcs_plat_exit_clk: %d\n", ret); return ret; } -- cgit v1.2.3 From eea365ed283cb2a7418b277fef87b462418e2283 Mon Sep 17 00:00:00 2001 From: Coia Prant Date: Fri, 23 Oct 2026 15:52:13 +0800 Subject: net: pcs: xpcs-plat: fix clock lifecycle for !CONFIG_PM builds With CONFIG_PM disabled, pm_runtime_enable() and the runtime resume path are compiled out, so the CSR clock is never prepared/enabled. On platforms that provide a real "csr" clock this makes register access fail. Handle this by preparing/enabling the CSR clock manually in the !PM case and disabling it in the devm cleanup action. The CONFIG_PM path is left unchanged and still relies on runtime PM to gate the clock. Use RUNTIME_PM_OPS() and pm_ptr() for the PM ops as well, so that in !CONFIG_PM builds the runtime callbacks are still referenced and .pm is NULL rather than pointing at an empty ops structure. This also drops the __maybe_unused annotations from the callbacks, which were only needed because SET_RUNTIME_PM_OPS() expands to nothing when CONFIG_PM is disabled. Both changes are no-ops when CONFIG_PM is enabled. The CSR clock is optional (devm_clk_get_optional()), so platforms without a clock keep working in both cases, and clk_prepare_enable() on a NULL clock is a no-op. Because the !PM case is handled in the driver, there is no need to select or depend on PM in Kconfig for all platforms; keep the current Kconfig as-is to preserve compile coverage. Signed-off-by: Coia Prant Link: https://patch.msgid.link/20261023075214.1793922-4-coiaprant@gmail.com Signed-off-by: Jakub Kicinski --- drivers/net/pcs/pcs-xpcs-plat.c | 46 +++++++++++++++++++++++++++++++++++------ 1 file changed, 40 insertions(+), 6 deletions(-) (limited to 'drivers') diff --git a/drivers/net/pcs/pcs-xpcs-plat.c b/drivers/net/pcs/pcs-xpcs-plat.c index 11c4239bfd74..c4404500bac6 100644 --- a/drivers/net/pcs/pcs-xpcs-plat.c +++ b/drivers/net/pcs/pcs-xpcs-plat.c @@ -275,6 +275,7 @@ static int xpcs_plat_init_res(struct dw_xpcs_plat *pxpcs) return 0; } +#ifdef CONFIG_PM static void xpcs_plat_exit_clk(void *data) { struct dw_xpcs_plat *pxpcs = data; @@ -304,6 +305,39 @@ static int xpcs_plat_init_clk(struct dw_xpcs_plat *pxpcs) return 0; } +#else +static void xpcs_plat_exit_clk(void *data) +{ + struct dw_xpcs_plat *pxpcs = data; + + clk_disable_unprepare(pxpcs->cclk); +} + +static int xpcs_plat_init_clk(struct dw_xpcs_plat *pxpcs) +{ + struct device *dev = &pxpcs->pdev->dev; + int ret; + + pxpcs->cclk = devm_clk_get_optional(dev, "csr"); + if (IS_ERR(pxpcs->cclk)) + return dev_err_probe(dev, PTR_ERR(pxpcs->cclk), + "Failed to get CSR clock\n"); + + ret = clk_prepare_enable(pxpcs->cclk); + if (ret) { + dev_err(dev, "Failed to enable CSR clock: %d\n", ret); + return ret; + } + + ret = devm_add_action_or_reset(dev, xpcs_plat_exit_clk, pxpcs); + if (ret) { + dev_err(dev, "Failed to register devm for xpcs_plat_exit_clk: %d\n", ret); + return ret; + } + + return 0; +} +#endif static int xpcs_plat_init_bus(struct dw_xpcs_plat *pxpcs) { @@ -408,7 +442,7 @@ static int xpcs_plat_probe(struct platform_device *pdev) return 0; } -static int __maybe_unused xpcs_plat_pm_runtime_suspend(struct device *dev) +static int xpcs_plat_pm_runtime_suspend(struct device *dev) { struct dw_xpcs_plat *pxpcs = dev_get_drvdata(dev); @@ -417,7 +451,7 @@ static int __maybe_unused xpcs_plat_pm_runtime_suspend(struct device *dev) return 0; } -static int __maybe_unused xpcs_plat_pm_runtime_resume(struct device *dev) +static int xpcs_plat_pm_runtime_resume(struct device *dev) { struct dw_xpcs_plat *pxpcs = dev_get_drvdata(dev); @@ -425,9 +459,9 @@ static int __maybe_unused xpcs_plat_pm_runtime_resume(struct device *dev) } static const struct dev_pm_ops xpcs_plat_pm_ops = { - SET_RUNTIME_PM_OPS(xpcs_plat_pm_runtime_suspend, - xpcs_plat_pm_runtime_resume, - NULL) + RUNTIME_PM_OPS(xpcs_plat_pm_runtime_suspend, + xpcs_plat_pm_runtime_resume, + NULL) }; DW_XPCS_INFO_DECLARE(xpcs_generic, DW_XPCS_ID_NATIVE, DW_XPCS_PMA_ID_NATIVE); @@ -456,7 +490,7 @@ static struct platform_driver xpcs_plat_driver = { .probe = xpcs_plat_probe, .driver = { .name = "dwxpcs", - .pm = &xpcs_plat_pm_ops, + .pm = pm_ptr(&xpcs_plat_pm_ops), .of_match_table = xpcs_of_ids, }, }; -- cgit v1.2.3 From e7cccfde61367452e9a43257a57eef0a95e7a6bf Mon Sep 17 00:00:00 2001 From: Coia Prant Date: Fri, 23 Oct 2026 15:52:14 +0800 Subject: net: pcs: xpcs-rk: fix cleanup action error message The devm action registered by xpcs_rk_init_clk() force-suspends the device and disables the EEE clock, but the current error message only mentions the EEE clock. Point at the cleanup action by name instead, so the message covers both parts of what the action does. Signed-off-by: Coia Prant Link: https://patch.msgid.link/20261023075214.1793922-5-coiaprant@gmail.com Signed-off-by: Jakub Kicinski --- drivers/net/pcs/pcs-xpcs-rk.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/net/pcs/pcs-xpcs-rk.c b/drivers/net/pcs/pcs-xpcs-rk.c index 698a43b75334..6f0a110e32d2 100644 --- a/drivers/net/pcs/pcs-xpcs-rk.c +++ b/drivers/net/pcs/pcs-xpcs-rk.c @@ -365,7 +365,7 @@ static int xpcs_rk_init_clk(struct dw_xpcs_rk *pxpcs) ret = devm_add_action_or_reset(dev, xpcs_rk_exit_clk, pxpcs); if (ret) { - dev_err(dev, "Failed to register devm for EEE clock: %d\n", ret); + dev_err(dev, "Failed to register devm for xpcs_rk_exit_clk: %d\n", ret); return ret; } -- cgit v1.2.3