[PATCH 2/6] riscv: dts: starfive: DeepComputing FML13V01 binman config

[Simon Glass]

Hi Heinrich,

On Sat, 8 Feb 2025 at 12:57, Heinrich Schuchardt
<heinrich.schuchardt at canonical.com> wrote:
>
>
>
> Simon Glass <sjg at chromium.org> schrieb am Sa., 8. Feb. 2025, 18:16:
>>
>> Hi Heinrich,
>>
>> On Fri, 7 Feb 2025 at 10:23, Heinrich Schuchardt
>> <heinrich.schuchardt at canonical.com> wrote:
>> >
>> > Add the DeepComputing Framework motherboard (FML13V01) to the binman
>> > include for the starfive_visionfive2_defconfig.
>> >
>> > Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>
>> > ---
>> >  arch/riscv/dts/starfive-visionfive2-binman.dtsi | 17 +++++++++++++++++
>> >  1 file changed, 17 insertions(+)
>> >
>> > diff --git a/arch/riscv/dts/starfive-visionfive2-binman.dtsi b/arch/riscv/dts/starfive-visionfive2-binman.dtsi
>> > index 4cce001e80d..63289405fbb 100644
>> > --- a/arch/riscv/dts/starfive-visionfive2-binman.dtsi
>> > +++ b/arch/riscv/dts/starfive-visionfive2-binman.dtsi
>> > @@ -16,6 +16,16 @@
>> >         itb {
>> >                 fit {
>> >                         images {
>> > +                               fdt-jh7110-deepcomputing-fml13v01 {
>>
>> fdt-1 is better than repeating all this information multiple times
>
>
>
> Thank you for reviewing
>
> Non-speaking references like fdt-1 lead to coding errors. This why we don't call our C variables i0000 - i9999.
>
>>
>> > +                                       description = "jh7110-deepcomputing-fml13v01";
>>
>> That should be a description of the board, e.g. its marketing name.
>
>
> The devicetree jh7110-deepcomputing-fml13v01.dtb has
>
> model = "DeepComputing FML13V01";
>
> for this JH7110 board. I could make the formatting a bit nicer.
>
>>
>> > +                                       load = <0x40400000>;
>> > +                                       compression = "none";
>> > +
>> > +                                       blob-ext {
>> > +                                               filename = "dts/upstream/src/riscv/starfive/jh7110-deepcomputing-fml13v01.dtb";
>>
>> This should not be blob-ext. It should be using the normal:
>>
>>    u-boot-dtb {
>>    };
>
>
> Then probably all occurrences should be updated in this file.
>

For this board I see:

CONFIG_SPL_OF_LIST="starfive/jh7110-milkv-mars
starfive/jh7110-pine64-star64
starfive/jh7110-starfive-visionfive-2-v1.2a
starfive/jh7110-starfive-visionfive-2-v1.3b"

so really we should not need to write this out in full. Binman
supports handling the OF_LIST. It looks like you can just delete
arch/riscv/dts/starfive-visionfive2-binman.dtsi and still get an
image, since there is already arch/riscv/dts/binman.dtsi

> Best regards
>
> Heinrich
>
>>
>> > +                                       };
>> > +                               };
>> > +
>> >                                 fdt-jh7110-milkv-mars {
>> >                                         description = "jh7110-milkv-mars";
>> >                                         load = <0x40400000>;
>> > @@ -58,6 +68,13 @@
>> >                         };
>> >
>> >                         configurations {
>> > +                               conf-jh7110-deepcomputing-fml13v01 {
>> > +                                       description = "jh7110-deepcomputing-fml13v01";
>> > +                                       firmware = "opensbi";
>> > +                                       loadables = "uboot";
>> > +                                       fdt = "fdt-jh7110-deepcomputing-fml13v01";
>> > +                               };
>> > +
>> >                                 conf-jh7110-milkv-mars {
>> >                                         description = "jh7110-milkv-mars";
>> >                                         firmware = "opensbi";
>> > --
>> > 2.47.1
>> >

Regards,
SImon