'struct etm4_drvdata' itself can carry the base address before etm4_probe() gets called. Just drop that redundant argument from etm4_probe().

Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Suzuki K Poulose suzuki.poulose@arm.com Cc: Mike Leach mike.leach@linaro.org Cc: Leo Yan leo.yan@linaro.org Cc: coresight@lists.linaro.org Cc: linux-arm-kernel@lists.infradead.org Cc: linux-kernel@vger.kernel.org Reviewed-by: James Clark james.clark@arm.com Signed-off-by: Anshuman Khandual anshuman.khandual@arm.com --- drivers/hwtracing/coresight/coresight-etm4x-core.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index 7e96293c638f..748d2ef53cb0 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -2044,7 +2044,7 @@ static int etm4_add_coresight_dev(struct etm4_init_arg *init_arg) return 0; }

-static int etm4_probe(struct device *dev, void __iomem *base, u32 etm_pid) +static int etm4_probe(struct device *dev, u32 etm_pid) { struct etmv4_drvdata *drvdata = dev_get_drvdata(dev); struct csdev_access access = { 0 }; @@ -2065,8 +2065,6 @@ static int etm4_probe(struct device *dev, void __iomem *base, u32 etm_pid) return -ENOMEM; }

- drvdata->base = base; - spin_lock_init(&drvdata->spinlock);

drvdata->cpu = coresight_get_cpu(dev); @@ -2120,8 +2118,9 @@ static int etm4_probe_amba(struct amba_device *adev, const struct amba_id *id) if (!drvdata) return -ENOMEM;

+ drvdata->base = base; dev_set_drvdata(dev, drvdata); - ret = etm4_probe(dev, base, id->id); + ret = etm4_probe(dev, id->id); if (!ret) pm_runtime_put(&adev->dev);

@@ -2137,6 +2136,7 @@ static int etm4_probe_platform_dev(struct platform_device *pdev) if (!drvdata) return -ENOMEM;

+ drvdata->base = NULL; dev_set_drvdata(&pdev->dev, drvdata); pm_runtime_get_noresume(&pdev->dev); pm_runtime_set_active(&pdev->dev); @@ -2147,7 +2147,7 @@ static int etm4_probe_platform_dev(struct platform_device *pdev) * HW by reading appropriate registers on the HW * and thus we could skip the PID. */ - ret = etm4_probe(&pdev->dev, NULL, 0); + ret = etm4_probe(&pdev->dev, 0);

pm_runtime_put(&pdev->dev); return ret;

Add support for handling MMIO based devices via platform driver. We need to make sure that :

1) The APB clock, if present is enabled at probe and via runtime_pm ops 2) Use the ETM4x architecture or CoreSight architecture registers to identify a device as CoreSight ETM4x, instead of relying a white list of "Peripheral IDs"

The driver doesn't get to handle the devices yet, until we wire the ACPI changes to move the devices to be handled via platform driver than the etm4_amba driver.

Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Suzuki K Poulose suzuki.poulose@arm.com Cc: Mike Leach mike.leach@linaro.org Cc: Leo Yan leo.yan@linaro.org Cc: coresight@lists.linaro.org Cc: linux-arm-kernel@lists.infradead.org Cc: linux-kernel@vger.kernel.org Acked-by: Sudeep Holla sudeep.holla@arm.com Signed-off-by: Anshuman Khandual anshuman.khandual@arm.com --- .../coresight/coresight-etm4x-core.c | 57 ++++++++++++++++++- drivers/hwtracing/coresight/coresight-etm4x.h | 4 ++ include/linux/coresight.h | 47 +++++++++++++++ 3 files changed, 106 insertions(+), 2 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index 57a7181017bd..e10f6676dd9b 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -30,6 +30,7 @@ #include <linux/platform_device.h> #include <linux/pm_runtime.h> #include <linux/property.h> +#include <linux/clk/clk-conf.h>

#include <asm/barrier.h> #include <asm/sections.h> @@ -1073,11 +1074,21 @@ static bool etm4_init_sysreg_access(struct etmv4_drvdata *drvdata, return true; }

+static bool is_devtype_cpu_trace(void __iomem *base) +{ + u32 devtype = readl(base + TRCDEVTYPE); + + return (devtype == CS_DEVTYPE_PE_TRACE); +} + static bool etm4_init_iomem_access(struct etmv4_drvdata *drvdata, struct csdev_access *csa) { u32 devarch = readl_relaxed(drvdata->base + TRCDEVARCH);

+ if (!is_coresight_device(drvdata->base) || !is_devtype_cpu_trace(drvdata->base)) + return false; + /* * All ETMs must implement TRCDEVARCH to indicate that * the component is an ETMv4. Even though TRCIDR1 also @@ -2135,6 +2146,7 @@ static int etm4_probe_amba(struct amba_device *adev, const struct amba_id *id)

static int etm4_probe_platform_dev(struct platform_device *pdev) { + struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0); struct etmv4_drvdata *drvdata; int ret;

@@ -2142,7 +2154,18 @@ static int etm4_probe_platform_dev(struct platform_device *pdev) if (!drvdata) return -ENOMEM;

- drvdata->base = NULL; + drvdata->pclk = coresight_get_enable_apb_pclk(&pdev->dev); + if (IS_ERR(drvdata->pclk)) + return -ENODEV; + + if (res) { + drvdata->base = devm_ioremap_resource(&pdev->dev, res); + if (IS_ERR(drvdata->base)) { + clk_put(drvdata->pclk); + return PTR_ERR(drvdata->base); + } + } + dev_set_drvdata(&pdev->dev, drvdata); pm_runtime_get_noresume(&pdev->dev); pm_runtime_set_active(&pdev->dev); @@ -2188,7 +2211,7 @@ static struct amba_cs_uci_id uci_id_etm4[] = { /* ETMv4 UCI data */ .devarch = ETM_DEVARCH_ETMv4x_ARCH, .devarch_mask = ETM_DEVARCH_ID_MASK, - .devtype = 0x00000013, + .devtype = CS_DEVTYPE_PE_TRACE, } };

@@ -2247,6 +2270,9 @@ static int __exit etm4_remove_platform_dev(struct platform_device *pdev) if (drvdata) ret = etm4_remove_dev(drvdata); pm_runtime_disable(&pdev->dev); + + if (drvdata->pclk) + clk_put(drvdata->pclk); return ret; }

@@ -2286,6 +2312,32 @@ static struct amba_driver etm4x_amba_driver = { .id_table = etm4_ids, };

+#ifdef CONFIG_PM +static int etm4_runtime_suspend(struct device *dev) +{ + struct etmv4_drvdata *drvdata = dev_get_drvdata(dev); + + if (drvdata->pclk && !IS_ERR(drvdata->pclk)) + clk_disable_unprepare(drvdata->pclk); + + return 0; +} + +static int etm4_runtime_resume(struct device *dev) +{ + struct etmv4_drvdata *drvdata = dev_get_drvdata(dev); + + if (drvdata->pclk && !IS_ERR(drvdata->pclk)) + clk_prepare_enable(drvdata->pclk); + + return 0; +} +#endif + +static const struct dev_pm_ops etm4_dev_pm_ops = { + SET_RUNTIME_PM_OPS(etm4_runtime_suspend, etm4_runtime_resume, NULL) +}; + static const struct of_device_id etm4_sysreg_match[] = { { .compatible = "arm,coresight-etm4x-sysreg" }, { .compatible = "arm,embedded-trace-extension" }, @@ -2299,6 +2351,7 @@ static struct platform_driver etm4_platform_driver = { .name = "coresight-etm4x", .of_match_table = etm4_sysreg_match, .suppress_bind_attrs = true, + .pm = &etm4_dev_pm_ops, }, };

diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h index 27c8a9901868..20e2e4cb7614 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x.h +++ b/drivers/hwtracing/coresight/coresight-etm4x.h @@ -701,6 +701,8 @@ #define ETM_DEVARCH_ETE_ARCH \ (ETM_DEVARCH_ARCHITECT_ARM | ETM_DEVARCH_ARCHID_ETE | ETM_DEVARCH_PRESENT)

+#define CS_DEVTYPE_PE_TRACE 0x00000013 + #define TRCSTATR_IDLE_BIT 0 #define TRCSTATR_PMSTABLE_BIT 1 #define ETM_DEFAULT_ADDR_COMP 0 @@ -944,6 +946,7 @@ struct etmv4_save_state {

/** * struct etm4_drvdata - specifics associated to an ETM component + * @pclk APB clock if present, otherwise NULL * @base: Memory mapped base address for this component. * @csdev: Component vitals needed by the framework. * @spinlock: Only one at a time pls. @@ -1009,6 +1012,7 @@ struct etmv4_save_state { * @arch_features: Bitmap of arch features of etmv4 devices. */ struct etmv4_drvdata { + struct clk *pclk; void __iomem *base; struct coresight_device *csdev; spinlock_t spinlock; diff --git a/include/linux/coresight.h b/include/linux/coresight.h index f85b041ea475..be60a8b84c49 100644 --- a/include/linux/coresight.h +++ b/include/linux/coresight.h @@ -6,6 +6,8 @@ #ifndef _LINUX_CORESIGHT_H #define _LINUX_CORESIGHT_H

+#include <linux/amba/bus.h> +#include <linux/clk.h> #include <linux/device.h> #include <linux/io.h> #include <linux/perf_event.h> @@ -370,6 +372,51 @@ static inline u32 csdev_access_relaxed_read32(struct csdev_access *csa, return csa->read(offset, true, false); }

+#define CORESIGHT_CIDRn(i) (0xFF0 + ((i) * 4)) + +static inline u32 coresight_get_cid(void __iomem *base) +{ + u32 i, cid = 0; + + for (i = 0; i < 4; i++) + cid |= readl(base + CORESIGHT_CIDRn(i)) << (i * 8); + + return cid; +} + +static inline bool is_coresight_device(void __iomem *base) +{ + u32 cid = coresight_get_cid(base); + + return cid == CORESIGHT_CID; +} + +/* + * Attempt to find and enable "APB clock" for the given device + * + * Returns: + * + * clk - Clock is found and enabled + * NULL - clock is not found + * 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)) + return NULL; + + ret = clk_prepare_enable(pclk); + if (ret) { + clk_put(pclk); + return ERR_PTR(ret); + } + return pclk; +} + #define CORESIGHT_PIDRn(i) (0xFE0 + ((i) * 4))

static inline u32 coresight_get_pid(struct csdev_access *csa)

From: Suzuki K Poulose suzuki.poulose@arm.com

Drop ETM4X ACPI ID from the AMBA ACPI device list, and instead just move it inside the new ACPI devices list detected and used via platform driver.

Cc: "Rafael J. Wysocki" rafael@kernel.org Cc: Len Brown lenb@kernel.org Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Suzuki K Poulose suzuki.poulose@arm.com Cc: Mike Leach mike.leach@linaro.org Cc: Leo Yan leo.yan@linaro.org Cc: Sudeep Holla sudeep.holla@arm.com Cc: Lorenzo Pieralisi lpieralisi@kernel.org Cc: linux-acpi@vger.kernel.org Cc: coresight@lists.linaro.org Cc: linux-arm-kernel@lists.infradead.org Cc: linux-kernel@vger.kernel.org Reviewed-by: Sudeep Holla sudeep.holla@arm.com (for ACPI specific changes) Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com Signed-off-by: Anshuman Khandual anshuman.khandual@arm.com --- drivers/acpi/acpi_amba.c | 1 - drivers/hwtracing/coresight/coresight-etm4x-core.c | 10 ++++++++++ 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/acpi_amba.c b/drivers/acpi/acpi_amba.c index f5b443ab01c2..099966cbac5a 100644 --- a/drivers/acpi/acpi_amba.c +++ b/drivers/acpi/acpi_amba.c @@ -22,7 +22,6 @@ static const struct acpi_device_id amba_id_list[] = { {"ARMH0061", 0}, /* PL061 GPIO Device */ {"ARMH0330", 0}, /* ARM DMA Controller DMA-330 */ - {"ARMHC500", 0}, /* ARM CoreSight ETM4x */ {"ARMHC501", 0}, /* ARM CoreSight ETR */ {"ARMHC502", 0}, /* ARM CoreSight STM */ {"ARMHC503", 0}, /* ARM CoreSight Debug */ diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index e10f6676dd9b..fd6f9dff5881 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -3,6 +3,7 @@ * Copyright (c) 2014, The Linux Foundation. All rights reserved. */

+#include <linux/acpi.h> #include <linux/bitops.h> #include <linux/kernel.h> #include <linux/moduleparam.h> @@ -2344,12 +2345,21 @@ static const struct of_device_id etm4_sysreg_match[] = { {} };

+#ifdef CONFIG_ACPI +static const struct acpi_device_id etm4x_acpi_ids[] = { + {"ARMHC500", 0}, /* ARM CoreSight ETM4x */ + {} +}; +MODULE_DEVICE_TABLE(acpi, etm4x_acpi_ids); +#endif + static struct platform_driver etm4_platform_driver = { .probe = etm4_probe_platform_dev, .remove = etm4_remove_platform_dev, .driver = { .name = "coresight-etm4x", .of_match_table = etm4_sysreg_match, + .acpi_match_table = ACPI_PTR(etm4x_acpi_ids), .suppress_bind_attrs = true, .pm = &etm4_dev_pm_ops, },

HI Ansuman,

On Mon, 29 May 2023 at 07:26, Anshuman Khandual anshuman.khandual@arm.com wrote:

From: Suzuki K Poulose suzuki.poulose@arm.com

Some components may not have graph connections for describing the trace path. e.g., ETE, where it could directly use the per CPU TRBE. Ignore the absence of graph connections

Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com Signed-off-by: Anshuman Khandual anshuman.khandual@arm.com

---

drivers/hwtracing/coresight/coresight-platform.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/hwtracing/coresight/coresight-platform.c b/drivers/hwtracing/coresight/coresight-platform.c index 475899714104..c4b4fbde8550 100644 --- a/drivers/hwtracing/coresight/coresight-platform.c +++ b/drivers/hwtracing/coresight/coresight-platform.c @@ -692,8 +692,12 @@ static int acpi_coresight_parse_graph(struct acpi_device *adev,

    pdata->nr_inport = pdata->nr_outport = 0;
    graph = acpi_get_coresight_graph(adev);

•   /*
  

•    * There are no graph connections, which is fine for some components.
  

•    * e.g., ETE
  

•    */
    if (!graph)
  

•           return -ENOENT;
  

•           return 0;
  
  

Should we not determine if it is valid for a particular component not to have a graph connection? Prior to this patch an incorrectly configured ETMv4 - which must have a graph would return an error, after this it will fail silently

Mike

    nlinks = graph->package.elements[2].integer.value;
    if (!nlinks)

-- 2.25.1

Hi Greg,

On 29/05/2023 07:25, Anshuman Khandual wrote:

From: Suzuki K Poulose suzuki.poulose@arm.com

Drop ETM4X ACPI ID from the AMBA ACPI device list, and instead just move it inside the new ACPI devices list detected and used via platform driver.

Cc: "Rafael J. Wysocki" rafael@kernel.org Cc: Len Brown lenb@kernel.org Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Suzuki K Poulose suzuki.poulose@arm.com Cc: Mike Leach mike.leach@linaro.org Cc: Leo Yan leo.yan@linaro.org Cc: Sudeep Holla sudeep.holla@arm.com Cc: Lorenzo Pieralisi lpieralisi@kernel.org Cc: linux-acpi@vger.kernel.org Cc: coresight@lists.linaro.org Cc: linux-arm-kernel@lists.infradead.org Cc: linux-kernel@vger.kernel.org Reviewed-by: Sudeep Holla sudeep.holla@arm.com (for ACPI specific changes) Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com Signed-off-by: Anshuman Khandual anshuman.khandual@arm.com

---

drivers/acpi/acpi_amba.c | 1 - drivers/hwtracing/coresight/coresight-etm4x-core.c | 10 ++++++++++ 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/acpi_amba.c b/drivers/acpi/acpi_amba.c index f5b443ab01c2..099966cbac5a 100644 --- a/drivers/acpi/acpi_amba.c +++ b/drivers/acpi/acpi_amba.c @@ -22,7 +22,6 @@ static const struct acpi_device_id amba_id_list[] = { {"ARMH0061", 0}, /* PL061 GPIO Device */ {"ARMH0330", 0}, /* ARM DMA Controller DMA-330 */

• {"ARMHC500", 0}, /* ARM CoreSight ETM4x */ {"ARMHC501", 0}, /* ARM CoreSight ETR */ {"ARMHC502", 0}, /* ARM CoreSight STM */ {"ARMHC503", 0}, /* ARM CoreSight Debug */

This is a bit awkward request.

I would like to get your opinion on merging this to coresight tree. This change is removing the coresight ETMv4 from the ACPI AMBA scan list and moving it to the coresight driver. This change is essential for 1) Adding ACPI support for later versions of ETMv4 that are not AMBA devices. 2) Adding power management support for AMBA ETMv4 with ACPI.

The above change has been reviewed by Sudeep (Arm64 ACPI reviewer), but hasn't been Acked by the ACPI maintainer (Rafael) even after a month of follow up with at least 4 reminders [0].

Are you happy with the Reviews from Sudeep and given the minimal change to the drivers/acpi/acpi_amba.c file ?

Kind regards Suzuki

[0] https://lkml.kernel.org/r/0327c5b6-9b6b-460a-dc7e-3a891485a9bd@arm.com

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index e10f6676dd9b..fd6f9dff5881 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -3,6 +3,7 @@

• Copyright (c) 2014, The Linux Foundation. All rights reserved.

*/ +#include <linux/acpi.h> #include <linux/bitops.h> #include <linux/kernel.h> #include <linux/moduleparam.h> @@ -2344,12 +2345,21 @@ static const struct of_device_id etm4_sysreg_match[] = { {} }; +#ifdef CONFIG_ACPI +static const struct acpi_device_id etm4x_acpi_ids[] = {

• {"ARMHC500", 0}, /* ARM CoreSight ETM4x */
• {}

+}; +MODULE_DEVICE_TABLE(acpi, etm4x_acpi_ids); +#endif

• static struct platform_driver etm4_platform_driver = { .probe = etm4_probe_platform_dev, .remove = etm4_remove_platform_dev, .driver = { .name = "coresight-etm4x", .of_match_table = etm4_sysreg_match,

• .acpi_match_table	= ACPI_PTR(etm4x_acpi_ids),
  

  .suppress_bind_attrs = true, .pm = &etm4_dev_pm_ops, },