[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Steffen Görtz
Subject: Re: [Qemu-devel] [PATCH 3/7] arm: Add chip variant property to nRF51 SoC
Date: Tue, 21 Aug 2018 12:03:03 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.0

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.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]