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

Mon Nov 13 04:31:28 UTC 2017

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