[U-Boot] [PATCH v3 1/4] rpi: push fw_dtb_pointer in the .data section

[Matthias Brugger]

On 12/11/2019 13:20, Alexander Graf wrote:
> 
> On 12.11.19 13:00, matthias.bgg at kernel.org wrote:
>> From: Matthias Brugger <mbrugger at suse.com>
>>
>> The fw_dtb_pointer was defined in the assembly code, which makes him
>> live in section .text_rest
>> Put that's not necessary, we can push the variable in the .data section.
>>
>> This will prevent relocation errors like:
>> board/raspberrypi/rpi/rpi.c:317:(.text.board_get_usable_ram_top+0x8):
>> relocation truncated to fit: R_AARCH64_LDST64_ABS_LO12_NC against symbol
>> `fw_dtb_pointer' defined in .text section in board/raspberrypi/rpi/built-in.o
>>
>> Signed-off-by: Matthias Brugger <mbrugger at suse.com>
>>
>> ---
>>
>> Changes in v3:
>> - fix armv7 build
>>
>> Changes in v2:
>> - push fw_dtb_pointer into the .data section
>>
>>   board/raspberrypi/rpi/lowlevel_init.S | 12 ++----------
>>   board/raspberrypi/rpi/rpi.c           |  4 ++--
>>   2 files changed, 4 insertions(+), 12 deletions(-)
>>
>> diff --git a/board/raspberrypi/rpi/lowlevel_init.S
>> b/board/raspberrypi/rpi/lowlevel_init.S
>> index 435eed521f..8c39b3e12e 100644
>> --- a/board/raspberrypi/rpi/lowlevel_init.S
>> +++ b/board/raspberrypi/rpi/lowlevel_init.S
>> @@ -6,15 +6,6 @@
>>     #include <config.h>
>>   -.align 8
>> -.global fw_dtb_pointer
>> -fw_dtb_pointer:
>> -#ifdef CONFIG_ARM64
>> -    .dword 0x0
>> -#else
>> -    .word 0x0
>> -#endif
>> -
>>   /*
>>    * Routine: save_boot_params (called after reset from start.S)
>>    * Description: save ATAG/FDT address provided by the firmware at boot time
>> @@ -28,7 +19,8 @@ save_boot_params:
>>       adr    x8, fw_dtb_pointer
>>       str    x0, [x8]
>>   #else
>> -    str    r2, fw_dtb_pointer
>> +    ldr    r8, =fw_dtb_pointer
>> +    str    r2, [r8]
>>   #endif
>>         /* Returns */
>> diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c
>> index 9e0abdda31..0e05d59e1f 100644
>> --- a/board/raspberrypi/rpi/rpi.c
>> +++ b/board/raspberrypi/rpi/rpi.c
>> @@ -27,8 +27,8 @@
>>     DECLARE_GLOBAL_DATA_PTR;
>>   -/* From lowlevel_init.S */
>> -extern unsigned long fw_dtb_pointer;
>> +/* Assigned in lowlevel_init.S */
>> +unsigned long fw_dtb_pointer = 0x1;
> 
> 
> I assume you assign the 0x1 here so that it doesn't land in .bss which may get
> cleared after you wrote it? If so, please document that in the comment. Also

Yes exactly.

> even better yet, document it in a comment and just manually assign the variable
> to the ".data" section using
> 
>   __attribute__((section(".data")))
> 

Agree that's a cleaner approach then adding any random value to the pointer.

Regards,
Matthias