[PATCH v7 04/10] dt-bindings: soc: google: gs101-pmu: allow power domains as children

André Draszik posted 10 patches 1 month ago
There is a newer version of this series
[PATCH v7 04/10] dt-bindings: soc: google: gs101-pmu: allow power domains as children
Posted by André Draszik 1 month ago
The power domains are a property of / implemented in the PMU. As such,
they should be modelled as child nodes of the PMU.

Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
v7:
- really be consistent with quoting (Krzysztof)
- drop invalid tested-by tag (Krzysztof)

v4:
- consistent quoting using " (Krzysztof)
- add samsung,dtzpc to example

Note:
Because the properties added are 'required', this commit breaks DT
validation of the existing DT for Pixel 6, but a) that's simply because
the DT is incomplete and b) a DT update will be posted once the binding
is accepted.
It is not possible to write the binding such that it supports old
(incomplete) DTs in addition to the full version, but as per above
it's not required to keep supporting old DTs.
---
 .../bindings/soc/google/google,gs101-pmu.yaml      | 41 ++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/Documentation/devicetree/bindings/soc/google/google,gs101-pmu.yaml b/Documentation/devicetree/bindings/soc/google/google,gs101-pmu.yaml
index a06bd8ec3c20..dfe6f87e5949 100644
--- a/Documentation/devicetree/bindings/soc/google/google,gs101-pmu.yaml
+++ b/Documentation/devicetree/bindings/soc/google/google,gs101-pmu.yaml
@@ -16,6 +16,14 @@ properties:
   reg:
     maxItems: 1
 
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 1
+
+  ranges: true
+
   reboot-mode:
     $ref: /schemas/power/reset/syscon-reboot-mode.yaml
     type: object
@@ -39,9 +47,23 @@ properties:
     description:
       Phandle to PMU interrupt generation interface.
 
+patternProperties:
+  "^power-domain@[0-9a-f]+$":
+    type: object
+    description: Child node describing one power domain within the PMU
+
+    additionalProperties: true
+
+    properties:
+      compatible:
+        const: google,gs101-pd
+
 required:
   - compatible
   - reg
+  - "#address-cells"
+  - "#size-cells"
+  - ranges
   - google,pmu-intr-gen-syscon
 
 additionalProperties: false
@@ -51,6 +73,25 @@ examples:
     system-controller@17460000 {
         compatible = "google,gs101-pmu";
         reg = <0x17460000 0x10000>;
+        #address-cells = <1>;
+        #size-cells = <1>;
+        ranges;
 
         google,pmu-intr-gen-syscon = <&pmu_intr_gen>;
+
+        pd_g3d: power-domain@1e00 {
+            compatible = "google,gs101-pd";
+            reg = <0x1e00 0x80>;
+            #power-domain-cells = <0>;
+            label = "g3d";
+            samsung,dtzpc = <&pd_g3d>;
+        };
+
+        power-domain@2000 {
+            compatible = "google,gs101-pd";
+            reg = <0x2000 0x80>;
+            #power-domain-cells = <0>;
+            power-domains = <&pd_g3d>;
+            label = "embedded_g3d";
+        };
     };

-- 
2.53.0.473.g4a7958ca14-goog

Re: [PATCH v7 04/10] dt-bindings: soc: google: gs101-pmu: allow power domains as children
Posted by Rob Herring 4 weeks ago
On Fri, Mar 06, 2026 at 10:29:55AM +0000, André Draszik wrote:
> The power domains are a property of / implemented in the PMU. As such,
> they should be modelled as child nodes of the PMU.
> 
> Signed-off-by: André Draszik <andre.draszik@linaro.org>
> ---
> v7:
> - really be consistent with quoting (Krzysztof)
> - drop invalid tested-by tag (Krzysztof)
> 
> v4:
> - consistent quoting using " (Krzysztof)
> - add samsung,dtzpc to example
> 
> Note:
> Because the properties added are 'required', this commit breaks DT
> validation of the existing DT for Pixel 6, but a) that's simply because
> the DT is incomplete and b) a DT update will be posted once the binding
> is accepted.
> It is not possible to write the binding such that it supports old
> (incomplete) DTs in addition to the full version, but as per above
> it's not required to keep supporting old DTs.

This information needs to go in the commit msg.

> ---
>  .../bindings/soc/google/google,gs101-pmu.yaml      | 41 ++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/soc/google/google,gs101-pmu.yaml b/Documentation/devicetree/bindings/soc/google/google,gs101-pmu.yaml
> index a06bd8ec3c20..dfe6f87e5949 100644
> --- a/Documentation/devicetree/bindings/soc/google/google,gs101-pmu.yaml
> +++ b/Documentation/devicetree/bindings/soc/google/google,gs101-pmu.yaml
> @@ -16,6 +16,14 @@ properties:
>    reg:
>      maxItems: 1
>  
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 1
> +
> +  ranges: true
> +
>    reboot-mode:
>      $ref: /schemas/power/reset/syscon-reboot-mode.yaml
>      type: object
> @@ -39,9 +47,23 @@ properties:
>      description:
>        Phandle to PMU interrupt generation interface.
>  
> +patternProperties:
> +  "^power-domain@[0-9a-f]+$":
> +    type: object
> +    description: Child node describing one power domain within the PMU
> +
> +    additionalProperties: true
> +
> +    properties:
> +      compatible:
> +        const: google,gs101-pd
> +
>  required:
>    - compatible
>    - reg
> +  - "#address-cells"
> +  - "#size-cells"
> +  - ranges
>    - google,pmu-intr-gen-syscon
>  
>  additionalProperties: false
> @@ -51,6 +73,25 @@ examples:
>      system-controller@17460000 {
>          compatible = "google,gs101-pmu";
>          reg = <0x17460000 0x10000>;
> +        #address-cells = <1>;
> +        #size-cells = <1>;
> +        ranges;
>  
>          google,pmu-intr-gen-syscon = <&pmu_intr_gen>;
> +
> +        pd_g3d: power-domain@1e00 {
> +            compatible = "google,gs101-pd";
> +            reg = <0x1e00 0x80>;

I'm assuming 0x1e00 is an offset from 0x17460000. That's not what ranges 
says though. It says both addresses are in the same address space 
(system-controller@17460000 parent's address space). You need:

ranges = <0x0 0x17460000 0x10000>;


> +            #power-domain-cells = <0>;
> +            label = "g3d";
> +            samsung,dtzpc = <&pd_g3d>;
> +        };
> +
> +        power-domain@2000 {
> +            compatible = "google,gs101-pd";
> +            reg = <0x2000 0x80>;
> +            #power-domain-cells = <0>;
> +            power-domains = <&pd_g3d>;
> +            label = "embedded_g3d";
> +        };
>      };
> 
> -- 
> 2.53.0.473.g4a7958ca14-goog
> 
Re: [PATCH v7 04/10] dt-bindings: soc: google: gs101-pmu: allow power domains as children
Posted by André Draszik 3 weeks, 6 days ago
On Thu, 2026-03-12 at 10:12 -0500, Rob Herring wrote:
> On Fri, Mar 06, 2026 at 10:29:55AM +0000, André Draszik wrote:
> 
> >      system-controller@17460000 {
> >          compatible = "google,gs101-pmu";
> >          reg = <0x17460000 0x10000>;
> > +        #address-cells = <1>;
> > +        #size-cells = <1>;
> > +        ranges;
> >  
> >          google,pmu-intr-gen-syscon = <&pmu_intr_gen>;
> > +
> > +        pd_g3d: power-domain@1e00 {
> > +            compatible = "google,gs101-pd";
> > +            reg = <0x1e00 0x80>;
> 
> I'm assuming 0x1e00 is an offset from 0x17460000. That's not what ranges 
> says though. It says both addresses are in the same address space 
> (system-controller@17460000 parent's address space). You need:
> 
> ranges = <0x0 0x17460000 0x10000>;

Thanks Rob! On gs101, the PD driver can not do direct mmio. Instead it
needs to use the regmap that the parent has created and apply the
offset from the PD's reg property (using struct resource::start)
(patch 7).

When using ranges as per your suggestion that doesn't work anymore,
as resource->start isn't the offset anymore but the final physical
address, and using that in combination with the parent's regmap
doesn't give us the right address anymore.

Is there an alternative other than using completely arbitrary indices
like e.g. rockchip is doing?


Cheers,
Andre'
Re: [PATCH v7 04/10] dt-bindings: soc: google: gs101-pmu: allow power domains as children
Posted by Rob Herring 3 weeks, 6 days ago
On Fri, Mar 13, 2026 at 5:48 AM André Draszik <andre.draszik@linaro.org> wrote:
>
> On Thu, 2026-03-12 at 10:12 -0500, Rob Herring wrote:
> > On Fri, Mar 06, 2026 at 10:29:55AM +0000, André Draszik wrote:
> >
> > >      system-controller@17460000 {
> > >          compatible = "google,gs101-pmu";
> > >          reg = <0x17460000 0x10000>;
> > > +        #address-cells = <1>;
> > > +        #size-cells = <1>;
> > > +        ranges;
> > >
> > >          google,pmu-intr-gen-syscon = <&pmu_intr_gen>;
> > > +
> > > +        pd_g3d: power-domain@1e00 {
> > > +            compatible = "google,gs101-pd";
> > > +            reg = <0x1e00 0x80>;
> >
> > I'm assuming 0x1e00 is an offset from 0x17460000. That's not what ranges
> > says though. It says both addresses are in the same address space
> > (system-controller@17460000 parent's address space). You need:
> >
> > ranges = <0x0 0x17460000 0x10000>;
>
> Thanks Rob! On gs101, the PD driver can not do direct mmio. Instead it
> needs to use the regmap that the parent has created and apply the
> offset from the PD's reg property (using struct resource::start)
> (patch 7).
>
> When using ranges as per your suggestion that doesn't work anymore,
> as resource->start isn't the offset anymore but the final physical
> address, and using that in combination with the parent's regmap
> doesn't give us the right address anymore.

You are mixing kernel implementation details and h/w. Are the
registers in the child nodes MMIO or not? If not, then drop ranges. If
they are, then what I suggested for ranges is correct.

For MMIO, your kernel implementation options are what you suggested,
do a regmap for each child, or use of_property_read_reg().

Rob
Re: [PATCH v7 04/10] dt-bindings: soc: google: gs101-pmu: allow power domains as children
Posted by André Draszik 3 weeks, 6 days ago
On Fri, 2026-03-13 at 08:26 -0500, Rob Herring wrote:
> On Fri, Mar 13, 2026 at 5:48 AM André Draszik <andre.draszik@linaro.org> wrote:
> > 
> > On Thu, 2026-03-12 at 10:12 -0500, Rob Herring wrote:
> > > On Fri, Mar 06, 2026 at 10:29:55AM +0000, André Draszik wrote:
> > > 
> > > >      system-controller@17460000 {
> > > >          compatible = "google,gs101-pmu";
> > > >          reg = <0x17460000 0x10000>;
> > > > +        #address-cells = <1>;
> > > > +        #size-cells = <1>;
> > > > +        ranges;
> > > > 
> > > >          google,pmu-intr-gen-syscon = <&pmu_intr_gen>;
> > > > +
> > > > +        pd_g3d: power-domain@1e00 {
> > > > +            compatible = "google,gs101-pd";
> > > > +            reg = <0x1e00 0x80>;
> > > 
> > > I'm assuming 0x1e00 is an offset from 0x17460000. That's not what ranges
> > > says though. It says both addresses are in the same address space
> > > (system-controller@17460000 parent's address space). You need:
> > > 
> > > ranges = <0x0 0x17460000 0x10000>;
> > 
> > Thanks Rob! On gs101, the PD driver can not do direct mmio. Instead it
> > needs to use the regmap that the parent has created and apply the
> > offset from the PD's reg property (using struct resource::start)
> > (patch 7).
> > 
> > When using ranges as per your suggestion that doesn't work anymore,
> > as resource->start isn't the offset anymore but the final physical
> > address, and using that in combination with the parent's regmap
> > doesn't give us the right address anymore.
> 
> You are mixing kernel implementation details and h/w.

Just trying to get to the best solution, considering both.

> Are the
> registers in the child nodes MMIO or not? If not, then drop ranges. If
> they are, then what I suggested for ranges is correct.

While they are MMIO in theory and offsets into the parent's address
space, in practice access from the OS is not possible via mmio APIs,
only via the custom regmap created by the parent (which defers to the
EL3 firmware) - hence the driver's need for the offset from the
parent's base address.

> For MMIO, your kernel implementation options are what you suggested,
> do a regmap for each child, or use of_property_read_reg().

OK, I'll go with the first option then, thanks for your patience Rob!

Cheers,
Andre'
Re: [PATCH v7 04/10] dt-bindings: soc: google: gs101-pmu: allow power domains as children
Posted by André Draszik 3 weeks, 6 days ago
On Fri, 2026-03-13 at 10:48 +0000, André Draszik wrote:
> On Thu, 2026-03-12 at 10:12 -0500, Rob Herring wrote:
> > On Fri, Mar 06, 2026 at 10:29:55AM +0000, André Draszik wrote:
> > 
> > >      system-controller@17460000 {
> > >          compatible = "google,gs101-pmu";
> > >          reg = <0x17460000 0x10000>;
> > > +        #address-cells = <1>;
> > > +        #size-cells = <1>;
> > > +        ranges;
> > >  
> > >          google,pmu-intr-gen-syscon = <&pmu_intr_gen>;
> > > +
> > > +        pd_g3d: power-domain@1e00 {
> > > +            compatible = "google,gs101-pd";
> > > +            reg = <0x1e00 0x80>;
> > 
> > I'm assuming 0x1e00 is an offset from 0x17460000. That's not what ranges 
> > says though. It says both addresses are in the same address space 
> > (system-controller@17460000 parent's address space). You need:
> > 
> > ranges = <0x0 0x17460000 0x10000>;
> 
> Thanks Rob! On gs101, the PD driver can not do direct mmio. Instead it
> needs to use the regmap that the parent has created and apply the
> offset from the PD's reg property (using struct resource::start)
> (patch 7).
> 
> When using ranges as per your suggestion that doesn't work anymore,
> as resource->start isn't the offset anymore but the final physical
> address, and using that in combination with the parent's regmap
> doesn't give us the right address anymore.
> 
> Is there an alternative other than using completely arbitrary indices
> like e.g. rockchip is doing?

My driver could of course peek into the parent and get the parent's
IORESOURCE_MEM and subtract that start address to get the offset
back.

Is that considered clean (enough)?

	ppdev = to_platform_device(dev->parent);
	pres = platform_get_resource(ppdev, IORESOURCE_MEM, 0);

	pd->regmap = syscon_node_to_regmap(dev->parent->of_node);

	pd->configuration_reg = res->start - pres->start;
	pd->status_reg = res->start - pres->start;


Cheers,
Andre'
Re: [PATCH v7 04/10] dt-bindings: soc: google: gs101-pmu: allow power domains as children
Posted by Peter Griffin 1 month ago
On Fri, 6 Mar 2026 at 10:29, André Draszik <andre.draszik@linaro.org> wrote:
>
> The power domains are a property of / implemented in the PMU. As such,
> they should be modelled as child nodes of the PMU.
>
> Signed-off-by: André Draszik <andre.draszik@linaro.org>
> ---

Reviewed-by: Peter Griffin <peter.griffin@linaro.org>

> v7:
> - really be consistent with quoting (Krzysztof)
> - drop invalid tested-by tag (Krzysztof)
>
> v4:
> - consistent quoting using " (Krzysztof)
> - add samsung,dtzpc to example
>
> Note:
> Because the properties added are 'required', this commit breaks DT
> validation of the existing DT for Pixel 6, but a) that's simply because
> the DT is incomplete and b) a DT update will be posted once the binding
> is accepted.
> It is not possible to write the binding such that it supports old
> (incomplete) DTs in addition to the full version, but as per above
> it's not required to keep supporting old DTs.
> ---
>  .../bindings/soc/google/google,gs101-pmu.yaml      | 41 ++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/soc/google/google,gs101-pmu.yaml b/Documentation/devicetree/bindings/soc/google/google,gs101-pmu.yaml
> index a06bd8ec3c20..dfe6f87e5949 100644
> --- a/Documentation/devicetree/bindings/soc/google/google,gs101-pmu.yaml
> +++ b/Documentation/devicetree/bindings/soc/google/google,gs101-pmu.yaml
> @@ -16,6 +16,14 @@ properties:
>    reg:
>      maxItems: 1
>
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 1
> +
> +  ranges: true
> +
>    reboot-mode:
>      $ref: /schemas/power/reset/syscon-reboot-mode.yaml
>      type: object
> @@ -39,9 +47,23 @@ properties:
>      description:
>        Phandle to PMU interrupt generation interface.
>
> +patternProperties:
> +  "^power-domain@[0-9a-f]+$":
> +    type: object
> +    description: Child node describing one power domain within the PMU
> +
> +    additionalProperties: true
> +
> +    properties:
> +      compatible:
> +        const: google,gs101-pd
> +
>  required:
>    - compatible
>    - reg
> +  - "#address-cells"
> +  - "#size-cells"
> +  - ranges
>    - google,pmu-intr-gen-syscon
>
>  additionalProperties: false
> @@ -51,6 +73,25 @@ examples:
>      system-controller@17460000 {
>          compatible = "google,gs101-pmu";
>          reg = <0x17460000 0x10000>;
> +        #address-cells = <1>;
> +        #size-cells = <1>;
> +        ranges;
>
>          google,pmu-intr-gen-syscon = <&pmu_intr_gen>;
> +
> +        pd_g3d: power-domain@1e00 {
> +            compatible = "google,gs101-pd";
> +            reg = <0x1e00 0x80>;
> +            #power-domain-cells = <0>;
> +            label = "g3d";
> +            samsung,dtzpc = <&pd_g3d>;
> +        };
> +
> +        power-domain@2000 {
> +            compatible = "google,gs101-pd";
> +            reg = <0x2000 0x80>;
> +            #power-domain-cells = <0>;
> +            power-domains = <&pd_g3d>;
> +            label = "embedded_g3d";
> +        };
>      };
>
> --
> 2.53.0.473.g4a7958ca14-goog
>