Re: [PATCH 1/6] arm: mach-k3: Add default ATF location for AM62/AM62a

[Wadim Egorov <w.egorov@phytec.de>]

Am 19.06.24 um 22:02 schrieb Andrew Davis:
> On 6/19/24 1:20 PM, Nishanth Menon wrote:
>> On 17:19-20240619, Dhruva Gole wrote:
>>> Hi,
>>>
>>> On Feb 14, 2024 at 10:30:04 -0600, Andrew Davis wrote:
>>>> There is a default ATF load address that is used for devices that have
>>>> ATF running in SRAM. For AM62 and AM62a, ATF runs from DRAM. Instead
>>>> of having to override the address in every defconfig, make add a
>>>> default for these ATF in DRAM devices.
>>>>
>>>> Signed-off-by: Andrew Davis <afd@ti.com>
>>>> ---
>>>>   arch/arm/mach-k3/Kconfig               | 5 +++--
>>>>   configs/am62ax_evm_a53_defconfig       | 1 -
>>>>   configs/am62x_beagleplay_a53_defconfig | 1 -
>>>>   configs/am62x_evm_a53_defconfig        | 1 -
>>>>   configs/phycore_am62x_a53_defconfig    | 1 -
>>>>   configs/verdin-am62_a53_defconfig      | 1 -
>>>>   6 files changed, 3 insertions(+), 7 deletions(-)
>>>>
>>>
>>> Beagleplay stops booting completely with latest U-boot so I did a little
>>> bisect and it seems like reverting this patch helps.
>>>
>>> I am not sure what other implications there are of reverting this so I
>>> don't suggest immediately dropping it, however some hints around what's
>>> missing on beagleplay vs other platforms would be helpful.
>>>
>>> Because other platforms based off the same SoC don't seem to be
>>> affected.
>>
>> Grumble... I had to rediscover this in parallel as well - Thanks Dhruva.
>> mkimage -l tispl.bin and comparing with kernel log of reserved mem
>> had me completely confused.
>>
>> a) Memory maps are already notorious to manage on complex heterogenous
>>    systems. From beagle perspective, we have no need to go and monkey
>>    with DT defined memory map and DT should be the default and modifying
> 
> Your DT source file is wrong, ATF is at 0x8000_0000 (or any address one
> wants to put in K3_ATF_LOAD_ADDR). DT doesn't "define" hardware, it 
> "describes"
> it. ATF's location is dynamic and configurable, it doesn't belong in DT.
> 
> You have two options, either go update your DT, then update it again every
> time ATF moves. *OR* simply turn on OF_SYSTEM_SETUP and let U-Boot add the
> correct location reserved memory node for you.
> 
> Then you can also drop out the reserved-node from the DT template file.
> Only U-Boot knows where ATF is really placed in RAM, so U-Boot must add
> this info to DT.

I think it is a good idea to remove the nodes from the device tree or at 
least mark them as "templates" and note that they are actually provided 
by the bootloader to make it less confusing.

> 
>>    DT should be explicitly called out with a log (instead of done
>>    "transparently") - so instead of CONFIG_K3_OPTEE_LOAD_ADDR default
>>    being forced from u-boot, it should be made optional, where when
>>    defined, it can overide the dt definition or some variant of that.
>>
>> b) Looks like
>>     https://lore.kernel.org/u-boot/20240214163009.983034-4-afd@ti.com/
>>     missed beagleplay? and we were in for a surprise there - i dont want
> 
> This was the only miss, OF_SYSTEM_SETUP didn't get added to the beagleplay
> config. Bryan just sent the fix for that now. I'll go look for a way to
> make this common across the whole SoC family so we don't again miss any
> new boards.
> 
> Andrew
> 
>>     to switch from default dts for beagleplay to something different just
>>     because of simplicity for users to know exactly the carveouts and
>>     with other s/w starting up on uC, dts is our "canonical truth".
>>