Re: [PATCH v11 25/27] coresight: sysfs: Increment refcount only for system tracers

[Suzuki K Poulose <suzuki.poulose@arm.com>]

On 01/05/2026 17:48, Leo Yan wrote:
> Except for system tracers (e.g. STM), other sources treat multiple
> enables as equivalent to a single enable. The device mode already
> tracks the binary state, so it is redundant to operate refcount.
> 
> Refactor to maintain the refcount only for system sources. This
> simplifies future CPU PM handling without refcount logic.
> 
> Tested-by: James Clark <james.clark@linaro.org>
> Reviewed-by: Yeoreum Yun <yeoreum.yun@arm.com>
> Reviewed-by: James Clark <james.clark@linaro.org>
> Tested-by: Jie Gan <jie.gan@oss.qualcomm.com>
> Signed-off-by: Leo Yan <leo.yan@arm.com>
> ---
>   drivers/hwtracing/coresight/coresight-sysfs.c | 41 ++++++++++++++++++---------
>   1 file changed, 27 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-sysfs.c b/drivers/hwtracing/coresight/coresight-sysfs.c
> index 0aebafcb8d0e8e699652244af5202e7c4dc4e9b1..42cf07b8d15a420e963650000dc32bbb38f0aa62 100644
> --- a/drivers/hwtracing/coresight/coresight-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-sysfs.c
> @@ -39,6 +39,28 @@ ssize_t coresight_simple_show32(struct device *_dev,
>   }
>   EXPORT_SYMBOL_GPL(coresight_simple_show32);
>   
> +static void coresight_source_get_refcnt(struct coresight_device *csdev)
> +{
> +	/*
> +	 * There could be multiple applications driving the software
> +	 * source. So keep the refcount for each such user when the
> +	 * source is already enabled.
> +	 *
> +	 * No need to increment the reference counter for other source
> +	 * types, as multiple enables are the same as a single enable.
> +	 */
> +	if (csdev->subtype.source_subtype ==
> +			CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE)

Minor nit: You must check the csdev->type == DEV_TYPE_SOURCE before
check the subtype.

Rest looks fine to me.

Suzuki


> +		csdev->refcnt++;
> +}
> +
> +static void coresight_source_put_refcnt(struct coresight_device *csdev)
> +{
> +	if (csdev->subtype.source_subtype ==
> +			CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE)
> +		csdev->refcnt--;
> +}
> +
>   static int coresight_enable_source_sysfs(struct coresight_device *csdev,
>   					 enum cs_mode mode,
>   					 struct coresight_path *path)
> @@ -57,14 +79,14 @@ static int coresight_enable_source_sysfs(struct coresight_device *csdev,
>   			return ret;
>   	}
>   
> -	csdev->refcnt++;
> +	coresight_source_get_refcnt(csdev);
>   
>   	return 0;
>   }
>   
>   /**
> - *  coresight_disable_source_sysfs - Drop the reference count by 1 and disable
> - *  the device if there are no users left.
> + *  coresight_disable_source_sysfs - Drop the reference count by 1 for software
> + *  sources. Disable the device if there are no users left.
>    *
>    *  @csdev: The coresight device to disable
>    *  @data: Opaque data to pass on to the disable function of the source device.
> @@ -79,7 +101,7 @@ static bool coresight_disable_source_sysfs(struct coresight_device *csdev,
>   	if (coresight_get_mode(csdev) != CS_MODE_SYSFS)
>   		return false;
>   
> -	csdev->refcnt--;
> +	coresight_source_put_refcnt(csdev);
>   	if (csdev->refcnt == 0) {
>   		coresight_disable_source(csdev, data);
>   		return true;
> @@ -156,9 +178,6 @@ int coresight_enable_sysfs(struct coresight_device *csdev)
>   	int ret = 0;
>   	struct coresight_device *sink;
>   	struct coresight_path *path;
> -	enum coresight_dev_subtype_source subtype;
> -
> -	subtype = csdev->subtype.source_subtype;
>   
>   	mutex_lock(&coresight_mutex);
>   
> @@ -173,13 +192,7 @@ int coresight_enable_sysfs(struct coresight_device *csdev)
>   	 * doesn't hold coresight_mutex.
>   	 */
>   	if (coresight_get_mode(csdev) == CS_MODE_SYSFS) {
> -		/*
> -		 * There could be multiple applications driving the software
> -		 * source. So keep the refcount for each such user when the
> -		 * source is already enabled.
> -		 */
> -		if (subtype == CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE)
> -			csdev->refcnt++;
> +		coresight_source_get_refcnt(csdev);
>   		goto out;
>   	}
>   
>