[PATCH 8/8] coresight: cti: Refactor cti_reg32_{show|store}()

[Mike Leach]

Hi Leo,

> -----Original Message-----
> From: Leo Yan <leo.yan at arm.com>
> Sent: Monday, February 9, 2026 6:01 PM
> To: Suzuki Poulose <Suzuki.Poulose at arm.com>; Mike Leach
> <Mike.Leach at arm.com>; James Clark <james.clark at linaro.org>; Alexander
> Shishkin <alexander.shishkin at linux.intel.com>; Greg Kroah-Hartman
> <gregkh at linuxfoundation.org>; Mathieu Poirier
> <mathieu.poirier at linaro.org>; Tingwei Zhang <quic_tingwei at quicinc.com>;
> Yingchao Deng <yingchao.deng at oss.qualcomm.com>; Jie Gan
> <jie.gan at oss.qualcomm.com>
> Cc: coresight at lists.linaro.org; linux-arm-kernel at lists.infradead.org; linux-
> kernel at vger.kernel.org; Leo Yan <Leo.Yan at arm.com>
> Subject: [PATCH 8/8] coresight: cti: Refactor cti_reg32_{show|store}()
> 
> Return an error for any negative offset.  Since the cached value is used
> to store user config, it is not updated when reading back the register
> in cti_reg32_show().
> 
> Signed-off-by: Leo Yan <leo.yan at arm.com>
> ---
>  drivers/hwtracing/coresight/coresight-cti-sysfs.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> index
> 9ef44956ebdc7781717d773fa014165989df2048..baac2a5dd467032fafbc6
> 523d8885de59cb2665b 100644
> --- a/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> @@ -252,14 +252,14 @@ static ssize_t cti_reg32_show(struct device *dev,
> char *buf,
>  	struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
>  	struct cti_config *config = &drvdata->config;
> 
> +	if (reg_offset < 0)
> +		return -EINVAL;
> +
>  	scoped_guard(raw_spinlock_irqsave, &drvdata->spinlock) {
> -		if ((reg_offset >= 0) && cti_is_active(config)) {
> +		if (cti_is_active(config))
>  			val = cti_read_single_reg(drvdata, reg_offset);
> -			if (pcached_val)
> -				*pcached_val = val;

I think we still need this. If a register with an none-zero default value / status that can change over time is read when active, then read when inactive, but never written, then the inactive value will not reflect the last read value.
Relatively minor issue but does represent a potential change in functionality for the driver - even if I cannot see specific issues for current ARM CTIs. This is a R/W cache so should be updated on both R and W.

> -		} else if (pcached_val) {
> +		else if (pcached_val)
>  			val = *pcached_val;
> -		}
>  	}
> 
>  	return sprintf(buf, "%#x\n", val);
> @@ -280,13 +280,16 @@ static ssize_t cti_reg32_store(struct device *dev,
> const char *buf,
>  	if (kstrtoul(buf, 0, &val))
>  		return -EINVAL;
> 
> +	if (reg_offset < 0)
> +		return -EINVAL;
> +
>  	scoped_guard(raw_spinlock_irqsave, &drvdata->spinlock) {
>  		/* local store */
>  		if (pcached_val)
>  			*pcached_val = (u32)val;
> 
>  		/* write through if offset and enabled */
> -		if ((reg_offset >= 0) && cti_is_active(config))
> +		if (cti_is_active(config))
>  			cti_write_single_reg(drvdata, reg_offset, val);
>  	}
> 
> 
> --
> 2.34.1

Regards

Mike