Re: [PATCH] Initial driver for the MAX31785 intelligent fan controller
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 09/20/2016 10:04 PM, Guenter Roeck wrote: > On 09/20/2016 01:01 PM, Timothy Pearson wrote: >> On 09/20/2016 02:59 PM, Guenter Roeck wrote: >> >> OK, sounds good. I looked back in the notes for this project and we had >> originally considered using a PMBus driver but ran into the kernel >> documentation, noted the missing control features, and I subsequently > > It is always useful to talk with the maintainer. > >> misinterpreted the datasheet using the kernel docs as a reference. This >> is why the hwmon driver was implemented. >> > > I am still not sure I understand what irked you off in the datasheet. > Is it the "supports a subset of the commands defined in the PMBus > Specfication" ? If so, please keep in mind that every single chip > supporting PMBus will only support a subset of PMBus commands. > >> Please let me know if I can be of any assistance. >> > Sure, I'll let you know. > > Thanks, > Guenter > Just checking back in to see how things are going with this driver. I don't really know how involved the surgery to PMBus will be to get this device working; are we looking at weeks / months or something sooner? Thanks again! - -- Timothy Pearson Raptor Engineering +1 (415) 727-8645 (direct line) +1 (512) 690-0200 (switchboard) https://www.raptorengineering.com -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iQEcBAEBAgAGBQJX/PmkAAoJEK+E3vEXDOFbYVQIAKhoDT+kU+fR2CML4S3wYRQP fjI/vJG6D3UPzb8M8IvCpGVSxR4bIl1uLC3pftT5TbF93XHXd6M39MUJYcoWRjnE iB//9KrYopFXcFxGY6wIKqJ9YwQLDHAMDFAd7GS852z6+gMRxU06L4vOWpCH+Xgv kNbU8vMuyie0a3SwgC6+gYn/hUyZNzVLUXMtM0AJZm4zpcMoTXXqsHINNc91d82D RZSHNZs+fHiEqWxAV2GPNGQ+ZnwE+rIYT4yXml6q13AywaUEkJTL45WN86ntt75G EbCsvzcfgaxz3tlis+UW/yqy6FEaHdVxFICq9FuawAfBjjgeC4rxP54ZD3BHOTM= =LJ5E -END PGP SIGNATURE- -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Initial driver for the MAX31785 intelligent fan controller
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 09/20/2016 10:04 PM, Guenter Roeck wrote: > On 09/20/2016 01:01 PM, Timothy Pearson wrote: >> -BEGIN PGP SIGNED MESSAGE- >> Hash: SHA1 >> >> On 09/20/2016 02:59 PM, Guenter Roeck wrote: >> OK, sounds good. I looked back in the notes for this project and we had >> originally considered using a PMBus driver but ran into the kernel >> documentation, noted the missing control features, and I subsequently > > It is always useful to talk with the maintainer. I'll keep that in mind next time. This particular area of the kernel seems much more responsive than some of the others that we've had the misfortune of trying to work with; multiple weeks of delay for a response to a simple two line patch tends to turn one off of the mailing lists... >> misinterpreted the datasheet using the kernel docs as a reference. This >> is why the hwmon driver was implemented. >> > > I am still not sure I understand what irked you off in the datasheet. > Is it the "supports a subset of the commands defined in the PMBus > Specfication" ? If so, please keep in mind that every single chip > supporting PMBus will only support a subset of PMBus commands. Yes, I definitely understand that now. This was the line that, when taken out of context and compared against the kernel documentation, caused an incorrect assumption on my part. >> Please let me know if I can be of any assistance. >> > Sure, I'll let you know. > > Thanks, > Guenter > - -- Timothy Pearson Raptor Engineering +1 (415) 727-8645 (direct line) +1 (512) 690-0200 (switchboard) https://www.raptorengineering.com -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iQEcBAEBAgAGBQJX4fnKAAoJEK+E3vEXDOFbyJEH/AqkLDgx6BgxcHRfl63G9YlE ln97m5X8NrWkGDQajzPHtv4g7imrli4ztTfJgjLnEPKu257clh1pEd32/gWKZYp5 tnewbANOjOg4E/75bM7I1hFSN4LXylwiPGPtNdtPqmHDsKk3RDLSweFn2wuKIvuP rMJVmZHElbZdM5BenWWBEaqJxkUe8SkWfwb4CcIPNigETMZckvpzP+Db9qm2t5pq Go3lYbamLK2KcrMngRTtnH3EmJY7KXa0+GsgmQthkxYA7ZvDG9DRBLFpPECEWZfr HZrld5hwnzGwh2GkTTvO1JUQvv5wrtu1tXAxR5EPCWWDjqHW3dUzG1WgmsSJ+mI= =xXkP -END PGP SIGNATURE- -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Initial driver for the MAX31785 intelligent fan controller
On 09/20/2016 01:01 PM, Timothy Pearson wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 09/20/2016 02:59 PM, Guenter Roeck wrote: On Tue, Sep 20, 2016 at 02:41:56PM -0500, Timothy Pearson wrote: On 09/19/2016 07:54 PM, Guenter Roeck wrote: On 09/19/2016 03:04 PM, Guenter Roeck wrote: And then you are using all pmbus commands ? Seems odd. I guess I'l buy an evaluation board if one is available and check if your claim is correct. I am not inclined to accept a non-pmbus driver for a pmbus device without good reason. Guenter Regarding the PMBus driver, I looked over the documentation available here: https://www.kernel.org/doc/Documentation/hwmon/pmbus From what I can tell PMBus drivers do not support configuring the fan control parameters, only monitoring the fan status and speed. Is this correct, and if not where would I find the correct documentation? So far that wasn't needed. That doesn't mean it can not be added. I orderd an evaluation board and will likely spend some time on it myself after I get it. Guenter OK, sounds good. I looked back in the notes for this project and we had originally considered using a PMBus driver but ran into the kernel documentation, noted the missing control features, and I subsequently It is always useful to talk with the maintainer. misinterpreted the datasheet using the kernel docs as a reference. This is why the hwmon driver was implemented. I am still not sure I understand what irked you off in the datasheet. Is it the "supports a subset of the commands defined in the PMBus Specfication" ? If so, please keep in mind that every single chip supporting PMBus will only support a subset of PMBus commands. Please let me know if I can be of any assistance. Sure, I'll let you know. Thanks, Guenter -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Initial driver for the MAX31785 intelligent fan controller
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 09/20/2016 02:59 PM, Guenter Roeck wrote: > On Tue, Sep 20, 2016 at 02:41:56PM -0500, Timothy Pearson wrote: >> On 09/19/2016 07:54 PM, Guenter Roeck wrote: On 09/19/2016 03:04 PM, Guenter Roeck wrote: And then you are using all pmbus commands ? Seems odd. >>> >>> I guess I'l buy an evaluation board if one is available and check if your >>> claim >>> is correct. I am not inclined to accept a non-pmbus driver for a pmbus >>> device without good reason. >>> >>> Guenter >> >> Regarding the PMBus driver, I looked over the documentation available >> here: https://www.kernel.org/doc/Documentation/hwmon/pmbus >> >> From what I can tell PMBus drivers do not support configuring the fan >> control parameters, only monitoring the fan status and speed. Is this >> correct, and if not where would I find the correct documentation? >> > So far that wasn't needed. That doesn't mean it can not be added. > > I orderd an evaluation board and will likely spend some time on it myself > after I get it. > > Guenter OK, sounds good. I looked back in the notes for this project and we had originally considered using a PMBus driver but ran into the kernel documentation, noted the missing control features, and I subsequently misinterpreted the datasheet using the kernel docs as a reference. This is why the hwmon driver was implemented. Please let me know if I can be of any assistance. Thanks! - -- Timothy Pearson Raptor Engineering +1 (415) 727-8645 (direct line) +1 (512) 690-0200 (switchboard) https://www.raptorengineering.com -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iQEcBAEBAgAGBQJX4ZWeAAoJEK+E3vEXDOFbSNkH/3IqLAbvz2aVyPUBhKBFHC72 0uaP/IhBIoDoKAdZ50jJljYQIub0hn6HrQH52SYXvNhkO43+UJwqOxIRliipueDg X9H2esdeaptjI+O3dXCv5bLyMgy+ohGv1aPkxon+BTo506NWlHOyYlbCscIgmuMu nsMmHVGQvEyMtbJHlwmT0tjE81IEYxFxc1eLPbPhcrdz1f6yIAY7tHFo5ZjE+wt8 mzU35ZI+zIkKbD88Qfs8Ca/aTfRzt5xDKHDI3//YouyiqlnDK7etoZrAtx2RxtcJ Qv8k2udTpQuuYwwGobwC0SSWaxeRMlRaumX+ZkIYJz7xSp1krnArx5na2r2cTr8= =Guwk -END PGP SIGNATURE- -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Initial driver for the MAX31785 intelligent fan controller
On Tue, Sep 20, 2016 at 02:41:56PM -0500, Timothy Pearson wrote: > On 09/19/2016 07:54 PM, Guenter Roeck wrote: > >> On 09/19/2016 03:04 PM, Guenter Roeck wrote: > >> And then you are using all pmbus commands ? Seems odd. > > > > I guess I'l buy an evaluation board if one is available and check if your > > claim > > is correct. I am not inclined to accept a non-pmbus driver for a pmbus > > device without good reason. > > > > Guenter > > Regarding the PMBus driver, I looked over the documentation available > here: https://www.kernel.org/doc/Documentation/hwmon/pmbus > > From what I can tell PMBus drivers do not support configuring the fan > control parameters, only monitoring the fan status and speed. Is this > correct, and if not where would I find the correct documentation? > So far that wasn't needed. That doesn't mean it can not be added. I orderd an evaluation board and will likely spend some time on it myself after I get it. Guenter -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Initial driver for the MAX31785 intelligent fan controller
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 09/19/2016 07:54 PM, Guenter Roeck wrote: >> On 09/19/2016 03:04 PM, Guenter Roeck wrote: >> And then you are using all pmbus commands ? Seems odd. > > I guess I'l buy an evaluation board if one is available and check if your > claim > is correct. I am not inclined to accept a non-pmbus driver for a pmbus > device without good reason. > > Guenter Regarding the PMBus driver, I looked over the documentation available here: https://www.kernel.org/doc/Documentation/hwmon/pmbus - From what I can tell PMBus drivers do not support configuring the fan control parameters, only monitoring the fan status and speed. Is this correct, and if not where would I find the correct documentation? Thank you, - -- Timothy Pearson Raptor Engineering +1 (415) 727-8645 (direct line) +1 (512) 690-0200 (switchboard) https://www.raptorengineering.com -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iQEcBAEBAgAGBQJX4ZECAAoJEK+E3vEXDOFb4XsIALCbk+/dO2LdTvfjDCVMrs9J 2IHDW3heUZn53v5evpMEvJCXEw8J0ARg2cYWxuzx8Ufu5hGzXNS40NW5mUHtkbXG FNWAqoSdeT1f9aOS8XIi/9+u27ZwJTuI5dZdb7nebfvsjlKc1CKw2FwtqUUBkyZA Ywa/kjSlEMTO2wGCea4xQXE8aO3UK3JHc0sUwzR3htEOp5nGIn7d8ScDaJvpzW+a xk0q97U4ezJEm2r3WmXpZGbUQqM2aaOdTSl9iiV7GZBAR6K50LGsacAcRBdanJaj FOuW9XZt0NjnrZOsMc2N9WIz9wXKYOuo/Y8d/0hVPp6SQ3po5d8coeNrSPDw1qw= =Q8So -END PGP SIGNATURE- -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Initial driver for the MAX31785 intelligent fan controller
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 09/19/2016 07:54 PM, Guenter Roeck wrote: > On Mon, Sep 19, 2016 at 03:31:33PM -0500, Timothy Pearson wrote: >> On 09/19/2016 03:04 PM, Guenter Roeck wrote: > And then you are using all pmbus commands ? Seems odd. > > I guess I'l buy an evaluation board if one is available and check if your > claim > is correct. I am not inclined to accept a non-pmbus driver for a pmbus > device without good reason. > > Guenter Well, I went back over the datasheet and I may have misinterpreted some of the introductory text. In any case I'm not likely to go back and re-do this as a pmbus driver as the original was done in my free time and works well enough for our needs at the moment. When the patch no longer applies perhaps someone else can pick up the porting effort. - -- Timothy Pearson Raptor Engineering +1 (415) 727-8645 (direct line) +1 (512) 690-0200 (switchboard) https://www.raptorengineering.com -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iQEcBAEBAgAGBQJX4JPYAAoJEK+E3vEXDOFb8koIAK1DQkicpt/CKU5vhGAxw8m9 MYGg+e+fe4s+PD3NmP0WZMAvDLtXOxYCBPRjqZWb4RJdfHNoAjjyJIFlDtG3UkBT /WIDO3gRADJLopabWSu591IKHfgHr/4v6+mfg8R2ZHrtbNNVnDlj1qnPanfPrskm h4Nq2Z1PPCBjmGA/HPr9NAkAASQmdcDRKkpO5k1zhUx5L4YL7QNQFphZxRkeiCKp mpA3hQIYayItCuRCt+PQWqb3dMB1hOkyEQP+xDzaLCkrajTt4wrh+lgjIfPEQZwC F71wiRuq5FVhZDqg/ko4/lkuwgmiXWR9lOpo7WSHHoFaYg54teFrYX3Z6Ci0Ug4= =ueAd -END PGP SIGNATURE- -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Initial driver for the MAX31785 intelligent fan controller
On Mon, Sep 19, 2016 at 03:31:33PM -0500, Timothy Pearson wrote: > On 09/19/2016 03:04 PM, Guenter Roeck wrote: > > On Mon, Sep 19, 2016 at 01:40:36PM -0500, Timothy Pearson wrote: > > Not sure I understand. pwm[1-6]_enable = 0 means no fan speed control. > > Do you mean the (full speed) comment ? > > "no fan speed control (i.e. fan at full speed)". What I would like to > see is "fan disabled (PWM turned off)". > > Is that an acceptable use of the "0" value? > > > Having said that, looking into the datasheet, I see that this is actually > > a PMBus device. Why don't you just write a PMBus extension driver for it ? > > AFAICS drivers/hwmon/pmbus/max34440.c would come pretty close and could > > possibly even be extended to support this chip. > > > > Guenter > > The datasheet states that many functions are not available over PMBus. > Rather than fight with potentially incomplete silicon support for PMBus > it was easier to simply write this driver. > And then you are using all pmbus commands ? Seems odd. I guess I'l buy an evaluation board if one is available and check if your claim is correct. I am not inclined to accept a non-pmbus driver for a pmbus device without good reason. Guenter > -- > Timothy Pearson > Raptor Engineering > +1 (415) 727-8645 (direct line) > +1 (512) 690-0200 (switchboard) > https://www.raptorengineering.com -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Initial driver for the MAX31785 intelligent fan controller
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 09/19/2016 03:04 PM, Guenter Roeck wrote: > On Mon, Sep 19, 2016 at 01:40:36PM -0500, Timothy Pearson wrote: > Not sure I understand. pwm[1-6]_enable = 0 means no fan speed control. > Do you mean the (full speed) comment ? "no fan speed control (i.e. fan at full speed)". What I would like to see is "fan disabled (PWM turned off)". Is that an acceptable use of the "0" value? > Having said that, looking into the datasheet, I see that this is actually > a PMBus device. Why don't you just write a PMBus extension driver for it ? > AFAICS drivers/hwmon/pmbus/max34440.c would come pretty close and could > possibly even be extended to support this chip. > > Guenter The datasheet states that many functions are not available over PMBus. Rather than fight with potentially incomplete silicon support for PMBus it was easier to simply write this driver. - -- Timothy Pearson Raptor Engineering +1 (415) 727-8645 (direct line) +1 (512) 690-0200 (switchboard) https://www.raptorengineering.com -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iQEcBAEBAgAGBQJX4EsiAAoJEK+E3vEXDOFbU7sH/1fcxUbs5inK+XL8tdpMnLDK y6uhsYMQaGzea4f4B6ek9XDDIyGJyMRwkAZy/4v9f+uqhK03DBrwRuY9zAeLPDd5 ilWPKNSJLNqs7vSOHz1f/ooIR20medAdx2oOAVX3QwoQjv/L40Sm9iLAmh/RQJtt urvLzcnBbhVnTVshuFwc2fwqlszLtaF9XUlPGzEWVZY0d7z+tPXwiseg/3nd444V LezNTkLgM9zkXwLz402zK9ckO5wABlbO1jd5mssLMnttyw4ilBwEpXgeeUlcsS71 GguTJnU35Wn0tD7DRUbqB76Jx5NX9Su/wHJ8X0hbe8CGAOHRzTYynTEzTHfGVFc= =XiuV -END PGP SIGNATURE- -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Initial driver for the MAX31785 intelligent fan controller
On Mon, Sep 19, 2016 at 01:14:49PM -0500, Timothy Pearson wrote: > On 09/18/2016 08:41 PM, Guenter Roeck wrote: > > On Sun, Sep 18, 2016 at 07:50:55PM -0500, Timothy Pearson 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. > >> > > Could you try using hwmon_device_register_with_info() as available in > > linux-next ? Hopefully that should reduce driver size (and give the new API > > some test coverage). > > Ordinarily I'd be willing to try this, but in this case I wouldn't be > able to test the module on real hardware; it's an embedded system and > uses the 4.7 kernel at the moment. Would you be OK with merging a fixed > version of this patch and I can throw together an untested variant using > hwmon_device_register_with_info() for future work? > After actually looking into the datasheet, I must admit that I don't understand why you did not model this as a PMBus driver. Guenter -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Initial driver for the MAX31785 intelligent fan controller
On Mon, Sep 19, 2016 at 01:40:36PM -0500, Timothy Pearson wrote: > On 09/18/2016 08:41 PM, Guenter Roeck wrote: > >> +Sysfs entries > >> +- > >> + > >> +fan[1-6]_input RO fan tachometer speed in RPM > >> +fan[1-6]_fault RO fan experienced fault > >> +fan[1-6]_target RW desired fan speed in RPM > >> +fan[1-6]_control_modeRW desired control mode: rpm, pwm, or auto > > > > Please use pwm[]_enable (see API) > > > >> +pwm[1-6]_enable RW output enabled, 0=disabled, 1=enabled > > > > Per API: 0 = no fan speed control (full speed), 1 = manual fan speed control > > enabled (using pwm[1-*]), 2+: automatic fan speed control > > > >> +pwm[1-6] RW fan target duty cycle (0-255) > > This particular device has a per-output PWM disable bit. How should > this be handled? I don't want to globally enable outputs that are not > connector or may possibly be incorrectly connected depending on hardware > design. > Not sure I understand. pwm[1-6]_enable = 0 means no fan speed control. Do you mean the (full speed) comment ? Having said that, looking into the datasheet, I see that this is actually a PMBus device. Why don't you just write a PMBus extension driver for it ? AFAICS drivers/hwmon/pmbus/max34440.c would come pretty close and could possibly even be extended to support this chip. Guenter -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Initial driver for the MAX31785 intelligent fan controller
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 09/18/2016 08:41 PM, Guenter Roeck wrote: >> +Sysfs entries >> +- >> + >> +fan[1-6]_input RO fan tachometer speed in RPM >> +fan[1-6]_fault RO fan experienced fault >> +fan[1-6]_target RW desired fan speed in RPM >> +fan[1-6]_control_modeRW desired control mode: rpm, pwm, or auto > > Please use pwm[]_enable (see API) > >> +pwm[1-6]_enable RW output enabled, 0=disabled, 1=enabled > > Per API: 0 = no fan speed control (full speed), 1 = manual fan speed control > enabled (using pwm[1-*]), 2+: automatic fan speed control > >> +pwm[1-6] RW fan target duty cycle (0-255) This particular device has a per-output PWM disable bit. How should this be handled? I don't want to globally enable outputs that are not connector or may possibly be incorrectly connected depending on hardware design. - -- Timothy Pearson Raptor Engineering +1 (415) 727-8645 (direct line) +1 (512) 690-0200 (switchboard) https://www.raptorengineering.com -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iQEcBAEBAgAGBQJX4DEgAAoJEK+E3vEXDOFbAz4IAKFCx0Mu6Q5HlsA5IloV1YcP qj5je1vBONSTxM1cfaYjLmZzUBVecbJZojQjhkoUXxrAkrtmrM9otHfw7k0qs/0q ppoVevrfp2jKMzvoBp3VytHTZy3rK1Oyxko1dIuwBHMvfMNNk1S8tm132jhv34bK C1z2l16kU7e99NQV1i4HvB8FyrwotHfPXuZuM1ukO+CmfhLsUzXGYcOsXPApKtdg CHijAp9qa+YmcEqmcNksncJn769IEhKc1aKWkjcqHg2chmXtz1aR9STzNRnHZJO8 RHivo6/9IGVNCOZSuup4HKhF1kle65W9tvS8D2eQnh/no+8rWHtMm+EZ+WEWFIs= =V8us -END PGP SIGNATURE- -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Initial driver for the MAX31785 intelligent fan controller
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 09/18/2016 08:41 PM, Guenter Roeck wrote: > On Sun, Sep 18, 2016 at 07:50:55PM -0500, Timothy Pearson 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. >> > Could you try using hwmon_device_register_with_info() as available in > linux-next ? Hopefully that should reduce driver size (and give the new API > some test coverage). Ordinarily I'd be willing to try this, but in this case I wouldn't be able to test the module on real hardware; it's an embedded system and uses the 4.7 kernel at the moment. Would you be OK with merging a fixed version of this patch and I can throw together an untested variant using hwmon_device_register_with_info() for future work? > Please run the patch through checkpatch --strict. I don't expect you to > fix all the problems is reports, but the following should be addressed. > > CHECK: Prefer kernel type 'u8' over 'uint8_t' > CHECK: Prefer kernel type 'u16' over 'uint16_t' > CHECK: Alignment should match open parenthesis > CHECK: Logical continuations should be on the previous line > WARNING: quoted string split across lines My fault, I completely forgot to run checkpatch. I'll get a V2 together shortly including addressing the inline comments. - -- Timothy Pearson Raptor Engineering +1 (415) 727-8645 (direct line) +1 (512) 690-0200 (switchboard) https://www.raptorengineering.com -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iQEcBAEBAgAGBQJX4CsUAAoJEK+E3vEXDOFb++AH/i1FxEmOJmNVlSowpV5EHsgQ xcquFnmeF/pK6Oh00LbcI3WvTCgVRBkUl32WG7m5ktKXLDn5fU9DFEQYQVVhzRFU jz83JyD8HvDhOKubk9G9mqE518ZBIEnYiREHFI9fVuOnUTVemrZtYdKdoA7ez3wi 5t7B7HJ4Gdcyi1FkNwLu+0/N7eLGyR9WfQAFqM0F64qRfQGOxWtwoi4lYgmbxHCg NFs7LAoJsj00K4DEPiK/3jjy5usfBpcIgkECAYK7+A/sUtDTq+TrBr1ADZY0mbV3 zqDo85alN5XQEvLTjACWXmauMhCUsV4OqzurtlBAVMJQDW6ZKeCnhxv+iUaDxh8= =Rr2j -END PGP SIGNATURE- -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Initial driver for the MAX31785 intelligent fan controller
On Sun, Sep 18, 2016 at 07:50:55PM -0500, Timothy Pearson 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. > Could you try using hwmon_device_register_with_info() as available in linux-next ? Hopefully that should reduce driver size (and give the new API some test coverage). Please run the patch through checkpatch --strict. I don't expect you to fix all the problems is reports, but the following should be addressed. CHECK: Prefer kernel type 'u8' over 'uint8_t' CHECK: Prefer kernel type 'u16' over 'uint16_t' CHECK: Alignment should match open parenthesis CHECK: Logical continuations should be on the previous line WARNING: quoted string split across lines Additional comments inline. > Signed-off-by: Timothy Pearson > --- > Documentation/hwmon/max31785 | 36 +++ > drivers/hwmon/Kconfig| 10 + > drivers/hwmon/Makefile |1 + > drivers/hwmon/max31785.c | 713 > ++ > 4 files changed, 760 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 000..34dca74 > --- /dev/null > +++ b/Documentation/hwmon/max31785 > @@ -0,0 +1,36 @@ > +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]_target RW desired fan speed in RPM > +fan[1-6]_control_modeRW desired control mode: rpm, pwm, or auto Please use pwm[]_enable (see API) > +pwm[1-6]_enable RW output enabled, 0=disabled, 1=enabled Per API: 0 = no fan speed control (full speed), 1 = manual fan speed control enabled (using pwm[1-*]), 2+: automatic fan speed control > +pwm[1-6] RW fan target duty cycle (0-255) > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig > index eaf2f91..a202fd5 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 b/drivers/hwmon/Makefile > index fe87d28..44c0c02 100644 > --- a/drivers/hwmon/Makefile > +++ b/drivers/hwmon/Makefile > @@ -119,6 +119,7 @@ obj-$(CONFIG_SENSORS_MAX6639) += max6639.o > obj-$(CONFIG_SENSORS_MAX6642)+= max6642.o > obj-$(CONFIG_SENSORS_MAX6650)+= max6650.o > obj-$(CONFIG_SENSORS_MAX6697)+= max6697.o > +obj-$(CONFIG_SENSORS_MAX31785) += max31785.o > obj-$(CONFIG_SENSORS_MAX31790) += max31790.o > obj-$(CONFIG_SENSORS_MC13783_ADC)+= mc13783-adc.o > obj-$(CONFIG_SENSORS_MCP3021)+= mcp3021.o > diff --git a/drivers/hwmon/max31785.c b/drivers/hwmon/max31785.c > new file mode 100644 > index 000..9a23edd > --- /dev/null > +++ b/drivers/hwmon/max31785.c > @@ -0,0 +1,713 @@ > +/* > + * max31785.c - Part of lm_sensors, Linux kernel modules for hardware > + * monitoring. > + * > + * (C) 2016 Raptor Engineering, LLC > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even th
[PATCH] Initial driver for the MAX31785 intelligent fan controller
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 --- Documentation/hwmon/max31785 | 36 +++ drivers/hwmon/Kconfig| 10 + drivers/hwmon/Makefile |1 + drivers/hwmon/max31785.c | 713 ++ 4 files changed, 760 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 000..34dca74 --- /dev/null +++ b/Documentation/hwmon/max31785 @@ -0,0 +1,36 @@ +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]_target RW desired fan speed in RPM +fan[1-6]_control_modeRW desired control mode: rpm, pwm, or auto +pwm[1-6]_enable RW output enabled, 0=disabled, 1=enabled +pwm[1-6] RW fan target duty cycle (0-255) diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index eaf2f91..a202fd5 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 b/drivers/hwmon/Makefile index fe87d28..44c0c02 100644 --- a/drivers/hwmon/Makefile +++ b/drivers/hwmon/Makefile @@ -119,6 +119,7 @@ obj-$(CONFIG_SENSORS_MAX6639) += max6639.o obj-$(CONFIG_SENSORS_MAX6642) += max6642.o obj-$(CONFIG_SENSORS_MAX6650) += max6650.o obj-$(CONFIG_SENSORS_MAX6697) += max6697.o +obj-$(CONFIG_SENSORS_MAX31785) += max31785.o obj-$(CONFIG_SENSORS_MAX31790) += max31790.o obj-$(CONFIG_SENSORS_MC13783_ADC)+= mc13783-adc.o obj-$(CONFIG_SENSORS_MCP3021) += mcp3021.o diff --git a/drivers/hwmon/max31785.c b/drivers/hwmon/max31785.c new file mode 100644 index 000..9a23edd --- /dev/null +++ b/drivers/hwmon/max31785.c @@ -0,0 +1,713 @@ +/* + * max31785.c - Part of lm_sensors, Linux kernel modules for hardware + * monitoring. + * + * (C) 2016 Raptor Engineering, LLC + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include +#include +#include +#include +#include +#include +#include +#include + +/* MAX31785 registers */ +#define MAX31785_REG_PAGE 0x00 +#define MAX31785_PAGE_FAN_CONFIG(ch) (0x00 + (ch)) +#define MAX31785_REG_FAN_CONFIG_1_20x3a +#define MAX31785_REG_FAN_COMMAND_1 0x3b +#define MAX31785_REG_STATUS_FANS_1_2 0x81 +#define MAX31785_REG_FAN_SPEED_1 0x90 +#define MAX31785_REG_MFR_ID0x99 +#define MAX31785_REG_MFR_MODEL 0x9a +#define MAX31785_REG_MFR_FAN_CONFIG0xf1 +#define MAX31785_REG_READ_FAN_PWM 0xf3 + +/* Fan Config register bits */ +#define MAX31785_FAN_CFG_PWM_ENABLE0x80 +#define MAX31785_FAN_CFG_CONTROL_MODE_RPM 0x40 + +/* Fan Status register bits */ +#define MAX31785_FAN_STATUS_FAULT_MASK 0x80 + +#define NR_CHANNEL
