Re: [PATCH v11 1/9] coresight: etm4x: prohibit modifying ss_status and cntr_val while session is enabled

[Yeoreum Yun <yeoreum.yun@arm.com>]

Hi Mike,

> Hi,
> 
> Preventing a write to the counter value register is correct, but protecting
> the ss_status is redundant.
> 
> On 9/15/26 12:34, Yeoreum Yun wrote:
> > ss_status and cntr_val are overwritten with the values read from the
> > corresponding registers by etm4_disable_hw() when the session is disabled.
> > 
> > This means that any changes to these values made while the session is
> > enabled would be lost when the session is disabled.
> > 
> > Therefore, prohibit modifying ss_status and cntr_val while the session is
> > enabled.
> > 
> > Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
> > ---
> >   drivers/hwtracing/coresight/coresight-etm4x-sysfs.c | 6 ++++++
> >   1 file changed, 6 insertions(+)
> > 
> > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> > index cc6cdd3ae29d5..8d73248b840a6 100644
> > --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> > +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> > @@ -1613,6 +1613,8 @@ static ssize_t cntr_val_store(struct device *dev,
> >   		return -EINVAL;
> >   	if (val > ETM_CNTR_MAX_VAL)
> >   		return -EINVAL;
> > +	if (coresight_get_mode(drvdata->csdev))
> > +		return -EBUSY;
> >   	raw_spin_lock(&drvdata->spinlock);
> >   	idx = config->cntr_idx;
> > @@ -1797,6 +1799,8 @@ static ssize_t sshot_ctrl_store(struct device *dev,
> >   	if (kstrtoul(buf, 16, &val))
> >   		return -EINVAL;
> > +	if (coresight_get_mode(drvdata->csdev))
> > +		return -EBUSY;
> 
> This is protecting a write clearing the STATUS bit in TRCSSCSR - but this is
> cleared automatically when next time the hardware is enabled in
> etm4_enable_hw(). So the write to ss_status in this function is redundant as
> is this protection.
> 
> The actual write to the targeted control register will persist.

Acked, I'll remove this protection in here.

Thanks!

[...]

-- 
Sincerely,
Yeoreum Yun