ARM: uniphier: only select TWD for SMP

PATCH New Arnd Bergmann 2 participants 5 messages May 18, 2015 – May 20, 2015

Subsystems (3)

Arm (32-bit) Arm sub-architectures Arm/uniphier architecture

lore

Share & tools

lore

Arnd Bergmann

author

2015-05-18 15:55 UTC #0 lore raw

PATCH 1/1 ARM: uniphier: only select TWD for SMP

This makes uniphier behave like all the other platforms that support TWD, and only select this driver when SMP is enabled. Without this, we get a compile error on UP builds:

arch/arm/kernel/smp_twd.c: In function 'twd_local_timer_of_register':
arch/arm/kernel/smp_twd.c:391:20: error: 'setup_max_cpus' undeclared (first use in this function)

Signed-off-by Arnd Bergmann <[email protected]>

---
I'd like to apply this directly to the next/soc branch

arch/arm/mach-uniphier/Kconfig +1 -1

index a017b1dd9c78..b640458fd757 100644
@@ -5,7 +5,7 @@ config ARCH_UNIPHIER
 	select ARM_GLOBAL_TIMER
 	select ARM_GIC
 	select HAVE_ARM_SCU
-	select HAVE_ARM_TWD
+	select HAVE_ARM_TWD if SMP
 	help
 	  Support for UniPhier SoC family developed by Socionext Inc.
 	  (formerly, System LSI Business Division of Panasonic Corporation)

Masahiro Yamada

2015-05-18 17:18 UTC #1 lore raw

PATCH 1/1 ARM: uniphier: only select TWD for SMP

Hi Arnd,

2015-05-19 0:55 GMT+09:00 Arnd Bergmann <[email protected]>:

Quoted 16 lines

This makes uniphier behave like all the other platforms that support TWD, and only select this driver when SMP is enabled. Without this, we get a compile error on UP builds:
arch/arm/kernel/smp_twd.c: In function 'twd_local_timer_of_register':
arch/arm/kernel/smp_twd.c:391:20: error: 'setup_max_cpus' undeclared (first use in this function)
Signed-off-by Arnd Bergmann <[email protected]>
---
I'd like to apply this directly to the next/soc branch
arch/arm/mach-uniphier/Kconfig +0 -0
index a017b1dd9c78..b640458fd757 100644

arch/arm/mach-uniphier/Kconfig config ARCH_UNIPHIER · line 11

Quoted 8 lines

arch/arm/mach-uniphier/Kconfig +1 -1

@@ -5,7 +5,7 @@ config ARCH_UNIPHIER
        select ARM_GLOBAL_TIMER
        select ARM_GIC
        select HAVE_ARM_SCU
-       select HAVE_ARM_TWD
+       select HAVE_ARM_TWD if SMP
        help
          Support for UniPhier SoC family developed by Socionext Inc.
          (formerly, System LSI Business Division of Panasonic Corporation)

I am not familiar with smp_twd.c, but I think
the local timer and watchdog still exist in the ARM mpcore even if it is a uni-processor implementation.

That is why I simply selected HAVE_ARM_TWD.
Am I doing something wrong?

Is it reasonable to have something depends on HAVE_ARM_TWD?

config SMP_ARM_TWD

      depends on SMP && HAVE_ARM_TWD

obj-$(SMP_ARM_TWD) += smp_twd.o

I am not sure...

Signature · 2 lines

Best Regards
Masahiro Yamada

Arnd Bergmann

author

2015-05-18 20:26 UTC #2 lore raw

PATCH 1/1 ARM: uniphier: only select TWD for SMP

Quoted 34 lines · On Tuesday 19 May 2015 02:18:53 Masahiro Yamada wrote:

Hi Arnd,
2015-05-19 0:55 GMT+09:00 Arnd Bergmann <[email protected]>:
Quoted 16 lines
This makes uniphier behave like all the other platforms that support TWD, and only select this driver when SMP is enabled. Without this, we get a compile error on UP builds:
arch/arm/kernel/smp_twd.c: In function 'twd_local_timer_of_register':
arch/arm/kernel/smp_twd.c:391:20: error: 'setup_max_cpus' undeclared (first use in this function)
Signed-off-by Arnd Bergmann <[email protected]>
---
I'd like to apply this directly to the next/soc branch
arch/arm/mach-uniphier/Kconfig +0 -0
index a017b1dd9c78..b640458fd757 100644
arch/arm/mach-uniphier/Kconfig config ARCH_UNIPHIER · line 11
Quoted 8 lines
arch/arm/mach-uniphier/Kconfig +1 -1
@@ -5,7 +5,7 @@ config ARCH_UNIPHIER
        select ARM_GLOBAL_TIMER
        select ARM_GIC
        select HAVE_ARM_SCU
-       select HAVE_ARM_TWD
+       select HAVE_ARM_TWD if SMP
        help
          Support for UniPhier SoC family developed by Socionext Inc.
          (formerly, System LSI Business Division of Panasonic Corporation)
I am not familiar with smp_twd.c, but I think
the local timer and watchdog still exist in the ARM mpcore even if it is a uni-processor implementation.

Quoted 2 lines

That is why I simply selected HAVE_ARM_TWD.
Am I doing something wrong?

I was wondering about this too. Everybody else has the 'if SMP' dependency here, and if I leave it out, I get the build error. We could probably fix that build error easily, but I don't know what the exact reason is we can't have use the code when there is only one CPU.

This was introduced as part of 904464b91eca8 ("ARM: 7655/1: smp_twd: make twd_local_timer_of_register() no-op for nosmp"), before that, we would just try to use the driver, but fail in one of these calls:

        twd_evt = alloc_percpu(struct clock_event_device);
        err = request_percpu_irq(twd_ppi, twd_handler, "twd", twd_evt);
        err = register_cpu_notifier(&twd_timer_cpu_nb);

which in turn causes a run-time warning. If we could fix that code to just work on non-SMP, we can probably use that driver on all machines that have the hardware.

Quoted 8 lines

Is it reasonable to have something depends on HAVE_ARM_TWD?
config SMP_ARM_TWD
      depends on SMP && HAVE_ARM_TWD

Quoted 5 lines

obj-$(SMP_ARM_TWD) += smp_twd.o
I am not sure...

This would probably work, but I'd like to understand the problem more thoroughly before we change the behavior.

[adding Russell and Will to Cc, maybe they remember better why we do things the way we do here]

Arnd

Masahiro Yamada

2015-05-19 00:52 UTC #3 lore raw

PATCH 1/1 ARM: uniphier: only select TWD for SMP

Hi Arnd,

2015-05-19 5:26 GMT+09:00 Arnd Bergmann <[email protected]>:

Quoted 69 lines · On Tuesday 19 May 2015 02:18:53 Masahiro Yamada wrote:

Quoted 34 lines
Hi Arnd,
2015-05-19 0:55 GMT+09:00 Arnd Bergmann <[email protected]>:
Quoted 16 lines
This makes uniphier behave like all the other platforms that support TWD, and only select this driver when SMP is enabled. Without this, we get a compile error on UP builds:
arch/arm/kernel/smp_twd.c: In function 'twd_local_timer_of_register':
arch/arm/kernel/smp_twd.c:391:20: error: 'setup_max_cpus' undeclared (first use in this function)
Signed-off-by Arnd Bergmann <[email protected]>
---
I'd like to apply this directly to the next/soc branch
arch/arm/mach-uniphier/Kconfig +0 -0
index a017b1dd9c78..b640458fd757 100644
arch/arm/mach-uniphier/Kconfig config ARCH_UNIPHIER · line 11
Quoted 8 lines
arch/arm/mach-uniphier/Kconfig +1 -1
@@ -5,7 +5,7 @@ config ARCH_UNIPHIER
        select ARM_GLOBAL_TIMER
        select ARM_GIC
        select HAVE_ARM_SCU
-       select HAVE_ARM_TWD
+       select HAVE_ARM_TWD if SMP
        help
          Support for UniPhier SoC family developed by Socionext Inc.
          (formerly, System LSI Business Division of Panasonic Corporation)
I am not familiar with smp_twd.c, but I think
the local timer and watchdog still exist in the ARM mpcore even if it is a uni-processor implementation.
Quoted 2 lines
That is why I simply selected HAVE_ARM_TWD.
Am I doing something wrong?
I was wondering about this too. Everybody else has the 'if SMP' dependency here, and if I leave it out, I get the build error. We could probably fix that build error easily, but I don't know what the exact reason is we can't have use the code when there is only one CPU.
This was introduced as part of 904464b91eca8 ("ARM: 7655/1: smp_twd: make twd_local_timer_of_register() no-op for nosmp"), before that, we would just try to use the driver, but fail in one of these calls:
        twd_evt = alloc_percpu(struct clock_event_device);
        err = request_percpu_irq(twd_ppi, twd_handler, "twd", twd_evt);
        err = register_cpu_notifier(&twd_timer_cpu_nb);
which in turn causes a run-time warning. If we could fix that code to just work on non-SMP, we can probably use that driver on all machines that have the hardware.
Quoted 8 lines
Is it reasonable to have something depends on HAVE_ARM_TWD?
config SMP_ARM_TWD
      depends on SMP && HAVE_ARM_TWD
Quoted 5 lines
obj-$(SMP_ARM_TWD) += smp_twd.o
I am not sure...

Quoted 7 lines

This would probably work, but I'd like to understand the problem more thoroughly before we change the behavior.
[adding Russell and Will to Cc, maybe they remember better why we do things the way we do here]
Arnd

OK, then. Anyway, we should fix the build error asap. Let's go with "if SMP" for now.

Acked-by Masahiro Yamada <[email protected]>

Signature · 2 lines

Best Regards
Masahiro Yamada

Arnd Bergmann

author

2015-05-20 15:21 UTC #4 lore raw

PATCH 1/1 ARM: uniphier: only select TWD for SMP

Quoted 7 lines · On Tuesday 19 May 2015 09:52:13 Masahiro Yamada wrote:

OK, then. Anyway, we should fix the build error asap. Let's go with "if SMP" for now.
Acked-by Masahiro Yamada <[email protected]>

Applied now, thanks!

Arnd