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

[Mike Leach]

Hi,

This register [bit 31] indicates if a single shot comparator has 
matched. So read-back provides information to the user post run to 
determine which if any of the comparators set in this way has actually 
matched.

Moreover, the specification states "Software must reset this bit to 0 to 
re-enable single-shot control" and "Reset state is unknown. STATUS must 
be written to set an initial state...."

Therefore this register must be written as part of any configuration so 
should be available in the drvdata->config for both read and write,

Regards

Mike

On 4/21/26 09:57, Suzuki K Poulose wrote:
> 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 )
>
>
> Cheers
> Suzuki
>
>
> > fine for only printing caps->ss_cmp, this can avoid any misleading.
> >
> > @Suzuki, @Mike, do you agree with the change above?
> >
> > If maintainers agree with this, as Jie suggested, it is good to add a
> > comment in the code and update the document:
> >
> >    Documentation/trace/coresight/coresight-etm4x-reference.rst
> >
> > Thanks,
> > Leo