From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dalon Westergreen Date: Wed, 22 Feb 2017 06:22:40 -0800 Subject: [U-Boot] [PATCH v3 1/2] common: image: update boot_get_fpga to support arbitrary fpga image In-Reply-To: References: <1487602565-6349-1-git-send-email-dwesterg@gmail.com> <1487602565-6349-2-git-send-email-dwesterg@gmail.com> Message-ID: <1487773360.782.4.camel@gmail.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Tue, 2017-02-21 at 21:00 -0700, Simon Glass wrote: > Hi Dalon, > > On 20 February 2017 at 07:56, Dalon Westergreen 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 > > > > -- > > 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