[U-Boot] [PATCH 0/5] Get socfpga gen5 SPL working again.
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(-)
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); }
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);
}
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
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@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 {
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@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 {
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@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
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@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@lists.denx.de https://lists.denx.de/listinfo/u-boot
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@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@lists.denx.de https://lists.denx.de/listinfo/u-boot
-- Emmanuel Vadot manu@bidouilliste.com manu@freebsd.org
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
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);
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);
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 oferror. */
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
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
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.
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
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; }
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
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(-)
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();
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();
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();
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();
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();
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.
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@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 {
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@ffc03000 {
[...]
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); }
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 :)
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);
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); }
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@lists.denx.de https://lists.denx.de/listinfo/u-boot
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 ?
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?
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.
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
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.
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
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 :)
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
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 ?
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
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!
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; }
participants (7)
-
Adam Ford -
Andy Shevchenko -
Emmanuel Vadot -
Marek Vasut -
Simon Glass -
Simon Goldschmidt -
sjg＠google.com