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

[Michal Simek]

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

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.

Thanks,
Michal

Thanks,
Michal