From mboxrd@z Thu Jan 1 00:00:00 1970 Delivery-date: Fri, 21 Jan 2022 09:43:52 +0100 Received: from metis.ext.pengutronix.de ([2001:67c:670:201:290:27ff:fe1d:cc33]) by lore.white.stw.pengutronix.de with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1nApWS-008bNl-DX for lore@lore.pengutronix.de; Fri, 21 Jan 2022 09:43:52 +0100 Received: from localhost ([127.0.0.1] helo=metis.ext.pengutronix.de) by metis.ext.pengutronix.de with esmtp (Exim 4.92) (envelope-from ) id 1nApWQ-0007as-UY; Fri, 21 Jan 2022 09:43:50 +0100 Received: from ptx.hi.pengutronix.de ([2001:67c:670:100:1d::c0]) by metis.ext.pengutronix.de with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1nApWH-0007aL-PX; Fri, 21 Jan 2022 09:43:41 +0100 Received: from sha by ptx.hi.pengutronix.de with local (Exim 4.92) (envelope-from ) id 1nApWH-0007o7-DH; Fri, 21 Jan 2022 09:43:41 +0100 Date: Fri, 21 Jan 2022 09:43:41 +0100 From: Sascha Hauer To: Michael Olbrich Message-ID: <20220121084341.GE22780@pengutronix.de> References: <20211220120857.3672237-1-michael.riesch@wolfvision.net> <20211220120857.3672237-5-michael.riesch@wolfvision.net> <707201be-88c0-8c7f-a2a6-518ddc5eb2f8@wolfvision.net> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-Sent-From: Pengutronix Hildesheim X-URL: http://www.pengutronix.de/ X-IRC: #ptxdist @freenode X-Accept-Language: de,en X-Accept-Content-Type: text/plain X-Uptime: 09:11:16 up 41 days, 16:56, 77 users, load average: 0.36, 0.29, 0.21 User-Agent: Mutt/1.10.1 (2018-07-13) Subject: Re: [ptxdist] [PATCH v4 4/4] barebox: add integration of firmware blobs X-BeenThere: ptxdist@pengutronix.de X-Mailman-Version: 2.1.29 Precedence: list List-Id: PTXdist Development Mailing List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: ptxdist@pengutronix.de Cc: ptxdist@pengutronix.de, Michael Riesch , m.tretter@pengutronix.de Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "ptxdist" X-SA-Exim-Connect-IP: 127.0.0.1 X-SA-Exim-Mail-From: ptxdist-bounces@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false On Fri, Jan 21, 2022 at 08:54:49AM +0100, Michael Olbrich wrote: > On Mon, Jan 10, 2022 at 07:39:20AM +0100, Michael Riesch wrote: > > On 1/7/22 3:09 PM, Michael Olbrich wrote: > > > On Mon, Dec 20, 2021 at 01:08:57PM +0100, Michael Riesch wrote: > > >> In some cases barebox requires firmware blobs, which may be > > >> provided in binary form by the vendor or compiled in a > > >> preceding step. Add the possibility to specify files (in > > >> separate rule files) which are injected in the barebox source > > >> directory during preparation. > > >> > > >> Signed-off-by: Michael Riesch > > >> --- > > >> platforms/barebox.in | 8 ++++++++ > > >> platforms/barebox.rockchip.in | 10 ++++++++++ > > >> rules/barebox.make | 4 ++++ > > >> rules/barebox.rockchip.make | 16 ++++++++++++++++ > > >> 4 files changed, 38 insertions(+) > > >> create mode 100644 platforms/barebox.rockchip.in > > >> create mode 100644 rules/barebox.rockchip.make > > >> > > >> diff --git a/platforms/barebox.in b/platforms/barebox.in > > >> index d35d16501..8a81be40f 100644 > > >> --- a/platforms/barebox.in > > >> +++ b/platforms/barebox.in > > >> @@ -55,6 +55,14 @@ config BAREBOX_CONFIG > > >> This entry specifies the .config file used to compile > > >> barebox. > > >> > > >> +menuconfig BAREBOX_FIRMWARE > > >> + bool > > >> + prompt "integrate firmware blobs " > > >> + > > >> +if BAREBOX_FIRMWARE > > >> +source "generated/barebox_firmware.in" > > >> +endif > > > > > > If you split out the generic parts and add empty files with just the > > > "## SECTION=" header (for both new sections) then we could maybe get that > > > merged while we figure out how to handle the rockchip part. > > > > OK, I'll consider this in v5. > > > > >> + > > >> config BAREBOX_EXTRA_ENV > > >> prompt "extend the builtin barebox environment" > > >> bool > > >> diff --git a/platforms/barebox.rockchip.in b/platforms/barebox.rockchip.in > > >> new file mode 100644 > > >> index 000000000..238798ce1 > > >> --- /dev/null > > >> +++ b/platforms/barebox.rockchip.in > > >> @@ -0,0 +1,10 @@ > > >> +## SECTION=barebox_firmware > > >> + > > >> +config BAREBOX_NEEDS_FIRMWARE_ROCKCHIP > > >> + prompt "barebox needs firmware-rockchip" > > >> + bool > > >> + depends on ARCH_ARM64 > > >> + select FIRMWARE_ROCKCHIP if BAREBOX_NEEDS_FIRMWARE_ROCKCHIP > > >> + help > > >> + Select this if barebox needs the non-free Rockchip firmware > > >> + blobs. > > >> diff --git a/rules/barebox.make b/rules/barebox.make > > >> index bea9f3adc..cc70a6ff5 100644 > > >> --- a/rules/barebox.make > > >> +++ b/rules/barebox.make > > >> @@ -94,6 +94,10 @@ ifdef PTXCONF_BAREBOX_EXTRA_ENV > > >> @rm -rf $(BAREBOX_BUILD_DIR)/defaultenv/barebox_default_env > > >> endif > > >> > > >> +ifdef PTXCONF_BAREBOX_FIRMWARE > > >> + @$(call world/inject, BAREBOX) > > >> +endif > > >> + > > >> @$(call touch) > > >> > > >> # ---------------------------------------------------------------------------- > > >> diff --git a/rules/barebox.rockchip.make b/rules/barebox.rockchip.make > > >> new file mode 100644 > > >> index 000000000..33795bee4 > > >> --- /dev/null > > >> +++ b/rules/barebox.rockchip.make > > >> @@ -0,0 +1,16 @@ > > >> +# -*-makefile-*- > > >> +# > > >> +# Copyright (C) 2021 by Michael Riesch > > >> +# > > >> +# For further information about the PTXdist project and license conditions > > >> +# see the README file. > > >> +# > > >> + > > >> +ifdef PTXCONF_BAREBOX_NEEDS_FIRMWARE_ROCKCHIP > > >> + > > >> +BAREBOX_INJECT_PATH += ${PTXDIST_SYSROOT_TARGET}/usr/lib/firmware > > > > > > This should be in barebox.make. BAREBOX_INJECT_PATH is a ':' separated > > > path, so expanding it like this doesn't work anyways. > > > > Will fix this in v5! > > > > > So in general I've been thinking which parts make sense in PTXdist upstream > > > and which parts should be in the BSPs. I mean it doesn't really scale, if > > > we add firmware files for all boards into PTXdist upstream. So I had the > > > vague idea that we could add files that are SoC specific to PTXdist and > > > leave the board specific files in the BSP. > > > > Not sure whether this is a SoC-specific/board-specific issue, but I > > agree that there are a manifold of possibilities here and naturally we > > cannot cover everything. I would say the aim here is to provide some > > generic means to integrate binary blobs into packages and (optionally) > > provide some reasonable defaults. As you stated previously, the defaults > > can be overwritten if desired. > > > > > Then I saw this: > > > > > >> +BAREBOX_INJECT_FILES += rk3568_ddr_1560MHz_v1.08.bin:arch/arm/boards/rockchip-rk3568-evb/sdram-init.bin > > > > > > My understanding was that this file is the code that does the SDRAM timing > > > calibration. So I expected it to be SoC specific and therefore in a more > > > generic path, not the board directory. > > > Am I missing something here? > > > > As far as I understand, the binary contains the SDRAM setup (rows, cols, > > etc.) which may be board specific. But you may want to talk to Sascha > > for more details. > > Sascha, can comment on this? I think it would be nice to provide any binary > blobs that can be used by multiple board in PTXdist upstream. However, for > that to work we would need a generic location in the barebox source tree. > Any reason why a board specific location is used here? The files are board specific so it felt natural to put them in a board specific location. Anyway, having a single directory where such files are located seems to be a good idea. Not only that it helps external build systems like ptxdist, but we could also create tarballs or repositories with binary blobs which could be taken as-is and the files wouldn't have to be copied to special locations in the barebox tree. > > > >> +BAREBOX_INJECT_FILES += rk3568_bl31_v1.24.elf:firmware/rk3568-bl31.bin > > > > > > That's TF-A right? > > > > Yes. > > > > >> +BAREBOX_INJECT_FILES += rk3568_bl32_v1.05.bin:firmware/rk3568-op-tee.bin > > > > > > I expected these files to be board specific, but they are in a generic > > > directory? Also, I would expect those to be built from source. > > > > Again, Sascha is the expert here. For all I know those are SoC-specific > > and barebox expects them to reside at this particular location. > > Sascha, same here, just the reverse. OP-TEE files are not necessarily board specific. Often enough they are, because things like serial ports are hardcoded, but in theory OP-TEE could take a device tree and may not even be SoC specific. I think the current OP-TEE codebase makes it hard to actually create board or SoC agnostic images, but who knows how things develop in the future. > > > Indeed one may want to generate these binaries from the mainline > > sources. However, at the moment those sources are not available for > > RK356x. Rockchip tends to provide only a binary blob when a new SoC is > > released, the sources follow about 1-2 years later. > > And if you need to make board specific changes? :-/ I guess you know the answer :) Generally I like the idea of having a single directory with binary blobs and other firmware files. This would also allow us to let barebox take board specific blobs which could point to more generic files using symlinks. I think ptxdist could provide ways to either extract specific files from a tarball or to let the user choose to build certain things like OP-TEE or TF-A himself. Sounds like some work for cleaning that up in barebox, but doable and worthwile. Sascha -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | _______________________________________________ ptxdist mailing list ptxdist@pengutronix.de To unsubscribe, send a mail with subject "unsubscribe" to ptxdist-request@pengutronix.de