CoreSight ETM4x devices could be accessed either via MMIO (handled via amba_driver) or CPU system instructions (handled via platform driver). But this has the following issues :
- Each new CPU comes up with its own PID and thus we need to keep on adding the "known" PIDs to get it working with AMBA driver. While the ETM4 architecture (and CoreSight architecture) defines way to identify a device as ETM4. Thus older kernels won't be able to "discover" a newer CPU, unless we add the PIDs.
- With ACPI, the ETM4x devices have the same HID to identify the device irrespective of the mode of access. This creates a problem where two different drivers (both AMBA based driver and platform driver) would hook into the "HID" and could conflict. e.g., if AMBA driver gets hold of a non-MMIO device, the probe fails. If we have single driver hooked into the given "HID", we could handle them seamlessly, irrespective of the mode of access.
- CoreSight is heavily dependent on the runtime power management. With ACPI, amba_driver doesn't get us anywhere with handling the power and thus one need to always turn the power ON to use them. Moving to platform driver gives us the power management for free.
Due to all of the above, we are moving the MMIO based etm4x devices to be supported via platform driver. The series makes the existing platform driver generic to handle both type of the access modes. With that we can also remove the etm4x amba driver.
Finally, we need a way to make sure the new driver gets control of the ETM4x device on a DT based system. CoreSight devices have always had the "arm,primecell" in the compatible list. But the way this is handled currently in OF code is a bit messy. The ETM4x devices are identified by "arm,coresight-etm4x". The platform driver can never get a chance to probe these devices, since the "arm,primecell" takes priority and is hard-coded in the OF code. We have two options here :
1) Remove the arm,primecell from all DTS. This is fine for "new" kernels with this change. But, for existing boards, using an older kernel will break. Thus, is not preferred.
2) Add a white list of "compatibles" where the "priority" of the "arm,primecell" can be ignored.
The series implements (2) above and applies on 6.3-rc2.
Cc: Steve Clevenger scclevenger@os.amperecomputing.com Cc: Rob Herring robh+dt@kernel.org Cc: Frank Rowand frowand.list@gmail.com Cc: Russell King (Oracle) linux@armlinux.org.uk Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org Cc: "Rafael J. Wysocki" rafael@kernel.org Cc: Len Brown lenb@kernel.org Cc: Sudeep Holla sudeep.holla@arm.com Cc: Lorenzo Pieralisi lpieralisi@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: devicetree@vger.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
Anshuman Khandual (6): coresight: etm4x: Allocate and device assign 'struct etmv4_drvdata' earlier coresight: etm4x: Drop iomem 'base' argument from etm4_probe() coresight: etm4x: Drop pid argument from etm4_probe() coresight: etm4x: Change etm4_platform_driver driver for MMIO devices of/platform: Skip coresight etm4x devices from AMBA bus coresight: etm4x: Drop the AMBA driver
Suzuki Poulose (1): coresight: etm4x: Add ACPI support in platform driver
drivers/acpi/acpi_amba.c | 1 - .../coresight/coresight-etm4x-core.c | 171 ++++++++---------- drivers/hwtracing/coresight/coresight-etm4x.h | 3 + drivers/of/platform.c | 10 +- include/linux/coresight.h | 56 ++++++ 5 files changed, 143 insertions(+), 98 deletions(-)
Allocate and device assign 'struct etmv4_drvdata' earlier during the driver probe, ensuring that it can be retrieved in power management based runtime callbacks if required. This will also help in dropping iomem base address argument from the function etm4_probe() later.
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 Signed-off-by: Anshuman Khandual anshuman.khandual@arm.com --- .../coresight/coresight-etm4x-core.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index 1ea8f173cca0..10119c223fbe 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -2050,17 +2050,14 @@ static int etm4_add_coresight_dev(struct etm4_init_arg *init_arg)
static int etm4_probe(struct device *dev, void __iomem *base, u32 etm_pid) { - struct etmv4_drvdata *drvdata; + struct etmv4_drvdata *drvdata = dev_get_drvdata(dev); struct csdev_access access = { 0 }; struct etm4_init_arg init_arg = { 0 }; struct etm4_init_arg *delayed;
- drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL); - if (!drvdata) + if (WARN_ON(!drvdata)) return -ENOMEM;
- dev_set_drvdata(dev, drvdata); - if (pm_save_enable == PARAM_PM_SAVE_FIRMWARE) pm_save_enable = coresight_loses_context_with_cpu(dev) ? PARAM_PM_SAVE_SELF_HOSTED : PARAM_PM_SAVE_NEVER; @@ -2112,6 +2109,7 @@ static int etm4_probe(struct device *dev, void __iomem *base, u32 etm_pid)
static int etm4_probe_amba(struct amba_device *adev, const struct amba_id *id) { + struct etmv4_drvdata *drvdata; void __iomem *base; struct device *dev = &adev->dev; struct resource *res = &adev->res; @@ -2122,6 +2120,11 @@ static int etm4_probe_amba(struct amba_device *adev, const struct amba_id *id) if (IS_ERR(base)) return PTR_ERR(base);
+ drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL); + if (!drvdata) + return -ENOMEM; + + dev_set_drvdata(dev, drvdata); ret = etm4_probe(dev, base, id->id); if (!ret) pm_runtime_put(&adev->dev); @@ -2131,8 +2134,14 @@ 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 etmv4_drvdata *drvdata; int ret;
+ drvdata = devm_kzalloc(&pdev->dev, sizeof(*drvdata), GFP_KERNEL); + if (!drvdata) + return -ENOMEM; + + dev_set_drvdata(&pdev->dev, drvdata); pm_runtime_get_noresume(&pdev->dev); pm_runtime_set_active(&pdev->dev); pm_runtime_enable(&pdev->dev);
'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 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 10119c223fbe..5d77571a8df9 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -2048,7 +2048,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 }; @@ -2069,8 +2069,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); @@ -2124,8 +2122,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);
@@ -2141,6 +2140,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); @@ -2151,7 +2151,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;
Coresight device pid can be retrieved from its iomem base address, which is stored in 'struct etm4x_drvdata'. This drops pid argument from etm4_probe() and 'struct etm4_init_arg'. Instead etm4_check_arch_features() derives the coresight device pid with a new helper coresight_get_pid(), right before it is consumed in etm4_hisi_match_pid().
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 Signed-off-by: Anshuman Khandual anshuman.khandual@arm.com --- .../coresight/coresight-etm4x-core.c | 19 +++++++------------ include/linux/coresight.h | 12 ++++++++++++ 2 files changed, 19 insertions(+), 12 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index 5d77571a8df9..a4c138e67920 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -66,7 +66,6 @@ static u64 etm4_get_access_type(struct etmv4_config *config); static enum cpuhp_state hp_online;
struct etm4_init_arg { - unsigned int pid; struct device *dev; struct csdev_access *csa; }; @@ -370,8 +369,10 @@ static void etm4_disable_arch_specific(struct etmv4_drvdata *drvdata) }
static void etm4_check_arch_features(struct etmv4_drvdata *drvdata, - unsigned int id) + struct csdev_access *csa) { + unsigned int id = coresight_get_pid(csa); + if (etm4_hisi_match_pid(id)) set_bit(ETM4_IMPDEF_HISI_CORE_COMMIT, drvdata->arch_features); } @@ -1165,7 +1166,7 @@ static void etm4_init_arch_data(void *info) etm4_os_unlock_csa(drvdata, csa); etm4_cs_unlock(drvdata, csa);
- etm4_check_arch_features(drvdata, init_arg->pid); + etm4_check_arch_features(drvdata, csa);
/* find all capabilities of the tracing unit */ etmidr0 = etm4x_relaxed_read32(csa, TRCIDR0); @@ -2048,7 +2049,7 @@ static int etm4_add_coresight_dev(struct etm4_init_arg *init_arg) return 0; }
-static int etm4_probe(struct device *dev, u32 etm_pid) +static int etm4_probe(struct device *dev) { struct etmv4_drvdata *drvdata = dev_get_drvdata(dev); struct csdev_access access = { 0 }; @@ -2077,7 +2078,6 @@ static int etm4_probe(struct device *dev, u32 etm_pid)
init_arg.dev = dev; init_arg.csa = &access; - init_arg.pid = etm_pid;
/* * Serialize against CPUHP callbacks to avoid race condition @@ -2124,7 +2124,7 @@ static int etm4_probe_amba(struct amba_device *adev, const struct amba_id *id)
drvdata->base = base; dev_set_drvdata(dev, drvdata); - ret = etm4_probe(dev, id->id); + ret = etm4_probe(dev); if (!ret) pm_runtime_put(&adev->dev);
@@ -2146,12 +2146,7 @@ static int etm4_probe_platform_dev(struct platform_device *pdev) pm_runtime_set_active(&pdev->dev); pm_runtime_enable(&pdev->dev);
- /* - * System register based devices could match the - * HW by reading appropriate registers on the HW - * and thus we could skip the PID. - */ - ret = etm4_probe(&pdev->dev, 0); + ret = etm4_probe(&pdev->dev);
pm_runtime_put(&pdev->dev); return ret; diff --git a/include/linux/coresight.h b/include/linux/coresight.h index f19a47b9bb5a..f85b041ea475 100644 --- a/include/linux/coresight.h +++ b/include/linux/coresight.h @@ -370,6 +370,18 @@ static inline u32 csdev_access_relaxed_read32(struct csdev_access *csa, return csa->read(offset, true, false); }
+#define CORESIGHT_PIDRn(i) (0xFE0 + ((i) * 4)) + +static inline u32 coresight_get_pid(struct csdev_access *csa) +{ + u32 i, pid = 0; + + for (i = 0; i < 4; i++) + pid |= csdev_access_relaxed_read32(csa, CORESIGHT_PIDRn(i)) << (i * 8); + + return pid; +} + static inline u64 csdev_access_relaxed_read_pair(struct csdev_access *csa, u32 lo_offset, u32 hi_offset) {
This updates existing etm4_platform_driver to accommodate MMIO based device tree represented coresight etm4x devices along with current sysreg ones. It first looks for 'apb_clk' clock and tries to enable it via a new helper i.e coresight_get_enable_apb_pclk(). If 'apb_clock' is not found on the system as indicated by a return value 'NULL', ignore and proceed further assuming that platform already has got required clocks enabled. But if the clock is but could not be enabled, device probe fails with -ENODEV. Similarly iomem base address is fetched via devm_ioremap_resource() onyl when the platform has valid 'struct resource'. The probed device is ensured to be a coresight etm4x, via two new helpers in etm4_init_iomem_access(). This also registers runtime power management callbacks i.e for suspend and resume operations.
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 Signed-off-by: Anshuman Khandual anshuman.khandual@arm.com --- .../coresight/coresight-etm4x-core.c | 62 +++++++++++++++++-- drivers/hwtracing/coresight/coresight-etm4x.h | 3 + include/linux/coresight.h | 44 +++++++++++++ 3 files changed, 105 insertions(+), 4 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index a4c138e67920..60f027e33aa0 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> @@ -1067,12 +1068,24 @@ static bool etm4_init_sysreg_access(struct etmv4_drvdata *drvdata, return true; }
+static bool is_etm4x_device(void __iomem *base) +{ + u32 devarch = readl(base + TRCDEVARCH); + u32 devtype = readl(base + TRCDEVTYPE); + + return (((devarch & ETM_DEVARCH_ID_MASK) == ETM_DEVARCH_ETMv4x_ARCH) && + (devtype == ETM_DEVTYPE_ETMv4x_ARCH)); +} + static bool etm4_init_iomem_access(struct etmv4_drvdata *drvdata, struct csdev_access *csa) { u32 devarch = readl_relaxed(drvdata->base + TRCDEVARCH); u32 idr1 = readl_relaxed(drvdata->base + TRCIDR1);
+ if (!is_coresight_device(drvdata->base) || !is_etm4x_device(drvdata->base)) + return false; + /* * All ETMs must implement TRCDEVARCH to indicate that * the component is an ETMv4. To support any broken @@ -2133,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;
@@ -2140,7 +2154,16 @@ 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)) + return PTR_ERR(drvdata->base); + } + dev_set_drvdata(&pdev->dev, drvdata); pm_runtime_get_noresume(&pdev->dev); pm_runtime_set_active(&pdev->dev); @@ -2186,7 +2209,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 = ETM_DEVTYPE_ETMv4x_ARCH, } };
@@ -2244,6 +2267,10 @@ static int __exit etm4_remove_platform_dev(struct platform_device *pdev)
if (drvdata) ret = etm4_remove_dev(drvdata); + + if (!IS_ERR(drvdata->pclk)) + clk_put(drvdata->pclk); + pm_runtime_disable(&pdev->dev); return ret; } @@ -2284,7 +2311,34 @@ static struct amba_driver etm4x_amba_driver = { .id_table = etm4_ids, };
-static const struct of_device_id etm4_sysreg_match[] = { +#ifdef CONFIG_PM +static int etm4_runtime_suspend(struct device *dev) +{ + struct etmv4_drvdata *drvdata = dev_get_drvdata(dev); + + if (!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 (!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_match[] = { + { .compatible = "arm,coresight-etm4x" }, { .compatible = "arm,coresight-etm4x-sysreg" }, { .compatible = "arm,embedded-trace-extension" }, {} @@ -2295,7 +2349,7 @@ static struct platform_driver etm4_platform_driver = { .remove = etm4_remove_platform_dev, .driver = { .name = "coresight-etm4x", - .of_match_table = etm4_sysreg_match, + .of_match_table = etm4_match, .suppress_bind_attrs = true, }, }; diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h index 434f4e95ee17..5a37df4a02e9 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 ETM_DEVTYPE_ETMv4x_ARCH 0x00000013 + #define TRCSTATR_IDLE_BIT 0 #define TRCSTATR_PMSTABLE_BIT 1 #define ETM_DEFAULT_ADDR_COMP 0 @@ -1017,6 +1019,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..75a7aa6d7444 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,48 @@ 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; +} + +/* + * This function attempts to find a 'apb_pclk' clock on the system and + * if found, enables it. This returns NULL if 'apb_pclk' clock is not + * and return error pointer from clk_prepare_enable(), if it fails to + * enable the discovered clock. + */ +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)
On 17/03/2023 03:04, Anshuman Khandual wrote:
This updates existing etm4_platform_driver to accommodate MMIO based device tree represented coresight etm4x devices along with current sysreg ones. It first looks for 'apb_clk' clock and tries to enable it via a new helper i.e coresight_get_enable_apb_pclk(). If 'apb_clock' is not found on the system as indicated by a return value 'NULL', ignore and proceed further assuming that platform already has got required clocks enabled. But if the clock is but could not be enabled, device probe fails with -ENODEV. Similarly iomem base address is fetched via devm_ioremap_resource() onyl when the platform has valid 'struct resource'. The probed device is ensured to be a coresight etm4x, via two new helpers in etm4_init_iomem_access(). This also registers runtime power management callbacks i.e for suspend and resume operations.
This looks too much detail about the actual implementation of HOW rather than WHAT.
Could it be something like :
"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/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 and DT 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 Signed-off-by: Anshuman Khandual anshuman.khandual@arm.com
.../coresight/coresight-etm4x-core.c | 62 +++++++++++++++++-- drivers/hwtracing/coresight/coresight-etm4x.h | 3 + include/linux/coresight.h | 44 +++++++++++++ 3 files changed, 105 insertions(+), 4 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index a4c138e67920..60f027e33aa0 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> @@ -1067,12 +1068,24 @@ static bool etm4_init_sysreg_access(struct etmv4_drvdata *drvdata, return true; } +static bool is_etm4x_device(void __iomem *base) +{
- u32 devarch = readl(base + TRCDEVARCH);
- u32 devtype = readl(base + TRCDEVTYPE);
- return (((devarch & ETM_DEVARCH_ID_MASK) == ETM_DEVARCH_ETMv4x_ARCH) &&
(devtype == ETM_DEVTYPE_ETMv4x_ARCH));+}
- static bool etm4_init_iomem_access(struct etmv4_drvdata *drvdata, struct csdev_access *csa) { u32 devarch = readl_relaxed(drvdata->base + TRCDEVARCH); u32 idr1 = readl_relaxed(drvdata->base + TRCIDR1);
- if (!is_coresight_device(drvdata->base) || !is_etm4x_device(drvdata->base))
return false;- /*
- All ETMs must implement TRCDEVARCH to indicate that
- the component is an ETMv4. To support any broken
Now that we do the is_etm4x_device(), we could the following ^^ TRCDEVARCH check.
@@ -2133,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;
@@ -2140,7 +2154,16 @@ 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))return PTR_ERR(drvdata->base);
Drop the clock reference ?
- }
- dev_set_drvdata(&pdev->dev, drvdata); pm_runtime_get_noresume(&pdev->dev); pm_runtime_set_active(&pdev->dev);
@@ -2186,7 +2209,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 = ETM_DEVTYPE_ETMv4x_ARCH,@@ -2244,6 +2267,10 @@ static int __exit etm4_remove_platform_dev(struct platform_device *pdev) if (drvdata) ret = etm4_remove_dev(drvdata);
- if (!IS_ERR(drvdata->pclk))
clk_put(drvdata->pclk);- pm_runtime_disable(&pdev->dev);
If we re-order the clk_put() after pm_runtime_disable(), we could use a label to jump here from the ioremap_resource failure.
return ret; } @@ -2284,7 +2311,34 @@ static struct amba_driver etm4x_amba_driver = { .id_table = etm4_ids, }; -static const struct of_device_id etm4_sysreg_match[] = { +#ifdef CONFIG_PM +static int etm4_runtime_suspend(struct device *dev) +{
- struct etmv4_drvdata *drvdata = dev_get_drvdata(dev);
- if (!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 (!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)
+};
Where is this hooked in ?
+static const struct of_device_id etm4_match[] = {
- { .compatible = "arm,coresight-etm4x" }, { .compatible = "arm,coresight-etm4x-sysreg" }, { .compatible = "arm,embedded-trace-extension" }, {}
@@ -2295,7 +2349,7 @@ static struct platform_driver etm4_platform_driver = { .remove = etm4_remove_platform_dev, .driver = { .name = "coresight-etm4x",
.of_match_table = etm4_sysreg_match,
.of_match_table = etm4_match,diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h index 434f4e95ee17..5a37df4a02e9 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 ETM_DEVTYPE_ETMv4x_ARCH 0x00000013
- #define TRCSTATR_IDLE_BIT 0 #define TRCSTATR_PMSTABLE_BIT 1 #define ETM_DEFAULT_ADDR_COMP 0
@@ -1017,6 +1019,7 @@ struct etmv4_save_state {
- @arch_features: Bitmap of arch features of etmv4 devices.
*/ struct etmv4_drvdata {
- struct clk *pclk;
Please document the field, above the structure.
void __iomem *base; struct coresight_device *csdev; spinlock_t spinlock; diff --git a/include/linux/coresight.h b/include/linux/coresight.h index f85b041ea475..75a7aa6d7444 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,48 @@ 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;
+}
+/*
- This function attempts to find a 'apb_pclk' clock on the system and
- if found, enables it. This returns NULL if 'apb_pclk' clock is not
- and return error pointer from clk_prepare_enable(), if it fails to
- enable the discovered clock.
minor nit: Easier if it is something like:
Attempt to find and enable "APB clock" for the given device.
Returns: NULL - No clock found clk - Clock is found and enabled. ERROR - Failed to enable the clock.
Suzuki
On 3/17/23 15:02, Suzuki K Poulose wrote:
On 17/03/2023 03:04, Anshuman Khandual wrote:
This updates existing etm4_platform_driver to accommodate MMIO based device tree represented coresight etm4x devices along with current sysreg ones. It first looks for 'apb_clk' clock and tries to enable it via a new helper i.e coresight_get_enable_apb_pclk(). If 'apb_clock' is not found on the system as indicated by a return value 'NULL', ignore and proceed further assuming that platform already has got required clocks enabled. But if the clock is but could not be enabled, device probe fails with -ENODEV. Similarly iomem base address is fetched via devm_ioremap_resource() onyl when the platform has valid 'struct resource'. The probed device is ensured to be a coresight etm4x, via two new helpers in etm4_init_iomem_access(). This also registers runtime power management callbacks i.e for suspend and resume operations.
This looks too much detail about the actual implementation of HOW rather than WHAT.
Could it be something like :
"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/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 and DT changes to move the devices to be handled via platform driver than the etm4_amba driver.> "
Sure, will update the commit message as above.
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 Signed-off-by: Anshuman Khandual anshuman.khandual@arm.com
.../coresight/coresight-etm4x-core.c | 62 +++++++++++++++++-- drivers/hwtracing/coresight/coresight-etm4x.h | 3 + include/linux/coresight.h | 44 +++++++++++++ 3 files changed, 105 insertions(+), 4 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index a4c138e67920..60f027e33aa0 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> @@ -1067,12 +1068,24 @@ static bool etm4_init_sysreg_access(struct etmv4_drvdata *drvdata, return true; } +static bool is_etm4x_device(void __iomem *base) +{ + u32 devarch = readl(base + TRCDEVARCH); + u32 devtype = readl(base + TRCDEVTYPE);
+ return (((devarch & ETM_DEVARCH_ID_MASK) == ETM_DEVARCH_ETMv4x_ARCH) && + (devtype == ETM_DEVTYPE_ETMv4x_ARCH)); +}
static bool etm4_init_iomem_access(struct etmv4_drvdata *drvdata, struct csdev_access *csa) { u32 devarch = readl_relaxed(drvdata->base + TRCDEVARCH); u32 idr1 = readl_relaxed(drvdata->base + TRCIDR1); + if (!is_coresight_device(drvdata->base) || !is_etm4x_device(drvdata->base)) + return false;
/* * All ETMs must implement TRCDEVARCH to indicate that * the component is an ETMv4. To support any broken
Now that we do the is_etm4x_device(), we could the following ^^ TRCDEVARCH check.
In order to keep the change at minimum, is_etm4x_device() has been changed to assert device type but not the devarch itself, which is being checked in the existing code.
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -1069,13 +1069,11 @@ static bool etm4_init_sysreg_access(struct etmv4_drvdata *drvdata, return true; }
-static bool is_etm4x_device(void __iomem *base) +static bool is_etm4x_devtype(void __iomem *base) { - u32 devarch = readl(base + TRCDEVARCH); u32 devtype = readl(base + TRCDEVTYPE);
- return (((devarch & ETM_DEVARCH_ID_MASK) == ETM_DEVARCH_ETMv4x_ARCH) && - (devtype == ETM_DEVTYPE_ETMv4x_ARCH)); + return (devtype == ETM_DEVTYPE_ETMv4x_ARCH); }
static bool etm4_init_iomem_access(struct etmv4_drvdata *drvdata, @@ -1084,7 +1082,7 @@ static bool etm4_init_iomem_access(struct etmv4_drvdata *drvdata, u32 devarch = readl_relaxed(drvdata->base + TRCDEVARCH); u32 idr1 = readl_relaxed(drvdata->base + TRCIDR1);
- if (!is_coresight_device(drvdata->base) || !is_etm4x_device(drvdata->base)) + if (!is_coresight_device(drvdata->base) || !is_etm4x_devtype(drvdata->base)) return false;
/*
@@ -2133,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; @@ -2140,7 +2154,16 @@ 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)) + return PTR_ERR(drvdata->base);
Drop the clock reference ?
Right, will drop the clock here.
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); @@ -2186,7 +2209,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 = ETM_DEVTYPE_ETMv4x_ARCH, } }; @@ -2244,6 +2267,10 @@ static int __exit etm4_remove_platform_dev(struct platform_device *pdev) if (drvdata) ret = etm4_remove_dev(drvdata);
+ if (!IS_ERR(drvdata->pclk)) + clk_put(drvdata->pclk);
pm_runtime_disable(&pdev->dev);
If we re-order the clk_put() after pm_runtime_disable(), we could use a label to jump here from the ioremap_resource failure.
This is in etm4_remove_platform_dev() and there is no ioremap_resource() failure ?
return ret; } @@ -2284,7 +2311,34 @@ static struct amba_driver etm4x_amba_driver = { .id_table = etm4_ids, }; -static const struct of_device_id etm4_sysreg_match[] = { +#ifdef CONFIG_PM +static int etm4_runtime_suspend(struct device *dev) +{ + struct etmv4_drvdata *drvdata = dev_get_drvdata(dev);
+ if (!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 (!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) +};
Where is this hooked in ?
These pm_ops needs to tagged with the platform driver.
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index 3ce2b4911a49..fe10dd91183e 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -2280,6 +2280,7 @@ static struct platform_driver etm4_platform_driver = { .of_match_table = etm4_match, .acpi_match_table = ACPI_PTR(etm4x_acpi_ids), .suppress_bind_attrs = true, + .pm = &etm4_dev_pm_ops, }, };
+static const struct of_device_id etm4_match[] = { + { .compatible = "arm,coresight-etm4x" }, { .compatible = "arm,coresight-etm4x-sysreg" }, { .compatible = "arm,embedded-trace-extension" }, {} @@ -2295,7 +2349,7 @@ static struct platform_driver etm4_platform_driver = { .remove = etm4_remove_platform_dev, .driver = { .name = "coresight-etm4x", - .of_match_table = etm4_sysreg_match, + .of_match_table = etm4_match, .suppress_bind_attrs = true, }, }; diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h index 434f4e95ee17..5a37df4a02e9 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 ETM_DEVTYPE_ETMv4x_ARCH 0x00000013
#define TRCSTATR_IDLE_BIT 0 #define TRCSTATR_PMSTABLE_BIT 1 #define ETM_DEFAULT_ADDR_COMP 0 @@ -1017,6 +1019,7 @@ struct etmv4_save_state { * @arch_features: Bitmap of arch features of etmv4 devices. */ struct etmv4_drvdata { + struct clk *pclk;
Please document the field, above the structure.
Will add the following document for the field.
--- a/drivers/hwtracing/coresight/coresight-etm4x.h +++ b/drivers/hwtracing/coresight/coresight-etm4x.h @@ -954,6 +954,7 @@ struct etmv4_save_state {
/** * struct etm4_drvdata - specifics associated to an ETM component + * @pclk APB clock for this component * @base: Memory mapped base address for this component. * @csdev: Component vitals needed by the framework. * @spinlock: Only one at a time pls.
void __iomem *base; struct coresight_device *csdev; spinlock_t spinlock; diff --git a/include/linux/coresight.h b/include/linux/coresight.h index f85b041ea475..75a7aa6d7444 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,48 @@ 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; +}
+/*
- This function attempts to find a 'apb_pclk' clock on the system and
- if found, enables it. This returns NULL if 'apb_pclk' clock is not
- and return error pointer from clk_prepare_enable(), if it fails to
- enable the discovered clock.
minor nit: Easier if it is something like:
Attempt to find and enable "APB clock" for the given device.
Returns: NULL - No clock found clk - Clock is found and enabled. ERROR - Failed to enable the clock.
Sure, will change as follows (slightly reorganized)
/* * 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 */
From: Suzuki 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 Signed-off-by: Suzuki 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 60f027e33aa0..fe494c9c6bad 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_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_match, + .acpi_match_table = ACPI_PTR(etm4x_acpi_ids), .suppress_bind_attrs = true, }, };
On 17/03/2023 03:04, Anshuman Khandual wrote:
From: Suzuki 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.
It may be a good idea to add a short summary of why we are doing this ? Though it is part of the cover letter, it would benefit people looking at the commit (e.g., maintainers or even in the future).
Suzuki
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 Signed-off-by: Suzuki 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 60f027e33aa0..fe494c9c6bad 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_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_match,
.acpi_match_table = ACPI_PTR(etm4x_acpi_ids),
Allow other drivers to claim a device, disregarding the "priority" of "arm,primecell". e.g., CoreSight ETM4x devices could be accessed via MMIO (AMBA Bus) or via CPU system instructions. The CoreSight ETM4x platform driver can now handle both types of devices. In order to make sure the driver gets to handle the "MMIO based" devices, which always had the "arm,primecell" compatible, we have two options :
1) Remove the "arm,primecell" from the DTS. But this may be problematic for an older kernel without the support.
2) The other option is to allow OF code to "ignore" the arm,primecell priority for a selected list of compatibles. This would make sure that both older kernels and the new kernels work fine without breaking the functionality. The new DTS could always have the "arm,primecell" removed.
This patch implements Option (2).
Cc: Rob Herring robh+dt@kernel.org Cc: Frank Rowand frowand.list@gmail.com Cc: Russell King (Oracle) linux@armlinux.org.uk Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org Cc: devicetree@vger.kernel.org Cc: linux-kernel@vger.kernel.org Co-developed-by: Suzuki Poulose suzuki.poulose@arm.com Signed-off-by: Suzuki Poulose suzuki.poulose@arm.com Signed-off-by: Anshuman Khandual anshuman.khandual@arm.com --- drivers/of/platform.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/of/platform.c b/drivers/of/platform.c index b2bd2e783445..59ff1a38ccaa 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -325,6 +325,13 @@ static const struct of_dev_auxdata *of_dev_lookup(const struct of_dev_auxdata *l return NULL; }
+static const struct of_device_id of_ignore_amba_table[] = { +#ifdef CONFIG_CORESIGHT_SOURCE_ETM4X + { .compatible = "arm,coresight-etm4x" }, +#endif + {} /* NULL terminated */ +}; + /** * of_platform_bus_create() - Create a device for a node and its children. * @bus: device node of the bus to instantiate @@ -373,7 +380,8 @@ static int of_platform_bus_create(struct device_node *bus, platform_data = auxdata->platform_data; }
- if (of_device_is_compatible(bus, "arm,primecell")) { + if (of_device_is_compatible(bus, "arm,primecell") && + unlikely(!of_match_node(of_ignore_amba_table, bus))) { /* * Don't return an error here to keep compatibility with older * device tree files.
MMIO based etm4x devices, represented with AMBA device tree entries are now detected and operated via the common platform driver. An explicit dedicated AMBA driver is no longer required, this just drops that off completely.
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 Signed-off-by: Anshuman Khandual anshuman.khandual@arm.com --- .../coresight/coresight-etm4x-core.c | 89 ------------------- 1 file changed, 89 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index fe494c9c6bad..e15551355975 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -2119,32 +2119,6 @@ static int etm4_probe(struct device *dev) return etm4_add_coresight_dev(&init_arg); }
-static int etm4_probe_amba(struct amba_device *adev, const struct amba_id *id) -{ - struct etmv4_drvdata *drvdata; - void __iomem *base; - struct device *dev = &adev->dev; - struct resource *res = &adev->res; - int ret; - - /* Validity for the resource is already checked by the AMBA core */ - base = devm_ioremap_resource(dev, res); - if (IS_ERR(base)) - return PTR_ERR(base); - - drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL); - if (!drvdata) - return -ENOMEM; - - drvdata->base = base; - dev_set_drvdata(dev, drvdata); - ret = etm4_probe(dev); - if (!ret) - pm_runtime_put(&adev->dev); - - return ret; -} - static int etm4_probe_platform_dev(struct platform_device *pdev) { struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0); @@ -2205,15 +2179,6 @@ static int etm4_probe_cpu(unsigned int cpu) return 0; }
-static struct amba_cs_uci_id uci_id_etm4[] = { - { - /* ETMv4 UCI data */ - .devarch = ETM_DEVARCH_ETMv4x_ARCH, - .devarch_mask = ETM_DEVARCH_ID_MASK, - .devtype = ETM_DEVTYPE_ETMv4x_ARCH, - } -}; - static void clear_etmdrvdata(void *info) { int cpu = *(int *)info; @@ -2253,14 +2218,6 @@ static int __exit etm4_remove_dev(struct etmv4_drvdata *drvdata) return 0; }
-static void __exit etm4_remove_amba(struct amba_device *adev) -{ - struct etmv4_drvdata *drvdata = dev_get_drvdata(&adev->dev); - - if (drvdata) - etm4_remove_dev(drvdata); -} - static int __exit etm4_remove_platform_dev(struct platform_device *pdev) { int ret = 0; @@ -2276,42 +2233,6 @@ static int __exit etm4_remove_platform_dev(struct platform_device *pdev) return ret; }
-static const struct amba_id etm4_ids[] = { - CS_AMBA_ID(0x000bb95d), /* Cortex-A53 */ - CS_AMBA_ID(0x000bb95e), /* Cortex-A57 */ - CS_AMBA_ID(0x000bb95a), /* Cortex-A72 */ - CS_AMBA_ID(0x000bb959), /* Cortex-A73 */ - CS_AMBA_UCI_ID(0x000bb9da, uci_id_etm4),/* Cortex-A35 */ - CS_AMBA_UCI_ID(0x000bbd05, uci_id_etm4),/* Cortex-A55 */ - CS_AMBA_UCI_ID(0x000bbd0a, uci_id_etm4),/* Cortex-A75 */ - CS_AMBA_UCI_ID(0x000bbd0c, uci_id_etm4),/* Neoverse N1 */ - CS_AMBA_UCI_ID(0x000bbd41, uci_id_etm4),/* Cortex-A78 */ - CS_AMBA_UCI_ID(0x000f0205, uci_id_etm4),/* Qualcomm Kryo */ - CS_AMBA_UCI_ID(0x000f0211, uci_id_etm4),/* Qualcomm Kryo */ - CS_AMBA_UCI_ID(0x000bb802, uci_id_etm4),/* Qualcomm Kryo 385 Cortex-A55 */ - CS_AMBA_UCI_ID(0x000bb803, uci_id_etm4),/* Qualcomm Kryo 385 Cortex-A75 */ - CS_AMBA_UCI_ID(0x000bb805, uci_id_etm4),/* Qualcomm Kryo 4XX Cortex-A55 */ - CS_AMBA_UCI_ID(0x000bb804, uci_id_etm4),/* Qualcomm Kryo 4XX Cortex-A76 */ - CS_AMBA_UCI_ID(0x000bbd0d, uci_id_etm4),/* Qualcomm Kryo 5XX Cortex-A77 */ - CS_AMBA_UCI_ID(0x000cc0af, uci_id_etm4),/* Marvell ThunderX2 */ - CS_AMBA_UCI_ID(0x000b6d01, uci_id_etm4),/* HiSilicon-Hip08 */ - CS_AMBA_UCI_ID(0x000b6d02, uci_id_etm4),/* HiSilicon-Hip09 */ - {}, -}; - -MODULE_DEVICE_TABLE(amba, etm4_ids); - -static struct amba_driver etm4x_amba_driver = { - .drv = { - .name = "coresight-etm4x", - .owner = THIS_MODULE, - .suppress_bind_attrs = true, - }, - .probe = etm4_probe_amba, - .remove = etm4_remove_amba, - .id_table = etm4_ids, -}; - #ifdef CONFIG_PM static int etm4_runtime_suspend(struct device *dev) { @@ -2374,27 +2295,17 @@ static int __init etm4x_init(void) if (ret) return ret;
- ret = amba_driver_register(&etm4x_amba_driver); - if (ret) { - pr_err("Error registering etm4x AMBA driver\n"); - goto clear_pm; - } - ret = platform_driver_register(&etm4_platform_driver); if (!ret) return 0;
pr_err("Error registering etm4x platform driver\n"); - amba_driver_unregister(&etm4x_amba_driver); - -clear_pm: etm4_pm_clear(); return ret; }
static void __exit etm4x_exit(void) { - amba_driver_unregister(&etm4x_amba_driver); platform_driver_unregister(&etm4_platform_driver); etm4_pm_clear(); }
-
Anshuman Khandual -
Suzuki K Poulose