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