| Message ID | 1358477120-19673-1-git-send-email-shawn.guo@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Friday 18 January 2013, Shawn Guo wrote: > +ENTRY(putc) > + addruart r1, r2, r3 > + waituart r3, r1 > + senduart r0, r1 > + busyuart r3, r1 > + mov pc, lr > +ENDPROC(putc) Ah, so it actually worked? I was expecting at least some part of my code to be wrong ;-) My assembler skills are very much lacking and I had not tried it. Upon closer inspection, it seems that the CR/LF logic from the printascii function is not here, and it probably should be. I also saw that some similar code is already present in arch/arm/boot/compressed/head.S as a local version of putc, maybe that can be combined with this rather than adding a new file. Arnd
On Fri, Jan 18, 2013 at 08:47:50AM +0000, Arnd Bergmann wrote: > On Friday 18 January 2013, Shawn Guo wrote: > > +ENTRY(putc) > > + addruart r1, r2, r3 > > + waituart r3, r1 > > + senduart r0, r1 > > + busyuart r3, r1 > > + mov pc, lr > > +ENDPROC(putc) > > Ah, so it actually worked? I was expecting at least some part of > my code to be wrong ;-) My assembler skills are very much > lacking and I had not tried it. > > Upon closer inspection, it seems that the CR/LF logic from > the printascii function is not here, and it probably should be. No it shouldn't. The CR/LF handling is already done (so actually aliasing it to printch in arch/arm/kernel/debug.S will result in two CRs per LF.
On Fri, Jan 18, 2013 at 08:47:50AM +0000, Arnd Bergmann wrote: > On Friday 18 January 2013, Shawn Guo wrote: > > +ENTRY(putc) > > + addruart r1, r2, r3 > > + waituart r3, r1 > > + senduart r0, r1 > > + busyuart r3, r1 > > + mov pc, lr > > +ENDPROC(putc) > > Ah, so it actually worked? I was expecting at least some part of > my code to be wrong ;-) My assembler skills are very much > lacking and I had not tried it. > Yes, it worked. Actually, I wrote it myself after reading your comment saying take your code as approximation. And then I compared my code with yours and found the only difference is the indent of the last statement :) > Upon closer inspection, it seems that the CR/LF logic from > the printascii function is not here, and it probably should be. > No. As Russell already pointed out, the current implementation of putc() is exactly what putstr() expect, with no CR/LF logic. Shawn
On Thu, Jan 17, 2013 at 6:45 PM, Shawn Guo <shawn.guo@linaro.org> wrote: > Instead of giving zero support of uncompress debug for multiplatform > build, the patch turns uncompress debug into one part of DEBUG_LL > support. When DEBUG_LL is turned on for a particular platform, > uncompress debug works too for that platform. > > It reuses the platform DEBUG_LL macros by creating a simple > arch/arm/boot/compressed/debug.S with CONFIG_DEBUG_LL_INCLUDE > included there, and implements a generic putc() using those macros. > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org> This looks great, and it's even cleaner than I was hoping it'd be based on my suggestion. Nice! :) Russell, do you want to apply this or should we? If the former, for the patch tracker: Acked-by: Olof Johansson <olof@lixom.net> If the latter, let me know and I'll pick it up. Thanks! -Olof
On Fri, Jan 18, 2013 at 10:45:20AM +0800, Shawn Guo wrote: > Instead of giving zero support of uncompress debug for multiplatform > build, the patch turns uncompress debug into one part of DEBUG_LL > support. When DEBUG_LL is turned on for a particular platform, > uncompress debug works too for that platform. > > It reuses the platform DEBUG_LL macros by creating a simple > arch/arm/boot/compressed/debug.S with CONFIG_DEBUG_LL_INCLUDE > included there, and implements a generic putc() using those macros. Ok, I've applied this on the previso that _no one_ in future whinges if the debug infrastructure doesn't quite meet their expectation. The debug infrastructure remains first and foremost that: a simple debug infrastructure suitable for use in the early assembly and the like. That is its primary concern and trumps any requirements from consoles, early printk, decompressor output, and the like.
On Mon, Feb 04, 2013 at 04:01:33PM +0000, Russell King - ARM Linux wrote: > On Fri, Jan 18, 2013 at 10:45:20AM +0800, Shawn Guo wrote: > > Instead of giving zero support of uncompress debug for multiplatform > > build, the patch turns uncompress debug into one part of DEBUG_LL > > support. When DEBUG_LL is turned on for a particular platform, > > uncompress debug works too for that platform. > > > > It reuses the platform DEBUG_LL macros by creating a simple > > arch/arm/boot/compressed/debug.S with CONFIG_DEBUG_LL_INCLUDE > > included there, and implements a generic putc() using those macros. > > Ok, I've applied this on the previso that _no one_ in future whinges if > the debug infrastructure doesn't quite meet their expectation. The > debug infrastructure remains first and foremost that: a simple debug > infrastructure suitable for use in the early assembly and the like. > > That is its primary concern and trumps any requirements from consoles, > early printk, decompressor output, and the like. ... and now I've dropped the two patches because it causes build failures for all OMAP and PXA platforms.
On 02/06/2013 02:32 AM, Russell King - ARM Linux wrote: > On Mon, Feb 04, 2013 at 04:01:33PM +0000, Russell King - ARM Linux wrote: >> On Fri, Jan 18, 2013 at 10:45:20AM +0800, Shawn Guo wrote: >>> Instead of giving zero support of uncompress debug for multiplatform >>> build, the patch turns uncompress debug into one part of DEBUG_LL >>> support. When DEBUG_LL is turned on for a particular platform, >>> uncompress debug works too for that platform. >>> >>> It reuses the platform DEBUG_LL macros by creating a simple >>> arch/arm/boot/compressed/debug.S with CONFIG_DEBUG_LL_INCLUDE >>> included there, and implements a generic putc() using those macros. >> >> Ok, I've applied this on the previso that _no one_ in future whinges if >> the debug infrastructure doesn't quite meet their expectation. The >> debug infrastructure remains first and foremost that: a simple debug >> infrastructure suitable for use in the early assembly and the like. >> >> That is its primary concern and trumps any requirements from consoles, >> early printk, decompressor output, and the like. > > ... and now I've dropped the two patches because it causes build failures > for all OMAP and PXA platforms. It also breaks tegra_defconfig. For reference, the (or perhaps just a) reason here is that arch/arm/include/debug/tegra.S references symbol tegra_uart_config, which is declared in arch/arm/mach-tegra/common.c, which isn't part of the decompressor build. Tegra's uncompress.h doesn't touch this symbol, hence avoids this problem. I'd guess OMAP is broken for similar reasons, since when I created the tegra_uart_config symbol I was inspired by the OMAP DEBUG_LL code. Perhaps the patch can be re-cast to only affect multi-platform kernels, and leave unconverted platforms using uncompress.h (at least, I assume that must be the problem).
diff --git a/arch/arm/boot/compressed/Makefile b/arch/arm/boot/compressed/Makefile index 5cad8a6..c9865f6 100644 --- a/arch/arm/boot/compressed/Makefile +++ b/arch/arm/boot/compressed/Makefile @@ -24,6 +24,9 @@ endif AFLAGS_head.o += -DTEXT_OFFSET=$(TEXT_OFFSET) HEAD = head.o OBJS += misc.o decompress.o +ifeq ($(CONFIG_DEBUG_LL),y) +OBJS += debug.o +endif FONTC = $(srctree)/drivers/video/console/font_acorn_8x8.c # string library code (-Os is enforced to keep it much smaller) diff --git a/arch/arm/boot/compressed/debug.S b/arch/arm/boot/compressed/debug.S new file mode 100644 index 0000000..bdb0e25 --- /dev/null +++ b/arch/arm/boot/compressed/debug.S @@ -0,0 +1,11 @@ +#include <linux/linkage.h> + +#include CONFIG_DEBUG_LL_INCLUDE + +ENTRY(putc) + addruart r1, r2, r3 + waituart r3, r1 + senduart r0, r1 + busyuart r3, r1 + mov pc, lr +ENDPROC(putc) diff --git a/arch/arm/include/debug/uncompress.h b/arch/arm/include/debug/uncompress.h index e19955d..9aa5314 100644 --- a/arch/arm/include/debug/uncompress.h +++ b/arch/arm/include/debug/uncompress.h @@ -1,3 +1,7 @@ +#ifdef CONFIG_DEBUG_LL +extern void putc(int c); +#else static inline void putc(int c) {} +#endif static inline void flush(void) {} static inline void arch_decomp_setup(void) {}
Instead of giving zero support of uncompress debug for multiplatform build, the patch turns uncompress debug into one part of DEBUG_LL support. When DEBUG_LL is turned on for a particular platform, uncompress debug works too for that platform. It reuses the platform DEBUG_LL macros by creating a simple arch/arm/boot/compressed/debug.S with CONFIG_DEBUG_LL_INCLUDE included there, and implements a generic putc() using those macros. Signed-off-by: Shawn Guo <shawn.guo@linaro.org> --- arch/arm/boot/compressed/Makefile | 3 +++ arch/arm/boot/compressed/debug.S | 11 +++++++++++ arch/arm/include/debug/uncompress.h | 4 ++++ 3 files changed, 18 insertions(+) create mode 100644 arch/arm/boot/compressed/debug.S