From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chee, Tien Fong Date: Tue, 31 Oct 2017 11:15:29 +0000 Subject: [U-Boot] [PATCH v4 06/20] common: Generic firmware loader for file system In-Reply-To: <848bf9ab-71a4-1540-2589-a79905d66bd5@denx.de> References: <1509447128-3182-1-git-send-email-tien.fong.chee@intel.com> <848bf9ab-71a4-1540-2589-a79905d66bd5@denx.de> Message-ID: <1509448528.2839.6.camel@intel.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit To: u-boot@lists.denx.de On Sel, 2017-10-31 at 12:01 +0100, Marek Vasut wrote: > On 10/31/2017 11:52 AM, tien.fong.chee at intel.com wrote: > > > > From: Tien Fong Chee > > > > Generic firmware loader framework contains some common > > functionality > > which is reusable by any specific driver file system firmware > > loader. > > Specific driver file system firmware loader handling can be defined > > with both weak function fsloader_preprocess and fs_loading. > > > > Signed-off-by: Tien Fong Chee > Why did you put everything into splash_source.c ? If this is supposed > to > be a generic loader, I'm sure not everyone would want to enable > splash > screen support to get generic firmware loader support. > > The API should looks more like the linux firmware API. > Initially, i created the new file called loadfs.c, which contains common codes and generic fs firmware loader. I plan to replace fs loader in splash_source.c at seperate patchset. Then, i changed the codes directly on splash_source.c based on your comment. May be i misunderstood your meaning. > > > > --- > >  common/Makefile                         |   7 +- > >  common/splash_source.c                  | 110 > > +++++++++++++++++++++++++------- > >  configs/socfpga_arria10_defconfig       |   2 + > >  include/configs/socfpga_arria10_socdk.h |   3 + > >  include/splash.h                        |   3 + > >  5 files changed, 99 insertions(+), 26 deletions(-) > > > > diff --git a/common/Makefile b/common/Makefile > > index 801ea31..965a217 100644 > > --- a/common/Makefile > > +++ b/common/Makefile > > @@ -47,8 +47,6 @@ obj-$(CONFIG_MTD_NOR_FLASH) += flash.o > >  obj-$(CONFIG_CMD_KGDB) += kgdb.o kgdb_stubs.o > >  obj-$(CONFIG_I2C_EDID) += edid.o > >  obj-$(CONFIG_KALLSYMS) += kallsyms.o > > -obj-y += splash.o > > -obj-$(CONFIG_SPLASH_SOURCE) += splash_source.o > >  ifndef CONFIG_DM_VIDEO > >  obj-$(CONFIG_LCD) += lcd.o lcd_console.o > >  endif > > @@ -102,7 +100,6 @@ endif > >  obj-y += image.o > >  obj-$(CONFIG_ANDROID_BOOT_IMAGE) += image-android.o > >  obj-$(CONFIG_$(SPL_TPL_)OF_LIBFDT) += image-fdt.o > > -obj-$(CONFIG_$(SPL_TPL_)FIT) += image-fit.o > >  obj-$(CONFIG_FIT_EMBED) += boot_fit.o common_fit.o > >  obj-$(CONFIG_$(SPL_TPL_)FIT_SIGNATURE) += image-sig.o > >  obj-$(CONFIG_IO_TRACE) += iotrace.o > > @@ -130,3 +127,7 @@ obj-$(CONFIG_CMD_DFU) += dfu.o > >  obj-y += command.o > >  obj-y += s_record.o > >  obj-y += xyzModem.o > > + > > +obj-y += splash.o > > +obj-$(CONFIG_SPLASH_SOURCE) += splash_source.o > > +obj-$(CONFIG_$(SPL_TPL_)FIT) += image-fit.o > > diff --git a/common/splash_source.c b/common/splash_source.c > > index e0defde..1413945 100644 > > --- a/common/splash_source.c > > +++ b/common/splash_source.c > > @@ -109,7 +109,7 @@ splash_address_too_high: > >   return -EFAULT; > >  } > >   > > -static int splash_select_fs_dev(struct splash_location *location) > > +int splash_select_fs_dev(struct splash_location *location) > >  { > >   int res; > >   > > @@ -140,6 +140,7 @@ static int splash_select_fs_dev(struct > > splash_location *location) > >   return res; > >  } > >   > > +#ifndef CONFIG_SPL_BUILD > >  #ifdef CONFIG_USB_STORAGE > >  static int splash_init_usb(void) > >  { > > @@ -175,6 +176,7 @@ static inline int splash_init_sata(void) > >   return -ENOSYS; > >  } > >  #endif > > +#endif > >   > >  #ifdef CONFIG_CMD_UBIFS > >  static int splash_mount_ubifs(struct splash_location *location) > > @@ -213,22 +215,99 @@ static inline int splash_umount_ubifs(void) > >   > >  #define SPLASH_SOURCE_DEFAULT_FILE_NAME "splash.bmp > > " > >   > > -static int splash_load_fs(struct splash_location *location, u32 > > bmp_load_addr) > > +/** > > + * fsloader_preprocess - Any prepocessing before calling > > filesystem loader. > > + * > > + * @locations: An array of supported splash > > locations. > > + * @file_info: Description and attributes to the > > image. > > + * Could be structure pointer, and any type > > pointer. > > + * @filename: Image filename in flashes. > > + * @bmp_load_addr: Target memory location image loaded to. > > + * > > + * @return: If 0, processing is succesfull. Filename > > pointer contains > > + * valid filename. > > + * If -ve, processing is failed. > > + */ > > +__weak int fsloader_preprocess(struct splash_location *location, > > +        void *file_info, char **filename, > > +        u32 bmp_load_addr) > >  { > >   int res = 0; > >   loff_t bmp_size; > > - loff_t actread; > >   char *splash_file; > >   > > - splash_file = env_get("splashfile"); > > + splash_file = env_get((char *)file_info); > > + > >   if (!splash_file) > >   splash_file = SPLASH_SOURCE_DEFAULT_FILE_NAME; > >   > > + res = splash_select_fs_dev(location); > > + if (res) { > > + error("Error : Failed to select FS device\n"); > > + return -EPERM; > > + } > > + > > + res = fs_size(splash_file, &bmp_size); > > + if (res) { > > + error("Error (%d): cannot determine file size\n", > > res); > > + return -ENOENT; > > + } > > + > > + if ((u32)bmp_load_addr + bmp_size >= gd->start_addr_sp) { > > + error("Error: splashimage address too high. Data > > overwrites "); > > + error("U-Boot and/or placed beyond DRAM > > boundaries.\n"); > > + res = -EFAULT; > > + return -EFAULT; > > + } > > + > > + *filename = splash_file; > > + > > + return res; > > +} > > + > > +/** > > + * fs_loading - This place is for implementing whaterver blob + > > whatever > > + * specific driver to the HW such as program raw > > binary file to > > + * FPGA. > > + * > > + * @locations: An array of supported splash > > locations. > > + * @file_info: Description and attributes to the > > image. > > + * Could be structure pointer, and any type > > pointer. > > + * @filename: Image filename in flashes. > > + * @bmp_load_addr: Target memory location image loaded to. > > + * @bsize: Size of target memory location. > > + * > > + * @return: If 0, loading is succesfull. Filename pointer > > contains > > + * valid filename. > > + * If non-zero, loading is failed. > > + */ > > +__weak int fs_loading(struct splash_location *location, void > > *file_info, > > +       char *filename, u32 bmp_load_addr, size_t > > bsize) > > +{ > > + loff_t actread; > > + > > + return fs_read(filename, (u32)bmp_load_addr, 0, 0, > > &actread); > > +} > > + > > +int splash_load_fs(struct splash_location *location, void > > *file_info, > > +    u32 bmp_load_addr, size_t bsize) > > +{ > > + int res = 0; > > + char *splash_file = NULL; > > + > > + res = fsloader_preprocess(location, file_info, > > &splash_file, > > +   bmp_load_addr); > > + > > + if (res) > > + goto out; > > + > > +#ifndef CONFIG_SPL_BUILD > >   if (location->storage == SPLASH_STORAGE_USB) > >   res = splash_init_usb(); > >   > >   if (location->storage == SPLASH_STORAGE_SATA) > >   res = splash_init_sata(); > > +#endif > >   > >   if (location->ubivol != NULL) > >   res = splash_mount_ubifs(location); > > @@ -236,24 +315,8 @@ static int splash_load_fs(struct > > splash_location *location, u32 bmp_load_addr) > >   if (res) > >   return res; > >   > > - res = splash_select_fs_dev(location); > > - if (res) > > - goto out; > > - > > - res = fs_size(splash_file, &bmp_size); > > - if (res) { > > - printf("Error (%d): cannot determine file size\n", > > res); > > - goto out; > > - } > > - > > - if (bmp_load_addr + bmp_size >= gd->start_addr_sp) { > > - printf("Error: splashimage address too high. Data > > overwrites U-Boot and/or placed beyond DRAM boundaries.\n"); > > - res = -EFAULT; > > - goto out; > > - } > > - > > - splash_select_fs_dev(location); > > - res = fs_read(splash_file, bmp_load_addr, 0, 0, &actread); > > + res = fs_loading(location, file_info, splash_file, > > bmp_load_addr, > > + bsize); > >   > >  out: > >   if (location->ubivol != NULL) > > @@ -405,7 +468,8 @@ int splash_source_load(struct splash_location > > *locations, uint size) > >   if (splash_location->flags == SPLASH_STORAGE_RAW) > >   return splash_load_raw(splash_location, > > bmp_load_addr); > >   else if (splash_location->flags == SPLASH_STORAGE_FS) > > - return splash_load_fs(splash_location, > > bmp_load_addr); > > + return splash_load_fs(splash_location, > > "splashfile", > > +       bmp_load_addr, 0); > >  #ifdef CONFIG_FIT > >   else if (splash_location->flags == SPLASH_STORAGE_FIT) > >   return splash_load_fit(splash_location, > > bmp_load_addr); > > diff --git a/configs/socfpga_arria10_defconfig > > b/configs/socfpga_arria10_defconfig > > index 4c73d73..2db34b5 100644 > > --- a/configs/socfpga_arria10_defconfig > > +++ b/configs/socfpga_arria10_defconfig > > @@ -7,6 +7,8 @@ > > CONFIG_DEFAULT_DEVICE_TREE="socfpga_arria10_socdk_sdmmc" > >  CONFIG_USE_BOOTARGS=y > >  CONFIG_BOOTARGS="console=ttyS0,115200" > >  CONFIG_DEFAULT_FDT_FILE="socfpga_arria10_socdk_sdmmc.dtb" > > +CONFIG_FIT=y > > +CONFIG_SPL_FIT=y > >  CONFIG_SPL=y > >  CONFIG_SPL_FPGA_SUPPORT=y > >  CONFIG_CMD_BOOTZ=y > > diff --git a/include/configs/socfpga_arria10_socdk.h > > b/include/configs/socfpga_arria10_socdk.h > > index 83718dd..9566d46 100644 > > --- a/include/configs/socfpga_arria10_socdk.h > > +++ b/include/configs/socfpga_arria10_socdk.h > > @@ -47,6 +47,9 @@ > >   */ > >  #define CONFIG_SYS_MAX_FLASH_BANKS     1 > >   > > +/* Generic FS loader */ > > +#define CONFIG_SPLASH_SOURCE > > + > >  /* The rest of the configuration is shared */ > >  #include > >   > > diff --git a/include/splash.h b/include/splash.h > > index 228aff4..83a6890 100644 > > --- a/include/splash.h > > +++ b/include/splash.h > > @@ -59,6 +59,9 @@ static inline int splash_source_load(struct > > splash_location *locations, > >  #endif > >   > >  int splash_screen_prepare(void); > > +int splash_select_fs_dev(struct splash_location *location); > > +int splash_load_fs(struct splash_location *location, void > > *file_info, > > +    u32 bmp_load_addr, size_t bsize); > >   > >  #ifdef CONFIG_SPLASH_SCREEN_ALIGN > >  void splash_get_pos(int *x, int *y); > > >