[PATCH 0/6] Move DRAM address of ATF

[PATCH 0/6] Move DRAM address of ATF

[Andrew Davis]

Hello all,

Explanation for this series is mostly in [4/6]. First 3
patches should be safe to take independent of the last 3.

Thanks,
Andrew

Andrew Davis (6):
  arm: mach-k3: Add default ATF location for AM62/AM62a
  arm: mach-k3: Add config option for setting OP-TEE address
  arm: mach-k3: am62: Enable OF_SYSTEM_SETUP for all boards
  arm: mach-k3: am62: Fixup TF-A/OP-TEE reserved-memory node in FDT
  arm: mach-k3: am62a: Fixup TF-A/OP-TEE reserved-memory node in FDT
  arm: mach-k3: Move DRAM address of ATF for AM62/AM62a

 arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi  |  4 +-
 arch/arm/dts/k3-am65-iot2050-boot-image.dtsi  |  4 +-
 arch/arm/dts/k3-binman.dtsi                   |  8 +--
 .../dts/k3-j721e-beagleboneai64-u-boot.dtsi   |  4 +-
 arch/arm/mach-k3/Kconfig                      | 10 +++-
 arch/arm/mach-k3/Makefile                     |  1 +
 arch/arm/mach-k3/am625_fdt.c                  |  2 +
 arch/arm/mach-k3/am62a7_fdt.c                 | 16 ++++++
 arch/arm/mach-k3/am62ax/Kconfig               |  1 +
 arch/arm/mach-k3/am62x/Kconfig                |  3 ++
 arch/arm/mach-k3/common_fdt.c                 | 52 +++++++++++++++++++
 arch/arm/mach-k3/common_fdt.h                 |  2 +
 configs/am62ax_evm_a53_defconfig              |  1 -
 configs/am62x_beagleplay_a53_defconfig        |  1 -
 configs/am62x_evm_a53_defconfig               |  1 -
 configs/phycore_am62x_a53_defconfig           |  1 -
 configs/verdin-am62_a53_defconfig             |  2 -
 17 files changed, 96 insertions(+), 17 deletions(-)
 create mode 100644 arch/arm/mach-k3/am62a7_fdt.c

-- 
2.39.2

[PATCH 1/6] arm: mach-k3: Add default ATF location for AM62/AM62a

[Andrew Davis]

There is a default ATF load address that is used for devices that have
ATF running in SRAM. For AM62 and AM62a, ATF runs from DRAM. Instead
of having to override the address in every defconfig, make add a
default for these ATF in DRAM devices.

Signed-off-by: Andrew Davis <afd@ti.com>
---
 arch/arm/mach-k3/Kconfig               | 5 +++--
 configs/am62ax_evm_a53_defconfig       | 1 -
 configs/am62x_beagleplay_a53_defconfig | 1 -
 configs/am62x_evm_a53_defconfig        | 1 -
 configs/phycore_am62x_a53_defconfig    | 1 -
 configs/verdin-am62_a53_defconfig      | 1 -
 6 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/arch/arm/mach-k3/Kconfig b/arch/arm/mach-k3/Kconfig
index 03898424c95..0bd3f9fa12d 100644
--- a/arch/arm/mach-k3/Kconfig
+++ b/arch/arm/mach-k3/Kconfig
@@ -123,10 +123,11 @@ config SYS_K3_SPL_ATF
 
 config K3_ATF_LOAD_ADDR
 	hex "Load address of ATF image"
+	default 0x9e780000 if (SOC_K3_AM625 || SOC_K3_AM62A7)
 	default 0x70000000
 	help
-	  The load address for the ATF image. This value defaults to 0x70000000
-	  if not provided in the board defconfig file.
+	  The load address for the ATF image. This value is used to build the
+	  FIT image header that places ATF in memory where it will run.
 
 config K3_DM_FW
 	bool "Separate DM firmware image"
diff --git a/configs/am62ax_evm_a53_defconfig b/configs/am62ax_evm_a53_defconfig
index 38083586a3e..03b2dea7d51 100644
--- a/configs/am62ax_evm_a53_defconfig
+++ b/configs/am62ax_evm_a53_defconfig
@@ -5,7 +5,6 @@ CONFIG_SPL_LIBCOMMON_SUPPORT=y
 CONFIG_SPL_LIBGENERIC_SUPPORT=y
 CONFIG_NR_DRAM_BANKS=2
 CONFIG_SOC_K3_AM62A7=y
-CONFIG_K3_ATF_LOAD_ADDR=0x9e780000
 CONFIG_TARGET_AM62A7_A53_EVM=y
 CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
 CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x80480000
diff --git a/configs/am62x_beagleplay_a53_defconfig b/configs/am62x_beagleplay_a53_defconfig
index 0be20045a97..122c1ba9299 100644
--- a/configs/am62x_beagleplay_a53_defconfig
+++ b/configs/am62x_beagleplay_a53_defconfig
@@ -6,7 +6,6 @@ CONFIG_SPL_LIBCOMMON_SUPPORT=y
 CONFIG_SPL_LIBGENERIC_SUPPORT=y
 CONFIG_NR_DRAM_BANKS=2
 CONFIG_SOC_K3_AM625=y
-CONFIG_K3_ATF_LOAD_ADDR=0x9e780000
 CONFIG_TARGET_AM625_A53_BEAGLEPLAY=y
 CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
 CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x80b80000
diff --git a/configs/am62x_evm_a53_defconfig b/configs/am62x_evm_a53_defconfig
index 457931faf21..e29df6ec523 100644
--- a/configs/am62x_evm_a53_defconfig
+++ b/configs/am62x_evm_a53_defconfig
@@ -5,7 +5,6 @@ CONFIG_SPL_LIBCOMMON_SUPPORT=y
 CONFIG_SPL_LIBGENERIC_SUPPORT=y
 CONFIG_NR_DRAM_BANKS=2
 CONFIG_SOC_K3_AM625=y
-CONFIG_K3_ATF_LOAD_ADDR=0x9e780000
 CONFIG_TARGET_AM625_A53_EVM=y
 CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
 CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x80b80000
diff --git a/configs/phycore_am62x_a53_defconfig b/configs/phycore_am62x_a53_defconfig
index 2d5d906a9d7..5ba08440268 100644
--- a/configs/phycore_am62x_a53_defconfig
+++ b/configs/phycore_am62x_a53_defconfig
@@ -5,7 +5,6 @@ CONFIG_SPL_LIBCOMMON_SUPPORT=y
 CONFIG_SPL_LIBGENERIC_SUPPORT=y
 CONFIG_NR_DRAM_BANKS=2
 CONFIG_SOC_K3_AM625=y
-CONFIG_K3_ATF_LOAD_ADDR=0x9e780000
 CONFIG_TARGET_PHYCORE_AM62X_A53=y
 CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
 CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x80b80000
diff --git a/configs/verdin-am62_a53_defconfig b/configs/verdin-am62_a53_defconfig
index 956e3a1ad75..2003a530c7d 100644
--- a/configs/verdin-am62_a53_defconfig
+++ b/configs/verdin-am62_a53_defconfig
@@ -8,7 +8,6 @@ CONFIG_SPL_LIBCOMMON_SUPPORT=y
 CONFIG_SPL_LIBGENERIC_SUPPORT=y
 CONFIG_NR_DRAM_BANKS=2
 CONFIG_SOC_K3_AM625=y
-CONFIG_K3_ATF_LOAD_ADDR=0x9e780000
 CONFIG_TARGET_VERDIN_AM62_A53=y
 CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
 CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x80b80000
-- 
2.39.2

Re: [PATCH 1/6] arm: mach-k3: Add default ATF location for AM62/AM62a

[Neha Malcom Francis]

Hi Andrew

On 14/02/24 22:00, Andrew Davis wrote:
> There is a default ATF load address that is used for devices that have
> ATF running in SRAM. For AM62 and AM62a, ATF runs from DRAM. Instead
> of having to override the address in every defconfig, make add a
> default for these ATF in DRAM devices.
> 
> Signed-off-by: Andrew Davis <afd@ti.com>
> ---
>   arch/arm/mach-k3/Kconfig               | 5 +++--
>   configs/am62ax_evm_a53_defconfig       | 1 -
>   configs/am62x_beagleplay_a53_defconfig | 1 -
>   configs/am62x_evm_a53_defconfig        | 1 -
>   configs/phycore_am62x_a53_defconfig    | 1 -
>   configs/verdin-am62_a53_defconfig      | 1 -
>   6 files changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm/mach-k3/Kconfig b/arch/arm/mach-k3/Kconfig
> index 03898424c95..0bd3f9fa12d 100644
> --- a/arch/arm/mach-k3/Kconfig
> +++ b/arch/arm/mach-k3/Kconfig
> @@ -123,10 +123,11 @@ config SYS_K3_SPL_ATF
>   
>   config K3_ATF_LOAD_ADDR
>   	hex "Load address of ATF image"
> +	default 0x9e780000 if (SOC_K3_AM625 || SOC_K3_AM62A7)
>   	default 0x70000000
>   	help
> -	  The load address for the ATF image. This value defaults to 0x70000000
> -	  if not provided in the board defconfig file.
> +	  The load address for the ATF image. This value is used to build the
> +	  FIT image header that places ATF in memory where it will run.
>   
>   config K3_DM_FW
>   	bool "Separate DM firmware image"
> diff --git a/configs/am62ax_evm_a53_defconfig b/configs/am62ax_evm_a53_defconfig
> index 38083586a3e..03b2dea7d51 100644
> --- a/configs/am62ax_evm_a53_defconfig
> +++ b/configs/am62ax_evm_a53_defconfig
> @@ -5,7 +5,6 @@ CONFIG_SPL_LIBCOMMON_SUPPORT=y
>   CONFIG_SPL_LIBGENERIC_SUPPORT=y
>   CONFIG_NR_DRAM_BANKS=2
>   CONFIG_SOC_K3_AM62A7=y
> -CONFIG_K3_ATF_LOAD_ADDR=0x9e780000
>   CONFIG_TARGET_AM62A7_A53_EVM=y
>   CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
>   CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x80480000
> diff --git a/configs/am62x_beagleplay_a53_defconfig b/configs/am62x_beagleplay_a53_defconfig
> index 0be20045a97..122c1ba9299 100644
> --- a/configs/am62x_beagleplay_a53_defconfig
> +++ b/configs/am62x_beagleplay_a53_defconfig
> @@ -6,7 +6,6 @@ CONFIG_SPL_LIBCOMMON_SUPPORT=y
>   CONFIG_SPL_LIBGENERIC_SUPPORT=y
>   CONFIG_NR_DRAM_BANKS=2
>   CONFIG_SOC_K3_AM625=y
> -CONFIG_K3_ATF_LOAD_ADDR=0x9e780000
>   CONFIG_TARGET_AM625_A53_BEAGLEPLAY=y
>   CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
>   CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x80b80000
> diff --git a/configs/am62x_evm_a53_defconfig b/configs/am62x_evm_a53_defconfig
> index 457931faf21..e29df6ec523 100644
> --- a/configs/am62x_evm_a53_defconfig
> +++ b/configs/am62x_evm_a53_defconfig
> @@ -5,7 +5,6 @@ CONFIG_SPL_LIBCOMMON_SUPPORT=y
>   CONFIG_SPL_LIBGENERIC_SUPPORT=y
>   CONFIG_NR_DRAM_BANKS=2
>   CONFIG_SOC_K3_AM625=y
> -CONFIG_K3_ATF_LOAD_ADDR=0x9e780000
>   CONFIG_TARGET_AM625_A53_EVM=y
>   CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
>   CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x80b80000
> diff --git a/configs/phycore_am62x_a53_defconfig b/configs/phycore_am62x_a53_defconfig
> index 2d5d906a9d7..5ba08440268 100644
> --- a/configs/phycore_am62x_a53_defconfig
> +++ b/configs/phycore_am62x_a53_defconfig
> @@ -5,7 +5,6 @@ CONFIG_SPL_LIBCOMMON_SUPPORT=y
>   CONFIG_SPL_LIBGENERIC_SUPPORT=y
>   CONFIG_NR_DRAM_BANKS=2
>   CONFIG_SOC_K3_AM625=y
> -CONFIG_K3_ATF_LOAD_ADDR=0x9e780000
>   CONFIG_TARGET_PHYCORE_AM62X_A53=y
>   CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
>   CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x80b80000
> diff --git a/configs/verdin-am62_a53_defconfig b/configs/verdin-am62_a53_defconfig
> index 956e3a1ad75..2003a530c7d 100644
> --- a/configs/verdin-am62_a53_defconfig
> +++ b/configs/verdin-am62_a53_defconfig
> @@ -8,7 +8,6 @@ CONFIG_SPL_LIBCOMMON_SUPPORT=y
>   CONFIG_SPL_LIBGENERIC_SUPPORT=y
>   CONFIG_NR_DRAM_BANKS=2
>   CONFIG_SOC_K3_AM625=y
> -CONFIG_K3_ATF_LOAD_ADDR=0x9e780000
>   CONFIG_TARGET_VERDIN_AM62_A53=y
>   CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
>   CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x80b80000

Reviewed-by: Neha Malcom Francis <n-francis@ti.com>

-- 
Thanking You
Neha Malcom Francis

Re: [PATCH 1/6] arm: mach-k3: Add default ATF location for AM62/AM62a

[Bryan Brattlof]

On February 14, 2024 thus sayeth Andrew Davis:
> There is a default ATF load address that is used for devices that have
> ATF running in SRAM. For AM62 and AM62a, ATF runs from DRAM. Instead
> of having to override the address in every defconfig, make add a
> default for these ATF in DRAM devices.
> 
> Signed-off-by: Andrew Davis <afd@ti.com>

Acked-by: Bryan Brattlof <bb@ti.com>

> ---
>  arch/arm/mach-k3/Kconfig               | 5 +++--
>  configs/am62ax_evm_a53_defconfig       | 1 -
>  configs/am62x_beagleplay_a53_defconfig | 1 -
>  configs/am62x_evm_a53_defconfig        | 1 -
>  configs/phycore_am62x_a53_defconfig    | 1 -
>  configs/verdin-am62_a53_defconfig      | 1 -
>  6 files changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm/mach-k3/Kconfig b/arch/arm/mach-k3/Kconfig
> index 03898424c95..0bd3f9fa12d 100644
> --- a/arch/arm/mach-k3/Kconfig
> +++ b/arch/arm/mach-k3/Kconfig
> @@ -123,10 +123,11 @@ config SYS_K3_SPL_ATF
>  
>  config K3_ATF_LOAD_ADDR
>  	hex "Load address of ATF image"
> +	default 0x9e780000 if (SOC_K3_AM625 || SOC_K3_AM62A7)

I went ahead an added this to the TODO for 62p's respin

~Bryan

Re: [PATCH 1/6] arm: mach-k3: Add default ATF location for AM62/AM62a

[Dhruva Gole]

Hi,

On Feb 14, 2024 at 10:30:04 -0600, Andrew Davis wrote:
> There is a default ATF load address that is used for devices that have
> ATF running in SRAM. For AM62 and AM62a, ATF runs from DRAM. Instead
> of having to override the address in every defconfig, make add a
> default for these ATF in DRAM devices.
> 
> Signed-off-by: Andrew Davis <afd@ti.com>
> ---
>  arch/arm/mach-k3/Kconfig               | 5 +++--
>  configs/am62ax_evm_a53_defconfig       | 1 -
>  configs/am62x_beagleplay_a53_defconfig | 1 -
>  configs/am62x_evm_a53_defconfig        | 1 -
>  configs/phycore_am62x_a53_defconfig    | 1 -
>  configs/verdin-am62_a53_defconfig      | 1 -
>  6 files changed, 3 insertions(+), 7 deletions(-)
> 

Beagleplay stops booting completely with latest U-boot so I did a little
bisect and it seems like reverting this patch helps.

I am not sure what other implications there are of reverting this so I
don't suggest immediately dropping it, however some hints around what's
missing on beagleplay vs other platforms would be helpful.

Because other platforms based off the same SoC don't seem to be
affected.

-- 
Best regards,
Dhruva Gole <d-gole@ti.com>

Re: [PATCH 1/6] arm: mach-k3: Add default ATF location for AM62/AM62a

[Nishanth Menon]

On 17:19-20240619, Dhruva Gole wrote:
> Hi,
> 
> On Feb 14, 2024 at 10:30:04 -0600, Andrew Davis wrote:
> > There is a default ATF load address that is used for devices that have
> > ATF running in SRAM. For AM62 and AM62a, ATF runs from DRAM. Instead
> > of having to override the address in every defconfig, make add a
> > default for these ATF in DRAM devices.
> > 
> > Signed-off-by: Andrew Davis <afd@ti.com>
> > ---
> >  arch/arm/mach-k3/Kconfig               | 5 +++--
> >  configs/am62ax_evm_a53_defconfig       | 1 -
> >  configs/am62x_beagleplay_a53_defconfig | 1 -
> >  configs/am62x_evm_a53_defconfig        | 1 -
> >  configs/phycore_am62x_a53_defconfig    | 1 -
> >  configs/verdin-am62_a53_defconfig      | 1 -
> >  6 files changed, 3 insertions(+), 7 deletions(-)
> > 
> 
> Beagleplay stops booting completely with latest U-boot so I did a little
> bisect and it seems like reverting this patch helps.
> 
> I am not sure what other implications there are of reverting this so I
> don't suggest immediately dropping it, however some hints around what's
> missing on beagleplay vs other platforms would be helpful.
> 
> Because other platforms based off the same SoC don't seem to be
> affected.

Grumble... I had to rediscover this in parallel as well - Thanks Dhruva.
mkimage -l tispl.bin and comparing with kernel log of reserved mem
had me completely confused.

a) Memory maps are already notorious to manage on complex heterogenous
  systems. From beagle perspective, we have no need to go and monkey
  with DT defined memory map and DT should be the default and modifying
  DT should be explicitly called out with a log (instead of done
  "transparently") - so instead of CONFIG_K3_OPTEE_LOAD_ADDR default
  being forced from u-boot, it should be made optional, where when
  defined, it can overide the dt definition or some variant of that.

b) Looks like
   https://lore.kernel.org/u-boot/20240214163009.983034-4-afd@ti.com/
   missed beagleplay? and we were in for a surprise there - i dont want
   to switch from default dts for beagleplay to something different just
   because of simplicity for users to know exactly the carveouts and
   with other s/w starting up on uC, dts is our "canonical truth".

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

Re: [PATCH 1/6] arm: mach-k3: Add default ATF location for AM62/AM62a

[Andrew Davis]

On 6/19/24 1:20 PM, Nishanth Menon wrote:
> On 17:19-20240619, Dhruva Gole wrote:
>> Hi,
>>
>> On Feb 14, 2024 at 10:30:04 -0600, Andrew Davis wrote:
>>> There is a default ATF load address that is used for devices that have
>>> ATF running in SRAM. For AM62 and AM62a, ATF runs from DRAM. Instead
>>> of having to override the address in every defconfig, make add a
>>> default for these ATF in DRAM devices.
>>>
>>> Signed-off-by: Andrew Davis <afd@ti.com>
>>> ---
>>>   arch/arm/mach-k3/Kconfig               | 5 +++--
>>>   configs/am62ax_evm_a53_defconfig       | 1 -
>>>   configs/am62x_beagleplay_a53_defconfig | 1 -
>>>   configs/am62x_evm_a53_defconfig        | 1 -
>>>   configs/phycore_am62x_a53_defconfig    | 1 -
>>>   configs/verdin-am62_a53_defconfig      | 1 -
>>>   6 files changed, 3 insertions(+), 7 deletions(-)
>>>
>>
>> Beagleplay stops booting completely with latest U-boot so I did a little
>> bisect and it seems like reverting this patch helps.
>>
>> I am not sure what other implications there are of reverting this so I
>> don't suggest immediately dropping it, however some hints around what's
>> missing on beagleplay vs other platforms would be helpful.
>>
>> Because other platforms based off the same SoC don't seem to be
>> affected.
> 
> Grumble... I had to rediscover this in parallel as well - Thanks Dhruva.
> mkimage -l tispl.bin and comparing with kernel log of reserved mem
> had me completely confused.
> 
> a) Memory maps are already notorious to manage on complex heterogenous
>    systems. From beagle perspective, we have no need to go and monkey
>    with DT defined memory map and DT should be the default and modifying

Your DT source file is wrong, ATF is at 0x8000_0000 (or any address one
wants to put in K3_ATF_LOAD_ADDR). DT doesn't "define" hardware, it "describes"
it. ATF's location is dynamic and configurable, it doesn't belong in DT.

You have two options, either go update your DT, then update it again every
time ATF moves. *OR* simply turn on OF_SYSTEM_SETUP and let U-Boot add the
correct location reserved memory node for you.

Then you can also drop out the reserved-node from the DT template file.
Only U-Boot knows where ATF is really placed in RAM, so U-Boot must add
this info to DT.

>    DT should be explicitly called out with a log (instead of done
>    "transparently") - so instead of CONFIG_K3_OPTEE_LOAD_ADDR default
>    being forced from u-boot, it should be made optional, where when
>    defined, it can overide the dt definition or some variant of that.
> 
> b) Looks like
>     https://lore.kernel.org/u-boot/20240214163009.983034-4-afd@ti.com/
>     missed beagleplay? and we were in for a surprise there - i dont want

This was the only miss, OF_SYSTEM_SETUP didn't get added to the beagleplay
config. Bryan just sent the fix for that now. I'll go look for a way to
make this common across the whole SoC family so we don't again miss any
new boards.

Andrew

>     to switch from default dts for beagleplay to something different just
>     because of simplicity for users to know exactly the carveouts and
>     with other s/w starting up on uC, dts is our "canonical truth".
> 

Re: [PATCH 1/6] arm: mach-k3: Add default ATF location for AM62/AM62a

[Wadim Egorov]

Am 19.06.24 um 22:02 schrieb Andrew Davis:
> On 6/19/24 1:20 PM, Nishanth Menon wrote:
>> On 17:19-20240619, Dhruva Gole wrote:
>>> Hi,
>>>
>>> On Feb 14, 2024 at 10:30:04 -0600, Andrew Davis wrote:
>>>> There is a default ATF load address that is used for devices that have
>>>> ATF running in SRAM. For AM62 and AM62a, ATF runs from DRAM. Instead
>>>> of having to override the address in every defconfig, make add a
>>>> default for these ATF in DRAM devices.
>>>>
>>>> Signed-off-by: Andrew Davis <afd@ti.com>
>>>> ---
>>>>   arch/arm/mach-k3/Kconfig               | 5 +++--
>>>>   configs/am62ax_evm_a53_defconfig       | 1 -
>>>>   configs/am62x_beagleplay_a53_defconfig | 1 -
>>>>   configs/am62x_evm_a53_defconfig        | 1 -
>>>>   configs/phycore_am62x_a53_defconfig    | 1 -
>>>>   configs/verdin-am62_a53_defconfig      | 1 -
>>>>   6 files changed, 3 insertions(+), 7 deletions(-)
>>>>
>>>
>>> Beagleplay stops booting completely with latest U-boot so I did a little
>>> bisect and it seems like reverting this patch helps.
>>>
>>> I am not sure what other implications there are of reverting this so I
>>> don't suggest immediately dropping it, however some hints around what's
>>> missing on beagleplay vs other platforms would be helpful.
>>>
>>> Because other platforms based off the same SoC don't seem to be
>>> affected.
>>
>> Grumble... I had to rediscover this in parallel as well - Thanks Dhruva.
>> mkimage -l tispl.bin and comparing with kernel log of reserved mem
>> had me completely confused.
>>
>> a) Memory maps are already notorious to manage on complex heterogenous
>>    systems. From beagle perspective, we have no need to go and monkey
>>    with DT defined memory map and DT should be the default and modifying
> 
> Your DT source file is wrong, ATF is at 0x8000_0000 (or any address one
> wants to put in K3_ATF_LOAD_ADDR). DT doesn't "define" hardware, it 
> "describes"
> it. ATF's location is dynamic and configurable, it doesn't belong in DT.
> 
> You have two options, either go update your DT, then update it again every
> time ATF moves. *OR* simply turn on OF_SYSTEM_SETUP and let U-Boot add the
> correct location reserved memory node for you.
> 
> Then you can also drop out the reserved-node from the DT template file.
> Only U-Boot knows where ATF is really placed in RAM, so U-Boot must add
> this info to DT.

I think it is a good idea to remove the nodes from the device tree or at 
least mark them as "templates" and note that they are actually provided 
by the bootloader to make it less confusing.

> 
>>    DT should be explicitly called out with a log (instead of done
>>    "transparently") - so instead of CONFIG_K3_OPTEE_LOAD_ADDR default
>>    being forced from u-boot, it should be made optional, where when
>>    defined, it can overide the dt definition or some variant of that.
>>
>> b) Looks like
>>     https://lore.kernel.org/u-boot/20240214163009.983034-4-afd@ti.com/
>>     missed beagleplay? and we were in for a surprise there - i dont want
> 
> This was the only miss, OF_SYSTEM_SETUP didn't get added to the beagleplay
> config. Bryan just sent the fix for that now. I'll go look for a way to
> make this common across the whole SoC family so we don't again miss any
> new boards.
> 
> Andrew
> 
>>     to switch from default dts for beagleplay to something different just
>>     because of simplicity for users to know exactly the carveouts and
>>     with other s/w starting up on uC, dts is our "canonical truth".
>>

Re: [PATCH 1/6] arm: mach-k3: Add default ATF location for AM62/AM62a

[Bryan Brattlof]

On June 20, 2024 thus sayeth Wadim Egorov:
> 
> 
> Am 19.06.24 um 22:02 schrieb Andrew Davis:
> > On 6/19/24 1:20 PM, Nishanth Menon wrote:
> > > On 17:19-20240619, Dhruva Gole wrote:
> > > > Hi,
> > > > 
> > > > On Feb 14, 2024 at 10:30:04 -0600, Andrew Davis wrote:
> > > > > There is a default ATF load address that is used for devices that have
> > > > > ATF running in SRAM. For AM62 and AM62a, ATF runs from DRAM. Instead
> > > > > of having to override the address in every defconfig, make add a
> > > > > default for these ATF in DRAM devices.
> > > > > 
> > > > > Signed-off-by: Andrew Davis <afd@ti.com>
> > > > > ---
> > > > >   arch/arm/mach-k3/Kconfig               | 5 +++--
> > > > >   configs/am62ax_evm_a53_defconfig       | 1 -
> > > > >   configs/am62x_beagleplay_a53_defconfig | 1 -
> > > > >   configs/am62x_evm_a53_defconfig        | 1 -
> > > > >   configs/phycore_am62x_a53_defconfig    | 1 -
> > > > >   configs/verdin-am62_a53_defconfig      | 1 -
> > > > >   6 files changed, 3 insertions(+), 7 deletions(-)
> > > > > 
> > > > 
> > > > Beagleplay stops booting completely with latest U-boot so I did a little
> > > > bisect and it seems like reverting this patch helps.
> > > > 
> > > > I am not sure what other implications there are of reverting this so I
> > > > don't suggest immediately dropping it, however some hints around what's
> > > > missing on beagleplay vs other platforms would be helpful.
> > > > 
> > > > Because other platforms based off the same SoC don't seem to be
> > > > affected.
> > > 
> > > Grumble... I had to rediscover this in parallel as well - Thanks Dhruva.
> > > mkimage -l tispl.bin and comparing with kernel log of reserved mem
> > > had me completely confused.
> > > 
> > > a) Memory maps are already notorious to manage on complex heterogenous
> > >    systems. From beagle perspective, we have no need to go and monkey
> > >    with DT defined memory map and DT should be the default and modifying
> > 
> > Your DT source file is wrong, ATF is at 0x8000_0000 (or any address one
> > wants to put in K3_ATF_LOAD_ADDR). DT doesn't "define" hardware, it
> > "describes"
> > it. ATF's location is dynamic and configurable, it doesn't belong in DT.
> > 
> > You have two options, either go update your DT, then update it again every
> > time ATF moves. *OR* simply turn on OF_SYSTEM_SETUP and let U-Boot add the
> > correct location reserved memory node for you.
> > 
> > Then you can also drop out the reserved-node from the DT template file.
> > Only U-Boot knows where ATF is really placed in RAM, so U-Boot must add
> > this info to DT.
> 
> I think it is a good idea to remove the nodes from the device tree or at
> least mark them as "templates" and note that they are actually provided by
> the bootloader to make it less confusing.
> 

I have a patch lined up for this. My hope was we could remove all memory 
nodes completely from the kernel and have U-Boot fix these up. It's 
initialize DDR knows about capacity, inline ECC, and trowing all the 
different binaries into it. 

Our memory{} node fixup isn't ready yet unfortunately so I think just the 
reserved-memory{} node can be ripped out for now.

~Bryan

>
> > 
> > >    DT should be explicitly called out with a log (instead of done
> > >    "transparently") - so instead of CONFIG_K3_OPTEE_LOAD_ADDR default
> > >    being forced from u-boot, it should be made optional, where when
> > >    defined, it can overide the dt definition or some variant of that.
> > > 
> > > b) Looks like
> > >     https://lore.kernel.org/u-boot/20240214163009.983034-4-afd@ti.com/
> > >     missed beagleplay? and we were in for a surprise there - i dont want
> > 
> > This was the only miss, OF_SYSTEM_SETUP didn't get added to the beagleplay
> > config. Bryan just sent the fix for that now. I'll go look for a way to
> > make this common across the whole SoC family so we don't again miss any
> > new boards.
> > 
> > Andrew
> > 
> > >     to switch from default dts for beagleplay to something different just
> > >     because of simplicity for users to know exactly the carveouts and
> > >     with other s/w starting up on uC, dts is our "canonical truth".
> > > 

[PATCH 2/6] arm: mach-k3: Add config option for setting OP-TEE address

[Andrew Davis]

Much like we have for ATF, OP-TEE has a standard address that we load
it too and run it from. Add a Kconfig item for this to remove some
hard-coding and allow this address to be more easily changed.

Signed-off-by: Andrew Davis <afd@ti.com>
---
 arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi     | 4 ++--
 arch/arm/dts/k3-am65-iot2050-boot-image.dtsi     | 4 ++--
 arch/arm/dts/k3-binman.dtsi                      | 8 ++++----
 arch/arm/dts/k3-j721e-beagleboneai64-u-boot.dtsi | 4 ++--
 arch/arm/mach-k3/Kconfig                         | 7 +++++++
 5 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi b/arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi
index a723caa5805..cca0f44b7d8 100644
--- a/arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi
+++ b/arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi
@@ -105,8 +105,8 @@
 					arch = "arm64";
 					compression = "none";
 					os = "tee";
-					load = <0x9e800000>;
-					entry = <0x9e800000>;
+					load = <CONFIG_K3_OPTEE_LOAD_ADDR>;
+					entry = <CONFIG_K3_OPTEE_LOAD_ADDR>;
 					tee-os {
 						filename = "tee-raw.bin";
 					};
diff --git a/arch/arm/dts/k3-am65-iot2050-boot-image.dtsi b/arch/arm/dts/k3-am65-iot2050-boot-image.dtsi
index 64318d09cf0..3a6db91e132 100644
--- a/arch/arm/dts/k3-am65-iot2050-boot-image.dtsi
+++ b/arch/arm/dts/k3-am65-iot2050-boot-image.dtsi
@@ -51,8 +51,8 @@
 					arch = "arm64";
 					compression = "none";
 					os = "tee";
-					load = <0x9e800000>;
-					entry = <0x9e800000>;
+					load = <CONFIG_K3_OPTEE_LOAD_ADDR>;
+					entry = <CONFIG_K3_OPTEE_LOAD_ADDR>;
 					tee-os {
 					};
 				};
diff --git a/arch/arm/dts/k3-binman.dtsi b/arch/arm/dts/k3-binman.dtsi
index 758c8bf6ea1..621653e9471 100644
--- a/arch/arm/dts/k3-binman.dtsi
+++ b/arch/arm/dts/k3-binman.dtsi
@@ -286,8 +286,8 @@
 					arch = "arm64";
 					compression = "none";
 					os = "tee";
-					load = <0x9e800000>;
-					entry = <0x9e800000>;
+					load = <CONFIG_K3_OPTEE_LOAD_ADDR>;
+					entry = <CONFIG_K3_OPTEE_LOAD_ADDR>;
 					ti-secure {
 						content = <&tee>;
 						keyfile = "custMpk.pem";
@@ -356,8 +356,8 @@
 					arch = "arm64";
 					compression = "none";
 					os = "tee";
-					load = <0x9e800000>;
-					entry = <0x9e800000>;
+					load = <CONFIG_K3_OPTEE_LOAD_ADDR>;
+					entry = <CONFIG_K3_OPTEE_LOAD_ADDR>;
 					tee-os {
 						filename = "tee-raw.bin";
 					};
diff --git a/arch/arm/dts/k3-j721e-beagleboneai64-u-boot.dtsi b/arch/arm/dts/k3-j721e-beagleboneai64-u-boot.dtsi
index 017a5a722e0..ca99fa0e690 100644
--- a/arch/arm/dts/k3-j721e-beagleboneai64-u-boot.dtsi
+++ b/arch/arm/dts/k3-j721e-beagleboneai64-u-boot.dtsi
@@ -250,8 +250,8 @@
 					arch = "arm64";
 					compression = "none";
 					os = "tee";
-					load = <0x9e800000>;
-					entry = <0x9e800000>;
+					load = <CONFIG_K3_OPTEE_LOAD_ADDR>;
+					entry = <CONFIG_K3_OPTEE_LOAD_ADDR>;
 					tee-os {
 						filename = "tee-raw.bin";
 					};
diff --git a/arch/arm/mach-k3/Kconfig b/arch/arm/mach-k3/Kconfig
index 0bd3f9fa12d..55bb874d9aa 100644
--- a/arch/arm/mach-k3/Kconfig
+++ b/arch/arm/mach-k3/Kconfig
@@ -129,6 +129,13 @@ config K3_ATF_LOAD_ADDR
 	  The load address for the ATF image. This value is used to build the
 	  FIT image header that places ATF in memory where it will run.
 
+config K3_OPTEE_LOAD_ADDR
+	hex "Load address of OPTEE image"
+	default 0x9e800000
+	help
+	  The load address for the OPTEE image. This value defaults to 0x9e800000
+	  if not provided in the board defconfig file.
+
 config K3_DM_FW
 	bool "Separate DM firmware image"
 	depends on CPU_V7R && (SOC_K3_J721E || SOC_K3_J721S2 || SOC_K3_AM625 || SOC_K3_AM62A7) && !CLK_TI_SCI && !TI_SCI_POWER_DOMAIN
-- 
2.39.2

Re: [PATCH 2/6] arm: mach-k3: Add config option for setting OP-TEE address

[Neha Malcom Francis]

On 14/02/24 22:00, Andrew Davis wrote:
> Much like we have for ATF, OP-TEE has a standard address that we load
> it too and run it from. Add a Kconfig item for this to remove some
> hard-coding and allow this address to be more easily changed.
> 
> Signed-off-by: Andrew Davis <afd@ti.com>
> ---
>   arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi     | 4 ++--
>   arch/arm/dts/k3-am65-iot2050-boot-image.dtsi     | 4 ++--
>   arch/arm/dts/k3-binman.dtsi                      | 8 ++++----
>   arch/arm/dts/k3-j721e-beagleboneai64-u-boot.dtsi | 4 ++--
>   arch/arm/mach-k3/Kconfig                         | 7 +++++++
>   5 files changed, 17 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi b/arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi
> index a723caa5805..cca0f44b7d8 100644
> --- a/arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi
> +++ b/arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi
> @@ -105,8 +105,8 @@
>   					arch = "arm64";
>   					compression = "none";
>   					os = "tee";
> -					load = <0x9e800000>;
> -					entry = <0x9e800000>;
> +					load = <CONFIG_K3_OPTEE_LOAD_ADDR>;
> +					entry = <CONFIG_K3_OPTEE_LOAD_ADDR>;
>   					tee-os {
>   						filename = "tee-raw.bin";
>   					};
> diff --git a/arch/arm/dts/k3-am65-iot2050-boot-image.dtsi b/arch/arm/dts/k3-am65-iot2050-boot-image.dtsi
> index 64318d09cf0..3a6db91e132 100644
> --- a/arch/arm/dts/k3-am65-iot2050-boot-image.dtsi
> +++ b/arch/arm/dts/k3-am65-iot2050-boot-image.dtsi
> @@ -51,8 +51,8 @@
>   					arch = "arm64";
>   					compression = "none";
>   					os = "tee";
> -					load = <0x9e800000>;
> -					entry = <0x9e800000>;
> +					load = <CONFIG_K3_OPTEE_LOAD_ADDR>;
> +					entry = <CONFIG_K3_OPTEE_LOAD_ADDR>;
>   					tee-os {
>   					};
>   				};
> diff --git a/arch/arm/dts/k3-binman.dtsi b/arch/arm/dts/k3-binman.dtsi
> index 758c8bf6ea1..621653e9471 100644
> --- a/arch/arm/dts/k3-binman.dtsi
> +++ b/arch/arm/dts/k3-binman.dtsi
> @@ -286,8 +286,8 @@
>   					arch = "arm64";
>   					compression = "none";
>   					os = "tee";
> -					load = <0x9e800000>;
> -					entry = <0x9e800000>;
> +					load = <CONFIG_K3_OPTEE_LOAD_ADDR>;
> +					entry = <CONFIG_K3_OPTEE_LOAD_ADDR>;
>   					ti-secure {
>   						content = <&tee>;
>   						keyfile = "custMpk.pem";
> @@ -356,8 +356,8 @@
>   					arch = "arm64";
>   					compression = "none";
>   					os = "tee";
> -					load = <0x9e800000>;
> -					entry = <0x9e800000>;
> +					load = <CONFIG_K3_OPTEE_LOAD_ADDR>;
> +					entry = <CONFIG_K3_OPTEE_LOAD_ADDR>;
>   					tee-os {
>   						filename = "tee-raw.bin";
>   					};
> diff --git a/arch/arm/dts/k3-j721e-beagleboneai64-u-boot.dtsi b/arch/arm/dts/k3-j721e-beagleboneai64-u-boot.dtsi
> index 017a5a722e0..ca99fa0e690 100644
> --- a/arch/arm/dts/k3-j721e-beagleboneai64-u-boot.dtsi
> +++ b/arch/arm/dts/k3-j721e-beagleboneai64-u-boot.dtsi
> @@ -250,8 +250,8 @@
>   					arch = "arm64";
>   					compression = "none";
>   					os = "tee";
> -					load = <0x9e800000>;
> -					entry = <0x9e800000>;
> +					load = <CONFIG_K3_OPTEE_LOAD_ADDR>;
> +					entry = <CONFIG_K3_OPTEE_LOAD_ADDR>;
>   					tee-os {
>   						filename = "tee-raw.bin";
>   					};
> diff --git a/arch/arm/mach-k3/Kconfig b/arch/arm/mach-k3/Kconfig
> index 0bd3f9fa12d..55bb874d9aa 100644
> --- a/arch/arm/mach-k3/Kconfig
> +++ b/arch/arm/mach-k3/Kconfig
> @@ -129,6 +129,13 @@ config K3_ATF_LOAD_ADDR
>   	  The load address for the ATF image. This value is used to build the
>   	  FIT image header that places ATF in memory where it will run.
>   
> +config K3_OPTEE_LOAD_ADDR
> +	hex "Load address of OPTEE image"
> +	default 0x9e800000
> +	help
> +	  The load address for the OPTEE image. This value defaults to 0x9e800000
> +	  if not provided in the board defconfig file.
> +
>   config K3_DM_FW
>   	bool "Separate DM firmware image"
>   	depends on CPU_V7R && (SOC_K3_J721E || SOC_K3_J721S2 || SOC_K3_AM625 || SOC_K3_AM62A7) && !CLK_TI_SCI && !TI_SCI_POWER_DOMAIN

Reviewed-by: Neha Malcom Francis <n-francis@ti.com>

-- 
Thanking You
Neha Malcom Francis

Re: [PATCH 2/6] arm: mach-k3: Add config option for setting OP-TEE address

[Manorit Chawdhry]

Hi Andrew,

On 10:30-20240214, Andrew Davis wrote:
> Much like we have for ATF, OP-TEE has a standard address that we load
> it too and run it from. Add a Kconfig item for this to remove some
> hard-coding and allow this address to be more easily changed.
> 
> Signed-off-by: Andrew Davis <afd@ti.com>
> ---
>  arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi     | 4 ++--
>  arch/arm/dts/k3-am65-iot2050-boot-image.dtsi     | 4 ++--
>  arch/arm/dts/k3-binman.dtsi                      | 8 ++++----
>  arch/arm/dts/k3-j721e-beagleboneai64-u-boot.dtsi | 4 ++--
>  arch/arm/mach-k3/Kconfig                         | 7 +++++++
>  5 files changed, 17 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi b/arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi
> index a723caa5805..cca0f44b7d8 100644
> --- a/arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi
> +++ b/arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi
> @@ -105,8 +105,8 @@
>  					arch = "arm64";
>  					compression = "none";
>  					os = "tee";
> -					load = <0x9e800000>;
> -					entry = <0x9e800000>;
> +					load = <CONFIG_K3_OPTEE_LOAD_ADDR>;
> +					entry = <CONFIG_K3_OPTEE_LOAD_ADDR>;
>  					tee-os {
>  						filename = "tee-raw.bin";
>  					};
> diff --git a/arch/arm/dts/k3-am65-iot2050-boot-image.dtsi b/arch/arm/dts/k3-am65-iot2050-boot-image.dtsi
> index 64318d09cf0..3a6db91e132 100644
> --- a/arch/arm/dts/k3-am65-iot2050-boot-image.dtsi
> +++ b/arch/arm/dts/k3-am65-iot2050-boot-image.dtsi
> @@ -51,8 +51,8 @@
>  					arch = "arm64";
>  					compression = "none";
>  					os = "tee";
> -					load = <0x9e800000>;
> -					entry = <0x9e800000>;
> +					load = <CONFIG_K3_OPTEE_LOAD_ADDR>;
> +					entry = <CONFIG_K3_OPTEE_LOAD_ADDR>;
>  					tee-os {
>  					};
>  				};
> diff --git a/arch/arm/dts/k3-binman.dtsi b/arch/arm/dts/k3-binman.dtsi
> index 758c8bf6ea1..621653e9471 100644
> --- a/arch/arm/dts/k3-binman.dtsi
> +++ b/arch/arm/dts/k3-binman.dtsi
> @@ -286,8 +286,8 @@
>  					arch = "arm64";
>  					compression = "none";
>  					os = "tee";
> -					load = <0x9e800000>;
> -					entry = <0x9e800000>;
> +					load = <CONFIG_K3_OPTEE_LOAD_ADDR>;
> +					entry = <CONFIG_K3_OPTEE_LOAD_ADDR>;
>  					ti-secure {
>  						content = <&tee>;
>  						keyfile = "custMpk.pem";
> @@ -356,8 +356,8 @@
>  					arch = "arm64";
>  					compression = "none";
>  					os = "tee";
> -					load = <0x9e800000>;
> -					entry = <0x9e800000>;
> +					load = <CONFIG_K3_OPTEE_LOAD_ADDR>;
> +					entry = <CONFIG_K3_OPTEE_LOAD_ADDR>;
>  					tee-os {
>  						filename = "tee-raw.bin";
>  					};
> diff --git a/arch/arm/dts/k3-j721e-beagleboneai64-u-boot.dtsi b/arch/arm/dts/k3-j721e-beagleboneai64-u-boot.dtsi
> index 017a5a722e0..ca99fa0e690 100644
> --- a/arch/arm/dts/k3-j721e-beagleboneai64-u-boot.dtsi
> +++ b/arch/arm/dts/k3-j721e-beagleboneai64-u-boot.dtsi
> @@ -250,8 +250,8 @@
>  					arch = "arm64";
>  					compression = "none";
>  					os = "tee";
> -					load = <0x9e800000>;
> -					entry = <0x9e800000>;
> +					load = <CONFIG_K3_OPTEE_LOAD_ADDR>;
> +					entry = <CONFIG_K3_OPTEE_LOAD_ADDR>;
>  					tee-os {
>  						filename = "tee-raw.bin";
>  					};
> diff --git a/arch/arm/mach-k3/Kconfig b/arch/arm/mach-k3/Kconfig
> index 0bd3f9fa12d..55bb874d9aa 100644
> --- a/arch/arm/mach-k3/Kconfig
> +++ b/arch/arm/mach-k3/Kconfig
> @@ -129,6 +129,13 @@ config K3_ATF_LOAD_ADDR
>  	  The load address for the ATF image. This value is used to build the
>  	  FIT image header that places ATF in memory where it will run.
>  
> +config K3_OPTEE_LOAD_ADDR
> +	hex "Load address of OPTEE image"
> +	default 0x9e800000
> +	help
> +	  The load address for the OPTEE image. This value defaults to 0x9e800000
> +	  if not provided in the board defconfig file.
> +

Wondering if we should call out updating the firewall configurations as
well as a part of this so people don't end up with random failures..

Regards,
Manorit

>  config K3_DM_FW
>  	bool "Separate DM firmware image"
>  	depends on CPU_V7R && (SOC_K3_J721E || SOC_K3_J721S2 || SOC_K3_AM625 || SOC_K3_AM62A7) && !CLK_TI_SCI && !TI_SCI_POWER_DOMAIN
> -- 
> 2.39.2
> 

Re: [PATCH 2/6] arm: mach-k3: Add config option for setting OP-TEE address

[Andrew Davis]

On 2/28/24 12:21 AM, Manorit Chawdhry wrote:
> Hi Andrew,
> 
> On 10:30-20240214, Andrew Davis wrote:
>> Much like we have for ATF, OP-TEE has a standard address that we load
>> it too and run it from. Add a Kconfig item for this to remove some
>> hard-coding and allow this address to be more easily changed.
>>
>> Signed-off-by: Andrew Davis <afd@ti.com>
>> ---
>>   arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi     | 4 ++--
>>   arch/arm/dts/k3-am65-iot2050-boot-image.dtsi     | 4 ++--
>>   arch/arm/dts/k3-binman.dtsi                      | 8 ++++----
>>   arch/arm/dts/k3-j721e-beagleboneai64-u-boot.dtsi | 4 ++--
>>   arch/arm/mach-k3/Kconfig                         | 7 +++++++
>>   5 files changed, 17 insertions(+), 10 deletions(-)
>>
>> diff --git a/arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi b/arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi
>> index a723caa5805..cca0f44b7d8 100644
>> --- a/arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi
>> +++ b/arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi
>> @@ -105,8 +105,8 @@
>>   					arch = "arm64";
>>   					compression = "none";
>>   					os = "tee";
>> -					load = <0x9e800000>;
>> -					entry = <0x9e800000>;
>> +					load = <CONFIG_K3_OPTEE_LOAD_ADDR>;
>> +					entry = <CONFIG_K3_OPTEE_LOAD_ADDR>;
>>   					tee-os {
>>   						filename = "tee-raw.bin";
>>   					};
>> diff --git a/arch/arm/dts/k3-am65-iot2050-boot-image.dtsi b/arch/arm/dts/k3-am65-iot2050-boot-image.dtsi
>> index 64318d09cf0..3a6db91e132 100644
>> --- a/arch/arm/dts/k3-am65-iot2050-boot-image.dtsi
>> +++ b/arch/arm/dts/k3-am65-iot2050-boot-image.dtsi
>> @@ -51,8 +51,8 @@
>>   					arch = "arm64";
>>   					compression = "none";
>>   					os = "tee";
>> -					load = <0x9e800000>;
>> -					entry = <0x9e800000>;
>> +					load = <CONFIG_K3_OPTEE_LOAD_ADDR>;
>> +					entry = <CONFIG_K3_OPTEE_LOAD_ADDR>;
>>   					tee-os {
>>   					};
>>   				};
>> diff --git a/arch/arm/dts/k3-binman.dtsi b/arch/arm/dts/k3-binman.dtsi
>> index 758c8bf6ea1..621653e9471 100644
>> --- a/arch/arm/dts/k3-binman.dtsi
>> +++ b/arch/arm/dts/k3-binman.dtsi
>> @@ -286,8 +286,8 @@
>>   					arch = "arm64";
>>   					compression = "none";
>>   					os = "tee";
>> -					load = <0x9e800000>;
>> -					entry = <0x9e800000>;
>> +					load = <CONFIG_K3_OPTEE_LOAD_ADDR>;
>> +					entry = <CONFIG_K3_OPTEE_LOAD_ADDR>;
>>   					ti-secure {
>>   						content = <&tee>;
>>   						keyfile = "custMpk.pem";
>> @@ -356,8 +356,8 @@
>>   					arch = "arm64";
>>   					compression = "none";
>>   					os = "tee";
>> -					load = <0x9e800000>;
>> -					entry = <0x9e800000>;
>> +					load = <CONFIG_K3_OPTEE_LOAD_ADDR>;
>> +					entry = <CONFIG_K3_OPTEE_LOAD_ADDR>;
>>   					tee-os {
>>   						filename = "tee-raw.bin";
>>   					};
>> diff --git a/arch/arm/dts/k3-j721e-beagleboneai64-u-boot.dtsi b/arch/arm/dts/k3-j721e-beagleboneai64-u-boot.dtsi
>> index 017a5a722e0..ca99fa0e690 100644
>> --- a/arch/arm/dts/k3-j721e-beagleboneai64-u-boot.dtsi
>> +++ b/arch/arm/dts/k3-j721e-beagleboneai64-u-boot.dtsi
>> @@ -250,8 +250,8 @@
>>   					arch = "arm64";
>>   					compression = "none";
>>   					os = "tee";
>> -					load = <0x9e800000>;
>> -					entry = <0x9e800000>;
>> +					load = <CONFIG_K3_OPTEE_LOAD_ADDR>;
>> +					entry = <CONFIG_K3_OPTEE_LOAD_ADDR>;
>>   					tee-os {
>>   						filename = "tee-raw.bin";
>>   					};
>> diff --git a/arch/arm/mach-k3/Kconfig b/arch/arm/mach-k3/Kconfig
>> index 0bd3f9fa12d..55bb874d9aa 100644
>> --- a/arch/arm/mach-k3/Kconfig
>> +++ b/arch/arm/mach-k3/Kconfig
>> @@ -129,6 +129,13 @@ config K3_ATF_LOAD_ADDR
>>   	  The load address for the ATF image. This value is used to build the
>>   	  FIT image header that places ATF in memory where it will run.
>>   
>> +config K3_OPTEE_LOAD_ADDR
>> +	hex "Load address of OPTEE image"
>> +	default 0x9e800000
>> +	help
>> +	  The load address for the OPTEE image. This value defaults to 0x9e800000
>> +	  if not provided in the board defconfig file.
>> +
> 
> Wondering if we should call out updating the firewall configurations as
> well as a part of this so people don't end up with random failures..
> 

Might be even better to build the firewall config based on this
new kconfig symbol. I can look into that at some later point.

Andrew

> Regards,
> Manorit
> 
>>   config K3_DM_FW
>>   	bool "Separate DM firmware image"
>>   	depends on CPU_V7R && (SOC_K3_J721E || SOC_K3_J721S2 || SOC_K3_AM625 || SOC_K3_AM62A7) && !CLK_TI_SCI && !TI_SCI_POWER_DOMAIN
>> -- 
>> 2.39.2
>>

Re: [PATCH 2/6] arm: mach-k3: Add config option for setting OP-TEE address

[Bryan Brattlof]

On February 14, 2024 thus sayeth Andrew Davis:
> Much like we have for ATF, OP-TEE has a standard address that we load
> it too and run it from. Add a Kconfig item for this to remove some
> hard-coding and allow this address to be more easily changed.
> 
> Signed-off-by: Andrew Davis <afd@ti.com>

Acked-by: Bryan Brattlof <bb@ti.com>

~Bryan

[PATCH 3/6] arm: mach-k3: am62: Enable OF_SYSTEM_SETUP for all boards

[Andrew Davis]

The fixups provided by ft_system_setup() are applicable for all AM62 based
boards. Select this at the target selection level for all AM62 boards and
remove it from any specific defconfig.

Signed-off-by: Andrew Davis <afd@ti.com>
---
 arch/arm/mach-k3/am62x/Kconfig    | 3 +++
 configs/verdin-am62_a53_defconfig | 1 -
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-k3/am62x/Kconfig b/arch/arm/mach-k3/am62x/Kconfig
index 935d596c879..7c9bac2ece8 100644
--- a/arch/arm/mach-k3/am62x/Kconfig
+++ b/arch/arm/mach-k3/am62x/Kconfig
@@ -13,6 +13,7 @@ config TARGET_AM625_A53_EVM
 	bool "TI K3 based AM625 EVM running on A53"
 	select ARM64
 	select BINMAN
+	select OF_SYSTEM_SETUP
 
 config TARGET_AM625_R5_EVM
 	bool "TI K3 based AM625 EVM running on R5"
@@ -29,6 +30,7 @@ config TARGET_PHYCORE_AM62X_A53
 	bool "PHYTEC phyCORE-AM62x running on A53"
 	select ARM64
 	select BINMAN
+	select OF_SYSTEM_SETUP
 
 config TARGET_PHYCORE_AM62X_R5
 	bool "PHYTEC phyCORE-AM62x running on R5"
@@ -45,6 +47,7 @@ config TARGET_VERDIN_AM62_A53
 	bool "Toradex Verdin AM62 running on A53"
 	select ARM64
 	select BINMAN
+	select OF_SYSTEM_SETUP
 
 config TARGET_VERDIN_AM62_R5
 	bool "Toradex Verdin AM62 running on R5"
diff --git a/configs/verdin-am62_a53_defconfig b/configs/verdin-am62_a53_defconfig
index 2003a530c7d..902c437d734 100644
--- a/configs/verdin-am62_a53_defconfig
+++ b/configs/verdin-am62_a53_defconfig
@@ -37,7 +37,6 @@ CONFIG_LEGACY_IMAGE_FORMAT=y
 CONFIG_SYS_BOOTM_LEN=0x40000000
 CONFIG_DISTRO_DEFAULTS=y
 CONFIG_BOOTDELAY=1
-CONFIG_OF_SYSTEM_SETUP=y
 CONFIG_USE_PREBOOT=y
 CONFIG_PREBOOT="test -n ${fdtfile} || setenv fdtfile k3-am625-verdin-${variant}-${fdt_board}.dtb"
 CONFIG_LOG=y
-- 
2.39.2

[PATCH 4/6] arm: mach-k3: am62: Fixup TF-A/OP-TEE reserved-memory node in FDT

[Andrew Davis]

The address we load TF-A and OP-TEE to is configurable by Kconfig
CONFIG_K3_{ATF,OPTEE}_LOAD_ADDR, but the DT nodes reserving this memory
are often statically defined. As these binaries are dynamically loadable,
and in the case of OP-TEE may not even be loaded at all, hard-coding these
addresses is not a hardware description, but rather a configuration.

If the address that U-Boot loaded TF-A or OP-TEE does not match the
address in hard-coded in DT, then fix that node address. This also handles
the case when no reserved memory for these is provided by DT, which is
more correct as explained above.

Add this fixup function, and enable it for AM62.

Signed-off-by: Andrew Davis <afd@ti.com>
---
 arch/arm/mach-k3/am625_fdt.c  |  2 ++
 arch/arm/mach-k3/common_fdt.c | 52 +++++++++++++++++++++++++++++++++++
 arch/arm/mach-k3/common_fdt.h |  2 ++
 3 files changed, 56 insertions(+)

diff --git a/arch/arm/mach-k3/am625_fdt.c b/arch/arm/mach-k3/am625_fdt.c
index 970dd3447de..b26186456f3 100644
--- a/arch/arm/mach-k3/am625_fdt.c
+++ b/arch/arm/mach-k3/am625_fdt.c
@@ -43,6 +43,8 @@ int ft_system_setup(void *blob, struct bd_info *bd)
 	fdt_fixup_cores_nodes_am625(blob, k3_get_core_nr());
 	fdt_fixup_gpu_nodes_am625(blob, k3_has_gpu());
 	fdt_fixup_pru_node_am625(blob, k3_has_pru());
+	fdt_fixup_reserved(blob, "tfa", CONFIG_K3_ATF_LOAD_ADDR, 0x80000);
+	fdt_fixup_reserved(blob, "optee", CONFIG_K3_OPTEE_LOAD_ADDR, 0x1800000);
 
 	return 0;
 }
diff --git a/arch/arm/mach-k3/common_fdt.c b/arch/arm/mach-k3/common_fdt.c
index 645c4de42f7..3bdedd7b509 100644
--- a/arch/arm/mach-k3/common_fdt.c
+++ b/arch/arm/mach-k3/common_fdt.c
@@ -112,3 +112,55 @@ int fdt_del_node_path(void *blob, const char *path)
 
 	return ret;
 }
+
+int fdt_fixup_reserved(void *blob, const char *name,
+		       unsigned int new_address, unsigned int new_size)
+{
+	int nodeoffset, subnode;
+	int ret;
+
+	/* Find reserved-memory */
+	nodeoffset = fdt_subnode_offset(blob, 0, "reserved-memory");
+	if (nodeoffset < 0) {
+		debug("Could not find reserved-memory node\n");
+		return 0;
+	}
+
+	/* Find existing matching subnode and remove it */
+	fdt_for_each_subnode(subnode, blob, nodeoffset) {
+		const char *node_name;
+		fdt_addr_t addr;
+		fdt_size_t size;
+
+		/* Name matching */
+		node_name = fdt_get_name(blob, subnode, NULL);
+		if (!name)
+			return -EINVAL;
+		if (!strncmp(node_name, name, strlen(name))) {
+			/* Read out old size first */
+			addr = fdtdec_get_addr_size(blob, subnode, "reg", &size);
+			if (addr == FDT_ADDR_T_NONE)
+				return -EINVAL;
+			new_size = size;
+
+			/* Delete node */
+			ret = fdt_del_node(blob, subnode);
+			if (ret < 0)
+				return ret;
+
+			/* Only one matching node */
+			break;
+		}
+	}
+
+	struct fdt_memory carveout = {
+		.start = new_address,
+		.end = new_address + new_size - 1,
+	};
+	ret = fdtdec_add_reserved_memory(blob, name, &carveout, NULL, 0, NULL,
+					 FDTDEC_RESERVED_MEMORY_NO_MAP);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
diff --git a/arch/arm/mach-k3/common_fdt.h b/arch/arm/mach-k3/common_fdt.h
index 4d23ae638ca..52c07957483 100644
--- a/arch/arm/mach-k3/common_fdt.h
+++ b/arch/arm/mach-k3/common_fdt.h
@@ -8,5 +8,7 @@
 
 int fdt_fixup_msmc_ram_k3(void *blob);
 int fdt_del_node_path(void *blob, const char *path);
+int fdt_fixup_reserved(void *blob, const char *name,
+		       unsigned int new_address, unsigned int new_size);
 
 #endif /* _COMMON_FDT_H */
-- 
2.39.2

Re: [PATCH 4/6] arm: mach-k3: am62: Fixup TF-A/OP-TEE reserved-memory node in FDT

[Neha Malcom Francis]

Hi Andrew

On 14/02/24 22:00, Andrew Davis wrote:
> The address we load TF-A and OP-TEE to is configurable by Kconfig
> CONFIG_K3_{ATF,OPTEE}_LOAD_ADDR, but the DT nodes reserving this memory
> are often statically defined. As these binaries are dynamically loadable,
> and in the case of OP-TEE may not even be loaded at all, hard-coding these
> addresses is not a hardware description, but rather a configuration.
> 
> If the address that U-Boot loaded TF-A or OP-TEE does not match the
> address in hard-coded in DT, then fix that node address. This also handles
> the case when no reserved memory for these is provided by DT, which is
> more correct as explained above.
> 
> Add this fixup function, and enable it for AM62.
> 
> Signed-off-by: Andrew Davis <afd@ti.com>
> ---
>   arch/arm/mach-k3/am625_fdt.c  |  2 ++
>   arch/arm/mach-k3/common_fdt.c | 52 +++++++++++++++++++++++++++++++++++
>   arch/arm/mach-k3/common_fdt.h |  2 ++
>   3 files changed, 56 insertions(+)
> 
> diff --git a/arch/arm/mach-k3/am625_fdt.c b/arch/arm/mach-k3/am625_fdt.c
> index 970dd3447de..b26186456f3 100644
> --- a/arch/arm/mach-k3/am625_fdt.c
> +++ b/arch/arm/mach-k3/am625_fdt.c
> @@ -43,6 +43,8 @@ int ft_system_setup(void *blob, struct bd_info *bd)
>   	fdt_fixup_cores_nodes_am625(blob, k3_get_core_nr());
>   	fdt_fixup_gpu_nodes_am625(blob, k3_has_gpu());
>   	fdt_fixup_pru_node_am625(blob, k3_has_pru());
> +	fdt_fixup_reserved(blob, "tfa", CONFIG_K3_ATF_LOAD_ADDR, 0x80000);
> +	fdt_fixup_reserved(blob, "optee", CONFIG_K3_OPTEE_LOAD_ADDR, 0x1800000);
>   
>   	return 0;
>   }
> diff --git a/arch/arm/mach-k3/common_fdt.c b/arch/arm/mach-k3/common_fdt.c
> index 645c4de42f7..3bdedd7b509 100644
> --- a/arch/arm/mach-k3/common_fdt.c
> +++ b/arch/arm/mach-k3/common_fdt.c
> @@ -112,3 +112,55 @@ int fdt_del_node_path(void *blob, const char *path)
>   
>   	return ret;
>   }
> +
> +int fdt_fixup_reserved(void *blob, const char *name,
> +		       unsigned int new_address, unsigned int new_size)
> +{
> +	int nodeoffset, subnode;
> +	int ret;
> +
> +	/* Find reserved-memory */
> +	nodeoffset = fdt_subnode_offset(blob, 0, "reserved-memory");
> +	if (nodeoffset < 0) {
> +		debug("Could not find reserved-memory node\n");
> +		return 0;
> +	}
> +
> +	/* Find existing matching subnode and remove it */
> +	fdt_for_each_subnode(subnode, blob, nodeoffset) {
> +		const char *node_name;
> +		fdt_addr_t addr;
> +		fdt_size_t size;
> +
> +		/* Name matching */
> +		node_name = fdt_get_name(blob, subnode, NULL);
> +		if (!name)
> +			return -EINVAL;
> +		if (!strncmp(node_name, name, strlen(name))) {
> +			/* Read out old size first */
> +			addr = fdtdec_get_addr_size(blob, subnode, "reg", &size);
> +			if (addr == FDT_ADDR_T_NONE)
> +				return -EINVAL;
> +			new_size = size;
> +
> +			/* Delete node */
> +			ret = fdt_del_node(blob, subnode);
> +			if (ret < 0)
> +				return ret;
> +
> +			/* Only one matching node */
> +			break;
> +		}
> +	}
> +
> +	struct fdt_memory carveout = {
> +		.start = new_address,
> +		.end = new_address + new_size - 1,
> +	};
> +	ret = fdtdec_add_reserved_memory(blob, name, &carveout, NULL, 0, NULL,
> +					 FDTDEC_RESERVED_MEMORY_NO_MAP);
> +	if (ret < 0)
> +		return ret;
> +
> +	return 0;
> +}
> diff --git a/arch/arm/mach-k3/common_fdt.h b/arch/arm/mach-k3/common_fdt.h
> index 4d23ae638ca..52c07957483 100644
> --- a/arch/arm/mach-k3/common_fdt.h
> +++ b/arch/arm/mach-k3/common_fdt.h
> @@ -8,5 +8,7 @@
>   
>   int fdt_fixup_msmc_ram_k3(void *blob);
>   int fdt_del_node_path(void *blob, const char *path);
> +int fdt_fixup_reserved(void *blob, const char *name,
> +		       unsigned int new_address, unsigned int new_size);
>   
>   #endif /* _COMMON_FDT_H */

This looks pretty neat! Thanks!

Reviewed-by: Neha Malcom Francis <n-francis@ti.com>

-- 
Thanking You
Neha Malcom Francis

Re: [PATCH 4/6] arm: mach-k3: am62: Fixup TF-A/OP-TEE reserved-memory node in FDT

[Manorit Chawdhry]

Hi Andrew,

On 10:30-20240214, Andrew Davis wrote:
> The address we load TF-A and OP-TEE to is configurable by Kconfig
> CONFIG_K3_{ATF,OPTEE}_LOAD_ADDR, but the DT nodes reserving this memory
> are often statically defined. As these binaries are dynamically loadable,
> and in the case of OP-TEE may not even be loaded at all, hard-coding these
> addresses is not a hardware description, but rather a configuration.
> 
> If the address that U-Boot loaded TF-A or OP-TEE does not match the
> address in hard-coded in DT, then fix that node address. This also handles
> the case when no reserved memory for these is provided by DT, which is
> more correct as explained above.
> 
> Add this fixup function, and enable it for AM62.

Why are we doing it only for AM62? Assuming that this can help in other
devices as well as all the DTs ideally should have these reserved nodes
in DT?

Regards,
Manorit
> 
> Signed-off-by: Andrew Davis <afd@ti.com>
> ---
>  arch/arm/mach-k3/am625_fdt.c  |  2 ++
>  arch/arm/mach-k3/common_fdt.c | 52 +++++++++++++++++++++++++++++++++++
>  arch/arm/mach-k3/common_fdt.h |  2 ++
>  3 files changed, 56 insertions(+)
> 
> diff --git a/arch/arm/mach-k3/am625_fdt.c b/arch/arm/mach-k3/am625_fdt.c
> index 970dd3447de..b26186456f3 100644
> --- a/arch/arm/mach-k3/am625_fdt.c
> +++ b/arch/arm/mach-k3/am625_fdt.c
> @@ -43,6 +43,8 @@ int ft_system_setup(void *blob, struct bd_info *bd)
>  	fdt_fixup_cores_nodes_am625(blob, k3_get_core_nr());
>  	fdt_fixup_gpu_nodes_am625(blob, k3_has_gpu());
>  	fdt_fixup_pru_node_am625(blob, k3_has_pru());
> +	fdt_fixup_reserved(blob, "tfa", CONFIG_K3_ATF_LOAD_ADDR, 0x80000);
> +	fdt_fixup_reserved(blob, "optee", CONFIG_K3_OPTEE_LOAD_ADDR, 0x1800000);
>  
>  	return 0;
>  }
> diff --git a/arch/arm/mach-k3/common_fdt.c b/arch/arm/mach-k3/common_fdt.c
> index 645c4de42f7..3bdedd7b509 100644
> --- a/arch/arm/mach-k3/common_fdt.c
> +++ b/arch/arm/mach-k3/common_fdt.c
> @@ -112,3 +112,55 @@ int fdt_del_node_path(void *blob, const char *path)
>  
>  	return ret;
>  }
> +
> +int fdt_fixup_reserved(void *blob, const char *name,
> +		       unsigned int new_address, unsigned int new_size)
> +{
> +	int nodeoffset, subnode;
> +	int ret;
> +
> +	/* Find reserved-memory */
> +	nodeoffset = fdt_subnode_offset(blob, 0, "reserved-memory");
> +	if (nodeoffset < 0) {
> +		debug("Could not find reserved-memory node\n");
> +		return 0;
> +	}
> +
> +	/* Find existing matching subnode and remove it */
> +	fdt_for_each_subnode(subnode, blob, nodeoffset) {
> +		const char *node_name;
> +		fdt_addr_t addr;
> +		fdt_size_t size;
> +
> +		/* Name matching */
> +		node_name = fdt_get_name(blob, subnode, NULL);
> +		if (!name)
> +			return -EINVAL;
> +		if (!strncmp(node_name, name, strlen(name))) {
> +			/* Read out old size first */
> +			addr = fdtdec_get_addr_size(blob, subnode, "reg", &size);
> +			if (addr == FDT_ADDR_T_NONE)
> +				return -EINVAL;
> +			new_size = size;
> +
> +			/* Delete node */
> +			ret = fdt_del_node(blob, subnode);
> +			if (ret < 0)
> +				return ret;
> +
> +			/* Only one matching node */
> +			break;
> +		}
> +	}
> +
> +	struct fdt_memory carveout = {
> +		.start = new_address,
> +		.end = new_address + new_size - 1,
> +	};
> +	ret = fdtdec_add_reserved_memory(blob, name, &carveout, NULL, 0, NULL,
> +					 FDTDEC_RESERVED_MEMORY_NO_MAP);
> +	if (ret < 0)
> +		return ret;
> +
> +	return 0;
> +}
> diff --git a/arch/arm/mach-k3/common_fdt.h b/arch/arm/mach-k3/common_fdt.h
> index 4d23ae638ca..52c07957483 100644
> --- a/arch/arm/mach-k3/common_fdt.h
> +++ b/arch/arm/mach-k3/common_fdt.h
> @@ -8,5 +8,7 @@
>  
>  int fdt_fixup_msmc_ram_k3(void *blob);
>  int fdt_del_node_path(void *blob, const char *path);
> +int fdt_fixup_reserved(void *blob, const char *name,
> +		       unsigned int new_address, unsigned int new_size);
>  
>  #endif /* _COMMON_FDT_H */
> -- 
> 2.39.2
> 

Re: [PATCH 4/6] arm: mach-k3: am62: Fixup TF-A/OP-TEE reserved-memory node in FDT

[Andrew Davis]

On 2/28/24 12:24 AM, Manorit Chawdhry wrote:
> Hi Andrew,
> 
> On 10:30-20240214, Andrew Davis wrote:
>> The address we load TF-A and OP-TEE to is configurable by Kconfig
>> CONFIG_K3_{ATF,OPTEE}_LOAD_ADDR, but the DT nodes reserving this memory
>> are often statically defined. As these binaries are dynamically loadable,
>> and in the case of OP-TEE may not even be loaded at all, hard-coding these
>> addresses is not a hardware description, but rather a configuration.
>>
>> If the address that U-Boot loaded TF-A or OP-TEE does not match the
>> address in hard-coded in DT, then fix that node address. This also handles
>> the case when no reserved memory for these is provided by DT, which is
>> more correct as explained above.
>>
>> Add this fixup function, and enable it for AM62.
> 
> Why are we doing it only for AM62? Assuming that this can help in other
> devices as well as all the DTs ideally should have these reserved nodes
> in DT?

Only AM62(a) have TF-A in DRAM, the rest of K3 put it in SRAM which
means it wont need to be moved currently. If we ever do move TF-A to
DRAM for other devices then they could make use of this helper function
just the same.

Andrew

> 
> Regards,
> Manorit
>>
>> Signed-off-by: Andrew Davis <afd@ti.com>
>> ---
>>   arch/arm/mach-k3/am625_fdt.c  |  2 ++
>>   arch/arm/mach-k3/common_fdt.c | 52 +++++++++++++++++++++++++++++++++++
>>   arch/arm/mach-k3/common_fdt.h |  2 ++
>>   3 files changed, 56 insertions(+)
>>
>> diff --git a/arch/arm/mach-k3/am625_fdt.c b/arch/arm/mach-k3/am625_fdt.c
>> index 970dd3447de..b26186456f3 100644
>> --- a/arch/arm/mach-k3/am625_fdt.c
>> +++ b/arch/arm/mach-k3/am625_fdt.c
>> @@ -43,6 +43,8 @@ int ft_system_setup(void *blob, struct bd_info *bd)
>>   	fdt_fixup_cores_nodes_am625(blob, k3_get_core_nr());
>>   	fdt_fixup_gpu_nodes_am625(blob, k3_has_gpu());
>>   	fdt_fixup_pru_node_am625(blob, k3_has_pru());
>> +	fdt_fixup_reserved(blob, "tfa", CONFIG_K3_ATF_LOAD_ADDR, 0x80000);
>> +	fdt_fixup_reserved(blob, "optee", CONFIG_K3_OPTEE_LOAD_ADDR, 0x1800000);
>>   
>>   	return 0;
>>   }
>> diff --git a/arch/arm/mach-k3/common_fdt.c b/arch/arm/mach-k3/common_fdt.c
>> index 645c4de42f7..3bdedd7b509 100644
>> --- a/arch/arm/mach-k3/common_fdt.c
>> +++ b/arch/arm/mach-k3/common_fdt.c
>> @@ -112,3 +112,55 @@ int fdt_del_node_path(void *blob, const char *path)
>>   
>>   	return ret;
>>   }
>> +
>> +int fdt_fixup_reserved(void *blob, const char *name,
>> +		       unsigned int new_address, unsigned int new_size)
>> +{
>> +	int nodeoffset, subnode;
>> +	int ret;
>> +
>> +	/* Find reserved-memory */
>> +	nodeoffset = fdt_subnode_offset(blob, 0, "reserved-memory");
>> +	if (nodeoffset < 0) {
>> +		debug("Could not find reserved-memory node\n");
>> +		return 0;
>> +	}
>> +
>> +	/* Find existing matching subnode and remove it */
>> +	fdt_for_each_subnode(subnode, blob, nodeoffset) {
>> +		const char *node_name;
>> +		fdt_addr_t addr;
>> +		fdt_size_t size;
>> +
>> +		/* Name matching */
>> +		node_name = fdt_get_name(blob, subnode, NULL);
>> +		if (!name)
>> +			return -EINVAL;
>> +		if (!strncmp(node_name, name, strlen(name))) {
>> +			/* Read out old size first */
>> +			addr = fdtdec_get_addr_size(blob, subnode, "reg", &size);
>> +			if (addr == FDT_ADDR_T_NONE)
>> +				return -EINVAL;
>> +			new_size = size;
>> +
>> +			/* Delete node */
>> +			ret = fdt_del_node(blob, subnode);
>> +			if (ret < 0)
>> +				return ret;
>> +
>> +			/* Only one matching node */
>> +			break;
>> +		}
>> +	}
>> +
>> +	struct fdt_memory carveout = {
>> +		.start = new_address,
>> +		.end = new_address + new_size - 1,
>> +	};
>> +	ret = fdtdec_add_reserved_memory(blob, name, &carveout, NULL, 0, NULL,
>> +					 FDTDEC_RESERVED_MEMORY_NO_MAP);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	return 0;
>> +}
>> diff --git a/arch/arm/mach-k3/common_fdt.h b/arch/arm/mach-k3/common_fdt.h
>> index 4d23ae638ca..52c07957483 100644
>> --- a/arch/arm/mach-k3/common_fdt.h
>> +++ b/arch/arm/mach-k3/common_fdt.h
>> @@ -8,5 +8,7 @@
>>   
>>   int fdt_fixup_msmc_ram_k3(void *blob);
>>   int fdt_del_node_path(void *blob, const char *path);
>> +int fdt_fixup_reserved(void *blob, const char *name,
>> +		       unsigned int new_address, unsigned int new_size);
>>   
>>   #endif /* _COMMON_FDT_H */
>> -- 
>> 2.39.2
>>

Re: [PATCH 4/6] arm: mach-k3: am62: Fixup TF-A/OP-TEE reserved-memory node in FDT

[Bryan Brattlof]

On February 14, 2024 thus sayeth Andrew Davis:
> The address we load TF-A and OP-TEE to is configurable by Kconfig
> CONFIG_K3_{ATF,OPTEE}_LOAD_ADDR, but the DT nodes reserving this memory
> are often statically defined. As these binaries are dynamically loadable,
> and in the case of OP-TEE may not even be loaded at all, hard-coding these
> addresses is not a hardware description, but rather a configuration.
> 
> If the address that U-Boot loaded TF-A or OP-TEE does not match the
> address in hard-coded in DT, then fix that node address. This also handles
> the case when no reserved memory for these is provided by DT, which is
> more correct as explained above.
> 
> Add this fixup function, and enable it for AM62.
> 
> Signed-off-by: Andrew Davis <afd@ti.com>

Acked-by: Bryan Brattlof <bb@ti.com>

Does this mean we need to no-map these regions in the MMU as well? Right 
now it's just statically defined. I was looking into trying to read the 
reserved-memory{} node to see if we could dynamically build the MMU 
config but that would have to happen after this step right?

~Bryan

Re: [PATCH 4/6] arm: mach-k3: am62: Fixup TF-A/OP-TEE reserved-memory node in FDT

[Andrew Davis]

On 3/6/24 7:35 AM, Bryan Brattlof wrote:
> On February 14, 2024 thus sayeth Andrew Davis:
>> The address we load TF-A and OP-TEE to is configurable by Kconfig
>> CONFIG_K3_{ATF,OPTEE}_LOAD_ADDR, but the DT nodes reserving this memory
>> are often statically defined. As these binaries are dynamically loadable,
>> and in the case of OP-TEE may not even be loaded at all, hard-coding these
>> addresses is not a hardware description, but rather a configuration.
>>
>> If the address that U-Boot loaded TF-A or OP-TEE does not match the
>> address in hard-coded in DT, then fix that node address. This also handles
>> the case when no reserved memory for these is provided by DT, which is
>> more correct as explained above.
>>
>> Add this fixup function, and enable it for AM62.
>>
>> Signed-off-by: Andrew Davis <afd@ti.com>
> 
> Acked-by: Bryan Brattlof <bb@ti.com>
> 
> Does this mean we need to no-map these regions in the MMU as well? Right
> now it's just statically defined. I was looking into trying to read the
> reserved-memory{} node to see if we could dynamically build the MMU
> config but that would have to happen after this step right?
> 

Good question. So TF-A/OP-TEE are loaded by R5 SPL, which means by
the time A53 SPL starts they are already in place and firewalled,
which means yes we should no-map them in MMU to prevent some
speculative fetch (A53 doesn't do that, but A72 on other K3 device
might) or similar from causing a firewall exception.

The fixup in this patch is done to the Linux DT file right before
it is handed off to Linux, which as you point out happens after
we setup the U-Boot MMU.

This is more a problem in theory, but not really one in practice.
I know, I don't like that answer either, but I can't think of a
better solution outside of doing this fixup in R5 SPL to the
U-Boot DTB also. (Which might work, but seems overkill)..

Andrew

> ~Bryan

Re: [PATCH 4/6] arm: mach-k3: am62: Fixup TF-A/OP-TEE reserved-memory node in FDT

[Bryan Brattlof]

On March  6, 2024 thus sayeth Andrew Davis:
> On 3/6/24 7:35 AM, Bryan Brattlof wrote:
> > On February 14, 2024 thus sayeth Andrew Davis:
> > > The address we load TF-A and OP-TEE to is configurable by Kconfig
> > > CONFIG_K3_{ATF,OPTEE}_LOAD_ADDR, but the DT nodes reserving this memory
> > > are often statically defined. As these binaries are dynamically loadable,
> > > and in the case of OP-TEE may not even be loaded at all, hard-coding these
> > > addresses is not a hardware description, but rather a configuration.
> > > 
> > > If the address that U-Boot loaded TF-A or OP-TEE does not match the
> > > address in hard-coded in DT, then fix that node address. This also handles
> > > the case when no reserved memory for these is provided by DT, which is
> > > more correct as explained above.
> > > 
> > > Add this fixup function, and enable it for AM62.
> > > 
> > > Signed-off-by: Andrew Davis <afd@ti.com>
> > 
> > Acked-by: Bryan Brattlof <bb@ti.com>
> > 
> > Does this mean we need to no-map these regions in the MMU as well? Right
> > now it's just statically defined. I was looking into trying to read the
> > reserved-memory{} node to see if we could dynamically build the MMU
> > config but that would have to happen after this step right?
> > 
> 
> Good question. So TF-A/OP-TEE are loaded by R5 SPL, which means by
> the time A53 SPL starts they are already in place and firewalled,
> which means yes we should no-map them in MMU to prevent some
> speculative fetch (A53 doesn't do that, but A72 on other K3 device
> might) or similar from causing a firewall exception.
> 
> The fixup in this patch is done to the Linux DT file right before
> it is handed off to Linux, which as you point out happens after
> we setup the U-Boot MMU.
> 
> This is more a problem in theory, but not really one in practice.
> I know, I don't like that answer either, but I can't think of a
> better solution outside of doing this fixup in R5 SPL to the
> U-Boot DTB also. (Which might work, but seems overkill)..
> 

Ah that's a good point. Only the SoCs with A53s are using DRAM for TFA 
so we should be good for now :)

Thanks Andrew

> Andrew
> 
> > ~Bryan

[PATCH 5/6] arm: mach-k3: am62a: Fixup TF-A/OP-TEE reserved-memory node in FDT

[Andrew Davis]

The address we load TFA and OP-TEE to is configurable by
CONFIG_K3_{ATF,OPTEE}_LOAD_ADDR, but the DT nodes reserving this memory
are static. Fix that by updating this node when the loaded address
does not match the address in DT.

Signed-off-by: Andrew Davis <afd@ti.com>
---
 arch/arm/mach-k3/Makefile       |  1 +
 arch/arm/mach-k3/am62a7_fdt.c   | 16 ++++++++++++++++
 arch/arm/mach-k3/am62ax/Kconfig |  1 +
 3 files changed, 18 insertions(+)
 create mode 100644 arch/arm/mach-k3/am62a7_fdt.c

diff --git a/arch/arm/mach-k3/Makefile b/arch/arm/mach-k3/Makefile
index 42161376469..fdb442773e3 100644
--- a/arch/arm/mach-k3/Makefile
+++ b/arch/arm/mach-k3/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_SOC_K3_AM654) += am654_fdt.o
 obj-$(CONFIG_SOC_K3_J721E) += j721e_fdt.o
 obj-$(CONFIG_SOC_K3_J721S2) += j721s2_fdt.o
 obj-$(CONFIG_SOC_K3_AM625) += am625_fdt.o
+obj-$(CONFIG_SOC_K3_AM62A7) += am62a7_fdt.o
 endif
 ifeq ($(CONFIG_SPL_BUILD),y)
 obj-$(CONFIG_SOC_K3_AM654) += am654_init.o
diff --git a/arch/arm/mach-k3/am62a7_fdt.c b/arch/arm/mach-k3/am62a7_fdt.c
new file mode 100644
index 00000000000..d67f012a5dc
--- /dev/null
+++ b/arch/arm/mach-k3/am62a7_fdt.c
@@ -0,0 +1,16 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2024 Texas Instruments Incorporated - https://www.ti.com/
+ */
+
+#include <asm/hardware.h>
+#include "common_fdt.h"
+#include <fdt_support.h>
+
+int ft_system_setup(void *blob, struct bd_info *bd)
+{
+	fdt_fixup_reserved(blob, "tfa", CONFIG_K3_ATF_LOAD_ADDR, 0x80000);
+	fdt_fixup_reserved(blob, "optee", CONFIG_K3_OPTEE_LOAD_ADDR, 0x1800000);
+
+	return 0;
+}
diff --git a/arch/arm/mach-k3/am62ax/Kconfig b/arch/arm/mach-k3/am62ax/Kconfig
index c5f1ef87126..bbd5497f2ae 100644
--- a/arch/arm/mach-k3/am62ax/Kconfig
+++ b/arch/arm/mach-k3/am62ax/Kconfig
@@ -13,6 +13,7 @@ config TARGET_AM62A7_A53_EVM
 	bool "TI K3 based AM62A7 EVM running on A53"
 	select ARM64
 	select BINMAN
+	select OF_SYSTEM_SETUP
 	imply BOARD
 	imply SPL_BOARD
 	imply TI_I2C_BOARD_DETECT
-- 
2.39.2

Re: [PATCH 5/6] arm: mach-k3: am62a: Fixup TF-A/OP-TEE reserved-memory node in FDT

[Bryan Brattlof]

On February 14, 2024 thus sayeth Andrew Davis:
> The address we load TFA and OP-TEE to is configurable by
> CONFIG_K3_{ATF,OPTEE}_LOAD_ADDR, but the DT nodes reserving this memory
> are static. Fix that by updating this node when the loaded address
> does not match the address in DT.
> 
> Signed-off-by: Andrew Davis <afd@ti.com>

Acked-by: Bryan Brattlof <bb@ti.com>

~Bryan

[PATCH 6/6] arm: mach-k3: Move DRAM address of ATF for AM62/AM62a

[Andrew Davis]

The current address of TF-A in DRAM is just below the 512MB address line.
This means if the DRAM in a system is 512MB then TF-A is right at the
end of memory which is often reused, for instance U-Boot relocates itself
here. If a system has less than 512MB then that system wouldn't work at
all as TF-A would fail to load.

To avoid the issues above, move TF-A to the start of DRAM, which doesn't
change from system to system.

As TF-A is position independent, this has no dependency on TF-A. We
also fixup DT as needed when TF-A address is moved, so this change also
has no dependency on Linux and is fully forward/backward compatible.

Signed-off-by: Andrew Davis <afd@ti.com>
---
 arch/arm/mach-k3/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-k3/Kconfig b/arch/arm/mach-k3/Kconfig
index 55bb874d9aa..33f20f61f83 100644
--- a/arch/arm/mach-k3/Kconfig
+++ b/arch/arm/mach-k3/Kconfig
@@ -123,7 +123,7 @@ config SYS_K3_SPL_ATF
 
 config K3_ATF_LOAD_ADDR
 	hex "Load address of ATF image"
-	default 0x9e780000 if (SOC_K3_AM625 || SOC_K3_AM62A7)
+	default 0x80000000 if (SOC_K3_AM625 || SOC_K3_AM62A7)
 	default 0x70000000
 	help
 	  The load address for the ATF image. This value is used to build the
-- 
2.39.2

Re: [PATCH 6/6] arm: mach-k3: Move DRAM address of ATF for AM62/AM62a

[Francesco Dolcini]

Hello Andrew,
thanks for this series.

On Wed, Feb 14, 2024 at 10:30:09AM -0600, Andrew Davis wrote:
> The current address of TF-A in DRAM is just below the 512MB address line.
> This means if the DRAM in a system is 512MB then TF-A is right at the
> end of memory which is often reused, for instance U-Boot relocates itself
> here. If a system has less than 512MB then that system wouldn't work at
> all as TF-A would fail to load.

Do you expect issue with system with exactly 512MB of RAM? We have such
a board available and this is something that was not on our radar.

The way we handle this is with `verdin-am62.c:board_get_usable_ram_top()`

There is also some other reserved memory just before the 512MB limit,
not just the TF-A.

Francesco

Re: [PATCH 6/6] arm: mach-k3: Move DRAM address of ATF for AM62/AM62a

[Andrew Davis]

On 2/15/24 2:06 AM, Francesco Dolcini wrote:
> Hello Andrew,
> thanks for this series.
> 
> On Wed, Feb 14, 2024 at 10:30:09AM -0600, Andrew Davis wrote:
>> The current address of TF-A in DRAM is just below the 512MB address line.
>> This means if the DRAM in a system is 512MB then TF-A is right at the
>> end of memory which is often reused, for instance U-Boot relocates itself
>> here. If a system has less than 512MB then that system wouldn't work at
>> all as TF-A would fail to load.
> 
> Do you expect issue with system with exactly 512MB of RAM? We have such
> a board available and this is something that was not on our radar.

The issues with exactly 512MB are much less than the systems with less.
Mostly small software issues like the one you work around below with
U-Boot relocation.

> 
> The way we handle this is with `verdin-am62.c:board_get_usable_ram_top()`
> 

The point of this patch is to avoid the need for workarounds like that in
the first place.

> There is also some other reserved memory just before the 512MB limit,
> not just the TF-A.

This series prepares for also moving OP-TEE at some point (which will be
a bit more involved as it still needs to be made position independent).
The other reserved memory is mostly for remoteproc firmware which should
be made dynamic instead of hard-coding memory carve-outs for their use.

Andrew

> 
> Francesco
> 

Re: [PATCH 6/6] arm: mach-k3: Move DRAM address of ATF for AM62/AM62a

[Bryan Brattlof]

On February 14, 2024 thus sayeth Andrew Davis:
> The current address of TF-A in DRAM is just below the 512MB address line.
> This means if the DRAM in a system is 512MB then TF-A is right at the
> end of memory which is often reused, for instance U-Boot relocates itself
> here. If a system has less than 512MB then that system wouldn't work at
> all as TF-A would fail to load.
> 
> To avoid the issues above, move TF-A to the start of DRAM, which doesn't
> change from system to system.
> 
> As TF-A is position independent, this has no dependency on TF-A. We
> also fixup DT as needed when TF-A address is moved, so this change also
> has no dependency on Linux and is fully forward/backward compatible.
> 
> Signed-off-by: Andrew Davis <afd@ti.com>

Acked-by: Bryan Brattlof <bb@ti.com>

Thanks for this! This will make adding SIP support much easier.

~Bryan

> ---
>  arch/arm/mach-k3/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-k3/Kconfig b/arch/arm/mach-k3/Kconfig
> index 55bb874d9aa..33f20f61f83 100644
> --- a/arch/arm/mach-k3/Kconfig
> +++ b/arch/arm/mach-k3/Kconfig
> @@ -123,7 +123,7 @@ config SYS_K3_SPL_ATF
>  
>  config K3_ATF_LOAD_ADDR
>  	hex "Load address of ATF image"
> -	default 0x9e780000 if (SOC_K3_AM625 || SOC_K3_AM62A7)
> +	default 0x80000000 if (SOC_K3_AM625 || SOC_K3_AM62A7)
>  	default 0x70000000
>  	help
>  	  The load address for the ATF image. This value is used to build the
> -- 
> 2.39.2
> 

Re: [PATCH 0/6] Move DRAM address of ATF

[Tom Rini]

On Wed, 14 Feb 2024 10:30:03 -0600, Andrew Davis wrote:

> Explanation for this series is mostly in [4/6]. First 3
> patches should be safe to take independent of the last 3.
> 
> Thanks,
> Andrew
> 
> Andrew Davis (6):
>   arm: mach-k3: Add default ATF location for AM62/AM62a
>   arm: mach-k3: Add config option for setting OP-TEE address
>   arm: mach-k3: am62: Enable OF_SYSTEM_SETUP for all boards
>   arm: mach-k3: am62: Fixup TF-A/OP-TEE reserved-memory node in FDT
>   arm: mach-k3: am62a: Fixup TF-A/OP-TEE reserved-memory node in FDT
>   arm: mach-k3: Move DRAM address of ATF for AM62/AM62a
> 
> [...]

Applied to u-boot/next, thanks!

-- 
Tom