[PATCH v2] dm: core: Report bootph-pre-ram/sram node as pre-reloc after relocation

[PATCH v2] dm: core: Report bootph-pre-ram/sram node as pre-reloc after relocation

[Jonas Karlman]

Nodes with bootph-pre-sram/ram props are bound in multiple phases:
1. At TPL (bootph-pre-sram) or SPL (bootph-pre-ram) phase
2. At U-Boot proper pre-relocation phase
3. At U-Boot proper normal phase

However the binding and U-Boot Driver Model documentation indicate that
only nodes marked with bootph-all or bootph-some-ram should be bound in
the U-Boot proper pre-relocation phase.

Change ofnode_pre_reloc to report a node with bootph-pre-ram/sram prop
with a pre-reloc status only after U-Boot proper pre-relocation phase.
Also update the ofnode_pre_reloc documentation to closer reflect the
binding and driver model documentation.

This changes behavior of what nodes are bound in the U-Boot proper
pre-relocation phase. Change to bootph-all or add bootph-some-ram prop
to restore prior behavior.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
Changes in v2:
- Drop use of !! to convert into bool
- Update documentation for ofnode_pre_reloc
- Rewrite commit message
- Collect r-b tag

 drivers/core/ofnode.c | 2 +-
 include/dm/ofnode.h   | 8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
index 8df16e56af5c..b1e94b2d60df 100644
--- a/drivers/core/ofnode.c
+++ b/drivers/core/ofnode.c
@@ -1353,7 +1353,7 @@ bool ofnode_pre_reloc(ofnode node)
 	 */
 	if (ofnode_read_bool(node, "bootph-pre-ram") ||
 	    ofnode_read_bool(node, "bootph-pre-sram"))
-		return true;
+		return gd->flags & GD_FLG_RELOC;
 
 	if (IS_ENABLED(CONFIG_OF_TAG_MIGRATE)) {
 		/* detect and handle old tags */
diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h
index 0f38b3e736de..13700f8266d7 100644
--- a/include/dm/ofnode.h
+++ b/include/dm/ofnode.h
@@ -1198,15 +1198,15 @@ int ofnode_read_simple_size_cells(ofnode node);
  * determine if a node was bound in one of SPL/TPL stages.
  *
  * There are 4 settings currently in use
- * - bootph-some-ram: U-Boot proper pre-relocation only
+ * - bootph-some-ram: U-Boot proper pre-relocation phase
  * - bootph-all: all phases
  * Existing platforms only use it to indicate nodes needed in
  * SPL. Should probably be replaced by bootph-pre-ram for new platforms.
- * - bootph-pre-ram: SPL and U-Boot pre-relocation
- * - bootph-pre-sram: TPL and U-Boot pre-relocation
+ * - bootph-pre-ram: SPL phase
+ * - bootph-pre-sram: TPL phase
  *
  * @node: node to check
- * Return: true if node is needed in SPL/TL, false otherwise
+ * Return: true if node should be or was bound, false otherwise
  */
 bool ofnode_pre_reloc(ofnode node);
 
-- 
2.41.0

Re: [PATCH v2] dm: core: Report bootph-pre-ram/sram node as pre-reloc after relocation

[Simon Glass]

Nodes with bootph-pre-sram/ram props are bound in multiple phases:
1. At TPL (bootph-pre-sram) or SPL (bootph-pre-ram) phase
2. At U-Boot proper pre-relocation phase
3. At U-Boot proper normal phase

However the binding and U-Boot Driver Model documentation indicate that
only nodes marked with bootph-all or bootph-some-ram should be bound in
the U-Boot proper pre-relocation phase.

Change ofnode_pre_reloc to report a node with bootph-pre-ram/sram prop
with a pre-reloc status only after U-Boot proper pre-relocation phase.
Also update the ofnode_pre_reloc documentation to closer reflect the
binding and driver model documentation.

This changes behavior of what nodes are bound in the U-Boot proper
pre-relocation phase. Change to bootph-all or add bootph-some-ram prop
to restore prior behavior.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
Changes in v2:
- Drop use of !! to convert into bool
- Update documentation for ofnode_pre_reloc
- Rewrite commit message
- Collect r-b tag

 drivers/core/ofnode.c | 2 +-
 include/dm/ofnode.h   | 8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

Applied to u-boot-dm/next, thanks!

Re: [PATCH v2] dm: core: Report bootph-pre-ram/sram node as pre-reloc after relocation : regression

[Roger Quadros]

Hi,

On 21/08/2023 01:03, Jonas Karlman wrote:
> Nodes with bootph-pre-sram/ram props are bound in multiple phases:
> 1. At TPL (bootph-pre-sram) or SPL (bootph-pre-ram) phase
> 2. At U-Boot proper pre-relocation phase
> 3. At U-Boot proper normal phase
> 
> However the binding and U-Boot Driver Model documentation indicate that
> only nodes marked with bootph-all or bootph-some-ram should be bound in
> the U-Boot proper pre-relocation phase.
> 
> Change ofnode_pre_reloc to report a node with bootph-pre-ram/sram prop
> with a pre-reloc status only after U-Boot proper pre-relocation phase.
> Also update the ofnode_pre_reloc documentation to closer reflect the
> binding and driver model documentation.
> 
> This changes behavior of what nodes are bound in the U-Boot proper
> pre-relocation phase. Change to bootph-all or add bootph-some-ram prop
> to restore prior behavior.
> 
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> ---
> Changes in v2:
> - Drop use of !! to convert into bool
> - Update documentation for ofnode_pre_reloc
> - Rewrite commit message
> - Collect r-b tag

This patch breaks boot on AM642-EVM. Boot log at the end.

> 
>  drivers/core/ofnode.c | 2 +-
>  include/dm/ofnode.h   | 8 ++++----
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
> index 8df16e56af5c..b1e94b2d60df 100644
> --- a/drivers/core/ofnode.c
> +++ b/drivers/core/ofnode.c
> @@ -1353,7 +1353,7 @@ bool ofnode_pre_reloc(ofnode node)
>  	 */
>  	if (ofnode_read_bool(node, "bootph-pre-ram") ||
>  	    ofnode_read_bool(node, "bootph-pre-sram"))
> -		return true;
> +		return gd->flags & GD_FLG_RELOC;
>  
>  	if (IS_ENABLED(CONFIG_OF_TAG_MIGRATE)) {
>  		/* detect and handle old tags */
> diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h
> index 0f38b3e736de..13700f8266d7 100644
> --- a/include/dm/ofnode.h
> +++ b/include/dm/ofnode.h
> @@ -1198,15 +1198,15 @@ int ofnode_read_simple_size_cells(ofnode node);
>   * determine if a node was bound in one of SPL/TPL stages.
>   *
>   * There are 4 settings currently in use
> - * - bootph-some-ram: U-Boot proper pre-relocation only
> + * - bootph-some-ram: U-Boot proper pre-relocation phase
>   * - bootph-all: all phases
>   * Existing platforms only use it to indicate nodes needed in
>   * SPL. Should probably be replaced by bootph-pre-ram for new platforms.
> - * - bootph-pre-ram: SPL and U-Boot pre-relocation
> - * - bootph-pre-sram: TPL and U-Boot pre-relocation
> + * - bootph-pre-ram: SPL phase
> + * - bootph-pre-sram: TPL phase
>   *
>   * @node: node to check
> - * Return: true if node is needed in SPL/TL, false otherwise
> + * Return: true if node should be or was bound, false otherwise
>   */
>  bool ofnode_pre_reloc(ofnode node);
>  

---boot log---

U-Boot SPL 2023.10-rc4-00480-g9e644284ab (Sep 28 2023 - 15:53:09 +0300)
Resetting on cold boot to workaround ErrataID:i2331
Please resend tiboot3.bin in case of UART/DFU boot
resetting ...

U-Boot SPL 2023.10-rc4-00480-g9e644284ab (Sep 28 2023 - 15:53:09 +0300)
SYSFW ABI: 3.1 (firmware rev 0x0009 '9.0.5--v09.00.05 (Kool Koala)')
SPL initial stack usage: 13408 bytes
Trying to boot from MMC2
Warning: Detected image signing certificate on GP device. Skipping certificate to prevent boot failure. This will fail if the image was also encrypted
Warning: Detected image signing certificate on GP device. Skipping certificate to prevent boot failure. This will fail if the image was also encrypted
Warning: Detected image signing certificate on GP device. Skipping certificate to prevent boot failure. This will fail if the image was also encrypted
Warning: Detected image signing certificate on GP device. Skipping certificate to prevent boot failure. This will fail if the image was also encrypted
Loading Environment from MMC... *** Warning - No MMC card found, using default environment

Starting ATF on ARM64 core...

NOTICE:  BL31: v2.8(release):v2.8-226-g2fcd408bb3-dirty
NOTICE:  BL31: Built : 00:42:57, Jan 13 2023
I/TC: 
I/TC: OP-TEE version: 3.20.0 (gcc version 11.3.0 (GCC)) #1 Fri Jan 20 15:42:54 UTC 2023 aarch64
I/TC: WARNING: This OP-TEE configuration might be insecure!
I/TC: WARNING: Please check https://optee.readthedocs.io/en/latest/architecture/porting_guidelines.html
I/TC: Primary CPU initializing
I/TC: SYSFW ABI: 3.1 (firmware rev 0x0009 '9.0.5--v09.00.05 (Kool Koala)')
I/TC: HUK Initialized
I/TC: Activated SA2UL device
I/TC: Fixing SA2UL firewall owner for GP device
I/TC: Enabled firewalls for SA2UL TRNG device
I/TC: SA2UL TRNG initialized
I/TC: SA2UL Drivers initialized
I/TC: Primary CPU switching to normal world boot

U-Boot SPL 2023.10-rc4-00480-g9e644284ab (Sep 28 2023 - 15:54:37 +0300)
SYSFW ABI: 3.1 (firmware rev 0x0009 '9.0.5--v09.00.05 (Kool Koala)')
Trying to boot from MMC2
Warning: Detected image signing certificate on GP device. Skipping certificate to prevent boot failure. This will fail if the image was also encrypted
Warning: Detected image signing certificate on GP device. Skipping certificate to prevent boot failure. This will fail if the image was also encrypted

---boot log end---

we expect U-Boot banner to be printed next but nothing is printed.

-- 
cheers,
-roger

Re: [PATCH v2] dm: core: Report bootph-pre-ram/sram node as pre-reloc after relocation : regression

[Jonas Karlman]

Hi Roger,

On 2023-09-28 14:59, Roger Quadros wrote:
> Hi,
> 
> On 21/08/2023 01:03, Jonas Karlman wrote:
>> Nodes with bootph-pre-sram/ram props are bound in multiple phases:
>> 1. At TPL (bootph-pre-sram) or SPL (bootph-pre-ram) phase
>> 2. At U-Boot proper pre-relocation phase
>> 3. At U-Boot proper normal phase
>>
>> However the binding and U-Boot Driver Model documentation indicate that
>> only nodes marked with bootph-all or bootph-some-ram should be bound in
>> the U-Boot proper pre-relocation phase.
>>
>> Change ofnode_pre_reloc to report a node with bootph-pre-ram/sram prop
>> with a pre-reloc status only after U-Boot proper pre-relocation phase.
>> Also update the ofnode_pre_reloc documentation to closer reflect the
>> binding and driver model documentation.
>>
>> This changes behavior of what nodes are bound in the U-Boot proper
>> pre-relocation phase. Change to bootph-all or add bootph-some-ram prop
>> to restore prior behavior.
>>
>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>> Reviewed-by: Simon Glass <sjg@chromium.org>
>> ---
>> Changes in v2:
>> - Drop use of !! to convert into bool
>> - Update documentation for ofnode_pre_reloc
>> - Rewrite commit message
>> - Collect r-b tag
> 
> This patch breaks boot on AM642-EVM. Boot log at the end.

From what I can tell your board use a lot of bootph-pre-ram.
https://source.denx.de/u-boot/u-boot/-/blob/master/arch/arm/dts/k3-am642-evm-u-boot.dtsi

What happens if you change to bootph-all or add bootph-some-ram next to
bootph-pre-ram on devices that is also needed in U-Boot proper
pre-relocation phase in addition to SPL phase?

Regards,
Jonas

> 
>>
>>  drivers/core/ofnode.c | 2 +-
>>  include/dm/ofnode.h   | 8 ++++----
>>  2 files changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
>> index 8df16e56af5c..b1e94b2d60df 100644
>> --- a/drivers/core/ofnode.c
>> +++ b/drivers/core/ofnode.c
>> @@ -1353,7 +1353,7 @@ bool ofnode_pre_reloc(ofnode node)
>>  	 */
>>  	if (ofnode_read_bool(node, "bootph-pre-ram") ||
>>  	    ofnode_read_bool(node, "bootph-pre-sram"))
>> -		return true;
>> +		return gd->flags & GD_FLG_RELOC;
>>  
>>  	if (IS_ENABLED(CONFIG_OF_TAG_MIGRATE)) {
>>  		/* detect and handle old tags */
>> diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h
>> index 0f38b3e736de..13700f8266d7 100644
>> --- a/include/dm/ofnode.h
>> +++ b/include/dm/ofnode.h
>> @@ -1198,15 +1198,15 @@ int ofnode_read_simple_size_cells(ofnode node);
>>   * determine if a node was bound in one of SPL/TPL stages.
>>   *
>>   * There are 4 settings currently in use
>> - * - bootph-some-ram: U-Boot proper pre-relocation only
>> + * - bootph-some-ram: U-Boot proper pre-relocation phase
>>   * - bootph-all: all phases
>>   * Existing platforms only use it to indicate nodes needed in
>>   * SPL. Should probably be replaced by bootph-pre-ram for new platforms.
>> - * - bootph-pre-ram: SPL and U-Boot pre-relocation
>> - * - bootph-pre-sram: TPL and U-Boot pre-relocation
>> + * - bootph-pre-ram: SPL phase
>> + * - bootph-pre-sram: TPL phase
>>   *
>>   * @node: node to check
>> - * Return: true if node is needed in SPL/TL, false otherwise
>> + * Return: true if node should be or was bound, false otherwise
>>   */
>>  bool ofnode_pre_reloc(ofnode node);
>>  
> 
> ---boot log---
> 
> U-Boot SPL 2023.10-rc4-00480-g9e644284ab (Sep 28 2023 - 15:53:09 +0300)
> Resetting on cold boot to workaround ErrataID:i2331
> Please resend tiboot3.bin in case of UART/DFU boot
> resetting ...
> 
> U-Boot SPL 2023.10-rc4-00480-g9e644284ab (Sep 28 2023 - 15:53:09 +0300)
> SYSFW ABI: 3.1 (firmware rev 0x0009 '9.0.5--v09.00.05 (Kool Koala)')
> SPL initial stack usage: 13408 bytes
> Trying to boot from MMC2
> Warning: Detected image signing certificate on GP device. Skipping certificate to prevent boot failure. This will fail if the image was also encrypted
> Warning: Detected image signing certificate on GP device. Skipping certificate to prevent boot failure. This will fail if the image was also encrypted
> Warning: Detected image signing certificate on GP device. Skipping certificate to prevent boot failure. This will fail if the image was also encrypted
> Warning: Detected image signing certificate on GP device. Skipping certificate to prevent boot failure. This will fail if the image was also encrypted
> Loading Environment from MMC... *** Warning - No MMC card found, using default environment
> 
> Starting ATF on ARM64 core...
> 
> NOTICE:  BL31: v2.8(release):v2.8-226-g2fcd408bb3-dirty
> NOTICE:  BL31: Built : 00:42:57, Jan 13 2023
> I/TC: 
> I/TC: OP-TEE version: 3.20.0 (gcc version 11.3.0 (GCC)) #1 Fri Jan 20 15:42:54 UTC 2023 aarch64
> I/TC: WARNING: This OP-TEE configuration might be insecure!
> I/TC: WARNING: Please check https://optee.readthedocs.io/en/latest/architecture/porting_guidelines.html
> I/TC: Primary CPU initializing
> I/TC: SYSFW ABI: 3.1 (firmware rev 0x0009 '9.0.5--v09.00.05 (Kool Koala)')
> I/TC: HUK Initialized
> I/TC: Activated SA2UL device
> I/TC: Fixing SA2UL firewall owner for GP device
> I/TC: Enabled firewalls for SA2UL TRNG device
> I/TC: SA2UL TRNG initialized
> I/TC: SA2UL Drivers initialized
> I/TC: Primary CPU switching to normal world boot
> 
> U-Boot SPL 2023.10-rc4-00480-g9e644284ab (Sep 28 2023 - 15:54:37 +0300)
> SYSFW ABI: 3.1 (firmware rev 0x0009 '9.0.5--v09.00.05 (Kool Koala)')
> Trying to boot from MMC2
> Warning: Detected image signing certificate on GP device. Skipping certificate to prevent boot failure. This will fail if the image was also encrypted
> Warning: Detected image signing certificate on GP device. Skipping certificate to prevent boot failure. This will fail if the image was also encrypted
> 
> ---boot log end---
> 
> we expect U-Boot banner to be printed next but nothing is printed.
> 

Re: [PATCH v2] dm: core: Report bootph-pre-ram/sram node as pre-reloc after relocation : regression

[Roger Quadros]

On 28/09/2023 22:18, Jonas Karlman wrote:
> Hi Roger,
> 
> On 2023-09-28 14:59, Roger Quadros wrote:
>> Hi,
>>
>> On 21/08/2023 01:03, Jonas Karlman wrote:
>>> Nodes with bootph-pre-sram/ram props are bound in multiple phases:
>>> 1. At TPL (bootph-pre-sram) or SPL (bootph-pre-ram) phase
>>> 2. At U-Boot proper pre-relocation phase
>>> 3. At U-Boot proper normal phase
>>>
>>> However the binding and U-Boot Driver Model documentation indicate that
>>> only nodes marked with bootph-all or bootph-some-ram should be bound in
>>> the U-Boot proper pre-relocation phase.
>>>
>>> Change ofnode_pre_reloc to report a node with bootph-pre-ram/sram prop
>>> with a pre-reloc status only after U-Boot proper pre-relocation phase.
>>> Also update the ofnode_pre_reloc documentation to closer reflect the
>>> binding and driver model documentation.
>>>
>>> This changes behavior of what nodes are bound in the U-Boot proper
>>> pre-relocation phase. Change to bootph-all or add bootph-some-ram prop
>>> to restore prior behavior.
>>>
>>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>> ---
>>> Changes in v2:
>>> - Drop use of !! to convert into bool
>>> - Update documentation for ofnode_pre_reloc
>>> - Rewrite commit message
>>> - Collect r-b tag
>>
>> This patch breaks boot on AM642-EVM. Boot log at the end.
> 
> From what I can tell your board use a lot of bootph-pre-ram.
> https://source.denx.de/u-boot/u-boot/-/blob/master/arch/arm/dts/k3-am642-evm-u-boot.dtsi
> 
> What happens if you change to bootph-all or add bootph-some-ram next to
> bootph-pre-ram on devices that is also needed in U-Boot proper
> pre-relocation phase in addition to SPL phase?

Then it works.

BTW, AM62-SK is broken as well and I suppose most K3 TI boards would be broken.

Nishanth / Tom,

What approach to take here?
Replacing bootph-pre-ram to bootph-all in *-u-boot.dtsi would be a quick fix.
Then we need to mark nodes required only for SPL as bootph-pre-ram.

Meanwhile I will suggest to revert the $subject patch till this is sorted out
and gets a Ack from board maintainers.

cheers,
-roger

> 
> Regards,
> Jonas
> 
>>
>>>
>>>  drivers/core/ofnode.c | 2 +-
>>>  include/dm/ofnode.h   | 8 ++++----
>>>  2 files changed, 5 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
>>> index 8df16e56af5c..b1e94b2d60df 100644
>>> --- a/drivers/core/ofnode.c
>>> +++ b/drivers/core/ofnode.c
>>> @@ -1353,7 +1353,7 @@ bool ofnode_pre_reloc(ofnode node)
>>>  	 */
>>>  	if (ofnode_read_bool(node, "bootph-pre-ram") ||
>>>  	    ofnode_read_bool(node, "bootph-pre-sram"))
>>> -		return true;
>>> +		return gd->flags & GD_FLG_RELOC;
>>>  
>>>  	if (IS_ENABLED(CONFIG_OF_TAG_MIGRATE)) {
>>>  		/* detect and handle old tags */
>>> diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h
>>> index 0f38b3e736de..13700f8266d7 100644
>>> --- a/include/dm/ofnode.h
>>> +++ b/include/dm/ofnode.h
>>> @@ -1198,15 +1198,15 @@ int ofnode_read_simple_size_cells(ofnode node);
>>>   * determine if a node was bound in one of SPL/TPL stages.
>>>   *
>>>   * There are 4 settings currently in use
>>> - * - bootph-some-ram: U-Boot proper pre-relocation only
>>> + * - bootph-some-ram: U-Boot proper pre-relocation phase
>>>   * - bootph-all: all phases
>>>   * Existing platforms only use it to indicate nodes needed in
>>>   * SPL. Should probably be replaced by bootph-pre-ram for new platforms.
>>> - * - bootph-pre-ram: SPL and U-Boot pre-relocation
>>> - * - bootph-pre-sram: TPL and U-Boot pre-relocation
>>> + * - bootph-pre-ram: SPL phase
>>> + * - bootph-pre-sram: TPL phase
>>>   *
>>>   * @node: node to check
>>> - * Return: true if node is needed in SPL/TL, false otherwise
>>> + * Return: true if node should be or was bound, false otherwise
>>>   */
>>>  bool ofnode_pre_reloc(ofnode node);
>>>  
>>
>> ---boot log---
>>
>> U-Boot SPL 2023.10-rc4-00480-g9e644284ab (Sep 28 2023 - 15:53:09 +0300)
>> Resetting on cold boot to workaround ErrataID:i2331
>> Please resend tiboot3.bin in case of UART/DFU boot
>> resetting ...
>>
>> U-Boot SPL 2023.10-rc4-00480-g9e644284ab (Sep 28 2023 - 15:53:09 +0300)
>> SYSFW ABI: 3.1 (firmware rev 0x0009 '9.0.5--v09.00.05 (Kool Koala)')
>> SPL initial stack usage: 13408 bytes
>> Trying to boot from MMC2
>> Warning: Detected image signing certificate on GP device. Skipping certificate to prevent boot failure. This will fail if the image was also encrypted
>> Warning: Detected image signing certificate on GP device. Skipping certificate to prevent boot failure. This will fail if the image was also encrypted
>> Warning: Detected image signing certificate on GP device. Skipping certificate to prevent boot failure. This will fail if the image was also encrypted
>> Warning: Detected image signing certificate on GP device. Skipping certificate to prevent boot failure. This will fail if the image was also encrypted
>> Loading Environment from MMC... *** Warning - No MMC card found, using default environment
>>
>> Starting ATF on ARM64 core...
>>
>> NOTICE:  BL31: v2.8(release):v2.8-226-g2fcd408bb3-dirty
>> NOTICE:  BL31: Built : 00:42:57, Jan 13 2023
>> I/TC: 
>> I/TC: OP-TEE version: 3.20.0 (gcc version 11.3.0 (GCC)) #1 Fri Jan 20 15:42:54 UTC 2023 aarch64
>> I/TC: WARNING: This OP-TEE configuration might be insecure!
>> I/TC: WARNING: Please check https://optee.readthedocs.io/en/latest/architecture/porting_guidelines.html
>> I/TC: Primary CPU initializing
>> I/TC: SYSFW ABI: 3.1 (firmware rev 0x0009 '9.0.5--v09.00.05 (Kool Koala)')
>> I/TC: HUK Initialized
>> I/TC: Activated SA2UL device
>> I/TC: Fixing SA2UL firewall owner for GP device
>> I/TC: Enabled firewalls for SA2UL TRNG device
>> I/TC: SA2UL TRNG initialized
>> I/TC: SA2UL Drivers initialized
>> I/TC: Primary CPU switching to normal world boot
>>
>> U-Boot SPL 2023.10-rc4-00480-g9e644284ab (Sep 28 2023 - 15:54:37 +0300)
>> SYSFW ABI: 3.1 (firmware rev 0x0009 '9.0.5--v09.00.05 (Kool Koala)')
>> Trying to boot from MMC2
>> Warning: Detected image signing certificate on GP device. Skipping certificate to prevent boot failure. This will fail if the image was also encrypted
>> Warning: Detected image signing certificate on GP device. Skipping certificate to prevent boot failure. This will fail if the image was also encrypted
>>
>> ---boot log end---
>>
>> we expect U-Boot banner to be printed next but nothing is printed.
>>
> 

Re: [PATCH v2] dm: core: Report bootph-pre-ram/sram node as pre-reloc after relocation : regression

[Massimo Pegorer]

Il giorno ven 29 set 2023 alle ore 13:23 Roger Quadros
<rogerq@kernel.org> ha scritto:
>
>
>
> On 28/09/2023 22:18, Jonas Karlman wrote:
> > Hi Roger,
> >
> > On 2023-09-28 14:59, Roger Quadros wrote:
> >> Hi,
> >>
> >> On 21/08/2023 01:03, Jonas Karlman wrote:
> >>> Nodes with bootph-pre-sram/ram props are bound in multiple phases:
> >>> 1. At TPL (bootph-pre-sram) or SPL (bootph-pre-ram) phase
> >>> 2. At U-Boot proper pre-relocation phase
> >>> 3. At U-Boot proper normal phase
> >>>
> >>> However the binding and U-Boot Driver Model documentation indicate that
> >>> only nodes marked with bootph-all or bootph-some-ram should be bound in
> >>> the U-Boot proper pre-relocation phase.
> >>>
> >>> Change ofnode_pre_reloc to report a node with bootph-pre-ram/sram prop
> >>> with a pre-reloc status only after U-Boot proper pre-relocation phase.
> >>> Also update the ofnode_pre_reloc documentation to closer reflect the
> >>> binding and driver model documentation.
> >>>
> >>> This changes behavior of what nodes are bound in the U-Boot proper
> >>> pre-relocation phase. Change to bootph-all or add bootph-some-ram prop
> >>> to restore prior behavior.
> >>>
> >>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> >>> Reviewed-by: Simon Glass <sjg@chromium.org>
> >>> ---
> >>> Changes in v2:
> >>> - Drop use of !! to convert into bool
> >>> - Update documentation for ofnode_pre_reloc
> >>> - Rewrite commit message
> >>> - Collect r-b tag
> >>
> >> This patch breaks boot on AM642-EVM. Boot log at the end.
> >
> > From what I can tell your board use a lot of bootph-pre-ram.
> > https://source.denx.de/u-boot/u-boot/-/blob/master/arch/arm/dts/k3-am642-evm-u-boot.dtsi
> >
> > What happens if you change to bootph-all or add bootph-some-ram next to
> > bootph-pre-ram on devices that is also needed in U-Boot proper
> > pre-relocation phase in addition to SPL phase?
>
> Then it works.
>
> BTW, AM62-SK is broken as well and I suppose most K3 TI boards would be broken.
>
> Nishanth / Tom,
>
> What approach to take here?
> Replacing bootph-pre-ram to bootph-all in *-u-boot.dtsi would be a quick fix.

An exact quick fix is to add bootph-some-ram next to bootph-pre-ram:
this will have the same effects of bootph-pre-ram before the patch.
Instead, replacing bootph-pre-ram with bootph-all will affect also
other boot phases (e.g. TPL) if any.

Regards,
Massimo

> Then we need to mark nodes required only for SPL as bootph-pre-ram.
>
> Meanwhile I will suggest to revert the $subject patch till this is sorted out
> and gets a Ack from board maintainers.
>
> cheers,
> -roger
>
> >
> > Regards,
> > Jonas
> >
> >>
> >>>
> >>>  drivers/core/ofnode.c | 2 +-
> >>>  include/dm/ofnode.h   | 8 ++++----
> >>>  2 files changed, 5 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
> >>> index 8df16e56af5c..b1e94b2d60df 100644
> >>> --- a/drivers/core/ofnode.c
> >>> +++ b/drivers/core/ofnode.c
> >>> @@ -1353,7 +1353,7 @@ bool ofnode_pre_reloc(ofnode node)
> >>>      */
> >>>     if (ofnode_read_bool(node, "bootph-pre-ram") ||
> >>>         ofnode_read_bool(node, "bootph-pre-sram"))
> >>> -           return true;
> >>> +           return gd->flags & GD_FLG_RELOC;
> >>>
> >>>     if (IS_ENABLED(CONFIG_OF_TAG_MIGRATE)) {
> >>>             /* detect and handle old tags */
> >>> diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h
> >>> index 0f38b3e736de..13700f8266d7 100644
> >>> --- a/include/dm/ofnode.h
> >>> +++ b/include/dm/ofnode.h
> >>> @@ -1198,15 +1198,15 @@ int ofnode_read_simple_size_cells(ofnode node);
> >>>   * determine if a node was bound in one of SPL/TPL stages.
> >>>   *
> >>>   * There are 4 settings currently in use
> >>> - * - bootph-some-ram: U-Boot proper pre-relocation only
> >>> + * - bootph-some-ram: U-Boot proper pre-relocation phase
> >>>   * - bootph-all: all phases
> >>>   * Existing platforms only use it to indicate nodes needed in
> >>>   * SPL. Should probably be replaced by bootph-pre-ram for new platforms.
> >>> - * - bootph-pre-ram: SPL and U-Boot pre-relocation
> >>> - * - bootph-pre-sram: TPL and U-Boot pre-relocation
> >>> + * - bootph-pre-ram: SPL phase
> >>> + * - bootph-pre-sram: TPL phase
> >>>   *
> >>>   * @node: node to check
> >>> - * Return: true if node is needed in SPL/TL, false otherwise
> >>> + * Return: true if node should be or was bound, false otherwise
> >>>   */
> >>>  bool ofnode_pre_reloc(ofnode node);
> >>>
> >>
> >> ---boot log---
> >>
> >> U-Boot SPL 2023.10-rc4-00480-g9e644284ab (Sep 28 2023 - 15:53:09 +0300)
> >> Resetting on cold boot to workaround ErrataID:i2331
> >> Please resend tiboot3.bin in case of UART/DFU boot
> >> resetting ...
> >>
> >> U-Boot SPL 2023.10-rc4-00480-g9e644284ab (Sep 28 2023 - 15:53:09 +0300)
> >> SYSFW ABI: 3.1 (firmware rev 0x0009 '9.0.5--v09.00.05 (Kool Koala)')
> >> SPL initial stack usage: 13408 bytes
> >> Trying to boot from MMC2
> >> Warning: Detected image signing certificate on GP device. Skipping certificate to prevent boot failure. This will fail if the image was also encrypted
> >> Warning: Detected image signing certificate on GP device. Skipping certificate to prevent boot failure. This will fail if the image was also encrypted
> >> Warning: Detected image signing certificate on GP device. Skipping certificate to prevent boot failure. This will fail if the image was also encrypted
> >> Warning: Detected image signing certificate on GP device. Skipping certificate to prevent boot failure. This will fail if the image was also encrypted
> >> Loading Environment from MMC... *** Warning - No MMC card found, using default environment
> >>
> >> Starting ATF on ARM64 core...
> >>
> >> NOTICE:  BL31: v2.8(release):v2.8-226-g2fcd408bb3-dirty
> >> NOTICE:  BL31: Built : 00:42:57, Jan 13 2023
> >> I/TC:
> >> I/TC: OP-TEE version: 3.20.0 (gcc version 11.3.0 (GCC)) #1 Fri Jan 20 15:42:54 UTC 2023 aarch64
> >> I/TC: WARNING: This OP-TEE configuration might be insecure!
> >> I/TC: WARNING: Please check https://optee.readthedocs.io/en/latest/architecture/porting_guidelines.html
> >> I/TC: Primary CPU initializing
> >> I/TC: SYSFW ABI: 3.1 (firmware rev 0x0009 '9.0.5--v09.00.05 (Kool Koala)')
> >> I/TC: HUK Initialized
> >> I/TC: Activated SA2UL device
> >> I/TC: Fixing SA2UL firewall owner for GP device
> >> I/TC: Enabled firewalls for SA2UL TRNG device
> >> I/TC: SA2UL TRNG initialized
> >> I/TC: SA2UL Drivers initialized
> >> I/TC: Primary CPU switching to normal world boot
> >>
> >> U-Boot SPL 2023.10-rc4-00480-g9e644284ab (Sep 28 2023 - 15:54:37 +0300)
> >> SYSFW ABI: 3.1 (firmware rev 0x0009 '9.0.5--v09.00.05 (Kool Koala)')
> >> Trying to boot from MMC2
> >> Warning: Detected image signing certificate on GP device. Skipping certificate to prevent boot failure. This will fail if the image was also encrypted
> >> Warning: Detected image signing certificate on GP device. Skipping certificate to prevent boot failure. This will fail if the image was also encrypted
> >>
> >> ---boot log end---
> >>
> >> we expect U-Boot banner to be printed next but nothing is printed.
> >>
> >
>

Re: [PATCH v2] dm: core: Report bootph-pre-ram/sram node as pre-reloc after relocation : regression

[Simon Glass]

Hi Massimo,

On Sun, 1 Oct 2023 at 07:29, Massimo Pegorer
<massimo.pegorer+oss@gmail.com> wrote:
>
> Il giorno ven 29 set 2023 alle ore 13:23 Roger Quadros
> <rogerq@kernel.org> ha scritto:
> >
> >
> >
> > On 28/09/2023 22:18, Jonas Karlman wrote:
> > > Hi Roger,
> > >
> > > On 2023-09-28 14:59, Roger Quadros wrote:
> > >> Hi,
> > >>
> > >> On 21/08/2023 01:03, Jonas Karlman wrote:
> > >>> Nodes with bootph-pre-sram/ram props are bound in multiple phases:
> > >>> 1. At TPL (bootph-pre-sram) or SPL (bootph-pre-ram) phase
> > >>> 2. At U-Boot proper pre-relocation phase
> > >>> 3. At U-Boot proper normal phase
> > >>>
> > >>> However the binding and U-Boot Driver Model documentation indicate that
> > >>> only nodes marked with bootph-all or bootph-some-ram should be bound in
> > >>> the U-Boot proper pre-relocation phase.
> > >>>
> > >>> Change ofnode_pre_reloc to report a node with bootph-pre-ram/sram prop
> > >>> with a pre-reloc status only after U-Boot proper pre-relocation phase.
> > >>> Also update the ofnode_pre_reloc documentation to closer reflect the
> > >>> binding and driver model documentation.
> > >>>
> > >>> This changes behavior of what nodes are bound in the U-Boot proper
> > >>> pre-relocation phase. Change to bootph-all or add bootph-some-ram prop
> > >>> to restore prior behavior.
> > >>>
> > >>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> > >>> Reviewed-by: Simon Glass <sjg@chromium.org>
> > >>> ---
> > >>> Changes in v2:
> > >>> - Drop use of !! to convert into bool
> > >>> - Update documentation for ofnode_pre_reloc
> > >>> - Rewrite commit message
> > >>> - Collect r-b tag
> > >>
> > >> This patch breaks boot on AM642-EVM. Boot log at the end.
> > >
> > > From what I can tell your board use a lot of bootph-pre-ram.
> > > https://source.denx.de/u-boot/u-boot/-/blob/master/arch/arm/dts/k3-am642-evm-u-boot.dtsi
> > >
> > > What happens if you change to bootph-all or add bootph-some-ram next to
> > > bootph-pre-ram on devices that is also needed in U-Boot proper
> > > pre-relocation phase in addition to SPL phase?
> >
> > Then it works.
> >
> > BTW, AM62-SK is broken as well and I suppose most K3 TI boards would be broken.
> >
> > Nishanth / Tom,
> >
> > What approach to take here?
> > Replacing bootph-pre-ram to bootph-all in *-u-boot.dtsi would be a quick fix.
>
> An exact quick fix is to add bootph-some-ram next to bootph-pre-ram:
> this will have the same effects of bootph-pre-ram before the patch.
> Instead, replacing bootph-pre-ram with bootph-all will affect also
> other boot phases (e.g. TPL) if any.

Yes.

Since this is in -next we have plenty of time to get things figured
out before it ends up in a release.

Regards,
Simon


>
> Regards,
> Massimo
>
> > Then we need to mark nodes required only for SPL as bootph-pre-ram.
> >
> > Meanwhile I will suggest to revert the $subject patch till this is sorted out
> > and gets a Ack from board maintainers.
> >
> > cheers,
> > -roger
> >
> > >
> > > Regards,
> > > Jonas
> > >
> > >>
> > >>>
> > >>>  drivers/core/ofnode.c | 2 +-
> > >>>  include/dm/ofnode.h   | 8 ++++----
> > >>>  2 files changed, 5 insertions(+), 5 deletions(-)
> > >>>
> > >>> diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
> > >>> index 8df16e56af5c..b1e94b2d60df 100644
> > >>> --- a/drivers/core/ofnode.c
> > >>> +++ b/drivers/core/ofnode.c
> > >>> @@ -1353,7 +1353,7 @@ bool ofnode_pre_reloc(ofnode node)
> > >>>      */
> > >>>     if (ofnode_read_bool(node, "bootph-pre-ram") ||
> > >>>         ofnode_read_bool(node, "bootph-pre-sram"))
> > >>> -           return true;
> > >>> +           return gd->flags & GD_FLG_RELOC;
> > >>>
> > >>>     if (IS_ENABLED(CONFIG_OF_TAG_MIGRATE)) {
> > >>>             /* detect and handle old tags */
> > >>> diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h
> > >>> index 0f38b3e736de..13700f8266d7 100644
> > >>> --- a/include/dm/ofnode.h
> > >>> +++ b/include/dm/ofnode.h
> > >>> @@ -1198,15 +1198,15 @@ int ofnode_read_simple_size_cells(ofnode node);
> > >>>   * determine if a node was bound in one of SPL/TPL stages.
> > >>>   *
> > >>>   * There are 4 settings currently in use
> > >>> - * - bootph-some-ram: U-Boot proper pre-relocation only
> > >>> + * - bootph-some-ram: U-Boot proper pre-relocation phase
> > >>>   * - bootph-all: all phases
> > >>>   * Existing platforms only use it to indicate nodes needed in
> > >>>   * SPL. Should probably be replaced by bootph-pre-ram for new platforms.
> > >>> - * - bootph-pre-ram: SPL and U-Boot pre-relocation
> > >>> - * - bootph-pre-sram: TPL and U-Boot pre-relocation
> > >>> + * - bootph-pre-ram: SPL phase
> > >>> + * - bootph-pre-sram: TPL phase
> > >>>   *
> > >>>   * @node: node to check
> > >>> - * Return: true if node is needed in SPL/TL, false otherwise
> > >>> + * Return: true if node should be or was bound, false otherwise
> > >>>   */
> > >>>  bool ofnode_pre_reloc(ofnode node);
> > >>>
> > >>
> > >> ---boot log---
> > >>
> > >> U-Boot SPL 2023.10-rc4-00480-g9e644284ab (Sep 28 2023 - 15:53:09 +0300)
> > >> Resetting on cold boot to workaround ErrataID:i2331
> > >> Please resend tiboot3.bin in case of UART/DFU boot
> > >> resetting ...
> > >>
> > >> U-Boot SPL 2023.10-rc4-00480-g9e644284ab (Sep 28 2023 - 15:53:09 +0300)
> > >> SYSFW ABI: 3.1 (firmware rev 0x0009 '9.0.5--v09.00.05 (Kool Koala)')
> > >> SPL initial stack usage: 13408 bytes
> > >> Trying to boot from MMC2
> > >> Warning: Detected image signing certificate on GP device. Skipping certificate to prevent boot failure. This will fail if the image was also encrypted
> > >> Warning: Detected image signing certificate on GP device. Skipping certificate to prevent boot failure. This will fail if the image was also encrypted
> > >> Warning: Detected image signing certificate on GP device. Skipping certificate to prevent boot failure. This will fail if the image was also encrypted
> > >> Warning: Detected image signing certificate on GP device. Skipping certificate to prevent boot failure. This will fail if the image was also encrypted
> > >> Loading Environment from MMC... *** Warning - No MMC card found, using default environment
> > >>
> > >> Starting ATF on ARM64 core...
> > >>
> > >> NOTICE:  BL31: v2.8(release):v2.8-226-g2fcd408bb3-dirty
> > >> NOTICE:  BL31: Built : 00:42:57, Jan 13 2023
> > >> I/TC:
> > >> I/TC: OP-TEE version: 3.20.0 (gcc version 11.3.0 (GCC)) #1 Fri Jan 20 15:42:54 UTC 2023 aarch64
> > >> I/TC: WARNING: This OP-TEE configuration might be insecure!
> > >> I/TC: WARNING: Please check https://optee.readthedocs.io/en/latest/architecture/porting_guidelines.html
> > >> I/TC: Primary CPU initializing
> > >> I/TC: SYSFW ABI: 3.1 (firmware rev 0x0009 '9.0.5--v09.00.05 (Kool Koala)')
> > >> I/TC: HUK Initialized
> > >> I/TC: Activated SA2UL device
> > >> I/TC: Fixing SA2UL firewall owner for GP device
> > >> I/TC: Enabled firewalls for SA2UL TRNG device
> > >> I/TC: SA2UL TRNG initialized
> > >> I/TC: SA2UL Drivers initialized
> > >> I/TC: Primary CPU switching to normal world boot
> > >>
> > >> U-Boot SPL 2023.10-rc4-00480-g9e644284ab (Sep 28 2023 - 15:54:37 +0300)
> > >> SYSFW ABI: 3.1 (firmware rev 0x0009 '9.0.5--v09.00.05 (Kool Koala)')
> > >> Trying to boot from MMC2
> > >> Warning: Detected image signing certificate on GP device. Skipping certificate to prevent boot failure. This will fail if the image was also encrypted
> > >> Warning: Detected image signing certificate on GP device. Skipping certificate to prevent boot failure. This will fail if the image was also encrypted
> > >>
> > >> ---boot log end---
> > >>
> > >> we expect U-Boot banner to be printed next but nothing is printed.
> > >>
> > >
> >

Re: [PATCH v2] dm: core: Report bootph-pre-ram/sram node as pre-reloc after relocation

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1601 bytes --]

On Sun, Aug 20, 2023 at 10:03:18PM +0000, Jonas Karlman wrote:

> Nodes with bootph-pre-sram/ram props are bound in multiple phases:
> 1. At TPL (bootph-pre-sram) or SPL (bootph-pre-ram) phase
> 2. At U-Boot proper pre-relocation phase
> 3. At U-Boot proper normal phase
> 
> However the binding and U-Boot Driver Model documentation indicate that
> only nodes marked with bootph-all or bootph-some-ram should be bound in
> the U-Boot proper pre-relocation phase.
> 
> Change ofnode_pre_reloc to report a node with bootph-pre-ram/sram prop
> with a pre-reloc status only after U-Boot proper pre-relocation phase.
> Also update the ofnode_pre_reloc documentation to closer reflect the
> binding and driver model documentation.
> 
> This changes behavior of what nodes are bound in the U-Boot proper
> pre-relocation phase. Change to bootph-all or add bootph-some-ram prop
> to restore prior behavior.
> 
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> Reviewed-by: Simon Glass <sjg@chromium.org>

I'm following up here now that this is committed because there's been a
large number of commits to update dts files because of the change this
commit makes. And I know there's other platforms still broken that need
the same update. And it seems the change is almos always to mark more
(or all!) nodes as bootph-all. So, is this patch really correct, and
really what we want to be doing here? I was not expecting this much
fallout when I saw this come in. Thanks.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]