diff mbox series

[U-Boot,RFC,4/4] arm: zynq: spl: implement FPGA load from FIT

Message ID 20180718074141.16539-5-luaraneda@gmail.com
State RFC
Delegated to: Michal Simek
Headers show
Series arm: zynq: implement FPGA load from SPL | expand

Commit Message

Luis Araneda July 18, 2018, 7:41 a.m. UTC
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(+)

Comments

Michal Simek July 18, 2018, 1:22 p.m. UTC | #1
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
Luis Araneda July 18, 2018, 6:14 p.m. UTC | #2
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.
Michal Simek July 19, 2018, 6:15 a.m. UTC | #3
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
Luis Araneda July 19, 2018, 5:22 p.m. UTC | #4
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.
Michal Simek July 20, 2018, 10:34 a.m. UTC | #5
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
diff mbox series

Patch

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