linux-next: frimware build failure

linux-next: frimware build failure

[Stephen Rothwell]

Hi David,

Yesterday's linux-next build (arm csb637_defconfig and others) failed
like this:

firmware/keyspan/usa19qi.fw.gen.S: Assembler messages:
firmware/keyspan/usa19qi.fw.gen.S:11: Error: junk at end of line, first unrecognized character is `,'

for the various firmware bits.  Turns out that '@' is a gas comment start
character on arm.

I have applied the following patch.  This contains some white space and
other changes as well - in particular the first .palign was not getting a
number.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

>From e99b5dc320af8c10af9a35a827176365db6648c9 Mon Sep 17 00:00:00 2001
From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Thu, 5 Jun 2008 17:17:56 +1000
Subject: [PATCH] firmware: build fixes 2

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 firmware/Makefile |   13 +++++++------
 1 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/firmware/Makefile b/firmware/Makefile
index 94bd5b7..b8df924 100644
--- a/firmware/Makefile
+++ b/firmware/Makefile
@@ -59,20 +59,21 @@ quiet_cmd_fwbin = MK_FW   $@
 				firmware/%.gen.S,%,$@))))";		     \
 		  ASM_WORD=$(if $(CONFIG_64BIT),.quad,.long);		     \
 		  ASM_ALIGN=$(if $(CONFIG_64BIT),3,2);			     \
+		  AT_SYM=$(if $(CONFIG_ARM),%,@);			     \
 		  echo "/* Generated by firmware/Makefile */"		> $@;\
 		  echo "    .section .rodata"				>>$@;\
-		  echo "    .p2align $(ASM_ALIGN)"			>>$@;\
+		  echo "    .p2align $${ASM_ALIGN}"			>>$@;\
 		  echo "_fw_$${FWSTR}_bin:"				>>$@;\
 		  echo "    .incbin \"$(2)\""				>>$@;\
 		  echo "_fw_end:"					>>$@;\
-		  echo "   .section .rodata.str,\"aMS\",@progbits,1"	>>$@;\
+		  echo "   .section .rodata.str,\"aMS\",$${AT_SYM}progbits,1"	>>$@;\
 		  echo "    .p2align $${ASM_ALIGN}"			>>$@;\
 		  echo "_fw_$${FWSTR}_name:"				>>$@;\
-		  echo "    .string \"$$FWNAME\""			>>$@;\
-		  echo "    .section .builtin_fw,\"a\",@progbits"	>>$@;\
+		  echo "    .string \"$${FWNAME}\""			>>$@;\
+		  echo "    .section .builtin_fw,\"a\",$${AT_SYM}progbits"	>>$@;\
 		  echo "    .p2align $${ASM_ALIGN}"			>>$@;\
-		  echo "    $${ASM_WORD} _fw_$${FWSTR}_name" 		>>$@;\
-		  echo "    $${ASM_WORD} _fw_$${FWSTR}_bin" 		>>$@;\
+		  echo "    $${ASM_WORD} _fw_$${FWSTR}_name"		>>$@;\
+		  echo "    $${ASM_WORD} _fw_$${FWSTR}_bin"		>>$@;\
 		  echo "    $${ASM_WORD} _fw_end - _fw_$${FWSTR}_bin"	>>$@;
 
 # One of these files will change, or come into existence, whenever
-- 
1.5.5.3

Re: linux-next: frimware build failure

[David Woodhouse]

On Thu, 2008-06-05 at 17:25 +1000, Stephen Rothwell wrote:
> Hi David,
> 
> Yesterday's linux-next build (arm csb637_defconfig and others) failed
> like this:
> 
> firmware/keyspan/usa19qi.fw.gen.S: Assembler messages:
> firmware/keyspan/usa19qi.fw.gen.S:11: Error: junk at end of line, first unrecognized character is `,'
> 
> for the various firmware bits.  Turns out that '@' is a gas comment start
> character on arm.
> 
> I have applied the following patch.  This contains some white space and
> other changes as well - in particular the first .palign was not getting a
> number.

Thanks for that; I'll rework the patch accordingly. Sorry for the
trouble in this area; I expected one or two issues getting this right in
assembly, but I still think it's better to be able to use .incbin rather
than doing it in C as I had it originally.

-- 
dwmw2

Re: linux-next: frimware build failure

[David Woodhouse]

On Thu, 2008-06-05 at 17:25 +1000, Stephen Rothwell wrote:
> Hi David,
> 
> Yesterday's linux-next build (arm csb637_defconfig and others) failed
> like this:
> 
> firmware/keyspan/usa19qi.fw.gen.S: Assembler messages:
> firmware/keyspan/usa19qi.fw.gen.S:11: Error: junk at end of line, first unrecognized character is `,'
> 
> for the various firmware bits.  Turns out that '@' is a gas comment start
> character on arm.
> 
> I have applied the following patch.  This contains some white space and
> other changes as well - in particular the first .palign was not getting a
> number.

I did it like this so I could keep it in 80 columns:

diff --git a/firmware/Makefile b/firmware/Makefile
index 94bd5b7..d60b4c4 100644
--- a/firmware/Makefile
+++ b/firmware/Makefile
@@ -59,20 +59,21 @@ quiet_cmd_fwbin = MK_FW   $@
 				firmware/%.gen.S,%,$@))))";		     \
 		  ASM_WORD=$(if $(CONFIG_64BIT),.quad,.long);		     \
 		  ASM_ALIGN=$(if $(CONFIG_64BIT),3,2);			     \
+		  PROGBITS=$(if $(CONFIG_ARM),%,@)progbits;		     \
 		  echo "/* Generated by firmware/Makefile */"		> $@;\
 		  echo "    .section .rodata"				>>$@;\
-		  echo "    .p2align $(ASM_ALIGN)"			>>$@;\
+		  echo "    .p2align $${ASM_ALIGN}"			>>$@;\
 		  echo "_fw_$${FWSTR}_bin:"				>>$@;\
 		  echo "    .incbin \"$(2)\""				>>$@;\
 		  echo "_fw_end:"					>>$@;\
-		  echo "   .section .rodata.str,\"aMS\",@progbits,1"	>>$@;\
+		  echo "   .section .rodata.str,\"aMS\",$${PROGBITS},1"	>>$@;\
 		  echo "    .p2align $${ASM_ALIGN}"			>>$@;\
 		  echo "_fw_$${FWSTR}_name:"				>>$@;\
 		  echo "    .string \"$$FWNAME\""			>>$@;\
-		  echo "    .section .builtin_fw,\"a\",@progbits"	>>$@;\
+		  echo "    .section .builtin_fw,\"a\",$${PROGBITS}"	>>$@;\
 		  echo "    .p2align $${ASM_ALIGN}"			>>$@;\
-		  echo "    $${ASM_WORD} _fw_$${FWSTR}_name" 		>>$@;\
-		  echo "    $${ASM_WORD} _fw_$${FWSTR}_bin" 		>>$@;\
+		  echo "    $${ASM_WORD} _fw_$${FWSTR}_name"		>>$@;\
+		  echo "    $${ASM_WORD} _fw_$${FWSTR}_bin"		>>$@;\
 		  echo "    $${ASM_WORD} _fw_end - _fw_$${FWSTR}_bin"	>>$@;
 
 # One of these files will change, or come into existence, whenever

-- 
dwmw2

Re: linux-next: frimware build failure

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 637 bytes --]

Hi David,

On Thu, 05 Jun 2008 08:48:10 +0100 David Woodhouse <dwmw2@infradead.org> wrote:
>
> I did it like this so I could keep it in 80 columns:

Looks just as good.

I am a little worried that this may be broken the same way on other
platforms ...  The gas manual said something like "On platforms where @
is the comment character (like ARM) another character is used instead.
For example, the ARM port uses the % character."

A grep through the kernel sources shows only @ except on arm, so it
should be ok.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: linux-next: frimware build failure

[David Woodhouse]

On Thu, 2008-06-05 at 17:59 +1000, Stephen Rothwell wrote:
> Hi David,
> 
> On Thu, 05 Jun 2008 08:48:10 +0100 David Woodhouse <dwmw2@infradead.org> wrote:
> >
> > I did it like this so I could keep it in 80 columns:
> 
> Looks just as good.
> 
> I am a little worried that this may be broken the same way on other
> platforms ...  The gas manual said something like "On platforms where @
> is the comment character (like ARM) another character is used instead.
> For example, the ARM port uses the % character."
> 
> A grep through the kernel sources shows only @ except on arm, so it
> should be ok.

I did ponder using 'gcc -S' on a simple template file and then
post-processing the output, but I figured there was just as much that
could go wrong with that one too. At least when we emit our own asm,
once we've finished the coverage testing on all architectures and fixed
it all, we can have a reasonable expectation that it'll stay working.
Post-processing the assembly output of GCC just scares me :)

-- 
dwmw2

Re: linux-next: frimware build failure

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 726 bytes --]

Hi David,

On Thu, 05 Jun 2008 09:03:42 +0100 David Woodhouse <dwmw2@infradead.org> wrote:
>
> I did ponder using 'gcc -S' on a simple template file and then
> post-processing the output, but I figured there was just as much that
> could go wrong with that one too. At least when we emit our own asm,
> once we've finished the coverage testing on all architectures and fixed
> it all, we can have a reasonable expectation that it'll stay working.
> Post-processing the assembly output of GCC just scares me :)

Yeah, we used to do that for an initialised structure in PowerPC
iSeries - never again.  :-)

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: linux-next: frimware build failure

[Segher Boessenkool]

> I am a little worried that this may be broken the same way on other
> platforms ...  The gas manual said something like "On platforms where @
> is the comment character (like ARM) another character is used instead.
> For example, the ARM port uses the % character."
>
> A grep through the kernel sources shows only @ except on arm, so it
> should be ok.

The GAS code shows the same: only ARM is affected currently.

Incidentally, all GAS targets accept both "@" and "%" as the
character before progbits etc. -- but please don't use "%"
universally, since it could be a comment character on some archs
as well [1] (or break in some other way).


Segher

[1] The mmix gas port uses % as comment char already, for example.

Re: linux-next: frimware build failure

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 567 bytes --]

Hi Segher,

On Sat, 7 Jun 2008 02:42:07 +0200 Segher Boessenkool <segher@kernel.crashing.org> wrote:
>
> The GAS code shows the same: only ARM is affected currently.
> 
> Incidentally, all GAS targets accept both "@" and "%" as the
> character before progbits etc. -- but please don't use "%"
> universally, since it could be a comment character on some archs
> as well [1] (or break in some other way).

Thanks for the confirmation and warning.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]