| Message ID | 20231106022603.3405551-17-seanga2@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | spl: Use common function for loading/parsing images | expand |
Hi Sean, On Sun, 5 Nov 2023 at 19:26, Sean Anderson <seanga2@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@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@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
On 11/7/23 23:24, Simon Glass wrote: > Hi Sean, > > On Sun, 5 Nov 2023 at 19:26, Sean Anderson <seanga2@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@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@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. From what I can tell, printf implementation is orthogonal to LIBCOMMON_SUPPORT. --Sean >> #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
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); #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)
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@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(-)