Hi Simon,

On 11.09.2015 02:42, Simon Glass wrote:
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.


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

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to