RE: [PATCH v1 2/3] dt-bindings: hwmon: add microchip,emc2306.yaml dt binding description.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Rob,

I fixed all dt binding check errors and changed the patch subject name
that you pointed out.
However, it's not clear completely your comment in this reply.
emc2305 device support 5 pwms.
The intention was to specify a simple configuration per pwm{n} - to allow setting per pwm{n} 
minimum and maximum duty cycle, cooling level stepping (10%, 5%, 1%).

Is there some way to provide such a configuration?
Do you think that the following example is OK?

fan {
    microchip,compatible = "microchip,emc2305";
    microchip,pwm-channel = <5>;
    microchip,cooling-levels = <10>;
	pwm1 {
		microchip,pwm-min = <0>;
		microchip,pwm-max = <255>;
	};
	pwm2 {
		microchip,pwm-min = <0>;
		microchip,pwm-max = <255>;
	};
	...
};

Regards,
    Michael.



> -----Original Message-----
> From: Rob Herring <robh@xxxxxxxxxx>
> Sent: Monday, May 2, 2022 9:17 PM
> To: Michael Shych <michaelsh@xxxxxxxxxx>
> Cc: linux@xxxxxxxxxxxx; linux-hwmon@xxxxxxxxxxxxxxx;
> devicetree@xxxxxxxxxxxxxxx; Vadim Pasternak <vadimp@xxxxxxxxxx>
> Subject: Re: [PATCH v1 2/3] dt-bindings: hwmon: add
> microchip,emc2306.yaml dt binding description.
> 
> On Sat, Apr 30, 2022 at 02:49:04PM +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>
> > ---
> >  .../bindings/hwmon/microchip,emc2305.yaml          | 55
> ++++++++++++++++++++++
> >  1 file changed, 55 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..c873172b7268
> > --- /dev/null
> > +++
> b/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml
> > @@ -0,0 +1,55 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) %YAML 1.2
> > +---
> > +
> > +$id: http://devicetree.org/schemas/hwmon/emc2305.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Microchip EMC2305 RPM-based PWM Fan Speed Controller
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - microcip,emc2305
> > +
> > +  emc2305,pwm-min:
> > +    description:
> > +      Min pwm of emc2305
> > +    maxItems: 1
> > +  emc2305,pwm-max:
> > +    description:
> > +      Max pwm of emc2305
> > +    maxItems: 1
> > +  emc2305,pwm-channel:
> > +    description:
> > +      Max number of pwm channels
> > +    maxItems: 1
> > +  emcs205,max-state:
> > +    description:
> > +    maxItems: 1
> > +  emc2305,cooling-levels:
> > +    description:
> > +      Quantity of cooling level state.
> > +    maxItems: 1
> > +
> > +required:
> > +  - compatible
> > +
> > +optional:
> > +  - emc2305,min-pwm
> > +  - emc2305,max-pwm
> > +  - emc2305,pwm-channels
> > +  - emc2305,cooling-levels
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    fan {
> > +        emc2305,compatible = "microchip,emc2305";
> 
> Err, what?
> 
> > +        emc2305,pwm-min = <0>;
> > +        emc2305,pwm-max = <255>;
> > +        emc2305,pwm-channel = <5>
> > +        emc2305,cooling-levels = <10>;
> 
> All possible fans attached to this controller are the same and don't have
> anything that needs to be described? Based on other fan controllers, I don't
> think so. As I've said multiple times, there's a need for a common fan and
> fan-controller binding. Until that happens, I'm not inclined to accept fan
> controller bindings with custom properties.
> 
> Rob




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux