[U-Boot] [linux-sunxi] [PATCH] Revert "sunxi: board: Print error after power initialization fails"

[Michael Nazzareno Trimarchi]

Hi André

On Wed, Dec 19, 2018 at 2:46 AM André Przywara <andre.przywara at 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 at tweak.net.au> wrote:
> >>>
> >>> From: Karl Palsson <karlp at 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 at 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 at 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               |