On 08/14/2017 05:58 AM, Chee, Tien Fong wrote: > On Sab, 2017-08-12 at 18:49 +0200, Marek Vasut wrote: >> 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 ? >> > It is because SDRAM size large enough to store whole rbf file and > reduce significantly of the repetitive block transfer. It requires one > time block transfer from SDRAM to FPGA compare to OCRAM which require > more than that.
OK, do that as a subsequent optimization. >>> >>>> >>>>> >>>>> 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 ? >> > I'am sorry to have confused you. What i trying to say is user no longer > require to specify the file size and location when calling fpga loadfs. > So, this would keep the thing simple. The driver in cff.c would handle > all these troublesome things and deciding the best route for good > performance. OK, separate patch >>> >>>> >>>>> >>>>> 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. >> > Those functions for extracting info from RBF and configuring the FPGA > are actually come from fpga driver which i have submited as in previous > patchset. The actualy implementation is in cff.c, as i try to explain > cff.c drivers are mainly to handle all activities of loading rbf from > flashes to program FPGA. Such functionality should be generic and split between fpga loadfs and the driver. >>> >>>> >>>>> >>>>> 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 . >> > This integrity checking is implemented during RBF loading from flash to > SDRAM before calling the functions from fpga driver to program FPGA. So > cff.c which is designed for handling these operation and it should > be appropriate place for checksum. Add a checksum hook into the FPGA framework ? >>> >>>> >>>>> >>>>> 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. >> > But the raw flash to FPGA implementation is designed fully tied to > common codes in cff.c. FPGA loadfs would be acted like a wrapper to it, > so i am not sure it can be shared by other FPGA. I do not want any ad-hoc wrapper, implement it properly be extending fpga loadfs or the fpga framework. >>> >>>> >>>>> >>>>> 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 ? >> > We need to program the FPGA IO ring buffer from very early phase in SPL > before SDRAM can be initialized. So we need the peripheral rbf file > name from DTS, which is defined by user in our BSP tool. I don't think > FPGA loadfs would be available in SPL by that time. So, i think FPGA > loadfs is suitable for reprogramming FPGA in U-boot console, and > filename is passed as parameter to fpga loadfs command. Then just call the required bits from fpga loadfs during SPL . Extract them into some fpga-loadfs-common and make both fpga loadfs and your SPL code call those functions. >>> >>>> >>>>> >>>>> >>>>>> >>>>>> >>>>>> 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). >> > Please consider it, as i think that implementation of loading rbf from > any flashes to memory before programming PFGA should not be part of > FPGA driver. That's correct. > FPGA driver should contains only functionality of init > fpga and programming FPGA. cff.c is designed so the fpga loadfs can be > acted wrapper on it. And that is the part I have real problem with. Extend the common code and then use it to load whatever stuff into the FPGA instead of making common code a wrapper around ad-hoc stuff. -- Best regards, Marek Vasut _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot