All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 10/10] arm: socfpga: arria10: Added Arria10 critical HW initialization to spl
@ 2016-12-06  8:11 Chee Tien Fong
  2016-12-19  4:15 ` Chee, Tien Fong
  2016-12-20 15:17 ` Dinh Nguyen
  0 siblings, 2 replies; 6+ messages in thread
From: Chee Tien Fong @ 2016-12-06  8:11 UTC (permalink / raw)
  To: u-boot

From: Tien Fong Chee <tien.fong.chee@intel.com>

This patch adding the Arria10 critical hardware initialization before
enabling console print out in spl.

Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
Cc: Marek Vasut <marex@denx.de>
Cc: Dinh Nguyen <dinguyen@kernel.org>
Cc: Chin Liang See <chin.liang.see@intel.com>
Cc: Tien Fong <skywindctf@gmail.com>
---
 arch/arm/mach-socfpga/spl.c |   86 +++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 83 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-socfpga/spl.c b/arch/arm/mach-socfpga/spl.c
index fec4c7a..9375514 100644
--- a/arch/arm/mach-socfpga/spl.c
+++ b/arch/arm/mach-socfpga/spl.c
@@ -1,7 +1,7 @@
 /*
- *  Copyright (C) 2012 Altera Corporation <www.altera.com>
+ *  Copyright (C) 2012-2016 Altera Corporation <www.altera.com>
  *
- * SPDX-License-Identifier:	GPL-2.0+
+ * SPDX-License-Identifier:	GPL-2.0
  */
 
 #include <common.h>
@@ -19,22 +19,32 @@
 #include <asm/arch/sdram.h>
 #include <asm/arch/scu.h>
 #include <asm/arch/nic301.h>
+#include <asm/sections.h>
+#include <watchdog.h>
+#include <fdtdec.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);
 
+#if defined(CONFIG_TARGET_SOCFPGA_GEN5)
 	switch (bsel & 0x7) {
 	case 0x1:	/* FPGA (HPS2FPGA Bridge) */
 		return BOOT_DEVICE_RAM;
@@ -55,6 +65,24 @@ u32 spl_boot_device(void)
 		printf("Invalid boot device (bsel=%08x)!\n", bsel);
 		hang();
 	}
+#elif defined(CONFIG_TARGET_SOCFPGA_ARRIA10)
+	switch ((bsel>>12) & 0x7) {
+	case 0x1:	/* FPGA (HPS2FPGA Bridge) */
+		return BOOT_DEVICE_RAM;
+	case 0x2:	/* NAND Flash (1.8V) */
+	case 0x3:	/* NAND Flash (3.0V) */
+		return BOOT_DEVICE_NAND;
+	case 0x4:	/* SD/MMC External Transceiver (1.8V) */
+	case 0x5:	/* SD/MMC Internal Transceiver (3.0V) */
+		return BOOT_DEVICE_MMC1;
+	case 0x6:	/* QSPI Flash (1.8V) */
+	case 0x7:	/* QSPI Flash (3.0V) */
+		return BOOT_DEVICE_SPI;
+	default:
+		printf("Invalid boot device (bsel=%08x)!\n", bsel);
+		hang();
+	}
+#endif
 }
 
 #ifdef CONFIG_SPL_MMC_SUPPORT
@@ -68,6 +96,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);
@@ -182,3 +211,54 @@ void board_init_f(ulong dummy)
 	/* Configure simple malloc base pointer into RAM. */
 	gd->malloc_base = CONFIG_SYS_TEXT_BASE + (1024 * 1024);
 }
+#elif defined(CONFIG_TARGET_SOCFPGA_ARRIA10)
+void board_init_f(ulong dummy)
+{
+	memset(__bss_start, 0, __bss_end - __bss_start);
+	/*
+	 * 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();
+
+	watchdog_disable();
+
+	arch_early_init_r();
+
+#ifdef CONFIG_HW_WATCHDOG
+	/* release osc1 watchdog timer 0 from reset */
+	reset_deassert_osc1wd0();
+
+	/* reconfigure and enable the watchdog */
+	hw_watchdog_init();
+	WATCHDOG_RESET();
+#endif /* CONFIG_HW_WATCHDOG */
+
+#ifdef CONFIG_OF_CONTROL
+	/* We need to access to FDT as this stage */
+	/* FDT is at end of image */
+	gd->fdt_blob = (void *)(__bss_end);
+	/* Check whether we have a valid FDT or not. */
+	if (fdtdec_prepare_fdt()) {
+		panic("** CONFIG_OF_CONTROL defined but no FDT - please see "
+			"doc/README.fdt-control");
+	}
+#endif /* CONFIG_OF_CONTROL */
+
+	/* Initialize the timer */
+	timer_init();
+
+	/* configuring the clock based on handoff */
+	cm_basic_init(gd->fdt_blob);
+	WATCHDOG_RESET();
+
+	config_dedicated_pins(gd->fdt_blob);
+	WATCHDOG_RESET();
+
+	/* configure the Reset Manager */
+	reset_deassert_dedicated_peripherals();
+
+	/* enable console uart printing */
+	preloader_console_init();
+}
+#endif
-- 
1.7.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH 10/10] arm: socfpga: arria10: Added Arria10 critical HW initialization to spl
  2016-12-06  8:11 [U-Boot] [PATCH 10/10] arm: socfpga: arria10: Added Arria10 critical HW initialization to spl Chee Tien Fong
@ 2016-12-19  4:15 ` Chee, Tien Fong
  2016-12-19  7:48   ` Marek Vasut
  2016-12-20 14:27   ` Dinh Nguyen
  2016-12-20 15:17 ` Dinh Nguyen
  1 sibling, 2 replies; 6+ messages in thread
From: Chee, Tien Fong @ 2016-12-19  4:15 UTC (permalink / raw)
  To: u-boot

On Sel, 2016-12-06 at 16:11 +0800, Chee Tien Fong wrote:
> From: Tien Fong Chee <tien.fong.chee@intel.com>
> 
> This patch adding the Arria10 critical hardware initialization before
> enabling console print out in spl.
> 
> Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
> Cc: Marek Vasut <marex@denx.de>
> Cc: Dinh Nguyen <dinguyen@kernel.org>
> Cc: Chin Liang See <chin.liang.see@intel.com>
> Cc: Tien Fong <skywindctf@gmail.com>
> ---
> ?arch/arm/mach-socfpga/spl.c |???86
> +++++++++++++++++++++++++++++++++++++++++-
> ?1 files changed, 83 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/mach-socfpga/spl.c b/arch/arm/mach-
> socfpga/spl.c
> index fec4c7a..9375514 100644
> --- a/arch/arm/mach-socfpga/spl.c
> +++ b/arch/arm/mach-socfpga/spl.c
Any comments on this patch before i start the version 2?

> @@ -1,7 +1,7 @@
> ?/*
> - *??Copyright (C) 2012 Altera Corporation <www.altera.com>
> + *??Copyright (C) 2012-2016 Altera Corporation <www.altera.com>
> ? *
> - * SPDX-License-Identifier:	GPL-2.0+
> + * SPDX-License-Identifier:	GPL-2.0
> ? */
> ?
> ?#include <common.h>
> @@ -19,22 +19,32 @@
> ?#include <asm/arch/sdram.h>
> ?#include <asm/arch/scu.h>
> ?#include <asm/arch/nic301.h>
> +#include <asm/sections.h>
> +#include <watchdog.h>
> +#include <fdtdec.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);
> ?
> +#if defined(CONFIG_TARGET_SOCFPGA_GEN5)
> ?	switch (bsel & 0x7) {
> ?	case 0x1:	/* FPGA (HPS2FPGA Bridge) */
> ?		return BOOT_DEVICE_RAM;
> @@ -55,6 +65,24 @@ u32 spl_boot_device(void)
> ?		printf("Invalid boot device (bsel=%08x)!\n", bsel);
> ?		hang();
> ?	}
> +#elif defined(CONFIG_TARGET_SOCFPGA_ARRIA10)
> +	switch ((bsel>>12) & 0x7) {
> +	case 0x1:	/* FPGA (HPS2FPGA Bridge) */
> +		return BOOT_DEVICE_RAM;
> +	case 0x2:	/* NAND Flash (1.8V) */
> +	case 0x3:	/* NAND Flash (3.0V) */
> +		return BOOT_DEVICE_NAND;
> +	case 0x4:	/* SD/MMC External Transceiver (1.8V) */
> +	case 0x5:	/* SD/MMC Internal Transceiver (3.0V) */
> +		return BOOT_DEVICE_MMC1;
> +	case 0x6:	/* QSPI Flash (1.8V) */
> +	case 0x7:	/* QSPI Flash (3.0V) */
> +		return BOOT_DEVICE_SPI;
> +	default:
> +		printf("Invalid boot device (bsel=%08x)!\n", bsel);
> +		hang();
> +	}
> +#endif
> ?}
> ?
> ?#ifdef CONFIG_SPL_MMC_SUPPORT
> @@ -68,6 +96,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);
> @@ -182,3 +211,54 @@ void board_init_f(ulong dummy)
> ?	/* Configure simple malloc base pointer into RAM. */
> ?	gd->malloc_base = CONFIG_SYS_TEXT_BASE + (1024 * 1024);
> ?}
> +#elif defined(CONFIG_TARGET_SOCFPGA_ARRIA10)
> +void board_init_f(ulong dummy)
> +{
> +	memset(__bss_start, 0, __bss_end - __bss_start);
> +	/*
> +	?* 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();
> +
> +	watchdog_disable();
> +
> +	arch_early_init_r();
> +
> +#ifdef CONFIG_HW_WATCHDOG
> +	/* release osc1 watchdog timer 0 from reset */
> +	reset_deassert_osc1wd0();
> +
> +	/* reconfigure and enable the watchdog */
> +	hw_watchdog_init();
> +	WATCHDOG_RESET();
> +#endif /* CONFIG_HW_WATCHDOG */
> +
> +#ifdef CONFIG_OF_CONTROL
> +	/* We need to access to FDT as this stage */
> +	/* FDT is at end of image */
> +	gd->fdt_blob = (void *)(__bss_end);
> +	/* Check whether we have a valid FDT or not. */
> +	if (fdtdec_prepare_fdt()) {
> +		panic("** CONFIG_OF_CONTROL defined but no FDT -
> please see "
> +			"doc/README.fdt-control");
> +	}
> +#endif /* CONFIG_OF_CONTROL */
> +
> +	/* Initialize the timer */
> +	timer_init();
> +
> +	/* configuring the clock based on handoff */
> +	cm_basic_init(gd->fdt_blob);
> +	WATCHDOG_RESET();
> +
> +	config_dedicated_pins(gd->fdt_blob);
> +	WATCHDOG_RESET();
> +
> +	/* configure the Reset Manager */
> +	reset_deassert_dedicated_peripherals();
> +
> +	/* enable console uart printing */
> +	preloader_console_init();
> +}
> +#endif

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH 10/10] arm: socfpga: arria10: Added Arria10 critical HW initialization to spl
  2016-12-19  4:15 ` Chee, Tien Fong
@ 2016-12-19  7:48   ` Marek Vasut
  2016-12-20 14:27   ` Dinh Nguyen
  1 sibling, 0 replies; 6+ messages in thread
From: Marek Vasut @ 2016-12-19  7:48 UTC (permalink / raw)
  To: u-boot

On 12/19/2016 05:15 AM, Chee, Tien Fong wrote:
> On Sel, 2016-12-06 at 16:11 +0800, Chee Tien Fong wrote:
>> From: Tien Fong Chee <tien.fong.chee@intel.com>
>>
>> This patch adding the Arria10 critical hardware initialization before
>> enabling console print out in spl.
>>
>> Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
>> Cc: Marek Vasut <marex@denx.de>
>> Cc: Dinh Nguyen <dinguyen@kernel.org>
>> Cc: Chin Liang See <chin.liang.see@intel.com>
>> Cc: Tien Fong <skywindctf@gmail.com>
>> ---
>>  arch/arm/mach-socfpga/spl.c |   86
>> +++++++++++++++++++++++++++++++++++++++++-
>>  1 files changed, 83 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm/mach-socfpga/spl.c b/arch/arm/mach-
>> socfpga/spl.c
>> index fec4c7a..9375514 100644
>> --- a/arch/arm/mach-socfpga/spl.c
>> +++ b/arch/arm/mach-socfpga/spl.c
> Any comments on this patch before i start the version 2?

This patch will probably need changes to work with v2, so I'll review it
then.

-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH 10/10] arm: socfpga: arria10: Added Arria10 critical HW initialization to spl
  2016-12-19  4:15 ` Chee, Tien Fong
  2016-12-19  7:48   ` Marek Vasut
@ 2016-12-20 14:27   ` Dinh Nguyen
  1 sibling, 0 replies; 6+ messages in thread
From: Dinh Nguyen @ 2016-12-20 14:27 UTC (permalink / raw)
  To: u-boot



On 12/18/2016 10:15 PM, Chee, Tien Fong wrote:
> On Sel, 2016-12-06 at 16:11 +0800, Chee Tien Fong wrote:
>> From: Tien Fong Chee <tien.fong.chee@intel.com>
>>
>> This patch adding the Arria10 critical hardware initialization before
>> enabling console print out in spl.
>>
>> Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
>> Cc: Marek Vasut <marex@denx.de>
>> Cc: Dinh Nguyen <dinguyen@kernel.org>
>> Cc: Chin Liang See <chin.liang.see@intel.com>
>> Cc: Tien Fong <skywindctf@gmail.com>
>> ---
>>  arch/arm/mach-socfpga/spl.c |   86
>> +++++++++++++++++++++++++++++++++++++++++-
>>  1 files changed, 83 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm/mach-socfpga/spl.c b/arch/arm/mach-
>> socfpga/spl.c
>> index fec4c7a..9375514 100644
>> --- a/arch/arm/mach-socfpga/spl.c
>> +++ b/arch/arm/mach-socfpga/spl.c
> Any comments on this patch before i start the version 2?
>

I know that the license change will need to be reverted, but let me 
review it again.

Dinh

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH 10/10] arm: socfpga: arria10: Added Arria10 critical HW initialization to spl
  2016-12-06  8:11 [U-Boot] [PATCH 10/10] arm: socfpga: arria10: Added Arria10 critical HW initialization to spl Chee Tien Fong
  2016-12-19  4:15 ` Chee, Tien Fong
@ 2016-12-20 15:17 ` Dinh Nguyen
  2016-12-21  3:57   ` Chee, Tien Fong
  1 sibling, 1 reply; 6+ messages in thread
From: Dinh Nguyen @ 2016-12-20 15:17 UTC (permalink / raw)
  To: u-boot



On 12/06/2016 02:11 AM, Chee Tien Fong wrote:
> From: Tien Fong Chee <tien.fong.chee@intel.com>
>
> This patch adding the Arria10 critical hardware initialization before
> enabling console print out in spl.
>
> Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
> Cc: Marek Vasut <marex@denx.de>
> Cc: Dinh Nguyen <dinguyen@kernel.org>
> Cc: Chin Liang See <chin.liang.see@intel.com>
> Cc: Tien Fong <skywindctf@gmail.com>
> ---
>  arch/arm/mach-socfpga/spl.c |   86 +++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 83 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/mach-socfpga/spl.c b/arch/arm/mach-socfpga/spl.c
> index fec4c7a..9375514 100644
> --- a/arch/arm/mach-socfpga/spl.c
> +++ b/arch/arm/mach-socfpga/spl.c
> @@ -1,7 +1,7 @@
>  /*
> - *  Copyright (C) 2012 Altera Corporation <www.altera.com>
> + *  Copyright (C) 2012-2016 Altera Corporation <www.altera.com>
>   *
> - * SPDX-License-Identifier:	GPL-2.0+
> + * SPDX-License-Identifier:	GPL-2.0
>   */

Shouldn't be changing the license.

>
>  #include <common.h>
> @@ -19,22 +19,32 @@
>  #include <asm/arch/sdram.h>
>  #include <asm/arch/scu.h>
>  #include <asm/arch/nic301.h>
> +#include <asm/sections.h>
> +#include <watchdog.h>
> +#include <fdtdec.h>
> +#if defined(CONFIG_TARGET_SOCFPGA_ARRIA10)
> +#include <asm/arch/pinmux.h>
> +#endif

I don't know about all these includes, do you really need them? I don't 
see where you would need pinmux.h in this patch.

>
>  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);
>
> +#if defined(CONFIG_TARGET_SOCFPGA_GEN5)
>  	switch (bsel & 0x7) {
>  	case 0x1:	/* FPGA (HPS2FPGA Bridge) */
>  		return BOOT_DEVICE_RAM;
> @@ -55,6 +65,24 @@ u32 spl_boot_device(void)
>  		printf("Invalid boot device (bsel=%08x)!\n", bsel);
>  		hang();
>  	}
> +#elif defined(CONFIG_TARGET_SOCFPGA_ARRIA10)
> +	switch ((bsel>>12) & 0x7) {
> +	case 0x1:	/* FPGA (HPS2FPGA Bridge) */
> +		return BOOT_DEVICE_RAM;
> +	case 0x2:	/* NAND Flash (1.8V) */
> +	case 0x3:	/* NAND Flash (3.0V) */
> +		return BOOT_DEVICE_NAND;
> +	case 0x4:	/* SD/MMC External Transceiver (1.8V) */
> +	case 0x5:	/* SD/MMC Internal Transceiver (3.0V) */
> +		return BOOT_DEVICE_MMC1;
> +	case 0x6:	/* QSPI Flash (1.8V) */
> +	case 0x7:	/* QSPI Flash (3.0V) */
> +		return BOOT_DEVICE_SPI;
> +	default:
> +		printf("Invalid boot device (bsel=%08x)!\n", bsel);
> +		hang();
> +	}
> +#endif
>  }

You should just do a shift define  here, so you don't have to add all 
this extra code here. Something like

	switch ((bsel >> BOOTINFO_BSEL_SHIFT) & 0x7)

>
>  #ifdef CONFIG_SPL_MMC_SUPPORT
> @@ -68,6 +96,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);
> @@ -182,3 +211,54 @@ void board_init_f(ulong dummy)
>  	/* Configure simple malloc base pointer into RAM. */
>  	gd->malloc_base = CONFIG_SYS_TEXT_BASE + (1024 * 1024);
>  }
> +#elif defined(CONFIG_TARGET_SOCFPGA_ARRIA10)
> +void board_init_f(ulong dummy)
> +{
> +	memset(__bss_start, 0, __bss_end - __bss_start);
> +	/*
> +	 * 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();
> +
> +	watchdog_disable();

Why?

> +
> +	arch_early_init_r();

I don't think you should be calling this here.

> +
> +#ifdef CONFIG_HW_WATCHDOG
> +	/* release osc1 watchdog timer 0 from reset */
> +	reset_deassert_osc1wd0();
> +
> +	/* reconfigure and enable the watchdog */
> +	hw_watchdog_init();
> +	WATCHDOG_RESET();

Do you really need all these WATCHDOG_RESET()'s?
> +#endif /* CONFIG_HW_WATCHDOG */
> +
> +#ifdef CONFIG_OF_CONTROL

There's no need for this #ifdef. If you build for ARCH_SOCFPGA is 
OF_CONTROL selected.

> +	/* We need to access to FDT as this stage */

Why?

Dinh

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH 10/10] arm: socfpga: arria10: Added Arria10 critical HW initialization to spl
  2016-12-20 15:17 ` Dinh Nguyen
@ 2016-12-21  3:57   ` Chee, Tien Fong
  0 siblings, 0 replies; 6+ messages in thread
From: Chee, Tien Fong @ 2016-12-21  3:57 UTC (permalink / raw)
  To: u-boot

On Sel, 2016-12-20 at 09:17 -0600, Dinh Nguyen wrote:
> 
> On 12/06/2016 02:11 AM, Chee Tien Fong wrote:
> > 
> > From: Tien Fong Chee <tien.fong.chee@intel.com>
> > 
> > This patch adding the Arria10 critical hardware initialization
> > before
> > enabling console print out in spl.
> > 
> > Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
> > Cc: Marek Vasut <marex@denx.de>
> > Cc: Dinh Nguyen <dinguyen@kernel.org>
> > Cc: Chin Liang See <chin.liang.see@intel.com>
> > Cc: Tien Fong <skywindctf@gmail.com>
> > ---
> > ?arch/arm/mach-socfpga/spl.c |???86
> > +++++++++++++++++++++++++++++++++++++++++-
> > ?1 files changed, 83 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/arm/mach-socfpga/spl.c b/arch/arm/mach-
> > socfpga/spl.c
> > index fec4c7a..9375514 100644
> > --- a/arch/arm/mach-socfpga/spl.c
> > +++ b/arch/arm/mach-socfpga/spl.c
> > @@ -1,7 +1,7 @@
> > ?/*
> > - *??Copyright (C) 2012 Altera Corporation <www.altera.com>
> > + *??Copyright (C) 2012-2016 Altera Corporation <www.altera.com>
> > ? *
> > - * SPDX-License-Identifier:	GPL-2.0+
> > + * SPDX-License-Identifier:	GPL-2.0
> > ? */
> Shouldn't be changing the license.
> 
Yeah, i will revert it.
> > 
> > 
> > ?#include <common.h>
> > @@ -19,22 +19,32 @@
> > ?#include <asm/arch/sdram.h>
> > ?#include <asm/arch/scu.h>
> > ?#include <asm/arch/nic301.h>
> > +#include <asm/sections.h>
> > +#include <watchdog.h>
> > +#include <fdtdec.h>
> > +#if defined(CONFIG_TARGET_SOCFPGA_ARRIA10)
> > +#include <asm/arch/pinmux.h>
> > +#endif
> I don't know about all these includes, do you really need them? I
> don't?
> see where you would need pinmux.h in this patch.
> 
Yeah, we those header, otherwise compilation failed.
we need pinmux.h also, because there is a
function?config_dedicated_pins(gd->fdt_blob); in void
board_init_f(ulong dummy). This function would configure the dedicated
IO such as UART IO before enable print console.
> > 
> > 
> > ?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);
> > 
> > +#if defined(CONFIG_TARGET_SOCFPGA_GEN5)
> > ?	switch (bsel & 0x7) {
> > ?	case 0x1:	/* FPGA (HPS2FPGA Bridge) */
> > ?		return BOOT_DEVICE_RAM;
> > @@ -55,6 +65,24 @@ u32 spl_boot_device(void)
> > ?		printf("Invalid boot device (bsel=%08x)!\n",
> > bsel);
> > ?		hang();
> > ?	}
> > +#elif defined(CONFIG_TARGET_SOCFPGA_ARRIA10)
> > +	switch ((bsel>>12) & 0x7) {
> > +	case 0x1:	/* FPGA (HPS2FPGA Bridge) */
> > +		return BOOT_DEVICE_RAM;
> > +	case 0x2:	/* NAND Flash (1.8V) */
> > +	case 0x3:	/* NAND Flash (3.0V) */
> > +		return BOOT_DEVICE_NAND;
> > +	case 0x4:	/* SD/MMC External Transceiver (1.8V) */
> > +	case 0x5:	/* SD/MMC Internal Transceiver (3.0V) */
> > +		return BOOT_DEVICE_MMC1;
> > +	case 0x6:	/* QSPI Flash (1.8V) */
> > +	case 0x7:	/* QSPI Flash (3.0V) */
> > +		return BOOT_DEVICE_SPI;
> > +	default:
> > +		printf("Invalid boot device (bsel=%08x)!\n",
> > bsel);
> > +		hang();
> > +	}
> > +#endif
> > ?}
> You should just do a shift define??here, so you don't have to add
> all?
> this extra code here. Something like
> 
> 	switch ((bsel >> BOOTINFO_BSEL_SHIFT) & 0x7)
> 
> > 
> > 
> > ?#ifdef CONFIG_SPL_MMC_SUPPORT
> > @@ -68,6 +96,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);
> > @@ -182,3 +211,54 @@ void board_init_f(ulong dummy)
> > ?	/* Configure simple malloc base pointer into RAM. */
> > ?	gd->malloc_base = CONFIG_SYS_TEXT_BASE + (1024 * 1024);
> > ?}
> > +#elif defined(CONFIG_TARGET_SOCFPGA_ARRIA10)
> > +void board_init_f(ulong dummy)
> > +{
> > +	memset(__bss_start, 0, __bss_end - __bss_start);
> > +	/*
> > +	?* 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();
> > +
> > +	watchdog_disable();
> Why?
> 
Disabled watchdog for watchdog configuration, this cater for scenario
watchdog already starting running such as warm reset.
> > 
> > +
> > +	arch_early_init_r();
> I don't think you should be calling this here.
> 
This function contains security policies initialization(before
accessing any slaves), asserting reset to all timers(L4) before HW
configuration. This is to ensure no firewall blocking on slave, timer
in idle state after power up/warm reset.
> > 
> > +
> > +#ifdef CONFIG_HW_WATCHDOG
> > +	/* release osc1 watchdog timer 0 from reset */
> > +	reset_deassert_osc1wd0();
> > +
> > +	/* reconfigure and enable the watchdog */
> > +	hw_watchdog_init();
> > +	WATCHDOG_RESET();
> Do you really need all these WATCHDOG_RESET()'s?
> > 
> > +#endif /* CONFIG_HW_WATCHDOG */
> > +
> > +#ifdef CONFIG_OF_CONTROL
> There's no need for this #ifdef. If you build for ARCH_SOCFPGA is?
> OF_CONTROL selected.
> 
> > 
> > +	/* We need to access to FDT as this stage */
> Why?
> 
we have dedicated IO configuration and clock configuration based on
handoff data in FDT.
> Dinh

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-12-21  3:57 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-06  8:11 [U-Boot] [PATCH 10/10] arm: socfpga: arria10: Added Arria10 critical HW initialization to spl Chee Tien Fong
2016-12-19  4:15 ` Chee, Tien Fong
2016-12-19  7:48   ` Marek Vasut
2016-12-20 14:27   ` Dinh Nguyen
2016-12-20 15:17 ` Dinh Nguyen
2016-12-21  3:57   ` Chee, Tien Fong

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.