Re: [PATCH v3 03/15] coresight: etm4x: Cleanup TRCIDR3 register accesses

Mike Leach
2022-04-12T09:36:27Z

On Fri, 4 Mar 2022 at 17:19, James Clark <james.clark@arm.com> wrote:
>
> This is a no-op change for style and consistency and has no effect on
> the binary output by the compiler. In sysreg.h fields are defined as
> the register name followed by the field name and then _MASK. This
> allows for grepping for fields by name rather than using magic numbers.
>
> Signed-off-by: James Clark <james.clark@arm.com>
> ---
> .../coresight/coresight-etm4x-core.c | 40 +++++--------------
> drivers/hwtracing/coresight/coresight-etm4x.h | 10 +++++
> 2 files changed, 20 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index fd44231e9d8a..1b377f6b31bf 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -1124,53 +1124,33 @@ static void etm4_init_arch_data(void *info)
>
> etmidr3 = etm4x_relaxed_read32(csa, TRCIDR3);
> /* CCITMIN, bits[11:0] minimum threshold value that can be programmed */
> - drvdata->ccitmin = BMVAL(etmidr3, 0, 11);
> + drvdata->ccitmin = FIELD_GET(TRCIDR3_CCITMIN_MASK, etmidr3);
> /* EXLEVEL_S, bits[19:16] Secure state instruction tracing */
> - drvdata->s_ex_level = BMVAL(etmidr3, 16, 19);
> + drvdata->s_ex_level = FIELD_GET(TRCIDR3_EXLEVEL_S_MASK, etmidr3);
> drvdata->config.s_ex_level = drvdata->s_ex_level;
> /* EXLEVEL_NS, bits[23:20] Non-secure state instruction tracing */
> - drvdata->ns_ex_level = BMVAL(etmidr3, 20, 23);
> -
> + drvdata->ns_ex_level = FIELD_GET(TRCIDR3_EXLEVEL_NS_MASK, etmidr3);
> /*
> * TRCERR, bit[24] whether a trace unit can trace a
> * system error exception.
> */
> - if (BMVAL(etmidr3, 24, 24))
> - drvdata->trc_error = true;
> - else
> - drvdata->trc_error = false;
> -
> + drvdata->trc_error = !!(etmidr3 & TRCIDR3_TRCERR);
> /* SYNCPR, bit[25] implementation has a fixed synchronization period? */
> - if (BMVAL(etmidr3, 25, 25))
> - drvdata->syncpr = true;
> - else
> - drvdata->syncpr = false;
> -
> + drvdata->syncpr = !!(etmidr3 & TRCIDR3_SYNCPR);
> /* STALLCTL, bit[26] is stall control implemented? */
> - if (BMVAL(etmidr3, 26, 26))
> - drvdata->stallctl = true;
> - else
> - drvdata->stallctl = false;
> -
> + drvdata->stallctl = !!(etmidr3 & TRCIDR3_STALLCTL);
> /* SYSSTALL, bit[27] implementation can support stall control? */
> - if (BMVAL(etmidr3, 27, 27))
> - drvdata->sysstall = true;
> - else
> - drvdata->sysstall = false;
> -
> + drvdata->sysstall = !!(etmidr3 & TRCIDR3_SYSSTALL);
> /*
> * NUMPROC - the number of PEs available for tracing, 5bits
> * = TRCIDR3.bits[13:12]bits[30:28]
> * bits[4:3] = TRCIDR3.bits[13:12] (since etm-v4.2, otherwise RES0)
> * bits[3:0] = TRCIDR3.bits[30:28]
> */
> - drvdata->nr_pe = (BMVAL(etmidr3, 12, 13) << 3) | BMVAL(etmidr3, 28, 30);
> -
> + drvdata->nr_pe = (FIELD_GET(TRCIDR3_NUMPROC_HI_MASK, etmidr3) << 3) |
> + FIELD_GET(TRCIDR3_NUMPROC_LO_MASK, etmidr3);
> /* NOOVERFLOW, bit[31] is trace overflow prevention supported */
> - if (BMVAL(etmidr3, 31, 31))
> - drvdata->nooverflow = true;
> - else
> - drvdata->nooverflow = false;
> + drvdata->nooverflow = !!(etmidr3 & TRCIDR3_NOOVERFLOW);
>
> /* number of resources trace unit supports */
> etmidr4 = etm4x_relaxed_read32(csa, TRCIDR4);
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
> index cfdf966016b7..1b95c63938f0 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x.h
> +++ b/drivers/hwtracing/coresight/coresight-etm4x.h
> @@ -147,6 +147,16 @@
> #define TRCIDR2_VMIDSIZE_MASK GENMASK(14, 10)
> #define TRCIDR2_CCSIZE_MASK GENMASK(28, 25)
>
> +#define TRCIDR3_CCITMIN_MASK GENMASK(11, 0)
> +#define TRCIDR3_EXLEVEL_S_MASK GENMASK(19, 16)
> +#define TRCIDR3_EXLEVEL_NS_MASK GENMASK(23, 20)
> +#define TRCIDR3_TRCERR BIT(24)
> +#define TRCIDR3_SYNCPR BIT(25)
> +#define TRCIDR3_STALLCTL BIT(26)
> +#define TRCIDR3_SYSSTALL BIT(27)
> +#define TRCIDR3_NUMPROC_LO_MASK GENMASK(30, 28)
> +#define TRCIDR3_NUMPROC_HI_MASK GENMASK(13, 12)
> +#define TRCIDR3_NOOVERFLOW BIT(31)
> /*
> * System instructions to access ETM registers.
> * See ETMv4.4 spec ARM IHI0064F section 4.3.6 System instructions
> --
> 2.28.0
>

Reviewed-by: Mike Leach <mike.leach@linaro.org>
--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK