On 02/19/2017 09:43 PM, Dalon Westergreen wrote: > On Sun, 2017-02-19 at 21:07 +0100, Marek Vasut wrote: >> On 02/19/2017 08:49 PM, 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 <dwest...@gmail.com> >> >> +CC Xilinx friends :) >> >>> >>> --- >>> common/image.c | 37 ++++++++++++++++++++++--------------- >>> 1 file changed, 22 insertions(+), 15 deletions(-) >>> >>> diff --git a/common/image.c b/common/image.c >>> index 0f88984..792d371 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) >>> { >>> @@ -1318,7 +1318,8 @@ int boot_get_fpga(int argc, char * const argv[], >>> bootm_headers_t *images, >>> 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; >>> + xilinx_desc *desc_xilinx; >>> + bitstream_type bstype; >>> >>> /* Check to see if the images struct has a FIT configuration */ >>> if (!genimg_has_config(images)) { >>> @@ -1365,22 +1366,28 @@ int boot_get_fpga(int argc, char * const argv[], >>> bootm_headers_t *images, >>> return fit_img_result; >>> } >>> >>> - if (img_len >= desc_xilinx->size) { >>> + switch (desc->devtype) { >> >> Do we need the switch statement at all ? We can have full configuration >> as a default mode of operation and have something like >> >> if (xilinx) { >> if (partial reconfiguration) { >> do_special_setup(); >> } >> } > > I only did the switch stuff b/c i envisioned a need for partial image > support for socfpga.
That'd be seriously cool :) > That said, i would suggest, as you mention, moving > this to platform specific code and perhaps an indication of the image type > in the fitimage. driver-specific code . It doesn't need to know the imagetype, just that the blob that you passed in is a partial-reconfiguration blob. I never really worked with P/R though, do you need some other metadata for that or is it contained in that P/R bitstream blob already ? >> >> But even better would be to move this platform-dependent stuff into >> drivers/fpga/ or somewhere there. This is common code, so it shouldn't >> be here in the first place. > > My preference would be to only call fpga_load and have the platform s/platform/driver/ > specific stuff figure out what they want to do. Agreed > My next comment would be > that perhaps it is best to add an fpgap type or some such in the fitimage > to specify the image is a partial image rather than looking at the image > size? Hmmmm, see my question above. If the driver cannot discern it from the blob, maybe a DT-alike property would work. ie. the image entry would also have a "xlnx,partial-reconfiguration" property or somesuch . > Also a consideration is that there should be a means of specifying the fpga > devnum somehow in the fitimage? it is plausible that a system could have > multiple fpgas, no? Hmmmm, yeeeeees, system with multiple FPGAs, I like that :-) Probably a similar DT prop in the fitimage indicating where this bitstream goes would do, like loadaddress for kernel, it'd be FPGA devid for bitstream. > --dalon > >>> >>> + case fpga_xilinx: >>> + desc_xilinx = desc->devdesc; >>> + if (img_len >= desc_xilinx->size) { >>> + name = "full"; >>> + bstype = BIT_FULL; >>> + } else { >>> + name = "partial"; >>> + bstype = BIT_PARTIAL; >>> + } >>> + break; >>> + default: >>> 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); >>> + bstype = BIT_FULL; >>> } >>> >>> + 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; >>> >>> >> >> -- Best regards, Marek Vasut _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot