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

[Tom Rini]

On Mon, Aug 03, 2015 at 09:52:50AM -0600, Simon Glass wrote:
> Hi Stephen,
> 
> On 3 August 2015 at 09:12, Stephen Warren <swarren at wwwdotorg.org> wrote:
> > On 08/02/2015 06:13 PM, Simon Glass wrote:
> >>
> >> This reverts commit 5b34436035fc862b5e8d0d2c3eab74ba36f1a7f4.
> >>
> >> This function has a few problems. It calls fdt_parent_offset() which as
> >> mentioned in code review is very slow.
> >>
> >> https://patchwork.ozlabs.org/patch/499482/
> >> https://patchwork.ozlabs.org/patch/452604/
> >>
> >> It also happens to break SPI flash on Minnowboard max which is how I
> >> noticed
> >> that this was applied. I can send a patch to tidy that up, but in any case
> >> I think we should consider a revert until the function is better
> >> implemented.
> >
> >
> > The fact that the function needs to perform a slow operation is not a good
> > reason for a revert. The slowness of the operation is just a matter of fact
> > with DT not having uplinks in its data structure, and U-Boot using those
> > data structures directly.
> >
> > You'd requested during review that I additionally implement a faster version
> > of the function in the case where the parent node is already known, and said
> > it was fine if that happened in a later patch. I have this on my TODO list,
> > but it's only been a couple of days.
> 
> I didn't expect this to go to mainline before your new patch.
> 
> >
> > Why not just fix the bug since you said you could? That seems far better
> > than breaking the newly added Tegra210 support.
> 
> I do have a patch but I'm not 100% comfortable with the approach. I
> don't see a good reason to move away from the idea of fdt_addr_t and
> fdt_addr_t being set correctly for the platform. Or maybe I
> misunderstand the problem the patch was trying to fix. As I said it
> did not have a commit message, so who knows :-)
> 
> >
> > P.S. What is the issue with SPI flash? The commit description doesn't
> > mention this at all.
> 
> It calls that function expecting it to pick up an address and size
> from two consecutive cells. With this patch, that fails (unless the
> property happens to be "reg").
> 
> Actually I do favour a revert as it will allow us to discuss the fix
> with a clean slate.

Well, we can unbreak one set of stuff and break another set of stuff,
that's not a clean slate for the now broken platforms.  If it helps to
come up with a better patch we can step one revert and step two better,
but I don't want to apply without step two ready.

... speaking as someone who has people poking me about if the SPI
flasher they sent me works so I can try U-Boot on x86 things.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150803/3394b6c4/attachment.sig>