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

[Lukasz Majewski]

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.

What I would like to have though is a some kind of generic code, which
would allow me to reuse it on other ARM + DSP SoCs.....

If we cannot re-use Linux stuff, then when we add something different
(more customer/industry aligned), please make it reusable for other
solutions (Xilinx, ADI, etc) - that would require a good documentation.

Those are my 2 cents.

Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20171109/4b356a11/attachment.sig>