[RFC] coresight: etm4x: Match all ETM4 instances based on DEVARCH and DEVTYPE

  All,

This is an RFC patch to allow all ETM4 instances to be detected via AMBA driver
without having to add the PIDs to the list. The AMBA driver already supports
checking the DEVTYPE and DEVARCH registers for CoreSight components. This patch
adds a pid,mask value that is bound to match all PIDs (with PIDR2.JEDEC field
mandated to be RA0).

With this patch, we wouldn't need to add the PIDs for newer CPUs to be able to
use them. An entry in the device tree is all we need. The only side effect of
this patch is :
    If a DT description exists for an ETM and the CPU ETM has an erratum, the
    driver may still probe it and use it. But then the DT shouldn't have
    described it in the first place.

Thoughts?

Suzuki

---8>---

coresight: etm4x: Match all ETM4 instances based on DEVARCH

Instead of adding the PIDs forever to the list for the new CPUs, let us detect
a component to be ETMv4 based on the CoreSight CID, DEVTYPE=PE_TRACE and
DEVARCH=ETMv4. This is already done for some of the ETMs. We can extend the PID
matching to match the PIDR2:JEDEC, BIT[3], which must be 1 (RA0) always.

Link: https://lkml.kernel.org/r/20230317030501.1811905-1-anshuman.khandual@arm.com
Cc: Anshuman Khandual <anshuman.khandual@arm.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: frowand.list@gmail.com
Cc: linux@armlinux.org.uk
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 .../coresight/coresight-etm4x-core.c          |  5 +++++
 drivers/hwtracing/coresight/coresight-priv.h  | 19 +++++++++++++++++--
 2 files changed, 22 insertions(+), 2 deletions(-)
  

HI Suzuki,

On Thu, 25 May 2023 at 10:58, Suzuki K Poulose <suzuki.poulose@arm.com> wrote:
>
> All,
>
> This is an RFC patch to allow all ETM4 instances to be detected via AMBA driver
> without having to add the PIDs to the list. The AMBA driver already supports
> checking the DEVTYPE and DEVARCH registers for CoreSight components. This patch
> adds a pid,mask value that is bound to match all PIDs (with PIDR2.JEDEC field
> mandated to be RA0).
>
> With this patch, we wouldn't need to add the PIDs for newer CPUs to be able to
> use them. An entry in the device tree is all we need. The only side effect of
> this patch is :
>     If a DT description exists for an ETM and the CPU ETM has an erratum, the
>     driver may still probe it and use it. But then the DT shouldn't have
>     described it in the first place.
>

Don't think this is an issue.

In the previous mechanism, with an ETM with an erratum - or indeed
need of some arch specific extension as we allow now - someone would
have added the PID - tested it, hit the erratum, and would have to
investigate and fix according to what is required. This changes
nothing in terms of handling errata on ETM hardware - it just removes
the add PID step.

For new ETM that work out of the box, this saves time re-spinning the
driver every time - which is kind of what we want from device tree!

I'd go for it.

Moreover, the same principle could be added to the CTI drivers -
though these are generally pretty standard anyway (i.e. based on
Coresight Soc 600/400), so may be no pressing need for this right now.

Mike



> Thoughts?
>
> Suzuki
>
> ---8>---
>
> coresight: etm4x: Match all ETM4 instances based on DEVARCH
>
> Instead of adding the PIDs forever to the list for the new CPUs, let us detect
> a component to be ETMv4 based on the CoreSight CID, DEVTYPE=PE_TRACE and
> DEVARCH=ETMv4. This is already done for some of the ETMs. We can extend the PID
> matching to match the PIDR2:JEDEC, BIT[3], which must be 1 (RA0) always.
>
> Link: https://lkml.kernel.org/r/20230317030501.1811905-1-anshuman.khandual@arm.com
> Cc: Anshuman Khandual <anshuman.khandual@arm.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: frowand.list@gmail.com
> Cc: linux@armlinux.org.uk
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
>  .../coresight/coresight-etm4x-core.c          |  5 +++++
>  drivers/hwtracing/coresight/coresight-priv.h  | 19 +++++++++++++++++--
>  2 files changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index 4c15fae534f3..8a2e24d5686a 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -2260,6 +2260,11 @@ static const struct amba_id etm4_ids[] = {
>         CS_AMBA_UCI_ID(0x000cc0af, uci_id_etm4),/* Marvell ThunderX2 */
>         CS_AMBA_UCI_ID(0x000b6d01, uci_id_etm4),/* HiSilicon-Hip08 */
>         CS_AMBA_UCI_ID(0x000b6d02, uci_id_etm4),/* HiSilicon-Hip09 */
> +       /*
> +        * Match all PIDs with ETM4 DEVARCH. No need for adding any of the new
> +        * CPUs to the list here.
> +        */
> +       CS_AMBA_MATCH_ALL_UCI(uci_id_etm4),
>         {},
>  };
>
> diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
> index 595ce5862056..72ec36c9232c 100644
> --- a/drivers/hwtracing/coresight/coresight-priv.h
> +++ b/drivers/hwtracing/coresight/coresight-priv.h
> @@ -193,12 +193,27 @@ extern void coresight_remove_cti_ops(void);
>         }
>
>  /* coresight AMBA ID, full UCI structure: id table entry. */
> -#define CS_AMBA_UCI_ID(pid, uci_ptr)           \
> +#define __CS_AMBA_UCI_ID(pid, m, uci_ptr)      \
>         {                                       \
>                 .id     = pid,                  \
> -               .mask   = 0x000fffff,           \
> +               .mask   = m,                    \
>                 .data   = (void *)uci_ptr       \
>         }
> +#define CS_AMBA_UCI_ID(pid, uci)       __CS_AMBA_UCI_ID(pid, 0x000fffff, uci)
> +/*
> + * PIDR2[JEDEC], BIT(3) must be 1 (Read As One) to indicate that rest of the
> + * PIDR1, PIDR2 DES_* fields follow JEDEC encoding for the designer. Use that
> + * as a match value for blanket matching all devices in the given CoreSight
> + * device type and architecture.
> + */
> +#define PIDR2_JEDEC                    BIT(3)
> +#define PID_PIDR2_JEDEC                        (PIDR2_JEDEC << 16)
> +/*
> + * Match all PIDs in a given CoreSight device type and architecture, defined
> + * by the uci.
> + */
> +#define CS_AMBA_MATCH_ALL_UCI(uci)                                     \
> +       __CS_AMBA_UCI_ID(PID_PIDR2_JEDEC, PID_PIDR2_JEDEC, uci)
>
>  /* extract the data value from a UCI structure given amba_id pointer. */
>  static inline void *coresight_get_uci_data(const struct amba_id *id)
> --
> 2.34.1
>


--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK
  

On 01/06/2023 16:59, Mike Leach wrote:
> HI Suzuki,
> 
> On Thu, 25 May 2023 at 10:58, Suzuki K Poulose <suzuki.poulose@arm.com> wrote:
>>
>> All,
>>
>> This is an RFC patch to allow all ETM4 instances to be detected via AMBA driver
>> without having to add the PIDs to the list. The AMBA driver already supports
>> checking the DEVTYPE and DEVARCH registers for CoreSight components. This patch
>> adds a pid,mask value that is bound to match all PIDs (with PIDR2.JEDEC field
>> mandated to be RA0).
>>
>> With this patch, we wouldn't need to add the PIDs for newer CPUs to be able to
>> use them. An entry in the device tree is all we need. The only side effect of
>> this patch is :
>>      If a DT description exists for an ETM and the CPU ETM has an erratum, the
>>      driver may still probe it and use it. But then the DT shouldn't have
>>      described it in the first place.
>>
> 
> Don't think this is an issue.
> 
> In the previous mechanism, with an ETM with an erratum - or indeed
> need of some arch specific extension as we allow now - someone would
> have added the PID - tested it, hit the erratum, and would have to
> investigate and fix according to what is required. This changes
> nothing in terms of handling errata on ETM hardware - it just removes
> the add PID step.
> 
> For new ETM that work out of the box, this saves time re-spinning the
> driver every time - which is kind of what we want from device tree!
> 
> I'd go for it.
> 
> Moreover, the same principle could be added to the CTI drivers -
> though these are generally pretty standard anyway (i.e. based on
> Coresight Soc 600/400), so may be no pressing need for this right now.
> 

Thank you Mike, I will send it as a proper patch.

Cheers
Suzuki