Re: [PATCH v2 1/6] dt-bindings: regulator: add Samsung s2dos05 pmic

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 24/10/2023 17:43, Dzmitry Sankouski wrote:
> Add binding for the s2dos05 pmic found in the Samsung S9.
> 
> Signed-off-by: Dzmitry Sankouski <dsankouski@xxxxxxxxx>
> Cc: Conor Dooley <conor+dt@xxxxxxxxxx>
> Cc: Dzmitry Sankouski <dsankouski@xxxxxxxxx>
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@xxxxxxxxxx>
> Cc: Liam Girdwood <lgirdwood@xxxxxxxxx>
> Cc: Mark Brown <broonie@xxxxxxxxxx>
> Cc: Rob Herring <robh+dt@xxxxxxxxxx>
> Cc: devicetree@xxxxxxxxxxxxxxx
> 
> ---
> 
> Changes in v2:
> - hex to decimal in regulator values
> - fix compatible value
> 
>  .../bindings/regulator/samsung,s2dos05.yaml   | 89 +++++++++++++++++++
>  1 file changed, 89 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/regulator/samsung,s2dos05.yaml
> 
> diff --git a/Documentation/devicetree/bindings/regulator/samsung,s2dos05.yaml b/Documentation/devicetree/bindings/regulator/samsung,s2dos05.yaml
> new file mode 100644
> index 000000000000..690537738e67
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/samsung,s2dos05.yaml
> @@ -0,0 +1,89 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/regulator/samsung,s2dos05.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Samsung s2dos05 regulator

Not a PMIC/MFD device? If not, then:
Samsung S2DOS05 Power Management IC

Otherwise, if this is only for regulators:
Samsung S2DOS05 Power Management IC Regulators


> +
> +maintainers:
> +  - Dzmitry Sankouski <dsankouski@xxxxxxxxx>
> +
> +description: |

Do not need '|' unless you need to preserve formatting.

> +  The S2DOS05 is a companion power management IC for the smart phones.
> +  Has 4 LDO and 1 BUCK regulators, and has capability to measure
> +  current and power. Can detect short circuit on outputs.
> +
> +properties:
> +  compatible:
> +    const: samsung,s2dos05

Blank line.

> +  reg:
> +    maxItems: 1
> +
> +  regulators:
> +    type: object
> +    description: List of regulators and its properties
> +
> +    patternProperties:
> +      "^s2dos05-buck1|s2dos05-ldo[1-4]$":

^buck1|ldo[1-4]$

> +        type: object
> +        $ref: "regulator.yaml#"

Drop quotes.

It does not look like you tested the bindings, at least after quick
look. Please run `make dt_binding_check` (see
Documentation/devicetree/bindings/writing-schema.rst for instructions).
Maybe you need to update your dtschema and yamllint.

> +        unevaluatedProperties: false
> +
> +    additionalProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - regulators
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    regulator@60 {
> +    	compatible = "samsung,s2dos05";
> +    	reg = <0x60>;
> +    	pinctrl-names = "default";
> +    	pinctrl-0 = <&s2dos05_irq>;
> +    	s2dos05,s2dos05_int = <&tlmm 0x31 0x0>;

Eh... No, please test before sending. Drop the property or explain what
is it supposed to be.

> +
> +    	regulators {
> +    		s2dos05_ldo1: s2dos05-ldo1 {

You have messed up indentation. Use 4 spaces for example indentation.

> +    			regulator-name = "s2dos05-ldo1";
> +    			regulator-min-microvolt = <1500000>;
> +    			regulator-max-microvolt = <2000000>;
> +    			regulator-active-discharge = <0x1>;

That's not a hex, but just decimal 1.


Best regards,
Krzysztof





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux