Re: [PATCH v3 05/12] hw/arm: Add NPCM730 and NPCM750 SoC models

[Philippe Mathieu-Daudé]

On 7/3/20 10:56 PM, Havard Skinnemoen wrote:
> On Fri, Jul 3, 2020 at 6:32 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>>
>> On 6/27/20 1:55 AM, Havard Skinnemoen wrote:
>>> +/*
>>> + * This covers the whole MMIO space. We'll use this to catch any MMIO 
>>> accesses
>>> + * that aren't handled by any device.
>>> + */
>>> +#define NPCM7XX_MMIO_BA         (0x80000000)
>>> +#define NPCM7XX_MMIO_SZ         (0x7FFD0000)
>>
>> Could be 0x80000000 since UNIMP_DEVICE is created with low
>> priority.
> 
> Good point, I'll do that.
> 
>>> +    /* I/O space -- unimplemented unless overridden below. */
>>> +    create_unimplemented_device("npcm7xx.io", NPCM7XX_MMIO_BA, 
>>> NPCM7XX_MMIO_SZ);
>>
>> Note by doing that you won't get transaction failures when accessing
>> unassigned regions. This not incorrect, but a bit overkill (this covers
>> almost 2GiB...).
> 
> A lot of that 2GiB space is used by six 128 MiB flash memory
> apertures. But there are some holes that probably should generate
> transaction failures instead of a default response.

See commit 529fc5fd3e ("hw/arm: Add the STM32F4xx SoC") for the
recommended verbose way.

> 
> Would it be OK if I send a patch to tighten up the unimplemented space
> once a few more of the peripherals have been implemented?

Fine by me, this is not a blocker. Joel reviewed your patch anyway,
I trust him :)

Regards,

Phil.