Hi,

I add in CC the ARM STM STM32MP Maintainers...

On 8/26/21 11:42 PM, Alexandru Gagniuc wrote:

The UART can reliably go up to 2000000 baud when connected to the on-board st-link. Unfortunately u-boot will fall back to 115200 unless higher rates are declared via CONFIG_SYS_BAUDRATE_TABLE.

Signed-off-by: Alexandru Gagniuc mr.nuke.me@gmail.com

include/configs/stm32mp1.h | 4 ++++ 1 file changed, 4 insertions(+)

diff --git a/include/configs/stm32mp1.h b/include/configs/stm32mp1.h index b372838be8..9fcd60285a 100644 --- a/include/configs/stm32mp1.h +++ b/include/configs/stm32mp1.h @@ -16,6 +16,10 @@ #define CONFIG_ARMV7_SECURE_MAX_SIZE STM32_SYSRAM_SIZE #endif

+#define CONFIG_SYS_BAUDRATE_TABLE { 9600, 19200, 38400, 57600, 115200, \

• 			 230400, 460800, 921600, \
  

• 			 1000000, 2000000 }
  

• /*
  • Configuration of the external SRAM memory used by U-Boot
  */

Reviewed-by: Patrick Delaunay patrick.delaunay@foss.st.com

Thanks Patrick

Hi,

Add in CC the MAINTAINERS

On 8/26/21 11:42 PM, Alexandru Gagniuc wrote:

Falcon mode requires a board-specific mechanism to select between fast and normal boot. This is done via spl_start_uboot()

Use the B2 button as the selection mechanism. This is connected to GPIO PA13. Incidentally, this GPIO is already accessible via the "st,fastboot-gpios" devicetree node.

Offsets for raw MMC loading are defined. These point to the partition after "ssbl".

Signed-off-by: Alexandru Gagniuc mr.nuke.me@gmail.com

board/st/stm32mp1/spl.c | 39 ++++++++++++++++++++++++++++++++++++++ include/configs/stm32mp1.h | 13 +++++++++++++ 2 files changed, 52 insertions(+)

diff --git a/board/st/stm32mp1/spl.c b/board/st/stm32mp1/spl.c index 8e4549a1b3..bb210d7727 100644 --- a/board/st/stm32mp1/spl.c +++ b/board/st/stm32mp1/spl.c @@ -8,6 +8,7 @@ #include <init.h> #include <asm/io.h> #include <asm/arch/sys_proto.h> +#include <asm/gpio.h> #include <linux/bitops.h> #include <linux/delay.h> #include "../common/stpmic1.h" @@ -29,6 +30,44 @@ int board_early_init_f(void) return 0; }

+#if IS_ENABLED(CONFIG_SPL_OS_BOOT) +int spl_start_uboot(void) +{

• ofnode node;
• struct gpio_desc gpio;
• int boot_uboot = 1;
• node = ofnode_path("/config");
• if (!ofnode_valid(node)) {

• pr_warn("%s: no /config node?\n", __func__);
  

• return 0;
  

• }
• if (gpio_request_by_name_nodev(node, "st,fastboot-gpios", 0, &gpio,

• 		       GPIOD_IS_IN)) {
  

• pr_warn("%s: could not find a /config/st,fastboot-gpios\n",
  

• 	__func__);
  

• return 1;
  

• }

The node "st,fastboot-gpios" is used in STMicroelectronics devicetree / board to select the KEY to launch the ANDROID command fastboot in board_key_check()

=> it can't be re-used to other purpose else you will have conflict when the key is pressed:

what append when key pressed in basic boot mode when KEY is pressed ? => falcon mode selected in SPL or FASTBOOT mode selected ....

"st,fastboot-gpios" meaning is defined in:

https://wiki.st.com/stm32mpu/wiki/How_to_configure_U-Boot_for_your_board#Con...

you should use a other config ? => "mrnuke,falcon-gpios" managed in your board .c file / dts file

or hardcoded in your board.c ?

or use environment.... if (env_get_yesno("boot_os") != 0)

• boot_uboot = dm_gpio_get_value(&gpio);
• dm_gpio_free(NULL, &gpio);
• return boot_uboot;

+}

+#if IS_ENABLED(CONFIG_ARMV7_NONSEC) +/*

• • A bit of a hack, but armv7_boot_nonsec() is provided by bootm.c. This is not
• • available in SPL, so we have to provide an implementation.
• */

+bool armv7_boot_nonsec(void) +{

• return 0;

+} +#endif /* CONFIG_ARMV7_NONSEC */ +#endif /* CONFIG_SPL_OS_BOOT */

• #ifdef CONFIG_DEBUG_UART_BOARD_INIT void board_debug_uart_init(void) {

diff --git a/include/configs/stm32mp1.h b/include/configs/stm32mp1.h index 9fcd60285a..0849a1bddb 100644 --- a/include/configs/stm32mp1.h +++ b/include/configs/stm32mp1.h @@ -10,6 +10,19 @@ #include <linux/sizes.h> #include <asm/arch/stm32.h>

+/*

• • Arguments if falcon mode is used
• • CONFIG_SYS_MMCSD_RAW_MODE_KERNEL_SECTOR is the partition after "ssbl"
• • CONFIG_SYS_SPL_ARGS_ADDR is not used, but needs to point to valid RAM.
• */

+#define CONFIG_SYS_MMCSD_RAW_MODE_KERNEL_SECTOR 5154

The offset of kernel is hardcoded

=> it is not acceptable here (generic stm32mp1 file)

+#define CONFIG_SYS_SPL_ARGS_ADDR 0xc4000000

+/* Falcon mode from SPI is not supported, but the defines are needed */ +#define CONFIG_SYS_SPI_KERNEL_OFFS (~0) +#define CONFIG_SYS_SPI_ARGS_OFFS (~0) +#define CONFIG_SYS_SPI_ARGS_SIZE 0

Falcon mode is not supported by ST Microelectronics and this file is expected to support the ST boards (board/st/stm32mp1)

=> these defines should be in your configuration not in the GENERIC stm32mp1 files

for example in

include/configs/stm32mp15_falcon.h

#include "stm32mp1.h"

....

with CONFIG_SYS_CONFIG_NAME =  "stm32mp15_falcon" in your Kconfig

#ifndef CONFIG_TFABOOT /* PSCI support */ #define CONFIG_ARMV7_SECURE_BASE STM32_SYSRAM_BASE

Patrick

Hi

Add in CC the MAINTAINERS.

On 8/26/21 11:42 PM, Alexandru Gagniuc wrote:

This function is needed when loading a FIT image from SPL. It selects the correct configuration node for the current board. Implement it.

Signed-off-by: Alexandru Gagniuc mr.nuke.me@gmail.com

board/st/stm32mp1/spl.c | 10 ++++++++++ 1 file changed, 10 insertions(+)

diff --git a/board/st/stm32mp1/spl.c b/board/st/stm32mp1/spl.c index bb210d7727..543c037ad8 100644 --- a/board/st/stm32mp1/spl.c +++ b/board/st/stm32mp1/spl.c @@ -5,6 +5,7 @@

#include <config.h> #include <common.h> +#include <dm/device.h> #include <init.h> #include <asm/io.h> #include <asm/arch/sys_proto.h> @@ -92,3 +93,12 @@ void board_debug_uart_init(void) #endif } #endif

+int board_fit_config_name_match(const char *name) +{

• if (of_machine_is_compatible("st,stm32mp157c-dk2"))

• return !strstr(name, "stm32mp157c-dk2");
  

• /* Okay, it's most likely an EV board */
• return !strstr(name, "stm32mp157") + !strstr(name, "-ev");

+}

It is not working for all STMicroelectronics boards....

=> st,stm32mp157a-dk1 for example

based on board_late_init => I propose board_name extraction from compatible :

#ifdef CONFIG_SPL_LOAD_FIT int board_fit_config_name_match(const char *name) { const void *fdt_compat; int fdt_compat_len;

fdt_compat = fdt_getprop(gd->fdt_blob, 0, "compatible", &fdt_compat_len);

/* only STMicrolectronics board are supported */ if (strncmp(fdt_compat, "st,", 3) != 0) return 1; return !strstr(name, fdt_compat + 3); } #endif

On 8/26/21 5:42 PM, Alexandru Gagniuc wrote:

Oftentimes we have MAC address information stored in a ROM or OTP. The way to add that to the FDT would be through the u-boot environment, and then fdt_fixup_ethernet(). This is not very useful in SPL.

It would be more helpful to be able to "set interface x to MAC y". This is where fdt_ethernet_set_macaddr() comes in. It is similar in function to fdt_fixup_ethernet(), but only updates one interface, without using the u-boot env, and without string processing.

Have you considered adopting the nvmem-cells property for ethernet controllers (added in Linux commit 0e839df92cf3 ("net: ethernet: provide nvmem_get_mac_address()"))?

--Sean

Signed-off-by: Alexandru Gagniuc mr.nuke.me@gmail.com

common/fdt_support.c | 30 ++++++++++++++++++++++++++++++ include/fdt_support.h | 17 +++++++++++++++++ 2 files changed, 47 insertions(+)

diff --git a/common/fdt_support.c b/common/fdt_support.c index 4341d84bd5..c4cbd4060e 100644 --- a/common/fdt_support.c +++ b/common/fdt_support.c @@ -592,6 +592,36 @@ void fdt_fixup_ethernet(void *fdt) } }

+int fdt_ethernet_set_macaddr(void *fdt, int ethnum, const uint8_t *mac_addr) +{

• const char *path, *name;
• int prop, aliases_node;
• char eth_name[16] = "ethernet";
• aliases_node = fdt_path_offset(fdt, "/aliases");
• if (aliases_node < 0)

• return aliases_node;
  

• if (ethnum >= 0)

• sprintf(eth_name, "ethernet%d", ethnum);
  

• fdt_for_each_property_offset(prop, fdt, aliases_node) {

• path = fdt_getprop_by_offset(fdt, prop, &name, NULL);
  

• if (!strcmp(name, eth_name))
  

• 	break;
  

• path = NULL;
  

• }
• if (!path)

• return -FDT_ERR_NOTFOUND;
  

• do_fixup_by_path(fdt, path, "mac-address", mac_addr, 6, 0);
• do_fixup_by_path(fdt, path, "local-mac-address", mac_addr, 6, 1);
• return 0;

+}

• int fdt_record_loadable(void *blob, u32 index, const char *name, uintptr_t load_addr, u32 size, uintptr_t entry_point, const char *type, const char *os, const char *arch)

diff --git a/include/fdt_support.h b/include/fdt_support.h index f6f46bb8e9..3f0bcb5a00 100644 --- a/include/fdt_support.h +++ b/include/fdt_support.h @@ -119,6 +119,23 @@ static inline int fdt_fixup_memory_banks(void *blob, u64 start[], u64 size[], #endif

void fdt_fixup_ethernet(void *fdt);

+/**

• • Set the "mac-address" and "local-mac-address" of ethernet node
• • The ethernet node is located from the "/aliases" section of the fdt. When
• • 'ethnum' is positive, then the name is matched exactly, e.g "ethernet0".
• • When ethnum is negative, the first ethernet alias is updated.
• • Unlike fdt_fixup_ethernet(), this function only updates one ethernet node,
• • and soes not use the "ethaddr" from the u-boot environment. This is useful,
• • for example, in SPL, when the environment is not initialized or available.
• • @param fdt FDT blob to update
• • @param ethnum Ethernet device index, or negative for any ethernet
• • @param mac_addr Pointer to 6-byte array containing the MAC address
• • @return 0 if ok, or -FDT_ERR_... on error
• */

+int fdt_ethernet_set_macaddr(void *fdt, int ethnum, const uint8_t *mac_addr); int fdt_find_and_setprop(void *fdt, const char *node, const char *prop, const void *val, int len, int create); void fdt_fixup_qe_firmware(void *fdt);

Hi,

On 8/26/21 11:42 PM, Alexandru Gagniuc wrote:

When OP-TEE is booted as the SPL payload, the stage after OP-TEE is not guaranteed to be u-boot. Thus the FDT patching in u-boot is not guaranteed to occur. Add this step to SPL.

The patching by stm32_fdt_setup_mac_addr() is done in SPL, and patches the target FDT directly. This differs is different from setup_mac_address(), which sets the "ethaddr" env variable, and does not work in SPL.

Signed-off-by: Alexandru Gagniuc mr.nuke.me@gmail.com

arch/arm/mach-stm32mp/cpu.c | 22 +++++++++++++++++++ .../arm/mach-stm32mp/include/mach/sys_proto.h | 3 +++ arch/arm/mach-stm32mp/spl.c | 1 + 3 files changed, 26 insertions(+)

diff --git a/arch/arm/mach-stm32mp/cpu.c b/arch/arm/mach-stm32mp/cpu.c index 8727de513c..2b8b67bb40 100644 --- a/arch/arm/mach-stm32mp/cpu.c +++ b/arch/arm/mach-stm32mp/cpu.c @@ -10,6 +10,7 @@ #include <cpu_func.h> #include <debug_uart.h> #include <env.h> +#include <fdt_support.h> #include <init.h> #include <log.h> #include <lmb.h> @@ -646,6 +647,27 @@ __weak int setup_mac_address(void) return 0; }

+int stm32_fdt_setup_mac_addr(void *fdt) +{

• int ret;
• uchar enetaddr[ARP_HLEN];
• ret = stm32_read_otp_mac(enetaddr);
• if (ret < 0)

• return ret;
  

• if (!is_valid_ethaddr(enetaddr)) {

• printf("invalid MAC address in OTP\n");
  

• return -EINVAL;
  

• }
• ret = fdt_ethernet_set_macaddr(fdt, 0, enetaddr);
• if (ret)

• debug("Failed to set mac address from OTP: %d\n", ret);
  

• return ret;

+}

• static int setup_serial_number(void) { char serial_string[25];

diff --git a/arch/arm/mach-stm32mp/include/mach/sys_proto.h b/arch/arm/mach-stm32mp/include/mach/sys_proto.h index 4149d3a133..2d24cfee3f 100644 --- a/arch/arm/mach-stm32mp/include/mach/sys_proto.h +++ b/arch/arm/mach-stm32mp/include/mach/sys_proto.h @@ -47,7 +47,10 @@ void get_soc_name(char name[SOC_NAME_SIZE]); /* return boot mode */ u32 get_bootmode(void);

+/* Set 'ethaddr' env variable with MAC from OTP (useful for u-boot proper) */ int setup_mac_address(void); +/* Patch the first 'ethernet' node of FDT with MAC from OTP (useful for SPL) */ +int stm32_fdt_setup_mac_addr(void *fdt);

/* board power management : configure vddcore according OPP */ void board_vddcore_init(u32 voltage_mv); diff --git a/arch/arm/mach-stm32mp/spl.c b/arch/arm/mach-stm32mp/spl.c index 405eff68a3..d9fdc5926c 100644 --- a/arch/arm/mach-stm32mp/spl.c +++ b/arch/arm/mach-stm32mp/spl.c @@ -181,6 +181,7 @@ void stm32_init_tzc_for_optee(void)

void spl_board_prepare_for_optee(void *fdt) {

• stm32_fdt_setup_mac_addr(fdt); stm32_init_tzc_for_optee(); }

I think that all this part is not required for falcon mode:

let the Linux driver fallback to nvmem access to read the MAC address in OTP when the DT is not updated by boot loader

remove also the patch 06/10 and 04/40

PS: it is already working in OpenSTLinux based on the device tree modification for your board

&bsec{             ethernet_mac_address: mac@e4 {             reg = <0xe4 0x6>; };

&ethernet0 {     nvmem-cells = <&ethernet_mac_address>;     nvmem-cell-names = "mac-address"; };

PS: this part is not yet upstreamed in Linux

Linux reference = Documentation/devicetree/bindings/net/ethernet-controller.yaml

nvmem-cells:     maxItems: 1     description:       Reference to an nvmem node for the MAC address

nvmem-cell-names:     const: mac-address

Patrick

Hi Patrick,

On 8/31/21 12:24 PM, Patrick DELAUNAY wrote:

Hi,

On 8/26/21 11:42 PM, Alexandru Gagniuc wrote:

...

OP-TEE does not take a devicetree for its own use. However, it does pass the devicetree to the normal world OS. In most cases that will be some other devicetree-bearing platform, such as linux.

As in other cases where there's an OPTEE payload (e.g. BOOTM_OPTEE), it is required to copy the optee nodes to he target's FDT. Do this as part of spl_board_prepare_for_optee().

Signed-off-by: Alexandru Gagniuc mr.nuke.me@gmail.com

arch/arm/mach-stm32mp/spl.c | 2 ++   1 file changed, 2 insertions(+)

diff --git a/arch/arm/mach-stm32mp/spl.c b/arch/arm/mach-stm32mp/spl.c index d9fdc5926c..94fbb45cf9 100644 --- a/arch/arm/mach-stm32mp/spl.c +++ b/arch/arm/mach-stm32mp/spl.c @@ -19,6 +19,7 @@   #include <asm/arch/sys_proto.h>   #include <mach/tzc.h>   #include <linux/libfdt.h> +#include <tee/optee.h>   u32 spl_boot_device(void)   { @@ -182,6 +183,7 @@ void stm32_init_tzc_for_optee(void)   void spl_board_prepare_for_optee(void *fdt)   {       stm32_fdt_setup_mac_addr(fdt); +    optee_copy_fdt_nodes(fdt);       stm32_init_tzc_for_optee();   }

NAK the OP-TEE nodes are ADDED by the OP-TEE firmware in the unsecure device tree (when CFG_DT is activated)

before to jump to the kernel (that remove the need to have DT allignement with U-Boot SPL device tree)

=> SPL should not copy the OP-TEE nodes in next stage DT

reference in optee_os = core/arch/arm/kernel/boot.c: add_optee_dt_node()

add_optee_dt_node() <= update_external_dt() <= paged_init_primary()

It is the expected configuration for OP-TEE on STM32MP15 platform.

I agree that if a component modifies the platform, it should be responsible for updating the devicetree. Two issues though

1. Consistency

The STM32IMAGE boot path expects u-boot to add the optee nodes, while the FIP path expects OP-TEE to add the nodes. Which one do we stick to?

2. Memory access

I don't understand is how OP-TEE would get past the TZC. If we look at optee_os => plat-0stm32mp1/plat_tzc400: "Early boot stage is in charge of configuring memory regions" The following memory has been set up by SPL (or TF-A for that matter):

Nonsec: c0000000->ddffffff Sec: de000000->dfdfffff SHMEM: dfe00000->dfffffff

The external DTB will be in the nonsec region, which OP-TEE allegedly can't access. So how would this get patched?

Alex

Hi Alex

On 9/2/21 7:34 PM, Alex G. wrote:

Hi Patrick,

On 9/1/21 10:10 AM, Alex G. wrote:

...

Hi Patrick,

On 8/31/21 12:24 PM, Patrick DELAUNAY wrote:

...

Hi,

On 8/26/21 11:42 PM, Alexandru Gagniuc wrote:

...

OP-TEE does not take a devicetree for its own use. However, it does pass the devicetree to the normal world OS. In most cases that will be some other devicetree-bearing platform, such as linux.

As in other cases where there's an OPTEE payload (e.g. BOOTM_OPTEE), it is required to copy the optee nodes to he target's FDT. Do this as part of spl_board_prepare_for_optee().

Signed-off-by: Alexandru Gagniuc mr.nuke.me@gmail.com

arch/arm/mach-stm32mp/spl.c | 2 ++   1 file changed, 2 insertions(+)

diff --git a/arch/arm/mach-stm32mp/spl.c b/arch/arm/mach-stm32mp/spl.c index d9fdc5926c..94fbb45cf9 100644 --- a/arch/arm/mach-stm32mp/spl.c +++ b/arch/arm/mach-stm32mp/spl.c @@ -19,6 +19,7 @@   #include <asm/arch/sys_proto.h>   #include <mach/tzc.h>   #include <linux/libfdt.h> +#include <tee/optee.h>   u32 spl_boot_device(void)   { @@ -182,6 +183,7 @@ void stm32_init_tzc_for_optee(void)   void spl_board_prepare_for_optee(void *fdt)   {       stm32_fdt_setup_mac_addr(fdt); +    optee_copy_fdt_nodes(fdt);       stm32_init_tzc_for_optee();   }

NAK the OP-TEE nodes are ADDED by the OP-TEE firmware in the unsecure device tree (when CFG_DT is activated)

before to jump to the kernel (that remove the need to have DT allignement with U-Boot SPL device tree)

=> SPL should not copy the OP-TEE nodes in next stage DT

reference in optee_os = core/arch/arm/kernel/boot.c: add_optee_dt_node()

add_optee_dt_node() <= update_external_dt() <= paged_init_primary()

It is the expected configuration for OP-TEE on STM32MP15 platform.

I agree that if a component modifies the platform, it should be responsible for updating the devicetree. Two issues though

1. Consistency

The STM32IMAGE boot path expects u-boot to add the optee nodes, while the FIP path expects OP-TEE to add the nodes. Which one do we stick to?

STM32MP platform expects U-Boot to copy the OP-TEE nodes because it is a generic U-Boot behavior when OP-TEE is supported

FIP path expects OP-TEE to add their nodes because it is a generic OP-TEE behavior

then the added nodes are copied by U-Boot in kernel DT

It is generic, flexible (OP-TEE configuration update wihout change in kernel DT or in U-Boot)

but not time optimized...

for STM32IMAGE image support, I have a issue (restriction)  = the U-Boot DT can't updated by

OP-TEE (U-Boot DT is embedded in u-boot.stm32 not available by OP-TEE)

=> I force presence of OP-TEE node in U-Boot DT for STM32IMAGE support

to allow copy to kernel DT.

But it is less generic (dependency between U-Boot device tree and OP-TEE):

it is one reason we switch to FIP support

in you use case: SPL -> OP-TEE -> kernel

I think the kernel device tree can be updated by OP-TEE (to be generic) or

the kernel DT have already the correct OP-TEE nodes to reduce the DT update time penalty,

as you are sure of the OP-TEE support / configuration for you board.

drawback: the kernel device tree need to be aligned with the OP-TEE configuration.

...

2. Memory access

I don't understand is how OP-TEE would get past the TZC. If we look at optee_os => plat-0stm32mp1/plat_tzc400:      "Early boot stage is in charge of configuring memory regions" The following memory has been set up by SPL (or TF-A for that matter):

Nonsec: c0000000->ddffffff      Sec:    de000000->dfdfffff      SHMEM:  dfe00000->dfffffff

The external DTB will be in the nonsec region, which OP-TEE allegedly can't access. So how would this get patched?

Let's set aside the above for a bit. I tried the following OP-TEE configs:

"CFG_DT=y CFG_DTB_MAX_SIZE=0x80000"

Crashes with 'unhandled pageable abort' somewhere in core/arch/arm/plat-stm32mp1/ . I don't have much more time to spend debugging this.

"CFG_DT=y CFG_DTB_MAX_SIZE=0x80000 PLATFORM_FLAVOR=157C_DK2"

This adds about 800 ms to the boot time. (power on to kernel printing the first line). I have an allowance of 1 second to get to the kernel. I cannot just add 800ms to the boot time and hope nobody notices. However, calling optee_copy_fdt_nodes() adds no measureable amount of time.

Unless OP-TEE can modify the devicetree without increasing boot time, I will not go down this path. This is an engineering decision driven by the product requirements.

That being said, I request that you reconsider your NAK based on this new data.

in my previous answer I think on generic support and you have specific boot time requirement.

but after cross-check with TF-A boot in FIP mode,

in TF-A set the correct firewall configuration at the end of BL2 (it is not done in OP-TEE as I expect)

based on configuration description found in FIP.

To be coherent with you configuration (SPL->OP-TEE->kernel),

this firewall configuration should be done by SPL before to start OP-TEE.

And to avoid to lost time in device tree parsing the DT modification or copy of node should be avoid:

the OP-TEE reserved should be already present in your kernel device tree => no more need to copied them

I dig again with OPTEE features support in SPL and to avoid dependancy issue I pushed a serie:

"lib: optee: remove the duplicate CONFIG_OPTEE"

http://patchwork.ozlabs.org/project/uboot/list/?series=260701&state=*

=> in your use case and for reduce the boot time

- activate CONFIG_SPL_OPTEE_IMAGE : load OP-TEE image by SPL

- manage OPTEE / firewall configuration with your defconfig

(=> no more need of OP-TEE nodes in SPL DT / reduced SPL size and parsing time)

by using the existing configurations in lib/optee/Kconfig instead DT information:

=> CONFIG_OPTEE_LOAD_ADDR

=> CONFIG_OPTEE_TZDRAM_SIZE

=> CONFIG_OPTEE_TZDRAM_BASE

PS: need to add a configuration for OPTEE SHMEM_SIZE ?

- manually add OP-TEE nodes / reserved memory in the kernel (and U-Boot)

device tree and OP-TEE config CFG_DT is not activated (update_external_dt not used)

=> OP-TEE don't modify the devicetre (as nodes are already present in NS DT)

=> in spl_board_prepare_for_optee, avoid to update device tree to lost time

in all the case : optee_copy_fdt_nodes in not done by SPL but by U-Boot started

by OP-TEE (for dynamic support)  or not done (no static configuration)

The only reason to copy the OP-TEE node for FIP support or for your use case is

to copy the nodes dynamically added by OP-TEE.

SPL / TFA -> OP-TEE -> U-Boot -> Kernel

(1)           (2)              (3)

OP-TEE only the U-Boot DT (non secure DT) in boot parameter

(1) => OP-TEE add the node in U-Boot DT

(2) => U-Boot load the kernel DT / copied the OP-TEE in kernel DT

(3) => kernel use the DT with correct OP-TEE node

The OP-TEE node copy is not necessary if the kernel start with the U-Boot DT

this sequence allow to have dynamic support of OP-TEE configuration.

But for product with fixed configuration it is not required.

I hope it is more clear.

one question:

why spl_fixup_fdt isn't called (for generic fixup = arch_fixup_fdt) for OP-TEE boot

before to call spl_board_prepare_for_optee ?

Alex

Patrick