[U-Boot] [PATCH] rpi: Fix device tree path on ARM64

The directory structure of device tree files produced by the kernel's 'make dtbs_install' is different on ARM64, the RPi3 device tree file is in a 'broadcom' subdirectory there. Make the set_fdtfile function account for this so that the distro boot scripts can locate the DTB file.
Signed-off-by: Tuomas Tynkkynen tuomas@tuxera.com --- board/raspberrypi/rpi/rpi.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c index 22e87a2..dbf69f8 100644 --- a/board/raspberrypi/rpi/rpi.c +++ b/board/raspberrypi/rpi/rpi.c @@ -243,12 +243,14 @@ int dram_init(void)
static void set_fdtfile(void) { - const char *fdtfile; + char fdtfile[64] = "";
if (getenv("fdtfile")) return;
- fdtfile = model->fdtfile; + if (IS_ENABLED(CONFIG_ARM64)) + strcat(fdtfile, "broadcom/"); + strcat(fdtfile, model->fdtfile); setenv("fdtfile", fdtfile); }

On 01/03/2017 03:39 AM, Tuomas Tynkkynen wrote:
The directory structure of device tree files produced by the kernel's 'make dtbs_install' is different on ARM64, the RPi3 device tree file is in a 'broadcom' subdirectory there. Make the set_fdtfile function account for this so that the distro boot scripts can locate the DTB file.
I'm not 100% sure there's an expectation/guarantee that "make dtbs_install" will be used to create the DTB layout that the bootloader uses, although I suppose this is a reasonable thing to do.
diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c
static void set_fdtfile(void)
- fdtfile = model->fdtfile;
- if (IS_ENABLED(CONFIG_ARM64))
strcat(fdtfile, "broadcom/");
- strcat(fdtfile, model->fdtfile);
Instead of writing code for this, and in particular code that doesn't check for buffer size/overflow/..., wouldn't it be better to simply edit the RPi 3 entry in rpi_models_new_scheme[] to contain "broadcom/" in the DTB filename string?

On Thu, 5 Jan 2017 16:44:43 -0700 Stephen Warren swarren@wwwdotorg.org wrote:
On 01/03/2017 03:39 AM, Tuomas Tynkkynen wrote:
The directory structure of device tree files produced by the kernel's 'make dtbs_install' is different on ARM64, the RPi3 device tree file is in a 'broadcom' subdirectory there. Make the set_fdtfile function account for this so that the distro boot scripts can locate the DTB file.
I'm not 100% sure there's an expectation/guarantee that "make dtbs_install" will be used to create the DTB layout that the bootloader uses, although I suppose this is a reasonable thing to do.
It certainly would be nice from the distro's point of view to have the same scripts work on both ARM and ARM64.
diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c
static void set_fdtfile(void)
- fdtfile = model->fdtfile;
- if (IS_ENABLED(CONFIG_ARM64))
strcat(fdtfile, "broadcom/");
- strcat(fdtfile, model->fdtfile);
Instead of writing code for this, and in particular code that doesn't check for buffer size/overflow/..., wouldn't it be better to simply edit the RPi 3 entry in rpi_models_new_scheme[] to contain "broadcom/" in the DTB filename string?
I did it this way to keep the 32-bit builds looking in the old location, as that's where it was planned to be added for 32-bit ARM:
https://patchwork.kernel.org/patch/9148261/
I did consider some preprocessor magic and compile-time string concatenation but this seemed a cleaner way.

On 01/09/2017 06:34 AM, Tuomas Tynkkynen wrote:
On Thu, 5 Jan 2017 16:44:43 -0700 Stephen Warren swarren@wwwdotorg.org wrote:
On 01/03/2017 03:39 AM, Tuomas Tynkkynen wrote:
The directory structure of device tree files produced by the kernel's 'make dtbs_install' is different on ARM64, the RPi3 device tree file is in a 'broadcom' subdirectory there. Make the set_fdtfile function account for this so that the distro boot scripts can locate the DTB file.
I'm not 100% sure there's an expectation/guarantee that "make dtbs_install" will be used to create the DTB layout that the bootloader uses, although I suppose this is a reasonable thing to do.
It certainly would be nice from the distro's point of view to have the same scripts work on both ARM and ARM64.
diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c
static void set_fdtfile(void)
- fdtfile = model->fdtfile;
- if (IS_ENABLED(CONFIG_ARM64))
strcat(fdtfile, "broadcom/");
- strcat(fdtfile, model->fdtfile);
Instead of writing code for this, and in particular code that doesn't check for buffer size/overflow/..., wouldn't it be better to simply edit the RPi 3 entry in rpi_models_new_scheme[] to contain "broadcom/" in the DTB filename string?
I did it this way to keep the 32-bit builds looking in the old location, as that's where it was planned to be added for 32-bit ARM:
https://patchwork.kernel.org/patch/9148261/
I did consider some preprocessor magic and compile-time string concatenation but this seemed a cleaner way.
I'm not convinced it's cleaner. It feels best to represent data as data, rather than writing code to calculate it at run-time, especially once all the complexity of error-checking is added in. I'd suggest:
Adding the following before the models table:
#ifdef CONFIG_ARM64 #define DTB_DIR "broadcom/" #else #define DTB_DIR "" #endif
Then changing the table entries from:
[0x8] = { "3 Model B", "bcm2837-rpi-3-b.dtb",
... to:
[0x8] = { "3 Model B", DTB_DIR "bcm2837-rpi-3-b.dtb",
participants (2)
-
Stephen Warren
-
Tuomas Tynkkynen