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

[Michal Simek]

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

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