On Mon, 2019-02-11 at 12:01 +0100, Marek Vasut wrote:
On 2/11/19 6:36 AM, Chee, Tien Fong wrote:
On Tue, 2019-02-05 at 09:46 +0100, Marek Vasut wrote:
On 2/1/19 5:02 PM, Chee, Tien Fong wrote:
On Fri, 2019-02-01 at 09:25 +0100, Marek Vasut wrote:
On 2/1/19 4:48 AM, Chee, Tien Fong wrote:
On Thu, 2019-01-31 at 15:54 +0100, Marek Vasut wrote: > > > > On 1/31/19 3:51 PM, tien.fong.chee@intel.com wrote: > > > > > > > > > > From: Tien Fong Chee tien.fong.chee@intel.com > > > > This patch adds description on properties about file > > name > > used > > for > > both > > peripheral bitstream and core bitstream. > > > > Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com > > > > > > > --- > > > > changes for v7 > > - Provided example of setting FPGA FIT image for both > > early > > IO > > release > > and full release FPGA configuration. > > --- > > .../fpga/altera-socfpga-a10-fpga-mgr.txt | > > 34 > > +++++++++++++++++++++- > > 1 file changed, 33 insertions(+), 1 deletion(-) > > > > diff --git a/doc/device-tree-bindings/fpga/altera- > > socfpga- > > a10- > > fpga- > > mgr.txt b/doc/device-tree-bindings/fpga/altera-socfpga- > > a10- > > fpga- > > mgr.txt > > index 2fd8e7a..5f81a32 100644 > > --- a/doc/device-tree-bindings/fpga/altera-socfpga-a10- > > fpga- > > mgr.txt > > +++ b/doc/device-tree-bindings/fpga/altera-socfpga-a10- > > fpga- > > mgr.txt > > @@ -7,8 +7,39 @@ Required properties: > > - The second index is for writing FPGA > > configuration data. > > - resets : Phandle and reset specifier for the > > device's > > reset. > > - clocks : Clocks used by the device. > > +- altr,bitstream : File name for FPGA peripheral > > bitstream > > which > > is used > > + to initialize FPGA IOs, PLL, IO48 > > and > > DDR. > > This > > bitstream is > > + required to get DDR up running. > > + or > > + File name for full bitstream, > > consist > > of > > peripheral bitstream > > + and core bitstream. > > +- altr,bitstream-core(optional) : File name for core > > bitstream > > which contains > Is the name of the property 'altr,bitstream- > core(optional)' ? > I > think > the "optional" part should be in the description. Yes, you are right. > > > > > > > > > > > > > > + FPGA design which is > > used to > > program FPGA CRAM > > + and ERAM. > > > > -Example: > > +Example: Bundles both peripheral bitstream and core > > bitstream > > into > > FIT image > > + called fit_spl_fpga.itb. This FIT image can > > be > > created > > through running > > + this command: tools/mkimage > > + -E -p 400 > > + -f board/altera/arria10- > > socdk/fit_spl_fpga.its > > + fit_spl_fpga.itb > > + > > + For details of describing structure and > > contents > > of > > the > > FIT image, > > + please refer board/altera/arria10- > > socdk/fit_spl_fpga.its > > + > > +- Examples for booting with early IO release, and > > enter > > early > > user > > mode: > > + > > + fpga_mgr: fpga-mgr@ffd03000 { > > + compatible = "altr,socfpga-a10-fpga- > > mgr"; > > + reg = <0xffd03000 0x100 > > + 0xffcfe400 0x20>; > > + clocks = <&l4_mp_clk>; > > + resets = <&rst FPGAMGR_RESET>; > > + altr,bitstream = "fit_spl_fpga.itb"; > > + altr,bitstream-core = > > "fit_spl_fpga.itb"; > It's the same file, why does it use two properties ?
- Allows user to run optional for program core. When "" is
set to altr,bitstream-core, then SPL would skip programming FPGA with core, so user can program it later on U-Boot or Linux.
You can just pass in a fitImage with only the periph image in it in such a case.
What if user want to program the core on U-Boot? User need to create two FIT images, one FIT with periph image, and another FIT with core image only.
Current implementation supports one FIT image for above configuration.
What if user want to program the core on U-Boot in this implementation? It is not possible either, is it ?
There are few ways user can do:
- Running commands such as imxtract/fatload with socfpga load
- Script
- Env
And how is that different from using a single fitImage with configuration node describing the parts of the bitstream that should be loaded ?
Please correct me if my answer doesn't match your question.
In previous implementation, fpga-1 must be periph.rbf, and fpga-2 must always be core.rbf. If altr,bitstream-core is not defined, then SPL would skip the core configuration. If user want to program core on U- Boot, then user need to extract the fpga-2 and program it.
Current proposal, altr,bitstream-core would be removed, and this would replaced by the rule of describing sequence of the fpga strings set in the fpga property in fitImage with configuration.
For property "fpga" = 1st string is for periph.rbf or full rbf.> > = 2nd string is for core.rbf. This is optional, no> > string is found, the core configuration would be skipped.> > = 3rd string onwards would be ignored.> >
For example, if user want to program periph.rbf or full rbf, user can describe the fpga sequence as below configurations { default = "config-1"; config-1 { fpga = "fpga-1"; <=perih.rbf or full rbf };
};
if user want to program periph.rbf and core.rbf in SPL configurations { default = "config-1"; config-1 { fpga = "fpga-1", "fpga-2"; };
};
if user want to program periph.rbf in spl, but core.rbf in U-Boot configurations { default = "config-1"; config-1 { fpga = "fpga-1"; <=perih.rbf }; config-2 { fpga = "fpga-1", "fpga-2"; };
};
So for core.rbf, user need to look the config-2, then extract the fpga- 2 from few ways described above.
- Allows core in different FIT file.
Is this really useful ?
Yes, for the use case which support different core image for different revision of board but using same periph image.
Seems you an Dalon are discussing this and it's not a supported flow ?
Dalon has some concerns.
But, we can keep the more flexibility for users by adding support of configuration in fit image, then removing the altr,bitstream-core properties in FPGA manager node. So, users are freely to use this implementation in their own use cases.
So, user need to add the default configuration for FPGA. For property "fpga" = 1st string is for periph.rbf or full rbf. = 2nd string is for core.rbf. This is optional, no string is found, the core configuration would be skipped. = 3rd string onwards would be ignored.
configurations { default = "config-1"; config-1 { description = "Boot with FPGA early IO release config"; fpga = "fpga-1", "fpga-2"; <= sequence for bitstream configuration }; };
I want to be able to do something like configuration { ... config-board1 { ... fpga = "fpga-core1"; }; config-board2 { ... fpga = "fpga-core2", "fpga-periph2"; }; };
Does this make sense ?
1. How user can tell the SPL which config if there is no default property described?
2. You want the code to recognize the string "fpga-core" for core.rbf and "fpga-periph" for periph.rbf?
3. What's your concern with the 1st and 2nd string represent periph.rbf and core.rbf respectively?
Dalon, is this mix-and-match approach to bitstreams something we want to support at all ? It seems a bit dangerous to me.
Anyway, please bear in mind that multi fpga nodes in one fpga fit image is not good for performance. I have tried few ideas to fix the performance penalty caused by buffer alignment at reading cluster, but it didn't work.
What do you think?
I think if there is a performance penalty when loading file from VFAT, it is VFAT code or SD/MMC driver that needs fixing. We definitely can not hack it up by realigning fitImage to magically work around that.
Please see my replied in patch 2/7
> > > And where is this > file loaded from ? You need to set the default source in DTS, for example "firmware- loader = &fs_loader0", that's for power boot up purpose. After that, generic firmware loader would go to the dsignated storage as described below to find the FPGA FIT image according description from above.
fs_loader0: fs-loader@0 { u-boot,dm-pre-reloc; compatible = "u-boot,fs-loader"; phandlepart = <&mmc 1>; };
How does the driver bound to fpga-mgr know which firmware loader instance to use ? There's no phandle.
Current firmware loader supports only one instance, that is default instance described in chosen node. It is good enough to solve our problem where to define a default storage for FPGA images and how to tell the SPL to load it from the default storage when the board is powered up. I don't see there is a need to support more than one instance for fpga-mgr during SPL runtime, at least for now. User can program the FPGA with core image from any storage with series of commands such as fatload and socfpga load on U-Boot console.
Since you already have the label for the fs-loader _and_ you have the address for it (fs-loader@0), you should supply the phandle. And eventually, someone will try to use multiple loaders, so we should do this correctly from the beginning -- by supplying the phandle to the loader node.
Okay, i will improve firmware loader in separate patch set. Basically, the ideas are for
- For majority HWs use default storage, defined in console node.
(Already supported)
I'm not sure I understand this point, default storage in console node ?
For example, we have 5 fpga manager nodes.
fpga_mgr1, fpga_mgr2, fpga_mgr3, and fpga_mgr4 are using sdmmc as storage, so we can define sdmmc as default storage in console node instead of setting phandle to each of fpga_mgr nodes.
Let say fpga_mgr5 is using QSPI, then we can set "firmware-loader = <phandle>" to fpga_mgr5.
- For minority HWs use, other than default storage, users should
add this property "firmware-loader = <phandle>" to their respective HW nodes. This would enable overwritten of default storage, phandle from console node. (planning to enhance) 3. Enable sequence number(DM_FLAG_PRE_RELOC) support through hard coding. (planning to enhance)
Why do we want to hard-code anything ?
The purpose here is to support the feature of DM_FLAG_PRE_RELOC. One of the use case i think of searching the file such as u-boot.img in every storage supported in firmware loader. I have no strong opinion for item 3, we can support it in later when it is needed.
So, the interface is still same for this patch set, using item 1, default storage.
What do you think?
It is good to improve the firmware loader to support the DM_FLAG_PRE_RELOC, which allow user to choose different firmware loader node through setting the right sequence number when creating the firmare loader instance in the parent driver such as fpga mgr, but i don't see there is urgency need to be done now.
> > > > > > > > > > > + }; > > + > > +- Examples for booting with full release, enter user > > mode > > with > > full bitstream: > > > > fpga_mgr: fpga-mgr@ffd03000 { > > compatible = "altr,socfpga-a10-fpga- > > mgr"; > > @@ -16,4 +47,5 @@ Example: > > 0xffcfe400 0x20>; > > clocks = <&l4_mp_clk>; > > resets = <&rst FPGAMGR_RESET>; > > + altr,bitstream = "fit_spl_fpga.itb"; > > }; > >