[U-Boot] [PATCH v2] drivers: core: Add translation in live tree case

[Mario Six]

On Fri, Dec 15, 2017 at 9:05 AM, Mario Six <mario.six at gdsys.cc> wrote:
> Hi Simon,
>
> On Thu, Dec 14, 2017 at 9:36 PM, Simon Glass <sjg at chromium.org> wrote:
>> +Stephen, Tom
>>
>> Hi Mario,
>>
>> I've had to drop this since it breaks tegra. Stephen feels that this
>> is likely a bug in the patch rather than anything wrong with Tegra. Do
>> you have any thoughts? I can potentially try things out on the Tegra
>> boards I have.
>>
>> Regards,
>> Simon
>>
>
> I think I've found the problem: It's an endianness issue. The
> of_translate_address function seems to return a raw big-endian address, while
> the of_read_number function translates it to CPU endianness before returning
> it. On the MPC8308 I tested on this made no difference, but little-endian
> systems will have problems. When I put a bogus dev_read_addr in the sandbox
> serial driver, I see the effect as well.
>
> Here's a hopefully fixed version (just the diff; I'll post a proper patch when
> your tests succeeded); it corrects the issue on sandbox, so I hope it will work
> on tegra as well:
>
> ----- >8 -----
>
> diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
> index 0030ab962e..ca22d09ff2 100644
> --- a/drivers/core/ofnode.c
> +++ b/drivers/core/ofnode.c
> @@ -200,13 +200,22 @@ fdt_addr_t ofnode_get_addr_index(ofnode node, int index)
>                 uint flags;
>                 u64 size;
>                 int na;
> +               __be32 addr;
>
>                 prop_val = of_get_address(ofnode_to_np(node), index, &size,
>                                           &flags);
>                 if (!prop_val)
>                         return FDT_ADDR_T_NONE;
>                 na = of_n_addr_cells(ofnode_to_np(node));
> -               return of_read_number(prop_val, na);
> +               addr = of_read_number(prop_val, na);
> +
> +               if (IS_ENABLED(CONFIG_OF_TRANSLATE)) {
> +                       u64 paddr =
> of_translate_address(ofnode_to_np(node), &addr);
> +
> +                       return (fdt_addr_t)(be64_to_cpu(paddr) >> 32);
> +               } else {
> +                       return addr;
> +               }
>         } else {
>                 return fdt_get_base_address(gd->fdt_blob,
>                                             ofnode_to_offset(node));
> ----- >8 -----
>
> Thanks for the help, and best regards,
>
> Mario
>

Small correction:

----- >8 -----

diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
index 0030ab962e..09f0aeab4f 100644
--- a/drivers/core/ofnode.c
+++ b/drivers/core/ofnode.c
@@ -200,13 +200,22 @@ fdt_addr_t ofnode_get_addr_index(ofnode node, int index)
                uint flags;
                u64 size;
                int na;
+               __be32 addr;

                prop_val = of_get_address(ofnode_to_np(node), index, &size,
                                          &flags);
                if (!prop_val)
                        return FDT_ADDR_T_NONE;
                na = of_n_addr_cells(ofnode_to_np(node));
-               return of_read_number(prop_val, na);
+               addr = of_read_number(prop_val, na);
+
+               if (IS_ENABLED(CONFIG_OF_TRANSLATE)) {
+                       u64 paddr =
of_translate_address(ofnode_to_np(node), &addr);
+
+                       return be32_to_cpu((fdt_addr_t)paddr);
+               } else {
+                       return addr;
+               }
        } else {
                return fdt_get_base_address(gd->fdt_blob,
                                            ofnode_to_offset(node));

----- >8 -----

Regards,

Mario

>>
>> On 8 December 2017 at 10:11,  <sjg at google.com> wrote:
>>> On 23 November 2017 at 23:51, Mario Six <mario.six at gdsys.cc> wrote:
>>>> The function dev_read_addr calls ofnode_get_addr_index in the live tree
>>>> case, which does not apply bus translations to the address read from the
>>>> device tree. This results in illegal addresses on boards that rely on
>>>> bus translations being applied.
>>>>
>>>> Fix this situation by applying bus translations in the live tree case as
>>>> well.
>>>>
>>>> Signed-off-by: Mario Six <mario.six at gdsys.cc>
>>>> ---
>>>>
>>>> Changes v1 -> v2:
>>>> * Added IS_ENABLED(CONFIG_OF_TRANSLATE) case distinction
>>>>
>>>> ---
>>>>  drivers/core/ofnode.c | 8 +++++++-
>>>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>>
>>> Reviewed-by: Simon Glass <sjg at chromium.org>
>>>
>>> Applied to u-boot-dm thanks!
>>