On Sun, Oct 30, 2022 at 11:37:15AM +0000, Paul Cercueil wrote: > Create YAML bindings for the Active-semi PMICs and remove the old text > files. > > The bindings aren't perfect, for instance I couldn't find good > descriptions for the vendor properties in the "charger" node of the > ACT8945A because I am not familiar with the hardware and these > properties were not documented anywhere. > > The YAML schemas are a bit different than what is described in the old > text files, because these were sometimes wrong or had missing > information. This is the case for the ACT8600 documentation, which > specified the valid node names for the regulators, while the driver was > expecting different names. This led to the current situation where we > have two different boards using different names for the regulators: > - arch/mips/boot/dts/ingenic/ci20.dts uses the names documented in the > text file, > - arch/mips/boot/dts/ingenic/gcw0.dts uses the names that the driver > expects. > In theory, the driver should be fixed to follow the documentation, and > accept both naming schemes. In practice though, when the PMIC node was > added to the ci20.dts board file, the names were already wrong in > regards to what the driver expected, so it never really worked > correctly and wasn't tested properly. Furthermore, in that board the > consumers of the regulators aren't working for various other reasons > (invalid GPIOs, etc.). > > For that reason, for the ACT8600 bindings I decided to only use the node > names that the driver expects (and that gcw0.dts uses), instead of > accepting both old and new names. A follow-up patch will update the CI20 > board to use the new regulator names. > > Signed-off-by: Paul Cercueil <paul@xxxxxxxxxxxxxxx> > --- > > Note: > I set Liam Girdwood and Mark Brown as the maintainers by default, since > it doesn't appear that anybody is managing the Active-semi drivers, but > if anybody steps up I can update it. > > Cheers, > -Paul > > .../bindings/regulator/act8865-regulator.txt | 117 -------- > .../bindings/regulator/act8945a-regulator.txt | 113 -------- > .../regulator/active-semi,act8600.yaml | 143 ++++++++++ > .../regulator/active-semi,act8846.yaml | 209 ++++++++++++++ > .../regulator/active-semi,act8865.yaml | 162 +++++++++++ > .../regulator/active-semi,act8945a.yaml | 263 ++++++++++++++++++ > 6 files changed, 777 insertions(+), 230 deletions(-) > delete mode 100644 Documentation/devicetree/bindings/regulator/act8865-regulator.txt > delete mode 100644 Documentation/devicetree/bindings/regulator/act8945a-regulator.txt > create mode 100644 Documentation/devicetree/bindings/regulator/active-semi,act8600.yaml > create mode 100644 Documentation/devicetree/bindings/regulator/active-semi,act8846.yaml > create mode 100644 Documentation/devicetree/bindings/regulator/active-semi,act8865.yaml > create mode 100644 Documentation/devicetree/bindings/regulator/active-semi,act8945a.yaml [...] > diff --git a/Documentation/devicetree/bindings/regulator/active-semi,act8600.yaml b/Documentation/devicetree/bindings/regulator/active-semi,act8600.yaml > new file mode 100644 > index 000000000000..bf8c5145939e > --- /dev/null > +++ b/Documentation/devicetree/bindings/regulator/active-semi,act8600.yaml > @@ -0,0 +1,143 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/regulator/active-semi,act8600.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Active-semi ACT8600 regulator > + > +maintainers: > + - Liam Girdwood <lgirdwood@xxxxxxxxx> > + - Mark Brown <broonie@xxxxxxxxxx> > + > +properties: > + compatible: > + const: active-semi,act8600 > + > + reg: > + description: I2C address > + maxItems: 1 > + > + system-power-controller: > + description: | Don't need '|'. > + Indicates that the ACT8600 is responsible for powering OFF > + the system. > + type: boolean > + > + active-semi,vsel-high: > + description: | > + Indicates the VSEL pin is high. If this property is missing, > + the VSEL pin is assumed to be low. > + type: boolean > + > + regulators: > + type: object > + unevaluatedProperties: false Don't need both this and additionalProperties. As additionalProperties is sufficient here, use it, but move it here. It's easier to read that way IMO. > + > + properties: > + DCDC1: > + type: object > + $ref: /schemas/regulator/regulator.yaml# > + unevaluatedProperties: false > + > + properties: > + vp1-supply: > + description: Handle to the VP1 input supply > + > + DCDC2: > + type: object > + $ref: /schemas/regulator/regulator.yaml# > + unevaluatedProperties: false > + > + properties: > + vp2-supply: > + description: Handle to the VP2 input supply > + > + DCDC3: > + type: object > + $ref: /schemas/regulator/regulator.yaml# > + unevaluatedProperties: false > + > + properties: > + vp3-supply: > + description: Handle to the VP3 input supply > + > + patternProperties: > + "^((SUDCDC_REG4)|(LDO_REG9)|(LDO_REG10))$": You can drop the inner parenthesis. > + type: object > + $ref: /schemas/regulator/regulator.yaml# > + unevaluatedProperties: false > + > + "^LDO[5-8]$": > + type: object > + $ref: /schemas/regulator/regulator.yaml# > + unevaluatedProperties: false > + > + properties: > + inl-supply: > + description: Handle to the INL input supply > + > + additionalProperties: false > + > +additionalProperties: false > + > +required: > + - reg > + - compatible > + - regulators > + > +examples: > + - | > + i2c3 { i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + > + act8600: pmic@5a { Drop unused labels. > + compatible = "active-semi,act8600"; > + reg = <0x5a>; > + > + regulators { > + SUDCDC_REG4 { > + regulator-min-microvolt = <5300000>; > + regulator-max-microvolt = <5300000>; > + inl-supply = <&vcc>; > + }; > + > + LDO5 { > + regulator-min-microvolt = <2500000>; > + regulator-max-microvolt = <2500000>; > + inl-supply = <&vcc>; > + }; > + > + LDO6 { > + regulator-min-microvolt = <3300000>; > + regulator-max-microvolt = <3300000>; > + inl-supply = <&vcc>; > + }; > + > + LDO7 { > + regulator-min-microvolt = <3300000>; > + regulator-max-microvolt = <3300000>; > + inl-supply = <&vcc>; > + }; > + > + LDO8 { > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <1800000>; > + regulator-always-on; > + inl-supply = <&vcc>; > + }; > + > + LDO_REG9 { > + regulator-min-microvolt = <3300000>; > + regulator-max-microvolt = <3300000>; > + regulator-always-on; > + inl-supply = <&vcc>; > + }; > + > + LDO_REG10 { > + inl-supply = <&vcc>; > + }; > + }; > + }; > + }; Similar comments on the rest. Rob
