From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stefan Roese <sr@denx.de>
Date: Thu, 1 Oct 2015 08:59:29 +0200
Subject: [U-Boot] [PATCH v3] dm: core: Enable optional use of
 fdt_translate_address()
In-Reply-To: <560C0A20.8030909@wwwdotorg.org>
References: <1441174944-8878-1-git-send-email-sr@denx.de>
 <1443589243-9521-1-git-send-email-sr@denx.de>
 <560C0A20.8030909@wwwdotorg.org>
Message-ID: <560CD9D1.4010309@denx.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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?

Thanks,
Stefan