[PATCH 2/2] env: add CONFIG_ENV_SECT_SIZE_AUTO

[Joakim Tjernlund]

On Wed, 2020-05-06 at 10:47 +0200, Rasmus Villemoes wrote:
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.
> 
> 
> This is roughly the U-Boot side equivalent to commit
> e282c422e0 (tools: fw_env: use erasesize from MEMGETINFO ioctl). The
> motivation is the case where one has a board with several revisions,
> where the SPI flashes have different erase sizes.
> 
> In our case, we have an 8K environment, and the flashes have erase
> sizes of 4K (newer boards) and 64K (older boards). Currently, we must
> set CONFIG_ENV_SECT_SIZE to 64K to make the code work on the older
> boards, but for the newer ones, that ends up wasting quite a bit of
> time reading/erasing/restoring the last 56K.
> 
> At first, I wanted to allow setting CONFIG_ENV_SECT_SIZE to 0 to mean
> "use the erase size the chip reports", but that config
> option is used in a number of preprocessor conditionals, and shared
> between ENV_IS_IN_FLASH and ENV_IS_IN_SPI_FLASH.

I see, It had been good if one could have used 0

> 
> So instead, introduce a new boolean config option, which for now can
> only be used with ENV_IS_IN_SPI_FLASH. If left off, there's no change
> in behaviour.

Please don't stop at just SPI Flash, we could use this for common NOR flashes too.

> 
> The only slightly annoying detail is that, when selected, the compiler
> is apparently not smart enough to see that the the saved_size and
> saved_offset variables are only used under the same "if (sect_size >
> CONFIG_ENV_SIZE)" condition as where they are computed, so we need to
> initialize them to 0 to avoid "may be used uninitialized" warnings.
> 
> On our newer boards with the 4K erase size, saving the environment now
> takes 0.080 seconds instead of 0.53 seconds, which directly translates
> to that much faster boot time since our logic always causes the
> environment to be written during boot.
> 
> Signed-off-by: Rasmus Villemoes <rasmus.villemoes at prevas.dk>
> ---
>  env/Kconfig | 14 ++++++++++++++
>  env/sf.c    | 10 ++++++++--
>  2 files changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/env/Kconfig b/env/Kconfig
> index 969308fe6c..c90cd04604 100644
> --- a/env/Kconfig
> +++ b/env/Kconfig
> @@ -317,6 +317,20 @@ config ENV_IS_IN_SPI_FLASH
>           during a "saveenv" operation. CONFIG_ENV_OFFSET_REDUND must be
>           aligned to an erase sector boundary.
> 
> +config ENV_SECT_SIZE_AUTO
> +       bool "Use automatically detected sector size"
> +       depends on ENV_IS_IN_SPI_FLASH
> +       help
> +         Some boards exist in multiple variants, with different
> +         flashes having different sector sizes. In such cases, you
> +         can select this option to make U-Boot use the actual sector
> +         size when figuring out how much to erase, which can thus be
> +         more efficient on the flashes with smaller erase size. Since
> +         the environment must always be aligned on a sector boundary,
> +         CONFIG_ENV_OFFSET must be aligned to the largest of the
> +         different sector sizes, and CONFIG_ENV_SECT_SIZE should be
> +         set to that value.
> +
>  config USE_ENV_SPI_BUS
>         bool "SPI flash bus for environment"
>         depends on ENV_IS_IN_SPI_FLASH
> diff --git a/env/sf.c b/env/sf.c
> index cd5339578b..644e78fe3d 100644
> --- a/env/sf.c
> +++ b/env/sf.c
> @@ -69,7 +69,7 @@ static int env_sf_save(void)
>  {
>         env_t   env_new;
>         char    *saved_buffer = NULL, flag = ENV_REDUND_OBSOLETE;
> -       u32     saved_size, saved_offset, sector;
> +       u32     saved_size = 0, saved_offset = 0, sector;
>         u32     sect_size = CONFIG_ENV_SECT_SIZE;
>         int     ret;
> 
> @@ -77,6 +77,9 @@ static int env_sf_save(void)
>         if (ret)
>                 return ret;
> 
> +       if (IS_ENABLED(CONFIG_ENV_SECT_SIZE_AUTO))
> +               sect_size = env_flash->mtd.erasesize;
> +
>         ret = env_export(&env_new);
>         if (ret)
>                 return -EIO;
> @@ -184,7 +187,7 @@ out:
>  #else
>  static int env_sf_save(void)
>  {
> -       u32     saved_size, saved_offset, sector;
> +       u32     saved_size = 0, saved_offset = 0, sector;
>         u32     sect_size = CONFIG_ENV_SECT_SIZE;
>         char    *saved_buffer = NULL;
>         int     ret = 1;
> @@ -194,6 +197,9 @@ static int env_sf_save(void)
>         if (ret)
>                 return ret;
> 
> +       if (IS_ENABLED(CONFIG_ENV_SECT_SIZE_AUTO))
> +               sect_size = env_flash->mtd.erasesize;
> +
>         /* Is the sector larger than the env (i.e. embedded) */
>         if (sect_size > CONFIG_ENV_SIZE) {
>                 saved_size = sect_size - CONFIG_ENV_SIZE;
> --
> 2.23.0
>