| Message ID | 20240528143652.64489-1-kory.maincent@bootlin.com |
|---|---|
| State | Superseded |
| Delegated to: | Michal Simek |
| Headers | show |
| Series | xilinx: zynqmp: Allow multiboot environment write even in saved environment | expand |
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
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,
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
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)
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(-)