Re: [PATCH v3] hwmon: Add support for MAX31785 intelligent fan controller

[Andrew Jeffery]

On Thu, 2017-06-08 at 05:37 -0700, Guenter Roeck wrote:
> On 06/08/2017 12:53 AM, Andrew Jeffery wrote:
> > On Wed, 2017-06-07 at 08:55 -0700, Guenter Roeck wrote:
> > > On Tue, Jun 06, 2017 at 04:32:30PM +0930, Andrew Jeffery wrote:
> > > > Add a basic driver for the MAX31785, focusing on the fan control
> > > > features but ignoring the temperature and voltage monitoring
> > > > features of the device.
> > > > 
> > > > This driver supports all fan control modes and tachometer / PWM
> > > > readback where applicable.
> > > > 
> > > > > > Signed-off-by: Timothy Pearson 
> > > > > > Signed-off-by: Andrew Jeffery 
> > > > 
> > > > ---
> > > > Hello,
> > > > 
> > > > This is a rework of Timothy Pearson's original patch:
> > > > 
> > > >  
> > > > https://www.mail-archive.com/linux-hwmon@vger.kernel.org/msg00868.html
> > > > 
> > > > I've labelled it as v3 to differentiate from Timothy's postings.
> > > > 
> > > > The original thread had some discussion about the MAX31785 being a 
> > > > PMBus device
> > > > and that it should thus be a PMBus driver. The implementation still 
> > > > makes use
> > > 
> > > After thinking about it, that is what it should be. If I accept it as 
> > > non-PMBus
> > > driver, it will be all but impossible to convert it to a PMBus driver 
> > > later on,
> > > and that just doesn't make any sense.
> > 
> > Hopefully not being too ignorant here, but can you expand on why it
> > would be all but impossible to convert?
> > 
> 
> I've got a lot of noise recently just for converting a driver from the old to 
> the
> new API (which changes the attribute location). Changing the driver from 
> non-PMBus
> to PMBus would very quite likely change some attributes as well.

Okay.

> 
> Besides that, I think it is a bad idea to bypass an infrastructure just 
> because
> it may require a few tweaks. That generates a bad precedent, and people 
> _would_
> use that to argue that the next PMBus chip driver should not use the 
> infrastructure
> either.

I understand not wanting to set a precedent. Thanks for your response.

Andrew

> 
> Guenter
> 
> > > 
> > > With no one interested in writing that driver, I'll try to give it some 
> > > more
> > > priority myself. I do have an evaluation board somewhere, which should 
> > > help.
> > > 
> > > Note that the second fan reading should be implemented as just that, not 
> > > with
> > > a non-standard attribute.
> > 
> > Agreed.
> > 
> > Andrew
> > 
> 
> 

signature.asc
Description: This is a digitally signed message part

Re: [PATCH v3] hwmon: Add support for MAX31785 intelligent fan controller

[Guenter Roeck]

On 06/08/2017 12:53 AM, Andrew Jeffery wrote:

On Wed, 2017-06-07 at 08:55 -0700, Guenter Roeck wrote:

On Tue, Jun 06, 2017 at 04:32:30PM +0930, Andrew Jeffery wrote:

Add a basic driver for the MAX31785, focusing on the fan control
features but ignoring the temperature and voltage monitoring
features of the device.

This driver supports all fan control modes and tachometer / PWM
readback where applicable.


Signed-off-by: Timothy Pearson 
Signed-off-by: Andrew Jeffery 

---
Hello,

This is a rework of Timothy Pearson's original patch:

 https://www.mail-archive.com/linux-hwmon@vger.kernel.org/msg00868.html

I've labelled it as v3 to differentiate from Timothy's postings.

The original thread had some discussion about the MAX31785 being a PMBus device
and that it should thus be a PMBus driver. The implementation still makes use


After thinking about it, that is what it should be. If I accept it as non-PMBus
driver, it will be all but impossible to convert it to a PMBus driver later on,
and that just doesn't make any sense.


Hopefully not being too ignorant here, but can you expand on why it
would be all but impossible to convert?



I've got a lot of noise recently just for converting a driver from the old to 
the
new API (which changes the attribute location). Changing the driver from 
non-PMBus
to PMBus would very quite likely change some attributes as well.

Besides that, I think it is a bad idea to bypass an infrastructure just because
it may require a few tweaks. That generates a bad precedent, and people _would_
use that to argue that the next PMBus chip driver should not use the 
infrastructure
either.

Guenter



With no one interested in writing that driver, I'll try to give it some more
priority myself. I do have an evaluation board somewhere, which should help.

Note that the second fan reading should be implemented as just that, not with
a non-standard attribute.


Agreed.

Andrew



--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] hwmon: Add support for MAX31785 intelligent fan controller

[Andrew Jeffery]

On Wed, 2017-06-07 at 08:55 -0700, Guenter Roeck wrote:
> On Tue, Jun 06, 2017 at 04:32:30PM +0930, Andrew Jeffery wrote:
> > Add a basic driver for the MAX31785, focusing on the fan control
> > features but ignoring the temperature and voltage monitoring
> > features of the device.
> > 
> > This driver supports all fan control modes and tachometer / PWM
> > readback where applicable.
> > 
> > > > Signed-off-by: Timothy Pearson 
> > > > Signed-off-by: Andrew Jeffery 
> > ---
> > Hello,
> > 
> > This is a rework of Timothy Pearson's original patch:
> > 
> > https://www.mail-archive.com/linux-hwmon@vger.kernel.org/msg00868.html
> > 
> > I've labelled it as v3 to differentiate from Timothy's postings.
> > 
> > The original thread had some discussion about the MAX31785 being a PMBus 
> > device
> > and that it should thus be a PMBus driver. The implementation still makes 
> > use
> 
> After thinking about it, that is what it should be. If I accept it as 
> non-PMBus
> driver, it will be all but impossible to convert it to a PMBus driver later 
> on,
> and that just doesn't make any sense.

Hopefully not being too ignorant here, but can you expand on why it
would be all but impossible to convert?

> 
> With no one interested in writing that driver, I'll try to give it some more
> priority myself. I do have an evaluation board somewhere, which should help.
> 
> Note that the second fan reading should be implemented as just that, not with
> a non-standard attribute.

Agreed.

Andrew

signature.asc
Description: This is a digitally signed message part

Re: [PATCH v3] hwmon: Add support for MAX31785 intelligent fan controller

[Andrew Jeffery]

On Wed, 2017-06-07 at 10:37 -0700, Guenter Roeck wrote:
> On Wed, Jun 07, 2017 at 04:15:06PM +0930, Andrew Jeffery wrote:
> > On Wed, 2017-06-07 at 12:18 +0930, Joel Stanley wrote:
> > > On Wed, Jun 7, 2017 at 1:50 AM, Matthew Barth
> > > > > > > >  wrote:
> > > > 
> > > > On 06/06/17 8:33 AM, Guenter Roeck wrote:
> > > > > 
> > > > > On 06/06/2017 12:02 AM, Andrew Jeffery wrote:
> > > > > > Over and above the features of the original patch is support for a
> > > > > > secondary
> > > > > > rotor measurement value that is provided by MAX31785 chips with a 
> > > > > > revised
> > > > > > firmware. The feature(s) of the firmware are determined at probe 
> > > > > > time and
> > > > > > extra
> > > > > > attributes exposed accordingly. Specifically, the MFR_REVISION 
> > > > > > 0x3040 of
> > > > > > the
> > > > > > firmware supports 'slow' and 'fast' rotor reads. The feature is
> > > > > > implemented by
> > > > > > command 0x90 (READ_FAN_SPEED_1) providing a 4-byte response (with 
> > > > > > the
> > > > > > 'fast'
> > > > > > measurement in the second word) rather than the 2-bytes response in 
> > > > > > the
> > > > > > original firmware (MFR_REVISION 0x3030).
> > > > > > 
> > > > > 
> > > > > Taking the pmbus driver question out, why would this warrant another
> > > > > non-standard
> > > > > attribute outside the ABI ? I could see the desire to replace the 
> > > > > 'slow'
> > > > > access
> > > > > with the 'fast' one, but provide two attributes ? No, I don't see the
> > > > > point, sorry,
> > > > > even more so without detailed explanation why the second attribute in
> > > > > addition
> > > > > to the first one would add any value.
> > > > 
> > > > In the case of counter-rotating(CR) fans which contain two rotors to 
> > > > provide
> > > > more airflow there are then two tach feedbacks. These CR fans take a 
> > > > single
> > > > target speed and provide individual feedbacks for each rotor contained
> > > > within the fan enclosure. Providing these individual feedbacks assists 
> > > > in
> > > > fan fault driven speed changes, improved thermal characterization among
> > > > other things.
> > > > 
> > > > Maxim provided this as a 'slow' / 'fast' set of bytes to be user
> > > > compatible(so the 'slow' rotor speed, regardless of which rotor, is in 
> > > > the
> > > > first 2 bytes with the 'slow' version of firmware as well). In some 
> > > > cases,
> > > > mfg systems could have a mix of these revisions.
> > > 
> > > Andrew, instead of creating the _fast sysfs nodes, I think you could
> > > expose the extra rotors are separate fan devices in sysfs. This would
> > > not introduce new ABI.
> > 
> > I considered this approach: I debated whether to consider the fan unit
> > as a single entity with two inputs, or just separate fans, and ended up
> > leaning towards the former. To go the latter path we need to consider
> > whether or not to expose the writeable properties for the second input
> > (given they also affect the first) and how to sensibly arrange the
> > pairs given the functionality is determined at probe-time. Not rocket
> > science but decisions we need to make.
> > 
> 
> There are many other examples with one writeable and multiple readable
> attributes. Temperature offset attributes are an excellent example.
> Next question would be what exactly would be writable. pwm attributes are
> commonly completely independent of fan attributes. pwm1 output doesn't
> mean that fan1 is the matching input; in fact, most of the time it isn't.
> The only question would be numbering (is the pair numbered fan1/2 or
> fan1/fan(1+X) ?) which is just a matter of personal preference. However,
> everything is better than coming up with non-standard attributes which
> can not be used with any standard application beyond the application of the
> person submitting the driver. It is bad enough if a non-standard attribute
> describes something really driver specific. But a non-standard attribute
> for a fan speed reading ? Please no. 

Yes, I've received loud and clear that I made the wrong choice :)
Apologies.

Thanks again for your feedback.

Andrew



signature.asc
Description: This is a digitally signed message part

Re: [PATCH v3] hwmon: Add support for MAX31785 intelligent fan controller

[Guenter Roeck]

On Wed, Jun 07, 2017 at 04:15:06PM +0930, Andrew Jeffery wrote:
> On Wed, 2017-06-07 at 12:18 +0930, Joel Stanley wrote:
> > On Wed, Jun 7, 2017 at 1:50 AM, Matthew Barth
> > >  wrote:
> > > 
> > > On 06/06/17 8:33 AM, Guenter Roeck wrote:
> > > > 
> > > > On 06/06/2017 12:02 AM, Andrew Jeffery wrote:
> > > > > Over and above the features of the original patch is support for a
> > > > > secondary
> > > > > rotor measurement value that is provided by MAX31785 chips with a 
> > > > > revised
> > > > > firmware. The feature(s) of the firmware are determined at probe time 
> > > > > and
> > > > > extra
> > > > > attributes exposed accordingly. Specifically, the MFR_REVISION 0x3040 
> > > > > of
> > > > > the
> > > > > firmware supports 'slow' and 'fast' rotor reads. The feature is
> > > > > implemented by
> > > > > command 0x90 (READ_FAN_SPEED_1) providing a 4-byte response (with the
> > > > > 'fast'
> > > > > measurement in the second word) rather than the 2-bytes response in 
> > > > > the
> > > > > original firmware (MFR_REVISION 0x3030).
> > > > > 
> > > > 
> > > > Taking the pmbus driver question out, why would this warrant another
> > > > non-standard
> > > > attribute outside the ABI ? I could see the desire to replace the 'slow'
> > > > access
> > > > with the 'fast' one, but provide two attributes ? No, I don't see the
> > > > point, sorry,
> > > > even more so without detailed explanation why the second attribute in
> > > > addition
> > > > to the first one would add any value.
> > > 
> > > In the case of counter-rotating(CR) fans which contain two rotors to 
> > > provide
> > > more airflow there are then two tach feedbacks. These CR fans take a 
> > > single
> > > target speed and provide individual feedbacks for each rotor contained
> > > within the fan enclosure. Providing these individual feedbacks assists in
> > > fan fault driven speed changes, improved thermal characterization among
> > > other things.
> > > 
> > > Maxim provided this as a 'slow' / 'fast' set of bytes to be user
> > > compatible(so the 'slow' rotor speed, regardless of which rotor, is in the
> > > first 2 bytes with the 'slow' version of firmware as well). In some cases,
> > > mfg systems could have a mix of these revisions.
> > 
> > Andrew, instead of creating the _fast sysfs nodes, I think you could
> > expose the extra rotors are separate fan devices in sysfs. This would
> > not introduce new ABI.
> 
> I considered this approach: I debated whether to consider the fan unit
> as a single entity with two inputs, or just separate fans, and ended up
> leaning towards the former. To go the latter path we need to consider
> whether or not to expose the writeable properties for the second input
> (given they also affect the first) and how to sensibly arrange the
> pairs given the functionality is determined at probe-time. Not rocket
> science but decisions we need to make.
> 

There are many other examples with one writeable and multiple readable
attributes. Temperature offset attributes are an excellent example.
Next question would be what exactly would be writable. pwm attributes are
commonly completely independent of fan attributes. pwm1 output doesn't
mean that fan1 is the matching input; in fact, most of the time it isn't.
The only question would be numbering (is the pair numbered fan1/2 or
fan1/fan(1+X) ?) which is just a matter of personal preference. However,
everything is better than coming up with non-standard attributes which
can not be used with any standard application beyond the application of the
person submitting the driver. It is bad enough if a non-standard attribute
describes something really driver specific. But a non-standard attribute
for a fan speed reading ? Please no. We don't use outX_output instead of
inX_input for voltage outputs either.

Guenter

> There's also the issue that the physical fan that each element of an
> input pair represents will change as the fan speeds vary, based on the
> behaviour that Matt outlined. I don't feel this maps well onto the
> expectations of the sysfs interface, but then I'm not sure there's much
> we can do to alleviate it either other than designating one of the
> input attributes of a fan pair as the fastest input.
> 
> Regardless, I'll look into it in the anticipation that this is a viable
> way forward.
> 
> Cheers,
> 
> Andrew


--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] hwmon: Add support for MAX31785 intelligent fan controller

[Guenter Roeck]

On Tue, Jun 06, 2017 at 04:32:30PM +0930, Andrew Jeffery wrote:
> Add a basic driver for the MAX31785, focusing on the fan control
> features but ignoring the temperature and voltage monitoring
> features of the device.
> 
> This driver supports all fan control modes and tachometer / PWM
> readback where applicable.
> 
> Signed-off-by: Timothy Pearson 
> Signed-off-by: Andrew Jeffery 
> ---
> Hello,
> 
> This is a rework of Timothy Pearson's original patch:
> 
> https://www.mail-archive.com/linux-hwmon@vger.kernel.org/msg00868.html
> 
> I've labelled it as v3 to differentiate from Timothy's postings.
> 
> The original thread had some discussion about the MAX31785 being a PMBus 
> device
> and that it should thus be a PMBus driver. The implementation still makes use

After thinking about it, that is what it should be. If I accept it as non-PMBus
driver, it will be all but impossible to convert it to a PMBus driver later on,
and that just doesn't make any sense.

With no one interested in writing that driver, I'll try to give it some more
priority myself. I do have an evaluation board somewhere, which should help.

Note that the second fan reading should be implemented as just that, not with
a non-standard attribute.

Guenter
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] hwmon: Add support for MAX31785 intelligent fan controller

[Andrew Jeffery]

On Tue, 2017-06-06 at 06:33 -0700, Guenter Roeck wrote:
> On 06/06/2017 12:02 AM, Andrew Jeffery wrote:
> > Add a basic driver for the MAX31785, focusing on the fan control
> > features but ignoring the temperature and voltage monitoring
> > features of the device.
> > 
> > This driver supports all fan control modes and tachometer / PWM
> > readback where applicable.
> > 
> > > > Signed-off-by: Timothy Pearson 
> > > > Signed-off-by: Andrew Jeffery 
> > ---
> > Hello,
> > 
> > This is a rework of Timothy Pearson's original patch:
> > 
> >  https://www.mail-archive.com/linux-hwmon@vger.kernel.org/msg00868.html
> > 
> > I've labelled it as v3 to differentiate from Timothy's postings.
> > 
> > The original thread had some discussion about the MAX31785 being a PMBus 
> > device
> > and that it should thus be a PMBus driver. The implementation still makes 
> > use
> > of features not available in the pmbus core, so I've taken up the earlier
> > suggestion and ported it to the devm_hwmon_device_register_with_info()
> > interface. This gave a modest reduction in lines-of-code and at least to me 
> > is
> > more aesthetically pleasing.
> > 
> > Over and above the features of the original patch is support for a secondary
> > rotor measurement value that is provided by MAX31785 chips with a revised
> > firmware. The feature(s) of the firmware are determined at probe time and 
> > extra
> > attributes exposed accordingly. Specifically, the MFR_REVISION 0x3040 of the
> > firmware supports 'slow' and 'fast' rotor reads. The feature is implemented 
> > by
> > command 0x90 (READ_FAN_SPEED_1) providing a 4-byte response (with the 'fast'
> > measurement in the second word) rather than the 2-bytes response in the
> > original firmware (MFR_REVISION 0x3030).
> > 
> 
> Taking the pmbus driver question out, why would this warrant another 
> non-standard
> attribute outside the ABI ? I could see the desire to replace the 'slow' 
> access
> with the 'fast' one, but provide two attributes ? No, I don't see the point, 
> sorry,
> even more so without detailed explanation why the second attribute in addition
> to the first one would add any value.

At the least I'll update the documentation in line with Matt Barth's
comment, as it was vague. Whether or not we keep the questionable
attribute I hope will be resolved down further in the thread.

> 
> > This feature is not documented in the public datasheet[1].
> > 
> > [1] https://datasheets.maximintegrated.com/en/ds/MAX31785.pdf
> > 
> > The need to read a 4-byte value drives the addition of a helper that is a
> > cut-down version of i2c_smbus_xfer_emulated(), as 4-byte transactions 
> > aren't a
> > defined transaction type in the PMBus spec. This seemed more tasteful than
> > hacking the PMBus core to support the quirks of a single device.
> > 
> 
> That is why we have PMBus helper drivers.

Right - I meant to say SMBus in the last sentence, not PMBus, as it was
with respect to hacking i2c_smbus_xfer_emulated(). So I understand that
the device-specific PMBus drivers exist to help with quirks, just that
it's not (yet) a PMBus driver.

Thanks for the feedback,

Andrew

> 
> Guenter
> 
> > Also changed from Timothy's original posting is I've massaged the locking a 
> > bit
> > and removed what seemed to be a copy/paste bug around 
> > max31785_fan_set_pulses()
> > setting the fan_command member.
> > 
> > Tested on an IBM Witherspoon machine.
> > 
> > Cheers,
> > 
> > Andrew
> > 
> >   Documentation/hwmon/max31785 |  44 +++
> >   drivers/hwmon/Kconfig|  10 +
> >   drivers/hwmon/Makefile   |   1 +
> >   drivers/hwmon/max31785.c | 824 
> > +++
> >   4 files changed, 879 insertions(+)
> >   create mode 100644 Documentation/hwmon/max31785
> >   create mode 100644 drivers/hwmon/max31785.c
> > 
> > diff --git a/Documentation/hwmon/max31785 b/Documentation/hwmon/max31785
> > new file mode 100644
> > index ..dd891c06401e
> > --- /dev/null
> > +++ b/Documentation/hwmon/max31785
> > @@ -0,0 +1,44 @@
> > +Kernel driver max31785
> > +==
> > +
> > +Supported chips:
> > +  * Maxim MAX31785
> > +Prefix: 'max31785'
> > +Addresses scanned: 0x52 0x53 0x54 0x55
> > +Datasheet: http://pdfserv.maximintegrated.com/en/ds/MAX31785.pdf
> > +
> > > > +Author: Timothy Pearson 
> > +
> > +
> > +Description
> > +---
> > +
> > +This driver implements support for the Maxim MAX31785 chip.
> > +
> > +The MAX31785 controls the speeds of up to six fans using six independent
> > +PWM outputs. The desired fan speeds (or PWM duty cycles) are written
> > +through the I2C interface. The outputs drive "4-wire" fans directly,
> > +or can be used to modulate the fan's power terminals using an external
> > +pass transistor.
> > +
> > +Tachometer inputs monitor fan tachometer logic outputs for precise (+/-1%)
> > +monitoring and control of 

Re: [PATCH v3] hwmon: Add support for MAX31785 intelligent fan controller

[Andrew Jeffery]

On Wed, 2017-06-07 at 12:18 +0930, Joel Stanley wrote:
> On Wed, Jun 7, 2017 at 1:50 AM, Matthew Barth
> >  wrote:
> > 
> > On 06/06/17 8:33 AM, Guenter Roeck wrote:
> > > 
> > > On 06/06/2017 12:02 AM, Andrew Jeffery wrote:
> > > > Over and above the features of the original patch is support for a
> > > > secondary
> > > > rotor measurement value that is provided by MAX31785 chips with a 
> > > > revised
> > > > firmware. The feature(s) of the firmware are determined at probe time 
> > > > and
> > > > extra
> > > > attributes exposed accordingly. Specifically, the MFR_REVISION 0x3040 of
> > > > the
> > > > firmware supports 'slow' and 'fast' rotor reads. The feature is
> > > > implemented by
> > > > command 0x90 (READ_FAN_SPEED_1) providing a 4-byte response (with the
> > > > 'fast'
> > > > measurement in the second word) rather than the 2-bytes response in the
> > > > original firmware (MFR_REVISION 0x3030).
> > > > 
> > > 
> > > Taking the pmbus driver question out, why would this warrant another
> > > non-standard
> > > attribute outside the ABI ? I could see the desire to replace the 'slow'
> > > access
> > > with the 'fast' one, but provide two attributes ? No, I don't see the
> > > point, sorry,
> > > even more so without detailed explanation why the second attribute in
> > > addition
> > > to the first one would add any value.
> > 
> > In the case of counter-rotating(CR) fans which contain two rotors to provide
> > more airflow there are then two tach feedbacks. These CR fans take a single
> > target speed and provide individual feedbacks for each rotor contained
> > within the fan enclosure. Providing these individual feedbacks assists in
> > fan fault driven speed changes, improved thermal characterization among
> > other things.
> > 
> > Maxim provided this as a 'slow' / 'fast' set of bytes to be user
> > compatible(so the 'slow' rotor speed, regardless of which rotor, is in the
> > first 2 bytes with the 'slow' version of firmware as well). In some cases,
> > mfg systems could have a mix of these revisions.
> 
> Andrew, instead of creating the _fast sysfs nodes, I think you could
> expose the extra rotors are separate fan devices in sysfs. This would
> not introduce new ABI.

I considered this approach: I debated whether to consider the fan unit
as a single entity with two inputs, or just separate fans, and ended up
leaning towards the former. To go the latter path we need to consider
whether or not to expose the writeable properties for the second input
(given they also affect the first) and how to sensibly arrange the
pairs given the functionality is determined at probe-time. Not rocket
science but decisions we need to make.

There's also the issue that the physical fan that each element of an
input pair represents will change as the fan speeds vary, based on the
behaviour that Matt outlined. I don't feel this maps well onto the
expectations of the sysfs interface, but then I'm not sure there's much
we can do to alleviate it either other than designating one of the
input attributes of a fan pair as the fastest input.

Regardless, I'll look into it in the anticipation that this is a viable
way forward.

Cheers,

Andrew

signature.asc
Description: This is a digitally signed message part

Re: [PATCH v3] hwmon: Add support for MAX31785 intelligent fan controller

[Joel Stanley]

On Wed, Jun 7, 2017 at 1:50 AM, Matthew Barth
 wrote:
>
> On 06/06/17 8:33 AM, Guenter Roeck wrote:
>>
>> On 06/06/2017 12:02 AM, Andrew Jeffery wrote:
>>> Over and above the features of the original patch is support for a
>>> secondary
>>> rotor measurement value that is provided by MAX31785 chips with a revised
>>> firmware. The feature(s) of the firmware are determined at probe time and
>>> extra
>>> attributes exposed accordingly. Specifically, the MFR_REVISION 0x3040 of
>>> the
>>> firmware supports 'slow' and 'fast' rotor reads. The feature is
>>> implemented by
>>> command 0x90 (READ_FAN_SPEED_1) providing a 4-byte response (with the
>>> 'fast'
>>> measurement in the second word) rather than the 2-bytes response in the
>>> original firmware (MFR_REVISION 0x3030).
>>>
>>
>> Taking the pmbus driver question out, why would this warrant another
>> non-standard
>> attribute outside the ABI ? I could see the desire to replace the 'slow'
>> access
>> with the 'fast' one, but provide two attributes ? No, I don't see the
>> point, sorry,
>> even more so without detailed explanation why the second attribute in
>> addition
>> to the first one would add any value.
>
> In the case of counter-rotating(CR) fans which contain two rotors to provide
> more airflow there are then two tach feedbacks. These CR fans take a single
> target speed and provide individual feedbacks for each rotor contained
> within the fan enclosure. Providing these individual feedbacks assists in
> fan fault driven speed changes, improved thermal characterization among
> other things.
>
> Maxim provided this as a 'slow' / 'fast' set of bytes to be user
> compatible(so the 'slow' rotor speed, regardless of which rotor, is in the
> first 2 bytes with the 'slow' version of firmware as well). In some cases,
> mfg systems could have a mix of these revisions.

Andrew, instead of creating the _fast sysfs nodes, I think you could
expose the extra rotors are separate fan devices in sysfs. This would
not introduce new ABI.

Guenter, would this be acceptable to you?

Cheers,

Joel


>
>>
>>> This feature is not documented in the public datasheet[1].
>>>
>>> [1] https://datasheets.maximintegrated.com/en/ds/MAX31785.pdf
>>>
>>> The need to read a 4-byte value drives the addition of a helper that is a
>>> cut-down version of i2c_smbus_xfer_emulated(), as 4-byte transactions
>>> aren't a
>>> defined transaction type in the PMBus spec. This seemed more tasteful
>>> than
>>> hacking the PMBus core to support the quirks of a single device.
>>>
>>
>> That is why we have PMBus helper drivers.
>>
>> Guenter
>>
>>> Also changed from Timothy's original posting is I've massaged the locking
>>> a bit
>>> and removed what seemed to be a copy/paste bug around
>>> max31785_fan_set_pulses()
>>> setting the fan_command member.
>>>
>>> Tested on an IBM Witherspoon machine.
>>>
>>> Cheers,
>>>
>>> Andrew
>>>
>>>   Documentation/hwmon/max31785 |  44 +++
>>>   drivers/hwmon/Kconfig|  10 +
>>>   drivers/hwmon/Makefile   |   1 +
>>>   drivers/hwmon/max31785.c | 824
>>> +++
>>>   4 files changed, 879 insertions(+)
>>>   create mode 100644 Documentation/hwmon/max31785
>>>   create mode 100644 drivers/hwmon/max31785.c
>>>
>>> diff --git a/Documentation/hwmon/max31785 b/Documentation/hwmon/max31785
>>> new file mode 100644
>>> index ..dd891c06401e
>>> --- /dev/null
>>> +++ b/Documentation/hwmon/max31785
>>> @@ -0,0 +1,44 @@
>>> +Kernel driver max31785
>>> +==
>>> +
>>> +Supported chips:
>>> +  * Maxim MAX31785
>>> +Prefix: 'max31785'
>>> +Addresses scanned: 0x52 0x53 0x54 0x55
>>> +Datasheet: http://pdfserv.maximintegrated.com/en/ds/MAX31785.pdf
>>> +
>>> +Author: Timothy Pearson 
>>> +
>>> +
>>> +Description
>>> +---
>>> +
>>> +This driver implements support for the Maxim MAX31785 chip.
>>> +
>>> +The MAX31785 controls the speeds of up to six fans using six independent
>>> +PWM outputs. The desired fan speeds (or PWM duty cycles) are written
>>> +through the I2C interface. The outputs drive "4-wire" fans directly,
>>> +or can be used to modulate the fan's power terminals using an external
>>> +pass transistor.
>>> +
>>> +Tachometer inputs monitor fan tachometer logic outputs for precise
>>> (+/-1%)
>>> +monitoring and control of fan RPM as well as detection of fan failure.
>>> +
>>> +
>>> +Sysfs entries
>>> +-
>>> +
>>> +fan[1-6]_input   RO  fan tachometer speed in RPM
>>> +fan[1-6]_fault   RO  fan experienced fault
>>> +fan[1-6]_pulses  RW  tachometer pulses per fan revolution
>>> +fan[1-6]_target  RW  desired fan speed in RPM
>>> +pwm[1-6]_enable  RW  pwm mode, 0=disabled, 1=pwm, 2=rpm,
>>> 3=automatic
>>> +pwm[1-6] RW  fan target duty cycle (0-255)
>>> +
>>> +Dynamic sysfs entries
>>> +
>>> +
>>> +Whether these entries are 

Re: [PATCH v3] hwmon: Add support for MAX31785 intelligent fan controller

[kbuild test robot]

Hi Andrew,

[auto build test ERROR on hwmon/hwmon-next]
[also build test ERROR on v4.12-rc4 next-20170606]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Andrew-Jeffery/hwmon-Add-support-for-MAX31785-intelligent-fan-controller/20170607-020015
base:   
https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git 
hwmon-next
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 6.2.0
reproduce:
wget 
https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=ia64 

All error/warnings (new ones prefixed by >>):

   In file included from include/linux/kobject.h:21:0,
from include/linux/device.h:17,
from include/linux/hwmon-sysfs.h:23,
from drivers/hwmon/max31785.c:20:
>> drivers/hwmon/max31785.c:727:50: error: initialization from incompatible 
>> pointer type [-Werror=incompatible-pointer-types]
static SENSOR_DEVICE_ATTR(fan1_input_fast, 0444, max31785_fan_get_fast,
 ^
   include/linux/sysfs.h:103:10: note: in definition of macro '__ATTR'
 .show = _show,  \
 ^
>> include/linux/hwmon-sysfs.h:38:4: note: in expansion of macro 'SENSOR_ATTR'
 = SENSOR_ATTR(_name, _mode, _show, _store, _index)
   ^~~
>> drivers/hwmon/max31785.c:727:8: note: in expansion of macro 
>> 'SENSOR_DEVICE_ATTR'
static SENSOR_DEVICE_ATTR(fan1_input_fast, 0444, max31785_fan_get_fast,
   ^~
   drivers/hwmon/max31785.c:727:50: note: (near initialization for 
'sensor_dev_attr_fan1_input_fast.dev_attr.show')
static SENSOR_DEVICE_ATTR(fan1_input_fast, 0444, max31785_fan_get_fast,
 ^
   include/linux/sysfs.h:103:10: note: in definition of macro '__ATTR'
 .show = _show,  \
 ^
>> include/linux/hwmon-sysfs.h:38:4: note: in expansion of macro 'SENSOR_ATTR'
 = SENSOR_ATTR(_name, _mode, _show, _store, _index)
   ^~~
>> drivers/hwmon/max31785.c:727:8: note: in expansion of macro 
>> 'SENSOR_DEVICE_ATTR'
static SENSOR_DEVICE_ATTR(fan1_input_fast, 0444, max31785_fan_get_fast,
   ^~
   drivers/hwmon/max31785.c:729:50: error: initialization from incompatible 
pointer type [-Werror=incompatible-pointer-types]
static SENSOR_DEVICE_ATTR(fan2_input_fast, 0444, max31785_fan_get_fast,
 ^
   include/linux/sysfs.h:103:10: note: in definition of macro '__ATTR'
 .show = _show,  \
 ^
>> include/linux/hwmon-sysfs.h:38:4: note: in expansion of macro 'SENSOR_ATTR'
 = SENSOR_ATTR(_name, _mode, _show, _store, _index)
   ^~~
   drivers/hwmon/max31785.c:729:8: note: in expansion of macro 
'SENSOR_DEVICE_ATTR'
static SENSOR_DEVICE_ATTR(fan2_input_fast, 0444, max31785_fan_get_fast,
   ^~
   drivers/hwmon/max31785.c:729:50: note: (near initialization for 
'sensor_dev_attr_fan2_input_fast.dev_attr.show')
static SENSOR_DEVICE_ATTR(fan2_input_fast, 0444, max31785_fan_get_fast,
 ^
   include/linux/sysfs.h:103:10: note: in definition of macro '__ATTR'
 .show = _show,  \
 ^
>> include/linux/hwmon-sysfs.h:38:4: note: in expansion of macro 'SENSOR_ATTR'
 = SENSOR_ATTR(_name, _mode, _show, _store, _index)
   ^~~
   drivers/hwmon/max31785.c:729:8: note: in expansion of macro 
'SENSOR_DEVICE_ATTR'
static SENSOR_DEVICE_ATTR(fan2_input_fast, 0444, max31785_fan_get_fast,
   ^~
   drivers/hwmon/max31785.c:731:50: error: initialization from incompatible 
pointer type [-Werror=incompatible-pointer-types]
static SENSOR_DEVICE_ATTR(fan3_input_fast, 0444, max31785_fan_get_fast,
 ^
   include/linux/sysfs.h:103:10: note: in definition of macro '__ATTR'
 .show = _show,  \
 ^
>> include/linux/hwmon-sysfs.h:38:4: note: in expansion of macro 'SENSOR_ATTR'
 = SENSOR_ATTR(_name, _mode, _show, _store, _index)
   ^~~
   drivers/hwmon/max31785.c:731:8: note: in expansion of macro 
'SENSOR_DEVICE_ATTR'
static SENSOR_DEVICE_ATTR(fan3_input_fast, 0444, max31785_fan_get_fast,
   ^~
   drivers/hwmon/max31785.c:731:50: note: (near initialization for 
'sensor_dev_attr_fan3_input_fast.dev_attr.show')
static SENSOR_DEVICE_ATTR(fan3_input_fast, 0444, max31785_fan_get_fast,
 ^
   include/linux/sysfs.h:103:10: note: in definition of macro '__ATTR'
 .show = _show,  \
 ^
>> 

Re: [PATCH v3] hwmon: Add support for MAX31785 intelligent fan controller

[Matthew Barth]

On 06/06/17 8:33 AM, Guenter Roeck wrote:

On 06/06/2017 12:02 AM, Andrew Jeffery wrote:

Add a basic driver for the MAX31785, focusing on the fan control
features but ignoring the temperature and voltage monitoring
features of the device.

This driver supports all fan control modes and tachometer / PWM
readback where applicable.

Signed-off-by: Timothy Pearson 
Signed-off-by: Andrew Jeffery 
---
Hello,

This is a rework of Timothy Pearson's original patch:

https://www.mail-archive.com/linux-hwmon@vger.kernel.org/msg00868.html

I've labelled it as v3 to differentiate from Timothy's postings.

The original thread had some discussion about the MAX31785 being a 
PMBus device
and that it should thus be a PMBus driver. The implementation still 
makes use
of features not available in the pmbus core, so I've taken up the 
earlier

suggestion and ported it to the devm_hwmon_device_register_with_info()
interface. This gave a modest reduction in lines-of-code and at least 
to me is

more aesthetically pleasing.

Over and above the features of the original patch is support for a 
secondary
rotor measurement value that is provided by MAX31785 chips with a 
revised
firmware. The feature(s) of the firmware are determined at probe time 
and extra
attributes exposed accordingly. Specifically, the MFR_REVISION 0x3040 
of the
firmware supports 'slow' and 'fast' rotor reads. The feature is 
implemented by
command 0x90 (READ_FAN_SPEED_1) providing a 4-byte response (with the 
'fast'

measurement in the second word) rather than the 2-bytes response in the
original firmware (MFR_REVISION 0x3030).



Taking the pmbus driver question out, why would this warrant another 
non-standard
attribute outside the ABI ? I could see the desire to replace the 
'slow' access
with the 'fast' one, but provide two attributes ? No, I don't see the 
point, sorry,
even more so without detailed explanation why the second attribute in 
addition

to the first one would add any value.
In the case of counter-rotating(CR) fans which contain two rotors to 
provide more airflow there are then two tach feedbacks. These CR fans 
take a single target speed and provide individual feedbacks for each 
rotor contained within the fan enclosure. Providing these individual 
feedbacks assists in fan fault driven speed changes, improved thermal 
characterization among other things.


Maxim provided this as a 'slow' / 'fast' set of bytes to be user 
compatible(so the 'slow' rotor speed, regardless of which rotor, is in 
the first 2 bytes with the 'slow' version of firmware as well). In some 
cases, mfg systems could have a mix of these revisions.



This feature is not documented in the public datasheet[1].

[1] https://datasheets.maximintegrated.com/en/ds/MAX31785.pdf

The need to read a 4-byte value drives the addition of a helper that 
is a
cut-down version of i2c_smbus_xfer_emulated(), as 4-byte transactions 
aren't a
defined transaction type in the PMBus spec. This seemed more tasteful 
than

hacking the PMBus core to support the quirks of a single device.



That is why we have PMBus helper drivers.

Guenter

Also changed from Timothy's original posting is I've massaged the 
locking a bit
and removed what seemed to be a copy/paste bug around 
max31785_fan_set_pulses()

setting the fan_command member.

Tested on an IBM Witherspoon machine.

Cheers,

Andrew

  Documentation/hwmon/max31785 |  44 +++
  drivers/hwmon/Kconfig|  10 +
  drivers/hwmon/Makefile   |   1 +
  drivers/hwmon/max31785.c | 824 
+++

  4 files changed, 879 insertions(+)
  create mode 100644 Documentation/hwmon/max31785
  create mode 100644 drivers/hwmon/max31785.c

diff --git a/Documentation/hwmon/max31785 b/Documentation/hwmon/max31785
new file mode 100644
index ..dd891c06401e
--- /dev/null
+++ b/Documentation/hwmon/max31785
@@ -0,0 +1,44 @@
+Kernel driver max31785
+==
+
+Supported chips:
+  * Maxim MAX31785
+Prefix: 'max31785'
+Addresses scanned: 0x52 0x53 0x54 0x55
+Datasheet: http://pdfserv.maximintegrated.com/en/ds/MAX31785.pdf
+
+Author: Timothy Pearson 
+
+
+Description
+---
+
+This driver implements support for the Maxim MAX31785 chip.
+
+The MAX31785 controls the speeds of up to six fans using six 
independent

+PWM outputs. The desired fan speeds (or PWM duty cycles) are written
+through the I2C interface. The outputs drive "4-wire" fans directly,
+or can be used to modulate the fan's power terminals using an external
+pass transistor.
+
+Tachometer inputs monitor fan tachometer logic outputs for precise 
(+/-1%)

+monitoring and control of fan RPM as well as detection of fan failure.
+
+
+Sysfs entries
+-
+
+fan[1-6]_input   RO  fan tachometer speed in RPM
+fan[1-6]_fault   RO  fan experienced fault
+fan[1-6]_pulses  RW  tachometer pulses per fan revolution

Re: [PATCH v3] hwmon: Add support for MAX31785 intelligent fan controller

[Guenter Roeck]

On 06/06/2017 12:02 AM, Andrew Jeffery wrote:

Add a basic driver for the MAX31785, focusing on the fan control
features but ignoring the temperature and voltage monitoring
features of the device.

This driver supports all fan control modes and tachometer / PWM
readback where applicable.

Signed-off-by: Timothy Pearson 
Signed-off-by: Andrew Jeffery 
---
Hello,

This is a rework of Timothy Pearson's original patch:

 https://www.mail-archive.com/linux-hwmon@vger.kernel.org/msg00868.html

I've labelled it as v3 to differentiate from Timothy's postings.

The original thread had some discussion about the MAX31785 being a PMBus device
and that it should thus be a PMBus driver. The implementation still makes use
of features not available in the pmbus core, so I've taken up the earlier
suggestion and ported it to the devm_hwmon_device_register_with_info()
interface. This gave a modest reduction in lines-of-code and at least to me is
more aesthetically pleasing.

Over and above the features of the original patch is support for a secondary
rotor measurement value that is provided by MAX31785 chips with a revised
firmware. The feature(s) of the firmware are determined at probe time and extra
attributes exposed accordingly. Specifically, the MFR_REVISION 0x3040 of the
firmware supports 'slow' and 'fast' rotor reads. The feature is implemented by
command 0x90 (READ_FAN_SPEED_1) providing a 4-byte response (with the 'fast'
measurement in the second word) rather than the 2-bytes response in the
original firmware (MFR_REVISION 0x3030).



Taking the pmbus driver question out, why would this warrant another 
non-standard
attribute outside the ABI ? I could see the desire to replace the 'slow' access
with the 'fast' one, but provide two attributes ? No, I don't see the point, 
sorry,
even more so without detailed explanation why the second attribute in addition
to the first one would add any value.


This feature is not documented in the public datasheet[1].

[1] https://datasheets.maximintegrated.com/en/ds/MAX31785.pdf

The need to read a 4-byte value drives the addition of a helper that is a
cut-down version of i2c_smbus_xfer_emulated(), as 4-byte transactions aren't a
defined transaction type in the PMBus spec. This seemed more tasteful than
hacking the PMBus core to support the quirks of a single device.



That is why we have PMBus helper drivers.

Guenter


Also changed from Timothy's original posting is I've massaged the locking a bit
and removed what seemed to be a copy/paste bug around max31785_fan_set_pulses()
setting the fan_command member.

Tested on an IBM Witherspoon machine.

Cheers,

Andrew

  Documentation/hwmon/max31785 |  44 +++
  drivers/hwmon/Kconfig|  10 +
  drivers/hwmon/Makefile   |   1 +
  drivers/hwmon/max31785.c | 824 +++
  4 files changed, 879 insertions(+)
  create mode 100644 Documentation/hwmon/max31785
  create mode 100644 drivers/hwmon/max31785.c

diff --git a/Documentation/hwmon/max31785 b/Documentation/hwmon/max31785
new file mode 100644
index ..dd891c06401e
--- /dev/null
+++ b/Documentation/hwmon/max31785
@@ -0,0 +1,44 @@
+Kernel driver max31785
+==
+
+Supported chips:
+  * Maxim MAX31785
+Prefix: 'max31785'
+Addresses scanned: 0x52 0x53 0x54 0x55
+Datasheet: http://pdfserv.maximintegrated.com/en/ds/MAX31785.pdf
+
+Author: Timothy Pearson 
+
+
+Description
+---
+
+This driver implements support for the Maxim MAX31785 chip.
+
+The MAX31785 controls the speeds of up to six fans using six independent
+PWM outputs. The desired fan speeds (or PWM duty cycles) are written
+through the I2C interface. The outputs drive "4-wire" fans directly,
+or can be used to modulate the fan's power terminals using an external
+pass transistor.
+
+Tachometer inputs monitor fan tachometer logic outputs for precise (+/-1%)
+monitoring and control of fan RPM as well as detection of fan failure.
+
+
+Sysfs entries
+-
+
+fan[1-6]_input   RO  fan tachometer speed in RPM
+fan[1-6]_fault   RO  fan experienced fault
+fan[1-6]_pulses  RW  tachometer pulses per fan revolution
+fan[1-6]_target  RW  desired fan speed in RPM
+pwm[1-6]_enable  RW  pwm mode, 0=disabled, 1=pwm, 2=rpm, 3=automatic
+pwm[1-6] RW  fan target duty cycle (0-255)
+
+Dynamic sysfs entries
+
+
+Whether these entries are present depends on the firmware features detected on
+the device during probe.
+
+fan[1-6]_input_fast  RO  fan tachometer speed in RPM (fast rotor 
measurement)
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index e80ca81577f4..c75d6072c823 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -886,6 +886,16 @@ config SENSORS_MAX6697
  This driver can also be built as a module.  If so, the module
  will be called 

[PATCH v3] hwmon: Add support for MAX31785 intelligent fan controller

[Andrew Jeffery]

Add a basic driver for the MAX31785, focusing on the fan control
features but ignoring the temperature and voltage monitoring
features of the device.

This driver supports all fan control modes and tachometer / PWM
readback where applicable.

Signed-off-by: Timothy Pearson 
Signed-off-by: Andrew Jeffery 
---
Hello,

This is a rework of Timothy Pearson's original patch:

https://www.mail-archive.com/linux-hwmon@vger.kernel.org/msg00868.html

I've labelled it as v3 to differentiate from Timothy's postings.

The original thread had some discussion about the MAX31785 being a PMBus device
and that it should thus be a PMBus driver. The implementation still makes use
of features not available in the pmbus core, so I've taken up the earlier
suggestion and ported it to the devm_hwmon_device_register_with_info()
interface. This gave a modest reduction in lines-of-code and at least to me is
more aesthetically pleasing.

Over and above the features of the original patch is support for a secondary
rotor measurement value that is provided by MAX31785 chips with a revised
firmware. The feature(s) of the firmware are determined at probe time and extra
attributes exposed accordingly. Specifically, the MFR_REVISION 0x3040 of the
firmware supports 'slow' and 'fast' rotor reads. The feature is implemented by
command 0x90 (READ_FAN_SPEED_1) providing a 4-byte response (with the 'fast'
measurement in the second word) rather than the 2-bytes response in the
original firmware (MFR_REVISION 0x3030).

This feature is not documented in the public datasheet[1].

[1] https://datasheets.maximintegrated.com/en/ds/MAX31785.pdf

The need to read a 4-byte value drives the addition of a helper that is a
cut-down version of i2c_smbus_xfer_emulated(), as 4-byte transactions aren't a
defined transaction type in the PMBus spec. This seemed more tasteful than
hacking the PMBus core to support the quirks of a single device.

Also changed from Timothy's original posting is I've massaged the locking a bit
and removed what seemed to be a copy/paste bug around max31785_fan_set_pulses()
setting the fan_command member.

Tested on an IBM Witherspoon machine.

Cheers,

Andrew

 Documentation/hwmon/max31785 |  44 +++
 drivers/hwmon/Kconfig|  10 +
 drivers/hwmon/Makefile   |   1 +
 drivers/hwmon/max31785.c | 824 +++
 4 files changed, 879 insertions(+)
 create mode 100644 Documentation/hwmon/max31785
 create mode 100644 drivers/hwmon/max31785.c

diff --git a/Documentation/hwmon/max31785 b/Documentation/hwmon/max31785
new file mode 100644
index ..dd891c06401e
--- /dev/null
+++ b/Documentation/hwmon/max31785
@@ -0,0 +1,44 @@
+Kernel driver max31785
+==
+
+Supported chips:
+  * Maxim MAX31785
+Prefix: 'max31785'
+Addresses scanned: 0x52 0x53 0x54 0x55
+Datasheet: http://pdfserv.maximintegrated.com/en/ds/MAX31785.pdf
+
+Author: Timothy Pearson 
+
+
+Description
+---
+
+This driver implements support for the Maxim MAX31785 chip.
+
+The MAX31785 controls the speeds of up to six fans using six independent
+PWM outputs. The desired fan speeds (or PWM duty cycles) are written
+through the I2C interface. The outputs drive "4-wire" fans directly,
+or can be used to modulate the fan's power terminals using an external
+pass transistor.
+
+Tachometer inputs monitor fan tachometer logic outputs for precise (+/-1%)
+monitoring and control of fan RPM as well as detection of fan failure.
+
+
+Sysfs entries
+-
+
+fan[1-6]_input   RO  fan tachometer speed in RPM
+fan[1-6]_fault   RO  fan experienced fault
+fan[1-6]_pulses  RW  tachometer pulses per fan revolution
+fan[1-6]_target  RW  desired fan speed in RPM
+pwm[1-6]_enable  RW  pwm mode, 0=disabled, 1=pwm, 2=rpm, 3=automatic
+pwm[1-6] RW  fan target duty cycle (0-255)
+
+Dynamic sysfs entries
+
+
+Whether these entries are present depends on the firmware features detected on
+the device during probe.
+
+fan[1-6]_input_fast  RO  fan tachometer speed in RPM (fast rotor 
measurement)
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index e80ca81577f4..c75d6072c823 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -886,6 +886,16 @@ config SENSORS_MAX6697
  This driver can also be built as a module.  If so, the module
  will be called max6697.
 
+config SENSORS_MAX31785
+   tristate "Maxim MAX31785 sensor chip"
+   depends on I2C
+   help
+ If you say yes here you get support for 6-Channel PWM-Output
+ Fan RPM Controller.
+
+ This driver can also be built as a module.  If so, the module
+ will be called max31785.
+
 config SENSORS_MAX31790
tristate "Maxim MAX31790 sensor chip"
depends on I2C
diff --git a/drivers/hwmon/Makefile