On Thu, Sep 3, 2020 at 3:24 PM Stephen Warren <swar...@wwwdotorg.org> wrote: > > On 9/3/20 2:14 PM, Dennis Gilmore wrote: > > On Thu, Sep 3, 2020 at 2:15 PM Stephen Warren <swar...@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 <den...@ausil.us> > >>> > >>> Cc: Atish Patra <atish.pa...@wdc.com> > >>> Cc: Lukas Auer <lukas.a...@aisec.fraunhofer.de> > >>> Cc: Tom Rini <tr...@konsulko.com> > >>> Cc: Masahiro Yamada <yamada.masah...@socionext.com> > >>> Cc: Vagrant Cascadian <vagr...@debian.org> > >>> Cc: Stephen Warren <swar...@nvidia.com> > >>> Cc: Karsten Merker <mer...@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