Re: [Qemu-arm] [PATCH 3/7] arm: Add chip variant property to nRF51 SoC

[Steffen Görtz]

Hi Peter,

object_property_set_bool(soc, true, "realized", &error_abort);>>
>> -    armv7m_load_kernel(ARM_CPU(first_cpu), machine->kernel_filename,
>> -            NRF51_SOC(soc)->flash_size);
>> +    armv7m_load_kernel(ARM_CPU(first_cpu), machine->kernel_filename, 0x00);
> 
> Hmm. Using 0x00 here doesn't seem ideal.

i agree. The actual sizes for RAM/ROM are calculated in the SOC at the moment.
I do not really know what impact max_sz has here. I could set the it to the
maximum code size defined in the memory map of the NRF51 series. Another way 
would be 
to lift the calculation of the ROM size from the SOC to the Board. What would 
you propose?
>> +#define PAGE_SIZE       1024
> 
> Page size isn't really an obvious concept for M profile.

Flash pages are referred to here, not MMU pages. This constant is moved and 
renamed to NRF51_PAGE_SIZE in the upcoming revision. Would you suggest another 
name or can with stick with this?

> Why have these lines been changed? (Rule of thumb: don't include code
> reformatting or style cleanups in the same patch as substantive changes.)

These lines where > 80 chars before. I should rather suggest a change of Joel's 
patch series.

Thank you for your remarks.

Steffen