On 09/13/2015 11:25 PM, Stefan Roese wrote:
Hi Stephen,
On 11.09.2015 19:07, Stephen Warren wrote:
On 09/09/2015 11:07 AM, Simon Glass wrote:
+Stephen
Hi Stefan,
On Thursday, 3 September 2015, Stefan Roese <s...@denx.de> 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 <s...@denx.de>
Cc: Simon Glass <s...@chromium.org>
Cc: Bin Meng <bmeng...@gmail.com>
Cc: Marek Vasut <ma...@denx.de>
Cc: Masahiro Yamada <yamada.masah...@socionext.com>
---
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).
Yes. I was also a bit surprised, that this current (limited)
implementation to translate the address worked on the platforms using
this interface right now.
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.
Ack. However, I definitely understand Simon's arguments about code size
here. On some platforms with limited RAM for SPL this additional code
for "correct" ranges parsing and address translation might break the
size limit. Not sure how to handle this. At least a comment in the code
would be helpful, explaining that simple_bus_translate() is limited here
in some aspects.
So in my AArch64 build, fdt_translate_address is 0x270 bytes. I can see
that might be pushing some extremely constrained binaries over a limit
if that function isn't already included in the binary. However, if we
are in that situation, I have a really hard time believing this one
patch/function will be the only issue; we'll constantly be hitting a
wall where we can't fix issues in DT parsing, DT handling, or other code
in these binaries since the fix will bloat the binary too much.
In those cases, I rather question whether DT support is the correct
approach; completely dropping DT support from those binaries would
likely remove large amounts of code and replace it with a tiny amount of
constant data. It seems like that'd be the best approach all around
since it'd head of the issue completely.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
