From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chee, Tien Fong Date: Mon, 13 May 2019 12:58:03 +0000 Subject: [U-Boot] [PATCH] ARM: socfpga: Fix FPGA bitstream loading code In-Reply-To: <260a9dd2-5dcb-4812-42b3-f65b97a669ef@denx.de> References: <20190507191922.12551-1-marex@denx.de> <3f92ec41-717e-4757-49a3-27ab4e1b8b60@gmail.com> <1557310671.10843.32.camel@intel.com> <1557374251.11379.4.camel@intel.com> <260a9dd2-5dcb-4812-42b3-f65b97a669ef@denx.de> Message-ID: <1557752283.11476.3.camel@intel.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit To: u-boot@lists.denx.de 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 > > > > > > > > > Cc: Chin Liang See > > > > > > > > > Cc: Dinh Nguyen > > > > > > > > > Cc: Simon Goldschmidt > > > > > > > > m> > > > > > > > > > Cc: Tien Fong Chee > > > > > > > > > --- > > > > > > > > >    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. >