Messages in this thread | | | Date | Mon, 18 Mar 2024 16:51:46 +0700 | From | Chanh Nguyen <> | Subject | Re: [PATCH 1/3] dt-bindings: hwmon: Add maxim max31790 driver bindings |
| |
On 11/03/2024 23:55, Krzysztof Kozlowski wrote: > On 11/03/2024 12:13, Chanh Nguyen wrote: >> Add a device tree bindings for max31790 device. > > Subject: drop "driver", bindings are about hardware. >
Yes, I'll drop "driver" at v2 updating.
> 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. >
I tested the binding, I didn't see any warning/error log. Please review my logs as below
=> make dt_binding_check DT_SCHEMA_FILES=/hwmon/max31790.yaml make[1]: Entering directory '/DISK4T/work/community/linux/out' DTEX Documentation/devicetree/bindings/hwmon/max31790.example.dts DTC_CHK Documentation/devicetree/bindings/hwmon/max31790.example.dtb make[1]: Leaving directory '/DISK4T/work/community/linux/out'
>> >> Signed-off-by: Chanh Nguyen <chanh@os.amperecomputing.com> >> --- >> .../devicetree/bindings/hwmon/max31790.yaml | 44 +++++++++++++++++++ >> 1 file changed, 44 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/hwmon/max31790.yaml >> >> diff --git a/Documentation/devicetree/bindings/hwmon/max31790.yaml b/Documentation/devicetree/bindings/hwmon/max31790.yaml >> new file mode 100644 >> index 000000000000..5a93e6bdebda >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/hwmon/max31790.yaml > > Filename like compatible.
Yes, I'll update that in v2
> >> @@ -0,0 +1,44 @@ >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/hwmon/max31790.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: The Maxim MAX31790 Fan Controller >> + >> +maintainers: >> + - Jean Delvare <jdelvare@suse.com> >> + - Guenter Roeck <linux@roeck-us.net> > > You should have here someone responsible for hardware, not subsystem > maintainers. >
Hi Krzysztof, I checked the history of the drivers/hwmon/max31790.c and see Guenter Roeck <linux@roeck-us.net> as an important maintainer. I saw many commits from him. So, I add him to maintainer list.
>> + >> +description: > >> + The MAX31790 controls the speeds of up to six fans using six >> + independent PWM outputs. The desired fan speeds (or PWM duty cycles) >> + are written through the I2C interface. >> + >> + Datasheets: >> + https://datasheets.maximintegrated.com/en/ds/MAX31790.pdf >> + >> +properties: >> + compatible: >> + const: maxim,max31790 >> + >> + reg: >> + maxItems: 1 > > That's weirdly empty. >
Hi Krzysztof, I have not yet understood your comment here. Please help give more details for my missing! Thank Krzysztof! >> + >> +required: >> + - compatible >> + - reg >> + > > You miss allOf: with $ref to fan controller schema. >
Thank Krzysztof, I'll add the allOf at v2.
>> +additionalProperties: false >> + >> +examples: >> + - | >> + i2c { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + max31790@20 { > > 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 >
I suggest some node names, such as "i2c-fan" or "fan-controller" . Can you please share your ideas with me!
> > Best regards, > Krzysztof >
| |