On Tue, Aug 03, 2021 at 02:09:38PM +0100, James Clark wrote:
[...]
-static enum _ocsd_arch_version cs_etm_decoder__get_arch_ver(u32 reg_idr1) +static enum _ocsd_arch_version cs_etm_decoder__get_arch_ver(u32 reg_idr1, u32 reg_devarch) {
- /* ETE has to be v9 so set arch version to v8.3+ (ARCH__AA64) */
- if (cs_etm__is_ete(reg_devarch))
return ARCH_AA64;
Based on values used in below change, I think we can unify the ETM versio number like:
ARCH_V8R3 : REVISION, bits[19:16] is 0x3 ARCH_V8R4 : REVISION, bits[19:16] is 0x4 ARCH_V8R5 : REVISION, bits[19:16] is 0x5
Do you mean make this change in OpenCSD? At the moment it understands these values so I'm not sure if the extra ones would be useful:
Yes. As Mike said, these new macros will cause big changes in OpenCSD, so I don't have strong opinion to add more macros for tracer versions.
+struct cs_ete_trace_params {
- struct cs_etmv4_trace_params base_params;
- u32 reg_devarch;
As we have said, can we directly support ETMv4.5, so that it can smoothly support ETE features? If so, we don't need to add a new structure "cs_ete_trace_params" at here.
I think with the new magic number change this is more likely to stay, what are your thoughts?
Agreed. Just wander if need to define the struct cs_ete_trace_params as below?
struct cs_ete_trace_params { u32 reg_idr0; u32 reg_idr1; u32 reg_idr2; u32 reg_idr8; u32 reg_configr; u32 reg_traceidr; u32 reg_devarch; }
+#define TRCDEVARCH_ARCHPART_SHIFT 0 +#define TRCDEVARCH_ARCHPART_MASK GENMASK(11, 0) +#define TRCDEVARCH_ARCHPART(x) (((x) & TRCDEVARCH_ARCHPART_MASK) >> TRCDEVARCH_ARCHPART_SHIFT)
+#define TRCDEVARCH_ARCHVER_SHIFT 12 +#define TRCDEVARCH_ARCHVER_MASK GENMASK(15, 12) +#define TRCDEVARCH_ARCHVER(x) (((x) & TRCDEVARCH_ARCHVER_MASK) >> TRCDEVARCH_ARCHVER_SHIFT)
+bool cs_etm__is_ete(u32 trcdevarch) +{
- /*
* ETE if ARCHVER is 5 (ARCHVER is 4 for ETM) and ARCHPART is 0xA13.
* See ETM_DEVARCH_ETE_ARCH in coresight-etm4x.h
*/
- return TRCDEVARCH_ARCHVER(trcdevarch) == 5 && TRCDEVARCH_ARCHPART(trcdevarch) == 0xA13;
I think this is incorrect.
Here should check the bit field "REVISION, bits[19:16]". If it's field value is >= 5, then we can say it supports ETE. I checked the spec for ETMv4.4 and ETMv4.6, both use the same values for the Bits[15:12] = 0x4, so the architecture ID is same for ETMv4.x IPs.
I tried to copy this as closely as possible from the ETE driver. See in coresight-etm4x.h
#define ETM_DEVARCH_ETE_ARCH \ (ETM_DEVARCH_ARCHITECT_ARM | ETM_DEVARCH_ARCHID_ETE | ETM_DEVARCH_PRESENT)
Where ETM_DEVARCH_ARCHID_ETE is ARCHVER == 5 and ARCHPART == 0xA13. I didn't check ETM_DEVARCH_ARCHITECT_ARM because I thought that wouldn't be necessary. If we want to make the change do detect >= 5 then I think this should be made in the driver first. @Suzuki, what do you think?
The tracer has two fields:
- ARCHID bits[15:12] - REVISION, bits[19:16]
For ETE its ARCHID[15:12] is 0x5 and ETMv4.x's ARCHID[15:12] is 0x4. So checking ARCHID[15:12] is the right way to distinguish if the tracer is ETE and creates corresponding decoder for it.
When reviewed this patch I assumed we also need to create ETE decoder if ETMv4.x has supported packet extension. As Mike confirmed, all ETMv4.x tracers keep to use existed way to create decoder; so it's not necessary to check REVISION bit field.
So please ignore my this comment.
Thanks, Leo