[U-Boot] [PATCH] dm: core: Enable optional use of fdt_translate_address()

[Stephen Warren]

On 09/09/2015 11:07 AM, Simon Glass wrote:
> +Stephen
> 
> Hi Stefan,
> 
> On Thursday, 3 September 2015, Stefan Roese <sr at denx.de> wrote:
>>
>> The current "simple" address translation simple_bus_translate() is not
>> working on some platforms (e.g. MVEBU). As here more complex "ranges"
>> properties are used in many nodes (multiple tuples etc). This patch
>> enables the optional use of the common fdt_translate_address() function
>> which handles this translation correctly.
>>
>> Signed-off-by: Stefan Roese <sr at denx.de>
>> Cc: Simon Glass <sjg at chromium.org>
>> Cc: Bin Meng <bmeng.cn at gmail.com>
>> Cc: Marek Vasut <marex at denx.de>
>> Cc: Masahiro Yamada <yamada.masahiro at socionext.com>
>> ---
>> v2:
>> - Rework code a bit as suggested by Simon. Also added some comments
>>   to make the use of the code paths more clear.
> 
> 
> While this works I'm reluctant to commit it as is. The call to
> fdt_parent_offset() is very slow.
> 
> I wonder if this code should be copied into a new file in
> drivers/core/, tidied up and updated to use dev->parent?
> 
> Other options:
> - Add a library to unflatten the tree - but this would not be very
> useful in SPL or before relocation due to memory/speed constraints
> - Add a helper to find a node parent which uses a cached tree scan to
> build a table of previous nodes (or some other means to go backwards
> in the tree)
> - Worry about it later and go ahead with this patch

I haven't looked at the code in detail, but I'm surprised there's a
Kconfig option for this, for either SPL or main U-Boot. In general, this
feature is simply a required part of parsing DT, so surely the code
should always be enabled. Without it, we're only getting lucky if DT
works (lucky the DT doesn't happen to contain a ranges property). Sure
the code does some searching through the DT, and that's slower than not
doing it, but I don't see how we can support DT without parsing DT
correctly. Now admittedly some platforms' DTs happen not to contain
ranges that require this code in practice. However, I feel that's a bit
of a micro-optimization, and a rather error-prone one at that. What if
someone pulls a more complete DT into U-Boot and suddenly the code is
required and they have to spend ages tracking down their problem to
missing functionality in a core DT parsing API - something they'd be
unlikely to initially suspect.