[U-Boot] [PATCH v6 13/16] arm: socfpga: Add SPL support for Arria 10

[Dalon Westergreen <dalon.westergreen@linux.intel.com>]

On Fri, 2017-04-21 at 15:31 +0200, Marek Vasut wrote:
> On 04/21/2017 03:17 PM, Dalon Westergreen wrote:
> > 
> > On Fri, 2017-04-21 at 14:17 +0200, Marek Vasut wrote:
> > > 
> > > On 04/21/2017 11:45 AM, Ley Foon Tan wrote:
> > > > 
> > > > 
> > > > On Fri, Apr 21, 2017 at 4:00 AM, Dalon Westergreen
> > > > <dalon.westergreen@linux.intel.com> wrote:
> > > > > 
> > > > > 
> > > > > On Thu, 2017-04-20 at 07:12 -0700, Dalon Westergreen wrote:
> > > > > > 
> > > > > > 
> > > > > > On Wed, 2017-04-19 at 23:58 -0500, Dinh Nguyen wrote:
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > On Wed, Apr 19, 2017 at 6:21 PM, Dalon Westergreen
> > > > > > > <dalon.westergreen@linux.intel.com> wrote:
> > > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > > > On Wed, 2017-04-19 at 13:54 -0700, Dalon Westergreen wrote:
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > On Wed, 2017-04-19 at 15:44 -0500, Dinh Nguyen wrote:
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > Really including Dalon
> > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > On Wed, Apr 19, 2017 at 3:26 PM, Dinh Nguyen <dinguyen@ker
> > > > > > > > > > > nel.
> > > > > > > > > > > org>
> > > > > > > > > > > wrote:
> > > > > > > > > > > CC: Dalon Westergreen
> > > > > > > > > > > 
> > > > > > > > > > > On 04/19/2017 02:49 PM, Dinh Nguyen wrote:
> > > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > On 04/19/2017 04:29 AM, Ley Foon Tan wrote:
> > > > > > > > > > > > > 
> > > > > > > > > > > > > 
> > > > > > > > > > > > > 
> > > > > > > > > > > > > 
> > > > > > > > > > > > > 
> > > > > > > > > > > > > Add SPL support for Arria 10.
> > > > > > > > > > > > > 
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > Signed-off-by: Tien Fong Chee <tien.fong.chee@
> > > > > > > > > > > > > > > > > inte
> > > > > > > > > > > > > > > > > l.com>
> > > > > > > > > > > > > > > > > Signed-off-by: Ley Foon Tan <ley.foon.tan@inte
> > > > > > > > > > > > > > > > > l.co
> > > > > > > > > > > > > > > > > m>
> > > > > > > > > > > > > ---
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > >  arch/arm/mach-socfpga/spl.c | 72
> > > > > > > > > +++++++++++++++++++++++++++++++++++++++++----
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > > 
> > > > > > > > > > > > > 
> > > > > > > > > > > > > 
> > > > > > > > > > > > > 
> > > > > > > > > > > > > 
> > > > > > > > > > > > >  1 file changed, 67 insertions(+), 5 deletions(-)
> > > > > > > > > > > > > 
> > > > > > > > > > > > > diff --git a/arch/arm/mach-socfpga/spl.c
> > > > > > > > > > > > > b/arch/arm/mach-
> > > > > > > > > > > > > socfpga/spl.c
> > > > > > > > > > > > > index 0064fc8..f4a3cdd 100644
> > > > > > > > > > > > > --- a/arch/arm/mach-socfpga/spl.c
> > > > > > > > > > > > > +++ b/arch/arm/mach-socfpga/spl.c
> > > > > > > > > > > > > @@ -19,23 +19,32 @@
> > > > > > > > > > > > >  #include <asm/arch/sdram.h>
> > > > > > > > > > > > >  #include <asm/arch/scu.h>
> > > > > > > > > > > > >  #include <asm/arch/nic301.h>
> > > > > > > > > > > > > +#include <asm/sections.h>
> > > > > > > > > > > > > +#include <fdtdec.h>
> > > > > > > > > > > > > +#include <watchdog.h>
> > > > > > > > > > > > > +#if defined(CONFIG_TARGET_SOCFPGA_ARRIA10)
> > > > > > > > > > > > > +#include <asm/arch/pinmux.h>
> > > > > > > > > > > > > +#endif
> > > > > > > > > > > > > 
> > > > > > > > > > > > >  DECLARE_GLOBAL_DATA_PTR;
> > > > > > > > > > > > > 
> > > > > > > > > > > > > +#if defined(CONFIG_TARGET_SOCFPGA_GEN5)
> > > > > > > > > > > > >  static struct pl310_regs *const pl310 =
> > > > > > > > > > > > >      (struct pl310_regs *)CONFIG_SYS_PL310_BASE;
> > > > > > > > > > > > >  static struct scu_registers *scu_regs =
> > > > > > > > > > > > >      (struct scu_registers *)SOCFPGA_MPUSCU_ADDRESS;
> > > > > > > > > > > > >  static struct nic301_registers *nic301_regs =
> > > > > > > > > > > > >      (struct nic301_registers
> > > > > > > > > > > > > *)SOCFPGA_L3REGS_ADDRESS;
> > > > > > > > > > > > > -static struct socfpga_system_manager *sysmgr_regs =
> > > > > > > > > > > > > +#endif
> > > > > > > > > > > > > +
> > > > > > > > > > > > > +static const struct socfpga_system_manager
> > > > > > > > > > > > > *sysmgr_regs =
> > > > > > > > > > > > >      (struct socfpga_system_manager
> > > > > > > > > > > > > *)SOCFPGA_SYSMGR_ADDRESS;
> > > > > > > > > > > > > 
> > > > > > > > > > > > >  u32 spl_boot_device(void)
> > > > > > > > > > > > >  {
> > > > > > > > > > > > >      const u32 bsel = readl(&sysmgr_regs->bootinfo);
> > > > > > > > > > > > > 
> > > > > > > > > > > > > -    switch (bsel & 0x7) {
> > > > > > > > > > > > > +    switch (SYSMGR_GET_BOOTINFO_BSEL(bsel)) {
> > > > > > > > > > > > >      case 0x1:       /* FPGA (HPS2FPGA Bridge) */
> > > > > > > > > > > > >              return BOOT_DEVICE_RAM;
> > > > > > > > > > > > >      case 0x2:       /* NAND Flash (1.8V) */
> > > > > > > > > > > > > @@ -68,6 +77,7 @@ u32 spl_boot_mode(const u32
> > > > > > > > > > > > > boot_device)
> > > > > > > > > > > > >  }
> > > > > > > > > > > > >  #endif
> > > > > > > > > > > > > 
> > > > > > > > > > > > > +#if defined(CONFIG_TARGET_SOCFPGA_GEN5)
> > > > > > > > > > > > >  static void socfpga_nic301_slave_ns(void)
> > > > > > > > > > > > >  {
> > > > > > > > > > > > >      writel(0x1, &nic301_regs->lwhps2fpgaregs);
> > > > > > > > > > > > > @@ -85,6 +95,7 @@ void board_init_f(ulong dummy)
> > > > > > > > > > > > >  #endif
> > > > > > > > > > > > >      unsigned long sdram_size;
> > > > > > > > > > > > >      unsigned long reg;
> > > > > > > > > > > > > +    int ret;
> > > > > > > > > > > > > 
> > > > > > > > > > > > >      /*
> > > > > > > > > > > > >       * First C code to run. Clear fake OCRAM ECC
> > > > > > > > > > > > > first as
> > > > > > > > > > > > > SBE
> > > > > > > > > > > > > @@ -117,7 +128,11 @@ void board_init_f(ulong dummy)
> > > > > > > > > > > > >      /* Put everything into reset but L4WD0. */
> > > > > > > > > > > > >      socfpga_per_reset_all();
> > > > > > > > > > > > >      /* Put FPGA bridges into reset too. */
> > > > > > > > > > > > > -    socfpga_bridges_reset(1);
> > > > > > > > > > > > > +    ret = socfpga_bridges_reset(1);
> > > > > > > > > > > > > +    if (ret) {
> > > > > > > > > > > > > +            printf("socfpga_bridges_reset() failed:
> > > > > > > > > > > > > %d\n",
> > > > > > > > > > > > > ret);
> > > > > > > > > > > > > +            hang();
> > > > > > > > > > > > > +    }
> > > > > > > > > > > > > 
> > > > > > > > > > > > >      socfpga_per_reset(SOCFPGA_RESET(SDR), 0);
> > > > > > > > > > > > >      socfpga_per_reset(SOCFPGA_RESET(UART0), 0);
> > > > > > > > > > > > > @@ -148,7 +163,11 @@ void board_init_f(ulong dummy)
> > > > > > > > > > > > > 
> > > > > > > > > > > > >      /* De-assert reset for peripherals and bridges
> > > > > > > > > > > > > based
> > > > > > > > > > > > > on
> > > > > > > > > > > > > handoff
> > > > > > > > > > > > > */
> > > > > > > > > > > > >      reset_deassert_peripherals_handoff();
> > > > > > > > > > > > > -    socfpga_bridges_reset(0);
> > > > > > > > > > > > > +    ret = socfpga_bridges_reset(0);
> > > > > > > > > > > > > +    if (ret) {
> > > > > > > > > > > > > +            printf("socfpga_bridges_reset() failed:
> > > > > > > > > > > > > %d\n",
> > > > > > > > > > > > > ret);
> > > > > > > > > > > > > +            hang();
> > > > > > > > > > > > 
> > > > > > > > > > > > If you keep this patch the way it is, this will cause
> > > > > > > > > > > > the
> > > > > > > > > > > > Atlas
> > > > > > > > > > > > board to
> > > > > > > > > > > > hang here.
> > > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > Hi Dalon,
> > > > > > > > > > > 
> > > > > > > > > > > Can you check this patch? On the Atlas board, I'm seeing
> > > > > > > > > > > the
> > > > > > > > > > > call to
> > > > > > > > > > > socfpga_bridges_reset(0) fail because
> > > > > > > > > > > fpgamgr_test_fpga_ready() is
> > > > > > > > > > > failing, because is_fpgamgr_initdone_high() is returning
> > > > > > > > > > > 0.
> > > > > > > > > > > 
> > > > > > > > > > > Dinh
> > > > > > > > > 
> > > > > > > > > i saw it the first go round.  i will test this out this
> > > > > > > > > afternoon.
> > > > > > > > > 
> > > > > > > > > --dalon
> > > > > > > > 
> > > > > > > > i see the exact same thing.  for the c5/a5 devices there is no
> > > > > > > > reason for
> > > > > > > > init
> > > > > > > > done to be high since fpga configuration isnt required to boot.
> > > > > > > 
> > > > > > > Okay, good! It doesn't happen on the C5 devkit though.
> > > > > > > 
> > > > > > > Ley Foon, did you test this on the Arria5?
> > > > > > > 
> > > > > > > Dinh
> > > > > > 
> > > > > > Odd, init done should not be high without the FPGA configured.  i
> > > > > > will
> > > > > > try it
> > > > > > on
> > > > > > the c5 board as well to try and figure out why init done is high.
> > > > > > 
> > > > > > --dalon
> > > > > 
> > > > > I ran this on the c5 socdk and noticed the same behavior as Dinh, then
> > > > > realized
> > > > > the board was setup to configure the fpga independently of the
> > > > > processor.  When
> > > > > i change the MSELs on the board to ensure the FPGA is unconfigured the
> > > > > code
> > > > > fails in the same location where init done is not high.  It is
> > > > > definitely
> > > > > an
> > > > > expected use case in c5/a5 that uboot can run without the fpga
> > > > > configured
> > > > > at
> > > > > all.
> > > > My proposal is revert back to its original code, revert
> > > > socfpga_bridges_reset() return type to void and don't check error from
> > > > fpgamgr_test_fpga_ready(). So, we can support Gen5 boot without
> > > > program FPGA.
> > > 
> > > Isn't the difference between CV/AV and A10 such that the former can boot
> > > without programmed FPGA while the later can NOT boot without programmed
> > > FPGA ? So this code is A10 specific ?
> > 
> > Yes, at a minimum in A10 the IO ring needs to be configured prior to boot.
> 
> We do the IOCSR configuration on C5/A5 too, so what's the difference ?
> 

in c5/a5 the iocsr config is done directly, the iocsr data is compiled into spl,
for a10 this is all done by the fpga bitstream, there is no direct iocsr config.

> > 
> >  Since these functions (socfpga_bridges_reset) are different for c5/a5 and
> > a10,
> > i would suggest changing the code to enable the bridges IF the fpga is
> > configured.  perhaps just:
> 
> If it works ... fine.
> 
> > 
> > ...
> >         } else
> > {                                                                
> >                 writel(0, &sysmgr_regs-
> > >iswgrp_handoff[0]);                     
> >                 writel(l3mask, &sysmgr_regs-
> > >iswgrp_handoff[1]);                
> >                                                                             
> >     
> >                 /* Check signal from FPGA.
> > */                                   
> >                 if (!fpgamgr_test_fpga_ready())
> > {                               
> >                        /* brdmodrst
> > */                                          
> >       			writel(0, &reset_manager_base-
> > >brg_mod_reset);             
> >                 }                                                           
> >     
> >                                                                             
> >  
> > ...
> > 
> > I think this actually mirrors the behavior of the 2013.01.01 fork where if
> > the
> > fpga is configured, spl enables the bridges.
> 
> Super :)
> 
> > 
> > --dalon
> > 
> 
>