[PATCH] ARM: dts: vexpress: Add CCI node to TC2 device-tree

[PATCH] ARM: dts: vexpress: Add CCI node to TC2 device-tree

[Jon Medhurst (Tixy)]

The Versatile Express V2P-CA15_A7 (aka TC2) has a CCI-400 which is
needed to get Multi-Cluster Power Management (MCPM) working.

Signed-off-by: Jon Medhurst <tixy@linaro.org>
---

I was unsure if the devicetree list should be cc'd when making use of
already agreed and documented bindings, so I erred on the side of
caution and added it. Please say if this is unnecessary noise, or is
expected for all device-tree changes, thanks.

 arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts |   25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts b/arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts
index d2803be..12bd4ea 100644
--- a/arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts
+++ b/arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts
@@ -37,30 +37,35 @@
 			device_type = "cpu";
 			compatible = "arm,cortex-a15";
 			reg = <0>;
+			cci-control-port = <&cci_control1>;
 		};
 
 		cpu1: cpu at 1 {
 			device_type = "cpu";
 			compatible = "arm,cortex-a15";
 			reg = <1>;
+			cci-control-port = <&cci_control1>;
 		};
 
 		cpu2: cpu at 2 {
 			device_type = "cpu";
 			compatible = "arm,cortex-a7";
 			reg = <0x100>;
+			cci-control-port = <&cci_control2>;
 		};
 
 		cpu3: cpu at 3 {
 			device_type = "cpu";
 			compatible = "arm,cortex-a7";
 			reg = <0x101>;
+			cci-control-port = <&cci_control2>;
 		};
 
 		cpu4: cpu at 4 {
 			device_type = "cpu";
 			compatible = "arm,cortex-a7";
 			reg = <0x102>;
+			cci-control-port = <&cci_control2>;
 		};
 	};
 
@@ -104,6 +109,26 @@
 		interrupts = <1 9 0xf04>;
 	};
 
+	cci at 2c090000 {
+		compatible = "arm,cci-400";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		reg = <0 0x2c090000 0 0x1000>;
+		ranges = <0x0 0x0 0x2c090000 0x10000>;
+
+		cci_control1: slave-if at 4000 {
+			compatible = "arm,cci-400-ctrl-if";
+			interface-type = "ace";
+			reg = <0x4000 0x1000>;
+		};
+
+		cci_control2: slave-if at 5000 {
+			compatible = "arm,cci-400-ctrl-if";
+			interface-type = "ace";
+			reg = <0x5000 0x1000>;
+		};
+	};
+
 	memory-controller at 7ffd0000 {
 		compatible = "arm,pl354", "arm,primecell";
 		reg = <0 0x7ffd0000 0 0x1000>;
-- 
1.7.10.4

[PATCH] ARM: dts: vexpress: Add CCI node to TC2 device-tree

[Pawel Moll]

On Fri, 2013-08-30 at 15:26 +0100, Jon Medhurst (Tixy) wrote:
> The Versatile Express V2P-CA15_A7 (aka TC2) has a CCI-400 which is
> needed to get Multi-Cluster Power Management (MCPM) working.
> 
> Signed-off-by: Jon Medhurst <tixy@linaro.org>
> ---
> 
> I was unsure if the devicetree list should be cc'd when making use of
> already agreed and documented bindings, so I erred on the side of
> caution and added it. Please say if this is unnecessary noise, or is
> expected for all device-tree changes, thanks.

MAINTAINERS say:

F:      arch/*/boot/dts/

so you did right. The reality is that there is more focus on
Documentation/devicetree/bindings/* than on *.dts? and if the changes
are complainant they will be most likely sort-of-ignored...

>  arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts |   25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts b/arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts
> index d2803be..12bd4ea 100644
> --- a/arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts
> +++ b/arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts
> @@ -37,30 +37,35 @@
>  			device_type = "cpu";
>  			compatible = "arm,cortex-a15";
>  			reg = <0>;
> +			cci-control-port = <&cci_control1>;
>  		};
>  
>  		cpu1: cpu at 1 {
>  			device_type = "cpu";
>  			compatible = "arm,cortex-a15";
>  			reg = <1>;
> +			cci-control-port = <&cci_control1>;
>  		};
>  
>  		cpu2: cpu at 2 {
>  			device_type = "cpu";
>  			compatible = "arm,cortex-a7";
>  			reg = <0x100>;
> +			cci-control-port = <&cci_control2>;
>  		};
>  
>  		cpu3: cpu at 3 {
>  			device_type = "cpu";
>  			compatible = "arm,cortex-a7";
>  			reg = <0x101>;
> +			cci-control-port = <&cci_control2>;
>  		};
>  
>  		cpu4: cpu at 4 {
>  			device_type = "cpu";
>  			compatible = "arm,cortex-a7";
>  			reg = <0x102>;
> +			cci-control-port = <&cci_control2>;
>  		};
>  	};
>  
> @@ -104,6 +109,26 @@
>  		interrupts = <1 9 0xf04>;
>  	};
>  
> +	cci at 2c090000 {
> +		compatible = "arm,cci-400";
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		reg = <0 0x2c090000 0 0x1000>;
> +		ranges = <0x0 0x0 0x2c090000 0x10000>;
> +
> +		cci_control1: slave-if at 4000 {
> +			compatible = "arm,cci-400-ctrl-if";
> +			interface-type = "ace";
> +			reg = <0x4000 0x1000>;
> +		};
> +
> +		cci_control2: slave-if at 5000 {
> +			compatible = "arm,cci-400-ctrl-if";
> +			interface-type = "ace";
> +			reg = <0x5000 0x1000>;
> +		};
> +	};
> +
>  	memory-controller at 7ffd0000 {
>  		compatible = "arm,pl354", "arm,primecell";
>  		reg = <0 0x7ffd0000 0 0x1000>;

With my VE-hat on, I can only say that if Lorenzo says it's fine, it's
fine :-) So:

Acked-by: Pawel Moll <pawel.moll@arm.com>

Now, it seems that we will really need it in 3.12. The problem is it's
quite late for this... Would arm-soc maintainers consider taking it in
on last-minute basis or should we wait to rc1 and post it then as a fix?

Thanks!

Pawel

[PATCH] ARM: dts: vexpress: Add CCI node to TC2 device-tree

[Lorenzo Pieralisi]

On Fri, Aug 30, 2013 at 03:26:00PM +0100, Jon Medhurst (Tixy) wrote:
> The Versatile Express V2P-CA15_A7 (aka TC2) has a CCI-400 which is
> needed to get Multi-Cluster Power Management (MCPM) working.
> 
> Signed-off-by: Jon Medhurst <tixy@linaro.org>
> ---
> 
> I was unsure if the devicetree list should be cc'd when making use of
> already agreed and documented bindings, so I erred on the side of
> caution and added it. Please say if this is unnecessary noise, or is
> expected for all device-tree changes, thanks.
> 
>  arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts |   25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts b/arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts
> index d2803be..12bd4ea 100644
> --- a/arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts
> +++ b/arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts
> @@ -37,30 +37,35 @@
>  			device_type = "cpu";
>  			compatible = "arm,cortex-a15";
>  			reg = <0>;
> +			cci-control-port = <&cci_control1>;
>  		};
>  
>  		cpu1: cpu at 1 {
>  			device_type = "cpu";
>  			compatible = "arm,cortex-a15";
>  			reg = <1>;
> +			cci-control-port = <&cci_control1>;
>  		};
>  
>  		cpu2: cpu at 2 {
>  			device_type = "cpu";
>  			compatible = "arm,cortex-a7";
>  			reg = <0x100>;
> +			cci-control-port = <&cci_control2>;
>  		};
>  
>  		cpu3: cpu at 3 {
>  			device_type = "cpu";
>  			compatible = "arm,cortex-a7";
>  			reg = <0x101>;
> +			cci-control-port = <&cci_control2>;
>  		};
>  
>  		cpu4: cpu at 4 {
>  			device_type = "cpu";
>  			compatible = "arm,cortex-a7";
>  			reg = <0x102>;
> +			cci-control-port = <&cci_control2>;
>  		};
>  	};
>  
> @@ -104,6 +109,26 @@
>  		interrupts = <1 9 0xf04>;
>  	};
>  
> +	cci at 2c090000 {
> +		compatible = "arm,cci-400";
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		reg = <0 0x2c090000 0 0x1000>;
> +		ranges = <0x0 0x0 0x2c090000 0x10000>;
> +
> +		cci_control1: slave-if at 4000 {
> +			compatible = "arm,cci-400-ctrl-if";
> +			interface-type = "ace";
> +			reg = <0x4000 0x1000>;
> +		};
> +
> +		cci_control2: slave-if at 5000 {
> +			compatible = "arm,cci-400-ctrl-if";
> +			interface-type = "ace";
> +			reg = <0x5000 0x1000>;
> +		};
> +	};
> +
>  	memory-controller at 7ffd0000 {
>  		compatible = "arm,pl354", "arm,primecell";
>  		reg = <0 0x7ffd0000 0 0x1000>;

Now that MCPM and CPU idle are heading to mainline, it should get queued asap,
thanks for that, probably as a fix at -rc1, since it is quite late now.

Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>

[PATCH] ARM: dts: vexpress: Add CCI node to TC2 device-tree

[Olof Johansson]

On Fri, Aug 30, 2013 at 9:13 AM, Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:

> Now that MCPM and CPU idle are heading to mainline, it should get queued asap,
> thanks for that, probably as a fix at -rc1, since it is quite late now.

This is not a regression, it's a missing piece of a feature. It's not
post-rc1 material.

It's also an indication that nobody actually tested the feature branch
before the pull request was sent. Hrmpf.

Shouldn't you be updating some vexpress_defconfig and probably
multi_v7_defconfig to at least get some build coverage for this as
well, by the way?

I really should hold this off for 3.13, but I'll apply it to the late/
branch anyway.


-Olof

[PATCH] ARM: dts: vexpress: Add CCI node to TC2 device-tree

[Pawel Moll]

On Fri, 2013-08-30 at 17:30 +0100, Olof Johansson wrote:
> Shouldn't you be updating some vexpress_defconfig and probably
> multi_v7_defconfig to at least get some build coverage for this as
> well, by the way?

Yeah, I've been told not to kill vexpress_defconfig after all, so I'll
refresh it in the next cycle.

Pawel 

[PATCH] ARM: dts: vexpress: Add CCI node to TC2 device-tree

[Lorenzo Pieralisi]

On Fri, Aug 30, 2013 at 05:30:17PM +0100, Olof Johansson wrote:
> On Fri, Aug 30, 2013 at 9:13 AM, Lorenzo Pieralisi
> <lorenzo.pieralisi@arm.com> wrote:
> 
> > Now that MCPM and CPU idle are heading to mainline, it should get queued asap,
> > thanks for that, probably as a fix at -rc1, since it is quite late now.
> 
> This is not a regression, it's a missing piece of a feature. It's not
> post-rc1 material.

It is an incomplete dts, which has been true since the dts file for TC2 has
been merged in the kernel or better since the CCI bindings got merged in
the kernel.

> It's also an indication that nobody actually tested the feature branch
> before the pull request was sent. Hrmpf.

It has been tested and we were aware that the dts in the kernel was missing
the CCI node, we were not really sure about what to do given the ongoing
discussions on dts in the kernel and so on and considered that we might
get the patch in as a fix eventually; we made a mistake, sorry.

> Shouldn't you be updating some vexpress_defconfig and probably
> multi_v7_defconfig to at least get some build coverage for this as
> well, by the way?
> 
> I really should hold this off for 3.13, but I'll apply it to the late/
> branch anyway.

Thanks,
Lorenzo