[PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls

[PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls

[Max Krummenacher]

From: Max Krummenacher <max.krummenacher@toradex.com>

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

-- 
2.20.1

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

[Max Krummenacher]

From: Max Krummenacher <max.krummenacher@toradex.com>

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

Signed-off-by: Max Krummenacher <max.krummenacher@toradex.com>
---

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

diff --git a/Documentation/devicetree/bindings/power/regulator-power-domain.yaml b/Documentation/devicetree/bindings/power/regulator-power-domain.yaml
new file mode 100644
index 000000000000..2b49c4f2f866
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/regulator-power-domain.yaml
@@ -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 <max.krummenacher@toradex.com>
+
+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>;
+    };
-- 
2.20.1

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

[Rob Herring]

On Thu, 09 Jun 2022 17:08:47 +0200, Max Krummenacher wrote:
> From: Max Krummenacher <max.krummenacher@toradex.com>
> 
> Adds binding for a power domain provider which uses a regulator to control
> the power domain.
> 
> Signed-off-by: Max Krummenacher <max.krummenacher@toradex.com>
> ---
> 
>  .../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.

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

[Max Krummenacher]

Hi

On Fri, Jun 10, 2022 at 4:57 AM Rob Herring <robh@kernel.org> wrote:
>
> On Thu, 09 Jun 2022 17:08:47 +0200, Max Krummenacher wrote:
> > From: Max Krummenacher <max.krummenacher@toradex.com>
> >
> > Adds binding for a power domain provider which uses a regulator to control
> > the power domain.
> >
> > Signed-off-by: Max Krummenacher <max.krummenacher@toradex.com>
> > ---
> >
> >  .../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

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

Regards
Max

>
> 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.
>

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

[Krzysztof Kozlowski]

On 15/06/2022 08:19, Max Krummenacher wrote:
> Hi
> 
> On Fri, Jun 10, 2022 at 4:57 AM Rob Herring <robh@kernel.org> wrote:
>>
>> On Thu, 09 Jun 2022 17:08:47 +0200, Max Krummenacher wrote:
>>> From: Max Krummenacher <max.krummenacher@toradex.com>
>>>
>>> Adds binding for a power domain provider which uses a regulator to control
>>> the power domain.
>>>
>>> Signed-off-by: Max Krummenacher <max.krummenacher@toradex.com>
>>> ---
>>>
>>>  .../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
> 
> Will change to 'power-domain-sleep-moci' in V2.
> 

Instead: power-domain
(names should be generic)


Best regards,
Krzysztof

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

[Geert Uytterhoeven]

Hi Max,

On Thu, Jun 9, 2022 at 5:16 PM Max Krummenacher <max.oss.09@gmail.com> wrote:
> From: Max Krummenacher <max.krummenacher@toradex.com>
>
> Adds binding for a power domain provider which uses a regulator to control
> the power domain.
>
> Signed-off-by: Max Krummenacher <max.krummenacher@toradex.com>

Thanks for your patch!

> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/regulator-power-domain.yaml
> @@ -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 <max.krummenacher@toradex.com>
> +
> +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.

I guess there can be more than one?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

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

[Max Krummenacher]

Hi

On Tue, Jun 14, 2022 at 9:24 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Max,
>
> On Thu, Jun 9, 2022 at 5:16 PM Max Krummenacher <max.oss.09@gmail.com> wrote:
> > From: Max Krummenacher <max.krummenacher@toradex.com>
> >
> > Adds binding for a power domain provider which uses a regulator to control
> > the power domain.
> >
> > Signed-off-by: Max Krummenacher <max.krummenacher@toradex.com>
>
> Thanks for your patch!
>
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/power/regulator-power-domain.yaml
> > @@ -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 <max.krummenacher@toradex.com>
> > +
> > +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.
>
> 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


>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> 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

[PATCH v1 2/5] pm: add regulator power domain controller

[Max Krummenacher]

From: Max Krummenacher <max.krummenacher@toradex.com>

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

Signed-off-by: Max Krummenacher <max.krummenacher@toradex.com>
---

 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

diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index 696bf77a7042..e8eebee629d5 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -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"
diff --git a/drivers/power/Makefile b/drivers/power/Makefile
index effbf0377f32..4323a8c8c328 100644
--- a/drivers/power/Makefile
+++ b/drivers/power/Makefile
@@ -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/
diff --git a/drivers/power/domain/Kconfig b/drivers/power/domain/Kconfig
new file mode 100644
index 000000000000..74c50baf2df0
--- /dev/null
+++ b/drivers/power/domain/Kconfig
@@ -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.
diff --git a/drivers/power/domain/Makefile b/drivers/power/domain/Makefile
new file mode 100644
index 000000000000..13b3378fb11f
--- /dev/null
+++ b/drivers/power/domain/Makefile
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_POWER_DOMAIN_REGULATOR)	+= regulator-pdc.o
diff --git a/drivers/power/domain/regulator-pdc.c b/drivers/power/domain/regulator-pdc.c
new file mode 100644
index 000000000000..e03cfe901d70
--- /dev/null
+++ b/drivers/power/domain/regulator-pdc.c
@@ -0,0 +1,112 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * A power domain controller which uses a regulator to control its power domain.
+ *
+ * Copyright 2022 Toradex
+ */
+
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/pm_domain.h>
+#include <linux/regulator/consumer.h>
+
+struct reg_pd {
+	struct device *dev;
+	struct generic_pm_domain pd;
+	struct regulator *supply;
+};
+
+static int reg_pd_power_off(struct generic_pm_domain *domain)
+{
+	struct reg_pd *pd = container_of(domain, struct reg_pd, pd);
+
+	return regulator_disable(pd->supply);
+}
+
+static int reg_pd_power_on(struct generic_pm_domain *domain)
+{
+	struct reg_pd *pd = container_of(domain, struct reg_pd, pd);
+
+	return regulator_enable(pd->supply);
+}
+
+static const struct of_device_id reg_pd_of_match[] = {
+	{
+		.compatible = "regulator-pm-pd",
+	},
+	{ },
+};
+
+static int reg_pd_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct of_phandle_args child, parent;
+	struct reg_pd *pd;
+	const char *name;
+	int ret;
+
+	pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL);
+	if (!pd) {
+		of_node_put(np);
+		return -ENOMEM;
+	}
+	pd->dev = dev;
+
+	if (of_property_read_string(np, "label", &name) < 0)
+		name = kbasename(np->full_name);
+	pd->pd.name = kstrdup_const(name, GFP_KERNEL);
+	if (!pd->pd.name)
+		return -ENOMEM;
+
+	pd->supply = devm_regulator_get(dev, "power");
+	if (IS_ERR(pd->supply))
+		return dev_err_probe(dev, PTR_ERR(pd->supply),
+				     "failed to get regulator\n");
+
+	pd->pd.power_off = reg_pd_power_off;
+	pd->pd.power_on = reg_pd_power_on;
+
+	ret = pm_genpd_init(&pd->pd, NULL, true);
+	if (ret)
+		goto err_out;
+	ret = of_genpd_add_provider_simple(np, &pd->pd);
+	if (ret)
+		goto err_out;
+
+	if (of_parse_phandle_with_args(np, "power-domains",
+				      "#power-domain-cells", 0, &parent) == 0) {
+		child.np = np;
+		child.args_count = 0;
+
+		if (of_genpd_add_subdomain(&parent, &child))
+			pr_warn("%pOF failed to add subdomain: %pOF\n",
+				parent.np, child.np);
+		else
+			pr_info("%pOF has as child subdomain: %pOF.\n",
+				parent.np, child.np);
+	}
+
+	return 0;
+
+err_out:
+	dev_warn(dev, "failed to probe pd %s", pd->pd.name);
+	return ret;
+}
+
+static struct platform_driver reg_pd_driver = {
+	.driver = {
+		.name	= "regulator_power_domain",
+		.of_match_table = reg_pd_of_match,
+		.owner	= THIS_MODULE,
+	},
+	.probe = reg_pd_probe,
+};
+
+static __init int reg_pd_init(void)
+{
+	return platform_driver_register(&reg_pd_driver);
+}
+core_initcall(reg_pd_init);
-- 
2.20.1

[PATCH v1 3/5] MAINTAINERS: add REGULATOR POWER DOMAIN

[Max Krummenacher]

From: Max Krummenacher <max.krummenacher@toradex.com>

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

Signed-off-by: Max Krummenacher <max.krummenacher@toradex.com>
---

 MAINTAINERS | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index a6d3bd9d2a8d..cb0cf81034c2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16922,6 +16922,15 @@ F:	Documentation/devicetree/bindings/regmap/
 F:	drivers/base/regmap/
 F:	include/linux/regmap.h
 
+REGULATOR POWER DOMAIN
+M:	Max Krummenacher <max.krummenacher@toradex.com>
+L:	linux-pm@vger.kernel.org
+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:	reiserfs-devel@vger.kernel.org
 S:	Supported
-- 
2.20.1

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

[Max Krummenacher]

From: Max Krummenacher <max.krummenacher@toradex.com>

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

Signed-off-by: Max Krummenacher <max.krummenacher@toradex.com>
---

 arch/arm64/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 7d1105343bc2..fa53a85cee18 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -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
-- 
2.20.1

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

[Max Krummenacher]

From: Max Krummenacher <max.krummenacher@toradex.com>

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.

Signed-off-by: Max Krummenacher <max.krummenacher@toradex.com>

---

 .../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(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mm-verdin-dahlia.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-verdin-dahlia.dtsi
index c2a5c2f7b204..03416dc593d8 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm-verdin-dahlia.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm-verdin-dahlia.dtsi
@@ -92,6 +92,7 @@
 
 /* Verdin PCIE_1 */
 &pcie0 {
+	power-domains = <&pd_sleep_moci>;
 	status = "okay";
 };
 
diff --git a/arch/arm64/boot/dts/freescale/imx8mm-verdin-dev.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-verdin-dev.dtsi
index 73cc3fafa018..f887b907fdde 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm-verdin-dev.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm-verdin-dev.dtsi
@@ -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 */
diff --git a/arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi
index eafa88d980b3..b5daa8f83bef 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi
@@ -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) */
-- 
2.20.1

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

[Geert Uytterhoeven]

Hi Max,

On Thu, Jun 9, 2022 at 5:16 PM Max Krummenacher <max.oss.09@gmail.com> wrote:
> From: Max Krummenacher <max.krummenacher@toradex.com>
>
> 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.
>
> Signed-off-by: Max Krummenacher <max.krummenacher@toradex.com>

Thanks for your patch!

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

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

>         status = "okay";
>  };

> --- a/arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi
> @@ -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.

> +               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 -- geert@linux-m68k.org

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

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

[Max Krummenacher]

Hi

On Tue, Jun 14, 2022 at 9:29 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Max,
>
> On Thu, Jun 9, 2022 at 5:16 PM Max Krummenacher <max.oss.09@gmail.com> wrote:
> > From: Max Krummenacher <max.krummenacher@toradex.com>
> >
> > 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.
> >
> > Signed-off-by: Max Krummenacher <max.krummenacher@toradex.com>
>
> Thanks for your patch!
>
> > --- a/arch/arm64/boot/dts/freescale/imx8mm-verdin-dahlia.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx8mm-verdin-dahlia.dtsi
> > @@ -92,6 +92,7 @@
> >
> >  /* Verdin PCIE_1 */
> >  &pcie0 {
> > +       power-domains = <&pd_sleep_moci>;
>
> This overrides "power-domains = <&pgc_pcie>;" from imx8mm.dtsi...
>
> >         status = "okay";
> >  };
>
> > --- a/arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi
> > @@ -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.

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

>
> > +               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 -- geert@linux-m68k.org
>
> 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

Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls

[Rob Herring]

On Thu, Jun 09, 2022 at 05:08:46PM +0200, Max Krummenacher wrote:
> From: Max Krummenacher <max.krummenacher@toradex.com>
> 
> 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...

> 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?

> 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.

> 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

Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls

[Geert Uytterhoeven]

Hi Rob,

On Mon, Jun 13, 2022 at 9:15 PM Rob Herring <robh@kernel.org> wrote:
> On Thu, Jun 09, 2022 at 05:08:46PM +0200, Max Krummenacher wrote:
> > From: Max Krummenacher <max.krummenacher@toradex.com>
> >
> > 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...

> > 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 -- geert@linux-m68k.org

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

Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls

[Max Krummenacher]

Hi

On Tue, Jun 14, 2022 at 9:22 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Rob,
>
> On Mon, Jun 13, 2022 at 9:15 PM Rob Herring <robh@kernel.org> wrote:
> > On Thu, Jun 09, 2022 at 05:08:46PM +0200, Max Krummenacher wrote:
> > > From: Max Krummenacher <max.krummenacher@toradex.com>
> > >
> > > 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.

>
> > > 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 -- geert@linux-m68k.org
>
> 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

Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls

[Krzysztof Kozlowski]

On 15/06/2022 09:10, Max Krummenacher wrote:
> Hi
> 
> On Tue, Jun 14, 2022 at 9:22 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>>
>> Hi Rob,
>>
>> On Mon, Jun 13, 2022 at 9:15 PM Rob Herring <robh@kernel.org> wrote:
>>> On Thu, Jun 09, 2022 at 05:08:46PM +0200, Max Krummenacher wrote:
>>>> From: Max Krummenacher <max.krummenacher@toradex.com>
>>>>
>>>> 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.

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

Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls

[Marcel Ziswiler]

Hi

On Wed, 2022-06-15 at 10:15 -0700, Krzysztof Kozlowski wrote:
> On 15/06/2022 09:10, Max Krummenacher wrote:
> > Hi
> > 
> > On Tue, Jun 14, 2022 at 9:22 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > 
> > > Hi Rob,
> > > 
> > > On Mon, Jun 13, 2022 at 9:15 PM Rob Herring <robh@kernel.org> wrote:
> > > > On Thu, Jun 09, 2022 at 05:08:46PM +0200, Max Krummenacher wrote:
> > > > > From: Max Krummenacher <max.krummenacher@toradex.com>
> > > > > 
> > > > > 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.
> 
> 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.

> Best regards,
> Krzysztof

Cheers

Marcel

Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls

[Krzysztof Kozlowski]

On 15/06/2022 10:31, Marcel Ziswiler wrote:
> Hi
> 
> On Wed, 2022-06-15 at 10:15 -0700, Krzysztof Kozlowski wrote:
>> On 15/06/2022 09:10, Max Krummenacher wrote:
>>> Hi
>>>
>>> On Tue, Jun 14, 2022 at 9:22 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>>>>
>>>> Hi Rob,
>>>>
>>>> On Mon, Jun 13, 2022 at 9:15 PM Rob Herring <robh@kernel.org> wrote:
>>>>> On Thu, Jun 09, 2022 at 05:08:46PM +0200, Max Krummenacher wrote:
>>>>>> From: Max Krummenacher <max.krummenacher@toradex.com>
>>>>>>
>>>>>> 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.
>>
>> 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. 

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.

> 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.

> 
> 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

Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls

[Marcel Ziswiler]

On Wed, 2022-06-15 at 10:37 -0700, Krzysztof Kozlowski wrote:
> On 15/06/2022 10:31, Marcel Ziswiler wrote:
> > Hi
> > 
> > On Wed, 2022-06-15 at 10:15 -0700, Krzysztof Kozlowski wrote:
> > > On 15/06/2022 09:10, Max Krummenacher wrote:
> > > > Hi
> > > > 
> > > > On Tue, Jun 14, 2022 at 9:22 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > > 
> > > > > Hi Rob,
> > > > > 
> > > > > On Mon, Jun 13, 2022 at 9:15 PM Rob Herring <robh@kernel.org> wrote:
> > > > > > On Thu, Jun 09, 2022 at 05:08:46PM +0200, Max Krummenacher wrote:
> > > > > > > From: Max Krummenacher <max.krummenacher@toradex.com>
> > > > > > > 
> > > > > > > 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.
> > > 
> > > 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. 
> 
> 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?

> > 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.

> > 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).

> 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.

> Best regards,
> Krzysztof

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

Cheers

Marcel

Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls

[Dmitry Baryshkov]

On 15/06/2022 21:13, Marcel Ziswiler wrote:
> On Wed, 2022-06-15 at 10:37 -0700, Krzysztof Kozlowski wrote:
>> On 15/06/2022 10:31, Marcel Ziswiler wrote:
>>> Hi
>>>
>>> On Wed, 2022-06-15 at 10:15 -0700, Krzysztof Kozlowski wrote:
>>>> On 15/06/2022 09:10, Max Krummenacher wrote:
>>>>> Hi
>>>>>
>>>>> On Tue, Jun 14, 2022 at 9:22 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>>>>>>
>>>>>> Hi Rob,
>>>>>>
>>>>>> On Mon, Jun 13, 2022 at 9:15 PM Rob Herring <robh@kernel.org> wrote:
>>>>>>> On Thu, Jun 09, 2022 at 05:08:46PM +0200, Max Krummenacher wrote:
>>>>>>>> From: Max Krummenacher <max.krummenacher@toradex.com>
>>>>>>>>
>>>>>>>> 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.
>>>>
>>>> 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.
>>
>> 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?
> 
>>> 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.

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.

-- 
With best wishes
Dmitry

Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls

[Krzysztof Kozlowski]

On 15/06/2022 11:13, Marcel Ziswiler wrote:
> On Wed, 2022-06-15 at 10:37 -0700, Krzysztof Kozlowski wrote:
>> On 15/06/2022 10:31, Marcel Ziswiler wrote:
>>> Hi
>>>
>>> On Wed, 2022-06-15 at 10:15 -0700, Krzysztof Kozlowski wrote:
>>>> On 15/06/2022 09:10, Max Krummenacher wrote:
>>>>> Hi
>>>>>
>>>>> On Tue, Jun 14, 2022 at 9:22 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>>>>>>
>>>>>> Hi Rob,
>>>>>>
>>>>>> On Mon, Jun 13, 2022 at 9:15 PM Rob Herring <robh@kernel.org> wrote:
>>>>>>> On Thu, Jun 09, 2022 at 05:08:46PM +0200, Max Krummenacher wrote:
>>>>>>>> From: Max Krummenacher <max.krummenacher@toradex.com>
>>>>>>>>
>>>>>>>> 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.
>>>>
>>>> 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. 
>>
>> 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?
> 
>>> 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 

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

> 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.

> 
>>> 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).
> 
>> 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

Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls

[Robin Murphy]

On 2022-06-15 18:31, Marcel Ziswiler wrote:
> Hi
> 
> On Wed, 2022-06-15 at 10:15 -0700, Krzysztof Kozlowski wrote:
>> On 15/06/2022 09:10, Max Krummenacher wrote:
>>> Hi
>>>
>>> On Tue, Jun 14, 2022 at 9:22 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>>>>
>>>> Hi Rob,
>>>>
>>>> On Mon, Jun 13, 2022 at 9:15 PM Rob Herring <robh@kernel.org> wrote:
>>>>> On Thu, Jun 09, 2022 at 05:08:46PM +0200, Max Krummenacher wrote:
>>>>>> From: Max Krummenacher <max.krummenacher@toradex.com>
>>>>>>
>>>>>> 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.
>>
>> 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.

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.

Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls

[Mark Brown]

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

On Wed, Jun 15, 2022 at 07:24:50PM +0100, Robin Murphy wrote:

> 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...

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.  

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

Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls

[Ulf Hansson]

On Thu, 9 Jun 2022 at 08:09, Max Krummenacher <max.oss.09@gmail.com> wrote:
>
> From: Max Krummenacher <max.krummenacher@toradex.com>
>
> 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.

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

Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls

[Linus Walleij]

On Thu, Jun 9, 2022 at 5:10 PM Max Krummenacher <max.oss.09@gmail.com> wrote:

> 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

Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls

[Max Krummenacher]

Hi all

On Thu, Jun 16, 2022 at 2:51 PM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Thu, Jun 9, 2022 at 5:10 PM Max Krummenacher <max.oss.09@gmail.com> wrote:
>
> > 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

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

Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls

[Ulf Hansson]

On Thu, 23 Jun 2022 at 18:14, Max Krummenacher <max.oss.09@gmail.com> wrote:
>
> Hi all
>
> On Thu, Jun 16, 2022 at 2:51 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> >
> > On Thu, Jun 9, 2022 at 5:10 PM Max Krummenacher <max.oss.09@gmail.com> wrote:
> >
> > > 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().

> >
> > 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.

> >
> > 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.

>
> 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.

>
>
> 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?

> Any suggestions on this topic? Thanks.
>
> Cheers
> Max

Kind regards
Uffe

Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls

[Linus Walleij]

On Wed, Jul 13, 2022 at 1:44 PM Ulf Hansson <ulf.hansson@linaro.org> wrote:

> > > 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!

> > > 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.

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

Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls

[Francesco Dolcini]

Hello Ulf and everybody,

On Wed, Jul 13, 2022 at 01:43:28PM +0200, Ulf Hansson wrote:
> On Thu, 23 Jun 2022 at 18:14, Max Krummenacher <max.oss.09@gmail.com> wrote:
> > 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?

> 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

Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls

[Ulf Hansson]

On Tue, 26 Jul 2022 at 18:03, Francesco Dolcini
<francesco.dolcini@toradex.com> wrote:
>
> Hello Ulf and everybody,
>
> On Wed, Jul 13, 2022 at 01:43:28PM +0200, Ulf Hansson wrote:
> > On Thu, 23 Jun 2022 at 18:14, Max Krummenacher <max.oss.09@gmail.com> wrote:
> > > 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.

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.

>
> 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.

>
> > 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
>

Kind regards
Uffe

Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls

[Francesco Dolcini]

On Thu, Jul 28, 2022 at 11:37:07AM +0200, Ulf Hansson wrote:
> On Tue, 26 Jul 2022 at 18:03, Francesco Dolcini
> <francesco.dolcini@toradex.com> wrote:
> >
> > Hello Ulf and everybody,
> >
> > On Wed, Jul 13, 2022 at 01:43:28PM +0200, Ulf Hansson wrote:
> > > On Thu, 23 Jun 2022 at 18:14, Max Krummenacher <max.oss.09@gmail.com> wrote:
> > > > 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.
> 
> 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.
> 
> >
> > 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.

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

Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls

[Ulf Hansson]

On Thu, 28 Jul 2022 at 13:21, Francesco Dolcini
<francesco.dolcini@toradex.com> wrote:
>
> On Thu, Jul 28, 2022 at 11:37:07AM +0200, Ulf Hansson wrote:
> > On Tue, 26 Jul 2022 at 18:03, Francesco Dolcini
> > <francesco.dolcini@toradex.com> wrote:
> > >
> > > Hello Ulf and everybody,
> > >
> > > On Wed, Jul 13, 2022 at 01:43:28PM +0200, Ulf Hansson wrote:
> > > > On Thu, 23 Jun 2022 at 18:14, Max Krummenacher <max.oss.09@gmail.com> wrote:
> > > > > 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.
> >
> > 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.
> >
> > >
> > > 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.
>
> 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

Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls

[Francesco Dolcini]

Hello Ulf,

On Fri, Aug 26, 2022 at 03:50:46PM +0200, Ulf Hansson wrote:
> On Thu, 28 Jul 2022 at 13:21, Francesco Dolcini
> <francesco.dolcini@toradex.com> wrote:
> >
> > On Thu, Jul 28, 2022 at 11:37:07AM +0200, Ulf Hansson wrote:
> > > On Tue, 26 Jul 2022 at 18:03, Francesco Dolcini
> > > <francesco.dolcini@toradex.com> wrote:
> > > >
> > > > Hello Ulf and everybody,
> > > >
> > > > On Wed, Jul 13, 2022 at 01:43:28PM +0200, Ulf Hansson wrote:
> > > > > On Thu, 23 Jun 2022 at 18:14, Max Krummenacher <max.oss.09@gmail.com> wrote:
> > > > > > 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.
> > >
> > > 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.
> > >
> > > >
> > > > 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.
> >
> > 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.
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.

> 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]:
  > 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

> 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/20220613191549.GA4092455-robh@kernel.org/
[2] https://lore.kernel.org/all/20220609150851.23084-6-max.oss.09@gmail.com/

Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls

[Ulf Hansson]

On Fri, 9 Sept 2022 at 16:22, Francesco Dolcini
<francesco.dolcini@toradex.com> wrote:
>
> Hello Ulf,
>
> On Fri, Aug 26, 2022 at 03:50:46PM +0200, Ulf Hansson wrote:
> > On Thu, 28 Jul 2022 at 13:21, Francesco Dolcini
> > <francesco.dolcini@toradex.com> wrote:
> > >
> > > On Thu, Jul 28, 2022 at 11:37:07AM +0200, Ulf Hansson wrote:
> > > > On Tue, 26 Jul 2022 at 18:03, Francesco Dolcini
> > > > <francesco.dolcini@toradex.com> wrote:
> > > > >
> > > > > Hello Ulf and everybody,
> > > > >
> > > > > On Wed, Jul 13, 2022 at 01:43:28PM +0200, Ulf Hansson wrote:
> > > > > > On Thu, 23 Jun 2022 at 18:14, Max Krummenacher <max.oss.09@gmail.com> wrote:
> > > > > > > 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.
> > > >
> > > > 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.
> > > >
> > > > >
> > > > > 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.
> > >
> > > 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.
> 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

>
> 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.

>
> > 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.

>
> 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.

>
> 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]:
>   > 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

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?

>
> > 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/20220613191549.GA4092455-robh@kernel.org/
> [2] https://lore.kernel.org/all/20220609150851.23084-6-max.oss.09@gmail.com/
>

Kind regards
Uffe

Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls

[Krzysztof Kozlowski]

On 22/09/2022 15:49, Ulf Hansson wrote:
> On Fri, 9 Sept 2022 at 16:22, Francesco Dolcini
> <francesco.dolcini@toradex.com> wrote:
>>
>> Hello Ulf,
>>
>> On Fri, Aug 26, 2022 at 03:50:46PM +0200, Ulf Hansson wrote:
>>> On Thu, 28 Jul 2022 at 13:21, Francesco Dolcini
>>> <francesco.dolcini@toradex.com> wrote:
>>>>
>>>> On Thu, Jul 28, 2022 at 11:37:07AM +0200, Ulf Hansson wrote:
>>>>> On Tue, 26 Jul 2022 at 18:03, Francesco Dolcini
>>>>> <francesco.dolcini@toradex.com> wrote:
>>>>>>
>>>>>> Hello Ulf and everybody,
>>>>>>
>>>>>> On Wed, Jul 13, 2022 at 01:43:28PM +0200, Ulf Hansson wrote:
>>>>>>> On Thu, 23 Jun 2022 at 18:14, Max Krummenacher <max.oss.09@gmail.com> wrote:
>>>>>>>> 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.
>>>>>
>>>>> 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.
>>>>>
>>>>>>
>>>>>> 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.
>>>>
>>>> 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.
>> 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
> 
>>
>> 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.
> 
>>
>>> 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.
> 
>>
>> 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.
> 
>>
>> 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]:
>>   > 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
> 
> 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?

> 
> 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

Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls

[Ulf Hansson]

On Fri, 23 Sept 2022 at 20:00, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 22/09/2022 15:49, Ulf Hansson wrote:
> > On Fri, 9 Sept 2022 at 16:22, Francesco Dolcini
> > <francesco.dolcini@toradex.com> wrote:
> >>
> >> Hello Ulf,
> >>
> >> On Fri, Aug 26, 2022 at 03:50:46PM +0200, Ulf Hansson wrote:
> >>> On Thu, 28 Jul 2022 at 13:21, Francesco Dolcini
> >>> <francesco.dolcini@toradex.com> wrote:
> >>>>
> >>>> On Thu, Jul 28, 2022 at 11:37:07AM +0200, Ulf Hansson wrote:
> >>>>> On Tue, 26 Jul 2022 at 18:03, Francesco Dolcini
> >>>>> <francesco.dolcini@toradex.com> wrote:
> >>>>>>
> >>>>>> Hello Ulf and everybody,
> >>>>>>
> >>>>>> On Wed, Jul 13, 2022 at 01:43:28PM +0200, Ulf Hansson wrote:
> >>>>>>> On Thu, 23 Jun 2022 at 18:14, Max Krummenacher <max.oss.09@gmail.com> wrote:
> >>>>>>>> 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.
> >>>>>
> >>>>> 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.
> >>>>>
> >>>>>>
> >>>>>> 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.
> >>>>
> >>>> 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.
> >> 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
> >
> >>
> >> 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.
> >
> >>
> >>> 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.
> >
> >>
> >> 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.
> >
> >>
> >> 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]:
> >>   > 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
> >
> > 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?
>
> >
> > 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.

>
> 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?

>
>
> Best regards,
> Krzysztof
>

Kind regards
Uffe

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

Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls

[Krzysztof Kozlowski]

On 26/09/2022 12:12, Ulf Hansson wrote:
> On Fri, 23 Sept 2022 at 20:00, Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 22/09/2022 15:49, Ulf Hansson wrote:
>>> On Fri, 9 Sept 2022 at 16:22, Francesco Dolcini
>>> <francesco.dolcini@toradex.com> wrote:
>>>>
>>>> Hello Ulf,
>>>>
>>>> On Fri, Aug 26, 2022 at 03:50:46PM +0200, Ulf Hansson wrote:
>>>>> On Thu, 28 Jul 2022 at 13:21, Francesco Dolcini
>>>>> <francesco.dolcini@toradex.com> wrote:
>>>>>>
>>>>>> On Thu, Jul 28, 2022 at 11:37:07AM +0200, Ulf Hansson wrote:
>>>>>>> On Tue, 26 Jul 2022 at 18:03, Francesco Dolcini
>>>>>>> <francesco.dolcini@toradex.com> wrote:
>>>>>>>>
>>>>>>>> Hello Ulf and everybody,
>>>>>>>>
>>>>>>>> On Wed, Jul 13, 2022 at 01:43:28PM +0200, Ulf Hansson wrote:
>>>>>>>>> On Thu, 23 Jun 2022 at 18:14, Max Krummenacher <max.oss.09@gmail.com> wrote:
>>>>>>>>>> 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.
>>>>>>>
>>>>>>> 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.
>>>>>>>
>>>>>>>>
>>>>>>>> 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.
>>>>>>
>>>>>> 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.
>>>> 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
>>>
>>>>
>>>> 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.
>>>
>>>>
>>>>> 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.
>>>
>>>>
>>>> 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.
>>>
>>>>
>>>> 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]:
>>>>   > 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
>>>
>>> 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?
>>
>>>
>>> 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.
> 
>>
>> 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.

> 
> 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

Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls

[Ulf Hansson]

[...]

> >>>>
> >>>> 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.
> >>>
> >>>>
> >>>> 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]:
> >>>>   > 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
> >>>
> >>> 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?
> >>
> >>>
> >>> 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.
> >
> >>
> >> 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.

Right, apologize for my ignorance.

>
> >
> > 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.

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

Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls

[Geert Uytterhoeven]

Hi Ulf,

On Tue, Sep 27, 2022 at 11:49 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> > >>>> 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.
> > >>>
> > >>>>
> > >>>> 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]:
> > >>>>   > 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
> > >>>
> > >>> 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?
> > >>
> > >>>
> > >>> 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.
> > >
> > >>
> > >> 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.
>
> Right, apologize for my ignorance.
>
> >
> > >
> > > 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.
>
> 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 -- geert@linux-m68k.org

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

Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls

[Krzysztof Kozlowski]

On 27/09/2022 14:49, Geert Uytterhoeven wrote:
> Hi Ulf,
> 
> On Tue, Sep 27, 2022 at 11:49 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>>>>>>> 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.
>>>>>>
>>>>>>>
>>>>>>> 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]:
>>>>>>>   > 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
>>>>>>
>>>>>> 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?
>>>>>
>>>>>>
>>>>>> 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.
>>>>
>>>>>
>>>>> 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.
>>
>> Right, apologize for my ignorance.
>>
>>>
>>>>
>>>> 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.
>>
>> 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.

>>
>> 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.

> 
> 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.


> 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

Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls

[Ulf Hansson]

On Tue, 27 Sept 2022 at 16:27, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 27/09/2022 14:49, Geert Uytterhoeven wrote:
> > Hi Ulf,
> >
> > On Tue, Sep 27, 2022 at 11:49 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> >>>>>>> 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.
> >>>>>>
> >>>>>>>
> >>>>>>> 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]:
> >>>>>>>   > 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
> >>>>>>
> >>>>>> 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?
> >>>>>
> >>>>>>
> >>>>>> 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.
> >>>>
> >>>>>
> >>>>> 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.
> >>
> >> Right, apologize for my ignorance.
> >>
> >>>
> >>>>
> >>>> 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.
> >>
> >> 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.

>
> >>
> >> 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.
>
> >
> > 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.

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