On 10/17/2012 10:22 AM, Stephen Warren wrote:

On 10/17/2012 08:38 AM, Rob Herring wrote:

quoted

On 10/15/2012 02:07 PM, Stephen Warren wrote:

quoted

From: Stephen Warren <redacted>

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

quoted

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?

So answering my own question here after testing this:

If I use a different (to iomap.h) virtual address in debug-macro.S, then
the very very early output from earlyprintk does work:

[    0.000000] Booting Linux on physical CPU 0
[    0.000000] Initializing cgroup subsys cpu
[    0.000000] Linux version 3.7.0-rc1-next-20121016-...
[    0.000000] CPU: ARMv7 Processor [411fc090] revision 0 (ARMv7), cr=10c5387d
[    0.000000] CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing instruction cache
[    0.000000] Machine: nVidia Tegra20 (Flattened Device Tree), model: NVIDIA Tegra2 Harmony evaluation board
[    0.000000] bootconsole [earlycon0] enabled
[    0.000000] Memory policy: ECC disabled, Data cache writealloc

However, the kernel either hangs or output simply stops working at that
point. If I modify Tegra's iotable_init() call to add an entry to map
the UART to the virtual address debug-macro.S assumes, then everything
works again.

That implies we really do need to keep the two pieces of code completely
in sync, so a shared header is the right way to go. It also implies that
having duplicate mappings of the same physical address doesn't cause any
immediate obvious catastrophic problems.

Ways we might avoid files in arch/arm/include/debug having to use
relative include paths to pick up that header are:

a) Move mach-${mach}/include/mach/iomap.h to iomap-${mach}.h in some
standard include path.

b) Rework debug-macro.S so that it isn't an include file, but rather a
regular top-level file. In other words, rather than compiling
arch/arm/kernel/debug.S, and having that #include DEBUG_LL_INCLUDE,
instead compile DEBUG_LL_SOURCE (i.e. arch/arm/mach-${mach}/debug.S by
default), and have that #include any common parts (e.g. implementation
of printhex8). This has benefits of:

b1) arch/arm/mach-${mach}/debug.S is in the mach directory that owns it,
rather than having them all dumped into a common location.

b2) Can use #include "iomap.h", a non-relative include, to pick up the
shared header

b3) Perhaps we can simplify the current debug.S e.g. have a common
debug-semihosting.S that contains the semihosting stuff, and only
include that from mach-*/debug.S if that machine uses semihosting, or
similar?

(b) seems like a lot of work. I don't see any advantage of (a) over just
using the relative include.