From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dalon Westergreen Date: Mon, 20 Feb 2017 06:30:19 -0800 Subject: [U-Boot] [PATCH 1/2] common: image: update boot_get_fpga to support arbitrary fpga image In-Reply-To: <7ed755d4-7ae0-d1c6-aeeb-8c463e6845c1@xilinx.com> References: <1487533799-28788-1-git-send-email-dwesterg@gmail.com> <1487533799-28788-2-git-send-email-dwesterg@gmail.com> <78c71e1d-999a-d9da-0326-c34e7caae3d3@denx.de> <1487537033.13957.12.camel@gmail.com> <1487537924.13957.20.camel@gmail.com> <7ed755d4-7ae0-d1c6-aeeb-8c463e6845c1@xilinx.com> Message-ID: <1487601019.6111.35.camel@gmail.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 > > > > > > > > > > +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. --dalon > Thanks, > Michal > > Thanks, > Michal >