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.c...@intel.com wrote: >>>>>>> >>>>>>> >>>>>>> >>>>>>> From: Tien Fong Chee <tien.fong.c...@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.c...@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 ? >>>>> 1. 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: > 1. Running commands such as imxtract/fatload with socfpga load > 2. Script > 3. Env
And how is that different from using a single fitImage with configuration node describing the parts of the bitstream that should be loaded ? >>>>> 2. 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 ? 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. >> >>> >>>> >>>>> >>>>>> >>>>>> 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 > 1. 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 ? > 2. 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 ? > 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"; >>>>>>> }; >>>>>>> -- Best regards, Marek Vasut _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot