[U-Boot] [PATCH v2 4/5] arm: socfpga: Add intermediate driver between flash and FPGA manager

[Marek Vasut]

On 08/12/2017 10:03 AM, Chee, Tien Fong wrote:
[...]
>>> 1: It having ability to the right memory(OCRAM or SDRAM) to achieve
>>> the
>>> best FPGA programing performance.
>> Did you find significant throughput difference ?
>>
> 80% performance improvement with SDRAM.

This looks more like caches are not enabled ... sort of problem ?

>>> 2: It can determine the right size buffer for the fpga rbf without
>>> info
>>> of buffer size defined by user.
>> You mean like $filesize variable in the command prompt ?
>>
> Yeah. No filesize is required.

You can use $filesize instead of reimplementing this functionality
yourself though ?

>>> 3: It has ability to know what kind of fpga rbf type, and security
>>> type, such as peripheral, core, combined rbf, encryption and
>>> unencryption based on any fpga file user pass in .
>> Is this information used for anything ? I was under the impression
>> that
>> the user just needs to load in the correct RBF file into the FPGA.
>>
> Yeah, the driver would decode the RBF image to know what type of RBF
> user loading, and applying correct method(buffer allocation, which
> memory to use and configuration on FPGA manager) to program FPGA.

If the code needs to extract some information from the RBF to correctly
configure something in the FPGA manager, then that's where this should
go then.

>>> 4: It supports the checksum.
>> What checksum ? Can we have a generic hook into the FPGA framework ?
>>
> This checksum is to ensure integrity of RBF file after loading from
> flash into SDRAM. This can help to prevent possibility system
> instability caused by programming corrupt rbf into FPGA. So, this
> should be implemented in cff.c .

Or the FPGA manager driver .

>>> 5: support raw flash without fs.
>> This should go into common code.
>>
> raw flash is part of common codes in cff.c because it is part of
> mechanism like fs to determine how loading rbf from flash and program
> into fpga.

By common code I mean the stuff around FPGA LOADFS , so other FPGAs can
also benefit from that.

>>> 6: support the file name defined in DTS and U-boot environment
>>> variable.
>> I think you should extend the FPGA LOADFS here instead.
>>
> The peripheral rbf filename and DTS are generated from our bsp tool.
> But user can run fpga loadfs to reconfigure FPGA in U-boot console
> after i have supported it.

And why don't you rather apply some FPGA LOADFS if this property is
detected in the DT instead of reimplementing it ?

>>>
>>>>
>>>>  Also, the ifdeffery is awful and the explicit
>>>> depedence on VFAT when loading from FS is real bad.
>>>>
>>> It is because a lot functions is common to sdmmc, nand and qspi in
>>> different fs such as vfat, ubi and raw. It is unavoidable to have
>>> some
>>> ifdeffery if we want to keep the function common to all flashes and
>>> fs. 
>> Can the FPGA LOADFS be extended generically ?
>>
> Yeah, FPGA loadfs is considered when design cff.c. But, i plan to to
> support FPGA loadfs after having complete boot to U-boot console.

Does that mean the Arria10 is still not even capable of booting into
U-Boot console ?

cff.c is unlikely to hit mainline unless there's compelling argument
that it is needed . Thus far, I see most of the functionality should be
added elsewhere or is already there (fpga loadfs).

-- 
Best regards,
Marek Vasut