All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] spl: fit: Minimally parse OS properties with FIT_IMAGE_TINY
@ 2020-05-07 23:20 Samuel Holland
  2020-05-07 23:20 ` [PATCH 2/3] mksunxi_fit_atf.sh: Update FIT component descriptions Samuel Holland
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Samuel Holland @ 2020-05-07 23:20 UTC (permalink / raw)
  To: u-boot

Some boards, specifically 64-bit Allwinner boards (sun50i), are
extremely limited on SPL size. One strategy that was used to make space
was to remove the FIT "os" property parsing code, because it uses a
rather large lookup table.

However, this forces the legacy FIT parsing code path, which requires
the "firmware" entry in the FIT to reference the U-Boot binary, even if
U-Boot is not the next binary in the boot sequence (for example, on
sun50i boards, ATF is run first).

This prevents the same FIT image from being used with a SPL with
CONFIG_SPL_FIT_IMAGE_TINY=n and CONFIG_SPL_ATF=y, because the boot
method selection code looks at `spl_image.os`, which is only set from
the "firmware" entry's "os" property.

To be able to use CONFIG_SPL_ATF=y, the "firmware" entry in the FIT
must be ATF, and U-Boot must be a loadable. For this to work, we need to
parse the "os" property just enough to tell U-Boot from other images, so
we can find it in the loadables list to append the FDT, and so we don't
try to append the FDT to ATF (which could clobber adjacent firmware).

So add the minimal code necessary to distinguish U-Boot/non-U-Boot
loadables with CONFIG_SPL_FIT_IMAGE_TINY=y. This adds about 300 bytes,
much less than the 7400 bytes added by CONFIG_SPL_FIT_IMAGE_TINY=n.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---
 common/spl/Kconfig   |  4 +---
 common/spl/spl_fit.c | 17 ++++++++++++++++-
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/common/spl/Kconfig b/common/spl/Kconfig
index 9feadb5e43..f2fa12354d 100644
--- a/common/spl/Kconfig
+++ b/common/spl/Kconfig
@@ -448,9 +448,7 @@ config SPL_FIT_IMAGE_TINY
 	  Enable this to reduce the size of the FIT image loading code
 	  in SPL, if space for the SPL binary is very tight.
 
-	  This removes the detection of image types (which forces the
-	  first image to be treated as having a U-Boot style calling
-	  convention) and skips the recording of each loaded payload
+	  This skips the recording of each loaded payload
 	  (i.e. loadable) into the FDT (modifying the loaded FDT to
 	  ensure this information is available to the next image
 	  invoked).
diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index c51e4beb1c..b9dd4211aa 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -464,7 +464,22 @@ static int spl_fit_record_loadable(const void *fit, int images, int index,
 static int spl_fit_image_get_os(const void *fit, int noffset, uint8_t *os)
 {
 #if CONFIG_IS_ENABLED(FIT_IMAGE_TINY) && !defined(CONFIG_SPL_OS_BOOT)
-	return -ENOTSUPP;
+	const char *name = fdt_getprop(fit, noffset, FIT_OS_PROP, NULL);
+
+	if (!name)
+		return -ENOENT;
+
+	/*
+	 * We don't care what the type of the image actually is,
+	 * only whether or not it is U-Boot. This saves some
+	 * space by omitting the large table of OS types.
+	 */
+	if (!strcmp(name, "u-boot"))
+		*os = IH_OS_U_BOOT;
+	else
+		*os = IH_OS_INVALID;
+
+	return 0;
 #else
 	return fit_image_get_os(fit, noffset, os);
 #endif
-- 
2.24.1

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/3] mksunxi_fit_atf.sh: Update FIT component descriptions
  2020-05-07 23:20 [PATCH 1/3] spl: fit: Minimally parse OS properties with FIT_IMAGE_TINY Samuel Holland
@ 2020-05-07 23:20 ` Samuel Holland
  2020-05-08  9:45   ` Patrick Wildt
  2020-05-07 23:20 ` [PATCH 3/3] sunxi: Add support for including SCP firmware Samuel Holland
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: Samuel Holland @ 2020-05-07 23:20 UTC (permalink / raw)
  To: u-boot

Since commit d879616e9e64 ("spl: fit: simplify logic for FDT loading for
non-OS boots"), the SPL looks at the "os" properties of FIT images to
determine where to append the FDT.

The "os" property of the "firmware" image also determines how to execute
the next stage of the boot process, as in 1d3790905d9c ("spl: atf:
introduce spl_invoke_atf and make bl31_entry private").

To support this additional functionality, and to properly model the boot
process, where ATF runs before U-Boot, add the "os" properties and swap
the firmware/loadable images in the FIT image.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---
 board/sunxi/mksunxi_fit_atf.sh | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/board/sunxi/mksunxi_fit_atf.sh b/board/sunxi/mksunxi_fit_atf.sh
index 88ad719747..4dfd22db78 100755
--- a/board/sunxi/mksunxi_fit_atf.sh
+++ b/board/sunxi/mksunxi_fit_atf.sh
@@ -31,6 +31,7 @@ cat << __HEADER_EOF
 			description = "U-Boot (64-bit)";
 			data = /incbin/("u-boot-nodtb.bin");
 			type = "standalone";
+			os = "u-boot";
 			arch = "arm64";
 			compression = "none";
 			load = <0x4a000000>;
@@ -39,6 +40,7 @@ cat << __HEADER_EOF
 			description = "ARM Trusted Firmware";
 			data = /incbin/("$BL31");
 			type = "firmware";
+			os = "arm-trusted-firmware";
 			arch = "arm64";
 			compression = "none";
 			load = <$BL31_ADDR>;
@@ -73,8 +75,8 @@ do
 	cat << __CONF_SECTION_EOF
 		config_$cnt {
 			description = "$(basename $dtname .dtb)";
-			firmware = "uboot";
-			loadables = "atf";
+			firmware = "atf";
+			loadables = "uboot";
 			fdt = "fdt_$cnt";
 		};
 __CONF_SECTION_EOF
-- 
2.24.1

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/3] sunxi: Add support for including SCP firmware
  2020-05-07 23:20 [PATCH 1/3] spl: fit: Minimally parse OS properties with FIT_IMAGE_TINY Samuel Holland
  2020-05-07 23:20 ` [PATCH 2/3] mksunxi_fit_atf.sh: Update FIT component descriptions Samuel Holland
@ 2020-05-07 23:20 ` Samuel Holland
  2020-05-08  9:47 ` [PATCH 1/3] spl: fit: Minimally parse OS properties with FIT_IMAGE_TINY Patrick Wildt
  2020-06-01 17:04 ` Jagan Teki
  3 siblings, 0 replies; 8+ messages in thread
From: Samuel Holland @ 2020-05-07 23:20 UTC (permalink / raw)
  To: u-boot

Allwinner sun50i SoCs contain an OpenRISC 1000 CPU that functions as a
System Control Processor, or SCP. ARM Trusted Firmware (ATF)
communicates with the SCP over SCPI to implement the PSCI system suspend
and shutdown functionality. Currently, SCP firmware is optional; the
system will boot and run without it, but system suspend will be
unavailable.

Since all communication with the SCP is mediated by ATF, the only thing
U-Boot needs to do is load the firmware into SRAM. The SCP firmware
occupies the last 16KiB of SRAM A2, immediately following ATF.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---
 board/sunxi/README.sunxi64     | 43 ++++++++++++++++++++++++++++------
 board/sunxi/mksunxi_fit_atf.sh | 23 +++++++++++++++---
 2 files changed, 56 insertions(+), 10 deletions(-)

diff --git a/board/sunxi/README.sunxi64 b/board/sunxi/README.sunxi64
index 258921af22..9a67e5301e 100644
--- a/board/sunxi/README.sunxi64
+++ b/board/sunxi/README.sunxi64
@@ -14,8 +14,12 @@ Quick Start / Overview
 - Build the ARM Trusted Firmware binary (see "ARM Trusted Firmware (ATF)" below)
   $ cd /src/arm-trusted-firmware
   $ make PLAT=sun50i_a64 DEBUG=1 bl31
+- Build the SCP firmware binary (see "SCP firmware (Crust)" below)
+  $ cd /src/crust
+  $ make pine64_plus_defconfig && make -j5 scp
 - Build U-Boot (see "SPL/U-Boot" below)
   $ export BL31=/path/to/bl31.bin
+  $ export SCP=/src/crust/build/scp/scp.bin
   $ make pine64_plus_defconfig && make -j5
 - Transfer to an uSD card (see "microSD card" below)
   $ dd if=u-boot-sunxi-with-spl.bin of=/dev/sdx bs=8k seek=1
@@ -24,13 +28,17 @@ Quick Start / Overview
 Building the firmware
 =====================
 
-The Allwinner A64/H5 firmware consists of three parts: U-Boot's SPL, an
-ARM Trusted Firmware (ATF) build and the U-Boot proper.
-The SPL will load both ATF and U-Boot proper along with the right device
-tree blob (.dtb) and will pass execution to ATF (in EL3), which in turn will
-drop into the U-Boot proper (in EL2).
-As the ATF binary will become part of the U-Boot image file, you will need
-to build it first.
+The Allwinner A64/H5/H6 firmware consists of several parts: U-Boot's SPL,
+ARM Trusted Firmware (ATF), optional System Control Processor (SCP) firmware
+(e.g. Crust), and the U-Boot proper.
+
+The SPL will load all of the other firmware binaries into RAM, along with the
+right device tree blob (.dtb), and will pass execution to ATF (in EL3). If SCP
+firmware was loaded, ATF will power on the SCP and wait for it to boot.
+ATF will then drop into U-Boot proper (in EL2).
+
+As the ATF binary and SCP firmware will become part of the U-Boot image file,
+you will need to build them first.
 
  ARM Trusted Firmware (ATF)
 ----------------------------
@@ -53,6 +61,27 @@ As sometimes the ATF build process is a bit picky about the toolchain used,
 or if you can't be bothered with building ATF, there are known working
 binaries in the firmware repository[3], purely for convenience reasons.
 
+ SCP firmware (Crust)
+----------------------
+SCP firmware is responsible for implementing system suspend/resume, and (on
+boards without a PMIC) soft poweroff/on. ATF contains fallback code for CPU
+power control, so SCP firmware is optional if you don't need either of these
+features. It runs on the AR100, with is an or1k CPU, not ARM, so it needs a
+different cross toolchain.
+
+There is one SCP firmware implementation currently available, Crust:
+$ git clone https://github.com/crust-firmware/crust
+$ cd crust
+$ export CROSS_COMPILE=or1k-linux-musl-
+$ make pine64_plus_defconfig
+$ make scp
+
+The same configuration generally works on any board with the same SoC (A64, H5,
+or H6), so if there is no config for your board, use one for a similar board.
+
+Like for ATF, U-Boot finds the SCP firmware binary via an environment variable:
+$ export SCP=/src/crust/build/scp/scp.bin
+
  SPL/U-Boot
 ------------
 Both U-Boot proper and the SPL are using the 64-bit mode. As the boot ROM
diff --git a/board/sunxi/mksunxi_fit_atf.sh b/board/sunxi/mksunxi_fit_atf.sh
index 4dfd22db78..07a2e619ee 100755
--- a/board/sunxi/mksunxi_fit_atf.sh
+++ b/board/sunxi/mksunxi_fit_atf.sh
@@ -1,11 +1,12 @@
 #!/bin/sh
 #
-# script to generate FIT image source for 64-bit sunxi boards with
-# ARM Trusted Firmware and multiple device trees (given on the command line)
+# script to generate FIT image source for 64-bit sunxi boards with ARM Trusted
+# Firmware, SCP firmware, and multiple device trees (given on the command line)
 #
 # usage: $0 <dt_name> [<dt_name> [<dt_name] ...]
 
 [ -z "$BL31" ] && BL31="bl31.bin"
+[ -z "$SCP" ] && SCP="scp.bin"
 
 if [ ! -f $BL31 ]; then
 	echo "WARNING: BL31 file $BL31 NOT found, resulting binary is non-functional" >&2
@@ -13,10 +14,18 @@ if [ ! -f $BL31 ]; then
 	BL31=/dev/null
 fi
 
+if [ ! -f $SCP ]; then
+	echo "WARNING: SCP firmware file $SCP NOT found, system suspend will be unavailable" >&2
+	echo "Please read the section on SCP firmware in board/sunxi/README.sunxi64" >&2
+	SCP=/dev/null
+fi
+
 if grep -q "^CONFIG_MACH_SUN50I_H6=y" .config; then
 	BL31_ADDR=0x104000
+	SCP_ADDR=0x114000
 else
 	BL31_ADDR=0x44000
+	SCP_ADDR=0x50000
 fi
 
 cat << __HEADER_EOF
@@ -46,6 +55,14 @@ cat << __HEADER_EOF
 			load = <$BL31_ADDR>;
 			entry = <$BL31_ADDR>;
 		};
+		scp {
+			description = "SCP firmware";
+			data = /incbin/("$SCP");
+			type = "firmware";
+			arch = "or1k";
+			compression = "none";
+			load = <$SCP_ADDR>;
+		};
 __HEADER_EOF
 
 cnt=1
@@ -76,7 +93,7 @@ do
 		config_$cnt {
 			description = "$(basename $dtname .dtb)";
 			firmware = "atf";
-			loadables = "uboot";
+			loadables = "scp", "uboot";
 			fdt = "fdt_$cnt";
 		};
 __CONF_SECTION_EOF
-- 
2.24.1

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/3] mksunxi_fit_atf.sh: Update FIT component descriptions
  2020-05-07 23:20 ` [PATCH 2/3] mksunxi_fit_atf.sh: Update FIT component descriptions Samuel Holland
@ 2020-05-08  9:45   ` Patrick Wildt
  2020-05-09 19:02     ` Samuel Holland
  0 siblings, 1 reply; 8+ messages in thread
From: Patrick Wildt @ 2020-05-08  9:45 UTC (permalink / raw)
  To: u-boot

Hi,

now this really confuses me.

commit 0db0ba6141f402b1d496ef53d9fa69978f75ec61 has explicitly made
u-boot the firmware and moved atf into the loadables on NXP i.MX.
Here you do the complete opposite for sunxi.

Can people please make up their minds how it is *supposed* to work?

Oh, and your previous diff about the "minimal os parsing", I need that
too for my use-case, so I like that one!

Patrick

On Thu, May 07, 2020 at 06:20:34PM -0500, Samuel Holland wrote:
> Since commit d879616e9e64 ("spl: fit: simplify logic for FDT loading for
> non-OS boots"), the SPL looks at the "os" properties of FIT images to
> determine where to append the FDT.
> 
> The "os" property of the "firmware" image also determines how to execute
> the next stage of the boot process, as in 1d3790905d9c ("spl: atf:
> introduce spl_invoke_atf and make bl31_entry private").
> 
> To support this additional functionality, and to properly model the boot
> process, where ATF runs before U-Boot, add the "os" properties and swap
> the firmware/loadable images in the FIT image.
> 
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---
>  board/sunxi/mksunxi_fit_atf.sh | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/board/sunxi/mksunxi_fit_atf.sh b/board/sunxi/mksunxi_fit_atf.sh
> index 88ad719747..4dfd22db78 100755
> --- a/board/sunxi/mksunxi_fit_atf.sh
> +++ b/board/sunxi/mksunxi_fit_atf.sh
> @@ -31,6 +31,7 @@ cat << __HEADER_EOF
>  			description = "U-Boot (64-bit)";
>  			data = /incbin/("u-boot-nodtb.bin");
>  			type = "standalone";
> +			os = "u-boot";
>  			arch = "arm64";
>  			compression = "none";
>  			load = <0x4a000000>;
> @@ -39,6 +40,7 @@ cat << __HEADER_EOF
>  			description = "ARM Trusted Firmware";
>  			data = /incbin/("$BL31");
>  			type = "firmware";
> +			os = "arm-trusted-firmware";
>  			arch = "arm64";
>  			compression = "none";
>  			load = <$BL31_ADDR>;
> @@ -73,8 +75,8 @@ do
>  	cat << __CONF_SECTION_EOF
>  		config_$cnt {
>  			description = "$(basename $dtname .dtb)";
> -			firmware = "uboot";
> -			loadables = "atf";
> +			firmware = "atf";
> +			loadables = "uboot";
>  			fdt = "fdt_$cnt";
>  		};
>  __CONF_SECTION_EOF
> -- 
> 2.24.1
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/3] spl: fit: Minimally parse OS properties with FIT_IMAGE_TINY
  2020-05-07 23:20 [PATCH 1/3] spl: fit: Minimally parse OS properties with FIT_IMAGE_TINY Samuel Holland
  2020-05-07 23:20 ` [PATCH 2/3] mksunxi_fit_atf.sh: Update FIT component descriptions Samuel Holland
  2020-05-07 23:20 ` [PATCH 3/3] sunxi: Add support for including SCP firmware Samuel Holland
@ 2020-05-08  9:47 ` Patrick Wildt
  2020-06-01 17:04 ` Jagan Teki
  3 siblings, 0 replies; 8+ messages in thread
From: Patrick Wildt @ 2020-05-08  9:47 UTC (permalink / raw)
  To: u-boot

On Thu, May 07, 2020 at 06:20:33PM -0500, Samuel Holland wrote:
> Some boards, specifically 64-bit Allwinner boards (sun50i), are
> extremely limited on SPL size. One strategy that was used to make space
> was to remove the FIT "os" property parsing code, because it uses a
> rather large lookup table.
> 
> However, this forces the legacy FIT parsing code path, which requires
> the "firmware" entry in the FIT to reference the U-Boot binary, even if
> U-Boot is not the next binary in the boot sequence (for example, on
> sun50i boards, ATF is run first).
> 
> This prevents the same FIT image from being used with a SPL with
> CONFIG_SPL_FIT_IMAGE_TINY=n and CONFIG_SPL_ATF=y, because the boot
> method selection code looks at `spl_image.os`, which is only set from
> the "firmware" entry's "os" property.
> 
> To be able to use CONFIG_SPL_ATF=y, the "firmware" entry in the FIT
> must be ATF, and U-Boot must be a loadable. For this to work, we need to
> parse the "os" property just enough to tell U-Boot from other images, so
> we can find it in the loadables list to append the FDT, and so we don't
> try to append the FDT to ATF (which could clobber adjacent firmware).
> 
> So add the minimal code necessary to distinguish U-Boot/non-U-Boot
> loadables with CONFIG_SPL_FIT_IMAGE_TINY=y. This adds about 300 bytes,
> much less than the 7400 bytes added by CONFIG_SPL_FIT_IMAGE_TINY=n.
> 
> Signed-off-by: Samuel Holland <samuel@sholland.org>

Acked-by: Patrick Wildt <patrick@blueri.se>

> ---
>  common/spl/Kconfig   |  4 +---
>  common/spl/spl_fit.c | 17 ++++++++++++++++-
>  2 files changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/common/spl/Kconfig b/common/spl/Kconfig
> index 9feadb5e43..f2fa12354d 100644
> --- a/common/spl/Kconfig
> +++ b/common/spl/Kconfig
> @@ -448,9 +448,7 @@ config SPL_FIT_IMAGE_TINY
>  	  Enable this to reduce the size of the FIT image loading code
>  	  in SPL, if space for the SPL binary is very tight.
>  
> -	  This removes the detection of image types (which forces the
> -	  first image to be treated as having a U-Boot style calling
> -	  convention) and skips the recording of each loaded payload
> +	  This skips the recording of each loaded payload
>  	  (i.e. loadable) into the FDT (modifying the loaded FDT to
>  	  ensure this information is available to the next image
>  	  invoked).
> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
> index c51e4beb1c..b9dd4211aa 100644
> --- a/common/spl/spl_fit.c
> +++ b/common/spl/spl_fit.c
> @@ -464,7 +464,22 @@ static int spl_fit_record_loadable(const void *fit, int images, int index,
>  static int spl_fit_image_get_os(const void *fit, int noffset, uint8_t *os)
>  {
>  #if CONFIG_IS_ENABLED(FIT_IMAGE_TINY) && !defined(CONFIG_SPL_OS_BOOT)
> -	return -ENOTSUPP;
> +	const char *name = fdt_getprop(fit, noffset, FIT_OS_PROP, NULL);
> +
> +	if (!name)
> +		return -ENOENT;
> +
> +	/*
> +	 * We don't care what the type of the image actually is,
> +	 * only whether or not it is U-Boot. This saves some
> +	 * space by omitting the large table of OS types.
> +	 */
> +	if (!strcmp(name, "u-boot"))
> +		*os = IH_OS_U_BOOT;
> +	else
> +		*os = IH_OS_INVALID;
> +
> +	return 0;
>  #else
>  	return fit_image_get_os(fit, noffset, os);
>  #endif
> -- 
> 2.24.1
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 2/3] mksunxi_fit_atf.sh: Update FIT component descriptions
  2020-05-08  9:45   ` Patrick Wildt
@ 2020-05-09 19:02     ` Samuel Holland
  2020-05-09 19:09       ` Patrick Wildt
  0 siblings, 1 reply; 8+ messages in thread
From: Samuel Holland @ 2020-05-09 19:02 UTC (permalink / raw)
  To: u-boot

Hi,

On 5/8/20 4:45 AM, Patrick Wildt wrote:
> Hi,
> 
> now this really confuses me.
> 
> commit 0db0ba6141f402b1d496ef53d9fa69978f75ec61 has explicitly made
> u-boot the firmware and moved atf into the loadables on NXP i.MX.
> Here you do the complete opposite for sunxi.
> 
> Can people please make up their minds how it is *supposed* to work?

I don't think that commit is suggesting how things are supposed to work; it's a
workaround responding to the existing limitations in SPL_FIT_IMAGE_TINY.
Specifically, that "firmware" is assumed to be U-Boot, and "loadables" are
assumed to be something else.

The first patch in this series removes those limitations by actually looking at
the "os" property. With my first patch applied, U-Boot would be detected in
either list, so booting would work with or without commit 0db0ba6141f4.

So for the reasons I outline below (the functionality of the "switch
(spl_image.os)" in board_init_r), it might make sense to revert that commit
after applying this series.

Cheers,
Samuel

> Oh, and your previous diff about the "minimal os parsing", I need that
> too for my use-case, so I like that one!
> 
> Patrick
> 
> On Thu, May 07, 2020 at 06:20:34PM -0500, Samuel Holland wrote:
>> Since commit d879616e9e64 ("spl: fit: simplify logic for FDT loading for
>> non-OS boots"), the SPL looks at the "os" properties of FIT images to
>> determine where to append the FDT.
>>
>> The "os" property of the "firmware" image also determines how to execute
>> the next stage of the boot process, as in 1d3790905d9c ("spl: atf:
>> introduce spl_invoke_atf and make bl31_entry private").
>>
>> To support this additional functionality, and to properly model the boot
>> process, where ATF runs before U-Boot, add the "os" properties and swap
>> the firmware/loadable images in the FIT image.
>>
>> Signed-off-by: Samuel Holland <samuel@sholland.org>
>> ---
>>  board/sunxi/mksunxi_fit_atf.sh | 6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/board/sunxi/mksunxi_fit_atf.sh b/board/sunxi/mksunxi_fit_atf.sh
>> index 88ad719747..4dfd22db78 100755
>> --- a/board/sunxi/mksunxi_fit_atf.sh
>> +++ b/board/sunxi/mksunxi_fit_atf.sh
>> @@ -31,6 +31,7 @@ cat << __HEADER_EOF
>>  			description = "U-Boot (64-bit)";
>>  			data = /incbin/("u-boot-nodtb.bin");
>>  			type = "standalone";
>> +			os = "u-boot";
>>  			arch = "arm64";
>>  			compression = "none";
>>  			load = <0x4a000000>;
>> @@ -39,6 +40,7 @@ cat << __HEADER_EOF
>>  			description = "ARM Trusted Firmware";
>>  			data = /incbin/("$BL31");
>>  			type = "firmware";
>> +			os = "arm-trusted-firmware";
>>  			arch = "arm64";
>>  			compression = "none";
>>  			load = <$BL31_ADDR>;
>> @@ -73,8 +75,8 @@ do
>>  	cat << __CONF_SECTION_EOF
>>  		config_$cnt {
>>  			description = "$(basename $dtname .dtb)";
>> -			firmware = "uboot";
>> -			loadables = "atf";
>> +			firmware = "atf";
>> +			loadables = "uboot";
>>  			fdt = "fdt_$cnt";
>>  		};
>>  __CONF_SECTION_EOF
>> -- 
>> 2.24.1
>>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 2/3] mksunxi_fit_atf.sh: Update FIT component descriptions
  2020-05-09 19:02     ` Samuel Holland
@ 2020-05-09 19:09       ` Patrick Wildt
  0 siblings, 0 replies; 8+ messages in thread
From: Patrick Wildt @ 2020-05-09 19:09 UTC (permalink / raw)
  To: u-boot

On Sat, May 09, 2020 at 02:02:19PM -0500, Samuel Holland wrote:
> On 5/8/20 4:45 AM, Patrick Wildt wrote:
> > Hi,
> > 
> > now this really confuses me.
> > 
> > commit 0db0ba6141f402b1d496ef53d9fa69978f75ec61 has explicitly made
> > u-boot the firmware and moved atf into the loadables on NXP i.MX.
> > Here you do the complete opposite for sunxi.
> > 
> > Can people please make up their minds how it is *supposed* to work?
> 
> I don't think that commit is suggesting how things are supposed to work; it's a
> workaround responding to the existing limitations in SPL_FIT_IMAGE_TINY.
> Specifically, that "firmware" is assumed to be U-Boot, and "loadables" are
> assumed to be something else.
> 
> The first patch in this series removes those limitations by actually looking at
> the "os" property. With my first patch applied, U-Boot would be detected in
> either list, so booting would work with or without commit 0db0ba6141f4.
> 
> So for the reasons I outline below (the functionality of the "switch
> (spl_image.os)" in board_init_r), it might make sense to revert that commit
> after applying this series.
> 
> Cheers,
> Samuel

I tend to agree.  Having ATF as a "firmware", spl_load_simple_fit()
would with your diff actually recognize that it's ATF and not U-Boot, so
it wouldn't append the FDT.  Then it goes over the loadables.  Loads
U-Boot, sees it is U-Boot, appends the FDT, and then (possibly) OP-TEE.

I'm still not sure what should be "firmware" and what "loadables", but
if ATF is supposed to be "firmware", your diff makes sense and it would
make sense to revert the change in the i.MX mkimage script.

So in that case I'd say:

Acked-by: Patrick Wildt <patrick@blueri.se>

Best regards,
Patrick

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/3] spl: fit: Minimally parse OS properties with FIT_IMAGE_TINY
  2020-05-07 23:20 [PATCH 1/3] spl: fit: Minimally parse OS properties with FIT_IMAGE_TINY Samuel Holland
                   ` (2 preceding siblings ...)
  2020-05-08  9:47 ` [PATCH 1/3] spl: fit: Minimally parse OS properties with FIT_IMAGE_TINY Patrick Wildt
@ 2020-06-01 17:04 ` Jagan Teki
  3 siblings, 0 replies; 8+ messages in thread
From: Jagan Teki @ 2020-06-01 17:04 UTC (permalink / raw)
  To: u-boot

On Fri, May 8, 2020 at 4:49 AM Samuel Holland <samuel@sholland.org> wrote:
>
> Some boards, specifically 64-bit Allwinner boards (sun50i), are
> extremely limited on SPL size. One strategy that was used to make space
> was to remove the FIT "os" property parsing code, because it uses a
> rather large lookup table.
>
> However, this forces the legacy FIT parsing code path, which requires
> the "firmware" entry in the FIT to reference the U-Boot binary, even if
> U-Boot is not the next binary in the boot sequence (for example, on
> sun50i boards, ATF is run first).
>
> This prevents the same FIT image from being used with a SPL with
> CONFIG_SPL_FIT_IMAGE_TINY=n and CONFIG_SPL_ATF=y, because the boot
> method selection code looks at `spl_image.os`, which is only set from
> the "firmware" entry's "os" property.
>
> To be able to use CONFIG_SPL_ATF=y, the "firmware" entry in the FIT
> must be ATF, and U-Boot must be a loadable. For this to work, we need to
> parse the "os" property just enough to tell U-Boot from other images, so
> we can find it in the loadables list to append the FDT, and so we don't
> try to append the FDT to ATF (which could clobber adjacent firmware).
>
> So add the minimal code necessary to distinguish U-Boot/non-U-Boot
> loadables with CONFIG_SPL_FIT_IMAGE_TINY=y. This adds about 300 bytes,
> much less than the 7400 bytes added by CONFIG_SPL_FIT_IMAGE_TINY=n.
>
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---

+ Andre

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-06-01 17:04 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-07 23:20 [PATCH 1/3] spl: fit: Minimally parse OS properties with FIT_IMAGE_TINY Samuel Holland
2020-05-07 23:20 ` [PATCH 2/3] mksunxi_fit_atf.sh: Update FIT component descriptions Samuel Holland
2020-05-08  9:45   ` Patrick Wildt
2020-05-09 19:02     ` Samuel Holland
2020-05-09 19:09       ` Patrick Wildt
2020-05-07 23:20 ` [PATCH 3/3] sunxi: Add support for including SCP firmware Samuel Holland
2020-05-08  9:47 ` [PATCH 1/3] spl: fit: Minimally parse OS properties with FIT_IMAGE_TINY Patrick Wildt
2020-06-01 17:04 ` Jagan Teki

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.