[U-Boot] [PATCH v3 1/2] common: image: update boot_get_fpga to support arbitrary fpga image

Dalon Westergreen dwesterg at gmail.com
Wed Feb 22 14:22:40 UTC 2017


On Tue, 2017-02-21 at 21:00 -0700, Simon Glass wrote:
> Hi Dalon,
> 
> On 20 February 2017 at 07:56, Dalon Westergreen <dwesterg at gmail.com> wrote:
> > 
> > The implementation of boot_get_fpga only supported one fpga family.
> > This modification allows for any of the fpga devices supported by
> > fpga_load to be used.
> 
> Can you add some docs somewhere to explain how this is used? E.g. you
> could update something in doc/uImage.FIT/

sure thing. 

> > 
> > 
> > Signed-off-by: Dalon Westergreen <dwesterg at gmail.com>
> > 
> > --
> > Changes in v3:
> >  - Fix typos/caps in comments
> > Changes in v2:
> >  - Add fitimage support for fpga-devnum and fpga-partial-image
> >  - Use above in boot_get_fpga
> >  - for xilinx fpgas double check using image size to determine
> >    if image is a partial image
> > ---
> >  common/image-fit.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
> >  common/image.c     | 51 ++++++++++++++++++++++++++++++++-------------------
> >  include/image.h    |  5 +++++
> >  3 files changed, 88 insertions(+), 19 deletions(-)
> > 
> > diff --git a/common/image-fit.c b/common/image-fit.c
> > index 109ecfa..eb0c633 100644
> > --- a/common/image-fit.c
> > +++ b/common/image-fit.c
> > @@ -916,6 +916,57 @@ ulong fit_get_end(const void *fit)
> >  }
> > 
> >  /**
> > + * fit_image_fpga_get_devnum - get fpga devnum
> > + * @fit: pointer to the FIT format image header
> > + * @noffset: fpga node offset
> > + * @devnum: pointer to an int, will hold fpga devnum
> > + *
> > + * fit_image_fpga_get_devnum() finds the fpga devnum for which the fpga
> > data is
> > + * intended.  If the property is not found, we default to 0.
> > + *
> > + * returns:
> > + *     0, on devnum not found
> > + *     value, on devnum found
> > + */
> > +int fit_image_fpga_get_devnum(const void *fit, int noffset, int *devnum)
> > +{
> > +       int len;
> > +       int *value;
> > +
> > +       value = (int *)fdt_getprop(fit, noffset, FIT_FPGA_DEVNUM_PROP,
> > &len);
> 
> Can you use fdtdec_get_int()? It handles the endian conversion automatically.
> 
> > 
> > +       if (value == NULL || len != sizeof(int))
> > +               *devnum = 0;
> > +       else
> > +               *devnum = *value;
> > +
> > +       return 0;
> > +}
> > +
> > +/**
> > + * fit_image_fpga_is_partial - is partial fpga
> 
> This doesn't explain very much - can you rephrase?

Yes, i will elaboate
> 
> > 
> > + * @fit: pointer to the FIT format image header
> > + * @noffset: fpga node offset
> > + *
> > + * fit_image_fpga_is_partial() checks if the fpga node sets the property
> > + * indicating the data represents a partial fpga image.
> > + *
> > + * returns:
> > + *     0, on devnum not found
> > + *     value, on devnum found
> 
> But it seems to return 0 or 1?

Bad cut and paste... i will fix this.

> > 
> > + */
> > +int fit_image_fpga_is_partial(const void *fit, int noffset)
> > +{
> > +       int len;
> > +       int *value;
> > +
> > +       value = (int *)fdt_getprop(fit, noffset, FIT_FPGA_PARTIAL_PROP,
> > &len);
> > +       if ((value == NULL || len != sizeof(int)) || (value == 0))
> 
> Is this boolean? Could you use fdtdec_get_bool()?
> 
> > 
> > +               return 0;
> > +       else
> > +               return 1;
> > +}
> > +
> > +/**
> >   * fit_set_timestamp - set node timestamp property
> >   * @fit: pointer to the FIT format image header
> >   * @noffset: node offset
> > diff --git a/common/image.c b/common/image.c
> > index 0f88984..6480b0a 100644
> > --- a/common/image.c
> > +++ b/common/image.c
> > @@ -1306,7 +1306,7 @@ int boot_get_setup(bootm_headers_t *images, uint8_t
> > arch,
> >  }
> > 
> >  #if IMAGE_ENABLE_FIT
> > -#if defined(CONFIG_FPGA) && defined(CONFIG_FPGA_XILINX)
> > +#if defined(CONFIG_FPGA)
> >  int boot_get_fpga(int argc, char * const argv[], bootm_headers_t *images,
> >                   uint8_t arch, const ulong *ld_start, ulong * const ld_len)
> >  {
> > @@ -1316,9 +1316,10 @@ int boot_get_fpga(int argc, char * const argv[],
> > bootm_headers_t *images,
> >         int fit_img_result;
> >         const char *uname, *name;
> >         int err;
> > -       int devnum = 0; /* TODO support multi fpga platforms */
> > -       const fpga_desc * const desc = fpga_get_desc(devnum);
> > -       xilinx_desc *desc_xilinx = desc->devdesc;
> > +       int devnum;
> > +       const fpga_desc *desc;
> > +       xilinx_desc *desc_xilinx;
> > +       bitstream_type bstype = BIT_FULL;
> > 
> >         /* Check to see if the images struct has a FIT configuration */
> >         if (!genimg_has_config(images)) {
> > @@ -1365,26 +1366,38 @@ int boot_get_fpga(int argc, char * const argv[],
> > bootm_headers_t *images,
> >                         return fit_img_result;
> >                 }
> > 
> > -               if (img_len >= desc_xilinx->size) {
> > -                       name = "full";
> > -                       err = fpga_loadbitstream(devnum, (char *)img_data,
> > -                                                img_len, BIT_FULL);
> > -                       if (err)
> > -                               err = fpga_load(devnum, (const void
> > *)img_data,
> > -                                               img_len, BIT_FULL);
> > -               } else {
> > -                       name = "partial";
> > -                       err = fpga_loadbitstream(devnum, (char *)img_data,
> > -                                                img_len, BIT_PARTIAL);
> > -                       if (err)
> > -                               err = fpga_load(devnum, (const void
> > *)img_data,
> > -                                               img_len, BIT_PARTIAL);
> > +               /* Get FPGA device number, defaults to 0 */
> > +               fit_image_fpga_get_devnum(buf, conf_noffset, &devnum);
> > +
> > +               /* Check bitstream type */
> > +               if (fit_image_fpga_is_partial(buf, conf_noffset))
> > +                       bstype = BIT_PARTIAL;
> > +
> > +               /* Legacy support detecting partial config files for Xilinx
> > */
> > +               desc = fpga_get_desc(devnum);
> > +               if (desc->devtype == fpga_xilinx) {
> 
> Can we use FPGA_XILINX?

This enum type is defined in fpga.h and is lower case there as well.

> > 
> > +                       desc_xilinx = desc->devdesc;
> > +                       if (img_len < desc_xilinx->size)
> > +                               bstype = BIT_PARTIAL;
> >                 }
> > 
> > +               /* Try bitstream format first */
> > +               err = fpga_loadbitstream(devnum, (char *)img_data,
> > +                                        img_len, bstype);
> > +               if (err)
> > +                       err = fpga_load(devnum, (const void *)img_data,
> > +                                       img_len, bstype);
> > +
> >                 if (err)
> >                         return err;
> > 
> > -               printf("   Programming %s bitstream... OK\n", name);
> > +               if (bstype == BIT_PARTIAL)
> > +                       name = "partial";
> > +               else
> > +                       name = "full";
> > +
> > +               printf("   Programming %s bitstream into fpga %d... OK\n",
> > +                      name, devnum);
> >                 break;
> >         default:
> >                 printf("The given image format is not supported
> > (corrupt?)\n");
> > diff --git a/include/image.h b/include/image.h
> > index 1e686b7..75d2afc 100644
> > --- a/include/image.h
> > +++ b/include/image.h
> > @@ -876,6 +876,8 @@ int bootz_setup(ulong image, ulong *start, ulong *end);
> >  #define FIT_COMP_PROP          "compression"
> >  #define FIT_ENTRY_PROP         "entry"
> >  #define FIT_LOAD_PROP          "load"
> > +#define FIT_FPGA_DEVNUM_PROP   "fpga-devnum"
> > +#define FIT_FPGA_PARTIAL_PROP  "fpga-partial-image"
> > 
> >  /* configuration node */
> >  #define FIT_KERNEL_PROP                "kernel"
> > @@ -955,6 +957,9 @@ int fit_image_hash_get_value(const void *fit, int
> > noffset, uint8_t **value,
> > 
> >  int fit_set_timestamp(void *fit, int noffset, time_t timestamp);
> > 
> > +int fit_image_fpga_get_devnum(const void *fit, int noffset, int *devnum);
> > +int fit_image_fpga_is_partial(const void *fit, int noffset);
> 
> Can you put the function comments here instead of in the C file?

sure thing.

Thanks for the feedback.

--dalon

> > 
> > +
> >  /**
> >   * fit_add_verification_data() - add verification data to FIT image nodes
> >   *
> > --
> > 2.7.4
> > 
> > _______________________________________________
> > U-Boot mailing list
> > U-Boot at lists.denx.de
> > http://lists.denx.de/mailman/listinfo/u-boot
> 
> Regards,
> Simon


More information about the U-Boot mailing list