[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 at denx.de>
> > > > > > > Cc: Chin Liang See <chin.liang.see at intel.com>
> > > > > > > Cc: Dinh Nguyen <dinguyen at kernel.org>
> > > > > > > Cc: Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com>
> > > > > > > Cc: Tien Fong Chee <tien.fong.chee at intel.com>
> > > > > > > ---
> > > > > > >    drivers/fpga/socfpga.c | 3 +--
> > > > > > >    1 file changed, 1 insertion(+), 2 deletions(-)
> > > > > > > 
> > > > > > > diff --git a/drivers/fpga/socfpga.c
> > > > > > > b/drivers/fpga/socfpga.c
> > > > > > > index 685957626b..6ecea771ce 100644
> > > > > > > --- a/drivers/fpga/socfpga.c
> > > > > > > +++ b/drivers/fpga/socfpga.c
> > > > > > > @@ -55,8 +55,7 @@ void fpgamgr_program_write(const void
> > > > > > > *rbf_data,
> > > > > > > size_t rbf_size)
> > > > > > >            "    cmp    %2,    #0\n"
> > > > > > >            "    beq    2f\n"
> > > > > > >            "1:    ldmia    %0!,    {r0-r7}\n"
> > > > > > > -        "    stmia    %1!,    {r0-r7}\n"
> > > > > > > -        "    sub    %1,    #32\n"
> > > > > > > +        "    stmia    %1,    {r0-r7}\n"
> > > > > > Iirc, stmia without the "!" still stores the registers to
> > > > > > different
> > > > > > addresses, it just does not change %1 any more if you leave
> > > > > > away the
> > > > > > "!"? So this would save on opcode, but not change anything?
> > > > > Uh oh, you're right. Do we have a bigger problem here then ?
> > > > > Or
> > > > > is the
> > > > > socfpga ignoring the bottom 5 bits of this register address ?
> > > > Well, bitsream programming works for me very well (we're
> > > > loading
> > > > all our
> > > > FGPAs in U-Boot from a FIT image), so maybe it's the
> > > > documentation
> > > > that
> > > > has a problem?
> > > That could indeed be, maybe someone on the CC list can take a
> > > look
> > > into
> > > it and crosscheck it with internal docs ?
> > I can't find any doc mention about "FPGA data must be written in
> > non-
> > incremting address", but i saw there is a description about
> > configuration data is buffered in a 64 deep x 32 bits wide FIFO in
> > the
> > FPGA Manager https://www.intel.com/content/dam/www/programmable/us/
> > en/p
> > dfs/literature/hb/arria-10/a10_5v4.pdf (pg. 204)
> Well yes, it's a FIFO, but is the FIFO populated by writing to a
> single
> non-incrementing address or are we supposed to write to subsequent
> incrementing addresses ?
> 
> > 
> > Based on my understand through this register fpga_mgr_fpgamgrdata
> > address map (0xFFCFE400-0xFFCFE7FF) on pg. 207 , the 256 bytes of
> > FIFO
> > buffer is mapping to above range addresses.
> 0xFFCFE7FF-0xFFCFE400 = 0x400 = 1024 Bytes , not 256 . Why ?

Finally, i have connected all scattered dot information from few
internal docs. The register fpga_mgr_fpgamgrdata is actually a space in
memory, acting like a buffer for the FPGA data. Regardless of the
programming mode, data input from this buffer is translated into a 32-
bit wide data path used by the configuration logic.

Thanks.