[PATCH 0/4] Add SCU device node support for Exynos4

[PATCH 0/4] Add SCU device node support for Exynos4

[Pankaj Dubey]

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(-)

-- 
2.7.4

[PATCH 0/4] Add SCU device node support for Exynos4

[Pankaj Dubey]

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(-)

-- 
2.7.4

[PATCH 1/4] ARM: EXYNOS: Remove smp_init_cpus hook from platsmp.c

[Pankaj Dubey]

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.

Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
---
 arch/arm/mach-exynos/platsmp.c | 31 -------------------------------
 1 file changed, 31 deletions(-)

diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c
index 98ffe1e..a5d6841 100644
--- a/arch/arm/mach-exynos/platsmp.c
+++ b/arch/arm/mach-exynos/platsmp.c
@@ -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,
-- 
2.7.4

[PATCH 1/4] ARM: EXYNOS: Remove smp_init_cpus hook from platsmp.c

[Pankaj Dubey]

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.

Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
---
 arch/arm/mach-exynos/platsmp.c | 31 -------------------------------
 1 file changed, 31 deletions(-)

diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c
index 98ffe1e..a5d6841 100644
--- a/arch/arm/mach-exynos/platsmp.c
+++ b/arch/arm/mach-exynos/platsmp.c
@@ -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,
-- 
2.7.4

Re: [1/4] ARM: EXYNOS: Remove smp_init_cpus hook from platsmp.c

[Alim Akhtar]

Hi Pankaj,

On 11/04/2016 09:09 AM, Pankaj Dubey wrote:
> 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.
>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> ---
Looks good.
Reviewed-by: Alim Akhtar <alim.akhtar@samsung.com>

>   arch/arm/mach-exynos/platsmp.c | 31 -------------------------------
>   1 file changed, 31 deletions(-)
>
> diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c
> index 98ffe1e..a5d6841 100644
> --- a/arch/arm/mach-exynos/platsmp.c
> +++ b/arch/arm/mach-exynos/platsmp.c
> @@ -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,
>

[1/4] ARM: EXYNOS: Remove smp_init_cpus hook from platsmp.c

[Alim Akhtar]

Hi Pankaj,

On 11/04/2016 09:09 AM, Pankaj Dubey wrote:
> 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.
>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> ---
Looks good.
Reviewed-by: Alim Akhtar <alim.akhtar@samsung.com>

>   arch/arm/mach-exynos/platsmp.c | 31 -------------------------------
>   1 file changed, 31 deletions(-)
>
> diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c
> index 98ffe1e..a5d6841 100644
> --- a/arch/arm/mach-exynos/platsmp.c
> +++ b/arch/arm/mach-exynos/platsmp.c
> @@ -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,
>

Re: [PATCH 1/4] ARM: EXYNOS: Remove smp_init_cpus hook from platsmp.c

[Krzysztof Kozlowski]

On Fri, Nov 04, 2016 at 09:09:21AM +0530, Pankaj Dubey wrote:
> 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.
> 
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> ---
>  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

[Krzysztof Kozlowski]

On Fri, Nov 04, 2016 at 09:09:21AM +0530, Pankaj Dubey wrote:
> 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.
> 
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> ---
>  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

Re: [PATCH 1/4] ARM: EXYNOS: Remove smp_init_cpus hook from platsmp.c

[pankaj.dubey]

Hi Krzysztof,

On Saturday 05 November 2016 09:11 PM, Krzysztof Kozlowski wrote:
> On Fri, Nov 04, 2016 at 09:09:21AM +0530, Pankaj Dubey wrote:
>> 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.
>>
>> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
>> ---
>>  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.
> 

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

> Best regards,
> Krzysztof
> 
> 
> 
> 

[PATCH 1/4] ARM: EXYNOS: Remove smp_init_cpus hook from platsmp.c

[pankaj.dubey]

Hi Krzysztof,

On Saturday 05 November 2016 09:11 PM, Krzysztof Kozlowski wrote:
> On Fri, Nov 04, 2016 at 09:09:21AM +0530, Pankaj Dubey wrote:
>> 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.
>>
>> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
>> ---
>>  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.
> 

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

> Best regards,
> Krzysztof
> 
> 
> 
> 

Re: [PATCH 1/4] ARM: EXYNOS: Remove smp_init_cpus hook from platsmp.c

[Krzysztof Kozlowski]

On Mon, Nov 7, 2016 at 5:37 AM, pankaj.dubey <pankaj.dubey@samsung.com> wrote:
> Hi Krzysztof,
>
> On Saturday 05 November 2016 09:11 PM, Krzysztof Kozlowski wrote:
>> On Fri, Nov 04, 2016 at 09:09:21AM +0530, Pankaj Dubey wrote:
>>> 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.
>>>
>>> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
>>> ---
>>>  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.
>>
>
> 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...


> 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

[Krzysztof Kozlowski]

On Mon, Nov 7, 2016 at 5:37 AM, pankaj.dubey <pankaj.dubey@samsung.com> wrote:
> Hi Krzysztof,
>
> On Saturday 05 November 2016 09:11 PM, Krzysztof Kozlowski wrote:
>> On Fri, Nov 04, 2016 at 09:09:21AM +0530, Pankaj Dubey wrote:
>>> 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.
>>>
>>> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
>>> ---
>>>  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.
>>
>
> 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...


> 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

Re: [PATCH 1/4] ARM: EXYNOS: Remove smp_init_cpus hook from platsmp.c

[Pankaj Dubey]

Hi Krzysztof,

On 7 November 2016 at 12:29, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> On Mon, Nov 7, 2016 at 5:37 AM, pankaj.dubey <pankaj.dubey@samsung.com> wrote:
>> Hi Krzysztof,
>>
>> On Saturday 05 November 2016 09:11 PM, Krzysztof Kozlowski wrote:
>>> On Fri, Nov 04, 2016 at 09:09:21AM +0530, Pankaj Dubey wrote:
>>>> 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.
>>>>
>>>> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
>>>> ---
>>>>  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.
>>>
>>
>> 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...
>
>
>> 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
> Best regards,
> Krzysztof
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/4] ARM: EXYNOS: Remove smp_init_cpus hook from platsmp.c

[Pankaj Dubey]

Hi Krzysztof,

On 7 November 2016 at 12:29, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> On Mon, Nov 7, 2016 at 5:37 AM, pankaj.dubey <pankaj.dubey@samsung.com> wrote:
>> Hi Krzysztof,
>>
>> On Saturday 05 November 2016 09:11 PM, Krzysztof Kozlowski wrote:
>>> On Fri, Nov 04, 2016 at 09:09:21AM +0530, Pankaj Dubey wrote:
>>>> 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.
>>>>
>>>> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
>>>> ---
>>>>  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.
>>>
>>
>> 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...
>
>
>> 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
> 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

[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 <pankaj.dubey@samsung.com>
---
 arch/arm/boot/dts/exynos4.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/exynos4.dtsi b/arch/arm/boot/dts/exynos4.dtsi
index 5f034eb..6865ca9 100644
--- a/arch/arm/boot/dts/exynos4.dtsi
+++ b/arch/arm/boot/dts/exynos4.dtsi
@@ -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>;
-- 
2.7.4

[PATCH 2/4] ARM: dts: exynos: Add SCU device node to exynos4.dtsi

[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 <pankaj.dubey@samsung.com>
---
 arch/arm/boot/dts/exynos4.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/exynos4.dtsi b/arch/arm/boot/dts/exynos4.dtsi
index 5f034eb..6865ca9 100644
--- a/arch/arm/boot/dts/exynos4.dtsi
+++ b/arch/arm/boot/dts/exynos4.dtsi
@@ -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>;
-- 
2.7.4

Re: [2/4] ARM: dts: exynos: Add SCU device node to exynos4.dtsi

[Alim Akhtar]

Hi Pankaj,

On 11/04/2016 09:09 AM, Pankaj Dubey wrote:
> 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 <pankaj.dubey@samsung.com>
> ---

Reviewed-by: Alim Akhtar <alim.akhtar@samsung.com>

>   arch/arm/boot/dts/exynos4.dtsi | 5 +++++
>   1 file changed, 5 insertions(+)
>
> diff --git a/arch/arm/boot/dts/exynos4.dtsi b/arch/arm/boot/dts/exynos4.dtsi
> index 5f034eb..6865ca9 100644
> --- a/arch/arm/boot/dts/exynos4.dtsi
> +++ b/arch/arm/boot/dts/exynos4.dtsi
> @@ -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>;
>

[2/4] ARM: dts: exynos: Add SCU device node to exynos4.dtsi

[Alim Akhtar]

Hi Pankaj,

On 11/04/2016 09:09 AM, Pankaj Dubey wrote:
> 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 <pankaj.dubey@samsung.com>
> ---

Reviewed-by: Alim Akhtar <alim.akhtar@samsung.com>

>   arch/arm/boot/dts/exynos4.dtsi | 5 +++++
>   1 file changed, 5 insertions(+)
>
> diff --git a/arch/arm/boot/dts/exynos4.dtsi b/arch/arm/boot/dts/exynos4.dtsi
> index 5f034eb..6865ca9 100644
> --- a/arch/arm/boot/dts/exynos4.dtsi
> +++ b/arch/arm/boot/dts/exynos4.dtsi
> @@ -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>;
>

Re: [PATCH 2/4] ARM: dts: exynos: Add SCU device node to exynos4.dtsi

[Krzysztof Kozlowski]

On Fri, Nov 04, 2016 at 09:09:22AM +0530, Pankaj Dubey wrote:
> 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 <pankaj.dubey@samsung.com>
> ---
>  arch/arm/boot/dts/exynos4.dtsi | 5 +++++
>  1 file changed, 5 insertions(+)
> 

Thanks, applied.

Best regards,
Krzysztof

[PATCH 2/4] ARM: dts: exynos: Add SCU device node to exynos4.dtsi

[Krzysztof Kozlowski]

On Fri, Nov 04, 2016 at 09:09:22AM +0530, Pankaj Dubey wrote:
> 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 <pankaj.dubey@samsung.com>
> ---
>  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

[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 <pankaj.dubey@samsung.com>
---
 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");
+		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
diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c
index 487295f..60e6827 100644
--- a/arch/arm/mach-exynos/pm.c
+++ b/arch/arm/mach-exynos/pm.c
@@ -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();
 	}
diff --git a/arch/arm/mach-exynos/suspend.c b/arch/arm/mach-exynos/suspend.c
index 06332f6..7ab7e67 100644
--- a/arch/arm/mach-exynos/suspend.c
+++ b/arch/arm/mach-exynos/suspend.c
@@ -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)
diff --git a/arch/arm/plat-samsung/include/plat/map-s5p.h b/arch/arm/plat-samsung/include/plat/map-s5p.h
index 0fe2828..512ed1f 100644
--- a/arch/arm/plat-samsung/include/plat/map-s5p.h
+++ b/arch/arm/plat-samsung/include/plat/map-s5p.h
@@ -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)
-- 
2.7.4

[PATCH 3/4] ARM: EXYNOS: Remove static mapping of SCU SFR

[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 <pankaj.dubey@samsung.com>
---
 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");
+		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
diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c
index 487295f..60e6827 100644
--- a/arch/arm/mach-exynos/pm.c
+++ b/arch/arm/mach-exynos/pm.c
@@ -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();
 	}
diff --git a/arch/arm/mach-exynos/suspend.c b/arch/arm/mach-exynos/suspend.c
index 06332f6..7ab7e67 100644
--- a/arch/arm/mach-exynos/suspend.c
+++ b/arch/arm/mach-exynos/suspend.c
@@ -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)
diff --git a/arch/arm/plat-samsung/include/plat/map-s5p.h b/arch/arm/plat-samsung/include/plat/map-s5p.h
index 0fe2828..512ed1f 100644
--- a/arch/arm/plat-samsung/include/plat/map-s5p.h
+++ b/arch/arm/plat-samsung/include/plat/map-s5p.h
@@ -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)
-- 
2.7.4

Re: [3/4] ARM: EXYNOS: Remove static mapping of SCU SFR

[Alim Akhtar]

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 <pankaj.dubey@samsung.com>
> ---
>   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?

> +		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
> diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c
> index 487295f..60e6827 100644
> --- a/arch/arm/mach-exynos/pm.c
> +++ b/arch/arm/mach-exynos/pm.c
> @@ -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");
same as above

> +		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();
>   	}
> diff --git a/arch/arm/mach-exynos/suspend.c b/arch/arm/mach-exynos/suspend.c
> index 06332f6..7ab7e67 100644
> --- a/arch/arm/mach-exynos/suspend.c
> +++ b/arch/arm/mach-exynos/suspend.c
> @@ -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");
and here otherwise patch looks good.

> +		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)
> diff --git a/arch/arm/plat-samsung/include/plat/map-s5p.h b/arch/arm/plat-samsung/include/plat/map-s5p.h
> index 0fe2828..512ed1f 100644
> --- a/arch/arm/plat-samsung/include/plat/map-s5p.h
> +++ b/arch/arm/plat-samsung/include/plat/map-s5p.h
> @@ -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)
>

[3/4] ARM: EXYNOS: Remove static mapping of SCU SFR

[Alim Akhtar]

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 <pankaj.dubey@samsung.com>
> ---
>   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?

> +		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
> diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c
> index 487295f..60e6827 100644
> --- a/arch/arm/mach-exynos/pm.c
> +++ b/arch/arm/mach-exynos/pm.c
> @@ -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");
same as above

> +		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();
>   	}
> diff --git a/arch/arm/mach-exynos/suspend.c b/arch/arm/mach-exynos/suspend.c
> index 06332f6..7ab7e67 100644
> --- a/arch/arm/mach-exynos/suspend.c
> +++ b/arch/arm/mach-exynos/suspend.c
> @@ -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");
and here otherwise patch looks good.

> +		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)
> diff --git a/arch/arm/plat-samsung/include/plat/map-s5p.h b/arch/arm/plat-samsung/include/plat/map-s5p.h
> index 0fe2828..512ed1f 100644
> --- a/arch/arm/plat-samsung/include/plat/map-s5p.h
> +++ b/arch/arm/plat-samsung/include/plat/map-s5p.h
> @@ -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)
>

Re: [3/4] ARM: EXYNOS: Remove static mapping of SCU SFR

[pankaj.dubey]

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 <pankaj.dubey@samsung.com>
>> ---
>>   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.


Thanks,
Pankaj Dubey

[3/4] ARM: EXYNOS: Remove static mapping of SCU SFR

[pankaj.dubey]

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 <pankaj.dubey@samsung.com>
>> ---
>>   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.


Thanks,
Pankaj Dubey

Re: [3/4] ARM: EXYNOS: Remove static mapping of SCU SFR

[Alim Akhtar]

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 <pankaj.dubey@samsung.com>
>>> ---
>>>    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 <alim.akhtar@samsung.com>

>
> Thanks,
> Pankaj Dubey
>
>

[3/4] ARM: EXYNOS: Remove static mapping of SCU SFR

[Alim Akhtar]

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 <pankaj.dubey@samsung.com>
>>> ---
>>>    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 <alim.akhtar@samsung.com>

>
> Thanks,
> Pankaj Dubey
>
>

Re: [3/4] ARM: EXYNOS: Remove static mapping of SCU SFR

[Pankaj Dubey]

Hi Alim,

On 7 November 2016 at 10:19, Alim Akhtar <alim.akhtar@samsung.com> wrote:
>
> 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 <pankaj.dubey@samsung.com>
>>>> ---
>>>>    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 <alim.akhtar@samsung.com>
>

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 refcount
incremented 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

>>
>>
>> Thanks,
>> Pankaj Dubey
>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[3/4] ARM: EXYNOS: Remove static mapping of SCU SFR

[Pankaj Dubey]

Hi Alim,

On 7 November 2016 at 10:19, Alim Akhtar <alim.akhtar@samsung.com> wrote:
>
> 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 <pankaj.dubey@samsung.com>
>>>> ---
>>>>    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 <alim.akhtar@samsung.com>
>

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 refcount
incremented 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

>>
>>
>> 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

Re: [PATCH 3/4] ARM: EXYNOS: Remove static mapping of SCU SFR

[Krzysztof Kozlowski]

On Fri, Nov 04, 2016 at 09:09:23AM +0530, 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 <pankaj.dubey@samsung.com>
> ---
>  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");
> +		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

[Krzysztof Kozlowski]

On Fri, Nov 04, 2016 at 09:09:23AM +0530, 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 <pankaj.dubey@samsung.com>
> ---
>  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");
> +		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

Re: [PATCH 3/4] ARM: EXYNOS: Remove static mapping of SCU SFR

[pankaj.dubey]

Hi Krzysztof,

On Monday 07 November 2016 11:23 PM, Krzysztof Kozlowski wrote:
> On Fri, Nov 04, 2016 at 09:09:23AM +0530, 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 <pankaj.dubey@samsung.com>
>> ---
>>  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");
>> +		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.

> 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

> Best regards,
> Krzysztof
> 
> 
> 
> 

[PATCH 3/4] ARM: EXYNOS: Remove static mapping of SCU SFR

[pankaj.dubey]

Hi Krzysztof,

On Monday 07 November 2016 11:23 PM, Krzysztof Kozlowski wrote:
> On Fri, Nov 04, 2016 at 09:09:23AM +0530, 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 <pankaj.dubey@samsung.com>
>> ---
>>  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");
>> +		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.

> 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

> Best regards,
> Krzysztof
> 
> 
> 
> 

[PATCH 4/4] ARM: EXYNOS: Remove unused soc_is_exynos{4,5}

[Pankaj Dubey]

As no more user of soc_is_exynos{4,5} we can safely remove them.

Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
---
 arch/arm/mach-exynos/common.h | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
index 9424a8a..d19064b 100644
--- a/arch/arm/mach-exynos/common.h
+++ b/arch/arm/mach-exynos/common.h
@@ -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;
 
-- 
2.7.4

[PATCH 4/4] ARM: EXYNOS: Remove unused soc_is_exynos{4,5}

[Pankaj Dubey]

As no more user of soc_is_exynos{4,5} we can safely remove them.

Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
---
 arch/arm/mach-exynos/common.h | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
index 9424a8a..d19064b 100644
--- a/arch/arm/mach-exynos/common.h
+++ b/arch/arm/mach-exynos/common.h
@@ -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;
 
-- 
2.7.4

Re: [4/4] ARM: EXYNOS: Remove unused soc_is_exynos{4,5}

[Alim Akhtar]

Hi Pankaj,

On 11/04/2016 09:09 AM, Pankaj Dubey wrote:
> As no more user of soc_is_exynos{4,5} we can safely remove them.
>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> ---

Reviewed-by: Alim Akhtar <alim.akhtar@samsung.com>

Also I have complied tested this series, looks good.

>   arch/arm/mach-exynos/common.h | 5 -----
>   1 file changed, 5 deletions(-)
>
> diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
> index 9424a8a..d19064b 100644
> --- a/arch/arm/mach-exynos/common.h
> +++ b/arch/arm/mach-exynos/common.h
> @@ -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;
>
>

[4/4] ARM: EXYNOS: Remove unused soc_is_exynos{4,5}

[Alim Akhtar]

Hi Pankaj,

On 11/04/2016 09:09 AM, Pankaj Dubey wrote:
> As no more user of soc_is_exynos{4,5} we can safely remove them.
>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> ---

Reviewed-by: Alim Akhtar <alim.akhtar@samsung.com>

Also I have complied tested this series, looks good.

>   arch/arm/mach-exynos/common.h | 5 -----
>   1 file changed, 5 deletions(-)
>
> diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
> index 9424a8a..d19064b 100644
> --- a/arch/arm/mach-exynos/common.h
> +++ b/arch/arm/mach-exynos/common.h
> @@ -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;
>
>

Re: [PATCH 0/4] Add SCU device node support for Exynos4

[Marek Szyprowski]

Hi Pankaj,

On 2016-11-04 04:39, Pankaj Dubey wrote:
> 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(-)

Nice cleanup! Works fine on Exynos4412-based Odroid U3 (SMP boot and S2R).

Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

[PATCH 0/4] Add SCU device node support for Exynos4

[Marek Szyprowski]

Hi Pankaj,

On 2016-11-04 04:39, Pankaj Dubey wrote:
> 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(-)

Nice cleanup! Works fine on Exynos4412-based Odroid U3 (SMP boot and S2R).

Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland