[PATCH] U-Boot ENV in EXT4 support for Espressobin

[Pali Rohár]

Hello! See inline comments below.

On Monday 07 February 2022 09:11:06 Rogier Stam wrote:
> Hi
> 
> I was attemting to boot from SATA on an Espressobin. As such I wanted to
> store the Environment in a SATA partition and not in the SPI flash. In order
> to get this running I had to make a few changes, as the original code
> assumes the Environment is always in SPI flash. Additionally I also had to
> force scsci_scan to ensure the AHCI disk / partitions could be found.
> 
> Hope it will be considered to merge
> 
> Regards
> 
> Rogier Stam
> 

> From 460eb1fc6d5189750d2ce791e45c89792c476167 Mon Sep 17 00:00:00 2001
> From: Rogier Stam <rogier at unrailed.org>
> Date: Mon, 31 Jan 2022 23:06:19 +0100
> Subject: [PATCH] Fix Espressobin build for configs where ENV is not in SPI
> 
> When storing the UBoot Environment in for example EXT4,
> the U-Boot build is broken for several reasons:
> 1. armada-385-turris-omnia-u-boot.dtsi will not allow
>    CONFIG_ENV_OFFSET and CONFIG_ENV_SIZE to be undefined
> 2. armada-37xx/board.c ft_board_setup function does not
>    exist if CONFIG_ENV_IS_IN_SPI_FLASH is not defined
> 3. When defining CONFIG_ENV_IS_IN_EXT4 and using AHCI
>    SCSI scan is not performed and hence the partition on
>    AHCI is not available when the environment is loaded.
> 
> This change fixes these 3 points so that the UBoot config
> can be on a AHCI (SATA) drive partition instead of
> enforced in SPI flash.
> 
> Signed-off-by: Rogier Stam <rogier at unrailed.org>
> ---
>  arch/arm/dts/armada-385-turris-omnia-u-boot.dtsi | 2 ++
>  board/Marvell/mvebu_armada-37xx/board.c          | 4 ++--
>  env/ext4.c                                       | 4 ++++
>  3 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/dts/armada-385-turris-omnia-u-boot.dtsi b/arch/arm/dts/armada-385-turris-omnia-u-boot.dtsi
> index 3ff76c9..7921298 100644
> --- a/arch/arm/dts/armada-385-turris-omnia-u-boot.dtsi
> +++ b/arch/arm/dts/armada-385-turris-omnia-u-boot.dtsi
> @@ -41,6 +41,7 @@
>  &spi0 {
>  	u-boot,dm-pre-reloc;
>  
> +#ifdef CONFIG_ENV_IS_IN_SPI_FLASH

In my opinion this #ifdef should be put above "&spi0 {" line.

But I will let this change to Marek for review (CCed).

>  	spi-nor at 0 {
>  		u-boot,dm-pre-reloc;
>  
> @@ -55,6 +56,7 @@
>  			};
>  		};
>  	};
> +#endif
>  };
>  
>  &uart0 {
> diff --git a/board/Marvell/mvebu_armada-37xx/board.c b/board/Marvell/mvebu_armada-37xx/board.c
> index d7b6eca..33b772a 100644
> --- a/board/Marvell/mvebu_armada-37xx/board.c
> +++ b/board/Marvell/mvebu_armada-37xx/board.c
> @@ -328,7 +328,6 @@ int board_network_enable(struct mii_dev *bus)
>  	return 0;
>  }
>  
> -#if defined(CONFIG_OF_BOARD_SETUP) && defined(CONFIG_ENV_IS_IN_SPI_FLASH)
>  int ft_board_setup(void *blob, struct bd_info *bd)
>  {
>  	int ret;
> @@ -340,6 +339,7 @@ int ft_board_setup(void *blob, struct bd_info *bd)
>  	if (!of_machine_is_compatible("globalscale,espressobin"))
>  		return 0;
>  
> +#if defined(CONFIG_OF_BOARD_SETUP) && defined(CONFIG_ENV_IS_IN_SPI_FLASH)

Even this change is not logically correct. Function ft_board_setup()
should be defined only when CONFIG_OF_BOARD_SETUP is enabled.

So correct change should look like:

#ifdef CONFIG_OF_BOARD_SETUP
int ft_board_setup(void *blob, struct bd_info *bd)
{
#ifdef CONFIG_ENV_IS_IN_SPI_FLASH
	int ret;
	int spi_off;
...
#endif
	return 0;
}
#endif

>  	spi_off = fdt_node_offset_by_compatible(blob, -1, "jedec,spi-nor");
>  	if (spi_off < 0)
>  		return 0;
> @@ -424,6 +424,6 @@ int ft_board_setup(void *blob, struct bd_info *bd)
>  		return 0;
>  	}
>  
> +#endif
>  	return 0;
>  }
> -#endif
> diff --git a/env/ext4.c b/env/ext4.c
> index 9f65afb..88214b9 100644
> --- a/env/ext4.c
> +++ b/env/ext4.c
> @@ -146,6 +146,10 @@ static int env_ext4_load(void)
>  	if (!strcmp(ifname, "mmc"))
>  		mmc_initialize(NULL);
>  #endif
> +#ifdef CONFIG_AHCI
> +	if (!strcmp(ifname, "scsi"))
> +		scsi_scan(true);
> +#endif

I guess that same change is required also for env/fat.c driver.

>  
>  	part = blk_get_device_part_str(ifname, dev_and_part,
>  				       &dev_desc, &info, 1);
> -- 
> 2.7.4
>