On 25/08/2023 18:42, Ninad Malwade wrote: > Convert the TI INA3221 bindings from the free-form text format to > json-schema. > > Signed-off-by: Thierry Reding <treding@xxxxxxxxxx> > Signed-off-by: Ninad Malwade <nmalwade@xxxxxxxxxx> > --- This is v2, so where is the changelog? > .../devicetree/bindings/hwmon/ina3221.txt | 54 --------- > .../devicetree/bindings/hwmon/ti,ina3221.yaml | 109 ++++++++++++++++++ > 2 files changed, 109 insertions(+), 54 deletions(-) > delete mode 100644 Documentation/devicetree/bindings/hwmon/ina3221.txt > create mode 100644 Documentation/devicetree/bindings/hwmon/ti,ina3221.yaml > ... > diff --git a/Documentation/devicetree/bindings/hwmon/ti,ina3221.yaml b/Documentation/devicetree/bindings/hwmon/ti,ina3221.yaml > new file mode 100644 > index 000000000000..0c6d41423d8c > --- /dev/null > +++ b/Documentation/devicetree/bindings/hwmon/ti,ina3221.yaml > @@ -0,0 +1,109 @@ > +# SPDX-License-Identifier: GPL-2.0-only I assume you do not use standard license because of copying the description? > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/hwmon/ti,ina3221.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Texas Instruments INA3221 Current and Voltage Monitor > + > +maintainers: > + - Jean Delvare <jdelvare@xxxxxxxx> > + - Guenter Roeck <linux@xxxxxxxxxxxx> > + > +properties: > + compatible: > + const: ti,ina3221 > + > + reg: > + maxItems: 1 > + > + ti,single-shot: > + description: | > + This chip has two power modes: single-shot (chip takes one measurement > + and then shuts itself down) and continuous (chip takes continuous > + measurements). The continuous mode is more reliable and suitable for > + hardware monitor type device, but the single-shot mode is more power- > + friendly and useful for battery-powered device which cares power > + consumptions while still needs some measurements occasionally. > + > + If this property is present, the single-shot mode will be used, instead > + of the default continuous one for monitoring. > + $ref: /schemas/types.yaml#/definitions/flag > + > + "#address-cells": > + description: Required only if a child node is present. > + const: 1 > + > + "#size-cells": > + description: Required only if a child node is present. > + const: 0 > + > +patternProperties: > + "^input@[0-2]$": > + description: The node contains optional child nodes for three channels. > + Each child node describes the information of input source. > + type: object > + properties: > + reg: > + description: Must be 0, 1 and 2, corresponding to the IN1, IN2 or IN3 > + ports of the INA3221, respectively. > + enum: [ 0, 1, 2 ] > + > + label: > + description: name of the input source > + > + shunt-resistor-micro-ohms: > + description: shunt resistor value in micro-Ohm > + > + additionalProperties: false This should be rather after type:object for readability. > + > + required: > + - reg > + > +additionalProperties: false And this please keep like in example schema, so after required:. > + > +required: > + - compatible > + - reg > + > +examples: > + - | > + #include <dt-bindings/clock/tegra186-clock.h> > + #include <dt-bindings/interrupt-controller/arm-gic.h> > + #include <dt-bindings/reset/tegra186-reset.h> > + > + i2c@3160000 { > + compatible = "nvidia,tegra186-i2c"; > + reg = <0x03160000 0x10000>; > + interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&bpmp TEGRA186_CLK_I2C1>; > + clock-names = "div-clk"; > + resets = <&bpmp TEGRA186_RESET_I2C1>; > + reset-names = "i2c"; Drop all this. Not related, You only need i2c node with address/size-cells. > + > + #address-cells = <1>; > + #size-cells = <0>; > + > + ina3221@40 { 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 > + compatible = "ti,ina3221"; > + reg = <0x40>; > + #address-cells = <1>; > + #size-cells = <0>; > + > + input@0 { > + reg = <0x0>; > + status = "disabled"; Why is this node present? Binding said nodes are optional, so I assume it can be just skipped. If all children must be there, then you should actually require them in the binding (and mention it briefly in commit msg). > + }; Best regards, Krzysztof
