Hi Oleksandr,

On 03.05.2022 09:42, Michal Simek wrote:


On 4/11/22 20:00, Adrian Fiergolski wrote:
From: Oleksandr Suvorov <oleksandr.suvo...@foundries.io>

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 <jo...@foundries.io> is reused.

Signed-off-by: Oleksandr Suvorov <oleksandr.suvo...@foundries.io>
Tested-by: Ricardo Salveti <rica...@foundries.io>
Co-developed-by: Adrian Fiergolski <adrian.fiergol...@fastree3d.com>
Signed-off-by: Adrian Fiergolski <adrian.fiergol...@fastree3d.com>
---
  common/spl/spl_fit.c |  6 ++----
  drivers/fpga/fpga.c  | 23 ++++++++++++++++++++++-
  include/fpga.h       |  4 ++++
  3 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index 1bbf824684..0e3c2a94b6 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -588,11 +588,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..a306dd81f9 100644
--- a/drivers/fpga/fpga.c
+++ b/drivers/fpga/fpga.c
@@ -249,8 +249,23 @@ 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_get_desc(devnum);

this generates warning which you should fix.

+  fpga_desc *desc = fpga_get_desc(devnum);
+                    ^~~~~~~~~~~~~
w+../drivers/fpga/fpga.c: In function ‘fit_fpga_load’:
w+../drivers/fpga/fpga.c:255:20: warning: initialization discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]

As it's your patch, could you address it?

The 'compatible' field can't be set here, as fpga_get_desc returns 'const fpga_desc * const'. The descriptors table is populated (fpga_add) in board_init. At that stage, I am afraid, we don't have access to the fit image information yet to set a proper 'compatible' (would need to check the code more carefully). Moreover, IMHO it's not the cleanest way to change the 'compatible' field after the driver's initialisation.



+
+    /*
+     * 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)
  {
@@ -270,6 +285,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 (strncmp(desc->compatible, "u-boot,fpga-legacy", 18))
+                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 +295,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 (strncmp(desc->compatible, "u-boot,fpga-legacy", 18))
+                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
Adrian

Reply via email to