On Sun, 2017-02-19 at 22:12 +0100, Marek Vasut wrote: > On 02/19/2017 09:58 PM, Dalon Westergreen wrote: > > > > On Sun, 2017-02-19 at 21:49 +0100, Marek Vasut wrote: > > > > > > 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 ? > > > > as far as i understand it, it is all in the blob. All that is needed is > > knowing > > whether the blob is a full or partial image. X seems to just use the image > > size > > to determine this, but that means having a table of all devices and their > > respective full image size. seems simpler to just specify the image type is > > partial or not in the fitimage. > > Can't you extract that info from the RBF though ? But then again, > extending the fitImage format for this would be fine IMO. > > btw is spelling the X in it's entirety somehow forbidden in A ? :-)
Ah, but you forget, we are now I... :) > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 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 . > > > > I dont think the property need be fpga vendor specific. > > Indeed. > > [...] > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot