subject:"\[PATCH v2 1\/2\] ARM\: EXYNOS\: Map SYSRAM through generic SRAM bindings"

Re: [PATCH v2 1/2] ARM: EXYNOS: Map SYSRAM through generic SRAM bindings

2014-05-07 Thread Tomasz Figa

On 06.05.2014 20:09, Sachin Kamat wrote:
[snip]
 On 6 May 2014 22:14, Tomasz Figa tomasz.f...@gmail.com wrote:
[snip]
 On 06.05.2014 10:10, Sachin Kamat wrote:
[snip]
 diff --git a/arch/arm/mach-exynos/platsmp.c
 b/arch/arm/mach-exynos/platsmp.c
 index 03e5e9f94705..ccbcdd7b8a86 100644
 --- a/arch/arm/mach-exynos/platsmp.c
 +++ b/arch/arm/mach-exynos/platsmp.c
 @@ -20,6 +20,7 @@
   #include linux/jiffies.h
   #include linux/smp.h
   #include linux/io.h
 +#include linux/of_address.h

   #include asm/cacheflush.h
   #include asm/smp_plat.h
 @@ -33,11 +34,35 @@

   extern void exynos4_secondary_startup(void);

 +static void __iomem *sram_base_addr;
 +void __iomem *sram_ns_base_addr;


 This is probably just a matter of preference, but I'd make this static and
 provide a getter function, like exynos_get_sram_ns_base();

 Also this address seems to be used by the firmware code exclusively. If we
 want to make the firmware code more self-contained, maybe the mapping of
 firmware-specific SRAM region could be handled there, instead? This would
 also eliminate the need for having an exported variable or getter function.
 What do you think?
 
 I thought of the same. However 2 reasons prevented me from implementing this.
 
 1. Code duplication
 2. This code should be executed only once. I put it in exynos_firmware_init.
 However it gave a crash while doing of_iomap. So moved it back to the current
 location.


Right, exynos_firmware_init() is called too early, before ioremap() is
available, so there is no good place to put this in firmware code. So,
let's keep this as is for now and eventually move it to firmware if
needed and/or a proper method of initialization is available.



 +
 +static void __init exynos_smp_prepare_sram(void)
 +{
 +   struct device_node *node;
 +
 +   for_each_compatible_node(node, NULL, samsung,exynos4210-sram) {
 +   if (of_device_is_available(node)) {
 +   sram_base_addr = of_iomap(node, 0);
 +   if (!sram_base_addr)
 +   pr_err(Secondary CPU boot address not
 found\n);


 I don't think this is an error at this stage. Some platforms might not have
 either of these SRAM reserved regions (such as those using INFORM registers
 instead). Instead, the base address should be checked whenever it is needed
 and errors should be handled then, like in exynos_set_cpu_boot_addr().
 
 Yes. This is more from an information perspective. Probably pr_warn or
 pr_info would
 be better?
 

IMHO this shouldn't produce any messages at this stage. Just whenever
either of addresses is actually needed and is not initialized a message
should be printed.

 +}
 +
   static inline void __iomem *cpu_boot_reg_base(void)
   {
 if (soc_is_exynos4210()  samsung_rev() == EXYNOS4210_REV_1_1)
 return S5P_INFORM5;
 -   return S5P_VA_SYSRAM;
 +   return sram_base_addr;
   }

   static inline void __iomem *cpu_boot_reg(int cpu)
 @@ -147,7 +172,8 @@ static int exynos_boot_secondary(unsigned int cpu,
 struct task_struct *idle)
  * and fall back to boot register if it fails.
  */
 if (call_firmware_op(set_cpu_boot_addr, phys_cpu,
 boot_addr))
 -   __raw_writel(boot_addr, cpu_boot_reg(phys_cpu));
 +   if (cpu_boot_reg_base())
 +   __raw_writel(boot_addr,
 cpu_boot_reg(phys_cpu));


 I'd rework this for proper error handling, e.g.

 int ret;

 /* ... */

 ret = call_firmware_op(set_cpu_boot_addr, phys_cpu, boot_addr);
 if (ret  ret != -ENOSYS)
 goto fail;
 if (ret == -ENOSYS) {
 /* Fall back to firmware-less way. */
 void __iomem *boot_reg = cpu_boot_reg(phys_cpu);

 if (IS_ERR(boot_reg)) {
 ret = PTR_ERR(boot_reg);
 goto fail;
 }
 }

 /* ... */

 fail:
 /* Clean-up after error */

 Of course, cpu_boot_reg() will need to be converted to follow the ERR_PTR()
 model, but IMHO proper error handling is a good reason to do so.
 
 How about handling this separately outside this patch?
 

This patch is already changing surroundings of this code, so I'd say it
should do it the right way from the start.

Best regards,
Tomasz
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] ARM: EXYNOS: Map SYSRAM through generic SRAM bindings

2014-05-07 Thread Sachin Kamat

Hi Tomasz,

On 7 May 2014 22:33, Tomasz Figa t.f...@samsung.com wrote:
 On 06.05.2014 20:09, Sachin Kamat wrote:
 [snip]
 On 6 May 2014 22:14, Tomasz Figa tomasz.f...@gmail.com wrote:
 [snip]
 On 06.05.2014 10:10, Sachin Kamat wrote:
 [snip]
 diff --git a/arch/arm/mach-exynos/platsmp.c
 b/arch/arm/mach-exynos/platsmp.c
 index 03e5e9f94705..ccbcdd7b8a86 100644
 --- a/arch/arm/mach-exynos/platsmp.c
 +++ b/arch/arm/mach-exynos/platsmp.c
 @@ -20,6 +20,7 @@
   #include linux/jiffies.h
   #include linux/smp.h
   #include linux/io.h
 +#include linux/of_address.h

   #include asm/cacheflush.h
   #include asm/smp_plat.h
 @@ -33,11 +34,35 @@

   extern void exynos4_secondary_startup(void);

 +static void __iomem *sram_base_addr;
 +void __iomem *sram_ns_base_addr;


 This is probably just a matter of preference, but I'd make this static and
 provide a getter function, like exynos_get_sram_ns_base();

 Also this address seems to be used by the firmware code exclusively. If we
 want to make the firmware code more self-contained, maybe the mapping of
 firmware-specific SRAM region could be handled there, instead? This would
 also eliminate the need for having an exported variable or getter function.
 What do you think?

 I thought of the same. However 2 reasons prevented me from implementing this.

 1. Code duplication
 2. This code should be executed only once. I put it in exynos_firmware_init.
 However it gave a crash while doing of_iomap. So moved it back to the current
 location.


 Right, exynos_firmware_init() is called too early, before ioremap() is
 available, so there is no good place to put this in firmware code. So,
 let's keep this as is for now and eventually move it to firmware if
 needed and/or a proper method of initialization is available.

OK.




 +
 +static void __init exynos_smp_prepare_sram(void)
 +{
 +   struct device_node *node;
 +
 +   for_each_compatible_node(node, NULL, samsung,exynos4210-sram) {
 +   if (of_device_is_available(node)) {
 +   sram_base_addr = of_iomap(node, 0);
 +   if (!sram_base_addr)
 +   pr_err(Secondary CPU boot address not
 found\n);


 I don't think this is an error at this stage. Some platforms might not have
 either of these SRAM reserved regions (such as those using INFORM registers
 instead). Instead, the base address should be checked whenever it is needed
 and errors should be handled then, like in exynos_set_cpu_boot_addr().

 Yes. This is more from an information perspective. Probably pr_warn or
 pr_info would
 be better?


 IMHO this shouldn't produce any messages at this stage. Just whenever
 either of addresses is actually needed and is not initialized a message
 should be printed.

OK.

 +}
 +
   static inline void __iomem *cpu_boot_reg_base(void)
   {
 if (soc_is_exynos4210()  samsung_rev() == EXYNOS4210_REV_1_1)
 return S5P_INFORM5;
 -   return S5P_VA_SYSRAM;
 +   return sram_base_addr;
   }

   static inline void __iomem *cpu_boot_reg(int cpu)
 @@ -147,7 +172,8 @@ static int exynos_boot_secondary(unsigned int cpu,
 struct task_struct *idle)
  * and fall back to boot register if it fails.
  */
 if (call_firmware_op(set_cpu_boot_addr, phys_cpu,
 boot_addr))
 -   __raw_writel(boot_addr, cpu_boot_reg(phys_cpu));
 +   if (cpu_boot_reg_base())
 +   __raw_writel(boot_addr,
 cpu_boot_reg(phys_cpu));


 I'd rework this for proper error handling, e.g.

 int ret;

 /* ... */

 ret = call_firmware_op(set_cpu_boot_addr, phys_cpu, boot_addr);
 if (ret  ret != -ENOSYS)
 goto fail;
 if (ret == -ENOSYS) {
 /* Fall back to firmware-less way. */
 void __iomem *boot_reg = cpu_boot_reg(phys_cpu);

 if (IS_ERR(boot_reg)) {
 ret = PTR_ERR(boot_reg);
 goto fail;
 }
 }

 /* ... */

 fail:
 /* Clean-up after error */

 Of course, cpu_boot_reg() will need to be converted to follow the ERR_PTR()
 model, but IMHO proper error handling is a good reason to do so.

 How about handling this separately outside this patch?


 This patch is already changing surroundings of this code, so I'd say it
 should do it the right way from the start.

Yes, I have incorporated it in my patch. Will send the same tomorrow.

-- 
With warm regards,
Sachin
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 1/2] ARM: EXYNOS: Map SYSRAM through generic SRAM bindings

2014-05-06 Thread Sachin Kamat

Instead of hardcoding the SYSRAM details for each SoC,
pass this information through device tree (DT) and make
the code SoC agnostic. Generic SRAM bindings are used
for achieving this.

Signed-off-by: Sachin Kamat sachin.ka...@linaro.org
Acked-by: Arnd Bergmann a...@arndb.de
Acked-by: Heiko Stuebner he...@sntech.de
---
Changes since v1:
* Addressed Tomasz Figa's comments
- Fixed sram node for universal_c210
- Check the node status before mapping the address

This patch is based on linux next (next-20140501) on top of
my Kconfig consolidation patch
http://comments.gmane.org/gmane.linux.kernel.samsung-soc/28642

Tested on 4210/4412 Origen, 5250/5420 Arndale and SMDK5420 boards.
---
 arch/arm/Kconfig|1 +
 arch/arm/boot/dts/exynos4210-universal_c210.dts |   24 +
 arch/arm/boot/dts/exynos4210.dtsi   |   18 +++
 arch/arm/boot/dts/exynos4x12.dtsi   |   18 +++
 arch/arm/boot/dts/exynos5250.dtsi   |   18 +++
 arch/arm/boot/dts/exynos5420.dtsi   |   18 +++
 arch/arm/mach-exynos/common.h   |1 +
 arch/arm/mach-exynos/exynos.c   |   64 ---
 arch/arm/mach-exynos/firmware.c |8 ++-
 arch/arm/mach-exynos/include/mach/map.h |7 ---
 arch/arm/mach-exynos/platsmp.c  |   35 +++--
 11 files changed, 137 insertions(+), 75 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index a6d19b1a..f66ea9453df9 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -855,6 +855,7 @@ config ARCH_EXYNOS
select S5P_DEV_MFC
select SAMSUNG_DMADEV
select SPARSE_IRQ
+   select SRAM
select USE_OF
help
  Support for SAMSUNG's EXYNOS SoCs (EXYNOS4/5)
diff --git a/arch/arm/boot/dts/exynos4210-universal_c210.dts 
b/arch/arm/boot/dts/exynos4210-universal_c210.dts
index 63e34b24b04f..9813b068cfd8 100644
--- a/arch/arm/boot/dts/exynos4210-universal_c210.dts
+++ b/arch/arm/boot/dts/exynos4210-universal_c210.dts
@@ -28,6 +28,30 @@
bootargs = console=ttySAC2,115200N8 root=/dev/mmcblk0p5 rw 
rootwait earlyprintk panic=5 maxcpus=1;
};
 
+   sram@0202 {
+   status = disabled;
+   smp-sram@0 {
+   status = disabled;
+   };
+
+   smp-sram@1f000 {
+   status = disabled;
+   };
+   };
+
+   sram@02025000 {
+   compatible = mmio-sram;
+   reg = 0x02025000 0x1000;
+   #address-cells = 1;
+   #size-cells = 1;
+   ranges = 0 0x02025000 0x1000;
+
+   smp-sram@0 {
+   compatible = samsung,exynos4210-sram;
+   reg = 0x0 0x1000;
+   };
+   };
+
mct@1005 {
compatible = none;
};
diff --git a/arch/arm/boot/dts/exynos4210.dtsi 
b/arch/arm/boot/dts/exynos4210.dtsi
index cacf6140dd2f..d3d727b0c263 100644
--- a/arch/arm/boot/dts/exynos4210.dtsi
+++ b/arch/arm/boot/dts/exynos4210.dtsi
@@ -31,6 +31,24 @@
pinctrl2 = pinctrl_2;
};
 
+   sram@0202 {
+   compatible = mmio-sram;
+   reg = 0x0202 0x2;
+   #address-cells = 1;
+   #size-cells = 1;
+   ranges = 0 0x0202 0x2;
+
+   smp-sram@0 {
+   compatible = samsung,exynos4210-sram;
+   reg = 0x0 0x1000;
+   };
+
+   smp-sram@1f000 {
+   compatible = samsung,exynos4210-sram-ns;
+   reg = 0x1f000 0x1000;
+   };
+   };
+
pd_lcd1: lcd1-power-domain@10023CA0 {
compatible = samsung,exynos4210-pd;
reg = 0x10023CA0 0x20;
diff --git a/arch/arm/boot/dts/exynos4x12.dtsi 
b/arch/arm/boot/dts/exynos4x12.dtsi
index c4a9306f8529..75fb3e7e3999 100644
--- a/arch/arm/boot/dts/exynos4x12.dtsi
+++ b/arch/arm/boot/dts/exynos4x12.dtsi
@@ -37,6 +37,24 @@
interrupts = 2 2, 3 2, 18 2, 19 2;
};
 
+   sram@0202 {
+   compatible = mmio-sram;
+   reg = 0x0202 0x4;
+   #address-cells = 1;
+   #size-cells = 1;
+   ranges = 0 0x0202 0x4;
+
+   smp-sram@0 {
+   compatible = samsung,exynos4210-sram;
+   reg = 0x0 0x1000;
+   };
+
+   smp-sram@2f000 {
+   compatible = samsung,exynos4210-sram-ns;
+   reg = 0x2f000 0x1000;
+   };
+   };
+
pd_isp: isp-power-domain@10023CA0 {
compatible = samsung,exynos4210-pd;
reg = 0x10023CA0 0x20;
diff --git a/arch/arm/boot/dts/exynos5250.dtsi 
b/arch/arm/boot/dts/exynos5250.dtsi
index

Re: [PATCH v2 1/2] ARM: EXYNOS: Map SYSRAM through generic SRAM bindings

2014-05-06 Thread Sachin Kamat

Hi Tomasz,

On 6 May 2014 22:14, Tomasz Figa tomasz.f...@gmail.com wrote:
 Hi Sachin,

 Thanks for addressing the comments. I need to verify few things on Universal
 C210 board first, before I could give my Reviewed-by tag or further
 comments.

 I also have some general comments that I missed before, due to limited time
 for review. Please see inline.

Thanks for your review.



 On 06.05.2014 10:10, Sachin Kamat wrote:

 Instead of hardcoding the SYSRAM details for each SoC,
 pass this information through device tree (DT) and make
 the code SoC agnostic. Generic SRAM bindings are used
 for achieving this.

 Signed-off-by: Sachin Kamat sachin.ka...@linaro.org
 Acked-by: Arnd Bergmann a...@arndb.de
 Acked-by: Heiko Stuebner he...@sntech.de
 ---
 Changes since v1:
 * Addressed Tomasz Figa's comments
 - Fixed sram node for universal_c210
 - Check the node status before mapping the address

 This patch is based on linux next (next-20140501) on top of
 my Kconfig consolidation patch
 http://comments.gmane.org/gmane.linux.kernel.samsung-soc/28642


 [snip]


 diff --git a/arch/arm/boot/dts/exynos4210-universal_c210.dts
 b/arch/arm/boot/dts/exynos4210-universal_c210.dts
 index 63e34b24b04f..9813b068cfd8 100644
 --- a/arch/arm/boot/dts/exynos4210-universal_c210.dts
 +++ b/arch/arm/boot/dts/exynos4210-universal_c210.dts
 @@ -28,6 +28,30 @@
 bootargs = console=ttySAC2,115200N8 root=/dev/mmcblk0p5
 rw rootwait earlyprintk panic=5 maxcpus=1;
 };

 +   sram@0202 {
 +   status = disabled;
 +   smp-sram@0 {
 +   status = disabled;
 +   };
 +
 +   smp-sram@1f000 {
 +   status = disabled;
 +   };
 +   };
 +
 +   sram@02025000 {
 +   compatible = mmio-sram;
 +   reg = 0x02025000 0x1000;
 +   #address-cells = 1;
 +   #size-cells = 1;
 +   ranges = 0 0x02025000 0x1000;
 +
 +   smp-sram@0 {
 +   compatible = samsung,exynos4210-sram;
 +   reg = 0x0 0x1000;
 +   };
 +   };


 I wonder if this really should be defined like this. 0x2025000 really looks
 just like an offset from the normal SRAM address. This is the thing I need
 to check in documentation and by experiment when I'll return back to work
 tomorrow, but maybe it could be possible to normally use the sram@0202
 and just disable smp-sram@0 and smp-sram@1f000, replacing them with
 smp-sram@5000 on Universal C210.

I do not have any info about universal C210 board and need your inputs
for the same :)



 +
 mct@1005 {
 compatible = none;
 };


 [snip]


 diff --git a/arch/arm/mach-exynos/firmware.c
 b/arch/arm/mach-exynos/firmware.c
 index 932129ef26c6..7d583cb73850 100644
 --- a/arch/arm/mach-exynos/firmware.c
 +++ b/arch/arm/mach-exynos/firmware.c
 @@ -18,6 +18,7 @@

   #include mach/map.h

 +#include common.h
   #include smc.h

   static int exynos_do_idle(void)
 @@ -34,7 +35,12 @@ static int exynos_cpu_boot(int cpu)

   static int exynos_set_cpu_boot_addr(int cpu, unsigned long boot_addr)
   {
 -   void __iomem *boot_reg = S5P_VA_SYSRAM_NS + 0x1c + 4*cpu;
 +   void __iomem *boot_reg;
 +
 +   if (!sram_ns_base_addr)
 +   return 0;


 Shouldn't this return an error instead? I'm not sure which one would be
 appropriate, though, probably one of -ENODEV, -ENXIO or -EFAULT.

IIRC, returning error here causes the system to hang and even primary
cpu does not boot.
Since any error or absence of sram nodes should atleast boot the
primary CPU, I thought
this is better.



 +
 +   boot_reg = sram_ns_base_addr + 0x1c + 4*cpu;

 __raw_writel(boot_addr, boot_reg);
 return 0;
 diff --git a/arch/arm/mach-exynos/include/mach/map.h
 b/arch/arm/mach-exynos/include/mach/map.h
 index 7b046b59d9ec..548269a60634 100644
 --- a/arch/arm/mach-exynos/include/mach/map.h
 +++ b/arch/arm/mach-exynos/include/mach/map.h
 @@ -23,13 +23,6 @@

   #include plat/map-s5p.h

 -#define EXYNOS4_PA_SYSRAM0 0x02025000
 -#define EXYNOS4_PA_SYSRAM1 0x0202
 -#define EXYNOS5_PA_SYSRAM  0x0202
 -#define EXYNOS4210_PA_SYSRAM_NS0x0203F000
 -#define EXYNOS4x12_PA_SYSRAM_NS0x0204F000
 -#define EXYNOS5250_PA_SYSRAM_NS0x0204F000
 -
   #define EXYNOS_PA_CHIPID  0x1000

   #define EXYNOS4_PA_SYSCON 0x1001
 diff --git a/arch/arm/mach-exynos/platsmp.c
 b/arch/arm/mach-exynos/platsmp.c
 index 03e5e9f94705..ccbcdd7b8a86 100644
 --- a/arch/arm/mach-exynos/platsmp.c
 +++ b/arch/arm/mach-exynos/platsmp.c
 @@ -20,6 +20,7 @@
   #include linux/jiffies.h
   #include linux/smp.h
   #include linux/io.h
 +#include linux/of_address.h

   #include asm/cacheflush.h
   #include asm/smp_plat.h
 @@ -33,11 +34,35 @@

   extern void exynos4_secondary_startup(void);

 +static

Re: [PATCH v2 1/2] ARM: EXYNOS: Map SYSRAM through generic SRAM bindings

2014-05-06 Thread Sachin Kamat

Hi Tomasz,

On 6 May 2014 23:39, Sachin Kamat sachin.ka...@linaro.org wrote:
 Hi Tomasz,

 On 6 May 2014 22:14, Tomasz Figa tomasz.f...@gmail.com wrote:
 Hi Sachin,

 Thanks for addressing the comments. I need to verify few things on Universal
 C210 board first, before I could give my Reviewed-by tag or further
 comments.

 I also have some general comments that I missed before, due to limited time
 for review. Please see inline.

 Thanks for your review.



 On 06.05.2014 10:10, Sachin Kamat wrote:

 Instead of hardcoding the SYSRAM details for each SoC,
 pass this information through device tree (DT) and make
 the code SoC agnostic. Generic SRAM bindings are used
 for achieving this.

 Signed-off-by: Sachin Kamat sachin.ka...@linaro.org
 Acked-by: Arnd Bergmann a...@arndb.de
 Acked-by: Heiko Stuebner he...@sntech.de
 ---
[snip]


   static int exynos_set_cpu_boot_addr(int cpu, unsigned long boot_addr)
   {
 -   void __iomem *boot_reg = S5P_VA_SYSRAM_NS + 0x1c + 4*cpu;
 +   void __iomem *boot_reg;
 +
 +   if (!sram_ns_base_addr)
 +   return 0;


 Shouldn't this return an error instead? I'm not sure which one would be
 appropriate, though, probably one of -ENODEV, -ENXIO or -EFAULT.

 IIRC, returning error here causes the system to hang and even primary
 cpu does not boot.
 Since any error or absence of sram nodes should atleast boot the
 primary CPU, I thought
 this is better.

Small correction. The above explanation was for the absence of the check.
Sorry about the confusion. Will add -ENODEV here.

-- 
With warm regards,
Sachin
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] ARM: EXYNOS: Map SYSRAM through generic SRAM bindings

2014-05-04 Thread Sachin Kamat

Hi Tomasz,

On 2 May 2014 23:24, Tomasz Figa tomasz.f...@gmail.com wrote:
 Hi Sachin,

 The whole series looks quite good,

Thanks :)

but I have one concern about support for
 Universal C210 board. Please see my comment inline.


 On 02.05.2014 07:06, Sachin Kamat wrote:

 Instead of hardcoding the SYSRAM details for each SoC,
 pass this information through device tree (DT) and make
 the code SoC agnostic. Generic SRAM bindings are used
 for achieving this.

 Signed-off-by: Sachin Kamat sachin.ka...@linaro.org
 Acked-by: Arnd Bergmann a...@arndb.de
 Acked-by: Heiko Stuebner he...@sntech.de
 ---
 This patch is based on linux next (next-20140501) on top of
 my Kconfig consolidation patch
 http://comments.gmane.org/gmane.linux.kernel.samsung-soc/28642

 Changes since v1:
 Type and presence of sram nodes is SoC/board dependent. V1 mandated the
 presence of both the nodes and used to return an error if one of the
 nodes was absent and thus fail the boot altogether. Removed this
 dependency.

 Tested on 4210/4412 Origen, 5250/5420 Arndale and SMDK5420 boards.
 ---
   arch/arm/Kconfig|1 +
   arch/arm/boot/dts/exynos4210-universal_c210.dts |   17 ++
   arch/arm/boot/dts/exynos4210.dtsi   |   18 +++
   arch/arm/boot/dts/exynos4x12.dtsi   |   18 +++
   arch/arm/boot/dts/exynos5250.dtsi   |   18 +++
   arch/arm/boot/dts/exynos5420.dtsi   |   18 +++
   arch/arm/mach-exynos/common.h   |1 +
   arch/arm/mach-exynos/exynos.c   |   64
 ---
   arch/arm/mach-exynos/firmware.c |8 ++-
   arch/arm/mach-exynos/include/mach/map.h |7 ---
   arch/arm/mach-exynos/platsmp.c  |   33 ++--
   11 files changed, 128 insertions(+), 75 deletions(-)

 diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
 index a6d19b1a..f66ea9453df9 100644
 --- a/arch/arm/Kconfig
 +++ b/arch/arm/Kconfig
 @@ -855,6 +855,7 @@ config ARCH_EXYNOS
 select S5P_DEV_MFC
 select SAMSUNG_DMADEV
 select SPARSE_IRQ
 +   select SRAM
 select USE_OF
 help
   Support for SAMSUNG's EXYNOS SoCs (EXYNOS4/5)
 diff --git a/arch/arm/boot/dts/exynos4210-universal_c210.dts
 b/arch/arm/boot/dts/exynos4210-universal_c210.dts
 index 63e34b24b04f..8d4de5c0d0c7 100644
 --- a/arch/arm/boot/dts/exynos4210-universal_c210.dts
 +++ b/arch/arm/boot/dts/exynos4210-universal_c210.dts
 @@ -28,6 +28,23 @@
 bootargs = console=ttySAC2,115200N8 root=/dev/mmcblk0p5
 rw rootwait earlyprintk panic=5 maxcpus=1;
 };

 +   sram@0202 {
 +   status = disabled;


 Here you just disable just the top level node of non-secure SYSRAM, but the
 sub-nodes are still present and enabled.

I was under the impression that disabling parent node would also
disable the sub-nodes.
I will disable all of them in this case.



 +   };
 +
 +   sram@02025000 {
 +   compatible = mmio-sram;
 +   reg = 0x02025000 0x1000;
 +   #address-cells = 1;
 +   #size-cells = 1;
 +   ranges = 0 0x02025000 0x1000;
 +
 +   smp-sram@0 {
 +   compatible = samsung,exynos4210-sram;
 +   reg = 0x0 0x1000;
 +   };
 +   };
 +
 mct@1005 {
 compatible = none;
 };


 [snip]


 diff --git a/arch/arm/mach-exynos/platsmp.c
 b/arch/arm/mach-exynos/platsmp.c
 index 03e5e9f94705..0aac03204f9f 100644
 --- a/arch/arm/mach-exynos/platsmp.c
 +++ b/arch/arm/mach-exynos/platsmp.c
 @@ -20,6 +20,7 @@
   #include linux/jiffies.h
   #include linux/smp.h
   #include linux/io.h
 +#include linux/of_address.h

   #include asm/cacheflush.h
   #include asm/smp_plat.h
 @@ -33,11 +34,33 @@

   extern void exynos4_secondary_startup(void);

 +static void __iomem *sram_base_addr;
 +void __iomem *sram_ns_base_addr;
 +
 +static void __init exynos_smp_prepare_sram(void)
 +{
 +   struct device_node *node;
 +
 +   node = of_find_compatible_node(NULL, NULL,
 samsung,exynos4210-sram);


 Now here you don't check whether the node is okay, so on Universal C210 it
 will pick just the first node with this compatible string,

Right. Missed that one.


 I think you should be using for_each_compatible_node() here, then check if
 the node is okay using of_devicE_is_available() and only then use this
 node to map the SYSRAM.

OK.



 +   if (node) {
 +   sram_base_addr = of_iomap(node, 0);
 +   if (!sram_base_addr)
 +   pr_err(Secondary CPU boot address not found\n);
 +   }
 +
 +   node = of_find_compatible_node(NULL, NULL,
 samsung,exynos4210-sram-ns);


 Same here.

OK.



 +   if (node) {
 +   sram_ns_base_addr = of_iomap(node, 0);
 +   if (!sram_ns_base_addr)
 +   pr_err(Secondary CPU boot address not

Re: [PATCH v2 1/2] ARM: EXYNOS: Map SYSRAM through generic SRAM bindings

2014-05-02 Thread Tomasz Figa


Hi Sachin,

The whole series looks quite good, but I have one concern about support 
for Universal C210 board. Please see my comment inline.


On 02.05.2014 07:06, Sachin Kamat wrote:

Instead of hardcoding the SYSRAM details for each SoC,
pass this information through device tree (DT) and make
the code SoC agnostic. Generic SRAM bindings are used
for achieving this.

Signed-off-by: Sachin Kamat sachin.ka...@linaro.org
Acked-by: Arnd Bergmann a...@arndb.de
Acked-by: Heiko Stuebner he...@sntech.de
---
This patch is based on linux next (next-20140501) on top of
my Kconfig consolidation patch
http://comments.gmane.org/gmane.linux.kernel.samsung-soc/28642

Changes since v1:
Type and presence of sram nodes is SoC/board dependent. V1 mandated the
presence of both the nodes and used to return an error if one of the
nodes was absent and thus fail the boot altogether. Removed this dependency.

Tested on 4210/4412 Origen, 5250/5420 Arndale and SMDK5420 boards.
---
  arch/arm/Kconfig|1 +
  arch/arm/boot/dts/exynos4210-universal_c210.dts |   17 ++
  arch/arm/boot/dts/exynos4210.dtsi   |   18 +++
  arch/arm/boot/dts/exynos4x12.dtsi   |   18 +++
  arch/arm/boot/dts/exynos5250.dtsi   |   18 +++
  arch/arm/boot/dts/exynos5420.dtsi   |   18 +++
  arch/arm/mach-exynos/common.h   |1 +
  arch/arm/mach-exynos/exynos.c   |   64 ---
  arch/arm/mach-exynos/firmware.c |8 ++-
  arch/arm/mach-exynos/include/mach/map.h |7 ---
  arch/arm/mach-exynos/platsmp.c  |   33 ++--
  11 files changed, 128 insertions(+), 75 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index a6d19b1a..f66ea9453df9 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -855,6 +855,7 @@ config ARCH_EXYNOS
select S5P_DEV_MFC
select SAMSUNG_DMADEV
select SPARSE_IRQ
+   select SRAM
select USE_OF
help
  Support for SAMSUNG's EXYNOS SoCs (EXYNOS4/5)
diff --git a/arch/arm/boot/dts/exynos4210-universal_c210.dts 
b/arch/arm/boot/dts/exynos4210-universal_c210.dts
index 63e34b24b04f..8d4de5c0d0c7 100644
--- a/arch/arm/boot/dts/exynos4210-universal_c210.dts
+++ b/arch/arm/boot/dts/exynos4210-universal_c210.dts
@@ -28,6 +28,23 @@
bootargs = console=ttySAC2,115200N8 root=/dev/mmcblk0p5 rw rootwait 
earlyprintk panic=5 maxcpus=1;
};

+   sram@0202 {
+   status = disabled;


Here you just disable just the top level node of non-secure SYSRAM, but 
the sub-nodes are still present and enabled.



+   };
+
+   sram@02025000 {
+   compatible = mmio-sram;
+   reg = 0x02025000 0x1000;
+   #address-cells = 1;
+   #size-cells = 1;
+   ranges = 0 0x02025000 0x1000;
+
+   smp-sram@0 {
+   compatible = samsung,exynos4210-sram;
+   reg = 0x0 0x1000;
+   };
+   };
+
mct@1005 {
compatible = none;
};


[snip]


diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c
index 03e5e9f94705..0aac03204f9f 100644
--- a/arch/arm/mach-exynos/platsmp.c
+++ b/arch/arm/mach-exynos/platsmp.c
@@ -20,6 +20,7 @@
  #include linux/jiffies.h
  #include linux/smp.h
  #include linux/io.h
+#include linux/of_address.h

  #include asm/cacheflush.h
  #include asm/smp_plat.h
@@ -33,11 +34,33 @@

  extern void exynos4_secondary_startup(void);

+static void __iomem *sram_base_addr;
+void __iomem *sram_ns_base_addr;
+
+static void __init exynos_smp_prepare_sram(void)
+{
+   struct device_node *node;
+
+   node = of_find_compatible_node(NULL, NULL, samsung,exynos4210-sram);


Now here you don't check whether the node is okay, so on Universal 
C210 it will pick just the first node with this compatible string,


I think you should be using for_each_compatible_node() here, then check 
if the node is okay using of_devicE_is_available() and only then use 
this node to map the SYSRAM.



+   if (node) {
+   sram_base_addr = of_iomap(node, 0);
+   if (!sram_base_addr)
+   pr_err(Secondary CPU boot address not found\n);
+   }
+
+   node = of_find_compatible_node(NULL, NULL, 
samsung,exynos4210-sram-ns);


Same here.


+   if (node) {
+   sram_ns_base_addr = of_iomap(node, 0);
+   if (!sram_ns_base_addr)
+   pr_err(Secondary CPU boot address not found\n);
+   }
+}
+
  static inline void __iomem *cpu_boot_reg_base(void)
  {
if (soc_is_exynos4210()  samsung_rev() == EXYNOS4210_REV_1_1)
return S5P_INFORM5;
-   return S5P_VA_SYSRAM;
+   return sram_base_addr;
  }

  static inline void __iomem *cpu_boot_reg(int cpu)
@@ -147,7 +170,8 @@ static int

[PATCH v2 1/2] ARM: EXYNOS: Map SYSRAM through generic SRAM bindings

2014-05-01 Thread Sachin Kamat

Instead of hardcoding the SYSRAM details for each SoC,
pass this information through device tree (DT) and make
the code SoC agnostic. Generic SRAM bindings are used
for achieving this.

Signed-off-by: Sachin Kamat sachin.ka...@linaro.org
Acked-by: Arnd Bergmann a...@arndb.de
Acked-by: Heiko Stuebner he...@sntech.de
---
This patch is based on linux next (next-20140501) on top of
my Kconfig consolidation patch
http://comments.gmane.org/gmane.linux.kernel.samsung-soc/28642

Changes since v1:
Type and presence of sram nodes is SoC/board dependent. V1 mandated the
presence of both the nodes and used to return an error if one of the
nodes was absent and thus fail the boot altogether. Removed this dependency.

Tested on 4210/4412 Origen, 5250/5420 Arndale and SMDK5420 boards.
---
 arch/arm/Kconfig|1 +
 arch/arm/boot/dts/exynos4210-universal_c210.dts |   17 ++
 arch/arm/boot/dts/exynos4210.dtsi   |   18 +++
 arch/arm/boot/dts/exynos4x12.dtsi   |   18 +++
 arch/arm/boot/dts/exynos5250.dtsi   |   18 +++
 arch/arm/boot/dts/exynos5420.dtsi   |   18 +++
 arch/arm/mach-exynos/common.h   |1 +
 arch/arm/mach-exynos/exynos.c   |   64 ---
 arch/arm/mach-exynos/firmware.c |8 ++-
 arch/arm/mach-exynos/include/mach/map.h |7 ---
 arch/arm/mach-exynos/platsmp.c  |   33 ++--
 11 files changed, 128 insertions(+), 75 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index a6d19b1a..f66ea9453df9 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -855,6 +855,7 @@ config ARCH_EXYNOS
select S5P_DEV_MFC
select SAMSUNG_DMADEV
select SPARSE_IRQ
+   select SRAM
select USE_OF
help
  Support for SAMSUNG's EXYNOS SoCs (EXYNOS4/5)
diff --git a/arch/arm/boot/dts/exynos4210-universal_c210.dts 
b/arch/arm/boot/dts/exynos4210-universal_c210.dts
index 63e34b24b04f..8d4de5c0d0c7 100644
--- a/arch/arm/boot/dts/exynos4210-universal_c210.dts
+++ b/arch/arm/boot/dts/exynos4210-universal_c210.dts
@@ -28,6 +28,23 @@
bootargs = console=ttySAC2,115200N8 root=/dev/mmcblk0p5 rw 
rootwait earlyprintk panic=5 maxcpus=1;
};
 
+   sram@0202 {
+   status = disabled;
+   };
+
+   sram@02025000 {
+   compatible = mmio-sram;
+   reg = 0x02025000 0x1000;
+   #address-cells = 1;
+   #size-cells = 1;
+   ranges = 0 0x02025000 0x1000;
+
+   smp-sram@0 {
+   compatible = samsung,exynos4210-sram;
+   reg = 0x0 0x1000;
+   };
+   };
+
mct@1005 {
compatible = none;
};
diff --git a/arch/arm/boot/dts/exynos4210.dtsi 
b/arch/arm/boot/dts/exynos4210.dtsi
index cacf6140dd2f..d3d727b0c263 100644
--- a/arch/arm/boot/dts/exynos4210.dtsi
+++ b/arch/arm/boot/dts/exynos4210.dtsi
@@ -31,6 +31,24 @@
pinctrl2 = pinctrl_2;
};
 
+   sram@0202 {
+   compatible = mmio-sram;
+   reg = 0x0202 0x2;
+   #address-cells = 1;
+   #size-cells = 1;
+   ranges = 0 0x0202 0x2;
+
+   smp-sram@0 {
+   compatible = samsung,exynos4210-sram;
+   reg = 0x0 0x1000;
+   };
+
+   smp-sram@1f000 {
+   compatible = samsung,exynos4210-sram-ns;
+   reg = 0x1f000 0x1000;
+   };
+   };
+
pd_lcd1: lcd1-power-domain@10023CA0 {
compatible = samsung,exynos4210-pd;
reg = 0x10023CA0 0x20;
diff --git a/arch/arm/boot/dts/exynos4x12.dtsi 
b/arch/arm/boot/dts/exynos4x12.dtsi
index c4a9306f8529..75fb3e7e3999 100644
--- a/arch/arm/boot/dts/exynos4x12.dtsi
+++ b/arch/arm/boot/dts/exynos4x12.dtsi
@@ -37,6 +37,24 @@
interrupts = 2 2, 3 2, 18 2, 19 2;
};
 
+   sram@0202 {
+   compatible = mmio-sram;
+   reg = 0x0202 0x4;
+   #address-cells = 1;
+   #size-cells = 1;
+   ranges = 0 0x0202 0x4;
+
+   smp-sram@0 {
+   compatible = samsung,exynos4210-sram;
+   reg = 0x0 0x1000;
+   };
+
+   smp-sram@2f000 {
+   compatible = samsung,exynos4210-sram-ns;
+   reg = 0x2f000 0x1000;
+   };
+   };
+
pd_isp: isp-power-domain@10023CA0 {
compatible = samsung,exynos4210-pd;
reg = 0x10023CA0 0x20;
diff --git a/arch/arm/boot/dts/exynos5250.dtsi 
b/arch/arm/boot/dts/exynos5250.dtsi
index 37423314a028..8d724d56a5c6 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@

Re: [PATCH v2 1/2] ARM: EXYNOS: Map SYSRAM through generic SRAM bindings

Re: [PATCH v2 1/2] ARM: EXYNOS: Map SYSRAM through generic SRAM bindings

[PATCH v2 1/2] ARM: EXYNOS: Map SYSRAM through generic SRAM bindings

Re: [PATCH v2 1/2] ARM: EXYNOS: Map SYSRAM through generic SRAM bindings

Re: [PATCH v2 1/2] ARM: EXYNOS: Map SYSRAM through generic SRAM bindings

Re: [PATCH v2 1/2] ARM: EXYNOS: Map SYSRAM through generic SRAM bindings

Re: [PATCH v2 1/2] ARM: EXYNOS: Map SYSRAM through generic SRAM bindings

[PATCH v2 1/2] ARM: EXYNOS: Map SYSRAM through generic SRAM bindings

8 matches

Site Navigation

Mail list logo

Footer information