diff mbox

[v7,04/12] register: Define REG and FIELD macros

Message ID 7d57d1d9d76bb4ac6c60b6509688e4589e5e9a95.1466626975.git.alistair.francis@xilinx.com
State New
Headers show

Commit Message

Alistair Francis June 22, 2016, 8:23 p.m. UTC 
From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

Define some macros that can be used for defining registers and fields.

The REG32 macro will define A_FOO, for the byte address of a register
as well as R_FOO for the uint32_t[] register number (A_FOO / 4).

The FIELD macro will define FOO_BAR_MASK, FOO_BAR_SHIFT and
FOO_BAR_LENGTH constants for field BAR in register FOO.

Finally, there are some shorthand helpers for extracting/depositing
fields from registers based on these naming schemes.

Usage can greatly reduce the verbosity of device code.

The deposit and extract macros (eg FIELD_EX32, FIELD_DP32  etc.) can be
used to generate extract and deposits without any repetition of the name
stems.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
[ EI Changes:
  * Add Deposit macros
]
Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
---
E.g. Currently you have to define something like:

\#define R_FOOREG (0x84/4)
\#define R_FOOREG_BARFIELD_SHIFT 10
\#define R_FOOREG_BARFIELD_LENGTH 5

uint32_t foobar_val = extract32(s->regs[R_FOOREG],
                                R_FOOREG_BARFIELD_SHIFT,
                                R_FOOREG_BARFIELD_LENGTH);

Which has:
2 macro definitions per field
3 register names ("FOOREG") per extract
2 field names ("BARFIELD") per extract

With these macros this becomes:

REG32(FOOREG, 0x84)
FIELD(FOOREG, BARFIELD, 10, 5)

uint32_t foobar_val = ARRAY_FIELD_EX32(s->regs, FOOREG, BARFIELD)

Which has:
1 macro definition per field
1 register name per extract
1 field name per extract

If you are not using arrays for the register data you can just use the
non-array "FIELD_" variants and still save 2 name stems:

uint32_t foobar_val = FIELD_EX32(s->fooreg, FOOREG, BARFIELD)

Deposit is similar for depositing values. Deposit has compile-time
overflow checking for literals.
For example:

REG32(XYZ1, 0x84)
FIELD(XYZ1, TRC, 0, 4)

/* Correctly set XYZ1.TRC = 5.  */
ARRAY_FIELD_DP32(s->regs, XYZ1, TRC, 5);

/* Incorrectly set XYZ1.TRC = 16.  */
ARRAY_FIELD_DP32(s->regs, XYZ1, TRC, 16);

The latter assignment results in:
warning: large integer implicitly truncated to unsigned type [-Woverflow]


 include/hw/register.h | 43 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

Comments

Peter Maydell June 23, 2016, 12:39 p.m. UTC | #1 
On 22 June 2016 at 21:23, Alistair Francis <alistair.francis@xilinx.com> wrote:
> From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>
> Define some macros that can be used for defining registers and fields.
>
> The REG32 macro will define A_FOO, for the byte address of a register
> as well as R_FOO for the uint32_t[] register number (A_FOO / 4).
>
> The FIELD macro will define FOO_BAR_MASK, FOO_BAR_SHIFT and
> FOO_BAR_LENGTH constants for field BAR in register FOO.
>
> Finally, there are some shorthand helpers for extracting/depositing
> fields from registers based on these naming schemes.
>
> Usage can greatly reduce the verbosity of device code.
>
> The deposit and extract macros (eg FIELD_EX32, FIELD_DP32  etc.) can be
> used to generate extract and deposits without any repetition of the name
> stems.
>
> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> [ EI Changes:
>   * Add Deposit macros
> ]
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
> ---

> diff --git a/include/hw/register.h b/include/hw/register.h
> index e160150..216b679 100644
> --- a/include/hw/register.h
> +++ b/include/hw/register.h
> @@ -150,4 +150,47 @@ void register_write_memory(void *opaque, hwaddr addr, uint64_t value,
>
>  uint64_t register_read_memory(void *opaque, hwaddr addr, unsigned size);
>
> +/* Define constants for a 32 bit register */
> +
> +/* This macro will define A_FOO, for the byte address of a register
> + * as well as R_FOO for the uint32_t[] register number (A_FOO / 4).
> + */
> +#define REG32(reg, addr)                                                  \
> +    enum { A_ ## reg = (addr) };                                          \
> +    enum { R_ ## reg = (addr) / 4 };
> +
> +/* Define SHIFT, LEGTH and MASK constants for a field within a register */
> +

"LENGTH".

> +/* Deposit a register field.  */
> +#define FIELD_DP32(storage, reg, field, val) ({                           \
> +    struct {                                                              \
> +        unsigned int v:R_ ## reg ## _ ## field ## _LENGTH;                \
> +    } v = { .v = val };                                                   \
> +    uint32_t d;                                                           \
> +    d = deposit32((storage), R_ ## reg ## _ ## field ## _SHIFT,           \
> +                  R_ ## reg ## _ ## field ## _LENGTH, v.v);               \
> +    d; })

If you insist on different semantics to deposit32() (which I
still think is a bad idea), can we at least have a comment
noting the difference?

(Do you get a warning for putting a signed negative value into
a field?)

Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM
Alistair Francis June 23, 2016, 5:51 p.m. UTC | #2 
On Thu, Jun 23, 2016 at 5:39 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 22 June 2016 at 21:23, Alistair Francis <alistair.francis@xilinx.com> wrote:
>> From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>>
>> Define some macros that can be used for defining registers and fields.
>>
>> The REG32 macro will define A_FOO, for the byte address of a register
>> as well as R_FOO for the uint32_t[] register number (A_FOO / 4).
>>
>> The FIELD macro will define FOO_BAR_MASK, FOO_BAR_SHIFT and
>> FOO_BAR_LENGTH constants for field BAR in register FOO.
>>
>> Finally, there are some shorthand helpers for extracting/depositing
>> fields from registers based on these naming schemes.
>>
>> Usage can greatly reduce the verbosity of device code.
>>
>> The deposit and extract macros (eg FIELD_EX32, FIELD_DP32  etc.) can be
>> used to generate extract and deposits without any repetition of the name
>> stems.
>>
>> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>> [ EI Changes:
>>   * Add Deposit macros
>> ]
>> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
>> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
>> ---
>
>> diff --git a/include/hw/register.h b/include/hw/register.h
>> index e160150..216b679 100644
>> --- a/include/hw/register.h
>> +++ b/include/hw/register.h
>> @@ -150,4 +150,47 @@ void register_write_memory(void *opaque, hwaddr addr, uint64_t value,
>>
>>  uint64_t register_read_memory(void *opaque, hwaddr addr, unsigned size);
>>
>> +/* Define constants for a 32 bit register */
>> +
>> +/* This macro will define A_FOO, for the byte address of a register
>> + * as well as R_FOO for the uint32_t[] register number (A_FOO / 4).
>> + */
>> +#define REG32(reg, addr)                                                  \
>> +    enum { A_ ## reg = (addr) };                                          \
>> +    enum { R_ ## reg = (addr) / 4 };
>> +
>> +/* Define SHIFT, LEGTH and MASK constants for a field within a register */
>> +
>
> "LENGTH".

Fixed

>
>> +/* Deposit a register field.  */
>> +#define FIELD_DP32(storage, reg, field, val) ({                           \
>> +    struct {                                                              \
>> +        unsigned int v:R_ ## reg ## _ ## field ## _LENGTH;                \
>> +    } v = { .v = val };                                                   \
>> +    uint32_t d;                                                           \
>> +    d = deposit32((storage), R_ ## reg ## _ ## field ## _SHIFT,           \
>> +                  R_ ## reg ## _ ## field ## _LENGTH, v.v);               \
>> +    d; })
>
> If you insist on different semantics to deposit32() (which I
> still think is a bad idea), can we at least have a comment
> noting the difference?

I have added a comment.

>
> (Do you get a warning for putting a signed negative value into
> a field?)

No, I don't see any warnings for forcing negative values in.

Thanks,

Alistair

>
> Otherwise
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>
> thanks
> -- PMM
>
diff mbox

Patch

diff --git a/include/hw/register.h b/include/hw/register.h
index e160150..216b679 100644
--- a/include/hw/register.h
+++ b/include/hw/register.h
@@ -150,4 +150,47 @@  void register_write_memory(void *opaque, hwaddr addr, uint64_t value,
 
 uint64_t register_read_memory(void *opaque, hwaddr addr, unsigned size);
 
+/* Define constants for a 32 bit register */
+
+/* This macro will define A_FOO, for the byte address of a register
+ * as well as R_FOO for the uint32_t[] register number (A_FOO / 4).
+ */
+#define REG32(reg, addr)                                                  \
+    enum { A_ ## reg = (addr) };                                          \
+    enum { R_ ## reg = (addr) / 4 };
+
+/* Define SHIFT, LEGTH and MASK constants for a field within a register */
+
+/* This macro will define FOO_BAR_MASK, FOO_BAR_SHIFT and FOO_BAR_LENGTH 
+ * constants for field BAR in register FOO.
+ */
+#define FIELD(reg, field, shift, length)                                  \
+    enum { R_ ## reg ## _ ## field ## _SHIFT = (shift)};                  \
+    enum { R_ ## reg ## _ ## field ## _LENGTH = (length)};                \
+    enum { R_ ## reg ## _ ## field ## _MASK =                             \
+                                        MAKE_64BIT_MASK(shift, length);
+
+/* Extract a field from a register */
+#define FIELD_EX32(storage, reg, field)                                   \
+    extract32((storage), R_ ## reg ## _ ## field ## _SHIFT,               \
+              R_ ## reg ## _ ## field ## _LENGTH)
+
+/* Extract a field from an array of registers */
+#define ARRAY_FIELD_EX32(regs, reg, field)                                \
+    FIELD_EX32((regs)[R_ ## reg], reg, field)
+
+/* Deposit a register field.  */
+#define FIELD_DP32(storage, reg, field, val) ({                           \
+    struct {                                                              \
+        unsigned int v:R_ ## reg ## _ ## field ## _LENGTH;                \
+    } v = { .v = val };                                                   \
+    uint32_t d;                                                           \
+    d = deposit32((storage), R_ ## reg ## _ ## field ## _SHIFT,           \
+                  R_ ## reg ## _ ## field ## _LENGTH, v.v);               \
+    d; })
+
+/* Deposit a field to array of registers.  */
+#define ARRAY_FIELD_DP32(regs, reg, field, val)                           \
+    (regs)[R_ ## reg] = FIELD_DP32((regs)[R_ ## reg], reg, field, val);
+
 #endif