diff mbox series

[1/2] dt-bindings: power: Add ZynqMP power domain bindings

Message ID 1519775750-21297-2-git-send-email-jollys@xilinx.com
State Changes Requested, archived
Headers show
Series drivers: soc: xilinx: Add support for ZynqMP power domain driver | expand

Commit Message

Jolly Shah Feb. 27, 2018, 11:55 p.m. UTC
Add documentation to describe ZynqMP power domain bindings.

Signed-off-by: Jolly Shah <jollys@xilinx.com>
Signed-off-by: Rajan Vaja <rajanv@xilinx.com>
---
 .../devicetree/bindings/power/zynqmp-genpd.txt     | 46 ++++++++++++++++++++++
 1 file changed, 46 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/zynqmp-genpd.txt

Comments

Rob Herring (Arm) March 5, 2018, 10:39 p.m. UTC | #1
On Tue, Feb 27, 2018 at 03:55:49PM -0800, Jolly Shah wrote:
> Add documentation to describe ZynqMP power domain bindings.
> 
> Signed-off-by: Jolly Shah <jollys@xilinx.com>
> Signed-off-by: Rajan Vaja <rajanv@xilinx.com>
> ---
>  .../devicetree/bindings/power/zynqmp-genpd.txt     | 46 ++++++++++++++++++++++
>  1 file changed, 46 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/power/zynqmp-genpd.txt
> 
> diff --git a/Documentation/devicetree/bindings/power/zynqmp-genpd.txt b/Documentation/devicetree/bindings/power/zynqmp-genpd.txt
> new file mode 100644
> index 0000000..25f9711
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/zynqmp-genpd.txt
> @@ -0,0 +1,46 @@
> +Device Tree bindings for Xilinx Zynq MPSoC PM domains
> +
> +The binding for zynqmp-genpd follow the common generic PM domain binding[1].
> +
> +[1] Documentation/devicetree/bindings/power/power_domain.txt
> +
> +== Zynq MPSoC Generic PM Domain Node ==
> +
> +Required properties:
> + - compatible: Must be: "xlnx,zynqmp-genpd"

genpd is a Linux term. The DT should contain a power controller that 
controls physical power islands on a chip.

> +
> +This node contains a number of subnodes, each representing a single PM domain
> +that PM domain consumer devices reference.
> +
> +== PM Domain Nodes ==
> +
> +Required properties:
> + - #power-domain-cells: Number of cells in a PM domain specifier. Must be 0.
> + - pd-id: List of domain identifiers of as defined by platform firmware. These
> +	  identifiers are passed to the PM firmware.
> +
> +Example:
> +	zynqmp-genpd {
> +		compatible = "xlnx,zynqmp-genpd";

What's the control interface for controlling the domains?
> +
> +		pd_usb0: pd-usb0 {
> +			pd-id = <22>;
> +			#power-domain-cells = <0>;

There's no need for all these sub nodes. Make #power-domain-cells 1 and 
put the id in the cell value.

> +		};
> +
> +		pd_sata: pd-sata {
> +			pd-id = <28>;
> +			#power-domain-cells = <0>;
> +		};
> +
> +		pd_gpu: pd-gpu {
> +			pd-id = <58 20 21>;
> +			#power-domain-cells = <0x0>;
> +		};
> +	};
> +
> +	sata0: ahci@SATA_AHCI_HBA {
> +		...
> +		power-domains = <&pd_sata>;
> +		...
> +	};
> -- 
> 2.7.4
> 
--
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
Geert Uytterhoeven March 6, 2018, 7:59 a.m. UTC | #2
Hi Rob, Jolly,

On Mon, Mar 5, 2018 at 11:39 PM, Rob Herring <robh@kernel.org> wrote:
> On Tue, Feb 27, 2018 at 03:55:49PM -0800, Jolly Shah wrote:
>> Add documentation to describe ZynqMP power domain bindings.
>>
>> Signed-off-by: Jolly Shah <jollys@xilinx.com>
>> Signed-off-by: Rajan Vaja <rajanv@xilinx.com>
>> ---
>>  .../devicetree/bindings/power/zynqmp-genpd.txt     | 46 ++++++++++++++++++++++
>>  1 file changed, 46 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/power/zynqmp-genpd.txt
>>
>> diff --git a/Documentation/devicetree/bindings/power/zynqmp-genpd.txt b/Documentation/devicetree/bindings/power/zynqmp-genpd.txt
>> new file mode 100644
>> index 0000000..25f9711
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/power/zynqmp-genpd.txt

>> +This node contains a number of subnodes, each representing a single PM domain
>> +that PM domain consumer devices reference.
>> +
>> +== PM Domain Nodes ==
>> +
>> +Required properties:
>> + - #power-domain-cells: Number of cells in a PM domain specifier. Must be 0.
>> + - pd-id: List of domain identifiers of as defined by platform firmware. These
>> +       identifiers are passed to the PM firmware.
>> +
>> +Example:
>> +     zynqmp-genpd {
>> +             compatible = "xlnx,zynqmp-genpd";
>
> What's the control interface for controlling the domains?
>> +
>> +             pd_usb0: pd-usb0 {
>> +                     pd-id = <22>;
>> +                     #power-domain-cells = <0>;
>
> There's no need for all these sub nodes. Make #power-domain-cells 1 and
> put the id in the cell value.

That was my first reaction, too...
>
>> +             };
>> +
>> +             pd_sata: pd-sata {
>> +                     pd-id = <28>;
>> +                     #power-domain-cells = <0>;
>> +             };
>> +
>> +             pd_gpu: pd-gpu {
>> +                     pd-id = <58 20 21>;

... until I saw the above.
Controlling the GPU power area requires controlling 3 physical areas?

However, doing it this way may bite you in the future, if a need arises to
control a subset. And what about power up/down order?

>> +                     #power-domain-cells = <0x0>;
>> +             };
>> +     };

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
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
Marek Szyprowski March 6, 2018, 8:06 a.m. UTC | #3
Hi All,

On 2018-03-06 08:59, Geert Uytterhoeven wrote:
> Hi Rob, Jolly,
>
> On Mon, Mar 5, 2018 at 11:39 PM, Rob Herring <robh@kernel.org> wrote:
>> On Tue, Feb 27, 2018 at 03:55:49PM -0800, Jolly Shah wrote:
>>> Add documentation to describe ZynqMP power domain bindings.
>>>
>>> Signed-off-by: Jolly Shah <jollys@xilinx.com>
>>> Signed-off-by: Rajan Vaja <rajanv@xilinx.com>
>>> ---
>>>   .../devicetree/bindings/power/zynqmp-genpd.txt     | 46 ++++++++++++++++++++++
>>>   1 file changed, 46 insertions(+)
>>>   create mode 100644 Documentation/devicetree/bindings/power/zynqmp-genpd.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/power/zynqmp-genpd.txt b/Documentation/devicetree/bindings/power/zynqmp-genpd.txt
>>> new file mode 100644
>>> index 0000000..25f9711
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/power/zynqmp-genpd.txt
>>> +This node contains a number of subnodes, each representing a single PM domain
>>> +that PM domain consumer devices reference.
>>> +
>>> +== PM Domain Nodes ==
>>> +
>>> +Required properties:
>>> + - #power-domain-cells: Number of cells in a PM domain specifier. Must be 0.
>>> + - pd-id: List of domain identifiers of as defined by platform firmware. These
>>> +       identifiers are passed to the PM firmware.
>>> +
>>> +Example:
>>> +     zynqmp-genpd {
>>> +             compatible = "xlnx,zynqmp-genpd";
>> What's the control interface for controlling the domains?
>>> +
>>> +             pd_usb0: pd-usb0 {
>>> +                     pd-id = <22>;
>>> +                     #power-domain-cells = <0>;
>> There's no need for all these sub nodes. Make #power-domain-cells 1 and
>> put the id in the cell value.
> That was my first reaction, too...
>>> +             };
>>> +
>>> +             pd_sata: pd-sata {
>>> +                     pd-id = <28>;
>>> +                     #power-domain-cells = <0>;
>>> +             };
>>> +
>>> +             pd_gpu: pd-gpu {
>>> +                     pd-id = <58 20 21>;
> ... until I saw the above.
> Controlling the GPU power area requires controlling 3 physical areas?
>
> However, doing it this way may bite you in the future, if a need arises to
> control a subset. And what about power up/down order?

What about defining 3 separate domains and arranging them in parent-child
relationship? generic power domains already supports that and this allows
to nicely define the power on/off order.

>>> +                     #power-domain-cells = <0x0>;
>>> +             };
>>> +     };

Best regards
Jolly Shah March 15, 2018, 5:47 p.m. UTC | #4
Hi Rob, Geert, Marek,

> -----Original Message-----

> From: Marek Szyprowski [mailto:m.szyprowski@samsung.com]

> Sent: Tuesday, March 06, 2018 12:06 AM

> To: Geert Uytterhoeven <geert@linux-m68k.org>; Rob Herring

> <robh@kernel.org>

> Cc: Jolly Shah <JOLLYS@xilinx.com>; Matthias Brugger

> <matthias.bgg@gmail.com>; Andy Gross <andy.gross@linaro.org>; Shawn Guo

> <shawnguo@kernel.org>; Geert Uytterhoeven <geert+renesas@glider.be>;

> Björn Andersson <bjorn.andersson@linaro.org>; sean.wang@mediatek.com;

> Michal Simek <michal.simek@xilinx.com>; Mark Rutland

> <mark.rutland@arm.com>; Rajan Vaja <RAJANV@xilinx.com>; open list:OPEN

> FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

> <devicetree@vger.kernel.org>; Linux ARM <linux-arm-

> kernel@lists.infradead.org>; Linux Kernel Mailing List <linux-

> kernel@vger.kernel.org>; Jolly Shah <JOLLYS@xilinx.com>

> Subject: Re: [PATCH 1/2] dt-bindings: power: Add ZynqMP power domain

> bindings

> 

> Hi All,

> 

> On 2018-03-06 08:59, Geert Uytterhoeven wrote:

> > Hi Rob, Jolly,

> >

> > On Mon, Mar 5, 2018 at 11:39 PM, Rob Herring <robh@kernel.org> wrote:

> >> On Tue, Feb 27, 2018 at 03:55:49PM -0800, Jolly Shah wrote:

> >>> Add documentation to describe ZynqMP power domain bindings.

> >>>

> >>> Signed-off-by: Jolly Shah <jollys@xilinx.com>

> >>> Signed-off-by: Rajan Vaja <rajanv@xilinx.com>

> >>> ---

> >>>   .../devicetree/bindings/power/zynqmp-genpd.txt     | 46

> ++++++++++++++++++++++

> >>>   1 file changed, 46 insertions(+)

> >>>   create mode 100644

> >>> Documentation/devicetree/bindings/power/zynqmp-genpd.txt

> >>>

> >>> diff --git

> >>> a/Documentation/devicetree/bindings/power/zynqmp-genpd.txt

> >>> b/Documentation/devicetree/bindings/power/zynqmp-genpd.txt

> >>> new file mode 100644

> >>> index 0000000..25f9711

> >>> --- /dev/null

> >>> +++ b/Documentation/devicetree/bindings/power/zynqmp-genpd.txt

> >>> +This node contains a number of subnodes, each representing a single

> >>> +PM domain that PM domain consumer devices reference.

> >>> +

> >>> +== PM Domain Nodes ==

> >>> +

> >>> +Required properties:

> >>> + - #power-domain-cells: Number of cells in a PM domain specifier. Must be

> 0.

> >>> + - pd-id: List of domain identifiers of as defined by platform firmware.

> These

> >>> +       identifiers are passed to the PM firmware.

> >>> +

> >>> +Example:

> >>> +     zynqmp-genpd {

> >>> +             compatible = "xlnx,zynqmp-genpd";

> >> What's the control interface for controlling the domains?

> >>> +

> >>> +             pd_usb0: pd-usb0 {

> >>> +                     pd-id = <22>;

> >>> +                     #power-domain-cells = <0>;

> >> There's no need for all these sub nodes. Make #power-domain-cells 1

> >> and put the id in the cell value.

> > That was my first reaction, too...

> >>> +             };

> >>> +

> >>> +             pd_sata: pd-sata {

> >>> +                     pd-id = <28>;

> >>> +                     #power-domain-cells = <0>;

> >>> +             };

> >>> +

> >>> +             pd_gpu: pd-gpu {

> >>> +                     pd-id = <58 20 21>;

> > ... until I saw the above.

> > Controlling the GPU power area requires controlling 3 physical areas?

> >

> > However, doing it this way may bite you in the future, if a need

> > arises to control a subset. And what about power up/down order?

> 

> What about defining 3 separate domains and arranging them in parent-child

> relationship? generic power domains already supports that and this allows to

> nicely define the power on/off order.

> 

> >>> +                     #power-domain-cells = <0x0>;

> >>> +             };

> >>> +     };

> 


I agree it should be arranged in as parent child order to control subset or control order. Will incorporate those changes in next version.

> Best regards

> --

> Marek Szyprowski, PhD

> Samsung R&D Institute Poland
Jolly Shah May 17, 2018, 9:10 p.m. UTC | #5
Hi Marek,

> -----Original Message-----
> From: linux-kernel-owner@vger.kernel.org [mailto:linux-kernel-
> owner@vger.kernel.org] On Behalf Of Jolly Shah
> Sent: Thursday, March 15, 2018 10:47 AM
> To: Marek Szyprowski <m.szyprowski@samsung.com>; Geert Uytterhoeven
> <geert@linux-m68k.org>; Rob Herring <robh@kernel.org>
> Cc: Matthias Brugger <matthias.bgg@gmail.com>; Andy Gross
> <andy.gross@linaro.org>; Shawn Guo <shawnguo@kernel.org>; Geert
> Uytterhoeven <geert+renesas@glider.be>; Björn Andersson
> <bjorn.andersson@linaro.org>; sean.wang@mediatek.com; Michal Simek
> <michal.simek@xilinx.com>; Mark Rutland <mark.rutland@arm.com>; Rajan
> Vaja <RAJANV@xilinx.com>; open list:OPEN FIRMWARE AND FLATTENED
> DEVICE TREE BINDINGS <devicetree@vger.kernel.org>; Linux ARM <linux-arm-
> kernel@lists.infradead.org>; Linux Kernel Mailing List <linux-
> kernel@vger.kernel.org>
> Subject: RE: [PATCH 1/2] dt-bindings: power: Add ZynqMP power domain
> bindings
> 
> [This sender failed our fraud detection checks and may not be who they appear
> to be. Learn about spoofing at http://aka.ms/LearnAboutSpoofing]
> 
> Hi Rob, Geert, Marek,
> 
> > -----Original Message-----
> > From: Marek Szyprowski [mailto:m.szyprowski@samsung.com]
> > Sent: Tuesday, March 06, 2018 12:06 AM
> > To: Geert Uytterhoeven <geert@linux-m68k.org>; Rob Herring
> > <robh@kernel.org>
> > Cc: Jolly Shah <JOLLYS@xilinx.com>; Matthias Brugger
> > <matthias.bgg@gmail.com>; Andy Gross <andy.gross@linaro.org>; Shawn
> > Guo <shawnguo@kernel.org>; Geert Uytterhoeven
> > <geert+renesas@glider.be>; Björn Andersson
> > <bjorn.andersson@linaro.org>; sean.wang@mediatek.com; Michal Simek
> > <michal.simek@xilinx.com>; Mark Rutland <mark.rutland@arm.com>; Rajan
> > Vaja <RAJANV@xilinx.com>; open list:OPEN FIRMWARE AND FLATTENED
> DEVICE
> > TREE BINDINGS <devicetree@vger.kernel.org>; Linux ARM <linux-arm-
> > kernel@lists.infradead.org>; Linux Kernel Mailing List <linux-
> > kernel@vger.kernel.org>; Jolly Shah <JOLLYS@xilinx.com>
> > Subject: Re: [PATCH 1/2] dt-bindings: power: Add ZynqMP power domain
> > bindings
> >
> > Hi All,
> >
> > On 2018-03-06 08:59, Geert Uytterhoeven wrote:
> > > Hi Rob, Jolly,
> > >
> > > On Mon, Mar 5, 2018 at 11:39 PM, Rob Herring <robh@kernel.org> wrote:
> > >> On Tue, Feb 27, 2018 at 03:55:49PM -0800, Jolly Shah wrote:
> > >>> Add documentation to describe ZynqMP power domain bindings.
> > >>>
> > >>> Signed-off-by: Jolly Shah <jollys@xilinx.com>
> > >>> Signed-off-by: Rajan Vaja <rajanv@xilinx.com>
> > >>> ---
> > >>>   .../devicetree/bindings/power/zynqmp-genpd.txt     | 46
> > ++++++++++++++++++++++
> > >>>   1 file changed, 46 insertions(+)
> > >>>   create mode 100644
> > >>> Documentation/devicetree/bindings/power/zynqmp-genpd.txt
> > >>>
> > >>> diff --git
> > >>> a/Documentation/devicetree/bindings/power/zynqmp-genpd.txt
> > >>> b/Documentation/devicetree/bindings/power/zynqmp-genpd.txt
> > >>> new file mode 100644
> > >>> index 0000000..25f9711
> > >>> --- /dev/null
> > >>> +++ b/Documentation/devicetree/bindings/power/zynqmp-genpd.txt
> > >>> +This node contains a number of subnodes, each representing a
> > >>> +single PM domain that PM domain consumer devices reference.
> > >>> +
> > >>> +== PM Domain Nodes ==
> > >>> +
> > >>> +Required properties:
> > >>> + - #power-domain-cells: Number of cells in a PM domain specifier.
> > >>> +Must be
> > 0.
> > >>> + - pd-id: List of domain identifiers of as defined by platform firmware.
> > These
> > >>> +       identifiers are passed to the PM firmware.
> > >>> +
> > >>> +Example:
> > >>> +     zynqmp-genpd {
> > >>> +             compatible = "xlnx,zynqmp-genpd";
> > >> What's the control interface for controlling the domains?
> > >>> +
> > >>> +             pd_usb0: pd-usb0 {
> > >>> +                     pd-id = <22>;
> > >>> +                     #power-domain-cells = <0>;
> > >> There's no need for all these sub nodes. Make #power-domain-cells 1
> > >> and put the id in the cell value.
> > > That was my first reaction, too...
> > >>> +             };
> > >>> +
> > >>> +             pd_sata: pd-sata {
> > >>> +                     pd-id = <28>;
> > >>> +                     #power-domain-cells = <0>;
> > >>> +             };
> > >>> +
> > >>> +             pd_gpu: pd-gpu {
> > >>> +                     pd-id = <58 20 21>;
> > > ... until I saw the above.
> > > Controlling the GPU power area requires controlling 3 physical areas?
> > >
> > > However, doing it this way may bite you in the future, if a need
> > > arises to control a subset. And what about power up/down order?
> >
> > What about defining 3 separate domains and arranging them in
> > parent-child relationship? generic power domains already supports that
> > and this allows to nicely define the power on/off order.
> >
> > >>> +                     #power-domain-cells = <0x0>;
> > >>> +             };
> > >>> +     };
> >
> 
> I agree it should be arranged in as parent child order to control subset or control
> order. Will incorporate those changes in next version.
> 
> > Best regards
> > --
> > Marek Szyprowski, PhD
> > Samsung R&D Institute Poland


As suggested, I tried out parent, child approach. However what I found is Genpd core takes care of parent child dependencies for power on off routines only. In our case, We need them in attach-detach routines too. In that case, we need to handle dependencies manually for those routines. Please suggest better approach, if any.

Thanks,
Jolly Shah
Marek Szyprowski May 18, 2018, 6:30 a.m. UTC | #6
Hi Jolly,

On 2018-05-17 23:10, Jolly Shah wrote:

>>>>>> +Example:
>>>>>> +     zynqmp-genpd {
>>>>>> +             compatible = "xlnx,zynqmp-genpd";
>>>>> What's the control interface for controlling the domains?
>>>>>> +
>>>>>> +             pd_usb0: pd-usb0 {
>>>>>> +                     pd-id = <22>;
>>>>>> +                     #power-domain-cells = <0>;
>>>>> There's no need for all these sub nodes. Make #power-domain-cells 1
>>>>> and put the id in the cell value.
>>>> That was my first reaction, too...
>>>>>> +             };
>>>>>> +
>>>>>> +             pd_sata: pd-sata {
>>>>>> +                     pd-id = <28>;
>>>>>> +                     #power-domain-cells = <0>;
>>>>>> +             };
>>>>>> +
>>>>>> +             pd_gpu: pd-gpu {
>>>>>> +                     pd-id = <58 20 21>;
>>>> ... until I saw the above.
>>>> Controlling the GPU power area requires controlling 3 physical areas?
>>>>
>>>> However, doing it this way may bite you in the future, if a need
>>>> arises to control a subset. And what about power up/down order?
>>> What about defining 3 separate domains and arranging them in
>>> parent-child relationship? generic power domains already supports that
>>> and this allows to nicely define the power on/off order.
>>>
>>>>>> +                     #power-domain-cells = <0x0>;
>>>>>> +             };
>>>>>> +     };
>> I agree it should be arranged in as parent child order to control subset or control
>> order. Will incorporate those changes in next version.
>
> As suggested, I tried out parent, child approach. However what I found is Genpd core takes care of parent child dependencies for power on off routines only. In our case, We need them in attach-detach routines too. In that case, we need to handle dependencies manually for those routines. Please suggest better approach, if any.

What do you mean to handle attach-detach?

Best regards
Jolly Shah May 18, 2018, 9:18 p.m. UTC | #7
Hi Marek,

> -----Original Message-----
> From: Marek Szyprowski [mailto:m.szyprowski@samsung.com]
> Sent: Thursday, May 17, 2018 11:31 PM
> To: Jolly Shah <JOLLYS@xilinx.com>; Geert Uytterhoeven <geert@linux-
> m68k.org>; Rob Herring <robh@kernel.org>
> Cc: Matthias Brugger <matthias.bgg@gmail.com>; Andy Gross
> <andy.gross@linaro.org>; Shawn Guo <shawnguo@kernel.org>; Geert
> Uytterhoeven <geert+renesas@glider.be>; Björn Andersson
> <bjorn.andersson@linaro.org>; sean.wang@mediatek.com; Michal Simek
> <michal.simek@xilinx.com>; Mark Rutland <mark.rutland@arm.com>; Rajan
> Vaja <RAJANV@xilinx.com>; open list:OPEN FIRMWARE AND FLATTENED
> DEVICE TREE BINDINGS <devicetree@vger.kernel.org>; Linux ARM <linux-arm-
> kernel@lists.infradead.org>; Linux Kernel Mailing List <linux-
> kernel@vger.kernel.org>
> Subject: Re: [PATCH 1/2] dt-bindings: power: Add ZynqMP power domain
> bindings
> 
> Hi Jolly,
> 
> On 2018-05-17 23:10, Jolly Shah wrote:
> 
> >>>>>> +Example:
> >>>>>> +     zynqmp-genpd {
> >>>>>> +             compatible = "xlnx,zynqmp-genpd";
> >>>>> What's the control interface for controlling the domains?
> >>>>>> +
> >>>>>> +             pd_usb0: pd-usb0 {
> >>>>>> +                     pd-id = <22>;
> >>>>>> +                     #power-domain-cells = <0>;
> >>>>> There's no need for all these sub nodes. Make #power-domain-cells
> >>>>> 1 and put the id in the cell value.
> >>>> That was my first reaction, too...
> >>>>>> +             };
> >>>>>> +
> >>>>>> +             pd_sata: pd-sata {
> >>>>>> +                     pd-id = <28>;
> >>>>>> +                     #power-domain-cells = <0>;
> >>>>>> +             };
> >>>>>> +
> >>>>>> +             pd_gpu: pd-gpu {
> >>>>>> +                     pd-id = <58 20 21>;
> >>>> ... until I saw the above.
> >>>> Controlling the GPU power area requires controlling 3 physical areas?
> >>>>
> >>>> However, doing it this way may bite you in the future, if a need
> >>>> arises to control a subset. And what about power up/down order?
> >>> What about defining 3 separate domains and arranging them in
> >>> parent-child relationship? generic power domains already supports
> >>> that and this allows to nicely define the power on/off order.
> >>>
> >>>>>> +                     #power-domain-cells = <0x0>;
> >>>>>> +             };
> >>>>>> +     };
> >> I agree it should be arranged in as parent child order to control
> >> subset or control order. Will incorporate those changes in next version.
> >
> > As suggested, I tried out parent, child approach. However what I found is
> Genpd core takes care of parent child dependencies for power on off routines
> only. In our case, We need them in attach-detach routines too. In that case, we
> need to handle dependencies manually for those routines. Please suggest better
> approach, if any.
> 
> What do you mean to handle attach-detach?
> 
> Best regards
> --
> Marek Szyprowski, PhD
> Samsung R&D Institute Poland

For our power domain driver,  we request usage of these nodes in attach routines and power them on in power on routine. So for below specific case, when attach_dev is called, all 3 nodes need to be requested.

> >>>>>> +             pd_gpu: pd-gpu {
> >>>>>> +                     pd-id = <58 20 21>;

Thanks,
Jolly Shah
Jolly Shah July 20, 2018, 4:57 p.m. UTC | #8
Hi Marek,

> -----Original Message-----
> From: Jolly Shah
> Sent: Friday, May 18, 2018 2:19 PM
> To: 'Marek Szyprowski' <m.szyprowski@samsung.com>
> Cc: Matthias Brugger <matthias.bgg@gmail.com>; Andy Gross
> <andy.gross@linaro.org>; Shawn Guo <shawnguo@kernel.org>; Geert
> Uytterhoeven <geert+renesas@glider.be>; Björn Andersson
> <bjorn.andersson@linaro.org>; sean.wang@mediatek.com; Michal Simek
> <michal.simek@xilinx.com>; Mark Rutland <mark.rutland@arm.com>; Rajan
> Vaja <RAJANV@xilinx.com>; open list:OPEN FIRMWARE AND FLATTENED
> DEVICE TREE BINDINGS <devicetree@vger.kernel.org>; Linux ARM <linux-arm-
> kernel@lists.infradead.org>; Linux Kernel Mailing List <linux-
> kernel@vger.kernel.org>; Geert Uytterhoeven <geert@linux-m68k.org>; Rob
> Herring <robh@kernel.org>
> Subject: RE: [PATCH 1/2] dt-bindings: power: Add ZynqMP power domain
> bindings
> 
> Hi Marek,
> 
> > -----Original Message-----
> > From: Marek Szyprowski [mailto:m.szyprowski@samsung.com]
> > Sent: Thursday, May 17, 2018 11:31 PM
> > To: Jolly Shah <JOLLYS@xilinx.com>; Geert Uytterhoeven <geert@linux-
> > m68k.org>; Rob Herring <robh@kernel.org>
> > Cc: Matthias Brugger <matthias.bgg@gmail.com>; Andy Gross
> > <andy.gross@linaro.org>; Shawn Guo <shawnguo@kernel.org>; Geert
> > Uytterhoeven <geert+renesas@glider.be>; Björn Andersson
> > <bjorn.andersson@linaro.org>; sean.wang@mediatek.com; Michal Simek
> > <michal.simek@xilinx.com>; Mark Rutland <mark.rutland@arm.com>; Rajan
> > Vaja <RAJANV@xilinx.com>; open list:OPEN FIRMWARE AND FLATTENED
> DEVICE
> > TREE BINDINGS <devicetree@vger.kernel.org>; Linux ARM <linux-arm-
> > kernel@lists.infradead.org>; Linux Kernel Mailing List <linux-
> > kernel@vger.kernel.org>
> > Subject: Re: [PATCH 1/2] dt-bindings: power: Add ZynqMP power domain
> > bindings
> >
> > Hi Jolly,
> >
> > On 2018-05-17 23:10, Jolly Shah wrote:
> >
> > >>>>>> +Example:
> > >>>>>> +     zynqmp-genpd {
> > >>>>>> +             compatible = "xlnx,zynqmp-genpd";
> > >>>>> What's the control interface for controlling the domains?
> > >>>>>> +
> > >>>>>> +             pd_usb0: pd-usb0 {
> > >>>>>> +                     pd-id = <22>;
> > >>>>>> +                     #power-domain-cells = <0>;
> > >>>>> There's no need for all these sub nodes. Make
> > >>>>> #power-domain-cells
> > >>>>> 1 and put the id in the cell value.
> > >>>> That was my first reaction, too...
> > >>>>>> +             };
> > >>>>>> +
> > >>>>>> +             pd_sata: pd-sata {
> > >>>>>> +                     pd-id = <28>;
> > >>>>>> +                     #power-domain-cells = <0>;
> > >>>>>> +             };
> > >>>>>> +
> > >>>>>> +             pd_gpu: pd-gpu {
> > >>>>>> +                     pd-id = <58 20 21>;
> > >>>> ... until I saw the above.
> > >>>> Controlling the GPU power area requires controlling 3 physical areas?
> > >>>>
> > >>>> However, doing it this way may bite you in the future, if a need
> > >>>> arises to control a subset. And what about power up/down order?
> > >>> What about defining 3 separate domains and arranging them in
> > >>> parent-child relationship? generic power domains already supports
> > >>> that and this allows to nicely define the power on/off order.
> > >>>
> > >>>>>> +                     #power-domain-cells = <0x0>;
> > >>>>>> +             };
> > >>>>>> +     };
> > >> I agree it should be arranged in as parent child order to control
> > >> subset or control order. Will incorporate those changes in next version.
> > >
> > > As suggested, I tried out parent, child approach. However what I
> > > found is
> > Genpd core takes care of parent child dependencies for power on off
> > routines only. In our case, We need them in attach-detach routines
> > too. In that case, we need to handle dependencies manually for those
> > routines. Please suggest better approach, if any.
> >
> > What do you mean to handle attach-detach?
> >
> > Best regards
> > --
> > Marek Szyprowski, PhD
> > Samsung R&D Institute Poland
> 
> For our power domain driver,  we request usage of these nodes in attach
> routines and power them on in power on routine. So for below specific case,
> when attach_dev is called, all 3 nodes need to be requested.
> 
> > >>>>>> +             pd_gpu: pd-gpu {
> > >>>>>> +                     pd-id = <58 20 21>;
> 
> Thanks,
> Jolly Shah
> 

Please let me know your opinion.

Thanks,
Jolly Shah
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/power/zynqmp-genpd.txt b/Documentation/devicetree/bindings/power/zynqmp-genpd.txt
new file mode 100644
index 0000000..25f9711
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/zynqmp-genpd.txt
@@ -0,0 +1,46 @@ 
+Device Tree bindings for Xilinx Zynq MPSoC PM domains
+
+The binding for zynqmp-genpd follow the common generic PM domain binding[1].
+
+[1] Documentation/devicetree/bindings/power/power_domain.txt
+
+== Zynq MPSoC Generic PM Domain Node ==
+
+Required properties:
+ - compatible: Must be: "xlnx,zynqmp-genpd"
+
+This node contains a number of subnodes, each representing a single PM domain
+that PM domain consumer devices reference.
+
+== PM Domain Nodes ==
+
+Required properties:
+ - #power-domain-cells: Number of cells in a PM domain specifier. Must be 0.
+ - pd-id: List of domain identifiers of as defined by platform firmware. These
+	  identifiers are passed to the PM firmware.
+
+Example:
+	zynqmp-genpd {
+		compatible = "xlnx,zynqmp-genpd";
+
+		pd_usb0: pd-usb0 {
+			pd-id = <22>;
+			#power-domain-cells = <0>;
+		};
+
+		pd_sata: pd-sata {
+			pd-id = <28>;
+			#power-domain-cells = <0>;
+		};
+
+		pd_gpu: pd-gpu {
+			pd-id = <58 20 21>;
+			#power-domain-cells = <0x0>;
+		};
+	};
+
+	sata0: ahci@SATA_AHCI_HBA {
+		...
+		power-domains = <&pd_sata>;
+		...
+	};