[U-Boot] [PATCH 1/2] common: image: update boot_get_fpga to support arbitrary fpga image

[Marek Vasut <marex@denx.de>]

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 <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.
>>
>> 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.

-- 
Best regards,
Marek Vasut