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@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@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