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

[Marek Vasut]

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 at intel.com wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> From: Tien Fong Chee <tien.fong.chee at 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 at 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 ...

-- 
Best regards,
Marek Vasut