hwmon: (aquacomputer_d5next) Set fan to direct PWM mode when writing value

Message ID 20240217181536.344386-1-savicaleksa83@gmail.com
State New
Headers
Series hwmon: (aquacomputer_d5next) Set fan to direct PWM mode when writing value |

Commit Message

Aleksa Savic Feb. 17, 2024, 6:15 p.m. UTC
  When setting a PWM value for a fan channel, ensure that the device
is actually in direct PWM value mode, as it could be in PID, curve or
fan following mode from previous user configurations. The byte
signifying the channel mode is just behind the offset for the value.
Otherwise, setting PWM speed might result in a no-op from the point
of the user.

Fixes: 752b927951ea ("hwmon: (aquacomputer_d5next) Add support for Aquacomputer Octo")
Signed-off-by: Aleksa Savic <savicaleksa83@gmail.com>
---
 drivers/hwmon/aquacomputer_d5next.c | 29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)
  

Comments

Guenter Roeck Feb. 17, 2024, 6:24 p.m. UTC | #1
On 2/17/24 10:15, Aleksa Savic wrote:
> When setting a PWM value for a fan channel, ensure that the device
> is actually in direct PWM value mode, as it could be in PID, curve or
> fan following mode from previous user configurations. The byte
> signifying the channel mode is just behind the offset for the value.
> Otherwise, setting PWM speed might result in a no-op from the point
> of the user.
> 

You can require that a device is in manual mode when setting pwm
modes, and return an error if it isn't. However, changing the mode
to manual automatically when a pwm value is written is not acceptable.

Guenter
  
Aleksa Savic Feb. 18, 2024, 4:02 p.m. UTC | #2
On 2024-02-17 19:24:50 GMT+01:00, Guenter Roeck wrote:
> 
> You can require that a device is in manual mode when setting pwm
> modes, and return an error if it isn't. However, changing the mode
> to manual automatically when a pwm value is written is not acceptable.
> 
> Guenter
> 

Right, thanks. Will implement pwm_enable, then.

Aleksa
  

Patch

diff --git a/drivers/hwmon/aquacomputer_d5next.c b/drivers/hwmon/aquacomputer_d5next.c
index 2efe97f8d003..809fbbd087f4 100644
--- a/drivers/hwmon/aquacomputer_d5next.c
+++ b/drivers/hwmon/aquacomputer_d5next.c
@@ -111,6 +111,9 @@  static u8 aquaero_secondary_ctrl_report[] = {
 #define AQC_FAN_POWER_OFFSET		0x06
 #define AQC_FAN_SPEED_OFFSET		0x08
 
+/* Report offsets for fan control */
+#define AQC_FAN_CTRL_PWM_OFFSET		1
+
 /* Specs of the Aquaero fan controllers */
 #define AQUAERO_SERIAL_START			0x07
 #define AQUAERO_FIRMWARE_VERSION		0x0B
@@ -160,7 +163,7 @@  static u16 d5next_sensor_fan_offsets[] = { D5NEXT_PUMP_OFFSET, D5NEXT_FAN_OFFSET
 
 /* Control report offsets for the D5 Next pump */
 #define D5NEXT_TEMP_CTRL_OFFSET		0x2D	/* Temperature sensor offsets location */
-static u16 d5next_ctrl_fan_offsets[] = { 0x97, 0x42 };	/* Pump and fan speed (from 0-100%) */
+static u16 d5next_ctrl_fan_offsets[] = { 0x96, 0x41 };	/* Pump and fan speed (from 0-100%) */
 
 /* Specs of the Aquastream Ultimate pump */
 /* Pump does not follow the standard structure, so only consider the fan */
@@ -213,7 +216,7 @@  static u16 octo_sensor_fan_offsets[] = { 0x7D, 0x8A, 0x97, 0xA4, 0xB1, 0xBE, 0xC
 /* Control report offsets for the Octo */
 #define OCTO_TEMP_CTRL_OFFSET		0xA
 /* Fan speed offsets (0-100%) */
-static u16 octo_ctrl_fan_offsets[] = { 0x5B, 0xB0, 0x105, 0x15A, 0x1AF, 0x204, 0x259, 0x2AE };
+static u16 octo_ctrl_fan_offsets[] = { 0x5A, 0xAF, 0x104, 0x159, 0x1AE, 0x203, 0x258, 0x2AD };
 
 /* Specs of Quadro fan controller */
 #define QUADRO_NUM_FANS			4
@@ -232,7 +235,7 @@  static u16 quadro_sensor_fan_offsets[] = { 0x70, 0x7D, 0x8A, 0x97 };
 /* Control report offsets for the Quadro */
 #define QUADRO_TEMP_CTRL_OFFSET		0xA
 #define QUADRO_FLOW_PULSES_CTRL_OFFSET	0x6
-static u16 quadro_ctrl_fan_offsets[] = { 0x37, 0x8c, 0xe1, 0x136 }; /* Fan speed offsets (0-100%) */
+static u16 quadro_ctrl_fan_offsets[] = { 0x36, 0x8b, 0xe0, 0x135 }; /* Fan speed offsets (0-100%) */
 
 /* Specs of High Flow Next flow sensor */
 #define HIGHFLOWNEXT_NUM_SENSORS	2
@@ -1094,8 +1097,9 @@  static int aqc_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
 			*val = aqc_percent_to_pwm(*val);
 			break;
 		default:
-			ret = aqc_get_ctrl_val(priv, priv->fan_ctrl_offsets[channel],
-					       val, AQC_BE16);
+			ret = aqc_get_ctrl_val(priv,
+					       priv->fan_ctrl_offsets[channel] +
+					       AQC_FAN_CTRL_PWM_OFFSET, val, AQC_BE16);
 			if (ret < 0)
 				return ret;
 
@@ -1233,8 +1237,19 @@  static int aqc_write(struct device *dev, enum hwmon_sensor_types type, u32 attr,
 					return ret;
 				break;
 			default:
-				ret = aqc_set_ctrl_val(priv, priv->fan_ctrl_offsets[channel],
-						       pwm_value, AQC_BE16);
+				/* Set fan controller to direct PWM mode */
+				ctrl_values_offsets[0] = priv->fan_ctrl_offsets[channel];
+				ctrl_values[0] = 0;	/* Use direct PWM mode */
+				ctrl_values_types[0] = AQC_8;
+
+				/* Set the PWM value */
+				ctrl_values_offsets[1] =
+				    priv->fan_ctrl_offsets[channel] + AQC_FAN_CTRL_PWM_OFFSET;
+				ctrl_values[1] = pwm_value;
+				ctrl_values_types[1] = AQC_BE16;
+
+				ret = aqc_set_ctrl_vals(priv, ctrl_values_offsets, ctrl_values,
+							ctrl_values_types, 2);
 				if (ret < 0)
 					return ret;
 				break;