[U-Boot,1/2] common: Generic file system firmware loader

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

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);
> +		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

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

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

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

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

> > --------------------------       ---------------------------

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

> 

> > 

> > > 

> > > > 

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

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

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

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

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

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

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

> 

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

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

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?

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