Messages in this thread | | | Date | Wed, 26 Jul 2023 11:19:12 -0700 | Subject | Re: [PATCH 2/2] dt-bindings: hwmon: add renesas,isl28022 | From | Guenter Roeck <> |
| |
On 7/26/23 09:14, Krzysztof Kozlowski wrote: > On 26/07/2023 17:22, Carsten Spieß wrote: >> Add dt-bindings for Renesase ISL28022 power monitor. >> >> Signed-off-by: Carsten Spieß <mail@carsten-spiess.de> >> --- > > 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@carsten-spiess.de> >> + >> +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"? >
Something with samples. adi,power-sample-average is similar. Others use average-samples, qcom,avg-samples, touchscreen-average-samples.
Guenter
>> + >> +required: >> + - compatible >> + - reg >> + >> +additionalProperties: false >> + > > > Best regards, > Krzysztof >
| |