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

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

[mathieu.poirier]

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 {
+		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>;
+			};
+		};
+	};
-- 
1.9.1

Re: [PATCH] ARM: vexpress: bindings: Add generic PD awareness to the spc controller

[Rob Herring]

On Tue, Jan 6, 2015 at 10:45 AM,  <mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> From: Mathieu Poirier <mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.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-QSEj5FYQhm4dnm+yROfE0A@public.gmane.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.

> +               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).

> +        };
> +
> +       ....
> +
> +       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 devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: vexpress: bindings: Add generic PD awareness to the spc controller

[Mathieu Poirier]

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
>>

Re: [PATCH] ARM: vexpress: bindings: Add generic PD awareness to the spc controller

[Rob Herring]

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
>>> 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

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.

>
>>
>>> +               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.

Rob

Re: [PATCH] ARM: vexpress: bindings: Add generic PD awareness to the spc controller

[Lorenzo Pieralisi]

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-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> > On 6 January 2015 at 10:02, Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >> On Tue, Jan 6, 2015 at 10:45 AM,  <mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> >>> From: Mathieu Poirier <mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.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-QSEj5FYQhm4dnm+yROfE0A@public.gmane.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.

> > [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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: vexpress: bindings: Add generic PD awareness to the spc controller

[Mathieu Poirier]

On 7 January 2015 at 03:54, Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org> 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-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>> > On 6 January 2015 at 10:02, Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> >> On Tue, Jan 6, 2015 at 10:45 AM,  <mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>> >>> From: Mathieu Poirier <mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.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-QSEj5FYQhm4dnm+yROfE0A@public.gmane.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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: vexpress: bindings: Add generic PD awareness to the spc controller

[Lorenzo Pieralisi]

On Wed, Jan 07, 2015 at 02:33:38PM +0000, Mathieu Poirier wrote:

[...]

> >> > 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 ?

Not necessarily, I think the creation is ok where it is, what we have
to do is to link the power domains to idle states instead of "disabling"
the spc power down command, see other thread.

I agree you need updated DT bindings for idle states to achieve that
and all the glue code that goes with it (in order for an idle state
to link to the power domain structs you are creating).

I copied in Kevin and Daniel since I know they are interested and are
working on this.

Thanks,
Lorenzo
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html