[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/7] hw/nvram/nrf51_nvm: Add nRF51 non-volatile
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH 2/7] hw/nvram/nrf51_nvm: Add nRF51 non-volatile memories |
Date: |
Mon, 6 Aug 2018 17:09:39 +0100 |
On Mon, Aug 6, 2018 at 11:01 AM, Steffen Görtz
<address@hidden> wrote:
> +static uint64_t uicr_read(void *opaque, hwaddr offset, unsigned int size)
> +{
> + Nrf51NVMState *s = NRF51_NVM(opaque);
> +
> + offset >>= 2;
> +
> + return s->uicr_content[offset];
> +}
> +
> +static void uicr_write(void *opaque, hwaddr offset, uint64_t value,
> + unsigned int size)
> +{
> + Nrf51NVMState *s = NRF51_NVM(opaque);
> +
> + offset >>= 2;
> +
> + if (offset >= ARRAY_SIZE(s->uicr_content)) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "%s: bad read offset 0x%" HWADDR_PRIx "\n", __func__,
> offset);
> + return;
> + }
There is asymmetry here: uicr_read() doesn't check offset before
indexing the array but uicr_write() does. Either the check is
necessary or it's not.
I think this check isn't necessary since the memory region is sized
appropriately:
memory_region_init_io(&s->uicr, NULL, &uicr_ops, s, "nrf51_soc.uicr",
sizeof(s->uicr_content));
> +static void nrf51_nvm_reset(DeviceState *dev)
> +{
> + Nrf51NVMState *s = NRF51_NVM(dev);
> +
> + memset(s->empty_page, 0xFF, s->page_size);
> +}
Can this be done in ->realize()? Nothing changes the contents of
empty_page, so a ->reset() function seems unnecessary.
- [Qemu-devel] [PATCH 0/7] arm: nRF51 Devices and Microbit Support, Steffen Görtz, 2018/08/06
- [Qemu-devel] [PATCH 1/7] hw/misc/nrf51_rng: Add NRF51 random number generator peripheral, Steffen Görtz, 2018/08/06
- [Qemu-devel] [PATCH 7/7] hw/display/led_matrix: Add LED matrix display device, Steffen Görtz, 2018/08/06
- [Qemu-devel] [PATCH 5/7] tests/microbit-test: Add Tests for nRF51 GPIO, Steffen Görtz, 2018/08/06
- [Qemu-devel] [PATCH 2/7] hw/nvram/nrf51_nvm: Add nRF51 non-volatile memories, Steffen Görtz, 2018/08/06
- [Qemu-devel] [PATCH 4/7] hw/gpio/nrf51_gpio: Add nRF51 GPIO peripheral, Steffen Görtz, 2018/08/06
- [Qemu-devel] [PATCH 3/7] tests: Add bbc:microbit / nRF51 test suite, Steffen Görtz, 2018/08/06