On 19/03/2025 12:00 pm, Suzuki K Poulose wrote:
On 18/03/2025 16:21, James Clark wrote:
The self hosted claim tag will be reset on device probe in a later commit. We'll want to do this before coresight_register() is called so won't have a coresight_device and have to use cs_access instead.
Also make them public and create locked and unlocked versions for later use.
Signed-off-by: James Clark james.clark@linaro.org
minor nit:
The restructuring looks a bit confusing. Could we start with :
coresight_{set,clear}_self_claim_tag_unlocked in Patch 1. Rather than rename what we renamed in patch 1 again here.
I considered collapsing these two actually, would you be ok with that?
drivers/hwtracing/coresight/coresight-core.c | 17 +++++++++++++---- include/linux/coresight.h | 3 ++- 2 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/ hwtracing/coresight/coresight-core.c index 3f1c996d668a..9ff601e2415a 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -151,12 +151,21 @@ static inline void coresight_set_self_claim_tag(struct coresight_device *csdev) isb(); } -static inline void coresight_clear_self_claim_tag(struct coresight_device *csdev) +void coresight_clear_self_claim_tag(struct csdev_access *csa) { - csdev_access_relaxed_write32(&csdev->access, CORESIGHT_CLAIM_SELF_HOSTED, + CS_UNLOCK(csa->base);
This looks to be hard coding MMIO based access ? Should we abstract it based on the csdev_access_** ?
Suzuki
I did something similar earlier but it ended up being huge to do it properly and I wasn't sure of the benefits so I deleted it. By 'properly' I mean change all of the CS_UNLOCK() calls to be abstracted and delete etm4_cs_unlock() because CS_UNLOCK() would handle both. It ends adding in an if and indirection for every lock/unlock for devices that are never sysreg access anyway.
Because there's already this:
static void etm4_cs_unlock(struct etmv4_drvdata *drvdata, struct csdev_access *csa) { if (csa->io_mem) CS_UNLOCK(csa->base);
But yeah I can add the if into coresight_clear_self_claim_tag(), although it technically doesn't do anything now because it's never called by any devices that aren't MMIO. And then it makes etm4_cs_unlock() stand out a bit because its not a core function when it could be. But that's probably fine.