[PATCH v2 0/6] hwmon: (aquacomputer_d5next) Add Aquacomputer Aquaero control

[PATCH v2 0/6] hwmon: (aquacomputer_d5next) Add Aquacomputer Aquaero control

[Leonard Anderweit]

Add support for controlling the Aquacomputer Aquaero 5/6 fan controllers. The
controllable settings include temperature offsets and fan PWM.

The first four patches expand the capabilities of the control system without
changing functionality. Patches 4-6 add support for Aquaero control.

changelog:
v2:
 - split original patch 3 into three patches to have one logical change each
 - explain why Aquaero does not need the checksum
 - improve wording of documentation
 - drop last patch with pwmX_mode

Leonard Anderweit (6):
  hwmon: (aquacomputer_d5next) Support one byte control values
  hwmon: (aquacomputer_d5next) Support writing multiple control values
    at once
  hwmon: (aquacomputer_d5next) Device dependent control report settings
  hwmon: (aquacomputer_d5next) Add infrastructure for Aquaero control
    reports
  hwmon: (aquacomputer_d5next) Add temperature offset control for
    Aquaero
  hwmon: (aquacomputer_d5next) Add fan PWM control for Aquaero

 Documentation/hwmon/aquacomputer_d5next.rst |   5 +-
 drivers/hwmon/aquacomputer_d5next.c         | 182 ++++++++++++++++----
 2 files changed, 155 insertions(+), 32 deletions(-)

-- 
2.39.1

[PATCH v2 1/6] hwmon: (aquacomputer_d5next) Support one byte control values

[Leonard Anderweit]

Add support for one byte control values. This extends aqc_set_ctrl_val() and
aqc_get_ctrl_val() with a type. Currently supported types are AQC_8 (one byte)
and AQC_BE16 (two bytes big endian). More types will be added in the future.

Signed-off-by: Leonard Anderweit <leonard.anderweit@gmail.com>
---
 drivers/hwmon/aquacomputer_d5next.c | 48 +++++++++++++++++++++++------
 1 file changed, 38 insertions(+), 10 deletions(-)

diff --git a/drivers/hwmon/aquacomputer_d5next.c b/drivers/hwmon/aquacomputer_d5next.c
index 12682a610ce7..babfd998e70c 100644
--- a/drivers/hwmon/aquacomputer_d5next.c
+++ b/drivers/hwmon/aquacomputer_d5next.c
@@ -70,6 +70,10 @@ static u8 secondary_ctrl_report[] = {
 /* Report IDs for legacy devices */
 #define POWERADJUST3_STATUS_REPORT_ID	0x03
 
+/* Data types for reading and writing control reports */
+#define AQC_8		0
+#define AQC_BE16	1
+
 /* Info, sensor sizes and offsets for most Aquacomputer devices */
 #define AQC_SERIAL_START		0x3
 #define AQC_FIRMWARE_VERSION		0xD
@@ -544,7 +548,7 @@ static int aqc_send_ctrl_data(struct aqc_data *priv)
 }
 
 /* Refreshes the control buffer and stores value at offset in val */
-static int aqc_get_ctrl_val(struct aqc_data *priv, int offset, long *val)
+static int aqc_get_ctrl_val(struct aqc_data *priv, int offset, long *val, int type)
 {
 	int ret;
 
@@ -554,14 +558,23 @@ static int aqc_get_ctrl_val(struct aqc_data *priv, int offset, long *val)
 	if (ret < 0)
 		goto unlock_and_return;
 
-	*val = (s16)get_unaligned_be16(priv->buffer + offset);
+	switch (type) {
+	case AQC_BE16:
+		*val = (s16)get_unaligned_be16(priv->buffer + offset);
+		break;
+	case AQC_8:
+		*val = priv->buffer[offset];
+		break;
+	default:
+		ret = -EINVAL;
+	}
 
 unlock_and_return:
 	mutex_unlock(&priv->mutex);
 	return ret;
 }
 
-static int aqc_set_ctrl_val(struct aqc_data *priv, int offset, long val)
+static int aqc_set_ctrl_val(struct aqc_data *priv, int offset, long val, int type)
 {
 	int ret;
 
@@ -571,7 +584,19 @@ static int aqc_set_ctrl_val(struct aqc_data *priv, int offset, long val)
 	if (ret < 0)
 		goto unlock_and_return;
 
-	put_unaligned_be16((s16)val, priv->buffer + offset);
+	switch (type) {
+	case AQC_BE16:
+		put_unaligned_be16((s16)val, priv->buffer + offset);
+		break;
+	case AQC_8:
+		priv->buffer[offset] = (u8)val;
+		break;
+	default:
+		ret = -EINVAL;
+	}
+
+	if (ret < 0)
+		goto unlock_and_return;
 
 	ret = aqc_send_ctrl_data(priv);
 
@@ -775,7 +800,7 @@ static int aqc_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
 		case hwmon_temp_offset:
 			ret =
 			    aqc_get_ctrl_val(priv, priv->temp_ctrl_offset +
-					     channel * AQC_SENSOR_SIZE, val);
+					     channel * AQC_SENSOR_SIZE, val, AQC_BE16);
 			if (ret < 0)
 				return ret;
 
@@ -791,7 +816,8 @@ static int aqc_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
 			*val = priv->speed_input[channel];
 			break;
 		case hwmon_fan_pulses:
-			ret = aqc_get_ctrl_val(priv, priv->flow_pulses_ctrl_offset, val);
+			ret = aqc_get_ctrl_val(priv, priv->flow_pulses_ctrl_offset,
+					       val, AQC_BE16);
 			if (ret < 0)
 				return ret;
 			break;
@@ -804,7 +830,8 @@ static int aqc_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
 		break;
 	case hwmon_pwm:
 		if (priv->fan_ctrl_offsets) {
-			ret = aqc_get_ctrl_val(priv, priv->fan_ctrl_offsets[channel], val);
+			ret = aqc_get_ctrl_val(priv, priv->fan_ctrl_offsets[channel],
+					       val, AQC_BE16);
 			if (ret < 0)
 				return ret;
 
@@ -877,7 +904,7 @@ static int aqc_write(struct device *dev, enum hwmon_sensor_types type, u32 attr,
 			val = clamp_val(val, -15000, 15000) / 10;
 			ret =
 			    aqc_set_ctrl_val(priv, priv->temp_ctrl_offset +
-					     channel * AQC_SENSOR_SIZE, val);
+					     channel * AQC_SENSOR_SIZE, val, AQC_BE16);
 			if (ret < 0)
 				return ret;
 			break;
@@ -889,7 +916,8 @@ static int aqc_write(struct device *dev, enum hwmon_sensor_types type, u32 attr,
 		switch (attr) {
 		case hwmon_fan_pulses:
 			val = clamp_val(val, 10, 1000);
-			ret = aqc_set_ctrl_val(priv, priv->flow_pulses_ctrl_offset, val);
+			ret = aqc_set_ctrl_val(priv, priv->flow_pulses_ctrl_offset,
+					       val, AQC_BE16);
 			if (ret < 0)
 				return ret;
 			break;
@@ -906,7 +934,7 @@ static int aqc_write(struct device *dev, enum hwmon_sensor_types type, u32 attr,
 					return pwm_value;
 
 				ret = aqc_set_ctrl_val(priv, priv->fan_ctrl_offsets[channel],
-						       pwm_value);
+						       pwm_value, AQC_BE16);
 				if (ret < 0)
 					return ret;
 			}
-- 
2.39.1

Re: [PATCH v2 1/6] hwmon: (aquacomputer_d5next) Support one byte control values

[Guenter Roeck]

On Tue, Feb 14, 2023 at 11:02:16PM +0100, Leonard Anderweit wrote:
> Add support for one byte control values. This extends aqc_set_ctrl_val() and
> aqc_get_ctrl_val() with a type. Currently supported types are AQC_8 (one byte)
> and AQC_BE16 (two bytes big endian). More types will be added in the future.
> 
> Signed-off-by: Leonard Anderweit <leonard.anderweit@gmail.com>

Applied. In the future, please make sure that the line length in the
description has no more than 75 columns.

Thanks,
Guenter

> ---
>  drivers/hwmon/aquacomputer_d5next.c | 48 +++++++++++++++++++++++------
>  1 file changed, 38 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/hwmon/aquacomputer_d5next.c b/drivers/hwmon/aquacomputer_d5next.c
> index 12682a610ce7..babfd998e70c 100644
> --- a/drivers/hwmon/aquacomputer_d5next.c
> +++ b/drivers/hwmon/aquacomputer_d5next.c
> @@ -70,6 +70,10 @@ static u8 secondary_ctrl_report[] = {
>  /* Report IDs for legacy devices */
>  #define POWERADJUST3_STATUS_REPORT_ID	0x03
>  
> +/* Data types for reading and writing control reports */
> +#define AQC_8		0
> +#define AQC_BE16	1
> +
>  /* Info, sensor sizes and offsets for most Aquacomputer devices */
>  #define AQC_SERIAL_START		0x3
>  #define AQC_FIRMWARE_VERSION		0xD
> @@ -544,7 +548,7 @@ static int aqc_send_ctrl_data(struct aqc_data *priv)
>  }
>  
>  /* Refreshes the control buffer and stores value at offset in val */
> -static int aqc_get_ctrl_val(struct aqc_data *priv, int offset, long *val)
> +static int aqc_get_ctrl_val(struct aqc_data *priv, int offset, long *val, int type)
>  {
>  	int ret;
>  
> @@ -554,14 +558,23 @@ static int aqc_get_ctrl_val(struct aqc_data *priv, int offset, long *val)
>  	if (ret < 0)
>  		goto unlock_and_return;
>  
> -	*val = (s16)get_unaligned_be16(priv->buffer + offset);
> +	switch (type) {
> +	case AQC_BE16:
> +		*val = (s16)get_unaligned_be16(priv->buffer + offset);
> +		break;
> +	case AQC_8:
> +		*val = priv->buffer[offset];
> +		break;
> +	default:
> +		ret = -EINVAL;
> +	}
>  
>  unlock_and_return:
>  	mutex_unlock(&priv->mutex);
>  	return ret;
>  }
>  
> -static int aqc_set_ctrl_val(struct aqc_data *priv, int offset, long val)
> +static int aqc_set_ctrl_val(struct aqc_data *priv, int offset, long val, int type)
>  {
>  	int ret;
>  
> @@ -571,7 +584,19 @@ static int aqc_set_ctrl_val(struct aqc_data *priv, int offset, long val)
>  	if (ret < 0)
>  		goto unlock_and_return;
>  
> -	put_unaligned_be16((s16)val, priv->buffer + offset);
> +	switch (type) {
> +	case AQC_BE16:
> +		put_unaligned_be16((s16)val, priv->buffer + offset);
> +		break;
> +	case AQC_8:
> +		priv->buffer[offset] = (u8)val;
> +		break;
> +	default:
> +		ret = -EINVAL;
> +	}
> +
> +	if (ret < 0)
> +		goto unlock_and_return;
>  
>  	ret = aqc_send_ctrl_data(priv);
>  
> @@ -775,7 +800,7 @@ static int aqc_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
>  		case hwmon_temp_offset:
>  			ret =
>  			    aqc_get_ctrl_val(priv, priv->temp_ctrl_offset +
> -					     channel * AQC_SENSOR_SIZE, val);
> +					     channel * AQC_SENSOR_SIZE, val, AQC_BE16);
>  			if (ret < 0)
>  				return ret;
>  
> @@ -791,7 +816,8 @@ static int aqc_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
>  			*val = priv->speed_input[channel];
>  			break;
>  		case hwmon_fan_pulses:
> -			ret = aqc_get_ctrl_val(priv, priv->flow_pulses_ctrl_offset, val);
> +			ret = aqc_get_ctrl_val(priv, priv->flow_pulses_ctrl_offset,
> +					       val, AQC_BE16);
>  			if (ret < 0)
>  				return ret;
>  			break;
> @@ -804,7 +830,8 @@ static int aqc_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
>  		break;
>  	case hwmon_pwm:
>  		if (priv->fan_ctrl_offsets) {
> -			ret = aqc_get_ctrl_val(priv, priv->fan_ctrl_offsets[channel], val);
> +			ret = aqc_get_ctrl_val(priv, priv->fan_ctrl_offsets[channel],
> +					       val, AQC_BE16);
>  			if (ret < 0)
>  				return ret;
>  
> @@ -877,7 +904,7 @@ static int aqc_write(struct device *dev, enum hwmon_sensor_types type, u32 attr,
>  			val = clamp_val(val, -15000, 15000) / 10;
>  			ret =
>  			    aqc_set_ctrl_val(priv, priv->temp_ctrl_offset +
> -					     channel * AQC_SENSOR_SIZE, val);
> +					     channel * AQC_SENSOR_SIZE, val, AQC_BE16);
>  			if (ret < 0)
>  				return ret;
>  			break;
> @@ -889,7 +916,8 @@ static int aqc_write(struct device *dev, enum hwmon_sensor_types type, u32 attr,
>  		switch (attr) {
>  		case hwmon_fan_pulses:
>  			val = clamp_val(val, 10, 1000);
> -			ret = aqc_set_ctrl_val(priv, priv->flow_pulses_ctrl_offset, val);
> +			ret = aqc_set_ctrl_val(priv, priv->flow_pulses_ctrl_offset,
> +					       val, AQC_BE16);
>  			if (ret < 0)
>  				return ret;
>  			break;
> @@ -906,7 +934,7 @@ static int aqc_write(struct device *dev, enum hwmon_sensor_types type, u32 attr,
>  					return pwm_value;
>  
>  				ret = aqc_set_ctrl_val(priv, priv->fan_ctrl_offsets[channel],
> -						       pwm_value);
> +						       pwm_value, AQC_BE16);
>  				if (ret < 0)
>  					return ret;
>  			}

[PATCH v2 2/6] hwmon: (aquacomputer_d5next) Support writing multiple control values at once

[Leonard Anderweit]

Add new function aqc_set_ctrl_vals() to support changing multiple control
values at once while sending only one control report.

Signed-off-by: Leonard Anderweit <leonard.anderweit@gmail.com>
---
 drivers/hwmon/aquacomputer_d5next.c | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/drivers/hwmon/aquacomputer_d5next.c b/drivers/hwmon/aquacomputer_d5next.c
index babfd998e70c..f0c036d38e91 100644
--- a/drivers/hwmon/aquacomputer_d5next.c
+++ b/drivers/hwmon/aquacomputer_d5next.c
@@ -574,9 +574,9 @@ static int aqc_get_ctrl_val(struct aqc_data *priv, int offset, long *val, int ty
 	return ret;
 }
 
-static int aqc_set_ctrl_val(struct aqc_data *priv, int offset, long val, int type)
+static int aqc_set_ctrl_vals(struct aqc_data *priv, int *offsets, long *vals, int *types, int len)
 {
-	int ret;
+	int ret, i;
 
 	mutex_lock(&priv->mutex);
 
@@ -584,15 +584,17 @@ static int aqc_set_ctrl_val(struct aqc_data *priv, int offset, long val, int typ
 	if (ret < 0)
 		goto unlock_and_return;
 
-	switch (type) {
-	case AQC_BE16:
-		put_unaligned_be16((s16)val, priv->buffer + offset);
-		break;
-	case AQC_8:
-		priv->buffer[offset] = (u8)val;
-		break;
-	default:
-		ret = -EINVAL;
+	for (i = 0; i < len; i++) {
+		switch (types[i]) {
+		case AQC_BE16:
+			put_unaligned_be16((s16)vals[i], priv->buffer + offsets[i]);
+			break;
+		case AQC_8:
+			priv->buffer[offsets[i]] = (u8)vals[i];
+			break;
+		default:
+			ret = -EINVAL;
+		}
 	}
 
 	if (ret < 0)
@@ -605,6 +607,11 @@ static int aqc_set_ctrl_val(struct aqc_data *priv, int offset, long val, int typ
 	return ret;
 }
 
+static int aqc_set_ctrl_val(struct aqc_data *priv, int offset, long val, int type)
+{
+	return aqc_set_ctrl_vals(priv, &offset, &val, &type, 1);
+}
+
 static umode_t aqc_is_visible(const void *data, enum hwmon_sensor_types type, u32 attr, int channel)
 {
 	const struct aqc_data *priv = data;
-- 
2.39.1

Re: [PATCH v2 2/6] hwmon: (aquacomputer_d5next) Support writing multiple control values at once

[Guenter Roeck]

On Tue, Feb 14, 2023 at 11:02:17PM +0100, Leonard Anderweit wrote:
> Add new function aqc_set_ctrl_vals() to support changing multiple control
> values at once while sending only one control report.
> 
> Signed-off-by: Leonard Anderweit <leonard.anderweit@gmail.com>

Applied.

Thanks,
Guenter

> ---
>  drivers/hwmon/aquacomputer_d5next.c | 29 ++++++++++++++++++-----------
>  1 file changed, 18 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/hwmon/aquacomputer_d5next.c b/drivers/hwmon/aquacomputer_d5next.c
> index babfd998e70c..f0c036d38e91 100644
> --- a/drivers/hwmon/aquacomputer_d5next.c
> +++ b/drivers/hwmon/aquacomputer_d5next.c
> @@ -574,9 +574,9 @@ static int aqc_get_ctrl_val(struct aqc_data *priv, int offset, long *val, int ty
>  	return ret;
>  }
>  
> -static int aqc_set_ctrl_val(struct aqc_data *priv, int offset, long val, int type)
> +static int aqc_set_ctrl_vals(struct aqc_data *priv, int *offsets, long *vals, int *types, int len)
>  {
> -	int ret;
> +	int ret, i;
>  
>  	mutex_lock(&priv->mutex);
>  
> @@ -584,15 +584,17 @@ static int aqc_set_ctrl_val(struct aqc_data *priv, int offset, long val, int typ
>  	if (ret < 0)
>  		goto unlock_and_return;
>  
> -	switch (type) {
> -	case AQC_BE16:
> -		put_unaligned_be16((s16)val, priv->buffer + offset);
> -		break;
> -	case AQC_8:
> -		priv->buffer[offset] = (u8)val;
> -		break;
> -	default:
> -		ret = -EINVAL;
> +	for (i = 0; i < len; i++) {
> +		switch (types[i]) {
> +		case AQC_BE16:
> +			put_unaligned_be16((s16)vals[i], priv->buffer + offsets[i]);
> +			break;
> +		case AQC_8:
> +			priv->buffer[offsets[i]] = (u8)vals[i];
> +			break;
> +		default:
> +			ret = -EINVAL;
> +		}
>  	}
>  
>  	if (ret < 0)
> @@ -605,6 +607,11 @@ static int aqc_set_ctrl_val(struct aqc_data *priv, int offset, long val, int typ
>  	return ret;
>  }
>  
> +static int aqc_set_ctrl_val(struct aqc_data *priv, int offset, long val, int type)
> +{
> +	return aqc_set_ctrl_vals(priv, &offset, &val, &type, 1);
> +}
> +
>  static umode_t aqc_is_visible(const void *data, enum hwmon_sensor_types type, u32 attr, int channel)
>  {
>  	const struct aqc_data *priv = data;

[PATCH v2 3/6] hwmon: (aquacomputer_d5next) Device dependent control report settings

[Leonard Anderweit]

Add device dependent control report id, secondary control report id, secondary
control report size and secondary control report for devices which need
different control report settings. All currently supported devices use the same
values.

Signed-off-by: Leonard Anderweit <leonard.anderweit@gmail.com>
---
 drivers/hwmon/aquacomputer_d5next.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/hwmon/aquacomputer_d5next.c b/drivers/hwmon/aquacomputer_d5next.c
index f0c036d38e91..535d2fc0e55c 100644
--- a/drivers/hwmon/aquacomputer_d5next.c
+++ b/drivers/hwmon/aquacomputer_d5next.c
@@ -441,6 +441,10 @@ struct aqc_data {
 	const char *name;
 
 	int status_report_id;	/* Used for legacy devices, report is stored in buffer */
+	int ctrl_report_id;
+	int secondary_ctrl_report_id;
+	int secondary_ctrl_report_size;
+	u8 *secondary_ctrl_report;
 
 	int buffer_size;
 	u8 *buffer;
@@ -513,7 +517,7 @@ static int aqc_get_ctrl_data(struct aqc_data *priv)
 	int ret;
 
 	memset(priv->buffer, 0x00, priv->buffer_size);
-	ret = hid_hw_raw_request(priv->hdev, CTRL_REPORT_ID, priv->buffer, priv->buffer_size,
+	ret = hid_hw_raw_request(priv->hdev, priv->ctrl_report_id, priv->buffer, priv->buffer_size,
 				 HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
 	if (ret < 0)
 		ret = -ENODATA;
@@ -535,15 +539,15 @@ static int aqc_send_ctrl_data(struct aqc_data *priv)
 	put_unaligned_be16(checksum, priv->buffer + priv->checksum_offset);
 
 	/* Send the patched up report back to the device */
-	ret = hid_hw_raw_request(priv->hdev, CTRL_REPORT_ID, priv->buffer, priv->buffer_size,
+	ret = hid_hw_raw_request(priv->hdev, priv->ctrl_report_id, priv->buffer, priv->buffer_size,
 				 HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
 	if (ret < 0)
 		return ret;
 
 	/* The official software sends this report after every change, so do it here as well */
-	ret = hid_hw_raw_request(priv->hdev, SECONDARY_CTRL_REPORT_ID, secondary_ctrl_report,
-				 SECONDARY_CTRL_REPORT_SIZE, HID_FEATURE_REPORT,
-				 HID_REQ_SET_REPORT);
+	ret = hid_hw_raw_request(priv->hdev, priv->secondary_ctrl_report_id,
+				 priv->secondary_ctrl_report, priv->secondary_ctrl_report_size,
+				 HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
 	return ret;
 }
 
@@ -1447,6 +1451,11 @@ static int aqc_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		priv->serial_number_start_offset = AQC_SERIAL_START;
 		priv->firmware_version_offset = AQC_FIRMWARE_VERSION;
 
+		priv->ctrl_report_id = CTRL_REPORT_ID;
+		priv->secondary_ctrl_report_id = SECONDARY_CTRL_REPORT_ID;
+		priv->secondary_ctrl_report_size = SECONDARY_CTRL_REPORT_SIZE;
+		priv->secondary_ctrl_report = secondary_ctrl_report;
+
 		if (priv->kind == aquastreamult)
 			priv->fan_structure = &aqc_aquastreamult_fan_structure;
 		else
-- 
2.39.1

Re: [PATCH v2 3/6] hwmon: (aquacomputer_d5next) Device dependent control report settings

[Guenter Roeck]

On Tue, Feb 14, 2023 at 11:02:18PM +0100, Leonard Anderweit wrote:
> Add device dependent control report id, secondary control report id, secondary
> control report size and secondary control report for devices which need
> different control report settings. All currently supported devices use the same
> values.
> 
> Signed-off-by: Leonard Anderweit <leonard.anderweit@gmail.com>

Applied. Same comment as before - please check line length in description.

Thanks,
Guenter

> ---
>  drivers/hwmon/aquacomputer_d5next.c | 19 ++++++++++++++-----
>  1 file changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/hwmon/aquacomputer_d5next.c b/drivers/hwmon/aquacomputer_d5next.c
> index f0c036d38e91..535d2fc0e55c 100644
> --- a/drivers/hwmon/aquacomputer_d5next.c
> +++ b/drivers/hwmon/aquacomputer_d5next.c
> @@ -441,6 +441,10 @@ struct aqc_data {
>  	const char *name;
>  
>  	int status_report_id;	/* Used for legacy devices, report is stored in buffer */
> +	int ctrl_report_id;
> +	int secondary_ctrl_report_id;
> +	int secondary_ctrl_report_size;
> +	u8 *secondary_ctrl_report;
>  
>  	int buffer_size;
>  	u8 *buffer;
> @@ -513,7 +517,7 @@ static int aqc_get_ctrl_data(struct aqc_data *priv)
>  	int ret;
>  
>  	memset(priv->buffer, 0x00, priv->buffer_size);
> -	ret = hid_hw_raw_request(priv->hdev, CTRL_REPORT_ID, priv->buffer, priv->buffer_size,
> +	ret = hid_hw_raw_request(priv->hdev, priv->ctrl_report_id, priv->buffer, priv->buffer_size,
>  				 HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
>  	if (ret < 0)
>  		ret = -ENODATA;
> @@ -535,15 +539,15 @@ static int aqc_send_ctrl_data(struct aqc_data *priv)
>  	put_unaligned_be16(checksum, priv->buffer + priv->checksum_offset);
>  
>  	/* Send the patched up report back to the device */
> -	ret = hid_hw_raw_request(priv->hdev, CTRL_REPORT_ID, priv->buffer, priv->buffer_size,
> +	ret = hid_hw_raw_request(priv->hdev, priv->ctrl_report_id, priv->buffer, priv->buffer_size,
>  				 HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
>  	if (ret < 0)
>  		return ret;
>  
>  	/* The official software sends this report after every change, so do it here as well */
> -	ret = hid_hw_raw_request(priv->hdev, SECONDARY_CTRL_REPORT_ID, secondary_ctrl_report,
> -				 SECONDARY_CTRL_REPORT_SIZE, HID_FEATURE_REPORT,
> -				 HID_REQ_SET_REPORT);
> +	ret = hid_hw_raw_request(priv->hdev, priv->secondary_ctrl_report_id,
> +				 priv->secondary_ctrl_report, priv->secondary_ctrl_report_size,
> +				 HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
>  	return ret;
>  }
>  
> @@ -1447,6 +1451,11 @@ static int aqc_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  		priv->serial_number_start_offset = AQC_SERIAL_START;
>  		priv->firmware_version_offset = AQC_FIRMWARE_VERSION;
>  
> +		priv->ctrl_report_id = CTRL_REPORT_ID;
> +		priv->secondary_ctrl_report_id = SECONDARY_CTRL_REPORT_ID;
> +		priv->secondary_ctrl_report_size = SECONDARY_CTRL_REPORT_SIZE;
> +		priv->secondary_ctrl_report = secondary_ctrl_report;
> +
>  		if (priv->kind == aquastreamult)
>  			priv->fan_structure = &aqc_aquastreamult_fan_structure;
>  		else

[PATCH v2 4/6] hwmon: (aquacomputer_d5next) Add infrastructure for Aquaero control reports

[Leonard Anderweit]

Add information on the Aquacomputer Aquaero control report and disable the
control report checksum for Aquaero. The Aquaero does not use the checksum so
it must be disabled to avoid overwriting the last two bytes of the control
report.

Signed-off-by: Leonard Anderweit <leonard.anderweit@gmail.com>
---
 drivers/hwmon/aquacomputer_d5next.c | 31 ++++++++++++++++++++++++-----
 1 file changed, 26 insertions(+), 5 deletions(-)

diff --git a/drivers/hwmon/aquacomputer_d5next.c b/drivers/hwmon/aquacomputer_d5next.c
index 535d2fc0e55c..eb185318098a 100644
--- a/drivers/hwmon/aquacomputer_d5next.c
+++ b/drivers/hwmon/aquacomputer_d5next.c
@@ -56,6 +56,7 @@ static const char *const aqc_device_names[] = {
 #define SERIAL_PART_OFFSET		2
 
 #define CTRL_REPORT_ID			0x03
+#define AQUAERO_CTRL_REPORT_ID		0x0b
 
 /* The HID report that the official software always sends
  * after writing values, currently same for all devices
@@ -67,6 +68,14 @@ static u8 secondary_ctrl_report[] = {
 	0x02, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x34, 0xC6
 };
 
+/* Secondary HID report values for Aquaero */
+#define AQUAERO_SECONDARY_CTRL_REPORT_ID	0x06
+#define AQUAERO_SECONDARY_CTRL_REPORT_SIZE	0x07
+
+static u8 aquaero_secondary_ctrl_report[] = {
+	0x06, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00
+};
+
 /* Report IDs for legacy devices */
 #define POWERADJUST3_STATUS_REPORT_ID	0x03
 
@@ -94,6 +103,7 @@ static u8 secondary_ctrl_report[] = {
 #define AQUAERO_NUM_VIRTUAL_SENSORS		8
 #define AQUAERO_NUM_CALC_VIRTUAL_SENSORS	4
 #define AQUAERO_NUM_FLOW_SENSORS		2
+#define AQUAERO_CTRL_REPORT_SIZE		0xa93
 
 /* Sensor report offsets for Aquaero fan controllers */
 #define AQUAERO_SENSOR_START			0x65
@@ -531,12 +541,16 @@ static int aqc_send_ctrl_data(struct aqc_data *priv)
 	int ret;
 	u16 checksum;
 
-	/* Init and xorout value for CRC-16/USB is 0xffff */
-	checksum = crc16(0xffff, priv->buffer + priv->checksum_start, priv->checksum_length);
-	checksum ^= 0xffff;
+	/* Checksum is not needed for Aquaero */
+	if (priv->kind != aquaero) {
+		/* Init and xorout value for CRC-16/USB is 0xffff */
+		checksum = crc16(0xffff, priv->buffer + priv->checksum_start,
+				 priv->checksum_length);
+		checksum ^= 0xffff;
 
-	/* Place the new checksum at the end of the report */
-	put_unaligned_be16(checksum, priv->buffer + priv->checksum_offset);
+		/* Place the new checksum at the end of the report */
+		put_unaligned_be16(checksum, priv->buffer + priv->checksum_offset);
+	}
 
 	/* Send the patched up report back to the device */
 	ret = hid_hw_raw_request(priv->hdev, priv->ctrl_report_id, priv->buffer, priv->buffer_size,
@@ -1280,6 +1294,8 @@ static int aqc_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		priv->num_flow_sensors = AQUAERO_NUM_FLOW_SENSORS;
 		priv->flow_sensors_start_offset = AQUAERO_FLOW_SENSORS_START;
 
+		priv->buffer_size = AQUAERO_CTRL_REPORT_SIZE;
+
 		priv->temp_label = label_temp_sensors;
 		priv->virtual_temp_label = label_virtual_temp_sensors;
 		priv->calc_virt_temp_label = label_aquaero_calc_temp_sensors;
@@ -1443,6 +1459,11 @@ static int aqc_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		priv->firmware_version_offset = AQUAERO_FIRMWARE_VERSION;
 
 		priv->fan_structure = &aqc_aquaero_fan_structure;
+
+		priv->ctrl_report_id = AQUAERO_CTRL_REPORT_ID;
+		priv->secondary_ctrl_report_id = AQUAERO_SECONDARY_CTRL_REPORT_ID;
+		priv->secondary_ctrl_report_size = AQUAERO_SECONDARY_CTRL_REPORT_SIZE;
+		priv->secondary_ctrl_report = aquaero_secondary_ctrl_report;
 		break;
 	case poweradjust3:
 		priv->status_report_id = POWERADJUST3_STATUS_REPORT_ID;
-- 
2.39.1

Re: [PATCH v2 4/6] hwmon: (aquacomputer_d5next) Add infrastructure for Aquaero control reports

[Guenter Roeck]

On Tue, Feb 14, 2023 at 11:02:19PM +0100, Leonard Anderweit wrote:
> Add information on the Aquacomputer Aquaero control report and disable the
> control report checksum for Aquaero. The Aquaero does not use the checksum so
> it must be disabled to avoid overwriting the last two bytes of the control
> report.
> 
> Signed-off-by: Leonard Anderweit <leonard.anderweit@gmail.com>

Applied. And again:

WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#85:
control report checksum for Aquaero. The Aquaero does not use the checksum so

Please keep in mind that you are causing extra work for me.

Guenter

> ---
>  drivers/hwmon/aquacomputer_d5next.c | 31 ++++++++++++++++++++++++-----
>  1 file changed, 26 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/hwmon/aquacomputer_d5next.c b/drivers/hwmon/aquacomputer_d5next.c
> index 535d2fc0e55c..eb185318098a 100644
> --- a/drivers/hwmon/aquacomputer_d5next.c
> +++ b/drivers/hwmon/aquacomputer_d5next.c
> @@ -56,6 +56,7 @@ static const char *const aqc_device_names[] = {
>  #define SERIAL_PART_OFFSET		2
>  
>  #define CTRL_REPORT_ID			0x03
> +#define AQUAERO_CTRL_REPORT_ID		0x0b
>  
>  /* The HID report that the official software always sends
>   * after writing values, currently same for all devices
> @@ -67,6 +68,14 @@ static u8 secondary_ctrl_report[] = {
>  	0x02, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x34, 0xC6
>  };
>  
> +/* Secondary HID report values for Aquaero */
> +#define AQUAERO_SECONDARY_CTRL_REPORT_ID	0x06
> +#define AQUAERO_SECONDARY_CTRL_REPORT_SIZE	0x07
> +
> +static u8 aquaero_secondary_ctrl_report[] = {
> +	0x06, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00
> +};
> +
>  /* Report IDs for legacy devices */
>  #define POWERADJUST3_STATUS_REPORT_ID	0x03
>  
> @@ -94,6 +103,7 @@ static u8 secondary_ctrl_report[] = {
>  #define AQUAERO_NUM_VIRTUAL_SENSORS		8
>  #define AQUAERO_NUM_CALC_VIRTUAL_SENSORS	4
>  #define AQUAERO_NUM_FLOW_SENSORS		2
> +#define AQUAERO_CTRL_REPORT_SIZE		0xa93
>  
>  /* Sensor report offsets for Aquaero fan controllers */
>  #define AQUAERO_SENSOR_START			0x65
> @@ -531,12 +541,16 @@ static int aqc_send_ctrl_data(struct aqc_data *priv)
>  	int ret;
>  	u16 checksum;
>  
> -	/* Init and xorout value for CRC-16/USB is 0xffff */
> -	checksum = crc16(0xffff, priv->buffer + priv->checksum_start, priv->checksum_length);
> -	checksum ^= 0xffff;
> +	/* Checksum is not needed for Aquaero */
> +	if (priv->kind != aquaero) {
> +		/* Init and xorout value for CRC-16/USB is 0xffff */
> +		checksum = crc16(0xffff, priv->buffer + priv->checksum_start,
> +				 priv->checksum_length);
> +		checksum ^= 0xffff;
>  
> -	/* Place the new checksum at the end of the report */
> -	put_unaligned_be16(checksum, priv->buffer + priv->checksum_offset);
> +		/* Place the new checksum at the end of the report */
> +		put_unaligned_be16(checksum, priv->buffer + priv->checksum_offset);
> +	}
>  
>  	/* Send the patched up report back to the device */
>  	ret = hid_hw_raw_request(priv->hdev, priv->ctrl_report_id, priv->buffer, priv->buffer_size,
> @@ -1280,6 +1294,8 @@ static int aqc_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  		priv->num_flow_sensors = AQUAERO_NUM_FLOW_SENSORS;
>  		priv->flow_sensors_start_offset = AQUAERO_FLOW_SENSORS_START;
>  
> +		priv->buffer_size = AQUAERO_CTRL_REPORT_SIZE;
> +
>  		priv->temp_label = label_temp_sensors;
>  		priv->virtual_temp_label = label_virtual_temp_sensors;
>  		priv->calc_virt_temp_label = label_aquaero_calc_temp_sensors;
> @@ -1443,6 +1459,11 @@ static int aqc_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  		priv->firmware_version_offset = AQUAERO_FIRMWARE_VERSION;
>  
>  		priv->fan_structure = &aqc_aquaero_fan_structure;
> +
> +		priv->ctrl_report_id = AQUAERO_CTRL_REPORT_ID;
> +		priv->secondary_ctrl_report_id = AQUAERO_SECONDARY_CTRL_REPORT_ID;
> +		priv->secondary_ctrl_report_size = AQUAERO_SECONDARY_CTRL_REPORT_SIZE;
> +		priv->secondary_ctrl_report = aquaero_secondary_ctrl_report;
>  		break;
>  	case poweradjust3:
>  		priv->status_report_id = POWERADJUST3_STATUS_REPORT_ID;

[PATCH v2 5/6] hwmon: (aquacomputer_d5next) Add temperature offset control for Aquaero

[Leonard Anderweit]

Adds control over the Aquacomputer Aquaero temperature offset for all eight
temperature sensors.

Signed-off-by: Leonard Anderweit <leonard.anderweit@gmail.com>
---
 Documentation/hwmon/aquacomputer_d5next.rst |  4 ++--
 drivers/hwmon/aquacomputer_d5next.c         | 12 ++++++++----
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/Documentation/hwmon/aquacomputer_d5next.rst b/Documentation/hwmon/aquacomputer_d5next.rst
index 7d0d015b1a52..618c826093a2 100644
--- a/Documentation/hwmon/aquacomputer_d5next.rst
+++ b/Documentation/hwmon/aquacomputer_d5next.rst
@@ -25,7 +25,7 @@ communicate through proprietary USB HID protocols.
 
 The Aquaero devices expose eight physical, eight virtual and four calculated
 virtual temperature sensors, as well as two flow sensors. The fans expose their
-speed (in RPM), power, voltage and current.
+speed (in RPM), power, voltage and current. Temperature offsets can be controlled.
 
 For the D5 Next pump, available sensors are pump and fan speed, power, voltage
 and current, as well as coolant temperature and eight virtual temp sensors. Also
@@ -75,7 +75,7 @@ Sysfs entries
 
 ================ ==============================================================
 temp[1-20]_input Physical/virtual temperature sensors (in millidegrees Celsius)
-temp[1-4]_offset Temperature sensor correction offset (in millidegrees Celsius)
+temp[1-8]_offset Temperature sensor correction offset (in millidegrees Celsius)
 fan[1-8]_input   Pump/fan speed (in RPM) / Flow speed (in dL/h)
 fan5_pulses      Quadro flow sensor pulses
 power[1-8]_input Pump/fan power (in micro Watts)
diff --git a/drivers/hwmon/aquacomputer_d5next.c b/drivers/hwmon/aquacomputer_d5next.c
index eb185318098a..61c1ffcd8f3b 100644
--- a/drivers/hwmon/aquacomputer_d5next.c
+++ b/drivers/hwmon/aquacomputer_d5next.c
@@ -116,6 +116,9 @@ static u8 aquaero_secondary_ctrl_report[] = {
 #define AQUAERO_FAN_SPEED_OFFSET		0x00
 static u16 aquaero_sensor_fan_offsets[] = { 0x167, 0x173, 0x17f, 0x18B };
 
+/* Control report offsets for the Aquaero fan controllers */
+#define AQUAERO_TEMP_CTRL_OFFSET	0xdb
+
 /* Specs of the D5 Next pump */
 #define D5NEXT_NUM_FANS			2
 #define D5NEXT_NUM_SENSORS		1
@@ -988,10 +991,10 @@ static const struct hwmon_channel_info *aqc_info[] = {
 			   HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_OFFSET,
 			   HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_OFFSET,
 			   HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_OFFSET,
-			   HWMON_T_INPUT | HWMON_T_LABEL,
-			   HWMON_T_INPUT | HWMON_T_LABEL,
-			   HWMON_T_INPUT | HWMON_T_LABEL,
-			   HWMON_T_INPUT | HWMON_T_LABEL,
+			   HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_OFFSET,
+			   HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_OFFSET,
+			   HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_OFFSET,
+			   HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_OFFSET,
 			   HWMON_T_INPUT | HWMON_T_LABEL,
 			   HWMON_T_INPUT | HWMON_T_LABEL,
 			   HWMON_T_INPUT | HWMON_T_LABEL,
@@ -1295,6 +1298,7 @@ static int aqc_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		priv->flow_sensors_start_offset = AQUAERO_FLOW_SENSORS_START;
 
 		priv->buffer_size = AQUAERO_CTRL_REPORT_SIZE;
+		priv->temp_ctrl_offset = AQUAERO_TEMP_CTRL_OFFSET;
 
 		priv->temp_label = label_temp_sensors;
 		priv->virtual_temp_label = label_virtual_temp_sensors;
-- 
2.39.1

Re: [PATCH v2 5/6] hwmon: (aquacomputer_d5next) Add temperature offset control for Aquaero

[Guenter Roeck]

On Tue, Feb 14, 2023 at 11:02:20PM +0100, Leonard Anderweit wrote:
> Adds control over the Aquacomputer Aquaero temperature offset for all eight
> temperature sensors.
> 
> Signed-off-by: Leonard Anderweit <leonard.anderweit@gmail.com>

Applied.

Thanks,
Guenter

> ---
>  Documentation/hwmon/aquacomputer_d5next.rst |  4 ++--
>  drivers/hwmon/aquacomputer_d5next.c         | 12 ++++++++----
>  2 files changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/hwmon/aquacomputer_d5next.rst b/Documentation/hwmon/aquacomputer_d5next.rst
> index 7d0d015b1a52..618c826093a2 100644
> --- a/Documentation/hwmon/aquacomputer_d5next.rst
> +++ b/Documentation/hwmon/aquacomputer_d5next.rst
> @@ -25,7 +25,7 @@ communicate through proprietary USB HID protocols.
>  
>  The Aquaero devices expose eight physical, eight virtual and four calculated
>  virtual temperature sensors, as well as two flow sensors. The fans expose their
> -speed (in RPM), power, voltage and current.
> +speed (in RPM), power, voltage and current. Temperature offsets can be controlled.
>  
>  For the D5 Next pump, available sensors are pump and fan speed, power, voltage
>  and current, as well as coolant temperature and eight virtual temp sensors. Also
> @@ -75,7 +75,7 @@ Sysfs entries
>  
>  ================ ==============================================================
>  temp[1-20]_input Physical/virtual temperature sensors (in millidegrees Celsius)
> -temp[1-4]_offset Temperature sensor correction offset (in millidegrees Celsius)
> +temp[1-8]_offset Temperature sensor correction offset (in millidegrees Celsius)
>  fan[1-8]_input   Pump/fan speed (in RPM) / Flow speed (in dL/h)
>  fan5_pulses      Quadro flow sensor pulses
>  power[1-8]_input Pump/fan power (in micro Watts)
> diff --git a/drivers/hwmon/aquacomputer_d5next.c b/drivers/hwmon/aquacomputer_d5next.c
> index eb185318098a..61c1ffcd8f3b 100644
> --- a/drivers/hwmon/aquacomputer_d5next.c
> +++ b/drivers/hwmon/aquacomputer_d5next.c
> @@ -116,6 +116,9 @@ static u8 aquaero_secondary_ctrl_report[] = {
>  #define AQUAERO_FAN_SPEED_OFFSET		0x00
>  static u16 aquaero_sensor_fan_offsets[] = { 0x167, 0x173, 0x17f, 0x18B };
>  
> +/* Control report offsets for the Aquaero fan controllers */
> +#define AQUAERO_TEMP_CTRL_OFFSET	0xdb
> +
>  /* Specs of the D5 Next pump */
>  #define D5NEXT_NUM_FANS			2
>  #define D5NEXT_NUM_SENSORS		1
> @@ -988,10 +991,10 @@ static const struct hwmon_channel_info *aqc_info[] = {
>  			   HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_OFFSET,
>  			   HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_OFFSET,
>  			   HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_OFFSET,
> -			   HWMON_T_INPUT | HWMON_T_LABEL,
> -			   HWMON_T_INPUT | HWMON_T_LABEL,
> -			   HWMON_T_INPUT | HWMON_T_LABEL,
> -			   HWMON_T_INPUT | HWMON_T_LABEL,
> +			   HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_OFFSET,
> +			   HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_OFFSET,
> +			   HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_OFFSET,
> +			   HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_OFFSET,
>  			   HWMON_T_INPUT | HWMON_T_LABEL,
>  			   HWMON_T_INPUT | HWMON_T_LABEL,
>  			   HWMON_T_INPUT | HWMON_T_LABEL,
> @@ -1295,6 +1298,7 @@ static int aqc_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  		priv->flow_sensors_start_offset = AQUAERO_FLOW_SENSORS_START;
>  
>  		priv->buffer_size = AQUAERO_CTRL_REPORT_SIZE;
> +		priv->temp_ctrl_offset = AQUAERO_TEMP_CTRL_OFFSET;
>  
>  		priv->temp_label = label_temp_sensors;
>  		priv->virtual_temp_label = label_virtual_temp_sensors;

[PATCH v2 6/6] hwmon: (aquacomputer_d5next) Add fan PWM control for Aquaero

[Leonard Anderweit]

Add the option to control fan PWM on Aquacomputer Aquaero. The Aquaero is the
most complex Aquacomputer device, control is therefore more complicated then on
already supported devices.
Setting PWM requires multiple steps. First, an internal static PWM controller
is set to the desired PWM value. Second, the fan is set to use that PWM
controller. Last, the minimum and maximum accepted PWM values of the fan are
set to allow all possible PWM values.

Signed-off-by: Leonard Anderweit <leonard.anderweit@gmail.com>
---
 Documentation/hwmon/aquacomputer_d5next.rst |  3 +-
 drivers/hwmon/aquacomputer_d5next.c         | 63 +++++++++++++++++++--
 2 files changed, 60 insertions(+), 6 deletions(-)

diff --git a/Documentation/hwmon/aquacomputer_d5next.rst b/Documentation/hwmon/aquacomputer_d5next.rst
index 618c826093a2..c604d4becb8d 100644
--- a/Documentation/hwmon/aquacomputer_d5next.rst
+++ b/Documentation/hwmon/aquacomputer_d5next.rst
@@ -25,7 +25,8 @@ communicate through proprietary USB HID protocols.
 
 The Aquaero devices expose eight physical, eight virtual and four calculated
 virtual temperature sensors, as well as two flow sensors. The fans expose their
-speed (in RPM), power, voltage and current. Temperature offsets can be controlled.
+speed (in RPM), power, voltage and current. Temperature offsets and fan speeds
+can be controlled.
 
 For the D5 Next pump, available sensors are pump and fan speed, power, voltage
 and current, as well as coolant temperature and eight virtual temp sensors. Also
diff --git a/drivers/hwmon/aquacomputer_d5next.c b/drivers/hwmon/aquacomputer_d5next.c
index 61c1ffcd8f3b..a6cff7f9cac9 100644
--- a/drivers/hwmon/aquacomputer_d5next.c
+++ b/drivers/hwmon/aquacomputer_d5next.c
@@ -104,6 +104,9 @@ static u8 aquaero_secondary_ctrl_report[] = {
 #define AQUAERO_NUM_CALC_VIRTUAL_SENSORS	4
 #define AQUAERO_NUM_FLOW_SENSORS		2
 #define AQUAERO_CTRL_REPORT_SIZE		0xa93
+#define AQUAERO_CTRL_PRESET_ID			0x5c
+#define AQUAERO_CTRL_PRESET_SIZE		0x02
+#define AQUAERO_CTRL_PRESET_START		0x55c
 
 /* Sensor report offsets for Aquaero fan controllers */
 #define AQUAERO_SENSOR_START			0x65
@@ -118,6 +121,10 @@ static u16 aquaero_sensor_fan_offsets[] = { 0x167, 0x173, 0x17f, 0x18B };
 
 /* Control report offsets for the Aquaero fan controllers */
 #define AQUAERO_TEMP_CTRL_OFFSET	0xdb
+#define AQUAERO_FAN_CTRL_MIN_PWR_OFFSET	0x04
+#define AQUAERO_FAN_CTRL_MAX_PWR_OFFSET	0x06
+#define AQUAERO_FAN_CTRL_SRC_OFFSET	0x10
+static u16 aquaero_ctrl_fan_offsets[] = { 0x20c, 0x220, 0x234, 0x248 };
 
 /* Specs of the D5 Next pump */
 #define D5NEXT_NUM_FANS			2
@@ -857,13 +864,22 @@ static int aqc_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
 		*val = priv->power_input[channel];
 		break;
 	case hwmon_pwm:
-		if (priv->fan_ctrl_offsets) {
+		switch (priv->kind) {
+		case aquaero:
+			ret = aqc_get_ctrl_val(priv, AQUAERO_CTRL_PRESET_START + channel * AQUAERO_CTRL_PRESET_SIZE,
+					       val, AQC_BE16);
+			if (ret < 0)
+				return ret;
+			*val = aqc_percent_to_pwm(*val);
+			break;
+		default:
 			ret = aqc_get_ctrl_val(priv, priv->fan_ctrl_offsets[channel],
 					       val, AQC_BE16);
 			if (ret < 0)
 				return ret;
 
 			*val = aqc_percent_to_pwm(ret);
+			break;
 		}
 		break;
 	case hwmon_in:
@@ -922,6 +938,10 @@ static int aqc_write(struct device *dev, enum hwmon_sensor_types type, u32 attr,
 		     long val)
 {
 	int ret, pwm_value;
+	/* Arrays for setting multiple values at once in the control report */
+	int ctrl_values_offsets[4];
+	long ctrl_values[4];
+	int ctrl_values_types[4];
 	struct aqc_data *priv = dev_get_drvdata(dev);
 
 	switch (type) {
@@ -956,15 +976,47 @@ static int aqc_write(struct device *dev, enum hwmon_sensor_types type, u32 attr,
 	case hwmon_pwm:
 		switch (attr) {
 		case hwmon_pwm_input:
-			if (priv->fan_ctrl_offsets) {
-				pwm_value = aqc_pwm_to_percent(val);
-				if (pwm_value < 0)
-					return pwm_value;
+			pwm_value = aqc_pwm_to_percent(val);
+			if (pwm_value < 0)
+				return pwm_value;
 
+			switch (priv->kind) {
+			case aquaero:
+				/* Write pwm value to preset corresponding to the channel */
+				ctrl_values_offsets[0] = AQUAERO_CTRL_PRESET_START +
+				    channel * AQUAERO_CTRL_PRESET_SIZE;
+				ctrl_values[0] = pwm_value;
+				ctrl_values_types[0] = AQC_BE16;
+
+				/* Write preset number in fan control source */
+				ctrl_values_offsets[1] = priv->fan_ctrl_offsets[channel] +
+				    AQUAERO_FAN_CTRL_SRC_OFFSET;
+				ctrl_values[1] = AQUAERO_CTRL_PRESET_ID + channel;
+				ctrl_values_types[1] = AQC_BE16;
+
+				/* Set minimum power to 0 to allow the fan to turn off */
+				ctrl_values_offsets[2] = priv->fan_ctrl_offsets[channel] +
+				    AQUAERO_FAN_CTRL_MIN_PWR_OFFSET;
+				ctrl_values[2] = 0;
+				ctrl_values_types[2] = AQC_BE16;
+
+				/* Set maximum power to 255 to allow the fan to reach max speed */
+				ctrl_values_offsets[3] = priv->fan_ctrl_offsets[channel] +
+				    AQUAERO_FAN_CTRL_MAX_PWR_OFFSET;
+				ctrl_values[3] = aqc_pwm_to_percent(255);
+				ctrl_values_types[3] = AQC_BE16;
+
+				ret = aqc_set_ctrl_vals(priv, ctrl_values_offsets, ctrl_values,
+							ctrl_values_types, 4);
+				if (ret < 0)
+					return ret;
+				break;
+			default:
 				ret = aqc_set_ctrl_val(priv, priv->fan_ctrl_offsets[channel],
 						       pwm_value, AQC_BE16);
 				if (ret < 0)
 					return ret;
+				break;
 			}
 			break;
 		default:
@@ -1287,6 +1339,7 @@ static int aqc_probe(struct hid_device *hdev, const struct hid_device_id *id)
 
 		priv->num_fans = AQUAERO_NUM_FANS;
 		priv->fan_sensor_offsets = aquaero_sensor_fan_offsets;
+		priv->fan_ctrl_offsets = aquaero_ctrl_fan_offsets;
 
 		priv->num_temp_sensors = AQUAERO_NUM_SENSORS;
 		priv->temp_sensor_start_offset = AQUAERO_SENSOR_START;
-- 
2.39.1

Re: [PATCH v2 6/6] hwmon: (aquacomputer_d5next) Add fan PWM control for Aquaero

[Guenter Roeck]

On Tue, Feb 14, 2023 at 11:02:21PM +0100, Leonard Anderweit wrote:
> Add the option to control fan PWM on Aquacomputer Aquaero. The Aquaero is the
> most complex Aquacomputer device, control is therefore more complicated then on
> already supported devices.
> Setting PWM requires multiple steps. First, an internal static PWM controller
> is set to the desired PWM value. Second, the fan is set to use that PWM
> controller. Last, the minimum and maximum accepted PWM values of the fan are
> set to allow all possible PWM values.
> 
> Signed-off-by: Leonard Anderweit <leonard.anderweit@gmail.com>

WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#84:
Add the option to control fan PWM on Aquacomputer Aquaero. The Aquaero is the

WARNING: line length of 116 exceeds 100 columns
#144: FILE: drivers/hwmon/aquacomputer_d5next.c:869:
+			ret = aqc_get_ctrl_val(priv, AQUAERO_CTRL_PRESET_START + channel * AQUAERO_CTRL_PRESET_SIZE,

Fixed up and applied, but, again: You are causing extra work for me.
If I encounter such problems, affected patches end up at the tail of
my review queue. Which happened here, in case you wonder why it took
so long to apply the series.

Guenter

> ---
>  Documentation/hwmon/aquacomputer_d5next.rst |  3 +-
>  drivers/hwmon/aquacomputer_d5next.c         | 63 +++++++++++++++++++--
>  2 files changed, 60 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/hwmon/aquacomputer_d5next.rst b/Documentation/hwmon/aquacomputer_d5next.rst
> index 618c826093a2..c604d4becb8d 100644
> --- a/Documentation/hwmon/aquacomputer_d5next.rst
> +++ b/Documentation/hwmon/aquacomputer_d5next.rst
> @@ -25,7 +25,8 @@ communicate through proprietary USB HID protocols.
>  
>  The Aquaero devices expose eight physical, eight virtual and four calculated
>  virtual temperature sensors, as well as two flow sensors. The fans expose their
> -speed (in RPM), power, voltage and current. Temperature offsets can be controlled.
> +speed (in RPM), power, voltage and current. Temperature offsets and fan speeds
> +can be controlled.
>  
>  For the D5 Next pump, available sensors are pump and fan speed, power, voltage
>  and current, as well as coolant temperature and eight virtual temp sensors. Also
> diff --git a/drivers/hwmon/aquacomputer_d5next.c b/drivers/hwmon/aquacomputer_d5next.c
> index 61c1ffcd8f3b..a6cff7f9cac9 100644
> --- a/drivers/hwmon/aquacomputer_d5next.c
> +++ b/drivers/hwmon/aquacomputer_d5next.c
> @@ -104,6 +104,9 @@ static u8 aquaero_secondary_ctrl_report[] = {
>  #define AQUAERO_NUM_CALC_VIRTUAL_SENSORS	4
>  #define AQUAERO_NUM_FLOW_SENSORS		2
>  #define AQUAERO_CTRL_REPORT_SIZE		0xa93
> +#define AQUAERO_CTRL_PRESET_ID			0x5c
> +#define AQUAERO_CTRL_PRESET_SIZE		0x02
> +#define AQUAERO_CTRL_PRESET_START		0x55c
>  
>  /* Sensor report offsets for Aquaero fan controllers */
>  #define AQUAERO_SENSOR_START			0x65
> @@ -118,6 +121,10 @@ static u16 aquaero_sensor_fan_offsets[] = { 0x167, 0x173, 0x17f, 0x18B };
>  
>  /* Control report offsets for the Aquaero fan controllers */
>  #define AQUAERO_TEMP_CTRL_OFFSET	0xdb
> +#define AQUAERO_FAN_CTRL_MIN_PWR_OFFSET	0x04
> +#define AQUAERO_FAN_CTRL_MAX_PWR_OFFSET	0x06
> +#define AQUAERO_FAN_CTRL_SRC_OFFSET	0x10
> +static u16 aquaero_ctrl_fan_offsets[] = { 0x20c, 0x220, 0x234, 0x248 };
>  
>  /* Specs of the D5 Next pump */
>  #define D5NEXT_NUM_FANS			2
> @@ -857,13 +864,22 @@ static int aqc_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
>  		*val = priv->power_input[channel];
>  		break;
>  	case hwmon_pwm:
> -		if (priv->fan_ctrl_offsets) {
> +		switch (priv->kind) {
> +		case aquaero:
> +			ret = aqc_get_ctrl_val(priv, AQUAERO_CTRL_PRESET_START + channel * AQUAERO_CTRL_PRESET_SIZE,
> +					       val, AQC_BE16);
> +			if (ret < 0)
> +				return ret;
> +			*val = aqc_percent_to_pwm(*val);
> +			break;
> +		default:
>  			ret = aqc_get_ctrl_val(priv, priv->fan_ctrl_offsets[channel],
>  					       val, AQC_BE16);
>  			if (ret < 0)
>  				return ret;
>  
>  			*val = aqc_percent_to_pwm(ret);
> +			break;
>  		}
>  		break;
>  	case hwmon_in:
> @@ -922,6 +938,10 @@ static int aqc_write(struct device *dev, enum hwmon_sensor_types type, u32 attr,
>  		     long val)
>  {
>  	int ret, pwm_value;
> +	/* Arrays for setting multiple values at once in the control report */
> +	int ctrl_values_offsets[4];
> +	long ctrl_values[4];
> +	int ctrl_values_types[4];
>  	struct aqc_data *priv = dev_get_drvdata(dev);
>  
>  	switch (type) {
> @@ -956,15 +976,47 @@ static int aqc_write(struct device *dev, enum hwmon_sensor_types type, u32 attr,
>  	case hwmon_pwm:
>  		switch (attr) {
>  		case hwmon_pwm_input:
> -			if (priv->fan_ctrl_offsets) {
> -				pwm_value = aqc_pwm_to_percent(val);
> -				if (pwm_value < 0)
> -					return pwm_value;
> +			pwm_value = aqc_pwm_to_percent(val);
> +			if (pwm_value < 0)
> +				return pwm_value;
>  
> +			switch (priv->kind) {
> +			case aquaero:
> +				/* Write pwm value to preset corresponding to the channel */
> +				ctrl_values_offsets[0] = AQUAERO_CTRL_PRESET_START +
> +				    channel * AQUAERO_CTRL_PRESET_SIZE;
> +				ctrl_values[0] = pwm_value;
> +				ctrl_values_types[0] = AQC_BE16;
> +
> +				/* Write preset number in fan control source */
> +				ctrl_values_offsets[1] = priv->fan_ctrl_offsets[channel] +
> +				    AQUAERO_FAN_CTRL_SRC_OFFSET;
> +				ctrl_values[1] = AQUAERO_CTRL_PRESET_ID + channel;
> +				ctrl_values_types[1] = AQC_BE16;
> +
> +				/* Set minimum power to 0 to allow the fan to turn off */
> +				ctrl_values_offsets[2] = priv->fan_ctrl_offsets[channel] +
> +				    AQUAERO_FAN_CTRL_MIN_PWR_OFFSET;
> +				ctrl_values[2] = 0;
> +				ctrl_values_types[2] = AQC_BE16;
> +
> +				/* Set maximum power to 255 to allow the fan to reach max speed */
> +				ctrl_values_offsets[3] = priv->fan_ctrl_offsets[channel] +
> +				    AQUAERO_FAN_CTRL_MAX_PWR_OFFSET;
> +				ctrl_values[3] = aqc_pwm_to_percent(255);
> +				ctrl_values_types[3] = AQC_BE16;
> +
> +				ret = aqc_set_ctrl_vals(priv, ctrl_values_offsets, ctrl_values,
> +							ctrl_values_types, 4);
> +				if (ret < 0)
> +					return ret;
> +				break;
> +			default:
>  				ret = aqc_set_ctrl_val(priv, priv->fan_ctrl_offsets[channel],
>  						       pwm_value, AQC_BE16);
>  				if (ret < 0)
>  					return ret;
> +				break;
>  			}
>  			break;
>  		default:
> @@ -1287,6 +1339,7 @@ static int aqc_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  
>  		priv->num_fans = AQUAERO_NUM_FANS;
>  		priv->fan_sensor_offsets = aquaero_sensor_fan_offsets;
> +		priv->fan_ctrl_offsets = aquaero_ctrl_fan_offsets;
>  
>  		priv->num_temp_sensors = AQUAERO_NUM_SENSORS;
>  		priv->temp_sensor_start_offset = AQUAERO_SENSOR_START;