On 26/07/2023 17:22, Carsten Spieß wrote: > Add dt-bindings for Renesase ISL28022 power monitor. > > Signed-off-by: Carsten Spieß <mail@xxxxxxxxxxxxxxxxx> > --- Thank you for your patch. There is something to discuss/improve. > .../bindings/hwmon/renesas,isl28022.yaml | 67 +++++++++++++++++++ > MAINTAINERS | 1 + > 2 files changed, 68 insertions(+) > create mode 100644 Documentation/devicetree/bindings/hwmon/renesas,isl28022.yaml > > diff --git a/Documentation/devicetree/bindings/hwmon/renesas,isl28022.yaml b/Documentation/devicetree/bindings/hwmon/renesas,isl28022.yaml > new file mode 100644 > index 000000000000..5ecf892db269 > --- /dev/null > +++ b/Documentation/devicetree/bindings/hwmon/renesas,isl28022.yaml > @@ -0,0 +1,67 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > + No blank line. > +$id: http://devicetree.org/schemas/hwmon/renesas,isl28022.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Renesas ISL28022 power monitor > + > +maintainers: > + - Carsten Spieß <mail@xxxxxxxxxxxxxxxxx> > + > +description: | > + The ISL28022 is a power monitor with I2C interface. The device monitors > + voltage, current via shunt resistor and calculated power. > + > + Datasheets: > + https://www.renesas.com/us/en/www/doc/datasheet/isl28022.pdf > + > +Required properties: > + compatible: > + enum: > + - renesas,isl28022 > + > + reg: > + maxItems: 1 > + > +Optional properties: 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. These are "properties" > + shunt-resistor-micro-ohms: > + description: > + Shunt resistor value in micro-Ohm > + defaults to <0> when not set > + monitoring of current and power not supported when <0> > + > + shunt-gain: 1. Missing vendor prefix (does not look like generic property) 2. -microvolt > + description: > + Shunt gain to scale maximal shunt voltage to > + 40mV, 80mV, 160mV, 320mV > + defaults to <8> (320mV) when not set And then enum is for 40, 80, 160 and 320. > + enum: [1, 2, 4, 8] > + > + average: > + description: | > + Number of samples to be used to report voltage, current and power values. > + defaults to <0> when not set > + enum: [0, 1, 2, 4, 8, 16, 32, 64, 128] I am sure hwmon has some property for this. Are you sure it is called "average"? > + > +required: > + - compatible > + - reg > + > +additionalProperties: false > + Best regards, Krzysztof
