[U-Boot] [PATCH 0/9] 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 FIT image which contains user FPGA design would be loaded into DDR location, where it's defined as 0x1000 by default in fit_spl_fpga.its. fpga load would be called to program core bitstream into FPGA and entering user mode.
Lastly, U-Boot from FIT image would be loaded to DDR, and up running from there.
For the 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: 1. u-boot.itb(default build) - uboot - fdt - fpga 2. ghrd_10as066n2.periph.rbf.mkimage mkimage -A arm -T firmware -C none -O u-boot -a 0 -e 0 -n "RBF" -d ghrd_10as066n2.periph.rbf ghrd_10as066n2.periph.rbf.mkimage
This series is working on top of u-boot.git - http://git.denx.de/u-boot.git .
Tien Fong Chee (9): ARM: socfpga: Description on FPGA bitstream type and file name for Arria 10 ARM: socfpga: Add FPGA drivers for Arria 10 FPGA bitstream loading spl : socfpga: Implement fpga bitstream loading with socfpga loadfs ARM: socfpga: Bundle U-Boot fitImage into SFP on Arria10 ARM: socfpga: Add SPL fitImage config match ARM: socfpga: Set default DTB address on A10 ARM: socfpga: Use custom header target buffer in SPL ARM: socfpga: Add default fitImage for Arria10 SoCDK ARM: socfpga: Synchronize the configuration for A10 SoCDK
Makefile | 9 +- arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts | 18 + arch/arm/mach-socfpga/board.c | 8 + .../include/mach/fpga_manager_arria10.h | 30 ++- arch/arm/mach-socfpga/spl_a10.c | 70 ++++- board/altera/arria10-socdk/fit_spl_fpga.its | 54 +++ common/spl/spl_mmc.c | 2 +- configs/socfpga_arria10_defconfig | 51 +++- .../fpga/altera-socfpga-a10-fpga-mgr.txt | 6 + drivers/fpga/Kconfig | 9 + drivers/fpga/socfpga_arria10.c | 417 +++++++++++++++++++- include/configs/socfpga_common.h | 4 + include/mmc.h | 1 + 13 files changed, 657 insertions(+), 22 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 --- .../fpga/altera-socfpga-a10-fpga-mgr.txt | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-)
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..010322a 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,6 +7,10 @@ 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_periph : File name for FPGA peripheral raw binary which is used + to initialize FPGA IOs, PLL, IO48 and DDR. +- altr,bitstream_core : File name for core raw binary which contains FPGA design + which is used to program FPGA CRAM and ERAM.
Example:
@@ -16,4 +20,6 @@ Example: 0xffcfe400 0x20>; clocks = <&l4_mp_clk>; resets = <&rst FPGAMGR_RESET>; + altr,bitstream_periph = "ghrd_10as066n2.periph.rbf.mkimage"; + altr,bitstream_core = "ghrd_10as066n2.core.rbf.mkimage"; };
On 11/21/2018 11:41 AM, 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
.../fpga/altera-socfpga-a10-fpga-mgr.txt | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-)
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..010322a 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,6 +7,10 @@ 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_periph : File name for FPGA peripheral raw binary which is used
to initialize FPGA IOs, PLL, IO48 and DDR.+- altr,bitstream_core : File name for core raw binary which contains FPGA design
which is used to program FPGA CRAM and ERAM.
bitstream- instead of bitstream_
btw can we get something that works with full bitstream too ?
Example:
@@ -16,4 +20,6 @@ Example: 0xffcfe400 0x20>; clocks = <&l4_mp_clk>; resets = <&rst FPGAMGR_RESET>;
altr,bitstream_periph = "ghrd_10as066n2.periph.rbf.mkimage";altr,bitstream_core = "ghrd_10as066n2.core.rbf.mkimage";
On Wed, 2018-11-21 at 15:11 +0100, Marek Vasut wrote:
On 11/21/2018 11:41 AM, 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
.../fpga/altera-socfpga-a10-fpga-mgr.txt | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-)
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..010322a 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,6 +7,10 @@ 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_periph : File name for FPGA peripheral raw binary which is used
to initialize FPGA IOs, PLL, IO48 andDDR. +- altr,bitstream_core : File name for core raw binary which contains FPGA design
which is used to program FPGA CRAM andERAM.
bitstream- instead of bitstream_
Noted.
btw can we get something that works with full bitstream too ?
This patchset actually support the full bitstream too, unfortunately it is blocked by hardware MPFE issue. The patchset for the MPFE workaround would come after this patchset. I would advice to use the early IO release method for the sake of performance.
For details of issue, you can read the from the link https://github.com /altera-opensource/u-boot-socfpga/commits/socfpga_v2014.10_arria10_brin gup FogBugz #410989-6: Masking hardware sequenced warm reset for logic in… …
Tien Fong Chee Tien Fong Chee committed on Feb 16, 2017 FogBugz #410989-5: Enable RAM boot …
Tien Fong Chee Tien Fong Chee committed on Feb 16, 2017 FogBugz #410989-4: Added software reset for QSPI …
Tien Fong Chee Tien Fong Chee committed on Feb 8, 2017 FogBugz #410989-3: Disable redundant redundant messages after a warm … …
Tien Fong Chee Tien Fong Chee committed on Dec 21, 2016 FogBugz #410989-2: Reset MPFE NoC after programming periperal rbf …
Tien Fong Chee Tien Fong Chee committed on Dec 21, 2016 FogBugz #410989-1: Functions for setting/checking magic no. to isw_ha… …
Tien Fong Chee Tien Fong Chee committed on Dec 21, 2016
Example: @@ -16,4 +20,6 @@ Example: 0xffcfe400 0x20>; clocks = <&l4_mp_clk>; resets = <&rst FPGAMGR_RESET>;
altr,bitstream_periph ="ghrd_10as066n2.periph.rbf.mkimage";
altr,bitstream_core ="ghrd_10as066n2.core.rbf.mkimage"; };
On 11/23/2018 10:19 AM, Chee, Tien Fong wrote:
On Wed, 2018-11-21 at 15:11 +0100, Marek Vasut wrote:
On 11/21/2018 11:41 AM, 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
.../fpga/altera-socfpga-a10-fpga-mgr.txt | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-)
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..010322a 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,6 +7,10 @@ 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_periph : File name for FPGA peripheral raw binary which is used
to initialize FPGA IOs, PLL, IO48 andDDR. +- altr,bitstream_core : File name for core raw binary which contains FPGA design
which is used to program FPGA CRAM andERAM.
bitstream- instead of bitstream_
Noted.
btw can we get something that works with full bitstream too ?
This patchset actually support the full bitstream too, unfortunately it is blocked by hardware MPFE issue. The patchset for the MPFE workaround would come after this patchset. I would advice to use the early IO release method for the sake of performance.
For details of issue, you can read the from the link https://github.com /altera-opensource/u-boot-socfpga/commits/socfpga_v2014.10_arria10_brin gup FogBugz #410989-6: Masking hardware sequenced warm reset for logic in… …
Does that work on ES2 ? I don't think so ...
On Fri, 2018-11-23 at 13:23 +0100, Marek Vasut wrote:
On 11/23/2018 10:19 AM, Chee, Tien Fong wrote:
On Wed, 2018-11-21 at 15:11 +0100, Marek Vasut wrote:
On 11/21/2018 11:41 AM, 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
.../fpga/altera-socfpga-a10-fpga-mgr.txt | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-)
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..010322a 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,6 +7,10 @@ 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_periph : File name for FPGA peripheral raw binary which is used
to initialize FPGA IOs, PLL, IO48and DDR. +- altr,bitstream_core : File name for core raw binary which contains FPGA design
which is used to program FPGA CRAM andERAM.
bitstream- instead of bitstream_
Noted.
btw can we get something that works with full bitstream too ?
This patchset actually support the full bitstream too, unfortunately it is blocked by hardware MPFE issue. The patchset for the MPFE workaround would come after this patchset. I would advice to use the early IO release method for the sake of performance.
For details of issue, you can read the from the link https://github .com /altera-opensource/u-boot- socfpga/commits/socfpga_v2014.10_arria10_brin gup FogBugz #410989-6: Masking hardware sequenced warm reset for logic in… …
Does that work on ES2 ? I don't think so ...
Why you think it doesn't work, using early IO or full rbf? The bitstream limitation? What you see from the print out?
On 11/26/2018 10:44 AM, Chee, Tien Fong wrote:
On Fri, 2018-11-23 at 13:23 +0100, Marek Vasut wrote:
On 11/23/2018 10:19 AM, Chee, Tien Fong wrote:
On Wed, 2018-11-21 at 15:11 +0100, Marek Vasut wrote:
On 11/21/2018 11:41 AM, 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
.../fpga/altera-socfpga-a10-fpga-mgr.txt | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-)
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..010322a 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,6 +7,10 @@ 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_periph : File name for FPGA peripheral raw binary which is used
to initialize FPGA IOs, PLL, IO48and DDR. +- altr,bitstream_core : File name for core raw binary which contains FPGA design
which is used to program FPGA CRAM andERAM.
bitstream- instead of bitstream_
Noted.
btw can we get something that works with full bitstream too ?
This patchset actually support the full bitstream too, unfortunately it is blocked by hardware MPFE issue. The patchset for the MPFE workaround would come after this patchset. I would advice to use the early IO release method for the sake of performance.
For details of issue, you can read the from the link https://github .com /altera-opensource/u-boot- socfpga/commits/socfpga_v2014.10_arria10_brin gup FogBugz #410989-6: Masking hardware sequenced warm reset for logic in… …
Does that work on ES2 ? I don't think so ...
Why you think it doesn't work, using early IO or full rbf? The bitstream limitation? What you see from the print out?
ES2 can only use full RBF, I don't think this is handled in this patchset at all.
On Mon, 2018-11-26 at 12:15 +0100, Marek Vasut wrote:
On 11/26/2018 10:44 AM, Chee, Tien Fong wrote:
On Fri, 2018-11-23 at 13:23 +0100, Marek Vasut wrote:
On 11/23/2018 10:19 AM, Chee, Tien Fong wrote:
On Wed, 2018-11-21 at 15:11 +0100, Marek Vasut wrote:
On 11/21/2018 11:41 AM, 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
.../fpga/altera-socfpga-a10-fpga-mgr.txt | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-)
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..010322a 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,6 +7,10 @@ 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_periph : File name for FPGA peripheral raw binary which is used
to initialize FPGA IOs, PLL,IO48 and DDR. +- altr,bitstream_core : File name for core raw binary which contains FPGA design
which is used to program FPGA CRAMand ERAM.
bitstream- instead of bitstream_
Noted.
btw can we get something that works with full bitstream too ?
This patchset actually support the full bitstream too, unfortunately it is blocked by hardware MPFE issue. The patchset for the MPFE workaround would come after this patchset. I would advice to use the early IO release method for the sake of performance.
For details of issue, you can read the from the link https://gi thub .com /altera-opensource/u-boot- socfpga/commits/socfpga_v2014.10_arria10_brin gup FogBugz #410989-6: Masking hardware sequenced warm reset for logic in… …
Does that work on ES2 ? I don't think so ...
Why you think it doesn't work, using early IO or full rbf? The bitstream limitation? What you see from the print out?
ES2 can only use full RBF, I don't think this is handled in this patchset at all.
i did testing the full rbf loading, but in the end i removed that portion of codes because it stuck in DDR calibration due to MPFE HW issue. So, i would put back that portion of codes after MPFE HW workaround. My plan is to let early IO release up 1st.
Actually ES2 board also support early IO release, just you need ACDS and SOCEDS version 17.1 onward to rebuild your hardware, and choosing the early IO release setting inside the tool. We can discuss this more if you need our help for early IO release RBFs.
On 11/27/2018 09:45 AM, Chee, Tien Fong wrote:
On Mon, 2018-11-26 at 12:15 +0100, Marek Vasut wrote:
On 11/26/2018 10:44 AM, Chee, Tien Fong wrote:
On Fri, 2018-11-23 at 13:23 +0100, Marek Vasut wrote:
On 11/23/2018 10:19 AM, Chee, Tien Fong wrote:
On Wed, 2018-11-21 at 15:11 +0100, Marek Vasut wrote:
On 11/21/2018 11:41 AM, 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 > --- > .../fpga/altera-socfpga-a10-fpga-mgr.txt | 6 > ++++++ > 1 files changed, 6 insertions(+), 0 deletions(-) > > 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..010322a 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,6 +7,10 @@ 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_periph : File name for FPGA peripheral > raw > binary > which is used > + to initialize FPGA IOs, PLL, > IO48 > and > DDR. > +- altr,bitstream_core : File name for core raw binary > which > contains FPGA design > + which is used to program FPGA CRAM > and > ERAM. bitstream- instead of bitstream_
Noted.
btw can we get something that works with full bitstream too ?
This patchset actually support the full bitstream too, unfortunately it is blocked by hardware MPFE issue. The patchset for the MPFE workaround would come after this patchset. I would advice to use the early IO release method for the sake of performance.
For details of issue, you can read the from the link https://gi thub .com /altera-opensource/u-boot- socfpga/commits/socfpga_v2014.10_arria10_brin gup FogBugz #410989-6: Masking hardware sequenced warm reset for logic in… …
Does that work on ES2 ? I don't think so ...
Why you think it doesn't work, using early IO or full rbf? The bitstream limitation? What you see from the print out?
ES2 can only use full RBF, I don't think this is handled in this patchset at all.
i did testing the full rbf loading, but in the end i removed that portion of codes because it stuck in DDR calibration due to MPFE HW issue. So, i would put back that portion of codes after MPFE HW workaround. My plan is to let early IO release up 1st.
Can you describe that workaround ? The code worked on the A10ES2 kit I have back around v2018.09, so what's the problem ?
Actually ES2 board also support early IO release, just you need ACDS and SOCEDS version 17.1 onward to rebuild your hardware, and choosing the early IO release setting inside the tool. We can discuss this more if you need our help for early IO release RBFs.
I do, otherwise I won't be able to test anything, so make sure this is supported.
On Tue, 2018-11-27 at 13:07 +0100, Marek Vasut wrote:
On 11/27/2018 09:45 AM, Chee, Tien Fong wrote:
On Mon, 2018-11-26 at 12:15 +0100, Marek Vasut wrote:
On 11/26/2018 10:44 AM, Chee, Tien Fong wrote:
On Fri, 2018-11-23 at 13:23 +0100, Marek Vasut wrote:
On 11/23/2018 10:19 AM, Chee, Tien Fong wrote:
On Wed, 2018-11-21 at 15:11 +0100, Marek Vasut wrote: > > > > On 11/21/2018 11:41 AM, 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 > > > > > --- > > .../fpga/altera-socfpga-a10-fpga- > > mgr.txt | 6 > > ++++++ > > 1 files changed, 6 insertions(+), 0 deletions(-) > > > > 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..010322a 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,6 +7,10 @@ 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_periph : File name for FPGA > > peripheral > > raw > > binary > > which is used > > + to initialize FPGA IOs, PLL, > > IO48 > > and > > DDR. > > +- altr,bitstream_core : File name for core raw binary > > which > > contains FPGA design > > + which is used to program FPGA > > CRAM > > and > > ERAM. > bitstream- instead of bitstream_ Noted. > > > > > btw can we get something that works with full bitstream > too ? This patchset actually support the full bitstream too, unfortunately it is blocked by hardware MPFE issue. The patchset for the MPFE workaround would come after this patchset. I would advice to use the early IO release method for the sake of performance.
For details of issue, you can read the from the link https: //gi thub .com /altera-opensource/u-boot- socfpga/commits/socfpga_v2014.10_arria10_brin gup FogBugz #410989-6: Masking hardware sequenced warm reset for logic in… …
Does that work on ES2 ? I don't think so ...
Why you think it doesn't work, using early IO or full rbf? The bitstream limitation? What you see from the print out?
ES2 can only use full RBF, I don't think this is handled in this patchset at all.
i did testing the full rbf loading, but in the end i removed that portion of codes because it stuck in DDR calibration due to MPFE HW issue. So, i would put back that portion of codes after MPFE HW workaround. My plan is to let early IO release up 1st.
Can you describe that workaround ? The code worked on the A10ES2 kit I have back around v2018.09, so what's the problem ?
Actually ES2 board also support early IO release, just you need ACDS and SOCEDS version 17.1 onward to rebuild your hardware, and choosing the early IO release setting inside the tool. We can discuss this more if you need our help for early IO release RBFs.
I do, otherwise I won't be able to test anything, so make sure this is supported.
Great, i would send out seprate email discussion with you, and inviting our aplication and hardware engineer into the pool.
By the way, how you get the full rbf or you build it yourself? Do you have the hardware design? What are the tools you have? what version?
On 11/28/2018 03:49 PM, Chee, Tien Fong wrote:
On Tue, 2018-11-27 at 13:07 +0100, Marek Vasut wrote:
On 11/27/2018 09:45 AM, Chee, Tien Fong wrote:
On Mon, 2018-11-26 at 12:15 +0100, Marek Vasut wrote:
On 11/26/2018 10:44 AM, Chee, Tien Fong wrote:
On Fri, 2018-11-23 at 13:23 +0100, Marek Vasut wrote:
On 11/23/2018 10:19 AM, Chee, Tien Fong wrote: > > > > On Wed, 2018-11-21 at 15:11 +0100, Marek Vasut wrote: >> >> >> >> On 11/21/2018 11:41 AM, 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 >>>> >>> --- >>> .../fpga/altera-socfpga-a10-fpga- >>> mgr.txt | 6 >>> ++++++ >>> 1 files changed, 6 insertions(+), 0 deletions(-) >>> >>> 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..010322a 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,6 +7,10 @@ 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_periph : File name for FPGA >>> peripheral >>> raw >>> binary >>> which is used >>> + to initialize FPGA IOs, PLL, >>> IO48 >>> and >>> DDR. >>> +- altr,bitstream_core : File name for core raw binary >>> which >>> contains FPGA design >>> + which is used to program FPGA >>> CRAM >>> and >>> ERAM. >> bitstream- instead of bitstream_ > Noted. >> >> >> >> >> btw can we get something that works with full bitstream >> too ? > This patchset actually support the full bitstream too, > unfortunately it > is blocked by hardware MPFE issue. The patchset for the > MPFE > workaround > would come after this patchset. I would advice to use the > early > IO > release method for the sake of performance. > > For details of issue, you can read the from the link https: > //gi > thub > .com > /altera-opensource/u-boot- > socfpga/commits/socfpga_v2014.10_arria10_brin > gup > FogBugz #410989-6: Masking hardware sequenced warm reset > for > logic > in… … Does that work on ES2 ? I don't think so ...
Why you think it doesn't work, using early IO or full rbf? The bitstream limitation? What you see from the print out?
ES2 can only use full RBF, I don't think this is handled in this patchset at all.
i did testing the full rbf loading, but in the end i removed that portion of codes because it stuck in DDR calibration due to MPFE HW issue. So, i would put back that portion of codes after MPFE HW workaround. My plan is to let early IO release up 1st.
Can you describe that workaround ? The code worked on the A10ES2 kit I have back around v2018.09, so what's the problem ?
Actually ES2 board also support early IO release, just you need ACDS and SOCEDS version 17.1 onward to rebuild your hardware, and choosing the early IO release setting inside the tool. We can discuss this more if you need our help for early IO release RBFs.
I do, otherwise I won't be able to test anything, so make sure this is supported.
Great, i would send out seprate email discussion with you, and inviting our aplication and hardware engineer into the pool.
By the way, how you get the full rbf or you build it yourself? Do you have the hardware design? What are the tools you have? what version?
I'm using the GHRD from altera wiki.
On Wed, 2018-11-28 at 16:10 +0100, Marek Vasut wrote:
On 11/28/2018 03:49 PM, Chee, Tien Fong wrote:
On Tue, 2018-11-27 at 13:07 +0100, Marek Vasut wrote:
On 11/27/2018 09:45 AM, Chee, Tien Fong wrote:
On Mon, 2018-11-26 at 12:15 +0100, Marek Vasut wrote:
On 11/26/2018 10:44 AM, Chee, Tien Fong wrote:
On Fri, 2018-11-23 at 13:23 +0100, Marek Vasut wrote: > > > > On 11/23/2018 10:19 AM, Chee, Tien Fong wrote: > > > > > > > > > > On Wed, 2018-11-21 at 15:11 +0100, Marek Vasut wrote: > > > > > > > > > > > > > > > On 11/21/2018 11:41 AM, 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 > > > > > > > > > > > > > > --- > > > > .../fpga/altera-socfpga-a10-fpga- > > > > mgr.txt | 6 > > > > ++++++ > > > > 1 files changed, 6 insertions(+), 0 deletions(-) > > > > > > > > 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..010322a 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,6 +7,10 @@ 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_periph : File name for FPGA > > > > peripheral > > > > raw > > > > binary > > > > which is used > > > > + to initialize FPGA IOs, > > > > PLL, > > > > IO48 > > > > and > > > > DDR. > > > > +- altr,bitstream_core : File name for core raw > > > > binary > > > > which > > > > contains FPGA design > > > > + which is used to program > > > > FPGA > > > > CRAM > > > > and > > > > ERAM. > > > bitstream- instead of bitstream_ > > Noted. > > > > > > > > > > > > > > > > > > btw can we get something that works with full > > > bitstream > > > too ? > > This patchset actually support the full bitstream too, > > unfortunately it > > is blocked by hardware MPFE issue. The patchset for the > > MPFE > > workaround > > would come after this patchset. I would advice to use > > the > > early > > IO > > release method for the sake of performance. > > > > For details of issue, you can read the from the > > link https: > > //gi > > thub > > .com > > /altera-opensource/u-boot- > > socfpga/commits/socfpga_v2014.10_arria10_brin > > gup > > FogBugz #410989-6: Masking hardware sequenced warm > > reset > > for > > logic > > in… … > Does that work on ES2 ? I don't think so ... Why you think it doesn't work, using early IO or full rbf? The bitstream limitation? What you see from the print out?
ES2 can only use full RBF, I don't think this is handled in this patchset at all.
i did testing the full rbf loading, but in the end i removed that portion of codes because it stuck in DDR calibration due to MPFE HW issue. So, i would put back that portion of codes after MPFE HW workaround. My plan is to let early IO release up 1st.
Can you describe that workaround ? The code worked on the A10ES2 kit I have back around v2018.09, so what's the problem ?
Actually ES2 board also support early IO release, just you need ACDS and SOCEDS version 17.1 onward to rebuild your hardware, and choosing the early IO release setting inside the tool. We can discuss this more if you need our help for early IO release RBFs.
I do, otherwise I won't be able to test anything, so make sure this is supported.
Great, i would send out seprate email discussion with you, and inviting our aplication and hardware engineer into the pool.
By the way, how you get the full rbf or you build it yourself? Do you have the hardware design? What are the tools you have? what version?
I'm using the GHRD from altera wiki.
Do you have the link?
On Wed, 2018-11-28 at 16:10 +0100, Marek Vasut wrote:
On 11/28/2018 03:49 PM, Chee, Tien Fong wrote:
On Tue, 2018-11-27 at 13:07 +0100, Marek Vasut wrote:
On 11/27/2018 09:45 AM, Chee, Tien Fong wrote:
On Mon, 2018-11-26 at 12:15 +0100, Marek Vasut wrote:
On 11/26/2018 10:44 AM, Chee, Tien Fong wrote:
On Fri, 2018-11-23 at 13:23 +0100, Marek Vasut wrote: > > > > On 11/23/2018 10:19 AM, Chee, Tien Fong wrote: > > > > > > > > > > On Wed, 2018-11-21 at 15:11 +0100, Marek Vasut wrote: > > > > > > > > > > > > > > > On 11/21/2018 11:41 AM, 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 > > > > > > > > > > > > > > --- > > > > .../fpga/altera-socfpga-a10-fpga- > > > > mgr.txt | 6 > > > > ++++++ > > > > 1 files changed, 6 insertions(+), 0 deletions(-) > > > > > > > > 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..010322a 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,6 +7,10 @@ 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_periph : File name for FPGA > > > > peripheral > > > > raw > > > > binary > > > > which is used > > > > + to initialize FPGA IOs, > > > > PLL, > > > > IO48 > > > > and > > > > DDR. > > > > +- altr,bitstream_core : File name for core raw > > > > binary > > > > which > > > > contains FPGA design > > > > + which is used to program > > > > FPGA > > > > CRAM > > > > and > > > > ERAM. > > > bitstream- instead of bitstream_ > > Noted. > > > > > > > > > > > > > > > > > > btw can we get something that works with full > > > bitstream > > > too ? > > This patchset actually support the full bitstream too, > > unfortunately it > > is blocked by hardware MPFE issue. The patchset for the > > MPFE > > workaround > > would come after this patchset. I would advice to use > > the > > early > > IO > > release method for the sake of performance. > > > > For details of issue, you can read the from the > > link https: > > //gi > > thub > > .com > > /altera-opensource/u-boot- > > socfpga/commits/socfpga_v2014.10_arria10_brin > > gup > > FogBugz #410989-6: Masking hardware sequenced warm > > reset > > for > > logic > > in… … > Does that work on ES2 ? I don't think so ... Why you think it doesn't work, using early IO or full rbf? The bitstream limitation? What you see from the print out?
ES2 can only use full RBF, I don't think this is handled in this patchset at all.
i did testing the full rbf loading, but in the end i removed that portion of codes because it stuck in DDR calibration due to MPFE HW issue. So, i would put back that portion of codes after MPFE HW workaround. My plan is to let early IO release up 1st.
Can you describe that workaround ? The code worked on the A10ES2 kit I have back around v2018.09, so what's the problem ?
There is a corruption to MPFE NoC(which function like ethernet QOS, but its main function for priotizing and controlling traffic to access DDR from MPU and FPGA) due to high fequency transient clock out from HPS EMIF IOPLL at VCO startup. The corruption happens intermittent on some boards. The workaround is to trigger warm reset to recover MPFE NoC from corruption after programing the periph rbf or full rbf. Once U- Boot reentrance after warm reset, only core rbf is allowed to configured FPGA.
Actually ES2 board also support early IO release, just you need ACDS and SOCEDS version 17.1 onward to rebuild your hardware, and choosing the early IO release setting inside the tool. We can discuss this more if you need our help for early IO release RBFs.
I do, otherwise I won't be able to test anything, so make sure this is supported.
Great, i would send out seprate email discussion with you, and inviting our aplication and hardware engineer into the pool.
By the way, how you get the full rbf or you build it yourself? Do you have the hardware design? What are the tools you have? what version?
I'm using the GHRD from altera wiki.
On 11/28/2018 05:17 PM, Chee, Tien Fong wrote:
On Wed, 2018-11-28 at 16:10 +0100, Marek Vasut wrote:
On 11/28/2018 03:49 PM, Chee, Tien Fong wrote:
On Tue, 2018-11-27 at 13:07 +0100, Marek Vasut wrote:
On 11/27/2018 09:45 AM, Chee, Tien Fong wrote:
On Mon, 2018-11-26 at 12:15 +0100, Marek Vasut wrote:
On 11/26/2018 10:44 AM, Chee, Tien Fong wrote: > > > > On Fri, 2018-11-23 at 13:23 +0100, Marek Vasut wrote: >> >> >> >> On 11/23/2018 10:19 AM, Chee, Tien Fong wrote: >>> >>> >>> >>> >>> On Wed, 2018-11-21 at 15:11 +0100, Marek Vasut wrote: >>>> >>>> >>>> >>>> >>>> On 11/21/2018 11:41 AM, 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 >>>>>> >>>>>> >>>>> --- >>>>> .../fpga/altera-socfpga-a10-fpga- >>>>> mgr.txt | 6 >>>>> ++++++ >>>>> 1 files changed, 6 insertions(+), 0 deletions(-) >>>>> >>>>> 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..010322a 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,6 +7,10 @@ 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_periph : File name for FPGA >>>>> peripheral >>>>> raw >>>>> binary >>>>> which is used >>>>> + to initialize FPGA IOs, >>>>> PLL, >>>>> IO48 >>>>> and >>>>> DDR. >>>>> +- altr,bitstream_core : File name for core raw >>>>> binary >>>>> which >>>>> contains FPGA design >>>>> + which is used to program >>>>> FPGA >>>>> CRAM >>>>> and >>>>> ERAM. >>>> bitstream- instead of bitstream_ >>> Noted. >>>> >>>> >>>> >>>> >>>> >>>> btw can we get something that works with full >>>> bitstream >>>> too ? >>> This patchset actually support the full bitstream too, >>> unfortunately it >>> is blocked by hardware MPFE issue. The patchset for the >>> MPFE >>> workaround >>> would come after this patchset. I would advice to use >>> the >>> early >>> IO >>> release method for the sake of performance. >>> >>> For details of issue, you can read the from the >>> link https: >>> //gi >>> thub >>> .com >>> /altera-opensource/u-boot- >>> socfpga/commits/socfpga_v2014.10_arria10_brin >>> gup >>> FogBugz #410989-6: Masking hardware sequenced warm >>> reset >>> for >>> logic >>> in… … >> Does that work on ES2 ? I don't think so ... > Why you think it doesn't work, using early IO or full rbf? > The > bitstream limitation? What you see from the print out? ES2 can only use full RBF, I don't think this is handled in this patchset at all.
i did testing the full rbf loading, but in the end i removed that portion of codes because it stuck in DDR calibration due to MPFE HW issue. So, i would put back that portion of codes after MPFE HW workaround. My plan is to let early IO release up 1st.
Can you describe that workaround ? The code worked on the A10ES2 kit I have back around v2018.09, so what's the problem ?
There is a corruption to MPFE NoC(which function like ethernet QOS, but its main function for priotizing and controlling traffic to access DDR from MPU and FPGA) due to high fequency transient clock out from HPS EMIF IOPLL at VCO startup. The corruption happens intermittent on some boards. The workaround is to trigger warm reset to recover MPFE NoC from corruption after programing the periph rbf or full rbf. Once U- Boot reentrance after warm reset, only core rbf is allowed to configured FPGA.
And this is present in ES1 or ES2 ? Or is it also in PS ?
I wonder whether we want to scrap the ES support afterall.
On Wed, 2018-11-28 at 18:55 +0100, Marek Vasut wrote:
On 11/28/2018 05:17 PM, Chee, Tien Fong wrote:
On Wed, 2018-11-28 at 16:10 +0100, Marek Vasut wrote:
On 11/28/2018 03:49 PM, Chee, Tien Fong wrote:
On Tue, 2018-11-27 at 13:07 +0100, Marek Vasut wrote:
On 11/27/2018 09:45 AM, Chee, Tien Fong wrote:
On Mon, 2018-11-26 at 12:15 +0100, Marek Vasut wrote: > > > > On 11/26/2018 10:44 AM, Chee, Tien Fong wrote: > > > > > > > > > > On Fri, 2018-11-23 at 13:23 +0100, Marek Vasut wrote: > > > > > > > > > > > > > > > On 11/23/2018 10:19 AM, Chee, Tien Fong wrote: > > > > > > > > > > > > > > > > > > > > > > > > On Wed, 2018-11-21 at 15:11 +0100, Marek Vasut > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On 11/21/2018 11:41 AM, 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@i > > > > > > ntel > > > > > > .com > > > > > > > > > > > > > > > > > > > > > > > > > > > --- > > > > > > .../fpga/altera-socfpga-a10-fpga- > > > > > > mgr.txt | 6 > > > > > > ++++++ > > > > > > 1 files changed, 6 insertions(+), 0 > > > > > > deletions(-) > > > > > > > > > > > > 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..010322a 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,6 +7,10 @@ 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_periph : File name for FPGA > > > > > > peripheral > > > > > > raw > > > > > > binary > > > > > > which is used > > > > > > + to initialize FPGA > > > > > > IOs, > > > > > > PLL, > > > > > > IO48 > > > > > > and > > > > > > DDR. > > > > > > +- altr,bitstream_core : File name for core raw > > > > > > binary > > > > > > which > > > > > > contains FPGA design > > > > > > + which is used to > > > > > > program > > > > > > FPGA > > > > > > CRAM > > > > > > and > > > > > > ERAM. > > > > > bitstream- instead of bitstream_ > > > > Noted. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > btw can we get something that works with full > > > > > bitstream > > > > > too ? > > > > This patchset actually support the full bitstream > > > > too, > > > > unfortunately it > > > > is blocked by hardware MPFE issue. The patchset for > > > > the > > > > MPFE > > > > workaround > > > > would come after this patchset. I would advice to > > > > use > > > > the > > > > early > > > > IO > > > > release method for the sake of performance. > > > > > > > > For details of issue, you can read the from the > > > > link https: > > > > //gi > > > > thub > > > > .com > > > > /altera-opensource/u-boot- > > > > socfpga/commits/socfpga_v2014.10_arria10_brin > > > > gup > > > > FogBugz #410989-6: Masking hardware sequenced warm > > > > reset > > > > for > > > > logic > > > > in… … > > > Does that work on ES2 ? I don't think so ... > > Why you think it doesn't work, using early IO or full > > rbf? > > The > > bitstream limitation? What you see from the print out? > ES2 can only use full RBF, I don't think this is handled > in > this > patchset at all. i did testing the full rbf loading, but in the end i removed that portion of codes because it stuck in DDR calibration due to MPFE HW issue. So, i would put back that portion of codes after MPFE HW workaround. My plan is to let early IO release up 1st.
Can you describe that workaround ? The code worked on the A10ES2 kit I have back around v2018.09, so what's the problem ?
There is a corruption to MPFE NoC(which function like ethernet QOS, but its main function for priotizing and controlling traffic to access DDR from MPU and FPGA) due to high fequency transient clock out from HPS EMIF IOPLL at VCO startup. The corruption happens intermittent on some boards. The workaround is to trigger warm reset to recover MPFE NoC from corruption after programing the periph rbf or full rbf. Once U- Boot reentrance after warm reset, only core rbf is allowed to configured FPGA.
And this is present in ES1 or ES2 ? Or is it also in PS ?
I have no idea would this present in ES1 or ES2, but this happened in some PS.
I wonder whether we want to scrap the ES support afterall.
We actually no longer support ES.
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 --- arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts | 18 + .../include/mach/fpga_manager_arria10.h | 30 ++- configs/socfpga_arria10_defconfig | 9 + drivers/fpga/Kconfig | 9 + drivers/fpga/socfpga_arria10.c | 417 +++++++++++++++++++- 5 files changed, 471 insertions(+), 12 deletions(-)
diff --git a/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts b/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts index 998d811..f4764d4 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_periph = "ghrd_10as066n2.periph.rbf.mkimage"; + altr,bitstream_core = "ghrd_10as066n2.core.rbf.mkimage"; +}; + &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..b48bc76 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,12 @@ /* SPDX-License-Identifier: GPL-2.0 */ /* - * Copyright (C) 2017 Intel Corporation <www.intel.com> + * Copyright (C) 2017-2018 Intel Corporation <www.intel.com> * All rights reserved. */
+#include <altera.h> +#include <image.h> + #ifndef _FPGA_MANAGER_ARRIA10_H_ #define _FPGA_MANAGER_ARRIA10_H_
@@ -51,6 +54,8 @@ #define ALT_FPGAMGR_IMGCFG_CTL_02_CFGWIDTH_SET_MSK BIT(24) #define ALT_FPGAMGR_IMGCFG_CTL_02_CDRATIO_LSB 16
+#define PERIPH_RBF 0 +#define CORE_RBF 1 #ifndef __ASSEMBLY__
struct socfpga_fpga_manager { @@ -88,12 +93,33 @@ 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; + u32 datacrc; + struct rbf_info rbfinfo; + struct image_header header; +}; + /* 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); +const char *get_fpga_filename(const void *fdt, int *len, u32 core); +void get_rbf_image_info(struct rbf_info *rbf, u16 *buffer); +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/configs/socfpga_arria10_defconfig b/configs/socfpga_arria10_defconfig index 6ebda81..f88910c 100644 --- a/configs/socfpga_arria10_defconfig +++ b/configs/socfpga_arria10_defconfig @@ -27,8 +27,17 @@ 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_FS_SUPPORT=y +CONFIG_SPL_EXT_SUPPORT=y +CONFIG_SPL_FAT_SUPPORT=y +CONFIG_SPL_DRIVERS_MISC_SUPPORT=y +CONFIG_FS_FAT_MAX_CLUSTSIZE=16384 +CONFIG_FS_LOADER=y CONFIG_FPGA_SOCFPGA=y CONFIG_DM_GPIO=y CONFIG_DWAPB_GPIO=y diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig index 50e9019..06a8204 100644 --- a/drivers/fpga/Kconfig +++ b/drivers/fpga/Kconfig @@ -21,6 +21,15 @@ config FPGA_SOCFPGA
This provides common functionality for Gen5 and Arria10 devices.
+config CHECK_FPGA_DATA_CRC + bool "Enable CRC cheking on Arria10 FPGA bistream" + depends on FPGA_SOCFPGA + help + Say Y here to enable the CRC checking on Arria 10 FPGA bitstream + + This provides CRC checking to ensure integrated of Arria 10 FPGA + bitstream is programmed into FPGA. + config FPGA_CYCLON2 bool "Enable Altera FPGA driver for Cyclone II" depends on FPGA_ALTERA diff --git a/drivers/fpga/socfpga_arria10.c b/drivers/fpga/socfpga_arria10.c index 114dd91..d9ad237 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-2018 Intel Corporation <www.intel.com> */
#include <asm/io.h> @@ -10,8 +10,10 @@ #include <asm/arch/sdram.h> #include <asm/arch/misc.h> #include <altera.h> +#include <asm/arch/pinmux.h> #include <common.h> #include <errno.h> +#include <fs_loader.h> #include <wait_bit.h> #include <watchdog.h>
@@ -21,6 +23,10 @@ #define COMPRESSION_OFFSET 229 #define FPGA_TIMEOUT_MSEC 1000 /* timeout in ms */ #define FPGA_TIMEOUT_CNT 0x1000000 +#define RBF_UNENCRYPTED 0xa65c +#define RBF_ENCRYPTED 0xa65d +#define ARRIA10RBF_PERIPH 0x0001 +#define ARRIA10RBF_CORE 0x8001
static const struct socfpga_fpga_manager *fpga_manager_base = (void *)SOCFPGA_FPGAMGRREGS_ADDRESS; @@ -64,7 +70,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,13 +453,393 @@ int fpgamgr_program_finish(void) return 0; }
+const char *get_fpga_filename(const void *fdt, int *len, u32 core) +{ + const char *fpga_filename = NULL; + const char *cell; + int nodeoffset; + + nodeoffset = fdtdec_next_compatible(fdt, 0, + COMPAT_ALTERA_SOCFPGA_FPGA0); + if (nodeoffset >= 0) { + if (core) { + cell = fdt_getprop(fdt, + nodeoffset, + "altr,bitstream_core", + len); + } else { + cell = fdt_getprop(fdt, nodeoffset, + "altr,bitstream_periph", len); + } + + if (cell) + fpga_filename = cell; + } + + return fpga_filename; +} + +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) == RBF_UNENCRYPTED) { + rbf->security = unencrypted; + } else if (*(buffer + i) == RBF_ENCRYPTED) { + rbf->security = encrypted; + } else if (*(buffer + i + 1) == RBF_UNENCRYPTED) { + rbf->security = unencrypted; + } else if (*(buffer + i + 1) == RBF_ENCRYPTED) { + rbf->security = encrypted; + } else { + rbf->security = invalid; + continue; + } + + /* PERIPH RBF(buffer + i + 1), CORE RBF(buffer + i + 2) */ + if (*(buffer + i + 1) == ARRIA10RBF_PERIPH) { + rbf->section = periph_section; + break; + } else if (*(buffer + i + 1) == ARRIA10RBF_CORE) { + rbf->section = core_section; + break; + } else if (*(buffer + i + 2) == ARRIA10RBF_PERIPH) { + rbf->section = periph_section; + break; + } else if (*(buffer + i + 2) == ARRIA10RBF_CORE) { + rbf->section = core_section; + break; + } + + rbf->section = unknown; + break; + + WATCHDOG_RESET(); + } +} + +static struct firmware *fw; + +int first_loading_rbf_to_buffer(struct device_platdata *plat, + struct fpga_loadfs_info *fpga_loadfs, + u32 *buffer, u32 *buffer_bsize) +{ + u32 *buffer_p_after_header = NULL; + u32 buffersz_after_header = 0; + u32 totalsz_header_rbf = 0; + u32 *buffer_p = (u32 *)*buffer; + struct image_header *header = &(fpga_loadfs->header); + size_t buffer_size = *buffer_bsize; + int ret = 0; + + /* Load mkimage header into buffer */ + ret = request_firmware_into_buf(plat, + fpga_loadfs->fpga_fsinfo->filename, + header, + sizeof(struct image_header), + fpga_loadfs->offset, + &fw); + if (ret < 0) { + debug("FPGA: Failed to read RBF mkimage header from flash.\n"); + return -ENOENT; + } + + WATCHDOG_RESET(); + + if (!image_check_magic(header)) { + debug("FPGA: Bad Magic Number.\n"); + return -EBADF; + } + + if (!image_check_hcrc(header)) { + debug("FPGA: Bad Header Checksum.\n"); + return -EPERM; + } + + /* Getting RBF data size from mkimage header */ + fpga_loadfs->remaining = image_get_data_size(header); + + /* Calculate total size of both RBF with mkimage header */ + totalsz_header_rbf = fpga_loadfs->remaining + + sizeof(struct image_header); + + /* + * Determine buffer size vs RBF size, and calculating number of + * chunk by chunk transfer is required due to smaller buffer size + * compare to RBF + */ + if (totalsz_header_rbf > buffer_size) { + /* Calculate size of RBF in the buffer */ + buffersz_after_header = + buffer_size - sizeof(struct image_header); + fpga_loadfs->remaining -= buffersz_after_header; + } else { + /* Loading whole RBF into buffer */ + buffer_size = totalsz_header_rbf; + /* Calculate size of RBF in the buffer */ + buffersz_after_header = + buffer_size - sizeof(struct image_header); + fpga_loadfs->remaining = 0; + } + + /* Loading mkimage header and RBFinto buffer */ + ret = request_firmware_into_buf(plat, + fpga_loadfs->fpga_fsinfo->filename, + buffer_p, + buffer_size, + fpga_loadfs->offset, + &fw); + if (ret < 0) { + debug("FPGA: Failed to read RBF mkimage header and RBF from "); + debug("flash.\n"); + return -ENOENT; + } + + /* + * Getting pointer of RBF starting address where it's + * right after mkimage header + */ + buffer_p_after_header = + (u32 *)((u_char *)buffer_p + sizeof(struct image_header)); + + /* Update next reading RBF offset */ + fpga_loadfs->offset += buffer_size; + + /* Getting info about RBF types */ + get_rbf_image_info(&fpga_loadfs->rbfinfo, (u16 *)buffer_p_after_header); + + /* + * Update the starting addr of RBF to init FPGA & programming RBF + * into FPGA + */ + *buffer = (u32)buffer_p_after_header; + + /* Update the size of RBF to be programmed into FPGA */ + *buffer_bsize = buffersz_after_header; + +#ifdef CONFIG_CHECK_FPGA_DATA_CRC + fpga_loadfs->datacrc = crc32(fpga_loadfs->datacrc, + (u_char *)buffer_p_after_header, + buffersz_after_header); +#endif + +if (fpga_loadfs->remaining == 0) { +#ifdef CONFIG_CHECK_FPGA_DATA_CRC + if (fpga_loadfs->datacrc != image_get_dcrc(&(fpga_loadfs->header))) { + debug("FPGA: Bad Data Checksum.\n"); + return -EPERM; + } +#endif +} + return 0; +} + +int subsequent_loading_rbf_to_buffer(struct device_platdata *plat, + struct fpga_loadfs_info *fpga_loadfs, + u32 *buffer, u32 *buffer_bsize) +{ + int ret = 0; + u32 *buffer_p = (u32 *)*buffer; + + /* Read the RBF 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(plat, + fpga_loadfs->fpga_fsinfo->filename, + buffer_p, + *buffer_bsize, + fpga_loadfs->offset, + &fw); + if (ret < 0) { + debug("FPGA: Failed to read RBF from flash.\n"); + return -ENOENT; + } + +#ifdef CONFIG_CHECK_FPGA_DATA_CRC + fpga_loadfs->datacrc = crc32(fpga_loadfs->datacrc, + (unsigned char *)buffer_p, + *buffer_bsize); +#endif + +if (fpga_loadfs->remaining == 0) { +#ifdef CONFIG_CHECK_FPGA_DATA_CRC + if (fpga_loadfs->datacrc != image_get_dcrc(&(fpga_loadfs->header))) { + debug("FPGA: Bad Data Checksum.\n"); + return -EPERM; + } +#endif +} + + /* Update next reading RBF 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; + u32 buffer_ori = (u32) buf; + size_t buffer_sizebytes = bsize; + size_t buffer_sizebytes_ori = bsize; + size_t total_sizeof_mkimage = sizeof(struct image_header); + 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("Start to program FPGA ...\n"); + + /* + * Note: Both buffer and buffer_sizebytes values can be altered by + * function below. + */ + ret = first_loading_rbf_to_buffer(dev->platdata, &fpga_loadfs, &buffer, + &buffer_sizebytes); + if (ret) { + release_firmware(fw); + fw = NULL; + return ret; + } + + if ((fpga_loadfs.rbfinfo.section == core_section) && + !(is_fpgamgr_early_user_mode() && !is_fpgamgr_user_mode())) { + debug("FPGA : FPGA must be in Early Release mode to program "); + debug("core.\n"); + release_firmware(fw); + fw = NULL; + 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 periph rbf failed.\n"); + release_firmware(fw); + fw = NULL; + return -EPERM; + } + } + + WATCHDOG_RESET(); + + /* Transfer RBF to FPGA Manager */ + fpgamgr_program_write((void *)buffer, buffer_sizebytes); + + total_sizeof_mkimage += buffer_sizebytes; + + WATCHDOG_RESET(); + + while (fpga_loadfs.remaining) { + ret = subsequent_loading_rbf_to_buffer(dev->platdata, + &fpga_loadfs, + &buffer_ori, + &buffer_sizebytes_ori); + + if (ret) { + release_firmware(fw); + fw = NULL; + return ret; + } + + /* Transfer data to FPGA Manager */ + fpgamgr_program_write((void *)buffer_ori, + buffer_sizebytes_ori); + + total_sizeof_mkimage += 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"); + release_firmware(fw); + fw = NULL; + return -EIO; + } + + /* For monolithic bitstream */ + if (is_fpgamgr_user_mode()) { + /* Ensure the FPGA entering config done */ + status = fpgamgr_program_finish(); + if (status) { + release_firmware(fw); + fw = NULL; + 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) { + release_firmware(fw); + fw = NULL; + return status; + } + + config_pins(gd->fdt_blob, "fpga"); + puts("FPGA: Enter user mode.\n"); + } else { + debug("Config Error: Unsupported FGPA RBF type.\n"); + release_firmware(fw); + fw = NULL; + return -ENOEXEC; + } + + release_firmware(fw); + fw = NULL; + + return (int)total_sizeof_mkimage; +} + /* - * FPGA Manager to program the FPGA. This is the interface used by FPGA driver. - * Return 0 for sucess, non-zero for error. + * This function is used to load the core RBF from the OCRAM where the location + * of the image is defined in the load property in the FIT image source file + * (.its) */ 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 */ writel(0, &system_manager_base->fpgaintf_en_global); @@ -461,13 +847,24 @@ int socfpga_load(Altera_desc *desc, const void *rbf_data, size_t rbf_size) /* 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 RBF 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; + }
/* Write the RBF data 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 11/21/2018 11:41 AM, 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
[...]
@@ -51,6 +54,8 @@ #define ALT_FPGAMGR_IMGCFG_CTL_02_CFGWIDTH_SET_MSK BIT(24) #define ALT_FPGAMGR_IMGCFG_CTL_02_CDRATIO_LSB 16
+#define PERIPH_RBF 0 +#define CORE_RBF 1
Enum, use something with specific prefix.
#ifndef __ASSEMBLY__
struct socfpga_fpga_manager { @@ -88,12 +93,33 @@ struct socfpga_fpga_manager { u32 imgcfg_fifo_status; };
+enum rbf_type {unknown, periph_section, core_section}; +enum rbf_security {invalid, unencrypted, encrypted};
enum should use one line per entry.
+struct rbf_info {
- enum rbf_type section;
- enum rbf_security security;
+};
+struct fpga_loadfs_info {
- fpga_fs_info *fpga_fsinfo;
- u32 remaining;
- u32 offset;
- u32 datacrc;
- struct rbf_info rbfinfo;
- struct image_header header;
+};
/* 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); +const char *get_fpga_filename(const void *fdt, int *len, u32 core); +void get_rbf_image_info(struct rbf_info *rbf, u16 *buffer); +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/configs/socfpga_arria10_defconfig b/configs/socfpga_arria10_defconfig index 6ebda81..f88910c 100644 --- a/configs/socfpga_arria10_defconfig +++ b/configs/socfpga_arria10_defconfig @@ -27,8 +27,17 @@ 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_FS_SUPPORT=y +CONFIG_SPL_EXT_SUPPORT=y +CONFIG_SPL_FAT_SUPPORT=y +CONFIG_SPL_DRIVERS_MISC_SUPPORT=y +CONFIG_FS_FAT_MAX_CLUSTSIZE=16384 +CONFIG_FS_LOADER=y
Separate patch
CONFIG_FPGA_SOCFPGA=y CONFIG_DM_GPIO=y CONFIG_DWAPB_GPIO=y diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig index 50e9019..06a8204 100644 --- a/drivers/fpga/Kconfig +++ b/drivers/fpga/Kconfig @@ -21,6 +21,15 @@ config FPGA_SOCFPGA
This provides common functionality for Gen5 and Arria10 devices.+config CHECK_FPGA_DATA_CRC
config FPGA_SOCFPGA_A10_CRC_CHECK
What is this for and why shouldn't this be ON by default ?
- bool "Enable CRC cheking on Arria10 FPGA bistream"
- depends on FPGA_SOCFPGA
- help
Say Y here to enable the CRC checking on Arria 10 FPGA bitstreamThis provides CRC checking to ensure integrated of Arria 10 FPGAbitstream is programmed into FPGA.config FPGA_CYCLON2 bool "Enable Altera FPGA driver for Cyclone II" depends on FPGA_ALTERA diff --git a/drivers/fpga/socfpga_arria10.c b/drivers/fpga/socfpga_arria10.c index 114dd91..d9ad237 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-2018 Intel Corporation <www.intel.com>
#include <asm/io.h> @@ -10,8 +10,10 @@ #include <asm/arch/sdram.h> #include <asm/arch/misc.h> #include <altera.h> +#include <asm/arch/pinmux.h> #include <common.h> #include <errno.h> +#include <fs_loader.h> #include <wait_bit.h> #include <watchdog.h>
@@ -21,6 +23,10 @@ #define COMPRESSION_OFFSET 229 #define FPGA_TIMEOUT_MSEC 1000 /* timeout in ms */ #define FPGA_TIMEOUT_CNT 0x1000000 +#define RBF_UNENCRYPTED 0xa65c +#define RBF_ENCRYPTED 0xa65d +#define ARRIA10RBF_PERIPH 0x0001 +#define ARRIA10RBF_CORE 0x8001
This looks awfully similar to the PERIPH_RBF and CORE_RBF above.
static const struct socfpga_fpga_manager *fpga_manager_base = (void *)SOCFPGA_FPGAMGRREGS_ADDRESS; @@ -64,7 +70,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,13 +453,393 @@ int fpgamgr_program_finish(void) return 0; }
+const char *get_fpga_filename(const void *fdt, int *len, u32 core)
static, fix globally .
+{
- const char *fpga_filename = NULL;
- const char *cell;
- int nodeoffset;
ofnode_read_string() , use ofnode globally.
- nodeoffset = fdtdec_next_compatible(fdt, 0,
COMPAT_ALTERA_SOCFPGA_FPGA0);- if (nodeoffset >= 0) {
if (core) {cell = fdt_getprop(fdt,nodeoffset,"altr,bitstream_core",len);} else {cell = fdt_getprop(fdt, nodeoffset,"altr,bitstream_periph", len);}if (cell)fpga_filename = cell;- }
- return fpga_filename;
+}
+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) == RBF_UNENCRYPTED) {rbf->security = unencrypted;} else if (*(buffer + i) == RBF_ENCRYPTED) {rbf->security = encrypted;} else if (*(buffer + i + 1) == RBF_UNENCRYPTED) {rbf->security = unencrypted;} else if (*(buffer + i + 1) == RBF_ENCRYPTED) {rbf->security = encrypted;} else {rbf->security = invalid;continue;}/* PERIPH RBF(buffer + i + 1), CORE RBF(buffer + i + 2) */if (*(buffer + i + 1) == ARRIA10RBF_PERIPH) {rbf->section = periph_section;break;} else if (*(buffer + i + 1) == ARRIA10RBF_CORE) {rbf->section = core_section;break;} else if (*(buffer + i + 2) == ARRIA10RBF_PERIPH) {rbf->section = periph_section;break;} else if (*(buffer + i + 2) == ARRIA10RBF_CORE) {rbf->section = core_section;break;}rbf->section = unknown;break;WATCHDOG_RESET();- }
+}
+static struct firmware *fw;
What is this static variable doing here ?
+int first_loading_rbf_to_buffer(struct device_platdata *plat,
struct fpga_loadfs_info *fpga_loadfs,u32 *buffer, u32 *buffer_bsize)+{
- u32 *buffer_p_after_header = NULL;
- u32 buffersz_after_header = 0;
- u32 totalsz_header_rbf = 0;
- u32 *buffer_p = (u32 *)*buffer;
- struct image_header *header = &(fpga_loadfs->header);
- size_t buffer_size = *buffer_bsize;
- int ret = 0;
- /* Load mkimage header into buffer */
- ret = request_firmware_into_buf(plat,
fpga_loadfs->fpga_fsinfo->filename,header,sizeof(struct image_header),fpga_loadfs->offset,&fw);- if (ret < 0) {
debug("FPGA: Failed to read RBF mkimage header from flash.\n");return -ENOENT;- }
- WATCHDOG_RESET();
- if (!image_check_magic(header)) {
debug("FPGA: Bad Magic Number.\n");return -EBADF;- }
- if (!image_check_hcrc(header)) {
debug("FPGA: Bad Header Checksum.\n");return -EPERM;- }
- /* Getting RBF data size from mkimage header */
- fpga_loadfs->remaining = image_get_data_size(header);
- /* Calculate total size of both RBF with mkimage header */
- totalsz_header_rbf = fpga_loadfs->remaining +
sizeof(struct image_header);- /*
* Determine buffer size vs RBF size, and calculating number of* chunk by chunk transfer is required due to smaller buffer size* compare to RBF*/- if (totalsz_header_rbf > buffer_size) {
/* Calculate size of RBF in the buffer */buffersz_after_header =buffer_size - sizeof(struct image_header);fpga_loadfs->remaining -= buffersz_after_header;- } else {
/* Loading whole RBF into buffer */buffer_size = totalsz_header_rbf;/* Calculate size of RBF in the buffer */buffersz_after_header =buffer_size - sizeof(struct image_header);fpga_loadfs->remaining = 0;- }
- /* Loading mkimage header and RBFinto buffer */
- ret = request_firmware_into_buf(plat,
fpga_loadfs->fpga_fsinfo->filename,buffer_p,buffer_size,fpga_loadfs->offset,&fw);
This looks like some unwound loop , since the request_firmware_into_buf() is called twice. This probably works for the 300 kiB core RBF, but doesn't work for the 16 MiB full RBF. Also, for() cycle should be used somehow.
- if (ret < 0) {
debug("FPGA: Failed to read RBF mkimage header and RBF from ");debug("flash.\n");return -ENOENT;- }
- /*
* Getting pointer of RBF starting address where it's* right after mkimage header*/- buffer_p_after_header =
(u32 *)((u_char *)buffer_p + sizeof(struct image_header));- /* Update next reading RBF offset */
- fpga_loadfs->offset += buffer_size;
- /* Getting info about RBF types */
- get_rbf_image_info(&fpga_loadfs->rbfinfo, (u16 *)buffer_p_after_header);
- /*
* Update the starting addr of RBF to init FPGA & programming RBF* into FPGA*/- *buffer = (u32)buffer_p_after_header;
- /* Update the size of RBF to be programmed into FPGA */
- *buffer_bsize = buffersz_after_header;
+#ifdef CONFIG_CHECK_FPGA_DATA_CRC
- fpga_loadfs->datacrc = crc32(fpga_loadfs->datacrc,
(u_char *)buffer_p_after_header,buffersz_after_header);
Why is this not ON by default ?
+#endif
+if (fpga_loadfs->remaining == 0) { +#ifdef CONFIG_CHECK_FPGA_DATA_CRC
- if (fpga_loadfs->datacrc != image_get_dcrc(&(fpga_loadfs->header))) {
debug("FPGA: Bad Data Checksum.\n");return -EPERM;- }
+#endif +}
- return 0;
+}
[...]
On Wed, 2018-11-21 at 15:18 +0100, Marek Vasut wrote:
On 11/21/2018 11:41 AM, 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
[...]
@@ -51,6 +54,8 @@ #define ALT_FPGAMGR_IMGCFG_CTL_02_CFGWIDTH_SET_MSK BIT(24) #define ALT_FPGAMGR_IMGCFG_CTL_02_CDRATIO_LSB 16 +#define PERIPH_RBF 0 +#define CORE_RBF 1
Enum, use something with specific prefix.
Noted.
#ifndef __ASSEMBLY__ struct socfpga_fpga_manager { @@ -88,12 +93,33 @@ struct socfpga_fpga_manager { u32 imgcfg_fifo_status; }; +enum rbf_type {unknown, periph_section, core_section}; +enum rbf_security {invalid, unencrypted, encrypted};
enum should use one line per entry.
Noted.
+struct rbf_info {
- enum rbf_type section;
- enum rbf_security security;
+};
+struct fpga_loadfs_info {
- fpga_fs_info *fpga_fsinfo;
- u32 remaining;
- u32 offset;
- u32 datacrc;
- struct rbf_info rbfinfo;
- struct image_header header;
+};
/* 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); +const char *get_fpga_filename(const void *fdt, int *len, u32 core); +void get_rbf_image_info(struct rbf_info *rbf, u16 *buffer); +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/configs/socfpga_arria10_defconfig b/configs/socfpga_arria10_defconfig index 6ebda81..f88910c 100644 --- a/configs/socfpga_arria10_defconfig +++ b/configs/socfpga_arria10_defconfig @@ -27,8 +27,17 @@ 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_FS_SUPPORT=y +CONFIG_SPL_EXT_SUPPORT=y +CONFIG_SPL_FAT_SUPPORT=y +CONFIG_SPL_DRIVERS_MISC_SUPPORT=y +CONFIG_FS_FAT_MAX_CLUSTSIZE=16384 +CONFIG_FS_LOADER=y
Separate patch
Okay.
CONFIG_FPGA_SOCFPGA=y CONFIG_DM_GPIO=y CONFIG_DWAPB_GPIO=y diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig index 50e9019..06a8204 100644 --- a/drivers/fpga/Kconfig +++ b/drivers/fpga/Kconfig @@ -21,6 +21,15 @@ config FPGA_SOCFPGA This provides common functionality for Gen5 and Arria10 devices. +config CHECK_FPGA_DATA_CRC
config FPGA_SOCFPGA_A10_CRC_CHECK
What is this for and why shouldn't this be ON by default ?
Both periph.rbf or full.rbf are wrapped with mkimage. So, this CRC checking can be used to check the integrity of the loading bitstream data against checksum from mkimage. It is off for the sake of performance.
- bool "Enable CRC cheking on Arria10 FPGA bistream"
- depends on FPGA_SOCFPGA
- help
- Say Y here to enable the CRC checking on Arria 10 FPGA
bitstream
- This provides CRC checking to ensure integrated of Arria
10 FPGA
- bitstream is programmed into FPGA.
config FPGA_CYCLON2 bool "Enable Altera FPGA driver for Cyclone II" depends on FPGA_ALTERA diff --git a/drivers/fpga/socfpga_arria10.c b/drivers/fpga/socfpga_arria10.c index 114dd91..d9ad237 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-2018 Intel Corporation <www.intel.com>
*/ #include <asm/io.h> @@ -10,8 +10,10 @@ #include <asm/arch/sdram.h> #include <asm/arch/misc.h> #include <altera.h> +#include <asm/arch/pinmux.h> #include <common.h> #include <errno.h> +#include <fs_loader.h> #include <wait_bit.h> #include <watchdog.h> @@ -21,6 +23,10 @@ #define COMPRESSION_OFFSET 229 #define FPGA_TIMEOUT_MSEC 1000 /* timeout in ms */ #define FPGA_TIMEOUT_CNT 0x1000000 +#define RBF_UNENCRYPTED 0xa65c +#define RBF_ENCRYPTED 0xa65d +#define ARRIA10RBF_PERIPH 0x0001 +#define ARRIA10RBF_CORE 0x8001
This looks awfully similar to the PERIPH_RBF and CORE_RBF above.
PERIPH_RBF and CORE_RBF are the flags, so i can change them to enum. But above #define are magic content used to identify the bistream type. If above #define are not suitable, what can you suggest?
static const struct socfpga_fpga_manager *fpga_manager_base = (void *)SOCFPGA_FPGAMGRREGS_ADDRESS; @@ -64,7 +70,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,13 +453,393 @@ int fpgamgr_program_finish(void) return 0; } +const char *get_fpga_filename(const void *fdt, int *len, u32 core)
static, fix globally .
+{
- const char *fpga_filename = NULL;
- const char *cell;
- int nodeoffset;
ofnode_read_string() , use ofnode globally.
Noted.
- nodeoffset = fdtdec_next_compatible(fdt, 0,
COMPAT_ALTERA_SOCFPGA_FPGA0);- if (nodeoffset >= 0) {
if (core) {cell = fdt_getprop(fdt,nodeoffset,"altr,bitstream_core",len);} else {cell = fdt_getprop(fdt, nodeoffset,"altr,bitstream_periph",len);
}if (cell)fpga_filename = cell;- }
- return fpga_filename;
+}
+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) == RBF_UNENCRYPTED) {rbf->security = unencrypted;} else if (*(buffer + i) == RBF_ENCRYPTED) {rbf->security = encrypted;} else if (*(buffer + i + 1) == RBF_UNENCRYPTED) {rbf->security = unencrypted;} else if (*(buffer + i + 1) == RBF_ENCRYPTED) {rbf->security = encrypted;} else {rbf->security = invalid;continue;}/* PERIPH RBF(buffer + i + 1), CORE RBF(buffer + i
- */
if (*(buffer + i + 1) == ARRIA10RBF_PERIPH) {rbf->section = periph_section;break;} else if (*(buffer + i + 1) == ARRIA10RBF_CORE) {rbf->section = core_section;break;} else if (*(buffer + i + 2) == ARRIA10RBF_PERIPH){
rbf->section = periph_section;break;} else if (*(buffer + i + 2) == ARRIA10RBF_CORE) {rbf->section = core_section;break;}rbf->section = unknown;break;WATCHDOG_RESET();- }
+}
+static struct firmware *fw;
What is this static variable doing here ?
A place for storing firmware and its attribute data. This would be shared across a few helper functions which contain firmware loader.
+int first_loading_rbf_to_buffer(struct device_platdata *plat,
struct fpga_loadfs_info*fpga_loadfs,
u32 *buffer, u32 *buffer_bsize)+{
- u32 *buffer_p_after_header = NULL;
- u32 buffersz_after_header = 0;
- u32 totalsz_header_rbf = 0;
- u32 *buffer_p = (u32 *)*buffer;
- struct image_header *header = &(fpga_loadfs->header);
- size_t buffer_size = *buffer_bsize;
- int ret = 0;
- /* Load mkimage header into buffer */
- ret = request_firmware_into_buf(plat,
fpga_loadfs->fpga_fsinfo-filename,
header,sizeof(structimage_header),
fpga_loadfs->offset,&fw);- if (ret < 0) {
debug("FPGA: Failed to read RBF mkimage headerfrom flash.\n");
return -ENOENT;- }
- WATCHDOG_RESET();
- if (!image_check_magic(header)) {
debug("FPGA: Bad Magic Number.\n");return -EBADF;- }
- if (!image_check_hcrc(header)) {
debug("FPGA: Bad Header Checksum.\n");return -EPERM;- }
- /* Getting RBF data size from mkimage header */
- fpga_loadfs->remaining = image_get_data_size(header);
- /* Calculate total size of both RBF with mkimage header */
- totalsz_header_rbf = fpga_loadfs->remaining +
sizeof(struct image_header);- /*
- * Determine buffer size vs RBF size, and calculating
number of
- * chunk by chunk transfer is required due to smaller
buffer size
- * compare to RBF
- */
- if (totalsz_header_rbf > buffer_size) {
/* Calculate size of RBF in the buffer */buffersz_after_header =buffer_size - sizeof(struct image_header);fpga_loadfs->remaining -= buffersz_after_header;- } else {
/* Loading whole RBF into buffer */buffer_size = totalsz_header_rbf;/* Calculate size of RBF in the buffer */buffersz_after_header =buffer_size - sizeof(struct image_header);fpga_loadfs->remaining = 0;- }
- /* Loading mkimage header and RBFinto buffer */
- ret = request_firmware_into_buf(plat,
fpga_loadfs->fpga_fsinfo-filename,
buffer_p,buffer_size,fpga_loadfs->offset,&fw);This looks like some unwound loop , since the request_firmware_into_buf() is called twice. This probably works for the 300 kiB core RBF, but doesn't work for the 16 MiB full RBF. Also, for() cycle should be used somehow.
This function is mainly for checking the mkimage header, searching the location of the bitstream image from the 1st chunk of reading data and updating the read location. The remaining of the bitstream data after 1st chunk would be processed by the function called subsequent_loading_rbf_to_buffer().
- if (ret < 0) {
debug("FPGA: Failed to read RBF mkimage header andRBF from ");
debug("flash.\n");return -ENOENT;- }
- /*
- * Getting pointer of RBF starting address where it's
- * right after mkimage header
- */
- buffer_p_after_header =
(u32 *)((u_char *)buffer_p + sizeof(structimage_header));
- /* Update next reading RBF offset */
- fpga_loadfs->offset += buffer_size;
- /* Getting info about RBF types */
- get_rbf_image_info(&fpga_loadfs->rbfinfo, (u16
*)buffer_p_after_header);
- /*
- * Update the starting addr of RBF to init FPGA &
programming RBF
- * into FPGA
- */
- *buffer = (u32)buffer_p_after_header;
- /* Update the size of RBF to be programmed into FPGA */
- *buffer_bsize = buffersz_after_header;
+#ifdef CONFIG_CHECK_FPGA_DATA_CRC
- fpga_loadfs->datacrc = crc32(fpga_loadfs->datacrc,
(u_char*)buffer_p_after_header,
buffersz_after_header);Why is this not ON by default ?
It is off for the sake of performance.
+#endif
+if (fpga_loadfs->remaining == 0) { +#ifdef CONFIG_CHECK_FPGA_DATA_CRC
- if (fpga_loadfs->datacrc != image_get_dcrc(&(fpga_loadfs-
header))) {
debug("FPGA: Bad Data Checksum.\n");return -EPERM;- }
+#endif +}
- return 0;
+}
[...]
On 11/23/2018 10:43 AM, Chee, Tien Fong wrote:
On Wed, 2018-11-21 at 15:18 +0100, Marek Vasut wrote:
On 11/21/2018 11:41 AM, 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
[...]
@@ -51,6 +54,8 @@ #define ALT_FPGAMGR_IMGCFG_CTL_02_CFGWIDTH_SET_MSK BIT(24) #define ALT_FPGAMGR_IMGCFG_CTL_02_CDRATIO_LSB 16 +#define PERIPH_RBF 0 +#define CORE_RBF 1
Enum, use something with specific prefix.
Noted.
#ifndef __ASSEMBLY__ struct socfpga_fpga_manager { @@ -88,12 +93,33 @@ struct socfpga_fpga_manager { u32 imgcfg_fifo_status; }; +enum rbf_type {unknown, periph_section, core_section}; +enum rbf_security {invalid, unencrypted, encrypted};
enum should use one line per entry.
Noted.
+struct rbf_info {
- enum rbf_type section;
- enum rbf_security security;
+};
+struct fpga_loadfs_info {
- fpga_fs_info *fpga_fsinfo;
- u32 remaining;
- u32 offset;
- u32 datacrc;
- struct rbf_info rbfinfo;
- struct image_header header;
+};
/* 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); +const char *get_fpga_filename(const void *fdt, int *len, u32 core); +void get_rbf_image_info(struct rbf_info *rbf, u16 *buffer); +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/configs/socfpga_arria10_defconfig b/configs/socfpga_arria10_defconfig index 6ebda81..f88910c 100644 --- a/configs/socfpga_arria10_defconfig +++ b/configs/socfpga_arria10_defconfig @@ -27,8 +27,17 @@ 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_FS_SUPPORT=y +CONFIG_SPL_EXT_SUPPORT=y +CONFIG_SPL_FAT_SUPPORT=y +CONFIG_SPL_DRIVERS_MISC_SUPPORT=y +CONFIG_FS_FAT_MAX_CLUSTSIZE=16384 +CONFIG_FS_LOADER=y
Separate patch
Okay.
CONFIG_FPGA_SOCFPGA=y CONFIG_DM_GPIO=y CONFIG_DWAPB_GPIO=y diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig index 50e9019..06a8204 100644 --- a/drivers/fpga/Kconfig +++ b/drivers/fpga/Kconfig @@ -21,6 +21,15 @@ config FPGA_SOCFPGA This provides common functionality for Gen5 and Arria10 devices. +config CHECK_FPGA_DATA_CRC
config FPGA_SOCFPGA_A10_CRC_CHECK
What is this for and why shouldn't this be ON by default ?
Both periph.rbf or full.rbf are wrapped with mkimage. So, this CRC checking can be used to check the integrity of the loading bitstream data against checksum from mkimage. It is off for the sake of performance.
Is there a measurable performance degradation ? I presume that's because caches are disabled at that point, yes? Enable caches and see if that helps.
- bool "Enable CRC cheking on Arria10 FPGA bistream"
- depends on FPGA_SOCFPGA
- help
- Say Y here to enable the CRC checking on Arria 10 FPGA
bitstream
- This provides CRC checking to ensure integrated of Arria
10 FPGA
- bitstream is programmed into FPGA.
config FPGA_CYCLON2 bool "Enable Altera FPGA driver for Cyclone II" depends on FPGA_ALTERA diff --git a/drivers/fpga/socfpga_arria10.c b/drivers/fpga/socfpga_arria10.c index 114dd91..d9ad237 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-2018 Intel Corporation <www.intel.com>
*/ #include <asm/io.h> @@ -10,8 +10,10 @@ #include <asm/arch/sdram.h> #include <asm/arch/misc.h> #include <altera.h> +#include <asm/arch/pinmux.h> #include <common.h> #include <errno.h> +#include <fs_loader.h> #include <wait_bit.h> #include <watchdog.h> @@ -21,6 +23,10 @@ #define COMPRESSION_OFFSET 229 #define FPGA_TIMEOUT_MSEC 1000 /* timeout in ms */ #define FPGA_TIMEOUT_CNT 0x1000000 +#define RBF_UNENCRYPTED 0xa65c +#define RBF_ENCRYPTED 0xa65d +#define ARRIA10RBF_PERIPH 0x0001 +#define ARRIA10RBF_CORE 0x8001
This looks awfully similar to the PERIPH_RBF and CORE_RBF above.
PERIPH_RBF and CORE_RBF are the flags, so i can change them to enum. But above #define are magic content used to identify the bistream type. If above #define are not suitable, what can you suggest?
Maybe you can just align those two to avoid duplication ?
static const struct socfpga_fpga_manager *fpga_manager_base = (void *)SOCFPGA_FPGAMGRREGS_ADDRESS; @@ -64,7 +70,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,13 +453,393 @@ int fpgamgr_program_finish(void) return 0; } +const char *get_fpga_filename(const void *fdt, int *len, u32 core)
static, fix globally .
+{
- const char *fpga_filename = NULL;
- const char *cell;
- int nodeoffset;
ofnode_read_string() , use ofnode globally.
Noted.
- nodeoffset = fdtdec_next_compatible(fdt, 0,
COMPAT_ALTERA_SOCFPGA_FPGA0);- if (nodeoffset >= 0) {
if (core) {cell = fdt_getprop(fdt,nodeoffset,"altr,bitstream_core",len);} else {cell = fdt_getprop(fdt, nodeoffset,"altr,bitstream_periph",len);
}if (cell)fpga_filename = cell;- }
- return fpga_filename;
+}
+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) == RBF_UNENCRYPTED) {rbf->security = unencrypted;} else if (*(buffer + i) == RBF_ENCRYPTED) {rbf->security = encrypted;} else if (*(buffer + i + 1) == RBF_UNENCRYPTED) {rbf->security = unencrypted;} else if (*(buffer + i + 1) == RBF_ENCRYPTED) {rbf->security = encrypted;} else {rbf->security = invalid;continue;}/* PERIPH RBF(buffer + i + 1), CORE RBF(buffer + i
- */
if (*(buffer + i + 1) == ARRIA10RBF_PERIPH) {rbf->section = periph_section;break;} else if (*(buffer + i + 1) == ARRIA10RBF_CORE) {rbf->section = core_section;break;} else if (*(buffer + i + 2) == ARRIA10RBF_PERIPH){
rbf->section = periph_section;break;} else if (*(buffer + i + 2) == ARRIA10RBF_CORE) {rbf->section = core_section;break;}rbf->section = unknown;break;WATCHDOG_RESET();- }
+}
+static struct firmware *fw;
What is this static variable doing here ?
A place for storing firmware and its attribute data. This would be shared across a few helper functions which contain firmware loader.
Why is it not in the device data instead ? If you had multiple FPGAs in the system, this would likely be randomly overwritten . Such static variables shouldn't be needed in DM/DT capable system.
+int first_loading_rbf_to_buffer(struct device_platdata *plat,
struct fpga_loadfs_info*fpga_loadfs,
u32 *buffer, u32 *buffer_bsize)+{
- u32 *buffer_p_after_header = NULL;
- u32 buffersz_after_header = 0;
- u32 totalsz_header_rbf = 0;
- u32 *buffer_p = (u32 *)*buffer;
- struct image_header *header = &(fpga_loadfs->header);
- size_t buffer_size = *buffer_bsize;
- int ret = 0;
- /* Load mkimage header into buffer */
- ret = request_firmware_into_buf(plat,
fpga_loadfs->fpga_fsinfo-filename,
header,sizeof(structimage_header),
fpga_loadfs->offset,&fw);- if (ret < 0) {
debug("FPGA: Failed to read RBF mkimage headerfrom flash.\n");
return -ENOENT;- }
- WATCHDOG_RESET();
- if (!image_check_magic(header)) {
debug("FPGA: Bad Magic Number.\n");return -EBADF;- }
- if (!image_check_hcrc(header)) {
debug("FPGA: Bad Header Checksum.\n");return -EPERM;- }
- /* Getting RBF data size from mkimage header */
- fpga_loadfs->remaining = image_get_data_size(header);
- /* Calculate total size of both RBF with mkimage header */
- totalsz_header_rbf = fpga_loadfs->remaining +
sizeof(struct image_header);- /*
- * Determine buffer size vs RBF size, and calculating
number of
- * chunk by chunk transfer is required due to smaller
buffer size
- * compare to RBF
- */
- if (totalsz_header_rbf > buffer_size) {
/* Calculate size of RBF in the buffer */buffersz_after_header =buffer_size - sizeof(struct image_header);fpga_loadfs->remaining -= buffersz_after_header;- } else {
/* Loading whole RBF into buffer */buffer_size = totalsz_header_rbf;/* Calculate size of RBF in the buffer */buffersz_after_header =buffer_size - sizeof(struct image_header);fpga_loadfs->remaining = 0;- }
- /* Loading mkimage header and RBFinto buffer */
- ret = request_firmware_into_buf(plat,
fpga_loadfs->fpga_fsinfo-filename,
buffer_p,buffer_size,fpga_loadfs->offset,&fw);This looks like some unwound loop , since the request_firmware_into_buf() is called twice. This probably works for the 300 kiB core RBF, but doesn't work for the 16 MiB full RBF. Also, for() cycle should be used somehow.
This function is mainly for checking the mkimage header, searching the location of the bitstream image from the 1st chunk of reading data and updating the read location. The remaining of the bitstream data after 1st chunk would be processed by the function called subsequent_loading_rbf_to_buffer().
I wonder if this can be somehow optimized, so it's not such a long function with repeated pieces of code.
- if (ret < 0) {
debug("FPGA: Failed to read RBF mkimage header andRBF from ");
debug("flash.\n");return -ENOENT;- }
- /*
- * Getting pointer of RBF starting address where it's
- * right after mkimage header
- */
- buffer_p_after_header =
(u32 *)((u_char *)buffer_p + sizeof(structimage_header));
- /* Update next reading RBF offset */
- fpga_loadfs->offset += buffer_size;
- /* Getting info about RBF types */
- get_rbf_image_info(&fpga_loadfs->rbfinfo, (u16
*)buffer_p_after_header);
- /*
- * Update the starting addr of RBF to init FPGA &
programming RBF
- * into FPGA
- */
- *buffer = (u32)buffer_p_after_header;
- /* Update the size of RBF to be programmed into FPGA */
- *buffer_bsize = buffersz_after_header;
+#ifdef CONFIG_CHECK_FPGA_DATA_CRC
- fpga_loadfs->datacrc = crc32(fpga_loadfs->datacrc,
(u_char*)buffer_p_after_header,
buffersz_after_header);Why is this not ON by default ?
It is off for the sake of performance.
Do you have some hard numbers to support this claim ?
+#endif
+if (fpga_loadfs->remaining == 0) { +#ifdef CONFIG_CHECK_FPGA_DATA_CRC
- if (fpga_loadfs->datacrc != image_get_dcrc(&(fpga_loadfs-
header))) {
debug("FPGA: Bad Data Checksum.\n");return -EPERM;- }
+#endif +}
- return 0;
+}
[...]
On Fri, 2018-11-23 at 13:28 +0100, Marek Vasut wrote:
On 11/23/2018 10:43 AM, Chee, Tien Fong wrote:
On Wed, 2018-11-21 at 15:18 +0100, Marek Vasut wrote:
On 11/21/2018 11:41 AM, 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
[...]
@@ -51,6 +54,8 @@ #define ALT_FPGAMGR_IMGCFG_CTL_02_CFGWIDTH_SET_MSK BIT(24) #define ALT_FPGAMGR_IMGCFG_CTL_02_CDRATIO_LSB 16 +#define PERIPH_RBF 0 +#define CORE_RBF 1
Enum, use something with specific prefix.
Noted.
#ifndef __ASSEMBLY__ struct socfpga_fpga_manager { @@ -88,12 +93,33 @@ struct socfpga_fpga_manager { u32 imgcfg_fifo_status; }; +enum rbf_type {unknown, periph_section, core_section}; +enum rbf_security {invalid, unencrypted, encrypted};
enum should use one line per entry.
Noted.
+struct rbf_info {
- enum rbf_type section;
- enum rbf_security security;
+};
+struct fpga_loadfs_info {
- fpga_fs_info *fpga_fsinfo;
- u32 remaining;
- u32 offset;
- u32 datacrc;
- struct rbf_info rbfinfo;
- struct image_header header;
+};
/* 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); +const char *get_fpga_filename(const void *fdt, int *len, u32 core); +void get_rbf_image_info(struct rbf_info *rbf, u16 *buffer); +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/configs/socfpga_arria10_defconfig b/configs/socfpga_arria10_defconfig index 6ebda81..f88910c 100644 --- a/configs/socfpga_arria10_defconfig +++ b/configs/socfpga_arria10_defconfig @@ -27,8 +27,17 @@ 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_FS_SUPPORT=y +CONFIG_SPL_EXT_SUPPORT=y +CONFIG_SPL_FAT_SUPPORT=y +CONFIG_SPL_DRIVERS_MISC_SUPPORT=y +CONFIG_FS_FAT_MAX_CLUSTSIZE=16384 +CONFIG_FS_LOADER=y
Separate patch
Okay.
CONFIG_FPGA_SOCFPGA=y CONFIG_DM_GPIO=y CONFIG_DWAPB_GPIO=y diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig index 50e9019..06a8204 100644 --- a/drivers/fpga/Kconfig +++ b/drivers/fpga/Kconfig @@ -21,6 +21,15 @@ config FPGA_SOCFPGA This provides common functionality for Gen5 and Arria10 devices. +config CHECK_FPGA_DATA_CRC
config FPGA_SOCFPGA_A10_CRC_CHECK
What is this for and why shouldn't this be ON by default ?
Both periph.rbf or full.rbf are wrapped with mkimage. So, this CRC checking can be used to check the integrity of the loading bitstream data against checksum from mkimage. It is off for the sake of performance.
Is there a measurable performance degradation ? I presume that's because caches are disabled at that point, yes? Enable caches and see if that helps.
Just logical sense, performance sure getting degraded, especially loading full rbf, but may be not that obvious for periph.rbf because of very small size, i can try to measure. If not much difference, i can enable checking in default.
- bool "Enable CRC cheking on Arria10 FPGA bistream"
- depends on FPGA_SOCFPGA
- help
- Say Y here to enable the CRC checking on Arria 10
FPGA bitstream
- This provides CRC checking to ensure integrated of
Arria 10 FPGA
- bitstream is programmed into FPGA.
config FPGA_CYCLON2 bool "Enable Altera FPGA driver for Cyclone II" depends on FPGA_ALTERA diff --git a/drivers/fpga/socfpga_arria10.c b/drivers/fpga/socfpga_arria10.c index 114dd91..d9ad237 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-2018 Intel Corporation <www.intel.com>
*/ #include <asm/io.h> @@ -10,8 +10,10 @@ #include <asm/arch/sdram.h> #include <asm/arch/misc.h> #include <altera.h> +#include <asm/arch/pinmux.h> #include <common.h> #include <errno.h> +#include <fs_loader.h> #include <wait_bit.h> #include <watchdog.h> @@ -21,6 +23,10 @@ #define COMPRESSION_OFFSET 229 #define FPGA_TIMEOUT_MSEC 1000 /* timeout in ms */ #define FPGA_TIMEOUT_CNT 0x1000000 +#define RBF_UNENCRYPTED 0xa65c +#define RBF_ENCRYPTED 0xa65d +#define ARRIA10RBF_PERIPH 0x0001 +#define ARRIA10RBF_CORE 0x8001
This looks awfully similar to the PERIPH_RBF and CORE_RBF above.
PERIPH_RBF and CORE_RBF are the flags, so i can change them to enum. But above #define are magic content used to identify the bistream type. If above #define are not suitable, what can you suggest?
Maybe you can just align those two to avoid duplication ?
What's you means with duplication, they are different thing. How about i change the name to ARRIA10RBF_PERIPH_TYPE and ARRIA10RBF_CORE_TYPE.
static const struct socfpga_fpga_manager *fpga_manager_base = (void *)SOCFPGA_FPGAMGRREGS_ADDRESS; @@ -64,7 +70,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,13 +453,393 @@ int fpgamgr_program_finish(void) return 0; } +const char *get_fpga_filename(const void *fdt, int *len, u32 core)
static, fix globally .
+{
- const char *fpga_filename = NULL;
- const char *cell;
- int nodeoffset;
ofnode_read_string() , use ofnode globally.
Noted.
- nodeoffset = fdtdec_next_compatible(fdt, 0,
COMPAT_ALTERA_SOCFPGA_FPGA0);- if (nodeoffset >= 0) {
if (core) {cell = fdt_getprop(fdt,nodeoffset,"altr,bitstream_core",len);} else {cell = fdt_getprop(fdt, nodeoffset,"altr,bitstream_periph", len);
}if (cell)fpga_filename = cell;- }
- return fpga_filename;
+}
+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) == RBF_UNENCRYPTED) {rbf->security = unencrypted;} else if (*(buffer + i) == RBF_ENCRYPTED) {rbf->security = encrypted;} else if (*(buffer + i + 1) ==RBF_UNENCRYPTED) {
rbf->security = unencrypted;} else if (*(buffer + i + 1) == RBF_ENCRYPTED){
rbf->security = encrypted;} else {rbf->security = invalid;continue;}/* PERIPH RBF(buffer + i + 1), CORE RBF(buffer- i
- */
if (*(buffer + i + 1) == ARRIA10RBF_PERIPH) {rbf->section = periph_section;break;} else if (*(buffer + i + 1) ==ARRIA10RBF_CORE) {
rbf->section = core_section;break;} else if (*(buffer + i + 2) ==ARRIA10RBF_PERIPH) {
rbf->section = periph_section;break;} else if (*(buffer + i + 2) ==ARRIA10RBF_CORE) {
rbf->section = core_section;break;}rbf->section = unknown;break;WATCHDOG_RESET();- }
+}
+static struct firmware *fw;
What is this static variable doing here ?
A place for storing firmware and its attribute data. This would be shared across a few helper functions which contain firmware loader.
Why is it not in the device data instead ? If you had multiple FPGAs in the system, this would likely be randomly overwritten . Such static variables shouldn't be needed in DM/DT capable system.
Noted. Made sense.
+int first_loading_rbf_to_buffer(struct device_platdata *plat,
struct fpga_loadfs_info*fpga_loadfs,
u32 *buffer, u32*buffer_bsize) +{
- u32 *buffer_p_after_header = NULL;
- u32 buffersz_after_header = 0;
- u32 totalsz_header_rbf = 0;
- u32 *buffer_p = (u32 *)*buffer;
- struct image_header *header = &(fpga_loadfs->header);
- size_t buffer_size = *buffer_bsize;
- int ret = 0;
- /* Load mkimage header into buffer */
- ret = request_firmware_into_buf(plat,
fpga_loadfs-fpga_fsinfo-
filename,
header,sizeof(structimage_header),
fpga_loadfs->offset,&fw);- if (ret < 0) {
debug("FPGA: Failed to read RBF mkimage headerfrom flash.\n");
return -ENOENT;- }
- WATCHDOG_RESET();
- if (!image_check_magic(header)) {
debug("FPGA: Bad Magic Number.\n");return -EBADF;- }
- if (!image_check_hcrc(header)) {
debug("FPGA: Bad Header Checksum.\n");return -EPERM;- }
- /* Getting RBF data size from mkimage header */
- fpga_loadfs->remaining = image_get_data_size(header);
- /* Calculate total size of both RBF with mkimage
header */
- totalsz_header_rbf = fpga_loadfs->remaining +
sizeof(struct image_header);- /*
- * Determine buffer size vs RBF size, and calculating
number of
- * chunk by chunk transfer is required due to smaller
buffer size
- * compare to RBF
- */
- if (totalsz_header_rbf > buffer_size) {
/* Calculate size of RBF in the buffer */buffersz_after_header =buffer_size - sizeof(structimage_header);
fpga_loadfs->remaining -=buffersz_after_header;
- } else {
/* Loading whole RBF into buffer */buffer_size = totalsz_header_rbf;/* Calculate size of RBF in the buffer */buffersz_after_header =buffer_size - sizeof(structimage_header);
fpga_loadfs->remaining = 0;- }
- /* Loading mkimage header and RBFinto buffer */
- ret = request_firmware_into_buf(plat,
fpga_loadfs-fpga_fsinfo-
filename,
buffer_p,buffer_size,fpga_loadfs->offset,&fw);This looks like some unwound loop , since the request_firmware_into_buf() is called twice. This probably works for the 300 kiB core RBF, but doesn't work for the 16 MiB full RBF. Also, for() cycle should be used somehow.
This function is mainly for checking the mkimage header, searching the location of the bitstream image from the 1st chunk of reading data and updating the read location. The remaining of the bitstream data after 1st chunk would be processed by the function called subsequent_loading_rbf_to_buffer().
I wonder if this can be somehow optimized, so it's not such a long function with repeated pieces of code.
I already try my best to optimize them without compromising consistent implementation for periph rbf, core rbf and full rbf.
- if (ret < 0) {
debug("FPGA: Failed to read RBF mkimage headerand RBF from ");
debug("flash.\n");return -ENOENT;- }
- /*
- * Getting pointer of RBF starting address where it's
- * right after mkimage header
- */
- buffer_p_after_header =
(u32 *)((u_char *)buffer_p + sizeof(structimage_header));
- /* Update next reading RBF offset */
- fpga_loadfs->offset += buffer_size;
- /* Getting info about RBF types */
- get_rbf_image_info(&fpga_loadfs->rbfinfo, (u16
*)buffer_p_after_header);
- /*
- * Update the starting addr of RBF to init FPGA &
programming RBF
- * into FPGA
- */
- *buffer = (u32)buffer_p_after_header;
- /* Update the size of RBF to be programmed into FPGA
*/
- *buffer_bsize = buffersz_after_header;
+#ifdef CONFIG_CHECK_FPGA_DATA_CRC
- fpga_loadfs->datacrc = crc32(fpga_loadfs->datacrc,
(u_char*)buffer_p_after_header,
buffersz_after_header);
Why is this not ON by default ?
It is off for the sake of performance.
Do you have some hard numbers to support this claim ?
+#endif
+if (fpga_loadfs->remaining == 0) { +#ifdef CONFIG_CHECK_FPGA_DATA_CRC
- if (fpga_loadfs->datacrc !=
image_get_dcrc(&(fpga_loadfs-
header))) {
debug("FPGA: Bad Data Checksum.\n");return -EPERM;- }
+#endif +}
- return 0;
+}
[...]
On 11/26/2018 11:09 AM, Chee, Tien Fong wrote: [...]
diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig index 50e9019..06a8204 100644 --- a/drivers/fpga/Kconfig +++ b/drivers/fpga/Kconfig @@ -21,6 +21,15 @@ config FPGA_SOCFPGA This provides common functionality for Gen5 and Arria10 devices. +config CHECK_FPGA_DATA_CRC
config FPGA_SOCFPGA_A10_CRC_CHECK
What is this for and why shouldn't this be ON by default ?
Both periph.rbf or full.rbf are wrapped with mkimage. So, this CRC checking can be used to check the integrity of the loading bitstream data against checksum from mkimage. It is off for the sake of performance.
Is there a measurable performance degradation ? I presume that's because caches are disabled at that point, yes? Enable caches and see if that helps.
Just logical sense, performance sure getting degraded, especially loading full rbf, but may be not that obvious for periph.rbf because of very small size, i can try to measure. If not much difference, i can enable checking in default.
Hard numbers are the only relevant argument here, please measure. And try it with caches enabled as much as possible, you want users to boot fast. Arria10 is particularly annoyingly slow at booting.
- bool "Enable CRC cheking on Arria10 FPGA bistream"
- depends on FPGA_SOCFPGA
- help
- Say Y here to enable the CRC checking on Arria 10
FPGA bitstream
- This provides CRC checking to ensure integrated of
Arria 10 FPGA
- bitstream is programmed into FPGA.
config FPGA_CYCLON2 bool "Enable Altera FPGA driver for Cyclone II" depends on FPGA_ALTERA diff --git a/drivers/fpga/socfpga_arria10.c b/drivers/fpga/socfpga_arria10.c index 114dd91..d9ad237 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-2018 Intel Corporation <www.intel.com>
*/ #include <asm/io.h> @@ -10,8 +10,10 @@ #include <asm/arch/sdram.h> #include <asm/arch/misc.h> #include <altera.h> +#include <asm/arch/pinmux.h> #include <common.h> #include <errno.h> +#include <fs_loader.h> #include <wait_bit.h> #include <watchdog.h> @@ -21,6 +23,10 @@ #define COMPRESSION_OFFSET 229 #define FPGA_TIMEOUT_MSEC 1000 /* timeout in ms */ #define FPGA_TIMEOUT_CNT 0x1000000 +#define RBF_UNENCRYPTED 0xa65c +#define RBF_ENCRYPTED 0xa65d +#define ARRIA10RBF_PERIPH 0x0001 +#define ARRIA10RBF_CORE 0x8001
This looks awfully similar to the PERIPH_RBF and CORE_RBF above.
PERIPH_RBF and CORE_RBF are the flags, so i can change them to enum. But above #define are magic content used to identify the bistream type. If above #define are not suitable, what can you suggest?
Maybe you can just align those two to avoid duplication ?
What's you means with duplication, they are different thing. How about i change the name to ARRIA10RBF_PERIPH_TYPE and ARRIA10RBF_CORE_TYPE.
ARRIA10RBF_PERIPH = (PERIPH_RBF << 15) | 1
same for ... _CORE ... is that a coincidence ?
[...]
On Mon, 2018-11-26 at 12:18 +0100, Marek Vasut wrote:
On 11/26/2018 11:09 AM, Chee, Tien Fong wrote: [...]
diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig index 50e9019..06a8204 100644 --- a/drivers/fpga/Kconfig +++ b/drivers/fpga/Kconfig @@ -21,6 +21,15 @@ config FPGA_SOCFPGA This provides common functionality for Gen5 and Arria10 devices. +config CHECK_FPGA_DATA_CRC
config FPGA_SOCFPGA_A10_CRC_CHECK
What is this for and why shouldn't this be ON by default ?
Both periph.rbf or full.rbf are wrapped with mkimage. So, this CRC checking can be used to check the integrity of the loading bitstream data against checksum from mkimage. It is off for the sake of performance.
Is there a measurable performance degradation ? I presume that's because caches are disabled at that point, yes? Enable caches and see if that helps.
Just logical sense, performance sure getting degraded, especially loading full rbf, but may be not that obvious for periph.rbf because of very small size, i can try to measure. If not much difference, i can enable checking in default.
Hard numbers are the only relevant argument here, please measure. And try it with caches enabled as much as possible, you want users to boot fast. Arria10 is particularly annoyingly slow at booting.
sure.
- bool "Enable CRC cheking on Arria10 FPGA bistream"
- depends on FPGA_SOCFPGA
- help
- Say Y here to enable the CRC checking on Arria 10
FPGA bitstream
- This provides CRC checking to ensure integrated
of Arria 10 FPGA
- bitstream is programmed into FPGA.
config FPGA_CYCLON2 bool "Enable Altera FPGA driver for Cyclone II" depends on FPGA_ALTERA diff --git a/drivers/fpga/socfpga_arria10.c b/drivers/fpga/socfpga_arria10.c index 114dd91..d9ad237 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-2018 Intel Corporation <www.intel.co
m> */ #include <asm/io.h> @@ -10,8 +10,10 @@ #include <asm/arch/sdram.h> #include <asm/arch/misc.h> #include <altera.h> +#include <asm/arch/pinmux.h> #include <common.h> #include <errno.h> +#include <fs_loader.h> #include <wait_bit.h> #include <watchdog.h> @@ -21,6 +23,10 @@ #define COMPRESSION_OFFSET 229 #define FPGA_TIMEOUT_MSEC 1000 /* timeout in ms */ #define FPGA_TIMEOUT_CNT 0x1000000 +#define RBF_UNENCRYPTED 0xa65c +#define RBF_ENCRYPTED 0xa65d +#define ARRIA10RBF_PERIPH 0x0001 +#define ARRIA10RBF_CORE 0x8001
This looks awfully similar to the PERIPH_RBF and CORE_RBF above.
PERIPH_RBF and CORE_RBF are the flags, so i can change them to enum. But above #define are magic content used to identify the bistream type. If above #define are not suitable, what can you suggest?
Maybe you can just align those two to avoid duplication ?
What's you means with duplication, they are different thing. How about i change the name to ARRIA10RBF_PERIPH_TYPE and ARRIA10RBF_CORE_TYPE.
ARRIA10RBF_PERIPH = (PERIPH_RBF << 15) | 1
We can't use the flag PERIPH_RBF(similar TRUE/FALSE) for magic content, because they are not related each other. Magic content is defined by HW design.
We identify the type of rbf such as periph, and core through this magic content within the rbf. After we getting the type, then only we setting the flag such as PERIPH_RBF to the function.
same for ... _CORE ... is that a coincidence ?
[...]
On 11/27/2018 09:54 AM, Chee, Tien Fong wrote:
On Mon, 2018-11-26 at 12:18 +0100, Marek Vasut wrote:
On 11/26/2018 11:09 AM, Chee, Tien Fong wrote: [...]
> > diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig > index 50e9019..06a8204 100644 > --- a/drivers/fpga/Kconfig > +++ b/drivers/fpga/Kconfig > @@ -21,6 +21,15 @@ config FPGA_SOCFPGA > > This provides common functionality for Gen5 and > Arria10 > devices. > > +config CHECK_FPGA_DATA_CRC config FPGA_SOCFPGA_A10_CRC_CHECK
What is this for and why shouldn't this be ON by default ?
Both periph.rbf or full.rbf are wrapped with mkimage. So, this CRC checking can be used to check the integrity of the loading bitstream data against checksum from mkimage. It is off for the sake of performance.
Is there a measurable performance degradation ? I presume that's because caches are disabled at that point, yes? Enable caches and see if that helps.
Just logical sense, performance sure getting degraded, especially loading full rbf, but may be not that obvious for periph.rbf because of very small size, i can try to measure. If not much difference, i can enable checking in default.
Hard numbers are the only relevant argument here, please measure. And try it with caches enabled as much as possible, you want users to boot fast. Arria10 is particularly annoyingly slow at booting.
sure.
> > > > + bool "Enable CRC cheking on Arria10 FPGA bistream" > + depends on FPGA_SOCFPGA > + help > + Say Y here to enable the CRC checking on Arria 10 > FPGA > bitstream > + > + This provides CRC checking to ensure integrated > of > Arria > 10 FPGA > + bitstream is programmed into FPGA. > + > config FPGA_CYCLON2 > bool "Enable Altera FPGA driver for Cyclone II" > depends on FPGA_ALTERA > diff --git a/drivers/fpga/socfpga_arria10.c > b/drivers/fpga/socfpga_arria10.c > index 114dd91..d9ad237 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-2018 Intel Corporation <www.intel.co > m> > */ > > #include <asm/io.h> > @@ -10,8 +10,10 @@ > #include <asm/arch/sdram.h> > #include <asm/arch/misc.h> > #include <altera.h> > +#include <asm/arch/pinmux.h> > #include <common.h> > #include <errno.h> > +#include <fs_loader.h> > #include <wait_bit.h> > #include <watchdog.h> > > @@ -21,6 +23,10 @@ > #define COMPRESSION_OFFSET 229 > #define FPGA_TIMEOUT_MSEC 1000 /* timeout in ms */ > #define FPGA_TIMEOUT_CNT 0x1000000 > +#define RBF_UNENCRYPTED 0xa65c > +#define RBF_ENCRYPTED 0xa65d > +#define ARRIA10RBF_PERIPH 0x0001 > +#define ARRIA10RBF_CORE 0x8001 This looks awfully similar to the PERIPH_RBF and CORE_RBF above.
PERIPH_RBF and CORE_RBF are the flags, so i can change them to enum. But above #define are magic content used to identify the bistream type. If above #define are not suitable, what can you suggest?
Maybe you can just align those two to avoid duplication ?
What's you means with duplication, they are different thing. How about i change the name to ARRIA10RBF_PERIPH_TYPE and ARRIA10RBF_CORE_TYPE.
ARRIA10RBF_PERIPH = (PERIPH_RBF << 15) | 1
We can't use the flag PERIPH_RBF(similar TRUE/FALSE) for magic content, because they are not related each other. Magic content is defined by HW design.
You can define the flags to match the HW design, which is probably a good idea ?
We identify the type of rbf such as periph, and core through this magic content within the rbf. After we getting the type, then only we setting the flag such as PERIPH_RBF to the function.
same for ... _CORE ... is that a coincidence ?
[...]
On Tue, 2018-11-27 at 13:08 +0100, Marek Vasut wrote:
On 11/27/2018 09:54 AM, Chee, Tien Fong wrote:
On Mon, 2018-11-26 at 12:18 +0100, Marek Vasut wrote:
On 11/26/2018 11:09 AM, Chee, Tien Fong wrote: [...]
> > > > > > > > diff --git a/drivers/fpga/Kconfig > > b/drivers/fpga/Kconfig > > index 50e9019..06a8204 100644 > > --- a/drivers/fpga/Kconfig > > +++ b/drivers/fpga/Kconfig > > @@ -21,6 +21,15 @@ config FPGA_SOCFPGA > > > > This provides common functionality for Gen5 > > and > > Arria10 > > devices. > > > > +config CHECK_FPGA_DATA_CRC > config FPGA_SOCFPGA_A10_CRC_CHECK > > What is this for and why shouldn't this be ON by default > ? Both periph.rbf or full.rbf are wrapped with mkimage. So, this CRC checking can be used to check the integrity of the loading bitstream data against checksum from mkimage. It is off for the sake of performance.
Is there a measurable performance degradation ? I presume that's because caches are disabled at that point, yes? Enable caches and see if that helps.
Just logical sense, performance sure getting degraded, especially loading full rbf, but may be not that obvious for periph.rbf because of very small size, i can try to measure. If not much difference, i can enable checking in default.
Hard numbers are the only relevant argument here, please measure. And try it with caches enabled as much as possible, you want users to boot fast. Arria10 is particularly annoyingly slow at booting.
sure.
> > > > > > > > > > > > > + bool "Enable CRC cheking on Arria10 FPGA > > bistream" > > + depends on FPGA_SOCFPGA > > + help > > + Say Y here to enable the CRC checking on > > Arria 10 > > FPGA > > bitstream > > + > > + This provides CRC checking to ensure > > integrated > > of > > Arria > > 10 FPGA > > + bitstream is programmed into FPGA. > > + > > config FPGA_CYCLON2 > > bool "Enable Altera FPGA driver for Cyclone > > II" > > depends on FPGA_ALTERA > > diff --git a/drivers/fpga/socfpga_arria10.c > > b/drivers/fpga/socfpga_arria10.c > > index 114dd91..d9ad237 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-2018 Intel Corporation <www.inte > > l.co > > m> > > */ > > > > #include <asm/io.h> > > @@ -10,8 +10,10 @@ > > #include <asm/arch/sdram.h> > > #include <asm/arch/misc.h> > > #include <altera.h> > > +#include <asm/arch/pinmux.h> > > #include <common.h> > > #include <errno.h> > > +#include <fs_loader.h> > > #include <wait_bit.h> > > #include <watchdog.h> > > > > @@ -21,6 +23,10 @@ > > #define COMPRESSION_OFFSET 229 > > #define FPGA_TIMEOUT_MSEC 1000 /* timeout in > > ms */ > > #define FPGA_TIMEOUT_CNT 0x1000000 > > +#define RBF_UNENCRYPTED 0xa65c > > +#define RBF_ENCRYPTED 0xa65d > > +#define ARRIA10RBF_PERIPH 0x0001 > > +#define ARRIA10RBF_CORE 0x8001 > This looks awfully similar to the PERIPH_RBF and CORE_RBF > above. PERIPH_RBF and CORE_RBF are the flags, so i can change them to enum. But above #define are magic content used to identify the bistream type. If above #define are not suitable, what can you suggest?
Maybe you can just align those two to avoid duplication ?
What's you means with duplication, they are different thing. How about i change the name to ARRIA10RBF_PERIPH_TYPE and ARRIA10RBF_CORE_TYPE.
ARRIA10RBF_PERIPH = (PERIPH_RBF << 15) | 1
We can't use the flag PERIPH_RBF(similar TRUE/FALSE) for magic content, because they are not related each other. Magic content is defined by HW design.
You can define the flags to match the HW design, which is probably a good idea ?
I have no strong opinion of this, i can do it.
We identify the type of rbf such as periph, and core through this magic content within the rbf. After we getting the type, then only we setting the flag such as PERIPH_RBF to the function.
same for ... _CORE ... is that a coincidence ?
[...]
On 11/28/2018 03:53 PM, Chee, Tien Fong wrote:
On Tue, 2018-11-27 at 13:08 +0100, Marek Vasut wrote:
On 11/27/2018 09:54 AM, Chee, Tien Fong wrote:
On Mon, 2018-11-26 at 12:18 +0100, Marek Vasut wrote:
On 11/26/2018 11:09 AM, Chee, Tien Fong wrote: [...]
> > >> >> >>> >>> >>> diff --git a/drivers/fpga/Kconfig >>> b/drivers/fpga/Kconfig >>> index 50e9019..06a8204 100644 >>> --- a/drivers/fpga/Kconfig >>> +++ b/drivers/fpga/Kconfig >>> @@ -21,6 +21,15 @@ config FPGA_SOCFPGA >>> >>> This provides common functionality for Gen5 >>> and >>> Arria10 >>> devices. >>> >>> +config CHECK_FPGA_DATA_CRC >> config FPGA_SOCFPGA_A10_CRC_CHECK >> >> What is this for and why shouldn't this be ON by default >> ? > Both periph.rbf or full.rbf are wrapped with mkimage. So, > this > CRC > checking can be used to check the integrity of the loading > bitstream > data against checksum from mkimage. It is off for the sake > of > performance. Is there a measurable performance degradation ? I presume that's because caches are disabled at that point, yes? Enable caches and see if that helps.
Just logical sense, performance sure getting degraded, especially loading full rbf, but may be not that obvious for periph.rbf because of very small size, i can try to measure. If not much difference, i can enable checking in default.
Hard numbers are the only relevant argument here, please measure. And try it with caches enabled as much as possible, you want users to boot fast. Arria10 is particularly annoyingly slow at booting.
sure.
> > > >> >> >> >>> >>> >>> >>> >>> + bool "Enable CRC cheking on Arria10 FPGA >>> bistream" >>> + depends on FPGA_SOCFPGA >>> + help >>> + Say Y here to enable the CRC checking on >>> Arria 10 >>> FPGA >>> bitstream >>> + >>> + This provides CRC checking to ensure >>> integrated >>> of >>> Arria >>> 10 FPGA >>> + bitstream is programmed into FPGA. >>> + >>> config FPGA_CYCLON2 >>> bool "Enable Altera FPGA driver for Cyclone >>> II" >>> depends on FPGA_ALTERA >>> diff --git a/drivers/fpga/socfpga_arria10.c >>> b/drivers/fpga/socfpga_arria10.c >>> index 114dd91..d9ad237 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-2018 Intel Corporation <www.inte >>> l.co >>> m> >>> */ >>> >>> #include <asm/io.h> >>> @@ -10,8 +10,10 @@ >>> #include <asm/arch/sdram.h> >>> #include <asm/arch/misc.h> >>> #include <altera.h> >>> +#include <asm/arch/pinmux.h> >>> #include <common.h> >>> #include <errno.h> >>> +#include <fs_loader.h> >>> #include <wait_bit.h> >>> #include <watchdog.h> >>> >>> @@ -21,6 +23,10 @@ >>> #define COMPRESSION_OFFSET 229 >>> #define FPGA_TIMEOUT_MSEC 1000 /* timeout in >>> ms */ >>> #define FPGA_TIMEOUT_CNT 0x1000000 >>> +#define RBF_UNENCRYPTED 0xa65c >>> +#define RBF_ENCRYPTED 0xa65d >>> +#define ARRIA10RBF_PERIPH 0x0001 >>> +#define ARRIA10RBF_CORE 0x8001 >> This looks awfully similar to the PERIPH_RBF and CORE_RBF >> above. > PERIPH_RBF and CORE_RBF are the flags, so i can change them > to > enum. > But above #define are magic content used to identify the > bistream > type. > If above #define are not suitable, what can you suggest? Maybe you can just align those two to avoid duplication ?
What's you means with duplication, they are different thing. How about i change the name to ARRIA10RBF_PERIPH_TYPE and ARRIA10RBF_CORE_TYPE.
ARRIA10RBF_PERIPH = (PERIPH_RBF << 15) | 1
We can't use the flag PERIPH_RBF(similar TRUE/FALSE) for magic content, because they are not related each other. Magic content is defined by HW design.
You can define the flags to match the HW design, which is probably a good idea ?
I have no strong opinion of this, i can do it.
If it can deduplicate things, that'd be good.
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 --- arch/arm/mach-socfpga/spl_a10.c | 49 ++++++++++++++++++++++++++++++++++++++- common/spl/spl_mmc.c | 2 +- include/mmc.h | 1 + 3 files changed, 50 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-socfpga/spl_a10.c b/arch/arm/mach-socfpga/spl_a10.c index 3ea64f7..67a4fac 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-2018 Altera Corporation <www.altera.com> */
#include <common.h> @@ -23,6 +23,10 @@ #include <fdtdec.h> #include <watchdog.h> #include <asm/arch/pinmux.h> +#include <asm/arch/fpga_manager.h> +#include <mmc.h> + +#define RBF 0
DECLARE_GLOBAL_DATA_PTR;
@@ -73,6 +77,49 @@ void spl_board_init(void) WATCHDOG_RESET();
arch_early_init_r(); + + /* If the full FPGA is already loaded, ie.from EPCQ, config fpga pins */ + if (is_fpgamgr_user_mode()) { + config_pins(gd->fdt_blob, "shared"); + config_pins(gd->fdt_blob, "fpga"); + } else if (!is_fpgamgr_early_user_mode()) { + /* Program IOSSM(early IO release) or full FPGA */ + fpga_fs_info fpga_fsinfo; + char buf[16 * 1024] __aligned(ARCH_DMA_MINALIGN); + struct spl_boot_device bootdev; + int len = 0; + + bootdev.boot_device = spl_boot_device(); + + /* Init MMC driver before reading FPGA bitstream from flash */ + if (bootdev.boot_device == BOOT_DEVICE_MMC1) { + struct mmc *mmc = NULL; + int err = 0; + + err = spl_mmc_find_device(&mmc, bootdev.boot_device); + if (err) + return; + + err = mmc_init(mmc); + + if (err) { + debug("spl: mmc init failed with error: %d\n", + err); + + return; + } + } + + fpga_fsinfo.filename = (char *)get_fpga_filename(gd->fdt_blob, + &len, + RBF); + + 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(); }
void board_init_f(ulong dummy) diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c index 4d55dcc..b85e146 100644 --- a/common/spl/spl_mmc.c +++ b/common/spl/spl_mmc.c @@ -111,7 +111,7 @@ static int spl_mmc_get_device_index(u32 boot_device) return -ENODEV; }
-static int spl_mmc_find_device(struct mmc **mmcp, u32 boot_device) +int spl_mmc_find_device(struct mmc **mmcp, u32 boot_device) { #if CONFIG_IS_ENABLED(DM_MMC) struct udevice *dev; diff --git a/include/mmc.h b/include/mmc.h index 95548e9..de92909 100644 --- a/include/mmc.h +++ b/include/mmc.h @@ -829,6 +829,7 @@ int board_mmc_init(bd_t *bis); int cpu_mmc_init(bd_t *bis); int mmc_get_env_addr(struct mmc *mmc, int copy, u32 *env_addr); int mmc_get_env_dev(void); +int spl_mmc_find_device(struct mmc **mmcp, u32 boot_device);
/* Set block count limit because of 16 bit register limit on some hardware*/ #ifndef CONFIG_SYS_MMC_MAX_BLK_COUNT
On 11/21/2018 11:41 AM, tien.fong.chee@intel.com wrote:
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
arch/arm/mach-socfpga/spl_a10.c | 49 ++++++++++++++++++++++++++++++++++++++- common/spl/spl_mmc.c | 2 +- include/mmc.h | 1 + 3 files changed, 50 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-socfpga/spl_a10.c b/arch/arm/mach-socfpga/spl_a10.c index 3ea64f7..67a4fac 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-2018 Altera Corporation <www.altera.com>
#include <common.h> @@ -23,6 +23,10 @@ #include <fdtdec.h> #include <watchdog.h> #include <asm/arch/pinmux.h> +#include <asm/arch/fpga_manager.h> +#include <mmc.h>
+#define RBF 0
DECLARE_GLOBAL_DATA_PTR;
@@ -73,6 +77,49 @@ void spl_board_init(void) WATCHDOG_RESET();
arch_early_init_r();
- /* If the full FPGA is already loaded, ie.from EPCQ, config fpga pins */
- if (is_fpgamgr_user_mode()) {
config_pins(gd->fdt_blob, "shared");config_pins(gd->fdt_blob, "fpga");- } else if (!is_fpgamgr_early_user_mode()) {
/* Program IOSSM(early IO release) or full FPGA */fpga_fs_info fpga_fsinfo;char buf[16 * 1024] __aligned(ARCH_DMA_MINALIGN);struct spl_boot_device bootdev;int len = 0;bootdev.boot_device = spl_boot_device();/* Init MMC driver before reading FPGA bitstream from flash */if (bootdev.boot_device == BOOT_DEVICE_MMC1) {struct mmc *mmc = NULL;int err = 0;err = spl_mmc_find_device(&mmc, bootdev.boot_device);if (err)return;err = mmc_init(mmc);
I thought all this backend specific stuff would be hidden in the FW loader.
if (err) {debug("spl: mmc init failed with error: %d\n",err);return;}}fpga_fsinfo.filename = (char *)get_fpga_filename(gd->fdt_blob,&len,RBF);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();}
void board_init_f(ulong dummy) diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c index 4d55dcc..b85e146 100644 --- a/common/spl/spl_mmc.c +++ b/common/spl/spl_mmc.c @@ -111,7 +111,7 @@ static int spl_mmc_get_device_index(u32 boot_device) return -ENODEV; }
-static int spl_mmc_find_device(struct mmc **mmcp, u32 boot_device) +int spl_mmc_find_device(struct mmc **mmcp, u32 boot_device) { #if CONFIG_IS_ENABLED(DM_MMC) struct udevice *dev; diff --git a/include/mmc.h b/include/mmc.h index 95548e9..de92909 100644 --- a/include/mmc.h +++ b/include/mmc.h @@ -829,6 +829,7 @@ int board_mmc_init(bd_t *bis); int cpu_mmc_init(bd_t *bis); int mmc_get_env_addr(struct mmc *mmc, int copy, u32 *env_addr); int mmc_get_env_dev(void); +int spl_mmc_find_device(struct mmc **mmcp, u32 boot_device);
/* Set block count limit because of 16 bit register limit on some hardware*/ #ifndef CONFIG_SYS_MMC_MAX_BLK_COUNT
On Wed, 2018-11-21 at 15:19 +0100, Marek Vasut wrote:
On 11/21/2018 11:41 AM, tien.fong.chee@intel.com wrote:
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
arch/arm/mach-socfpga/spl_a10.c | 49 ++++++++++++++++++++++++++++++++++++++- common/spl/spl_mmc.c | 2 +- include/mmc.h | 1 + 3 files changed, 50 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-socfpga/spl_a10.c b/arch/arm/mach- socfpga/spl_a10.c index 3ea64f7..67a4fac 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-2018 Altera Corporation <www.altera.com>
*/ #include <common.h> @@ -23,6 +23,10 @@ #include <fdtdec.h> #include <watchdog.h> #include <asm/arch/pinmux.h> +#include <asm/arch/fpga_manager.h> +#include <mmc.h>
+#define RBF 0 DECLARE_GLOBAL_DATA_PTR; @@ -73,6 +77,49 @@ void spl_board_init(void) WATCHDOG_RESET(); arch_early_init_r();
- /* If the full FPGA is already loaded, ie.from EPCQ,
config fpga pins */
- if (is_fpgamgr_user_mode()) {
config_pins(gd->fdt_blob, "shared");config_pins(gd->fdt_blob, "fpga");- } else if (!is_fpgamgr_early_user_mode()) {
/* Program IOSSM(early IO release) or full FPGA */fpga_fs_info fpga_fsinfo;char buf[16 * 1024] __aligned(ARCH_DMA_MINALIGN);struct spl_boot_device bootdev;int len = 0;bootdev.boot_device = spl_boot_device();/* Init MMC driver before reading FPGA bitstreamfrom flash */
if (bootdev.boot_device == BOOT_DEVICE_MMC1) {struct mmc *mmc = NULL;int err = 0;err = spl_mmc_find_device(&mmc,bootdev.boot_device);
if (err)return;err = mmc_init(mmc);I thought all this backend specific stuff would be hidden in the FW loader.
The backend supported by FW loader is up to generic file system interface layer. flash driver init is expected done by SPL/U-Boot common init sequence framwork or user. Unfortunately, fw loader need to access flash before init sequence.
if (err) {debug("spl: mmc init failed witherror: %d\n",
err);return;}}fpga_fsinfo.filename = (char*)get_fpga_filename(gd->fdt_blob,
&len,
RBF);
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();} void board_init_f(ulong dummy) diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c index 4d55dcc..b85e146 100644 --- a/common/spl/spl_mmc.c +++ b/common/spl/spl_mmc.c @@ -111,7 +111,7 @@ static int spl_mmc_get_device_index(u32 boot_device) return -ENODEV; } -static int spl_mmc_find_device(struct mmc **mmcp, u32 boot_device) +int spl_mmc_find_device(struct mmc **mmcp, u32 boot_device) { #if CONFIG_IS_ENABLED(DM_MMC) struct udevice *dev; diff --git a/include/mmc.h b/include/mmc.h index 95548e9..de92909 100644 --- a/include/mmc.h +++ b/include/mmc.h @@ -829,6 +829,7 @@ int board_mmc_init(bd_t *bis); int cpu_mmc_init(bd_t *bis); int mmc_get_env_addr(struct mmc *mmc, int copy, u32 *env_addr); int mmc_get_env_dev(void); +int spl_mmc_find_device(struct mmc **mmcp, u32 boot_device); /* Set block count limit because of 16 bit register limit on some hardware*/ #ifndef CONFIG_SYS_MMC_MAX_BLK_COUNT
On 11/23/2018 10:51 AM, Chee, Tien Fong wrote:
On Wed, 2018-11-21 at 15:19 +0100, Marek Vasut wrote:
On 11/21/2018 11:41 AM, tien.fong.chee@intel.com wrote:
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
arch/arm/mach-socfpga/spl_a10.c | 49 ++++++++++++++++++++++++++++++++++++++- common/spl/spl_mmc.c | 2 +- include/mmc.h | 1 + 3 files changed, 50 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-socfpga/spl_a10.c b/arch/arm/mach- socfpga/spl_a10.c index 3ea64f7..67a4fac 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-2018 Altera Corporation <www.altera.com>
*/ #include <common.h> @@ -23,6 +23,10 @@ #include <fdtdec.h> #include <watchdog.h> #include <asm/arch/pinmux.h> +#include <asm/arch/fpga_manager.h> +#include <mmc.h>
+#define RBF 0 DECLARE_GLOBAL_DATA_PTR; @@ -73,6 +77,49 @@ void spl_board_init(void) WATCHDOG_RESET(); arch_early_init_r();
- /* If the full FPGA is already loaded, ie.from EPCQ,
config fpga pins */
- if (is_fpgamgr_user_mode()) {
config_pins(gd->fdt_blob, "shared");config_pins(gd->fdt_blob, "fpga");- } else if (!is_fpgamgr_early_user_mode()) {
/* Program IOSSM(early IO release) or full FPGA */fpga_fs_info fpga_fsinfo;char buf[16 * 1024] __aligned(ARCH_DMA_MINALIGN);struct spl_boot_device bootdev;int len = 0;bootdev.boot_device = spl_boot_device();/* Init MMC driver before reading FPGA bitstreamfrom flash */
if (bootdev.boot_device == BOOT_DEVICE_MMC1) {struct mmc *mmc = NULL;int err = 0;err = spl_mmc_find_device(&mmc,bootdev.boot_device);
if (err)return;err = mmc_init(mmc);I thought all this backend specific stuff would be hidden in the FW loader.
The backend supported by FW loader is up to generic file system interface layer. flash driver init is expected done by SPL/U-Boot common init sequence framwork or user. Unfortunately, fw loader need to access flash before init sequence.
This is actually accessing eMMC though , not flash . If we need this huge boilerplate code every time we use the FW loader, than the FW loader needs fixing. I can understand the spl_boot_device() being outside of the FW loader, but not the mmc_init() and co.
On Fri, 2018-11-23 at 13:31 +0100, Marek Vasut wrote:
On 11/23/2018 10:51 AM, Chee, Tien Fong wrote:
On Wed, 2018-11-21 at 15:19 +0100, Marek Vasut wrote:
On 11/21/2018 11:41 AM, tien.fong.chee@intel.com wrote:
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
arch/arm/mach-socfpga/spl_a10.c | 49 ++++++++++++++++++++++++++++++++++++++- common/spl/spl_mmc.c | 2 +- include/mmc.h | 1 + 3 files changed, 50 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-socfpga/spl_a10.c b/arch/arm/mach- socfpga/spl_a10.c index 3ea64f7..67a4fac 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-2018 Altera Corporation <www.altera.com
*/ #include <common.h> @@ -23,6 +23,10 @@ #include <fdtdec.h> #include <watchdog.h> #include <asm/arch/pinmux.h> +#include <asm/arch/fpga_manager.h> +#include <mmc.h>
+#define RBF 0 DECLARE_GLOBAL_DATA_PTR; @@ -73,6 +77,49 @@ void spl_board_init(void) WATCHDOG_RESET(); arch_early_init_r();
- /* If the full FPGA is already loaded, ie.from EPCQ,
config fpga pins */
- if (is_fpgamgr_user_mode()) {
config_pins(gd->fdt_blob, "shared");config_pins(gd->fdt_blob, "fpga");- } else if (!is_fpgamgr_early_user_mode()) {
/* Program IOSSM(early IO release) or fullFPGA */
fpga_fs_info fpga_fsinfo;char buf[16 * 1024]__aligned(ARCH_DMA_MINALIGN);
struct spl_boot_device bootdev;int len = 0;bootdev.boot_device = spl_boot_device();/* Init MMC driver before reading FPGAbitstream from flash */
if (bootdev.boot_device == BOOT_DEVICE_MMC1) {struct mmc *mmc = NULL;int err = 0;err = spl_mmc_find_device(&mmc,bootdev.boot_device);
if (err)return;err = mmc_init(mmc);I thought all this backend specific stuff would be hidden in the FW loader.
The backend supported by FW loader is up to generic file system interface layer. flash driver init is expected done by SPL/U-Boot common init sequence framwork or user. Unfortunately, fw loader need to access flash before init sequence.
This is actually accessing eMMC though , not flash . If we need this huge boilerplate code every time we use the FW loader, than the FW loader needs fixing. I can understand the spl_boot_device() being outside of the FW loader, but not the mmc_init() and co.
I can explore the posibility of adding the flash int mechanism into the fm loader probe function.
On 11/26/2018 11:10 AM, Chee, Tien Fong wrote:
On Fri, 2018-11-23 at 13:31 +0100, Marek Vasut wrote:
On 11/23/2018 10:51 AM, Chee, Tien Fong wrote:
On Wed, 2018-11-21 at 15:19 +0100, Marek Vasut wrote:
On 11/21/2018 11:41 AM, tien.fong.chee@intel.com wrote:
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
arch/arm/mach-socfpga/spl_a10.c | 49 ++++++++++++++++++++++++++++++++++++++- common/spl/spl_mmc.c | 2 +- include/mmc.h | 1 + 3 files changed, 50 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-socfpga/spl_a10.c b/arch/arm/mach- socfpga/spl_a10.c index 3ea64f7..67a4fac 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-2018 Altera Corporation <www.altera.com
*/ #include <common.h> @@ -23,6 +23,10 @@ #include <fdtdec.h> #include <watchdog.h> #include <asm/arch/pinmux.h> +#include <asm/arch/fpga_manager.h> +#include <mmc.h>
+#define RBF 0 DECLARE_GLOBAL_DATA_PTR; @@ -73,6 +77,49 @@ void spl_board_init(void) WATCHDOG_RESET(); arch_early_init_r();
- /* If the full FPGA is already loaded, ie.from EPCQ,
config fpga pins */
- if (is_fpgamgr_user_mode()) {
config_pins(gd->fdt_blob, "shared");config_pins(gd->fdt_blob, "fpga");- } else if (!is_fpgamgr_early_user_mode()) {
/* Program IOSSM(early IO release) or fullFPGA */
fpga_fs_info fpga_fsinfo;char buf[16 * 1024]__aligned(ARCH_DMA_MINALIGN);
struct spl_boot_device bootdev;int len = 0;bootdev.boot_device = spl_boot_device();/* Init MMC driver before reading FPGAbitstream from flash */
if (bootdev.boot_device == BOOT_DEVICE_MMC1) {struct mmc *mmc = NULL;int err = 0;err = spl_mmc_find_device(&mmc,bootdev.boot_device);
if (err)return;err = mmc_init(mmc);I thought all this backend specific stuff would be hidden in the FW loader.
The backend supported by FW loader is up to generic file system interface layer. flash driver init is expected done by SPL/U-Boot common init sequence framwork or user. Unfortunately, fw loader need to access flash before init sequence.
This is actually accessing eMMC though , not flash . If we need this huge boilerplate code every time we use the FW loader, than the FW loader needs fixing. I can understand the spl_boot_device() being outside of the FW loader, but not the mmc_init() and co.
I can explore the posibility of adding the flash int mechanism into the fm loader probe function.
What do you mean by "flash int" ? Note that we're talking about eMMC here, not flash. Unless you mean "backend init" by all that, in which case that'd only make sense, thanks.
On Mon, 2018-11-26 at 12:20 +0100, Marek Vasut wrote:
On 11/26/2018 11:10 AM, Chee, Tien Fong wrote:
On Fri, 2018-11-23 at 13:31 +0100, Marek Vasut wrote:
On 11/23/2018 10:51 AM, Chee, Tien Fong wrote:
On Wed, 2018-11-21 at 15:19 +0100, Marek Vasut wrote:
On 11/21/2018 11:41 AM, tien.fong.chee@intel.com wrote:
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
arch/arm/mach-socfpga/spl_a10.c | 49 ++++++++++++++++++++++++++++++++++++++- common/spl/spl_mmc.c | 2 +- include/mmc.h | 1 + 3 files changed, 50 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-socfpga/spl_a10.c b/arch/arm/mach- socfpga/spl_a10.c index 3ea64f7..67a4fac 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-2018 Altera Corporation <www.altera
.com > > */ #include <common.h> @@ -23,6 +23,10 @@ #include <fdtdec.h> #include <watchdog.h> #include <asm/arch/pinmux.h> +#include <asm/arch/fpga_manager.h> +#include <mmc.h>
+#define RBF 0 DECLARE_GLOBAL_DATA_PTR; @@ -73,6 +77,49 @@ void spl_board_init(void) WATCHDOG_RESET(); arch_early_init_r();
- /* If the full FPGA is already loaded, ie.from
EPCQ, config fpga pins */
- if (is_fpgamgr_user_mode()) {
config_pins(gd->fdt_blob, "shared");config_pins(gd->fdt_blob, "fpga");- } else if (!is_fpgamgr_early_user_mode()) {
/* Program IOSSM(early IO release) or fullFPGA */
fpga_fs_info fpga_fsinfo;char buf[16 * 1024]__aligned(ARCH_DMA_MINALIGN);
struct spl_boot_device bootdev;int len = 0;bootdev.boot_device = spl_boot_device();/* Init MMC driver before reading FPGAbitstream from flash */
if (bootdev.boot_device ==BOOT_DEVICE_MMC1) {
struct mmc *mmc = NULL;int err = 0;err = spl_mmc_find_device(&mmc,bootdev.boot_device);
if (err)return;err = mmc_init(mmc);I thought all this backend specific stuff would be hidden in the FW loader.
The backend supported by FW loader is up to generic file system interface layer. flash driver init is expected done by SPL/U- Boot common init sequence framwork or user. Unfortunately, fw loader need to access flash before init sequence.
This is actually accessing eMMC though , not flash . If we need this huge boilerplate code every time we use the FW loader, than the FW loader needs fixing. I can understand the spl_boot_device() being outside of the FW loader, but not the mmc_init() and co.
I can explore the posibility of adding the flash int mechanism into the fm loader probe function.
What do you mean by "flash int" ? Note that we're talking about eMMC here, not flash. Unless you mean "backend init" by all that, in which case that'd only make sense, thanks.
I means backend init such as MMC, and NAND driver init.
On 11/27/2018 09:55 AM, Chee, Tien Fong wrote:
On Mon, 2018-11-26 at 12:20 +0100, Marek Vasut wrote:
On 11/26/2018 11:10 AM, Chee, Tien Fong wrote:
On Fri, 2018-11-23 at 13:31 +0100, Marek Vasut wrote:
On 11/23/2018 10:51 AM, Chee, Tien Fong wrote:
On Wed, 2018-11-21 at 15:19 +0100, Marek Vasut wrote:
On 11/21/2018 11:41 AM, tien.fong.chee@intel.com wrote: > > > > 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 > --- > arch/arm/mach-socfpga/spl_a10.c | 49 > ++++++++++++++++++++++++++++++++++++++- > common/spl/spl_mmc.c | 2 +- > include/mmc.h | 1 + > 3 files changed, 50 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/mach-socfpga/spl_a10.c > b/arch/arm/mach- > socfpga/spl_a10.c > index 3ea64f7..67a4fac 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-2018 Altera Corporation <www.altera > .com >> >> > */ > > #include <common.h> > @@ -23,6 +23,10 @@ > #include <fdtdec.h> > #include <watchdog.h> > #include <asm/arch/pinmux.h> > +#include <asm/arch/fpga_manager.h> > +#include <mmc.h> > + > +#define RBF 0 > > DECLARE_GLOBAL_DATA_PTR; > > @@ -73,6 +77,49 @@ void spl_board_init(void) > WATCHDOG_RESET(); > > arch_early_init_r(); > + > + /* If the full FPGA is already loaded, ie.from > EPCQ, > config fpga pins */ > + if (is_fpgamgr_user_mode()) { > + config_pins(gd->fdt_blob, "shared"); > + config_pins(gd->fdt_blob, "fpga"); > + } else if (!is_fpgamgr_early_user_mode()) { > + /* Program IOSSM(early IO release) or full > FPGA */ > + fpga_fs_info fpga_fsinfo; > + char buf[16 * 1024] > __aligned(ARCH_DMA_MINALIGN); > + struct spl_boot_device bootdev; > + int len = 0; > + > + bootdev.boot_device = spl_boot_device(); > + > + /* Init MMC driver before reading FPGA > bitstream > from flash */ > + if (bootdev.boot_device == > BOOT_DEVICE_MMC1) { > + struct mmc *mmc = NULL; > + int err = 0; > + > + err = spl_mmc_find_device(&mmc, > bootdev.boot_device); > + if (err) > + return; > + > + err = mmc_init(mmc); I thought all this backend specific stuff would be hidden in the FW loader.
The backend supported by FW loader is up to generic file system interface layer. flash driver init is expected done by SPL/U- Boot common init sequence framwork or user. Unfortunately, fw loader need to access flash before init sequence.
This is actually accessing eMMC though , not flash . If we need this huge boilerplate code every time we use the FW loader, than the FW loader needs fixing. I can understand the spl_boot_device() being outside of the FW loader, but not the mmc_init() and co.
I can explore the posibility of adding the flash int mechanism into the fm loader probe function.
What do you mean by "flash int" ? Note that we're talking about eMMC here, not flash. Unless you mean "backend init" by all that, in which case that'd only make sense, thanks.
I means backend init such as MMC, and NAND driver init.
Right, backend init, that should be part of the FW loader.
From: Tien Fong Chee tien.fong.chee@intel.com
Bundle U-Boot fitImage containing U-Boot and FPGA bitstream into the u-boot-with-spl.sfp on Arria10. This lets U-Boot operate in a very similar fashion to Gen5, where the U-Boot binary got loaded by the SPL from a uImage concatenated at the end of the SPL SFP image. On Gen10, the U-Boot is in fitImage which contains the FPGA bitstream as well. In this case, the SPL can load the FPGA bitstream first and load the U-Boot afterward in the same manner. This is nonetheless a stopgap measure until there is a proper firmware loader in U-Boot.
Signed-off-by: Marek Vasut marex@denx.de Signed-off-by: Tien Fong Chee tien.fong.chee@intel.com --- Makefile | 9 +++++++-- include/configs/socfpga_common.h | 4 ++++ 2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile index a55915d..4ecc19d 100644 --- a/Makefile +++ b/Makefile @@ -1212,9 +1212,14 @@ ifneq ($(CONFIG_ARCH_SOCFPGA),) quiet_cmd_socboot = SOCBOOT $@ cmd_socboot = cat spl/u-boot-spl.sfp spl/u-boot-spl.sfp \ spl/u-boot-spl.sfp spl/u-boot-spl.sfp \ - u-boot.img > $@ || rm -f $@ + $2 > $@ || rm -f $@ +ifdef CONFIG_TARGET_SOCFPGA_ARRIA10 +u-boot-with-spl.sfp: spl/u-boot-spl.sfp u-boot.itb FORCE + $(call if_changed,socboot,u-boot.itb) +else u-boot-with-spl.sfp: spl/u-boot-spl.sfp u-boot.img FORCE - $(call if_changed,socboot) + $(call if_changed,socboot,u-boot.img) +endif endif
ifeq ($(CONFIG_MPC85xx)$(CONFIG_OF_SEPARATE),yy) diff --git a/include/configs/socfpga_common.h b/include/configs/socfpga_common.h index bd8f5c8..ffdc6eb 100644 --- a/include/configs/socfpga_common.h +++ b/include/configs/socfpga_common.h @@ -268,7 +268,11 @@ unsigned int cm_get_qspi_controller_clk_hz(void); /* SPL SDMMC boot support */ #ifdef CONFIG_SPL_MMC_SUPPORT #if defined(CONFIG_SPL_FAT_SUPPORT) || defined(CONFIG_SPL_EXT_SUPPORT) +#if CONFIG_SPL_FIT +#define CONFIG_SPL_FS_LOAD_PAYLOAD_NAME "u-boot.itb" +#else #define CONFIG_SPL_FS_LOAD_PAYLOAD_NAME "u-boot-dtb.img" +#endif #define CONFIG_SYS_MMCSD_FS_BOOT_PARTITION 1 #endif #else
On 11/21/2018 11:41 AM, tien.fong.chee@intel.com wrote:
From: Tien Fong Chee tien.fong.chee@intel.com
Did you change Author:ship of the patch ?
Bundle U-Boot fitImage containing U-Boot and FPGA bitstream into the u-boot-with-spl.sfp on Arria10. This lets U-Boot operate in a very similar fashion to Gen5, where the U-Boot binary got loaded by the SPL from a uImage concatenated at the end of the SPL SFP image. On Gen10, the U-Boot is in fitImage which contains the FPGA bitstream as well. In this case, the SPL can load the FPGA bitstream first and load the U-Boot afterward in the same manner. This is nonetheless a stopgap measure until there is a proper firmware loader in U-Boot.
Right, this is a stopgap measure until FW loader is present. Why is this patch needed at all in this series ?
Signed-off-by: Marek Vasut marex@denx.de Signed-off-by: Tien Fong Chee tien.fong.chee@intel.com
Makefile | 9 +++++++-- include/configs/socfpga_common.h | 4 ++++ 2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile index a55915d..4ecc19d 100644 --- a/Makefile +++ b/Makefile @@ -1212,9 +1212,14 @@ ifneq ($(CONFIG_ARCH_SOCFPGA),) quiet_cmd_socboot = SOCBOOT $@ cmd_socboot = cat spl/u-boot-spl.sfp spl/u-boot-spl.sfp \ spl/u-boot-spl.sfp spl/u-boot-spl.sfp \
u-boot.img > $@ || rm -f $@
$2 > $@ || rm -f $@+ifdef CONFIG_TARGET_SOCFPGA_ARRIA10 +u-boot-with-spl.sfp: spl/u-boot-spl.sfp u-boot.itb FORCE
- $(call if_changed,socboot,u-boot.itb)
+else u-boot-with-spl.sfp: spl/u-boot-spl.sfp u-boot.img FORCE
- $(call if_changed,socboot)
- $(call if_changed,socboot,u-boot.img)
+endif endif
ifeq ($(CONFIG_MPC85xx)$(CONFIG_OF_SEPARATE),yy) diff --git a/include/configs/socfpga_common.h b/include/configs/socfpga_common.h index bd8f5c8..ffdc6eb 100644 --- a/include/configs/socfpga_common.h +++ b/include/configs/socfpga_common.h @@ -268,7 +268,11 @@ unsigned int cm_get_qspi_controller_clk_hz(void); /* SPL SDMMC boot support */ #ifdef CONFIG_SPL_MMC_SUPPORT #if defined(CONFIG_SPL_FAT_SUPPORT) || defined(CONFIG_SPL_EXT_SUPPORT) +#if CONFIG_SPL_FIT +#define CONFIG_SPL_FS_LOAD_PAYLOAD_NAME "u-boot.itb" +#else #define CONFIG_SPL_FS_LOAD_PAYLOAD_NAME "u-boot-dtb.img" +#endif #define CONFIG_SYS_MMCSD_FS_BOOT_PARTITION 1 #endif #else
On Wed, 2018-11-21 at 15:21 +0100, Marek Vasut wrote:
On 11/21/2018 11:41 AM, tien.fong.chee@intel.com wrote:
From: Tien Fong Chee tien.fong.chee@intel.com
Did you change Author:ship of the patch ?
Bundle U-Boot fitImage containing U-Boot and FPGA bitstream into the u-boot-with-spl.sfp on Arria10. This lets U-Boot operate in a very similar fashion to Gen5, where the U-Boot binary got loaded by the SPL from a uImage concatenated at the end of the SPL SFP image. On Gen10, the U-Boot is in fitImage which contains the FPGA bitstream as well. In this case, the SPL can load the FPGA bitstream first and load the U-Boot afterward in the same manner. This is nonetheless a stopgap measure until there is a proper firmware loader in U-Boot.
Right, this is a stopgap measure until FW loader is present. Why is this patch needed at all in this series ?
This patch is cherry picked from the sdmmc_next custodian, so this patch is required for generating FIT image. I can remove the stopgap comment to avoid confusing.
Signed-off-by: Marek Vasut marex@denx.de Signed-off-by: Tien Fong Chee tien.fong.chee@intel.com
Makefile | 9 +++++++-- include/configs/socfpga_common.h | 4 ++++ 2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile index a55915d..4ecc19d 100644 --- a/Makefile +++ b/Makefile @@ -1212,9 +1212,14 @@ ifneq ($(CONFIG_ARCH_SOCFPGA),) quiet_cmd_socboot = SOCBOOT $@ cmd_socboot = cat spl/u-boot-spl.sfp spl/u-boot-spl.sfp \ spl/u-boot-spl.sfp spl/u-boot-spl.sfp \
u-boot.img > $@ || rm -f $@
$2 > $@ || rm -f $@+ifdef CONFIG_TARGET_SOCFPGA_ARRIA10 +u-boot-with-spl.sfp: spl/u-boot-spl.sfp u-boot.itb FORCE
- $(call if_changed,socboot,u-boot.itb)
+else u-boot-with-spl.sfp: spl/u-boot-spl.sfp u-boot.img FORCE
- $(call if_changed,socboot)
- $(call if_changed,socboot,u-boot.img)
+endif endif ifeq ($(CONFIG_MPC85xx)$(CONFIG_OF_SEPARATE),yy) diff --git a/include/configs/socfpga_common.h b/include/configs/socfpga_common.h index bd8f5c8..ffdc6eb 100644 --- a/include/configs/socfpga_common.h +++ b/include/configs/socfpga_common.h @@ -268,7 +268,11 @@ unsigned int cm_get_qspi_controller_clk_hz(void); /* SPL SDMMC boot support */ #ifdef CONFIG_SPL_MMC_SUPPORT #if defined(CONFIG_SPL_FAT_SUPPORT) || defined(CONFIG_SPL_EXT_SUPPORT) +#if CONFIG_SPL_FIT +#define CONFIG_SPL_FS_LOAD_PAYLOAD_NAME "u- boot.itb" +#else #define CONFIG_SPL_FS_LOAD_PAYLOAD_NAME "u-boot- dtb.img" +#endif #define CONFIG_SYS_MMCSD_FS_BOOT_PARTITION 1 #endif #else
On 11/23/2018 10:54 AM, Chee, Tien Fong wrote:
On Wed, 2018-11-21 at 15:21 +0100, Marek Vasut wrote:
On 11/21/2018 11:41 AM, tien.fong.chee@intel.com wrote:
From: Tien Fong Chee tien.fong.chee@intel.com
Did you change Author:ship of the patch ?
I believe you did, so please fix that.
Bundle U-Boot fitImage containing U-Boot and FPGA bitstream into the u-boot-with-spl.sfp on Arria10. This lets U-Boot operate in a very similar fashion to Gen5, where the U-Boot binary got loaded by the SPL from a uImage concatenated at the end of the SPL SFP image. On Gen10, the U-Boot is in fitImage which contains the FPGA bitstream as well. In this case, the SPL can load the FPGA bitstream first and load the U-Boot afterward in the same manner. This is nonetheless a stopgap measure until there is a proper firmware loader in U-Boot.
Right, this is a stopgap measure until FW loader is present. Why is this patch needed at all in this series ?
This patch is cherry picked from the sdmmc_next custodian, so this patch is required for generating FIT image. I can remove the stopgap comment to avoid confusing.
But why is this patch needed at all ? You use the firmware loader to load the FPGA bitstream. Where does the fitImage come into play ?
The fitImage was used to circumvent the missing FW loader, when I needed to load multiple files (bitstream and u-boot binary). Now there is no such requirement anymore, so the entire fitImage machinery is probably not needed ?
On Fri, 2018-11-23 at 13:40 +0100, Marek Vasut wrote:
On 11/23/2018 10:54 AM, Chee, Tien Fong wrote:
On Wed, 2018-11-21 at 15:21 +0100, Marek Vasut wrote:
On 11/21/2018 11:41 AM, tien.fong.chee@intel.com wrote:
From: Tien Fong Chee tien.fong.chee@intel.com
Did you change Author:ship of the patch ?
I believe you did, so please fix that.
Very sorry. I din't realize the author name was changed.
Bundle U-Boot fitImage containing U-Boot and FPGA bitstream into the u-boot-with-spl.sfp on Arria10. This lets U-Boot operate in a very similar fashion to Gen5, where the U-Boot binary got loaded by the SPL from a uImage concatenated at the end of the SPL SFP image. On Gen10, the U-Boot is in fitImage which contains the FPGA bitstream as well. In this case, the SPL can load the FPGA bitstream first and load the U-Boot afterward in the same manner. This is nonetheless a stopgap measure until there is a proper firmware loader in U- Boot.
Right, this is a stopgap measure until FW loader is present. Why is this patch needed at all in this series ?
This patch is cherry picked from the sdmmc_next custodian, so this patch is required for generating FIT image. I can remove the stopgap comment to avoid confusing.
But why is this patch needed at all ? You use the firmware loader to load the FPGA bitstream. Where does the fitImage come into play ?
The fitImage was used to circumvent the missing FW loader, when I needed to load multiple files (bitstream and u-boot binary). Now there is no such requirement anymore, so the entire fitImage machinery is probably not needed ?
Loading issue is not the reason we choose the fitImage. We choose it because it allows more flexibility in handling various type images, especially it allows user more choices to enhance integrity and security protection.
Although we plan to use fitImage as our default implementation, but this series of patches are still allow fw loading individual bitstream image in filesystem partition. So, it is up to user to made the choice.
On 11/26/2018 11:30 AM, Chee, Tien Fong wrote:
On Fri, 2018-11-23 at 13:40 +0100, Marek Vasut wrote:
On 11/23/2018 10:54 AM, Chee, Tien Fong wrote:
On Wed, 2018-11-21 at 15:21 +0100, Marek Vasut wrote:
On 11/21/2018 11:41 AM, tien.fong.chee@intel.com wrote:
From: Tien Fong Chee tien.fong.chee@intel.com
Did you change Author:ship of the patch ?
I believe you did, so please fix that.
Very sorry. I din't realize the author name was changed.
Please be careful next time.
Bundle U-Boot fitImage containing U-Boot and FPGA bitstream into the u-boot-with-spl.sfp on Arria10. This lets U-Boot operate in a very similar fashion to Gen5, where the U-Boot binary got loaded by the SPL from a uImage concatenated at the end of the SPL SFP image. On Gen10, the U-Boot is in fitImage which contains the FPGA bitstream as well. In this case, the SPL can load the FPGA bitstream first and load the U-Boot afterward in the same manner. This is nonetheless a stopgap measure until there is a proper firmware loader in U- Boot.
Right, this is a stopgap measure until FW loader is present. Why is this patch needed at all in this series ?
This patch is cherry picked from the sdmmc_next custodian, so this patch is required for generating FIT image. I can remove the stopgap comment to avoid confusing.
But why is this patch needed at all ? You use the firmware loader to load the FPGA bitstream. Where does the fitImage come into play ?
The fitImage was used to circumvent the missing FW loader, when I needed to load multiple files (bitstream and u-boot binary). Now there is no such requirement anymore, so the entire fitImage machinery is probably not needed ?
Loading issue is not the reason we choose the fitImage. We choose it because it allows more flexibility in handling various type images, especially it allows user more choices to enhance integrity and security protection.
Do you need to load multiple images at all ? Do you need the extra flexibility or does it only bloat and slow down the boot process for no benefit at all? If a user needs it, they can enable it, but do we need it by default ?
Although we plan to use fitImage as our default implementation, but this series of patches are still allow fw loading individual bitstream image in filesystem partition. So, it is up to user to made the choice.
Right, so is the fitImage needed at all ?
On Mon, 2018-11-26 at 12:22 +0100, Marek Vasut wrote:
On 11/26/2018 11:30 AM, Chee, Tien Fong wrote:
On Fri, 2018-11-23 at 13:40 +0100, Marek Vasut wrote:
On 11/23/2018 10:54 AM, Chee, Tien Fong wrote:
On Wed, 2018-11-21 at 15:21 +0100, Marek Vasut wrote:
On 11/21/2018 11:41 AM, tien.fong.chee@intel.com wrote:
From: Tien Fong Chee tien.fong.chee@intel.com
Did you change Author:ship of the patch ?
I believe you did, so please fix that.
Very sorry. I din't realize the author name was changed.
Please be careful next time.
Sure.
Bundle U-Boot fitImage containing U-Boot and FPGA bitstream into the u-boot-with-spl.sfp on Arria10. This lets U-Boot operate in a very similar fashion to Gen5, where the U-Boot binary got loaded by the SPL from a uImage concatenated at the end of the SPL SFP image. On Gen10, the U-Boot is in fitImage which contains the FPGA bitstream as well. In this case, the SPL can load the FPGA bitstream first and load the U-Boot afterward in the same manner. This is nonetheless a stopgap measure until there is a proper firmware loader in U- Boot.
Right, this is a stopgap measure until FW loader is present. Why is this patch needed at all in this series ?
This patch is cherry picked from the sdmmc_next custodian, so this patch is required for generating FIT image. I can remove the stopgap comment to avoid confusing.
But why is this patch needed at all ? You use the firmware loader to load the FPGA bitstream. Where does the fitImage come into play ?
The fitImage was used to circumvent the missing FW loader, when I needed to load multiple files (bitstream and u-boot binary). Now there is no such requirement anymore, so the entire fitImage machinery is probably not needed ?
Loading issue is not the reason we choose the fitImage. We choose it because it allows more flexibility in handling various type images, especially it allows user more choices to enhance integrity and security protection.
Do you need to load multiple images at all ? Do you need the extra flexibility or does it only bloat and slow down the boot process for no benefit at all? If a user needs it, they can enable it, but do we need it by default ?
Okay, then we add in the fitImage support and let user to enable it. So, without CONFIG_SPL_FIT is defined, then the boot process would be with individual files such as u-boot-dtb.img instead of u-boot.itb.
Although we plan to use fitImage as our default implementation, but this series of patches are still allow fw loading individual bitstream image in filesystem partition. So, it is up to user to made the choice.
Right, so is the fitImage needed at all ?
On 11/27/2018 10:00 AM, Chee, Tien Fong wrote:
On Mon, 2018-11-26 at 12:22 +0100, Marek Vasut wrote:
On 11/26/2018 11:30 AM, Chee, Tien Fong wrote:
On Fri, 2018-11-23 at 13:40 +0100, Marek Vasut wrote:
On 11/23/2018 10:54 AM, Chee, Tien Fong wrote:
On Wed, 2018-11-21 at 15:21 +0100, Marek Vasut wrote:
On 11/21/2018 11:41 AM, tien.fong.chee@intel.com wrote: > > > > From: Tien Fong Chee tien.fong.chee@intel.com Did you change Author:ship of the patch ?
I believe you did, so please fix that.
Very sorry. I din't realize the author name was changed.
Please be careful next time.
Sure.
> > > Bundle U-Boot fitImage containing U-Boot and FPGA bitstream > into > the > u-boot-with-spl.sfp on Arria10. This lets U-Boot operate in > a > very > similar fashion to Gen5, where the U-Boot binary got loaded > by > the > SPL from a uImage concatenated at the end of the SPL SFP > image. > On > Gen10, the U-Boot is in fitImage which contains the FPGA > bitstream > as well. In this case, the SPL can load the FPGA bitstream > first > and > load the U-Boot afterward in the same manner. This is > nonetheless a > stopgap measure until there is a proper firmware loader in > U- > Boot. Right, this is a stopgap measure until FW loader is present. Why is this patch needed at all in this series ?
This patch is cherry picked from the sdmmc_next custodian, so this patch is required for generating FIT image. I can remove the stopgap comment to avoid confusing.
But why is this patch needed at all ? You use the firmware loader to load the FPGA bitstream. Where does the fitImage come into play ?
The fitImage was used to circumvent the missing FW loader, when I needed to load multiple files (bitstream and u-boot binary). Now there is no such requirement anymore, so the entire fitImage machinery is probably not needed ?
Loading issue is not the reason we choose the fitImage. We choose it because it allows more flexibility in handling various type images, especially it allows user more choices to enhance integrity and security protection.
Do you need to load multiple images at all ? Do you need the extra flexibility or does it only bloat and slow down the boot process for no benefit at all? If a user needs it, they can enable it, but do we need it by default ?
Okay, then we add in the fitImage support and let user to enable it. So, without CONFIG_SPL_FIT is defined, then the boot process would be with individual files such as u-boot-dtb.img instead of u-boot.itb.
Yes, so all these fitImage patches can be dropped for now ?
On Tue, 2018-11-27 at 13:09 +0100, Marek Vasut wrote:
On 11/27/2018 10:00 AM, Chee, Tien Fong wrote:
On Mon, 2018-11-26 at 12:22 +0100, Marek Vasut wrote:
On 11/26/2018 11:30 AM, Chee, Tien Fong wrote:
On Fri, 2018-11-23 at 13:40 +0100, Marek Vasut wrote:
On 11/23/2018 10:54 AM, Chee, Tien Fong wrote:
On Wed, 2018-11-21 at 15:21 +0100, Marek Vasut wrote: > > > > On 11/21/2018 11:41 AM, tien.fong.chee@intel.com wrote: > > > > > > > > > > From: Tien Fong Chee tien.fong.chee@intel.com > Did you change Author:ship of the patch ?
I believe you did, so please fix that.
Very sorry. I din't realize the author name was changed.
Please be careful next time.
Sure.
> > > > > > > > > > > Bundle U-Boot fitImage containing U-Boot and FPGA > > bitstream > > into > > the > > u-boot-with-spl.sfp on Arria10. This lets U-Boot > > operate in > > a > > very > > similar fashion to Gen5, where the U-Boot binary got > > loaded > > by > > the > > SPL from a uImage concatenated at the end of the SPL > > SFP > > image. > > On > > Gen10, the U-Boot is in fitImage which contains the > > FPGA > > bitstream > > as well. In this case, the SPL can load the FPGA > > bitstream > > first > > and > > load the U-Boot afterward in the same manner. This is > > nonetheless a > > stopgap measure until there is a proper firmware loader > > in > > U- > > Boot. > Right, this is a stopgap measure until FW loader is > present. > Why > is > this > patch needed at all in this series ? This patch is cherry picked from the sdmmc_next custodian, so this patch is required for generating FIT image. I can remove the stopgap comment to avoid confusing.
But why is this patch needed at all ? You use the firmware loader to load the FPGA bitstream. Where does the fitImage come into play ?
The fitImage was used to circumvent the missing FW loader, when I needed to load multiple files (bitstream and u-boot binary). Now there is no such requirement anymore, so the entire fitImage machinery is probably not needed ?
Loading issue is not the reason we choose the fitImage. We choose it because it allows more flexibility in handling various type images, especially it allows user more choices to enhance integrity and security protection.
Do you need to load multiple images at all ? Do you need the extra flexibility or does it only bloat and slow down the boot process for no benefit at all? If a user needs it, they can enable it, but do we need it by default ?
Okay, then we add in the fitImage support and let user to enable it. So, without CONFIG_SPL_FIT is defined, then the boot process would be with individual files such as u-boot-dtb.img instead of u-boot.itb.
Yes, so all these fitImage patches can be dropped for now ?
This patch can be dropped. But i don't know it is good idea to reserve the patch 5-8, this would be easier for user to enable CONFIG_SPL_FIT in future.
On 11/28/2018 03:43 PM, Chee, Tien Fong wrote:
On Tue, 2018-11-27 at 13:09 +0100, Marek Vasut wrote:
On 11/27/2018 10:00 AM, Chee, Tien Fong wrote:
On Mon, 2018-11-26 at 12:22 +0100, Marek Vasut wrote:
On 11/26/2018 11:30 AM, Chee, Tien Fong wrote:
On Fri, 2018-11-23 at 13:40 +0100, Marek Vasut wrote:
On 11/23/2018 10:54 AM, Chee, Tien Fong wrote: > > > > On Wed, 2018-11-21 at 15:21 +0100, Marek Vasut wrote: >> >> >> >> On 11/21/2018 11:41 AM, tien.fong.chee@intel.com wrote: >>> >>> >>> >>> >>> From: Tien Fong Chee tien.fong.chee@intel.com >> Did you change Author:ship of the patch ? I believe you did, so please fix that.
Very sorry. I din't realize the author name was changed.
Please be careful next time.
Sure.
> > > >> >> >> >>> >>> >>> >>> Bundle U-Boot fitImage containing U-Boot and FPGA >>> bitstream >>> into >>> the >>> u-boot-with-spl.sfp on Arria10. This lets U-Boot >>> operate in >>> a >>> very >>> similar fashion to Gen5, where the U-Boot binary got >>> loaded >>> by >>> the >>> SPL from a uImage concatenated at the end of the SPL >>> SFP >>> image. >>> On >>> Gen10, the U-Boot is in fitImage which contains the >>> FPGA >>> bitstream >>> as well. In this case, the SPL can load the FPGA >>> bitstream >>> first >>> and >>> load the U-Boot afterward in the same manner. This is >>> nonetheless a >>> stopgap measure until there is a proper firmware loader >>> in >>> U- >>> Boot. >> Right, this is a stopgap measure until FW loader is >> present. >> Why >> is >> this >> patch needed at all in this series ? > This patch is cherry picked from the sdmmc_next custodian, > so > this > patch is required for generating FIT image. I can remove > the > stopgap > comment to avoid confusing. But why is this patch needed at all ? You use the firmware loader to load the FPGA bitstream. Where does the fitImage come into play ?
The fitImage was used to circumvent the missing FW loader, when I needed to load multiple files (bitstream and u-boot binary). Now there is no such requirement anymore, so the entire fitImage machinery is probably not needed ?
Loading issue is not the reason we choose the fitImage. We choose it because it allows more flexibility in handling various type images, especially it allows user more choices to enhance integrity and security protection.
Do you need to load multiple images at all ? Do you need the extra flexibility or does it only bloat and slow down the boot process for no benefit at all? If a user needs it, they can enable it, but do we need it by default ?
Okay, then we add in the fitImage support and let user to enable it. So, without CONFIG_SPL_FIT is defined, then the boot process would be with individual files such as u-boot-dtb.img instead of u-boot.itb.
Yes, so all these fitImage patches can be dropped for now ?
This patch can be dropped. But i don't know it is good idea to reserve the patch 5-8, this would be easier for user to enable CONFIG_SPL_FIT in future.
Drop them for now, let's revisit this later.
From: Tien Fong Chee tien.fong.chee@intel.com
Add empty SPL fitImage configuration match. This can be extended if there is ever need to support multiple boards with single SFP image.
Signed-off-by: Marek Vasut marex@denx.de --- arch/arm/mach-socfpga/spl_a10.c | 10 ++++++++++ 1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-socfpga/spl_a10.c b/arch/arm/mach-socfpga/spl_a10.c index 67a4fac..2baeba6 100644 --- a/arch/arm/mach-socfpga/spl_a10.c +++ b/arch/arm/mach-socfpga/spl_a10.c @@ -148,3 +148,13 @@ void board_init_f(ulong dummy) config_dedicated_pins(gd->fdt_blob); WATCHDOG_RESET(); } + +#ifdef CONFIG_SPL_LOAD_FIT +int board_fit_config_name_match(const char *name) +{ + /* Just empty function now - can't decide what to choose */ + debug("%s: %s\n", __func__, name); + + return 0; +} +#endif
On 11/21/2018 11:41 AM, tien.fong.chee@intel.com wrote:
From: Tien Fong Chee tien.fong.chee@intel.com
Add empty SPL fitImage configuration match. This can be extended if there is ever need to support multiple boards with single SFP image.
Signed-off-by: Marek Vasut marex@denx.de
It's missing your SoB line, but again, why is this patch needed ?
arch/arm/mach-socfpga/spl_a10.c | 10 ++++++++++ 1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-socfpga/spl_a10.c b/arch/arm/mach-socfpga/spl_a10.c index 67a4fac..2baeba6 100644 --- a/arch/arm/mach-socfpga/spl_a10.c +++ b/arch/arm/mach-socfpga/spl_a10.c @@ -148,3 +148,13 @@ void board_init_f(ulong dummy) config_dedicated_pins(gd->fdt_blob); WATCHDOG_RESET(); }
+#ifdef CONFIG_SPL_LOAD_FIT +int board_fit_config_name_match(const char *name) +{
- /* Just empty function now - can't decide what to choose */
- debug("%s: %s\n", __func__, name);
- return 0;
+} +#endif
On Wed, 2018-11-21 at 15:21 +0100, Marek Vasut wrote:
On 11/21/2018 11:41 AM, tien.fong.chee@intel.com wrote:
From: Tien Fong Chee tien.fong.chee@intel.com
Add empty SPL fitImage configuration match. This can be extended if there is ever need to support multiple boards with single SFP image.
Signed-off-by: Marek Vasut marex@denx.de
It's missing your SoB line, but again, why is this patch needed ?
This patch i also cherry picked from sdmmc_next custodian, and i didn't made any changes, so i still need SoB? Without this patch, compiling failed with error"/uboot- socfpga/common/common_fit.c:66: undefined reference to `board_fit_config_name_match'"
arch/arm/mach-socfpga/spl_a10.c | 10 ++++++++++ 1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-socfpga/spl_a10.c b/arch/arm/mach- socfpga/spl_a10.c index 67a4fac..2baeba6 100644 --- a/arch/arm/mach-socfpga/spl_a10.c +++ b/arch/arm/mach-socfpga/spl_a10.c @@ -148,3 +148,13 @@ void board_init_f(ulong dummy) config_dedicated_pins(gd->fdt_blob); WATCHDOG_RESET(); }
+#ifdef CONFIG_SPL_LOAD_FIT +int board_fit_config_name_match(const char *name) +{
- /* Just empty function now - can't decide what to choose
*/
- debug("%s: %s\n", __func__, name);
- return 0;
+} +#endif
On 11/23/2018 11:05 AM, Chee, Tien Fong wrote:
On Wed, 2018-11-21 at 15:21 +0100, Marek Vasut wrote:
On 11/21/2018 11:41 AM, tien.fong.chee@intel.com wrote:
From: Tien Fong Chee tien.fong.chee@intel.com
Add empty SPL fitImage configuration match. This can be extended if there is ever need to support multiple boards with single SFP image.
Signed-off-by: Marek Vasut marex@denx.de
It's missing your SoB line, but again, why is this patch needed ?
This patch i also cherry picked from sdmmc_next custodian, and i didn't made any changes, so i still need SoB?
Yes
Without this patch, compiling failed with error"/uboot- socfpga/common/common_fit.c:66: undefined reference to `board_fit_config_name_match'"
Sure, it will fail. Let's continue the discussion at 4/9.
arch/arm/mach-socfpga/spl_a10.c | 10 ++++++++++ 1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-socfpga/spl_a10.c b/arch/arm/mach- socfpga/spl_a10.c index 67a4fac..2baeba6 100644 --- a/arch/arm/mach-socfpga/spl_a10.c +++ b/arch/arm/mach-socfpga/spl_a10.c @@ -148,3 +148,13 @@ void board_init_f(ulong dummy) config_dedicated_pins(gd->fdt_blob); WATCHDOG_RESET(); }
+#ifdef CONFIG_SPL_LOAD_FIT +int board_fit_config_name_match(const char *name) +{
- /* Just empty function now - can't decide what to choose
*/
- debug("%s: %s\n", __func__, name);
- return 0;
+} +#endif
On Fri, 2018-11-23 at 13:34 +0100, Marek Vasut wrote:
On 11/23/2018 11:05 AM, Chee, Tien Fong wrote:
On Wed, 2018-11-21 at 15:21 +0100, Marek Vasut wrote:
On 11/21/2018 11:41 AM, tien.fong.chee@intel.com wrote:
From: Tien Fong Chee tien.fong.chee@intel.com
Add empty SPL fitImage configuration match. This can be extended if there is ever need to support multiple boards with single SFP image.
Signed-off-by: Marek Vasut marex@denx.de
It's missing your SoB line, but again, why is this patch needed ?
This patch i also cherry picked from sdmmc_next custodian, and i didn't made any changes, so i still need SoB?
Yes
Noted.
Without this patch, compiling failed with error"/uboot- socfpga/common/common_fit.c:66: undefined reference to `board_fit_config_name_match'"
Sure, it will fail. Let's continue the discussion at 4/9.
Okay.
arch/arm/mach-socfpga/spl_a10.c | 10 ++++++++++ 1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-socfpga/spl_a10.c b/arch/arm/mach- socfpga/spl_a10.c index 67a4fac..2baeba6 100644 --- a/arch/arm/mach-socfpga/spl_a10.c +++ b/arch/arm/mach-socfpga/spl_a10.c @@ -148,3 +148,13 @@ void board_init_f(ulong dummy) config_dedicated_pins(gd->fdt_blob); WATCHDOG_RESET(); }
+#ifdef CONFIG_SPL_LOAD_FIT +int board_fit_config_name_match(const char *name) +{
- /* Just empty function now - can't decide what to
choose */
- debug("%s: %s\n", __func__, name);
- return 0;
+} +#endif
From: Tien Fong Chee tien.fong.chee@intel.com
Set default DT blob address on A10 SoCDK, since this SoC uses OF separate configuration. The 0xf0000 address is just below the text base and still leaves enough room for the DT to grow.
Signed-off-by: Marek Vasut marex@denx.de Signed-off-by: Tien Fong Chee tien.fong.chee@intel.com --- arch/arm/mach-socfpga/board.c | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-socfpga/board.c b/arch/arm/mach-socfpga/board.c index 7c8c05c..436a8a8 100644 --- a/arch/arm/mach-socfpga/board.c +++ b/arch/arm/mach-socfpga/board.c @@ -86,3 +86,11 @@ int g_dnl_board_usb_cable_connected(void) return 1; } #endif + +#if defined(CONFIG_TARGET_SOCFPGA_ARRIA10) && \ +!defined(CONFIG_SPL_BUILD) && defined(CONFIG_OF_SEPARATE) +void *board_fdt_blob_setup(void) +{ + return (void *)0xf00000; +} +#endif
On 11/21/2018 11:41 AM, tien.fong.chee@intel.com wrote:
From: Tien Fong Chee tien.fong.chee@intel.com
Again, not your patch, please don't change authorship.
http://git.denx.de/?p=u-boot/u-boot-socfpga.git;a=commit;h=eb32e568869b5f91f...
Set default DT blob address on A10 SoCDK, since this SoC uses OF separate configuration. The 0xf0000 address is just below the text base and still leaves enough room for the DT to grow.
Why is this needed ?
Signed-off-by: Marek Vasut marex@denx.de Signed-off-by: Tien Fong Chee tien.fong.chee@intel.com
arch/arm/mach-socfpga/board.c | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-socfpga/board.c b/arch/arm/mach-socfpga/board.c index 7c8c05c..436a8a8 100644 --- a/arch/arm/mach-socfpga/board.c +++ b/arch/arm/mach-socfpga/board.c @@ -86,3 +86,11 @@ int g_dnl_board_usb_cable_connected(void) return 1; } #endif
+#if defined(CONFIG_TARGET_SOCFPGA_ARRIA10) && \ +!defined(CONFIG_SPL_BUILD) && defined(CONFIG_OF_SEPARATE) +void *board_fdt_blob_setup(void) +{
- return (void *)0xf00000;
+} +#endif
On Wed, 2018-11-21 at 15:22 +0100, Marek Vasut wrote:
On 11/21/2018 11:41 AM, tien.fong.chee@intel.com wrote:
From: Tien Fong Chee tien.fong.chee@intel.com
Again, not your patch, please don't change authorship.
http://git.denx.de/?p=u-boot/u-boot-socfpga.git;a=commit;h=eb32e56886 9b5f91fe34efb2642875a8da5f0ebd
Set default DT blob address on A10 SoCDK, since this SoC uses OF separate configuration. The 0xf0000 address is just below the text base and still leaves enough room for the DT to grow.
Why is this needed ?
This patch i also cherry picked from sdmmc_next custodian. I think you put this for FIT implementation? I saw there is load property "0xf00000" defined for fdt in .its.
Signed-off-by: Marek Vasut marex@denx.de Signed-off-by: Tien Fong Chee tien.fong.chee@intel.com
arch/arm/mach-socfpga/board.c | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-socfpga/board.c b/arch/arm/mach- socfpga/board.c index 7c8c05c..436a8a8 100644 --- a/arch/arm/mach-socfpga/board.c +++ b/arch/arm/mach-socfpga/board.c @@ -86,3 +86,11 @@ int g_dnl_board_usb_cable_connected(void) return 1; } #endif
+#if defined(CONFIG_TARGET_SOCFPGA_ARRIA10) && \ +!defined(CONFIG_SPL_BUILD) && defined(CONFIG_OF_SEPARATE) +void *board_fdt_blob_setup(void) +{
- return (void *)0xf00000;
+} +#endif
On 11/23/2018 11:10 AM, Chee, Tien Fong wrote:
On Wed, 2018-11-21 at 15:22 +0100, Marek Vasut wrote:
On 11/21/2018 11:41 AM, tien.fong.chee@intel.com wrote:
From: Tien Fong Chee tien.fong.chee@intel.com
Again, not your patch, please don't change authorship.
http://git.denx.de/?p=u-boot/u-boot-socfpga.git;a=commit;h=eb32e56886 9b5f91fe34efb2642875a8da5f0ebd
Set default DT blob address on A10 SoCDK, since this SoC uses OF separate configuration. The 0xf0000 address is just below the text base and still leaves enough room for the DT to grow.
Why is this needed ?
This patch i also cherry picked from sdmmc_next custodian. I think you put this for FIT implementation? I saw there is load property "0xf00000" defined for fdt in .its.
Thanks for confirming that I am the original author of some of these patches and you just changed that field when submitting them. That's a big no-no.
Let's continue the discussion under patch 4/9.
Signed-off-by: Marek Vasut marex@denx.de Signed-off-by: Tien Fong Chee tien.fong.chee@intel.com
arch/arm/mach-socfpga/board.c | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-socfpga/board.c b/arch/arm/mach- socfpga/board.c index 7c8c05c..436a8a8 100644 --- a/arch/arm/mach-socfpga/board.c +++ b/arch/arm/mach-socfpga/board.c @@ -86,3 +86,11 @@ int g_dnl_board_usb_cable_connected(void) return 1; } #endif
+#if defined(CONFIG_TARGET_SOCFPGA_ARRIA10) && \ +!defined(CONFIG_SPL_BUILD) && defined(CONFIG_OF_SEPARATE) +void *board_fdt_blob_setup(void) +{
- return (void *)0xf00000;
+} +#endif
From: Tien Fong Chee tien.fong.chee@intel.com
Allocate buffers from OCRAM heap for the image headers in SPL on Arria10, since DRAM is not available at that point. This allows U-Boot to load the fitImage header, parse it, extract the FPGA bitstream section from it, program the FPGA and make DRAM available.
Signed-off-by: Marek Vasut marex@denx.de --- arch/arm/mach-socfpga/spl_a10.c | 11 +++++++++++ 1 files changed, 11 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-socfpga/spl_a10.c b/arch/arm/mach-socfpga/spl_a10.c index 2baeba6..7fe9790 100644 --- a/arch/arm/mach-socfpga/spl_a10.c +++ b/arch/arm/mach-socfpga/spl_a10.c @@ -9,6 +9,7 @@ #include <asm/u-boot.h> #include <asm/utils.h> #include <image.h> +#include <malloc.h> #include <asm/arch/reset_manager.h> #include <spl.h> #include <asm/arch/system_manager.h> @@ -157,4 +158,14 @@ int board_fit_config_name_match(const char *name)
return 0; } + +struct image_header *spl_get_load_buffer(int offset, size_t size) +{ + struct image_header *mem = memalign(4, size); + + if (!mem) + hang(); + + return mem; +} #endif
From: Tien Fong Chee tien.fong.chee@intel.com
Add default fitImage file bundling U-Boot and FPGA bitstream for Arria10.
Signed-off-by: Marek Vasut marex@denx.de Signed-off-by: Tien Fong Chee tien.fong.chee@intel.com --- board/altera/arria10-socdk/fit_spl_fpga.its | 54 +++++++++++++++++++++++++++ 1 files changed, 54 insertions(+), 0 deletions(-) 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..7782740 --- /dev/null +++ b/board/altera/arria10-socdk/fit_spl_fpga.its @@ -0,0 +1,54 @@ +/* + * The fitImage source for Arria 10 SoCDK loading FPGA bitstream and U-Boot + * + * Copyright (C) 2018 Marek Vasut marex@denx.de + * + * SPDX-License-Identifier: GPL-2.0+ X11 + */ + +/dts-v1/; + +/ { + description = "FIT image with U-Boot proper and FPGA bitstream"; + #address-cells = <1>; + + images { + uboot { + description = "U-Boot (32-bit)"; + data = /incbin/("../../../u-boot-nodtb.bin"); + type = "standalone"; + os = "U-Boot"; + arch = "arm"; + compression = "none"; + load = <0x01000040>; + entry = <0x01000040>; + }; + + fdt { + description = "Arria10 SoCDK flat device-tree"; + data = /incbin/("../../../u-boot.dtb"); + type = "flat_dt"; + arch = "arm"; + compression = "none"; + load = <0x00f00000>; + }; + + fpga { + description = "FPGA bitstream"; + data = /incbin/("../../../ghrd_10as066n2.core.rbf"); + type = "fpga"; + compression = "none"; + load = <0x1000>; + }; + }; + + configurations { + default = "conf"; + conf { + description = "Altera Arria10 SoCDK"; + loadables = "uboot"; + fdt = "fdt"; + fpga = "fpga"; + }; + }; +};
From: Tien Fong Chee tien.fong.chee@intel.com
Update the default configuration file to enable the necessary functionality the get the kit working. That includes fitImage loading in SPL, SPL SD/MMC support, USB, I2C.
Signed-off-by: Marek Vasut marex@denx.de Signed-off-by: Tien Fong Chee tien.fong.chee@intel.com --- configs/socfpga_arria10_defconfig | 42 +++++++++++++++++++++++++++++++----- 1 files changed, 36 insertions(+), 6 deletions(-)
diff --git a/configs/socfpga_arria10_defconfig b/configs/socfpga_arria10_defconfig index f88910c..86f078b 100644 --- a/configs/socfpga_arria10_defconfig +++ b/configs/socfpga_arria10_defconfig @@ -1,35 +1,48 @@ CONFIG_ARM=y CONFIG_ARCH_SOCFPGA=y CONFIG_SYS_TEXT_BASE=0x01000040 -CONFIG_SYS_MALLOC_F_LEN=0x2000 +CONFIG_SYS_MALLOC_F_LEN=0x8000 CONFIG_TARGET_SOCFPGA_ARRIA10_SOCDK=y CONFIG_SPL=y CONFIG_IDENT_STRING="socfpga_arria10" CONFIG_DISTRO_DEFAULTS=y CONFIG_NR_DRAM_BANKS=1 +CONFIG_FIT=y +CONFIG_FIT_VERBOSE=y +CONFIG_SPL_LOAD_FIT=y +CONFIG_SPL_FIT_SOURCE="board/altera/arria10-socdk/fit_spl_fpga.its" 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_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x800 CONFIG_SPL_FPGA_SUPPORT=y -CONFIG_SPL_SPI_LOAD=y CONFIG_CMD_ASKENV=y CONFIG_CMD_GREPENV=y +CONFIG_CMD_DFU=y # CONFIG_CMD_FLASH is not set CONFIG_CMD_GPIO=y +CONFIG_CMD_I2C=y CONFIG_CMD_MMC=y +CONFIG_CMD_SF=y +CONFIG_CMD_SPI=y +CONFIG_CMD_USB=y +CONFIG_CMD_USB_MASS_STORAGE=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 CONFIG_SPL_DM=y CONFIG_SPL_DM_SEQ_ALIAS=y +CONFIG_DFU_MMC=y CONFIG_SPL_DM_MMC=y CONFIG_SPL_MMC_SUPPORT=y CONFIG_SPL_FS_SUPPORT=y @@ -41,13 +54,30 @@ CONFIG_FS_LOADER=y CONFIG_FPGA_SOCFPGA=y CONFIG_DM_GPIO=y CONFIG_DWAPB_GPIO=y +CONFIG_SYS_I2C_DW=y CONFIG_DM_MMC=y CONFIG_MTD_DEVICE=y +CONFIG_MTD_PARTITIONS=y +CONFIG_MMC_DW=y +CONFIG_SPI_FLASH=y +CONFIG_SPI_FLASH_BAR=y +CONFIG_SPI_FLASH_SPANSION=y +CONFIG_SPI_FLASH_STMICRO=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 +CONFIG_DESIGNWARE_SPI=y +CONFIG_USB=y +CONFIG_DM_USB=y +CONFIG_USB_DWC2=y +CONFIG_USB_STORAGE=y +CONFIG_USB_GADGET=y +CONFIG_USB_GADGET_DWC2_OTG=y +CONFIG_USB_GADGET_DOWNLOAD=y
participants (3)
-
Chee, Tien Fong -
Marek Vasut -
tien.fong.chee＠intel.com