Hi Simon,
On Tue, 26 Oct 2021 at 18:27, Simon Glass sjg@chromium.org wrote:
Hi Ilias,
On Tue, 26 Oct 2021 at 07:56, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
Hi Simon,
As I said here [1], this is moving on an entirely different direction I had in mind. I'd much prefer starting the discussions for a solution that allows us to scale.
I am missing the point here. Is there something in the plans that I don't know about?
I have some ideas, but haven't found time to code it and send patches yet.
FWIW I think the current code is still not clean for my taste. Commit 3b595da441cf ("fdtdec: allow board to provide fdt for CONFIG_OF_SEPARATE") allowed this function to be used regardless of the config options. IMHO we should have 2 clear options:
- U-Boot provides the DTB
Supported with: OF_SEPARATE
- It's somehow passed over to U-Boot
Supported with: OF_SEPARATE + OF_BOARD
That's exactly what I don't personally like. In your example OF_BOARD means "U-Boot has a DTB and here's a way to override it". In my head that options means "U-Boot was handed over a DTB on a register and it must be able to deal with it". The latter is obviously less restrictive to previous bootloaders.
I actually hit this problem with my qemu testing, in that it is actually not possible to use U-Boot's devicetree without hacking the code. We need to be able to select this feature explicitly, or turn it off, at least for development.
Alternatively we can just keep U-Boot config node in a .dtbo and apply it on the fly after QEMU has handed us over the DTB it created.
Regards /Ilias
Regards, Simon
On Mon, Oct 25, 2021 at 06:23:44PM -0600, Simon Glass wrote:
At present this override function is called even when OF_BOARD Is not enabled. This makes it impossible to disable this feature and in fact makes the OF_BOARD option useless.
Reinstate its intended purpose, so that it is possible to switch between the appended devicetree and one provided by the board's custom function.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v5:
- Add new patches to clean up fdtdec_setup() and surrounds
include/fdtdec.h | 7 +++++-- lib/fdtdec.c | 17 +++++++++++------ 2 files changed, 16 insertions(+), 8 deletions(-)
diff --git a/include/fdtdec.h b/include/fdtdec.h index 386f6611294..b2faa84008e 100644 --- a/include/fdtdec.h +++ b/include/fdtdec.h @@ -1170,8 +1170,11 @@ int fdtdec_resetup(int *rescan);
/**
- Board-specific FDT initialization. Returns the address to a device tree blob.
- Called when CONFIG_OF_BOARD is defined, or if CONFIG_OF_SEPARATE is defined
- and the board implements it.
- Called when CONFIG_OF_BOARD is defined.
- The existing devicetree is available at gd->fdt_blob
*/
- @returns new devicetree blob pointer
void *board_fdt_blob_setup(void);
diff --git a/lib/fdtdec.c b/lib/fdtdec.c index 067c27d0aa3..da36dffec62 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -1203,11 +1203,12 @@ static int uncompress_blob(const void *src, ulong sz_src, void **dstp) return 0; }
-/*
- For CONFIG_OF_SEPARATE, the board may optionally implement this to
- provide and/or fixup the fdt.
+/**
- fdt_find_separate() - Find a devicetree at the end of the image
*/
- @return pointer to FDT blob
-__weak void *board_fdt_blob_setup(void) +static void *fdt_find_separate(void) { void *fdt_blob = NULL; #ifdef CONFIG_SPL_BUILD @@ -1623,11 +1624,15 @@ int fdtdec_setup(void) int ret;
/* The devicetree is typically appended to U-Boot */
if (IS_ENABLED(CONFIG_OF_SEPARATE) || IS_ENABLED(CONFIG_OF_BOARD))gd->fdt_blob = board_fdt_blob_setup();
if (IS_ENABLED(CONFIG_OF_SEPARATE))gd->fdt_blob = fdt_find_separate(); else /* embed dtb in ELF file for testing / development */ gd->fdt_blob = dtb_dt_embedded();/* Allow the board to override the fdt address. */if (IS_ENABLED(CONFIG_OF_BOARD))gd->fdt_blob = board_fdt_blob_setup();if (!IS_ENABLED(CONFIG_SPL_BUILD)) { /* Allow the early environment to override the fdt address */ gd->fdt_blob = map_sysmem(env_get_ulong("fdtcontroladdr", 16,-- 2.33.0.1079.g6e70778dc9-goog
[1] https://lore.kernel.org/u-boot/YXekTkeL73NM0UOU@apalos.home/
Regards /Ilias