[PATCH 0/3] arm64: dts: r8a7796: Add Secondary CPU Cores

[PATCH 0/3] arm64: dts: r8a7796: Add Secondary CPU Cores

[Geert Uytterhoeven]

	Hi Simon, Magnus,

This patch series adds the second Cortex-A57 CPU core, and the
Cortex-A53 L2 cache-controller and CPU nodes on the Renesas R-Car M3-W
SoC to its DTS file.

Note that these patches add hardware description; actual enabling of the
CPU depends on the PSCI firmware.

With the current firmware version (v2.16.0), only the CA57 CPU cores are
enabled, hence the last patch does not introduce undeterministic
scheduling behavior due to migration between big and LITTLE cores.

Tested on r8a7796/salvator-x, with CPU hot(un)plug and system suspend.

Thanks for applying!

Geert Uytterhoeven (2):
  arm64: dts: r8a7796: Add CA53 L2 cache-controller node
  arm64: dts: r8a7796: Add Cortex-A53 CPU cores

Takeshi Kihara (1):
  arm64: dts: r8a7796: Add Cortex-A57 CPU cores

 arch/arm64/boot/dts/renesas/r8a7796.dtsi | 64 +++++++++++++++++++++++++++++---
 1 file changed, 58 insertions(+), 6 deletions(-)

-- 
1.9.1

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

[PATCH 1/3] arm64: dts: r8a7796: Add Cortex-A57 CPU cores

[Geert Uytterhoeven]

From: Takeshi Kihara <takeshi.kihara.df@renesas.com>

This patch adds Cortex-A57 CPU cores to R8A7796 SoC for a total of
2 x Cortex-A57.

Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
[geert: Rebased]
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 arch/arm64/boot/dts/renesas/r8a7796.dtsi | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/r8a7796.dtsi b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
index 2656ff5eb09bef6a..6c0a65abf9fd09eb 100644
--- a/arch/arm64/boot/dts/renesas/r8a7796.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
@@ -37,7 +37,6 @@
 		#address-cells = <1>;
 		#size-cells = <0>;
 
-		/* 1 core only at this point */
 		a57_0: cpu@0 {
 			compatible = "arm,cortex-a57", "arm,armv8";
 			reg = <0x0>;
@@ -47,6 +46,15 @@
 			enable-method = "psci";
 		};
 
+		a57_1: cpu@1 {
+			compatible = "arm,cortex-a57","arm,armv8";
+			reg = <0x1>;
+			device_type = "cpu";
+			power-domains = <&sysc R8A7796_PD_CA57_CPU1>;
+			next-level-cache = <&L2_CA57>;
+			enable-method = "psci";
+		};
+
 		L2_CA57: cache-controller@0 {
 			compatible = "cache";
 			reg = <0>;
@@ -101,7 +109,7 @@
 			      <0x0 0xf1040000 0 0x20000>,
 			      <0x0 0xf1060000 0 0x20000>;
 			interrupts = <GIC_PPI 9
-					(GIC_CPU_MASK_SIMPLE(1) | IRQ_TYPE_LEVEL_HIGH)>;
+					(GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_HIGH)>;
 			clocks = <&cpg CPG_MOD 408>;
 			clock-names = "clk";
 			power-domains = <&sysc R8A7796_PD_ALWAYS_ON>;
@@ -111,13 +119,13 @@
 		timer {
 			compatible = "arm,armv8-timer";
 			interrupts = <GIC_PPI 13
-					(GIC_CPU_MASK_SIMPLE(1) | IRQ_TYPE_LEVEL_LOW)>,
+					(GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_LOW)>,
 				     <GIC_PPI 14
-					(GIC_CPU_MASK_SIMPLE(1) | IRQ_TYPE_LEVEL_LOW)>,
+					(GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_LOW)>,
 				     <GIC_PPI 11
-					(GIC_CPU_MASK_SIMPLE(1) | IRQ_TYPE_LEVEL_LOW)>,
+					(GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_LOW)>,
 				     <GIC_PPI 10
-					(GIC_CPU_MASK_SIMPLE(1) | IRQ_TYPE_LEVEL_LOW)>;
+					(GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_LOW)>;
 		};
 
 		wdt0: watchdog@e6020000 {
-- 
1.9.1

[PATCH 2/3] arm64: dts: r8a7796: Add CA53 L2 cache-controller node

[Geert Uytterhoeven]

Add a device node for the Cortex-A53 L2 cache-controller.

The L2 cache for the Cortex-A53 CPU cores is 512 KiB large (organized as
32 KiB x 16 ways).

Extracted from a patch by Takeshi Kihara in the BSP.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 arch/arm64/boot/dts/renesas/r8a7796.dtsi | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a7796.dtsi b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
index 6c0a65abf9fd09eb..d848e94d7282e5aa 100644
--- a/arch/arm64/boot/dts/renesas/r8a7796.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
@@ -62,6 +62,14 @@
 			cache-unified;
 			cache-level = <2>;
 		};
+
+		L2_CA53: cache-controller@100 {
+			compatible = "cache";
+			reg = <0x100>;
+			power-domains = <&sysc R8A7796_PD_CA53_SCU>;
+			cache-unified;
+			cache-level = <2>;
+		};
 	};
 
 	extal_clk: extal {
-- 
1.9.1

Re: [PATCH 2/3] arm64: dts: r8a7796: Add CA53 L2 cache-controller node

[Sudeep Holla]

On 17/02/17 15:30, Geert Uytterhoeven wrote:
> Add a device node for the Cortex-A53 L2 cache-controller.
> 
> The L2 cache for the Cortex-A53 CPU cores is 512 KiB large (organized as
> 32 KiB x 16 ways).
> 
> Extracted from a patch by Takeshi Kihara in the BSP.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  arch/arm64/boot/dts/renesas/r8a7796.dtsi | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a7796.dtsi b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
> index 6c0a65abf9fd09eb..d848e94d7282e5aa 100644
> --- a/arch/arm64/boot/dts/renesas/r8a7796.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
> @@ -62,6 +62,14 @@
>  			cache-unified;
>  			cache-level = <2>;
>  		};
> +
> +		L2_CA53: cache-controller@100 {
> +			compatible = "cache";
> +			reg = <0x100>;

Is this not integrated L2 cache ? IIUC reg is MPIDR of the cpu and
representing it as cache controller with some reg value doesn't sound
correct IMO.

> +			power-domains = <&sysc R8A7796_PD_CA53_SCU>;
> +			cache-unified;
> +			cache-level = <2>;
> +		};
>  	};
>  
>  	extal_clk: extal {
> 

-- 
Regards,
Sudeep

Re: [PATCH 2/3] arm64: dts: r8a7796: Add CA53 L2 cache-controller node

[Geert Uytterhoeven]

Hi Sudeep,

On Fri, Feb 17, 2017 at 6:51 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
> On 17/02/17 15:30, Geert Uytterhoeven wrote:
>> Add a device node for the Cortex-A53 L2 cache-controller.
>>
>> The L2 cache for the Cortex-A53 CPU cores is 512 KiB large (organized as
>> 32 KiB x 16 ways).
>>
>> Extracted from a patch by Takeshi Kihara in the BSP.
>>
>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> ---
>>  arch/arm64/boot/dts/renesas/r8a7796.dtsi | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/renesas/r8a7796.dtsi b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
>> index 6c0a65abf9fd09eb..d848e94d7282e5aa 100644
>> --- a/arch/arm64/boot/dts/renesas/r8a7796.dtsi
>> +++ b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
>> @@ -62,6 +62,14 @@
>>                       cache-unified;
>>                       cache-level = <2>;
>>               };
>> +
>> +             L2_CA53: cache-controller@100 {
>> +                     compatible = "cache";
>> +                     reg = <0x100>;
>
> Is this not integrated L2 cache ? IIUC reg is MPIDR of the cpu and
> representing it as cache controller with some reg value doesn't sound
> correct IMO.

So this should be cache-controller-1, without a reg property?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 2/3] arm64: dts: r8a7796: Add CA53 L2 cache-controller node

[Geert Uytterhoeven]

On Fri, Feb 17, 2017 at 8:07 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Fri, Feb 17, 2017 at 6:51 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>> On 17/02/17 15:30, Geert Uytterhoeven wrote:
>>> Add a device node for the Cortex-A53 L2 cache-controller.
>>>
>>> The L2 cache for the Cortex-A53 CPU cores is 512 KiB large (organized as
>>> 32 KiB x 16 ways).
>>>
>>> Extracted from a patch by Takeshi Kihara in the BSP.
>>>
>>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>>> ---
>>>  arch/arm64/boot/dts/renesas/r8a7796.dtsi | 8 ++++++++
>>>  1 file changed, 8 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/renesas/r8a7796.dtsi b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
>>> index 6c0a65abf9fd09eb..d848e94d7282e5aa 100644
>>> --- a/arch/arm64/boot/dts/renesas/r8a7796.dtsi
>>> +++ b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
>>> @@ -62,6 +62,14 @@
>>>                       cache-unified;
>>>                       cache-level = <2>;
>>>               };
>>> +
>>> +             L2_CA53: cache-controller@100 {
>>> +                     compatible = "cache";
>>> +                     reg = <0x100>;
>>
>> Is this not integrated L2 cache ? IIUC reg is MPIDR of the cpu and
>> representing it as cache controller with some reg value doesn't sound
>> correct IMO.
>
> So this should be cache-controller-1, without a reg property?

BTW, that means the advice from https://lkml.org/lkml/2016/3/8/80:

| Just add a reg property. The values should probably match the MPIDR in
| some way (e.g. 0 and 100).

was wrong, and we should fix all cache-controller nodes that got "fixed"?

Having better DT documentation for caches on ARM would be nice...
There's only a (too) minimalist example in
Documentation/devicetree/bindings/arm/cpu-capacity.txt

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 2/3] arm64: dts: r8a7796: Add CA53 L2 cache-controller node

[Sudeep Holla]

On 17/02/17 20:40, Geert Uytterhoeven wrote:
> On Fri, Feb 17, 2017 at 8:07 PM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>> On Fri, Feb 17, 2017 at 6:51 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>>> On 17/02/17 15:30, Geert Uytterhoeven wrote:
>>>> Add a device node for the Cortex-A53 L2 cache-controller.
>>>>
>>>> The L2 cache for the Cortex-A53 CPU cores is 512 KiB large (organized as
>>>> 32 KiB x 16 ways).
>>>>
>>>> Extracted from a patch by Takeshi Kihara in the BSP.
>>>>
>>>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>>>> ---
>>>>  arch/arm64/boot/dts/renesas/r8a7796.dtsi | 8 ++++++++
>>>>  1 file changed, 8 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/renesas/r8a7796.dtsi b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
>>>> index 6c0a65abf9fd09eb..d848e94d7282e5aa 100644
>>>> --- a/arch/arm64/boot/dts/renesas/r8a7796.dtsi
>>>> +++ b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
>>>> @@ -62,6 +62,14 @@
>>>>                       cache-unified;
>>>>                       cache-level = <2>;
>>>>               };
>>>> +
>>>> +             L2_CA53: cache-controller@100 {
>>>> +                     compatible = "cache";
>>>> +                     reg = <0x100>;
>>>
>>> Is this not integrated L2 cache ? IIUC reg is MPIDR of the cpu and
>>> representing it as cache controller with some reg value doesn't sound
>>> correct IMO.
>>
>> So this should be cache-controller-1, without a reg property?
> 
> BTW, that means the advice from https://lkml.org/lkml/2016/3/8/80:
> 
> | Just add a reg property. The values should probably match the MPIDR in
> | some way (e.g. 0 and 100).
> 
> was wrong, and we should fix all cache-controller nodes that got "fixed"?
> 

OK. IMO it's cpu peripheral which has no mmio similar to architected
timers that are accessed via system registers. So representing them with
reg = mpidr sounds not correct. If DT maintainers are OK with such
representation, it should be fine but better to document it.

> Having better DT documentation for caches on ARM would be nice...
> There's only a (too) minimalist example in
> Documentation/devicetree/bindings/arm/cpu-capacity.txt
> 

Agreed as I mentioned above.

-- 
Regards,
Sudeep

[PATCH 3/3] arm64: dts: r8a7796: Add Cortex-A53 CPU cores

[Geert Uytterhoeven]

This patch adds Cortex-A53 CPU cores of R8A7796 SoC, and sets a total of
6 cores (2 x Cortex-A57 + 4 x Cortex-A53).

Based on a patch by Takeshi Kihara in the BSP.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 arch/arm64/boot/dts/renesas/r8a7796.dtsi | 46 ++++++++++++++++++++++++++++----
 1 file changed, 41 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/r8a7796.dtsi b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
index d848e94d7282e5aa..b902917d47ad5663 100644
--- a/arch/arm64/boot/dts/renesas/r8a7796.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
@@ -55,6 +55,42 @@
 			enable-method = "psci";
 		};
 
+		a53_0: cpu@100 {
+			compatible = "arm,cortex-a53", "arm,armv8";
+			reg = <0x100>;
+			device_type = "cpu";
+			power-domains = <&sysc R8A7796_PD_CA53_CPU0>;
+			next-level-cache = <&L2_CA53>;
+			enable-method = "psci";
+		};
+
+		a53_1: cpu@101 {
+			compatible = "arm,cortex-a53","arm,armv8";
+			reg = <0x101>;
+			device_type = "cpu";
+			power-domains = <&sysc R8A7796_PD_CA53_CPU1>;
+			next-level-cache = <&L2_CA53>;
+			enable-method = "psci";
+		};
+
+		a53_2: cpu@102 {
+			compatible = "arm,cortex-a53","arm,armv8";
+			reg = <0x102>;
+			device_type = "cpu";
+			power-domains = <&sysc R8A7796_PD_CA53_CPU2>;
+			next-level-cache = <&L2_CA53>;
+			enable-method = "psci";
+		};
+
+		a53_3: cpu@103 {
+			compatible = "arm,cortex-a53","arm,armv8";
+			reg = <0x103>;
+			device_type = "cpu";
+			power-domains = <&sysc R8A7796_PD_CA53_CPU3>;
+			next-level-cache = <&L2_CA53>;
+			enable-method = "psci";
+		};
+
 		L2_CA57: cache-controller@0 {
 			compatible = "cache";
 			reg = <0>;
@@ -117,7 +153,7 @@
 			      <0x0 0xf1040000 0 0x20000>,
 			      <0x0 0xf1060000 0 0x20000>;
 			interrupts = <GIC_PPI 9
-					(GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_HIGH)>;
+					(GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_HIGH)>;
 			clocks = <&cpg CPG_MOD 408>;
 			clock-names = "clk";
 			power-domains = <&sysc R8A7796_PD_ALWAYS_ON>;
@@ -127,13 +163,13 @@
 		timer {
 			compatible = "arm,armv8-timer";
 			interrupts = <GIC_PPI 13
-					(GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_LOW)>,
+					(GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_LOW)>,
 				     <GIC_PPI 14
-					(GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_LOW)>,
+					(GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_LOW)>,
 				     <GIC_PPI 11
-					(GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_LOW)>,
+					(GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_LOW)>,
 				     <GIC_PPI 10
-					(GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_LOW)>;
+					(GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_LOW)>;
 		};
 
 		wdt0: watchdog@e6020000 {
-- 
1.9.1

Re: [PATCH 0/3] arm64: dts: r8a7796: Add Secondary CPU Cores

[Simon Horman]

On Fri, Feb 17, 2017 at 04:30:32PM +0100, Geert Uytterhoeven wrote:
> 	Hi Simon, Magnus,
> 
> This patch series adds the second Cortex-A57 CPU core, and the
> Cortex-A53 L2 cache-controller and CPU nodes on the Renesas R-Car M3-W
> SoC to its DTS file.
> 
> Note that these patches add hardware description; actual enabling of the
> CPU depends on the PSCI firmware.
> 
> With the current firmware version (v2.16.0), only the CA57 CPU cores are
> enabled, hence the last patch does not introduce undeterministic
> scheduling behavior due to migration between big and LITTLE cores.
> 
> Tested on r8a7796/salvator-x, with CPU hot(un)plug and system suspend.
> 
> Thanks for applying!
> 
> Geert Uytterhoeven (2):
>   arm64: dts: r8a7796: Add CA53 L2 cache-controller node
>   arm64: dts: r8a7796: Add Cortex-A53 CPU cores
> 
> Takeshi Kihara (1):
>   arm64: dts: r8a7796: Add Cortex-A57 CPU cores

Hi Geert,

thanks for your work in this area.

There seems to be some more work required to get patch 2/3 across the line
so I am holding off on applying this series for now.