Re: [PATCH v4 17/45] Add RNG200 skeleton

[Peter Maydell]

On Fri, 8 Dec 2023 at 02:33, Sergey Kambalin <serg.oker@gmail.com> wrote:
>
> Signed-off-by: Sergey Kambalin <sergey.kambalin@auriga.com>
> ---
>  hw/misc/bcm2838_rng200.c         | 152 +++++++++++++++++++++++++++++++
>  hw/misc/meson.build              |   1 +
>  hw/misc/trace-events             |   9 ++
>  include/hw/misc/bcm2838_rng200.h |  51 +++++++++++
>  4 files changed, 213 insertions(+)
>  create mode 100644 hw/misc/bcm2838_rng200.c
>  create mode 100644 include/hw/misc/bcm2838_rng200.h
>
> diff --git a/hw/misc/bcm2838_rng200.c b/hw/misc/bcm2838_rng200.c
> new file mode 100644
> index 0000000000..8f64e6a20f
> --- /dev/null
> +++ b/hw/misc/bcm2838_rng200.c
> @@ -0,0 +1,152 @@
> +/*
> + * BCM2838 Random Number Generator emulation
> + *
> + * Copyright (C) 2022 Sergey Pushkarev <sergey.pushkarev@auriga.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qapi/error.h"
> +#include "hw/qdev-properties.h"
> +#include "hw/misc/bcm2838_rng200.h"
> +#include "migration/vmstate.h"
> +#include "trace.h"
> +
> +static const VMStateDescription vmstate_bcm2838_rng200_regs = {
> +    .name = "bcm2838_rng200_regs",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT32(ctrl, BCM2838_rng_regs_t),
> +        VMSTATE_UINT32(int_status, BCM2838_rng_regs_t),
> +        VMSTATE_UINT32(fifo_count, BCM2838_rng_regs_t),
> +        VMSTATE_UINT32(fifo_count_threshold, BCM2838_rng_regs_t),
> +        VMSTATE_UINT32(total_bit_count_threshold, BCM2838_rng_regs_t),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};

We add this vmstate struct in this patch, but then the
following patch deletes it again and opts to instead
define the registers with a simple uint32_t array.

The trace events are also added in this patch but not
used until the next patch.

Since this device isn't a very large one, I'm OK with
you squashing together this patch and the next patch
if you want, so that there's a single patch that implements
the whole device. That makes the patch more than our usual
preference for patch size, but it's probably less effort on
your end than moving code between the two patches so the
second patch isn't deleting parts the first one added.
(You can do that if you prefer, though.)

> +
> +static const VMStateDescription vmstate_bcm2838_rng200 = {
> +    .name = "bcm2838_rng200",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT32(rbg_period, BCM2838Rng200State),
> +        VMSTATE_UINT32(rng_fifo_cap, BCM2838Rng200State),
> +        VMSTATE_BOOL(use_timer, BCM2838Rng200State),
> +
> +        VMSTATE_STRUCT(regs, BCM2838Rng200State, 0, 
> vmstate_bcm2838_rng200_regs,
> +                       BCM2838_rng_regs_t),
> +
> +        VMSTATE_END_OF_LIST()
> +    }
> +};

thanks
-- PMM