Hi Bin,
On 04.12.2015 07:17, Bin Meng wrote:
Hi,
On Fri, Dec 4, 2015 at 1:31 PM, Bin Meng <bmeng...@gmail.com> wrote:
Hi Stefan,
On Thu, Dec 3, 2015 at 10:12 PM, Stefan Roese <s...@denx.de> wrote:
Hi Bin,
On 03.12.2015 14:34, Bin Meng wrote:
Hi Stefan, Simon,
On Mon, Oct 19, 2015 at 7:16 AM, Simon Glass <s...@chromium.org> wrote:
On 29 September 2015 at 23:00, 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>
Cc: Stephen Warren <swar...@nvidia.com>
Cc: Lukasz Majewski <l.majew...@samsung.com>
---
v3:
- Rebased on current U-Boot version
- Added Stephen and Lukasz to Cc
v2:
- Rework code a bit as suggested by Simon. Also added some comments
to make the use of the code paths more clear.
drivers/core/Kconfig | 30 ++++++++++++++++++++++++++++++
drivers/core/device.c | 20 ++++++++++++++++++++
2 files changed, 50 insertions(+)
Applied to u-boot-dm, thanks!
When testing Simon's patch [1], I found PCI UART on Intel Crown Bay no
longer works. git bisect leads to this commit. Somehow I missed this
patch before although I see the commit message get me cc'ed but the
email did not bring to my attention.
I see this patch introduced OF_TRANSLATE and by default set it to y.
This makes the code logic in dev_get_addr() go through
fdt_translate_address(), which breaks the things.
I'm a bit surprised that using the common fdt_translate_address()
function instead of the DM internal simple_bus_translate() causes
problems on your platform. Are you sure that the ranges are
described correctly in your dts? Is the dts a copy from the Linux
original one? Ah, probably not, since we're talking about x86
which has no DT support in Linux, right?
Is fdt_translate_address() able to handle PCI bus ranges property? PCI
has special ranges.
The arch/x86/dts/crownbay.dts has something like below:
90 pci {
91 #address-cells = <3>;
92 #size-cells = <2>;
93 compatible = "pci-x86";
94 u-boot,dm-pre-reloc;
95 ranges = <0x02000000 0x0 0x40000000 0x40000000 0 0x80000000
96 0x42000000 0x0 0xc0000000 0xc0000000 0 0x20000000
97 0x01000000 0x0 0x2000 0x2000 0 0xe000>;
98
99 pcie@17,0 {
100 #address-cells = <3>;
101 #size-cells = <2>;
102 compatible = "pci-bridge";
103 u-boot,dm-pre-reloc;
104 reg = <0x0000b800 0x0 0x0 0x0 0x0>;
Should we set
OF_TRANSLATE to n by default? If set to y, this requires dts to have
complete ranges property everywhere.
My understanding here is that x86 is a special case. As it doesn't
use the full-blown dts sources from Linux. But most likely some
"simple" ones, written exactly for U-Boot / DM.
I would still prefer to have this OF_TRANSLATE set to y as default.
As its needed for at least some platforms. But if we decide to
set it to n, I can live with it as well.
Looks like the issue is:
dev_get_addr() return value is of type fdt_addr_t, and if no valid
address found returns FDT_ADDR_T_NONE. But FDT_ADDR_T_NONE is defined
as follows:
#ifdef CONFIG_PHYS_64BIT
#define FDT_ADDR_T_NONE (-1ULL)
#define fdt_addr_to_cpu(reg) be64_to_cpu(reg)
#define fdt_size_to_cpu(reg) be64_to_cpu(reg)
#else
#define FDT_ADDR_T_NONE (-1U)
#define fdt_addr_to_cpu(reg) be32_to_cpu(reg)
#define fdt_size_to_cpu(reg) be32_to_cpu(reg)
#endif
On x86, CONFIG_PHYS_64BIT is not defined, so FDT_ADDR_T_NONE becomes -1U.
In the ns16550 driver, the code logic is:
/* try Processor Local Bus device first */
addr = dev_get_addr(dev);
#ifdef CONFIG_PCI
if (addr == FDT_ADDR_T_NONE) {
/* then try pci device */
With OF_TRANSLATE set to y, dev_get_addr() returns OF_BAD_ADDR if no
valid address found, but OF_BAD_ADDR is defined as:
#define OF_BAD_ADDR ((u64)-1)
This creates a size mismatch as FDT_ADDR_T_NONE can be -1U or -1ULL
depending on CONFIG_PHYS_64BIT but OF_BAD_ADDR is always -1ULL.
The patch below fixes this issue:
diff --git a/common/fdt_support.c b/common/fdt_support.c
index f86365e..8930f34 100644
--- a/common/fdt_support.c
+++ b/common/fdt_support.c
@@ -16,6 +16,7 @@
#include <libfdt.h>
#include <fdt_support.h>
#include <exports.h>
+#include <fdtdec.h>
/**
* fdt_getprop_u32_default_node - Return a node's property or a default
@@ -945,7 +946,7 @@ void fdt_del_node_and_alias(void *blob, const char *alias)
/* Max address size we deal with */
#define OF_MAX_ADDR_CELLS 4
-#define OF_BAD_ADDR ((u64)-1)
+#define OF_BAD_ADDR FDT_ADDR_T_NONE
#define OF_CHECK_COUNTS(na, ns) ((na) > 0 && (na) <=
OF_MAX_ADDR_CELLS && \
(ns) > 0)
I remember stumbling over such a related problem as well a few
weeks ago. With a mismatch of address-cells size and non-64bit
platform support. But I got distracted from this issue at that
time.
Thanks for looking into this. This change looks good to me. Please
send a patch to the list.
Thanks,
Stefan
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
