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

Chee, Tien Fong
2019-02-02T02:37:12Z

On Fri, 2019-02-01 at 12:29 -0800, Dalon L Westergreen wrote:
> 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.
That's work when the same design on periph rbf, and different design
only on core rbf. So, you can pass in FIT image with static periph rbf
at altr,bitstream, then another FIT image just for core rbf only
at altr,bitstream-core.
> >
> > 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 assume this question is for item 1. Item 1 allows SPL easily skipping
the default support core image programming without codes changes, and
also keeping both periph and core in one FIT image, so you can program
the core image after SPL.
> >
> > 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?)
That's is default implementation for SPL to program core, but you can
easily change that without code changes and keeping both RBFs in same
FIT image as long as item 1 is still there.

There is a big difference in the performance between full image and
split rbfs. Full image is programmed into FPGA chunk by chunk on small
OCRAM.
The core for split rbfs is programmed into FPGA in one big chunk on
DDR.
> >
> 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
Yes, you can use spl_load_fit_image from common framework to support
this.
>
> --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";
> > > > > > >   };
> > > > > > >