[PATCH 0/3] a few debug_ll fixes and improvements

[PATCH 0/3] a few debug_ll fixes and improvements

[Nicolas Pitre]

This includes a couple fixes and improvements for using the early console
with semihosting on a v7m processor.


 arch/arm/boot/compressed/debug.S |  4 ++++
 arch/arm/kernel/debug.S          | 22 +++++++++++++++++++---
 arch/arm/kernel/early_printk.c   | 16 ++++++++++------
 3 files changed, 33 insertions(+), 9 deletions(-)

[PATCH 1/3] ARM: debug.S: move hexbuf to a writable section

[Nicolas Pitre]

This was located in .text which is meant to be read-only. And in the XIP
case this shortcut simply doesn't work and may trigger a Flash controller
mode switch and crash the kernel. Move it to the .bss area.

Signed-off-by: Nicolas Pitre <nico@linaro.org>
---
 arch/arm/kernel/debug.S | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/arch/arm/kernel/debug.S b/arch/arm/kernel/debug.S
index ea9646cc2a..423f443258 100644
--- a/arch/arm/kernel/debug.S
+++ b/arch/arm/kernel/debug.S
@@ -55,7 +55,9 @@ ENDPROC(printhex4)
 
 ENTRY(printhex2)
 		mov	r1, #2
-printhex:	adr	r2, hexbuf
+printhex:	adr	r2, hexbuf_rel
+		ldr	r3, [r2]
+		add	r2, r2, r3
 		add	r3, r2, r1
 		mov	r1, #0
 		strb	r1, [r3]
@@ -71,7 +73,11 @@ printhex:	adr	r2, hexbuf
 		b	printascii
 ENDPROC(printhex2)
 
-hexbuf:		.space 16
+		.pushsection .bss
+hexbuf_addr:	.space 16
+		.popsection
+		.align
+hexbuf_rel:	.long	hexbuf_addr - .
 
 		.ltorg
 
@@ -120,7 +126,9 @@ ENTRY(printascii)
 ENDPROC(printascii)
 
 ENTRY(printch)
-		adr	r1, hexbuf
+		adr	r1, hexbuf_rel
+		ldr	r2, [r1]
+		add	r1, r1, r2
 		strb	r0, [r1]
 		mov	r0, #0x03		@ SYS_WRITEC
 	ARM(	svc	#0x123456	)
-- 
2.9.5

[PATCH 2/3] ARM: semihosting: use proper instruction on v7m processors

[Nicolas Pitre]

The svc instruction doesn't exist on v7m processors. Semihosting ops are
invoked with the bkpt instruction instead.

Signed-off-by: Nicolas Pitre <nico@linaro.org>
---
 arch/arm/boot/compressed/debug.S | 4 ++++
 arch/arm/kernel/debug.S          | 8 ++++++++
 2 files changed, 12 insertions(+)

diff --git a/arch/arm/boot/compressed/debug.S b/arch/arm/boot/compressed/debug.S
index 5392ee6333..8f6e37177d 100644
--- a/arch/arm/boot/compressed/debug.S
+++ b/arch/arm/boot/compressed/debug.S
@@ -23,7 +23,11 @@ ENTRY(putc)
 	strb	r0, [r1]
 	mov	r0, #0x03		@ SYS_WRITEC
    ARM(	svc	#0x123456	)
+#ifdef CONFIG_CPU_V7M
+ THUMB(	bkpt	#0xab		)
+#else
  THUMB(	svc	#0xab		)
+#endif
 	mov	pc, lr
 	.align	2
 1:	.word	_GLOBAL_OFFSET_TABLE_ - .
diff --git a/arch/arm/kernel/debug.S b/arch/arm/kernel/debug.S
index 423f443258..35145b618c 100644
--- a/arch/arm/kernel/debug.S
+++ b/arch/arm/kernel/debug.S
@@ -121,7 +121,11 @@ ENTRY(printascii)
 		mov	r1, r0
 		mov	r0, #0x04		@ SYS_WRITE0
 	ARM(	svc	#0x123456	)
+#ifdef CONFIG_CPU_V7M
+	THUMB(	bkpt	#0xab		)
+#else
 	THUMB(	svc	#0xab		)
+#endif
 		ret	lr
 ENDPROC(printascii)
 
@@ -132,7 +136,11 @@ ENTRY(printch)
 		strb	r0, [r1]
 		mov	r0, #0x03		@ SYS_WRITEC
 	ARM(	svc	#0x123456	)
+#ifdef CONFIG_CPU_V7M
+	THUMB(	bkpt	#0xab		)
+#else
 	THUMB(	svc	#0xab		)
+#endif
 		ret	lr
 ENDPROC(printch)
 
-- 
2.9.5

[PATCH 3/3] ARM: early_printk: use printascii() rather than printch()

[Nicolas Pitre]

With printch() the console messages are sent out one character at a time
which is agonizingly slow especially with semihosting as the whole trap
intercept, remote byte access, and system resume danse is performed for
every single character across a relatively slow remote debug connection.
Let's use printascii() to send a whole string at once. This is also going
to be more efficient, albeit to a quite lesser extent, with serial ports
as well.

Signed-off-by: Nicolas Pitre <nico@linaro.org>
---
 arch/arm/kernel/early_printk.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/arch/arm/kernel/early_printk.c b/arch/arm/kernel/early_printk.c
index 4307653696..9257736ec9 100644
--- a/arch/arm/kernel/early_printk.c
+++ b/arch/arm/kernel/early_printk.c
@@ -11,16 +11,20 @@
 #include <linux/kernel.h>
 #include <linux/console.h>
 #include <linux/init.h>
+#include <linux/string.h>
 
-extern void printch(int);
+extern void printascii(const char *);
 
 static void early_write(const char *s, unsigned n)
 {
-	while (n-- > 0) {
-		if (*s == '\n')
-			printch('\r');
-		printch(*s);
-		s++;
+	char buf[128];
+	while (n) {
+		unsigned l = min(n, sizeof(buf)-1);
+		memcpy(buf, s, l);
+		buf[l] = 0;
+		s += l;
+		n -= l;
+		printascii(buf);
 	}
 }
 
-- 
2.9.5

[PATCH 3/3] ARM: early_printk: use printascii() rather than printch()

[Uwe Kleine-König]

Hello Nicolas,

On Sun, Oct 01, 2017 at 10:06:18PM -0400, Nicolas Pitre wrote:
> With printch() the console messages are sent out one character at a time
> which is agonizingly slow especially with semihosting as the whole trap
> intercept, remote byte access, and system resume danse is performed for

s/danse/dance/ ?

> every single character across a relatively slow remote debug connection.
> Let's use printascii() to send a whole string at once. This is also going
> to be more efficient, albeit to a quite lesser extent, with serial ports
> as well.
> 
> Signed-off-by: Nicolas Pitre <nico@linaro.org>
> ---
>  arch/arm/kernel/early_printk.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm/kernel/early_printk.c b/arch/arm/kernel/early_printk.c
> index 4307653696..9257736ec9 100644
> --- a/arch/arm/kernel/early_printk.c
> +++ b/arch/arm/kernel/early_printk.c
> @@ -11,16 +11,20 @@
>  #include <linux/kernel.h>
>  #include <linux/console.h>
>  #include <linux/init.h>
> +#include <linux/string.h>
>  
> -extern void printch(int);
> +extern void printascii(const char *);
>  
>  static void early_write(const char *s, unsigned n)
>  {
> -	while (n-- > 0) {
> -		if (*s == '\n')
> -			printch('\r');
> -		printch(*s);
> -		s++;
> +	char buf[128];
> +	while (n) {
> +		unsigned l = min(n, sizeof(buf)-1);
> +		memcpy(buf, s, l);
> +		buf[l] = 0;
> +		s += l;
> +		n -= l;
> +		printascii(buf);

I wonder if it is a usual case that n < 128 and s[n] == '\0'. If so this
could be tested for and then the memcpy could be dropped.
(The downside of course is that s[n] might also be something completely
different?)

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[PATCH 3/3] ARM: early_printk: use printascii() rather than printch()

[Nicolas Pitre]

On Mon, 2 Oct 2017, Uwe Kleine-K?nig wrote:

> Hello Nicolas,
> 
> On Sun, Oct 01, 2017 at 10:06:18PM -0400, Nicolas Pitre wrote:
> > With printch() the console messages are sent out one character at a time
> > which is agonizingly slow especially with semihosting as the whole trap
> > intercept, remote byte access, and system resume danse is performed for
> 
> s/danse/dance/ ?
> 
> > every single character across a relatively slow remote debug connection.
> > Let's use printascii() to send a whole string at once. This is also going
> > to be more efficient, albeit to a quite lesser extent, with serial ports
> > as well.
> > 
> > Signed-off-by: Nicolas Pitre <nico@linaro.org>
> > ---
> >  arch/arm/kernel/early_printk.c | 16 ++++++++++------
> >  1 file changed, 10 insertions(+), 6 deletions(-)
> > 
> > diff --git a/arch/arm/kernel/early_printk.c b/arch/arm/kernel/early_printk.c
> > index 4307653696..9257736ec9 100644
> > --- a/arch/arm/kernel/early_printk.c
> > +++ b/arch/arm/kernel/early_printk.c
> > @@ -11,16 +11,20 @@
> >  #include <linux/kernel.h>
> >  #include <linux/console.h>
> >  #include <linux/init.h>
> > +#include <linux/string.h>
> >  
> > -extern void printch(int);
> > +extern void printascii(const char *);
> >  
> >  static void early_write(const char *s, unsigned n)
> >  {
> > -	while (n-- > 0) {
> > -		if (*s == '\n')
> > -			printch('\r');
> > -		printch(*s);
> > -		s++;
> > +	char buf[128];
> > +	while (n) {
> > +		unsigned l = min(n, sizeof(buf)-1);
> > +		memcpy(buf, s, l);
> > +		buf[l] = 0;
> > +		s += l;
> > +		n -= l;
> > +		printascii(buf);
> 
> I wonder if it is a usual case that n < 128 and s[n] == '\0'. If so this
> could be tested for and then the memcpy could be dropped.

I did try it. Unfortunately the passed string is not null terminated.


Nicolas

[PATCH 3/3] ARM: early_printk: use printascii() rather than printch()

[Chris Brandt]

On Sunday, October 01, 2017 1, Nicolas Pitre wrote:
> With printch() the console messages are sent out one character at a time
> which is agonizingly slow especially with semihosting as the whole trap
> intercept, remote byte access, and system resume danse is performed for
> every single character across a relatively slow remote debug connection.
> Let's use printascii() to send a whole string at once. This is also going
> to be more efficient, albeit to a quite lesser extent, with serial ports
> as well.
> 
> Signed-off-by: Nicolas Pitre <nico@linaro.org>
> ---
>  arch/arm/kernel/early_printk.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)

Now that this patch has hit -next, I'm noticing an issue with it.

There are no carriage returns, just line feeds, which makes for a very 
ugly display.


For example:

Booting Linux...
[    0.000000] Booting Linux on physical CPU 0x0
                                                [    0.000000] Linux version 4.14.0-rc5-00014-g5689707d65e6 (chris at ubuntu) (gcc version 5.4.1 20161213 (Linaro GCC 5.4-2017.01)) #735 Mon Oct 30 12:59:37 EST 2017
                                                                                          [    0.000000] CPU: ARMv7 Processor [413fc090] revision 0 (ARMv7), cr=58c53c7d
                                                [    0.000000] CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing instruction cache
               [    0.000000] OF: fdt: Machine model: RSKRZA1
                                                             [    0.000000] debug: ignoring loglevel setting.
                                                                                                             [    0.000000] bootconsole [earlycon0] enabled
                                   [    0.000000] Memory policy: Data cache writeback
                                                                                     [    0.000000] On node 0 totalpages: 8192
      [    0.000000] free_area_init_node: node 0, pgdat c003dfb4, node_mem_map c1fb8000
                                                                                       [    0.000000]   Normal zone: 64 pages used for memmap
                     [    0.000000]   Normal zone: 0 pages reserved
                                                                   [    0.000000]   Normal zone: 8192 pages, LIFO batch:0
 [    0.000000] CPU: All CPU(s) started in SVC mode.




Of course as soon as the normal console driver is loaded later it boot, 
everything looks fine again. But I use early_printk a lot, so I wonder 
if I could get my carriage returns back.

Chris

[PATCH 3/3] ARM: early_printk: use printascii() rather than printch()

[Nicolas Pitre]

On Tue, 31 Oct 2017, Chris Brandt wrote:

> On Sunday, October 01, 2017 1, Nicolas Pitre wrote:
> > With printch() the console messages are sent out one character at a time
> > which is agonizingly slow especially with semihosting as the whole trap
> > intercept, remote byte access, and system resume danse is performed for
> > every single character across a relatively slow remote debug connection.
> > Let's use printascii() to send a whole string at once. This is also going
> > to be more efficient, albeit to a quite lesser extent, with serial ports
> > as well.
> > 
> > Signed-off-by: Nicolas Pitre <nico@linaro.org>
> > ---
> >  arch/arm/kernel/early_printk.c | 16 ++++++++++------
> >  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> Now that this patch has hit -next, I'm noticing an issue with it.
> 
> There are no carriage returns, just line feeds, which makes for a very 
> ugly display.

Hmmm....

If you look at printascii in arch/arm/kernel/debug.S you'll find the 
following code:

1:              waituart r2, r3
                senduart r1, r3
                busyuart r2, r3
                teq     r1, #'\n'
                moveq   r1, #'\r'
                beq     1b

Why is that not working for you?

Are you using semihosting?

Do you have another printascii implementation?


Nicolas

[PATCH 3/3] ARM: early_printk: use printascii() rather than printch()

[Robin Murphy]

On 31/10/17 16:22, Nicolas Pitre wrote:
> On Tue, 31 Oct 2017, Chris Brandt wrote:
> 
>> On Sunday, October 01, 2017 1, Nicolas Pitre wrote:
>>> With printch() the console messages are sent out one character at a time
>>> which is agonizingly slow especially with semihosting as the whole trap
>>> intercept, remote byte access, and system resume danse is performed for
>>> every single character across a relatively slow remote debug connection.
>>> Let's use printascii() to send a whole string at once. This is also going
>>> to be more efficient, albeit to a quite lesser extent, with serial ports
>>> as well.
>>>
>>> Signed-off-by: Nicolas Pitre <nico@linaro.org>
>>> ---
>>>  arch/arm/kernel/early_printk.c | 16 ++++++++++------
>>>  1 file changed, 10 insertions(+), 6 deletions(-)
>>
>> Now that this patch has hit -next, I'm noticing an issue with it.
>>
>> There are no carriage returns, just line feeds, which makes for a very 
>> ugly display.
> 
> Hmmm....
> 
> If you look at printascii in arch/arm/kernel/debug.S you'll find the 
> following code:
> 
> 1:              waituart r2, r3
>                 senduart r1, r3
>                 busyuart r2, r3
>                 teq     r1, #'\n'
>                 moveq   r1, #'\r'
>                 beq     1b
> 
> Why is that not working for you?

By inspection, the removed early_write() code inserted the '\r' before
the '\n' in the usual fashion; the printascii() code above ends up doing
the reverse, and I can imagine the atypical "\n\r" sequence probably
confuses some terminals trying to be clever with line ending detection.

Robin.

> 
> Are you using semihosting?
> 
> Do you have another printascii implementation?
> 
> 
> Nicolas
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

[PATCH 3/3] ARM: early_printk: use printascii() rather than printch()

[Nicolas Pitre]

On Tue, 31 Oct 2017, Robin Murphy wrote:

> On 31/10/17 16:22, Nicolas Pitre wrote:
> > On Tue, 31 Oct 2017, Chris Brandt wrote:
> > 
> >> On Sunday, October 01, 2017 1, Nicolas Pitre wrote:
> >>> With printch() the console messages are sent out one character at a time
> >>> which is agonizingly slow especially with semihosting as the whole trap
> >>> intercept, remote byte access, and system resume danse is performed for
> >>> every single character across a relatively slow remote debug connection.
> >>> Let's use printascii() to send a whole string at once. This is also going
> >>> to be more efficient, albeit to a quite lesser extent, with serial ports
> >>> as well.
> >>>
> >>> Signed-off-by: Nicolas Pitre <nico@linaro.org>
> >>> ---
> >>>  arch/arm/kernel/early_printk.c | 16 ++++++++++------
> >>>  1 file changed, 10 insertions(+), 6 deletions(-)
> >>
> >> Now that this patch has hit -next, I'm noticing an issue with it.
> >>
> >> There are no carriage returns, just line feeds, which makes for a very 
> >> ugly display.
> > 
> > Hmmm....
> > 
> > If you look at printascii in arch/arm/kernel/debug.S you'll find the 
> > following code:
> > 
> > 1:              waituart r2, r3
> >                 senduart r1, r3
> >                 busyuart r2, r3
> >                 teq     r1, #'\n'
> >                 moveq   r1, #'\r'
> >                 beq     1b
> > 
> > Why is that not working for you?
> 
> By inspection, the removed early_write() code inserted the '\r' before
> the '\n' in the usual fashion; the printascii() code above ends up doing
> the reverse, and I can imagine the atypical "\n\r" sequence probably
> confuses some terminals trying to be clever with line ending detection.

That's easy to veryfy with this patch:

diff --git a/arch/arm/kernel/debug.S b/arch/arm/kernel/debug.S
index ea9646cc2a..40023a4871 100644
--- a/arch/arm/kernel/debug.S
+++ b/arch/arm/kernel/debug.S
@@ -79,18 +79,21 @@ 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)

@Chris: does the above fix your display issue?


Nicolas

[PATCH 3/3] ARM: early_printk: use printascii() rather than printch()

[Russell King - ARM Linux]

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.

> diff --git a/arch/arm/kernel/debug.S b/arch/arm/kernel/debug.S
> index ea9646cc2a..40023a4871 100644
> --- a/arch/arm/kernel/debug.S
> +++ b/arch/arm/kernel/debug.S
> @@ -79,18 +79,21 @@ hexbuf:		.space 16
>  

The new code is:

>  ENTRY(printascii)
>  		addruart_current r3, r1, r2
> +1:		teq	r0, #0
>  		ldrneb	r1, [r0], #1
>  		teqne	r1, #0
> +		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)

and printch jumps to the 1: label with r0 = 0 and r1 = character.  Your
change has the effect that the "reteq" will always be taken when called
by printch.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

[PATCH 3/3] ARM: early_printk: use printascii() rather than printch()

[Nicolas Pitre]

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)
 
 #ifdef CONFIG_MMU




> 
> > diff --git a/arch/arm/kernel/debug.S b/arch/arm/kernel/debug.S
> > index ea9646cc2a..40023a4871 100644
> > --- a/arch/arm/kernel/debug.S
> > +++ b/arch/arm/kernel/debug.S
> > @@ -79,18 +79,21 @@ hexbuf:		.space 16
> >  
> 
> The new code is:
> 
> >  ENTRY(printascii)
> >  		addruart_current r3, r1, r2
> > +1:		teq	r0, #0
> >  		ldrneb	r1, [r0], #1
> >  		teqne	r1, #0
> > +		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)
> 
> and printch jumps to the 1: label with r0 = 0 and r1 = character.  Your
> change has the effect that the "reteq" will always be taken when called
> by printch.
> 
> -- 
> RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
> According to speedtest.net: 8.21Mbps down 510kbps up
> 

[PATCH 3/3] ARM: early_printk: use printascii() rather than printch()

[Russell King - ARM Linux]

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

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

[PATCH 3/3] ARM: early_printk: use printascii() rather than printch()

[Nicolas Pitre]

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

[PATCH 3/3] ARM: early_printk: use printascii() rather than printch()

[Russell King - ARM Linux]

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.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

[PATCH 3/3] ARM: early_printk: use printascii() rather than printch()

[Nicolas Pitre]

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

[PATCH 3/3] ARM: early_printk: use printascii() rather than printch()

[Chris Brandt]

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

[PATCH 3/3] ARM: early_printk: use printascii() rather than printch()

[Nicolas Pitre]

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

[PATCH 3/3] ARM: early_printk: use printascii() rather than printch()

[Russell King - ARM Linux]

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.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

[PATCH 3/3] ARM: early_printk: use printascii() rather than printch()

[Nicolas Pitre]

On Tue, 31 Oct 2017, Russell King - ARM Linux wrote:

> On Tue, Oct 31, 2017 at 03:28:11PM -0400, Nicolas Pitre wrote:
> > 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.

Let's make a separate patch for that one. Something like:

diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
index 8a756870c2..28a48ad3dd 100644
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -1296,19 +1296,24 @@ phex:		adr	r3, phexbuf
 
 @ puts corrupts {r0, r1, r2, r3}
 puts:		loadsp	r3, r1
-1:		ldrb	r2, [r0], #1
-		teq	r2, #0
+1:		teq	r0, #0
+		ldrneb	r2, [r0], #1
+		teqne	r2, #0
 		moveq	pc, lr
-2:		writeb	r2, r3
+2:		teq	r2, #'\n'
+		bne	4f
+		mov	r2, #'\r'
+		writeb	r2, r3
 		mov	r1, #0x00020000
 3:		subs	r1, r1, #1
 		bne	3b
-		teq	r2, #'\n'
-		moveq	r2, #'\r'
-		beq	2b
-		teq	r0, #0
-		bne	1b
-		mov	pc, lr
+		mov	r2, #'\n'
+4:		writeb	r2, r3
+		mov	r1, #0x00020000
+5:		subs	r1, r1, #1
+		bne	5b
+		b	1b
+
 @ putc corrupts {r0, r1, r2, r3}
 putc:
 		mov	r2, r0

Please review as this is untested.


Nicolas

[PATCH 3/3] ARM: early_printk: use printascii() rather than printch()

[Russell King - ARM Linux]

On Tue, Oct 31, 2017 at 07:35:56PM -0400, Nicolas Pitre wrote:
> On Tue, 31 Oct 2017, Russell King - ARM Linux wrote:
> 
> > On Tue, Oct 31, 2017 at 03:28:11PM -0400, Nicolas Pitre wrote:
> > > 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.
> 
> Let's make a separate patch for that one. Something like:
> 
> diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
> index 8a756870c2..28a48ad3dd 100644
> --- a/arch/arm/boot/compressed/head.S
> +++ b/arch/arm/boot/compressed/head.S
> @@ -1296,19 +1296,24 @@ phex:		adr	r3, phexbuf
>  
>  @ puts corrupts {r0, r1, r2, r3}
>  puts:		loadsp	r3, r1
> -1:		ldrb	r2, [r0], #1
> -		teq	r2, #0
> +1:		teq	r0, #0
> +		ldrneb	r2, [r0], #1
> +		teqne	r2, #0
>  		moveq	pc, lr
> -2:		writeb	r2, r3
> +2:		teq	r2, #'\n'
> +		bne	4f
> +		mov	r2, #'\r'
> +		writeb	r2, r3
>  		mov	r1, #0x00020000
>  3:		subs	r1, r1, #1
>  		bne	3b
> -		teq	r2, #'\n'
> -		moveq	r2, #'\r'
> -		beq	2b
> -		teq	r0, #0
> -		bne	1b
> -		mov	pc, lr
> +		mov	r2, #'\n'
> +4:		writeb	r2, r3
> +		mov	r1, #0x00020000
> +5:		subs	r1, r1, #1
> +		bne	5b
> +		b	1b
> +
>  @ putc corrupts {r0, r1, r2, r3}
>  putc:
>  		mov	r2, r0
> 
> Please review as this is untested.

Looks good from a quick read through, thanks.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

[PATCH 3/3] ARM: early_printk: use printascii() rather than printch()

[Russell King - ARM Linux]

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.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

[PATCH 3/3] ARM: early_printk: use printascii() rather than printch()

[Nicolas Pitre]

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

[PATCH 3/3] ARM: early_printk: use printascii() rather than printch()

[Nicolas Pitre]

On Wed, 1 Nov 2017, Nicolas Pitre wrote:

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

Here's the fixed patch. I won't submit it to the patch system before we 
understand how the previous one worked for Chris.

----- >8
Subject: debug printch/printascii: translate '\n' to "\r\n" not "\n\r"

Some terminals apparently have issues with "\n\r" and mess up the 
display. Let's use the traditional "\r\n" ordering.

Signed-off-by: Nicolas Pitre <nico@linaro.org>
Reported-by: Chris Brandt <Chris.Brandt@renesas.com>

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

[PATCH 3/3] ARM: early_printk: use printascii() rather than printch()

[Chris Brandt]

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@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

[PATCH 3/3] ARM: early_printk: use printascii() rather than printch()

[Russell King - ARM Linux]

On Thu, Nov 02, 2017 at 12:16:54AM -0400, Nicolas Pitre wrote:
> On Wed, 1 Nov 2017, Nicolas Pitre wrote:
> 
> > 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.
> 
> Here's the fixed patch. I won't submit it to the patch system before we 
> understand how the previous one worked for Chris.

As we have seen, there's risk involved with merging apparently tested
fixes, and it can result in major build breakage.

Today is the last linux-next fetch before the potential opening of the
merge window on Sunday.  I'd prefer not to merge anything in the next
four days, but instead let things settle ready for that.

Most of last night's builds for my autobuilder failed due to this
problem (I didn't have a chance to regenerate that tree after kernelci
reported the problem), so I'm going to have to wait until tonight's
builds to see how things are - and that's starting to leave precious
little available time before the merge window.  I don't run the
builder during the day as it eats around six hours of time.

So, in short, my tree is now closed in expectation of 4.14 on Sunday.

However, if you want me to remove "8705/1: early_printk: use printascii()
rather than printch()" I can do that, but only if you tell me before
4pm UTC today.

If we get 4.14-rc8 instead, I'll re-open for a few days for this.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

[PATCH 3/3] ARM: early_printk: use printascii() rather than printch()

[Russell King - ARM Linux]

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!

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

[PATCH 3/3] ARM: early_printk: use printascii() rather than printch()

[Chris Brandt]

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

[PATCH 3/3] ARM: early_printk: use printascii() rather than printch()

[Russell King - ARM Linux]

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.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

[PATCH 3/3] ARM: early_printk: use printascii() rather than printch()

[Nicolas Pitre]

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

[PATCH 3/3] ARM: early_printk: use printascii() rather than printch()

[Nicolas Pitre]

On Thu, 2 Nov 2017, Russell King - ARM Linux wrote:

> So, in short, my tree is now closed in expectation of 4.14 on Sunday.
> 
> However, if you want me to remove "8705/1: early_printk: use printascii()
> rather than printch()" I can do that, but only if you tell me before
> 4pm UTC today.

I'd say: just leave things as they are. No point disturbing your tree 
again now. The downside is only cosmetic during the early boot when 
early_printk is enabled. That can be fixed in v4.15-rc1.

> If we get 4.14-rc8 instead, I'll re-open for a few days for this.

Sure.


Nicolas

[PATCH 3/3] ARM: early_printk: use printascii() rather than printch()

[Robin Murphy]

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
> 

[PATCH 3/3] ARM: early_printk: use printascii() rather than printch()

[Russell King - ARM Linux]

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.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

[PATCH 3/3] ARM: early_printk: use printascii() rather than printch()

[Chris Brandt]

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

[PATCH 3/3] ARM: early_printk: use printascii() rather than printch()

[Chris Brandt]

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

[PATCH 3/3] ARM: early_printk: use printascii() rather than printch()

[Nicolas Pitre]

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

[PATCH 3/3] ARM: early_printk: use printascii() rather than printch()

[Russell King - ARM Linux]

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!

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

[PATCH 3/3] ARM: early_printk: use printascii() rather than printch()

[Nicolas Pitre]

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

[PATCH 3/3] ARM: early_printk: use printascii() rather than printch()

[Chris Brandt]

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

[PATCH 3/3] ARM: early_printk: use printascii() rather than printch()

[Chris Brandt]

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

[PATCH 3/3] ARM: early_printk: use printascii() rather than printch()

[Russell King - ARM Linux]

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.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

[PATCH 3/3] ARM: early_printk: use printascii() rather than printch()

[Nicolas Pitre]

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

[PATCH 3/3] ARM: early_printk: use printascii() rather than printch()

[Chris Brandt]

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

[PATCH 3/3] ARM: early_printk: use printascii() rather than printch()

[Robin Murphy]

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.

syntax unified, was Re: [PATCH 3/3] ARM: early_printk: use printascii() rather than printch()

[Nicolas Pitre]

On Thu, 2 Nov 2017, Robin Murphy wrote:
> On 02/11/17 17:10, Russell King - ARM Linux wrote:
> > 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.

You have a point here. So let's see in details...

Documentation/process/changes.rst sais:

|Upgrade to at **least** these software revisions before thinking you've
|encountered a bug!  If you're unsure what version you're currently
|running, the suggested command should tell you.
[...]
|GNU C                  3.2              gcc --version
|GNU make               3.81             make --version
|binutils               2.20             ld -v
[...]

On ARM we enforce these additional restrictions in 
arch/arm/kernel/asm-offsets.c:

|/*
| * GCC 3.0, 3.1: general bad code generation.
| * GCC 3.2.0: incorrect function argument offset calculation.
| * GCC 3.2.x: miscompiles NEW_AUX_ENT in fs/binfmt_elf.c
| *            (http://gcc.gnu.org/PR8896) and incorrect structure
| *            initialisation in fs/jffs2/erase.c
| * GCC 4.8.0-4.8.2: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58854
| *            miscompiles find_get_entry(), and can result in EXT3 and EXT4
| *            filesystem corruption (possibly other FS too).
| */
|#ifdef __GNUC__
|#if (__GNUC__ == 3 && __GNUC_MINOR__ < 3)
|#error Your compiler is too buggy; it is known to miscompile kernels.
|#error    Known good compilers: 3.3, 4.x
|#endif
[...]

It should be quite safe to assume that any gcc-3.3 installations came 
with@least binutils-2.20.

So let's try that out:

binutils-2.20.1]$ ./gas/as-new --version
GNU assembler (GNU Binutils) 2.20.1.20100303
[...]
binutils-2.20.1]$ echo "mov r0, '\r'" | ./gas/as-new - -o /tmp/t.o
{standard input}: Assembler messages:
{standard input}:1: Error: immediate expression requires a # prefix -- `mov r0,13'
binutils-2.20.1]$ echo ".syntax unified; mov r0, '\r'" | ./gas/as-new - -o /tmp/t.o
(nothing)
binutils-2.20.1]$ ./binutils/objdump  -d /tmp/t.o

/tmp/t.o:     file format elf32-littlearm

Disassembly of section .text:

00000000 <.text>:
   0:   e3a0000d        mov     r0, #13

Digging into the binutils git repository, it looks like the unified 
syntax was indeed supported in 2005.

Therefore every setup capable of compiling the latest linux kernel for 
ARM should be ".syntax unified" ready as Robin said.

So... I think the best thing to do at this point is to enable the 
unified syntax unconditionally as suggested. This even has the side 
effect of removing a bunch of macros that cause trouble with LTO builds.

I propose the following patch:

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 7888c9803e..de1dd6e9cf 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1531,12 +1531,10 @@ config THUMB2_KERNEL
 	bool "Compile the kernel in Thumb-2 mode" if !CPU_THUMBONLY
 	depends on (CPU_V7 || CPU_V7M) && !CPU_V6 && !CPU_V6K
 	default y if CPU_THUMBONLY
-	select ARM_ASM_UNIFIED
 	select ARM_UNWIND
 	help
 	  By enabling this option, the kernel will be compiled in
-	  Thumb-2 mode. A compiler/assembler that understand the unified
-	  ARM-Thumb syntax is needed.
+	  Thumb-2 mode.
 
 	  If unsure, say N.
 
@@ -1571,9 +1569,6 @@ config THUMB2_AVOID_R_ARM_THM_JUMP11
 
 	  Unless you are sure your tools don't have this problem, say Y.
 
-config ARM_ASM_UNIFIED
-	bool
-
 config ARM_PATCH_IDIV
 	bool "Runtime patch udiv/sdiv instructions into __aeabi_{u}idiv()"
 	depends on CPU_32v7 && !XIP_KERNEL
diff --git a/arch/arm/include/asm/unified.h b/arch/arm/include/asm/unified.h
index a91ae49961..2c3b952be6 100644
--- a/arch/arm/include/asm/unified.h
+++ b/arch/arm/include/asm/unified.h
@@ -20,8 +20,10 @@
 #ifndef __ASM_UNIFIED_H
 #define __ASM_UNIFIED_H
 
-#if defined(__ASSEMBLY__) && defined(CONFIG_ARM_ASM_UNIFIED)
+#if defined(__ASSEMBLY__)
 	.syntax unified
+#else
+__asm__(".syntax unified");
 #endif
 
 #ifdef CONFIG_CPU_V7M
@@ -64,77 +66,4 @@
 
 #endif	/* CONFIG_THUMB2_KERNEL */
 
-#ifndef CONFIG_ARM_ASM_UNIFIED
-
-/*
- * If the unified assembly syntax isn't used (in ARM mode), these
- * macros expand to an empty string
- */
-#ifdef __ASSEMBLY__
-	.macro	it, cond
-	.endm
-	.macro	itt, cond
-	.endm
-	.macro	ite, cond
-	.endm
-	.macro	ittt, cond
-	.endm
-	.macro	itte, cond
-	.endm
-	.macro	itet, cond
-	.endm
-	.macro	itee, cond
-	.endm
-	.macro	itttt, cond
-	.endm
-	.macro	ittte, cond
-	.endm
-	.macro	ittet, cond
-	.endm
-	.macro	ittee, cond
-	.endm
-	.macro	itett, cond
-	.endm
-	.macro	itete, cond
-	.endm
-	.macro	iteet, cond
-	.endm
-	.macro	iteee, cond
-	.endm
-#else	/* !__ASSEMBLY__ */
-__asm__(
-"	.macro	it, cond\n"
-"	.endm\n"
-"	.macro	itt, cond\n"
-"	.endm\n"
-"	.macro	ite, cond\n"
-"	.endm\n"
-"	.macro	ittt, cond\n"
-"	.endm\n"
-"	.macro	itte, cond\n"
-"	.endm\n"
-"	.macro	itet, cond\n"
-"	.endm\n"
-"	.macro	itee, cond\n"
-"	.endm\n"
-"	.macro	itttt, cond\n"
-"	.endm\n"
-"	.macro	ittte, cond\n"
-"	.endm\n"
-"	.macro	ittet, cond\n"
-"	.endm\n"
-"	.macro	ittee, cond\n"
-"	.endm\n"
-"	.macro	itett, cond\n"
-"	.endm\n"
-"	.macro	itete, cond\n"
-"	.endm\n"
-"	.macro	iteet, cond\n"
-"	.endm\n"
-"	.macro	iteee, cond\n"
-"	.endm\n");
-#endif	/* __ASSEMBLY__ */
-
-#endif	/* CONFIG_ARM_ASM_UNIFIED */
-
 #endif	/* !__ASM_UNIFIED_H */