[U-Boot] [PATCH 0/2] Creating generic file system firmware loader
From: Tien Fong Chee tien.fong.chee@intel.com
Factoring out common file system loader functionality from splash loader as generic file system firmware loader. Convert splash file system loader portion and its dependency files to use generic file system firmware loader. The generic file system firmware loader supports file image loading from flash into target location, and its specific HW driver processing file image before calling FS loading and in the process of loading into target location can be defined in both weak functions fsloader_preprocess and fs_loading.
This series is working on top of u-boot-socfpga.git - http://git.denx.de/u-boot-socfpga.git .
Tien Fong Chee (2): common: Generic file system firmware loader common: Convert splash FS loader to use generic FS firmware loader
board/compulab/cm_fx6/cm_fx6.c | 17 ++-- board/compulab/cm_t35/cm_t35.c | 6 +- common/Makefile | 1 + common/load_fs.c | 217 +++++++++++++++++++++++++++++++++++++++++ common/splash.c | 18 ++-- common/splash_source.c | 193 +++++++++--------------------------- include/load_fs.h | 38 ++++++++ include/splash.h | 29 +----- 8 files changed, 323 insertions(+), 196 deletions(-) create mode 100644 common/load_fs.c create mode 100644 include/load_fs.h
From: Tien Fong Chee tien.fong.chee@intel.com
Generic firmware loader framework contains some common functionality which is factored out from splash loader. It 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 --- common/Makefile | 1 + common/load_fs.c | 217 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ include/load_fs.h | 38 ++++++++++ 3 files changed, 256 insertions(+) create mode 100644 common/load_fs.c create mode 100644 include/load_fs.h
diff --git a/common/Makefile b/common/Makefile index 801ea31..89f9365 100644 --- a/common/Makefile +++ b/common/Makefile @@ -130,3 +130,4 @@ obj-$(CONFIG_CMD_DFU) += dfu.o obj-y += command.o obj-y += s_record.o obj-y += xyzModem.o +obj-y += load_fs.o diff --git a/common/load_fs.c b/common/load_fs.c new file mode 100644 index 0000000..112b4f6 --- /dev/null +++ b/common/load_fs.c @@ -0,0 +1,217 @@ +/* + * Copyright (C) 2017 Intel Corporation <www.intel.com> + * + * SPDX-License-Identifier: GPL-2.0 + */ + +#include <common.h> +#include <errno.h> +#include <fs.h> +#include <load_fs.h> +#include <nand.h> +#include <sata.h> +#include <spi.h> +#include <spi_flash.h> +#include <usb.h> + +int flash_select_fs_dev(struct flash_location *location) +{ + int res; + + switch (location->storage) { + case FLASH_STORAGE_MMC: + res = fs_set_blk_dev("mmc", location->devpart, FS_TYPE_ANY); + break; + case FLASH_STORAGE_USB: + res = fs_set_blk_dev("usb", location->devpart, FS_TYPE_ANY); + break; + case FLASH_STORAGE_SATA: + res = fs_set_blk_dev("sata", location->devpart, FS_TYPE_ANY); + break; + case FLASH_STORAGE_NAND: + if (location->ubivol != NULL) + res = fs_set_blk_dev("ubi", NULL, FS_TYPE_UBIFS); + else + res = -ENODEV; + break; + default: + error("Error: unsupported location storage.\n"); + return -ENODEV; + } + + if (res) + error("Error: could not access storage.\n"); + + return res; +} + +#ifndef CONFIG_SPL_BUILD +#ifdef CONFIG_USB_STORAGE +static int flash_init_usb(void) +{ + int err; + + err = usb_init(); + if (err) + return err; + +#ifndef CONFIG_DM_USB + err = usb_stor_scan(1) < 0 ? -ENODEV : 0; +#endif + + return err; +} +#else +static inline int flash_init_usb(void) +{ + error("Error: Cannot load flash image: no USB support\n"); + return -ENOSYS; +} +#endif +#endif + +#ifdef CONFIG_SATA +static int flash_init_sata(void) +{ + return sata_probe(0); +} +#else +static inline int flash_init_sata(void) +{ + error("Error: Cannot load flash image: no SATA support\n"); + return -ENOSYS; +} +#endif + +#ifdef CONFIG_CMD_UBIFS +static int flash_mount_ubifs(struct flash_location *location) +{ + int res; + char cmd[32]; + + sprintf(cmd, "ubi part %s", location->mtdpart); + res = run_command(cmd, 0); + if (res) + return res; + + sprintf(cmd, "ubifsmount %s", location->ubivol); + res = run_command(cmd, 0); + + return res; +} + +static inline int flash_umount_ubifs(void) +{ + return run_command("ubifsumount", 0); +} +#else +static inline int flash_mount_ubifs(struct flash_location *location) +{ + error("Error: Cannot load flash image: no UBIFS support\n"); + return -ENOSYS; +} + +static inline int flash_umount_ubifs(void) +{ + error("Error: Cannot unmount UBIFS: no UBIFS support\n"); + return -ENOSYS; +} +#endif + +/** + * fsloader_preprocess - Any prepocessing before calling filesystem loader such + * getting filename, and flash partition information. + * + * @locations: An array of supported flash locations. Contains default + * flash setting for the file image. + * @file_info: Description and attributes to the image. + * Could be structure pointer, and any type pointer. + * @filename: Image filename in flashes. Storing the file image name + * to this pointer after retriving the name from DTB, + * or environment or source code. + * @load_addr: Target 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 flash_location *location, + void *file_info, char **filename, + u32 load_addr) +{ + return 0; +} + +/* + * fs_loading - Place for implementing whatever specific driver to + * the target HW such as program raw binary file to FPGA. + * + * @locations: An array of supported flash locations. Contains default + * flash setting for the file image. + * @file_info: Description and attributes to the image. + * Could be structure pointer, and any type pointer. + * @filename: Image filename in flashes. + * @load_addr: Target location image loaded to. + * @bsize: Size of target location. + * + * @return: If 0, loading is succesfull. filename pointer contains + * valid filename. + * If non-zero, loading is failed. + */ +__weak int fs_loading(struct flash_location *location, void *file_info, + char *filename, u32 load_addr, size_t bsize) +{ + return 0; +} + +/* + * flash_load_fs - Generic filesystem firmware loader, this should be called + * for any loader with filesystem. + * + * @locations: An array of supported flash locations. Contains default + * flash setting for the file image. + * @file_info: Description and attributes to the file image. + * Could be structure pointer, and any type pointer. + * @load_addr: Target location file image loaded to. + * @bsize: Size of target location. + * + * @return: If 0, loading is succesfull. + * If non-zero, loading is failed. + */ +int flash_load_fs(struct flash_location *location, void *file_info, + u32 load_addr, size_t bsize) +{ + int res = 0; + char *flash_file = NULL; + + res = fsloader_preprocess(location, file_info, &flash_file, + load_addr); + + if (res) + goto out; + +#ifndef CONFIG_SPL_BUILD + /* Loading from USB is not supported yet in SPL */ + if (location->storage == FLASH_STORAGE_USB) + res = flash_init_usb(); +#endif + + if (location->storage == FLASH_STORAGE_SATA) + res = flash_init_sata(); + + if (location->ubivol != NULL) + res = flash_mount_ubifs(location); + + if (res) + return res; + + res = fs_loading(location, file_info, flash_file, load_addr, + bsize); + +out: + if (location->ubivol != NULL) + flash_umount_ubifs(); + + return res; +} + diff --git a/include/load_fs.h b/include/load_fs.h new file mode 100644 index 0000000..dbde8d0 --- /dev/null +++ b/include/load_fs.h @@ -0,0 +1,38 @@ +/* + * Copyright (C) 2017 Intel Corporation <www.intel.com> + * + * SPDX-License-Identifier: GPL-2.0 + */ + + +#ifndef _LOAD_FS_H_ +#define _LOAD_FS_H_ + +enum flash_storage { + FLASH_STORAGE_NAND, + FLASH_STORAGE_SF, + FLASH_STORAGE_MMC, + FLASH_STORAGE_USB, + FLASH_STORAGE_SATA, +}; + +enum flash_flags { + FLASH_STORAGE_RAW, /* Stored in raw memory */ + FLASH_STORAGE_FS, /* Stored within a file system */ + FLASH_STORAGE_FIT, /* Stored inside a FIT image */ +}; + +struct flash_location { + char *name; + enum flash_storage storage; + enum flash_flags flags; + u32 offset; /* offset from start of storage */ + char *devpart; /* Use the load command dev:part conventions */ + char *mtdpart; /* MTD partition for ubi part */ + char *ubivol; /* UBI volume-name for ubifsmount */ +}; + +int flash_load_fs(struct flash_location *location, void *file_info, + u32 load_addr, size_t bsize); +int flash_select_fs_dev(struct flash_location *location); +#endif
On 11/01/2017 10:18 AM, tien.fong.chee@intel.com wrote:
From: Tien Fong Chee tien.fong.chee@intel.com
Generic firmware loader framework contains some common functionality which is factored out from splash loader. It 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
common/Makefile | 1 + common/load_fs.c | 217 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ include/load_fs.h | 38 ++++++++++ 3 files changed, 256 insertions(+) create mode 100644 common/load_fs.c create mode 100644 include/load_fs.h
[...]
+int flash_select_fs_dev(struct flash_location *location)
Why does everything have flash_ prefix ?
I also mentioned the API should copy the linux firmware loader API.
+{
- int res;
- switch (location->storage) {
- case FLASH_STORAGE_MMC:
res = fs_set_blk_dev("mmc", location->devpart, FS_TYPE_ANY);break;- case FLASH_STORAGE_USB:
res = fs_set_blk_dev("usb", location->devpart, FS_TYPE_ANY);break;- case FLASH_STORAGE_SATA:
res = fs_set_blk_dev("sata", location->devpart, FS_TYPE_ANY);break;- case FLASH_STORAGE_NAND:
if (location->ubivol != NULL)res = fs_set_blk_dev("ubi", NULL, FS_TYPE_UBIFS);elseres = -ENODEV;break;- default:
error("Error: unsupported location storage.\n");return -ENODEV;- }
- if (res)
error("Error: could not access storage.\n");- return res;
+}
+#ifndef CONFIG_SPL_BUILD +#ifdef CONFIG_USB_STORAGE
This looks wrong, the USB can be supported in SPL no problem. And this USB init shouldn't be duplicated here IMO.
+static int flash_init_usb(void) +{
- int err;
- err = usb_init();
- if (err)
return err;+#ifndef CONFIG_DM_USB
- err = usb_stor_scan(1) < 0 ? -ENODEV : 0;
+#endif
- return err;
+} +#else +static inline int flash_init_usb(void)
Don't use inline , the compiler can decide better.
[...]
On Rab, 2017-11-01 at 10:26 +0100, Marek Vasut wrote:
On 11/01/2017 10:18 AM, tien.fong.chee@intel.com wrote:
From: Tien Fong Chee tien.fong.chee@intel.com
Generic firmware loader framework contains some common functionality which is factored out from splash loader. It 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
common/Makefile | 1 + common/load_fs.c | 217 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ include/load_fs.h | 38 ++++++++++ 3 files changed, 256 insertions(+) create mode 100644 common/load_fs.c create mode 100644 include/load_fs.h
[...]
+int flash_select_fs_dev(struct flash_location *location)
Why does everything have flash_ prefix ?
I can remove the flash_ prefix, this generic FS loader should support for all filesystem instead of flash.
I also mentioned the API should copy the linux firmware loader API.
If i'm not mistaken, you are referring firmware loader API in this link https://github.com/torvalds/linux/blob/f007cad159e99fa2acd3b2e9364 fbb32ad28b971/drivers/base/firmware_class.c#L1264.
Actually we have almost same framework in filesystem loader portion, just different implementation, and Linux firmware loader is more specific to Linux environment such as hard code path searching in RFS. The generic FS loader in this patch is much more flexible, let user to define their own prefer implementation. Linux FS firmware loader <---> U-Boot FS firmware loader -------------------------- --------------------------- 1) request_firmware flash_load_fs 2) _request_firmware_prepare weak fsloader_preprocess 3) fw_get_filesystem_firmware weak fs_loading
- int res;
- switch (location->storage) {
- case FLASH_STORAGE_MMC:
res = fs_set_blk_dev("mmc", location->devpart,FS_TYPE_ANY);
break;- case FLASH_STORAGE_USB:
res = fs_set_blk_dev("usb", location->devpart,FS_TYPE_ANY);
break;- case FLASH_STORAGE_SATA:
res = fs_set_blk_dev("sata", location->devpart,FS_TYPE_ANY);
break;- case FLASH_STORAGE_NAND:
if (location->ubivol != NULL)res = fs_set_blk_dev("ubi", NULL,FS_TYPE_UBIFS);
elseres = -ENODEV;break;- default:
error("Error: unsupported location storage.\n");return -ENODEV;- }
- if (res)
error("Error: could not access storage.\n");- return res;
+}
+#ifndef CONFIG_SPL_BUILD +#ifdef CONFIG_USB_STORAGE
This looks wrong, the USB can be supported in SPL no problem. And this
Technically, USB can be supported in SPL, but the build for USB in SPL is not supported yet.
USB init shouldn't be duplicated here IMO.
This is just for the case USB init is not yet started, but loader is called 1st.
+static int flash_init_usb(void) +{
- int err;
- err = usb_init();
- if (err)
return err;+#ifndef CONFIG_DM_USB
- err = usb_stor_scan(1) < 0 ? -ENODEV : 0;
+#endif
- return err;
+} +#else +static inline int flash_init_usb(void)
Don't use inline , the compiler can decide better.
Okay.
[...]
On 11/02/2017 09:20 AM, Chee, Tien Fong wrote:
On Rab, 2017-11-01 at 10:26 +0100, Marek Vasut wrote:
On 11/01/2017 10:18 AM, tien.fong.chee@intel.com wrote:
From: Tien Fong Chee tien.fong.chee@intel.com
Generic firmware loader framework contains some common functionality which is factored out from splash loader. It 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
common/Makefile | 1 + common/load_fs.c | 217 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ include/load_fs.h | 38 ++++++++++ 3 files changed, 256 insertions(+) create mode 100644 common/load_fs.c create mode 100644 include/load_fs.h
[...]
+int flash_select_fs_dev(struct flash_location *location)
Why does everything have flash_ prefix ?
I can remove the flash_ prefix, this generic FS loader should support for all filesystem instead of flash.
I also mentioned the API should copy the linux firmware loader API.
If i'm not mistaken, you are referring firmware loader API in this link https://github.com/torvalds/linux/blob/f007cad159e99fa2acd3b2e9364 fbb32ad28b971/drivers/base/firmware_class.c#L1264.
Actually we have almost same framework in filesystem loader portion, just different implementation, and Linux firmware loader is more specific to Linux environment such as hard code path searching in RFS. The generic FS loader in this patch is much more flexible, let user to define their own prefer implementation. Linux FS firmware loader <---> U-Boot FS firmware loader -------------------------- ---------------------------
- request_firmware flash_load_fs
- _request_firmware_prepare weak fsloader_preprocess
- fw_get_filesystem_firmware weak fs_loading
The API should be the same or very similar to make porting of drivers from Linux easy and allow people to know only one API, not two.
- int res;
- switch (location->storage) {
- case FLASH_STORAGE_MMC:
res = fs_set_blk_dev("mmc", location->devpart,FS_TYPE_ANY);
break;- case FLASH_STORAGE_USB:
res = fs_set_blk_dev("usb", location->devpart,FS_TYPE_ANY);
break;- case FLASH_STORAGE_SATA:
res = fs_set_blk_dev("sata", location->devpart,FS_TYPE_ANY);
break;- case FLASH_STORAGE_NAND:
if (location->ubivol != NULL)res = fs_set_blk_dev("ubi", NULL,FS_TYPE_UBIFS);
elseres = -ENODEV;break;- default:
error("Error: unsupported location storage.\n");return -ENODEV;- }
- if (res)
error("Error: could not access storage.\n");- return res;
+}
+#ifndef CONFIG_SPL_BUILD +#ifdef CONFIG_USB_STORAGE
This looks wrong, the USB can be supported in SPL no problem. And this
Technically, USB can be supported in SPL, but the build for USB in SPL is not supported yet.
USB init shouldn't be duplicated here IMO.
This is just for the case USB init is not yet started, but loader is called 1st.
I am not asking WHY this is needed. I suspect we have this code somewhere already, so it's a duplicate here.
On Ahd, 2017-11-05 at 17:43 +0100, Marek Vasut wrote:
On 11/02/2017 09:20 AM, Chee, Tien Fong wrote:
On Rab, 2017-11-01 at 10:26 +0100, Marek Vasut wrote:
On 11/01/2017 10:18 AM, tien.fong.chee@intel.com wrote:
From: Tien Fong Chee tien.fong.chee@intel.com
Generic firmware loader framework contains some common functionality which is factored out from splash loader. It 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
common/Makefile | 1 + common/load_fs.c | 217 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ include/load_fs.h | 38 ++++++++++ 3 files changed, 256 insertions(+) create mode 100644 common/load_fs.c create mode 100644 include/load_fs.h
[...]
+int flash_select_fs_dev(struct flash_location *location)
Why does everything have flash_ prefix ?
I can remove the flash_ prefix, this generic FS loader should support for all filesystem instead of flash.
I also mentioned the API should copy the linux firmware loader API.
If i'm not mistaken, you are referring firmware loader API in this link https://github.com/torvalds/linux/blob/f007cad159e99fa2acd3b2e 9364 fbb32ad28b971/drivers/base/firmware_class.c#L1264.
I would like to confirm with you whether we are talking to the same API above?
Actually we have almost same framework in filesystem loader portion, just different implementation, and Linux firmware loader is more specific to Linux environment such as hard code path searching in RFS. The generic FS loader in this patch is much more flexible, let user to define their own prefer implementation. Linux FS firmware loader <---> U-Boot FS firmware loader -------------------------- ---------------------------
- request_firmware flash_load_fs
- _request_firmware_prepare weak fsloader_preprocess
- fw_get_filesystem_firmware weak fs_loading
The API should be the same or very similar to make porting of drivers from Linux easy and allow people to know only one API, not two.
- int res;
- switch (location->storage) {
- case FLASH_STORAGE_MMC:
res = fs_set_blk_dev("mmc", location->devpart,FS_TYPE_ANY);
break;- case FLASH_STORAGE_USB:
res = fs_set_blk_dev("usb", location->devpart,FS_TYPE_ANY);
break;- case FLASH_STORAGE_SATA:
res = fs_set_blk_dev("sata", location-devpart,
FS_TYPE_ANY);
break;- case FLASH_STORAGE_NAND:
if (location->ubivol != NULL)res = fs_set_blk_dev("ubi", NULL,FS_TYPE_UBIFS);
elseres = -ENODEV;break;- default:
error("Error: unsupported locationstorage.\n");
return -ENODEV;- }
- if (res)
error("Error: could not access storage.\n");- return res;
+}
+#ifndef CONFIG_SPL_BUILD +#ifdef CONFIG_USB_STORAGE
This looks wrong, the USB can be supported in SPL no problem. And this
Technically, USB can be supported in SPL, but the build for USB in SPL is not supported yet.
USB init shouldn't be duplicated here IMO.
This is just for the case USB init is not yet started, but loader is called 1st.
I am not asking WHY this is needed. I suspect we have this code somewhere already, so it's a duplicate here.
On 11/06/2017 05:15 AM, Chee, Tien Fong wrote:
On Ahd, 2017-11-05 at 17:43 +0100, Marek Vasut wrote:
On 11/02/2017 09:20 AM, Chee, Tien Fong wrote:
On Rab, 2017-11-01 at 10:26 +0100, Marek Vasut wrote:
On 11/01/2017 10:18 AM, tien.fong.chee@intel.com wrote:
From: Tien Fong Chee tien.fong.chee@intel.com
Generic firmware loader framework contains some common functionality which is factored out from splash loader. It 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
common/Makefile | 1 + common/load_fs.c | 217 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ include/load_fs.h | 38 ++++++++++ 3 files changed, 256 insertions(+) create mode 100644 common/load_fs.c create mode 100644 include/load_fs.h
[...]
+int flash_select_fs_dev(struct flash_location *location)
Why does everything have flash_ prefix ?
I can remove the flash_ prefix, this generic FS loader should support for all filesystem instead of flash.
I also mentioned the API should copy the linux firmware loader API.
If i'm not mistaken, you are referring firmware loader API in this link https://github.com/torvalds/linux/blob/f007cad159e99fa2acd3b2e 9364 fbb32ad28b971/drivers/base/firmware_class.c#L1264.
I would like to confirm with you whether we are talking to the same API above?
https://www.kernel.org/doc/html/v4.13/driver-api/firmware/index.html
first link on google btw . You might be able to avoid the firmware structure.
[...]
On Isn, 2017-11-06 at 11:56 +0100, Marek Vasut wrote:
On 11/06/2017 05:15 AM, Chee, Tien Fong wrote:
On Ahd, 2017-11-05 at 17:43 +0100, Marek Vasut wrote:
On 11/02/2017 09:20 AM, Chee, Tien Fong wrote:
On Rab, 2017-11-01 at 10:26 +0100, Marek Vasut wrote:
On 11/01/2017 10:18 AM, tien.fong.chee@intel.com wrote:
From: Tien Fong Chee tien.fong.chee@intel.com
Generic firmware loader framework contains some common functionality which is factored out from splash loader. It 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
common/Makefile | 1 + common/load_fs.c | 217 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ include/load_fs.h | 38 ++++++++++ 3 files changed, 256 insertions(+) create mode 100644 common/load_fs.c create mode 100644 include/load_fs.h
[...]
+int flash_select_fs_dev(struct flash_location *location)
Why does everything have flash_ prefix ?
I can remove the flash_ prefix, this generic FS loader should support for all filesystem instead of flash.
I also mentioned the API should copy the linux firmware loader API.
If i'm not mistaken, you are referring firmware loader API in this link https://github.com/torvalds/linux/blob/f007cad159e99fa2acd 3b2e 9364 fbb32ad28b971/drivers/base/firmware_class.c#L1264.
I would like to confirm with you whether we are talking to the same API above?
https://www.kernel.org/doc/html/v4.13/driver-api/firmware/index.html
first link on google btw . You might be able to avoid the firmware structure.
After assessment, i found that Linux loader is not suitable for fpga loader as fpga loader need 1) Able to program FPGA image in SPL chunk bu chunk with small memory allocatted. 2) Name of FPGA image defined in DTS, and path of FPGA image in FAT and UBI partition.
Linux loader is strongly designed based on Linux environment, always assume having RFF, env support(which SPL don't have), sysfs and udev feature.
Thanks.
[...]
On 11/07/2017 10:03 AM, Chee, Tien Fong wrote:
On Isn, 2017-11-06 at 11:56 +0100, Marek Vasut wrote:
On 11/06/2017 05:15 AM, Chee, Tien Fong wrote:
On Ahd, 2017-11-05 at 17:43 +0100, Marek Vasut wrote:
On 11/02/2017 09:20 AM, Chee, Tien Fong wrote:
On Rab, 2017-11-01 at 10:26 +0100, Marek Vasut wrote:
On 11/01/2017 10:18 AM, tien.fong.chee@intel.com wrote: > > > > From: Tien Fong Chee tien.fong.chee@intel.com > > Generic firmware loader framework contains some common > functionality > which is factored out from splash loader. It 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 > --- > common/Makefile | 1 + > common/load_fs.c | 217 > ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > include/load_fs.h | 38 ++++++++++ > 3 files changed, 256 insertions(+) > create mode 100644 common/load_fs.c > create mode 100644 include/load_fs.h [...]
> > > > +int flash_select_fs_dev(struct flash_location *location) Why does everything have flash_ prefix ?
I can remove the flash_ prefix, this generic FS loader should support for all filesystem instead of flash.
I also mentioned the API should copy the linux firmware loader API.
If i'm not mistaken, you are referring firmware loader API in this link https://github.com/torvalds/linux/blob/f007cad159e99fa2acd 3b2e 9364 fbb32ad28b971/drivers/base/firmware_class.c#L1264.
I would like to confirm with you whether we are talking to the same API above?
https://www.kernel.org/doc/html/v4.13/driver-api/firmware/index.html
first link on google btw . You might be able to avoid the firmware structure.
After assessment, i found that Linux loader is not suitable for fpga loader as fpga loader need
- Able to program FPGA image in SPL chunk bu chunk with small memory
allocatted. 2) Name of FPGA image defined in DTS, and path of FPGA image in FAT and UBI partition.
Linux loader is strongly designed based on Linux environment, always assume having RFF, env support(which SPL don't have), sysfs and udev feature.
Sigh, you can just have some additional function call to fetch smaller chunks from a file, I don't think it's that hard of a problem ...
On Sel, 2017-11-07 at 10:34 +0100, Marek Vasut wrote:
On 11/07/2017 10:03 AM, Chee, Tien Fong wrote:
On Isn, 2017-11-06 at 11:56 +0100, Marek Vasut wrote:
On 11/06/2017 05:15 AM, Chee, Tien Fong wrote:
On Ahd, 2017-11-05 at 17:43 +0100, Marek Vasut wrote:
On 11/02/2017 09:20 AM, Chee, Tien Fong wrote:
On Rab, 2017-11-01 at 10:26 +0100, Marek Vasut wrote: > > > > On 11/01/2017 10:18 AM, tien.fong.chee@intel.com wrote: > > > > > > > > > > From: Tien Fong Chee tien.fong.chee@intel.com > > > > Generic firmware loader framework contains some common > > functionality > > which is factored out from splash loader. It 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 > > > > > --- > > common/Makefile | 1 + > > common/load_fs.c | 217 > > ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > include/load_fs.h | 38 ++++++++++ > > 3 files changed, 256 insertions(+) > > create mode 100644 common/load_fs.c > > create mode 100644 include/load_fs.h > [...] > > > > > > > > > > > +int flash_select_fs_dev(struct flash_location > > *location) > Why does everything have flash_ prefix ? > I can remove the flash_ prefix, this generic FS loader should support for all filesystem instead of flash.
> > > > I also mentioned the API should copy the linux firmware > loader > API. > If i'm not mistaken, you are referring firmware loader API in this link https://github.com/torvalds/linux/blob/f007cad159e99fa 2acd 3b2e 9364 fbb32ad28b971/drivers/base/firmware_class.c#L1264.
I would like to confirm with you whether we are talking to the same API above?
https://www.kernel.org/doc/html/v4.13/driver-api/firmware/index.h tml
first link on google btw . You might be able to avoid the firmware structure.
After assessment, i found that Linux loader is not suitable for fpga loader as fpga loader need
- Able to program FPGA image in SPL chunk bu chunk with small
memory allocatted. 2) Name of FPGA image defined in DTS, and path of FPGA image in FAT and UBI partition.
Linux loader is strongly designed based on Linux environment, always assume having RFF, env support(which SPL don't have), sysfs and udev feature.
Sigh, you can just have some additional function call to fetch smaller chunks from a file, I don't think it's that hard of a problem ...
We already have that function to support smaller chunks, and it also work for single large image when enough memory is available in ver 1 series of fpga loadfs.
Since the Linux loader API is totally not suitable for fpga loadfs, and also nothing i can leverage from there for fpga loadfs, could you please consider to accept implementation for this series patches or implementation for fpga loadfs series ver1?
Thanks.
On 11/09/2017 07:04 AM, Chee, Tien Fong wrote:
On Sel, 2017-11-07 at 10:34 +0100, Marek Vasut wrote:
On 11/07/2017 10:03 AM, Chee, Tien Fong wrote:
On Isn, 2017-11-06 at 11:56 +0100, Marek Vasut wrote:
On 11/06/2017 05:15 AM, Chee, Tien Fong wrote:
On Ahd, 2017-11-05 at 17:43 +0100, Marek Vasut wrote:
On 11/02/2017 09:20 AM, Chee, Tien Fong wrote: > > > > On Rab, 2017-11-01 at 10:26 +0100, Marek Vasut wrote: >> >> >> >> On 11/01/2017 10:18 AM, tien.fong.chee@intel.com wrote: >>> >>> >>> >>> >>> From: Tien Fong Chee tien.fong.chee@intel.com >>> >>> Generic firmware loader framework contains some common >>> functionality >>> which is factored out from splash loader. It 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 >>>> >>> --- >>> common/Makefile | 1 + >>> common/load_fs.c | 217 >>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++ >>> include/load_fs.h | 38 ++++++++++ >>> 3 files changed, 256 insertions(+) >>> create mode 100644 common/load_fs.c >>> create mode 100644 include/load_fs.h >> [...] >> >>> >>> >>> >>> >>> +int flash_select_fs_dev(struct flash_location >>> *location) >> Why does everything have flash_ prefix ? >> > I can remove the flash_ prefix, this generic FS loader > should > support > for all filesystem instead of flash. > >> >> >> >> I also mentioned the API should copy the linux firmware >> loader >> API. >> > If i'm not mistaken, you are referring firmware loader API > in > this > link https://github.com/torvalds/linux/blob/f007cad159e99fa > 2acd > 3b2e > 9364 > fbb32ad28b971/drivers/base/firmware_class.c#L1264. >
I would like to confirm with you whether we are talking to the same API above?
https://www.kernel.org/doc/html/v4.13/driver-api/firmware/index.h tml
first link on google btw . You might be able to avoid the firmware structure.
After assessment, i found that Linux loader is not suitable for fpga loader as fpga loader need
- Able to program FPGA image in SPL chunk bu chunk with small
memory allocatted. 2) Name of FPGA image defined in DTS, and path of FPGA image in FAT and UBI partition.
Linux loader is strongly designed based on Linux environment, always assume having RFF, env support(which SPL don't have), sysfs and udev feature.
Sigh, you can just have some additional function call to fetch smaller chunks from a file, I don't think it's that hard of a problem ...
We already have that function to support smaller chunks, and it also work for single large image when enough memory is available in ver 1 series of fpga loadfs.
Since the Linux loader API is totally not suitable for fpga loadfs, and also nothing i can leverage from there for fpga loadfs, could you please consider to accept implementation for this series patches or implementation for fpga loadfs series ver1?
You mean going back to completely custom non-generic altera-only ad-hoc interface ? I'd like to hear opinion of the others on CC ...
On Kha, 2017-11-09 at 08:05 +0100, Marek Vasut wrote:
On 11/09/2017 07:04 AM, Chee, Tien Fong wrote:
On Sel, 2017-11-07 at 10:34 +0100, Marek Vasut wrote:
On 11/07/2017 10:03 AM, Chee, Tien Fong wrote:
On Isn, 2017-11-06 at 11:56 +0100, Marek Vasut wrote:
On 11/06/2017 05:15 AM, Chee, Tien Fong wrote:
On Ahd, 2017-11-05 at 17:43 +0100, Marek Vasut wrote: > > > > On 11/02/2017 09:20 AM, Chee, Tien Fong wrote: > > > > > > > > > > On Rab, 2017-11-01 at 10:26 +0100, Marek Vasut wrote: > > > > > > > > > > > > > > > On 11/01/2017 10:18 AM, tien.fong.chee@intel.com > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > From: Tien Fong Chee tien.fong.chee@intel.com > > > > > > > > Generic firmware loader framework contains some > > > > common > > > > functionality > > > > which is factored out from splash loader. It 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 > > > > > > > > > > > > > > --- > > > > common/Makefile | 1 + > > > > common/load_fs.c | 217 > > > > +++++++++++++++++++++++++++++++++++++++++++++++++++ > > > > +++ > > > > include/load_fs.h | 38 ++++++++++ > > > > 3 files changed, 256 insertions(+) > > > > create mode 100644 common/load_fs.c > > > > create mode 100644 include/load_fs.h > > > [...] > > > > > > > > > > > > > > > > > > > > > > > > > > > +int flash_select_fs_dev(struct flash_location > > > > *location) > > > Why does everything have flash_ prefix ? > > > > > I can remove the flash_ prefix, this generic FS loader > > should > > support > > for all filesystem instead of flash. > > > > > > > > > > > > > > > > > I also mentioned the API should copy the linux > > > firmware > > > loader > > > API. > > > > > If i'm not mistaken, you are referring firmware loader > > API > > in > > this > > link https://github.com/torvalds/linux/blob/f007cad159e > > 99fa > > 2acd > > 3b2e > > 9364 > > fbb32ad28b971/drivers/base/firmware_class.c#L1264. > > I would like to confirm with you whether we are talking to the same API above?
https://www.kernel.org/doc/html/v4.13/driver-api/firmware/ind ex.h tml
first link on google btw . You might be able to avoid the firmware structure.
After assessment, i found that Linux loader is not suitable for fpga loader as fpga loader need
- Able to program FPGA image in SPL chunk bu chunk with small
memory allocatted. 2) Name of FPGA image defined in DTS, and path of FPGA image in FAT and UBI partition.
Linux loader is strongly designed based on Linux environment, always assume having RFF, env support(which SPL don't have), sysfs and udev feature.
Sigh, you can just have some additional function call to fetch smaller chunks from a file, I don't think it's that hard of a problem ...
We already have that function to support smaller chunks, and it also work for single large image when enough memory is available in ver 1 series of fpga loadfs.
Since the Linux loader API is totally not suitable for fpga loadfs, and also nothing i can leverage from there for fpga loadfs, could you please consider to accept implementation for this series patches or implementation for fpga loadfs series ver1?
You mean going back to completely custom non-generic altera-only ad- hoc interface ? I'd like to hear opinion of the others on CC ...
or this patch.
On Thu, 9 Nov 2017 08:05:18 +0100 Marek Vasut marex@denx.de wrote:
On 11/09/2017 07:04 AM, Chee, Tien Fong wrote:
On Sel, 2017-11-07 at 10:34 +0100, Marek Vasut wrote:
On 11/07/2017 10:03 AM, Chee, Tien Fong wrote:
On Isn, 2017-11-06 at 11:56 +0100, Marek Vasut wrote:
On 11/06/2017 05:15 AM, Chee, Tien Fong wrote:
On Ahd, 2017-11-05 at 17:43 +0100, Marek Vasut wrote: > > > On 11/02/2017 09:20 AM, Chee, Tien Fong wrote: >> >> >> >> On Rab, 2017-11-01 at 10:26 +0100, Marek Vasut wrote: >>> >>> >>> >>> On 11/01/2017 10:18 AM, tien.fong.chee@intel.com wrote: >>>> >>>> >>>> >>>> >>>> From: Tien Fong Chee tien.fong.chee@intel.com >>>> >>>> Generic firmware loader framework contains some common >>>> functionality >>>> which is factored out from splash loader. It 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 >>>>> >>>> --- >>>> common/Makefile | 1 + >>>> common/load_fs.c | 217 >>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++ >>>> include/load_fs.h | 38 ++++++++++ >>>> 3 files changed, 256 insertions(+) >>>> create mode 100644 common/load_fs.c >>>> create mode 100644 include/load_fs.h >>> [...] >>> >>>> >>>> >>>> >>>> >>>> +int flash_select_fs_dev(struct flash_location >>>> *location) >>> Why does everything have flash_ prefix ? >>> >> I can remove the flash_ prefix, this generic FS loader >> should >> support >> for all filesystem instead of flash. >> >>> >>> >>> >>> I also mentioned the API should copy the linux firmware >>> loader >>> API. >>> >> If i'm not mistaken, you are referring firmware loader API >> in >> this >> link https://github.com/torvalds/linux/blob/f007cad159e99fa >> 2acd >> 3b2e >> 9364 >> fbb32ad28b971/drivers/base/firmware_class.c#L1264. >> I would like to confirm with you whether we are talking to the same API above?
https://www.kernel.org/doc/html/v4.13/driver-api/firmware/index.h tml
first link on google btw . You might be able to avoid the firmware structure.
After assessment, i found that Linux loader is not suitable for fpga loader as fpga loader need
- Able to program FPGA image in SPL chunk bu chunk with small
memory allocatted. 2) Name of FPGA image defined in DTS, and path of FPGA image in FAT and UBI partition.
Linux loader is strongly designed based on Linux environment, always assume having RFF, env support(which SPL don't have), sysfs and udev feature.
Sigh, you can just have some additional function call to fetch smaller chunks from a file, I don't think it's that hard of a problem ...
We already have that function to support smaller chunks, and it also work for single large image when enough memory is available in ver 1 series of fpga loadfs.
Since the Linux loader API is totally not suitable for fpga loadfs, and also nothing i can leverage from there for fpga loadfs, could you please consider to accept implementation for this series patches or implementation for fpga loadfs series ver1?
You mean going back to completely custom non-generic altera-only ad-hoc interface ? I'd like to hear opinion of the others on CC ...
I must admit that I don't know the exact Altera API for loading their bitstream.
What I would like to have though is a some kind of generic code, which would allow me to reuse it on other ARM + DSP SoCs.....
If we cannot re-use Linux stuff, then when we add something different (more customer/industry aligned), please make it reusable for other solutions (Xilinx, ADI, etc) - that would require a good documentation.
Those are my 2 cents.
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
On 11/09/2017 11:00 AM, Lukasz Majewski wrote:
On Thu, 9 Nov 2017 08:05:18 +0100 Marek Vasut marex@denx.de wrote:
On 11/09/2017 07:04 AM, Chee, Tien Fong wrote:
On Sel, 2017-11-07 at 10:34 +0100, Marek Vasut wrote:
On 11/07/2017 10:03 AM, Chee, Tien Fong wrote:
On Isn, 2017-11-06 at 11:56 +0100, Marek Vasut wrote:
On 11/06/2017 05:15 AM, Chee, Tien Fong wrote: > > > On Ahd, 2017-11-05 at 17:43 +0100, Marek Vasut wrote: >> >> >> On 11/02/2017 09:20 AM, Chee, Tien Fong wrote: >>> >>> >>> >>> On Rab, 2017-11-01 at 10:26 +0100, Marek Vasut wrote: >>>> >>>> >>>> >>>> On 11/01/2017 10:18 AM, tien.fong.chee@intel.com wrote: >>>>> >>>>> >>>>> >>>>> >>>>> From: Tien Fong Chee tien.fong.chee@intel.com >>>>> >>>>> Generic firmware loader framework contains some common >>>>> functionality >>>>> which is factored out from splash loader. It 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 >>>>>> >>>>> --- >>>>> common/Makefile | 1 + >>>>> common/load_fs.c | 217 >>>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++ >>>>> include/load_fs.h | 38 ++++++++++ >>>>> 3 files changed, 256 insertions(+) >>>>> create mode 100644 common/load_fs.c >>>>> create mode 100644 include/load_fs.h >>>> [...] >>>> >>>>> >>>>> >>>>> >>>>> >>>>> +int flash_select_fs_dev(struct flash_location >>>>> *location) >>>> Why does everything have flash_ prefix ? >>>> >>> I can remove the flash_ prefix, this generic FS loader >>> should >>> support >>> for all filesystem instead of flash. >>> >>>> >>>> >>>> >>>> I also mentioned the API should copy the linux firmware >>>> loader >>>> API. >>>> >>> If i'm not mistaken, you are referring firmware loader API >>> in >>> this >>> link https://github.com/torvalds/linux/blob/f007cad159e99fa >>> 2acd >>> 3b2e >>> 9364 >>> fbb32ad28b971/drivers/base/firmware_class.c#L1264. >>> > I would like to confirm with you whether we are talking to the > same > API > above? https://www.kernel.org/doc/html/v4.13/driver-api/firmware/index.h tml
first link on google btw . You might be able to avoid the firmware structure.
After assessment, i found that Linux loader is not suitable for fpga loader as fpga loader need
- Able to program FPGA image in SPL chunk bu chunk with small
memory allocatted. 2) Name of FPGA image defined in DTS, and path of FPGA image in FAT and UBI partition.
Linux loader is strongly designed based on Linux environment, always assume having RFF, env support(which SPL don't have), sysfs and udev feature.
Sigh, you can just have some additional function call to fetch smaller chunks from a file, I don't think it's that hard of a problem ...
We already have that function to support smaller chunks, and it also work for single large image when enough memory is available in ver 1 series of fpga loadfs.
Since the Linux loader API is totally not suitable for fpga loadfs, and also nothing i can leverage from there for fpga loadfs, could you please consider to accept implementation for this series patches or implementation for fpga loadfs series ver1?
You mean going back to completely custom non-generic altera-only ad-hoc interface ? I'd like to hear opinion of the others on CC ...
I must admit that I don't know the exact Altera API for loading their bitstream.
That's irrelevant for a generic loader. The loader should provide a file or ability to read chunks of file if needed, that's all. The consumer driver would then use that API to program whatever, ie. the FPGA.
What I would like to have though is a some kind of generic code, which would allow me to reuse it on other ARM + DSP SoCs.....
... on other platforms in general.
If we cannot re-use Linux stuff, then when we add something different (more customer/industry aligned), please make it reusable for other solutions (Xilinx, ADI, etc) - that would require a good documentation.
And what is the problem with the Linux API ? I am not saying to reuse the Linux code, but the API is quite well fleshed out.
On Kha, 2017-11-09 at 11:31 +0100, Marek Vasut wrote:
On 11/09/2017 11:00 AM, Lukasz Majewski wrote:
On Thu, 9 Nov 2017 08:05:18 +0100 Marek Vasut marex@denx.de wrote:
On 11/09/2017 07:04 AM, Chee, Tien Fong wrote:
On Sel, 2017-11-07 at 10:34 +0100, Marek Vasut wrote:
On 11/07/2017 10:03 AM, Chee, Tien Fong wrote:
On Isn, 2017-11-06 at 11:56 +0100, Marek Vasut wrote: > > > On 11/06/2017 05:15 AM, Chee, Tien Fong wrote: > > > > > > > > On Ahd, 2017-11-05 at 17:43 +0100, Marek Vasut wrote: > > > > > > > > > > > > On 11/02/2017 09:20 AM, Chee, Tien Fong wrote: > > > > > > > > > > > > > > > > > > > > On Rab, 2017-11-01 at 10:26 +0100, Marek Vasut > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > On 11/01/2017 10:18 AM, tien.fong.chee@intel.com > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > From: Tien Fong Chee tien.fong.chee@intel.com > > > > > > > > > > > > Generic firmware loader framework contains some > > > > > > common > > > > > > functionality > > > > > > which is factored out from splash loader. It 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@i > > > > > > ntel.com > > > > > > > > > > > > > > > > > > > > --- > > > > > > common/Makefile | 1 + > > > > > > common/load_fs.c | 217 > > > > > > +++++++++++++++++++++++++++++++++++++++++++++++ > > > > > > +++++++ > > > > > > include/load_fs.h | 38 ++++++++++ > > > > > > 3 files changed, 256 insertions(+) > > > > > > create mode 100644 common/load_fs.c > > > > > > create mode 100644 include/load_fs.h > > > > > [...] > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > +int flash_select_fs_dev(struct flash_location > > > > > > *location) > > > > > Why does everything have flash_ prefix ? > > > > > > > > > I can remove the flash_ prefix, this generic FS > > > > loader > > > > should > > > > support > > > > for all filesystem instead of flash. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I also mentioned the API should copy the linux > > > > > firmware > > > > > loader > > > > > API. > > > > > > > > > If i'm not mistaken, you are referring firmware > > > > loader API > > > > in > > > > this > > > > link https://github.com/torvalds/linux/blob/f007cad > > > > 159e99fa > > > > 2acd > > > > 3b2e > > > > 9364 > > > > fbb32ad28b971/drivers/base/firmware_class.c#L1264. > > > > > > I would like to confirm with you whether we are talking > > to the > > same > > API > > above? > https://www.kernel.org/doc/html/v4.13/driver-api/firmware > /index.h > tml > > first link on google btw . You might be able to avoid the > firmware > structure. > After assessment, i found that Linux loader is not suitable for fpga loader as fpga loader need
- Able to program FPGA image in SPL chunk bu chunk with
small memory allocatted. 2) Name of FPGA image defined in DTS, and path of FPGA image in FAT and UBI partition.
Linux loader is strongly designed based on Linux environment, always assume having RFF, env support(which SPL don't have), sysfs and udev feature.
Sigh, you can just have some additional function call to fetch smaller chunks from a file, I don't think it's that hard of a problem ...
We already have that function to support smaller chunks, and it also work for single large image when enough memory is available in ver 1 series of fpga loadfs.
Since the Linux loader API is totally not suitable for fpga loadfs, and also nothing i can leverage from there for fpga loadfs, could you please consider to accept implementation for this series patches or implementation for fpga loadfs series ver1?
You mean going back to completely custom non-generic altera-only ad-hoc interface ? I'd like to hear opinion of the others on CC ...
I must admit that I don't know the exact Altera API for loading their bitstream.
That's irrelevant for a generic loader. The loader should provide a file or ability to read chunks of file if needed, that's all. The consumer driver would then use that API to program whatever, ie. the FPGA.
What I would like to have though is a some kind of generic code, which would allow me to reuse it on other ARM + DSP SoCs.....
... on other platforms in general.
If we cannot re-use Linux stuff, then when we add something different (more customer/industry aligned), please make it reusable for other solutions (Xilinx, ADI, etc) - that would require a good documentation.
And what is the problem with the Linux API ? I am not saying to reuse the Linux code, but the API is quite well fleshed out.
I found there is one function called "request_firmware_into_buf" from Linux firmware loader API which can be used for reading a file, or ability to read chunks of file.
So, how about we just go ahead with this function implemented in U- Boot?
Thanks.
On 11/10/2017 10:05 AM, Chee, Tien Fong wrote:
On Kha, 2017-11-09 at 11:31 +0100, Marek Vasut wrote:
On 11/09/2017 11:00 AM, Lukasz Majewski wrote:
On Thu, 9 Nov 2017 08:05:18 +0100 Marek Vasut marex@denx.de wrote:
On 11/09/2017 07:04 AM, Chee, Tien Fong wrote:
On Sel, 2017-11-07 at 10:34 +0100, Marek Vasut wrote:
On 11/07/2017 10:03 AM, Chee, Tien Fong wrote: > > > On Isn, 2017-11-06 at 11:56 +0100, Marek Vasut wrote: >> >> >> On 11/06/2017 05:15 AM, Chee, Tien Fong wrote: >>> >>> >>> >>> On Ahd, 2017-11-05 at 17:43 +0100, Marek Vasut wrote: >>>> >>>> >>>> >>>> On 11/02/2017 09:20 AM, Chee, Tien Fong wrote: >>>>> >>>>> >>>>> >>>>> >>>>> On Rab, 2017-11-01 at 10:26 +0100, Marek Vasut >>>>> wrote: >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> On 11/01/2017 10:18 AM, tien.fong.chee@intel.com >>>>>> wrote: >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> From: Tien Fong Chee tien.fong.chee@intel.com >>>>>>> >>>>>>> Generic firmware loader framework contains some >>>>>>> common >>>>>>> functionality >>>>>>> which is factored out from splash loader. It 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@i >>>>>>> ntel.com >>>>>>>> >>>>>>>> >>>>>>> --- >>>>>>> common/Makefile | 1 + >>>>>>> common/load_fs.c | 217 >>>>>>> +++++++++++++++++++++++++++++++++++++++++++++++ >>>>>>> +++++++ >>>>>>> include/load_fs.h | 38 ++++++++++ >>>>>>> 3 files changed, 256 insertions(+) >>>>>>> create mode 100644 common/load_fs.c >>>>>>> create mode 100644 include/load_fs.h >>>>>> [...] >>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> +int flash_select_fs_dev(struct flash_location >>>>>>> *location) >>>>>> Why does everything have flash_ prefix ? >>>>>> >>>>> I can remove the flash_ prefix, this generic FS >>>>> loader >>>>> should >>>>> support >>>>> for all filesystem instead of flash. >>>>> >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> I also mentioned the API should copy the linux >>>>>> firmware >>>>>> loader >>>>>> API. >>>>>> >>>>> If i'm not mistaken, you are referring firmware >>>>> loader API >>>>> in >>>>> this >>>>> link https://github.com/torvalds/linux/blob/f007cad >>>>> 159e99fa >>>>> 2acd >>>>> 3b2e >>>>> 9364 >>>>> fbb32ad28b971/drivers/base/firmware_class.c#L1264. >>>>> >>> I would like to confirm with you whether we are talking >>> to the >>> same >>> API >>> above? >> https://www.kernel.org/doc/html/v4.13/driver-api/firmware >> /index.h >> tml >> >> first link on google btw . You might be able to avoid the >> firmware >> structure. >> > After assessment, i found that Linux loader is not suitable > for > fpga > loader as fpga loader need > 1) Able to program FPGA image in SPL chunk bu chunk with > small > memory > allocatted. > 2) Name of FPGA image defined in DTS, and path of FPGA > image in > FAT and > UBI partition. > > Linux loader is strongly designed based on Linux > environment, > always > assume having RFF, env support(which SPL don't have), sysfs > and > udev > feature. Sigh, you can just have some additional function call to fetch smaller chunks from a file, I don't think it's that hard of a problem ...
We already have that function to support smaller chunks, and it also work for single large image when enough memory is available in ver 1 series of fpga loadfs.
Since the Linux loader API is totally not suitable for fpga loadfs, and also nothing i can leverage from there for fpga loadfs, could you please consider to accept implementation for this series patches or implementation for fpga loadfs series ver1?
You mean going back to completely custom non-generic altera-only ad-hoc interface ? I'd like to hear opinion of the others on CC ...
I must admit that I don't know the exact Altera API for loading their bitstream.
That's irrelevant for a generic loader. The loader should provide a file or ability to read chunks of file if needed, that's all. The consumer driver would then use that API to program whatever, ie. the FPGA.
What I would like to have though is a some kind of generic code, which would allow me to reuse it on other ARM + DSP SoCs.....
... on other platforms in general.
If we cannot re-use Linux stuff, then when we add something different (more customer/industry aligned), please make it reusable for other solutions (Xilinx, ADI, etc) - that would require a good documentation.
And what is the problem with the Linux API ? I am not saying to reuse the Linux code, but the API is quite well fleshed out.
I found there is one function called "request_firmware_into_buf" from Linux firmware loader API which can be used for reading a file, or ability to read chunks of file.
So, how about we just go ahead with this function implemented in U- Boot?
You can also have request_firmware_chunk() function, since request_firmware_into_buf() doesn't have offset. That's fine.
On Jum, 2017-11-10 at 11:04 +0100, Marek Vasut wrote:
On 11/10/2017 10:05 AM, Chee, Tien Fong wrote:
On Kha, 2017-11-09 at 11:31 +0100, Marek Vasut wrote:
On 11/09/2017 11:00 AM, Lukasz Majewski wrote:
On Thu, 9 Nov 2017 08:05:18 +0100 Marek Vasut marex@denx.de wrote:
On 11/09/2017 07:04 AM, Chee, Tien Fong wrote:
On Sel, 2017-11-07 at 10:34 +0100, Marek Vasut wrote: > > > On 11/07/2017 10:03 AM, Chee, Tien Fong wrote: > > > > > > > > On Isn, 2017-11-06 at 11:56 +0100, Marek Vasut wrote: > > > > > > > > > > > > On 11/06/2017 05:15 AM, Chee, Tien Fong wrote: > > > > > > > > > > > > > > > > > > > > On Ahd, 2017-11-05 at 17:43 +0100, Marek Vasut > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > On 11/02/2017 09:20 AM, Chee, Tien Fong wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Rab, 2017-11-01 at 10:26 +0100, Marek Vasut > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On 11/01/2017 10:18 AM, tien.fong.chee@intel. > > > > > > > com > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > From: Tien Fong Chee <tien.fong.chee@intel. > > > > > > > > com> > > > > > > > > > > > > > > > > Generic firmware loader framework contains > > > > > > > > some > > > > > > > > common > > > > > > > > functionality > > > > > > > > which is factored out from splash loader. > > > > > > > > It 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.ch > > > > > > > > ee@i > > > > > > > > ntel.com > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > --- > > > > > > > > common/Makefile | 1 + > > > > > > > > common/load_fs.c | 217 > > > > > > > > +++++++++++++++++++++++++++++++++++++++++++ > > > > > > > > ++++ > > > > > > > > +++++++ > > > > > > > > include/load_fs.h | 38 ++++++++++ > > > > > > > > 3 files changed, 256 insertions(+) > > > > > > > > create mode 100644 common/load_fs.c > > > > > > > > create mode 100644 include/load_fs.h > > > > > > > [...] > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > +int flash_select_fs_dev(struct > > > > > > > > flash_location > > > > > > > > *location) > > > > > > > Why does everything have flash_ prefix ? > > > > > > > > > > > > > I can remove the flash_ prefix, this generic FS > > > > > > loader > > > > > > should > > > > > > support > > > > > > for all filesystem instead of flash. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I also mentioned the API should copy the > > > > > > > linux > > > > > > > firmware > > > > > > > loader > > > > > > > API. > > > > > > > > > > > > > If i'm not mistaken, you are referring firmware > > > > > > loader API > > > > > > in > > > > > > this > > > > > > link https://github.com/torvalds/linux/blob/f00 > > > > > > 7cad > > > > > > 159e99fa > > > > > > 2acd > > > > > > 3b2e > > > > > > 9364 > > > > > > fbb32ad28b971/drivers/base/firmware_class.c#L12 > > > > > > 64. > > > > > > > > > > I would like to confirm with you whether we are > > > > talking > > > > to the > > > > same > > > > API > > > > above? > > > https://www.kernel.org/doc/html/v4.13/driver-api/firm > > > ware > > > /index.h > > > tml > > > > > > first link on google btw . You might be able to avoid > > > the > > > firmware > > > structure. > > > > > After assessment, i found that Linux loader is not > > suitable > > for > > fpga > > loader as fpga loader need > > 1) Able to program FPGA image in SPL chunk bu chunk > > with > > small > > memory > > allocatted. > > 2) Name of FPGA image defined in DTS, and path of FPGA > > image in > > FAT and > > UBI partition. > > > > Linux loader is strongly designed based on Linux > > environment, > > always > > assume having RFF, env support(which SPL don't have), > > sysfs > > and > > udev > > feature. > Sigh, you can just have some additional function call to > fetch > smaller > chunks from a file, I don't think it's that hard of a > problem > ... > We already have that function to support smaller chunks, and it also work for single large image when enough memory is available in ver 1 series of fpga loadfs.
Since the Linux loader API is totally not suitable for fpga loadfs, and also nothing i can leverage from there for fpga loadfs, could you please consider to accept implementation for this series patches or implementation for fpga loadfs series ver1?
You mean going back to completely custom non-generic altera- only ad-hoc interface ? I'd like to hear opinion of the others on CC ...
I must admit that I don't know the exact Altera API for loading their bitstream.
That's irrelevant for a generic loader. The loader should provide a file or ability to read chunks of file if needed, that's all. The consumer driver would then use that API to program whatever, ie. the FPGA.
What I would like to have though is a some kind of generic code, which would allow me to reuse it on other ARM + DSP SoCs.....
... on other platforms in general.
If we cannot re-use Linux stuff, then when we add something different (more customer/industry aligned), please make it reusable for other solutions (Xilinx, ADI, etc) - that would require a good documentation.
And what is the problem with the Linux API ? I am not saying to reuse the Linux code, but the API is quite well fleshed out.
I found there is one function called "request_firmware_into_buf" from Linux firmware loader API which can be used for reading a file, or ability to read chunks of file.
So, how about we just go ahead with this function implemented in U- Boot?
You can also have request_firmware_chunk() function, since request_firmware_into_buf() doesn't have offset. That's fine.
Okay.
On Jum, 2017-11-10 at 11:04 +0100, Marek Vasut wrote:
On 11/10/2017 10:05 AM, Chee, Tien Fong wrote:
On Kha, 2017-11-09 at 11:31 +0100, Marek Vasut wrote:
On 11/09/2017 11:00 AM, Lukasz Majewski wrote:
On Thu, 9 Nov 2017 08:05:18 +0100 Marek Vasut marex@denx.de wrote:
On 11/09/2017 07:04 AM, Chee, Tien Fong wrote:
On Sel, 2017-11-07 at 10:34 +0100, Marek Vasut wrote: > > > On 11/07/2017 10:03 AM, Chee, Tien Fong wrote: > > > > > > > > On Isn, 2017-11-06 at 11:56 +0100, Marek Vasut wrote: > > > > > > > > > > > > On 11/06/2017 05:15 AM, Chee, Tien Fong wrote: > > > > > > > > > > > > > > > > > > > > On Ahd, 2017-11-05 at 17:43 +0100, Marek Vasut > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > On 11/02/2017 09:20 AM, Chee, Tien Fong wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Rab, 2017-11-01 at 10:26 +0100, Marek Vasut > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On 11/01/2017 10:18 AM, tien.fong.chee@intel. > > > > > > > com > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > From: Tien Fong Chee <tien.fong.chee@intel. > > > > > > > > com> > > > > > > > > > > > > > > > > Generic firmware loader framework contains > > > > > > > > some > > > > > > > > common > > > > > > > > functionality > > > > > > > > which is factored out from splash loader. > > > > > > > > It 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.ch > > > > > > > > ee@i > > > > > > > > ntel.com > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > --- > > > > > > > > common/Makefile | 1 + > > > > > > > > common/load_fs.c | 217 > > > > > > > > +++++++++++++++++++++++++++++++++++++++++++ > > > > > > > > ++++ > > > > > > > > +++++++ > > > > > > > > include/load_fs.h | 38 ++++++++++ > > > > > > > > 3 files changed, 256 insertions(+) > > > > > > > > create mode 100644 common/load_fs.c > > > > > > > > create mode 100644 include/load_fs.h > > > > > > > [...] > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > +int flash_select_fs_dev(struct > > > > > > > > flash_location > > > > > > > > *location) > > > > > > > Why does everything have flash_ prefix ? > > > > > > > > > > > > > I can remove the flash_ prefix, this generic FS > > > > > > loader > > > > > > should > > > > > > support > > > > > > for all filesystem instead of flash. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I also mentioned the API should copy the > > > > > > > linux > > > > > > > firmware > > > > > > > loader > > > > > > > API. > > > > > > > > > > > > > If i'm not mistaken, you are referring firmware > > > > > > loader API > > > > > > in > > > > > > this > > > > > > link https://github.com/torvalds/linux/blob/f00 > > > > > > 7cad > > > > > > 159e99fa > > > > > > 2acd > > > > > > 3b2e > > > > > > 9364 > > > > > > fbb32ad28b971/drivers/base/firmware_class.c#L12 > > > > > > 64. > > > > > > > > > > I would like to confirm with you whether we are > > > > talking > > > > to the > > > > same > > > > API > > > > above? > > > https://www.kernel.org/doc/html/v4.13/driver-api/firm > > > ware > > > /index.h > > > tml > > > > > > first link on google btw . You might be able to avoid > > > the > > > firmware > > > structure. > > > > > After assessment, i found that Linux loader is not > > suitable > > for > > fpga > > loader as fpga loader need > > 1) Able to program FPGA image in SPL chunk bu chunk > > with > > small > > memory > > allocatted. > > 2) Name of FPGA image defined in DTS, and path of FPGA > > image in > > FAT and > > UBI partition. > > > > Linux loader is strongly designed based on Linux > > environment, > > always > > assume having RFF, env support(which SPL don't have), > > sysfs > > and > > udev > > feature. > Sigh, you can just have some additional function call to > fetch > smaller > chunks from a file, I don't think it's that hard of a > problem > ... > We already have that function to support smaller chunks, and it also work for single large image when enough memory is available in ver 1 series of fpga loadfs.
Since the Linux loader API is totally not suitable for fpga loadfs, and also nothing i can leverage from there for fpga loadfs, could you please consider to accept implementation for this series patches or implementation for fpga loadfs series ver1?
You mean going back to completely custom non-generic altera- only ad-hoc interface ? I'd like to hear opinion of the others on CC ...
I must admit that I don't know the exact Altera API for loading their bitstream.
That's irrelevant for a generic loader. The loader should provide a file or ability to read chunks of file if needed, that's all. The consumer driver would then use that API to program whatever, ie. the FPGA.
What I would like to have though is a some kind of generic code, which would allow me to reuse it on other ARM + DSP SoCs.....
... on other platforms in general.
If we cannot re-use Linux stuff, then when we add something different (more customer/industry aligned), please make it reusable for other solutions (Xilinx, ADI, etc) - that would require a good documentation.
And what is the problem with the Linux API ? I am not saying to reuse the Linux code, but the API is quite well fleshed out.
I found there is one function called "request_firmware_into_buf" from Linux firmware loader API which can be used for reading a file, or ability to read chunks of file.
So, how about we just go ahead with this function implemented in U- Boot?
You can also have request_firmware_chunk() function, since request_firmware_into_buf() doesn't have offset. That's fine.
I think we can having same function request_firmware_into_buf for chunk reading, because i can add the offset into firmware structure so the last write location can be stored into offset. Since we know the size of the buffer, so we can do the calculation outside of the function and repetitive calling the function with the return offset until whole file is read. What do you think?
Thanks.
On 11/16/2017 09:09 AM, Chee, Tien Fong wrote:
On Jum, 2017-11-10 at 11:04 +0100, Marek Vasut wrote:
On 11/10/2017 10:05 AM, Chee, Tien Fong wrote:
On Kha, 2017-11-09 at 11:31 +0100, Marek Vasut wrote:
On 11/09/2017 11:00 AM, Lukasz Majewski wrote:
On Thu, 9 Nov 2017 08:05:18 +0100 Marek Vasut marex@denx.de wrote:
On 11/09/2017 07:04 AM, Chee, Tien Fong wrote: > > > On Sel, 2017-11-07 at 10:34 +0100, Marek Vasut wrote: >> >> >> On 11/07/2017 10:03 AM, Chee, Tien Fong wrote: >>> >>> >>> >>> On Isn, 2017-11-06 at 11:56 +0100, Marek Vasut wrote: >>>> >>>> >>>> >>>> On 11/06/2017 05:15 AM, Chee, Tien Fong wrote: >>>>> >>>>> >>>>> >>>>> >>>>> On Ahd, 2017-11-05 at 17:43 +0100, Marek Vasut >>>>> wrote: >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> On 11/02/2017 09:20 AM, Chee, Tien Fong wrote: >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> On Rab, 2017-11-01 at 10:26 +0100, Marek Vasut >>>>>>> wrote: >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> On 11/01/2017 10:18 AM, tien.fong.chee@intel. >>>>>>>> com >>>>>>>> wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> From: Tien Fong Chee <tien.fong.chee@intel. >>>>>>>>> com> >>>>>>>>> >>>>>>>>> Generic firmware loader framework contains >>>>>>>>> some >>>>>>>>> common >>>>>>>>> functionality >>>>>>>>> which is factored out from splash loader. >>>>>>>>> It 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.ch >>>>>>>>> ee@i >>>>>>>>> ntel.com >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>> --- >>>>>>>>> common/Makefile | 1 + >>>>>>>>> common/load_fs.c | 217 >>>>>>>>> +++++++++++++++++++++++++++++++++++++++++++ >>>>>>>>> ++++ >>>>>>>>> +++++++ >>>>>>>>> include/load_fs.h | 38 ++++++++++ >>>>>>>>> 3 files changed, 256 insertions(+) >>>>>>>>> create mode 100644 common/load_fs.c >>>>>>>>> create mode 100644 include/load_fs.h >>>>>>>> [...] >>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> +int flash_select_fs_dev(struct >>>>>>>>> flash_location >>>>>>>>> *location) >>>>>>>> Why does everything have flash_ prefix ? >>>>>>>> >>>>>>> I can remove the flash_ prefix, this generic FS >>>>>>> loader >>>>>>> should >>>>>>> support >>>>>>> for all filesystem instead of flash. >>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> I also mentioned the API should copy the >>>>>>>> linux >>>>>>>> firmware >>>>>>>> loader >>>>>>>> API. >>>>>>>> >>>>>>> If i'm not mistaken, you are referring firmware >>>>>>> loader API >>>>>>> in >>>>>>> this >>>>>>> link https://github.com/torvalds/linux/blob/f00 >>>>>>> 7cad >>>>>>> 159e99fa >>>>>>> 2acd >>>>>>> 3b2e >>>>>>> 9364 >>>>>>> fbb32ad28b971/drivers/base/firmware_class.c#L12 >>>>>>> 64. >>>>>>> >>>>> I would like to confirm with you whether we are >>>>> talking >>>>> to the >>>>> same >>>>> API >>>>> above? >>>> https://www.kernel.org/doc/html/v4.13/driver-api/firm >>>> ware >>>> /index.h >>>> tml >>>> >>>> first link on google btw . You might be able to avoid >>>> the >>>> firmware >>>> structure. >>>> >>> After assessment, i found that Linux loader is not >>> suitable >>> for >>> fpga >>> loader as fpga loader need >>> 1) Able to program FPGA image in SPL chunk bu chunk >>> with >>> small >>> memory >>> allocatted. >>> 2) Name of FPGA image defined in DTS, and path of FPGA >>> image in >>> FAT and >>> UBI partition. >>> >>> Linux loader is strongly designed based on Linux >>> environment, >>> always >>> assume having RFF, env support(which SPL don't have), >>> sysfs >>> and >>> udev >>> feature. >> Sigh, you can just have some additional function call to >> fetch >> smaller >> chunks from a file, I don't think it's that hard of a >> problem >> ... >> > We already have that function to support smaller chunks, > and it > also > work for single large image when enough memory is available > in > ver 1 > series of fpga loadfs. > > Since the Linux loader API is totally not suitable for fpga > loadfs, > and also nothing i can leverage from there for fpga loadfs, > could > you please consider to accept implementation for this > series > patches or implementation for fpga loadfs series ver1? You mean going back to completely custom non-generic altera- only ad-hoc interface ? I'd like to hear opinion of the others on CC ...
I must admit that I don't know the exact Altera API for loading their bitstream.
That's irrelevant for a generic loader. The loader should provide a file or ability to read chunks of file if needed, that's all. The consumer driver would then use that API to program whatever, ie. the FPGA.
What I would like to have though is a some kind of generic code, which would allow me to reuse it on other ARM + DSP SoCs.....
... on other platforms in general.
If we cannot re-use Linux stuff, then when we add something different (more customer/industry aligned), please make it reusable for other solutions (Xilinx, ADI, etc) - that would require a good documentation.
And what is the problem with the Linux API ? I am not saying to reuse the Linux code, but the API is quite well fleshed out.
I found there is one function called "request_firmware_into_buf" from Linux firmware loader API which can be used for reading a file, or ability to read chunks of file.
So, how about we just go ahead with this function implemented in U- Boot?
You can also have request_firmware_chunk() function, since request_firmware_into_buf() doesn't have offset. That's fine.
I think we can having same function request_firmware_into_buf for chunk reading, because i can add the offset into firmware structure so the last write location can be stored into offset. Since we know the size of the buffer, so we can do the calculation outside of the function and repetitive calling the function with the return offset until whole file is read. What do you think?
Something like that.
Thanks.
From: Tien Fong Chee tien.fong.chee@intel.com
Convert splash file system loader portion to use generic file system firmware loader. The rest of files which use splash loader would be converted to use generic file system firmware loader.
Signed-off-by: Tien Fong Chee tien.fong.chee@intel.com --- board/compulab/cm_fx6/cm_fx6.c | 17 ++-- board/compulab/cm_t35/cm_t35.c | 6 +- common/splash.c | 18 ++-- common/splash_source.c | 193 ++++++++++------------------------------- include/splash.h | 29 +------ 5 files changed, 67 insertions(+), 196 deletions(-)
diff --git a/board/compulab/cm_fx6/cm_fx6.c b/board/compulab/cm_fx6/cm_fx6.c index 638e9f3..4404979 100644 --- a/board/compulab/cm_fx6/cm_fx6.c +++ b/board/compulab/cm_fx6/cm_fx6.c @@ -40,29 +40,28 @@ DECLARE_GLOBAL_DATA_PTR;
#ifdef CONFIG_SPLASH_SCREEN -static struct splash_location cm_fx6_splash_locations[] = { +static struct flash_location cm_fx6_splash_locations[] = { { .name = "sf", - .storage = SPLASH_STORAGE_SF, - .flags = SPLASH_STORAGE_RAW, + .storage = FLASH_STORAGE_SF, + .flags = FLASH_STORAGE_RAW, .offset = 0x100000, }, { .name = "mmc_fs", - .storage = SPLASH_STORAGE_MMC, - .flags = SPLASH_STORAGE_FS, + .storage = FLASH_STORAGE_MMC, + .flags = FLASH_STORAGE_FS, .devpart = "2:1", }, { .name = "usb_fs", - .storage = SPLASH_STORAGE_USB, - .flags = SPLASH_STORAGE_FS, + .storage = FLASH_STORAGE_USB, + .flags = FLASH_STORAGE_FS, .devpart = "0:1", }, { .name = "sata_fs", - .storage = SPLASH_STORAGE_SATA, - .flags = SPLASH_STORAGE_FS, + .storage = FLASH_STORAGE_SATA, .devpart = "0:1", }, }; diff --git a/board/compulab/cm_t35/cm_t35.c b/board/compulab/cm_t35/cm_t35.c index be938eb..6ae84a0 100644 --- a/board/compulab/cm_t35/cm_t35.c +++ b/board/compulab/cm_t35/cm_t35.c @@ -60,11 +60,11 @@ void get_board_mem_timings(struct board_sdrc_timings *timings) } #endif
-struct splash_location splash_locations[] = { +struct flash_location splash_locations[] = { { .name = "nand", - .storage = SPLASH_STORAGE_NAND, - .flags = SPLASH_STORAGE_RAW, + .storage = FLASH_STORAGE_NAND, + .flags = FLASH_STORAGE_RAW, .offset = 0x100000, }, }; diff --git a/common/splash.c b/common/splash.c index d251b3b..9ed6aa6 100644 --- a/common/splash.c +++ b/common/splash.c @@ -24,29 +24,29 @@ #include <splash.h> #include <lcd.h>
-static struct splash_location default_splash_locations[] = { +static struct flash_location default_splash_locations[] = { { .name = "sf", - .storage = SPLASH_STORAGE_SF, - .flags = SPLASH_STORAGE_RAW, + .storage = FLASH_STORAGE_SF, + .flags = FLASH_STORAGE_RAW, .offset = 0x0, }, { .name = "mmc_fs", - .storage = SPLASH_STORAGE_MMC, - .flags = SPLASH_STORAGE_FS, + .storage = FLASH_STORAGE_MMC, + .flags = FLASH_STORAGE_FS, .devpart = "0:1", }, { .name = "usb_fs", - .storage = SPLASH_STORAGE_USB, - .flags = SPLASH_STORAGE_FS, + .storage = FLASH_STORAGE_USB, + .flags = FLASH_STORAGE_FS, .devpart = "0:1", }, { .name = "sata_fs", - .storage = SPLASH_STORAGE_SATA, - .flags = SPLASH_STORAGE_FS, + .storage = FLASH_STORAGE_SATA, + .flags = FLASH_STORAGE_FS, .devpart = "0:1", }, }; diff --git a/common/splash_source.c b/common/splash_source.c index e0defde..b5edd8b 100644 --- a/common/splash_source.c +++ b/common/splash_source.c @@ -61,7 +61,7 @@ static int splash_nand_read_raw(u32 bmp_load_addr, int offset, size_t read_size) } #endif
-static int splash_storage_read_raw(struct splash_location *location, +static int splash_storage_read_raw(struct flash_location *location, u32 bmp_load_addr, size_t read_size) { u32 offset; @@ -71,9 +71,9 @@ static int splash_storage_read_raw(struct splash_location *location,
offset = location->offset; switch (location->storage) { - case SPLASH_STORAGE_NAND: + case FLASH_STORAGE_NAND: return splash_nand_read_raw(bmp_load_addr, offset, read_size); - case SPLASH_STORAGE_SF: + case FLASH_STORAGE_SF: return splash_sf_read_raw(bmp_load_addr, offset, read_size); default: printf("Unknown splash location\n"); @@ -82,7 +82,7 @@ static int splash_storage_read_raw(struct splash_location *location, return -EINVAL; }
-static int splash_load_raw(struct splash_location *location, u32 bmp_load_addr) +static int splash_load_raw(struct flash_location *location, u32 bmp_load_addr) { struct bmp_header *bmp_hdr; int res; @@ -109,159 +109,53 @@ splash_address_too_high: return -EFAULT; }
-static int splash_select_fs_dev(struct splash_location *location) -{ - int res; - - switch (location->storage) { - case SPLASH_STORAGE_MMC: - res = fs_set_blk_dev("mmc", location->devpart, FS_TYPE_ANY); - break; - case SPLASH_STORAGE_USB: - res = fs_set_blk_dev("usb", location->devpart, FS_TYPE_ANY); - break; - case SPLASH_STORAGE_SATA: - res = fs_set_blk_dev("sata", location->devpart, FS_TYPE_ANY); - break; - case SPLASH_STORAGE_NAND: - if (location->ubivol != NULL) - res = fs_set_blk_dev("ubi", NULL, FS_TYPE_UBIFS); - else - res = -ENODEV; - break; - default: - printf("Error: unsupported location storage.\n"); - return -ENODEV; - } - - if (res) - printf("Error: could not access storage.\n"); - - return res; -} - -#ifdef CONFIG_USB_STORAGE -static int splash_init_usb(void) -{ - int err; - - err = usb_init(); - if (err) - return err; - -#ifndef CONFIG_DM_USB - err = usb_stor_scan(1) < 0 ? -ENODEV : 0; -#endif - - return err; -} -#else -static inline int splash_init_usb(void) -{ - printf("Cannot load splash image: no USB support\n"); - return -ENOSYS; -} -#endif +#define FLASH_SOURCE_DEFAULT_FILE_NAME "splash.bmp"
-#ifdef CONFIG_SATA -static int splash_init_sata(void) -{ - return sata_probe(0); -} -#else -static inline int splash_init_sata(void) -{ - printf("Cannot load splash image: no SATA support\n"); - return -ENOSYS; -} -#endif - -#ifdef CONFIG_CMD_UBIFS -static int splash_mount_ubifs(struct splash_location *location) -{ - int res; - char cmd[32]; - - sprintf(cmd, "ubi part %s", location->mtdpart); - res = run_command(cmd, 0); - if (res) - return res; - - sprintf(cmd, "ubifsmount %s", location->ubivol); - res = run_command(cmd, 0); - - return res; -} - -static inline int splash_umount_ubifs(void) -{ - return run_command("ubifsumount", 0); -} -#else -static inline int splash_mount_ubifs(struct splash_location *location) -{ - printf("Cannot load splash image: no UBIFS support\n"); - return -ENOSYS; -} - -static inline int splash_umount_ubifs(void) -{ - printf("Cannot unmount UBIFS: no UBIFS support\n"); - return -ENOSYS; -} -#endif - -#define SPLASH_SOURCE_DEFAULT_FILE_NAME "splash.bmp" - -static int splash_load_fs(struct splash_location *location, u32 bmp_load_addr) +int fsloader_preprocess(struct flash_location *location, + void *file_info, char **filename, + u32 load_addr) { int res = 0; - loff_t bmp_size; - loff_t actread; - char *splash_file; - - splash_file = env_get("splashfile"); - if (!splash_file) - splash_file = SPLASH_SOURCE_DEFAULT_FILE_NAME; - - if (location->storage == SPLASH_STORAGE_USB) - res = splash_init_usb(); + loff_t file_size; + char *flash_file;
- if (location->storage == SPLASH_STORAGE_SATA) - res = splash_init_sata(); + flash_file = env_get((char *)file_info);
- if (location->ubivol != NULL) - res = splash_mount_ubifs(location); + if (!flash_file) + flash_file = FLASH_SOURCE_DEFAULT_FILE_NAME;
- if (res) - return res; - - res = splash_select_fs_dev(location); - if (res) - goto out; + res = flash_select_fs_dev(location); + if (res) { + error("Error : Failed to select FS device\n"); + return -EPERM; + }
- res = fs_size(splash_file, &bmp_size); + res = fs_size(flash_file, &file_size); if (res) { - printf("Error (%d): cannot determine file size\n", res); - goto out; + error("Error (%d): cannot determine file size\n", res); + return -ENOENT; }
- 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"); + if ((u32)load_addr + file_size >= gd->start_addr_sp) { + error("Error: Flashimage address too high. Data overwrites "); + error("U-Boot and/or placed beyond DRAM boundaries.\n"); res = -EFAULT; - goto out; + return -EFAULT; }
- splash_select_fs_dev(location); - res = fs_read(splash_file, bmp_load_addr, 0, 0, &actread); - -out: - if (location->ubivol != NULL) - splash_umount_ubifs(); + *filename = flash_file;
return res; }
+int fs_loading(struct flash_location *location, void *file_info, + char *filename, u32 load_addr, size_t bsize) +{ + loff_t actread; + + return fs_read(filename, (u32)load_addr, 0, 0, &actread); +} + /** * select_splash_location - return the splash location based on board support * and env variable "splashsource". @@ -277,8 +171,8 @@ out: * If splashsource env variable contains a supported value * return the location selected by splashsource. */ -static struct splash_location *select_splash_location( - struct splash_location *locations, uint size) +static struct flash_location *select_splash_location( + struct flash_location *locations, uint size) { int i; char *env_splashsource; @@ -300,7 +194,7 @@ static struct splash_location *select_splash_location( }
#ifdef CONFIG_FIT -static int splash_load_fit(struct splash_location *location, u32 bmp_load_addr) +static int splash_load_fit(struct flash_location *location, u32 bmp_load_addr) { int res; int node_offset; @@ -382,9 +276,9 @@ static int splash_load_fit(struct splash_location *location, u32 bmp_load_addr) * * @return: 0 on success, negative value on failure. */ -int splash_source_load(struct splash_location *locations, uint size) +int splash_source_load(struct flash_location *locations, uint size) { - struct splash_location *splash_location; + struct flash_location *splash_location; char *env_splashimage_value; u32 bmp_load_addr;
@@ -402,12 +296,13 @@ int splash_source_load(struct splash_location *locations, uint size) if (!splash_location) return -EINVAL;
- if (splash_location->flags == SPLASH_STORAGE_RAW) + if (splash_location->flags == FLASH_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); + else if (splash_location->flags == FLASH_STORAGE_FS) + return flash_load_fs(splash_location, "splashfile", + bmp_load_addr, 0); #ifdef CONFIG_FIT - else if (splash_location->flags == SPLASH_STORAGE_FIT) + else if (splash_location->flags == FLASH_STORAGE_FIT) return splash_load_fit(splash_location, bmp_load_addr); #endif return -EINVAL; diff --git a/include/splash.h b/include/splash.h index 228aff4..5ca2b6a 100644 --- a/include/splash.h +++ b/include/splash.h @@ -23,35 +23,12 @@ #define _SPLASH_H_
#include <errno.h> - -enum splash_storage { - SPLASH_STORAGE_NAND, - SPLASH_STORAGE_SF, - SPLASH_STORAGE_MMC, - SPLASH_STORAGE_USB, - SPLASH_STORAGE_SATA, -}; - -enum splash_flags { - SPLASH_STORAGE_RAW, /* Stored in raw memory */ - SPLASH_STORAGE_FS, /* Stored within a file system */ - SPLASH_STORAGE_FIT, /* Stored inside a FIT image */ -}; - -struct splash_location { - char *name; - enum splash_storage storage; - enum splash_flags flags; - u32 offset; /* offset from start of storage */ - char *devpart; /* Use the load command dev:part conventions */ - char *mtdpart; /* MTD partition for ubi part */ - char *ubivol; /* UBI volume-name for ubifsmount */ -}; +#include <load_fs.h>
#ifdef CONFIG_SPLASH_SOURCE -int splash_source_load(struct splash_location *locations, uint size); +int splash_source_load(struct flash_location *locations, uint size); #else -static inline int splash_source_load(struct splash_location *locations, +static inline int splash_source_load(struct flash_location *locations, uint size) { return 0;
participants (4)
-
Chee, Tien Fong -
Lukasz Majewski -
Marek Vasut -
tien.fong.chee＠intel.com