diff mbox series

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

Message ID 20190507191922.12551-1-marex@denx.de
State Deferred
Delegated to: Simon Goldschmidt
Headers show
Series [U-Boot] ARM: socfpga: Fix FPGA bitstream loading code | expand

Commit Message

Marek Vasut May 7, 2019, 7:19 p.m. UTC
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(-)

Comments

Simon Goldschmidt May 7, 2019, 7:36 p.m. UTC | #1
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"
>
Marek Vasut May 7, 2019, 7:41 p.m. UTC | #2
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 ?
Simon Goldschmidt May 7, 2019, 7:43 p.m. UTC | #3
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
Marek Vasut May 7, 2019, 7:44 p.m. UTC | #4
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 ?
Chee, Tien Fong May 8, 2019, 3:51 a.m. UTC | #5
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.
>
Chee, Tien Fong May 8, 2019, 10:17 a.m. UTC | #6
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.
>
Marek Vasut May 8, 2019, 12:55 p.m. UTC | #7
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 ?
Chee, Tien Fong May 9, 2019, 3:57 a.m. UTC | #8
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.
Marek Vasut May 9, 2019, 8:34 a.m. UTC | #9
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 ?
Chee, Tien Fong May 13, 2019, 12:58 p.m. UTC | #10
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.
>
Marek Vasut May 13, 2019, 1:12 p.m. UTC | #11
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 ...
Simon Goldschmidt Oct. 9, 2019, 6:06 p.m. UTC | #12
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
Marek Vasut Oct. 9, 2019, 8:47 p.m. UTC | #13
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.
Simon Goldschmidt Oct. 10, 2019, 5:15 a.m. UTC | #14
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

>
Marek Vasut Oct. 10, 2019, 7:21 a.m. UTC | #15
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.
jérémy alcim Nov. 29, 2019, 7:32 a.m. UTC | #16
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@lists.denx.de
> https://lists.denx.de/listinfo/u-boot
>
Simon Goldschmidt Dec. 1, 2019, 8:02 p.m. UTC | #17
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@lists.denx.de
>> https://lists.denx.de/listinfo/u-boot
>>
>
diff mbox series

Patch

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"