[PATCH 1/4] ARM: dt: Add doc for ARM Snoop Control Unit(SCU)

[PATCH 1/4] ARM: dt: Add doc for ARM Snoop Control Unit(SCU)

[Hiroshi Doyu]

Add scu.txt under arm for ARM Snoop Control Unit(SCU)

Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 Documentation/devicetree/bindings/arm/scu.txt |   15 +++++++++++++++
 1 file changed, 15 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/scu.txt

diff --git a/Documentation/devicetree/bindings/arm/scu.txt b/Documentation/devicetree/bindings/arm/scu.txt
new file mode 100644
index 0000000..96264cf
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/scu.txt
@@ -0,0 +1,15 @@
+* ARM Snoop Control Unit(SCU)
+
+ARM cores often have a configurable SCU to the memory system through
+the AXI interfaces.
+
+Required properties:
+
+- compatible : "arm,cortex-a9-scu"
+- reg : Specifies base physical address and size of the SCU registers.
+
+Example:
+	scu {
+		compatible = "arm,cortex-a9-scu";
+		reg = <0x50040000 0x58>;
+	};
-- 
1.7.9.5

[PATCH 2/4] ARM: dt: tegra20.dtsi: Add SCU node

[Hiroshi Doyu]

Add Snoop Control Unit(SCU) node for Cortex A9 MP.

Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
---
 arch/arm/boot/dts/tegra20.dtsip |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/tegra20.dtsip b/arch/arm/boot/dts/tegra20.dtsip
index 3e046b1..d325aed 100644
--- a/arch/arm/boot/dts/tegra20.dtsip
+++ b/arch/arm/boot/dts/tegra20.dtsip
@@ -91,6 +91,11 @@
 		};
 	};
 
+	scu {
+		compatible = "arm,cortex-a9-scu";
+		reg = <0x50040000 0x58>;
+	};
+
 	timer@50004600 {
 		compatible = "arm,cortex-a9-twd-timer";
 		reg = <0x50040600 0x20>;
-- 
1.7.9.5

[PATCH 3/4] ARM: dt: tegra30.dtsi: Add SCU node

[Hiroshi Doyu]

Add Snoop Control Unit(SCU) node for Cortex A9 MP.

Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
---
 arch/arm/boot/dts/tegra30.dtsip |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/tegra30.dtsip b/arch/arm/boot/dts/tegra30.dtsip
index 8feba6d..84a41c2 100644
--- a/arch/arm/boot/dts/tegra30.dtsip
+++ b/arch/arm/boot/dts/tegra30.dtsip
@@ -91,6 +91,11 @@
 		};
 	};
 
+	scu {
+		compatible = "arm,cortex-a9-scu";
+		reg = <0x50040000 0x58>;
+	};
+
 	timer@50004600 {
 		compatible = "arm,cortex-a9-twd-timer";
 		reg = <0x50040600 0x20>;
-- 
1.7.9.5

[PATCH 4/4] ARM: tegra: Set SCU base address dynamically from DT

[Hiroshi Doyu]

Set Snoop Control Unit(SCU) register base address dynamically from DT.

Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
---
 arch/arm/mach-tegra/platsmp.c |   23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-tegra/platsmp.c b/arch/arm/mach-tegra/platsmp.c
index 1b926df..45c0b79 100644
--- a/arch/arm/mach-tegra/platsmp.c
+++ b/arch/arm/mach-tegra/platsmp.c
@@ -18,6 +18,8 @@
 #include <linux/jiffies.h>
 #include <linux/smp.h>
 #include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
 
 #include <asm/cacheflush.h>
 #include <asm/hardware/gic.h>
@@ -36,7 +38,7 @@
 
 extern void tegra_secondary_startup(void);
 
-static void __iomem *scu_base = IO_ADDRESS(TEGRA_ARM_PERIF_BASE);
+static void __iomem *scu_base;
 
 #define EVP_CPU_RESET_VECTOR \
 	(IO_ADDRESS(TEGRA_EXCEPTION_VECTORS_BASE) + 0x100)
@@ -143,14 +145,28 @@ done:
 	return status;
 }
 
+static const struct of_device_id cortex_a9_scu_match[] __initconst = {
+	{ .compatible = "arm,cortex-a9-scu", },
+	{}
+};
+
 /*
  * Initialise the CPU possible map early - this describes the CPUs
  * which may be present or become present in the system.
  */
 static void __init tegra_smp_init_cpus(void)
 {
-	unsigned int i, ncores = scu_get_core_count(scu_base);
+	struct device_node *np;
+	unsigned int i, ncores = 1;
+
+	np = of_find_matching_node(NULL, cortex_a9_scu_match);
+	if (!np)
+		return;
+	scu_base = of_iomap(np, 0);
+	if (!scu_base)
+		return;
 
+	ncores = scu_get_core_count(scu_base);
 	if (ncores > nr_cpu_ids) {
 		pr_warn("SMP: %u cores greater than maximum (%u), clipping\n",
 			ncores, nr_cpu_ids);
@@ -166,7 +182,8 @@ static void __init tegra_smp_init_cpus(void)
 static void __init tegra_smp_prepare_cpus(unsigned int max_cpus)
 {
 	tegra_cpu_reset_handler_init();
-	scu_enable(scu_base);
+	if (scu_base)
+		scu_enable(scu_base);
 }
 
 struct smp_operations tegra_smp_ops __initdata = {
-- 
1.7.9.5

Re: [PATCH 4/4] ARM: tegra: Set SCU base address dynamically from DT

[Rob Herring]

On 12/17/2012 12:18 AM, Hiroshi Doyu wrote:
> Set Snoop Control Unit(SCU) register base address dynamically from DT.
> 
> Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
> ---
>  arch/arm/mach-tegra/platsmp.c |   23 ++++++++++++++++++++---
>  1 file changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/mach-tegra/platsmp.c b/arch/arm/mach-tegra/platsmp.c
> index 1b926df..45c0b79 100644
> --- a/arch/arm/mach-tegra/platsmp.c
> +++ b/arch/arm/mach-tegra/platsmp.c
> @@ -18,6 +18,8 @@
>  #include <linux/jiffies.h>
>  #include <linux/smp.h>
>  #include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
>  
>  #include <asm/cacheflush.h>
>  #include <asm/hardware/gic.h>
> @@ -36,7 +38,7 @@
>  
>  extern void tegra_secondary_startup(void);
>  
> -static void __iomem *scu_base = IO_ADDRESS(TEGRA_ARM_PERIF_BASE);
> +static void __iomem *scu_base;
>  
>  #define EVP_CPU_RESET_VECTOR \
>  	(IO_ADDRESS(TEGRA_EXCEPTION_VECTORS_BASE) + 0x100)
> @@ -143,14 +145,28 @@ done:
>  	return status;
>  }
>  
> +static const struct of_device_id cortex_a9_scu_match[] __initconst = {
> +	{ .compatible = "arm,cortex-a9-scu", },
> +	{}
> +};
> +
>  /*
>   * Initialise the CPU possible map early - this describes the CPUs
>   * which may be present or become present in the system.
>   */
>  static void __init tegra_smp_init_cpus(void)
>  {
> -	unsigned int i, ncores = scu_get_core_count(scu_base);
> +	struct device_node *np;
> +	unsigned int i, ncores = 1;
> +
> +	np = of_find_matching_node(NULL, cortex_a9_scu_match);
> +	if (!np)
> +		return;
> +	scu_base = of_iomap(np, 0);

Did you actually test this? Unless something changed, ioremap does not
work this early. The only reason to have it mapped this early is to get
the core count, but that doesn't work on A15 or A7. So we really need to
get core count/mask in a standard way. At least some work to get core
count from DT went into 3.8.

BTW, you can get the scu address on the A9 by reading cp15 register:

	/* Get SCU base */
	asm("mrc p15, 4, %0, c15, c0, 0" : "=r" (base));

It's still probably good to have the DT node, but the reg property can
be optional in this case.

We need to move away from having the DT matching code within the
platforms. This should all be moved to the scu code in a scu_of_init
function that could be called from common code.

Rob

> +	if (!scu_base)
> +		return;
>  
> +	ncores = scu_get_core_count(scu_base);
>  	if (ncores > nr_cpu_ids) {
>  		pr_warn("SMP: %u cores greater than maximum (%u), clipping\n",
>  			ncores, nr_cpu_ids);
> @@ -166,7 +182,8 @@ static void __init tegra_smp_init_cpus(void)
>  static void __init tegra_smp_prepare_cpus(unsigned int max_cpus)
>  {
>  	tegra_cpu_reset_handler_init();
> -	scu_enable(scu_base);
> +	if (scu_base)
> +		scu_enable(scu_base);
>  }
>  
>  struct smp_operations tegra_smp_ops __initdata = {
> 

Re: [PATCH 4/4] ARM: tegra: Set SCU base address dynamically from DT

[Stephen Warren]

On 12/17/2012 07:00 AM, Rob Herring wrote:
> On 12/17/2012 12:18 AM, Hiroshi Doyu wrote:
>> Set Snoop Control Unit(SCU) register base address dynamically from DT.

>> diff --git a/arch/arm/mach-tegra/platsmp.c b/arch/arm/mach-tegra/platsmp.c

>>  static void __init tegra_smp_init_cpus(void)
>>  {
>> -	unsigned int i, ncores = scu_get_core_count(scu_base);
>> +	struct device_node *np;
>> +	unsigned int i, ncores = 1;
>> +
>> +	np = of_find_matching_node(NULL, cortex_a9_scu_match);
>> +	if (!np)
>> +		return;
>> +	scu_base = of_iomap(np, 0);
> 
> Did you actually test this? Unless something changed, ioremap does not
> work this early. The only reason to have it mapped this early is to get
> the core count, but that doesn't work on A15 or A7. So we really need to
> get core count/mask in a standard way. At least some work to get core
> count from DT went into 3.8.

Does it work if the machine's .map_io() function has set up a static
mapping that includes the specified region? I believe that is the case
on Tegra. What is the alternative if the registers can't be mapped;
should the code count the number of child nodes in /cpus?

Re: [PATCH 4/4] ARM: tegra: Set SCU base address dynamically from DT

[Hiroshi Doyu]

Hi Rob,

Rob Herring <robherring2@gmail.com> wrote @ Mon, 17 Dec 2012 15:00:46 +0100:

> On 12/17/2012 12:18 AM, Hiroshi Doyu wrote:
> > Set Snoop Control Unit(SCU) register base address dynamically from DT.
> > 
> > Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
> > ---
> >  arch/arm/mach-tegra/platsmp.c |   23 ++++++++++++++++++++---
> >  1 file changed, 20 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/arm/mach-tegra/platsmp.c b/arch/arm/mach-tegra/platsmp.c
> > index 1b926df..45c0b79 100644
> > --- a/arch/arm/mach-tegra/platsmp.c
> > +++ b/arch/arm/mach-tegra/platsmp.c
> > @@ -18,6 +18,8 @@
> >  #include <linux/jiffies.h>
> >  #include <linux/smp.h>
> >  #include <linux/io.h>
> > +#include <linux/of.h>
> > +#include <linux/of_address.h>
> >  
> >  #include <asm/cacheflush.h>
> >  #include <asm/hardware/gic.h>
> > @@ -36,7 +38,7 @@
> >  
> >  extern void tegra_secondary_startup(void);
> >  
> > -static void __iomem *scu_base = IO_ADDRESS(TEGRA_ARM_PERIF_BASE);
> > +static void __iomem *scu_base;
> >  
> >  #define EVP_CPU_RESET_VECTOR \
> >  	(IO_ADDRESS(TEGRA_EXCEPTION_VECTORS_BASE) + 0x100)
> > @@ -143,14 +145,28 @@ done:
> >  	return status;
> >  }
> >  
> > +static const struct of_device_id cortex_a9_scu_match[] __initconst = {
> > +	{ .compatible = "arm,cortex-a9-scu", },
> > +	{}
> > +};
> > +
> >  /*
> >   * Initialise the CPU possible map early - this describes the CPUs
> >   * which may be present or become present in the system.
> >   */
> >  static void __init tegra_smp_init_cpus(void)
> >  {
> > -	unsigned int i, ncores = scu_get_core_count(scu_base);
> > +	struct device_node *np;
> > +	unsigned int i, ncores = 1;
> > +
> > +	np = of_find_matching_node(NULL, cortex_a9_scu_match);
> > +	if (!np)
> > +		return;
> > +	scu_base = of_iomap(np, 0);
> 
> Did you actually test this? Unless something changed, ioremap does not
> work this early. The only reason to have it mapped this early is to get
> the core count, but that doesn't work on A15 or A7. So we really need to
> get core count/mask in a standard way. At least some work to get core
> count from DT went into 3.8.
> 
> BTW, you can get the scu address on the A9 by reading cp15 register:
> 
> 	/* Get SCU base */
> 	asm("mrc p15, 4, %0, c15, c0, 0" : "=r" (base));
> 
> It's still probably good to have the DT node, but the reg property can
> be optional in this case.

I'm simply wondering, if the above cp15 works with Cortex-A9, do we
still need SCU DT node? At least from Cortex-A15 TRM, it seems that
SCU is tighly integrated into CPU core and it doesn't have any user
control. So Cortex-A15 doesn't seem to need to configure SCU. For
Cortex-A7, I haven't yet found S/W configurable register definitions
in TRM. So if neither of A15/A7 need SCU base, would the above cp15
intructions be enough?

> We need to move away from having the DT matching code within the
> platforms. This should all be moved to the scu code in a scu_of_init
> function that could be called from common code.

True if SCU DT node is still necessary.

Re: [PATCH 4/4] ARM: tegra: Set SCU base address dynamically from DT

[Rob Herring]

On 12/18/2012 03:21 AM, Hiroshi Doyu wrote:
> Hi Rob,
> 
> Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote @ Mon, 17 Dec 2012 15:00:46 +0100:
> 
>> On 12/17/2012 12:18 AM, Hiroshi Doyu wrote:
>>> Set Snoop Control Unit(SCU) register base address dynamically from DT.
>>>
>>> Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>>> ---
>>>  arch/arm/mach-tegra/platsmp.c |   23 ++++++++++++++++++++---
>>>  1 file changed, 20 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-tegra/platsmp.c b/arch/arm/mach-tegra/platsmp.c
>>> index 1b926df..45c0b79 100644
>>> --- a/arch/arm/mach-tegra/platsmp.c
>>> +++ b/arch/arm/mach-tegra/platsmp.c
>>> @@ -18,6 +18,8 @@
>>>  #include <linux/jiffies.h>
>>>  #include <linux/smp.h>
>>>  #include <linux/io.h>
>>> +#include <linux/of.h>
>>> +#include <linux/of_address.h>
>>>  
>>>  #include <asm/cacheflush.h>
>>>  #include <asm/hardware/gic.h>
>>> @@ -36,7 +38,7 @@
>>>  
>>>  extern void tegra_secondary_startup(void);
>>>  
>>> -static void __iomem *scu_base = IO_ADDRESS(TEGRA_ARM_PERIF_BASE);
>>> +static void __iomem *scu_base;
>>>  
>>>  #define EVP_CPU_RESET_VECTOR \
>>>  	(IO_ADDRESS(TEGRA_EXCEPTION_VECTORS_BASE) + 0x100)
>>> @@ -143,14 +145,28 @@ done:
>>>  	return status;
>>>  }
>>>  
>>> +static const struct of_device_id cortex_a9_scu_match[] __initconst = {
>>> +	{ .compatible = "arm,cortex-a9-scu", },
>>> +	{}
>>> +};
>>> +
>>>  /*
>>>   * Initialise the CPU possible map early - this describes the CPUs
>>>   * which may be present or become present in the system.
>>>   */
>>>  static void __init tegra_smp_init_cpus(void)
>>>  {
>>> -	unsigned int i, ncores = scu_get_core_count(scu_base);
>>> +	struct device_node *np;
>>> +	unsigned int i, ncores = 1;
>>> +
>>> +	np = of_find_matching_node(NULL, cortex_a9_scu_match);
>>> +	if (!np)
>>> +		return;
>>> +	scu_base = of_iomap(np, 0);
>>
>> Did you actually test this? Unless something changed, ioremap does not
>> work this early. The only reason to have it mapped this early is to get
>> the core count, but that doesn't work on A15 or A7. So we really need to
>> get core count/mask in a standard way. At least some work to get core
>> count from DT went into 3.8.
>>
>> BTW, you can get the scu address on the A9 by reading cp15 register:
>>
>> 	/* Get SCU base */
>> 	asm("mrc p15, 4, %0, c15, c0, 0" : "=r" (base));
>>
>> It's still probably good to have the DT node, but the reg property can
>> be optional in this case.
> 
> I'm simply wondering, if the above cp15 works with Cortex-A9, do we
> still need SCU DT node? At least from Cortex-A15 TRM, it seems that
> SCU is tighly integrated into CPU core and it doesn't have any user
> control. So Cortex-A15 doesn't seem to need to configure SCU. For
> Cortex-A7, I haven't yet found S/W configurable register definitions
> in TRM. So if neither of A15/A7 need SCU base, would the above cp15
> intructions be enough?

The A15/A7 still have the register for the other peripherals like the
GIC, but there are no SCU registers or other way to get a core count
from the h/w.

The DT node could be used to determine if you have an SCU or not. I just
used the cpu node compatible value to determine that.

>> We need to move away from having the DT matching code within the
>> platforms. This should all be moved to the scu code in a scu_of_init
>> function that could be called from common code.
> 
> True if SCU DT node is still necessary.

Well, reading the cp15 register and mapping the registers could be
common code independent of DT. I'm not sure if there are non-A9
implementations of the SCU which don't have the cp15 register.

Rob

Re: [PATCH 4/4] ARM: tegra: Set SCU base address dynamically from DT

[Hiroshi Doyu]

Rob Herring <robherring2@gmail.com> wrote @ Tue, 18 Dec 2012 14:46:36 +0100:

> On 12/18/2012 03:21 AM, Hiroshi Doyu wrote:
> > Hi Rob,
> > 
> > Rob Herring <robherring2@gmail.com> wrote @ Mon, 17 Dec 2012 15:00:46 +0100:
> > 
> >> On 12/17/2012 12:18 AM, Hiroshi Doyu wrote:
> >>> Set Snoop Control Unit(SCU) register base address dynamically from DT.
> >>>
> >>> Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
> >>> ---
> >>>  arch/arm/mach-tegra/platsmp.c |   23 ++++++++++++++++++++---
> >>>  1 file changed, 20 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/arch/arm/mach-tegra/platsmp.c b/arch/arm/mach-tegra/platsmp.c
> >>> index 1b926df..45c0b79 100644
> >>> --- a/arch/arm/mach-tegra/platsmp.c
> >>> +++ b/arch/arm/mach-tegra/platsmp.c
> >>> @@ -18,6 +18,8 @@
> >>>  #include <linux/jiffies.h>
> >>>  #include <linux/smp.h>
> >>>  #include <linux/io.h>
> >>> +#include <linux/of.h>
> >>> +#include <linux/of_address.h>
> >>>  
> >>>  #include <asm/cacheflush.h>
> >>>  #include <asm/hardware/gic.h>
> >>> @@ -36,7 +38,7 @@
> >>>  
> >>>  extern void tegra_secondary_startup(void);
> >>>  
> >>> -static void __iomem *scu_base = IO_ADDRESS(TEGRA_ARM_PERIF_BASE);
> >>> +static void __iomem *scu_base;
> >>>  
> >>>  #define EVP_CPU_RESET_VECTOR \
> >>>  	(IO_ADDRESS(TEGRA_EXCEPTION_VECTORS_BASE) + 0x100)
> >>> @@ -143,14 +145,28 @@ done:
> >>>  	return status;
> >>>  }
> >>>  
> >>> +static const struct of_device_id cortex_a9_scu_match[] __initconst = {
> >>> +	{ .compatible = "arm,cortex-a9-scu", },
> >>> +	{}
> >>> +};
> >>> +
> >>>  /*
> >>>   * Initialise the CPU possible map early - this describes the CPUs
> >>>   * which may be present or become present in the system.
> >>>   */
> >>>  static void __init tegra_smp_init_cpus(void)
> >>>  {
> >>> -	unsigned int i, ncores = scu_get_core_count(scu_base);
> >>> +	struct device_node *np;
> >>> +	unsigned int i, ncores = 1;
> >>> +
> >>> +	np = of_find_matching_node(NULL, cortex_a9_scu_match);
> >>> +	if (!np)
> >>> +		return;
> >>> +	scu_base = of_iomap(np, 0);
> >>
> >> Did you actually test this? Unless something changed, ioremap does not
> >> work this early. The only reason to have it mapped this early is to get
> >> the core count, but that doesn't work on A15 or A7. So we really need to
> >> get core count/mask in a standard way. At least some work to get core
> >> count from DT went into 3.8.
> >>
> >> BTW, you can get the scu address on the A9 by reading cp15 register:
> >>
> >> 	/* Get SCU base */
> >> 	asm("mrc p15, 4, %0, c15, c0, 0" : "=r" (base));
> >>
> >> It's still probably good to have the DT node, but the reg property can
> >> be optional in this case.
> > 
> > I'm simply wondering, if the above cp15 works with Cortex-A9, do we
> > still need SCU DT node? At least from Cortex-A15 TRM, it seems that
> > SCU is tighly integrated into CPU core and it doesn't have any user
> > control. So Cortex-A15 doesn't seem to need to configure SCU. For
> > Cortex-A7, I haven't yet found S/W configurable register definitions
> > in TRM. So if neither of A15/A7 need SCU base, would the above cp15
> > intructions be enough?
> 
> The A15/A7 still have the register for the other peripherals like the
> GIC, but there are no SCU registers or other way to get a core count
> from the h/w.
> 
> The DT node could be used to determine if you have an SCU or not. I just
> used the cpu node compatible value to determine that.

Taking a look at A15 TRM again, it seems that A15 can get number of
processors via(*1):

  asm("mrc p15, 1, %0, c9, c0, 2\n" : "=r" (l2ctlr));

At least, A15 is ok without SCU/DT to get the number of processors. I
haven't found the above instruction in A7, but is there any way to get
the number of core in A7?

If both A15/A7 can get # of CPU cores via coprocessor instruction,
what others should we take care of with DT node?

*1: http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0438e/BABBACEE.html

> >> We need to move away from having the DT matching code within the
> >> platforms. This should all be moved to the scu code in a scu_of_init
> >> function that could be called from common code.
> > 
> > True if SCU DT node is still necessary.
> 
> Well, reading the cp15 register and mapping the registers could be
> common code independent of DT. I'm not sure if there are non-A9
> implementations of the SCU which don't have the cp15 register.

Re: [PATCH 4/4] ARM: tegra: Set SCU base address dynamically from DT

[Hiroshi Doyu]

Hiroshi Doyu <hdoyu@nvidia.com> wrote @ Tue, 18 Dec 2012 17:15:46 +0200 (EET):

> Rob Herring <robherring2@gmail.com> wrote @ Tue, 18 Dec 2012 14:46:36 +0100:
> 
> > On 12/18/2012 03:21 AM, Hiroshi Doyu wrote:
> > > Hi Rob,
> > > 
> > > Rob Herring <robherring2@gmail.com> wrote @ Mon, 17 Dec 2012 15:00:46 +0100:
> > > 
> > >> On 12/17/2012 12:18 AM, Hiroshi Doyu wrote:
> > >>> Set Snoop Control Unit(SCU) register base address dynamically from DT.
> > >>>
> > >>> Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
> > >>> ---
> > >>>  arch/arm/mach-tegra/platsmp.c |   23 ++++++++++++++++++++---
> > >>>  1 file changed, 20 insertions(+), 3 deletions(-)
> > >>>
> > >>> diff --git a/arch/arm/mach-tegra/platsmp.c b/arch/arm/mach-tegra/platsmp.c
> > >>> index 1b926df..45c0b79 100644
> > >>> --- a/arch/arm/mach-tegra/platsmp.c
> > >>> +++ b/arch/arm/mach-tegra/platsmp.c
> > >>> @@ -18,6 +18,8 @@
> > >>>  #include <linux/jiffies.h>
> > >>>  #include <linux/smp.h>
> > >>>  #include <linux/io.h>
> > >>> +#include <linux/of.h>
> > >>> +#include <linux/of_address.h>
> > >>>  
> > >>>  #include <asm/cacheflush.h>
> > >>>  #include <asm/hardware/gic.h>
> > >>> @@ -36,7 +38,7 @@
> > >>>  
> > >>>  extern void tegra_secondary_startup(void);
> > >>>  
> > >>> -static void __iomem *scu_base = IO_ADDRESS(TEGRA_ARM_PERIF_BASE);
> > >>> +static void __iomem *scu_base;
> > >>>  
> > >>>  #define EVP_CPU_RESET_VECTOR \
> > >>>  	(IO_ADDRESS(TEGRA_EXCEPTION_VECTORS_BASE) + 0x100)
> > >>> @@ -143,14 +145,28 @@ done:
> > >>>  	return status;
> > >>>  }
> > >>>  
> > >>> +static const struct of_device_id cortex_a9_scu_match[] __initconst = {
> > >>> +	{ .compatible = "arm,cortex-a9-scu", },
> > >>> +	{}
> > >>> +};
> > >>> +
> > >>>  /*
> > >>>   * Initialise the CPU possible map early - this describes the CPUs
> > >>>   * which may be present or become present in the system.
> > >>>   */
> > >>>  static void __init tegra_smp_init_cpus(void)
> > >>>  {
> > >>> -	unsigned int i, ncores = scu_get_core_count(scu_base);
> > >>> +	struct device_node *np;
> > >>> +	unsigned int i, ncores = 1;
> > >>> +
> > >>> +	np = of_find_matching_node(NULL, cortex_a9_scu_match);
> > >>> +	if (!np)
> > >>> +		return;
> > >>> +	scu_base = of_iomap(np, 0);
> > >>
> > >> Did you actually test this? Unless something changed, ioremap does not
> > >> work this early. The only reason to have it mapped this early is to get
> > >> the core count, but that doesn't work on A15 or A7. So we really need to
> > >> get core count/mask in a standard way. At least some work to get core
> > >> count from DT went into 3.8.
> > >>
> > >> BTW, you can get the scu address on the A9 by reading cp15 register:
> > >>
> > >> 	/* Get SCU base */
> > >> 	asm("mrc p15, 4, %0, c15, c0, 0" : "=r" (base));
> > >>
> > >> It's still probably good to have the DT node, but the reg property can
> > >> be optional in this case.
> > > 
> > > I'm simply wondering, if the above cp15 works with Cortex-A9, do we
> > > still need SCU DT node? At least from Cortex-A15 TRM, it seems that
> > > SCU is tighly integrated into CPU core and it doesn't have any user
> > > control. So Cortex-A15 doesn't seem to need to configure SCU. For
> > > Cortex-A7, I haven't yet found S/W configurable register definitions
> > > in TRM. So if neither of A15/A7 need SCU base, would the above cp15
> > > intructions be enough?
> > 
> > The A15/A7 still have the register for the other peripherals like the
> > GIC, but there are no SCU registers or other way to get a core count
> > from the h/w.
> > 
> > The DT node could be used to determine if you have an SCU or not. I just
> > used the cpu node compatible value to determine that.
> 
> Taking a look at A15 TRM again, it seems that A15 can get number of
> processors via(*1):
> 
>   asm("mrc p15, 1, %0, c9, c0, 2\n" : "=r" (l2ctlr));
> 
> At least, A15 is ok without SCU/DT to get the number of processors. I
> haven't found the above instruction in A7, but is there any way to get
> the number of core in A7?

L2CTLR can be used to detect # of cores for A7 as well:

http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0464e/BABBACEE.html

> If both A15/A7 can get # of CPU cores via coprocessor instruction,
> what others should we take care of with DT node?
> 
> *1: http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0438e/BABBACEE.html
> 
> > >> We need to move away from having the DT matching code within the
> > >> platforms. This should all be moved to the scu code in a scu_of_init
> > >> function that could be called from common code.
> > > 
> > > True if SCU DT node is still necessary.
> > 
> > Well, reading the cp15 register and mapping the registers could be
> > common code independent of DT. I'm not sure if there are non-A9
> > implementations of the SCU which don't have the cp15 register.
> 
> 

Re: [PATCH 4/4] ARM: tegra: Set SCU base address dynamically from DT

[Hiroshi Doyu]

Rob Herring <robherring2@gmail.com> wrote @ Mon, 17 Dec 2012 15:00:46 +0100:

> On 12/17/2012 12:18 AM, Hiroshi Doyu wrote:
> > Set Snoop Control Unit(SCU) register base address dynamically from DT.
> > 
> > Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
> > ---
> >  arch/arm/mach-tegra/platsmp.c |   23 ++++++++++++++++++++---
> >  1 file changed, 20 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/arm/mach-tegra/platsmp.c b/arch/arm/mach-tegra/platsmp.c
> > index 1b926df..45c0b79 100644
> > --- a/arch/arm/mach-tegra/platsmp.c
> > +++ b/arch/arm/mach-tegra/platsmp.c
> > @@ -18,6 +18,8 @@
> >  #include <linux/jiffies.h>
> >  #include <linux/smp.h>
> >  #include <linux/io.h>
> > +#include <linux/of.h>
> > +#include <linux/of_address.h>
> >  
> >  #include <asm/cacheflush.h>
> >  #include <asm/hardware/gic.h>
> > @@ -36,7 +38,7 @@
> >  
> >  extern void tegra_secondary_startup(void);
> >  
> > -static void __iomem *scu_base = IO_ADDRESS(TEGRA_ARM_PERIF_BASE);
> > +static void __iomem *scu_base;
> >  
> >  #define EVP_CPU_RESET_VECTOR \
> >  	(IO_ADDRESS(TEGRA_EXCEPTION_VECTORS_BASE) + 0x100)
> > @@ -143,14 +145,28 @@ done:
> >  	return status;
> >  }
> >  
> > +static const struct of_device_id cortex_a9_scu_match[] __initconst = {
> > +	{ .compatible = "arm,cortex-a9-scu", },
> > +	{}
> > +};
> > +
> >  /*
> >   * Initialise the CPU possible map early - this describes the CPUs
> >   * which may be present or become present in the system.
> >   */
> >  static void __init tegra_smp_init_cpus(void)
> >  {
> > -	unsigned int i, ncores = scu_get_core_count(scu_base);
> > +	struct device_node *np;
> > +	unsigned int i, ncores = 1;
> > +
> > +	np = of_find_matching_node(NULL, cortex_a9_scu_match);
> > +	if (!np)
> > +		return;
> > +	scu_base = of_iomap(np, 0);
> 
> Did you actually test this? Unless something changed, ioremap does not
> work this early. The only reason to have it mapped this early is to get
> the core count, but that doesn't work on A15 or A7. So we really need to
> get core count/mask in a standard way. At least some work to get core
> count from DT went into 3.8.
> 
> BTW, you can get the scu address on the A9 by reading cp15 register:
> 
> 	/* Get SCU base */
> 	asm("mrc p15, 4, %0, c15, c0, 0" : "=r" (base));
> 
> It's still probably good to have the DT node, but the reg property can
> be optional in this case.
> 
> We need to move away from having the DT matching code within the
> platforms. This should all be moved to the scu code in a scu_of_init
> function that could be called from common code.

If we can get SCU base address from CP15, do we still need SCU entry
in DT? If not, the following would be the only API to get SCU base?

>From 9bbecb50759f39d9c762977145407ea4f8a4d5ac Mon Sep 17 00:00:00 2001
From: Hiroshi Doyu <hdoyu@nvidia.com>
Date: Mon, 14 Jan 2013 12:35:33 +0200
Subject: [PATCH 1/1] ARM: Add API to detect SCU base address from CP15

Add API to detect SCU base address from CP15.

Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
---
 arch/arm/include/asm/smp_scu.h |   14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm/include/asm/smp_scu.h b/arch/arm/include/asm/smp_scu.h
index 4eb6d00..6015ede 100644
--- a/arch/arm/include/asm/smp_scu.h
+++ b/arch/arm/include/asm/smp_scu.h
@@ -6,6 +6,20 @@
 #define SCU_PM_POWEROFF	3
 
 #ifndef __ASSEMBLER__
+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

Re: [PATCH 4/4] ARM: tegra: Set SCU base address dynamically from DT

[Stephen Warren]

On 12/16/2012 11:18 PM, Hiroshi Doyu wrote:
> Set Snoop Control Unit(SCU) register base address dynamically from DT.

Hiroshi, what's the status on this patch series? I think Rob had
comments/objections on patch 4; are you going to post an updated series,
or is this series replaced by patch 3 in your Tegra114 series?

Re: [PATCH 4/4] ARM: tegra: Set SCU base address dynamically from DT

[Hiroshi Doyu]

Stephen Warren <swarren@wwwdotorg.org> wrote @ Wed, 2 Jan 2013 20:02:53 +0100:

> On 12/16/2012 11:18 PM, Hiroshi Doyu wrote:
> > Set Snoop Control Unit(SCU) register base address dynamically from DT.
> 
> Hiroshi, what's the status on this patch series? I think Rob had
> comments/objections on patch 4; are you going to post an updated series,
> or is this series replaced by patch 3 in your Tegra114 series?

As Rob suggested, at least, SCU base address should be detected in
common code, scu_of_init() even if DT /cpus is the primary way to
detect # of cores. I'll repost later.

This patch can be done independent of Tegra114 series.