[PATCH net v2 0/4] net: pcs: xpcs: fix runtime PM and clock lifecycle

[PATCH net v2 0/4] net: pcs: xpcs: fix runtime PM and clock lifecycle

[Coia Prant]

This series fixes the runtime PM and CSR clock lifecycle in the DW XPCS
platform driver, a misleading error message in the Rockchip glue driver,
and the MAINTAINERS coverage for the platform driver and its binding.

The issue was found while using pcs-xpcs-plat.c as a reference for the
Rockchip XPCS glue driver (pcs-xpcs-rk.c). On Rockchip the CSR clock
(PCLK_XPCS) is required for register access, which exposed two problems
in the generic platform driver:

  - it calls pm_runtime_set_active() before runtime PM is enabled and
    before the clock is prepared/enabled, so a later suspend tries to
    disable a clock that was never enabled;

  - with CONFIG_PM disabled, the runtime resume path is compiled out
    and the CSR clock is never enabled at all.

Patch 1 fixes the initial runtime PM status and registers a devm action
that force-suspends the device on teardown, so the clock does not leak
when the driver is removed while runtime-active.

Patch 2 handles !CONFIG_PM builds by preparing/enabling the CSR clock
manually and disabling it in the cleanup action. It also switches to
RUNTIME_PM_OPS() and pm_ptr(), which are no-ops when CONFIG_PM is
enabled, so that the runtime callbacks stay referenced and .pm is NULL
in !CONFIG_PM builds. Because the CSR clock is optional and both cases
are handled in the driver, Kconfig is left untouched and no PM
dependency is added.

Patch 3 rewords the cleanup action error message in the Rockchip glue
driver, which was merged into mainline today and currently mentions only
the EEE clock even though the action also tears down runtime PM.

Patch 4 lists pcs-xpcs-plat.c and its DT binding under the DW XPCS
entry. pcs-xpcs-plat.c currently only matches the ETHERNET PHY LIBRARY
entry, and the binding is only matched by the generic DT and networking
rules, so the entry does not cover the whole driver.

No upstream platform currently enables pcs-xpcs-plat with a real CSR
clock dependency, so neither runtime issue is reachable in mainline
today. Because of that, no Fixes: or Cc: stable tags are included, as
discussed in the v1 thread.

The patches touch code that is already in mainline: the platform driver
has been there since f6bb3e9d98c2, and the Rockchip glue driver was
merged today. They are fixes to existing code, not new drivers or new
features.

Changes in v2:
  - drop Fixes: and Cc: stable, and state in the commit messages that
    no upstream platform can trigger the issue;
  - add a note on Kconfig: the CSR clock is optional, so there is no
    need to select or depend on PM for all platforms;
  - add a fix for the Rockchip glue driver error message;
  - add MAINTAINERS coverage for pcs-xpcs-plat.c and its binding.

Thread v1: https://lore.kernel.org/all/20260704214808.1566710-1-coiaprant@gmail.com/
---
Coia Prant (4):
  net: pcs: xpcs-plat: fix runtime PM initialization
  net: pcs: xpcs-plat: fix clock lifecycle for !CONFIG_PM builds
  net: pcs: xpcs-rk: fix cleanup action error message
  MAINTAINERS: list DW XPCS platform driver and binding under XPCS entry

 MAINTAINERS                     |  2 ++
 drivers/net/pcs/pcs-xpcs-plat.c | 62 ++++++++++++++++++++++++++++-----
 drivers/net/pcs/pcs-xpcs-rk.c   |  2 +-
 3 files changed, 56 insertions(+), 10 deletions(-)

-- 
2.47.3

[PATCH net v2 1/4] net: pcs: xpcs-plat: fix runtime PM initialization

[Coia Prant]

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 <coiaprant@gmail.com>
---
 drivers/net/pcs/pcs-xpcs-plat.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

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;
 	}
 
-- 
2.47.3

[PATCH net v2 2/4] net: pcs: xpcs-plat: fix clock lifecycle for !CONFIG_PM builds

[Coia Prant]

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 <coiaprant@gmail.com>
---
 drivers/net/pcs/pcs-xpcs-plat.c | 46 ++++++++++++++++++++++++++++-----
 1 file changed, 40 insertions(+), 6 deletions(-)

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,
 	},
 };
-- 
2.47.3

[PATCH net v2 3/4] net: pcs: xpcs-rk: fix cleanup action error message

[Coia Prant]

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 <coiaprant@gmail.com>
---
 drivers/net/pcs/pcs-xpcs-rk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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;
 	}
 
-- 
2.47.3

[PATCH net v2 4/4] MAINTAINERS: list DW XPCS platform driver and binding under XPCS entry

[Coia Prant]

The Synopsys DW XPCS entry covers pcs-xpcs.c, pcs-xpcs.h and the public
header, but not pcs-xpcs-plat.c or its DT binding. pcs-xpcs-plat.c only
matches the ETHERNET PHY LIBRARY entry today, and the binding is only
matched by the generic DT and networking rules.

Add both files so the entry covers the whole driver.

Signed-off-by: Coia Prant <coiaprant@gmail.com>
---
 MAINTAINERS | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 7336600952c8..4f24656f7653 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -26729,6 +26729,8 @@ F:	drivers/net/ethernet/synopsys/
 SYNOPSYS DESIGNWARE ETHERNET XPCS DRIVER
 L:	netdev@vger.kernel.org
 S:	Orphan
+F:	Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml
+F:	drivers/net/pcs/pcs-xpcs-plat.c
 F:	drivers/net/pcs/pcs-xpcs.c
 F:	drivers/net/pcs/pcs-xpcs.h
 F:	include/linux/pcs/pcs-xpcs.h
-- 
2.47.3