linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/1] ARM: Add API to detect SCU base address from CP15
@ 2013-01-18 10:59 Hiroshi Doyu
  2013-01-18 12:54 ` Santosh Shilimkar
  2013-01-18 16:53 ` Stephen Warren
  0 siblings, 2 replies; 8+ messages in thread
From: Hiroshi Doyu @ 2013-01-18 10:59 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 1/1] ARM: Add API to detect SCU base address from CP15
  2013-01-18 10:59 [PATCH 1/1] ARM: Add API to detect SCU base address from CP15 Hiroshi Doyu
@ 2013-01-18 12:54 ` Santosh Shilimkar
  2013-01-18 14:29   ` Hiroshi Doyu
  2013-01-18 16:53 ` Stephen Warren
  1 sibling, 1 reply; 8+ messages in thread
From: Santosh Shilimkar @ 2013-01-18 12:54 UTC (permalink / raw)
  To: linux-arm-kernel

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
	

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/1] ARM: Add API to detect SCU base address from CP15
  2013-01-18 12:54 ` Santosh Shilimkar
@ 2013-01-18 14:29   ` Hiroshi Doyu
  2013-01-18 14:33     ` Santosh Shilimkar
  0 siblings, 1 reply; 8+ messages in thread
From: Hiroshi Doyu @ 2013-01-18 14:29 UTC (permalink / raw)
  To: linux-arm-kernel

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.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/1] ARM: Add API to detect SCU base address from CP15
  2013-01-18 14:29   ` Hiroshi Doyu
@ 2013-01-18 14:33     ` Santosh Shilimkar
  0 siblings, 0 replies; 8+ messages in thread
From: Santosh Shilimkar @ 2013-01-18 14:33 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/1] ARM: Add API to detect SCU base address from CP15
  2013-01-18 10:59 [PATCH 1/1] ARM: Add API to detect SCU base address from CP15 Hiroshi Doyu
  2013-01-18 12:54 ` Santosh Shilimkar
@ 2013-01-18 16:53 ` Stephen Warren
  2013-01-21  7:42   ` [v2 " Hiroshi Doyu
  1 sibling, 1 reply; 8+ messages in thread
From: Stephen Warren @ 2013-01-18 16:53 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [v2 1/1] ARM: Add API to detect SCU base address from CP15
  2013-01-18 16:53 ` Stephen Warren
@ 2013-01-21  7:42   ` Hiroshi Doyu
  2013-01-21 15:31     ` Russell King - ARM Linux
  2013-01-31 16:39     ` Hiroshi Doyu
  0 siblings, 2 replies; 8+ messages in thread
From: Hiroshi Doyu @ 2013-01-21  7:42 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [v2 1/1] ARM: Add API to detect SCU base address from CP15
  2013-01-21  7:42   ` [v2 " Hiroshi Doyu
@ 2013-01-21 15:31     ` Russell King - ARM Linux
  2013-01-31 16:39     ` Hiroshi Doyu
  1 sibling, 0 replies; 8+ messages in thread
From: Russell King - ARM Linux @ 2013-01-21 15:31 UTC (permalink / raw)
  To: linux-arm-kernel

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.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [v2 1/1] ARM: Add API to detect SCU base address from CP15
  2013-01-21  7:42   ` [v2 " Hiroshi Doyu
  2013-01-21 15:31     ` Russell King - ARM Linux
@ 2013-01-31 16:39     ` Hiroshi Doyu
  1 sibling, 0 replies; 8+ messages in thread
From: Hiroshi Doyu @ 2013-01-31 16:39 UTC (permalink / raw)
  To: linux-arm-kernel

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.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2013-01-31 16:39 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-01-18 10:59 [PATCH 1/1] ARM: Add API to detect SCU base address from CP15 Hiroshi Doyu
2013-01-18 12:54 ` Santosh Shilimkar
2013-01-18 14:29   ` Hiroshi Doyu
2013-01-18 14:33     ` Santosh Shilimkar
2013-01-18 16:53 ` Stephen Warren
2013-01-21  7:42   ` [v2 " Hiroshi Doyu
2013-01-21 15:31     ` Russell King - ARM Linux
2013-01-31 16:39     ` Hiroshi Doyu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).