| Message ID | 1377872760.3655.48.camel@linaro1.home (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@1 { > device_type = "cpu"; > compatible = "arm,cortex-a15"; > reg = <1>; > + cci-control-port = <&cci_control1>; > }; > > cpu2: cpu@2 { > device_type = "cpu"; > compatible = "arm,cortex-a7"; > reg = <0x100>; > + cci-control-port = <&cci_control2>; > }; > > cpu3: cpu@3 { > device_type = "cpu"; > compatible = "arm,cortex-a7"; > reg = <0x101>; > + cci-control-port = <&cci_control2>; > }; > > cpu4: cpu@4 { > device_type = "cpu"; > compatible = "arm,cortex-a7"; > reg = <0x102>; > + cci-control-port = <&cci_control2>; > }; > }; > > @@ -104,6 +109,26 @@ > interrupts = <1 9 0xf04>; > }; > > + cci@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@4000 { > + compatible = "arm,cci-400-ctrl-if"; > + interface-type = "ace"; > + reg = <0x4000 0x1000>; > + }; > + > + cci_control2: slave-if@5000 { > + compatible = "arm,cci-400-ctrl-if"; > + interface-type = "ace"; > + reg = <0x5000 0x1000>; > + }; > + }; > + > memory-controller@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
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@1 { > device_type = "cpu"; > compatible = "arm,cortex-a15"; > reg = <1>; > + cci-control-port = <&cci_control1>; > }; > > cpu2: cpu@2 { > device_type = "cpu"; > compatible = "arm,cortex-a7"; > reg = <0x100>; > + cci-control-port = <&cci_control2>; > }; > > cpu3: cpu@3 { > device_type = "cpu"; > compatible = "arm,cortex-a7"; > reg = <0x101>; > + cci-control-port = <&cci_control2>; > }; > > cpu4: cpu@4 { > device_type = "cpu"; > compatible = "arm,cortex-a7"; > reg = <0x102>; > + cci-control-port = <&cci_control2>; > }; > }; > > @@ -104,6 +109,26 @@ > interrupts = <1 9 0xf04>; > }; > > + cci@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@4000 { > + compatible = "arm,cci-400-ctrl-if"; > + interface-type = "ace"; > + reg = <0x4000 0x1000>; > + }; > + > + cci_control2: slave-if@5000 { > + compatible = "arm,cci-400-ctrl-if"; > + interface-type = "ace"; > + reg = <0x5000 0x1000>; > + }; > + }; > + > memory-controller@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>
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
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
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
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@1 { device_type = "cpu"; compatible = "arm,cortex-a15"; reg = <1>; + cci-control-port = <&cci_control1>; }; cpu2: cpu@2 { device_type = "cpu"; compatible = "arm,cortex-a7"; reg = <0x100>; + cci-control-port = <&cci_control2>; }; cpu3: cpu@3 { device_type = "cpu"; compatible = "arm,cortex-a7"; reg = <0x101>; + cci-control-port = <&cci_control2>; }; cpu4: cpu@4 { device_type = "cpu"; compatible = "arm,cortex-a7"; reg = <0x102>; + cci-control-port = <&cci_control2>; }; }; @@ -104,6 +109,26 @@ interrupts = <1 9 0xf04>; }; + cci@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@4000 { + compatible = "arm,cci-400-ctrl-if"; + interface-type = "ace"; + reg = <0x4000 0x1000>; + }; + + cci_control2: slave-if@5000 { + compatible = "arm,cci-400-ctrl-if"; + interface-type = "ace"; + reg = <0x5000 0x1000>; + }; + }; + memory-controller@7ffd0000 { compatible = "arm,pl354", "arm,primecell"; reg = <0 0x7ffd0000 0 0x1000>;
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(+)