Patchwork [RFC] ARM: compile fix for DEBUG_LL=y && MMU=n

login
register
mail settings
Submitter Uwe Kleine-König
Date Jan. 16, 2013, 2:32 p.m.
Message ID <1358346726-27199-1-git-send-email-u.kleine-koenig@pengutronix.de>
Download mbox | patch
Permalink /patch/212528/
State New
Headers show

Comments

Uwe Kleine-König - Jan. 16, 2013, 2:32 p.m.
debug_ll_addr is only used on machines with an MMU so it can be #ifdef'ed
out safely. This fixes:

arch/arm/kernel/debug.S: Assembler messages:
arch/arm/kernel/debug.S:104: Error: too many positional arguments

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Stephen Warren <swarren@nvidia.com>
Cc: Olof Johansson <olof@lixom.net>
---
The obvious alternative fix is to make addruart on !MMU take 3
arguments, too.

The problem was introduced in

	e5c5f2a (ARM: implement debug_ll_io_init())

which appeared in v3.8-rc1.
---
 arch/arm/kernel/debug.S |    2 ++
 1 file changed, 2 insertions(+)
Stephen Warren - Jan. 16, 2013, 5:43 p.m.
On 01/16/2013 07:32 AM, Uwe Kleine-König wrote:
> debug_ll_addr is only used on machines with an MMU so it can be #ifdef'ed
> out safely. This fixes:
> 
> arch/arm/kernel/debug.S: Assembler messages:
> arch/arm/kernel/debug.S:104: Error: too many positional arguments
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> Cc: Rob Herring <rob.herring@calxeda.com>
> Cc: Stephen Warren <swarren@nvidia.com>
> Cc: Olof Johansson <olof@lixom.net>
> ---
> The obvious alternative fix is to make addruart on !MMU take 3
> arguments, too.
> 
> The problem was introduced in
> 
> 	e5c5f2a (ARM: implement debug_ll_io_init())

It may be useful to mention that in the commit message.

Sorry for the breakage. Are there any ARM defconfigs for !MMU?

Reviewed-by: Stephen Warren <swarren@nvidia.com>

(although should the second copy of debug_ll_addr at the end of the file
be ifdef'd away too; the one for CONFIG_DEBUG_SEMIHOSTING)?
Rob Herring - Jan. 16, 2013, 7:17 p.m.
On 01/16/2013 08:32 AM, Uwe Kleine-König wrote:
> debug_ll_addr is only used on machines with an MMU so it can be #ifdef'ed
> out safely. This fixes:
> 
> arch/arm/kernel/debug.S: Assembler messages:
> arch/arm/kernel/debug.S:104: Error: too many positional arguments
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> Cc: Rob Herring <rob.herring@calxeda.com>
> Cc: Stephen Warren <swarren@nvidia.com>
> Cc: Olof Johansson <olof@lixom.net>
> ---
> The obvious alternative fix is to make addruart on !MMU take 3
> arguments, too.

We never envision a need to get the uart address from C code on !MMU?

Either way:

Acked-by: Rob Herring <rob.herring@calxeda.com>

> 
> The problem was introduced in
> 
> 	e5c5f2a (ARM: implement debug_ll_io_init())
> 
> which appeared in v3.8-rc1.
> ---
>  arch/arm/kernel/debug.S |    2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm/kernel/debug.S b/arch/arm/kernel/debug.S
> index 6809200..14f7c3b 100644
> --- a/arch/arm/kernel/debug.S
> +++ b/arch/arm/kernel/debug.S
> @@ -100,12 +100,14 @@ ENTRY(printch)
>  		b	1b
>  ENDPROC(printch)
>  
> +#ifdef CONFIG_MMU
>  ENTRY(debug_ll_addr)
>  		addruart r2, r3, ip
>  		str	r2, [r0]
>  		str	r3, [r1]
>  		mov	pc, lr
>  ENDPROC(debug_ll_addr)
> +#endif
>  
>  #else
>  
>
Uwe Kleine-König - Jan. 16, 2013, 7:26 p.m.
On Wed, Jan 16, 2013 at 10:43:32AM -0700, Stephen Warren wrote:
> On 01/16/2013 07:32 AM, Uwe Kleine-König wrote:
> > debug_ll_addr is only used on machines with an MMU so it can be #ifdef'ed
> > out safely. This fixes:
> > 
> > arch/arm/kernel/debug.S: Assembler messages:
> > arch/arm/kernel/debug.S:104: Error: too many positional arguments
> > 
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > Cc: Rob Herring <rob.herring@calxeda.com>
> > Cc: Stephen Warren <swarren@nvidia.com>
> > Cc: Olof Johansson <olof@lixom.net>
> > ---
> > The obvious alternative fix is to make addruart on !MMU take 3
> > arguments, too.
> > 
> > The problem was introduced in
> > 
> > 	e5c5f2a (ARM: implement debug_ll_io_init())
> 
> It may be useful to mention that in the commit message.
right.

> Sorry for the breakage. Are there any ARM defconfigs for !MMU?
I'm not aware of anything in mainline.
 
> Reviewed-by: Stephen Warren <swarren@nvidia.com>
thanks

> (although should the second copy of debug_ll_addr at the end of the file
> be ifdef'd away too; the one for CONFIG_DEBUG_SEMIHOSTING)?
In contrast to the !CONFIG_DEBUG_SEMIHOSTING implementation this one
doesn't call addruart and should compile just fine. I don't care much.

Best regards
Uwe
Olof Johansson - Jan. 16, 2013, 10:31 p.m.
On Wed, Jan 16, 2013 at 03:32:06PM +0100, Uwe Kleine-König wrote:
> debug_ll_addr is only used on machines with an MMU so it can be #ifdef'ed
> out safely. This fixes:
> 
> arch/arm/kernel/debug.S: Assembler messages:
> arch/arm/kernel/debug.S:104: Error: too many positional arguments
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> Cc: Rob Herring <rob.herring@calxeda.com>
> Cc: Stephen Warren <swarren@nvidia.com>
> Cc: Olof Johansson <olof@lixom.net>
> ---
> The obvious alternative fix is to make addruart on !MMU take 3
> arguments, too.
> 
> The problem was introduced in
> 
> 	e5c5f2a (ARM: implement debug_ll_io_init())
> 
> which appeared in v3.8-rc1.

Since we introduced the breakage in arm-soc, I've picked up the fix (I guess
it could have gone through Russell's tree as well though).

I've added the reference to the introducing commit, and the acks/review tags.

Adding a defconfig for a !MMU platform seems like a good idea, since there's
several of us who make sure that there is build coverage of the defconfigs and
follow up on most of the breakage. Feel free to send one in for 3.9.


-Olof
Uwe Kleine-König - Jan. 17, 2013, 8:12 a.m.
On Wed, Jan 16, 2013 at 01:17:20PM -0600, Rob Herring wrote:
> On 01/16/2013 08:32 AM, Uwe Kleine-König wrote:
> > debug_ll_addr is only used on machines with an MMU so it can be #ifdef'ed
> > out safely. This fixes:
> > 
> > arch/arm/kernel/debug.S: Assembler messages:
> > arch/arm/kernel/debug.S:104: Error: too many positional arguments
> > 
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > Cc: Rob Herring <rob.herring@calxeda.com>
> > Cc: Stephen Warren <swarren@nvidia.com>
> > Cc: Olof Johansson <olof@lixom.net>
> > ---
> > The obvious alternative fix is to make addruart on !MMU take 3
> > arguments, too.
> 
> We never envision a need to get the uart address from C code on !MMU?
On MMU addruart takes 3 arguments: physical address output, virtual
address output and a tmp. On !MMU physical == virtual, so only one
register for the address is needed.

(BTW, it was commit 639da5e (ARM: add an extra temp register to the low
level debugging addruart macro) that only changed the MMU variants of
addruart.)
 
Best regards
Uwe
Uwe Kleine-König - Jan. 17, 2013, 4:14 p.m.
On Wed, Jan 16, 2013 at 02:31:44PM -0800, Olof Johansson wrote:
> On Wed, Jan 16, 2013 at 03:32:06PM +0100, Uwe Kleine-König wrote:
> > debug_ll_addr is only used on machines with an MMU so it can be #ifdef'ed
> > out safely. This fixes:
> > 
> > arch/arm/kernel/debug.S: Assembler messages:
> > arch/arm/kernel/debug.S:104: Error: too many positional arguments
> > 
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > Cc: Rob Herring <rob.herring@calxeda.com>
> > Cc: Stephen Warren <swarren@nvidia.com>
> > Cc: Olof Johansson <olof@lixom.net>
> > ---
> > The obvious alternative fix is to make addruart on !MMU take 3
> > arguments, too.
> > 
> > The problem was introduced in
> > 
> > 	e5c5f2a (ARM: implement debug_ll_io_init())
> > 
> > which appeared in v3.8-rc1.
> 
> Since we introduced the breakage in arm-soc, I've picked up the fix (I guess
> it could have gone through Russell's tree as well though).
> 
> I've added the reference to the introducing commit, and the acks/review tags.
> 
> Adding a defconfig for a !MMU platform seems like a good idea, since there's
> several of us who make sure that there is build coverage of the defconfigs and
> follow up on most of the breakage. Feel free to send one in for 3.9.
Jonny just pointed out in irc that there is at91x40_defconfig.

Best regards
Uwe
Stephen Warren - Jan. 17, 2013, 10:09 p.m.
On 01/17/2013 09:14 AM, Uwe Kleine-König wrote:
> On Wed, Jan 16, 2013 at 02:31:44PM -0800, Olof Johansson wrote:
>> On Wed, Jan 16, 2013 at 03:32:06PM +0100, Uwe Kleine-König wrote:
>>> debug_ll_addr is only used on machines with an MMU so it can be #ifdef'ed
>>> out safely. This fixes:
...
>> Adding a defconfig for a !MMU platform seems like a good idea, since there's
>> several of us who make sure that there is build coverage of the defconfigs and
>> follow up on most of the breakage. Feel free to send one in for 3.9.
>
> Jonny just pointed out in irc that there is at91x40_defconfig.

Ah. I have built that a few times recently, but since it doesn't enable
DEBUG_LL, this problem didn't show up.

Patch

diff --git a/arch/arm/kernel/debug.S b/arch/arm/kernel/debug.S
index 6809200..14f7c3b 100644
--- a/arch/arm/kernel/debug.S
+++ b/arch/arm/kernel/debug.S
@@ -100,12 +100,14 @@  ENTRY(printch)
 		b	1b
 ENDPROC(printch)
 
+#ifdef CONFIG_MMU
 ENTRY(debug_ll_addr)
 		addruart r2, r3, ip
 		str	r2, [r0]
 		str	r3, [r1]
 		mov	pc, lr
 ENDPROC(debug_ll_addr)
+#endif
 
 #else