From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chee, Tien Fong Date: Mon, 13 Nov 2017 04:31:28 +0000 Subject: [U-Boot] [PATCH 1/2] common: Generic file system firmware loader In-Reply-To: <5d2b0215-8086-9816-14b4-e57d7c52be65@denx.de> References: <1509527902-5080-1-git-send-email-tien.fong.chee@intel.com> <1509527902-5080-2-git-send-email-tien.fong.chee@intel.com> <05168571-9b72-0ff5-e1c7-07c47b2222a6@denx.de> <1509610841.2368.18.camel@intel.com> <1509941716.2383.1.camel@intel.com> <25cf9828-2888-45a7-5df5-2d879460e6f8@denx.de> <1510045437.2479.10.camel@intel.com> <1510207462.2518.6.camel@intel.com> <20171109110019.622dcd3d@jawa> <1510304705.2328.5.camel@intel.com> <5d2b0215-8086-9816-14b4-e57d7c52be65@denx.de> Message-ID: <1510547486.3140.0.camel@intel.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit To: u-boot@lists.denx.de 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 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 > > > > > > > > > > > > > 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 > > > > > > > > > > > > > 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.