diff mbox series

ARM: Distro boot: document the need for fdtfile variable to be set

Message ID 20200903164034.115210-1-dennis@ausil.us
State Superseded
Delegated to: Tom Rini
Headers show
Series ARM: Distro boot: document the need for fdtfile variable to be set | expand

Commit Message

Dennis Gilmore Sept. 3, 2020, 4:40 p.m. UTC
When testing builds provided in https://github.com/openwrt/openwrt/pull/3360
I discovered that fdtfile was not set and as a result the firmware was not
functional. So I am documenting what is needed.

Signed-off-by: Dennis Gilmore <dennis@ausil.us>

Cc: Atish Patra <atish.patra@wdc.com>
Cc: Lukas Auer <lukas.auer@aisec.fraunhofer.de>
Cc: Tom Rini <trini@konsulko.com>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Vagrant Cascadian <vagrant@debian.org>
Cc: Stephen Warren <swarren@nvidia.com>
Cc: Karsten Merker <merker@debian.org>
---
 doc/README.distro | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Andre Heider Sept. 3, 2020, 5:50 p.m. UTC | #1
Hi Dennis,

On 03/09/2020 18:40, Dennis Gilmore wrote:
> When testing builds provided in https://github.com/openwrt/openwrt/pull/3360
> I discovered that fdtfile was not set and as a result the firmware was not
> functional. So I am documenting what is needed.
> 
> Signed-off-by: Dennis Gilmore <dennis@ausil.us>
> 
> Cc: Atish Patra <atish.patra@wdc.com>
> Cc: Lukas Auer <lukas.auer@aisec.fraunhofer.de>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> Cc: Vagrant Cascadian <vagrant@debian.org>
> Cc: Stephen Warren <swarren@nvidia.com>
> Cc: Karsten Merker <merker@debian.org>
> ---
>   doc/README.distro | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/doc/README.distro b/doc/README.distro
> index 5076bebd18..3eb70aeb14 100644
> --- a/doc/README.distro
> +++ b/doc/README.distro
> @@ -224,6 +224,14 @@ fdt_addr_r:
>   
>     A size of 1MB for the FDT/DTB seems reasonable.
>   
> +fdtfile:
> +
> +  Mandatory. the name of the DTB file for the specific board for instance
> +  the espressobin v5 board the value is "marvell/armada-3720-espressobin.dtb"
> +  while on a clearfog pro it is "armada-388-clearfog-pro.dtb" in the case of
> +  a board providing its firmware based DTB this value can be used to override
> +  the DTB with a different DTB.

is that really supposed to include the vendor subdir on arm64?

If so, adding fdtfile like that would break booting debian using its 
flash-image. It installs dtbs without that subdir into /boot:
/boot/dtbs/4.19.0-9-arm64/armada-3720-espressobin.dtb

But it already supports $fdtfile:
https://salsa.debian.org/installer-team/flash-kernel/-/blob/master/bootscript/arm64/bootscr.uboot-generic#L32

If I read that script correctly, it would fail to boot with 
fdtfile=marvell/armada-3720-espressobin.dtb...

Regards,
Andre
Tom Rini Sept. 3, 2020, 5:54 p.m. UTC | #2
On Thu, Sep 03, 2020 at 07:50:32PM +0200, Andre Heider wrote:
> Hi Dennis,
> 
> On 03/09/2020 18:40, Dennis Gilmore wrote:
> > When testing builds provided in https://github.com/openwrt/openwrt/pull/3360
> > I discovered that fdtfile was not set and as a result the firmware was not
> > functional. So I am documenting what is needed.
> > 
> > Signed-off-by: Dennis Gilmore <dennis@ausil.us>
> > 
> > Cc: Atish Patra <atish.patra@wdc.com>
> > Cc: Lukas Auer <lukas.auer@aisec.fraunhofer.de>
> > Cc: Tom Rini <trini@konsulko.com>
> > Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> > Cc: Vagrant Cascadian <vagrant@debian.org>
> > Cc: Stephen Warren <swarren@nvidia.com>
> > Cc: Karsten Merker <merker@debian.org>
> > ---
> >   doc/README.distro | 8 ++++++++
> >   1 file changed, 8 insertions(+)
> > 
> > diff --git a/doc/README.distro b/doc/README.distro
> > index 5076bebd18..3eb70aeb14 100644
> > --- a/doc/README.distro
> > +++ b/doc/README.distro
> > @@ -224,6 +224,14 @@ fdt_addr_r:
> >     A size of 1MB for the FDT/DTB seems reasonable.
> > +fdtfile:
> > +
> > +  Mandatory. the name of the DTB file for the specific board for instance
> > +  the espressobin v5 board the value is "marvell/armada-3720-espressobin.dtb"
> > +  while on a clearfog pro it is "armada-388-clearfog-pro.dtb" in the case of
> > +  a board providing its firmware based DTB this value can be used to override
> > +  the DTB with a different DTB.
> 
> is that really supposed to include the vendor subdir on arm64?
> 
> If so, adding fdtfile like that would break booting debian using its
> flash-image. It installs dtbs without that subdir into /boot:
> /boot/dtbs/4.19.0-9-arm64/armada-3720-espressobin.dtb
> 
> But it already supports $fdtfile:
> https://salsa.debian.org/installer-team/flash-kernel/-/blob/master/bootscript/arm64/bootscr.uboot-generic#L32
> 
> If I read that script correctly, it would fail to boot with
> fdtfile=marvell/armada-3720-espressobin.dtb...

Ugh.  I thought vendors were not supposed to flatten the dtbs as
installed.  Did that change?
Vagrant Cascadian Sept. 3, 2020, 6:59 p.m. UTC | #3
On 2020-09-03, Andre Heider wrote:
> On 03/09/2020 18:40, Dennis Gilmore wrote:
>> When testing builds provided in https://github.com/openwrt/openwrt/pull/3360
>> I discovered that fdtfile was not set and as a result the firmware was not
>> functional. So I am documenting what is needed.
>> 
>> Signed-off-by: Dennis Gilmore <dennis@ausil.us>
>> 
>> Cc: Atish Patra <atish.patra@wdc.com>
>> Cc: Lukas Auer <lukas.auer@aisec.fraunhofer.de>
>> Cc: Tom Rini <trini@konsulko.com>
>> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
>> Cc: Vagrant Cascadian <vagrant@debian.org>
>> Cc: Stephen Warren <swarren@nvidia.com>
>> Cc: Karsten Merker <merker@debian.org>
>> ---
>>   doc/README.distro | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>> 
>> diff --git a/doc/README.distro b/doc/README.distro
>> index 5076bebd18..3eb70aeb14 100644
>> --- a/doc/README.distro
>> +++ b/doc/README.distro
>> @@ -224,6 +224,14 @@ fdt_addr_r:
>>   
>>     A size of 1MB for the FDT/DTB seems reasonable.
>>   
>> +fdtfile:
>> +
>> +  Mandatory. the name of the DTB file for the specific board for instance
>> +  the espressobin v5 board the value is "marvell/armada-3720-espressobin.dtb"
>> +  while on a clearfog pro it is "armada-388-clearfog-pro.dtb" in the case of
>> +  a board providing its firmware based DTB this value can be used to override
>> +  the DTB with a different DTB.

Thanks for documenting this!

With my Debian hat on...

Reviewed-by: Vagrant Cascadian <vagrant@debian.org>


> is that really supposed to include the vendor subdir on arm64?

Yes.


> If so, adding fdtfile like that would break booting debian using its 
> flash-image. It installs dtbs without that subdir into /boot:
> /boot/dtbs/4.19.0-9-arm64/armada-3720-espressobin.dtb
>
> But it already supports $fdtfile:
> https://salsa.debian.org/installer-team/flash-kernel/-/blob/master/bootscript/arm64/bootscr.uboot-generic#L32
>
> If I read that script correctly, it would fail to boot with 
> fdtfile=marvell/armada-3720-espressobin.dtb...

Support for installing in vendor subdirs was added to flash-kernel 3.91
from 2018, and I believe the vendor subdirs are included in the
debian-installer boot media as well.


live well,
  vagrant
Stephen Warren Sept. 3, 2020, 7:15 p.m. UTC | #4
On 9/3/20 10:40 AM, Dennis Gilmore wrote:
> When testing builds provided in https://github.com/openwrt/openwrt/pull/3360
> I discovered that fdtfile was not set and as a result the firmware was not
> functional. So I am documenting what is needed.
> 
> Signed-off-by: Dennis Gilmore <dennis@ausil.us>
> 
> Cc: Atish Patra <atish.patra@wdc.com>
> Cc: Lukas Auer <lukas.auer@aisec.fraunhofer.de>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> Cc: Vagrant Cascadian <vagrant@debian.org>
> Cc: Stephen Warren <swarren@nvidia.com>
> Cc: Karsten Merker <merker@debian.org>
> ---
>  doc/README.distro | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/doc/README.distro b/doc/README.distro
> index 5076bebd18..3eb70aeb14 100644
> --- a/doc/README.distro
> +++ b/doc/README.distro
> @@ -224,6 +224,14 @@ fdt_addr_r:
>  
>    A size of 1MB for the FDT/DTB seems reasonable.
>  
> +fdtfile:
> +
> +  Mandatory. the name of the DTB file for the specific board for instance
> +  the espressobin v5 board the value is "marvell/armada-3720-espressobin.dtb"
> +  while on a clearfog pro it is "armada-388-clearfog-pro.dtb" in the case of
> +  a board providing its firmware based DTB this value can be used to override
> +  the DTB with a different DTB.

IIRC this variable isn't mandatory; if the DT filename follows expected
${soc}-${board}.dtb naming, then U-Boot has a default value that will
work without the user or U-Boot author having to manually set this variable.

So it's certainly mandatory that U-Boot know this value at runtime, but
perhaps the text should be expanded to indicate that sometimes U-Boot
can provide the value itself, but sometimes the variable needs to be set?
Andre Heider Sept. 3, 2020, 7:53 p.m. UTC | #5
Hi Vagrant,

On 03/09/2020 20:59, Vagrant Cascadian wrote:
> On 2020-09-03, Andre Heider wrote:
>> On 03/09/2020 18:40, Dennis Gilmore wrote:
>>> When testing builds provided in https://github.com/openwrt/openwrt/pull/3360
>>> I discovered that fdtfile was not set and as a result the firmware was not
>>> functional. So I am documenting what is needed.
>>>
>>> Signed-off-by: Dennis Gilmore <dennis@ausil.us>
>>>
>>> Cc: Atish Patra <atish.patra@wdc.com>
>>> Cc: Lukas Auer <lukas.auer@aisec.fraunhofer.de>
>>> Cc: Tom Rini <trini@konsulko.com>
>>> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
>>> Cc: Vagrant Cascadian <vagrant@debian.org>
>>> Cc: Stephen Warren <swarren@nvidia.com>
>>> Cc: Karsten Merker <merker@debian.org>
>>> ---
>>>    doc/README.distro | 8 ++++++++
>>>    1 file changed, 8 insertions(+)
>>>
>>> diff --git a/doc/README.distro b/doc/README.distro
>>> index 5076bebd18..3eb70aeb14 100644
>>> --- a/doc/README.distro
>>> +++ b/doc/README.distro
>>> @@ -224,6 +224,14 @@ fdt_addr_r:
>>>    
>>>      A size of 1MB for the FDT/DTB seems reasonable.
>>>    
>>> +fdtfile:
>>> +
>>> +  Mandatory. the name of the DTB file for the specific board for instance
>>> +  the espressobin v5 board the value is "marvell/armada-3720-espressobin.dtb"
>>> +  while on a clearfog pro it is "armada-388-clearfog-pro.dtb" in the case of
>>> +  a board providing its firmware based DTB this value can be used to override
>>> +  the DTB with a different DTB.
> 
> Thanks for documenting this!
> 
> With my Debian hat on...
> 
> Reviewed-by: Vagrant Cascadian <vagrant@debian.org>
> 
> 
>> is that really supposed to include the vendor subdir on arm64?
> 
> Yes.
> 
> 
>> If so, adding fdtfile like that would break booting debian using its
>> flash-image. It installs dtbs without that subdir into /boot:
>> /boot/dtbs/4.19.0-9-arm64/armada-3720-espressobin.dtb
>>
>> But it already supports $fdtfile:
>> https://salsa.debian.org/installer-team/flash-kernel/-/blob/master/bootscript/arm64/bootscr.uboot-generic#L32
>>
>> If I read that script correctly, it would fail to boot with
>> fdtfile=marvell/armada-3720-espressobin.dtb...
> 
> Support for installing in vendor subdirs was added to flash-kernel 3.91
> from 2018, and I believe the vendor subdirs are included in the
> debian-installer boot media as well.

alright, but if the vendor path is missing from flash-kernel's db, 
everything silently works with a flattened tree. See attached patch, now 
it works here too :)

Before:
/boot/dtbs/4.19.0-10-arm64/armada-3720-espressobin.dtb
After:
/boot/dtbs/4.19.0-10-arm64/marvell/armada-3720-espressobin.dtb

Thanks,
Andre
Dennis Gilmore Sept. 3, 2020, 8:14 p.m. UTC | #6
On Thu, Sep 3, 2020 at 2:15 PM Stephen Warren <swarren@wwwdotorg.org> wrote:
>
> On 9/3/20 10:40 AM, Dennis Gilmore wrote:
> > When testing builds provided in https://github.com/openwrt/openwrt/pull/3360
> > I discovered that fdtfile was not set and as a result the firmware was not
> > functional. So I am documenting what is needed.
> >
> > Signed-off-by: Dennis Gilmore <dennis@ausil.us>
> >
> > Cc: Atish Patra <atish.patra@wdc.com>
> > Cc: Lukas Auer <lukas.auer@aisec.fraunhofer.de>
> > Cc: Tom Rini <trini@konsulko.com>
> > Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> > Cc: Vagrant Cascadian <vagrant@debian.org>
> > Cc: Stephen Warren <swarren@nvidia.com>
> > Cc: Karsten Merker <merker@debian.org>
> > ---
> >  doc/README.distro | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/doc/README.distro b/doc/README.distro
> > index 5076bebd18..3eb70aeb14 100644
> > --- a/doc/README.distro
> > +++ b/doc/README.distro
> > @@ -224,6 +224,14 @@ fdt_addr_r:
> >
> >    A size of 1MB for the FDT/DTB seems reasonable.
> >
> > +fdtfile:
> > +
> > +  Mandatory. the name of the DTB file for the specific board for instance
> > +  the espressobin v5 board the value is "marvell/armada-3720-espressobin.dtb"
> > +  while on a clearfog pro it is "armada-388-clearfog-pro.dtb" in the case of
> > +  a board providing its firmware based DTB this value can be used to override
> > +  the DTB with a different DTB.
>
> IIRC this variable isn't mandatory; if the DT filename follows expected
> ${soc}-${board}.dtb naming, then U-Boot has a default value that will
> work without the user or U-Boot author having to manually set this variable.
>
> So it's certainly mandatory that U-Boot know this value at runtime, but
> perhaps the text should be expanded to indicate that sometimes U-Boot
> can provide the value itself, but sometimes the variable needs to be set?

in include/config_distro_bootcmd.h we have the following
/*
 * On 32bit ARM systems there is a reasonable number of systems that follow
 * the $soc-$board$boardver.dtb name scheme for their device trees. Use that
 * scheme if we don't have an explicit fdtfile variable.
 */
#define BOOTENV_EFI_SET_FDTFILE_FALLBACK                                  \
        "if test -z \"${fdtfile}\" -a -n \"${soc}\"; then "               \
          "setenv efi_fdtfile ${soc}-${board}${boardver}.dtb; "           \
        "fi; "
#else
#define BOOTENV_EFI_SET_FDTFILE_FALLBACK
#endif

that schema is not true on AArch64 and the method only works with efi
booting. I will update the text to list out the conditions you could
get away with not setting the variable

Dennis
Stephen Warren Sept. 3, 2020, 8:24 p.m. UTC | #7
On 9/3/20 2:14 PM, Dennis Gilmore wrote:
> On Thu, Sep 3, 2020 at 2:15 PM Stephen Warren <swarren@wwwdotorg.org> wrote:
>>
>> On 9/3/20 10:40 AM, Dennis Gilmore wrote:
>>> When testing builds provided in https://github.com/openwrt/openwrt/pull/3360
>>> I discovered that fdtfile was not set and as a result the firmware was not
>>> functional. So I am documenting what is needed.
>>>
>>> Signed-off-by: Dennis Gilmore <dennis@ausil.us>
>>>
>>> Cc: Atish Patra <atish.patra@wdc.com>
>>> Cc: Lukas Auer <lukas.auer@aisec.fraunhofer.de>
>>> Cc: Tom Rini <trini@konsulko.com>
>>> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
>>> Cc: Vagrant Cascadian <vagrant@debian.org>
>>> Cc: Stephen Warren <swarren@nvidia.com>
>>> Cc: Karsten Merker <merker@debian.org>
>>> ---
>>>  doc/README.distro | 8 ++++++++
>>>  1 file changed, 8 insertions(+)
>>>
>>> diff --git a/doc/README.distro b/doc/README.distro
>>> index 5076bebd18..3eb70aeb14 100644
>>> --- a/doc/README.distro
>>> +++ b/doc/README.distro
>>> @@ -224,6 +224,14 @@ fdt_addr_r:
>>>
>>>    A size of 1MB for the FDT/DTB seems reasonable.
>>>
>>> +fdtfile:
>>> +
>>> +  Mandatory. the name of the DTB file for the specific board for instance
>>> +  the espressobin v5 board the value is "marvell/armada-3720-espressobin.dtb"
>>> +  while on a clearfog pro it is "armada-388-clearfog-pro.dtb" in the case of
>>> +  a board providing its firmware based DTB this value can be used to override
>>> +  the DTB with a different DTB.
>>
>> IIRC this variable isn't mandatory; if the DT filename follows expected
>> ${soc}-${board}.dtb naming, then U-Boot has a default value that will
>> work without the user or U-Boot author having to manually set this variable.
>>
>> So it's certainly mandatory that U-Boot know this value at runtime, but
>> perhaps the text should be expanded to indicate that sometimes U-Boot
>> can provide the value itself, but sometimes the variable needs to be set?
> 
> in include/config_distro_bootcmd.h we have the following
> /*
>  * On 32bit ARM systems there is a reasonable number of systems that follow
>  * the $soc-$board$boardver.dtb name scheme for their device trees. Use that
>  * scheme if we don't have an explicit fdtfile variable.
>  */
> #define BOOTENV_EFI_SET_FDTFILE_FALLBACK                                  \
>         "if test -z \"${fdtfile}\" -a -n \"${soc}\"; then "               \
>           "setenv efi_fdtfile ${soc}-${board}${boardver}.dtb; "           \
>         "fi; "
> #else
> #define BOOTENV_EFI_SET_FDTFILE_FALLBACK
> #endif
> 
> that schema is not true on AArch64 and the method only works with efi
> booting. I will update the text to list out the conditions you could
> get away with not setting the variable

I was thinking more of cmd/pxe_utils.c:label_boot(), which covers the
extlinux.conf rather then EFI case. This certainly applies to both 32-
and 64-bit Tegra systems at least.

Specifically:
>                        f1 = env_get("fdtfile");
>                         if (f1) {
>                                 f2 = "";
>                                 f3 = "";
>                                 f4 = "";
>                         } else {
>                                 /*
>                                  * For complex cases where this code doesn't
>                                  * generate the correct filename, the board
>                                  * code should set $fdtfile during early boot,
>                                  * or the boot scripts should set $fdtfile
>                                  * before invoking "pxe" or "sysboot".
>                                  */
>                                 f1 = env_get("soc");
>                                 f2 = "-";
>                                 f3 = env_get("board");
>                                 f4 = ".dtb";
>                         }
Dennis Gilmore Sept. 3, 2020, 9:18 p.m. UTC | #8
On Thu, Sep 3, 2020 at 3:24 PM Stephen Warren <swarren@wwwdotorg.org> wrote:
>
> On 9/3/20 2:14 PM, Dennis Gilmore wrote:
> > On Thu, Sep 3, 2020 at 2:15 PM Stephen Warren <swarren@wwwdotorg.org> wrote:
> >>
> >> On 9/3/20 10:40 AM, Dennis Gilmore wrote:
> >>> When testing builds provided in https://github.com/openwrt/openwrt/pull/3360
> >>> I discovered that fdtfile was not set and as a result the firmware was not
> >>> functional. So I am documenting what is needed.
> >>>
> >>> Signed-off-by: Dennis Gilmore <dennis@ausil.us>
> >>>
> >>> Cc: Atish Patra <atish.patra@wdc.com>
> >>> Cc: Lukas Auer <lukas.auer@aisec.fraunhofer.de>
> >>> Cc: Tom Rini <trini@konsulko.com>
> >>> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> >>> Cc: Vagrant Cascadian <vagrant@debian.org>
> >>> Cc: Stephen Warren <swarren@nvidia.com>
> >>> Cc: Karsten Merker <merker@debian.org>
> >>> ---
> >>>  doc/README.distro | 8 ++++++++
> >>>  1 file changed, 8 insertions(+)
> >>>
> >>> diff --git a/doc/README.distro b/doc/README.distro
> >>> index 5076bebd18..3eb70aeb14 100644
> >>> --- a/doc/README.distro
> >>> +++ b/doc/README.distro
> >>> @@ -224,6 +224,14 @@ fdt_addr_r:
> >>>
> >>>    A size of 1MB for the FDT/DTB seems reasonable.
> >>>
> >>> +fdtfile:
> >>> +
> >>> +  Mandatory. the name of the DTB file for the specific board for instance
> >>> +  the espressobin v5 board the value is "marvell/armada-3720-espressobin.dtb"
> >>> +  while on a clearfog pro it is "armada-388-clearfog-pro.dtb" in the case of
> >>> +  a board providing its firmware based DTB this value can be used to override
> >>> +  the DTB with a different DTB.
> >>
> >> IIRC this variable isn't mandatory; if the DT filename follows expected
> >> ${soc}-${board}.dtb naming, then U-Boot has a default value that will
> >> work without the user or U-Boot author having to manually set this variable.
> >>
> >> So it's certainly mandatory that U-Boot know this value at runtime, but
> >> perhaps the text should be expanded to indicate that sometimes U-Boot
> >> can provide the value itself, but sometimes the variable needs to be set?
> >
> > in include/config_distro_bootcmd.h we have the following
> > /*
> >  * On 32bit ARM systems there is a reasonable number of systems that follow
> >  * the $soc-$board$boardver.dtb name scheme for their device trees. Use that
> >  * scheme if we don't have an explicit fdtfile variable.
> >  */
> > #define BOOTENV_EFI_SET_FDTFILE_FALLBACK                                  \
> >         "if test -z \"${fdtfile}\" -a -n \"${soc}\"; then "               \
> >           "setenv efi_fdtfile ${soc}-${board}${boardver}.dtb; "           \
> >         "fi; "
> > #else
> > #define BOOTENV_EFI_SET_FDTFILE_FALLBACK
> > #endif
> >
> > that schema is not true on AArch64 and the method only works with efi
> > booting. I will update the text to list out the conditions you could
> > get away with not setting the variable
>
> I was thinking more of cmd/pxe_utils.c:label_boot(), which covers the
> extlinux.conf rather then EFI case. This certainly applies to both 32-
> and 64-bit Tegra systems at least.
>
> Specifically:
> >                        f1 = env_get("fdtfile");
> >                         if (f1) {
> >                                 f2 = "";
> >                                 f3 = "";
> >                                 f4 = "";
> >                         } else {
> >                                 /*
> >                                  * For complex cases where this code doesn't
> >                                  * generate the correct filename, the board
> >                                  * code should set $fdtfile during early boot,
> >                                  * or the boot scripts should set $fdtfile
> >                                  * before invoking "pxe" or "sysboot".
> >                                  */
> >                                 f1 = env_get("soc");
> >                                 f2 = "-";
> >                                 f3 = env_get("board");
> >                                 f4 = ".dtb";
> >                         }

That still only covers the 32-bit case, at least unless the OS is
removing the vendor directory from the dtbs. At least for fedora on
aarch64 we leave the vendor directories in place

ls /boot/dtb/
allwinner  amlogic  arm       cavium     hisilicon  nvidia  rockchip
amd        apm      broadcom  freescale  marvell    qcom    xilinx

though fedora is only supporting efi on AArch64

Dennis
Vagrant Cascadian Sept. 4, 2020, 12:30 a.m. UTC | #9
Package: flash-kernel
Tags: patch
Control: submitter -1 a.heider@gmail.com
X-Debbugs-Cc: Andre Heider <a.heider@gmail.com>

I'm submitting this to the Debian bug tracking system, since this isn't
directly related to u-boot. I've dropped most of the CCs on the thread,
presumably should also drop the u-boot CC on follow-ups.


On 2020-09-03, Andre Heider wrote:
> On 03/09/2020 20:59, Vagrant Cascadian wrote:
>> On 2020-09-03, Andre Heider wrote:
>>> On 03/09/2020 18:40, Dennis Gilmore wrote:
>>> If so, adding fdtfile like that would break booting debian using its
>>> flash-image. It installs dtbs without that subdir into /boot:
>>> /boot/dtbs/4.19.0-9-arm64/armada-3720-espressobin.dtb
>>>
>>> But it already supports $fdtfile:
>>> https://salsa.debian.org/installer-team/flash-kernel/-/blob/master/bootscript/arm64/bootscr.uboot-generic#L32
>>>
>>> If I read that script correctly, it would fail to boot with
>>> fdtfile=marvell/armada-3720-espressobin.dtb...

It probably would fall back to the /boot/dtb-$version symlinks...


>> Support for installing in vendor subdirs was added to flash-kernel 3.91
>> from 2018, and I believe the vendor subdirs are included in the
>> debian-installer boot media as well.
>
> alright, but if the vendor path is missing from flash-kernel's db, 
> everything silently works with a flattened tree. See attached patch, now 
> it works here too :)

Yes, I believe this was intentional to smooth the transition without
breaking booting for some machines...


> Before:
> /boot/dtbs/4.19.0-10-arm64/armada-3720-espressobin.dtb
> After:
> /boot/dtbs/4.19.0-10-arm64/marvell/armada-3720-espressobin.dtb
>
> Thanks,
> Andre
> From 9565a3a066f59258886ad1216ce957d63fd390cc Mon Sep 17 00:00:00 2001
> From: Andre Heider <a.heider@gmail.com>
> Date: Thu, 3 Sep 2020 21:33:18 +0200
> Subject: [PATCH] Fix missing vendor subdirs for arm64 boards.
>
> The subdir is required for installing the fdt file to the correct path,
> which is a prerequisite for a boot loader provided $fdtfile.
> ---
>  db/all.db | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/db/all.db b/db/all.db
> index 16f9606..d7303f7 100644
> --- a/db/all.db
> +++ b/db/all.db
> @@ -487,7 +487,7 @@ Required-Packages: u-boot-tools
>  Bootloader-Sets-Incorrect-Root: yes
>  
>  Machine: Globalscale Marvell ESPRESSOBin Board
> -DTB-Id: armada-3720-espressobin.dtb
> +DTB-Id: marvell/armada-3720-espressobin.dtb
>  Boot-Script-Path: /boot/boot.scr
>  U-Boot-Script-Name: bootscr.uboot-generic
>  Required-Packages: u-boot-tools
> @@ -874,7 +874,7 @@ Required-Packages: u-boot-tools
>  
>  Machine: Marvell Armada 8040 DB board
>  Kernel-Flavors: arm64
> -DTB-Id: armada-8040-db.dtb
> +DTB-Id: marvell/armada-8040-db.dtb
>  Boot-Script-Path: /boot/boot.scr
>  U-Boot-Script-Name: bootscr.uboot-generic
>  Required-Packages: u-boot-tools
> @@ -1186,7 +1186,7 @@ Required-Packages: u-boot-tools
>  
>  Machine: Olimex A64 Teres-I
>  Kernel-Flavors: arm64
> -DTB-Id: sun50i-a64-teres-i.dtb
> +DTB-Id: allwinner/sun50i-a64-teres-i.dtb
>  Boot-Script-Path: /boot/boot.scr
>  U-Boot-Script-Name: bootscr.uboot-generic
>  Required-Packages: u-boot-tools
> @@ -1345,7 +1345,7 @@ Required-Packages: u-boot-tools
>  
>  Machine: Purism Librem 5 devkit
>  Kernel-Flavors: arm64
> -DTB-Id: imx8mq-librem5-devkit.dtb
> +DTB-Id: freescale/imx8mq-librem5-devkit.dtb
>  Boot-Script-Path: /boot/boot.scr
>  U-Boot-Script-Name: bootscr.uboot-generic
>  Required-Packages: u-boot-tools
> -- 
> 2.20.1


live well,
  vagrant
diff mbox series

Patch

diff --git a/doc/README.distro b/doc/README.distro
index 5076bebd18..3eb70aeb14 100644
--- a/doc/README.distro
+++ b/doc/README.distro
@@ -224,6 +224,14 @@  fdt_addr_r:
 
   A size of 1MB for the FDT/DTB seems reasonable.
 
+fdtfile:
+
+  Mandatory. the name of the DTB file for the specific board for instance
+  the espressobin v5 board the value is "marvell/armada-3720-espressobin.dtb"
+  while on a clearfog pro it is "armada-388-clearfog-pro.dtb" in the case of
+  a board providing its firmware based DTB this value can be used to override
+  the DTB with a different DTB.
+
 ramdisk_addr_r:
 
   Mandatory. The location in RAM where the initial ramdisk will be loaded to