RE: [PATCH hwmon-next v4 2/3] dt-bindings: hwmon: add Microchip EMC2305 fan controller.

[Michael Shych <michaelsh@nvidia.com>]

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