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

[Stefan Roese]

Hi Simon,

On 11.09.2015 02:42, 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.

So it really shouldn't make a big difference.

>>
>>> 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?

Marvell MVEBU won't boot without this option enabled. So I can't really 
compare it here. Someone with a platform that doesn't need this option 
enabled can definitely better do this test and compare the results.

Thanks,
Stefan