[U-Boot] [PATCH] Revert "fdt: Fix fdtdec_get_addr_size() for 64-bit"

[Simon Glass]

Hi Tom,

On 16 September 2015 at 15:46, Tom Warren <TWarren at nvidia.com> wrote:
> Simon,
>
>> -----Original Message-----
>> From: Tom Warren
>> Sent: Wednesday, September 02, 2015 7:02 PM
>> To: 'Stephen Warren'; Simon Glass
>> Cc: U-Boot Mailing List; Thierry Reding; Tom Rini
>> Subject: RE: [U-Boot] [PATCH] Revert "fdt: Fix fdtdec_get_addr_size() for 64-
>> bit"
>>
>> > -----Original Message-----
>> > From: Stephen Warren [mailto:swarren at wwwdotorg.org]
>> > Sent: Wednesday, September 02, 2015 4:44 PM
>> > To: Tom Warren; Simon Glass
>> > Cc: U-Boot Mailing List; Thierry Reding; Tom Rini
>> > Subject: Re: [U-Boot] [PATCH] Revert "fdt: Fix fdtdec_get_addr_size()
>> > for 64- bit"
>> >
>> > On 09/02/2015 01:54 PM, Stephen Warren wrote:
>> > > On 09/02/2015 01:39 PM, Tom Warren wrote:
>> > >>
>> > >>
>> > >>> -----Original Message-----
>> > >>> From: Stephen Warren
>> > >>> Sent: Wednesday, September 02, 2015 1:05 PM
>> > >>> To: Tom Warren; Simon Glass
>> > >>> Cc: Bin Meng; Thierry Reding; Tom Rini; U-Boot Mailing List
>> > >>> Subject: Re: [U-Boot] [PATCH] Revert "fdt: Fix
>> > >>> fdtdec_get_addr_size() for 64- bit"
>> > >>>
>> > >>> On 09/02/2015 09:52 AM, Tom Warren wrote:
>> > >>>> Simon, et al,
>> > >>>>
>> > >>>>> Simon Glass wrote at Friday, August 14, 2015 3:05 AM:
>> > >>>>> I plan to apply this revert to u-boot-x86 (where SPI is
>> > >>>>> currently
>> > >>>>> broken) and (once it has a bit more testing) also this patch
>> > >>>>> which I think makes the change in a safer way:
>> > >>>>>
>> > >>>>> https://patchwork.ozlabs.org/patch/504918/
>> > >>>>>
>> > >>>>> At present that patch breaks at least one x86 board and I have
>> > >>>>> not dug into it yet.
>> > >>>>>
>> > >>>>> The revert should not break tegra, according to Stephen.
>> > >>>>
>> > >>>> Unfortunately, my testing on P2571 with TOT u-boot-tegra (rebased
>> > >>>> against
>> > >>> TOT u-boot/master this morning) shows that that is not true.
>> > >>>>
>> > >>>> The revert of the disputed 'fdtdec_get_addr_size' patch _does_
>> > >>>> break Tegra
>> > >>> 64-bit (P2571, at least). Nyan-big is OK.  With Simon's revert in
>> > >>> place, my board just loops on SPL signon, so I assume it's
>> > >>> faulting, etc. in CPU init.  Note that this is the current state of TOT u-
>> boot/master.
>> > >>>
>> > >>> I'm a bit confused. So far, we don't support SPL on T210 since we
>> > >>> assume some other bootloader runs on the boot CPU and starts just
>> > >>> the main U-Boot on the main CPU. It sounds like you're testing
>> > >>> some local-
>> > only SPL support?
>> > >>
>> > >> Currently there are a couple of ways to get T210 64-bit U-Boot
>> > loaded/executed. The first is the way I've always done it (during
>> > development and today) - use a 32-bit SPL that I built when I first
>> > ported 32-bit U-Boot to T210. I've saved away the SPL portion as a
>> > binary, and combine it with the current 64-bit T210 U-Boot proper when
>> > building my image.  It's always worked up to now.  What I'm seeing is
>> > a failure in the 64-bit CPU U-Boot portion. I just mentioned the
>> > looping SPL signon symptom because that's what I see as the indicator of a
>> broken 64-bit image.
>> > >
>> > > Oh I see; the SPL is only looping because the main U-Boot binary
>> > > crashes/... and resets the CPU, hence re-executing the SPL. I
>> > > thought you were referring to a loop purely within SPL.
>> > >
>> > > Now it makes more sense.
>> > >
>> > >> The other way is to use your proprietary NV bootloader for the
>> > >> 32-bit
>> > portion (this will become the de facto standard when we release said
>> > NV bootloader code as open-source, or a binary first-stage loader
>> > 'tool').  I haven't tried that, since my way works and is an easy part of my
>> workflow.
>> > >>
>> > >> If you can point me to your boot CPU loader internally, I can try
>> > >> your
>> > method and see if it makes a difference, but I doubt it will.
>> > >
>> > > I sent you an internal email message.
>> > >
>> > > Perhaps you could also try my upstream U-Boot dev branch at:
>> > >
>> > > repo git://github.com/swarren/u-boot.git branch tegra_dev
>> > >
>> > > That has the revert of the original patch in, but also has the
>> > > following replacement which you'd want to revert (or perhaps best:
>> > > try with and without it):
>> > >
>> > > c1fd5e1d5586 fdt: add new fdt address parsing functions
>> > >
>> > > I'm sure I tested Simon's revert at the time I said it was OK. I
>> > > wonder if the revert used to work fine, but something since then
>> > > fails if the revert is in place? I would try testing this now, but
>> > > I'm travelling so it's a bit more painful.
>> >
>> > I worked out how to remote control my device, and tested the current
>> > u-boot- tegra/master (which contains the patch revert this email
>> > thread is about) with and without "fdt: add new fdt address parsing
>> functions"
>> > removed, and it works fine for me.
>> >
>> > When you're concatenating SPL+U-Boot+DTB, are you using the DTB from
>> > the same source tree as the main U-Boot (likely by getting U-Boot+DTB
>> > from u- boot-dtb.bin in that source tree)?
>> Yes
>>
>
> I'm not sure if this was the last thread on this (I was on vacation for a few days), but have you resolved the problem you had with Stephen's new 'fdt: Add new fdt address parsing functions" patch? I'd really like to get this resolved so I can send a PR to TomR.

Yes I believe the issues are resolved (not with the patch BTW - the
only issue with the patch was the #define DEBUG which I removed).

The patch has just been applied to mainline.

Regards,
Simon