6.17-stable review patch. If anyone has any objections, please let me know.
------------------
From: Leo Yan leo.yan@arm.com
[ Upstream commit 1abc1b212effe920f4729353880c8e03f1d76b4b ]
Some CoreSight components have programming clocks (pclk) and are enabled using clk_get() and clk_prepare_enable(). However, in many cases, these clocks are not disabled when modules exit and only released by clk_put().
To fix the issue, this commit refactors programming clock by replacing clk_get() and clk_prepare_enable() with devm_clk_get_optional_enabled() for enabling APB clock. If the "apb_pclk" clock is not found, a NULL pointer is returned, and the function proceeds to attempt enabling the "apb" clock.
Since ACPI platforms rely on firmware to manage clocks, returning a NULL pointer in this case leaves clock management to the firmware rather than the driver. This effectively avoids a clock imbalance issue during module removal - where the clock could be disabled twice: once during the ACPI runtime suspend and again during the devm resource release.
Callers are updated to reuse the returned error value.
With the change, programming clocks are managed as resources in driver model layer, allowing clock cleanup to be handled automatically. As a result, manual cleanup operations are no longer needed and are removed from the Coresight drivers.
Fixes: 73d779a03a76 ("coresight: etm4x: Change etm4_platform_driver driver for MMIO devices") Reviewed-by: Yeoreum Yun yeoreum.yun@arm.com Tested-by: James Clark james.clark@linaro.org Signed-off-by: Leo Yan leo.yan@arm.com Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com Link: https://lore.kernel.org/r/20250731-arm_cs_fix_clock_v4-v6-4-1dfe10bb3f6f@arm... Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/hwtracing/coresight/coresight-catu.c | 9 ++------ .../hwtracing/coresight/coresight-cpu-debug.c | 6 +---- .../hwtracing/coresight/coresight-ctcu-core.c | 10 ++------- .../coresight/coresight-etm4x-core.c | 9 ++------ .../hwtracing/coresight/coresight-funnel.c | 6 +---- .../coresight/coresight-replicator.c | 6 +---- drivers/hwtracing/coresight/coresight-stm.c | 4 +--- .../hwtracing/coresight/coresight-tmc-core.c | 4 +--- drivers/hwtracing/coresight/coresight-tpiu.c | 4 +--- include/linux/coresight.h | 22 ++++++++----------- 10 files changed, 21 insertions(+), 59 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-catu.c b/drivers/hwtracing/coresight/coresight-catu.c index af2a55f0c907c..4c345ff2cff14 100644 --- a/drivers/hwtracing/coresight/coresight-catu.c +++ b/drivers/hwtracing/coresight/coresight-catu.c @@ -636,7 +636,7 @@ static int catu_platform_probe(struct platform_device *pdev)
drvdata->pclk = coresight_get_enable_apb_pclk(&pdev->dev); if (IS_ERR(drvdata->pclk)) - return -ENODEV; + return PTR_ERR(drvdata->pclk);
pm_runtime_get_noresume(&pdev->dev); pm_runtime_set_active(&pdev->dev); @@ -645,11 +645,8 @@ static int catu_platform_probe(struct platform_device *pdev) dev_set_drvdata(&pdev->dev, drvdata); ret = __catu_probe(&pdev->dev, res); pm_runtime_put(&pdev->dev); - if (ret) { + if (ret) pm_runtime_disable(&pdev->dev); - if (!IS_ERR_OR_NULL(drvdata->pclk)) - clk_put(drvdata->pclk); - }
return ret; } @@ -663,8 +660,6 @@ static void catu_platform_remove(struct platform_device *pdev)
__catu_remove(&pdev->dev); pm_runtime_disable(&pdev->dev); - if (!IS_ERR_OR_NULL(drvdata->pclk)) - clk_put(drvdata->pclk); }
#ifdef CONFIG_PM diff --git a/drivers/hwtracing/coresight/coresight-cpu-debug.c b/drivers/hwtracing/coresight/coresight-cpu-debug.c index a871d997330b0..e39dfb886688e 100644 --- a/drivers/hwtracing/coresight/coresight-cpu-debug.c +++ b/drivers/hwtracing/coresight/coresight-cpu-debug.c @@ -699,7 +699,7 @@ static int debug_platform_probe(struct platform_device *pdev)
drvdata->pclk = coresight_get_enable_apb_pclk(&pdev->dev); if (IS_ERR(drvdata->pclk)) - return -ENODEV; + return PTR_ERR(drvdata->pclk);
dev_set_drvdata(&pdev->dev, drvdata); pm_runtime_get_noresume(&pdev->dev); @@ -710,8 +710,6 @@ static int debug_platform_probe(struct platform_device *pdev) if (ret) { pm_runtime_put_noidle(&pdev->dev); pm_runtime_disable(&pdev->dev); - if (!IS_ERR_OR_NULL(drvdata->pclk)) - clk_put(drvdata->pclk); } return ret; } @@ -725,8 +723,6 @@ static void debug_platform_remove(struct platform_device *pdev)
__debug_remove(&pdev->dev); pm_runtime_disable(&pdev->dev); - if (!IS_ERR_OR_NULL(drvdata->pclk)) - clk_put(drvdata->pclk); }
#ifdef CONFIG_ACPI diff --git a/drivers/hwtracing/coresight/coresight-ctcu-core.c b/drivers/hwtracing/coresight/coresight-ctcu-core.c index c6bafc96db963..de279efe34058 100644 --- a/drivers/hwtracing/coresight/coresight-ctcu-core.c +++ b/drivers/hwtracing/coresight/coresight-ctcu-core.c @@ -209,7 +209,7 @@ static int ctcu_probe(struct platform_device *pdev)
drvdata->apb_clk = coresight_get_enable_apb_pclk(dev); if (IS_ERR(drvdata->apb_clk)) - return -ENODEV; + return PTR_ERR(drvdata->apb_clk);
cfgs = of_device_get_match_data(dev); if (cfgs) { @@ -233,12 +233,8 @@ static int ctcu_probe(struct platform_device *pdev) desc.access = CSDEV_ACCESS_IOMEM(base);
drvdata->csdev = coresight_register(&desc); - if (IS_ERR(drvdata->csdev)) { - if (!IS_ERR_OR_NULL(drvdata->apb_clk)) - clk_put(drvdata->apb_clk); - + if (IS_ERR(drvdata->csdev)) return PTR_ERR(drvdata->csdev); - }
return 0; } @@ -275,8 +271,6 @@ static void ctcu_platform_remove(struct platform_device *pdev)
ctcu_remove(pdev); pm_runtime_disable(&pdev->dev); - if (!IS_ERR_OR_NULL(drvdata->apb_clk)) - clk_put(drvdata->apb_clk); }
#ifdef CONFIG_PM diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index 81f20a167e001..4b98a7bf4cb73 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -2309,14 +2309,12 @@ static int etm4_probe_platform_dev(struct platform_device *pdev)
drvdata->pclk = coresight_get_enable_apb_pclk(&pdev->dev); if (IS_ERR(drvdata->pclk)) - return -ENODEV; + return PTR_ERR(drvdata->pclk);
if (res) { drvdata->base = devm_ioremap_resource(&pdev->dev, res); - if (IS_ERR(drvdata->base)) { - clk_put(drvdata->pclk); + if (IS_ERR(drvdata->base)) return PTR_ERR(drvdata->base); - } }
dev_set_drvdata(&pdev->dev, drvdata); @@ -2423,9 +2421,6 @@ static void etm4_remove_platform_dev(struct platform_device *pdev) if (drvdata) etm4_remove_dev(drvdata); pm_runtime_disable(&pdev->dev); - - if (drvdata && !IS_ERR_OR_NULL(drvdata->pclk)) - clk_put(drvdata->pclk); }
static const struct amba_id etm4_ids[] = { diff --git a/drivers/hwtracing/coresight/coresight-funnel.c b/drivers/hwtracing/coresight/coresight-funnel.c index b1922dbe9292b..36fc4e991458c 100644 --- a/drivers/hwtracing/coresight/coresight-funnel.c +++ b/drivers/hwtracing/coresight/coresight-funnel.c @@ -240,7 +240,7 @@ static int funnel_probe(struct device *dev, struct resource *res)
drvdata->pclk = coresight_get_enable_apb_pclk(dev); if (IS_ERR(drvdata->pclk)) - return -ENODEV; + return PTR_ERR(drvdata->pclk);
/* * Map the device base for dynamic-funnel, which has been @@ -284,8 +284,6 @@ static int funnel_probe(struct device *dev, struct resource *res) out_disable_clk: if (ret && !IS_ERR_OR_NULL(drvdata->atclk)) clk_disable_unprepare(drvdata->atclk); - if (ret && !IS_ERR_OR_NULL(drvdata->pclk)) - clk_disable_unprepare(drvdata->pclk); return ret; }
@@ -355,8 +353,6 @@ static void funnel_platform_remove(struct platform_device *pdev)
funnel_remove(&pdev->dev); pm_runtime_disable(&pdev->dev); - if (!IS_ERR_OR_NULL(drvdata->pclk)) - clk_put(drvdata->pclk); }
static const struct of_device_id funnel_match[] = { diff --git a/drivers/hwtracing/coresight/coresight-replicator.c b/drivers/hwtracing/coresight/coresight-replicator.c index 06efd2b01a0f7..6dd24eb10a94b 100644 --- a/drivers/hwtracing/coresight/coresight-replicator.c +++ b/drivers/hwtracing/coresight/coresight-replicator.c @@ -247,7 +247,7 @@ static int replicator_probe(struct device *dev, struct resource *res)
drvdata->pclk = coresight_get_enable_apb_pclk(dev); if (IS_ERR(drvdata->pclk)) - return -ENODEV; + return PTR_ERR(drvdata->pclk);
/* * Map the device base for dynamic-replicator, which has been @@ -296,8 +296,6 @@ static int replicator_probe(struct device *dev, struct resource *res) out_disable_clk: if (ret && !IS_ERR_OR_NULL(drvdata->atclk)) clk_disable_unprepare(drvdata->atclk); - if (ret && !IS_ERR_OR_NULL(drvdata->pclk)) - clk_disable_unprepare(drvdata->pclk); return ret; }
@@ -335,8 +333,6 @@ static void replicator_platform_remove(struct platform_device *pdev)
replicator_remove(&pdev->dev); pm_runtime_disable(&pdev->dev); - if (!IS_ERR_OR_NULL(drvdata->pclk)) - clk_put(drvdata->pclk); }
#ifdef CONFIG_PM diff --git a/drivers/hwtracing/coresight/coresight-stm.c b/drivers/hwtracing/coresight/coresight-stm.c index e45c6c7204b44..88ee453b28154 100644 --- a/drivers/hwtracing/coresight/coresight-stm.c +++ b/drivers/hwtracing/coresight/coresight-stm.c @@ -851,7 +851,7 @@ static int __stm_probe(struct device *dev, struct resource *res)
drvdata->pclk = coresight_get_enable_apb_pclk(dev); if (IS_ERR(drvdata->pclk)) - return -ENODEV; + return PTR_ERR(drvdata->pclk); dev_set_drvdata(dev, drvdata);
base = devm_ioremap_resource(dev, res); @@ -1033,8 +1033,6 @@ static void stm_platform_remove(struct platform_device *pdev)
__stm_remove(&pdev->dev); pm_runtime_disable(&pdev->dev); - if (!IS_ERR_OR_NULL(drvdata->pclk)) - clk_put(drvdata->pclk); }
#ifdef CONFIG_ACPI diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c index 0b5e7635a084d..e867198b03e82 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-core.c +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c @@ -991,7 +991,7 @@ static int tmc_platform_probe(struct platform_device *pdev)
drvdata->pclk = coresight_get_enable_apb_pclk(&pdev->dev); if (IS_ERR(drvdata->pclk)) - return -ENODEV; + return PTR_ERR(drvdata->pclk);
dev_set_drvdata(&pdev->dev, drvdata); pm_runtime_get_noresume(&pdev->dev); @@ -1015,8 +1015,6 @@ static void tmc_platform_remove(struct platform_device *pdev)
__tmc_remove(&pdev->dev); pm_runtime_disable(&pdev->dev); - if (!IS_ERR_OR_NULL(drvdata->pclk)) - clk_put(drvdata->pclk); }
#ifdef CONFIG_PM diff --git a/drivers/hwtracing/coresight/coresight-tpiu.c b/drivers/hwtracing/coresight/coresight-tpiu.c index 3e01592884280..b2559c6fac6d2 100644 --- a/drivers/hwtracing/coresight/coresight-tpiu.c +++ b/drivers/hwtracing/coresight/coresight-tpiu.c @@ -153,7 +153,7 @@ static int __tpiu_probe(struct device *dev, struct resource *res)
drvdata->pclk = coresight_get_enable_apb_pclk(dev); if (IS_ERR(drvdata->pclk)) - return -ENODEV; + return PTR_ERR(drvdata->pclk); dev_set_drvdata(dev, drvdata);
/* Validity for the resource is already checked by the AMBA core */ @@ -293,8 +293,6 @@ static void tpiu_platform_remove(struct platform_device *pdev)
__tpiu_remove(&pdev->dev); pm_runtime_disable(&pdev->dev); - if (!IS_ERR_OR_NULL(drvdata->pclk)) - clk_put(drvdata->pclk); }
#ifdef CONFIG_ACPI diff --git a/include/linux/coresight.h b/include/linux/coresight.h index 4ac65c68bbf44..1e652e1578419 100644 --- a/include/linux/coresight.h +++ b/include/linux/coresight.h @@ -6,6 +6,7 @@ #ifndef _LINUX_CORESIGHT_H #define _LINUX_CORESIGHT_H
+#include <linux/acpi.h> #include <linux/amba/bus.h> #include <linux/clk.h> #include <linux/device.h> @@ -480,26 +481,21 @@ static inline bool is_coresight_device(void __iomem *base) * Returns: * * clk - Clock is found and enabled - * NULL - clock is not found + * NULL - Clock is controlled by firmware (ACPI device only) * ERROR - Clock is found but failed to enable */ static inline struct clk *coresight_get_enable_apb_pclk(struct device *dev) { struct clk *pclk; - int ret;
- pclk = clk_get(dev, "apb_pclk"); - if (IS_ERR(pclk)) { - pclk = clk_get(dev, "apb"); - if (IS_ERR(pclk)) - return NULL; - } + /* Firmware controls clocks for an ACPI device. */ + if (has_acpi_companion(dev)) + return NULL; + + pclk = devm_clk_get_optional_enabled(dev, "apb_pclk"); + if (!pclk) + pclk = devm_clk_get_optional_enabled(dev, "apb");
- ret = clk_prepare_enable(pclk); - if (ret) { - clk_put(pclk); - return ERR_PTR(ret); - } return pclk; }