[U-Boot] [PATCH v7 0/7] Add support for loading FPGA bitstream
From: Tien Fong Chee tien.fong.chee@intel.com
These series of patches enable peripheral bitstream being programmed into FPGA to get the DDR up running. This's also called early IO release, because the peripheral bitstream is only initializing FPGA IOs, PLL, IO48 and DDR.
Once DDR is up running, core bitstream from MMC which contains user FPGA design would be loaded into DDR location. socfpga loadfs would be called to program core bitstream into FPGA and entering user mode.
Lastly, u-boot-dtb.img from MMC FAT partition would be loaded to DDR, and up running from there.
For this whole mechanism to work, the SDMMC flash layout would be designed as shown in below:
RAW partition: 1. spl_w_dtb-mkpimage.bin mkpimage -hv 1 -o spl/spl_w_dtb-mkpimage.bin spl/u-boot-spl-dtb.bin spl/u-boot-spl-dtb.bin spl/u-boot-spl-dtb.bin spl/u-boot-spl-dtb.bin
FAT partition contains: FPGA FIT image(fit_spl_fpga.itb) -------------------------------- Early IO release method is recommended for the sake of performance, improve up to 86% compare to full bitstream.
To generate FPGA FIT image for Early IO release, runs this command: tools/mkimage -E -p 400 -f board/altera/arria10-socdk/fit_spl_fpga.its fit_spl_fpga.itb
Before running above command, ensure U-Boot source directory contains both peripheral bitstream(ghrd_10as066n2.periph.rbf) and core bitstream(ghrd_10as066n2.core.rbf_.
For details of describing structure and contents of the FIT image, please refer board/altera/arria10-socdk/fit_spl_fpga.its
Additonal note: --------------- There is performance penalty if the memory used for loading the bitstream is unalligned to ARCH_DMA_MINALIGN inside function "static int get_cluster()" in fs/fat/fat.c. Sometimes child functions would alter the memory location and causing unalligned to ARCH_DMA_MINALIGN. This only happend when reading a FAT file by offset.
To avoid performance penalty, core bitstream(fpga-2) contains large size should be at 1st then following by peripheral bitstream(fpga-1) as shown in fit_spl_fpga.its.
After that, enable the DEBUG in fs/fat/fat.c and ensure no print out "FAT: Misaligned buffer address (%p)" when loading core bitstream on SPL console. Print out when loading peripheral bitstream can be ignored because the size is too small.
If there is print out, please set an allign data position by running mkimage with optional -p xxx when generating FIT image.
U-Boot image ------------ 3. u-boot-dtb.img
For the testing purpose, these 3 series of patches are required to apply 1st before applying this series of patches. 1. [U-Boot] [PATCH v4] misc: fs_loader: Add support for initializing block device https://www.mail-archive.com/u-boot@lists.denx.de/msg314453.html Version 4 under review
2. [U-Boot] [PATCH v2 1/4] spl: Kconfig: Replace CONFIG_SPL_FAT_SUPPORT with CONFIG_SPL_FS_FAT https://www.mail-archive.com/u-boot@lists.denx.de/msg313259.html All series Reviewed-by: Tom Rini trini@konsulko.com Some patches of the series Reviewed-by: Simon Goldschmidt
3. [U-Boot] [PATCH v2 1/2] fs: fat: dynamically allocate memory for temporary buffer https://www.mail-archive.com/u-boot@lists.denx.de/msg314470.html Version 2 under review
This series is working on top of u-boot.git - http://git.denx.de/u-boot.git .
Tien Fong Chee (7): ARM: socfpga: Description on FPGA bitstream type and file name for Arria 10 ARM: socfpga: Add default FPGA bitstream fitImage for Arria10 SoCDK ARM: socfpga: Add FPGA drivers for Arria 10 FPGA bitstream loading ARM: socfpga: Add the configuration for FPGA SoCFPGA A10 SoCDK spl : socfpga: Implement fpga bitstream loading with socfpga loadfs ARM: socfpga: Synchronize the configuration for A10 SoCDK ARM: socfpga: Increase Malloc pool size to support FAT filesystem in SPL
arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts | 18 + .../include/mach/fpga_manager_arria10.h | 39 +- arch/arm/mach-socfpga/spl_a10.c | 46 ++- board/altera/arria10-socdk/fit_spl_fpga.its | 31 ++ configs/socfpga_arria10_defconfig | 21 +- .../fpga/altera-socfpga-a10-fpga-mgr.txt | 34 +- drivers/fpga/socfpga_arria10.c | 417 ++++++++++++++++++++- include/configs/socfpga_common.h | 4 +- 8 files changed, 584 insertions(+), 26 deletions(-) create mode 100644 board/altera/arria10-socdk/fit_spl_fpga.its
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 + 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"; + }; + +- 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"; };
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 isrequired to get DDR up running.orFile name for full bitstream, consist of peripheral bitstreamand 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.
FPGA design which is used to program FPGA CRAMand 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 runningthis command: tools/mkimage-E -p 400-f board/altera/arria10-socdk/fit_spl_fpga.itsfit_spl_fpga.itbFor 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 0x1000xffcfe400 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 ? And where is this file loaded from ?
- };
+- 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";
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. Thisbitstream is
required to get DDR up running.orFile name for full bitstream, consist ofperipheral 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 toprogram 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 0x1000xffcfe400 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. 2. Allows core in different FIT file.
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>; };
- };
+- 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";};
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. Thisbitstream is
required to get DDR up running.orFile name for full bitstream, consist ofperipheral 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 toprogram 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 0x1000xffcfe400 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.
- Allows core in different FIT file.
Is this really useful ?
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.
- };
+- 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";};
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.orFile name for full bitstream, consist ofperipheral 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 toprogram 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 0x1000xffcfe400 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.
- 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.
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.
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";};
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@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.orFile name for full bitstream, consist ofperipheral 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 toprogram 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 becreated through running
this command: tools/mkimage-E -p 400-f board/altera/arria10-socdk/fit_spl_fpga.its
fit_spl_fpga.itbFor details of describing structure and contents ofthe 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 0x1000xffcfe400 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.
- 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?)
--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@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@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";
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@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.orFile name for full bitstream, consist ofperipheral 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 toprogram 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 becreated through running
this command: tools/mkimage-E -p 400-f board/altera/arria10-socdk/fit_spl_fpga.its
fit_spl_fpga.itbFor details of describing structure and contents ofthe 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 0x1000xffcfe400 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.
- 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@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@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";
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@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.
- 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@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@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"; > }; >
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.orFile name for full bitstream, consist ofperipheral 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 toprogram 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 0x1000xffcfe400 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 ?
- 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 ?
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.
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";};
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 andDDR. This bitstream is
required to get DDR up running.orFile name for full bitstream, consistof 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 isused 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 0x1000xffcfe400 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: 1. Running commands such as imxtract/fatload with socfpga load 2. Script 3. Env
- 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 }; };
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?
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) 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)
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";};
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 ?
- 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
- 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 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"; > }; >
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 ?
- 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@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 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"; > > }; > >
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"; > > }; > >
On Tue, 2019-02-12 at 01:01 +0800, Chee, Tien Fong wrote:
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 ? > 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:
- 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.
Or, store the core rbf outside of the fit image, no?
my preference for the fit image would be
... images { fpga@1 { description = "FPGA Periph"; ... type = "fpga_periph"; ... } fpga@2 { description = "FPGA Core"; ... type = "fpga" or "fpga_core"; ... } }; configurations { default = "config@1" config@1 { fpga = "fpga@1"; // periph only }; config@2 { fpga = "fpga@1", "fpga@2"; }; };
with the expectation that the order of fpga@1 and fpga@2 in config@2 is not relevant. the code should find the fpga_periph type and program it first. just my comment, i dont like rellying on the order or name.
--dalon
> 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 ?
- How user can tell the SPL which config if there is no default
property described?
- You want the code to recognize the string "fpga-core" for core.rbf
and "fpga-periph" for periph.rbf?
- 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"; > > > }; > > >
On Mon, 2019-02-11 at 17:19 +0000, Westergreen, Dalon wrote:
On Tue, 2019-02-12 at 01:01 +0800, Chee, Tien Fong wrote:
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 ? > > 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:
- 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.
Or, store the core rbf outside of the fit image, no?
my preference for the fit image would be
... images { fpga@1 { description = "FPGA Periph"; ... type = "fpga_periph"; ... } fpga@2 { description = "FPGA Core"; ... type = "fpga" or "fpga_core";
I'm good with "fpga".
... } }; configurations { default = "config@1" config@1 { fpga = "fpga@1"; // periph only }; config@2 { fpga = "fpga@1", "fpga@2"; }; };
with the expectation that the order of fpga@1 and fpga@2 in config@2 is not relevant. the code should find the fpga_periph type and program it first. just my comment, i dont like rellying on the order or name.
I can add support for above implementation although this adds more complexity to the driver.
Marek, are you OK with this implementation?
--dalon
> > > > > 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 ?
- How user can tell the SPL which config if there is no default
property described?
- You want the code to recognize the string "fpga-core" for
core.rbf and "fpga-periph" for periph.rbf?
- 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"; > > > > }; > > > >
On 2/12/19 10:35 AM, Chee, Tien Fong wrote: [...]
my preference for the fit image would be
... images { fpga@1 { description = "FPGA Periph"; ... type = "fpga_periph"; ... } fpga@2 { description = "FPGA Core"; ... type = "fpga" or "fpga_core";
I'm good with "fpga".
... } }; configurations { default = "config@1" config@1 { fpga = "fpga@1"; // periph only }; config@2 { fpga = "fpga@1", "fpga@2"; }; };
with the expectation that the order of fpga@1 and fpga@2 in config@2 is not relevant. the code should find the fpga_periph type and program it first. just my comment, i dont like rellying on the order or name.
I can add support for above implementation although this adds more complexity to the driver.
You can have fpga node and e.g. fpga-name node in the configurations section to discern which phandle there is the core and which is the peripheral RBF. Would that work ?
Marek, are you OK with this implementation?
Looks OK to me. Dalon ?
[...]
On Tue, 2019-02-12 at 10:43 +0100, Marek Vasut wrote:
On 2/12/19 10:35 AM, Chee, Tien Fong wrote: [...]
my preference for the fit image would be
... images { fpga@1 { description = "FPGA Periph"; ... type = "fpga_periph"; ... } fpga@2 { description = "FPGA Core"; ... type = "fpga" or "fpga_core";
I'm good with "fpga".
... } }; configurations { default = "config@1" config@1 { fpga = "fpga@1"; // periph only }; config@2 { fpga = "fpga@1", "fpga@2"; }; };
with the expectation that the order of fpga@1 and fpga@2 in confi g@2 is not relevant. the code should find the fpga_periph type and program it first. just my comment, i dont like rellying on the order or name.
I can add support for above implementation although this adds more complexity to the driver.
You can have fpga node and e.g. fpga-name node in the configurations section to discern which phandle there is the core and which is the peripheral RBF. Would that work ?
So something like that?
...
images { fpga-periph@1 { description = "FPGA Periph"; ... type = "fpga_periph"; ... } fpga-core@2 { description = "FPGA Core"; ... type = "fpga"; ... } }; configurations { default = "config@1" config@1 { fpga = "fpga-periph@1"; // periph only }; config@2 { fpga = "fpga-periph@1", "fpga-core@2"; }; };
Marek, are you OK with this implementation?
Looks OK to me. Dalon ?
[...]
On 2/12/19 11:13 AM, Chee, Tien Fong wrote:
On Tue, 2019-02-12 at 10:43 +0100, Marek Vasut wrote:
On 2/12/19 10:35 AM, Chee, Tien Fong wrote: [...]
my preference for the fit image would be
... images { fpga@1 { description = "FPGA Periph"; ... type = "fpga_periph"; ... } fpga@2 { description = "FPGA Core"; ... type = "fpga" or "fpga_core";
I'm good with "fpga".
... } }; configurations { default = "config@1" config@1 { fpga = "fpga@1"; // periph only }; config@2 { fpga = "fpga@1", "fpga@2"; }; };
with the expectation that the order of fpga@1 and fpga@2 in confi g@2 is not relevant. the code should find the fpga_periph type and program it first. just my comment, i dont like rellying on the order or name.
I can add support for above implementation although this adds more complexity to the driver.
You can have fpga node and e.g. fpga-name node in the configurations section to discern which phandle there is the core and which is the peripheral RBF. Would that work ?
So something like that?
...
images { fpga-periph@1 { description = "FPGA Periph"; ... type = "fpga_periph";
Do we need a new type for the periph/core distinction ?
... } fpga-core@2 { description = "FPGA Core"; ... type = "fpga"; ... } }; configurations { default = "config@1" config@1 { fpga = "fpga-periph@1"; // periph only }; config@2 { fpga = "fpga-periph@1", "fpga-core@2"; }; };
Marek, are you OK with this implementation?
Looks OK to me. Dalon ?
[...]
On Tue, 2019-02-12 at 11:17 +0100, Marek Vasut wrote:
On 2/12/19 11:13 AM, Chee, Tien Fong wrote:
On Tue, 2019-02-12 at 10:43 +0100, Marek Vasut wrote:
On 2/12/19 10:35 AM, Chee, Tien Fong wrote: [...]
my preference for the fit image would be
... images { fpga@1 { description = "FPGA Periph"; ... type = "fpga_periph"; ... } fpga@2 { description = "FPGA Core"; ... type = "fpga" or "fpga_core";
I'm good with "fpga".
... } }; configurations { default = "config@1" config@1 { fpga = "fpga@1"; // periph only }; config@2 { fpga = "fpga@1", "fpga@2"; }; };
with the expectation that the order of fpga@1 and fpga@2 in confi g@2 is not relevant. the code should find the fpga_periph type and program it first. just my comment, i dont like rellying on the order or name.
I can add support for above implementation although this adds more complexity to the driver.
You can have fpga node and e.g. fpga-name node in the configurations section to discern which phandle there is the core and which is the peripheral RBF. Would that work ?
So something like that?
...
images { fpga-periph@1 { description = "FPGA Periph"; ... type = "fpga_periph";
Do we need a new type for the periph/core distinction ?
not really, using the node name is fine with me as is shown below. when i suggested that my intent was to allow the name to be independant of the node name still envisioning scenarios where the fit image has multiple fpga images.
... } fpga-core@2 { description = "FPGA Core"; ... type = "fpga"; ... } }; configurations { default = "config@1" config@1 { fpga = "fpga-periph@1"; // periph only }; config@2 { fpga = "fpga-periph@1", "fpga-core@2"; }; };
Marek, are you OK with this implementation?
Looks OK to me. Dalon ?
[...]
On Tue, 2019-02-12 at 05:49 -0800, Dalon L Westergreen wrote:
On Tue, 2019-02-12 at 11:17 +0100, Marek Vasut wrote:
On 2/12/19 11:13 AM, Chee, Tien Fong wrote:
On Tue, 2019-02-12 at 10:43 +0100, Marek Vasut wrote:
On 2/12/19 10:35 AM, Chee, Tien Fong wrote: [...]
my preference for the fit image would be
... images { fpga@1 { description = "FPGA Periph"; ... type = "fpga_periph"; ... } fpga@2 { description = "FPGA Core"; ... type = "fpga" or "fpga_core";
I'm good with "fpga".
... } }; configurations { default = "config@1" config@1 { fpga = "fpga@1"; // periph only }; config@2 { fpga = "fpga@1", "fpga@2"; }; };
with the expectation that the order of fpga@1 and fpga@2 in confi g@2 is not relevant. the code should find the fpga_periph type and program it first. just my comment, i dont like rellying on the order or name.
I can add support for above implementation although this adds more complexity to the driver.
You can have fpga node and e.g. fpga-name node in the configurations section to discern which phandle there is the core and which is the peripheral RBF. Would that work ?
So something like that?
...
images { fpga-periph@1 { description = "FPGA Periph"; ... type = "fpga_periph";
Do we need a new type for the periph/core distinction ?
not really, using the node name is fine with me as is shown below. when i suggested that my intent was to allow the name to be independant of the node name still envisioning scenarios where the fit image has multiple fpga images.
Great, then i will work out the codes which able to identify the bitstream type based on the node name "fpga-core" and "fpga-periph"
Thanks for finalizing the implementation.
... } fpga-core@2 { description = "FPGA Core"; ... type = "fpga"; ... } }; configurations { default = "config@1" config@1 { fpga = "fpga-periph@1"; // periph only }; config@2 { fpga = "fpga-periph@1", "fpga-core@2"; }; };
Marek, are you OK with this implementation?
Looks OK to me. Dalon ?
[...]
From: Tien Fong Chee tien.fong.chee@intel.com
Add default fitImage file bundling FPGA bitstreams for Arria10.
Signed-off-by: Tien Fong Chee tien.fong.chee@intel.com --- board/altera/arria10-socdk/fit_spl_fpga.its | 31 +++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 board/altera/arria10-socdk/fit_spl_fpga.its
diff --git a/board/altera/arria10-socdk/fit_spl_fpga.its b/board/altera/arria10-socdk/fit_spl_fpga.its new file mode 100644 index 0000000..46b125c --- /dev/null +++ b/board/altera/arria10-socdk/fit_spl_fpga.its @@ -0,0 +1,31 @@ +// SPDX-License-Identifier: GPL-2.0 + /* + * Copyright (C) 2019 Intel Corporation <www.intel.com> + * + */ + +/dts-v1/; + +/ { + description = "FIT image with FPGA bistream"; + #address-cells = <1>; + + images { + fpga-2 { + description = "FPGA core bitstream"; + data = /incbin/("../../../ghrd_10as066n2.core.rbf"); + type = "fpga"; + arch = "arm"; + compression = "none"; + load = <0x400>; + }; + + fpga-1 { + description = "FPGA peripheral bitstream"; + data = /incbin/("../../../ghrd_10as066n2.periph.rbf"); + type = "fpga"; + arch = "arm"; + compression = "none"; + }; + }; +};
On 1/31/19 3:51 PM, tien.fong.chee@intel.com wrote:
From: Tien Fong Chee tien.fong.chee@intel.com
Add default fitImage file bundling FPGA bitstreams for Arria10.
Signed-off-by: Tien Fong Chee tien.fong.chee@intel.com
board/altera/arria10-socdk/fit_spl_fpga.its | 31 +++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 board/altera/arria10-socdk/fit_spl_fpga.its
diff --git a/board/altera/arria10-socdk/fit_spl_fpga.its b/board/altera/arria10-socdk/fit_spl_fpga.its new file mode 100644 index 0000000..46b125c --- /dev/null +++ b/board/altera/arria10-socdk/fit_spl_fpga.its @@ -0,0 +1,31 @@ +// SPDX-License-Identifier: GPL-2.0
- /*
- Copyright (C) 2019 Intel Corporation <www.intel.com>
- */
+/dts-v1/;
+/ {
- description = "FIT image with FPGA bistream";
- #address-cells = <1>;
- images {
fpga-2 {
Why is fpga-2 before fpga-1 ?
description = "FPGA core bitstream";data = /incbin/("../../../ghrd_10as066n2.core.rbf");type = "fpga";arch = "arm";compression = "none";load = <0x400>;};fpga-1 {description = "FPGA peripheral bitstream";data = /incbin/("../../../ghrd_10as066n2.periph.rbf");type = "fpga";arch = "arm";compression = "none";};- };
+};
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
Add default fitImage file bundling FPGA bitstreams for Arria10.
Signed-off-by: Tien Fong Chee tien.fong.chee@intel.com
board/altera/arria10-socdk/fit_spl_fpga.its | 31 +++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 board/altera/arria10-socdk/fit_spl_fpga.its
diff --git a/board/altera/arria10-socdk/fit_spl_fpga.its b/board/altera/arria10-socdk/fit_spl_fpga.its new file mode 100644 index 0000000..46b125c --- /dev/null +++ b/board/altera/arria10-socdk/fit_spl_fpga.its @@ -0,0 +1,31 @@ +// SPDX-License-Identifier: GPL-2.0
- /*
- Copyright (C) 2019 Intel Corporation <www.intel.com>
- */
+/dts-v1/;
+/ {
- description = "FIT image with FPGA bistream";
- #address-cells = <1>;
- images {
fpga-2 {Why is fpga-2 before fpga-1 ?
1. The main purpose is for solving the performance issue as i described in cover letter. We can decide the absolute data position for core image, and ensure it is in allignment.
2. Users know where is the data position for core, so easy for them to program themself with series commands on U-Boot console.
description = "FPGA core bitstream";data =/incbin/("../../../ghrd_10as066n2.core.rbf");
type = "fpga";arch = "arm";compression = "none";load = <0x400>;};fpga-1 {description = "FPGA peripheral bitstream";data =/incbin/("../../../ghrd_10as066n2.periph.rbf");
type = "fpga";arch = "arm";compression = "none";};- };
+};
On 2/1/19 4:59 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
Add default fitImage file bundling FPGA bitstreams for Arria10.
Signed-off-by: Tien Fong Chee tien.fong.chee@intel.com
board/altera/arria10-socdk/fit_spl_fpga.its | 31 +++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 board/altera/arria10-socdk/fit_spl_fpga.its
diff --git a/board/altera/arria10-socdk/fit_spl_fpga.its b/board/altera/arria10-socdk/fit_spl_fpga.its new file mode 100644 index 0000000..46b125c --- /dev/null +++ b/board/altera/arria10-socdk/fit_spl_fpga.its @@ -0,0 +1,31 @@ +// SPDX-License-Identifier: GPL-2.0
- /*
- Copyright (C) 2019 Intel Corporation <www.intel.com>
- */
+/dts-v1/;
+/ {
- description = "FIT image with FPGA bistream";
- #address-cells = <1>;
- images {
fpga-2 {Why is fpga-2 before fpga-1 ?
- The main purpose is for solving the performance issue as i described
in cover letter. We can decide the absolute data position for core image, and ensure it is in allignment.
Where does the alignment problem happen exactly ?
Anyway, you cannot rely on this, the alignment within the fitImage may be changed just by using different strings in the ITS file.
- Users know where is the data position for core, so easy for them to
program themself with series commands on U-Boot console.
You should use imxtract to pull out the file from fitImage and then program it. imxtract can refer to image name, so there's no need to access raw data within the fitImage by offset.
description = "FPGA core bitstream";data =/incbin/("../../../ghrd_10as066n2.core.rbf");
type = "fpga";arch = "arm";compression = "none";load = <0x400>;};fpga-1 {description = "FPGA peripheral bitstream";data =/incbin/("../../../ghrd_10as066n2.periph.rbf");
type = "fpga";arch = "arm";compression = "none";};- };
+};
On Fri, 2019-02-01 at 09:29 +0100, Marek Vasut wrote:
On 2/1/19 4:59 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
Add default fitImage file bundling FPGA bitstreams for Arria10.
Signed-off-by: Tien Fong Chee tien.fong.chee@intel.com
board/altera/arria10-socdk/fit_spl_fpga.its | 31 +++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 board/altera/arria10-socdk/fit_spl_fpga.its
diff --git a/board/altera/arria10-socdk/fit_spl_fpga.its b/board/altera/arria10-socdk/fit_spl_fpga.its new file mode 100644 index 0000000..46b125c --- /dev/null +++ b/board/altera/arria10-socdk/fit_spl_fpga.its @@ -0,0 +1,31 @@ +// SPDX-License-Identifier: GPL-2.0
- /*
- Copyright (C) 2019 Intel Corporation <www.intel.com>
- */
+/dts-v1/;
+/ {
- description = "FIT image with FPGA bistream";
- #address-cells = <1>;
- images {
fpga-2 {Why is fpga-2 before fpga-1 ?
- The main purpose is for solving the performance issue as i
described in cover letter. We can decide the absolute data position for core image, and ensure it is in allignment.
Where does the alignment problem happen exactly ?
The allignment problem happen in get_contents function, line 373, at fs/fat/fat.c . This happens only when reading offset from a file, that's why absolute position is very important to set the right offset for the core image. The performance penalty can be significantly incurred with large size core image.
filesize -= actsize; actsize -= pos; memcpy(buffer, tmp_buffer + pos, actsize); free(tmp_buffer); *gotsize += actsize; if (!filesize) return 0; buffer += actsize; <= buffer sometimes is altered to unaligned
This function is basically finding the cluster where the pos resides in, adjusting the pos, actsize and file size accordingly when the base being changed from beginning of the file to the beginning of the cluster where the pos resides in.
Then copying the actsize size of content from pos to the end of the cluster to the buffer above, and updating buffer to the next write. The updated buffer can be unaligned especially the pos is not being set properly, hence we need the absolute position to fix that.
When the unaligned buffer is passed as argument to the get_cluster function, you would see the print out of "FAT: Misaligned buffer address" at line 264 in that function. A very slow disk_read would be implemented to transfer the sector by sector content to the unaligned buffer.
Anyway, you cannot rely on this, the alignment within the fitImage may be changed just by using different strings in the ITS file.
No change for absolute position, it is always same offset based on the beginning of a FIT.
- Users know where is the data position for core, so easy for them
to program themself with series commands on U-Boot console.
You should use imxtract to pull out the file from fitImage and then program it. imxtract can refer to image name, so there's no need to access raw data within the fitImage by offset.
Yes, that's one of the most effective way. Another is using fatload with offset. But the item 1 is the main reason why absolute position is important for large core image.
Eventually positive solution, it is good to improve the performance handling for both get_content and get_cluster functions.
description = "FPGA core bitstream";data =/incbin/("../../../ghrd_10as066n2.core.rbf");
type = "fpga";arch = "arm";compression = "none";load = <0x400>;};fpga-1 {description = "FPGA peripheralbitstream";
data =/incbin/("../../../ghrd_10as066n2.periph.rbf");
type = "fpga";arch = "arm";compression = "none";};- };
+};
On 2/1/19 5:50 PM, Chee, Tien Fong wrote:
On Fri, 2019-02-01 at 09:29 +0100, Marek Vasut wrote:
On 2/1/19 4:59 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
Add default fitImage file bundling FPGA bitstreams for Arria10.
Signed-off-by: Tien Fong Chee tien.fong.chee@intel.com
board/altera/arria10-socdk/fit_spl_fpga.its | 31 +++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 board/altera/arria10-socdk/fit_spl_fpga.its
diff --git a/board/altera/arria10-socdk/fit_spl_fpga.its b/board/altera/arria10-socdk/fit_spl_fpga.its new file mode 100644 index 0000000..46b125c --- /dev/null +++ b/board/altera/arria10-socdk/fit_spl_fpga.its @@ -0,0 +1,31 @@ +// SPDX-License-Identifier: GPL-2.0
- /*
- Copyright (C) 2019 Intel Corporation <www.intel.com>
- */
+/dts-v1/;
+/ {
- description = "FIT image with FPGA bistream";
- #address-cells = <1>;
- images {
fpga-2 {Why is fpga-2 before fpga-1 ?
- The main purpose is for solving the performance issue as i
described in cover letter. We can decide the absolute data position for core image, and ensure it is in allignment.
Where does the alignment problem happen exactly ?
The allignment problem happen in get_contents function, line 373, at fs/fat/fat.c .
But then you're trying to work around a memcpy performance pentalty in VFAT code by frobbing with file position within a fitImage ? This can not work, since the file alignment within fitImage is not guaranteed.
This happens only when reading offset from a file, that's why absolute position is very important to set the right offset for the core image. The performance penalty can be significantly incurred with large size core image.
filesize -= actsize; actsize -= pos; memcpy(buffer, tmp_buffer + pos, actsize); free(tmp_buffer); *gotsize += actsize; if (!filesize) return 0; buffer += actsize; <= buffer sometimes is altered tounaligned
This function is basically finding the cluster where the pos resides in, adjusting the pos, actsize and file size accordingly when the base being changed from beginning of the file to the beginning of the cluster where the pos resides in.
Then copying the actsize size of content from pos to the end of the cluster to the buffer above, and updating buffer to the next write. The updated buffer can be unaligned especially the pos is not being set properly, hence we need the absolute position to fix that.
When the unaligned buffer is passed as argument to the get_cluster function, you would see the print out of "FAT: Misaligned buffer address" at line 264 in that function. A very slow disk_read would be implemented to transfer the sector by sector content to the unaligned buffer.
Can this be fixed then ?
Anyway, you cannot rely on this, the alignment within the fitImage may be changed just by using different strings in the ITS file.
No change for absolute position, it is always same offset based on the beginning of a FIT.
Try adding a few properties here and there and/or changing the length of some of the strings, you'll see the file offset changes.
- Users know where is the data position for core, so easy for them
to program themself with series commands on U-Boot console.
You should use imxtract to pull out the file from fitImage and then program it. imxtract can refer to image name, so there's no need to access raw data within the fitImage by offset.
Yes, that's one of the most effective way. Another is using fatload with offset.
No, it is not, because you do not know the offset. imxtract parses the fitImage structure and computes the offset for you.
But the item 1 is the main reason why absolute position is important for large core image.
Eventually positive solution, it is good to improve the performance handling for both get_content and get_cluster functions.
description = "FPGA core bitstream";data =/incbin/("../../../ghrd_10as066n2.core.rbf");
type = "fpga";arch = "arm";compression = "none";load = <0x400>;};fpga-1 {description = "FPGA peripheralbitstream";
data =/incbin/("../../../ghrd_10as066n2.periph.rbf");
type = "fpga";arch = "arm";compression = "none";};- };
+};
On Tue, 2019-02-05 at 09:51 +0100, Marek Vasut wrote:
On 2/1/19 5:50 PM, Chee, Tien Fong wrote:
On Fri, 2019-02-01 at 09:29 +0100, Marek Vasut wrote:
On 2/1/19 4:59 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
Add default fitImage file bundling FPGA bitstreams for Arria10.
Signed-off-by: Tien Fong Chee tien.fong.chee@intel.com
board/altera/arria10-socdk/fit_spl_fpga.its | 31 +++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 board/altera/arria10- socdk/fit_spl_fpga.its
diff --git a/board/altera/arria10-socdk/fit_spl_fpga.its b/board/altera/arria10-socdk/fit_spl_fpga.its new file mode 100644 index 0000000..46b125c --- /dev/null +++ b/board/altera/arria10-socdk/fit_spl_fpga.its @@ -0,0 +1,31 @@ +// SPDX-License-Identifier: GPL-2.0
- /*
- Copyright (C) 2019 Intel Corporation <www.intel.com>
- */
+/dts-v1/;
+/ {
- description = "FIT image with FPGA bistream";
- #address-cells = <1>;
- images {
fpga-2 {Why is fpga-2 before fpga-1 ?
- The main purpose is for solving the performance issue as i
described in cover letter. We can decide the absolute data position for core image, and ensure it is in allignment.
Where does the alignment problem happen exactly ?
The allignment problem happen in get_contents function, line 373, at fs/fat/fat.c .
But then you're trying to work around a memcpy performance pentalty in VFAT code by frobbing with file position within a fitImage ? This can not work, since the file alignment within fitImage is not guaranteed
Yes, setting the absolute data position for the large core rbf file in fitImage.
so, when generating the fitImage through mkimage, you need to set the absolute position as argument to -p. Absolute data position is always fixed offset based on fitImage base.
This happens only when reading offset from a file, that's why absolute position is very important to set the right offset for the core image. The performance penalty can be significantly incurred with large size core image.
filesize -= actsize; actsize -= pos; memcpy(buffer, tmp_buffer + pos, actsize); free(tmp_buffer); *gotsize += actsize; if (!filesize) return 0; buffer += actsize; <= buffer sometimes is altered tounaligned
This function is basically finding the cluster where the pos resides in, adjusting the pos, actsize and file size accordingly when the base being changed from beginning of the file to the beginning of the cluster where the pos resides in.
Then copying the actsize size of content from pos to the end of the cluster to the buffer above, and updating buffer to the next write. The updated buffer can be unaligned especially the pos is not being set properly, hence we need the absolute position to fix that.
When the unaligned buffer is passed as argument to the get_cluster function, you would see the print out of "FAT: Misaligned buffer address" at line 264 in that function. A very slow disk_read would be implemented to transfer the sector by sector content to the unaligned buffer.
Can this be fixed then ?
I have tried few ideas, no one of them work.
Could you help to take a look?
Anyway, you cannot rely on this, the alignment within the fitImage may be changed just by using different strings in the ITS file.
No change for absolute position, it is always same offset based on the beginning of a FIT.
Try adding a few properties here and there and/or changing the length of some of the strings, you'll see the file offset changes.
Absolute data position is always fixed offset from base of fitImage regardless how many properties are added or changing. But, there is potential the overlapping would be happended if data position is too close with fit.
- Users know where is the data position for core, so easy for
them to program themself with series commands on U-Boot console.
You should use imxtract to pull out the file from fitImage and then program it. imxtract can refer to image name, so there's no need to access raw data within the fitImage by offset.
Yes, that's one of the most effective way. Another is using fatload with offset.
No, it is not, because you do not know the offset. imxtract parses the fitImage structure and computes the offset for you.
You know the offset, this is absolute offset from the base of fitImage, you set it as argument to -p when running mkimage such as "-p 400"
tools/mkimage -E -p 400 -f board/altera/arria10-socdk/fit_spl_fpga.its fit_spl_fpga.itb
But the item 1 is the main reason why absolute position is important for large core image.
Eventually positive solution, it is good to improve the performance handling for both get_content and get_cluster functions.
description = "FPGA corebitstream";
data =/incbin/("../../../ghrd_10as066n2.core.rbf");
type = "fpga";arch = "arm";compression = "none";load = <0x400>;};fpga-1 {description = "FPGA peripheralbitstream";
data =/incbin/("../../../ghrd_10as066n2.periph.rbf");
type = "fpga";arch = "arm";compression = "none";};- };
+};
On 2/11/19 7:23 AM, Chee, Tien Fong wrote:
On Tue, 2019-02-05 at 09:51 +0100, Marek Vasut wrote:
On 2/1/19 5:50 PM, Chee, Tien Fong wrote:
On Fri, 2019-02-01 at 09:29 +0100, Marek Vasut wrote:
On 2/1/19 4:59 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 > > Add default fitImage file bundling FPGA bitstreams for > Arria10. > > Signed-off-by: Tien Fong Chee tien.fong.chee@intel.com > --- > board/altera/arria10-socdk/fit_spl_fpga.its | 31 > +++++++++++++++++++++++++++++ > 1 file changed, 31 insertions(+) > create mode 100644 board/altera/arria10- > socdk/fit_spl_fpga.its > > diff --git a/board/altera/arria10-socdk/fit_spl_fpga.its > b/board/altera/arria10-socdk/fit_spl_fpga.its > new file mode 100644 > index 0000000..46b125c > --- /dev/null > +++ b/board/altera/arria10-socdk/fit_spl_fpga.its > @@ -0,0 +1,31 @@ > +// SPDX-License-Identifier: GPL-2.0 > + /* > + * Copyright (C) 2019 Intel Corporation <www.intel.com> > + * > + */ > + > +/dts-v1/; > + > +/ { > + description = "FIT image with FPGA bistream"; > + #address-cells = <1>; > + > + images { > + fpga-2 { Why is fpga-2 before fpga-1 ?
- The main purpose is for solving the performance issue as i
described in cover letter. We can decide the absolute data position for core image, and ensure it is in allignment.
Where does the alignment problem happen exactly ?
The allignment problem happen in get_contents function, line 373, at fs/fat/fat.c .
But then you're trying to work around a memcpy performance pentalty in VFAT code by frobbing with file position within a fitImage ? This can not work, since the file alignment within fitImage is not guaranteed
Yes, setting the absolute data position for the large core rbf file in fitImage.
so, when generating the fitImage through mkimage, you need to set the absolute position as argument to -p. Absolute data position is always fixed offset based on fitImage base.
This can not work, consider the different filesystems the fitImage can be stored on. It's not just VFAT with one cluster size, it can be any configuration of VFAT U-Boot supports, or any filesystem U-Boot supports. And then the performance penalty could be back.
The proper fix is to optimize what VFAT does, if that is a problem. Maybe the block cache can help here ?
This happens only when reading offset from a file, that's why absolute position is very important to set the right offset for the core image. The performance penalty can be significantly incurred with large size core image.
filesize -= actsize; actsize -= pos; memcpy(buffer, tmp_buffer + pos, actsize); free(tmp_buffer); *gotsize += actsize; if (!filesize) return 0; buffer += actsize; <= buffer sometimes is altered tounaligned
This function is basically finding the cluster where the pos resides in, adjusting the pos, actsize and file size accordingly when the base being changed from beginning of the file to the beginning of the cluster where the pos resides in.
Then copying the actsize size of content from pos to the end of the cluster to the buffer above, and updating buffer to the next write. The updated buffer can be unaligned especially the pos is not being set properly, hence we need the absolute position to fix that.
When the unaligned buffer is passed as argument to the get_cluster function, you would see the print out of "FAT: Misaligned buffer address" at line 264 in that function. A very slow disk_read would be implemented to transfer the sector by sector content to the unaligned buffer.
Can this be fixed then ?
I have tried few ideas, no one of them work.
Could you help to take a look?
I am tremendously overloaded, sorry. Try taking a look at the block cache , drivers/block/blkcache.c , maybe this can help ?
Anyway, you cannot rely on this, the alignment within the fitImage may be changed just by using different strings in the ITS file.
No change for absolute position, it is always same offset based on the beginning of a FIT.
Try adding a few properties here and there and/or changing the length of some of the strings, you'll see the file offset changes.
Absolute data position is always fixed offset from base of fitImage regardless how many properties are added or changing. But, there is potential the overlapping would be happended if data position is too close with fit.
- Users know where is the data position for core, so easy for
them to program themself with series commands on U-Boot console.
You should use imxtract to pull out the file from fitImage and then program it. imxtract can refer to image name, so there's no need to access raw data within the fitImage by offset.
Yes, that's one of the most effective way. Another is using fatload with offset.
No, it is not, because you do not know the offset. imxtract parses the fitImage structure and computes the offset for you.
You know the offset, this is absolute offset from the base of fitImage, you set it as argument to -p when running mkimage such as "-p 400"
tools/mkimage -E -p 400 -f board/altera/arria10-socdk/fit_spl_fpga.its fit_spl_fpga.itb
You're just working around the alignment problem for one specific configuration of the VFAT, this does not scale.
On Mon, 2019-02-11 at 12:06 +0100, Marek Vasut wrote:
On 2/11/19 7:23 AM, Chee, Tien Fong wrote:
On Tue, 2019-02-05 at 09:51 +0100, Marek Vasut wrote:
On 2/1/19 5:50 PM, Chee, Tien Fong wrote:
On Fri, 2019-02-01 at 09:29 +0100, Marek Vasut wrote:
On 2/1/19 4:59 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 > > > > Add default fitImage file bundling FPGA bitstreams for > > Arria10. > > > > Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com > > > > > --- > > board/altera/arria10-socdk/fit_spl_fpga.its | 31 > > +++++++++++++++++++++++++++++ > > 1 file changed, 31 insertions(+) > > create mode 100644 board/altera/arria10- > > socdk/fit_spl_fpga.its > > > > diff --git a/board/altera/arria10- > > socdk/fit_spl_fpga.its > > b/board/altera/arria10-socdk/fit_spl_fpga.its > > new file mode 100644 > > index 0000000..46b125c > > --- /dev/null > > +++ b/board/altera/arria10-socdk/fit_spl_fpga.its > > @@ -0,0 +1,31 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > + /* > > + * Copyright (C) 2019 Intel Corporation <www.intel.com > > > > > + * > > + */ > > + > > +/dts-v1/; > > + > > +/ { > > + description = "FIT image with FPGA bistream"; > > + #address-cells = <1>; > > + > > + images { > > + fpga-2 { > Why is fpga-2 before fpga-1 ?
- The main purpose is for solving the performance issue as
i described in cover letter. We can decide the absolute data position for core image, and ensure it is in allignment.
Where does the alignment problem happen exactly ?
The allignment problem happen in get_contents function, line 373, at fs/fat/fat.c .
But then you're trying to work around a memcpy performance pentalty in VFAT code by frobbing with file position within a fitImage ? This can not work, since the file alignment within fitImage is not guaranteed
Yes, setting the absolute data position for the large core rbf file in fitImage.
so, when generating the fitImage through mkimage, you need to set the absolute position as argument to -p. Absolute data position is always fixed offset based on fitImage base.
This can not work, consider the different filesystems the fitImage can be stored on. It's not just VFAT with one cluster size, it can be any configuration of VFAT U-Boot supports, or any filesystem U-Boot supports. And then the performance penalty could be back.
Yes, i agree with you.
This is temporary workaround i can think of until the issue is getting solved, but i will keep trying.
If there is a solution, i will submit in separate patch set.
The proper fix is to optimize what VFAT does, if that is a problem. Maybe the block cache can help here ?
I tried that before, but it didn't help in this use case. The block cache only help in condition which repetitive reading at the same/adjacent locations.(no cache miss)
This happens only when reading offset from a file, that's why absolute position is very important to set the right offset for the core image. The performance penalty can be significantly incurred with large size core image.
filesize -= actsize; actsize -= pos; memcpy(buffer, tmp_buffer + pos, actsize); free(tmp_buffer); *gotsize += actsize; if (!filesize) return 0; buffer += actsize; <= buffer sometimes isaltered to unaligned
This function is basically finding the cluster where the pos resides in, adjusting the pos, actsize and file size accordingly when the base being changed from beginning of the file to the beginning of the cluster where the pos resides in.
Then copying the actsize size of content from pos to the end of the cluster to the buffer above, and updating buffer to the next write. The updated buffer can be unaligned especially the pos is not being set properly, hence we need the absolute position to fix that.
When the unaligned buffer is passed as argument to the get_cluster function, you would see the print out of "FAT: Misaligned buffer address" at line 264 in that function. A very slow disk_read would be implemented to transfer the sector by sector content to the unaligned buffer.
Can this be fixed then ?
I have tried few ideas, no one of them work.
Could you help to take a look?
I am tremendously overloaded, sorry. Try taking a look at the block cache , drivers/block/blkcache.c , maybe this can help ?
Replied in above.
Anyway, you cannot rely on this, the alignment within the fitImage may be changed just by using different strings in the ITS file.
No change for absolute position, it is always same offset based on the beginning of a FIT.
Try adding a few properties here and there and/or changing the length of some of the strings, you'll see the file offset changes.
Absolute data position is always fixed offset from base of fitImage regardless how many properties are added or changing. But, there is potential the overlapping would be happended if data position is too close with fit.
- Users know where is the data position for core, so easy
for them to program themself with series commands on U-Boot console.
You should use imxtract to pull out the file from fitImage and then program it. imxtract can refer to image name, so there's no need to access raw data within the fitImage by offset.
Yes, that's one of the most effective way. Another is using fatload with offset.
No, it is not, because you do not know the offset. imxtract parses the fitImage structure and computes the offset for you.
You know the offset, this is absolute offset from the base of fitImage, you set it as argument to -p when running mkimage such as "-p 400"
tools/mkimage -E -p 400 -f board/altera/arria10- socdk/fit_spl_fpga.its fit_spl_fpga.itb
You're just working around the alignment problem for one specific configuration of the VFAT, this does not scale.
Replied in above.
From: Tien Fong Chee tien.fong.chee@intel.com
Add FPGA driver to support program FPGA with FPGA bitstream loading from filesystem. The driver are designed based on generic firmware loader framework. The driver can handle FPGA program operation from loading FPGA bitstream in flash to memory and then to program FPGA.
Signed-off-by: Tien Fong Chee tien.fong.chee@intel.com
---
changes for v7 - Restructure the FPGA driver to support both peripheral bitstream and core bitstream bundled into FIT image. - Support loadable property for core bitstream. User can set loadable in DDR for better performance. This loading would be done in one large chunk instead of chunk by chunk loading with small memory buffer. --- arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts | 18 + .../include/mach/fpga_manager_arria10.h | 39 +- drivers/fpga/socfpga_arria10.c | 417 ++++++++++++++++++++- 3 files changed, 457 insertions(+), 17 deletions(-)
diff --git a/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts b/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts index 998d811..dc55618 100644 --- a/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts +++ b/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts @@ -18,6 +18,24 @@ /dts-v1/; #include "socfpga_arria10_socdk.dtsi"
+/ { + chosen { + firmware-loader = &fs_loader0; + }; + + fs_loader0: fs-loader@0 { + u-boot,dm-pre-reloc; + compatible = "u-boot,fs-loader"; + phandlepart = <&mmc 1>; + }; +}; + +&fpga_mgr { + u-boot,dm-pre-reloc; + altr,bitstream = "fit_spl_fpga.itb"; + altr,bitstream-core = "fit_spl_fpga.itb"; +}; + &mmc { u-boot,dm-pre-reloc; status = "okay"; diff --git a/arch/arm/mach-socfpga/include/mach/fpga_manager_arria10.h b/arch/arm/mach-socfpga/include/mach/fpga_manager_arria10.h index 09d13f6..683c84c 100644 --- a/arch/arm/mach-socfpga/include/mach/fpga_manager_arria10.h +++ b/arch/arm/mach-socfpga/include/mach/fpga_manager_arria10.h @@ -1,9 +1,13 @@ /* SPDX-License-Identifier: GPL-2.0 */ /* - * Copyright (C) 2017 Intel Corporation <www.intel.com> + * Copyright (C) 2017-2019 Intel Corporation <www.intel.com> * All rights reserved. */
+#include <asm/cache.h> +#include <altera.h> +#include <image.h> + #ifndef _FPGA_MANAGER_ARRIA10_H_ #define _FPGA_MANAGER_ARRIA10_H_
@@ -51,6 +55,10 @@ #define ALT_FPGAMGR_IMGCFG_CTL_02_CFGWIDTH_SET_MSK BIT(24) #define ALT_FPGAMGR_IMGCFG_CTL_02_CDRATIO_LSB 16
+#define FPGA_SOCFPGA_A10_RBF_UNENCRYPTED 0xa65c +#define FPGA_SOCFPGA_A10_RBF_ENCRYPTED 0xa65d +#define FPGA_SOCFPGA_A10_RBF_PERIPH 0x0001 +#define FPGA_SOCFPGA_A10_RBF_CORE 0x8001 #ifndef __ASSEMBLY__
struct socfpga_fpga_manager { @@ -88,12 +96,39 @@ struct socfpga_fpga_manager { u32 imgcfg_fifo_status; };
+enum rbf_type { + unknown, + periph_section, + core_section +}; + +enum rbf_security { + invalid, + unencrypted, + encrypted +}; + +struct rbf_info { + enum rbf_type section; + enum rbf_security security; +}; + +struct fpga_loadfs_info { + fpga_fs_info *fpga_fsinfo; + u32 remaining; + u32 offset; + struct rbf_info rbfinfo; +}; + /* Functions */ int fpgamgr_program_init(u32 * rbf_data, size_t rbf_size); int fpgamgr_program_finish(void); int is_fpgamgr_user_mode(void); int fpgamgr_wait_early_user_mode(void); - +int is_fpgamgr_early_user_mode(void); +char *get_fpga_filename(const void *fdt, int *len, u32 rbf_type); +int socfpga_loadfs(fpga_fs_info *fpga_fsinfo, const void *buf, size_t bsize, + u32 offset); #endif /* __ASSEMBLY__ */
#endif /* _FPGA_MANAGER_ARRIA10_H_ */ diff --git a/drivers/fpga/socfpga_arria10.c b/drivers/fpga/socfpga_arria10.c index 114dd91..019dbd3 100644 --- a/drivers/fpga/socfpga_arria10.c +++ b/drivers/fpga/socfpga_arria10.c @@ -1,6 +1,6 @@ // SPDX-License-Identifier: GPL-2.0 /* - * Copyright (C) 2017 Intel Corporation <www.intel.com> + * Copyright (C) 2017-2019 Intel Corporation <www.intel.com> */
#include <asm/io.h> @@ -10,8 +10,11 @@ #include <asm/arch/sdram.h> #include <asm/arch/misc.h> #include <altera.h> +#include <asm/arch/pinmux.h> #include <common.h> +#include <dm/ofnode.h> #include <errno.h> +#include <fs_loader.h> #include <wait_bit.h> #include <watchdog.h>
@@ -64,7 +67,7 @@ static int wait_for_user_mode(void) 1, FPGA_TIMEOUT_MSEC, false); }
-static int is_fpgamgr_early_user_mode(void) +int is_fpgamgr_early_user_mode(void) { return (readl(&fpga_manager_base->imgcfg_stat) & ALT_FPGAMGR_IMGCFG_STAT_F2S_EARLY_USERMODE_SET_MSK) != 0; @@ -447,27 +450,411 @@ int fpgamgr_program_finish(void) return 0; }
-/* - * FPGA Manager to program the FPGA. This is the interface used by FPGA driver. - * Return 0 for sucess, non-zero for error. - */ +char *get_fpga_filename(const void *fdt, int *len, u32 rbf_type) +{ + char *fpga_filename = NULL; + int node_offset; + + fdtdec_find_aliases_for_id(gd->fdt_blob, "fpga_mgr", + COMPAT_ALTERA_SOCFPGA_FPGA0, + &node_offset, 1); + + ofnode fpgamgr_node = offset_to_ofnode(node_offset); + + if (ofnode_valid(fpgamgr_node)) { + if (rbf_type == FPGA_SOCFPGA_A10_RBF_CORE) + fpga_filename = (char *)ofnode_read_string(fpgamgr_node, + "altr,bitstream-core"); + else if (rbf_type == FPGA_SOCFPGA_A10_RBF_PERIPH) + fpga_filename = (char *)ofnode_read_string(fpgamgr_node, + "altr,bitstream"); + } + + return fpga_filename; +} + +static void get_rbf_image_info(struct rbf_info *rbf, u16 *buffer) +{ + /* + * Magic ID starting at: + * -> 1st dword[15:0] in periph.rbf + * -> 2nd dword[15:0] in core.rbf + * Note: dword == 32 bits + */ + u32 word_reading_max = 2; + u32 i; + + for (i = 0; i < word_reading_max; i++) { + if (*(buffer + i) == FPGA_SOCFPGA_A10_RBF_UNENCRYPTED) { + rbf->security = unencrypted; + } else if (*(buffer + i) == FPGA_SOCFPGA_A10_RBF_ENCRYPTED) { + rbf->security = encrypted; + } else if (*(buffer + i + 1) == + FPGA_SOCFPGA_A10_RBF_UNENCRYPTED) { + rbf->security = unencrypted; + } else if (*(buffer + i + 1) == + FPGA_SOCFPGA_A10_RBF_ENCRYPTED) { + rbf->security = encrypted; + } else { + rbf->security = invalid; + continue; + } + + /* PERIPH RBF(buffer + i + 1), CORE RBF(buffer + i + 2) */ + if (*(buffer + i + 1) == FPGA_SOCFPGA_A10_RBF_PERIPH) { + rbf->section = periph_section; + break; + } else if (*(buffer + i + 1) == FPGA_SOCFPGA_A10_RBF_CORE) { + rbf->section = core_section; + break; + } else if (*(buffer + i + 2) == FPGA_SOCFPGA_A10_RBF_PERIPH) { + rbf->section = periph_section; + break; + } else if (*(buffer + i + 2) == FPGA_SOCFPGA_A10_RBF_CORE) { + rbf->section = core_section; + break; + } + + rbf->section = unknown; + break; + + WATCHDOG_RESET(); + } +} + +#ifdef CONFIG_FS_LOADER +static int first_loading_rbf_to_buffer(struct udevice *dev, + struct fpga_loadfs_info *fpga_loadfs, + u32 *buffer, size_t *buffer_bsize) +{ + u32 *buffer_p = (u32 *)*buffer; + u32 *loadable = buffer_p; + size_t buffer_size = *buffer_bsize; + size_t fit_size; + int ret = 0; + int node_offset; + int rbf_offset; + int rbf_size; + char *fpga_node_name = NULL; + + /* Load image header into buffer */ + ret = request_firmware_into_buf(dev, + fpga_loadfs->fpga_fsinfo->filename, + buffer_p, + sizeof(struct image_header), + 0); + if (ret < 0) { + debug("FPGA: Failed to read image header from flash.\n"); + return -ENOENT; + } + + WATCHDOG_RESET(); + + if (image_get_magic((struct image_header *)buffer_p) != FDT_MAGIC) { + debug("FPGA: Could not find FDT magic\n"); + return -EBADF; + } + + fit_size = fdt_totalsize(buffer_p); + + if (fit_size > buffer_size) { + debug("FPGA: FIT image is larger than available buffer.\n"); + debug("Please use FIT external data or increasing buffer.\n"); + return -ENOMEM; + } + + /* Load entire FIT into buffer */ + ret = request_firmware_into_buf(dev, + fpga_loadfs->fpga_fsinfo->filename, + buffer_p, + fit_size, + 0); + + if (ret < 0) + return ret; + + ret = fit_check_format(buffer_p); + if (!ret) { + debug("FPGA: Could not find valid FIT image\n"); + return -EBADF; + } + + if (!is_fpgamgr_early_user_mode() || is_fpgamgr_user_mode()) { + fpga_node_name = "fpga-1"; + printf("FPGA: Start to program peripheral bitstream ...\n"); + } else if (!is_fpgamgr_user_mode()) { + fpga_node_name = "fpga-2"; + printf("FPGA: Start to program core bitstream ...\n"); + } + + node_offset = fit_image_get_node(buffer_p, fpga_node_name); + if (node_offset < 0) { + debug("FPGA: Could not find node '%s' in FIT\n", + fpga_node_name); + return -ENOENT; + } + + const char *uname = fit_get_name(buffer_p, node_offset, NULL); + + if (uname) + debug("FPGA: %s\n", uname); + + ret = fit_image_get_data_position(buffer_p, node_offset, &rbf_offset); + if (ret < 0) { + debug("FPGA: Could not find data position (err=%d)\n", ret); + return -ENOENT; + } + + ret = fit_image_get_data_size(buffer_p, node_offset, &rbf_size); + if (ret < 0) { + debug("FPGA: Could not find data size (err=%d)\n", ret); + return -ENOENT; + } + + ret = fit_image_get_load(buffer_p, node_offset, (ulong *)loadable); + if (ret < 0) { + debug("FPGA: Could not find loadable (err=%d)\n", ret); + debug("FPGA: Using default buffer and size\n"); + } else { + buffer_p = (u32 *)*loadable; + buffer_size = rbf_size; + debug("FPGA: Found loadable address = 0x%x\n", *loadable); + } + + debug("FPGA: External data: offset = 0x%x, size = 0x%x\n", + rbf_offset, rbf_size); + + fpga_loadfs->remaining = rbf_size; + + /* + * Determine buffer size vs bitstream size, and calculating number of + * chunk by chunk transfer is required due to smaller buffer size + * compare to bitstream + */ + if (rbf_size <= buffer_size) { + /* Loading whole bitstream into buffer */ + buffer_size = rbf_size; + fpga_loadfs->remaining = 0; + } else { + fpga_loadfs->remaining -= buffer_size; + } + + fpga_loadfs->offset = rbf_offset; + /* Loading bitstream into buffer */ + ret = request_firmware_into_buf(dev, + fpga_loadfs->fpga_fsinfo->filename, + buffer_p, + buffer_size, + fpga_loadfs->offset); + if (ret < 0) { + debug("FPGA: Failed to read bitstream from flash.\n"); + return -ENOENT; + } + + /* Update next reading bitstream offset */ + fpga_loadfs->offset += buffer_size; + + /* Getting info about bitstream types */ + get_rbf_image_info(&fpga_loadfs->rbfinfo, (u16 *)buffer_p); + + /* Update the final addr for bitstream */ + *buffer = (u32)buffer_p; + + /* Update the size of bitstream to be programmed into FPGA */ + *buffer_bsize = buffer_size; + + return 0; +} + +static int subsequent_loading_rbf_to_buffer(struct udevice *dev, + struct fpga_loadfs_info *fpga_loadfs, + u32 *buffer, size_t *buffer_bsize) +{ + int ret = 0; + u32 *buffer_p = (u32 *)*buffer; + + /* Read the bitstream chunk by chunk. */ + if (fpga_loadfs->remaining > *buffer_bsize) { + fpga_loadfs->remaining -= *buffer_bsize; + } else { + *buffer_bsize = fpga_loadfs->remaining; + fpga_loadfs->remaining = 0; + } + + ret = request_firmware_into_buf(dev, + fpga_loadfs->fpga_fsinfo->filename, + buffer_p, + *buffer_bsize, + fpga_loadfs->offset); + if (ret < 0) { + debug("FPGA: Failed to read bitstream from flash.\n"); + return -ENOENT; + } + + /* Update next reading bitstream offset */ + fpga_loadfs->offset += *buffer_bsize; + + return 0; +} + +int socfpga_loadfs(fpga_fs_info *fpga_fsinfo, const void *buf, size_t bsize, + u32 offset) +{ + struct fpga_loadfs_info fpga_loadfs; + int status = 0; + int ret = 0; + u32 buffer = (u32)buf; + size_t buffer_sizebytes = bsize; + size_t buffer_sizebytes_ori = bsize; + size_t total_sizeof_image = 0; + struct udevice *dev; + + ret = uclass_get_device(UCLASS_FS_FIRMWARE_LOADER, 0, &dev); + if (ret) + return ret; + + memset(&fpga_loadfs, 0, sizeof(fpga_loadfs)); + + WATCHDOG_RESET(); + + fpga_loadfs.fpga_fsinfo = fpga_fsinfo; + fpga_loadfs.offset = offset; + + printf("FPGA: Start to program ...\n"); + + /* + * Note: Both buffer and buffer_sizebytes values can be altered by + * function below. + */ + ret = first_loading_rbf_to_buffer(dev, &fpga_loadfs, &buffer, + &buffer_sizebytes); + if (ret) + return ret; + + if (fpga_loadfs.rbfinfo.section == core_section && + !(is_fpgamgr_early_user_mode() && !is_fpgamgr_user_mode())) { + debug("FPGA : Must be in Early Release mode to program "); + debug("core bitstream.\n"); + return 0; + } + + /* Disable all signals from HPS peripheral controller to FPGA */ + writel(0, &system_manager_base->fpgaintf_en_global); + + /* Disable all axi bridges (hps2fpga, lwhps2fpga & fpga2hps) */ + socfpga_bridges_reset(); + + if (fpga_loadfs.rbfinfo.section == periph_section) { + /* Initialize the FPGA Manager */ + status = fpgamgr_program_init((u32 *)buffer, buffer_sizebytes); + if (status) { + debug("FPGA: Init with peripheral bitstream failed.\n"); + return -EPERM; + } + } + + WATCHDOG_RESET(); + + /* Transfer bitstream to FPGA Manager */ + fpgamgr_program_write((void *)buffer, buffer_sizebytes); + + total_sizeof_image += buffer_sizebytes; + + WATCHDOG_RESET(); + + while (fpga_loadfs.remaining) { + ret = subsequent_loading_rbf_to_buffer(dev, + &fpga_loadfs, + &buffer, + &buffer_sizebytes_ori); + + if (ret) + return ret; + + /* Transfer data to FPGA Manager */ + fpgamgr_program_write((void *)buffer, + buffer_sizebytes_ori); + + total_sizeof_image += buffer_sizebytes_ori; + + WATCHDOG_RESET(); + } + + if (fpga_loadfs.rbfinfo.section == periph_section) { + if (fpgamgr_wait_early_user_mode() != -ETIMEDOUT) { + config_pins(gd->fdt_blob, "shared"); + puts("FPGA: Early Release Succeeded.\n"); + } else { + debug("FPGA: Failed to see Early Release.\n"); + return -EIO; + } + + /* For monolithic bitstream */ + if (is_fpgamgr_user_mode()) { + /* Ensure the FPGA entering config done */ + status = fpgamgr_program_finish(); + if (status) + return status; + + config_pins(gd->fdt_blob, "fpga"); + puts("FPGA: Enter user mode.\n"); + } + } else if (fpga_loadfs.rbfinfo.section == core_section) { + /* Ensure the FPGA entering config done */ + status = fpgamgr_program_finish(); + if (status) + return status; + + config_pins(gd->fdt_blob, "fpga"); + puts("FPGA: Enter user mode.\n"); + } else { + debug("FPGA: Config Error: Unsupported bitstream type.\n"); + return -ENOEXEC; + } + + return (int)total_sizeof_image; +} +#endif + +/* This function is used to load the core bitstream from the OCRAM. */ int socfpga_load(Altera_desc *desc, const void *rbf_data, size_t rbf_size) { - int status; + unsigned long status; + struct rbf_info rbfinfo;
- /* disable all signals from hps peripheral controller to fpga */ + memset(&rbfinfo, 0, sizeof(rbfinfo)); + + /* Disable all signals from hps peripheral controller to fpga */ writel(0, &system_manager_base->fpgaintf_en_global);
- /* disable all axi bridge (hps2fpga, lwhps2fpga & fpga2hps) */ + /* Disable all axi bridge (hps2fpga, lwhps2fpga & fpga2hps) */ socfpga_bridges_reset();
- /* Initialize the FPGA Manager */ - status = fpgamgr_program_init((u32 *)rbf_data, rbf_size); - if (status) - return status; + /* Getting info about bitstream types */ + get_rbf_image_info(&rbfinfo, (u16 *)rbf_data); + + if (rbfinfo.section == periph_section) { + /* Initialize the FPGA Manager */ + status = fpgamgr_program_init((u32 *)rbf_data, rbf_size); + if (status) + return status; + } + + if (rbfinfo.section == core_section && + !(is_fpgamgr_early_user_mode() && !is_fpgamgr_user_mode())) { + debug("FPGA : Must be in early release mode to program "); + debug("core bitstream.\n"); + return 0; + }
- /* Write the RBF data to FPGA Manager */ + /* Write the bitstream to FPGA Manager */ fpgamgr_program_write(rbf_data, rbf_size);
- return fpgamgr_program_finish(); + status = fpgamgr_program_finish(); + if (status) { + config_pins(gd->fdt_blob, "fpga"); + puts("FPGA: Enter user mode.\n"); + } + + return status; }
On 1/31/19 3:51 PM, tien.fong.chee@intel.com wrote:
From: Tien Fong Chee tien.fong.chee@intel.com
Add FPGA driver to support program FPGA with FPGA bitstream loading from filesystem. The driver are designed based on generic firmware loader framework. The driver can handle FPGA program operation from loading FPGA bitstream in flash to memory and then to program FPGA.
Signed-off-by: Tien Fong Chee tien.fong.chee@intel.com
changes for v7
- Restructure the FPGA driver to support both peripheral bitstream and core bitstream bundled into FIT image.
- Support loadable property for core bitstream. User can set loadable in DDR for better performance. This loading would be done in one large chunk instead of chunk by chunk loading with small memory buffer.
arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts | 18 + .../include/mach/fpga_manager_arria10.h | 39 +- drivers/fpga/socfpga_arria10.c | 417 ++++++++++++++++++++- 3 files changed, 457 insertions(+), 17 deletions(-)
diff --git a/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts b/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts index 998d811..dc55618 100644 --- a/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts +++ b/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts @@ -18,6 +18,24 @@ /dts-v1/; #include "socfpga_arria10_socdk.dtsi"
+/ {
- chosen {
firmware-loader = &fs_loader0;
Shouldn't this be <&fs_loader0>; ? How did this even pass the DTC ?
- };
- fs_loader0: fs-loader@0 {
u-boot,dm-pre-reloc;compatible = "u-boot,fs-loader";phandlepart = <&mmc 1>;- };
+};
+&fpga_mgr {
- u-boot,dm-pre-reloc;
- altr,bitstream = "fit_spl_fpga.itb";
- altr,bitstream-core = "fit_spl_fpga.itb";
+};
&mmc { u-boot,dm-pre-reloc; status = "okay"; diff --git a/arch/arm/mach-socfpga/include/mach/fpga_manager_arria10.h b/arch/arm/mach-socfpga/include/mach/fpga_manager_arria10.h index 09d13f6..683c84c 100644 --- a/arch/arm/mach-socfpga/include/mach/fpga_manager_arria10.h +++ b/arch/arm/mach-socfpga/include/mach/fpga_manager_arria10.h @@ -1,9 +1,13 @@ /* SPDX-License-Identifier: GPL-2.0 */ /*
- Copyright (C) 2017 Intel Corporation <www.intel.com>
*/
- Copyright (C) 2017-2019 Intel Corporation <www.intel.com>
- All rights reserved.
+#include <asm/cache.h> +#include <altera.h> +#include <image.h>
#ifndef _FPGA_MANAGER_ARRIA10_H_ #define _FPGA_MANAGER_ARRIA10_H_
@@ -51,6 +55,10 @@ #define ALT_FPGAMGR_IMGCFG_CTL_02_CFGWIDTH_SET_MSK BIT(24) #define ALT_FPGAMGR_IMGCFG_CTL_02_CDRATIO_LSB 16
+#define FPGA_SOCFPGA_A10_RBF_UNENCRYPTED 0xa65c +#define FPGA_SOCFPGA_A10_RBF_ENCRYPTED 0xa65d +#define FPGA_SOCFPGA_A10_RBF_PERIPH 0x0001 +#define FPGA_SOCFPGA_A10_RBF_CORE 0x8001 #ifndef __ASSEMBLY__
struct socfpga_fpga_manager { @@ -88,12 +96,39 @@ struct socfpga_fpga_manager { u32 imgcfg_fifo_status; };
+enum rbf_type {
- unknown,
- periph_section,
- core_section
+};
[...]
On Thu, 2019-01-31 at 15:55 +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
Add FPGA driver to support program FPGA with FPGA bitstream loading from filesystem. The driver are designed based on generic firmware loader framework. The driver can handle FPGA program operation from loading FPGA bitstream in flash to memory and then to program FPGA.
Signed-off-by: Tien Fong Chee tien.fong.chee@intel.com
changes for v7
- Restructure the FPGA driver to support both peripheral bitstream
and core bitstream bundled into FIT image.
- Support loadable property for core bitstream. User can set
loadable in DDR for better performance. This loading would be done in one large chunk instead of chunk by chunk loading with small memory buffer.
arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts | 18 + .../include/mach/fpga_manager_arria10.h | 39 +- drivers/fpga/socfpga_arria10.c | 417 ++++++++++++++++++++- 3 files changed, 457 insertions(+), 17 deletions(-)
diff --git a/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts b/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts index 998d811..dc55618 100644 --- a/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts +++ b/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts @@ -18,6 +18,24 @@ /dts-v1/; #include "socfpga_arria10_socdk.dtsi" +/ {
- chosen {
firmware-loader = &fs_loader0;Shouldn't this be <&fs_loader0>; ? How did this even pass the DTC ?
So <> is compulsory required for phandle? No error complaint from DTC.
- };
- fs_loader0: fs-loader@0 {
u-boot,dm-pre-reloc;compatible = "u-boot,fs-loader";phandlepart = <&mmc 1>;- };
+};
+&fpga_mgr {
- u-boot,dm-pre-reloc;
- altr,bitstream = "fit_spl_fpga.itb";
- altr,bitstream-core = "fit_spl_fpga.itb";
+};
&mmc { u-boot,dm-pre-reloc; status = "okay"; diff --git a/arch/arm/mach- socfpga/include/mach/fpga_manager_arria10.h b/arch/arm/mach- socfpga/include/mach/fpga_manager_arria10.h index 09d13f6..683c84c 100644 --- a/arch/arm/mach-socfpga/include/mach/fpga_manager_arria10.h +++ b/arch/arm/mach-socfpga/include/mach/fpga_manager_arria10.h @@ -1,9 +1,13 @@ /* SPDX-License-Identifier: GPL-2.0 */ /*
- Copyright (C) 2017 Intel Corporation <www.intel.com>
- Copyright (C) 2017-2019 Intel Corporation <www.intel.com>
* All rights reserved. */ +#include <asm/cache.h> +#include <altera.h> +#include <image.h>
#ifndef _FPGA_MANAGER_ARRIA10_H_ #define _FPGA_MANAGER_ARRIA10_H_ @@ -51,6 +55,10 @@ #define ALT_FPGAMGR_IMGCFG_CTL_02_CFGWIDTH_SET_MSK BIT(24) #define ALT_FPGAMGR_IMGCFG_CTL_02_CDRATIO_LSB 16 +#define FPGA_SOCFPGA_A10_RBF_UNENCRYPTED 0xa65c +#define FPGA_SOCFPGA_A10_RBF_ENCRYPTED 0xa65d +#define FPGA_SOCFPGA_A10_RBF_PERIPH 0x0001 +#define FPGA_SOCFPGA_A10_RBF_CORE 0x8001 #ifndef __ASSEMBLY__ struct socfpga_fpga_manager { @@ -88,12 +96,39 @@ struct socfpga_fpga_manager { u32 imgcfg_fifo_status; }; +enum rbf_type {
- unknown,
- periph_section,
- core_section
+};
[...]
On 2/1/19 5:04 AM, Chee, Tien Fong wrote:
On Thu, 2019-01-31 at 15:55 +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
Add FPGA driver to support program FPGA with FPGA bitstream loading from filesystem. The driver are designed based on generic firmware loader framework. The driver can handle FPGA program operation from loading FPGA bitstream in flash to memory and then to program FPGA.
Signed-off-by: Tien Fong Chee tien.fong.chee@intel.com
changes for v7
- Restructure the FPGA driver to support both peripheral bitstream
and core bitstream bundled into FIT image.
- Support loadable property for core bitstream. User can set
loadable in DDR for better performance. This loading would be done in one large chunk instead of chunk by chunk loading with small memory buffer.
arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts | 18 + .../include/mach/fpga_manager_arria10.h | 39 +- drivers/fpga/socfpga_arria10.c | 417 ++++++++++++++++++++- 3 files changed, 457 insertions(+), 17 deletions(-)
diff --git a/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts b/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts index 998d811..dc55618 100644 --- a/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts +++ b/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts @@ -18,6 +18,24 @@ /dts-v1/; #include "socfpga_arria10_socdk.dtsi" +/ {
- chosen {
firmware-loader = &fs_loader0;Shouldn't this be <&fs_loader0>; ? How did this even pass the DTC ?
So <> is compulsory required for phandle? No error complaint from DTC.
Yes
On Fri, 2019-02-01 at 09:29 +0100, Marek Vasut wrote:
On 2/1/19 5:04 AM, Chee, Tien Fong wrote:
On Thu, 2019-01-31 at 15:55 +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
Add FPGA driver to support program FPGA with FPGA bitstream loading from filesystem. The driver are designed based on generic firmware loader framework. The driver can handle FPGA program operation from loading FPGA bitstream in flash to memory and then to program FPGA.
Signed-off-by: Tien Fong Chee tien.fong.chee@intel.com
changes for v7
- Restructure the FPGA driver to support both peripheral
bitstream and core bitstream bundled into FIT image.
- Support loadable property for core bitstream. User can set
loadable in DDR for better performance. This loading would be done in one large chunk instead of chunk by chunk loading with small memory buffer.
arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts | 18 + .../include/mach/fpga_manager_arria10.h | 39 +- drivers/fpga/socfpga_arria10.c | 417 ++++++++++++++++++++- 3 files changed, 457 insertions(+), 17 deletions(-)
diff --git a/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts b/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts index 998d811..dc55618 100644 --- a/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts +++ b/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts @@ -18,6 +18,24 @@ /dts-v1/; #include "socfpga_arria10_socdk.dtsi" +/ {
- chosen {
firmware-loader = &fs_loader0;Shouldn't this be <&fs_loader0>; ? How did this even pass the DTC ?
So <> is compulsory required for phandle? No error complaint from DTC.
Yes
Okay, i will fix it.
On Fri, 2019-02-01 at 09:29 +0100, Marek Vasut wrote:
On 2/1/19 5:04 AM, Chee, Tien Fong wrote:
On Thu, 2019-01-31 at 15:55 +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
Add FPGA driver to support program FPGA with FPGA bitstream loading from filesystem. The driver are designed based on generic firmware loader framework. The driver can handle FPGA program operation from loading FPGA bitstream in flash to memory and then to program FPGA.
Signed-off-by: Tien Fong Chee tien.fong.chee@intel.com
changes for v7
- Restructure the FPGA driver to support both peripheral
bitstream and core bitstream bundled into FIT image.
- Support loadable property for core bitstream. User can set
loadable in DDR for better performance. This loading would be done in one large chunk instead of chunk by chunk loading with small memory buffer.
arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts | 18 + .../include/mach/fpga_manager_arria10.h | 39 +- drivers/fpga/socfpga_arria10.c | 417 ++++++++++++++++++++- 3 files changed, 457 insertions(+), 17 deletions(-)
diff --git a/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts b/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts index 998d811..dc55618 100644 --- a/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts +++ b/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts @@ -18,6 +18,24 @@ /dts-v1/; #include "socfpga_arria10_socdk.dtsi" +/ {
- chosen {
firmware-loader = &fs_loader0;Shouldn't this be <&fs_loader0>; ? How did this even pass the DTC ?
So <> is compulsory required for phandle? No error complaint from DTC.
Yes
I just checked the codes, this &fs_loader0 without <> is valid, because this is not a phandle, instead it is a label which will be expanded to the node's full path.
On 2/13/19 9:22 AM, Chee, Tien Fong wrote:
On Fri, 2019-02-01 at 09:29 +0100, Marek Vasut wrote:
On 2/1/19 5:04 AM, Chee, Tien Fong wrote:
On Thu, 2019-01-31 at 15:55 +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
Add FPGA driver to support program FPGA with FPGA bitstream loading from filesystem. The driver are designed based on generic firmware loader framework. The driver can handle FPGA program operation from loading FPGA bitstream in flash to memory and then to program FPGA.
Signed-off-by: Tien Fong Chee tien.fong.chee@intel.com
changes for v7
- Restructure the FPGA driver to support both peripheral
bitstream and core bitstream bundled into FIT image.
- Support loadable property for core bitstream. User can set
loadable in DDR for better performance. This loading would be done in one large chunk instead of chunk by chunk loading with small memory buffer.
arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts | 18 + .../include/mach/fpga_manager_arria10.h | 39 +- drivers/fpga/socfpga_arria10.c | 417 ++++++++++++++++++++- 3 files changed, 457 insertions(+), 17 deletions(-)
diff --git a/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts b/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts index 998d811..dc55618 100644 --- a/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts +++ b/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts @@ -18,6 +18,24 @@ /dts-v1/; #include "socfpga_arria10_socdk.dtsi" +/ {
- chosen {
firmware-loader = &fs_loader0;Shouldn't this be <&fs_loader0>; ? How did this even pass the DTC ?
So <> is compulsory required for phandle? No error complaint from DTC.
Yes
I just checked the codes, this &fs_loader0 without <> is valid, because this is not a phandle, instead it is a label which will be expanded to the node's full path.
Shouldn't it be a phandle ?
On Wed, 2019-02-13 at 13:00 +0100, Marek Vasut wrote:
On 2/13/19 9:22 AM, Chee, Tien Fong wrote:
On Fri, 2019-02-01 at 09:29 +0100, Marek Vasut wrote:
On 2/1/19 5:04 AM, Chee, Tien Fong wrote:
On Thu, 2019-01-31 at 15:55 +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
Add FPGA driver to support program FPGA with FPGA bitstream loading from filesystem. The driver are designed based on generic firmware loader framework. The driver can handle FPGA program operation from loading FPGA bitstream in flash to memory and then to program FPGA.
Signed-off-by: Tien Fong Chee tien.fong.chee@intel.com
changes for v7
- Restructure the FPGA driver to support both peripheral
bitstream and core bitstream bundled into FIT image.
- Support loadable property for core bitstream. User can
set loadable in DDR for better performance. This loading would be done in one large chunk instead of chunk by chunk loading with small memory buffer.
arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts | 18 + .../include/mach/fpga_manager_arria10.h | 39 +- drivers/fpga/socfpga_arria10.c | 417 ++++++++++++++++++++- 3 files changed, 457 insertions(+), 17 deletions(-)
diff --git a/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts b/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts index 998d811..dc55618 100644 --- a/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts +++ b/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts @@ -18,6 +18,24 @@ /dts-v1/; #include "socfpga_arria10_socdk.dtsi" +/ {
- chosen {
firmware-loader = &fs_loader0;Shouldn't this be <&fs_loader0>; ? How did this even pass the DTC ?
So <> is compulsory required for phandle? No error complaint from DTC.
Yes
I just checked the codes, this &fs_loader0 without <> is valid, because this is not a phandle, instead it is a label which will be expanded to the node's full path.
Shouldn't it be a phandle ?
If my memory is correct, i choose label because there is already has API support to read the property from chosen node.
Both phandle and label i believe they serving the same purpose.
Any concern with that?
On 2/13/19 1:15 PM, Chee, Tien Fong wrote:
On Wed, 2019-02-13 at 13:00 +0100, Marek Vasut wrote:
On 2/13/19 9:22 AM, Chee, Tien Fong wrote:
On Fri, 2019-02-01 at 09:29 +0100, Marek Vasut wrote:
On 2/1/19 5:04 AM, Chee, Tien Fong wrote:
On Thu, 2019-01-31 at 15:55 +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 > > Add FPGA driver to support program FPGA with FPGA bitstream > loading > from > filesystem. The driver are designed based on generic > firmware > loader > framework. The driver can handle FPGA program operation > from > loading FPGA > bitstream in flash to memory and then to program FPGA. > > Signed-off-by: Tien Fong Chee tien.fong.chee@intel.com > > --- > > changes for v7 > - Restructure the FPGA driver to support both peripheral > bitstream > and core > bitstream bundled into FIT image. > - Support loadable property for core bitstream. User can > set > loadable > in DDR for better performance. This loading would be done > in > one > large > chunk instead of chunk by chunk loading with small memory > buffer. > --- > arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts | 18 + > .../include/mach/fpga_manager_arria10.h | 39 > +- > drivers/fpga/socfpga_arria10.c | 417 > ++++++++++++++++++++- > 3 files changed, 457 insertions(+), 17 deletions(-) > > diff --git a/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts > b/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts > index 998d811..dc55618 100644 > --- a/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts > +++ b/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts > @@ -18,6 +18,24 @@ > /dts-v1/; > #include "socfpga_arria10_socdk.dtsi" > > +/ { > + chosen { > + firmware-loader = &fs_loader0; Shouldn't this be <&fs_loader0>; ? How did this even pass the DTC ?
So <> is compulsory required for phandle? No error complaint from DTC.
Yes
I just checked the codes, this &fs_loader0 without <> is valid, because this is not a phandle, instead it is a label which will be expanded to the node's full path.
Shouldn't it be a phandle ?
If my memory is correct, i choose label because there is already has API support to read the property from chosen node.
Both phandle and label i believe they serving the same purpose.
Any concern with that?
It seems DTs today use phandles, I haven't seen labels much.
On Thu, 2019-01-31 at 22:51 +0800, tien.fong.chee@intel.com wrote:
From: Tien Fong Chee tien.fong.chee@intel.com
Add FPGA driver to support program FPGA with FPGA bitstream loading from filesystem. The driver are designed based on generic firmware loader framework. The driver can handle FPGA program operation from loading FPGA bitstream in flash to memory and then to program FPGA.
Signed-off-by: Tien Fong Chee tien.fong.chee@intel.com
changes for v7
- Restructure the FPGA driver to support both peripheral bitstream and core bitstream bundled into FIT image.
- Support loadable property for core bitstream. User can set loadable in DDR for better performance. This loading would be done in one large chunk instead of chunk by chunk loading with small memory buffer.
arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts | 18 + .../include/mach/fpga_manager_arria10.h | 39 +- drivers/fpga/socfpga_arria10.c | 417 ++++++++++++++++++++- 3 files changed, 457 insertions(+), 17 deletions(-)
diff --git a/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts b/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts index 998d811..dc55618 100644 --- a/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts +++ b/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts @@ -18,6 +18,24 @@ /dts-v1/; #include "socfpga_arria10_socdk.dtsi"
+/ {
- chosen {
firmware-loader = &fs_loader0;- };
- fs_loader0: fs-loader@0 {
u-boot,dm-pre-reloc;compatible = "u-boot,fs-loader";phandlepart = <&mmc 1>;- };
+};
+&fpga_mgr {
- u-boot,dm-pre-reloc;
- altr,bitstream = "fit_spl_fpga.itb";
- altr,bitstream-core = "fit_spl_fpga.itb";
+};
&mmc { u-boot,dm-pre-reloc; status = "okay"; diff --git a/arch/arm/mach-socfpga/include/mach/fpga_manager_arria10.h b/arch/arm/mach-socfpga/include/mach/fpga_manager_arria10.h index 09d13f6..683c84c 100644 --- a/arch/arm/mach-socfpga/include/mach/fpga_manager_arria10.h +++ b/arch/arm/mach-socfpga/include/mach/fpga_manager_arria10.h @@ -1,9 +1,13 @@ /* SPDX-License-Identifier: GPL-2.0 */ /*
- Copyright (C) 2017 Intel Corporation <www.intel.com>
*/
- Copyright (C) 2017-2019 Intel Corporation <www.intel.com>
- All rights reserved.
[...] }
- if (!is_fpgamgr_early_user_mode() || is_fpgamgr_user_mode()) {
fpga_node_name = "fpga-1";printf("FPGA: Start to program peripheral bitstream ...\n");- } else if (!is_fpgamgr_user_mode()) {
fpga_node_name = "fpga-2";printf("FPGA: Start to program core bitstream ...\n");- }
Why are you relying on the node name to define the core / periph RBF? Would it not be better to define this in a property? how would one have multiple core and periph rbfs in a single fit image?
--dalon
- node_offset = fit_image_get_node(buffer_p, fpga_node_name);
- if (node_offset < 0) {
debug("FPGA: Could not find node '%s' in FIT\n",fpga_node_name);return -ENOENT;- }
- const char *uname = fit_get_name(buffer_p, node_offset, NULL);
- if (uname)
debug("FPGA: %s\n", uname);- ret = fit_image_get_data_position(buffer_p, node_offset, &rbf_offset);
- if (ret < 0) {
debug("FPGA: Could not find data position (err=%d)\n", ret);return -ENOENT;- }
- ret = fit_image_get_data_size(buffer_p, node_offset, &rbf_size);
- if (ret < 0) {
debug("FPGA: Could not find data size (err=%d)\n", ret);return -ENOENT;- }
- ret = fit_image_get_load(buffer_p, node_offset, (ulong *)loadable);
- if (ret < 0) {
debug("FPGA: Could not find loadable (err=%d)\n", ret);debug("FPGA: Using default buffer and size\n");- } else {
buffer_p = (u32 *)*loadable;buffer_size = rbf_size;debug("FPGA: Found loadable address = 0x%x\n", *loadable);- }
- debug("FPGA: External data: offset = 0x%x, size = 0x%x\n",
rbf_offset, rbf_size);- fpga_loadfs->remaining = rbf_size;
- /*
* Determine buffer size vs bitstream size, and calculating number of* chunk by chunk transfer is required due to smaller buffer size* compare to bitstream*/- if (rbf_size <= buffer_size) {
/* Loading whole bitstream into buffer */buffer_size = rbf_size;fpga_loadfs->remaining = 0;- } else {
fpga_loadfs->remaining -= buffer_size;- }
- fpga_loadfs->offset = rbf_offset;
- /* Loading bitstream into buffer */
- ret = request_firmware_into_buf(dev,
fpga_loadfs->fpga_fsinfo->filename,buffer_p,buffer_size,fpga_loadfs->offset);- if (ret < 0) {
debug("FPGA: Failed to read bitstream from flash.\n");return -ENOENT;- }
- /* Update next reading bitstream offset */
- fpga_loadfs->offset += buffer_size;
- /* Getting info about bitstream types */
- get_rbf_image_info(&fpga_loadfs->rbfinfo, (u16 *)buffer_p);
- /* Update the final addr for bitstream */
- *buffer = (u32)buffer_p;
- /* Update the size of bitstream to be programmed into FPGA */
- *buffer_bsize = buffer_size;
- return 0;
+}
+static int subsequent_loading_rbf_to_buffer(struct udevice *dev,
struct fpga_loadfs_info *fpga_loadfs,u32 *buffer, size_t *buffer_bsize)+{
- int ret = 0;
- u32 *buffer_p = (u32 *)*buffer;
- /* Read the bitstream chunk by chunk. */
- if (fpga_loadfs->remaining > *buffer_bsize) {
fpga_loadfs->remaining -= *buffer_bsize;- } else {
*buffer_bsize = fpga_loadfs->remaining;fpga_loadfs->remaining = 0;- }
- ret = request_firmware_into_buf(dev,
fpga_loadfs->fpga_fsinfo->filename,buffer_p,*buffer_bsize,fpga_loadfs->offset);- if (ret < 0) {
debug("FPGA: Failed to read bitstream from flash.\n");return -ENOENT;- }
- /* Update next reading bitstream offset */
- fpga_loadfs->offset += *buffer_bsize;
- return 0;
+}
+int socfpga_loadfs(fpga_fs_info *fpga_fsinfo, const void *buf, size_t bsize,
u32 offset)+{
- struct fpga_loadfs_info fpga_loadfs;
- int status = 0;
- int ret = 0;
- u32 buffer = (u32)buf;
- size_t buffer_sizebytes = bsize;
- size_t buffer_sizebytes_ori = bsize;
- size_t total_sizeof_image = 0;
- struct udevice *dev;
- ret = uclass_get_device(UCLASS_FS_FIRMWARE_LOADER, 0, &dev);
- if (ret)
return ret;- memset(&fpga_loadfs, 0, sizeof(fpga_loadfs));
- WATCHDOG_RESET();
- fpga_loadfs.fpga_fsinfo = fpga_fsinfo;
- fpga_loadfs.offset = offset;
- printf("FPGA: Start to program ...\n");
- /*
* Note: Both buffer and buffer_sizebytes values can be altered by* function below.*/- ret = first_loading_rbf_to_buffer(dev, &fpga_loadfs, &buffer,
&buffer_sizebytes);- if (ret)
return ret;- if (fpga_loadfs.rbfinfo.section == core_section &&
!(is_fpgamgr_early_user_mode() && !is_fpgamgr_user_mode())) {debug("FPGA : Must be in Early Release mode to program ");debug("core bitstream.\n");return 0;- }
- /* Disable all signals from HPS peripheral controller to FPGA */
- writel(0, &system_manager_base->fpgaintf_en_global);
- /* Disable all axi bridges (hps2fpga, lwhps2fpga & fpga2hps) */
- socfpga_bridges_reset();
- if (fpga_loadfs.rbfinfo.section == periph_section) {
/* Initialize the FPGA Manager */status = fpgamgr_program_init((u32 *)buffer, buffer_sizebytes);if (status) {debug("FPGA: Init with peripheral bitstream failed.\n");return -EPERM;}- }
- WATCHDOG_RESET();
- /* Transfer bitstream to FPGA Manager */
- fpgamgr_program_write((void *)buffer, buffer_sizebytes);
- total_sizeof_image += buffer_sizebytes;
- WATCHDOG_RESET();
- while (fpga_loadfs.remaining) {
ret = subsequent_loading_rbf_to_buffer(dev,&fpga_loadfs,&buffer,&buffer_sizebytes_ori);if (ret)return ret;/* Transfer data to FPGA Manager */fpgamgr_program_write((void *)buffer,buffer_sizebytes_ori);total_sizeof_image += buffer_sizebytes_ori;WATCHDOG_RESET();- }
- if (fpga_loadfs.rbfinfo.section == periph_section) {
if (fpgamgr_wait_early_user_mode() != -ETIMEDOUT) {config_pins(gd->fdt_blob, "shared");puts("FPGA: Early Release Succeeded.\n");} else {debug("FPGA: Failed to see Early Release.\n");return -EIO;}/* For monolithic bitstream */if (is_fpgamgr_user_mode()) {/* Ensure the FPGA entering config done */status = fpgamgr_program_finish();if (status)return status;config_pins(gd->fdt_blob, "fpga");puts("FPGA: Enter user mode.\n");}- } else if (fpga_loadfs.rbfinfo.section == core_section) {
/* Ensure the FPGA entering config done */status = fpgamgr_program_finish();if (status)return status;config_pins(gd->fdt_blob, "fpga");puts("FPGA: Enter user mode.\n");- } else {
debug("FPGA: Config Error: Unsupported bitstream type.\n");return -ENOEXEC;- }
- return (int)total_sizeof_image;
+} +#endif
+/* This function is used to load the core bitstream from the OCRAM. */ int socfpga_load(Altera_desc *desc, const void *rbf_data, size_t rbf_size) {
- int status;
- unsigned long status;
- struct rbf_info rbfinfo;
- /* disable all signals from hps peripheral controller to fpga */
- memset(&rbfinfo, 0, sizeof(rbfinfo));
- /* Disable all signals from hps peripheral controller to fpga */ writel(0, &system_manager_base->fpgaintf_en_global);
- /* disable all axi bridge (hps2fpga, lwhps2fpga & fpga2hps) */
- /* Disable all axi bridge (hps2fpga, lwhps2fpga & fpga2hps) */ socfpga_bridges_reset();
- /* Initialize the FPGA Manager */
- status = fpgamgr_program_init((u32 *)rbf_data, rbf_size);
- if (status)
return status;
- /* Getting info about bitstream types */
- get_rbf_image_info(&rbfinfo, (u16 *)rbf_data);
- if (rbfinfo.section == periph_section) {
/* Initialize the FPGA Manager */status = fpgamgr_program_init((u32 *)rbf_data, rbf_size);if (status)return status;- }
- if (rbfinfo.section == core_section &&
!(is_fpgamgr_early_user_mode() && !is_fpgamgr_user_mode())) {debug("FPGA : Must be in early release mode to program ");debug("core bitstream.\n");return 0;- }
- /* Write the RBF data to FPGA Manager */
- /* Write the bitstream to FPGA Manager */ fpgamgr_program_write(rbf_data, rbf_size);
- return fpgamgr_program_finish();
- status = fpgamgr_program_finish();
- if (status) {
config_pins(gd->fdt_blob, "fpga");puts("FPGA: Enter user mode.\n");- }
- return status;
}
On Fri, 2019-02-01 at 12:12 -0800, Dalon L Westergreen wrote:
On Thu, 2019-01-31 at 22:51 +0800, tien.fong.chee@intel.com wrote:
From: Tien Fong Chee tien.fong.chee@intel.com
Add FPGA driver to support program FPGA with FPGA bitstream loading from filesystem. The driver are designed based on generic firmware loader framework. The driver can handle FPGA program operation from loading FPGA bitstream in flash to memory and then to program FPGA.
Signed-off-by: Tien Fong Chee tien.fong.chee@intel.com
changes for v7
- Restructure the FPGA driver to support both peripheral bitstream
and core bitstream bundled into FIT image.
- Support loadable property for core bitstream. User can set
loadable in DDR for better performance. This loading would be done in one large chunk instead of chunk by chunk loading with small memory buffer.
arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts | 18 + .../include/mach/fpga_manager_arria10.h | 39 +- drivers/fpga/socfpga_arria10.c | 417 ++++++++++++++++++++- 3 files changed, 457 insertions(+), 17 deletions(-)
diff --git a/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts b/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts index 998d811..dc55618 100644 --- a/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts +++ b/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts @@ -18,6 +18,24 @@ /dts-v1/; #include "socfpga_arria10_socdk.dtsi" +/ {
- chosen {
firmware-loader = &fs_loader0;- };
- fs_loader0: fs-loader@0 {
u-boot,dm-pre-reloc;compatible = "u-boot,fs-loader";phandlepart = <&mmc 1>;- };
+};
+&fpga_mgr {
- u-boot,dm-pre-reloc;
- altr,bitstream = "fit_spl_fpga.itb";
- altr,bitstream-core = "fit_spl_fpga.itb";
+};
&mmc { u-boot,dm-pre-reloc; status = "okay"; diff --git a/arch/arm/mach- socfpga/include/mach/fpga_manager_arria10.h b/arch/arm/mach-socfpga/include/mach/fpga_manager_arria10.h index 09d13f6..683c84c 100644 --- a/arch/arm/mach-socfpga/include/mach/fpga_manager_arria10.h +++ b/arch/arm/mach-socfpga/include/mach/fpga_manager_arria10.h @@ -1,9 +1,13 @@ /* SPDX-License-Identifier: GPL-2.0 */ /*
- Copyright (C) 2017 Intel Corporation <www.intel.com>
- Copyright (C) 2017-2019 Intel Corporation <www.intel.com>
* All rights reserved. */
[...] }
- if (!is_fpgamgr_early_user_mode() ||
is_fpgamgr_user_mode()) {
fpga_node_name = "fpga-1";printf("FPGA: Start to program peripheralbitstream ...\n");
- } else if (!is_fpgamgr_user_mode()) {
fpga_node_name = "fpga-2";printf("FPGA: Start to program core bitstream...\n");
- }
Why are you relying on the node name to define the core / periph RBF? Would it not be better to define this in a property?
I need to ensure correct sequence of programming the rbf images at different FPGA mode. fpga with index 1 always means the 1st to program.
how would one have multiple core and periph rbfs in a single fit image?
I want strict control to avoid the performance penalty, when this happen, performance is almost same as programming full image.
So, in a single FIT image, only have one core rbf and periph rbf, and core rbf must always be the 1st and with the right absolute data position to avoid the buffer unaligned. Strict sequence control is also one of the reason having one core rbf and one periph rbf in a single FIT image.
You can create multiple FIT image for multiple core and periph rbfs. There are properties such as "altr,bitstream" and "altr,bitstream-core" for supporting different FIT image.
You can change those properties in U-Boot env(for runtime), or using different DTS for differnt configuration.
--dalon
- node_offset = fit_image_get_node(buffer_p,
fpga_node_name);
- if (node_offset < 0) {
debug("FPGA: Could not find node '%s' in FIT\n",fpga_node_name);return -ENOENT;- }
- const char *uname = fit_get_name(buffer_p, node_offset,
NULL);
- if (uname)
debug("FPGA: %s\n", uname);- ret = fit_image_get_data_position(buffer_p, node_offset,
&rbf_offset);
- if (ret < 0) {
debug("FPGA: Could not find data position(err=%d)\n", ret);
return -ENOENT;- }
- ret = fit_image_get_data_size(buffer_p, node_offset,
&rbf_size);
- if (ret < 0) {
debug("FPGA: Could not find data size (err=%d)\n",ret);
return -ENOENT;- }
- ret = fit_image_get_load(buffer_p, node_offset, (ulong
*)loadable);
- if (ret < 0) {
debug("FPGA: Could not find loadable (err=%d)\n",ret);
debug("FPGA: Using default buffer and size\n");- } else {
buffer_p = (u32 *)*loadable;buffer_size = rbf_size;debug("FPGA: Found loadable address = 0x%x\n",*loadable);
- }
- debug("FPGA: External data: offset = 0x%x, size = 0x%x\n",
- rbf_offset, rbf_size);
- fpga_loadfs->remaining = rbf_size;
- /*
- * Determine buffer size vs bitstream size, and
calculating number of
- * chunk by chunk transfer is required due to smaller
buffer size
- * compare to bitstream
- */
- if (rbf_size <= buffer_size) {
/* Loading whole bitstream into buffer */buffer_size = rbf_size;fpga_loadfs->remaining = 0;- } else {
fpga_loadfs->remaining -= buffer_size;- }
- fpga_loadfs->offset = rbf_offset;
- /* Loading bitstream into buffer */
- ret = request_firmware_into_buf(dev,
fpga_loadfs->fpga_fsinfo-filename,
buffer_p,buffer_size,fpga_loadfs->offset);- if (ret < 0) {
debug("FPGA: Failed to read bitstream fromflash.\n");
return -ENOENT;- }
- /* Update next reading bitstream offset */
- fpga_loadfs->offset += buffer_size;
- /* Getting info about bitstream types */
- get_rbf_image_info(&fpga_loadfs->rbfinfo, (u16
*)buffer_p);
- /* Update the final addr for bitstream */
- *buffer = (u32)buffer_p;
- /* Update the size of bitstream to be programmed into FPGA
*/
- *buffer_bsize = buffer_size;
- return 0;
+}
+static int subsequent_loading_rbf_to_buffer(struct udevice *dev,
struct fpga_loadfs_info*fpga_loadfs,
u32 *buffer, size_t*buffer_bsize) +{
- int ret = 0;
- u32 *buffer_p = (u32 *)*buffer;
- /* Read the bitstream chunk by chunk. */
- if (fpga_loadfs->remaining > *buffer_bsize) {
fpga_loadfs->remaining -= *buffer_bsize;- } else {
*buffer_bsize = fpga_loadfs->remaining;fpga_loadfs->remaining = 0;- }
- ret = request_firmware_into_buf(dev,
fpga_loadfs->fpga_fsinfo-filename,
buffer_p,*buffer_bsize,fpga_loadfs->offset);- if (ret < 0) {
debug("FPGA: Failed to read bitstream fromflash.\n");
return -ENOENT;- }
- /* Update next reading bitstream offset */
- fpga_loadfs->offset += *buffer_bsize;
- return 0;
+}
+int socfpga_loadfs(fpga_fs_info *fpga_fsinfo, const void *buf, size_t bsize,
u32 offset)+{
- struct fpga_loadfs_info fpga_loadfs;
- int status = 0;
- int ret = 0;
- u32 buffer = (u32)buf;
- size_t buffer_sizebytes = bsize;
- size_t buffer_sizebytes_ori = bsize;
- size_t total_sizeof_image = 0;
- struct udevice *dev;
- ret = uclass_get_device(UCLASS_FS_FIRMWARE_LOADER, 0,
&dev);
- if (ret)
return ret;- memset(&fpga_loadfs, 0, sizeof(fpga_loadfs));
- WATCHDOG_RESET();
- fpga_loadfs.fpga_fsinfo = fpga_fsinfo;
- fpga_loadfs.offset = offset;
- printf("FPGA: Start to program ...\n");
- /*
- * Note: Both buffer and buffer_sizebytes values can be
altered by
- * function below.
- */
- ret = first_loading_rbf_to_buffer(dev, &fpga_loadfs,
&buffer,
&buffer_sizebytes);- if (ret)
return ret;- if (fpga_loadfs.rbfinfo.section == core_section &&
!(is_fpgamgr_early_user_mode() &&!is_fpgamgr_user_mode())) {
debug("FPGA : Must be in Early Release mode toprogram ");
debug("core bitstream.\n");return 0;- }
- /* Disable all signals from HPS peripheral controller to
FPGA */
- writel(0, &system_manager_base->fpgaintf_en_global);
- /* Disable all axi bridges (hps2fpga, lwhps2fpga &
fpga2hps) */
- socfpga_bridges_reset();
- if (fpga_loadfs.rbfinfo.section == periph_section) {
/* Initialize the FPGA Manager */status = fpgamgr_program_init((u32 *)buffer,buffer_sizebytes);
if (status) {debug("FPGA: Init with peripheralbitstream failed.\n");
return -EPERM;}- }
- WATCHDOG_RESET();
- /* Transfer bitstream to FPGA Manager */
- fpgamgr_program_write((void *)buffer, buffer_sizebytes);
- total_sizeof_image += buffer_sizebytes;
- WATCHDOG_RESET();
- while (fpga_loadfs.remaining) {
ret = subsequent_loading_rbf_to_buffer(dev,&fpga_loadfs,
&buffer,&buffer_sizebytes_ori);
if (ret)return ret;/* Transfer data to FPGA Manager */fpgamgr_program_write((void *)buffer,buffer_sizebytes_ori);total_sizeof_image += buffer_sizebytes_ori;WATCHDOG_RESET();- }
- if (fpga_loadfs.rbfinfo.section == periph_section) {
if (fpgamgr_wait_early_user_mode() != -ETIMEDOUT){
config_pins(gd->fdt_blob, "shared");puts("FPGA: Early Release Succeeded.\n");} else {debug("FPGA: Failed to see EarlyRelease.\n");
return -EIO;}/* For monolithic bitstream */if (is_fpgamgr_user_mode()) {/* Ensure the FPGA entering config done */status = fpgamgr_program_finish();if (status)return status;config_pins(gd->fdt_blob, "fpga");puts("FPGA: Enter user mode.\n");}- } else if (fpga_loadfs.rbfinfo.section == core_section) {
/* Ensure the FPGA entering config done */status = fpgamgr_program_finish();if (status)return status;config_pins(gd->fdt_blob, "fpga");puts("FPGA: Enter user mode.\n");- } else {
debug("FPGA: Config Error: Unsupported bitstreamtype.\n");
return -ENOEXEC;- }
- return (int)total_sizeof_image;
+} +#endif
+/* This function is used to load the core bitstream from the OCRAM. */ int socfpga_load(Altera_desc *desc, const void *rbf_data, size_t rbf_size) {
- int status;
- unsigned long status;
- struct rbf_info rbfinfo;
- /* disable all signals from hps peripheral controller to
fpga */
- memset(&rbfinfo, 0, sizeof(rbfinfo));
- /* Disable all signals from hps peripheral controller to
fpga */ writel(0, &system_manager_base->fpgaintf_en_global);
- /* disable all axi bridge (hps2fpga, lwhps2fpga &
fpga2hps) */
- /* Disable all axi bridge (hps2fpga, lwhps2fpga &
fpga2hps) */ socfpga_bridges_reset();
- /* Initialize the FPGA Manager */
- status = fpgamgr_program_init((u32 *)rbf_data, rbf_size);
- if (status)
return status;
- /* Getting info about bitstream types */
- get_rbf_image_info(&rbfinfo, (u16 *)rbf_data);
- if (rbfinfo.section == periph_section) {
/* Initialize the FPGA Manager */status = fpgamgr_program_init((u32 *)rbf_data,rbf_size);
if (status)return status;- }
- if (rbfinfo.section == core_section &&
!(is_fpgamgr_early_user_mode() &&!is_fpgamgr_user_mode())) {
debug("FPGA : Must be in early release mode toprogram ");
debug("core bitstream.\n");return 0;- }
- /* Write the RBF data to FPGA Manager */
- /* Write the bitstream to FPGA Manager */
fpgamgr_program_write(rbf_data, rbf_size);
- return fpgamgr_program_finish();
- status = fpgamgr_program_finish();
- if (status) {
config_pins(gd->fdt_blob, "fpga");puts("FPGA: Enter user mode.\n");- }
- return status;
}
On 2/2/19 4:27 AM, Chee, Tien Fong wrote:
On Fri, 2019-02-01 at 12:12 -0800, Dalon L Westergreen wrote:
On Thu, 2019-01-31 at 22:51 +0800, tien.fong.chee@intel.com wrote:
From: Tien Fong Chee tien.fong.chee@intel.com
Add FPGA driver to support program FPGA with FPGA bitstream loading from filesystem. The driver are designed based on generic firmware loader framework. The driver can handle FPGA program operation from loading FPGA bitstream in flash to memory and then to program FPGA.
Signed-off-by: Tien Fong Chee tien.fong.chee@intel.com
changes for v7
- Restructure the FPGA driver to support both peripheral bitstream
and core bitstream bundled into FIT image.
- Support loadable property for core bitstream. User can set
loadable in DDR for better performance. This loading would be done in one large chunk instead of chunk by chunk loading with small memory buffer.
arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts | 18 + .../include/mach/fpga_manager_arria10.h | 39 +- drivers/fpga/socfpga_arria10.c | 417 ++++++++++++++++++++- 3 files changed, 457 insertions(+), 17 deletions(-)
diff --git a/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts b/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts index 998d811..dc55618 100644 --- a/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts +++ b/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts @@ -18,6 +18,24 @@ /dts-v1/; #include "socfpga_arria10_socdk.dtsi" +/ {
- chosen {
firmware-loader = &fs_loader0;- };
- fs_loader0: fs-loader@0 {
u-boot,dm-pre-reloc;compatible = "u-boot,fs-loader";phandlepart = <&mmc 1>;- };
+};
+&fpga_mgr {
- u-boot,dm-pre-reloc;
- altr,bitstream = "fit_spl_fpga.itb";
- altr,bitstream-core = "fit_spl_fpga.itb";
+};
&mmc { u-boot,dm-pre-reloc; status = "okay"; diff --git a/arch/arm/mach- socfpga/include/mach/fpga_manager_arria10.h b/arch/arm/mach-socfpga/include/mach/fpga_manager_arria10.h index 09d13f6..683c84c 100644 --- a/arch/arm/mach-socfpga/include/mach/fpga_manager_arria10.h +++ b/arch/arm/mach-socfpga/include/mach/fpga_manager_arria10.h @@ -1,9 +1,13 @@ /* SPDX-License-Identifier: GPL-2.0 */ /*
- Copyright (C) 2017 Intel Corporation <www.intel.com>
- Copyright (C) 2017-2019 Intel Corporation <www.intel.com>
* All rights reserved. */
[...] }
- if (!is_fpgamgr_early_user_mode() ||
is_fpgamgr_user_mode()) {
fpga_node_name = "fpga-1";printf("FPGA: Start to program peripheralbitstream ...\n");
- } else if (!is_fpgamgr_user_mode()) {
fpga_node_name = "fpga-2";printf("FPGA: Start to program core bitstream...\n");
- }
Why are you relying on the node name to define the core / periph RBF? Would it not be better to define this in a property?
I need to ensure correct sequence of programming the rbf images at different FPGA mode. fpga with index 1 always means the 1st to program.
how would one have multiple core and periph rbfs in a single fit image?
I want strict control to avoid the performance penalty, when this happen, performance is almost same as programming full image.
So, in a single FIT image, only have one core rbf and periph rbf, and core rbf must always be the 1st and with the right absolute data position to avoid the buffer unaligned. Strict sequence control is also one of the reason having one core rbf and one periph rbf in a single FIT image.
I thought we established that you cannot depend on the placement and alignment of files within fitImage ?
Also, you should be able to use fitImage configurations to select from multiple core and periph RBFs, there's no point in limiting ourselves to 1 core and 1 periph RBF with flexible format such as the fitImage.
You can create multiple FIT image for multiple core and periph rbfs. There are properties such as "altr,bitstream" and "altr,bitstream-core" for supporting different FIT image.
You can change those properties in U-Boot env(for runtime), or using different DTS for differnt configuration.
--dalon
- node_offset = fit_image_get_node(buffer_p,
fpga_node_name);
- if (node_offset < 0) {
debug("FPGA: Could not find node '%s' in FIT\n",fpga_node_name);return -ENOENT;- }
- const char *uname = fit_get_name(buffer_p, node_offset,
NULL);
- if (uname)
debug("FPGA: %s\n", uname);- ret = fit_image_get_data_position(buffer_p, node_offset,
&rbf_offset);
- if (ret < 0) {
debug("FPGA: Could not find data position(err=%d)\n", ret);
return -ENOENT;- }
- ret = fit_image_get_data_size(buffer_p, node_offset,
&rbf_size);
- if (ret < 0) {
debug("FPGA: Could not find data size (err=%d)\n",ret);
return -ENOENT;- }
- ret = fit_image_get_load(buffer_p, node_offset, (ulong
*)loadable);
- if (ret < 0) {
debug("FPGA: Could not find loadable (err=%d)\n",ret);
debug("FPGA: Using default buffer and size\n");- } else {
buffer_p = (u32 *)*loadable;buffer_size = rbf_size;debug("FPGA: Found loadable address = 0x%x\n",*loadable);
- }
- debug("FPGA: External data: offset = 0x%x, size = 0x%x\n",
- rbf_offset, rbf_size);
- fpga_loadfs->remaining = rbf_size;
- /*
- * Determine buffer size vs bitstream size, and
calculating number of
- * chunk by chunk transfer is required due to smaller
buffer size
- * compare to bitstream
- */
- if (rbf_size <= buffer_size) {
/* Loading whole bitstream into buffer */buffer_size = rbf_size;fpga_loadfs->remaining = 0;- } else {
fpga_loadfs->remaining -= buffer_size;- }
- fpga_loadfs->offset = rbf_offset;
- /* Loading bitstream into buffer */
- ret = request_firmware_into_buf(dev,
fpga_loadfs->fpga_fsinfo-filename,
buffer_p,buffer_size,fpga_loadfs->offset);- if (ret < 0) {
debug("FPGA: Failed to read bitstream fromflash.\n");
return -ENOENT;- }
- /* Update next reading bitstream offset */
- fpga_loadfs->offset += buffer_size;
- /* Getting info about bitstream types */
- get_rbf_image_info(&fpga_loadfs->rbfinfo, (u16
*)buffer_p);
- /* Update the final addr for bitstream */
- *buffer = (u32)buffer_p;
- /* Update the size of bitstream to be programmed into FPGA
*/
- *buffer_bsize = buffer_size;
- return 0;
+}
+static int subsequent_loading_rbf_to_buffer(struct udevice *dev,
struct fpga_loadfs_info*fpga_loadfs,
u32 *buffer, size_t*buffer_bsize) +{
- int ret = 0;
- u32 *buffer_p = (u32 *)*buffer;
- /* Read the bitstream chunk by chunk. */
- if (fpga_loadfs->remaining > *buffer_bsize) {
fpga_loadfs->remaining -= *buffer_bsize;- } else {
*buffer_bsize = fpga_loadfs->remaining;fpga_loadfs->remaining = 0;- }
- ret = request_firmware_into_buf(dev,
fpga_loadfs->fpga_fsinfo-filename,
buffer_p,*buffer_bsize,fpga_loadfs->offset);- if (ret < 0) {
debug("FPGA: Failed to read bitstream fromflash.\n");
return -ENOENT;- }
- /* Update next reading bitstream offset */
- fpga_loadfs->offset += *buffer_bsize;
- return 0;
+}
+int socfpga_loadfs(fpga_fs_info *fpga_fsinfo, const void *buf, size_t bsize,
u32 offset)+{
- struct fpga_loadfs_info fpga_loadfs;
- int status = 0;
- int ret = 0;
- u32 buffer = (u32)buf;
- size_t buffer_sizebytes = bsize;
- size_t buffer_sizebytes_ori = bsize;
- size_t total_sizeof_image = 0;
- struct udevice *dev;
- ret = uclass_get_device(UCLASS_FS_FIRMWARE_LOADER, 0,
&dev);
- if (ret)
return ret;- memset(&fpga_loadfs, 0, sizeof(fpga_loadfs));
- WATCHDOG_RESET();
- fpga_loadfs.fpga_fsinfo = fpga_fsinfo;
- fpga_loadfs.offset = offset;
- printf("FPGA: Start to program ...\n");
- /*
- * Note: Both buffer and buffer_sizebytes values can be
altered by
- * function below.
- */
- ret = first_loading_rbf_to_buffer(dev, &fpga_loadfs,
&buffer,
&buffer_sizebytes);- if (ret)
return ret;- if (fpga_loadfs.rbfinfo.section == core_section &&
!(is_fpgamgr_early_user_mode() &&!is_fpgamgr_user_mode())) {
debug("FPGA : Must be in Early Release mode toprogram ");
debug("core bitstream.\n");return 0;- }
- /* Disable all signals from HPS peripheral controller to
FPGA */
- writel(0, &system_manager_base->fpgaintf_en_global);
- /* Disable all axi bridges (hps2fpga, lwhps2fpga &
fpga2hps) */
- socfpga_bridges_reset();
- if (fpga_loadfs.rbfinfo.section == periph_section) {
/* Initialize the FPGA Manager */status = fpgamgr_program_init((u32 *)buffer,buffer_sizebytes);
if (status) {debug("FPGA: Init with peripheralbitstream failed.\n");
return -EPERM;}- }
- WATCHDOG_RESET();
- /* Transfer bitstream to FPGA Manager */
- fpgamgr_program_write((void *)buffer, buffer_sizebytes);
- total_sizeof_image += buffer_sizebytes;
- WATCHDOG_RESET();
- while (fpga_loadfs.remaining) {
ret = subsequent_loading_rbf_to_buffer(dev,&fpga_loadfs,
&buffer,&buffer_sizebytes_ori);
if (ret)return ret;/* Transfer data to FPGA Manager */fpgamgr_program_write((void *)buffer,buffer_sizebytes_ori);total_sizeof_image += buffer_sizebytes_ori;WATCHDOG_RESET();- }
- if (fpga_loadfs.rbfinfo.section == periph_section) {
if (fpgamgr_wait_early_user_mode() != -ETIMEDOUT){
config_pins(gd->fdt_blob, "shared");puts("FPGA: Early Release Succeeded.\n");} else {debug("FPGA: Failed to see EarlyRelease.\n");
return -EIO;}/* For monolithic bitstream */if (is_fpgamgr_user_mode()) {/* Ensure the FPGA entering config done */status = fpgamgr_program_finish();if (status)return status;config_pins(gd->fdt_blob, "fpga");puts("FPGA: Enter user mode.\n");}- } else if (fpga_loadfs.rbfinfo.section == core_section) {
/* Ensure the FPGA entering config done */status = fpgamgr_program_finish();if (status)return status;config_pins(gd->fdt_blob, "fpga");puts("FPGA: Enter user mode.\n");- } else {
debug("FPGA: Config Error: Unsupported bitstreamtype.\n");
return -ENOEXEC;- }
- return (int)total_sizeof_image;
+} +#endif
+/* This function is used to load the core bitstream from the OCRAM. */ int socfpga_load(Altera_desc *desc, const void *rbf_data, size_t rbf_size) {
- int status;
- unsigned long status;
- struct rbf_info rbfinfo;
- /* disable all signals from hps peripheral controller to
fpga */
- memset(&rbfinfo, 0, sizeof(rbfinfo));
- /* Disable all signals from hps peripheral controller to
fpga */ writel(0, &system_manager_base->fpgaintf_en_global);
- /* disable all axi bridge (hps2fpga, lwhps2fpga &
fpga2hps) */
- /* Disable all axi bridge (hps2fpga, lwhps2fpga &
fpga2hps) */ socfpga_bridges_reset();
- /* Initialize the FPGA Manager */
- status = fpgamgr_program_init((u32 *)rbf_data, rbf_size);
- if (status)
return status;
- /* Getting info about bitstream types */
- get_rbf_image_info(&rbfinfo, (u16 *)rbf_data);
- if (rbfinfo.section == periph_section) {
/* Initialize the FPGA Manager */status = fpgamgr_program_init((u32 *)rbf_data,rbf_size);
if (status)return status;- }
- if (rbfinfo.section == core_section &&
!(is_fpgamgr_early_user_mode() &&!is_fpgamgr_user_mode())) {
debug("FPGA : Must be in early release mode toprogram ");
debug("core bitstream.\n");return 0;- }
- /* Write the RBF data to FPGA Manager */
- /* Write the bitstream to FPGA Manager */
fpgamgr_program_write(rbf_data, rbf_size);
- return fpgamgr_program_finish();
- status = fpgamgr_program_finish();
- if (status) {
config_pins(gd->fdt_blob, "fpga");puts("FPGA: Enter user mode.\n");- }
- return status;
}
On Tue, 2019-02-05 at 09:41 +0100, Marek Vasut wrote:
On 2/2/19 4:27 AM, Chee, Tien Fong wrote:
On Fri, 2019-02-01 at 12:12 -0800, Dalon L Westergreen wrote:
On Thu, 2019-01-31 at 22:51 +0800, tien.fong.chee@intel.com wrote:
From: Tien Fong Chee tien.fong.chee@intel.com
Add FPGA driver to support program FPGA with FPGA bitstream loading from filesystem. The driver are designed based on generic firmware loader framework. The driver can handle FPGA program operation from loading FPGA bitstream in flash to memory and then to program FPGA.
Signed-off-by: Tien Fong Chee tien.fong.chee@intel.com
changes for v7
- Restructure the FPGA driver to support both peripheral
bitstream and core bitstream bundled into FIT image.
- Support loadable property for core bitstream. User can set
loadable in DDR for better performance. This loading would be done in one large chunk instead of chunk by chunk loading with small memory buffer.
arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts | 18 + .../include/mach/fpga_manager_arria10.h | 39 +- drivers/fpga/socfpga_arria10.c | 417 ++++++++++++++++++++- 3 files changed, 457 insertions(+), 17 deletions(-)
diff --git a/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts b/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts index 998d811..dc55618 100644 --- a/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts +++ b/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts @@ -18,6 +18,24 @@ /dts-v1/; #include "socfpga_arria10_socdk.dtsi" +/ {
- chosen {
firmware-loader = &fs_loader0;- };
- fs_loader0: fs-loader@0 {
u-boot,dm-pre-reloc;compatible = "u-boot,fs-loader";phandlepart = <&mmc 1>;- };
+};
+&fpga_mgr {
- u-boot,dm-pre-reloc;
- altr,bitstream = "fit_spl_fpga.itb";
- altr,bitstream-core = "fit_spl_fpga.itb";
+};
&mmc { u-boot,dm-pre-reloc; status = "okay"; diff --git a/arch/arm/mach- socfpga/include/mach/fpga_manager_arria10.h b/arch/arm/mach-socfpga/include/mach/fpga_manager_arria10.h index 09d13f6..683c84c 100644 --- a/arch/arm/mach-socfpga/include/mach/fpga_manager_arria10.h +++ b/arch/arm/mach-socfpga/include/mach/fpga_manager_arria10.h @@ -1,9 +1,13 @@ /* SPDX-License-Identifier: GPL-2.0 */ /*
- Copyright (C) 2017 Intel Corporation <www.intel.com>
- Copyright (C) 2017-2019 Intel Corporation <www.intel.com>
* All rights reserved. */
[...] }
- if (!is_fpgamgr_early_user_mode() ||
is_fpgamgr_user_mode()) {
fpga_node_name = "fpga-1";printf("FPGA: Start to program peripheralbitstream ...\n");
- } else if (!is_fpgamgr_user_mode()) {
fpga_node_name = "fpga-2";printf("FPGA: Start to program core bitstream...\n");
- }
Why are you relying on the node name to define the core / periph RBF? Would it not be better to define this in a property?
I need to ensure correct sequence of programming the rbf images at different FPGA mode. fpga with index 1 always means the 1st to program.
how would one have multiple core and periph rbfs in a single fit image?
I want strict control to avoid the performance penalty, when this happen, performance is almost same as programming full image.
So, in a single FIT image, only have one core rbf and periph rbf, and core rbf must always be the 1st and with the right absolute data position to avoid the buffer unaligned. Strict sequence control is also one of the reason having one core rbf and one periph rbf in a single FIT image.
I thought we established that you cannot depend on the placement and alignment of files within fitImage ?
This is just workaround, until performance penalty issue at reading cluster is solved. See the details of explanation at patch 2/7.
Also, you should be able to use fitImage configurations to select from multiple core and periph RBFs, there's no point in limiting ourselves to 1 core and 1 periph RBF with flexible format such as the fitImage.
See the details of explanation/proposal at patch 1/7.
You can create multiple FIT image for multiple core and periph rbfs. There are properties such as "altr,bitstream" and "altr,bitstream- core" for supporting different FIT image.
You can change those properties in U-Boot env(for runtime), or using different DTS for differnt configuration.
--dalon
- node_offset = fit_image_get_node(buffer_p,
fpga_node_name);
- if (node_offset < 0) {
debug("FPGA: Could not find node '%s' inFIT\n",
fpga_node_name);return -ENOENT;- }
- const char *uname = fit_get_name(buffer_p,
node_offset, NULL);
- if (uname)
debug("FPGA: %s\n", uname);- ret = fit_image_get_data_position(buffer_p,
node_offset, &rbf_offset);
- if (ret < 0) {
debug("FPGA: Could not find data position(err=%d)\n", ret);
return -ENOENT;- }
- ret = fit_image_get_data_size(buffer_p, node_offset,
&rbf_size);
- if (ret < 0) {
debug("FPGA: Could not find data size(err=%d)\n", ret);
return -ENOENT;- }
- ret = fit_image_get_load(buffer_p, node_offset, (ulong
*)loadable);
- if (ret < 0) {
debug("FPGA: Could not find loadable(err=%d)\n", ret);
debug("FPGA: Using default buffer andsize\n");
- } else {
buffer_p = (u32 *)*loadable;buffer_size = rbf_size;debug("FPGA: Found loadable address = 0x%x\n",*loadable);
- }
- debug("FPGA: External data: offset = 0x%x, size =
0x%x\n",
- rbf_offset, rbf_size);
- fpga_loadfs->remaining = rbf_size;
- /*
- * Determine buffer size vs bitstream size, and
calculating number of
- * chunk by chunk transfer is required due to smaller
buffer size
- * compare to bitstream
- */
- if (rbf_size <= buffer_size) {
/* Loading whole bitstream into buffer */buffer_size = rbf_size;fpga_loadfs->remaining = 0;- } else {
fpga_loadfs->remaining -= buffer_size;- }
- fpga_loadfs->offset = rbf_offset;
- /* Loading bitstream into buffer */
- ret = request_firmware_into_buf(dev,
fpga_loadfs-fpga_fsinfo-
filename,
buffer_p,buffer_size,fpga_loadfs->offset);- if (ret < 0) {
debug("FPGA: Failed to read bitstream fromflash.\n");
return -ENOENT;- }
- /* Update next reading bitstream offset */
- fpga_loadfs->offset += buffer_size;
- /* Getting info about bitstream types */
- get_rbf_image_info(&fpga_loadfs->rbfinfo, (u16
*)buffer_p);
- /* Update the final addr for bitstream */
- *buffer = (u32)buffer_p;
- /* Update the size of bitstream to be programmed into
FPGA */
- *buffer_bsize = buffer_size;
- return 0;
+}
+static int subsequent_loading_rbf_to_buffer(struct udevice *dev,
structfpga_loadfs_info *fpga_loadfs,
u32 *buffer, size_t*buffer_bsize) +{
- int ret = 0;
- u32 *buffer_p = (u32 *)*buffer;
- /* Read the bitstream chunk by chunk. */
- if (fpga_loadfs->remaining > *buffer_bsize) {
fpga_loadfs->remaining -= *buffer_bsize;- } else {
*buffer_bsize = fpga_loadfs->remaining;fpga_loadfs->remaining = 0;- }
- ret = request_firmware_into_buf(dev,
fpga_loadfs-fpga_fsinfo-
filename,
buffer_p,*buffer_bsize,fpga_loadfs->offset);- if (ret < 0) {
debug("FPGA: Failed to read bitstream fromflash.\n");
return -ENOENT;- }
- /* Update next reading bitstream offset */
- fpga_loadfs->offset += *buffer_bsize;
- return 0;
+}
+int socfpga_loadfs(fpga_fs_info *fpga_fsinfo, const void *buf, size_t bsize,
u32 offset)+{
- struct fpga_loadfs_info fpga_loadfs;
- int status = 0;
- int ret = 0;
- u32 buffer = (u32)buf;
- size_t buffer_sizebytes = bsize;
- size_t buffer_sizebytes_ori = bsize;
- size_t total_sizeof_image = 0;
- struct udevice *dev;
- ret = uclass_get_device(UCLASS_FS_FIRMWARE_LOADER, 0,
&dev);
- if (ret)
return ret;- memset(&fpga_loadfs, 0, sizeof(fpga_loadfs));
- WATCHDOG_RESET();
- fpga_loadfs.fpga_fsinfo = fpga_fsinfo;
- fpga_loadfs.offset = offset;
- printf("FPGA: Start to program ...\n");
- /*
- * Note: Both buffer and buffer_sizebytes values can
be altered by
- * function below.
- */
- ret = first_loading_rbf_to_buffer(dev, &fpga_loadfs,
&buffer,
&buffer_sizebytes);- if (ret)
return ret;- if (fpga_loadfs.rbfinfo.section == core_section &&
!(is_fpgamgr_early_user_mode() &&!is_fpgamgr_user_mode())) {
debug("FPGA : Must be in Early Release mode toprogram ");
debug("core bitstream.\n");return 0;- }
- /* Disable all signals from HPS peripheral controller
to FPGA */
- writel(0, &system_manager_base->fpgaintf_en_global);
- /* Disable all axi bridges (hps2fpga, lwhps2fpga &
fpga2hps) */
- socfpga_bridges_reset();
- if (fpga_loadfs.rbfinfo.section == periph_section) {
/* Initialize the FPGA Manager */status = fpgamgr_program_init((u32 *)buffer,buffer_sizebytes);
if (status) {debug("FPGA: Init with peripheralbitstream failed.\n");
return -EPERM;}- }
- WATCHDOG_RESET();
- /* Transfer bitstream to FPGA Manager */
- fpgamgr_program_write((void *)buffer,
buffer_sizebytes);
- total_sizeof_image += buffer_sizebytes;
- WATCHDOG_RESET();
- while (fpga_loadfs.remaining) {
ret = subsequent_loading_rbf_to_buffer(dev,&fpga_load fs,
&buffer,
&buffer_si zebytes_ori);
if (ret)return ret;/* Transfer data to FPGA Manager */fpgamgr_program_write((void *)buffer,buffer_sizebytes_ori);total_sizeof_image += buffer_sizebytes_ori;WATCHDOG_RESET();- }
- if (fpga_loadfs.rbfinfo.section == periph_section) {
if (fpgamgr_wait_early_user_mode() !=-ETIMEDOUT) {
config_pins(gd->fdt_blob, "shared");puts("FPGA: Early ReleaseSucceeded.\n");
} else {debug("FPGA: Failed to see EarlyRelease.\n");
return -EIO;}/* For monolithic bitstream */if (is_fpgamgr_user_mode()) {/* Ensure the FPGA entering configdone */
status = fpgamgr_program_finish();if (status)return status;config_pins(gd->fdt_blob, "fpga");puts("FPGA: Enter user mode.\n");}- } else if (fpga_loadfs.rbfinfo.section ==
core_section) {
/* Ensure the FPGA entering config done */status = fpgamgr_program_finish();if (status)return status;config_pins(gd->fdt_blob, "fpga");puts("FPGA: Enter user mode.\n");- } else {
debug("FPGA: Config Error: Unsupportedbitstream type.\n");
return -ENOEXEC;- }
- return (int)total_sizeof_image;
+} +#endif
+/* This function is used to load the core bitstream from the OCRAM. */ int socfpga_load(Altera_desc *desc, const void *rbf_data, size_t rbf_size) {
- int status;
- unsigned long status;
- struct rbf_info rbfinfo;
- /* disable all signals from hps peripheral controller
to fpga */
- memset(&rbfinfo, 0, sizeof(rbfinfo));
- /* Disable all signals from hps peripheral controller
to fpga */ writel(0, &system_manager_base->fpgaintf_en_global);
- /* disable all axi bridge (hps2fpga, lwhps2fpga &
fpga2hps) */
- /* Disable all axi bridge (hps2fpga, lwhps2fpga &
fpga2hps) */ socfpga_bridges_reset();
- /* Initialize the FPGA Manager */
- status = fpgamgr_program_init((u32 *)rbf_data,
rbf_size);
- if (status)
return status;
- /* Getting info about bitstream types */
- get_rbf_image_info(&rbfinfo, (u16 *)rbf_data);
- if (rbfinfo.section == periph_section) {
/* Initialize the FPGA Manager */status = fpgamgr_program_init((u32 *)rbf_data,rbf_size);
if (status)return status;- }
- if (rbfinfo.section == core_section &&
!(is_fpgamgr_early_user_mode() &&!is_fpgamgr_user_mode())) {
debug("FPGA : Must be in early release mode toprogram ");
debug("core bitstream.\n");return 0;- }
- /* Write the RBF data to FPGA Manager */
- /* Write the bitstream to FPGA Manager */
fpgamgr_program_write(rbf_data, rbf_size);
- return fpgamgr_program_finish();
- status = fpgamgr_program_finish();
- if (status) {
config_pins(gd->fdt_blob, "fpga");puts("FPGA: Enter user mode.\n");- }
- return status;
}
From: Tien Fong Chee tien.fong.chee@intel.com
Update the default configuration file to enable the necessary functionality to get the SoCFPGA loadfs driver support. This would enable the implementation of programming bitstream into FPGA from MMC.
Signed-off-by: Tien Fong Chee tien.fong.chee@intel.com
---
changes for v7 - Removed limit set for CONFIG_FS_FAT_MAX_CLUSTSIZE --- configs/socfpga_arria10_defconfig | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/configs/socfpga_arria10_defconfig b/configs/socfpga_arria10_defconfig index 0554f1b..fc0dfa4 100644 --- a/configs/socfpga_arria10_defconfig +++ b/configs/socfpga_arria10_defconfig @@ -27,9 +27,15 @@ CONFIG_MTDIDS_DEFAULT="nor0=ff705000.spi.0" # CONFIG_EFI_PARTITION is not set CONFIG_DEFAULT_DEVICE_TREE="socfpga_arria10_socdk_sdmmc" CONFIG_ENV_IS_IN_MMC=y +CONFIG_SPL_ENV_SUPPORT=y CONFIG_SPL_DM=y CONFIG_SPL_DM_SEQ_ALIAS=y +CONFIG_SPL_DM_MMC=y +CONFIG_SPL_MMC_SUPPORT=y +CONFIG_SPL_EXT_SUPPORT=y CONFIG_SPL_FS_FAT=y +CONFIG_SPL_DRIVERS_MISC_SUPPORT=y +CONFIG_FS_LOADER=y CONFIG_FPGA_SOCFPGA=y CONFIG_DM_GPIO=y CONFIG_DWAPB_GPIO=y
On 1/31/19 3:51 PM, tien.fong.chee@intel.com wrote:
From: Tien Fong Chee tien.fong.chee@intel.com
Update the default configuration file to enable the necessary functionality to get the SoCFPGA loadfs driver support. This would enable the implementation of programming bitstream into FPGA from MMC.
Signed-off-by: Tien Fong Chee tien.fong.chee@intel.com
changes for v7
- Removed limit set for CONFIG_FS_FAT_MAX_CLUSTSIZE
configs/socfpga_arria10_defconfig | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/configs/socfpga_arria10_defconfig b/configs/socfpga_arria10_defconfig index 0554f1b..fc0dfa4 100644 --- a/configs/socfpga_arria10_defconfig +++ b/configs/socfpga_arria10_defconfig @@ -27,9 +27,15 @@ CONFIG_MTDIDS_DEFAULT="nor0=ff705000.spi.0" # CONFIG_EFI_PARTITION is not set CONFIG_DEFAULT_DEVICE_TREE="socfpga_arria10_socdk_sdmmc" CONFIG_ENV_IS_IN_MMC=y +CONFIG_SPL_ENV_SUPPORT=y CONFIG_SPL_DM=y CONFIG_SPL_DM_SEQ_ALIAS=y +CONFIG_SPL_DM_MMC=y
Oddly enough, only 6/7 enables CONFIG_DW_MMC=y, so unless you apply also 6/7, this patch cannot work as intended.
+CONFIG_SPL_MMC_SUPPORT=y +CONFIG_SPL_EXT_SUPPORT=y CONFIG_SPL_FS_FAT=y +CONFIG_SPL_DRIVERS_MISC_SUPPORT=y +CONFIG_FS_LOADER=y CONFIG_FPGA_SOCFPGA=y CONFIG_DM_GPIO=y CONFIG_DWAPB_GPIO=y
On Thu, 2019-01-31 at 15:57 +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
Update the default configuration file to enable the necessary functionality to get the SoCFPGA loadfs driver support. This would enable the implementation of programming bitstream into FPGA from MMC.
Signed-off-by: Tien Fong Chee tien.fong.chee@intel.com
changes for v7
- Removed limit set for CONFIG_FS_FAT_MAX_CLUSTSIZE
configs/socfpga_arria10_defconfig | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/configs/socfpga_arria10_defconfig b/configs/socfpga_arria10_defconfig index 0554f1b..fc0dfa4 100644 --- a/configs/socfpga_arria10_defconfig +++ b/configs/socfpga_arria10_defconfig @@ -27,9 +27,15 @@ CONFIG_MTDIDS_DEFAULT="nor0=ff705000.spi.0" # CONFIG_EFI_PARTITION is not set CONFIG_DEFAULT_DEVICE_TREE="socfpga_arria10_socdk_sdmmc" CONFIG_ENV_IS_IN_MMC=y +CONFIG_SPL_ENV_SUPPORT=y CONFIG_SPL_DM=y CONFIG_SPL_DM_SEQ_ALIAS=y +CONFIG_SPL_DM_MMC=y
Oddly enough, only 6/7 enables CONFIG_DW_MMC=y, so unless you apply also 6/7, this patch cannot work as intended.
Good catch!! I think all FIT related configs from 6/7 should be moved to here because the FPGA driver now is FPGA FIT image implementation.
+CONFIG_SPL_MMC_SUPPORT=y +CONFIG_SPL_EXT_SUPPORT=y CONFIG_SPL_FS_FAT=y +CONFIG_SPL_DRIVERS_MISC_SUPPORT=y +CONFIG_FS_LOADER=y CONFIG_FPGA_SOCFPGA=y CONFIG_DM_GPIO=y CONFIG_DWAPB_GPIO=y
From: Tien Fong Chee tien.fong.chee@intel.com
Add support for loading FPGA bitstream to get DDR up running before U-Boot is loaded into DDR. Boot device initialization, generic firmware loader and SPL FAT support are required for this whole mechanism to work.
Signed-off-by: Tien Fong Chee tien.fong.chee@intel.com
---
changes for v7 - Removed casting for get_fpga_filename - Removed hard coding DDR address for loading core bistream, using loadable property from FIT. - Added checking for config_pins, return if error. --- arch/arm/mach-socfpga/spl_a10.c | 46 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mach-socfpga/spl_a10.c b/arch/arm/mach-socfpga/spl_a10.c index c97eacb..dae3b8c 100644 --- a/arch/arm/mach-socfpga/spl_a10.c +++ b/arch/arm/mach-socfpga/spl_a10.c @@ -1,6 +1,6 @@ // SPDX-License-Identifier: GPL-2.0+ /* - * Copyright (C) 2012 Altera Corporation <www.altera.com> + * Copyright (C) 2012-2019 Altera Corporation <www.altera.com> */
#include <common.h> @@ -23,6 +23,8 @@ #include <fdtdec.h> #include <watchdog.h> #include <asm/arch/pinmux.h> +#include <asm/arch/fpga_manager.h> +#include <mmc.h>
DECLARE_GLOBAL_DATA_PTR;
@@ -68,11 +70,53 @@ u32 spl_boot_mode(const u32 boot_device)
void spl_board_init(void) { + char buf[16 * 1024] __aligned(ARCH_DMA_MINALIGN); + /* enable console uart printing */ preloader_console_init(); WATCHDOG_RESET();
arch_early_init_r(); + + /* If the full FPGA is already loaded, ie.from EPCQ, config fpga pins */ + if (is_fpgamgr_user_mode()) { + int ret = config_pins(gd->fdt_blob, "shared"); + if (ret) + return; + + ret = config_pins(gd->fdt_blob, "fpga"); + if (ret) + return; + } else if (!is_fpgamgr_early_user_mode()) { + /* Program IOSSM(early IO release) or full FPGA */ + fpga_fs_info fpga_fsinfo; + int len; + + fpga_fsinfo.filename = get_fpga_filename( + gd->fdt_blob, + &len, + FPGA_SOCFPGA_A10_RBF_PERIPH); + + if (fpga_fsinfo.filename) + socfpga_loadfs(&fpga_fsinfo, buf, sizeof(buf), 0); + } + + /* If the IOSSM/full FPGA is already loaded, start DDR */ + if (is_fpgamgr_early_user_mode() || is_fpgamgr_user_mode()) + ddr_calibration_sequence(); + + if (!is_fpgamgr_user_mode()) { + fpga_fs_info fpga_fsinfo; + int len; + + fpga_fsinfo.filename = get_fpga_filename( + gd->fdt_blob, + &len, + FPGA_SOCFPGA_A10_RBF_CORE); + + if (fpga_fsinfo.filename) + socfpga_loadfs(&fpga_fsinfo, buf, sizeof(buf), 0); + } }
void board_init_f(ulong dummy)
From: Tien Fong Chee tien.fong.chee@intel.com
Update the default configuration file to enable the necessary functionality the get the kit working.
Signed-off-by: Tien Fong Chee tien.fong.chee@intel.com
---
changes for v7 - Keep minimal configs --- configs/socfpga_arria10_defconfig | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/configs/socfpga_arria10_defconfig b/configs/socfpga_arria10_defconfig index fc0dfa4..bdbf90e 100644 --- a/configs/socfpga_arria10_defconfig +++ b/configs/socfpga_arria10_defconfig @@ -10,10 +10,13 @@ CONFIG_NR_DRAM_BANKS=1 CONFIG_USE_BOOTARGS=y CONFIG_BOOTARGS="console=ttyS0,115200" # CONFIG_USE_BOOTCOMMAND is not set +CONFIG_SYS_CONSOLE_IS_IN_ENV=y +CONFIG_SYS_CONSOLE_OVERWRITE_ROUTINE=y +CONFIG_SYS_CONSOLE_ENV_OVERWRITE=y CONFIG_DEFAULT_FDT_FILE="socfpga_arria10_socdk_sdmmc.dtb" +CONFIG_VERSION_VARIABLE=y CONFIG_DISPLAY_BOARDINFO_LATE=y CONFIG_SPL_FPGA_SUPPORT=y -CONFIG_SPL_SPI_LOAD=y CONFIG_CMD_ASKENV=y CONFIG_CMD_GREPENV=y # CONFIG_CMD_FLASH is not set @@ -22,9 +25,7 @@ CONFIG_CMD_MMC=y CONFIG_CMD_CACHE=y CONFIG_CMD_EXT4_WRITE=y CONFIG_MTDIDS_DEFAULT="nor0=ff705000.spi.0" -# CONFIG_SPL_DOS_PARTITION is not set -# CONFIG_ISO_PARTITION is not set -# CONFIG_EFI_PARTITION is not set +CONFIG_OF_SPL_REMOVE_PROPS="interrupts interrupt-parent dmas dma-names" CONFIG_DEFAULT_DEVICE_TREE="socfpga_arria10_socdk_sdmmc" CONFIG_ENV_IS_IN_MMC=y CONFIG_SPL_ENV_SUPPORT=y @@ -32,20 +33,24 @@ CONFIG_SPL_DM=y CONFIG_SPL_DM_SEQ_ALIAS=y CONFIG_SPL_DM_MMC=y CONFIG_SPL_MMC_SUPPORT=y -CONFIG_SPL_EXT_SUPPORT=y CONFIG_SPL_FS_FAT=y CONFIG_SPL_DRIVERS_MISC_SUPPORT=y CONFIG_FS_LOADER=y CONFIG_FPGA_SOCFPGA=y +CONFIG_SPL_FIT=y +CONFIG_FIT=y CONFIG_DM_GPIO=y CONFIG_DWAPB_GPIO=y CONFIG_DM_MMC=y CONFIG_MTD_DEVICE=y +CONFIG_MMC_DW=y +CONFIG_PHY_MICREL=y +CONFIG_PHY_MICREL_KSZ90X1=y CONFIG_DM_ETH=y CONFIG_ETH_DESIGNWARE=y CONFIG_MII=y +CONFIG_SYS_NS16550=y CONFIG_SPI=y CONFIG_TIMER=y CONFIG_SPL_TIMER=y CONFIG_DESIGNWARE_APB_TIMER=y -CONFIG_USE_TINY_PRINTF=y
From: Tien Fong Chee tien.fong.chee@intel.com
After some series of patches to maximise reusable of memory pool, here come to result of reasonable size required for whole SDMMC boot working on A10 SoCDK. Size required come from default max cluster(0x100000) + others(0x2000) + additional memory for headroom(0x3000).
Signed-off-by: Tien Fong Chee tien.fong.chee@intel.com
---
changes for v7 - Added 0x3000 for memory headroom. --- include/configs/socfpga_common.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/configs/socfpga_common.h b/include/configs/socfpga_common.h index 4551cb2..548b458 100644 --- a/include/configs/socfpga_common.h +++ b/include/configs/socfpga_common.h @@ -1,6 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0+ */ /* - * Copyright (C) 2012 Altera Corporation <www.altera.com> + * Copyright (C) 2012-2019 Altera Corporation <www.altera.com> */ #ifndef __CONFIG_SOCFPGA_COMMON_H__ #define __CONFIG_SOCFPGA_COMMON_H__ @@ -258,7 +258,7 @@ unsigned int cm_get_qspi_controller_clk_hz(void); #if defined(CONFIG_TARGET_SOCFPGA_ARRIA10) /* SPL memory allocation configuration, this is for FAT implementation */ #ifndef CONFIG_SYS_SPL_MALLOC_START -#define CONFIG_SYS_SPL_MALLOC_SIZE 0x00010000 +#define CONFIG_SYS_SPL_MALLOC_SIZE 0x00015000 #define CONFIG_SYS_SPL_MALLOC_START (CONFIG_SYS_INIT_RAM_SIZE - \ CONFIG_SYS_SPL_MALLOC_SIZE + \ CONFIG_SYS_INIT_RAM_ADDR)
On 1/31/19 3:51 PM, tien.fong.chee@intel.com wrote:
From: Tien Fong Chee tien.fong.chee@intel.com
After some series of patches to maximise reusable of memory pool, here come to result of reasonable size required for whole SDMMC boot working on A10 SoCDK. Size required come from default max cluster(0x100000) +
The max cluster size is 0x10000 , one zero too many in the description.
others(0x2000) + additional memory for headroom(0x3000).
Signed-off-by: Tien Fong Chee tien.fong.chee@intel.com
changes for v7
- Added 0x3000 for memory headroom.
include/configs/socfpga_common.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/configs/socfpga_common.h b/include/configs/socfpga_common.h index 4551cb2..548b458 100644 --- a/include/configs/socfpga_common.h +++ b/include/configs/socfpga_common.h @@ -1,6 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0+ */ /*
- Copyright (C) 2012 Altera Corporation <www.altera.com>
*/
- Copyright (C) 2012-2019 Altera Corporation <www.altera.com>
#ifndef __CONFIG_SOCFPGA_COMMON_H__ #define __CONFIG_SOCFPGA_COMMON_H__ @@ -258,7 +258,7 @@ unsigned int cm_get_qspi_controller_clk_hz(void); #if defined(CONFIG_TARGET_SOCFPGA_ARRIA10) /* SPL memory allocation configuration, this is for FAT implementation */ #ifndef CONFIG_SYS_SPL_MALLOC_START -#define CONFIG_SYS_SPL_MALLOC_SIZE 0x00010000 +#define CONFIG_SYS_SPL_MALLOC_SIZE 0x00015000 #define CONFIG_SYS_SPL_MALLOC_START (CONFIG_SYS_INIT_RAM_SIZE - \ CONFIG_SYS_SPL_MALLOC_SIZE + \ CONFIG_SYS_INIT_RAM_ADDR)
On Thu, 2019-01-31 at 15:58 +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
After some series of patches to maximise reusable of memory pool, here come to result of reasonable size required for whole SDMMC boot working on A10 SoCDK. Size required come from default max cluster(0x100000) +
The max cluster size is 0x10000 , one zero too many in the description.
Good catch!! Blurry vision on my eyes when i was working too late :).
others(0x2000) + additional memory for headroom(0x3000).
Signed-off-by: Tien Fong Chee tien.fong.chee@intel.com
changes for v7
- Added 0x3000 for memory headroom.
include/configs/socfpga_common.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/configs/socfpga_common.h b/include/configs/socfpga_common.h index 4551cb2..548b458 100644 --- a/include/configs/socfpga_common.h +++ b/include/configs/socfpga_common.h @@ -1,6 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0+ */ /*
- Copyright (C) 2012 Altera Corporation <www.altera.com>
- Copyright (C) 2012-2019 Altera Corporation <www.altera.com>
*/ #ifndef __CONFIG_SOCFPGA_COMMON_H__ #define __CONFIG_SOCFPGA_COMMON_H__ @@ -258,7 +258,7 @@ unsigned int cm_get_qspi_controller_clk_hz(void); #if defined(CONFIG_TARGET_SOCFPGA_ARRIA10) /* SPL memory allocation configuration, this is for FAT implementation */ #ifndef CONFIG_SYS_SPL_MALLOC_START -#define CONFIG_SYS_SPL_MALLOC_SIZE 0x00010000 +#define CONFIG_SYS_SPL_MALLOC_SIZE 0x00015000 #define CONFIG_SYS_SPL_MALLOC_START (CONFIG_SYS_INIT_RAM_SI ZE - \ CONFIG_SYS_SPL_MALLOC_SIZ E + \ CONFIG_SYS_INIT_RAM_ADDR)
participants (5)
-
Chee, Tien Fong -
Dalon L Westergreen -
Marek Vasut -
tien.fong.chee＠intel.com
-
Westergreen, Dalon