[PATCH v6 16/25] spl: Convert fat to spl_load

[Simon Glass]

Hi Sean,

On Sun, 5 Nov 2023 at 19:26, Sean Anderson <seanga2 at gmail.com> wrote:
>
> This converts the fat loader to use spl_load. Some platforms are very
> tight on space, so we take care to only include the code we really need.
>
> Signed-off-by: Sean Anderson <seanga2 at gmail.com>
> ---
>
> (no changes since v5)
>
> Changes in v5:
> - Rework to load header in spl_load
>
> Changes in v3:
> - Fix failing on success
>
>  common/spl/spl_fat.c     | 56 +++++++++++++---------------------------
>  include/spl_load.h       |  1 +
>  test/image/spl_load_fs.c |  3 +--
>  3 files changed, 20 insertions(+), 40 deletions(-)

Reviewed-by: Simon Glass <sjg at chromium.org>

nits below

>
> diff --git a/common/spl/spl_fat.c b/common/spl/spl_fat.c
> index a0c34eba48f..569f2b32928 100644
> --- a/common/spl/spl_fat.c
> +++ b/common/spl/spl_fat.c
> @@ -11,8 +11,8 @@
>  #include <common.h>
>  #include <env.h>
>  #include <log.h>
> -#include <mapmem.h>
>  #include <spl.h>
> +#include <spl_load.h>
>  #include <asm/u-boot.h>
>  #include <fat.h>
>  #include <errno.h>
> @@ -66,58 +66,38 @@ int spl_load_image_fat(struct spl_image_info *spl_image,
>                        const char *filename)
>  {
>         int err;
> -       struct legacy_img_hdr *header;
> +       loff_t size;
> +       struct spl_load_info load;
>
>         err = spl_register_fat_device(block_dev, partition);
>         if (err)
>                 goto end;
>
> -       header = spl_get_load_buffer(-sizeof(*header), sizeof(*header));
> -
> -       err = file_fat_read(filename, header, sizeof(struct legacy_img_hdr));
> -       if (err <= 0)
> -               goto end;
> -
> -       if (IS_ENABLED(CONFIG_SPL_LOAD_FIT_FULL) &&
> -           image_get_magic(header) == FDT_MAGIC) {
> -               err = file_fat_read(filename,
> -                                   map_sysmem(CONFIG_SYS_LOAD_ADDR, 0), 0);
> -               if (err <= 0)
> -                       goto end;
> -               err = spl_parse_image_header(spl_image, bootdev,
> -                                            map_sysmem(CONFIG_SYS_LOAD_ADDR,
> -                                                       err));
> -               if (err == -EAGAIN)
> -                       return err;
> -               if (err == 0)
> -                       err = 1;
> -       } else if (IS_ENABLED(CONFIG_SPL_LOAD_FIT) &&
> -           image_get_magic(header) == FDT_MAGIC) {
> -               struct spl_load_info load;
> -
> -               debug("Found FIT\n");
> -               load.read = spl_fit_read;
> -               spl_set_bl_len(&load, ARCH_DMA_MINALIGN);
> -               load.priv = (void *)filename;
> -
> -               return spl_load_simple_fit(spl_image, &load, 0, header);
> -       } else {
> -               err = spl_parse_image_header(spl_image, bootdev, header);
> +       /*
> +        * Avoid pulling in this function for other image types since we are
> +        * very short on space on some boards.
> +        */
> +       if (IS_ENABLED(CONFIG_SPL_LOAD_FIT_FULL)) {
> +               err = fat_size(filename, &size);
>                 if (err)
>                         goto end;
> -
> -               err = file_fat_read(filename, map_sysmem(spl_image->load_addr,
> -                                                        spl_image->size), 0);
> +       } else {
> +               size = 0;
>         }
>
> +       load.read = spl_fit_read;
> +       spl_set_bl_len(&load, ARCH_DMA_MINALIGN);
> +       load.priv = (void *)filename;
> +       err = spl_load(spl_image, bootdev, &load, size, 0);
> +
>  end:
>  #ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
> -       if (err <= 0)
> +       if (err < 0)
>                 printf("%s: error reading image %s, err - %d\n",
>                        __func__, filename, err);

Do we still need this sort of #ifdef? I thought printf() would melt
away without SPL_LIBCOMMON_SUPPORT? I haven't looked though. Also it
isn't related to your patch.

>  #endif
>
> -       return (err <= 0);
> +       return err;
>  }
>
>  #if CONFIG_IS_ENABLED(OS_BOOT)
> diff --git a/include/spl_load.h b/include/spl_load.h
> index 65aa6bb4493..5e0460d956d 100644
> --- a/include/spl_load.h
> +++ b/include/spl_load.h
> @@ -96,6 +96,7 @@ static inline int _spl_load(struct spl_image_info *spl_image,
>   */
>  #define SPL_LOAD_USERS \
>         IS_ENABLED(CONFIG_SPL_FS_EXT4) + \
> +       IS_ENABLED(CONFIG_SPL_FS_FAT) + \
>         0
>
>  #if SPL_LOAD_USERS > 1
> diff --git a/test/image/spl_load_fs.c b/test/image/spl_load_fs.c
> index 01559e98c4f..333df2dfb53 100644
> --- a/test/image/spl_load_fs.c
> +++ b/test/image/spl_load_fs.c
> @@ -425,8 +425,7 @@ static int spl_test_mmc(struct unit_test_state *uts, const char *test_name,
>         if (spl_test_mmc_fs(uts, test_name, type, create_ext2, false))
>                 return CMD_RET_FAILURE;
>
> -       if (type != IMX8 && type != LEGACY_LZMA &&
> -           spl_test_mmc_fs(uts, test_name, type, create_fat, false))
> +       if (spl_test_mmc_fs(uts, test_name, type, create_fat, false))
>                 return CMD_RET_FAILURE;
>
>         if (type == LEGACY_LZMA)
> --
> 2.37.1
>

Regards,
Simon