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

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

Hi Mike,

> Hi,
>
> On 4/21/26 11:30, Yeoreum Yun wrote:
> > > Hi Mike,
> > >
> > > > 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.
> > >
> > > Okay. so after disable sysfs session, to check former session
> > > check whether comprator has 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,
> > >
> > > But I don't think this is the reason for locate ss_status into "config"
> > > since its write purpose is not to configure but the "clear" former bit.
> > > That's why I think it's enough to clear when the new sysfs session starts.
> > >
> >
> > IOW, I think it's better to remove ss_status from configfs item
> > and
> >    - add field ss_cmp in etm4_cpas
> >    - add another field ss_status under "etm4_drvdata" to show "PENDING
> >      and STATUS" bits to sysfs after finishing session.
> >
> > Is is valid for you?
> >
>
> No. Why two different locations for a single register read? If I have the
> ETMv4 hardware manual I am going to look for a something that is
> recognizable as being related to the single shot comparator status
> register(s).
>
> So in sysfs I would expect to see all the bits from the register, displayed,
> without masking off the STATUS and PENDING bits as happens now.
>
> In the code I would expect to see a single location with a sensible name -
> ss_cmp doesn't really correlate terribly well with TRCSSCSR. If you do not
> like the original ss_status, then ss_cmp_status may actually be better,
> ss_cmp could be either the ss comparator status or control register.

Fine. But I'm still we don't need consider ss_status as configfs item.
So it's enough to move ss_status under etm4_drvdata from etm4_config and
use it.


--
Sincerely,
Yeoreum Yun