Re: [RFC 1/2] dt-bindings: power: Convert Generic Power Domain bindings to json-schema

From: Krzysztof Kozlowski
Date: Wed Oct 02 2019 - 05:29:10 EST

Next message: Jani Nikula: "Re: New sysfs interface for privacy screens"
Previous message: Hawa, Hanna: "Re: [PATCH v4 1/2] edac: Add an API for edac device to report for multiple errors"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Mon, Sep 30, 2019 at 09:00:06AM -0500, Rob Herring wrote:
> On Sun, Sep 08, 2019 at 05:28:12PM +0200, Krzysztof Kozlowski wrote:
> > Convert Generic Power Domain bindings to DT schema format using
> > json-schema. The consumer bindings are split to separate file.
> >
> > Signed-off-by: Krzysztof Kozlowski <krzk@xxxxxxxxxx>
> >
> > ---
> >
> > TODO:
> > 1. How the consumer bindings should match nodes?
>
> How it already done. The consumer binding is always applied so we
> always apply the type schema to the consumer properties. Device specific
> bindings have to list the consumer properties because they need to
> define how many power domains and the names.

I'll apply then select:true so the "required-opps" will be matched.

>
> > 2. Somehow the example complains:
> > Documentation/devicetree/bindings/power/power-domain.example.dt.yaml: power-controller@12341000: power-domains:0: [1, 0] is too long
> > Documentation/devicetree/bindings/power/power-domain.example.dt.yaml: power-controller@12341000: power-domains:0:1: missing phandle tag in 0
>
> You used 'phandle' instead of 'phandle-array'. If you have cells, the
> latter is used. Perhaps we should distinguish between having cells or no
> cells.

Thanks, I'll drop it anyway as it is duplicating dt-schema.

>
> > 3. Current matching by node name causes false positives:
> > arch/arm/boot/dts/am335x-icev2.dt.yaml: power-controller@2d: '#power-domain-cells' is a required property
> > arch/arm/boot/dts/qcom-apq8064-cm-qs600.dt.yaml: power-controller@2099000: '#power-domain-cells' is a required property
>
> They should have different node names if they are not power-domain
> providers.
>
> > ---
> > .../devicetree/bindings/arm/arm,scmi.txt | 2 +-
> > .../devicetree/bindings/arm/arm,scpi.txt | 2 +-
> > .../bindings/arm/freescale/fsl,scu.txt | 2 +-
> > .../bindings/clock/clk-exynos-audss.txt | 2 +-
> > .../bindings/clock/exynos5433-clock.txt | 4 +-
> > .../bindings/clock/renesas,cpg-mssr.txt | 2 +-
> > .../clock/renesas,r8a7778-cpg-clocks.txt | 2 +-
> > .../clock/renesas,r8a7779-cpg-clocks.txt | 2 +-
> > .../clock/renesas,rcar-gen2-cpg-clocks.txt | 2 +-
> > .../bindings/clock/renesas,rz-cpg-clocks.txt | 2 +-
> > .../bindings/clock/ti/davinci/psc.txt | 2 +-
> > .../bindings/display/etnaviv/etnaviv-drm.txt | 2 +-
> > .../devicetree/bindings/display/msm/dpu.txt | 2 +-
> > .../devicetree/bindings/display/msm/mdp5.txt | 2 +-
> > .../devicetree/bindings/dsp/fsl,dsp.yaml | 2 +-
> > .../firmware/nvidia,tegra186-bpmp.txt | 2 +-
> > .../bindings/media/imx7-mipi-csi2.txt | 3 +-
> > .../bindings/media/mediatek-jpeg-decoder.txt | 3 +-
> > .../bindings/media/mediatek-mdp.txt | 3 +-
> > .../bindings/opp/qcom-nvmem-cpufreq.txt | 2 +-
> > .../devicetree/bindings/pci/pci-keystone.txt | 2 +-
> > .../bindings/phy/ti,phy-am654-serdes.txt | 2 +-
> > .../bindings/power/amlogic,meson-gx-pwrc.txt | 2 +-
> > .../devicetree/bindings/power/fsl,imx-gpc.txt | 2 +-
> > .../bindings/power/fsl,imx-gpcv2.txt | 2 +-
> > .../power/power-domain-consumers.yaml | 124 +++++++++++
> > .../bindings/power/power-domain.yaml | 136 ++++++++++++
> > .../bindings/power/power_domain.txt | 205 ------------------
> > .../devicetree/bindings/power/qcom,rpmpd.txt | 2 +-
> > .../bindings/power/renesas,rcar-sysc.txt | 2 +-
> > .../bindings/power/renesas,sysc-rmobile.txt | 2 +-
> > .../bindings/power/xlnx,zynqmp-genpd.txt | 2 +-
> > .../bindings/soc/bcm/brcm,bcm2835-pm.txt | 2 +-
> > .../bindings/soc/mediatek/scpsys.txt | 2 +-
> > .../bindings/soc/ti/sci-pm-domain.txt | 2 +-
> > .../bindings/usb/nvidia,tegra124-xusb.txt | 4 +-
> > MAINTAINERS | 2 +-
> > 37 files changed, 299 insertions(+), 241 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/power/power-domain-consumers.yaml
> > create mode 100644 Documentation/devicetree/bindings/power/power-domain.yaml
> > delete mode 100644 Documentation/devicetree/bindings/power/power_domain.txt
>
>
> > diff --git a/Documentation/devicetree/bindings/power/power-domain-consumers.yaml b/Documentation/devicetree/bindings/power/power-domain-consumers.yaml
> > new file mode 100644
> > index 000000000000..95cc6da1e8a8
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/power/power-domain-consumers.yaml
> > @@ -0,0 +1,124 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/power/power-domain-consumers.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: PM domain consumers
> > +
> > +maintainers:
> > + - Rafael J. Wysocki <rjw@xxxxxxxxxxxxx>
> > + - Kevin Hilman <khilman@xxxxxxxxxx>
> > + - Ulf Hansson <ulf.hansson@xxxxxxxxxx>
> > +
> > +description: |+
> > + See power-domain.yaml
> > +
> > +allOf:
> > + - $ref: /schemas/power-domain/power-domain-consumer.yaml
> > +
> > +# TODO: missing select (compatible?)
> > +
> > +properties:
> > + power-domains:
> > + $ref: /schemas/types.yaml#/definitions/phandle-array
> > + description:
> > + A list of PM domain specifiers, as defined by bindings of the power
> > + controller that is the PM domain provider.
> > +
> > + power-domain-names:
> > + $ref: /schemas/types.yaml#/definitions/string-array
> > + description:
> > + A list of power domain name strings sorted in the same order
> > + as the power-domains property. Consumers drivers will use
> > + power-domain-names to match power domains with power-domains specifiers.
>
> Why are we duplicating this?

My bad.

>
> > +
> > + required-opps:
> > + $ref: /schemas/types.yaml#/definitions/phandle
> > + description:
> > + This contains phandle to an OPP node in another device's OPP table.
> > + It may contain an array of phandles, where each phandle points to an OPP
> > + of a different device. It should not contain multiple phandles to the OPP
> > + nodes in the same OPP table. This specifies the minimum required OPP
> > + of the device(s), whose OPP's phandle is present in this property,
> > + for the functioning of the current device at the current OPP (where this
> > + property is present).
> > +
> > +required:
> > + - power-domains
> > +
> > +dependencies:
> > + power-domain-names: [ power-domains ]
> > +
> > +examples:
> > + - |
> > + leaky-device@12350000 {
> > + compatible = "foo,i-leak-current";
> > + reg = <0x12350000 0x1000>;
> > + power-domains = <&power 0>;
> > + power-domain-names = "io";
> > + };
> > +
> > + leaky-device@12351000 {
> > + compatible = "foo,i-leak-current";
> > + reg = <0x12351000 0x1000>;
> > + power-domains = <&power 0>, <&power 1> ;
> > + power-domain-names = "io", "clk";
> > + };
> > +
> > + // The first example above defines a typical PM domain consumer device, which is
> > + // located inside a PM domain with index 0 of a power controller represented by a
> > + // node with the label "power".
> > + // In the second example the consumer device are partitioned across two PM domains,
> > + // the first with index 0 and the second with index 1, of a power controller that
> > + // is represented by a node with the label "power".
> > +
> > + - |
> > + // Example with OPP table for domain provider that provides two domains:
> > +
> > + domain0_opp_table: opp-table0 {
> > + compatible = "operating-points-v2";
> > +
> > + domain0_opp_0: opp-1000000000 {
> > + opp-hz = /bits/ 64 <1000000000>;
> > + opp-microvolt = <975000 970000 985000>;
> > + };
> > + domain0_opp_1: opp-1100000000 {
> > + opp-hz = /bits/ 64 <1100000000>;
> > + opp-microvolt = <1000000 980000 1010000>;
> > + };
> > + };
> > +
> > + domain1_opp_table: opp-table1 {
> > + compatible = "operating-points-v2";
> > +
> > + domain1_opp_0: opp-1200000000 {
> > + opp-hz = /bits/ 64 <1200000000>;
> > + opp-microvolt = <975000 970000 985000>;
> > + };
> > + domain1_opp_1: opp-1300000000 {
> > + opp-hz = /bits/ 64 <1300000000>;
> > + opp-microvolt = <1000000 980000 1010000>;
> > + };
> > + };
> > +
> > + power: power-controller@12340000 {
> > + compatible = "foo,power-controller";
> > + reg = <0x12340000 0x1000>;
> > + #power-domain-cells = <1>;
> > + operating-points-v2 = <&domain0_opp_table>, <&domain1_opp_table>;
> > + };
> > +
> > + leaky-device0@12350000 {
> > + compatible = "foo,i-leak-current";
> > + reg = <0x12350000 0x1000>;
> > + power-domains = <&power 0>;
> > + required-opps = <&domain0_opp_0>;
> > + };
> > +
> > + leaky-device1@12350000 {
> > + compatible = "foo,i-leak-current";
> > + reg = <0x12350000 0x1000>;
> > + power-domains = <&power 1>;
> > + required-opps = <&domain1_opp_1>;
> > + };
> > diff --git a/Documentation/devicetree/bindings/power/power-domain.yaml b/Documentation/devicetree/bindings/power/power-domain.yaml
> > new file mode 100644
> > index 000000000000..9454fdb904ae
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/power/power-domain.yaml
> > @@ -0,0 +1,136 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/power/power-domain.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Generic PM domains
> > +
> > +maintainers:
> > + - Rafael J. Wysocki <rjw@xxxxxxxxxxxxx>
> > + - Kevin Hilman <khilman@xxxxxxxxxx>
> > + - Ulf Hansson <ulf.hansson@xxxxxxxxxx>
> > +
> > +description: |+
> > + System on chip designs are often divided into multiple PM domains that can be
> > + used for power gating of selected IP blocks for power saving by reduced leakage
> > + current.
> > +
> > + This device tree binding can be used to bind PM domain consumer devices with
> > + their PM domains provided by PM domain providers. A PM domain provider can be
> > + represented by any node in the device tree and can provide one or more PM
> > + domains. A consumer node can refer to the provider by a phandle and a set of
> > + phandle arguments (so called PM domain specifiers) of length specified by the
> > + \#power-domain-cells property in the PM domain provider node.
> > +
> > +properties:
> > + $nodename:
> > + pattern: "^(power-controller|power-domain)(@.*)?"
> > +
> > + domain-idle-states:
> > + $ref: /schemas/types.yaml#/definitions/phandle-array
> > + description:
> > + A phandle of an idle-state that shall be soaked into a generic domain
> > + power state. The idle state definitions are compatible with
> > + domain-idle-state specified in
> > + Documentation/devicetree/bindings/power/domain-idle-state.txt
> > + phandles that are not compatible with domain-idle-state will be ignored.
> > + The domain-idle-state property reflects the idle state of this PM domain
> > + and not the idle states of the devices or sub-domains in the PM domain.
> > + Devices and sub-domains have their own idle-states independent
> > + of the parent domain's idle states. In the absence of this property,
> > + the domain would be considered as capable of being powered-on
> > + or powered-off.
> > +
> > + operating-points-v2:
> > + $ref: /schemas/types.yaml#/definitions/phandle-array
>
> This needs to be defined as part of an OPP schema. I guess there's some
> value listing it here, but a device specific binding will still have to
> list it to define how many entries.

So for now I'll leave it here. Consecutive patches can split it.

>
> > + description:
> > + Phandles to the OPP tables of power domains provided by a power domain
> > + provider. If the provider provides a single power domain only or all
> > + the power domains provided by the provider have identical OPP tables,
> > + then this shall contain a single phandle. Refer to ../opp/opp.txt
> > + for more information.
> > +
> > + "#power-domain-cells":
> > + $ref: /schemas/types.yaml#/definitions/uint32
>
> Strictly speaking, we don't need to define the type as '#.*-cells' has a
> type.

OK

>
> > + description:
> > + Number of cells in a PM domain specifier. Typically 0 for nodes
> > + representing a single PM domain and 1 for nodes providing multiple PM
> > + domains (e.g. power controllers), but can be any value as specified
> > + by device tree binding documentation of particular provider.
> > +
> > + power-domains:
> > + $ref: /schemas/types.yaml#/definitions/phandle
>
> Don't need type here either.

OK

Best regards,
Krzysztof

Next message: Jani Nikula: "Re: New sysfs interface for privacy screens"
Previous message: Hawa, Hanna: "Re: [PATCH v4 1/2] edac: Add an API for edac device to report for multiple errors"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]