[PATCH 1/1] ARM: Add API to detect SCU base address from CP15

[PATCH 1/1] ARM: Add API to detect SCU base address from CP15

[Hiroshi Doyu]

Add API to detect SCU base address from CP15.

Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
---
NOTE:
This wasn't delivered to linux-arm-kernel at lists.infradead.org, resending....

For usage: http://patchwork.ozlabs.org/patch/212013/
---
 arch/arm/include/asm/smp_scu.h |   17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/arch/arm/include/asm/smp_scu.h b/arch/arm/include/asm/smp_scu.h
index 4eb6d00..f619eef 100644
--- a/arch/arm/include/asm/smp_scu.h
+++ b/arch/arm/include/asm/smp_scu.h
@@ -6,6 +6,23 @@
 #define SCU_PM_POWEROFF	3
 
 #ifndef __ASSEMBLER__
+
+#include <asm/cputype.h>
+
+static inline phys_addr_t scu_get_base(void)
+{
+	phys_addr_t pa;
+	unsigned long part_number = read_cpuid_part_number();
+
+	switch (part_number) {
+	case ARM_CPU_PART_CORTEX_A9:
+		/* Get SCU physical base */
+		asm("mrc p15, 4, %0, c15, c0, 0" : "=r" (pa));
+		return pa;
+	default:
+		return 0;
+	}
+}
 unsigned int scu_get_core_count(void __iomem *);
 void scu_enable(void __iomem *);
 int scu_power_mode(void __iomem *, unsigned int);
-- 
1.7.9.5

[PATCH 1/1] ARM: Add API to detect SCU base address from CP15

[Santosh Shilimkar]

On Friday 18 January 2013 04:29 PM, Hiroshi Doyu wrote:
> Add API to detect SCU base address from CP15.
>
> Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
> ---
> NOTE:
> This wasn't delivered to linux-arm-kernel at lists.infradead.org, resending....
>
> For usage: http://patchwork.ozlabs.org/patch/212013/
> ---
>   arch/arm/include/asm/smp_scu.h |   17 +++++++++++++++++
>   1 file changed, 17 insertions(+)
>
> diff --git a/arch/arm/include/asm/smp_scu.h b/arch/arm/include/asm/smp_scu.h
> index 4eb6d00..f619eef 100644
> --- a/arch/arm/include/asm/smp_scu.h
> +++ b/arch/arm/include/asm/smp_scu.h
> @@ -6,6 +6,23 @@
>   #define SCU_PM_POWEROFF	3
>
>   #ifndef __ASSEMBLER__
> +
> +#include <asm/cputype.h>
> +
> +static inline phys_addr_t scu_get_base(void)
> +{
> +	phys_addr_t pa;
> +	unsigned long part_number = read_cpuid_part_number();
> +
> +	switch (part_number) {
> +	case ARM_CPU_PART_CORTEX_A9:
> +		/* Get SCU physical base */
> +		asm("mrc p15, 4, %0, c15, c0, 0" : "=r" (pa));
> +		return pa;
> +	default:
> +		return 0;
> +	}
> +}
You may not need the switch case considering peripheral SCU is
specific to A9 SOCs. Would just if like below is better ?

phys_addr_t pa = 0;

if (ARM_CPU_PART_CORTEX_A9 == read_cpuid_part_number())
	asm("mrc p15, 4, %0, c15, c0, 0" : "=r" (pa));
return pa;

Regards,
Santosh
	

[PATCH 1/1] ARM: Add API to detect SCU base address from CP15

[Hiroshi Doyu]

On Fri, 18 Jan 2013 13:54:34 +0100
Santosh Shilimkar <santosh.shilimkar@ti.com> wrote:

> On Friday 18 January 2013 04:29 PM, Hiroshi Doyu wrote:
> > Add API to detect SCU base address from CP15.
> >
> > Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
> > ---
> > NOTE:
> > This wasn't delivered to linux-arm-kernel at lists.infradead.org, resending....
> >
> > For usage: http://patchwork.ozlabs.org/patch/212013/
> > ---
> >   arch/arm/include/asm/smp_scu.h |   17 +++++++++++++++++
> >   1 file changed, 17 insertions(+)
> >
> > diff --git a/arch/arm/include/asm/smp_scu.h b/arch/arm/include/asm/smp_scu.h
> > index 4eb6d00..f619eef 100644
> > --- a/arch/arm/include/asm/smp_scu.h
> > +++ b/arch/arm/include/asm/smp_scu.h
> > @@ -6,6 +6,23 @@
> >   #define SCU_PM_POWEROFF	3
> >
> >   #ifndef __ASSEMBLER__
> > +
> > +#include <asm/cputype.h>
> > +
> > +static inline phys_addr_t scu_get_base(void)
> > +{
> > +	phys_addr_t pa;
> > +	unsigned long part_number = read_cpuid_part_number();
> > +
> > +	switch (part_number) {
> > +	case ARM_CPU_PART_CORTEX_A9:
> > +		/* Get SCU physical base */
> > +		asm("mrc p15, 4, %0, c15, c0, 0" : "=r" (pa));
> > +		return pa;
> > +	default:
> > +		return 0;
> > +	}
> > +}
> You may not need the switch case considering peripheral SCU is
> specific to A9 SOCs. Would just if like below is better ?
> 
> phys_addr_t pa = 0;
> 
> if (ARM_CPU_PART_CORTEX_A9 == read_cpuid_part_number())
> 	asm("mrc p15, 4, %0, c15, c0, 0" : "=r" (pa));
> return pa;

I just considered the case if there will be another A?, which is SCU
detectable, added later. If no possibility, yours would be enough.

[PATCH 1/1] ARM: Add API to detect SCU base address from CP15

[Santosh Shilimkar]

On Friday 18 January 2013 07:59 PM, Hiroshi Doyu wrote:
> On Fri, 18 Jan 2013 13:54:34 +0100
> Santosh Shilimkar <santosh.shilimkar@ti.com> wrote:
>
>> On Friday 18 January 2013 04:29 PM, Hiroshi Doyu wrote:
>>> Add API to detect SCU base address from CP15.
>>>
>>> Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
>>> ---
>>> NOTE:
>>> This wasn't delivered to linux-arm-kernel at lists.infradead.org, resending....
>>>
>>> For usage: http://patchwork.ozlabs.org/patch/212013/
>>> ---
>>>    arch/arm/include/asm/smp_scu.h |   17 +++++++++++++++++
>>>    1 file changed, 17 insertions(+)
>>>
>>> diff --git a/arch/arm/include/asm/smp_scu.h b/arch/arm/include/asm/smp_scu.h
>>> index 4eb6d00..f619eef 100644
>>> --- a/arch/arm/include/asm/smp_scu.h
>>> +++ b/arch/arm/include/asm/smp_scu.h
>>> @@ -6,6 +6,23 @@
>>>    #define SCU_PM_POWEROFF	3
>>>
>>>    #ifndef __ASSEMBLER__
>>> +
>>> +#include <asm/cputype.h>
>>> +
>>> +static inline phys_addr_t scu_get_base(void)
>>> +{
>>> +	phys_addr_t pa;
>>> +	unsigned long part_number = read_cpuid_part_number();
>>> +
>>> +	switch (part_number) {
>>> +	case ARM_CPU_PART_CORTEX_A9:
>>> +		/* Get SCU physical base */
>>> +		asm("mrc p15, 4, %0, c15, c0, 0" : "=r" (pa));
>>> +		return pa;
>>> +	default:
>>> +		return 0;
>>> +	}
>>> +}
>> You may not need the switch case considering peripheral SCU is
>> specific to A9 SOCs. Would just if like below is better ?
>>
>> phys_addr_t pa = 0;
>>
>> if (ARM_CPU_PART_CORTEX_A9 == read_cpuid_part_number())
>> 	asm("mrc p15, 4, %0, c15, c0, 0" : "=r" (pa));
>> return pa;
>
> I just considered the case if there will be another A?, which is SCU
> detectable, added later. If no possibility, yours would be enough.
>
We can convert if into switch case if we need that in
future :-) For now if() should be just fine. Feel
free add my ack on updated patch.

Regards,
Santosh

Re: [PATCH 1/1] ARM: Add API to detect SCU base address from CP15

[Stephen Warren]

On 01/18/2013 03:59 AM, Hiroshi Doyu wrote:
> Add API to detect SCU base address from CP15.

So this patch appears to be a dependency for the Tegra114 series from
Hiroshi. As such, I need to get it into the Tegra tree somehow.

What I'd like to propose is that I put this one patch in a topic branch
based on v3.8-rcN, send a pull request to arm-soc containing that
branch, and then merge the branch into the Tegra tree. This will allow
anyone else to use the patch in code destined for 3.9.

Or, if nobody else wants to use this function, I could just apply it
directly to the Tegra tree.

But either way, I'd need an ack from a core ARM maintainer in order to
apply this (well, v2 once it's posted).

[PATCH 1/1] ARM: Add API to detect SCU base address from CP15

[Stephen Warren]

On 01/18/2013 03:59 AM, Hiroshi Doyu wrote:
> Add API to detect SCU base address from CP15.

So this patch appears to be a dependency for the Tegra114 series from
Hiroshi. As such, I need to get it into the Tegra tree somehow.

What I'd like to propose is that I put this one patch in a topic branch
based on v3.8-rcN, send a pull request to arm-soc containing that
branch, and then merge the branch into the Tegra tree. This will allow
anyone else to use the patch in code destined for 3.9.

Or, if nobody else wants to use this function, I could just apply it
directly to the Tegra tree.

But either way, I'd need an ack from a core ARM maintainer in order to
apply this (well, v2 once it's posted).

[v2 1/1] ARM: Add API to detect SCU base address from CP15

[Hiroshi Doyu]

Add API to detect SCU base address from CP15.

Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Acked-by: Santosh Shilimkar <santosh.shilimkar-l0cyMroinI0@public.gmane.org>
---
For usage: http://patchwork.ozlabs.org/patch/212013/
---
 arch/arm/include/asm/smp_scu.h |   12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm/include/asm/smp_scu.h b/arch/arm/include/asm/smp_scu.h
index 4eb6d00..1733ec7 100644
--- a/arch/arm/include/asm/smp_scu.h
+++ b/arch/arm/include/asm/smp_scu.h
@@ -6,6 +6,18 @@
 #define SCU_PM_POWEROFF	3
 
 #ifndef __ASSEMBLER__
+
+#include <asm/cputype.h>
+
+static inline phys_addr_t scu_get_base(void)
+{
+	if (read_cpuid_part_number() == ARM_CPU_PART_CORTEX_A9) {
+		phys_addr_t pa;
+		asm("mrc p15, 4, %0, c15, c0, 0" : "=r" (pa));
+		return pa;
+	}
+	return 0;
+}
 unsigned int scu_get_core_count(void __iomem *);
 void scu_enable(void __iomem *);
 int scu_power_mode(void __iomem *, unsigned int);
-- 
1.7.9.5

[v2 1/1] ARM: Add API to detect SCU base address from CP15

[Hiroshi Doyu]

Add API to detect SCU base address from CP15.

Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
---
For usage: http://patchwork.ozlabs.org/patch/212013/
---
 arch/arm/include/asm/smp_scu.h |   12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm/include/asm/smp_scu.h b/arch/arm/include/asm/smp_scu.h
index 4eb6d00..1733ec7 100644
--- a/arch/arm/include/asm/smp_scu.h
+++ b/arch/arm/include/asm/smp_scu.h
@@ -6,6 +6,18 @@
 #define SCU_PM_POWEROFF	3
 
 #ifndef __ASSEMBLER__
+
+#include <asm/cputype.h>
+
+static inline phys_addr_t scu_get_base(void)
+{
+	if (read_cpuid_part_number() == ARM_CPU_PART_CORTEX_A9) {
+		phys_addr_t pa;
+		asm("mrc p15, 4, %0, c15, c0, 0" : "=r" (pa));
+		return pa;
+	}
+	return 0;
+}
 unsigned int scu_get_core_count(void __iomem *);
 void scu_enable(void __iomem *);
 int scu_power_mode(void __iomem *, unsigned int);
-- 
1.7.9.5

Re: [v2 1/1] ARM: Add API to detect SCU base address from CP15

[Russell King - ARM Linux]

On Mon, Jan 21, 2013 at 09:42:55AM +0200, Hiroshi Doyu wrote:
> Add API to detect SCU base address from CP15.
> 
> Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> Acked-by: Santosh Shilimkar <santosh.shilimkar-l0cyMroinI0@public.gmane.org>
> ---
> For usage: http://patchwork.ozlabs.org/patch/212013/
> ---
>  arch/arm/include/asm/smp_scu.h |   12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/arch/arm/include/asm/smp_scu.h b/arch/arm/include/asm/smp_scu.h
> index 4eb6d00..1733ec7 100644
> --- a/arch/arm/include/asm/smp_scu.h
> +++ b/arch/arm/include/asm/smp_scu.h
> @@ -6,6 +6,18 @@
>  #define SCU_PM_POWEROFF	3
>  
>  #ifndef __ASSEMBLER__
> +
> +#include <asm/cputype.h>
> +
> +static inline phys_addr_t scu_get_base(void)
> +{
> +	if (read_cpuid_part_number() == ARM_CPU_PART_CORTEX_A9) {
> +		phys_addr_t pa;
> +		asm("mrc p15, 4, %0, c15, c0, 0" : "=r" (pa));
> +		return pa;
> +	}
> +	return 0;
> +}
>  unsigned int scu_get_core_count(void __iomem *);
>  void scu_enable(void __iomem *);
>  int scu_power_mode(void __iomem *, unsigned int);

Not sure what iteration this patch is at but... it's easy to avoid more
iterations when you review the patch yourself before sending.

Reasonable coding style suggests there should be a blank line after the
new } and before the prototypes.

However, as I _am_ commenting on this patch because of the above, I'll
also suggest that we don't do it like this.  And actually, the above
code is buggy.  If phys_addr_t is 64-bit, the upper half of 'pa' won't
be set.

I'd suggest this instead:

static inline bool scu_a9_has_base(void)
{
	return read_cpuid_part_number() == ARM_CPU_PART_CORTEX_A9;
}

static inline unsigned long scu_a9_get_base(void)
{
	unsigned long pa;

	asm("mrc p15, 4, %0, c15, c0, 0" : "=r" (pa));

	return pa;
}

and let the user of these functions decide whether to read it using
scu_a9_has_base().

And why 'unsigned long' ?  Well, it could also be u32, but on 32-bit
ARM, it's been more conventional to use unsigned long for phys addresses
which can't be larger than 32-bit - which this one can't because the
mrc instruction is limited to writing one 32-bit register.

[v2 1/1] ARM: Add API to detect SCU base address from CP15

[Russell King - ARM Linux]

On Mon, Jan 21, 2013 at 09:42:55AM +0200, Hiroshi Doyu wrote:
> Add API to detect SCU base address from CP15.
> 
> Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
> Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> ---
> For usage: http://patchwork.ozlabs.org/patch/212013/
> ---
>  arch/arm/include/asm/smp_scu.h |   12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/arch/arm/include/asm/smp_scu.h b/arch/arm/include/asm/smp_scu.h
> index 4eb6d00..1733ec7 100644
> --- a/arch/arm/include/asm/smp_scu.h
> +++ b/arch/arm/include/asm/smp_scu.h
> @@ -6,6 +6,18 @@
>  #define SCU_PM_POWEROFF	3
>  
>  #ifndef __ASSEMBLER__
> +
> +#include <asm/cputype.h>
> +
> +static inline phys_addr_t scu_get_base(void)
> +{
> +	if (read_cpuid_part_number() == ARM_CPU_PART_CORTEX_A9) {
> +		phys_addr_t pa;
> +		asm("mrc p15, 4, %0, c15, c0, 0" : "=r" (pa));
> +		return pa;
> +	}
> +	return 0;
> +}
>  unsigned int scu_get_core_count(void __iomem *);
>  void scu_enable(void __iomem *);
>  int scu_power_mode(void __iomem *, unsigned int);

Not sure what iteration this patch is at but... it's easy to avoid more
iterations when you review the patch yourself before sending.

Reasonable coding style suggests there should be a blank line after the
new } and before the prototypes.

However, as I _am_ commenting on this patch because of the above, I'll
also suggest that we don't do it like this.  And actually, the above
code is buggy.  If phys_addr_t is 64-bit, the upper half of 'pa' won't
be set.

I'd suggest this instead:

static inline bool scu_a9_has_base(void)
{
	return read_cpuid_part_number() == ARM_CPU_PART_CORTEX_A9;
}

static inline unsigned long scu_a9_get_base(void)
{
	unsigned long pa;

	asm("mrc p15, 4, %0, c15, c0, 0" : "=r" (pa));

	return pa;
}

and let the user of these functions decide whether to read it using
scu_a9_has_base().

And why 'unsigned long' ?  Well, it could also be u32, but on 32-bit
ARM, it's been more conventional to use unsigned long for phys addresses
which can't be larger than 32-bit - which this one can't because the
mrc instruction is limited to writing one 32-bit register.

Re: [v2 1/1] ARM: Add API to detect SCU base address from CP15

[Hiroshi Doyu]

Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote @ Mon, 21 Jan 2013 08:42:55 +0100:

> Add API to detect SCU base address from CP15.
> 
> Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> Acked-by: Santosh Shilimkar <santosh.shilimkar-l0cyMroinI0@public.gmane.org>
> ---
> For usage: http://patchwork.ozlabs.org/patch/212013/
> ---
>  arch/arm/include/asm/smp_scu.h |   12 ++++++++++++
>  1 file changed, 12 insertions(+)

Please ignore this series. The later version has been already queued for v3.9.

[v2 1/1] ARM: Add API to detect SCU base address from CP15

[Hiroshi Doyu]

Hiroshi Doyu <hdoyu@nvidia.com> wrote @ Mon, 21 Jan 2013 08:42:55 +0100:

> Add API to detect SCU base address from CP15.
> 
> Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
> Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> ---
> For usage: http://patchwork.ozlabs.org/patch/212013/
> ---
>  arch/arm/include/asm/smp_scu.h |   12 ++++++++++++
>  1 file changed, 12 insertions(+)

Please ignore this series. The later version has been already queued for v3.9.