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

Marek Vasut marex at denx.de
Fri Apr 21 13:31:42 UTC 2017


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 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.

We do the IOCSR configuration on C5/A5 too, so what's the difference ?

>  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
> 


-- 
Best regards,
Marek Vasut


More information about the U-Boot mailing list