From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Warren Date: Fri, 11 Sep 2015 10:07:47 -0700 Subject: [U-Boot] [PATCH] dm: core: Enable optional use of fdt_translate_address() In-Reply-To: References: <1441174944-8878-1-git-send-email-sr@denx.de> <1441343506-28473-1-git-send-email-sr@denx.de> Message-ID: <55F30A63.1050201@nvidia.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 09/09/2015 11:07 AM, Simon Glass wrote: > +Stephen > > Hi Stefan, > > On Thursday, 3 September 2015, 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. >> >> Signed-off-by: Stefan Roese >> Cc: Simon Glass >> Cc: Bin Meng >> Cc: Marek Vasut >> Cc: Masahiro Yamada >> --- >> 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. > > I wonder if this code should be copied into a new file in > drivers/core/, tidied up and updated to use dev->parent? > > 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 haven't looked at the code in detail, but I'm surprised there's a Kconfig option for this, for either SPL or main U-Boot. In general, this feature is simply a required part of parsing DT, so surely the code should always be enabled. Without it, we're only getting lucky if DT works (lucky the DT doesn't happen to contain a ranges property). Sure the code does some searching through the DT, and that's slower than not doing it, but I don't see how we can support DT without parsing DT correctly. Now admittedly some platforms' DTs happen not to contain ranges that require this code in practice. However, I feel that's a bit of a micro-optimization, and a rather error-prone one at that. What if someone pulls a more complete DT into U-Boot and suddenly the code is required and they have to spend ages tracking down their problem to missing functionality in a core DT parsing API - something they'd be unlikely to initially suspect.