Re: [PATCH] drivers: hwmon: Add max31760 fan speed controller driver

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

 



On 7/20/22 03:08, Tilki, Ibrahim wrote:
On Mon, Jul 04, 2022 at 04:06:05PM +0000, Ibrahim Tilki wrote:
MAX31760 is a presicion fan speed controller with nonvolatile lookup table.
Device has one internal and one external temperature sensor support.
Controls two fans and measures their speeds.
Generates hardware alerts when programmable max and critical temperatures are exceeded.

Driver creates following sysfs attributes for configuring lookup table:
pwm1_auto_point[1-48]_{pwm,temp,temp_hyst}

Signed-off-by: Ibrahim Tilki <Ibrahim.Tilki@xxxxxxxxxx>
Reviewed-by: Nurettin Bolucu <Nurettin.Bolucu@xxxxxxxxxx>
---
  drivers/hwmon/Kconfig    |  10 +
  drivers/hwmon/Makefile   |   1 +
  drivers/hwmon/max31760.c | 614
+++++++++++++++++++++++++++++++++++++++

Please add doucmentation describing the supported sysfs attributes.


Driver does not create a sysfs attribute other than the standard sysfs attributes defined here:
https://www.kernel.org/doc/Documentation/hwmon/sysfs-interface

Should I still add documentation?


Yes. The documentation describes the sysfs attributes supported by the driver.
See other documentation files in Documentation/hwmon for examples. Also please
read and follow Documentation/hwmon/submitting-patches.rst.

  3 files changed, 625 insertions(+)
  create mode 100644 drivers/hwmon/max31760.c

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index
590d3d550..59cc44a5c 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig

...

+	case hwmon_pwm:
+		switch (attr) {
+		case hwmon_pwm_input:
+			if (val < 0 || val > 255)
+				return -EINVAL;
+
+			return regmap_write(state->regmap, REG_PWMR, val);
+		case hwmon_pwm_enable:
+			if (val == 0)
+				return -EOPNOTSUPP;
+
+			if (val == 1)
+				return regmap_set_bits(state->regmap, REG_CR2, CR2_DFC);
+

Accepting all other values ?


Standard sysfs documentation defines pwm_enable as follows:
   - 0: no fan speed control (i.e. fan at full speed)
   - 1: manual fan speed control enabled (using pwm[1-*])
   - 2+: automatic fan speed control enabled

So I assumed I should accept all other values as automatic fan speed control.
If that is not the case, what is the correct way to handle this attribute?

That only says that drivers may implement values 2+ as fit for the chip.
For example, 2 could mean RPM based automatic fan speed control, and 3
could mean pwm based automatic fan speed control. It doesn't mean "accept
any random value".

Guenter



[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux