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

Thu Nov 16 08:11:14 UTC 2017

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 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.ch
>>>>>>>>>>>>>>> ee at i
>>>>>>>>>>>>>>> ntel.com  
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>  
>>>>>>>>>>>>>>> ---
>>>>>>>>>>>>>>>  common/Makefile   |   1 +
>>>>>>>>>>>>>>>  common/load_fs.c  | 217
>>>>>>>>>>>>>>> +++++++++++++++++++++++++++++++++++++++++++
>>>>>>>>>>>>>>> ++++
>>>>>>>>>>>>>>> +++++++
>>>>>>>>>>>>>>>  include/load_fs.h |  38 ++++++++++
>>>>>>>>>>>>>>>  3 files changed, 256 insertions(+)
>>>>>>>>>>>>>>>  create mode 100644 common/load_fs.c
>>>>>>>>>>>>>>>  create mode 100644 include/load_fs.h  
>>>>>>>>>>>>>> [...]
>>>>>>>>>>>>>>  
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> +int flash_select_fs_dev(struct
>>>>>>>>>>>>>>> flash_location
>>>>>>>>>>>>>>> *location)  
>>>>>>>>>>>>>> Why does everything have flash_ prefix ?
>>>>>>>>>>>>>>  
>>>>>>>>>>>>> I can remove the flash_ prefix, this generic FS
>>>>>>>>>>>>> loader
>>>>>>>>>>>>> should
>>>>>>>>>>>>> support
>>>>>>>>>>>>> for all filesystem instead of flash.
>>>>>>>>>>>>>  
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> I also mentioned the API should copy the
>>>>>>>>>>>>>> linux
>>>>>>>>>>>>>> firmware
>>>>>>>>>>>>>> loader
>>>>>>>>>>>>>> API.
>>>>>>>>>>>>>>  
>>>>>>>>>>>>> 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.
> 

-- 
Best regards,
Marek Vasut