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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id B55FEC5479D for ; Mon, 9 Jan 2023 15:34:22 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 6210185482; Mon, 9 Jan 2023 16:34:20 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=fail (p=none dis=none) header.from=chromium.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=chromium.org header.i=@chromium.org header.b="Iyd9iWfB"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id E77C585023; Mon, 9 Jan 2023 16:34:18 +0100 (CET) Received: from mail-ej1-x62f.google.com (mail-ej1-x62f.google.com [IPv6:2a00:1450:4864:20::62f]) (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 32E4985023 for ; Mon, 9 Jan 2023 16:34:14 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=chromium.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=sjg@google.com Received: by mail-ej1-x62f.google.com with SMTP id u9so21137895ejo.0 for ; Mon, 09 Jan 2023 07:34:14 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=z9VGIwBRcUKhKlti/qO1TFptkJP9SwZnyEwKwWw+VA8=; b=Iyd9iWfBXIQocmFxCl6+TRu8QlSF3kLaVlTzG1rgw8Kuo3t44jyCOW5N0qopK64z4T xnnNvyhEb02rZ34N6+KwXQsheDBU/wTGc15qRduLjJ3hiIyGvcCrm0g3wpLlXIING+MI D9V/gDVkU8VpD6zA2F8SN1DJgsYU48WXuPhP0= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=z9VGIwBRcUKhKlti/qO1TFptkJP9SwZnyEwKwWw+VA8=; b=QVcbuH+z0sLC+jfPrcrH3Sa6ndiGTdVJARvu94/LbdABXqXhNbkDWqiBoObv8rzEmX D5/QIbTBy4QM/mF9eYUFHUw0MT/Vcw2EMCNRBFnxdy5Fw52iarEugm9q390dlY9HfVsS QsfBWzMlptFGXu2lXfUyeYA+pcn6lnW4PDOxxc+9S9MgFqV7HKR1Fwy0DTlijxZyxA+S 1zKedDCFp+rYHjqsERdTgWn/zCFJenKWHms8ePiq67247fh9O8oJEPe4gSfDnKYoXQVu dC7QJ9+jF6Lz/vf2z7VWBdGh/zRik9o/LMaQQjNpbbBpNuskMU/O9jyvKQfUDhY7p8T8 Ngzw== X-Gm-Message-State: AFqh2kpvFR+b1GfT9w2w59MhlAmfKsCn0GQUdnqOon7e/CmoHxgGGuXe bm99GmovKurWECuRlbWaLRBeS7JUEX4wzBvdmSNFFQ== X-Google-Smtp-Source: AMrXdXsObkaO+R7iXqmHPG0PNP/tYUse3akqCitOisvrYoQwq+/reYrtotMW95Uf7EW/3wzI9vgPKUCQDWgEs/emQrI= X-Received: by 2002:a17:906:c291:b0:84d:cc3:fd78 with SMTP id r17-20020a170906c29100b0084d0cc3fd78mr2107235ejz.745.1673278453411; Mon, 09 Jan 2023 07:34:13 -0800 (PST) MIME-Version: 1.0 References: <20230107210721.478062-1-sjg@chromium.org> <20230107210721.478062-15-sjg@chromium.org> <20230108134132.GY3787616@bill-the-cat> <20230108160605.GB3787616@bill-the-cat> <20230108162438.GC3787616@bill-the-cat> <128e89b7-db5b-a842-6990-19c52de9da22@amd.com> In-Reply-To: <128e89b7-db5b-a842-6990-19c52de9da22@amd.com> From: Simon Glass Date: Mon, 9 Jan 2023 08:34:02 -0700 Message-ID: Subject: Re: [PATCH v9 14/14] treewide: Disable USE_SPL_FIT_GENERATOR by default To: Michal Simek Cc: Tom Rini , Luca Ceresoli , U-Boot Mailing List , Roger Quadros , Alper Nebi Yasak , Peter Geis , Jerome Forissier , huang lin , Jeffy Chen , Kever Yang , Philipp Tomsich , Ivan Mikhaylov , Quentin Schulz , Philippe Reynes Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.39 X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 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.6 at phobos.denx.de X-Virus-Status: Clean Hi Michal, On Mon, 9 Jan 2023 at 04:07, Michal Simek wrote: > > Hi, > > On 1/8/23 20:36, Simon Glass wrote: > > Hi Tom, > > > > On Sun, 8 Jan 2023 at 09:24, Tom Rini wrote: > >> > >> On Sun, Jan 08, 2023 at 09:20:09AM -0700, Simon Glass wrote: > >>> Hi Tom, > >>> > >>> On Sun, 8 Jan 2023 at 09:06, Tom Rini wrote: > >>>> > >>>> On Sun, Jan 08, 2023 at 08:48:37AM -0700, Simon Glass wrote: > >>>>> Hi Tom, > >>>>> > >>>>> On Sun, 8 Jan 2023 at 06:41, Tom Rini wrote: > >>>>>> > >>>>>> On Sat, Jan 07, 2023 at 02:07:21PM -0700, Simon Glass wrote: > >>>>>> > >>>>>>> This option is deprecated and only used by two boards. Enable it for just > >>>>>>> those two boards, so others don't accidentally enable it. > >>>>>>> > >>>>>>> Signed-off-by: Simon Glass > >>>>>> [snip] > >>>>>>> diff --git a/boot/Kconfig b/boot/Kconfig > >>>>>>> index 55f06761ef8..7ab0dd14211 100644 > >>>>>>> --- a/boot/Kconfig > >>>>>>> +++ b/boot/Kconfig > >>>>>>> @@ -282,12 +282,13 @@ config SPL_FIT_SOURCE > >>>>>>> config USE_SPL_FIT_GENERATOR > >>>>>>> bool "Use a script to generate the .its script" > >>>>>>> depends on SPL_FIT > >>>>>>> - default y if SPL_FIT && ARCH_ZYNQMP > >>>>>>> + help > >>>>>>> + This is deprecated. Please do not use it. Use binman instead. > >>>>>> > >>>>>> Lets remove the text around bool so it can't be enabled, and move to > >>>>>> select'ing it from the two boards that need it. Michal, Luca, what's > >>>>>> needed to move your two platforms > >>>>>> (avnet_ultrazedev_cc_v1_0_ultrazedev_som_v1_0 and xilinx_zynqmp_virt off > >>>>>> of this very legacy option, given that other xilinx platforms have > >>>>>> already migrated to binman ? > >>>>> > >>>>> That's a good idea, but these two boards do not have TARGET Kconfig > >>>>> options so it is not possible without adding some Kconfig specific to > >>>>> those boards, then defining it in the defconfig files. > >>>>> > >>>>> We already have the legacy warning. > >>>> > >>>> Yes, but I swear these are new legacy users as when we started trying to > >>>> kill off this option it was just a few i.MX platforms lagging behind. > >>>> > >>>> Maybe make ARCH_ZYNQMP select DEPRECATED, USE_SPL_FIT_GENERATOR depend > >>>> on DEPRECATED and add "DERECATED" to the end of the text line? Or maybe > >>>> Michal or Luca will speak up soon and migrate these over quickly so we > >>>> can just nuke this. > >>> > >>> Isn't that just more tortuous? I can disable SPL_LOAD_FIT on these two > >>> boards since they don't appear in CI. Then they can convert them when > >>> ready. > >>> > >>> That way we can drop the option now, if that is your goal. > >> > >> I thought xilinx_zynqmp_virt was in CI, but I see I'm mistaken. I still > >> don't want to break platforms outright, and since it's Sunday right now > >> afterall, we should let Michal and Luca a chance to catch up and chime > >> in. I hope it's either going to be a quick conversion or expose > >> something missing and needed in binman, as to why these still haven't > >> been converted. > >> > > > > OK let's hold off on this patch for now. It is just a clean-up anyway. > > First of all. ZynqMP is not wired in CI simply because we are missing some bits > and pieces in upstream qemu to run it. If we can wire it with Xilinx version we > can do it. I have asked to fix it our qemu team but they have never done it. > > In U-Boot SPL flow this script is used all the time. I use it all the time when > I build work on ZynqMP. Also buildroot is using it. > > In connection to binman. I have looked at it 2/3 times in past. The biggest > issue which I have with it is that DT node presence in DT which goes to the system. > > Here is example > > [u-boot](binman)$ make kontron_sl28_defconfig >/dev/null > [u-boot](binman)$ make -j8 >/dev/null > [u-boot](binman)$ dtc -I dtb -O dts dts/dt.dtb 2>/dev/null | grep binman > binman { > binman = "/binman"; > u_boot_rom = "/binman/u-boot-rom"; > > where binmap is the part of target DT. Thanks for the info. > > It has side effects. > 1. DT is bigger >From my experience the size increase typically is less than 2% which does not seem significant. > 2. It contains information how firmware was packed which is just additional > information which don't need to be shared. That depends on the application. For firmware update and debugging it is sometime necessary to see where everything is. While I understand the EFI model of hiding everything away I believe that a better future for the industry is to have the OS and firmware work together more. > 3. binman node as is placed in not documented in dt binding to be able to pass > dtb check. This is the root cause of all of the device tree woes. There was a small amount of slow progress at the end of last year but now it has gone quiet. I don't know what to do about this other than prod and wait, since Linux controls the bindings. Perhaps this is something that could be worked out f2f, I'm not sure. For now I will keep prodding and see what can be done. But the initial target is the DM tags, with binman bindings somewhere down the trackz although they may actually be easier. It is easy enough to use fdtgrep to strip out the binman {} node if it is not wanted. I will take a look at a feature for that, but I have a bit of a backlog,with so many things up in the air, pending patches, etc. > > I can call binman externally but that would mean additional build step which > everybody wants to avoid. > Can we update Makefile and add Kconfig option to pass configuration dtb file > instead of u-boot.dtb by default? > > Something like: > > diff --git a/Makefile b/Makefile > index 75a599b2c437..4d8d67c7d938 100644 > --- a/Makefile > +++ b/Makefile > @@ -1322,7 +1322,7 @@ cmd_binman = $(srctree)/tools/binman/binman $(if > $(BINMAN_DEBUG),-D) \ > $(foreach f,$(BINMAN_TOOLPATHS),--toolpath $(f)) \ > --toolpath $(objtree)/tools \ > $(if $(BINMAN_VERBOSE),-v$(BINMAN_VERBOSE)) \ > - build -u -d u-boot.dtb -O . -m \ > + build -u -d $(CONFIG_BINMAN_CONFIG_DTB) -O . -m \ > $(if $(BINMAN_ALLOW_MISSING),--allow-missing --ignore-missing) \ > -I . -I $(srctree) -I $(srctree)/board/$(BOARDDIR) \ > -I arch/$(ARCH)/dts -a of-list=$(CONFIG_OF_LIST) \ > > Then by default u-boot.dtb can be used or platforms can use different file. > Then I would add zynqmp-binman.dts with configurations which can replace > existing arch/arm/mach-zynqmp/mkimage_fit_atf.sh script. > What do you think? I'm sure you could do that locally, but I'd prefer to get the bindings sorted out so that binman nodes can go in there, along with a Kconfig option to enable/disable them. The more we bifurcate things, the more complicated everything is. For now, can you live with the binman nodes? What problem does it cause? > > BTW: Is there a way to generate also capsules from binman? Is that an EFI capsule? If so, I haven't seen any work on that yet. Regards, Simon