[PATCH] rpi: always set fdt_addr with firmware-provided FDT address

[Matthias Brugger]

Hi Mauro,

On 29/09/2021 12:14, Mauro Salvini wrote:
> Hi Matthias
> 
> On 15/09/21 13:16, mbrugger at suse.com (Matthias Brugger) wrote:
>> Hi Mauro,
>>
>> On 07/06/2021 11:27, Mauro Salvini wrote:
>>> On 12/05/21 14:39, Mauro Salvini wrote:
>>>> Raspberry firmware prepares the FDT blob in memory at an address
>>>> that depends on both the memory size and the blob size [1].
>>>> After commit ade243a211d6 ("rpi: passthrough of the firmware provided FDT
>>>> blob") this FDT is passed to kernel through fdt_addr environment variable,
>>>> handled in set_fdt_addr() function in board file.
>>>>
>>>> When u-boot environment is persistently saved, if a change happens
>>>> in loaded FDT (e.g. for a new overlay applied), firmware produces a FDT
>>>> address different from the saved one, but u-boot still use the saved
>>>> one because set_fdt_addr() function does not overwrite the fdt_addr
>>>> variable. So, for example, if there is a script that uses fdt commands for
>>>> e.g. manipulate the bootargs, boot hangs with error
>>>>
>>>> libfdt fdt_check_header(): FDT_ERR_BADMAGIC
>>>>
>>>> Removing the fdt_addr variable in saved environment allows to boot.
>>>>
>>>> With this patch set_fdt_addr() function always overwrite fdt_addr value.
>>>>
>>>> [1] https://www.raspberrypi.org/forums//viewtopic.php?f=107&t=134018
>>>>
>>
>> First of all sorry for the very late reply.
>> I'm hesitant to apply this patch, basically because it can break other setups
>> where people load a custom DTB to fdt_addr.
>>
>> I wonder why you can't erase fdt_addr from your persistent storage. There is a
>> command called eraseenv that should to the job.
> 
> Sorry me too for the late reply.
> 
> So your suggestion is to erase the fdt_addr variable from the environment each 
> time one needs to "refresh" it (one example could be the situation that I ponted 
> out).
> 
> Yes, this could be the solution, but the need to delete the fdt_addr variable 
> when e.g. one changes the dtb loaded by rpi firmware should be documented 
> somewhere.
> 

Hm, maybe I didn't understand the problem. My understanding is, that when you 
save the environment with saveenv, you also save the fdt_addr. And that's a 
problem because if you add a overlay later, the fdt_addr changed, which will not 
be reflected. So my question was if you couldn't just delete the fdt_addr 
variable from you saved environment so that when lateron you load overlays, you 
won't hit the problem.

My understanding was that you are setting a custom environment for your boards, 
but later your customers might add a overlay via e.g. config.txt and that will 
break booting the system.

But from your response it seems thats not what you are experiencing. Or do you 
change the DTB loaded from FW in the U-Boot shell?

Regards,
Matthias

> Thanks, regards
> Mauro
> 
>>
>> Regards,
>> Matthias
>>
>>>> Signed-off-by: Mauro Salvini <m.salvini at koansoftware.com>
>>>> Cc: C?dric Schieli <cschieli at gmail.com>
>>>> Cc: Matthias Brugger <mbrugger at suse.com>
>>>> ---
>>>> ? board/raspberrypi/rpi/rpi.c | 3 ---
>>>> ? 1 file changed, 3 deletions(-)
>>>>
>>>> diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c
>>>> index df52a4689f..611013471e 100644
>>>> --- a/board/raspberrypi/rpi/rpi.c
>>>> +++ b/board/raspberrypi/rpi/rpi.c
>>>> @@ -318,9 +318,6 @@ static void set_fdtfile(void)
>>>> ?? */
>>>> ? static void set_fdt_addr(void)
>>>> ? {
>>>> -??? if (env_get("fdt_addr"))
>>>> -??????? return;
>>>> -
>>>> ????? if (fdt_magic(fw_dtb_pointer) != FDT_MAGIC)
>>>> ????????? return;
>>>>
>>>
>>>
>>> Hi all,
>>>
>>> kind ping.
>>>
>>> Regards
>>>
> 
>