Re: [PATCH V2 3/5] coresight: etm4x: Drop pid argument from etm4_probe()

[Suzuki K Poulose]

On 31/03/2023 22:24, Steve Clevenger wrote:
> On 3/31/2023 4:06 AM, Suzuki K Poulose wrote:
> > On 27/03/2023 06:05, Anshuman Khandual wrote:
> > > Coresight device pid can be retrieved from its iomem base address,
> > > which is
> > > stored in 'struct etm4x_drvdata'. This drops pid argument from
> > > etm4_probe()
> > > and 'struct etm4_init_arg'. Instead etm4_check_arch_features() derives
> > > the
> > > coresight device pid with a new helper coresight_get_pid(), right
> > > before it
> > > is consumed in etm4_hisi_match_pid().
> > >
> > > Cc: Mathieu Poirier <mathieu.poirier@xxxxxxxxxx>
> > > Cc: Suzuki K Poulose <suzuki.poulose@xxxxxxx>
> > > Cc: Mike Leach <mike.leach@xxxxxxxxxx>
> > > Cc: Leo Yan <leo.yan@xxxxxxxxxx>
> > > Cc: coresight@xxxxxxxxxxxxxxxx
> > > Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
> > > Cc: linux-kernel@xxxxxxxxxxxxxxx
> > > Signed-off-by: Anshuman Khandual <anshuman.khandual@xxxxxxx>
> > > ---
> > >    .../coresight/coresight-etm4x-core.c          | 21 +++++++------------
> > >    include/linux/coresight.h                     | 12 +++++++++++
> > >    2 files changed, 20 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> > > b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> > > index 5d77571a8df9..3521838ab4fb 100644
> > > --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> > > +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> > > @@ -66,7 +66,6 @@ static u64 etm4_get_access_type(struct etmv4_config
> > > *config);
> > >    static enum cpuhp_state hp_online;
> > >      struct etm4_init_arg {
> > > -    unsigned int        pid;
> > >        struct device        *dev;
> > >        struct csdev_access    *csa;
> > >    };
> > > @@ -370,8 +369,10 @@ static void etm4_disable_arch_specific(struct
> > > etmv4_drvdata *drvdata)
> > >    }
> > >      static void etm4_check_arch_features(struct etmv4_drvdata *drvdata,
> > > -                      unsigned int id)
> > > +                     struct csdev_access *csa)
> > >    {
> > > +    unsigned int id = coresight_get_pid(csa);
> > > +
> >
> > This throws up the following error on an ETE.
> >
> > ete: trying to read unsupported register @fe0
> >
> > So, I guess this must be performed only for iomem based
> > devices. System instruction based device must be identified
> > by MIDR_EL1/REVIDR_EL1 if needed for specific erratum.
> > This is not required now. So, we could bail out early
> > if we are system instruction based device.
>
> Besides this, the PID is limited to (I think) 4 bits of ID. TRCIDRs
> offer revision information, but nothing manufacturer specific save for
> the designer. Register fields like MIDR_EL1 Variant + PartNum + Revision
> and TRCPIDR3 REVAND offer help. It may be a combination of registers are
> needed for a manufacturer to adequately ID a part to apply an erratum.
> Perhaps you could at least cache MIDR_EL1 for possible future use?

Like I said, if we ever need them, we could add it. I don't see a point
in storing it right now, if we don't use it.

Suzuki