[PATCH] xilinx: Add option to load environment from outside of boot media

Vasileios Amoiridis vasileios.amoiridis at cern.ch
Wed May 29 16:46:24 CEST 2024


On Mon, 2024-05-27 at 09:34 +0200, Michal Simek wrote:
> 
> 
> On 5/22/24 19:47, Vasileios Amoiridis wrote:
> > From: Vasileios Amoiridis <vasileios.amoiridis at cern.ch>
> > 
> > Currently, if the environment is not in the current boot media, the
> > env_get_location() is returning ENVL_UNKNOWN or ENVL_NOWHERE which
> > is not true (i.e booting from FLASH with environment in eMMC). This
> > commit adds an extra check to find the environment in the other
> > supported boot media, keeping the same priority as of now.
> > 
> > Signed-off-by: Vasileios Amoiridis <vasileios.amoiridis at cern.ch>
> > ---
> >   board/xilinx/versal-net/board.c | 21 +++++++++++++++++++--
> >   board/xilinx/versal/board.c     | 23 ++++++++++++++++++++---
> >   board/xilinx/zynq/board.c       | 31 +++++++++++++++++++++++++++-
> > ---
> >   board/xilinx/zynqmp/zynqmp.c    | 31 +++++++++++++++++++++++++++-
> > ---
> >   4 files changed, 93 insertions(+), 13 deletions(-)
> > 
> > diff --git a/board/xilinx/versal-net/board.c b/board/xilinx/versal-
> > net/board.c
> > index da03024e16..5648d6685e 100644
> > --- a/board/xilinx/versal-net/board.c
> > +++ b/board/xilinx/versal-net/board.c
> > @@ -372,6 +372,21 @@ void reset_cpu(void)
> >   {
> >   }
> >   
> > +static enum env_location env_locations[] = {
> > +#ifdef CONFIG_ENV_IS_IN_FAT
> > +       ENVL_FAT,
> > +#endif
> > +#ifdef CONFIG_ENV_IS_IN_EXT4
> > +       ENVL_EXT4,
> > +#endif
> > +#ifdef CONFIG_ENV_IS_IN_SPI_FLASH
> > +       ENVL_SPI_FLASH,
> > +#endif
> > +#ifdef CONFIG_ENV_IS_NOWHERE
> > +       ENVL_NOWHERE,
> > +#endif
> > +};
> > +
> >   #if defined(CONFIG_ENV_IS_NOWHERE)
> >   enum env_location env_get_location(enum env_operation op, int
> > prio)
> >   {
> > @@ -389,17 +404,19 @@ enum env_location env_get_location(enum
> > env_operation op, int prio)
> >                         return ENVL_FAT;
> >                 if (IS_ENABLED(CONFIG_ENV_IS_IN_EXT4))
> >                         return ENVL_EXT4;
> > -               return ENVL_NOWHERE;
> > +               break;
> >         case OSPI_MODE:
> >         case QSPI_MODE_24BIT:
> >         case QSPI_MODE_32BIT:
> >                 if (IS_ENABLED(CONFIG_ENV_IS_IN_SPI_FLASH))
> >                         return ENVL_SPI_FLASH;
> > -               return ENVL_NOWHERE;
> > +               break;
> >         case JTAG_MODE:
> >         case SELECTMAP_MODE:
> >         default:
> >                 return ENVL_NOWHERE;
> >         }
> > +
> > +       return env_locations[prio];
> >   }
> >   #endif
> > diff --git a/board/xilinx/versal/board.c
> > b/board/xilinx/versal/board.c
> > index 4f6d56119d..8aed2e97df 100644
> > --- a/board/xilinx/versal/board.c
> > +++ b/board/xilinx/versal/board.c
> > @@ -291,12 +291,27 @@ void reset_cpu(void)
> >   {
> >   }
> >   
> > +static enum env_location env_locations[] = {
> > +#ifdef CONFIG_ENV_IS_IN_FAT
> > +       ENVL_FAT,
> > +#endif
> > +#ifdef CONFIG_ENV_IS_IN_EXT4
> > +       ENVL_EXT4,
> > +#endif
> > +#ifdef CONFIG_ENV_IS_IN_SPI_FLASH
> > +       ENVL_SPI_FLASH,
> > +#endif
> > +#ifdef CONFIG_ENV_IS_NOWHERE
> > +       ENVL_NOWHERE,
> > +#endif
> > +};
> > +
> >   #if defined(CONFIG_ENV_IS_NOWHERE)
> >   enum env_location env_get_location(enum env_operation op, int
> > prio)
> >   {
> >         u32 bootmode = versal_get_bootmode();
> >   
> > -       if (prio)
> > +       if (prio >= ARRAY_SIZE(env_locations))
> >                 return ENVL_UNKNOWN;
> >   
> >         switch (bootmode) {
> > @@ -308,17 +323,19 @@ enum env_location env_get_location(enum
> > env_operation op, int prio)
> >                         return ENVL_FAT;
> >                 if (IS_ENABLED(CONFIG_ENV_IS_IN_EXT4))
> >                         return ENVL_EXT4;
> > -               return ENVL_NOWHERE;
> > +               break;
> >         case OSPI_MODE:
> >         case QSPI_MODE_24BIT:
> >         case QSPI_MODE_32BIT:
> >                 if (IS_ENABLED(CONFIG_ENV_IS_IN_SPI_FLASH))
> >                         return ENVL_SPI_FLASH;
> > -               return ENVL_NOWHERE;
> > +               break;
> >         case JTAG_MODE:
> >         case SELECTMAP_MODE:
> >         default:
> >                 return ENVL_NOWHERE;
> >         }
> > +
> > +       return env_locations[prio];
> >   }
> >   #endif
> > diff --git a/board/xilinx/zynq/board.c b/board/xilinx/zynq/board.c
> > index 6c36591001..6fa5016cdd 100644
> > --- a/board/xilinx/zynq/board.c
> > +++ b/board/xilinx/zynq/board.c
> > @@ -134,11 +134,32 @@ int dram_init(void)
> >   }
> >   #endif
> >   
> > +static enum env_location env_locations[] = {
> > +#ifdef CONFIG_ENV_IS_IN_FAT
> > +       ENVL_FAT,
> > +#endif
> > +#ifdef CONFIG_ENV_IS_IN_EXT4
> > +       ENVL_EXT4,
> > +#endif
> > +#ifdef CONFIG_ENV_IS_IN_NAND
> > +       ENVL_NAND,
> > +#endif
> > +#ifdef CONFIG_ENV_IS_IN_UBI
> > +       ENVL_UBI,
> > +#endif
> > +#ifdef CONFIG_ENV_IS_IN_SPI_FLASH
> > +       ENVL_SPI_FLASH,
> > +#endif
> > +#ifdef CONFIG_ENV_IS_NOWHERE
> > +       ENVL_NOWHERE,
> > +#endif
> > +};
> > +
> >   enum env_location env_get_location(enum env_operation op, int
> > prio)
> >   {
> >         u32 bootmode = zynq_slcr_get_boot_mode() & ZYNQ_BM_MASK;
> >   
> > -       if (prio)
> > +       if (prio >= ARRAY_SIZE(env_locations))
> >                 return ENVL_UNKNOWN;
> >   
> >         switch (bootmode) {
> > @@ -147,22 +168,24 @@ enum env_location env_get_location(enum
> > env_operation op, int prio)
> >                         return ENVL_FAT;
> >                 if (IS_ENABLED(CONFIG_ENV_IS_IN_EXT4))
> >                         return ENVL_EXT4;
> > -               return ENVL_NOWHERE;
> > +               break;
> >         case ZYNQ_BM_NAND:
> >                 if (IS_ENABLED(CONFIG_ENV_IS_IN_NAND))
> >                         return ENVL_NAND;
> >                 if (IS_ENABLED(CONFIG_ENV_IS_IN_UBI))
> >                         return ENVL_UBI;
> > -               return ENVL_NOWHERE;
> > +               break;
> >         case ZYNQ_BM_NOR:
> >         case ZYNQ_BM_QSPI:
> >                 if (IS_ENABLED(CONFIG_ENV_IS_IN_SPI_FLASH))
> >                         return ENVL_SPI_FLASH;
> > -               return ENVL_NOWHERE;
> > +               break;
> >         case ZYNQ_BM_JTAG:
> >         default:
> >                 return ENVL_NOWHERE;
> >         }
> > +
> > +       return env_locations[prio];
> >   }
> >   
> >   #if defined(CONFIG_SET_DFU_ALT_INFO)
> > diff --git a/board/xilinx/zynqmp/zynqmp.c
> > b/board/xilinx/zynqmp/zynqmp.c
> > index f370fb7347..7e646d342b 100644
> > --- a/board/xilinx/zynqmp/zynqmp.c
> > +++ b/board/xilinx/zynqmp/zynqmp.c
> > @@ -588,12 +588,33 @@ int mmc_get_env_dev(void)
> >         return bootseq;
> >   }
> >   
> > +static enum env_location env_locations[] = {
> > +#ifdef CONFIG_ENV_IS_IN_FAT
> > +       ENVL_FAT,
> > +#endif
> > +#ifdef CONFIG_ENV_IS_IN_EXT4
> > +       ENVL_EXT4,
> > +#endif
> > +#ifdef CONFIG_ENV_IS_IN_NAND
> > +       ENVL_NAND,
> > +#endif
> > +#ifdef CONFIG_ENV_IS_IN_UBI
> > +       ENVL_UBI,
> > +#endif
> > +#ifdef CONFIG_ENV_IS_IN_SPI_FLASH
> > +       ENVL_SPI_FLASH,
> > +#endif
> > +#ifdef CONFIG_ENV_IS_NOWHERE
> > +       ENVL_NOWHERE,
> > +#endif
> > +};
> > +
> >   #if defined(CONFIG_ENV_IS_NOWHERE)
> >   enum env_location env_get_location(enum env_operation op, int
> > prio)
> >   {
> >         u32 bootmode = zynqmp_get_bootmode();
> >   
> > -       if (prio)
> > +       if (prio >= ARRAY_SIZE(env_locations))
> >                 return ENVL_UNKNOWN;
> >   
> 
> if (!prio) {
>         switch ...
> }
> 
> please look below.
> (NOTE: Above you are not fixing all handling around prio > 0)
> 
> 

Ok I see your point.

> >         switch (bootmode) {
> > @@ -605,22 +626,24 @@ enum env_location env_get_location(enum
> > env_operation op, int prio)
> >                         return ENVL_FAT;
> >                 if (IS_ENABLED(CONFIG_ENV_IS_IN_EXT4))
> >                         return ENVL_EXT4;
> > -               return ENVL_NOWHERE;
> > +               break;
> >         case NAND_MODE:
> >                 if (IS_ENABLED(CONFIG_ENV_IS_IN_NAND))
> >                         return ENVL_NAND;
> >                 if (IS_ENABLED(CONFIG_ENV_IS_IN_UBI))
> >                         return ENVL_UBI;
> > -               return ENVL_NOWHERE;
> > +               break;
> >         case QSPI_MODE_24BIT:
> >         case QSPI_MODE_32BIT:
> >                 if (IS_ENABLED(CONFIG_ENV_IS_IN_SPI_FLASH))
> >                         return ENVL_SPI_FLASH;
> > -               return ENVL_NOWHERE;
> > +               break;
> >         case JTAG_MODE:
> >         default:
> >                 return ENVL_NOWHERE;
> >         }
> > +
> > +       return env_locations[prio];
> 
> And here call
> 
> return arch_env_get_location(op, prio);
> 
> This can avoid duplication of these env_locations[] across all our
> SOCs.
> And pretty much fallback to standard implementation which is IMHO
> good solution.
> 
> What do you think?
> 
> Thanks,
> Michal

That's a much cleaner approach, I was not aware of the 
arch_env_get_location(op, prio). Thank you very much for pointing
this out, I will resubmit!

Cheers,
Vasilis


More information about the U-Boot mailing list