On 19/02/2025 14:32, florin.leotescu@xxxxxxxxxxx wrote: > From: Florin Leotescu <florin.leotescu@xxxxxxx> > A nit, subject: drop second/last, redundant "YAML binding documentation". Three useless/redundant terms. The "dt-bindings" prefix is already stating that these are bindings. See also: https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18 And drop all driver references - you put it everywhere. > Add yaml-based Device Tree bindings documentation for emc2305 driver > The file provides the necessary structure, configuration > and other parameters for Device Tree definition. > > Signed-off-by: Florin Leotescu <florin.leotescu@xxxxxxx> > --- > .../devicetree/bindings/hwmon/emc2305.yaml | 95 +++++++++++++++++++ Filename matching compatible. > 1 file changed, 95 insertions(+) > create mode 100644 Documentation/devicetree/bindings/hwmon/emc2305.yaml > > diff --git a/Documentation/devicetree/bindings/hwmon/emc2305.yaml b/Documentation/devicetree/bindings/hwmon/emc2305.yaml > new file mode 100644 > index 000000000000..51e2a82d8f25 > --- /dev/null > +++ b/Documentation/devicetree/bindings/hwmon/emc2305.yaml > @@ -0,0 +1,95 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/hwmon/emc2305.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: EMC2305 i2c pwm fan controller > + > +maintainers: > + - Michael Shych <michaelsh@xxxxxxxxxx> > + > +description: | > + The driver implements support for Microchip EMC2301/2/3/5 PWM Fan Controller. This is a binding so describe hardware, not your implementation. > + The EMC2301 Fan Controller supports only one controlled PWM fan channel. > + The EMC2305 Fan Controller supports up to 5 independently > + controlled PWM fan drives. > + Missing fan-common reference. > +properties: > + compatible: > + enum: > + - hwmon,emc2301 > + - hwmon,emc2302 > + - hwmon,emc2303 > + - hwmon,emc2305 Nope. Was it ever internally reviewed? > + > + reg: > + description: I2C address of the emc2305 device Look how other bindings do it. > + > + pwm_output: NAK. There are so many issues with this code, from trivial incorrect quotes and not following DTS coding style, to actual duplicating other schemas or common part. Sorry, get it first internally reviewed. > + description: "PWM output type Push-Pull/ Open Drain" > + maxItems: 1 > + > + pwm_polarity: > + description: "PWM polarity" > + maxItems: 1 > + > + '#cooling-cells': > + description: "cooling state range" Do you see any binding like that? Any? > + > +required: > + - compatible > + - reg > + > +additionalProperties: false > + > +examples: > + - | > + thermal_zones { > + a55-thermal { > + trips { > + atrip0: trip0 { > + temperature = <55000>; > + hysteresis = <2000>; > + type = "active"; > + }; > + > + atrip1: trip1 { > + temperature = <65000>; > + hysteresis = <2000>; > + type = "active"; > + }; > + > + atrip2: trip2 { > + temperature = <75000>; > + hysteresis = <2000>; > + type = "active"; > + }; > + }; > + > + cooling-maps { > + map1 { > + trip = <&atrip0>; > + cooling-device = <&emc2301 4 6>; > + }; > + > + map2 { > + trip = <&atrip1>; > + cooling-device = <&emc2301 6 8>; > + }; > + > + map3 { > + trip = <&atrip2>; > + cooling-device = <&emc2301 8 10>; > + }; > + }; > + }; > + > + } This DTS is also in very poor shape - even your indentation does not match. Drop redundant parts - entire thermal. > + emc2301: pwm@2f { > + compatible = "hwmon,emc2301"; > + reg = <0x2f>; > + #cooling-cells = <2>; > + pwm_output = <0x1>; > + pwm_polarity = <0x1>; > + }; Best regards, Krzysztof
