Skip to contentSkip to conversation

Creating generic file system firmware loader

PATCH New tien.fong.chee at intel.com 4 participants 20 messages Nov 1, 2017 – Nov 16, 2017

lore
Share & tools
lore
#0 lore raw

cover Creating generic file system firmware loader

From: Tien Fong Chee <[email protected]>

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
Signature · 1 line
2.2.0
#1 lore raw

PATCH 1/2 common: Generic file system firmware loader

From: Tien Fong Chee <[email protected]>

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.

---
 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
common/Makefile +1 -0
index 801ea31..89f9365 100644
@@ -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
common/load_fs.c new file +217 -0

217 lines not on this page · raw message

include/load_fs.h new file +38 -0
new file mode 100644
index 0000000..dbde8d0
@@ -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
Signature · 1 line
2.2.0
#2 lore raw

PATCH 1/2 common: Generic file system firmware loader

Quoted 16 lines · tien.fong.chee at intel.com

From: Tien Fong Chee <[email protected]>

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.

---
 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

[...]

Quoted 1 line

+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.

Quoted 22 lines
common/load_fs.c new file +22 -0
+{
+	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;

common/load_fs.c line 49

Quoted 10 lines
common/load_fs.c new file +10 -0
+	}
+
+	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.

Quoted 6 lines
common/load_fs.c new file +6 -0
+static int flash_init_usb(void)
+{
+	int err;
+
+	err = usb_init();
+	if (err)

common/load_fs.c line 65

Quoted 10 lines
common/load_fs.c new file +10 -0
+		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.

[...]

Signature · 2 lines
Best regards,
Marek Vasut
#3 lore raw

PATCH 1/2 common: Generic file system firmware loader

Quoted 21 lines · Marek Vasut
Quoted 12 lines · tien.fong.chee at intel.com

From: Tien Fong Chee <[email protected]>

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.

Quoted 8 lines

---
 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

Quoted 6 lines

[...]

Quoted 2 lines

+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.

Quoted 2 lines

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

Quoted 29 lines
Quoted 29 lines
common/load_fs.c new file +5 -0
+	int res;
+
+	switch (location->storage) {
+	case FLASH_STORAGE_MMC:
+		res = fs_set_blk_dev("mmc", location->devpart,

FS_TYPE_ANY);

common/load_fs.c new file +3 -0
+		break;
+	case FLASH_STORAGE_USB:
+		res = fs_set_blk_dev("usb", location->devpart,

FS_TYPE_ANY);

common/load_fs.c new file +3 -0
+		break;
+	case FLASH_STORAGE_SATA:
+		res = fs_set_blk_dev("sata", location->devpart,

FS_TYPE_ANY);

common/load_fs.c new file +4 -0
+		break;
+	case FLASH_STORAGE_NAND:
+		if (location->ubivol != NULL)
+			res = fs_set_blk_dev("ubi", NULL,

FS_TYPE_UBIFS);

common/load_fs.c new file +10 -0
+		else
+			res = -ENODEV;
+		break;
+	default:
+		error("Error: unsupported location storage.\n");
+		return -ENODEV;
+	}
+
+	if (res)
+		error("Error: could not access storage.\n");
Quoted 8 lines

common/load_fs.c line 49

Quoted 6 lines
common/load_fs.c new file +6 -0
+
+	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.

Quoted 2 lines

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.

Quoted 10 lines
Quoted 10 lines
common/load_fs.c new file +9 -0
+static int flash_init_usb(void)
+{
+	int err;
+
+	err = usb_init();
+	if (err)
+		return err;
+
+#ifndef CONFIG_DM_USB
Quoted 9 lines

common/load_fs.c line 65

Quoted 7 lines
common/load_fs.c new file +7 -0
+	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.

Quoted 3 lines

[...]

#4 lore raw

PATCH 1/2 common: Generic file system firmware loader

Quoted 37 lines · Tien Fong
Quoted 21 lines · Marek Vasut
Quoted 12 lines · tien.fong.chee at intel.com

From: Tien Fong Chee <[email protected]>

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.

Quoted 8 lines

---
 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

Quoted 6 lines

[...]

Quoted 2 lines

+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.

Quoted 2 lines

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.

Quoted 10 lines

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

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.

Quoted 37 lines
Quoted 29 lines
Quoted 29 lines
common/load_fs.c new file +5 -0
+	int res;
+
+	switch (location->storage) {
+	case FLASH_STORAGE_MMC:
+		res = fs_set_blk_dev("mmc", location->devpart,

FS_TYPE_ANY);

common/load_fs.c new file +3 -0
+		break;
+	case FLASH_STORAGE_USB:
+		res = fs_set_blk_dev("usb", location->devpart,

FS_TYPE_ANY);

common/load_fs.c new file +3 -0
+		break;
+	case FLASH_STORAGE_SATA:
+		res = fs_set_blk_dev("sata", location->devpart,

FS_TYPE_ANY);

common/load_fs.c new file +4 -0
+		break;
+	case FLASH_STORAGE_NAND:
+		if (location->ubivol != NULL)
+			res = fs_set_blk_dev("ubi", NULL,

FS_TYPE_UBIFS);

common/load_fs.c new file +10 -0
+		else
+			res = -ENODEV;
+		break;
+	default:
+		error("Error: unsupported location storage.\n");
+		return -ENODEV;
+	}
+
+	if (res)
+		error("Error: could not access storage.\n");
Quoted 8 lines

common/load_fs.c line 49

Quoted 6 lines
common/load_fs.c new file +6 -0
+
+	return res;
+}
+
+#ifndef CONFIG_SPL_BUILD
+#ifdef CONFIG_USB_STORAGE

This looks wrong, the USB can be supported in SPL no problem. And this

Quoted 6 lines

Technically, USB can be supported in SPL, but the build for USB in SPL is not supported yet.

Quoted 2 lines

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.

Signature · 2 lines
Best regards,
Marek Vasut
#5 lore raw

PATCH 1/2 common: Generic file system firmware loader

Quoted 39 lines · Marek Vasut
Quoted 38 lines · Tien Fong
Quoted 24 lines · Marek Vasut
Quoted 14 lines · tien.fong.chee at intel.com

From: Tien Fong Chee <[email protected]>

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.

Quoted 8 lines

---
 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

Quoted 7 lines

[...]

Quoted 3 lines

+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.

Quoted 1 line
Quoted 8 lines
Quoted 8 lines
Quoted 3 lines

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?

Quoted 71 lines
Quoted 6 lines

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

Quoted 8 lines

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

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.

Quoted 46 lines
Quoted 34 lines
Quoted 34 lines
common/load_fs.c new file +5 -0
+	int res;
+
+	switch (location->storage) {
+	case FLASH_STORAGE_MMC:
+		res = fs_set_blk_dev("mmc", location->devpart,

FS_TYPE_ANY);

common/load_fs.c new file +3 -0
+		break;
+	case FLASH_STORAGE_USB:
+		res = fs_set_blk_dev("usb", location->devpart,

FS_TYPE_ANY);

common/load_fs.c new file +3 -0
+		break;
+	case FLASH_STORAGE_SATA:
+		res = fs_set_blk_dev("sata", location-
Quoted 1 line

devpart,

FS_TYPE_ANY);

common/load_fs.c new file +4 -0
+		break;
+	case FLASH_STORAGE_NAND:
+		if (location->ubivol != NULL)
+			res = fs_set_blk_dev("ubi", NULL,

FS_TYPE_UBIFS);

common/load_fs.c new file +5 -0
+		else
+			res = -ENODEV;
+		break;
+	default:
+		error("Error: unsupported location

storage.\n");

common/load_fs.c new file +5 -0
+		return -ENODEV;
+	}
+
+	if (res)
+		error("Error: could not access storage.\n");
Quoted 8 lines

common/load_fs.c line 49

Quoted 6 lines
common/load_fs.c new file +6 -0
+
+	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.

Quoted 1 line
Quoted 5 lines
Quoted 2 lines

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.

#6 lore raw

PATCH 1/2 common: Generic file system firmware loader

Quoted 43 lines · Tien Fong
Quoted 42 lines · Marek Vasut
Quoted 41 lines · Tien Fong
Quoted 24 lines · Marek Vasut
Quoted 14 lines · tien.fong.chee at intel.com

From: Tien Fong Chee <[email protected]>

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.

Quoted 8 lines

---
 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

Quoted 7 lines

[...]

Quoted 3 lines

+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.

Quoted 4 lines

I also mentioned the API should copy the linux firmware loader API.

Quoted 7 lines
Quoted 5 lines
Quoted 5 lines

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.

[...]

Signature · 2 lines
Best regards,
Marek Vasut
#7 lore raw

PATCH 1/2 common: Generic file system firmware loader

Quoted 62 lines · Marek Vasut
Quoted 61 lines · Tien Fong
Quoted 59 lines · Marek Vasut
Quoted 57 lines · Tien Fong
Quoted 29 lines · Marek Vasut
Quoted 18 lines · tien.fong.chee at intel.com

From: Tien Fong Chee <[email protected]>

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.

Quoted 8 lines

---
 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

Quoted 8 lines

[...]

Quoted 4 lines

+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.

Quoted 6 lines

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.

Quoted 8 lines
Quoted 3 lines

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.

Quoted 2 lines

[...]

#8 lore raw

PATCH 1/2 common: Generic file system firmware loader

Quoted 78 lines · Tien Fong
Quoted 62 lines · Marek Vasut
Quoted 61 lines · Tien Fong
Quoted 59 lines · Marek Vasut
Quoted 57 lines · Tien Fong
Quoted 29 lines · Marek Vasut
Quoted 18 lines · tien.fong.chee at intel.com

From: Tien Fong Chee <[email protected]>

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.

Quoted 8 lines

---
 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

Quoted 8 lines

[...]

Quoted 4 lines

+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.

Quoted 6 lines

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.

Quoted 8 lines
Quoted 3 lines

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.

Quoted 3 lines

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 ...

Signature · 2 lines
Best regards,
Marek Vasut
#9 lore raw

PATCH 1/2 common: Generic file system firmware loader

Quoted 100 lines · Marek Vasut
Quoted 99 lines · Tien Fong
Quoted 80 lines · Marek Vasut
Quoted 74 lines · Tien Fong
Quoted 62 lines · Marek Vasut
Quoted 59 lines · Tien Fong
Quoted 39 lines · Marek Vasut
Quoted 29 lines · tien.fong.chee at intel.com

From: Tien Fong Chee <[email protected]>

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.

Quoted 1 line

---
 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

[...]

Quoted 4 lines
Quoted 4 lines
Quoted 2 lines

+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.

Quoted 7 lines

I also mentioned the API should copy the linux firmware loader
API.

Quoted 9 lines
Quoted 9 lines

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.

Quoted 4 lines

I would like to confirm with you whether we are talking to the same
API
above?

Quoted 7 lines

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.

Quoted 9 lines
Quoted 5 lines

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.

#10 lore raw

PATCH 1/2 common: Generic file system firmware loader

Quoted 110 lines · Tien Fong
Quoted 100 lines · Marek Vasut
Quoted 99 lines · Tien Fong
Quoted 80 lines · Marek Vasut
Quoted 74 lines · Tien Fong
Quoted 62 lines · Marek Vasut
Quoted 59 lines · Tien Fong
Quoted 39 lines · Marek Vasut
Quoted 29 lines · tien.fong.chee at intel.com

From: Tien Fong Chee <[email protected]>

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.

Quoted 1 line

---
 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

[...]

Quoted 4 lines
Quoted 4 lines
Quoted 2 lines

+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.

Quoted 7 lines

I also mentioned the API should copy the linux firmware loader
API.

Quoted 9 lines
Quoted 9 lines

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.

Quoted 4 lines

I would like to confirm with you whether we are talking to the same
API
above?

Quoted 7 lines

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.

Quoted 9 lines
Quoted 5 lines

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 ...

Quoted 8 lines

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 ...

Signature · 2 lines
Best regards,
Marek Vasut
#11 lore raw

PATCH 1/2 common: Generic file system firmware loader

Quoted 141 lines · Marek Vasut
Quoted 140 lines · Tien Fong
Quoted 123 lines · Marek Vasut
Quoted 121 lines · Tien Fong
Quoted 99 lines · Marek Vasut
Quoted 91 lines · Tien Fong
Quoted 76 lines · Marek Vasut
Quoted 72 lines · Tien Fong
Quoted 49 lines · Marek Vasut
Quoted 36 lines · tien.fong.chee at intel.com

From: Tien Fong Chee <[email protected]>

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.

.com

Quoted 2 lines

---
 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

[...]

Quoted 5 lines
Quoted 4 lines
Quoted 2 lines

+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.

Quoted 9 lines

I also mentioned the API should copy the linux
firmware
loader
API.

Quoted 11 lines
Quoted 11 lines

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.

Quoted 5 lines

I would like to confirm with you whether we are talking to the
same
API
above?

Quoted 8 lines

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
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.

Quoted 9 lines
Quoted 5 lines

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.

Quoted 9 lines
Quoted 5 lines

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.

#12 lore raw

PATCH 1/2 common: Generic file system firmware loader

In reply to Marek Vasut
Quoted 115 lines · On Thu, 9 Nov 2017 08:05:18 +0100 Marek Vasut <[email protected]> wrote:
Quoted 114 lines · Tien Fong
Quoted 100 lines · Marek Vasut
Quoted 99 lines · Tien Fong
Quoted 80 lines · Marek Vasut
Quoted 74 lines · Tien Fong
Quoted 62 lines · Marek Vasut
Quoted 59 lines · Tien Fong
Quoted 39 lines · Marek Vasut
Quoted 29 lines · tien.fong.chee at intel.com

From: Tien Fong Chee <[email protected]>

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.

Quoted 1 line

---
 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

[...]

Quoted 4 lines
Quoted 4 lines
Quoted 2 lines

+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.

Quoted 7 lines

I also mentioned the API should copy the linux firmware loader
API.

Quoted 9 lines
Quoted 9 lines

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.

Quoted 4 lines

I would like to confirm with you whether we are talking to the same
API
above?

Quoted 7 lines

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.

Quoted 9 lines
Quoted 5 lines

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.

Quoted 8 lines
Quoted 4 lines

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 at denx.de -------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20171109/4b356a11/attachment.sig>

#13 lore raw

PATCH 1/2 common: Generic file system firmware loader

Quoted 123 lines · Lukasz Majewski
Quoted 120 lines · On Thu, 9 Nov 2017 08:05:18 +0100 Marek Vasut <[email protected]> wrote:
Quoted 118 lines · Tien Fong
Quoted 100 lines · Marek Vasut
Quoted 99 lines · Tien Fong
Quoted 80 lines · Marek Vasut
Quoted 74 lines · Tien Fong
Quoted 62 lines · Marek Vasut
Quoted 59 lines · Tien Fong
Quoted 39 lines · Marek Vasut
Quoted 29 lines · tien.fong.chee at intel.com

From: Tien Fong Chee <[email protected]>

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.

Quoted 1 line

---
 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

[...]

Quoted 4 lines
Quoted 4 lines
Quoted 2 lines

+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.

Quoted 7 lines

I also mentioned the API should copy the linux firmware loader
API.

Quoted 9 lines
Quoted 9 lines

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.

Quoted 4 lines

I would like to confirm with you whether we are talking to the same
API
above?

Quoted 7 lines

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.

Quoted 9 lines
Quoted 5 lines

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?

Quoted 6 lines
Quoted 3 lines

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.

Quoted 2 lines

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.

Quoted 3 lines

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.

Signature · 2 lines
Best regards,
Marek Vasut
#14 lore raw

PATCH 1/2 common: Generic file system firmware loader

Quoted 180 lines · Marek Vasut
Quoted 160 lines · Lukasz Majewski
Quoted 156 lines · On Thu, 9 Nov 2017 08:05:18 +0100 Marek Vasut <[email protected]> wrote:
Quoted 154 lines · Tien Fong
Quoted 134 lines · Marek Vasut
Quoted 125 lines · Tien Fong
Quoted 101 lines · Marek Vasut
Quoted 93 lines · Tien Fong
Quoted 78 lines · Marek Vasut
Quoted 74 lines · Tien Fong
Quoted 49 lines · Marek Vasut
Quoted 33 lines · tien.fong.chee at intel.com

From: Tien Fong Chee <[email protected]>

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.

ntel.com

Quoted 2 lines

---
 common/Makefile   |   1 +
 common/load_fs.c  | 217
+++++++++++++++++++++++++++++++++++++++++++++++
+++++++
 include/load_fs.h |  38 ++++++++++

Quoted 3 lines
 3 files changed, 256 insertions(+)

 create mode 100644 common/load_fs.c
 create mode 100644 include/load_fs.h

[...]

Quoted 5 lines
Quoted 4 lines
Quoted 2 lines

+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.

Quoted 9 lines

I also mentioned the API should copy the linux
firmware
loader
API.

Quoted 11 lines
Quoted 11 lines

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.

Quoted 5 lines

I would like to confirm with you whether we are talking to the
same
API
above?

Quoted 8 lines

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.

Quoted 7 lines

Linux loader is strongly designed based on Linux environment,
always
assume having RFF, env support(which SPL don't have), sysfs and
udev
feature.

Quoted 6 lines

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?

Quoted 7 lines
Quoted 4 lines

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.

Quoted 4 lines

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.

Quoted 1 line
Quoted 8 lines
Quoted 5 lines

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.

#15 lore raw

PATCH 1/2 common: Generic file system firmware loader

Quoted 189 lines · Tien Fong
Quoted 180 lines · Marek Vasut
Quoted 160 lines · Lukasz Majewski
Quoted 156 lines · On Thu, 9 Nov 2017 08:05:18 +0100 Marek Vasut <[email protected]> wrote:
Quoted 154 lines · Tien Fong
Quoted 134 lines · Marek Vasut
Quoted 125 lines · Tien Fong
Quoted 101 lines · Marek Vasut
Quoted 93 lines · Tien Fong
Quoted 78 lines · Marek Vasut
Quoted 74 lines · Tien Fong
Quoted 49 lines · Marek Vasut
Quoted 36 lines · tien.fong.chee at intel.com

From: Tien Fong Chee <[email protected]>

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

[...]

Quoted 5 lines
Quoted 4 lines
Quoted 2 lines

+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.

Quoted 9 lines

I also mentioned the API should copy the linux
firmware
loader
API.

Quoted 11 lines
Quoted 11 lines

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.

Quoted 5 lines

I would like to confirm with you whether we are talking to the
same
API
above?

Quoted 8 lines

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.

Quoted 7 lines

Linux loader is strongly designed based on Linux environment,
always
assume having RFF, env support(which SPL don't have), sysfs and
udev
feature.

Quoted 6 lines

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?

Quoted 7 lines
Quoted 4 lines

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.

Quoted 4 lines

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.

Quoted 1 line
Quoted 8 lines
Quoted 5 lines

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.

Quoted 6 lines

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.

Signature · 2 lines
Best regards,
Marek Vasut
#16 lore raw

PATCH 1/2 common: Generic file system firmware loader

Quoted 243 lines · Marek Vasut
Quoted 242 lines · Tien Fong
Quoted 231 lines · Marek Vasut
Quoted 203 lines · Lukasz Majewski
Quoted 192 lines · On Thu, 9 Nov 2017 08:05:18 +0100 Marek Vasut <[email protected]> wrote:
Quoted 182 lines · Tien Fong
Quoted 164 lines · Marek Vasut
Quoted 153 lines · Tien Fong
Quoted 129 lines · Marek Vasut
Quoted 114 lines · Tien Fong
Quoted 95 lines · Marek Vasut
Quoted 90 lines · Tien Fong
Quoted 66 lines · Marek Vasut

On 11/01/2017 10:18 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 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 at 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.

Quoted 11 lines

I also mentioned the API should copy the
linux
firmware
loader
API.

Quoted 13 lines
Quoted 5 lines

If i'm not mistaken, you are referring firmware
loader API
in
this
link https://github.com/torvalds/linux/blob/f00

Quoted 8 lines

7cad
159e99fa
2acd
3b2e
9364
fbb32ad28b971/drivers/base/firmware_class.c#L12
64.

Quoted 6 lines

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

Quoted 5 lines

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.

Quoted 8 lines

Linux loader is strongly designed based on Linux environment,
always
assume having RFF, env support(which SPL don't have), sysfs
and
udev
feature.

Quoted 7 lines

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.

Quoted 7 lines

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?

Quoted 6 lines

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
...

Quoted 3 lines

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.

Quoted 6 lines

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.

Quoted 8 lines

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.

Quoted 4 lines

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.

Quoted 5 lines
Quoted 2 lines

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.

#17 lore raw

PATCH 1/2 common: Generic file system firmware loader

In reply to Marek Vasut
Quoted 243 lines · Marek Vasut
Quoted 242 lines · Tien Fong
Quoted 231 lines · Marek Vasut
Quoted 203 lines · Lukasz Majewski
Quoted 192 lines · On Thu, 9 Nov 2017 08:05:18 +0100 Marek Vasut <[email protected]> wrote:
Quoted 182 lines · Tien Fong
Quoted 164 lines · Marek Vasut
Quoted 153 lines · Tien Fong
Quoted 129 lines · Marek Vasut
Quoted 114 lines · Tien Fong
Quoted 95 lines · Marek Vasut
Quoted 90 lines · Tien Fong
Quoted 66 lines · Marek Vasut

On 11/01/2017 10:18 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 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 at 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.

Quoted 11 lines

I also mentioned the API should copy the
linux
firmware
loader
API.

Quoted 13 lines
Quoted 5 lines

If i'm not mistaken, you are referring firmware
loader API
in
this
link https://github.com/torvalds/linux/blob/f00

Quoted 8 lines

7cad
159e99fa
2acd
3b2e
9364
fbb32ad28b971/drivers/base/firmware_class.c#L12
64.

Quoted 6 lines

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

Quoted 5 lines

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.

Quoted 8 lines

Linux loader is strongly designed based on Linux environment,
always
assume having RFF, env support(which SPL don't have), sysfs
and
udev
feature.

Quoted 7 lines

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.

Quoted 7 lines

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?

Quoted 6 lines

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
...

Quoted 3 lines

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.

Quoted 6 lines

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.

Quoted 8 lines

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.

Quoted 4 lines

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.

Quoted 5 lines
Quoted 2 lines

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.

#18 lore raw

PATCH 1/2 common: Generic file system firmware loader

Quoted 249 lines · Tien Fong
Quoted 243 lines · Marek Vasut
Quoted 242 lines · Tien Fong
Quoted 231 lines · Marek Vasut
Quoted 203 lines · Lukasz Majewski
Quoted 192 lines · On Thu, 9 Nov 2017 08:05:18 +0100 Marek Vasut <[email protected]> wrote:
Quoted 182 lines · Tien Fong
Quoted 164 lines · Marek Vasut
Quoted 153 lines · Tien Fong
Quoted 129 lines · Marek Vasut
Quoted 114 lines · Tien Fong
Quoted 95 lines · Marek Vasut
Quoted 90 lines · Tien Fong

On Rab, 2017-11-01 at 10:26 +0100, Marek Vasut
wrote:

On 11/01/2017 10:18 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 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 at 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.

Quoted 13 lines
Quoted 13 lines

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.

Quoted 6 lines

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

Quoted 5 lines

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.

Quoted 8 lines

Linux loader is strongly designed based on Linux environment,
always
assume having RFF, env support(which SPL don't have), sysfs
and
udev
feature.

Quoted 7 lines

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.

Quoted 7 lines

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?

Quoted 6 lines

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
...

Quoted 3 lines

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.

Quoted 6 lines

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.

Quoted 8 lines

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.

Quoted 4 lines

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.

Quoted 5 lines
Quoted 2 lines

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.

Quoted 6 lines

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.

Quoted 2 lines

Thanks.

Signature · 2 lines
Best regards,
Marek Vasut
#19 lore raw

PATCH 2/2 common: Convert splash FS loader to use generic FS firmware loader

In reply to tien.fong.chee at intel.com

From: Tien Fong Chee <[email protected]>

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.

---
 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(-)
board/compulab/cm_fx6/cm_fx6.c +8 -9
index 638e9f3..4404979 100644
@@ -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",
 	},
 };
board/compulab/cm_t35/cm_t35.c +3 -3
index be938eb..6ae84a0 100644
@@ -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,
 	},
 };
common/splash.c +9 -9
index d251b3b..9ed6aa6 100644
@@ -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",
 	},
 };
common/splash_source.c +44 -149

262 lines not on this page · raw message

include/splash.h +3 -26
index 228aff4..5ca2b6a 100644
@@ -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;
Signature · 1 line
2.2.0