Re: [Qemu-devel] [PATCH v6 02/13] register: Add Register API

[Peter Maydell <peter.maydell@linaro.org>]

On 12 May 2016 at 23:45, Alistair Francis <alistair.francis@xilinx.com> wrote:
> This API provides some encapsulation of registers and factors our some

"out"

> common functionality to common code. Bits of device state (usually MMIO
> registers), often have all sorts of access restrictions and semantics

spurious comma

> associated with them. This API allow you to define what those

"allows"

> restrictions are on a bit-by-bit basis.
>
> Helper functions are then used to access the register which observe the
> semantics defined by the RegisterAccessInfo struct.
>
> Some features:
> Bits can be marked as read_only (ro field)
> Bits can be marked as write-1-clear (w1c field)
> Bits can be marked as reserved (rsvd field)
> Reset values can be defined (reset)
> Bits can be marked clear on read (cor)
> Pre and post action callbacks can be added to read and write ops
> Verbose debugging info can be enabled/disabled
>
> Useful for defining device register spaces in a data driven way. Cuts
> down on a lot of the verbosity and repetition in the switch-case blocks
> in the standard foo_mmio_read/write functions.
>
> Also useful for automated generation of device models from hardware
> design sources.
>
> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> ---
> V5:
>  - Convert to using only one memory region
> V4:
>  - Rebase
>  - Remove the guest error masking
>  - Simplify the unimplemented masking
>  - Use the reserved value in the write calculations
>  - Remove read_lite and write_lite
>  - General fixes to asserts and log printing
> V3:
>  - Address some comments from Fred
>
>  hw/core/Makefile.objs |   1 +
>  hw/core/register.c    | 149 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/hw/register.h | 110 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 260 insertions(+)
>  create mode 100644 hw/core/register.c
>  create mode 100644 include/hw/register.h
>
> diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs
> index abb3560..bf95db5 100644
> --- a/hw/core/Makefile.objs
> +++ b/hw/core/Makefile.objs
> @@ -14,4 +14,5 @@ common-obj-$(CONFIG_SOFTMMU) += machine.o
>  common-obj-$(CONFIG_SOFTMMU) += null-machine.o
>  common-obj-$(CONFIG_SOFTMMU) += loader.o
>  common-obj-$(CONFIG_SOFTMMU) += qdev-properties-system.o
> +common-obj-$(CONFIG_SOFTMMU) += register.o
>  common-obj-$(CONFIG_PLATFORM_BUS) += platform-bus.o
> diff --git a/hw/core/register.c b/hw/core/register.c
> new file mode 100644
> index 0000000..5e6f621
> --- /dev/null
> +++ b/hw/core/register.c
> @@ -0,0 +1,149 @@
> +/*
> + * Register Definition API
> + *
> + * Copyright (c) 2016 Xilinx Inc.
> + * Copyright (c) 2013 Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2.  See
> + * the COPYING file in the top-level directory.
> + */

Is this deliberately GPL2-only rather than 2-or-later ?

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

thanks
-- PMM