[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <cdcd4eda-a7a9-2aa2-1316-e7184ff30bf3@canonical.com>
Date: Mon, 4 Oct 2021 16:48:39 +0200
From: Krzysztof Kozlowski <krzysztof.kozlowski@...onical.com>
To: Rob Herring <robh@...nel.org>
Cc: Michael Turquette <mturquette@...libre.com>,
Stephen Boyd <sboyd@...nel.org>,
Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>,
Sylwester Nawrocki <s.nawrocki@...sung.com>,
Tomasz Figa <tomasz.figa@...il.com>,
Chanwoo Choi <cw00.choi@...sung.com>,
Lee Jones <lee.jones@...aro.org>,
Liam Girdwood <lgirdwood@...il.com>,
Mark Brown <broonie@...nel.org>, linux-clk@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-samsung-soc@...r.kernel.org
Subject: Re: [PATCH v2 06/10] regulator: dt-bindings: samsung,s5m8767: convert
to dtschema
On 04/10/2021 16:41, Rob Herring wrote:
> On Fri, Oct 01, 2021 at 11:41:02AM +0200, Krzysztof Kozlowski wrote:
>> Convert the regulators of Samsung S5M8767 PMIC to DT schema format.
>>
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@...onical.com>
>> ---
>> .../bindings/regulator/samsung,s5m8767.txt | 140 ------------------
>> .../bindings/regulator/samsung,s5m8767.yaml | 83 +++++++++++
>> MAINTAINERS | 2 +-
>> 3 files changed, 84 insertions(+), 141 deletions(-)
>> delete mode 100644 Documentation/devicetree/bindings/regulator/samsung,s5m8767.txt
>> create mode 100644 Documentation/devicetree/bindings/regulator/samsung,s5m8767.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/regulator/samsung,s5m8767.txt b/Documentation/devicetree/bindings/regulator/samsung,s5m8767.txt
>> deleted file mode 100644
>> index 6cd83d920155..000000000000
>> --- a/Documentation/devicetree/bindings/regulator/samsung,s5m8767.txt
>> +++ /dev/null
>> @@ -1,140 +0,0 @@
>> -Binding for Samsung S5M8767 regulator block
>> -===========================================
>> -
>> -This is a part of device tree bindings for S5M family multi-function devices.
>> -More information can be found in bindings/mfd/sec-core.txt file.
>> -
>> -The S5M8767 device provide buck and LDO regulators.
>> -
>> -To register these with regulator framework instantiate under main device node
>> -a sub-node named "regulators" with more sub-nodes for each regulator using the
>> -common regulator binding documented in:
>> - - Documentation/devicetree/bindings/regulator/regulator.txt
>> -
>> -
>> -Required properties of the main device node (the parent!):
>> - - s5m8767,pmic-buck-ds-gpios: GPIO specifiers for three host gpio's used
>> - for selecting GPIO DVS lines. It is one-to-one mapped to dvs gpio lines.
>> -
>> - [1] If either of the 's5m8767,pmic-buck[2/3/4]-uses-gpio-dvs' optional
>> - property is specified, then all the eight voltage values for the
>> - 's5m8767,pmic-buck[2/3/4]-dvs-voltage' should be specified.
>> -
>> -Optional properties of the main device node (the parent!):
>> - - s5m8767,pmic-buck2-dvs-voltage: A set of 8 voltage values in micro-volt (uV)
>> - units for buck2 when changing voltage using gpio dvs. Refer to [1] below
>> - for additional information.
>> -
>> - - s5m8767,pmic-buck3-dvs-voltage: A set of 8 voltage values in micro-volt (uV)
>> - units for buck3 when changing voltage using gpio dvs. Refer to [1] below
>> - for additional information.
>> -
>> - - s5m8767,pmic-buck4-dvs-voltage: A set of 8 voltage values in micro-volt (uV)
>> - units for buck4 when changing voltage using gpio dvs. Refer to [1] below
>> - for additional information.
>> -
>> - - s5m8767,pmic-buck2-uses-gpio-dvs: 'buck2' can be controlled by gpio dvs.
>> - - s5m8767,pmic-buck3-uses-gpio-dvs: 'buck3' can be controlled by gpio dvs.
>> - - s5m8767,pmic-buck4-uses-gpio-dvs: 'buck4' can be controlled by gpio dvs.
>> -
>> -Additional properties required if either of the optional properties are used:
>> -
>> - - s5m8767,pmic-buck-default-dvs-idx: Default voltage setting selected from
>> - the possible 8 options selectable by the dvs gpios. The value of this
>> - property should be between 0 and 7. If not specified or if out of range, the
>> - default value of this property is set to 0.
>> -
>> - - s5m8767,pmic-buck-dvs-gpios: GPIO specifiers for three host gpio's used
>> - for dvs. The format of the gpio specifier depends in the gpio controller.
>> -
>> -
>> -Names of regulators supported by S5M8767 device:
>> - - LDOn
>> - - valid values for n are 1 to 28
>> - - Example: LDO1, LDO2, LDO28
>> - - BUCKn
>> - - valid values for n are 1 to 9.
>> - - Example: BUCK1, BUCK2, BUCK9
>> -Note: The 'n' in LDOn and BUCKn represents the LDO or BUCK number
>> -as per the datasheet of device.
>> -
>> -
>> -Optional properties of the nodes under "regulators" sub-node:
>> - - op_mode: describes the different operating modes of the LDO's with
>> - power mode change in SOC. The different possible values are,
>> - 0 - always off mode
>> - 1 - on in normal mode
>> - 2 - low power mode
>> - 3 - suspend mode
>> - - s5m8767,pmic-ext-control-gpios: (optional) GPIO specifier for one
>> - GPIO controlling this regulator
>> - (enable/disable); This is valid only
>> - for buck9.
>> -
>> -Example:
>> -
>> - s5m8767_pmic@66 {
>> - compatible = "samsung,s5m8767-pmic";
>> - reg = <0x66>;
>> -
>> - s5m8767,pmic-buck2-uses-gpio-dvs;
>> - s5m8767,pmic-buck3-uses-gpio-dvs;
>> - s5m8767,pmic-buck4-uses-gpio-dvs;
>> -
>> - s5m8767,pmic-buck-default-dvs-idx = <0>;
>> -
>> - s5m8767,pmic-buck-dvs-gpios = <&gpx0 0 0>, /* DVS1 */
>> - <&gpx0 1 0>, /* DVS2 */
>> - <&gpx0 2 0>; /* DVS3 */
>> -
>> - s5m8767,pmic-buck-ds-gpios = <&gpx2 3 0>, /* SET1 */
>> - <&gpx2 4 0>, /* SET2 */
>> - <&gpx2 5 0>; /* SET3 */
>> -
>> - s5m8767,pmic-buck2-dvs-voltage = <1350000>, <1300000>,
>> - <1250000>, <1200000>,
>> - <1150000>, <1100000>,
>> - <1000000>, <950000>;
>> -
>> - s5m8767,pmic-buck3-dvs-voltage = <1100000>, <1100000>,
>> - <1100000>, <1100000>,
>> - <1000000>, <1000000>,
>> - <1000000>, <1000000>;
>> -
>> - s5m8767,pmic-buck4-dvs-voltage = <1200000>, <1200000>,
>> - <1200000>, <1200000>,
>> - <1200000>, <1200000>,
>> - <1200000>, <1200000>;
>> -
>> - regulators {
>> - ldo1_reg: LDO1 {
>> - regulator-name = "VDD_ABB_3.3V";
>> - regulator-min-microvolt = <3300000>;
>> - regulator-max-microvolt = <3300000>;
>> - op_mode = <1>; /* Normal Mode */
>> - };
>> -
>> - ldo2_reg: LDO2 {
>> - regulator-name = "VDD_ALIVE_1.1V";
>> - regulator-min-microvolt = <1100000>;
>> - regulator-max-microvolt = <1100000>;
>> - regulator-always-on;
>> - };
>> -
>> - buck1_reg: BUCK1 {
>> - regulator-name = "VDD_MIF_1.2V";
>> - regulator-min-microvolt = <950000>;
>> - regulator-max-microvolt = <1350000>;
>> - regulator-always-on;
>> - regulator-boot-on;
>> - };
>> -
>> - vemmc_reg: BUCK9 {
>> - regulator-name = "VMEM_VDD_2.8V";
>> - regulator-min-microvolt = <2800000>;
>> - regulator-max-microvolt = <2800000>;
>> - op_mode = <3>; /* Standby Mode */
>> - s5m8767,pmic-ext-control-gpios = <&gpk0 2 0>;
>> - };
>> - };
>> - };
>> diff --git a/Documentation/devicetree/bindings/regulator/samsung,s5m8767.yaml b/Documentation/devicetree/bindings/regulator/samsung,s5m8767.yaml
>> new file mode 100644
>> index 000000000000..3192a06b5ef9
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/regulator/samsung,s5m8767.yaml
>> @@ -0,0 +1,83 @@
>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/regulator/samsung,s5m8767.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Samsung S5M8767 Power Management IC regulators
>> +
>> +maintainers:
>> + - Krzysztof Kozlowski <krzysztof.kozlowski@...onical.com>
>> +
>> +description: |
>> + This is a part of device tree bindings for S2M and S5M family of Power
>> + Management IC (PMIC).
>> +
>> + The S5M8767 provides buck and LDO regulators.
>> +
>> + See also Documentation/devicetree/bindings/mfd/samsung,s5m8767.yaml for
>> + additional information and example.
>> +
>> +patternProperties:
>> + # 28 LDOs
>> + "^LDO([1-9]|1[0-9]|2[0-8])$":
>> + type: object
>> + $ref: regulator.yaml#
>> + description:
>> + Properties for single LDO regulator.
>> +
>> + properties:
>> + regulator-name: true
>
> This is fine, but technically it is not needed. There's not any
> requirement that items in 'required' be listed under 'properties'.
I'll skip it here and in further patches - smaller bindings.
>
>> +
>> + op_mode:
>
> Needs a type reference.
Indeed, thanks.
>
>> + enum: [0, 1, 2, 3]
>> + default: 1
>> + description: |
>> + Describes the different operating modes of the LDO's with power mode
>> + change in SOC. The different possible values are:
>> + 0 - always off mode
>> + 1 - on in normal mode
>> + 2 - low power mode
>> + 3 - suspend mode
>> +
>> + required:
>> + - regulator-name
>> +
>> + unevaluatedProperties: false
>
> On child nodes, I think it's better to place this up with the $ref and
> 'type: object' since the indentation can be hard to follow.
OK.
Best regards,
Krzysztof
Powered by blists - more mailing lists