[PATCH v2 2/2] riscv: dts: starfive: generate u-boot-spl.bin.normal.out

[Jonas Karlman]

On 2023-09-17 02:00, Heinrich Schuchardt wrote:
> On 9/17/23 00:19, Jonas Karlman wrote:
>> On 2023-09-06 14:00, Heinrich Schuchardt wrote:
>>> The StarFive VisionFive 2 board cannot load spl/u-boot-spl.bin but needs a
>>> prefixed header. We have referring to a vendor tool (spl_tool) for this
>>> task. 'mkimage -T sfspl' can generate the prefixed file.
>>>
>>> Use binman to invoke mkimage for the generation of file
>>> spl/u-boot-spl.bin.normal.out.
>>>
>>> Update the documentation.
>>>
>>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>
>>> Tested-by: Chanho Park <chanho61.park at samsung.com>
>>> ---
>>> v2:
>>> 	Fix a typo in a comment in tools/sfspl.c
>>> 	Add Tested-by credit
>>> ---
>>>   .../dts/jh7110-starfive-visionfive-2-u-boot.dtsi   | 10 ++++++++++
>>>   doc/board/starfive/visionfive2.rst                 | 14 ++------------
>>>   2 files changed, 12 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/arch/riscv/dts/jh7110-starfive-visionfive-2-u-boot.dtsi b/arch/riscv/dts/jh7110-starfive-visionfive-2-u-boot.dtsi
>>> index 13f69da31e..defe2b605f 100644
>>> --- a/arch/riscv/dts/jh7110-starfive-visionfive-2-u-boot.dtsi
>>> +++ b/arch/riscv/dts/jh7110-starfive-visionfive-2-u-boot.dtsi
>>> @@ -103,4 +103,14 @@
>>>   			};
>>>   		};
>>>   	};
>>> +	u-boot-spl {
>>> +		filename = "spl/u-boot-spl.bin.normal.out";
>>> +
>>> +		mkimage {
>>> +			args = "-T sfspl";
>>> +			blob {
>>> +				filename = "spl/u-boot-spl.bin";
>>> +			};
>>> +		};
>>> +	};
>>
>> This should probably be:
>>
>> mkimage {
>> 	filename = "spl/u-boot-spl.bin.normal.out";
>> 	args = "-T sfspl";
>>
>> 	u-boot-spl {
>> 	};
>> };
> 
> @Jonas
> If I replace the node u-boot-spl by the suggested mkimage node, I get a 
> file spl/u-boot-spl.bin.normal.out which is identical to 
> spl/u-boot-spl.bin. It lacks the header that mkimage should create.

You are correct, with the multiple-images prop in binman node each
subnode is treated as an image and not as an entry type node.
See tools/binman/image.py for some description of image type.

This means we need to have a wrapping image node and cannot directly
use an entry type node at top level under binman node.

> 
> Replacing the blob node by a u-boot-spl node is possible. Whether it is 
> better readable is a matter of taste.

The u-boot-spl entry type is meant to be used for referencing the U-Boot
SPL binary and should be preferred over a plain blob entry.

I would also suggest you name the image node something other then
u-boot-spl because it was easily confused with the entry type of same
name.

E.g. something like:

spl {
	filename = "spl/u-boot-spl.bin.normal.out";

	mkimage {
		args = "-T sfspl";

		u-boot-spl {
		};
	};
};

Regards,
Jonas

> 
> @Simon:
> Could you, please, have a look at doc/develop/package/entries.rst. It 
> seems not to fully describe how binman is controlled via the device-tree.
> 
> * A blob sub-node for the mkimage node is not described.
> * A mkimage node which is not a direct subnode of binman is not mentioned.
> * Why the target filename must be outside of the mkimage node in my case 
> is not evident.
> 
> Maybe a more formal description of the schema in a yaml file would help.
> 
> Best regards
> 
> Heinrich
> 
>>
>> Regards,
>> Jonas
>>
>>>   };
>>> diff --git a/doc/board/starfive/visionfive2.rst b/doc/board/starfive/visionfive2.rst
>>> index 941899a0a4..f5575ab68b 100644
>>> --- a/doc/board/starfive/visionfive2.rst
>>> +++ b/doc/board/starfive/visionfive2.rst
>>> @@ -65,18 +65,8 @@ Now build the U-Boot SPL and U-Boot proper
>>>   	make starfive_visionfive2_defconfig
>>>   	make OPENSBI=$(opensbi_dir)/opensbi/build/platform/generic/firmware/fw_dynamic.bin
>>>   
>>> -This will generate spl/u-boot-spl.bin and FIT image (u-boot.itb)
>>> -
>>> -u-boot-spl.bin cannot be used directly on StarFive VisionFive2,we need
>>> -to convert the u-boot-spl.bin to u-boot-spl.bin.normal.out with
>>> -the below command:
>>> -
>>> -	./spl_tool -c -f $(Uboot_PATH)/spl/u-boot-spl.bin
>>> -
>>> -More detailed description of spl_tool,please refer spl_tool documenation.
>>> -(Note: spl_tool git repo is at https://github.com/starfive-tech/Tools/tree/master/spl_tool
>>> -
>>> -This will generate u-boot-spl.bin.normal.out file.
>>> +This will generate the U-Boot SPL image (spl/u-boot-spl.bin.normal.out) as well
>>> +as the FIT image (u-boot.itb) with OpenSBI and U-Boot.
>>>   
>>>   Flashing
>>>   ~~~~~~~~
>>