From: Hector Martin <marcan@marcan.st> To: Rob Herring <robh+dt@kernel.org> Cc: linux-arm-kernel <linux-arm-kernel@lists.infradead.org>, Marc Zyngier <maz@kernel.org>, Arnd Bergmann <arnd@kernel.org>, Linus Walleij <linus.walleij@linaro.org>, Alyssa Rosenzweig <alyssa@rosenzweig.io>, Krzysztof Kozlowski <krzk@kernel.org>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Mark Kettenis <mark.kettenis@xs4all.nl>, Philipp Zabel <p.zabel@pengutronix.de>, "Rafael J. Wysocki" <rafael@kernel.org>, devicetree@vger.kernel.org, "open list:THERMAL" <linux-pm@vger.kernel.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, linux-samsung-soc <linux-samsung-soc@vger.kernel.org>, "open list:SERIAL DRIVERS" <linux-serial@vger.kernel.org> Subject: Re: [PATCH 2/7] dt-bindings: power: Add apple,pmgr-pwrstate binding Date: Thu, 7 Oct 2021 00:52:07 +0900 [thread overview] Message-ID: <f95f6d61-8809-e668-0458-453a8dfbe641@marcan.st> (raw) In-Reply-To: <CAL_JsqJenHAOw4gApzGpuj-8nZjkYhmBg0qBj-DV+CEJ7zXuVw@mail.gmail.com> On 06/10/2021 09.58, Rob Herring wrote: > On Tue, Oct 5, 2021 at 10:59 AM Hector Martin <marcan@marcan.st> wrote: >> Future SoCs are expected to use backwards compatible registers, and the >> "apple,pmgr-pwrstate" represents any such interfaces (possibly with >> additional features gated by the more specific compatible), allowing >> them to be bound without driver updates. If a backwards incompatible >> change is introduced in future SoCs, it will require a new compatible, >> such as "apple,pmgr-pwrstate-v2". > > Is that because past SoCs used the same registers? I don't see how > else you have any insight to what future SoCs will do. > > Normally we don't do 1 node per register type bindings, so I'm a bit > leery about doing 1 node per domain. Yes, we can trace a pretty clear lineage from past SoCs. Plus Apple folks themselves have confirmed that this is an explicit policy: https://twitter.com/stuntpants/status/1442276493669724160 Already within this SoC we have two PMGR instances with different register sets (one covers all devices that stay on during system sleep), although I am only instantiating one in our devicetree at the moment. And of course there is the A14, which is the same generation as the M1 and has exactly the same register format, but a different set of domains. Having sub-nodes describing individual power domains has some prior art (e.g. power/rockchip,power-controller.yaml). In that case the nodes are all managed by the parent node, and use the hierarchical format, but the hierarchical format cannot handle multi-parent nodes (which we do have, or at least Apple describes them that way). Since we really have no "top-level" implementation specifics to worry about, I think it makes sense to just bind to single nodes at this point, which makes the driver very simple since it doesn't have to perform any bookkeeping for multiple domains. I realize this is all kind of "not the way things are usually done", but I don't want to pass up on the opportunity to have one driver last us multiple SoCs if we have the chance, and it's looking like it should :-) Note that as new features are implemented (e.g. auto-PM, which I will add to this driver later), that also naturally lends itself to forwards-compat, as SoCs without those features at all simply wouldn't request them in the DT. In this case an "apple,auto-pm" flag would enable that for domains where we want it, and those that don't support it (or a hypothetical past SoC without the feature at all) would simply not use it. >> +properties: >> + $nodename: >> + pattern: "^power-controller@[0-9a-f]+$" > > Drop this and define this node in the syscon schema with a $ref to this schema. Ack, makes sense. >> + apple,domain-name: >> + description: | >> + Specifies the name of the SoC device being controlled. This is used to >> + name the power/reset domains. >> + $ref: /schemas/types.yaml#/definitions/string > > No other power domain binding needs this, why do you? Because they all hardcode the domain names in the drivers for every SoC :-) Without a name of some sort in the devicetree, all our genpds would have to use numeric register offsets or the like, which seems quite ugly. > I prefer 1 complete example in the MFD schema rather than piecemeal examples. Sure. Would we leave this schema without examples then? > As the child nodes are memory mapped devices, size should be 1. Then > address translation works (though Linux doesn't care (currently)). This requires all the reg properties to also declare the reg size, right? One thing I wonder is whether it would make sense to allow #power-domain-cells = <1> and then be able to declare consecutive ranges of related power registers in one node. This would be useful for e.g. the 9 UARTs, the 4 SPI controllers, the 6 MCAs, the 5 I2C controllers, the 5 PWM controllers, etc (which all have uniform parents and features and are consecutive, so could be described together). I'm not sure if it's worth it, thoughts? -- Hector Martin (marcan@marcan.st) Public Key: https://mrcn.st/pub
WARNING: multiple messages have this Message-ID (diff)
From: Hector Martin <marcan@marcan.st> To: Rob Herring <robh+dt@kernel.org> Cc: linux-arm-kernel <linux-arm-kernel@lists.infradead.org>, Marc Zyngier <maz@kernel.org>, Arnd Bergmann <arnd@kernel.org>, Linus Walleij <linus.walleij@linaro.org>, Alyssa Rosenzweig <alyssa@rosenzweig.io>, Krzysztof Kozlowski <krzk@kernel.org>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Mark Kettenis <mark.kettenis@xs4all.nl>, Philipp Zabel <p.zabel@pengutronix.de>, "Rafael J. Wysocki" <rafael@kernel.org>, devicetree@vger.kernel.org, "open list:THERMAL" <linux-pm@vger.kernel.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, linux-samsung-soc <linux-samsung-soc@vger.kernel.org>, "open list:SERIAL DRIVERS" <linux-serial@vger.kernel.org> Subject: Re: [PATCH 2/7] dt-bindings: power: Add apple,pmgr-pwrstate binding Date: Thu, 7 Oct 2021 00:52:07 +0900 [thread overview] Message-ID: <f95f6d61-8809-e668-0458-453a8dfbe641@marcan.st> (raw) In-Reply-To: <CAL_JsqJenHAOw4gApzGpuj-8nZjkYhmBg0qBj-DV+CEJ7zXuVw@mail.gmail.com> On 06/10/2021 09.58, Rob Herring wrote: > On Tue, Oct 5, 2021 at 10:59 AM Hector Martin <marcan@marcan.st> wrote: >> Future SoCs are expected to use backwards compatible registers, and the >> "apple,pmgr-pwrstate" represents any such interfaces (possibly with >> additional features gated by the more specific compatible), allowing >> them to be bound without driver updates. If a backwards incompatible >> change is introduced in future SoCs, it will require a new compatible, >> such as "apple,pmgr-pwrstate-v2". > > Is that because past SoCs used the same registers? I don't see how > else you have any insight to what future SoCs will do. > > Normally we don't do 1 node per register type bindings, so I'm a bit > leery about doing 1 node per domain. Yes, we can trace a pretty clear lineage from past SoCs. Plus Apple folks themselves have confirmed that this is an explicit policy: https://twitter.com/stuntpants/status/1442276493669724160 Already within this SoC we have two PMGR instances with different register sets (one covers all devices that stay on during system sleep), although I am only instantiating one in our devicetree at the moment. And of course there is the A14, which is the same generation as the M1 and has exactly the same register format, but a different set of domains. Having sub-nodes describing individual power domains has some prior art (e.g. power/rockchip,power-controller.yaml). In that case the nodes are all managed by the parent node, and use the hierarchical format, but the hierarchical format cannot handle multi-parent nodes (which we do have, or at least Apple describes them that way). Since we really have no "top-level" implementation specifics to worry about, I think it makes sense to just bind to single nodes at this point, which makes the driver very simple since it doesn't have to perform any bookkeeping for multiple domains. I realize this is all kind of "not the way things are usually done", but I don't want to pass up on the opportunity to have one driver last us multiple SoCs if we have the chance, and it's looking like it should :-) Note that as new features are implemented (e.g. auto-PM, which I will add to this driver later), that also naturally lends itself to forwards-compat, as SoCs without those features at all simply wouldn't request them in the DT. In this case an "apple,auto-pm" flag would enable that for domains where we want it, and those that don't support it (or a hypothetical past SoC without the feature at all) would simply not use it. >> +properties: >> + $nodename: >> + pattern: "^power-controller@[0-9a-f]+$" > > Drop this and define this node in the syscon schema with a $ref to this schema. Ack, makes sense. >> + apple,domain-name: >> + description: | >> + Specifies the name of the SoC device being controlled. This is used to >> + name the power/reset domains. >> + $ref: /schemas/types.yaml#/definitions/string > > No other power domain binding needs this, why do you? Because they all hardcode the domain names in the drivers for every SoC :-) Without a name of some sort in the devicetree, all our genpds would have to use numeric register offsets or the like, which seems quite ugly. > I prefer 1 complete example in the MFD schema rather than piecemeal examples. Sure. Would we leave this schema without examples then? > As the child nodes are memory mapped devices, size should be 1. Then > address translation works (though Linux doesn't care (currently)). This requires all the reg properties to also declare the reg size, right? One thing I wonder is whether it would make sense to allow #power-domain-cells = <1> and then be able to declare consecutive ranges of related power registers in one node. This would be useful for e.g. the 9 UARTs, the 4 SPI controllers, the 6 MCAs, the 5 I2C controllers, the 5 PWM controllers, etc (which all have uniform parents and features and are consecutive, so could be described together). I'm not sure if it's worth it, thoughts? -- Hector Martin (marcan@marcan.st) Public Key: https://mrcn.st/pub _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-10-06 15:52 UTC|newest] Thread overview: 90+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-10-05 15:59 [PATCH 0/7] Apple SoC PMGR device power states driver Hector Martin 2021-10-05 15:59 ` Hector Martin 2021-10-05 15:59 ` [PATCH 1/7] dt-bindings: arm: apple: Add apple,pmgr binding Hector Martin 2021-10-05 15:59 ` Hector Martin 2021-10-05 20:09 ` Mark Kettenis 2021-10-05 20:09 ` Mark Kettenis 2021-10-05 22:45 ` Rob Herring 2021-10-05 22:45 ` Rob Herring 2021-10-06 15:17 ` Hector Martin 2021-10-06 15:17 ` Hector Martin 2021-10-06 6:56 ` Krzysztof Kozlowski 2021-10-06 6:56 ` Krzysztof Kozlowski 2021-10-06 7:30 ` Krzysztof Kozlowski 2021-10-06 7:30 ` Krzysztof Kozlowski 2021-10-06 15:21 ` Hector Martin 2021-10-06 15:21 ` Hector Martin 2021-10-06 15:26 ` Hector Martin 2021-10-06 15:26 ` Hector Martin 2021-10-07 13:10 ` Krzysztof Kozlowski 2021-10-07 13:10 ` Krzysztof Kozlowski 2021-10-05 15:59 ` [PATCH 2/7] dt-bindings: power: Add apple,pmgr-pwrstate binding Hector Martin 2021-10-05 15:59 ` Hector Martin 2021-10-05 20:16 ` Mark Kettenis 2021-10-05 20:16 ` Mark Kettenis 2021-10-06 15:27 ` Hector Martin 2021-10-06 15:27 ` Hector Martin 2021-10-06 0:58 ` Rob Herring 2021-10-06 0:58 ` Rob Herring 2021-10-06 15:52 ` Hector Martin [this message] 2021-10-06 15:52 ` Hector Martin 2021-10-06 15:55 ` Hector Martin 2021-10-06 15:55 ` Hector Martin 2021-10-08 7:50 ` Krzysztof Kozlowski 2021-10-08 7:50 ` Krzysztof Kozlowski 2021-10-11 5:17 ` Hector Martin 2021-10-11 5:17 ` Hector Martin 2021-10-06 7:05 ` Krzysztof Kozlowski 2021-10-06 7:05 ` Krzysztof Kozlowski 2021-10-06 15:59 ` Hector Martin 2021-10-06 15:59 ` Hector Martin 2021-10-07 13:12 ` Krzysztof Kozlowski 2021-10-07 13:12 ` Krzysztof Kozlowski 2021-10-11 4:42 ` Hector Martin 2021-10-11 4:42 ` Hector Martin 2021-10-05 15:59 ` [PATCH 3/7] soc: apple: Add driver for Apple PMGR power state controls Hector Martin 2021-10-05 15:59 ` Hector Martin 2021-10-05 16:08 ` Linus Walleij 2021-10-05 16:08 ` Linus Walleij 2021-10-05 16:15 ` Hector Martin 2021-10-05 16:15 ` Hector Martin 2021-10-05 19:49 ` Linus Walleij 2021-10-05 19:49 ` Linus Walleij 2021-10-05 20:21 ` Mark Kettenis 2021-10-05 20:21 ` Mark Kettenis 2021-10-06 16:00 ` Hector Martin 2021-10-06 16:00 ` Hector Martin 2021-10-06 7:28 ` Krzysztof Kozlowski 2021-10-06 7:28 ` Krzysztof Kozlowski 2021-10-06 16:08 ` Hector Martin 2021-10-06 16:08 ` Hector Martin 2021-10-06 9:24 ` Philipp Zabel 2021-10-06 9:24 ` Philipp Zabel 2021-10-06 16:11 ` Hector Martin 2021-10-06 16:11 ` Hector Martin 2021-10-05 15:59 ` [PATCH 4/7] arm64: dts: apple: t8103: Rename clk24 to clkref Hector Martin 2021-10-05 15:59 ` Hector Martin 2021-10-05 20:22 ` Mark Kettenis 2021-10-05 20:22 ` Mark Kettenis 2021-10-05 15:59 ` [PATCH 5/7] arm64: dts: apple: t8103: Add the UART PMGR tree Hector Martin 2021-10-05 15:59 ` Hector Martin 2021-10-05 20:25 ` Mark Kettenis 2021-10-05 20:25 ` Mark Kettenis 2021-10-05 15:59 ` [PATCH 6/7] tty: serial: samsung_tty: Support runtime PM Hector Martin 2021-10-05 15:59 ` Hector Martin 2021-10-06 7:43 ` Krzysztof Kozlowski 2021-10-06 7:43 ` Krzysztof Kozlowski 2021-10-06 13:25 ` Rafael J. Wysocki 2021-10-06 13:25 ` Rafael J. Wysocki 2021-10-06 13:29 ` Krzysztof Kozlowski 2021-10-06 13:29 ` Krzysztof Kozlowski 2021-10-11 5:32 ` Hector Martin 2021-10-11 5:32 ` Hector Martin 2021-10-11 6:48 ` Krzysztof Kozlowski 2021-10-11 6:48 ` Krzysztof Kozlowski 2021-10-11 8:27 ` Johan Hovold 2021-10-11 8:27 ` Johan Hovold 2021-10-05 15:59 ` [PATCH 7/7] arm64: dts: apple: t8103: Add UART2 Hector Martin 2021-10-05 15:59 ` Hector Martin 2021-10-05 20:26 ` Mark Kettenis 2021-10-05 20:26 ` Mark Kettenis
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=f95f6d61-8809-e668-0458-453a8dfbe641@marcan.st \ --to=marcan@marcan.st \ --cc=alyssa@rosenzweig.io \ --cc=arnd@kernel.org \ --cc=devicetree@vger.kernel.org \ --cc=gregkh@linuxfoundation.org \ --cc=krzk@kernel.org \ --cc=linus.walleij@linaro.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pm@vger.kernel.org \ --cc=linux-samsung-soc@vger.kernel.org \ --cc=linux-serial@vger.kernel.org \ --cc=mark.kettenis@xs4all.nl \ --cc=maz@kernel.org \ --cc=p.zabel@pengutronix.de \ --cc=rafael@kernel.org \ --cc=robh+dt@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.