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 <[email protected]>
---
drivers/hwmon/aquacomputer_d5next.c | 29 ++++++++++++++++++++++-------
1 file changed, 22 insertions(+), 7 deletions(-)
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;
--
2.43.0
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
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