[U-Boot] [PATCH v7 1/7] ARM: socfpga: Description on FPGA bitstream type and file name for Arria 10
Dalon L Westergreen
dalon.westergreen at linux.intel.com
Mon Feb 11 16:33:34 UTC 2019
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 at intel.com wrote:
> > > > > > > >
> > > > > > > > From: Tien Fong Chee <tien.fong.chee at 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 at 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.
> > > 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.
We definitely do not want to support using different core images with a
single periph image. That said, i would like to see a single fit image
have multiple fpga images with the goal of allowing board specific code to
do a select on the config much like is supported now in devicetrees,
I believe.
> > 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.
I agree. Let the performance fall where is may. The VFAT or SD/MMC code should
be fixed. If the alignment in the fitimage happens to work out, then great.
Until the issue fixed, users can easily configure the core FPGA using a u-boot
script or in linux.
--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.
> > > Since you already have the label for the fs-loader _and_ you have the
> > > address for it (fs-loader at 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 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";
> > > > > > > > };
> > > > > > > >
More information about the U-Boot
mailing list