linux-next regression on ARM926

linux-next regression on ARM926

[Linus Walleij]

Hi Dave,

do you have any hints on how to resolve this build error in the -next
tree:

  LD      .tmp_vmlinux1
arch/arm/mm/built-in.o:(.init.data+0xe0): undefined reference to
`cpu_arm926_do_suspend'
arch/arm/mm/built-in.o:(.init.data+0xe4): undefined reference to
`cpu_arm926_do_resume'
make[2]: *** [.tmp_vmlinux1] Error 1
make[1]: *** [sub-make] Error 2
make[1]: Leaving directory `/home/linus/linux-next'
make: *** [build] Error 2

This is while building the U300, I can't really tell if the error is on my
(U300) side or in the recent patches to the proc_arm926 stuff?
It seems all ARM926 SoCs were affected.

Thanks,
Linus Walleij

linux-next regression on ARM926

[Russell King - ARM Linux]

On Mon, Jul 18, 2011 at 01:02:01PM +0200, Linus Walleij wrote:
> Hi Dave,
> 
> do you have any hints on how to resolve this build error in the -next
> tree:
> 
>   LD      .tmp_vmlinux1
> arch/arm/mm/built-in.o:(.init.data+0xe0): undefined reference to
> `cpu_arm926_do_suspend'
> arch/arm/mm/built-in.o:(.init.data+0xe4): undefined reference to
> `cpu_arm926_do_resume'
> make[2]: *** [.tmp_vmlinux1] Error 1
> make[1]: *** [sub-make] Error 2
> make[1]: Leaving directory `/home/linus/linux-next'
> make: *** [build] Error 2
> 
> This is while building the U300, I can't really tell if the error is on my
> (U300) side or in the recent patches to the proc_arm926 stuff?
> It seems all ARM926 SoCs were affected.

Hmm.

That happens because without CONFIG_PM_SLEEP, we do this:

#define cpu_arm926_do_suspend   0
#define cpu_arm926_do_resume    0

whereas the macro assembler does this:

        .word   cpu_\name\()_do_suspend
        .word   cpu_\name\()_do_resume

and this means that neither the preprocessor nor the assembler can tie
these two together.

One solution would be to put an #ifdef CONFIG_PM_SLEEP around that in
mm/proc-macros.S to select .word 0 instead, and get rid of the #else
in the individual proc-*.S files - something like this (untested):

diff --git a/arch/arm/mm/proc-arm926.S b/arch/arm/mm/proc-arm926.S
index b2f9bde..2bbcf05 100644
--- a/arch/arm/mm/proc-arm926.S
+++ b/arch/arm/mm/proc-arm926.S
@@ -421,9 +421,6 @@ ENTRY(cpu_arm926_do_resume)
 		     PMD_SECT_CACHEABLE | PMD_BIT4 | PMD_SECT_AP_WRITE
 	b	cpu_resume_mmu
 ENDPROC(cpu_arm926_do_resume)
-#else
-#define cpu_arm926_do_suspend	0
-#define cpu_arm926_do_resume	0
 #endif
 
 	__CPUINIT
diff --git a/arch/arm/mm/proc-macros.S b/arch/arm/mm/proc-macros.S
index 4ae9b44..307a4de 100644
--- a/arch/arm/mm/proc-macros.S
+++ b/arch/arm/mm/proc-macros.S
@@ -276,8 +276,13 @@ ENTRY(\name\()_processor_functions)
 
 	.if \suspend
 	.word	cpu_\name\()_suspend_size
+#ifdef CONFIG_PM_SLEEP
 	.word	cpu_\name\()_do_suspend
 	.word	cpu_\name\()_do_resume
+#else
+	.word	0
+	.word	0
+#endif
 	.else
 	.word	0
 	.word	0

linux-next regression on ARM926

[Linus Walleij]

On Mon, Jul 18, 2011 at 1:27 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:

> One solution would be to put an #ifdef CONFIG_PM_SLEEP around that in
> mm/proc-macros.S to select .word 0 instead, and get rid of the #else
> in the individual proc-*.S files - something like this (untested):

It works like a charm for U300. Thanks Russell!
Tested-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

linux-next regression on ARM926

[Dave Martin]

On Mon, Jul 18, 2011 at 12:27 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Mon, Jul 18, 2011 at 01:02:01PM +0200, Linus Walleij wrote:
>> Hi Dave,
>>
>> do you have any hints on how to resolve this build error in the -next
>> tree:
>>
>> ? LD ? ? ?.tmp_vmlinux1
>> arch/arm/mm/built-in.o:(.init.data+0xe0): undefined reference to
>> `cpu_arm926_do_suspend'
>> arch/arm/mm/built-in.o:(.init.data+0xe4): undefined reference to
>> `cpu_arm926_do_resume'
>> make[2]: *** [.tmp_vmlinux1] Error 1
>> make[1]: *** [sub-make] Error 2
>> make[1]: Leaving directory `/home/linus/linux-next'
>> make: *** [build] Error 2
>>
>> This is while building the U300, I can't really tell if the error is on my
>> (U300) side or in the recent patches to the proc_arm926 stuff?
>> It seems all ARM926 SoCs were affected.
>
> Hmm.
>
> That happens because without CONFIG_PM_SLEEP, we do this:
>
> #define cpu_arm926_do_suspend ? 0
> #define cpu_arm926_do_resume ? ?0
>
> whereas the macro assembler does this:
>
> ? ? ? ?.word ? cpu_\name\()_do_suspend
> ? ? ? ?.word ? cpu_\name\()_do_resume
>
> and this means that neither the preprocessor nor the assembler can tie
> these two together.
>
> One solution would be to put an #ifdef CONFIG_PM_SLEEP around that in
> mm/proc-macros.S to select .word 0 instead, and get rid of the #else
> in the individual proc-*.S files - something like this (untested):
>
> diff --git a/arch/arm/mm/proc-arm926.S b/arch/arm/mm/proc-arm926.S
> index b2f9bde..2bbcf05 100644
> --- a/arch/arm/mm/proc-arm926.S
> +++ b/arch/arm/mm/proc-arm926.S
> @@ -421,9 +421,6 @@ ENTRY(cpu_arm926_do_resume)
> ? ? ? ? ? ? ? ? ? ? PMD_SECT_CACHEABLE | PMD_BIT4 | PMD_SECT_AP_WRITE
> ? ? ? ?b ? ? ? cpu_resume_mmu
> ?ENDPROC(cpu_arm926_do_resume)
> -#else
> -#define cpu_arm926_do_suspend ?0
> -#define cpu_arm926_do_resume ? 0
> ?#endif
>
> ? ? ? ?__CPUINIT
> diff --git a/arch/arm/mm/proc-macros.S b/arch/arm/mm/proc-macros.S
> index 4ae9b44..307a4de 100644
> --- a/arch/arm/mm/proc-macros.S
> +++ b/arch/arm/mm/proc-macros.S
> @@ -276,8 +276,13 @@ ENTRY(\name\()_processor_functions)
>
> ? ? ? ?.if \suspend
> ? ? ? ?.word ? cpu_\name\()_suspend_size
> +#ifdef CONFIG_PM_SLEEP
> ? ? ? ?.word ? cpu_\name\()_do_suspend
> ? ? ? ?.word ? cpu_\name\()_do_resume
> +#else
> + ? ? ? .word ? 0
> + ? ? ? .word ? 0
> +#endif
> ? ? ? ?.else
> ? ? ? ?.word ? 0
> ? ? ? ?.word ? 0
>
>

The intended meaning of "suspend=1" for define_processor_functions was
"this cpu can do suspend" -- but this makes sense only if
CONFIG_PM_SLEEP is enabled.  Where processors define their suspend
functions unconditionally, that isn't a problem.  But processors
shouldn't be required (or even encouraged) to define those functions
if the kernel doesn't have suspend support at all.

So the above fix looks entirely sensible to me.

I'm offline for the next day or two, but I trust Linus' test -- so if you like:

Acked-by: Dave Martin <dave.martin@linaro.org>


We could rename the suspend argument to something like "can_suspend"
to make things clearer, but that would hit a lot of files again, so I
don't think resultant the churn would really be justified.

Cheers
---Dave

linux-next regression on ARM926

[Russell King - ARM Linux]

On Tue, Jul 19, 2011 at 11:27:38AM +0100, Dave Martin wrote:
> On Mon, Jul 18, 2011 at 12:27 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > On Mon, Jul 18, 2011 at 01:02:01PM +0200, Linus Walleij wrote:
> >> Hi Dave,
> >>
> >> do you have any hints on how to resolve this build error in the -next
> >> tree:
> >>
> >> ? LD ? ? ?.tmp_vmlinux1
> >> arch/arm/mm/built-in.o:(.init.data+0xe0): undefined reference to
> >> `cpu_arm926_do_suspend'
> >> arch/arm/mm/built-in.o:(.init.data+0xe4): undefined reference to
> >> `cpu_arm926_do_resume'
> >> make[2]: *** [.tmp_vmlinux1] Error 1
> >> make[1]: *** [sub-make] Error 2
> >> make[1]: Leaving directory `/home/linus/linux-next'
> >> make: *** [build] Error 2
> >>
> >> This is while building the U300, I can't really tell if the error is on my
> >> (U300) side or in the recent patches to the proc_arm926 stuff?
> >> It seems all ARM926 SoCs were affected.
> >
> > Hmm.
> >
> > That happens because without CONFIG_PM_SLEEP, we do this:
> >
> > #define cpu_arm926_do_suspend ? 0
> > #define cpu_arm926_do_resume ? ?0
> >
> > whereas the macro assembler does this:
> >
> > ? ? ? ?.word ? cpu_\name\()_do_suspend
> > ? ? ? ?.word ? cpu_\name\()_do_resume
> >
> > and this means that neither the preprocessor nor the assembler can tie
> > these two together.
> >
> > One solution would be to put an #ifdef CONFIG_PM_SLEEP around that in
> > mm/proc-macros.S to select .word 0 instead, and get rid of the #else
> > in the individual proc-*.S files - something like this (untested):
> >
> > diff --git a/arch/arm/mm/proc-arm926.S b/arch/arm/mm/proc-arm926.S
> > index b2f9bde..2bbcf05 100644
> > --- a/arch/arm/mm/proc-arm926.S
> > +++ b/arch/arm/mm/proc-arm926.S
> > @@ -421,9 +421,6 @@ ENTRY(cpu_arm926_do_resume)
> > ? ? ? ? ? ? ? ? ? ? PMD_SECT_CACHEABLE | PMD_BIT4 | PMD_SECT_AP_WRITE
> > ? ? ? ?b ? ? ? cpu_resume_mmu
> > ?ENDPROC(cpu_arm926_do_resume)
> > -#else
> > -#define cpu_arm926_do_suspend ?0
> > -#define cpu_arm926_do_resume ? 0
> > ?#endif
> >
> > ? ? ? ?__CPUINIT
> > diff --git a/arch/arm/mm/proc-macros.S b/arch/arm/mm/proc-macros.S
> > index 4ae9b44..307a4de 100644
> > --- a/arch/arm/mm/proc-macros.S
> > +++ b/arch/arm/mm/proc-macros.S
> > @@ -276,8 +276,13 @@ ENTRY(\name\()_processor_functions)
> >
> > ? ? ? ?.if \suspend
> > ? ? ? ?.word ? cpu_\name\()_suspend_size
> > +#ifdef CONFIG_PM_SLEEP
> > ? ? ? ?.word ? cpu_\name\()_do_suspend
> > ? ? ? ?.word ? cpu_\name\()_do_resume
> > +#else
> > + ? ? ? .word ? 0
> > + ? ? ? .word ? 0
> > +#endif
> > ? ? ? ?.else
> > ? ? ? ?.word ? 0
> > ? ? ? ?.word ? 0
> >
> >
> 
> The intended meaning of "suspend=1" for define_processor_functions was
> "this cpu can do suspend" -- but this makes sense only if
> CONFIG_PM_SLEEP is enabled.  Where processors define their suspend
> functions unconditionally, that isn't a problem.  But processors
> shouldn't be required (or even encouraged) to define those functions
> if the kernel doesn't have suspend support at all.
> 
> So the above fix looks entirely sensible to me.
> 
> I'm offline for the next day or two, but I trust Linus' test -- so if you like:
> 
> Acked-by: Dave Martin <dave.martin@linaro.org>

We need to fix up the other proc-*.S files to remove the #else clause too,
so the above patch was just supposed to be an example...

linux-next regression on ARM926

[Dave Martin]

On Tue, Jul 19, 2011 at 12:09 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Tue, Jul 19, 2011 at 11:27:38AM +0100, Dave Martin wrote:
>> On Mon, Jul 18, 2011 at 12:27 PM, Russell King - ARM Linux
>> <linux@arm.linux.org.uk> wrote:
>> > On Mon, Jul 18, 2011 at 01:02:01PM +0200, Linus Walleij wrote:
>> >> Hi Dave,
>> >>
>> >> do you have any hints on how to resolve this build error in the -next
>> >> tree:
>> >>
>> >> ? LD ? ? ?.tmp_vmlinux1
>> >> arch/arm/mm/built-in.o:(.init.data+0xe0): undefined reference to
>> >> `cpu_arm926_do_suspend'
>> >> arch/arm/mm/built-in.o:(.init.data+0xe4): undefined reference to
>> >> `cpu_arm926_do_resume'
>> >> make[2]: *** [.tmp_vmlinux1] Error 1
>> >> make[1]: *** [sub-make] Error 2
>> >> make[1]: Leaving directory `/home/linus/linux-next'
>> >> make: *** [build] Error 2
>> >>
>> >> This is while building the U300, I can't really tell if the error is on my
>> >> (U300) side or in the recent patches to the proc_arm926 stuff?
>> >> It seems all ARM926 SoCs were affected.
>> >
>> > Hmm.
>> >
>> > That happens because without CONFIG_PM_SLEEP, we do this:
>> >
>> > #define cpu_arm926_do_suspend ? 0
>> > #define cpu_arm926_do_resume ? ?0
>> >
>> > whereas the macro assembler does this:
>> >
>> > ? ? ? ?.word ? cpu_\name\()_do_suspend
>> > ? ? ? ?.word ? cpu_\name\()_do_resume
>> >
>> > and this means that neither the preprocessor nor the assembler can tie
>> > these two together.
>> >
>> > One solution would be to put an #ifdef CONFIG_PM_SLEEP around that in
>> > mm/proc-macros.S to select .word 0 instead, and get rid of the #else
>> > in the individual proc-*.S files - something like this (untested):
>> >
>> > diff --git a/arch/arm/mm/proc-arm926.S b/arch/arm/mm/proc-arm926.S
>> > index b2f9bde..2bbcf05 100644
>> > --- a/arch/arm/mm/proc-arm926.S
>> > +++ b/arch/arm/mm/proc-arm926.S
>> > @@ -421,9 +421,6 @@ ENTRY(cpu_arm926_do_resume)
>> > ? ? ? ? ? ? ? ? ? ? PMD_SECT_CACHEABLE | PMD_BIT4 | PMD_SECT_AP_WRITE
>> > ? ? ? ?b ? ? ? cpu_resume_mmu
>> > ?ENDPROC(cpu_arm926_do_resume)
>> > -#else
>> > -#define cpu_arm926_do_suspend ?0
>> > -#define cpu_arm926_do_resume ? 0
>> > ?#endif
>> >
>> > ? ? ? ?__CPUINIT
>> > diff --git a/arch/arm/mm/proc-macros.S b/arch/arm/mm/proc-macros.S
>> > index 4ae9b44..307a4de 100644
>> > --- a/arch/arm/mm/proc-macros.S
>> > +++ b/arch/arm/mm/proc-macros.S
>> > @@ -276,8 +276,13 @@ ENTRY(\name\()_processor_functions)
>> >
>> > ? ? ? ?.if \suspend
>> > ? ? ? ?.word ? cpu_\name\()_suspend_size
>> > +#ifdef CONFIG_PM_SLEEP
>> > ? ? ? ?.word ? cpu_\name\()_do_suspend
>> > ? ? ? ?.word ? cpu_\name\()_do_resume
>> > +#else
>> > + ? ? ? .word ? 0
>> > + ? ? ? .word ? 0
>> > +#endif
>> > ? ? ? ?.else
>> > ? ? ? ?.word ? 0
>> > ? ? ? ?.word ? 0
>> >
>> >
>>
>> The intended meaning of "suspend=1" for define_processor_functions was
>> "this cpu can do suspend" -- but this makes sense only if
>> CONFIG_PM_SLEEP is enabled. ?Where processors define their suspend
>> functions unconditionally, that isn't a problem. ?But processors
>> shouldn't be required (or even encouraged) to define those functions
>> if the kernel doesn't have suspend support at all.
>>
>> So the above fix looks entirely sensible to me.
>>
>> I'm offline for the next day or two, but I trust Linus' test -- so if you like:
>>
>> Acked-by: Dave Martin <dave.martin@linaro.org>
>
> We need to fix up the other proc-*.S files to remove the #else clause too,
> so the above patch was just supposed to be an example...
>

Hmmm, I'll take a closer look at the implications ... but
unfortunately I'm not going to be able to do much until Thursday.

Cheers
---Dave

linux-next regression on ARM926

[Russell King - ARM Linux]

On Wed, Jul 20, 2011 at 09:35:03AM +0100, Dave Martin wrote:
> On Tue, Jul 19, 2011 at 12:09 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > On Tue, Jul 19, 2011 at 11:27:38AM +0100, Dave Martin wrote:
> >> On Mon, Jul 18, 2011 at 12:27 PM, Russell King - ARM Linux
> >> <linux@arm.linux.org.uk> wrote:
> >> > On Mon, Jul 18, 2011 at 01:02:01PM +0200, Linus Walleij wrote:
> >> >> Hi Dave,
> >> >>
> >> >> do you have any hints on how to resolve this build error in the -next
> >> >> tree:
> >> >>
> >> >> ? LD ? ? ?.tmp_vmlinux1
> >> >> arch/arm/mm/built-in.o:(.init.data+0xe0): undefined reference to
> >> >> `cpu_arm926_do_suspend'
> >> >> arch/arm/mm/built-in.o:(.init.data+0xe4): undefined reference to
> >> >> `cpu_arm926_do_resume'
> >> >> make[2]: *** [.tmp_vmlinux1] Error 1
> >> >> make[1]: *** [sub-make] Error 2
> >> >> make[1]: Leaving directory `/home/linus/linux-next'
> >> >> make: *** [build] Error 2
> >> >>
> >> >> This is while building the U300, I can't really tell if the error is on my
> >> >> (U300) side or in the recent patches to the proc_arm926 stuff?
> >> >> It seems all ARM926 SoCs were affected.
> >> >
> >> > Hmm.
> >> >
> >> > That happens because without CONFIG_PM_SLEEP, we do this:
> >> >
> >> > #define cpu_arm926_do_suspend ? 0
> >> > #define cpu_arm926_do_resume ? ?0
> >> >
> >> > whereas the macro assembler does this:
> >> >
> >> > ? ? ? ?.word ? cpu_\name\()_do_suspend
> >> > ? ? ? ?.word ? cpu_\name\()_do_resume
> >> >
> >> > and this means that neither the preprocessor nor the assembler can tie
> >> > these two together.
> >> >
> >> > One solution would be to put an #ifdef CONFIG_PM_SLEEP around that in
> >> > mm/proc-macros.S to select .word 0 instead, and get rid of the #else
> >> > in the individual proc-*.S files - something like this (untested):
> >> >
> >> > diff --git a/arch/arm/mm/proc-arm926.S b/arch/arm/mm/proc-arm926.S
> >> > index b2f9bde..2bbcf05 100644
> >> > --- a/arch/arm/mm/proc-arm926.S
> >> > +++ b/arch/arm/mm/proc-arm926.S
> >> > @@ -421,9 +421,6 @@ ENTRY(cpu_arm926_do_resume)
> >> > ? ? ? ? ? ? ? ? ? ? PMD_SECT_CACHEABLE | PMD_BIT4 | PMD_SECT_AP_WRITE
> >> > ? ? ? ?b ? ? ? cpu_resume_mmu
> >> > ?ENDPROC(cpu_arm926_do_resume)
> >> > -#else
> >> > -#define cpu_arm926_do_suspend ?0
> >> > -#define cpu_arm926_do_resume ? 0
> >> > ?#endif
> >> >
> >> > ? ? ? ?__CPUINIT
> >> > diff --git a/arch/arm/mm/proc-macros.S b/arch/arm/mm/proc-macros.S
> >> > index 4ae9b44..307a4de 100644
> >> > --- a/arch/arm/mm/proc-macros.S
> >> > +++ b/arch/arm/mm/proc-macros.S
> >> > @@ -276,8 +276,13 @@ ENTRY(\name\()_processor_functions)
> >> >
> >> > ? ? ? ?.if \suspend
> >> > ? ? ? ?.word ? cpu_\name\()_suspend_size
> >> > +#ifdef CONFIG_PM_SLEEP
> >> > ? ? ? ?.word ? cpu_\name\()_do_suspend
> >> > ? ? ? ?.word ? cpu_\name\()_do_resume
> >> > +#else
> >> > + ? ? ? .word ? 0
> >> > + ? ? ? .word ? 0
> >> > +#endif
> >> > ? ? ? ?.else
> >> > ? ? ? ?.word ? 0
> >> > ? ? ? ?.word ? 0
> >> >
> >> >
> >>
> >> The intended meaning of "suspend=1" for define_processor_functions was
> >> "this cpu can do suspend" -- but this makes sense only if
> >> CONFIG_PM_SLEEP is enabled. ?Where processors define their suspend
> >> functions unconditionally, that isn't a problem. ?But processors
> >> shouldn't be required (or even encouraged) to define those functions
> >> if the kernel doesn't have suspend support at all.
> >>
> >> So the above fix looks entirely sensible to me.
> >>
> >> I'm offline for the next day or two, but I trust Linus' test -- so if you like:
> >>
> >> Acked-by: Dave Martin <dave.martin@linaro.org>
> >
> > We need to fix up the other proc-*.S files to remove the #else clause too,
> > so the above patch was just supposed to be an example...
> >
> 
> Hmmm, I'll take a closer look at the implications ... but
> unfortunately I'm not going to be able to do much until Thursday.

I've just applied such an extended patch covering the other proc-*.S
files - with your ack:

8<--------
From: Russell King - ARM Linux <linux@arm.linux.org.uk>
Subject: [PATCH] ARM: Fix build errors caused by adding generic macros

Commit 66a625a (ARM: mm: proc-macros: Add generic proc/cache/tlb struct
definition macros) introduced build errors when PM_SLEEP is not enabled.
The per-CPU do_suspend/do_resume functions are defined via the
preprocessor to constant 0.  However, the macros which use these were
converted to assembly, resulting in undefined references to these
functions.  Fix that by moving the ! ifdef section into proc-macros.S
and deleting it from all effected proc-*.S files.

Acked-by: Dave Martin <dave.martin@linaro.org>
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 arch/arm/mm/proc-arm920.S |    3 ---
 arch/arm/mm/proc-arm926.S |    3 ---
 arch/arm/mm/proc-macros.S |    5 +++++
 arch/arm/mm/proc-sa1100.S |    3 ---
 arch/arm/mm/proc-v7.S     |    3 ---
 arch/arm/mm/proc-xsc3.S   |    3 ---
 arch/arm/mm/proc-xscale.S |    3 ---
 7 files changed, 5 insertions(+), 18 deletions(-)

diff --git a/arch/arm/mm/proc-arm920.S b/arch/arm/mm/proc-arm920.S
index 0dea376..92bd102 100644
--- a/arch/arm/mm/proc-arm920.S
+++ b/arch/arm/mm/proc-arm920.S
@@ -406,9 +406,6 @@ ENTRY(cpu_arm920_do_resume)
 		     PMD_SECT_CACHEABLE | PMD_BIT4 | PMD_SECT_AP_WRITE
 	b	cpu_resume_mmu
 ENDPROC(cpu_arm920_do_resume)
-#else
-#define cpu_arm920_do_suspend	0
-#define cpu_arm920_do_resume	0
 #endif
 
 	__CPUINIT
diff --git a/arch/arm/mm/proc-arm926.S b/arch/arm/mm/proc-arm926.S
index b2f9bde..2bbcf05 100644
--- a/arch/arm/mm/proc-arm926.S
+++ b/arch/arm/mm/proc-arm926.S
@@ -421,9 +421,6 @@ ENTRY(cpu_arm926_do_resume)
 		     PMD_SECT_CACHEABLE | PMD_BIT4 | PMD_SECT_AP_WRITE
 	b	cpu_resume_mmu
 ENDPROC(cpu_arm926_do_resume)
-#else
-#define cpu_arm926_do_suspend	0
-#define cpu_arm926_do_resume	0
 #endif
 
 	__CPUINIT
diff --git a/arch/arm/mm/proc-macros.S b/arch/arm/mm/proc-macros.S
index 4ae9b44..307a4de 100644
--- a/arch/arm/mm/proc-macros.S
+++ b/arch/arm/mm/proc-macros.S
@@ -276,8 +276,13 @@ ENTRY(\name\()_processor_functions)
 
 	.if \suspend
 	.word	cpu_\name\()_suspend_size
+#ifdef CONFIG_PM_SLEEP
 	.word	cpu_\name\()_do_suspend
 	.word	cpu_\name\()_do_resume
+#else
+	.word	0
+	.word	0
+#endif
 	.else
 	.word	0
 	.word	0
diff --git a/arch/arm/mm/proc-sa1100.S b/arch/arm/mm/proc-sa1100.S
index c7e08ca..e715878 100644
--- a/arch/arm/mm/proc-sa1100.S
+++ b/arch/arm/mm/proc-sa1100.S
@@ -200,9 +200,6 @@ ENTRY(cpu_sa1100_do_resume)
 		     PMD_SECT_CACHEABLE | PMD_SECT_AP_WRITE
 	b	cpu_resume_mmu
 ENDPROC(cpu_sa1100_do_resume)
-#else
-#define cpu_sa1100_do_suspend	0
-#define cpu_sa1100_do_resume	0
 #endif
 
 	__CPUINIT
diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
index 54d1a63..a30e785 100644
--- a/arch/arm/mm/proc-v7.S
+++ b/arch/arm/mm/proc-v7.S
@@ -263,9 +263,6 @@ ENDPROC(cpu_v7_do_resume)
 cpu_resume_l1_flags:
 	ALT_SMP(.long PMD_TYPE_SECT | PMD_SECT_AP_WRITE | PMD_FLAGS_SMP)
 	ALT_UP(.long  PMD_TYPE_SECT | PMD_SECT_AP_WRITE | PMD_FLAGS_UP)
-#else
-#define cpu_v7_do_suspend	0
-#define cpu_v7_do_resume	0
 #endif
 
 	__CPUINIT
diff --git a/arch/arm/mm/proc-xsc3.S b/arch/arm/mm/proc-xsc3.S
index 1508f9b..64f1fc7 100644
--- a/arch/arm/mm/proc-xsc3.S
+++ b/arch/arm/mm/proc-xsc3.S
@@ -445,9 +445,6 @@ ENTRY(cpu_xsc3_do_resume)
 	ldr	r3, =0x542e		@ section flags
 	b	cpu_resume_mmu
 ENDPROC(cpu_xsc3_do_resume)
-#else
-#define cpu_xsc3_do_suspend	0
-#define cpu_xsc3_do_resume	0
 #endif
 
 	__CPUINIT
diff --git a/arch/arm/mm/proc-xscale.S b/arch/arm/mm/proc-xscale.S
index 76a8046..fbc06e5 100644
--- a/arch/arm/mm/proc-xscale.S
+++ b/arch/arm/mm/proc-xscale.S
@@ -554,9 +554,6 @@ ENTRY(cpu_xscale_do_resume)
 		     PMD_SECT_CACHEABLE | PMD_SECT_AP_WRITE
 	b	cpu_resume_mmu
 ENDPROC(cpu_xscale_do_resume)
-#else
-#define cpu_xscale_do_suspend	0
-#define cpu_xscale_do_resume	0
 #endif
 
 	__CPUINIT
-- 
1.7.4.4

linux-next regression on ARM926

[Dave Martin]

On Thu, Jul 21, 2011 at 2:48 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Wed, Jul 20, 2011 at 09:35:03AM +0100, Dave Martin wrote:
>> On Tue, Jul 19, 2011 at 12:09 PM, Russell King - ARM Linux
>> <linux@arm.linux.org.uk> wrote:
>> > On Tue, Jul 19, 2011 at 11:27:38AM +0100, Dave Martin wrote:
>> >> On Mon, Jul 18, 2011 at 12:27 PM, Russell King - ARM Linux
>> >> <linux@arm.linux.org.uk> wrote:
>> >> > On Mon, Jul 18, 2011 at 01:02:01PM +0200, Linus Walleij wrote:
>> >> >> Hi Dave,
>> >> >>
>> >> >> do you have any hints on how to resolve this build error in the -next
>> >> >> tree:
>> >> >>
>> >> >> ? LD ? ? ?.tmp_vmlinux1
>> >> >> arch/arm/mm/built-in.o:(.init.data+0xe0): undefined reference to
>> >> >> `cpu_arm926_do_suspend'
>> >> >> arch/arm/mm/built-in.o:(.init.data+0xe4): undefined reference to
>> >> >> `cpu_arm926_do_resume'
>> >> >> make[2]: *** [.tmp_vmlinux1] Error 1
>> >> >> make[1]: *** [sub-make] Error 2
>> >> >> make[1]: Leaving directory `/home/linus/linux-next'
>> >> >> make: *** [build] Error 2
>> >> >>
>> >> >> This is while building the U300, I can't really tell if the error is on my
>> >> >> (U300) side or in the recent patches to the proc_arm926 stuff?
>> >> >> It seems all ARM926 SoCs were affected.
>> >> >
>> >> > Hmm.
>> >> >
>> >> > That happens because without CONFIG_PM_SLEEP, we do this:
>> >> >
>> >> > #define cpu_arm926_do_suspend ? 0
>> >> > #define cpu_arm926_do_resume ? ?0
>> >> >
>> >> > whereas the macro assembler does this:
>> >> >
>> >> > ? ? ? ?.word ? cpu_\name\()_do_suspend
>> >> > ? ? ? ?.word ? cpu_\name\()_do_resume
>> >> >
>> >> > and this means that neither the preprocessor nor the assembler can tie
>> >> > these two together.
>> >> >
>> >> > One solution would be to put an #ifdef CONFIG_PM_SLEEP around that in
>> >> > mm/proc-macros.S to select .word 0 instead, and get rid of the #else
>> >> > in the individual proc-*.S files - something like this (untested):
>> >> >
>> >> > diff --git a/arch/arm/mm/proc-arm926.S b/arch/arm/mm/proc-arm926.S
>> >> > index b2f9bde..2bbcf05 100644
>> >> > --- a/arch/arm/mm/proc-arm926.S
>> >> > +++ b/arch/arm/mm/proc-arm926.S
>> >> > @@ -421,9 +421,6 @@ ENTRY(cpu_arm926_do_resume)
>> >> > ? ? ? ? ? ? ? ? ? ? PMD_SECT_CACHEABLE | PMD_BIT4 | PMD_SECT_AP_WRITE
>> >> > ? ? ? ?b ? ? ? cpu_resume_mmu
>> >> > ?ENDPROC(cpu_arm926_do_resume)
>> >> > -#else
>> >> > -#define cpu_arm926_do_suspend ?0
>> >> > -#define cpu_arm926_do_resume ? 0
>> >> > ?#endif
>> >> >
>> >> > ? ? ? ?__CPUINIT
>> >> > diff --git a/arch/arm/mm/proc-macros.S b/arch/arm/mm/proc-macros.S
>> >> > index 4ae9b44..307a4de 100644
>> >> > --- a/arch/arm/mm/proc-macros.S
>> >> > +++ b/arch/arm/mm/proc-macros.S
>> >> > @@ -276,8 +276,13 @@ ENTRY(\name\()_processor_functions)
>> >> >
>> >> > ? ? ? ?.if \suspend
>> >> > ? ? ? ?.word ? cpu_\name\()_suspend_size
>> >> > +#ifdef CONFIG_PM_SLEEP
>> >> > ? ? ? ?.word ? cpu_\name\()_do_suspend
>> >> > ? ? ? ?.word ? cpu_\name\()_do_resume
>> >> > +#else
>> >> > + ? ? ? .word ? 0
>> >> > + ? ? ? .word ? 0
>> >> > +#endif
>> >> > ? ? ? ?.else
>> >> > ? ? ? ?.word ? 0
>> >> > ? ? ? ?.word ? 0
>> >> >
>> >> >
>> >>
>> >> The intended meaning of "suspend=1" for define_processor_functions was
>> >> "this cpu can do suspend" -- but this makes sense only if
>> >> CONFIG_PM_SLEEP is enabled. ?Where processors define their suspend
>> >> functions unconditionally, that isn't a problem. ?But processors
>> >> shouldn't be required (or even encouraged) to define those functions
>> >> if the kernel doesn't have suspend support at all.
>> >>
>> >> So the above fix looks entirely sensible to me.
>> >>
>> >> I'm offline for the next day or two, but I trust Linus' test -- so if you like:
>> >>
>> >> Acked-by: Dave Martin <dave.martin@linaro.org>
>> >
>> > We need to fix up the other proc-*.S files to remove the #else clause too,
>> > so the above patch was just supposed to be an example...
>> >
>>
>> Hmmm, I'll take a closer look at the implications ... but
>> unfortunately I'm not going to be able to do much until Thursday.
>
> I've just applied such an extended patch covering the other proc-*.S
> files - with your ack:
>
> 8<--------
> From: Russell King - ARM Linux <linux@arm.linux.org.uk>
> Subject: [PATCH] ARM: Fix build errors caused by adding generic macros
>
> Commit 66a625a (ARM: mm: proc-macros: Add generic proc/cache/tlb struct
> definition macros) introduced build errors when PM_SLEEP is not enabled.
> The per-CPU do_suspend/do_resume functions are defined via the
> preprocessor to constant 0. ?However, the macros which use these were
> converted to assembly, resulting in undefined references to these
> functions. ?Fix that by moving the ! ifdef section into proc-macros.S
> and deleting it from all effected proc-*.S files.
>
> Acked-by: Dave Martin <dave.martin@linaro.org>
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> ---
> ?arch/arm/mm/proc-arm920.S | ? ?3 ---
> ?arch/arm/mm/proc-arm926.S | ? ?3 ---
> ?arch/arm/mm/proc-macros.S | ? ?5 +++++
> ?arch/arm/mm/proc-sa1100.S | ? ?3 ---
> ?arch/arm/mm/proc-v7.S ? ? | ? ?3 ---
> ?arch/arm/mm/proc-xsc3.S ? | ? ?3 ---
> ?arch/arm/mm/proc-xscale.S | ? ?3 ---
> ?7 files changed, 5 insertions(+), 18 deletions(-)
>
> diff --git a/arch/arm/mm/proc-arm920.S b/arch/arm/mm/proc-arm920.S
> index 0dea376..92bd102 100644
> --- a/arch/arm/mm/proc-arm920.S
> +++ b/arch/arm/mm/proc-arm920.S
> @@ -406,9 +406,6 @@ ENTRY(cpu_arm920_do_resume)
> ? ? ? ? ? ? ? ? ? ? PMD_SECT_CACHEABLE | PMD_BIT4 | PMD_SECT_AP_WRITE
> ? ? ? ?b ? ? ? cpu_resume_mmu
> ?ENDPROC(cpu_arm920_do_resume)
> -#else
> -#define cpu_arm920_do_suspend ?0
> -#define cpu_arm920_do_resume ? 0
> ?#endif
>
> ? ? ? ?__CPUINIT
> diff --git a/arch/arm/mm/proc-arm926.S b/arch/arm/mm/proc-arm926.S
> index b2f9bde..2bbcf05 100644
> --- a/arch/arm/mm/proc-arm926.S
> +++ b/arch/arm/mm/proc-arm926.S
> @@ -421,9 +421,6 @@ ENTRY(cpu_arm926_do_resume)
> ? ? ? ? ? ? ? ? ? ? PMD_SECT_CACHEABLE | PMD_BIT4 | PMD_SECT_AP_WRITE
> ? ? ? ?b ? ? ? cpu_resume_mmu
> ?ENDPROC(cpu_arm926_do_resume)
> -#else
> -#define cpu_arm926_do_suspend ?0
> -#define cpu_arm926_do_resume ? 0
> ?#endif
>
> ? ? ? ?__CPUINIT
> diff --git a/arch/arm/mm/proc-macros.S b/arch/arm/mm/proc-macros.S
> index 4ae9b44..307a4de 100644
> --- a/arch/arm/mm/proc-macros.S
> +++ b/arch/arm/mm/proc-macros.S
> @@ -276,8 +276,13 @@ ENTRY(\name\()_processor_functions)
>
> ? ? ? ?.if \suspend
> ? ? ? ?.word ? cpu_\name\()_suspend_size
> +#ifdef CONFIG_PM_SLEEP
> ? ? ? ?.word ? cpu_\name\()_do_suspend
> ? ? ? ?.word ? cpu_\name\()_do_resume
> +#else
> + ? ? ? .word ? 0
> + ? ? ? .word ? 0
> +#endif
> ? ? ? ?.else
> ? ? ? ?.word ? 0
> ? ? ? ?.word ? 0
> diff --git a/arch/arm/mm/proc-sa1100.S b/arch/arm/mm/proc-sa1100.S
> index c7e08ca..e715878 100644
> --- a/arch/arm/mm/proc-sa1100.S
> +++ b/arch/arm/mm/proc-sa1100.S
> @@ -200,9 +200,6 @@ ENTRY(cpu_sa1100_do_resume)
> ? ? ? ? ? ? ? ? ? ? PMD_SECT_CACHEABLE | PMD_SECT_AP_WRITE
> ? ? ? ?b ? ? ? cpu_resume_mmu
> ?ENDPROC(cpu_sa1100_do_resume)
> -#else
> -#define cpu_sa1100_do_suspend ?0
> -#define cpu_sa1100_do_resume ? 0
> ?#endif
>
> ? ? ? ?__CPUINIT
> diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
> index 54d1a63..a30e785 100644
> --- a/arch/arm/mm/proc-v7.S
> +++ b/arch/arm/mm/proc-v7.S
> @@ -263,9 +263,6 @@ ENDPROC(cpu_v7_do_resume)
> ?cpu_resume_l1_flags:
> ? ? ? ?ALT_SMP(.long PMD_TYPE_SECT | PMD_SECT_AP_WRITE | PMD_FLAGS_SMP)
> ? ? ? ?ALT_UP(.long ?PMD_TYPE_SECT | PMD_SECT_AP_WRITE | PMD_FLAGS_UP)
> -#else
> -#define cpu_v7_do_suspend ? ? ?0
> -#define cpu_v7_do_resume ? ? ? 0
> ?#endif
>
> ? ? ? ?__CPUINIT
> diff --git a/arch/arm/mm/proc-xsc3.S b/arch/arm/mm/proc-xsc3.S
> index 1508f9b..64f1fc7 100644
> --- a/arch/arm/mm/proc-xsc3.S
> +++ b/arch/arm/mm/proc-xsc3.S
> @@ -445,9 +445,6 @@ ENTRY(cpu_xsc3_do_resume)
> ? ? ? ?ldr ? ? r3, =0x542e ? ? ? ? ? ? @ section flags
> ? ? ? ?b ? ? ? cpu_resume_mmu
> ?ENDPROC(cpu_xsc3_do_resume)
> -#else
> -#define cpu_xsc3_do_suspend ? ?0
> -#define cpu_xsc3_do_resume ? ? 0
> ?#endif
>
> ? ? ? ?__CPUINIT
> diff --git a/arch/arm/mm/proc-xscale.S b/arch/arm/mm/proc-xscale.S
> index 76a8046..fbc06e5 100644
> --- a/arch/arm/mm/proc-xscale.S
> +++ b/arch/arm/mm/proc-xscale.S
> @@ -554,9 +554,6 @@ ENTRY(cpu_xscale_do_resume)
> ? ? ? ? ? ? ? ? ? ? PMD_SECT_CACHEABLE | PMD_SECT_AP_WRITE
> ? ? ? ?b ? ? ? cpu_resume_mmu
> ?ENDPROC(cpu_xscale_do_resume)
> -#else
> -#define cpu_xscale_do_suspend ?0
> -#define cpu_xscale_do_resume ? 0
> ?#endif
>
> ? ? ? ?__CPUINIT
> --
> 1.7.4.4
>
>
>

Looks fine ... though I just spent a couple of hours doing the same
thing.  Unfortunately my email access has been a bit erratic today.

Did you miss proc-v6, or is that touched by some other patch?  I
already have the relevant patch, so I'm happy to post that if you
don't already have it.

My proc-maros.S patch is slightly different in that it avoids defining
the default values twice (i.e., once for !CONFIG_PM_SUSPEND and once
for suspend=0); but it's otherwise equivalent.

I also took the opportunity to remove the definitions of
cpu_<name>_suspend_size for the !CONFIG_PM_SUSPEND case, since this
helps to ensure they are not used accidentally.  That's a more
cosmetic change.

Do you want me to post my whole series anyway, or otherwise can you
point me to where you applied the patches so I can filter my series?

Cheers
---Dave

linux-next regression on ARM926

[Russell King - ARM Linux]

On Thu, Jul 21, 2011 at 05:39:06PM +0100, Dave Martin wrote:
> Did you miss proc-v6, or is that touched by some other patch?  I
> already have the relevant patch, so I'm happy to post that if you
> don't already have it.

Well spotted, I've just corrected that.

> My proc-maros.S patch is slightly different in that it avoids defining
> the default values twice (i.e., once for !CONFIG_PM_SUSPEND and once
> for suspend=0); but it's otherwise equivalent.
> 
> I also took the opportunity to remove the definitions of
> cpu_<name>_suspend_size for the !CONFIG_PM_SUSPEND case, since this
> helps to ensure they are not used accidentally.  That's a more
> cosmetic change.

The missing symbols for do_suspend/do_resume should be enough on their
own to catch people using it, so I don't think that's necessary.

> Do you want me to post my whole series anyway, or otherwise can you
> point me to where you applied the patches so I can filter my series?

I'll push it out later this evening, it'll be part of the devel-stable
branch so once pushed it becomes immutable.

linux-next regression on ARM926

[Dave Martin]

On Thu, Jul 21, 2011 at 5:52 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Thu, Jul 21, 2011 at 05:39:06PM +0100, Dave Martin wrote:
>> Did you miss proc-v6, or is that touched by some other patch? ?I
>> already have the relevant patch, so I'm happy to post that if you
>> don't already have it.
>
> Well spotted, I've just corrected that.

I think that should be all of them -- I had hacked up a quick script
to find and change all instances, but proc-v6 was the only one it
touched which wasn't in your series.

I also build-tested the change in several configurations just to be
sure.  It looks sound, so far as I can tell.

>
>> My proc-maros.S patch is slightly different in that it avoids defining
>> the default values twice (i.e., once for !CONFIG_PM_SUSPEND and once
>> for suspend=0); but it's otherwise equivalent.
>>
>> I also took the opportunity to remove the definitions of
>> cpu_<name>_suspend_size for the !CONFIG_PM_SUSPEND case, since this
>> helps to ensure they are not used accidentally. ?That's a more
>> cosmetic change.
>
> The missing symbols for do_suspend/do_resume should be enough on their
> own to catch people using it, so I don't think that's necessary.

OK, agreed.

>> Do you want me to post my whole series anyway, or otherwise can you
>> point me to where you applied the patches so I can filter my series?
>
> I'll push it out later this evening, it'll be part of the devel-stable
> branch so once pushed it becomes immutable.

in that that case I guess I don't have anything to add, so I'll drop
my patches.  Apologies for the last-minute churn...

Thanks
---Dave

linux-next regression on ARM926

[Linus Walleij]

On Thu, Jul 21, 2011 at 3:48 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Wed, Jul 20, 2011 at 09:35:03AM +0100, Dave Martin wrote:
>> Hmmm, I'll take a closer look at the implications ... but
>> unfortunately I'm not going to be able to do much until Thursday.
>
> I've just applied such an extended patch covering the other proc-*.S
> files - with your ack:

Looks good to me too, thanks Russell!

Yours,
Linus Walleij

linux-next regression on ARM926

[Russell King - ARM Linux]

On Thu, Jul 21, 2011 at 07:46:13PM +0200, Linus Walleij wrote:
> On Thu, Jul 21, 2011 at 3:48 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > On Wed, Jul 20, 2011 at 09:35:03AM +0100, Dave Martin wrote:
> >> Hmmm, I'll take a closer look at the implications ... but
> >> unfortunately I'm not going to be able to do much until Thursday.
> >
> > I've just applied such an extended patch covering the other proc-*.S
> > files - with your ack:
> 
> Looks good to me too, thanks Russell!

Before it push it out, is that an ack or something like that?

linux-next regression on ARM926

[Dave Martin]

On Thu, Jul 21, 2011 at 06:00:05PM +0100, Dave Martin wrote:
> On Thu, Jul 21, 2011 at 5:52 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > On Thu, Jul 21, 2011 at 05:39:06PM +0100, Dave Martin wrote:
> >> Did you miss proc-v6, or is that touched by some other patch? ?I
> >> already have the relevant patch, so I'm happy to post that if you
> >> don't already have it.
> >
> > Well spotted, I've just corrected that.
> 
> I think that should be all of them -- I had hacked up a quick script
> to find and change all instances, but proc-v6 was the only one it
> touched which wasn't in your series.

I didn't mention, but you can add my ack to the patch for proc-v6 as well
(though you probably did already, which is fine.)

Cheers
---Dave

linux-next regression on ARM926

[Linus Walleij]

On Thu, Jul 21, 2011 at 8:01 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Thu, Jul 21, 2011 at 07:46:13PM +0200, Linus Walleij wrote:
>>
>> Looks good to me too, thanks Russell!
>
> Before it push it out, is that an ack or something like that?

Sorry,
Acked-by: Linus Walleij <linus.walleij@linaro.org>

Thanks again,
Linus Walleij

linux-next regression on ARM926

[Russell King - ARM Linux]

On Fri, Jul 22, 2011 at 03:49:37PM +0200, Linus Walleij wrote:
> On Thu, Jul 21, 2011 at 8:01 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > On Thu, Jul 21, 2011 at 07:46:13PM +0200, Linus Walleij wrote:
> >>
> >> Looks good to me too, thanks Russell!
> >
> > Before it push it out, is that an ack or something like that?
> 
> Sorry,
> Acked-by: Linus Walleij <linus.walleij@linaro.org>

Too late, sorry.