| Message ID | 20211005155923.173399-2-marcan@marcan.st (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Apple SoC PMGR device power states driver | expand |
> From: Hector Martin <marcan@marcan.st> > Date: Wed, 6 Oct 2021 00:59:17 +0900 > > The PMGR block in Apple Silicon SoCs is responsible for SoC power > management. There are two PMGRs in T8103, with different register > layouts but compatible registers. In order to support this as well > as future SoC generations with backwards-compatible registers, we > declare these blocks as syscons and bind to individual registers > in child nodes. Each register controls one SoC device. > > The respective apple compatibles are defined in case device-specific > quirks are necessary in the future, but currently these nodes are > expected to be bound by the generic syscon driver. > > Signed-off-by: Hector Martin <marcan@marcan.st> > --- > .../bindings/arm/apple/apple,pmgr.yaml | 74 +++++++++++++++++++ > MAINTAINERS | 1 + > 2 files changed, 75 insertions(+) > create mode 100644 Documentation/devicetree/bindings/arm/apple/apple,pmgr.yaml This works for U-Boot and OpenBSD. Reviewed-by: Mark Kettenis <kettenis@openbsd.org>. > diff --git a/Documentation/devicetree/bindings/arm/apple/apple,pmgr.yaml b/Documentation/devicetree/bindings/arm/apple/apple,pmgr.yaml > new file mode 100644 > index 000000000000..0304164e4140 > --- /dev/null > +++ b/Documentation/devicetree/bindings/arm/apple/apple,pmgr.yaml > @@ -0,0 +1,74 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/arm/apple/apple,pmgr.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Apple SoC Power Manager (PMGR) > + > +maintainers: > + - Hector Martin <marcan@marcan.st> > + > +description: | > + Apple SoCs include a PMGR block responsible for power management, > + which can control various clocks, resets, power states, and > + performance features. This node represents the PMGR as a syscon, > + with sub-nodes representing individual features. > + > + Apple SoCs may have a secondary "mini-PMGR"; it is represented > + separately in the device tree, but works the same way. > + > +select: > + properties: > + compatible: > + contains: > + enum: > + - apple,t8103-pmgr > + - apple,t8103-minipmgr > + - apple,pmgr > + > + required: > + - compatible > + > +properties: > + $nodename: > + pattern: "^power-management@[0-9a-f]+$" > + > + compatible: > + items: > + - enum: > + - apple,t8103-pmgr > + - apple,t8103-minipmgr > + - const: apple,pmgr > + - const: syscon > + - const: simple-mfd > + > + reg: > + maxItems: 1 > + > +required: > + - compatible > + - reg > + > +additionalProperties: true > + > +examples: > + - | > + soc { > + #address-cells = <2>; > + #size-cells = <2>; > + > + power-management@23b700000 { > + compatible = "apple,t8103-pmgr", "apple,pmgr", "syscon", "simple-mfd"; > + #address-cells = <1>; > + #size-cells = <0>; > + reg = <0x2 0x3b700000 0x0 0x14000>; > + }; > + > + power-management@23b700000 { > + compatible = "apple,t8103-minipmgr", "apple,pmgr", "syscon", "simple-mfd"; > + #address-cells = <1>; > + #size-cells = <0>; > + reg = <0x2 0x3d280000 0x0 0xc000>; > + }; > + }; > diff --git a/MAINTAINERS b/MAINTAINERS > index abdcbcfef73d..d25598842d15 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -1719,6 +1719,7 @@ B: https://github.com/AsahiLinux/linux/issues > C: irc://irc.oftc.net/asahi-dev > T: git https://github.com/AsahiLinux/linux.git > F: Documentation/devicetree/bindings/arm/apple.yaml > +F: Documentation/devicetree/bindings/arm/apple/* > F: Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml > F: Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml > F: arch/arm64/boot/dts/apple/ > -- > 2.33.0 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >
On Wed, 06 Oct 2021 00:59:17 +0900, Hector Martin wrote: > The PMGR block in Apple Silicon SoCs is responsible for SoC power > management. There are two PMGRs in T8103, with different register > layouts but compatible registers. In order to support this as well > as future SoC generations with backwards-compatible registers, we > declare these blocks as syscons and bind to individual registers > in child nodes. Each register controls one SoC device. > > The respective apple compatibles are defined in case device-specific > quirks are necessary in the future, but currently these nodes are > expected to be bound by the generic syscon driver. > > Signed-off-by: Hector Martin <marcan@marcan.st> > --- > .../bindings/arm/apple/apple,pmgr.yaml | 74 +++++++++++++++++++ > MAINTAINERS | 1 + > 2 files changed, 75 insertions(+) > create mode 100644 Documentation/devicetree/bindings/arm/apple/apple,pmgr.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: Documentation/devicetree/bindings/arm/apple/apple,pmgr.example.dts:30.40-35.15: ERROR (duplicate_node_names): /example-0/soc/power-management@23b700000: Duplicate node name ERROR: Input tree has errors, aborting (use -f to force output) make[1]: *** [scripts/Makefile.lib:385: Documentation/devicetree/bindings/arm/apple/apple,pmgr.example.dt.yaml] Error 2 make[1]: *** Waiting for unfinished jobs.... make: *** [Makefile:1441: dt_binding_check] Error 2 doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/patch/1536742 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.
On 05/10/2021 17:59, Hector Martin wrote: > The PMGR block in Apple Silicon SoCs is responsible for SoC power > management. There are two PMGRs in T8103, with different register > layouts but compatible registers. In order to support this as well > as future SoC generations with backwards-compatible registers, we > declare these blocks as syscons and bind to individual registers > in child nodes. Each register controls one SoC device. > > The respective apple compatibles are defined in case device-specific > quirks are necessary in the future, but currently these nodes are > expected to be bound by the generic syscon driver. > > Signed-off-by: Hector Martin <marcan@marcan.st> > --- > .../bindings/arm/apple/apple,pmgr.yaml | 74 +++++++++++++++++++ > MAINTAINERS | 1 + > 2 files changed, 75 insertions(+) > create mode 100644 Documentation/devicetree/bindings/arm/apple/apple,pmgr.yaml > > diff --git a/Documentation/devicetree/bindings/arm/apple/apple,pmgr.yaml b/Documentation/devicetree/bindings/arm/apple/apple,pmgr.yaml > new file mode 100644 > index 000000000000..0304164e4140 > --- /dev/null > +++ b/Documentation/devicetree/bindings/arm/apple/apple,pmgr.yaml > @@ -0,0 +1,74 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/arm/apple/apple,pmgr.yaml# Please don't store all Apple-related bindings in bindings/arm/apple, but instead group per device type like in most of other bindings. In this case - this looks like something close to power domain controller, so it should be in bindings/power/ > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Apple SoC Power Manager (PMGR) > + > +maintainers: > + - Hector Martin <marcan@marcan.st> > + > +description: | > + Apple SoCs include a PMGR block responsible for power management, > + which can control various clocks, resets, power states, and > + performance features. This node represents the PMGR as a syscon, > + with sub-nodes representing individual features. > + > + Apple SoCs may have a secondary "mini-PMGR"; it is represented > + separately in the device tree, but works the same way. > + > +select: > + properties: > + compatible: > + contains: > + enum: > + - apple,t8103-pmgr > + - apple,t8103-minipmgr > + - apple,pmgr > + > + required: > + - compatible > + > +properties: > + $nodename: > + pattern: "^power-management@[0-9a-f]+$" > + > + compatible: > + items: > + - enum: > + - apple,t8103-pmgr > + - apple,t8103-minipmgr > + - const: apple,pmgr > + - const: syscon > + - const: simple-mfd No power-domain-cells? Why? What exactly this device is going to do? Maybe I'll check the driver first.... :) > + > + reg: > + maxItems: 1 > + > +required: > + - compatible > + - reg > + > +additionalProperties: true additionalProperties: false > + > +examples: > + - | > + soc { > + #address-cells = <2>; > + #size-cells = <2>; > + > + power-management@23b700000 { > + compatible = "apple,t8103-pmgr", "apple,pmgr", "syscon", "simple-mfd"; > + #address-cells = <1>; > + #size-cells = <0>; > + reg = <0x2 0x3b700000 0x0 0x14000>; > + }; > + > + power-management@23b700000 { > + compatible = "apple,t8103-minipmgr", "apple,pmgr", "syscon", "simple-mfd"; > + #address-cells = <1>; > + #size-cells = <0>; > + reg = <0x2 0x3d280000 0x0 0xc000>; > + }; > + }; > diff --git a/MAINTAINERS b/MAINTAINERS > index abdcbcfef73d..d25598842d15 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -1719,6 +1719,7 @@ B: https://github.com/AsahiLinux/linux/issues > C: irc://irc.oftc.net/asahi-dev > T: git https://github.com/AsahiLinux/linux.git > F: Documentation/devicetree/bindings/arm/apple.yaml > +F: Documentation/devicetree/bindings/arm/apple/* > F: Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml > F: Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml > F: arch/arm64/boot/dts/apple/ > Best regards, Krzysztof
On 06/10/2021 08:56, Krzysztof Kozlowski wrote: > On 05/10/2021 17:59, Hector Martin wrote: >> The PMGR block in Apple Silicon SoCs is responsible for SoC power >> management. There are two PMGRs in T8103, with different register >> layouts but compatible registers. In order to support this as well >> as future SoC generations with backwards-compatible registers, we >> declare these blocks as syscons and bind to individual registers >> in child nodes. Each register controls one SoC device. >> >> The respective apple compatibles are defined in case device-specific >> quirks are necessary in the future, but currently these nodes are >> expected to be bound by the generic syscon driver. >> >> Signed-off-by: Hector Martin <marcan@marcan.st> >> --- >> .../bindings/arm/apple/apple,pmgr.yaml | 74 +++++++++++++++++++ >> MAINTAINERS | 1 + >> 2 files changed, 75 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/arm/apple/apple,pmgr.yaml >> >> diff --git a/Documentation/devicetree/bindings/arm/apple/apple,pmgr.yaml b/Documentation/devicetree/bindings/arm/apple/apple,pmgr.yaml >> new file mode 100644 >> index 000000000000..0304164e4140 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/arm/apple/apple,pmgr.yaml >> @@ -0,0 +1,74 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/arm/apple/apple,pmgr.yaml# > > Please don't store all Apple-related bindings in bindings/arm/apple, but > instead group per device type like in most of other bindings. In this > case - this looks like something close to power domain controller, so it > should be in bindings/power/ > >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Apple SoC Power Manager (PMGR) >> + >> +maintainers: >> + - Hector Martin <marcan@marcan.st> >> + >> +description: | >> + Apple SoCs include a PMGR block responsible for power management, >> + which can control various clocks, resets, power states, and >> + performance features. This node represents the PMGR as a syscon, >> + with sub-nodes representing individual features. >> + >> + Apple SoCs may have a secondary "mini-PMGR"; it is represented >> + separately in the device tree, but works the same way. >> + >> +select: >> + properties: >> + compatible: >> + contains: >> + enum: >> + - apple,t8103-pmgr >> + - apple,t8103-minipmgr >> + - apple,pmgr >> + >> + required: >> + - compatible >> + >> +properties: >> + $nodename: >> + pattern: "^power-management@[0-9a-f]+$" >> + >> + compatible: >> + items: >> + - enum: >> + - apple,t8103-pmgr >> + - apple,t8103-minipmgr >> + - const: apple,pmgr >> + - const: syscon >> + - const: simple-mfd > > No power-domain-cells? Why? What exactly this device is going to do? > Maybe I'll check the driver first.... :) > After looking at the code, there is no device for apple,t8103-pmgr/apple,pmgr. What is this binding about? Is there really a central (central as in "one device for SoC") block managing power which you want to model here? Best regards, Krzysztof
On 06/10/2021 07.45, Rob Herring wrote: > 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: > Documentation/devicetree/bindings/arm/apple/apple,pmgr.example.dts:30.40-35.15: ERROR (duplicate_node_names): /example-0/soc/power-management@23b700000: Duplicate node name > ERROR: Input tree has errors, aborting (use -f to force output) > make[1]: *** [scripts/Makefile.lib:385: Documentation/devicetree/bindings/arm/apple/apple,pmgr.example.dt.yaml] Error 2 > make[1]: *** Waiting for unfinished jobs.... > make: *** [Makefile:1441: dt_binding_check] Error 2 Argh, sorry about that. I ran the check before adding the mini-pmgr node to the example right before sending out the series, and of course I screwed it up. It'll be fixed and double checked for v2.
On 06/10/2021 16.30, Krzysztof Kozlowski wrote: > After looking at the code, there is no device for > apple,t8103-pmgr/apple,pmgr. What is this binding about? Is there really > a central (central as in "one device for SoC") block managing power > which you want to model here? The pwrstate driver binds to individual power control registers within the syscon node. The parent node is bound by the generic syscon driver, so there is no specific SoC driver for it, but I still want to include SoC-specific compatibles so we can have something to use for quirks if we run into trouble in the future. There are two PMGRs in the Apple M1, and thus there would be two syscon nodes, each containing one subnode per PM domain. The devicetree in this series currently only instantiates one of those, though.
On 06/10/2021 15.56, Krzysztof Kozlowski wrote: >> diff --git a/Documentation/devicetree/bindings/arm/apple/apple,pmgr.yaml b/Documentation/devicetree/bindings/arm/apple/apple,pmgr.yaml >> new file mode 100644 >> index 000000000000..0304164e4140 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/arm/apple/apple,pmgr.yaml >> @@ -0,0 +1,74 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/arm/apple/apple,pmgr.yaml# > > Please don't store all Apple-related bindings in bindings/arm/apple, but > instead group per device type like in most of other bindings. In this > case - this looks like something close to power domain controller, so it > should be in bindings/power/ This is a controller that, right now, is only used to instantiate device power management controls, but the controller itself is just a generic syscon device. Depending on the register range, it could conceivably encompass other register types (e.g. clock selects) within it, though I'm not sure I want to do that right now. Apple calls several of these different register sets as a whole a "PMGR". So I'm not sure if it really qualifies as "just" a power domain controller. If we want to restrict this to the power state portion of PMGR, then it might make sense to call it something more specific... See arm/rockchip/pmu.yaml for the setup this is modeled after. > No power-domain-cells? Why? What exactly this device is going to do? > Maybe I'll check the driver first.... :) It's a syscon, it does nothing on its own. All the work is done by the child nodes and the driver that binds to those. >> +additionalProperties: true > > additionalProperties: false Fixed for v2.
On 06/10/2021 17:26, Hector Martin wrote: > On 06/10/2021 15.56, Krzysztof Kozlowski wrote: >>> diff --git a/Documentation/devicetree/bindings/arm/apple/apple,pmgr.yaml b/Documentation/devicetree/bindings/arm/apple/apple,pmgr.yaml >>> new file mode 100644 >>> index 000000000000..0304164e4140 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/arm/apple/apple,pmgr.yaml >>> @@ -0,0 +1,74 @@ >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/arm/apple/apple,pmgr.yaml# >> >> Please don't store all Apple-related bindings in bindings/arm/apple, but >> instead group per device type like in most of other bindings. In this >> case - this looks like something close to power domain controller, so it >> should be in bindings/power/ > > This is a controller that, right now, is only used to instantiate device > power management controls, but the controller itself is just a generic > syscon device. Depending on the register range, it could conceivably > encompass other register types (e.g. clock selects) within it, though > I'm not sure I want to do that right now. Apple calls several of these > different register sets as a whole a "PMGR". So I'm not sure if it > really qualifies as "just" a power domain controller. If we want to > restrict this to the power state portion of PMGR, then it might make > sense to call it something more specific... > > See arm/rockchip/pmu.yaml for the setup this is modeled after. > Makes sense now and actually few other designs including Samsung Exynos have it as well. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/arm/apple/apple,pmgr.yaml b/Documentation/devicetree/bindings/arm/apple/apple,pmgr.yaml new file mode 100644 index 000000000000..0304164e4140 --- /dev/null +++ b/Documentation/devicetree/bindings/arm/apple/apple,pmgr.yaml @@ -0,0 +1,74 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/arm/apple/apple,pmgr.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Apple SoC Power Manager (PMGR) + +maintainers: + - Hector Martin <marcan@marcan.st> + +description: | + Apple SoCs include a PMGR block responsible for power management, + which can control various clocks, resets, power states, and + performance features. This node represents the PMGR as a syscon, + with sub-nodes representing individual features. + + Apple SoCs may have a secondary "mini-PMGR"; it is represented + separately in the device tree, but works the same way. + +select: + properties: + compatible: + contains: + enum: + - apple,t8103-pmgr + - apple,t8103-minipmgr + - apple,pmgr + + required: + - compatible + +properties: + $nodename: + pattern: "^power-management@[0-9a-f]+$" + + compatible: + items: + - enum: + - apple,t8103-pmgr + - apple,t8103-minipmgr + - const: apple,pmgr + - const: syscon + - const: simple-mfd + + reg: + maxItems: 1 + +required: + - compatible + - reg + +additionalProperties: true + +examples: + - | + soc { + #address-cells = <2>; + #size-cells = <2>; + + power-management@23b700000 { + compatible = "apple,t8103-pmgr", "apple,pmgr", "syscon", "simple-mfd"; + #address-cells = <1>; + #size-cells = <0>; + reg = <0x2 0x3b700000 0x0 0x14000>; + }; + + power-management@23b700000 { + compatible = "apple,t8103-minipmgr", "apple,pmgr", "syscon", "simple-mfd"; + #address-cells = <1>; + #size-cells = <0>; + reg = <0x2 0x3d280000 0x0 0xc000>; + }; + }; diff --git a/MAINTAINERS b/MAINTAINERS index abdcbcfef73d..d25598842d15 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1719,6 +1719,7 @@ B: https://github.com/AsahiLinux/linux/issues C: irc://irc.oftc.net/asahi-dev T: git https://github.com/AsahiLinux/linux.git F: Documentation/devicetree/bindings/arm/apple.yaml +F: Documentation/devicetree/bindings/arm/apple/* F: Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml F: Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml F: arch/arm64/boot/dts/apple/
The PMGR block in Apple Silicon SoCs is responsible for SoC power management. There are two PMGRs in T8103, with different register layouts but compatible registers. In order to support this as well as future SoC generations with backwards-compatible registers, we declare these blocks as syscons and bind to individual registers in child nodes. Each register controls one SoC device. The respective apple compatibles are defined in case device-specific quirks are necessary in the future, but currently these nodes are expected to be bound by the generic syscon driver. Signed-off-by: Hector Martin <marcan@marcan.st> --- .../bindings/arm/apple/apple,pmgr.yaml | 74 +++++++++++++++++++ MAINTAINERS | 1 + 2 files changed, 75 insertions(+) create mode 100644 Documentation/devicetree/bindings/arm/apple/apple,pmgr.yaml