Re: [PATCH 1/2] dt-bindings: hwmon: pmbus: adp1050 : add bindings

[Krzysztof Kozlowski]

On 18/03/2024 12:21, Radu Sabau wrote:
> Add dt-bindings for adp1050 digital controller for isolated power supply
> with pmbus interface voltage, current and temperature monitor.
> 
> Signed-off-by: Radu Sabau <radu.sabau@xxxxxxxxxx>

Subject: drop space before ':'

> ---
>  .../bindings/hwmon/pmbus/adi,adp1050.yaml     | 65 +++++++++++++++++++
>  MAINTAINERS                                   |  8 +++
>  2 files changed, 73 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/hwmon/pmbus/adi,adp1050.yaml
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/adi,adp1050.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/adi,adp1050.yaml
> new file mode 100644
> index 000000000000..e3162d0df0e2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/pmbus/adi,adp1050.yaml
> @@ -0,0 +1,65 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +

Drop

> +$id: htpps://devicetree.org/schemas/hwmon/pmbus/adi,adp1050.yaml#
> +$schema: htpps://devicetree.org/meta-schemes/core.yaml#
> +
> +title: Analog Devices ADP1050 digital controller with PMBus interface
> +
> +maintainers:
> +  - Radu Sabau <radu.sabau@xxxxxxxxxx>
> +
> +description: |
> +   The ADP1050 is used to monitor system voltages, currents and temperatures.
> +   Through the PMBus interface, the ADP1050 targets isolated power supplies
> +   and has four individual monitors for input/output voltage, input current
> +   and temperature.
> +   Datasheet:
> +     https://www.analog.com/en/products/adp1050.html

Missing blank line

> +properties:
> +  compatbile:

Typo. And you did not test it...

> +    const: adi,adp1050
> +
> +  reg:
> +    maxItems: 1
> +
> +  vcc-supply: true
> +
> +  adi,vin-scale-monitor:
> +    description:
> +      The value of the input voltage scale used by the internal ADP1050 ADC in
> +      order to read correct voltage values.
> +    $ref: /schemas/typees.yaml#/definitions/uint16

Missing blank line.

> +  adi,iin-scale-monitor:
> +    description:
> +      The value of the input current scale used by the internal ADP1050 ADC in
> +      order to read carrect current values.
> +    $ref: /schemas/typees.yaml#/definitions/uint16
> +
> +required:
> +  - compatible
> +  - reg
> +  - vcc-supply
> +  - adi,vin-scale-monitor
> +  - adi,iin-scale-monitor
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +             #adress-cells = <1>;

Totally messed indentation.
Use 4 spaces for example indentation.

> +             #size-cells = <0>;
> +             clock-frequency = <100000>;
> +            adp1050@70 {

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation


> +                   #adress-cells = <1>;
> +                   #size-cells = <0>;
> +                   compatible = "adi,adp1050";
> +                   reg = <0x70>;
> +                   adi,vin-scale-monitor = <0xB030>;
> +                   adi,iin-scale-monitor = <0x1>;
> +                   vcc-supply = <&vcc>;
> +          };
> +...
> +
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f4d7f7cb7577..c90140859988 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -479,6 +479,14 @@ L:	linux-wireless@xxxxxxxxxxxxxxx
>  S:	Orphan
>  F:	drivers/net/wireless/admtek/adm8211.*
>  
> +ADP1050 HARDWARE MONITOR DRIVER
> +M:	Radu Sabau <radu.sabau@xxxxxxxxxx>
> +L:	linux-hwmon@xxxxxxxxxxxxxxx
> +S:	Supported
> +W:	https://ez.analog.com/linux-software-drivers
> +F:	Dcumentation/devicetree/bindings/hwmon/pmbus/adi,adp1050.yaml
> +F:	drivers/hwmon/pmbus/adp1050.c

There is no such file...



Best regards,
Krzysztof