| Message ID | nycvar.YSQ.7.76.1710311346190.21665@knanqh.ubzr (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Oct 31, 2017 at 01:48:01PM -0400, Nicolas Pitre wrote: > On Tue, 31 Oct 2017, Russell King - ARM Linux wrote: > > > On Tue, Oct 31, 2017 at 01:06:35PM -0400, Nicolas Pitre wrote: > > > That's easy to veryfy with this patch: > > > > Unfortunately not that easy, this patch breaks printch. > > Right, missed that. > > New patch below: > > diff --git a/arch/arm/kernel/debug.S b/arch/arm/kernel/debug.S > index ea9646cc2a..0d1cef4b6e 100644 > --- a/arch/arm/kernel/debug.S > +++ b/arch/arm/kernel/debug.S > @@ -79,25 +79,28 @@ hexbuf: .space 16 > > ENTRY(printascii) > addruart_current r3, r1, r2 > - b 2f > -1: waituart r2, r3 > - senduart r1, r3 > - busyuart r2, r3 > - teq r1, #'\n' > - moveq r1, #'\r' > - beq 1b > -2: teq r0, #0 > +1: teq r0, #0 > ldrneb r1, [r0], #1 > teqne r1, #0 > - bne 1b > - ret lr > + reteq lr > + teq r1, #'\n' > + bne 2f > + mov r1, '\r' > + waituart r2, r3 > + senduart r1, r3 > + busyuart r2, r3 > + mov r1, '\n' > +2: waituart r2, r3 > + senduart r1, r3 > + busyuart r2, r3 > + b 1b > ENDPROC(printascii) > > ENTRY(printch) > addruart_current r3, r1, r2 > mov r1, r0 > mov r0, #0 > - b 1b > + b 2b > ENDPROC(printch) Still, not quite! printch('\n') with the old code would do this (in terms of executed instructions): mov r1, r0 mov r0, #0 b 1b 1: waituart r2, r3 senduart r1, r3 busyuart r2, r3 teq r1, #'\n' moveq r1, #'\r' beq 1b 1: waituart r2, r3 senduart r1, r3 busyuart r2, r3 teq r1, #'\n' ... 2: teq r0, #0 ret lr So a printch('\n') produces "\n\r" on the UART. If we're fixing printascii() to emit "\r\n" instead of "\n\r" for a '\n', then printch() should have the same fix, and should not truncate to just '\n'.
On Tue, 31 Oct 2017, Russell King - ARM Linux wrote: > On Tue, Oct 31, 2017 at 01:48:01PM -0400, Nicolas Pitre wrote: > > On Tue, 31 Oct 2017, Russell King - ARM Linux wrote: > > > > > On Tue, Oct 31, 2017 at 01:06:35PM -0400, Nicolas Pitre wrote: > > > > That's easy to veryfy with this patch: > > > > > > Unfortunately not that easy, this patch breaks printch. > > > > Right, missed that. > > > > New patch below: > > > > diff --git a/arch/arm/kernel/debug.S b/arch/arm/kernel/debug.S > > index ea9646cc2a..0d1cef4b6e 100644 > > --- a/arch/arm/kernel/debug.S > > +++ b/arch/arm/kernel/debug.S > > @@ -79,25 +79,28 @@ hexbuf: .space 16 > > > > ENTRY(printascii) > > addruart_current r3, r1, r2 > > - b 2f > > -1: waituart r2, r3 > > - senduart r1, r3 > > - busyuart r2, r3 > > - teq r1, #'\n' > > - moveq r1, #'\r' > > - beq 1b > > -2: teq r0, #0 > > +1: teq r0, #0 > > ldrneb r1, [r0], #1 > > teqne r1, #0 > > - bne 1b > > - ret lr > > + reteq lr > > + teq r1, #'\n' > > + bne 2f > > + mov r1, '\r' > > + waituart r2, r3 > > + senduart r1, r3 > > + busyuart r2, r3 > > + mov r1, '\n' > > +2: waituart r2, r3 > > + senduart r1, r3 > > + busyuart r2, r3 > > + b 1b > > ENDPROC(printascii) > > > > ENTRY(printch) > > addruart_current r3, r1, r2 > > mov r1, r0 > > mov r0, #0 > > - b 1b > > + b 2b > > ENDPROC(printch) > > Still, not quite! printch('\n') with the old code would do this > (in terms of executed instructions): > > mov r1, r0 > mov r0, #0 > b 1b > 1: waituart r2, r3 > senduart r1, r3 > busyuart r2, r3 > teq r1, #'\n' > moveq r1, #'\r' > beq 1b > 1: waituart r2, r3 > senduart r1, r3 > busyuart r2, r3 > teq r1, #'\n' > ... > 2: teq r0, #0 > ret lr > > So a printch('\n') produces "\n\r" on the UART. If we're fixing > printascii() to emit "\r\n" instead of "\n\r" for a '\n', then > printch() should have the same fix, and should not truncate to > just '\n'. OK... That's easy to achieve, but is it desirable? That would imply that early_write() before my patch used to be wrong wrt its princh() expectation as it did the extra \r manually on top. This probably went unnoticed given that "\r\n\r" produces the same display result as "\r\n". IMHO the semantics of a single character should be just that: a single character. And if some user relied on printch('\n') inserting the \r automatically then this would have run into the same display issue we're attempting to fix here. That might be why early_write() did the extra \r itself. Thing is: that only printch user user is now gone. So I'd fix the semantic issue by simply removing printch altogether. What do you think? Nicolas
On Tue, Oct 31, 2017 at 02:15:14PM -0400, Nicolas Pitre wrote: > On Tue, 31 Oct 2017, Russell King - ARM Linux wrote: > > On Tue, Oct 31, 2017 at 01:48:01PM -0400, Nicolas Pitre wrote: > > > On Tue, 31 Oct 2017, Russell King - ARM Linux wrote: > > > > > > > On Tue, Oct 31, 2017 at 01:06:35PM -0400, Nicolas Pitre wrote: > > > > > That's easy to veryfy with this patch: > > > > > > > > Unfortunately not that easy, this patch breaks printch. > > > > > > Right, missed that. > > > > > > New patch below: > > > > > > diff --git a/arch/arm/kernel/debug.S b/arch/arm/kernel/debug.S > > > index ea9646cc2a..0d1cef4b6e 100644 > > > --- a/arch/arm/kernel/debug.S > > > +++ b/arch/arm/kernel/debug.S > > > @@ -79,25 +79,28 @@ hexbuf: .space 16 > > > > > > ENTRY(printascii) > > > addruart_current r3, r1, r2 > > > - b 2f > > > -1: waituart r2, r3 > > > - senduart r1, r3 > > > - busyuart r2, r3 > > > - teq r1, #'\n' > > > - moveq r1, #'\r' > > > - beq 1b > > > -2: teq r0, #0 > > > +1: teq r0, #0 > > > ldrneb r1, [r0], #1 > > > teqne r1, #0 > > > - bne 1b > > > - ret lr > > > + reteq lr > > > + teq r1, #'\n' > > > + bne 2f > > > + mov r1, '\r' > > > + waituart r2, r3 > > > + senduart r1, r3 > > > + busyuart r2, r3 > > > + mov r1, '\n' > > > +2: waituart r2, r3 > > > + senduart r1, r3 > > > + busyuart r2, r3 > > > + b 1b > > > ENDPROC(printascii) > > > > > > ENTRY(printch) > > > addruart_current r3, r1, r2 > > > mov r1, r0 > > > mov r0, #0 > > > - b 1b > > > + b 2b > > > ENDPROC(printch) > > > > Still, not quite! printch('\n') with the old code would do this > > (in terms of executed instructions): > > > > mov r1, r0 > > mov r0, #0 > > b 1b > > 1: waituart r2, r3 > > senduart r1, r3 > > busyuart r2, r3 > > teq r1, #'\n' > > moveq r1, #'\r' > > beq 1b > > 1: waituart r2, r3 > > senduart r1, r3 > > busyuart r2, r3 > > teq r1, #'\n' > > ... > > 2: teq r0, #0 > > ret lr > > > > So a printch('\n') produces "\n\r" on the UART. If we're fixing > > printascii() to emit "\r\n" instead of "\n\r" for a '\n', then > > printch() should have the same fix, and should not truncate to > > just '\n'. > > OK... That's easy to achieve, but is it desirable? Yes - remember, these are supposed to be usable from assembly, and we really don't want to have the complexity of: mov r0, #'\r' bl printch mov r0, #'\n' bl printch each time we want to begin a new line. > IMHO the semantics of a single character should be just that: a single > character. And if some user relied on printch('\n') inserting the \r > automatically then this would have run into the same display issue we're > attempting to fix here. That might be why early_write() did the extra \r > itself. You're thinking about these functions as C functions that you'd call from C code. That's not their primary purpose or intention, they're for debugging assembly. They just happen to be re-used for the early printk crap because people find these a convenient implementation. > Thing is: that only printch user user is now gone. So I'd fix the > semantic issue by simply removing printch altogether. No. You could make the same argument for the printhex*() functions too, but removing them means next time we need to stuff in some debug that prints hex numbers before the MMU is up, we need to reinvent those wheels yet again. The point of this assembly is to provide a set of debugging functions that can be used from assembly. They were never really meant for C code to use.
On Tuesday, October 31, 2017, Nicolas Pitre wrote: > On Tue, 31 Oct 2017, Russell King - ARM Linux wrote: > > On Tue, Oct 31, 2017 at 02:15:14PM -0400, Nicolas Pitre wrote: > > > On Tue, 31 Oct 2017, Russell King - ARM Linux wrote: > > > > So a printch('\n') produces "\n\r" on the UART. If we're fixing > > > > printascii() to emit "\r\n" instead of "\n\r" for a '\n', then > > > > printch() should have the same fix, and should not truncate to > > > > just '\n'. > > > > > > OK... That's easy to achieve, but is it desirable? > > > > Yes - remember, these are supposed to be usable from assembly, > > and we really don't want to have the complexity of: > > > > mov r0, #'\r' > > bl printch > > mov r0, #'\n' > > bl printch > > > > each time we want to begin a new line. > > Fine with me. > > diff --git a/arch/arm/kernel/debug.S b/arch/arm/kernel/debug.S > index ea9646cc2a..01d746efff 100644 > --- a/arch/arm/kernel/debug.S > +++ b/arch/arm/kernel/debug.S > @@ -79,25 +79,28 @@ hexbuf: .space 16 > > ENTRY(printascii) > addruart_current r3, r1, r2 > - b 2f > -1: waituart r2, r3 > - senduart r1, r3 > - busyuart r2, r3 > - teq r1, #'\n' > - moveq r1, #'\r' > - beq 1b > -2: teq r0, #0 > +1: teq r0, #0 > ldrneb r1, [r0], #1 > teqne r1, #0 > - bne 1b > - ret lr > + reteq lr > +2: teq r1, #'\n' > + bne 3f > + mov r1, '\r' > + waituart r2, r3 > + senduart r1, r3 > + busyuart r2, r3 > + mov r1, '\n' > +3: waituart r2, r3 > + senduart r1, r3 > + busyuart r2, r3 > + b 1b > ENDPROC(printascii) > > ENTRY(printch) > addruart_current r3, r1, r2 > mov r1, r0 > mov r0, #0 > - b 1b > + b 2b > ENDPROC(printch) > > #ifdef CONFIG_MMU This patch worked for me. I get my carriage returns again. Thanks. Chris
On Tue, 31 Oct 2017, Chris Brandt wrote: > On Tuesday, October 31, 2017, Nicolas Pitre wrote: > > On Tue, 31 Oct 2017, Russell King - ARM Linux wrote: > > > On Tue, Oct 31, 2017 at 02:15:14PM -0400, Nicolas Pitre wrote: > > > > On Tue, 31 Oct 2017, Russell King - ARM Linux wrote: > > > > > So a printch('\n') produces "\n\r" on the UART. If we're fixing > > > > > printascii() to emit "\r\n" instead of "\n\r" for a '\n', then > > > > > printch() should have the same fix, and should not truncate to > > > > > just '\n'. > > > > > > > > OK... That's easy to achieve, but is it desirable? > > > > > > Yes - remember, these are supposed to be usable from assembly, > > > and we really don't want to have the complexity of: > > > > > > mov r0, #'\r' > > > bl printch > > > mov r0, #'\n' > > > bl printch > > > > > > each time we want to begin a new line. > > > > Fine with me. > > > > diff --git a/arch/arm/kernel/debug.S b/arch/arm/kernel/debug.S > > index ea9646cc2a..01d746efff 100644 > > --- a/arch/arm/kernel/debug.S > > +++ b/arch/arm/kernel/debug.S > > @@ -79,25 +79,28 @@ hexbuf: .space 16 > > > > ENTRY(printascii) > > addruart_current r3, r1, r2 > > - b 2f > > -1: waituart r2, r3 > > - senduart r1, r3 > > - busyuart r2, r3 > > - teq r1, #'\n' > > - moveq r1, #'\r' > > - beq 1b > > -2: teq r0, #0 > > +1: teq r0, #0 > > ldrneb r1, [r0], #1 > > teqne r1, #0 > > - bne 1b > > - ret lr > > + reteq lr > > +2: teq r1, #'\n' > > + bne 3f > > + mov r1, '\r' > > + waituart r2, r3 > > + senduart r1, r3 > > + busyuart r2, r3 > > + mov r1, '\n' > > +3: waituart r2, r3 > > + senduart r1, r3 > > + busyuart r2, r3 > > + b 1b > > ENDPROC(printascii) > > > > ENTRY(printch) > > addruart_current r3, r1, r2 > > mov r1, r0 > > mov r0, #0 > > - b 1b > > + b 2b > > ENDPROC(printch) > > > > #ifdef CONFIG_MMU > > > This patch worked for me. > I get my carriage returns again. Good! Queued as patch #8717. Nicolas
On Tue, Oct 31, 2017 at 03:28:11PM -0400, Nicolas Pitre wrote: > On Tue, 31 Oct 2017, Chris Brandt wrote: > > This patch worked for me. > > I get my carriage returns again. > > Good! Queued as patch #8717. I was going to say that there's another implementation of the same in arch/arm/boot/compressed/head.S, and if we fix one we should apply the same fix to the other.
On Tue, Oct 31, 2017 at 07:12:32PM +0000, Chris Brandt wrote: > On Tuesday, October 31, 2017, Nicolas Pitre wrote: > > On Tue, 31 Oct 2017, Russell King - ARM Linux wrote: > > > On Tue, Oct 31, 2017 at 02:15:14PM -0400, Nicolas Pitre wrote: > > > > On Tue, 31 Oct 2017, Russell King - ARM Linux wrote: > > > > > So a printch('\n') produces "\n\r" on the UART. If we're fixing > > > > > printascii() to emit "\r\n" instead of "\n\r" for a '\n', then > > > > > printch() should have the same fix, and should not truncate to > > > > > just '\n'. > > > > > > > > OK... That's easy to achieve, but is it desirable? > > > > > > Yes - remember, these are supposed to be usable from assembly, > > > and we really don't want to have the complexity of: > > > > > > mov r0, #'\r' > > > bl printch > > > mov r0, #'\n' > > > bl printch > > > > > > each time we want to begin a new line. > > > > Fine with me. > > > > diff --git a/arch/arm/kernel/debug.S b/arch/arm/kernel/debug.S > > index ea9646cc2a..01d746efff 100644 > > --- a/arch/arm/kernel/debug.S > > +++ b/arch/arm/kernel/debug.S > > @@ -79,25 +79,28 @@ hexbuf: .space 16 > > > > ENTRY(printascii) > > addruart_current r3, r1, r2 > > - b 2f > > -1: waituart r2, r3 > > - senduart r1, r3 > > - busyuart r2, r3 > > - teq r1, #'\n' > > - moveq r1, #'\r' > > - beq 1b > > -2: teq r0, #0 > > +1: teq r0, #0 > > ldrneb r1, [r0], #1 > > teqne r1, #0 > > - bne 1b > > - ret lr > > + reteq lr > > +2: teq r1, #'\n' > > + bne 3f > > + mov r1, '\r' > > + waituart r2, r3 > > + senduart r1, r3 > > + busyuart r2, r3 > > + mov r1, '\n' > > +3: waituart r2, r3 > > + senduart r1, r3 > > + busyuart r2, r3 > > + b 1b > > ENDPROC(printascii) > > > > ENTRY(printch) > > addruart_current r3, r1, r2 > > mov r1, r0 > > mov r0, #0 > > - b 1b > > + b 2b > > ENDPROC(printch) > > > > #ifdef CONFIG_MMU > > > This patch worked for me. > I get my carriage returns again. Sorry, but no. This is crap. The kernelci.org test resulting from the tree I pushed out this evening with both of the patches in is very unhappy: 42 arch/arm/kernel/debug.S:98: Error: immediate expression requires a # prefix -- `mov r1,10' 42 arch/arm/kernel/debug.S:94: Error: immediate expression requires a # prefix -- `mov r1,13' I can't believe that anyone actually build-tested this patch as it stands - maybe, Chris, you just think you did but you ended up testing something else? Or maybe your binutils is broken because it now accepts constants without the preceding '#' ? Shrug, whatever, Nico's patches are broken, and it's way too late to go through another round of potential broken-ness. Dropping both from my tree until after the merge window. Sorry. Last minute fixes hardly ever work out.
On Thu, 2 Nov 2017, Russell King - ARM Linux wrote: > On Tue, Oct 31, 2017 at 07:12:32PM +0000, Chris Brandt wrote: > > On Tuesday, October 31, 2017, Nicolas Pitre wrote: > > > On Tue, 31 Oct 2017, Russell King - ARM Linux wrote: > > > > On Tue, Oct 31, 2017 at 02:15:14PM -0400, Nicolas Pitre wrote: > > > > > On Tue, 31 Oct 2017, Russell King - ARM Linux wrote: > > > > > > So a printch('\n') produces "\n\r" on the UART. If we're fixing > > > > > > printascii() to emit "\r\n" instead of "\n\r" for a '\n', then > > > > > > printch() should have the same fix, and should not truncate to > > > > > > just '\n'. > > > > > > > > > > OK... That's easy to achieve, but is it desirable? > > > > > > > > Yes - remember, these are supposed to be usable from assembly, > > > > and we really don't want to have the complexity of: > > > > > > > > mov r0, #'\r' > > > > bl printch > > > > mov r0, #'\n' > > > > bl printch > > > > > > > > each time we want to begin a new line. > > > > > > Fine with me. > > > > > > diff --git a/arch/arm/kernel/debug.S b/arch/arm/kernel/debug.S > > > index ea9646cc2a..01d746efff 100644 > > > --- a/arch/arm/kernel/debug.S > > > +++ b/arch/arm/kernel/debug.S > > > @@ -79,25 +79,28 @@ hexbuf: .space 16 > > > > > > ENTRY(printascii) > > > addruart_current r3, r1, r2 > > > - b 2f > > > -1: waituart r2, r3 > > > - senduart r1, r3 > > > - busyuart r2, r3 > > > - teq r1, #'\n' > > > - moveq r1, #'\r' > > > - beq 1b > > > -2: teq r0, #0 > > > +1: teq r0, #0 > > > ldrneb r1, [r0], #1 > > > teqne r1, #0 > > > - bne 1b > > > - ret lr > > > + reteq lr > > > +2: teq r1, #'\n' > > > + bne 3f > > > + mov r1, '\r' > > > + waituart r2, r3 > > > + senduart r1, r3 > > > + busyuart r2, r3 > > > + mov r1, '\n' > > > +3: waituart r2, r3 > > > + senduart r1, r3 > > > + busyuart r2, r3 > > > + b 1b > > > ENDPROC(printascii) > > > > > > ENTRY(printch) > > > addruart_current r3, r1, r2 > > > mov r1, r0 > > > mov r0, #0 > > > - b 1b > > > + b 2b > > > ENDPROC(printch) > > > > > > #ifdef CONFIG_MMU > > > > > > This patch worked for me. > > I get my carriage returns again. > > Sorry, but no. This is crap. > > The kernelci.org test resulting from the tree I pushed out this evening > with both of the patches in is very unhappy: > > 42 arch/arm/kernel/debug.S:98: Error: immediate expression requires a # prefix -- `mov r1,10' > 42 arch/arm/kernel/debug.S:94: Error: immediate expression requires a # prefix -- `mov r1,13' > > I can't believe that anyone actually build-tested this patch as it > stands - maybe, Chris, you just think you did but you ended up > testing something else? Or maybe your binutils is broken because > it now accepts constants without the preceding '#' ? Well... I don't know what happened with Chris' testing either. I *thought* I build tested it, but my .config had CONFIG_DEBUG_SEMIHOSTING=y. Oh well... Nicolas
On Wednesday, November 01, 2017 1, Nicolas Pitre wrote: > > > This patch worked for me. > > > I get my carriage returns again. > > > > Sorry, but no. This is crap. > > > > The kernelci.org test resulting from the tree I pushed out this evening > > with both of the patches in is very unhappy: > > > > 42 arch/arm/kernel/debug.S:98: Error: immediate expression > requires a # prefix -- `mov r1,10' > > 42 arch/arm/kernel/debug.S:94: Error: immediate expression > requires a # prefix -- `mov r1,13' > > > > I can't believe that anyone actually build-tested this patch as it > > stands - maybe, Chris, you just think you did but you ended up > > testing something else? Or maybe your binutils is broken because > > it now accepts constants without the preceding '#' ? > > Well... I don't know what happened with Chris' testing either. So, just to show I'm not crazy... # Yes, patch was applied: $ grep "mov r1" arch/arm/kernel/debug.S mov r1, #8 mov r1, #4 mov r1, #2 mov r1, #0 mov r1, '\r' <<<<<<<<<< mov r1, '\n' <<<<<<<<<< mov r1, r0 mov r1, r0 # Yes it builds: $ touch arch/arm/kernel/debug.S $ make -j8 O=.out uImage LOADADDR=0x08008000 make[1]: Entering directory '/home/renesas/tools/upstream/renesas-drivers/.out' CHK include/config/kernel.release GEN ./Makefile CHK include/generated/uapi/linux/version.h CHK scripts/mod/devicetable-offsets.h UPD include/config/kernel.release Using .. as source for kernel CHK include/generated/utsrelease.h UPD include/generated/utsrelease.h CHK include/generated/timeconst.h CHK include/generated/bounds.h CHK include/generated/asm-offsets.h CALL ../scripts/checksyscalls.sh CHK include/generated/compile.h CC init/version.o AS arch/arm/kernel/debug.o <<<<<<<<<< # Have a look at the output: $ arm-linux-gnueabihf-objdump -SdCg .out/arch/arm/kernel/debug.o > /tmp/debug.lst $ cat /tmp/debug.lst ------------------------------------------------------------ mov r1, '\r' <<<<<<<<<< 70: f04f 010d mov.w r1, #13 <<<<<<<<<< waituart r2, r3 74: 8a1a ldrh r2, [r3, #16] 76: f012 0f20 tst.w r2, #32 7a: d0fb beq.n 74 <printascii+0x2c> senduart r1, r3 7c: 7319 strb r1, [r3, #12] 7e: 8a19 ldrh r1, [r3, #16] 80: f021 0140 bic.w r1, r1, #64 ; 0x40 84: 8219 strh r1, [r3, #16] busyuart r2, r3 86: 8a1a ldrh r2, [r3, #16] 88: f012 0f40 tst.w r2, #64 ; 0x40 8c: d0fb beq.n 86 <printascii+0x3e> mov r1, '\n' <<<<<<<<<< 8e: f04f 010a mov.w r1, #10 <<<<<<<<<< ------------------------------------------------------------ So, the compiler realized what you wanted to do even if there wasn't a # in front of the constant. $ arm-linux-gnueabihf-gcc --version arm-linux-gnueabihf-gcc (Linaro GCC 5.4-2017.01) 5.4.1 20161213 Copyright (C) 2015 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. Chris
On Thu, Nov 02, 2017 at 11:06:54AM +0000, Chris Brandt wrote: > On Wednesday, November 01, 2017 1, Nicolas Pitre wrote: > > > > This patch worked for me. > > > > I get my carriage returns again. > > > > > > Sorry, but no. This is crap. > > > > > > The kernelci.org test resulting from the tree I pushed out this evening > > > with both of the patches in is very unhappy: > > > > > > 42 arch/arm/kernel/debug.S:98: Error: immediate expression > > requires a # prefix -- `mov r1,10' > > > 42 arch/arm/kernel/debug.S:94: Error: immediate expression > > requires a # prefix -- `mov r1,13' > > > > > > I can't believe that anyone actually build-tested this patch as it > > > stands - maybe, Chris, you just think you did but you ended up > > > testing something else? Or maybe your binutils is broken because > > > it now accepts constants without the preceding '#' ? > > > > Well... I don't know what happened with Chris' testing either. > > > So, just to show I'm not crazy... > > > # Yes, patch was applied: > $ grep "mov r1" arch/arm/kernel/debug.S > mov r1, #8 > mov r1, #4 > mov r1, #2 > mov r1, #0 > mov r1, '\r' <<<<<<<<<< > mov r1, '\n' <<<<<<<<<< > mov r1, r0 > mov r1, r0 > > # Yes it builds: > $ touch arch/arm/kernel/debug.S > $ make -j8 O=.out uImage LOADADDR=0x08008000 > make[1]: Entering directory '/home/renesas/tools/upstream/renesas-drivers/.out' > CHK include/config/kernel.release > GEN ./Makefile > CHK include/generated/uapi/linux/version.h > CHK scripts/mod/devicetable-offsets.h > UPD include/config/kernel.release > Using .. as source for kernel > CHK include/generated/utsrelease.h > UPD include/generated/utsrelease.h > CHK include/generated/timeconst.h > CHK include/generated/bounds.h > CHK include/generated/asm-offsets.h > CALL ../scripts/checksyscalls.sh > CHK include/generated/compile.h > CC init/version.o > AS arch/arm/kernel/debug.o <<<<<<<<<< > > > # Have a look at the output: > $ arm-linux-gnueabihf-objdump -SdCg .out/arch/arm/kernel/debug.o > /tmp/debug.lst > $ cat /tmp/debug.lst > > ------------------------------------------------------------ > mov r1, '\r' <<<<<<<<<< > 70: f04f 010d mov.w r1, #13 <<<<<<<<<< > waituart r2, r3 > 74: 8a1a ldrh r2, [r3, #16] > 76: f012 0f20 tst.w r2, #32 > 7a: d0fb beq.n 74 <printascii+0x2c> > senduart r1, r3 > 7c: 7319 strb r1, [r3, #12] > 7e: 8a19 ldrh r1, [r3, #16] > 80: f021 0140 bic.w r1, r1, #64 ; 0x40 > 84: 8219 strh r1, [r3, #16] > busyuart r2, r3 > 86: 8a1a ldrh r2, [r3, #16] > 88: f012 0f40 tst.w r2, #64 ; 0x40 > 8c: d0fb beq.n 86 <printascii+0x3e> > mov r1, '\n' <<<<<<<<<< > 8e: f04f 010a mov.w r1, #10 <<<<<<<<<< > ------------------------------------------------------------ > > > So, the compiler realized what you wanted to do even if there wasn't a > # in front of the constant. > > > $ arm-linux-gnueabihf-gcc --version > arm-linux-gnueabihf-gcc (Linaro GCC 5.4-2017.01) 5.4.1 20161213 > Copyright (C) 2015 Free Software Foundation, Inc. > This is free software; see the source for copying conditions. There is NO > warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. The compiler is only involved for the C pre-processor front-end. It's not involved in parsing the resulting assembly - as far as gcc is concerned, it could be forth in the post-processed file. GCC will then pass the post-processed output to binutils 'as' to do the actual assembly, and that's what should complain. Most people's assemblers will object to the second instruction: mov r0, #'\r' mov r1, '\r' $ arm-linux-as -o /dev/null t.s t.s: Assembler messages: t.s:2: Error: immediate expression requires a # prefix -- `mov r1,13' The reason being that the ARM instruction set has, for a few decades now, required the # for constants. There are two possibilities: 1) Your binutils version is buggy, in that it now accepts constants in ARM assembly without a preceding # and binutils needs to be fixed. 2) The change in binutils is a gratuitous change - which is a really stupidly dumb thing to do because it means that we'll end up in this exact situation and it breaks the established norm that has been present for a long time. Either way, the fact is that many binutils versions out there will not accept the syntax that Nicolas used, and therefore the patch is broken as far as the kernel is concerned. So, as far as ARM assembly in the Linux kernel goes, all constants must be preceded by # whether or not binutils requires it - no exceptions. Please always test assembly changes with a binutils version that is not gratuitously broken!
On Thursday, November 02, 2017, Russell King - ARM Linux wrote: > The compiler is only involved for the C pre-processor front-end. It's > not involved in parsing the resulting assembly - as far as gcc is > concerned, it could be forth in the post-processed file. > > GCC will then pass the post-processed output to binutils 'as' to do the > actual assembly, and that's what should complain. Oops, I meant to show this: $ arm-linux-gnueabihf-as -version GNU assembler (Linaro_Binutils-2017.01) 2.25.2 Linaro 2016_02 Copyright (C) 2014 Free Software Foundation, Inc. This program is free software; you may redistribute it under the terms of the GNU General Public License version 3 or later. This program has absolutely no warranty. This assembler was configured for a target of `arm-linux-gnueabihf'. > So, as far as ARM assembly in the Linux kernel goes, all constants must > be preceded by # whether or not binutils requires it - no exceptions. > Please always test assembly changes with a binutils version that is not > gratuitously broken! Somewhat ironic since Nicolas works for Linaro. Chris
On Thu, Nov 02, 2017 at 11:28:55AM +0000, Chris Brandt wrote: > On Thursday, November 02, 2017, Russell King - ARM Linux wrote: > > So, as far as ARM assembly in the Linux kernel goes, all constants must > > be preceded by # whether or not binutils requires it - no exceptions. > > Please always test assembly changes with a binutils version that is not > > gratuitously broken! > > Somewhat ironic since Nicolas works for Linaro. Sorry, I don't see the connection. As Nicolas has already admitted, he didn't actually test the patch because the code was configured out of his test build.
On Thu, 2 Nov 2017, Chris Brandt wrote: > Oops, I meant to show this: > > $ arm-linux-gnueabihf-as -version > GNU assembler (Linaro_Binutils-2017.01) 2.25.2 Linaro 2016_02 > Copyright (C) 2014 Free Software Foundation, Inc. > This program is free software; you may redistribute it under the terms of > the GNU General Public License version 3 or later. > This program has absolutely no warranty. > This assembler was configured for a target of `arm-linux-gnueabihf'. This is very strange. Here's a quick test with all the binutils versions I have lying around on my system: $ echo -e "mov r0, #'\\\r'\nmov r1, '\\\r'" mov r0, #'\r' mov r1, '\r' $ echo -e "mov r0, #'\\\r'\nmov r1, '\\\r'" > /tmp/t.s $ for as in $(find /opt -name arm-linux-\*-as); do \ > $as --version | head -1; \ > $as /tmp/t.s -o /tmp/t.o; done GNU assembler (Linaro_Binutils-2017.05) 2.27.0.20161019 /tmp/t.s: Assembler messages: /tmp/t.s:2: Error: immediate expression requires a # prefix -- `mov r1,13' GNU assembler (crosstool-NG linaro-1.13.1-4.7-2013.02-01-20130221 - Linaro GCC 2013.02) 2.23.1 /tmp/t.s: Assembler messages: /tmp/t.s:2: Error: immediate expression requires a # prefix -- `mov r1,13' GNU assembler (Linaro_Binutils-2017.05) 2.25.2 Linaro 2016_02 /tmp/t.s: Assembler messages: /tmp/t.s:2: Error: immediate expression requires a # prefix -- `mov r1,13' GNU assembler (crosstool-NG linaro-1.13.1-4.9-2014.09 - Linaro GCC 4.9-2014.09) 2.24.0.20140829 Linaro 2014.09 /tmp/t.s: Assembler messages: /tmp/t.s:2: Error: immediate expression requires a # prefix -- `mov r1,13' GNU assembler (GNU Binutils) Linaro 2014.11-3-git 2.24.0.20141017 /tmp/t.s: Assembler messages: /tmp/t.s:2: Error: immediate expression requires a # prefix -- `mov r1,13' GNU assembler (Linaro_Binutils-2017.08) 2.28.2.20170706 /tmp/t.s: Assembler messages: /tmp/t.s:2: Error: immediate expression requires a # prefix -- `mov r1,13' They all fail, including the version that looks like the one you have. Could you try that little test above on your side? > > So, as far as ARM assembly in the Linux kernel goes, all constants must > > be preceded by # whether or not binutils requires it - no exceptions. > > Please always test assembly changes with a binutils version that is not > > gratuitously broken! > > Somewhat ironic since Nicolas works for Linaro. I'm not involved with the toolchain people though, other than using their output. Nicolas
On 02/11/17 15:04, Nicolas Pitre wrote: > On Thu, 2 Nov 2017, Chris Brandt wrote: > >> Oops, I meant to show this: >> >> $ arm-linux-gnueabihf-as -version >> GNU assembler (Linaro_Binutils-2017.01) 2.25.2 Linaro 2016_02 >> Copyright (C) 2014 Free Software Foundation, Inc. >> This program is free software; you may redistribute it under the terms of >> the GNU General Public License version 3 or later. >> This program has absolutely no warranty. >> This assembler was configured for a target of `arm-linux-gnueabihf'. > > This is very strange. Here's a quick test with all the binutils versions > I have lying around on my system: > > $ echo -e "mov r0, #'\\\r'\nmov r1, '\\\r'" > mov r0, #'\r' > mov r1, '\r' > $ echo -e "mov r0, #'\\\r'\nmov r1, '\\\r'" > /tmp/t.s > $ for as in $(find /opt -name arm-linux-\*-as); do \ >> $as --version | head -1; \ >> $as /tmp/t.s -o /tmp/t.o; done > GNU assembler (Linaro_Binutils-2017.05) 2.27.0.20161019 > /tmp/t.s: Assembler messages: > /tmp/t.s:2: Error: immediate expression requires a # prefix -- `mov r1,13' > GNU assembler (crosstool-NG linaro-1.13.1-4.7-2013.02-01-20130221 - Linaro GCC 2013.02) 2.23.1 > /tmp/t.s: Assembler messages: > /tmp/t.s:2: Error: immediate expression requires a # prefix -- `mov r1,13' > GNU assembler (Linaro_Binutils-2017.05) 2.25.2 Linaro 2016_02 > /tmp/t.s: Assembler messages: > /tmp/t.s:2: Error: immediate expression requires a # prefix -- `mov r1,13' > GNU assembler (crosstool-NG linaro-1.13.1-4.9-2014.09 - Linaro GCC 4.9-2014.09) 2.24.0.20140829 Linaro 2014.09 > /tmp/t.s: Assembler messages: > /tmp/t.s:2: Error: immediate expression requires a # prefix -- `mov r1,13' > GNU assembler (GNU Binutils) Linaro 2014.11-3-git 2.24.0.20141017 > /tmp/t.s: Assembler messages: > /tmp/t.s:2: Error: immediate expression requires a # prefix -- `mov r1,13' > GNU assembler (Linaro_Binutils-2017.08) 2.28.2.20170706 > /tmp/t.s: Assembler messages: > /tmp/t.s:2: Error: immediate expression requires a # prefix -- `mov r1,13' > > They all fail, including the version that looks like the one you have. Note that for UAL syntax, GAS follows the ARM ARM recommendation and considers the '#' on immediate values optional everywhere. I'm going to hazard a guess that Chris might be building a Thumb-2 kernel or somehow otherwise has ARM_ASM_UNIFIED turned on. Robin. > Could you try that little test above on your side? > >>> So, as far as ARM assembly in the Linux kernel goes, all constants must >>> be preceded by # whether or not binutils requires it - no exceptions. >>> Please always test assembly changes with a binutils version that is not >>> gratuitously broken! >> >> Somewhat ironic since Nicolas works for Linaro. > > I'm not involved with the toolchain people though, other than using > their output. > > > Nicolas >
On Thu, Nov 02, 2017 at 03:18:11PM +0000, Robin Murphy wrote: > Note that for UAL syntax, GAS follows the ARM ARM recommendation and > considers the '#' on immediate values optional everywhere. I'm going to > hazard a guess that Chris might be building a Thumb-2 kernel or somehow > otherwise has ARM_ASM_UNIFIED turned on. I hope people can see what a silly idea that is from the amount of problems this is causing.
On Thursday, November 02, 2017 1, Nicolas Pitre wrote: > > > So, as far as ARM assembly in the Linux kernel goes, all constants > must > > > be preceded by # whether or not binutils requires it - no exceptions. > > > Please always test assembly changes with a binutils version that is > not > > > gratuitously broken! > > > > Somewhat ironic since Nicolas works for Linaro. > > I'm not involved with the toolchain people though, other than using > their output. That was the irony! As in... Even if you built the code, you would have probably used a Linaro toolchain and it would have worked like on my system. Forget it. (mailing lists are so dry when it comes to humor) > They all fail, including the version that looks like the one you have. > > Could you try that little test above on your side? Yes, I also get a failure: arm-linux-gnueabihf-as /tmp/t.s -o /tmp/t.o /tmp/t.s: Assembler messages: /tmp/t.s:2: Error: immediate expression requires a # prefix -- `mov r1,13' But I think the answer is not that simple. You have no command line options. Chris
On Thursday, November 02, 2017 1, Robin Murphy wrote: > > > > They all fail, including the version that looks like the one you have. > > Note that for UAL syntax, GAS follows the ARM ARM recommendation and > considers the '#' on immediate values optional everywhere. I'm going to > hazard a guess that Chris might be building a Thumb-2 kernel or somehow > otherwise has ARM_ASM_UNIFIED turned on. Correct. $ grep THUMB2 .config CONFIG_THUMB2_KERNEL=y $ grep UNIFIED .config CONFIG_ARM_ASM_UNIFIED=y
On Thu, 2 Nov 2017, Chris Brandt wrote: > On Thursday, November 02, 2017 1, Nicolas Pitre wrote: > > > > So, as far as ARM assembly in the Linux kernel goes, all constants > > must > > > > be preceded by # whether or not binutils requires it - no exceptions. > > > > Please always test assembly changes with a binutils version that is > > not > > > > gratuitously broken! > > > > > > Somewhat ironic since Nicolas works for Linaro. > > > > I'm not involved with the toolchain people though, other than using > > their output. > > That was the irony! > > As in... > Even if you built the code, you would have probably used a Linaro > toolchain and it would have worked like on my system. Thing is... I *did* test it after I figured out I needed to turn off semihosting support. And the build failed. > Forget it. (mailing lists are so dry when it comes to humor) Life is tough. > > They all fail, including the version that looks like the one you have. > > > > Could you try that little test above on your side? > > Yes, I also get a failure: > > arm-linux-gnueabihf-as /tmp/t.s -o /tmp/t.o > /tmp/t.s: Assembler messages: > /tmp/t.s:2: Error: immediate expression requires a # prefix -- `mov r1,13' > > But I think the answer is not that simple. > You have no command line options. Would be good to figure out what option makes it accept no # and see if that can be avoided for kernel build. Nicolas
On Thu, Nov 02, 2017 at 03:23:14PM +0000, Chris Brandt wrote: > On Thursday, November 02, 2017 1, Nicolas Pitre wrote: > > > > So, as far as ARM assembly in the Linux kernel goes, all constants > > must > > > > be preceded by # whether or not binutils requires it - no exceptions. > > > > Please always test assembly changes with a binutils version that is > > not > > > > gratuitously broken! > > > > > > Somewhat ironic since Nicolas works for Linaro. > > > > I'm not involved with the toolchain people though, other than using > > their output. > > That was the irony! > > As in... > Even if you built the code, you would have probably used a Linaro > toolchain and it would have worked like on my system. > > Forget it. (mailing lists are so dry when it comes to humor) It's not mailing lists, it's email. Email lacks the facial expressions, and the voice inflections and tone necessary to convey this extra "metadata". It's a well known problem. Email is dry and devoid of the subtle hints that humans need to effectively communicate. Communication is *not* just about the words on a page. It's why we have smilies and emojis, as an attempt to fill that void, but I bet most of us (me included) don't use them enough. http://www.youmeworks.com/no_honking.html The 44% figure in that URL, if correct, is shocking. It probably means that approaching half of all emails on this list are misconstrued by the reader(s) of them!
On Thu, 2 Nov 2017, Nicolas Pitre wrote: > Would be good to figure out what option makes it accept no # and see if > that can be avoided for kernel build. OK.. I wanted to get to the bottom of this. The gas documentation says: |9.4.2.1 Instruction Set Syntax |.............................. | |Two slightly different syntaxes are supported for ARM and THUMB |instructions. The default, `divided', uses the old style where ARM and |THUMB instructions had their own, separate syntaxes. The new, |`unified' syntax, which can be selected via the `.syntax' directive, |and has the following main features: | | * Immediate operands do not require a `#' prefix. | | * The `IT' instruction may appear, and if it does it is validated | against subsequent conditional affixes. In ARM mode it does not | generate machine code, in THUMB mode it does. | | * For ARM instructions the conditional affixes always appear at the | end of the instruction. For THUMB instructions conditional | affixes can be used, but only inside the scope of an `IT' | instruction. | | * All of the instructions new to the V6T2 architecture (and later) | are available. (Only a few such instructions can be written in the | `divided' syntax). | | * The `.N' and `.W' suffixes are recognized and honored. | | * All instructions set the flags if and only if they have an `s' | affix. So this is a mixed bag of features and, unless I'm missing something, there is no way to get some and not the others. And we do need most of those features for Thumb2 kernel build. Nicolas
On Thursday, November 02, 2017 1, Russell King - ARM Linux wrote: > > Forget it. (mailing lists are so dry when it comes to humor) > > It's not mailing lists, it's email. Email lacks the facial expressions, > and the voice inflections and tone necessary to convey this extra > "metadata". It's a well known problem. Email is dry and devoid of the > subtle hints that humans need to effectively communicate. Communication > is *not* just about the words on a page. Ya I know (insert Frowning Face U+2639 here) > It's why we have smilies and emojis, as an attempt to fill that void, but > I bet most of us (me included) don't use them enough. I bet people would take their patches being rejected much better if you just sent back a Pile of Poo emoji (U+1F4A9) instead. (yes, said in a sarcastic voice) > http://www.youmeworks.com/no_honking.html * Don't use e-mail for emotional or sensitive topics. Pick up the phone or visit in person. I look forward to seeing you at the next ELC! (yes, sarcasm again....) Cheers Chris
On Thursday, November 02, 2017 1, Nicolas Pitre wrote: > OK.. I wanted to get to the bottom of this. The gas documentation says: > > |9.4.2.1 Instruction Set Syntax > |.............................. > | > |Two slightly different syntaxes are supported for ARM and THUMB > |instructions. The default, `divided', uses the old style where ARM and > |THUMB instructions had their own, separate syntaxes. The new, > |`unified' syntax, which can be selected via the `.syntax' directive, > |and has the following main features: > | > | * Immediate operands do not require a `#' prefix. Well there you go! We all have "gratuitously broken binutils" ;) Chris
On Thu, Nov 02, 2017 at 04:38:11PM +0000, Chris Brandt wrote: > On Thursday, November 02, 2017 1, Nicolas Pitre wrote: > > OK.. I wanted to get to the bottom of this. The gas documentation says: > > > > |9.4.2.1 Instruction Set Syntax > > |.............................. > > | > > |Two slightly different syntaxes are supported for ARM and THUMB > > |instructions. The default, `divided', uses the old style where ARM and > > |THUMB instructions had their own, separate syntaxes. The new, > > |`unified' syntax, which can be selected via the `.syntax' directive, > > |and has the following main features: > > | > > | * Immediate operands do not require a `#' prefix. > > Well there you go! > > We all have "gratuitously broken binutils" > > ;) Yes! What reason could there be to drop the well established norm of prefixing constants with "#" in ARM assembly, other than maybe political pressure? As I've already pointed out, we can see that this causes problems, and what it means is that people now must test their changes with Thumb2 support disabled in the kernel for there to be any valid testing of assembly. That basically means I can't trust anyone elses testing of patches that contain assembly, because I don't know what configuration they've tested. This is very bad, and it's going to make it slower to get such patches into the kernel. That's an unintended side effect of what was probably thought to be a trivial decision by the ARM ISA team, but it unfortunately has wider effects than they could have imagined. There is another solution to this: I augment the patch system with an ARM assembly parser that detects this before it gets accepted, rejecting patches that omit the # for constants. However, that is incomplete, because we now live in a world where ARM assembly gets added to the kernel via many different git trees. Basically, this change in the ARM syntax should never have been made.
On Thu, 2 Nov 2017, Russell King - ARM Linux wrote: > On Thu, Nov 02, 2017 at 04:38:11PM +0000, Chris Brandt wrote: > > On Thursday, November 02, 2017 1, Nicolas Pitre wrote: > > > OK.. I wanted to get to the bottom of this. The gas documentation says: > > > > > > |9.4.2.1 Instruction Set Syntax > > > |.............................. > > > | > > > |Two slightly different syntaxes are supported for ARM and THUMB > > > |instructions. The default, `divided', uses the old style where ARM and > > > |THUMB instructions had their own, separate syntaxes. The new, > > > |`unified' syntax, which can be selected via the `.syntax' directive, > > > |and has the following main features: > > > | > > > | * Immediate operands do not require a `#' prefix. > > > > Well there you go! > > > > We all have "gratuitously broken binutils" > > > > ;) > > Yes! > > What reason could there be to drop the well established norm of > prefixing constants with "#" in ARM assembly, other than maybe > political pressure? Dunno. But I wouldn't mind it at all if such a "feature" was selectable *separately* from the others, or if it could be opted out from the unified syntax. As it is I don't see how to achieve that. > There is another solution to this: I augment the patch system with an > ARM assembly parser that detects this before it gets accepted, > rejecting patches that omit the # for constants. However, that is > incomplete, because we now live in a world where ARM assembly gets > added to the kernel via many different git trees. I'd go for the ".syntax require_pound_literals" addition to the assembler. That will make the solution available to everyone eventually. Nicolas
On Thursday, November 02, 2017, Russell King - ARM Linux wrote: > This is very bad, and it's going to make it slower to get such > patches into the kernel. Well crap! All I wanted was my carriage returns back. This is going to come back and bite me in the ass later when I try to submit some XIP code changes in head.S that I've been meaning to get to. Chris
On 02/11/17 17:10, Russell King - ARM Linux wrote: > On Thu, Nov 02, 2017 at 04:38:11PM +0000, Chris Brandt wrote: >> On Thursday, November 02, 2017 1, Nicolas Pitre wrote: >>> OK.. I wanted to get to the bottom of this. The gas documentation says: >>> >>> |9.4.2.1 Instruction Set Syntax >>> |.............................. >>> | >>> |Two slightly different syntaxes are supported for ARM and THUMB >>> |instructions. The default, `divided', uses the old style where ARM and >>> |THUMB instructions had their own, separate syntaxes. The new, >>> |`unified' syntax, which can be selected via the `.syntax' directive, >>> |and has the following main features: >>> | >>> | * Immediate operands do not require a `#' prefix. >> >> Well there you go! >> >> We all have "gratuitously broken binutils" >> >> ;) > > Yes! > > What reason could there be to drop the well established norm of > prefixing constants with "#" in ARM assembly, other than maybe > political pressure? > > As I've already pointed out, we can see that this causes problems, > and what it means is that people now must test their changes with > Thumb2 support disabled in the kernel for there to be any valid > testing of assembly. That basically means I can't trust anyone > elses testing of patches that contain assembly, because I don't > know what configuration they've tested. But that's already been true since the introduction of THUMB2_KERNEL - there are instructions which exist in ARM but not in Thumb, and vice versa; there are constants which can be encoded in Thumb, but not in ARM; in general this syntactic difference doesn't really add anything other than being perhaps slightly easier to fall foul of. > This is very bad, and it's going to make it slower to get such > patches into the kernel. > > That's an unintended side effect of what was probably thought to be > a trivial decision by the ARM ISA team, but it unfortunately has > wider effects than they could have imagined. > > There is another solution to this: I augment the patch system with an > ARM assembly parser that detects this before it gets accepted, > rejecting patches that omit the # for constants. However, that is > incomplete, because we now live in a world where ARM assembly gets > added to the kernel via many different git trees. Or we could just enable unified syntax by default. AFAICT, binutils has supported UAL for over 12 years now, and the minimal supported version of 2.20 quoted in Documentation/process/ is considerably more recent than that. > Basically, this change in the ARM syntax should never have been made. Careful what you wish for - if GAS should be strict about unified syntax and not take an allowable implementation option, it should definitely be strict about legacy syntax and not accept UAL mnemonics which don't even exist in the old language. Then we'd have much bigger problems for antique toolchains ;) Robin.
diff --git a/arch/arm/kernel/debug.S b/arch/arm/kernel/debug.S index ea9646cc2a..0d1cef4b6e 100644 --- a/arch/arm/kernel/debug.S +++ b/arch/arm/kernel/debug.S @@ -79,25 +79,28 @@ hexbuf: .space 16 ENTRY(printascii) addruart_current r3, r1, r2 - b 2f -1: waituart r2, r3 - senduart r1, r3 - busyuart r2, r3 - teq r1, #'\n' - moveq r1, #'\r' - beq 1b -2: teq r0, #0 +1: teq r0, #0 ldrneb r1, [r0], #1 teqne r1, #0 - bne 1b - ret lr + reteq lr + teq r1, #'\n' + bne 2f + mov r1, '\r' + waituart r2, r3 + senduart r1, r3 + busyuart r2, r3 + mov r1, '\n' +2: waituart r2, r3 + senduart r1, r3 + busyuart r2, r3 + b 1b ENDPROC(printascii) ENTRY(printch) addruart_current r3, r1, r2 mov r1, r0 mov r0, #0 - b 1b + b 2b ENDPROC(printch) #ifdef CONFIG_MMU