Hi On Tue, Dec 08, 2020 at 01:52:14PM +0100, Michael Klein wrote: > Thanks for reviewing! > > On Tue, Dec 08, 2020 at 11:13:58AM +0100, Maxime Ripard wrote: > > On Mon, Dec 07, 2020 at 03:27:55PM +0100, Michael Klein wrote: > > > Add devicetree binding documentation for regulator-poweroff driver. > > > > > > Signed-off-by: Michael Klein <michael@xxxxxxxxxxxx> > > > --- > > > .../power/reset/regulator-poweroff.yaml | 53 +++++++++++++++++++ > > > 1 file changed, 53 insertions(+) > > > create mode 100644 Documentation/devicetree/bindings/power/reset/regulator-poweroff.yaml > > > > > > diff --git a/Documentation/devicetree/bindings/power/reset/regulator-poweroff.yaml b/Documentation/devicetree/bindings/power/reset/regulator-poweroff.yaml > > > new file mode 100644 > > > index 000000000000..8c8ce6bb031a > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/power/reset/regulator-poweroff.yaml > > > @@ -0,0 +1,53 @@ > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > +%YAML 1.2 > > > +--- > > > +$id: http://devicetree.org/schemas/power/reset/regulator-poweroff.yaml# > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > + > > > +title: Force-disable power regulators to turn the power off. > > > + > > > +maintainers: > > > + - Michael Klein <michael@xxxxxxxxxxxx> > > > + > > > +description: | > > > + When the power-off handler is called, one more regulators are disabled > > > + by calling regulator_force_disable(). If the power is still on and the > > > + CPU still running after a 3000ms delay, a WARN_ON(1) is emitted. > > > + > > > +properties: > > > + compatible: > > > + const: "regulator-poweroff" > > > + > > > + regulator-names: > > > + description: > > > + Array of regulator names > > > + $ref: /schemas/types.yaml#/definitions/string-array > > > + > > > + REGULATOR-supply: > > > > This should be a patternProperties > > > > > + description: > > > + For any REGULATOR listed in regulator-names, a phandle > > > + to the corresponding regulator node > > > + $ref: /schemas/types.yaml#/definitions/phandle > > > + > > > + timeout-ms: > > > + description: > > > + Time to wait before asserting a WARN_ON(1). If nothing is > > > + specified, 3000 ms is used. > > > + $ref: /schemas/types.yaml#/definitions/uint32 > > > + > > > +required: > > > + - compatible > > > + - regulator-names > > > + - REGULATOR-supply > > > + > > > +additionalProperties: false > > > + > > > +examples: > > > + - | > > > + regulator-poweroff { > > > + compatible = "regulator-poweroff"; > > > + regulator-names = "vcc1v2", "vcc-dram"; > > > + vcc1v2-supply = <®_vcc1v2>; > > > + vcc-dram-supply = <®_vcc_dram>; > > > + }; > > > > I'm not entirely sure how multiple regulators would work here. I guess > > the ordering is board/purpose sensitive. In this particular case, I > > assume that vcc1v2 would be shut down before vcc-dram? > > yes, the regulators are shut down from left to right. > > > If so, I would expect that one regulator_force_disable is run, the CPU > > is disabled and you never get the chance to cut vcc-dram. > > I assume that any relevant regulator here has enough capacitance on the > output that provides enough charge to disable any remaining regulators (my > board has 3*10µF for vcc1v2 and 1*10µF for vcc-dram). But there is of course > no guarantee, so I'm shutting down the most relevant (in terms of current > consumption) regulator first. > > In any case, if it's deemed unnecessary to allow more than one regulator in > the driver I could remove the regulator-names property altogether and reduce > the DT node to: > > regulator-poweroff { > compatible = "regulator-poweroff"; > poweroff-supply = <®_vcc1v2>; > }; It's mostly about semantics at this point but there's value in shutting down the RAM as well if we're taking some precautions I mentionned. Maybe we can name that regulator cpu-supply, so that we can easily add the DRAM one if needed, and the semantics are clear? Maxime
Attachment:
signature.asc
Description: PGP signature
