2023-12-01 06:22:09

by Anshuman Khandual

[permalink] [raw]
Subject: [PATCH V2 5/7] coresight: tmc: Move ACPI support from AMBA driver to platform driver

Add support for the tmc devices in the platform driver, which can then be
used on ACPI based platforms. This change would now allow runtime power
management for ACPI based systems. The driver would try to enable the APB
clock if available.

Cc: Lorenzo Pieralisi <[email protected]>
Cc: Sudeep Holla <[email protected]>
Cc: Suzuki K Poulose <[email protected]>
Cc: Mike Leach <[email protected]>
Cc: James Clark <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Anshuman Khandual <[email protected]>
---
drivers/acpi/arm64/amba.c | 2 -
.../hwtracing/coresight/coresight-tmc-core.c | 130 +++++++++++++++---
drivers/hwtracing/coresight/coresight-tmc.h | 1 +
3 files changed, 115 insertions(+), 18 deletions(-)

diff --git a/drivers/acpi/arm64/amba.c b/drivers/acpi/arm64/amba.c
index 6d24a8f7914b..d3c1defa7bc8 100644
--- a/drivers/acpi/arm64/amba.c
+++ b/drivers/acpi/arm64/amba.c
@@ -22,10 +22,8 @@
static const struct acpi_device_id amba_id_list[] = {
{"ARMH0061", 0}, /* PL061 GPIO Device */
{"ARMH0330", 0}, /* ARM DMA Controller DMA-330 */
- {"ARMHC501", 0}, /* ARM CoreSight ETR */
{"ARMHC502", 0}, /* ARM CoreSight STM */
{"ARMHC503", 0}, /* ARM CoreSight Debug */
- {"ARMHC97C", 0}, /* ARM CoreSight SoC-400 TMC, SoC-600 ETF/ETB */
{"", 0},
};

diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c
index c106d142e632..13b6dcb8b390 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-core.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-core.c
@@ -23,6 +23,8 @@
#include <linux/of.h>
#include <linux/coresight.h>
#include <linux/amba/bus.h>
+#include <linux/platform_device.h>
+#include <linux/acpi.h>

#include "coresight-priv.h"
#include "coresight-tmc.h"
@@ -429,24 +431,17 @@ static u32 tmc_etr_get_max_burst_size(struct device *dev)
return burst_size;
}

-static int tmc_probe(struct amba_device *adev, const struct amba_id *id)
+static int __tmc_probe(struct device *dev, struct resource *res, void *dev_caps)
{
int ret = 0;
u32 devid;
void __iomem *base;
- struct device *dev = &adev->dev;
struct coresight_platform_data *pdata = NULL;
- struct tmc_drvdata *drvdata;
- struct resource *res = &adev->res;
+ struct tmc_drvdata *drvdata = dev_get_drvdata(dev);
struct coresight_desc desc = { 0 };
struct coresight_dev_list *dev_list = NULL;

ret = -ENOMEM;
- drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
- if (!drvdata)
- goto out;
-
- dev_set_drvdata(dev, drvdata);

/* Validity for the resource is already checked by the AMBA core */
base = devm_ioremap_resource(dev, res);
@@ -487,8 +482,7 @@ static int tmc_probe(struct amba_device *adev, const struct amba_id *id)
desc.type = CORESIGHT_DEV_TYPE_SINK;
desc.subtype.sink_subtype = CORESIGHT_DEV_SUBTYPE_SINK_SYSMEM;
desc.ops = &tmc_etr_cs_ops;
- ret = tmc_etr_setup_caps(dev, devid,
- coresight_get_uci_data(id));
+ ret = tmc_etr_setup_caps(dev, devid, dev_caps);
if (ret)
goto out;
idr_init(&drvdata->idr);
@@ -519,7 +513,7 @@ static int tmc_probe(struct amba_device *adev, const struct amba_id *id)
ret = PTR_ERR(pdata);
goto out;
}
- adev->dev.platform_data = pdata;
+ dev->platform_data = pdata;
desc.pdata = pdata;

drvdata->csdev = coresight_register(&desc);
@@ -535,11 +529,23 @@ static int tmc_probe(struct amba_device *adev, const struct amba_id *id)
if (ret)
coresight_unregister(drvdata->csdev);
else
- pm_runtime_put(&adev->dev);
+ pm_runtime_put(dev);
out:
return ret;
}

+static int tmc_probe(struct amba_device *adev, const struct amba_id *id)
+{
+ struct tmc_drvdata *drvdata;
+
+ drvdata = devm_kzalloc(&adev->dev, sizeof(*drvdata), GFP_KERNEL);
+ if (!drvdata)
+ return -ENOMEM;
+
+ amba_set_drvdata(adev, drvdata);
+ return __tmc_probe(&adev->dev, &adev->res, coresight_get_uci_data(id));
+}
+
static void tmc_shutdown(struct amba_device *adev)
{
unsigned long flags;
@@ -562,9 +568,9 @@ static void tmc_shutdown(struct amba_device *adev)
spin_unlock_irqrestore(&drvdata->spinlock, flags);
}

-static void tmc_remove(struct amba_device *adev)
+static void __tmc_remove(struct device *dev)
{
- struct tmc_drvdata *drvdata = dev_get_drvdata(&adev->dev);
+ struct tmc_drvdata *drvdata = dev_get_drvdata(dev);

/*
* Since misc_open() holds a refcount on the f_ops, which is
@@ -575,6 +581,11 @@ static void tmc_remove(struct amba_device *adev)
coresight_unregister(drvdata->csdev);
}

+static void tmc_remove(struct amba_device *adev)
+{
+ __tmc_remove(&adev->dev);
+}
+
static const struct amba_id tmc_ids[] = {
CS_AMBA_ID(0x000bb961),
/* Coresight SoC 600 TMC-ETR/ETS */
@@ -600,7 +611,94 @@ static struct amba_driver tmc_driver = {
.id_table = tmc_ids,
};

-module_amba_driver(tmc_driver);
+static int tmc_platform_probe(struct platform_device *pdev)
+{
+ struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ struct tmc_drvdata *drvdata;
+ int ret = 0;
+
+ drvdata = devm_kzalloc(&pdev->dev, sizeof(*drvdata), GFP_KERNEL);
+ if (!drvdata)
+ return -ENOMEM;
+
+ drvdata->pclk = coresight_get_enable_apb_pclk(&pdev->dev);
+ if (IS_ERR(drvdata->pclk))
+ return -ENODEV;
+
+ dev_set_drvdata(&pdev->dev, drvdata);
+ pm_runtime_get_noresume(&pdev->dev);
+ pm_runtime_set_active(&pdev->dev);
+ pm_runtime_enable(&pdev->dev);
+
+ ret = __tmc_probe(&pdev->dev, res, NULL);
+ if (ret) {
+ pm_runtime_put_noidle(&pdev->dev);
+ pm_runtime_disable(&pdev->dev);
+ }
+ return ret;
+}
+
+static int tmc_platform_remove(struct platform_device *pdev)
+{
+ __tmc_remove(&pdev->dev);
+ return 0;
+}
+
+#ifdef CONFIG_PM
+static int tmc_runtime_suspend(struct device *dev)
+{
+ struct tmc_drvdata *drvdata = dev_get_drvdata(dev);
+
+ if (drvdata->pclk && !IS_ERR_OR_NULL(drvdata->pclk))
+ clk_disable_unprepare(drvdata->pclk);
+ return 0;
+}
+
+static int tmc_runtime_resume(struct device *dev)
+{
+ struct tmc_drvdata *drvdata = dev_get_drvdata(dev);
+
+ if (drvdata->pclk && !IS_ERR_OR_NULL(drvdata->pclk))
+ clk_prepare_enable(drvdata->pclk);
+ return 0;
+}
+#endif
+
+static const struct dev_pm_ops tmc_dev_pm_ops = {
+ SET_RUNTIME_PM_OPS(tmc_runtime_suspend, tmc_runtime_resume, NULL)
+};
+
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id tmc_acpi_ids[] = {
+ {"ARMHC501", 0}, /* ARM CoreSight ETR */
+ {"ARMHC97C", 0}, /* ARM CoreSight SoC-400 TMC, SoC-600 ETF/ETB */
+ {},
+};
+MODULE_DEVICE_TABLE(acpi, tmc_acpi_ids);
+#endif
+
+static struct platform_driver tmc_platform_driver = {
+ .probe = tmc_platform_probe,
+ .remove = tmc_platform_remove,
+ .driver = {
+ .name = "coresight-tmc-platform",
+ .acpi_match_table = ACPI_PTR(tmc_acpi_ids),
+ .suppress_bind_attrs = true,
+ .pm = &tmc_dev_pm_ops,
+ },
+};
+
+static int __init tmc_init(void)
+{
+ return coresight_init_driver("tmc", &tmc_driver, &tmc_platform_driver);
+}
+
+static void __exit tmc_exit(void)
+{
+ coresight_remove_driver(&tmc_driver, &tmc_platform_driver);
+}
+module_init(tmc_init);
+module_exit(tmc_exit);

MODULE_AUTHOR("Pratik Patel <[email protected]>");
MODULE_DESCRIPTION("Arm CoreSight Trace Memory Controller driver");
diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
index 0ee48c5ba764..193af5959f2c 100644
--- a/drivers/hwtracing/coresight/coresight-tmc.h
+++ b/drivers/hwtracing/coresight/coresight-tmc.h
@@ -189,6 +189,7 @@ struct etr_buf {
* @perf_buf: PERF buffer for ETR.
*/
struct tmc_drvdata {
+ struct clk *pclk;
void __iomem *base;
struct coresight_device *csdev;
struct miscdevice miscdev;
--
2.25.1


2023-12-01 12:39:04

by Sudeep Holla

[permalink] [raw]
Subject: Re: [PATCH V2 5/7] coresight: tmc: Move ACPI support from AMBA driver to platform driver

On Fri, Dec 01, 2023 at 11:50:51AM +0530, Anshuman Khandual wrote:
> Add support for the tmc devices in the platform driver, which can then be
> used on ACPI based platforms. This change would now allow runtime power
> management for ACPI based systems. The driver would try to enable the APB
> clock if available.
>
> Cc: Lorenzo Pieralisi <[email protected]>
> Cc: Sudeep Holla <[email protected]>

Acked-by: Sudeep Holla <[email protected]> # For ACPI related changes
Tested-by: Sudeep Holla <[email protected]> # Boot and driver probe only

--
Regards,
Sudeep

2023-12-04 10:42:25

by James Clark

[permalink] [raw]
Subject: Re: [PATCH V2 5/7] coresight: tmc: Move ACPI support from AMBA driver to platform driver



On 01/12/2023 06:20, Anshuman Khandual wrote:
> Add support for the tmc devices in the platform driver, which can then be
> used on ACPI based platforms. This change would now allow runtime power
> management for ACPI based systems. The driver would try to enable the APB
> clock if available.
>
> Cc: Lorenzo Pieralisi <[email protected]>
> Cc: Sudeep Holla <[email protected]>
> Cc: Suzuki K Poulose <[email protected]>
> Cc: Mike Leach <[email protected]>
> Cc: James Clark <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Anshuman Khandual <[email protected]>
> ---
> drivers/acpi/arm64/amba.c | 2 -
> .../hwtracing/coresight/coresight-tmc-core.c | 130 +++++++++++++++---
> drivers/hwtracing/coresight/coresight-tmc.h | 1 +
> 3 files changed, 115 insertions(+), 18 deletions(-)
>
[...]
> -static int tmc_probe(struct amba_device *adev, const struct amba_id *id)
> +static int __tmc_probe(struct device *dev, struct resource *res, void *dev_caps)
> {
> int ret = 0;
> u32 devid;
> void __iomem *base;
> - struct device *dev = &adev->dev;
> struct coresight_platform_data *pdata = NULL;
> - struct tmc_drvdata *drvdata;
> - struct resource *res = &adev->res;
> + struct tmc_drvdata *drvdata = dev_get_drvdata(dev);
> struct coresight_desc desc = { 0 };
> struct coresight_dev_list *dev_list = NULL;
>
> ret = -ENOMEM;
> - drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
> - if (!drvdata)
> - goto out;
> -
> - dev_set_drvdata(dev, drvdata);
>
> /* Validity for the resource is already checked by the AMBA core */
> base = devm_ioremap_resource(dev, res);
> @@ -487,8 +482,7 @@ static int tmc_probe(struct amba_device *adev, const struct amba_id *id)
> desc.type = CORESIGHT_DEV_TYPE_SINK;
> desc.subtype.sink_subtype = CORESIGHT_DEV_SUBTYPE_SINK_SYSMEM;
> desc.ops = &tmc_etr_cs_ops;
> - ret = tmc_etr_setup_caps(dev, devid,
> - coresight_get_uci_data(id));
> + ret = tmc_etr_setup_caps(dev, devid, dev_caps);
> if (ret)
> goto out;
[...]
> -module_amba_driver(tmc_driver);
> +static int tmc_platform_probe(struct platform_device *pdev)
> +{
> + struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + struct tmc_drvdata *drvdata;
> + int ret = 0;
> +
> + drvdata = devm_kzalloc(&pdev->dev, sizeof(*drvdata), GFP_KERNEL);
> + if (!drvdata)
> + return -ENOMEM;
> +
> + drvdata->pclk = coresight_get_enable_apb_pclk(&pdev->dev);
> + if (IS_ERR(drvdata->pclk))
> + return -ENODEV;
> +
> + dev_set_drvdata(&pdev->dev, drvdata);
> + pm_runtime_get_noresume(&pdev->dev);
> + pm_runtime_set_active(&pdev->dev);
> + pm_runtime_enable(&pdev->dev);
> +
> + ret = __tmc_probe(&pdev->dev, res, NULL);

What effect does passing NULL here have? It seems like the dev_caps did
something useful when it's set to CORESIGHT_SOC_600_ETR_CAPS. Maybe
Suzuki knows as he added it?

2023-12-04 10:51:10

by Suzuki K Poulose

[permalink] [raw]
Subject: Re: [PATCH V2 5/7] coresight: tmc: Move ACPI support from AMBA driver to platform driver

On 04/12/2023 10:42, James Clark wrote:
>
>
> On 01/12/2023 06:20, Anshuman Khandual wrote:
>> Add support for the tmc devices in the platform driver, which can then be
>> used on ACPI based platforms. This change would now allow runtime power
>> management for ACPI based systems. The driver would try to enable the APB
>> clock if available.
>>
>> Cc: Lorenzo Pieralisi <[email protected]>
>> Cc: Sudeep Holla <[email protected]>
>> Cc: Suzuki K Poulose <[email protected]>
>> Cc: Mike Leach <[email protected]>
>> Cc: James Clark <[email protected]>
>> Cc: [email protected]
>> Cc: [email protected]
>> Cc: [email protected]
>> Cc: [email protected]
>> Signed-off-by: Anshuman Khandual <[email protected]>
>> ---
>> drivers/acpi/arm64/amba.c | 2 -
>> .../hwtracing/coresight/coresight-tmc-core.c | 130 +++++++++++++++---
>> drivers/hwtracing/coresight/coresight-tmc.h | 1 +
>> 3 files changed, 115 insertions(+), 18 deletions(-)
>>
> [...]
>> -static int tmc_probe(struct amba_device *adev, const struct amba_id *id)
>> +static int __tmc_probe(struct device *dev, struct resource *res, void *dev_caps)
>> {
>> int ret = 0;
>> u32 devid;
>> void __iomem *base;
>> - struct device *dev = &adev->dev;
>> struct coresight_platform_data *pdata = NULL;
>> - struct tmc_drvdata *drvdata;
>> - struct resource *res = &adev->res;
>> + struct tmc_drvdata *drvdata = dev_get_drvdata(dev);
>> struct coresight_desc desc = { 0 };
>> struct coresight_dev_list *dev_list = NULL;
>>
>> ret = -ENOMEM;
>> - drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
>> - if (!drvdata)
>> - goto out;
>> -
>> - dev_set_drvdata(dev, drvdata);
>>
>> /* Validity for the resource is already checked by the AMBA core */
>> base = devm_ioremap_resource(dev, res);
>> @@ -487,8 +482,7 @@ static int tmc_probe(struct amba_device *adev, const struct amba_id *id)
>> desc.type = CORESIGHT_DEV_TYPE_SINK;
>> desc.subtype.sink_subtype = CORESIGHT_DEV_SUBTYPE_SINK_SYSMEM;
>> desc.ops = &tmc_etr_cs_ops;
>> - ret = tmc_etr_setup_caps(dev, devid,
>> - coresight_get_uci_data(id));
>> + ret = tmc_etr_setup_caps(dev, devid, dev_caps);
>> if (ret)
>> goto out;
> [...]
>> -module_amba_driver(tmc_driver);
>> +static int tmc_platform_probe(struct platform_device *pdev)
>> +{
>> + struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + struct tmc_drvdata *drvdata;
>> + int ret = 0;
>> +
>> + drvdata = devm_kzalloc(&pdev->dev, sizeof(*drvdata), GFP_KERNEL);
>> + if (!drvdata)
>> + return -ENOMEM;
>> +
>> + drvdata->pclk = coresight_get_enable_apb_pclk(&pdev->dev);
>> + if (IS_ERR(drvdata->pclk))
>> + return -ENODEV;
>> +
>> + dev_set_drvdata(&pdev->dev, drvdata);
>> + pm_runtime_get_noresume(&pdev->dev);
>> + pm_runtime_set_active(&pdev->dev);
>> + pm_runtime_enable(&pdev->dev);
>> +
>> + ret = __tmc_probe(&pdev->dev, res, NULL);
>
> What effect does passing NULL here have? It seems like the dev_caps did
> something useful when it's set to CORESIGHT_SOC_600_ETR_CAPS. Maybe
> Suzuki knows as he added it?

That was supposed to initialise the capabilities of the TMC-ETR, which
are not discoverable by ID registers. (e.g., SAVE restore of RRP/RWP).
I guess, we could get rid of that cap and use the "PID" register match
to figure out the capabilities.

Suzuki


2023-12-04 10:54:28

by James Clark

[permalink] [raw]
Subject: Re: [PATCH V2 5/7] coresight: tmc: Move ACPI support from AMBA driver to platform driver



On 01/12/2023 06:20, Anshuman Khandual wrote:
> Add support for the tmc devices in the platform driver, which can then be
> used on ACPI based platforms. This change would now allow runtime power
> management for ACPI based systems. The driver would try to enable the APB
> clock if available.
>
[...]
> -module_amba_driver(tmc_driver);
> +static int tmc_platform_probe(struct platform_device *pdev)
> +{
> + struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + struct tmc_drvdata *drvdata;
> + int ret = 0;
> +
> + drvdata = devm_kzalloc(&pdev->dev, sizeof(*drvdata), GFP_KERNEL);
> + if (!drvdata)
> + return -ENOMEM;
> +
> + drvdata->pclk = coresight_get_enable_apb_pclk(&pdev->dev);
> + if (IS_ERR(drvdata->pclk))
> + return -ENODEV;
> +
> + dev_set_drvdata(&pdev->dev, drvdata);
> + pm_runtime_get_noresume(&pdev->dev);
> + pm_runtime_set_active(&pdev->dev);
> + pm_runtime_enable(&pdev->dev);
> +
> + ret = __tmc_probe(&pdev->dev, res, NULL);
> + if (ret) {
> + pm_runtime_put_noidle(&pdev->dev);
> + pm_runtime_disable(&pdev->dev);
> + }

I'm not sure if these pm_runtime()s are right because there is already a
put inside of __tmc_probe() if it fails. If you unload and then reload
all the coresight modules with these patches you get these errors which
are new:

coresight-tpiu-platform ARMHC979:00: Unbalanced pm_runtime_enable!
CSCFG registered etm0
coresight etm0: CPU0: etm v4.2 initialized
CSCFG registered etm1
coresight etm1: CPU1: etm v4.2 initialized
CSCFG registered etm2
coresight etm2: CPU2: etm v4.2 initialized
CSCFG registered etm3
coresight etm3: CPU3: etm v4.2 initialized
coresight-tmc-platform ARMHC97C:00: Unbalanced pm_runtime_enable!
coresight-tmc-platform ARMHC97C:01: Unbalanced pm_runtime_enable!
coresight-tmc-platform ARMHC97C:02: Unbalanced pm_runtime_enable!
coresight-tmc-platform ARMHC97C:03: Unbalanced pm_runtime_enable!

It might be worth testing all of these pm_runtime()s, including the
error case ones, because loading and unloading the modules doesn't even
include the error scenarios, so there are probably more bad ones in
there too.

2023-12-04 11:41:43

by Sudeep Holla

[permalink] [raw]
Subject: Re: [PATCH V2 5/7] coresight: tmc: Move ACPI support from AMBA driver to platform driver

On Mon, Dec 04, 2023 at 10:54:15AM +0000, James Clark wrote:
>
>
> On 01/12/2023 06:20, Anshuman Khandual wrote:
> > Add support for the tmc devices in the platform driver, which can then be
> > used on ACPI based platforms. This change would now allow runtime power
> > management for ACPI based systems. The driver would try to enable the APB
> > clock if available.
> >
> [...]
> > -module_amba_driver(tmc_driver);
> > +static int tmc_platform_probe(struct platform_device *pdev)
> > +{
> > + struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + struct tmc_drvdata *drvdata;
> > + int ret = 0;
> > +
> > + drvdata = devm_kzalloc(&pdev->dev, sizeof(*drvdata), GFP_KERNEL);
> > + if (!drvdata)
> > + return -ENOMEM;
> > +
> > + drvdata->pclk = coresight_get_enable_apb_pclk(&pdev->dev);
> > + if (IS_ERR(drvdata->pclk))
> > + return -ENODEV;
> > +
> > + dev_set_drvdata(&pdev->dev, drvdata);
> > + pm_runtime_get_noresume(&pdev->dev);
> > + pm_runtime_set_active(&pdev->dev);
> > + pm_runtime_enable(&pdev->dev);
> > +
> > + ret = __tmc_probe(&pdev->dev, res, NULL);
> > + if (ret) {
> > + pm_runtime_put_noidle(&pdev->dev);
> > + pm_runtime_disable(&pdev->dev);
> > + }
>
> I'm not sure if these pm_runtime()s are right because there is already a
> put inside of __tmc_probe() if it fails. If you unload and then reload
> all the coresight modules with these patches you get these errors which
> are new:
>

Ah good find. I have just tested with ACPI and these modules as built-in
to be more specific.

--
Regards,
Sudeep

2023-12-05 03:57:12

by Anshuman Khandual

[permalink] [raw]
Subject: Re: [PATCH V2 5/7] coresight: tmc: Move ACPI support from AMBA driver to platform driver

On 12/4/23 16:24, James Clark wrote:
>
>
> On 01/12/2023 06:20, Anshuman Khandual wrote:
>> Add support for the tmc devices in the platform driver, which can then be
>> used on ACPI based platforms. This change would now allow runtime power
>> management for ACPI based systems. The driver would try to enable the APB
>> clock if available.
>>
> [...]
>> -module_amba_driver(tmc_driver);
>> +static int tmc_platform_probe(struct platform_device *pdev)
>> +{
>> + struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + struct tmc_drvdata *drvdata;
>> + int ret = 0;
>> +
>> + drvdata = devm_kzalloc(&pdev->dev, sizeof(*drvdata), GFP_KERNEL);
>> + if (!drvdata)
>> + return -ENOMEM;
>> +
>> + drvdata->pclk = coresight_get_enable_apb_pclk(&pdev->dev);
>> + if (IS_ERR(drvdata->pclk))
>> + return -ENODEV;
>> +
>> + dev_set_drvdata(&pdev->dev, drvdata);
>> + pm_runtime_get_noresume(&pdev->dev);
>> + pm_runtime_set_active(&pdev->dev);
>> + pm_runtime_enable(&pdev->dev);
>> +
>> + ret = __tmc_probe(&pdev->dev, res, NULL);
>> + if (ret) {
>> + pm_runtime_put_noidle(&pdev->dev);
>> + pm_runtime_disable(&pdev->dev);
>> + }
>
> I'm not sure if these pm_runtime()s are right because there is already a
> put inside of __tmc_probe() if it fails. If you unload and then reload

Actually there is a pm_runtime_put() on the success path, not when it
fails. So pm_runtime_put() gets called when __tmc_probe() returns 0.

__tmc_probe()
{
....
ret = misc_register(&drvdata->miscdev);
if (ret)
coresight_unregister(drvdata->csdev);
else
pm_runtime_put(dev);
out:
return ret;
}

tmc_platform_probe()
{
....
pm_runtime_get_noresume(&pdev->dev);
pm_runtime_set_active(&pdev->dev);
pm_runtime_enable(&pdev->dev);

ret = __tmc_probe(&pdev->dev, res, NULL);
if (ret) {
pm_runtime_put_noidle(&pdev->dev);
pm_runtime_disable(&pdev->dev);
}
return ret;
}

tmc_probe()
{
....
return __tmc_probe(&adev->dev, &adev->res, coresight_get_uci_data(id));
}

Currently pm_runtime_put() gets called

- In success path both for AMBA and platform drivers
- In error path only for platform driver

Although the problem might be with pm_runtime_disable() instead

- pm_runtime_disable() is not required in the platform driver probe() path
- But might be required in tmc_platform_remove() along with a clk_put()

> all the coresight modules with these patches you get these errors which
> are new:
>
> coresight-tpiu-platform ARMHC979:00: Unbalanced pm_runtime_enable!

The code is similar in TPIU platform driver as well.

> CSCFG registered etm0
> coresight etm0: CPU0: etm v4.2 initialized
> CSCFG registered etm1
> coresight etm1: CPU1: etm v4.2 initialized
> CSCFG registered etm2
> coresight etm2: CPU2: etm v4.2 initialized
> CSCFG registered etm3
> coresight etm3: CPU3: etm v4.2 initialized
> coresight-tmc-platform ARMHC97C:00: Unbalanced pm_runtime_enable!
> coresight-tmc-platform ARMHC97C:01: Unbalanced pm_runtime_enable!
> coresight-tmc-platform ARMHC97C:02: Unbalanced pm_runtime_enable!
> coresight-tmc-platform ARMHC97C:03: Unbalanced pm_runtime_enable!
>
> It might be worth testing all of these pm_runtime()s, including the
> error case ones, because loading and unloading the modules doesn't even
> include the error scenarios, so there are probably more bad ones in
> there too.
The code is very similar in CATU, STM as well but debug_platform_remove()
seems to be doing this right.

I am not very familiar with all the power management aspects in coresight,
please do let me know if I missing something here.