[U-Boot] [PATCH v3 0/2] common: fitimage support for arbitrary fpga type
The intent of these patches is to modify existing fitimage support for fpga configuration to allow configuration of any fpga type supported by the fpga_load command.
In the fpga node, two additional optional parameters are used to indicate the fpga device number (for systems with multiple fpgas) and whether the fpga image is a full or partial image.
fpga-devnum: FPGA device number, defaults to 0 fpga-partial-image: 0 = full, other = partial, defaults to full
Changes in v3: - Fix typos and caps in comments Changes in v2: - Add support for devnum and partial - for backward compatibility, do check of image size for xilinx to determine if the image is a partial image
Dalon Westergreen (2): common: image: update boot_get_fpga to support arbitrary fpga image common: bootm: add support for arbitrary fgpa configuration
common/bootm.c | 2 +- common/image-fit.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++ common/image.c | 51 ++++++++++++++++++++++++++++++++------------------- include/image.h | 5 +++++ 4 files changed, 89 insertions(+), 20 deletions(-)
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.
Signed-off-by: Dalon Westergreen dwesterg@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); + if (value == NULL || len != sizeof(int)) + *devnum = 0; + else + *devnum = *value; + + return 0; +} + +/** + * fit_image_fpga_is_partial - is partial fpga + * @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 + */ +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)) + 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) { + 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); + /** * fit_add_verification_data() - add verification data to FIT image nodes *
On 20.2.2017 15: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.
Signed-off-by: Dalon Westergreen dwesterg@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);
- if (value == NULL || len != sizeof(int))
*devnum = 0;- else
*devnum = *value;- return 0;
+}
+/**
- fit_image_fpga_is_partial - is partial fpga
bitstream.
- @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- */
+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))
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) {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";elsename = "full";printf(" Programming %s bitstream into fpga %d... OK\n",name, devnum);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"
tbh I am not quite sure if we should introduce this. Please look below.
+#define FIT_FPGA_PARTIAL_PROP "fpga-partial-image"
I would suggest to use "partial-fpga-config" because this is what Alan is pushing to mainline kernel. Make no sense to use something different.
/* 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);
/**
- fit_add_verification_data() - add verification data to FIT image nodes
I have not a problem with this patch. I didn't test it yet but I will do it. It should probably go through my tree anyway.
There are some things which should be good to keep in mind. 1. DM - all fpga drivers are using legacy model and we should really consider to move that stuff to DM and use binding for it.
2. overlays. DT overlays were added recently to u-boot and we should add support for it. Then in general fpga commands can go away entirely and we will just apply overlays directly and this will require DM.
/ { fragment { target = <&fpga_region0>; __overlay__ { #address-cells = <1>; #size-cells = <1>; firmware-name = "XXXX"; partial-fpga-config; gpio_on: gpio@412c0000 { ... }; }; }; };
Here is visible that target replaces devnum.
3. maybe before overlays we can add support for simple cases as I supported now.
Definitely there is a lot of work which need to be done.
Thanks, Michal
On Mon, 2017-02-20 at 16:16 +0100, Michal Simek wrote:
On 20.2.2017 15: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.
Signed-off-by: Dalon Westergreen dwesterg@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);
- if (value == NULL || len != sizeof(int))
*devnum = 0;- else
*devnum = *value;- return 0;
+}
+/**
- fit_image_fpga_is_partial - is partial fpga
bitstream.
will do.
- @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
- */
+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))
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) {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";elsename = "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"
tbh I am not quite sure if we should introduce this. Please look below.
+#define FIT_FPGA_PARTIAL_PROP "fpga-partial-image"
I would suggest to use "partial-fpga-config" because this is what Alan is pushing to mainline kernel. Make no sense to use something different.
Good point.
/* 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);
/** * fit_add_verification_data() - add verification data to FIT image nodes *
I have not a problem with this patch. I didn't test it yet but I will do it. It should probably go through my tree anyway.
There are some things which should be good to keep in mind.
- DM - all fpga drivers are using legacy model and we should really
consider to move that stuff to DM and use binding for it.
- overlays. DT overlays were added recently to u-boot and we should add
support for it. Then in general fpga commands can go away entirely and we will just apply overlays directly and this will require DM.
/ { fragment { target = <&fpga_region0>; __overlay__ { #address-cells = <1>; #size-cells = <1>; firmware-name = "XXXX"; partial-fpga-config; gpio_on: gpio@412c0000 { ... }; }; }; };
Here is visible that target replaces devnum.
I do love overlays, but am not sure if target would replace devnum. My intent for devnum is to support multiple fpgas, not multiple fpga regions. That said, nothing says an fpga region couldnt point to fpga managers for the different fgpa devices...
I also think that even if support for overlays is implemented in uboot there is a case for the simpler method where the end os is not linux. Maybe after DM is implemented the fitimage can use target instead of devnum?
--dalon
- maybe before overlays we can add support for simple cases as I
supported now.
Definitely there is a lot of work which need to be done.
Thanks, Michal
On 20.2.2017 16:46, Dalon Westergreen wrote:
On Mon, 2017-02-20 at 16:16 +0100, Michal Simek wrote:
On 20.2.2017 15: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.
Signed-off-by: Dalon Westergreen dwesterg@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);
- if (value == NULL || len != sizeof(int))
*devnum = 0;- else
*devnum = *value;- return 0;
+}
+/**
- fit_image_fpga_is_partial - is partial fpga
bitstream.
will do.
- @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- */
+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))
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) {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";elsename = "full";printf(" Programming %s bitstream into fpga %d... OK\n",name, devnum);(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"
tbh I am not quite sure if we should introduce this. Please look below.
+#define FIT_FPGA_PARTIAL_PROP "fpga-partial-image"
I would suggest to use "partial-fpga-config" because this is what Alan is pushing to mainline kernel. Make no sense to use something different.
Good point.
/* 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);
/**
- fit_add_verification_data() - add verification data to FIT image nodes
I have not a problem with this patch. I didn't test it yet but I will do it. It should probably go through my tree anyway.
There are some things which should be good to keep in mind.
- DM - all fpga drivers are using legacy model and we should really
consider to move that stuff to DM and use binding for it.
- overlays. DT overlays were added recently to u-boot and we should add
support for it. Then in general fpga commands can go away entirely and we will just apply overlays directly and this will require DM.
/ { fragment { target = <&fpga_region0>; __overlay__ { #address-cells = <1>; #size-cells = <1>; firmware-name = "XXXX"; partial-fpga-config; gpio_on: gpio@412c0000 { ... }; }; }; };
Here is visible that target replaces devnum.
I do love overlays, but am not sure if target would replace devnum. My intent for devnum is to support multiple fpgas, not multiple fpga regions. That said, nothing says an fpga region couldnt point to fpga managers for the different fgpa devices...
right. fpga region has fpga-mgr property and in general you can use this link as target instead.
I also think that even if support for overlays is implemented in uboot there is a case for the simpler method where the end os is not linux. Maybe after DM is implemented the fitimage can use target instead of devnum?
not sure if you have system with multiple fpgas to load. Definitely it is easy to set it up. If you don't have this setup I would add this patch without devnum specified and 0 is used all the time which is common case. And let's start to look at DM where we can get that link for others fpgas.
Thanks, Michal
Hi Dalon,
On 20 February 2017 at 07:56, Dalon Westergreen dwesterg@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/
Signed-off-by: Dalon Westergreen dwesterg@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?
- @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?
- */
+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;elsereturn 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?
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";elsename = "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?
/**
- fit_add_verification_data() - add verification data to FIT image nodes
-- 2.7.4
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Regards, Simon
Hi Dalon,
On 02/22/2017 06:00 AM, Simon Glass wrote:
Hi Dalon,
On 20 February 2017 at 07:56, Dalon Westergreen dwesterg@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/
Signed-off-by: Dalon Westergreen dwesterg@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.
Repeating these function names in the decriptions could IMHO be avoided. Also please check double spacing.
- returns:
0, on devnum not found
value, on devnum found
This seems to always return 0. Should it instead of providing devnum as an argument return devnum?
- */
+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?
- @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 foundBut it seems to return 0 or 1?
- */
+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;elsereturn 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?
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";elsename = "full";printf(" Programming %s bitstream into fpga %d... OK\n",name, devnum);
At this point the bitstream is already programmed. Should the above print reflect this? Also, why the extra spacing?
BR, Tomas
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?
/**
- fit_add_verification_data() - add verification data to FIT image nodes
-- 2.7.4
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
On Wed, 2017-02-22 at 14:08 +0200, Tomas Melin wrote:
Hi Dalon,
On 02/22/2017 06:00 AM, Simon Glass wrote:
Hi Dalon,
On 20 February 2017 at 07:56, Dalon Westergreen dwesterg@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/
Signed-off-by: Dalon Westergreen dwesterg@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.
Repeating these function names in the decriptions could IMHO be avoided. Also please check double spacing.
i can remove that, i just followed what seemed like the convention in the functions surrounding this one.
- returns:
- * 0, on devnum not found
- * value, on devnum found
This seems to always return 0. Should it instead of providing devnum as an argument return devnum?
Yes, easy enough.
- */
+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?
- @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?
- */
+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?
+ 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);
At this point the bitstream is already programmed. Should the above print reflect this? Also, why the extra spacing?
I agree, frankly i just left this how it was originally. I will reorder this.
Thanks, Dalon
BR, Tomas
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?
/** * fit_add_verification_data() - add verification data to FIT image nodes * -- 2.7.4
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
On Tue, 2017-02-21 at 21:00 -0700, Simon Glass wrote:
Hi Dalon,
On 20 February 2017 at 07:56, Dalon Westergreen dwesterg@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@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@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Regards, Simon
On Tue, 2017-02-21 at 21:00 -0700, Simon Glass wrote:
Hi Dalon,
On 20 February 2017 at 07:56, Dalon Westergreen dwesterg@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.
[snip ...]
/* 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?
Doing this would be counter to every other function in the c file these are added to, so i think it best to leave as is.
--dalon
/** * fit_add_verification_data() - add verification data to FIT image nodes * -- 2.7.4
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Regards, Simon
This adds support for fpga configuration data in fitimages for any fpga device supported by fpga_load. At this point fitimages only support configuration of fpga images for fpga devnum 0.
Signed-off-by: Dalon Westergreen dwesterg@gmail.com --- common/bootm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/bootm.c b/common/bootm.c index b2c0912..4a4b47c 100644 --- a/common/bootm.c +++ b/common/bootm.c @@ -248,7 +248,7 @@ int bootm_find_images(int flag, int argc, char * const argv[]) #endif
#if IMAGE_ENABLE_FIT -#if defined(CONFIG_FPGA) && defined(CONFIG_FPGA_XILINX) +#if defined(CONFIG_FPGA) /* find bitstreams */ ret = boot_get_fpga(argc, argv, &images, IH_ARCH_DEFAULT, NULL, NULL);
participants (4)
-
Dalon Westergreen -
Michal Simek -
Simon Glass -
Tomas Melin