| Message ID | 1548860827-29796-2-git-send-email-stefan.wahren@i2se.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | hwmon: pwm-fan: Add RPM support | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success |
On Wed, Jan 30, 2019 at 04:07:05PM +0100, Stefan Wahren wrote: > This adds the tachometer interrupt to the pwm-fan binding, which is > necessary for RPM support. > > Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com> > --- > Documentation/devicetree/bindings/hwmon/pwm-fan.txt | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/Documentation/devicetree/bindings/hwmon/pwm-fan.txt b/Documentation/devicetree/bindings/hwmon/pwm-fan.txt > index 49ca5d8..7f69b0b 100644 > --- a/Documentation/devicetree/bindings/hwmon/pwm-fan.txt > +++ b/Documentation/devicetree/bindings/hwmon/pwm-fan.txt > @@ -8,6 +8,9 @@ Required properties: > > Optional properties: > - fan-supply : phandle to the regulator that provides power to the fan > +- interrupts : contains a single interrupt specifier which describes the > + tachometer pin output of a 2 pulse-per-revolution fan. > + See interrupt-controller/interrupts.txt for the format. So a hypothetical {1,4} pulse-per-revolution fan would explicitly not be supported ? Why ? Guenter
Hi Guenter, > Guenter Roeck <linux@roeck-us.net> hat am 30. Januar 2019 um 18:28 geschrieben: > > > On Wed, Jan 30, 2019 at 04:07:05PM +0100, Stefan Wahren wrote: > > This adds the tachometer interrupt to the pwm-fan binding, which is > > necessary for RPM support. > > > > Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com> > > --- > > Documentation/devicetree/bindings/hwmon/pwm-fan.txt | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/hwmon/pwm-fan.txt b/Documentation/devicetree/bindings/hwmon/pwm-fan.txt > > index 49ca5d8..7f69b0b 100644 > > --- a/Documentation/devicetree/bindings/hwmon/pwm-fan.txt > > +++ b/Documentation/devicetree/bindings/hwmon/pwm-fan.txt > > @@ -8,6 +8,9 @@ Required properties: > > > > Optional properties: > > - fan-supply : phandle to the regulator that provides power to the fan > > +- interrupts : contains a single interrupt specifier which describes the > > + tachometer pin output of a 2 pulse-per-revolution fan. > > + See interrupt-controller/interrupts.txt for the format. > > So a hypothetical {1,4} pulse-per-revolution fan would explicitly not be > supported ? Why ? i could add an additional property to specify the pulse per revolution and use the 2 as default (according to the Intel spec for 4 pin pwm fan) which should fit in most cases. > > Guenter
On Wed, Jan 30, 2019 at 09:23:31PM +0100, Stefan Wahren wrote: > Hi Guenter, > > > Guenter Roeck <linux@roeck-us.net> hat am 30. Januar 2019 um 18:28 geschrieben: > > > > > > On Wed, Jan 30, 2019 at 04:07:05PM +0100, Stefan Wahren wrote: > > > This adds the tachometer interrupt to the pwm-fan binding, which is > > > necessary for RPM support. > > > > > > Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com> > > > --- > > > Documentation/devicetree/bindings/hwmon/pwm-fan.txt | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/Documentation/devicetree/bindings/hwmon/pwm-fan.txt b/Documentation/devicetree/bindings/hwmon/pwm-fan.txt > > > index 49ca5d8..7f69b0b 100644 > > > --- a/Documentation/devicetree/bindings/hwmon/pwm-fan.txt > > > +++ b/Documentation/devicetree/bindings/hwmon/pwm-fan.txt > > > @@ -8,6 +8,9 @@ Required properties: > > > > > > Optional properties: > > > - fan-supply : phandle to the regulator that provides power to the fan > > > +- interrupts : contains a single interrupt specifier which describes the > > > + tachometer pin output of a 2 pulse-per-revolution fan. > > > + See interrupt-controller/interrupts.txt for the format. > > > > So a hypothetical {1,4} pulse-per-revolution fan would explicitly not be > > supported ? Why ? > > i could add an additional property to specify the pulse per revolution and use the 2 as default (according to the Intel spec for 4 pin pwm fan) which should fit in most cases. > That would be a possibility and make sense, but that is not the point here. The "interrupts" property does not and should not care how many pulses per revolution the fan provides. Guenter
Hi Guenter, Am 30.01.19 um 21:35 schrieb Guenter Roeck: > On Wed, Jan 30, 2019 at 09:23:31PM +0100, Stefan Wahren wrote: >> Hi Guenter, >> >>> Guenter Roeck <linux@roeck-us.net> hat am 30. Januar 2019 um 18:28 geschrieben: >>> >>> >>> On Wed, Jan 30, 2019 at 04:07:05PM +0100, Stefan Wahren wrote: >>>> This adds the tachometer interrupt to the pwm-fan binding, which is >>>> necessary for RPM support. >>>> >>>> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com> >>>> --- >>>> Documentation/devicetree/bindings/hwmon/pwm-fan.txt | 3 +++ >>>> 1 file changed, 3 insertions(+) >>>> >>>> diff --git a/Documentation/devicetree/bindings/hwmon/pwm-fan.txt b/Documentation/devicetree/bindings/hwmon/pwm-fan.txt >>>> index 49ca5d8..7f69b0b 100644 >>>> --- a/Documentation/devicetree/bindings/hwmon/pwm-fan.txt >>>> +++ b/Documentation/devicetree/bindings/hwmon/pwm-fan.txt >>>> @@ -8,6 +8,9 @@ Required properties: >>>> >>>> Optional properties: >>>> - fan-supply : phandle to the regulator that provides power to the fan >>>> +- interrupts : contains a single interrupt specifier which describes the >>>> + tachometer pin output of a 2 pulse-per-revolution fan. >>>> + See interrupt-controller/interrupts.txt for the format. >>> So a hypothetical {1,4} pulse-per-revolution fan would explicitly not be >>> supported ? Why ? >> i could add an additional property to specify the pulse per revolution and use the 2 as default (according to the Intel spec for 4 pin pwm fan) which should fit in most cases. >> > That would be a possibility and make sense, but that is not > the point here. The "interrupts" property does not and should > not care how many pulses per revolution the fan provides. sorry, i'm not sure what's the problem about. Do you want me to use a GPIO instead of interrupt? Or is it the wording here? Suggestion: contains a single interrupt specifier which is describes the tachometer output of the fan as an input Stefan > > Guenter
On Thu, Jan 31, 2019 at 09:02:33AM +0100, Stefan Wahren wrote: > Hi Guenter, > > Am 30.01.19 um 21:35 schrieb Guenter Roeck: > > On Wed, Jan 30, 2019 at 09:23:31PM +0100, Stefan Wahren wrote: > >> Hi Guenter, > >> > >>> Guenter Roeck <linux@roeck-us.net> hat am 30. Januar 2019 um 18:28 geschrieben: > >>> > >>> > >>> On Wed, Jan 30, 2019 at 04:07:05PM +0100, Stefan Wahren wrote: > >>>> This adds the tachometer interrupt to the pwm-fan binding, which is > >>>> necessary for RPM support. > >>>> > >>>> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com> > >>>> --- > >>>> Documentation/devicetree/bindings/hwmon/pwm-fan.txt | 3 +++ > >>>> 1 file changed, 3 insertions(+) > >>>> > >>>> diff --git a/Documentation/devicetree/bindings/hwmon/pwm-fan.txt b/Documentation/devicetree/bindings/hwmon/pwm-fan.txt > >>>> index 49ca5d8..7f69b0b 100644 > >>>> --- a/Documentation/devicetree/bindings/hwmon/pwm-fan.txt > >>>> +++ b/Documentation/devicetree/bindings/hwmon/pwm-fan.txt > >>>> @@ -8,6 +8,9 @@ Required properties: > >>>> > >>>> Optional properties: > >>>> - fan-supply : phandle to the regulator that provides power to the fan > >>>> +- interrupts : contains a single interrupt specifier which describes the > >>>> + tachometer pin output of a 2 pulse-per-revolution fan. > >>>> + See interrupt-controller/interrupts.txt for the format. > >>> So a hypothetical {1,4} pulse-per-revolution fan would explicitly not be > >>> supported ? Why ? > >> i could add an additional property to specify the pulse per revolution and use the 2 as default (according to the Intel spec for 4 pin pwm fan) which should fit in most cases. > >> > > That would be a possibility and make sense, but that is not > > the point here. The "interrupts" property does not and should > > not care how many pulses per revolution the fan provides. > > sorry, i'm not sure what's the problem about. Do you want me to use a > GPIO instead of interrupt? > > Or is it the wording here? > You wording limits the use of interrupts with pwm fans to fans with 2 pulses per revolution. You do not explain why that restriction would be required or even make sense, or why it should be associated with the 'interrupts' property. Logically I assume that is because you expect an interrupt per pulse, but that is not explained (or what the interrupt is expected to be used for in the first place - it might, after all, be some kind of error interrupt). Guenter
diff --git a/Documentation/devicetree/bindings/hwmon/pwm-fan.txt b/Documentation/devicetree/bindings/hwmon/pwm-fan.txt index 49ca5d8..7f69b0b 100644 --- a/Documentation/devicetree/bindings/hwmon/pwm-fan.txt +++ b/Documentation/devicetree/bindings/hwmon/pwm-fan.txt @@ -8,6 +8,9 @@ Required properties: Optional properties: - fan-supply : phandle to the regulator that provides power to the fan +- interrupts : contains a single interrupt specifier which describes the + tachometer pin output of a 2 pulse-per-revolution fan. + See interrupt-controller/interrupts.txt for the format. Example: fan0: pwm-fan {
This adds the tachometer interrupt to the pwm-fan binding, which is necessary for RPM support. Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com> --- Documentation/devicetree/bindings/hwmon/pwm-fan.txt | 3 +++ 1 file changed, 3 insertions(+)