Hi Adrian, On Wed, May 4, 2022 at 5:28 PM Adrian Fiergolski <adrian.fiergol...@fastree3d.com> wrote: > > 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.
Thanks for catching this. I'm addressing the issue this Friday. > > > >> + > >> + /* > >> + * 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 -- Best regards Oleksandr Oleksandr Suvorov cryo...@gmail.com