[PATCH v3 00/15] Make ETM register accesses consistent with sysreg.h

[PATCH v3 00/15] Make ETM register accesses consistent with sysreg.h

[James Clark]

Changes since v2:
 * Implement Mike's suggestion of not having _SHIFT and using the existing
   FIELD_GET and FIELD_PREP methods.
 * Dropped the change to add the new REG_VAL macro because of the above.
 * FIELD_PREP could be used in some trivial cases, but in some cases the
   shift is still required but can be calculated with __bf_shf
 * Improved the commit messages.
 * The change is still binary equivalent, but requires an extra step 
   mentioned at the end of this cover letter.

Applies to coresight/next 3619ee28488
Also available at https://gitlab.arm.com/linux-arm/linux-jc/-/tree/james-cs-register-refactor-v3

To check for binary equivalence follow the same steps in the cover letter
of v2, but apply the following change to coresight-priv.h. This is because
the existing version of the macros wrap the expression in a new scope {}
that flips something in the compiler:

  #undef FIELD_GET
  #define FIELD_GET(_mask, _reg) (((_reg) & (_mask)) >> __bf_shf(_mask))
  #undef FIELD_PREP
  #define FIELD_PREP(_mask, _val) (((_val) << __bf_shf(_mask)) & (_mask))

Thanks
James

James Clark (15):
  coresight: etm4x: Cleanup TRCIDR0 register accesses
  coresight: etm4x: Cleanup TRCIDR2 register accesses
  coresight: etm4x: Cleanup TRCIDR3 register accesses
  coresight: etm4x: Cleanup TRCIDR4 register accesses
  coresight: etm4x: Cleanup TRCIDR5 register accesses
  coresight: etm4x: Cleanup TRCCONFIGR register accesses
  coresight: etm4x: Cleanup TRCEVENTCTL1R register accesses
  coresight: etm4x: Cleanup TRCSTALLCTLR register accesses
  coresight: etm4x: Cleanup TRCVICTLR register accesses
  coresight: etm3x: Cleanup ETMTECR1 register accesses
  coresight: etm4x: Cleanup TRCACATRn register accesses
  coresight: etm4x: Cleanup TRCSSCCRn and TRCSSCSRn register accesses
  coresight: etm4x: Cleanup TRCSSPCICRn register accesses
  coresight: etm4x: Cleanup TRCBBCTLR register accesses
  coresight: etm4x: Cleanup TRCRSCTLRn register accesses

 .../coresight/coresight-etm3x-core.c          |   2 +-
 .../coresight/coresight-etm3x-sysfs.c         |   2 +-
 .../coresight/coresight-etm4x-core.c          | 136 +++++--------
 .../coresight/coresight-etm4x-sysfs.c         | 180 +++++++++---------
 drivers/hwtracing/coresight/coresight-etm4x.h | 122 ++++++++++--
 5 files changed, 244 insertions(+), 198 deletions(-)

-- 
2.28.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 01/15] coresight: etm4x: Cleanup TRCIDR0 register accesses

[James Clark]

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          | 36 +++++--------------
 drivers/hwtracing/coresight/coresight-etm4x.h | 13 +++++++
 2 files changed, 21 insertions(+), 28 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index 7f416a12000e..9120390a7613 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -1097,41 +1097,21 @@ static void etm4_init_arch_data(void *info)
 	etmidr0 = etm4x_relaxed_read32(csa, TRCIDR0);
 
 	/* INSTP0, bits[2:1] P0 tracing support field */
-	if (BMVAL(etmidr0, 1, 2) == 0b11)
-		drvdata->instrp0 = true;
-	else
-		drvdata->instrp0 = false;
-
+	drvdata->instrp0 = !!(FIELD_GET(TRCIDR0_INSTP0_MASK, etmidr0) == 0b11);
 	/* TRCBB, bit[5] Branch broadcast tracing support bit */
-	if (BMVAL(etmidr0, 5, 5))
-		drvdata->trcbb = true;
-	else
-		drvdata->trcbb = false;
-
+	drvdata->trcbb = !!(etmidr0 & TRCIDR0_TRCBB);
 	/* TRCCOND, bit[6] Conditional instruction tracing support bit */
-	if (BMVAL(etmidr0, 6, 6))
-		drvdata->trccond = true;
-	else
-		drvdata->trccond = false;
-
+	drvdata->trccond = !!(etmidr0 & TRCIDR0_TRCCOND);
 	/* TRCCCI, bit[7] Cycle counting instruction bit */
-	if (BMVAL(etmidr0, 7, 7))
-		drvdata->trccci = true;
-	else
-		drvdata->trccci = false;
-
+	drvdata->trccci = !!(etmidr0 & TRCIDR0_TRCCCI);
 	/* RETSTACK, bit[9] Return stack bit */
-	if (BMVAL(etmidr0, 9, 9))
-		drvdata->retstack = true;
-	else
-		drvdata->retstack = false;
-
+	drvdata->retstack = !!(etmidr0 & TRCIDR0_RETSTACK);
 	/* NUMEVENT, bits[11:10] Number of events field */
-	drvdata->nr_event = BMVAL(etmidr0, 10, 11);
+	drvdata->nr_event = FIELD_GET(TRCIDR0_NUMEVENT_MASK, etmidr0);
 	/* QSUPP, bits[16:15] Q element support field */
-	drvdata->q_support = BMVAL(etmidr0, 15, 16);
+	drvdata->q_support = FIELD_GET(TRCIDR0_QSUPP_MASK, etmidr0);
 	/* TSSIZE, bits[28:24] Global timestamp size field */
-	drvdata->ts_size = BMVAL(etmidr0, 24, 28);
+	drvdata->ts_size = FIELD_GET(TRCIDR0_TSSIZE_MASK, etmidr0);
 
 	/* maximum size of resources */
 	etmidr2 = etm4x_relaxed_read32(csa, TRCIDR2);
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
index 3c4d69b096ca..300741fbc0de 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.h
+++ b/drivers/hwtracing/coresight/coresight-etm4x.h
@@ -130,6 +130,19 @@
 
 #define TRCRSR_TA			BIT(12)
 
+/*
+ * Bit positions of registers that are defined above, in the sysreg.h style
+ * of _MASK for multi bit fields and BIT() for single bits.
+ */
+#define TRCIDR0_INSTP0_MASK			GENMASK(2, 1)
+#define TRCIDR0_TRCBB				BIT(5)
+#define TRCIDR0_TRCCOND				BIT(6)
+#define TRCIDR0_TRCCCI				BIT(7)
+#define TRCIDR0_RETSTACK			BIT(9)
+#define TRCIDR0_NUMEVENT_MASK			GENMASK(11, 10)
+#define TRCIDR0_QSUPP_MASK			GENMASK(16, 15)
+#define TRCIDR0_TSSIZE_MASK			GENMASK(28, 24)
+
 /*
  * System instructions to access ETM registers.
  * See ETMv4.4 spec ARM IHI0064F section 4.3.6 System instructions
-- 
2.28.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 01/15] coresight: etm4x: Cleanup TRCIDR0 register accesses

[Mike Leach]

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          | 36 +++++--------------
>  drivers/hwtracing/coresight/coresight-etm4x.h | 13 +++++++
>  2 files changed, 21 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index 7f416a12000e..9120390a7613 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -1097,41 +1097,21 @@ static void etm4_init_arch_data(void *info)
>         etmidr0 = etm4x_relaxed_read32(csa, TRCIDR0);
>
>         /* INSTP0, bits[2:1] P0 tracing support field */
> -       if (BMVAL(etmidr0, 1, 2) == 0b11)
> -               drvdata->instrp0 = true;
> -       else
> -               drvdata->instrp0 = false;
> -
> +       drvdata->instrp0 = !!(FIELD_GET(TRCIDR0_INSTP0_MASK, etmidr0) == 0b11);
>         /* TRCBB, bit[5] Branch broadcast tracing support bit */
> -       if (BMVAL(etmidr0, 5, 5))
> -               drvdata->trcbb = true;
> -       else
> -               drvdata->trcbb = false;
> -
> +       drvdata->trcbb = !!(etmidr0 & TRCIDR0_TRCBB);
>         /* TRCCOND, bit[6] Conditional instruction tracing support bit */
> -       if (BMVAL(etmidr0, 6, 6))
> -               drvdata->trccond = true;
> -       else
> -               drvdata->trccond = false;
> -
> +       drvdata->trccond = !!(etmidr0 & TRCIDR0_TRCCOND);
>         /* TRCCCI, bit[7] Cycle counting instruction bit */
> -       if (BMVAL(etmidr0, 7, 7))
> -               drvdata->trccci = true;
> -       else
> -               drvdata->trccci = false;
> -
> +       drvdata->trccci = !!(etmidr0 & TRCIDR0_TRCCCI);
>         /* RETSTACK, bit[9] Return stack bit */
> -       if (BMVAL(etmidr0, 9, 9))
> -               drvdata->retstack = true;
> -       else
> -               drvdata->retstack = false;
> -
> +       drvdata->retstack = !!(etmidr0 & TRCIDR0_RETSTACK);
>         /* NUMEVENT, bits[11:10] Number of events field */
> -       drvdata->nr_event = BMVAL(etmidr0, 10, 11);
> +       drvdata->nr_event = FIELD_GET(TRCIDR0_NUMEVENT_MASK, etmidr0);
>         /* QSUPP, bits[16:15] Q element support field */
> -       drvdata->q_support = BMVAL(etmidr0, 15, 16);
> +       drvdata->q_support = FIELD_GET(TRCIDR0_QSUPP_MASK, etmidr0);
>         /* TSSIZE, bits[28:24] Global timestamp size field */
> -       drvdata->ts_size = BMVAL(etmidr0, 24, 28);
> +       drvdata->ts_size = FIELD_GET(TRCIDR0_TSSIZE_MASK, etmidr0);
>
>         /* maximum size of resources */
>         etmidr2 = etm4x_relaxed_read32(csa, TRCIDR2);
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
> index 3c4d69b096ca..300741fbc0de 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x.h
> +++ b/drivers/hwtracing/coresight/coresight-etm4x.h
> @@ -130,6 +130,19 @@
>
>  #define TRCRSR_TA                      BIT(12)
>
> +/*
> + * Bit positions of registers that are defined above, in the sysreg.h style
> + * of _MASK for multi bit fields and BIT() for single bits.
> + */
> +#define TRCIDR0_INSTP0_MASK                    GENMASK(2, 1)
> +#define TRCIDR0_TRCBB                          BIT(5)
> +#define TRCIDR0_TRCCOND                                BIT(6)
> +#define TRCIDR0_TRCCCI                         BIT(7)
> +#define TRCIDR0_RETSTACK                       BIT(9)
> +#define TRCIDR0_NUMEVENT_MASK                  GENMASK(11, 10)
> +#define TRCIDR0_QSUPP_MASK                     GENMASK(16, 15)
> +#define TRCIDR0_TSSIZE_MASK                    GENMASK(28, 24)
> +
>  /*
>   * System instructions to access ETM registers.
>   * See ETMv4.4 spec ARM IHI0064F section 4.3.6 System instructions
> --
> 2.28.0
>
> _______________________________________________
> CoreSight mailing list -- coresight@lists.linaro.org
> To unsubscribe send an email to coresight-leave@lists.linaro.org

Reviewed-by: Mike Leach <mike.leach@linaro.org>

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 02/15] coresight: etm4x: Cleanup TRCIDR2 register accesses

[James Clark]

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>
---
 drivers/hwtracing/coresight/coresight-etm4x-core.c | 6 +++---
 drivers/hwtracing/coresight/coresight-etm4x.h      | 4 ++++
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index 9120390a7613..fd44231e9d8a 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -1116,11 +1116,11 @@ static void etm4_init_arch_data(void *info)
 	/* maximum size of resources */
 	etmidr2 = etm4x_relaxed_read32(csa, TRCIDR2);
 	/* CIDSIZE, bits[9:5] Indicates the Context ID size */
-	drvdata->ctxid_size = BMVAL(etmidr2, 5, 9);
+	drvdata->ctxid_size = FIELD_GET(TRCIDR2_CIDSIZE_MASK, etmidr2);
 	/* VMIDSIZE, bits[14:10] Indicates the VMID size */
-	drvdata->vmid_size = BMVAL(etmidr2, 10, 14);
+	drvdata->vmid_size = FIELD_GET(TRCIDR2_VMIDSIZE_MASK, etmidr2);
 	/* CCSIZE, bits[28:25] size of the cycle counter in bits minus 12 */
-	drvdata->ccsize = BMVAL(etmidr2, 25, 28);
+	drvdata->ccsize = FIELD_GET(TRCIDR2_CCSIZE_MASK, etmidr2);
 
 	etmidr3 = etm4x_relaxed_read32(csa, TRCIDR3);
 	/* CCITMIN, bits[11:0] minimum threshold value that can be programmed */
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
index 300741fbc0de..cfdf966016b7 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.h
+++ b/drivers/hwtracing/coresight/coresight-etm4x.h
@@ -143,6 +143,10 @@
 #define TRCIDR0_QSUPP_MASK			GENMASK(16, 15)
 #define TRCIDR0_TSSIZE_MASK			GENMASK(28, 24)
 
+#define TRCIDR2_CIDSIZE_MASK			GENMASK(9, 5)
+#define TRCIDR2_VMIDSIZE_MASK			GENMASK(14, 10)
+#define TRCIDR2_CCSIZE_MASK			GENMASK(28, 25)
+
 /*
  * System instructions to access ETM registers.
  * See ETMv4.4 spec ARM IHI0064F section 4.3.6 System instructions
-- 
2.28.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 02/15] coresight: etm4x: Cleanup TRCIDR2 register accesses

[Mike Leach]

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>
> ---
>  drivers/hwtracing/coresight/coresight-etm4x-core.c | 6 +++---
>  drivers/hwtracing/coresight/coresight-etm4x.h      | 4 ++++
>  2 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index 9120390a7613..fd44231e9d8a 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -1116,11 +1116,11 @@ static void etm4_init_arch_data(void *info)
>         /* maximum size of resources */
>         etmidr2 = etm4x_relaxed_read32(csa, TRCIDR2);
>         /* CIDSIZE, bits[9:5] Indicates the Context ID size */
> -       drvdata->ctxid_size = BMVAL(etmidr2, 5, 9);
> +       drvdata->ctxid_size = FIELD_GET(TRCIDR2_CIDSIZE_MASK, etmidr2);
>         /* VMIDSIZE, bits[14:10] Indicates the VMID size */
> -       drvdata->vmid_size = BMVAL(etmidr2, 10, 14);
> +       drvdata->vmid_size = FIELD_GET(TRCIDR2_VMIDSIZE_MASK, etmidr2);
>         /* CCSIZE, bits[28:25] size of the cycle counter in bits minus 12 */
> -       drvdata->ccsize = BMVAL(etmidr2, 25, 28);
> +       drvdata->ccsize = FIELD_GET(TRCIDR2_CCSIZE_MASK, etmidr2);
>
>         etmidr3 = etm4x_relaxed_read32(csa, TRCIDR3);
>         /* CCITMIN, bits[11:0] minimum threshold value that can be programmed */
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
> index 300741fbc0de..cfdf966016b7 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x.h
> +++ b/drivers/hwtracing/coresight/coresight-etm4x.h
> @@ -143,6 +143,10 @@
>  #define TRCIDR0_QSUPP_MASK                     GENMASK(16, 15)
>  #define TRCIDR0_TSSIZE_MASK                    GENMASK(28, 24)
>
> +#define TRCIDR2_CIDSIZE_MASK                   GENMASK(9, 5)
> +#define TRCIDR2_VMIDSIZE_MASK                  GENMASK(14, 10)
> +#define TRCIDR2_CCSIZE_MASK                    GENMASK(28, 25)
> +
>  /*
>   * 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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[James Clark]

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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Mike Leach]

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 04/15] coresight: etm4x: Cleanup TRCIDR4 register accesses

[James Clark]

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>
---
 drivers/hwtracing/coresight/coresight-etm4x-core.c | 12 ++++++------
 drivers/hwtracing/coresight/coresight-etm4x.h      |  8 ++++++++
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index 1b377f6b31bf..c52ab7f29f41 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -1155,9 +1155,9 @@ static void etm4_init_arch_data(void *info)
 	/* number of resources trace unit supports */
 	etmidr4 = etm4x_relaxed_read32(csa, TRCIDR4);
 	/* NUMACPAIRS, bits[0:3] number of addr comparator pairs for tracing */
-	drvdata->nr_addr_cmp = BMVAL(etmidr4, 0, 3);
+	drvdata->nr_addr_cmp = FIELD_GET(TRCIDR4_NUMACPAIRS_MASK, etmidr4);
 	/* NUMPC, bits[15:12] number of PE comparator inputs for tracing */
-	drvdata->nr_pe_cmp = BMVAL(etmidr4, 12, 15);
+	drvdata->nr_pe_cmp = FIELD_GET(TRCIDR4_NUMPC_MASK, etmidr4);
 	/*
 	 * NUMRSPAIR, bits[19:16]
 	 * The number of resource pairs conveyed by the HW starts at 0, i.e a
@@ -1168,7 +1168,7 @@ static void etm4_init_arch_data(void *info)
 	 * the default TRUE and FALSE resource selectors are omitted.
 	 * Otherwise for values 0x1 and above the number is N + 1 as per v4.2.
 	 */
-	drvdata->nr_resource = BMVAL(etmidr4, 16, 19);
+	drvdata->nr_resource = FIELD_GET(TRCIDR4_NUMRSPAIR_MASK, etmidr4);
 	if ((drvdata->arch < ETM_ARCH_V4_3) || (drvdata->nr_resource > 0))
 		drvdata->nr_resource += 1;
 	/*
@@ -1176,15 +1176,15 @@ static void etm4_init_arch_data(void *info)
 	 * comparator control for tracing. Read any status regs as these
 	 * also contain RO capability data.
 	 */
-	drvdata->nr_ss_cmp = BMVAL(etmidr4, 20, 23);
+	drvdata->nr_ss_cmp = FIELD_GET(TRCIDR4_NUMSSCC_MASK, etmidr4);
 	for (i = 0; i < drvdata->nr_ss_cmp; i++) {
 		drvdata->config.ss_status[i] =
 			etm4x_relaxed_read32(csa, TRCSSCSRn(i));
 	}
 	/* NUMCIDC, bits[27:24] number of Context ID comparators for tracing */
-	drvdata->numcidc = BMVAL(etmidr4, 24, 27);
+	drvdata->numcidc = FIELD_GET(TRCIDR4_NUMCIDC_MASK, etmidr4);
 	/* NUMVMIDC, bits[31:28] number of VMID comparators for tracing */
-	drvdata->numvmidc = BMVAL(etmidr4, 28, 31);
+	drvdata->numvmidc = FIELD_GET(TRCIDR4_NUMVMIDC_MASK, etmidr4);
 
 	etmidr5 = etm4x_relaxed_read32(csa, TRCIDR5);
 	/* NUMEXTIN, bits[8:0] number of external inputs implemented */
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
index 1b95c63938f0..c9c5fd655196 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.h
+++ b/drivers/hwtracing/coresight/coresight-etm4x.h
@@ -157,6 +157,14 @@
 #define TRCIDR3_NUMPROC_LO_MASK			GENMASK(30, 28)
 #define TRCIDR3_NUMPROC_HI_MASK			GENMASK(13, 12)
 #define TRCIDR3_NOOVERFLOW			BIT(31)
+
+#define TRCIDR4_NUMACPAIRS_MASK			GENMASK(3, 0)
+#define TRCIDR4_NUMPC_MASK			GENMASK(15, 12)
+#define TRCIDR4_NUMRSPAIR_MASK			GENMASK(19, 16)
+#define TRCIDR4_NUMSSCC_MASK			GENMASK(23, 20)
+#define TRCIDR4_NUMCIDC_MASK			GENMASK(27, 24)
+#define TRCIDR4_NUMVMIDC_MASK			GENMASK(31, 28)
+
 /*
  * System instructions to access ETM registers.
  * See ETMv4.4 spec ARM IHI0064F section 4.3.6 System instructions
-- 
2.28.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 04/15] coresight: etm4x: Cleanup TRCIDR4 register accesses

[Mike Leach]

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>
> ---
>  drivers/hwtracing/coresight/coresight-etm4x-core.c | 12 ++++++------
>  drivers/hwtracing/coresight/coresight-etm4x.h      |  8 ++++++++
>  2 files changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index 1b377f6b31bf..c52ab7f29f41 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -1155,9 +1155,9 @@ static void etm4_init_arch_data(void *info)
>         /* number of resources trace unit supports */
>         etmidr4 = etm4x_relaxed_read32(csa, TRCIDR4);
>         /* NUMACPAIRS, bits[0:3] number of addr comparator pairs for tracing */
> -       drvdata->nr_addr_cmp = BMVAL(etmidr4, 0, 3);
> +       drvdata->nr_addr_cmp = FIELD_GET(TRCIDR4_NUMACPAIRS_MASK, etmidr4);
>         /* NUMPC, bits[15:12] number of PE comparator inputs for tracing */
> -       drvdata->nr_pe_cmp = BMVAL(etmidr4, 12, 15);
> +       drvdata->nr_pe_cmp = FIELD_GET(TRCIDR4_NUMPC_MASK, etmidr4);
>         /*
>          * NUMRSPAIR, bits[19:16]
>          * The number of resource pairs conveyed by the HW starts at 0, i.e a
> @@ -1168,7 +1168,7 @@ static void etm4_init_arch_data(void *info)
>          * the default TRUE and FALSE resource selectors are omitted.
>          * Otherwise for values 0x1 and above the number is N + 1 as per v4.2.
>          */
> -       drvdata->nr_resource = BMVAL(etmidr4, 16, 19);
> +       drvdata->nr_resource = FIELD_GET(TRCIDR4_NUMRSPAIR_MASK, etmidr4);
>         if ((drvdata->arch < ETM_ARCH_V4_3) || (drvdata->nr_resource > 0))
>                 drvdata->nr_resource += 1;
>         /*
> @@ -1176,15 +1176,15 @@ static void etm4_init_arch_data(void *info)
>          * comparator control for tracing. Read any status regs as these
>          * also contain RO capability data.
>          */
> -       drvdata->nr_ss_cmp = BMVAL(etmidr4, 20, 23);
> +       drvdata->nr_ss_cmp = FIELD_GET(TRCIDR4_NUMSSCC_MASK, etmidr4);
>         for (i = 0; i < drvdata->nr_ss_cmp; i++) {
>                 drvdata->config.ss_status[i] =
>                         etm4x_relaxed_read32(csa, TRCSSCSRn(i));
>         }
>         /* NUMCIDC, bits[27:24] number of Context ID comparators for tracing */
> -       drvdata->numcidc = BMVAL(etmidr4, 24, 27);
> +       drvdata->numcidc = FIELD_GET(TRCIDR4_NUMCIDC_MASK, etmidr4);
>         /* NUMVMIDC, bits[31:28] number of VMID comparators for tracing */
> -       drvdata->numvmidc = BMVAL(etmidr4, 28, 31);
> +       drvdata->numvmidc = FIELD_GET(TRCIDR4_NUMVMIDC_MASK, etmidr4);
>
>         etmidr5 = etm4x_relaxed_read32(csa, TRCIDR5);
>         /* NUMEXTIN, bits[8:0] number of external inputs implemented */
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
> index 1b95c63938f0..c9c5fd655196 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x.h
> +++ b/drivers/hwtracing/coresight/coresight-etm4x.h
> @@ -157,6 +157,14 @@
>  #define TRCIDR3_NUMPROC_LO_MASK                        GENMASK(30, 28)
>  #define TRCIDR3_NUMPROC_HI_MASK                        GENMASK(13, 12)
>  #define TRCIDR3_NOOVERFLOW                     BIT(31)
> +
> +#define TRCIDR4_NUMACPAIRS_MASK                        GENMASK(3, 0)
> +#define TRCIDR4_NUMPC_MASK                     GENMASK(15, 12)
> +#define TRCIDR4_NUMRSPAIR_MASK                 GENMASK(19, 16)
> +#define TRCIDR4_NUMSSCC_MASK                   GENMASK(23, 20)
> +#define TRCIDR4_NUMCIDC_MASK                   GENMASK(27, 24)
> +#define TRCIDR4_NUMVMIDC_MASK                  GENMASK(31, 28)
> +
>  /*
>   * 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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 05/15] coresight: etm4x: Cleanup TRCIDR5 register accesses

[James Clark]

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>
---
 .../hwtracing/coresight/coresight-etm4x-core.c | 18 ++++++------------
 drivers/hwtracing/coresight/coresight-etm4x.h  |  7 +++++++
 2 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index c52ab7f29f41..3f4263117570 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -1188,26 +1188,20 @@ static void etm4_init_arch_data(void *info)
 
 	etmidr5 = etm4x_relaxed_read32(csa, TRCIDR5);
 	/* NUMEXTIN, bits[8:0] number of external inputs implemented */
-	drvdata->nr_ext_inp = BMVAL(etmidr5, 0, 8);
+	drvdata->nr_ext_inp = FIELD_GET(TRCIDR5_NUMEXTIN_MASK, etmidr5);
 	/* TRACEIDSIZE, bits[21:16] indicates the trace ID width */
-	drvdata->trcid_size = BMVAL(etmidr5, 16, 21);
+	drvdata->trcid_size = FIELD_GET(TRCIDR5_TRACEIDSIZE_MASK, etmidr5);
 	/* ATBTRIG, bit[22] implementation can support ATB triggers? */
-	if (BMVAL(etmidr5, 22, 22))
-		drvdata->atbtrig = true;
-	else
-		drvdata->atbtrig = false;
+	drvdata->atbtrig = !!(etmidr5 & TRCIDR5_ATBTRIG);
 	/*
 	 * LPOVERRIDE, bit[23] implementation supports
 	 * low-power state override
 	 */
-	if (BMVAL(etmidr5, 23, 23) && (!drvdata->skip_power_up))
-		drvdata->lpoverride = true;
-	else
-		drvdata->lpoverride = false;
+	drvdata->lpoverride = (etmidr5 & TRCIDR5_LPOVERRIDE) && (!drvdata->skip_power_up);
 	/* NUMSEQSTATE, bits[27:25] number of sequencer states implemented */
-	drvdata->nrseqstate = BMVAL(etmidr5, 25, 27);
+	drvdata->nrseqstate = FIELD_GET(TRCIDR5_NUMSEQSTATE_MASK, etmidr5);
 	/* NUMCNTR, bits[30:28] number of counters available for tracing */
-	drvdata->nr_cntr = BMVAL(etmidr5, 28, 30);
+	drvdata->nr_cntr = FIELD_GET(TRCIDR5_NUMCNTR_MASK, etmidr5);
 	etm4_cs_lock(drvdata, csa);
 	cpu_detect_trace_filtering(drvdata);
 }
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
index c9c5fd655196..3b604cde668b 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.h
+++ b/drivers/hwtracing/coresight/coresight-etm4x.h
@@ -165,6 +165,13 @@
 #define TRCIDR4_NUMCIDC_MASK			GENMASK(27, 24)
 #define TRCIDR4_NUMVMIDC_MASK			GENMASK(31, 28)
 
+#define TRCIDR5_NUMEXTIN_MASK			GENMASK(8, 0)
+#define TRCIDR5_TRACEIDSIZE_MASK		GENMASK(21, 16)
+#define TRCIDR5_ATBTRIG				BIT(22)
+#define TRCIDR5_LPOVERRIDE			BIT(23)
+#define TRCIDR5_NUMSEQSTATE_MASK		GENMASK(27, 25)
+#define TRCIDR5_NUMCNTR_MASK			GENMASK(30, 28)
+
 /*
  * System instructions to access ETM registers.
  * See ETMv4.4 spec ARM IHI0064F section 4.3.6 System instructions
-- 
2.28.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 05/15] coresight: etm4x: Cleanup TRCIDR5 register accesses

[Mike Leach]

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>
> ---
>  .../hwtracing/coresight/coresight-etm4x-core.c | 18 ++++++------------
>  drivers/hwtracing/coresight/coresight-etm4x.h  |  7 +++++++
>  2 files changed, 13 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index c52ab7f29f41..3f4263117570 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -1188,26 +1188,20 @@ static void etm4_init_arch_data(void *info)
>
>         etmidr5 = etm4x_relaxed_read32(csa, TRCIDR5);
>         /* NUMEXTIN, bits[8:0] number of external inputs implemented */
> -       drvdata->nr_ext_inp = BMVAL(etmidr5, 0, 8);
> +       drvdata->nr_ext_inp = FIELD_GET(TRCIDR5_NUMEXTIN_MASK, etmidr5);
>         /* TRACEIDSIZE, bits[21:16] indicates the trace ID width */
> -       drvdata->trcid_size = BMVAL(etmidr5, 16, 21);
> +       drvdata->trcid_size = FIELD_GET(TRCIDR5_TRACEIDSIZE_MASK, etmidr5);
>         /* ATBTRIG, bit[22] implementation can support ATB triggers? */
> -       if (BMVAL(etmidr5, 22, 22))
> -               drvdata->atbtrig = true;
> -       else
> -               drvdata->atbtrig = false;
> +       drvdata->atbtrig = !!(etmidr5 & TRCIDR5_ATBTRIG);
>         /*
>          * LPOVERRIDE, bit[23] implementation supports
>          * low-power state override
>          */
> -       if (BMVAL(etmidr5, 23, 23) && (!drvdata->skip_power_up))
> -               drvdata->lpoverride = true;
> -       else
> -               drvdata->lpoverride = false;
> +       drvdata->lpoverride = (etmidr5 & TRCIDR5_LPOVERRIDE) && (!drvdata->skip_power_up);
>         /* NUMSEQSTATE, bits[27:25] number of sequencer states implemented */
> -       drvdata->nrseqstate = BMVAL(etmidr5, 25, 27);
> +       drvdata->nrseqstate = FIELD_GET(TRCIDR5_NUMSEQSTATE_MASK, etmidr5);
>         /* NUMCNTR, bits[30:28] number of counters available for tracing */
> -       drvdata->nr_cntr = BMVAL(etmidr5, 28, 30);
> +       drvdata->nr_cntr = FIELD_GET(TRCIDR5_NUMCNTR_MASK, etmidr5);
>         etm4_cs_lock(drvdata, csa);
>         cpu_detect_trace_filtering(drvdata);
>  }
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
> index c9c5fd655196..3b604cde668b 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x.h
> +++ b/drivers/hwtracing/coresight/coresight-etm4x.h
> @@ -165,6 +165,13 @@
>  #define TRCIDR4_NUMCIDC_MASK                   GENMASK(27, 24)
>  #define TRCIDR4_NUMVMIDC_MASK                  GENMASK(31, 28)
>
> +#define TRCIDR5_NUMEXTIN_MASK                  GENMASK(8, 0)
> +#define TRCIDR5_TRACEIDSIZE_MASK               GENMASK(21, 16)
> +#define TRCIDR5_ATBTRIG                                BIT(22)
> +#define TRCIDR5_LPOVERRIDE                     BIT(23)
> +#define TRCIDR5_NUMSEQSTATE_MASK               GENMASK(27, 25)
> +#define TRCIDR5_NUMCNTR_MASK                   GENMASK(30, 28)
> +
>  /*
>   * 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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 06/15] coresight: etm4x: Cleanup TRCCONFIGR register accesses

[James Clark]

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          | 12 ++---
 .../coresight/coresight-etm4x-sysfs.c         | 46 +++++++++----------
 drivers/hwtracing/coresight/coresight-etm4x.h | 16 +++++++
 3 files changed, 44 insertions(+), 30 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index 3f4263117570..445e2057d5ed 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -633,7 +633,7 @@ static int etm4_parse_event_config(struct coresight_device *csdev,
 
 	/* Go from generic option to ETMv4 specifics */
 	if (attr->config & BIT(ETM_OPT_CYCACC)) {
-		config->cfg |= BIT(4);
+		config->cfg |= TRCCONFIGR_CCI;
 		/* TRM: Must program this for cycacc to work */
 		config->ccctlr = ETM_CYC_THRESHOLD_DEFAULT;
 	}
@@ -653,14 +653,14 @@ static int etm4_parse_event_config(struct coresight_device *csdev,
 			goto out;
 
 		/* bit[11], Global timestamp tracing bit */
-		config->cfg |= BIT(11);
+		config->cfg |= TRCCONFIGR_TS;
 	}
 
 	/* Only trace contextID when runs in root PID namespace */
 	if ((attr->config & BIT(ETM_OPT_CTXTID)) &&
 	    task_is_in_init_pid_ns(current))
 		/* bit[6], Context ID tracing bit */
-		config->cfg |= BIT(ETM4_CFG_BIT_CTXTID);
+		config->cfg |= TRCCONFIGR_CID;
 
 	/*
 	 * If set bit ETM_OPT_CTXTID2 in perf config, this asks to trace VMID
@@ -672,17 +672,15 @@ static int etm4_parse_event_config(struct coresight_device *csdev,
 			ret = -EINVAL;
 			goto out;
 		}
-
 		/* Only trace virtual contextID when runs in root PID namespace */
 		if (task_is_in_init_pid_ns(current))
-			config->cfg |= BIT(ETM4_CFG_BIT_VMID) |
-				       BIT(ETM4_CFG_BIT_VMID_OPT);
+			config->cfg |= TRCCONFIGR_VMID | TRCCONFIGR_VMIDOPT;
 	}
 
 	/* return stack - enable if selected and supported */
 	if ((attr->config & BIT(ETM_OPT_RETSTK)) && drvdata->retstack)
 		/* bit[12], Return stack enable bit */
-		config->cfg |= BIT(12);
+		config->cfg |= TRCCONFIGR_RS;
 
 	/*
 	 * Set any selected configuration and preset.
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
index 21687cc1e4e2..53f84da3fe44 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
@@ -180,12 +180,12 @@ static ssize_t reset_store(struct device *dev,
 
 	/* Disable data tracing: do not trace load and store data transfers */
 	config->mode &= ~(ETM_MODE_LOAD | ETM_MODE_STORE);
-	config->cfg &= ~(BIT(1) | BIT(2));
+	config->cfg &= ~(TRCCONFIGR_INSTP0_LOAD | TRCCONFIGR_INSTP0_STORE);
 
 	/* Disable data value and data address tracing */
 	config->mode &= ~(ETM_MODE_DATA_TRACE_ADDR |
 			   ETM_MODE_DATA_TRACE_VAL);
-	config->cfg &= ~(BIT(16) | BIT(17));
+	config->cfg &= ~(TRCCONFIGR_DA | TRCCONFIGR_DV);
 
 	/* Disable all events tracing */
 	config->eventctrl0 = 0x0;
@@ -304,82 +304,82 @@ static ssize_t mode_store(struct device *dev,
 
 	if (drvdata->instrp0 == true) {
 		/* start by clearing instruction P0 field */
-		config->cfg  &= ~(BIT(1) | BIT(2));
+		config->cfg  &= ~TRCCONFIGR_INSTP0_LOAD_STORE;
 		if (config->mode & ETM_MODE_LOAD)
 			/* 0b01 Trace load instructions as P0 instructions */
-			config->cfg  |= BIT(1);
+			config->cfg  |= TRCCONFIGR_INSTP0_LOAD;
 		if (config->mode & ETM_MODE_STORE)
 			/* 0b10 Trace store instructions as P0 instructions */
-			config->cfg  |= BIT(2);
+			config->cfg  |= TRCCONFIGR_INSTP0_STORE;
 		if (config->mode & ETM_MODE_LOAD_STORE)
 			/*
 			 * 0b11 Trace load and store instructions
 			 * as P0 instructions
 			 */
-			config->cfg  |= BIT(1) | BIT(2);
+			config->cfg  |= TRCCONFIGR_INSTP0_LOAD_STORE;
 	}
 
 	/* bit[3], Branch broadcast mode */
 	if ((config->mode & ETM_MODE_BB) && (drvdata->trcbb == true))
-		config->cfg |= BIT(3);
+		config->cfg |= TRCCONFIGR_BB;
 	else
-		config->cfg &= ~BIT(3);
+		config->cfg &= ~TRCCONFIGR_BB;
 
 	/* bit[4], Cycle counting instruction trace bit */
 	if ((config->mode & ETMv4_MODE_CYCACC) &&
 		(drvdata->trccci == true))
-		config->cfg |= BIT(4);
+		config->cfg |= TRCCONFIGR_CCI;
 	else
-		config->cfg &= ~BIT(4);
+		config->cfg &= ~TRCCONFIGR_CCI;
 
 	/* bit[6], Context ID tracing bit */
 	if ((config->mode & ETMv4_MODE_CTXID) && (drvdata->ctxid_size))
-		config->cfg |= BIT(6);
+		config->cfg |= TRCCONFIGR_CID;
 	else
-		config->cfg &= ~BIT(6);
+		config->cfg &= ~TRCCONFIGR_CID;
 
 	if ((config->mode & ETM_MODE_VMID) && (drvdata->vmid_size))
-		config->cfg |= BIT(7);
+		config->cfg |= TRCCONFIGR_VMID;
 	else
-		config->cfg &= ~BIT(7);
+		config->cfg &= ~TRCCONFIGR_VMID;
 
 	/* bits[10:8], Conditional instruction tracing bit */
 	mode = ETM_MODE_COND(config->mode);
 	if (drvdata->trccond == true) {
-		config->cfg &= ~(BIT(8) | BIT(9) | BIT(10));
-		config->cfg |= mode << 8;
+		config->cfg &= ~TRCCONFIGR_COND_MASK;
+		config->cfg |= mode << __bf_shf(TRCCONFIGR_COND_MASK);
 	}
 
 	/* bit[11], Global timestamp tracing bit */
 	if ((config->mode & ETMv4_MODE_TIMESTAMP) && (drvdata->ts_size))
-		config->cfg |= BIT(11);
+		config->cfg |= TRCCONFIGR_TS;
 	else
-		config->cfg &= ~BIT(11);
+		config->cfg &= ~TRCCONFIGR_TS;
 
 	/* bit[12], Return stack enable bit */
 	if ((config->mode & ETM_MODE_RETURNSTACK) &&
 					(drvdata->retstack == true))
-		config->cfg |= BIT(12);
+		config->cfg |= TRCCONFIGR_RS;
 	else
-		config->cfg &= ~BIT(12);
+		config->cfg &= ~TRCCONFIGR_RS;
 
 	/* bits[14:13], Q element enable field */
 	mode = ETM_MODE_QELEM(config->mode);
 	/* start by clearing QE bits */
-	config->cfg &= ~(BIT(13) | BIT(14));
+	config->cfg &= ~(TRCCONFIGR_QE_W_COUNTS | TRCCONFIGR_QE_WO_COUNTS);
 	/*
 	 * if supported, Q elements with instruction counts are enabled.
 	 * Always set the low bit for any requested mode. Valid combos are
 	 * 0b00, 0b01 and 0b11.
 	 */
 	if (mode && drvdata->q_support)
-		config->cfg |= BIT(13);
+		config->cfg |= TRCCONFIGR_QE_W_COUNTS;
 	/*
 	 * if supported, Q elements with and without instruction
 	 * counts are enabled
 	 */
 	if ((mode & BIT(1)) && (drvdata->q_support & BIT(1)))
-		config->cfg |= BIT(14);
+		config->cfg |= TRCCONFIGR_QE_WO_COUNTS;
 
 	/* bit[11], AMBA Trace Bus (ATB) trigger enable bit */
 	if ((config->mode & ETM_MODE_ATB_TRIGGER) &&
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
index 3b604cde668b..4c8d7be3c159 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.h
+++ b/drivers/hwtracing/coresight/coresight-etm4x.h
@@ -172,6 +172,22 @@
 #define TRCIDR5_NUMSEQSTATE_MASK		GENMASK(27, 25)
 #define TRCIDR5_NUMCNTR_MASK			GENMASK(30, 28)
 
+#define TRCCONFIGR_INSTP0_LOAD			BIT(1)
+#define TRCCONFIGR_INSTP0_STORE			BIT(2)
+#define TRCCONFIGR_INSTP0_LOAD_STORE		(TRCCONFIGR_INSTP0_LOAD | TRCCONFIGR_INSTP0_STORE)
+#define TRCCONFIGR_BB				BIT(3)
+#define TRCCONFIGR_CCI				BIT(4)
+#define TRCCONFIGR_CID				BIT(6)
+#define TRCCONFIGR_VMID				BIT(7)
+#define TRCCONFIGR_COND_MASK			GENMASK(10, 8)
+#define TRCCONFIGR_TS				BIT(11)
+#define TRCCONFIGR_RS				BIT(12)
+#define TRCCONFIGR_QE_W_COUNTS			BIT(13)
+#define TRCCONFIGR_QE_WO_COUNTS			BIT(14)
+#define TRCCONFIGR_VMIDOPT			BIT(15)
+#define TRCCONFIGR_DA				BIT(16)
+#define TRCCONFIGR_DV				BIT(17)
+
 /*
  * System instructions to access ETM registers.
  * See ETMv4.4 spec ARM IHI0064F section 4.3.6 System instructions
-- 
2.28.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 06/15] coresight: etm4x: Cleanup TRCCONFIGR register accesses

[Mike Leach]

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          | 12 ++---
>  .../coresight/coresight-etm4x-sysfs.c         | 46 +++++++++----------
>  drivers/hwtracing/coresight/coresight-etm4x.h | 16 +++++++
>  3 files changed, 44 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index 3f4263117570..445e2057d5ed 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -633,7 +633,7 @@ static int etm4_parse_event_config(struct coresight_device *csdev,
>
>         /* Go from generic option to ETMv4 specifics */
>         if (attr->config & BIT(ETM_OPT_CYCACC)) {
> -               config->cfg |= BIT(4);
> +               config->cfg |= TRCCONFIGR_CCI;
>                 /* TRM: Must program this for cycacc to work */
>                 config->ccctlr = ETM_CYC_THRESHOLD_DEFAULT;
>         }
> @@ -653,14 +653,14 @@ static int etm4_parse_event_config(struct coresight_device *csdev,
>                         goto out;
>
>                 /* bit[11], Global timestamp tracing bit */
> -               config->cfg |= BIT(11);
> +               config->cfg |= TRCCONFIGR_TS;
>         }
>
>         /* Only trace contextID when runs in root PID namespace */
>         if ((attr->config & BIT(ETM_OPT_CTXTID)) &&
>             task_is_in_init_pid_ns(current))
>                 /* bit[6], Context ID tracing bit */
> -               config->cfg |= BIT(ETM4_CFG_BIT_CTXTID);
> +               config->cfg |= TRCCONFIGR_CID;
>
>         /*
>          * If set bit ETM_OPT_CTXTID2 in perf config, this asks to trace VMID
> @@ -672,17 +672,15 @@ static int etm4_parse_event_config(struct coresight_device *csdev,
>                         ret = -EINVAL;
>                         goto out;
>                 }
> -
>                 /* Only trace virtual contextID when runs in root PID namespace */
>                 if (task_is_in_init_pid_ns(current))
> -                       config->cfg |= BIT(ETM4_CFG_BIT_VMID) |
> -                                      BIT(ETM4_CFG_BIT_VMID_OPT);
> +                       config->cfg |= TRCCONFIGR_VMID | TRCCONFIGR_VMIDOPT;
>         }
>
>         /* return stack - enable if selected and supported */
>         if ((attr->config & BIT(ETM_OPT_RETSTK)) && drvdata->retstack)
>                 /* bit[12], Return stack enable bit */
> -               config->cfg |= BIT(12);
> +               config->cfg |= TRCCONFIGR_RS;
>
>         /*
>          * Set any selected configuration and preset.
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> index 21687cc1e4e2..53f84da3fe44 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> @@ -180,12 +180,12 @@ static ssize_t reset_store(struct device *dev,
>
>         /* Disable data tracing: do not trace load and store data transfers */
>         config->mode &= ~(ETM_MODE_LOAD | ETM_MODE_STORE);
> -       config->cfg &= ~(BIT(1) | BIT(2));
> +       config->cfg &= ~(TRCCONFIGR_INSTP0_LOAD | TRCCONFIGR_INSTP0_STORE);
>
>         /* Disable data value and data address tracing */
>         config->mode &= ~(ETM_MODE_DATA_TRACE_ADDR |
>                            ETM_MODE_DATA_TRACE_VAL);
> -       config->cfg &= ~(BIT(16) | BIT(17));
> +       config->cfg &= ~(TRCCONFIGR_DA | TRCCONFIGR_DV);
>
>         /* Disable all events tracing */
>         config->eventctrl0 = 0x0;
> @@ -304,82 +304,82 @@ static ssize_t mode_store(struct device *dev,
>
>         if (drvdata->instrp0 == true) {
>                 /* start by clearing instruction P0 field */
> -               config->cfg  &= ~(BIT(1) | BIT(2));
> +               config->cfg  &= ~TRCCONFIGR_INSTP0_LOAD_STORE;
>                 if (config->mode & ETM_MODE_LOAD)
>                         /* 0b01 Trace load instructions as P0 instructions */
> -                       config->cfg  |= BIT(1);
> +                       config->cfg  |= TRCCONFIGR_INSTP0_LOAD;
>                 if (config->mode & ETM_MODE_STORE)
>                         /* 0b10 Trace store instructions as P0 instructions */
> -                       config->cfg  |= BIT(2);
> +                       config->cfg  |= TRCCONFIGR_INSTP0_STORE;
>                 if (config->mode & ETM_MODE_LOAD_STORE)
>                         /*
>                          * 0b11 Trace load and store instructions
>                          * as P0 instructions
>                          */
> -                       config->cfg  |= BIT(1) | BIT(2);
> +                       config->cfg  |= TRCCONFIGR_INSTP0_LOAD_STORE;
>         }
>
>         /* bit[3], Branch broadcast mode */
>         if ((config->mode & ETM_MODE_BB) && (drvdata->trcbb == true))
> -               config->cfg |= BIT(3);
> +               config->cfg |= TRCCONFIGR_BB;
>         else
> -               config->cfg &= ~BIT(3);
> +               config->cfg &= ~TRCCONFIGR_BB;
>
>         /* bit[4], Cycle counting instruction trace bit */
>         if ((config->mode & ETMv4_MODE_CYCACC) &&
>                 (drvdata->trccci == true))
> -               config->cfg |= BIT(4);
> +               config->cfg |= TRCCONFIGR_CCI;
>         else
> -               config->cfg &= ~BIT(4);
> +               config->cfg &= ~TRCCONFIGR_CCI;
>
>         /* bit[6], Context ID tracing bit */
>         if ((config->mode & ETMv4_MODE_CTXID) && (drvdata->ctxid_size))
> -               config->cfg |= BIT(6);
> +               config->cfg |= TRCCONFIGR_CID;
>         else
> -               config->cfg &= ~BIT(6);
> +               config->cfg &= ~TRCCONFIGR_CID;
>
>         if ((config->mode & ETM_MODE_VMID) && (drvdata->vmid_size))
> -               config->cfg |= BIT(7);
> +               config->cfg |= TRCCONFIGR_VMID;
>         else
> -               config->cfg &= ~BIT(7);
> +               config->cfg &= ~TRCCONFIGR_VMID;
>
>         /* bits[10:8], Conditional instruction tracing bit */
>         mode = ETM_MODE_COND(config->mode);
>         if (drvdata->trccond == true) {
> -               config->cfg &= ~(BIT(8) | BIT(9) | BIT(10));
> -               config->cfg |= mode << 8;
> +               config->cfg &= ~TRCCONFIGR_COND_MASK;
> +               config->cfg |= mode << __bf_shf(TRCCONFIGR_COND_MASK);
>         }
>
>         /* bit[11], Global timestamp tracing bit */
>         if ((config->mode & ETMv4_MODE_TIMESTAMP) && (drvdata->ts_size))
> -               config->cfg |= BIT(11);
> +               config->cfg |= TRCCONFIGR_TS;
>         else
> -               config->cfg &= ~BIT(11);
> +               config->cfg &= ~TRCCONFIGR_TS;
>
>         /* bit[12], Return stack enable bit */
>         if ((config->mode & ETM_MODE_RETURNSTACK) &&
>                                         (drvdata->retstack == true))
> -               config->cfg |= BIT(12);
> +               config->cfg |= TRCCONFIGR_RS;
>         else
> -               config->cfg &= ~BIT(12);
> +               config->cfg &= ~TRCCONFIGR_RS;
>
>         /* bits[14:13], Q element enable field */
>         mode = ETM_MODE_QELEM(config->mode);
>         /* start by clearing QE bits */
> -       config->cfg &= ~(BIT(13) | BIT(14));
> +       config->cfg &= ~(TRCCONFIGR_QE_W_COUNTS | TRCCONFIGR_QE_WO_COUNTS);
>         /*
>          * if supported, Q elements with instruction counts are enabled.
>          * Always set the low bit for any requested mode. Valid combos are
>          * 0b00, 0b01 and 0b11.
>          */
>         if (mode && drvdata->q_support)
> -               config->cfg |= BIT(13);
> +               config->cfg |= TRCCONFIGR_QE_W_COUNTS;
>         /*
>          * if supported, Q elements with and without instruction
>          * counts are enabled
>          */
>         if ((mode & BIT(1)) && (drvdata->q_support & BIT(1)))
> -               config->cfg |= BIT(14);
> +               config->cfg |= TRCCONFIGR_QE_WO_COUNTS;
>
>         /* bit[11], AMBA Trace Bus (ATB) trigger enable bit */
>         if ((config->mode & ETM_MODE_ATB_TRIGGER) &&
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
> index 3b604cde668b..4c8d7be3c159 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x.h
> +++ b/drivers/hwtracing/coresight/coresight-etm4x.h
> @@ -172,6 +172,22 @@
>  #define TRCIDR5_NUMSEQSTATE_MASK               GENMASK(27, 25)
>  #define TRCIDR5_NUMCNTR_MASK                   GENMASK(30, 28)
>
> +#define TRCCONFIGR_INSTP0_LOAD                 BIT(1)
> +#define TRCCONFIGR_INSTP0_STORE                        BIT(2)
> +#define TRCCONFIGR_INSTP0_LOAD_STORE           (TRCCONFIGR_INSTP0_LOAD | TRCCONFIGR_INSTP0_STORE)
> +#define TRCCONFIGR_BB                          BIT(3)
> +#define TRCCONFIGR_CCI                         BIT(4)
> +#define TRCCONFIGR_CID                         BIT(6)
> +#define TRCCONFIGR_VMID                                BIT(7)
> +#define TRCCONFIGR_COND_MASK                   GENMASK(10, 8)
> +#define TRCCONFIGR_TS                          BIT(11)
> +#define TRCCONFIGR_RS                          BIT(12)
> +#define TRCCONFIGR_QE_W_COUNTS                 BIT(13)
> +#define TRCCONFIGR_QE_WO_COUNTS                        BIT(14)
> +#define TRCCONFIGR_VMIDOPT                     BIT(15)
> +#define TRCCONFIGR_DA                          BIT(16)
> +#define TRCCONFIGR_DV                          BIT(17)
> +
>  /*
>   * 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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 07/15] coresight: etm4x: Cleanup TRCEVENTCTL1R register accesses

[James Clark]

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-sysfs.c         | 25 +++++++++++--------
 drivers/hwtracing/coresight/coresight-etm4x.h |  8 ++++++
 2 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
index 53f84da3fe44..2d29e9daf515 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
@@ -384,16 +384,16 @@ static ssize_t mode_store(struct device *dev,
 	/* bit[11], AMBA Trace Bus (ATB) trigger enable bit */
 	if ((config->mode & ETM_MODE_ATB_TRIGGER) &&
 	    (drvdata->atbtrig == true))
-		config->eventctrl1 |= BIT(11);
+		config->eventctrl1 |= TRCEVENTCTL1R_ATB;
 	else
-		config->eventctrl1 &= ~BIT(11);
+		config->eventctrl1 &= ~TRCEVENTCTL1R_ATB;
 
 	/* bit[12], Low-power state behavior override bit */
 	if ((config->mode & ETM_MODE_LPOVERRIDE) &&
 	    (drvdata->lpoverride == true))
-		config->eventctrl1 |= BIT(12);
+		config->eventctrl1 |= TRCEVENTCTL1R_LPOVERRIDE;
 	else
-		config->eventctrl1 &= ~BIT(12);
+		config->eventctrl1 &= ~TRCEVENTCTL1R_LPOVERRIDE;
 
 	/* bit[8], Instruction stall bit */
 	if ((config->mode & ETM_MODE_ISTALL_EN) && (drvdata->stallctl == true))
@@ -534,7 +534,7 @@ static ssize_t event_instren_show(struct device *dev,
 	struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
 	struct etmv4_config *config = &drvdata->config;
 
-	val = BMVAL(config->eventctrl1, 0, 3);
+	val = FIELD_GET(TRCEVENTCTL1R_INSTEN_MASK, config->eventctrl1);
 	return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
 }
 
@@ -551,23 +551,28 @@ static ssize_t event_instren_store(struct device *dev,
 
 	spin_lock(&drvdata->spinlock);
 	/* start by clearing all instruction event enable bits */
-	config->eventctrl1 &= ~(BIT(0) | BIT(1) | BIT(2) | BIT(3));
+	config->eventctrl1 &= ~TRCEVENTCTL1R_INSTEN_MASK;
 	switch (drvdata->nr_event) {
 	case 0x0:
 		/* generate Event element for event 1 */
-		config->eventctrl1 |= val & BIT(1);
+		config->eventctrl1 |= val & TRCEVENTCTL1R_INSTEN_1;
 		break;
 	case 0x1:
 		/* generate Event element for event 1 and 2 */
-		config->eventctrl1 |= val & (BIT(0) | BIT(1));
+		config->eventctrl1 |= val & (TRCEVENTCTL1R_INSTEN_0 | TRCEVENTCTL1R_INSTEN_1);
 		break;
 	case 0x2:
 		/* generate Event element for event 1, 2 and 3 */
-		config->eventctrl1 |= val & (BIT(0) | BIT(1) | BIT(2));
+		config->eventctrl1 |= val & (TRCEVENTCTL1R_INSTEN_0 |
+					     TRCEVENTCTL1R_INSTEN_1 |
+					     TRCEVENTCTL1R_INSTEN_2);
 		break;
 	case 0x3:
 		/* generate Event element for all 4 events */
-		config->eventctrl1 |= val & 0xF;
+		config->eventctrl1 |= val & (TRCEVENTCTL1R_INSTEN_0 |
+					     TRCEVENTCTL1R_INSTEN_1 |
+					     TRCEVENTCTL1R_INSTEN_2 |
+					     TRCEVENTCTL1R_INSTEN_3);
 		break;
 	default:
 		break;
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
index 4c8d7be3c159..cbba46f14ada 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.h
+++ b/drivers/hwtracing/coresight/coresight-etm4x.h
@@ -188,6 +188,14 @@
 #define TRCCONFIGR_DA				BIT(16)
 #define TRCCONFIGR_DV				BIT(17)
 
+#define TRCEVENTCTL1R_INSTEN_MASK		GENMASK(3, 0)
+#define TRCEVENTCTL1R_INSTEN_0			BIT(0)
+#define TRCEVENTCTL1R_INSTEN_1			BIT(1)
+#define TRCEVENTCTL1R_INSTEN_2			BIT(2)
+#define TRCEVENTCTL1R_INSTEN_3			BIT(3)
+#define TRCEVENTCTL1R_ATB			BIT(11)
+#define TRCEVENTCTL1R_LPOVERRIDE		BIT(12)
+
 /*
  * System instructions to access ETM registers.
  * See ETMv4.4 spec ARM IHI0064F section 4.3.6 System instructions
-- 
2.28.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 07/15] coresight: etm4x: Cleanup TRCEVENTCTL1R register accesses

[Mike Leach]

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-sysfs.c         | 25 +++++++++++--------
>  drivers/hwtracing/coresight/coresight-etm4x.h |  8 ++++++
>  2 files changed, 23 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> index 53f84da3fe44..2d29e9daf515 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> @@ -384,16 +384,16 @@ static ssize_t mode_store(struct device *dev,
>         /* bit[11], AMBA Trace Bus (ATB) trigger enable bit */
>         if ((config->mode & ETM_MODE_ATB_TRIGGER) &&
>             (drvdata->atbtrig == true))
> -               config->eventctrl1 |= BIT(11);
> +               config->eventctrl1 |= TRCEVENTCTL1R_ATB;
>         else
> -               config->eventctrl1 &= ~BIT(11);
> +               config->eventctrl1 &= ~TRCEVENTCTL1R_ATB;
>
>         /* bit[12], Low-power state behavior override bit */
>         if ((config->mode & ETM_MODE_LPOVERRIDE) &&
>             (drvdata->lpoverride == true))
> -               config->eventctrl1 |= BIT(12);
> +               config->eventctrl1 |= TRCEVENTCTL1R_LPOVERRIDE;
>         else
> -               config->eventctrl1 &= ~BIT(12);
> +               config->eventctrl1 &= ~TRCEVENTCTL1R_LPOVERRIDE;
>
>         /* bit[8], Instruction stall bit */
>         if ((config->mode & ETM_MODE_ISTALL_EN) && (drvdata->stallctl == true))
> @@ -534,7 +534,7 @@ static ssize_t event_instren_show(struct device *dev,
>         struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
>         struct etmv4_config *config = &drvdata->config;
>
> -       val = BMVAL(config->eventctrl1, 0, 3);
> +       val = FIELD_GET(TRCEVENTCTL1R_INSTEN_MASK, config->eventctrl1);
>         return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
>  }
>
> @@ -551,23 +551,28 @@ static ssize_t event_instren_store(struct device *dev,
>
>         spin_lock(&drvdata->spinlock);
>         /* start by clearing all instruction event enable bits */
> -       config->eventctrl1 &= ~(BIT(0) | BIT(1) | BIT(2) | BIT(3));
> +       config->eventctrl1 &= ~TRCEVENTCTL1R_INSTEN_MASK;
>         switch (drvdata->nr_event) {
>         case 0x0:
>                 /* generate Event element for event 1 */
> -               config->eventctrl1 |= val & BIT(1);
> +               config->eventctrl1 |= val & TRCEVENTCTL1R_INSTEN_1;
>                 break;
>         case 0x1:
>                 /* generate Event element for event 1 and 2 */
> -               config->eventctrl1 |= val & (BIT(0) | BIT(1));
> +               config->eventctrl1 |= val & (TRCEVENTCTL1R_INSTEN_0 | TRCEVENTCTL1R_INSTEN_1);
>                 break;
>         case 0x2:
>                 /* generate Event element for event 1, 2 and 3 */
> -               config->eventctrl1 |= val & (BIT(0) | BIT(1) | BIT(2));
> +               config->eventctrl1 |= val & (TRCEVENTCTL1R_INSTEN_0 |
> +                                            TRCEVENTCTL1R_INSTEN_1 |
> +                                            TRCEVENTCTL1R_INSTEN_2);
>                 break;
>         case 0x3:
>                 /* generate Event element for all 4 events */
> -               config->eventctrl1 |= val & 0xF;
> +               config->eventctrl1 |= val & (TRCEVENTCTL1R_INSTEN_0 |
> +                                            TRCEVENTCTL1R_INSTEN_1 |
> +                                            TRCEVENTCTL1R_INSTEN_2 |
> +                                            TRCEVENTCTL1R_INSTEN_3);
>                 break;
>         default:
>                 break;
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
> index 4c8d7be3c159..cbba46f14ada 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x.h
> +++ b/drivers/hwtracing/coresight/coresight-etm4x.h
> @@ -188,6 +188,14 @@
>  #define TRCCONFIGR_DA                          BIT(16)
>  #define TRCCONFIGR_DV                          BIT(17)
>
> +#define TRCEVENTCTL1R_INSTEN_MASK              GENMASK(3, 0)
> +#define TRCEVENTCTL1R_INSTEN_0                 BIT(0)
> +#define TRCEVENTCTL1R_INSTEN_1                 BIT(1)
> +#define TRCEVENTCTL1R_INSTEN_2                 BIT(2)
> +#define TRCEVENTCTL1R_INSTEN_3                 BIT(3)
> +#define TRCEVENTCTL1R_ATB                      BIT(11)
> +#define TRCEVENTCTL1R_LPOVERRIDE               BIT(12)
> +
>  /*
>   * 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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 08/15] coresight: etm4x: Cleanup TRCSTALLCTLR register accesses

[James Clark]

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>
---
 drivers/hwtracing/coresight/coresight-etm4x-sysfs.c | 12 ++++++------
 drivers/hwtracing/coresight/coresight-etm4x.h       |  4 ++++
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
index 2d29e9daf515..cd24590ea38a 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
@@ -397,22 +397,22 @@ static ssize_t mode_store(struct device *dev,
 
 	/* bit[8], Instruction stall bit */
 	if ((config->mode & ETM_MODE_ISTALL_EN) && (drvdata->stallctl == true))
-		config->stall_ctrl |= BIT(8);
+		config->stall_ctrl |= TRCSTALLCTLR_ISTALL;
 	else
-		config->stall_ctrl &= ~BIT(8);
+		config->stall_ctrl &= ~TRCSTALLCTLR_ISTALL;
 
 	/* bit[10], Prioritize instruction trace bit */
 	if (config->mode & ETM_MODE_INSTPRIO)
-		config->stall_ctrl |= BIT(10);
+		config->stall_ctrl |= TRCSTALLCTLR_INSTPRIORITY;
 	else
-		config->stall_ctrl &= ~BIT(10);
+		config->stall_ctrl &= ~TRCSTALLCTLR_INSTPRIORITY;
 
 	/* bit[13], Trace overflow prevention bit */
 	if ((config->mode & ETM_MODE_NOOVERFLOW) &&
 		(drvdata->nooverflow == true))
-		config->stall_ctrl |= BIT(13);
+		config->stall_ctrl |= TRCSTALLCTLR_NOOVERFLOW;
 	else
-		config->stall_ctrl &= ~BIT(13);
+		config->stall_ctrl &= ~TRCSTALLCTLR_NOOVERFLOW;
 
 	/* bit[9] Start/stop logic control bit */
 	if (config->mode & ETM_MODE_VIEWINST_STARTSTOP)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
index cbba46f14ada..36934056a5dc 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.h
+++ b/drivers/hwtracing/coresight/coresight-etm4x.h
@@ -196,6 +196,10 @@
 #define TRCEVENTCTL1R_ATB			BIT(11)
 #define TRCEVENTCTL1R_LPOVERRIDE		BIT(12)
 
+#define TRCSTALLCTLR_ISTALL			BIT(8)
+#define TRCSTALLCTLR_INSTPRIORITY		BIT(10)
+#define TRCSTALLCTLR_NOOVERFLOW			BIT(13)
+
 /*
  * System instructions to access ETM registers.
  * See ETMv4.4 spec ARM IHI0064F section 4.3.6 System instructions
-- 
2.28.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 08/15] coresight: etm4x: Cleanup TRCSTALLCTLR register accesses

[Mike Leach]

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>
> ---
>  drivers/hwtracing/coresight/coresight-etm4x-sysfs.c | 12 ++++++------
>  drivers/hwtracing/coresight/coresight-etm4x.h       |  4 ++++
>  2 files changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> index 2d29e9daf515..cd24590ea38a 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> @@ -397,22 +397,22 @@ static ssize_t mode_store(struct device *dev,
>
>         /* bit[8], Instruction stall bit */
>         if ((config->mode & ETM_MODE_ISTALL_EN) && (drvdata->stallctl == true))
> -               config->stall_ctrl |= BIT(8);
> +               config->stall_ctrl |= TRCSTALLCTLR_ISTALL;
>         else
> -               config->stall_ctrl &= ~BIT(8);
> +               config->stall_ctrl &= ~TRCSTALLCTLR_ISTALL;
>
>         /* bit[10], Prioritize instruction trace bit */
>         if (config->mode & ETM_MODE_INSTPRIO)
> -               config->stall_ctrl |= BIT(10);
> +               config->stall_ctrl |= TRCSTALLCTLR_INSTPRIORITY;
>         else
> -               config->stall_ctrl &= ~BIT(10);
> +               config->stall_ctrl &= ~TRCSTALLCTLR_INSTPRIORITY;
>
>         /* bit[13], Trace overflow prevention bit */
>         if ((config->mode & ETM_MODE_NOOVERFLOW) &&
>                 (drvdata->nooverflow == true))
> -               config->stall_ctrl |= BIT(13);
> +               config->stall_ctrl |= TRCSTALLCTLR_NOOVERFLOW;
>         else
> -               config->stall_ctrl &= ~BIT(13);
> +               config->stall_ctrl &= ~TRCSTALLCTLR_NOOVERFLOW;
>
>         /* bit[9] Start/stop logic control bit */
>         if (config->mode & ETM_MODE_VIEWINST_STARTSTOP)
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
> index cbba46f14ada..36934056a5dc 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x.h
> +++ b/drivers/hwtracing/coresight/coresight-etm4x.h
> @@ -196,6 +196,10 @@
>  #define TRCEVENTCTL1R_ATB                      BIT(11)
>  #define TRCEVENTCTL1R_LPOVERRIDE               BIT(12)
>
> +#define TRCSTALLCTLR_ISTALL                    BIT(8)
> +#define TRCSTALLCTLR_INSTPRIORITY              BIT(10)
> +#define TRCSTALLCTLR_NOOVERFLOW                        BIT(13)
> +
>  /*
>   * 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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 09/15] coresight: etm4x: Cleanup TRCVICTLR register accesses

[James Clark]

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          | 10 +++---
 .../coresight/coresight-etm4x-sysfs.c         | 36 +++++++++----------
 drivers/hwtracing/coresight/coresight-etm4x.h | 22 +++++-------
 3 files changed, 31 insertions(+), 37 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index 445e2057d5ed..88353f8ba414 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -1206,7 +1206,7 @@ static void etm4_init_arch_data(void *info)
 
 static inline u32 etm4_get_victlr_access_type(struct etmv4_config *config)
 {
-	return etm4_get_access_type(config) << TRCVICTLR_EXLEVEL_SHIFT;
+	return etm4_get_access_type(config) << __bf_shf(TRCVICTLR_EXLEVEL_MASK);
 }
 
 /* Set ELx trace filter access in the TRCVICTLR register */
@@ -1232,7 +1232,7 @@ static void etm4_set_default_config(struct etmv4_config *config)
 	config->ts_ctrl = 0x0;
 
 	/* TRCVICTLR::EVENT = 0x01, select the always on logic */
-	config->vinst_ctrl = BIT(0);
+	config->vinst_ctrl = FIELD_PREP(TRCVICTLR_EVENT_MASK, 0x01);
 
 	/* TRCVICTLR::EXLEVEL_NS:EXLEVELS: Set kernel / user filtering */
 	etm4_set_victlr_access(config);
@@ -1341,7 +1341,7 @@ static void etm4_set_default_filter(struct etmv4_config *config)
 	 * TRCVICTLR::SSSTATUS == 1, the start-stop logic is
 	 * in the started state
 	 */
-	config->vinst_ctrl |= BIT(9);
+	config->vinst_ctrl |= TRCVICTLR_SSSTATUS;
 	config->mode |= ETM_MODE_VIEWINST_STARTSTOP;
 
 	/* No start-stop filtering for ViewInst */
@@ -1445,7 +1445,7 @@ static int etm4_set_event_filters(struct etmv4_drvdata *drvdata,
 			 * TRCVICTLR::SSSTATUS == 1, the start-stop logic is
 			 * in the started state
 			 */
-			config->vinst_ctrl |= BIT(9);
+			config->vinst_ctrl |= TRCVICTLR_SSSTATUS;
 
 			/* No start-stop filtering for ViewInst */
 			config->vissctlr = 0x0;
@@ -1473,7 +1473,7 @@ static int etm4_set_event_filters(struct etmv4_drvdata *drvdata,
 			 * etm4_disable_perf().
 			 */
 			if (filters->ssstatus)
-				config->vinst_ctrl |= BIT(9);
+				config->vinst_ctrl |= TRCVICTLR_SSSTATUS;
 
 			/* No include/exclude filtering for ViewInst */
 			config->viiectlr = 0x0;
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
index cd24590ea38a..b3b1b92909cc 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
@@ -206,11 +206,11 @@ static ssize_t reset_store(struct device *dev,
 	 * started state. ARM recommends start-stop logic is set before
 	 * each trace run.
 	 */
-	config->vinst_ctrl = BIT(0);
+	config->vinst_ctrl = FIELD_PREP(TRCVICTLR_EVENT_MASK, 0x01);
 	if (drvdata->nr_addr_cmp > 0) {
 		config->mode |= ETM_MODE_VIEWINST_STARTSTOP;
 		/* SSSTATUS, bit[9] */
-		config->vinst_ctrl |= BIT(9);
+		config->vinst_ctrl |= TRCVICTLR_SSSTATUS;
 	}
 
 	/* No address range filtering for ViewInst */
@@ -416,22 +416,22 @@ static ssize_t mode_store(struct device *dev,
 
 	/* bit[9] Start/stop logic control bit */
 	if (config->mode & ETM_MODE_VIEWINST_STARTSTOP)
-		config->vinst_ctrl |= BIT(9);
+		config->vinst_ctrl |= TRCVICTLR_SSSTATUS;
 	else
-		config->vinst_ctrl &= ~BIT(9);
+		config->vinst_ctrl &= ~TRCVICTLR_SSSTATUS;
 
 	/* bit[10], Whether a trace unit must trace a Reset exception */
 	if (config->mode & ETM_MODE_TRACE_RESET)
-		config->vinst_ctrl |= BIT(10);
+		config->vinst_ctrl |= TRCVICTLR_TRCRESET;
 	else
-		config->vinst_ctrl &= ~BIT(10);
+		config->vinst_ctrl &= ~TRCVICTLR_TRCRESET;
 
 	/* bit[11], Whether a trace unit must trace a system error exception */
 	if ((config->mode & ETM_MODE_TRACE_ERR) &&
 		(drvdata->trc_error == true))
-		config->vinst_ctrl |= BIT(11);
+		config->vinst_ctrl |= TRCVICTLR_TRCERR;
 	else
-		config->vinst_ctrl &= ~BIT(11);
+		config->vinst_ctrl &= ~TRCVICTLR_TRCERR;
 
 	if (config->mode & (ETM_MODE_EXCL_KERN | ETM_MODE_EXCL_USER))
 		etm4_config_trace_mode(config);
@@ -723,7 +723,7 @@ static ssize_t event_vinst_show(struct device *dev,
 	struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
 	struct etmv4_config *config = &drvdata->config;
 
-	val = config->vinst_ctrl & ETMv4_EVENT_MASK;
+	val = FIELD_GET(TRCVICTLR_EVENT_MASK, config->vinst_ctrl);
 	return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
 }
 
@@ -739,9 +739,9 @@ static ssize_t event_vinst_store(struct device *dev,
 		return -EINVAL;
 
 	spin_lock(&drvdata->spinlock);
-	val &= ETMv4_EVENT_MASK;
-	config->vinst_ctrl &= ~ETMv4_EVENT_MASK;
-	config->vinst_ctrl |= val;
+	val &= TRCVICTLR_EVENT_MASK >> __bf_shf(TRCVICTLR_EVENT_MASK);
+	config->vinst_ctrl &= ~TRCVICTLR_EVENT_MASK;
+	config->vinst_ctrl |= FIELD_PREP(TRCVICTLR_EVENT_MASK, val);
 	spin_unlock(&drvdata->spinlock);
 	return size;
 }
@@ -755,7 +755,7 @@ static ssize_t s_exlevel_vinst_show(struct device *dev,
 	struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
 	struct etmv4_config *config = &drvdata->config;
 
-	val = (config->vinst_ctrl & TRCVICTLR_EXLEVEL_S_MASK) >> TRCVICTLR_EXLEVEL_S_SHIFT;
+	val = FIELD_GET(TRCVICTLR_EXLEVEL_S_MASK, config->vinst_ctrl);
 	return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
 }
 
@@ -772,10 +772,10 @@ static ssize_t s_exlevel_vinst_store(struct device *dev,
 
 	spin_lock(&drvdata->spinlock);
 	/* clear all EXLEVEL_S bits  */
-	config->vinst_ctrl &= ~(TRCVICTLR_EXLEVEL_S_MASK);
+	config->vinst_ctrl &= ~TRCVICTLR_EXLEVEL_S_MASK;
 	/* enable instruction tracing for corresponding exception level */
 	val &= drvdata->s_ex_level;
-	config->vinst_ctrl |= (val << TRCVICTLR_EXLEVEL_S_SHIFT);
+	config->vinst_ctrl |= val << __bf_shf(TRCVICTLR_EXLEVEL_S_MASK);
 	spin_unlock(&drvdata->spinlock);
 	return size;
 }
@@ -790,7 +790,7 @@ static ssize_t ns_exlevel_vinst_show(struct device *dev,
 	struct etmv4_config *config = &drvdata->config;
 
 	/* EXLEVEL_NS, bits[23:20] */
-	val = (config->vinst_ctrl & TRCVICTLR_EXLEVEL_NS_MASK) >> TRCVICTLR_EXLEVEL_NS_SHIFT;
+	val = FIELD_GET(TRCVICTLR_EXLEVEL_NS_MASK, config->vinst_ctrl);
 	return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
 }
 
@@ -807,10 +807,10 @@ static ssize_t ns_exlevel_vinst_store(struct device *dev,
 
 	spin_lock(&drvdata->spinlock);
 	/* clear EXLEVEL_NS bits  */
-	config->vinst_ctrl &= ~(TRCVICTLR_EXLEVEL_NS_MASK);
+	config->vinst_ctrl &= ~TRCVICTLR_EXLEVEL_NS_MASK;
 	/* enable instruction tracing for corresponding exception level */
 	val &= drvdata->ns_ex_level;
-	config->vinst_ctrl |= (val << TRCVICTLR_EXLEVEL_NS_SHIFT);
+	config->vinst_ctrl |= val << __bf_shf(TRCVICTLR_EXLEVEL_NS_MASK);
 	spin_unlock(&drvdata->spinlock);
 	return size;
 }
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
index 36934056a5dc..9cacc38b1890 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.h
+++ b/drivers/hwtracing/coresight/coresight-etm4x.h
@@ -200,6 +200,14 @@
 #define TRCSTALLCTLR_INSTPRIORITY		BIT(10)
 #define TRCSTALLCTLR_NOOVERFLOW			BIT(13)
 
+#define TRCVICTLR_EVENT_MASK			GENMASK(7, 0)
+#define TRCVICTLR_SSSTATUS			BIT(9)
+#define TRCVICTLR_TRCRESET			BIT(10)
+#define TRCVICTLR_TRCERR			BIT(11)
+#define TRCVICTLR_EXLEVEL_MASK			GENMASK(22, 16)
+#define TRCVICTLR_EXLEVEL_S_MASK		GENMASK(19, 16)
+#define TRCVICTLR_EXLEVEL_NS_MASK		GENMASK(22, 20)
+
 /*
  * System instructions to access ETM registers.
  * See ETMv4.4 spec ARM IHI0064F section 4.3.6 System instructions
@@ -700,23 +708,9 @@
 #define ETM_EXLEVEL_NS_OS		BIT(5)	/* NonSecure EL1	*/
 #define ETM_EXLEVEL_NS_HYP		BIT(6)	/* NonSecure EL2	*/
 
-#define ETM_EXLEVEL_MASK		(GENMASK(6, 0))
-#define ETM_EXLEVEL_S_MASK		(GENMASK(3, 0))
-#define ETM_EXLEVEL_NS_MASK		(GENMASK(6, 4))
-
 /* access level controls in TRCACATRn */
 #define TRCACATR_EXLEVEL_SHIFT		8
 
-/* access level control in TRCVICTLR */
-#define TRCVICTLR_EXLEVEL_SHIFT		16
-#define TRCVICTLR_EXLEVEL_S_SHIFT	16
-#define TRCVICTLR_EXLEVEL_NS_SHIFT	20
-
-/* secure / non secure masks - TRCVICTLR, IDR3 */
-#define TRCVICTLR_EXLEVEL_MASK		(ETM_EXLEVEL_MASK << TRCVICTLR_EXLEVEL_SHIFT)
-#define TRCVICTLR_EXLEVEL_S_MASK	(ETM_EXLEVEL_S_MASK << TRCVICTLR_EXLEVEL_SHIFT)
-#define TRCVICTLR_EXLEVEL_NS_MASK	(ETM_EXLEVEL_NS_MASK << TRCVICTLR_EXLEVEL_SHIFT)
-
 #define ETM_TRCIDR1_ARCH_MAJOR_SHIFT	8
 #define ETM_TRCIDR1_ARCH_MAJOR_MASK	(0xfU << ETM_TRCIDR1_ARCH_MAJOR_SHIFT)
 #define ETM_TRCIDR1_ARCH_MAJOR(x)	\
-- 
2.28.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 09/15] coresight: etm4x: Cleanup TRCVICTLR register accesses

[Mathieu Poirier]

Hi James,

On Fri, Mar 04, 2022 at 05:19:06PM +0000, James Clark 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          | 10 +++---
>  .../coresight/coresight-etm4x-sysfs.c         | 36 +++++++++----------
>  drivers/hwtracing/coresight/coresight-etm4x.h | 22 +++++-------
>  3 files changed, 31 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index 445e2057d5ed..88353f8ba414 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -1206,7 +1206,7 @@ static void etm4_init_arch_data(void *info)
>  
>  static inline u32 etm4_get_victlr_access_type(struct etmv4_config *config)
>  {
> -	return etm4_get_access_type(config) << TRCVICTLR_EXLEVEL_SHIFT;
> +	return etm4_get_access_type(config) << __bf_shf(TRCVICTLR_EXLEVEL_MASK);
>  }
>  
>  /* Set ELx trace filter access in the TRCVICTLR register */
> @@ -1232,7 +1232,7 @@ static void etm4_set_default_config(struct etmv4_config *config)
>  	config->ts_ctrl = 0x0;
>  
>  	/* TRCVICTLR::EVENT = 0x01, select the always on logic */
> -	config->vinst_ctrl = BIT(0);
> +	config->vinst_ctrl = FIELD_PREP(TRCVICTLR_EVENT_MASK, 0x01);
>  
>  	/* TRCVICTLR::EXLEVEL_NS:EXLEVELS: Set kernel / user filtering */
>  	etm4_set_victlr_access(config);
> @@ -1341,7 +1341,7 @@ static void etm4_set_default_filter(struct etmv4_config *config)
>  	 * TRCVICTLR::SSSTATUS == 1, the start-stop logic is
>  	 * in the started state
>  	 */
> -	config->vinst_ctrl |= BIT(9);
> +	config->vinst_ctrl |= TRCVICTLR_SSSTATUS;
>  	config->mode |= ETM_MODE_VIEWINST_STARTSTOP;
>  
>  	/* No start-stop filtering for ViewInst */
> @@ -1445,7 +1445,7 @@ static int etm4_set_event_filters(struct etmv4_drvdata *drvdata,
>  			 * TRCVICTLR::SSSTATUS == 1, the start-stop logic is
>  			 * in the started state
>  			 */
> -			config->vinst_ctrl |= BIT(9);
> +			config->vinst_ctrl |= TRCVICTLR_SSSTATUS;
>  
>  			/* No start-stop filtering for ViewInst */
>  			config->vissctlr = 0x0;
> @@ -1473,7 +1473,7 @@ static int etm4_set_event_filters(struct etmv4_drvdata *drvdata,
>  			 * etm4_disable_perf().
>  			 */
>  			if (filters->ssstatus)
> -				config->vinst_ctrl |= BIT(9);
> +				config->vinst_ctrl |= TRCVICTLR_SSSTATUS;
>  
>  			/* No include/exclude filtering for ViewInst */
>  			config->viiectlr = 0x0;
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> index cd24590ea38a..b3b1b92909cc 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> @@ -206,11 +206,11 @@ static ssize_t reset_store(struct device *dev,
>  	 * started state. ARM recommends start-stop logic is set before
>  	 * each trace run.
>  	 */
> -	config->vinst_ctrl = BIT(0);
> +	config->vinst_ctrl = FIELD_PREP(TRCVICTLR_EVENT_MASK, 0x01);
>  	if (drvdata->nr_addr_cmp > 0) {
>  		config->mode |= ETM_MODE_VIEWINST_STARTSTOP;
>  		/* SSSTATUS, bit[9] */
> -		config->vinst_ctrl |= BIT(9);
> +		config->vinst_ctrl |= TRCVICTLR_SSSTATUS;
>  	}
>  
>  	/* No address range filtering for ViewInst */
> @@ -416,22 +416,22 @@ static ssize_t mode_store(struct device *dev,
>  
>  	/* bit[9] Start/stop logic control bit */
>  	if (config->mode & ETM_MODE_VIEWINST_STARTSTOP)
> -		config->vinst_ctrl |= BIT(9);
> +		config->vinst_ctrl |= TRCVICTLR_SSSTATUS;
>  	else
> -		config->vinst_ctrl &= ~BIT(9);
> +		config->vinst_ctrl &= ~TRCVICTLR_SSSTATUS;
>  
>  	/* bit[10], Whether a trace unit must trace a Reset exception */
>  	if (config->mode & ETM_MODE_TRACE_RESET)
> -		config->vinst_ctrl |= BIT(10);
> +		config->vinst_ctrl |= TRCVICTLR_TRCRESET;
>  	else
> -		config->vinst_ctrl &= ~BIT(10);
> +		config->vinst_ctrl &= ~TRCVICTLR_TRCRESET;
>  
>  	/* bit[11], Whether a trace unit must trace a system error exception */
>  	if ((config->mode & ETM_MODE_TRACE_ERR) &&
>  		(drvdata->trc_error == true))
> -		config->vinst_ctrl |= BIT(11);
> +		config->vinst_ctrl |= TRCVICTLR_TRCERR;
>  	else
> -		config->vinst_ctrl &= ~BIT(11);
> +		config->vinst_ctrl &= ~TRCVICTLR_TRCERR;
>  
>  	if (config->mode & (ETM_MODE_EXCL_KERN | ETM_MODE_EXCL_USER))
>  		etm4_config_trace_mode(config);
> @@ -723,7 +723,7 @@ static ssize_t event_vinst_show(struct device *dev,
>  	struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
>  	struct etmv4_config *config = &drvdata->config;
>  
> -	val = config->vinst_ctrl & ETMv4_EVENT_MASK;
> +	val = FIELD_GET(TRCVICTLR_EVENT_MASK, config->vinst_ctrl);
>  	return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
>  }
>  
> @@ -739,9 +739,9 @@ static ssize_t event_vinst_store(struct device *dev,
>  		return -EINVAL;
>  
>  	spin_lock(&drvdata->spinlock);
> -	val &= ETMv4_EVENT_MASK;
> -	config->vinst_ctrl &= ~ETMv4_EVENT_MASK;
> -	config->vinst_ctrl |= val;
> +	val &= TRCVICTLR_EVENT_MASK >> __bf_shf(TRCVICTLR_EVENT_MASK);

Not sure why the right-shifting operation is needed since the mask starts at bit
0.  Please consider fixing _if_ you end up respinning this.

Thanks,
Mathieu

> +	config->vinst_ctrl &= ~TRCVICTLR_EVENT_MASK;
> +	config->vinst_ctrl |= FIELD_PREP(TRCVICTLR_EVENT_MASK, val);
>  	spin_unlock(&drvdata->spinlock);
>  	return size;
>  }
> @@ -755,7 +755,7 @@ static ssize_t s_exlevel_vinst_show(struct device *dev,
>  	struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
>  	struct etmv4_config *config = &drvdata->config;
>  
> -	val = (config->vinst_ctrl & TRCVICTLR_EXLEVEL_S_MASK) >> TRCVICTLR_EXLEVEL_S_SHIFT;
> +	val = FIELD_GET(TRCVICTLR_EXLEVEL_S_MASK, config->vinst_ctrl);
>  	return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
>  }
>  
> @@ -772,10 +772,10 @@ static ssize_t s_exlevel_vinst_store(struct device *dev,
>  
>  	spin_lock(&drvdata->spinlock);
>  	/* clear all EXLEVEL_S bits  */
> -	config->vinst_ctrl &= ~(TRCVICTLR_EXLEVEL_S_MASK);
> +	config->vinst_ctrl &= ~TRCVICTLR_EXLEVEL_S_MASK;
>  	/* enable instruction tracing for corresponding exception level */
>  	val &= drvdata->s_ex_level;
> -	config->vinst_ctrl |= (val << TRCVICTLR_EXLEVEL_S_SHIFT);
> +	config->vinst_ctrl |= val << __bf_shf(TRCVICTLR_EXLEVEL_S_MASK);
>  	spin_unlock(&drvdata->spinlock);
>  	return size;
>  }
> @@ -790,7 +790,7 @@ static ssize_t ns_exlevel_vinst_show(struct device *dev,
>  	struct etmv4_config *config = &drvdata->config;
>  
>  	/* EXLEVEL_NS, bits[23:20] */
> -	val = (config->vinst_ctrl & TRCVICTLR_EXLEVEL_NS_MASK) >> TRCVICTLR_EXLEVEL_NS_SHIFT;
> +	val = FIELD_GET(TRCVICTLR_EXLEVEL_NS_MASK, config->vinst_ctrl);
>  	return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
>  }
>  
> @@ -807,10 +807,10 @@ static ssize_t ns_exlevel_vinst_store(struct device *dev,
>  
>  	spin_lock(&drvdata->spinlock);
>  	/* clear EXLEVEL_NS bits  */
> -	config->vinst_ctrl &= ~(TRCVICTLR_EXLEVEL_NS_MASK);
> +	config->vinst_ctrl &= ~TRCVICTLR_EXLEVEL_NS_MASK;
>  	/* enable instruction tracing for corresponding exception level */
>  	val &= drvdata->ns_ex_level;
> -	config->vinst_ctrl |= (val << TRCVICTLR_EXLEVEL_NS_SHIFT);
> +	config->vinst_ctrl |= val << __bf_shf(TRCVICTLR_EXLEVEL_NS_MASK);
>  	spin_unlock(&drvdata->spinlock);
>  	return size;
>  }
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
> index 36934056a5dc..9cacc38b1890 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x.h
> +++ b/drivers/hwtracing/coresight/coresight-etm4x.h
> @@ -200,6 +200,14 @@
>  #define TRCSTALLCTLR_INSTPRIORITY		BIT(10)
>  #define TRCSTALLCTLR_NOOVERFLOW			BIT(13)
>  
> +#define TRCVICTLR_EVENT_MASK			GENMASK(7, 0)
> +#define TRCVICTLR_SSSTATUS			BIT(9)
> +#define TRCVICTLR_TRCRESET			BIT(10)
> +#define TRCVICTLR_TRCERR			BIT(11)
> +#define TRCVICTLR_EXLEVEL_MASK			GENMASK(22, 16)
> +#define TRCVICTLR_EXLEVEL_S_MASK		GENMASK(19, 16)
> +#define TRCVICTLR_EXLEVEL_NS_MASK		GENMASK(22, 20)
> +
>  /*
>   * System instructions to access ETM registers.
>   * See ETMv4.4 spec ARM IHI0064F section 4.3.6 System instructions
> @@ -700,23 +708,9 @@
>  #define ETM_EXLEVEL_NS_OS		BIT(5)	/* NonSecure EL1	*/
>  #define ETM_EXLEVEL_NS_HYP		BIT(6)	/* NonSecure EL2	*/
>  
> -#define ETM_EXLEVEL_MASK		(GENMASK(6, 0))
> -#define ETM_EXLEVEL_S_MASK		(GENMASK(3, 0))
> -#define ETM_EXLEVEL_NS_MASK		(GENMASK(6, 4))
> -
>  /* access level controls in TRCACATRn */
>  #define TRCACATR_EXLEVEL_SHIFT		8
>  
> -/* access level control in TRCVICTLR */
> -#define TRCVICTLR_EXLEVEL_SHIFT		16
> -#define TRCVICTLR_EXLEVEL_S_SHIFT	16
> -#define TRCVICTLR_EXLEVEL_NS_SHIFT	20
> -
> -/* secure / non secure masks - TRCVICTLR, IDR3 */
> -#define TRCVICTLR_EXLEVEL_MASK		(ETM_EXLEVEL_MASK << TRCVICTLR_EXLEVEL_SHIFT)
> -#define TRCVICTLR_EXLEVEL_S_MASK	(ETM_EXLEVEL_S_MASK << TRCVICTLR_EXLEVEL_SHIFT)
> -#define TRCVICTLR_EXLEVEL_NS_MASK	(ETM_EXLEVEL_NS_MASK << TRCVICTLR_EXLEVEL_SHIFT)
> -
>  #define ETM_TRCIDR1_ARCH_MAJOR_SHIFT	8
>  #define ETM_TRCIDR1_ARCH_MAJOR_MASK	(0xfU << ETM_TRCIDR1_ARCH_MAJOR_SHIFT)
>  #define ETM_TRCIDR1_ARCH_MAJOR(x)	\
> -- 
> 2.28.0
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 09/15] coresight: etm4x: Cleanup TRCVICTLR register accesses

[James Clark]

On 23/03/2022 15:59, Mathieu Poirier wrote:
> Hi James,
> 
> On Fri, Mar 04, 2022 at 05:19:06PM +0000, James Clark 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          | 10 +++---
>>  .../coresight/coresight-etm4x-sysfs.c         | 36 +++++++++----------
>>  drivers/hwtracing/coresight/coresight-etm4x.h | 22 +++++-------
>>  3 files changed, 31 insertions(+), 37 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> index 445e2057d5ed..88353f8ba414 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> @@ -1206,7 +1206,7 @@ static void etm4_init_arch_data(void *info)
>>  
>>  static inline u32 etm4_get_victlr_access_type(struct etmv4_config *config)
>>  {
>> -	return etm4_get_access_type(config) << TRCVICTLR_EXLEVEL_SHIFT;
>> +	return etm4_get_access_type(config) << __bf_shf(TRCVICTLR_EXLEVEL_MASK);
>>  }
>>  
>>  /* Set ELx trace filter access in the TRCVICTLR register */
>> @@ -1232,7 +1232,7 @@ static void etm4_set_default_config(struct etmv4_config *config)
>>  	config->ts_ctrl = 0x0;
>>  
>>  	/* TRCVICTLR::EVENT = 0x01, select the always on logic */
>> -	config->vinst_ctrl = BIT(0);
>> +	config->vinst_ctrl = FIELD_PREP(TRCVICTLR_EVENT_MASK, 0x01);
>>  
>>  	/* TRCVICTLR::EXLEVEL_NS:EXLEVELS: Set kernel / user filtering */
>>  	etm4_set_victlr_access(config);
>> @@ -1341,7 +1341,7 @@ static void etm4_set_default_filter(struct etmv4_config *config)
>>  	 * TRCVICTLR::SSSTATUS == 1, the start-stop logic is
>>  	 * in the started state
>>  	 */
>> -	config->vinst_ctrl |= BIT(9);
>> +	config->vinst_ctrl |= TRCVICTLR_SSSTATUS;
>>  	config->mode |= ETM_MODE_VIEWINST_STARTSTOP;
>>  
>>  	/* No start-stop filtering for ViewInst */
>> @@ -1445,7 +1445,7 @@ static int etm4_set_event_filters(struct etmv4_drvdata *drvdata,
>>  			 * TRCVICTLR::SSSTATUS == 1, the start-stop logic is
>>  			 * in the started state
>>  			 */
>> -			config->vinst_ctrl |= BIT(9);
>> +			config->vinst_ctrl |= TRCVICTLR_SSSTATUS;
>>  
>>  			/* No start-stop filtering for ViewInst */
>>  			config->vissctlr = 0x0;
>> @@ -1473,7 +1473,7 @@ static int etm4_set_event_filters(struct etmv4_drvdata *drvdata,
>>  			 * etm4_disable_perf().
>>  			 */
>>  			if (filters->ssstatus)
>> -				config->vinst_ctrl |= BIT(9);
>> +				config->vinst_ctrl |= TRCVICTLR_SSSTATUS;
>>  
>>  			/* No include/exclude filtering for ViewInst */
>>  			config->viiectlr = 0x0;
>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
>> index cd24590ea38a..b3b1b92909cc 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
>> +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
>> @@ -206,11 +206,11 @@ static ssize_t reset_store(struct device *dev,
>>  	 * started state. ARM recommends start-stop logic is set before
>>  	 * each trace run.
>>  	 */
>> -	config->vinst_ctrl = BIT(0);
>> +	config->vinst_ctrl = FIELD_PREP(TRCVICTLR_EVENT_MASK, 0x01);
>>  	if (drvdata->nr_addr_cmp > 0) {
>>  		config->mode |= ETM_MODE_VIEWINST_STARTSTOP;
>>  		/* SSSTATUS, bit[9] */
>> -		config->vinst_ctrl |= BIT(9);
>> +		config->vinst_ctrl |= TRCVICTLR_SSSTATUS;
>>  	}
>>  
>>  	/* No address range filtering for ViewInst */
>> @@ -416,22 +416,22 @@ static ssize_t mode_store(struct device *dev,
>>  
>>  	/* bit[9] Start/stop logic control bit */
>>  	if (config->mode & ETM_MODE_VIEWINST_STARTSTOP)
>> -		config->vinst_ctrl |= BIT(9);
>> +		config->vinst_ctrl |= TRCVICTLR_SSSTATUS;
>>  	else
>> -		config->vinst_ctrl &= ~BIT(9);
>> +		config->vinst_ctrl &= ~TRCVICTLR_SSSTATUS;
>>  
>>  	/* bit[10], Whether a trace unit must trace a Reset exception */
>>  	if (config->mode & ETM_MODE_TRACE_RESET)
>> -		config->vinst_ctrl |= BIT(10);
>> +		config->vinst_ctrl |= TRCVICTLR_TRCRESET;
>>  	else
>> -		config->vinst_ctrl &= ~BIT(10);
>> +		config->vinst_ctrl &= ~TRCVICTLR_TRCRESET;
>>  
>>  	/* bit[11], Whether a trace unit must trace a system error exception */
>>  	if ((config->mode & ETM_MODE_TRACE_ERR) &&
>>  		(drvdata->trc_error == true))
>> -		config->vinst_ctrl |= BIT(11);
>> +		config->vinst_ctrl |= TRCVICTLR_TRCERR;
>>  	else
>> -		config->vinst_ctrl &= ~BIT(11);
>> +		config->vinst_ctrl &= ~TRCVICTLR_TRCERR;
>>  
>>  	if (config->mode & (ETM_MODE_EXCL_KERN | ETM_MODE_EXCL_USER))
>>  		etm4_config_trace_mode(config);
>> @@ -723,7 +723,7 @@ static ssize_t event_vinst_show(struct device *dev,
>>  	struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
>>  	struct etmv4_config *config = &drvdata->config;
>>  
>> -	val = config->vinst_ctrl & ETMv4_EVENT_MASK;
>> +	val = FIELD_GET(TRCVICTLR_EVENT_MASK, config->vinst_ctrl);
>>  	return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
>>  }
>>  
>> @@ -739,9 +739,9 @@ static ssize_t event_vinst_store(struct device *dev,
>>  		return -EINVAL;
>>  
>>  	spin_lock(&drvdata->spinlock);
>> -	val &= ETMv4_EVENT_MASK;
>> -	config->vinst_ctrl &= ~ETMv4_EVENT_MASK;
>> -	config->vinst_ctrl |= val;
>> +	val &= TRCVICTLR_EVENT_MASK >> __bf_shf(TRCVICTLR_EVENT_MASK);
> 
> Not sure why the right-shifting operation is needed since the mask starts at bit
> 0.  Please consider fixing _if_ you end up respinning this.

Yes you're right it doesn't need shifting. I suppose I did it that way so that when
reading the code it's obvious that 'val' is a value starting at bit 0, rather than
the original state of the register shifted to some arbitrary position.

Someone reading the code wouldn't know that TRCVICTLR_EVENT_MASK starts at 0,
so without the explicit shift they might think excluding it was a mistake. It also 
makes it consistent with other uses of masks that don't start at 0.

James

>
> Thanks,
> Mathieu
> 
>> +	config->vinst_ctrl &= ~TRCVICTLR_EVENT_MASK;
>> +	config->vinst_ctrl |= FIELD_PREP(TRCVICTLR_EVENT_MASK, val);
>>  	spin_unlock(&drvdata->spinlock);
>>  	return size;
>>  }
>> @@ -755,7 +755,7 @@ static ssize_t s_exlevel_vinst_show(struct device *dev,
>>  	struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
>>  	struct etmv4_config *config = &drvdata->config;
>>  
>> -	val = (config->vinst_ctrl & TRCVICTLR_EXLEVEL_S_MASK) >> TRCVICTLR_EXLEVEL_S_SHIFT;
>> +	val = FIELD_GET(TRCVICTLR_EXLEVEL_S_MASK, config->vinst_ctrl);
>>  	return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
>>  }
>>  
>> @@ -772,10 +772,10 @@ static ssize_t s_exlevel_vinst_store(struct device *dev,
>>  
>>  	spin_lock(&drvdata->spinlock);
>>  	/* clear all EXLEVEL_S bits  */
>> -	config->vinst_ctrl &= ~(TRCVICTLR_EXLEVEL_S_MASK);
>> +	config->vinst_ctrl &= ~TRCVICTLR_EXLEVEL_S_MASK;
>>  	/* enable instruction tracing for corresponding exception level */
>>  	val &= drvdata->s_ex_level;
>> -	config->vinst_ctrl |= (val << TRCVICTLR_EXLEVEL_S_SHIFT);
>> +	config->vinst_ctrl |= val << __bf_shf(TRCVICTLR_EXLEVEL_S_MASK);
>>  	spin_unlock(&drvdata->spinlock);
>>  	return size;
>>  }
>> @@ -790,7 +790,7 @@ static ssize_t ns_exlevel_vinst_show(struct device *dev,
>>  	struct etmv4_config *config = &drvdata->config;
>>  
>>  	/* EXLEVEL_NS, bits[23:20] */
>> -	val = (config->vinst_ctrl & TRCVICTLR_EXLEVEL_NS_MASK) >> TRCVICTLR_EXLEVEL_NS_SHIFT;
>> +	val = FIELD_GET(TRCVICTLR_EXLEVEL_NS_MASK, config->vinst_ctrl);
>>  	return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
>>  }
>>  
>> @@ -807,10 +807,10 @@ static ssize_t ns_exlevel_vinst_store(struct device *dev,
>>  
>>  	spin_lock(&drvdata->spinlock);
>>  	/* clear EXLEVEL_NS bits  */
>> -	config->vinst_ctrl &= ~(TRCVICTLR_EXLEVEL_NS_MASK);
>> +	config->vinst_ctrl &= ~TRCVICTLR_EXLEVEL_NS_MASK;
>>  	/* enable instruction tracing for corresponding exception level */
>>  	val &= drvdata->ns_ex_level;
>> -	config->vinst_ctrl |= (val << TRCVICTLR_EXLEVEL_NS_SHIFT);
>> +	config->vinst_ctrl |= val << __bf_shf(TRCVICTLR_EXLEVEL_NS_MASK);
>>  	spin_unlock(&drvdata->spinlock);
>>  	return size;
>>  }
>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
>> index 36934056a5dc..9cacc38b1890 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm4x.h
>> +++ b/drivers/hwtracing/coresight/coresight-etm4x.h
>> @@ -200,6 +200,14 @@
>>  #define TRCSTALLCTLR_INSTPRIORITY		BIT(10)
>>  #define TRCSTALLCTLR_NOOVERFLOW			BIT(13)
>>  
>> +#define TRCVICTLR_EVENT_MASK			GENMASK(7, 0)
>> +#define TRCVICTLR_SSSTATUS			BIT(9)
>> +#define TRCVICTLR_TRCRESET			BIT(10)
>> +#define TRCVICTLR_TRCERR			BIT(11)
>> +#define TRCVICTLR_EXLEVEL_MASK			GENMASK(22, 16)
>> +#define TRCVICTLR_EXLEVEL_S_MASK		GENMASK(19, 16)
>> +#define TRCVICTLR_EXLEVEL_NS_MASK		GENMASK(22, 20)
>> +
>>  /*
>>   * System instructions to access ETM registers.
>>   * See ETMv4.4 spec ARM IHI0064F section 4.3.6 System instructions
>> @@ -700,23 +708,9 @@
>>  #define ETM_EXLEVEL_NS_OS		BIT(5)	/* NonSecure EL1	*/
>>  #define ETM_EXLEVEL_NS_HYP		BIT(6)	/* NonSecure EL2	*/
>>  
>> -#define ETM_EXLEVEL_MASK		(GENMASK(6, 0))
>> -#define ETM_EXLEVEL_S_MASK		(GENMASK(3, 0))
>> -#define ETM_EXLEVEL_NS_MASK		(GENMASK(6, 4))
>> -
>>  /* access level controls in TRCACATRn */
>>  #define TRCACATR_EXLEVEL_SHIFT		8
>>  
>> -/* access level control in TRCVICTLR */
>> -#define TRCVICTLR_EXLEVEL_SHIFT		16
>> -#define TRCVICTLR_EXLEVEL_S_SHIFT	16
>> -#define TRCVICTLR_EXLEVEL_NS_SHIFT	20
>> -
>> -/* secure / non secure masks - TRCVICTLR, IDR3 */
>> -#define TRCVICTLR_EXLEVEL_MASK		(ETM_EXLEVEL_MASK << TRCVICTLR_EXLEVEL_SHIFT)
>> -#define TRCVICTLR_EXLEVEL_S_MASK	(ETM_EXLEVEL_S_MASK << TRCVICTLR_EXLEVEL_SHIFT)
>> -#define TRCVICTLR_EXLEVEL_NS_MASK	(ETM_EXLEVEL_NS_MASK << TRCVICTLR_EXLEVEL_SHIFT)
>> -
>>  #define ETM_TRCIDR1_ARCH_MAJOR_SHIFT	8
>>  #define ETM_TRCIDR1_ARCH_MAJOR_MASK	(0xfU << ETM_TRCIDR1_ARCH_MAJOR_SHIFT)
>>  #define ETM_TRCIDR1_ARCH_MAJOR(x)	\
>> -- 
>> 2.28.0
>>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 09/15] coresight: etm4x: Cleanup TRCVICTLR register accesses

[Mike Leach]

On Mon, 28 Mar 2022 at 11:41, James Clark <james.clark@arm.com> wrote:
>
>
>
> On 23/03/2022 15:59, Mathieu Poirier wrote:
> > Hi James,
> >
> > On Fri, Mar 04, 2022 at 05:19:06PM +0000, James Clark 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          | 10 +++---
> >>  .../coresight/coresight-etm4x-sysfs.c         | 36 +++++++++----------
> >>  drivers/hwtracing/coresight/coresight-etm4x.h | 22 +++++-------
> >>  3 files changed, 31 insertions(+), 37 deletions(-)
> >>
> >> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> >> index 445e2057d5ed..88353f8ba414 100644
> >> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> >> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> >> @@ -1206,7 +1206,7 @@ static void etm4_init_arch_data(void *info)
> >>
> >>  static inline u32 etm4_get_victlr_access_type(struct etmv4_config *config)
> >>  {
> >> -    return etm4_get_access_type(config) << TRCVICTLR_EXLEVEL_SHIFT;
> >> +    return etm4_get_access_type(config) << __bf_shf(TRCVICTLR_EXLEVEL_MASK);
> >>  }
> >>
> >>  /* Set ELx trace filter access in the TRCVICTLR register */
> >> @@ -1232,7 +1232,7 @@ static void etm4_set_default_config(struct etmv4_config *config)
> >>      config->ts_ctrl = 0x0;
> >>
> >>      /* TRCVICTLR::EVENT = 0x01, select the always on logic */
> >> -    config->vinst_ctrl = BIT(0);
> >> +    config->vinst_ctrl = FIELD_PREP(TRCVICTLR_EVENT_MASK, 0x01);
> >>
> >>      /* TRCVICTLR::EXLEVEL_NS:EXLEVELS: Set kernel / user filtering */
> >>      etm4_set_victlr_access(config);
> >> @@ -1341,7 +1341,7 @@ static void etm4_set_default_filter(struct etmv4_config *config)
> >>       * TRCVICTLR::SSSTATUS == 1, the start-stop logic is
> >>       * in the started state
> >>       */
> >> -    config->vinst_ctrl |= BIT(9);
> >> +    config->vinst_ctrl |= TRCVICTLR_SSSTATUS;
> >>      config->mode |= ETM_MODE_VIEWINST_STARTSTOP;
> >>
> >>      /* No start-stop filtering for ViewInst */
> >> @@ -1445,7 +1445,7 @@ static int etm4_set_event_filters(struct etmv4_drvdata *drvdata,
> >>                       * TRCVICTLR::SSSTATUS == 1, the start-stop logic is
> >>                       * in the started state
> >>                       */
> >> -                    config->vinst_ctrl |= BIT(9);
> >> +                    config->vinst_ctrl |= TRCVICTLR_SSSTATUS;
> >>
> >>                      /* No start-stop filtering for ViewInst */
> >>                      config->vissctlr = 0x0;
> >> @@ -1473,7 +1473,7 @@ static int etm4_set_event_filters(struct etmv4_drvdata *drvdata,
> >>                       * etm4_disable_perf().
> >>                       */
> >>                      if (filters->ssstatus)
> >> -                            config->vinst_ctrl |= BIT(9);
> >> +                            config->vinst_ctrl |= TRCVICTLR_SSSTATUS;
> >>
> >>                      /* No include/exclude filtering for ViewInst */
> >>                      config->viiectlr = 0x0;
> >> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> >> index cd24590ea38a..b3b1b92909cc 100644
> >> --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> >> +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> >> @@ -206,11 +206,11 @@ static ssize_t reset_store(struct device *dev,
> >>       * started state. ARM recommends start-stop logic is set before
> >>       * each trace run.
> >>       */
> >> -    config->vinst_ctrl = BIT(0);
> >> +    config->vinst_ctrl = FIELD_PREP(TRCVICTLR_EVENT_MASK, 0x01);
> >>      if (drvdata->nr_addr_cmp > 0) {
> >>              config->mode |= ETM_MODE_VIEWINST_STARTSTOP;
> >>              /* SSSTATUS, bit[9] */
> >> -            config->vinst_ctrl |= BIT(9);
> >> +            config->vinst_ctrl |= TRCVICTLR_SSSTATUS;
> >>      }
> >>
> >>      /* No address range filtering for ViewInst */
> >> @@ -416,22 +416,22 @@ static ssize_t mode_store(struct device *dev,
> >>
> >>      /* bit[9] Start/stop logic control bit */
> >>      if (config->mode & ETM_MODE_VIEWINST_STARTSTOP)
> >> -            config->vinst_ctrl |= BIT(9);
> >> +            config->vinst_ctrl |= TRCVICTLR_SSSTATUS;
> >>      else
> >> -            config->vinst_ctrl &= ~BIT(9);
> >> +            config->vinst_ctrl &= ~TRCVICTLR_SSSTATUS;
> >>
> >>      /* bit[10], Whether a trace unit must trace a Reset exception */
> >>      if (config->mode & ETM_MODE_TRACE_RESET)
> >> -            config->vinst_ctrl |= BIT(10);
> >> +            config->vinst_ctrl |= TRCVICTLR_TRCRESET;
> >>      else
> >> -            config->vinst_ctrl &= ~BIT(10);
> >> +            config->vinst_ctrl &= ~TRCVICTLR_TRCRESET;
> >>
> >>      /* bit[11], Whether a trace unit must trace a system error exception */
> >>      if ((config->mode & ETM_MODE_TRACE_ERR) &&
> >>              (drvdata->trc_error == true))
> >> -            config->vinst_ctrl |= BIT(11);
> >> +            config->vinst_ctrl |= TRCVICTLR_TRCERR;
> >>      else
> >> -            config->vinst_ctrl &= ~BIT(11);
> >> +            config->vinst_ctrl &= ~TRCVICTLR_TRCERR;
> >>
> >>      if (config->mode & (ETM_MODE_EXCL_KERN | ETM_MODE_EXCL_USER))
> >>              etm4_config_trace_mode(config);
> >> @@ -723,7 +723,7 @@ static ssize_t event_vinst_show(struct device *dev,
> >>      struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
> >>      struct etmv4_config *config = &drvdata->config;
> >>
> >> -    val = config->vinst_ctrl & ETMv4_EVENT_MASK;
> >> +    val = FIELD_GET(TRCVICTLR_EVENT_MASK, config->vinst_ctrl);
> >>      return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
> >>  }
> >>
> >> @@ -739,9 +739,9 @@ static ssize_t event_vinst_store(struct device *dev,
> >>              return -EINVAL;
> >>
> >>      spin_lock(&drvdata->spinlock);
> >> -    val &= ETMv4_EVENT_MASK;
> >> -    config->vinst_ctrl &= ~ETMv4_EVENT_MASK;
> >> -    config->vinst_ctrl |= val;
> >> +    val &= TRCVICTLR_EVENT_MASK >> __bf_shf(TRCVICTLR_EVENT_MASK);
> >
> > Not sure why the right-shifting operation is needed since the mask starts at bit
> > 0.  Please consider fixing _if_ you end up respinning this.
>
> Yes you're right it doesn't need shifting. I suppose I did it that way so that when
> reading the code it's obvious that 'val' is a value starting at bit 0, rather than
> the original state of the register shifted to some arbitrary position.
>
> Someone reading the code wouldn't know that TRCVICTLR_EVENT_MASK starts at 0,
> so without the explicit shift they might think excluding it was a mistake. It also
> makes it consistent with other uses of masks that don't start at 0.
>
> James
>
> >
> > Thanks,
> > Mathieu
> >
> >> +    config->vinst_ctrl &= ~TRCVICTLR_EVENT_MASK;
> >> +    config->vinst_ctrl |= FIELD_PREP(TRCVICTLR_EVENT_MASK, val);
> >>      spin_unlock(&drvdata->spinlock);
> >>      return size;
> >>  }
> >> @@ -755,7 +755,7 @@ static ssize_t s_exlevel_vinst_show(struct device *dev,
> >>      struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
> >>      struct etmv4_config *config = &drvdata->config;
> >>
> >> -    val = (config->vinst_ctrl & TRCVICTLR_EXLEVEL_S_MASK) >> TRCVICTLR_EXLEVEL_S_SHIFT;
> >> +    val = FIELD_GET(TRCVICTLR_EXLEVEL_S_MASK, config->vinst_ctrl);
> >>      return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
> >>  }
> >>
> >> @@ -772,10 +772,10 @@ static ssize_t s_exlevel_vinst_store(struct device *dev,
> >>
> >>      spin_lock(&drvdata->spinlock);
> >>      /* clear all EXLEVEL_S bits  */
> >> -    config->vinst_ctrl &= ~(TRCVICTLR_EXLEVEL_S_MASK);
> >> +    config->vinst_ctrl &= ~TRCVICTLR_EXLEVEL_S_MASK;
> >>      /* enable instruction tracing for corresponding exception level */
> >>      val &= drvdata->s_ex_level;
> >> -    config->vinst_ctrl |= (val << TRCVICTLR_EXLEVEL_S_SHIFT);
> >> +    config->vinst_ctrl |= val << __bf_shf(TRCVICTLR_EXLEVEL_S_MASK);
> >>      spin_unlock(&drvdata->spinlock);
> >>      return size;
> >>  }
> >> @@ -790,7 +790,7 @@ static ssize_t ns_exlevel_vinst_show(struct device *dev,
> >>      struct etmv4_config *config = &drvdata->config;
> >>
> >>      /* EXLEVEL_NS, bits[23:20] */
> >> -    val = (config->vinst_ctrl & TRCVICTLR_EXLEVEL_NS_MASK) >> TRCVICTLR_EXLEVEL_NS_SHIFT;
> >> +    val = FIELD_GET(TRCVICTLR_EXLEVEL_NS_MASK, config->vinst_ctrl);
> >>      return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
> >>  }
> >>
> >> @@ -807,10 +807,10 @@ static ssize_t ns_exlevel_vinst_store(struct device *dev,
> >>
> >>      spin_lock(&drvdata->spinlock);
> >>      /* clear EXLEVEL_NS bits  */
> >> -    config->vinst_ctrl &= ~(TRCVICTLR_EXLEVEL_NS_MASK);
> >> +    config->vinst_ctrl &= ~TRCVICTLR_EXLEVEL_NS_MASK;
> >>      /* enable instruction tracing for corresponding exception level */
> >>      val &= drvdata->ns_ex_level;
> >> -    config->vinst_ctrl |= (val << TRCVICTLR_EXLEVEL_NS_SHIFT);
> >> +    config->vinst_ctrl |= val << __bf_shf(TRCVICTLR_EXLEVEL_NS_MASK);
> >>      spin_unlock(&drvdata->spinlock);
> >>      return size;
> >>  }
> >> diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
> >> index 36934056a5dc..9cacc38b1890 100644
> >> --- a/drivers/hwtracing/coresight/coresight-etm4x.h
> >> +++ b/drivers/hwtracing/coresight/coresight-etm4x.h
> >> @@ -200,6 +200,14 @@
> >>  #define TRCSTALLCTLR_INSTPRIORITY           BIT(10)
> >>  #define TRCSTALLCTLR_NOOVERFLOW                     BIT(13)
> >>
> >> +#define TRCVICTLR_EVENT_MASK                        GENMASK(7, 0)
> >> +#define TRCVICTLR_SSSTATUS                  BIT(9)
> >> +#define TRCVICTLR_TRCRESET                  BIT(10)
> >> +#define TRCVICTLR_TRCERR                    BIT(11)
> >> +#define TRCVICTLR_EXLEVEL_MASK                      GENMASK(22, 16)
> >> +#define TRCVICTLR_EXLEVEL_S_MASK            GENMASK(19, 16)
> >> +#define TRCVICTLR_EXLEVEL_NS_MASK           GENMASK(22, 20)
> >> +
> >>  /*
> >>   * System instructions to access ETM registers.
> >>   * See ETMv4.4 spec ARM IHI0064F section 4.3.6 System instructions
> >> @@ -700,23 +708,9 @@
> >>  #define ETM_EXLEVEL_NS_OS           BIT(5)  /* NonSecure EL1        */
> >>  #define ETM_EXLEVEL_NS_HYP          BIT(6)  /* NonSecure EL2        */
> >>
> >> -#define ETM_EXLEVEL_MASK            (GENMASK(6, 0))
> >> -#define ETM_EXLEVEL_S_MASK          (GENMASK(3, 0))
> >> -#define ETM_EXLEVEL_NS_MASK         (GENMASK(6, 4))
> >> -
> >>  /* access level controls in TRCACATRn */
> >>  #define TRCACATR_EXLEVEL_SHIFT              8
> >>
> >> -/* access level control in TRCVICTLR */
> >> -#define TRCVICTLR_EXLEVEL_SHIFT             16
> >> -#define TRCVICTLR_EXLEVEL_S_SHIFT   16
> >> -#define TRCVICTLR_EXLEVEL_NS_SHIFT  20
> >> -
> >> -/* secure / non secure masks - TRCVICTLR, IDR3 */
> >> -#define TRCVICTLR_EXLEVEL_MASK              (ETM_EXLEVEL_MASK << TRCVICTLR_EXLEVEL_SHIFT)
> >> -#define TRCVICTLR_EXLEVEL_S_MASK    (ETM_EXLEVEL_S_MASK << TRCVICTLR_EXLEVEL_SHIFT)
> >> -#define TRCVICTLR_EXLEVEL_NS_MASK   (ETM_EXLEVEL_NS_MASK << TRCVICTLR_EXLEVEL_SHIFT)
> >> -
> >>  #define ETM_TRCIDR1_ARCH_MAJOR_SHIFT        8
> >>  #define ETM_TRCIDR1_ARCH_MAJOR_MASK (0xfU << ETM_TRCIDR1_ARCH_MAJOR_SHIFT)
> >>  #define ETM_TRCIDR1_ARCH_MAJOR(x)   \
> >> --
> >> 2.28.0
> >>


Reviewed-by: Mike Leach <mike.leach@linaro.org>

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 10/15] coresight: etm3x: Cleanup ETMTECR1 register accesses

[James Clark]

This is a no-op change for style and consistency and has no effect on
the binary output by the compiler. These fields already have macros
to define them so use them instead of magic numbers.

Signed-off-by: James Clark <james.clark@arm.com>
---
 drivers/hwtracing/coresight/coresight-etm3x-core.c  | 2 +-
 drivers/hwtracing/coresight/coresight-etm3x-sysfs.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm3x-core.c b/drivers/hwtracing/coresight/coresight-etm3x-core.c
index 7d413ba8b823..d0ab9933472b 100644
--- a/drivers/hwtracing/coresight/coresight-etm3x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm3x-core.c
@@ -204,7 +204,7 @@ void etm_set_default(struct etm_config *config)
 	 *  set all bits in register 0x007, the ETMTECR2, to 0
 	 *  set register 0x008, the ETMTEEVR, to 0x6F (TRUE).
 	 */
-	config->enable_ctrl1 = BIT(24);
+	config->enable_ctrl1 = ETMTECR1_INC_EXC;
 	config->enable_ctrl2 = 0x0;
 	config->enable_event = ETM_HARD_WIRE_RES_A;
 
diff --git a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
index e8c7649f123e..68fcbf4ce7a8 100644
--- a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
@@ -474,7 +474,7 @@ static ssize_t addr_start_store(struct device *dev,
 	config->addr_val[idx] = val;
 	config->addr_type[idx] = ETM_ADDR_TYPE_START;
 	config->startstop_ctrl |= (1 << idx);
-	config->enable_ctrl1 |= BIT(25);
+	config->enable_ctrl1 |= ETMTECR1_START_STOP;
 	spin_unlock(&drvdata->spinlock);
 
 	return size;
-- 
2.28.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 10/15] coresight: etm3x: Cleanup ETMTECR1 register accesses

[Mike Leach]

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. These fields already have macros
> to define them so use them instead of magic numbers.
>
> Signed-off-by: James Clark <james.clark@arm.com>
> ---
>  drivers/hwtracing/coresight/coresight-etm3x-core.c  | 2 +-
>  drivers/hwtracing/coresight/coresight-etm3x-sysfs.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm3x-core.c b/drivers/hwtracing/coresight/coresight-etm3x-core.c
> index 7d413ba8b823..d0ab9933472b 100644
> --- a/drivers/hwtracing/coresight/coresight-etm3x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm3x-core.c
> @@ -204,7 +204,7 @@ void etm_set_default(struct etm_config *config)
>          *  set all bits in register 0x007, the ETMTECR2, to 0
>          *  set register 0x008, the ETMTEEVR, to 0x6F (TRUE).
>          */
> -       config->enable_ctrl1 = BIT(24);
> +       config->enable_ctrl1 = ETMTECR1_INC_EXC;
>         config->enable_ctrl2 = 0x0;
>         config->enable_event = ETM_HARD_WIRE_RES_A;
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
> index e8c7649f123e..68fcbf4ce7a8 100644
> --- a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
> @@ -474,7 +474,7 @@ static ssize_t addr_start_store(struct device *dev,
>         config->addr_val[idx] = val;
>         config->addr_type[idx] = ETM_ADDR_TYPE_START;
>         config->startstop_ctrl |= (1 << idx);
> -       config->enable_ctrl1 |= BIT(25);
> +       config->enable_ctrl1 |= ETMTECR1_START_STOP;
>         spin_unlock(&drvdata->spinlock);
>
>         return size;
> --
> 2.28.0
>

Reviewed-by: Mike Leach <mike.leach@linaro.org>

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 11/15] coresight: etm4x: Cleanup TRCACATRn register accesses

[James Clark]

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-sysfs.c         | 42 +++++++++----------
 drivers/hwtracing/coresight/coresight-etm4x.h | 14 +++++--
 2 files changed, 31 insertions(+), 25 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
index b3b1b92909cc..29188b1a4646 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
@@ -22,7 +22,7 @@ static int etm4_set_mode_exclude(struct etmv4_drvdata *drvdata, bool exclude)
 	 * TRCACATRn.TYPE bit[1:0]: type of comparison
 	 * the trace unit performs
 	 */
-	if (BMVAL(config->addr_acc[idx], 0, 1) == ETM_INSTR_ADDR) {
+	if (FIELD_GET(TRCACATRn_TYPE_MASK, config->addr_acc[idx]) == TRCACATRn_TYPE_ADDR) {
 		if (idx % 2 != 0)
 			return -EINVAL;
 
@@ -863,11 +863,11 @@ static ssize_t addr_instdatatype_show(struct device *dev,
 
 	spin_lock(&drvdata->spinlock);
 	idx = config->addr_idx;
-	val = BMVAL(config->addr_acc[idx], 0, 1);
+	val = FIELD_GET(TRCACATRn_TYPE_MASK, config->addr_acc[idx]);
 	len = scnprintf(buf, PAGE_SIZE, "%s\n",
-			val == ETM_INSTR_ADDR ? "instr" :
-			(val == ETM_DATA_LOAD_ADDR ? "data_load" :
-			(val == ETM_DATA_STORE_ADDR ? "data_store" :
+			val == TRCACATRn_TYPE_ADDR ? "instr" :
+			(val == TRCACATRn_TYPE_DATA_LOAD_ADDR ? "data_load" :
+			(val == TRCACATRn_TYPE_DATA_STORE_ADDR ? "data_store" :
 			"data_load_store")));
 	spin_unlock(&drvdata->spinlock);
 	return len;
@@ -891,7 +891,7 @@ static ssize_t addr_instdatatype_store(struct device *dev,
 	idx = config->addr_idx;
 	if (!strcmp(str, "instr"))
 		/* TYPE, bits[1:0] */
-		config->addr_acc[idx] &= ~(BIT(0) | BIT(1));
+		config->addr_acc[idx] &= ~TRCACATRn_TYPE_MASK;
 
 	spin_unlock(&drvdata->spinlock);
 	return size;
@@ -1149,7 +1149,7 @@ static ssize_t addr_ctxtype_show(struct device *dev,
 	spin_lock(&drvdata->spinlock);
 	idx = config->addr_idx;
 	/* CONTEXTTYPE, bits[3:2] */
-	val = BMVAL(config->addr_acc[idx], 2, 3);
+	val = FIELD_GET(TRCACATRn_CONTEXTTYPE_MASK, config->addr_acc[idx]);
 	len = scnprintf(buf, PAGE_SIZE, "%s\n", val == ETM_CTX_NONE ? "none" :
 			(val == ETM_CTX_CTXID ? "ctxid" :
 			(val == ETM_CTX_VMID ? "vmid" : "all")));
@@ -1175,18 +1175,18 @@ static ssize_t addr_ctxtype_store(struct device *dev,
 	idx = config->addr_idx;
 	if (!strcmp(str, "none"))
 		/* start by clearing context type bits */
-		config->addr_acc[idx] &= ~(BIT(2) | BIT(3));
+		config->addr_acc[idx] &= ~TRCACATRn_CONTEXTTYPE_MASK;
 	else if (!strcmp(str, "ctxid")) {
 		/* 0b01 The trace unit performs a Context ID */
 		if (drvdata->numcidc) {
-			config->addr_acc[idx] |= BIT(2);
-			config->addr_acc[idx] &= ~BIT(3);
+			config->addr_acc[idx] |= TRCACATRn_CONTEXTTYPE_CTXID;
+			config->addr_acc[idx] &= ~TRCACATRn_CONTEXTTYPE_VMID;
 		}
 	} else if (!strcmp(str, "vmid")) {
 		/* 0b10 The trace unit performs a VMID */
 		if (drvdata->numvmidc) {
-			config->addr_acc[idx] &= ~BIT(2);
-			config->addr_acc[idx] |= BIT(3);
+			config->addr_acc[idx] &= ~TRCACATRn_CONTEXTTYPE_CTXID;
+			config->addr_acc[idx] |= TRCACATRn_CONTEXTTYPE_VMID;
 		}
 	} else if (!strcmp(str, "all")) {
 		/*
@@ -1194,9 +1194,9 @@ static ssize_t addr_ctxtype_store(struct device *dev,
 		 * comparison and a VMID
 		 */
 		if (drvdata->numcidc)
-			config->addr_acc[idx] |= BIT(2);
+			config->addr_acc[idx] |= TRCACATRn_CONTEXTTYPE_CTXID;
 		if (drvdata->numvmidc)
-			config->addr_acc[idx] |= BIT(3);
+			config->addr_acc[idx] |= TRCACATRn_CONTEXTTYPE_VMID;
 	}
 	spin_unlock(&drvdata->spinlock);
 	return size;
@@ -1215,7 +1215,7 @@ static ssize_t addr_context_show(struct device *dev,
 	spin_lock(&drvdata->spinlock);
 	idx = config->addr_idx;
 	/* context ID comparator bits[6:4] */
-	val = BMVAL(config->addr_acc[idx], 4, 6);
+	val = FIELD_GET(TRCACATRn_CONTEXT_MASK, config->addr_acc[idx]);
 	spin_unlock(&drvdata->spinlock);
 	return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
 }
@@ -1240,8 +1240,8 @@ static ssize_t addr_context_store(struct device *dev,
 	spin_lock(&drvdata->spinlock);
 	idx = config->addr_idx;
 	/* clear context ID comparator bits[6:4] */
-	config->addr_acc[idx] &= ~(BIT(4) | BIT(5) | BIT(6));
-	config->addr_acc[idx] |= (val << 4);
+	config->addr_acc[idx] &= ~TRCACATRn_CONTEXT_MASK;
+	config->addr_acc[idx] |= val << __bf_shf(TRCACATRn_CONTEXT_MASK);
 	spin_unlock(&drvdata->spinlock);
 	return size;
 }
@@ -1258,7 +1258,7 @@ static ssize_t addr_exlevel_s_ns_show(struct device *dev,
 
 	spin_lock(&drvdata->spinlock);
 	idx = config->addr_idx;
-	val = BMVAL(config->addr_acc[idx], 8, 14);
+	val = FIELD_GET(TRCACATRn_EXLEVEL_MASK, config->addr_acc[idx]);
 	spin_unlock(&drvdata->spinlock);
 	return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
 }
@@ -1275,14 +1275,14 @@ static ssize_t addr_exlevel_s_ns_store(struct device *dev,
 	if (kstrtoul(buf, 0, &val))
 		return -EINVAL;
 
-	if (val & ~((GENMASK(14, 8) >> 8)))
+	if (val & ~(TRCACATRn_EXLEVEL_MASK >> __bf_shf(TRCACATRn_EXLEVEL_MASK)))
 		return -EINVAL;
 
 	spin_lock(&drvdata->spinlock);
 	idx = config->addr_idx;
 	/* clear Exlevel_ns & Exlevel_s bits[14:12, 11:8], bit[15] is res0 */
-	config->addr_acc[idx] &= ~(GENMASK(14, 8));
-	config->addr_acc[idx] |= (val << 8);
+	config->addr_acc[idx] &= ~TRCACATRn_EXLEVEL_MASK;
+	config->addr_acc[idx] |= val << __bf_shf(TRCACATRn_EXLEVEL_MASK);
 	spin_unlock(&drvdata->spinlock);
 	return size;
 }
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
index 9cacc38b1890..802ddbe2eecd 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.h
+++ b/drivers/hwtracing/coresight/coresight-etm4x.h
@@ -208,6 +208,12 @@
 #define TRCVICTLR_EXLEVEL_S_MASK		GENMASK(19, 16)
 #define TRCVICTLR_EXLEVEL_NS_MASK		GENMASK(22, 20)
 
+#define TRCACATRn_TYPE_MASK			GENMASK(1, 0)
+#define TRCACATRn_CONTEXTTYPE_MASK		GENMASK(3, 2)
+#define TRCACATRn_CONTEXTTYPE_CTXID		BIT(2)
+#define TRCACATRn_CONTEXTTYPE_VMID		BIT(3)
+#define TRCACATRn_CONTEXT_MASK			GENMASK(6, 4)
+#define TRCACATRn_EXLEVEL_MASK			GENMASK(14, 8)
 /*
  * System instructions to access ETM registers.
  * See ETMv4.4 spec ARM IHI0064F section 4.3.6 System instructions
@@ -1050,10 +1056,10 @@ struct etmv4_drvdata {
 
 /* Address comparator access types */
 enum etm_addr_acctype {
-	ETM_INSTR_ADDR,
-	ETM_DATA_LOAD_ADDR,
-	ETM_DATA_STORE_ADDR,
-	ETM_DATA_LOAD_STORE_ADDR,
+	TRCACATRn_TYPE_ADDR,
+	TRCACATRn_TYPE_DATA_LOAD_ADDR,
+	TRCACATRn_TYPE_DATA_STORE_ADDR,
+	TRCACATRn_TYPE_DATA_LOAD_STORE_ADDR,
 };
 
 /* Address comparator context types */
-- 
2.28.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 11/15] coresight: etm4x: Cleanup TRCACATRn register accesses

[Mike Leach]

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-sysfs.c         | 42 +++++++++----------
>  drivers/hwtracing/coresight/coresight-etm4x.h | 14 +++++--
>  2 files changed, 31 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> index b3b1b92909cc..29188b1a4646 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> @@ -22,7 +22,7 @@ static int etm4_set_mode_exclude(struct etmv4_drvdata *drvdata, bool exclude)
>          * TRCACATRn.TYPE bit[1:0]: type of comparison
>          * the trace unit performs
>          */
> -       if (BMVAL(config->addr_acc[idx], 0, 1) == ETM_INSTR_ADDR) {
> +       if (FIELD_GET(TRCACATRn_TYPE_MASK, config->addr_acc[idx]) == TRCACATRn_TYPE_ADDR) {
>                 if (idx % 2 != 0)
>                         return -EINVAL;
>
> @@ -863,11 +863,11 @@ static ssize_t addr_instdatatype_show(struct device *dev,
>
>         spin_lock(&drvdata->spinlock);
>         idx = config->addr_idx;
> -       val = BMVAL(config->addr_acc[idx], 0, 1);
> +       val = FIELD_GET(TRCACATRn_TYPE_MASK, config->addr_acc[idx]);
>         len = scnprintf(buf, PAGE_SIZE, "%s\n",
> -                       val == ETM_INSTR_ADDR ? "instr" :
> -                       (val == ETM_DATA_LOAD_ADDR ? "data_load" :
> -                       (val == ETM_DATA_STORE_ADDR ? "data_store" :
> +                       val == TRCACATRn_TYPE_ADDR ? "instr" :
> +                       (val == TRCACATRn_TYPE_DATA_LOAD_ADDR ? "data_load" :
> +                       (val == TRCACATRn_TYPE_DATA_STORE_ADDR ? "data_store" :
>                         "data_load_store")));
>         spin_unlock(&drvdata->spinlock);
>         return len;
> @@ -891,7 +891,7 @@ static ssize_t addr_instdatatype_store(struct device *dev,
>         idx = config->addr_idx;
>         if (!strcmp(str, "instr"))
>                 /* TYPE, bits[1:0] */
> -               config->addr_acc[idx] &= ~(BIT(0) | BIT(1));
> +               config->addr_acc[idx] &= ~TRCACATRn_TYPE_MASK;
>
>         spin_unlock(&drvdata->spinlock);
>         return size;
> @@ -1149,7 +1149,7 @@ static ssize_t addr_ctxtype_show(struct device *dev,
>         spin_lock(&drvdata->spinlock);
>         idx = config->addr_idx;
>         /* CONTEXTTYPE, bits[3:2] */
> -       val = BMVAL(config->addr_acc[idx], 2, 3);
> +       val = FIELD_GET(TRCACATRn_CONTEXTTYPE_MASK, config->addr_acc[idx]);
>         len = scnprintf(buf, PAGE_SIZE, "%s\n", val == ETM_CTX_NONE ? "none" :
>                         (val == ETM_CTX_CTXID ? "ctxid" :
>                         (val == ETM_CTX_VMID ? "vmid" : "all")));
> @@ -1175,18 +1175,18 @@ static ssize_t addr_ctxtype_store(struct device *dev,
>         idx = config->addr_idx;
>         if (!strcmp(str, "none"))
>                 /* start by clearing context type bits */
> -               config->addr_acc[idx] &= ~(BIT(2) | BIT(3));
> +               config->addr_acc[idx] &= ~TRCACATRn_CONTEXTTYPE_MASK;
>         else if (!strcmp(str, "ctxid")) {
>                 /* 0b01 The trace unit performs a Context ID */
>                 if (drvdata->numcidc) {
> -                       config->addr_acc[idx] |= BIT(2);
> -                       config->addr_acc[idx] &= ~BIT(3);
> +                       config->addr_acc[idx] |= TRCACATRn_CONTEXTTYPE_CTXID;
> +                       config->addr_acc[idx] &= ~TRCACATRn_CONTEXTTYPE_VMID;
>                 }
>         } else if (!strcmp(str, "vmid")) {
>                 /* 0b10 The trace unit performs a VMID */
>                 if (drvdata->numvmidc) {
> -                       config->addr_acc[idx] &= ~BIT(2);
> -                       config->addr_acc[idx] |= BIT(3);
> +                       config->addr_acc[idx] &= ~TRCACATRn_CONTEXTTYPE_CTXID;
> +                       config->addr_acc[idx] |= TRCACATRn_CONTEXTTYPE_VMID;
>                 }
>         } else if (!strcmp(str, "all")) {
>                 /*
> @@ -1194,9 +1194,9 @@ static ssize_t addr_ctxtype_store(struct device *dev,
>                  * comparison and a VMID
>                  */
>                 if (drvdata->numcidc)
> -                       config->addr_acc[idx] |= BIT(2);
> +                       config->addr_acc[idx] |= TRCACATRn_CONTEXTTYPE_CTXID;
>                 if (drvdata->numvmidc)
> -                       config->addr_acc[idx] |= BIT(3);
> +                       config->addr_acc[idx] |= TRCACATRn_CONTEXTTYPE_VMID;
>         }
>         spin_unlock(&drvdata->spinlock);
>         return size;
> @@ -1215,7 +1215,7 @@ static ssize_t addr_context_show(struct device *dev,
>         spin_lock(&drvdata->spinlock);
>         idx = config->addr_idx;
>         /* context ID comparator bits[6:4] */
> -       val = BMVAL(config->addr_acc[idx], 4, 6);
> +       val = FIELD_GET(TRCACATRn_CONTEXT_MASK, config->addr_acc[idx]);
>         spin_unlock(&drvdata->spinlock);
>         return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
>  }
> @@ -1240,8 +1240,8 @@ static ssize_t addr_context_store(struct device *dev,
>         spin_lock(&drvdata->spinlock);
>         idx = config->addr_idx;
>         /* clear context ID comparator bits[6:4] */
> -       config->addr_acc[idx] &= ~(BIT(4) | BIT(5) | BIT(6));
> -       config->addr_acc[idx] |= (val << 4);
> +       config->addr_acc[idx] &= ~TRCACATRn_CONTEXT_MASK;
> +       config->addr_acc[idx] |= val << __bf_shf(TRCACATRn_CONTEXT_MASK);
>         spin_unlock(&drvdata->spinlock);
>         return size;
>  }
> @@ -1258,7 +1258,7 @@ static ssize_t addr_exlevel_s_ns_show(struct device *dev,
>
>         spin_lock(&drvdata->spinlock);
>         idx = config->addr_idx;
> -       val = BMVAL(config->addr_acc[idx], 8, 14);
> +       val = FIELD_GET(TRCACATRn_EXLEVEL_MASK, config->addr_acc[idx]);
>         spin_unlock(&drvdata->spinlock);
>         return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
>  }
> @@ -1275,14 +1275,14 @@ static ssize_t addr_exlevel_s_ns_store(struct device *dev,
>         if (kstrtoul(buf, 0, &val))
>                 return -EINVAL;
>
> -       if (val & ~((GENMASK(14, 8) >> 8)))
> +       if (val & ~(TRCACATRn_EXLEVEL_MASK >> __bf_shf(TRCACATRn_EXLEVEL_MASK)))
>                 return -EINVAL;
>
>         spin_lock(&drvdata->spinlock);
>         idx = config->addr_idx;
>         /* clear Exlevel_ns & Exlevel_s bits[14:12, 11:8], bit[15] is res0 */
> -       config->addr_acc[idx] &= ~(GENMASK(14, 8));
> -       config->addr_acc[idx] |= (val << 8);
> +       config->addr_acc[idx] &= ~TRCACATRn_EXLEVEL_MASK;
> +       config->addr_acc[idx] |= val << __bf_shf(TRCACATRn_EXLEVEL_MASK);
>         spin_unlock(&drvdata->spinlock);
>         return size;
>  }
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
> index 9cacc38b1890..802ddbe2eecd 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x.h
> +++ b/drivers/hwtracing/coresight/coresight-etm4x.h
> @@ -208,6 +208,12 @@
>  #define TRCVICTLR_EXLEVEL_S_MASK               GENMASK(19, 16)
>  #define TRCVICTLR_EXLEVEL_NS_MASK              GENMASK(22, 20)
>
> +#define TRCACATRn_TYPE_MASK                    GENMASK(1, 0)
> +#define TRCACATRn_CONTEXTTYPE_MASK             GENMASK(3, 2)
> +#define TRCACATRn_CONTEXTTYPE_CTXID            BIT(2)
> +#define TRCACATRn_CONTEXTTYPE_VMID             BIT(3)
> +#define TRCACATRn_CONTEXT_MASK                 GENMASK(6, 4)
> +#define TRCACATRn_EXLEVEL_MASK                 GENMASK(14, 8)
>  /*
>   * System instructions to access ETM registers.
>   * See ETMv4.4 spec ARM IHI0064F section 4.3.6 System instructions
> @@ -1050,10 +1056,10 @@ struct etmv4_drvdata {
>
>  /* Address comparator access types */
>  enum etm_addr_acctype {
> -       ETM_INSTR_ADDR,
> -       ETM_DATA_LOAD_ADDR,
> -       ETM_DATA_STORE_ADDR,
> -       ETM_DATA_LOAD_STORE_ADDR,
> +       TRCACATRn_TYPE_ADDR,
> +       TRCACATRn_TYPE_DATA_LOAD_ADDR,
> +       TRCACATRn_TYPE_DATA_STORE_ADDR,
> +       TRCACATRn_TYPE_DATA_LOAD_STORE_ADDR,
>  };
>
>  /* Address comparator context types */
> --
> 2.28.0
>

Reviewed-by: Mike Leach <mike.leach@linaro.org>

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 12/15] coresight: etm4x: Cleanup TRCSSCCRn and TRCSSCSRn register accesses

[James Clark]

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>
---
 drivers/hwtracing/coresight/coresight-etm4x-core.c  | 2 +-
 drivers/hwtracing/coresight/coresight-etm4x-sysfs.c | 6 +++---
 drivers/hwtracing/coresight/coresight-etm4x.h       | 4 ++++
 3 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index 88353f8ba414..87299e99dabb 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -443,7 +443,7 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata)
 	for (i = 0; i < drvdata->nr_ss_cmp; i++) {
 		/* always clear status bit on restart if using single-shot */
 		if (config->ss_ctrl[i] || config->ss_pe_cmp[i])
-			config->ss_status[i] &= ~BIT(31);
+			config->ss_status[i] &= ~TRCSSCSRn_STATUS;
 		etm4x_relaxed_write32(csa, config->ss_ctrl[i], TRCSSCCRn(i));
 		etm4x_relaxed_write32(csa, config->ss_status[i], TRCSSCSRn(i));
 		if (etm4x_sspcicrn_present(drvdata, i))
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
index 29188b1a4646..7dd7636fc2a7 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
@@ -1792,9 +1792,9 @@ static ssize_t sshot_ctrl_store(struct device *dev,
 
 	spin_lock(&drvdata->spinlock);
 	idx = config->ss_idx;
-	config->ss_ctrl[idx] = val & GENMASK(24, 0);
+	config->ss_ctrl[idx] = FIELD_PREP(TRCSSCCRn_SAC_ARC_RST_MASK, val);
 	/* must clear bit 31 in related status register on programming */
-	config->ss_status[idx] &= ~BIT(31);
+	config->ss_status[idx] &= ~TRCSSCSRn_STATUS;
 	spin_unlock(&drvdata->spinlock);
 	return size;
 }
@@ -1844,7 +1844,7 @@ static ssize_t sshot_pe_ctrl_store(struct device *dev,
 	idx = config->ss_idx;
 	config->ss_pe_cmp[idx] = val & GENMASK(7, 0);
 	/* must clear bit 31 in related status register on programming */
-	config->ss_status[idx] &= ~BIT(31);
+	config->ss_status[idx] &= ~TRCSSCSRn_STATUS;
 	spin_unlock(&drvdata->spinlock);
 	return size;
 }
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
index 802ddbe2eecd..b4217eaab450 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.h
+++ b/drivers/hwtracing/coresight/coresight-etm4x.h
@@ -214,6 +214,10 @@
 #define TRCACATRn_CONTEXTTYPE_VMID		BIT(3)
 #define TRCACATRn_CONTEXT_MASK			GENMASK(6, 4)
 #define TRCACATRn_EXLEVEL_MASK			GENMASK(14, 8)
+
+#define TRCSSCSRn_STATUS			BIT(31)
+#define TRCSSCCRn_SAC_ARC_RST_MASK		GENMASK(24, 0)
+
 /*
  * System instructions to access ETM registers.
  * See ETMv4.4 spec ARM IHI0064F section 4.3.6 System instructions
-- 
2.28.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 12/15] coresight: etm4x: Cleanup TRCSSCCRn and TRCSSCSRn register accesses

[Mike Leach]

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>
> ---
>  drivers/hwtracing/coresight/coresight-etm4x-core.c  | 2 +-
>  drivers/hwtracing/coresight/coresight-etm4x-sysfs.c | 6 +++---
>  drivers/hwtracing/coresight/coresight-etm4x.h       | 4 ++++
>  3 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index 88353f8ba414..87299e99dabb 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -443,7 +443,7 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata)
>         for (i = 0; i < drvdata->nr_ss_cmp; i++) {
>                 /* always clear status bit on restart if using single-shot */
>                 if (config->ss_ctrl[i] || config->ss_pe_cmp[i])
> -                       config->ss_status[i] &= ~BIT(31);
> +                       config->ss_status[i] &= ~TRCSSCSRn_STATUS;
>                 etm4x_relaxed_write32(csa, config->ss_ctrl[i], TRCSSCCRn(i));
>                 etm4x_relaxed_write32(csa, config->ss_status[i], TRCSSCSRn(i));
>                 if (etm4x_sspcicrn_present(drvdata, i))
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> index 29188b1a4646..7dd7636fc2a7 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> @@ -1792,9 +1792,9 @@ static ssize_t sshot_ctrl_store(struct device *dev,
>
>         spin_lock(&drvdata->spinlock);
>         idx = config->ss_idx;
> -       config->ss_ctrl[idx] = val & GENMASK(24, 0);
> +       config->ss_ctrl[idx] = FIELD_PREP(TRCSSCCRn_SAC_ARC_RST_MASK, val);
>         /* must clear bit 31 in related status register on programming */
> -       config->ss_status[idx] &= ~BIT(31);
> +       config->ss_status[idx] &= ~TRCSSCSRn_STATUS;
>         spin_unlock(&drvdata->spinlock);
>         return size;
>  }
> @@ -1844,7 +1844,7 @@ static ssize_t sshot_pe_ctrl_store(struct device *dev,
>         idx = config->ss_idx;
>         config->ss_pe_cmp[idx] = val & GENMASK(7, 0);
>         /* must clear bit 31 in related status register on programming */
> -       config->ss_status[idx] &= ~BIT(31);
> +       config->ss_status[idx] &= ~TRCSSCSRn_STATUS;
>         spin_unlock(&drvdata->spinlock);
>         return size;
>  }
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
> index 802ddbe2eecd..b4217eaab450 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x.h
> +++ b/drivers/hwtracing/coresight/coresight-etm4x.h
> @@ -214,6 +214,10 @@
>  #define TRCACATRn_CONTEXTTYPE_VMID             BIT(3)
>  #define TRCACATRn_CONTEXT_MASK                 GENMASK(6, 4)
>  #define TRCACATRn_EXLEVEL_MASK                 GENMASK(14, 8)
> +
> +#define TRCSSCSRn_STATUS                       BIT(31)
> +#define TRCSSCCRn_SAC_ARC_RST_MASK             GENMASK(24, 0)
> +
>  /*
>   * 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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 13/15] coresight: etm4x: Cleanup TRCSSPCICRn register accesses

[James Clark]

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>
---
 drivers/hwtracing/coresight/coresight-etm4x-sysfs.c | 2 +-
 drivers/hwtracing/coresight/coresight-etm4x.h       | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
index 7dd7636fc2a7..25f76a656308 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
@@ -1842,7 +1842,7 @@ static ssize_t sshot_pe_ctrl_store(struct device *dev,
 
 	spin_lock(&drvdata->spinlock);
 	idx = config->ss_idx;
-	config->ss_pe_cmp[idx] = val & GENMASK(7, 0);
+	config->ss_pe_cmp[idx] = FIELD_PREP(TRCSSPCICRn_PC_MASK, val);
 	/* must clear bit 31 in related status register on programming */
 	config->ss_status[idx] &= ~TRCSSCSRn_STATUS;
 	spin_unlock(&drvdata->spinlock);
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
index b4217eaab450..3b81c104a44b 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.h
+++ b/drivers/hwtracing/coresight/coresight-etm4x.h
@@ -218,6 +218,8 @@
 #define TRCSSCSRn_STATUS			BIT(31)
 #define TRCSSCCRn_SAC_ARC_RST_MASK		GENMASK(24, 0)
 
+#define TRCSSPCICRn_PC_MASK			GENMASK(7, 0)
+
 /*
  * System instructions to access ETM registers.
  * See ETMv4.4 spec ARM IHI0064F section 4.3.6 System instructions
-- 
2.28.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 13/15] coresight: etm4x: Cleanup TRCSSPCICRn register accesses

[Mike Leach]

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>
> ---
>  drivers/hwtracing/coresight/coresight-etm4x-sysfs.c | 2 +-
>  drivers/hwtracing/coresight/coresight-etm4x.h       | 2 ++
>  2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> index 7dd7636fc2a7..25f76a656308 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> @@ -1842,7 +1842,7 @@ static ssize_t sshot_pe_ctrl_store(struct device *dev,
>
>         spin_lock(&drvdata->spinlock);
>         idx = config->ss_idx;
> -       config->ss_pe_cmp[idx] = val & GENMASK(7, 0);
> +       config->ss_pe_cmp[idx] = FIELD_PREP(TRCSSPCICRn_PC_MASK, val);
>         /* must clear bit 31 in related status register on programming */
>         config->ss_status[idx] &= ~TRCSSCSRn_STATUS;
>         spin_unlock(&drvdata->spinlock);
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
> index b4217eaab450..3b81c104a44b 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x.h
> +++ b/drivers/hwtracing/coresight/coresight-etm4x.h
> @@ -218,6 +218,8 @@
>  #define TRCSSCSRn_STATUS                       BIT(31)
>  #define TRCSSCCRn_SAC_ARC_RST_MASK             GENMASK(24, 0)
>
> +#define TRCSSPCICRn_PC_MASK                    GENMASK(7, 0)
> +
>  /*
>   * 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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 14/15] coresight: etm4x: Cleanup TRCBBCTLR register accesses

[James Clark]

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>
---
 drivers/hwtracing/coresight/coresight-etm4x-sysfs.c | 4 ++--
 drivers/hwtracing/coresight/coresight-etm4x.h       | 3 +++
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
index 25f76a656308..3ae6f4432646 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
@@ -707,10 +707,10 @@ static ssize_t bb_ctrl_store(struct device *dev,
 	 * individual range comparators. If include then at least 1
 	 * range must be selected.
 	 */
-	if ((val & BIT(8)) && (BMVAL(val, 0, 7) == 0))
+	if ((val & TRCBBCTLR_MODE) && (FIELD_GET(TRCBBCTLR_RANGE_MASK, val) == 0))
 		return -EINVAL;
 
-	config->bb_ctrl = val & GENMASK(8, 0);
+	config->bb_ctrl = val & (TRCBBCTLR_MODE | TRCBBCTLR_RANGE_MASK);
 	return size;
 }
 static DEVICE_ATTR_RW(bb_ctrl);
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
index 3b81c104a44b..15704982357f 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.h
+++ b/drivers/hwtracing/coresight/coresight-etm4x.h
@@ -220,6 +220,9 @@
 
 #define TRCSSPCICRn_PC_MASK			GENMASK(7, 0)
 
+#define TRCBBCTLR_MODE				BIT(8)
+#define TRCBBCTLR_RANGE_MASK			GENMASK(7, 0)
+
 /*
  * System instructions to access ETM registers.
  * See ETMv4.4 spec ARM IHI0064F section 4.3.6 System instructions
-- 
2.28.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 14/15] coresight: etm4x: Cleanup TRCBBCTLR register accesses

[Mike Leach]

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>
> ---
>  drivers/hwtracing/coresight/coresight-etm4x-sysfs.c | 4 ++--
>  drivers/hwtracing/coresight/coresight-etm4x.h       | 3 +++
>  2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> index 25f76a656308..3ae6f4432646 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> @@ -707,10 +707,10 @@ static ssize_t bb_ctrl_store(struct device *dev,
>          * individual range comparators. If include then at least 1
>          * range must be selected.
>          */
> -       if ((val & BIT(8)) && (BMVAL(val, 0, 7) == 0))
> +       if ((val & TRCBBCTLR_MODE) && (FIELD_GET(TRCBBCTLR_RANGE_MASK, val) == 0))
>                 return -EINVAL;
>
> -       config->bb_ctrl = val & GENMASK(8, 0);
> +       config->bb_ctrl = val & (TRCBBCTLR_MODE | TRCBBCTLR_RANGE_MASK);
>         return size;
>  }
>  static DEVICE_ATTR_RW(bb_ctrl);
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
> index 3b81c104a44b..15704982357f 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x.h
> +++ b/drivers/hwtracing/coresight/coresight-etm4x.h
> @@ -220,6 +220,9 @@
>
>  #define TRCSSPCICRn_PC_MASK                    GENMASK(7, 0)
>
> +#define TRCBBCTLR_MODE                         BIT(8)
> +#define TRCBBCTLR_RANGE_MASK                   GENMASK(7, 0)
> +
>  /*
>   * 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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 15/15] coresight: etm4x: Cleanup TRCRSCTLRn register accesses

[James Clark]

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>
---
 drivers/hwtracing/coresight/coresight-etm4x-sysfs.c | 7 +++++--
 drivers/hwtracing/coresight/coresight-etm4x.h       | 7 +++++++
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
index 3ae6f4432646..6ea8181816fc 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
@@ -1726,8 +1726,11 @@ static ssize_t res_ctrl_store(struct device *dev,
 	/* For odd idx pair inversal bit is RES0 */
 	if (idx % 2 != 0)
 		/* PAIRINV, bit[21] */
-		val &= ~BIT(21);
-	config->res_ctrl[idx] = val & GENMASK(21, 0);
+		val &= ~TRCRSCTLRn_PAIRINV;
+	config->res_ctrl[idx] = val & (TRCRSCTLRn_PAIRINV |
+				       TRCRSCTLRn_INV |
+				       TRCRSCTLRn_GROUP_MASK |
+				       TRCRSCTLRn_SELECT_MASK);
 	spin_unlock(&drvdata->spinlock);
 	return size;
 }
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
index 15704982357f..2c412841b126 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.h
+++ b/drivers/hwtracing/coresight/coresight-etm4x.h
@@ -223,6 +223,13 @@
 #define TRCBBCTLR_MODE				BIT(8)
 #define TRCBBCTLR_RANGE_MASK			GENMASK(7, 0)
 
+#define TRCRSCTLRn_PAIRINV			BIT(21)
+#define TRCRSCTLRn_INV				BIT(20)
+#define TRCRSCTLRn_GROUP_MASK			GENMASK(19, 16)
+#define TRCRSCTLRn_SELECT_MASK			GENMASK(15, 0)
+
+
+
 /*
  * System instructions to access ETM registers.
  * See ETMv4.4 spec ARM IHI0064F section 4.3.6 System instructions
-- 
2.28.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Mathieu Poirier]

On Fri, Mar 04, 2022 at 05:19:12PM +0000, James Clark 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>
> ---
>  drivers/hwtracing/coresight/coresight-etm4x-sysfs.c | 7 +++++--
>  drivers/hwtracing/coresight/coresight-etm4x.h       | 7 +++++++
>  2 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> index 3ae6f4432646..6ea8181816fc 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> @@ -1726,8 +1726,11 @@ static ssize_t res_ctrl_store(struct device *dev,
>  	/* For odd idx pair inversal bit is RES0 */
>  	if (idx % 2 != 0)
>  		/* PAIRINV, bit[21] */
> -		val &= ~BIT(21);
> -	config->res_ctrl[idx] = val & GENMASK(21, 0);
> +		val &= ~TRCRSCTLRn_PAIRINV;
> +	config->res_ctrl[idx] = val & (TRCRSCTLRn_PAIRINV |
> +				       TRCRSCTLRn_INV |
> +				       TRCRSCTLRn_GROUP_MASK |
> +				       TRCRSCTLRn_SELECT_MASK);
>  	spin_unlock(&drvdata->spinlock);
>  	return size;
>  }
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
> index 15704982357f..2c412841b126 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x.h
> +++ b/drivers/hwtracing/coresight/coresight-etm4x.h
> @@ -223,6 +223,13 @@
>  #define TRCBBCTLR_MODE				BIT(8)
>  #define TRCBBCTLR_RANGE_MASK			GENMASK(7, 0)
>  
> +#define TRCRSCTLRn_PAIRINV			BIT(21)
> +#define TRCRSCTLRn_INV				BIT(20)
> +#define TRCRSCTLRn_GROUP_MASK			GENMASK(19, 16)
> +#define TRCRSCTLRn_SELECT_MASK			GENMASK(15, 0)
> +
> +
> +

Two extra lines.

>  /*
>   * System instructions to access ETM registers.
>   * See ETMv4.4 spec ARM IHI0064F section 4.3.6 System instructions
> -- 
> 2.28.0
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Mike Leach]

On Wed, 23 Mar 2022 at 16:15, Mathieu Poirier
<mathieu.poirier@linaro.org> wrote:
>
> On Fri, Mar 04, 2022 at 05:19:12PM +0000, James Clark 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>
> > ---
> >  drivers/hwtracing/coresight/coresight-etm4x-sysfs.c | 7 +++++--
> >  drivers/hwtracing/coresight/coresight-etm4x.h       | 7 +++++++
> >  2 files changed, 12 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> > index 3ae6f4432646..6ea8181816fc 100644
> > --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> > +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> > @@ -1726,8 +1726,11 @@ static ssize_t res_ctrl_store(struct device *dev,
> >       /* For odd idx pair inversal bit is RES0 */
> >       if (idx % 2 != 0)
> >               /* PAIRINV, bit[21] */
> > -             val &= ~BIT(21);
> > -     config->res_ctrl[idx] = val & GENMASK(21, 0);
> > +             val &= ~TRCRSCTLRn_PAIRINV;
> > +     config->res_ctrl[idx] = val & (TRCRSCTLRn_PAIRINV |
> > +                                    TRCRSCTLRn_INV |
> > +                                    TRCRSCTLRn_GROUP_MASK |
> > +                                    TRCRSCTLRn_SELECT_MASK);
> >       spin_unlock(&drvdata->spinlock);
> >       return size;
> >  }
> > diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
> > index 15704982357f..2c412841b126 100644
> > --- a/drivers/hwtracing/coresight/coresight-etm4x.h
> > +++ b/drivers/hwtracing/coresight/coresight-etm4x.h
> > @@ -223,6 +223,13 @@
> >  #define TRCBBCTLR_MODE                               BIT(8)
> >  #define TRCBBCTLR_RANGE_MASK                 GENMASK(7, 0)
> >
> > +#define TRCRSCTLRn_PAIRINV                   BIT(21)
> > +#define TRCRSCTLRn_INV                               BIT(20)
> > +#define TRCRSCTLRn_GROUP_MASK                        GENMASK(19, 16)
> > +#define TRCRSCTLRn_SELECT_MASK                       GENMASK(15, 0)
> > +
> > +
> > +
>
> Two extra lines.
>
> >  /*
> >   * 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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 00/15] Make ETM register accesses consistent with sysreg.h

[Mathieu Poirier]

On Fri, Mar 04, 2022 at 05:18:57PM +0000, James Clark wrote:
> Changes since v2:
>  * Implement Mike's suggestion of not having _SHIFT and using the existing
>    FIELD_GET and FIELD_PREP methods.
>  * Dropped the change to add the new REG_VAL macro because of the above.
>  * FIELD_PREP could be used in some trivial cases, but in some cases the
>    shift is still required but can be calculated with __bf_shf
>  * Improved the commit messages.
>  * The change is still binary equivalent, but requires an extra step 
>    mentioned at the end of this cover letter.
> 
> Applies to coresight/next 3619ee28488
> Also available at https://gitlab.arm.com/linux-arm/linux-jc/-/tree/james-cs-register-refactor-v3
> 
> To check for binary equivalence follow the same steps in the cover letter
> of v2, but apply the following change to coresight-priv.h. This is because
> the existing version of the macros wrap the expression in a new scope {}
> that flips something in the compiler:
> 
>   #undef FIELD_GET
>   #define FIELD_GET(_mask, _reg) (((_reg) & (_mask)) >> __bf_shf(_mask))
>   #undef FIELD_PREP
>   #define FIELD_PREP(_mask, _val) (((_val) << __bf_shf(_mask)) & (_mask))
> 
> Thanks
> James
> 
> James Clark (15):
>   coresight: etm4x: Cleanup TRCIDR0 register accesses
>   coresight: etm4x: Cleanup TRCIDR2 register accesses
>   coresight: etm4x: Cleanup TRCIDR3 register accesses
>   coresight: etm4x: Cleanup TRCIDR4 register accesses
>   coresight: etm4x: Cleanup TRCIDR5 register accesses
>   coresight: etm4x: Cleanup TRCCONFIGR register accesses
>   coresight: etm4x: Cleanup TRCEVENTCTL1R register accesses
>   coresight: etm4x: Cleanup TRCSTALLCTLR register accesses
>   coresight: etm4x: Cleanup TRCVICTLR register accesses
>   coresight: etm3x: Cleanup ETMTECR1 register accesses
>   coresight: etm4x: Cleanup TRCACATRn register accesses
>   coresight: etm4x: Cleanup TRCSSCCRn and TRCSSCSRn register accesses
>   coresight: etm4x: Cleanup TRCSSPCICRn register accesses
>   coresight: etm4x: Cleanup TRCBBCTLR register accesses
>   coresight: etm4x: Cleanup TRCRSCTLRn register accesses
> 
>  .../coresight/coresight-etm3x-core.c          |   2 +-
>  .../coresight/coresight-etm3x-sysfs.c         |   2 +-
>  .../coresight/coresight-etm4x-core.c          | 136 +++++--------
>  .../coresight/coresight-etm4x-sysfs.c         | 180 +++++++++---------
>  drivers/hwtracing/coresight/coresight-etm4x.h | 122 ++++++++++--
>  5 files changed, 244 insertions(+), 198 deletions(-)

I am done reviewing this set.  I will wait until rc1 or rc2 before moving
forward.  If there are other comments needing a respin then I will wait for the
next revision.  Otherwise I will apply this one after correcting the extra lines
at the end of patch 15.
 
Thanks,
Mathieu

> 
> -- 
> 2.28.0
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 00/15] Make ETM register accesses consistent with sysreg.h

[James Clark]

On 23/03/2022 16:22, Mathieu Poirier wrote:
> On Fri, Mar 04, 2022 at 05:18:57PM +0000, James Clark wrote:
>> Changes since v2:
>>  * Implement Mike's suggestion of not having _SHIFT and using the existing
>>    FIELD_GET and FIELD_PREP methods.
>>  * Dropped the change to add the new REG_VAL macro because of the above.
>>  * FIELD_PREP could be used in some trivial cases, but in some cases the
>>    shift is still required but can be calculated with __bf_shf
>>  * Improved the commit messages.
>>  * The change is still binary equivalent, but requires an extra step 
>>    mentioned at the end of this cover letter.
>>
>> Applies to coresight/next 3619ee28488
>> Also available at https://gitlab.arm.com/linux-arm/linux-jc/-/tree/james-cs-register-refactor-v3
>>
>> To check for binary equivalence follow the same steps in the cover letter
>> of v2, but apply the following change to coresight-priv.h. This is because
>> the existing version of the macros wrap the expression in a new scope {}
>> that flips something in the compiler:
>>
>>   #undef FIELD_GET
>>   #define FIELD_GET(_mask, _reg) (((_reg) & (_mask)) >> __bf_shf(_mask))
>>   #undef FIELD_PREP
>>   #define FIELD_PREP(_mask, _val) (((_val) << __bf_shf(_mask)) & (_mask))
>>
>> Thanks
>> James
>>
>> James Clark (15):
>>   coresight: etm4x: Cleanup TRCIDR0 register accesses
>>   coresight: etm4x: Cleanup TRCIDR2 register accesses
>>   coresight: etm4x: Cleanup TRCIDR3 register accesses
>>   coresight: etm4x: Cleanup TRCIDR4 register accesses
>>   coresight: etm4x: Cleanup TRCIDR5 register accesses
>>   coresight: etm4x: Cleanup TRCCONFIGR register accesses
>>   coresight: etm4x: Cleanup TRCEVENTCTL1R register accesses
>>   coresight: etm4x: Cleanup TRCSTALLCTLR register accesses
>>   coresight: etm4x: Cleanup TRCVICTLR register accesses
>>   coresight: etm3x: Cleanup ETMTECR1 register accesses
>>   coresight: etm4x: Cleanup TRCACATRn register accesses
>>   coresight: etm4x: Cleanup TRCSSCCRn and TRCSSCSRn register accesses
>>   coresight: etm4x: Cleanup TRCSSPCICRn register accesses
>>   coresight: etm4x: Cleanup TRCBBCTLR register accesses
>>   coresight: etm4x: Cleanup TRCRSCTLRn register accesses
>>
>>  .../coresight/coresight-etm3x-core.c          |   2 +-
>>  .../coresight/coresight-etm3x-sysfs.c         |   2 +-
>>  .../coresight/coresight-etm4x-core.c          | 136 +++++--------
>>  .../coresight/coresight-etm4x-sysfs.c         | 180 +++++++++---------
>>  drivers/hwtracing/coresight/coresight-etm4x.h | 122 ++++++++++--
>>  5 files changed, 244 insertions(+), 198 deletions(-)
> 
> I am done reviewing this set.  I will wait until rc1 or rc2 before moving
> forward.  If there are other comments needing a respin then I will wait for the
> next revision.  Otherwise I will apply this one after correcting the extra lines
> at the end of patch 15.
>  

Thanks for the review!

> Thanks,
> Mathieu
> 
>>
>> -- 
>> 2.28.0
>>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 00/15] Make ETM register accesses consistent with sysreg.h

[Mathieu Poirier]

On Mon, Mar 28, 2022 at 11:41:48AM +0100, James Clark wrote:
> 
> 
> On 23/03/2022 16:22, Mathieu Poirier wrote:
> > On Fri, Mar 04, 2022 at 05:18:57PM +0000, James Clark wrote:
> >> Changes since v2:
> >>  * Implement Mike's suggestion of not having _SHIFT and using the existing
> >>    FIELD_GET and FIELD_PREP methods.
> >>  * Dropped the change to add the new REG_VAL macro because of the above.
> >>  * FIELD_PREP could be used in some trivial cases, but in some cases the
> >>    shift is still required but can be calculated with __bf_shf
> >>  * Improved the commit messages.
> >>  * The change is still binary equivalent, but requires an extra step 
> >>    mentioned at the end of this cover letter.
> >>
> >> Applies to coresight/next 3619ee28488
> >> Also available at https://gitlab.arm.com/linux-arm/linux-jc/-/tree/james-cs-register-refactor-v3
> >>
> >> To check for binary equivalence follow the same steps in the cover letter
> >> of v2, but apply the following change to coresight-priv.h. This is because
> >> the existing version of the macros wrap the expression in a new scope {}
> >> that flips something in the compiler:
> >>
> >>   #undef FIELD_GET
> >>   #define FIELD_GET(_mask, _reg) (((_reg) & (_mask)) >> __bf_shf(_mask))
> >>   #undef FIELD_PREP
> >>   #define FIELD_PREP(_mask, _val) (((_val) << __bf_shf(_mask)) & (_mask))
> >>
> >> Thanks
> >> James
> >>
> >> James Clark (15):
> >>   coresight: etm4x: Cleanup TRCIDR0 register accesses
> >>   coresight: etm4x: Cleanup TRCIDR2 register accesses
> >>   coresight: etm4x: Cleanup TRCIDR3 register accesses
> >>   coresight: etm4x: Cleanup TRCIDR4 register accesses
> >>   coresight: etm4x: Cleanup TRCIDR5 register accesses
> >>   coresight: etm4x: Cleanup TRCCONFIGR register accesses
> >>   coresight: etm4x: Cleanup TRCEVENTCTL1R register accesses
> >>   coresight: etm4x: Cleanup TRCSTALLCTLR register accesses
> >>   coresight: etm4x: Cleanup TRCVICTLR register accesses
> >>   coresight: etm3x: Cleanup ETMTECR1 register accesses
> >>   coresight: etm4x: Cleanup TRCACATRn register accesses
> >>   coresight: etm4x: Cleanup TRCSSCCRn and TRCSSCSRn register accesses
> >>   coresight: etm4x: Cleanup TRCSSPCICRn register accesses
> >>   coresight: etm4x: Cleanup TRCBBCTLR register accesses
> >>   coresight: etm4x: Cleanup TRCRSCTLRn register accesses
> >>
> >>  .../coresight/coresight-etm3x-core.c          |   2 +-
> >>  .../coresight/coresight-etm3x-sysfs.c         |   2 +-
> >>  .../coresight/coresight-etm4x-core.c          | 136 +++++--------
> >>  .../coresight/coresight-etm4x-sysfs.c         | 180 +++++++++---------
> >>  drivers/hwtracing/coresight/coresight-etm4x.h | 122 ++++++++++--
> >>  5 files changed, 244 insertions(+), 198 deletions(-)
> > 
> > I am done reviewing this set.  I will wait until rc1 or rc2 before moving
> > forward.  If there are other comments needing a respin then I will wait for the
> > next revision.  Otherwise I will apply this one after correcting the extra lines
> > at the end of patch 15.
> >  
> 
> Thanks for the review!
>

I have applied this set.

> > Thanks,
> > Mathieu
> > 
> >>
> >> -- 
> >> 2.28.0
> >>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 00/15] Make ETM register accesses consistent with sysreg.h

[James Clark]

On 13/04/2022 18:08, Mathieu Poirier wrote:
> On Mon, Mar 28, 2022 at 11:41:48AM +0100, James Clark wrote:
>>
>>
>> On 23/03/2022 16:22, Mathieu Poirier wrote:
>>> On Fri, Mar 04, 2022 at 05:18:57PM +0000, James Clark wrote:
>>>> Changes since v2:
>>>>  * Implement Mike's suggestion of not having _SHIFT and using the existing
>>>>    FIELD_GET and FIELD_PREP methods.
>>>>  * Dropped the change to add the new REG_VAL macro because of the above.
>>>>  * FIELD_PREP could be used in some trivial cases, but in some cases the
>>>>    shift is still required but can be calculated with __bf_shf
>>>>  * Improved the commit messages.
>>>>  * The change is still binary equivalent, but requires an extra step 
>>>>    mentioned at the end of this cover letter.
>>>>
>>>> Applies to coresight/next 3619ee28488
>>>> Also available at https://gitlab.arm.com/linux-arm/linux-jc/-/tree/james-cs-register-refactor-v3
>>>>
>>>> To check for binary equivalence follow the same steps in the cover letter
>>>> of v2, but apply the following change to coresight-priv.h. This is because
>>>> the existing version of the macros wrap the expression in a new scope {}
>>>> that flips something in the compiler:
>>>>
>>>>   #undef FIELD_GET
>>>>   #define FIELD_GET(_mask, _reg) (((_reg) & (_mask)) >> __bf_shf(_mask))
>>>>   #undef FIELD_PREP
>>>>   #define FIELD_PREP(_mask, _val) (((_val) << __bf_shf(_mask)) & (_mask))
>>>>
>>>> Thanks
>>>> James
>>>>
>>>> James Clark (15):
>>>>   coresight: etm4x: Cleanup TRCIDR0 register accesses
>>>>   coresight: etm4x: Cleanup TRCIDR2 register accesses
>>>>   coresight: etm4x: Cleanup TRCIDR3 register accesses
>>>>   coresight: etm4x: Cleanup TRCIDR4 register accesses
>>>>   coresight: etm4x: Cleanup TRCIDR5 register accesses
>>>>   coresight: etm4x: Cleanup TRCCONFIGR register accesses
>>>>   coresight: etm4x: Cleanup TRCEVENTCTL1R register accesses
>>>>   coresight: etm4x: Cleanup TRCSTALLCTLR register accesses
>>>>   coresight: etm4x: Cleanup TRCVICTLR register accesses
>>>>   coresight: etm3x: Cleanup ETMTECR1 register accesses
>>>>   coresight: etm4x: Cleanup TRCACATRn register accesses
>>>>   coresight: etm4x: Cleanup TRCSSCCRn and TRCSSCSRn register accesses
>>>>   coresight: etm4x: Cleanup TRCSSPCICRn register accesses
>>>>   coresight: etm4x: Cleanup TRCBBCTLR register accesses
>>>>   coresight: etm4x: Cleanup TRCRSCTLRn register accesses
>>>>
>>>>  .../coresight/coresight-etm3x-core.c          |   2 +-
>>>>  .../coresight/coresight-etm3x-sysfs.c         |   2 +-
>>>>  .../coresight/coresight-etm4x-core.c          | 136 +++++--------
>>>>  .../coresight/coresight-etm4x-sysfs.c         | 180 +++++++++---------
>>>>  drivers/hwtracing/coresight/coresight-etm4x.h | 122 ++++++++++--
>>>>  5 files changed, 244 insertions(+), 198 deletions(-)
>>>
>>> I am done reviewing this set.  I will wait until rc1 or rc2 before moving
>>> forward.  If there are other comments needing a respin then I will wait for the
>>> next revision.  Otherwise I will apply this one after correcting the extra lines
>>> at the end of patch 15.
>>>  
>>
>> Thanks for the review!
>>
> 
> I have applied this set.

Thanks Mathieu

> 
>>> Thanks,
>>> Mathieu
>>>
>>>>
>>>> -- 
>>>> 2.28.0
>>>>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel