Re: [PATCH V2 3/3] ARM: tegra: move debug-macro.S to include/debug

[Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>]

On 10/17/2012 08:38 AM, Rob Herring wrote:
> On 10/15/2012 02:07 PM, Stephen Warren wrote:
>> From: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>>
>> Move Tegra's debug-macro.S over to the common debug macro directory.
>>
>> Move Tegra's debug UART selection menu into ARM's Kconfig.debug, so that
>> all related options are selected in the same place.
>>
>> Tegra's uncompress.h is left in mach-tegra/include/mach; it will be
>> removed whenever Tegra is converted to multi-platform.
>>
>> Signed-off-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>> ---
>> Rob, Arnd, Olof, I'd particularly like feedback on whether the following:
>>
>> #include "../../mach-tegra/iomap.h"
>>
>> in arch/arm/include/debug/tegra.S is acceptable. I'd really like to
>> continue to #include a header to share the defines to Tegra physical
>> memory layout and virtual based addresses with Tegra's io.c's struct
>> map_desc entries, so they can't get out of sync. So, the include can
>> either use the relative path as quoted above (which I don't think will
>> cause any significant maintenance issue), or Tegra's iomap.h would have
>> to be moved somewhere public so e.g. <tegra-iomap.h> could be included.
> 
> We already have a way to get the phys and virt addresses at runtime with
> addruart macro.

So this discussion is mainly about the implementation of addruart.

> Couldn't we wrap this with a proper function and setup
> the mapping at runtime. This would move it out of the platforms.

So, the mapping already is set up at run-time at least during early
boot; __create_page_tables() in arch/arm/kernel/head.S calls addruart
and sets up an entry for it.

I suppose the implication here is that the virtual address that addruart
returns doesn't have to match anything that the machine later sets up
using iotable_init().

If that's true, then Tegra's debug-macro.S only needs to know the UART
physical address, and can make up almost any arbitrary virtual address
(perhaps even driven by the logic you mention in your next paragraph
below) and hence need not rely on Tegra's iomap.h. That said, we'd still
have to manually remember not to create conflicting virtual address
setups in the two places, which would still be easier with a shared header.

However, I then have two questions:

1) How long do the page tables set up by __create_page_tables() last; do
they stick around forever, or at least as long as the macros from
debug-macro.S are used, or are they replaced sometime, on the assumption
that the machine's .map_io() will call iotable_init() and end up setting
up the same mapping?

2) If the virtual address returned by addruart on Tegra is different
than any virtual addresses set up by Tegra's .map_io(), and Tegra's
.map_io() sets up a very broad mapping that covers all peripherals
including the UART, and hence the UART physical registers get mapped
into two virtual addresses, will this cause any problems? IIRC,
duplicate mappings can cause some issues on ARM, but perhaps that only
applies to memory-like mappings, and not completely uncached IO mappings?

> I'd also like to make the virtual address the same on all platforms (but
> different offsets within a 1MB section) and make the phys address a
> kconfig option. This would also eliminate the need for the platform
> include and potentially addruart for that matter.