[PATCH RFC 0/5] Marvell PXA1908 power domains

[PATCH RFC 0/5] Marvell PXA1908 power domains

[Duje Mihanović]

Hello,

This series implements support for the power domains found in Marvell's
PXA1908 SoC. The domains control power for the graphics, video and image
processors along with the DSI PHY.

The series is based on master as the MAINTAINERS and device tree patches
depend on the very recently merged initial Marvell PXA1908 support series.
That series can be found at the following link:
https://lore.kernel.org/all/20250708-pxa1908-lkml-v16-0-b4392c484180@dujemihanovic.xyz

Signed-off-by: Duje Mihanović <duje@dujemihanovic.xyz>
---
Duje Mihanović (5):
      dt-bindings: clock: marvell,pxa1908: Add simple-mfd, syscon compatible to apmu
      dt-bindings: power: Add Marvell PXA1908 domains
      pmdomain: marvell: Add PXA1908 power domains
      MAINTAINERS: PXA1908: Add power domain controller
      arm64: dts: marvell: pxa1908: Add power controller

 .../devicetree/bindings/clock/marvell,pxa1908.yaml |  36 ++-
 .../power/marvell,pxa1908-power-controller.yaml    | 105 +++++++
 MAINTAINERS                                        |   5 +
 .../marvell/mmp/pxa1908-samsung-coreprimevelte.dts |   1 +
 arch/arm64/boot/dts/marvell/mmp/pxa1908.dtsi       |  36 ++-
 drivers/pmdomain/Kconfig                           |   1 +
 drivers/pmdomain/Makefile                          |   1 +
 drivers/pmdomain/marvell/Kconfig                   |  16 +
 drivers/pmdomain/marvell/Makefile                  |   3 +
 .../pmdomain/marvell/pxa1908-power-controller.c    | 347 +++++++++++++++++++++
 include/dt-bindings/power/marvell,pxa1908-power.h  |  17 +
 11 files changed, 561 insertions(+), 7 deletions(-)
---
base-commit: cca7a0aae8958c9b1cd14116cb8b2f22ace2205e
change-id: 20250803-pxa1908-genpd-15918db5260c

Best regards,
-- 
Duje Mihanović <duje@dujemihanovic.xyz>

[PATCH RFC 1/5] dt-bindings: clock: marvell,pxa1908: Add simple-mfd, syscon compatible to apmu

[Duje Mihanović]

Add required syscon and simple-mfd compatibles to the APMU controller.
This is required for the SoC's power domain controller as the registers
are shared. The simple-mfd compatible allows devices whose registers are
completely contained in the APMU range (such as the power domain
controller and potentially more) to be children of the clock controller
node.

Also add an optional power-controller child node to the APMU controller
to accommodate the new power domain driver.

Signed-off-by: Duje Mihanović <duje@dujemihanovic.xyz>
---
 .../devicetree/bindings/clock/marvell,pxa1908.yaml | 36 ++++++++++++++++++----
 1 file changed, 30 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/marvell,pxa1908.yaml b/Documentation/devicetree/bindings/clock/marvell,pxa1908.yaml
index 4e78933232b6b925811425f853bedf6e9f01a27d..5e924ebd97e6457191ac021addafd22caba48964 100644
--- a/Documentation/devicetree/bindings/clock/marvell,pxa1908.yaml
+++ b/Documentation/devicetree/bindings/clock/marvell,pxa1908.yaml
@@ -19,11 +19,15 @@ description: |
 
 properties:
   compatible:
-    enum:
-      - marvell,pxa1908-apbc
-      - marvell,pxa1908-apbcp
-      - marvell,pxa1908-mpmu
-      - marvell,pxa1908-apmu
+    oneOf:
+      - enum:
+          - marvell,pxa1908-apbc
+          - marvell,pxa1908-apbcp
+          - marvell,pxa1908-mpmu
+      - items:
+          - const: marvell,pxa1908-apmu
+          - const: simple-mfd
+          - const: syscon
 
   reg:
     maxItems: 1
@@ -31,18 +35,38 @@ properties:
   '#clock-cells':
     const: 1
 
+  power-controller:
+    description: |
+      Optional power domain controller node.
+    type: object
+    additionalProperties: true
+    properties:
+      compatible:
+        const: marvell,pxa1908-power-controller
+
 required:
   - compatible
   - reg
   - '#clock-cells'
 
+allOf:
+  - if:
+      not:
+        properties:
+          compatible:
+            contains:
+              const: marvell,pxa1908-apmu
+    then:
+      properties:
+        power-controller: false
+
 additionalProperties: false
 
 examples:
   # APMU block:
   - |
     clock-controller@d4282800 {
-      compatible = "marvell,pxa1908-apmu";
+      compatible = "marvell,pxa1908-apmu", "simple-mfd", "syscon";
       reg = <0xd4282800 0x400>;
       #clock-cells = <1>;
     };

-- 
2.50.1

Re: [PATCH RFC 1/5] dt-bindings: clock: marvell,pxa1908: Add simple-mfd, syscon compatible to apmu

[Krzysztof Kozlowski]

On Wed, Aug 06, 2025 at 07:33:20PM +0200, Duje Mihanović wrote:
> Add required syscon and simple-mfd compatibles to the APMU controller.
> This is required for the SoC's power domain controller as the registers
> are shared. The simple-mfd compatible allows devices whose registers are
> completely contained in the APMU range (such as the power domain
> controller and potentially more) to be children of the clock controller
> node.
> 
> Also add an optional power-controller child node to the APMU controller
> to accommodate the new power domain driver.
> 
> Signed-off-by: Duje Mihanović <duje@dujemihanovic.xyz>
> ---
>  .../devicetree/bindings/clock/marvell,pxa1908.yaml | 36 ++++++++++++++++++----
>  1 file changed, 30 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/clock/marvell,pxa1908.yaml b/Documentation/devicetree/bindings/clock/marvell,pxa1908.yaml
> index 4e78933232b6b925811425f853bedf6e9f01a27d..5e924ebd97e6457191ac021addafd22caba48964 100644
> --- a/Documentation/devicetree/bindings/clock/marvell,pxa1908.yaml
> +++ b/Documentation/devicetree/bindings/clock/marvell,pxa1908.yaml
> @@ -19,11 +19,15 @@ description: |
>  
>  properties:
>    compatible:
> -    enum:
> -      - marvell,pxa1908-apbc
> -      - marvell,pxa1908-apbcp
> -      - marvell,pxa1908-mpmu
> -      - marvell,pxa1908-apmu
> +    oneOf:
> +      - enum:
> +          - marvell,pxa1908-apbc
> +          - marvell,pxa1908-apbcp
> +          - marvell,pxa1908-mpmu
> +      - items:
> +          - const: marvell,pxa1908-apmu
> +          - const: simple-mfd
> +          - const: syscon
>  
>    reg:
>      maxItems: 1
> @@ -31,18 +35,38 @@ properties:
>    '#clock-cells':
>      const: 1
>  
> +  power-controller:
> +    description: |
> +      Optional power domain controller node.
> +    type: object
> +    additionalProperties: true
> +    properties:
> +      compatible:
> +        const: marvell,pxa1908-power-controller

I commented on next patch, so oly to re-iterate here: no, don't create
nodes just to instantiate drivers. You do not have any resources here.

Best regards,
Krzysztof

[PATCH RFC 2/5] dt-bindings: power: Add Marvell PXA1908 domains

[Duje Mihanović]

Add device tree bindings for Marvell PXA1908's power domains.

Signed-off-by: Duje Mihanović <duje@dujemihanovic.xyz>
---
 .../power/marvell,pxa1908-power-controller.yaml    | 105 +++++++++++++++++++++
 include/dt-bindings/power/marvell,pxa1908-power.h  |  17 ++++
 2 files changed, 122 insertions(+)

diff --git a/Documentation/devicetree/bindings/power/marvell,pxa1908-power-controller.yaml b/Documentation/devicetree/bindings/power/marvell,pxa1908-power-controller.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..1cf3a45d56cbb7b75f7204d656016a9a569da186
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/marvell,pxa1908-power-controller.yaml
@@ -0,0 +1,105 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/power/marvell,pxa1908-power-controller.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Marvell PXA1908 Power Domain Controller
+
+maintainers:
+  - Duje Mihanović <duje@dujemihanovic.xyz>
+
+description: |
+  The Marvell PXA1908 SoC includes multiple power domains which can be powered
+  on/off to save power when different IP cores are not in use.
+
+properties:
+  $nodename:
+    pattern: '^power-controller$'
+
+  compatible:
+    const: marvell,pxa1908-power-controller
+
+  '#power-domain-cells':
+    const: 1
+
+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 0
+
+patternProperties:
+  "^power-domain@[0-9a-f]+$":
+    type: object
+
+    description: |
+      Represents a power domain within the power controller node as documented
+      in Documentation/devicetree/bindings/power/power-domain.yaml.
+
+    properties:
+      reg:
+        description: |
+          Power domain index. Valid values are defined in:
+              "include/dt-bindings/power/marvell,pxa1908-power.h"
+        maxItems: 1
+
+      clocks:
+        description: |
+          A number of phandles to clocks that need to be enabled during domain
+          power up.
+
+      '#power-domain-cells':
+        const: 0
+
+    required:
+      - reg
+
+    unevaluatedProperties: false
+
+required:
+  - compatible
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/power/marvell,pxa1908-power.h>
+
+    clock-controller@d4282800 {
+      compatible = "marvell,pxa1908-apmu", "simple-mfd", "syscon";
+      reg = <0xd4282800 0x400>;
+      #clock-cells = <1>;
+
+      power-controller {
+        compatible = "marvell,pxa1908-power-controller";
+        #address-cells = <1>;
+        #size-cells = <0>;
+        #power-domain-cells = <1>;
+
+        power-domain@PXA1908_POWER_DOMAIN_VPU {
+          reg = <PXA1908_POWER_DOMAIN_VPU>;
+          #power-domain-cells = <0>;
+        };
+
+        power-domain@PXA1908_POWER_DOMAIN_GPU {
+          reg = <PXA1908_POWER_DOMAIN_GPU>;
+          #power-domain-cells = <0>;
+        };
+
+        power-domain@PXA1908_POWER_DOMAIN_GPU2D {
+          reg = <PXA1908_POWER_DOMAIN_GPU2D>;
+          #power-domain-cells = <0>;
+        };
+
+        power-domain@PXA1908_POWER_DOMAIN_DSI {
+          reg = <PXA1908_POWER_DOMAIN_DSI>;
+          #power-domain-cells = <0>;
+        };
+
+        power-domain@PXA1908_POWER_DOMAIN_ISP {
+          reg = <PXA1908_POWER_DOMAIN_ISP>;
+          #power-domain-cells = <0>;
+        };
+      };
+    };
diff --git a/include/dt-bindings/power/marvell,pxa1908-power.h b/include/dt-bindings/power/marvell,pxa1908-power.h
new file mode 100644
index 0000000000000000000000000000000000000000..19b088351af138823505a774ff27203429fe2d97
--- /dev/null
+++ b/include/dt-bindings/power/marvell,pxa1908-power.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
+/*
+ * Marvell PXA1908 power domains
+ *
+ * Copyright 2025, Duje Mihanović <duje@dujemihanovic.xyz>
+ */
+
+#ifndef __DTS_MARVELL_PXA1908_POWER_H
+#define __DTS_MARVELL_PXA1908_POWER_H
+
+#define PXA1908_POWER_DOMAIN_VPU	0
+#define PXA1908_POWER_DOMAIN_GPU	1
+#define PXA1908_POWER_DOMAIN_GPU2D	2
+#define PXA1908_POWER_DOMAIN_DSI	3
+#define PXA1908_POWER_DOMAIN_ISP	4
+
+#endif

-- 
2.50.1

Re: [PATCH RFC 2/5] dt-bindings: power: Add Marvell PXA1908 domains

[Krzysztof Kozlowski]

On Wed, Aug 06, 2025 at 07:33:21PM +0200, Duje Mihanović wrote:
> Add device tree bindings for Marvell PXA1908's power domains.
> 
> Signed-off-by: Duje Mihanović <duje@dujemihanovic.xyz>
> ---
>  .../power/marvell,pxa1908-power-controller.yaml    | 105 +++++++++++++++++++++
>  include/dt-bindings/power/marvell,pxa1908-power.h  |  17 ++++
>  2 files changed, 122 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/power/marvell,pxa1908-power-controller.yaml b/Documentation/devicetree/bindings/power/marvell,pxa1908-power-controller.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..1cf3a45d56cbb7b75f7204d656016a9a569da186
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/marvell,pxa1908-power-controller.yaml
> @@ -0,0 +1,105 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/power/marvell,pxa1908-power-controller.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Marvell PXA1908 Power Domain Controller
> +
> +maintainers:
> +  - Duje Mihanović <duje@dujemihanovic.xyz>
> +
> +description: |
> +  The Marvell PXA1908 SoC includes multiple power domains which can be powered
> +  on/off to save power when different IP cores are not in use.
> +
> +properties:
> +  $nodename:
> +    pattern: '^power-controller$'
> +
> +  compatible:
> +    const: marvell,pxa1908-power-controller
> +
> +  '#power-domain-cells':
> +    const: 1

So this is power domain controller?

> +
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 0
> +
> +patternProperties:
> +  "^power-domain@[0-9a-f]+$":
> +    type: object

And this is as well?

You duplicated them.

> +
> +    description: |
> +      Represents a power domain within the power controller node as documented
> +      in Documentation/devicetree/bindings/power/power-domain.yaml.

We do not represent individual power domains, just like we do not
represent individual clocks.



> +
> +    properties:
> +      reg:
> +        description: |
> +          Power domain index. Valid values are defined in:
> +              "include/dt-bindings/power/marvell,pxa1908-power.h"
> +        maxItems: 1

So no address space, thus this is not a separate device node.

> +
> +      clocks:
> +        description: |

Drop everywhere |

> +          A number of phandles to clocks that need to be enabled during domain
> +          power up.

This does not exist in your example, so it is just confusing.

> +
> +      '#power-domain-cells':
> +        const: 0
> +
> +    required:
> +      - reg
> +
> +    unevaluatedProperties: false
> +
> +required:
> +  - compatible
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/power/marvell,pxa1908-power.h>
> +
> +    clock-controller@d4282800 {
> +      compatible = "marvell,pxa1908-apmu", "simple-mfd", "syscon";
> +      reg = <0xd4282800 0x400>;
> +      #clock-cells = <1>;
> +
> +      power-controller {
> +        compatible = "marvell,pxa1908-power-controller";

No address space, so this should be folded into the parent.

Best regards,
Krzysztof

Re: [PATCH RFC 2/5] dt-bindings: power: Add Marvell PXA1908 domains

[Duje Mihanović]

On Friday, 8 August 2025 09:34:54 Central European Summer Time Krzysztof Kozlowski wrote:
> On Wed, Aug 06, 2025 at 07:33:21PM +0200, Duje Mihanović wrote:
> > +          A number of phandles to clocks that need to be enabled during
> > domain +          power up.
> 
> This does not exist in your example, so it is just confusing.

This is because I have not implemented any of the clocks used by the
domains at this moment.

Actually, I am not sure anymore whether it is necessary to assign
clocks to the domains as I have just yesterday successfully brought up
the GPU with some out-of-tree code and that did not require giving the
domains any clocks even though the vendor kernel does this. Should I
just go with that and drop all clock handling from the power domain
driver, at which point there would be no need for the individual domain
nodes? If not, how should I in the future assign clocks to the domains?

> > +examples:
> > +  - |
> > +    #include <dt-bindings/power/marvell,pxa1908-power.h>
> > +
> > +    clock-controller@d4282800 {
> > +      compatible = "marvell,pxa1908-apmu", "simple-mfd", "syscon";
> > +      reg = <0xd4282800 0x400>;
> > +      #clock-cells = <1>;
> > +
> > +      power-controller {
> > +        compatible = "marvell,pxa1908-power-controller";
> 
> No address space, so this should be folded into the parent.

By this, do you mean that the clock driver registers the power domain
controller through devm_mfd_add_devices()?

Regards,
--
Duje

Re: [PATCH RFC 2/5] dt-bindings: power: Add Marvell PXA1908 domains

[Krzysztof Kozlowski]

On 08/08/2025 21:46, Duje Mihanović wrote:
> On Friday, 8 August 2025 09:34:54 Central European Summer Time Krzysztof Kozlowski wrote:
>> On Wed, Aug 06, 2025 at 07:33:21PM +0200, Duje Mihanović wrote:
>>> +          A number of phandles to clocks that need to be enabled during
>>> domain +          power up.
>>
>> This does not exist in your example, so it is just confusing.
> 
> This is because I have not implemented any of the clocks used by the
> domains at this moment.
> 
> Actually, I am not sure anymore whether it is necessary to assign
> clocks to the domains as I have just yesterday successfully brought up
> the GPU with some out-of-tree code and that did not require giving the
> domains any clocks even though the vendor kernel does this. Should I
> just go with that and drop all clock handling from the power domain
> driver, at which point there would be no need for the individual domain
> nodes? If not, how should I in the future assign clocks to the domains?

I am asking to see complete binding with complete DTS in example and
submitted to SoC maintainer.

I did not comment on drivers. This is not a driver patch.

> 
>>> +examples:
>>> +  - |
>>> +    #include <dt-bindings/power/marvell,pxa1908-power.h>
>>> +
>>> +    clock-controller@d4282800 {
>>> +      compatible = "marvell,pxa1908-apmu", "simple-mfd", "syscon";
>>> +      reg = <0xd4282800 0x400>;
>>> +      #clock-cells = <1>;
>>> +
>>> +      power-controller {
>>> +        compatible = "marvell,pxa1908-power-controller";
>>
>> No address space, so this should be folded into the parent.
> 
> By this, do you mean that the clock driver registers the power domain
> controller through devm_mfd_add_devices()?


There are multiple ways this is being solved but NONE of them are
binding ways. You again bring driver into bindings discussion.


Best regards,
Krzysztof

Re: [PATCH RFC 2/5] dt-bindings: power: Add Marvell PXA1908 domains

[Duje Mihanović]

On Monday, 11 August 2025 08:38:15 Central European Summer Time 
Krzysztof Kozlowski wrote:
> On 08/08/2025 21:46, Duje Mihanović wrote:
> > On Friday, 8 August 2025 09:34:54 Central European Summer Time Krzysztof 
> > Kozlowski wrote:
> >> On Wed, Aug 06, 2025 at 07:33:21PM +0200, Duje Mihanović wrote:
> >>> +          A number of phandles to clocks that need to be enabled during
> >>> domain +          power up.
> >> 
> >> This does not exist in your example, so it is just confusing.
> > 
> > This is because I have not implemented any of the clocks used by the
> > domains at this moment.
> > 
> > Actually, I am not sure anymore whether it is necessary to assign
> > clocks to the domains as I have just yesterday successfully brought up
> > the GPU with some out-of-tree code and that did not require giving the
> > domains any clocks even though the vendor kernel does this. Should I
> > just go with that and drop all clock handling from the power domain
> > driver, at which point there would be no need for the individual domain
> > nodes? If not, how should I in the future assign clocks to the domains?
> 
> I am asking to see complete binding with complete DTS in example and
> submitted to SoC maintainer.

Hm, so if in the example (and the actual DTS) each domain is assigned a clock, 
can I then keep the domain and domain controller nodes like Mediatek and 
Rockchip have?

Does SoC maintainer here mean the SoC mailing list or the maintainer of the 
particular SoC family in question?

Regards,
--
Duje

Re: [PATCH RFC 2/5] dt-bindings: power: Add Marvell PXA1908 domains

[Krzysztof Kozlowski]

On 15/08/2025 00:08, Duje Mihanović wrote:
> On Monday, 11 August 2025 08:38:15 Central European Summer Time 
> Krzysztof Kozlowski wrote:
>> On 08/08/2025 21:46, Duje Mihanović wrote:
>>> On Friday, 8 August 2025 09:34:54 Central European Summer Time Krzysztof 
>>> Kozlowski wrote:
>>>> On Wed, Aug 06, 2025 at 07:33:21PM +0200, Duje Mihanović wrote:
>>>>> +          A number of phandles to clocks that need to be enabled during
>>>>> domain +          power up.
>>>>
>>>> This does not exist in your example, so it is just confusing.
>>>
>>> This is because I have not implemented any of the clocks used by the
>>> domains at this moment.
>>>
>>> Actually, I am not sure anymore whether it is necessary to assign
>>> clocks to the domains as I have just yesterday successfully brought up
>>> the GPU with some out-of-tree code and that did not require giving the
>>> domains any clocks even though the vendor kernel does this. Should I
>>> just go with that and drop all clock handling from the power domain
>>> driver, at which point there would be no need for the individual domain
>>> nodes? If not, how should I in the future assign clocks to the domains?
>>
>> I am asking to see complete binding with complete DTS in example and
>> submitted to SoC maintainer.
> 
> Hm, so if in the example (and the actual DTS) each domain is assigned a clock, 
> can I then keep the domain and domain controller nodes like Mediatek and 
> Rockchip have?

You would need to point me to specific files or show some code.

> 
> Does SoC maintainer here mean the SoC mailing list or the maintainer of the 
> particular SoC family in question?

I meant rather post complete DTS to mailing lists (so maintainer of
given SoC family can see it as well), does not have to be the same patchset.



Best regards,
Krzysztof

Re: [PATCH RFC 2/5] dt-bindings: power: Add Marvell PXA1908 domains

[Duje Mihanović]

On Friday, 15 August 2025 08:08:24 Central European Summer Time Krzysztof Kozlowski wrote:
> On 15/08/2025 00:08, Duje Mihanović wrote:
> > > I am asking to see complete binding with complete DTS in example and
> > > submitted to SoC maintainer.
> > 
> > Hm, so if in the example (and the actual DTS) each domain is assigned a
> > clock, can I then keep the domain and domain controller nodes like Mediatek
> > and Rockchip have?
> 
> You would need to point me to specific files or show some code.

Sure, mediatek,power-controller.yaml and rockchip,power-controller.yaml
in Documentation/devicetree/bindings/power.

Regards,
--
Duje

Re: [PATCH RFC 2/5] dt-bindings: power: Add Marvell PXA1908 domains

[Krzysztof Kozlowski]

On 16/08/2025 17:13, Duje Mihanović wrote:
> On Friday, 15 August 2025 08:08:24 Central European Summer Time Krzysztof Kozlowski wrote:
>> On 15/08/2025 00:08, Duje Mihanović wrote:
>>>> I am asking to see complete binding with complete DTS in example and
>>>> submitted to SoC maintainer.
>>>
>>> Hm, so if in the example (and the actual DTS) each domain is assigned a
>>> clock, can I then keep the domain and domain controller nodes like Mediatek
>>> and Rockchip have?
>>
>> You would need to point me to specific files or show some code.
> 
> Sure, mediatek,power-controller.yaml and rockchip,power-controller.yaml
> in Documentation/devicetree/bindings/power.

I see, but your DTS is nothing like that.

Best regards,
Krzysztof

[PATCH RFC 3/5] pmdomain: marvell: Add PXA1908 power domains

[Duje Mihanović]

Marvell's PXA1908 SoC has a few power domains for its VPU, GPU, image
processor and DSI PHY. Add a driver to control these.

Signed-off-by: Duje Mihanović <duje@dujemihanovic.xyz>
---
 drivers/pmdomain/Kconfig                           |   1 +
 drivers/pmdomain/Makefile                          |   1 +
 drivers/pmdomain/marvell/Kconfig                   |  16 +
 drivers/pmdomain/marvell/Makefile                  |   3 +
 .../pmdomain/marvell/pxa1908-power-controller.c    | 347 +++++++++++++++++++++
 5 files changed, 368 insertions(+)

diff --git a/drivers/pmdomain/Kconfig b/drivers/pmdomain/Kconfig
index 91f04ace35d4b024fafdf6af4e26a179640eb82f..23076ae90e6641dea8e5dbc851d041cd7929cee6 100644
--- a/drivers/pmdomain/Kconfig
+++ b/drivers/pmdomain/Kconfig
@@ -7,6 +7,7 @@ source "drivers/pmdomain/apple/Kconfig"
 source "drivers/pmdomain/arm/Kconfig"
 source "drivers/pmdomain/bcm/Kconfig"
 source "drivers/pmdomain/imx/Kconfig"
+source "drivers/pmdomain/marvell/Kconfig"
 source "drivers/pmdomain/mediatek/Kconfig"
 source "drivers/pmdomain/qcom/Kconfig"
 source "drivers/pmdomain/renesas/Kconfig"
diff --git a/drivers/pmdomain/Makefile b/drivers/pmdomain/Makefile
index 7030f44a49df9e91b1c9d1b6d12690a6248671fb..ebc802f13eb953db750f5a9507caa64c637a957a 100644
--- a/drivers/pmdomain/Makefile
+++ b/drivers/pmdomain/Makefile
@@ -5,6 +5,7 @@ obj-y					+= apple/
 obj-y					+= arm/
 obj-y					+= bcm/
 obj-y					+= imx/
+obj-y					+= marvell/
 obj-y					+= mediatek/
 obj-y					+= qcom/
 obj-y					+= renesas/
diff --git a/drivers/pmdomain/marvell/Kconfig b/drivers/pmdomain/marvell/Kconfig
new file mode 100644
index 0000000000000000000000000000000000000000..be2036726cc563ba2a3d1a82ca24763e2148fec2
--- /dev/null
+++ b/drivers/pmdomain/marvell/Kconfig
@@ -0,0 +1,16 @@
+# SPDX-License-Identifier: GPL-2.0-only
+
+menu "Marvell PM Domains"
+	depends on ARCH_MMP || COMPILE_TEST
+
+config PXA1908_PM_DOMAINS
+	tristate "Marvell PXA1908 power domains"
+	depends on OF
+	depends on PM
+	default ARCH_MMP && ARM64
+	select REGMAP
+	select PM_GENERIC_DOMAINS
+	help
+	  Say Y here to enable support for Marvell PXA1908's power domains.
+
+endmenu
diff --git a/drivers/pmdomain/marvell/Makefile b/drivers/pmdomain/marvell/Makefile
new file mode 100644
index 0000000000000000000000000000000000000000..6163bcbcb00ca7256e4c893117b7443b6fb195e7
--- /dev/null
+++ b/drivers/pmdomain/marvell/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0-only
+
+obj-$(CONFIG_PXA1908_PM_DOMAINS)	+= pxa1908-power-controller.o
diff --git a/drivers/pmdomain/marvell/pxa1908-power-controller.c b/drivers/pmdomain/marvell/pxa1908-power-controller.c
new file mode 100644
index 0000000000000000000000000000000000000000..a8940e6dc2eaad2b14e9e6d8aa875c11e114b9dd
--- /dev/null
+++ b/drivers/pmdomain/marvell/pxa1908-power-controller.c
@@ -0,0 +1,347 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2025 Duje Mihanović <duje@dujemihanovic.xyz>
+ */
+
+#include <linux/clk.h>
+#include <linux/container_of.h>
+#include <linux/mfd/syscon.h>
+#include <linux/mod_devicetable.h>
+#include <linux/of.h>
+#include <linux/of_clk.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/pm_domain.h>
+#include <linux/regmap.h>
+#include <linux/units.h>
+
+#include <dt-bindings/power/marvell,pxa1908-power.h>
+
+/* VPU, GPU, ISP */
+#define APMU_PWR_CTRL_REG	0xd8
+#define APMU_PWR_BLK_TMR_REG	0xdc
+#define APMU_PWR_STATUS_REG	0xf0
+
+/* DSI */
+#define APMU_DEBUG		0x88
+#define DSI_PHY_DVM_MASK	BIT(31)
+
+#define POWER_ON_LATENCY_US	300
+#define POWER_OFF_LATENCY_US	20
+
+struct pxa1908_pd_ctrl {
+	struct genpd_onecell_data onecell_data;
+	struct regmap *base;
+	struct generic_pm_domain *domains[];
+};
+
+struct pxa1908_pd_data {
+	u32 reg_clk_res_ctrl;
+	u32 hw_mode;
+	u32 pwr_state;
+	bool keep_on;
+	int id;
+};
+
+struct pxa1908_pd {
+	const struct pxa1908_pd_data data;
+	struct generic_pm_domain genpd;
+	struct clk_bulk_data *clks;
+	struct device *dev;
+	bool initialized;
+	int num_clks;
+};
+
+static bool pxa1908_pd_is_on(struct pxa1908_pd *pd)
+{
+	struct pxa1908_pd_ctrl *ctrl = dev_get_drvdata(pd->dev);
+
+	return regmap_test_bits(ctrl->base, APMU_PWR_STATUS_REG, pd->data.pwr_state);
+}
+
+static int pxa1908_pd_power_on(struct generic_pm_domain *genpd)
+{
+	struct pxa1908_pd *pd = container_of(genpd, struct pxa1908_pd, genpd);
+	struct pxa1908_pd_ctrl *ctrl = dev_get_drvdata(pd->dev);
+	const struct pxa1908_pd_data *data = &pd->data;
+	unsigned int status;
+	int ret = 0;
+
+	if (pd->clks)
+		ret = clk_bulk_prepare_enable(pd->num_clks, pd->clks);
+
+	regmap_set_bits(ctrl->base, data->reg_clk_res_ctrl, data->hw_mode);
+	if (data->id != PXA1908_POWER_DOMAIN_ISP)
+		regmap_write(ctrl->base, APMU_PWR_BLK_TMR_REG, 0x20001fff);
+	regmap_set_bits(ctrl->base, APMU_PWR_CTRL_REG, data->pwr_state);
+
+	usleep_range(POWER_ON_LATENCY_US, POWER_ON_LATENCY_US * 2);
+
+	ret = regmap_read_poll_timeout(ctrl->base, APMU_PWR_STATUS_REG, status,
+				       status & data->pwr_state, 6, 25 * USEC_PER_MSEC);
+	if (ret == -ETIMEDOUT)
+		dev_err(pd->dev, "timed out powering on domain '%s'\n", pd->genpd.name);
+
+	if (pd->clks)
+		clk_bulk_disable_unprepare(pd->num_clks, pd->clks);
+
+	return ret;
+}
+
+static int pxa1908_pd_power_off(struct generic_pm_domain *genpd)
+{
+	struct pxa1908_pd *pd = container_of(genpd, struct pxa1908_pd, genpd);
+	struct pxa1908_pd_ctrl *ctrl = dev_get_drvdata(pd->dev);
+	const struct pxa1908_pd_data *data = &pd->data;
+	unsigned int status;
+	int ret;
+
+	regmap_clear_bits(ctrl->base, APMU_PWR_CTRL_REG, data->pwr_state);
+
+	usleep_range(POWER_OFF_LATENCY_US, POWER_OFF_LATENCY_US * 2);
+
+	ret = regmap_read_poll_timeout(ctrl->base, APMU_PWR_STATUS_REG, status,
+				       !(status & data->pwr_state), 6, 25 * USEC_PER_MSEC);
+	if (ret == -ETIMEDOUT) {
+		dev_err(pd->dev, "timed out powering off domain '%s'\n", pd->genpd.name);
+		return ret;
+	}
+
+	regmap_clear_bits(ctrl->base, data->reg_clk_res_ctrl, data->hw_mode);
+
+	return 0;
+}
+
+static int pxa1908_dsi_power_on(struct generic_pm_domain *genpd)
+{
+	struct pxa1908_pd *pd = container_of(genpd, struct pxa1908_pd, genpd);
+	struct pxa1908_pd_ctrl *ctrl = dev_get_drvdata(pd->dev);
+
+	if (pd->clks) {
+		int ret = clk_bulk_prepare_enable(pd->num_clks, pd->clks);
+
+		if (ret) {
+			dev_err(pd->dev, "failed to enable clocks for domain '%s': %d\n",
+				pd->genpd.name, ret);
+			return ret;
+		}
+	}
+
+	regmap_set_bits(ctrl->base, APMU_DEBUG, DSI_PHY_DVM_MASK);
+
+	return 0;
+}
+
+static int pxa1908_dsi_power_off(struct generic_pm_domain *genpd)
+{
+	struct pxa1908_pd *pd = container_of(genpd, struct pxa1908_pd, genpd);
+	struct pxa1908_pd_ctrl *ctrl = dev_get_drvdata(pd->dev);
+
+	regmap_clear_bits(ctrl->base, APMU_DEBUG, DSI_PHY_DVM_MASK);
+
+	if (pd->clks)
+		clk_bulk_disable_unprepare(pd->num_clks, pd->clks);
+
+	return 0;
+}
+
+#define DOMAIN(_id, _name, ctrl, mode, state) \
+	[_id] = { \
+		.data = { \
+			.reg_clk_res_ctrl = ctrl, \
+			.hw_mode = BIT(mode), \
+			.pwr_state = BIT(state), \
+			.id = _id, \
+		}, \
+		.genpd = { \
+			.name = _name, \
+			.power_on = pxa1908_pd_power_on, \
+			.power_off = pxa1908_pd_power_off, \
+		}, \
+	}
+
+static struct pxa1908_pd domains[] = {
+	DOMAIN(PXA1908_POWER_DOMAIN_VPU, "vpu", 0xa4, 19, 2),
+	DOMAIN(PXA1908_POWER_DOMAIN_GPU, "gpu", 0xcc, 11, 0),
+	DOMAIN(PXA1908_POWER_DOMAIN_GPU2D, "gpu2d", 0xf4, 11, 6),
+	DOMAIN(PXA1908_POWER_DOMAIN_ISP, "isp", 0x38, 15, 4),
+	[PXA1908_POWER_DOMAIN_DSI] = {
+		.genpd = {
+			.name = "dsi",
+			.power_on = pxa1908_dsi_power_on,
+			.power_off = pxa1908_dsi_power_off,
+			/*
+			 * TODO: There is no DSI driver written yet and until then we probably
+			 * don't want to power off the DSI PHY ever.
+			 */
+			.flags = GENPD_FLAG_ALWAYS_ON,
+		},
+		.data = {
+			/* See above. */
+			.keep_on = true,
+		},
+	},
+};
+
+static void pxa1908_pd_cleanup(struct pxa1908_pd_ctrl *ctrl)
+{
+	struct pxa1908_pd *pd;
+	int ret;
+
+	for (int i = ARRAY_SIZE(domains) - 1; i >= 0; i--) {
+		pd = &domains[i];
+
+		if (!pd->initialized)
+			continue;
+
+		ret = pm_genpd_remove(&pd->genpd);
+		if (ret)
+			dev_err(pd->dev, "failed to remove domain '%s': %d\n",
+				pd->genpd.name, ret);
+		if (pxa1908_pd_is_on(pd) && !pd->data.keep_on)
+			pxa1908_pd_power_off(&pd->genpd);
+
+		clk_bulk_put_all(pd->num_clks, pd->clks);
+	}
+}
+
+static int
+pxa1908_pd_init(struct pxa1908_pd_ctrl *ctrl, struct device_node *node, struct device *dev)
+{
+	struct pxa1908_pd *pd;
+	int clk_idx = 0, ret;
+	u32 id;
+
+	ret = of_property_read_u32(node, "reg", &id);
+	if (ret) {
+		dev_err(dev, "failed to get domain id from reg: %d\n", ret);
+		return ret;
+	}
+
+	if (id >= ARRAY_SIZE(domains)) {
+		dev_err(dev, "invalid domain id %d\n", id);
+		return ret;
+	}
+
+	pd = &domains[id];
+	pd->dev = dev;
+	pd->num_clks = of_clk_get_parent_count(node);
+	ctrl->domains[id] = &pd->genpd;
+
+	if (pd->num_clks > 0) {
+		pd->clks = devm_kcalloc(dev, pd->num_clks, sizeof(*pd->clks), GFP_KERNEL);
+		if (!pd->clks)
+			return -ENOMEM;
+	}
+
+	for (int i = 0; i < pd->num_clks; i++) {
+		struct clk *clk = of_clk_get(node, i);
+
+		if (IS_ERR(clk)) {
+			ret = PTR_ERR(clk);
+			dev_err(dev, "failed to get clk for domain '%s': %d\n",
+				pd->genpd.name, ret);
+			goto err;
+		}
+
+		pd->clks[clk_idx++].clk = clk;
+	}
+
+	/* Make sure the state of the hardware is synced with the domain table above. */
+	if (pd->data.keep_on) {
+		ret = pd->genpd.power_on(&pd->genpd);
+		if (ret) {
+			dev_err(dev, "failed to power on domain '%s': %d\n", pd->genpd.name, ret);
+			goto err;
+		}
+	} else {
+		if (pxa1908_pd_is_on(pd)) {
+			dev_warn(dev,
+				 "domain '%s' is on despite being default off; powering off\n",
+				 pd->genpd.name);
+
+			ret = pxa1908_pd_power_off(&pd->genpd);
+			if (ret) {
+				dev_err(dev, "failed to power off domain '%s': %d\n",
+					pd->genpd.name, ret);
+				goto err;
+			}
+		}
+	}
+
+	ret = pm_genpd_init(&pd->genpd, NULL, !pd->data.keep_on);
+	if (ret) {
+		dev_err(dev, "domain '%s' failed to initialize: %d\n", pd->genpd.name, ret);
+		goto err;
+	}
+
+	pd->initialized = true;
+
+	return 0;
+
+err:
+	clk_bulk_put_all(pd->num_clks, pd->clks);
+	return ret;
+}
+
+static int pxa1908_pd_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct pxa1908_pd_ctrl *ctrl;
+	struct device_node *node;
+	int ret;
+
+	ctrl = devm_kzalloc(dev, struct_size(ctrl, domains, ARRAY_SIZE(domains)), GFP_KERNEL);
+	if (!ctrl)
+		return -ENOMEM;
+
+	ctrl->base = syscon_node_to_regmap(dev->parent->of_node);
+	if (IS_ERR(ctrl->base)) {
+		dev_err(dev, "no regmap available\n");
+		return PTR_ERR(ctrl->base);
+	}
+
+	platform_set_drvdata(pdev, ctrl);
+
+	ctrl->onecell_data.domains = ctrl->domains;
+	ctrl->onecell_data.num_domains = ARRAY_SIZE(domains);
+
+	for_each_available_child_of_node(dev->of_node, node) {
+		ret = pxa1908_pd_init(ctrl, node, dev);
+		if (ret)
+			goto err;
+	}
+
+	return of_genpd_add_provider_onecell(dev->of_node, &ctrl->onecell_data);
+
+err:
+	pxa1908_pd_cleanup(ctrl);
+	return ret;
+}
+
+static void pxa1908_pd_remove(struct platform_device *pdev)
+{
+	pxa1908_pd_cleanup(platform_get_drvdata(pdev));
+}
+
+static const struct of_device_id pxa1908_pd_match[] = {
+	{
+		.compatible = "marvell,pxa1908-power-controller",
+	},
+	{ }
+};
+MODULE_DEVICE_TABLE(of, pxa1908_pd_match);
+
+static struct platform_driver pxa1908_pd_driver = {
+	.probe = pxa1908_pd_probe,
+	.remove = pxa1908_pd_remove,
+	.driver = {
+		.name = "pxa1908-power-controller",
+		.of_match_table = pxa1908_pd_match,
+	},
+};
+module_platform_driver(pxa1908_pd_driver);
+
+MODULE_AUTHOR("Duje Mihanović <duje@dujemihanovic.xyz>");
+MODULE_DESCRIPTION("Marvell PXA1908 power domain driver");
+MODULE_LICENSE("GPL");

-- 
2.50.1

Re: [PATCH RFC 3/5] pmdomain: marvell: Add PXA1908 power domains

[Ulf Hansson]

On Wed, 6 Aug 2025 at 19:35, Duje Mihanović <duje@dujemihanovic.xyz> wrote:
>
> Marvell's PXA1908 SoC has a few power domains for its VPU, GPU, image
> processor and DSI PHY. Add a driver to control these.
>
> Signed-off-by: Duje Mihanović <duje@dujemihanovic.xyz>

FYI, overall this looks okay to me, but I will review your next
version more in detail.

Kind regards
Uffe

> ---
>  drivers/pmdomain/Kconfig                           |   1 +
>  drivers/pmdomain/Makefile                          |   1 +
>  drivers/pmdomain/marvell/Kconfig                   |  16 +
>  drivers/pmdomain/marvell/Makefile                  |   3 +
>  .../pmdomain/marvell/pxa1908-power-controller.c    | 347 +++++++++++++++++++++
>  5 files changed, 368 insertions(+)
>
> diff --git a/drivers/pmdomain/Kconfig b/drivers/pmdomain/Kconfig
> index 91f04ace35d4b024fafdf6af4e26a179640eb82f..23076ae90e6641dea8e5dbc851d041cd7929cee6 100644
> --- a/drivers/pmdomain/Kconfig
> +++ b/drivers/pmdomain/Kconfig
> @@ -7,6 +7,7 @@ source "drivers/pmdomain/apple/Kconfig"
>  source "drivers/pmdomain/arm/Kconfig"
>  source "drivers/pmdomain/bcm/Kconfig"
>  source "drivers/pmdomain/imx/Kconfig"
> +source "drivers/pmdomain/marvell/Kconfig"
>  source "drivers/pmdomain/mediatek/Kconfig"
>  source "drivers/pmdomain/qcom/Kconfig"
>  source "drivers/pmdomain/renesas/Kconfig"
> diff --git a/drivers/pmdomain/Makefile b/drivers/pmdomain/Makefile
> index 7030f44a49df9e91b1c9d1b6d12690a6248671fb..ebc802f13eb953db750f5a9507caa64c637a957a 100644
> --- a/drivers/pmdomain/Makefile
> +++ b/drivers/pmdomain/Makefile
> @@ -5,6 +5,7 @@ obj-y                                   += apple/
>  obj-y                                  += arm/
>  obj-y                                  += bcm/
>  obj-y                                  += imx/
> +obj-y                                  += marvell/
>  obj-y                                  += mediatek/
>  obj-y                                  += qcom/
>  obj-y                                  += renesas/
> diff --git a/drivers/pmdomain/marvell/Kconfig b/drivers/pmdomain/marvell/Kconfig
> new file mode 100644
> index 0000000000000000000000000000000000000000..be2036726cc563ba2a3d1a82ca24763e2148fec2
> --- /dev/null
> +++ b/drivers/pmdomain/marvell/Kconfig
> @@ -0,0 +1,16 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +
> +menu "Marvell PM Domains"
> +       depends on ARCH_MMP || COMPILE_TEST
> +
> +config PXA1908_PM_DOMAINS
> +       tristate "Marvell PXA1908 power domains"
> +       depends on OF
> +       depends on PM
> +       default ARCH_MMP && ARM64
> +       select REGMAP
> +       select PM_GENERIC_DOMAINS
> +       help
> +         Say Y here to enable support for Marvell PXA1908's power domains.
> +
> +endmenu
> diff --git a/drivers/pmdomain/marvell/Makefile b/drivers/pmdomain/marvell/Makefile
> new file mode 100644
> index 0000000000000000000000000000000000000000..6163bcbcb00ca7256e4c893117b7443b6fb195e7
> --- /dev/null
> +++ b/drivers/pmdomain/marvell/Makefile
> @@ -0,0 +1,3 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +
> +obj-$(CONFIG_PXA1908_PM_DOMAINS)       += pxa1908-power-controller.o
> diff --git a/drivers/pmdomain/marvell/pxa1908-power-controller.c b/drivers/pmdomain/marvell/pxa1908-power-controller.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..a8940e6dc2eaad2b14e9e6d8aa875c11e114b9dd
> --- /dev/null
> +++ b/drivers/pmdomain/marvell/pxa1908-power-controller.c
> @@ -0,0 +1,347 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright 2025 Duje Mihanović <duje@dujemihanovic.xyz>
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/container_of.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/of.h>
> +#include <linux/of_clk.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
> +#include <linux/regmap.h>
> +#include <linux/units.h>
> +
> +#include <dt-bindings/power/marvell,pxa1908-power.h>
> +
> +/* VPU, GPU, ISP */
> +#define APMU_PWR_CTRL_REG      0xd8
> +#define APMU_PWR_BLK_TMR_REG   0xdc
> +#define APMU_PWR_STATUS_REG    0xf0
> +
> +/* DSI */
> +#define APMU_DEBUG             0x88
> +#define DSI_PHY_DVM_MASK       BIT(31)
> +
> +#define POWER_ON_LATENCY_US    300
> +#define POWER_OFF_LATENCY_US   20
> +
> +struct pxa1908_pd_ctrl {
> +       struct genpd_onecell_data onecell_data;
> +       struct regmap *base;
> +       struct generic_pm_domain *domains[];
> +};
> +
> +struct pxa1908_pd_data {
> +       u32 reg_clk_res_ctrl;
> +       u32 hw_mode;
> +       u32 pwr_state;
> +       bool keep_on;
> +       int id;
> +};
> +
> +struct pxa1908_pd {
> +       const struct pxa1908_pd_data data;
> +       struct generic_pm_domain genpd;
> +       struct clk_bulk_data *clks;
> +       struct device *dev;
> +       bool initialized;
> +       int num_clks;
> +};
> +
> +static bool pxa1908_pd_is_on(struct pxa1908_pd *pd)
> +{
> +       struct pxa1908_pd_ctrl *ctrl = dev_get_drvdata(pd->dev);
> +
> +       return regmap_test_bits(ctrl->base, APMU_PWR_STATUS_REG, pd->data.pwr_state);
> +}
> +
> +static int pxa1908_pd_power_on(struct generic_pm_domain *genpd)
> +{
> +       struct pxa1908_pd *pd = container_of(genpd, struct pxa1908_pd, genpd);
> +       struct pxa1908_pd_ctrl *ctrl = dev_get_drvdata(pd->dev);
> +       const struct pxa1908_pd_data *data = &pd->data;
> +       unsigned int status;
> +       int ret = 0;
> +
> +       if (pd->clks)
> +               ret = clk_bulk_prepare_enable(pd->num_clks, pd->clks);
> +
> +       regmap_set_bits(ctrl->base, data->reg_clk_res_ctrl, data->hw_mode);
> +       if (data->id != PXA1908_POWER_DOMAIN_ISP)
> +               regmap_write(ctrl->base, APMU_PWR_BLK_TMR_REG, 0x20001fff);
> +       regmap_set_bits(ctrl->base, APMU_PWR_CTRL_REG, data->pwr_state);
> +
> +       usleep_range(POWER_ON_LATENCY_US, POWER_ON_LATENCY_US * 2);
> +
> +       ret = regmap_read_poll_timeout(ctrl->base, APMU_PWR_STATUS_REG, status,
> +                                      status & data->pwr_state, 6, 25 * USEC_PER_MSEC);
> +       if (ret == -ETIMEDOUT)
> +               dev_err(pd->dev, "timed out powering on domain '%s'\n", pd->genpd.name);
> +
> +       if (pd->clks)
> +               clk_bulk_disable_unprepare(pd->num_clks, pd->clks);
> +
> +       return ret;
> +}
> +
> +static int pxa1908_pd_power_off(struct generic_pm_domain *genpd)
> +{
> +       struct pxa1908_pd *pd = container_of(genpd, struct pxa1908_pd, genpd);
> +       struct pxa1908_pd_ctrl *ctrl = dev_get_drvdata(pd->dev);
> +       const struct pxa1908_pd_data *data = &pd->data;
> +       unsigned int status;
> +       int ret;
> +
> +       regmap_clear_bits(ctrl->base, APMU_PWR_CTRL_REG, data->pwr_state);
> +
> +       usleep_range(POWER_OFF_LATENCY_US, POWER_OFF_LATENCY_US * 2);
> +
> +       ret = regmap_read_poll_timeout(ctrl->base, APMU_PWR_STATUS_REG, status,
> +                                      !(status & data->pwr_state), 6, 25 * USEC_PER_MSEC);
> +       if (ret == -ETIMEDOUT) {
> +               dev_err(pd->dev, "timed out powering off domain '%s'\n", pd->genpd.name);
> +               return ret;
> +       }
> +
> +       regmap_clear_bits(ctrl->base, data->reg_clk_res_ctrl, data->hw_mode);
> +
> +       return 0;
> +}
> +
> +static int pxa1908_dsi_power_on(struct generic_pm_domain *genpd)
> +{
> +       struct pxa1908_pd *pd = container_of(genpd, struct pxa1908_pd, genpd);
> +       struct pxa1908_pd_ctrl *ctrl = dev_get_drvdata(pd->dev);
> +
> +       if (pd->clks) {
> +               int ret = clk_bulk_prepare_enable(pd->num_clks, pd->clks);
> +
> +               if (ret) {
> +                       dev_err(pd->dev, "failed to enable clocks for domain '%s': %d\n",
> +                               pd->genpd.name, ret);
> +                       return ret;
> +               }
> +       }
> +
> +       regmap_set_bits(ctrl->base, APMU_DEBUG, DSI_PHY_DVM_MASK);
> +
> +       return 0;
> +}
> +
> +static int pxa1908_dsi_power_off(struct generic_pm_domain *genpd)
> +{
> +       struct pxa1908_pd *pd = container_of(genpd, struct pxa1908_pd, genpd);
> +       struct pxa1908_pd_ctrl *ctrl = dev_get_drvdata(pd->dev);
> +
> +       regmap_clear_bits(ctrl->base, APMU_DEBUG, DSI_PHY_DVM_MASK);
> +
> +       if (pd->clks)
> +               clk_bulk_disable_unprepare(pd->num_clks, pd->clks);
> +
> +       return 0;
> +}
> +
> +#define DOMAIN(_id, _name, ctrl, mode, state) \
> +       [_id] = { \
> +               .data = { \
> +                       .reg_clk_res_ctrl = ctrl, \
> +                       .hw_mode = BIT(mode), \
> +                       .pwr_state = BIT(state), \
> +                       .id = _id, \
> +               }, \
> +               .genpd = { \
> +                       .name = _name, \
> +                       .power_on = pxa1908_pd_power_on, \
> +                       .power_off = pxa1908_pd_power_off, \
> +               }, \
> +       }
> +
> +static struct pxa1908_pd domains[] = {
> +       DOMAIN(PXA1908_POWER_DOMAIN_VPU, "vpu", 0xa4, 19, 2),
> +       DOMAIN(PXA1908_POWER_DOMAIN_GPU, "gpu", 0xcc, 11, 0),
> +       DOMAIN(PXA1908_POWER_DOMAIN_GPU2D, "gpu2d", 0xf4, 11, 6),
> +       DOMAIN(PXA1908_POWER_DOMAIN_ISP, "isp", 0x38, 15, 4),
> +       [PXA1908_POWER_DOMAIN_DSI] = {
> +               .genpd = {
> +                       .name = "dsi",
> +                       .power_on = pxa1908_dsi_power_on,
> +                       .power_off = pxa1908_dsi_power_off,
> +                       /*
> +                        * TODO: There is no DSI driver written yet and until then we probably
> +                        * don't want to power off the DSI PHY ever.
> +                        */
> +                       .flags = GENPD_FLAG_ALWAYS_ON,
> +               },
> +               .data = {
> +                       /* See above. */
> +                       .keep_on = true,
> +               },
> +       },
> +};
> +
> +static void pxa1908_pd_cleanup(struct pxa1908_pd_ctrl *ctrl)
> +{
> +       struct pxa1908_pd *pd;
> +       int ret;
> +
> +       for (int i = ARRAY_SIZE(domains) - 1; i >= 0; i--) {
> +               pd = &domains[i];
> +
> +               if (!pd->initialized)
> +                       continue;
> +
> +               ret = pm_genpd_remove(&pd->genpd);
> +               if (ret)
> +                       dev_err(pd->dev, "failed to remove domain '%s': %d\n",
> +                               pd->genpd.name, ret);
> +               if (pxa1908_pd_is_on(pd) && !pd->data.keep_on)
> +                       pxa1908_pd_power_off(&pd->genpd);
> +
> +               clk_bulk_put_all(pd->num_clks, pd->clks);
> +       }
> +}
> +
> +static int
> +pxa1908_pd_init(struct pxa1908_pd_ctrl *ctrl, struct device_node *node, struct device *dev)
> +{
> +       struct pxa1908_pd *pd;
> +       int clk_idx = 0, ret;
> +       u32 id;
> +
> +       ret = of_property_read_u32(node, "reg", &id);
> +       if (ret) {
> +               dev_err(dev, "failed to get domain id from reg: %d\n", ret);
> +               return ret;
> +       }
> +
> +       if (id >= ARRAY_SIZE(domains)) {
> +               dev_err(dev, "invalid domain id %d\n", id);
> +               return ret;
> +       }
> +
> +       pd = &domains[id];
> +       pd->dev = dev;
> +       pd->num_clks = of_clk_get_parent_count(node);
> +       ctrl->domains[id] = &pd->genpd;
> +
> +       if (pd->num_clks > 0) {
> +               pd->clks = devm_kcalloc(dev, pd->num_clks, sizeof(*pd->clks), GFP_KERNEL);
> +               if (!pd->clks)
> +                       return -ENOMEM;
> +       }
> +
> +       for (int i = 0; i < pd->num_clks; i++) {
> +               struct clk *clk = of_clk_get(node, i);
> +
> +               if (IS_ERR(clk)) {
> +                       ret = PTR_ERR(clk);
> +                       dev_err(dev, "failed to get clk for domain '%s': %d\n",
> +                               pd->genpd.name, ret);
> +                       goto err;
> +               }
> +
> +               pd->clks[clk_idx++].clk = clk;
> +       }
> +
> +       /* Make sure the state of the hardware is synced with the domain table above. */
> +       if (pd->data.keep_on) {
> +               ret = pd->genpd.power_on(&pd->genpd);
> +               if (ret) {
> +                       dev_err(dev, "failed to power on domain '%s': %d\n", pd->genpd.name, ret);
> +                       goto err;
> +               }
> +       } else {
> +               if (pxa1908_pd_is_on(pd)) {
> +                       dev_warn(dev,
> +                                "domain '%s' is on despite being default off; powering off\n",
> +                                pd->genpd.name);
> +
> +                       ret = pxa1908_pd_power_off(&pd->genpd);
> +                       if (ret) {
> +                               dev_err(dev, "failed to power off domain '%s': %d\n",
> +                                       pd->genpd.name, ret);
> +                               goto err;
> +                       }
> +               }
> +       }
> +
> +       ret = pm_genpd_init(&pd->genpd, NULL, !pd->data.keep_on);
> +       if (ret) {
> +               dev_err(dev, "domain '%s' failed to initialize: %d\n", pd->genpd.name, ret);
> +               goto err;
> +       }
> +
> +       pd->initialized = true;
> +
> +       return 0;
> +
> +err:
> +       clk_bulk_put_all(pd->num_clks, pd->clks);
> +       return ret;
> +}
> +
> +static int pxa1908_pd_probe(struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +       struct pxa1908_pd_ctrl *ctrl;
> +       struct device_node *node;
> +       int ret;
> +
> +       ctrl = devm_kzalloc(dev, struct_size(ctrl, domains, ARRAY_SIZE(domains)), GFP_KERNEL);
> +       if (!ctrl)
> +               return -ENOMEM;
> +
> +       ctrl->base = syscon_node_to_regmap(dev->parent->of_node);
> +       if (IS_ERR(ctrl->base)) {
> +               dev_err(dev, "no regmap available\n");
> +               return PTR_ERR(ctrl->base);
> +       }
> +
> +       platform_set_drvdata(pdev, ctrl);
> +
> +       ctrl->onecell_data.domains = ctrl->domains;
> +       ctrl->onecell_data.num_domains = ARRAY_SIZE(domains);
> +
> +       for_each_available_child_of_node(dev->of_node, node) {
> +               ret = pxa1908_pd_init(ctrl, node, dev);
> +               if (ret)
> +                       goto err;
> +       }
> +
> +       return of_genpd_add_provider_onecell(dev->of_node, &ctrl->onecell_data);
> +
> +err:
> +       pxa1908_pd_cleanup(ctrl);
> +       return ret;
> +}
> +
> +static void pxa1908_pd_remove(struct platform_device *pdev)
> +{
> +       pxa1908_pd_cleanup(platform_get_drvdata(pdev));
> +}
> +
> +static const struct of_device_id pxa1908_pd_match[] = {
> +       {
> +               .compatible = "marvell,pxa1908-power-controller",
> +       },
> +       { }
> +};
> +MODULE_DEVICE_TABLE(of, pxa1908_pd_match);
> +
> +static struct platform_driver pxa1908_pd_driver = {
> +       .probe = pxa1908_pd_probe,
> +       .remove = pxa1908_pd_remove,
> +       .driver = {
> +               .name = "pxa1908-power-controller",
> +               .of_match_table = pxa1908_pd_match,
> +       },
> +};
> +module_platform_driver(pxa1908_pd_driver);
> +
> +MODULE_AUTHOR("Duje Mihanović <duje@dujemihanovic.xyz>");
> +MODULE_DESCRIPTION("Marvell PXA1908 power domain driver");
> +MODULE_LICENSE("GPL");
>
> --
> 2.50.1
>

[PATCH RFC 4/5] MAINTAINERS: PXA1908: Add power domain controller

[Duje Mihanović]

Add the new PXA1908 power domain driver under the PXA1908 entry. Also
add the clock schema (unintentionally omitted previously) and a link to
the PXA1908 mainlining chatroom.

Signed-off-by: Duje Mihanović <duje@dujemihanovic.xyz>
---
 MAINTAINERS | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index fda151dbf229e48d791e082b1f6be2e43fdb8d1c..905f3027d00e0cd5edf59fdc7dcf6aa69ec608d5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2869,9 +2869,14 @@ ARM/Marvell PXA1908 SOC support
 M:	Duje Mihanović <duje@dujemihanovic.xyz>
 L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
 S:	Maintained
+C:	irc://irc.oftc.net/pxa1908-mainline
+F:	Documentation/devicetree/bindings/clock/marvell,pxa1908.yaml
+F:	Documentation/devicetree/bindings/power/marvell,pxa1908-power-controller.yaml
 F:	arch/arm64/boot/dts/marvell/mmp/
 F:	drivers/clk/mmp/clk-pxa1908*.c
+F:	drivers/pmdomain/marvell/
 F:	include/dt-bindings/clock/marvell,pxa1908.h
+F:	include/dt-bindings/power/marvell,pxa1908-power.h
 
 ARM/Mediatek RTC DRIVER
 M:	Eddie Huang <eddie.huang@mediatek.com>

-- 
2.50.1

[PATCH RFC 5/5] arm64: dts: marvell: pxa1908: Add power controller

[Duje Mihanović]

Add a node for the newly implemented power domain controller. Also add
the first two power domain consumers: IOMMU (fixes probing) and
framebuffer.

Signed-off-by: Duje Mihanović <duje@dujemihanovic.xyz>
---
 .../marvell/mmp/pxa1908-samsung-coreprimevelte.dts |  1 +
 arch/arm64/boot/dts/marvell/mmp/pxa1908.dtsi       | 36 +++++++++++++++++++++-
 2 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/marvell/mmp/pxa1908-samsung-coreprimevelte.dts b/arch/arm64/boot/dts/marvell/mmp/pxa1908-samsung-coreprimevelte.dts
index 47a4f01a7077bfafe2cc50d0e59c37685ec9c2e9..2f175ae48c6a2371c407b3a6ffd3cdd577f44e56 100644
--- a/arch/arm64/boot/dts/marvell/mmp/pxa1908-samsung-coreprimevelte.dts
+++ b/arch/arm64/boot/dts/marvell/mmp/pxa1908-samsung-coreprimevelte.dts
@@ -23,6 +23,7 @@ chosen {
 		fb0: framebuffer@17177000 {
 			compatible = "simple-framebuffer";
 			reg = <0 0x17177000 0 (480 * 800 * 4)>;
+			power-domains = <&pd PXA1908_POWER_DOMAIN_DSI>;
 			width = <480>;
 			height = <800>;
 			stride = <(480 * 4)>;
diff --git a/arch/arm64/boot/dts/marvell/mmp/pxa1908.dtsi b/arch/arm64/boot/dts/marvell/mmp/pxa1908.dtsi
index cf2b9109688ce560eec8a1397251ead68d78a239..630e99f2c309dca0872d824a098ac93b6e55c3a4 100644
--- a/arch/arm64/boot/dts/marvell/mmp/pxa1908.dtsi
+++ b/arch/arm64/boot/dts/marvell/mmp/pxa1908.dtsi
@@ -3,6 +3,7 @@
 
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/clock/marvell,pxa1908.h>
+#include <dt-bindings/power/marvell,pxa1908-power.h>
 
 / {
 	model = "Marvell Armada PXA1908";
@@ -79,6 +80,7 @@ smmu: iommu@c0010000 {
 			#iommu-cells = <1>;
 			interrupts = <GIC_SPI 78 IRQ_TYPE_LEVEL_HIGH>,
 				<GIC_SPI 78 IRQ_TYPE_LEVEL_HIGH>;
+			power-domains = <&pd PXA1908_POWER_DOMAIN_VPU>;
 			status = "disabled";
 		};
 
@@ -291,9 +293,41 @@ sdh2: mmc@81000 {
 			};
 
 			apmu: clock-controller@82800 {
-				compatible = "marvell,pxa1908-apmu";
+				compatible = "marvell,pxa1908-apmu", "simple-mfd", "syscon";
 				reg = <0x82800 0x400>;
 				#clock-cells = <1>;
+
+				pd: power-controller {
+					compatible = "marvell,pxa1908-power-controller";
+					#address-cells = <1>;
+					#size-cells = <0>;
+					#power-domain-cells = <1>;
+
+					power-domain@PXA1908_POWER_DOMAIN_VPU {
+						reg = <PXA1908_POWER_DOMAIN_VPU>;
+						#power-domain-cells = <0>;
+					};
+
+					power-domain@PXA1908_POWER_DOMAIN_GPU {
+						reg = <PXA1908_POWER_DOMAIN_GPU>;
+						#power-domain-cells = <0>;
+					};
+
+					power-domain@PXA1908_POWER_DOMAIN_GPU2D {
+						reg = <PXA1908_POWER_DOMAIN_GPU2D>;
+						#power-domain-cells = <0>;
+					};
+
+					power-domain@PXA1908_POWER_DOMAIN_DSI {
+						reg = <PXA1908_POWER_DOMAIN_DSI>;
+						#power-domain-cells = <0>;
+					};
+
+					power-domain@PXA1908_POWER_DOMAIN_ISP {
+						reg = <PXA1908_POWER_DOMAIN_ISP>;
+						#power-domain-cells = <0>;
+					};
+				};
 			};
 		};
 	};

-- 
2.50.1

Re: [PATCH RFC 0/5] Marvell PXA1908 power domains

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2083 bytes --]

On Wed, Aug 06, 2025 at 07:33:19PM +0200, Duje Mihanović wrote:
> Hello,
> 
> This series implements support for the power domains found in Marvell's
> PXA1908 SoC. The domains control power for the graphics, video and image
> processors along with the DSI PHY.
> 
> The series is based on master as the MAINTAINERS and device tree patches
> depend on the very recently merged initial Marvell PXA1908 support series.
> That series can be found at the following link:
> https://lore.kernel.org/all/20250708-pxa1908-lkml-v16-0-b4392c484180@dujemihanovic.xyz

It's not clear to me, nor mentioned anywhere I could see, why this is an
RFC. What are you actually soliciting feedback on?

> 
> Signed-off-by: Duje Mihanović <duje@dujemihanovic.xyz>
> ---
> Duje Mihanović (5):
>       dt-bindings: clock: marvell,pxa1908: Add simple-mfd, syscon compatible to apmu
>       dt-bindings: power: Add Marvell PXA1908 domains
>       pmdomain: marvell: Add PXA1908 power domains
>       MAINTAINERS: PXA1908: Add power domain controller
>       arm64: dts: marvell: pxa1908: Add power controller
> 
>  .../devicetree/bindings/clock/marvell,pxa1908.yaml |  36 ++-
>  .../power/marvell,pxa1908-power-controller.yaml    | 105 +++++++
>  MAINTAINERS                                        |   5 +
>  .../marvell/mmp/pxa1908-samsung-coreprimevelte.dts |   1 +
>  arch/arm64/boot/dts/marvell/mmp/pxa1908.dtsi       |  36 ++-
>  drivers/pmdomain/Kconfig                           |   1 +
>  drivers/pmdomain/Makefile                          |   1 +
>  drivers/pmdomain/marvell/Kconfig                   |  16 +
>  drivers/pmdomain/marvell/Makefile                  |   3 +
>  .../pmdomain/marvell/pxa1908-power-controller.c    | 347 +++++++++++++++++++++
>  include/dt-bindings/power/marvell,pxa1908-power.h  |  17 +
>  11 files changed, 561 insertions(+), 7 deletions(-)
> ---
> base-commit: cca7a0aae8958c9b1cd14116cb8b2f22ace2205e
> change-id: 20250803-pxa1908-genpd-15918db5260c
> 
> Best regards,
> -- 
> Duje Mihanović <duje@dujemihanovic.xyz>
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH RFC 0/5] Marvell PXA1908 power domains

[Duje Mihanović]

On Thursday, 7 August 2025 17:40:28 Central European Summer Time Conor Dooley wrote:
> On Wed, Aug 06, 2025 at 07:33:19PM +0200, Duje Mihanović wrote:
> > Hello,
> > 
> > This series implements support for the power domains found in Marvell's
> > PXA1908 SoC. The domains control power for the graphics, video and image
> > processors along with the DSI PHY.
> > 
> > The series is based on master as the MAINTAINERS and device tree patches
> > depend on the very recently merged initial Marvell PXA1908 support series.
> > That series can be found at the following link:
> > https://lore.kernel.org/all/20250708-pxa1908-lkml-v16-0-b4392c484180@dujemih
> > anovic.xyz
> It's not clear to me, nor mentioned anywhere I could see, why this is an
> RFC. What are you actually soliciting feedback on?

Apologies for that, should have mentioned it right away. I was unsure
whether I was handling domain clocks correctly, which is one of the
things Krzysztof since commented on.

Regards,
--
Duje