From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michal Simek Date: Mon, 20 Feb 2017 10:24:43 +0100 Subject: [U-Boot] [PATCH 1/2] common: image: update boot_get_fpga to support arbitrary fpga image In-Reply-To: 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> <3ce21661-ab01-ab00-ce01-3e9c3d5f57b8@denx.de> <1487539305.13957.23.camel@gmail.com> Message-ID: <0968048e-5cbe-db61-fa69-cda308974c0c@xilinx.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 19.2.2017 22:26, Marek Vasut wrote: > On 02/19/2017 10:21 PM, Dalon Westergreen wrote: >> 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 >>>>>>> >>>>>>> +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... :) > > So it's now A+I ... ? [1] ? :-) > [1] http://jisho.org/search/%E6%84%9B > > Looking forward to V2 of this , we seem to have some good stuff coming up. I can only have some thoughts about it but if you work for I you should use that I email. The same in A case and I do something for X that's why I use X email. Thanks, Michal