[RFC PATCH linux dev-4.10 3/7] hwmon: max31785: Document implementation oddities

[Andrew Jeffery]

Some possible bugs are discussed, and the implementation semantics of
fall-through cases in switch statements are documented.

Signed-off-by: Andrew Jeffery <andrew at aj.id.au>
---
 drivers/hwmon/max31785.c | 29 +++++++++++++++++++++++++----
 1 file changed, 25 insertions(+), 4 deletions(-)

diff --git a/drivers/hwmon/max31785.c b/drivers/hwmon/max31785.c
index fb7b3f010bd8..ad86082ffc7d 100644
--- a/drivers/hwmon/max31785.c
+++ b/drivers/hwmon/max31785.c
@@ -167,7 +167,12 @@ static struct max31785_data *max31785_update_device(struct device *dev)
 			}
 
 			if (!is_automatic_control_mode(data, i)) {
-				/* Poke watchdog for manual fan control */
+				/*
+				 * Poke watchdog for manual fan control
+				 *
+				 * XXX: This isn't documented in the MAX31785
+				 * datasheet, or anywhere else.
+				 */
 				rv = max31785_write_fan_data(client,
 					i, MAX31785_REG_FAN_COMMAND_1,
 					data->fan_command[i], 0);
@@ -290,6 +295,7 @@ static ssize_t set_fan_pulses(struct device *dev,
 	if (pulses > 4)
 		return -EINVAL;
 
+	/* XXX: This sequence disables the fan and sets in PWM mode */
 	data->fan_config[attr->index] &= MAX31785_FAN_CFG_PULSE_MASK;
 	data->fan_config[attr->index] |=
 				((pulses - MAX31785_FAN_CFG_PULSE_OFFSET)
@@ -297,8 +303,23 @@ static ssize_t set_fan_pulses(struct device *dev,
 
 	mutex_lock(&data->device_lock);
 
-	/* Write new pulse value */
+	/*
+	 * XXX: This seems suspect. Pulses must be in [1, 4], however
+	 * fan_command expects the following ranges:
+	 *
+	 * PWM:
+	 * [0x0000, 0x2710]: [0, 100]% PWM duty cycle
+	 * [0x2711, 0x7fff]: 100% PWM duty cycle
+	 *
+	 * RPM:
+	 * [0x0000, 0x7fff]: [0, 32767]RPM
+	 * [0x8000, 0xffff]: Automatic fan control
+	 *
+	 * In all cases [1-4] is quite low
+	 */
 	data->fan_command[attr->index] = pulses;
+
+	/* Write new pulse value */
 	err = max31785_write_fan_data(client, attr->index,
 				MAX31785_REG_FAN_CONFIG_1_2,
 				data->fan_config[attr->index], 1);
@@ -408,8 +429,8 @@ static ssize_t set_pwm_enable(struct device *dev,
 			data->fan_config[attr->index]
 			& ~MAX31785_FAN_CFG_PWM_ENABLE;
 		break;
-	case 1:
-	case 2:
+	case 1: /* fallthrough */
+	case 2: /* fallthrough */
 	case 3:
 		data->fan_config[attr->index] =
 			data->fan_config[attr->index]
-- 
2.11.0