From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alim Akhtar Subject: Re: [3/4] ARM: EXYNOS: Remove static mapping of SCU SFR Date: Mon, 07 Nov 2016 10:19:53 +0530 Message-ID: <582007F1.5050803@samsung.com> References: <1478230764-13748-4-git-send-email-pankaj.dubey@samsung.com> <581C8C8F.5080100@samsung.com> <581FE87E.2040607@samsung.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Return-path: Received: from mailout4.samsung.com ([203.254.224.34]:53387 "EHLO mailout4.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752096AbcKGEvo (ORCPT ); Sun, 6 Nov 2016 23:51:44 -0500 Received: from epcas2p2.samsung.com (unknown [182.195.41.54]) by mailout4.samsung.com (Oracle Communications Messaging Server 7.0.5.31.0 64bit (built May 5 2014)) with ESMTP id <0OG9022P29I6AE00@mailout4.samsung.com> for linux-samsung-soc@vger.kernel.org; Mon, 07 Nov 2016 13:51:42 +0900 (KST) In-reply-to: <581FE87E.2040607@samsung.com> Sender: linux-samsung-soc-owner@vger.kernel.org List-Id: linux-samsung-soc@vger.kernel.org To: "pankaj.dubey" , linux-samsung-soc@vger.kernel.org, linux-arm-kernel@lists.infradead.org Cc: krzk@kernel.org, kgene@kernel.org, thomas.ab@samsung.com Hi Pankaj, On 11/07/2016 08:05 AM, pankaj.dubey wrote: > Hi Alim, > > On Friday 04 November 2016 06:56 PM, Alim Akhtar wrote: >> Hi Pankaj, >> >> On 11/04/2016 09:09 AM, Pankaj Dubey wrote: >>> Lets remove static mapping of SCU SFR mainly used in CORTEX-A9 SoC >>> based boards. >>> Instead use mapping from device tree node of SCU. >>> >>> Signed-off-by: Pankaj Dubey >>> --- >>> arch/arm/mach-exynos/exynos.c | 22 >>> ---------------------- >>> arch/arm/mach-exynos/include/mach/map.h | 2 -- >>> arch/arm/mach-exynos/platsmp.c | 18 +++++++++++------- >>> arch/arm/mach-exynos/pm.c | 14 +++++++++++--- >>> arch/arm/mach-exynos/suspend.c | 15 +++++++++++---- >>> arch/arm/plat-samsung/include/plat/map-s5p.h | 4 ---- >>> 6 files changed, 33 insertions(+), 42 deletions(-) >>> >>> diff --git a/arch/arm/mach-exynos/exynos.c >>> b/arch/arm/mach-exynos/exynos.c >>> index 757fc11..fa08ef9 100644 >>> --- a/arch/arm/mach-exynos/exynos.c >>> +++ b/arch/arm/mach-exynos/exynos.c >>> @@ -28,15 +28,6 @@ >>> >>> #include "common.h" >>> >>> -static struct map_desc exynos4_iodesc[] __initdata = { >>> - { >>> - .virtual = (unsigned long)S5P_VA_COREPERI_BASE, >>> - .pfn = __phys_to_pfn(EXYNOS4_PA_COREPERI), >>> - .length = SZ_8K, >>> - .type = MT_DEVICE, >>> - }, >>> -}; >>> - >>> static struct platform_device exynos_cpuidle = { >>> .name = "exynos_cpuidle", >>> #ifdef CONFIG_ARM_EXYNOS_CPUIDLE >>> @@ -99,17 +90,6 @@ static int __init exynos_fdt_map_chipid(unsigned >>> long node, const char *uname, >>> return 1; >>> } >>> >>> -/* >>> - * exynos_map_io >>> - * >>> - * register the standard cpu IO areas >>> - */ >>> -static void __init exynos_map_io(void) >>> -{ >>> - if (soc_is_exynos4()) >>> - iotable_init(exynos4_iodesc, ARRAY_SIZE(exynos4_iodesc)); >>> -} >>> - >>> static void __init exynos_init_io(void) >>> { >>> debug_ll_io_init(); >>> @@ -118,8 +98,6 @@ static void __init exynos_init_io(void) >>> >>> /* detect cpu id and rev. */ >>> s5p_init_cpu(S5P_VA_CHIPID); >>> - >>> - exynos_map_io(); >>> } >>> >>> /* >>> diff --git a/arch/arm/mach-exynos/include/mach/map.h >>> b/arch/arm/mach-exynos/include/mach/map.h >>> index 5fb0040..0eef407 100644 >>> --- a/arch/arm/mach-exynos/include/mach/map.h >>> +++ b/arch/arm/mach-exynos/include/mach/map.h >>> @@ -18,6 +18,4 @@ >>> >>> #define EXYNOS_PA_CHIPID 0x10000000 >>> >>> -#define EXYNOS4_PA_COREPERI 0x10500000 >>> - >>> #endif /* __ASM_ARCH_MAP_H */ >>> diff --git a/arch/arm/mach-exynos/platsmp.c >>> b/arch/arm/mach-exynos/platsmp.c >>> index a5d6841..553d0d9 100644 >>> --- a/arch/arm/mach-exynos/platsmp.c >>> +++ b/arch/arm/mach-exynos/platsmp.c >>> @@ -224,11 +224,6 @@ static void write_pen_release(int val) >>> sync_cache_w(&pen_release); >>> } >>> >>> -static void __iomem *scu_base_addr(void) >>> -{ >>> - return (void __iomem *)(S5P_VA_SCU); >>> -} >>> - >>> static DEFINE_SPINLOCK(boot_lock); >>> >>> static void exynos_secondary_init(unsigned int cpu) >>> @@ -387,14 +382,23 @@ fail: >>> >>> static void __init exynos_smp_prepare_cpus(unsigned int max_cpus) >>> { >>> + struct device_node *np; >>> + void __iomem *scu_base; >>> int i; >>> >>> exynos_sysram_init(); >>> >>> exynos_set_delayed_reset_assertion(true); >>> >>> - if (read_cpuid_part() == ARM_CPU_PART_CORTEX_A9) >>> - scu_enable(scu_base_addr()); >>> + if (read_cpuid_part() == ARM_CPU_PART_CORTEX_A9) { >>> + np = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-scu"); >> >> what if of_find_compatible_node() fails? May be add a error check for >> the same? > > Thanks for review. > > You are right of_find_compatible_node() is bound to fail, but only in > case supplied compatible is missing in DT. In our case this piece of > code will execute only for Cortex-A9 based SoC (which in case of Exynos > SoC is applicable only for Exynos4 series) and we will for sure > providing "arm,cortex-a9-scu" in DT, so there is no chance of failure. > So I feel extra check on "np" for NULL will add no benefit here. > Well I am not entirely convenience here, I still feel it better to have those check, lets not assume anything about future, but when I see of_find_compatible_node() uses elsewhere in kernel, both kind of uses are there (with/without error check). So, I leave it to you and maintainer to take a call on this, otherwise this patch looks good. Reviewed-by: Alim Akhtar > > Thanks, > Pankaj Dubey > > From mboxrd@z Thu Jan 1 00:00:00 1970 From: alim.akhtar@samsung.com (Alim Akhtar) Date: Mon, 07 Nov 2016 10:19:53 +0530 Subject: [3/4] ARM: EXYNOS: Remove static mapping of SCU SFR In-Reply-To: <581FE87E.2040607@samsung.com> References: <1478230764-13748-4-git-send-email-pankaj.dubey@samsung.com> <581C8C8F.5080100@samsung.com> <581FE87E.2040607@samsung.com> Message-ID: <582007F1.5050803@samsung.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Pankaj, On 11/07/2016 08:05 AM, pankaj.dubey wrote: > Hi Alim, > > On Friday 04 November 2016 06:56 PM, Alim Akhtar wrote: >> Hi Pankaj, >> >> On 11/04/2016 09:09 AM, Pankaj Dubey wrote: >>> Lets remove static mapping of SCU SFR mainly used in CORTEX-A9 SoC >>> based boards. >>> Instead use mapping from device tree node of SCU. >>> >>> Signed-off-by: Pankaj Dubey >>> --- >>> arch/arm/mach-exynos/exynos.c | 22 >>> ---------------------- >>> arch/arm/mach-exynos/include/mach/map.h | 2 -- >>> arch/arm/mach-exynos/platsmp.c | 18 +++++++++++------- >>> arch/arm/mach-exynos/pm.c | 14 +++++++++++--- >>> arch/arm/mach-exynos/suspend.c | 15 +++++++++++---- >>> arch/arm/plat-samsung/include/plat/map-s5p.h | 4 ---- >>> 6 files changed, 33 insertions(+), 42 deletions(-) >>> >>> diff --git a/arch/arm/mach-exynos/exynos.c >>> b/arch/arm/mach-exynos/exynos.c >>> index 757fc11..fa08ef9 100644 >>> --- a/arch/arm/mach-exynos/exynos.c >>> +++ b/arch/arm/mach-exynos/exynos.c >>> @@ -28,15 +28,6 @@ >>> >>> #include "common.h" >>> >>> -static struct map_desc exynos4_iodesc[] __initdata = { >>> - { >>> - .virtual = (unsigned long)S5P_VA_COREPERI_BASE, >>> - .pfn = __phys_to_pfn(EXYNOS4_PA_COREPERI), >>> - .length = SZ_8K, >>> - .type = MT_DEVICE, >>> - }, >>> -}; >>> - >>> static struct platform_device exynos_cpuidle = { >>> .name = "exynos_cpuidle", >>> #ifdef CONFIG_ARM_EXYNOS_CPUIDLE >>> @@ -99,17 +90,6 @@ static int __init exynos_fdt_map_chipid(unsigned >>> long node, const char *uname, >>> return 1; >>> } >>> >>> -/* >>> - * exynos_map_io >>> - * >>> - * register the standard cpu IO areas >>> - */ >>> -static void __init exynos_map_io(void) >>> -{ >>> - if (soc_is_exynos4()) >>> - iotable_init(exynos4_iodesc, ARRAY_SIZE(exynos4_iodesc)); >>> -} >>> - >>> static void __init exynos_init_io(void) >>> { >>> debug_ll_io_init(); >>> @@ -118,8 +98,6 @@ static void __init exynos_init_io(void) >>> >>> /* detect cpu id and rev. */ >>> s5p_init_cpu(S5P_VA_CHIPID); >>> - >>> - exynos_map_io(); >>> } >>> >>> /* >>> diff --git a/arch/arm/mach-exynos/include/mach/map.h >>> b/arch/arm/mach-exynos/include/mach/map.h >>> index 5fb0040..0eef407 100644 >>> --- a/arch/arm/mach-exynos/include/mach/map.h >>> +++ b/arch/arm/mach-exynos/include/mach/map.h >>> @@ -18,6 +18,4 @@ >>> >>> #define EXYNOS_PA_CHIPID 0x10000000 >>> >>> -#define EXYNOS4_PA_COREPERI 0x10500000 >>> - >>> #endif /* __ASM_ARCH_MAP_H */ >>> diff --git a/arch/arm/mach-exynos/platsmp.c >>> b/arch/arm/mach-exynos/platsmp.c >>> index a5d6841..553d0d9 100644 >>> --- a/arch/arm/mach-exynos/platsmp.c >>> +++ b/arch/arm/mach-exynos/platsmp.c >>> @@ -224,11 +224,6 @@ static void write_pen_release(int val) >>> sync_cache_w(&pen_release); >>> } >>> >>> -static void __iomem *scu_base_addr(void) >>> -{ >>> - return (void __iomem *)(S5P_VA_SCU); >>> -} >>> - >>> static DEFINE_SPINLOCK(boot_lock); >>> >>> static void exynos_secondary_init(unsigned int cpu) >>> @@ -387,14 +382,23 @@ fail: >>> >>> static void __init exynos_smp_prepare_cpus(unsigned int max_cpus) >>> { >>> + struct device_node *np; >>> + void __iomem *scu_base; >>> int i; >>> >>> exynos_sysram_init(); >>> >>> exynos_set_delayed_reset_assertion(true); >>> >>> - if (read_cpuid_part() == ARM_CPU_PART_CORTEX_A9) >>> - scu_enable(scu_base_addr()); >>> + if (read_cpuid_part() == ARM_CPU_PART_CORTEX_A9) { >>> + np = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-scu"); >> >> what if of_find_compatible_node() fails? May be add a error check for >> the same? > > Thanks for review. > > You are right of_find_compatible_node() is bound to fail, but only in > case supplied compatible is missing in DT. In our case this piece of > code will execute only for Cortex-A9 based SoC (which in case of Exynos > SoC is applicable only for Exynos4 series) and we will for sure > providing "arm,cortex-a9-scu" in DT, so there is no chance of failure. > So I feel extra check on "np" for NULL will add no benefit here. > Well I am not entirely convenience here, I still feel it better to have those check, lets not assume anything about future, but when I see of_find_compatible_node() uses elsewhere in kernel, both kind of uses are there (with/without error check). So, I leave it to you and maintainer to take a call on this, otherwise this patch looks good. Reviewed-by: Alim Akhtar > > Thanks, > Pankaj Dubey > >