[U-Boot] [PATCH v7 1/7] ARM: socfpga: Description on FPGA bitstream type and file name for Arria 10

Dalon L Westergreen
2019-02-01T20:29:50Z

On Fri, 2019-02-01 at 12:02 -0800, Dalon L Westergreen wrote:
> On Sat, 2019-02-02 at 00:02 +0800, 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 at 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 at 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.
> > > > 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.
>
> I would have to disagree here. This is not a supported flow, the periph image
> and core image must come from the same compilation of quartus for arria10 to
> guarantee functionality. That is unless something has changed that I am not
> aware of? I believe this holds true in S10, the difference being that in s10
> you can reconfigure the periph rbf without affecting the processor IO.
>
> Why are there separate fit images for uboot and the fpga? would it not
> make sense to combine them into a single fit image? Wouldn't you want to
> couple a devicetree and a periph image together and then allow for
> selection of configurations?
>
> I also wonder why there is a need to support core image programming
> in SPL? If the desire is to have the full image programmed, would
> you not just create a single rbf (combined periph and core?)
>

Looking further, i see now why you want a separate core image. I do
see a use case for multiple fpga images in the fit image though and
allowing selecting a configuration as in:

u-boot/doc/uImage.FIT/multi-with-fpga.its

--dalon

> --dalon
>
> > > > > 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 at 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.
> >
> > 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 at 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";
> > > > > > };
> > > > > >