* [U-Boot] [RFC PATCH 1/4] spl: fit: display a message when an FPGA image is loaded
2018-07-18 7:41 [U-Boot] [RFC PATCH 0/4] arm: zynq: implement FPGA load from SPL Luis Araneda
@ 2018-07-18 7:41 ` Luis Araneda
2018-07-18 13:55 ` Michal Simek
2018-07-18 7:41 ` [U-Boot] [RFC PATCH 2/4] drivers: fpga: zynqpl: fix compilation with SPL Luis Araneda
` (3 subsequent siblings)
4 siblings, 1 reply; 20+ messages in thread
From: Luis Araneda @ 2018-07-18 7:41 UTC (permalink / raw)
To: u-boot
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 <luaraneda@gmail.com>
---
common/spl/spl_fit.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index 5b51a28a08..9eabb1c105 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -412,6 +412,7 @@ int spl_load_simple_fit(struct spl_image_info *spl_image,
printf("%s: Cannot load the FPGA: %i\n", __func__, ret);
return ret;
}
+ puts("FPGA image loaded from FIT\n");
node = -1;
}
#endif
--
2.18.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [U-Boot] [RFC PATCH 1/4] spl: fit: display a message when an FPGA image is loaded
2018-07-18 7:41 ` [U-Boot] [RFC PATCH 1/4] spl: fit: display a message when an FPGA image is loaded Luis Araneda
@ 2018-07-18 13:55 ` Michal Simek
0 siblings, 0 replies; 20+ messages in thread
From: Michal Simek @ 2018-07-18 13:55 UTC (permalink / raw)
To: u-boot
On 18.7.2018 09:41, Luis Araneda wrote:
> 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 <luaraneda@gmail.com>
> ---
> common/spl/spl_fit.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
> index 5b51a28a08..9eabb1c105 100644
> --- a/common/spl/spl_fit.c
> +++ b/common/spl/spl_fit.c
> @@ -412,6 +412,7 @@ int spl_load_simple_fit(struct spl_image_info *spl_image,
> printf("%s: Cannot load the FPGA: %i\n", __func__, ret);
> return ret;
> }
> + puts("FPGA image loaded from FIT\n");
> node = -1;
> }
> #endif
>
This looks good.
M
^ permalink raw reply [flat|nested] 20+ messages in thread
* [U-Boot] [RFC PATCH 2/4] drivers: fpga: zynqpl: fix compilation with SPL
2018-07-18 7:41 [U-Boot] [RFC PATCH 0/4] arm: zynq: implement FPGA load from SPL Luis Araneda
2018-07-18 7:41 ` [U-Boot] [RFC PATCH 1/4] spl: fit: display a message when an FPGA image is loaded Luis Araneda
@ 2018-07-18 7:41 ` Luis Araneda
2018-07-18 13:55 ` Michal Simek
2018-07-18 7:41 ` [U-Boot] [RFC PATCH 3/4] arm: zynq: spl: fix FPGA initialization Luis Araneda
` (2 subsequent siblings)
4 siblings, 1 reply; 20+ messages in thread
From: Luis Araneda @ 2018-07-18 7:41 UTC (permalink / raw)
To: u-boot
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 <luaraneda@gmail.com>
---
drivers/fpga/zynqpl.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/fpga/zynqpl.c b/drivers/fpga/zynqpl.c
index fd37d18c7f..8e49c7010e 100644
--- a/drivers/fpga/zynqpl.c
+++ b/drivers/fpga/zynqpl.c
@@ -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)
{
@@ -493,7 +493,7 @@ static int zynq_loadfs(xilinx_desc *desc, const void *buf, size_t bsize,
struct xilinx_fpga_op zynq_op = {
.load = zynq_load,
-#if defined(CONFIG_CMD_FPGA_LOADFS)
+#if defined(CONFIG_CMD_FPGA_LOADFS) && !defined(CONFIG_SPL_BUILD)
.loadfs = zynq_loadfs,
#endif
};
--
2.18.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [U-Boot] [RFC PATCH 2/4] drivers: fpga: zynqpl: fix compilation with SPL
2018-07-18 7:41 ` [U-Boot] [RFC PATCH 2/4] drivers: fpga: zynqpl: fix compilation with SPL Luis Araneda
@ 2018-07-18 13:55 ` Michal Simek
0 siblings, 0 replies; 20+ messages in thread
From: Michal Simek @ 2018-07-18 13:55 UTC (permalink / raw)
To: u-boot
On 18.7.2018 09:41, Luis Araneda wrote:
> 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 <luaraneda@gmail.com>
> ---
> drivers/fpga/zynqpl.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/fpga/zynqpl.c b/drivers/fpga/zynqpl.c
> index fd37d18c7f..8e49c7010e 100644
> --- a/drivers/fpga/zynqpl.c
> +++ b/drivers/fpga/zynqpl.c
> @@ -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)
> {
> @@ -493,7 +493,7 @@ static int zynq_loadfs(xilinx_desc *desc, const void *buf, size_t bsize,
>
> struct xilinx_fpga_op zynq_op = {
> .load = zynq_load,
> -#if defined(CONFIG_CMD_FPGA_LOADFS)
> +#if defined(CONFIG_CMD_FPGA_LOADFS) && !defined(CONFIG_SPL_BUILD)
> .loadfs = zynq_loadfs,
> #endif
> };
>
This looks good.
M
^ permalink raw reply [flat|nested] 20+ messages in thread
* [U-Boot] [RFC PATCH 3/4] arm: zynq: spl: fix FPGA initialization
2018-07-18 7:41 [U-Boot] [RFC PATCH 0/4] arm: zynq: implement FPGA load from SPL Luis Araneda
2018-07-18 7:41 ` [U-Boot] [RFC PATCH 1/4] spl: fit: display a message when an FPGA image is loaded Luis Araneda
2018-07-18 7:41 ` [U-Boot] [RFC PATCH 2/4] drivers: fpga: zynqpl: fix compilation with SPL Luis Araneda
@ 2018-07-18 7:41 ` Luis Araneda
2018-07-18 13:55 ` Michal Simek
2018-07-18 7:41 ` [U-Boot] [RFC PATCH 4/4] arm: zynq: spl: implement FPGA load from FIT Luis Araneda
2018-07-18 8:00 ` [U-Boot] [RFC PATCH 0/4] arm: zynq: implement FPGA load from SPL Michal Simek
4 siblings, 1 reply; 20+ messages in thread
From: Luis Araneda @ 2018-07-18 7:41 UTC (permalink / raw)
To: u-boot
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 <luaraneda@gmail.com>
---
arch/arm/mach-zynq/spl.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/arm/mach-zynq/spl.c b/arch/arm/mach-zynq/spl.c
index 83297d6c69..9b7c0be951 100644
--- a/arch/arm/mach-zynq/spl.c
+++ b/arch/arm/mach-zynq/spl.c
@@ -29,6 +29,9 @@ void board_init_f(ulong dummy)
void spl_board_init(void)
{
preloader_console_init();
+#if defined(CONFIG_ARCH_EARLY_INIT_R) && defined(CONFIG_SPL_FPGA_SUPPORT)
+ arch_early_init_r();
+#endif
board_init();
}
#endif
--
2.18.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [U-Boot] [RFC PATCH 3/4] arm: zynq: spl: fix FPGA initialization
2018-07-18 7:41 ` [U-Boot] [RFC PATCH 3/4] arm: zynq: spl: fix FPGA initialization Luis Araneda
@ 2018-07-18 13:55 ` Michal Simek
0 siblings, 0 replies; 20+ messages in thread
From: Michal Simek @ 2018-07-18 13:55 UTC (permalink / raw)
To: u-boot
On 18.7.2018 09:41, Luis Araneda wrote:
> 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 <luaraneda@gmail.com>
> ---
> arch/arm/mach-zynq/spl.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/arch/arm/mach-zynq/spl.c b/arch/arm/mach-zynq/spl.c
> index 83297d6c69..9b7c0be951 100644
> --- a/arch/arm/mach-zynq/spl.c
> +++ b/arch/arm/mach-zynq/spl.c
> @@ -29,6 +29,9 @@ void board_init_f(ulong dummy)
> void spl_board_init(void)
> {
> preloader_console_init();
> +#if defined(CONFIG_ARCH_EARLY_INIT_R) && defined(CONFIG_SPL_FPGA_SUPPORT)
> + arch_early_init_r();
> +#endif
> board_init();
> }
> #endif
>
This looks good.
M
^ permalink raw reply [flat|nested] 20+ messages in thread
* [U-Boot] [RFC PATCH 4/4] arm: zynq: spl: implement FPGA load from FIT
2018-07-18 7:41 [U-Boot] [RFC PATCH 0/4] arm: zynq: implement FPGA load from SPL Luis Araneda
` (2 preceding siblings ...)
2018-07-18 7:41 ` [U-Boot] [RFC PATCH 3/4] arm: zynq: spl: fix FPGA initialization Luis Araneda
@ 2018-07-18 7:41 ` Luis Araneda
2018-07-18 13:22 ` Michal Simek
2018-07-18 8:00 ` [U-Boot] [RFC PATCH 0/4] arm: zynq: implement FPGA load from SPL Michal Simek
4 siblings, 1 reply; 20+ messages in thread
From: Luis Araneda @ 2018-07-18 7:41 UTC (permalink / raw)
To: u-boot
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 <luaraneda@gmail.com>
---
arch/arm/mach-zynq/spl.c | 39 +++++++++++++++++++++++++++++++++++++++
1 file changed, 39 insertions(+)
diff --git a/arch/arm/mach-zynq/spl.c b/arch/arm/mach-zynq/spl.c
index 9b7c0be951..42907afa94 100644
--- a/arch/arm/mach-zynq/spl.c
+++ b/arch/arm/mach-zynq/spl.c
@@ -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);
+ if (ret) {
+ debug("%s: Cannot load the image to the FPGA\n", __func__);
+ free(data_buf);
+ return ret;
+ }
+
+ free(data_buf);
+ return 0;
+}
+#endif
--
2.18.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [U-Boot] [RFC PATCH 4/4] arm: zynq: spl: implement FPGA load from FIT
2018-07-18 7:41 ` [U-Boot] [RFC PATCH 4/4] arm: zynq: spl: implement FPGA load from FIT Luis Araneda
@ 2018-07-18 13:22 ` Michal Simek
2018-07-18 18:14 ` Luis Araneda
0 siblings, 1 reply; 20+ messages in thread
From: Michal Simek @ 2018-07-18 13:22 UTC (permalink / raw)
To: u-boot
On 18.7.2018 09:41, Luis Araneda wrote:
> 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 <luaraneda@gmail.com>
> ---
> arch/arm/mach-zynq/spl.c | 39 +++++++++++++++++++++++++++++++++++++++
> 1 file changed, 39 insertions(+)
>
> diff --git a/arch/arm/mach-zynq/spl.c b/arch/arm/mach-zynq/spl.c
> index 9b7c0be951..42907afa94 100644
> --- a/arch/arm/mach-zynq/spl.c
> +++ b/arch/arm/mach-zynq/spl.c
> @@ -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);
> + if (ret) {
> + debug("%s: Cannot load the image to the FPGA\n", __func__);
> + free(data_buf);
> + return ret;
> + }
> +
> + free(data_buf);
> + return 0;
> +}
> +#endif
>
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.
Thanks,
Michal
^ permalink raw reply [flat|nested] 20+ messages in thread
* [U-Boot] [RFC PATCH 4/4] arm: zynq: spl: implement FPGA load from FIT
2018-07-18 13:22 ` Michal Simek
@ 2018-07-18 18:14 ` Luis Araneda
2018-07-19 6:15 ` Michal Simek
0 siblings, 1 reply; 20+ messages in thread
From: Luis Araneda @ 2018-07-18 18:14 UTC (permalink / raw)
To: u-boot
Hi Michal,
On Wed, Jul 18, 2018 at 9:22 AM Michal Simek <michal.simek@xilinx.com> wrote:
> 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.
Thanks,
Luis Araneda.
^ permalink raw reply [flat|nested] 20+ messages in thread
* [U-Boot] [RFC PATCH 4/4] arm: zynq: spl: implement FPGA load from FIT
2018-07-18 18:14 ` Luis Araneda
@ 2018-07-19 6:15 ` Michal Simek
2018-07-19 17:22 ` Luis Araneda
0 siblings, 1 reply; 20+ messages in thread
From: Michal Simek @ 2018-07-19 6:15 UTC (permalink / raw)
To: u-boot
Hi,
On 18.7.2018 20:14, Luis Araneda wrote:
> Hi Michal,
>
> On Wed, Jul 18, 2018 at 9:22 AM Michal Simek <michal.simek@xilinx.com> wrote:
>> 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.
Feel free to join that thread with Marek.
I think that what we have in u-boot source code is not correct and it is
one ugly hack which has no user and nothing is broken because support
hasn't been merged to mainline.
I have played with that yesterday and send this patch which should be
enough for you to go.
https://lists.denx.de/pipermail/u-boot/2018-July/335169.html
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.
Thanks,
Michal
^ permalink raw reply [flat|nested] 20+ messages in thread
* [U-Boot] [RFC PATCH 4/4] arm: zynq: spl: implement FPGA load from FIT
2018-07-19 6:15 ` Michal Simek
@ 2018-07-19 17:22 ` Luis Araneda
2018-07-20 10:34 ` Michal Simek
0 siblings, 1 reply; 20+ messages in thread
From: Luis Araneda @ 2018-07-19 17:22 UTC (permalink / raw)
To: u-boot
Hi Michal,
On Thu, Jul 19, 2018 at 2:16 AM Michal Simek <michal.simek@xilinx.com> wrote:
> 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:
> ./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 :)
Thanks,
Luis Araneda.
^ permalink raw reply [flat|nested] 20+ messages in thread
* [U-Boot] [RFC PATCH 4/4] arm: zynq: spl: implement FPGA load from FIT
2018-07-19 17:22 ` Luis Araneda
@ 2018-07-20 10:34 ` Michal Simek
0 siblings, 0 replies; 20+ messages in thread
From: Michal Simek @ 2018-07-20 10:34 UTC (permalink / raw)
To: u-boot
On 19.7.2018 19:22, Luis Araneda wrote:
> Hi Michal,
>
> On Thu, Jul 19, 2018 at 2:16 AM Michal Simek <michal.simek@xilinx.com> wrote:
>> 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:
>> ./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 :)
ok.
Thanks,
Michal
^ permalink raw reply [flat|nested] 20+ messages in thread
* [U-Boot] [RFC PATCH 0/4] arm: zynq: implement FPGA load from SPL
2018-07-18 7:41 [U-Boot] [RFC PATCH 0/4] arm: zynq: implement FPGA load from SPL Luis Araneda
` (3 preceding siblings ...)
2018-07-18 7:41 ` [U-Boot] [RFC PATCH 4/4] arm: zynq: spl: implement FPGA load from FIT Luis Araneda
@ 2018-07-18 8:00 ` Michal Simek
2018-07-18 18:02 ` Luis Araneda
4 siblings, 1 reply; 20+ messages in thread
From: Michal Simek @ 2018-07-18 8:00 UTC (permalink / raw)
To: u-boot
On 18.7.2018 09:41, Luis Araneda wrote:
> 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.
Thanks,
Michal
^ permalink raw reply [flat|nested] 20+ messages in thread
* [U-Boot] [RFC PATCH 0/4] arm: zynq: implement FPGA load from SPL
2018-07-18 8:00 ` [U-Boot] [RFC PATCH 0/4] arm: zynq: implement FPGA load from SPL Michal Simek
@ 2018-07-18 18:02 ` Luis Araneda
2018-07-19 6:22 ` Michal Simek
0 siblings, 1 reply; 20+ messages in thread
From: Luis Araneda @ 2018-07-18 18:02 UTC (permalink / raw)
To: u-boot
Hi Michal,
On Wed, Jul 18, 2018 at 4:00 AM Michal Simek <michal.simek@xilinx.com> wrote:
> 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.
Thanks,
Luis Araneda.
^ permalink raw reply [flat|nested] 20+ messages in thread
* [U-Boot] [RFC PATCH 0/4] arm: zynq: implement FPGA load from SPL
2018-07-18 18:02 ` Luis Araneda
@ 2018-07-19 6:22 ` Michal Simek
2018-07-19 23:37 ` Luis Araneda
0 siblings, 1 reply; 20+ messages in thread
From: Michal Simek @ 2018-07-19 6:22 UTC (permalink / raw)
To: u-boot
On 18.7.2018 20:02, Luis Araneda wrote:
> Hi Michal,
>
> On Wed, Jul 18, 2018 at 4:00 AM Michal Simek <michal.simek@xilinx.com> wrote:
>> 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,
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
> 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>;
> };
But that's separate issue how to automate building image which contain
fpga.
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.
> 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.
Take a look at pmufw handling for zynqmp for inspiration. But again
that's different issue and there shouldn't be a problem to enable this
feature without fit generation.
>
> 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.
My reactions on xilinx.com should be much faster because I am trying to
keep number of emails low compare to my second email where all mailing
lists are coming.
I will look at eeprom series.
Thanks,
Michal
^ permalink raw reply [flat|nested] 20+ messages in thread
* [U-Boot] [RFC PATCH 0/4] arm: zynq: implement FPGA load from SPL
2018-07-19 6:22 ` Michal Simek
@ 2018-07-19 23:37 ` Luis Araneda
2018-07-20 10:38 ` Michal Simek
0 siblings, 1 reply; 20+ messages in thread
From: Luis Araneda @ 2018-07-19 23:37 UTC (permalink / raw)
To: u-boot
Hi Michal,
On Thu, Jul 19, 2018 at 2:23 AM Michal Simek <michal.simek@xilinx.com> wrote:
> On 18.7.2018 20:02, Luis Araneda wrote:
> > [...]
> > 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.
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?
> 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.
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:
> 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.
> > 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.
Will do, thanks for the heads up.
Thanks,
Luis Araneda.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: gzip_test.patch
Type: text/x-patch
Size: 2137 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180719/9160dcbd/attachment.bin>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [U-Boot] [RFC PATCH 0/4] arm: zynq: implement FPGA load from SPL
2018-07-19 23:37 ` Luis Araneda
@ 2018-07-20 10:38 ` Michal Simek
2018-07-20 16:17 ` Luis Araneda
0 siblings, 1 reply; 20+ messages in thread
From: Michal Simek @ 2018-07-20 10:38 UTC (permalink / raw)
To: u-boot
Hi,
On 20.7.2018 01:37, Luis Araneda wrote:
> Hi Michal,
>
> On Thu, Jul 19, 2018 at 2:23 AM Michal Simek <michal.simek@xilinx.com> wrote:
>> On 18.7.2018 20:02, Luis Araneda wrote:
>>> [...]
>>> 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.
>
> 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?
We need that functionality first but then enable it for all boards is
fine for me and via one patch.
>
>> 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.
>
> 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:
>> 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.
Can you please be more specific what time1/time2 and time3 means?
>
> At least on my setup (Zybo Z7-20), gzip just increase the boot time.
It could be because of bitstream size is quite small for this chip.
Thanks,
Michal
^ permalink raw reply [flat|nested] 20+ messages in thread
* [U-Boot] [RFC PATCH 0/4] arm: zynq: implement FPGA load from SPL
2018-07-20 10:38 ` Michal Simek
@ 2018-07-20 16:17 ` Luis Araneda
2018-07-24 13:42 ` Michal Simek
0 siblings, 1 reply; 20+ messages in thread
From: Luis Araneda @ 2018-07-20 16:17 UTC (permalink / raw)
To: u-boot
Hi Michal,
On Fri, Jul 20, 2018 at 6:38 AM Michal Simek <michal.simek@xilinx.com> wrote:
> On 20.7.2018 01:37, Luis Araneda wrote:
> > Hi Michal,
> >
> > On Thu, Jul 19, 2018 at 2:23 AM Michal Simek <michal.simek@xilinx.com> wrote:
> We need that functionality first but then enable it for all boards is
> fine for me and via one patch.
Ok
> 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
Thanks,
Luis Araneda.
^ permalink raw reply [flat|nested] 20+ messages in thread
* [U-Boot] [RFC PATCH 0/4] arm: zynq: implement FPGA load from SPL
2018-07-20 16:17 ` Luis Araneda
@ 2018-07-24 13:42 ` Michal Simek
0 siblings, 0 replies; 20+ messages in thread
From: Michal Simek @ 2018-07-24 13:42 UTC (permalink / raw)
To: u-boot
Hi,
On 20.7.2018 18:17, Luis Araneda wrote:
> Hi Michal,
>
> On Fri, Jul 20, 2018 at 6:38 AM Michal Simek <michal.simek@xilinx.com> wrote:
>> On 20.7.2018 01:37, Luis Araneda wrote:
>>> Hi Michal,
>>>
>>> On Thu, Jul 19, 2018 at 2:23 AM Michal Simek <michal.simek@xilinx.com> wrote:
>> We need that functionality first but then enable it for all boards is
>> fine for me and via one patch.
>
> Ok
>
>> 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
Sorry I missed that attachment.
First of all I have sent patch for that gzip.
On zc706 with 13MB bitstream size this looks much better.
file size (bytes) time1 time2 time3
uncompressed 13869613 2533 2694 4422
compressed -9 599149 144 765 2491
This is SD boot mode and initial time depends on SD you use.
Thanks,
Michal
^ permalink raw reply [flat|nested] 20+ messages in thread