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

[Simon Glass]

Hi Stefan,

On 9 September 2015 at 22:54, Stefan Roese <sr at denx.de> wrote:
> Hi Simon,
>
> On 09.09.2015 20:07, Simon Glass wrote:
>>
>> 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.
>
>
> You've mentioned this before. But how slow could this function really be?

It scans the tree from the start. There is no back link.

> And it should not be called that often via dev_get_addr(). Usually only once
> for each driver in the probe function. Or am I missing something?

Sounds correct.

>
>> I wonder if this code should be copied into a new file in
>> drivers/core/, tidied up and updated to use dev->parent?
>
>
> You mean fdt_translate_address()? It references many functions from
> fdt_support.c though which we would need to duplicate here as well.
>

Right. Seems like a pain.

>> 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 see no problems to defer this patch (or a "better" version of it) to after
> this release. The Marvell mvebu DM patches are also not targeted for this
> release.

OK - and if the time slowdown is not too large then we can just use
this patch, particularly as it is an optional CONFIG. Can you check
how much slower it is to use your new case versus the original code?

Regards,
Simon