* [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 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
* 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
* [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 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
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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox