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

Krzysztof Kozlowski
2024-03-18T16:10:55Z

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@analog.com>

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@analog.com>
> +
> +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@vger.kernel.org
> S: Orphan
> F: drivers/net/wireless/admtek/adm8211.*
>
> +ADP1050 HARDWARE MONITOR DRIVER
> +M: Radu Sabau <radu.sabau@analog.com>
> +L: linux-hwmon@vger.kernel.org
> +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