[U-Boot] [PATCH] ARM: socfpga: Fix FPGA bitstream loading code

[Marek Vasut]

On 5/13/19 2:58 PM, Chee, Tien Fong wrote:
> On Thu, 2019-05-09 at 10:34 +0200, Marek Vasut wrote:
>> On 5/9/19 5:57 AM, Chee, Tien Fong wrote:
>>>
>>> On Wed, 2019-05-08 at 14:55 +0200, Marek Vasut wrote:
>>>>
>>>> On 5/8/19 12:17 PM, Chee, Tien Fong wrote:
>>>>>
>>>>>
>>>>> On Tue, 2019-05-07 at 21:44 +0200, Marek Vasut wrote:
>>>>>>
>>>>>>
>>>>>> On 5/7/19 9:43 PM, Simon Goldschmidt wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On 07.05.19 21:41, Marek Vasut wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On 5/7/19 9:36 PM, Simon Goldschmidt wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 07.05.19 21:19, Marek Vasut wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> According to SoCFPGA Cyclone V datasheet
>>>>>>>>>> rev.2018.01.26
>>>>>>>>>> page
>>>>>>>>>> 175
>>>>>>>>>> (Chapter 5, FPGA Manager, data register) and Arria10
>>>>>>>>>> datasheet
>>>>>>>>>> rev.2017.07.22 page 211 (Chapter 5.4.1.2, FPGA
>>>>>>>>>> Manager,
>>>>>>>>>> img_data_w
>>>>>>>>>> register), the FPGA data register must be written
>>>>>>>>>> with
>>>>>>>>>> writes
>>>>>>>>>> with
>>>>>>>>>> non-incrementing address.
>>>>>>>>>>
>>>>>>>>>> The current code increments the address in 32-byte
>>>>>>>>>> bursts.
>>>>>>>>>> Fix the
>>>>>>>>>> code so it does not increment the address and writes
>>>>>>>>>> the
>>>>>>>>>> register
>>>>>>>>>> repeatedly instead. >
>>>>>>>>>> Signed-off-by: Marek Vasut <marex at denx.de>
>>>>>>>>>> Cc: Chin Liang See <chin.liang.see at intel.com>
>>>>>>>>>> Cc: Dinh Nguyen <dinguyen at kernel.org>
>>>>>>>>>> Cc: Simon Goldschmidt <simon.k.r.goldschmidt at gmail.co
>>>>>>>>>> m>
>>>>>>>>>> Cc: Tien Fong Chee <tien.fong.chee at intel.com>
>>>>>>>>>> ---
>>>>>>>>>>    drivers/fpga/socfpga.c | 3 +--
>>>>>>>>>>    1 file changed, 1 insertion(+), 2 deletions(-)
>>>>>>>>>>
>>>>>>>>>> diff --git a/drivers/fpga/socfpga.c
>>>>>>>>>> b/drivers/fpga/socfpga.c
>>>>>>>>>> index 685957626b..6ecea771ce 100644
>>>>>>>>>> --- a/drivers/fpga/socfpga.c
>>>>>>>>>> +++ b/drivers/fpga/socfpga.c
>>>>>>>>>> @@ -55,8 +55,7 @@ void fpgamgr_program_write(const
>>>>>>>>>> void
>>>>>>>>>> *rbf_data,
>>>>>>>>>> size_t rbf_size)
>>>>>>>>>>            "    cmp    %2,    #0\n"
>>>>>>>>>>            "    beq    2f\n"
>>>>>>>>>>            "1:    ldmia    %0!,    {r0-r7}\n"
>>>>>>>>>> -        "    stmia    %1!,    {r0-r7}\n"
>>>>>>>>>> -        "    sub    %1,    #32\n"
>>>>>>>>>> +        "    stmia    %1,    {r0-r7}\n"
>>>>>>>>> Iirc, stmia without the "!" still stores the registers
>>>>>>>>> to
>>>>>>>>> different
>>>>>>>>> addresses, it just does not change %1 any more if you
>>>>>>>>> leave
>>>>>>>>> away the
>>>>>>>>> "!"? So this would save on opcode, but not change
>>>>>>>>> anything?
>>>>>>>> Uh oh, you're right. Do we have a bigger problem here
>>>>>>>> then ?
>>>>>>>> Or
>>>>>>>> is the
>>>>>>>> socfpga ignoring the bottom 5 bits of this register
>>>>>>>> address ?
>>>>>>> Well, bitsream programming works for me very well (we're
>>>>>>> loading
>>>>>>> all our
>>>>>>> FGPAs in U-Boot from a FIT image), so maybe it's the
>>>>>>> documentation
>>>>>>> that
>>>>>>> has a problem?
>>>>>> That could indeed be, maybe someone on the CC list can take a
>>>>>> look
>>>>>> into
>>>>>> it and crosscheck it with internal docs ?
>>>>> I can't find any doc mention about "FPGA data must be written
>>>>> in
>>>>> non-
>>>>> incremting address", but i saw there is a description about
>>>>> configuration data is buffered in a 64 deep x 32 bits wide FIFO
>>>>> in
>>>>> the
>>>>> FPGA Manager https://www.intel.com/content/dam/www/programmable
>>>>> /us/
>>>>> en/p
>>>>> dfs/literature/hb/arria-10/a10_5v4.pdf (pg. 204)
>>>> Well yes, it's a FIFO, but is the FIFO populated by writing to a
>>>> single
>>>> non-incrementing address or are we supposed to write to
>>>> subsequent
>>>> incrementing addresses ?
>>>>
>>>>>
>>>>>
>>>>> Based on my understand through this register
>>>>> fpga_mgr_fpgamgrdata
>>>>> address map (0xFFCFE400-0xFFCFE7FF) on pg. 207 , the 256 bytes
>>>>> of
>>>>> FIFO
>>>>> buffer is mapping to above range addresses.
>>>> 0xFFCFE7FF-0xFFCFE400 = 0x400 = 1024 Bytes , not 256 . Why ?
>>> Finally, i have connected all scattered dot information from few
>>> internal docs. The register fpga_mgr_fpgamgrdata is actually a
>>> space in
>>> memory, acting like a buffer for the FPGA data. Regardless of the
>>> programming mode, data input from this buffer is translated into a
>>> 32-
>>> bit wide data path used by the configuration logic.
>> Does that mean that a write anywhere in 0xFFCFE400..0xFFCFE7FF ends
>> up
>> in the same register / FIFO ? Does that mean that whole address range
>> ignores the bottom 0x3ff MSbits ? Does it matter to which address in
>> that range the CPU writes the data or not ?
> 
> Sorry, that's all information i have. Anyway, i have already engaged
> the HW engineer in the loop, and i will update you once i have more
> details.

Thanks, let's wait and see ...

-- 
Best regards,
Marek Vasut