diff mbox

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

Message ID 1492594195-13370-14-git-send-email-ley.foon.tan@intel.com
State Superseded
Headers show

Commit Message

Ley Foon Tan April 19, 2017, 9:29 a.m. UTC
Add SPL support for Arria 10.

Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
---
 arch/arm/mach-socfpga/spl.c | 72 +++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 67 insertions(+), 5 deletions(-)

Comments

Dinh Nguyen April 19, 2017, 7:49 p.m. UTC | #1
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@intel.com>
> Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
> ---
>  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.

Dinh
Dinh Nguyen April 19, 2017, 8:26 p.m. UTC | #2
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@intel.com>
>> Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
>> ---
>>  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
Dinh Nguyen April 19, 2017, 8:44 p.m. UTC | #3
Really including Dalon

On Wed, Apr 19, 2017 at 3:26 PM, Dinh Nguyen <dinguyen@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@intel.com>
>>> Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
>>> ---
>>>  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
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> https://lists.denx.de/listinfo/u-boot
Dalon L Westergreen April 19, 2017, 8:54 p.m. UTC | #4
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@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@intel.com>
> > > > > > > > Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
> > > > ---
> > > > > > > >  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

> > > > > > > > > _______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Dalon L Westergreen April 19, 2017, 11:21 p.m. UTC | #5
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@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@intel.com>
> > > > > > > > > Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
> > > > > ---
> > > > > > 
> > > > > > > 
> > > > > > > > 
> > > > > > > > > 
> > > > > > > > >  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. 

--dalon

> 
> > 
> > > 
> > > > 
> > > > > 
> > > > > > 
> > > > > > > 
> > > > > > > > 
> > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> https://lists.denx.de/listinfo/u-boot
> 
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> https://lists.denx.de/listinfo/u-boot
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> https://lists.denx.de/listinfo/u-boot
Dinh Nguyen April 20, 2017, 4:58 a.m. UTC | #6
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@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@intel.com>
>> > > > > > > > > Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
>> > > > > ---
>> > > > > >
>> > > > > > >
>> > > > > > > >
>> > > > > > > > >
>> > > > > > > > >  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
Dalon L Westergreen April 20, 2017, 2:12 p.m. UTC | #7
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@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@intel.com>
> > > > > > > > > > > Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
> > > > > > > ---
> > > > > > > > 
> > > > > > > > 
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > >  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
Dalon L Westergreen April 20, 2017, 8 p.m. UTC | #8
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@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@intel.com>
> > > > > > > > > > > > Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
> > > > > > > > ---
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > >  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.

--dalon

> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> https://lists.denx.de/listinfo/u-boot
Ley Foon Tan April 21, 2017, 9:45 a.m. UTC | #9
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@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@intel.com>
>> > > > > > > > > > > > Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
>> > > > > > > > ---
>> > > > > > > > >
>> > > > > > > > >
>> > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > > > >
>> > > > > > > > > > >
>> > > > > > > > > > >
>> > > > > > > > > > > >
>> > > > > > > > > > > >
>> > > > > > > > > > > >
>> > > > > > > > > > > >  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.
Thanks.

Regards
Ley Foon
Marek Vasut April 21, 2017, 12:17 p.m. UTC | #10
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@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@intel.com>
>>>>>>>>>>>>>> Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
>>>>>>>>>> ---
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>  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 ?
Dalon L Westergreen April 21, 2017, 1:17 p.m. UTC | #11
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@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@inte
> > > > > > > > > > > > > > > l.com>
> > > > > > > > > > > > > > > Signed-off-by: Ley Foon Tan <ley.foon.tan@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);             
                }                                                               
                                                                             
...

I think this actually mirrors the behavior of the 2013.01.01 fork where if the
fpga is configured, spl enables the bridges.

--dalon
Marek Vasut April 21, 2017, 1:31 p.m. UTC | #12
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@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@inte
>>>>>>>>>>>>>>>> l.com>
>>>>>>>>>>>>>>>> Signed-off-by: Ley Foon Tan <ley.foon.tan@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
>
Dalon L Westergreen April 21, 2017, 4:37 p.m. UTC | #13
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
> > 
> 
>
Marek Vasut April 21, 2017, 4:47 p.m. UTC | #14
On 04/21/2017 06:37 PM, Dalon Westergreen wrote:

[...]

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

So that's why the FPGA must always be programmed with something to get
the A10 working ? I see ... thanks for clarifying.

>>>
>>>  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
>>>
>>
>>
Ley Foon Tan April 25, 2017, 12:42 a.m. UTC | #15
On Fri, Apr 21, 2017 at 9:17 PM, Dalon Westergreen
<dalon.westergreen@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@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@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@inte
>> > > > > > > > > > > > > > > l.com>
>> > > > > > > > > > > > > > > Signed-off-by: Ley Foon Tan <ley.foon.tan@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
diff mbox

Patch

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();
+	}
 
 	debug("Unfreezing/Thaw all I/O banks\n");
 	/* unfreeze / thaw all IO banks */
@@ -178,8 +197,51 @@  void board_init_f(ulong dummy)
 		hang();
 	}
 
-	socfpga_bridges_reset(1);
+	ret = socfpga_bridges_reset(1);
+	if (ret) {
+		printf("socfpga_bridges_reset() failed: %d\n", ret);
+		hang();
+	}
 
 	/* Configure simple malloc base pointer into RAM. */
 	gd->malloc_base = CONFIG_SYS_TEXT_BASE + (1024 * 1024);
 }
+#elif defined(CONFIG_TARGET_SOCFPGA_ARRIA10)
+void spl_board_init(void)
+{
+	/* configuring the clock based on handoff */
+	cm_basic_init(gd->fdt_blob);
+	WATCHDOG_RESET();
+
+	config_dedicated_pins(gd->fdt_blob);
+	WATCHDOG_RESET();
+
+	/* Release UART from reset */
+	socfpga_reset_uart(0);
+
+	/* enable console uart printing */
+	preloader_console_init();
+}
+
+void board_init_f(ulong dummy)
+{
+	/*
+	 * Configure Clock Manager to use intosc clock instead external osc to
+	 * ensure success watchdog operation. We do it as early as possible.
+	 */
+	cm_use_intosc();
+
+	socfpga_watchdog_disable();
+
+	arch_early_init_r();
+
+#ifdef CONFIG_HW_WATCHDOG
+	/* release osc1 watchdog timer 0 from reset */
+	socfpga_reset_deassert_osc1wd0();
+
+	/* reconfigure and enable the watchdog */
+	hw_watchdog_init();
+	WATCHDOG_RESET();
+#endif /* CONFIG_HW_WATCHDOG */
+}
+#endif