* [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls @ 2022-06-09 15:08 Max Krummenacher 2022-06-09 15:08 ` [PATCH v1 1/5] dt-bindings: power: Add bindings for a power domain controlled by a regulator Max Krummenacher ` (4 more replies) 0 siblings, 5 replies; 39+ messages in thread From: Max Krummenacher @ 2022-06-09 15:08 UTC (permalink / raw) To: max.krummenacher Cc: Ulf Hansson, linux-pm, Francesco Dolcini, Mark Brown, Rafael J . Wysocki, Kevin Hilman, Andrejs Cainikovs, Biju Das, Bjorn Andersson, Catalin Marinas, Dmitry Baryshkov, Fabio Estevam, Geert Uytterhoeven, Krzysztof Kozlowski, Marcel Ziswiler, NXP Linux Team, Pengutronix Kernel Team, Rob Herring, Sascha Hauer, Shawn Guo, Vinod Koul, Will Deacon, devicetree, linux-arm-kernel, linux-kernel 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v1 1/5] dt-bindings: power: Add bindings for a power domain controlled by a regulator 2022-06-09 15:08 [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls Max Krummenacher @ 2022-06-09 15:08 ` Max Krummenacher 2022-06-10 2:57 ` Rob Herring 2022-06-14 7:23 ` Geert Uytterhoeven 2022-06-09 15:08 ` [RFC PATCH v1 5/5] ARM64: verdin-imx8mm: use regulator power domain to model sleep-moci Max Krummenacher ` (3 subsequent siblings) 4 siblings, 2 replies; 39+ messages in thread From: Max Krummenacher @ 2022-06-09 15:08 UTC (permalink / raw) To: max.krummenacher Cc: Ulf Hansson, linux-pm, Francesco Dolcini, Mark Brown, Rafael J . Wysocki, Kevin Hilman, Krzysztof Kozlowski, Rob Herring, devicetree, linux-kernel 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 = <®_pd_sleep_moci>; + label = "pd_sleep_moci"; + #power-domain-cells = <0>; + }; -- 2.20.1 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH v1 1/5] dt-bindings: power: Add bindings for a power domain controlled by a regulator 2022-06-09 15:08 ` [PATCH v1 1/5] dt-bindings: power: Add bindings for a power domain controlled by a regulator Max Krummenacher @ 2022-06-10 2:57 ` Rob Herring 2022-06-15 15:19 ` Max Krummenacher 2022-06-14 7:23 ` Geert Uytterhoeven 1 sibling, 1 reply; 39+ messages in thread From: Rob Herring @ 2022-06-10 2:57 UTC (permalink / raw) To: Max Krummenacher Cc: Kevin Hilman, Krzysztof Kozlowski, Ulf Hansson, Francesco Dolcini, Rob Herring, linux-kernel, Rafael J . Wysocki, Mark Brown, devicetree, linux-pm, max.krummenacher 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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 1/5] dt-bindings: power: Add bindings for a power domain controlled by a regulator 2022-06-10 2:57 ` Rob Herring @ 2022-06-15 15:19 ` Max Krummenacher 2022-06-15 17:16 ` Krzysztof Kozlowski 0 siblings, 1 reply; 39+ messages in thread From: Max Krummenacher @ 2022-06-15 15:19 UTC (permalink / raw) To: Rob Herring Cc: Kevin Hilman, Krzysztof Kozlowski, Ulf Hansson, Francesco Dolcini, Rob Herring, Linux Kernel Mailing List, Rafael J . Wysocki, Mark Brown, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, linux-pm, 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. > ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 1/5] dt-bindings: power: Add bindings for a power domain controlled by a regulator 2022-06-15 15:19 ` Max Krummenacher @ 2022-06-15 17:16 ` Krzysztof Kozlowski 0 siblings, 0 replies; 39+ messages in thread From: Krzysztof Kozlowski @ 2022-06-15 17:16 UTC (permalink / raw) To: Max Krummenacher, Rob Herring Cc: Kevin Hilman, Krzysztof Kozlowski, Ulf Hansson, Francesco Dolcini, Rob Herring, Linux Kernel Mailing List, Rafael J . Wysocki, Mark Brown, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, linux-pm, Max Krummenacher 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 1/5] dt-bindings: power: Add bindings for a power domain controlled by a regulator 2022-06-09 15:08 ` [PATCH v1 1/5] dt-bindings: power: Add bindings for a power domain controlled by a regulator Max Krummenacher 2022-06-10 2:57 ` Rob Herring @ 2022-06-14 7:23 ` Geert Uytterhoeven 2022-06-15 15:18 ` Max Krummenacher 1 sibling, 1 reply; 39+ messages in thread From: Geert Uytterhoeven @ 2022-06-14 7:23 UTC (permalink / raw) To: Max Krummenacher Cc: Max Krummenacher, Ulf Hansson, Linux PM list, Francesco Dolcini, Mark Brown, Rafael J . Wysocki, Kevin Hilman, Krzysztof Kozlowski, Rob Herring, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Linux Kernel Mailing List 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 1/5] dt-bindings: power: Add bindings for a power domain controlled by a regulator 2022-06-14 7:23 ` Geert Uytterhoeven @ 2022-06-15 15:18 ` Max Krummenacher 0 siblings, 0 replies; 39+ messages in thread From: Max Krummenacher @ 2022-06-15 15:18 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Max Krummenacher, Ulf Hansson, Linux PM list, Francesco Dolcini, Mark Brown, Rafael J . Wysocki, Kevin Hilman, Krzysztof Kozlowski, Rob Herring, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Linux Kernel Mailing List 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* [RFC PATCH v1 5/5] ARM64: verdin-imx8mm: use regulator power domain to model sleep-moci 2022-06-09 15:08 [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls Max Krummenacher 2022-06-09 15:08 ` [PATCH v1 1/5] dt-bindings: power: Add bindings for a power domain controlled by a regulator Max Krummenacher @ 2022-06-09 15:08 ` Max Krummenacher 2022-06-14 7:29 ` Geert Uytterhoeven 2022-06-13 19:15 ` [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls Rob Herring ` (2 subsequent siblings) 4 siblings, 1 reply; 39+ messages in thread From: Max Krummenacher @ 2022-06-09 15:08 UTC (permalink / raw) To: max.krummenacher Cc: Ulf Hansson, linux-pm, Francesco Dolcini, Mark Brown, Rafael J . Wysocki, Kevin Hilman, Andrejs Cainikovs, Fabio Estevam, Krzysztof Kozlowski, Marcel Ziswiler, NXP Linux Team, Pengutronix Kernel Team, Rob Herring, Sascha Hauer, Shawn Guo, devicetree, linux-arm-kernel, linux-kernel 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 = <®_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 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [RFC PATCH v1 5/5] ARM64: verdin-imx8mm: use regulator power domain to model sleep-moci 2022-06-09 15:08 ` [RFC PATCH v1 5/5] ARM64: verdin-imx8mm: use regulator power domain to model sleep-moci Max Krummenacher @ 2022-06-14 7:29 ` Geert Uytterhoeven 2022-06-15 15:32 ` Max Krummenacher 0 siblings, 1 reply; 39+ messages in thread From: Geert Uytterhoeven @ 2022-06-14 7:29 UTC (permalink / raw) To: Max Krummenacher Cc: Max Krummenacher, Ulf Hansson, Linux PM list, Francesco Dolcini, Mark Brown, Rafael J . Wysocki, Kevin Hilman, Andrejs Cainikovs, Fabio Estevam, Krzysztof Kozlowski, Marcel Ziswiler, NXP Linux Team, Pengutronix Kernel Team, Rob Herring, Sascha Hauer, Shawn Guo, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Linux ARM, Linux Kernel Mailing List 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 = <®_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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC PATCH v1 5/5] ARM64: verdin-imx8mm: use regulator power domain to model sleep-moci 2022-06-14 7:29 ` Geert Uytterhoeven @ 2022-06-15 15:32 ` Max Krummenacher 0 siblings, 0 replies; 39+ messages in thread From: Max Krummenacher @ 2022-06-15 15:32 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Max Krummenacher, Ulf Hansson, Linux PM list, Francesco Dolcini, Mark Brown, Rafael J . Wysocki, Kevin Hilman, Andrejs Cainikovs, Fabio Estevam, Krzysztof Kozlowski, Marcel Ziswiler, NXP Linux Team, Pengutronix Kernel Team, Rob Herring, Sascha Hauer, Shawn Guo, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Linux ARM, Linux Kernel Mailing List 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 = <®_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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls 2022-06-09 15:08 [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls Max Krummenacher 2022-06-09 15:08 ` [PATCH v1 1/5] dt-bindings: power: Add bindings for a power domain controlled by a regulator Max Krummenacher 2022-06-09 15:08 ` [RFC PATCH v1 5/5] ARM64: verdin-imx8mm: use regulator power domain to model sleep-moci Max Krummenacher @ 2022-06-13 19:15 ` Rob Herring 2022-06-14 7:22 ` Geert Uytterhoeven 2022-06-15 21:14 ` Ulf Hansson 2022-06-16 12:50 ` Linus Walleij 4 siblings, 1 reply; 39+ messages in thread From: Rob Herring @ 2022-06-13 19:15 UTC (permalink / raw) To: Max Krummenacher Cc: max.krummenacher, Ulf Hansson, linux-pm, Francesco Dolcini, Mark Brown, Rafael J . Wysocki, Kevin Hilman, Andrejs Cainikovs, Biju Das, Bjorn Andersson, Catalin Marinas, Dmitry Baryshkov, Fabio Estevam, Geert Uytterhoeven, Krzysztof Kozlowski, Marcel Ziswiler, NXP Linux Team, Pengutronix Kernel Team, Sascha Hauer, Shawn Guo, Vinod Koul, Will Deacon, devicetree, linux-arm-kernel, linux-kernel 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls 2022-06-13 19:15 ` [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls Rob Herring @ 2022-06-14 7:22 ` Geert Uytterhoeven 2022-06-15 16:10 ` Max Krummenacher 0 siblings, 1 reply; 39+ messages in thread From: Geert Uytterhoeven @ 2022-06-14 7:22 UTC (permalink / raw) To: Rob Herring Cc: Max Krummenacher, Max Krummenacher, Ulf Hansson, Linux PM list, Francesco Dolcini, Mark Brown, Rafael J . Wysocki, Kevin Hilman, Andrejs Cainikovs, Biju Das, Bjorn Andersson, Catalin Marinas, Dmitry Baryshkov, Fabio Estevam, Geert Uytterhoeven, Krzysztof Kozlowski, Marcel Ziswiler, NXP Linux Team, Pengutronix Kernel Team, Sascha Hauer, Shawn Guo, Vinod Koul, Will Deacon, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Linux ARM, Linux Kernel Mailing List 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls 2022-06-14 7:22 ` Geert Uytterhoeven @ 2022-06-15 16:10 ` Max Krummenacher 2022-06-15 17:15 ` Krzysztof Kozlowski 0 siblings, 1 reply; 39+ messages in thread From: Max Krummenacher @ 2022-06-15 16:10 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Rob Herring, Max Krummenacher, Ulf Hansson, Linux PM list, Francesco Dolcini, Mark Brown, Rafael J . Wysocki, Kevin Hilman, Andrejs Cainikovs, Biju Das, Bjorn Andersson, Catalin Marinas, Dmitry Baryshkov, Fabio Estevam, Geert Uytterhoeven, Krzysztof Kozlowski, Marcel Ziswiler, NXP Linux Team, Pengutronix Kernel Team, Sascha Hauer, Shawn Guo, Vinod Koul, Will Deacon, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Linux ARM, Linux Kernel Mailing List 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls 2022-06-15 16:10 ` Max Krummenacher @ 2022-06-15 17:15 ` Krzysztof Kozlowski 2022-06-15 17:31 ` Marcel Ziswiler 0 siblings, 1 reply; 39+ messages in thread From: Krzysztof Kozlowski @ 2022-06-15 17:15 UTC (permalink / raw) To: Max Krummenacher, Geert Uytterhoeven Cc: Rob Herring, Max Krummenacher, Ulf Hansson, Linux PM list, Francesco Dolcini, Mark Brown, Rafael J . Wysocki, Kevin Hilman, Andrejs Cainikovs, Biju Das, Bjorn Andersson, Catalin Marinas, Dmitry Baryshkov, Fabio Estevam, Geert Uytterhoeven, Krzysztof Kozlowski, Marcel Ziswiler, NXP Linux Team, Pengutronix Kernel Team, Sascha Hauer, Shawn Guo, Vinod Koul, Will Deacon, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Linux ARM, Linux Kernel Mailing List 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls 2022-06-15 17:15 ` Krzysztof Kozlowski @ 2022-06-15 17:31 ` Marcel Ziswiler 2022-06-15 17:37 ` Krzysztof Kozlowski 2022-06-15 18:24 ` Robin Murphy 0 siblings, 2 replies; 39+ messages in thread From: Marcel Ziswiler @ 2022-06-15 17:31 UTC (permalink / raw) To: max.oss.09@gmail.com, krzysztof.kozlowski@linaro.org, geert@linux-m68k.org Cc: linux-imx@nxp.com, Francesco Dolcini, robh@kernel.org, krzysztof.kozlowski+dt@linaro.org, ulf.hansson@linaro.org, linux-arm-kernel@lists.infradead.org, dmitry.baryshkov@linaro.org, biju.das.jz@bp.renesas.com, catalin.marinas@arm.com, geert+renesas@glider.be, bjorn.andersson@linaro.org, vkoul@kernel.org, shawnguo@kernel.org, kernel@pengutronix.de, khilman@kernel.org, s.hauer@pengutronix.de, Andrejs Cainikovs, will@kernel.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, linux-pm@vger.kernel.org, rafael@kernel.org, festevam@gmail.com, Max Krummenacher, broonie@kernel.org 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls 2022-06-15 17:31 ` Marcel Ziswiler @ 2022-06-15 17:37 ` Krzysztof Kozlowski 2022-06-15 18:13 ` Marcel Ziswiler 2022-06-15 18:24 ` Robin Murphy 1 sibling, 1 reply; 39+ messages in thread From: Krzysztof Kozlowski @ 2022-06-15 17:37 UTC (permalink / raw) To: Marcel Ziswiler, max.oss.09@gmail.com, geert@linux-m68k.org Cc: linux-imx@nxp.com, Francesco Dolcini, robh@kernel.org, krzysztof.kozlowski+dt@linaro.org, ulf.hansson@linaro.org, linux-arm-kernel@lists.infradead.org, dmitry.baryshkov@linaro.org, biju.das.jz@bp.renesas.com, catalin.marinas@arm.com, geert+renesas@glider.be, bjorn.andersson@linaro.org, vkoul@kernel.org, shawnguo@kernel.org, kernel@pengutronix.de, khilman@kernel.org, s.hauer@pengutronix.de, Andrejs Cainikovs, will@kernel.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, linux-pm@vger.kernel.org, rafael@kernel.org, festevam@gmail.com, Max Krummenacher, broonie@kernel.org 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls 2022-06-15 17:37 ` Krzysztof Kozlowski @ 2022-06-15 18:13 ` Marcel Ziswiler 2022-06-15 18:48 ` Dmitry Baryshkov 2022-06-15 20:48 ` Krzysztof Kozlowski 0 siblings, 2 replies; 39+ messages in thread From: Marcel Ziswiler @ 2022-06-15 18:13 UTC (permalink / raw) To: max.oss.09@gmail.com, krzysztof.kozlowski@linaro.org, geert@linux-m68k.org Cc: linux-imx@nxp.com, broonie@kernel.org, Francesco Dolcini, robh@kernel.org, krzysztof.kozlowski+dt@linaro.org, ulf.hansson@linaro.org, biju.das.jz@bp.renesas.com, bjorn.andersson@linaro.org, catalin.marinas@arm.com, dmitry.baryshkov@linaro.org, shawnguo@kernel.org, vkoul@kernel.org, geert+renesas@glider.be, kernel@pengutronix.de, khilman@kernel.org, s.hauer@pengutronix.de, Andrejs Cainikovs, will@kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org, rafael@kernel.org, festevam@gmail.com, devicetree@vger.kernel.org, Max Krummenacher 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls 2022-06-15 18:13 ` Marcel Ziswiler @ 2022-06-15 18:48 ` Dmitry Baryshkov 2022-06-15 20:48 ` Krzysztof Kozlowski 1 sibling, 0 replies; 39+ messages in thread From: Dmitry Baryshkov @ 2022-06-15 18:48 UTC (permalink / raw) To: Marcel Ziswiler, max.oss.09@gmail.com, krzysztof.kozlowski@linaro.org, geert@linux-m68k.org Cc: linux-imx@nxp.com, broonie@kernel.org, Francesco Dolcini, robh@kernel.org, krzysztof.kozlowski+dt@linaro.org, ulf.hansson@linaro.org, biju.das.jz@bp.renesas.com, bjorn.andersson@linaro.org, catalin.marinas@arm.com, shawnguo@kernel.org, vkoul@kernel.org, geert+renesas@glider.be, kernel@pengutronix.de, khilman@kernel.org, s.hauer@pengutronix.de, Andrejs Cainikovs, will@kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org, rafael@kernel.org, festevam@gmail.com, devicetree@vger.kernel.org, Max Krummenacher 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls 2022-06-15 18:13 ` Marcel Ziswiler 2022-06-15 18:48 ` Dmitry Baryshkov @ 2022-06-15 20:48 ` Krzysztof Kozlowski 1 sibling, 0 replies; 39+ messages in thread From: Krzysztof Kozlowski @ 2022-06-15 20:48 UTC (permalink / raw) To: Marcel Ziswiler, max.oss.09@gmail.com, geert@linux-m68k.org Cc: linux-imx@nxp.com, broonie@kernel.org, Francesco Dolcini, robh@kernel.org, krzysztof.kozlowski+dt@linaro.org, ulf.hansson@linaro.org, biju.das.jz@bp.renesas.com, bjorn.andersson@linaro.org, catalin.marinas@arm.com, dmitry.baryshkov@linaro.org, shawnguo@kernel.org, vkoul@kernel.org, geert+renesas@glider.be, kernel@pengutronix.de, khilman@kernel.org, s.hauer@pengutronix.de, Andrejs Cainikovs, will@kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org, rafael@kernel.org, festevam@gmail.com, devicetree@vger.kernel.org, Max Krummenacher 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls 2022-06-15 17:31 ` Marcel Ziswiler 2022-06-15 17:37 ` Krzysztof Kozlowski @ 2022-06-15 18:24 ` Robin Murphy 2022-06-15 19:12 ` Mark Brown 1 sibling, 1 reply; 39+ messages in thread From: Robin Murphy @ 2022-06-15 18:24 UTC (permalink / raw) To: Marcel Ziswiler, max.oss.09@gmail.com, krzysztof.kozlowski@linaro.org, geert@linux-m68k.org Cc: linux-imx@nxp.com, Francesco Dolcini, robh@kernel.org, krzysztof.kozlowski+dt@linaro.org, ulf.hansson@linaro.org, linux-arm-kernel@lists.infradead.org, dmitry.baryshkov@linaro.org, biju.das.jz@bp.renesas.com, catalin.marinas@arm.com, geert+renesas@glider.be, bjorn.andersson@linaro.org, vkoul@kernel.org, shawnguo@kernel.org, kernel@pengutronix.de, khilman@kernel.org, s.hauer@pengutronix.de, Andrejs Cainikovs, will@kernel.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, linux-pm@vger.kernel.org, rafael@kernel.org, festevam@gmail.com, Max Krummenacher, broonie@kernel.org 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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls 2022-06-15 18:24 ` Robin Murphy @ 2022-06-15 19:12 ` Mark Brown 0 siblings, 0 replies; 39+ messages in thread From: Mark Brown @ 2022-06-15 19:12 UTC (permalink / raw) To: Robin Murphy Cc: Marcel Ziswiler, max.oss.09@gmail.com, krzysztof.kozlowski@linaro.org, geert@linux-m68k.org, linux-imx@nxp.com, Francesco Dolcini, robh@kernel.org, krzysztof.kozlowski+dt@linaro.org, ulf.hansson@linaro.org, linux-arm-kernel@lists.infradead.org, dmitry.baryshkov@linaro.org, biju.das.jz@bp.renesas.com, catalin.marinas@arm.com, geert+renesas@glider.be, bjorn.andersson@linaro.org, vkoul@kernel.org, shawnguo@kernel.org, kernel@pengutronix.de, khilman@kernel.org, s.hauer@pengutronix.de, Andrejs Cainikovs, will@kernel.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, linux-pm@vger.kernel.org, rafael@kernel.org, festevam@gmail.com, Max Krummenacher [-- 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 --] ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls 2022-06-09 15:08 [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls Max Krummenacher ` (2 preceding siblings ...) 2022-06-13 19:15 ` [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls Rob Herring @ 2022-06-15 21:14 ` Ulf Hansson 2022-06-16 12:50 ` Linus Walleij 4 siblings, 0 replies; 39+ messages in thread From: Ulf Hansson @ 2022-06-15 21:14 UTC (permalink / raw) To: Max Krummenacher Cc: max.krummenacher, linux-pm, Francesco Dolcini, Mark Brown, Rafael J . Wysocki, Kevin Hilman, Andrejs Cainikovs, Biju Das, Bjorn Andersson, Catalin Marinas, Dmitry Baryshkov, Fabio Estevam, Geert Uytterhoeven, Krzysztof Kozlowski, Marcel Ziswiler, NXP Linux Team, Pengutronix Kernel Team, Rob Herring, Sascha Hauer, Shawn Guo, Vinod Koul, Will Deacon, devicetree, linux-arm-kernel, linux-kernel 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls 2022-06-09 15:08 [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls Max Krummenacher ` (3 preceding siblings ...) 2022-06-15 21:14 ` Ulf Hansson @ 2022-06-16 12:50 ` Linus Walleij 2022-06-23 16:14 ` Max Krummenacher 4 siblings, 1 reply; 39+ messages in thread From: Linus Walleij @ 2022-06-16 12:50 UTC (permalink / raw) To: Max Krummenacher Cc: max.krummenacher, Ulf Hansson, linux-pm, Francesco Dolcini, Mark Brown, Rafael J . Wysocki, Kevin Hilman, Andrejs Cainikovs, Biju Das, Bjorn Andersson, Catalin Marinas, Dmitry Baryshkov, Fabio Estevam, Geert Uytterhoeven, Krzysztof Kozlowski, Marcel Ziswiler, NXP Linux Team, Pengutronix Kernel Team, Rob Herring, Sascha Hauer, Shawn Guo, Vinod Koul, Will Deacon, devicetree, linux-arm-kernel, linux-kernel 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls 2022-06-16 12:50 ` Linus Walleij @ 2022-06-23 16:14 ` Max Krummenacher 2022-07-13 11:43 ` Ulf Hansson 0 siblings, 1 reply; 39+ messages in thread From: Max Krummenacher @ 2022-06-23 16:14 UTC (permalink / raw) To: Linus Walleij Cc: Max Krummenacher, Ulf Hansson, Linux PM list, Francesco Dolcini, Mark Brown, Rafael J . Wysocki, Kevin Hilman, Andrejs Cainikovs, Biju Das, Bjorn Andersson, Catalin Marinas, Dmitry Baryshkov, Fabio Estevam, Geert Uytterhoeven, Krzysztof Kozlowski, Marcel Ziswiler, NXP Linux Team, Pengutronix Kernel Team, Rob Herring, Sascha Hauer, Shawn Guo, Vinod Koul, Will Deacon, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Linux ARM, Linux Kernel Mailing List 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls 2022-06-23 16:14 ` Max Krummenacher @ 2022-07-13 11:43 ` Ulf Hansson 2022-07-18 9:49 ` Linus Walleij 2022-07-26 16:03 ` Francesco Dolcini 0 siblings, 2 replies; 39+ messages in thread From: Ulf Hansson @ 2022-07-13 11:43 UTC (permalink / raw) To: Max Krummenacher, Linus Walleij Cc: Max Krummenacher, Linux PM list, Francesco Dolcini, Mark Brown, Rafael J . Wysocki, Kevin Hilman, Andrejs Cainikovs, Biju Das, Bjorn Andersson, Catalin Marinas, Dmitry Baryshkov, Fabio Estevam, Geert Uytterhoeven, Krzysztof Kozlowski, Marcel Ziswiler, NXP Linux Team, Pengutronix Kernel Team, Rob Herring, Sascha Hauer, Shawn Guo, Vinod Koul, Will Deacon, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Linux ARM, Linux Kernel Mailing List 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls 2022-07-13 11:43 ` Ulf Hansson @ 2022-07-18 9:49 ` Linus Walleij 2022-07-26 16:03 ` Francesco Dolcini 1 sibling, 0 replies; 39+ messages in thread From: Linus Walleij @ 2022-07-18 9:49 UTC (permalink / raw) To: Ulf Hansson Cc: Max Krummenacher, Max Krummenacher, Linux PM list, Francesco Dolcini, Mark Brown, Rafael J . Wysocki, Kevin Hilman, Andrejs Cainikovs, Biju Das, Bjorn Andersson, Catalin Marinas, Dmitry Baryshkov, Fabio Estevam, Geert Uytterhoeven, Krzysztof Kozlowski, Marcel Ziswiler, NXP Linux Team, Pengutronix Kernel Team, Rob Herring, Sascha Hauer, Shawn Guo, Vinod Koul, Will Deacon, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Linux ARM, Linux Kernel Mailing List 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls 2022-07-13 11:43 ` Ulf Hansson 2022-07-18 9:49 ` Linus Walleij @ 2022-07-26 16:03 ` Francesco Dolcini 2022-07-28 9:37 ` Ulf Hansson 1 sibling, 1 reply; 39+ messages in thread From: Francesco Dolcini @ 2022-07-26 16:03 UTC (permalink / raw) To: Ulf Hansson Cc: Max Krummenacher, Linus Walleij, Max Krummenacher, Linux PM list, Francesco Dolcini, Mark Brown, Rafael J . Wysocki, Kevin Hilman, Andrejs Cainikovs, Biju Das, Bjorn Andersson, Catalin Marinas, Dmitry Baryshkov, Fabio Estevam, Geert Uytterhoeven, Krzysztof Kozlowski, Marcel Ziswiler, NXP Linux Team, Pengutronix Kernel Team, Rob Herring, Sascha Hauer, Shawn Guo, Vinod Koul, Will Deacon, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Linux ARM, Linux Kernel Mailing List 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls 2022-07-26 16:03 ` Francesco Dolcini @ 2022-07-28 9:37 ` Ulf Hansson 2022-07-28 11:21 ` Francesco Dolcini 0 siblings, 1 reply; 39+ messages in thread From: Ulf Hansson @ 2022-07-28 9:37 UTC (permalink / raw) To: Francesco Dolcini Cc: Max Krummenacher, Linus Walleij, Max Krummenacher, Linux PM list, Mark Brown, Rafael J . Wysocki, Kevin Hilman, Andrejs Cainikovs, Biju Das, Bjorn Andersson, Catalin Marinas, Dmitry Baryshkov, Fabio Estevam, Geert Uytterhoeven, Krzysztof Kozlowski, Marcel Ziswiler, NXP Linux Team, Pengutronix Kernel Team, Rob Herring, Sascha Hauer, Shawn Guo, Vinod Koul, Will Deacon, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Linux ARM, Linux Kernel Mailing List 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls 2022-07-28 9:37 ` Ulf Hansson @ 2022-07-28 11:21 ` Francesco Dolcini 2022-08-26 13:50 ` Ulf Hansson 0 siblings, 1 reply; 39+ messages in thread From: Francesco Dolcini @ 2022-07-28 11:21 UTC (permalink / raw) To: Ulf Hansson, Mark Brown, Krzysztof Kozlowski, Robin Murphy Cc: Francesco Dolcini, Max Krummenacher, Linus Walleij, Max Krummenacher, Linux PM list, Rafael J . Wysocki, Kevin Hilman, Andrejs Cainikovs, Biju Das, Bjorn Andersson, Catalin Marinas, Dmitry Baryshkov, Fabio Estevam, Geert Uytterhoeven, Marcel Ziswiler, NXP Linux Team, Pengutronix Kernel Team, Rob Herring, Sascha Hauer, Shawn Guo, Vinod Koul, Will Deacon, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Linux ARM, Linux Kernel Mailing List 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls 2022-07-28 11:21 ` Francesco Dolcini @ 2022-08-26 13:50 ` Ulf Hansson 2022-09-09 14:22 ` Francesco Dolcini 0 siblings, 1 reply; 39+ messages in thread From: Ulf Hansson @ 2022-08-26 13:50 UTC (permalink / raw) To: Francesco Dolcini Cc: Mark Brown, Krzysztof Kozlowski, Robin Murphy, Max Krummenacher, Linus Walleij, Max Krummenacher, Linux PM list, Rafael J . Wysocki, Kevin Hilman, Andrejs Cainikovs, Biju Das, Bjorn Andersson, Catalin Marinas, Dmitry Baryshkov, Fabio Estevam, Geert Uytterhoeven, Marcel Ziswiler, NXP Linux Team, Pengutronix Kernel Team, Rob Herring, Sascha Hauer, Shawn Guo, Vinod Koul, Will Deacon, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Linux ARM, Linux Kernel Mailing List 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls 2022-08-26 13:50 ` Ulf Hansson @ 2022-09-09 14:22 ` Francesco Dolcini 2022-09-22 13:49 ` Ulf Hansson 0 siblings, 1 reply; 39+ messages in thread From: Francesco Dolcini @ 2022-09-09 14:22 UTC (permalink / raw) To: Ulf Hansson Cc: Francesco Dolcini, Mark Brown, Krzysztof Kozlowski, Robin Murphy, Max Krummenacher, Linus Walleij, Max Krummenacher, Linux PM list, Rafael J . Wysocki, Kevin Hilman, Andrejs Cainikovs, Biju Das, Bjorn Andersson, Catalin Marinas, Dmitry Baryshkov, Fabio Estevam, Geert Uytterhoeven, Marcel Ziswiler, NXP Linux Team, Pengutronix Kernel Team, Rob Herring, Sascha Hauer, Shawn Guo, Vinod Koul, Will Deacon, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Linux ARM, Linux Kernel Mailing List 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/ ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls 2022-09-09 14:22 ` Francesco Dolcini @ 2022-09-22 13:49 ` Ulf Hansson 2022-09-23 18:00 ` Krzysztof Kozlowski 0 siblings, 1 reply; 39+ messages in thread From: Ulf Hansson @ 2022-09-22 13:49 UTC (permalink / raw) To: Francesco Dolcini Cc: Mark Brown, Krzysztof Kozlowski, Robin Murphy, Max Krummenacher, Linus Walleij, Max Krummenacher, Linux PM list, Rafael J . Wysocki, Kevin Hilman, Andrejs Cainikovs, Biju Das, Bjorn Andersson, Catalin Marinas, Dmitry Baryshkov, Fabio Estevam, Geert Uytterhoeven, Marcel Ziswiler, NXP Linux Team, Pengutronix Kernel Team, Rob Herring, Sascha Hauer, Shawn Guo, Vinod Koul, Will Deacon, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Linux ARM, Linux Kernel Mailing List 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls 2022-09-22 13:49 ` Ulf Hansson @ 2022-09-23 18:00 ` Krzysztof Kozlowski 2022-09-26 10:12 ` Ulf Hansson 0 siblings, 1 reply; 39+ messages in thread From: Krzysztof Kozlowski @ 2022-09-23 18:00 UTC (permalink / raw) To: Ulf Hansson, Francesco Dolcini Cc: Mark Brown, Krzysztof Kozlowski, Robin Murphy, Max Krummenacher, Linus Walleij, Max Krummenacher, Linux PM list, Rafael J . Wysocki, Kevin Hilman, Andrejs Cainikovs, Biju Das, Bjorn Andersson, Catalin Marinas, Dmitry Baryshkov, Fabio Estevam, Geert Uytterhoeven, Marcel Ziswiler, NXP Linux Team, Pengutronix Kernel Team, Rob Herring, Sascha Hauer, Shawn Guo, Vinod Koul, Will Deacon, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Linux ARM, Linux Kernel Mailing List 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls 2022-09-23 18:00 ` Krzysztof Kozlowski @ 2022-09-26 10:12 ` Ulf Hansson 2022-09-26 15:11 ` Krzysztof Kozlowski 0 siblings, 1 reply; 39+ messages in thread From: Ulf Hansson @ 2022-09-26 10:12 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Francesco Dolcini, Mark Brown, Krzysztof Kozlowski, Robin Murphy, Max Krummenacher, Linus Walleij, Max Krummenacher, Linux PM list, Rafael J . Wysocki, Kevin Hilman, Andrejs Cainikovs, Biju Das, Bjorn Andersson, Catalin Marinas, Dmitry Baryshkov, Fabio Estevam, Geert Uytterhoeven, Marcel Ziswiler, NXP Linux Team, Pengutronix Kernel Team, Rob Herring, Sascha Hauer, Shawn Guo, Vinod Koul, Will Deacon, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Linux ARM, Linux Kernel Mailing List 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls 2022-09-26 10:12 ` Ulf Hansson @ 2022-09-26 15:11 ` Krzysztof Kozlowski 2022-09-27 9:48 ` Ulf Hansson 0 siblings, 1 reply; 39+ messages in thread From: Krzysztof Kozlowski @ 2022-09-26 15:11 UTC (permalink / raw) To: Ulf Hansson Cc: Francesco Dolcini, Mark Brown, Krzysztof Kozlowski, Robin Murphy, Max Krummenacher, Linus Walleij, Max Krummenacher, Linux PM list, Rafael J . Wysocki, Kevin Hilman, Andrejs Cainikovs, Biju Das, Bjorn Andersson, Catalin Marinas, Dmitry Baryshkov, Fabio Estevam, Geert Uytterhoeven, Marcel Ziswiler, NXP Linux Team, Pengutronix Kernel Team, Rob Herring, Sascha Hauer, Shawn Guo, Vinod Koul, Will Deacon, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Linux ARM, Linux Kernel Mailing List 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls 2022-09-26 15:11 ` Krzysztof Kozlowski @ 2022-09-27 9:48 ` Ulf Hansson 2022-09-27 12:49 ` Geert Uytterhoeven 0 siblings, 1 reply; 39+ messages in thread From: Ulf Hansson @ 2022-09-27 9:48 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Francesco Dolcini, Mark Brown, Krzysztof Kozlowski, Robin Murphy, Max Krummenacher, Linus Walleij, Max Krummenacher, Linux PM list, Rafael J . Wysocki, Kevin Hilman, Andrejs Cainikovs, Biju Das, Bjorn Andersson, Catalin Marinas, Dmitry Baryshkov, Fabio Estevam, Geert Uytterhoeven, Marcel Ziswiler, NXP Linux Team, Pengutronix Kernel Team, Rob Herring, Sascha Hauer, Shawn Guo, Vinod Koul, Will Deacon, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Linux ARM, Linux Kernel Mailing List [...] > >>>> > >>>> 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls 2022-09-27 9:48 ` Ulf Hansson @ 2022-09-27 12:49 ` Geert Uytterhoeven 2022-09-27 14:26 ` Krzysztof Kozlowski 0 siblings, 1 reply; 39+ messages in thread From: Geert Uytterhoeven @ 2022-09-27 12:49 UTC (permalink / raw) To: Ulf Hansson Cc: Krzysztof Kozlowski, Francesco Dolcini, Mark Brown, Krzysztof Kozlowski, Robin Murphy, Max Krummenacher, Linus Walleij, Max Krummenacher, Linux PM list, Rafael J . Wysocki, Kevin Hilman, Andrejs Cainikovs, Biju Das, Bjorn Andersson, Catalin Marinas, Dmitry Baryshkov, Fabio Estevam, Geert Uytterhoeven, Marcel Ziswiler, NXP Linux Team, Pengutronix Kernel Team, Rob Herring, Sascha Hauer, Shawn Guo, Vinod Koul, Will Deacon, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Linux ARM, Linux Kernel Mailing List 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls 2022-09-27 12:49 ` Geert Uytterhoeven @ 2022-09-27 14:26 ` Krzysztof Kozlowski 2022-09-28 7:59 ` Ulf Hansson 0 siblings, 1 reply; 39+ messages in thread From: Krzysztof Kozlowski @ 2022-09-27 14:26 UTC (permalink / raw) To: Geert Uytterhoeven, Ulf Hansson Cc: Francesco Dolcini, Mark Brown, Krzysztof Kozlowski, Robin Murphy, Max Krummenacher, Linus Walleij, Max Krummenacher, Linux PM list, Rafael J . Wysocki, Kevin Hilman, Andrejs Cainikovs, Biju Das, Bjorn Andersson, Catalin Marinas, Dmitry Baryshkov, Fabio Estevam, Geert Uytterhoeven, Marcel Ziswiler, NXP Linux Team, Pengutronix Kernel Team, Rob Herring, Sascha Hauer, Shawn Guo, Vinod Koul, Will Deacon, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Linux ARM, Linux Kernel Mailing List 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls 2022-09-27 14:26 ` Krzysztof Kozlowski @ 2022-09-28 7:59 ` Ulf Hansson 0 siblings, 0 replies; 39+ messages in thread From: Ulf Hansson @ 2022-09-28 7:59 UTC (permalink / raw) To: Krzysztof Kozlowski, Geert Uytterhoeven Cc: Francesco Dolcini, Mark Brown, Krzysztof Kozlowski, Robin Murphy, Max Krummenacher, Linus Walleij, Max Krummenacher, Linux PM list, Rafael J . Wysocki, Kevin Hilman, Andrejs Cainikovs, Biju Das, Bjorn Andersson, Catalin Marinas, Dmitry Baryshkov, Fabio Estevam, Geert Uytterhoeven, Marcel Ziswiler, NXP Linux Team, Pengutronix Kernel Team, Rob Herring, Sascha Hauer, Shawn Guo, Vinod Koul, Will Deacon, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Linux ARM, Linux Kernel Mailing List 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
end of thread, other threads:[~2022-09-28 8:00 UTC | newest] Thread overview: 39+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-06-09 15:08 [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls Max Krummenacher 2022-06-09 15:08 ` [PATCH v1 1/5] dt-bindings: power: Add bindings for a power domain controlled by a regulator Max Krummenacher 2022-06-10 2:57 ` Rob Herring 2022-06-15 15:19 ` Max Krummenacher 2022-06-15 17:16 ` Krzysztof Kozlowski 2022-06-14 7:23 ` Geert Uytterhoeven 2022-06-15 15:18 ` Max Krummenacher 2022-06-09 15:08 ` [RFC PATCH v1 5/5] ARM64: verdin-imx8mm: use regulator power domain to model sleep-moci Max Krummenacher 2022-06-14 7:29 ` Geert Uytterhoeven 2022-06-15 15:32 ` Max Krummenacher 2022-06-13 19:15 ` [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls Rob Herring 2022-06-14 7:22 ` Geert Uytterhoeven 2022-06-15 16:10 ` Max Krummenacher 2022-06-15 17:15 ` Krzysztof Kozlowski 2022-06-15 17:31 ` Marcel Ziswiler 2022-06-15 17:37 ` Krzysztof Kozlowski 2022-06-15 18:13 ` Marcel Ziswiler 2022-06-15 18:48 ` Dmitry Baryshkov 2022-06-15 20:48 ` Krzysztof Kozlowski 2022-06-15 18:24 ` Robin Murphy 2022-06-15 19:12 ` Mark Brown 2022-06-15 21:14 ` Ulf Hansson 2022-06-16 12:50 ` Linus Walleij 2022-06-23 16:14 ` Max Krummenacher 2022-07-13 11:43 ` Ulf Hansson 2022-07-18 9:49 ` Linus Walleij 2022-07-26 16:03 ` Francesco Dolcini 2022-07-28 9:37 ` Ulf Hansson 2022-07-28 11:21 ` Francesco Dolcini 2022-08-26 13:50 ` Ulf Hansson 2022-09-09 14:22 ` Francesco Dolcini 2022-09-22 13:49 ` Ulf Hansson 2022-09-23 18:00 ` Krzysztof Kozlowski 2022-09-26 10:12 ` Ulf Hansson 2022-09-26 15:11 ` Krzysztof Kozlowski 2022-09-27 9:48 ` Ulf Hansson 2022-09-27 12:49 ` Geert Uytterhoeven 2022-09-27 14:26 ` Krzysztof Kozlowski 2022-09-28 7:59 ` Ulf Hansson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).