[PATCH v2 0/1] coresight: fix issue with insufficient claim tags

[PATCH v2 0/1] coresight: fix issue with insufficient claim tags

[Mike Leach]

All CoreSight compliant components have an implementation defined number
of 0 to 8 claim tag bits in the claim tag registers.

These are used to claim the CoreSight resources by system agents.

ARM recommends implementions have 4 claim tag registers.

The CoreSight drivers implement a 2 claim tag bit protocol to allow
self hosted and external debug agents to manage access to the hardware.

However, if there are less than 2 claim tags available the protocol
incorrectly returns an error on device claim, as no checks are made.

If insufficient claim tags are present in a component then the protocol
must return success on claim / disclaim to allow components to be used
normally.

Changes read the CLAIMSET bits to establish the number of available tags,
and adjust the claim returns accordingly.

Cache the claimtag protocol availablity in the coresight_device to reduce
reads for the main claim/disclaim api.

changes since v1:
1) Added claim tag availability cache into coresight_device when using the
main coresight_claim_device() / coresight_disclaim_device() API.

Applies to coresight/next

Mike Leach (1):
  coresight: fix issue where coresight component has no claimtags

 drivers/hwtracing/coresight/coresight-core.c | 46 ++++++++++++++++++--
 drivers/hwtracing/coresight/coresight-priv.h | 10 +++++
 include/linux/coresight.h                    | 15 +++++++
 3 files changed, 68 insertions(+), 3 deletions(-)

-- 
2.32.0

[PATCH v2 1/1] coresight: fix issue where coresight component has no claimtags

[Mike Leach]

Coresight components have 0 to 8 claim tag bits. ARM recommends 4 and
the implemented claim tag protocol uses two of these.

If a component has insufficient claim tags then the protocol incorrectly
returns an error when attempting to claim a component.

Fix by reading CLAIMSET to establish then actual number of claim tags
and return success when attempting to claim a component were there are
insufficient tags to implement the protocol.

Signed-off-by: Mike Leach <mike.leach@linaro.org>
---
 drivers/hwtracing/coresight/coresight-core.c | 46 ++++++++++++++++++--
 drivers/hwtracing/coresight/coresight-priv.h | 10 +++++
 include/linux/coresight.h                    | 15 +++++++
 3 files changed, 68 insertions(+), 3 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index 3267192f0c1c..61b4902b0ead 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -131,6 +131,39 @@ coresight_find_out_connection(struct coresight_device *csdev,
 	return ERR_PTR(-ENODEV);
 }
 
+/*
+ * Reading CLAiMSET returns  a bitfield representing the number of claim tags
+ * implemented from bit 0 to bit nTag-1, valid bits set to 1.
+ *
+ * Claim protocol requires 2 bits so test for MS bit required,
+ * bit 1 -  CORESIGHT_CLAIM_BIT_PROTOCOL_HI
+ *
+ * return true if sufficient claim tags implemented for protocol
+ */
+static bool coresight_claim_tags_implemented_unlocked(struct csdev_access *csa)
+{
+	u32 claim_bits_impl = FIELD_GET(CORESIGHT_CLAIM_BITS_MAX_MASK,
+			 csdev_access_relaxed_read32(csa, CORESIGHT_CLAIMSET));
+	return ((claim_bits_impl & CORESIGHT_CLAIM_BIT_PROTOCOL_HI) != 0);
+}
+
+/*
+ * check if we can use the claim tag protocol.
+ *
+ * If currently unknown - read register to determine if sufficient tags and
+ * save value, otherwise return current value.
+ */
+static bool coresight_check_use_claim_tag_unlocked(struct coresight_device *csdev)
+{
+	if (csdev->claim_tag_info == CS_CLAIM_TAG_UNKNOWN) {
+		if (coresight_claim_tags_implemented_unlocked(&csdev->access))
+			csdev->claim_tag_info = CS_CLAIM_TAG_STD_PROTOCOL;
+		else
+			csdev->claim_tag_info = CS_CLAIM_TAG_NOT_IMPL;
+	}
+	return (csdev->claim_tag_info == CS_CLAIM_TAG_STD_PROTOCOL);
+}
+
 static u32 coresight_read_claim_tags_unlocked(struct coresight_device *csdev)
 {
 	return FIELD_GET(CORESIGHT_CLAIM_MASK,
@@ -156,6 +189,9 @@ EXPORT_SYMBOL_GPL(coresight_clear_self_claim_tag);
 
 void coresight_clear_self_claim_tag_unlocked(struct csdev_access *csa)
 {
+	if (!coresight_claim_tags_implemented_unlocked(csa))
+		return;
+
 	csdev_access_relaxed_write32(csa, CORESIGHT_CLAIM_SELF_HOSTED,
 				     CORESIGHT_CLAIMCLR);
 	isb();
@@ -175,12 +211,13 @@ EXPORT_SYMBOL_GPL(coresight_clear_self_claim_tag_unlocked);
 int coresight_claim_device_unlocked(struct coresight_device *csdev)
 {
 	int tag;
-	struct csdev_access *csa;
 
 	if (WARN_ON(!csdev))
 		return -EINVAL;
 
-	csa = &csdev->access;
+	if (!coresight_check_use_claim_tag_unlocked(csdev))
+		return 0;
+
 	tag = coresight_read_claim_tags_unlocked(csdev);
 
 	switch (tag) {
@@ -190,7 +227,7 @@ int coresight_claim_device_unlocked(struct coresight_device *csdev)
 			return 0;
 
 		/* There was a race setting the tag, clean up and fail */
-		coresight_clear_self_claim_tag_unlocked(csa);
+		coresight_clear_self_claim_tag_unlocked(&csdev->access);
 		dev_dbg(&csdev->dev, "Busy: Couldn't set self claim tag");
 		return -EBUSY;
 
@@ -237,6 +274,9 @@ void coresight_disclaim_device_unlocked(struct coresight_device *csdev)
 	if (WARN_ON(!csdev))
 		return;
 
+	if (!coresight_check_use_claim_tag_unlocked(csdev))
+		return;
+
 	if (coresight_read_claim_tags_unlocked(csdev) == CORESIGHT_CLAIM_SELF_HOSTED)
 		coresight_clear_self_claim_tag_unlocked(&csdev->access);
 	else
diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
index 33e22b1ba043..270f179c8535 100644
--- a/drivers/hwtracing/coresight/coresight-priv.h
+++ b/drivers/hwtracing/coresight/coresight-priv.h
@@ -41,6 +41,16 @@ extern const struct device_type coresight_dev_type[];
 #define CORESIGHT_CLAIM_SELF_HOSTED	2
 #define CORESIGHT_CLAIM_INVALID		3
 
+/*
+ * 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)
+
+/* MS Bit required by the protocol */
+#define CORESIGHT_CLAIM_BIT_PROTOCOL_HI	BIT(1)
+
 #define TIMEOUT_US		100
 #define BMVAL(val, lsb, msb)	((val & GENMASK(msb, lsb)) >> lsb)
 
diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index 6de59ce8ef8c..fefe4317a793 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -242,6 +242,19 @@ struct coresight_trace_id_map {
 	raw_spinlock_t lock;
 };
 
+/*
+ * Coresight claim tag info:
+ * CS_CLAIM_TAG_UNKNOWN      - not yet checked.
+ * CS_CLAIM_TAG_STD_PROTOCOL - using standard claim/release protocol.
+ * CS_CLAIM_TAG_NOT_IMPL     - no claim tags available.
+ */
+enum coresight_claim_tag_info {
+	CS_CLAIM_TAG_UNKNOWN,
+	CS_CLAIM_TAG_STD_PROTOCOL,
+	CS_CLAIM_TAG_NOT_IMPL,
+};
+
+
 /**
  * struct coresight_device - representation of a device as used by the framework
  * @pdata:	Platform data with device connections associated to this device.
@@ -265,6 +278,7 @@ struct coresight_trace_id_map {
  *		CS_MODE_SYSFS. Otherwise it must be accessed from inside the
  *		spinlock.
  * @orphan:	true if the component has connections that haven't been linked.
+ * @claim_tag_info: how the device is using claim tags.
  * @sysfs_sink_activated: 'true' when a sink has been selected for use via sysfs
  *		by writing a 1 to the 'enable_sink' file.  A sink can be
  *		activated but not yet enabled.  Enabling for a _sink_ happens
@@ -291,6 +305,7 @@ struct coresight_device {
 	local_t	mode;
 	int refcnt;
 	bool orphan;
+	enum coresight_claim_tag_info claim_tag_info;
 	/* sink specific fields */
 	bool sysfs_sink_activated;
 	struct dev_ext_attribute *ea;
-- 
2.32.0

Re: [PATCH v2 1/1] coresight: fix issue where coresight component has no claimtags

[Leo Yan]

On Mon, Oct 27, 2025 at 10:35:45PM +0000, Mike Leach wrote:
> Coresight components have 0 to 8 claim tag bits. ARM recommends 4 and
> the implemented claim tag protocol uses two of these.
> 
> If a component has insufficient claim tags then the protocol incorrectly
> returns an error when attempting to claim a component.
> 
> Fix by reading CLAIMSET to establish then actual number of claim tags
> and return success when attempting to claim a component were there are
> insufficient tags to implement the protocol.
> 
> Signed-off-by: Mike Leach <mike.leach@linaro.org>
> ---
>  drivers/hwtracing/coresight/coresight-core.c | 46 ++++++++++++++++++--
>  drivers/hwtracing/coresight/coresight-priv.h | 10 +++++
>  include/linux/coresight.h                    | 15 +++++++
>  3 files changed, 68 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index 3267192f0c1c..61b4902b0ead 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -131,6 +131,39 @@ coresight_find_out_connection(struct coresight_device *csdev,
>  	return ERR_PTR(-ENODEV);
>  }
>  
> +/*
> + * Reading CLAiMSET returns  a bitfield representing the number of claim tags

s/CLAiMSET/CLAIMSET

> + * implemented from bit 0 to bit nTag-1, valid bits set to 1.
> + *
> + * Claim protocol requires 2 bits so test for MS bit required,
> + * bit 1 -  CORESIGHT_CLAIM_BIT_PROTOCOL_HI
> + *
> + * return true if sufficient claim tags implemented for protocol
> + */
> +static bool coresight_claim_tags_implemented_unlocked(struct csdev_access *csa)
> +{
> +	u32 claim_bits_impl = FIELD_GET(CORESIGHT_CLAIM_BITS_MAX_MASK,
> +			 csdev_access_relaxed_read32(csa, CORESIGHT_CLAIMSET));
> +	return ((claim_bits_impl & CORESIGHT_CLAIM_BIT_PROTOCOL_HI) != 0);

How about a simple one?

        return !!FIELD_GET(CORESIGHT_CLAIM_BITS_MAX_MASK,
                           csdev_access_relaxed_read32(csa, CORESIGHT_CLAIMSET));
> +}
> +
> +/*
> + * check if we can use the claim tag protocol.
> + *
> + * If currently unknown - read register to determine if sufficient tags and
> + * save value, otherwise return current value.
> + */
> +static bool coresight_check_use_claim_tag_unlocked(struct coresight_device *csdev)
> +{
> +	if (csdev->claim_tag_info == CS_CLAIM_TAG_UNKNOWN) {
> +		if (coresight_claim_tags_implemented_unlocked(&csdev->access))
> +			csdev->claim_tag_info = CS_CLAIM_TAG_STD_PROTOCOL;
> +		else
> +			csdev->claim_tag_info = CS_CLAIM_TAG_NOT_IMPL;
> +	}
> +	return (csdev->claim_tag_info == CS_CLAIM_TAG_STD_PROTOCOL);
> +}
> +
>  static u32 coresight_read_claim_tags_unlocked(struct coresight_device *csdev)
>  {
>  	return FIELD_GET(CORESIGHT_CLAIM_MASK,

Why do not apply claim tag implemented check in
coresight_read_claim_tags_unlocked() ?

> @@ -156,6 +189,9 @@ EXPORT_SYMBOL_GPL(coresight_clear_self_claim_tag);
>  
>  void coresight_clear_self_claim_tag_unlocked(struct csdev_access *csa)
>  {
> +	if (!coresight_claim_tags_implemented_unlocked(csa))
> +		return;

Here it does not call coresight_check_use_claim_tag_unlocked(), which
means the cached flag is not used. I think we should be consistent
across all claim tag functions.

This is why I suggested to put the flag into csdev_access rather than
in csdev. Seems to me, it is nature to use csdev_access to store this
flag, as it presents a attribute for register access (we can consider
csdev presents a device on bus).

If agree with above, then we can create a new function:

  void coresight_init_self_claim_tag(struct csdev_access *csa)
  {
      csa->claim_tag_implemented = !!FIELD_GET(CORESIGHT_CLAIM_BITS_MAX_MASK,
                           csdev_access_relaxed_read32(csa, CORESIGHT_CLAIMSET));

      if (csa->claim_tag_implemented)
          coresight_clear_self_claim_tag(csa);
  }

This function is called by probe()s in CoreSight drivers, afterwards,
simply check the flag "csa->claim_tag_implemented".

A minor benefit is we can drop coresight_check_use_claim_tag_unlocked()
and the enum coresight_claim_tag_info, as we just need to initialize
csa->claim_tag_implemented at the init phase.

> +
>  	csdev_access_relaxed_write32(csa, CORESIGHT_CLAIM_SELF_HOSTED,
>  				     CORESIGHT_CLAIMCLR);
>  	isb();
> @@ -175,12 +211,13 @@ EXPORT_SYMBOL_GPL(coresight_clear_self_claim_tag_unlocked);
>  int coresight_claim_device_unlocked(struct coresight_device *csdev)
>  {
>  	int tag;
> -	struct csdev_access *csa;
>  
>  	if (WARN_ON(!csdev))
>  		return -EINVAL;
>  
> -	csa = &csdev->access;
> +	if (!coresight_check_use_claim_tag_unlocked(csdev))
> +		return 0;

> +
>  	tag = coresight_read_claim_tags_unlocked(csdev);
>  
>  	switch (tag) {
> @@ -190,7 +227,7 @@ int coresight_claim_device_unlocked(struct coresight_device *csdev)
>  			return 0;
>  
>  		/* There was a race setting the tag, clean up and fail */
> -		coresight_clear_self_claim_tag_unlocked(csa);
> +		coresight_clear_self_claim_tag_unlocked(&csdev->access);
>  		dev_dbg(&csdev->dev, "Busy: Couldn't set self claim tag");
>  		return -EBUSY;
>  
> @@ -237,6 +274,9 @@ void coresight_disclaim_device_unlocked(struct coresight_device *csdev)
>  	if (WARN_ON(!csdev))
>  		return;
>  
> +	if (!coresight_check_use_claim_tag_unlocked(csdev))
> +		return;

If use the method mentioned, no need to spread
coresight_check_use_claim_tag_unlocked() in claim and disclaim functions.

Thanks,
Leo

> +
>  	if (coresight_read_claim_tags_unlocked(csdev) == CORESIGHT_CLAIM_SELF_HOSTED)
>  		coresight_clear_self_claim_tag_unlocked(&csdev->access);
>  	else
> diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
> index 33e22b1ba043..270f179c8535 100644
> --- a/drivers/hwtracing/coresight/coresight-priv.h
> +++ b/drivers/hwtracing/coresight/coresight-priv.h
> @@ -41,6 +41,16 @@ extern const struct device_type coresight_dev_type[];
>  #define CORESIGHT_CLAIM_SELF_HOSTED	2
>  #define CORESIGHT_CLAIM_INVALID		3
>  
> +/*
> + * 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)
> +
> +/* MS Bit required by the protocol */
> +#define CORESIGHT_CLAIM_BIT_PROTOCOL_HI	BIT(1)
> +
>  #define TIMEOUT_US		100
>  #define BMVAL(val, lsb, msb)	((val & GENMASK(msb, lsb)) >> lsb)
>  
> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
> index 6de59ce8ef8c..fefe4317a793 100644
> --- a/include/linux/coresight.h
> +++ b/include/linux/coresight.h
> @@ -242,6 +242,19 @@ struct coresight_trace_id_map {
>  	raw_spinlock_t lock;
>  };
>  
> +/*
> + * Coresight claim tag info:
> + * CS_CLAIM_TAG_UNKNOWN      - not yet checked.
> + * CS_CLAIM_TAG_STD_PROTOCOL - using standard claim/release protocol.
> + * CS_CLAIM_TAG_NOT_IMPL     - no claim tags available.
> + */
> +enum coresight_claim_tag_info {
> +	CS_CLAIM_TAG_UNKNOWN,
> +	CS_CLAIM_TAG_STD_PROTOCOL,
> +	CS_CLAIM_TAG_NOT_IMPL,
> +};
> +
> +
>  /**
>   * struct coresight_device - representation of a device as used by the framework
>   * @pdata:	Platform data with device connections associated to this device.
> @@ -265,6 +278,7 @@ struct coresight_trace_id_map {
>   *		CS_MODE_SYSFS. Otherwise it must be accessed from inside the
>   *		spinlock.
>   * @orphan:	true if the component has connections that haven't been linked.
> + * @claim_tag_info: how the device is using claim tags.
>   * @sysfs_sink_activated: 'true' when a sink has been selected for use via sysfs
>   *		by writing a 1 to the 'enable_sink' file.  A sink can be
>   *		activated but not yet enabled.  Enabling for a _sink_ happens
> @@ -291,6 +305,7 @@ struct coresight_device {
>  	local_t	mode;
>  	int refcnt;
>  	bool orphan;
> +	enum coresight_claim_tag_info claim_tag_info;
>  	/* sink specific fields */
>  	bool sysfs_sink_activated;
>  	struct dev_ext_attribute *ea;
> -- 
> 2.32.0
> 

Re: [PATCH v2 1/1] coresight: fix issue where coresight component has no claimtags

[Mike Leach]

Hi Leo

On Thu, 13 Nov 2025 at 17:36, Leo Yan <leo.yan@arm.com> wrote:
>
> On Mon, Oct 27, 2025 at 10:35:45PM +0000, Mike Leach wrote:
> > Coresight components have 0 to 8 claim tag bits. ARM recommends 4 and
> > the implemented claim tag protocol uses two of these.
> >
> > If a component has insufficient claim tags then the protocol incorrectly
> > returns an error when attempting to claim a component.
> >
> > Fix by reading CLAIMSET to establish then actual number of claim tags
> > and return success when attempting to claim a component were there are
> > insufficient tags to implement the protocol.
> >
> > Signed-off-by: Mike Leach <mike.leach@linaro.org>
> > ---
> >  drivers/hwtracing/coresight/coresight-core.c | 46 ++++++++++++++++++--
> >  drivers/hwtracing/coresight/coresight-priv.h | 10 +++++
> >  include/linux/coresight.h                    | 15 +++++++
> >  3 files changed, 68 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> > index 3267192f0c1c..61b4902b0ead 100644
> > --- a/drivers/hwtracing/coresight/coresight-core.c
> > +++ b/drivers/hwtracing/coresight/coresight-core.c
> > @@ -131,6 +131,39 @@ coresight_find_out_connection(struct coresight_device *csdev,
> >       return ERR_PTR(-ENODEV);
> >  }
> >
> > +/*
> > + * Reading CLAiMSET returns  a bitfield representing the number of claim tags
>
> s/CLAiMSET/CLAIMSET
>
> > + * implemented from bit 0 to bit nTag-1, valid bits set to 1.
> > + *
> > + * Claim protocol requires 2 bits so test for MS bit required,
> > + * bit 1 -  CORESIGHT_CLAIM_BIT_PROTOCOL_HI
> > + *
> > + * return true if sufficient claim tags implemented for protocol
> > + */
> > +static bool coresight_claim_tags_implemented_unlocked(struct csdev_access *csa)
> > +{
> > +     u32 claim_bits_impl = FIELD_GET(CORESIGHT_CLAIM_BITS_MAX_MASK,
> > +                      csdev_access_relaxed_read32(csa, CORESIGHT_CLAIMSET));
> > +     return ((claim_bits_impl & CORESIGHT_CLAIM_BIT_PROTOCOL_HI) != 0);
>
> How about a simple one?
>
>         return !!FIELD_GET(CORESIGHT_CLAIM_BITS_MAX_MASK,
>                            csdev_access_relaxed_read32(csa, CORESIGHT_CLAIMSET));

Cannot use this as it is valid, if unlikely, to have 1 claim bit which
would give a false positive as we need 2 bits for the existing
protocol.

> > +}
> > +
> > +/*
> > + * check if we can use the claim tag protocol.
> > + *
> > + * If currently unknown - read register to determine if sufficient tags and
> > + * save value, otherwise return current value.
> > + */
> > +static bool coresight_check_use_claim_tag_unlocked(struct coresight_device *csdev)
> > +{
> > +     if (csdev->claim_tag_info == CS_CLAIM_TAG_UNKNOWN) {
> > +             if (coresight_claim_tags_implemented_unlocked(&csdev->access))
> > +                     csdev->claim_tag_info = CS_CLAIM_TAG_STD_PROTOCOL;
> > +             else
> > +                     csdev->claim_tag_info = CS_CLAIM_TAG_NOT_IMPL;
> > +     }
> > +     return (csdev->claim_tag_info == CS_CLAIM_TAG_STD_PROTOCOL);
> > +}
> > +
> >  static u32 coresight_read_claim_tags_unlocked(struct coresight_device *csdev)
> >  {
> >       return FIELD_GET(CORESIGHT_CLAIM_MASK,
>
> Why do not apply claim tag implemented check in
> coresight_read_claim_tags_unlocked() ?

This is a static local only called from either
coresight_claim_device_unlocked or coresight_disclaim_device_unlocked,
both of which check for claim tag implemented before calling this
function.

>
> > @@ -156,6 +189,9 @@ EXPORT_SYMBOL_GPL(coresight_clear_self_claim_tag);
> >
> >  void coresight_clear_self_claim_tag_unlocked(struct csdev_access *csa)
> >  {
> > +     if (!coresight_claim_tags_implemented_unlocked(csa))
> > +             return;
>
> Here it does not call coresight_check_use_claim_tag_unlocked(), which
> means the cached flag is not used. I think we should be consistent
> across all claim tag functions.

the relevant csdev may not yet exist, and is only used once in init -
see below for further explanation

>
> This is why I suggested to put the flag into csdev_access rather than
> in csdev. Seems to me, it is nature to use csdev_access to store this
> flag, as it presents a attribute for register access (we can consider
> csdev presents a device on bus).

Not used here as these claim tag functions are not consistent
themselves - some use a csdev, some use a csa.

The ones that use csa are exclusively used in clearing down a stale
claim tag during initialization of a device in a probe function or sub
function and called only once per device - sometimes  before a csdev
is in existence. This is safe in cases where claim tags do not exist
or are not operating correctly as writes will be ignored. This
therefore only represents a single write per device, and not something
that occurs multiple times and might benefit from caching the claim
tag usage status.

All other uses of claim tags in the drivers use the claim/disclaim
functions using the csdev as a parameter, after probing and when the
claim infrastructure is being used.

Having the claimtag information in the csdev is natural as this has
all the other general cs information - such as cs type, subtype etc -
describing the coresight features of the hardware.

The csa is a structure describing only how the registers are accessed.
with no register or device specifics. This should remain device/
register agnostic and avoid coupling in device/register specific
parameters.

An additional use of the flag for claimtag usage state (beyond the
slight gain in avoiding a read per call to the claim tag API), is to
allows customers whose claim tags are non-operative, or as in an
actual case - also give a false positive of
operationally sufficient claim tags - to set the test to "no claim
tags" during the probe sequence, so as to ensure the test is skipped
and claim tags are never attempted to be used during enable / disable
device

The clearing down of the self claim tag on probe is to remove stale
self claim tags when a kernel is restarted without disclaiming
hardware (e.g. debugging/crash etc) and without repowering the device.
From a kernel perspective, this does not have to happen until the
first claim attempt is made for the device. Given that on the first
claim call we now do a one time check, this one time clearing of
lingering claim tags could be done here, and remove the clear tag apis
completely simplifying the code  - though this would leave the devices
apparently claimed until that first use.

>
> If agree with above, then we can create a new function:
>
>   void coresight_init_self_claim_tag(struct csdev_access *csa)
>   {
>       csa->claim_tag_implemented = !!FIELD_GET(CORESIGHT_CLAIM_BITS_MAX_MASK,
>                            csdev_access_relaxed_read32(csa, CORESIGHT_CLAIMSET));

This fails in the case where a system reports an incorrect number of
claim bits. (existing customer use case)


>
>       if (csa->claim_tag_implemented)
>           coresight_clear_self_claim_tag(csa);
>   }
>
> This function is called by probe()s in CoreSight drivers, afterwards,
> simply check the flag "csa->claim_tag_implemented".
>
> A minor benefit is we can drop coresight_check_use_claim_tag_unlocked()
> and the enum coresight_claim_tag_info, as we just need to initialize
> csa->claim_tag_implemented at the init phase.
>
> > +
> >       csdev_access_relaxed_write32(csa, CORESIGHT_CLAIM_SELF_HOSTED,
> >                                    CORESIGHT_CLAIMCLR);
> >       isb();
> > @@ -175,12 +211,13 @@ EXPORT_SYMBOL_GPL(coresight_clear_self_claim_tag_unlocked);
> >  int coresight_claim_device_unlocked(struct coresight_device *csdev)
> >  {
> >       int tag;
> > -     struct csdev_access *csa;
> >
> >       if (WARN_ON(!csdev))
> >               return -EINVAL;
> >
> > -     csa = &csdev->access;
> > +     if (!coresight_check_use_claim_tag_unlocked(csdev))
> > +             return 0;
>
> > +
> >       tag = coresight_read_claim_tags_unlocked(csdev);
> >
> >       switch (tag) {
> > @@ -190,7 +227,7 @@ int coresight_claim_device_unlocked(struct coresight_device *csdev)
> >                       return 0;
> >
> >               /* There was a race setting the tag, clean up and fail */
> > -             coresight_clear_self_claim_tag_unlocked(csa);
> > +             coresight_clear_self_claim_tag_unlocked(&csdev->access);
> >               dev_dbg(&csdev->dev, "Busy: Couldn't set self claim tag");
> >               return -EBUSY;
> >
> > @@ -237,6 +274,9 @@ void coresight_disclaim_device_unlocked(struct coresight_device *csdev)
> >       if (WARN_ON(!csdev))
> >               return;
> >
> > +     if (!coresight_check_use_claim_tag_unlocked(csdev))
> > +             return;
>
> If use the method mentioned, no need to spread
> coresight_check_use_claim_tag_unlocked() in claim and disclaim functions.
>
> Thanks,
> Leo
>
> > +
> >       if (coresight_read_claim_tags_unlocked(csdev) == CORESIGHT_CLAIM_SELF_HOSTED)
> >               coresight_clear_self_claim_tag_unlocked(&csdev->access);
> >       else
> > diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
> > index 33e22b1ba043..270f179c8535 100644
> > --- a/drivers/hwtracing/coresight/coresight-priv.h
> > +++ b/drivers/hwtracing/coresight/coresight-priv.h
> > @@ -41,6 +41,16 @@ extern const struct device_type coresight_dev_type[];
> >  #define CORESIGHT_CLAIM_SELF_HOSTED  2
> >  #define CORESIGHT_CLAIM_INVALID              3
> >
> > +/*
> > + * 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)
> > +
> > +/* MS Bit required by the protocol */
> > +#define CORESIGHT_CLAIM_BIT_PROTOCOL_HI      BIT(1)
> > +
> >  #define TIMEOUT_US           100
> >  #define BMVAL(val, lsb, msb) ((val & GENMASK(msb, lsb)) >> lsb)
> >
> > diff --git a/include/linux/coresight.h b/include/linux/coresight.h
> > index 6de59ce8ef8c..fefe4317a793 100644
> > --- a/include/linux/coresight.h
> > +++ b/include/linux/coresight.h
> > @@ -242,6 +242,19 @@ struct coresight_trace_id_map {
> >       raw_spinlock_t lock;
> >  };
> >
> > +/*
> > + * Coresight claim tag info:
> > + * CS_CLAIM_TAG_UNKNOWN      - not yet checked.
> > + * CS_CLAIM_TAG_STD_PROTOCOL - using standard claim/release protocol.
> > + * CS_CLAIM_TAG_NOT_IMPL     - no claim tags available.
> > + */
> > +enum coresight_claim_tag_info {
> > +     CS_CLAIM_TAG_UNKNOWN,
> > +     CS_CLAIM_TAG_STD_PROTOCOL,
> > +     CS_CLAIM_TAG_NOT_IMPL,
> > +};
> > +
> > +
> >  /**
> >   * struct coresight_device - representation of a device as used by the framework
> >   * @pdata:   Platform data with device connections associated to this device.
> > @@ -265,6 +278,7 @@ struct coresight_trace_id_map {
> >   *           CS_MODE_SYSFS. Otherwise it must be accessed from inside the
> >   *           spinlock.
> >   * @orphan:  true if the component has connections that haven't been linked.
> > + * @claim_tag_info: how the device is using claim tags.
> >   * @sysfs_sink_activated: 'true' when a sink has been selected for use via sysfs
> >   *           by writing a 1 to the 'enable_sink' file.  A sink can be
> >   *           activated but not yet enabled.  Enabling for a _sink_ happens
> > @@ -291,6 +305,7 @@ struct coresight_device {
> >       local_t mode;
> >       int refcnt;
> >       bool orphan;
> > +     enum coresight_claim_tag_info claim_tag_info;
> >       /* sink specific fields */
> >       bool sysfs_sink_activated;
> >       struct dev_ext_attribute *ea;
> > --
> > 2.32.0
> >

Regards


Mike




--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK

Re: [PATCH v2 1/1] coresight: fix issue where coresight component has no claimtags

[Leo Yan]

Hi Mike,

On Fri, Nov 14, 2025 at 06:03:25PM +0000, Mike Leach wrote:

[...]

> > > +static bool coresight_claim_tags_implemented_unlocked(struct csdev_access *csa)
> > > +{
> > > +     u32 claim_bits_impl = FIELD_GET(CORESIGHT_CLAIM_BITS_MAX_MASK,
> > > +                      csdev_access_relaxed_read32(csa, CORESIGHT_CLAIMSET));
> > > +     return ((claim_bits_impl & CORESIGHT_CLAIM_BIT_PROTOCOL_HI) != 0);
> >
> > How about a simple one?
> >
> >         return !!FIELD_GET(CORESIGHT_CLAIM_BITS_MAX_MASK,
> >                            csdev_access_relaxed_read32(csa, CORESIGHT_CLAIMSET));
> 
> Cannot use this as it is valid, if unlikely, to have 1 claim bit which
> would give a false positive as we need 2 bits for the existing
> protocol.

Understand now.  As the protocol works with multiple bits, I would
check CLAIM_MASK instead:

    u32 claim_bits = FIELD_GET(CORESIGHT_CLAIM_MASK,
                        csdev_access_relaxed_read32(csa, CORESIGHT_CLAIMSET));
    return claim_bits == CORESIGHT_CLAIM_MASK;

[...]

> > >  void coresight_clear_self_claim_tag_unlocked(struct csdev_access *csa)
> > >  {
> > > +     if (!coresight_claim_tags_implemented_unlocked(csa))
> > > +             return;
> >
> > Here it does not call coresight_check_use_claim_tag_unlocked(), which
> > means the cached flag is not used. I think we should be consistent
> > across all claim tag functions.
> 
> the relevant csdev may not yet exist, and is only used once in init -
> see below for further explanation
> 
> >
> > This is why I suggested to put the flag into csdev_access rather than
> > in csdev. Seems to me, it is nature to use csdev_access to store this
> > flag, as it presents a attribute for register access (we can consider
> > csdev presents a device on bus).
> 
> Not used here as these claim tag functions are not consistent
> themselves - some use a csdev, some use a csa.

Yes.  I have noticed that the clear claim functions use "csa" as its
parameter when I reviewed the change.  This is not bad for me, we
might consolidate all claim function for using "csa" parameter.

> The ones that use csa are exclusively used in clearing down a stale
> claim tag during initialization of a device in a probe function or sub
> function and called only once per device - sometimes  before a csdev
> is in existence. This is safe in cases where claim tags do not exist
> or are not operating correctly as writes will be ignored. This
> therefore only represents a single write per device, and not something
> that occurs multiple times and might benefit from caching the claim
> tag usage status.

When the probe() in CoreSight drivers uses a temporary "csa" to clear
claim tags, in the end, the temporary "csa" will be assigned to
csdev->access when invoke coresight_register().  So it is not true for
"use csa in a probe does not benefit from caching the claim tag usage
status".

A corner case is coresight-etm3x-core.c, the "drvdata->csa" is used
to clear claim tag but not synced to csdev.  We can use a separate patch
to fix it.

> All other uses of claim tags in the drivers use the claim/disclaim
> functions using the csdev as a parameter, after probing and when the
> claim infrastructure is being used.

This is not a barrier.  We can easily dereference the flag via csdev:

    if (csdev->access.claim_tag_impl)
         ...

> Having the claimtag information in the csdev is natural as this has
> all the other general cs information - such as cs type, subtype etc -
> describing the coresight features of the hardware.

> The csa is a structure describing only how the registers are accessed.
> with no register or device specifics. This should remain device/
> register agnostic and avoid coupling in device/register specific
> parameters.

csdev presents a device on CoreSight bus, so it makes sense to store
device type and sub type.  It is not necessary to store every hardware
specific info into it.  More imporatnt, a well organized structure can
avoid complexity in flows.

If store flag in csdev, you will face dilemma when exploring a neat
flow.  For instance, during ETMv4's per CPU init, the csdev is not ready
because it has not registered on the bus yet.  If using csdev to store
the cached flag, we will run into a chicken-and-egg problem, on the
other hand, it would be much easier if we store the flag in "csa", as
"csa" is prepared much earlier.

> An additional use of the flag for claimtag usage state (beyond the
> slight gain in avoiding a read per call to the claim tag API), is to
> allows customers whose claim tags are non-operative, or as in an
> actual case - also give a false positive of
> operationally sufficient claim tags - to set the test to "no claim
> tags" during the probe sequence, so as to ensure the test is skipped
> and claim tags are never attempted to be used during enable / disable
> device
> 
> The clearing down of the self claim tag on probe is to remove stale
> self claim tags when a kernel is restarted without disclaiming
> hardware (e.g. debugging/crash etc) and without repowering the device.
> From a kernel perspective, this does not have to happen until the
> first claim attempt is made for the device. Given that on the first
> claim call we now do a one time check, this one time clearing of
> lingering claim tags could be done here, and remove the clear tag apis
> completely simplifying the code  - though this would leave the devices
> apparently claimed until that first use.

Now we already have callsites in device probes for clear claims, seems
to me, we can simply reuse these callsites for claim tag init.  (Just
rename to new functions, coresight_claim_tag_init{_unlocked}()).

I am fine with using the deferring method if the logic is simple
enough.  However, I don't like the idea of separating the claim tag
clearing and the initialization flag - why not maintain the init code
in single place?

Thanks,
Leo