arm: zynq: implement FPGA load from SPL

A message should be displayed if an image is loaded to an FPGA, because the hardware might have changed, and the user should be informed

Signed-off-by Luis Araneda <[email protected]>

---
 common/spl/spl_fit.c | 1 +
 1 file changed, 1 insertion(+)

common/spl/spl_fit.c +0 -0

index 5b51a28a08..9eabb1c105 100644

Disable the use of function zynq_loadfs when compiling the driver for the SPL, as the following filesystem functions are not found by the linker:
- fs_set_blk_dev
- fs_read
- fs_set_blk_dev
- fs_read
- fs_read

Signed-off-by Luis Araneda <[email protected]>

---
 drivers/fpga/zynqpl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/fpga/zynqpl.c +1 -1

index fd37d18c7f..8e49c7010e 100644
@@ -410,7 +410,7 @@ static int zynq_load(xilinx_desc *desc, const void *buf, size_t bsize,
 	return FPGA_SUCCESS;
 }
 
-#if defined(CONFIG_CMD_FPGA_LOADFS)
+#if defined(CONFIG_CMD_FPGA_LOADFS) && !defined(CONFIG_SPL_BUILD)
 static int zynq_loadfs(xilinx_desc *desc, const void *buf, size_t bsize,
 		       fpga_fs_info *fsinfo)
 {

commit 4aba5fb857c1 ("arm: zynq: Rework FPGA initialization") moved FPGA initialization from board_init() to arch_early_init_r(), which is not called as part of the SPL

Fix this by calling arch_early_init_r() in the spl_board_init() function, so the FPGA is correctly initialized

Signed-off-by Luis Araneda <[email protected]>

---
 arch/arm/mach-zynq/spl.c | 3 +++
 1 file changed, 3 insertions(+)

arch/arm/mach-zynq/spl.c +0 -0

index 83297d6c69..9b7c0be951 100644

Implement FPGA bitstream loading from SPL. The bitstream is loaded from a FIT image into dynamically allocated memory before programming the FPGA.

Signed-off-by Luis Araneda <[email protected]>

---
 arch/arm/mach-zynq/spl.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

arch/arm/mach-zynq/spl.c +29 -0

index 9b7c0be951..42907afa94 100644
@@ -4,6 +4,8 @@
  */
 #include <common.h>
 #include <debug_uart.h>
+#include <fpga.h>
+#include <memalign.h>
 #include <spl.h>
 
 #include <asm/io.h>

@@ -93,3 +95,40 @@ int board_fit_config_name_match(const char *name)
 	return 0;
 }
 #endif
+
+#ifdef CONFIG_SPL_FPGA_SUPPORT
+int spl_load_fpga_image(struct spl_load_info *info, size_t length,
+			int nr_sectors, int sector_offset)
+{
+	char *data_buf;
+	int ret;
+
+	data_buf = malloc_cache_aligned(nr_sectors);
+	if (!data_buf) {
+		debug("%s: Cannot reserve memory\n", __func__);
+		return -ENOMEM;
+	}
+
+	if (info->read(info, sector_offset, nr_sectors,
+		       data_buf) != nr_sectors) {
+		debug("%s: Cannot read the FPGA image\n", __func__);
+		free(data_buf);
+		return -EIO;
+	}
+
+	memmove(data_buf, data_buf + (nr_sectors - length), length);
+
+	debug("%s: FPGA image loaded to 0x%p (%zu bytes)\n",
+	      __func__, data_buf, length);
+
+	ret = fpga_load(0, data_buf, length, BIT_FULL);

I was playing with this a little bit. There is no reason to allocate any space in malloc area because its/fit should already contain load address which you should use instead.

Hi Michal,

Quoted 3 lines · Michal Simek

I was playing with this a little bit. There is no reason to allocate any space in malloc area because its/fit should already contain load address which you should use instead.

I initially thought the same, but unfortunately the load address is not a parameter passed to the function, nor can it be extracted from the spl_load_info structure.
Like you have probably discovered by now, the spl_load_fpga_image() function was introduced to support the use case of boards with uninitialized DRAM, so the load address was not necessary. On the other hand, the zynpl driver doesn't have functions to program the FPGA by chunks, so I had to allocate memory.

Also on zc706 without FULL_FIT my path in spl_load_fit_image is not jumping to "if (external_data) {" branch where spl_load_fpga_image is which is kind of interesting because it looks like you are going there.

./tools/mkimage -f <input_its_file> -E <output_file>

Hi Michal,

Quoted 3 lines · Michal Simek

Also on zc706 without FULL_FIT my path in spl_load_fit_image is not jumping to "if (external_data) {" branch where spl_load_fpga_image is which is kind of interesting because it looks like you are going there.

To enter the path "if (external_data) {" branch where spl_load_fpga_image is}" you have to create the FIT image with the "-E" option, I'm creating it with:

Quoted 1 line

./tools/mkimage -f <input_its_file> -E <output_file>

I started to use the option because my initial tests weren't entering the path either, and I found it when analyzing the (generated) .u-boot.img.cmd file.

I just tested your patch with and without the "-E" option, and it works on both cases :)

This series implements FPGA bitstream loading from SPL. Programming the FPGA from the SPL is necessary on some boards, like the Zybo, because the FPGA fabric routes the I2C bus to an EEPROM for reading the Ethernet MAC address.

The bitstream is loaded from a FIT image into a dynamically-allocated memory before programming the FPGA.

Additionally, some fixes are applied to compile the zynqpl driver for SPL, and to properly detect an FPGA from the SPL.

I'm not sure if the fixes are correct or I'm missing something, hence the RFC.

Tested on a Digilent Zybo Z7-20 board

Luis Araneda (4):
spl: fit: display a message when an FPGA image is loaded
drivers: fpga: zynqpl: fix compilation with SPL
arm: zynq: spl: fix FPGA initialization
arm: zynq: spl: implement FPGA load from FIT

 arch/arm/mach-zynq/spl.c | 42 ++++++++++++++++++++++++++++++++++++++++
 common/spl/spl_fit.c     |  1 +
 drivers/fpga/zynqpl.c    |  4 ++--
 3 files changed, 45 insertions(+), 2 deletions(-)

Can you please also send defconfig/config changes? Separate patch is fine.

Hi Michal,

Quoted 2 lines · Michal Simek

Can you please also send defconfig/config changes? Separate patch is fine.

The changes required to the defconfigs test/support this are:
CONFIG_SPL_LOAD_FIT=y
CONFIG_SPL_FPGA_SUPPORT=y

I didn't send them because just changing the defconfig isn't enough,

I
had to manually create a FIT image to replace the default u-boot.img, which is generated
by U-Boot's Makefile.
The .its file (to generate the FIT image) has the following FPGA node:

fpga-1 {
description = "Zybo Z7-20 FPGA image";
data = /incbin/("zybo-z7-20-preboot.bin");
type = "fpga";
arch = "arm";
compression = "none";
load = <0x30000000>;
};

I run-tested the changes on a Zybo Z7-20, and compile-tested on a Zybo.

I have an idea to automate the FIT generation. The build system could scan for the existence of a file, for example "board/xilinx/zynq/<board>/preboot.bin", and add the fpga node automatically (to .its) if the file exists. Because I think that storing .bin files in the U-Boot repository is infeasible. That will require additions and modifications to the current way the build system works, and I'm still thinking how to implement them.

By the way, sorry for changing the subject, but I realized that you use two e-mail accounts. Do you have a preferred one? I'm asking because I sent the Zybo-Z7 support series (and the I2C DM one) to the @monstr.eu account, which you hasn't responded yet, and this series to the @xilinx.com account, which you replied quickly. Maybe you are just waiting for other people to review/test it.

Quoted 2 lines · Luis Araneda

[...]
I didn't send them because just changing the defconfig isn't enough,

It should be enough. It is configuration option and just enabling that feature. You should still be able to use just u-boot.img in legacy or fit format without any issue.

I think it will be good if you can look at my patch and also compare boot up time when you setup compression to gzip. I expect some changes in connection to this code.

        if (IS_ENABLED(CONFIG_SPL_OS_BOOT)      &&
            IS_ENABLED(CONFIG_SPL_GZIP)         &&
            image_comp == IH_COMP_GZIP          &&
            type == IH_TYPE_KERNEL) {

And I would expect that copying smaller fit with unziping bitstream will be faster then what you have now. Especially on boards which bigger fpga.

gzip -c -n <comp_level> <bin_file> > <out_file>

Quoted 7 lines

I have an idea to automate the FIT generation. The build system could scan for the existence of a file, for example "board/xilinx/zynq/<board>/preboot.bin", and add the fpga node automatically (to .its) if the file exists. Because I think that storing .bin files in the U-Boot repository is infeasible. That will require additions and modifications to the current way the build system works, and I'm still thinking how to implement them.

Take a look at pmufw handling for zynqmp for inspiration.

Hi Michal,

Quoted 4 lines · Michal Simek

Quoted 2 lines · Luis Araneda

[...]
I didn't send them because just changing the defconfig isn't enough,

Quoted 3 lines

It should be enough. It is configuration option and just enabling that feature. You should still be able to use just u-boot.img in legacy or fit format without any issue.

Ok. Should I send a patch only for the Zybo or all zynq boards? Also, for than one board, should I create one patch per board or only one big patch for all of them?

Quoted 5 lines

I think it will be good if you can look at my patch and also compare boot up time when you setup compression to gzip. I expect some changes in connection to this code.

Quoted 7 lines

        if (IS_ENABLED(CONFIG_SPL_OS_BOOT)      &&
            IS_ENABLED(CONFIG_SPL_GZIP)         &&
            image_comp == IH_COMP_GZIP          &&
            type == IH_TYPE_KERNEL) {

And I would expect that copying smaller fit with unziping bitstream will be faster then what you have now. Especially on boards which bigger fpga.

I made some modifications to make gzip work, and another one dirty (non-upstremeable) to make external data work. Additionally, I added time reporting on three places. The modifications are attached.

I tested several bitstreams, with different compression levels:

Quoted 1 line

gzip -c -n <comp_level> <bin_file> > <out_file>

The results, for a fit image with embedded data, are:
file size (bytes) time1 time2 time3

uncompressed   2,434,112     567   597   623
compressed -1    446,028     208  1165  1190
compressed -4    407,764     205  1063  1088
compressed -5    398,346     203  1094  1119
compressed -9    376,821     200  1141  1166

The time for a fit image with external data (-E option for mkimage) is ~100 time units (ms?) less, and time1 remains constant at ~12 time units.

At least on my setup (Zybo Z7-20), gzip just increase the boot time.

Quoted 3 lines · Luis Araneda

Hi Michal,

On Thu, Jul 19, 2018 at 2:23 AM Michal Simek <[email protected]> wrote:

We need that functionality first but then enable it for all boards is fine for me and via one patch.

Can you please be more specific what time1/time2 and time3 means?

Hi Michal,

Quoted 6 lines · Michal Simek

Quoted 3 lines · Luis Araneda

Hi Michal,

On Thu, Jul 19, 2018 at 2:23 AM Michal Simek <[email protected]> wrote:

We need that functionality first but then enable it for all boards is fine for me and via one patch.

Ok

Quoted 1 line

Can you please be more specific what time1/time2 and time3 means?

The exact location of time 1/2/3 are on the attached diff file, and they are placed within the spl_load_simple_fit() function. They represent, roughly:
- time1: Time to load the the FIT image
- time2: Time to extract (and decompress)
the FPGA image from the FIT image
- time3: Time to program the FPGA