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

Suzuki K Poulose
2026-05-06T09:58:45Z

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;
> }
>
>