Add FPGA, SDRAM, SPL loads U-boot & booting to console

From: Tien Fong Chee <[email protected]>

This DT binding doc is porting from Linux DT binding doc. commit 1adcbea4201a6852362aa5ece573f1f169b28113

Add a device tree bindings document for the SoCFPGA Arria10 FPGA Manager driver.

Signed-off-by Alan Tull <[email protected]>

Acked-by Rob Herring <[email protected]>

Acked-by Moritz Fischer <[email protected]>

Signed-off-by Rob Herring <[email protected]>

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

---
 .../fpga/altera-socfpga-a10-fpga-mgr.txt              | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
 create mode 100644 doc/device-tree-bindings/fpga/altera-socfpga-a10-fpga-mgr.txt

doc/device-tree-bindings/fpga/altera-socfpga-a10-fpga-mgr.txt new file +19 -0

new file mode 100644
index 0000000..2fd8e7a
@@ -0,0 +1,19 @@
+Altera SOCFPGA Arria10 FPGA Manager
+
+Required properties:
+- compatible : should contain "altr,socfpga-a10-fpga-mgr"
+- reg        : base address and size for memory mapped io.
+               - The first index is for FPGA manager register access.
+               - 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.
+
+Example:
+
+	fpga_mgr: fpga-mgr at ffd03000 {
+		compatible = "altr,socfpga-a10-fpga-mgr";
+		reg = <0xffd03000 0x100
+		       0xffcfe400 0x20>;
+		clocks = <&l4_mp_clk>;
+		resets = <&rst FPGAMGR_RESET>;
+	};

From: Tien Fong Chee <[email protected]>

This patch adds description on properties about location of FPGA RBFs are stored, type and functionality of RBF used to configure FPGA.

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

---
 doc/device-tree-bindings/fpga/altera-socfpga-a10-fpga-mgr.txt | 11 +++++++++++
 1 file changed, 11 insertions(+)

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

index 2fd8e7a..7abb746 100644
@@ -7,6 +7,14 @@ 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.
+- bitstream_periph : FPGA peripheral raw binary file which is used to
+                     initialize FPGA IOs, PLL, IO48 and DDR.
+- bitstream_core : FPGA core raw binary file contains FPGA design which is used
+                   to program FPGA CRAM and ERAM.
+- bitstream_devpart : Partition of flash device where bitstream files are
+		       stored.
+                      <dev[:part]> - dev is flash device number, part is flash
+                                     device partition.
 
 Example:
 

@@ -16,4 +24,7 @@ Example:
 		       0xffcfe400 0x20>;
 		clocks = <&l4_mp_clk>;
 		resets = <&rst FPGAMGR_RESET>;
+		bitstream_periph = "ghrd_10as066n2.periph.rbf.mkimage";
+		bitstream_core = "ghrd_10as066n2.core.rbf.mkimage";
+		bitstream_devpart = "0:1";
 	};

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

From: Tien Fong Chee <[email protected]>

Quoted 5 lines

This patch adds description on properties about location of FPGA RBFs are
stored, type and functionality of RBF used to configure FPGA.

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

Why does this patch have different tags than 1/19 ? Please keep things
consistent ...

Quoted 43 lines

---
 doc/device-tree-bindings/fpga/altera-socfpga-a10-fpga-mgr.txt | 11
+++++++++++

 1 file changed, 11 insertions(+)

(quoted diff) +0 -0

mgr.txt b/doc/device-tree-bindings/fpga/altera-socfpga-a10-fpga- mgr.txt
index 2fd8e7a..7abb746 100644

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

@@ -7,6 +7,14 @@ 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.
+- bitstream_periph : FPGA peripheral raw binary file which is used to
+                     initialize FPGA IOs, PLL, IO48 and DDR. +- bitstream_core : FPGA core raw binary file contains FPGA design which is used
+                   to program FPGA CRAM and ERAM.
+- bitstream_devpart : Partition of flash device where bitstream files are

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

+		       stored.
+                      <dev[:part]> - dev is flash device number,

part is flash
+                                     device partition.
 Example:

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

@@ -16,4 +24,7 @@ Example:
  		       0xffcfe400 0x20>;
  		clocks = <&l4_mp_clk>;
  		resets = <&rst FPGAMGR_RESET>;
+		bitstream_periph =
 "ghrd_10as066n2.periph.rbf.mkimage";
+		bitstream_core =

"ghrd_10as066n2.core.rbf.mkimage";

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

+		bitstream_devpart = "0:1";

  };

Quoted 8 lines · Marek Vasut

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

From: Tien Fong Chee <[email protected]>

This patch adds description on properties about location of FPGA RBFs are
stored, type and functionality of RBF used to configure FPGA.

Quoted 5 lines

Quoted 1 line

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

Why does this patch have different tags than 1/19 ? Please keep things
consistent ...

Not get you. What's you means for tags?

Quoted 42 lines

Quoted 42 lines

---
 doc/device-tree-bindings/fpga/altera-socfpga-a10-fpga-mgr.txt | 11
+++++++++++

 1 file changed, 11 insertions(+)

(quoted diff) +0 -0

mgr.txt b/doc/device-tree-bindings/fpga/altera-socfpga-a10-fpga- mgr.txt
index 2fd8e7a..7abb746 100644

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

@@ -7,6 +7,14 @@ 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.
+- bitstream_periph : FPGA peripheral raw binary file which is used to
+                     initialize FPGA IOs, PLL, IO48 and DDR. +- bitstream_core : FPGA core raw binary file contains FPGA design which is used
+                   to program FPGA CRAM and ERAM.
+- bitstream_devpart : Partition of flash device where bitstream files are

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

+		       stored.
+                      <dev[:part]> - dev is flash device number,

part is flash
+                                     device partition.
 Example:

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

@@ -16,4 +24,7 @@ Example:
  		       0xffcfe400 0x20>;
  		clocks = <&l4_mp_clk>;
  		resets = <&rst FPGAMGR_RESET>;
+		bitstream_periph =
 "ghrd_10as066n2.periph.rbf.mkimage";
+		bitstream_core =

"ghrd_10as066n2.core.rbf.mkimage";

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

+		bitstream_devpart = "0:1";

  };

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

This patch adds description on properties about location of FPGA
RBFs are
stored, type and functionality of RBF used to configure FPGA.

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

Quoted 5 lines

Quoted 4 lines

Why does this patch have different tags than 1/19 ? Please keep things
consistent ...

Not get you. What's you means for tags?

ARM: socfpga: , not the random doc: dtbinding: . Heck, the first and second patch change the same file, yet have different tags, why ?

Quoted 55 lines

Quoted 55 lines

Quoted 55 lines

---
 doc/device-tree-bindings/fpga/altera-socfpga-a10-fpga-mgr.txt | 11
+++++++++++

 1 file changed, 11 insertions(+)

(quoted diff) +0 -0

fpga-
mgr.txt b/doc/device-tree-bindings/fpga/altera-socfpga-a10- fpga-
mgr.txt
index 2fd8e7a..7abb746 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,14 @@ 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.
+- bitstream_periph : FPGA peripheral raw binary file which is used
to
+                     initialize FPGA IOs, PLL, IO48 and DDR. +- bitstream_core : FPGA core raw binary file contains FPGA design
which is used
+                   to program FPGA CRAM and ERAM. +- bitstream_devpart : Partition of flash device where bitstream
files are

(quoted diff) +2 -0

+		       stored.
+                      <dev[:part]> - dev is flash device

number,
part is flash
+                                     device partition.
 Example:

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

@@ -16,4 +24,7 @@ Example:
  		       0xffcfe400 0x20>;
  		clocks = <&l4_mp_clk>;
  		resets = <&rst FPGAMGR_RESET>;
+		bitstream_periph =
 "ghrd_10as066n2.periph.rbf.mkimage";
+		bitstream_core =

"ghrd_10as066n2.core.rbf.mkimage";

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

+		bitstream_devpart = "0:1";

  };

Quoted 20 lines · Marek Vasut

Quoted 19 lines · Tien Fong

Quoted 11 lines · Marek Vasut

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

From: Tien Fong Chee <[email protected]>

This patch adds description on properties about location of FPGA
RBFs are
stored, type and functionality of RBF used to configure FPGA.

Quoted 5 lines

Quoted 1 line

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

Why does this patch have different tags than 1/19 ? Please keep things
consistent ...

Not get you. What's you means for tags?

Quoted 4 lines

ARM: socfpga: , not the random doc: dtbinding: . Heck, the first and second patch change the same file, yet have different tags, why ?

I ported patch 01 from Linux, so i keep everything intact. For patch 02, i put doc:dtbinding because i think that is more descriptive to the file i changed.
I can change to ARM:socfpga .

Quoted 55 lines

Quoted 55 lines

Quoted 55 lines

Quoted 55 lines

---
 doc/device-tree-bindings/fpga/altera-socfpga-a10-fpga-mgr.txt | 11
+++++++++++

 1 file changed, 11 insertions(+)

(quoted diff) +0 -0

fpga-
mgr.txt b/doc/device-tree-bindings/fpga/altera-socfpga-a10- fpga-
mgr.txt
index 2fd8e7a..7abb746 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,14 @@ 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.
+- bitstream_periph : FPGA peripheral raw binary file which is used
to
+                     initialize FPGA IOs, PLL, IO48 and DDR. +- bitstream_core : FPGA core raw binary file contains FPGA design
which is used
+                   to program FPGA CRAM and ERAM. +- bitstream_devpart : Partition of flash device where bitstream
files are

(quoted diff) +2 -0

+		       stored.
+                      <dev[:part]> - dev is flash device

number,
part is flash
+                                     device partition.
 Example:

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

@@ -16,4 +24,7 @@ Example:
  		       0xffcfe400 0x20>;
  		clocks = <&l4_mp_clk>;
  		resets = <&rst FPGAMGR_RESET>;
+		bitstream_periph =
 "ghrd_10as066n2.periph.rbf.mkimage";
+		bitstream_core =

"ghrd_10as066n2.core.rbf.mkimage";

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

+		bitstream_devpart = "0:1";

  };

Quoted 32 lines · Tien Fong

Quoted 30 lines · Marek Vasut

Quoted 17 lines · Tien Fong

Quoted 14 lines · Marek Vasut

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

From: Tien Fong Chee <[email protected]>

This patch adds description on properties about location of FPGA
RBFs are
stored, type and functionality of RBF used to configure FPGA.

Quoted 7 lines

Quoted 6 lines

Quoted 1 line

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

Why does this patch have different tags than 1/19 ? Please keep
things
consistent ...

Not get you. What's you means for tags?

ARM: socfpga: , not the random doc: dtbinding: . Heck, the first and second patch change the same file, yet have different tags, why ?

Quoted 6 lines

I ported patch 01 from Linux, so i keep everything intact. For patch
02, i put doc:dtbinding because i think that is more descriptive to the
file i changed.
I can change to ARM:socfpga .

The tags are standardized ... inventing new random ones only messes things up.

Quoted 68 lines

Quoted 68 lines

Quoted 68 lines

Quoted 68 lines

Quoted 68 lines

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

Quoted 2 lines

11

+++++++++++

 1 file changed, 11 insertions(+)

(quoted diff) +0 -0

a10-
fpga-
mgr.txt b/doc/device-tree-bindings/fpga/altera-socfpga-a10- fpga-
mgr.txt
index 2fd8e7a..7abb746 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,14 @@ 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.
+- bitstream_periph : FPGA peripheral raw binary file which is
used
to
+                     initialize FPGA IOs, PLL, IO48 and DDR.
+- bitstream_core : FPGA core raw binary file contains FPGA design
which is used
+                   to program FPGA CRAM and ERAM. +- bitstream_devpart : Partition of flash device where bitstream
files are

(quoted diff) +2 -0

+		       stored.
+                      <dev[:part]> - dev is flash device

number,
part is flash
+                                     device partition.
 Example:

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

@@ -16,4 +24,7 @@ Example:
  		       0xffcfe400 0x20>;
  		clocks = <&l4_mp_clk>;
  		resets = <&rst FPGAMGR_RESET>;
+		bitstream_periph =
 "ghrd_10as066n2.periph.rbf.mkimage";
+		bitstream_core =

"ghrd_10as066n2.core.rbf.mkimage";

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

+		bitstream_devpart = "0:1";

  };

From: Tien Fong Chee <[email protected]>

This patch adds description on properties about location of FPGA RBFs are stored, type and functionality of RBF used to configure FPGA.

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

---
 doc/device-tree-bindings/fpga/altera-socfpga-a10-fpga-mgr.txt | 11 +++++++++++
 1 file changed, 11 insertions(+)

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

index 2fd8e7a..7abb746 100644
@@ -7,6 +7,14 @@ 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.
+- bitstream_periph : FPGA peripheral raw binary file which is used to
+                     initialize FPGA IOs, PLL, IO48 and DDR.
+- bitstream_core : FPGA core raw binary file contains FPGA design which is used
+                   to program FPGA CRAM and ERAM.
+- bitstream_devpart : Partition of flash device where bitstream files are
+		       stored.
+                      <dev[:part]> - dev is flash device number, part is flash
+                                     device partition.
 
 Example:
 

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

 	};

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

From: Tien Fong Chee <[email protected]>

This patch adds description on properties about location of FPGA RBFs are
stored, type and functionality of RBF used to configure FPGA.

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

---
 doc/device-tree-bindings/fpga/altera-socfpga-a10-fpga-mgr.txt | 11
+++++++++++

 1 file changed, 11 insertions(+)

(quoted diff) +0 -0

mgr.txt b/doc/device-tree-bindings/fpga/altera-socfpga-a10-fpga- mgr.txt
index 2fd8e7a..7abb746 100644

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

@@ -7,6 +7,14 @@ 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.
+- bitstream_periph : FPGA peripheral raw binary file which is used to
+                     initialize FPGA IOs, PLL, IO48 and DDR. +- bitstream_core : FPGA core raw binary file contains FPGA design which is used
+                   to program FPGA CRAM and ERAM.
+- bitstream_devpart : Partition of flash device where bitstream files are

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

+		       stored.
+                      <dev[:part]> - dev is flash device number,

part is flash
+                                     device partition.
 Example:

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

@@ -16,4 +24,7 @@ Example:
  		       0xffcfe400 0x20>;
  		clocks = <&l4_mp_clk>;

doc/device-tree-bindings/fpga/altera-socfpga-a10-fpga-mgr.txt Example: · line 29

Quoted 6 lines

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

  		resets = <&rst FPGAMGR_RESET>;
+		bitstream_periph =
 "ghrd_10as066n2.periph.rbf.mkimage";
+		bitstream_core =

"ghrd_10as066n2.core.rbf.mkimage";

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

+		bitstream_devpart = "0:1";

These should probably be altr,something-something ... they are definitely not generic bindings

Quoted 3 lines

  };

From: Tien Fong Chee <[email protected]>

Add code necessary into the FPGA driver framework in U-Boot so it can be used via the 'fpga' command for programing Arria 10 SoCFPGA.

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

---

drivers/fpga/altera.c +4 -0

+#if defined(CONFIG_CMD_FPGA_LOADFS)
+int altera_loadfs(Altera_desc *desc, const void *buf, size_t bsize,
+		   fpga_fs_info *fpga_fsinfo)
+{

drivers/fpga/altera.c +2 -0

+}
+#endif

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

From: Tien Fong Chee <[email protected]>

Add code necessary into the FPGA driver framework in U-Boot so it can be used via the 'fpga' command for programing Arria 10 SoCFPGA.

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

---

[...]

Quoted 12 lines

(quoted diff) +2 -0

+#if defined(CONFIG_CMD_FPGA_LOADFS)
+int altera_loadfs(Altera_desc *desc, const void *buf, size_t

bsize,

drivers/fpga/altera.c +3 -0

+		   fpga_fs_info *fpga_fsinfo)
+{
+	const struct altera_fpga *fpga = altera_desc_to_fpga(desc,

__func__);

drivers/fpga/altera.c +4 -0

+
+	if (!fpga)
+		return FPGA_FAIL;
+

drivers/fpga/altera.c int altera_info(Altera_desc *desc) · line 197

Quoted 7 lines

(quoted diff) +1 -0

+	debug_cond(FPGA_DEBUG, "%s: Launching the %s FS

Loader...\n",

drivers/fpga/altera.c +3 -0

+		   __func__, fpga->name);
+	if (fpga->loadfs)
+		return fpga->loadfs(desc, buf, bsize,

fpga_fsinfo);

drivers/fpga/altera.c +1 -0

+	return 0;

return -EINVAL , since the loader could not be started ?

Quoted 3 lines

drivers/fpga/altera.c +2 -0

+}
+#endif

[...]

From: Tien Fong Chee <[email protected]>

Existing FPGA program write is always assume RBF data >= 32 bytes, so any rbf data less than 32 bytes writing to FPGA would be failed. This patch enhances the FPGA program write to support rbf data with size >= 4 bytes.

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

---
 drivers/fpga/socfpga.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

drivers/fpga/socfpga.c +8 -6

index 28fa16b..6e14ebd 100644
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2012 Altera Corporation <www.altera.com>
+ * Copyright (C) 2012-2017 Altera Corporation <www.altera.com>
  * All rights reserved.
  *
  * SPDX-License-Identifier:	BSD-3-Clause

@@ -55,18 +55,20 @@ void fpgamgr_program_write(const void *rbf_data, size_t rbf_size)
 	uint32_t loops4 = DIV_ROUND_UP(rbf_size % 32, 4);
 
 	asm volatile(
+		"	cmp	%2,	#0\n"
+		"	beq	2f\n"
 		"1:	ldmia	%0!,	{r0-r7}\n"
 		"	stmia	%1!,	{r0-r7}\n"
 		"	sub	%1,	#32\n"
 		"	subs	%2,	#1\n"
 		"	bne	1b\n"
-		"	cmp	%3,	#0\n"
-		"	beq	3f\n"
-		"2:	ldr	%2,	[%0],	#4\n"
+		"2:	cmp	%3,	#0\n"
+		"	beq	4f\n"
+		"3:	ldr	%2,	[%0],	#4\n"
 		"	str	%2,	[%1]\n"
 		"	subs	%3,	#1\n"
-		"	bne	2b\n"
-		"3:	nop\n"
+		"	bne	3b\n"
+		"4:	nop\n"
 		: "+r"(src), "+r"(dst), "+r"(loops32), "+r"(loops4) :
 		: "r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7", "cc");
 }

From: Tien Fong Chee <[email protected]>

Add compatible strings for Intel Arria 10 SoCFPGA device.

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

---
 include/fdtdec.h | 2 ++
 lib/fdtdec.c     | 2 ++
 2 files changed, 4 insertions(+)

include/fdtdec.h +2 -0

index 4a0947c..095ff36 100644
@@ -157,6 +157,8 @@ enum fdt_compat_id {
 	COMPAT_ALTERA_SOCFPGA_F2SDR0,           /* SoCFPGA fpga2SDRAM0 bridge */
 	COMPAT_ALTERA_SOCFPGA_F2SDR1,           /* SoCFPGA fpga2SDRAM1 bridge */
 	COMPAT_ALTERA_SOCFPGA_F2SDR2,           /* SoCFPGA fpga2SDRAM2 bridge */
+	COMPAT_ALTERA_SOCFPGA_FPGA0,		/* SOCFPGA FPGA manager */
+	COMPAT_ALTERA_SOCFPGA_NOC,		/* SOCFPGA Arria 10 NOC */
 
 	COMPAT_COUNT,
 };

lib/fdtdec.c +2 -0

index 107a892..03ff75e 100644
@@ -70,6 +70,8 @@ static const char * const compat_names[COMPAT_COUNT] = {
 	COMPAT(ALTERA_SOCFPGA_F2SDR0, "altr,socfpga-fpga2sdram0-bridge"),
 	COMPAT(ALTERA_SOCFPGA_F2SDR1, "altr,socfpga-fpga2sdram1-bridge"),
 	COMPAT(ALTERA_SOCFPGA_F2SDR2, "altr,socfpga-fpga2sdram2-bridge"),
+	COMPAT(ALTERA_SOCFPGA_FPGA0, "altr,socfpga-a10-fpga-mgr"),
+	COMPAT(ALTERA_SOCFPGA_NOC, "altr,socfpga-a10-noc"),
 };
 
 const char *fdtdec_get_compatible(enum fdt_compat_id id)

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

From: Tien Fong Chee <[email protected]>

Add compatible strings for Intel Arria 10 SoCFPGA device.

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

Applied, thanks

Quoted 5 lines

---
 include/fdtdec.h | 2 ++
 lib/fdtdec.c     | 2 ++
 2 files changed, 4 insertions(+)

include/fdtdec.h enum fdt_compat_id { · line 161

Quoted 10 lines

include/fdtdec.h +2 -0

index 4a0947c..095ff36 100644
@@ -157,6 +157,8 @@ enum fdt_compat_id {
      COMPAT_ALTERA_SOCFPGA_F2SDR0,           /* SoCFPGA fpga2SDRAM0 bridge */
      COMPAT_ALTERA_SOCFPGA_F2SDR1,           /* SoCFPGA fpga2SDRAM1 bridge */
      COMPAT_ALTERA_SOCFPGA_F2SDR2,           /* SoCFPGA fpga2SDRAM2 bridge */
+     COMPAT_ALTERA_SOCFPGA_FPGA0,            /* SOCFPGA FPGA manager */
+     COMPAT_ALTERA_SOCFPGA_NOC,              /* SOCFPGA Arria 10 NOC */

Quoted 15 lines

include/fdtdec.h +0 -0

      COMPAT_COUNT,
 };

lib/fdtdec.c +2 -0

index 107a892..03ff75e 100644
@@ -70,6 +70,8 @@ static const char * const compat_names[COMPAT_COUNT] = {
      COMPAT(ALTERA_SOCFPGA_F2SDR0, "altr,socfpga-fpga2sdram0-bridge"),
      COMPAT(ALTERA_SOCFPGA_F2SDR1, "altr,socfpga-fpga2sdram1-bridge"),
      COMPAT(ALTERA_SOCFPGA_F2SDR2, "altr,socfpga-fpga2sdram2-bridge"),
+     COMPAT(ALTERA_SOCFPGA_FPGA0, "altr,socfpga-a10-fpga-mgr"),
+     COMPAT(ALTERA_SOCFPGA_NOC, "altr,socfpga-a10-noc"),
 };
 

lib/fdtdec.c static const char * const compat_names[COMPAT_COUNT] = { · line 77

Quoted 2 lines

lib/fdtdec.c +0 -0

 const char *fdtdec_get_compatible(enum fdt_compat_id id)

--
Best regards,
Marek Vasut

From: Tien Fong Chee <[email protected]>

Enable generic filesystem interface drivers(fs.c and fat/) build for SPL. This would allow generic filesystem being used in SPL.

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

---
 common/spl/Kconfig | 8 ++++++++
 doc/README.SPL     | 1 +
 fs/Makefile        | 1 +
 3 files changed, 10 insertions(+)

common/spl/Kconfig +8 -0

index 9446cd9..5f4bd92 100644
@@ -304,6 +304,14 @@ config SPL_ETH_SUPPORT
 	  is required since the network stack uses a number of environment
 	  variables. See also SPL_NET_SUPPORT.
 
+config SPL_FS_GENERIC
+	bool "Support Generic filesystems interface driver"
+	help
+	  Enable support for generic filesystems interface with SPL. This
+	  permits U-Boot (or Linux in Falcon mode) to be loaded from a generic
+	  filesystem from within SPL. Support for the underlying block
+	  device (e.g. MMC or USB) must be enabled separately.
+
 config SPL_EXT_SUPPORT
 	bool "Support EXT filesystems"
 	help

doc/README.SPL +1 -0

index 3ba313c..32628a4 100644
@@ -55,6 +55,7 @@ CONFIG_SPL_SPI_FLASH_SUPPORT (drivers/mtd/spi/libspi_flash.o)
 CONFIG_SPL_SPI_SUPPORT (drivers/spi/libspi.o)
 CONFIG_SPL_FAT_SUPPORT (fs/fat/libfat.o)
 CONFIG_SPL_EXT_SUPPORT
+CONFIG_SPL_FS_GENERIC (fs/fs.o fat/)
 CONFIG_SPL_LIBGENERIC_SUPPORT (lib/libgeneric.o)
 CONFIG_SPL_POWER_SUPPORT (drivers/power/libpower.o)
 CONFIG_SPL_NAND_SUPPORT (drivers/mtd/nand/libnand.o)

fs/Makefile +1 -0

index 5770f41..fdacac8 100644
@@ -7,6 +7,7 @@
 #
 
 ifdef CONFIG_SPL_BUILD
+obj-$(CONFIG_SPL_FS_GENERIC) += fs.o fat/
 obj-$(CONFIG_SPL_FAT_SUPPORT) += fat/
 obj-$(CONFIG_SPL_EXT_SUPPORT) += ext4/
 else

From: Tien Fong Chee <[email protected]>

Enable generic filesystem interface drivers(fs.c and fat/) build for SPL. This would allow generic filesystem being used in SPL.

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

---
 common/spl/Kconfig | 8 ++++++++
 doc/README.SPL     | 1 +
 fs/Makefile        | 1 +
 3 files changed, 10 insertions(+)

From: Tien Fong Chee <[email protected]>

These drivers handle FPGA program operation from flash loading RBF to memory and then to program FPGA.

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

---
 .../include/mach/fpga_manager_arria10.h            |  27 ++
 drivers/fpga/socfpga_arria10.c                     | 391 ++++++++++++++++++++-
 include/altera.h                                   |   6 +
 include/configs/socfpga_common.h                   |   4 +
 4 files changed, 425 insertions(+), 3 deletions(-)

drivers/fpga/socfpga_arria10.c +0 -2

@@ -112,13 +122,14 @@ static int wait_for_nconfig_pin_and_nstatus_pin(void)
 	unsigned long mask = ALT_FPGAMGR_IMGCFG_STAT_F2S_NCONFIG_PIN_SET_MSK |
 				ALT_FPGAMGR_IMGCFG_STAT_F2S_NSTATUS_PIN_SET_MSK;
 
-	/* Poll until f2s_nconfig_pin and f2s_nstatus_pin; loop until de-asserted,
-	 * timeout at 1000ms

drivers/fpga/socfpga_arria10.c +33 -0

 static int wait_for_f2s_nstatus_pin(unsigned long value)

@@ -469,6 +480,7 @@ int socfpga_load(Altera_desc *desc, const void *rbf_data, size_t rbf_size)
 
 	/* Initialize the FPGA Manager */
 	status = fpgamgr_program_init((u32 *)rbf_data, rbf_size);
+
 	if (status)
 		return status;
 

@@ -477,3 +489,376 @@ int socfpga_load(Altera_desc *desc, const void *rbf_data, size_t rbf_size)
 
 	return fpgamgr_program_finish();
 }
+
+#if defined(CONFIG_CMD_FPGA_LOADFS)
+const char *get_cff_filename(const void *fdt, int *len, u32 core)
+{
+	const char *cff_filename = NULL;
+	const char *cell;
+	int nodeoffset;
+	nodeoffset = fdtdec_next_compatible(fdt, 0,
+			 COMPAT_ALTERA_SOCFPGA_FPGA0);
+
+	if (nodeoffset >= 0) {
+		if (core)
+			cell = fdt_getprop(fdt,
+					nodeoffset,
+					"bitstream_core",
+					len);
+		else
+			cell = fdt_getprop(fdt, nodeoffset, "bitstream_periph",
+					 len);
+
+		if (cell)
+			cff_filename = cell;
+	}
+
+	return cff_filename;
+}
+
+const char *get_cff_devpart(const void *fdt, int *len)
+{
+	const char *cff_devpart = NULL;
+	const char *cell;
+	int nodeoffset;

include/configs/socfpga_common.h +0 -0

 #define CONFIG_SYS_MEMTEST_START	PHYS_SDRAM_1
 #define CONFIG_SYS_MEMTEST_END		PHYS_SDRAM_1_SIZE
 #if defined(CONFIG_TARGET_SOCFPGA_GEN5)

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

From: Tien Fong Chee <[email protected]>

These drivers handle FPGA program operation from flash loading RBF to memory and then to program FPGA.

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

Did you run checkpatch on this before submitting ? I presume no ...

Quoted 8 lines

---
 .../include/mach/fpga_manager_arria10.h            |  27 ++ drivers/fpga/socfpga_arria10.c                     | 391
++++++++++++++++++++-
 include/altera.h                                   |   6 + include/configs/socfpga_common.h                   |   4 +

 4 files changed, 425 insertions(+), 3 deletions(-)

[...]

Quoted 15 lines

(quoted diff) +0 -0

@@ -112,13 +122,14 @@ static int
 wait_for_nconfig_pin_and_nstatus_pin(void)

  unsigned long mask = ALT_FPGAMGR_IMGCFG_STAT_F2S_NCONFIG_PIN_SET_MSK | ALT_FPGAMGR_IMGCFG_STAT_F2S_NSTATU S_PIN_SET_MSK;

(quoted diff) +0 -1

-	/* Poll until f2s_nconfig_pin and f2s_nstatus_pin; loop

until de-asserted,

drivers/fpga/socfpga_arria10.c +2 -1

-	 * timeout at 1000ms
+	/*
+	 * Poll until f2s_nconfig_pin and f2s_nstatus_pin; loop

until

drivers/fpga/socfpga_arria10.c +1 -0

+	 * de-asserted, timeout at 1000ms

Quoted 7 lines

   */
  return wait_for_bit(__func__, &fpga_manager_base->imgcfg_stat, mask,

drivers/fpga/socfpga_arria10.c +1 -1

-			    false, FPGA_TIMEOUT_MSEC, false);
+			    true, FPGA_TIMEOUT_MSEC, false);

 }

Seems more like a fix, split this out.

Quoted 54 lines

 static int wait_for_f2s_nstatus_pin(unsigned long value)

(quoted diff) +0 -0

@@ -469,6 +480,7 @@ int socfpga_load(Altera_desc *desc, const void
 *rbf_data, size_t rbf_size)

  /* Initialize the FPGA Manager */
  status = fpgamgr_program_init((u32 *)rbf_data, rbf_size); +
  if (status)
  return status;

(quoted diff) +0 -0

@@ -477,3 +489,376 @@ int socfpga_load(Altera_desc *desc, const
 void *rbf_data, size_t rbf_size)

  return fpgamgr_program_finish();
 }

drivers/fpga/socfpga_arria10.c +18 -0

+
+#if defined(CONFIG_CMD_FPGA_LOADFS)
+const char *get_cff_filename(const void *fdt, int *len, u32 core)
+{
+	const char *cff_filename = NULL;
+	const char *cell;
+	int nodeoffset;
+	nodeoffset = fdtdec_next_compatible(fdt, 0,
+			 COMPAT_ALTERA_SOCFPGA_FPGA0);
+
+	if (nodeoffset >= 0) {
+		if (core)
+			cell = fdt_getprop(fdt,
+					nodeoffset,
+					"bitstream_core",
+					len);
+		else
+			cell = fdt_getprop(fdt, nodeoffset,

"bitstream_periph",

drivers/fpga/socfpga_arria10.c +18 -0

+					 len);
+
+		if (cell)
+			cff_filename = cell;
+	}
+
+	return cff_filename;
+}
+
+const char *get_cff_devpart(const void *fdt, int *len)
+{
+	const char *cff_devpart = NULL;
+	const char *cell;
+	int nodeoffset;
+	nodeoffset = fdtdec_next_compatible(fdt, 0,
+			 COMPAT_ALTERA_SOCFPGA_FPGA0);
+
+	cell = fdt_getprop(fdt, nodeoffset, "bitstream_devpart",

len);

drivers/fpga/socfpga_arria10.c int socfpga_load(Altera_desc *desc, const void *rbf_data, size_t rbf_size) · line 533

Quoted 6 lines

drivers/fpga/socfpga_arria10.c +6 -0

+
+	if (cell)
+		cff_devpart = cell;
+
+	return cff_devpart;
+}

Take a look at splash*.c , I believe that can be reworked into generic
firmware loader , which you could then use here.

[...]

Quoted 1 line

Quoted 6 lines

include/configs/socfpga_common.h +0 -0

index 9897e11..eadce2d 100644

include/configs/socfpga_common.h line 34

Quoted 8 lines

include/configs/socfpga_common.h +4 -0

@@ -27,7 +27,11 @@
   */
  #define CONFIG_NR_DRAM_BANKS		1
  #define PHYS_SDRAM_1			0x0
+#if defined(CONFIG_TARGET_SOCFPGA_GEN5)
  #define CONFIG_SYS_MALLOC_LEN		(64 * 1024 * 1024)
+#elif defined(CONFIG_TARGET_SOCFPGA_ARRIA10)
+#define CONFIG_SYS_MALLOC_LEN		(128 * 1024 * 1024)
+#endif

You definitely don't need 128 MiB of malloc area.

Quoted 5 lines

include/configs/socfpga_common.h +0 -0

  #define CONFIG_SYS_MEMTEST_START	PHYS_SDRAM_1
  #define CONFIG_SYS_MEMTEST_END		PHYS_SDRAM_1_SIZE
  #if defined(CONFIG_TARGET_SOCFPGA_GEN5)

Quoted 4 lines · Marek Vasut

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

From: Tien Fong Chee <[email protected]>

Quoted 6 lines

Quoted 4 lines

These drivers handle FPGA program operation from flash loading RBF to memory and then to program FPGA.

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

Did you run checkpatch on this before submitting ? I presume no ...

Yeah, i run checkpatch for all patches. What's the issue here?

Quoted 25 lines

Quoted 8 lines

---
 .../include/mach/fpga_manager_arria10.h            |  27 ++ drivers/fpga/socfpga_arria10.c                     | 391
++++++++++++++++++++-
 include/altera.h                                   |   6 + include/configs/socfpga_common.h                   |   4 +

 4 files changed, 425 insertions(+), 3 deletions(-)

[...]

Quoted 15 lines

(quoted diff) +0 -0

@@ -112,13 +122,14 @@ static int
 wait_for_nconfig_pin_and_nstatus_pin(void)

  unsigned long mask = ALT_FPGAMGR_IMGCFG_STAT_F2S_NCONFIG_PIN_SET_MSK | ALT_FPGAMGR_IMGCFG_STAT_F2S_NSTATU S_PIN_SET_MSK;

(quoted diff) +0 -1

-	/* Poll until f2s_nconfig_pin and f2s_nstatus_pin; loop

until de-asserted,

drivers/fpga/socfpga_arria10.c +2 -1

-	 * timeout at 1000ms
+	/*
+	 * Poll until f2s_nconfig_pin and f2s_nstatus_pin; loop

until

drivers/fpga/socfpga_arria10.c +1 -0

+	 * de-asserted, timeout at 1000ms

Quoted 9 lines

Quoted 7 lines

   */
  return wait_for_bit(__func__, &fpga_manager_base->imgcfg_stat, mask,

drivers/fpga/socfpga_arria10.c +1 -1

-			    false, FPGA_TIMEOUT_MSEC, false);
+			    true, FPGA_TIMEOUT_MSEC, false);

 }

Seems more like a fix, split this out.

Okay.

Quoted 63 lines

Quoted 54 lines

 static int wait_for_f2s_nstatus_pin(unsigned long value)

(quoted diff) +0 -0

@@ -469,6 +480,7 @@ int socfpga_load(Altera_desc *desc, const void
 *rbf_data, size_t rbf_size)

  /* Initialize the FPGA Manager */
  status = fpgamgr_program_init((u32 *)rbf_data, rbf_size); +
  if (status)
  return status;

(quoted diff) +0 -0

@@ -477,3 +489,376 @@ int socfpga_load(Altera_desc *desc, const
 void *rbf_data, size_t rbf_size)

  return fpgamgr_program_finish();
 }

drivers/fpga/socfpga_arria10.c +18 -0

+
+#if defined(CONFIG_CMD_FPGA_LOADFS)
+const char *get_cff_filename(const void *fdt, int *len, u32 core)
+{
+	const char *cff_filename = NULL;
+	const char *cell;
+	int nodeoffset;
+	nodeoffset = fdtdec_next_compatible(fdt, 0,
+			 COMPAT_ALTERA_SOCFPGA_FPGA0);
+
+	if (nodeoffset >= 0) {
+		if (core)
+			cell = fdt_getprop(fdt,
+					nodeoffset,
+					"bitstream_core",
+					len);
+		else
+			cell = fdt_getprop(fdt, nodeoffset,

"bitstream_periph",

drivers/fpga/socfpga_arria10.c +18 -0

+					 len);
+
+		if (cell)
+			cff_filename = cell;
+	}
+
+	return cff_filename;
+}
+
+const char *get_cff_devpart(const void *fdt, int *len)
+{
+	const char *cff_devpart = NULL;
+	const char *cell;
+	int nodeoffset;
+	nodeoffset = fdtdec_next_compatible(fdt, 0,
+			 COMPAT_ALTERA_SOCFPGA_FPGA0);
+
+	cell = fdt_getprop(fdt, nodeoffset, "bitstream_devpart",

len);

drivers/fpga/socfpga_arria10.c int socfpga_load(Altera_desc *desc, const void *rbf_data, size_t rbf_size) · line 533

Quoted 6 lines

drivers/fpga/socfpga_arria10.c +6 -0

+
+	if (cell)
+		cff_devpart = cell;
+
+	return cff_devpart;
+}

Take a look at splash*.c , I believe that can be reworked into generic
firmware loader , which you could then use here.

Quoted 19 lines

[...]

Quoted 7 lines

include/configs/socfpga_common.h +0 -0

index 9897e11..eadce2d 100644

include/configs/socfpga_common.h line 34

Quoted 8 lines

include/configs/socfpga_common.h +4 -0

@@ -27,7 +27,11 @@
   */
  #define CONFIG_NR_DRAM_BANKS		1
  #define PHYS_SDRAM_1			0x0
+#if defined(CONFIG_TARGET_SOCFPGA_GEN5)
  #define CONFIG_SYS_MALLOC_LEN		(64 * 1024 * 1024)
+#elif defined(CONFIG_TARGET_SOCFPGA_ARRIA10)
+#define CONFIG_SYS_MALLOC_LEN		(128 * 1024 * 1024)
+#endif

You definitely don't need 128 MiB of malloc area.

Okay, i will try out with smaller size.

Quoted 5 lines

Quoted 5 lines

include/configs/socfpga_common.h +0 -0

  #define CONFIG_SYS_MEMTEST_START	PHYS_SDRAM_1
  #define CONFIG_SYS_MEMTEST_END		PHYS_SDRAM_1_SIZE
  #if defined(CONFIG_TARGET_SOCFPGA_GEN5)

Quoted 11 lines · Tien Fong

Quoted 9 lines · Marek Vasut

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

From: Tien Fong Chee <[email protected]>

These drivers handle FPGA program operation from flash loading RBF to memory and then to program FPGA.

Quoted 5 lines

Quoted 4 lines

Quoted 1 line

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

Did you run checkpatch on this before submitting ? I presume no
...

Yeah, i run checkpatch for all patches. What's the issue here?

It should definitely indicate problem with ie. yoda-notation +if (0 == flashinfo->remaining) {
and indent ...

Quoted 41 lines

Quoted 32 lines

Quoted 11 lines

---
 .../include/mach/fpga_manager_arria10.h            |  27 ++ drivers/fpga/socfpga_arria10.c                     | 391
++++++++++++++++++++-
 include/altera.h                                   |   6 + include/configs/socfpga_common.h                   |   4 +

 4 files changed, 425 insertions(+), 3 deletions(-)

[...]

Quoted 19 lines

(quoted diff) +0 -0

@@ -112,13 +122,14 @@ static int
 wait_for_nconfig_pin_and_nstatus_pin(void)

  unsigned long mask = ALT_FPGAMGR_IMGCFG_STAT_F2S_NCONFIG_PIN_SET_MSK | ALT_FPGAMGR_IMGCFG_STAT_F2S_NS TATU
S_PIN_SET_MSK;

(quoted diff) +0 -1

-	/* Poll until f2s_nconfig_pin and f2s_nstatus_pin;

loop
until de-asserted,

drivers/fpga/socfpga_arria10.c +2 -1

-	 * timeout at 1000ms
+	/*
+	 * Poll until f2s_nconfig_pin and f2s_nstatus_pin;

loop
until

drivers/fpga/socfpga_arria10.c +1 -0

+	 * de-asserted, timeout at 1000ms

Quoted 9 lines

Quoted 7 lines

   */
  return wait_for_bit(__func__, &fpga_manager_base->imgcfg_stat, mask,

drivers/fpga/socfpga_arria10.c +1 -1

-			    false, FPGA_TIMEOUT_MSEC, false);
+			    true, FPGA_TIMEOUT_MSEC, false);

 }

Seems more like a fix, split this out.

Quoted 70 lines

Okay.

Quoted 69 lines

Quoted 60 lines

 static int wait_for_f2s_nstatus_pin(unsigned long value)

(quoted diff) +0 -0

@@ -469,6 +480,7 @@ int socfpga_load(Altera_desc *desc, const

void
*rbf_data, size_t rbf_size)

  /* Initialize the FPGA Manager */ status = fpgamgr_program_init((u32 *)rbf_data, rbf_size);
+
  if (status)
  return status;

(quoted diff) +0 -0

@@ -477,3 +489,376 @@ int socfpga_load(Altera_desc *desc, const
 void *rbf_data, size_t rbf_size)

  return fpgamgr_program_finish();
 }

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

+
+#if defined(CONFIG_CMD_FPGA_LOADFS)
+const char *get_cff_filename(const void *fdt, int *len, u32

core)

drivers/fpga/socfpga_arria10.c +15 -0

+{
+	const char *cff_filename = NULL;
+	const char *cell;
+	int nodeoffset;
+	nodeoffset = fdtdec_next_compatible(fdt, 0,
+			 COMPAT_ALTERA_SOCFPGA_FPGA0);
+
+	if (nodeoffset >= 0) {
+		if (core)
+			cell = fdt_getprop(fdt,
+					nodeoffset,
+					"bitstream_core",
+					len);
+		else
+			cell = fdt_getprop(fdt, nodeoffset,

"bitstream_periph",

drivers/fpga/socfpga_arria10.c +18 -0

+					 len);
+
+		if (cell)
+			cff_filename = cell;
+	}
+
+	return cff_filename;
+}
+
+const char *get_cff_devpart(const void *fdt, int *len)
+{
+	const char *cff_devpart = NULL;
+	const char *cell;
+	int nodeoffset;
+	nodeoffset = fdtdec_next_compatible(fdt, 0,
+			 COMPAT_ALTERA_SOCFPGA_FPGA0);
+
+	cell = fdt_getprop(fdt, nodeoffset,

"bitstream_devpart",
len);

drivers/fpga/socfpga_arria10.c int socfpga_load(Altera_desc *desc, const void *rbf_data, size_t rbf_size) · line 533

Quoted 6 lines

drivers/fpga/socfpga_arria10.c +6 -0

+
+	if (cell)
+		cff_devpart = cell;
+
+	return cff_devpart;
+}

Take a look at splash*.c , I believe that can be reworked into generic
firmware loader , which you could then use here.

This is important here, I don't want yet another ad-hoc loader ...

Quoted 24 lines

Quoted 24 lines

[...]

Quoted 8 lines

include/configs/socfpga_common.h +0 -0

index 9897e11..eadce2d 100644

include/configs/socfpga_common.h line 36

Quoted 10 lines

include/configs/socfpga_common.h +4 -0

@@ -27,7 +27,11 @@
   */
  #define CONFIG_NR_DRAM_BANKS		1
  #define PHYS_SDRAM_1			0x0
+#if defined(CONFIG_TARGET_SOCFPGA_GEN5)
  #define CONFIG_SYS_MALLOC_LEN		(64 * 1024 *
 1024)
+#elif defined(CONFIG_TARGET_SOCFPGA_ARRIA10)
+#define CONFIG_SYS_MALLOC_LEN		(128 * 1024 *
 1024)
+#endif

You definitely don't need 128 MiB of malloc area.

Quoted 1 line

Okay, i will try out with smaller size.

Why do you need such massive area ? It's not a matter of "try out", you
should know why this change was needed for your use-case.

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

include/configs/socfpga_common.h +0 -0

  #define CONFIG_SYS_MEMTEST_START	PHYS_SDRAM_1

 #define CONFIG_SYS_MEMTEST_END PHYS_SDRAM_1_SIZ
E
 #if defined(CONFIG_TARGET_SOCFPGA_GEN5)

Quoted 12 lines · Marek Vasut

Quoted 11 lines · Tien Fong

Quoted 9 lines · Marek Vasut

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

From: Tien Fong Chee <[email protected]>

These drivers handle FPGA program operation from flash loading RBF to memory and then to program FPGA.

Quoted 9 lines

Quoted 5 lines

Quoted 4 lines

Quoted 1 line

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

Did you run checkpatch on this before submitting ? I presume no
...

Yeah, i run checkpatch for all patches. What's the issue here?

It should definitely indicate problem with ie. yoda-notation +if (0 == flashinfo->remaining) {
and indent ...

No complaint from checkpath. I know someone saying bad for readbility, but yoda-notation at this simple implementation doesn't impact the readbility, and having benefit to leverage detection of compiler on missing "=". Overall, this can help to improve coding quality. I can remove it if this doesn't favored in U-boot.

Quoted 111 lines

Quoted 41 lines

Quoted 32 lines

Quoted 11 lines

---
 .../include/mach/fpga_manager_arria10.h            |  27 ++ drivers/fpga/socfpga_arria10.c                     | 391
++++++++++++++++++++-
 include/altera.h                                   |   6 + include/configs/socfpga_common.h                   |   4 +

 4 files changed, 425 insertions(+), 3 deletions(-)

[...]

Quoted 19 lines

(quoted diff) +0 -0

@@ -112,13 +122,14 @@ static int
 wait_for_nconfig_pin_and_nstatus_pin(void)

  unsigned long mask = ALT_FPGAMGR_IMGCFG_STAT_F2S_NCONFIG_PIN_SET_MSK | ALT_FPGAMGR_IMGCFG_STAT_F2S_NS TATU
S_PIN_SET_MSK;

(quoted diff) +0 -1

-	/* Poll until f2s_nconfig_pin and f2s_nstatus_pin;

loop
until de-asserted,

drivers/fpga/socfpga_arria10.c +2 -1

-	 * timeout at 1000ms
+	/*
+	 * Poll until f2s_nconfig_pin and f2s_nstatus_pin;

loop
until

drivers/fpga/socfpga_arria10.c +1 -0

+	 * de-asserted, timeout at 1000ms

Quoted 9 lines

Quoted 7 lines

   */
  return wait_for_bit(__func__, &fpga_manager_base->imgcfg_stat, mask,

drivers/fpga/socfpga_arria10.c +1 -1

-			    false, FPGA_TIMEOUT_MSEC, false);
+			    true, FPGA_TIMEOUT_MSEC, false);

 }

Seems more like a fix, split this out.

Quoted 70 lines

Okay.

Quoted 69 lines

Quoted 60 lines

 static int wait_for_f2s_nstatus_pin(unsigned long value)

(quoted diff) +0 -0

@@ -469,6 +480,7 @@ int socfpga_load(Altera_desc *desc, const

void
*rbf_data, size_t rbf_size)

  /* Initialize the FPGA Manager */ status = fpgamgr_program_init((u32 *)rbf_data, rbf_size);
+
  if (status)
  return status;

(quoted diff) +0 -0

@@ -477,3 +489,376 @@ int socfpga_load(Altera_desc *desc, const
 void *rbf_data, size_t rbf_size)

  return fpgamgr_program_finish();
 }

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

+
+#if defined(CONFIG_CMD_FPGA_LOADFS)
+const char *get_cff_filename(const void *fdt, int *len, u32

core)

drivers/fpga/socfpga_arria10.c +15 -0

+{
+	const char *cff_filename = NULL;
+	const char *cell;
+	int nodeoffset;
+	nodeoffset = fdtdec_next_compatible(fdt, 0,
+			 COMPAT_ALTERA_SOCFPGA_FPGA0);
+
+	if (nodeoffset >= 0) {
+		if (core)
+			cell = fdt_getprop(fdt,
+					nodeoffset,
+					"bitstream_core",
+					len);
+		else
+			cell = fdt_getprop(fdt, nodeoffset,

"bitstream_periph",

drivers/fpga/socfpga_arria10.c +18 -0

+					 len);
+
+		if (cell)
+			cff_filename = cell;
+	}
+
+	return cff_filename;
+}
+
+const char *get_cff_devpart(const void *fdt, int *len)
+{
+	const char *cff_devpart = NULL;
+	const char *cell;
+	int nodeoffset;
+	nodeoffset = fdtdec_next_compatible(fdt, 0,
+			 COMPAT_ALTERA_SOCFPGA_FPGA0);
+
+	cell = fdt_getprop(fdt, nodeoffset,

"bitstream_devpart",
len);

drivers/fpga/socfpga_arria10.c int socfpga_load(Altera_desc *desc, const void *rbf_data, size_t rbf_size) · line 533

Quoted 6 lines

drivers/fpga/socfpga_arria10.c +6 -0

+
+	if (cell)
+		cff_devpart = cell;
+
+	return cff_devpart;
+}

Take a look at splash*.c , I believe that can be reworked into generic
firmware loader , which you could then use here.

Quoted 2 lines

This is important here, I don't want yet another ad-hoc loader ...

Disucss in another email reply separately. I need time to think.

Quoted 24 lines

Quoted 24 lines

Quoted 24 lines

[...]

Quoted 8 lines

include/configs/socfpga_common.h +0 -0

index 9897e11..eadce2d 100644

include/configs/socfpga_common.h line 36

Quoted 10 lines

include/configs/socfpga_common.h +4 -0

@@ -27,7 +27,11 @@
   */
  #define CONFIG_NR_DRAM_BANKS		1
  #define PHYS_SDRAM_1			0x0
+#if defined(CONFIG_TARGET_SOCFPGA_GEN5)
  #define CONFIG_SYS_MALLOC_LEN		(64 * 1024 *
 1024)
+#elif defined(CONFIG_TARGET_SOCFPGA_ARRIA10)
+#define CONFIG_SYS_MALLOC_LEN		(128 * 1024 *
 1024)
+#endif

You definitely don't need 128 MiB of malloc area.

Quoted 5 lines

Quoted 1 line

Okay, i will try out with smaller size.

Why do you need such massive area ? It's not a matter of "try out", you
should know why this change was needed for your use-case.

I forgot what reason i put this value. I need to find out.

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

include/configs/socfpga_common.h +0 -0

  #define CONFIG_SYS_MEMTEST_START	PHYS_SDRAM_1

 #define CONFIG_SYS_MEMTEST_END PHYS_SDRAM_1_SIZ
E
 #if defined(CONFIG_TARGET_SOCFPGA_GEN5)

Quoted 29 lines · Tien Fong

Quoted 27 lines · Marek Vasut

Quoted 15 lines · Tien Fong

Quoted 12 lines · Marek Vasut

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

From: Tien Fong Chee <[email protected]>

These drivers handle FPGA program operation from flash loading
RBF to memory and then to program FPGA.

Quoted 6 lines

Quoted 5 lines

Quoted 1 line

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

Did you run checkpatch on this before submitting ? I presume no
...

Yeah, i run checkpatch for all patches. What's the issue here?

It should definitely indicate problem with ie. yoda-notation +if (0 == flashinfo->remaining) {
and indent ...

Quoted 7 lines

No complaint from checkpath. I know someone saying bad for readbility,
but yoda-notation at this simple implementation doesn't impact the readbility, and having benefit to leverage detection of compiler on missing "=". Overall, this can help to improve coding quality. I can
remove it if this doesn't favored in U-boot.

It is not welcome and modern gcc warns you about such things .

Quoted 162 lines

Quoted 129 lines

Quoted 52 lines

Quoted 43 lines

Quoted 17 lines

---
 .../include/mach/fpga_manager_arria10.h            |  27 ++
 drivers/fpga/socfpga_arria10.c                     | 391
++++++++++++++++++++-
 include/altera.h                                   |   6 + include/configs/socfpga_common.h                   |   4 +

 4 files changed, 425 insertions(+), 3 deletions(-)

[...]

Quoted 24 lines

(quoted diff) +0 -0

@@ -112,13 +122,14 @@ static int
 wait_for_nconfig_pin_and_nstatus_pin(void)

  unsigned long mask = ALT_FPGAMGR_IMGCFG_STAT_F2S_NCONFIG_PIN_SET_MSK | ALT_FPGAMGR_IMGCFG_STAT_F2 S_NS
TATU
S_PIN_SET_MSK;

(quoted diff) +0 -1

-	/* Poll until f2s_nconfig_pin and f2s_nstatus_pin;

loop
until de-asserted,

drivers/fpga/socfpga_arria10.c +2 -1

-	 * timeout at 1000ms
+	/*
+	 * Poll until f2s_nconfig_pin and f2s_nstatus_pin;

loop
until

drivers/fpga/socfpga_arria10.c +1 -0

+	 * de-asserted, timeout at 1000ms

   */
  return wait_for_bit(__func__,
      &fpga_manager_base-

Quoted 9 lines

Quoted 7 lines

Quoted 1 line

imgcfg_stat,

      mask,

(quoted diff) +0 -1

-			    false, FPGA_TIMEOUT_MSEC,

false);

(quoted diff) +1 -0

+			    true, FPGA_TIMEOUT_MSEC,

false);
 }

Seems more like a fix, split this out.

Quoted 77 lines

Okay.

Quoted 76 lines

Quoted 66 lines

 static int wait_for_f2s_nstatus_pin(unsigned long value)

(quoted diff) +0 -0

@@ -469,6 +480,7 @@ int socfpga_load(Altera_desc *desc,

const
void
*rbf_data, size_t rbf_size)

  /* Initialize the FPGA Manager */ status = fpgamgr_program_init((u32 *)rbf_data, rbf_size);
+
  if (status)
  return status;

(quoted diff) +0 -0

@@ -477,3 +489,376 @@ int socfpga_load(Altera_desc *desc,

const
void *rbf_data, size_t rbf_size)

  return fpgamgr_program_finish();
 }

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

+
+#if defined(CONFIG_CMD_FPGA_LOADFS)
+const char *get_cff_filename(const void *fdt, int *len,

u32
core)

drivers/fpga/socfpga_arria10.c +15 -0

+{
+	const char *cff_filename = NULL;
+	const char *cell;
+	int nodeoffset;
+	nodeoffset = fdtdec_next_compatible(fdt, 0,
+			 COMPAT_ALTERA_SOCFPGA_FPGA0);
+
+	if (nodeoffset >= 0) {
+		if (core)
+			cell = fdt_getprop(fdt,
+					nodeoffset,
+					"bitstream_core",
+					len);
+		else
+			cell = fdt_getprop(fdt,

nodeoffset,
"bitstream_periph",

drivers/fpga/socfpga_arria10.c +18 -0

+					 len);
+
+		if (cell)
+			cff_filename = cell;
+	}
+
+	return cff_filename;
+}
+
+const char *get_cff_devpart(const void *fdt, int *len)
+{
+	const char *cff_devpart = NULL;
+	const char *cell;
+	int nodeoffset;
+	nodeoffset = fdtdec_next_compatible(fdt, 0,
+			 COMPAT_ALTERA_SOCFPGA_FPGA0);
+
+	cell = fdt_getprop(fdt, nodeoffset,

"bitstream_devpart",
len);

drivers/fpga/socfpga_arria10.c int socfpga_load(Altera_desc *desc, const void *rbf_data, size_t rbf_size) · line 533

Quoted 6 lines

drivers/fpga/socfpga_arria10.c +6 -0

+
+	if (cell)
+		cff_devpart = cell;
+
+	return cff_devpart;
+}

Take a look at splash*.c , I believe that can be reworked into
generic
firmware loader , which you could then use here.

Quoted 3 lines

This is important here, I don't want yet another ad-hoc loader
...

Disucss in another email reply separately. I need time to think.

Quoted 29 lines

Quoted 29 lines

Quoted 19 lines

[...]

Quoted 12 lines

include/configs/socfpga_common.h +0 -0

index 9897e11..eadce2d 100644
@@ -27,7 +27,11 @@
   */
  #define CONFIG_NR_DRAM_BANKS		1
  #define PHYS_SDRAM_1			0x0

Quoted 9 lines

include/configs/socfpga_common.h line 36

Quoted 7 lines

include/configs/socfpga_common.h +4 -0

+#if defined(CONFIG_TARGET_SOCFPGA_GEN5)
  #define CONFIG_SYS_MALLOC_LEN		(64 * 1024 *
 1024)
+#elif defined(CONFIG_TARGET_SOCFPGA_ARRIA10)
+#define CONFIG_SYS_MALLOC_LEN		(128 * 1024 *
 1024)
+#endif

You definitely don't need 128 MiB of malloc area.

Okay, i will try out with smaller size.

Quoted 6 lines

Quoted 5 lines

Why do you need such massive area ? It's not a matter of "try out",
you
should know why this change was needed for your use-case.

I forgot what reason i put this value. I need to find out.

If this was properly documented in the commit message, you would not have to ...

Quoted 15 lines

Quoted 15 lines

Quoted 15 lines

Quoted 15 lines

Quoted 15 lines

include/configs/socfpga_common.h +0 -0

  #define CONFIG_SYS_MEMTEST_START	PHYS_SDRAM_1

 #define CONFIG_SYS_MEMTEST_END PHYS_SDRAM_1
_SIZ
E
 #if defined(CONFIG_TARGET_SOCFPGA_GEN5)

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

From: Tien Fong Chee <[email protected]>

These drivers handle FPGA program operation from flash loading RBF to memory and then to program FPGA.

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

Did you run checkpatch on this before submitting ? I presume no ...

Quoted 8 lines

---
 .../include/mach/fpga_manager_arria10.h            |  27 ++ drivers/fpga/socfpga_arria10.c                     | 391
++++++++++++++++++++-
 include/altera.h                                   |   6 + include/configs/socfpga_common.h                   |   4 +

 4 files changed, 425 insertions(+), 3 deletions(-)

[...]

Quoted 21 lines

(quoted diff) +0 -0

@@ -112,13 +122,14 @@ static int
 wait_for_nconfig_pin_and_nstatus_pin(void)

  unsigned long mask = ALT_FPGAMGR_IMGCFG_STAT_F2S_NCONFIG_PIN_SET_MSK | ALT_FPGAMGR_IMGCFG_STAT_F2S_NSTATU S_PIN_SET_MSK;

(quoted diff) +0 -1

-	/* Poll until f2s_nconfig_pin and f2s_nstatus_pin; loop

until de-asserted,

drivers/fpga/socfpga_arria10.c +2 -1

-	 * timeout at 1000ms
+	/*
+	 * Poll until f2s_nconfig_pin and f2s_nstatus_pin; loop

until

drivers/fpga/socfpga_arria10.c +1 -0

+	 * de-asserted, timeout at 1000ms

   */
  return wait_for_bit(__func__, &fpga_manager_base->imgcfg_stat, mask,

drivers/fpga/socfpga_arria10.c +1 -1

-			    false, FPGA_TIMEOUT_MSEC, false);
+			    true, FPGA_TIMEOUT_MSEC, false);

Quoted 1 line

 }

Seems more like a fix, split this out.

Quoted 55 lines

 static int wait_for_f2s_nstatus_pin(unsigned long value)

(quoted diff) +0 -0

@@ -469,6 +480,7 @@ int socfpga_load(Altera_desc *desc, const void
 *rbf_data, size_t rbf_size)

  /* Initialize the FPGA Manager */
  status = fpgamgr_program_init((u32 *)rbf_data, rbf_size); +
  if (status)
  return status;

(quoted diff) +0 -0

@@ -477,3 +489,376 @@ int socfpga_load(Altera_desc *desc, const
 void *rbf_data, size_t rbf_size)

  return fpgamgr_program_finish();
 }

drivers/fpga/socfpga_arria10.c +18 -0

+
+#if defined(CONFIG_CMD_FPGA_LOADFS)
+const char *get_cff_filename(const void *fdt, int *len, u32 core)
+{
+	const char *cff_filename = NULL;
+	const char *cell;
+	int nodeoffset;
+	nodeoffset = fdtdec_next_compatible(fdt, 0,
+			 COMPAT_ALTERA_SOCFPGA_FPGA0);
+
+	if (nodeoffset >= 0) {
+		if (core)
+			cell = fdt_getprop(fdt,
+					nodeoffset,
+					"bitstream_core",
+					len);
+		else
+			cell = fdt_getprop(fdt, nodeoffset,

"bitstream_periph",

drivers/fpga/socfpga_arria10.c +18 -0

+					 len);
+
+		if (cell)
+			cff_filename = cell;
+	}
+
+	return cff_filename;
+}
+
+const char *get_cff_devpart(const void *fdt, int *len)
+{
+	const char *cff_devpart = NULL;
+	const char *cell;
+	int nodeoffset;
+	nodeoffset = fdtdec_next_compatible(fdt, 0,
+			 COMPAT_ALTERA_SOCFPGA_FPGA0);
+
+	cell = fdt_getprop(fdt, nodeoffset, "bitstream_devpart",

len);
+

drivers/fpga/socfpga_arria10.c int socfpga_load(Altera_desc *desc, const void *rbf_data, size_t rbf_size) · line 533

Quoted 5 lines

drivers/fpga/socfpga_arria10.c +5 -0

+	if (cell)
+		cff_devpart = cell;
+
+	return cff_devpart;
+}

Take a look at splash*.c , I believe that can be reworked into generic
firmware loader , which you could then use here.

[...]

Quoted 7 lines

include/configs/socfpga_common.h +0 -0

index 9897e11..eadce2d 100644

include/configs/socfpga_common.h line 34

Quoted 8 lines

include/configs/socfpga_common.h +4 -0

@@ -27,7 +27,11 @@
   */
  #define CONFIG_NR_DRAM_BANKS		1
  #define PHYS_SDRAM_1			0x0
+#if defined(CONFIG_TARGET_SOCFPGA_GEN5)
  #define CONFIG_SYS_MALLOC_LEN		(64 * 1024 * 1024)
+#elif defined(CONFIG_TARGET_SOCFPGA_ARRIA10)
+#define CONFIG_SYS_MALLOC_LEN		(128 * 1024 * 1024)
+#endif

You definitely don't need 128 MiB of malloc area.

Quoted 5 lines

include/configs/socfpga_common.h +0 -0

  #define CONFIG_SYS_MEMTEST_START	PHYS_SDRAM_1
  #define CONFIG_SYS_MEMTEST_END		PHYS_SDRAM_1_SIZE
  #if defined(CONFIG_TARGET_SOCFPGA_GEN5)

Quoted 8 lines · Marek Vasut

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

From: Tien Fong Chee <[email protected]>

These drivers handle FPGA program operation from flash loading RBF to memory and then to program FPGA.

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

Quoted 11 lines

Quoted 11 lines

drivers/fpga/socfpga_arria10.c +9 -0

+const char *get_cff_devpart(const void *fdt, int *len)
+{
+	const char *cff_devpart = NULL;
+	const char *cell;
+	int nodeoffset;
+	nodeoffset = fdtdec_next_compatible(fdt, 0,
+			 COMPAT_ALTERA_SOCFPGA_FPGA0);
+
+	cell = fdt_getprop(fdt, nodeoffset, "bitstream_devpart",

len);
+

Quoted 9 lines

drivers/fpga/socfpga_arria10.c int socfpga_load(Altera_desc *desc, const void *rbf_data, size_t rbf_size) · line 533

Quoted 5 lines

drivers/fpga/socfpga_arria10.c +5 -0

+	if (cell)
+		cff_devpart = cell;
+
+	return cff_devpart;
+}

Take a look at splash*.c , I believe that can be reworked into generic
firmware loader , which you could then use here.

the devpart is hard coded in splash*.c. The function here is getting devpart info from DTS. So, is there any similar function in splash*.c? May be you can share more about your idea.

Quoted 12 lines · Tien Fong

Quoted 10 lines · Marek Vasut

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

From: Tien Fong Chee <[email protected]>

These drivers handle FPGA program operation from flash loading RBF to memory and then to program FPGA.

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

[...]

Quoted 24 lines

Quoted 24 lines

Quoted 14 lines

drivers/fpga/socfpga_arria10.c +9 -0

+const char *get_cff_devpart(const void *fdt, int *len)
+{
+	const char *cff_devpart = NULL;
+	const char *cell;
+	int nodeoffset;
+	nodeoffset = fdtdec_next_compatible(fdt, 0,
+			 COMPAT_ALTERA_SOCFPGA_FPGA0);
+
+	cell = fdt_getprop(fdt, nodeoffset,

"bitstream_devpart",
len);

drivers/fpga/socfpga_arria10.c int socfpga_load(Altera_desc *desc, const void *rbf_data, size_t rbf_size) · line 533

Quoted 6 lines

drivers/fpga/socfpga_arria10.c +6 -0

+
+	if (cell)
+		cff_devpart = cell;
+
+	return cff_devpart;
+}

Take a look at splash*.c , I believe that can be reworked into generic
firmware loader , which you could then use here.

Quoted 5 lines

the devpart is hard coded in splash*.c. The function here is getting
devpart info from DTS. So, is there any similar function in splash*.c?
May be you can share more about your idea.

The generic loader could use some work of course ...

[...]

Quoted 44 lines · Marek Vasut

Quoted 12 lines · Tien Fong

Quoted 10 lines · Marek Vasut

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

From: Tien Fong Chee <[email protected]>

These drivers handle FPGA program operation from flash loading RBF to memory and then to program FPGA.

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

[...]

Quoted 29 lines

Quoted 15 lines

Quoted 15 lines

drivers/fpga/socfpga_arria10.c +9 -0

+const char *get_cff_devpart(const void *fdt, int *len)
+{
+	const char *cff_devpart = NULL;
+	const char *cell;
+	int nodeoffset;
+	nodeoffset = fdtdec_next_compatible(fdt, 0,
+			 COMPAT_ALTERA_SOCFPGA_FPGA0);
+
+	cell = fdt_getprop(fdt, nodeoffset,

"bitstream_devpart",
len);
+

Quoted 9 lines

drivers/fpga/socfpga_arria10.c int socfpga_load(Altera_desc *desc, const void *rbf_data, size_t rbf_size) · line 533

Quoted 5 lines

drivers/fpga/socfpga_arria10.c +5 -0

+	if (cell)
+		cff_devpart = cell;
+
+	return cff_devpart;
+}

Take a look at splash*.c , I believe that can be reworked into generic
firmware loader , which you could then use here.

the devpart is hard coded in splash*.c. The function here is getting
devpart info from DTS. So, is there any similar function in splash*.c?
May be you can share more about your idea.

Quoted 2 lines

The generic loader could use some work of course ...

Sorry, i am still confusing. Allow me to ask you more:
1. Is the generic firmware loader already exists in splash*.c?

2. Are you talking about get_cff_devpart or whole fpga laodfs?
3. You want me integrate get_cff_devpart function into splash*.c?
4. Are you means to hard code the devpart instead providing dynamic devpart described in DTS?

Current implementation are located in spl_board_init func (arcg/arm/mach-socfpga/spl.c). Based on boot device such as mmc, nand and QSPI, then reading some info from DTS, setting dev and partition with generic fs functions, and reading with generic fs function before programming RBF into FPGA. All these are in patch 19.

Thanks.

Quoted 1 line

[...]

Quoted 55 lines · Tien Fong

Quoted 53 lines · Marek Vasut

Quoted 16 lines · Tien Fong

Quoted 13 lines · Marek Vasut

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

From: Tien Fong Chee <[email protected]>

These drivers handle FPGA program operation from flash loading
RBF to memory and then to program FPGA.

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

[...]

Quoted 33 lines

Quoted 19 lines

Quoted 19 lines

drivers/fpga/socfpga_arria10.c +9 -0

+const char *get_cff_devpart(const void *fdt, int *len)
+{
+	const char *cff_devpart = NULL;
+	const char *cell;
+	int nodeoffset;
+	nodeoffset = fdtdec_next_compatible(fdt, 0,
+			 COMPAT_ALTERA_SOCFPGA_FPGA0);
+
+	cell = fdt_getprop(fdt, nodeoffset,

"bitstream_devpart",
len);

(quoted diff) +2 -0

+
+	if (cell)

Quoted 9 lines

drivers/fpga/socfpga_arria10.c int socfpga_load(Altera_desc *desc, const void *rbf_data, size_t rbf_size) · line 533

Quoted 4 lines

drivers/fpga/socfpga_arria10.c +4 -0

+		cff_devpart = cell;
+
+	return cff_devpart;
+}

Take a look at splash*.c , I believe that can be reworked into
generic
firmware loader , which you could then use here.

the devpart is hard coded in splash*.c. The function here is getting
devpart info from DTS. So, is there any similar function in splash*.c?
May be you can share more about your idea.

Quoted 4 lines

Quoted 2 lines

The generic loader could use some work of course ...

Sorry, i am still confusing. Allow me to ask you more:
1. Is the generic firmware loader already exists in splash*.c?

No

Quoted 5 lines

2. Are you talking about get_cff_devpart or whole fpga laodfs?
3. You want me integrate get_cff_devpart function into splash*.c?
4. Are you means to hard code the devpart instead providing dynamic devpart described in DTS?

I am talking about factoring out generic firmware loader from splash*c ,
since it already contains most of the parts for such a thing.

Quoted 9 lines

Current implementation are located in spl_board_init func (arcg/arm/mach-socfpga/spl.c). Based on boot device such as mmc, nand
and QSPI, then reading some info from DTS, setting dev and partition
with generic fs functions, and reading with generic fs function before
programming RBF into FPGA. All these are in patch 19.

That's what splash*c also does, so adding separate parallel implementation of the same functionality is a no-no.

fdt_blob,

fdt_blob,

Quoted 4 lines

Thanks.

Quoted 2 lines

[...]

Quoted 80 lines · Marek Vasut

Quoted 55 lines · Tien Fong

Quoted 53 lines · Marek Vasut

Quoted 16 lines · Tien Fong

Quoted 13 lines · Marek Vasut

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

From: Tien Fong Chee <[email protected]>

These drivers handle FPGA program operation from flash loading
RBF to memory and then to program FPGA.

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

[...]

Quoted 33 lines

Quoted 19 lines

Quoted 19 lines

(quoted diff) +9 -0

+const char *get_cff_devpart(const void *fdt, int *len)
+{
+	const char *cff_devpart = NULL;
+	const char *cell;
+	int nodeoffset;
+	nodeoffset = fdtdec_next_compatible(fdt, 0,
+			 COMPAT_ALTERA_SOCFPGA_FPGA0);
+
+	cell = fdt_getprop(fdt, nodeoffset,

"bitstream_devpart",
len);

(quoted diff) +2 -0

+
+	if (cell)

Quoted 9 lines

Review

Quoted 4 lines

(quoted diff) +4 -0

+		cff_devpart = cell;
+
+	return cff_devpart;
+}

Take a look at splash*.c , I believe that can be reworked into
generic
firmware loader , which you could then use here.

the devpart is hard coded in splash*.c. The function here is getting
devpart info from DTS. So, is there any similar function in splash*.c?
May be you can share more about your idea.

Quoted 4 lines

Quoted 2 lines

The generic loader could use some work of course ...

Sorry, i am still confusing. Allow me to ask you more:
1. Is the generic firmware loader already exists in splash*.c?

No

Quoted 5 lines

2. Are you talking about get_cff_devpart or whole fpga laodfs?
3. You want me integrate get_cff_devpart function into splash*.c?
4. Are you means to hard code the devpart instead providing dynamic devpart described in DTS?

I am talking about factoring out generic firmware loader from splash*c ,
since it already contains most of the parts for such a thing.

Quoted 9 lines

Current implementation are located in spl_board_init func (arcg/arm/mach-socfpga/spl.c). Based on boot device such as mmc, nand
and QSPI, then reading some info from DTS, setting dev and partition
with generic fs functions, and reading with generic fs function before
programming RBF into FPGA. All these are in patch 19.

Quoted 3 lines

That's what splash*c also does, so adding separate parallel implementation of the same functionality is a no-no.

After reading through splash*c, i found there are two functions bear a close similarity to.
1st function -->
In /common/splash.c :

static struct splash_location default_splash_locations[] = {
	{
		.name = "sf",
		.storage = SPLASH_STORAGE_SF,
		.flags = SPLASH_STORAGE_RAW,
		.offset = 0x0,
	},
	{
		.name = "mmc_fs",
		.storage = SPLASH_STORAGE_MMC,
		.flags = SPLASH_STORAGE_FS,
		.devpart = "0:1",
	},
	{
		.name = "usb_fs",
		.storage = SPLASH_STORAGE_USB,
		.flags = SPLASH_STORAGE_FS,
		.devpart = "0:1",
	},
	{
		.name = "sata_fs",
		.storage = SPLASH_STORAGE_SATA,
		.flags = SPLASH_STORAGE_FS,
		.devpart = "0:1",
	},
};

In my /arch/arm/mach-socfpga/spl.c (spl_board_init(void)) bootdev.boot_device = spl_boot_device();

	if (BOOT_DEVICE_MMC1 == bootdev.boot_device) {
		struct mmc *mmc = NULL;
		int err = 0;

		spl_mmc_find_device(&mmc, bootdev.boot_device);

		err = mmc_init(mmc);

		if (err) {
#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
			printf("spl: mmc init failed with error: %d\n",
err);
#endif
		}

		fpga_fsinfo.dev_part = (char *)get_cff_devpart(gd-

Quoted 1 line

fdt_blob,

								 &len);

		fpga_fsinfo.filename = (char *)get_cff_filename(gd-

Quoted 1 line

fdt_blob,

								 &len,
								PERIPH_

RBF);

		fpga_fsinfo.interface = "mmc";

		fpga_fsinfo.fstype = FS_TYPE_FAT;
	} else {
		printf("Invalid boot device!\n");
		return;
	}

	/* Program peripheral RBF */
	if (fpga_fsinfo.filename && fpga_fsinfo.dev_part && (len > 0))
		rval = fpga_fsload(0, buffer, BSIZE, &fpga_fsinfo);

In /common/splash.c, dev_Part and flash type everything are hard coded in struct splash_location. In my spl.c, flash type are determined on run time and dev_part are retrived from DTS, and then assigned to struct fpga_fsinfo. Please note that this is in SPL, mmc need to be initialized 1st before loading raw file into memory. In SPL, raw file are coppied to OCRAM chunk by chunk, but In u-boot it would normally done in one big chunk to DRAM. This would be handled by fpga loadfs.

So, you want me hard code everthing like in splash.c?

2nd function -->
In /common/splash_source.c
static int splash_select_fs_dev(struct splash_location *location)

{
	int res;

	switch (location->storage) {
	case SPLASH_STORAGE_MMC:
		res = fs_set_blk_dev("mmc", location->devpart,
FS_TYPE_ANY);
		break;
	case SPLASH_STORAGE_USB:
		res = fs_set_blk_dev("usb", location->devpart,
FS_TYPE_ANY);
		break;
	case SPLASH_STORAGE_SATA:
		res = fs_set_blk_dev("sata", location->devpart,
FS_TYPE_ANY);
		break;
	case SPLASH_STORAGE_NAND:
		if (location->ubivol != NULL)
			res = fs_set_blk_dev("ubi", NULL,
FS_TYPE_UBIFS);
		else
			res = -ENODEV;
		break;
	default:
		printf("Error: unsupported location storage.\n");
		return -ENODEV;
	}

	if (res)
		printf("Error: could not access storage.\n");

	return res;
}

In my /drivers/fpga/socfpga_arria10.c static int flash_read(struct flash_info *flashinfo,

	u32 size_read,
	u32 *buffer_ptr)
{
	size_t ret = EEXIST;
	loff_t actread = 0;

	if (fs_set_blk_dev(flashinfo->interface, flashinfo->dev_part,
				flashinfo->fstype))
		return FPGA_FAIL;

	ret = fs_read(flashinfo->filename,
			(u32) buffer_ptr, flashinfo->flash_offset,
			size_read, &actread);

	if (ret || actread != size_read) {
		printf("Failed to read %s from flash %d ",
			flashinfo->filename,
			 ret);
		printf("!= %d.\n", size_read);
		return -EPERM;
		} else
			ret = actread;

	return ret;
}

Some attributes like flash type is determined on run time and and dev_part is retrieved from DTS, so every infos driver need to know are assinged into struct flashinfo and passed to fs_set_blk_dev as arguments. I found that function in splash_source.c some like flash type are getting from env variable, but we are still in SPL phase, those env variable is not set up yet. So, i think that is very ineffcient to factor out them as common.

If you want me create a generic firmware loader which is generic enough loading content for all components like flashes, FPGA, splash ....etc, i don't think that is effient enough, as fpga loadfs has different handling in both SPL and U-boot like copying raw into memory.

It would be good you can direct point me out which functions have similirity and how to factor them out as common.

Thanks a lot.

Quoted 4 lines

Quoted 4 lines

Thanks.

Quoted 2 lines

[...]

From: Tien Fong Chee <[email protected]>

These drivers handle FPGA program operation from flash loading RBF to memory and then to program FPGA.

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

---
 .../include/mach/fpga_manager_arria10.h            |  27 ++
 drivers/fpga/socfpga_arria10.c                     | 391 ++++++++++++++++++++-
 include/altera.h                                   |   6 +
 include/configs/socfpga_common.h                   |   4 +
 4 files changed, 425 insertions(+), 3 deletions(-)

From: Tien Fong Chee <[email protected]>

Current sdram driver is only applied to gen5 device, hence it is better to rename sdram driver to more specific name which is related to gen5 device.

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

---
 arch/arm/mach-socfpga/include/mach/sdram.h         | 434 +--------------------
 .../include/mach/{sdram.h => sdram_gen5.h}         |   6 +-
 drivers/ddr/altera/Makefile                        |   2 +-
 drivers/ddr/altera/{sdram.c => sdram_gen5.c}       |   0
 4 files changed, 8 insertions(+), 434 deletions(-)
 copy arch/arm/mach-socfpga/include/mach/{sdram.h => sdram_gen5.h} (99%)
 rename drivers/ddr/altera/{sdram.c => sdram_gen5.c} (100%)

arch/arm/mach-socfpga/include/mach/sdram.h +0 -0

index b11228f..4a9754e 100644

arch/arm/mach-socfpga/include/mach/sdram.h +0 -9

-unsigned long sdram_calculate_size(void);
-int sdram_mmr_init_full(unsigned int sdr_phy_reg);
-int sdram_calibration_full(void);
-
-const struct socfpga_sdram_config *socfpga_get_sdram_config(void);
-
-void socfpga_get_seq_ac_init(const u32 **init, unsigned int *nelem);
-void socfpga_get_seq_inst_init(const u32 **init, unsigned int *nelem);
-const struct socfpga_sdram_rw_mgr_config *socfpga_get_sdram_rwmgr_config(void);

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

From: Tien Fong Chee <[email protected]>

Current sdram driver is only applied to gen5 device, hence it is better
to rename sdram driver to more specific name which is related to gen5
device.

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

---
 arch/arm/mach-socfpga/include/mach/sdram.h         | 434 +------
--------------
 .../include/mach/{sdram.h => sdram_gen5.h}         |   6 +- drivers/ddr/altera/Makefile                        |   2 +- drivers/ddr/altera/{sdram.c => sdram_gen5.c}       |   0

 4 files changed, 8 insertions(+), 434 deletions(-)

 copy arch/arm/mach-socfpga/include/mach/{sdram.h => sdram_gen5.h} (99%)
 rename drivers/ddr/altera/{sdram.c => sdram_gen5.c} (100%)

arch/arm/mach-socfpga/include/mach/sdram.h line 2

Quoted 9 lines

arch/arm/mach-socfpga/include/mach/sdram.h +1 -1

index b11228f..4a9754e 100644
@@ -1,5 +1,5 @@
  /*
- * Copyright Altera Corporation (C) 2014-2015
+ * Copyright (C) 2017 Intel Corporation <www.intel.com>

Retain the old copyright ?