[PATCH] spl: fit: List DTOs applied by SPL in U-Boot control DT

[Quentin Schulz]

Hi Marek,

On 6/28/24 4:42 AM, Marek Vasut wrote:
> On 6/27/24 2:42 PM, Quentin Schulz wrote:
>> Hi Marek,
> 
> Hi,
> 
>> On 6/27/24 1:58 AM, Marek Vasut wrote:
>>> Insert /u-boot,<dto-name> = <index> property into the U-Boot control DT
>>> during SPL DTO application process. This can be used by user to inspect
>>> which DTOs got applied by the SPL and in which order from running 
>>> U-Boot.
>>>
>>> Example:
>>> ```
>>> u-boot=> fdt addr $fdtcontroladdr && fdt list /
>>> Working FDT set to aee9aeb0
>>> / {
>>>          u-boot,fdt-dto-imx8mp-dhcom-pdk3-overlay-rev100 = <0x00000005>;
>>>          u-boot,fdt-dto-imx8mp-dhcom-som-overlay-rev100 = <0x00000004>;
>>>          u-boot,fdt-dto-imx8mp-dhcom-pdk-overlay-eth2xfast = 
>>> <0x00000003>;
>>>          u-boot,fdt-dto-imx8mp-dhcom-som-overlay-eth2xfast = 
>>> <0x00000002>;
>>> ...
>>> ```
>>>
>>
>> Shouldn't this rather be in /config node?
> 
> This is what I had there originally, but then I realized that some DTs 
> might not have the /config node in them (or am I mistaken?), so I moved 
> the new properties into the root node, which surely exists.
> 
> It also keeps the code simpler, as it doesn't have to fiddle with 
> creation of the /config node if it doesn't exist.
> 

I don't think we should pollute the root node with this. If we want to 
be able to expose this in a proper binding to the upstream DT binding 
repo, I'm not sure this is going to fly :/

>> Also, I am highly suggesting to have an additional prefix aside from 
>> u-boot to avoid unfortunate name clashes between DTO and existing 
>> properties.
> 
> OK, fixed in V2.
> 
>> Or could be in its own child node of /config ?
> 
> Please see above.
> 
>> In any case, can you please add or edit a file somewhere to define 
>> this new dt-binding.
> 
> Done in V2
> 
>> Additional question, what is this index for?
> 
> To indicate in which order the DTOs were applied by the SPL. It matches 
> the order in which the the DTOs are stored in the fitImage and the order 
> in which SPL iterates over them. In case the SPL skips a DTO using 
> board_spl_fit_append_fdt_skip(), the integer sequence might have gaps.
> 
>> I am wondering if we cannot simply have a string array with overlays 
>> applied left-to-right or right-to-left instead? Or u-boot,fdt-dto-N = 
>> "imx8mp-dhcom-pdk3-overlay-rev100";
> 
> We can, but that is much harder to test for presence of specific DTO on 
> U-Boot command line. It is easy to test for presence of boolean or 
> integer DT property using 'fdt' command.
> 

Not sure dt-bindings people would love to hear that we adapt the fdt to 
be easier to work with from the U-Boot CLI :)

In any case, another few thoughts that popped up overnight:
1- do we want to have this for SPL DTB modified by TPL? If so, I'm sure 
we want it different from the one passed from SPL to U-Boot proper IFF 
we start from a clean sheet (new DTB) and/or add the DTBO we applied on 
top the DTBOs already applied by the TPL to the SPL DTB before passing 
it to proper
2- Add info about which DTBOs were applied to the kernel DTB?

Cheers,
Quentin