| Message ID | 20220524161755.43833-3-michaelsh@nvidia.com |
|---|---|
| State | New |
| Headers | show |
| Series | [hwmon-next,v2,1/3] hwmon: (emc2305) add support for EMC2301/2/3/5 RPM-based PWM Fan Speed Controller. | expand |
On Tue, 24 May 2022 19:17:54 +0300, michaelsh@nvidia.com wrote: > From: Michael Shych <michaelsh@nvidia.com> > > Add basic description of emc2305 driver device tree binding. > > Signed-off-by: Michael Shych <michaelsh@nvidia.com> > Reviewed-by: Vadim Pasternak <vadimp@nvidia.com> > --- > v1->v2 > - Fix dt binding check errors; > - Add descriptions; > - Add missing fields; > - Change the patch subject name; > - Separate pwm-min, pwm-max per PWM channel. > --- > .../bindings/hwmon/microchip,emc2305.yaml | 87 ++++++++++++++++++++++ > 1 file changed, 87 insertions(+) > create mode 100644 Documentation/devicetree/bindings/hwmon/microchip,emc2305.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: Documentation/devicetree/bindings/hwmon/microchip,emc2305.example.dts:23.23-26.19: Warning (unit_address_vs_reg): /example-0/fan/microchip,pwms/pwm@0: node has a unit name, but no reg or ranges property Documentation/devicetree/bindings/hwmon/microchip,emc2305.example.dts:27.23-30.19: Warning (unit_address_vs_reg): /example-0/fan/microchip,pwms/pwm@1: node has a unit name, but no reg or ranges property 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 5/25/22 19:11, Rob Herring wrote: > On Tue, May 24, 2022 at 07:17:54PM +0300, michaelsh@nvidia.com wrote: >> From: Michael Shych <michaelsh@nvidia.com> >> >> Add basic description of emc2305 driver device tree binding. >> >> Signed-off-by: Michael Shych <michaelsh@nvidia.com> >> Reviewed-by: Vadim Pasternak <vadimp@nvidia.com> >> --- >> v1->v2 >> - Fix dt binding check errors; >> - Add descriptions; >> - Add missing fields; >> - Change the patch subject name; >> - Separate pwm-min, pwm-max per PWM channel. >> --- >> .../bindings/hwmon/microchip,emc2305.yaml | 87 ++++++++++++++++++++++ >> 1 file changed, 87 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml >> >> diff --git a/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml b/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml >> new file mode 100644 >> index 000000000000..0aa65dcfd238 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml >> @@ -0,0 +1,87 @@ >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> + >> +$id: http://devicetree.org/schemas/hwmon/microchip,emc2305.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Microchip EMC2305 RPM-based PWM Fan Speed Controller >> + >> +maintainers: >> + - Michael Shych <michaelsh@nvidia.com> >> + >> +description: | >> + Microchip EMC2301/2/3/5 are RPM-based PWM Fan Controller. >> + The Fan Controller supports up to 5 independently controlled PWM fan drives. >> + Fan rotation speeds are reported in RPM. >> + >> + Datasheet: https://www.microchip.com/en-us/product/EMC2305 >> + >> +properties: >> + compatible: >> + enum: >> + - microcip,emc2305 >> + - microcip,emc2303 >> + - microcip,emc2302 >> + - microcip,emc2301 > > typo. > >> + >> + microchip,pwm-channel: >> + description: >> + Max number of pwm channels. >> + Zero index is used in case of one common PWM setting. >> + Number of used separted pwm channels can be less than >> + maximum number of available pwm chnnels on this FAN controller. >> + $ref: /schemas/types.yaml#/definitions/uint8 >> + items: >> + minimum: 0 >> + maximum: 5 >> + default: 0 >> + maxItems: 1 >> + >> + microchip,cooling-levels: >> + description: >> + Quantity of cooling level state. >> + $ref: /schemas/types.yaml#/definitions/uint8 >> + items: >> + minimum: 0 >> + maximum: 10 >> + default: 10 >> + maxItems: 1 >> + >> + microchip,pwm-min: >> + description: >> + Min pwm of emc2305 >> + $ref: /schemas/types.yaml#/definitions/uint8 >> + default: 0 >> + maxItems: 1 >> + >> + microchip,pwm-max: >> + description: >> + Max pwm of emc2305 >> + $ref: /schemas/types.yaml#/definitions/uint8 >> + default: 255 >> + maxItems: 1 >> + >> +required: >> + - compatible >> + >> +additionalProperties: false >> + >> +examples: >> + - | >> + fan { >> + microchip,compatible = "microchip,emc2305"; > > microchip,compatible?? Again... > > The bigger problem remains of this needing a common fan and fan > controller bindings. > I am more concerned that the bindings don't really reflect the fan controller but thermal device configuration. Shouldn't that be handled with thermal subsystem bindings ? At the same time, there are no bindings suggested for the actual pwm controller configuration (such as pwm inversion, pwm frequency, or pwm output type), and there is nothing at all for the fan controller either. I can understand that this is maybe not needed for the given application (it seems to assume manual fan control driven by the thermal subsystem), but the bindings have to take into account that there are other applications. Also, the number of available pwm/fan control channels is determined by the chip. If some of the channels are unused, that could be handled with status properties for the individual channels. I don't see a need for a "pwm-channel" property. Similar, the maximum pwm value is determined by the chip; I don't see the point of the microchip,pwm-max property. Again, this is a thermal subsystem property, and I would hope that the range can be set with thermal subsystem properties. Something like the following might do. fan-controller@1a { compatible = "microchip,emc2305"; #address-cells = <1>; #size-cells = <0>; <chip properties, eg external clock> channel@0 { reg = <0x0>; pwm-frequency = < xxx >; pwm-output-type = <0 | 1>; // for open drain / push-pull pwm-inverted; // if pwm output is inverted microchip,closed-loop; // for closed loop fan control fan-pulses = < 1..4 >; ... }; channel@1 { ... }; channel@4 { ... status = "disabled"; // if unused }; ... }; Thanks, Guenter > Rob > >> + microchip,pwm-channel = <5>; >> + microchip,cooling-levels = <10>; >> + microchip,pwms { >> + pwm@0 { >> + pwm-min = <0>; >> + pwm-max = <255>; >> + }; >> + pwm@1 { >> + pwm-min = <0>; >> + pwm-max = <255>; >> + }; >> + }; >> + }; >> + >> -- >> 2.14.1 >> >>
diff --git a/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml b/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml new file mode 100644 index 000000000000..0aa65dcfd238 --- /dev/null +++ b/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml @@ -0,0 +1,87 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- + +$id: http://devicetree.org/schemas/hwmon/microchip,emc2305.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Microchip EMC2305 RPM-based PWM Fan Speed Controller + +maintainers: + - Michael Shych <michaelsh@nvidia.com> + +description: | + Microchip EMC2301/2/3/5 are RPM-based PWM Fan Controller. + The Fan Controller supports up to 5 independently controlled PWM fan drives. + Fan rotation speeds are reported in RPM. + + Datasheet: https://www.microchip.com/en-us/product/EMC2305 + +properties: + compatible: + enum: + - microcip,emc2305 + - microcip,emc2303 + - microcip,emc2302 + - microcip,emc2301 + + microchip,pwm-channel: + description: + Max number of pwm channels. + Zero index is used in case of one common PWM setting. + Number of used separted pwm channels can be less than + maximum number of available pwm chnnels on this FAN controller. + $ref: /schemas/types.yaml#/definitions/uint8 + items: + minimum: 0 + maximum: 5 + default: 0 + maxItems: 1 + + microchip,cooling-levels: + description: + Quantity of cooling level state. + $ref: /schemas/types.yaml#/definitions/uint8 + items: + minimum: 0 + maximum: 10 + default: 10 + maxItems: 1 + + microchip,pwm-min: + description: + Min pwm of emc2305 + $ref: /schemas/types.yaml#/definitions/uint8 + default: 0 + maxItems: 1 + + microchip,pwm-max: + description: + Max pwm of emc2305 + $ref: /schemas/types.yaml#/definitions/uint8 + default: 255 + maxItems: 1 + +required: + - compatible + +additionalProperties: false + +examples: + - | + fan { + microchip,compatible = "microchip,emc2305"; + microchip,pwm-channel = <5>; + microchip,cooling-levels = <10>; + microchip,pwms { + pwm@0 { + pwm-min = <0>; + pwm-max = <255>; + }; + pwm@1 { + pwm-min = <0>; + pwm-max = <255>; + }; + }; + }; +