[PATCH] ARM: smp: allow get the core count from L2 control on A15

[PATCH] ARM: smp: allow get the core count from L2 control on A15

[Kukjin Kim]

Actually, the number of A15 CPU core gets from L2 control
register not SCU configuration.

Suggested-by: Changhwan Youn <chaos.youn@samsung.com>
Signed-off-by: Kukjin Kim <kgene.kim@samsung.com>
Cc: Russell King <rmk+kernel@arm.linux.org.uk>
---

Hi Russell,

As you know, the method of A15 core counting is different with A9.
So the function will be required on that but I'm not sure where the
function should be added at.

 arch/arm/include/asm/smp.h |    5 +++++
 arch/arm/kernel/smp.c      |   16 ++++++++++++++++
 2 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/arch/arm/include/asm/smp.h b/arch/arm/include/asm/smp.h
index 1e5717a..b67084f 100644
--- a/arch/arm/include/asm/smp.h
+++ b/arch/arm/include/asm/smp.h
@@ -71,6 +71,11 @@ extern void platform_secondary_init(unsigned int cpu);
 extern void platform_smp_prepare_cpus(unsigned int);
 
 /*
+ * Get the number of CPU cores from the L2 control register on A15
+ */
+extern unsigned long a15_get_core_count(void);
+
+/*
  * Logical CPU mapping.
  */
 extern int __cpu_logical_map[NR_CPUS];
diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
index 57db122..be4d31d 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -233,6 +233,22 @@ void __ref cpu_die(void)
 }
 #endif /* CONFIG_HOTPLUG_CPU */
 
+/*
+ * Get the number of CPU cores from the L2 control register on A15
+ */
+unsigned long a15_get_core_count(void)
+{
+	unsigned long val;
+
+	/* Read L2 control register */
+	asm volatile("mrc p15, 1, %0, c9, c0, 2" : "=r"(val));
+
+	/* [25:24] of L2 control register means core count - 1 */
+	val = ((val >> 24) & 0x3) + 1;
+
+	return val;
+}
+
 int __cpu_logical_map[NR_CPUS];
 
 void __init smp_setup_processor_id(void)
-- 
1.7.1

[PATCH] ARM: smp: allow get the core count from L2 control on A15

[Will Deacon]

Hi Kukjin,

On Tue, Jan 31, 2012 at 12:11:10PM +0000, Kukjin Kim wrote:
> 
> Actually, the number of A15 CPU core gets from L2 control
> register not SCU configuration.
> 
> Suggested-by: Changhwan Youn <chaos.youn@samsung.com>
> Signed-off-by: Kukjin Kim <kgene.kim@samsung.com>
> Cc: Russell King <rmk+kernel@arm.linux.org.uk>
> ---

NAK.

> diff --git a/arch/arm/include/asm/smp.h b/arch/arm/include/asm/smp.h
> index 1e5717a..b67084f 100644
> --- a/arch/arm/include/asm/smp.h
> +++ b/arch/arm/include/asm/smp.h
> @@ -71,6 +71,11 @@ extern void platform_secondary_init(unsigned int cpu);
>  extern void platform_smp_prepare_cpus(unsigned int);
>  
>  /*
> + * Get the number of CPU cores from the L2 control register on A15
> + */
> +extern unsigned long a15_get_core_count(void);
> +
> +/*
>   * Logical CPU mapping.
>   */
>  extern int __cpu_logical_map[NR_CPUS];
> diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
> index 57db122..be4d31d 100644
> --- a/arch/arm/kernel/smp.c
> +++ b/arch/arm/kernel/smp.c
> @@ -233,6 +233,22 @@ void __ref cpu_die(void)
>  }
>  #endif /* CONFIG_HOTPLUG_CPU */
>  
> +/*
> + * Get the number of CPU cores from the L2 control register on A15
> + */
> +unsigned long a15_get_core_count(void)
> +{
> +	unsigned long val;
> +
> +	/* Read L2 control register */
> +	asm volatile("mrc p15, 1, %0, c9, c0, 2" : "=r"(val));
> +
> +	/* [25:24] of L2 control register means core count - 1 */
> +	val = ((val >> 24) & 0x3) + 1;
> +
> +	return val;
> +}

This doesn't belong in smp.c but, more importantly, this doesn't work for
multi-cluster configurations at all. Since all A15 implementations will be
on new platforms, the code will be device-tree only and so we should use
that to determine the CPU topology as, unfortunately, there is no architected
way of doing this.

I believe Lorenzo posted some patches which you could look at.

Will

[PATCH] ARM: smp: allow get the core count from L2 control on A15

[Kukjin Kim]

On 01/31/12 23:13, Will Deacon wrote:
> Hi Kukjin,
>
> On Tue, Jan 31, 2012 at 12:11:10PM +0000, Kukjin Kim wrote:
>>
>> Actually, the number of A15 CPU core gets from L2 control
>> register not SCU configuration.
>>
>> Suggested-by: Changhwan Youn<chaos.youn@samsung.com>
>> Signed-off-by: Kukjin Kim<kgene.kim@samsung.com>
>> Cc: Russell King<rmk+kernel@arm.linux.org.uk>
>> ---
>
> NAK.
>
>> diff --git a/arch/arm/include/asm/smp.h b/arch/arm/include/asm/smp.h
>> index 1e5717a..b67084f 100644
>> --- a/arch/arm/include/asm/smp.h
>> +++ b/arch/arm/include/asm/smp.h
>> @@ -71,6 +71,11 @@ extern void platform_secondary_init(unsigned int cpu);
>>   extern void platform_smp_prepare_cpus(unsigned int);
>>
>>   /*
>> + * Get the number of CPU cores from the L2 control register on A15
>> + */
>> +extern unsigned long a15_get_core_count(void);
>> +
>> +/*
>>    * Logical CPU mapping.
>>    */
>>   extern int __cpu_logical_map[NR_CPUS];
>> diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
>> index 57db122..be4d31d 100644
>> --- a/arch/arm/kernel/smp.c
>> +++ b/arch/arm/kernel/smp.c
>> @@ -233,6 +233,22 @@ void __ref cpu_die(void)
>>   }
>>   #endif /* CONFIG_HOTPLUG_CPU */
>>
>> +/*
>> + * Get the number of CPU cores from the L2 control register on A15
>> + */
>> +unsigned long a15_get_core_count(void)
>> +{
>> +	unsigned long val;
>> +
>> +	/* Read L2 control register */
>> +	asm volatile("mrc p15, 1, %0, c9, c0, 2" : "=r"(val));
>> +
>> +	/* [25:24] of L2 control register means core count - 1 */
>> +	val = ((val>>  24)&  0x3) + 1;
>> +
>> +	return val;
>> +}
>
> This doesn't belong in smp.c but, more importantly, this doesn't work for
> multi-cluster configurations at all. Since all A15 implementations will be
> on new platforms, the code will be device-tree only and so we should use

Why not? As I know, current arm kernel ARMv7 arch can support A15 
without device-tree. And you know, the core number should be counted by 
L2 control register. no?

> that to determine the CPU topology as, unfortunately, there is no architected
> way of doing this.
>
> I believe Lorenzo posted some patches which you could look at.
>
OK, Would be better.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

[PATCH] ARM: smp: allow get the core count from L2 control on A15

[Will Deacon]

On Tue, Jan 31, 2012 at 02:21:28PM +0000, Kukjin Kim wrote:
> On 01/31/12 23:13, Will Deacon wrote:
> >
> > This doesn't belong in smp.c but, more importantly, this doesn't work for
> > multi-cluster configurations at all. Since all A15 implementations will be
> > on new platforms, the code will be device-tree only and so we should use
> 
> Why not? As I know, current arm kernel ARMv7 arch can support A15 
> without device-tree. And you know, the core number should be counted by 
> L2 control register. no?

I'll answer these in order:

1.) The code doesn't belong in smp.c because it's specific to the A15
2.) The architecture code may well support A15, but since there are no
    platforms in mainline that support A15 yet, then all new platforms will
    need to be DT-based. That means we can rely on the DT to provide this
    information.
3.) The L2 control register only tells you how many cores are hanging off
    that particular L2. This will be wrong for multi-cluster systems since
    it will only identify a subset of the cores.

> > I believe Lorenzo posted some patches which you could look at.
> >
> OK, Would be better.

Yes, I think it's the only way to solve this problem without adding an
architected method for enumerating the CPU topology.

Will

[PATCH] ARM: smp: allow get the core count from L2 control on A15

[Kukjin Kim]

On 01/31/12 23:32, Will Deacon wrote:
> On Tue, Jan 31, 2012 at 02:21:28PM +0000, Kukjin Kim wrote:
>> On 01/31/12 23:13, Will Deacon wrote:
>>>
>>> This doesn't belong in smp.c but, more importantly, this doesn't work for
>>> multi-cluster configurations at all. Since all A15 implementations will be
>>> on new platforms, the code will be device-tree only and so we should use
>>
>> Why not? As I know, current arm kernel ARMv7 arch can support A15
>> without device-tree. And you know, the core number should be counted by
>> L2 control register. no?
>
> I'll answer these in order:

Hmm, let
>
> 1.) The code doesn't belong in smp.c because it's specific to the A15

Yes, agree. So I just commented in my patch, I'm not sure where it 
should be added at.

> 2.) The architecture code may well support A15, but since there are no
>      platforms in mainline that support A15 yet, then all new platforms will
>      need to be DT-based. That means we can rely on the DT to provide this
>      information.

Well, I will submit EXYNOS5 which has two A15 cores today and I don't 
know why it should be supported only with DT, EXYNOS5 DT supporting will 
be submitted though.

> 3.) The L2 control register only tells you how many cores are hanging off
>      that particular L2. This will be wrong for multi-cluster systems since
>      it will only identify a subset of the cores.

OK, let me check it.

>
>>> I believe Lorenzo posted some patches which you could look at.
>>>
>> OK, Would be better.
>
> Yes, I think it's the only way to solve this problem without adding an
> architected method for enumerating the CPU topology.
>
I'm not sure it is the only way...

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

[PATCH] ARM: smp: allow get the core count from L2 control on A15

[Lorenzo Pieralisi]

On Tue, Jan 31, 2012 at 02:40:41PM +0000, Kukjin Kim wrote:
> On 01/31/12 23:32, Will Deacon wrote:
> > On Tue, Jan 31, 2012 at 02:21:28PM +0000, Kukjin Kim wrote:
> >> On 01/31/12 23:13, Will Deacon wrote:
> >>>
> >>> This doesn't belong in smp.c but, more importantly, this doesn't work for
> >>> multi-cluster configurations at all. Since all A15 implementations will be
> >>> on new platforms, the code will be device-tree only and so we should use
> >>
> >> Why not? As I know, current arm kernel ARMv7 arch can support A15
> >> without device-tree. And you know, the core number should be counted by
> >> L2 control register. no?
> >
> > I'll answer these in order:
> 
> Hmm, let
> >
> > 1.) The code doesn't belong in smp.c because it's specific to the A15
> 
> Yes, agree. So I just commented in my patch, I'm not sure where it 
> should be added at.

I do not think it should be added at all. And if you add it at least add
it to your BSP code (not in smp.c), because it works for a single cluster A15 
configuration, it does not for other A15 based systems. 
I think Will's comment was pretty clear about this.

> > 2.) The architecture code may well support A15, but since there are no
> >      platforms in mainline that support A15 yet, then all new platforms will
> >      need to be DT-based. That means we can rely on the DT to provide this
> >      information.
> 
> Well, I will submit EXYNOS5 which has two A15 cores today and I don't 
> know why it should be supported only with DT, EXYNOS5 DT supporting will 
> be submitted though.
> 

See above.

> > 3.) The L2 control register only tells you how many cores are hanging off
> >      that particular L2. This will be wrong for multi-cluster systems since
> >      it will only identify a subset of the cores.
> 
> OK, let me check it.

Check it and you will notice that it does not work for multi-cluster SMP
systems.

> >
> >>> I believe Lorenzo posted some patches which you could look at.
> >>>
> >> OK, Would be better.
> >
> > Yes, I think it's the only way to solve this problem without adding an
> > architected method for enumerating the CPU topology.
> >
> I'm not sure it is the only way...

We are not stating that it is the only way to do it. We are saying it is the
only way to do it in a generic way without relying on a per platform hook (if 
it exists).
If it does not and we rely on this patch, we end up booting only one cluster out
of the possible ones, which is awesome.

I am against adding this code for the aforementioned reasons.

Lorenzo

[PATCH] ARM: smp: allow get the core count from L2 control on A15

[Russell King - ARM Linux]

On Tue, Jan 31, 2012 at 09:11:10PM +0900, Kukjin Kim wrote:
> diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
> index 57db122..be4d31d 100644
> --- a/arch/arm/kernel/smp.c
> +++ b/arch/arm/kernel/smp.c
> @@ -233,6 +233,22 @@ void __ref cpu_die(void)
>  }
>  #endif /* CONFIG_HOTPLUG_CPU */
>  
> +/*
> + * Get the number of CPU cores from the L2 control register on A15
> + */
> +unsigned long a15_get_core_count(void)
> +{
> +	unsigned long val;
> +
> +	/* Read L2 control register */
> +	asm volatile("mrc p15, 1, %0, c9, c0, 2" : "=r"(val));
> +
> +	/* [25:24] of L2 control register means core count - 1 */
> +	val = ((val >> 24) & 0x3) + 1;
> +
> +	return val;
> +}
> +

This is *definitely* the wrong place for this.  Do we have the reading
of the number of cores from the SCU in smp.c ?  No.  Do we want to litter
smp.c with each architecture revisions own way of reading this information?
No.

Keep this kind of crap out of the main smp.c file please.  It contains
zero CPU and platform specifics and should stay that way.