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@xxxxxxxxxx> wrote:
> On 6 January 2015 at 10:02, Rob Herring <robherring2@xxxxxxxxx> wrote:
>> On Tue, Jan 6, 2015 at 10:45 AM,  <mathieu.poirier@xxxxxxxxxx> wrote:
>>> From: Mathieu Poirier <mathieu.poirier@xxxxxxxxxx>
>>>
>>> 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@xxxxxxxxxx>
>>> ---
>>>  .../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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/