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@xxxxxxxxxx> > > Sent: Friday, July 1, 2022 1:12 AM > > To: Michael Shych <michaelsh@xxxxxxxxxx> > > Cc: linux@xxxxxxxxxxxx; linux-hwmon@xxxxxxxxxxxxxxx; > > devicetree@xxxxxxxxxxxxxxx; Vadim Pasternak <vadimp@xxxxxxxxxx> > > 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@xxxxxxxxxx wrote: > > > From: Michael Shych <michaelsh@xxxxxxxxxx> > > > > > > Add basic description of emc2305 driver device tree binding. > > > > > > Signed-off-by: Michael Shych <michaelsh@xxxxxxxxxx> > > > Reviewed-by: Vadim Pasternak <vadimp@xxxxxxxxxx> > > > --- > > > 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
