Introduce a function which passes an fpga compatible string from FIT images to FPGA drivers. This lets the different implementations decide how to handle it.

Some code of Jorge Ramirez-Ortiz jorge@foundries.io is reused.

Signed-off-by: Oleksandr Suvorov oleksandr.suvorov@foundries.io ---

(no changes since v1)

common/spl/spl_fit.c | 6 ++---- drivers/fpga/fpga.c | 26 ++++++++++++++++++++++++-- include/fpga.h | 4 ++++ 3 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c index 5fe0273d66..bd29e8c59d 100644 --- a/common/spl/spl_fit.c +++ b/common/spl/spl_fit.c @@ -580,11 +580,9 @@ static int spl_fit_upload_fpga(struct spl_fit_info *ctx, int node, compatible = fdt_getprop(ctx->fit, node, "compatible", NULL); if (!compatible) warn_deprecated("'fpga' image without 'compatible' property"); - else if (strcmp(compatible, "u-boot,fpga-legacy")) - printf("Ignoring compatible = %s property\n", compatible);

- ret = fpga_load(0, (void *)fpga_image->load_addr, fpga_image->size, - BIT_FULL); + ret = fit_fpga_load(0, (void *)fpga_image->load_addr, fpga_image->size, + BIT_FULL, compatible); if (ret) { printf("%s: Cannot load the image to the FPGA\n", __func__); return ret; diff --git a/drivers/fpga/fpga.c b/drivers/fpga/fpga.c index 3b0a44b242..e4b5dd392e 100644 --- a/drivers/fpga/fpga.c +++ b/drivers/fpga/fpga.c @@ -249,15 +249,31 @@ int fpga_loads(int devnum, const void *buf, size_t size, } #endif

+int fit_fpga_load(int devnum, const void *buf, size_t bsize, + bitstream_type bstype, const char *compatible) +{ + fpga_desc *desc = (fpga_desc *) fpga_validate(devnum, buf, bsize, + (char *)__func__); + + if (!desc) + return FPGA_FAIL; + /* + * Store the compatible string to proceed it in underlying + * functions + */ + desc->compatible = (char *)compatible; + + return fpga_load(devnum, buf, bsize, bstype); +} /* - * Generic multiplexing code + * Generic multiplexing code: + * Each architecture must handle the mandatory FPGA DT compatible property. */ int fpga_load(int devnum, const void *buf, size_t bsize, bitstream_type bstype) { int ret_val = FPGA_FAIL; /* assume failure */ const fpga_desc *desc = fpga_validate(devnum, buf, bsize, (char *)__func__); - if (desc) { switch (desc->devtype) { case fpga_xilinx: @@ -270,6 +286,9 @@ int fpga_load(int devnum, const void *buf, size_t bsize, bitstream_type bstype) break; case fpga_altera: #if defined(CONFIG_FPGA_ALTERA) + if (strcmp(desc->compatible, "u-boot,fpga-legacy")) + printf("Ignoring compatible = %s property\n", + desc->compatible); ret_val = altera_load(desc->devdesc, buf, bsize); #else fpga_no_sup((char *)__func__, "Altera devices"); @@ -277,6 +296,9 @@ int fpga_load(int devnum, const void *buf, size_t bsize, bitstream_type bstype) break; case fpga_lattice: #if defined(CONFIG_FPGA_LATTICE) + if (strcmp(desc->compatible, "u-boot,fpga-legacy")) + printf("Ignoring compatible = %s property\n", + desc->compatible); ret_val = lattice_load(desc->devdesc, buf, bsize); #else fpga_no_sup((char *)__func__, "Lattice devices"); diff --git a/include/fpga.h b/include/fpga.h index ec5144334d..2891f32106 100644 --- a/include/fpga.h +++ b/include/fpga.h @@ -35,6 +35,7 @@ typedef enum { /* typedef fpga_type */ typedef struct { /* typedef fpga_desc */ fpga_type devtype; /* switch value to select sub-functions */ void *devdesc; /* real device descriptor */ + char *compatible; /* device compatible string */ } fpga_desc; /* end, typedef fpga_desc */

typedef struct { /* typedef fpga_desc */ @@ -63,6 +64,9 @@ int fpga_add(fpga_type devtype, void *desc); int fpga_count(void); const fpga_desc *const fpga_get_desc(int devnum); int fpga_is_partial_data(int devnum, size_t img_len); +/* the DT compatible property must be handled by the different FPGA archs */ +int fit_fpga_load(int devnum, const void *buf, size_t bsize, + bitstream_type bstype, const char *compatible); int fpga_load(int devnum, const void *buf, size_t bsize, bitstream_type bstype); int fpga_fsload(int devnum, const void *buf, size_t size,

On 11/1/21 11:47, Oleksandr Suvorov wrote:

Add supporting new compatible string "u-boot,zynqmp-fpga-ddrauth" to handle loading authenticated images (DDR).

Based on solution by Jorge Ramirez-Ortiz jorge@foundries.io Signed-off-by: Oleksandr Suvorov oleksandr.suvorov@foundries.io Co-developed-by: Ricardo Salveti ricardo@foundries.io Signed-off-by: Ricardo Salveti ricardo@foundries.io Tested-by: Ricardo Salveti ricardo@foundries.io

---

Changes in v2:

• add function fit_fpga_load() to simplify calls of fpga_load() from contexts without a compatible attribute.

• move all ZynqMP-specific logic to drivers/fpga/zynqmppl.c

• prepare for passing a "compatible" FDT property to any fpga driver.

  drivers/fpga/zynqmppl.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)

diff --git a/drivers/fpga/zynqmppl.c b/drivers/fpga/zynqmppl.c index 8ff12bf50a..ce25381890 100644 --- a/drivers/fpga/zynqmppl.c +++ b/drivers/fpga/zynqmppl.c @@ -9,6 +9,7 @@ #include <common.h> #include <compiler.h> #include <cpu_func.h> +#include <fpga.h> #include <log.h> #include <zynqmppl.h> #include <zynqmp_firmware.h> @@ -209,6 +210,26 @@ static int zynqmp_load(xilinx_desc *desc, const void *buf, size_t bsize, u32 buf_lo, buf_hi; u32 ret_payload[PAYLOAD_ARG_CNT]; bool xilfpga_old = false;

• fpga_desc *fdesc = container_of((void *)desc, fpga_desc, devdesc);
• if (fdesc && fdesc->compatible &&

•    !strcmp(fdesc->compatible, "u-boot,zynqmp-fpga-ddrauth")) {
  

+#if CONFIG_IS_ENABLED(FPGA_LOAD_SECURE)

• struct fpga_secure_info info = { 0 };
  

• if (!desc->operations->loads) {
  

• 	printf("%s: Missing load operation\n", __func__);
  

• 	return FPGA_FAIL;
  

• }
  

• /* DDR authentication */
  

• info.authflag = 1;
  

• info.encflag = 2;
  

• return desc->operations->loads(desc, buf, bsize, &info);
  

+#else

• printf("No support for %s\n", fdesc->compatible);
  

• return FPGA_FAIL;
  

+#endif

• }

  if (zynqmp_firmware_version() <= PMUFW_V1_0) { puts("WARN: PMUFW v1.0 or less is detected\n");

This is fine but I would like to also see update in documentation to describe that possible/supported types.

Thanks, Michal

On 11/1/21 11:47, Oleksandr Suvorov wrote:

Introduce a function which passes an fpga compatible string from FIT images to FPGA drivers. This lets the different implementations decide how to handle it.

Some code of Jorge Ramirez-Ortiz jorge@foundries.io is reused.

Signed-off-by: Oleksandr Suvorov oleksandr.suvorov@foundries.io

(no changes since v1)

common/spl/spl_fit.c | 6 ++---- drivers/fpga/fpga.c | 26 ++++++++++++++++++++++++-- include/fpga.h | 4 ++++ 3 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c index 5fe0273d66..bd29e8c59d 100644 --- a/common/spl/spl_fit.c +++ b/common/spl/spl_fit.c @@ -580,11 +580,9 @@ static int spl_fit_upload_fpga(struct spl_fit_info *ctx, int node, compatible = fdt_getprop(ctx->fit, node, "compatible", NULL); if (!compatible) warn_deprecated("'fpga' image without 'compatible' property");

• else if (strcmp(compatible, "u-boot,fpga-legacy"))

• printf("Ignoring compatible = %s property\n", compatible);
  
  

• ret = fpga_load(0, (void *)fpga_image->load_addr, fpga_image->size,

• 	BIT_FULL);
  

• ret = fit_fpga_load(0, (void *)fpga_image->load_addr, fpga_image->size,

• 	    BIT_FULL, compatible);
  

  if (ret) { printf("%s: Cannot load the image to the FPGA\n", __func__); return ret;

diff --git a/drivers/fpga/fpga.c b/drivers/fpga/fpga.c index 3b0a44b242..e4b5dd392e 100644 --- a/drivers/fpga/fpga.c +++ b/drivers/fpga/fpga.c @@ -249,15 +249,31 @@ int fpga_loads(int devnum, const void *buf, size_t size, } #endif

+int fit_fpga_load(int devnum, const void *buf, size_t bsize,

•   bitstream_type bstype, const char *compatible)
  

+{

• fpga_desc *desc = (fpga_desc *) fpga_validate(devnum, buf, bsize,

• 			      (char *)__func__);
  

nit: Better to separate it:

fpga_desc *desc;

desc = (fpga_desc *) fpga_validate(devnum, buf, bsize, (char *)__func__); if (!desc) ...

• if (!desc)

• return FPGA_FAIL;
  

• /*

• * Store the compatible string to proceed it in underlying
  

• * functions
  

• */
  

• desc->compatible = (char *)compatible;
• return fpga_load(devnum, buf, bsize, bstype);

+} /*

• • Generic multiplexing code

• • Generic multiplexing code:
• • Each architecture must handle the mandatory FPGA DT compatible property.
  */ int fpga_load(int devnum, const void *buf, size_t bsize, bitstream_type bstype) { int ret_val = FPGA_FAIL; /* assume failure */ const fpga_desc *desc = fpga_validate(devnum, buf, bsize, (char *)__func__);

this should break the rule that you should separate variable declaration/definitions from the actual code. Do it via two steps as is described above.

And in this content it is separate change.

if (desc) { switch (desc->devtype) { case fpga_xilinx: @@ -270,6 +286,9 @@ int fpga_load(int devnum, const void *buf, size_t bsize, bitstream_type bstype) break; case fpga_altera: #if defined(CONFIG_FPGA_ALTERA)

• 	if (strcmp(desc->compatible, "u-boot,fpga-legacy"))
  

strncmp please

• 		printf("Ignoring compatible = %s property\n",
  

• 		       desc->compatible);
  ret_val = altera_load(desc->devdesc, buf, bsize);
  

  #else fpga_no_sup((char *)__func__, "Altera devices");

@@ -277,6 +296,9 @@ int fpga_load(int devnum, const void *buf, size_t bsize, bitstream_type bstype) break; case fpga_lattice: #if defined(CONFIG_FPGA_LATTICE)

• 	if (strcmp(desc->compatible, "u-boot,fpga-legacy"))
  

• 		printf("Ignoring compatible = %s property\n",
  

• 		       desc->compatible);
  ret_val = lattice_load(desc->devdesc, buf, bsize);
  

  #else fpga_no_sup((char *)__func__, "Lattice devices");

diff --git a/include/fpga.h b/include/fpga.h index ec5144334d..2891f32106 100644 --- a/include/fpga.h +++ b/include/fpga.h @@ -35,6 +35,7 @@ typedef enum { /* typedef fpga_type */ typedef struct { /* typedef fpga_desc */ fpga_type devtype; /* switch value to select sub-functions */ void *devdesc; /* real device descriptor */

• char *compatible; /* device compatible string */ } fpga_desc; /* end, typedef fpga_desc */

  typedef struct { /* typedef fpga_desc */

@@ -63,6 +64,9 @@ int fpga_add(fpga_type devtype, void *desc); int fpga_count(void); const fpga_desc *const fpga_get_desc(int devnum); int fpga_is_partial_data(int devnum, size_t img_len); +/* the DT compatible property must be handled by the different FPGA archs */ +int fit_fpga_load(int devnum, const void *buf, size_t bsize,

•   bitstream_type bstype, const char *compatible);
  

  int fpga_load(int devnum, const void *buf, size_t bsize, bitstream_type bstype); int fpga_fsload(int devnum, const void *buf, size_t size,

M

On 11/1/21 11:47, Oleksandr Suvorov wrote:

It allows using this feature without enabling the "fpga loads" command.

Signed-off-by: Oleksandr Suvorov oleksandr.suvorov@foundries.io

(no changes since v1)

cmd/Kconfig | 3 ++- drivers/fpga/Kconfig | 14 ++++++++++++++ drivers/fpga/fpga.c | 2 +- drivers/fpga/xilinx.c | 2 +- drivers/fpga/zynqmppl.c | 4 ++-- 5 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/cmd/Kconfig b/cmd/Kconfig index 5b30b13e43..270d1765d3 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -949,8 +949,9 @@ config CMD_FPGA_LOADP a partial bitstream.

config CMD_FPGA_LOAD_SECURE

• bool "fpga loads - loads secure bitstreams (Xilinx only)"

• bool "fpga loads - loads secure bitstreams" depends on CMD_FPGA
• select FPGA_LOAD_SECURE help Enables the fpga loads command which is used to load secure (authenticated or encrypted or both) bitstreams on to FPGA.

diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig index dc0b3dd31b..262f95a252 100644 --- a/drivers/fpga/Kconfig +++ b/drivers/fpga/Kconfig @@ -85,4 +85,18 @@ config FPGA_ZYNQPL Enable FPGA driver for loading bitstream in BIT and BIN format on Xilinx Zynq devices.

+config FPGA_LOAD_SECURE

• bool "Enable loading secure bitstreams"
• depends on FPGA
• help

•  Enables the fpga loads() functions that are used to load secure
  

•  (authenticated or encrypted or both) bitstreams on to FPGA.
  

+config SPL_FPGA_LOAD_SECURE

• bool "Enable loading secure bitstreams for SPL"
• depends on FPGA
• help

•  Enables the fpga loads() functions that are used to load secure
  

•  (authenticated or encrypted or both) bitstreams on to FPGA.
  

• endmenu

diff --git a/drivers/fpga/fpga.c b/drivers/fpga/fpga.c index fe3dfa1233..3b0a44b242 100644 --- a/drivers/fpga/fpga.c +++ b/drivers/fpga/fpga.c @@ -220,7 +220,7 @@ int fpga_fsload(int devnum, const void *buf, size_t size, } #endif

-#if defined(CONFIG_CMD_FPGA_LOAD_SECURE) +#if CONFIG_IS_ENABLED(FPGA_LOAD_SECURE) int fpga_loads(int devnum, const void *buf, size_t size, struct fpga_secure_info *fpga_sec_info) { diff --git a/drivers/fpga/xilinx.c b/drivers/fpga/xilinx.c index cbebefb55f..6bc1bc491f 100644 --- a/drivers/fpga/xilinx.c +++ b/drivers/fpga/xilinx.c @@ -172,7 +172,7 @@ int xilinx_loadfs(xilinx_desc *desc, const void *buf, size_t bsize, } #endif

-#if defined(CONFIG_CMD_FPGA_LOAD_SECURE) +#if CONFIG_IS_ENABLED(FPGA_LOAD_SECURE) int xilinx_loads(xilinx_desc *desc, const void *buf, size_t bsize, struct fpga_secure_info *fpga_sec_info) { diff --git a/drivers/fpga/zynqmppl.c b/drivers/fpga/zynqmppl.c index 6b394869db..8ff12bf50a 100644 --- a/drivers/fpga/zynqmppl.c +++ b/drivers/fpga/zynqmppl.c @@ -245,7 +245,7 @@ static int zynqmp_load(xilinx_desc *desc, const void *buf, size_t bsize, return ret; }

-#if defined(CONFIG_CMD_FPGA_LOAD_SECURE) && !defined(CONFIG_SPL_BUILD) +#if CONFIG_IS_ENABLED(FPGA_LOAD_SECURE) static int zynqmp_loads(xilinx_desc *desc, const void *buf, size_t bsize, struct fpga_secure_info *fpga_sec_info) { @@ -306,7 +306,7 @@ static int zynqmp_pcap_info(xilinx_desc *desc)

struct xilinx_fpga_op zynqmp_op = { .load = zynqmp_load, -#if defined(CONFIG_CMD_FPGA_LOAD_SECURE) && !defined(CONFIG_SPL_BUILD) +#if CONFIG_IS_ENABLED(FPGA_LOAD_SECURE) .loads = zynqmp_loads, #endif .info = zynqmp_pcap_info,

Acked-by: Michal Simek michal.simek@xilinx.com

Thanks, Michal