diff mbox

[12/16] ARM: imx: use generic API for enabling SCU

Message ID 1479099731-28108-13-git-send-email-pankaj.dubey@samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Pankaj Dubey Nov. 14, 2016, 5:02 a.m. UTC
Now as we have of_scu_enable which takes care of mapping
scu base from DT, lets use it.

At the same time this patch cleans up mach-imx platform files by
removing static mapping of SCU and dropping imx_scu_map_io function.

CC: Shawn Guo <shawnguo@kernel.org>
CC: Sascha Hauer <kernel@pengutronix.de>
Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
---
 arch/arm/mach-imx/common.h     |  5 -----
 arch/arm/mach-imx/mach-imx6q.c |  8 +-------
 arch/arm/mach-imx/platsmp.c    | 32 +++++---------------------------
 arch/arm/mach-imx/pm-imx6.c    |  3 ++-
 4 files changed, 8 insertions(+), 40 deletions(-)

Comments

Shawn Guo Nov. 14, 2016, 2:26 p.m. UTC | #1
On Mon, Nov 14, 2016 at 10:32:07AM +0530, Pankaj Dubey wrote:
> Now as we have of_scu_enable which takes care of mapping
> scu base from DT, lets use it.
> 
> At the same time this patch cleans up mach-imx platform files by
> removing static mapping of SCU and dropping imx_scu_map_io function.

I remember that the static mapping of SCU is necessary because SCU is
being accessed at very early boot stage where dynamic mapping hasn't
been set up.

> CC: Shawn Guo <shawnguo@kernel.org>
> CC: Sascha Hauer <kernel@pengutronix.de>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> ---
>  arch/arm/mach-imx/common.h     |  5 -----
>  arch/arm/mach-imx/mach-imx6q.c |  8 +-------
>  arch/arm/mach-imx/platsmp.c    | 32 +++++---------------------------
>  arch/arm/mach-imx/pm-imx6.c    |  3 ++-
>  4 files changed, 8 insertions(+), 40 deletions(-)

I tested it and saw that the booting of imx6q is broken like below.

[    0.000000] Booting Linux on physical CPU 0x0
[    0.000000] Linux version 4.9.0-rc5-00002-g3e5aac418b91 (r65073@dragon) (gcc version 4.9.3 20150413 (prerelease) (Linaro GCC 4.9-2015.04-1) ) #5 SMP Mon Nov 14 22:17:36 CST 2016
[    0.000000] CPU: ARMv7 Processor [412fc09a] revision 10 (ARMv7), cr=10c5387d
[    0.000000] CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing instruction cache
[    0.000000] OF: fdt:Machine model: Freescale i.MX6 Quad SABRE Smart Device Board
[    0.000000] earlycon: ec_imx21 at MMIO 0x02020000 (options '')
[    0.000000] bootconsole [ec_imx21] enabled
[    0.000000] cma: Reserved 16 MiB at 0x4f000000
[    0.000000] Memory policy: Data cache writealloc
[    0.000000] Unable to handle kernel NULL pointer dereference at virtual address 00000004
[    0.000000] pgd = c0004000
[    0.000000] [00000004] *pgd=00000000
[    0.000000] Internal error: Oops: 5 [#1] SMP ARM
[    0.000000] Modules linked in:
[    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 4.9.0-rc5-00002-g3e5aac418b91 #5
[    0.000000] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
[    0.000000] task: c0e086c0 task.stack: c0e00000
[    0.000000] PC is at scu_get_core_count+0xc/0x1c
[    0.000000] LR is at imx_smp_init_cpus+0x20/0x4c
[    0.000000] pc : [<c0d05438>]    lr : [<c0d0df2c>]    psr: 000000d3
[    0.000000] sp : c0e01f10  ip : c0e01f20  fp : c0e01f1c
[    0.000000] r10: c0bed850  r9 : c0e051c0  r8 : c0e75140
[    0.000000] r7 : c164c5f0  r6 : c0e09f08  r5 : c0d5d6fc  r4 : c0e08340
[    0.000000] r3 : c0e755f8  r2 : 00000000  r1 : c0120d3c  r0 : 00000000
[    0.000000] Flags: nzcv  IRQs off  FIQs off  Mode SVC_32  ISA ARM  Segment none
[    0.000000] Control: 10c5387d  Table: 1000404a  DAC: 00000051
[    0.000000] Process swapper (pid: 0, stack limit = 0xc0e00210)
[    0.000000] Stack: (0xc0e01f10 to 0xc0e02000)
[    0.000000] 1f00:                                     c0e01f34 c0e01f20 c0d0df2c c0d05438
[    0.000000] 1f20: c0e08340 c0d5d6fc c0e01f44 c0e01f38 c0d052b8 c0d0df18 c0e01fac c0e01f48
[    0.000000] 1f40: c0d0457c c0d052a4 ffffffff 10c5387d c0e050c0 1000406a 4fffffff efffeec0
[    0.000000] 1f60: c0e01f84 c0e01f70 c017b614 c017af04 c0bebc44 c0e01fa4 c0e01f9c c0e01f88
[    0.000000] 1f80: c01cf6dc c0e75294 c0e050d8 c0d5fa44 c0e050c0 1000406a 412fc09a 00000000
[    0.000000] 1fa0: c0e01ff4 c0e01fb0 c0d009b0 c0d03d24 00000000 00000000 00000000 00000000
[    0.000000] 1fc0: 00000000 c0d5fa48 00000000 c0e75294 c0e050d8 c0d5fa44 c0e0a070 1000406a
[    0.000000] 1fe0: 412fc09a 00000000 00000000 c0e01ff8 1000807c c0d0096c 00000000 00000000
[    0.000000] Backtrace: 
[    0.000000] [<c0d0542c>] (scu_get_core_count) from [<c0d0df2c>] (imx_smp_init_cpus+0x20/0x4c)
[    0.000000] [<c0d0df0c>] (imx_smp_init_cpus) from [<c0d052b8>] (smp_init_cpus+0x20/0x28)
[    0.000000]  r5:c0d5d6fc[    0.000000]  r4:c0e08340
[    0.000000] 
[    0.000000] [<c0d05298>] (smp_init_cpus) from [<c0d0457c>] (setup_arch+0x864/0xc50)
[    0.000000] [<c0d03d18>] (setup_arch) from [<c0d009b0>] (start_kernel+0x50/0x398)
[    0.000000]  r10:00000000[    0.000000]  r9:412fc09a
 r8:1000406a[    0.000000]  r7:c0e050c0
 r6:c0d5fa44[    0.000000]  r5:c0e050d8
[    0.000000]  r4:c0e75294[    0.000000] 
[    0.000000] [<c0d00960>] (start_kernel) from [<1000807c>] (0x1000807c)
[    0.000000]  r10:00000000[    0.000000]  r9:412fc09a
 r8:1000406a[    0.000000]  r7:c0e0a070
 r6:c0d5fa44[    0.000000]  r5:c0e050d8
[    0.000000]  r4:c0e75294[    0.000000] 
[    0.000000] Code: c0e75338 e1a0c00d e92dd800 e24cb004 (e5900004) 
[    0.000000] ---[ end trace 0000000000000000 ]---
[    0.000000] Kernel panic - not syncing: Attempted to kill the idle task!
[    0.000000] ---[ end Kernel panic - not syncing: Attempted to kill the idle task!
Pankaj Dubey Nov. 17, 2016, 4:29 a.m. UTC | #2
Hi Shawn,

On Monday 14 November 2016 07:56 PM, Shawn Guo wrote:
> On Mon, Nov 14, 2016 at 10:32:07AM +0530, Pankaj Dubey wrote:
>> Now as we have of_scu_enable which takes care of mapping
>> scu base from DT, lets use it.
>>
>> At the same time this patch cleans up mach-imx platform files by
>> removing static mapping of SCU and dropping imx_scu_map_io function.
> 
> I remember that the static mapping of SCU is necessary because SCU is
> being accessed at very early boot stage where dynamic mapping hasn't
> been set up.
> 
>> CC: Shawn Guo <shawnguo@kernel.org>
>> CC: Sascha Hauer <kernel@pengutronix.de>
>> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
>> ---
>>  arch/arm/mach-imx/common.h     |  5 -----
>>  arch/arm/mach-imx/mach-imx6q.c |  8 +-------
>>  arch/arm/mach-imx/platsmp.c    | 32 +++++---------------------------
>>  arch/arm/mach-imx/pm-imx6.c    |  3 ++-
>>  4 files changed, 8 insertions(+), 40 deletions(-)
> 
> I tested it and saw that the booting of imx6q is broken like below.

Thanks for testing and letting me know about this.

Currently only two platforms (IMX and ZYNQ) are using SCU address at
very early stage of boot, for configuring possible cpus via
set_cpu_possible().. rest platforms are either using DT method or they
handle this in smp_prepare_cpus.

Since I am not sure if all boards based on IMX has been moved to DT
based boot, if it has moved to completely DT based then we do not need
this set_cpu_possible in smp_init_cpus, it will be taken care via
"arm_dt_init_cpu_maps" and this will avoid need of early mapping of SCU
base as well.

If not then, currently I can't see any other way to handle this and in
that case I will drop-out IMX platform's patch of using generic SCU APIs
in v2 version of series.

Thanks,
Pankaj Dubey
diff mbox

Patch

diff --git a/arch/arm/mach-imx/common.h b/arch/arm/mach-imx/common.h
index c4436d9..9787d5f 100644
--- a/arch/arm/mach-imx/common.h
+++ b/arch/arm/mach-imx/common.h
@@ -87,11 +87,6 @@  u32 imx_get_cpu_arg(int cpu);
 void imx_set_cpu_arg(int cpu, u32 arg);
 #ifdef CONFIG_SMP
 void v7_secondary_startup(void);
-void imx_scu_map_io(void);
-void imx_smp_prepare(void);
-#else
-static inline void imx_scu_map_io(void) {}
-static inline void imx_smp_prepare(void) {}
 #endif
 void imx_src_init(void);
 void imx_gpc_pre_suspend(bool arm_power_off);
diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
index 45801b2..1c6cc9f 100644
--- a/arch/arm/mach-imx/mach-imx6q.c
+++ b/arch/arm/mach-imx/mach-imx6q.c
@@ -383,12 +383,6 @@  static void __init imx6q_init_late(void)
 	}
 }
 
-static void __init imx6q_map_io(void)
-{
-	debug_ll_io_init();
-	imx_scu_map_io();
-}
-
 static void __init imx6q_init_irq(void)
 {
 	imx_gpc_check_dt();
@@ -410,7 +404,7 @@  DT_MACHINE_START(IMX6Q, "Freescale i.MX6 Quad/DualLite (Device Tree)")
 	.l2c_aux_val 	= 0,
 	.l2c_aux_mask	= ~0,
 	.smp		= smp_ops(imx_smp_ops),
-	.map_io		= imx6q_map_io,
+	.map_io		= debug_ll_io_init,
 	.init_irq	= imx6q_init_irq,
 	.init_machine	= imx6q_init_machine,
 	.init_late      = imx6q_init_late,
diff --git a/arch/arm/mach-imx/platsmp.c b/arch/arm/mach-imx/platsmp.c
index 711dbbd..c032369 100644
--- a/arch/arm/mach-imx/platsmp.c
+++ b/arch/arm/mach-imx/platsmp.c
@@ -26,26 +26,6 @@ 
 u32 g_diag_reg;
 static void __iomem *scu_base;
 
-static struct map_desc scu_io_desc __initdata = {
-	/* .virtual and .pfn are run-time assigned */
-	.length		= SZ_4K,
-	.type		= MT_DEVICE,
-};
-
-void __init imx_scu_map_io(void)
-{
-	unsigned long base;
-
-	/* Get SCU base */
-	asm("mrc p15, 4, %0, c15, c0, 0" : "=r" (base));
-
-	scu_io_desc.virtual = IMX_IO_P2V(base);
-	scu_io_desc.pfn = __phys_to_pfn(base);
-	iotable_init(&scu_io_desc, 1);
-
-	scu_base = IMX_IO_ADDRESS(base);
-}
-
 static int imx_boot_secondary(unsigned int cpu, struct task_struct *idle)
 {
 	imx_set_cpu_jump(cpu, v7_secondary_startup);
@@ -61,20 +41,18 @@  static void __init imx_smp_init_cpus(void)
 {
 	int i, ncores;
 
-	ncores = scu_get_core_count(scu_base);
+	if (!IS_ERR(scu_base))
+		ncores = scu_get_core_count(scu_base);
 
 	for (i = ncores; i < NR_CPUS; i++)
 		set_cpu_possible(i, false);
 }
 
-void imx_smp_prepare(void)
-{
-	scu_enable(scu_base);
-}
-
 static void __init imx_smp_prepare_cpus(unsigned int max_cpus)
 {
-	imx_smp_prepare();
+	scu_base = of_scu_get_base();
+	if (!IS_ERR(scu_base))
+		scu_enable(scu_base);
 
 	/*
 	 * The diagnostic register holds the errata bits.  Mostly bootloader
diff --git a/arch/arm/mach-imx/pm-imx6.c b/arch/arm/mach-imx/pm-imx6.c
index 1515e49..859aacb 100644
--- a/arch/arm/mach-imx/pm-imx6.c
+++ b/arch/arm/mach-imx/pm-imx6.c
@@ -26,6 +26,7 @@ 
 #include <asm/fncpy.h>
 #include <asm/proc-fns.h>
 #include <asm/suspend.h>
+#include <asm/smp_scu.h>
 #include <asm/tlb.h>
 
 #include "common.h"
@@ -393,7 +394,7 @@  static int imx6q_pm_enter(suspend_state_t state)
 		/* Zzz ... */
 		cpu_suspend(0, imx6q_suspend_finish);
 		if (cpu_is_imx6q() || cpu_is_imx6dl())
-			imx_smp_prepare();
+			of_scu_enable();
 		imx_anatop_post_resume();
 		imx_gpc_post_resume();
 		imx6_enable_rbc(false);