Re: [Qemu-devel] [PATCH 1/7] hw/misc/nrf51_rng: Add NRF51 random number generator peripheral

[Peter Maydell]

On 6 August 2018 at 11:01, Steffen Görtz <address@hidden> wrote:
> Add a model of the NRF51 random number generator peripheral.
> This is a simple random generator that continuously generates
> new random values after startup.
>
> Reference Manual: http://infocenter.nordicsemi.com/pdf/nRF51_RM_v3.0.pdf
>
> Signed-off-by: Steffen Görtz <address@hidden>
> ---
>  hw/misc/Makefile.objs       |   1 +
>  hw/misc/nrf51_rng.c         | 273 ++++++++++++++++++++++++++++++++++++
>  include/hw/misc/nrf51_rng.h |  71 ++++++++++
>  3 files changed, 345 insertions(+)
>  create mode 100644 hw/misc/nrf51_rng.c
>  create mode 100644 include/hw/misc/nrf51_rng.h

> +static void update_irq(Nrf51RNGState *s)

See remark on other patch about consistency of struct naming.

> +{
> +    bool irq = false;
> +    irq |= s->interrupt_enabled && s->event_valrdy;

You don't need to set irq to false and then modify it;
this is equivalent to
   bool irq = s->interrupt_enabled && s->event_valrdy;

> +    qemu_set_irq(s->irq, irq);
> +}

> +static void nrf51_rng_reset(DeviceState *dev)
> +{
> +    Nrf51RNGState *s = NRF51_RNG(dev);

This seems to be missing code to actually reset the fields ?

> +
> +    rng_update_timer(s);
> +}

Otherwise the device looks good.

thanks
-- PMM