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

[Simon Glass]

Hi Stefan,

On 1 October 2015 at 07:59, Stefan Roese <sr at denx.de> wrote:
> Hi Stephen,
>
>
> On 30.09.2015 18:13, Stephen Warren wrote:
>>
>> On 09/29/2015 11:00 PM, Stefan Roese 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.
>>
>>
>> This change makes sense to me, but one comment:
>>
>>> diff --git a/drivers/core/Kconfig b/drivers/core/Kconfig
>>
>>
>>> +config OF_TRANSLATE
>>> +    bool "Translate addresses using fdt_translate_address"
>>> +    depends on DM && OF_CONTROL
>>> +    default y
>>
>>
>> So this is on by default, which I think is correct since applying this
>> technique is required to parse DT correctly. However, ...
>>
>>> diff --git a/drivers/core/device.c b/drivers/core/device.c
>>
>>
>>> +    if (CONFIG_IS_ENABLED(OF_TRANSLATE)) {
>>
>>
>>> +        /*
>>> +         * Use the full-fledged translate function for complex
>>> +         * bus setups.
>>> +         */
>>> +        return fdt_translate_address((void *)gd->fdt_blob,
>>> +                         dev->of_offset, reg);
>>
>>
>> fdt_translate_address() is a simple wrapper around
>> __of_translate_address(), and that function calls fdt_parent_offset()
>> which is "slow" per Simon. Surely this patch will receive the same
>> objection as when I added a (single) call to fdt_parent_offset() into
>> the DT address parsing routine (and this patch is worse, since it adds a
>> call to fdt_parent_offset() for each level of DT sub-nodes).
>
>
> Yes, Simon already mentioned his speed related concerns. But I think
> I convinced him that calling this function once for each driver probe
> can't be that bad this in the v2 thread:
>
> https://patchwork.ozlabs.org/patch/514331/
>
> Simon, is this okay? Will you pull this patch after the v2015.10
> release?

It's not great but it is enabled by an option so I think it is OK for
now. We talked about the alternatives and they were not too great
either. Things like this suggest that we might move to an unflattened
device tree post-relocation one day.

Regards,
Simon