Add support for loading FPGA bitstream

PATCH New tien.fong.chee at intel.com 3 participants 55 messages Nov 21, 2018 – Dec 14, 2018

Subsystems (5)

Arm (32-bit) Arm sub-architectures Kbuild FPGA manager framework +1 more subsystems

lore

Share & tools

lore

tien.fong.chee at intel.com

author

2018-11-21 10:41 UTC #0 lore raw

cover Add support for loading FPGA bitstream

From: Tien Fong Chee <[email protected]>

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

Signature · 1 line

1.7.7.4

tien.fong.chee at intel.com

author

2018-11-21 10:41 UTC #1 lore raw

PATCH 1/9 ARM: socfpga: Description on FPGA bitstream type and file name for Arria 10

From: Tien Fong Chee <[email protected]>

This patch adds description on properties about file name used for both peripheral bitstream and core bitstream.

Signed-off-by Tien Fong Chee <[email protected]>

---
 .../fpga/altera-socfpga-a10-fpga-mgr.txt           |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

doc/device-tree-bindings/fpga/altera-socfpga-a10-fpga-mgr.txt +6 -0

index 2fd8e7a..010322a 100644
@@ -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";
 	};

Signature · 1 line

1.7.7.4

Marek Vasut

2018-11-21 14:11 UTC #2 lore raw

PATCH 1/9 ARM: socfpga: Description on FPGA bitstream type and file name for Arria 10

Quoted 15 lines · tien.fong.chee at intel.com

From: Tien Fong Chee <[email protected]>
This patch adds description on properties about file name used for both peripheral bitstream and core bitstream.
Signed-off-by Tien Fong Chee <[email protected]>
---
 .../fpga/altera-socfpga-a10-fpga-mgr.txt           |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)
doc/device-tree-bindings/fpga/altera-socfpga-a10-fpga-mgr.txt +0 -0
index 2fd8e7a..010322a 100644

doc/device-tree-bindings/fpga/altera-socfpga-a10-fpga-mgr.txt Required properties: · line 13

Quoted 7 lines

doc/device-tree-bindings/fpga/altera-socfpga-a10-fpga-mgr.txt +4 -0

@@ -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 ?

Quoted 10 lines

doc/device-tree-bindings/fpga/altera-socfpga-a10-fpga-mgr.txt +2 -0

 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";
 	};

Signature · 2 lines

Best regards,
Marek Vasut

Chee

2018-11-23 09:19 UTC #3 lore raw

PATCH 1/9 ARM: socfpga: Description on FPGA bitstream type and file name for Arria 10

Quoted 25 lines · Marek Vasut

Quoted 24 lines · tien.fong.chee at intel.com
From: Tien Fong Chee <[email protected]>
This patch adds description on properties about file name used for both
peripheral bitstream and core bitstream.
Signed-off-by Tien Fong Chee <[email protected]>
---
.../fpga/altera-socfpga-a10-fpga-mgr.txt | 6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)
(quoted diff) +0 -0
mgr.txt b/doc/device-tree-bindings/fpga/altera-socfpga-a10-fpga- mgr.txt
index 2fd8e7a..010322a 100644
doc/device-tree-bindings/fpga/altera-socfpga-a10-fpga-mgr.txt +0 -0
@@ -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

Quoted 8 lines

Quoted 7 lines
which is used
doc/device-tree-bindings/fpga/altera-socfpga-a10-fpga-mgr.txt +1 -0
+			  to initialize FPGA IOs, PLL, IO48 and
DDR.
+- altr,bitstream_core : File name for core raw binary which contains FPGA design
doc/device-tree-bindings/fpga/altera-socfpga-a10-fpga-mgr.txt +1 -0
+			which is used to program FPGA CRAM and
ERAM.
bitstream- instead of bitstream_

Noted.

Quoted 2 lines

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

Quoted 15 lines

Quoted 14 lines

Example:

doc/device-tree-bindings/fpga/altera-socfpga-a10-fpga-mgr.txt +2 -0

@@ -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";
};

Marek Vasut

2018-11-23 12:23 UTC #4 lore raw

PATCH 1/9 ARM: socfpga: Description on FPGA bitstream type and file name for Arria 10

Quoted 42 lines · Tien Fong

Quoted 25 lines · Marek Vasut
Quoted 24 lines · tien.fong.chee at intel.com
From: Tien Fong Chee <[email protected]>
This patch adds description on properties about file name used for both
peripheral bitstream and core bitstream.
Signed-off-by Tien Fong Chee <[email protected]>
---
.../fpga/altera-socfpga-a10-fpga-mgr.txt | 6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)
(quoted diff) +0 -0
mgr.txt b/doc/device-tree-bindings/fpga/altera-socfpga-a10-fpga- mgr.txt
index 2fd8e7a..010322a 100644
doc/device-tree-bindings/fpga/altera-socfpga-a10-fpga-mgr.txt +0 -0
@@ -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
Quoted 8 lines
Quoted 7 lines
which is used
doc/device-tree-bindings/fpga/altera-socfpga-a10-fpga-mgr.txt +1 -0
+			  to initialize FPGA IOs, PLL, IO48 and
DDR.
+- altr,bitstream_core : File name for core raw binary which contains FPGA design
doc/device-tree-bindings/fpga/altera-socfpga-a10-fpga-mgr.txt +1 -0
+			which is used to program FPGA CRAM and
ERAM.
bitstream- instead of bitstream_
Noted.
Quoted 2 lines
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.

Quoted 5 lines

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 ...

Signature · 2 lines

Best regards,
Marek Vasut

Chee

2018-11-26 09:44 UTC #5 lore raw

PATCH 1/9 ARM: socfpga: Description on FPGA bitstream type and file name for Arria 10

Quoted 58 lines · Marek Vasut

Quoted 57 lines · Tien Fong
Quoted 36 lines · Marek Vasut
Quoted 34 lines · tien.fong.chee at intel.com
From: Tien Fong Chee <[email protected]>
This patch adds description on properties about file name used for
both
peripheral bitstream and core bitstream.
Signed-off-by Tien Fong Chee <[email protected]>
---
.../fpga/altera-socfpga-a10-fpga-mgr.txt | 6
++++++
 1 files changed, 6 insertions(+), 0 deletions(-)
(quoted diff) +0 -0
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
(quoted diff) +0 -0
@@ -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
Quoted 8 lines
Quoted 7 lines
(quoted diff) +1 -0
+			  to initialize FPGA IOs, PLL, IO48
and
DDR.
+- altr,bitstream_core : File name for core raw binary which contains FPGA design
(quoted diff) +1 -0
+			which is used to program FPGA CRAM and
ERAM.
bitstream- instead of bitstream_
Noted.
Quoted 3 lines
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.

Quoted 8 lines

Quoted 7 lines
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?

Quoted 1 line

Marek Vasut

2018-11-26 11:15 UTC #6 lore raw

PATCH 1/9 ARM: socfpga: Description on FPGA bitstream type and file name for Arria 10

Quoted 66 lines · Tien Fong

Quoted 65 lines · Marek Vasut
Quoted 64 lines · Tien Fong
Quoted 36 lines · Marek Vasut
Quoted 34 lines · tien.fong.chee at intel.com
From: Tien Fong Chee <[email protected]>
This patch adds description on properties about file name used for
both
peripheral bitstream and core bitstream.
Signed-off-by Tien Fong Chee <[email protected]>
---
.../fpga/altera-socfpga-a10-fpga-mgr.txt | 6
++++++
 1 files changed, 6 insertions(+), 0 deletions(-)
(quoted diff) +0 -0
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
(quoted diff) +0 -0
@@ -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
Quoted 8 lines
Quoted 7 lines
(quoted diff) +1 -0
+			  to initialize FPGA IOs, PLL, IO48
and
DDR.
+- altr,bitstream_core : File name for core raw binary which contains FPGA design
(quoted diff) +1 -0
+			which is used to program FPGA CRAM and
ERAM.
bitstream- instead of bitstream_
Noted.
Quoted 3 lines
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… …

Quoted 3 lines

Quoted 1 line
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.

Signature · 2 lines

Best regards,
Marek Vasut

Chee

2018-11-27 08:45 UTC #7 lore raw

PATCH 1/9 ARM: socfpga: Description on FPGA bitstream type and file name for Arria 10

Quoted 85 lines · Marek Vasut

Quoted 84 lines · Tien Fong
Quoted 82 lines · Marek Vasut
Quoted 80 lines · Tien Fong
Quoted 47 lines · Marek Vasut
Quoted 44 lines · tien.fong.chee at intel.com
From: Tien Fong Chee <[email protected]>
This patch adds description on properties about file name used
for
both
peripheral bitstream and core bitstream.
Signed-off-by Tien Fong Chee <[email protected]>
---
.../fpga/altera-socfpga-a10-fpga-mgr.txt | 6
++++++
 1 files changed, 6 insertions(+), 0 deletions(-)
(quoted diff) +0 -0
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
(quoted diff) +0 -0
@@ -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
(quoted diff) +1 -0
+			  to initialize FPGA IOs, PLL,
IO48
and
Quoted 8 lines
Quoted 7 lines
DDR.
+- altr,bitstream_core : File name for core raw binary which
contains FPGA design
(quoted diff) +1 -0
+			which is used to program FPGA CRAM
and
ERAM.
bitstream- instead of bitstream_
Noted.
Quoted 4 lines
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… …

Quoted 5 lines

Quoted 3 lines
Quoted 1 line
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.

Quoted 1 line

Marek Vasut

2018-11-27 12:07 UTC #8 lore raw

PATCH 1/9 ARM: socfpga: Description on FPGA bitstream type and file name for Arria 10

Quoted 87 lines · Tien Fong

Quoted 86 lines · Marek Vasut
Quoted 85 lines · Tien Fong
Quoted 83 lines · Marek Vasut
Quoted 71 lines · Tien Fong
Quoted 47 lines · Marek Vasut
Quoted 44 lines · tien.fong.chee at intel.com
From: Tien Fong Chee <[email protected]>
This patch adds description on properties about file name used
for
both
peripheral bitstream and core bitstream.
Signed-off-by Tien Fong Chee <[email protected]>
---
.../fpga/altera-socfpga-a10-fpga-mgr.txt | 6
++++++
 1 files changed, 6 insertions(+), 0 deletions(-)
(quoted diff) +0 -0
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
(quoted diff) +0 -0
@@ -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
(quoted diff) +1 -0
+			  to initialize FPGA IOs, PLL,
IO48
and
Quoted 8 lines
Quoted 7 lines
DDR.
+- altr,bitstream_core : File name for core raw binary which
contains FPGA design
(quoted diff) +1 -0
+			which is used to program FPGA CRAM
and
ERAM.
bitstream- instead of bitstream_
Noted.
Quoted 4 lines
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.
Quoted 9 lines
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 ...

Quoted 8 lines

Quoted 4 lines
Quoted 2 lines
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 ?

Quoted 4 lines

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.

Signature · 2 lines

Best regards,
Marek Vasut

Chee

2018-11-28 14:49 UTC #9 lore raw

PATCH 1/9 ARM: socfpga: Description on FPGA bitstream type and file name for Arria 10

Quoted 128 lines · Marek Vasut

Quoted 107 lines · Tien Fong
Quoted 105 lines · Marek Vasut
Quoted 103 lines · Tien Fong
Quoted 100 lines · Marek Vasut
Quoted 85 lines · Tien Fong
Quoted 56 lines · Marek Vasut
Quoted 52 lines · tien.fong.chee at intel.com
From: Tien Fong Chee <[email protected]>
This patch adds description on properties about file name
used
for
both
peripheral bitstream and core bitstream.
Signed-off-by Tien Fong Chee <[email protected]
Quoted 1 line
---
.../fpga/altera-socfpga-a10-fpga-
mgr.txt | 6
++++++
 1 files changed, 6 insertions(+), 0 deletions(-)
(quoted diff) +0 -0
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
(quoted diff) +0 -0
@@ -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
(quoted diff) +1 -0
+			  to initialize FPGA IOs, PLL,
IO48
and
DDR.
Quoted 8 lines
Quoted 7 lines
+- altr,bitstream_core : File name for core raw binary which
contains FPGA design
(quoted diff) +1 -0
+			which is used to program FPGA
CRAM
and
ERAM.
bitstream- instead of bitstream_
Noted.
Quoted 6 lines
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.
Quoted 11 lines
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 ...
Quoted 8 lines
Quoted 4 lines
Quoted 2 lines
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 ?
Quoted 8 lines
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.

Quoted 3 lines

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?

Quoted 1 line

Marek Vasut

2018-11-28 15:10 UTC #10 lore raw

PATCH 1/9 ARM: socfpga: Description on FPGA bitstream type and file name for Arria 10

Quoted 129 lines · Tien Fong

Quoted 128 lines · Marek Vasut
Quoted 107 lines · Tien Fong
Quoted 105 lines · Marek Vasut
Quoted 103 lines · Tien Fong
Quoted 100 lines · Marek Vasut
Quoted 85 lines · Tien Fong
Quoted 56 lines · Marek Vasut
Quoted 52 lines · tien.fong.chee at intel.com
From: Tien Fong Chee <[email protected]>
This patch adds description on properties about file name
used
for
both
peripheral bitstream and core bitstream.
Signed-off-by Tien Fong Chee <[email protected]
Quoted 1 line
---
.../fpga/altera-socfpga-a10-fpga-
mgr.txt | 6
++++++
 1 files changed, 6 insertions(+), 0 deletions(-)
(quoted diff) +0 -0
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
(quoted diff) +0 -0
@@ -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
(quoted diff) +1 -0
+			  to initialize FPGA IOs, PLL,
IO48
and
DDR.
Quoted 8 lines
Quoted 7 lines
+- altr,bitstream_core : File name for core raw binary which
contains FPGA design
(quoted diff) +1 -0
+			which is used to program FPGA
CRAM
and
ERAM.
bitstream- instead of bitstream_
Noted.
Quoted 6 lines
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.
Quoted 11 lines
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 ...
Quoted 8 lines
Quoted 4 lines
Quoted 2 lines
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 ?
Quoted 8 lines
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.

Quoted 8 lines

Quoted 3 lines
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.

Signature · 2 lines

Best regards,
Marek Vasut

Chee

2018-11-28 15:36 UTC #11 lore raw

PATCH 1/9 ARM: socfpga: Description on FPGA bitstream type and file name for Arria 10

Quoted 170 lines · Marek Vasut

Quoted 169 lines · Tien Fong
Quoted 159 lines · Marek Vasut
Quoted 136 lines · Tien Fong
Quoted 126 lines · Marek Vasut
Quoted 123 lines · Tien Fong
Quoted 119 lines · Marek Vasut
Quoted 115 lines · Tien Fong
Quoted 69 lines · Marek Vasut
Quoted 63 lines · tien.fong.chee at intel.com
From: Tien Fong Chee <[email protected]>
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
Quoted 2 lines
---
.../fpga/altera-socfpga-a10-fpga-
mgr.txt | 6
++++++
 1 files changed, 6 insertions(+), 0 deletions(-)
(quoted diff) +0 -0
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
(quoted diff) +0 -0
@@ -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
(quoted diff) +1 -0
+			  to initialize FPGA IOs,
PLL,
IO48
and
DDR.
+- altr,bitstream_core : File name for core raw
binary
Quoted 8 lines
Quoted 7 lines
which
contains FPGA design
(quoted diff) +1 -0
+			which is used to program
FPGA
CRAM
and
ERAM.
bitstream- instead of bitstream_
Noted.
Quoted 8 lines
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… …
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
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.
Quoted 6 lines
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 ?
Quoted 10 lines
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.
Quoted 4 lines
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.

Quoted 5 lines

Quoted 4 lines
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?

Quoted 1 line

Chee

2018-11-28 16:17 UTC #12 lore raw

PATCH 1/9 ARM: socfpga: Description on FPGA bitstream type and file name for Arria 10

In reply to Marek Vasut

Quoted 147 lines · Marek Vasut

Quoted 146 lines · Tien Fong
Quoted 144 lines · Marek Vasut
Quoted 142 lines · Tien Fong
Quoted 126 lines · Marek Vasut
Quoted 123 lines · Tien Fong
Quoted 119 lines · Marek Vasut
Quoted 115 lines · Tien Fong
Quoted 69 lines · Marek Vasut
Quoted 63 lines · tien.fong.chee at intel.com
From: Tien Fong Chee <[email protected]>
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
Quoted 2 lines
---
.../fpga/altera-socfpga-a10-fpga-
mgr.txt | 6
++++++
 1 files changed, 6 insertions(+), 0 deletions(-)
(quoted diff) +0 -0
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
(quoted diff) +0 -0
@@ -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
(quoted diff) +1 -0
+			  to initialize FPGA IOs,
PLL,
IO48
and
DDR.
+- altr,bitstream_core : File name for core raw
binary
Quoted 8 lines
Quoted 7 lines
which
contains FPGA design
(quoted diff) +1 -0
+			which is used to program
FPGA
CRAM
and
ERAM.
bitstream- instead of bitstream_
Noted.
Quoted 8 lines
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… …
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
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.

Quoted 4 lines

Quoted 4 lines
Quoted 4 lines
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.

Quoted 25 lines

Quoted 15 lines
Quoted 11 lines
Quoted 11 lines
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.
Quoted 4 lines
I do, otherwise I won't be able to test anything, so make sure this
is
supported.
Quoted 8 lines
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.

Marek Vasut

2018-11-28 17:55 UTC #13 lore raw

PATCH 1/9 ARM: socfpga: Description on FPGA bitstream type and file name for Arria 10

Quoted 152 lines · Tien Fong

Quoted 147 lines · Marek Vasut
Quoted 146 lines · Tien Fong
Quoted 144 lines · Marek Vasut
Quoted 142 lines · Tien Fong
Quoted 126 lines · Marek Vasut
Quoted 123 lines · Tien Fong
Quoted 119 lines · Marek Vasut
Quoted 115 lines · Tien Fong
Quoted 69 lines · Marek Vasut
Quoted 63 lines · tien.fong.chee at intel.com
From: Tien Fong Chee <[email protected]>
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
Quoted 2 lines
---
.../fpga/altera-socfpga-a10-fpga-
mgr.txt | 6
++++++
 1 files changed, 6 insertions(+), 0 deletions(-)
(quoted diff) +0 -0
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
(quoted diff) +0 -0
@@ -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
(quoted diff) +1 -0
+			  to initialize FPGA IOs,
PLL,
IO48
and
DDR.
+- altr,bitstream_core : File name for core raw
binary
Quoted 8 lines
Quoted 7 lines
which
contains FPGA design
(quoted diff) +1 -0
+			which is used to program
FPGA
CRAM
and
ERAM.
bitstream- instead of bitstream_
Noted.
Quoted 8 lines
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… …
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
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.
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Can you describe that workaround ? The code worked on the A10ES2 kit
I
have back around v2018.09, so what's the problem ?

Quoted 8 lines

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.

Signature · 2 lines

Best regards,
Marek Vasut

Chee

2018-12-14 08:07 UTC #14 lore raw

PATCH 1/9 ARM: socfpga: Description on FPGA bitstream type and file name for Arria 10

Quoted 197 lines · Marek Vasut

Quoted 183 lines · Tien Fong
Quoted 176 lines · Marek Vasut
Quoted 174 lines · Tien Fong
Quoted 171 lines · Marek Vasut
Quoted 168 lines · Tien Fong
Quoted 148 lines · Marek Vasut
Quoted 144 lines · Tien Fong
Quoted 139 lines · Marek Vasut
Quoted 120 lines · Tien Fong
Quoted 81 lines · Marek Vasut
Quoted 74 lines · tien.fong.chee at intel.com
From: Tien Fong Chee <[email protected]>
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
Quoted 3 lines
---
.../fpga/altera-socfpga-a10-fpga-
mgr.txt | 6
++++++
 1 files changed, 6 insertions(+), 0
deletions(-)
(quoted diff) +0 -0
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
(quoted diff) +0 -0
@@ -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
(quoted diff) +1 -0
+			  to initialize FPGA
IOs,
PLL,
IO48
and
DDR.
+- altr,bitstream_core : File name for core raw
binary
which
Quoted 8 lines
Quoted 7 lines
contains FPGA design
(quoted diff) +1 -0
+			which is used to
program
FPGA
CRAM
and
ERAM.
bitstream- instead of bitstream_
Noted.
Quoted 9 lines
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.
Quoted 13 lines
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 ...
Quoted 8 lines
Quoted 4 lines
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.
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Can you describe that workaround ? The code worked on the A10ES2
kit
I
have back around v2018.09, so what's the problem ?
Quoted 13 lines
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.

Quoted 1 line

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.

Quoted 2 lines

I wonder whether we want to scrap the ES support afterall.

We actually no longer support ES.

Quoted 1 line

tien.fong.chee at intel.com

author

2018-11-21 10:41 UTC #15 lore raw

PATCH 2/9 ARM: socfpga: Add FPGA drivers for Arria 10 FPGA bitstream loading

In reply to tien.fong.chee at intel.com

From: Tien Fong Chee <[email protected]>

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 <[email protected]>

---
 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(-)

arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts +18 -0

index 998d811..f4764d4 100644
@@ -18,6 +18,24 @@
 /dts-v1/;
 #include "socfpga_arria10_socdk.dtsi"
 
+/ {
+	chosen {
+		firmware-loader = &fs_loader0;
+	};
+
+	fs_loader0: fs-loader at 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";

arch/arm/mach-socfpga/include/mach/fpga_manager_arria10.h +28 -2

index 09d13f6..b48bc76 100644
@@ -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_ */

configs/socfpga_arria10_defconfig +9 -0

index 6ebda81..f88910c 100644
@@ -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

drivers/fpga/Kconfig +9 -0

index 50e9019..06a8204 100644
@@ -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

drivers/fpga/socfpga_arria10.c +407 ~~-10~~

460 lines not on this page · raw message

Signature · 1 line

1.7.7.4

Marek Vasut

2018-11-21 14:18 UTC #16 lore raw

PATCH 2/9 ARM: socfpga: Add FPGA drivers for Arria 10 FPGA bitstream loading

Quoted 8 lines · tien.fong.chee at intel.com

From: Tien Fong Chee <[email protected]>
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 <[email protected]>

[...]

arch/arm/mach-socfpga/include/mach/fpga_manager_arria10.h line 58

Quoted 6 lines

arch/arm/mach-socfpga/include/mach/fpga_manager_arria10.h +2 -0

@@ -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.

arch/arm/mach-socfpga/include/mach/fpga_manager_arria10.h struct socfpga_fpga_manager { · line 97

Quoted 9 lines

arch/arm/mach-socfpga/include/mach/fpga_manager_arria10.h +2 -0

 #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.

Quoted 37 lines

arch/arm/mach-socfpga/include/mach/fpga_manager_arria10.h +19 -1

+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_ */

configs/socfpga_arria10_defconfig +1 -0

index 6ebda81..f88910c 100644
@@ -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

configs/socfpga_arria10_defconfig CONFIG_MTDIDS_DEFAULT="nor0=ff705000.spi.0" · line 40

Quoted 10 lines

configs/socfpga_arria10_defconfig +8 -0

 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

drivers/fpga/Kconfig config FPGA_SOCFPGA · line 24

Quoted 12 lines

configs/socfpga_arria10_defconfig +0 -0

 CONFIG_FPGA_SOCFPGA=y
 CONFIG_DM_GPIO=y
 CONFIG_DWAPB_GPIO=y

drivers/fpga/Kconfig +1 -0

index 50e9019..06a8204 100644
@@ -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 ?

Quoted 35 lines

drivers/fpga/Kconfig +8 -0

+	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

drivers/fpga/socfpga_arria10.c +3 -1

index 114dd91..d9ad237 100644
@@ -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>

drivers/fpga/socfpga_arria10.c line 29

Quoted 7 lines

drivers/fpga/socfpga_arria10.c +4 -0

@@ -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.

Quoted 12 lines

drivers/fpga/socfpga_arria10.c +1 -1

 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;

drivers/fpga/socfpga_arria10.c int fpgamgr_program_finish(void) · line 456

Quoted 4 lines

drivers/fpga/socfpga_arria10.c +1 -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 .

drivers/fpga/socfpga_arria10.c int fpgamgr_program_finish(void) · line 460

Quoted 4 lines

drivers/fpga/socfpga_arria10.c +4 -0

+{
+	const char *fpga_filename = NULL;
+	const char *cell;
+	int nodeoffset;

ofnode_read_string() , use ofnode globally.

Quoted 58 lines

drivers/fpga/socfpga_arria10.c +58 -0

+	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;

drivers/fpga/socfpga_arria10.c int fpgamgr_program_finish(void) · line 529

Quoted 10 lines

drivers/fpga/socfpga_arria10.c +10 -0

+		}
+
+		rbf->section = unknown;
+		break;
+
+		WATCHDOG_RESET();
+	}
+}
+
+static struct firmware *fw;

What is this static variable doing here ?

Quoted 59 lines

drivers/fpga/socfpga_arria10.c +59 -0

+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);

drivers/fpga/socfpga_arria10.c int fpgamgr_program_finish(void) · line 599

Quoted 10 lines

drivers/fpga/socfpga_arria10.c +10 -0

+		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.

Quoted 22 lines

drivers/fpga/socfpga_arria10.c +22 -0

+	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

drivers/fpga/socfpga_arria10.c int fpgamgr_program_finish(void) · line 631

Quoted 10 lines

drivers/fpga/socfpga_arria10.c +10 -0

+	 */
+	*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 ?

Quoted 12 lines

drivers/fpga/socfpga_arria10.c +12 -0

+#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;
+}

[...]

Signature · 2 lines

Best regards,
Marek Vasut

Chee

2018-11-23 09:43 UTC #17 lore raw

PATCH 2/9 ARM: socfpga: Add FPGA drivers for Arria 10 FPGA bitstream loading

Quoted 16 lines · Marek Vasut

Quoted 12 lines · tien.fong.chee at intel.com
From: Tien Fong Chee <[email protected]>
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 <[email protected]>
[...]
Quoted 1 line

Quoted 10 lines

Quoted 9 lines
(quoted diff) +0 -0
@@ -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.

Quoted 12 lines

arch/arm/mach-socfpga/include/mach/fpga_manager_arria10.h struct socfpga_fpga_manager { · line 97
Quoted 11 lines
#ifndef __ASSEMBLY__

struct socfpga_fpga_manager {
arch/arm/mach-socfpga/include/mach/fpga_manager_arria10.h +0 -0
@@ -88,12 +93,33 @@ struct socfpga_fpga_manager {
  	u32  imgcfg_fifo_status;
  };
arch/arm/mach-socfpga/include/mach/fpga_manager_arria10.h +2 -0
+enum rbf_type {unknown, periph_section, core_section};
+enum rbf_security {invalid, unencrypted, encrypted};
enum should use one line per entry.

Noted.

Quoted 45 lines

Quoted 45 lines
arch/arm/mach-socfpga/include/mach/fpga_manager_arria10.h +14 -0
+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);
arch/arm/mach-socfpga/include/mach/fpga_manager_arria10.h +2 -1
-
+int is_fpgamgr_early_user_mode(void);
+const char *get_fpga_filename(const void *fdt, int *len, u32
core);
(quoted diff) +2 -0
+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,
arch/arm/mach-socfpga/include/mach/fpga_manager_arria10.h +1 -0
+			u32 offset);
#endif /* __ASSEMBLY__ */

#endif /* _FPGA_MANAGER_ARRIA10_H_ */
configs/socfpga_arria10_defconfig +0 -0
index 6ebda81..f88910c 100644
@@ -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

Quoted 8 lines

configs/socfpga_arria10_defconfig CONFIG_MTDIDS_DEFAULT="nor0=ff705000.spi.0" · line 40
Quoted 7 lines
configs/socfpga_arria10_defconfig +7 -0
+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.

Quoted 17 lines

Quoted 10 lines
CONFIG_FPGA_SOCFPGA=y
CONFIG_DM_GPIO=y
CONFIG_DWAPB_GPIO=y
drivers/fpga/Kconfig +0 -0
index 50e9019..06a8204 100644
@@ -21,6 +21,15 @@ config FPGA_SOCFPGA
Quoted 5 lines

This provides common functionality for Gen5 and Arria10 devices.

+config CHECK_FPGA_DATA_CRC
config FPGA_SOCFPGA_A10_CRC_CHECK

Quoted 1 line

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.

Quoted 40 lines

drivers/fpga/Kconfig +4 -0

+	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

drivers/fpga/Kconfig +2 -0

+
+	 This provides CRC checking to ensure integrated of Arria

10 FPGA

drivers/fpga/Kconfig +2 -0

+	 bitstream is programmed into FPGA.
+

config FPGA_CYCLON2
bool "Enable Altera FPGA driver for Cyclone II" depends on FPGA_ALTERA

drivers/fpga/socfpga_arria10.c +1 -1

index 114dd91..d9ad237 100644
@@ -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>

drivers/fpga/socfpga_arria10.c +2 -0

@@ -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>

drivers/fpga/socfpga_arria10.c +0 -0

@@ -21,6 +23,10 @@

Quoted 8 lines

drivers/fpga/socfpga_arria10.c line 29
Quoted 7 lines
drivers/fpga/socfpga_arria10.c +4 -0
  #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?

Quoted 22 lines

Quoted 17 lines
static const struct socfpga_fpga_manager *fpga_manager_base = (void *)SOCFPGA_FPGAMGRREGS_ADDRESS;
drivers/fpga/socfpga_arria10.c +0 -0
@@ -64,7 +70,7 @@ static int wait_for_user_mode(void)
  		1, FPGA_TIMEOUT_MSEC, false);
  }
drivers/fpga/socfpga_arria10.c +1 -1
-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;
drivers/fpga/socfpga_arria10.c +0 -0
@@ -447,13 +453,393 @@ int fpgamgr_program_finish(void)
  	return 0;
  }
Quoted 2 lines

+const char *get_fpga_filename(const void *fdt, int *len, u32 core)
static, fix globally .
Quoted 1 line

Quoted 5 lines

drivers/fpga/socfpga_arria10.c int fpgamgr_program_finish(void) · line 460
Quoted 4 lines
drivers/fpga/socfpga_arria10.c +4 -0
+{
+	const char *fpga_filename = NULL;
+	const char *cell;
+	int nodeoffset;
ofnode_read_string() , use ofnode globally.

Noted.

Quoted 66 lines

drivers/fpga/socfpga_arria10.c +11 -0

+	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);

drivers/fpga/socfpga_arria10.c +43 -0

+		}
+
+		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)

{

drivers/fpga/socfpga_arria10.c +8 -0

+			rbf->section = periph_section;
+			break;
+		} else if (*(buffer + i + 2) == ARRIA10RBF_CORE) {
+			rbf->section = core_section;
+			break;
+		}
+
+		rbf->section = unknown;

Quoted 8 lines

drivers/fpga/socfpga_arria10.c int fpgamgr_program_finish(void) · line 529
Quoted 7 lines
drivers/fpga/socfpga_arria10.c +7 -0
+		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.

Quoted 74 lines

drivers/fpga/socfpga_arria10.c +2 -0

+int first_loading_rbf_to_buffer(struct device_platdata *plat,
+				struct fpga_loadfs_info

*fpga_loadfs,

drivers/fpga/socfpga_arria10.c +13 -0

+				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-

Quoted 1 line

filename,

drivers/fpga/socfpga_arria10.c +2 -0

+					header,
+					sizeof(struct

image_header),

drivers/fpga/socfpga_arria10.c +4 -0

+					fpga_loadfs->offset,
+					&fw);
+	if (ret < 0) {
+		debug("FPGA: Failed to read RBF mkimage header

from flash.\n");

drivers/fpga/socfpga_arria10.c +24 -0

+		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

drivers/fpga/socfpga_arria10.c +1 -0

+	 * chunk by chunk transfer is required due to smaller

buffer size

drivers/fpga/socfpga_arria10.c +19 -0

+	 * 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-

Quoted 1 line

filename,

Quoted 10 lines

drivers/fpga/socfpga_arria10.c int fpgamgr_program_finish(void) · line 599
Quoted 4 lines
drivers/fpga/socfpga_arria10.c +4 -0
+					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().

Quoted 32 lines

drivers/fpga/socfpga_arria10.c +2 -0

+	if (ret < 0) {
+		debug("FPGA: Failed to read RBF mkimage header and

RBF from ");

drivers/fpga/socfpga_arria10.c +10 -0

+		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));

drivers/fpga/socfpga_arria10.c +6 -0

+
+	/* 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);

drivers/fpga/socfpga_arria10.c +3 -0

+
+	/*
+	 * Update the starting addr of RBF to init FPGA &

programming RBF

drivers/fpga/socfpga_arria10.c +5 -0

+	 * into FPGA
+	 */
+	*buffer = (u32)buffer_p_after_header;
+
+	/* Update the size of RBF to be programmed into FPGA */

Quoted 8 lines

drivers/fpga/socfpga_arria10.c int fpgamgr_program_finish(void) · line 631
Quoted 7 lines
drivers/fpga/socfpga_arria10.c +5 -0
+	*buffer_bsize = buffersz_after_header;
+
+#ifdef CONFIG_CHECK_FPGA_DATA_CRC
+	fpga_loadfs->datacrc = crc32(fpga_loadfs->datacrc,
+					(u_char
*)buffer_p_after_header,
drivers/fpga/socfpga_arria10.c +1 -0
+					buffersz_after_header);
Why is this not ON by default ?

It is off for the sake of performance.

Quoted 16 lines

Quoted 15 lines

drivers/fpga/socfpga_arria10.c +5 -0

+#endif
+
+if (fpga_loadfs->remaining == 0) {
+#ifdef CONFIG_CHECK_FPGA_DATA_CRC
+	if (fpga_loadfs->datacrc != image_get_dcrc(&(fpga_loadfs-

Quoted 1 line

header))) {

drivers/fpga/socfpga_arria10.c +7 -0

+		debug("FPGA: Bad Data Checksum.\n");
+		return -EPERM;
+	}
+#endif
+}
+	return 0;
+}

[...]

Marek Vasut

2018-11-23 12:28 UTC #18 lore raw

PATCH 2/9 ARM: socfpga: Add FPGA drivers for Arria 10 FPGA bitstream loading

Quoted 112 lines · Tien Fong

Quoted 16 lines · Marek Vasut
Quoted 12 lines · tien.fong.chee at intel.com
From: Tien Fong Chee <[email protected]>
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 <[email protected]>
[...]
Quoted 1 line
Quoted 10 lines
Quoted 9 lines
(quoted diff) +0 -0
@@ -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.
Quoted 12 lines
arch/arm/mach-socfpga/include/mach/fpga_manager_arria10.h struct socfpga_fpga_manager { · line 97
Quoted 11 lines
#ifndef __ASSEMBLY__

struct socfpga_fpga_manager {
arch/arm/mach-socfpga/include/mach/fpga_manager_arria10.h +0 -0
@@ -88,12 +93,33 @@ struct socfpga_fpga_manager {
  	u32  imgcfg_fifo_status;
  };
arch/arm/mach-socfpga/include/mach/fpga_manager_arria10.h +2 -0
+enum rbf_type {unknown, periph_section, core_section};
+enum rbf_security {invalid, unencrypted, encrypted};
enum should use one line per entry.
Noted.
Quoted 45 lines
Quoted 45 lines
arch/arm/mach-socfpga/include/mach/fpga_manager_arria10.h +14 -0
+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);
arch/arm/mach-socfpga/include/mach/fpga_manager_arria10.h +2 -1
-
+int is_fpgamgr_early_user_mode(void);
+const char *get_fpga_filename(const void *fdt, int *len, u32
core);
(quoted diff) +2 -0
+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,
arch/arm/mach-socfpga/include/mach/fpga_manager_arria10.h +1 -0
+			u32 offset);
#endif /* __ASSEMBLY__ */

#endif /* _FPGA_MANAGER_ARRIA10_H_ */
configs/socfpga_arria10_defconfig +0 -0
index 6ebda81..f88910c 100644
@@ -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
Quoted 8 lines
configs/socfpga_arria10_defconfig CONFIG_MTDIDS_DEFAULT="nor0=ff705000.spi.0" · line 40
Quoted 7 lines
configs/socfpga_arria10_defconfig +7 -0
+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.
Quoted 17 lines
Quoted 10 lines
CONFIG_FPGA_SOCFPGA=y
CONFIG_DM_GPIO=y
CONFIG_DWAPB_GPIO=y
drivers/fpga/Kconfig +0 -0
index 50e9019..06a8204 100644
@@ -21,6 +21,15 @@ config FPGA_SOCFPGA
Quoted 5 lines

This provides common functionality for Gen5 and Arria10 devices.

+config CHECK_FPGA_DATA_CRC
config FPGA_SOCFPGA_A10_CRC_CHECK

Quoted 5 lines

Quoted 1 line
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.

Quoted 47 lines

Quoted 39 lines

drivers/fpga/Kconfig +4 -0

+	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

drivers/fpga/Kconfig +2 -0

+
+	 This provides CRC checking to ensure integrated of Arria

10 FPGA

drivers/fpga/Kconfig +2 -0

+	 bitstream is programmed into FPGA.
+

config FPGA_CYCLON2
bool "Enable Altera FPGA driver for Cyclone II" depends on FPGA_ALTERA

drivers/fpga/socfpga_arria10.c +1 -1

index 114dd91..d9ad237 100644
@@ -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>

drivers/fpga/socfpga_arria10.c +2 -0

@@ -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>

drivers/fpga/socfpga_arria10.c +0 -0

@@ -21,6 +23,10 @@

Quoted 8 lines

drivers/fpga/socfpga_arria10.c line 29
Quoted 7 lines
drivers/fpga/socfpga_arria10.c +4 -0
  #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.

Quoted 3 lines

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 ?

Quoted 102 lines

Quoted 22 lines

Quoted 17 lines
static const struct socfpga_fpga_manager *fpga_manager_base = (void *)SOCFPGA_FPGAMGRREGS_ADDRESS;
drivers/fpga/socfpga_arria10.c +0 -0
@@ -64,7 +70,7 @@ static int wait_for_user_mode(void)
  		1, FPGA_TIMEOUT_MSEC, false);
  }
drivers/fpga/socfpga_arria10.c +1 -1
-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;
drivers/fpga/socfpga_arria10.c +0 -0
@@ -447,13 +453,393 @@ int fpgamgr_program_finish(void)
  	return 0;
  }
Quoted 2 lines

+const char *get_fpga_filename(const void *fdt, int *len, u32 core)
static, fix globally .
Quoted 1 line

Quoted 5 lines

drivers/fpga/socfpga_arria10.c int fpgamgr_program_finish(void) · line 460
Quoted 4 lines
drivers/fpga/socfpga_arria10.c +4 -0
+{
+	const char *fpga_filename = NULL;
+	const char *cell;
+	int nodeoffset;
ofnode_read_string() , use ofnode globally.

Noted.

Quoted 66 lines

drivers/fpga/socfpga_arria10.c +11 -0

+	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);

drivers/fpga/socfpga_arria10.c +43 -0

+		}
+
+		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)

{

drivers/fpga/socfpga_arria10.c +8 -0

+			rbf->section = periph_section;
+			break;
+		} else if (*(buffer + i + 2) == ARRIA10RBF_CORE) {
+			rbf->section = core_section;
+			break;
+		}
+
+		rbf->section = unknown;

Quoted 8 lines

drivers/fpga/socfpga_arria10.c int fpgamgr_program_finish(void) · line 529
Quoted 7 lines
drivers/fpga/socfpga_arria10.c +7 -0
+		break;
+
+		WATCHDOG_RESET();
+	}
+}
+
+static struct firmware *fw;
What is this static variable doing here ?

Quoted 2 lines

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.

Quoted 82 lines

Quoted 72 lines

drivers/fpga/socfpga_arria10.c +2 -0

+int first_loading_rbf_to_buffer(struct device_platdata *plat,
+				struct fpga_loadfs_info

*fpga_loadfs,

drivers/fpga/socfpga_arria10.c +13 -0

+				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-

Quoted 1 line

filename,

drivers/fpga/socfpga_arria10.c +2 -0

+					header,
+					sizeof(struct

image_header),

drivers/fpga/socfpga_arria10.c +4 -0

+					fpga_loadfs->offset,
+					&fw);
+	if (ret < 0) {
+		debug("FPGA: Failed to read RBF mkimage header

from flash.\n");

drivers/fpga/socfpga_arria10.c +24 -0

+		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

drivers/fpga/socfpga_arria10.c +1 -0

+	 * chunk by chunk transfer is required due to smaller

buffer size

drivers/fpga/socfpga_arria10.c +19 -0

+	 * 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-

Quoted 1 line

filename,

Quoted 10 lines

drivers/fpga/socfpga_arria10.c int fpgamgr_program_finish(void) · line 599
Quoted 4 lines
drivers/fpga/socfpga_arria10.c +4 -0
+					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.

Quoted 5 lines

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.

Quoted 39 lines

Quoted 31 lines
Quoted 31 lines
drivers/fpga/socfpga_arria10.c +2 -0
+	if (ret < 0) {
+		debug("FPGA: Failed to read RBF mkimage header and
RBF from ");
drivers/fpga/socfpga_arria10.c +10 -0
+		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));
drivers/fpga/socfpga_arria10.c +6 -0
+
+	/* 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);
drivers/fpga/socfpga_arria10.c +3 -0
+
+	/*
+	 * Update the starting addr of RBF to init FPGA &
programming RBF
drivers/fpga/socfpga_arria10.c +5 -0
+	 * into FPGA
+	 */
+	*buffer = (u32)buffer_p_after_header;
+
+	/* Update the size of RBF to be programmed into FPGA */
Quoted 8 lines
drivers/fpga/socfpga_arria10.c int fpgamgr_program_finish(void) · line 631
Quoted 7 lines
drivers/fpga/socfpga_arria10.c +5 -0
+	*buffer_bsize = buffersz_after_header;
+
+#ifdef CONFIG_CHECK_FPGA_DATA_CRC
+	fpga_loadfs->datacrc = crc32(fpga_loadfs->datacrc,
+					(u_char
*)buffer_p_after_header,
drivers/fpga/socfpga_arria10.c +1 -0
+					buffersz_after_header);
Why is this not ON by default ?

Quoted 1 line

It is off for the sake of performance.

Do you have some hard numbers to support this claim ?

Quoted 16 lines

Quoted 16 lines
Quoted 15 lines
drivers/fpga/socfpga_arria10.c +5 -0
+#endif
+
+if (fpga_loadfs->remaining == 0) {
+#ifdef CONFIG_CHECK_FPGA_DATA_CRC
+	if (fpga_loadfs->datacrc != image_get_dcrc(&(fpga_loadfs-
Quoted 1 line
header))) {
drivers/fpga/socfpga_arria10.c +7 -0
+		debug("FPGA: Bad Data Checksum.\n");
+		return -EPERM;
+	}
+#endif
+}
+	return 0;
+}
[...]

Signature · 2 lines

Best regards,
Marek Vasut

Chee

2018-11-26 10:09 UTC #19 lore raw

PATCH 2/9 ARM: socfpga: Add FPGA drivers for Arria 10 FPGA bitstream loading

Quoted 132 lines · Marek Vasut

Quoted 125 lines · Tien Fong
Quoted 20 lines · Marek Vasut
Quoted 14 lines · tien.fong.chee at intel.com
From: Tien Fong Chee <[email protected]>
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 <[email protected]>
[...]
Quoted 2 lines
Quoted 11 lines
Quoted 10 lines
(quoted diff) +0 -0
@@ -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.
Quoted 4 lines
Quoted 4 lines
Quoted 10 lines
arch/arm/mach-socfpga/include/mach/fpga_manager_arria10.h struct socfpga_fpga_manager { · line 97
Quoted 9 lines
#ifndef __ASSEMBLY__

struct socfpga_fpga_manager {
arch/arm/mach-socfpga/include/mach/fpga_manager_arria10.h +0 -0
@@ -88,12 +93,33 @@ struct socfpga_fpga_manager {
  	u32  imgcfg_fifo_status;
  };
arch/arm/mach-socfpga/include/mach/fpga_manager_arria10.h +2 -0
+enum rbf_type {unknown, periph_section, core_section};
+enum rbf_security {invalid, unencrypted, encrypted};
enum should use one line per entry.
Noted.
Quoted 47 lines
Quoted 47 lines
arch/arm/mach-socfpga/include/mach/fpga_manager_arria10.h +14 -0
+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);
arch/arm/mach-socfpga/include/mach/fpga_manager_arria10.h +2 -1
-
+int is_fpgamgr_early_user_mode(void);
+const char *get_fpga_filename(const void *fdt, int *len, u32
core);
(quoted diff) +2 -0
+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,
arch/arm/mach-socfpga/include/mach/fpga_manager_arria10.h +1 -0
+			u32 offset);
#endif /* __ASSEMBLY__ */

#endif /* _FPGA_MANAGER_ARRIA10_H_ */
configs/socfpga_arria10_defconfig +0 -0
index 6ebda81..f88910c 100644
@@ -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
Quoted 8 lines
configs/socfpga_arria10_defconfig CONFIG_MTDIDS_DEFAULT="nor0=ff705000.spi.0" · line 40
Quoted 7 lines
configs/socfpga_arria10_defconfig +7 -0
+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.
Quoted 20 lines
Quoted 12 lines
CONFIG_FPGA_SOCFPGA=y
CONFIG_DM_GPIO=y
CONFIG_DWAPB_GPIO=y
drivers/fpga/Kconfig +0 -0
index 50e9019..06a8204 100644
@@ -21,6 +21,15 @@ config FPGA_SOCFPGA
Quoted 6 lines

This provides common functionality for Gen5 and Arria10
devices.

+config CHECK_FPGA_DATA_CRC
config FPGA_SOCFPGA_A10_CRC_CHECK
Quoted 6 lines
Quoted 1 line
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.

Quoted 4 lines

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.

Quoted 53 lines

Quoted 45 lines

drivers/fpga/Kconfig +4 -0

+	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

drivers/fpga/Kconfig +2 -0

+
+	 This provides CRC checking to ensure integrated of

Arria
10 FPGA

drivers/fpga/Kconfig +2 -0

+	 bitstream is programmed into FPGA.
+

config FPGA_CYCLON2
bool "Enable Altera FPGA driver for Cyclone II" depends on FPGA_ALTERA

drivers/fpga/socfpga_arria10.c +1 -1

index 114dd91..d9ad237 100644
@@ -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>

drivers/fpga/socfpga_arria10.c +2 -0

@@ -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>

drivers/fpga/socfpga_arria10.c line 29

Quoted 7 lines

drivers/fpga/socfpga_arria10.c +4 -0

@@ -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.

Quoted 6 lines

Quoted 5 lines
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.

Quoted 118 lines

Quoted 118 lines
Quoted 29 lines
Quoted 22 lines
static const struct socfpga_fpga_manager *fpga_manager_base = (void *)SOCFPGA_FPGAMGRREGS_ADDRESS;
drivers/fpga/socfpga_arria10.c +0 -0
@@ -64,7 +70,7 @@ static int wait_for_user_mode(void)
  		1, FPGA_TIMEOUT_MSEC, false);
  }
drivers/fpga/socfpga_arria10.c +1 -1
-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;
drivers/fpga/socfpga_arria10.c +0 -0
@@ -447,13 +453,393 @@ int fpgamgr_program_finish(void)
  	return 0;
  }
Quoted 3 lines

+const char *get_fpga_filename(const void *fdt, int *len, u32 core)
static, fix globally .
Quoted 2 lines
Quoted 5 lines
drivers/fpga/socfpga_arria10.c int fpgamgr_program_finish(void) · line 460
Quoted 4 lines
drivers/fpga/socfpga_arria10.c +4 -0
+{
+	const char *fpga_filename = NULL;
+	const char *cell;
+	int nodeoffset;
ofnode_read_string() , use ofnode globally.
Noted.
Quoted 83 lines
Quoted 72 lines
drivers/fpga/socfpga_arria10.c +11 -0
+	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);
drivers/fpga/socfpga_arria10.c +26 -0
+		}
+
+		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) {
drivers/fpga/socfpga_arria10.c +2 -0
+			rbf->security = unencrypted;
+		} else if (*(buffer + i + 1) == RBF_ENCRYPTED)
{
drivers/fpga/socfpga_arria10.c +13 -0
+			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) {
drivers/fpga/socfpga_arria10.c +3 -0
+			rbf->section = core_section;
+			break;
+		} else if (*(buffer + i + 2) ==
ARRIA10RBF_PERIPH)
{
drivers/fpga/socfpga_arria10.c +3 -0
+			rbf->section = periph_section;
+			break;
+		} else if (*(buffer + i + 2) ==
ARRIA10RBF_CORE) {
drivers/fpga/socfpga_arria10.c +2 -0
+			rbf->section = core_section;
+			break;
drivers/fpga/socfpga_arria10.c int fpgamgr_program_finish(void) · line 529
Quoted 10 lines
drivers/fpga/socfpga_arria10.c +10 -0
+		}
+
+		rbf->section = unknown;
+		break;
+
+		WATCHDOG_RESET();
+	}
+}
+
+static struct firmware *fw;
What is this static variable doing here ?

Quoted 6 lines

Quoted 2 lines
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.

Quoted 104 lines

Quoted 96 lines
Quoted 85 lines
Quoted 85 lines
drivers/fpga/socfpga_arria10.c +2 -0
+int first_loading_rbf_to_buffer(struct device_platdata *plat,
+				struct fpga_loadfs_info
*fpga_loadfs,
drivers/fpga/socfpga_arria10.c +1 -0
+				u32 *buffer, u32
*buffer_bsize)
drivers/fpga/socfpga_arria10.c +12 -0
+{
+	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-
Quoted 3 lines
fpga_fsinfo-
filename,
drivers/fpga/socfpga_arria10.c +2 -0
+					header,
+					sizeof(struct
image_header),
drivers/fpga/socfpga_arria10.c +4 -0
+					fpga_loadfs->offset,
+					&fw);
+	if (ret < 0) {
+		debug("FPGA: Failed to read RBF mkimage header
from flash.\n");
drivers/fpga/socfpga_arria10.c +19 -0
+		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 */
drivers/fpga/socfpga_arria10.c +5 -0
+	totalsz_header_rbf = fpga_loadfs->remaining +
+				sizeof(struct image_header);
+
+	/*
+	 * Determine buffer size vs RBF size, and calculating
number of
drivers/fpga/socfpga_arria10.c +1 -0
+	 * chunk by chunk transfer is required due to smaller
buffer size
drivers/fpga/socfpga_arria10.c +6 -0
+	 * 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);
drivers/fpga/socfpga_arria10.c +1 -0
+		fpga_loadfs->remaining -=
buffersz_after_header;
drivers/fpga/socfpga_arria10.c +6 -0
+	} 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);
drivers/fpga/socfpga_arria10.c +6 -0
+		fpga_loadfs->remaining = 0;
+	}
+
+	/* Loading mkimage header and RBFinto buffer */
+	ret = request_firmware_into_buf(plat,
+					fpga_loadfs-
Quoted 3 lines
fpga_fsinfo-
filename,
Quoted 11 lines
drivers/fpga/socfpga_arria10.c int fpgamgr_program_finish(void) · line 599
Quoted 4 lines
drivers/fpga/socfpga_arria10.c +4 -0
+					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.
Quoted 8 lines
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().

Quoted 2 lines

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.

Quoted 71 lines

Quoted 46 lines
Quoted 38 lines
Quoted 38 lines
drivers/fpga/socfpga_arria10.c +2 -0
+	if (ret < 0) {
+		debug("FPGA: Failed to read RBF mkimage header
and
RBF from ");
drivers/fpga/socfpga_arria10.c +10 -0
+		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));
drivers/fpga/socfpga_arria10.c +6 -0
+
+	/* 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);
drivers/fpga/socfpga_arria10.c +3 -0
+
+	/*
+	 * Update the starting addr of RBF to init FPGA &
programming RBF
drivers/fpga/socfpga_arria10.c +5 -0
+	 * into FPGA
+	 */
+	*buffer = (u32)buffer_p_after_header;
+
+	/* Update the size of RBF to be programmed into FPGA
*/
drivers/fpga/socfpga_arria10.c +1 -0
+	*buffer_bsize = buffersz_after_header;
Quoted 8 lines
Quoted 7 lines
drivers/fpga/socfpga_arria10.c +4 -0
+
+#ifdef CONFIG_CHECK_FPGA_DATA_CRC
+	fpga_loadfs->datacrc = crc32(fpga_loadfs->datacrc,
+					(u_char
*)buffer_p_after_header,
drivers/fpga/socfpga_arria10.c +1 -0
+					buffersz_after_header)
;
Why is this not ON by default ?
Quoted 1 line
It is off for the sake of performance.
Do you have some hard numbers to support this claim ?
Quoted 21 lines
Quoted 21 lines
Quoted 20 lines
drivers/fpga/socfpga_arria10.c +5 -0
+#endif
+
+if (fpga_loadfs->remaining == 0) {
+#ifdef CONFIG_CHECK_FPGA_DATA_CRC
+	if (fpga_loadfs->datacrc !=
image_get_dcrc(&(fpga_loadfs-
Quoted 2 lines
header))) {
drivers/fpga/socfpga_arria10.c +7 -0
+		debug("FPGA: Bad Data Checksum.\n");
+		return -EPERM;
+	}
+#endif
+}
+	return 0;
+}
[...]

Marek Vasut

2018-11-26 11:18 UTC #20 lore raw

PATCH 2/9 ARM: socfpga: Add FPGA drivers for Arria 10 FPGA bitstream loading

On 11/26/2018 11:09 AM, Chee, Tien Fong wrote:
[...]

Quoted 19 lines

Quoted 19 lines
Quoted 19 lines
Quoted 13 lines
Quoted 11 lines
drivers/fpga/Kconfig +0 -0
index 50e9019..06a8204 100644
@@ -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
Quoted 1 line
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.

Quoted 8 lines

Quoted 4 lines
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.

Quoted 58 lines

Quoted 58 lines
Quoted 58 lines
Quoted 45 lines
Quoted 45 lines
drivers/fpga/Kconfig +4 -0
+	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
drivers/fpga/Kconfig +2 -0
+
+	 This provides CRC checking to ensure integrated of
Arria
10 FPGA
drivers/fpga/Kconfig +2 -0
+	 bitstream is programmed into FPGA.
+
config FPGA_CYCLON2
bool "Enable Altera FPGA driver for Cyclone II" depends on FPGA_ALTERA
drivers/fpga/socfpga_arria10.c +1 -1
index 114dd91..d9ad237 100644
@@ -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>
drivers/fpga/socfpga_arria10.c +2 -0
@@ -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>
drivers/fpga/socfpga_arria10.c +0 -0
@@ -21,6 +23,10 @@
Quoted 8 lines
drivers/fpga/socfpga_arria10.c line 29
Quoted 7 lines
drivers/fpga/socfpga_arria10.c +4 -0
  #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?

Quoted 4 lines

Quoted 1 line
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 ?

[...]

Signature · 2 lines

Best regards,
Marek Vasut

Chee

2018-11-27 08:54 UTC #21 lore raw

PATCH 2/9 ARM: socfpga: Add FPGA drivers for Arria 10 FPGA bitstream loading

Quoted 38 lines · Marek Vasut

On 11/26/2018 11:09 AM, Chee, Tien Fong wrote:
[...]
Quoted 30 lines
Quoted 25 lines
Quoted 18 lines
Quoted 18 lines
Quoted 10 lines
drivers/fpga/Kconfig +0 -0
index 50e9019..06a8204 100644
@@ -21,6 +21,15 @@ config FPGA_SOCFPGA
Quoted 6 lines

This provides common functionality for Gen5 and Arria10
devices.

+config CHECK_FPGA_DATA_CRC
config FPGA_SOCFPGA_A10_CRC_CHECK
Quoted 7 lines
Quoted 1 line
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.
Quoted 5 lines
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.
Quoted 6 lines
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.

Quoted 3 lines

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.

Quoted 68 lines

Quoted 68 lines
Quoted 68 lines
Quoted 68 lines
Quoted 54 lines
Quoted 54 lines
drivers/fpga/Kconfig +4 -0
+	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
drivers/fpga/Kconfig +2 -0
+
+	 This provides CRC checking to ensure integrated
of
Arria
10 FPGA
drivers/fpga/Kconfig +2 -0
+	 bitstream is programmed into FPGA.
+
config FPGA_CYCLON2
bool "Enable Altera FPGA driver for Cyclone II" depends on FPGA_ALTERA
drivers/fpga/socfpga_arria10.c +1 -1
index 114dd91..d9ad237 100644
@@ -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>
drivers/fpga/socfpga_arria10.c +2 -0
@@ -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>
drivers/fpga/socfpga_arria10.c +0 -0
@@ -21,6 +23,10 @@
  #define COMPRESSION_OFFSET	229
Quoted 8 lines
drivers/fpga/socfpga_arria10.c line 29
Quoted 6 lines
drivers/fpga/socfpga_arria10.c +4 -0
  #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?

Quoted 5 lines

Quoted 4 lines
Quoted 1 line
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.

Quoted 5 lines

same for ... _CORE ... is that a coincidence ?
[...]

Marek Vasut

2018-11-27 12:08 UTC #22 lore raw

PATCH 2/9 ARM: socfpga: Add FPGA drivers for Arria 10 FPGA bitstream loading

Quoted 111 lines · Tien Fong

Quoted 38 lines · Marek Vasut
On 11/26/2018 11:09 AM, Chee, Tien Fong wrote:
[...]
Quoted 30 lines
Quoted 25 lines
Quoted 18 lines
Quoted 18 lines
Quoted 10 lines
drivers/fpga/Kconfig +0 -0
index 50e9019..06a8204 100644
@@ -21,6 +21,15 @@ config FPGA_SOCFPGA
Quoted 6 lines

This provides common functionality for Gen5 and Arria10
devices.

+config CHECK_FPGA_DATA_CRC
config FPGA_SOCFPGA_A10_CRC_CHECK
Quoted 7 lines
Quoted 1 line
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.
Quoted 5 lines
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.
Quoted 6 lines
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.
Quoted 3 lines
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.
Quoted 68 lines
Quoted 68 lines
Quoted 68 lines
Quoted 68 lines
Quoted 54 lines
Quoted 54 lines
drivers/fpga/Kconfig +4 -0
+	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
drivers/fpga/Kconfig +2 -0
+
+	 This provides CRC checking to ensure integrated
of
Arria
10 FPGA
drivers/fpga/Kconfig +2 -0
+	 bitstream is programmed into FPGA.
+
config FPGA_CYCLON2
bool "Enable Altera FPGA driver for Cyclone II" depends on FPGA_ALTERA
drivers/fpga/socfpga_arria10.c +1 -1
index 114dd91..d9ad237 100644
@@ -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>
drivers/fpga/socfpga_arria10.c +2 -0
@@ -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>
drivers/fpga/socfpga_arria10.c +0 -0
@@ -21,6 +23,10 @@
  #define COMPRESSION_OFFSET	229
Quoted 8 lines
drivers/fpga/socfpga_arria10.c line 29
Quoted 6 lines
drivers/fpga/socfpga_arria10.c +4 -0
  #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?

Quoted 8 lines

Quoted 5 lines
Quoted 4 lines
Quoted 1 line
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 ?

Quoted 7 lines

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.
Quoted 4 lines
same for ... _CORE ... is that a coincidence ?
[...]

Signature · 2 lines

Best regards,
Marek Vasut

Chee

2018-11-28 14:53 UTC #23 lore raw

PATCH 2/9 ARM: socfpga: Add FPGA drivers for Arria 10 FPGA bitstream loading

Quoted 147 lines · Marek Vasut

Quoted 146 lines · Tien Fong
Quoted 53 lines · Marek Vasut
On 11/26/2018 11:09 AM, Chee, Tien Fong wrote:
[...]
Quoted 42 lines
Quoted 35 lines
Quoted 27 lines
Quoted 25 lines
Quoted 16 lines
drivers/fpga/Kconfig +0 -0
index 50e9019..06a8204 100644
@@ -21,6 +21,15 @@ config FPGA_SOCFPGA
Quoted 7 lines

This provides common functionality for Gen5 and
Arria10
devices.

+config CHECK_FPGA_DATA_CRC
config FPGA_SOCFPGA_A10_CRC_CHECK
Quoted 2 lines
What is this for and why shouldn't this be ON by default ?
Quoted 8 lines
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.
Quoted 7 lines
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.
Quoted 8 lines
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.
Quoted 4 lines
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.
Quoted 86 lines
Quoted 86 lines
Quoted 75 lines
Quoted 75 lines
Quoted 75 lines
Quoted 69 lines
drivers/fpga/Kconfig +1 -0
+	bool "Enable CRC cheking on Arria10 FPGA
bistream"
drivers/fpga/Kconfig +3 -0
+	depends on FPGA_SOCFPGA
+	help
+	 Say Y here to enable the CRC checking on
Arria 10
FPGA
bitstream
drivers/fpga/Kconfig +2 -0
+
+	 This provides CRC checking to ensure
integrated
of
Arria
10 FPGA
drivers/fpga/Kconfig +2 -0
+	 bitstream is programmed into FPGA.
+
config FPGA_CYCLON2
bool "Enable Altera FPGA driver for Cyclone II"
depends on FPGA_ALTERA
drivers/fpga/socfpga_arria10.c +0 -1
index 114dd91..d9ad237 100644
@@ -1,6 +1,6 @@
  // SPDX-License-Identifier: GPL-2.0
  /*
- * Copyright (C) 2017 Intel Corporation <www.intel.com
Quoted 1 line
+ * Copyright (C) 2017-2018 Intel Corporation <www.inte l.co
m>
*/

#include <asm/io.h>
drivers/fpga/socfpga_arria10.c +2 -0
@@ -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>
drivers/fpga/socfpga_arria10.c +0 -0
@@ -21,6 +23,10 @@
  #define COMPRESSION_OFFSET	229
  #define FPGA_TIMEOUT_MSEC	1000  /* timeout in
ms */
#define FPGA_TIMEOUT_CNT 0x1000000
drivers/fpga/socfpga_arria10.c
Quoted 4 lines
drivers/fpga/socfpga_arria10.c +4 -0
+#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.
Quoted 8 lines
Quoted 7 lines
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.

Quoted 8 lines

Quoted 6 lines
Quoted 1 line
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.

Quoted 13 lines

Quoted 12 lines
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.
Quoted 5 lines
same for ... _CORE ... is that a coincidence ?
[...]

Marek Vasut

2018-11-28 15:11 UTC #24 lore raw

PATCH 2/9 ARM: socfpga: Add FPGA drivers for Arria 10 FPGA bitstream loading

Quoted 149 lines · Tien Fong

Quoted 148 lines · Marek Vasut
Quoted 147 lines · Tien Fong
Quoted 53 lines · Marek Vasut
On 11/26/2018 11:09 AM, Chee, Tien Fong wrote:
[...]
Quoted 42 lines
Quoted 35 lines
Quoted 27 lines
Quoted 25 lines
Quoted 16 lines
drivers/fpga/Kconfig +0 -0
index 50e9019..06a8204 100644
@@ -21,6 +21,15 @@ config FPGA_SOCFPGA
Quoted 7 lines

This provides common functionality for Gen5 and
Arria10
devices.

+config CHECK_FPGA_DATA_CRC
config FPGA_SOCFPGA_A10_CRC_CHECK
Quoted 2 lines
What is this for and why shouldn't this be ON by default ?
Quoted 8 lines
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.
Quoted 7 lines
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.
Quoted 8 lines
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.
Quoted 4 lines
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.
Quoted 87 lines
Quoted 82 lines
Quoted 82 lines
Quoted 82 lines
Quoted 67 lines
Quoted 67 lines
drivers/fpga/Kconfig +1 -0
+	bool "Enable CRC cheking on Arria10 FPGA
bistream"
drivers/fpga/Kconfig +3 -0
+	depends on FPGA_SOCFPGA
+	help
+	 Say Y here to enable the CRC checking on
Arria 10
FPGA
bitstream
drivers/fpga/Kconfig +2 -0
+
+	 This provides CRC checking to ensure
integrated
of
Arria
10 FPGA
drivers/fpga/Kconfig +2 -0
+	 bitstream is programmed into FPGA.
+
config FPGA_CYCLON2
bool "Enable Altera FPGA driver for Cyclone II"
depends on FPGA_ALTERA
drivers/fpga/socfpga_arria10.c +0 -1
index 114dd91..d9ad237 100644
@@ -1,6 +1,6 @@
  // SPDX-License-Identifier: GPL-2.0
  /*
- * Copyright (C) 2017 Intel Corporation <www.intel.com
Quoted 1 line
+ * Copyright (C) 2017-2018 Intel Corporation <www.inte l.co
m>
*/

#include <asm/io.h>
drivers/fpga/socfpga_arria10.c +2 -0
@@ -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>
drivers/fpga/socfpga_arria10.c +0 -0
@@ -21,6 +23,10 @@
  #define COMPRESSION_OFFSET	229
  #define FPGA_TIMEOUT_MSEC	1000  /* timeout in
Quoted 8 lines
drivers/fpga/socfpga_arria10.c
Quoted 6 lines
ms */
#define FPGA_TIMEOUT_CNT 0x1000000
drivers/fpga/socfpga_arria10.c +4 -0
+#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?
Quoted 4 lines
Quoted 1 line
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

Quoted 8 lines

Quoted 7 lines
Quoted 5 lines
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.

Signature · 2 lines

Best regards,
Marek Vasut

tien.fong.chee at intel.com

author

2018-11-21 10:41 UTC #25 lore raw

PATCH 3/9 spl : socfpga: Implement fpga bitstream loading with socfpga loadfs

In reply to tien.fong.chee at intel.com

From: Tien Fong Chee <[email protected]>

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 <[email protected]>

---
 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(-)

arch/arm/mach-socfpga/spl_a10.c +48 -1

index 3ea64f7..67a4fac 100644
@@ -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)

common/spl/spl_mmc.c +1 -1

index 4d55dcc..b85e146 100644
@@ -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;

include/mmc.h +1 -0

index 95548e9..de92909 100644
@@ -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

Signature · 1 line

1.7.7.4

Marek Vasut

2018-11-21 14:19 UTC #26 lore raw

PATCH 3/9 spl : socfpga: Implement fpga bitstream loading with socfpga loadfs

Quoted 54 lines · tien.fong.chee at intel.com

From: Tien Fong Chee <[email protected]>

Signed-off-by Tien Fong Chee <[email protected]>

---
 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(-)

arch/arm/mach-socfpga/spl_a10.c +19 -1

index 3ea64f7..67a4fac 100644
@@ -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();
+

arch/arm/mach-socfpga/spl_a10.c void spl_board_init(void) · line 103

Quoted 10 lines

arch/arm/mach-socfpga/spl_a10.c +10 -0

+		/* 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.

Quoted 47 lines

arch/arm/mach-socfpga/spl_a10.c +18 -0

+			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)

common/spl/spl_mmc.c +1 -1

index 4d55dcc..b85e146 100644
@@ -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;

include/mmc.h +1 -0

index 95548e9..de92909 100644
@@ -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

Signature · 2 lines

Best regards,
Marek Vasut

Chee

2018-11-23 09:51 UTC #27 lore raw

PATCH 3/9 spl : socfpga: Implement fpga bitstream loading with socfpga loadfs

Quoted 67 lines · Marek Vasut

Quoted 66 lines · tien.fong.chee at intel.com
From: Tien Fong Chee <[email protected]>
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 <[email protected]>
---
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(-)
arch/arm/mach- renamed from arch/arm/mach-socfpga/spl_a10.c +0 -0
socfpga/spl_a10.c
index 3ea64f7..67a4fac 100644
arch/arm/mach-socfpga/spl_a10.c +1 -1
@@ -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>
arch/arm/mach-socfpga/spl_a10.c +4 -0
@@ -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;
arch/arm/mach-socfpga/spl_a10.c +0 -0
@@ -73,6 +77,49 @@ void spl_board_init(void)
  	WATCHDOG_RESET();
arch_early_init_r();
arch/arm/mach-socfpga/spl_a10.c +2 -0
+
+	/* If the full FPGA is already loaded, ie.from EPCQ,
config fpga pins */
arch/arm/mach-socfpga/spl_a10.c +13 -0
+	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 */
arch/arm/mach-socfpga/spl_a10.c +4 -0
+		if (bootdev.boot_device == BOOT_DEVICE_MMC1) {
+			struct mmc *mmc = NULL;
+			int err = 0;
+

Quoted 8 lines

arch/arm/mach-socfpga/spl_a10.c void spl_board_init(void) · line 103
Quoted 6 lines
arch/arm/mach-socfpga/spl_a10.c +1 -0
+			err = spl_mmc_find_device(&mmc,
bootdev.boot_device);
arch/arm/mach-socfpga/spl_a10.c +4 -0
+			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.

Quoted 57 lines

Quoted 56 lines
arch/arm/mach-socfpga/spl_a10.c +2 -0
+			if (err) {
+				debug("spl: mmc init failed with
error: %d\n",
arch/arm/mach-socfpga/spl_a10.c +7 -0
+					err);
+
+				return;
+			}
+		}
+
+		fpga_fsinfo.filename = (char
*)get_fpga_filename(gd->fdt_blob,
arch/arm/mach-socfpga/spl_a10.c +1 -0
+								 &
len,
arch/arm/mach-socfpga/spl_a10.c +1 -0
+								 R
BF);
arch/arm/mach-socfpga/spl_a10.c +6 -0
+
+		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())
arch/arm/mach-socfpga/spl_a10.c +1 -0
+		ddr_calibration_sequence();
}

void board_init_f(ulong dummy)
common/spl/spl_mmc.c +0 -0
index 4d55dcc..b85e146 100644
@@ -111,7 +111,7 @@ static int spl_mmc_get_device_index(u32
 boot_device)
  	return -ENODEV;
  }
common/spl/spl_mmc.c +1 -1
-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;
include/mmc.h +1 -0
index 95548e9..de92909 100644
@@ -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

Marek Vasut

2018-11-23 12:31 UTC #28 lore raw

PATCH 3/9 spl : socfpga: Implement fpga bitstream loading with socfpga loadfs

Quoted 76 lines · Tien Fong

Quoted 67 lines · Marek Vasut
Quoted 66 lines · tien.fong.chee at intel.com
From: Tien Fong Chee <[email protected]>
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 <[email protected]>
---
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(-)
arch/arm/mach- renamed from arch/arm/mach-socfpga/spl_a10.c +0 -0
socfpga/spl_a10.c
index 3ea64f7..67a4fac 100644
arch/arm/mach-socfpga/spl_a10.c +1 -1
@@ -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>
arch/arm/mach-socfpga/spl_a10.c +4 -0
@@ -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;
arch/arm/mach-socfpga/spl_a10.c +0 -0
@@ -73,6 +77,49 @@ void spl_board_init(void)
  	WATCHDOG_RESET();
arch_early_init_r();
arch/arm/mach-socfpga/spl_a10.c +2 -0
+
+	/* If the full FPGA is already loaded, ie.from EPCQ,
config fpga pins */
arch/arm/mach-socfpga/spl_a10.c +13 -0
+	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 */
arch/arm/mach-socfpga/spl_a10.c +4 -0
+		if (bootdev.boot_device == BOOT_DEVICE_MMC1) {
+			struct mmc *mmc = NULL;
+			int err = 0;
+
Quoted 8 lines
arch/arm/mach-socfpga/spl_a10.c void spl_board_init(void) · line 103
Quoted 6 lines
arch/arm/mach-socfpga/spl_a10.c +1 -0
+			err = spl_mmc_find_device(&mmc,
bootdev.boot_device);
arch/arm/mach-socfpga/spl_a10.c +4 -0
+			if (err)
+				return;
+
+			err = mmc_init(mmc);
I thought all this backend specific stuff would be hidden in the FW loader.

Quoted 4 lines

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.

Signature · 2 lines

Best regards,
Marek Vasut

Chee

2018-11-26 10:10 UTC #29 lore raw

PATCH 3/9 spl : socfpga: Implement fpga bitstream loading with socfpga loadfs

Quoted 92 lines · Marek Vasut

Quoted 86 lines · Tien Fong
Quoted 76 lines · Marek Vasut
Quoted 74 lines · tien.fong.chee at intel.com
From: Tien Fong Chee <[email protected]>
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 <[email protected]>
---
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(-)
arch/arm/mach- renamed from arch/arm/mach-socfpga/spl_a10.c +0 -0
socfpga/spl_a10.c
index 3ea64f7..67a4fac 100644
arch/arm/mach-socfpga/spl_a10.c +1 -1
@@ -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
Quoted 1 line
*/

#include <common.h>
arch/arm/mach-socfpga/spl_a10.c +4 -0
@@ -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;
arch/arm/mach-socfpga/spl_a10.c +0 -0
@@ -73,6 +77,49 @@ void spl_board_init(void)
  	WATCHDOG_RESET();
arch_early_init_r();
arch/arm/mach-socfpga/spl_a10.c +2 -0
+
+	/* If the full FPGA is already loaded, ie.from EPCQ,
config fpga pins */
arch/arm/mach-socfpga/spl_a10.c +5 -0
+	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 */
arch/arm/mach-socfpga/spl_a10.c +2 -0
+		fpga_fs_info fpga_fsinfo;
+		char buf[16 * 1024]
__aligned(ARCH_DMA_MINALIGN);
arch/arm/mach-socfpga/spl_a10.c +6 -0
+		struct spl_boot_device bootdev;
+		int len = 0;
+
+		bootdev.boot_device = spl_boot_device();
+
+		/* Init MMC driver before reading FPGA
bitstream
from flash */
arch/arm/mach-socfpga/spl_a10.c +5 -0
+		if (bootdev.boot_device == BOOT_DEVICE_MMC1) {
+			struct mmc *mmc = NULL;
+			int err = 0;
+
+			err = spl_mmc_find_device(&mmc,
Quoted 8 lines
arch/arm/mach-socfpga/spl_a10.c void spl_board_init(void) · line 103
Quoted 5 lines
bootdev.boot_device);
arch/arm/mach-socfpga/spl_a10.c +4 -0
+			if (err)
+				return;
+
+			err = mmc_init(mmc);
I thought all this backend specific stuff would be hidden in the FW
loader.
Quoted 5 lines
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.

Quoted 4 lines

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.

Quoted 1 line

Marek Vasut

2018-11-26 11:20 UTC #30 lore raw

PATCH 3/9 spl : socfpga: Implement fpga bitstream loading with socfpga loadfs

Quoted 93 lines · Tien Fong

Quoted 92 lines · Marek Vasut
Quoted 91 lines · Tien Fong
Quoted 76 lines · Marek Vasut
Quoted 74 lines · tien.fong.chee at intel.com
From: Tien Fong Chee <[email protected]>
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 <[email protected]>
---
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(-)
arch/arm/mach- renamed from arch/arm/mach-socfpga/spl_a10.c +0 -0
socfpga/spl_a10.c
index 3ea64f7..67a4fac 100644
arch/arm/mach-socfpga/spl_a10.c +1 -1
@@ -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
Quoted 1 line
*/

#include <common.h>
arch/arm/mach-socfpga/spl_a10.c +4 -0
@@ -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;
arch/arm/mach-socfpga/spl_a10.c +0 -0
@@ -73,6 +77,49 @@ void spl_board_init(void)
  	WATCHDOG_RESET();
arch_early_init_r();
arch/arm/mach-socfpga/spl_a10.c +2 -0
+
+	/* If the full FPGA is already loaded, ie.from EPCQ,
config fpga pins */
arch/arm/mach-socfpga/spl_a10.c +5 -0
+	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 */
arch/arm/mach-socfpga/spl_a10.c +2 -0
+		fpga_fs_info fpga_fsinfo;
+		char buf[16 * 1024]
__aligned(ARCH_DMA_MINALIGN);
arch/arm/mach-socfpga/spl_a10.c +6 -0
+		struct spl_boot_device bootdev;
+		int len = 0;
+
+		bootdev.boot_device = spl_boot_device();
+
+		/* Init MMC driver before reading FPGA
bitstream
from flash */
arch/arm/mach-socfpga/spl_a10.c +5 -0
+		if (bootdev.boot_device == BOOT_DEVICE_MMC1) {
+			struct mmc *mmc = NULL;
+			int err = 0;
+
+			err = spl_mmc_find_device(&mmc,
Quoted 8 lines
arch/arm/mach-socfpga/spl_a10.c void spl_board_init(void) · line 103
Quoted 5 lines
bootdev.boot_device);
arch/arm/mach-socfpga/spl_a10.c +4 -0
+			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.

Quoted 6 lines

Quoted 4 lines
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.

Signature · 2 lines

Best regards,
Marek Vasut

Chee

2018-11-27 08:55 UTC #31 lore raw

PATCH 3/9 spl : socfpga: Implement fpga bitstream loading with socfpga loadfs

Quoted 114 lines · Marek Vasut

Quoted 113 lines · Tien Fong
Quoted 111 lines · Marek Vasut
Quoted 97 lines · Tien Fong
Quoted 86 lines · Marek Vasut
Quoted 83 lines · tien.fong.chee at intel.com
From: Tien Fong Chee <[email protected]>
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 <[email protected]>
---
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(-)
arch/arm/mach- renamed from arch/arm/mach-socfpga/spl_a10.c +0 -0
socfpga/spl_a10.c
index 3ea64f7..67a4fac 100644
arch/arm/mach-socfpga/spl_a10.c +1 -1
@@ -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
Quoted 2 lines
*/

#include <common.h>
arch/arm/mach-socfpga/spl_a10.c +4 -0
@@ -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;
arch/arm/mach-socfpga/spl_a10.c +0 -0
@@ -73,6 +77,49 @@ void spl_board_init(void)
  	WATCHDOG_RESET();
arch_early_init_r();
arch/arm/mach-socfpga/spl_a10.c +2 -0
+
+	/* If the full FPGA is already loaded, ie.from
EPCQ,
config fpga pins */
arch/arm/mach-socfpga/spl_a10.c +5 -0
+	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 */
arch/arm/mach-socfpga/spl_a10.c +2 -0
+		fpga_fs_info fpga_fsinfo;
+		char buf[16 * 1024]
__aligned(ARCH_DMA_MINALIGN);
arch/arm/mach-socfpga/spl_a10.c +6 -0
+		struct spl_boot_device bootdev;
+		int len = 0;
+
+		bootdev.boot_device = spl_boot_device();
+
+		/* Init MMC driver before reading FPGA
bitstream
from flash */
arch/arm/mach-socfpga/spl_a10.c +1 -0
+		if (bootdev.boot_device ==
BOOT_DEVICE_MMC1) {
arch/arm/mach-socfpga/spl_a10.c +4 -0
+			struct mmc *mmc = NULL;
+			int err = 0;
+
+			err = spl_mmc_find_device(&mmc,
bootdev.boot_device);
Quoted 8 lines
arch/arm/mach-socfpga/spl_a10.c void spl_board_init(void) · line 103
Quoted 4 lines
arch/arm/mach-socfpga/spl_a10.c +4 -0
+			if (err)
+				return;
+
+			err = mmc_init(mmc);
I thought all this backend specific stuff would be hidden in the
FW
loader.
Quoted 6 lines
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.

Quoted 6 lines

Quoted 3 lines
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.

Quoted 1 line

Marek Vasut

2018-11-27 12:08 UTC #32 lore raw

PATCH 3/9 spl : socfpga: Implement fpga bitstream loading with socfpga loadfs

Quoted 115 lines · Tien Fong

Quoted 114 lines · Marek Vasut
Quoted 113 lines · Tien Fong
Quoted 111 lines · Marek Vasut
Quoted 97 lines · Tien Fong
Quoted 86 lines · Marek Vasut
Quoted 83 lines · tien.fong.chee at intel.com
From: Tien Fong Chee <[email protected]>
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 <[email protected]>
---
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(-)
arch/arm/mach- renamed from arch/arm/mach-socfpga/spl_a10.c +0 -0
socfpga/spl_a10.c
index 3ea64f7..67a4fac 100644
arch/arm/mach-socfpga/spl_a10.c +1 -1
@@ -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
Quoted 2 lines
*/

#include <common.h>
arch/arm/mach-socfpga/spl_a10.c +4 -0
@@ -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;
arch/arm/mach-socfpga/spl_a10.c +0 -0
@@ -73,6 +77,49 @@ void spl_board_init(void)
  	WATCHDOG_RESET();
arch_early_init_r();
arch/arm/mach-socfpga/spl_a10.c +2 -0
+
+	/* If the full FPGA is already loaded, ie.from
EPCQ,
config fpga pins */
arch/arm/mach-socfpga/spl_a10.c +5 -0
+	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 */
arch/arm/mach-socfpga/spl_a10.c +2 -0
+		fpga_fs_info fpga_fsinfo;
+		char buf[16 * 1024]
__aligned(ARCH_DMA_MINALIGN);
arch/arm/mach-socfpga/spl_a10.c +6 -0
+		struct spl_boot_device bootdev;
+		int len = 0;
+
+		bootdev.boot_device = spl_boot_device();
+
+		/* Init MMC driver before reading FPGA
bitstream
from flash */
arch/arm/mach-socfpga/spl_a10.c +1 -0
+		if (bootdev.boot_device ==
BOOT_DEVICE_MMC1) {
arch/arm/mach-socfpga/spl_a10.c +4 -0
+			struct mmc *mmc = NULL;
+			int err = 0;
+
+			err = spl_mmc_find_device(&mmc,
bootdev.boot_device);
Quoted 8 lines
arch/arm/mach-socfpga/spl_a10.c
Quoted 4 lines
arch/arm/mach-socfpga/spl_a10.c +4 -0
+			if (err)
+				return;
+
+			err = mmc_init(mmc);
I thought all this backend specific stuff would be hidden in the
FW
loader.
Quoted 6 lines
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.

Quoted 7 lines

Quoted 6 lines
Quoted 3 lines
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.

Signature · 2 lines

Best regards,
Marek Vasut

tien.fong.chee at intel.com

author

2018-11-21 10:41 UTC #33 lore raw

PATCH 4/9 ARM: socfpga: Bundle U-Boot fitImage into SFP on Arria10

In reply to tien.fong.chee at intel.com

From: Tien Fong Chee <[email protected]>

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 <[email protected]>

Signed-off-by Tien Fong Chee <[email protected]>

---
 Makefile                         |    9 +++++++--
 include/configs/socfpga_common.h |    4 ++++
 2 files changed, 11 insertions(+), 2 deletions(-)

Makefile +7 -2

index a55915d..4ecc19d 100644
@@ -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)

include/configs/socfpga_common.h +4 -0

index bd8f5c8..ffdc6eb 100644
@@ -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

Signature · 1 line

1.7.7.4

Marek Vasut

2018-11-21 14:21 UTC #34 lore raw

PATCH 4/9 ARM: socfpga: Bundle U-Boot fitImage into SFP on Arria10

Quoted 1 line · tien.fong.chee at intel.com

From: Tien Fong Chee <[email protected]>

Did you change Author:ship of the patch ?

Quoted 8 lines

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 ?

Quoted 45 lines

Signed-off-by Marek Vasut <[email protected]>

Signed-off-by Tien Fong Chee <[email protected]>

---
 Makefile                         |    9 +++++++--
 include/configs/socfpga_common.h |    4 ++++
 2 files changed, 11 insertions(+), 2 deletions(-)

Makefile +7 -2

index a55915d..4ecc19d 100644
@@ -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)

include/configs/socfpga_common.h +4 -0

index bd8f5c8..ffdc6eb 100644
@@ -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

Signature · 2 lines

Best regards,
Marek Vasut

Chee

2018-11-23 09:54 UTC #35 lore raw

PATCH 4/9 ARM: socfpga: Bundle U-Boot fitImage into SFP on Arria10

Quoted 16 lines · Marek Vasut

Quoted 2 lines · tien.fong.chee at intel.com
From: Tien Fong Chee <[email protected]>
Did you change Author:ship of the patch ?
Quoted 11 lines
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.

Quoted 3 lines

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.

Quoted 55 lines

Quoted 54 lines
Signed-off-by Marek Vasut <[email protected]>
Signed-off-by Tien Fong Chee <[email protected]>
---
Makefile | 9 +++++++-- include/configs/socfpga_common.h | 4 ++++
 2 files changed, 11 insertions(+), 2 deletions(-)
Makefile +0 -0
index a55915d..4ecc19d 100644
@@ -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 \
Makefile +5 -1
-			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
Makefile +2 -1
-	$(call if_changed,socboot)
+	$(call if_changed,socboot,u-boot.img)
+endif
endif

ifeq ($(CONFIG_MPC85xx)$(CONFIG_OF_SEPARATE),yy)
include/configs/socfpga_common.h +2 -0
index bd8f5c8..ffdc6eb 100644
@@ -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

Marek Vasut

2018-11-23 12:40 UTC #36 lore raw

PATCH 4/9 ARM: socfpga: Bundle U-Boot fitImage into SFP on Arria10

Quoted 5 lines · Tien Fong

Quoted 4 lines · Marek Vasut
Quoted 2 lines · tien.fong.chee at intel.com
From: Tien Fong Chee <[email protected]>
Did you change Author:ship of the patch ?

I believe you did, so please fix that.

Quoted 10 lines

Quoted 10 lines
Quoted 10 lines
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.

Quoted 6 lines

Quoted 3 lines
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 ?

Signature · 2 lines

Best regards,
Marek Vasut

Chee

2018-11-26 10:30 UTC #37 lore raw

PATCH 4/9 ARM: socfpga: Bundle U-Boot fitImage into SFP on Arria10

Quoted 10 lines · Marek Vasut

Quoted 8 lines · Tien Fong
Quoted 6 lines · Marek Vasut
Quoted 3 lines · tien.fong.chee at intel.com
From: Tien Fong Chee <[email protected]>
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.

Quoted 33 lines

Quoted 22 lines
Quoted 22 lines
Quoted 22 lines
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.
Quoted 8 lines
Quoted 4 lines
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 ?

Quoted 6 lines

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.

Quoted 1 line

Marek Vasut

2018-11-26 11:22 UTC #38 lore raw

PATCH 4/9 ARM: socfpga: Bundle U-Boot fitImage into SFP on Arria10

Quoted 5 lines · Tien Fong

Quoted 4 lines · Marek Vasut
Quoted 3 lines · Tien Fong
Quoted 1 line · Marek Vasut

Quoted 7 lines

Quoted 6 lines
Quoted 5 lines
Quoted 5 lines · tien.fong.chee at intel.com
Quoted 3 lines
From: Tien Fong Chee <[email protected]>
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.

Quoted 39 lines

Quoted 33 lines
Quoted 22 lines
Quoted 22 lines
Quoted 22 lines
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.
Quoted 8 lines
Quoted 4 lines
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 ?
Quoted 6 lines
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 ?

Quoted 4 lines

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 ?

Quoted 3 lines

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 ?

Signature · 2 lines

Best regards,
Marek Vasut

Chee

2018-11-27 09:00 UTC #39 lore raw

PATCH 4/9 ARM: socfpga: Bundle U-Boot fitImage into SFP on Arria10

Quoted 14 lines · Marek Vasut

Quoted 13 lines · Tien Fong
Quoted 11 lines · Marek Vasut
Quoted 9 lines · Tien Fong
Quoted 6 lines · Marek Vasut
Quoted 3 lines · tien.fong.chee at intel.com

Quoted 5 lines

Quoted 4 lines
Quoted 3 lines
Quoted 2 lines
Quoted 2 lines
Quoted 1 line
From: Tien Fong Chee <[email protected]>
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.

Quoted 58 lines

Quoted 53 lines
Quoted 46 lines
Quoted 37 lines
Quoted 32 lines
Quoted 24 lines
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
Quoted 8 lines
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.
Quoted 5 lines
Right, this is a stopgap measure until FW loader is present. Why
is
this
patch needed at all in this series ?
Quoted 5 lines
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 ?
Quoted 7 lines
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 ?
Quoted 5 lines
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.

Quoted 6 lines

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.

Quoted 9 lines

Quoted 7 lines
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 ?

Marek Vasut

2018-11-27 12:09 UTC #40 lore raw

PATCH 4/9 ARM: socfpga: Bundle U-Boot fitImage into SFP on Arria10

Quoted 85 lines · Tien Fong

Quoted 14 lines · Marek Vasut
Quoted 13 lines · Tien Fong
Quoted 11 lines · Marek Vasut
Quoted 9 lines · Tien Fong
Quoted 6 lines · Marek Vasut
Quoted 3 lines · tien.fong.chee at intel.com
Quoted 5 lines
Quoted 4 lines
Quoted 3 lines
Quoted 2 lines
Quoted 2 lines
Quoted 1 line
From: Tien Fong Chee <[email protected]>
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.
Quoted 58 lines
Quoted 53 lines
Quoted 46 lines
Quoted 37 lines
Quoted 32 lines
Quoted 24 lines
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
Quoted 8 lines
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.
Quoted 5 lines
Right, this is a stopgap measure until FW loader is present. Why
is
this
patch needed at all in this series ?
Quoted 5 lines
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 ?
Quoted 7 lines
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 ?
Quoted 5 lines
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.
Quoted 6 lines
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 ?

Quoted 3 lines

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 ?

Signature · 2 lines

Best regards,
Marek Vasut

Chee

2018-11-28 14:43 UTC #41 lore raw

PATCH 4/9 ARM: socfpga: Bundle U-Boot fitImage into SFP on Arria10

Quoted 117 lines · Marek Vasut

Quoted 116 lines · Tien Fong
Quoted 20 lines · Marek Vasut
Quoted 18 lines · Tien Fong
Quoted 15 lines · Marek Vasut
Quoted 12 lines · Tien Fong
Quoted 8 lines · Marek Vasut
Quoted 4 lines · tien.fong.chee at intel.com
Quoted 5 lines
Quoted 4 lines
Quoted 3 lines
Quoted 2 lines
Quoted 2 lines
Quoted 1 line
From: Tien Fong Chee <[email protected]>
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.
Quoted 88 lines
Quoted 73 lines
Quoted 64 lines
Quoted 51 lines
Quoted 45 lines
Quoted 37 lines
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
Quoted 8 lines
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.
Quoted 6 lines
Right, this is a stopgap measure until FW loader is present.
Why
is
this
patch needed at all in this series ?
Quoted 7 lines
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 ?
Quoted 9 lines
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 ?
Quoted 7 lines
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 ?

Quoted 6 lines

Quoted 5 lines
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.

Quoted 1 line

Marek Vasut

2018-11-28 15:11 UTC #42 lore raw

PATCH 4/9 ARM: socfpga: Bundle U-Boot fitImage into SFP on Arria10

Quoted 123 lines · Tien Fong

Quoted 122 lines · Marek Vasut
Quoted 121 lines · Tien Fong
Quoted 20 lines · Marek Vasut
Quoted 18 lines · Tien Fong
Quoted 15 lines · Marek Vasut
Quoted 12 lines · Tien Fong
Quoted 8 lines · Marek Vasut
Quoted 4 lines · tien.fong.chee at intel.com
Quoted 5 lines
Quoted 4 lines
Quoted 3 lines
Quoted 2 lines
Quoted 2 lines
Quoted 1 line
From: Tien Fong Chee <[email protected]>
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.
Quoted 80 lines
Quoted 73 lines
Quoted 64 lines
Quoted 51 lines
Quoted 45 lines
Quoted 37 lines
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
Quoted 8 lines
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.
Quoted 6 lines
Right, this is a stopgap measure until FW loader is present.
Why
is
this
patch needed at all in this series ?
Quoted 7 lines
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 ?
Quoted 9 lines
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 ?
Quoted 7 lines
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.
Quoted 8 lines
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.

Quoted 4 lines

Quoted 1 line
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.

Signature · 2 lines

Best regards,
Marek Vasut

tien.fong.chee at intel.com

author

2018-11-21 10:41 UTC #43 lore raw

PATCH 5/9 ARM: socfpga: Add SPL fitImage config match

In reply to tien.fong.chee at intel.com

From: Tien Fong Chee <[email protected]>

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 <[email protected]>

---
 arch/arm/mach-socfpga/spl_a10.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

arch/arm/mach-socfpga/spl_a10.c +10 -0

index 67a4fac..2baeba6 100644
@@ -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

Signature · 1 line

1.7.7.4

Marek Vasut

2018-11-21 14:21 UTC #44 lore raw

PATCH 5/9 ARM: socfpga: Add SPL fitImage config match

Quoted 7 lines · tien.fong.chee at intel.com

From: Tien Fong Chee <[email protected]>
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 <[email protected]>

It's missing your SoB line, but again, why is this patch needed ?

Quoted 23 lines

---
 arch/arm/mach-socfpga/spl_a10.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

arch/arm/mach-socfpga/spl_a10.c +10 -0

index 67a4fac..2baeba6 100644
@@ -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

Signature · 2 lines

Best regards,
Marek Vasut

Chee

2018-11-23 10:05 UTC #45 lore raw

PATCH 5/9 ARM: socfpga: Add SPL fitImage config match

Quoted 10 lines · Marek Vasut

Quoted 8 lines · tien.fong.chee at intel.com
From: Tien Fong Chee <[email protected]>
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 <[email protected]>
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'"

Quoted 28 lines

Quoted 27 lines
---
arch/arm/mach-socfpga/spl_a10.c | 10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)
arch/arm/mach- renamed from arch/arm/mach-socfpga/spl_a10.c +0 -0
socfpga/spl_a10.c
index 67a4fac..2baeba6 100644
arch/arm/mach-socfpga/spl_a10.c +5 -0
@@ -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
*/
arch/arm/mach-socfpga/spl_a10.c +5 -0
+	debug("%s: %s\n", __func__, name);
+
+	return 0;
+}
+#endif

Marek Vasut

2018-11-23 12:34 UTC #46 lore raw

PATCH 5/9 ARM: socfpga: Add SPL fitImage config match

Quoted 5 lines · Tien Fong

Quoted 4 lines · Marek Vasut
Quoted 3 lines · tien.fong.chee at intel.com
From: Tien Fong Chee <[email protected]>

Quoted 8 lines

Quoted 6 lines
Quoted 5 lines
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 <[email protected]>
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

Quoted 3 lines

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.

Quoted 25 lines

Quoted 25 lines
Quoted 25 lines
---
arch/arm/mach-socfpga/spl_a10.c | 10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)
arch/arm/mach- renamed from arch/arm/mach-socfpga/spl_a10.c +0 -0
socfpga/spl_a10.c
index 67a4fac..2baeba6 100644
arch/arm/mach-socfpga/spl_a10.c +5 -0
@@ -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
*/
arch/arm/mach-socfpga/spl_a10.c +5 -0
+	debug("%s: %s\n", __func__, name);
+
+	return 0;
+}
+#endif

Signature · 2 lines

Best regards,
Marek Vasut

Chee

2018-11-26 10:11 UTC #47 lore raw

PATCH 5/9 ARM: socfpga: Add SPL fitImage config match

Quoted 15 lines · Marek Vasut

Quoted 14 lines · Tien Fong
Quoted 12 lines · Marek Vasut
Quoted 10 lines · tien.fong.chee at intel.com
From: Tien Fong Chee <[email protected]>
Add empty SPL fitImage configuration match. This can be extended
if there is ever need to support multiple boards with single SFP
image.

Quoted 6 lines

Quoted 5 lines
Quoted 2 lines
Quoted 1 line
Signed-off-by Marek Vasut <[email protected]>
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.

Quoted 6 lines

Quoted 5 lines
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.

Quoted 31 lines

Quoted 30 lines
Quoted 30 lines
Quoted 30 lines
---
arch/arm/mach-socfpga/spl_a10.c | 10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)
arch/arm/mach- renamed from arch/arm/mach-socfpga/spl_a10.c +0 -0
socfpga/spl_a10.c
index 67a4fac..2baeba6 100644
arch/arm/mach-socfpga/spl_a10.c +5 -0
@@ -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
*/
arch/arm/mach-socfpga/spl_a10.c +5 -0
+	debug("%s: %s\n", __func__, name);
+
+	return 0;
+}
+#endif

tien.fong.chee at intel.com

author

2018-11-21 10:41 UTC #48 lore raw

PATCH 6/9 ARM: socfpga: Set default DTB address on A10

In reply to tien.fong.chee at intel.com

From: Tien Fong Chee <[email protected]>

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 <[email protected]>

Signed-off-by Tien Fong Chee <[email protected]>

---
 arch/arm/mach-socfpga/board.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

arch/arm/mach-socfpga/board.c +8 -0

index 7c8c05c..436a8a8 100644
@@ -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

Signature · 1 line

1.7.7.4

Marek Vasut

2018-11-21 14:22 UTC #49 lore raw

PATCH 6/9 ARM: socfpga: Set default DTB address on A10

Quoted 1 line · tien.fong.chee at intel.com

From: Tien Fong Chee <[email protected]>

Again, not your patch, please don't change authorship.

http://git.denx.de/?p=u-boot/u-boot-socfpga.git;a=commit;h=eb32e568869b5f91fe34efb2642875a8da5f0ebd

Quoted 3 lines

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 ?

Quoted 23 lines

Signed-off-by Marek Vasut <[email protected]>

Signed-off-by Tien Fong Chee <[email protected]>

---
 arch/arm/mach-socfpga/board.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

arch/arm/mach-socfpga/board.c +8 -0

index 7c8c05c..436a8a8 100644
@@ -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

Signature · 2 lines

Best regards,
Marek Vasut

Chee

2018-11-23 10:10 UTC #50 lore raw

PATCH 6/9 ARM: socfpga: Set default DTB address on A10

Quoted 5 lines · Marek Vasut

Quoted 2 lines · tien.fong.chee at intel.com
From: Tien Fong Chee <[email protected]>
Again, not your patch, please don't change authorship.

Quoted 8 lines

http://git.denx.de/?p=u-boot/u-boot-socfpga.git;a=commit;h=eb32e56886
9b5f91fe34efb2642875a8da5f0ebd
Quoted 4 lines
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.

Quoted 27 lines

Quoted 26 lines
Signed-off-by Marek Vasut <[email protected]>
Signed-off-by Tien Fong Chee <[email protected]>
---
arch/arm/mach-socfpga/board.c | 8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)
arch/arm/mach- renamed from arch/arm/mach-socfpga/board.c +0 -0
socfpga/board.c
index 7c8c05c..436a8a8 100644
arch/arm/mach-socfpga/board.c +8 -0
@@ -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

Marek Vasut

2018-11-23 12:39 UTC #51 lore raw

PATCH 6/9 ARM: socfpga: Set default DTB address on A10

Quoted 10 lines · Tien Fong

Quoted 9 lines · Marek Vasut
Quoted 2 lines · tien.fong.chee at intel.com
From: Tien Fong Chee <[email protected]>
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
Quoted 1 line

Quoted 7 lines

Quoted 4 lines
Quoted 3 lines
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.

Quoted 24 lines

Quoted 24 lines
Quoted 24 lines
Signed-off-by Marek Vasut <[email protected]>
Signed-off-by Tien Fong Chee <[email protected]>
---
arch/arm/mach-socfpga/board.c | 8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)
arch/arm/mach- renamed from arch/arm/mach-socfpga/board.c +0 -0
socfpga/board.c
index 7c8c05c..436a8a8 100644
arch/arm/mach-socfpga/board.c +8 -0
@@ -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

Signature · 2 lines

Best regards,
Marek Vasut

tien.fong.chee at intel.com

author

2018-11-21 10:41 UTC #52 lore raw

PATCH 7/9 ARM: socfpga: Use custom header target buffer in SPL

In reply to tien.fong.chee at intel.com

From: Tien Fong Chee <[email protected]>

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 <[email protected]>

---
 arch/arm/mach-socfpga/spl_a10.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

arch/arm/mach-socfpga/spl_a10.c +11 -0

index 2baeba6..7fe9790 100644
@@ -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

Signature · 1 line

1.7.7.4

tien.fong.chee at intel.com

author

2018-11-21 10:41 UTC #53 lore raw

PATCH 8/9 ARM: socfpga: Add default fitImage for Arria10 SoCDK

In reply to tien.fong.chee at intel.com

From: Tien Fong Chee <[email protected]>

Add default fitImage file bundling U-Boot and FPGA bitstream for Arria10.

Signed-off-by Marek Vasut <[email protected]>

Signed-off-by Tien Fong Chee <[email protected]>

---
 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

board/altera/arria10-socdk/fit_spl_fpga.its new file +54 -0

new file mode 100644
index 0000000..7782740
@@ -0,0 +1,54 @@
+/*
+ * The fitImage source for Arria 10 SoCDK loading FPGA bitstream and U-Boot
+ *
+ * Copyright (C) 2018 Marek Vasut <[email protected]>
+ *
+ * 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";
+		};
+	};
+};

Signature · 1 line

1.7.7.4

tien.fong.chee at intel.com

author

2018-11-21 10:41 UTC #54 lore raw

PATCH 9/9 ARM: socfpga: Synchronize the configuration for A10 SoCDK

In reply to tien.fong.chee at intel.com

From: Tien Fong Chee <[email protected]>

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 <[email protected]>

Signed-off-by Tien Fong Chee <[email protected]>

---
 configs/socfpga_arria10_defconfig |   42 +++++++++++++++++++++++++++++++-----
 1 files changed, 36 insertions(+), 6 deletions(-)

configs/socfpga_arria10_defconfig +36 -6

index f88910c..86f078b 100644
@@ -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

Signature · 1 line

1.7.7.4