[U-Boot] [PATCH v4 06/20] common: Generic firmware loader for file system

[Chee, Tien Fong <tien.fong.chee@intel.com>]

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 <tien.fong.chee@intel.com>
> > 
> > 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 <tien.fong.chee@intel.com>
> 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 <configs/socfpga_common.h>
> >  
> > 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);
> > 
>