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

[Marek Vasut]

On 11/09/2017 11:00 AM, Lukasz Majewski wrote:
> On Thu, 9 Nov 2017 08:05:18 +0100
> Marek Vasut <marex at 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 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 ...
>>
> 
> 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.

-- 
Best regards,
Marek Vasut