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

[Dr. Philipp Tomsich]

> 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?

> 
> 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