Hi Leo
On Thu, 23 Oct 2025 at 11:18, Mike Leach mike.leach@linaro.org wrote:
Hi Leo
On Wed, 22 Oct 2025 at 15:04, Leo Yan leo.yan@arm.com wrote:
Hi Mike,
On Wed, Oct 22, 2025 at 01:35:46PM +0100, Mike Leach wrote:
[...]
- We can add a new flag ("bool claim_impl" in the struct csdev_access), by default the field will be zero.
I considered a bool - but the correct place for this would be coresight_device - where we keep all the information about the hardware features.
Maybe coresight_device is suitable. But this might require to update furthermore for claim function arguments (e.g., need to pass "csdev" rather than "csdev_access").
- The drivers support claim tags call coresight_clear_self_claim_tag() in probe (see __catu_probe() as an example), we can call a new function coresight_init_claim_tag() to replace it, this function sets "claim_impl" properly and clear the tag if supported.
I considered moving this initialisation to the common coresight code where we create the coresight_device.
Seems to me, this is dangerous. If a module is not CoreSight compliant and claim registers are absent, when access we will get unknown values or even cause serious result (external abort or bus lockup).
Currently all our drivers assume claim registers are compliant - so this makes no functional difference to the current situation.
If the hardware is non-compliant, and is bound to the current drivers, then there will be an access to the registers that could cause the same issues.
There is little sense in having a call in individual drivers to init claim tags which are regarded as common management structure. Any coresight component with no claim tags must still either have registers indicating no claim tags or as per the specification unimplemented register locations have to be RAZ/WI - which amounts to the same thing.
Having re-visited this - the cpu bound ETMs use a smp call before creating the coresight_device so it would be impractical to move this into the common code or use csdev parameters to make the checks.
The only time we claim / disclaim is during enable / disable device when we are writing many 10s of registers anyway. It does not seem too much of a performance hit to add one additional read per enable
Regards
Mike
[...]
+/*
- Coresight specification defines a maximum of 8 claim tag bits.
- The precise number is implementation defined, and may be obtained by
- reading the CLAIMSET register.
- */
+#define CORESIGHT_CLAIM_BITS_MAX_MASK GENMASK(7, 0) +#define CORESIGHT_CLAIM_SELF_HOSTED_BIT BIT(1)
Now we only care about the self-host bit. Can reuse existed macros ?
I feel that drivers should be written to match the specification - the macros above are a correct mask value per specification and the correct bitfield comparison for the MSBit of the protocol.
The ones below are a protocol masks field, and a specific protocol value that is used in value rather than bit comparisons in the claimtag code. I felt it clearer to differentiate between the uses when reading the code.
How about change CORESIGHT_CLAIM_EXTERNAL and CORESIGHT_CLAIM_SELF_HOSTED to MSBit ?
The checking code for the protocol does a value compare on the pair of bits, not individual bit checks.
Regards
Mike
Thanks, Leo
-- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK