On 20.2.2017 15:30, Dalon Westergreen wrote:
On Mon, 2017-02-20 at 10:22 +0100, Michal Simek wrote:
On 19.2.2017 21:58, 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 dwesterg@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.
We did that for zynq when we did that for the first time. But not for zynqmp. Zynq is maybe still using it but it shouldn't now. It is not 100% reliable way. Definitely having DT property is the best option because you can add sort of "nop" which extend bitstream size and does nothing which breaks that checking.
For full u-boot there is loadb, loadbp, load and loadp to distinguish it.
That brings up an interesting point, right now the fpga_loadbitstream doesn't follow the same method as fpga_load for allowing multiple FPGA types to be supported simultaneously. Would it not be prudent to move in that direction? I believe only xilinx implements this right now.
loadbitstream is there for a long time which is in general just header bitstream header parser written in pretty bad way. Right now we are adding support for encrypted and authenticated images and for that we are adding fpga loads command where you specify by flag if you want to pass encrypted or authenticated image and keys.
There is also loadmk which should be improved a lot exactly by what you are trying to do. It means we should wrap bitstreams with flags which says what it is inside. On the other hand having separate commands it is easy for testing.
Definitely I am open to improve this subsystem to make it more flexible.
Thanks, Michal