[U-Boot] [PATCH 0/5] Get socfpga gen5 SPL working again.

[U-Boot] [PATCH 0/5] Get socfpga gen5 SPL working again.

[Simon Goldschmidt]

Socfpga gen5 SPL has been broken since moving to DM serial with
v2018.07.. Also, U-Boot console output has been broken since then.
This series fixes this and makes some related small improvements.

Simon Goldschmidt (5):
  arm: socfpga: fix SPL on gen5 after moving to DM serial
  arm: socfpga: fix device trees to work with DM serial
  arm: socfpga: cyclone5: handle debug uart
  board_init.c: fix simple malloc by storing malloc_limit
  malloc_simple: calloc: don't call memset if malloc failed

 arch/arm/dts/socfpga.dtsi                      |  1 +
 arch/arm/dts/socfpga_arria5_socdk.dts          |  1 +
 arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts     |  1 +
 arch/arm/dts/socfpga_cyclone5_de0_nano_soc.dts |  1 +
 arch/arm/dts/socfpga_cyclone5_de10_nano.dts    |  1 +
 arch/arm/dts/socfpga_cyclone5_de1_soc.dts      |  1 +
 arch/arm/dts/socfpga_cyclone5_is1.dts          |  1 +
 arch/arm/dts/socfpga_cyclone5_socdk.dts        |  1 +
 arch/arm/dts/socfpga_cyclone5_sockit.dts       |  1 +
 arch/arm/dts/socfpga_cyclone5_socrates.dts     |  6 ++++++
 arch/arm/dts/socfpga_cyclone5_sr1500.dts       |  1 +
 arch/arm/dts/socfpga_cyclone5_vining_fpga.dts  |  1 +
 arch/arm/mach-socfpga/reset_manager_gen5.c     |  5 +++--
 arch/arm/mach-socfpga/spl_gen5.c               | 16 +++++++++++++---
 common/init/board_init.c                       |  1 +
 common/malloc_simple.c                         |  3 ++-
 16 files changed, 36 insertions(+), 6 deletions(-)

-- 
2.17.1

[U-Boot] [PATCH 1/5] arm: socfpga: fix SPL on gen5 after moving to DM serial

[Simon Goldschmidt]

There were some NULL pointers dereferenced because DM was used
too early without correct initialization.

This fixes commit 73172753f4f3 ("ARM: socfpga: Convert to DM serial")

Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
---

 arch/arm/mach-socfpga/spl_gen5.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-socfpga/spl_gen5.c b/arch/arm/mach-socfpga/spl_gen5.c
index d6fe7d35af..0d5526656d 100644
--- a/arch/arm/mach-socfpga/spl_gen5.c
+++ b/arch/arm/mach-socfpga/spl_gen5.c
@@ -86,6 +86,7 @@ void board_init_f(ulong dummy)
 	const struct cm_config *cm_default_cfg = cm_get_default_config();
 	unsigned long sdram_size;
 	unsigned long reg;
+	int ret;
 
 	/*
 	 * First C code to run. Clear fake OCRAM ECC first as SBE
@@ -152,6 +153,12 @@ void board_init_f(ulong dummy)
 	/* unfreeze / thaw all IO banks */
 	sys_mgr_frzctrl_thaw_req();
 
+	ret = spl_early_init();
+	if (ret) {
+		debug("spl_early_init() failed: %d\n", ret);
+		hang();
+	}
+
 	/* enable console uart printing */
 	preloader_console_init();
 
@@ -177,7 +184,4 @@ void board_init_f(ulong dummy)
 	}
 
 	socfpga_bridges_reset(1);
-
-	/* Configure simple malloc base pointer into RAM. */
-	gd->malloc_base = CONFIG_SYS_TEXT_BASE + (1024 * 1024);
 }
-- 
2.17.1

[U-Boot] [PATCH 2/5] arm: socfpga: fix device trees to work with DM serial

[Simon Goldschmidt]

Device trees need to have the serial console device available
before relocation and require a stdout-path in chosen at least
for SPL to have a console.

Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
---

 arch/arm/dts/socfpga.dtsi                      | 1 +
 arch/arm/dts/socfpga_arria5_socdk.dts          | 1 +
 arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts     | 1 +
 arch/arm/dts/socfpga_cyclone5_de0_nano_soc.dts | 1 +
 arch/arm/dts/socfpga_cyclone5_de10_nano.dts    | 1 +
 arch/arm/dts/socfpga_cyclone5_de1_soc.dts      | 1 +
 arch/arm/dts/socfpga_cyclone5_is1.dts          | 1 +
 arch/arm/dts/socfpga_cyclone5_socdk.dts        | 1 +
 arch/arm/dts/socfpga_cyclone5_sockit.dts       | 1 +
 arch/arm/dts/socfpga_cyclone5_socrates.dts     | 6 ++++++
 arch/arm/dts/socfpga_cyclone5_sr1500.dts       | 1 +
 arch/arm/dts/socfpga_cyclone5_vining_fpga.dts  | 1 +
 12 files changed, 17 insertions(+)

diff --git a/arch/arm/dts/socfpga.dtsi b/arch/arm/dts/socfpga.dtsi
index 314449478d..0e5445cd1b 100644
--- a/arch/arm/dts/socfpga.dtsi
+++ b/arch/arm/dts/socfpga.dtsi
@@ -738,6 +738,7 @@
 			reg-io-width = <4>;
 			clocks = <&l4_sp_clk>;
 			clock-frequency = <100000000>;
+			u-boot,dm-pre-reloc;
 		};
 
 		uart1: serial1 at ffc03000 {
diff --git a/arch/arm/dts/socfpga_arria5_socdk.dts b/arch/arm/dts/socfpga_arria5_socdk.dts
index 449ba9cbb9..128f0c9762 100644
--- a/arch/arm/dts/socfpga_arria5_socdk.dts
+++ b/arch/arm/dts/socfpga_arria5_socdk.dts
@@ -11,6 +11,7 @@
 
 	chosen {
 		bootargs = "console=ttyS0,115200";
+		stdout-path = "serial0:115200n8";
 	};
 
 	memory {
diff --git a/arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts b/arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts
index aeb327dd5b..8e01a27320 100644
--- a/arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts
+++ b/arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts
@@ -11,6 +11,7 @@
 
 	chosen {
 		bootargs = "console=ttyS0,115200";
+		stdout-path = "serial0:115200n8";
 	};
 
 	aliases {
diff --git a/arch/arm/dts/socfpga_cyclone5_de0_nano_soc.dts b/arch/arm/dts/socfpga_cyclone5_de0_nano_soc.dts
index f4a98e4bb0..16b86ce631 100644
--- a/arch/arm/dts/socfpga_cyclone5_de0_nano_soc.dts
+++ b/arch/arm/dts/socfpga_cyclone5_de0_nano_soc.dts
@@ -11,6 +11,7 @@
 
 	chosen {
 		bootargs = "console=ttyS0,115200";
+		stdout-path = "serial0:115200n8";
 	};
 
 	aliases {
diff --git a/arch/arm/dts/socfpga_cyclone5_de10_nano.dts b/arch/arm/dts/socfpga_cyclone5_de10_nano.dts
index 7da2d8b043..9d40ce912e 100644
--- a/arch/arm/dts/socfpga_cyclone5_de10_nano.dts
+++ b/arch/arm/dts/socfpga_cyclone5_de10_nano.dts
@@ -13,6 +13,7 @@
 
 	chosen {
 		bootargs = "console=ttyS0,115200";
+		stdout-path = "serial0:115200n8";
 	};
 
 	aliases {
diff --git a/arch/arm/dts/socfpga_cyclone5_de1_soc.dts b/arch/arm/dts/socfpga_cyclone5_de1_soc.dts
index e6fadb4fc9..d7dd809162 100644
--- a/arch/arm/dts/socfpga_cyclone5_de1_soc.dts
+++ b/arch/arm/dts/socfpga_cyclone5_de1_soc.dts
@@ -11,6 +11,7 @@
 
 	chosen {
 		bootargs = "console=ttyS0,115200";
+		stdout-path = "serial0:115200n8";
 	};
 
 	aliases {
diff --git a/arch/arm/dts/socfpga_cyclone5_is1.dts b/arch/arm/dts/socfpga_cyclone5_is1.dts
index aa1ce2c3e2..e6306fb285 100644
--- a/arch/arm/dts/socfpga_cyclone5_is1.dts
+++ b/arch/arm/dts/socfpga_cyclone5_is1.dts
@@ -11,6 +11,7 @@
 
 	chosen {
 		bootargs = "console=ttyS0,115200";
+		stdout-path = "serial0:115200n8";
 	};
 
 	memory {
diff --git a/arch/arm/dts/socfpga_cyclone5_socdk.dts b/arch/arm/dts/socfpga_cyclone5_socdk.dts
index 55c70abb02..b24c39e1a3 100644
--- a/arch/arm/dts/socfpga_cyclone5_socdk.dts
+++ b/arch/arm/dts/socfpga_cyclone5_socdk.dts
@@ -11,6 +11,7 @@
 
 	chosen {
 		bootargs = "console=ttyS0,115200";
+		stdout-path = "serial0:115200n8";
 	};
 
 	memory {
diff --git a/arch/arm/dts/socfpga_cyclone5_sockit.dts b/arch/arm/dts/socfpga_cyclone5_sockit.dts
index 08d8356d80..734e682ed2 100644
--- a/arch/arm/dts/socfpga_cyclone5_sockit.dts
+++ b/arch/arm/dts/socfpga_cyclone5_sockit.dts
@@ -11,6 +11,7 @@
 
 	chosen {
 		bootargs = "console=ttyS0,115200";
+		stdout-path = "serial0:115200n8";
 	};
 
 	aliases {
diff --git a/arch/arm/dts/socfpga_cyclone5_socrates.dts b/arch/arm/dts/socfpga_cyclone5_socrates.dts
index 0d452ae300..7f9b48a839 100644
--- a/arch/arm/dts/socfpga_cyclone5_socrates.dts
+++ b/arch/arm/dts/socfpga_cyclone5_socrates.dts
@@ -11,6 +11,7 @@
 
 	chosen {
 		bootargs = "console=ttyS0,115200";
+		stdout-path = "serial0:115200n8";
 	};
 
 	aliases {
@@ -84,3 +85,8 @@
 	disable-over-current;
 	status = "okay";
 };
+
+&uart0 {
+	u-boot,dm-pre-reloc;
+	status = "okay";
+};
diff --git a/arch/arm/dts/socfpga_cyclone5_sr1500.dts b/arch/arm/dts/socfpga_cyclone5_sr1500.dts
index 341df7a3e7..1993ea2e81 100644
--- a/arch/arm/dts/socfpga_cyclone5_sr1500.dts
+++ b/arch/arm/dts/socfpga_cyclone5_sr1500.dts
@@ -11,6 +11,7 @@
 
 	chosen {
 		bootargs = "console=ttyS0,115200";
+		stdout-path = "serial0:115200n8";
 	};
 
 	aliases {
diff --git a/arch/arm/dts/socfpga_cyclone5_vining_fpga.dts b/arch/arm/dts/socfpga_cyclone5_vining_fpga.dts
index 7a032af3a4..27dd5e82d6 100644
--- a/arch/arm/dts/socfpga_cyclone5_vining_fpga.dts
+++ b/arch/arm/dts/socfpga_cyclone5_vining_fpga.dts
@@ -11,6 +11,7 @@
 
 	chosen {
 		bootargs = "console=ttyS0,115200";
+		stdout-path = "serial0:115200n8";
 	};
 
 	aliases {
-- 
2.17.1

[U-Boot] [PATCH 3/5] arm: socfpga: cyclone5: handle debug uart

[Simon Goldschmidt]

If CONFIG_DEBUG_UART is enabled, correctly initialize
the debug uart before console is initialized to debug
early boot problems in SPL.

This also changes a printf in reset_manager_gen5 to
a debug to prevent calling into debug uart before it
is initialized.

Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
---

 arch/arm/mach-socfpga/reset_manager_gen5.c | 5 +++--
 arch/arm/mach-socfpga/spl_gen5.c           | 6 ++++++
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-socfpga/reset_manager_gen5.c b/arch/arm/mach-socfpga/reset_manager_gen5.c
index 25baef79bc..3dfa09b742 100644
--- a/arch/arm/mach-socfpga/reset_manager_gen5.c
+++ b/arch/arm/mach-socfpga/reset_manager_gen5.c
@@ -89,8 +89,9 @@ void socfpga_bridges_reset(int enable)
 		/* Check signal from FPGA. */
 		if (!fpgamgr_test_fpga_ready()) {
 			/* FPGA not ready, do nothing. We allow system to boot
-			 * without FPGA ready. So, return 0 instead of error. */
-			printf("%s: FPGA not ready, aborting.\n", __func__);
+			 * without FPGA ready.
+			 */
+			debug("%s: FPGA not ready, aborting.\n", __func__);
 			return;
 		}
 
diff --git a/arch/arm/mach-socfpga/spl_gen5.c b/arch/arm/mach-socfpga/spl_gen5.c
index 0d5526656d..0e685f6ee5 100644
--- a/arch/arm/mach-socfpga/spl_gen5.c
+++ b/arch/arm/mach-socfpga/spl_gen5.c
@@ -20,6 +20,7 @@
 #include <asm/arch/scu.h>
 #include <asm/arch/nic301.h>
 #include <asm/sections.h>
+#include <debug_uart.h>
 #include <fdtdec.h>
 #include <watchdog.h>
 
@@ -153,6 +154,11 @@ void board_init_f(ulong dummy)
 	/* unfreeze / thaw all IO banks */
 	sys_mgr_frzctrl_thaw_req();
 
+#ifdef CONFIG_DEBUG_UART
+	socfpga_per_reset(SOCFPGA_RESET(UART0), 0);
+	debug_uart_init();
+#endif
+
 	ret = spl_early_init();
 	if (ret) {
 		debug("spl_early_init() failed: %d\n", ret);
-- 
2.17.1

[U-Boot] [PATCH 4/5] board_init.c: fix simple malloc by storing malloc_limit

[Simon Goldschmidt]

board_init_f_init_reserve() sets gd->malloc_base but does
not set gd->malloc_limit. This results in malloc_simple()
failing, so let's set this here.

Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
---

 common/init/board_init.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/common/init/board_init.c b/common/init/board_init.c
index 526fee35ff..a9b21a7111 100644
--- a/common/init/board_init.c
+++ b/common/init/board_init.c
@@ -123,6 +123,7 @@ void board_init_f_init_reserve(ulong base)
 #if CONFIG_VAL(SYS_MALLOC_F_LEN)
 	/* go down one 'early malloc arena' */
 	gd->malloc_base = base;
+	gd->malloc_limit = CONFIG_VAL(SYS_MALLOC_F_LEN);
 	/* next alloc will be higher by one 'early malloc arena' size */
 	base += CONFIG_VAL(SYS_MALLOC_F_LEN);
 #endif
-- 
2.17.1

[U-Boot] [PATCH 5/5] malloc_simple: calloc: don't call memset if malloc failed

[Simon Goldschmidt]

malloc_simple() can return 0 if out of memory. Don't call memset
from calloc() in this case but rely on the caller checking
the return value.

Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
---

 common/malloc_simple.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/common/malloc_simple.c b/common/malloc_simple.c
index c14f8b59c1..871b5444bd 100644
--- a/common/malloc_simple.c
+++ b/common/malloc_simple.c
@@ -57,7 +57,8 @@ void *calloc(size_t nmemb, size_t elem_size)
 	void *ptr;
 
 	ptr = malloc(size);
-	memset(ptr, '\0', size);
+	if (ptr)
+		memset(ptr, '\0', size);
 
 	return ptr;
 }
-- 
2.17.1

[U-Boot] [PATCH 1/5] arm: socfpga: fix SPL on gen5 after moving to DM serial

[Marek Vasut]

On 08/05/2018 09:34 PM, Simon Goldschmidt wrote:
> There were some NULL pointers dereferenced because DM was used
> too early without correct initialization.

This needs better explanation, really.

> This fixes commit 73172753f4f3 ("ARM: socfpga: Convert to DM serial")
> 
> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> ---
> 
>  arch/arm/mach-socfpga/spl_gen5.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/mach-socfpga/spl_gen5.c b/arch/arm/mach-socfpga/spl_gen5.c
> index d6fe7d35af..0d5526656d 100644
> --- a/arch/arm/mach-socfpga/spl_gen5.c
> +++ b/arch/arm/mach-socfpga/spl_gen5.c
> @@ -86,6 +86,7 @@ void board_init_f(ulong dummy)
>  	const struct cm_config *cm_default_cfg = cm_get_default_config();
>  	unsigned long sdram_size;
>  	unsigned long reg;
> +	int ret;
>  
>  	/*
>  	 * First C code to run. Clear fake OCRAM ECC first as SBE
> @@ -152,6 +153,12 @@ void board_init_f(ulong dummy)
>  	/* unfreeze / thaw all IO banks */
>  	sys_mgr_frzctrl_thaw_req();
>  
> +	ret = spl_early_init();
> +	if (ret) {
> +		debug("spl_early_init() failed: %d\n", ret);
> +		hang();
> +	}
> +
>  	/* enable console uart printing */
>  	preloader_console_init();
>  
> @@ -177,7 +184,4 @@ void board_init_f(ulong dummy)
>  	}
>  
>  	socfpga_bridges_reset(1);
> -
> -	/* Configure simple malloc base pointer into RAM. */
> -	gd->malloc_base = CONFIG_SYS_TEXT_BASE + (1024 * 1024);
>  }
> 


-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 2/5] arm: socfpga: fix device trees to work with DM serial

[Marek Vasut]

On 08/05/2018 09:34 PM, Simon Goldschmidt wrote:
> Device trees need to have the serial console device available
> before relocation and require a stdout-path in chosen at least
> for SPL to have a console.
> 
> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>

This should be upstreamed to Linux too ?

> ---
> 
>  arch/arm/dts/socfpga.dtsi                      | 1 +
>  arch/arm/dts/socfpga_arria5_socdk.dts          | 1 +
>  arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts     | 1 +
>  arch/arm/dts/socfpga_cyclone5_de0_nano_soc.dts | 1 +
>  arch/arm/dts/socfpga_cyclone5_de10_nano.dts    | 1 +
>  arch/arm/dts/socfpga_cyclone5_de1_soc.dts      | 1 +
>  arch/arm/dts/socfpga_cyclone5_is1.dts          | 1 +
>  arch/arm/dts/socfpga_cyclone5_socdk.dts        | 1 +
>  arch/arm/dts/socfpga_cyclone5_sockit.dts       | 1 +
>  arch/arm/dts/socfpga_cyclone5_socrates.dts     | 6 ++++++
>  arch/arm/dts/socfpga_cyclone5_sr1500.dts       | 1 +
>  arch/arm/dts/socfpga_cyclone5_vining_fpga.dts  | 1 +
>  12 files changed, 17 insertions(+)
> 
> diff --git a/arch/arm/dts/socfpga.dtsi b/arch/arm/dts/socfpga.dtsi
> index 314449478d..0e5445cd1b 100644
> --- a/arch/arm/dts/socfpga.dtsi
> +++ b/arch/arm/dts/socfpga.dtsi
> @@ -738,6 +738,7 @@
>  			reg-io-width = <4>;
>  			clocks = <&l4_sp_clk>;
>  			clock-frequency = <100000000>;
> +			u-boot,dm-pre-reloc;
>  		};
>  
>  		uart1: serial1 at ffc03000 {
> diff --git a/arch/arm/dts/socfpga_arria5_socdk.dts b/arch/arm/dts/socfpga_arria5_socdk.dts
> index 449ba9cbb9..128f0c9762 100644
> --- a/arch/arm/dts/socfpga_arria5_socdk.dts
> +++ b/arch/arm/dts/socfpga_arria5_socdk.dts
> @@ -11,6 +11,7 @@
>  
>  	chosen {
>  		bootargs = "console=ttyS0,115200";
> +		stdout-path = "serial0:115200n8";
>  	};
>  
>  	memory {
> diff --git a/arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts b/arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts
> index aeb327dd5b..8e01a27320 100644
> --- a/arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts
> +++ b/arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts
> @@ -11,6 +11,7 @@
>  
>  	chosen {
>  		bootargs = "console=ttyS0,115200";
> +		stdout-path = "serial0:115200n8";
>  	};
>  
>  	aliases {
> diff --git a/arch/arm/dts/socfpga_cyclone5_de0_nano_soc.dts b/arch/arm/dts/socfpga_cyclone5_de0_nano_soc.dts
> index f4a98e4bb0..16b86ce631 100644
> --- a/arch/arm/dts/socfpga_cyclone5_de0_nano_soc.dts
> +++ b/arch/arm/dts/socfpga_cyclone5_de0_nano_soc.dts
> @@ -11,6 +11,7 @@
>  
>  	chosen {
>  		bootargs = "console=ttyS0,115200";
> +		stdout-path = "serial0:115200n8";
>  	};
>  
>  	aliases {
> diff --git a/arch/arm/dts/socfpga_cyclone5_de10_nano.dts b/arch/arm/dts/socfpga_cyclone5_de10_nano.dts
> index 7da2d8b043..9d40ce912e 100644
> --- a/arch/arm/dts/socfpga_cyclone5_de10_nano.dts
> +++ b/arch/arm/dts/socfpga_cyclone5_de10_nano.dts
> @@ -13,6 +13,7 @@
>  
>  	chosen {
>  		bootargs = "console=ttyS0,115200";
> +		stdout-path = "serial0:115200n8";
>  	};
>  
>  	aliases {
> diff --git a/arch/arm/dts/socfpga_cyclone5_de1_soc.dts b/arch/arm/dts/socfpga_cyclone5_de1_soc.dts
> index e6fadb4fc9..d7dd809162 100644
> --- a/arch/arm/dts/socfpga_cyclone5_de1_soc.dts
> +++ b/arch/arm/dts/socfpga_cyclone5_de1_soc.dts
> @@ -11,6 +11,7 @@
>  
>  	chosen {
>  		bootargs = "console=ttyS0,115200";
> +		stdout-path = "serial0:115200n8";
>  	};
>  
>  	aliases {
> diff --git a/arch/arm/dts/socfpga_cyclone5_is1.dts b/arch/arm/dts/socfpga_cyclone5_is1.dts
> index aa1ce2c3e2..e6306fb285 100644
> --- a/arch/arm/dts/socfpga_cyclone5_is1.dts
> +++ b/arch/arm/dts/socfpga_cyclone5_is1.dts
> @@ -11,6 +11,7 @@
>  
>  	chosen {
>  		bootargs = "console=ttyS0,115200";
> +		stdout-path = "serial0:115200n8";
>  	};
>  
>  	memory {
> diff --git a/arch/arm/dts/socfpga_cyclone5_socdk.dts b/arch/arm/dts/socfpga_cyclone5_socdk.dts
> index 55c70abb02..b24c39e1a3 100644
> --- a/arch/arm/dts/socfpga_cyclone5_socdk.dts
> +++ b/arch/arm/dts/socfpga_cyclone5_socdk.dts
> @@ -11,6 +11,7 @@
>  
>  	chosen {
>  		bootargs = "console=ttyS0,115200";
> +		stdout-path = "serial0:115200n8";
>  	};
>  
>  	memory {
> diff --git a/arch/arm/dts/socfpga_cyclone5_sockit.dts b/arch/arm/dts/socfpga_cyclone5_sockit.dts
> index 08d8356d80..734e682ed2 100644
> --- a/arch/arm/dts/socfpga_cyclone5_sockit.dts
> +++ b/arch/arm/dts/socfpga_cyclone5_sockit.dts
> @@ -11,6 +11,7 @@
>  
>  	chosen {
>  		bootargs = "console=ttyS0,115200";
> +		stdout-path = "serial0:115200n8";
>  	};
>  
>  	aliases {
> diff --git a/arch/arm/dts/socfpga_cyclone5_socrates.dts b/arch/arm/dts/socfpga_cyclone5_socrates.dts
> index 0d452ae300..7f9b48a839 100644
> --- a/arch/arm/dts/socfpga_cyclone5_socrates.dts
> +++ b/arch/arm/dts/socfpga_cyclone5_socrates.dts
> @@ -11,6 +11,7 @@
>  
>  	chosen {
>  		bootargs = "console=ttyS0,115200";
> +		stdout-path = "serial0:115200n8";
>  	};
>  
>  	aliases {
> @@ -84,3 +85,8 @@
>  	disable-over-current;
>  	status = "okay";
>  };
> +
> +&uart0 {
> +	u-boot,dm-pre-reloc;
> +	status = "okay";
> +};
> diff --git a/arch/arm/dts/socfpga_cyclone5_sr1500.dts b/arch/arm/dts/socfpga_cyclone5_sr1500.dts
> index 341df7a3e7..1993ea2e81 100644
> --- a/arch/arm/dts/socfpga_cyclone5_sr1500.dts
> +++ b/arch/arm/dts/socfpga_cyclone5_sr1500.dts
> @@ -11,6 +11,7 @@
>  
>  	chosen {
>  		bootargs = "console=ttyS0,115200";
> +		stdout-path = "serial0:115200n8";
>  	};
>  
>  	aliases {
> diff --git a/arch/arm/dts/socfpga_cyclone5_vining_fpga.dts b/arch/arm/dts/socfpga_cyclone5_vining_fpga.dts
> index 7a032af3a4..27dd5e82d6 100644
> --- a/arch/arm/dts/socfpga_cyclone5_vining_fpga.dts
> +++ b/arch/arm/dts/socfpga_cyclone5_vining_fpga.dts
> @@ -11,6 +11,7 @@
>  
>  	chosen {
>  		bootargs = "console=ttyS0,115200";
> +		stdout-path = "serial0:115200n8";
>  	};
>  
>  	aliases {
> 


-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 3/5] arm: socfpga: cyclone5: handle debug uart

[Marek Vasut]

On 08/05/2018 09:34 PM, Simon Goldschmidt wrote:
> If CONFIG_DEBUG_UART is enabled, correctly initialize
> the debug uart before console is initialized to debug
> early boot problems in SPL.
> 
> This also changes a printf in reset_manager_gen5 to
> a debug to prevent calling into debug uart before it
> is initialized.
> 
> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> ---
> 
>  arch/arm/mach-socfpga/reset_manager_gen5.c | 5 +++--
>  arch/arm/mach-socfpga/spl_gen5.c           | 6 ++++++
>  2 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-socfpga/reset_manager_gen5.c b/arch/arm/mach-socfpga/reset_manager_gen5.c
> index 25baef79bc..3dfa09b742 100644
> --- a/arch/arm/mach-socfpga/reset_manager_gen5.c
> +++ b/arch/arm/mach-socfpga/reset_manager_gen5.c
> @@ -89,8 +89,9 @@ void socfpga_bridges_reset(int enable)
>  		/* Check signal from FPGA. */
>  		if (!fpgamgr_test_fpga_ready()) {
>  			/* FPGA not ready, do nothing. We allow system to boot
> -			 * without FPGA ready. So, return 0 instead of error. */
> -			printf("%s: FPGA not ready, aborting.\n", __func__);
> +			 * without FPGA ready.
> +			 */
> +			debug("%s: FPGA not ready, aborting.\n", __func__);

This seems to be papering over some sort of problem with the debug UART.
I'd like to keep the print here, since it's a valid error, not a debug
print.

>  			return;
>  		}
>  
> diff --git a/arch/arm/mach-socfpga/spl_gen5.c b/arch/arm/mach-socfpga/spl_gen5.c
> index 0d5526656d..0e685f6ee5 100644
> --- a/arch/arm/mach-socfpga/spl_gen5.c
> +++ b/arch/arm/mach-socfpga/spl_gen5.c
> @@ -20,6 +20,7 @@
>  #include <asm/arch/scu.h>
>  #include <asm/arch/nic301.h>
>  #include <asm/sections.h>
> +#include <debug_uart.h>
>  #include <fdtdec.h>
>  #include <watchdog.h>
>  
> @@ -153,6 +154,11 @@ void board_init_f(ulong dummy)
>  	/* unfreeze / thaw all IO banks */
>  	sys_mgr_frzctrl_thaw_req();
>  
> +#ifdef CONFIG_DEBUG_UART
> +	socfpga_per_reset(SOCFPGA_RESET(UART0), 0);
> +	debug_uart_init();
> +#endif
> +
>  	ret = spl_early_init();
>  	if (ret) {
>  		debug("spl_early_init() failed: %d\n", ret);
> 


-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 4/5] board_init.c: fix simple malloc by storing malloc_limit

[Marek Vasut]

On 08/05/2018 09:34 PM, Simon Goldschmidt wrote:
> board_init_f_init_reserve() sets gd->malloc_base but does
> not set gd->malloc_limit. This results in malloc_simple()
> failing, so let's set this here.
> 
> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> ---
> 
>  common/init/board_init.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/common/init/board_init.c b/common/init/board_init.c
> index 526fee35ff..a9b21a7111 100644
> --- a/common/init/board_init.c
> +++ b/common/init/board_init.c
> @@ -123,6 +123,7 @@ void board_init_f_init_reserve(ulong base)
>  #if CONFIG_VAL(SYS_MALLOC_F_LEN)
>  	/* go down one 'early malloc arena' */
>  	gd->malloc_base = base;
> +	gd->malloc_limit = CONFIG_VAL(SYS_MALLOC_F_LEN);
>  	/* next alloc will be higher by one 'early malloc arena' size */
>  	base += CONFIG_VAL(SYS_MALLOC_F_LEN);
>  #endif

+CC Simon, I'd like at least one A-B/R-B on this since this is quite
intrusive change.

This should be a separate patch from this series too.

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 5/5] malloc_simple: calloc: don't call memset if malloc failed

[Marek Vasut]

On 08/05/2018 09:35 PM, Simon Goldschmidt wrote:
> malloc_simple() can return 0 if out of memory. Don't call memset
> from calloc() in this case but rely on the caller checking
> the return value.
> 
> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> ---
> 
>  common/malloc_simple.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/common/malloc_simple.c b/common/malloc_simple.c
> index c14f8b59c1..871b5444bd 100644
> --- a/common/malloc_simple.c
> +++ b/common/malloc_simple.c
> @@ -57,7 +57,8 @@ void *calloc(size_t nmemb, size_t elem_size)
>  	void *ptr;
>  
>  	ptr = malloc(size);
> -	memset(ptr, '\0', size);
> +	if (ptr)
> +		memset(ptr, '\0', size);
>  
>  	return ptr;
>  }
> 
Reviewed-by: Marek Vasut <marex@denx.de>

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 4/5] board_init.c: fix simple malloc by storing malloc_limit

[Simon Goldschmidt]

Marek Vasut <marex@denx.de> schrieb am Mo., 6. Aug. 2018, 15:19:

> On 08/05/2018 09:34 PM, Simon Goldschmidt wrote:
> > board_init_f_init_reserve() sets gd->malloc_base but does
> > not set gd->malloc_limit. This results in malloc_simple()
> > failing, so let's set this here.
> >
> > Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> > ---
> >
> >  common/init/board_init.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/common/init/board_init.c b/common/init/board_init.c
> > index 526fee35ff..a9b21a7111 100644
> > --- a/common/init/board_init.c
> > +++ b/common/init/board_init.c
> > @@ -123,6 +123,7 @@ void board_init_f_init_reserve(ulong base)
> >  #if CONFIG_VAL(SYS_MALLOC_F_LEN)
> >       /* go down one 'early malloc arena' */
> >       gd->malloc_base = base;
> > +     gd->malloc_limit = CONFIG_VAL(SYS_MALLOC_F_LEN);
> >       /* next alloc will be higher by one 'early malloc arena' size */
> >       base += CONFIG_VAL(SYS_MALLOC_F_LEN);
> >  #endif
>
> +CC Simon, I'd like at least one A-B/R-B on this since this is quite
> intrusive change.
>
> This should be a separate patch from this series too.
>

Ok, after changing board_init_f() for gen5 spl to call spl_early_init(),
this should not be required for me any more. I only wanted to keep others
from running into this.

I don't rally get why this assignment is here though. Maybe it can be
removed? But I guess that's hard to tell for all boards our there...

Simon Goldschmidt

[U-Boot] [PATCH 3/5] arm: socfpga: cyclone5: handle debug uart

[Simon Goldschmidt]

Marek Vasut <marex@denx.de> schrieb am Mo., 6. Aug. 2018, 15:19:

> On 08/05/2018 09:34 PM, Simon Goldschmidt wrote:
> > If CONFIG_DEBUG_UART is enabled, correctly initialize
> > the debug uart before console is initialized to debug
> > early boot problems in SPL.
> >
> > This also changes a printf in reset_manager_gen5 to
> > a debug to prevent calling into debug uart before it
> > is initialized.
> >
> > Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> > ---
> >
> >  arch/arm/mach-socfpga/reset_manager_gen5.c | 5 +++--
> >  arch/arm/mach-socfpga/spl_gen5.c           | 6 ++++++
> >  2 files changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm/mach-socfpga/reset_manager_gen5.c
> b/arch/arm/mach-socfpga/reset_manager_gen5.c
> > index 25baef79bc..3dfa09b742 100644
> > --- a/arch/arm/mach-socfpga/reset_manager_gen5.c
> > +++ b/arch/arm/mach-socfpga/reset_manager_gen5.c
> > @@ -89,8 +89,9 @@ void socfpga_bridges_reset(int enable)
> >               /* Check signal from FPGA. */
> >               if (!fpgamgr_test_fpga_ready()) {
> >                       /* FPGA not ready, do nothing. We allow system to
> boot
> > -                      * without FPGA ready. So, return 0 instead of
> error. */
> > -                     printf("%s: FPGA not ready, aborting.\n",
> __func__);
> > +                      * without FPGA ready.
> > +                      */
> > +                     debug("%s: FPGA not ready, aborting.\n", __func__);
>
> This seems to be papering over some sort of problem with the debug UART.
>

You're right, it is papering over ns16550 debut uart staying in a tight
loop when printf is called before the debut uart is initialized.

This might be fixed by keeping & checking the init state of debut uart, but
does a global bool work for this in SPL?

I'd like to keep the print here, since it's a valid error, not a debug
> print.
>

As I see it, it's not a real error: my SPL always goes down this path as
the fpga is never initialized for me.

Note that booting from fpga doesn't work for me. That's a to-do on my
list...

Simon Goldschmidt


> >                       return;
> >               }
> >
> > diff --git a/arch/arm/mach-socfpga/spl_gen5.c
> b/arch/arm/mach-socfpga/spl_gen5.c
> > index 0d5526656d..0e685f6ee5 100644
> > --- a/arch/arm/mach-socfpga/spl_gen5.c
> > +++ b/arch/arm/mach-socfpga/spl_gen5.c
> > @@ -20,6 +20,7 @@
> >  #include <asm/arch/scu.h>
> >  #include <asm/arch/nic301.h>
> >  #include <asm/sections.h>
> > +#include <debug_uart.h>
> >  #include <fdtdec.h>
> >  #include <watchdog.h>
> >
> > @@ -153,6 +154,11 @@ void board_init_f(ulong dummy)
> >       /* unfreeze / thaw all IO banks */
> >       sys_mgr_frzctrl_thaw_req();
> >
> > +#ifdef CONFIG_DEBUG_UART
> > +     socfpga_per_reset(SOCFPGA_RESET(UART0), 0);
> > +     debug_uart_init();
> > +#endif
> > +
> >       ret = spl_early_init();
> >       if (ret) {
> >               debug("spl_early_init() failed: %d\n", ret);
> >
>
>
> --
> Best regards,
> Marek Vasut
>

[U-Boot] [PATCH 1/5] arm: socfpga: fix SPL on gen5 after moving to DM serial

[Simon Goldschmidt]

Marek Vasut <marex@denx.de> schrieb am Mo., 6. Aug. 2018, 15:18:

> On 08/05/2018 09:34 PM, Simon Goldschmidt wrote:
> > There were some NULL pointers dereferenced because DM was used
> > too early without correct initialization.
>
> This needs better explanation, really.
>

Ok.

> This fixes commit 73172753f4f3 ("ARM: socfpga: Convert to DM serial")
> >
> > Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> > ---
> >
> >  arch/arm/mach-socfpga/spl_gen5.c | 10 +++++++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/arm/mach-socfpga/spl_gen5.c
> b/arch/arm/mach-socfpga/spl_gen5.c
> > index d6fe7d35af..0d5526656d 100644
> > --- a/arch/arm/mach-socfpga/spl_gen5.c
> > +++ b/arch/arm/mach-socfpga/spl_gen5.c
> > @@ -86,6 +86,7 @@ void board_init_f(ulong dummy)
> >       const struct cm_config *cm_default_cfg = cm_get_default_config();
> >       unsigned long sdram_size;
> >       unsigned long reg;
> > +     int ret;
> >
> >       /*
> >        * First C code to run. Clear fake OCRAM ECC first as SBE
> > @@ -152,6 +153,12 @@ void board_init_f(ulong dummy)
> >       /* unfreeze / thaw all IO banks */
> >       sys_mgr_frzctrl_thaw_req();
> >
> > +     ret = spl_early_init();
> > +     if (ret) {
> > +             debug("spl_early_init() failed: %d\n", ret);
> > +             hang();
> > +     }
> > +
> >       /* enable console uart printing */
> >       preloader_console_init();
> >
> > @@ -177,7 +184,4 @@ void board_init_f(ulong dummy)
> >       }
> >
> >       socfpga_bridges_reset(1);
> > -
> > -     /* Configure simple malloc base pointer into RAM. */
> > -     gd->malloc_base = CONFIG_SYS_TEXT_BASE + (1024 * 1024);
> >  }
> >
>
>
> --
> Best regards,
> Marek Vasut
>

[U-Boot] [PATCH 2/5] arm: socfpga: fix device trees to work with DM serial

[Simon Goldschmidt]

Marek Vasut <marex@denx.de> schrieb am Mo., 6. Aug. 2018, 15:19:

> On 08/05/2018 09:34 PM, Simon Goldschmidt wrote:
> > Device trees need to have the serial console device available
> > before relocation and require a stdout-path in chosen at least
> > for SPL to have a console.
> >
> > Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
>
> This should be upstreamed to Linux too ?
>

Hmm, I'm not sure. Does Linux use the stdout-path too? I always use
bootargs only...


> > ---
> >
> >  arch/arm/dts/socfpga.dtsi                      | 1 +
> >  arch/arm/dts/socfpga_arria5_socdk.dts          | 1 +
> >  arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts     | 1 +
> >  arch/arm/dts/socfpga_cyclone5_de0_nano_soc.dts | 1 +
> >  arch/arm/dts/socfpga_cyclone5_de10_nano.dts    | 1 +
> >  arch/arm/dts/socfpga_cyclone5_de1_soc.dts      | 1 +
> >  arch/arm/dts/socfpga_cyclone5_is1.dts          | 1 +
> >  arch/arm/dts/socfpga_cyclone5_socdk.dts        | 1 +
> >  arch/arm/dts/socfpga_cyclone5_sockit.dts       | 1 +
> >  arch/arm/dts/socfpga_cyclone5_socrates.dts     | 6 ++++++
> >  arch/arm/dts/socfpga_cyclone5_sr1500.dts       | 1 +
> >  arch/arm/dts/socfpga_cyclone5_vining_fpga.dts  | 1 +
> >  12 files changed, 17 insertions(+)
> >
> > diff --git a/arch/arm/dts/socfpga.dtsi b/arch/arm/dts/socfpga.dtsi
> > index 314449478d..0e5445cd1b 100644
> > --- a/arch/arm/dts/socfpga.dtsi
> > +++ b/arch/arm/dts/socfpga.dtsi
> > @@ -738,6 +738,7 @@
> >                       reg-io-width = <4>;
> >                       clocks = <&l4_sp_clk>;
> >                       clock-frequency = <100000000>;
> > +                     u-boot,dm-pre-reloc;
> >               };
> >
> >               uart1: serial1 at ffc03000 {
> > diff --git a/arch/arm/dts/socfpga_arria5_socdk.dts
> b/arch/arm/dts/socfpga_arria5_socdk.dts
> > index 449ba9cbb9..128f0c9762 100644
> > --- a/arch/arm/dts/socfpga_arria5_socdk.dts
> > +++ b/arch/arm/dts/socfpga_arria5_socdk.dts
> > @@ -11,6 +11,7 @@
> >
> >       chosen {
> >               bootargs = "console=ttyS0,115200";
> > +             stdout-path = "serial0:115200n8";
> >       };
> >
> >       memory {
> > diff --git a/arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts
> b/arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts
> > index aeb327dd5b..8e01a27320 100644
> > --- a/arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts
> > +++ b/arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts
> > @@ -11,6 +11,7 @@
> >
> >       chosen {
> >               bootargs = "console=ttyS0,115200";
> > +             stdout-path = "serial0:115200n8";
> >       };
> >
> >       aliases {
> > diff --git a/arch/arm/dts/socfpga_cyclone5_de0_nano_soc.dts
> b/arch/arm/dts/socfpga_cyclone5_de0_nano_soc.dts
> > index f4a98e4bb0..16b86ce631 100644
> > --- a/arch/arm/dts/socfpga_cyclone5_de0_nano_soc.dts
> > +++ b/arch/arm/dts/socfpga_cyclone5_de0_nano_soc.dts
> > @@ -11,6 +11,7 @@
> >
> >       chosen {
> >               bootargs = "console=ttyS0,115200";
> > +             stdout-path = "serial0:115200n8";
> >       };
> >
> >       aliases {
> > diff --git a/arch/arm/dts/socfpga_cyclone5_de10_nano.dts
> b/arch/arm/dts/socfpga_cyclone5_de10_nano.dts
> > index 7da2d8b043..9d40ce912e 100644
> > --- a/arch/arm/dts/socfpga_cyclone5_de10_nano.dts
> > +++ b/arch/arm/dts/socfpga_cyclone5_de10_nano.dts
> > @@ -13,6 +13,7 @@
> >
> >       chosen {
> >               bootargs = "console=ttyS0,115200";
> > +             stdout-path = "serial0:115200n8";
> >       };
> >
> >       aliases {
> > diff --git a/arch/arm/dts/socfpga_cyclone5_de1_soc.dts
> b/arch/arm/dts/socfpga_cyclone5_de1_soc.dts
> > index e6fadb4fc9..d7dd809162 100644
> > --- a/arch/arm/dts/socfpga_cyclone5_de1_soc.dts
> > +++ b/arch/arm/dts/socfpga_cyclone5_de1_soc.dts
> > @@ -11,6 +11,7 @@
> >
> >       chosen {
> >               bootargs = "console=ttyS0,115200";
> > +             stdout-path = "serial0:115200n8";
> >       };
> >
> >       aliases {
> > diff --git a/arch/arm/dts/socfpga_cyclone5_is1.dts
> b/arch/arm/dts/socfpga_cyclone5_is1.dts
> > index aa1ce2c3e2..e6306fb285 100644
> > --- a/arch/arm/dts/socfpga_cyclone5_is1.dts
> > +++ b/arch/arm/dts/socfpga_cyclone5_is1.dts
> > @@ -11,6 +11,7 @@
> >
> >       chosen {
> >               bootargs = "console=ttyS0,115200";
> > +             stdout-path = "serial0:115200n8";
> >       };
> >
> >       memory {
> > diff --git a/arch/arm/dts/socfpga_cyclone5_socdk.dts
> b/arch/arm/dts/socfpga_cyclone5_socdk.dts
> > index 55c70abb02..b24c39e1a3 100644
> > --- a/arch/arm/dts/socfpga_cyclone5_socdk.dts
> > +++ b/arch/arm/dts/socfpga_cyclone5_socdk.dts
> > @@ -11,6 +11,7 @@
> >
> >       chosen {
> >               bootargs = "console=ttyS0,115200";
> > +             stdout-path = "serial0:115200n8";
> >       };
> >
> >       memory {
> > diff --git a/arch/arm/dts/socfpga_cyclone5_sockit.dts
> b/arch/arm/dts/socfpga_cyclone5_sockit.dts
> > index 08d8356d80..734e682ed2 100644
> > --- a/arch/arm/dts/socfpga_cyclone5_sockit.dts
> > +++ b/arch/arm/dts/socfpga_cyclone5_sockit.dts
> > @@ -11,6 +11,7 @@
> >
> >       chosen {
> >               bootargs = "console=ttyS0,115200";
> > +             stdout-path = "serial0:115200n8";
> >       };
> >
> >       aliases {
> > diff --git a/arch/arm/dts/socfpga_cyclone5_socrates.dts
> b/arch/arm/dts/socfpga_cyclone5_socrates.dts
> > index 0d452ae300..7f9b48a839 100644
> > --- a/arch/arm/dts/socfpga_cyclone5_socrates.dts
> > +++ b/arch/arm/dts/socfpga_cyclone5_socrates.dts
> > @@ -11,6 +11,7 @@
> >
> >       chosen {
> >               bootargs = "console=ttyS0,115200";
> > +             stdout-path = "serial0:115200n8";
> >       };
> >
> >       aliases {
> > @@ -84,3 +85,8 @@
> >       disable-over-current;
> >       status = "okay";
> >  };
> > +
> > +&uart0 {
> > +     u-boot,dm-pre-reloc;
> > +     status = "okay";
> > +};
> > diff --git a/arch/arm/dts/socfpga_cyclone5_sr1500.dts
> b/arch/arm/dts/socfpga_cyclone5_sr1500.dts
> > index 341df7a3e7..1993ea2e81 100644
> > --- a/arch/arm/dts/socfpga_cyclone5_sr1500.dts
> > +++ b/arch/arm/dts/socfpga_cyclone5_sr1500.dts
> > @@ -11,6 +11,7 @@
> >
> >       chosen {
> >               bootargs = "console=ttyS0,115200";
> > +             stdout-path = "serial0:115200n8";
> >       };
> >
> >       aliases {
> > diff --git a/arch/arm/dts/socfpga_cyclone5_vining_fpga.dts
> b/arch/arm/dts/socfpga_cyclone5_vining_fpga.dts
> > index 7a032af3a4..27dd5e82d6 100644
> > --- a/arch/arm/dts/socfpga_cyclone5_vining_fpga.dts
> > +++ b/arch/arm/dts/socfpga_cyclone5_vining_fpga.dts
> > @@ -11,6 +11,7 @@
> >
> >       chosen {
> >               bootargs = "console=ttyS0,115200";
> > +             stdout-path = "serial0:115200n8";
> >       };
> >
> >       aliases {
> >
>
>
> --
> Best regards,
> Marek Vasut
>

[U-Boot] [PATCH 2/5] arm: socfpga: fix device trees to work with DM serial

[Emmanuel Vadot]

On Mon, 6 Aug 2018 15:42:01 +0200
Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> wrote:

> Marek Vasut <marex@denx.de> schrieb am Mo., 6. Aug. 2018, 15:19:
> 
> > On 08/05/2018 09:34 PM, Simon Goldschmidt wrote:
> > > Device trees need to have the serial console device available
> > > before relocation and require a stdout-path in chosen at least
> > > for SPL to have a console.
> > >
> > > Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> >
> > This should be upstreamed to Linux too ?
> >
> 
> Hmm, I'm not sure. Does Linux use the stdout-path too? I always use
> bootargs only...

 Linux is the standard repo where other project (like FreeBSD) pull the
DTS and stdout-path is standard, so it should be upstreamed.

> 
> > > ---
> > >
> > >  arch/arm/dts/socfpga.dtsi                      | 1 +
> > >  arch/arm/dts/socfpga_arria5_socdk.dts          | 1 +
> > >  arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts     | 1 +
> > >  arch/arm/dts/socfpga_cyclone5_de0_nano_soc.dts | 1 +
> > >  arch/arm/dts/socfpga_cyclone5_de10_nano.dts    | 1 +
> > >  arch/arm/dts/socfpga_cyclone5_de1_soc.dts      | 1 +
> > >  arch/arm/dts/socfpga_cyclone5_is1.dts          | 1 +
> > >  arch/arm/dts/socfpga_cyclone5_socdk.dts        | 1 +
> > >  arch/arm/dts/socfpga_cyclone5_sockit.dts       | 1 +
> > >  arch/arm/dts/socfpga_cyclone5_socrates.dts     | 6 ++++++
> > >  arch/arm/dts/socfpga_cyclone5_sr1500.dts       | 1 +
> > >  arch/arm/dts/socfpga_cyclone5_vining_fpga.dts  | 1 +
> > >  12 files changed, 17 insertions(+)
> > >
> > > diff --git a/arch/arm/dts/socfpga.dtsi b/arch/arm/dts/socfpga.dtsi
> > > index 314449478d..0e5445cd1b 100644
> > > --- a/arch/arm/dts/socfpga.dtsi
> > > +++ b/arch/arm/dts/socfpga.dtsi
> > > @@ -738,6 +738,7 @@
> > >                       reg-io-width = <4>;
> > >                       clocks = <&l4_sp_clk>;
> > >                       clock-frequency = <100000000>;
> > > +                     u-boot,dm-pre-reloc;
> > >               };
> > >
> > >               uart1: serial1 at ffc03000 {
> > > diff --git a/arch/arm/dts/socfpga_arria5_socdk.dts
> > b/arch/arm/dts/socfpga_arria5_socdk.dts
> > > index 449ba9cbb9..128f0c9762 100644
> > > --- a/arch/arm/dts/socfpga_arria5_socdk.dts
> > > +++ b/arch/arm/dts/socfpga_arria5_socdk.dts
> > > @@ -11,6 +11,7 @@
> > >
> > >       chosen {
> > >               bootargs = "console=ttyS0,115200";
> > > +             stdout-path = "serial0:115200n8";
> > >       };
> > >
> > >       memory {
> > > diff --git a/arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts
> > b/arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts
> > > index aeb327dd5b..8e01a27320 100644
> > > --- a/arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts
> > > +++ b/arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts
> > > @@ -11,6 +11,7 @@
> > >
> > >       chosen {
> > >               bootargs = "console=ttyS0,115200";
> > > +             stdout-path = "serial0:115200n8";
> > >       };
> > >
> > >       aliases {
> > > diff --git a/arch/arm/dts/socfpga_cyclone5_de0_nano_soc.dts
> > b/arch/arm/dts/socfpga_cyclone5_de0_nano_soc.dts
> > > index f4a98e4bb0..16b86ce631 100644
> > > --- a/arch/arm/dts/socfpga_cyclone5_de0_nano_soc.dts
> > > +++ b/arch/arm/dts/socfpga_cyclone5_de0_nano_soc.dts
> > > @@ -11,6 +11,7 @@
> > >
> > >       chosen {
> > >               bootargs = "console=ttyS0,115200";
> > > +             stdout-path = "serial0:115200n8";
> > >       };
> > >
> > >       aliases {
> > > diff --git a/arch/arm/dts/socfpga_cyclone5_de10_nano.dts
> > b/arch/arm/dts/socfpga_cyclone5_de10_nano.dts
> > > index 7da2d8b043..9d40ce912e 100644
> > > --- a/arch/arm/dts/socfpga_cyclone5_de10_nano.dts
> > > +++ b/arch/arm/dts/socfpga_cyclone5_de10_nano.dts
> > > @@ -13,6 +13,7 @@
> > >
> > >       chosen {
> > >               bootargs = "console=ttyS0,115200";
> > > +             stdout-path = "serial0:115200n8";
> > >       };
> > >
> > >       aliases {
> > > diff --git a/arch/arm/dts/socfpga_cyclone5_de1_soc.dts
> > b/arch/arm/dts/socfpga_cyclone5_de1_soc.dts
> > > index e6fadb4fc9..d7dd809162 100644
> > > --- a/arch/arm/dts/socfpga_cyclone5_de1_soc.dts
> > > +++ b/arch/arm/dts/socfpga_cyclone5_de1_soc.dts
> > > @@ -11,6 +11,7 @@
> > >
> > >       chosen {
> > >               bootargs = "console=ttyS0,115200";
> > > +             stdout-path = "serial0:115200n8";
> > >       };
> > >
> > >       aliases {
> > > diff --git a/arch/arm/dts/socfpga_cyclone5_is1.dts
> > b/arch/arm/dts/socfpga_cyclone5_is1.dts
> > > index aa1ce2c3e2..e6306fb285 100644
> > > --- a/arch/arm/dts/socfpga_cyclone5_is1.dts
> > > +++ b/arch/arm/dts/socfpga_cyclone5_is1.dts
> > > @@ -11,6 +11,7 @@
> > >
> > >       chosen {
> > >               bootargs = "console=ttyS0,115200";
> > > +             stdout-path = "serial0:115200n8";
> > >       };
> > >
> > >       memory {
> > > diff --git a/arch/arm/dts/socfpga_cyclone5_socdk.dts
> > b/arch/arm/dts/socfpga_cyclone5_socdk.dts
> > > index 55c70abb02..b24c39e1a3 100644
> > > --- a/arch/arm/dts/socfpga_cyclone5_socdk.dts
> > > +++ b/arch/arm/dts/socfpga_cyclone5_socdk.dts
> > > @@ -11,6 +11,7 @@
> > >
> > >       chosen {
> > >               bootargs = "console=ttyS0,115200";
> > > +             stdout-path = "serial0:115200n8";
> > >       };
> > >
> > >       memory {
> > > diff --git a/arch/arm/dts/socfpga_cyclone5_sockit.dts
> > b/arch/arm/dts/socfpga_cyclone5_sockit.dts
> > > index 08d8356d80..734e682ed2 100644
> > > --- a/arch/arm/dts/socfpga_cyclone5_sockit.dts
> > > +++ b/arch/arm/dts/socfpga_cyclone5_sockit.dts
> > > @@ -11,6 +11,7 @@
> > >
> > >       chosen {
> > >               bootargs = "console=ttyS0,115200";
> > > +             stdout-path = "serial0:115200n8";
> > >       };
> > >
> > >       aliases {
> > > diff --git a/arch/arm/dts/socfpga_cyclone5_socrates.dts
> > b/arch/arm/dts/socfpga_cyclone5_socrates.dts
> > > index 0d452ae300..7f9b48a839 100644
> > > --- a/arch/arm/dts/socfpga_cyclone5_socrates.dts
> > > +++ b/arch/arm/dts/socfpga_cyclone5_socrates.dts
> > > @@ -11,6 +11,7 @@
> > >
> > >       chosen {
> > >               bootargs = "console=ttyS0,115200";
> > > +             stdout-path = "serial0:115200n8";
> > >       };
> > >
> > >       aliases {
> > > @@ -84,3 +85,8 @@
> > >       disable-over-current;
> > >       status = "okay";
> > >  };
> > > +
> > > +&uart0 {
> > > +     u-boot,dm-pre-reloc;
> > > +     status = "okay";
> > > +};
> > > diff --git a/arch/arm/dts/socfpga_cyclone5_sr1500.dts
> > b/arch/arm/dts/socfpga_cyclone5_sr1500.dts
> > > index 341df7a3e7..1993ea2e81 100644
> > > --- a/arch/arm/dts/socfpga_cyclone5_sr1500.dts
> > > +++ b/arch/arm/dts/socfpga_cyclone5_sr1500.dts
> > > @@ -11,6 +11,7 @@
> > >
> > >       chosen {
> > >               bootargs = "console=ttyS0,115200";
> > > +             stdout-path = "serial0:115200n8";
> > >       };
> > >
> > >       aliases {
> > > diff --git a/arch/arm/dts/socfpga_cyclone5_vining_fpga.dts
> > b/arch/arm/dts/socfpga_cyclone5_vining_fpga.dts
> > > index 7a032af3a4..27dd5e82d6 100644
> > > --- a/arch/arm/dts/socfpga_cyclone5_vining_fpga.dts
> > > +++ b/arch/arm/dts/socfpga_cyclone5_vining_fpga.dts
> > > @@ -11,6 +11,7 @@
> > >
> > >       chosen {
> > >               bootargs = "console=ttyS0,115200";
> > > +             stdout-path = "serial0:115200n8";
> > >       };
> > >
> > >       aliases {
> > >
> >
> >
> > --
> > Best regards,
> > Marek Vasut
> >
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot


-- 
Emmanuel Vadot <manu@bidouilliste.com> <manu@freebsd.org>

[U-Boot] [PATCH 2/5] arm: socfpga: fix device trees to work with DM serial

[Simon Goldschmidt]

Emmanuel Vadot <manu@bidouilliste.com> schrieb am Mo., 6. Aug. 2018, 15:48:

> On Mon, 6 Aug 2018 15:42:01 +0200
> Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> wrote:
>
> > Marek Vasut <marex@denx.de> schrieb am Mo., 6. Aug. 2018, 15:19:
> >
> > > On 08/05/2018 09:34 PM, Simon Goldschmidt wrote:
> > > > Device trees need to have the serial console device available
> > > > before relocation and require a stdout-path in chosen at least
> > > > for SPL to have a console.
> > > >
> > > > Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> > >
> > > This should be upstreamed to Linux too ?
> > >
> >
> > Hmm, I'm not sure. Does Linux use the stdout-path too? I always use
> > bootargs only...
>
>  Linux is the standard repo where other project (like FreeBSD) pull the
> DTS and stdout-path is standard, so it should be upstreamed.
>

Ok then, I can upstream them. How is the workflow, via which repository or
list so socfpga device trees get pushed?

Simon


> >
> > > > ---
> > > >
> > > >  arch/arm/dts/socfpga.dtsi                      | 1 +
> > > >  arch/arm/dts/socfpga_arria5_socdk.dts          | 1 +
> > > >  arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts     | 1 +
> > > >  arch/arm/dts/socfpga_cyclone5_de0_nano_soc.dts | 1 +
> > > >  arch/arm/dts/socfpga_cyclone5_de10_nano.dts    | 1 +
> > > >  arch/arm/dts/socfpga_cyclone5_de1_soc.dts      | 1 +
> > > >  arch/arm/dts/socfpga_cyclone5_is1.dts          | 1 +
> > > >  arch/arm/dts/socfpga_cyclone5_socdk.dts        | 1 +
> > > >  arch/arm/dts/socfpga_cyclone5_sockit.dts       | 1 +
> > > >  arch/arm/dts/socfpga_cyclone5_socrates.dts     | 6 ++++++
> > > >  arch/arm/dts/socfpga_cyclone5_sr1500.dts       | 1 +
> > > >  arch/arm/dts/socfpga_cyclone5_vining_fpga.dts  | 1 +
> > > >  12 files changed, 17 insertions(+)
> > > >
> > > > diff --git a/arch/arm/dts/socfpga.dtsi b/arch/arm/dts/socfpga.dtsi
> > > > index 314449478d..0e5445cd1b 100644
> > > > --- a/arch/arm/dts/socfpga.dtsi
> > > > +++ b/arch/arm/dts/socfpga.dtsi
> > > > @@ -738,6 +738,7 @@
> > > >                       reg-io-width = <4>;
> > > >                       clocks = <&l4_sp_clk>;
> > > >                       clock-frequency = <100000000>;
> > > > +                     u-boot,dm-pre-reloc;
> > > >               };
> > > >
> > > >               uart1: serial1 at ffc03000 {
> > > > diff --git a/arch/arm/dts/socfpga_arria5_socdk.dts
> > > b/arch/arm/dts/socfpga_arria5_socdk.dts
> > > > index 449ba9cbb9..128f0c9762 100644
> > > > --- a/arch/arm/dts/socfpga_arria5_socdk.dts
> > > > +++ b/arch/arm/dts/socfpga_arria5_socdk.dts
> > > > @@ -11,6 +11,7 @@
> > > >
> > > >       chosen {
> > > >               bootargs = "console=ttyS0,115200";
> > > > +             stdout-path = "serial0:115200n8";
> > > >       };
> > > >
> > > >       memory {
> > > > diff --git a/arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts
> > > b/arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts
> > > > index aeb327dd5b..8e01a27320 100644
> > > > --- a/arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts
> > > > +++ b/arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts
> > > > @@ -11,6 +11,7 @@
> > > >
> > > >       chosen {
> > > >               bootargs = "console=ttyS0,115200";
> > > > +             stdout-path = "serial0:115200n8";
> > > >       };
> > > >
> > > >       aliases {
> > > > diff --git a/arch/arm/dts/socfpga_cyclone5_de0_nano_soc.dts
> > > b/arch/arm/dts/socfpga_cyclone5_de0_nano_soc.dts
> > > > index f4a98e4bb0..16b86ce631 100644
> > > > --- a/arch/arm/dts/socfpga_cyclone5_de0_nano_soc.dts
> > > > +++ b/arch/arm/dts/socfpga_cyclone5_de0_nano_soc.dts
> > > > @@ -11,6 +11,7 @@
> > > >
> > > >       chosen {
> > > >               bootargs = "console=ttyS0,115200";
> > > > +             stdout-path = "serial0:115200n8";
> > > >       };
> > > >
> > > >       aliases {
> > > > diff --git a/arch/arm/dts/socfpga_cyclone5_de10_nano.dts
> > > b/arch/arm/dts/socfpga_cyclone5_de10_nano.dts
> > > > index 7da2d8b043..9d40ce912e 100644
> > > > --- a/arch/arm/dts/socfpga_cyclone5_de10_nano.dts
> > > > +++ b/arch/arm/dts/socfpga_cyclone5_de10_nano.dts
> > > > @@ -13,6 +13,7 @@
> > > >
> > > >       chosen {
> > > >               bootargs = "console=ttyS0,115200";
> > > > +             stdout-path = "serial0:115200n8";
> > > >       };
> > > >
> > > >       aliases {
> > > > diff --git a/arch/arm/dts/socfpga_cyclone5_de1_soc.dts
> > > b/arch/arm/dts/socfpga_cyclone5_de1_soc.dts
> > > > index e6fadb4fc9..d7dd809162 100644
> > > > --- a/arch/arm/dts/socfpga_cyclone5_de1_soc.dts
> > > > +++ b/arch/arm/dts/socfpga_cyclone5_de1_soc.dts
> > > > @@ -11,6 +11,7 @@
> > > >
> > > >       chosen {
> > > >               bootargs = "console=ttyS0,115200";
> > > > +             stdout-path = "serial0:115200n8";
> > > >       };
> > > >
> > > >       aliases {
> > > > diff --git a/arch/arm/dts/socfpga_cyclone5_is1.dts
> > > b/arch/arm/dts/socfpga_cyclone5_is1.dts
> > > > index aa1ce2c3e2..e6306fb285 100644
> > > > --- a/arch/arm/dts/socfpga_cyclone5_is1.dts
> > > > +++ b/arch/arm/dts/socfpga_cyclone5_is1.dts
> > > > @@ -11,6 +11,7 @@
> > > >
> > > >       chosen {
> > > >               bootargs = "console=ttyS0,115200";
> > > > +             stdout-path = "serial0:115200n8";
> > > >       };
> > > >
> > > >       memory {
> > > > diff --git a/arch/arm/dts/socfpga_cyclone5_socdk.dts
> > > b/arch/arm/dts/socfpga_cyclone5_socdk.dts
> > > > index 55c70abb02..b24c39e1a3 100644
> > > > --- a/arch/arm/dts/socfpga_cyclone5_socdk.dts
> > > > +++ b/arch/arm/dts/socfpga_cyclone5_socdk.dts
> > > > @@ -11,6 +11,7 @@
> > > >
> > > >       chosen {
> > > >               bootargs = "console=ttyS0,115200";
> > > > +             stdout-path = "serial0:115200n8";
> > > >       };
> > > >
> > > >       memory {
> > > > diff --git a/arch/arm/dts/socfpga_cyclone5_sockit.dts
> > > b/arch/arm/dts/socfpga_cyclone5_sockit.dts
> > > > index 08d8356d80..734e682ed2 100644
> > > > --- a/arch/arm/dts/socfpga_cyclone5_sockit.dts
> > > > +++ b/arch/arm/dts/socfpga_cyclone5_sockit.dts
> > > > @@ -11,6 +11,7 @@
> > > >
> > > >       chosen {
> > > >               bootargs = "console=ttyS0,115200";
> > > > +             stdout-path = "serial0:115200n8";
> > > >       };
> > > >
> > > >       aliases {
> > > > diff --git a/arch/arm/dts/socfpga_cyclone5_socrates.dts
> > > b/arch/arm/dts/socfpga_cyclone5_socrates.dts
> > > > index 0d452ae300..7f9b48a839 100644
> > > > --- a/arch/arm/dts/socfpga_cyclone5_socrates.dts
> > > > +++ b/arch/arm/dts/socfpga_cyclone5_socrates.dts
> > > > @@ -11,6 +11,7 @@
> > > >
> > > >       chosen {
> > > >               bootargs = "console=ttyS0,115200";
> > > > +             stdout-path = "serial0:115200n8";
> > > >       };
> > > >
> > > >       aliases {
> > > > @@ -84,3 +85,8 @@
> > > >       disable-over-current;
> > > >       status = "okay";
> > > >  };
> > > > +
> > > > +&uart0 {
> > > > +     u-boot,dm-pre-reloc;
> > > > +     status = "okay";
> > > > +};
> > > > diff --git a/arch/arm/dts/socfpga_cyclone5_sr1500.dts
> > > b/arch/arm/dts/socfpga_cyclone5_sr1500.dts
> > > > index 341df7a3e7..1993ea2e81 100644
> > > > --- a/arch/arm/dts/socfpga_cyclone5_sr1500.dts
> > > > +++ b/arch/arm/dts/socfpga_cyclone5_sr1500.dts
> > > > @@ -11,6 +11,7 @@
> > > >
> > > >       chosen {
> > > >               bootargs = "console=ttyS0,115200";
> > > > +             stdout-path = "serial0:115200n8";
> > > >       };
> > > >
> > > >       aliases {
> > > > diff --git a/arch/arm/dts/socfpga_cyclone5_vining_fpga.dts
> > > b/arch/arm/dts/socfpga_cyclone5_vining_fpga.dts
> > > > index 7a032af3a4..27dd5e82d6 100644
> > > > --- a/arch/arm/dts/socfpga_cyclone5_vining_fpga.dts
> > > > +++ b/arch/arm/dts/socfpga_cyclone5_vining_fpga.dts
> > > > @@ -11,6 +11,7 @@
> > > >
> > > >       chosen {
> > > >               bootargs = "console=ttyS0,115200";
> > > > +             stdout-path = "serial0:115200n8";
> > > >       };
> > > >
> > > >       aliases {
> > > >
> > >
> > >
> > > --
> > > Best regards,
> > > Marek Vasut
> > >
> > _______________________________________________
> > U-Boot mailing list
> > U-Boot at lists.denx.de
> > https://lists.denx.de/listinfo/u-boot
>
>
> --
> Emmanuel Vadot <manu@bidouilliste.com> <manu@freebsd.org>
>

[U-Boot] [PATCH 2/5] arm: socfpga: fix device trees to work with DM serial

[Simon Goldschmidt]

On Mon, Aug 6, 2018 at 4:41 PM Simon Goldschmidt
<simon.k.r.goldschmidt@gmail.com> wrote:
>
>
>
> Emmanuel Vadot <manu@bidouilliste.com> schrieb am Mo., 6. Aug. 2018, 15:48:
>>
>> On Mon, 6 Aug 2018 15:42:01 +0200
>> Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> wrote:
>>
>> > Marek Vasut <marex@denx.de> schrieb am Mo., 6. Aug. 2018, 15:19:
>> >
>> > > On 08/05/2018 09:34 PM, Simon Goldschmidt wrote:
>> > > > Device trees need to have the serial console device available
>> > > > before relocation and require a stdout-path in chosen at least
>> > > > for SPL to have a console.
>> > > >
>> > > > Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
>> > >
>> > > This should be upstreamed to Linux too ?
>> > >
>> >
>> > Hmm, I'm not sure. Does Linux use the stdout-path too? I always use
>> > bootargs only...
>>
>>  Linux is the standard repo where other project (like FreeBSD) pull the
>> DTS and stdout-path is standard, so it should be upstreamed.
>
>
> Ok then, I can upstream them. How is the workflow, via which repository or list so socfpga device trees get pushed?

Ignore that question.

I had a look at the current socfpga device trees in Linux and only
socrates and vining seem to be missing the stdout-path. I'll send a
patch to add these.
The rest of the device trees, however, is very different to the U-Boot
ones. What's the procedure here, shall we just copy them from Linux or
do we keep ours?

In any case, we do need this patch to get U-Boot and SPL running
correctly on gen5, which is has been broken since v2018.07.

Tanks,
Simon

[U-Boot] [PATCH 0/6] Get socfpga gen5 SPL working again.

[Simon Goldschmidt]

Socfpga gen5 SPL has been broken since moving to DM serial with
v2018.07. Also, U-Boot console output has been broken since then.
This series fixes this and makes some related small improvements.

Changes in v2:
- Improved comment on patch 1
- Removing gd->malloc_base assignment at the end of board_init_f()
  moved to an extra patch
- don't change printf() to debug() in reset_manager_gen5.c
  socfpga_bridges_reset() (instead make debug uart handle this)
- make ns16550 debug uart handle putc being called before init
- removed the assignment of gd->malloc_limit from board_init()


Simon Goldschmidt (6):
  arm: socfpga: fix SPL on gen5 after moving to DM serial
  arm: socfpga: fix device trees to work with DM serial
  arm: socfpga: spl_gen5: clean up malloc_base assignment
  arm: socfpga: cyclone5: handle debug uart
  serial: ns16550: fix debug uart putc called before init
  malloc_simple: calloc: don't call memset if malloc failed

 arch/arm/dts/socfpga.dtsi                      |  1 +
 arch/arm/dts/socfpga_arria5_socdk.dts          |  1 +
 arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts     |  1 +
 arch/arm/dts/socfpga_cyclone5_de0_nano_soc.dts |  1 +
 arch/arm/dts/socfpga_cyclone5_de10_nano.dts    |  1 +
 arch/arm/dts/socfpga_cyclone5_de1_soc.dts      |  1 +
 arch/arm/dts/socfpga_cyclone5_is1.dts          |  1 +
 arch/arm/dts/socfpga_cyclone5_socdk.dts        |  1 +
 arch/arm/dts/socfpga_cyclone5_sockit.dts       |  1 +
 arch/arm/dts/socfpga_cyclone5_socrates.dts     |  6 ++++++
 arch/arm/dts/socfpga_cyclone5_sr1500.dts       |  1 +
 arch/arm/dts/socfpga_cyclone5_vining_fpga.dts  |  1 +
 arch/arm/mach-socfpga/spl_gen5.c               | 16 +++++++++++++---
 common/malloc_simple.c                         |  3 ++-
 drivers/serial/ns16550.c                       | 18 ++++++++++++++++--
 15 files changed, 48 insertions(+), 6 deletions(-)

-- 
2.17.1

[U-Boot] [PATCH 1/6] arm: socfpga: fix SPL on gen5 after moving to DM serial

[Simon Goldschmidt]

There were NULL pointers dereferenced because DM was used
too early without correct initialization:
- malloc_simple returned NULL when called from preloader_console_init()
  because gd->malloc_limit was 0
- uclass_add dereferenced gd->uclass_root members which were NULL because
  dm_init (or one of its relatives) has not been called.

All this is fixed by calling spl_early_init before calling
preloader_console_init.

This fixes commit 73172753f4f3 ("ARM: socfpga: Convert to DM serial")

Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
---
v2:
- Don't remove gd->malloc_base assignment at the end of board_init_f()
  (moved to an extra patch)

 arch/arm/mach-socfpga/spl_gen5.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/mach-socfpga/spl_gen5.c b/arch/arm/mach-socfpga/spl_gen5.c
index d6fe7d35af..9bdfaa3c1e 100644
--- a/arch/arm/mach-socfpga/spl_gen5.c
+++ b/arch/arm/mach-socfpga/spl_gen5.c
@@ -86,6 +86,7 @@ void board_init_f(ulong dummy)
 	const struct cm_config *cm_default_cfg = cm_get_default_config();
 	unsigned long sdram_size;
 	unsigned long reg;
+	int ret;
 
 	/*
 	 * First C code to run. Clear fake OCRAM ECC first as SBE
@@ -152,6 +153,12 @@ void board_init_f(ulong dummy)
 	/* unfreeze / thaw all IO banks */
 	sys_mgr_frzctrl_thaw_req();
 
+	ret = spl_early_init();
+	if (ret) {
+		debug("spl_early_init() failed: %d\n", ret);
+		hang();
+	}
+
 	/* enable console uart printing */
 	preloader_console_init();
 
-- 
2.17.1

[U-Boot] [PATCH 2/6] arm: socfpga: fix device trees to work with DM serial

[Simon Goldschmidt]

Device trees need to have the serial console device available
before relocation and require a stdout-path in chosen at least
for SPL to have a console.

Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
---
v2: no changes

 arch/arm/dts/socfpga.dtsi                      | 1 +
 arch/arm/dts/socfpga_arria5_socdk.dts          | 1 +
 arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts     | 1 +
 arch/arm/dts/socfpga_cyclone5_de0_nano_soc.dts | 1 +
 arch/arm/dts/socfpga_cyclone5_de10_nano.dts    | 1 +
 arch/arm/dts/socfpga_cyclone5_de1_soc.dts      | 1 +
 arch/arm/dts/socfpga_cyclone5_is1.dts          | 1 +
 arch/arm/dts/socfpga_cyclone5_socdk.dts        | 1 +
 arch/arm/dts/socfpga_cyclone5_sockit.dts       | 1 +
 arch/arm/dts/socfpga_cyclone5_socrates.dts     | 6 ++++++
 arch/arm/dts/socfpga_cyclone5_sr1500.dts       | 1 +
 arch/arm/dts/socfpga_cyclone5_vining_fpga.dts  | 1 +
 12 files changed, 17 insertions(+)

diff --git a/arch/arm/dts/socfpga.dtsi b/arch/arm/dts/socfpga.dtsi
index 314449478d..0e5445cd1b 100644
--- a/arch/arm/dts/socfpga.dtsi
+++ b/arch/arm/dts/socfpga.dtsi
@@ -738,6 +738,7 @@
 			reg-io-width = <4>;
 			clocks = <&l4_sp_clk>;
 			clock-frequency = <100000000>;
+			u-boot,dm-pre-reloc;
 		};
 
 		uart1: serial1 at ffc03000 {
diff --git a/arch/arm/dts/socfpga_arria5_socdk.dts b/arch/arm/dts/socfpga_arria5_socdk.dts
index 449ba9cbb9..128f0c9762 100644
--- a/arch/arm/dts/socfpga_arria5_socdk.dts
+++ b/arch/arm/dts/socfpga_arria5_socdk.dts
@@ -11,6 +11,7 @@
 
 	chosen {
 		bootargs = "console=ttyS0,115200";
+		stdout-path = "serial0:115200n8";
 	};
 
 	memory {
diff --git a/arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts b/arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts
index aeb327dd5b..8e01a27320 100644
--- a/arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts
+++ b/arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts
@@ -11,6 +11,7 @@
 
 	chosen {
 		bootargs = "console=ttyS0,115200";
+		stdout-path = "serial0:115200n8";
 	};
 
 	aliases {
diff --git a/arch/arm/dts/socfpga_cyclone5_de0_nano_soc.dts b/arch/arm/dts/socfpga_cyclone5_de0_nano_soc.dts
index f4a98e4bb0..16b86ce631 100644
--- a/arch/arm/dts/socfpga_cyclone5_de0_nano_soc.dts
+++ b/arch/arm/dts/socfpga_cyclone5_de0_nano_soc.dts
@@ -11,6 +11,7 @@
 
 	chosen {
 		bootargs = "console=ttyS0,115200";
+		stdout-path = "serial0:115200n8";
 	};
 
 	aliases {
diff --git a/arch/arm/dts/socfpga_cyclone5_de10_nano.dts b/arch/arm/dts/socfpga_cyclone5_de10_nano.dts
index 7da2d8b043..9d40ce912e 100644
--- a/arch/arm/dts/socfpga_cyclone5_de10_nano.dts
+++ b/arch/arm/dts/socfpga_cyclone5_de10_nano.dts
@@ -13,6 +13,7 @@
 
 	chosen {
 		bootargs = "console=ttyS0,115200";
+		stdout-path = "serial0:115200n8";
 	};
 
 	aliases {
diff --git a/arch/arm/dts/socfpga_cyclone5_de1_soc.dts b/arch/arm/dts/socfpga_cyclone5_de1_soc.dts
index e6fadb4fc9..d7dd809162 100644
--- a/arch/arm/dts/socfpga_cyclone5_de1_soc.dts
+++ b/arch/arm/dts/socfpga_cyclone5_de1_soc.dts
@@ -11,6 +11,7 @@
 
 	chosen {
 		bootargs = "console=ttyS0,115200";
+		stdout-path = "serial0:115200n8";
 	};
 
 	aliases {
diff --git a/arch/arm/dts/socfpga_cyclone5_is1.dts b/arch/arm/dts/socfpga_cyclone5_is1.dts
index aa1ce2c3e2..e6306fb285 100644
--- a/arch/arm/dts/socfpga_cyclone5_is1.dts
+++ b/arch/arm/dts/socfpga_cyclone5_is1.dts
@@ -11,6 +11,7 @@
 
 	chosen {
 		bootargs = "console=ttyS0,115200";
+		stdout-path = "serial0:115200n8";
 	};
 
 	memory {
diff --git a/arch/arm/dts/socfpga_cyclone5_socdk.dts b/arch/arm/dts/socfpga_cyclone5_socdk.dts
index 55c70abb02..b24c39e1a3 100644
--- a/arch/arm/dts/socfpga_cyclone5_socdk.dts
+++ b/arch/arm/dts/socfpga_cyclone5_socdk.dts
@@ -11,6 +11,7 @@
 
 	chosen {
 		bootargs = "console=ttyS0,115200";
+		stdout-path = "serial0:115200n8";
 	};
 
 	memory {
diff --git a/arch/arm/dts/socfpga_cyclone5_sockit.dts b/arch/arm/dts/socfpga_cyclone5_sockit.dts
index 08d8356d80..734e682ed2 100644
--- a/arch/arm/dts/socfpga_cyclone5_sockit.dts
+++ b/arch/arm/dts/socfpga_cyclone5_sockit.dts
@@ -11,6 +11,7 @@
 
 	chosen {
 		bootargs = "console=ttyS0,115200";
+		stdout-path = "serial0:115200n8";
 	};
 
 	aliases {
diff --git a/arch/arm/dts/socfpga_cyclone5_socrates.dts b/arch/arm/dts/socfpga_cyclone5_socrates.dts
index 0d452ae300..7f9b48a839 100644
--- a/arch/arm/dts/socfpga_cyclone5_socrates.dts
+++ b/arch/arm/dts/socfpga_cyclone5_socrates.dts
@@ -11,6 +11,7 @@
 
 	chosen {
 		bootargs = "console=ttyS0,115200";
+		stdout-path = "serial0:115200n8";
 	};
 
 	aliases {
@@ -84,3 +85,8 @@
 	disable-over-current;
 	status = "okay";
 };
+
+&uart0 {
+	u-boot,dm-pre-reloc;
+	status = "okay";
+};
diff --git a/arch/arm/dts/socfpga_cyclone5_sr1500.dts b/arch/arm/dts/socfpga_cyclone5_sr1500.dts
index 341df7a3e7..1993ea2e81 100644
--- a/arch/arm/dts/socfpga_cyclone5_sr1500.dts
+++ b/arch/arm/dts/socfpga_cyclone5_sr1500.dts
@@ -11,6 +11,7 @@
 
 	chosen {
 		bootargs = "console=ttyS0,115200";
+		stdout-path = "serial0:115200n8";
 	};
 
 	aliases {
diff --git a/arch/arm/dts/socfpga_cyclone5_vining_fpga.dts b/arch/arm/dts/socfpga_cyclone5_vining_fpga.dts
index 7a032af3a4..27dd5e82d6 100644
--- a/arch/arm/dts/socfpga_cyclone5_vining_fpga.dts
+++ b/arch/arm/dts/socfpga_cyclone5_vining_fpga.dts
@@ -11,6 +11,7 @@
 
 	chosen {
 		bootargs = "console=ttyS0,115200";
+		stdout-path = "serial0:115200n8";
 	};
 
 	aliases {
-- 
2.17.1

[U-Boot] [PATCH 3/6] arm: socfpga: spl_gen5: clean up malloc_base assignment

[Simon Goldschmidt]

From: Simon Goldschmidt <sgoldschmidt@de.pepperl-fuchs.com>

In spl_gen5's board_init_f(), gd->malloc_base is manually assigned
at the end of the function to point to sdram.  This code is outdated
as by now, the heap is switched to sdram by the common function
spl_relocate_stack_gd() if the appropriate defines are set.

As it was, the value assigned manually was directly overwritten by
this common code, so remove the manual assignment.

Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
---
v2:
- this patch is new in v2 of the series (extracted from PATCH v1 1/6)

 arch/arm/mach-socfpga/spl_gen5.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/arch/arm/mach-socfpga/spl_gen5.c b/arch/arm/mach-socfpga/spl_gen5.c
index 9bdfaa3c1e..0d5526656d 100644
--- a/arch/arm/mach-socfpga/spl_gen5.c
+++ b/arch/arm/mach-socfpga/spl_gen5.c
@@ -184,7 +184,4 @@ void board_init_f(ulong dummy)
 	}
 
 	socfpga_bridges_reset(1);
-
-	/* Configure simple malloc base pointer into RAM. */
-	gd->malloc_base = CONFIG_SYS_TEXT_BASE + (1024 * 1024);
 }
-- 
2.17.1

[U-Boot] [PATCH 4/6] arm: socfpga: cyclone5: handle debug uart

[Simon Goldschmidt]

If CONFIG_DEBUG_UART is enabled, correctly initialize
the debug uart before console is initialized to debug
early boot problems in SPL.

Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
---
v2:
 - don't change printf() to debug() in reset_manager_gen5.c
  socfpga_bridges_reset()

 arch/arm/mach-socfpga/spl_gen5.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/mach-socfpga/spl_gen5.c b/arch/arm/mach-socfpga/spl_gen5.c
index 0d5526656d..0e685f6ee5 100644
--- a/arch/arm/mach-socfpga/spl_gen5.c
+++ b/arch/arm/mach-socfpga/spl_gen5.c
@@ -20,6 +20,7 @@
 #include <asm/arch/scu.h>
 #include <asm/arch/nic301.h>
 #include <asm/sections.h>
+#include <debug_uart.h>
 #include <fdtdec.h>
 #include <watchdog.h>
 
@@ -153,6 +154,11 @@ void board_init_f(ulong dummy)
 	/* unfreeze / thaw all IO banks */
 	sys_mgr_frzctrl_thaw_req();
 
+#ifdef CONFIG_DEBUG_UART
+	socfpga_per_reset(SOCFPGA_RESET(UART0), 0);
+	debug_uart_init();
+#endif
+
 	ret = spl_early_init();
 	if (ret) {
 		debug("spl_early_init() failed: %d\n", ret);
-- 
2.17.1

[U-Boot] [PATCH 5/6] serial: ns16550: fix debug uart putc called before init

[Simon Goldschmidt]

If _debug_uart_putc() is called before _debug_uart_init(), the
ns16550 debug uart driver hangs in a tight loop waiting for the
tx FIFO to get empty.

As this can happen via a printf sneaking in before the port calls
debug_uart_init(), let's rather ignore characters before the debug
uart is initialized.

This is done by reading the baudrate divisor and aborting if is zero.

Tested on socfpga_cyclone5_socrates.

Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
---
v2:
 - this patch is new in v2 of the series. It replaces the printf/debug
  change in reset_manager_gen5.c from v1

 drivers/serial/ns16550.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
index 9c80090aa7..475075c03c 100644
--- a/drivers/serial/ns16550.c
+++ b/drivers/serial/ns16550.c
@@ -266,12 +266,26 @@ static inline void _debug_uart_init(void)
 	serial_dout(&com_port->lcr, UART_LCRVAL);
 }
 
+static inline int NS16550_read_baud_divisor(struct NS16550 *com_port)
+{
+	int ret;
+
+	serial_dout(&com_port->lcr, UART_LCR_BKSE | UART_LCRVAL);
+	ret = serial_din(&com_port->dll) & 0xff;
+	ret |= (serial_din(&com_port->dlm) & 0xff) << 8;
+	serial_dout(&com_port->lcr, UART_LCRVAL);
+
+	return ret;
+}
+
 static inline void _debug_uart_putc(int ch)
 {
 	struct NS16550 *com_port = (struct NS16550 *)CONFIG_DEBUG_UART_BASE;
 
-	while (!(serial_din(&com_port->lsr) & UART_LSR_THRE))
-		;
+	while (!(serial_din(&com_port->lsr) & UART_LSR_THRE)) {
+		if (!NS16550_read_baud_divisor(com_port))
+			return;
+	}
 	serial_dout(&com_port->thr, ch);
 }
 
-- 
2.17.1

[U-Boot] [PATCH 6/6] malloc_simple: calloc: don't call memset if malloc failed

[Simon Goldschmidt]

malloc_simple() can return 0 if out of memory. Don't call memset
from calloc() in this case but rely on the caller checking
the return value.

Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
Reviewed-by: Marek Vasut <marex@denx.de>

---
v2: no changes

 common/malloc_simple.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/common/malloc_simple.c b/common/malloc_simple.c
index c14f8b59c1..871b5444bd 100644
--- a/common/malloc_simple.c
+++ b/common/malloc_simple.c
@@ -57,7 +57,8 @@ void *calloc(size_t nmemb, size_t elem_size)
 	void *ptr;
 
 	ptr = malloc(size);
-	memset(ptr, '\0', size);
+	if (ptr)
+		memset(ptr, '\0', size);
 
 	return ptr;
 }
-- 
2.17.1

[U-Boot] [PATCH 5/6] serial: ns16550: fix debug uart putc called before init

[Adam Ford]

On Thu, Aug 9, 2018 at 2:08 PM Simon Goldschmidt
<simon.k.r.goldschmidt@gmail.com> wrote:
>
> If _debug_uart_putc() is called before _debug_uart_init(), the
> ns16550 debug uart driver hangs in a tight loop waiting for the
> tx FIFO to get empty.
>
> As this can happen via a printf sneaking in before the port calls
> debug_uart_init(), let's rather ignore characters before the debug
> uart is initialized.
>
> This is done by reading the baudrate divisor and aborting if is zero.
>
> Tested on socfpga_cyclone5_socrates.
>
> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> ---
> v2:
>  - this patch is new in v2 of the series. It replaces the printf/debug
>   change in reset_manager_gen5.c from v1
>
>  drivers/serial/ns16550.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
> index 9c80090aa7..475075c03c 100644
> --- a/drivers/serial/ns16550.c
> +++ b/drivers/serial/ns16550.c
> @@ -266,12 +266,26 @@ static inline void _debug_uart_init(void)
>         serial_dout(&com_port->lcr, UART_LCRVAL);
>  }
>
> +static inline int NS16550_read_baud_divisor(struct NS16550 *com_port)
> +{
> +       int ret;
> +
> +       serial_dout(&com_port->lcr, UART_LCR_BKSE | UART_LCRVAL);
> +       ret = serial_din(&com_port->dll) & 0xff;
> +       ret |= (serial_din(&com_port->dlm) & 0xff) << 8;
> +       serial_dout(&com_port->lcr, UART_LCRVAL);
> +
> +       return ret;
> +}
> +
>  static inline void _debug_uart_putc(int ch)
>  {
>         struct NS16550 *com_port = (struct NS16550 *)CONFIG_DEBUG_UART_BASE;
>
> -       while (!(serial_din(&com_port->lsr) & UART_LSR_THRE))
> -               ;
> +       while (!(serial_din(&com_port->lsr) & UART_LSR_THRE)) {
> +               if (!NS16550_read_baud_divisor(com_port))

Unless there is a change that the read_baud_divisor will change while
we're waiting for the character, could we move this check before the
while statement?  This would reduce the check for the divisor to 1x
and the while statement would only have one comparison to do.  I
realize it's rather trivial, but the way I see it, there is no reason
to do the while statement at all if the read_baud_divisor fails and
there if there is a baud divisor, we should only need to check it
once.

just my two-cents.

adam
> +                       return;
> +       }
>         serial_dout(&com_port->thr, ch);
>  }
>
> --
> 2.17.1
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH 1/6] arm: socfpga: fix SPL on gen5 after moving to DM serial

[Marek Vasut]

On 08/09/2018 09:04 PM, Simon Goldschmidt wrote:
> There were NULL pointers dereferenced because DM was used
> too early without correct initialization:
> - malloc_simple returned NULL when called from preloader_console_init()
>   because gd->malloc_limit was 0
> - uclass_add dereferenced gd->uclass_root members which were NULL because
>   dm_init (or one of its relatives) has not been called.
> 
> All this is fixed by calling spl_early_init before calling
> preloader_console_init.
> 
> This fixes commit 73172753f4f3 ("ARM: socfpga: Convert to DM serial")
> 
> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> ---
> v2:
> - Don't remove gd->malloc_base assignment at the end of board_init_f()
>   (moved to an extra patch)
> 
>  arch/arm/mach-socfpga/spl_gen5.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/arch/arm/mach-socfpga/spl_gen5.c b/arch/arm/mach-socfpga/spl_gen5.c
> index d6fe7d35af..9bdfaa3c1e 100644
> --- a/arch/arm/mach-socfpga/spl_gen5.c
> +++ b/arch/arm/mach-socfpga/spl_gen5.c
> @@ -86,6 +86,7 @@ void board_init_f(ulong dummy)
>  	const struct cm_config *cm_default_cfg = cm_get_default_config();
>  	unsigned long sdram_size;
>  	unsigned long reg;
> +	int ret;
>  
>  	/*
>  	 * First C code to run. Clear fake OCRAM ECC first as SBE
> @@ -152,6 +153,12 @@ void board_init_f(ulong dummy)
>  	/* unfreeze / thaw all IO banks */
>  	sys_mgr_frzctrl_thaw_req();
>  
> +	ret = spl_early_init();

Uh, but isn't this called from common/spl/spl.c ? I suspect the SoCFPGA
SPL is a bit weird.

> +	if (ret) {
> +		debug("spl_early_init() failed: %d\n", ret);
> +		hang();
> +	}
> +
>  	/* enable console uart printing */
>  	preloader_console_init();
>  
> 


-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 2/6] arm: socfpga: fix device trees to work with DM serial

[Marek Vasut]

On 08/09/2018 09:04 PM, Simon Goldschmidt wrote:
> Device trees need to have the serial console device available
> before relocation and require a stdout-path in chosen at least
> for SPL to have a console.
> 
> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> ---
> v2: no changes
> 
>  arch/arm/dts/socfpga.dtsi                      | 1 +
>  arch/arm/dts/socfpga_arria5_socdk.dts          | 1 +
>  arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts     | 1 +
>  arch/arm/dts/socfpga_cyclone5_de0_nano_soc.dts | 1 +
>  arch/arm/dts/socfpga_cyclone5_de10_nano.dts    | 1 +
>  arch/arm/dts/socfpga_cyclone5_de1_soc.dts      | 1 +
>  arch/arm/dts/socfpga_cyclone5_is1.dts          | 1 +
>  arch/arm/dts/socfpga_cyclone5_socdk.dts        | 1 +
>  arch/arm/dts/socfpga_cyclone5_sockit.dts       | 1 +
>  arch/arm/dts/socfpga_cyclone5_socrates.dts     | 6 ++++++
>  arch/arm/dts/socfpga_cyclone5_sr1500.dts       | 1 +
>  arch/arm/dts/socfpga_cyclone5_vining_fpga.dts  | 1 +
>  12 files changed, 17 insertions(+)
> 
> diff --git a/arch/arm/dts/socfpga.dtsi b/arch/arm/dts/socfpga.dtsi
> index 314449478d..0e5445cd1b 100644
> --- a/arch/arm/dts/socfpga.dtsi
> +++ b/arch/arm/dts/socfpga.dtsi
> @@ -738,6 +738,7 @@
>  			reg-io-width = <4>;
>  			clocks = <&l4_sp_clk>;
>  			clock-frequency = <100000000>;
> +			u-boot,dm-pre-reloc;

This is board-specific, not all boards use uart0 .

>  		};
>  
>  		uart1: serial1 at ffc03000 {
[...]

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 3/6] arm: socfpga: spl_gen5: clean up malloc_base assignment

[Marek Vasut]

On 08/09/2018 09:04 PM, Simon Goldschmidt wrote:
> From: Simon Goldschmidt <sgoldschmidt@de.pepperl-fuchs.com>
> 
> In spl_gen5's board_init_f(), gd->malloc_base is manually assigned
> at the end of the function to point to sdram.  This code is outdated
> as by now, the heap is switched to sdram by the common function
> spl_relocate_stack_gd() if the appropriate defines are set.
> 
> As it was, the value assigned manually was directly overwritten by
> this common code, so remove the manual assignment.
> 
> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> ---
> v2:
> - this patch is new in v2 of the series (extracted from PATCH v1 1/6)
> 
>  arch/arm/mach-socfpga/spl_gen5.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/arch/arm/mach-socfpga/spl_gen5.c b/arch/arm/mach-socfpga/spl_gen5.c
> index 9bdfaa3c1e..0d5526656d 100644
> --- a/arch/arm/mach-socfpga/spl_gen5.c
> +++ b/arch/arm/mach-socfpga/spl_gen5.c
> @@ -184,7 +184,4 @@ void board_init_f(ulong dummy)
>  	}
>  
>  	socfpga_bridges_reset(1);
> -
> -	/* Configure simple malloc base pointer into RAM. */
> -	gd->malloc_base = CONFIG_SYS_TEXT_BASE + (1024 * 1024);
>  }
> 
I like this patch :)

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 5/6] serial: ns16550: fix debug uart putc called before init

[Marek Vasut]

On 08/09/2018 11:13 PM, Adam Ford wrote:
> On Thu, Aug 9, 2018 at 2:08 PM Simon Goldschmidt
> <simon.k.r.goldschmidt@gmail.com> wrote:
>>
>> If _debug_uart_putc() is called before _debug_uart_init(), the
>> ns16550 debug uart driver hangs in a tight loop waiting for the
>> tx FIFO to get empty.
>>
>> As this can happen via a printf sneaking in before the port calls
>> debug_uart_init(), let's rather ignore characters before the debug
>> uart is initialized.
>>
>> This is done by reading the baudrate divisor and aborting if is zero.
>>
>> Tested on socfpga_cyclone5_socrates.
>>
>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
>> ---
>> v2:
>>  - this patch is new in v2 of the series. It replaces the printf/debug
>>   change in reset_manager_gen5.c from v1
>>
>>  drivers/serial/ns16550.c | 18 ++++++++++++++++--
>>  1 file changed, 16 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
>> index 9c80090aa7..475075c03c 100644
>> --- a/drivers/serial/ns16550.c
>> +++ b/drivers/serial/ns16550.c
>> @@ -266,12 +266,26 @@ static inline void _debug_uart_init(void)
>>         serial_dout(&com_port->lcr, UART_LCRVAL);
>>  }
>>
>> +static inline int NS16550_read_baud_divisor(struct NS16550 *com_port)
>> +{
>> +       int ret;
>> +
>> +       serial_dout(&com_port->lcr, UART_LCR_BKSE | UART_LCRVAL);
>> +       ret = serial_din(&com_port->dll) & 0xff;
>> +       ret |= (serial_din(&com_port->dlm) & 0xff) << 8;
>> +       serial_dout(&com_port->lcr, UART_LCRVAL);
>> +
>> +       return ret;
>> +}
>> +
>>  static inline void _debug_uart_putc(int ch)
>>  {
>>         struct NS16550 *com_port = (struct NS16550 *)CONFIG_DEBUG_UART_BASE;
>>
>> -       while (!(serial_din(&com_port->lsr) & UART_LSR_THRE))
>> -               ;
>> +       while (!(serial_din(&com_port->lsr) & UART_LSR_THRE)) {
>> +               if (!NS16550_read_baud_divisor(com_port))
> 
> Unless there is a change that the read_baud_divisor will change while
> we're waiting for the character, could we move this check before the
> while statement?  This would reduce the check for the divisor to 1x
> and the while statement would only have one comparison to do.  I
> realize it's rather trivial, but the way I see it, there is no reason
> to do the while statement at all if the read_baud_divisor fails and
> there if there is a baud divisor, we should only need to check it
> once.

This looks like a massive hack -- what about having a flag which says
that the debug uart was/was not inited somewhere ?

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 5/6] serial: ns16550: fix debug uart putc called before init

[Andy Shevchenko]

On Fri, Aug 10, 2018 at 12:45 AM, Marek Vasut <marex@denx.de> wrote:
> On 08/09/2018 11:13 PM, Adam Ford wrote:
>> On Thu, Aug 9, 2018 at 2:08 PM Simon Goldschmidt
>> <simon.k.r.goldschmidt@gmail.com> wrote:
>>>
>>> If _debug_uart_putc() is called before _debug_uart_init(), the
>>> ns16550 debug uart driver hangs in a tight loop waiting for the
>>> tx FIFO to get empty.
>>>
>>> As this can happen via a printf sneaking in before the port calls
>>> debug_uart_init(), let's rather ignore characters before the debug
>>> uart is initialized.
>>>
>>> This is done by reading the baudrate divisor and aborting if is zero.

>>>  static inline void _debug_uart_putc(int ch)
>>>  {
>>>         struct NS16550 *com_port = (struct NS16550 *)CONFIG_DEBUG_UART_BASE;

>>> +       while (!(serial_din(&com_port->lsr) & UART_LSR_THRE)) {
>>> +               if (!NS16550_read_baud_divisor(com_port))
>>
>> Unless there is a change that the read_baud_divisor will change while
>> we're waiting for the character, could we move this check before the
>> while statement?  This would reduce the check for the divisor to 1x
>> and the while statement would only have one comparison to do.  I
>> realize it's rather trivial, but the way I see it, there is no reason
>> to do the while statement at all if the read_baud_divisor fails and
>> there if there is a baud divisor, we should only need to check it
>> once.
>
> This looks like a massive hack -- what about having a flag which says
> that the debug uart was/was not inited somewhere ?

Agree, why not to cache divisor value, for example, instead of doing slow I/O?

-- 
With Best Regards,
Andy Shevchenko

[U-Boot] [PATCH 5/6] serial: ns16550: fix debug uart putc called before init

[Marek Vasut]

On 08/10/2018 12:35 AM, Andy Shevchenko wrote:
> On Fri, Aug 10, 2018 at 12:45 AM, Marek Vasut <marex@denx.de> wrote:
>> On 08/09/2018 11:13 PM, Adam Ford wrote:
>>> On Thu, Aug 9, 2018 at 2:08 PM Simon Goldschmidt
>>> <simon.k.r.goldschmidt@gmail.com> wrote:
>>>>
>>>> If _debug_uart_putc() is called before _debug_uart_init(), the
>>>> ns16550 debug uart driver hangs in a tight loop waiting for the
>>>> tx FIFO to get empty.
>>>>
>>>> As this can happen via a printf sneaking in before the port calls
>>>> debug_uart_init(), let's rather ignore characters before the debug
>>>> uart is initialized.
>>>>
>>>> This is done by reading the baudrate divisor and aborting if is zero.
> 
>>>>  static inline void _debug_uart_putc(int ch)
>>>>  {
>>>>         struct NS16550 *com_port = (struct NS16550 *)CONFIG_DEBUG_UART_BASE;
> 
>>>> +       while (!(serial_din(&com_port->lsr) & UART_LSR_THRE)) {
>>>> +               if (!NS16550_read_baud_divisor(com_port))
>>>
>>> Unless there is a change that the read_baud_divisor will change while
>>> we're waiting for the character, could we move this check before the
>>> while statement?  This would reduce the check for the divisor to 1x
>>> and the while statement would only have one comparison to do.  I
>>> realize it's rather trivial, but the way I see it, there is no reason
>>> to do the while statement at all if the read_baud_divisor fails and
>>> there if there is a baud divisor, we should only need to check it
>>> once.
>>
>> This looks like a massive hack -- what about having a flag which says
>> that the debug uart was/was not inited somewhere ?
> 
> Agree, why not to cache divisor value, for example, instead of doing slow I/O?

But why do we care about the divisor at all ? The real problem I believe
is that someone can call debug UART print/read functions before it is
inited.

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 5/6] serial: ns16550: fix debug uart putc called before init

[Simon Goldschmidt]

On 10.08.2018 00:41, Marek Vasut wrote:
> On 08/10/2018 12:35 AM, Andy Shevchenko wrote:
>> On Fri, Aug 10, 2018 at 12:45 AM, Marek Vasut <marex@denx.de> wrote:
>>> On 08/09/2018 11:13 PM, Adam Ford wrote:
>>>> On Thu, Aug 9, 2018 at 2:08 PM Simon Goldschmidt
>>>> <simon.k.r.goldschmidt@gmail.com> wrote:
>>>>> If _debug_uart_putc() is called before _debug_uart_init(), the
>>>>> ns16550 debug uart driver hangs in a tight loop waiting for the
>>>>> tx FIFO to get empty.
>>>>>
>>>>> As this can happen via a printf sneaking in before the port calls
>>>>> debug_uart_init(), let's rather ignore characters before the debug
>>>>> uart is initialized.
>>>>>
>>>>> This is done by reading the baudrate divisor and aborting if is zero.
>>>>>   static inline void _debug_uart_putc(int ch)
>>>>>   {
>>>>>          struct NS16550 *com_port = (struct NS16550 *)CONFIG_DEBUG_UART_BASE;
>>>>> +       while (!(serial_din(&com_port->lsr) & UART_LSR_THRE)) {
>>>>> +               if (!NS16550_read_baud_divisor(com_port))
>>>> Unless there is a change that the read_baud_divisor will change while
>>>> we're waiting for the character, could we move this check before the
>>>> while statement?  This would reduce the check for the divisor to 1x
>>>> and the while statement would only have one comparison to do.  I
>>>> realize it's rather trivial, but the way I see it, there is no reason
>>>> to do the while statement at all if the read_baud_divisor fails and
>>>> there if there is a baud divisor, we should only need to check it
>>>> once.
>>> This looks like a massive hack -- what about having a flag which says
>>> that the debug uart was/was not inited somewhere ?
>> Agree, why not to cache divisor value, for example, instead of doing slow I/O?
> But why do we care about the divisor at all ?

Because if the divisor is zero, the UART is disabled.

> The real problem I believe
> is that someone can call debug UART print/read functions before it is
> inited.
>
I know this is a hack. I did it like that because I need something like 
this to get debug uart to work on socfpga gen5 (there always is a printf 
before debug uart init is possible).

A generic solution for all debug uarts would be better of course, but 
given the point in SPL runtime, we might have to add a field to 'gd' for 
that, or does a global variable work at that point already?


Simon

[U-Boot] [PATCH 5/6] serial: ns16550: fix debug uart putc called before init

[Marek Vasut]

On 08/10/2018 07:22 AM, Simon Goldschmidt wrote:
> On 10.08.2018 00:41, Marek Vasut wrote:
>> On 08/10/2018 12:35 AM, Andy Shevchenko wrote:
>>> On Fri, Aug 10, 2018 at 12:45 AM, Marek Vasut <marex@denx.de> wrote:
>>>> On 08/09/2018 11:13 PM, Adam Ford wrote:
>>>>> On Thu, Aug 9, 2018 at 2:08 PM Simon Goldschmidt
>>>>> <simon.k.r.goldschmidt@gmail.com> wrote:
>>>>>> If _debug_uart_putc() is called before _debug_uart_init(), the
>>>>>> ns16550 debug uart driver hangs in a tight loop waiting for the
>>>>>> tx FIFO to get empty.
>>>>>>
>>>>>> As this can happen via a printf sneaking in before the port calls
>>>>>> debug_uart_init(), let's rather ignore characters before the debug
>>>>>> uart is initialized.
>>>>>>
>>>>>> This is done by reading the baudrate divisor and aborting if is zero.
>>>>>>   static inline void _debug_uart_putc(int ch)
>>>>>>   {
>>>>>>          struct NS16550 *com_port = (struct NS16550
>>>>>> *)CONFIG_DEBUG_UART_BASE;
>>>>>> +       while (!(serial_din(&com_port->lsr) & UART_LSR_THRE)) {
>>>>>> +               if (!NS16550_read_baud_divisor(com_port))
>>>>> Unless there is a change that the read_baud_divisor will change while
>>>>> we're waiting for the character, could we move this check before the
>>>>> while statement?  This would reduce the check for the divisor to 1x
>>>>> and the while statement would only have one comparison to do.  I
>>>>> realize it's rather trivial, but the way I see it, there is no reason
>>>>> to do the while statement at all if the read_baud_divisor fails and
>>>>> there if there is a baud divisor, we should only need to check it
>>>>> once.
>>>> This looks like a massive hack -- what about having a flag which says
>>>> that the debug uart was/was not inited somewhere ?
>>> Agree, why not to cache divisor value, for example, instead of doing
>>> slow I/O?
>> But why do we care about the divisor at all ?
> 
> Because if the divisor is zero, the UART is disabled.
> 
>> The real problem I believe
>> is that someone can call debug UART print/read functions before it is
>> inited.
>>
> I know this is a hack. I did it like that because I need something like
> this to get debug uart to work on socfpga gen5 (there always is a printf
> before debug uart init is possible).
> 
> A generic solution for all debug uarts would be better of course, but
> given the point in SPL runtime, we might have to add a field to 'gd' for
> that, or does a global variable work at that point already?

GD field might be needed indeed.

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 5/6] serial: ns16550: fix debug uart putc called before init

[Simon Goldschmidt]

On 10.08.2018 11:51, Marek Vasut wrote:
> On 08/10/2018 07:22 AM, Simon Goldschmidt wrote:
>> On 10.08.2018 00:41, Marek Vasut wrote:
>>> On 08/10/2018 12:35 AM, Andy Shevchenko wrote:
>>>> On Fri, Aug 10, 2018 at 12:45 AM, Marek Vasut <marex@denx.de> wrote:
>>>>> On 08/09/2018 11:13 PM, Adam Ford wrote:
>>>>>> On Thu, Aug 9, 2018 at 2:08 PM Simon Goldschmidt
>>>>>> <simon.k.r.goldschmidt@gmail.com> wrote:
>>>>>>> If _debug_uart_putc() is called before _debug_uart_init(), the
>>>>>>> ns16550 debug uart driver hangs in a tight loop waiting for the
>>>>>>> tx FIFO to get empty.
>>>>>>>
>>>>>>> As this can happen via a printf sneaking in before the port calls
>>>>>>> debug_uart_init(), let's rather ignore characters before the debug
>>>>>>> uart is initialized.
>>>>>>>
>>>>>>> This is done by reading the baudrate divisor and aborting if is zero.
>>>>>>>    static inline void _debug_uart_putc(int ch)
>>>>>>>    {
>>>>>>>           struct NS16550 *com_port = (struct NS16550
>>>>>>> *)CONFIG_DEBUG_UART_BASE;
>>>>>>> +       while (!(serial_din(&com_port->lsr) & UART_LSR_THRE)) {
>>>>>>> +               if (!NS16550_read_baud_divisor(com_port))
>>>>>> Unless there is a change that the read_baud_divisor will change while
>>>>>> we're waiting for the character, could we move this check before the
>>>>>> while statement?  This would reduce the check for the divisor to 1x
>>>>>> and the while statement would only have one comparison to do.  I
>>>>>> realize it's rather trivial, but the way I see it, there is no reason
>>>>>> to do the while statement at all if the read_baud_divisor fails and
>>>>>> there if there is a baud divisor, we should only need to check it
>>>>>> once.
>>>>> This looks like a massive hack -- what about having a flag which says
>>>>> that the debug uart was/was not inited somewhere ?
>>>> Agree, why not to cache divisor value, for example, instead of doing
>>>> slow I/O?
>>> But why do we care about the divisor at all ?
>> Because if the divisor is zero, the UART is disabled.
>>
>>> The real problem I believe
>>> is that someone can call debug UART print/read functions before it is
>>> inited.
>>>
>> I know this is a hack. I did it like that because I need something like
>> this to get debug uart to work on socfpga gen5 (there always is a printf
>> before debug uart init is possible).
>>
>> A generic solution for all debug uarts would be better of course, but
>> given the point in SPL runtime, we might have to add a field to 'gd' for
>> that, or does a global variable work at that point already?
> GD field might be needed indeed.
>
Right. I'll drop this patch in the next version of the series and 
instead I'll try to work out something that works for all debug uarts 
drivers using a gd field.


Thanks,

Simon

[U-Boot] [PATCH 5/6] serial: ns16550: fix debug uart putc called before init

[Marek Vasut]

On 08/10/2018 01:37 PM, Simon Goldschmidt wrote:
> On 10.08.2018 11:51, Marek Vasut wrote:
>> On 08/10/2018 07:22 AM, Simon Goldschmidt wrote:
>>> On 10.08.2018 00:41, Marek Vasut wrote:
>>>> On 08/10/2018 12:35 AM, Andy Shevchenko wrote:
>>>>> On Fri, Aug 10, 2018 at 12:45 AM, Marek Vasut <marex@denx.de> wrote:
>>>>>> On 08/09/2018 11:13 PM, Adam Ford wrote:
>>>>>>> On Thu, Aug 9, 2018 at 2:08 PM Simon Goldschmidt
>>>>>>> <simon.k.r.goldschmidt@gmail.com> wrote:
>>>>>>>> If _debug_uart_putc() is called before _debug_uart_init(), the
>>>>>>>> ns16550 debug uart driver hangs in a tight loop waiting for the
>>>>>>>> tx FIFO to get empty.
>>>>>>>>
>>>>>>>> As this can happen via a printf sneaking in before the port calls
>>>>>>>> debug_uart_init(), let's rather ignore characters before the debug
>>>>>>>> uart is initialized.
>>>>>>>>
>>>>>>>> This is done by reading the baudrate divisor and aborting if is
>>>>>>>> zero.
>>>>>>>>    static inline void _debug_uart_putc(int ch)
>>>>>>>>    {
>>>>>>>>           struct NS16550 *com_port = (struct NS16550
>>>>>>>> *)CONFIG_DEBUG_UART_BASE;
>>>>>>>> +       while (!(serial_din(&com_port->lsr) & UART_LSR_THRE)) {
>>>>>>>> +               if (!NS16550_read_baud_divisor(com_port))
>>>>>>> Unless there is a change that the read_baud_divisor will change
>>>>>>> while
>>>>>>> we're waiting for the character, could we move this check before the
>>>>>>> while statement?  This would reduce the check for the divisor to 1x
>>>>>>> and the while statement would only have one comparison to do.  I
>>>>>>> realize it's rather trivial, but the way I see it, there is no
>>>>>>> reason
>>>>>>> to do the while statement at all if the read_baud_divisor fails and
>>>>>>> there if there is a baud divisor, we should only need to check it
>>>>>>> once.
>>>>>> This looks like a massive hack -- what about having a flag which says
>>>>>> that the debug uart was/was not inited somewhere ?
>>>>> Agree, why not to cache divisor value, for example, instead of doing
>>>>> slow I/O?
>>>> But why do we care about the divisor at all ?
>>> Because if the divisor is zero, the UART is disabled.
>>>
>>>> The real problem I believe
>>>> is that someone can call debug UART print/read functions before it is
>>>> inited.
>>>>
>>> I know this is a hack. I did it like that because I need something like
>>> this to get debug uart to work on socfpga gen5 (there always is a printf
>>> before debug uart init is possible).
>>>
>>> A generic solution for all debug uarts would be better of course, but
>>> given the point in SPL runtime, we might have to add a field to 'gd' for
>>> that, or does a global variable work at that point already?
>> GD field might be needed indeed.
>>
> Right. I'll drop this patch in the next version of the series and
> instead I'll try to work out something that works for all debug uarts
> drivers using a gd field.

Thanks, much appreciated :)

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 1/6] arm: socfpga: fix SPL on gen5 after moving to DM serial

[Simon Goldschmidt]

On 09.08.2018 23:42, Marek Vasut wrote:
> On 08/09/2018 09:04 PM, Simon Goldschmidt wrote:
>> There were NULL pointers dereferenced because DM was used
>> too early without correct initialization:
>> - malloc_simple returned NULL when called from preloader_console_init()
>>    because gd->malloc_limit was 0
>> - uclass_add dereferenced gd->uclass_root members which were NULL because
>>    dm_init (or one of its relatives) has not been called.
>>
>> All this is fixed by calling spl_early_init before calling
>> preloader_console_init.
>>
>> This fixes commit 73172753f4f3 ("ARM: socfpga: Convert to DM serial")
>>
>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
>> ---
>> v2:
>> - Don't remove gd->malloc_base assignment at the end of board_init_f()
>>    (moved to an extra patch)
>>
>>   arch/arm/mach-socfpga/spl_gen5.c | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/arch/arm/mach-socfpga/spl_gen5.c b/arch/arm/mach-socfpga/spl_gen5.c
>> index d6fe7d35af..9bdfaa3c1e 100644
>> --- a/arch/arm/mach-socfpga/spl_gen5.c
>> +++ b/arch/arm/mach-socfpga/spl_gen5.c
>> @@ -86,6 +86,7 @@ void board_init_f(ulong dummy)
>>   	const struct cm_config *cm_default_cfg = cm_get_default_config();
>>   	unsigned long sdram_size;
>>   	unsigned long reg;
>> +	int ret;
>>   
>>   	/*
>>   	 * First C code to run. Clear fake OCRAM ECC first as SBE
>> @@ -152,6 +153,12 @@ void board_init_f(ulong dummy)
>>   	/* unfreeze / thaw all IO banks */
>>   	sys_mgr_frzctrl_thaw_req();
>>   
>> +	ret = spl_early_init();
> Uh, but isn't this called from common/spl/spl.c ? I suspect the SoCFPGA
> SPL is a bit weird.

Ehrm, I copied this from spl_s10.c, but other boards seem to do this, 
too. Honestly, I don't know how any SPL can use DM serial without this 
being called. Maybe other SPLs initialize the serial port later (not in 
board_init_f).

common/spl/spl.c calls spl_init(), which also calls the part that 
spl_early_init() calls.

I can only take other SPLs as reference and from reading all the code, I 
think this should be good.

Simon

>
>> +	if (ret) {
>> +		debug("spl_early_init() failed: %d\n", ret);
>> +		hang();
>> +	}
>> +
>>   	/* enable console uart printing */
>>   	preloader_console_init();
>>   
>>
>

[U-Boot] [PATCH 1/6] arm: socfpga: fix SPL on gen5 after moving to DM serial

[Marek Vasut]

On 08/10/2018 02:39 PM, Simon Goldschmidt wrote:
> On 09.08.2018 23:42, Marek Vasut wrote:
>> On 08/09/2018 09:04 PM, Simon Goldschmidt wrote:
>>> There were NULL pointers dereferenced because DM was used
>>> too early without correct initialization:
>>> - malloc_simple returned NULL when called from preloader_console_init()
>>>    because gd->malloc_limit was 0
>>> - uclass_add dereferenced gd->uclass_root members which were NULL
>>> because
>>>    dm_init (or one of its relatives) has not been called.
>>>
>>> All this is fixed by calling spl_early_init before calling
>>> preloader_console_init.
>>>
>>> This fixes commit 73172753f4f3 ("ARM: socfpga: Convert to DM serial")
>>>
>>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
>>> ---
>>> v2:
>>> - Don't remove gd->malloc_base assignment at the end of board_init_f()
>>>    (moved to an extra patch)
>>>
>>>   arch/arm/mach-socfpga/spl_gen5.c | 7 +++++++
>>>   1 file changed, 7 insertions(+)
>>>
>>> diff --git a/arch/arm/mach-socfpga/spl_gen5.c
>>> b/arch/arm/mach-socfpga/spl_gen5.c
>>> index d6fe7d35af..9bdfaa3c1e 100644
>>> --- a/arch/arm/mach-socfpga/spl_gen5.c
>>> +++ b/arch/arm/mach-socfpga/spl_gen5.c
>>> @@ -86,6 +86,7 @@ void board_init_f(ulong dummy)
>>>       const struct cm_config *cm_default_cfg = cm_get_default_config();
>>>       unsigned long sdram_size;
>>>       unsigned long reg;
>>> +    int ret;
>>>         /*
>>>        * First C code to run. Clear fake OCRAM ECC first as SBE
>>> @@ -152,6 +153,12 @@ void board_init_f(ulong dummy)
>>>       /* unfreeze / thaw all IO banks */
>>>       sys_mgr_frzctrl_thaw_req();
>>>   +    ret = spl_early_init();
>> Uh, but isn't this called from common/spl/spl.c ? I suspect the SoCFPGA
>> SPL is a bit weird.
> 
> Ehrm, I copied this from spl_s10.c, but other boards seem to do this,
> too. Honestly, I don't know how any SPL can use DM serial without this
> being called. Maybe other SPLs initialize the serial port later (not in
> board_init_f).

I mean, spl_early_init() is called in common/spl/spl.c , which is common
code. Maybe the socfpga SPL is structured in a really weird way (I think
it is).

> common/spl/spl.c calls spl_init(), which also calls the part that
> spl_early_init() calls.
> 
> I can only take other SPLs as reference and from reading all the code, I
> think this should be good.

Right

> Simon
> 
>>
>>> +    if (ret) {
>>> +        debug("spl_early_init() failed: %d\n", ret);
>>> +        hang();
>>> +    }
>>> +
>>>       /* enable console uart printing */
>>>       preloader_console_init();
>>>  
>>
> 


-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 1/6] arm: socfpga: fix SPL on gen5 after moving to DM serial

[Simon Goldschmidt]

On 10.08.2018 14:41, Marek Vasut wrote:
> On 08/10/2018 02:39 PM, Simon Goldschmidt wrote:
>> On 09.08.2018 23:42, Marek Vasut wrote:
>>> On 08/09/2018 09:04 PM, Simon Goldschmidt wrote:
>>>> There were NULL pointers dereferenced because DM was used
>>>> too early without correct initialization:
>>>> - malloc_simple returned NULL when called from preloader_console_init()
>>>>     because gd->malloc_limit was 0
>>>> - uclass_add dereferenced gd->uclass_root members which were NULL
>>>> because
>>>>     dm_init (or one of its relatives) has not been called.
>>>>
>>>> All this is fixed by calling spl_early_init before calling
>>>> preloader_console_init.
>>>>
>>>> This fixes commit 73172753f4f3 ("ARM: socfpga: Convert to DM serial")
>>>>
>>>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
>>>> ---
>>>> v2:
>>>> - Don't remove gd->malloc_base assignment at the end of board_init_f()
>>>>     (moved to an extra patch)
>>>>
>>>>    arch/arm/mach-socfpga/spl_gen5.c | 7 +++++++
>>>>    1 file changed, 7 insertions(+)
>>>>
>>>> diff --git a/arch/arm/mach-socfpga/spl_gen5.c
>>>> b/arch/arm/mach-socfpga/spl_gen5.c
>>>> index d6fe7d35af..9bdfaa3c1e 100644
>>>> --- a/arch/arm/mach-socfpga/spl_gen5.c
>>>> +++ b/arch/arm/mach-socfpga/spl_gen5.c
>>>> @@ -86,6 +86,7 @@ void board_init_f(ulong dummy)
>>>>        const struct cm_config *cm_default_cfg = cm_get_default_config();
>>>>        unsigned long sdram_size;
>>>>        unsigned long reg;
>>>> +    int ret;
>>>>          /*
>>>>         * First C code to run. Clear fake OCRAM ECC first as SBE
>>>> @@ -152,6 +153,12 @@ void board_init_f(ulong dummy)
>>>>        /* unfreeze / thaw all IO banks */
>>>>        sys_mgr_frzctrl_thaw_req();
>>>>    +    ret = spl_early_init();
>>> Uh, but isn't this called from common/spl/spl.c ? I suspect the SoCFPGA
>>> SPL is a bit weird.
>> Ehrm, I copied this from spl_s10.c, but other boards seem to do this,
>> too. Honestly, I don't know how any SPL can use DM serial without this
>> being called. Maybe other SPLs initialize the serial port later (not in
>> board_init_f).
> I mean, spl_early_init() is called in common/spl/spl.c , which is common
> code. Maybe the socfpga SPL is structured in a really weird way (I think
> it is).

Not exactly: common/spl/spl.c calls spl_common_init(), just like 
spl_early_init() does. Given the names, I think spl_early_init() is 
meant to be called early, e.g. from board_init_f() ;-)

Oh, I just saw spl_common_init() emits a debug print "spl_early_init()", 
so that might have tricked you...?

>> common/spl/spl.c calls spl_init(), which also calls the part that
>> spl_early_init() calls.
>>
>> I can only take other SPLs as reference and from reading all the code, I
>> think this should be good.
> Right

So is this change OK for v2018.09 once I fix the dts thing? Given that 
v2018.07 is broken for socfpga gen5, it would be good to merge it 
before. I can prepare a v3 of the series with only minimal changes in 
the socfpga files and resend the rest as detached patches.

Simon

>>>> +    if (ret) {
>>>> +        debug("spl_early_init() failed: %d\n", ret);
>>>> +        hang();
>>>> +    }
>>>> +
>>>>        /* enable console uart printing */
>>>>        preloader_console_init();
>>>>   
>

[U-Boot] [PATCH 1/6] arm: socfpga: fix SPL on gen5 after moving to DM serial

[Marek Vasut]

On 08/10/2018 02:55 PM, Simon Goldschmidt wrote:
> On 10.08.2018 14:41, Marek Vasut wrote:
>> On 08/10/2018 02:39 PM, Simon Goldschmidt wrote:
>>> On 09.08.2018 23:42, Marek Vasut wrote:
>>>> On 08/09/2018 09:04 PM, Simon Goldschmidt wrote:
>>>>> There were NULL pointers dereferenced because DM was used
>>>>> too early without correct initialization:
>>>>> - malloc_simple returned NULL when called from
>>>>> preloader_console_init()
>>>>>     because gd->malloc_limit was 0
>>>>> - uclass_add dereferenced gd->uclass_root members which were NULL
>>>>> because
>>>>>     dm_init (or one of its relatives) has not been called.
>>>>>
>>>>> All this is fixed by calling spl_early_init before calling
>>>>> preloader_console_init.
>>>>>
>>>>> This fixes commit 73172753f4f3 ("ARM: socfpga: Convert to DM serial")
>>>>>
>>>>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
>>>>> ---
>>>>> v2:
>>>>> - Don't remove gd->malloc_base assignment at the end of board_init_f()
>>>>>     (moved to an extra patch)
>>>>>
>>>>>    arch/arm/mach-socfpga/spl_gen5.c | 7 +++++++
>>>>>    1 file changed, 7 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm/mach-socfpga/spl_gen5.c
>>>>> b/arch/arm/mach-socfpga/spl_gen5.c
>>>>> index d6fe7d35af..9bdfaa3c1e 100644
>>>>> --- a/arch/arm/mach-socfpga/spl_gen5.c
>>>>> +++ b/arch/arm/mach-socfpga/spl_gen5.c
>>>>> @@ -86,6 +86,7 @@ void board_init_f(ulong dummy)
>>>>>        const struct cm_config *cm_default_cfg =
>>>>> cm_get_default_config();
>>>>>        unsigned long sdram_size;
>>>>>        unsigned long reg;
>>>>> +    int ret;
>>>>>          /*
>>>>>         * First C code to run. Clear fake OCRAM ECC first as SBE
>>>>> @@ -152,6 +153,12 @@ void board_init_f(ulong dummy)
>>>>>        /* unfreeze / thaw all IO banks */
>>>>>        sys_mgr_frzctrl_thaw_req();
>>>>>    +    ret = spl_early_init();
>>>> Uh, but isn't this called from common/spl/spl.c ? I suspect the SoCFPGA
>>>> SPL is a bit weird.
>>> Ehrm, I copied this from spl_s10.c, but other boards seem to do this,
>>> too. Honestly, I don't know how any SPL can use DM serial without this
>>> being called. Maybe other SPLs initialize the serial port later (not in
>>> board_init_f).
>> I mean, spl_early_init() is called in common/spl/spl.c , which is common
>> code. Maybe the socfpga SPL is structured in a really weird way (I think
>> it is).
> 
> Not exactly: common/spl/spl.c calls spl_common_init(), just like
> spl_early_init() does. Given the names, I think spl_early_init() is
> meant to be called early, e.g. from board_init_f() ;-)
> 
> Oh, I just saw spl_common_init() emits a debug print "spl_early_init()",
> so that might have tricked you...?

Ah, yes, I think so.

>>> common/spl/spl.c calls spl_init(), which also calls the part that
>>> spl_early_init() calls.
>>>
>>> I can only take other SPLs as reference and from reading all the code, I
>>> think this should be good.
>> Right
> 
> So is this change OK for v2018.09 once I fix the dts thing? Given that
> v2018.07 is broken for socfpga gen5, it would be good to merge it
> before. I can prepare a v3 of the series with only minimal changes in
> the socfpga files and resend the rest as detached patches.
Looks good, yes.

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 5/6] serial: ns16550: fix debug uart putc called before init

[Simon Glass]

On 9 August 2018 at 13:04, Simon Goldschmidt
<simon.k.r.goldschmidt@gmail.com> wrote:
> If _debug_uart_putc() is called before _debug_uart_init(), the
> ns16550 debug uart driver hangs in a tight loop waiting for the
> tx FIFO to get empty.
>
> As this can happen via a printf sneaking in before the port calls
> debug_uart_init(), let's rather ignore characters before the debug
> uart is initialized.
>
> This is done by reading the baudrate divisor and aborting if is zero.
>
> Tested on socfpga_cyclone5_socrates.
>
> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> ---
> v2:
>  - this patch is new in v2 of the series. It replaces the printf/debug
>   change in reset_manager_gen5.c from v1
>
>  drivers/serial/ns16550.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)

We cannot use global_data before it is set up, so I think this is the
best solution.

Acked-by: Simon Glass <sjg@chromium.org>

Regards,
Simon

[U-Boot] [PATCH 5/6] serial: ns16550: fix debug uart putc called before init

[Marek Vasut]

On 10/19/2018 05:25 AM, Simon Glass wrote:
> On 9 August 2018 at 13:04, Simon Goldschmidt
> <simon.k.r.goldschmidt@gmail.com> wrote:
>> If _debug_uart_putc() is called before _debug_uart_init(), the
>> ns16550 debug uart driver hangs in a tight loop waiting for the
>> tx FIFO to get empty.
>>
>> As this can happen via a printf sneaking in before the port calls
>> debug_uart_init(), let's rather ignore characters before the debug
>> uart is initialized.
>>
>> This is done by reading the baudrate divisor and aborting if is zero.
>>
>> Tested on socfpga_cyclone5_socrates.
>>
>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
>> ---
>> v2:
>>  - this patch is new in v2 of the series. It replaces the printf/debug
>>   change in reset_manager_gen5.c from v1
>>
>>  drivers/serial/ns16550.c | 18 ++++++++++++++++--
>>  1 file changed, 16 insertions(+), 2 deletions(-)
> 
> We cannot use global_data before it is set up, so I think this is the
> best solution.
> 
> Acked-by: Simon Glass <sjg@chromium.org>

So there's no GD available when using debug uart ? Hum.

btw. Does the NS16550_read_baud_divisor() need to be called within the
while loop ?

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 5/6] serial: ns16550: fix debug uart putc called before init

[Simon Goldschmidt]

On Tue, Oct 23, 2018 at 11:01 AM Marek Vasut <marex@denx.de> wrote:
>
> On 10/19/2018 05:25 AM, Simon Glass wrote:
> > On 9 August 2018 at 13:04, Simon Goldschmidt
> > <simon.k.r.goldschmidt@gmail.com> wrote:
> >> If _debug_uart_putc() is called before _debug_uart_init(), the
> >> ns16550 debug uart driver hangs in a tight loop waiting for the
> >> tx FIFO to get empty.
> >>
> >> As this can happen via a printf sneaking in before the port calls
> >> debug_uart_init(), let's rather ignore characters before the debug
> >> uart is initialized.
> >>
> >> This is done by reading the baudrate divisor and aborting if is zero.
> >>
> >> Tested on socfpga_cyclone5_socrates.
> >>
> >> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> >> ---
> >> v2:
> >>  - this patch is new in v2 of the series. It replaces the printf/debug
> >>   change in reset_manager_gen5.c from v1
> >>
> >>  drivers/serial/ns16550.c | 18 ++++++++++++++++--
> >>  1 file changed, 16 insertions(+), 2 deletions(-)
> >
> > We cannot use global_data before it is set up, so I think this is the
> > best solution.
> >
> > Acked-by: Simon Glass <sjg@chromium.org>
>
> So there's no GD available when using debug uart ? Hum.
>
> btw. Does the NS16550_read_baud_divisor() need to be called within the
> while loop ?

No. I just decided to put it there so that it is not executed unless
we wait in a tight loop anyway.
So if the transmit buffer is empty, code flow is unchanged by this
patch. Only if it is not empty, the baud divisor is read. But in this
case, we would cycle the while loop a few hundred times, anyway, I
guess...


Simon

[U-Boot] [PATCH 5/6] serial: ns16550: fix debug uart putc called before init

[sjg at google.com]

On Tue, Oct 23, 2018 at 11:01 AM Marek Vasut <marex@denx.de> wrote:
>
> On 10/19/2018 05:25 AM, Simon Glass wrote:
> > On 9 August 2018 at 13:04, Simon Goldschmidt
> > <simon.k.r.goldschmidt@gmail.com> wrote:
> >> If _debug_uart_putc() is called before _debug_uart_init(), the
> >> ns16550 debug uart driver hangs in a tight loop waiting for the
> >> tx FIFO to get empty.
> >>
> >> As this can happen via a printf sneaking in before the port calls
> >> debug_uart_init(), let's rather ignore characters before the debug
> >> uart is initialized.
> >>
> >> This is done by reading the baudrate divisor and aborting if is zero.
> >>
> >> Tested on socfpga_cyclone5_socrates.
> >>
> >> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> >> ---
> >> v2:
> >>  - this patch is new in v2 of the series. It replaces the printf/debug
> >>   change in reset_manager_gen5.c from v1
> >>
> >>  drivers/serial/ns16550.c | 18 ++++++++++++++++--
> >>  1 file changed, 16 insertions(+), 2 deletions(-)
> >
> > We cannot use global_data before it is set up, so I think this is the
> > best solution.
> >
> > Acked-by: Simon Glass <sjg@chromium.org>
>
> So there's no GD available when using debug uart ? Hum.
>
> btw. Does the NS16550_read_baud_divisor() need to be called within the
> while loop ?

No. I just decided to put it there so that it is not executed unless
we wait in a tight loop anyway.
So if the transmit buffer is empty, code flow is unchanged by this
patch. Only if it is not empty, the baud divisor is read. But in this
case, we would cycle the while loop a few hundred times, anyway, I
guess...


Simon

Applied to u-boot-dm/next, thanks!