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

[Suzuki K Poulose <suzuki.poulose@arm.com>]

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@linaro.org>
>>> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
>>> Cc: Mike Leach <mike.leach@linaro.org>
>>> Cc: Leo Yan <leo.yan@linaro.org>
>>> Cc: coresight@lists.linaro.org
>>> Cc: linux-arm-kernel@lists.infradead.org
>>> Cc: linux-kernel@vger.kernel.org
>>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>>> ---
>>>    .../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