Skip to contentSkip to conversation

power: domain: Add driver for a PM domain provider which controls

PATCH In review Max Krummenacher 11 participants 42 messages Jun 9, 2022 – Sep 28, 2022

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

cover power: domain: Add driver for a PM domain provider which controls

From: Max Krummenacher <[email protected]>

its power enable by using a regulator.

The currently implemented PM domain providers are all specific to a particular system on chip.

This series adds a PM domain provider driver which enables/disables a regulator to control its power state. Additionally, marked with RFC, it adds two commits which actually make use of the new driver to instantiate a power domain provider and have a number of power domain consumers use the power domain.

The perceived use case is to control a common power domain used by several devices for which not all device drivers nessesarily have a means to control a regulator.

It also handles the suspend / resume use case for such devices, the generic power domain framework will disable the domain once the last device has been suspend and will enable it again before resuming the first device.

The generic power domain code handles a power domain consumer generically outside of the driver's code. (assuming the 'power-domains' property references exactly one power domain).
This allows to use the "regulator-pm-pd" driver with an arbitrary device just by adding the 'power-domains' property to the devices device tree node. However the device's dt-bindings schema likely does not allow the property 'power-domains'.
One way to solve this would be to allow 'power-domains' globally similarly how 'status' and other common properties are allowed as implicit properties.

Max Krummenacher (5):
dt-bindings: power: Add bindings for a power domain controlled by a
regulator
pm: add regulator power domain controller
MAINTAINERS: add REGULATOR POWER DOMAIN
arm64: defconfig: Enable generic power domain controller controlling a
regulator
ARM64: verdin-imx8mm: use regulator power domain to model sleep-moci

 .../power/regulator-power-domain.yaml         |  58 +++++++++
 MAINTAINERS                                   |   9 ++
 .../dts/freescale/imx8mm-verdin-dahlia.dtsi   |   1 +
 .../boot/dts/freescale/imx8mm-verdin-dev.dtsi |   2 +
 .../boot/dts/freescale/imx8mm-verdin.dtsi     |  35 ++++--
 arch/arm64/configs/defconfig                  |   1 +
 drivers/power/Kconfig                         |   1 +
 drivers/power/Makefile                        |   5 +-
 drivers/power/domain/Kconfig                  |   7 ++
 drivers/power/domain/Makefile                 |   2 +
 drivers/power/domain/regulator-pdc.c          | 112 ++++++++++++++++++
 11 files changed, 221 insertions(+), 12 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/power/regulator-power-domain.yaml
 create mode 100644 drivers/power/domain/Kconfig
 create mode 100644 drivers/power/domain/Makefile
 create mode 100644 drivers/power/domain/regulator-pdc.c
Signature · 1 line
2.20.1
#1 lore raw

PATCH 1/5 dt-bindings: power: Add bindings for a power domain controlled by a regulator

From: Max Krummenacher <[email protected]>

Adds binding for a power domain provider which uses a regulator to control the power domain.

---

 .../power/regulator-power-domain.yaml         | 58 +++++++++++++++++++
 1 file changed, 58 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/regulator-power-domain.yaml
Documentation/devicetree/bindings/power/regulator-power-domain.yaml new file +58 -0
new file mode 100644
index 000000000000..2b49c4f2f866
@@ -0,0 +1,58 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/power/regulator-power-domain.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Power domain controlled by a regulator
+
+maintainers:
+  - Max Krummenacher <[email protected]>
+
+description: |+
+  Power domain provider which uses a regulator to control
+  the power domain.
+
+allOf:
+  - $ref: "power-domain.yaml#"
+
+properties:
+  compatible:
+    enum:
+      - regulator-pm-pd
+
+  power-supply:
+    description: The regulator used to control the power domain.
+
+  label:
+    description: Human readable string defining the domain.
+
+  "#power-domain-cells":
+    const: 0
+
+  power-domains:
+    maxItems: 1
+
+required:
+  - compatible
+  - "#power-domain-cells"
+  - power-supply
+
+additionalProperties: true
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    reg_pd_sleep_moci: regulator-sleep-moci {
+        compatible = "regulator-fixed";
+        enable-active-high;
+        gpio = <&gpio5 1 GPIO_ACTIVE_HIGH>;
+        regulator-name = "CTRL_SLEEP_MOCI";
+    };
+
+    pd_sleep_moci: power-sleep-moci {
+        compatible = "regulator-pm-pd";
+        power-supply = <&reg_pd_sleep_moci>;
+        label = "pd_sleep_moci";
+        #power-domain-cells = <0>;
+    };
Signature · 1 line
2.20.1
maintainer
#2 lore raw

Re: [PATCH v1 1/5] dt-bindings: power: Add bindings for a power domain controlled by a regulator

Quoted 5 lines · Max Krummenacher

From: Max Krummenacher <[email protected]>

Adds binding for a power domain provider which uses a regulator to control the power domain.

Quoted 7 lines

---

 .../power/regulator-power-domain.yaml         | 58 +++++++++++++++++++
 1 file changed, 58 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/regulator-power-domain.yaml

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/power/regulator-power-domain.example.dtb: power-sleep-moci: $nodename:0: 'power-sleep-moci' does not match '^(power-controller|power-domain)([@-].*)?$'
From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/power/regulator-power-domain.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/

This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date:

pip3 install dtschema --upgrade

Please check and re-submit.

#3 lore raw

Re: [PATCH v1 1/5] dt-bindings: power: Add bindings for a power domain controlled by a regulator

Hi

Quoted 15 lines · Rob Herring
Quoted 5 lines · Max Krummenacher

From: Max Krummenacher <[email protected]>

Adds binding for a power domain provider which uses a regulator to control the power domain.

Quoted 7 lines

---

 .../power/regulator-power-domain.yaml         | 58 +++++++++++++++++++
 1 file changed, 58 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/regulator-power-domain.yaml
Quoted 8 lines

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/power/regulator-power-domain.example.dtb: power-sleep-moci: $nodename:0: 'power-sleep-moci' does not match '^(power-controller|power-domain)([@-].*)?$'
From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/power/regulator-power-domain.yaml

Will change to 'power-domain-sleep-moci' in V2.

Regards
Max

Quoted 16 lines

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/

This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date:

pip3 install dtschema --upgrade

Please check and re-submit.

#4 lore raw

Re: [PATCH v1 1/5] dt-bindings: power: Add bindings for a power domain controlled by a regulator

Quoted 21 lines · Max Krummenacher

Hi

Quoted 18 lines · Rob Herring
Quoted 5 lines · Max Krummenacher

From: Max Krummenacher <[email protected]>

Adds binding for a power domain provider which uses a regulator to control the power domain.

Quoted 7 lines

---

 .../power/regulator-power-domain.yaml         | 58 +++++++++++++++++++
 1 file changed, 58 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/regulator-power-domain.yaml

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13):

Quoted 8 lines
Quoted 5 lines

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/power/regulator-power-domain.example.dtb: power-sleep-moci: $nodename:0: 'power-sleep-moci' does not match '^(power-controller|power-domain)([@-].*)?$'
From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/power/regulator-power-domain.yaml

Will change to 'power-domain-sleep-moci' in V2.

Instead: power-domain
(names should be generic)

Best regards,
Krzysztof

#5 lore raw

Re: [PATCH v1 1/5] dt-bindings: power: Add bindings for a power domain controlled by a regulator

In reply to Max Krummenacher

Hi Max,

Quoted 6 lines · Max Krummenacher

From: Max Krummenacher <[email protected]>

Adds binding for a power domain provider which uses a regulator to control the power domain.

Thanks for your patch!

Quoted 18 lines
Documentation/devicetree/bindings/power/regulator-power-domain.yaml new file +15 -0
@@ -0,0 +1,58 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/power/regulator-power-domain.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Power domain controlled by a regulator
+
+maintainers:
+  - Max Krummenacher <[email protected]>
+
+description: |+
+  Power domain provider which uses a regulator to control
+  the power domain.
+

Documentation/devicetree/bindings/power/regulator-power-domain.yaml line 25

Quoted 10 lines
Documentation/devicetree/bindings/power/regulator-power-domain.yaml new file +10 -0
+allOf:
+  - $ref: "power-domain.yaml#"
+
+properties:
+  compatible:
+    enum:
+      - regulator-pm-pd
+
+  power-supply:
+    description: The regulator used to control the power domain.

I guess there can be more than one?

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

#6 lore raw

Re: [PATCH v1 1/5] dt-bindings: power: Add bindings for a power domain controlled by a regulator

Hi

Quoted 35 lines · Geert Uytterhoeven

Hi Max,

Quoted 6 lines · Max Krummenacher

From: Max Krummenacher <[email protected]>

Adds binding for a power domain provider which uses a regulator to control the power domain.

Thanks for your patch!

Quoted 22 lines
Documentation/devicetree/bindings/power/regulator-power-domain.yaml new file +19 -0
@@ -0,0 +1,58 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/power/regulator-power-domain.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Power domain controlled by a regulator
+
+maintainers:
+  - Max Krummenacher <[email protected]>
+
+description: |+
+  Power domain provider which uses a regulator to control
+  the power domain.
+
+allOf:
+  - $ref: "power-domain.yaml#"
+
+properties:
Quoted 8 lines

Documentation/devicetree/bindings/power/regulator-power-domain.yaml line 25

Quoted 6 lines
Documentation/devicetree/bindings/power/regulator-power-domain.yaml new file +6 -0
+  compatible:
+    enum:
+      - regulator-pm-pd
+
+  power-supply:
+    description: The regulator used to control the power domain.

I guess there can be more than one?

The proposed implementation currently only uses one.

When I did it I considered more than one regulator a rare use case and I was under the impression that the generic power domain code can handle multiple power domains. With that in mind I assumed that one would create multiple regulator-pm-pd instances each controlling one regulator and add all of them to the power-domains property of the power domain consumer.

But it seems the implementation requires the power domain consumer to handle that case in its code rather than relying on the generic code. [1]

Do you see a real world use case to handle multiple regulators?

Max

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/base/power/domain.c?id=8cb1cbd644d5bba5b72eedd632f249c1c677b792#n2290

Quoted 11 lines

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

#7 lore raw

PATCH 2/5 pm: add regulator power domain controller

In reply to Max Krummenacher

From: Max Krummenacher <[email protected]>

Acts as a power domain controller which switches its domain power by enabling/disabling a regulator.

---

 drivers/power/Kconfig                |   1 +
 drivers/power/Makefile               |   5 +-
 drivers/power/domain/Kconfig         |   7 ++
 drivers/power/domain/Makefile        |   2 +
 drivers/power/domain/regulator-pdc.c | 112 +++++++++++++++++++++++++++
 5 files changed, 125 insertions(+), 2 deletions(-)
 create mode 100644 drivers/power/domain/Kconfig
 create mode 100644 drivers/power/domain/Makefile
 create mode 100644 drivers/power/domain/regulator-pdc.c
drivers/power/Kconfig +1 -0
index 696bf77a7042..e8eebee629d5 100644
@@ -1,3 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0-only
+source "drivers/power/domain/Kconfig"
 source "drivers/power/reset/Kconfig"
 source "drivers/power/supply/Kconfig"
drivers/power/Makefile +3 -2
index effbf0377f32..4323a8c8c328 100644
@@ -1,3 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0-only
-obj-$(CONFIG_POWER_RESET)	+= reset/
-obj-$(CONFIG_POWER_SUPPLY)	+= supply/
+obj-$(CONFIG_PM_GENERIC_DOMAINS)	+= domain/
+obj-$(CONFIG_POWER_RESET)		+= reset/
+obj-$(CONFIG_POWER_SUPPLY)		+= supply/
drivers/power/domain/Kconfig new file +7 -0
new file mode 100644
index 000000000000..74c50baf2df0
@@ -0,0 +1,7 @@
+# SPDX-License-Identifier: GPL-2.0-only
+
+config POWER_DOMAIN_REGULATOR
+	bool "A power domain controller using a regulator for power control"
+	help
+	  Say Y here to enable a power domain controller which uses a regulator
+	  to control its power domain.
drivers/power/domain/Makefile new file +2 -0
new file mode 100644
index 000000000000..13b3378fb11f
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_POWER_DOMAIN_REGULATOR)	+= regulator-pdc.o
drivers/power/domain/regulator-pdc.c new file +112 -0

112 lines not on this page · raw message

Signature · 1 line
2.20.1
#8 lore raw

PATCH 3/5 MAINTAINERS: add REGULATOR POWER DOMAIN

In reply to Max Krummenacher

From: Max Krummenacher <[email protected]>

Create a maintainer entry for the newly added regulator power domain controller driver.

---

 MAINTAINERS | 9 +++++++++
 1 file changed, 9 insertions(+)
MAINTAINERS +9 -0
index a6d3bd9d2a8d..cb0cf81034c2 100644
@@ -16922,6 +16922,15 @@ F:	Documentation/devicetree/bindings/regmap/
 F:	drivers/base/regmap/
 F:	include/linux/regmap.h
 
+REGULATOR POWER DOMAIN
+M:	Max Krummenacher <[email protected]>
+L:	[email protected]
+S:	Maintained
+F:	Documentation/devicetree/bindings/power/regulator-power-domain.yaml
+F:	drivers/power/domain/Kconfig
+F:	drivers/power/domain/Makefile
+F:	drivers/power/domain/regulator-pdc.c
+
 REISERFS FILE SYSTEM
 L:	[email protected]
 S:	Supported
Signature · 1 line
2.20.1
#9 lore raw

RFC PATCH 4/5 arm64: defconfig: Enable generic power domain controller controlling a regulator

In reply to Max Krummenacher

From: Max Krummenacher <[email protected]>

Enable CONFIG_POWER_DOMAIN_REGULATOR which is used for controlling CTRL_SLEEP_MOCI# on a Verdin iMX8M Mini SoM.

---

 arch/arm64/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)
arch/arm64/configs/defconfig +1 -0
index 7d1105343bc2..fa53a85cee18 100644
@@ -564,6 +564,7 @@ CONFIG_GPIO_PCA953X_IRQ=y
 CONFIG_GPIO_BD9571MWV=m
 CONFIG_GPIO_MAX77620=y
 CONFIG_GPIO_SL28CPLD=m
+CONFIG_POWER_DOMAIN_REGULATOR=y
 CONFIG_POWER_RESET_MSM=y
 CONFIG_POWER_RESET_QCOM_PON=m
 CONFIG_POWER_RESET_XGENE=y
Signature · 1 line
2.20.1
#10 lore raw

RFC PATCH 5/5 ARM64: verdin-imx8mm: use regulator power domain to model sleep-moci

In reply to Max Krummenacher

From: Max Krummenacher <[email protected]>

The Verdin CTRL_SLEEP_MOCI# pin signals the carrier board that the module is in sleep and it may switch off unneeded power.

Control this pin with a regulator power domain controller which uses a fixed regulator with a gpio enable.

---

 .../dts/freescale/imx8mm-verdin-dahlia.dtsi   |  1 +
 .../boot/dts/freescale/imx8mm-verdin-dev.dtsi |  2 ++
 .../boot/dts/freescale/imx8mm-verdin.dtsi     | 35 +++++++++++++------
 3 files changed, 28 insertions(+), 10 deletions(-)
arch/arm64/boot/dts/freescale/imx8mm-verdin-dahlia.dtsi +1 -0
index c2a5c2f7b204..03416dc593d8 100644
@@ -92,6 +92,7 @@
 
 /* Verdin PCIE_1 */
 &pcie0 {
+	power-domains = <&pd_sleep_moci>;
 	status = "okay";
 };
 
arch/arm64/boot/dts/freescale/imx8mm-verdin-dev.dtsi +2 -0
index 73cc3fafa018..f887b907fdde 100644
@@ -50,6 +50,7 @@
 	/* Audio Codec */
 	nau8822_1a: audio-codec@1a {
 		compatible = "nuvoton,nau8822";
+		power-domains = <&pd_sleep_moci>;
 		reg = <0x1a>;
 	};
 };
@@ -59,6 +60,7 @@
 	linux,rs485-enabled-at-boot-time;
 	rs485-rts-active-low;
 	rs485-rx-during-tx;
+	power-domains = <&pd_sleep_moci>;
 };
 
 /* Limit frequency on dev board due to long traces and bad signal integrity */
arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi +25 -10
index eafa88d980b3..b5daa8f83bef 100644
@@ -53,6 +53,14 @@
 		};
 	};
 
+	pd_sleep_moci: power-domain-sleep-moci {
+		compatible = "regulator-pm-pd";
+		label = "pd_sleep_moci";
+		power-domains = <&pgc_pcie>;
+		power-supply = <&reg_sleep_moci>;
+		#power-domain-cells = <0>;
+	};
+
 	/* Carrier Board Supplies */
 	reg_1p8v: regulator-1p8v {
 		compatible = "regulator-fixed";
@@ -90,6 +98,19 @@
 		startup-delay-us = <200000>;
 	};
 
+	reg_sleep_moci: regulator-sleep-moci {
+		compatible = "regulator-fixed";
+		enable-active-high;
+		gpio = <&gpio5 1 GPIO_ACTIVE_HIGH>;
+		off-on-delay = <2000>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_ctrl_sleep_moci>;
+		regulator-max-microvolt = <1800000>;
+		regulator-min-microvolt = <1800000>;
+		regulator-name = "CTRL_SLEEP_MOCI#";
+		startup-delay-us = <2000>;
+	};
+
 	reg_usb_otg1_vbus: regulator-usb-otg1 {
 		compatible = "regulator-fixed";
 		enable-active-high;
@@ -109,6 +130,7 @@
 		gpio = <&gpio1 14 GPIO_ACTIVE_HIGH>;
 		pinctrl-names = "default";
 		pinctrl-0 = <&pinctrl_reg_usb2_en>;
+		power-domains = <&pd_sleep_moci>;
 		regulator-max-microvolt = <5000000>;
 		regulator-min-microvolt = <5000000>;
 		regulator-name = "USB_2_EN";
@@ -198,6 +220,7 @@
 		interrupts-extended = <&gpio1 6 IRQ_TYPE_EDGE_FALLING>;
 		pinctrl-names = "default";
 		pinctrl-0 = <&pinctrl_can1_int>;
+		power-domains = <&pd_sleep_moci>;
 		reg = <0>;
 		spi-max-frequency = <8500000>;
 	};
@@ -305,16 +328,6 @@
 			  "SODIMM_212",
 			  "SODIMM_151",
 			  "SODIMM_153";
-
-	ctrl-sleep-moci-hog {
-		gpio-hog;
-		/* Verdin CTRL_SLEEP_MOCI# (SODIMM 256) */
-		gpios = <1 GPIO_ACTIVE_HIGH>;
-		line-name = "CTRL_SLEEP_MOCI#";
-		output-high;
-		pinctrl-names = "default";
-		pinctrl-0 = <&pinctrl_ctrl_sleep_moci>;
-	};
 };
 
 /* On-module I2C */
@@ -560,6 +573,7 @@
 		enable-gpios = <&gpio3 3 GPIO_ACTIVE_HIGH>;
 		pinctrl-names = "default";
 		pinctrl-0 = <&pinctrl_gpio_10_dsi>;
+		power-domains = <&pd_sleep_moci>;
 		reg = <0x2c>;
 		status = "disabled";
 	};
@@ -576,6 +590,7 @@
 		compatible = "lontium,lt8912b";
 		pinctrl-names = "default";
 		pinctrl-0 = <&pinctrl_gpio_10_dsi>, <&pinctrl_pwm_3_dsi_hpd_gpio>;
+		power-domains = <&pd_sleep_moci>;
 		reg = <0x48>;
 		/* Verdin GPIO_9_DSI (LT8912 INT, SODIMM 17, unused) */
 		/* Verdin GPIO_10_DSI (SODIMM 21) */
Signature · 1 line
2.20.1
#11 lore raw

Re: [RFC PATCH v1 5/5] ARM64: verdin-imx8mm: use regulator power domain to model sleep-moci

Hi Max,

Quoted 9 lines · Max Krummenacher

From: Max Krummenacher <[email protected]>

The Verdin CTRL_SLEEP_MOCI# pin signals the carrier board that the module is in sleep and it may switch off unneeded power.

Control this pin with a regulator power domain controller which uses a fixed regulator with a gpio enable.

Thanks for your patch!

arch/arm64/boot/dts/freescale/imx8mm-verdin-dahlia.dtsi line 95

Quoted 7 lines
arch/arm64/boot/dts/freescale/imx8mm-verdin-dahlia.dtsi +1 -0
@@ -92,6 +92,7 @@
 
 /* Verdin PCIE_1 */
 &pcie0 {
+       power-domains = <&pd_sleep_moci>;

This overrides "power-domains = <&pgc_pcie>;" from imx8mm.dtsi...

arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi line 59

Quoted 13 lines
arch/arm64/boot/dts/freescale/imx8mm-verdin-dahlia.dtsi +0 -0
        status = "okay";
 };
 
arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi +4 -0
@@ -53,6 +53,14 @@
                };
        };
 
+       pd_sleep_moci: power-domain-sleep-moci {
+               compatible = "regulator-pm-pd";
+               label = "pd_sleep_moci";
+               power-domains = <&pgc_pcie>;

... and here you work around that by re-binding <&pgc_pcie>.

I think you:
1. must not override the power-domains property for pcie0, as conceptually, the PCIe bus is still in the on-SoC power domain. What if some lanes are connected to devices in pd_sleep_moci, but other lanes are not?
2. should only use pd_sleep_moci for the off-chip devices that are actually controlled by the corresponding regulator.

arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi line 66

Quoted 7 lines
arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi +4 -0
+               power-supply = <&reg_sleep_moci>;
+               #power-domain-cells = <0>;
+       };
+
        /* Carrier Board Supplies */
        reg_1p8v: regulator-1p8v {
                compatible = "regulator-fixed";

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

#12 lore raw

Re: [RFC PATCH v1 5/5] ARM64: verdin-imx8mm: use regulator power domain to model sleep-moci

Hi

Quoted 42 lines · Geert Uytterhoeven

Hi Max,

Quoted 9 lines · Max Krummenacher

From: Max Krummenacher <[email protected]>

The Verdin CTRL_SLEEP_MOCI# pin signals the carrier board that the module is in sleep and it may switch off unneeded power.

Control this pin with a regulator power domain controller which uses a fixed regulator with a gpio enable.

Thanks for your patch!

arch/arm64/boot/dts/freescale/imx8mm-verdin-dahlia.dtsi line 95

Quoted 7 lines
arch/arm64/boot/dts/freescale/imx8mm-verdin-dahlia.dtsi +1 -0
@@ -92,6 +92,7 @@
 
 /* Verdin PCIE_1 */
 &pcie0 {
+       power-domains = <&pd_sleep_moci>;

This overrides "power-domains = <&pgc_pcie>;" from imx8mm.dtsi...

arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi line 59

Quoted 13 lines
arch/arm64/boot/dts/freescale/imx8mm-verdin-dahlia.dtsi +0 -0
        status = "okay";
 };
 
arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi +4 -0
@@ -53,6 +53,14 @@
                };
        };
 
+       pd_sleep_moci: power-domain-sleep-moci {
+               compatible = "regulator-pm-pd";
+               label = "pd_sleep_moci";
+               power-domains = <&pgc_pcie>;

... and here you work around that by re-binding <&pgc_pcie>.

Quoted 7 lines

I think you:
1. must not override the power-domains property for pcie0, as conceptually, the PCIe bus is still in the on-SoC power domain. What if some lanes are connected to devices in pd_sleep_moci, but other lanes are not?
2. should only use pd_sleep_moci for the off-chip devices that are actually controlled by the corresponding regulator.

I fully agree.
I wanted to send along the implementation commits for the power domain controller something which actually uses it and which did work in my testing.
That is why I marked this as RFC and I know that more work is needed.

Cheers
Max

Quoted 19 lines

arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi line 66

Quoted 8 lines
arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi +4 -0
+               power-supply = <&reg_sleep_moci>;
+               #power-domain-cells = <0>;
+       };
+
        /* Carrier Board Supplies */
        reg_1p8v: regulator-1p8v {
                compatible = "regulator-fixed";

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

maintainer
#13 lore raw
In reply to Max Krummenacher
Quoted 6 lines · Max Krummenacher

From: Max Krummenacher <[email protected]>

its power enable by using a regulator.

The currently implemented PM domain providers are all specific to a particular system on chip.

Yes, power domains tend to be specific to an SoC... 'power-domains' is supposed to be power islands in a chip. Linux 'PM domains' can be anything...

Quoted 9 lines

This series adds a PM domain provider driver which enables/disables a regulator to control its power state. Additionally, marked with RFC, it adds two commits which actually make use of the new driver to instantiate a power domain provider and have a number of power domain consumers use the power domain.

The perceived use case is to control a common power domain used by several devices for which not all device drivers nessesarily have a means to control a regulator.

Why wouldn't they have means?

Quoted 7 lines

It also handles the suspend / resume use case for such devices, the generic power domain framework will disable the domain once the last device has been suspend and will enable it again before resuming the first device.
The generic power domain code handles a power domain consumer generically outside of the driver's code. (assuming the 'power-domains' property references exactly one power domain).

That's Linux implementation details.

Quoted 7 lines

This allows to use the "regulator-pm-pd" driver with an arbitrary device just by adding the 'power-domains' property to the devices device tree node. However the device's dt-bindings schema likely does not allow the property 'power-domains'.
One way to solve this would be to allow 'power-domains' globally similarly how 'status' and other common properties are allowed as implicit properties.

No. For 'power-domains' bindings have to define how many there are and what each one is.

Rob

_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

#14 lore raw

Hi Rob,

Quoted 20 lines · Rob Herring
Quoted 6 lines · Max Krummenacher

From: Max Krummenacher <[email protected]>

its power enable by using a regulator.

The currently implemented PM domain providers are all specific to a particular system on chip.

Yes, power domains tend to be specific to an SoC... 'power-domains' is supposed to be power islands in a chip. Linux 'PM domains' can be anything...

Quoted 7 lines

This allows to use the "regulator-pm-pd" driver with an arbitrary device just by adding the 'power-domains' property to the devices device tree node. However the device's dt-bindings schema likely does not allow the property 'power-domains'.
One way to solve this would be to allow 'power-domains' globally similarly how 'status' and other common properties are allowed as implicit properties.

Quoted 2 lines

No. For 'power-domains' bindings have to define how many there are and what each one is.

IMO "power-domains" are an integration feature, i.e. orthogonal to the actual device that is part of the domain. Hence the "power-domains" property may appear everywhere.

It is actually the same for on-chip devices, as an IP core may be reused on a new SoC that does have power or clock domains. For these, we managed to handle that fine because most devices do have some form of family- or SoC-specific compatible values to control if the power-domains property can be present/is required or not.

But for off-chip devices, the integrator (board designed) can do whatever he wants. Off-chip devices do have the advantage that it is usually well documented which power supply (if there are multiple) serves which purpose, which is not always clear for on-chip devices.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

#15 lore raw

Hi

Quoted 15 lines · Geert Uytterhoeven

Hi Rob,

Quoted 11 lines · Rob Herring
Quoted 6 lines · Max Krummenacher

From: Max Krummenacher <[email protected]>

its power enable by using a regulator.

The currently implemented PM domain providers are all specific to a particular system on chip.

Yes, power domains tend to be specific to an SoC... 'power-domains' is supposed to be power islands in a chip. Linux 'PM domains' can be anything...

I don't see why such power islands should be restricted to a SoC. You can build the exact same idea on a PCB or even more modular designs.

Quoted 37 lines
Quoted 11 lines
Quoted 8 lines

This allows to use the "regulator-pm-pd" driver with an arbitrary device just by adding the 'power-domains' property to the devices device tree node. However the device's dt-bindings schema likely does not allow the property 'power-domains'.
One way to solve this would be to allow 'power-domains' globally similarly how 'status' and other common properties are allowed as implicit properties.

No. For 'power-domains' bindings have to define how many there are and what each one is.

IMO "power-domains" are an integration feature, i.e. orthogonal to the actual device that is part of the domain. Hence the "power-domains" property may appear everywhere.

It is actually the same for on-chip devices, as an IP core may be reused on a new SoC that does have power or clock domains. For these, we managed to handle that fine because most devices do have some form of family- or SoC-specific compatible values to control if the power-domains property can be present/is required or not.

But for off-chip devices, the integrator (board designed) can do whatever he wants. Off-chip devices do have the advantage that it is usually well documented which power supply (if there are multiple) serves which purpose, which is not always clear for on-chip devices.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

#16 lore raw
Quoted 15 lines · Max Krummenacher

Hi

Quoted 12 lines · Geert Uytterhoeven

Hi Rob,

Quoted 8 lines · Rob Herring
Quoted 6 lines · Max Krummenacher

From: Max Krummenacher <[email protected]>

its power enable by using a regulator.

The currently implemented PM domain providers are all specific to a particular system on chip.

Quoted 6 lines
Quoted 3 lines
Quoted 3 lines

Yes, power domains tend to be specific to an SoC... 'power-domains' is supposed to be power islands in a chip. Linux 'PM domains' can be anything...

I don't see why such power islands should be restricted to a SoC. You can build the exact same idea on a PCB or even more modular designs.

In the SoC these power islands are more-or-less defined. These are real regions gated by some control knob.

Calling few devices on a board "power domain" does not make it a power domain. There is no grouping, there is no control knob.

Aren't you now re-implementing regulator supplies? How is this different than existing supplies?

Best regards,
Krzysztof

#17 lore raw

Hi

Quoted 23 lines · Krzysztof Kozlowski
Quoted 15 lines · Max Krummenacher

Hi

Quoted 12 lines · Geert Uytterhoeven

Hi Rob,

Quoted 8 lines · Rob Herring
Quoted 6 lines · Max Krummenacher

From: Max Krummenacher <[email protected]>

its power enable by using a regulator.

The currently implemented PM domain providers are all specific to a particular system on chip.

Quoted 6 lines
Quoted 3 lines
Quoted 3 lines

Yes, power domains tend to be specific to an SoC... 'power-domains' is supposed to be power islands in a chip. Linux 'PM domains' can be anything...

I don't see why such power islands should be restricted to a SoC. You can build the exact same idea on a PCB or even more modular designs.

Quoted 8 lines

In the SoC these power islands are more-or-less defined. These are real regions gated by some control knob.

Calling few devices on a board "power domain" does not make it a power domain. There is no grouping, there is no control knob.

Aren't you now re-implementing regulator supplies? How is this different than existing supplies?

I believe the biggest difference between power-domains and regulator-supplies lays in the former being driver agnostic while the later is driver specific. Meaning with power-domains one can just add such arbitrary structure to the device tree without any further driver specific changes/handling required. While with regulator-supplies each and every driver actually needs to have driver specific handling thereof added. Or do I miss anything?

We are really trying to model something where a single GPIO pin (via a GPIO regulator or whatever) can control power to a variety of on-board peripherals. And, of course, we envision runtime PM actually making use of it e.g. when doing suspend/resume.

Quoted 2 lines

Best regards,
Krzysztof

Cheers

Marcel
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

#18 lore raw
Quoted 29 lines · Marcel Ziswiler

Hi

Quoted 26 lines · Krzysztof Kozlowski
Quoted 15 lines · Max Krummenacher

Hi

Quoted 12 lines · Geert Uytterhoeven

Hi Rob,

Quoted 8 lines · Rob Herring
Quoted 6 lines · Max Krummenacher

From: Max Krummenacher <[email protected]>

its power enable by using a regulator.

The currently implemented PM domain providers are all specific to a particular system on chip.

Quoted 6 lines
Quoted 3 lines
Quoted 3 lines

Yes, power domains tend to be specific to an SoC... 'power-domains' is supposed to be power islands in a chip. Linux 'PM domains' can be anything...

I don't see why such power islands should be restricted to a SoC. You can build the exact same idea on a PCB or even more modular designs.

In the SoC these power islands are more-or-less defined. These are real regions gated by some control knob.

Quoted 8 lines
Quoted 5 lines

Calling few devices on a board "power domain" does not make it a power domain. There is no grouping, there is no control knob.

Aren't you now re-implementing regulator supplies? How is this different than existing supplies?

I believe the biggest difference between power-domains and regulator-supplies lays in the former being driver agnostic while the later is driver specific.

That's one way to look, but the other way (matching the bindings purpose) is to look at hardware. You have physical wire / voltage rail supply - use regulator supply. In the terms of the hardware - what is that power domain? It's a concept, not a physical object.

Quoted 4 lines

Meaning with power-domains one can just add such arbitrary
structure to the device tree without any further driver specific changes/handling required. While with regulator-supplies each and every driver actually needs to have driver specific handling thereof added. Or do I miss anything?

Thanks for clarification but I am not sure if it matches the purpose of bindings and DTS. You can change the implementation as well to have implicit regulators. No need for new bindings for that.

Quoted 4 lines

We are really trying to model something where a single GPIO pin (via a GPIO regulator or whatever) can control power to a variety of on-board peripherals. And, of course, we envision runtime PM actually making use of it e.g. when doing suspend/resume.

And this GPIO pin controls what? Some power switch which cuts the power of one regulator or many? If many different regulators, how do you handle small differences in ramp up time?

Best regards,
Krzysztof

#19 lore raw
Quoted 36 lines · Krzysztof Kozlowski
Quoted 35 lines · Marcel Ziswiler

Hi

Quoted 31 lines · Krzysztof Kozlowski
Quoted 15 lines · Max Krummenacher

Hi

Quoted 12 lines · Geert Uytterhoeven

Hi Rob,

Quoted 8 lines · Rob Herring
Quoted 6 lines · Max Krummenacher

From: Max Krummenacher <[email protected]>

its power enable by using a regulator.

The currently implemented PM domain providers are all specific to a particular system on chip.

Quoted 6 lines
Quoted 3 lines
Quoted 3 lines

Yes, power domains tend to be specific to an SoC... 'power-domains' is supposed to be power islands in a chip. Linux 'PM domains' can be anything...

I don't see why such power islands should be restricted to a SoC. You can build the exact same idea on a PCB or even more modular designs.

In the SoC these power islands are more-or-less defined. These are real regions gated by some control knob.

Calling few devices on a board "power domain" does not make it a power domain. There is no grouping, there is no control knob.

Aren't you now re-implementing regulator supplies? How is this different than existing supplies?

Quoted 8 lines
Quoted 3 lines

I believe the biggest difference between power-domains and regulator-supplies lays in the former being driver
agnostic while the later is driver specific.

That's one way to look, but the other way (matching the bindings purpose) is to look at hardware. You have physical wire / voltage rail supply - use regulator supply. In the terms of the hardware - what is that power domain? It's a concept, not a physical object.

Well, but how can that concept then exist within the SoC but not outside? I don't get it. Isn't it just the exact same physical power gating thingy whether inside the SoC or on a PCB?

Quoted 9 lines
Quoted 5 lines

Meaning with power-domains one can just add such arbitrary
structure to the device tree without any further driver specific changes/handling required. While with regulator-supplies each and every driver actually needs to have driver specific handling thereof added. Or do I
miss anything?

Thanks for clarification but I am not sure if it matches the purpose of bindings and DTS. You can change the implementation as well to have implicit regulators. No need for new bindings for that.

Okay, maybe that would also work, of course. So basically add a new binding which allows adding regulators to arbitrary nodes which then will be generically handled by e.g. runtime PM. Almost something like assigned- clocks [1] you mean? I guess that could work. Remember that's why Max posted it as an RFC to get such feedback. Thanks for further refining those ideas.

Quoted 8 lines
Quoted 5 lines

We are really trying to model something where a single GPIO pin (via a GPIO regulator or whatever) can control
power to a variety of on-board peripherals. And, of course, we envision runtime PM actually making use of it
e.g. when doing suspend/resume.

And this GPIO pin controls what? Some power switch which cuts the power of one regulator or many?

Well, that doesn't really matter. Resp. this part one should be able to sufficiently model using whatever available regulator lore we already have (e.g. whatever delays/times).

Quoted 2 lines

If many different regulators, how do you
handle small differences in ramp up time?

Well, I don't think this is any different to any other regulator(s), not? Them HW folks will just need to tell us some reasonable numbers for those delays/times.

Quoted 2 lines

Best regards,
Krzysztof

[1] https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/clock/clock.yaml#L22

Cheers

Marcel

#20 lore raw
Quoted 55 lines · Marcel Ziswiler
Quoted 36 lines · Krzysztof Kozlowski
Quoted 35 lines · Marcel Ziswiler

Hi

Quoted 31 lines · Krzysztof Kozlowski
Quoted 15 lines · Max Krummenacher

Hi

Quoted 12 lines · Geert Uytterhoeven

Hi Rob,

Quoted 8 lines · Rob Herring
Quoted 6 lines · Max Krummenacher

From: Max Krummenacher <[email protected]>

its power enable by using a regulator.

The currently implemented PM domain providers are all specific to a particular system on chip.

Quoted 6 lines
Quoted 3 lines
Quoted 3 lines

Yes, power domains tend to be specific to an SoC... 'power-domains' is supposed to be power islands in a chip. Linux 'PM domains' can be anything...

I don't see why such power islands should be restricted to a SoC. You can build the exact same idea on a PCB or even more modular designs.

In the SoC these power islands are more-or-less defined. These are real regions gated by some control knob.

Calling few devices on a board "power domain" does not make it a power domain. There is no grouping, there is no control knob.

Aren't you now re-implementing regulator supplies? How is this different than existing supplies?

Quoted 8 lines
Quoted 3 lines

I believe the biggest difference between power-domains and regulator-supplies lays in the former being driver
agnostic while the later is driver specific.

That's one way to look, but the other way (matching the bindings purpose) is to look at hardware. You have physical wire / voltage rail supply - use regulator supply. In the terms of the hardware - what is that power domain? It's a concept, not a physical object.

Well, but how can that concept then exist within the SoC but not outside? I don't get it. Isn't it just the exact same physical power gating thingy whether inside the SoC or on a PCB?

Quoted 6 lines
Quoted 5 lines

Meaning with power-domains one can just add such arbitrary
structure to the device tree without any further driver specific changes/handling required. While with regulator-supplies each and every driver actually needs to have driver specific handling thereof added. Or do I
miss anything?

Quoted 8 lines
Quoted 3 lines

Thanks for clarification but I am not sure if it matches the purpose of bindings and DTS. You can change the implementation as well to have implicit regulators. No need for new bindings for that.

Okay, maybe that would also work, of course. So basically add a new binding which allows adding regulators to arbitrary nodes which then will be generically handled by e.g. runtime PM. Almost something like assigned- clocks [1] you mean? I guess that could work. Remember that's why Max posted it as an RFC to get such feedback. Thanks for further refining those ideas.

Please do not do this. You have an external device. It has some input voltage rails. Please define -supply properties for each of the voltage rails. Explicitly power them on and off. Use fixed-regulator for your GPIO regulators. Other boards might have other ways to control the power supply.

Then define the pm_runtime callbacks doing proper work for you. If you wish to do the magic, consider looking on the pm_clock.h interface (and adding the pm_regulators.h). But this approach can also be frowned upon by the PM maintainers. Nevertheless, this is the driver/core issue. The DT interface should be the same: a set of regulators and a set of -supply properties.

Signature · 2 lines
With best wishes
Dmitry
#21 lore raw
In reply to Marcel Ziswiler
Quoted 55 lines · Marcel Ziswiler
Quoted 36 lines · Krzysztof Kozlowski
Quoted 35 lines · Marcel Ziswiler

Hi

Quoted 31 lines · Krzysztof Kozlowski
Quoted 15 lines · Max Krummenacher

Hi

Quoted 12 lines · Geert Uytterhoeven

Hi Rob,

Quoted 8 lines · Rob Herring
Quoted 6 lines · Max Krummenacher

From: Max Krummenacher <[email protected]>

its power enable by using a regulator.

The currently implemented PM domain providers are all specific to a particular system on chip.

Quoted 6 lines
Quoted 3 lines
Quoted 3 lines

Yes, power domains tend to be specific to an SoC... 'power-domains' is supposed to be power islands in a chip. Linux 'PM domains' can be anything...

I don't see why such power islands should be restricted to a SoC. You can build the exact same idea on a PCB or even more modular designs.

In the SoC these power islands are more-or-less defined. These are real regions gated by some control knob.

Calling few devices on a board "power domain" does not make it a power domain. There is no grouping, there is no control knob.

Aren't you now re-implementing regulator supplies? How is this different than existing supplies?

Quoted 8 lines
Quoted 3 lines

I believe the biggest difference between power-domains and regulator-supplies lays in the former being driver
agnostic while the later is driver specific.

That's one way to look, but the other way (matching the bindings purpose) is to look at hardware. You have physical wire / voltage rail supply - use regulator supply. In the terms of the hardware - what is that power domain? It's a concept, not a physical object.

Well, but how can that concept then exist within the SoC but not outside? I don't get it. Isn't it just the exact same physical power gating thingy whether inside the SoC or on a PCB?

Quoted 6 lines
Quoted 5 lines

Meaning with power-domains one can just add such arbitrary
structure to the device tree without any further driver specific changes/handling required. While with regulator-supplies each and every driver actually needs to have driver specific handling thereof added. Or do I
miss anything?

Quoted 5 lines
Quoted 3 lines

Thanks for clarification but I am not sure if it matches the purpose of bindings and DTS. You can change the implementation as well to have implicit regulators. No need for new bindings for that.

Okay, maybe that would also work, of course. So basically add a new binding

That I did not propose. :) We have a binding for regulator supplies so you do no need a new one.

Quoted 4 lines

which allows adding regulators to
arbitrary nodes which then will be generically handled by e.g. runtime PM. Almost something like assigned- clocks [1] you mean? I guess that could work. Remember that's why Max posted it as an RFC to get such feedback. Thanks for further refining those ideas.

DTS and bindings describe here the hardware, so focus on that. Device is supplied by some regulator which I assume can be controlled by GPIO. I don't think you need new bindings for that.

Implementation of bindings, so Linux driver, is different thing.

Quoted 10 lines
Quoted 9 lines
Quoted 6 lines

We are really trying to model something where a single GPIO pin (via a GPIO regulator or whatever) can control
power to a variety of on-board peripherals. And, of course, we envision runtime PM actually making use of it
e.g. when doing suspend/resume.

And this GPIO pin controls what? Some power switch which cuts the power of one regulator or many?

Quoted 8 lines

Well, that doesn't really matter. Resp. this part one should be able to sufficiently model using whatever available regulator lore we already have (e.g. whatever delays/times).

Quoted 2 lines

If many different regulators, how do you
handle small differences in ramp up time?

Well, I don't think this is any different to any other regulator(s), not? Them HW folks will just need to tell us some reasonable numbers for those delays/times.

Probably... I just wonder how that would work in practice.

Best regards,
Krzysztof

#22 lore raw
In reply to Marcel Ziswiler
Quoted 41 lines · Marcel Ziswiler

Hi

Quoted 23 lines · Krzysztof Kozlowski
Quoted 15 lines · Max Krummenacher

Hi

Quoted 12 lines · Geert Uytterhoeven

Hi Rob,

Quoted 8 lines · Rob Herring
Quoted 6 lines · Max Krummenacher

From: Max Krummenacher <[email protected]>

its power enable by using a regulator.

The currently implemented PM domain providers are all specific to a particular system on chip.

Quoted 6 lines
Quoted 3 lines
Quoted 3 lines

Yes, power domains tend to be specific to an SoC... 'power-domains' is supposed to be power islands in a chip. Linux 'PM domains' can be anything...

I don't see why such power islands should be restricted to a SoC. You can build the exact same idea on a PCB or even more modular designs.

Quoted 8 lines

In the SoC these power islands are more-or-less defined. These are real regions gated by some control knob.

Calling few devices on a board "power domain" does not make it a power domain. There is no grouping, there is no control knob.

Aren't you now re-implementing regulator supplies? How is this different than existing supplies?

I believe the biggest difference between power-domains and regulator-supplies lays in the former being driver agnostic while the later is driver specific. Meaning with power-domains one can just add such arbitrary structure to the device tree without any further driver specific changes/handling required. While with regulator-supplies each and every driver actually needs to have driver specific handling thereof added. Or do I miss anything?

Quoted 3 lines

We are really trying to model something where a single GPIO pin (via a GPIO regulator or whatever) can control power to a variety of on-board peripherals. And, of course, we envision runtime PM actually making use of it e.g. when doing suspend/resume.

FWIW, this really seems to beg the question of PM support in the drivers for those peripherals. If they'll need to be modified to add suspend/resume routines anyway, then adding a handful more lines to control a supply regulator at the same time shouldn't be too big a deal. Conversely, I'd be surprised if they *did* have PM support if there wasn't already some way to make use of it.

Multiple consumers sharing a voltage rail provided by a single regulator is so standard and well-supported that it barely seems worth pointing out, but for the avoidance of doubt I shall. Adding a new non-standard way to hide a specific subset of regulator functionality behind behind a magic driver because it seems like slightly less work than handling it the well-known established way sounds like a great recipe for technical debt and future compatibility headaches. What if down the line you end up with a situation where if device A is suspended, devices B and C are happy to save some power by running the "domain" at a lower voltage? Do we stubbornly start duplicating more of the regulator framework in the magic power domain driver, or is that the point where we have to switch all the consumers to explicit supplies, and get to regret having "saved" that effort in the first place...

Cheers,
Robin.

#23 lore raw
Quoted 5 lines · Robin Murphy

Multiple consumers sharing a voltage rail provided by a single regulator is so standard and well-supported that it barely seems worth pointing out, but for the avoidance of doubt I shall. Adding a new non-standard way to hide a specific subset of regulator functionality behind behind a magic driver because it seems like slightly less work than handling it the well-known

Quoted 8 lines

established way sounds like a great recipe for technical debt and future compatibility headaches. What if down the line you end up with a situation where if device A is suspended, devices B and C are happy to save some power by running the "domain" at a lower voltage? Do we stubbornly start duplicating more of the regulator framework in the magic power domain driver, or is that the point where we have to switch all the consumers to explicit supplies, and get to regret having "saved" that effort in the first place...

We also loose the runtime validation that the supplies being described in the DT correspond to the hardware in any meaningful way which would also make it harder to transition to explicit control of the supplies further down the line.

#24 lore raw
In reply to Max Krummenacher
Quoted 23 lines · Max Krummenacher

From: Max Krummenacher <[email protected]>

its power enable by using a regulator.

The currently implemented PM domain providers are all specific to a particular system on chip.

This series adds a PM domain provider driver which enables/disables a regulator to control its power state. Additionally, marked with RFC, it adds two commits which actually make use of the new driver to instantiate a power domain provider and have a number of power domain consumers use the power domain.

The perceived use case is to control a common power domain used by several devices for which not all device drivers nessesarily have a means to control a regulator.

It also handles the suspend / resume use case for such devices, the generic power domain framework will disable the domain once the last device has been suspend and will enable it again before resuming the first device.

Quoted 10 lines

The generic power domain code handles a power domain consumer generically outside of the driver's code. (assuming the 'power-domains' property references exactly one power domain).
This allows to use the "regulator-pm-pd" driver with an arbitrary device just by adding the 'power-domains' property to the devices device tree node. However the device's dt-bindings schema likely does not allow the property 'power-domains'.
One way to solve this would be to allow 'power-domains' globally similarly how 'status' and other common properties are allowed as implicit properties.

I don't want to interrupt the discussion, but I still wanted to share my overall thoughts around the suggested approach.

Rather than adding some new DT bindings and a new generic DT compatible, I think the current power-domains bindings are sufficient to describe these types of HWs.

To me, it looks rather like you are striving towards avoiding open coding for power domain providers that make use of a regulator. Right?

To address that problem, I think a better option is to consider introducing a helper library with a set of functions that can be used by these types of power domain providers, in a way to simplify the code.

[...]

Kind regards
Uffe

#25 lore raw
In reply to Max Krummenacher
Quoted 2 lines · Max Krummenacher

This series adds a PM domain provider driver which enables/disables a regulator to control its power state.

Actually, we did this on the U8500 in 2011.

IIRC this led to problems because we had to invent "atomic regulators" because regulators use kernel abstractions that assume slowpath (process context) and power domains does not, i.e. they execute in fastpath, such as an interrupt handler.

The atomic regulator was a subset of regulator that only handled regulators that would result in something like an atomic register write.

In the end it was not worth trying to upstream this approach, and as I remember it, Ulf Hansson intended to let the power domains poke these registers directly, which was easier. (It's on Ulfs TODO list to actually implement this, hehe.)

Yours,
Linus Walleij

#26 lore raw

Hi all

Quoted 16 lines · Linus Walleij
Quoted 2 lines · Max Krummenacher

This series adds a PM domain provider driver which enables/disables a regulator to control its power state.

Actually, we did this on the U8500 in 2011.

IIRC this led to problems because we had to invent "atomic regulators" because regulators use kernel abstractions that assume slowpath (process context) and power domains does not, i.e. they execute in fastpath, such as an interrupt handler.

The atomic regulator was a subset of regulator that only handled regulators that would result in something like an atomic register write.

Quoted 7 lines

In the end it was not worth trying to upstream this approach, and as I remember it, Ulf Hansson intended to let the power domains poke these registers directly, which was easier. (It's on Ulfs TODO list to actually implement this, hehe.)

Yours,
Linus Walleij

Thanks for all the feedback.

The approach taken with the patchset seems to be architecturally wrong and as Linus pointed out has additionally the flaw that the regulator would need to be controllable in atomic context which depending on the regulator driver / configuration may not be fulfilled.

So our plan is to explicitly handle a (shared) regulator in every driver involved, adding that regulator capability for drivers not already having one.

The question which remains is on how one would model functionality which by itself does not need a driver but would need regulator support to switch its supply on in run state and off in suspend states and poweroff, like for example a simple level shifter. Any suggestions on this topic? Thanks.

Cheers
Max

#27 lore raw
Quoted 16 lines · Max Krummenacher

Hi all

Quoted 12 lines · Linus Walleij
Quoted 2 lines · Max Krummenacher

This series adds a PM domain provider driver which enables/disables a regulator to control its power state.

Actually, we did this on the U8500 in 2011.

IIRC this led to problems because we had to invent "atomic regulators" because regulators use kernel abstractions that assume slowpath (process context) and power domains does not, i.e. they execute in fastpath, such as an interrupt handler.

This isn't entirely correct. The callbacks of a genpd, *may* execute in atomic context, but that depends on whether the GENPD_FLAG_IRQ_SAFE is set for it or not.

Similar to what we have for runtime PM callbacks, with pm_runtime_irq_safe().

Quoted 8 lines
Quoted 8 lines

The atomic regulator was a subset of regulator that only handled regulators that would result in something like an atomic register write.

In the end it was not worth trying to upstream this approach, and as I remember it, Ulf Hansson intended to let the power domains poke these registers directly, which was easier. (It's on Ulfs TODO list to actually implement this, hehe.)

Yep, unfortunately I never got to the point. However, poking the registers directly from the genpd provider's on/off callbacks has never been my plan.

Instead I would rather expect us to call into a Ux500 specific interface for the prcmu FW. Simply because it's not really a regulator and must not be modelled like it. Instead it is a voltage/frequency domain that is managed behind a FW interface.

Quoted 10 lines
Quoted 3 lines

Yours,
Linus Walleij

Thanks for all the feedback.

The approach taken with the patchset seems to be architecturally wrong and as Linus pointed out has additionally the flaw that the regulator would need to be controllable in atomic context which depending on the regulator driver / configuration may not be fulfilled.

See above. Perhaps GENPD_FLAG_IRQ_SAFE and pm_runtime_irq_safe() can help with this?

Note that, power domains can be modelled with child/parents too, which perhaps can help around this too.

Quoted 4 lines

So our plan is to explicitly handle a (shared) regulator in every driver involved, adding that regulator capability for drivers not already having one.

Please don't! I have recently rejected a similar approach for Tegra platforms, which now have been converted into using the power domain approach.

If it's a powerail that is shared between controllers (devices), used to keep their registers values for example, that should be modelled as a power domain. Moreover for power domains, we can support voltage/frequency (performance) scaling, which isn't really applicable to a plain regulator.

However, if the actual powerrail fits well to be modelled as regulator, please go ahead. Although, in this case, the regulator must only be controlled behind a genpd provider's on/off callback, so you still need the power domain approach, rather than using the regulator in each driver and for each device.

Quoted 6 lines

The question which remains is on how one would model functionality which by itself does not need a driver but would need regulator support to switch its supply on in run state and off in suspend states and poweroff, like for example a simple level shifter.

Not sure I understand the question properly, but perhaps by adopting my proposal above this problem becomes easier to solve?

Quoted 4 lines

Any suggestions on this topic? Thanks.

Cheers
Max

Kind regards
Uffe

#28 lore raw
Quoted 10 lines · Ulf Hansson
Quoted 4 lines
Quoted 4 lines

IIRC this led to problems because we had to invent "atomic regulators" because regulators use kernel abstractions that assume slowpath (process context) and power domains does not, i.e. they execute in fastpath, such as an interrupt handler.

This isn't entirely correct. The callbacks of a genpd, *may* execute in atomic context, but that depends on whether the GENPD_FLAG_IRQ_SAFE is set for it or not.

Similar to what we have for runtime PM callbacks, with pm_runtime_irq_safe().

Aha I stand corrected!

Quoted 8 lines
Quoted 7 lines
Quoted 7 lines

The atomic regulator was a subset of regulator that only handled regulators that would result in something like an atomic register write.

In the end it was not worth trying to upstream this approach, and as I remember it, Ulf Hansson intended to let the power domains poke these registers directly, which was easier. (It's on Ulfs TODO list to actually implement this, hehe.)

Quoted 8 lines

Yep, unfortunately I never got to the point. However, poking the registers directly from the genpd provider's on/off callbacks has never been my plan.

Instead I would rather expect us to call into a Ux500 specific interface for the prcmu FW. Simply because it's not really a regulator and must not be modelled like it. Instead it is a voltage/frequency domain that is managed behind a FW interface.

We should take a stab at this, PostmarketOS just added support for three more U8500 phones so they support all the Samsung models and we have actual users of these systems. I think this would save them quite a lot of power. Also I use these targets for a lot of misc testing (like Kasan etc).

Yours,
Linus Walleij

#29 lore raw
In reply to Ulf Hansson

Hello Ulf and everybody,

Quoted 8 lines · Ulf Hansson
Quoted 3 lines · Max Krummenacher

So our plan is to explicitly handle a (shared) regulator in every driver involved, adding that regulator capability for drivers not already having one.

Please don't! I have recently rejected a similar approach for Tegra platforms, which now have been converted into using the power domain approach.

Just to quickly re-iterate how our hardware design looks like, we do have a single gpio that control the power of a whole board area that is supposed to be powered-off in suspend mode, this area could contains devices that have a proper Linux driver and some passive driver-less components (e.g. level shifter) - the exact mix varies.

Our proposal in this series was to model this as a power domain that could be controlled with a regulator. Krzysztof, Robin and others clearly argued against this idea.

The other approach would be to have a single regulator shared with the multiple devices we have there (still not clear how that would work in case we have only driver-less passive components). This is just a device-tree matter, maybe we would need to add support for a supply to some device drivers.

Honestly my conclusion from this discussion is that the only viable option is this second one, do I miss something?

Quoted 11 lines

If it's a powerail that is shared between controllers (devices), used to keep their registers values for example, that should be modelled as a power domain. Moreover for power domains, we can support voltage/frequency (performance) scaling, which isn't really applicable to a plain regulator.

However, if the actual powerrail fits well to be modelled as regulator, please go ahead. Although, in this case, the regulator must only be controlled behind a genpd provider's on/off callback, so you still need the power domain approach, rather than using the regulator in each driver and for each device.

Francesco

#30 lore raw
Quoted 19 lines · Francesco Dolcini

Hello Ulf and everybody,

Quoted 8 lines · Ulf Hansson
Quoted 3 lines · Max Krummenacher

So our plan is to explicitly handle a (shared) regulator in every driver involved, adding that regulator capability for drivers not already having one.

Please don't! I have recently rejected a similar approach for Tegra platforms, which now have been converted into using the power domain approach.

Just to quickly re-iterate how our hardware design looks like, we do have a single gpio that control the power of a whole board area that is supposed to be powered-off in suspend mode, this area could contains devices that have a proper Linux driver and some passive driver-less components (e.g. level shifter) - the exact mix varies.

Quoted 3 lines

Our proposal in this series was to model this as a power domain that could be controlled with a regulator. Krzysztof, Robin and others clearly argued against this idea.

Well, historically we haven't modelled these kinds of power-rails other than through power-domains. And this is exactly what genpd and PM domains in Linux are there to help us with.

Moreover, on another SoC/platform, maybe the power-rails are deployed differently and maybe those have the ability to scale performance too. Then it doesn't really fit well with the regulator model anymore.

If we want to continue to keep drivers portable, I don't see any better option than continuing to model these power-rails as power-domains.

Quoted 9 lines

The other approach would be to have a single regulator shared with the multiple devices we have there (still not clear how that would work in case we have only driver-less passive components). This is just a device-tree matter, maybe we would need to add support for a supply to some device drivers.

Honestly my conclusion from this discussion is that the only viable option is this second one, do I miss something?

No thanks!

Well, unless you can convince me there are benefits to this approach over the power-domain approach.

Quoted 7 lines
Quoted 7 lines

If it's a powerail that is shared between controllers (devices), used to keep their registers values for example, that should be modelled as a power domain. Moreover for power domains, we can support voltage/frequency (performance) scaling, which isn't really applicable to a plain regulator.

Quoted 8 lines
Quoted 5 lines

However, if the actual powerrail fits well to be modelled as regulator, please go ahead. Although, in this case, the regulator must only be controlled behind a genpd provider's on/off callback, so you still need the power domain approach, rather than using the regulator in each driver and for each device.

Francesco

Kind regards
Uffe

#31 lore raw
Quoted 44 lines · Ulf Hansson
Quoted 19 lines · Francesco Dolcini

Hello Ulf and everybody,

Quoted 8 lines · Ulf Hansson
Quoted 3 lines · Max Krummenacher

So our plan is to explicitly handle a (shared) regulator in every driver involved, adding that regulator capability for drivers not already having one.

Please don't! I have recently rejected a similar approach for Tegra platforms, which now have been converted into using the power domain approach.

Just to quickly re-iterate how our hardware design looks like, we do have a single gpio that control the power of a whole board area that is supposed to be powered-off in suspend mode, this area could contains devices that have a proper Linux driver and some passive driver-less components (e.g. level shifter) - the exact mix varies.

Quoted 3 lines

Our proposal in this series was to model this as a power domain that could be controlled with a regulator. Krzysztof, Robin and others clearly argued against this idea.

Well, historically we haven't modelled these kinds of power-rails other than through power-domains. And this is exactly what genpd and PM domains in Linux are there to help us with.

Moreover, on another SoC/platform, maybe the power-rails are deployed differently and maybe those have the ability to scale performance too. Then it doesn't really fit well with the regulator model anymore.

If we want to continue to keep drivers portable, I don't see any better option than continuing to model these power-rails as power-domains.

Quoted 7 lines

The other approach would be to have a single regulator shared with the multiple devices we have there (still not clear how that would work in case we have only driver-less passive components). This is just a device-tree matter, maybe we would need to add support for a supply to some device drivers.

Quoted 7 lines
Quoted 2 lines

Honestly my conclusion from this discussion is that the only viable option is this second one, do I miss something?

No thanks!

Well, unless you can convince me there are benefits to this approach over the power-domain approach.

I'm fine with our current power-domain proposal here, I do not need to convince you, I have the other problem to convince someone to merge it :-)

Maybe Krzysztof, Robin or Mark can comment again after you explained your view on this topic.

Francesco

_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

#32 lore raw
Quoted 54 lines · Francesco Dolcini
Quoted 44 lines · Ulf Hansson
Quoted 19 lines · Francesco Dolcini

Hello Ulf and everybody,

Quoted 8 lines · Ulf Hansson
Quoted 3 lines · Max Krummenacher

So our plan is to explicitly handle a (shared) regulator in every driver involved, adding that regulator capability for drivers not already having one.

Please don't! I have recently rejected a similar approach for Tegra platforms, which now have been converted into using the power domain approach.

Just to quickly re-iterate how our hardware design looks like, we do have a single gpio that control the power of a whole board area that is supposed to be powered-off in suspend mode, this area could contains devices that have a proper Linux driver and some passive driver-less components (e.g. level shifter) - the exact mix varies.

Quoted 3 lines

Our proposal in this series was to model this as a power domain that could be controlled with a regulator. Krzysztof, Robin and others clearly argued against this idea.

Well, historically we haven't modelled these kinds of power-rails other than through power-domains. And this is exactly what genpd and PM domains in Linux are there to help us with.

Moreover, on another SoC/platform, maybe the power-rails are deployed differently and maybe those have the ability to scale performance too. Then it doesn't really fit well with the regulator model anymore.

If we want to continue to keep drivers portable, I don't see any better option than continuing to model these power-rails as power-domains.

Quoted 7 lines

The other approach would be to have a single regulator shared with the multiple devices we have there (still not clear how that would work in case we have only driver-less passive components). This is just a device-tree matter, maybe we would need to add support for a supply to some device drivers.

Quoted 7 lines
Quoted 2 lines

Honestly my conclusion from this discussion is that the only viable option is this second one, do I miss something?

No thanks!

Well, unless you can convince me there are benefits to this approach over the power-domain approach.

Quoted 6 lines

I'm fine with our current power-domain proposal here, I do not need to convince you, I have the other problem to convince someone to merge it :-)

Maybe Krzysztof, Robin or Mark can comment again after you explained your view on this topic.

To move things forward, I suggest you re-start with the power domain approach.

Moreover, to avoid any churns, just implement it as another new SoC specific genpd provider and let the provider deal with the regulator. In this way, you don't need to invent any new types of DT bindings, but can re-use existing ones.

If you post a new version, please keep me cced, then I will help to review it.

Kind regards
Uffe

#33 lore raw

Hello Ulf,

Quoted 60 lines · Ulf Hansson
Quoted 58 lines · Francesco Dolcini
Quoted 44 lines · Ulf Hansson
Quoted 19 lines · Francesco Dolcini

Hello Ulf and everybody,

Quoted 8 lines · Ulf Hansson
Quoted 3 lines · Max Krummenacher

So our plan is to explicitly handle a (shared) regulator in every driver involved, adding that regulator capability for drivers not already having one.

Please don't! I have recently rejected a similar approach for Tegra platforms, which now have been converted into using the power domain approach.

Just to quickly re-iterate how our hardware design looks like, we do have a single gpio that control the power of a whole board area that is supposed to be powered-off in suspend mode, this area could contains devices that have a proper Linux driver and some passive driver-less components (e.g. level shifter) - the exact mix varies.

Quoted 3 lines

Our proposal in this series was to model this as a power domain that could be controlled with a regulator. Krzysztof, Robin and others clearly argued against this idea.

Well, historically we haven't modelled these kinds of power-rails other than through power-domains. And this is exactly what genpd and PM domains in Linux are there to help us with.

Moreover, on another SoC/platform, maybe the power-rails are deployed differently and maybe those have the ability to scale performance too. Then it doesn't really fit well with the regulator model anymore.

If we want to continue to keep drivers portable, I don't see any better option than continuing to model these power-rails as power-domains.

Quoted 7 lines

The other approach would be to have a single regulator shared with the multiple devices we have there (still not clear how that would work in case we have only driver-less passive components). This is just a device-tree matter, maybe we would need to add support for a supply to some device drivers.

Quoted 7 lines
Quoted 2 lines

Honestly my conclusion from this discussion is that the only viable option is this second one, do I miss something?

No thanks!

Well, unless you can convince me there are benefits to this approach over the power-domain approach.

I'm fine with our current power-domain proposal here, I do not need to convince you, I have the other problem to convince someone to merge it :-)

Quoted 7 lines
Quoted 2 lines

Maybe Krzysztof, Robin or Mark can comment again after you explained your view on this topic.

To move things forward, I suggest you re-start with the power domain approach.

Moreover, to avoid any churns, just implement it as another new SoC specific genpd provider and let the provider deal with the regulator.

I'm sorry, but I was not able to understand what you mean, can you provide some additional hint on the topic? Some reference driver we can look at?

The driver we implemented and proposed with this patch is just connecting a power-domain to a regulator, it's something at the board level, not at the SoC one.
We do not have a (existing) SoC driver were we could add the power domain provider as an additional functionality.

Quoted 2 lines

In this way, you don't need to invent any new types of DT bindings, but can re-use existing ones.

The only new binding would be a new "compatible" to have a place to tie the regulator instance used in the device tree, but I do not think that this is an issue at all.

The main concern that was raised on this topic was that we have to somehow link the power-domain to the specific peripherals (the power domain consumer) in the device tree.

Adding the power-domain property there will trigger validation errors unless we do explicitly add the power-domains to the schema for each peripheral we need this. To me this does not really work, but maybe I'm not understanding something.

This is what Rob wrote on the topic [1]:

Quoted 2 lines

No. For 'power-domains' bindings have to define how many there are and what each one is.

Just as an example from patch [2]:

  can1: can@0 {
    compatible = "microchip,mcp251xfd";
    power-domains = <&pd_sleep_moci>;
  };

leads to:

imx8mm-verdin-nonwifi-dahlia.dtb: can@0: 'power-domains' does not match any of the regexes: 'pinctrl-[0-9]+'
From schema: .../bindings/net/can/microchip,mcp251xfd.yaml

Quoted 1 line

If you post a new version, please keep me cced, then I will help to review it.

Thanks!

Francesco

[1] https://lore.kernel.org/all/[email protected]/
[2] https://lore.kernel.org/all/[email protected]/

#34 lore raw
Quoted 67 lines · Francesco Dolcini

Hello Ulf,

Quoted 63 lines · Ulf Hansson
Quoted 60 lines · Francesco Dolcini
Quoted 44 lines · Ulf Hansson
Quoted 19 lines · Francesco Dolcini

Hello Ulf and everybody,

Quoted 8 lines · Ulf Hansson
Quoted 3 lines · Max Krummenacher

So our plan is to explicitly handle a (shared) regulator in every driver involved, adding that regulator capability for drivers not already having one.

Please don't! I have recently rejected a similar approach for Tegra platforms, which now have been converted into using the power domain approach.

Just to quickly re-iterate how our hardware design looks like, we do have a single gpio that control the power of a whole board area that is supposed to be powered-off in suspend mode, this area could contains devices that have a proper Linux driver and some passive driver-less components (e.g. level shifter) - the exact mix varies.

Quoted 3 lines

Our proposal in this series was to model this as a power domain that could be controlled with a regulator. Krzysztof, Robin and others clearly argued against this idea.

Well, historically we haven't modelled these kinds of power-rails other than through power-domains. And this is exactly what genpd and PM domains in Linux are there to help us with.

Moreover, on another SoC/platform, maybe the power-rails are deployed differently and maybe those have the ability to scale performance too. Then it doesn't really fit well with the regulator model anymore.

If we want to continue to keep drivers portable, I don't see any better option than continuing to model these power-rails as power-domains.

Quoted 7 lines

The other approach would be to have a single regulator shared with the multiple devices we have there (still not clear how that would work in case we have only driver-less passive components). This is just a device-tree matter, maybe we would need to add support for a supply to some device drivers.

Quoted 7 lines
Quoted 2 lines

Honestly my conclusion from this discussion is that the only viable option is this second one, do I miss something?

No thanks!

Well, unless you can convince me there are benefits to this approach over the power-domain approach.

I'm fine with our current power-domain proposal here, I do not need to convince you, I have the other problem to convince someone to merge it :-)

Maybe Krzysztof, Robin or Mark can comment again after you explained your view on this topic.

Quoted 7 lines
Quoted 4 lines

To move things forward, I suggest you re-start with the power domain approach.

Moreover, to avoid any churns, just implement it as another new SoC specific genpd provider and let the provider deal with the regulator.

I'm sorry, but I was not able to understand what you mean, can you provide some additional hint on the topic? Some reference driver we can look at?

Typically, "git grep pm_genpd_init" will find genpd providers.

There are a couple of examples where a regulator (among other things) is being controlled from the genpd's ->power_on|off() callbacks, such as:

drivers/soc/mediatek/mtk-pm-domains.c
drivers/soc/imx/gpc.c

Quoted 6 lines

The driver we implemented and proposed with this patch is just connecting a power-domain to a regulator, it's something at the board level, not at the SoC one.
We do not have a (existing) SoC driver were we could add the power domain provider as an additional functionality.

Right, so you need to add a new SoC/platform driver for this.

Quoted 6 lines
Quoted 3 lines

In this way, you don't need to invent any new types of DT bindings, but can re-use existing ones.

The only new binding would be a new "compatible" to have a place to tie the regulator instance used in the device tree, but I do not think that this is an issue at all.

Yes, I agree.

Quoted 4 lines

The main concern that was raised on this topic was that we have to somehow link the power-domain to the specific peripherals (the power domain consumer) in the device tree.

Yes, that is needed. Although, I don't see how that is a concern?

We already have the valid bindings to use for this, see more below.

Quoted 17 lines

Adding the power-domain property there will trigger validation errors unless we do explicitly add the power-domains to the schema for each peripheral we need this. To me this does not really work, but maybe I'm not understanding something.

This is what Rob wrote on the topic [1]:

Quoted 2 lines

No. For 'power-domains' bindings have to define how many there are and what each one is.

Just as an example from patch [2]:

  can1: can@0 {
    compatible = "microchip,mcp251xfd";
    power-domains = <&pd_sleep_moci>;
  };
Quoted 4 lines

leads to:

imx8mm-verdin-nonwifi-dahlia.dtb: can@0: 'power-domains' does not match any of the regexes: 'pinctrl-[0-9]+'
From schema: .../bindings/net/can/microchip,mcp251xfd.yaml

I think it should be fine to just add the below line to the DT bindings, for each peripheral device to fix the above problem.

power-domains: true

That should be okay, right?

Quoted 9 lines
Quoted 2 lines

If you post a new version, please keep me cced, then I will help to review it.

Thanks!

Francesco

[1] https://lore.kernel.org/all/[email protected]/
[2] https://lore.kernel.org/all/[email protected]/

Kind regards
Uffe

#35 lore raw
Quoted 132 lines · Ulf Hansson
Quoted 67 lines · Francesco Dolcini

Hello Ulf,

Quoted 63 lines · Ulf Hansson
Quoted 60 lines · Francesco Dolcini
Quoted 44 lines · Ulf Hansson
Quoted 19 lines · Francesco Dolcini

Hello Ulf and everybody,

Quoted 8 lines · Ulf Hansson
Quoted 3 lines · Max Krummenacher

So our plan is to explicitly handle a (shared) regulator in every driver involved, adding that regulator capability for drivers not already having one.

Please don't! I have recently rejected a similar approach for Tegra platforms, which now have been converted into using the power domain approach.

Just to quickly re-iterate how our hardware design looks like, we do have a single gpio that control the power of a whole board area that is supposed to be powered-off in suspend mode, this area could contains devices that have a proper Linux driver and some passive driver-less components (e.g. level shifter) - the exact mix varies.

Quoted 3 lines

Our proposal in this series was to model this as a power domain that could be controlled with a regulator. Krzysztof, Robin and others clearly argued against this idea.

Well, historically we haven't modelled these kinds of power-rails other than through power-domains. And this is exactly what genpd and PM domains in Linux are there to help us with.

Moreover, on another SoC/platform, maybe the power-rails are deployed differently and maybe those have the ability to scale performance too. Then it doesn't really fit well with the regulator model anymore.

If we want to continue to keep drivers portable, I don't see any better option than continuing to model these power-rails as power-domains.

Quoted 7 lines

The other approach would be to have a single regulator shared with the multiple devices we have there (still not clear how that would work in case we have only driver-less passive components). This is just a device-tree matter, maybe we would need to add support for a supply to some device drivers.

Quoted 7 lines
Quoted 2 lines

Honestly my conclusion from this discussion is that the only viable option is this second one, do I miss something?

No thanks!

Well, unless you can convince me there are benefits to this approach over the power-domain approach.

I'm fine with our current power-domain proposal here, I do not need to convince you, I have the other problem to convince someone to merge it :-)

Maybe Krzysztof, Robin or Mark can comment again after you explained your view on this topic.

Quoted 7 lines
Quoted 4 lines

To move things forward, I suggest you re-start with the power domain approach.

Moreover, to avoid any churns, just implement it as another new SoC specific genpd provider and let the provider deal with the regulator.

I'm sorry, but I was not able to understand what you mean, can you provide some additional hint on the topic? Some reference driver we can look at?

Typically, "git grep pm_genpd_init" will find genpd providers.

There are a couple of examples where a regulator (among other things) is being controlled from the genpd's ->power_on|off() callbacks, such as:

drivers/soc/mediatek/mtk-pm-domains.c
drivers/soc/imx/gpc.c

Quoted 6 lines

The driver we implemented and proposed with this patch is just connecting a power-domain to a regulator, it's something at the board level, not at the SoC one.
We do not have a (existing) SoC driver were we could add the power domain provider as an additional functionality.

Right, so you need to add a new SoC/platform driver for this.

Quoted 6 lines
Quoted 3 lines

In this way, you don't need to invent any new types of DT bindings, but can re-use existing ones.

The only new binding would be a new "compatible" to have a place to tie the regulator instance used in the device tree, but I do not think that this is an issue at all.

Yes, I agree.

Quoted 4 lines

The main concern that was raised on this topic was that we have to somehow link the power-domain to the specific peripherals (the power domain consumer) in the device tree.

Yes, that is needed. Although, I don't see how that is a concern?

We already have the valid bindings to use for this, see more below.

Quoted 19 lines

Adding the power-domain property there will trigger validation errors unless we do explicitly add the power-domains to the schema for each peripheral we need this. To me this does not really work, but maybe I'm not understanding something.

This is what Rob wrote on the topic [1]:

Quoted 2 lines

No. For 'power-domains' bindings have to define how many there are and what each one is.

Just as an example from patch [2]:

  can1: can@0 {
    compatible = "microchip,mcp251xfd";
    power-domains = <&pd_sleep_moci>;
  };

leads to:

Quoted 7 lines
Quoted 2 lines

imx8mm-verdin-nonwifi-dahlia.dtb: can@0: 'power-domains' does not match any of the regexes: 'pinctrl-[0-9]+'
From schema: .../bindings/net/can/microchip,mcp251xfd.yaml

I think it should be fine to just add the below line to the DT bindings, for each peripheral device to fix the above problem.

power-domains: true

Again, as Rob said, no, because it must be strictly defined. So for
example: "maxItems: 1" for simple cases. But what if device is then part of two power domains?

Quoted 2 lines

That should be okay, right?

Adding it to each peripheral scales poorly. Especially that literally any device can be part of such power domain.

If we are going with power domain approach, then it should be applicable basically to every device or to every device of some class (e.g. I2C, SPI). This means it should be added to respective core schema in dtschema repo, in a way it does not interfere with other power-domains properties (existing ones).

Best regards,
Krzysztof

#36 lore raw
Quoted 147 lines · Krzysztof Kozlowski
Quoted 132 lines · Ulf Hansson
Quoted 67 lines · Francesco Dolcini

Hello Ulf,

Quoted 63 lines · Ulf Hansson
Quoted 60 lines · Francesco Dolcini
Quoted 44 lines · Ulf Hansson
Quoted 19 lines · Francesco Dolcini

Hello Ulf and everybody,

Quoted 8 lines · Ulf Hansson
Quoted 3 lines · Max Krummenacher

So our plan is to explicitly handle a (shared) regulator in every driver involved, adding that regulator capability for drivers not already having one.

Please don't! I have recently rejected a similar approach for Tegra platforms, which now have been converted into using the power domain approach.

Just to quickly re-iterate how our hardware design looks like, we do have a single gpio that control the power of a whole board area that is supposed to be powered-off in suspend mode, this area could contains devices that have a proper Linux driver and some passive driver-less components (e.g. level shifter) - the exact mix varies.

Quoted 3 lines

Our proposal in this series was to model this as a power domain that could be controlled with a regulator. Krzysztof, Robin and others clearly argued against this idea.

Well, historically we haven't modelled these kinds of power-rails other than through power-domains. And this is exactly what genpd and PM domains in Linux are there to help us with.

Moreover, on another SoC/platform, maybe the power-rails are deployed differently and maybe those have the ability to scale performance too. Then it doesn't really fit well with the regulator model anymore.

If we want to continue to keep drivers portable, I don't see any better option than continuing to model these power-rails as power-domains.

Quoted 7 lines

The other approach would be to have a single regulator shared with the multiple devices we have there (still not clear how that would work in case we have only driver-less passive components). This is just a device-tree matter, maybe we would need to add support for a supply to some device drivers.

Quoted 7 lines
Quoted 2 lines

Honestly my conclusion from this discussion is that the only viable option is this second one, do I miss something?

No thanks!

Well, unless you can convince me there are benefits to this approach over the power-domain approach.

I'm fine with our current power-domain proposal here, I do not need to convince you, I have the other problem to convince someone to merge it :-)

Maybe Krzysztof, Robin or Mark can comment again after you explained your view on this topic.

Quoted 7 lines
Quoted 4 lines

To move things forward, I suggest you re-start with the power domain approach.

Moreover, to avoid any churns, just implement it as another new SoC specific genpd provider and let the provider deal with the regulator.

I'm sorry, but I was not able to understand what you mean, can you provide some additional hint on the topic? Some reference driver we can look at?

Typically, "git grep pm_genpd_init" will find genpd providers.

There are a couple of examples where a regulator (among other things) is being controlled from the genpd's ->power_on|off() callbacks, such as:

drivers/soc/mediatek/mtk-pm-domains.c
drivers/soc/imx/gpc.c

Quoted 6 lines

The driver we implemented and proposed with this patch is just connecting a power-domain to a regulator, it's something at the board level, not at the SoC one.
We do not have a (existing) SoC driver were we could add the power domain provider as an additional functionality.

Right, so you need to add a new SoC/platform driver for this.

Quoted 6 lines
Quoted 3 lines

In this way, you don't need to invent any new types of DT bindings, but can re-use existing ones.

The only new binding would be a new "compatible" to have a place to tie the regulator instance used in the device tree, but I do not think that this is an issue at all.

Yes, I agree.

Quoted 4 lines

The main concern that was raised on this topic was that we have to somehow link the power-domain to the specific peripherals (the power domain consumer) in the device tree.

Yes, that is needed. Although, I don't see how that is a concern?

We already have the valid bindings to use for this, see more below.

Quoted 19 lines

Adding the power-domain property there will trigger validation errors unless we do explicitly add the power-domains to the schema for each peripheral we need this. To me this does not really work, but maybe I'm not understanding something.

This is what Rob wrote on the topic [1]:

Quoted 2 lines

No. For 'power-domains' bindings have to define how many there are and what each one is.

Just as an example from patch [2]:

  can1: can@0 {
    compatible = "microchip,mcp251xfd";
    power-domains = <&pd_sleep_moci>;
  };

leads to:

Quoted 7 lines
Quoted 2 lines

imx8mm-verdin-nonwifi-dahlia.dtb: can@0: 'power-domains' does not match any of the regexes: 'pinctrl-[0-9]+'
From schema: .../bindings/net/can/microchip,mcp251xfd.yaml

I think it should be fine to just add the below line to the DT bindings, for each peripheral device to fix the above problem.

power-domains: true

Again, as Rob said, no, because it must be strictly defined. So for
example: "maxItems: 1" for simple cases. But what if device is then part of two power domains?

Quoted 1 line
Quoted 4 lines
Quoted 1 line

That should be okay, right?

Adding it to each peripheral scales poorly. Especially that literally any device can be part of such power domain.

Right.

Quoted 6 lines

If we are going with power domain approach, then it should be applicable basically to every device or to every device of some class (e.g. I2C, SPI). This means it should be added to respective core schema in dtschema repo, in a way it does not interfere with other power-domains properties (existing ones).

Isn't that already taken care of [1]?

If there is more than one power domain per device, perhaps we may need to extend it with a more strict binding? But, that doesn't seem to be the case here - and if it turns out to be needed later on, we can always extend the bindings, no?

Note also that we already have DT bindings specifying "power-domains:
true" to deal with the above. Isn't that what we want?

Quoted 5 lines

Best regards,
Krzysztof

Kind regards
Uffe

[1]
https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/power-domain/power-domain-consumer.yaml

#37 lore raw
Quoted 157 lines · Ulf Hansson
Quoted 147 lines · Krzysztof Kozlowski
Quoted 132 lines · Ulf Hansson
Quoted 67 lines · Francesco Dolcini

Hello Ulf,

Quoted 63 lines · Ulf Hansson
Quoted 60 lines · Francesco Dolcini
Quoted 44 lines · Ulf Hansson
Quoted 19 lines · Francesco Dolcini

Hello Ulf and everybody,

Quoted 8 lines · Ulf Hansson
Quoted 3 lines · Max Krummenacher

So our plan is to explicitly handle a (shared) regulator in every driver involved, adding that regulator capability for drivers not already having one.

Please don't! I have recently rejected a similar approach for Tegra platforms, which now have been converted into using the power domain approach.

Just to quickly re-iterate how our hardware design looks like, we do have a single gpio that control the power of a whole board area that is supposed to be powered-off in suspend mode, this area could contains devices that have a proper Linux driver and some passive driver-less components (e.g. level shifter) - the exact mix varies.

Quoted 3 lines

Our proposal in this series was to model this as a power domain that could be controlled with a regulator. Krzysztof, Robin and others clearly argued against this idea.

Well, historically we haven't modelled these kinds of power-rails other than through power-domains. And this is exactly what genpd and PM domains in Linux are there to help us with.

Moreover, on another SoC/platform, maybe the power-rails are deployed differently and maybe those have the ability to scale performance too. Then it doesn't really fit well with the regulator model anymore.

If we want to continue to keep drivers portable, I don't see any better option than continuing to model these power-rails as power-domains.

Quoted 7 lines

The other approach would be to have a single regulator shared with the multiple devices we have there (still not clear how that would work in case we have only driver-less passive components). This is just a device-tree matter, maybe we would need to add support for a supply to some device drivers.

Quoted 7 lines
Quoted 2 lines

Honestly my conclusion from this discussion is that the only viable option is this second one, do I miss something?

No thanks!

Well, unless you can convince me there are benefits to this approach over the power-domain approach.

I'm fine with our current power-domain proposal here, I do not need to convince you, I have the other problem to convince someone to merge it :-)

Maybe Krzysztof, Robin or Mark can comment again after you explained your view on this topic.

Quoted 7 lines
Quoted 4 lines

To move things forward, I suggest you re-start with the power domain approach.

Moreover, to avoid any churns, just implement it as another new SoC specific genpd provider and let the provider deal with the regulator.

I'm sorry, but I was not able to understand what you mean, can you provide some additional hint on the topic? Some reference driver we can look at?

Typically, "git grep pm_genpd_init" will find genpd providers.

There are a couple of examples where a regulator (among other things) is being controlled from the genpd's ->power_on|off() callbacks, such as:

drivers/soc/mediatek/mtk-pm-domains.c
drivers/soc/imx/gpc.c

Quoted 6 lines

The driver we implemented and proposed with this patch is just connecting a power-domain to a regulator, it's something at the board level, not at the SoC one.
We do not have a (existing) SoC driver were we could add the power domain provider as an additional functionality.

Right, so you need to add a new SoC/platform driver for this.

Quoted 6 lines
Quoted 3 lines

In this way, you don't need to invent any new types of DT bindings, but can re-use existing ones.

The only new binding would be a new "compatible" to have a place to tie the regulator instance used in the device tree, but I do not think that this is an issue at all.

Yes, I agree.

Quoted 4 lines

The main concern that was raised on this topic was that we have to somehow link the power-domain to the specific peripherals (the power domain consumer) in the device tree.

Yes, that is needed. Although, I don't see how that is a concern?

We already have the valid bindings to use for this, see more below.

Quoted 19 lines

Adding the power-domain property there will trigger validation errors unless we do explicitly add the power-domains to the schema for each peripheral we need this. To me this does not really work, but maybe I'm not understanding something.

This is what Rob wrote on the topic [1]:

Quoted 2 lines

No. For 'power-domains' bindings have to define how many there are and what each one is.

Just as an example from patch [2]:

  can1: can@0 {
    compatible = "microchip,mcp251xfd";
    power-domains = <&pd_sleep_moci>;
  };

leads to:

Quoted 7 lines
Quoted 2 lines

imx8mm-verdin-nonwifi-dahlia.dtb: can@0: 'power-domains' does not match any of the regexes: 'pinctrl-[0-9]+'
From schema: .../bindings/net/can/microchip,mcp251xfd.yaml

I think it should be fine to just add the below line to the DT bindings, for each peripheral device to fix the above problem.

power-domains: true

Again, as Rob said, no, because it must be strictly defined. So for
example: "maxItems: 1" for simple cases. But what if device is then part of two power domains?

Quoted 1 line
Quoted 4 lines
Quoted 1 line

That should be okay, right?

Adding it to each peripheral scales poorly. Especially that literally any device can be part of such power domain.

Right.

Quoted 1 line
Quoted 7 lines
Quoted 5 lines

If we are going with power domain approach, then it should be applicable basically to every device or to every device of some class (e.g. I2C, SPI). This means it should be added to respective core schema in dtschema repo, in a way it does not interfere with other power-domains properties (existing ones).

Isn't that already taken care of [1]?

No, because it does not define the items (what are the power domains and how many). This binding expects that any device has maxItems restricting it.

Quoted 8 lines

If there is more than one power domain per device, perhaps we may need to extend it with a more strict binding? But, that doesn't seem to be the case here - and if it turns out to be needed later on, we can always extend the bindings, no?

Note also that we already have DT bindings specifying "power-domains:
true" to deal with the above. Isn't that what we want?

You mentioned it before and both me and Rob already responded - no, because it does not restrict the number of items.

Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

#38 lore raw

[...]

Quoted 55 lines
Quoted 55 lines
Quoted 41 lines
Quoted 28 lines
Quoted 4 lines

The main concern that was raised on this topic was that we have to somehow link the power-domain to the specific peripherals (the power domain consumer) in the device tree.

Yes, that is needed. Although, I don't see how that is a concern?

We already have the valid bindings to use for this, see more below.

Quoted 19 lines

Adding the power-domain property there will trigger validation errors unless we do explicitly add the power-domains to the schema for each peripheral we need this. To me this does not really work, but maybe I'm not understanding something.

This is what Rob wrote on the topic [1]:

Quoted 2 lines

No. For 'power-domains' bindings have to define how many there are and what each one is.

Just as an example from patch [2]:

  can1: can@0 {
    compatible = "microchip,mcp251xfd";
    power-domains = <&pd_sleep_moci>;
  };

leads to:

Quoted 7 lines
Quoted 2 lines

imx8mm-verdin-nonwifi-dahlia.dtb: can@0: 'power-domains' does not match any of the regexes: 'pinctrl-[0-9]+'
From schema: .../bindings/net/can/microchip,mcp251xfd.yaml

I think it should be fine to just add the below line to the DT bindings, for each peripheral device to fix the above problem.

power-domains: true

Again, as Rob said, no, because it must be strictly defined. So for
example: "maxItems: 1" for simple cases. But what if device is then part of two power domains?

Quoted 1 line
Quoted 4 lines
Quoted 1 line

That should be okay, right?

Adding it to each peripheral scales poorly. Especially that literally any device can be part of such power domain.

Right.

Quoted 6 lines

If we are going with power domain approach, then it should be applicable basically to every device or to every device of some class (e.g. I2C, SPI). This means it should be added to respective core schema in dtschema repo, in a way it does not interfere with other power-domains properties (existing ones).

Quoted 4 lines
Quoted 1 line

Isn't that already taken care of [1]?

No, because it does not define the items (what are the power domains and how many). This binding expects that any device has maxItems restricting it.

Right, apologize for my ignorance.

Quoted 7 lines
Quoted 7 lines

If there is more than one power domain per device, perhaps we may need to extend it with a more strict binding? But, that doesn't seem to be the case here - and if it turns out to be needed later on, we can always extend the bindings, no?

Quoted 5 lines
Quoted 2 lines

Note also that we already have DT bindings specifying "power-domains:
true" to deal with the above. Isn't that what we want?

You mentioned it before and both me and Rob already responded - no, because it does not restrict the number of items.

Okay, so maxItems need to be specified for each peripheral. It's not a big deal, right?

Of course, it would be even easier if the core schema would use a default "maxItems: 1" for power domain consumers, which of course must be possible to be overridden for those consumers that need something else. But perhaps it's not that simple. :-)

Kind regards
Uffe

#39 lore raw

Hi Ulf,

Quoted 74 lines · Ulf Hansson
Quoted 54 lines
Quoted 54 lines
Quoted 40 lines
Quoted 27 lines
Quoted 3 lines

The main concern that was raised on this topic was that we have to somehow link the power-domain to the specific peripherals (the power domain consumer) in the device tree.

Yes, that is needed. Although, I don't see how that is a concern?

We already have the valid bindings to use for this, see more below.

Quoted 19 lines

Adding the power-domain property there will trigger validation errors unless we do explicitly add the power-domains to the schema for each peripheral we need this. To me this does not really work, but maybe I'm not understanding something.

This is what Rob wrote on the topic [1]:

Quoted 2 lines

No. For 'power-domains' bindings have to define how many there are and what each one is.

Just as an example from patch [2]:

  can1: can@0 {
    compatible = "microchip,mcp251xfd";
    power-domains = <&pd_sleep_moci>;
  };

leads to:

Quoted 7 lines
Quoted 2 lines

imx8mm-verdin-nonwifi-dahlia.dtb: can@0: 'power-domains' does not match any of the regexes: 'pinctrl-[0-9]+'
From schema: .../bindings/net/can/microchip,mcp251xfd.yaml

I think it should be fine to just add the below line to the DT bindings, for each peripheral device to fix the above problem.

power-domains: true

Again, as Rob said, no, because it must be strictly defined. So for
example: "maxItems: 1" for simple cases. But what if device is then part of two power domains?

Quoted 1 line
Quoted 4 lines
Quoted 1 line

That should be okay, right?

Adding it to each peripheral scales poorly. Especially that literally any device can be part of such power domain.

Right.

Quoted 6 lines

If we are going with power domain approach, then it should be applicable basically to every device or to every device of some class (e.g. I2C, SPI). This means it should be added to respective core schema in dtschema repo, in a way it does not interfere with other power-domains properties (existing ones).

Quoted 4 lines
Quoted 1 line

Isn't that already taken care of [1]?

No, because it does not define the items (what are the power domains and how many). This binding expects that any device has maxItems restricting it.

Right, apologize for my ignorance.

Quoted 7 lines
Quoted 7 lines

If there is more than one power domain per device, perhaps we may need to extend it with a more strict binding? But, that doesn't seem to be the case here - and if it turns out to be needed later on, we can always extend the bindings, no?

Quoted 5 lines
Quoted 2 lines

Note also that we already have DT bindings specifying "power-domains:
true" to deal with the above. Isn't that what we want?

You mentioned it before and both me and Rob already responded - no, because it does not restrict the number of items.

Quoted 7 lines

Okay, so maxItems need to be specified for each peripheral. It's not a big deal, right?

Of course, it would be even easier if the core schema would use a default "maxItems: 1" for power domain consumers, which of course must be possible to be overridden for those consumers that need something else. But perhaps it's not that simple. :-)

It's not that simple: being part of a PM Domain is not a property of the device being described, but a property of the integration into the SoC.

All synchronous hardware needs power (single/multiple), clock(s), and reset(s). But the granularity of control over power(s), clocks, and resets depends on the integration. So the related properties can appear anywhere.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

#40 lore raw
Quoted 79 lines · Geert Uytterhoeven

Hi Ulf,

Quoted 76 lines · Ulf Hansson
Quoted 54 lines
Quoted 54 lines
Quoted 40 lines
Quoted 27 lines
Quoted 3 lines

The main concern that was raised on this topic was that we have to somehow link the power-domain to the specific peripherals (the power domain consumer) in the device tree.

Yes, that is needed. Although, I don't see how that is a concern?

We already have the valid bindings to use for this, see more below.

Quoted 19 lines

Adding the power-domain property there will trigger validation errors unless we do explicitly add the power-domains to the schema for each peripheral we need this. To me this does not really work, but maybe I'm not understanding something.

This is what Rob wrote on the topic [1]:

Quoted 2 lines

No. For 'power-domains' bindings have to define how many there are and what each one is.

Just as an example from patch [2]:

  can1: can@0 {
    compatible = "microchip,mcp251xfd";
    power-domains = <&pd_sleep_moci>;
  };

leads to:

Quoted 7 lines
Quoted 2 lines

imx8mm-verdin-nonwifi-dahlia.dtb: can@0: 'power-domains' does not match any of the regexes: 'pinctrl-[0-9]+'
From schema: .../bindings/net/can/microchip,mcp251xfd.yaml

I think it should be fine to just add the below line to the DT bindings, for each peripheral device to fix the above problem.

power-domains: true

Again, as Rob said, no, because it must be strictly defined. So for
example: "maxItems: 1" for simple cases. But what if device is then part of two power domains?

Quoted 1 line
Quoted 4 lines
Quoted 1 line

That should be okay, right?

Adding it to each peripheral scales poorly. Especially that literally any device can be part of such power domain.

Right.

Quoted 6 lines

If we are going with power domain approach, then it should be applicable basically to every device or to every device of some class (e.g. I2C, SPI). This means it should be added to respective core schema in dtschema repo, in a way it does not interfere with other power-domains properties (existing ones).

Quoted 4 lines
Quoted 1 line

Isn't that already taken care of [1]?

No, because it does not define the items (what are the power domains and how many). This binding expects that any device has maxItems restricting it.

Right, apologize for my ignorance.

Quoted 7 lines
Quoted 7 lines

If there is more than one power domain per device, perhaps we may need to extend it with a more strict binding? But, that doesn't seem to be the case here - and if it turns out to be needed later on, we can always extend the bindings, no?

Quoted 5 lines
Quoted 2 lines

Note also that we already have DT bindings specifying "power-domains:
true" to deal with the above. Isn't that what we want?

You mentioned it before and both me and Rob already responded - no, because it does not restrict the number of items.

Okay, so maxItems need to be specified for each peripheral. It's not a big deal, right?

It's a bit of effort to add it manually to each device binding. It just does not scale well.

Quoted 5 lines
Quoted 5 lines

Of course, it would be even easier if the core schema would use a default "maxItems: 1" for power domain consumers, which of course must be possible to be overridden for those consumers that need something else. But perhaps it's not that simple. :-)

I think this would be the way to do it properly.

Quoted 3 lines

It's not that simple: being part of a PM Domain is not a property of the device being described, but a property of the integration into the SoC.

I agree.

This concept of power domains for every device does not look like really describing the hardware. The hardware itself, e.g. some camera sensor or I2C device, might have power supply and reset pin. It does not have something like power-domain.

Although one could also argue that it is the same case with SoC blocks - being part of power domain is a property of a SoC and its power domain controller.

Quoted 4 lines

All synchronous hardware needs power (single/multiple), clock(s), and reset(s). But the granularity of control over power(s), clocks, and resets depends on the integration. So the related properties can appear anywhere.

Best regards,
Krzysztof

#41 lore raw
Quoted 76 lines · Krzysztof Kozlowski
Quoted 74 lines · Geert Uytterhoeven

Hi Ulf,

Quoted 71 lines · Ulf Hansson
Quoted 54 lines
Quoted 54 lines
Quoted 40 lines
Quoted 27 lines
Quoted 3 lines

The main concern that was raised on this topic was that we have to somehow link the power-domain to the specific peripherals (the power domain consumer) in the device tree.

Yes, that is needed. Although, I don't see how that is a concern?

We already have the valid bindings to use for this, see more below.

Quoted 19 lines

Adding the power-domain property there will trigger validation errors unless we do explicitly add the power-domains to the schema for each peripheral we need this. To me this does not really work, but maybe I'm not understanding something.

This is what Rob wrote on the topic [1]:

Quoted 2 lines

No. For 'power-domains' bindings have to define how many there are and what each one is.

Just as an example from patch [2]:

  can1: can@0 {
    compatible = "microchip,mcp251xfd";
    power-domains = <&pd_sleep_moci>;
  };

leads to:

Quoted 7 lines
Quoted 2 lines

imx8mm-verdin-nonwifi-dahlia.dtb: can@0: 'power-domains' does not match any of the regexes: 'pinctrl-[0-9]+'
From schema: .../bindings/net/can/microchip,mcp251xfd.yaml

I think it should be fine to just add the below line to the DT bindings, for each peripheral device to fix the above problem.

power-domains: true

Again, as Rob said, no, because it must be strictly defined. So for
example: "maxItems: 1" for simple cases. But what if device is then part of two power domains?

Quoted 1 line
Quoted 4 lines
Quoted 1 line

That should be okay, right?

Adding it to each peripheral scales poorly. Especially that literally any device can be part of such power domain.

Right.

Quoted 6 lines

If we are going with power domain approach, then it should be applicable basically to every device or to every device of some class (e.g. I2C, SPI). This means it should be added to respective core schema in dtschema repo, in a way it does not interfere with other power-domains properties (existing ones).

Quoted 4 lines
Quoted 1 line

Isn't that already taken care of [1]?

No, because it does not define the items (what are the power domains and how many). This binding expects that any device has maxItems restricting it.

Right, apologize for my ignorance.

Quoted 10 lines
Quoted 9 lines

If there is more than one power domain per device, perhaps we may need to extend it with a more strict binding? But, that doesn't seem to be the case here - and if it turns out to be needed later on, we can always extend the bindings, no?

Note also that we already have DT bindings specifying "power-domains:
true" to deal with the above. Isn't that what we want?

Quoted 8 lines
Quoted 5 lines
Quoted 5 lines
Quoted 2 lines

You mentioned it before and both me and Rob already responded - no, because it does not restrict the number of items.

Okay, so maxItems need to be specified for each peripheral. It's not a big deal, right?

It's a bit of effort to add it manually to each device binding. It just does not scale well.

Whether it scales or not, that's how the power-domain bindings for consumers look like. It's the similar situation for clocks, regulators and other resources too.

My point is, for the discussion around $subject patch, it doesn't really matter what option we pick. The DT docs for each peripheral need to be updated anyway.

Quoted 15 lines
Quoted 6 lines
Quoted 6 lines

Of course, it would be even easier if the core schema would use a default "maxItems: 1" for power domain consumers, which of course must be possible to be overridden for those consumers that need something else. But perhaps it's not that simple. :-)

I think this would be the way to do it properly.

Quoted 3 lines

It's not that simple: being part of a PM Domain is not a property of the device being described, but a property of the integration into the SoC.

I agree.

Quoted 8 lines

This concept of power domains for every device does not look like really describing the hardware. The hardware itself, e.g. some camera sensor or I2C device, might have power supply and reset pin. It does not have something like power-domain.

Although one could also argue that it is the same case with SoC blocks - being part of power domain is a property of a SoC and its power domain controller.

Yes. DT describes the platform too, not only the SoC and its IP blocks.

Moreover, as the power domain bindings were added back in kernel v3.18, that's what we have to describe these kinds of platforms.

[...]

Kind regards
Uffe