diff mbox series

[v4,2/4] dt-bindings: power: rockchip: Convert to json-schema

Message ID 20210324071609.7531-3-zhangqing@rock-chips.com (mailing list archive)
State New
Headers show
Series soc: rockchip: power-domain: add rk3568 powerdomains | expand

Commit Message

Elaine Zhang March 24, 2021, 7:16 a.m. UTC
Convert the soc/rockchip/power_domain.txt binding document to
json-schema and move to the power bindings directory.

Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
---
 .../power/rockchip,power-controller.yaml      | 284 ++++++++++++++++++
 .../bindings/soc/rockchip/power_domain.txt    | 136 ---------
 2 files changed, 284 insertions(+), 136 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
 delete mode 100644 Documentation/devicetree/bindings/soc/rockchip/power_domain.txt

Comments

Johan Jonker March 24, 2021, 9:17 a.m. UTC | #1
Hi Elaine,

From Rob's build log it turns out that 2 more properties must be added.
Add these new properties in separate patch.
Retest with commands below.
====
See rk3288.dtsi

			assigned-clocks = <&cru SCLK_EDP_24M>;
			assigned-clock-parents = <&xin24m>;
====
https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20210324071609.7531-3-zhangqing@rock-chips.com/

make ARCH=arm dtbs_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/power/rockchip,power-controller.yaml

make ARCH=arm64 dtbs_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/power/rockchip,power-controller.yaml

On 3/24/21 8:16 AM, Elaine Zhang wrote:
> Convert the soc/rockchip/power_domain.txt binding document to
> json-schema and move to the power bindings directory.
> 
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
> ---
>  .../power/rockchip,power-controller.yaml      | 284 ++++++++++++++++++
>  .../bindings/soc/rockchip/power_domain.txt    | 136 ---------
>  2 files changed, 284 insertions(+), 136 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
>  delete mode 100644 Documentation/devicetree/bindings/soc/rockchip/power_domain.txt
> 
> diff --git a/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml b/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
> new file mode 100644
> index 000000000000..a220322c5139
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
> @@ -0,0 +1,284 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/power/rockchip,power-controller.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Rockchip Power Domains
> +
> +maintainers:
> +  - Elaine Zhang <zhangqing@rock-chips.com>
> +  - Rob Herring <robh+dt@kernel.org>
> +  - Heiko Stuebner <heiko@sntech.de>
> +
> +description: |
> +  Rockchip processors include support for multiple power domains which can be
> +  powered up/down by software based on different application scenarios to save power.
> +
> +  Power domains contained within power-controller node are generic power domain
> +  providers documented in Documentation/devicetree/bindings/power/power-domain.yaml.
> +
> +  IP cores belonging to a power domain should contain a "power-domains"
> +  property that is a phandle for the power domain node representing the domain.
> +
> +properties:
> +  $nodename:
> +    const: power-controller
> +
> +  compatible:
> +    enum:
> +      - rockchip,px30-power-controller
> +      - rockchip,rk3036-power-controller
> +      - rockchip,rk3066-power-controller
> +      - rockchip,rk3128-power-controller
> +      - rockchip,rk3188-power-controller
> +      - rockchip,rk3228-power-controller
> +      - rockchip,rk3288-power-controller
> +      - rockchip,rk3328-power-controller
> +      - rockchip,rk3366-power-controller
> +      - rockchip,rk3368-power-controller
> +      - rockchip,rk3399-power-controller
> +
> +  "#power-domain-cells":
> +    const: 1
> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 0


assigned-clocks:
  maxItems: 1

assigned-clock-parents:
  maxItems: 1

> +
> +patternProperties:
> +  "^pd_[0-9a-z_]{2,10}@[0-9a-f]+$":
> +    type: object
> +    description: |
> +      Represents the power domains within the power controller node as documented
> +      in Documentation/devicetree/bindings/power/power-domain.yaml.
> +
> +    properties:
> +
> +      "#power-domain-cells":
> +        description:
> +          Must be 0 for nodes representing a single PM domain and 1 for nodes
> +          providing multiple PM domains.
> +
> +      "#address-cells":
> +        const: 1
> +
> +      "#size-cells":
> +        const: 0
> +
> +      reg:
> +        maxItems: 1
> +        description: |
> +          Power domain index. Valid values are defined in
> +          "include/dt-bindings/power/px30-power.h"
> +          "include/dt-bindings/power/rk3036-power.h"
> +          "include/dt-bindings/power/rk3066-power.h"
> +          "include/dt-bindings/power/rk3128-power.h"
> +          "include/dt-bindings/power/rk3188-power.h"
> +          "include/dt-bindings/power/rk3228-power.h"
> +          "include/dt-bindings/power/rk3288-power.h"
> +          "include/dt-bindings/power/rk3328-power.h"
> +          "include/dt-bindings/power/rk3366-power.h"
> +          "include/dt-bindings/power/rk3368-power.h"
> +          "include/dt-bindings/power/rk3399-power.h"
> +
> +      clocks:
> +        description: |
> +          A number of phandles to clocks that need to be enabled while power domain
> +          switches state.
> +
> +      pm_qos:
> +        description: |
> +          A number of phandles to qos blocks which need to be saved and restored
> +          while power domain switches state.
> +
> +    patternProperties:
> +      "^pd_[0-9a-z_]{2,10}@[0-9a-f]+$":
> +        type: object
> +        description: |
> +          Represents a power domain child within a power domain parent node.
> +
> +        properties:
> +
> +          "#power-domain-cells":
> +            description:
> +              Must be 0 for nodes representing a single PM domain and 1 for nodes
> +              providing multiple PM domains.
> +
> +          "#address-cells":
> +            const: 1
> +
> +          "#size-cells":
> +            const: 0
> +
> +          reg:
> +            maxItems: 1
> +
> +          clocks:
> +            description: |
> +              A number of phandles to clocks that need to be enabled while power domain
> +              switches state.
> +
> +          pm_qos:
> +            description: |
> +              A number of phandles to qos blocks which need to be saved and restored
> +              while power domain switches state.
> +
> +        patternProperties:
> +          "^pd_[0-9a-z_]{2,10}@[0-9a-f]+$":
> +            type: object
> +            description: |
> +              Represents a power domain child within a power domain parent node.
> +
> +            properties:
> +
> +              "#power-domain-cells":
> +                description:
> +                  Must be 0 for nodes representing a single PM domain and 1 for nodes
> +                  providing multiple PM domains.
> +
> +              "#address-cells":
> +                const: 1
> +
> +              "#size-cells":
> +                const: 0
> +
> +              reg:
> +                maxItems: 1
> +
> +              clocks:
> +                description: |
> +                  A number of phandles to clocks that need to be enabled while power domain
> +                  switches state.
> +
> +              pm_qos:
> +                description: |
> +                  A number of phandles to qos blocks which need to be saved and restored
> +                  while power domain switches state.
> +
> +            required:
> +              - reg
> +
> +            additionalProperties: false
> +
> +        required:
> +          - reg
> +
> +        additionalProperties: false
> +
> +    required:
> +      - reg
> +
> +    additionalProperties: false
> +
> +required:
> +  - compatible
> +  - "#power-domain-cells"
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/rk3399-cru.h>
> +    #include <dt-bindings/power/rk3399-power.h>
> +
> +    soc {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +
> +        qos_hdcp: qos@ffa90000 {
> +            compatible = "rockchip,rk3399-qos", "syscon";
> +            reg = <0x0 0xffa90000 0x0 0x20>;
> +        };
> +
> +        qos_iep: qos@ffa98000 {
> +            compatible = "rockchip,rk3399-qos", "syscon";
> +            reg = <0x0 0xffa98000 0x0 0x20>;
> +        };
> +
> +        qos_rga_r: qos@ffab0000 {
> +            compatible = "rockchip,rk3399-qos", "syscon";
> +            reg = <0x0 0xffab0000 0x0 0x20>;
> +        };
> +
> +        qos_rga_w: qos@ffab0080 {
> +            compatible = "rockchip,rk3399-qos", "syscon";
> +            reg = <0x0 0xffab0080 0x0 0x20>;
> +        };
> +
> +        qos_video_m0: qos@ffab8000 {
> +            compatible = "rockchip,rk3399-qos", "syscon";
> +            reg = <0x0 0xffab8000 0x0 0x20>;
> +        };
> +
> +        qos_video_m1_r: qos@ffac0000 {
> +            compatible = "rockchip,rk3399-qos", "syscon";
> +            reg = <0x0 0xffac0000 0x0 0x20>;
> +        };
> +
> +        qos_video_m1_w: qos@ffac0080 {
> +            compatible = "rockchip,rk3399-qos", "syscon";
> +            reg = <0x0 0xffac0080 0x0 0x20>;
> +        };
> +
> +        power-management@ff310000 {
> +            compatible = "rockchip,rk3399-pmu", "syscon", "simple-mfd";
> +            reg = <0x0 0xff310000 0x0 0x1000>;
> +
> +            power-controller {
> +                compatible = "rockchip,rk3399-power-controller";
> +                #power-domain-cells = <1>;
> +                #address-cells = <1>;
> +                #size-cells = <0>;
> +
> +                /* These power domains are grouped by VD_CENTER */
> +                pd_iep@RK3399_PD_IEP {
> +                    reg = <RK3399_PD_IEP>;
> +                    clocks = <&cru ACLK_IEP>,
> +                             <&cru HCLK_IEP>;
> +                    pm_qos = <&qos_iep>;
> +                    #power-domain-cells = <0>;
> +                };
> +                pd_rga@RK3399_PD_RGA {
> +                    reg = <RK3399_PD_RGA>;
> +                    clocks = <&cru ACLK_RGA>,
> +                             <&cru HCLK_RGA>;
> +                    pm_qos = <&qos_rga_r>,
> +                             <&qos_rga_w>;
> +                    #power-domain-cells = <0>;
> +                };
> +                pd_vcodec@RK3399_PD_VCODEC {
> +                    reg = <RK3399_PD_VCODEC>;
> +                    clocks = <&cru ACLK_VCODEC>,
> +                             <&cru HCLK_VCODEC>;
> +                    pm_qos = <&qos_video_m0>;
> +                    #power-domain-cells = <0>;
> +                };
> +                pd_vdu@RK3399_PD_VDU {
> +                    reg = <RK3399_PD_VDU>;
> +                    clocks = <&cru ACLK_VDU>,
> +                             <&cru HCLK_VDU>;
> +                    pm_qos = <&qos_video_m1_r>,
> +                             <&qos_video_m1_w>;
> +                    #power-domain-cells = <0>;
> +                };
> +                pd_vio@RK3399_PD_VIO {
> +                    reg = <RK3399_PD_VIO>;
> +                    #power-domain-cells = <1>;
> +                    #address-cells = <1>;
> +                    #size-cells = <0>;
> +
> +                    pd_hdcp@RK3399_PD_HDCP {
> +                        reg = <RK3399_PD_HDCP>;
> +                        clocks = <&cru ACLK_HDCP>,
> +                                 <&cru HCLK_HDCP>,
> +                                 <&cru PCLK_HDCP>;
> +                        pm_qos = <&qos_hdcp>;
> +                        #power-domain-cells = <0>;
> +                    };
> +                };
> +            };
> +        };
> +    };
> diff --git a/Documentation/devicetree/bindings/soc/rockchip/power_domain.txt b/Documentation/devicetree/bindings/soc/rockchip/power_domain.txt
> deleted file mode 100644
> index 8304eceb62e4..000000000000
> --- a/Documentation/devicetree/bindings/soc/rockchip/power_domain.txt
> +++ /dev/null
> @@ -1,136 +0,0 @@
> -* Rockchip Power Domains
> -
> -Rockchip processors include support for multiple power domains which can be
> -powered up/down by software based on different application scenes to save power.
> -
> -Required properties for power domain controller:
> -- compatible: Should be one of the following.
> -	"rockchip,px30-power-controller" - for PX30 SoCs.
> -	"rockchip,rk3036-power-controller" - for RK3036 SoCs.
> -	"rockchip,rk3066-power-controller" - for RK3066 SoCs.
> -	"rockchip,rk3128-power-controller" - for RK3128 SoCs.
> -	"rockchip,rk3188-power-controller" - for RK3188 SoCs.
> -	"rockchip,rk3228-power-controller" - for RK3228 SoCs.
> -	"rockchip,rk3288-power-controller" - for RK3288 SoCs.
> -	"rockchip,rk3328-power-controller" - for RK3328 SoCs.
> -	"rockchip,rk3366-power-controller" - for RK3366 SoCs.
> -	"rockchip,rk3368-power-controller" - for RK3368 SoCs.
> -	"rockchip,rk3399-power-controller" - for RK3399 SoCs.
> -- #power-domain-cells: Number of cells in a power-domain specifier.
> -	Should be 1 for multiple PM domains.
> -- #address-cells: Should be 1.
> -- #size-cells: Should be 0.
> -
> -Required properties for power domain sub nodes:
> -- reg: index of the power domain, should use macros in:
> -	"include/dt-bindings/power/px30-power.h" - for PX30 type power domain.
> -	"include/dt-bindings/power/rk3036-power.h" - for RK3036 type power domain.
> -	"include/dt-bindings/power/rk3066-power.h" - for RK3066 type power domain.
> -	"include/dt-bindings/power/rk3128-power.h" - for RK3128 type power domain.
> -	"include/dt-bindings/power/rk3188-power.h" - for RK3188 type power domain.
> -	"include/dt-bindings/power/rk3228-power.h" - for RK3228 type power domain.
> -	"include/dt-bindings/power/rk3288-power.h" - for RK3288 type power domain.
> -	"include/dt-bindings/power/rk3328-power.h" - for RK3328 type power domain.
> -	"include/dt-bindings/power/rk3366-power.h" - for RK3366 type power domain.
> -	"include/dt-bindings/power/rk3368-power.h" - for RK3368 type power domain.
> -	"include/dt-bindings/power/rk3399-power.h" - for RK3399 type power domain.
> -- clocks (optional): phandles to clocks which need to be enabled while power domain
> -	switches state.
> -- pm_qos (optional): phandles to qos blocks which need to be saved and restored
> -	while power domain switches state.
> -
> -Qos Example:
> -
> -	qos_gpu: qos_gpu@ffaf0000 {
> -		compatible ="syscon";
> -		reg = <0x0 0xffaf0000 0x0 0x20>;
> -	};
> -
> -Example:
> -
> -	power: power-controller {
> -		compatible = "rockchip,rk3288-power-controller";
> -		#power-domain-cells = <1>;
> -		#address-cells = <1>;
> -		#size-cells = <0>;
> -
> -		pd_gpu {
> -			reg = <RK3288_PD_GPU>;
> -			clocks = <&cru ACLK_GPU>;
> -			pm_qos = <&qos_gpu>;
> -		};
> -	};
> -
> -	 power: power-controller {
> -                compatible = "rockchip,rk3368-power-controller";
> -                #power-domain-cells = <1>;
> -                #address-cells = <1>;
> -                #size-cells = <0>;
> -
> -                pd_gpu_1 {
> -                        reg = <RK3368_PD_GPU_1>;
> -                        clocks = <&cru ACLK_GPU_CFG>;
> -                };
> -        };
> -
> -Example 2:
> -		power: power-controller {
> -			compatible = "rockchip,rk3399-power-controller";
> -			#power-domain-cells = <1>;
> -			#address-cells = <1>;
> -			#size-cells = <0>;
> -
> -			pd_vio {
> -				#address-cells = <1>;
> -				#size-cells = <0>;
> -				reg = <RK3399_PD_VIO>;
> -
> -				pd_vo {
> -					#address-cells = <1>;
> -					#size-cells = <0>;
> -					reg = <RK3399_PD_VO>;
> -
> -					pd_vopb {
> -						reg = <RK3399_PD_VOPB>;
> -					};
> -
> -					pd_vopl {
> -						reg = <RK3399_PD_VOPL>;
> -					};
> -				};
> -			};
> -		};
> -
> -Node of a device using power domains must have a power-domains property,
> -containing a phandle to the power device node and an index specifying which
> -power domain to use.
> -The index should use macros in:
> -	"include/dt-bindings/power/px30-power.h" - for px30 type power domain.
> -	"include/dt-bindings/power/rk3036-power.h" - for rk3036 type power domain.
> -	"include/dt-bindings/power/rk3128-power.h" - for rk3128 type power domain.
> -	"include/dt-bindings/power/rk3128-power.h" - for rk3228 type power domain.
> -	"include/dt-bindings/power/rk3288-power.h" - for rk3288 type power domain.
> -	"include/dt-bindings/power/rk3328-power.h" - for rk3328 type power domain.
> -	"include/dt-bindings/power/rk3366-power.h" - for rk3366 type power domain.
> -	"include/dt-bindings/power/rk3368-power.h" - for rk3368 type power domain.
> -	"include/dt-bindings/power/rk3399-power.h" - for rk3399 type power domain.
> -
> -Example of the node using power domain:
> -
> -	node {
> -		/* ... */
> -		power-domains = <&power RK3288_PD_GPU>;
> -		/* ... */
> -	};
> -
> -	node {
> -                /* ... */
> -                power-domains = <&power RK3368_PD_GPU_1>;
> -                /* ... */
> -        };
> -
> -	node {
> -		/* ... */
> -		power-domains = <&power RK3399_PD_VOPB>;
> -		/* ... */
> -	};
>
Enric Balletbo i Serra March 24, 2021, 9:56 a.m. UTC | #2
Hi Elaine,

This is not the exact version I sent, and you reintroduced a "problem" that were
already solved/discussed on previous versions. See below:

On 24/3/21 8:16, Elaine Zhang wrote:
> Convert the soc/rockchip/power_domain.txt binding document to
> json-schema and move to the power bindings directory.
> 
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>

If you do significant is a good practice shortly describe them within [] here.

> Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>

Note that my last version already had the

Reviewed-by: Rob Herring <robh@kernel.org>

Which should be fine for merging (with probably only minor changes) and you
could maintain if you don't do significant changes, but that's not the case, as
I said, you are reintroducing one problem. Please review the comments already
received on this patchset or similar patchsets to avoid this.

> ---
>  .../power/rockchip,power-controller.yaml      | 284 ++++++++++++++++++
>  .../bindings/soc/rockchip/power_domain.txt    | 136 ---------
>  2 files changed, 284 insertions(+), 136 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
>  delete mode 100644 Documentation/devicetree/bindings/soc/rockchip/power_domain.txt
> 
> diff --git a/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml b/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
> new file mode 100644
> index 000000000000..a220322c5139
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
> @@ -0,0 +1,284 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/power/rockchip,power-controller.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Rockchip Power Domains
> +
> +maintainers:
> +  - Elaine Zhang <zhangqing@rock-chips.com>
> +  - Rob Herring <robh+dt@kernel.org>

Up to Rob, but I don't think Rob would like to be the maintainer. I think you
can only include yourself and Heiko.


> +  - Heiko Stuebner <heiko@sntech.de>
> +
> +description: |
> +  Rockchip processors include support for multiple power domains which can be
> +  powered up/down by software based on different application scenarios to save power.
> +
> +  Power domains contained within power-controller node are generic power domain
> +  providers documented in Documentation/devicetree/bindings/power/power-domain.yaml.
> +
> +  IP cores belonging to a power domain should contain a "power-domains"
> +  property that is a phandle for the power domain node representing the domain.
> +
> +properties:
> +  $nodename:
> +    const: power-controller
> +
> +  compatible:
> +    enum:
> +      - rockchip,px30-power-controller
> +      - rockchip,rk3036-power-controller
> +      - rockchip,rk3066-power-controller
> +      - rockchip,rk3128-power-controller
> +      - rockchip,rk3188-power-controller
> +      - rockchip,rk3228-power-controller
> +      - rockchip,rk3288-power-controller
> +      - rockchip,rk3328-power-controller
> +      - rockchip,rk3366-power-controller
> +      - rockchip,rk3368-power-controller
> +      - rockchip,rk3399-power-controller
> +
> +  "#power-domain-cells":
> +    const: 1
> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 0
> +
> +patternProperties:
> +  "^pd_[0-9a-z_]{2,10}@[0-9a-f]+$":
> +    type: object
> +    description: |
> +      Represents the power domains within the power controller node as documented
> +      in Documentation/devicetree/bindings/power/power-domain.yaml.
> +

The node names must be generic, as this is power-domain must be in the form:

+patternProperties:
+  "^power-domain@[0-9a-f]+$":


> +    properties:
> +
> +      "#power-domain-cells":
> +        description:
> +          Must be 0 for nodes representing a single PM domain and 1 for nodes
> +          providing multiple PM domains.
> +
> +      "#address-cells":
> +        const: 1
> +
> +      "#size-cells":
> +        const: 0
> +
> +      reg:
> +        maxItems: 1
> +        description: |
> +          Power domain index. Valid values are defined in
> +          "include/dt-bindings/power/px30-power.h"
> +          "include/dt-bindings/power/rk3036-power.h"
> +          "include/dt-bindings/power/rk3066-power.h"
> +          "include/dt-bindings/power/rk3128-power.h"
> +          "include/dt-bindings/power/rk3188-power.h"
> +          "include/dt-bindings/power/rk3228-power.h"
> +          "include/dt-bindings/power/rk3288-power.h"
> +          "include/dt-bindings/power/rk3328-power.h"
> +          "include/dt-bindings/power/rk3366-power.h"
> +          "include/dt-bindings/power/rk3368-power.h"
> +          "include/dt-bindings/power/rk3399-power.h"
> +
> +      clocks:
> +        description: |
> +          A number of phandles to clocks that need to be enabled while power domain
> +          switches state.
> +
> +      pm_qos:
> +        description: |
> +          A number of phandles to qos blocks which need to be saved and restored
> +          while power domain switches state.
> +
> +    patternProperties:
> +      "^pd_[0-9a-z_]{2,10}@[0-9a-f]+$":

The same here:

+patternProperties:
+  "^power-domain@[0-9a-f]+$":



> +        type: object
> +        description: |
> +          Represents a power domain child within a power domain parent node.
> +
> +        properties:
> +
> +          "#power-domain-cells":
> +            description:
> +              Must be 0 for nodes representing a single PM domain and 1 for nodes
> +              providing multiple PM domains.
> +
> +          "#address-cells":
> +            const: 1
> +
> +          "#size-cells":
> +            const: 0
> +
> +          reg:
> +            maxItems: 1
> +
> +          clocks:
> +            description: |
> +              A number of phandles to clocks that need to be enabled while power domain
> +              switches state.
> +
> +          pm_qos:
> +            description: |
> +              A number of phandles to qos blocks which need to be saved and restored
> +              while power domain switches state.
> +
> +        patternProperties:
> +          "^pd_[0-9a-z_]{2,10}@[0-9a-f]+$":

And here:

> +            type: object
> +            description: |
> +              Represents a power domain child within a power domain parent node.
> +
> +            properties:
> +
> +              "#power-domain-cells":
> +                description:
> +                  Must be 0 for nodes representing a single PM domain and 1 for nodes
> +                  providing multiple PM domains.
> +
> +              "#address-cells":
> +                const: 1
> +
> +              "#size-cells":
> +                const: 0
> +
> +              reg:
> +                maxItems: 1
> +
> +              clocks:
> +                description: |
> +                  A number of phandles to clocks that need to be enabled while power domain
> +                  switches state.
> +
> +              pm_qos:
> +                description: |
> +                  A number of phandles to qos blocks which need to be saved and restored
> +                  while power domain switches state.
> +
> +            required:
> +              - reg
> +
> +            additionalProperties: false
> +
> +        required:
> +          - reg
> +
> +        additionalProperties: false
> +
> +    required:
> +      - reg
> +
> +    additionalProperties: false
> +
> +required:
> +  - compatible
> +  - "#power-domain-cells"
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/rk3399-cru.h>
> +    #include <dt-bindings/power/rk3399-power.h>
> +
> +    soc {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +
> +        qos_hdcp: qos@ffa90000 {
> +            compatible = "rockchip,rk3399-qos", "syscon";
> +            reg = <0x0 0xffa90000 0x0 0x20>;
> +        };
> +
> +        qos_iep: qos@ffa98000 {
> +            compatible = "rockchip,rk3399-qos", "syscon";
> +            reg = <0x0 0xffa98000 0x0 0x20>;
> +        };
> +
> +        qos_rga_r: qos@ffab0000 {
> +            compatible = "rockchip,rk3399-qos", "syscon";
> +            reg = <0x0 0xffab0000 0x0 0x20>;
> +        };
> +
> +        qos_rga_w: qos@ffab0080 {
> +            compatible = "rockchip,rk3399-qos", "syscon";
> +            reg = <0x0 0xffab0080 0x0 0x20>;
> +        };
> +
> +        qos_video_m0: qos@ffab8000 {
> +            compatible = "rockchip,rk3399-qos", "syscon";
> +            reg = <0x0 0xffab8000 0x0 0x20>;
> +        };
> +
> +        qos_video_m1_r: qos@ffac0000 {
> +            compatible = "rockchip,rk3399-qos", "syscon";
> +            reg = <0x0 0xffac0000 0x0 0x20>;
> +        };
> +
> +        qos_video_m1_w: qos@ffac0080 {
> +            compatible = "rockchip,rk3399-qos", "syscon";
> +            reg = <0x0 0xffac0080 0x0 0x20>;
> +        };
> +
> +        power-management@ff310000 {
> +            compatible = "rockchip,rk3399-pmu", "syscon", "simple-mfd";
> +            reg = <0x0 0xff310000 0x0 0x1000>;
> +
> +            power-controller {
> +                compatible = "rockchip,rk3399-power-controller";
> +                #power-domain-cells = <1>;
> +                #address-cells = <1>;
> +                #size-cells = <0>;
> +
> +                /* These power domains are grouped by VD_CENTER */
> +                pd_iep@RK3399_PD_IEP {

Here and below must be generic power-domain@RK3399_PD_IEP


> +                    reg = <RK3399_PD_IEP>;
> +                    clocks = <&cru ACLK_IEP>,
> +                             <&cru HCLK_IEP>;
> +                    pm_qos = <&qos_iep>;
> +                    #power-domain-cells = <0>;
> +                };
> +                pd_rga@RK3399_PD_RGA {
> +                    reg = <RK3399_PD_RGA>;
> +                    clocks = <&cru ACLK_RGA>,
> +                             <&cru HCLK_RGA>;
> +                    pm_qos = <&qos_rga_r>,
> +                             <&qos_rga_w>;
> +                    #power-domain-cells = <0>;
> +                };
> +                pd_vcodec@RK3399_PD_VCODEC {
> +                    reg = <RK3399_PD_VCODEC>;
> +                    clocks = <&cru ACLK_VCODEC>,
> +                             <&cru HCLK_VCODEC>;
> +                    pm_qos = <&qos_video_m0>;
> +                    #power-domain-cells = <0>;
> +                };
> +                pd_vdu@RK3399_PD_VDU {
> +                    reg = <RK3399_PD_VDU>;
> +                    clocks = <&cru ACLK_VDU>,
> +                             <&cru HCLK_VDU>;
> +                    pm_qos = <&qos_video_m1_r>,
> +                             <&qos_video_m1_w>;
> +                    #power-domain-cells = <0>;
> +                };
> +                pd_vio@RK3399_PD_VIO {
> +                    reg = <RK3399_PD_VIO>;
> +                    #power-domain-cells = <1>;
> +                    #address-cells = <1>;
> +                    #size-cells = <0>;
> +
> +                    pd_hdcp@RK3399_PD_HDCP {
> +                        reg = <RK3399_PD_HDCP>;
> +                        clocks = <&cru ACLK_HDCP>,
> +                                 <&cru HCLK_HDCP>,
> +                                 <&cru PCLK_HDCP>;
> +                        pm_qos = <&qos_hdcp>;
> +                        #power-domain-cells = <0>;
> +                    };
> +                };
> +            };
> +        };
> +    };
> diff --git a/Documentation/devicetree/bindings/soc/rockchip/power_domain.txt b/Documentation/devicetree/bindings/soc/rockchip/power_domain.txt
> deleted file mode 100644
> index 8304eceb62e4..000000000000
> --- a/Documentation/devicetree/bindings/soc/rockchip/power_domain.txt
> +++ /dev/null
> @@ -1,136 +0,0 @@
> -* Rockchip Power Domains
> -
> -Rockchip processors include support for multiple power domains which can be
> -powered up/down by software based on different application scenes to save power.
> -
> -Required properties for power domain controller:
> -- compatible: Should be one of the following.
> -	"rockchip,px30-power-controller" - for PX30 SoCs.
> -	"rockchip,rk3036-power-controller" - for RK3036 SoCs.
> -	"rockchip,rk3066-power-controller" - for RK3066 SoCs.
> -	"rockchip,rk3128-power-controller" - for RK3128 SoCs.
> -	"rockchip,rk3188-power-controller" - for RK3188 SoCs.
> -	"rockchip,rk3228-power-controller" - for RK3228 SoCs.
> -	"rockchip,rk3288-power-controller" - for RK3288 SoCs.
> -	"rockchip,rk3328-power-controller" - for RK3328 SoCs.
> -	"rockchip,rk3366-power-controller" - for RK3366 SoCs.
> -	"rockchip,rk3368-power-controller" - for RK3368 SoCs.
> -	"rockchip,rk3399-power-controller" - for RK3399 SoCs.
> -- #power-domain-cells: Number of cells in a power-domain specifier.
> -	Should be 1 for multiple PM domains.
> -- #address-cells: Should be 1.
> -- #size-cells: Should be 0.
> -
> -Required properties for power domain sub nodes:
> -- reg: index of the power domain, should use macros in:
> -	"include/dt-bindings/power/px30-power.h" - for PX30 type power domain.
> -	"include/dt-bindings/power/rk3036-power.h" - for RK3036 type power domain.
> -	"include/dt-bindings/power/rk3066-power.h" - for RK3066 type power domain.
> -	"include/dt-bindings/power/rk3128-power.h" - for RK3128 type power domain.
> -	"include/dt-bindings/power/rk3188-power.h" - for RK3188 type power domain.
> -	"include/dt-bindings/power/rk3228-power.h" - for RK3228 type power domain.
> -	"include/dt-bindings/power/rk3288-power.h" - for RK3288 type power domain.
> -	"include/dt-bindings/power/rk3328-power.h" - for RK3328 type power domain.
> -	"include/dt-bindings/power/rk3366-power.h" - for RK3366 type power domain.
> -	"include/dt-bindings/power/rk3368-power.h" - for RK3368 type power domain.
> -	"include/dt-bindings/power/rk3399-power.h" - for RK3399 type power domain.
> -- clocks (optional): phandles to clocks which need to be enabled while power domain
> -	switches state.
> -- pm_qos (optional): phandles to qos blocks which need to be saved and restored
> -	while power domain switches state.
> -
> -Qos Example:
> -
> -	qos_gpu: qos_gpu@ffaf0000 {
> -		compatible ="syscon";
> -		reg = <0x0 0xffaf0000 0x0 0x20>;
> -	};
> -
> -Example:
> -
> -	power: power-controller {
> -		compatible = "rockchip,rk3288-power-controller";
> -		#power-domain-cells = <1>;
> -		#address-cells = <1>;
> -		#size-cells = <0>;
> -
> -		pd_gpu {
> -			reg = <RK3288_PD_GPU>;
> -			clocks = <&cru ACLK_GPU>;
> -			pm_qos = <&qos_gpu>;
> -		};
> -	};
> -
> -	 power: power-controller {
> -                compatible = "rockchip,rk3368-power-controller";
> -                #power-domain-cells = <1>;
> -                #address-cells = <1>;
> -                #size-cells = <0>;
> -
> -                pd_gpu_1 {
> -                        reg = <RK3368_PD_GPU_1>;
> -                        clocks = <&cru ACLK_GPU_CFG>;
> -                };
> -        };
> -
> -Example 2:
> -		power: power-controller {
> -			compatible = "rockchip,rk3399-power-controller";
> -			#power-domain-cells = <1>;
> -			#address-cells = <1>;
> -			#size-cells = <0>;
> -
> -			pd_vio {
> -				#address-cells = <1>;
> -				#size-cells = <0>;
> -				reg = <RK3399_PD_VIO>;
> -
> -				pd_vo {
> -					#address-cells = <1>;
> -					#size-cells = <0>;
> -					reg = <RK3399_PD_VO>;
> -
> -					pd_vopb {
> -						reg = <RK3399_PD_VOPB>;
> -					};
> -
> -					pd_vopl {
> -						reg = <RK3399_PD_VOPL>;
> -					};
> -				};
> -			};
> -		};
> -
> -Node of a device using power domains must have a power-domains property,
> -containing a phandle to the power device node and an index specifying which
> -power domain to use.
> -The index should use macros in:
> -	"include/dt-bindings/power/px30-power.h" - for px30 type power domain.
> -	"include/dt-bindings/power/rk3036-power.h" - for rk3036 type power domain.
> -	"include/dt-bindings/power/rk3128-power.h" - for rk3128 type power domain.
> -	"include/dt-bindings/power/rk3128-power.h" - for rk3228 type power domain.
> -	"include/dt-bindings/power/rk3288-power.h" - for rk3288 type power domain.
> -	"include/dt-bindings/power/rk3328-power.h" - for rk3328 type power domain.
> -	"include/dt-bindings/power/rk3366-power.h" - for rk3366 type power domain.
> -	"include/dt-bindings/power/rk3368-power.h" - for rk3368 type power domain.
> -	"include/dt-bindings/power/rk3399-power.h" - for rk3399 type power domain.
> -
> -Example of the node using power domain:
> -
> -	node {
> -		/* ... */
> -		power-domains = <&power RK3288_PD_GPU>;
> -		/* ... */
> -	};
> -
> -	node {
> -                /* ... */
> -                power-domains = <&power RK3368_PD_GPU_1>;
> -                /* ... */
> -        };
> -
> -	node {
> -		/* ... */
> -		power-domains = <&power RK3399_PD_VOPB>;
> -		/* ... */
> -	};
>
Elaine Zhang March 24, 2021, 10:14 a.m. UTC | #3
Hi, Johan:

在 2021/3/24 下午5:17, Johan Jonker 写道:
> Hi Elaine,
>
> >From Rob's build log it turns out that 2 more properties must be added.
> Add these new properties in separate patch.
> Retest with commands below.
> ====
> See rk3288.dtsi
>
> 			assigned-clocks = <&cru SCLK_EDP_24M>;
> 			assigned-clock-parents = <&xin24m>;
This should not be in the power node.
It should be on the CRU node, or on the EDP's own node.
I could have added it just to solve dtbs_check .
> ====
> https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20210324071609.7531-3-zhangqing@rock-chips.com/
>
> make ARCH=arm dtbs_check
> DT_SCHEMA_FILES=Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
>
> make ARCH=arm64 dtbs_check
> DT_SCHEMA_FILES=Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
>
> On 3/24/21 8:16 AM, Elaine Zhang wrote:
>> Convert the soc/rockchip/power_domain.txt binding document to
>> json-schema and move to the power bindings directory.
>>
>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>> Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
>> ---
>>   .../power/rockchip,power-controller.yaml      | 284 ++++++++++++++++++
>>   .../bindings/soc/rockchip/power_domain.txt    | 136 ---------
>>   2 files changed, 284 insertions(+), 136 deletions(-)
>>   create mode 100644 Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
>>   delete mode 100644 Documentation/devicetree/bindings/soc/rockchip/power_domain.txt
>>
>> diff --git a/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml b/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
>> new file mode 100644
>> index 000000000000..a220322c5139
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
>> @@ -0,0 +1,284 @@
>> +# SPDX-License-Identifier: GPL-2.0-only
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/power/rockchip,power-controller.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Rockchip Power Domains
>> +
>> +maintainers:
>> +  - Elaine Zhang <zhangqing@rock-chips.com>
>> +  - Rob Herring <robh+dt@kernel.org>
>> +  - Heiko Stuebner <heiko@sntech.de>
>> +
>> +description: |
>> +  Rockchip processors include support for multiple power domains which can be
>> +  powered up/down by software based on different application scenarios to save power.
>> +
>> +  Power domains contained within power-controller node are generic power domain
>> +  providers documented in Documentation/devicetree/bindings/power/power-domain.yaml.
>> +
>> +  IP cores belonging to a power domain should contain a "power-domains"
>> +  property that is a phandle for the power domain node representing the domain.
>> +
>> +properties:
>> +  $nodename:
>> +    const: power-controller
>> +
>> +  compatible:
>> +    enum:
>> +      - rockchip,px30-power-controller
>> +      - rockchip,rk3036-power-controller
>> +      - rockchip,rk3066-power-controller
>> +      - rockchip,rk3128-power-controller
>> +      - rockchip,rk3188-power-controller
>> +      - rockchip,rk3228-power-controller
>> +      - rockchip,rk3288-power-controller
>> +      - rockchip,rk3328-power-controller
>> +      - rockchip,rk3366-power-controller
>> +      - rockchip,rk3368-power-controller
>> +      - rockchip,rk3399-power-controller
>> +
>> +  "#power-domain-cells":
>> +    const: 1
>> +
>> +  "#address-cells":
>> +    const: 1
>> +
>> +  "#size-cells":
>> +    const: 0
>
> assigned-clocks:
>    maxItems: 1
>
> assigned-clock-parents:
>    maxItems: 1
>
>> +
>> +patternProperties:
>> +  "^pd_[0-9a-z_]{2,10}@[0-9a-f]+$":
>> +    type: object
>> +    description: |
>> +      Represents the power domains within the power controller node as documented
>> +      in Documentation/devicetree/bindings/power/power-domain.yaml.
>> +
>> +    properties:
>> +
>> +      "#power-domain-cells":
>> +        description:
>> +          Must be 0 for nodes representing a single PM domain and 1 for nodes
>> +          providing multiple PM domains.
>> +
>> +      "#address-cells":
>> +        const: 1
>> +
>> +      "#size-cells":
>> +        const: 0
>> +
>> +      reg:
>> +        maxItems: 1
>> +        description: |
>> +          Power domain index. Valid values are defined in
>> +          "include/dt-bindings/power/px30-power.h"
>> +          "include/dt-bindings/power/rk3036-power.h"
>> +          "include/dt-bindings/power/rk3066-power.h"
>> +          "include/dt-bindings/power/rk3128-power.h"
>> +          "include/dt-bindings/power/rk3188-power.h"
>> +          "include/dt-bindings/power/rk3228-power.h"
>> +          "include/dt-bindings/power/rk3288-power.h"
>> +          "include/dt-bindings/power/rk3328-power.h"
>> +          "include/dt-bindings/power/rk3366-power.h"
>> +          "include/dt-bindings/power/rk3368-power.h"
>> +          "include/dt-bindings/power/rk3399-power.h"
>> +
>> +      clocks:
>> +        description: |
>> +          A number of phandles to clocks that need to be enabled while power domain
>> +          switches state.
>> +
>> +      pm_qos:
>> +        description: |
>> +          A number of phandles to qos blocks which need to be saved and restored
>> +          while power domain switches state.
>> +
>> +    patternProperties:
>> +      "^pd_[0-9a-z_]{2,10}@[0-9a-f]+$":
>> +        type: object
>> +        description: |
>> +          Represents a power domain child within a power domain parent node.
>> +
>> +        properties:
>> +
>> +          "#power-domain-cells":
>> +            description:
>> +              Must be 0 for nodes representing a single PM domain and 1 for nodes
>> +              providing multiple PM domains.
>> +
>> +          "#address-cells":
>> +            const: 1
>> +
>> +          "#size-cells":
>> +            const: 0
>> +
>> +          reg:
>> +            maxItems: 1
>> +
>> +          clocks:
>> +            description: |
>> +              A number of phandles to clocks that need to be enabled while power domain
>> +              switches state.
>> +
>> +          pm_qos:
>> +            description: |
>> +              A number of phandles to qos blocks which need to be saved and restored
>> +              while power domain switches state.
>> +
>> +        patternProperties:
>> +          "^pd_[0-9a-z_]{2,10}@[0-9a-f]+$":
>> +            type: object
>> +            description: |
>> +              Represents a power domain child within a power domain parent node.
>> +
>> +            properties:
>> +
>> +              "#power-domain-cells":
>> +                description:
>> +                  Must be 0 for nodes representing a single PM domain and 1 for nodes
>> +                  providing multiple PM domains.
>> +
>> +              "#address-cells":
>> +                const: 1
>> +
>> +              "#size-cells":
>> +                const: 0
>> +
>> +              reg:
>> +                maxItems: 1
>> +
>> +              clocks:
>> +                description: |
>> +                  A number of phandles to clocks that need to be enabled while power domain
>> +                  switches state.
>> +
>> +              pm_qos:
>> +                description: |
>> +                  A number of phandles to qos blocks which need to be saved and restored
>> +                  while power domain switches state.
>> +
>> +            required:
>> +              - reg
>> +
>> +            additionalProperties: false
>> +
>> +        required:
>> +          - reg
>> +
>> +        additionalProperties: false
>> +
>> +    required:
>> +      - reg
>> +
>> +    additionalProperties: false
>> +
>> +required:
>> +  - compatible
>> +  - "#power-domain-cells"
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/clock/rk3399-cru.h>
>> +    #include <dt-bindings/power/rk3399-power.h>
>> +
>> +    soc {
>> +        #address-cells = <2>;
>> +        #size-cells = <2>;
>> +
>> +        qos_hdcp: qos@ffa90000 {
>> +            compatible = "rockchip,rk3399-qos", "syscon";
>> +            reg = <0x0 0xffa90000 0x0 0x20>;
>> +        };
>> +
>> +        qos_iep: qos@ffa98000 {
>> +            compatible = "rockchip,rk3399-qos", "syscon";
>> +            reg = <0x0 0xffa98000 0x0 0x20>;
>> +        };
>> +
>> +        qos_rga_r: qos@ffab0000 {
>> +            compatible = "rockchip,rk3399-qos", "syscon";
>> +            reg = <0x0 0xffab0000 0x0 0x20>;
>> +        };
>> +
>> +        qos_rga_w: qos@ffab0080 {
>> +            compatible = "rockchip,rk3399-qos", "syscon";
>> +            reg = <0x0 0xffab0080 0x0 0x20>;
>> +        };
>> +
>> +        qos_video_m0: qos@ffab8000 {
>> +            compatible = "rockchip,rk3399-qos", "syscon";
>> +            reg = <0x0 0xffab8000 0x0 0x20>;
>> +        };
>> +
>> +        qos_video_m1_r: qos@ffac0000 {
>> +            compatible = "rockchip,rk3399-qos", "syscon";
>> +            reg = <0x0 0xffac0000 0x0 0x20>;
>> +        };
>> +
>> +        qos_video_m1_w: qos@ffac0080 {
>> +            compatible = "rockchip,rk3399-qos", "syscon";
>> +            reg = <0x0 0xffac0080 0x0 0x20>;
>> +        };
>> +
>> +        power-management@ff310000 {
>> +            compatible = "rockchip,rk3399-pmu", "syscon", "simple-mfd";
>> +            reg = <0x0 0xff310000 0x0 0x1000>;
>> +
>> +            power-controller {
>> +                compatible = "rockchip,rk3399-power-controller";
>> +                #power-domain-cells = <1>;
>> +                #address-cells = <1>;
>> +                #size-cells = <0>;
>> +
>> +                /* These power domains are grouped by VD_CENTER */
>> +                pd_iep@RK3399_PD_IEP {
>> +                    reg = <RK3399_PD_IEP>;
>> +                    clocks = <&cru ACLK_IEP>,
>> +                             <&cru HCLK_IEP>;
>> +                    pm_qos = <&qos_iep>;
>> +                    #power-domain-cells = <0>;
>> +                };
>> +                pd_rga@RK3399_PD_RGA {
>> +                    reg = <RK3399_PD_RGA>;
>> +                    clocks = <&cru ACLK_RGA>,
>> +                             <&cru HCLK_RGA>;
>> +                    pm_qos = <&qos_rga_r>,
>> +                             <&qos_rga_w>;
>> +                    #power-domain-cells = <0>;
>> +                };
>> +                pd_vcodec@RK3399_PD_VCODEC {
>> +                    reg = <RK3399_PD_VCODEC>;
>> +                    clocks = <&cru ACLK_VCODEC>,
>> +                             <&cru HCLK_VCODEC>;
>> +                    pm_qos = <&qos_video_m0>;
>> +                    #power-domain-cells = <0>;
>> +                };
>> +                pd_vdu@RK3399_PD_VDU {
>> +                    reg = <RK3399_PD_VDU>;
>> +                    clocks = <&cru ACLK_VDU>,
>> +                             <&cru HCLK_VDU>;
>> +                    pm_qos = <&qos_video_m1_r>,
>> +                             <&qos_video_m1_w>;
>> +                    #power-domain-cells = <0>;
>> +                };
>> +                pd_vio@RK3399_PD_VIO {
>> +                    reg = <RK3399_PD_VIO>;
>> +                    #power-domain-cells = <1>;
>> +                    #address-cells = <1>;
>> +                    #size-cells = <0>;
>> +
>> +                    pd_hdcp@RK3399_PD_HDCP {
>> +                        reg = <RK3399_PD_HDCP>;
>> +                        clocks = <&cru ACLK_HDCP>,
>> +                                 <&cru HCLK_HDCP>,
>> +                                 <&cru PCLK_HDCP>;
>> +                        pm_qos = <&qos_hdcp>;
>> +                        #power-domain-cells = <0>;
>> +                    };
>> +                };
>> +            };
>> +        };
>> +    };
>> diff --git a/Documentation/devicetree/bindings/soc/rockchip/power_domain.txt b/Documentation/devicetree/bindings/soc/rockchip/power_domain.txt
>> deleted file mode 100644
>> index 8304eceb62e4..000000000000
>> --- a/Documentation/devicetree/bindings/soc/rockchip/power_domain.txt
>> +++ /dev/null
>> @@ -1,136 +0,0 @@
>> -* Rockchip Power Domains
>> -
>> -Rockchip processors include support for multiple power domains which can be
>> -powered up/down by software based on different application scenes to save power.
>> -
>> -Required properties for power domain controller:
>> -- compatible: Should be one of the following.
>> -	"rockchip,px30-power-controller" - for PX30 SoCs.
>> -	"rockchip,rk3036-power-controller" - for RK3036 SoCs.
>> -	"rockchip,rk3066-power-controller" - for RK3066 SoCs.
>> -	"rockchip,rk3128-power-controller" - for RK3128 SoCs.
>> -	"rockchip,rk3188-power-controller" - for RK3188 SoCs.
>> -	"rockchip,rk3228-power-controller" - for RK3228 SoCs.
>> -	"rockchip,rk3288-power-controller" - for RK3288 SoCs.
>> -	"rockchip,rk3328-power-controller" - for RK3328 SoCs.
>> -	"rockchip,rk3366-power-controller" - for RK3366 SoCs.
>> -	"rockchip,rk3368-power-controller" - for RK3368 SoCs.
>> -	"rockchip,rk3399-power-controller" - for RK3399 SoCs.
>> -- #power-domain-cells: Number of cells in a power-domain specifier.
>> -	Should be 1 for multiple PM domains.
>> -- #address-cells: Should be 1.
>> -- #size-cells: Should be 0.
>> -
>> -Required properties for power domain sub nodes:
>> -- reg: index of the power domain, should use macros in:
>> -	"include/dt-bindings/power/px30-power.h" - for PX30 type power domain.
>> -	"include/dt-bindings/power/rk3036-power.h" - for RK3036 type power domain.
>> -	"include/dt-bindings/power/rk3066-power.h" - for RK3066 type power domain.
>> -	"include/dt-bindings/power/rk3128-power.h" - for RK3128 type power domain.
>> -	"include/dt-bindings/power/rk3188-power.h" - for RK3188 type power domain.
>> -	"include/dt-bindings/power/rk3228-power.h" - for RK3228 type power domain.
>> -	"include/dt-bindings/power/rk3288-power.h" - for RK3288 type power domain.
>> -	"include/dt-bindings/power/rk3328-power.h" - for RK3328 type power domain.
>> -	"include/dt-bindings/power/rk3366-power.h" - for RK3366 type power domain.
>> -	"include/dt-bindings/power/rk3368-power.h" - for RK3368 type power domain.
>> -	"include/dt-bindings/power/rk3399-power.h" - for RK3399 type power domain.
>> -- clocks (optional): phandles to clocks which need to be enabled while power domain
>> -	switches state.
>> -- pm_qos (optional): phandles to qos blocks which need to be saved and restored
>> -	while power domain switches state.
>> -
>> -Qos Example:
>> -
>> -	qos_gpu: qos_gpu@ffaf0000 {
>> -		compatible ="syscon";
>> -		reg = <0x0 0xffaf0000 0x0 0x20>;
>> -	};
>> -
>> -Example:
>> -
>> -	power: power-controller {
>> -		compatible = "rockchip,rk3288-power-controller";
>> -		#power-domain-cells = <1>;
>> -		#address-cells = <1>;
>> -		#size-cells = <0>;
>> -
>> -		pd_gpu {
>> -			reg = <RK3288_PD_GPU>;
>> -			clocks = <&cru ACLK_GPU>;
>> -			pm_qos = <&qos_gpu>;
>> -		};
>> -	};
>> -
>> -	 power: power-controller {
>> -                compatible = "rockchip,rk3368-power-controller";
>> -                #power-domain-cells = <1>;
>> -                #address-cells = <1>;
>> -                #size-cells = <0>;
>> -
>> -                pd_gpu_1 {
>> -                        reg = <RK3368_PD_GPU_1>;
>> -                        clocks = <&cru ACLK_GPU_CFG>;
>> -                };
>> -        };
>> -
>> -Example 2:
>> -		power: power-controller {
>> -			compatible = "rockchip,rk3399-power-controller";
>> -			#power-domain-cells = <1>;
>> -			#address-cells = <1>;
>> -			#size-cells = <0>;
>> -
>> -			pd_vio {
>> -				#address-cells = <1>;
>> -				#size-cells = <0>;
>> -				reg = <RK3399_PD_VIO>;
>> -
>> -				pd_vo {
>> -					#address-cells = <1>;
>> -					#size-cells = <0>;
>> -					reg = <RK3399_PD_VO>;
>> -
>> -					pd_vopb {
>> -						reg = <RK3399_PD_VOPB>;
>> -					};
>> -
>> -					pd_vopl {
>> -						reg = <RK3399_PD_VOPL>;
>> -					};
>> -				};
>> -			};
>> -		};
>> -
>> -Node of a device using power domains must have a power-domains property,
>> -containing a phandle to the power device node and an index specifying which
>> -power domain to use.
>> -The index should use macros in:
>> -	"include/dt-bindings/power/px30-power.h" - for px30 type power domain.
>> -	"include/dt-bindings/power/rk3036-power.h" - for rk3036 type power domain.
>> -	"include/dt-bindings/power/rk3128-power.h" - for rk3128 type power domain.
>> -	"include/dt-bindings/power/rk3128-power.h" - for rk3228 type power domain.
>> -	"include/dt-bindings/power/rk3288-power.h" - for rk3288 type power domain.
>> -	"include/dt-bindings/power/rk3328-power.h" - for rk3328 type power domain.
>> -	"include/dt-bindings/power/rk3366-power.h" - for rk3366 type power domain.
>> -	"include/dt-bindings/power/rk3368-power.h" - for rk3368 type power domain.
>> -	"include/dt-bindings/power/rk3399-power.h" - for rk3399 type power domain.
>> -
>> -Example of the node using power domain:
>> -
>> -	node {
>> -		/* ... */
>> -		power-domains = <&power RK3288_PD_GPU>;
>> -		/* ... */
>> -	};
>> -
>> -	node {
>> -                /* ... */
>> -                power-domains = <&power RK3368_PD_GPU_1>;
>> -                /* ... */
>> -        };
>> -
>> -	node {
>> -		/* ... */
>> -		power-domains = <&power RK3399_PD_VOPB>;
>> -		/* ... */
>> -	};
>>
>
>
Elaine Zhang March 24, 2021, 10:18 a.m. UTC | #4
Hi,  Enric

在 2021/3/24 下午5:56, Enric Balletbo i Serra 写道:
> Hi Elaine,
>
> This is not the exact version I sent, and you reintroduced a "problem" that were
> already solved/discussed on previous versions. See below:
>
> On 24/3/21 8:16, Elaine Zhang wrote:
>> Convert the soc/rockchip/power_domain.txt binding document to
>> json-schema and move to the power bindings directory.
>>
>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> If you do significant is a good practice shortly describe them within [] here.
>
>> Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
> Note that my last version already had the
>
> Reviewed-by: Rob Herring <robh@kernel.org>
>
> Which should be fine for merging (with probably only minor changes) and you
> could maintain if you don't do significant changes, but that's not the case, as
> I said, you are reintroducing one problem. Please review the comments already
> received on this patchset or similar patchsets to avoid this.
>
>> ---
>>   .../power/rockchip,power-controller.yaml      | 284 ++++++++++++++++++
>>   .../bindings/soc/rockchip/power_domain.txt    | 136 ---------
>>   2 files changed, 284 insertions(+), 136 deletions(-)
>>   create mode 100644 Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
>>   delete mode 100644 Documentation/devicetree/bindings/soc/rockchip/power_domain.txt
>>
>> diff --git a/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml b/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
>> new file mode 100644
>> index 000000000000..a220322c5139
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
>> @@ -0,0 +1,284 @@
>> +# SPDX-License-Identifier: GPL-2.0-only
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/power/rockchip,power-controller.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Rockchip Power Domains
>> +
>> +maintainers:
>> +  - Elaine Zhang <zhangqing@rock-chips.com>
>> +  - Rob Herring <robh+dt@kernel.org>
> Up to Rob, but I don't think Rob would like to be the maintainer. I think you
> can only include yourself and Heiko.
>
>
>> +  - Heiko Stuebner <heiko@sntech.de>
>> +
>> +description: |
>> +  Rockchip processors include support for multiple power domains which can be
>> +  powered up/down by software based on different application scenarios to save power.
>> +
>> +  Power domains contained within power-controller node are generic power domain
>> +  providers documented in Documentation/devicetree/bindings/power/power-domain.yaml.
>> +
>> +  IP cores belonging to a power domain should contain a "power-domains"
>> +  property that is a phandle for the power domain node representing the domain.
>> +
>> +properties:
>> +  $nodename:
>> +    const: power-controller
>> +
>> +  compatible:
>> +    enum:
>> +      - rockchip,px30-power-controller
>> +      - rockchip,rk3036-power-controller
>> +      - rockchip,rk3066-power-controller
>> +      - rockchip,rk3128-power-controller
>> +      - rockchip,rk3188-power-controller
>> +      - rockchip,rk3228-power-controller
>> +      - rockchip,rk3288-power-controller
>> +      - rockchip,rk3328-power-controller
>> +      - rockchip,rk3366-power-controller
>> +      - rockchip,rk3368-power-controller
>> +      - rockchip,rk3399-power-controller
>> +
>> +  "#power-domain-cells":
>> +    const: 1
>> +
>> +  "#address-cells":
>> +    const: 1
>> +
>> +  "#size-cells":
>> +    const: 0
>> +
>> +patternProperties:
>> +  "^pd_[0-9a-z_]{2,10}@[0-9a-f]+$":
>> +    type: object
>> +    description: |
>> +      Represents the power domains within the power controller node as documented
>> +      in Documentation/devicetree/bindings/power/power-domain.yaml.
>> +
> The node names must be generic, as this is power-domain must be in the form:
>
> +patternProperties:
> +  "^power-domain@[0-9a-f]+$":
In this way, dtbs_check cannot be passed, and all the usage methods in 
dts of Rockchip need to be corrected, which I think is a bigger change.
>
>
>> +    properties:
>> +
>> +      "#power-domain-cells":
>> +        description:
>> +          Must be 0 for nodes representing a single PM domain and 1 for nodes
>> +          providing multiple PM domains.
>> +
>> +      "#address-cells":
>> +        const: 1
>> +
>> +      "#size-cells":
>> +        const: 0
>> +
>> +      reg:
>> +        maxItems: 1
>> +        description: |
>> +          Power domain index. Valid values are defined in
>> +          "include/dt-bindings/power/px30-power.h"
>> +          "include/dt-bindings/power/rk3036-power.h"
>> +          "include/dt-bindings/power/rk3066-power.h"
>> +          "include/dt-bindings/power/rk3128-power.h"
>> +          "include/dt-bindings/power/rk3188-power.h"
>> +          "include/dt-bindings/power/rk3228-power.h"
>> +          "include/dt-bindings/power/rk3288-power.h"
>> +          "include/dt-bindings/power/rk3328-power.h"
>> +          "include/dt-bindings/power/rk3366-power.h"
>> +          "include/dt-bindings/power/rk3368-power.h"
>> +          "include/dt-bindings/power/rk3399-power.h"
>> +
>> +      clocks:
>> +        description: |
>> +          A number of phandles to clocks that need to be enabled while power domain
>> +          switches state.
>> +
>> +      pm_qos:
>> +        description: |
>> +          A number of phandles to qos blocks which need to be saved and restored
>> +          while power domain switches state.
>> +
>> +    patternProperties:
>> +      "^pd_[0-9a-z_]{2,10}@[0-9a-f]+$":
> The same here:
>
> +patternProperties:
> +  "^power-domain@[0-9a-f]+$":
>
>
>
>> +        type: object
>> +        description: |
>> +          Represents a power domain child within a power domain parent node.
>> +
>> +        properties:
>> +
>> +          "#power-domain-cells":
>> +            description:
>> +              Must be 0 for nodes representing a single PM domain and 1 for nodes
>> +              providing multiple PM domains.
>> +
>> +          "#address-cells":
>> +            const: 1
>> +
>> +          "#size-cells":
>> +            const: 0
>> +
>> +          reg:
>> +            maxItems: 1
>> +
>> +          clocks:
>> +            description: |
>> +              A number of phandles to clocks that need to be enabled while power domain
>> +              switches state.
>> +
>> +          pm_qos:
>> +            description: |
>> +              A number of phandles to qos blocks which need to be saved and restored
>> +              while power domain switches state.
>> +
>> +        patternProperties:
>> +          "^pd_[0-9a-z_]{2,10}@[0-9a-f]+$":
> And here:
>
>> +            type: object
>> +            description: |
>> +              Represents a power domain child within a power domain parent node.
>> +
>> +            properties:
>> +
>> +              "#power-domain-cells":
>> +                description:
>> +                  Must be 0 for nodes representing a single PM domain and 1 for nodes
>> +                  providing multiple PM domains.
>> +
>> +              "#address-cells":
>> +                const: 1
>> +
>> +              "#size-cells":
>> +                const: 0
>> +
>> +              reg:
>> +                maxItems: 1
>> +
>> +              clocks:
>> +                description: |
>> +                  A number of phandles to clocks that need to be enabled while power domain
>> +                  switches state.
>> +
>> +              pm_qos:
>> +                description: |
>> +                  A number of phandles to qos blocks which need to be saved and restored
>> +                  while power domain switches state.
>> +
>> +            required:
>> +              - reg
>> +
>> +            additionalProperties: false
>> +
>> +        required:
>> +          - reg
>> +
>> +        additionalProperties: false
>> +
>> +    required:
>> +      - reg
>> +
>> +    additionalProperties: false
>> +
>> +required:
>> +  - compatible
>> +  - "#power-domain-cells"
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/clock/rk3399-cru.h>
>> +    #include <dt-bindings/power/rk3399-power.h>
>> +
>> +    soc {
>> +        #address-cells = <2>;
>> +        #size-cells = <2>;
>> +
>> +        qos_hdcp: qos@ffa90000 {
>> +            compatible = "rockchip,rk3399-qos", "syscon";
>> +            reg = <0x0 0xffa90000 0x0 0x20>;
>> +        };
>> +
>> +        qos_iep: qos@ffa98000 {
>> +            compatible = "rockchip,rk3399-qos", "syscon";
>> +            reg = <0x0 0xffa98000 0x0 0x20>;
>> +        };
>> +
>> +        qos_rga_r: qos@ffab0000 {
>> +            compatible = "rockchip,rk3399-qos", "syscon";
>> +            reg = <0x0 0xffab0000 0x0 0x20>;
>> +        };
>> +
>> +        qos_rga_w: qos@ffab0080 {
>> +            compatible = "rockchip,rk3399-qos", "syscon";
>> +            reg = <0x0 0xffab0080 0x0 0x20>;
>> +        };
>> +
>> +        qos_video_m0: qos@ffab8000 {
>> +            compatible = "rockchip,rk3399-qos", "syscon";
>> +            reg = <0x0 0xffab8000 0x0 0x20>;
>> +        };
>> +
>> +        qos_video_m1_r: qos@ffac0000 {
>> +            compatible = "rockchip,rk3399-qos", "syscon";
>> +            reg = <0x0 0xffac0000 0x0 0x20>;
>> +        };
>> +
>> +        qos_video_m1_w: qos@ffac0080 {
>> +            compatible = "rockchip,rk3399-qos", "syscon";
>> +            reg = <0x0 0xffac0080 0x0 0x20>;
>> +        };
>> +
>> +        power-management@ff310000 {
>> +            compatible = "rockchip,rk3399-pmu", "syscon", "simple-mfd";
>> +            reg = <0x0 0xff310000 0x0 0x1000>;
>> +
>> +            power-controller {
>> +                compatible = "rockchip,rk3399-power-controller";
>> +                #power-domain-cells = <1>;
>> +                #address-cells = <1>;
>> +                #size-cells = <0>;
>> +
>> +                /* These power domains are grouped by VD_CENTER */
>> +                pd_iep@RK3399_PD_IEP {
> Here and below must be generic power-domain@RK3399_PD_IEP
>
>
>> +                    reg = <RK3399_PD_IEP>;
>> +                    clocks = <&cru ACLK_IEP>,
>> +                             <&cru HCLK_IEP>;
>> +                    pm_qos = <&qos_iep>;
>> +                    #power-domain-cells = <0>;
>> +                };
>> +                pd_rga@RK3399_PD_RGA {
>> +                    reg = <RK3399_PD_RGA>;
>> +                    clocks = <&cru ACLK_RGA>,
>> +                             <&cru HCLK_RGA>;
>> +                    pm_qos = <&qos_rga_r>,
>> +                             <&qos_rga_w>;
>> +                    #power-domain-cells = <0>;
>> +                };
>> +                pd_vcodec@RK3399_PD_VCODEC {
>> +                    reg = <RK3399_PD_VCODEC>;
>> +                    clocks = <&cru ACLK_VCODEC>,
>> +                             <&cru HCLK_VCODEC>;
>> +                    pm_qos = <&qos_video_m0>;
>> +                    #power-domain-cells = <0>;
>> +                };
>> +                pd_vdu@RK3399_PD_VDU {
>> +                    reg = <RK3399_PD_VDU>;
>> +                    clocks = <&cru ACLK_VDU>,
>> +                             <&cru HCLK_VDU>;
>> +                    pm_qos = <&qos_video_m1_r>,
>> +                             <&qos_video_m1_w>;
>> +                    #power-domain-cells = <0>;
>> +                };
>> +                pd_vio@RK3399_PD_VIO {
>> +                    reg = <RK3399_PD_VIO>;
>> +                    #power-domain-cells = <1>;
>> +                    #address-cells = <1>;
>> +                    #size-cells = <0>;
>> +
>> +                    pd_hdcp@RK3399_PD_HDCP {
>> +                        reg = <RK3399_PD_HDCP>;
>> +                        clocks = <&cru ACLK_HDCP>,
>> +                                 <&cru HCLK_HDCP>,
>> +                                 <&cru PCLK_HDCP>;
>> +                        pm_qos = <&qos_hdcp>;
>> +                        #power-domain-cells = <0>;
>> +                    };
>> +                };
>> +            };
>> +        };
>> +    };
>> diff --git a/Documentation/devicetree/bindings/soc/rockchip/power_domain.txt b/Documentation/devicetree/bindings/soc/rockchip/power_domain.txt
>> deleted file mode 100644
>> index 8304eceb62e4..000000000000
>> --- a/Documentation/devicetree/bindings/soc/rockchip/power_domain.txt
>> +++ /dev/null
>> @@ -1,136 +0,0 @@
>> -* Rockchip Power Domains
>> -
>> -Rockchip processors include support for multiple power domains which can be
>> -powered up/down by software based on different application scenes to save power.
>> -
>> -Required properties for power domain controller:
>> -- compatible: Should be one of the following.
>> -	"rockchip,px30-power-controller" - for PX30 SoCs.
>> -	"rockchip,rk3036-power-controller" - for RK3036 SoCs.
>> -	"rockchip,rk3066-power-controller" - for RK3066 SoCs.
>> -	"rockchip,rk3128-power-controller" - for RK3128 SoCs.
>> -	"rockchip,rk3188-power-controller" - for RK3188 SoCs.
>> -	"rockchip,rk3228-power-controller" - for RK3228 SoCs.
>> -	"rockchip,rk3288-power-controller" - for RK3288 SoCs.
>> -	"rockchip,rk3328-power-controller" - for RK3328 SoCs.
>> -	"rockchip,rk3366-power-controller" - for RK3366 SoCs.
>> -	"rockchip,rk3368-power-controller" - for RK3368 SoCs.
>> -	"rockchip,rk3399-power-controller" - for RK3399 SoCs.
>> -- #power-domain-cells: Number of cells in a power-domain specifier.
>> -	Should be 1 for multiple PM domains.
>> -- #address-cells: Should be 1.
>> -- #size-cells: Should be 0.
>> -
>> -Required properties for power domain sub nodes:
>> -- reg: index of the power domain, should use macros in:
>> -	"include/dt-bindings/power/px30-power.h" - for PX30 type power domain.
>> -	"include/dt-bindings/power/rk3036-power.h" - for RK3036 type power domain.
>> -	"include/dt-bindings/power/rk3066-power.h" - for RK3066 type power domain.
>> -	"include/dt-bindings/power/rk3128-power.h" - for RK3128 type power domain.
>> -	"include/dt-bindings/power/rk3188-power.h" - for RK3188 type power domain.
>> -	"include/dt-bindings/power/rk3228-power.h" - for RK3228 type power domain.
>> -	"include/dt-bindings/power/rk3288-power.h" - for RK3288 type power domain.
>> -	"include/dt-bindings/power/rk3328-power.h" - for RK3328 type power domain.
>> -	"include/dt-bindings/power/rk3366-power.h" - for RK3366 type power domain.
>> -	"include/dt-bindings/power/rk3368-power.h" - for RK3368 type power domain.
>> -	"include/dt-bindings/power/rk3399-power.h" - for RK3399 type power domain.
>> -- clocks (optional): phandles to clocks which need to be enabled while power domain
>> -	switches state.
>> -- pm_qos (optional): phandles to qos blocks which need to be saved and restored
>> -	while power domain switches state.
>> -
>> -Qos Example:
>> -
>> -	qos_gpu: qos_gpu@ffaf0000 {
>> -		compatible ="syscon";
>> -		reg = <0x0 0xffaf0000 0x0 0x20>;
>> -	};
>> -
>> -Example:
>> -
>> -	power: power-controller {
>> -		compatible = "rockchip,rk3288-power-controller";
>> -		#power-domain-cells = <1>;
>> -		#address-cells = <1>;
>> -		#size-cells = <0>;
>> -
>> -		pd_gpu {
>> -			reg = <RK3288_PD_GPU>;
>> -			clocks = <&cru ACLK_GPU>;
>> -			pm_qos = <&qos_gpu>;
>> -		};
>> -	};
>> -
>> -	 power: power-controller {
>> -                compatible = "rockchip,rk3368-power-controller";
>> -                #power-domain-cells = <1>;
>> -                #address-cells = <1>;
>> -                #size-cells = <0>;
>> -
>> -                pd_gpu_1 {
>> -                        reg = <RK3368_PD_GPU_1>;
>> -                        clocks = <&cru ACLK_GPU_CFG>;
>> -                };
>> -        };
>> -
>> -Example 2:
>> -		power: power-controller {
>> -			compatible = "rockchip,rk3399-power-controller";
>> -			#power-domain-cells = <1>;
>> -			#address-cells = <1>;
>> -			#size-cells = <0>;
>> -
>> -			pd_vio {
>> -				#address-cells = <1>;
>> -				#size-cells = <0>;
>> -				reg = <RK3399_PD_VIO>;
>> -
>> -				pd_vo {
>> -					#address-cells = <1>;
>> -					#size-cells = <0>;
>> -					reg = <RK3399_PD_VO>;
>> -
>> -					pd_vopb {
>> -						reg = <RK3399_PD_VOPB>;
>> -					};
>> -
>> -					pd_vopl {
>> -						reg = <RK3399_PD_VOPL>;
>> -					};
>> -				};
>> -			};
>> -		};
>> -
>> -Node of a device using power domains must have a power-domains property,
>> -containing a phandle to the power device node and an index specifying which
>> -power domain to use.
>> -The index should use macros in:
>> -	"include/dt-bindings/power/px30-power.h" - for px30 type power domain.
>> -	"include/dt-bindings/power/rk3036-power.h" - for rk3036 type power domain.
>> -	"include/dt-bindings/power/rk3128-power.h" - for rk3128 type power domain.
>> -	"include/dt-bindings/power/rk3128-power.h" - for rk3228 type power domain.
>> -	"include/dt-bindings/power/rk3288-power.h" - for rk3288 type power domain.
>> -	"include/dt-bindings/power/rk3328-power.h" - for rk3328 type power domain.
>> -	"include/dt-bindings/power/rk3366-power.h" - for rk3366 type power domain.
>> -	"include/dt-bindings/power/rk3368-power.h" - for rk3368 type power domain.
>> -	"include/dt-bindings/power/rk3399-power.h" - for rk3399 type power domain.
>> -
>> -Example of the node using power domain:
>> -
>> -	node {
>> -		/* ... */
>> -		power-domains = <&power RK3288_PD_GPU>;
>> -		/* ... */
>> -	};
>> -
>> -	node {
>> -                /* ... */
>> -                power-domains = <&power RK3368_PD_GPU_1>;
>> -                /* ... */
>> -        };
>> -
>> -	node {
>> -		/* ... */
>> -		power-domains = <&power RK3399_PD_VOPB>;
>> -		/* ... */
>> -	};
>>
>
>
Enric Balletbo i Serra March 24, 2021, 10:25 a.m. UTC | #5
Hi Elaine,

On 24/3/21 11:18, elaine.zhang wrote:
> Hi,  Enric
> 
> 在 2021/3/24 下午5:56, Enric Balletbo i Serra 写道:
>> Hi Elaine,
>>
>> This is not the exact version I sent, and you reintroduced a "problem" that were
>> already solved/discussed on previous versions. See below:
>>
>> On 24/3/21 8:16, Elaine Zhang wrote:
>>> Convert the soc/rockchip/power_domain.txt binding document to
>>> json-schema and move to the power bindings directory.
>>>
>>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>> If you do significant is a good practice shortly describe them within [] here.
>>
>>> Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
>> Note that my last version already had the
>>
>> Reviewed-by: Rob Herring <robh@kernel.org>
>>
>> Which should be fine for merging (with probably only minor changes) and you
>> could maintain if you don't do significant changes, but that's not the case, as
>> I said, you are reintroducing one problem. Please review the comments already
>> received on this patchset or similar patchsets to avoid this.
>>
>>> ---
>>>   .../power/rockchip,power-controller.yaml      | 284 ++++++++++++++++++
>>>   .../bindings/soc/rockchip/power_domain.txt    | 136 ---------
>>>   2 files changed, 284 insertions(+), 136 deletions(-)
>>>   create mode 100644
>>> Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
>>>   delete mode 100644
>>> Documentation/devicetree/bindings/soc/rockchip/power_domain.txt
>>>
>>> diff --git
>>> a/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
>>> b/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
>>> new file mode 100644
>>> index 000000000000..a220322c5139
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
>>> @@ -0,0 +1,284 @@
>>> +# SPDX-License-Identifier: GPL-2.0-only
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/power/rockchip,power-controller.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Rockchip Power Domains
>>> +
>>> +maintainers:
>>> +  - Elaine Zhang <zhangqing@rock-chips.com>
>>> +  - Rob Herring <robh+dt@kernel.org>
>> Up to Rob, but I don't think Rob would like to be the maintainer. I think you
>> can only include yourself and Heiko.
>>
>>
>>> +  - Heiko Stuebner <heiko@sntech.de>
>>> +
>>> +description: |
>>> +  Rockchip processors include support for multiple power domains which can be
>>> +  powered up/down by software based on different application scenarios to
>>> save power.
>>> +
>>> +  Power domains contained within power-controller node are generic power domain
>>> +  providers documented in
>>> Documentation/devicetree/bindings/power/power-domain.yaml.
>>> +
>>> +  IP cores belonging to a power domain should contain a "power-domains"
>>> +  property that is a phandle for the power domain node representing the domain.
>>> +
>>> +properties:
>>> +  $nodename:
>>> +    const: power-controller
>>> +
>>> +  compatible:
>>> +    enum:
>>> +      - rockchip,px30-power-controller
>>> +      - rockchip,rk3036-power-controller
>>> +      - rockchip,rk3066-power-controller
>>> +      - rockchip,rk3128-power-controller
>>> +      - rockchip,rk3188-power-controller
>>> +      - rockchip,rk3228-power-controller
>>> +      - rockchip,rk3288-power-controller
>>> +      - rockchip,rk3328-power-controller
>>> +      - rockchip,rk3366-power-controller
>>> +      - rockchip,rk3368-power-controller
>>> +      - rockchip,rk3399-power-controller
>>> +
>>> +  "#power-domain-cells":
>>> +    const: 1
>>> +
>>> +  "#address-cells":
>>> +    const: 1
>>> +
>>> +  "#size-cells":
>>> +    const: 0
>>> +
>>> +patternProperties:
>>> +  "^pd_[0-9a-z_]{2,10}@[0-9a-f]+$":
>>> +    type: object
>>> +    description: |
>>> +      Represents the power domains within the power controller node as
>>> documented
>>> +      in Documentation/devicetree/bindings/power/power-domain.yaml.
>>> +
>> The node names must be generic, as this is power-domain must be in the form:
>>
>> +patternProperties:
>> +  "^power-domain@[0-9a-f]+$":
> In this way, dtbs_check cannot be passed, and all the usage methods in dts of
> Rockchip need to be corrected, which I think is a bigger change.

Well, the problem is in the Rockchip dtbs, so needs to be fixed there. The
bindings must describe hardware in a generic way, not describe the actual dtbs
to not report errors.

Thanks,
 Enric

>>
>>
>>> +    properties:
>>> +
>>> +      "#power-domain-cells":
>>> +        description:
>>> +          Must be 0 for nodes representing a single PM domain and 1 for nodes
>>> +          providing multiple PM domains.
>>> +
>>> +      "#address-cells":
>>> +        const: 1
>>> +
>>> +      "#size-cells":
>>> +        const: 0
>>> +
>>> +      reg:
>>> +        maxItems: 1
>>> +        description: |
>>> +          Power domain index. Valid values are defined in
>>> +          "include/dt-bindings/power/px30-power.h"
>>> +          "include/dt-bindings/power/rk3036-power.h"
>>> +          "include/dt-bindings/power/rk3066-power.h"
>>> +          "include/dt-bindings/power/rk3128-power.h"
>>> +          "include/dt-bindings/power/rk3188-power.h"
>>> +          "include/dt-bindings/power/rk3228-power.h"
>>> +          "include/dt-bindings/power/rk3288-power.h"
>>> +          "include/dt-bindings/power/rk3328-power.h"
>>> +          "include/dt-bindings/power/rk3366-power.h"
>>> +          "include/dt-bindings/power/rk3368-power.h"
>>> +          "include/dt-bindings/power/rk3399-power.h"
>>> +
>>> +      clocks:
>>> +        description: |
>>> +          A number of phandles to clocks that need to be enabled while power
>>> domain
>>> +          switches state.
>>> +
>>> +      pm_qos:
>>> +        description: |
>>> +          A number of phandles to qos blocks which need to be saved and
>>> restored
>>> +          while power domain switches state.
>>> +
>>> +    patternProperties:
>>> +      "^pd_[0-9a-z_]{2,10}@[0-9a-f]+$":
>> The same here:
>>
>> +patternProperties:
>> +  "^power-domain@[0-9a-f]+$":
>>
>>
>>
>>> +        type: object
>>> +        description: |
>>> +          Represents a power domain child within a power domain parent node.
>>> +
>>> +        properties:
>>> +
>>> +          "#power-domain-cells":
>>> +            description:
>>> +              Must be 0 for nodes representing a single PM domain and 1 for
>>> nodes
>>> +              providing multiple PM domains.
>>> +
>>> +          "#address-cells":
>>> +            const: 1
>>> +
>>> +          "#size-cells":
>>> +            const: 0
>>> +
>>> +          reg:
>>> +            maxItems: 1
>>> +
>>> +          clocks:
>>> +            description: |
>>> +              A number of phandles to clocks that need to be enabled while
>>> power domain
>>> +              switches state.
>>> +
>>> +          pm_qos:
>>> +            description: |
>>> +              A number of phandles to qos blocks which need to be saved and
>>> restored
>>> +              while power domain switches state.
>>> +
>>> +        patternProperties:
>>> +          "^pd_[0-9a-z_]{2,10}@[0-9a-f]+$":
>> And here:
>>
>>> +            type: object
>>> +            description: |
>>> +              Represents a power domain child within a power domain parent
>>> node.
>>> +
>>> +            properties:
>>> +
>>> +              "#power-domain-cells":
>>> +                description:
>>> +                  Must be 0 for nodes representing a single PM domain and 1
>>> for nodes
>>> +                  providing multiple PM domains.
>>> +
>>> +              "#address-cells":
>>> +                const: 1
>>> +
>>> +              "#size-cells":
>>> +                const: 0
>>> +
>>> +              reg:
>>> +                maxItems: 1
>>> +
>>> +              clocks:
>>> +                description: |
>>> +                  A number of phandles to clocks that need to be enabled
>>> while power domain
>>> +                  switches state.
>>> +
>>> +              pm_qos:
>>> +                description: |
>>> +                  A number of phandles to qos blocks which need to be saved
>>> and restored
>>> +                  while power domain switches state.
>>> +
>>> +            required:
>>> +              - reg
>>> +
>>> +            additionalProperties: false
>>> +
>>> +        required:
>>> +          - reg
>>> +
>>> +        additionalProperties: false
>>> +
>>> +    required:
>>> +      - reg
>>> +
>>> +    additionalProperties: false
>>> +
>>> +required:
>>> +  - compatible
>>> +  - "#power-domain-cells"
>>> +
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> +  - |
>>> +    #include <dt-bindings/clock/rk3399-cru.h>
>>> +    #include <dt-bindings/power/rk3399-power.h>
>>> +
>>> +    soc {
>>> +        #address-cells = <2>;
>>> +        #size-cells = <2>;
>>> +
>>> +        qos_hdcp: qos@ffa90000 {
>>> +            compatible = "rockchip,rk3399-qos", "syscon";
>>> +            reg = <0x0 0xffa90000 0x0 0x20>;
>>> +        };
>>> +
>>> +        qos_iep: qos@ffa98000 {
>>> +            compatible = "rockchip,rk3399-qos", "syscon";
>>> +            reg = <0x0 0xffa98000 0x0 0x20>;
>>> +        };
>>> +
>>> +        qos_rga_r: qos@ffab0000 {
>>> +            compatible = "rockchip,rk3399-qos", "syscon";
>>> +            reg = <0x0 0xffab0000 0x0 0x20>;
>>> +        };
>>> +
>>> +        qos_rga_w: qos@ffab0080 {
>>> +            compatible = "rockchip,rk3399-qos", "syscon";
>>> +            reg = <0x0 0xffab0080 0x0 0x20>;
>>> +        };
>>> +
>>> +        qos_video_m0: qos@ffab8000 {
>>> +            compatible = "rockchip,rk3399-qos", "syscon";
>>> +            reg = <0x0 0xffab8000 0x0 0x20>;
>>> +        };
>>> +
>>> +        qos_video_m1_r: qos@ffac0000 {
>>> +            compatible = "rockchip,rk3399-qos", "syscon";
>>> +            reg = <0x0 0xffac0000 0x0 0x20>;
>>> +        };
>>> +
>>> +        qos_video_m1_w: qos@ffac0080 {
>>> +            compatible = "rockchip,rk3399-qos", "syscon";
>>> +            reg = <0x0 0xffac0080 0x0 0x20>;
>>> +        };
>>> +
>>> +        power-management@ff310000 {
>>> +            compatible = "rockchip,rk3399-pmu", "syscon", "simple-mfd";
>>> +            reg = <0x0 0xff310000 0x0 0x1000>;
>>> +
>>> +            power-controller {
>>> +                compatible = "rockchip,rk3399-power-controller";
>>> +                #power-domain-cells = <1>;
>>> +                #address-cells = <1>;
>>> +                #size-cells = <0>;
>>> +
>>> +                /* These power domains are grouped by VD_CENTER */
>>> +                pd_iep@RK3399_PD_IEP {
>> Here and below must be generic power-domain@RK3399_PD_IEP
>>
>>
>>> +                    reg = <RK3399_PD_IEP>;
>>> +                    clocks = <&cru ACLK_IEP>,
>>> +                             <&cru HCLK_IEP>;
>>> +                    pm_qos = <&qos_iep>;
>>> +                    #power-domain-cells = <0>;
>>> +                };
>>> +                pd_rga@RK3399_PD_RGA {
>>> +                    reg = <RK3399_PD_RGA>;
>>> +                    clocks = <&cru ACLK_RGA>,
>>> +                             <&cru HCLK_RGA>;
>>> +                    pm_qos = <&qos_rga_r>,
>>> +                             <&qos_rga_w>;
>>> +                    #power-domain-cells = <0>;
>>> +                };
>>> +                pd_vcodec@RK3399_PD_VCODEC {
>>> +                    reg = <RK3399_PD_VCODEC>;
>>> +                    clocks = <&cru ACLK_VCODEC>,
>>> +                             <&cru HCLK_VCODEC>;
>>> +                    pm_qos = <&qos_video_m0>;
>>> +                    #power-domain-cells = <0>;
>>> +                };
>>> +                pd_vdu@RK3399_PD_VDU {
>>> +                    reg = <RK3399_PD_VDU>;
>>> +                    clocks = <&cru ACLK_VDU>,
>>> +                             <&cru HCLK_VDU>;
>>> +                    pm_qos = <&qos_video_m1_r>,
>>> +                             <&qos_video_m1_w>;
>>> +                    #power-domain-cells = <0>;
>>> +                };
>>> +                pd_vio@RK3399_PD_VIO {
>>> +                    reg = <RK3399_PD_VIO>;
>>> +                    #power-domain-cells = <1>;
>>> +                    #address-cells = <1>;
>>> +                    #size-cells = <0>;
>>> +
>>> +                    pd_hdcp@RK3399_PD_HDCP {
>>> +                        reg = <RK3399_PD_HDCP>;
>>> +                        clocks = <&cru ACLK_HDCP>,
>>> +                                 <&cru HCLK_HDCP>,
>>> +                                 <&cru PCLK_HDCP>;
>>> +                        pm_qos = <&qos_hdcp>;
>>> +                        #power-domain-cells = <0>;
>>> +                    };
>>> +                };
>>> +            };
>>> +        };
>>> +    };
>>> diff --git a/Documentation/devicetree/bindings/soc/rockchip/power_domain.txt
>>> b/Documentation/devicetree/bindings/soc/rockchip/power_domain.txt
>>> deleted file mode 100644
>>> index 8304eceb62e4..000000000000
>>> --- a/Documentation/devicetree/bindings/soc/rockchip/power_domain.txt
>>> +++ /dev/null
>>> @@ -1,136 +0,0 @@
>>> -* Rockchip Power Domains
>>> -
>>> -Rockchip processors include support for multiple power domains which can be
>>> -powered up/down by software based on different application scenes to save
>>> power.
>>> -
>>> -Required properties for power domain controller:
>>> -- compatible: Should be one of the following.
>>> -    "rockchip,px30-power-controller" - for PX30 SoCs.
>>> -    "rockchip,rk3036-power-controller" - for RK3036 SoCs.
>>> -    "rockchip,rk3066-power-controller" - for RK3066 SoCs.
>>> -    "rockchip,rk3128-power-controller" - for RK3128 SoCs.
>>> -    "rockchip,rk3188-power-controller" - for RK3188 SoCs.
>>> -    "rockchip,rk3228-power-controller" - for RK3228 SoCs.
>>> -    "rockchip,rk3288-power-controller" - for RK3288 SoCs.
>>> -    "rockchip,rk3328-power-controller" - for RK3328 SoCs.
>>> -    "rockchip,rk3366-power-controller" - for RK3366 SoCs.
>>> -    "rockchip,rk3368-power-controller" - for RK3368 SoCs.
>>> -    "rockchip,rk3399-power-controller" - for RK3399 SoCs.
>>> -- #power-domain-cells: Number of cells in a power-domain specifier.
>>> -    Should be 1 for multiple PM domains.
>>> -- #address-cells: Should be 1.
>>> -- #size-cells: Should be 0.
>>> -
>>> -Required properties for power domain sub nodes:
>>> -- reg: index of the power domain, should use macros in:
>>> -    "include/dt-bindings/power/px30-power.h" - for PX30 type power domain.
>>> -    "include/dt-bindings/power/rk3036-power.h" - for RK3036 type power domain.
>>> -    "include/dt-bindings/power/rk3066-power.h" - for RK3066 type power domain.
>>> -    "include/dt-bindings/power/rk3128-power.h" - for RK3128 type power domain.
>>> -    "include/dt-bindings/power/rk3188-power.h" - for RK3188 type power domain.
>>> -    "include/dt-bindings/power/rk3228-power.h" - for RK3228 type power domain.
>>> -    "include/dt-bindings/power/rk3288-power.h" - for RK3288 type power domain.
>>> -    "include/dt-bindings/power/rk3328-power.h" - for RK3328 type power domain.
>>> -    "include/dt-bindings/power/rk3366-power.h" - for RK3366 type power domain.
>>> -    "include/dt-bindings/power/rk3368-power.h" - for RK3368 type power domain.
>>> -    "include/dt-bindings/power/rk3399-power.h" - for RK3399 type power domain.
>>> -- clocks (optional): phandles to clocks which need to be enabled while power
>>> domain
>>> -    switches state.
>>> -- pm_qos (optional): phandles to qos blocks which need to be saved and restored
>>> -    while power domain switches state.
>>> -
>>> -Qos Example:
>>> -
>>> -    qos_gpu: qos_gpu@ffaf0000 {
>>> -        compatible ="syscon";
>>> -        reg = <0x0 0xffaf0000 0x0 0x20>;
>>> -    };
>>> -
>>> -Example:
>>> -
>>> -    power: power-controller {
>>> -        compatible = "rockchip,rk3288-power-controller";
>>> -        #power-domain-cells = <1>;
>>> -        #address-cells = <1>;
>>> -        #size-cells = <0>;
>>> -
>>> -        pd_gpu {
>>> -            reg = <RK3288_PD_GPU>;
>>> -            clocks = <&cru ACLK_GPU>;
>>> -            pm_qos = <&qos_gpu>;
>>> -        };
>>> -    };
>>> -
>>> -     power: power-controller {
>>> -                compatible = "rockchip,rk3368-power-controller";
>>> -                #power-domain-cells = <1>;
>>> -                #address-cells = <1>;
>>> -                #size-cells = <0>;
>>> -
>>> -                pd_gpu_1 {
>>> -                        reg = <RK3368_PD_GPU_1>;
>>> -                        clocks = <&cru ACLK_GPU_CFG>;
>>> -                };
>>> -        };
>>> -
>>> -Example 2:
>>> -        power: power-controller {
>>> -            compatible = "rockchip,rk3399-power-controller";
>>> -            #power-domain-cells = <1>;
>>> -            #address-cells = <1>;
>>> -            #size-cells = <0>;
>>> -
>>> -            pd_vio {
>>> -                #address-cells = <1>;
>>> -                #size-cells = <0>;
>>> -                reg = <RK3399_PD_VIO>;
>>> -
>>> -                pd_vo {
>>> -                    #address-cells = <1>;
>>> -                    #size-cells = <0>;
>>> -                    reg = <RK3399_PD_VO>;
>>> -
>>> -                    pd_vopb {
>>> -                        reg = <RK3399_PD_VOPB>;
>>> -                    };
>>> -
>>> -                    pd_vopl {
>>> -                        reg = <RK3399_PD_VOPL>;
>>> -                    };
>>> -                };
>>> -            };
>>> -        };
>>> -
>>> -Node of a device using power domains must have a power-domains property,
>>> -containing a phandle to the power device node and an index specifying which
>>> -power domain to use.
>>> -The index should use macros in:
>>> -    "include/dt-bindings/power/px30-power.h" - for px30 type power domain.
>>> -    "include/dt-bindings/power/rk3036-power.h" - for rk3036 type power domain.
>>> -    "include/dt-bindings/power/rk3128-power.h" - for rk3128 type power domain.
>>> -    "include/dt-bindings/power/rk3128-power.h" - for rk3228 type power domain.
>>> -    "include/dt-bindings/power/rk3288-power.h" - for rk3288 type power domain.
>>> -    "include/dt-bindings/power/rk3328-power.h" - for rk3328 type power domain.
>>> -    "include/dt-bindings/power/rk3366-power.h" - for rk3366 type power domain.
>>> -    "include/dt-bindings/power/rk3368-power.h" - for rk3368 type power domain.
>>> -    "include/dt-bindings/power/rk3399-power.h" - for rk3399 type power domain.
>>> -
>>> -Example of the node using power domain:
>>> -
>>> -    node {
>>> -        /* ... */
>>> -        power-domains = <&power RK3288_PD_GPU>;
>>> -        /* ... */
>>> -    };
>>> -
>>> -    node {
>>> -                /* ... */
>>> -                power-domains = <&power RK3368_PD_GPU_1>;
>>> -                /* ... */
>>> -        };
>>> -
>>> -    node {
>>> -        /* ... */
>>> -        power-domains = <&power RK3399_PD_VOPB>;
>>> -        /* ... */
>>> -    };
>>>
>>
>>
> 
>
Enric Balletbo i Serra March 24, 2021, 10:32 a.m. UTC | #6
On 24/3/21 11:25, Enric Balletbo i Serra wrote:
> Hi Elaine,
> 
> On 24/3/21 11:18, elaine.zhang wrote:
>> Hi,  Enric
>>
>> 在 2021/3/24 下午5:56, Enric Balletbo i Serra 写道:
>>> Hi Elaine,
>>>
>>> This is not the exact version I sent, and you reintroduced a "problem" that were
>>> already solved/discussed on previous versions. See below:
>>>
>>> On 24/3/21 8:16, Elaine Zhang wrote:
>>>> Convert the soc/rockchip/power_domain.txt binding document to
>>>> json-schema and move to the power bindings directory.
>>>>
>>>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>>> If you do significant is a good practice shortly describe them within [] here.
>>>
>>>> Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
>>> Note that my last version already had the
>>>
>>> Reviewed-by: Rob Herring <robh@kernel.org>
>>>
>>> Which should be fine for merging (with probably only minor changes) and you
>>> could maintain if you don't do significant changes, but that's not the case, as
>>> I said, you are reintroducing one problem. Please review the comments already
>>> received on this patchset or similar patchsets to avoid this.
>>>
>>>> ---
>>>>   .../power/rockchip,power-controller.yaml      | 284 ++++++++++++++++++
>>>>   .../bindings/soc/rockchip/power_domain.txt    | 136 ---------
>>>>   2 files changed, 284 insertions(+), 136 deletions(-)
>>>>   create mode 100644
>>>> Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
>>>>   delete mode 100644
>>>> Documentation/devicetree/bindings/soc/rockchip/power_domain.txt
>>>>
>>>> diff --git
>>>> a/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
>>>> b/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
>>>> new file mode 100644
>>>> index 000000000000..a220322c5139
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
>>>> @@ -0,0 +1,284 @@
>>>> +# SPDX-License-Identifier: GPL-2.0-only
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/power/rockchip,power-controller.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: Rockchip Power Domains
>>>> +
>>>> +maintainers:
>>>> +  - Elaine Zhang <zhangqing@rock-chips.com>
>>>> +  - Rob Herring <robh+dt@kernel.org>
>>> Up to Rob, but I don't think Rob would like to be the maintainer. I think you
>>> can only include yourself and Heiko.
>>>
>>>
>>>> +  - Heiko Stuebner <heiko@sntech.de>
>>>> +
>>>> +description: |
>>>> +  Rockchip processors include support for multiple power domains which can be
>>>> +  powered up/down by software based on different application scenarios to
>>>> save power.
>>>> +
>>>> +  Power domains contained within power-controller node are generic power domain
>>>> +  providers documented in
>>>> Documentation/devicetree/bindings/power/power-domain.yaml.
>>>> +
>>>> +  IP cores belonging to a power domain should contain a "power-domains"
>>>> +  property that is a phandle for the power domain node representing the domain.
>>>> +
>>>> +properties:
>>>> +  $nodename:
>>>> +    const: power-controller
>>>> +
>>>> +  compatible:
>>>> +    enum:
>>>> +      - rockchip,px30-power-controller
>>>> +      - rockchip,rk3036-power-controller
>>>> +      - rockchip,rk3066-power-controller
>>>> +      - rockchip,rk3128-power-controller
>>>> +      - rockchip,rk3188-power-controller
>>>> +      - rockchip,rk3228-power-controller
>>>> +      - rockchip,rk3288-power-controller
>>>> +      - rockchip,rk3328-power-controller
>>>> +      - rockchip,rk3366-power-controller
>>>> +      - rockchip,rk3368-power-controller
>>>> +      - rockchip,rk3399-power-controller
>>>> +
>>>> +  "#power-domain-cells":
>>>> +    const: 1
>>>> +
>>>> +  "#address-cells":
>>>> +    const: 1
>>>> +
>>>> +  "#size-cells":
>>>> +    const: 0
>>>> +
>>>> +patternProperties:
>>>> +  "^pd_[0-9a-z_]{2,10}@[0-9a-f]+$":
>>>> +    type: object
>>>> +    description: |
>>>> +      Represents the power domains within the power controller node as
>>>> documented
>>>> +      in Documentation/devicetree/bindings/power/power-domain.yaml.
>>>> +
>>> The node names must be generic, as this is power-domain must be in the form:
>>>
>>> +patternProperties:
>>> +  "^power-domain@[0-9a-f]+$":
>> In this way, dtbs_check cannot be passed, and all the usage methods in dts of
>> Rockchip need to be corrected, which I think is a bigger change.
> 
> Well, the problem is in the Rockchip dtbs, so needs to be fixed there. The
> bindings must describe hardware in a generic way, not describe the actual dtbs
> to not report errors.
> 

FWIW I remember I did something regarding this but never sent to upstream, feel
free to pick if you find useful.

*
https://gitlab.collabora.com/eballetbo/linux/-/commit/12499f223e3d33602449b9102404fe573fb804f5
*
https://gitlab.collabora.com/eballetbo/linux/-/commit/12499f223e3d33602449b9102404fe573fb804f5
*
https://gitlab.collabora.com/eballetbo/linux/-/commit/492bf2213c341152a1c2423242c5634b9e53ff27

> Thanks,
>  Enric
> 
>>>
>>>
>>>> +    properties:
>>>> +
>>>> +      "#power-domain-cells":
>>>> +        description:
>>>> +          Must be 0 for nodes representing a single PM domain and 1 for nodes
>>>> +          providing multiple PM domains.
>>>> +
>>>> +      "#address-cells":
>>>> +        const: 1
>>>> +
>>>> +      "#size-cells":
>>>> +        const: 0
>>>> +
>>>> +      reg:
>>>> +        maxItems: 1
>>>> +        description: |
>>>> +          Power domain index. Valid values are defined in
>>>> +          "include/dt-bindings/power/px30-power.h"
>>>> +          "include/dt-bindings/power/rk3036-power.h"
>>>> +          "include/dt-bindings/power/rk3066-power.h"
>>>> +          "include/dt-bindings/power/rk3128-power.h"
>>>> +          "include/dt-bindings/power/rk3188-power.h"
>>>> +          "include/dt-bindings/power/rk3228-power.h"
>>>> +          "include/dt-bindings/power/rk3288-power.h"
>>>> +          "include/dt-bindings/power/rk3328-power.h"
>>>> +          "include/dt-bindings/power/rk3366-power.h"
>>>> +          "include/dt-bindings/power/rk3368-power.h"
>>>> +          "include/dt-bindings/power/rk3399-power.h"
>>>> +
>>>> +      clocks:
>>>> +        description: |
>>>> +          A number of phandles to clocks that need to be enabled while power
>>>> domain
>>>> +          switches state.
>>>> +
>>>> +      pm_qos:
>>>> +        description: |
>>>> +          A number of phandles to qos blocks which need to be saved and
>>>> restored
>>>> +          while power domain switches state.
>>>> +
>>>> +    patternProperties:
>>>> +      "^pd_[0-9a-z_]{2,10}@[0-9a-f]+$":
>>> The same here:
>>>
>>> +patternProperties:
>>> +  "^power-domain@[0-9a-f]+$":
>>>
>>>
>>>
>>>> +        type: object
>>>> +        description: |
>>>> +          Represents a power domain child within a power domain parent node.
>>>> +
>>>> +        properties:
>>>> +
>>>> +          "#power-domain-cells":
>>>> +            description:
>>>> +              Must be 0 for nodes representing a single PM domain and 1 for
>>>> nodes
>>>> +              providing multiple PM domains.
>>>> +
>>>> +          "#address-cells":
>>>> +            const: 1
>>>> +
>>>> +          "#size-cells":
>>>> +            const: 0
>>>> +
>>>> +          reg:
>>>> +            maxItems: 1
>>>> +
>>>> +          clocks:
>>>> +            description: |
>>>> +              A number of phandles to clocks that need to be enabled while
>>>> power domain
>>>> +              switches state.
>>>> +
>>>> +          pm_qos:
>>>> +            description: |
>>>> +              A number of phandles to qos blocks which need to be saved and
>>>> restored
>>>> +              while power domain switches state.
>>>> +
>>>> +        patternProperties:
>>>> +          "^pd_[0-9a-z_]{2,10}@[0-9a-f]+$":
>>> And here:
>>>
>>>> +            type: object
>>>> +            description: |
>>>> +              Represents a power domain child within a power domain parent
>>>> node.
>>>> +
>>>> +            properties:
>>>> +
>>>> +              "#power-domain-cells":
>>>> +                description:
>>>> +                  Must be 0 for nodes representing a single PM domain and 1
>>>> for nodes
>>>> +                  providing multiple PM domains.
>>>> +
>>>> +              "#address-cells":
>>>> +                const: 1
>>>> +
>>>> +              "#size-cells":
>>>> +                const: 0
>>>> +
>>>> +              reg:
>>>> +                maxItems: 1
>>>> +
>>>> +              clocks:
>>>> +                description: |
>>>> +                  A number of phandles to clocks that need to be enabled
>>>> while power domain
>>>> +                  switches state.
>>>> +
>>>> +              pm_qos:
>>>> +                description: |
>>>> +                  A number of phandles to qos blocks which need to be saved
>>>> and restored
>>>> +                  while power domain switches state.
>>>> +
>>>> +            required:
>>>> +              - reg
>>>> +
>>>> +            additionalProperties: false
>>>> +
>>>> +        required:
>>>> +          - reg
>>>> +
>>>> +        additionalProperties: false
>>>> +
>>>> +    required:
>>>> +      - reg
>>>> +
>>>> +    additionalProperties: false
>>>> +
>>>> +required:
>>>> +  - compatible
>>>> +  - "#power-domain-cells"
>>>> +
>>>> +additionalProperties: false
>>>> +
>>>> +examples:
>>>> +  - |
>>>> +    #include <dt-bindings/clock/rk3399-cru.h>
>>>> +    #include <dt-bindings/power/rk3399-power.h>
>>>> +
>>>> +    soc {
>>>> +        #address-cells = <2>;
>>>> +        #size-cells = <2>;
>>>> +
>>>> +        qos_hdcp: qos@ffa90000 {
>>>> +            compatible = "rockchip,rk3399-qos", "syscon";
>>>> +            reg = <0x0 0xffa90000 0x0 0x20>;
>>>> +        };
>>>> +
>>>> +        qos_iep: qos@ffa98000 {
>>>> +            compatible = "rockchip,rk3399-qos", "syscon";
>>>> +            reg = <0x0 0xffa98000 0x0 0x20>;
>>>> +        };
>>>> +
>>>> +        qos_rga_r: qos@ffab0000 {
>>>> +            compatible = "rockchip,rk3399-qos", "syscon";
>>>> +            reg = <0x0 0xffab0000 0x0 0x20>;
>>>> +        };
>>>> +
>>>> +        qos_rga_w: qos@ffab0080 {
>>>> +            compatible = "rockchip,rk3399-qos", "syscon";
>>>> +            reg = <0x0 0xffab0080 0x0 0x20>;
>>>> +        };
>>>> +
>>>> +        qos_video_m0: qos@ffab8000 {
>>>> +            compatible = "rockchip,rk3399-qos", "syscon";
>>>> +            reg = <0x0 0xffab8000 0x0 0x20>;
>>>> +        };
>>>> +
>>>> +        qos_video_m1_r: qos@ffac0000 {
>>>> +            compatible = "rockchip,rk3399-qos", "syscon";
>>>> +            reg = <0x0 0xffac0000 0x0 0x20>;
>>>> +        };
>>>> +
>>>> +        qos_video_m1_w: qos@ffac0080 {
>>>> +            compatible = "rockchip,rk3399-qos", "syscon";
>>>> +            reg = <0x0 0xffac0080 0x0 0x20>;
>>>> +        };
>>>> +
>>>> +        power-management@ff310000 {
>>>> +            compatible = "rockchip,rk3399-pmu", "syscon", "simple-mfd";
>>>> +            reg = <0x0 0xff310000 0x0 0x1000>;
>>>> +
>>>> +            power-controller {
>>>> +                compatible = "rockchip,rk3399-power-controller";
>>>> +                #power-domain-cells = <1>;
>>>> +                #address-cells = <1>;
>>>> +                #size-cells = <0>;
>>>> +
>>>> +                /* These power domains are grouped by VD_CENTER */
>>>> +                pd_iep@RK3399_PD_IEP {
>>> Here and below must be generic power-domain@RK3399_PD_IEP
>>>
>>>
>>>> +                    reg = <RK3399_PD_IEP>;
>>>> +                    clocks = <&cru ACLK_IEP>,
>>>> +                             <&cru HCLK_IEP>;
>>>> +                    pm_qos = <&qos_iep>;
>>>> +                    #power-domain-cells = <0>;
>>>> +                };
>>>> +                pd_rga@RK3399_PD_RGA {
>>>> +                    reg = <RK3399_PD_RGA>;
>>>> +                    clocks = <&cru ACLK_RGA>,
>>>> +                             <&cru HCLK_RGA>;
>>>> +                    pm_qos = <&qos_rga_r>,
>>>> +                             <&qos_rga_w>;
>>>> +                    #power-domain-cells = <0>;
>>>> +                };
>>>> +                pd_vcodec@RK3399_PD_VCODEC {
>>>> +                    reg = <RK3399_PD_VCODEC>;
>>>> +                    clocks = <&cru ACLK_VCODEC>,
>>>> +                             <&cru HCLK_VCODEC>;
>>>> +                    pm_qos = <&qos_video_m0>;
>>>> +                    #power-domain-cells = <0>;
>>>> +                };
>>>> +                pd_vdu@RK3399_PD_VDU {
>>>> +                    reg = <RK3399_PD_VDU>;
>>>> +                    clocks = <&cru ACLK_VDU>,
>>>> +                             <&cru HCLK_VDU>;
>>>> +                    pm_qos = <&qos_video_m1_r>,
>>>> +                             <&qos_video_m1_w>;
>>>> +                    #power-domain-cells = <0>;
>>>> +                };
>>>> +                pd_vio@RK3399_PD_VIO {
>>>> +                    reg = <RK3399_PD_VIO>;
>>>> +                    #power-domain-cells = <1>;
>>>> +                    #address-cells = <1>;
>>>> +                    #size-cells = <0>;
>>>> +
>>>> +                    pd_hdcp@RK3399_PD_HDCP {
>>>> +                        reg = <RK3399_PD_HDCP>;
>>>> +                        clocks = <&cru ACLK_HDCP>,
>>>> +                                 <&cru HCLK_HDCP>,
>>>> +                                 <&cru PCLK_HDCP>;
>>>> +                        pm_qos = <&qos_hdcp>;
>>>> +                        #power-domain-cells = <0>;
>>>> +                    };
>>>> +                };
>>>> +            };
>>>> +        };
>>>> +    };
>>>> diff --git a/Documentation/devicetree/bindings/soc/rockchip/power_domain.txt
>>>> b/Documentation/devicetree/bindings/soc/rockchip/power_domain.txt
>>>> deleted file mode 100644
>>>> index 8304eceb62e4..000000000000
>>>> --- a/Documentation/devicetree/bindings/soc/rockchip/power_domain.txt
>>>> +++ /dev/null
>>>> @@ -1,136 +0,0 @@
>>>> -* Rockchip Power Domains
>>>> -
>>>> -Rockchip processors include support for multiple power domains which can be
>>>> -powered up/down by software based on different application scenes to save
>>>> power.
>>>> -
>>>> -Required properties for power domain controller:
>>>> -- compatible: Should be one of the following.
>>>> -    "rockchip,px30-power-controller" - for PX30 SoCs.
>>>> -    "rockchip,rk3036-power-controller" - for RK3036 SoCs.
>>>> -    "rockchip,rk3066-power-controller" - for RK3066 SoCs.
>>>> -    "rockchip,rk3128-power-controller" - for RK3128 SoCs.
>>>> -    "rockchip,rk3188-power-controller" - for RK3188 SoCs.
>>>> -    "rockchip,rk3228-power-controller" - for RK3228 SoCs.
>>>> -    "rockchip,rk3288-power-controller" - for RK3288 SoCs.
>>>> -    "rockchip,rk3328-power-controller" - for RK3328 SoCs.
>>>> -    "rockchip,rk3366-power-controller" - for RK3366 SoCs.
>>>> -    "rockchip,rk3368-power-controller" - for RK3368 SoCs.
>>>> -    "rockchip,rk3399-power-controller" - for RK3399 SoCs.
>>>> -- #power-domain-cells: Number of cells in a power-domain specifier.
>>>> -    Should be 1 for multiple PM domains.
>>>> -- #address-cells: Should be 1.
>>>> -- #size-cells: Should be 0.
>>>> -
>>>> -Required properties for power domain sub nodes:
>>>> -- reg: index of the power domain, should use macros in:
>>>> -    "include/dt-bindings/power/px30-power.h" - for PX30 type power domain.
>>>> -    "include/dt-bindings/power/rk3036-power.h" - for RK3036 type power domain.
>>>> -    "include/dt-bindings/power/rk3066-power.h" - for RK3066 type power domain.
>>>> -    "include/dt-bindings/power/rk3128-power.h" - for RK3128 type power domain.
>>>> -    "include/dt-bindings/power/rk3188-power.h" - for RK3188 type power domain.
>>>> -    "include/dt-bindings/power/rk3228-power.h" - for RK3228 type power domain.
>>>> -    "include/dt-bindings/power/rk3288-power.h" - for RK3288 type power domain.
>>>> -    "include/dt-bindings/power/rk3328-power.h" - for RK3328 type power domain.
>>>> -    "include/dt-bindings/power/rk3366-power.h" - for RK3366 type power domain.
>>>> -    "include/dt-bindings/power/rk3368-power.h" - for RK3368 type power domain.
>>>> -    "include/dt-bindings/power/rk3399-power.h" - for RK3399 type power domain.
>>>> -- clocks (optional): phandles to clocks which need to be enabled while power
>>>> domain
>>>> -    switches state.
>>>> -- pm_qos (optional): phandles to qos blocks which need to be saved and restored
>>>> -    while power domain switches state.
>>>> -
>>>> -Qos Example:
>>>> -
>>>> -    qos_gpu: qos_gpu@ffaf0000 {
>>>> -        compatible ="syscon";
>>>> -        reg = <0x0 0xffaf0000 0x0 0x20>;
>>>> -    };
>>>> -
>>>> -Example:
>>>> -
>>>> -    power: power-controller {
>>>> -        compatible = "rockchip,rk3288-power-controller";
>>>> -        #power-domain-cells = <1>;
>>>> -        #address-cells = <1>;
>>>> -        #size-cells = <0>;
>>>> -
>>>> -        pd_gpu {
>>>> -            reg = <RK3288_PD_GPU>;
>>>> -            clocks = <&cru ACLK_GPU>;
>>>> -            pm_qos = <&qos_gpu>;
>>>> -        };
>>>> -    };
>>>> -
>>>> -     power: power-controller {
>>>> -                compatible = "rockchip,rk3368-power-controller";
>>>> -                #power-domain-cells = <1>;
>>>> -                #address-cells = <1>;
>>>> -                #size-cells = <0>;
>>>> -
>>>> -                pd_gpu_1 {
>>>> -                        reg = <RK3368_PD_GPU_1>;
>>>> -                        clocks = <&cru ACLK_GPU_CFG>;
>>>> -                };
>>>> -        };
>>>> -
>>>> -Example 2:
>>>> -        power: power-controller {
>>>> -            compatible = "rockchip,rk3399-power-controller";
>>>> -            #power-domain-cells = <1>;
>>>> -            #address-cells = <1>;
>>>> -            #size-cells = <0>;
>>>> -
>>>> -            pd_vio {
>>>> -                #address-cells = <1>;
>>>> -                #size-cells = <0>;
>>>> -                reg = <RK3399_PD_VIO>;
>>>> -
>>>> -                pd_vo {
>>>> -                    #address-cells = <1>;
>>>> -                    #size-cells = <0>;
>>>> -                    reg = <RK3399_PD_VO>;
>>>> -
>>>> -                    pd_vopb {
>>>> -                        reg = <RK3399_PD_VOPB>;
>>>> -                    };
>>>> -
>>>> -                    pd_vopl {
>>>> -                        reg = <RK3399_PD_VOPL>;
>>>> -                    };
>>>> -                };
>>>> -            };
>>>> -        };
>>>> -
>>>> -Node of a device using power domains must have a power-domains property,
>>>> -containing a phandle to the power device node and an index specifying which
>>>> -power domain to use.
>>>> -The index should use macros in:
>>>> -    "include/dt-bindings/power/px30-power.h" - for px30 type power domain.
>>>> -    "include/dt-bindings/power/rk3036-power.h" - for rk3036 type power domain.
>>>> -    "include/dt-bindings/power/rk3128-power.h" - for rk3128 type power domain.
>>>> -    "include/dt-bindings/power/rk3128-power.h" - for rk3228 type power domain.
>>>> -    "include/dt-bindings/power/rk3288-power.h" - for rk3288 type power domain.
>>>> -    "include/dt-bindings/power/rk3328-power.h" - for rk3328 type power domain.
>>>> -    "include/dt-bindings/power/rk3366-power.h" - for rk3366 type power domain.
>>>> -    "include/dt-bindings/power/rk3368-power.h" - for rk3368 type power domain.
>>>> -    "include/dt-bindings/power/rk3399-power.h" - for rk3399 type power domain.
>>>> -
>>>> -Example of the node using power domain:
>>>> -
>>>> -    node {
>>>> -        /* ... */
>>>> -        power-domains = <&power RK3288_PD_GPU>;
>>>> -        /* ... */
>>>> -    };
>>>> -
>>>> -    node {
>>>> -                /* ... */
>>>> -                power-domains = <&power RK3368_PD_GPU_1>;
>>>> -                /* ... */
>>>> -        };
>>>> -
>>>> -    node {
>>>> -        /* ... */
>>>> -        power-domains = <&power RK3399_PD_VOPB>;
>>>> -        /* ... */
>>>> -    };
>>>>
>>>
>>>
>>
>>
Johan Jonker March 24, 2021, 10:32 a.m. UTC | #7
Hi all,

Could Heiko and Rob advise if all these power-controller dtsi subnode
names should be changed?

Thanks!

On 3/24/21 11:14 AM, elaine.zhang wrote:
> Hi, Johan:
> 
> 在 2021/3/24 下午5:17, Johan Jonker 写道:
>> Hi Elaine,
>>
>> >From Rob's build log it turns out that 2 more properties must be added.
>> Add these new properties in separate patch.
>> Retest with commands below.
>> ====
>> See rk3288.dtsi
>>
>>             assigned-clocks = <&cru SCLK_EDP_24M>;
>>             assigned-clock-parents = <&xin24m>;
> This should not be in the power node.
> It should be on the CRU node, or on the EDP's own node.
> I could have added it just to solve dtbs_check .

If this should not be in the power node then could you add a patch
before the YAML conversion?

example:

- ARM: dts: Move assigned-clocks in rk3288.dtsi
- dt-bindings: power: rockchip: Convert to json-schema
- dt-bindings: power: rockchip: Add bindings for RK3568 Soc
- soc: rockchip: power-domain: add rk3568 powerdomains

When that regex must be changed then also change all dtsi files.... ;)

- ARM: dts: rk3288: change power-controller sub nodenames.
- etc...

>> ====
>> https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20210324071609.7531-3-zhangqing@rock-chips.com/
>>
>>
>> make ARCH=arm dtbs_check
>> DT_SCHEMA_FILES=Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
>>
>>
>> make ARCH=arm64 dtbs_check
>> DT_SCHEMA_FILES=Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
>>
>>
>> On 3/24/21 8:16 AM, Elaine Zhang wrote:
>>> Convert the soc/rockchip/power_domain.txt binding document to
>>> json-schema and move to the power bindings directory.
>>>
>>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>>> Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
>>> ---
>>>   .../power/rockchip,power-controller.yaml      | 284 ++++++++++++++++++
>>>   .../bindings/soc/rockchip/power_domain.txt    | 136 ---------
>>>   2 files changed, 284 insertions(+), 136 deletions(-)
>>>   create mode 100644
>>> Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
>>>   delete mode 100644
>>> Documentation/devicetree/bindings/soc/rockchip/power_domain.txt
>>>
>>> diff --git
>>> a/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
>>> b/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
>>> new file mode 100644
>>> index 000000000000..a220322c5139
>>> --- /dev/null
>>> +++
>>> b/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
>>> @@ -0,0 +1,284 @@
>>> +# SPDX-License-Identifier: GPL-2.0-only
>>> +%YAML 1.2
>>> +---
>>> +$id:
>>> http://devicetree.org/schemas/power/rockchip,power-controller.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Rockchip Power Domains
>>> +
>>> +maintainers:
>>> +  - Elaine Zhang <zhangqing@rock-chips.com>
>>> +  - Rob Herring <robh+dt@kernel.org>
>>> +  - Heiko Stuebner <heiko@sntech.de>
>>> +
>>> +description: |
>>> +  Rockchip processors include support for multiple power domains
>>> which can be
>>> +  powered up/down by software based on different application
>>> scenarios to save power.
>>> +
>>> +  Power domains contained within power-controller node are generic
>>> power domain
>>> +  providers documented in
>>> Documentation/devicetree/bindings/power/power-domain.yaml.
>>> +
>>> +  IP cores belonging to a power domain should contain a "power-domains"
>>> +  property that is a phandle for the power domain node representing
>>> the domain.
>>> +
>>> +properties:
>>> +  $nodename:
>>> +    const: power-controller
>>> +
>>> +  compatible:
>>> +    enum:
>>> +      - rockchip,px30-power-controller
>>> +      - rockchip,rk3036-power-controller
>>> +      - rockchip,rk3066-power-controller
>>> +      - rockchip,rk3128-power-controller
>>> +      - rockchip,rk3188-power-controller
>>> +      - rockchip,rk3228-power-controller
>>> +      - rockchip,rk3288-power-controller
>>> +      - rockchip,rk3328-power-controller
>>> +      - rockchip,rk3366-power-controller
>>> +      - rockchip,rk3368-power-controller
>>> +      - rockchip,rk3399-power-controller
>>> +
>>> +  "#power-domain-cells":
>>> +    const: 1
>>> +
>>> +  "#address-cells":
>>> +    const: 1
>>> +
>>> +  "#size-cells":
>>> +    const: 0
>>
>> assigned-clocks:
>>    maxItems: 1
>>
>> assigned-clock-parents:
>>    maxItems: 1
>>
>>> +
>>> +patternProperties:
>>> +  "^pd_[0-9a-z_]{2,10}@[0-9a-f]+$":
>>> +    type: object
>>> +    description: |
>>> +      Represents the power domains within the power controller node
>>> as documented
>>> +      in Documentation/devicetree/bindings/power/power-domain.yaml.
>>> +
>>> +    properties:
>>> +
>>> +      "#power-domain-cells":
>>> +        description:
>>> +          Must be 0 for nodes representing a single PM domain and 1
>>> for nodes
>>> +          providing multiple PM domains.
>>> +
>>> +      "#address-cells":
>>> +        const: 1
>>> +
>>> +      "#size-cells":
>>> +        const: 0
>>> +
>>> +      reg:
>>> +        maxItems: 1
>>> +        description: |
>>> +          Power domain index. Valid values are defined in
>>> +          "include/dt-bindings/power/px30-power.h"
>>> +          "include/dt-bindings/power/rk3036-power.h"
>>> +          "include/dt-bindings/power/rk3066-power.h"
>>> +          "include/dt-bindings/power/rk3128-power.h"
>>> +          "include/dt-bindings/power/rk3188-power.h"
>>> +          "include/dt-bindings/power/rk3228-power.h"
>>> +          "include/dt-bindings/power/rk3288-power.h"
>>> +          "include/dt-bindings/power/rk3328-power.h"
>>> +          "include/dt-bindings/power/rk3366-power.h"
>>> +          "include/dt-bindings/power/rk3368-power.h"
>>> +          "include/dt-bindings/power/rk3399-power.h"
>>> +
>>> +      clocks:
>>> +        description: |
>>> +          A number of phandles to clocks that need to be enabled
>>> while power domain
>>> +          switches state.
>>> +
>>> +      pm_qos:
>>> +        description: |
>>> +          A number of phandles to qos blocks which need to be saved
>>> and restored
>>> +          while power domain switches state.
>>> +
>>> +    patternProperties:
>>> +      "^pd_[0-9a-z_]{2,10}@[0-9a-f]+$":
>>> +        type: object
>>> +        description: |
>>> +          Represents a power domain child within a power domain
>>> parent node.
>>> +
>>> +        properties:
>>> +
>>> +          "#power-domain-cells":
>>> +            description:
>>> +              Must be 0 for nodes representing a single PM domain
>>> and 1 for nodes
>>> +              providing multiple PM domains.
>>> +
>>> +          "#address-cells":
>>> +            const: 1
>>> +
>>> +          "#size-cells":
>>> +            const: 0
>>> +
>>> +          reg:
>>> +            maxItems: 1
>>> +
>>> +          clocks:
>>> +            description: |
>>> +              A number of phandles to clocks that need to be enabled
>>> while power domain
>>> +              switches state.
>>> +
>>> +          pm_qos:
>>> +            description: |
>>> +              A number of phandles to qos blocks which need to be
>>> saved and restored
>>> +              while power domain switches state.
>>> +
>>> +        patternProperties:
>>> +          "^pd_[0-9a-z_]{2,10}@[0-9a-f]+$":
>>> +            type: object
>>> +            description: |
>>> +              Represents a power domain child within a power domain
>>> parent node.
>>> +
>>> +            properties:
>>> +
>>> +              "#power-domain-cells":
>>> +                description:
>>> +                  Must be 0 for nodes representing a single PM
>>> domain and 1 for nodes
>>> +                  providing multiple PM domains.
>>> +
>>> +              "#address-cells":
>>> +                const: 1
>>> +
>>> +              "#size-cells":
>>> +                const: 0
>>> +
>>> +              reg:
>>> +                maxItems: 1
>>> +
>>> +              clocks:
>>> +                description: |
>>> +                  A number of phandles to clocks that need to be
>>> enabled while power domain
>>> +                  switches state.
>>> +
>>> +              pm_qos:
>>> +                description: |
>>> +                  A number of phandles to qos blocks which need to
>>> be saved and restored
>>> +                  while power domain switches state.
>>> +
>>> +            required:
>>> +              - reg
>>> +
>>> +            additionalProperties: false
>>> +
>>> +        required:
>>> +          - reg
>>> +
>>> +        additionalProperties: false
>>> +
>>> +    required:
>>> +      - reg
>>> +
>>> +    additionalProperties: false
>>> +
>>> +required:
>>> +  - compatible
>>> +  - "#power-domain-cells"
>>> +
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> +  - |
>>> +    #include <dt-bindings/clock/rk3399-cru.h>
>>> +    #include <dt-bindings/power/rk3399-power.h>
>>> +
>>> +    soc {
>>> +        #address-cells = <2>;
>>> +        #size-cells = <2>;
>>> +
>>> +        qos_hdcp: qos@ffa90000 {
>>> +            compatible = "rockchip,rk3399-qos", "syscon";
>>> +            reg = <0x0 0xffa90000 0x0 0x20>;
>>> +        };
>>> +
>>> +        qos_iep: qos@ffa98000 {
>>> +            compatible = "rockchip,rk3399-qos", "syscon";
>>> +            reg = <0x0 0xffa98000 0x0 0x20>;
>>> +        };
>>> +
>>> +        qos_rga_r: qos@ffab0000 {
>>> +            compatible = "rockchip,rk3399-qos", "syscon";
>>> +            reg = <0x0 0xffab0000 0x0 0x20>;
>>> +        };
>>> +
>>> +        qos_rga_w: qos@ffab0080 {
>>> +            compatible = "rockchip,rk3399-qos", "syscon";
>>> +            reg = <0x0 0xffab0080 0x0 0x20>;
>>> +        };
>>> +
>>> +        qos_video_m0: qos@ffab8000 {
>>> +            compatible = "rockchip,rk3399-qos", "syscon";
>>> +            reg = <0x0 0xffab8000 0x0 0x20>;
>>> +        };
>>> +
>>> +        qos_video_m1_r: qos@ffac0000 {
>>> +            compatible = "rockchip,rk3399-qos", "syscon";
>>> +            reg = <0x0 0xffac0000 0x0 0x20>;
>>> +        };
>>> +
>>> +        qos_video_m1_w: qos@ffac0080 {
>>> +            compatible = "rockchip,rk3399-qos", "syscon";
>>> +            reg = <0x0 0xffac0080 0x0 0x20>;
>>> +        };
>>> +
>>> +        power-management@ff310000 {
>>> +            compatible = "rockchip,rk3399-pmu", "syscon", "simple-mfd";
>>> +            reg = <0x0 0xff310000 0x0 0x1000>;
>>> +
>>> +            power-controller {
>>> +                compatible = "rockchip,rk3399-power-controller";
>>> +                #power-domain-cells = <1>;
>>> +                #address-cells = <1>;
>>> +                #size-cells = <0>;
>>> +
>>> +                /* These power domains are grouped by VD_CENTER */
>>> +                pd_iep@RK3399_PD_IEP {
>>> +                    reg = <RK3399_PD_IEP>;
>>> +                    clocks = <&cru ACLK_IEP>,
>>> +                             <&cru HCLK_IEP>;
>>> +                    pm_qos = <&qos_iep>;
>>> +                    #power-domain-cells = <0>;
>>> +                };
>>> +                pd_rga@RK3399_PD_RGA {
>>> +                    reg = <RK3399_PD_RGA>;
>>> +                    clocks = <&cru ACLK_RGA>,
>>> +                             <&cru HCLK_RGA>;
>>> +                    pm_qos = <&qos_rga_r>,
>>> +                             <&qos_rga_w>;
>>> +                    #power-domain-cells = <0>;
>>> +                };
>>> +                pd_vcodec@RK3399_PD_VCODEC {
>>> +                    reg = <RK3399_PD_VCODEC>;
>>> +                    clocks = <&cru ACLK_VCODEC>,
>>> +                             <&cru HCLK_VCODEC>;
>>> +                    pm_qos = <&qos_video_m0>;
>>> +                    #power-domain-cells = <0>;
>>> +                };
>>> +                pd_vdu@RK3399_PD_VDU {
>>> +                    reg = <RK3399_PD_VDU>;
>>> +                    clocks = <&cru ACLK_VDU>,
>>> +                             <&cru HCLK_VDU>;
>>> +                    pm_qos = <&qos_video_m1_r>,
>>> +                             <&qos_video_m1_w>;
>>> +                    #power-domain-cells = <0>;
>>> +                };
>>> +                pd_vio@RK3399_PD_VIO {
>>> +                    reg = <RK3399_PD_VIO>;
>>> +                    #power-domain-cells = <1>;
>>> +                    #address-cells = <1>;
>>> +                    #size-cells = <0>;
>>> +
>>> +                    pd_hdcp@RK3399_PD_HDCP {
>>> +                        reg = <RK3399_PD_HDCP>;
>>> +                        clocks = <&cru ACLK_HDCP>,
>>> +                                 <&cru HCLK_HDCP>,
>>> +                                 <&cru PCLK_HDCP>;
>>> +                        pm_qos = <&qos_hdcp>;
>>> +                        #power-domain-cells = <0>;
>>> +                    };
>>> +                };
>>> +            };
>>> +        };
>>> +    };
>>> diff --git
>>> a/Documentation/devicetree/bindings/soc/rockchip/power_domain.txt
>>> b/Documentation/devicetree/bindings/soc/rockchip/power_domain.txt
>>> deleted file mode 100644
>>> index 8304eceb62e4..000000000000
>>> --- a/Documentation/devicetree/bindings/soc/rockchip/power_domain.txt
>>> +++ /dev/null
>>> @@ -1,136 +0,0 @@
>>> -* Rockchip Power Domains
>>> -
>>> -Rockchip processors include support for multiple power domains which
>>> can be
>>> -powered up/down by software based on different application scenes to
>>> save power.
>>> -
>>> -Required properties for power domain controller:
>>> -- compatible: Should be one of the following.
>>> -    "rockchip,px30-power-controller" - for PX30 SoCs.
>>> -    "rockchip,rk3036-power-controller" - for RK3036 SoCs.
>>> -    "rockchip,rk3066-power-controller" - for RK3066 SoCs.
>>> -    "rockchip,rk3128-power-controller" - for RK3128 SoCs.
>>> -    "rockchip,rk3188-power-controller" - for RK3188 SoCs.
>>> -    "rockchip,rk3228-power-controller" - for RK3228 SoCs.
>>> -    "rockchip,rk3288-power-controller" - for RK3288 SoCs.
>>> -    "rockchip,rk3328-power-controller" - for RK3328 SoCs.
>>> -    "rockchip,rk3366-power-controller" - for RK3366 SoCs.
>>> -    "rockchip,rk3368-power-controller" - for RK3368 SoCs.
>>> -    "rockchip,rk3399-power-controller" - for RK3399 SoCs.
>>> -- #power-domain-cells: Number of cells in a power-domain specifier.
>>> -    Should be 1 for multiple PM domains.
>>> -- #address-cells: Should be 1.
>>> -- #size-cells: Should be 0.
>>> -
>>> -Required properties for power domain sub nodes:
>>> -- reg: index of the power domain, should use macros in:
>>> -    "include/dt-bindings/power/px30-power.h" - for PX30 type power
>>> domain.
>>> -    "include/dt-bindings/power/rk3036-power.h" - for RK3036 type
>>> power domain.
>>> -    "include/dt-bindings/power/rk3066-power.h" - for RK3066 type
>>> power domain.
>>> -    "include/dt-bindings/power/rk3128-power.h" - for RK3128 type
>>> power domain.
>>> -    "include/dt-bindings/power/rk3188-power.h" - for RK3188 type
>>> power domain.
>>> -    "include/dt-bindings/power/rk3228-power.h" - for RK3228 type
>>> power domain.
>>> -    "include/dt-bindings/power/rk3288-power.h" - for RK3288 type
>>> power domain.
>>> -    "include/dt-bindings/power/rk3328-power.h" - for RK3328 type
>>> power domain.
>>> -    "include/dt-bindings/power/rk3366-power.h" - for RK3366 type
>>> power domain.
>>> -    "include/dt-bindings/power/rk3368-power.h" - for RK3368 type
>>> power domain.
>>> -    "include/dt-bindings/power/rk3399-power.h" - for RK3399 type
>>> power domain.
>>> -- clocks (optional): phandles to clocks which need to be enabled
>>> while power domain
>>> -    switches state.
>>> -- pm_qos (optional): phandles to qos blocks which need to be saved
>>> and restored
>>> -    while power domain switches state.
>>> -
>>> -Qos Example:
>>> -
>>> -    qos_gpu: qos_gpu@ffaf0000 {
>>> -        compatible ="syscon";
>>> -        reg = <0x0 0xffaf0000 0x0 0x20>;
>>> -    };
>>> -
>>> -Example:
>>> -
>>> -    power: power-controller {
>>> -        compatible = "rockchip,rk3288-power-controller";
>>> -        #power-domain-cells = <1>;
>>> -        #address-cells = <1>;
>>> -        #size-cells = <0>;
>>> -
>>> -        pd_gpu {
>>> -            reg = <RK3288_PD_GPU>;
>>> -            clocks = <&cru ACLK_GPU>;
>>> -            pm_qos = <&qos_gpu>;
>>> -        };
>>> -    };
>>> -
>>> -     power: power-controller {
>>> -                compatible = "rockchip,rk3368-power-controller";
>>> -                #power-domain-cells = <1>;
>>> -                #address-cells = <1>;
>>> -                #size-cells = <0>;
>>> -
>>> -                pd_gpu_1 {
>>> -                        reg = <RK3368_PD_GPU_1>;
>>> -                        clocks = <&cru ACLK_GPU_CFG>;
>>> -                };
>>> -        };
>>> -
>>> -Example 2:
>>> -        power: power-controller {
>>> -            compatible = "rockchip,rk3399-power-controller";
>>> -            #power-domain-cells = <1>;
>>> -            #address-cells = <1>;
>>> -            #size-cells = <0>;
>>> -
>>> -            pd_vio {
>>> -                #address-cells = <1>;
>>> -                #size-cells = <0>;
>>> -                reg = <RK3399_PD_VIO>;
>>> -
>>> -                pd_vo {
>>> -                    #address-cells = <1>;
>>> -                    #size-cells = <0>;
>>> -                    reg = <RK3399_PD_VO>;
>>> -
>>> -                    pd_vopb {
>>> -                        reg = <RK3399_PD_VOPB>;
>>> -                    };
>>> -
>>> -                    pd_vopl {
>>> -                        reg = <RK3399_PD_VOPL>;
>>> -                    };
>>> -                };
>>> -            };
>>> -        };
>>> -
>>> -Node of a device using power domains must have a power-domains
>>> property,
>>> -containing a phandle to the power device node and an index
>>> specifying which
>>> -power domain to use.
>>> -The index should use macros in:
>>> -    "include/dt-bindings/power/px30-power.h" - for px30 type power
>>> domain.
>>> -    "include/dt-bindings/power/rk3036-power.h" - for rk3036 type
>>> power domain.
>>> -    "include/dt-bindings/power/rk3128-power.h" - for rk3128 type
>>> power domain.
>>> -    "include/dt-bindings/power/rk3128-power.h" - for rk3228 type
>>> power domain.
>>> -    "include/dt-bindings/power/rk3288-power.h" - for rk3288 type
>>> power domain.
>>> -    "include/dt-bindings/power/rk3328-power.h" - for rk3328 type
>>> power domain.
>>> -    "include/dt-bindings/power/rk3366-power.h" - for rk3366 type
>>> power domain.
>>> -    "include/dt-bindings/power/rk3368-power.h" - for rk3368 type
>>> power domain.
>>> -    "include/dt-bindings/power/rk3399-power.h" - for rk3399 type
>>> power domain.
>>> -
>>> -Example of the node using power domain:
>>> -
>>> -    node {
>>> -        /* ... */
>>> -        power-domains = <&power RK3288_PD_GPU>;
>>> -        /* ... */
>>> -    };
>>> -
>>> -    node {
>>> -                /* ... */
>>> -                power-domains = <&power RK3368_PD_GPU_1>;
>>> -                /* ... */
>>> -        };
>>> -
>>> -    node {
>>> -        /* ... */
>>> -        power-domains = <&power RK3399_PD_VOPB>;
>>> -        /* ... */
>>> -    };
>>>
>>
>>
> 
>
Heiko Stuebner March 24, 2021, 1:31 p.m. UTC | #8
Am Mittwoch, 24. März 2021, 11:32:42 CET schrieb Enric Balletbo i Serra:
> 
> On 24/3/21 11:25, Enric Balletbo i Serra wrote:
> > Hi Elaine,
> > 
> > On 24/3/21 11:18, elaine.zhang wrote:
> >> Hi,  Enric
> >>
> >> 在 2021/3/24 下午5:56, Enric Balletbo i Serra 写道:
> >>> Hi Elaine,
> >>>
> >>> This is not the exact version I sent, and you reintroduced a "problem" that were
> >>> already solved/discussed on previous versions. See below:
> >>>
> >>> On 24/3/21 8:16, Elaine Zhang wrote:
> >>>> Convert the soc/rockchip/power_domain.txt binding document to
> >>>> json-schema and move to the power bindings directory.
> >>>>
> >>>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> >>> If you do significant is a good practice shortly describe them within [] here.
> >>>
> >>>> Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
> >>> Note that my last version already had the
> >>>
> >>> Reviewed-by: Rob Herring <robh@kernel.org>
> >>>
> >>> Which should be fine for merging (with probably only minor changes) and you
> >>> could maintain if you don't do significant changes, but that's not the case, as
> >>> I said, you are reintroducing one problem. Please review the comments already
> >>> received on this patchset or similar patchsets to avoid this.
> >>>
> >>>> ---
> >>>>   .../power/rockchip,power-controller.yaml      | 284 ++++++++++++++++++
> >>>>   .../bindings/soc/rockchip/power_domain.txt    | 136 ---------
> >>>>   2 files changed, 284 insertions(+), 136 deletions(-)
> >>>>   create mode 100644
> >>>> Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
> >>>>   delete mode 100644
> >>>> Documentation/devicetree/bindings/soc/rockchip/power_domain.txt
> >>>>
> >>>> diff --git
> >>>> a/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
> >>>> b/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
> >>>> new file mode 100644
> >>>> index 000000000000..a220322c5139
> >>>> --- /dev/null
> >>>> +++ b/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
> >>>> @@ -0,0 +1,284 @@
> >>>> +# SPDX-License-Identifier: GPL-2.0-only
> >>>> +%YAML 1.2
> >>>> +---
> >>>> +$id: http://devicetree.org/schemas/power/rockchip,power-controller.yaml#
> >>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >>>> +
> >>>> +title: Rockchip Power Domains
> >>>> +
> >>>> +maintainers:
> >>>> +  - Elaine Zhang <zhangqing@rock-chips.com>
> >>>> +  - Rob Herring <robh+dt@kernel.org>
> >>> Up to Rob, but I don't think Rob would like to be the maintainer. I think you
> >>> can only include yourself and Heiko.
> >>>
> >>>
> >>>> +  - Heiko Stuebner <heiko@sntech.de>
> >>>> +
> >>>> +description: |
> >>>> +  Rockchip processors include support for multiple power domains which can be
> >>>> +  powered up/down by software based on different application scenarios to
> >>>> save power.
> >>>> +
> >>>> +  Power domains contained within power-controller node are generic power domain
> >>>> +  providers documented in
> >>>> Documentation/devicetree/bindings/power/power-domain.yaml.
> >>>> +
> >>>> +  IP cores belonging to a power domain should contain a "power-domains"
> >>>> +  property that is a phandle for the power domain node representing the domain.
> >>>> +
> >>>> +properties:
> >>>> +  $nodename:
> >>>> +    const: power-controller
> >>>> +
> >>>> +  compatible:
> >>>> +    enum:
> >>>> +      - rockchip,px30-power-controller
> >>>> +      - rockchip,rk3036-power-controller
> >>>> +      - rockchip,rk3066-power-controller
> >>>> +      - rockchip,rk3128-power-controller
> >>>> +      - rockchip,rk3188-power-controller
> >>>> +      - rockchip,rk3228-power-controller
> >>>> +      - rockchip,rk3288-power-controller
> >>>> +      - rockchip,rk3328-power-controller
> >>>> +      - rockchip,rk3366-power-controller
> >>>> +      - rockchip,rk3368-power-controller
> >>>> +      - rockchip,rk3399-power-controller
> >>>> +
> >>>> +  "#power-domain-cells":
> >>>> +    const: 1
> >>>> +
> >>>> +  "#address-cells":
> >>>> +    const: 1
> >>>> +
> >>>> +  "#size-cells":
> >>>> +    const: 0
> >>>> +
> >>>> +patternProperties:
> >>>> +  "^pd_[0-9a-z_]{2,10}@[0-9a-f]+$":
> >>>> +    type: object
> >>>> +    description: |
> >>>> +      Represents the power domains within the power controller node as
> >>>> documented
> >>>> +      in Documentation/devicetree/bindings/power/power-domain.yaml.
> >>>> +
> >>> The node names must be generic, as this is power-domain must be in the form:
> >>>
> >>> +patternProperties:
> >>> +  "^power-domain@[0-9a-f]+$":
> >> In this way, dtbs_check cannot be passed, and all the usage methods in dts of
> >> Rockchip need to be corrected, which I think is a bigger change.
> > 
> > Well, the problem is in the Rockchip dtbs, so needs to be fixed there. The
> > bindings must describe hardware in a generic way, not describe the actual dtbs
> > to not report errors.
> > 
> 
> FWIW I remember I did something regarding this but never sent to upstream, feel
> free to pick if you find useful.
> 
> *
> https://gitlab.collabora.com/eballetbo/linux/-/commit/12499f223e3d33602449b9102404fe573fb804f5
> *
> https://gitlab.collabora.com/eballetbo/linux/-/commit/12499f223e3d33602449b9102404fe573fb804f5
> *
> https://gitlab.collabora.com/eballetbo/linux/-/commit/492bf2213c341152a1c2423242c5634b9e53ff27

looks good that way. I did look at the power-domain driver and
we're (of course) not doing anything with the name in front of the @ :-) .

So I'd be happy to get these.

Heiko
Elaine Zhang March 25, 2021, 6:41 a.m. UTC | #9
Hi,Heiko:

在 2021/3/24 下午9:31, Heiko Stübner 写道:
> Am Mittwoch, 24. März 2021, 11:32:42 CET schrieb Enric Balletbo i Serra:
>> On 24/3/21 11:25, Enric Balletbo i Serra wrote:
>>> Hi Elaine,
>>>
>>> On 24/3/21 11:18, elaine.zhang wrote:
>>>> Hi,  Enric
>>>>
>>>> 在 2021/3/24 下午5:56, Enric Balletbo i Serra 写道:
>>>>> Hi Elaine,
>>>>>
>>>>> This is not the exact version I sent, and you reintroduced a "problem" that were
>>>>> already solved/discussed on previous versions. See below:
>>>>>
>>>>> On 24/3/21 8:16, Elaine Zhang wrote:
>>>>>> Convert the soc/rockchip/power_domain.txt binding document to
>>>>>> json-schema and move to the power bindings directory.
>>>>>>
>>>>>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>>>>> If you do significant is a good practice shortly describe them within [] here.
>>>>>
>>>>>> Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
>>>>> Note that my last version already had the
>>>>>
>>>>> Reviewed-by: Rob Herring <robh@kernel.org>
>>>>>
>>>>> Which should be fine for merging (with probably only minor changes) and you
>>>>> could maintain if you don't do significant changes, but that's not the case, as
>>>>> I said, you are reintroducing one problem. Please review the comments already
>>>>> received on this patchset or similar patchsets to avoid this.
>>>>>
>>>>>> ---
>>>>>>    .../power/rockchip,power-controller.yaml      | 284 ++++++++++++++++++
>>>>>>    .../bindings/soc/rockchip/power_domain.txt    | 136 ---------
>>>>>>    2 files changed, 284 insertions(+), 136 deletions(-)
>>>>>>    create mode 100644
>>>>>> Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
>>>>>>    delete mode 100644
>>>>>> Documentation/devicetree/bindings/soc/rockchip/power_domain.txt
>>>>>>
>>>>>> diff --git
>>>>>> a/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
>>>>>> b/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
>>>>>> new file mode 100644
>>>>>> index 000000000000..a220322c5139
>>>>>> --- /dev/null
>>>>>> +++ b/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
>>>>>> @@ -0,0 +1,284 @@
>>>>>> +# SPDX-License-Identifier: GPL-2.0-only
>>>>>> +%YAML 1.2
>>>>>> +---
>>>>>> +$id: http://devicetree.org/schemas/power/rockchip,power-controller.yaml#
>>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>>> +
>>>>>> +title: Rockchip Power Domains
>>>>>> +
>>>>>> +maintainers:
>>>>>> +  - Elaine Zhang <zhangqing@rock-chips.com>
>>>>>> +  - Rob Herring <robh+dt@kernel.org>
>>>>> Up to Rob, but I don't think Rob would like to be the maintainer. I think you
>>>>> can only include yourself and Heiko.
>>>>>
>>>>>
>>>>>> +  - Heiko Stuebner <heiko@sntech.de>
>>>>>> +
>>>>>> +description: |
>>>>>> +  Rockchip processors include support for multiple power domains which can be
>>>>>> +  powered up/down by software based on different application scenarios to
>>>>>> save power.
>>>>>> +
>>>>>> +  Power domains contained within power-controller node are generic power domain
>>>>>> +  providers documented in
>>>>>> Documentation/devicetree/bindings/power/power-domain.yaml.
>>>>>> +
>>>>>> +  IP cores belonging to a power domain should contain a "power-domains"
>>>>>> +  property that is a phandle for the power domain node representing the domain.
>>>>>> +
>>>>>> +properties:
>>>>>> +  $nodename:
>>>>>> +    const: power-controller
>>>>>> +
>>>>>> +  compatible:
>>>>>> +    enum:
>>>>>> +      - rockchip,px30-power-controller
>>>>>> +      - rockchip,rk3036-power-controller
>>>>>> +      - rockchip,rk3066-power-controller
>>>>>> +      - rockchip,rk3128-power-controller
>>>>>> +      - rockchip,rk3188-power-controller
>>>>>> +      - rockchip,rk3228-power-controller
>>>>>> +      - rockchip,rk3288-power-controller
>>>>>> +      - rockchip,rk3328-power-controller
>>>>>> +      - rockchip,rk3366-power-controller
>>>>>> +      - rockchip,rk3368-power-controller
>>>>>> +      - rockchip,rk3399-power-controller
>>>>>> +
>>>>>> +  "#power-domain-cells":
>>>>>> +    const: 1
>>>>>> +
>>>>>> +  "#address-cells":
>>>>>> +    const: 1
>>>>>> +
>>>>>> +  "#size-cells":
>>>>>> +    const: 0
>>>>>> +
>>>>>> +patternProperties:
>>>>>> +  "^pd_[0-9a-z_]{2,10}@[0-9a-f]+$":
>>>>>> +    type: object
>>>>>> +    description: |
>>>>>> +      Represents the power domains within the power controller node as
>>>>>> documented
>>>>>> +      in Documentation/devicetree/bindings/power/power-domain.yaml.
>>>>>> +
>>>>> The node names must be generic, as this is power-domain must be in the form:
>>>>>
>>>>> +patternProperties:
>>>>> +  "^power-domain@[0-9a-f]+$":
>>>> In this way, dtbs_check cannot be passed, and all the usage methods in dts of
>>>> Rockchip need to be corrected, which I think is a bigger change.
>>> Well, the problem is in the Rockchip dtbs, so needs to be fixed there. The
>>> bindings must describe hardware in a generic way, not describe the actual dtbs
>>> to not report errors.
>>>
>> FWIW I remember I did something regarding this but never sent to upstream, feel
>> free to pick if you find useful.
>>
>> *
>> https://gitlab.collabora.com/eballetbo/linux/-/commit/12499f223e3d33602449b9102404fe573fb804f5
>> *
>> https://gitlab.collabora.com/eballetbo/linux/-/commit/12499f223e3d33602449b9102404fe573fb804f5
>> *
>> https://gitlab.collabora.com/eballetbo/linux/-/commit/492bf2213c341152a1c2423242c5634b9e53ff27
> looks good that way. I did look at the power-domain driver and
> we're (of course) not doing anything with the name in front of the @ :-) .
>
> So I'd be happy to get these.
>
> Heiko

I still don't want to modify this,  I think the PD summary will become 
less intuitive.

Now:

domain                          status          slaves
     /device                                             runtime status
----------------------------------------------------------------------
pd_pipe                         on
     /devices/platform/3c0800000.pcie unsupported
     /devices/platform/fc800000.sata                     active
     /devices/platform/usbdrd/fcc00000.dwc3              active
     /devices/platform/usbhost/fd000000.dwc3             active
pd_rkvenc                       off-0
     /devices/platform/fdf40f00.iommu                    suspended
     /devices/platform/fdf40000.rkvenc                   suspended
pd_rkvdec                       off-0
     /devices/platform/fdf80800.iommu                    suspended
     /devices/platform/fdf80200.rkvdec                   suspended
pd_vpu                          off-0
     /devices/platform/fdea0800.iommu                    suspended
     /devices/platform/fdea0400.vdpu                     suspended
pd_rga                          off-0
     /devices/platform/fded0480.iommu                    suspended
     /devices/platform/fdee0800.iommu                    suspended
     /devices/platform/fdef0800.iommu                    suspended
     /devices/platform/fdee0000.vepu                     suspended
     /devices/platform/fdef0000.iep                      suspended
     /devices/platform/fded0000.jpegd                    suspended
     /devices/platform/fdeb0000.rk_rga                   suspended
pd_vo                           on
     /devices/platform/fe850000.mipi-dphy                suspended
     /devices/platform/fe040000.vop                      active
     /devices/platform/fe0c0000.edp                      suspended
     /devices/platform/fe0a0000.hdmi                     active
     /devices/platform/fe060000.dsi                      active
pd_vi                           off-0
     /devices/platform/fdff1a00.iommu                    suspended
     /devices/platform/fdff0000.rkisp                    suspended
pd_gpu                          on
     /devices/platform/fde60000.gpu                      active
pd_npu                          off-0

If modify to power-domain@:

domain                          status          slaves
     /device                                             runtime status
----------------------------------------------------------------------
power-domain                    on
     /devices/platform/3c0800000.pcie unsupported
     /devices/platform/fc800000.sata                     active
     /devices/platform/usbdrd/fcc00000.dwc3              active
     /devices/platform/usbhost/fd000000.dwc3             active
power-domain                    off-0
     /devices/platform/fdf40f00.iommu                    suspended
     /devices/platform/fdf40000.rkvenc                   suspended
power-domain                    off-0
     /devices/platform/fdf80800.iommu                    suspended
     /devices/platform/fdf80200.rkvdec                   suspended
power-domain                    off-0
     /devices/platform/fdea0800.iommu                    suspended
     /devices/platform/fdea0400.vdpu                     suspended
power-domain                    off-0
     /devices/platform/fded0480.iommu                    suspended
     /devices/platform/fdee0800.iommu                    suspended
     /devices/platform/fdef0800.iommu                    suspended
     /devices/platform/fdee0000.vepu                     suspended
     /devices/platform/fdef0000.iep                      suspended
     /devices/platform/fded0000.jpegd                    suspended
     /devices/platform/fdeb0000.rk_rga                   suspended
power-domain                    on
     /devices/platform/fe850000.mipi-dphy                suspended
     /devices/platform/fe040000.vop                      active
     /devices/platform/fe0c0000.edp                      suspended
     /devices/platform/fe0a0000.hdmi                     active
     /devices/platform/fe060000.dsi                      active
power-domain                    off-0
     /devices/platform/fdff1a00.iommu                    suspended
     /devices/platform/fdff0000.rkisp                    suspended
power-domain                    on
     /devices/platform/fde60000.gpu                      active
power-domain                    off-0
console:/ #

I think the now one is more intuitive, so I don't want to fix it.

>
>
>
>
Enric Balletbo Serra March 25, 2021, 11:16 a.m. UTC | #10
Hi Elaine,

Missatge de elaine.zhang <zhangqing@rock-chips.com> del dia dj., 25 de
març 2021 a les 7:43:
>
> Hi,Heiko:
>
> 在 2021/3/24 下午9:31, Heiko Stübner 写道:
> > Am Mittwoch, 24. März 2021, 11:32:42 CET schrieb Enric Balletbo i Serra:
> >> On 24/3/21 11:25, Enric Balletbo i Serra wrote:
> >>> Hi Elaine,
> >>>
> >>> On 24/3/21 11:18, elaine.zhang wrote:
> >>>> Hi,  Enric
> >>>>
> >>>> 在 2021/3/24 下午5:56, Enric Balletbo i Serra 写道:
> >>>>> Hi Elaine,
> >>>>>
> >>>>> This is not the exact version I sent, and you reintroduced a "problem" that were
> >>>>> already solved/discussed on previous versions. See below:
> >>>>>
> >>>>> On 24/3/21 8:16, Elaine Zhang wrote:
> >>>>>> Convert the soc/rockchip/power_domain.txt binding document to
> >>>>>> json-schema and move to the power bindings directory.
> >>>>>>
> >>>>>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> >>>>> If you do significant is a good practice shortly describe them within [] here.
> >>>>>
> >>>>>> Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
> >>>>> Note that my last version already had the
> >>>>>
> >>>>> Reviewed-by: Rob Herring <robh@kernel.org>
> >>>>>
> >>>>> Which should be fine for merging (with probably only minor changes) and you
> >>>>> could maintain if you don't do significant changes, but that's not the case, as
> >>>>> I said, you are reintroducing one problem. Please review the comments already
> >>>>> received on this patchset or similar patchsets to avoid this.
> >>>>>
> >>>>>> ---
> >>>>>>    .../power/rockchip,power-controller.yaml      | 284 ++++++++++++++++++
> >>>>>>    .../bindings/soc/rockchip/power_domain.txt    | 136 ---------
> >>>>>>    2 files changed, 284 insertions(+), 136 deletions(-)
> >>>>>>    create mode 100644
> >>>>>> Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
> >>>>>>    delete mode 100644
> >>>>>> Documentation/devicetree/bindings/soc/rockchip/power_domain.txt
> >>>>>>
> >>>>>> diff --git
> >>>>>> a/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
> >>>>>> b/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
> >>>>>> new file mode 100644
> >>>>>> index 000000000000..a220322c5139
> >>>>>> --- /dev/null
> >>>>>> +++ b/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
> >>>>>> @@ -0,0 +1,284 @@
> >>>>>> +# SPDX-License-Identifier: GPL-2.0-only
> >>>>>> +%YAML 1.2
> >>>>>> +---
> >>>>>> +$id: http://devicetree.org/schemas/power/rockchip,power-controller.yaml#
> >>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >>>>>> +
> >>>>>> +title: Rockchip Power Domains
> >>>>>> +
> >>>>>> +maintainers:
> >>>>>> +  - Elaine Zhang <zhangqing@rock-chips.com>
> >>>>>> +  - Rob Herring <robh+dt@kernel.org>
> >>>>> Up to Rob, but I don't think Rob would like to be the maintainer. I think you
> >>>>> can only include yourself and Heiko.
> >>>>>
> >>>>>
> >>>>>> +  - Heiko Stuebner <heiko@sntech.de>
> >>>>>> +
> >>>>>> +description: |
> >>>>>> +  Rockchip processors include support for multiple power domains which can be
> >>>>>> +  powered up/down by software based on different application scenarios to
> >>>>>> save power.
> >>>>>> +
> >>>>>> +  Power domains contained within power-controller node are generic power domain
> >>>>>> +  providers documented in
> >>>>>> Documentation/devicetree/bindings/power/power-domain.yaml.
> >>>>>> +
> >>>>>> +  IP cores belonging to a power domain should contain a "power-domains"
> >>>>>> +  property that is a phandle for the power domain node representing the domain.
> >>>>>> +
> >>>>>> +properties:
> >>>>>> +  $nodename:
> >>>>>> +    const: power-controller
> >>>>>> +
> >>>>>> +  compatible:
> >>>>>> +    enum:
> >>>>>> +      - rockchip,px30-power-controller
> >>>>>> +      - rockchip,rk3036-power-controller
> >>>>>> +      - rockchip,rk3066-power-controller
> >>>>>> +      - rockchip,rk3128-power-controller
> >>>>>> +      - rockchip,rk3188-power-controller
> >>>>>> +      - rockchip,rk3228-power-controller
> >>>>>> +      - rockchip,rk3288-power-controller
> >>>>>> +      - rockchip,rk3328-power-controller
> >>>>>> +      - rockchip,rk3366-power-controller
> >>>>>> +      - rockchip,rk3368-power-controller
> >>>>>> +      - rockchip,rk3399-power-controller
> >>>>>> +
> >>>>>> +  "#power-domain-cells":
> >>>>>> +    const: 1
> >>>>>> +
> >>>>>> +  "#address-cells":
> >>>>>> +    const: 1
> >>>>>> +
> >>>>>> +  "#size-cells":
> >>>>>> +    const: 0
> >>>>>> +
> >>>>>> +patternProperties:
> >>>>>> +  "^pd_[0-9a-z_]{2,10}@[0-9a-f]+$":
> >>>>>> +    type: object
> >>>>>> +    description: |
> >>>>>> +      Represents the power domains within the power controller node as
> >>>>>> documented
> >>>>>> +      in Documentation/devicetree/bindings/power/power-domain.yaml.
> >>>>>> +
> >>>>> The node names must be generic, as this is power-domain must be in the form:
> >>>>>
> >>>>> +patternProperties:
> >>>>> +  "^power-domain@[0-9a-f]+$":
> >>>> In this way, dtbs_check cannot be passed, and all the usage methods in dts of
> >>>> Rockchip need to be corrected, which I think is a bigger change.
> >>> Well, the problem is in the Rockchip dtbs, so needs to be fixed there. The
> >>> bindings must describe hardware in a generic way, not describe the actual dtbs
> >>> to not report errors.
> >>>
> >> FWIW I remember I did something regarding this but never sent to upstream, feel
> >> free to pick if you find useful.
> >>
> >> *
> >> https://gitlab.collabora.com/eballetbo/linux/-/commit/12499f223e3d33602449b9102404fe573fb804f5
> >> *
> >> https://gitlab.collabora.com/eballetbo/linux/-/commit/12499f223e3d33602449b9102404fe573fb804f5
> >> *
> >> https://gitlab.collabora.com/eballetbo/linux/-/commit/492bf2213c341152a1c2423242c5634b9e53ff27
> > looks good that way. I did look at the power-domain driver and
> > we're (of course) not doing anything with the name in front of the @ :-) .
> >
> > So I'd be happy to get these.
> >
> > Heiko
>
> I still don't want to modify this,  I think the PD summary will become
> less intuitive.
>

This is because in the pm_domainss driver you have:

    pd->genpd.name = node->name;

You can use the full_name or add a meaningful name to the driver if
you want. We solved this on Mediatek with the following patches [1]

Again, IMO this is a driver problem, not a binding problem. The
bindings shouldn't know about debugfs entry names.

Thanks,
  Enric

[1] https://patchwork.kernel.org/project/linux-mediatek/list/?series=438523

> Now:
>
> domain                          status          slaves
>      /device                                             runtime status
> ----------------------------------------------------------------------
> pd_pipe                         on
>      /devices/platform/3c0800000.pcie unsupported
>      /devices/platform/fc800000.sata                     active
>      /devices/platform/usbdrd/fcc00000.dwc3              active
>      /devices/platform/usbhost/fd000000.dwc3             active
> pd_rkvenc                       off-0
>      /devices/platform/fdf40f00.iommu                    suspended
>      /devices/platform/fdf40000.rkvenc                   suspended
> pd_rkvdec                       off-0
>      /devices/platform/fdf80800.iommu                    suspended
>      /devices/platform/fdf80200.rkvdec                   suspended
> pd_vpu                          off-0
>      /devices/platform/fdea0800.iommu                    suspended
>      /devices/platform/fdea0400.vdpu                     suspended
> pd_rga                          off-0
>      /devices/platform/fded0480.iommu                    suspended
>      /devices/platform/fdee0800.iommu                    suspended
>      /devices/platform/fdef0800.iommu                    suspended
>      /devices/platform/fdee0000.vepu                     suspended
>      /devices/platform/fdef0000.iep                      suspended
>      /devices/platform/fded0000.jpegd                    suspended
>      /devices/platform/fdeb0000.rk_rga                   suspended
> pd_vo                           on
>      /devices/platform/fe850000.mipi-dphy                suspended
>      /devices/platform/fe040000.vop                      active
>      /devices/platform/fe0c0000.edp                      suspended
>      /devices/platform/fe0a0000.hdmi                     active
>      /devices/platform/fe060000.dsi                      active
> pd_vi                           off-0
>      /devices/platform/fdff1a00.iommu                    suspended
>      /devices/platform/fdff0000.rkisp                    suspended
> pd_gpu                          on
>      /devices/platform/fde60000.gpu                      active
> pd_npu                          off-0
>
> If modify to power-domain@:
>
> domain                          status          slaves
>      /device                                             runtime status
> ----------------------------------------------------------------------
> power-domain                    on
>      /devices/platform/3c0800000.pcie unsupported
>      /devices/platform/fc800000.sata                     active
>      /devices/platform/usbdrd/fcc00000.dwc3              active
>      /devices/platform/usbhost/fd000000.dwc3             active
> power-domain                    off-0
>      /devices/platform/fdf40f00.iommu                    suspended
>      /devices/platform/fdf40000.rkvenc                   suspended
> power-domain                    off-0
>      /devices/platform/fdf80800.iommu                    suspended
>      /devices/platform/fdf80200.rkvdec                   suspended
> power-domain                    off-0
>      /devices/platform/fdea0800.iommu                    suspended
>      /devices/platform/fdea0400.vdpu                     suspended
> power-domain                    off-0
>      /devices/platform/fded0480.iommu                    suspended
>      /devices/platform/fdee0800.iommu                    suspended
>      /devices/platform/fdef0800.iommu                    suspended
>      /devices/platform/fdee0000.vepu                     suspended
>      /devices/platform/fdef0000.iep                      suspended
>      /devices/platform/fded0000.jpegd                    suspended
>      /devices/platform/fdeb0000.rk_rga                   suspended
> power-domain                    on
>      /devices/platform/fe850000.mipi-dphy                suspended
>      /devices/platform/fe040000.vop                      active
>      /devices/platform/fe0c0000.edp                      suspended
>      /devices/platform/fe0a0000.hdmi                     active
>      /devices/platform/fe060000.dsi                      active
> power-domain                    off-0
>      /devices/platform/fdff1a00.iommu                    suspended
>      /devices/platform/fdff0000.rkisp                    suspended
> power-domain                    on
>      /devices/platform/fde60000.gpu                      active
> power-domain                    off-0
> console:/ #
>
> I think the now one is more intuitive, so I don't want to fix it.
>
> >
> >
> >
> >
>
>
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml b/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
new file mode 100644
index 000000000000..a220322c5139
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
@@ -0,0 +1,284 @@ 
+# SPDX-License-Identifier: GPL-2.0-only
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/power/rockchip,power-controller.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Rockchip Power Domains
+
+maintainers:
+  - Elaine Zhang <zhangqing@rock-chips.com>
+  - Rob Herring <robh+dt@kernel.org>
+  - Heiko Stuebner <heiko@sntech.de>
+
+description: |
+  Rockchip processors include support for multiple power domains which can be
+  powered up/down by software based on different application scenarios to save power.
+
+  Power domains contained within power-controller node are generic power domain
+  providers documented in Documentation/devicetree/bindings/power/power-domain.yaml.
+
+  IP cores belonging to a power domain should contain a "power-domains"
+  property that is a phandle for the power domain node representing the domain.
+
+properties:
+  $nodename:
+    const: power-controller
+
+  compatible:
+    enum:
+      - rockchip,px30-power-controller
+      - rockchip,rk3036-power-controller
+      - rockchip,rk3066-power-controller
+      - rockchip,rk3128-power-controller
+      - rockchip,rk3188-power-controller
+      - rockchip,rk3228-power-controller
+      - rockchip,rk3288-power-controller
+      - rockchip,rk3328-power-controller
+      - rockchip,rk3366-power-controller
+      - rockchip,rk3368-power-controller
+      - rockchip,rk3399-power-controller
+
+  "#power-domain-cells":
+    const: 1
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
+patternProperties:
+  "^pd_[0-9a-z_]{2,10}@[0-9a-f]+$":
+    type: object
+    description: |
+      Represents the power domains within the power controller node as documented
+      in Documentation/devicetree/bindings/power/power-domain.yaml.
+
+    properties:
+
+      "#power-domain-cells":
+        description:
+          Must be 0 for nodes representing a single PM domain and 1 for nodes
+          providing multiple PM domains.
+
+      "#address-cells":
+        const: 1
+
+      "#size-cells":
+        const: 0
+
+      reg:
+        maxItems: 1
+        description: |
+          Power domain index. Valid values are defined in
+          "include/dt-bindings/power/px30-power.h"
+          "include/dt-bindings/power/rk3036-power.h"
+          "include/dt-bindings/power/rk3066-power.h"
+          "include/dt-bindings/power/rk3128-power.h"
+          "include/dt-bindings/power/rk3188-power.h"
+          "include/dt-bindings/power/rk3228-power.h"
+          "include/dt-bindings/power/rk3288-power.h"
+          "include/dt-bindings/power/rk3328-power.h"
+          "include/dt-bindings/power/rk3366-power.h"
+          "include/dt-bindings/power/rk3368-power.h"
+          "include/dt-bindings/power/rk3399-power.h"
+
+      clocks:
+        description: |
+          A number of phandles to clocks that need to be enabled while power domain
+          switches state.
+
+      pm_qos:
+        description: |
+          A number of phandles to qos blocks which need to be saved and restored
+          while power domain switches state.
+
+    patternProperties:
+      "^pd_[0-9a-z_]{2,10}@[0-9a-f]+$":
+        type: object
+        description: |
+          Represents a power domain child within a power domain parent node.
+
+        properties:
+
+          "#power-domain-cells":
+            description:
+              Must be 0 for nodes representing a single PM domain and 1 for nodes
+              providing multiple PM domains.
+
+          "#address-cells":
+            const: 1
+
+          "#size-cells":
+            const: 0
+
+          reg:
+            maxItems: 1
+
+          clocks:
+            description: |
+              A number of phandles to clocks that need to be enabled while power domain
+              switches state.
+
+          pm_qos:
+            description: |
+              A number of phandles to qos blocks which need to be saved and restored
+              while power domain switches state.
+
+        patternProperties:
+          "^pd_[0-9a-z_]{2,10}@[0-9a-f]+$":
+            type: object
+            description: |
+              Represents a power domain child within a power domain parent node.
+
+            properties:
+
+              "#power-domain-cells":
+                description:
+                  Must be 0 for nodes representing a single PM domain and 1 for nodes
+                  providing multiple PM domains.
+
+              "#address-cells":
+                const: 1
+
+              "#size-cells":
+                const: 0
+
+              reg:
+                maxItems: 1
+
+              clocks:
+                description: |
+                  A number of phandles to clocks that need to be enabled while power domain
+                  switches state.
+
+              pm_qos:
+                description: |
+                  A number of phandles to qos blocks which need to be saved and restored
+                  while power domain switches state.
+
+            required:
+              - reg
+
+            additionalProperties: false
+
+        required:
+          - reg
+
+        additionalProperties: false
+
+    required:
+      - reg
+
+    additionalProperties: false
+
+required:
+  - compatible
+  - "#power-domain-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/rk3399-cru.h>
+    #include <dt-bindings/power/rk3399-power.h>
+
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        qos_hdcp: qos@ffa90000 {
+            compatible = "rockchip,rk3399-qos", "syscon";
+            reg = <0x0 0xffa90000 0x0 0x20>;
+        };
+
+        qos_iep: qos@ffa98000 {
+            compatible = "rockchip,rk3399-qos", "syscon";
+            reg = <0x0 0xffa98000 0x0 0x20>;
+        };
+
+        qos_rga_r: qos@ffab0000 {
+            compatible = "rockchip,rk3399-qos", "syscon";
+            reg = <0x0 0xffab0000 0x0 0x20>;
+        };
+
+        qos_rga_w: qos@ffab0080 {
+            compatible = "rockchip,rk3399-qos", "syscon";
+            reg = <0x0 0xffab0080 0x0 0x20>;
+        };
+
+        qos_video_m0: qos@ffab8000 {
+            compatible = "rockchip,rk3399-qos", "syscon";
+            reg = <0x0 0xffab8000 0x0 0x20>;
+        };
+
+        qos_video_m1_r: qos@ffac0000 {
+            compatible = "rockchip,rk3399-qos", "syscon";
+            reg = <0x0 0xffac0000 0x0 0x20>;
+        };
+
+        qos_video_m1_w: qos@ffac0080 {
+            compatible = "rockchip,rk3399-qos", "syscon";
+            reg = <0x0 0xffac0080 0x0 0x20>;
+        };
+
+        power-management@ff310000 {
+            compatible = "rockchip,rk3399-pmu", "syscon", "simple-mfd";
+            reg = <0x0 0xff310000 0x0 0x1000>;
+
+            power-controller {
+                compatible = "rockchip,rk3399-power-controller";
+                #power-domain-cells = <1>;
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                /* These power domains are grouped by VD_CENTER */
+                pd_iep@RK3399_PD_IEP {
+                    reg = <RK3399_PD_IEP>;
+                    clocks = <&cru ACLK_IEP>,
+                             <&cru HCLK_IEP>;
+                    pm_qos = <&qos_iep>;
+                    #power-domain-cells = <0>;
+                };
+                pd_rga@RK3399_PD_RGA {
+                    reg = <RK3399_PD_RGA>;
+                    clocks = <&cru ACLK_RGA>,
+                             <&cru HCLK_RGA>;
+                    pm_qos = <&qos_rga_r>,
+                             <&qos_rga_w>;
+                    #power-domain-cells = <0>;
+                };
+                pd_vcodec@RK3399_PD_VCODEC {
+                    reg = <RK3399_PD_VCODEC>;
+                    clocks = <&cru ACLK_VCODEC>,
+                             <&cru HCLK_VCODEC>;
+                    pm_qos = <&qos_video_m0>;
+                    #power-domain-cells = <0>;
+                };
+                pd_vdu@RK3399_PD_VDU {
+                    reg = <RK3399_PD_VDU>;
+                    clocks = <&cru ACLK_VDU>,
+                             <&cru HCLK_VDU>;
+                    pm_qos = <&qos_video_m1_r>,
+                             <&qos_video_m1_w>;
+                    #power-domain-cells = <0>;
+                };
+                pd_vio@RK3399_PD_VIO {
+                    reg = <RK3399_PD_VIO>;
+                    #power-domain-cells = <1>;
+                    #address-cells = <1>;
+                    #size-cells = <0>;
+
+                    pd_hdcp@RK3399_PD_HDCP {
+                        reg = <RK3399_PD_HDCP>;
+                        clocks = <&cru ACLK_HDCP>,
+                                 <&cru HCLK_HDCP>,
+                                 <&cru PCLK_HDCP>;
+                        pm_qos = <&qos_hdcp>;
+                        #power-domain-cells = <0>;
+                    };
+                };
+            };
+        };
+    };
diff --git a/Documentation/devicetree/bindings/soc/rockchip/power_domain.txt b/Documentation/devicetree/bindings/soc/rockchip/power_domain.txt
deleted file mode 100644
index 8304eceb62e4..000000000000
--- a/Documentation/devicetree/bindings/soc/rockchip/power_domain.txt
+++ /dev/null
@@ -1,136 +0,0 @@ 
-* Rockchip Power Domains
-
-Rockchip processors include support for multiple power domains which can be
-powered up/down by software based on different application scenes to save power.
-
-Required properties for power domain controller:
-- compatible: Should be one of the following.
-	"rockchip,px30-power-controller" - for PX30 SoCs.
-	"rockchip,rk3036-power-controller" - for RK3036 SoCs.
-	"rockchip,rk3066-power-controller" - for RK3066 SoCs.
-	"rockchip,rk3128-power-controller" - for RK3128 SoCs.
-	"rockchip,rk3188-power-controller" - for RK3188 SoCs.
-	"rockchip,rk3228-power-controller" - for RK3228 SoCs.
-	"rockchip,rk3288-power-controller" - for RK3288 SoCs.
-	"rockchip,rk3328-power-controller" - for RK3328 SoCs.
-	"rockchip,rk3366-power-controller" - for RK3366 SoCs.
-	"rockchip,rk3368-power-controller" - for RK3368 SoCs.
-	"rockchip,rk3399-power-controller" - for RK3399 SoCs.
-- #power-domain-cells: Number of cells in a power-domain specifier.
-	Should be 1 for multiple PM domains.
-- #address-cells: Should be 1.
-- #size-cells: Should be 0.
-
-Required properties for power domain sub nodes:
-- reg: index of the power domain, should use macros in:
-	"include/dt-bindings/power/px30-power.h" - for PX30 type power domain.
-	"include/dt-bindings/power/rk3036-power.h" - for RK3036 type power domain.
-	"include/dt-bindings/power/rk3066-power.h" - for RK3066 type power domain.
-	"include/dt-bindings/power/rk3128-power.h" - for RK3128 type power domain.
-	"include/dt-bindings/power/rk3188-power.h" - for RK3188 type power domain.
-	"include/dt-bindings/power/rk3228-power.h" - for RK3228 type power domain.
-	"include/dt-bindings/power/rk3288-power.h" - for RK3288 type power domain.
-	"include/dt-bindings/power/rk3328-power.h" - for RK3328 type power domain.
-	"include/dt-bindings/power/rk3366-power.h" - for RK3366 type power domain.
-	"include/dt-bindings/power/rk3368-power.h" - for RK3368 type power domain.
-	"include/dt-bindings/power/rk3399-power.h" - for RK3399 type power domain.
-- clocks (optional): phandles to clocks which need to be enabled while power domain
-	switches state.
-- pm_qos (optional): phandles to qos blocks which need to be saved and restored
-	while power domain switches state.
-
-Qos Example:
-
-	qos_gpu: qos_gpu@ffaf0000 {
-		compatible ="syscon";
-		reg = <0x0 0xffaf0000 0x0 0x20>;
-	};
-
-Example:
-
-	power: power-controller {
-		compatible = "rockchip,rk3288-power-controller";
-		#power-domain-cells = <1>;
-		#address-cells = <1>;
-		#size-cells = <0>;
-
-		pd_gpu {
-			reg = <RK3288_PD_GPU>;
-			clocks = <&cru ACLK_GPU>;
-			pm_qos = <&qos_gpu>;
-		};
-	};
-
-	 power: power-controller {
-                compatible = "rockchip,rk3368-power-controller";
-                #power-domain-cells = <1>;
-                #address-cells = <1>;
-                #size-cells = <0>;
-
-                pd_gpu_1 {
-                        reg = <RK3368_PD_GPU_1>;
-                        clocks = <&cru ACLK_GPU_CFG>;
-                };
-        };
-
-Example 2:
-		power: power-controller {
-			compatible = "rockchip,rk3399-power-controller";
-			#power-domain-cells = <1>;
-			#address-cells = <1>;
-			#size-cells = <0>;
-
-			pd_vio {
-				#address-cells = <1>;
-				#size-cells = <0>;
-				reg = <RK3399_PD_VIO>;
-
-				pd_vo {
-					#address-cells = <1>;
-					#size-cells = <0>;
-					reg = <RK3399_PD_VO>;
-
-					pd_vopb {
-						reg = <RK3399_PD_VOPB>;
-					};
-
-					pd_vopl {
-						reg = <RK3399_PD_VOPL>;
-					};
-				};
-			};
-		};
-
-Node of a device using power domains must have a power-domains property,
-containing a phandle to the power device node and an index specifying which
-power domain to use.
-The index should use macros in:
-	"include/dt-bindings/power/px30-power.h" - for px30 type power domain.
-	"include/dt-bindings/power/rk3036-power.h" - for rk3036 type power domain.
-	"include/dt-bindings/power/rk3128-power.h" - for rk3128 type power domain.
-	"include/dt-bindings/power/rk3128-power.h" - for rk3228 type power domain.
-	"include/dt-bindings/power/rk3288-power.h" - for rk3288 type power domain.
-	"include/dt-bindings/power/rk3328-power.h" - for rk3328 type power domain.
-	"include/dt-bindings/power/rk3366-power.h" - for rk3366 type power domain.
-	"include/dt-bindings/power/rk3368-power.h" - for rk3368 type power domain.
-	"include/dt-bindings/power/rk3399-power.h" - for rk3399 type power domain.
-
-Example of the node using power domain:
-
-	node {
-		/* ... */
-		power-domains = <&power RK3288_PD_GPU>;
-		/* ... */
-	};
-
-	node {
-                /* ... */
-                power-domains = <&power RK3368_PD_GPU_1>;
-                /* ... */
-        };
-
-	node {
-		/* ... */
-		power-domains = <&power RK3399_PD_VOPB>;
-		/* ... */
-	};