Re: [Qemu-devel] [PATCH 2/7] hw/nvram/nrf51_nvm: Add nRF51 non-volatile memories

[Steffen Görtz]

Hi Stefan,

thank you for your review!

> 
> 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.
> 

Addressed in next revision of this patch.

Steffen