[PATCH RFC] ARM: tango4: Fix build issues

[PATCH RFC] ARM: tango4: Fix build issues

[Mason]

From: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>

Move SMP setup to platsmp.c
Specify the CPU type to the assembler, and only for smc.S

Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
---
 arch/arm/mach-tango/Makefile  |  3 ++-
 arch/arm/mach-tango/platsmp.c | 16 ++++++++++++++++
 arch/arm/mach-tango/setup.c   | 14 --------------
 3 files changed, 18 insertions(+), 15 deletions(-)
 create mode 100644 arch/arm/mach-tango/platsmp.c

diff --git a/arch/arm/mach-tango/Makefile b/arch/arm/mach-tango/Makefile
index 0d7e2b5976e3..d33eab2941cb 100644
--- a/arch/arm/mach-tango/Makefile
+++ b/arch/arm/mach-tango/Makefile
@@ -1,2 +1,3 @@
-asflags-y += -mcpu=cortex-a9
 obj-y += setup.o smc.o
+obj-$(CONFIG_SMP) += platsmp.o
+AFLAGS_smc.o := -Wa,-mcpu=cortex-a9
diff --git a/arch/arm/mach-tango/platsmp.c b/arch/arm/mach-tango/platsmp.c
new file mode 100644
index 000000000000..a18d5a34e2f5
--- /dev/null
+++ b/arch/arm/mach-tango/platsmp.c
@@ -0,0 +1,16 @@
+#include <linux/init.h>
+#include <linux/smp.h>
+#include "smc.h"
+
+static int tango_boot_secondary(unsigned int cpu, struct task_struct *idle)
+{
+	tango_set_aux_boot_addr(virt_to_phys(secondary_startup));
+	tango_start_aux_core(cpu);
+	return 0;
+}
+
+static struct smp_operations tango_smp_ops __initdata = {
+	.smp_boot_secondary	= tango_boot_secondary,
+};
+
+CPU_METHOD_OF_DECLARE(tango4_smp, "sigma,tango4-smp", &tango_smp_ops);
diff --git a/arch/arm/mach-tango/setup.c b/arch/arm/mach-tango/setup.c
index d86900550e63..f14b6c7d255b 100644
--- a/arch/arm/mach-tango/setup.c
+++ b/arch/arm/mach-tango/setup.c
@@ -1,21 +1,7 @@
-#include <linux/smp.h>
 #include <asm/mach/arch.h>
 #include <asm/hardware/cache-l2x0.h>
 #include "smc.h"
 
-static int tango4_boot_secondary(unsigned int cpu, struct task_struct *idle)
-{
-	tango_set_aux_boot_addr(virt_to_phys(secondary_startup));
-	tango_start_aux_core(cpu);
-	return 0;
-}
-
-static struct smp_operations tango4_smp_ops __initdata = {
-	.smp_boot_secondary	= tango4_boot_secondary,
-};
-
-CPU_METHOD_OF_DECLARE(tango4_smp, "sigma,tango4-smp", &tango4_smp_ops);
-
 static void tango_l2c_write(unsigned long val, unsigned int reg)
 {
 	if (reg == L2X0_CTRL)
-- 
2.4.3

[PATCH RFC] ARM: tango4: Fix build issues

[Mason]

On 25/12/2015 22:49, Mason wrote:

> Move SMP setup to platsmp.c
> Specify the CPU type to the assembler, and only for smc.S
> 
> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
> ---
>  arch/arm/mach-tango/Makefile  |  3 ++-
>  arch/arm/mach-tango/platsmp.c | 16 ++++++++++++++++
>  arch/arm/mach-tango/setup.c   | 14 --------------
>  3 files changed, 18 insertions(+), 15 deletions(-)
>  create mode 100644 arch/arm/mach-tango/platsmp.c

I have a question. When the original commit hasn't been pushed to
Linus' tree yet, is the original commit simply amended?
(fixup option in rebase -i)

Or are they handled as regular commits on top of the previous commits?

Regards.

[PATCH RFC] ARM: tango4: Fix build issues

[Joshua Clayton]

Hi Marc,
On Tuesday, December 29, 2015 09:48:47 PM Mason wrote:
> On 25/12/2015 22:49, Mason wrote:
> 
> > Move SMP setup to platsmp.c
> > Specify the CPU type to the assembler, and only for smc.S
> > 
> > Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
> > ---
> >  arch/arm/mach-tango/Makefile  |  3 ++-
> >  arch/arm/mach-tango/platsmp.c | 16 ++++++++++++++++
> >  arch/arm/mach-tango/setup.c   | 14 --------------
> >  3 files changed, 18 insertions(+), 15 deletions(-)
> >  create mode 100644 arch/arm/mach-tango/platsmp.c
> 
> I have a question. When the original commit hasn't been pushed to
> Linus' tree yet, is the original commit simply amended?
> (fixup option in rebase -i)
> 
> Or are they handled as regular commits on top of the previous commits?
> 

 Unless you are asked to revise and resubmit a patch, so that the 
original is never accepted, all follow on patches end up a separate commits.

> Regards.
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kerne

Merry Christmas & Happy New Year,

Joshua

[PATCH RFC] ARM: tango4: Fix build issues

[Olof Johansson]

On Tue, Dec 29, 2015 at 09:48:47PM +0100, Mason wrote:
> On 25/12/2015 22:49, Mason wrote:
> 
> > Move SMP setup to platsmp.c
> > Specify the CPU type to the assembler, and only for smc.S
> > 
> > Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
> > ---
> >  arch/arm/mach-tango/Makefile  |  3 ++-
> >  arch/arm/mach-tango/platsmp.c | 16 ++++++++++++++++
> >  arch/arm/mach-tango/setup.c   | 14 --------------
> >  3 files changed, 18 insertions(+), 15 deletions(-)
> >  create mode 100644 arch/arm/mach-tango/platsmp.c
> 
> I have a question. When the original commit hasn't been pushed to
> Linus' tree yet, is the original commit simply amended?
> (fixup option in rebase -i)
> 
> Or are they handled as regular commits on top of the previous commits?

In general, even if our branch is not yet in Linus' tree, we prefer to not
rebuild and rewind/rebase our branches. So yes, in this case it should be
an incremental patch like this is.


-Olof

[PATCH RFC] ARM: tango4: Fix build issues

[Olof Johansson]

On Fri, Dec 25, 2015 at 10:49:56PM +0100, Mason wrote:
> From: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
> 
> Move SMP setup to platsmp.c
> Specify the CPU type to the assembler, and only for smc.S
> 
> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
> ---

Applied, thanks.


-Olof

[PATCH RFC] ARM: tango4: Fix build issues

[Olof Johansson]

On Thu, Dec 31, 2015 at 2:21 PM, Olof Johansson <olof@lixom.net> wrote:
> On Fri, Dec 25, 2015 at 10:49:56PM +0100, Mason wrote:
>> From: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
>>
>> Move SMP setup to platsmp.c
>> Specify the CPU type to the assembler, and only for smc.S
>>
>> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
>> ---
>
> Applied, thanks.

Actually, I just noticed that the smc.c Makefile part isn't done as it
should, so I'll drop this again.


-Olof

[PATCH RFC] ARM: tango4: Fix build issues

[Arnd Bergmann]

On Thursday 31 December 2015 17:31:57 Olof Johansson wrote:
> On Thu, Dec 31, 2015 at 2:21 PM, Olof Johansson <olof@lixom.net> wrote:
> > On Fri, Dec 25, 2015 at 10:49:56PM +0100, Mason wrote:
> >> From: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
> >>
> >> Move SMP setup to platsmp.c
> >> Specify the CPU type to the assembler, and only for smc.S
> >>
> >> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
> >> ---
> >
> > Applied, thanks.
> 
> Actually, I just noticed that the smc.c Makefile part isn't done as it
> should, so I'll drop this again.
> 

I've applied a different patch now, so we can at least get allmodconfig
to build again in linux-next:

commit 765ff22d54591a24fe81f6a451b953f43e076ced
Author: Arnd Bergmann <arnd@arndb.de>
Date:   Tue Dec 29 13:43:35 2015 +0100

    ARM: tango: pass ARM arch level for smc.S
    
    allmodconfig is broken by the addition of the -mcpu=cortex-a9
    flag for smc.S:
    
    arch/arm/mach-tango/smc.S:1:0: warning: switch -mcpu=cortex-a9 conflicts with -march=armv6k switch
    
    This sets the flag in the same way that all other platforms
    do, to fix the compile error.
    
    Signed-off-by: Arnd Bergmann <arnd@arndb.de>

diff --git a/arch/arm/mach-tango/Makefile b/arch/arm/mach-tango/Makefile
index 0d7e2b5..99422ea 100644
--- a/arch/arm/mach-tango/Makefile
+++ b/arch/arm/mach-tango/Makefile
@@ -1,2 +1,3 @@
-asflags-y += -mcpu=cortex-a9
+plus_sec := $(call as-instr,.arch_extension sec,+sec)
+AFLAGS_smc.o		:=-Wa,-march=armv7-a$(plus_sec)
 obj-y += setup.o smc.o

I think we still need the other half of the patch to fix non-SMP compilation.

	Arnd

[PATCH RFC] ARM: tango4: Fix build issues

[Mason]

On 31/12/2015 17:31, Olof Johansson wrote:
> On Thu, Dec 31, 2015 at 2:21 PM, Olof Johansson <olof@lixom.net> wrote:
>> On Fri, Dec 25, 2015 at 10:49:56PM +0100, Mason wrote:
>>> From: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
>>>
>>> Move SMP setup to platsmp.c
>>> Specify the CPU type to the assembler, and only for smc.S
>>>
>>> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
>>> ---
>>
>> Applied, thanks.
> 
> Actually, I just noticed that the smc.c Makefile part isn't done as it
> should, so I'll drop this again.

<grumble>

This is probably a bike-shed issue that I should let slide...

But can someone explain to me: in what circumstances

plus_sec := $(call as-instr,.arch_extension sec,+sec)
AFLAGS_smc.o :=-Wa,-march=armv7-a$(plus_sec)

is preferable over

AFLAGS_smc.o := -Wa,-mcpu=cortex-a9

in a machine-specific Makefile?


Arnd mentioned consistency with other machines; ISTM that
simplicity and obviousness are also desirable characteristics.

Regards.

[PATCH RFC] ARM: tango4: Fix build issues

[Mason]

On 31/12/2015 17:51, Arnd Bergmann wrote:
> On Thursday 31 December 2015 17:31:57 Olof Johansson wrote:
>> On Thu, Dec 31, 2015 at 2:21 PM, Olof Johansson <olof@lixom.net> wrote:
>>> On Fri, Dec 25, 2015 at 10:49:56PM +0100, Mason wrote:
>>>> From: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
>>>>
>>>> Move SMP setup to platsmp.c
>>>> Specify the CPU type to the assembler, and only for smc.S
>>>>
>>>> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
>>>> ---
>>>
>>> Applied, thanks.
>>
>> Actually, I just noticed that the smc.c Makefile part isn't done as it
>> should, so I'll drop this again.
> 
> I've applied a different patch now, so we can at least get allmodconfig
> to build again in linux-next:

<confused> Are you saying allmodconfig doesn't build?
The warning you mention is non-fatal.
Is there a setting to make that warning become a fatal error?
(Is -Werror enabled on the linux-next builds?)

> commit 765ff22d54591a24fe81f6a451b953f43e076ced
> Author: Arnd Bergmann <arnd@arndb.de>
> Date:   Tue Dec 29 13:43:35 2015 +0100
> 
>     ARM: tango: pass ARM arch level for smc.S
>     
>     allmodconfig is broken by the addition of the -mcpu=cortex-a9
>     flag for smc.S:
>
>     arch/arm/mach-tango/smc.S:1:0: warning: switch -mcpu=cortex-a9 conflicts with -march=armv6k switch
>     
>     This sets the flag in the same way that all other platforms
>     do, to fix the compile error.
>     
>     Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> 
> diff --git a/arch/arm/mach-tango/Makefile b/arch/arm/mach-tango/Makefile
> index 0d7e2b5..99422ea 100644
> --- a/arch/arm/mach-tango/Makefile
> +++ b/arch/arm/mach-tango/Makefile
> @@ -1,2 +1,3 @@
> -asflags-y += -mcpu=cortex-a9
> +plus_sec := $(call as-instr,.arch_extension sec,+sec)
> +AFLAGS_smc.o		:=-Wa,-march=armv7-a$(plus_sec)

Could you please amend the patch to drop the two extra tabs here?

Regards.

[PATCH RFC] ARM: tango4: Fix build issues

[Mason]

On 01/01/2016 11:52, Mason wrote:
> On 31/12/2015 17:31, Olof Johansson wrote:
>> On Thu, Dec 31, 2015 at 2:21 PM, Olof Johansson <olof@lixom.net> wrote:
>>> On Fri, Dec 25, 2015 at 10:49:56PM +0100, Mason wrote:
>>>> From: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
>>>>
>>>> Move SMP setup to platsmp.c
>>>> Specify the CPU type to the assembler, and only for smc.S
>>>>
>>>> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
>>>> ---
>>>
>>> Applied, thanks.
>>
>> Actually, I just noticed that the smc.c Makefile part isn't done as it
>> should, so I'll drop this again.
> 
> <grumble>
> 
> This is probably a bike-shed issue that I should let slide...
> 
> But can someone explain to me: in what circumstances
> 
> plus_sec := $(call as-instr,.arch_extension sec,+sec)
> AFLAGS_smc.o :=-Wa,-march=armv7-a$(plus_sec)
> 
> is preferable over
> 
> AFLAGS_smc.o := -Wa,-mcpu=cortex-a9
> 
> in a machine-specific Makefile?
> 
> 
> Arnd mentioned consistency with other machines; ISTM that
> simplicity and obviousness are also desirable characteristics.

For the sake of completeness, the +sec shenanigans are required
because of this 2010 binutils patch:

https://sourceware.org/ml/binutils/2010-09/msg00412.html

Specifying the CPU instead of an ISA (plus extensions) leaves
the instruction set selection up to the assembler, so the same
flag works for both old and new binutils.

Regards.