Re: [RFC PATCH 1/2] fpga_load: pass compatible string

["Jorge Ramirez-Ortiz, Foundries" <jorge@foundries.io>]

On 14/10/21, Simon Glass wrote:
> Hi Jorge,
> 
> On Tue, 5 Oct 2021 at 05:13, Jorge Ramirez-Ortiz <jorge@foundries.io> wrote:
> >
> > Instead of ignoring the mandatory fpga compatible string, let the
> > different implementations decide how to handle it
> >
> > Signed-off-by: Jorge Ramirez-Ortiz <jorge@foundries.io>
> > ---
> >  cmd/fpga.c           |  8 ++++----
> >  common/image.c       |  4 ++--
> >  common/spl/spl_fit.c |  4 +---
> >  drivers/fpga/fpga.c  | 11 +++++++++--
> >  include/fpga.h       |  2 +-
> >  5 files changed, 17 insertions(+), 12 deletions(-)
> >
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>

thanks, I'll add your tag

> > diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
> 
> [..]
> 
> > index f41abca0cc..4db22efd8c 100644
> > --- a/common/spl/spl_fit.c
> > +++ b/common/spl/spl_fit.c
> > @@ -566,11 +566,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);
> > +                       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 fe3dfa1233..00aa4190b4 100644
> > --- a/drivers/fpga/fpga.c
> > +++ b/drivers/fpga/fpga.c
> > @@ -252,7 +252,8 @@ int fpga_loads(int devnum, const void *buf, size_t size,
> >  /*
> >   * Generic multiplexing code
> >   */
> > -int fpga_load(int devnum, const void *buf, size_t bsize, bitstream_type bstype)
> > +int fpga_load(int devnum, const void *buf, size_t bsize, bitstream_type bstype,
> > +             const char *compatible)
> >  {
> >         int ret_val = FPGA_FAIL;           /* assume failure */
> >         const fpga_desc *desc = fpga_validate(devnum, buf, bsize,
> > @@ -263,13 +264,16 @@ int fpga_load(int devnum, const void *buf, size_t bsize, bitstream_type bstype)
> >                 case fpga_xilinx:
> >  #if defined(CONFIG_FPGA_XILINX)
> >                         ret_val = xilinx_load(desc->devdesc, buf, bsize,
> > -                                             bstype);
> > +                                             bstype, compatible);
> >  #else
> >                         fpga_no_sup((char *)__func__, "Xilinx devices");
> >  #endif
> >                         break;
> >                 case fpga_altera:
> >  #if defined(CONFIG_FPGA_ALTERA)
> > +                       if (strcmp(compatible, "u-boot,fpga-legacy"))
> > +                               printf("Ignoring compatible = %s property\n",
> > +                                      compatible);
> >                         ret_val = altera_load(desc->devdesc, buf, bsize);
> >  #else
> >                         fpga_no_sup((char *)__func__, "Altera devices");
> > @@ -277,6 +281,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(compatible, "u-boot,fpga-legacy"))
> > +                               printf("Ignoring compatible = %s property\n",
> > +                                      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..d85ef2cf18 100644
> > --- a/include/fpga.h
> > +++ b/include/fpga.h
> > @@ -64,7 +64,7 @@ int fpga_count(void);
> >  const fpga_desc *const fpga_get_desc(int devnum);
> >  int fpga_is_partial_data(int devnum, size_t img_len);
> >  int fpga_load(int devnum, const void *buf, size_t bsize,
> > -             bitstream_type bstype);
> > +             bitstream_type bstype, const char *compatible);
> 
> Please can you add a function comment?

ok

> 
> >  int fpga_fsload(int devnum, const void *buf, size_t size,
> >                 fpga_fs_info *fpga_fsinfo);
> >  int fpga_loads(int devnum, const void *buf, size_t size,
> > --
> > 2.31.1
> >
> 
> Also the FPGA uclass is missing a sandbox driver/emulator and a test,
> if you have time to do that. At present FPGA tests in CI are skipped
> (e.g. with make qcheck).

I am really short of time ATM with another deadline coming (sorry!)
I believe Oleksandr might be willing to pick this up but I'll let him
add further.


> 
> Regards,
> Simon