* [PATCH] ARM: Distro boot: document the need for fdtfile variable to be set
@ 2020-09-03 16:40 Dennis Gilmore
2020-09-03 17:50 ` Andre Heider
2020-09-03 19:15 ` [PATCH] ARM: Distro boot: document the need for fdtfile variable to be set Stephen Warren
0 siblings, 2 replies; 10+ messages in thread
From: Dennis Gilmore @ 2020-09-03 16:40 UTC (permalink / raw
To: u-boot
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.
+
ramdisk_addr_r:
Mandatory. The location in RAM where the initial ramdisk will be loaded to
--
2.28.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH] ARM: Distro boot: document the need for fdtfile variable to be set
2020-09-03 16:40 [PATCH] ARM: Distro boot: document the need for fdtfile variable to be set Dennis Gilmore
@ 2020-09-03 17:50 ` Andre Heider
2020-09-03 17:54 ` Tom Rini
2020-09-03 18:59 ` Vagrant Cascadian
2020-09-03 19:15 ` [PATCH] ARM: Distro boot: document the need for fdtfile variable to be set Stephen Warren
1 sibling, 2 replies; 10+ messages in thread
From: Andre Heider @ 2020-09-03 17:50 UTC (permalink / raw
To: u-boot
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] ARM: Distro boot: document the need for fdtfile variable to be set
2020-09-03 17:50 ` Andre Heider
@ 2020-09-03 17:54 ` Tom Rini
2020-09-03 18:59 ` Vagrant Cascadian
1 sibling, 0 replies; 10+ messages in thread
From: Tom Rini @ 2020-09-03 17:54 UTC (permalink / raw
To: u-boot
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?
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200903/0c9a8622/attachment.sig>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] ARM: Distro boot: document the need for fdtfile variable to be set
2020-09-03 17:50 ` Andre Heider
2020-09-03 17:54 ` Tom Rini
@ 2020-09-03 18:59 ` Vagrant Cascadian
2020-09-03 19:53 ` Andre Heider
1 sibling, 1 reply; 10+ messages in thread
From: Vagrant Cascadian @ 2020-09-03 18:59 UTC (permalink / raw
To: u-boot
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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 227 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200903/1663f873/attachment.sig>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] ARM: Distro boot: document the need for fdtfile variable to be set
2020-09-03 16:40 [PATCH] ARM: Distro boot: document the need for fdtfile variable to be set Dennis Gilmore
2020-09-03 17:50 ` Andre Heider
@ 2020-09-03 19:15 ` Stephen Warren
2020-09-03 20:14 ` Dennis Gilmore
1 sibling, 1 reply; 10+ messages in thread
From: Stephen Warren @ 2020-09-03 19:15 UTC (permalink / raw
To: u-boot
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?
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] ARM: Distro boot: document the need for fdtfile variable to be set
2020-09-03 18:59 ` Vagrant Cascadian
@ 2020-09-03 19:53 ` Andre Heider
2020-09-04 0:30 ` flash-kernel: Add vendor path for some arm64 machines Vagrant Cascadian
0 siblings, 1 reply; 10+ messages in thread
From: Andre Heider @ 2020-09-03 19:53 UTC (permalink / raw
To: u-boot
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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Fix-missing-vendor-subdirs-for-arm64-boards.patch
Type: text/x-patch
Size: 1747 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200903/65f6ee32/attachment.bin>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] ARM: Distro boot: document the need for fdtfile variable to be set
2020-09-03 19:15 ` [PATCH] ARM: Distro boot: document the need for fdtfile variable to be set Stephen Warren
@ 2020-09-03 20:14 ` Dennis Gilmore
2020-09-03 20:24 ` Stephen Warren
0 siblings, 1 reply; 10+ messages in thread
From: Dennis Gilmore @ 2020-09-03 20:14 UTC (permalink / raw
To: u-boot
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] ARM: Distro boot: document the need for fdtfile variable to be set
2020-09-03 20:14 ` Dennis Gilmore
@ 2020-09-03 20:24 ` Stephen Warren
2020-09-03 21:18 ` Dennis Gilmore
0 siblings, 1 reply; 10+ messages in thread
From: Stephen Warren @ 2020-09-03 20:24 UTC (permalink / raw
To: u-boot
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";
> }
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] ARM: Distro boot: document the need for fdtfile variable to be set
2020-09-03 20:24 ` Stephen Warren
@ 2020-09-03 21:18 ` Dennis Gilmore
0 siblings, 0 replies; 10+ messages in thread
From: Dennis Gilmore @ 2020-09-03 21:18 UTC (permalink / raw
To: u-boot
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* flash-kernel: Add vendor path for some arm64 machines
2020-09-03 19:53 ` Andre Heider
@ 2020-09-04 0:30 ` Vagrant Cascadian
0 siblings, 0 replies; 10+ messages in thread
From: Vagrant Cascadian @ 2020-09-04 0:30 UTC (permalink / raw
To: u-boot
Package: flash-kernel
Tags: patch
Control: submitter -1 a.heider at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 227 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200903/e5b3f3e2/attachment.sig>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-09-04 0:30 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-09-03 16:40 [PATCH] ARM: Distro boot: document the need for fdtfile variable to be set Dennis Gilmore
2020-09-03 17:50 ` Andre Heider
2020-09-03 17:54 ` Tom Rini
2020-09-03 18:59 ` Vagrant Cascadian
2020-09-03 19:53 ` Andre Heider
2020-09-04 0:30 ` flash-kernel: Add vendor path for some arm64 machines Vagrant Cascadian
2020-09-03 19:15 ` [PATCH] ARM: Distro boot: document the need for fdtfile variable to be set Stephen Warren
2020-09-03 20:14 ` Dennis Gilmore
2020-09-03 20:24 ` Stephen Warren
2020-09-03 21:18 ` Dennis Gilmore
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.