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

[Mike Leach]

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.

Regards

Mike


> > --
> > Sincerely,
> > Yeoreum Yun
>
> --
> Sincerely,
> Yeoreum Yun