| Message ID | 20220623165217.59252-3-michaelsh@nvidia.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add support for EMC2305 Fan Speed Controller. | expand |
On Thu, Jun 23, 2022 at 07:52:16PM +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> > --- > v2->v3 > Changes pointed out by Rob Herring and Guenter Roeck: > - Describe separate channels of fan-controller; > - Remove pwm_max property; > - Fix compatible property. > Changes added by Michael Shych: > - Fix dt binding check warnings. > 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 | 106 +++++++++++++++++++++ > 1 file changed, 106 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..d054ba46ae23 > --- /dev/null > +++ b/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml > @@ -0,0 +1,106 @@ > +# 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 RPM-based? So there is a tach signal too? Don't those need the number of pulses per revolution that the fan provides. To repeat what I say for every fan controller binding now, until there's a common binding to describe fan controllers, fans and their relationship to each other, I'm not signing off on any fan binding doing its own thing. Rob
Hi, Sorry for long delay in getting back to you. Please see below. Thanks, Michael. > -----Original Message----- > From: Rob Herring <robh@kernel.org> > Sent: Friday, July 1, 2022 1:12 AM > To: Michael Shych <michaelsh@nvidia.com> > Cc: linux@roeck-us.net; linux-hwmon@vger.kernel.org; > devicetree@vger.kernel.org; Vadim Pasternak <vadimp@nvidia.com> > Subject: Re: [PATCH hwmon-next v4 2/3] dt-bindings: hwmon: add Microchip > EMC2305 fan controller. > > On Thu, Jun 23, 2022 at 07:52:16PM +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> > > --- > > v2->v3 > > Changes pointed out by Rob Herring and Guenter Roeck: > > - Describe separate channels of fan-controller; > > - Remove pwm_max property; > > - Fix compatible property. > > Changes added by Michael Shych: > > - Fix dt binding check warnings. > > 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 | 106 > +++++++++++++++++++++ > > 1 file changed, 106 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..d054ba46ae23 > > --- /dev/null > > +++ > b/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml > > @@ -0,0 +1,106 @@ > > +# 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 > > RPM-based? So there is a tach signal too? Don't those need the number of > pulses per revolution that the fan provides. > It's not relevant. The driver implements Direct setting mode according to the Datasheet: https://www.microchip.com/en-us/product/EMC2305 I can add this note to the documentation patch. > To repeat what I say for every fan controller binding now, until there's a > common binding to describe fan controllers, fans and their relationship to > each other, I'm not signing off on any fan binding doing its own thing. > I'm confused here as I thought that I already changed to common fan-controller with advice of Gunter in patch series V3. Do you mean that we should use some common FAN/PWM/ TACHO etc. generic binding mechanism that fits all drivers? Could you advise if it's already existed and reference to example? If it doesn't exist, it'll be too complicated to provide such a new generic description within the submission of a driver for EMC2305 device. We can just completely remove OF interface and pass the necessary configuration through the platform data. > Rob
On Mon, Jul 18, 2022 at 12:38:58PM +0000, Michael Shych wrote: > Hi, > > Sorry for long delay in getting back to you. > Please see below. > > Thanks, > Michael. > > > -----Original Message----- > > From: Rob Herring <robh@kernel.org> > > Sent: Friday, July 1, 2022 1:12 AM > > To: Michael Shych <michaelsh@nvidia.com> > > Cc: linux@roeck-us.net; linux-hwmon@vger.kernel.org; > > devicetree@vger.kernel.org; Vadim Pasternak <vadimp@nvidia.com> > > Subject: Re: [PATCH hwmon-next v4 2/3] dt-bindings: hwmon: add Microchip > > EMC2305 fan controller. > > > > On Thu, Jun 23, 2022 at 07:52:16PM +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> > > > --- > > > v2->v3 > > > Changes pointed out by Rob Herring and Guenter Roeck: > > > - Describe separate channels of fan-controller; > > > - Remove pwm_max property; > > > - Fix compatible property. > > > Changes added by Michael Shych: > > > - Fix dt binding check warnings. > > > 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 | 106 > > +++++++++++++++++++++ > > > 1 file changed, 106 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..d054ba46ae23 > > > --- /dev/null > > > +++ > > b/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml > > > @@ -0,0 +1,106 @@ > > > +# 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 > > > > RPM-based? So there is a tach signal too? Don't those need the number of > > pulses per revolution that the fan provides. > > > It's not relevant. The driver implements Direct setting mode according > to the Datasheet: https://www.microchip.com/en-us/product/EMC2305 > I can add this note to the documentation patch. For the binding, it doesn't matter what a driver currently implements. That's one driver at one point in time. Bindings shouldn't be evolving and need to support more than 1 OS. The binding needs to be able to describe the h/w. If there's a tach connection, you need to describe that and the properties of the fan's tach pulses. > > > To repeat what I say for every fan controller binding now, until there's a > > common binding to describe fan controllers, fans and their relationship to > > each other, I'm not signing off on any fan binding doing its own thing. > > > I'm confused here as I thought that I already changed to common fan-controller with advice of > Gunter in patch series V3. > Do you mean that we should use some common FAN/PWM/ TACHO etc. generic binding > mechanism that fits all drivers? > Could you advise if it's already existed and reference to example? It doesn't exist. Probably the closest binding to something as a basis for something common is npcm750-pwm-fan.txt. > If it doesn't exist, it'll be too complicated to provide such a new generic description within > the submission of a driver for EMC2305 device. > We can just completely remove OF interface and pass the necessary configuration through > the platform data. That's your choice... Rob
diff --git a/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml b/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml new file mode 100644 index 000000000000..d054ba46ae23 --- /dev/null +++ b/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml @@ -0,0 +1,106 @@ +# 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 + +additionalProperties: false + +properties: + compatible: + enum: + - microchip,emc2305 + - microchip,emc2303 + - microchip,emc2302 + - microchip,emc2301 + + reg: + maxItems: 1 + + "#address-cells": + const: 1 + + "#size-cells": + const: 0 + + microchip,cooling-levels: + description: + Quantity of cooling level state. + $ref: /schemas/types.yaml#/definitions/uint8 + minimum: 0 + maximum: 255 + + microchip,pwm-separate: + description: + This flag indicates that separate PWM setting will be used + for different channels. + $ref: /schemas/types.yaml#/definitions/flag + +patternProperties: + "^channel@[0-4]$": + type: object + + additionalProperties: false + + properties: + reg: + items: + - enum: + - 0 + - 1 + - 2 + - 3 + - 4 + + pwm-min: + description: + Min pwm of emc2305 channel + $ref: /schemas/types.yaml#/definitions/uint8 + minimum: 0 + maximum: 255 + + required: + - reg + +required: + - compatible + + +examples: + - | + i2c { + #address-cells = <1>; + #size-cells = <0>; + + fan-controller@4d { + compatible = "microchip,emc2305"; + reg = <0x4d>; + #address-cells = <1>; + #size-cells = <0>; + microchip,pwm-separate; + microchip,cooling-levels = /bits/ 8 <10>; + + channel@0 { + reg = <0>; + pwm-min = /bits/ 8 <0>; + }; + channel@1 { + reg = <1>; + pwm-min = /bits/ 8 <0>; + }; + }; + }; +