| Message ID | 20211005111324.19749-2-jorge@foundries.io |
|---|---|
| State | Superseded |
| Delegated to: | Michal Simek |
| Headers | show |
| Series | [RFC,1/2] fpga_load: pass compatible string | expand |
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> > 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? > 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). Regards, Simon
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
diff --git a/cmd/fpga.c b/cmd/fpga.c index 3fdd0b35e8..f3ed1fcdff 100644 --- a/cmd/fpga.c +++ b/cmd/fpga.c @@ -178,7 +178,7 @@ static int do_fpga_load(struct cmd_tbl *cmdtp, int flag, int argc, if (ret) return ret; - return fpga_load(dev, (void *)fpga_data, data_size, BIT_FULL); + return fpga_load(dev, (void *)fpga_data, data_size, BIT_FULL, NULL); } static int do_fpga_loadb(struct cmd_tbl *cmdtp, int flag, int argc, @@ -209,7 +209,7 @@ static int do_fpga_loadp(struct cmd_tbl *cmdtp, int flag, int argc, if (ret) return ret; - return fpga_load(dev, (void *)fpga_data, data_size, BIT_PARTIAL); + return fpga_load(dev, (void *)fpga_data, data_size, BIT_PARTIAL, NULL); } #endif @@ -315,7 +315,7 @@ static int do_fpga_loadmk(struct cmd_tbl *cmdtp, int flag, int argc, data_size = image_get_data_size(hdr); } return fpga_load(dev, (void *)data, data_size, - BIT_FULL); + BIT_FULL, NULL); } #endif #if defined(CONFIG_FIT) @@ -355,7 +355,7 @@ static int do_fpga_loadmk(struct cmd_tbl *cmdtp, int flag, int argc, return CMD_RET_FAILURE; } - return fpga_load(dev, fit_data, data_size, BIT_FULL); + return fpga_load(dev, fit_data, data_size, BIT_FULL, NULL); } #endif default: diff --git a/common/image.c b/common/image.c index e199d61a4c..97f3deda24 100644 --- a/common/image.c +++ b/common/image.c @@ -1516,14 +1516,14 @@ int boot_get_fpga(int argc, char *const argv[], bootm_headers_t *images, img_len, BIT_FULL); if (err) err = fpga_load(devnum, (const void *)img_data, - img_len, BIT_FULL); + img_len, BIT_FULL, NULL); } 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); + img_len, BIT_PARTIAL, NULL); } if (err) 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); 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,
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(-)