| Message ID | 20231228100208.2932-4-karelb@gimli.ms.mff.cuni.cz |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | regulator: support for Marvell 88PM886 LDOs and bucks | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | fail | build log |
On 28/12/2023 10:39, Karel Balej wrote: > From: Karel Balej <balejk@matfyz.cz> > A nit, subject: drop second/last, redundant "documentation entry". The "dt-bindings" prefix is already stating that these are bindings. > The Marvell 88PM88X PMICs provide regulators among other things. > Document how to use them. You did not document them in the bindings. You just added example to make it complete. Where is the binding change? What's more, I have doubts that you tested it. > > Signed-off-by: Karel Balej <balejk@matfyz.cz> > --- > .../bindings/mfd/marvell,88pm88x.yaml | 17 +++++++++++ > .../regulator/marvell,88pm88x-regulator.yaml | 28 +++++++++++++++++++ > 2 files changed, 45 insertions(+) > create mode 100644 Documentation/devicetree/bindings/regulator/marvell,88pm88x-regulator.yaml > > diff --git a/Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml b/Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml > index 115b41c9f22c..e6944369fc5c 100644 > --- a/Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml > +++ b/Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml > @@ -54,6 +54,23 @@ examples: > onkey { > compatible = "marvell,88pm88x-onkey"; > }; > + > + regulators { > + ldo2: ldo2 { > + regulator-min-microvolt = <3100000>; > + regulator-max-microvolt = <3300000>; > + }; > + > + ldo15: ldo15 { > + regulator-min-microvolt = <3300000>; > + regulator-max-microvolt = <3300000>; > + }; > + > + buck2: buck2 { > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <1800000>; > + }; > + }; > }; > }; > ... > diff --git a/Documentation/devicetree/bindings/regulator/marvell,88pm88x-regulator.yaml b/Documentation/devicetree/bindings/regulator/marvell,88pm88x-regulator.yaml > new file mode 100644 > index 000000000000..c6ac17b113e7 > --- /dev/null > +++ b/Documentation/devicetree/bindings/regulator/marvell,88pm88x-regulator.yaml > @@ -0,0 +1,28 @@ > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/regulator/marvell,88pm88x-regulator.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Marvell 88PM88X PMICs regulators > + > +maintainers: > + - Karel Balej <balejk@matfyz.cz> > + > +description: | > + This module is part of the Marvell 88PM88X MFD device. For more details > + see Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml. > + > + The regulator controller is represented as a sub-node of the PMIC node > + in the device tree. > + > + The valid names for 88PM886 regulator nodes are ldo[1-9], ldo1[0-6], buck[1-5]. Don't repeat constraints in free form text. > + > +patternProperties: > + "^(ldo|buck)[0-9]+$": You need to fix the pattern to be narrow. buck0 and buck6 are not correct. > + type: object > + description: > + Properties for single regulator. > + $ref: regulator.yaml# Not many benefits of this being in its own schema. > + > +additionalProperties: false Best regards, Krzysztof
On 04/01/2024 10:25, Krzysztof Kozlowski wrote: > On 28/12/2023 10:39, Karel Balej wrote: >> From: Karel Balej <balejk@matfyz.cz> >> > > A nit, subject: drop second/last, redundant "documentation entry". The > "dt-bindings" prefix is already stating that these are bindings. > >> The Marvell 88PM88X PMICs provide regulators among other things. >> Document how to use them. > > > You did not document them in the bindings. You just added example to > make it complete. Where is the binding change? > > What's more, I have doubts that you tested it. I found your other patchset - now I am 100% sure you did not test it. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml b/Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml index 115b41c9f22c..e6944369fc5c 100644 --- a/Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml +++ b/Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml @@ -54,6 +54,23 @@ examples: onkey { compatible = "marvell,88pm88x-onkey"; }; + + regulators { + ldo2: ldo2 { + regulator-min-microvolt = <3100000>; + regulator-max-microvolt = <3300000>; + }; + + ldo15: ldo15 { + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + }; + + buck2: buck2 { + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + }; + }; }; }; ... diff --git a/Documentation/devicetree/bindings/regulator/marvell,88pm88x-regulator.yaml b/Documentation/devicetree/bindings/regulator/marvell,88pm88x-regulator.yaml new file mode 100644 index 000000000000..c6ac17b113e7 --- /dev/null +++ b/Documentation/devicetree/bindings/regulator/marvell,88pm88x-regulator.yaml @@ -0,0 +1,28 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/regulator/marvell,88pm88x-regulator.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Marvell 88PM88X PMICs regulators + +maintainers: + - Karel Balej <balejk@matfyz.cz> + +description: | + This module is part of the Marvell 88PM88X MFD device. For more details + see Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml. + + The regulator controller is represented as a sub-node of the PMIC node + in the device tree. + + The valid names for 88PM886 regulator nodes are ldo[1-9], ldo1[0-6], buck[1-5]. + +patternProperties: + "^(ldo|buck)[0-9]+$": + type: object + description: + Properties for single regulator. + $ref: regulator.yaml# + +additionalProperties: false