Re: [Qemu-devel] [RFC v2] arm: Add NRF51 random number generator peripheral

[Stefan Hajnoczi <stefanha@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2198 bytes --]

On Tue, Jun 26, 2018 at 12:49:43PM +0200, Steffen Görtz wrote:
> Add a model of the NRF51 random number generator peripheral.
> 
> Changes since v1:
>   - Add implementation access size hints to MemoryRegionOps
>   - Fail on error if qcrypto_random_bytes fails
>   - Add references to Nrf51 datasheets

Please put the changelog below '---' so it's excluded from the git log.
(Once the final version of the patch has been merged, the changelog is
no longer useful since git log lacks previous patch versions.)

> 
> Signed-off-by: Steffen Görtz <contrib@steffen-goertz.de>
> ---
>  hw/misc/Makefile.objs       |   1 +
>  hw/misc/nrf51_rng.c         | 242 ++++++++++++++++++++++++++++++++++++
>  include/hw/misc/nrf51_rng.h |  73 +++++++++++
>  3 files changed, 316 insertions(+)
>  create mode 100644 hw/misc/nrf51_rng.c
>  create mode 100644 include/hw/misc/nrf51_rng.h
> 
> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
> index 00e834d0f0..fd8cc97249 100644
> --- a/hw/misc/Makefile.objs
> +++ b/hw/misc/Makefile.objs
> @@ -70,3 +70,4 @@ obj-$(CONFIG_AUX) += auxbus.o
>  obj-$(CONFIG_ASPEED_SOC) += aspeed_scu.o aspeed_sdmc.o
>  obj-y += mmio_interface.o
>  obj-$(CONFIG_MSF2) += msf2-sysreg.o
> +obj-$(CONFIG_NRF51_SOC) += nrf51_rng.o
> diff --git a/hw/misc/nrf51_rng.c b/hw/misc/nrf51_rng.c
> new file mode 100644
> index 0000000000..f9fdcd08e6
> --- /dev/null
> +++ b/hw/misc/nrf51_rng.c
> @@ -0,0 +1,242 @@
> +/*
> + * nrf51_rng.c
> + *
> + * Copyright 2018 Steffen Görtz <contrib@steffen-goertz.de>
> + *
> + * This code is licensed under the GPL version 2 or later.  See
> + * the COPYING file in the top-level directory.
> + */

From my v1 review:

Please include a reference to the datasheet for this device and
mention the full name "Random Number Generator" so the purpose of this
device is clear.

The reference manual URL is:
http://infocenter.nordicsemi.com/pdf/nRF51_RM_v3.0.pdf

(Including the URL is nice because it saves the reader from doing web
searches and deciding whether the results correspond with the device
that was modeled.)

Aside from that:

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]