[PATCH] xilinx: zynqmp: Allow multiboot environment write even in saved environment

[PATCH] xilinx: zynqmp: Allow multiboot environment write even in saved environment

[Kory Maincent]

Once the environment was saved, we could not retrieve information about
the multiboot image used. When dealing with firmware updates, this
information is necessary alongside the saved environment.

Move the multiboot environment set operation before the saved environment
check to ensure this information is always available.

Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---
 board/xilinx/zynqmp/zynqmp.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
index f370fb7347a..16292ed1c7e 100644
--- a/board/xilinx/zynqmp/zynqmp.c
+++ b/board/xilinx/zynqmp/zynqmp.c
@@ -519,6 +519,10 @@ int board_late_init(void)
 	usb_ether_init();
 #endif
 
+	multiboot = multi_boot();
+	if (multiboot >= 0)
+		env_set_hex("multiboot", multiboot);
+
 	if (!(gd->flags & GD_FLG_ENV_DEFAULT)) {
 		debug("Saved variables - Skipping\n");
 		return 0;
@@ -531,10 +535,6 @@ int board_late_init(void)
 	if (ret)
 		return ret;
 
-	multiboot = multi_boot();
-	if (multiboot >= 0)
-		env_set_hex("multiboot", multiboot);
-
 	if (IS_ENABLED(CONFIG_DISTRO_DEFAULTS)) {
 		ret = boot_targets_setup();
 		if (ret)
-- 
2.34.1

Re: [PATCH] xilinx: zynqmp: Allow multiboot environment write even in saved environment

[Michal Simek]

On 5/28/24 16:36, Kory Maincent wrote:
> Once the environment was saved, we could not retrieve information about

nit: use imperative mood.

> the multiboot image used. When dealing with firmware updates, this
> information is necessary alongside the saved environment.

what exactly are you trying to do?

> 
> Move the multiboot environment set operation before the saved environment
> check to ensure this information is always available.

that make sense.

> 
> Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
> ---
>   board/xilinx/zynqmp/zynqmp.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
> index f370fb7347a..16292ed1c7e 100644
> --- a/board/xilinx/zynqmp/zynqmp.c
> +++ b/board/xilinx/zynqmp/zynqmp.c
> @@ -519,6 +519,10 @@ int board_late_init(void)
>   	usb_ether_init();
>   #endif
>   
> +	multiboot = multi_boot();
> +	if (multiboot >= 0)

likely make sense to remove this check to because you want to have even 
multiboot=0 too.

> +		env_set_hex("multiboot", multiboot);
> +
>   	if (!(gd->flags & GD_FLG_ENV_DEFAULT)) {
>   		debug("Saved variables - Skipping\n");
>   		return 0;
> @@ -531,10 +535,6 @@ int board_late_init(void)
>   	if (ret)
>   		return ret;
>   
> -	multiboot = multi_boot();
> -	if (multiboot >= 0)
> -		env_set_hex("multiboot", multiboot);
> -
>   	if (IS_ENABLED(CONFIG_DISTRO_DEFAULTS)) {
>   		ret = boot_targets_setup();
>   		if (ret)

M

Re: [PATCH] xilinx: zynqmp: Allow multiboot environment write even in saved environment

[Kory Maincent]

On Tue, 28 May 2024 16:53:42 +0200
Michal Simek <michal.simek@amd.com> wrote:

> On 5/28/24 16:36, Kory Maincent wrote:
> > Once the environment was saved, we could not retrieve information about  
> 
> nit: use imperative mood.

Ah indeed.

> 
> > the multiboot image used. When dealing with firmware updates, this
> > information is necessary alongside the saved environment.  
> 
> what exactly are you trying to do?

Retrieving the multiboot information to know which boot000x.bin image we have
booted. This would allow to check the boot status of a firmware update.

> > Move the multiboot environment set operation before the saved environment
> > check to ensure this information is always available.  
> 
> that make sense.
> 
> > 
> > Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
> > ---
> >   board/xilinx/zynqmp/zynqmp.c | 8 ++++----
> >   1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
> > index f370fb7347a..16292ed1c7e 100644
> > --- a/board/xilinx/zynqmp/zynqmp.c
> > +++ b/board/xilinx/zynqmp/zynqmp.c
> > @@ -519,6 +519,10 @@ int board_late_init(void)
> >   	usb_ether_init();
> >   #endif
> >   
> > +	multiboot = multi_boot();
> > +	if (multiboot >= 0)  
> 
> likely make sense to remove this check to because you want to have even 
> multiboot=0 too.

I don't understand, this check is >= so it supports multiboot=0.

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

Re: [PATCH] xilinx: zynqmp: Allow multiboot environment write even in saved environment

[Michal Simek]

On 5/28/24 17:11, Kory Maincent wrote:
> On Tue, 28 May 2024 16:53:42 +0200
> Michal Simek <michal.simek@amd.com> wrote:
> 
>> On 5/28/24 16:36, Kory Maincent wrote:
>>> Once the environment was saved, we could not retrieve information about
>>
>> nit: use imperative mood.
> 
> Ah indeed.
> 
>>
>>> the multiboot image used. When dealing with firmware updates, this
>>> information is necessary alongside the saved environment.
>>
>> what exactly are you trying to do?
> 
> Retrieving the multiboot information to know which boot000x.bin image we have
> booted. This would allow to check the boot status of a firmware update.
> 
>>> Move the multiboot environment set operation before the saved environment
>>> check to ensure this information is always available.
>>
>> that make sense.
>>
>>>
>>> Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
>>> ---
>>>    board/xilinx/zynqmp/zynqmp.c | 8 ++++----
>>>    1 file changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
>>> index f370fb7347a..16292ed1c7e 100644
>>> --- a/board/xilinx/zynqmp/zynqmp.c
>>> +++ b/board/xilinx/zynqmp/zynqmp.c
>>> @@ -519,6 +519,10 @@ int board_late_init(void)
>>>    	usb_ether_init();
>>>    #endif
>>>    
>>> +	multiboot = multi_boot();
>>> +	if (multiboot >= 0)
>>
>> likely make sense to remove this check to because you want to have even
>> multiboot=0 too.
> 
> I don't understand, this check is >= so it supports multiboot=0.

Sorry my fault. You are right. It was there for catching the fault.

Thanks,
Michal