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

[Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>]

Marek,

Am 13.05.2019 um 15:12 schrieb 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@denx.de>
>>>>>>>>>>> Cc: Chin Liang See <chin.liang.see@intel.com>
>>>>>>>>>>> Cc: Dinh Nguyen <dinguyen@kernel.org>
>>>>>>>>>>> Cc: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.co
>>>>>>>>>>> m>
>>>>>>>>>>> Cc: Tien Fong Chee <tien.fong.chee@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 ...

Have you dropped this? It's assigned to me in patchwork (I'm going 
through the list of old items assigned to me...).

Regards,
Simon