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.
Thanks,
Stefan
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
