On Mon, Aug 6, 2018 at 11:01 AM, Steffen Görtz
<cont...@steffen-goertz.de> 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.
