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

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

[Marek Vasut]

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.com>
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"
 		"	subs	%2,	#1\n"
 		"	bne	1b\n"
 		"2:	cmp	%3,	#0\n"
-- 
2.20.1

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

[Simon Goldschmidt]

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

Regards,
Simon

>   		"	subs	%2,	#1\n"
>   		"	bne	1b\n"
>   		"2:	cmp	%3,	#0\n"
> 

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

[Marek Vasut]

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

-- 
Best regards,
Marek Vasut

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

[Simon Goldschmidt]

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

Regards,
Simon

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

[Marek Vasut]

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

-- 
Best regards,
Marek Vasut

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

[Chee, Tien Fong]

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.com>
> > > > > 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 ?
sure. let me check.

Thanks for finding.
> 

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

[Chee, Tien Fong]

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

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.

Thanks.

Regards,
TF.
> 

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

[Marek Vasut]

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

-- 
Best regards,
Marek Vasut

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

[Chee, Tien Fong]

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

Thanks.

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

[Marek Vasut]

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

-- 
Best regards,
Marek Vasut

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

[Chee, Tien Fong]

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

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

-- 
Best regards,
Marek Vasut

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

[Simon Goldschmidt]

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

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

[Marek Vasut]

On 10/9/19 8:06 PM, Simon Goldschmidt wrote:
[...]
>>>>>>> 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...).

I don't know, sorry. Apparently there isn't enough information to decide
whether this patch is correct or not.

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

[Simon Goldschmidt]

Marek Vasut <marex@denx.de> schrieb am Mi., 9. Okt. 2019, 23:01:

> On 10/9/19 8:06 PM, Simon Goldschmidt wrote:
> [...]
> >>>>>>> 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...).
>
> I don't know, sorry. Apparently there isn't enough information to decide
> whether this patch is correct or not.
>

Right. However, since it seems to work as is, I don't think we have a real
problem.

Regards,
Simon

>

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

[Marek Vasut]

On 10/10/19 7:15 AM, Simon Goldschmidt wrote:
[...]
>>> Have you dropped this? It's assigned to me in patchwork (I'm going
>>> through the list of old items assigned to me...).
>>
>> I don't know, sorry. Apparently there isn't enough information to decide
>> whether this patch is correct or not.
>>
> 
> Right. However, since it seems to work as is, I don't think we have a real
> problem.

I think the datasheet could use clarification in that aspect. But it
might be way too late for that.

-- 
Best regards,
Marek Vasut

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

[jérémy alcim]

Hi, i think i have find the problem, but i think i doesn't have the
experience for modifie that.
on the file : drivers/fpga/ socfpga_gen5.c : line 161 : function
<fpgamgr_program_poll_initphase> :

   1. we wait fot the return of the fonction <fpgamgr_get_mode> with the
   value <FPGAMGRREGS_MODE_INITPHASE> or <FPGAMGRREGS_MODE_USERMODE>
   2. but he never comming its always the value <FPGAMGRREGS_MODE_CFGPHASE>
   then we are the timing out.

Then the fpga is on configuration mode, my question is:

   1. How to set him on user mode ?
   2. Why the configuration is not finalised ?
   3. Where i can find information for finalise it ?

I want to contribute, but im a noobies :)

Le jeu. 10 oct. 2019 à 09:48, Marek Vasut <marex@denx.de> a écrit :

> On 10/10/19 7:15 AM, Simon Goldschmidt wrote:
> [...]
> >>> Have you dropped this? It's assigned to me in patchwork (I'm going
> >>> through the list of old items assigned to me...).
> >>
> >> I don't know, sorry. Apparently there isn't enough information to decide
> >> whether this patch is correct or not.
> >>
> >
> > Right. However, since it seems to work as is, I don't think we have a
> real
> > problem.
>
> I think the datasheet could use clarification in that aspect. But it
> might be way too late for that.
>
> --
> Best regards,
> Marek Vasut
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot
>

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

[Simon Goldschmidt]

jérémy alcim <alcim.dev@gmail.com> schrieb am Fr., 29. Nov. 2019, 08:32:

> Hi, i think i have find the problem, but i think i doesn't have the
> experience for modifie that.
> on the file : drivers/fpga/ socfpga_gen5.c : line 161 : function
> <fpgamgr_program_poll_initphase> :
>
>    1. we wait fot the return of the fonction <fpgamgr_get_mode> with the
>    value <FPGAMGRREGS_MODE_INITPHASE> or <FPGAMGRREGS_MODE_USERMODE>
>    2. but he never comming its always the value
>    <FPGAMGRREGS_MODE_CFGPHASE> then we are the timing out.
>
> Then the fpga is on configuration mode, my question is:
>
>    1. How to set him on user mode ?
>    2. Why the configuration is not finalised ?
>    3. Where i can find information for finalise it ?
>
> I want to contribute, but im a noobies :)
>

Most probably, the FPGA is stuck in configuration phase because the file
you were trying to program is not a valid programming file. Unless I'm
really mistaken, this does not have anything to do with what's discussed in
this thread.

Regards,
Simon


> Le jeu. 10 oct. 2019 à 09:48, Marek Vasut <marex@denx.de> a écrit :
>
>> On 10/10/19 7:15 AM, Simon Goldschmidt wrote:
>> [...]
>> >>> Have you dropped this? It's assigned to me in patchwork (I'm going
>> >>> through the list of old items assigned to me...).
>> >>
>> >> I don't know, sorry. Apparently there isn't enough information to
>> decide
>> >> whether this patch is correct or not.
>> >>
>> >
>> > Right. However, since it seems to work as is, I don't think we have a
>> real
>> > problem.
>>
>> I think the datasheet could use clarification in that aspect. But it
>> might be way too late for that.
>>
>> --
>> Best regards,
>> Marek Vasut
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de
>> https://lists.denx.de/listinfo/u-boot
>>
>

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

[Marek Vasut]

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.com>
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"
 		"	subs	%2,	#1\n"
 		"	bne	1b\n"
 		"2:	cmp	%3,	#0\n"
-- 
2.20.1