Hi Ilias, On Tue, 26 Oct 2021 at 07:56, Ilias Apalodimas <ilias.apalodi...@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? > 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 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. 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 <s...@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/yxektkel73nm0...@apalos.home/ > > Regards > /Ilias