[U-Boot] [PATCH] Revert "sunxi: board: Print error after power initialization fails"
From: Karl Palsson karlp@tweak.net.au
This reverts commit a8011eb84dfac5187cebf00ed8bc981bdb5c1fa1
This causes DRAM init failures on (at least) * allwinner h3 nanopi-duo2 * allwinner h2+ orangepi zero v1.1
Signed-off-by: Karl Palsson karlp@tweak.net.au ---
Ideally, this should get into 2019.01, so that this release is not broken on those targets.
board/sunxi/board.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/board/sunxi/board.c b/board/sunxi/board.c index 917f5b18f6..f022f365e9 100644 --- a/board/sunxi/board.c +++ b/board/sunxi/board.c @@ -637,6 +637,13 @@ void sunxi_board_init(void) power_failed |= axp_set_sw(IS_ENABLED(CONFIG_AXP_SW_ON)); #endif #endif + printf("DRAM:"); + gd->ram_size = sunxi_dram_init(); + printf(" %d MiB\n", (int)(gd->ram_size >> 20)); + if (!gd->ram_size) + hang(); + + sunxi_spl_store_dram_size(gd->ram_size);
/* * Only clock up the CPU to full speed if we are reasonably @@ -645,16 +652,7 @@ void sunxi_board_init(void) if (!power_failed) clock_set_pll1(CONFIG_SYS_CLK_FREQ); else - printf("Error setting up the power controller.\n" - "CPU frequency not set.\n"); - - printf("DRAM:"); - gd->ram_size = sunxi_dram_init(); - printf(" %d MiB\n", (int)(gd->ram_size >> 20)); - if (!gd->ram_size) - hang(); - - sunxi_spl_store_dram_size(gd->ram_size); + printf("Failed to set core voltage! Can't set CPU frequency\n"); } #endif
On Tue, Dec 18, 2018 at 4:09 PM karlp@tweak.net.au wrote:
From: Karl Palsson karlp@tweak.net.au
This reverts commit a8011eb84dfac5187cebf00ed8bc981bdb5c1fa1
This causes DRAM init failures on (at least)
- allwinner h3 nanopi-duo2
- allwinner h2+ orangepi zero v1.1
Signed-off-by: Karl Palsson karlp@tweak.net.au
Ideally, this should get into 2019.01, so that this release is not broken on those targets.
Better to understand the issue here, since I have BPI-M2+ which boots fine w/o this revert.
Jagan Teki jagan@amarulasolutions.com wrote:
On Tue, Dec 18, 2018 at 4:09 PM karlp@tweak.net.au wrote:
From: Karl Palsson karlp@tweak.net.au
This reverts commit a8011eb84dfac5187cebf00ed8bc981bdb5c1fa1
This causes DRAM init failures on (at least)
- allwinner h3 nanopi-duo2
- allwinner h2+ orangepi zero v1.1
Signed-off-by: Karl Palsson karlp@tweak.net.au
Ideally, this should get into 2019.01, so that this release is not broken on those targets.
Better to understand the issue here, since I have BPI-M2+ which boots fine w/o this revert.
Fine with me, I posted about it as a "please look into this" a week ago, but sending a revert draws more attention it seems :) I only have a duo2 to test with for h3/h2+ devices, (plaes confirmed on the orangepi zero) but it's 100% works/doesn't work before/after this commit for me, not a question of reliability or "fails sometimes"
Cheers, Karl P
On Tue, Dec 18, 2018 at 6:02 PM Karl Palsson karlp@tweak.net.au wrote:
Jagan Teki jagan@amarulasolutions.com wrote:
On Tue, Dec 18, 2018 at 4:09 PM karlp@tweak.net.au wrote:
From: Karl Palsson karlp@tweak.net.au
This reverts commit a8011eb84dfac5187cebf00ed8bc981bdb5c1fa1
This causes DRAM init failures on (at least)
- allwinner h3 nanopi-duo2
- allwinner h2+ orangepi zero v1.1
Signed-off-by: Karl Palsson karlp@tweak.net.au
Ideally, this should get into 2019.01, so that this release is not broken on those targets.
Better to understand the issue here, since I have BPI-M2+ which boots fine w/o this revert.
Fine with me, I posted about it as a "please look into this" a week ago, but sending a revert draws more attention it seems :) I
It's 4 days before, "U-Boot] sunxi: regression in dram init for h3 board"
7 days is not 4-days :)
I would expect some debugging on this during these days and find the real root cause.
only have a duo2 to test with for h3/h2+ devices, (plaes confirmed on the orangepi zero) but it's 100% works/doesn't work before/after this commit for me, not a question of reliability or "fails sometimes"
BPI-M2+ is H3 board.
Jagan Teki jagan@amarulasolutions.com wrote:
It's 4 days before, "U-Boot] sunxi: regression in dram init for h3 board"
7 days is not 4-days :)
True, it was last week, not a week ago.
I would expect some debugging on this during these days and find the real root cause.
Just so there's no unmet expectations here, I'm not doing any _debugging_ of this. (Nor do I have any planned) Given that this is a regression in working devices, I would _presume_ (quite likely wrongly) that the honus of debugging this would be on the people who suggested this patch, not people affected by it. I'm happy to test things, but I have zero experience with dram init, (only marginally greater than zero experience with uboot overall) and no sane methods for debugging this further, nor enough devices to give any test confidence that any debugging I could do would be useful.
Sincerely, Karl P
Hi Karl
On Tue, Dec 18, 2018 at 7:12 PM Karl Palsson karlp@tweak.net.au wrote:
Jagan Teki jagan@amarulasolutions.com wrote:
It's 4 days before, "U-Boot] sunxi: regression in dram init for h3 board"
7 days is not 4-days :)
True, it was last week, not a week ago.
I would expect some debugging on this during these days and find the real root cause.
Just so there's no unmet expectations here, I'm not doing any _debugging_ of this. (Nor do I have any planned) Given that this is a regression in working devices, I would _presume_ (quite likely wrongly) that the honus of debugging this would be on the people who suggested this patch, not people affected by it. I'm happy to test things, but I have zero experience with dram init, (only marginally greater than zero experience with uboot overall) and no sane methods for debugging this further, nor enough devices to give any test confidence that any debugging I could do would be useful.
Do you have error after relocation or before relocation? Does it go outside and print DRAM size?
Michael
Sincerely, Karl P_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Michael Nazzareno Trimarchi michael@amarulasolutions.com wrote:
Do you have error after relocation or before relocation? Does it go outside and print DRAM size?
The only output I get is what I posted in https://lists.denx.de/pipermail/u-boot/2018-December/352036.html (bad output below)
Bad behaviour:
U-Boot spl 2019.01-rc1-00101-ga8011eb84d (Dec 13 2018 - 16:44:48 +0000) DRAM: 0 MiB ### ERROR ### Please RESET the board ###
If there's debug options you'd like turned on and rebuilt, I can certainly do that.
Cheers, Karl P
On Tue, Dec 18, 2018 at 09:17:27PM +0100, Michael Nazzareno Trimarchi wrote:
Hi Karl
On Tue, Dec 18, 2018 at 7:12 PM Karl Palsson karlp@tweak.net.au wrote:
Jagan Teki jagan@amarulasolutions.com wrote:
It's 4 days before, "U-Boot] sunxi: regression in dram init for h3 board"
7 days is not 4-days :)
True, it was last week, not a week ago.
I would expect some debugging on this during these days and find the real root cause.
Just so there's no unmet expectations here, I'm not doing any _debugging_ of this. (Nor do I have any planned) Given that this is a regression in working devices, I would _presume_ (quite likely wrongly) that the honus of debugging this would be on the people who suggested this patch, not people affected by it. I'm happy to test things, but I have zero experience with dram init, (only marginally greater than zero experience with uboot overall) and no sane methods for debugging this further, nor enough devices to give any test confidence that any debugging I could do would be useful.
Do you have error after relocation or before relocation? Does it go outside and print DRAM size?
When I tested, I replaced hang there with reboot and got following outputs after multiple FEL uboot uploads commands:
[snip] U-Boot SPL 2019.01-rc1-00341-gb2f6eec007-dirty (Dec 18 2018 - 09:31:01 +0200) DRAM: 0 MiB resetting ...
U-Boot SPL 2019.01-rc1-00341-gb2f6eec007-dirty (Dec 18 2018 - 09:31:01 +0200) DRAM: 2048 MiB Trying to boot from FEL
U-Boot SPL 2019.01-rc1-00341-gb2f6eec007-dirty (Dec 18 2018 - 09:31:01 +0200) DRAM: 1024 MiB Trying to boot from FEL [/snip]
Michael
Sincerely, Karl P_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
-- | Michael Nazzareno Trimarchi Amarula Solutions BV | | COO - Founder Cruquiuskade 47 | | +31(0)851119172 Amsterdam 1018 AM NL | | [`as] http://www.amarulasolutions.com |
-- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
On 18/12/2018 12:06, Jagan Teki wrote:
On Tue, Dec 18, 2018 at 4:09 PM karlp@tweak.net.au wrote:
From: Karl Palsson karlp@tweak.net.au
This reverts commit a8011eb84dfac5187cebf00ed8bc981bdb5c1fa1
This causes DRAM init failures on (at least)
- allwinner h3 nanopi-duo2
- allwinner h2+ orangepi zero v1.1
Signed-off-by: Karl Palsson karlp@tweak.net.au
Ideally, this should get into 2019.01, so that this release is not broken on those targets.
Better to understand the issue here, since I have BPI-M2+ which boots fine w/o this revert.
Could this be a .bss issue? This lies in DRAM and is thus only available *after* DRAM init. IIRC we silently rely on not accessing anything in .bss before the DRAM is up, see 59d07ee08e85 for instance. I don't immediately spot any .bss usage in clock_set_pll1(), though.
Or is the 1GHz CPU clock speed too fast for the DRAM init? If I am not mistaken, we run with 24MHz before, so there might be some "natural" delay in some setup routines. Some DRAM chips or board layout might be more susceptible to this than others, which might explain why it only fails on some boards.
So if the original patch is about bailing out on error early, can't we just do *that* before the DRAM init, keeping the CPU clock setup still after DRAM?
Cheers, Andre.
On 19/12/2018 00:51, André Przywara wrote:
On 18/12/2018 12:06, Jagan Teki wrote:
On Tue, Dec 18, 2018 at 4:09 PM karlp@tweak.net.au wrote:
From: Karl Palsson karlp@tweak.net.au
This reverts commit a8011eb84dfac5187cebf00ed8bc981bdb5c1fa1
This causes DRAM init failures on (at least)
- allwinner h3 nanopi-duo2
- allwinner h2+ orangepi zero v1.1
Signed-off-by: Karl Palsson karlp@tweak.net.au
Ideally, this should get into 2019.01, so that this release is not broken on those targets.
Better to understand the issue here, since I have BPI-M2+ which boots fine w/o this revert.
Could this be a .bss issue? This lies in DRAM and is thus only available *after* DRAM init. IIRC we silently rely on not accessing anything in .bss before the DRAM is up, see 59d07ee08e85 for instance. I don't immediately spot any .bss usage in clock_set_pll1(), though.
Or is the 1GHz CPU clock speed too fast for the DRAM init? If I am not mistaken, we run with 24MHz before, so there might be some "natural" delay in some setup routines. Some DRAM chips or board layout might be more susceptible to this than others, which might explain why it only fails on some boards.
Just did some testing on my OrangePi Zero: if I force the CPU frequency to 408 MHz, it works, but fails with the standard 1008 MHz. So this smells that we indeed miss some delays in the DRAM setup code, which sounds tedious to find. There are a number of delays, but those are fine as they are timer controlled, so independent from the CPU frequency.
For now I think that reverting would be the easiest solution. Somewhat in contrast to what the commit message says, we don't really change anything in terms of error *checking*, as the code carries on anyways (just without increasing the CPU frequency). The only real difference is the order of CPU clock setup and DRAM init.
Karl, can you please amend the commit message to mention the CPU frequency issue?
So if the original patch is about bailing out on error early, can't we just do *that* before the DRAM init, keeping the CPU clock setup still after DRAM?
Just checked, this works as well, but is a bit pointless. Just reverting this and meanwhile checking the DRAM init code seems the easier solution.
Cheers, Andre
Hi André
On Wed, Dec 19, 2018 at 2:46 AM André Przywara andre.przywara@arm.com wrote:
On 19/12/2018 00:51, André Przywara wrote:
On 18/12/2018 12:06, Jagan Teki wrote:
On Tue, Dec 18, 2018 at 4:09 PM karlp@tweak.net.au wrote:
From: Karl Palsson karlp@tweak.net.au
This reverts commit a8011eb84dfac5187cebf00ed8bc981bdb5c1fa1
This causes DRAM init failures on (at least)
- allwinner h3 nanopi-duo2
- allwinner h2+ orangepi zero v1.1
Signed-off-by: Karl Palsson karlp@tweak.net.au
Ideally, this should get into 2019.01, so that this release is not broken on those targets.
Better to understand the issue here, since I have BPI-M2+ which boots fine w/o this revert.
Could this be a .bss issue? This lies in DRAM and is thus only available *after* DRAM init. IIRC we silently rely on not accessing anything in .bss before the DRAM is up, see 59d07ee08e85 for instance. I don't immediately spot any .bss usage in clock_set_pll1(), though.
Or is the 1GHz CPU clock speed too fast for the DRAM init? If I am not mistaken, we run with 24MHz before, so there might be some "natural" delay in some setup routines. Some DRAM chips or board layout might be more susceptible to this than others, which might explain why it only fails on some boards.
Just did some testing on my OrangePi Zero: if I force the CPU frequency to 408 MHz, it works, but fails with the standard 1008 MHz. So this smells that we indeed miss some delays in the DRAM setup code, which sounds tedious to find. There are a number of delays, but those are fine as they are timer controlled, so independent from the CPU frequency.
For now I think that reverting would be the easiest solution. Somewhat in contrast to what the commit message says, we don't really change anything in terms of error *checking*, as the code carries on anyways (just without increasing the CPU frequency). The only real difference is the order of CPU clock setup and DRAM init.
Karl, can you please amend the commit message to mention the CPU frequency issue?
So if the original patch is about bailing out on error early, can't we just do *that* before the DRAM init, keeping the CPU clock setup still after DRAM?
Just checked, this works as well, but is a bit pointless. Just reverting this and meanwhile checking the DRAM init code seems the easier solution.
Definitely have the same impression. Please send the same revert but adjust the commit message
Michael
Cheers, Andre _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
On Wed, Dec 19, 2018 at 01:45:29AM +0000, André Przywara wrote:
On 19/12/2018 00:51, André Przywara wrote:
On 18/12/2018 12:06, Jagan Teki wrote:
On Tue, Dec 18, 2018 at 4:09 PM karlp@tweak.net.au wrote:
From: Karl Palsson karlp@tweak.net.au
This reverts commit a8011eb84dfac5187cebf00ed8bc981bdb5c1fa1
This causes DRAM init failures on (at least)
- allwinner h3 nanopi-duo2
- allwinner h2+ orangepi zero v1.1
Signed-off-by: Karl Palsson karlp@tweak.net.au
Ideally, this should get into 2019.01, so that this release is not broken on those targets.
Better to understand the issue here, since I have BPI-M2+ which boots fine w/o this revert.
Could this be a .bss issue? This lies in DRAM and is thus only available *after* DRAM init. IIRC we silently rely on not accessing anything in .bss before the DRAM is up, see 59d07ee08e85 for instance. I don't immediately spot any .bss usage in clock_set_pll1(), though.
Or is the 1GHz CPU clock speed too fast for the DRAM init? If I am not mistaken, we run with 24MHz before, so there might be some "natural" delay in some setup routines. Some DRAM chips or board layout might be more susceptible to this than others, which might explain why it only fails on some boards.
Just did some testing on my OrangePi Zero: if I force the CPU frequency to 408 MHz, it works, but fails with the standard 1008 MHz. So this smells that we indeed miss some delays in the DRAM setup code, which sounds tedious to find. There are a number of delays, but those are fine as they are timer controlled, so independent from the CPU frequency.
With latest u-boot, I got it only working when lowering DRAM_CLK to 312MHz (default being 624 MHz(. Even with 336 and 360 MHz it hung just after displaying the 'Trying to boot from FEL' message, while reporting wrong DRAM size (1GiB and 2048MiB).
For now I think that reverting would be the easiest solution. Somewhat in contrast to what the commit message says, we don't really change anything in terms of error *checking*, as the code carries on anyways (just without increasing the CPU frequency). The only real difference is the order of CPU clock setup and DRAM init.
Karl, can you please amend the commit message to mention the CPU frequency issue?
So if the original patch is about bailing out on error early, can't we just do *that* before the DRAM init, keeping the CPU clock setup still after DRAM?
Just checked, this works as well, but is a bit pointless. Just reverting this and meanwhile checking the DRAM init code seems the easier solution.
Cheers, Andre
-- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
participants (6)
-
André Przywara -
Jagan Teki -
Karl Palsson -
karlp＠tweak.net.au
-
Michael Nazzareno Trimarchi -
Priit Laes