Add SCU device node support for Exynos4
Subsystems (5)
cover Add SCU device node support for Exynos4
This patch series does some cleanup for Exynos4 SoC based boards. We are currently statically mapping SCU SFRs in mach-exynos/exynos.c which can be avoided and related dependencies can be removed by introduction of SCU device node.
This patch series is tested on Exynos4210 based Origen board for SMP boot.
Pankaj Dubey (4):
ARM: EXYNOS: Remove smp_init_cpus hook from platsmp.c
ARM: dts: exynos: Add SCU device node to exynos4.dtsi
ARM: EXYNOS: Remove static mapping of SCU SFR
ARM: EXYNOS: Remove unused soc_is_exynos{4,5}
arch/arm/boot/dts/exynos4.dtsi | 5 +++ arch/arm/mach-exynos/common.h | 5 --- arch/arm/mach-exynos/exynos.c | 22 ------------- arch/arm/mach-exynos/include/mach/map.h | 2 -- arch/arm/mach-exynos/platsmp.c | 49 +++++++--------------------- arch/arm/mach-exynos/pm.c | 14 ++++++-- arch/arm/mach-exynos/suspend.c | 15 ++++++--- arch/arm/plat-samsung/include/plat/map-s5p.h | 4 --- 8 files changed, 38 insertions(+), 78 deletions(-)
Signature · 1 line
2.7.4
PATCH 1/4 ARM: EXYNOS: Remove smp_init_cpus hook from platsmp.c
We can safely remove exynos_smp_init_cpus() hook from mach-exynos/platsmp.c, as all SMP platforms in mach-exynos can rely on DT for CPU core description instead of determining number of cores from the SCU.
--- arch/arm/mach-exynos/platsmp.c | 31 ------------------------------- 1 file changed, 31 deletions(-)
arch/arm/mach-exynos/platsmp.c +0 -31
@@ -385,36 +385,6 @@ fail:return pen_release !=-1 ? ret :0 ; } -/* -* Initialise the CPU possible map early - this describes the CPUs -* which may be present or become present in the system. -*/ - -static void __init exynos_smp_init_cpus (void ) -{ -void __iomem *scu_base =scu_base_addr (); -unsigned int i, ncores; - -if (read_cpuid_part () ==ARM_CPU_PART_CORTEX_A9 ) - ncores = scu_base ?scu_get_core_count (scu_base) :1 ; -else -/* -* CPU Nodes are passed thru DT and set_cpu_possible -* is set by "arm_dt_init_cpu_maps". -*/ -return ; - -/* sanity check */ -if (ncores > nr_cpu_ids) { -pr_warn ("SMP: %u cores greater than maximum (%u), clipping\n" , - ncores, nr_cpu_ids); - ncores = nr_cpu_ids; - } - -for (i =0 ; i < ncores; i++) -set_cpu_possible (i, true); -} -static void __init exynos_smp_prepare_cpus (unsigned int max_cpus) {int i;
@@ -479,7 +449,6 @@ static void exynos_cpu_die(unsigned int cpu)#endif /* CONFIG_HOTPLUG_CPU */ const struct smp_operations exynos_smp_ops__initconst = { - .smp_init_cpus = exynos_smp_init_cpus, .smp_prepare_cpus = exynos_smp_prepare_cpus, .smp_secondary_init = exynos_secondary_init, .smp_boot_secondary = exynos_boot_secondary,
Signature · 1 line
2.7.4
Alim Akhtar
reviewerPATCH 1/4 ARM: EXYNOS: Remove smp_init_cpus hook from platsmp.c
Hi Pankaj,
Quoted 6 lines · Pankaj Dubey
We can safely remove
exynos_smp_init_cpus()hook frommach-exynos/platsmp.c, as all SMP platforms in mach-exynos can rely on DT for CPU core description instead of determining number of cores from the SCU.Signed-off-by Pankaj Dubey <[email protected]>---
Looks good.
Quoted 53 lines
arch/arm/mach-exynos/platsmp.c | 31 ------------------------------- 1 file changed, 31 deletions(-)arch/arm/mach-exynos/platsmp.c +0
-31@@ -385,36 +385,6 @@ fail:return pen_release !=-1 ? ret :0 ; } -/* -* Initialise the CPU possible map early - this describes the CPUs -* which may be present or become present in the system. -*/ - -static void __init exynos_smp_init_cpus (void ) -{ -void __iomem *scu_base =scu_base_addr (); -unsigned int i, ncores; - -if (read_cpuid_part () ==ARM_CPU_PART_CORTEX_A9 ) - ncores = scu_base ?scu_get_core_count (scu_base) :1 ; -else -/* -* CPU Nodes are passed thru DT and set_cpu_possible -* is set by "arm_dt_init_cpu_maps". -*/ -return ; - -/* sanity check */ -if (ncores > nr_cpu_ids) { -pr_warn ("SMP: %u cores greater than maximum (%u), clipping\n" , - ncores, nr_cpu_ids); - ncores = nr_cpu_ids; - } - -for (i =0 ; i < ncores; i++) -set_cpu_possible (i, true); -} -static void __init exynos_smp_prepare_cpus (unsigned int max_cpus) {int i;@@ -479,7 +449,6 @@ static void exynos_cpu_die(unsigned int cpu)#endif /* CONFIG_HOTPLUG_CPU */ const struct smp_operations exynos_smp_ops__initconst = { - .smp_init_cpus = exynos_smp_init_cpus, .smp_prepare_cpus = exynos_smp_prepare_cpus, .smp_secondary_init = exynos_secondary_init, .smp_boot_secondary = exynos_boot_secondary,
PATCH 1/4 ARM: EXYNOS: Remove smp_init_cpus hook from platsmp.c
In reply to Pankaj DubeyQuoted 9 lines · Pankaj Dubey
We can safely remove
exynos_smp_init_cpus()hook frommach-exynos/platsmp.c, as all SMP platforms in mach-exynos can rely on DT for CPU core description instead of determining number of cores from the SCU.Signed-off-by Pankaj Dubey <[email protected]>--- arch/arm/mach-exynos/platsmp.c | 31 ------------------------------- 1 file changed, 31 deletions(-)
Thanks, applied. Somehow your patchset did not reach linux-samsung-soc's Parchwork. I don't have a clue why... It is on linux-arm-kernel's Patchwork.
Best regards,
Krzysztof
PATCH 1/4 ARM: EXYNOS: Remove smp_init_cpus hook from platsmp.c
Hi Krzysztof,
Quoted 12 lines · Krzysztof Kozlowski
Quoted 9 lines · Pankaj Dubey
We can safely remove
exynos_smp_init_cpus()hook frommach-exynos/platsmp.c, as all SMP platforms in mach-exynos can rely on DT for CPU core description instead of determining number of cores from the SCU.Signed-off-by Pankaj Dubey <[email protected]>--- arch/arm/mach-exynos/platsmp.c | 31 ------------------------------- 1 file changed, 31 deletions(-)
Quoted 4 lines
Thanks, applied. Somehow your patchset did not reach linux-samsung-soc's Parchwork. I don't have a clue why... It is on linux-arm-kernel's Patchwork.
Thanks for looking into this series.
I am also not sure why its not reflecting for you on linux-samsung-soc's Patchwork, but I can see this series in linux-samsung-soc Patchwork at below links:
[1/4]: https://patchwork.kernel.org/patch/9411787/
[2/4]: https://patchwork.kernel.org/patch/9411789/
[3/4]: https://patchwork.kernel.org/patch/9411791/
[4/4]: https://patchwork.kernel.org/patch/9411793/
Will you please review other two patches (3/4 and 4/4) in this series?
Thanks,
Pankaj Dubey
Quoted 6 lines
Best regards,
Krzysztof
Krzysztof Kozlowski
maintainerPATCH 1/4 ARM: EXYNOS: Remove smp_init_cpus hook from platsmp.c
Quoted 20 lines · pankaj.dubey
Hi Krzysztof,
Quoted 12 lines · Krzysztof Kozlowski
Quoted 9 lines · Pankaj Dubey
We can safely remove
exynos_smp_init_cpus()hook frommach-exynos/platsmp.c, as all SMP platforms in mach-exynos can rely on DT for CPU core description instead of determining number of cores from the SCU.Signed-off-by Pankaj Dubey <[email protected]>--- arch/arm/mach-exynos/platsmp.c | 31 ------------------------------- 1 file changed, 31 deletions(-)Quoted 4 lines
Thanks, applied. Somehow your patchset did not reach linux-samsung-soc's Parchwork. I don't have a clue why... It is on linux-arm-kernel's Patchwork.
Quoted 5 lines
Thanks for looking into this series.
I am also not sure why its not reflecting for you on linux-samsung-soc's Patchwork, but I can see this series in linux-samsung-soc Patchwork at below links:
Hmmm... it is weird. Why I didn't see them before? It seems that I even updated their status. Confusing...
Quoted 1 line
Will you please review other two patches (3/4 and 4/4) in this series?
4/4 is okay but it depends on 3/4 which already has a valid comment - what will happen when DT node is not present (which first of all will happen because DTS is applied on separate branch... and anyway the code must be prepared for different DTSes). Initially I thought there will be NULL pointer exception on of_iomap() but after looking at the code it might work, I mean fail in a reasonable way.
Best regards,
Krzysztof
PATCH 1/4 ARM: EXYNOS: Remove smp_init_cpus hook from platsmp.c
Hi Krzysztof,
Quoted 31 lines · Krzysztof Kozlowski
Quoted 20 lines · pankaj.dubey
Hi Krzysztof,
Quoted 12 lines · Krzysztof Kozlowski
Quoted 9 lines · Pankaj Dubey
We can safely remove
exynos_smp_init_cpus()hook frommach-exynos/platsmp.c, as all SMP platforms in mach-exynos can rely on DT for CPU core description instead of determining number of cores from the SCU.Signed-off-by Pankaj Dubey <[email protected]>--- arch/arm/mach-exynos/platsmp.c | 31 ------------------------------- 1 file changed, 31 deletions(-)Quoted 4 lines
Thanks, applied. Somehow your patchset did not reach linux-samsung-soc's Parchwork. I don't have a clue why... It is on linux-arm-kernel's Patchwork.
Quoted 5 lines
Thanks for looking into this series.
I am also not sure why its not reflecting for you on linux-samsung-soc's Patchwork, but I can see this series in linux-samsung-soc Patchwork at below links:
Hmmm... it is weird. Why I didn't see them before? It seems that I even updated their status. Confusing...
Quoted 9 lines
Quoted 1 line
Will you please review other two patches (3/4 and 4/4) in this series?
4/4 is okay but it depends on 3/4 which already has a valid comment - what will happen when DT node is not present (which first of all will happen because DTS is applied on separate branch... and anyway the code must be prepared for different DTSes). Initially I thought there will be NULL pointer exception on
of_iomap()but after looking at the code it might work, I mean fail in a reasonable way.
Yes, you are right, even we do not add check for NULL on "np" it won't cause NULL pointer exception on of_iomap(), and it handles it gracefully, I have given explanation about this on patch [4/4] as reply to Alim.
So we can safely avoid check for NULL on "np" without assuming anything, but at the same time I noticed that in case of_iomap returns NULL, we can't move ahead and configure secondary startup address in exynos_smp_prepare_cpus, so I will resubmit [3/4] and [4/4] with appropriate changes to take care of this case. If you see any other concern please let me know.
Thanks,
Pankaj Dubey
Quoted 6 lines
Best regards,
Krzysztof
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH 2/4 ARM: dts: exynos: Add SCU device node to exynos4.dtsi
In reply to Pankaj DubeyExynos4 like other Cortex-A9 SoC's has a Snoop Control Unit(SCU) and its SFR are used during SMP boot and S2R. Add SCU node to the device tree.
--- arch/arm/boot/dts/exynos4.dtsi | 5 +++++ 1 file changed, 5 insertions(+)
arch/arm/boot/dts/exynos4.dtsi +5 -0
@@ -78,6 +78,11 @@ reg = <0x10000000 0x100>; }; + scu: snoop-control-unit@10500000 { + compatible = "arm,cortex-a9-scu"; + reg = <0x10500000 0x2000>; + }; + memory-controller@12570000 { compatible = "samsung,exynos4210-srom"; reg = <0x12570000 0x14>;
Signature · 1 line
2.7.4
Alim Akhtar
reviewerPATCH 2/4 ARM: dts: exynos: Add SCU device node to exynos4.dtsi
Hi Pankaj,
Quoted 5 lines · Pankaj Dubey
Exynos4 like other Cortex-A9 SoC's has a Snoop Control Unit(SCU) and its SFR are used during SMP boot and S2R. Add SCU node to the device tree.
Signed-off-by Pankaj Dubey <[email protected]>---
Quoted 20 lines
arch/arm/boot/dts/exynos4.dtsi | 5 +++++ 1 file changed, 5 insertions(+)arch/arm/boot/dts/exynos4.dtsi +5
-0@@ -78,6 +78,11 @@ reg = <0x10000000 0x100>; }; + scu: snoop-control-unit at 10500000 { + compatible = "arm,cortex-a9-scu"; + reg = <0x10500000 0x2000>; + }; + memory-controller at 12570000 { compatible = "samsung,exynos4210-srom"; reg = <0x12570000 0x14>;
PATCH 2/4 ARM: dts: exynos: Add SCU device node to exynos4.dtsi
In reply to Pankaj DubeyQuoted 8 lines · Pankaj Dubey
Exynos4 like other Cortex-A9 SoC's has a Snoop Control Unit(SCU) and its SFR are used during SMP boot and S2R. Add SCU node to the device tree.
Signed-off-by Pankaj Dubey <[email protected]>--- arch/arm/boot/dts/exynos4.dtsi | 5 +++++ 1 file changed, 5 insertions(+)
Thanks, applied.
Best regards,
Krzysztof
PATCH 3/4 ARM: EXYNOS: Remove static mapping of SCU SFR
In reply to Pankaj DubeyLets remove static mapping of SCU SFR mainly used in CORTEX-A9 SoC based boards. Instead use mapping from device tree node of SCU.
--- 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(-)
arch/arm/mach-exynos/exynos.c +0 -22
@@ -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 (); }/*
arch/arm/mach-exynos/include/mach/map.h +0 -2
@@ -18,6 +18,4 @@#define EXYNOS_PA_CHIPID 0x10000000 -#define EXYNOS4_PA_COREPERI 0x10500000 -#endif /* __ASM_ARCH_MAP_H */
arch/arm/mach-exynos/platsmp.c +11 -7
@@ -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" ); + scu_base =of_iomap (np,0 ); +if (scu_base) { +scu_enable (scu_base); +iounmap (scu_base); + } +of_node_put (np); + }/* * Write the address of secondary startup into the
arch/arm/mach-exynos/pm.c +11 -3
@@ -18,6 +18,7 @@#include <linux/cpu_pm.h> #include <linux/io.h> #include <linux/of.h> +#include <linux/of_address.h> #include <linux/soc/samsung/exynos-regs-pmu.h> #include <linux/soc/samsung/exynos-pmu.h>
@@ -26,8 +27,6 @@#include <asm/suspend.h> #include <asm/cacheflush.h> -#include <mach/map.h> -#include "common.h" static inline void __iomem *exynos_boot_vector_addr (void )
@@ -158,6 +157,8 @@ static int exynos_aftr_finisher(unsigned long flags)void exynos_enter_aftr (void ) { +struct device_node *np; +void __iomem *scu_base;unsigned int cpuid =smp_processor_id ();cpu_pm_enter ();
@@ -177,7 +178,14 @@ void exynos_enter_aftr(void)cpu_suspend (0 , exynos_aftr_finisher);if (read_cpuid_part () ==ARM_CPU_PART_CORTEX_A9 ) { -scu_enable (S5P_VA_SCU ); + np =of_find_compatible_node (NULL ,NULL ,"arm,cortex-a9-scu" ); + scu_base =of_iomap (np,0 ); +if (scu_base) { +scu_enable (scu_base); +iounmap (scu_base); + } +of_node_put (np); +if (call_firmware_op (resume) == -ENOSYS )exynos_cpu_restore_register (); }
arch/arm/mach-exynos/suspend.c +11 -4
@@ -34,8 +34,6 @@#include <asm/smp_scu.h> #include <asm/suspend.h> -#include <mach/map.h> -#include <plat/pm-common.h> #include "common.h"
@@ -453,6 +451,8 @@ static void exynos_pm_release_retention(void)static void exynos_pm_resume (void ) { +struct device_node *np; +void __iomem *scu_base;u32 cpuid =read_cpuid_part ();if (exynos_pm_central_resume ())
@@ -461,8 +461,15 @@ static void exynos_pm_resume(void)/* For release retention */ exynos_pm_release_retention (); -if (cpuid ==ARM_CPU_PART_CORTEX_A9 ) -scu_enable (S5P_VA_SCU ); +if (cpuid ==ARM_CPU_PART_CORTEX_A9 ) { + np =of_find_compatible_node (NULL ,NULL ,"arm,cortex-a9-scu" ); + scu_base =of_iomap (np,0 ); +if (scu_base) { +scu_enable (scu_base); +iounmap (scu_base); + } +of_node_put (np); + }if (call_firmware_op (resume) == -ENOSYS && cpuid ==ARM_CPU_PART_CORTEX_A9 )
arch/arm/plat-samsung/include/plat/map-s5p.h +0 -4
@@ -15,10 +15,6 @@#define S5P_VA_CHIPID S3C_ADDR(0x02000000) -#define S5P_VA_COREPERI_BASE S3C_ADDR(0x02800000) -#define S5P_VA_COREPERI (x) (S5P_VA_COREPERI_BASE + (x)) -#define S5P_VA_SCU S5P_VA_COREPERI(0x0) -#define VA_VIC (x) (S3C_VA_IRQ + ((x) * 0x10000))#define VA_VIC0 VA_VIC(0)#define VA_VIC1 VA_VIC(1)
Signature · 1 line
2.7.4
Alim Akhtar
reviewerPATCH 3/4 ARM: EXYNOS: Remove static mapping of SCU SFR
Hi Pankaj,
Quoted 93 lines · Pankaj Dubey
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 <[email protected]>--- 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(-)arch/arm/mach-exynos/exynos.c +0
-22@@ -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 (); }/* arch/arm/mach-exynos/include/mach/map.h +0
-2@@ -18,6 +18,4 @@#define EXYNOS_PA_CHIPID 0x10000000 -#define EXYNOS4_PA_COREPERI 0x10500000 -#endif /* __ASM_ARCH_MAP_H */ arch/arm/mach-exynos/platsmp.c +2
-5@@ -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;
arch/arm/mach-exynos/platsmp.c fail: · line 394
Quoted 10 lines
arch/arm/mach-exynos/platsmp.c +2
-2int 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?
Quoted 41 lines
arch/arm/mach-exynos/platsmp.c +7
-0+ scu_base =of_iomap (np,0 ); +if (scu_base) { +scu_enable (scu_base); +iounmap (scu_base); + } +of_node_put (np); + }/* * Write the address of secondary startup into the arch/arm/mach-exynos/pm.c +3
-2@@ -18,6 +18,7 @@#include <linux/cpu_pm.h> #include <linux/io.h> #include <linux/of.h> +#include <linux/of_address.h> #include <linux/soc/samsung/exynos-regs-pmu.h> #include <linux/soc/samsung/exynos-pmu.h> @@ -26,8 +27,6 @@#include <asm/suspend.h> #include <asm/cacheflush.h> -#include <mach/map.h> -#include "common.h" static inline void __iomem *exynos_boot_vector_addr (void )@@ -158,6 +157,8 @@ static int exynos_aftr_finisher(unsigned long flags)void exynos_enter_aftr (void ) { +struct device_node *np; +void __iomem *scu_base;unsigned int cpuid =smp_processor_id ();cpu_pm_enter ();
arch/arm/mach-exynos/pm.c void exynos_enter_aftr(void) · line 181
Quoted 5 lines
arch/arm/mach-exynos/pm.c +1
-1@@ -177,7 +178,14 @@ void exynos_enter_aftr(void)cpu_suspend (0 , exynos_aftr_finisher);if (read_cpuid_part () ==ARM_CPU_PART_CORTEX_A9 ) { -scu_enable (S5P_VA_SCU ); + np =of_find_compatible_node (NULL ,NULL ,"arm,cortex-a9-scu" );
same as above
Quoted 33 lines
arch/arm/mach-exynos/pm.c +7
-0+ scu_base =of_iomap (np,0 ); +if (scu_base) { +scu_enable (scu_base); +iounmap (scu_base); + } +of_node_put (np); +if (call_firmware_op (resume) == -ENOSYS )exynos_cpu_restore_register (); }arch/arm/mach-exynos/suspend.c +2
-2@@ -34,8 +34,6 @@#include <asm/smp_scu.h> #include <asm/suspend.h> -#include <mach/map.h> -#include <plat/pm-common.h> #include "common.h" @@ -453,6 +451,8 @@ static void exynos_pm_release_retention(void)static void exynos_pm_resume (void ) { +struct device_node *np; +void __iomem *scu_base;u32 cpuid =read_cpuid_part ();if (exynos_pm_central_resume ())
arch/arm/mach-exynos/suspend.c static void exynos_pm_resume(void) · line 465
Quoted 7 lines
arch/arm/mach-exynos/suspend.c +2
-2@@ -461,8 +461,15 @@ static void exynos_pm_resume(void)/* For release retention */ exynos_pm_release_retention (); -if (cpuid ==ARM_CPU_PART_CORTEX_A9 ) -scu_enable (S5P_VA_SCU ); +if (cpuid ==ARM_CPU_PART_CORTEX_A9 ) { + np =of_find_compatible_node (NULL ,NULL ,"arm,cortex-a9-scu" );
and here otherwise patch looks good.
Quoted 26 lines
arch/arm/mach-exynos/suspend.c +7
-0+ scu_base =of_iomap (np,0 ); +if (scu_base) { +scu_enable (scu_base); +iounmap (scu_base); + } +of_node_put (np); + }if (call_firmware_op (resume) == -ENOSYS && cpuid ==ARM_CPU_PART_CORTEX_A9 )arch/arm/plat-samsung/include/plat/map-s5p.h +0
-4@@ -15,10 +15,6 @@#define S5P_VA_CHIPID S3C_ADDR(0x02000000) -#define S5P_VA_COREPERI_BASE S3C_ADDR(0x02800000) -#define S5P_VA_COREPERI (x) (S5P_VA_COREPERI_BASE + (x)) -#define S5P_VA_SCU S5P_VA_COREPERI(0x0) -#define VA_VIC (x) (S3C_VA_IRQ + ((x) * 0x10000))#define VA_VIC0 VA_VIC(0)#define VA_VIC1 VA_VIC(1)
PATCH 3/4 ARM: EXYNOS: Remove static mapping of SCU SFR
Hi Alim,
Quoted 108 lines · Alim Akhtar
Hi Pankaj,
Quoted 105 lines · Pankaj Dubey
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 <[email protected]>--- 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(-)arch/arm/mach-exynos/exynos.c +0
-20@@ -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();arch/arm/mach-exynos/exynos.c +0
-2@@ -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 (); }/* arch/arm/mach-exynos/include/mach/map.h +0
-2@@ -18,6 +18,4 @@#define EXYNOS_PA_CHIPID 0x10000000 -#define EXYNOS4_PA_COREPERI 0x10500000 -#endif /* __ASM_ARCH_MAP_H */ arch/arm/mach-exynos/platsmp.c +2
-5@@ -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);
Quoted 7 lines
arch/arm/mach-exynos/platsmp.c fail: · line 394
Quoted 4 lines
arch/arm/mach-exynos/platsmp.c +2
-2-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.
Thanks,
Pankaj Dubey
Alim Akhtar
reviewerPATCH 3/4 ARM: EXYNOS: Remove static mapping of SCU SFR
Hi Pankaj,
Quoted 119 lines · pankaj.dubey
Hi Alim,
Quoted 108 lines · Alim Akhtar
Hi Pankaj,
Quoted 105 lines · Pankaj Dubey
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 <[email protected]>--- 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(-)arch/arm/mach-exynos/exynos.c +0
-20@@ -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();arch/arm/mach-exynos/exynos.c +0
-2@@ -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 (); }/* arch/arm/mach-exynos/include/mach/map.h +0
-2@@ -18,6 +18,4 @@#define EXYNOS_PA_CHIPID 0x10000000 -#define EXYNOS4_PA_COREPERI 0x10500000 -#endif /* __ASM_ARCH_MAP_H */ arch/arm/mach-exynos/platsmp.c +2
-5@@ -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);Quoted 7 lines
arch/arm/mach-exynos/platsmp.c fail: · line 394
Quoted 4 lines
arch/arm/mach-exynos/platsmp.c +2
-2-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?
Quoted 9 lines
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.
Quoted 5 lines
Thanks,
Pankaj Dubey
PATCH 3/4 ARM: EXYNOS: Remove static mapping of SCU SFR
Hi Alim,
Quoted 138 lines · Alim Akhtar
Hi Pankaj,
Quoted 124 lines · pankaj.dubey
Hi Alim,
Quoted 110 lines · Alim Akhtar
Hi Pankaj,
Quoted 106 lines · Pankaj Dubey
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 <[email protected]>--- 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(-)arch/arm/mach-exynos/exynos.c +0
-20@@ -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();arch/arm/mach-exynos/exynos.c +0
-2@@ -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 (); }/* arch/arm/mach-exynos/include/mach/map.h +0
-2@@ -18,6 +18,4 @@#define EXYNOS_PA_CHIPID 0x10000000 -#define EXYNOS4_PA_COREPERI 0x10500000 -#endif /* __ASM_ARCH_MAP_H */ arch/arm/mach-exynos/platsmp.c +2
-5@@ -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);Quoted 8 lines
arch/arm/mach-exynos/platsmp.c fail: · line 394
Quoted 4 lines
arch/arm/mach-exynos/platsmp.c +2
-2-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?Quoted 9 lines
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.
Quoted 5 lines
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 <[email protected]>
Well, I further checked details of of_find_compatible_node() and of_iomap and below is function call chain
of_find_compatible_node //Returns vaild pointer for match, and NULL for no match
--> __of_device_is_compatible () //Returns 0 for no match, and a
positive integer on match
So in our case lets say for any reason "arm,cortex-a9-scu" compatible is not present, "np" will be set as NULL.
Next we have following line without check on "np" for NULL, scu_base = of_iomap(np, 0);
If we see function call sequence of of_iomap we have,
of_iomap(..) //Returns NULL is of_address_to_resource returns non-zero
--> of_address_to_resource(...) // Returns -EINVAL if "addrp" is NULL
--> of_get_address(...) //Returns NULL if "parent" is NULL
--> of_get_parent(...) //Returns np with refcountincremented if np is NOT NULL else returns NULL
So in our case we will get scu_base as NULL if we pass "np" as NULL, and on very next line we have check on scu_base, which will prevent us from de-referencing a NULL pointer. So there won't be any CRASH or abnormal behavior, due to not checking "np" for NULL.
I think this is the reason in many places in kernel there is no check for NULL for of_find_compatible_node, surely some places there are checks, but as per current code flow if someone is calling of_iomap with np as NULL there won't be any crash and of_iomap will gracefully returns NULL in that case. Also of_node_put and of_node_get has check for NULL received device_node and handles it gracefully.
But one thing I missed is that in case scu_base is NULL we are not suppose to move ahead in function exynos_smp_prepare_cpus(..), This part I will update and post next version with appropriate change.
Thanks,
Pankaj Dubey
Quoted 10 lines
Quoted 6 lines
Thanks,
Pankaj Dubey--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Krzysztof Kozlowski
maintainerPATCH 3/4 ARM: EXYNOS: Remove static mapping of SCU SFR
In reply to Pankaj DubeyQuoted 99 lines · Pankaj Dubey
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 <[email protected]>--- 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(-)arch/arm/mach-exynos/exynos.c +0
-22@@ -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 (); }/* arch/arm/mach-exynos/include/mach/map.h +0
-2@@ -18,6 +18,4 @@#define EXYNOS_PA_CHIPID 0x10000000 -#define EXYNOS4_PA_COREPERI 0x10500000 -#endif /* __ASM_ARCH_MAP_H */ arch/arm/mach-exynos/platsmp.c +2
-5@@ -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);
arch/arm/mach-exynos/platsmp.c fail: · line 400
Quoted 10 lines
arch/arm/mach-exynos/platsmp.c +8
-2-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" ); + scu_base =of_iomap (np,0 ); +if (scu_base) { +scu_enable (scu_base); +iounmap (scu_base); + } +of_node_put (np);
I do not like the duplication - this code appears in three places and they are the same. Could you split it into separate function?
As you mentioned to Alim, in case of lack of node, it would be nice to notify the user (pr_err() etc).
Best regards,
Krzysztof
PATCH 3/4 ARM: EXYNOS: Remove static mapping of SCU SFR
Hi Krzysztof,
Quoted 100 lines · Krzysztof Kozlowski
Quoted 99 lines · Pankaj Dubey
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 <[email protected]>--- 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(-)arch/arm/mach-exynos/exynos.c +0
-22@@ -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 (); }/* arch/arm/mach-exynos/include/mach/map.h +0
-2@@ -18,6 +18,4 @@#define EXYNOS_PA_CHIPID 0x10000000 -#define EXYNOS4_PA_COREPERI 0x10500000 -#endif /* __ASM_ARCH_MAP_H */ arch/arm/mach-exynos/platsmp.c +2
-5@@ -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);
Quoted 14 lines
arch/arm/mach-exynos/platsmp.c fail: · line 400
Quoted 10 lines
arch/arm/mach-exynos/platsmp.c +8
-2-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" ); + scu_base =of_iomap (np,0 ); +if (scu_base) { +scu_enable (scu_base); +iounmap (scu_base); + } +of_node_put (np);I do not like the duplication - this code appears in three places and they are the same. Could you split it into separate function?
OK, will do these changes in v2.
Only pm.c and suspend.c file's scu_enable can be moved to single place without introducing more dependencies between these mach files.
Quoted 3 lines
As you mentioned to Alim, in case of lack of node, it would be nice to notify the user (
pr_err()etc).
OK, will do these changes in v2.
Thanks,
Pankaj Dubey
Quoted 6 lines
Best regards,
Krzysztof
PATCH 4/4 ARM: EXYNOS: Remove unused soc_is_exynos{4,5}
In reply to Pankaj DubeyAs no more user of soc_is_exynos{4,5} we can safely remove them.
--- arch/arm/mach-exynos/common.h | 5 ----- 1 file changed, 5 deletions(-)
arch/arm/mach-exynos/common.h +0 -5
@@ -105,11 +105,6 @@ IS_SAMSUNG_CPU(exynos5800, EXYNOS5800_SOC_ID, EXYNOS5_SOC_MASK)# define soc_is_exynos5800 () 0#endif -#define soc_is_exynos4 () (soc_is_exynos4210() || soc_is_exynos4212() || \ - soc_is_exynos4412()) -#define soc_is_exynos5 () (soc_is_exynos5250() || soc_is_exynos5410() || \ - soc_is_exynos5420() || soc_is_exynos5800()) -extern u32 cp15_save_diag;extern u32 cp15_save_power;
Signature · 1 line
2.7.4
Alim Akhtar
reviewerPATCH 4/4 ARM: EXYNOS: Remove unused soc_is_exynos{4,5}
Hi Pankaj,
Quoted 4 lines · Pankaj Dubey
As no more user of soc_is_exynos{4,5} we can safely remove them.
Signed-off-by Pankaj Dubey <[email protected]>---
Also I have complied tested this series, looks good.
Quoted 20 lines
arch/arm/mach-exynos/common.h | 5 ----- 1 file changed, 5 deletions(-)arch/arm/mach-exynos/common.h +0
-5@@ -105,11 +105,6 @@ IS_SAMSUNG_CPU(exynos5800, EXYNOS5800_SOC_ID, EXYNOS5_SOC_MASK)# define soc_is_exynos5800 () 0#endif -#define soc_is_exynos4 () (soc_is_exynos4210() || soc_is_exynos4212() || \ - soc_is_exynos4412()) -#define soc_is_exynos5 () (soc_is_exynos5250() || soc_is_exynos5410() || \ - soc_is_exynos5420() || soc_is_exynos5800()) -extern u32 cp15_save_diag;extern u32 cp15_save_power;
cover Add SCU device node support for Exynos4
In reply to Pankaj DubeyHi Pankaj,
Quoted 13 lines · Pankaj Dubey
This patch series does some cleanup for Exynos4 SoC based boards. We are currently statically mapping SCU SFRs in
mach-exynos/exynos.cwhich can be avoided and related dependencies can be removed by introduction of SCU device node.This patch series is tested on Exynos4210 based Origen board for SMP boot.
Pankaj Dubey (4):
ARM: EXYNOS: Removesmp_init_cpushook fromplatsmp.c
ARM: dts: exynos: Add SCU device node toexynos4.dtsi
ARM: EXYNOS: Remove static mapping of SCU SFR
ARM: EXYNOS: Remove unused soc_is_exynos{4,5}
Quoted 9 lines
arch/arm/boot/dts/exynos4.dtsi | 5 +++ arch/arm/mach-exynos/common.h | 5 --- arch/arm/mach-exynos/exynos.c | 22 ------------- arch/arm/mach-exynos/include/mach/map.h | 2 -- arch/arm/mach-exynos/platsmp.c | 49 +++++++--------------------- arch/arm/mach-exynos/pm.c | 14 ++++++-- arch/arm/mach-exynos/suspend.c | 15 ++++++--- arch/arm/plat-samsung/include/plat/map-s5p.h | 4 --- 8 files changed, 38 insertions(+), 78 deletions(-)
Nice cleanup! Works fine on Exynos4412-based Odroid U3 (SMP boot and S2R).
Best regards
Signature · 2 lines
Marek Szyprowski, PhD Samsung R&D Institute Poland