From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dalon Westergreen Date: Fri, 21 Apr 2017 09:37:01 -0700 Subject: [U-Boot] [PATCH v6 13/16] arm: socfpga: Add SPL support for Arria 10 In-Reply-To: <3e5284c4-236b-ff69-8fe8-1c1f7c2425ce@denx.de> References: <1492594195-13370-1-git-send-email-ley.foon.tan@intel.com> <1492594195-13370-14-git-send-email-ley.foon.tan@intel.com> <2151f109-e75e-94e6-6f41-095238af6871@kernel.org> <1492635270.7030.52.camel@linux.intel.com> <1492644092.7030.127.camel@linux.intel.com> <1492697541.21251.16.camel@linux.intel.com> <1492718403.5092.9.camel@linux.intel.com> <1492780640.17796.41.camel@linux.intel.com> <3e5284c4-236b-ff69-8fe8-1c1f7c2425ce@denx.de> Message-ID: <1492792621.17796.91.camel@linux.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 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 > > > > 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 > > > > > > > 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 > > > > > > > > > > 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 > > > > > > > > > > > > > > > > inte > > > > > > > > > > > > > > > > > l.com> > > > > > > > > > > > > > > > > > Signed-off-by: Ley Foon Tan > > > > > > > > > > > > > > > > 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 > > > > > > > > > > > > >  #include > > > > > > > > > > > > >  #include > > > > > > > > > > > > > +#include > > > > > > > > > > > > > +#include > > > > > > > > > > > > > +#include > > > > > > > > > > > > > +#if defined(CONFIG_TARGET_SOCFPGA_ARRIA10) > > > > > > > > > > > > > +#include > > > > > > > > > > > > > +#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 > > > >