| Message ID | 20250106071337.3017926-2-Leo-Yang@quantatw.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | hwmon: Add support for INA233 | expand |
On 06/01/2025 08:13, Leo Yang wrote: > Add TI INA233 Current and Power Monitor bindings. Please use subject prefixes matching the subsystem. You can get them for example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory your patch is touching. For bindings, the preferred subjects are explained here: https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters > > Signed-off-by: Leo Yang <Leo-Yang@quantatw.com> > --- > .../bindings/hwmon/pmbus/ti,ina233.yaml | 57 +++++++++++++++++++ > 1 file changed, 57 insertions(+) > create mode 100644 Documentation/devicetree/bindings/hwmon/pmbus/ti,ina233.yaml > > diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/ti,ina233.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/ti,ina233.yaml > new file mode 100644 > index 000000000000..2677c98dadd1 > --- /dev/null > +++ b/Documentation/devicetree/bindings/hwmon/pmbus/ti,ina233.yaml > @@ -0,0 +1,57 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > + Drop blank line > +$id: http://devicetree.org/schemas/hwmon/pmbus/ti,ina233.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Texas Instruments INA233 of power/voltage/current monitors > + > +maintainers: > + - Leo Yang <Leo-Yang@quantatw.com> > + > +description: | > + INA233 High-Side or Low-Side Measurement, Bidirectional Current > + and Power Monitor With I2C-, SMBus-, and PMBus-Compatible Interface. > + > + Datasheet: https://www.ti.com/lit/ds/symlink/ina233.pdf > + > +properties: > + compatible: > + enum: > + - ti,ina233 This does not fit existing ti,ina bindings? > + > + reg: > + maxItems: 1 > + > + shunt-resistor: > + description: > + Shunt resistor value in micro-Ohms, Please refer to the actual circuit > + resistor used. > + default: 2000 Which schema brings the $ref for this property? > + > + current-lsb: > + description: > + Calculate the Maximum Expected Current(A) / 2^15 in micro ampere (uA/bit). > + e.g. 30A / 2^15 = 915 uA/bit > + default: 1000 Missing ref, missing vendor prefix and missing actual explanation what it is for. I don't understand the description at all. > + > +required: > + - compatible > + - reg > + > +additionalProperties: false > + > +examples: > + - | > + i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + > + power-sensor@40 { > + compatible = "ti,ina233"; > + reg = <0x40>; > + shunt-resistor = /bits/ 32 <5000>; Drop unusual syntax. Please take a look how all other bindings and DTS is doing this. > + current-lsb = /bits/ 16 <1000>; > + }; > + }; > \ No newline at end of file You have patch errors. Best regards, Krzysztof
On Mon, 06 Jan 2025 15:13:36 +0800, Leo Yang wrote: > Add TI INA233 Current and Power Monitor bindings. > > Signed-off-by: Leo Yang <Leo-Yang@quantatw.com> > --- > .../bindings/hwmon/pmbus/ti,ina233.yaml | 57 +++++++++++++++++++ > 1 file changed, 57 insertions(+) > create mode 100644 Documentation/devicetree/bindings/hwmon/pmbus/ti,ina233.yaml > My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: ./Documentation/devicetree/bindings/hwmon/pmbus/ti,ina233.yaml:35:1: [error] syntax error: found character '\t' that cannot start any token (syntax) dtschema/dtc warnings/errors: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/hwmon/pmbus/ti,ina233.yaml: ignoring, error parsing file ./Documentation/devicetree/bindings/hwmon/pmbus/ti,ina233.yaml:35:1: found character that cannot start any token make[2]: *** Deleting file 'Documentation/devicetree/bindings/hwmon/pmbus/ti,ina233.example.dts' Documentation/devicetree/bindings/hwmon/pmbus/ti,ina233.yaml:35:1: found character that cannot start any token make[2]: *** [Documentation/devicetree/bindings/Makefile:26: Documentation/devicetree/bindings/hwmon/pmbus/ti,ina233.example.dts] Error 1 make[2]: *** Waiting for unfinished jobs.... make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1506: dt_binding_check] Error 2 make: *** [Makefile:251: __sub-make] Error 2 doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250106071337.3017926-2-Leo-Yang@quantatw.com The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
On 1/6/25 02:50, Krzysztof Kozlowski wrote: > On 06/01/2025 08:13, Leo Yang wrote: >> Add TI INA233 Current and Power Monitor bindings. > > Please use subject prefixes matching the subsystem. You can get them for > example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory > your patch is touching. For bindings, the preferred subjects are > explained here: > https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters > >> >> Signed-off-by: Leo Yang <Leo-Yang@quantatw.com> >> --- >> .../bindings/hwmon/pmbus/ti,ina233.yaml | 57 +++++++++++++++++++ >> 1 file changed, 57 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/hwmon/pmbus/ti,ina233.yaml >> >> diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/ti,ina233.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/ti,ina233.yaml >> new file mode 100644 >> index 000000000000..2677c98dadd1 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/hwmon/pmbus/ti,ina233.yaml >> @@ -0,0 +1,57 @@ >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> + > > Drop blank line > >> +$id: http://devicetree.org/schemas/hwmon/pmbus/ti,ina233.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Texas Instruments INA233 of power/voltage/current monitors >> + >> +maintainers: >> + - Leo Yang <Leo-Yang@quantatw.com> >> + >> +description: | >> + INA233 High-Side or Low-Side Measurement, Bidirectional Current >> + and Power Monitor With I2C-, SMBus-, and PMBus-Compatible Interface. >> + >> + Datasheet: https://www.ti.com/lit/ds/symlink/ina233.pdf >> + >> +properties: >> + compatible: >> + enum: >> + - ti,ina233 > > This does not fit existing ti,ina bindings? > >> + >> + reg: >> + maxItems: 1 >> + >> + shunt-resistor: Should probably be shunt-resistor-micro-ohms. >> + description: >> + Shunt resistor value in micro-Ohms, Please refer to the actual circuit >> + resistor used. >> + default: 2000 > > Which schema brings the $ref for this property? > >> + >> + current-lsb: >> + description: >> + Calculate the Maximum Expected Current(A) / 2^15 in micro ampere (uA/bit). >> + e.g. 30A / 2^15 = 915 uA/bit >> + default: 1000 > > Missing ref, missing vendor prefix and missing actual explanation what > it is for. I don't understand the description at all. > Also, the unit is missing. >> + >> +required: >> + - compatible >> + - reg >> + >> +additionalProperties: false >> + >> +examples: >> + - | >> + i2c { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + power-sensor@40 { >> + compatible = "ti,ina233"; >> + reg = <0x40>; >> + shunt-resistor = /bits/ 32 <5000>; > > Drop unusual syntax. Please take a look how all other bindings and DTS > is doing this. > >> + current-lsb = /bits/ 16 <1000>; >> + }; >> + }; >> \ No newline at end of file > > You have patch errors. > > Best regards, > Krzysztof >
diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/ti,ina233.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/ti,ina233.yaml new file mode 100644 index 000000000000..2677c98dadd1 --- /dev/null +++ b/Documentation/devicetree/bindings/hwmon/pmbus/ti,ina233.yaml @@ -0,0 +1,57 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- + +$id: http://devicetree.org/schemas/hwmon/pmbus/ti,ina233.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Texas Instruments INA233 of power/voltage/current monitors + +maintainers: + - Leo Yang <Leo-Yang@quantatw.com> + +description: | + INA233 High-Side or Low-Side Measurement, Bidirectional Current + and Power Monitor With I2C-, SMBus-, and PMBus-Compatible Interface. + + Datasheet: https://www.ti.com/lit/ds/symlink/ina233.pdf + +properties: + compatible: + enum: + - ti,ina233 + + reg: + maxItems: 1 + + shunt-resistor: + description: + Shunt resistor value in micro-Ohms, Please refer to the actual circuit + resistor used. + default: 2000 + + current-lsb: + description: + Calculate the Maximum Expected Current(A) / 2^15 in micro ampere (uA/bit). + e.g. 30A / 2^15 = 915 uA/bit + default: 1000 + +required: + - compatible + - reg + +additionalProperties: false + +examples: + - | + i2c { + #address-cells = <1>; + #size-cells = <0>; + + power-sensor@40 { + compatible = "ti,ina233"; + reg = <0x40>; + shunt-resistor = /bits/ 32 <5000>; + current-lsb = /bits/ 16 <1000>; + }; + }; \ No newline at end of file
Add TI INA233 Current and Power Monitor bindings. Signed-off-by: Leo Yang <Leo-Yang@quantatw.com> --- .../bindings/hwmon/pmbus/ti,ina233.yaml | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwmon/pmbus/ti,ina233.yaml