diff mbox

ARM: vexpress: bindings: Add generic PD awareness to the spc controller

Message ID 1420562732-4353-1-git-send-email-mathieu.poirier@linaro.org
State New
Headers show

Commit Message

Mathieu Poirier Jan. 6, 2015, 4:45 p.m. UTC
From: Mathieu Poirier <mathieu.poirier@linaro.org>

Among other things, the serial power controller (SPC) controls power to
the A7 and A15 clusters.  Theses clusters also happen to contains the
coresight tracers used for HW assisted tracing.

By modellling these to power domains in a way that is comprehensible to
the generic power domain sub-system and using the runtime PM API in the
coresight drivers, we can prevent power to the domains from being
turned off while tracing related operations are still pending.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 .../bindings/arm/vexpress-power-controller.txt     | 54 ++++++++++++++++++++++
 1 file changed, 54 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/vexpress-power-controller.txt

Comments

Mathieu Poirier Jan. 6, 2015, 6:36 p.m. UTC | #1
On 6 January 2015 at 10:02, Rob Herring <robherring2@gmail.com> wrote:
> On Tue, Jan 6, 2015 at 10:45 AM,  <mathieu.poirier@linaro.org> wrote:
>> From: Mathieu Poirier <mathieu.poirier@linaro.org>
>>
>> Among other things, the serial power controller (SPC) controls power to
>> the A7 and A15 clusters.  Theses clusters also happen to contains the
>> coresight tracers used for HW assisted tracing.
>>
>> By modellling these to power domains in a way that is comprehensible to
>> the generic power domain sub-system and using the runtime PM API in the
>> coresight drivers, we can prevent power to the domains from being
>> turned off while tracing related operations are still pending.
>>
>> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
>> ---
>>  .../bindings/arm/vexpress-power-controller.txt     | 54 ++++++++++++++++++++++
>>  1 file changed, 54 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/arm/vexpress-power-controller.txt
>>
>> diff --git a/Documentation/devicetree/bindings/arm/vexpress-power-controller.txt b/Documentation/devicetree/bindings/arm/vexpress-power-controller.txt
>> new file mode 100644
>> index 000000000000..3af5624dc5cb
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/arm/vexpress-power-controller.txt
>> @@ -0,0 +1,54 @@
>> +ARM Versatile Express Power Controller
>> +--------------------------------------
>> +
>> +This binding models the serial power controller (SPC) in a way that is
>> +intelligible to the generic power domain subsystem and in accordance
>> +with the guidelines from:
>> +
>> +Documentation/devicetree/bindings/power/power_domain.txt
>> +
>> +The binding doesn't have a '<reg>' property as the base address for HW
>> +access is provided by the vexpress-scc sub-system.
>> +
>> +Required node properties:
>> +- compatible value : = "arm,vexpress-power-controller";
>> +- #power-domain-cells : = Number of cells in a PM domain specifier, as
>> +  specified in "power_domain.txt" referenced above.
>> +
>> +Example:
>> +       A7_A15_cluster_pd: A7-A15-cluster-pd {
>
> This is more a description of the power domain than the power
> controller IP block.

This one is a little tricky - the controller itself (SPC) is accessed
via registers mapped by another driver (SCC) and have two completely
separate files.  The SPC driver itself is not DT'ed, simply because it
doesn't have too,  hence writing things the way I did.

I will ask the vexpress maintainers where they want to see this code
going.  Would you accept this binding as part of the existing
"arm,vexpress-sysreg" [1] or even "arm,vexpress-scc" [2]?

[1]. Documentation/devicetree/bindings/arm/vexpress-sysreg.txt
[2]. Documentation/devicetree/bindings/arm/vexpress-scc.txt

>
>> +               compatible = "arm,vexpress-power-controller";
>> +               #power-domain-cells = <1>;
>
> You need to define what goes in the cell. That is specific to the
> power controller. It could be generic in that N corresponds to power
> domain N in the controller if the controller is generic in that way
> (i.e. all the register accesses are just indexed).

That is exactly how things are - one controller and two power domains.
Based on what the generic power domain code does #power-domain-cells
is the argument indicating what domain a device should be added to.
From what I read this is also how things are (sparsely) explained in
"power_domain.txt".  As such I'm not exactly sure of what you'd like
to see modified - enlightenment would be appreciated.

>
>> +        };
>> +
>> +       ....
>> +
>> +       ptm@0,2201d000 {
>
> Drop the comma.
>
>> +               compatible = "arm,coresight-etm3x", "arm,primecell";
>> +               reg = <0 0x2201d000 0 0x1000>;
>> +
>> +               cpu = <&cpu1>;
>> +               clocks = <&oscclk6a>;
>> +               clock-names = "apb_pclk";
>> +               power-domains = <&A7_A15_cluster_pd 0>;
>> +               port {
>> +                       ptm1_out_port: endpoint {
>> +                               remote-endpoint = <&funnel_in_port1>;
>> +                       };
>> +               };
>> +       };
>> +
>> +       etm@0,2203c000 {
>
> ditto
>
>> +               compatible = "arm,coresight-etm3x", "arm,primecell";
>> +               reg = <0 0x2203c000 0 0x1000>;
>> +
>> +               cpu = <&cpu2>;
>> +               clocks = <&oscclk6a>;
>> +               clock-names = "apb_pclk";
>> +               power-domains = <&A7_A15_cluster_pd 1>;
>> +               port {
>> +                       etm0_out_port: endpoint {
>> +                               remote-endpoint = <&funnel_in_port2>;
>> +                       };
>> +               };
>> +       };
>> --
>> 1.9.1
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Mathieu Poirier Jan. 7, 2015, 2:33 p.m. UTC | #2
On 7 January 2015 at 03:54, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote:
> On Tue, Jan 06, 2015 at 10:01:03PM +0000, Rob Herring wrote:
>> On Tue, Jan 6, 2015 at 12:36 PM, Mathieu Poirier
>> <mathieu.poirier@linaro.org> wrote:
>> > On 6 January 2015 at 10:02, Rob Herring <robherring2@gmail.com> wrote:
>> >> On Tue, Jan 6, 2015 at 10:45 AM,  <mathieu.poirier@linaro.org> wrote:
>> >>> From: Mathieu Poirier <mathieu.poirier@linaro.org>
>> >>>
>> >>> Among other things, the serial power controller (SPC) controls power to
>> >>> the A7 and A15 clusters.  Theses clusters also happen to contains the
>
> "These clusters also happen to contain"
>
>> >>> coresight tracers used for HW assisted tracing.
>> >>>
>> >>> By modellling these to power domains in a way that is comprehensible to
>                ^ too many l
>
>> >>> the generic power domain sub-system and using the runtime PM API in the
>> >>> coresight drivers, we can prevent power to the domains from being
>> >>> turned off while tracing related operations are still pending.
>> >>>
>> >>> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
>> >>> ---
>> >>>  .../bindings/arm/vexpress-power-controller.txt     | 54 ++++++++++++++++++++++
>> >>>  1 file changed, 54 insertions(+)
>> >>>  create mode 100644 Documentation/devicetree/bindings/arm/vexpress-power-controller.txt
>> >>>
>> >>> diff --git a/Documentation/devicetree/bindings/arm/vexpress-power-controller.txt b/Documentation/devicetree/bindings/arm/vexpress-power-controller.txt
>> >>> new file mode 100644
>> >>> index 000000000000..3af5624dc5cb
>> >>> --- /dev/null
>> >>> +++ b/Documentation/devicetree/bindings/arm/vexpress-power-controller.txt
>> >>> @@ -0,0 +1,54 @@
>> >>> +ARM Versatile Express Power Controller
>> >>> +--------------------------------------
>> >>> +
>> >>> +This binding models the serial power controller (SPC) in a way that is
>> >>> +intelligible to the generic power domain subsystem and in accordance
>> >>> +with the guidelines from:
>> >>> +
>> >>> +Documentation/devicetree/bindings/power/power_domain.txt
>> >>> +
>> >>> +The binding doesn't have a '<reg>' property as the base address for HW
>> >>> +access is provided by the vexpress-scc sub-system.
>> >>> +
>> >>> +Required node properties:
>> >>> +- compatible value : = "arm,vexpress-power-controller";
>> >>> +- #power-domain-cells : = Number of cells in a PM domain specifier, as
>> >>> +  specified in "power_domain.txt" referenced above.
>> >>> +
>> >>> +Example:
>> >>> +       A7_A15_cluster_pd: A7-A15-cluster-pd {
>> >>
>> >> This is more a description of the power domain than the power
>> >> controller IP block.
>> >
>> > This one is a little tricky - the controller itself (SPC) is accessed
>> > via registers mapped by another driver (SCC) and have two completely
>> > separate files.  The SPC driver itself is not DT'ed, simply because it
>> > doesn't have too,  hence writing things the way I did.
>> >
>> > I will ask the vexpress maintainers where they want to see this code
>> > going.  Would you accept this binding as part of the existing
>> > "arm,vexpress-sysreg" [1] or even "arm,vexpress-scc" [2]?
>
> We are not CC'ed :). [2] is my answer, see below.

Thanks for the review - does it also mean you'd like to see the
creation of the generic power domains moved to tc2_pm.c ?

>
>> > [1]. Documentation/devicetree/bindings/arm/vexpress-sysreg.txt
>> > [2]. Documentation/devicetree/bindings/arm/vexpress-scc.txt
>>
>> Yes, that makes sense. I think either you could have sub node for the
>> SPC or just use the sysreg node phandle directly. Given the
>> simplicity, I'd probably go for the latter, but I haven't looked
>> closely at these drivers and will defer to VExpress maintainers.
>
> There is no such a thing as "vexpress-power-controller", SCC (and SPC)
> are testchip specific, adding the bindings to [2] is what I would like to
> see, with a compatible string that is testchip specific, please refer
> to the bindings for details (ie arm,vexpress-scc,v2p-ca15_a7).
>
>>
>> >
>> >>
>> >>> +               compatible = "arm,vexpress-power-controller";
>> >>> +               #power-domain-cells = <1>;
>> >>
>> >> You need to define what goes in the cell. That is specific to the
>> >> power controller. It could be generic in that N corresponds to power
>> >> domain N in the controller if the controller is generic in that way
>> >> (i.e. all the register accesses are just indexed).
>> >
>> > That is exactly how things are - one controller and two power domains.
>> > Based on what the generic power domain code does #power-domain-cells
>> > is the argument indicating what domain a device should be added to.
>> > From what I read this is also how things are (sparsely) explained in
>> > "power_domain.txt".  As such I'm not exactly sure of what you'd like
>> > to see modified - enlightenment would be appreciated.
>>
>> I wasn't sure if there are more domains you didn't add. If what 0 and
>> 1 correspond to varies by board, then it's probably fine as it. If
>> they are always cluster 0 and cluster 1, then just say that.
>
> It is testchip specific (see above), and it should be documented as
> such. You should define what the cell number corresponds to (eg cell == 0 A15
> cluster power domain, cell == 1 A7 cluster power domain).
>
> Lorenzo
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/arm/vexpress-power-controller.txt b/Documentation/devicetree/bindings/arm/vexpress-power-controller.txt
new file mode 100644
index 000000000000..3af5624dc5cb
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/vexpress-power-controller.txt
@@ -0,0 +1,54 @@ 
+ARM Versatile Express Power Controller
+--------------------------------------
+
+This binding models the serial power controller (SPC) in a way that is
+intelligible to the generic power domain subsystem and in accordance
+with the guidelines from:
+
+Documentation/devicetree/bindings/power/power_domain.txt
+
+The binding doesn't have a '<reg>' property as the base address for HW
+access is provided by the vexpress-scc sub-system.
+
+Required node properties:
+- compatible value : = "arm,vexpress-power-controller";
+- #power-domain-cells : = Number of cells in a PM domain specifier, as
+  specified in "power_domain.txt" referenced above.
+
+Example:
+	A7_A15_cluster_pd: A7-A15-cluster-pd {
+		compatible = "arm,vexpress-power-controller";
+		#power-domain-cells = <1>;
+        };
+
+	....
+
+	ptm@0,2201d000 {
+		compatible = "arm,coresight-etm3x", "arm,primecell";
+		reg = <0 0x2201d000 0 0x1000>;
+
+		cpu = <&cpu1>;
+		clocks = <&oscclk6a>;
+		clock-names = "apb_pclk";
+		power-domains = <&A7_A15_cluster_pd 0>;
+		port {
+			ptm1_out_port: endpoint {
+				remote-endpoint = <&funnel_in_port1>;
+			};
+		};
+	};
+
+	etm@0,2203c000 {
+		compatible = "arm,coresight-etm3x", "arm,primecell";
+		reg = <0 0x2203c000 0 0x1000>;
+
+		cpu = <&cpu2>;
+		clocks = <&oscclk6a>;
+		clock-names = "apb_pclk";
+		power-domains = <&A7_A15_cluster_pd 1>;
+		port {
+			etm0_out_port: endpoint {
+				remote-endpoint = <&funnel_in_port2>;
+			};
+		};
+	};