Re: [PATCH V2 3/5] coresight: etm4x: Drop pid argument from etm4_probe()
From: Suzuki K Poulose
Date: Wed May 17 2023 - 05:38:57 EST
On 17/05/2023 05:32, Anshuman Khandual wrote:
On 3/31/23 16:36, Suzuki K Poulose wrote:
On 27/03/2023 06:05, Anshuman Khandual wrote:
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@xxxxxxxxxx>
Cc: Suzuki K Poulose <suzuki.poulose@xxxxxxx>
Cc: Mike Leach <mike.leach@xxxxxxxxxx>
Cc: Leo Yan <leo.yan@xxxxxxxxxx>
Cc: coresight@xxxxxxxxxxxxxxxx
Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
Cc: linux-kernel@xxxxxxxxxxxxxxx
Signed-off-by: Anshuman Khandual <anshuman.khandual@xxxxxxx>
---
.../coresight/coresight-etm4x-core.c | 21 +++++++------------
include/linux/coresight.h | 12 +++++++++++
2 files changed, 20 insertions(+), 13 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index 5d77571a8df9..3521838ab4fb 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);
+
This throws up the following error on an ETE.
ete: trying to read unsupported register @fe0
So, I guess this must be performed only for iomem based
devices. System instruction based device must be identified
by MIDR_EL1/REVIDR_EL1 if needed for specific erratum.
This is not required now. So, we could bail out early
if we are system instruction based device.
Will bail out on non iomem devices via drvdata->base address switch.
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -373,9 +373,10 @@ static void etm4_disable_arch_specific(struct etmv4_drvdata *drvdata)
static void etm4_check_arch_features(struct etmv4_drvdata *drvdata,
struct csdev_access *csa)
{
- unsigned int id = coresight_get_pid(csa);
+ if (!drvdata->base)
+ return;
I would use !csa->io_mem to be more readerf friendly.
- if (etm4_hisi_match_pid(id))
+ if (etm4_hisi_match_pid(coresight_get_pid(csa)))
set_bit(ETM4_IMPDEF_HISI_CORE_COMMIT, drvdata->arch_features);
}
#else
if (etm4_hisi_match_pid(id))
set_bit(ETM4_IMPDEF_HISI_CORE_COMMIT, drvdata->arch_features);
}
@@ -385,7 +386,7 @@ 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)
{
}
#endif /* CONFIG_ETM4X_IMPDEF_FEATURE */
@@ -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);
Given the above, we could make this iomem specific.
We could change coresight_get_pid() to take iomem base address instead
and fetch the pid. But is not the existing csdev_access based approach
better and more generic ?
Yes, true, lets leave it with csdev_access, as it is coresight specific
and not ETMv4.
Cheers
Suzuki