Re: [PATCH v2] dt-bindings: Convert active-semi PMIC docs to YAML schemas

From: Krzysztof Kozlowski
Date: Sun Nov 06 2022 - 04:55:21 EST

Next message: Chen Guokai: "[PATCH v4 6/8] riscv/kprobe: Add code to check if kprobe can be optimized"
Previous message: Mengqi Zhang (张梦琦): "Re: [PATCH 2/2] dt-bingdings: mmc: Mediatek: add ICE clock"
In reply to: Paul Cercueil: "[PATCH v2] dt-bindings: Convert active-semi PMIC docs to YAML schemas"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On 05/11/2022 23:58, 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>
> ---
>
> Notes:
> v2:
> - Avoid | character in descriptions that can be single-line
> - Remove unevaluatedProperties when additionalProperties is also present
> - Remove useless inner parentheses in regular expressions
> - Rename I2C nodes to just... i2c
> - Remove node handles
>
> 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.

It should not be Liam and Mark, but someone having/knowing this
particular hardware.

>
> .../bindings/regulator/act8865-regulator.txt | 117 --------
> .../bindings/regulator/act8945a-regulator.txt | 113 --------
> .../regulator/active-semi,act8600.yaml | 141 ++++++++++
> .../regulator/active-semi,act8846.yaml | 207 ++++++++++++++
> .../regulator/active-semi,act8865.yaml | 162 +++++++++++
> .../regulator/active-semi,act8945a.yaml | 261 ++++++++++++++++++
> 6 files changed, 771 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/act8865-regulator.txt b/Documentation/devicetree/bindings/regulator/act8865-regulator.txt
> deleted file mode 100644
> index b9f58e480349..000000000000
> --- a/Documentation/devicetree/bindings/regulator/act8865-regulator.txt
> +++ /dev/null
> @@ -1,117 +0,0 @@
> -ACT88xx regulators
> --------------------
> -
> -Required properties:
> -- compatible: "active-semi,act8846" or "active-semi,act8865" or "active-semi,act8600"
> -- reg: I2C slave address
> -
> -Optional properties:
> -- system-power-controller: Telling whether or not this pmic is controlling
> - the system power. See Documentation/devicetree/bindings/power/power-controller.txt .
> -- active-semi,vsel-high: Indicates the VSEL pin is high.
> - If this property is missing, assume the VSEL pin is low(0).
> -
> -Optional input supply properties:
> -- for act8600:
> - - vp1-supply: The input supply for DCDC_REG1
> - - vp2-supply: The input supply for DCDC_REG2
> - - vp3-supply: The input supply for DCDC_REG3
> - - inl-supply: The input supply for LDO_REG5, LDO_REG6, LDO_REG7 and LDO_REG8
> - SUDCDC_REG4, LDO_REG9 and LDO_REG10 do not have separate supplies.
> -- for act8846:
> - - vp1-supply: The input supply for REG1
> - - vp2-supply: The input supply for REG2
> - - vp3-supply: The input supply for REG3
> - - vp4-supply: The input supply for REG4
> - - inl1-supply: The input supply for REG5, REG6 and REG7
> - - inl2-supply: The input supply for REG8 and LDO_REG9
> - - inl3-supply: The input supply for REG10, REG11 and REG12
> -- for act8865:
> - - vp1-supply: The input supply for DCDC_REG1
> - - vp2-supply: The input supply for DCDC_REG2
> - - vp3-supply: The input supply for DCDC_REG3
> - - inl45-supply: The input supply for LDO_REG1 and LDO_REG2
> - - inl67-supply: The input supply for LDO_REG3 and LDO_REG4
> -
> -Any standard regulator properties can be used to configure the single regulator.
> -regulator-initial-mode, regulator-allowed-modes and regulator-mode could be specified
> -for act8865 using mode values from dt-bindings/regulator/active-semi,8865-regulator.h
> -file.
> -
> -The valid names for regulators are:
> - - for act8846:
> - REG1, REG2, REG3, REG4, REG5, REG6, REG7, REG8, REG9, REG10, REG11, REG12
> - - for act8865:
> - DCDC_REG1, DCDC_REG2, DCDC_REG3, LDO_REG1, LDO_REG2, LDO_REG3, LDO_REG4.
> - - for act8600:
> - DCDC_REG1, DCDC_REG2, DCDC_REG3, SUDCDC_REG4, LDO_REG5, LDO_REG6, LDO_REG7,
> - LDO_REG8, LDO_REG9, LDO_REG10,
> -
> -Example:
> ---------
> -
> -#include <dt-bindings/regulator/active-semi,8865-regulator.h>
> -
> - i2c1: i2c@f0018000 {
> - pmic: act8865@5b {
> - compatible = "active-semi,act8865";
> - reg = <0x5b>;
> - active-semi,vsel-high;
> -
> - regulators {
> - vcc_1v8_reg: DCDC_REG1 {
> - regulator-name = "VCC_1V8";
> - regulator-min-microvolt = <1800000>;
> - regulator-max-microvolt = <1800000>;
> - regulator-always-on;
> - };
> -
> - vcc_1v2_reg: DCDC_REG2 {
> - regulator-name = "VCC_1V2";
> - regulator-min-microvolt = <1100000>;
> - regulator-max-microvolt = <1300000>;
> - regulator-always-on;
> -
> - regulator-allowed-modes = <ACT8865_REGULATOR_MODE_FIXED>,
> - <ACT8865_REGULATOR_MODE_LOWPOWER>;
> - regulator-initial-mode = <ACT8865_REGULATOR_MODE_FIXED>;
> -
> - regulator-state-mem {
> - regulator-on-in-suspend;
> - regulator-suspend-min-microvolt = <1150000>;
> - regulator-suspend-max-microvolt = <1150000>;
> - regulator-changeable-in-suspend;
> - regulator-mode = <ACT8865_REGULATOR_MODE_LOWPOWER>;
> - };
> - };
> -
> - vcc_3v3_reg: DCDC_REG3 {
> - regulator-name = "VCC_3V3";
> - regulator-min-microvolt = <3300000>;
> - regulator-max-microvolt = <3300000>;
> - regulator-always-on;
> - };
> -
> - vddana_reg: LDO_REG1 {
> - regulator-name = "VDDANA";
> - regulator-min-microvolt = <3300000>;
> - regulator-max-microvolt = <3300000>;
> - regulator-always-on;
> -
> - regulator-allowed-modes = <ACT8865_REGULATOR_MODE_NORMAL>,
> - <ACT8865_REGULATOR_MODE_LOWPOWER>;
> - regulator-initial-mode = <ACT8865_REGULATOR_MODE_NORMAL>;
> -
> - regulator-state-mem {
> - regulator-off-in-suspend;
> - };
> - };
> -
> - vddfuse_reg: LDO_REG2 {
> - regulator-name = "FUSE_2V5";
> - regulator-min-microvolt = <2500000>;
> - regulator-max-microvolt = <2500000>;
> - };
> - };
> - };
> - };
> diff --git a/Documentation/devicetree/bindings/regulator/act8945a-regulator.txt b/Documentation/devicetree/bindings/regulator/act8945a-regulator.txt
> deleted file mode 100644
> index 4017527619ab..000000000000
> --- a/Documentation/devicetree/bindings/regulator/act8945a-regulator.txt
> +++ /dev/null
> @@ -1,113 +0,0 @@
> -Device-Tree bindings for regulators of Active-semi ACT8945A Multi-Function Device
> -
> -Required properties:
> - - compatible: "active-semi,act8945a", please refer to ../mfd/act8945a.txt.
> -
> -Optional properties:
> -- active-semi,vsel-high: Indicates if the VSEL pin is set to logic-high.
> - If this property is missing, assume the VSEL pin is set to logic-low.
> -
> -Optional input supply properties:
> - - vp1-supply: The input supply for REG_DCDC1
> - - vp2-supply: The input supply for REG_DCDC2
> - - vp3-supply: The input supply for REG_DCDC3
> - - inl45-supply: The input supply for REG_LDO1 and REG_LDO2
> - - inl67-supply: The input supply for REG_LDO3 and REG_LDO4
> -
> -Any standard regulator properties can be used to configure the single regulator.
> -regulator-initial-mode, regulator-allowed-modes and regulator-mode could be
> -specified using mode values from dt-bindings/regulator/active-semi,8945a-regulator.h
> -file.
> -
> -The valid names for regulators are:
> - REG_DCDC1, REG_DCDC2, REG_DCDC3, REG_LDO1, REG_LDO2, REG_LDO3, REG_LDO4.
> -
> -Example:
> -
> -#include <dt-bindings/regulator/active-semi,8945a-regulator.h>
> -
> - pmic@5b {
> - compatible = "active-semi,act8945a";
> - reg = <0x5b>;
> -
> - active-semi,vsel-high;
> -
> - regulators {
> - vdd_1v35_reg: REG_DCDC1 {
> - regulator-name = "VDD_1V35";
> - regulator-min-microvolt = <1350000>;
> - regulator-max-microvolt = <1350000>;
> - regulator-always-on;
> -
> - regulator-allowed-modes = <ACT8945A_REGULATOR_MODE_FIXED>,
> - <ACT8945A_REGULATOR_MODE_LOWPOWER>;
> - regulator-initial-mode = <ACT8945A_REGULATOR_MODE_FIXED>;
> -
> - regulator-state-mem {
> - regulator-on-in-suspend;
> - regulator-suspend-min-microvolt=<1400000>;
> - regulator-suspend-max-microvolt=<1400000>;
> - regulator-changeable-in-suspend;
> - regulator-mode=<ACT8945A_REGULATOR_MODE_LOWPOWER>;
> - };
> - };
> -
> - vdd_1v2_reg: REG_DCDC2 {
> - regulator-name = "VDD_1V2";
> - regulator-min-microvolt = <1100000>;
> - regulator-max-microvolt = <1300000>;
> - regulator-always-on;
> -
> - regulator-allowed-modes = <ACT8945A_REGULATOR_MODE_FIXED>,
> - <ACT8945A_REGULATOR_MODE_LOWPOWER>;
> - regulator-initial-mode = <ACT8945A_REGULATOR_MODE_FIXED>;
> -
> - regulator-state-mem {
> - regulator-off-in-suspend;
> - };
> - };
> -
> - vdd_3v3_reg: REG_DCDC3 {
> - regulator-name = "VDD_3V3";
> - regulator-min-microvolt = <3300000>;
> - regulator-max-microvolt = <3300000>;
> - regulator-always-on;
> - };
> -
> - vdd_fuse_reg: REG_LDO1 {
> - regulator-name = "VDD_FUSE";
> - regulator-min-microvolt = <2500000>;
> - regulator-max-microvolt = <2500000>;
> - regulator-always-on;
> -
> - regulator-allowed-modes = <ACT8945A_REGULATOR_MODE_NORMAL>,
> - <ACT8945A_REGULATOR_MODE_LOWPOWER>;
> - regulator-initial-mode = <ACT8945A_REGULATOR_MODE_NORMAL>;
> -
> - regulator-state-mem {
> - regulator-off-in-suspend;
> - };
> - };
> -
> - vdd_3v3_lp_reg: REG_LDO2 {
> - regulator-name = "VDD_3V3_LP";
> - regulator-min-microvolt = <3300000>;
> - regulator-max-microvolt = <3300000>;
> - regulator-always-on;
> - };
> -
> - vdd_led_reg: REG_LDO3 {
> - regulator-name = "VDD_LED";
> - regulator-min-microvolt = <3300000>;
> - regulator-max-microvolt = <3300000>;
> - regulator-always-on;
> - };
> -
> - vdd_sdhc_1v8_reg: REG_LDO4 {
> - regulator-name = "VDD_SDHC_1V8";
> - regulator-min-microvolt = <1800000>;
> - regulator-max-microvolt = <1800000>;
> - regulator-always-on;
> - };
> - };
> - };
> 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..ca0ac316dff2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/active-semi,act8600.yaml
> @@ -0,0 +1,141 @@
> +# 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

Drop description, it's obvious. The same in other files.

> + maxItems: 1
> +
> + system-power-controller:
> + description:
> + Indicates that the ACT8600 is responsible for powering OFF
> + the system.
> + type: boolean
> +

(...)

> +examples:
> + - |
> + #include <dt-bindings/regulator/active-semi,8865-regulator.h>
> +
> + i2c1 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + pmic: act8865@5b {
> + compatible = "active-semi,act8865";
> + reg = <0x5b>;
> + active-semi,vsel-high;
> +
> + regulators {
> + vcc_1v8_reg: DCDC_REG1 {
> + regulator-name = "VCC_1V8";
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + regulator-always-on;
> + };
> +
> + vcc_1v2_reg: DCDC_REG2 {
> + regulator-name = "VCC_1V2";
> + regulator-min-microvolt = <1100000>;
> + regulator-max-microvolt = <1300000>;
> + regulator-always-on;
> +
> + regulator-allowed-modes = <ACT8865_REGULATOR_MODE_FIXED>,
> + <ACT8865_REGULATOR_MODE_LOWPOWER>;
> + regulator-initial-mode = <ACT8865_REGULATOR_MODE_FIXED>;
> +
> + regulator-state-mem {
> + regulator-on-in-suspend;
> + regulator-suspend-min-microvolt = <1150000>;
> + regulator-suspend-max-microvolt = <1150000>;
> + regulator-changeable-in-suspend;
> + regulator-mode = <ACT8865_REGULATOR_MODE_LOWPOWER>;
> + };
> + };
> +
> + vcc_3v3_reg: DCDC_REG3 {
> + regulator-name = "VCC_3V3";
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + regulator-always-on;
> + };
> +
> + vddana_reg: LDO_REG1 {
> + regulator-name = "VDDANA";
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + regulator-always-on;
> +
> + regulator-allowed-modes = <ACT8865_REGULATOR_MODE_NORMAL>,
> + <ACT8865_REGULATOR_MODE_LOWPOWER>;

Align it with previous <.

> + regulator-initial-mode = <ACT8865_REGULATOR_MODE_NORMAL>;

(...)

> +
> + charger {
> + compatible = "active-semi,act8945a-charger";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_charger_chglev &pinctrl_charger_lbo &pinctrl_charger_irq>;
> + interrupt-parent = <&pioA>;
> + interrupts = <45 IRQ_TYPE_EDGE_RISING>;
> +
> + active-semi,chglev-gpios = <&pioA 12 GPIO_ACTIVE_HIGH>;
> + active-semi,lbo-gpios = <&pioA 72 GPIO_ACTIVE_LOW>;
> + active-semi,input-voltage-threshold-microvolt = <6600>;
> + active-semi,precondition-timeout = <40>;
> + active-semi,total-timeout = <3>;
> + status = "okay";

Drop status

> + };
> + };
> + };

Best regards,
Krzysztof

Next message: Chen Guokai: "[PATCH v4 6/8] riscv/kprobe: Add code to check if kprobe can be optimized"
Previous message: Mengqi Zhang (张梦琦): "Re: [PATCH 2/2] dt-bingdings: mmc: Mediatek: add ICE clock"
In reply to: Paul Cercueil: "[PATCH v2] dt-bindings: Convert active-semi PMIC docs to YAML schemas"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]