[U-Boot] [PATCH] dm: core: Add platform specific bus translation function

[Simon Glass]

Hi Stefan,

On 29 November 2015 at 23:52, Stefan Roese <sr at denx.de> wrote:
> Hi Simon,
>
> On 27.11.2015 19:36, Simon Glass wrote:
>> On 27 November 2015 at 02:22, Stefan Roese <sr at denx.de> wrote:
>>> This patch adds the additional platform_translate_address() call to
>>> dev_get_addr(). A weak default with a 1-to-1 translation is also
>>> provided. Platforms that need a special address translation can
>>> overwrite this function.
>>>
>>> Here the explanation, why this is needed for MVEBU:
>>>
>>> When using DM with DT address translation, this does not work
>>> with the standard fdt_translate_address() function on MVEBU
>>> in SPL. Since the DT translates to the 0xf100.0000 base
>>> address for the internal registers. But SPL still has the
>>> registers mapped to the 0xd000.0000 (SOC_REGS_PHY_BASE)
>>> address that is used by the BootROM. This is because SPL
>>> may return to the BootROM for boot continuation (e.g. UART
>>> xmodem boot mode).
>>>
>>> Signed-off-by: Stefan Roese <sr at denx.de>
>>> Cc: Simon Glass <sjg at chromium.org>
>>> Cc: Luka Perkov <luka.perkov at sartura.hr>
>>> Cc: Dirk Eibach <dirk.eibach at gdsys.cc>
>>> ---
>>>   drivers/core/device.c | 36 +++++++++++++++++++++++++-----------
>>>   1 file changed, 25 insertions(+), 11 deletions(-)
>>
>> I wonder if there is a way to handle this with device tree? I would
>> very much like to avoid adding weak functions and other types of
>> hooks.
>
> I've thought about this also for quite a bit. But couldn't come
> up with a "better", less intrusive implementation for this
> problem yet.
>
>> Are you saying that there are two values for 'ranges', one in
>> SPL and one for U-Boot proper?
>
> You can think of it as 2 values for "ranges", yes. Basically
> its a difference in the upper 8 bits of all addresses of the
> internal registers (e.g. UART, SDRAM controller...).
>
> The BootROM starts with 0xd000.0000 and SPL also needs to
> use this value. As SPL returns back into the BootROM in
> some cases. And Linux (and other OS'es) expect 0xf100.0000
> as base address. So the main U-Boot reconfigured the base
> address to this value quite early.
>
>> What actually triggers the change?
>
> This is no change. Its just, that now SPL has added DM and DTS
> support. Before this SPL-DM support this was handled by
> something like this:
>
> #if defined(CONFIG_SPL_BUILD)
> #define SOC_REGS_PHY_BASE       0xd0000000
> #else
> #define SOC_REGS_PHY_BASE       0xf1000000
> #endif
> #define MVEBU_REGISTER(x)       (SOC_REGS_PHY_BASE + x)
> #define MVEBU_SDRAM_SCRATCH     (MVEBU_REGISTER(0x01504))
> #define MVEBU_L2_CACHE_BASE     (MVEBU_REGISTER(0x08000))
> ...
>
> And now (nearly) all addresses are taken from the DT. And the
> SPL vs. U-Boot proper base address difference needs to get
> handled otherwise - here in the DT.

No, I mean what causes the hardware address to move? Is there a
register somewhere that it adjusted to tell the addressing to change?

>
>> One option would be to have a ranges-spl property, or similar.
>
> Hmmm. you mean to add these "ranges-spl" properties additionally
> to the normal "ranges" properties? I would really like to not
> change the "ranges" in the dts files. As especially in the
> MVEBU cases (Armada XP / 38x etc), the occurrences are very
> high. And this would result in quite a big difference to the
> "mainline Linux dts" version.

Yes I mean a new property. After all, the existing one is incorrect
for your hardware at least in some configuration.

>
> I could also add this functionality via a new Kconfig option.
> Like this:
>
> +       if (CONFIG_IS_ENABLED(PLATFORM_TRANSLATE_ADDRESS)) {
> +               addr = platform_translate_address((void *)gd->fdt_blob,
> +                                                 dev->of_offset, addr);
> +       }
>
> So no weak default would be needed. Just let me know if you
> would prefer it this way. And I'll send a v2 using this
> approach.

I'd like to exhaust the DT option first, as this adds another level of
complexity...the DT is supposed to describe the hardware.

Regards,
Simon