[U-Boot] [PATCH 1/1] odroid-c2: make fdtfile conform with README.pxe

[Andreas Färber]

Heinrich,

Am 23.04.2017 um 10:09 schrieb Heinrich Schuchardt:
> According to doc/README.pxe fdtfile shall only contain a filename
> and no path. If a path is needed it shall be specified with
> ftddir.

That is absolute BS!!! It does NOT say that at all.

http://git.denx.de/?p=u-boot.git;a=commitdiff;h=6015f8f1b6fc30de7b4839bd691058583ec7f521

fdtdir (sic!) is not a U-Boot environment variable, it is some extlinux
config file setting, with no connection to EFI boot or boot scripts.

And the PXE documentation only says that <path>/ gets prepended, i.e.
fdtdir might be "boot" or "dtb/4.10.9-1", which says nothing about
whether or not $fdtfile may contain an additional vendor dir or not!

bootefi instead uses $efi_dtb_prefixes that the user can override if
necessary, such as /, dtb/ and boot/dtb/. See below.

> 
> Putting the vendor path into ftdname breaks package flash-kernel
> in Linux distribution Debian and its derivatives.
> 
> The following other arm64 boards do not have a vendor directory
> prefixed to ftddir:

fdt* (3x) - Flat Device Tree

> 
> hikey_defconfig: fdtfile=hi6220-hikey.dtb
> dragonboard410c_defconfig: fdtfile=apq8016-sbc.dtb
> pine64_plus_defconfig: fdtfile=sun50i-a64-pine6-plus.dtb
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
> ---
>  include/configs/odroid-c2.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/configs/odroid-c2.h b/include/configs/odroid-c2.h
> index 117c0e418a..f2e1034b50 100644
> --- a/include/configs/odroid-c2.h
> +++ b/include/configs/odroid-c2.h
> @@ -13,7 +13,7 @@
>  /* Serial setup */
>  #define CONFIG_CONS_INDEX		0
>  
> -#define MESON_FDTFILE_SETTING "fdtfile=amlogic/meson-gxbb-odroidc2.dtb\0"
> +#define MESON_FDTFILE_SETTING "fdtfile=meson-gxbb-odroidc2.dtb\0"

NACK. This patch is a regression of the EFI boot path. You're not even
setting your own mistaken fdtdir.

And once again in your commit message you are conveniently ignoring that
this is what fdtfile is like on the Raspberry Pi 3:

U-Boot> printenv fdtdir
## Error: "fdtdir" not defined
U-Boot> printenv fdtfile
fdtfile=broadcom/bcm2837-rpi-3-b.dtb

The Pine64 (and Orange Pi PC 2) don't set fdtdir either:

=> printenv fdtdir
## Error: "fdtdir" not defined

At least in v2016.09.1 the HiKey didn't, and in v2017.01 the
Dragonboard410c didn't either.

Do you have a single example of any arm64 board doing so???

Some documentation, taken out of context, is no excuse for deliberately
regressing the boot for others. Either post a patch that works for
everyone, or leave it. When I added fdtfile I did not regress anyone.

I'm concluding you still haven't found my old patch series - it was
called "[U-Boot] [PATCH 0/6] efi_loader: Improvements to .dtb handling".
In that discussion Tom said that we should not probe both flat dir and
vendor subdir in $efi_dtb_prefixes as I was implementing, and should
rather hardcode the vendor dir and use only that for those architectures
where upstream Linux has them (arm64, mips, ...).

Tom proposed a CONFIG_FDTFILE_VENDOR_DIRECTORY, which I wasn't sure
where to set - Kconfig, defconfig, header, ... (and had not been working
on U-Boot for some time)

Later for Raspberry Pi the vendor dir was integrated with fdtfile
instead of going via some config option, because IIRC it was supposed to
work with both 32-bit (flat) and 64-bit (vendor dir) kernels, and the
fdtfile is auto-detected rather than hardcoded in defconfig.
See v1 "[U-Boot] [PATCH] rpi: Fix device tree path on ARM64".

You now seem to be pushing for a flat dir again, which the Linux
maintainers expressly rejected for arm64 - therefore your distro is
wrong in installing them flat. There are no guarantees for
collision-freeness between vendor dirs. Imagine that had I called it
s905.dtsi instead of meson-gxbb.dtsi, then we will soon have an
s900.dtsi for Actions Semi, and who knows what their next models will be
called, so a generic board name like s905-evb.dtb might clash. No
contributor or maintainer will think about such conflicts because they
simply don't happen when using install_dtbs or its equivalent.

For the record I also consider it unclean that U-Boot is importing
.dts[i] files in a flat way into arch/arm/dts. So far I guess it hasn't
caused conflicts, and it's an internal implementation detail, so doesn't
affect Linux distributions.

Regards,
Andreas

>  
>  #include <configs/meson-gxbb-common.h>
>  

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)