Michal,
On 09.05.2022 15:28, Oleksandr Suvorov wrote:
Hi Adrian,
On Mon, May 9, 2022 at 2:35 PM Adrian Fiergolski adrian.fiergolski@fastree3d.com wrote:
Hi Oleksandr,
On 09.05.2022 13:02, Oleksandr Suvorov wrote:
Hi Adrian,
On Wed, May 4, 2022 at 5:28 PM Adrian Fiergolski adrian.fiergolski@fastree3d.com wrote:
Hi Oleksandr,
On 03.05.2022 09:42, Michal Simek wrote:
On 4/11/22 20:00, Adrian Fiergolski wrote:
From: Oleksandr Suvorov oleksandr.suvorov@foundries.io
Introduce a function which passes an fpga compatible string from FIT images to FPGA drivers. This lets the different implementations decide how to handle it.
Some code of Jorge Ramirez-Ortiz jorge@foundries.io is reused.
Signed-off-by: Oleksandr Suvorov oleksandr.suvorov@foundries.io Tested-by: Ricardo Salveti ricardo@foundries.io Co-developed-by: Adrian Fiergolski adrian.fiergolski@fastree3d.com Signed-off-by: Adrian Fiergolski adrian.fiergolski@fastree3d.com
common/spl/spl_fit.c | 6 ++---- drivers/fpga/fpga.c | 23 ++++++++++++++++++++++- include/fpga.h | 4 ++++ 3 files changed, 28 insertions(+), 5 deletions(-)diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c index 1bbf824684..0e3c2a94b6 100644 --- a/common/spl/spl_fit.c +++ b/common/spl/spl_fit.c @@ -588,11 +588,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);
- ret = fit_fpga_load(0, (void *)fpga_image->load_addr,
fpga_image->size,
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 3b0a44b242..a306dd81f9 100644 --- a/drivers/fpga/fpga.c +++ b/drivers/fpga/fpga.c @@ -249,8 +249,23 @@ int fpga_loads(int devnum, const void *buf, size_t size, } #endif +int fit_fpga_load(int devnum, const void *buf, size_t bsize,
bitstream_type bstype, const char *compatible)+{
- fpga_desc *desc = fpga_get_desc(devnum);
this generates warning which you should fix.
- fpga_desc *desc = fpga_get_desc(devnum);
^~~~~~~~~~~~~w+../drivers/fpga/fpga.c: In function ‘fit_fpga_load’: w+../drivers/fpga/fpga.c:255:20: warning: initialization discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
As it's your patch, could you address it?
The 'compatible' field can't be set here, as fpga_get_desc returns 'const fpga_desc * const'. The descriptors table is populated (fpga_add) in board_init. At that stage, I am afraid, we don't have access to the fit image information yet to set a proper 'compatible' (would need to check the code more carefully). Moreover, IMHO it's not the cleanest way to change the 'compatible' field after the driver's initialisation.
Obviously, the "compatible" attribute belongs to an image, not to an FPGA driver. Unfortunately, I don't see an easy way to stick this attribute to an image in the current architecture. Maybe I missed that way, so I'd appreciate any help with this.
Anyway, we could come back to the FPGA driver subsystem later and rework it better and deeper.
For now, I'd only fix the warning. Are you ok with this?
I haven't seen straightforward implementation in the current architecture as well. However, it's this series of patches which introduces 'compatible', so IMHO, it should be done properly. Michal, any ideas/opinions?
I see 2 ways to keep all "compatible"-logic in spl_fit_upload_fpga():
- by extending each vendor-specific *_load() function with another
parameter, which will contain a unique type for any supported "compatible" value.
- making up a better-fit structure like "fpga_bitstream", more
specific for fpga bs only and smaller than spl_image_info, packing there a bs ptr, bs size, bs type, and bs "compatible" members. Both ways require changing the interface for all fpga load()-family functions.
Which option from Oleksandr's proposal do you think will be easier to push upstream?
Regards, Adrian