| Message ID | 20220427010154.29749-2-zev@bewilderbeest.net (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | hwmon: (nct6775) Convert to regmap, add i2c support | expand |
On 27/04/2022 03:01, Zev Weiss wrote: > These Super I/O chips have an i2c interface that some systems expose > to a BMC; the BMC's device tree can now describe that via this > binding. > > Signed-off-by: Zev Weiss <zev@bewilderbeest.net> I already reviewed it so I guess you did not include the tag because of significant changes? > --- > .../bindings/hwmon/nuvoton,nct6775.yaml | 56 +++++++++++++++++++ > 1 file changed, 56 insertions(+) > create mode 100644 Documentation/devicetree/bindings/hwmon/nuvoton,nct6775.yaml > > diff --git a/Documentation/devicetree/bindings/hwmon/nuvoton,nct6775.yaml b/Documentation/devicetree/bindings/hwmon/nuvoton,nct6775.yaml > new file mode 100644 > index 000000000000..418477374fdb > --- /dev/null > +++ b/Documentation/devicetree/bindings/hwmon/nuvoton,nct6775.yaml > @@ -0,0 +1,56 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > + > +$id: http://devicetree.org/schemas/hwmon/nuvoton,nct6775.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Nuvoton NCT6775 and compatible Super I/O chips > + > +maintainers: > + - Zev Weiss <zev@bewilderbeest.net> > + > +properties: > + compatible: > + enum: > + - nuvoton,nct6106 > + - nuvoton,nct6116 > + - nuvoton,nct6775 > + - nuvoton,nct6776 > + - nuvoton,nct6779 > + - nuvoton,nct6791 > + - nuvoton,nct6792 > + - nuvoton,nct6793 > + - nuvoton,nct6795 > + - nuvoton,nct6796 > + - nuvoton,nct6797 > + - nuvoton,nct6798 > + > + reg: > + maxItems: 1 > + > + nuvoton,tsi-channel-mask: > + description: > + Bitmask indicating which TSI temperature sensor channels are > + active. LSB is TSI0, bit 1 is TSI1, etc. Need a type/ref. > + maximum: 0xff > + default: 0 Since by default it is disabled, doesn't it make a required property? IOW, if you add a node without this mask, will the device operate properly and usefully? Best regards, Krzysztof
On Tue, Apr 26, 2022 at 11:04:30PM PDT, Krzysztof Kozlowski wrote: >On 27/04/2022 03:01, Zev Weiss wrote: >> These Super I/O chips have an i2c interface that some systems expose >> to a BMC; the BMC's device tree can now describe that via this >> binding. >> >> Signed-off-by: Zev Weiss <zev@bewilderbeest.net> > >I already reviewed it so I guess you did not include the tag because of >significant changes? > Yeah, the nuvoton,tsi-channel-mask property is new this round, so I dropped the previous R-B -- and since it looks like I missed some stuff, thanks for taking another look (though perhaps some of it could have been avoided if I'd remembered to run 'make dt_binding_check'). >> --- >> .../bindings/hwmon/nuvoton,nct6775.yaml | 56 +++++++++++++++++++ >> 1 file changed, 56 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/hwmon/nuvoton,nct6775.yaml >> >> diff --git a/Documentation/devicetree/bindings/hwmon/nuvoton,nct6775.yaml b/Documentation/devicetree/bindings/hwmon/nuvoton,nct6775.yaml >> new file mode 100644 >> index 000000000000..418477374fdb >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/hwmon/nuvoton,nct6775.yaml >> @@ -0,0 +1,56 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> + >> +$id: http://devicetree.org/schemas/hwmon/nuvoton,nct6775.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Nuvoton NCT6775 and compatible Super I/O chips >> + >> +maintainers: >> + - Zev Weiss <zev@bewilderbeest.net> >> + >> +properties: >> + compatible: >> + enum: >> + - nuvoton,nct6106 >> + - nuvoton,nct6116 >> + - nuvoton,nct6775 >> + - nuvoton,nct6776 >> + - nuvoton,nct6779 >> + - nuvoton,nct6791 >> + - nuvoton,nct6792 >> + - nuvoton,nct6793 >> + - nuvoton,nct6795 >> + - nuvoton,nct6796 >> + - nuvoton,nct6797 >> + - nuvoton,nct6798 >> + >> + reg: >> + maxItems: 1 >> + >> + nuvoton,tsi-channel-mask: >> + description: >> + Bitmask indicating which TSI temperature sensor channels are >> + active. LSB is TSI0, bit 1 is TSI1, etc. > >Need a type/ref. > Ack, thanks. >> + maximum: 0xff >> + default: 0 > >Since by default it is disabled, doesn't it make a required property? >IOW, if you add a node without this mask, will the device operate >properly and usefully? > Yeah, zero active TSI channels is a totally legitimate way for these devices to operate. TSI is just an optional source of additional temperature readings that's used on some (AMD) systems; all the basic Super I/O functionality works fine without it. Thanks, Zev
On 27/04/2022 08:44, Zev Weiss wrote: > >>> + maximum: 0xff >>> + default: 0 >> >> Since by default it is disabled, doesn't it make a required property? >> IOW, if you add a node without this mask, will the device operate >> properly and usefully? >> > > Yeah, zero active TSI channels is a totally legitimate way for these > devices to operate. TSI is just an optional source of additional > temperature readings that's used on some (AMD) systems; all the basic > Super I/O functionality works fine without it. OK, thanks. Best regards, Krzysztof
On Tue, 26 Apr 2022 18:01:48 -0700, Zev Weiss wrote: > These Super I/O chips have an i2c interface that some systems expose > to a BMC; the BMC's device tree can now describe that via this > binding. > > Signed-off-by: Zev Weiss <zev@bewilderbeest.net> > --- > .../bindings/hwmon/nuvoton,nct6775.yaml | 56 +++++++++++++++++++ > 1 file changed, 56 insertions(+) > create mode 100644 Documentation/devicetree/bindings/hwmon/nuvoton,nct6775.yaml > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/hwmon/nuvoton,nct6775.yaml: properties:nuvoton,tsi-channel-mask: 'oneOf' conditional failed, one must be fixed: 'type' is a required property hint: A vendor boolean property can use "type: boolean" Additional properties are not allowed ('default', 'maximum' were unexpected) hint: A vendor boolean property can use "type: boolean" /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/hwmon/nuvoton,nct6775.yaml: properties:nuvoton,tsi-channel-mask: 'oneOf' conditional failed, one must be fixed: 'enum' is a required property 'const' is a required property hint: A vendor string property with exact values has an implicit type from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml# /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/hwmon/nuvoton,nct6775.yaml: properties:nuvoton,tsi-channel-mask: 'oneOf' conditional failed, one must be fixed: '$ref' is a required property 'allOf' is a required property hint: A vendor property needs a $ref to types.yaml from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml# hint: Vendor specific properties must have a type and description unless they have a defined, common suffix. from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml# /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/hwmon/nuvoton,nct6775.yaml: ignoring, error in schema: properties: nuvoton,tsi-channel-mask Documentation/devicetree/bindings/hwmon/nuvoton,nct6775.example.dtb:0:0: /example-0/i2c/superio@4d: failed to match any schema with compatible: ['nuvoton,nct6779'] doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/patch/ This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1. 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.
On 27/04/2022 08:44, Zev Weiss wrote: >>> + reg: >>> + maxItems: 1 >>> + >>> + nuvoton,tsi-channel-mask: >>> + description: >>> + Bitmask indicating which TSI temperature sensor channels are >>> + active. LSB is TSI0, bit 1 is TSI1, etc. >> >> Need a type/ref. >> > > Ack, thanks. Did you test the bindings after the changes? Using reviewers time instead of testing by yourself with an automated tool is quite a waste. Best regards, Krzysztof
On Wed, Apr 27, 2022 at 09:37:20AM PDT, Krzysztof Kozlowski wrote: >On 27/04/2022 08:44, Zev Weiss wrote: >>>> + reg: >>>> + maxItems: 1 >>>> + >>>> + nuvoton,tsi-channel-mask: >>>> + description: >>>> + Bitmask indicating which TSI temperature sensor channels are >>>> + active. LSB is TSI0, bit 1 is TSI1, etc. >>> >>> Need a type/ref. >>> >> >> Ack, thanks. > >Did you test the bindings after the changes? Using reviewers time >instead of testing by yourself with an automated tool is quite a waste. > Yeah, sorry about that -- with uint32 $ref added it passes dt_binding_check; I'll re-send with that change. Zev
diff --git a/Documentation/devicetree/bindings/hwmon/nuvoton,nct6775.yaml b/Documentation/devicetree/bindings/hwmon/nuvoton,nct6775.yaml new file mode 100644 index 000000000000..418477374fdb --- /dev/null +++ b/Documentation/devicetree/bindings/hwmon/nuvoton,nct6775.yaml @@ -0,0 +1,56 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- + +$id: http://devicetree.org/schemas/hwmon/nuvoton,nct6775.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Nuvoton NCT6775 and compatible Super I/O chips + +maintainers: + - Zev Weiss <zev@bewilderbeest.net> + +properties: + compatible: + enum: + - nuvoton,nct6106 + - nuvoton,nct6116 + - nuvoton,nct6775 + - nuvoton,nct6776 + - nuvoton,nct6779 + - nuvoton,nct6791 + - nuvoton,nct6792 + - nuvoton,nct6793 + - nuvoton,nct6795 + - nuvoton,nct6796 + - nuvoton,nct6797 + - nuvoton,nct6798 + + reg: + maxItems: 1 + + nuvoton,tsi-channel-mask: + description: + Bitmask indicating which TSI temperature sensor channels are + active. LSB is TSI0, bit 1 is TSI1, etc. + maximum: 0xff + default: 0 + +required: + - compatible + - reg + +additionalProperties: false + +examples: + - | + i2c { + #address-cells = <1>; + #size-cells = <0>; + + superio@4d { + compatible = "nuvoton,nct6779"; + reg = <0x4d>; + nuvoton,tsi-channel-mask = <0x03>; + }; + };
These Super I/O chips have an i2c interface that some systems expose to a BMC; the BMC's device tree can now describe that via this binding. Signed-off-by: Zev Weiss <zev@bewilderbeest.net> --- .../bindings/hwmon/nuvoton,nct6775.yaml | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwmon/nuvoton,nct6775.yaml