[U-Boot] [PATCH 6/7] sunxi: Enable CONFIG_SPL_STACK_R

[Hans de Goede]

Hi,

On 13-09-15 20:50, Ian Campbell wrote:
> On Sun, 2015-09-13 at 18:38 +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 13-09-15 18:33, Ian Campbell wrote:
>>> On Sun, 2015-09-13 at 17:42 +0200, Hans de Goede wrote:
>>>> index 7c1507b..6aa1bf2 100644
>>>> --- a/include/configs/sunxi-common.h
>>>> +++ b/include/configs/sunxi-common.h
>>>> @@ -73,6 +73,9 @@
>>>>    #define CONFIG_SYS_LOAD_ADDR		0x22000000 /*
>> default
>>>> load address */
>>>>    #define CONFIG_SYS_TEXT_BASE		0x2a000000
>>>>    #define CONFIG_PRE_CON_BUF_ADDR		0x2f000000
>>>> +/* Note this is primarily set through Kconfig, we redefine it
>> here so that
>>>> + * we get warnings if the Kconfig value mismatches. */
>>>
>>> Mismatches with what? Why can't we just use the Kconfig supplied
>> value
>>> throughout?
>>
>> Mismatches with the value defined here in sunxi-common.h, sunxi
>> -common.h
>> lists all other spl memory addresses right in this block, making it
>> possible to quickly see what goes there. If someone ever decides to
>> tweak
>> the layout, then they will likely forget the single value which is
>> set
>> in the defconfig-s, but they will (presumably) update the copy in
>> sunxi-common.h, as that is sitting there right next to the others.
>>
>> If this happens then the compiler will give a warning (for each C
>> -file)
>> that CONFIG_SPL_STACK_R_ADDR is being redefined.
>>
>> So functionality wise this does nothing, leaving it out will result
>> in an identical build. It is just there to help us poor humans to
>> not forger to update the value in the defconfig files if we ever
>> decide to tweak the SPL memory layout.
>
> Got it, in which case I would drop the "primarily" from the comment,
> since that suggests it is defined "secondarily" here, when really it is
> just for documentation (with a clever trick to stop the docs getting
> out of date).
>
> Maybe even:
>
> /* Note SPL_STACK_R_ADDR is set through Kconfig, we include it here
>   * since it needs to fit in with the other values. By also #defining it
>   * we get warnings if the Kconfig value mismatches. */

That works for me, I'll replace my comment with the one you've suggested.

Regards,

Hans