[U-Boot] [PATCH v6 13/16] arm: socfpga: Add SPL support for Arria 10
Ley Foon Tan
lftan.linux at gmail.com
Tue Apr 25 00:42:01 UTC 2017
On Fri, Apr 21, 2017 at 9:17 PM, Dalon Westergreen
<dalon.westergreen at linux.intel.com> 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 at 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 at 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 at kernel.
>> > > > > > > > > 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 at inte
>> > > > > > > > > > > > > > > l.com>
>> > > > > > > > > > > > > > > Signed-off-by: Ley Foon Tan <ley.foon.tan at intel.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.
> 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:
>
> ...
> } 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);
> }
>
> ...
Code above will reset bridge if FPGA configuration is failed. So, it
should be something like this (original code):
...
} else {
writel(0, &sysmgr_regs->iswgrp_handoff[0]);
writel(l3mask, &sysmgr_regs->iswgrp_handoff[1]);
/* Check signal from FPGA. */
if (!fpgamgr_test_fpga_ready()) {
/* FPGA not ready, do nothing. */
printf("%s: FPGA not ready, aborting.\n", __func__);
return;
}
/* brdmodrst */
writel(0, &reset_manager_base->brg_mod_reset);
/* Remap the bridges into memory map */
writel(l3mask, SOCFPGA_L3REGS_ADDRESS);
}
....
>
> I think this actually mirrors the behavior of the 2013.01.01 fork where if the
> fpga is configured, spl enables the bridges.
>
Thanks
Regards
Ley Foon
More information about the U-Boot
mailing list