coresight: Appropriately disable programming clocks

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: 73d779a03a ("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.com
This commit is contained in:
Leo Yan 2025-07-31 13:23:40 +01:00 committed by Suzuki K Poulose
parent 40c0cdc9cb
commit 1abc1b212e
10 changed files with 21 additions and 59 deletions

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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[] = {

View File

@ -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[] = {

View File

@ -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

View File

@ -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

View File

@ -981,7 +981,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);
@ -1005,8 +1005,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

View File

@ -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

View File

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