From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5368CC433F5 for ; Wed, 27 Oct 2021 07:17:53 +0000 (UTC) Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 6167260F9B for ; Wed, 27 Oct 2021 07:17:51 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 6167260F9B Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linaro.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 93CCF832AD; Wed, 27 Oct 2021 09:17:49 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=linaro.org header.i=@linaro.org header.b="zyVd7Rte"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 0CA47832AD; Wed, 27 Oct 2021 09:17:48 +0200 (CEST) Received: from mail-yb1-xb2d.google.com (mail-yb1-xb2d.google.com [IPv6:2607:f8b0:4864:20::b2d]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 81D7782952 for ; Wed, 27 Oct 2021 09:17:43 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=ilias.apalodimas@linaro.org Received: by mail-yb1-xb2d.google.com with SMTP id a6so3915319ybq.9 for ; Wed, 27 Oct 2021 00:17:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=KkRKI2ifNdsoG45M6qb/3/uOE6d81YwgdL3BiEIExnI=; b=zyVd7Rte5zeNnT+F2qGmT21Z23VtLOH8QR57UTflhLSRGQ3JFSvWhPeDRjqwQT6SuO zfsSPDPwQ5t2HCP/hntn4QXftKw2N9rVUvH9IvjE8hxHDjmkpXhbbgf+USHH64ohzKTF 4Qe5N+n5gBBuAMuYZsg0wWSenEIVLyvcZQk8b3zNFyvh9yVGNJnmcCx1W/2pNaOihqyZ mqQzZbGNvUSHnSyElytQBz9X/TyvXTKGXfhol4OkgFicbbcBb4N6JtQz402FvFJWd765 0sWqzVctWhg+Izhx/G/o5uefJdft+5HwQQLgX4HCpJD881BVpwR9CV/U8FrZEUt82Qvv Aicw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=KkRKI2ifNdsoG45M6qb/3/uOE6d81YwgdL3BiEIExnI=; b=wIaOmKWZuzEPwSd1A3BuaDn0YJp/QBUAgIPZGcDo/BGasY0J1XaAZDiOoy5Q0mlR/h i24yj1kflAifBdhjszjiEzpybV1DxJZ+dNF7OzVVMy3jdYEcsr0FSN728Jys6NXtfq6m Tp3RMRqcCLogFDaWiQ43fcqFNPCIOj3/cQwGEhlZ43fuukhUKZlPwLei2HaQfwepY60Z qyE3G3KYhWtrJa3nQuxXcFeF0+n9W5KOx+9osXQ0GEtNQrEu2Jk/BmEDX87DoufykK+z MiQGfhwf2dKne12nse+ay9KrwrKdnFsn4HkD9xeUS/Z0abqkShurW9NjwOLRsXBgrxng YoQw== X-Gm-Message-State: AOAM530mzPI/65XlKLALzYD8Gf+kRFiyuwMGyVQdCc/Gb6fJSJw2X7Gd dsW0JHoN0VLiM6quJAgJisaz4e7ZQU7HO6QiFD2wbA== X-Google-Smtp-Source: ABdhPJymz+oKUnGhSUY2rSEXCY4X74J2uRa9MNdQ4EkBXQpDUOAodypB4k3PEPySPboaQOjLwPQ57ZTP8CIaeqhzwuY= X-Received: by 2002:a25:d4cc:: with SMTP id m195mr2349027ybf.401.1635319062005; Wed, 27 Oct 2021 00:17:42 -0700 (PDT) MIME-Version: 1.0 References: <20211026002344.405160-1-sjg@chromium.org> <20211026002344.405160-27-sjg@chromium.org> In-Reply-To: From: Ilias Apalodimas Date: Wed, 27 Oct 2021 10:17:05 +0300 Message-ID: Subject: Re: [PATCH v5 26/26] fdt: Don't call board_fdt_blob_setup() without OF_BOARD To: Simon Glass Cc: U-Boot Mailing List , Mark Kettenis , Heinrich Schuchardt , Tom Rini , Sean Anderson , Jerry Van Baren Content-Type: text/plain; charset="UTF-8" X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.34 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.2 at phobos.denx.de X-Virus-Status: Clean Hi Simon, On Tue, 26 Oct 2021 at 18:27, Simon Glass wrote: > > Hi Ilias, > > On Tue, 26 Oct 2021 at 07:56, Ilias Apalodimas > 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 > > > --- > > > > > > 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