Re: [PATCH v5 04/12] coresight: etm4x: exclude ss_status from drvdata->config

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

Hi Suzuki,

> On 16/04/2026 16:51, Leo Yan wrote:
> > On Wed, Apr 15, 2026 at 05:55:20PM +0100, Yeoreum Yun wrote:
> >
> > [...]
> >
> > > @@ -573,11 +573,9 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata)
> > >   		etm4x_relaxed_write32(csa, config->res_ctrl[i], TRCRSCTLRn(i));
> > >   	for (i = 0; i < caps->nr_ss_cmp; i++) {
> > > -		/* always clear status bit on restart if using single-shot */
> > > -		if (config->ss_ctrl[i] || config->ss_pe_cmp[i])
> > > -			config->ss_status[i] &= ~TRCSSCSRn_STATUS;
> > >   		etm4x_relaxed_write32(csa, config->ss_ctrl[i], TRCSSCCRn(i));
> > > -		etm4x_relaxed_write32(csa, config->ss_status[i], TRCSSCSRn(i));
> > > +		/* always clear status and pending bits on restart if using single-shot */
> > > +		etm4x_relaxed_write32(csa, 0x0, TRCSSCSRn(i));
> >
> > In Arm ARM, D24.4.60 TRCSSCSR<n>, bits[0..3] are RO.  I think it is
> > fine for directly clear the regiser with zero (means it will only
> > clear status / pending bits).
> >
> > [...]
> >
> > > @@ -1841,10 +1839,11 @@ static ssize_t sshot_status_show(struct device *dev,
> > >   {
> > >   	unsigned long val;
> > >   	struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
> > > +	const struct etmv4_caps *caps = &drvdata->caps;
> > >   	struct etmv4_config *config = &drvdata->config;
> > >   	raw_spin_lock(&drvdata->spinlock);
> > > -	val = config->ss_status[config->ss_idx];
> > > +	val = caps->ss_cmp[config->ss_idx];
> > >   	raw_spin_unlock(&drvdata->spinlock);
> > >   	return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
> > >   }
> >
> > This sysfs knob never can print out a realtime status for sshot, I am
>
> Won't it give the status, when the ETM was disabled (and saved back to
> config), for as sysfs mode operation, where the user collects
> information about the status via sysfs ? ( The question of if someone
> actually makes use of this is a different question )

But I'm asking whether it's meaningful to give information
for PENDING and STATUS after "seesion" is disabled.
As you said, it gives a status after *disabled*.
However, Is this information meaningful and user can something with
these information? TBH I don't think so it would be better to remove
from config...

If this's required ... might be we need to move ss_status into
etm4_drvdata directly anyway.. (ss_status).


--
Sincerely,
Yeoreum Yun