Re: [PATCH v2 9/9] dt-bindings: Add documentation for rt5033 mfd, regulator and charger

From: Krzysztof Kozlowski
Date: Sun Apr 16 2023 - 14:39:38 EST

On 16/04/2023 14:44, Jakob Hauser wrote:
> Add device tree binding documentation for rt5033 multifunction device, voltage
> regulator and battery charger.

Subject: drop second/last, redundant "documentation". The "dt-bindings"
prefix is already stating that these are documentation.

>
> Cc: Beomho Seo <beomho.seo@xxxxxxxxxxx>
> Cc: Chanwoo Choi <cw00.choi@xxxxxxxxxxx>
> Signed-off-by: Jakob Hauser <jahau@xxxxxxxxxxxxxx>
> ---
> The patch is based on linux-next (tag "next-20230413").
>
> .../bindings/mfd/richtek,rt5033.yaml | 90 +++++++++++++++++++
> .../power/supply/richtek,rt5033-charger.yaml | 76 ++++++++++++++++
> .../regulator/richtek,rt5033-regulator.yaml | 24 +++++
> 3 files changed, 190 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mfd/richtek,rt5033.yaml
> create mode 100644 Documentation/devicetree/bindings/power/supply/richtek,rt5033-charger.yaml
> create mode 100644 Documentation/devicetree/bindings/regulator/richtek,rt5033-regulator.yaml
>



> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + pmic@34 {
> + compatible = "richtek,rt5033";
> + reg = <0x34>;
> +
> + interrupt-parent = <&msmgpio>;
> + interrupts = <62 IRQ_TYPE_EDGE_FALLING>;
> +
> + pinctrl-names = "default";
> + pinctrl-0 = <&pmic_int_default>;
> +
> + regulators {
> + safe_ldo_reg: safe_ldo {

If you could change it: No underscores in node names... but you cannot.
This is old driver so you will break the users.

> + regulator-name = "safe_ldo";
> + regulator-min-microvolt = <4900000>;
> + regulator-max-microvolt = <4900000>;
> + regulator-always-on;
> + };
> + ldo_reg: ldo {
> + regulator-name = "ldo";
> + regulator-min-microvolt = <2800000>;
> + regulator-max-microvolt = <2800000>;
> + };
> + buck_reg: buck {
> + regulator-name = "buck";
> + regulator-min-microvolt = <1200000>;
> + regulator-max-microvolt = <1200000>;
> + };
> + };
> +
> + charger {
> + compatible = "richtek,rt5033-charger";
> + richtek,pre-microamp = <450000>;
> + richtek,fast-microamp = <1000000>;
> + richtek,eoc-microamp = <150000>;
> + richtek,pre-threshold-microvolt = <3500000>;
> + richtek,const-microvolt = <4350000>;
> + extcon = <&muic>;
> + };
> + };
> + };
> diff --git a/Documentation/devicetree/bindings/power/supply/richtek,rt5033-charger.yaml b/Documentation/devicetree/bindings/power/supply/richtek,rt5033-charger.yaml
> new file mode 100644
> index 000000000000..439e0b7962f3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/supply/richtek,rt5033-charger.yaml
> @@ -0,0 +1,76 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/power/supply/richtek,rt5033-charger.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Richtek RT5033 PIMC Battery Charger
> +
> +maintainers:
> + - Jakob Hauser <jahau@xxxxxxxxxxxxxx>
> +
> +description:
> + The battery charger of the multifunction device RT5033 has to be instantiated
> + under sub-node named "charger" using the following format.
> +
> +properties:
> + compatible:
> + const: richtek,rt5033-charger
> +
> + richtek,pre-microamp:
> + description:
> + Current of pre-charge mode. The pre-charge current levels are 350 mA to
> + 650 mA programmed by I2C per 100 mA.

minimum:
maximum:
multipleOf: 100

Same for other cases.

> + maxItems: 1
> +
> + richtek,fast-microamp:
> + description:
> + Current of fast-charge mode. The fast-charge current levels are 700 mA
> + to 2000 mA programmed by I2C per 100 mA.
> + maxItems: 1
> +
> + richtek,eoc-microamp:
> + description:
> + This property is end of charge current. Its level ranges from 150 mA to
> + 600 mA. Between 150 mA and 300 mA in 50 mA steps, between 300 mA and 600 mA
> + in 100 mA steps.
> + maxItems: 1
> +
> + richtek,pre-threshold-microvolt:
> + description:
> + Voltage of pre-charge mode. If the battery voltage is below the pre-charge
> + threshold voltage, the charger is in pre-charge mode with pre-charge current.
> + Its levels are 2.3 V to 3.8 V programmed by I2C per 0.1 V.
> + maxItems: 1
> +
> + richtek,const-microvolt:
> + description:
> + Battery regulation voltage of constant voltage mode. This voltage levels from
> + 3.65 V to 4.4 V by I2C per 0.025 V.
> + maxItems: 1
> +
> + extcon:
> + description:
> + Phandle to the extcon device.
> + maxItems: 1
> +
> +required:
> + - richtek,pre-microamp
> + - richtek,fast-microamp
> + - richtek,eoc-microamp
> + - richtek,pre-threshold-microvolt
> + - richtek,const-microvolt
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + charger {
> + compatible = "richtek,rt5033-charger";
> + richtek,pre-microamp = <450000>;
> + richtek,fast-microamp = <1000000>;
> + richtek,eoc-microamp = <150000>;
> + richtek,pre-threshold-microvolt = <3500000>;
> + richtek,const-microvolt = <4350000>;
> + extcon = <&muic>;
> + };
> diff --git a/Documentation/devicetree/bindings/regulator/richtek,rt5033-regulator.yaml b/Documentation/devicetree/bindings/regulator/richtek,rt5033-regulator.yaml
> new file mode 100644
> index 000000000000..66c8a0692e10
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/richtek,rt5033-regulator.yaml
> @@ -0,0 +1,24 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/regulator/richtek,rt5033-regulator.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Richtek RT5033 PIMC Voltage Regulator

You should explain in commit msg that you document existing driver in
the Linux kernel. We would not cut some slack, e.g. stricter rules
applied to new bindings.

> +
> +maintainers:
> + - Jakob Hauser <jahau@xxxxxxxxxxxxxx>
> +
> +description:
> + The regulators of RT5033 have to be instantiated under a sub-node named
> + "regulators". For "safe_ldo" voltage there is only one value of 4.9 V. "ldo"
> + voltage ranges from 1.2 V to 3.0 V in 0.1 V steps. "buck" voltage ranges from
> + 1.0 V to 3.0 V in 0.1 V steps.
> +
> +patternProperties:
> + "^(safe_ldo|ldo|buck)$":
> + type: object
> + $ref: /schemas/regulator/regulator.yaml#

Just squash it with parent schema. No real benefits of having regulators
separate - it's very small one.

Best regards,
Krzysztof