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

[Michal Simek]

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)


>   	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