All,
This is an RFC patch to allow all ETM4 instances to be detected via AMBA driver without having to add the PIDs to the list. The AMBA driver already supports checking the DEVTYPE and DEVARCH registers for CoreSight components. This patch adds a pid,mask value that is bound to match all PIDs (with PIDR2.JEDEC field mandated to be RA0).
With this patch, we wouldn't need to add the PIDs for newer CPUs to be able to use them. An entry in the device tree is all we need. The only side effect of this patch is : If a DT description exists for an ETM and the CPU ETM has an erratum, the driver may still probe it and use it. But then the DT shouldn't have described it in the first place.
Thoughts?
Suzuki
---8>---
coresight: etm4x: Match all ETM4 instances based on DEVARCH
Instead of adding the PIDs forever to the list for the new CPUs, let us detect a component to be ETMv4 based on the CoreSight CID, DEVTYPE=PE_TRACE and DEVARCH=ETMv4. This is already done for some of the ETMs. We can extend the PID matching to match the PIDR2:JEDEC, BIT[3], which must be 1 (RA0) always.
Link: https://lkml.kernel.org/r/20230317030501.1811905-1-anshuman.khandual@arm.com Cc: Anshuman Khandual anshuman.khandual@arm.com Cc: Rob Herring robh+dt@kernel.org Cc: frowand.list@gmail.com Cc: linux@armlinux.org.uk Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com --- .../coresight/coresight-etm4x-core.c | 5 +++++ drivers/hwtracing/coresight/coresight-priv.h | 19 +++++++++++++++++-- 2 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index 4c15fae534f3..8a2e24d5686a 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -2260,6 +2260,11 @@ static const struct amba_id etm4_ids[] = { 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 */ + /* + * Match all PIDs with ETM4 DEVARCH. No need for adding any of the new + * CPUs to the list here. + */ + CS_AMBA_MATCH_ALL_UCI(uci_id_etm4), {}, };
diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h index 595ce5862056..72ec36c9232c 100644 --- a/drivers/hwtracing/coresight/coresight-priv.h +++ b/drivers/hwtracing/coresight/coresight-priv.h @@ -193,12 +193,27 @@ extern void coresight_remove_cti_ops(void); }
/* coresight AMBA ID, full UCI structure: id table entry. */ -#define CS_AMBA_UCI_ID(pid, uci_ptr) \ +#define __CS_AMBA_UCI_ID(pid, m, uci_ptr) \ { \ .id = pid, \ - .mask = 0x000fffff, \ + .mask = m, \ .data = (void *)uci_ptr \ } +#define CS_AMBA_UCI_ID(pid, uci) __CS_AMBA_UCI_ID(pid, 0x000fffff, uci) +/* + * PIDR2[JEDEC], BIT(3) must be 1 (Read As One) to indicate that rest of the + * PIDR1, PIDR2 DES_* fields follow JEDEC encoding for the designer. Use that + * as a match value for blanket matching all devices in the given CoreSight + * device type and architecture. + */ +#define PIDR2_JEDEC BIT(3) +#define PID_PIDR2_JEDEC (PIDR2_JEDEC << 16) +/* + * Match all PIDs in a given CoreSight device type and architecture, defined + * by the uci. + */ +#define CS_AMBA_MATCH_ALL_UCI(uci) \ + __CS_AMBA_UCI_ID(PID_PIDR2_JEDEC, PID_PIDR2_JEDEC, uci)
/* extract the data value from a UCI structure given amba_id pointer. */ static inline void *coresight_get_uci_data(const struct amba_id *id)
HI Suzuki,
On Thu, 25 May 2023 at 10:58, Suzuki K Poulose suzuki.poulose@arm.com wrote:
All,
This is an RFC patch to allow all ETM4 instances to be detected via AMBA driver without having to add the PIDs to the list. The AMBA driver already supports checking the DEVTYPE and DEVARCH registers for CoreSight components. This patch adds a pid,mask value that is bound to match all PIDs (with PIDR2.JEDEC field mandated to be RA0).
With this patch, we wouldn't need to add the PIDs for newer CPUs to be able to use them. An entry in the device tree is all we need. The only side effect of this patch is : If a DT description exists for an ETM and the CPU ETM has an erratum, the driver may still probe it and use it. But then the DT shouldn't have described it in the first place.
Don't think this is an issue.
In the previous mechanism, with an ETM with an erratum - or indeed need of some arch specific extension as we allow now - someone would have added the PID - tested it, hit the erratum, and would have to investigate and fix according to what is required. This changes nothing in terms of handling errata on ETM hardware - it just removes the add PID step.
For new ETM that work out of the box, this saves time re-spinning the driver every time - which is kind of what we want from device tree!
I'd go for it.
Moreover, the same principle could be added to the CTI drivers - though these are generally pretty standard anyway (i.e. based on Coresight Soc 600/400), so may be no pressing need for this right now.
Mike
Thoughts?
Suzuki
---8>---
coresight: etm4x: Match all ETM4 instances based on DEVARCH
Instead of adding the PIDs forever to the list for the new CPUs, let us detect a component to be ETMv4 based on the CoreSight CID, DEVTYPE=PE_TRACE and DEVARCH=ETMv4. This is already done for some of the ETMs. We can extend the PID matching to match the PIDR2:JEDEC, BIT[3], which must be 1 (RA0) always.
Link: https://lkml.kernel.org/r/20230317030501.1811905-1-anshuman.khandual@arm.com Cc: Anshuman Khandual anshuman.khandual@arm.com Cc: Rob Herring robh+dt@kernel.org Cc: frowand.list@gmail.com Cc: linux@armlinux.org.uk Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com
.../coresight/coresight-etm4x-core.c | 5 +++++ drivers/hwtracing/coresight/coresight-priv.h | 19 +++++++++++++++++-- 2 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index 4c15fae534f3..8a2e24d5686a 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -2260,6 +2260,11 @@ static const struct amba_id etm4_ids[] = { 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 */
/** Match all PIDs with ETM4 DEVARCH. No need for adding any of the new* CPUs to the list here.*/CS_AMBA_MATCH_ALL_UCI(uci_id_etm4), {},};
diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h index 595ce5862056..72ec36c9232c 100644 --- a/drivers/hwtracing/coresight/coresight-priv.h +++ b/drivers/hwtracing/coresight/coresight-priv.h @@ -193,12 +193,27 @@ extern void coresight_remove_cti_ops(void); }
/* coresight AMBA ID, full UCI structure: id table entry. */ -#define CS_AMBA_UCI_ID(pid, uci_ptr) \ +#define __CS_AMBA_UCI_ID(pid, m, uci_ptr) \ { \ .id = pid, \
.mask = 0x000fffff, \
.mask = m, \ .data = (void *)uci_ptr \ }+#define CS_AMBA_UCI_ID(pid, uci) __CS_AMBA_UCI_ID(pid, 0x000fffff, uci) +/*
- PIDR2[JEDEC], BIT(3) must be 1 (Read As One) to indicate that rest of the
- PIDR1, PIDR2 DES_* fields follow JEDEC encoding for the designer. Use that
- as a match value for blanket matching all devices in the given CoreSight
- device type and architecture.
- */
+#define PIDR2_JEDEC BIT(3) +#define PID_PIDR2_JEDEC (PIDR2_JEDEC << 16) +/*
- Match all PIDs in a given CoreSight device type and architecture, defined
- by the uci.
- */
+#define CS_AMBA_MATCH_ALL_UCI(uci) \
__CS_AMBA_UCI_ID(PID_PIDR2_JEDEC, PID_PIDR2_JEDEC, uci)/* extract the data value from a UCI structure given amba_id pointer. */ static inline void *coresight_get_uci_data(const struct amba_id *id) -- 2.34.1
-- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK
On 01/06/2023 16:59, Mike Leach wrote:
HI Suzuki,
On Thu, 25 May 2023 at 10:58, Suzuki K Poulose suzuki.poulose@arm.com wrote:
All,
This is an RFC patch to allow all ETM4 instances to be detected via AMBA driver without having to add the PIDs to the list. The AMBA driver already supports checking the DEVTYPE and DEVARCH registers for CoreSight components. This patch adds a pid,mask value that is bound to match all PIDs (with PIDR2.JEDEC field mandated to be RA0).
With this patch, we wouldn't need to add the PIDs for newer CPUs to be able to use them. An entry in the device tree is all we need. The only side effect of this patch is : If a DT description exists for an ETM and the CPU ETM has an erratum, the driver may still probe it and use it. But then the DT shouldn't have described it in the first place.
Don't think this is an issue.
In the previous mechanism, with an ETM with an erratum - or indeed need of some arch specific extension as we allow now - someone would have added the PID - tested it, hit the erratum, and would have to investigate and fix according to what is required. This changes nothing in terms of handling errata on ETM hardware - it just removes the add PID step.
For new ETM that work out of the box, this saves time re-spinning the driver every time - which is kind of what we want from device tree!
I'd go for it.
Moreover, the same principle could be added to the CTI drivers - though these are generally pretty standard anyway (i.e. based on Coresight Soc 600/400), so may be no pressing need for this right now.
Thank you Mike, I will send it as a proper patch.
Cheers Suzuki
-
Mike Leach -
Suzuki K Poulose