Re: [PATCH v8 08/13] coresight: etm4x: fix inconsistencies with sysfs configuration

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

> On 06/07/2026 18:39, Leo Yan wrote:
> > On Mon, Jun 29, 2026 at 10:00:01AM +0100, Yeoreum Yun wrote:
> > 
> > [...]
> > 
> > > @@ -922,25 +950,7 @@ static int etm4_enable_sysfs(struct coresight_device *csdev, struct coresight_pa
> > >   {
> > >   	struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> > >   	struct etm4_enable_arg arg = { };
> > > -	unsigned long cfg_hash;
> > > -	int ret, preset;
> > > -
> > > -	/* enable any config activated by configfs */
> > > -	cscfg_config_sysfs_get_active_cfg(&cfg_hash, &preset);
> > > -	if (cfg_hash) {
> > > -		ret = cscfg_csdev_enable_active_config(csdev, cfg_hash, preset);
> > > -		if (ret) {
> > > -			etm4_release_trace_id(drvdata);
> > > -			return ret;
> > > -		}
> > > -	}
> > > -
> > > -	raw_spin_lock(&drvdata->spinlock);
> > > -
> > > -	drvdata->trcid = path->trace_id;
> > > -
> > > -	/* Tracer will never be paused in sysfs mode */
> > > -	drvdata->paused = false;
> > > +	int ret;
> > >   	/*
> > >   	 * Executing etm4_enable_hw on the cpu whose ETM is being enabled
> > > @@ -948,20 +958,20 @@ static int etm4_enable_sysfs(struct coresight_device *csdev, struct coresight_pa
> > >   	 */
> > >   	arg.drvdata = drvdata;
> > >   	arg.path = path;
> > > +
> > > +	raw_spin_lock(&drvdata->spinlock);
> > > +	arg.config = drvdata->config;
> > > +	raw_spin_unlock(&drvdata->spinlock);
> > > +
> > >   	ret = smp_call_function_single(drvdata->cpu,
> > >   				       etm4_enable_sysfs_smp_call, &arg, 1);
> > >   	if (!ret)
> > >   		ret = arg.rc;
> > >   	if (!ret)
> > > -		drvdata->sticky_enable = true;
> > > -
> > > -	if (ret)
> > > +		dev_dbg(&csdev->dev, "ETM tracing enabled\n");
> > > +	else
> > >   		etm4_release_trace_id(drvdata);
> > > -	raw_spin_unlock(&drvdata->spinlock);
> > > -
> > > -	if (!ret)
> > > -		dev_dbg(&csdev->dev, "ETM tracing enabled\n");
> > >   	return ret;
> > >   }
> > 
> > This is most valuable change for me, as now we will have much clear
> > scope for what is protected ("drvdata->config").
> > 
> > However, a corner case was mentioned by Sashiko:
> > 
> >   | It appears etm4_enable_hw() modifies drvdata->ss_status while executing
> >   | via IPI, but sshot_ctrl_store() can modify the same array concurrently
> 
> Could we stop modifying the drvdata->ss_status in enable_hw() ? We moved
> it outside the config claiming that the status is not used(or useful). #
> Then why not remove the updating from enable_hw()

This is because of the consistency with the former.
If we remove those parts in the etm4x_enbale_hw(), the PENDING and STATUS
bits will be never cleared according to the case.

This will be handled in the future for the PENDING and STATUS bit
hanlding in the perf session and sysfs accordingly
before that, to keep the old behavior, remain as-is.

-- 
Sincerely,
Yeoreum Yun