diff mbox

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

Message ID 1478230764-13748-2-git-send-email-pankaj.dubey@samsung.com (mailing list archive)
State Accepted
Headers show

Commit Message

Pankaj Dubey Nov. 4, 2016, 3:39 a.m. UTC
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(-)

Comments

Alim Akhtar Nov. 4, 2016, 12:53 p.m. UTC | #1
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,
>
--
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
Krzysztof Kozlowski Nov. 5, 2016, 3:41 p.m. UTC | #2
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

--
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
Pankaj Dubey Nov. 7, 2016, 3:37 a.m. UTC | #3
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
> 
> 
> 
> 
--
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
Krzysztof Kozlowski Nov. 7, 2016, 6:59 a.m. UTC | #4
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
--
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
Pankaj Dubey Nov. 7, 2016, 5:10 p.m. UTC | #5
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
--
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
diff mbox

Patch

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,