From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefan Roese Date: Tue, 1 Dec 2015 07:05:39 +0100 Subject: [U-Boot] [PATCH] dm: core: Add platform specific bus translation function In-Reply-To: References: <1448619748-26759-1-git-send-email-sr@denx.de> <565BF232.4030602@denx.de> Message-ID: <565D38B3.6010602@denx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Simon, On 01.12.2015 00:17, Simon Glass wrote: > Hi Stefan, > > On 29 November 2015 at 23:52, Stefan Roese wrote: >> Hi Simon, >> >> On 27.11.2015 19:36, Simon Glass wrote: >>> On 27 November 2015 at 02:22, Stefan Roese 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 >>>> Cc: Simon Glass >>>> Cc: Luka Perkov >>>> Cc: Dirk Eibach >>>> --- >>>> 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? Yes. U-Boot proper reconfigures this base address. Quite early in arch_cpu_init(). Note that this change / configuration can't be detected. So you have to know, where the internal registers are mapped. >> >>> 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. I understand. But your suggestion of a new "ranges-spl" property would result in changes to the dts files (for all MVEBU boards) and additional support for this "ranges-spl" property in the U-Boot code. This looks more complex than the 2 lines to the common code I suggested above. And definitely easier to maintain. As new MVEBU boards would always have to patch their dts files for U-Boot. Thanks, Stefan