[U-Boot] [PATCH 031/080] serial: ns16550: Fix address translation

[Simon Glass]

Hi,

On 9 October 2017 at 06:55, Dr. Philipp Tomsich
<philipp.tomsich at theobroma-systems.com> wrote:
>
>> On 9 Oct 2017, at 14:45, Mario Six <mario.six at gdsys.cc> wrote:
>>
>> (adding Philipp since he converted lots of drivers to livetree recently)
>>
>> On Mon, Oct 9, 2017 at 6:48 AM, Simon Glass <sjg at chromium.org> wrote:
>>> Hi Mario,
>>>
>>> On 29 September 2017 at 06:51, Mario Six <mario.six at gdsys.cc> wrote:
>>>> The dev_read_addr function does not do any bus translations, and just
>>>> returns the raw address read from the device tree, which makes the
>>>> driver not work on systems that need bus translations to get the actual
>>>> memory address of the device's register space.
>>>
>>> Aside from any current functionality, what is the correct thing for
>>> dev_read_addr() to do? I worry that the two parts (live/flat tree)
>>> might do different things.
>>>
>>
>> I took a closer look, and indeed, the two parts of dev_read_addr behave
>> differently: dev_read_addr calls dev_read_addr_index, which calls either
>> ofnode_get_addr_index when live tree is active, or devfdt_get_addr_index when
>> it's not. devfdt_get_addr_index applies bus translations, but
>> ofnode_get_addr_index returns the untranslated address using of_read_number
>> (the else part doesn't run, since we have an active live tree if
>> ofnode_get_addr_index was called).
>
> Good point. I would expect the livetree case to use translated addresses
> during runtime, as dev_read_addr_index calls either ofnode_get_addr_index
> or devfdt_get_addr_index.
>
> In other words: are we missing an address translation from ofnode_get_addr_index
> or should the address retrieved via ofnode_get_addr_index have been
> translated by earlier processing?

Thank you both. Yes it seems like the right answer is to add the
missing translation in. There is a CONFIG_OF_TRANSLATE which enables
this. I think most boards don't use it, which is probably why there
are no problems without it, but I think it is becoming more common as
boards become more complex.

>
>>
>> We could fix this by running of_translate_address on the value returned by
>> of_read_number, so that dev_get_addr would then always return a translated
>> address.
>>
>> But what I think is strange is that most live tree conversion patches (e.g.
>> a9d3037 ("usb: dwc2: convert to livetree") or 4aac33f ("dm: mmc: fsl_esdhc:
>> Update to support livetree")) simply replaced devfdt_get_addr (which does apply
>> bus translations) with dev_read_addr (which does not apply bus translations in
>> the live tree case). Shouldn't the converted driver have failed in the live
>> tree case? Or were all drivers converted until now not depending on any bus
>> translations?
>>
>>> In any case, we should not compound the problem if dev_read_addr() is wrong.
>>>
>>>>
>>>> Since the dev_read_addr function is widely used, we refrain from
>>>> modifying it, and instead read the raw address from the device tree, and
>>>> apply the bus translations using the recently introduced
>>>> dev_translate_address function.
>>>>
>>>> Signed-off-by: Mario Six <mario.six at gdsys.cc>
>>>> ---
>>>> drivers/serial/ns16550.c | 5 +++--
>>>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> REgards,
>>> Simon
>>
>> Best regards,
>>
>> Mario
>

Regards,
Simon