diff mbox

[V2] ARM: allow DEBUG_UNCOMPRESS for Tegra

Message ID 1375379471-15615-1-git-send-email-swarren@wwwdotorg.org (mailing list archive)
State New, archived
Headers show

Commit Message

Stephen Warren Aug. 1, 2013, 5:51 p.m. UTC
From: Stephen Warren <swarren@nvidia.com>

DEBUG_UNCOMPRESS was previously disallowed for Tegra due to tegra.S's
use of global data that was not linked into the decompressor. Solve this
by declaring this symbol in tegra.S when it is being built into the
decompressor. For the kernel proper, leave the declaration in
mach-tegra/common.c as explained in the comment.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
v2: Don't select DEBUG_UNCOMPRESS with DEBUG_TEGRA_UART unless
    !ZBOOT_ROM. Add notes on ZBOOT_ROM to comment.

Russell, I assume this should go in through the ARM patch tracker. Or,
do you want me to take it through the Tegra tree?

 arch/arm/Kconfig.debug         |  2 +-
 arch/arm/include/debug/tegra.S | 29 +++++++++++++++++++++++++++++
 2 files changed, 30 insertions(+), 1 deletion(-)

Comments

Alexandre Courbot Aug. 2, 2013, 1:59 a.m. UTC | #1
On Fri, Aug 2, 2013 at 2:51 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> From: Stephen Warren <swarren@nvidia.com>
>
> DEBUG_UNCOMPRESS was previously disallowed for Tegra due to tegra.S's
> use of global data that was not linked into the decompressor. Solve this
> by declaring this symbol in tegra.S when it is being built into the
> decompressor. For the kernel proper, leave the declaration in
> mach-tegra/common.c as explained in the comment.

Tested-by: Alexandre Courbot <acourbot@nvidia.com>

Thanks, I had to workaround this when debugging a kernel uncompression
issue, but have not yet found the time to craft a patch.

Alex.
Alexandre Courbot Aug. 2, 2013, 7:21 a.m. UTC | #2
On Fri, Aug 2, 2013 at 10:59 AM, Alexandre Courbot <gnurou@gmail.com> wrote:
> On Fri, Aug 2, 2013 at 2:51 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> From: Stephen Warren <swarren@nvidia.com>
>>
>> DEBUG_UNCOMPRESS was previously disallowed for Tegra due to tegra.S's
>> use of global data that was not linked into the decompressor. Solve this
>> by declaring this symbol in tegra.S when it is being built into the
>> decompressor. For the kernel proper, leave the declaration in
>> mach-tegra/common.c as explained in the comment.
>
> Tested-by: Alexandre Courbot <acourbot@nvidia.com>
>
> Thanks, I had to workaround this when debugging a kernel uncompression
> issue, but have not yet found the time to craft a patch.

Also on a related note there is a compilation error on
arch/arm/boot/compressed/head.S if the DEBUG macro is defined (useful
to take advantage of puts and putc for early debugging) at least on
Tegra devices. The loadsp macro definition makes use of addruart,
which has been changed to take 3 arguments since about 2 years ago
(commit 639da5ee), but loadsp still uses the 2 arguments form. I'm not
sure how this should be fixed, either by adding another argument to
loadsp (which will turn out to be useless for most implementations) or
by using an arbitrary register in the definition that uses addruart
(probably dangerous).

Alex.
Stephen Warren Aug. 14, 2013, 9:30 p.m. UTC | #3
On 08/01/2013 11:51 AM, Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
> 
> DEBUG_UNCOMPRESS was previously disallowed for Tegra due to tegra.S's
> use of global data that was not linked into the decompressor. Solve this
> by declaring this symbol in tegra.S when it is being built into the
> decompressor. For the kernel proper, leave the declaration in
> mach-tegra/common.c as explained in the comment.
> 
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> ---
> v2: Don't select DEBUG_UNCOMPRESS with DEBUG_TEGRA_UART unless
>     !ZBOOT_ROM. Add notes on ZBOOT_ROM to comment.
> 
> Russell, I assume this should go in through the ARM patch tracker. Or,
> do you want me to take it through the Tegra tree?

Russell, I put this patch into the ARM patch tracker a while back. I
assume it's something you plan to take, or should I put it into the
Tegra tree? Thanks.
Russell King - ARM Linux Aug. 14, 2013, 10:43 p.m. UTC | #4
On Wed, Aug 14, 2013 at 03:30:29PM -0600, Stephen Warren wrote:
> On 08/01/2013 11:51 AM, Stephen Warren wrote:
> > From: Stephen Warren <swarren@nvidia.com>
> > 
> > DEBUG_UNCOMPRESS was previously disallowed for Tegra due to tegra.S's
> > use of global data that was not linked into the decompressor. Solve this
> > by declaring this symbol in tegra.S when it is being built into the
> > decompressor. For the kernel proper, leave the declaration in
> > mach-tegra/common.c as explained in the comment.
> > 
> > Signed-off-by: Stephen Warren <swarren@nvidia.com>
> > ---
> > v2: Don't select DEBUG_UNCOMPRESS with DEBUG_TEGRA_UART unless
> >     !ZBOOT_ROM. Add notes on ZBOOT_ROM to comment.
> > 
> > Russell, I assume this should go in through the ARM patch tracker. Or,
> > do you want me to take it through the Tegra tree?
> 
> Russell, I put this patch into the ARM patch tracker a while back. I
> assume it's something you plan to take, or should I put it into the
> Tegra tree? Thanks.

I haven't applied it yet because it doesn't apply to the branch it needs
to (that being the debug choice cleanup):

Patching 7806/1...
git apply --whitespace=fix -p1 --index --check > /tmp/pdb.12252 2>&1 exited with non-zero status: 256
error: patch failed: arch/arm/Kconfig.debug:1052
error: arch/arm/Kconfig.debug: patch does not apply

Not even with gnu patch:

patching file arch/arm/Kconfig.debug
Hunk #1 FAILED at 1052.
1 out of 1 hunk FAILED -- saving rejects to file arch/arm/Kconfig.debug.rej
patching file arch/arm/include/debug/tegra.S

This is because at the moment, that branch is based before other
changes which have gone in during the -rcs, namely b6992fa9a7.  It
needs that branch rebased before it can be applied.  I'll get to it
eventually.
diff mbox

Patch

diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
index af5d084..7f92c84 100644
--- a/arch/arm/Kconfig.debug
+++ b/arch/arm/Kconfig.debug
@@ -1052,7 +1052,7 @@  config DEBUG_UNCOMPRESS
 	bool
 	depends on ARCH_MULTIPLATFORM || ARCH_MSM
 	default y if DEBUG_LL && !DEBUG_OMAP2PLUS_UART && \
-		     !DEBUG_TEGRA_UART
+		     (!DEBUG_TEGRA_UART || !ZBOOT_ROM)
 	help
 	  This option influences the normal decompressor output for
 	  multiplatform kernels.  Normally, multiplatform kernels disable
diff --git a/arch/arm/include/debug/tegra.S b/arch/arm/include/debug/tegra.S
index 883d7c2..be6a720 100644
--- a/arch/arm/include/debug/tegra.S
+++ b/arch/arm/include/debug/tegra.S
@@ -221,3 +221,32 @@ 
 1002:
 #endif
 		.endm
+
+/*
+ * Storage for the state maintained by the macros above.
+ *
+ * In the kernel proper, this data is located in arch/arm/mach-tegra/common.c.
+ * That's because this header is included from multiple files, and we only
+ * want a single copy of the data. In particular, the UART probing code above
+ * assumes it's running using physical addresses. This is true when this file
+ * is included from head.o, but not when included from debug.o. So we need
+ * to share the probe results between the two copies, rather than having
+ * to re-run the probing again later.
+ *
+ * In the decompressor, we put the symbol/storage right here, since common.c
+ * isn't included in the decompressor build. This symbol gets put in .text
+ * even though it's really data, since .data is discarded from the
+ * decompressor. Luckily, .text is writeable in the decompressor, unless
+ * CONFIG_ZBOOT_ROM. That dependency is handled in arch/arm/Kconfig.debug.
+ */
+#if defined(ZIMAGE)
+tegra_uart_config:
+	/* Debug UART initialization required */
+	.word 1
+	/* Debug UART physical address */
+	.word 0
+	/* Debug UART virtual address */
+	.word 0
+	/* Scratch space for debug macro */
+	.word 0
+#endif