Re: [Qemu-devel] [PATCH v7 04/12] register: Define REG and FIELD macros

[Peter Maydell]

On 22 June 2016 at 21:23, Alistair Francis <address@hidden> wrote:
> From: Peter Crosthwaite <address@hidden>
>
> 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 <address@hidden>
> [ EI Changes:
>   * Add Deposit macros
> ]
> Signed-off-by: Edgar E. Iglesias <address@hidden>
> Signed-off-by: Alistair Francis <address@hidden>
> ---

> 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 <address@hidden>

thanks
-- PMM