Skip to contentSkip to conversation

Samsung fixes-1 for v4.0

GIT PULL Kukjin Kim 3 participants 4 messages Mar 2, 2015 – Mar 4, 2015

Subsystems (5)
Arm (32-bit) Arm sub-architectures Arm Samsung Exynos CPU idle time management framework +1 more subsystems
lore
Share & tools
lore

Kukjin Kim

author maintainer
#0 lore raw

The following changes since commit c517d838eb7d07bbe9507871fab3931deccff539:

Linux 4.0-rc1 (2015-02-22 18:21:14 -0800)

are available in the git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/kgene/linux-samsung.git
tags/samsung-fixes-1

for you to fetch changes up to 8a68de64e984cfc49ecc22ac18155a7fe6dbbfaf:

ARM: EXYNOS: fix exynos randconfig build error (2015-03-03 03:37:57 +0900)

----------------------------------------------------------------
Samsung fixes for v4.0

- fix exynos randconfig build error with SMP=n
- fix wrong hwirq of RTC interrupt for exynos3250
- fix usage of LDREX and STREX after disabling cache coherency

----------------------------------------------------------------
Chanwoo Choi (1):
ARM: EXYNOS: Fix wrong hwirq of RTC interrupt for Exynos3250 SoC

Krzysztof Kozlowski (1):
ARM: EXYNOS: Don't use LDREX and STREX after disabling cache coherency

Russell King (1):
ARM: EXYNOS: fix exynos randconfig build error

 arch/arm/mach-exynos/Kconfig   | 1 +
 arch/arm/mach-exynos/platsmp.c | 3 +--
 arch/arm/mach-exynos/suspend.c | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)
#1 lore raw

Hi,

Quoted 24 lines · Kukjin Kim

The following changes since commit c517d838eb7d07bbe9507871fab3931deccff539:

Linux 4.0-rc1 (2015-02-22 18:21:14 -0800)

are available in the git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/kgene/linux-samsung.git
tags/samsung-fixes-1

for you to fetch changes up to 8a68de64e984cfc49ecc22ac18155a7fe6dbbfaf:

ARM: EXYNOS: fix exynos randconfig build error (2015-03-03 03:37:57 +0900)

----------------------------------------------------------------
Samsung fixes for v4.0

- fix exynos randconfig build error with SMP=n
- fix wrong hwirq of RTC interrupt for exynos3250
- fix usage of LDREX and STREX after disabling cache coherency

----------------------------------------------------------------
Chanwoo Choi (1):
ARM: EXYNOS: Fix wrong hwirq of RTC interrupt for Exynos3250 SoC

Quoted 5 lines

Krzysztof Kozlowski (1):
ARM: EXYNOS: Don't use LDREX and STREX after disabling cache coherency

Russell King (1):
ARM: EXYNOS: fix exynos randconfig build error

Has anyone actually tested this patch with SMP=n && PM_SLEEP=y or SMP=n && CONFIG_ARM_EXYNOS_CPUIDLE=y?

The patch is just insufficient as it prevents build of arch/arm/mach-exynos/pm.o and arch/arm/mach-exynos/sleep.o for SMP=n. Both files are needed for PM_SLEEP=y code that resides in arch/arm/mach-exynos/suspend.c:

LD init/built-in.o
arch/arm/mach-exynos/built-in.o: In function `exynos_save_cp15':
/home/bzolnier/sam/linux-sprc/arch/arm/mach-exynos/firmware.c:36: undefined reference to exynos_cpu_resume_ns' /home/bzolnier/sam/linux-sprc/arch/arm/mach-exynos/firmware.c:36: undefined reference to cp15_save_power' /home/bzolnier/sam/linux-sprc/arch/arm/mach-exynos/firmware.c:36: undefined reference to cp15_save_diag' arch/arm/mach-exynos/built-in.o: In function exynos5420_pm_resume':
/home/bzolnier/sam/linux-sprc/arch/arm/mach-exynos/suspend.c:435: undefined reference to exynos_pm_central_resume' arch/arm/mach-exynos/built-in.o: In function __virt_to_phys':
/home/bzolnier/sam/linux-sprc/./arch/arm/include/asm/memory.h:228: undefined reference to exynos_cpu_resume' /home/bzolnier/sam/linux-sprc/./arch/arm/include/asm/memory.h:228: undefined reference to exynos_cpu_resume' arch/arm/mach-exynos/built-in.o: In function `exynos3250_pm_resume':
/home/bzolnier/sam/linux-sprc/arch/arm/mach-exynos/suspend.c:395: undefined reference to exynos_pm_central_resume' /home/bzolnier/sam/linux-sprc/arch/arm/mach-exynos/suspend.c:405: undefined reference to exynos_cpu_restore_register' arch/arm/mach-exynos/built-in.o: In function `exynos_pm_resume':
/home/bzolnier/sam/linux-sprc/arch/arm/mach-exynos/suspend.c:365: undefined reference to exynos_pm_central_resume' /home/bzolnier/sam/linux-sprc/arch/arm/mach-exynos/suspend.c:382: undefined reference to exynos_cpu_restore_register' arch/arm/mach-exynos/built-in.o: In function `exynos_pm_prepare':
/home/bzolnier/sam/linux-sprc/arch/arm/mach-exynos/suspend.c:253: undefined reference to exynos_cpu_resume' arch/arm/mach-exynos/built-in.o: In function exynos_pm_suspend':
/home/bzolnier/sam/linux-sprc/arch/arm/mach-exynos/suspend.c:322: undefined reference to exynos_pm_central_suspend' /home/bzolnier/sam/linux-sprc/arch/arm/mach-exynos/suspend.c:329: undefined reference to exynos_cpu_save_register' arch/arm/mach-exynos/built-in.o: In function `exynos5420_pm_suspend':
/home/bzolnier/sam/linux-sprc/arch/arm/mach-exynos/suspend.c:338: undefined reference to exynos_pm_central_suspend' arch/arm/mach-exynos/built-in.o: In function exynos_dt_machine_init':
/home/bzolnier/sam/linux-sprc/arch/arm/mach-exynos/exynos.c:284: undefined reference to cpuidle_coupled_exynos_data' arch/arm/mach-exynos/built-in.o:(.data+0x70): undefined reference to exynos_enter_aftr'
make: *** [vmlinux] Error 1

Moreover it still allows to select Exynos cpuidle support (CONFIG_ARM_EXYNOS_CPUIDLE=y) with SMP=n which results in:

LD init/built-in.o
arch/arm/mach-exynos/built-in.o: In function `exynos_dt_machine_init':
/home/bzolnier/sam/linux-sprc/arch/arm/mach-exynos/exynos.c:284: undefined reference to cpuidle_coupled_exynos_data' arch/arm/mach-exynos/built-in.o:(.data+0x70): undefined reference to exynos_enter_aftr'
make: *** [vmlinux] Error 1

The full solution for issue that "ARM: EXYNOS: fix exynos randconfig build error" patch attempted to fix has been posted by me on 4th of February (https://lkml.org/lkml/2015/2/4/521) and for completness is also included below. It fixes a v4.0-rc1 regression caused by a recent addition of cpuidle coupled support for Exynos4210 SoC. It makes only the new Exynos4210 coupled cpuidle support to be dependent on SMP. On UP cpuidle will use the old non-coupled code for Exynos4210 which is an expected behavior (as it was in v3.19). The patch has been tested on Exynos4210 Origen board with both SMP and UP kernels and in both cases it worked fine.

From: Bartlomiej Zolnierkiewicz <[email protected]>
Subject: [PATCH] ARM: EXYNOS: cpuidle: Fix build breakage on !SMP

The Exynos cpuidle driver has coupled cpuidle built-in so it cannot be built without SMP:

arch/arm/mach-exynos/pm.c: In function ?exynos_cpu0_enter_aftr?:
arch/arm/mach-exynos/pm.c:246:4: error: implicit declaration of function ?arch_send_wakeup_ipi_mask? [-Werror=implicit-function-declaration] arch/arm/mach-exynos/built-in.o: In function `exynos_pre_enter_aftr':
../arch/arm/mach-exynos/pm.c:300: undefined reference to `cpu_boot_reg_base'
arch/arm/mach-exynos/built-in.o: In function `exynos_cpu1_powerdown':
../arch/arm/mach-exynos/pm.c:282: undefined reference to `exynos_cpu_power_down'

Fix it by adding missing checks for SMP.

Reported-by Krzysztof Kozlowski <[email protected]>
Signed-off-by Bartlomiej Zolnierkiewicz <[email protected]>
---
 arch/arm/mach-exynos/exynos.c    |    2 +-
 arch/arm/mach-exynos/pm.c        |    2 ++
 drivers/cpuidle/cpuidle-exynos.c |    3 ++-
 3 files changed, 5 insertions(+), 2 deletions(-)
arch/arm/mach-exynos/exynos.c +1 -1
Index: b/arch/arm/mach-exynos/exynos.c
===================================================================
@@ -211,7 +211,7 @@ static void __init exynos_dt_machine_ini
 	if (!IS_ENABLED(CONFIG_SMP))
 		exynos_sysram_init();
 
-#ifdef CONFIG_ARM_EXYNOS_CPUIDLE
+#if defined(CONFIG_SMP) && defined(CONFIG_ARM_EXYNOS_CPUIDLE)
 	if (of_machine_is_compatible("samsung,exynos4210"))
 		exynos_cpuidle.dev.platform_data = &cpuidle_coupled_exynos_data;
 #endif
arch/arm/mach-exynos/pm.c +2 -0
Index: b/arch/arm/mach-exynos/pm.c
===================================================================
@@ -181,6 +181,7 @@ void exynos_enter_aftr(void)
 	cpu_pm_exit();
 }
 
+#if defined(CONFIG_SMP) && defined(CONFIG_ARM_EXYNOS_CPUIDLE)
 static atomic_t cpu1_wakeup = ATOMIC_INIT(0);
 
 static int exynos_cpu0_enter_aftr(void)

@@ -302,3 +303,4 @@ struct cpuidle_exynos_data cpuidle_coupl
 	.pre_enter_aftr		= exynos_pre_enter_aftr,
 	.post_enter_aftr		= exynos_post_enter_aftr,
 };
+#endif /* CONFIG_SMP && CONFIG_ARM_EXYNOS_CPUIDLE */
drivers/cpuidle/cpuidle-exynos.c +2 -1
Index: b/drivers/cpuidle/cpuidle-exynos.c
===================================================================
@@ -117,7 +117,8 @@ static int exynos_cpuidle_probe(struct p
 {
 	int ret;
 
-	if (of_machine_is_compatible("samsung,exynos4210")) {
+	if (IS_ENABLED(CONFIG_SMP) &&
+	    of_machine_is_compatible("samsung,exynos4210")) {
 		exynos_cpuidle_pdata = pdev->dev.platform_data;
 
 		ret = cpuidle_register(&exynos_coupled_idle_driver,
#2 lore raw
Quoted 31 lines · Bartlomiej Zolnierkiewicz

Hi,

Quoted 27 lines · Kukjin Kim

The following changes since commit c517d838eb7d07bbe9507871fab3931deccff539:

Linux 4.0-rc1 (2015-02-22 18:21:14 -0800)

are available in the git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/kgene/linux-samsung.git
tags/samsung-fixes-1

for you to fetch changes up to 8a68de64e984cfc49ecc22ac18155a7fe6dbbfaf:

ARM: EXYNOS: fix exynos randconfig build error (2015-03-03 03:37:57 +0900)

----------------------------------------------------------------
Samsung fixes for v4.0

- fix exynos randconfig build error with SMP=n
- fix wrong hwirq of RTC interrupt for exynos3250
- fix usage of LDREX and STREX after disabling cache coherency

----------------------------------------------------------------
Chanwoo Choi (1):
ARM: EXYNOS: Fix wrong hwirq of RTC interrupt for Exynos3250 SoC

Krzysztof Kozlowski (1):
ARM: EXYNOS: Don't use LDREX and STREX after disabling cache coherency

Quoted 7 lines
Quoted 2 lines

Russell King (1):
ARM: EXYNOS: fix exynos randconfig build error

Has anyone actually tested this patch with SMP=n && PM_SLEEP=y or SMP=n && CONFIG_ARM_EXYNOS_CPUIDLE=y?

The patch is just insufficient as it prevents build of

By insufficient here I mean compeltely bogus. I don't mean to be rude howeover since the wrong patch was picked please let me explain it in more technical details.

What the patch does is adding an SMP dependency to EXYNOS_CPU_SUSPEND option in order to "fix" SMP=n build:

arch/arm/mach-exynos/Kconfig +1 -0
@@ -131,6 +131,7 @@ config EXYNOS5420_MCPM
 
 config EXYNOS_CPU_SUSPEND
        bool
+       depends on SMP
        select ARM_CPU_SUSPEND
        default PM_SLEEP || ARM_EXYNOS_CPUIDLE

Please note that the EXYNOS_CPU_SUSPEND option itself will be selected only if PM_SLEEP=y or ARM_EXYNOS_CPUIDLE=y.

Now lets look at arch/arm/mach-exynos/Makefile:

...
obj-$(CONFIG_ARCH_EXYNOS) += exynos.o pmu.o exynos-smc.o firmware.o

obj-$(CONFIG_EXYNOS_CPU_SUSPEND) += pm.o sleep.o
obj-$(CONFIG_PM_SLEEP) += suspend.o
...

Code in suspend.c references code from from pm.c and sleep.S. It will fail to link for PM_SLEEP=y as seen below:

Quoted 32 lines

arch/arm/mach-exynos/pm.o and arch/arm/mach-exynos/sleep.o for SMP=n. Both files are needed for PM_SLEEP=y code that resides in arch/arm/mach-exynos/suspend.c:

LD init/built-in.o
arch/arm/mach-exynos/built-in.o: In function `exynos_save_cp15':
/home/bzolnier/sam/linux-sprc/arch/arm/mach-exynos/firmware.c:36: undefined reference to exynos_cpu_resume_ns' /home/bzolnier/sam/linux-sprc/arch/arm/mach-exynos/firmware.c:36: undefined reference to cp15_save_power' /home/bzolnier/sam/linux-sprc/arch/arm/mach-exynos/firmware.c:36: undefined reference to cp15_save_diag' arch/arm/mach-exynos/built-in.o: In function exynos5420_pm_resume':
/home/bzolnier/sam/linux-sprc/arch/arm/mach-exynos/suspend.c:435: undefined reference to exynos_pm_central_resume' arch/arm/mach-exynos/built-in.o: In function __virt_to_phys':
/home/bzolnier/sam/linux-sprc/./arch/arm/include/asm/memory.h:228: undefined reference to exynos_cpu_resume' /home/bzolnier/sam/linux-sprc/./arch/arm/include/asm/memory.h:228: undefined reference to exynos_cpu_resume' arch/arm/mach-exynos/built-in.o: In function `exynos3250_pm_resume':
/home/bzolnier/sam/linux-sprc/arch/arm/mach-exynos/suspend.c:395: undefined reference to exynos_pm_central_resume' /home/bzolnier/sam/linux-sprc/arch/arm/mach-exynos/suspend.c:405: undefined reference to exynos_cpu_restore_register' arch/arm/mach-exynos/built-in.o: In function `exynos_pm_resume':
/home/bzolnier/sam/linux-sprc/arch/arm/mach-exynos/suspend.c:365: undefined reference to exynos_pm_central_resume' /home/bzolnier/sam/linux-sprc/arch/arm/mach-exynos/suspend.c:382: undefined reference to exynos_cpu_restore_register' arch/arm/mach-exynos/built-in.o: In function `exynos_pm_prepare':
/home/bzolnier/sam/linux-sprc/arch/arm/mach-exynos/suspend.c:253: undefined reference to exynos_cpu_resume' arch/arm/mach-exynos/built-in.o: In function exynos_pm_suspend':
/home/bzolnier/sam/linux-sprc/arch/arm/mach-exynos/suspend.c:322: undefined reference to exynos_pm_central_suspend' /home/bzolnier/sam/linux-sprc/arch/arm/mach-exynos/suspend.c:329: undefined reference to exynos_cpu_save_register' arch/arm/mach-exynos/built-in.o: In function `exynos5420_pm_suspend':
/home/bzolnier/sam/linux-sprc/arch/arm/mach-exynos/suspend.c:338: undefined reference to exynos_pm_central_suspend' arch/arm/mach-exynos/built-in.o: In function exynos_dt_machine_init':
/home/bzolnier/sam/linux-sprc/arch/arm/mach-exynos/exynos.c:284: undefined reference to cpuidle_coupled_exynos_data' arch/arm/mach-exynos/built-in.o:(.data+0x70): undefined reference to exynos_enter_aftr'
make: *** [vmlinux] Error 1

Quoted 2 lines

Moreover it still allows to select Exynos cpuidle support (CONFIG_ARM_EXYNOS_CPUIDLE=y) with SMP=n which results in:

Code in exynos.c that is covered with CONFIG_ARM_EXYNOS_CPUIDLE ifdefs references code from pm.c for case when ARM_EXYNOS_CPUIDLE=y and will also fail to link:

Quoted 5 lines

LD init/built-in.o
arch/arm/mach-exynos/built-in.o: In function `exynos_dt_machine_init':
/home/bzolnier/sam/linux-sprc/arch/arm/mach-exynos/exynos.c:284: undefined reference to cpuidle_coupled_exynos_data' arch/arm/mach-exynos/built-in.o:(.data+0x70): undefined reference to exynos_enter_aftr'
make: *** [vmlinux] Error 1

So the SMP=n kernel won't link when PM_SLEEP=y or ARM_EXYNOS_CPUIDLE=y is selected. Now please take look at the config EXYNOS_CPU_SUSPEND option again and recall that it is selected only if PM_SLEEP=y or ARM_EXYNOS_CPUIDLE=y is set. Yes.. the patch doesn't actually fix anything...

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Quoted 79 lines

The full solution for issue that "ARM: EXYNOS: fix exynos randconfig build error" patch attempted to fix has been posted by me on 4th of February (https://lkml.org/lkml/2015/2/4/521) and for completness is also included below. It fixes a v4.0-rc1 regression caused by a recent addition of cpuidle coupled support for Exynos4210 SoC. It makes only the new Exynos4210 coupled cpuidle support to be dependent on SMP. On UP cpuidle will use the old non-coupled code for Exynos4210 which is an expected behavior (as it was in v3.19). The patch has been tested on Exynos4210 Origen board with both SMP and UP kernels and in both cases it worked fine.

From: Bartlomiej Zolnierkiewicz <[email protected]>
Subject: [PATCH] ARM: EXYNOS: cpuidle: Fix build breakage on !SMP

The Exynos cpuidle driver has coupled cpuidle built-in so it cannot be built without SMP:

arch/arm/mach-exynos/pm.c: In function ?exynos_cpu0_enter_aftr?:
arch/arm/mach-exynos/pm.c:246:4: error: implicit declaration of function ?arch_send_wakeup_ipi_mask? [-Werror=implicit-function-declaration] arch/arm/mach-exynos/built-in.o: In function `exynos_pre_enter_aftr':
../arch/arm/mach-exynos/pm.c:300: undefined reference to `cpu_boot_reg_base'
arch/arm/mach-exynos/built-in.o: In function `exynos_cpu1_powerdown':
../arch/arm/mach-exynos/pm.c:282: undefined reference to `exynos_cpu_power_down'

Fix it by adding missing checks for SMP.

Reported-by Krzysztof Kozlowski <[email protected]>
Signed-off-by Bartlomiej Zolnierkiewicz <[email protected]>
---
 arch/arm/mach-exynos/exynos.c    |    2 +-
 arch/arm/mach-exynos/pm.c        |    2 ++
 drivers/cpuidle/cpuidle-exynos.c |    3 ++-
 3 files changed, 5 insertions(+), 2 deletions(-)

Index: b/arch/arm/mach-exynos/exynos.c
===================================================================

arch/arm/mach-exynos/exynos.c +1 -1
@@ -211,7 +211,7 @@ static void __init exynos_dt_machine_ini
 	if (!IS_ENABLED(CONFIG_SMP))
 		exynos_sysram_init();
 
-#ifdef CONFIG_ARM_EXYNOS_CPUIDLE
+#if defined(CONFIG_SMP) && defined(CONFIG_ARM_EXYNOS_CPUIDLE)
 	if (of_machine_is_compatible("samsung,exynos4210"))
 		exynos_cpuidle.dev.platform_data = &cpuidle_coupled_exynos_data;
 #endif

Index: b/arch/arm/mach-exynos/pm.c
===================================================================

arch/arm/mach-exynos/pm.c +2 -0
@@ -181,6 +181,7 @@ void exynos_enter_aftr(void)
 	cpu_pm_exit();
 }
 
+#if defined(CONFIG_SMP) && defined(CONFIG_ARM_EXYNOS_CPUIDLE)
 static atomic_t cpu1_wakeup = ATOMIC_INIT(0);
 
 static int exynos_cpu0_enter_aftr(void)

@@ -302,3 +303,4 @@ struct cpuidle_exynos_data cpuidle_coupl
 	.pre_enter_aftr		= exynos_pre_enter_aftr,
 	.post_enter_aftr		= exynos_post_enter_aftr,
 };
+#endif /* CONFIG_SMP && CONFIG_ARM_EXYNOS_CPUIDLE */

Index: b/drivers/cpuidle/cpuidle-exynos.c
===================================================================

drivers/cpuidle/cpuidle-exynos.c +2 -1
@@ -117,7 +117,8 @@ static int exynos_cpuidle_probe(struct p
 {
 	int ret;
 
-	if (of_machine_is_compatible("samsung,exynos4210")) {
+	if (IS_ENABLED(CONFIG_SMP) &&
+	    of_machine_is_compatible("samsung,exynos4210")) {
 		exynos_cpuidle_pdata = pdev->dev.platform_data;
 
 		ret = cpuidle_register(&exynos_coupled_idle_driver,
#3 lore raw
Quoted 18 lines · On Tuesday 03 March 2015 17:23:52 Bartlomiej Zolnierkiewicz wrote:
Quoted 17 lines · Bartlomiej Zolnierkiewicz
Quoted 16 lines · Kukjin Kim

The following changes since commit c517d838eb7d07bbe9507871fab3931deccff539:

Linux 4.0-rc1 (2015-02-22 18:21:14 -0800)

are available in the git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/kgene/linux-samsung.git
tags/samsung-fixes-1

for you to fetch changes up to 8a68de64e984cfc49ecc22ac18155a7fe6dbbfaf:

ARM: EXYNOS: fix exynos randconfig build error (2015-03-03 03:37:57 +0900)

----------------------------------------------------------------
Samsung fixes for v4.0

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

- fix exynos randconfig build error with SMP=n
- fix wrong hwirq of RTC interrupt for exynos3250
- fix usage of LDREX and STREX after disabling cache coherency

----------------------------------------------------------------
Chanwoo Choi (1):
ARM: EXYNOS: Fix wrong hwirq of RTC interrupt for Exynos3250 SoC

Question: why doesn't the number come from DT?

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

Krzysztof Kozlowski (1):
ARM: EXYNOS: Don't use LDREX and STREX after disabling cache coherency

I suppose the right long-term fix would be to kill off those soc_is_exynos* macros as well, and derive the right set of SMP operations from the machine descriptor that would then use the top-level compatible string to match the machine.

Quoted 11 lines
Quoted 7 lines
Quoted 2 lines

Russell King (1):
ARM: EXYNOS: fix exynos randconfig build error

Has anyone actually tested this patch with SMP=n && PM_SLEEP=y or SMP=n && CONFIG_ARM_EXYNOS_CPUIDLE=y?

The patch is just insufficient as it prevents build of

By insufficient here I mean compeltely bogus. I don't mean to be rude howeover since the wrong patch was picked please let me explain it in more technical details.

Thanks for the detailed analysis. I noticed the same thing earlier when it showed up in linux-next. I've done a partial merge now and took only the other two patches, which seem necessary.

Arnd