From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefan Roese Date: Fri, 11 Sep 2015 07:41:05 +0200 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> <55F11B18.2010002@denx.de> Message-ID: <55F26971.1020109@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 11.09.2015 02:42, Simon Glass wrote: >>> 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. >> >> >> You've mentioned this before. But how slow could this function really be? > > It scans the tree from the start. There is no back link. > >> And it should not be called that often via dev_get_addr(). Usually only once >> for each driver in the probe function. Or am I missing something? > > Sounds correct. So it really shouldn't make a big difference. >> >>> I wonder if this code should be copied into a new file in >>> drivers/core/, tidied up and updated to use dev->parent? >> >> >> You mean fdt_translate_address()? It references many functions from >> fdt_support.c though which we would need to duplicate here as well. >> > > Right. Seems like a pain. > >>> 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 see no problems to defer this patch (or a "better" version of it) to after >> this release. The Marvell mvebu DM patches are also not targeted for this >> release. > > OK - and if the time slowdown is not too large then we can just use > this patch, particularly as it is an optional CONFIG. Can you check > how much slower it is to use your new case versus the original code? Marvell MVEBU won't boot without this option enabled. So I can't really compare it here. Someone with a platform that doesn't need this option enabled can definitely better do this test and compare the results. Thanks, Stefan