[PATCH v3 0/4] hwmon: AMPINEL support and shared fixes

[PATCH v3 0/4] hwmon: AMPINEL support and shared fixes

[Vas Zayarskiy]

This revision addresses Guenter's review by separating shared fixes from
AMPINEL support and leaving the keyboard interface with hid-generic.

Changes since v2:
- Add a HID prerequisite: hid-generic must consult a specialized driver's
  match callback before yielding. ID-table matching alone makes it yield
  the keyboard even if the hwmon driver rejects that interface. The AMPINEL
  match callback accepts only USB interface 1 and retains the transport
  check before accessing the parent. HID maintainers are copied for review.
- Widen the shared power/current caches, clamp overflowing power to
  LONG_MAX, handle unavailable power, and prevent negative Aquastream XT
  calibration results from wrapping into the unsigned current cache.
- Expire the initial cache for every device in a separate patch.
- Use the existing raw-event path for serial numbers, temperatures and
  timestamps. Store AMPINEL values in hwmon units during decoding.
- Omit current deviation until its semantics are established; remove
  AMPINEL keyboard callbacks and the unrelated expiry documentation.

Validation:
- Both modules build and link against Linux 7.2.5. Both changed objects
  build on this 7.3-rc3 base for x86_64 and i386 with W=1 and Sparse.
- All 995 captured AMPINEL reports pass the C replay harness with ASan/UBSan.
  Shared regression cases cover High Flow Next power and unavailable data,
  existing fan/pump power paths, Aquastream XT current, and initial expiry.
  The tests reproduce failures with the previous conversion/cache code.
- A userspace harness extracts the actual core/generic/AMPINEL match
  functions and tests both driver registration orders, keyboard fallback,
  non-USB rejection and special-driver overrides. The original hid-generic
  fails the fallback test. Probe descriptor validation is also covered.
- All seven Python tests pass. Strict checkpatch passes for all four
  signed patches. The changed RST renders without warnings.

Both revised modules are now installed on Hearth, Linux 7.2.5. Their loaded
ELF build IDs match the packaged binaries. All 60 raw USB/hwmon comparisons
matched across initial loading, telemetry-driver reload, persistent
activation, deep suspend/resume and reboot. The AMPINEL keyboard stays bound
to hid-generic, with its input registration unchanged through the reload
and resume. HID input names and capabilities were preserved across reboot.
The reboot selected both revised binaries without manual intervention.
The installation journal had no warnings or faults. Resume reinitialized
the USB controller and reset several devices including AMPINEL; the same
warning pattern is present in logs from 2026-08-29, before v3. Both interfaces
recovered without intervention. No Oops, BUG or call trace was found during
resume or reboot. Boot had no AMPINEL failure or USB-controller reset warning;
it retained the expected out-of-tree module notice and existing system
warnings. Live loading used generic-first order; the opposite order is
covered only by the userspace harness. Power-off cold boot and alarm-generated
keys remain untested. Shared-device numeric tests are synthetic. The HID
prerequisite needs review with the AMPINEL patch.

Telemetry units were checked against the device display at idle and under
load. Channel labels are report order. Alarms, protection configuration,
negative hotspot temperatures and other firmware remain unverified.

AI assistance: Codex assisted with the implementation, tests and submission
text; Sparse was used for static analysis.

V2: https://lore.kernel.org/linux-hwmon/20260914220128.287145-1-contact@vasily.cc/

Vas Zayarskiy (4):
  HID: generic: Respect specialized drivers match callbacks
  hwmon: (aquacomputer_d5next) Avoid truncating scaled sensor readings
  hwmon: (aquacomputer_d5next) Expire the initial sensor cache
  hwmon: (aquacomputer_d5next) Add AMPINEL support

 Documentation/hwmon/aquacomputer_d5next.rst |  11 ++
 drivers/hid/hid-generic.c                   |   8 +-
 drivers/hwmon/Kconfig                       |   4 +-
 drivers/hwmon/aquacomputer_d5next.c         | 121 ++++++++++++++++++--
 4 files changed, 132 insertions(+), 12 deletions(-)


base-commit: 46fde107b4e881f179bf02bca69d8a0020561b3c

[PATCH v3 1/4] HID: generic: Respect specialized drivers match callbacks

[Vas Zayarskiy]

A driver can reject an interface in its match callback even when its ID
table matches. hid-generic currently yields based on the ID table alone,
leaving such an interface without a driver.

Consult the callback after matching the ID table so hid-generic can keep
interfaces declined by specialized drivers. This allows the AMPINEL
hwmon driver to leave the separate keyboard interface to hid-generic.

The same callback check was proposed previously for Logitech devices:

Link: https://lkml.iu.edu/hypermail/linux/kernel/2212.0/06724.html

Assisted-by: LLM sparse
Signed-off-by: Vas Zayarskiy <contact@vasily.cc>
---
 drivers/hid/hid-generic.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-generic.c b/drivers/hid/hid-generic.c
index c2de91674..ebc38ae76 100644
--- a/drivers/hid/hid-generic.c
+++ b/drivers/hid/hid-generic.c
@@ -31,7 +31,13 @@ static int __check_hid_generic(struct device_driver *drv, void *data)
 	if (hdrv == &hid_generic)
 		return 0;
 
-	return hid_match_device(hdev, hdrv) != NULL;
+	if (!hid_match_device(hdev, hdrv))
+		return 0;
+
+	if (hdrv->match)
+		return hdrv->match(hdev, false);
+
+	return 1;
 }
 
 static bool hid_generic_match(struct hid_device *hdev,

[PATCH v3 2/4] hwmon: (aquacomputer_d5next) Avoid truncating scaled sensor readings

[Vas Zayarskiy]

High Flow Next power is reported in watts. Multiplying the 16-bit value
by one million can overflow both the signed intermediate and the u32
cache. Store power in long, perform the conversion in u64, and clamp to
LONG_MAX. The other existing power conversions fit in a signed 32-bit
value. Return ENODATA for unavailable power instead of exposing the
stored error as a reading.

Widen the shared current cache to u32. The Aquastream XT calibration can
exceed 65535 mA; clamp its negative calibration results to zero before
assigning to the unsigned cache. This also permits devices with scaled
aggregate currents to store milliamperes directly.

Assisted-by: LLM sparse
Signed-off-by: Vas Zayarskiy <contact@vasily.cc>
---
 drivers/hwmon/aquacomputer_d5next.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/hwmon/aquacomputer_d5next.c b/drivers/hwmon/aquacomputer_d5next.c
index 1ca70e726..9178ac4cb 100644
--- a/drivers/hwmon/aquacomputer_d5next.c
+++ b/drivers/hwmon/aquacomputer_d5next.c
@@ -19,6 +19,7 @@
 #include <linux/hwmon.h>
 #include <linux/jiffies.h>
 #include <linux/ktime.h>
+#include <linux/limits.h>
 #include <linux/module.h>
 #include <linux/seq_file.h>
 #include <linux/unaligned.h>
@@ -599,9 +600,9 @@ struct aqc_data {
 	u32 speed_input_min[1];
 	u32 speed_input_target[1];
 	u32 speed_input_max[1];
-	u32 power_input[8];
+	long power_input[8];
 	u16 voltage_input[8];
-	u16 current_input[8];
+	u32 current_input[8];
 
 	/* Label values */
 	const char *const *temp_label;
@@ -976,7 +977,7 @@ static int aqc_legacy_read(struct aqc_data *priv)
 
 		/* Calculation derived from linear regression */
 		sensor_value = get_unaligned_le16(priv->buffer + AQUASTREAMXT_PUMP_CURR_OFFSET);
-		priv->current_input[0] = DIV_ROUND_CLOSEST(sensor_value * 176, 100) - 52;
+		priv->current_input[0] = max(DIV_ROUND_CLOSEST(sensor_value * 176, 100) - 52, 0);
 
 		sensor_value = get_unaligned_le16(priv->buffer + AQUASTREAMXT_PUMP_VOLTAGE_OFFSET);
 		priv->voltage_input[0] = DIV_ROUND_CLOSEST(sensor_value * 1000, 61);
@@ -1070,6 +1071,8 @@ static int aqc_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
 		}
 		break;
 	case hwmon_power:
+		if (priv->power_input[channel] == -ENODATA)
+			return -ENODATA;
 		*val = priv->power_input[channel];
 		break;
 	case hwmon_pwm:
@@ -1426,7 +1429,8 @@ static int aqc_raw_event(struct hid_device *hdev, struct hid_report *report, u8
 			priv->power_input[0] = -ENODATA;
 		else
 			priv->power_input[0] =
-			    get_unaligned_be16(data + HIGHFLOWNEXT_POWER) * 1000000;
+			    min_t(u64, get_unaligned_be16(data + HIGHFLOWNEXT_POWER) * 1000000ULL,
+				  LONG_MAX);
 
 		priv->voltage_input[0] = get_unaligned_be16(data + HIGHFLOWNEXT_5V_VOLTAGE) * 10;
 		priv->voltage_input[1] =

[PATCH v3 3/4] hwmon: (aquacomputer_d5next) Expire the initial sensor cache

[Vas Zayarskiy]

The initial update timestamp makes the cache expire at the current
jiffy. Since aqc_read() uses time_after(), a read in that same jiffy can
expose zero-initialized data or skip the first legacy report request.

Initialize the timestamp one tick earlier so the cache starts expired
for every supported device.

Assisted-by: LLM sparse
Signed-off-by: Vas Zayarskiy <contact@vasily.cc>
---
 drivers/hwmon/aquacomputer_d5next.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hwmon/aquacomputer_d5next.c b/drivers/hwmon/aquacomputer_d5next.c
index 9178ac4cb..b58f5b5c9 100644
--- a/drivers/hwmon/aquacomputer_d5next.c
+++ b/drivers/hwmon/aquacomputer_d5next.c
@@ -1531,7 +1531,7 @@ static int aqc_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	priv->hdev = hdev;
 	hid_set_drvdata(hdev, priv);
 
-	priv->updated = jiffies - STATUS_UPDATE_INTERVAL;
+	priv->updated = jiffies - STATUS_UPDATE_INTERVAL - 1;
 
 	ret = hid_parse(hdev);
 	if (ret)

[PATCH v3 4/4] hwmon: (aquacomputer_d5next) Add AMPINEL support

[Vas Zayarskiy]

Add read-only total power, total current, input voltage, hotspot
temperature and six channel currents for the Aquacomputer AMPINEL.

Decode its 544-byte input report through the existing serial-number,
temperature and update path, with power, current and voltage handled in
the device-specific switch. Store all readings in hwmon units.

Match only the USB telemetry interface, leaving the separate keyboard
interface to hid-generic. Validate the telemetry collection and report
size before opening the device. Channel labels follow report order;
configuration and alarms are not supported.

Assisted-by: LLM sparse
Signed-off-by: Vas Zayarskiy <contact@vasily.cc>
---
 Documentation/hwmon/aquacomputer_d5next.rst |  11 ++
 drivers/hwmon/Kconfig                       |   4 +-
 drivers/hwmon/aquacomputer_d5next.c         | 107 +++++++++++++++++++-
 3 files changed, 116 insertions(+), 6 deletions(-)

diff --git a/Documentation/hwmon/aquacomputer_d5next.rst b/Documentation/hwmon/aquacomputer_d5next.rst
index 49163f387..faf1c51c1 100644
--- a/Documentation/hwmon/aquacomputer_d5next.rst
+++ b/Documentation/hwmon/aquacomputer_d5next.rst
@@ -13,6 +13,7 @@ Supported devices:
 * Aquacomputer Quadro fan controller
 * Aquacomputer High Flow Next sensor
 * Aquacomputer Leakshield leak prevention system
+* Aquacomputer AMPINEL power monitor
 * Aquacomputer Aquastream XT watercooling pump
 * Aquacomputer Aquastream Ultimate watercooling pump
 * Aquacomputer Poweradjust 3 fan controller
@@ -69,6 +70,16 @@ The Aquastream XT pump exposes temperature readings for the coolant, external se
 and fan IC. It also exposes pump and fan speeds (in RPM), voltages, as well as pump
 current.
 
+The AMPINEL exposes total power, total current, input voltage, hotspot
+temperature and six channel currents. Channel numbers follow report order;
+their mapping to physical connector pins has not been verified. The readings
+are received approximately twice per second. Only sensor reads are supported;
+the driver does not configure protection settings or report alarms.
+
+For AMPINEL, ``power1_input`` is total power, ``in0_input`` is input voltage,
+``temp1_input`` is hotspot temperature, ``curr1_input`` is total current, and
+``curr[2-7]_input`` are the six channel currents.
+
 The Aquastream Ultimate pump exposes coolant temp and an external temp sensor, along
 with speed, power, voltage and current of both the pump and optionally connected fan.
 It also exposes pressure and flow speed readings.
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 3d8b2c958..06cb246ff 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -257,14 +257,14 @@ config SENSORS_AHT10
 	  will be called aht10.
 
 config SENSORS_AQUACOMPUTER_D5NEXT
-	tristate "Aquacomputer D5 Next, Octo, Quadro, Farbwerk, Farbwerk 360, High Flow Next"
+	tristate "Aquacomputer D5 Next and other USB devices"
 	depends on USB_HID
 	select CRC16
 	help
 	  If you say yes here you get support for sensors and fans of
 	  the Aquacomputer D5 Next watercooling pump, Octo and Quadro fan
 	  controllers, Farbwerk and Farbwerk 360 RGB controllers, High Flow
-	  Next sensor, where available.
+	  Next sensor and AMPINEL power monitor, where available.
 
 	  This driver can also be built as a module. If so, the module
 	  will be called aquacomputer_d5next.
diff --git a/drivers/hwmon/aquacomputer_d5next.c b/drivers/hwmon/aquacomputer_d5next.c
index b58f5b5c9..e661c4dfa 100644
--- a/drivers/hwmon/aquacomputer_d5next.c
+++ b/drivers/hwmon/aquacomputer_d5next.c
@@ -2,7 +2,7 @@
 /*
  * hwmon driver for Aquacomputer devices (D5 Next, Farbwerk, Farbwerk 360, Octo,
  * Quadro, High Flow Next, Aquaero, Aquastream Ultimate, Leakshield,
- * High Flow USB/MPS Flow family)
+ * High Flow USB/MPS Flow family, AMPINEL)
  *
  * Aquacomputer devices send HID reports (with ID 0x01) every second to report
  * sensor values, except for devices that communicate through the
@@ -23,6 +23,7 @@
 #include <linux/module.h>
 #include <linux/seq_file.h>
 #include <linux/unaligned.h>
+#include <linux/usb.h>
 
 #define USB_VENDOR_ID_AQUACOMPUTER	0x0c70
 #define USB_PRODUCT_ID_AQUAERO		0xf001
@@ -33,6 +34,7 @@
 #define USB_PRODUCT_ID_OCTO		0xf011
 #define USB_PRODUCT_ID_HIGHFLOWNEXT	0xf012
 #define USB_PRODUCT_ID_LEAKSHIELD	0xf014
+#define USB_PRODUCT_ID_AMPINEL		0xf015
 #define USB_PRODUCT_ID_AQUASTREAMXT	0xf0b6
 #define USB_PRODUCT_ID_AQUASTREAMULT	0xf00b
 #define USB_PRODUCT_ID_POWERADJUST3	0xf0bd
@@ -41,7 +43,7 @@
 enum kinds {
 	d5next, farbwerk, farbwerk360, octo, quadro,
 	highflownext, aquaero, poweradjust3, aquastreamult,
-	aquastreamxt, leakshield, highflow
+	aquastreamxt, leakshield, highflow, ampinel
 };
 
 static const char *const aqc_device_names[] = {
@@ -56,7 +58,8 @@ static const char *const aqc_device_names[] = {
 	[aquaero] = "aquaero",
 	[aquastreamult] = "aquastreamultimate",
 	[poweradjust3] = "poweradjust3",
-	[highflow] = "highflow" /* Covers MPS Flow devices */
+	[highflow] = "highflow", /* Covers MPS Flow devices */
+	[ampinel] = "ampinel"
 };
 
 #define DRIVER_NAME			"aquacomputer_d5next"
@@ -111,6 +114,16 @@ static u8 aquaero_secondary_ctrl_report[] = {
 #define AQC_FAN_POWER_OFFSET		0x06
 #define AQC_FAN_SPEED_OFFSET		0x08
 
+/* AMPINEL offsets include the report ID byte. */
+#define AMPINEL_DATA_INTERFACE		1
+#define AMPINEL_STATUS_REPORT_SIZE	544
+#define AMPINEL_NUM_CHANNELS		6
+#define AMPINEL_CHANNEL_CURRENT_START	0x6f
+#define AMPINEL_POWER			0xc7
+#define AMPINEL_CURRENT			0xc9
+#define AMPINEL_VOLTAGE			0xcf
+#define AMPINEL_HOTSPOT			0xe1
+
 /* Specs of the Aquaero fan controllers */
 #define AQUAERO_SERIAL_START			0x07
 #define AQUAERO_FIRMWARE_VERSION		0x0B
@@ -516,6 +529,24 @@ static const char *const label_highflow_speeds[] = {
 	"Flow speed [dL/h]"
 };
 
+static const char *const label_ampinel_temp[] = {
+	"Hotspot"
+};
+
+static const char *const label_ampinel_power[] = {
+	"Total power"
+};
+
+static const char *const label_ampinel_voltage[] = {
+	"VCC In"
+};
+
+static const char *const label_ampinel_current[] = {
+	"Total current",
+	"Channel 1", "Channel 2", "Channel 3",
+	"Channel 4", "Channel 5", "Channel 6"
+};
+
 struct aqc_fan_structure_offsets {
 	u8 voltage;
 	u8 curr;
@@ -869,6 +900,10 @@ static umode_t aqc_is_visible(const void *data, enum hwmon_sensor_types type, u3
 		break;
 	case hwmon_power:
 		switch (priv->kind) {
+		case ampinel:
+			if (channel == 0)
+				return 0444;
+			break;
 		case aquastreamult:
 			/* Special case to support pump and fan power */
 			if (channel < 2)
@@ -889,6 +924,10 @@ static umode_t aqc_is_visible(const void *data, enum hwmon_sensor_types type, u3
 		break;
 	case hwmon_curr:
 		switch (priv->kind) {
+		case ampinel:
+			if (channel < AMPINEL_NUM_CHANNELS + 1)
+				return 0444;
+			break;
 		case aquastreamult:
 			/* Special case to support pump and fan current */
 			if (channel < 2)
@@ -907,6 +946,10 @@ static umode_t aqc_is_visible(const void *data, enum hwmon_sensor_types type, u3
 		break;
 	case hwmon_in:
 		switch (priv->kind) {
+		case ampinel:
+			if (channel == 0)
+				return 0444;
+			break;
 		case d5next:
 			/* Special case to support +5V and +12V voltage sensors */
 			if (channel < priv->num_fans + 2)
@@ -1336,12 +1379,17 @@ static int aqc_raw_event(struct hid_device *hdev, struct hid_report *report, u8
 		return 0;
 
 	priv = hid_get_drvdata(hdev);
+	if (priv->kind == ampinel &&
+	    (report->type != HID_INPUT_REPORT || size != AMPINEL_STATUS_REPORT_SIZE ||
+	     data[0] != STATUS_REPORT_ID))
+		return 0;
 
 	/* Info provided with every report */
 	priv->serial_number[0] = get_unaligned_be16(data + priv->serial_number_start_offset);
 	priv->serial_number[1] = get_unaligned_be16(data + priv->serial_number_start_offset +
 						    SERIAL_PART_OFFSET);
-	priv->firmware_version = get_unaligned_be16(data + priv->firmware_version_offset);
+	if (priv->firmware_version_offset)
+		priv->firmware_version = get_unaligned_be16(data + priv->firmware_version_offset);
 
 	/* Physical temperature sensor readings */
 	for (i = 0; i < priv->num_temp_sensors; i++) {
@@ -1394,6 +1442,17 @@ static int aqc_raw_event(struct hid_device *hdev, struct hid_report *report, u8
 
 	/* Special-case sensor readings */
 	switch (priv->kind) {
+	case ampinel:
+		priv->power_input[0] =
+			min_t(u64, get_unaligned_be16(data + AMPINEL_POWER) * 100000ULL,
+			      LONG_MAX);
+		priv->current_input[0] = get_unaligned_be16(data + AMPINEL_CURRENT) * 10;
+		for (i = 0; i < AMPINEL_NUM_CHANNELS; i++)
+			priv->current_input[i + 1] =
+				get_unaligned_be16(data + AMPINEL_CHANNEL_CURRENT_START +
+						   i * AQC_SENSOR_SIZE);
+		priv->voltage_input[0] = get_unaligned_be16(data + AMPINEL_VOLTAGE);
+		break;
 	case aquaero:
 		/* Read calculated virtual temp sensors */
 		i = priv->num_temp_sensors + priv->num_virtual_temp_sensors;
@@ -1519,8 +1578,26 @@ static void aqc_debugfs_init(struct aqc_data *priv)
 		debugfs_create_file("power_cycles", 0444, priv->debugfs, priv, &power_cycles_fops);
 }
 
+static bool aqc_match(struct hid_device *hdev, bool ignore_special_driver)
+{
+	struct usb_interface *intf;
+
+	if (ignore_special_driver || hdev->quirks & HID_QUIRK_IGNORE_SPECIAL_DRIVER)
+		return false;
+
+	if (hdev->product != USB_PRODUCT_ID_AMPINEL)
+		return true;
+
+	if (!hid_is_usb(hdev))
+		return false;
+
+	intf = to_usb_interface(hdev->dev.parent);
+	return intf->cur_altsetting->desc.bInterfaceNumber == AMPINEL_DATA_INTERFACE;
+}
+
 static int aqc_probe(struct hid_device *hdev, const struct hid_device_id *id)
 {
+	struct hid_report *report;
 	struct aqc_data *priv;
 	int ret;
 
@@ -1537,6 +1614,14 @@ static int aqc_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	if (ret)
 		return ret;
 
+	if (hdev->product == USB_PRODUCT_ID_AMPINEL) {
+		report = hdev->report_enum[HID_INPUT_REPORT].report_id_hash[STATUS_REPORT_ID];
+		if (hdev->maxcollection != 1 ||
+		    hdev->collection[0].usage != (HID_UP_MSVENDOR | 1) ||
+		    !report || report->size != (AMPINEL_STATUS_REPORT_SIZE - 1) * 8)
+			return -ENODEV;
+	}
+
 	ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
 	if (ret)
 		return ret;
@@ -1546,6 +1631,15 @@ static int aqc_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		goto fail_and_stop;
 
 	switch (hdev->product) {
+	case USB_PRODUCT_ID_AMPINEL:
+		priv->kind = ampinel;
+		priv->num_temp_sensors = 1;
+		priv->temp_sensor_start_offset = AMPINEL_HOTSPOT;
+		priv->temp_label = label_ampinel_temp;
+		priv->power_label = label_ampinel_power;
+		priv->voltage_label = label_ampinel_voltage;
+		priv->current_label = label_ampinel_current;
+		break;
 	case USB_PRODUCT_ID_AQUAERO:
 		/*
 		 * Aquaero presents itself as three HID devices under the same product ID:
@@ -1795,6 +1889,9 @@ static int aqc_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	}
 
 	switch (priv->kind) {
+	case ampinel:
+		priv->serial_number_start_offset = AQC_SERIAL_START;
+		break;
 	case aquaero:
 		priv->serial_number_start_offset = AQUAERO_SERIAL_START;
 		priv->firmware_version_offset = AQUAERO_FIRMWARE_VERSION;
@@ -1890,6 +1987,7 @@ static const struct hid_device_id aqc_table[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_AQUACOMPUTER, USB_PRODUCT_ID_QUADRO) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_AQUACOMPUTER, USB_PRODUCT_ID_HIGHFLOWNEXT) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_AQUACOMPUTER, USB_PRODUCT_ID_LEAKSHIELD) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_AQUACOMPUTER, USB_PRODUCT_ID_AMPINEL) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_AQUACOMPUTER, USB_PRODUCT_ID_AQUASTREAMXT) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_AQUACOMPUTER, USB_PRODUCT_ID_AQUASTREAMULT) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_AQUACOMPUTER, USB_PRODUCT_ID_POWERADJUST3) },
@@ -1902,6 +2000,7 @@ MODULE_DEVICE_TABLE(hid, aqc_table);
 static struct hid_driver aqc_driver = {
 	.name = DRIVER_NAME,
 	.id_table = aqc_table,
+	.match = aqc_match,
 	.probe = aqc_probe,
 	.remove = aqc_remove,
 	.raw_event = aqc_raw_event,

Re: [PATCH v3 2/4] hwmon: (aquacomputer_d5next) Avoid truncating scaled sensor readings

[sashiko-bot]

> High Flow Next power is reported in watts. Multiplying the 16-bit value
> by one million can overflow both the signed intermediate and the u32
> cache. Store power in long, perform the conversion in u64, and clamp to
> LONG_MAX. The other existing power conversions fit in a signed 32-bit
> value. Return ENODATA for unavailable power instead of exposing the
> stored error as a reading.
> 
> Widen the shared current cache to u32. The Aquastream XT calibration can
> exceed 65535 mA; clamp its negative calibration results to zero before
> assigning to the unsigned cache. This also permits devices with scaled
> aggregate currents to store milliamperes directly.
> 
> Assisted-by: LLM sparse
> Signed-off-by: Vas Zayarskiy <contact@vasily.cc>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260914233728.22725-1-contact@vasily.cc?part=2

Re: [PATCH v3 1/4] HID: generic: Respect specialized drivers match callbacks

[sashiko-bot]

> A driver can reject an interface in its match callback even when its ID
> table matches. hid-generic currently yields based on the ID table alone,
> leaving such an interface without a driver.
> 
> Consult the callback after matching the ID table so hid-generic can keep
> interfaces declined by specialized drivers. This allows the AMPINEL
> hwmon driver to leave the separate keyboard interface to hid-generic.
> 
> The same callback check was proposed previously for Logitech devices:
> 
> Link: https://lkml.iu.edu/hypermail/linux/kernel/2212.0/06724.html
> 
> Assisted-by: LLM sparse
> Signed-off-by: Vas Zayarskiy <contact@vasily.cc>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260914233728.22725-1-contact@vasily.cc?part=1

Re: [PATCH v3 4/4] hwmon: (aquacomputer_d5next) Add AMPINEL support

[sashiko-bot]

> Add read-only total power, total current, input voltage, hotspot
> temperature and six channel currents for the Aquacomputer AMPINEL.
> 
> Decode its 544-byte input report through the existing serial-number,
> temperature and update path, with power, current and voltage handled in
> the device-specific switch. Store all readings in hwmon units.
> 
> Match only the USB telemetry interface, leaving the separate keyboard
> interface to hid-generic. Validate the telemetry collection and report
> size before opening the device. Channel labels follow report order;
> configuration and alarms are not supported.
> 
> Assisted-by: LLM sparse
> Signed-off-by: Vas Zayarskiy <contact@vasily.cc>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260914233728.22725-1-contact@vasily.cc?part=4

Re: [PATCH v3 3/4] hwmon: (aquacomputer_d5next) Expire the initial sensor cache

[sashiko-bot]

> The initial update timestamp makes the cache expire at the current
> jiffy. Since aqc_read() uses time_after(), a read in that same jiffy can
> expose zero-initialized data or skip the first legacy report request.
> 
> Initialize the timestamp one tick earlier so the cache starts expired
> for every supported device.
> 
> Assisted-by: LLM sparse
> Signed-off-by: Vas Zayarskiy <contact@vasily.cc>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260914233728.22725-1-contact@vasily.cc?part=3

Re: [PATCH v3 4/4] hwmon: (aquacomputer_d5next) Add AMPINEL support

[Guenter Roeck]

On 9/14/26 16:37, Vas Zayarskiy wrote:
> Add read-only total power, total current, input voltage, hotspot
> temperature and six channel currents for the Aquacomputer AMPINEL.
> 
> Decode its 544-byte input report through the existing serial-number,
> temperature and update path, with power, current and voltage handled in
> the device-specific switch. Store all readings in hwmon units.
> 
> Match only the USB telemetry interface, leaving the separate keyboard
> interface to hid-generic. Validate the telemetry collection and report
> size before opening the device. Channel labels follow report order;
> configuration and alarms are not supported.
> 
> Assisted-by: LLM sparse
> Signed-off-by: Vas Zayarskiy <contact@vasily.cc>
> ---
>   Documentation/hwmon/aquacomputer_d5next.rst |  11 ++
>   drivers/hwmon/Kconfig                       |   4 +-
>   drivers/hwmon/aquacomputer_d5next.c         | 107 +++++++++++++++++++-
>   3 files changed, 116 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/hwmon/aquacomputer_d5next.rst b/Documentation/hwmon/aquacomputer_d5next.rst
> index 49163f387..faf1c51c1 100644
> --- a/Documentation/hwmon/aquacomputer_d5next.rst
> +++ b/Documentation/hwmon/aquacomputer_d5next.rst
> @@ -13,6 +13,7 @@ Supported devices:
>   * Aquacomputer Quadro fan controller
>   * Aquacomputer High Flow Next sensor
>   * Aquacomputer Leakshield leak prevention system
> +* Aquacomputer AMPINEL power monitor
>   * Aquacomputer Aquastream XT watercooling pump
>   * Aquacomputer Aquastream Ultimate watercooling pump
>   * Aquacomputer Poweradjust 3 fan controller
> @@ -69,6 +70,16 @@ The Aquastream XT pump exposes temperature readings for the coolant, external se
>   and fan IC. It also exposes pump and fan speeds (in RPM), voltages, as well as pump
>   current.
>   
> +The AMPINEL exposes total power, total current, input voltage, hotspot
> +temperature and six channel currents. Channel numbers follow report order;
> +their mapping to physical connector pins has not been verified. The readings
> +are received approximately twice per second. Only sensor reads are supported;
> +the driver does not configure protection settings or report alarms.
> +
> +For AMPINEL, ``power1_input`` is total power, ``in0_input`` is input voltage,
> +``temp1_input`` is hotspot temperature, ``curr1_input`` is total current, and
> +``curr[2-7]_input`` are the six channel currents.
> +
>   The Aquastream Ultimate pump exposes coolant temp and an external temp sensor, along
>   with speed, power, voltage and current of both the pump and optionally connected fan.
>   It also exposes pressure and flow speed readings.
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 3d8b2c958..06cb246ff 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -257,14 +257,14 @@ config SENSORS_AHT10
>   	  will be called aht10.
>   
>   config SENSORS_AQUACOMPUTER_D5NEXT
> -	tristate "Aquacomputer D5 Next, Octo, Quadro, Farbwerk, Farbwerk 360, High Flow Next"
> +	tristate "Aquacomputer D5 Next and other USB devices"
>   	depends on USB_HID
>   	select CRC16
>   	help
>   	  If you say yes here you get support for sensors and fans of
>   	  the Aquacomputer D5 Next watercooling pump, Octo and Quadro fan
>   	  controllers, Farbwerk and Farbwerk 360 RGB controllers, High Flow
> -	  Next sensor, where available.
> +	  Next sensor and AMPINEL power monitor, where available.
>   
>   	  This driver can also be built as a module. If so, the module
>   	  will be called aquacomputer_d5next.
> diff --git a/drivers/hwmon/aquacomputer_d5next.c b/drivers/hwmon/aquacomputer_d5next.c
> index b58f5b5c9..e661c4dfa 100644
> --- a/drivers/hwmon/aquacomputer_d5next.c
> +++ b/drivers/hwmon/aquacomputer_d5next.c
> @@ -2,7 +2,7 @@
>   /*
>    * hwmon driver for Aquacomputer devices (D5 Next, Farbwerk, Farbwerk 360, Octo,
>    * Quadro, High Flow Next, Aquaero, Aquastream Ultimate, Leakshield,
> - * High Flow USB/MPS Flow family)
> + * High Flow USB/MPS Flow family, AMPINEL)
>    *
>    * Aquacomputer devices send HID reports (with ID 0x01) every second to report
>    * sensor values, except for devices that communicate through the
> @@ -23,6 +23,7 @@
>   #include <linux/module.h>
>   #include <linux/seq_file.h>
>   #include <linux/unaligned.h>
> +#include <linux/usb.h>
>   
>   #define USB_VENDOR_ID_AQUACOMPUTER	0x0c70
>   #define USB_PRODUCT_ID_AQUAERO		0xf001
> @@ -33,6 +34,7 @@
>   #define USB_PRODUCT_ID_OCTO		0xf011
>   #define USB_PRODUCT_ID_HIGHFLOWNEXT	0xf012
>   #define USB_PRODUCT_ID_LEAKSHIELD	0xf014
> +#define USB_PRODUCT_ID_AMPINEL		0xf015
>   #define USB_PRODUCT_ID_AQUASTREAMXT	0xf0b6
>   #define USB_PRODUCT_ID_AQUASTREAMULT	0xf00b
>   #define USB_PRODUCT_ID_POWERADJUST3	0xf0bd
> @@ -41,7 +43,7 @@
>   enum kinds {
>   	d5next, farbwerk, farbwerk360, octo, quadro,
>   	highflownext, aquaero, poweradjust3, aquastreamult,
> -	aquastreamxt, leakshield, highflow
> +	aquastreamxt, leakshield, highflow, ampinel
>   };
>   
>   static const char *const aqc_device_names[] = {
> @@ -56,7 +58,8 @@ static const char *const aqc_device_names[] = {
>   	[aquaero] = "aquaero",
>   	[aquastreamult] = "aquastreamultimate",
>   	[poweradjust3] = "poweradjust3",
> -	[highflow] = "highflow" /* Covers MPS Flow devices */
> +	[highflow] = "highflow", /* Covers MPS Flow devices */
> +	[ampinel] = "ampinel"

Add "," at the end to avoid having to change it whenever a new device is added.

>   };
>   
>   #define DRIVER_NAME			"aquacomputer_d5next"
> @@ -111,6 +114,16 @@ static u8 aquaero_secondary_ctrl_report[] = {
>   #define AQC_FAN_POWER_OFFSET		0x06
>   #define AQC_FAN_SPEED_OFFSET		0x08
>   
> +/* AMPINEL offsets include the report ID byte. */
> +#define AMPINEL_DATA_INTERFACE		1
> +#define AMPINEL_STATUS_REPORT_SIZE	544
> +#define AMPINEL_NUM_CHANNELS		6
> +#define AMPINEL_CHANNEL_CURRENT_START	0x6f
> +#define AMPINEL_POWER			0xc7
> +#define AMPINEL_CURRENT			0xc9
> +#define AMPINEL_VOLTAGE			0xcf
> +#define AMPINEL_HOTSPOT			0xe1
> +
>   /* Specs of the Aquaero fan controllers */
>   #define AQUAERO_SERIAL_START			0x07
>   #define AQUAERO_FIRMWARE_VERSION		0x0B
> @@ -516,6 +529,24 @@ static const char *const label_highflow_speeds[] = {
>   	"Flow speed [dL/h]"
>   };
>   
> +static const char *const label_ampinel_temp[] = {
> +	"Hotspot"
> +};
> +
> +static const char *const label_ampinel_power[] = {
> +	"Total power"
> +};
> +
> +static const char *const label_ampinel_voltage[] = {
> +	"VCC In"
> +};
> +
> +static const char *const label_ampinel_current[] = {
> +	"Total current",
> +	"Channel 1", "Channel 2", "Channel 3",
> +	"Channel 4", "Channel 5", "Channel 6"
> +};
> +
>   struct aqc_fan_structure_offsets {
>   	u8 voltage;
>   	u8 curr;
> @@ -869,6 +900,10 @@ static umode_t aqc_is_visible(const void *data, enum hwmon_sensor_types type, u3
>   		break;
>   	case hwmon_power:
>   		switch (priv->kind) {
> +		case ampinel:
> +			if (channel == 0)
> +				return 0444;
> +			break;
>   		case aquastreamult:
>   			/* Special case to support pump and fan power */
>   			if (channel < 2)
> @@ -889,6 +924,10 @@ static umode_t aqc_is_visible(const void *data, enum hwmon_sensor_types type, u3
>   		break;
>   	case hwmon_curr:
>   		switch (priv->kind) {
> +		case ampinel:
> +			if (channel < AMPINEL_NUM_CHANNELS + 1)
> +				return 0444;
> +			break;
>   		case aquastreamult:
>   			/* Special case to support pump and fan current */
>   			if (channel < 2)
> @@ -907,6 +946,10 @@ static umode_t aqc_is_visible(const void *data, enum hwmon_sensor_types type, u3
>   		break;
>   	case hwmon_in:
>   		switch (priv->kind) {
> +		case ampinel:
> +			if (channel == 0)
> +				return 0444;
> +			break;
>   		case d5next:
>   			/* Special case to support +5V and +12V voltage sensors */
>   			if (channel < priv->num_fans + 2)
> @@ -1336,12 +1379,17 @@ static int aqc_raw_event(struct hid_device *hdev, struct hid_report *report, u8
>   		return 0;
>   
>   	priv = hid_get_drvdata(hdev);
> +	if (priv->kind == ampinel &&
> +	    (report->type != HID_INPUT_REPORT || size != AMPINEL_STATUS_REPORT_SIZE ||
> +	     data[0] != STATUS_REPORT_ID))
> +		return 0;
>   

This is again a bug fix affecting other controllers as well (report size and
input type check).

>   	/* Info provided with every report */
>   	priv->serial_number[0] = get_unaligned_be16(data + priv->serial_number_start_offset);
>   	priv->serial_number[1] = get_unaligned_be16(data + priv->serial_number_start_offset +
>   						    SERIAL_PART_OFFSET);
> -	priv->firmware_version = get_unaligned_be16(data + priv->firmware_version_offset);
> +	if (priv->firmware_version_offset)
> +		priv->firmware_version = get_unaligned_be16(data + priv->firmware_version_offset);

Unrelated to adding support for ampinel.

>   
>   	/* Physical temperature sensor readings */
>   	for (i = 0; i < priv->num_temp_sensors; i++) {
> @@ -1394,6 +1442,17 @@ static int aqc_raw_event(struct hid_device *hdev, struct hid_report *report, u8
>   
>   	/* Special-case sensor readings */
>   	switch (priv->kind) {
> +	case ampinel:
> +		priv->power_input[0] =
> +			min_t(u64, get_unaligned_be16(data + AMPINEL_POWER) * 100000ULL,
> +			      LONG_MAX);
> +		priv->current_input[0] = get_unaligned_be16(data + AMPINEL_CURRENT) * 10;
> +		for (i = 0; i < AMPINEL_NUM_CHANNELS; i++)
> +			priv->current_input[i + 1] =
> +				get_unaligned_be16(data + AMPINEL_CHANNEL_CURRENT_START +
> +						   i * AQC_SENSOR_SIZE);
> +		priv->voltage_input[0] = get_unaligned_be16(data + AMPINEL_VOLTAGE);
> +		break;
>   	case aquaero:
>   		/* Read calculated virtual temp sensors */
>   		i = priv->num_temp_sensors + priv->num_virtual_temp_sensors;
> @@ -1519,8 +1578,26 @@ static void aqc_debugfs_init(struct aqc_data *priv)
>   		debugfs_create_file("power_cycles", 0444, priv->debugfs, priv, &power_cycles_fops);
>   }
>   
> +static bool aqc_match(struct hid_device *hdev, bool ignore_special_driver)
> +{
> +	struct usb_interface *intf;
> +
> +	if (ignore_special_driver || hdev->quirks & HID_QUIRK_IGNORE_SPECIAL_DRIVER)
> +		return false;
> +
> +	if (hdev->product != USB_PRODUCT_ID_AMPINEL)
> +		return true;
> +
> +	if (!hid_is_usb(hdev))
> +		return false;
> +
> +	intf = to_usb_interface(hdev->dev.parent);
> +	return intf->cur_altsetting->desc.bInterfaceNumber == AMPINEL_DATA_INTERFACE;
> +}
> +
>   static int aqc_probe(struct hid_device *hdev, const struct hid_device_id *id)
>   {
> +	struct hid_report *report;
>   	struct aqc_data *priv;
>   	int ret;
>   
> @@ -1537,6 +1614,14 @@ static int aqc_probe(struct hid_device *hdev, const struct hid_device_id *id)
>   	if (ret)
>   		return ret;
>   
> +	if (hdev->product == USB_PRODUCT_ID_AMPINEL) {
> +		report = hdev->report_enum[HID_INPUT_REPORT].report_id_hash[STATUS_REPORT_ID];
> +		if (hdev->maxcollection != 1 ||
> +		    hdev->collection[0].usage != (HID_UP_MSVENDOR | 1) ||
> +		    !report || report->size != (AMPINEL_STATUS_REPORT_SIZE - 1) * 8)
> +			return -ENODEV;
> +	}

USB_PRODUCT_ID_AQUAERO has the same or a similar problem and handles it
in the switch statement below. I do not see a reason to handle this one
differently (including the need for the match function which doesn't seem
to be needed for USB_PRODUCT_ID_AQUAERO).

If the match function _is_ needed, the reason needs to be explained, even
more so since this may impact USB_PRODUCT_ID_AQUAERO.

> +
>   	ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
>   	if (ret)
>   		return ret;
> @@ -1546,6 +1631,15 @@ static int aqc_probe(struct hid_device *hdev, const struct hid_device_id *id)
>   		goto fail_and_stop;
>   
>   	switch (hdev->product) {
> +	case USB_PRODUCT_ID_AMPINEL:
> +		priv->kind = ampinel;
> +		priv->num_temp_sensors = 1;
> +		priv->temp_sensor_start_offset = AMPINEL_HOTSPOT;

FWIW, the raw even can happen prior to this, which will result
in bad data. This also affetcs other devices supported by this
driver (and those with fans can crash when it happens).

> +		priv->temp_label = label_ampinel_temp;
> +		priv->power_label = label_ampinel_power;
> +		priv->voltage_label = label_ampinel_voltage;
> +		priv->current_label = label_ampinel_current;
> +		break;
>   	case USB_PRODUCT_ID_AQUAERO:
>   		/*
>   		 * Aquaero presents itself as three HID devices under the same product ID:
> @@ -1795,6 +1889,9 @@ static int aqc_probe(struct hid_device *hdev, const struct hid_device_id *id)
>   	}
>   
>   	switch (priv->kind) {
> +	case ampinel:
> +		priv->serial_number_start_offset = AQC_SERIAL_START;
> +		break;
>   	case aquaero:
>   		priv->serial_number_start_offset = AQUAERO_SERIAL_START;
>   		priv->firmware_version_offset = AQUAERO_FIRMWARE_VERSION;
> @@ -1890,6 +1987,7 @@ static const struct hid_device_id aqc_table[] = {
>   	{ HID_USB_DEVICE(USB_VENDOR_ID_AQUACOMPUTER, USB_PRODUCT_ID_QUADRO) },
>   	{ HID_USB_DEVICE(USB_VENDOR_ID_AQUACOMPUTER, USB_PRODUCT_ID_HIGHFLOWNEXT) },
>   	{ HID_USB_DEVICE(USB_VENDOR_ID_AQUACOMPUTER, USB_PRODUCT_ID_LEAKSHIELD) },
> +	{ HID_USB_DEVICE(USB_VENDOR_ID_AQUACOMPUTER, USB_PRODUCT_ID_AMPINEL) },
>   	{ HID_USB_DEVICE(USB_VENDOR_ID_AQUACOMPUTER, USB_PRODUCT_ID_AQUASTREAMXT) },
>   	{ HID_USB_DEVICE(USB_VENDOR_ID_AQUACOMPUTER, USB_PRODUCT_ID_AQUASTREAMULT) },
>   	{ HID_USB_DEVICE(USB_VENDOR_ID_AQUACOMPUTER, USB_PRODUCT_ID_POWERADJUST3) },
> @@ -1902,6 +2000,7 @@ MODULE_DEVICE_TABLE(hid, aqc_table);
>   static struct hid_driver aqc_driver = {
>   	.name = DRIVER_NAME,
>   	.id_table = aqc_table,
> +	.match = aqc_match,
>   	.probe = aqc_probe,
>   	.remove = aqc_remove,
>   	.raw_event = aqc_raw_event,

Re: [PATCH v3 1/4] HID: generic: Respect specialized drivers match callbacks

[Guenter Roeck]

On 9/14/26 16:37, Vas Zayarskiy wrote:
> A driver can reject an interface in its match callback even when its ID
> table matches. hid-generic currently yields based on the ID table alone,
> leaving such an interface without a driver.
> 
> Consult the callback after matching the ID table so hid-generic can keep
> interfaces declined by specialized drivers. This allows the AMPINEL
> hwmon driver to leave the separate keyboard interface to hid-generic.
> 
> The same callback check was proposed previously for Logitech devices:
> 
> Link: https://lkml.iu.edu/hypermail/linux/kernel/2212.0/06724.html

That was from 2022 and seems to have been abandoned, making me wonder
if it is still broken for Logitech devices or if another solution
was found.

Guenter

> 
> Assisted-by: LLM sparse
> Signed-off-by: Vas Zayarskiy <contact@vasily.cc>
> ---
>   drivers/hid/hid-generic.c | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hid/hid-generic.c b/drivers/hid/hid-generic.c
> index c2de91674..ebc38ae76 100644
> --- a/drivers/hid/hid-generic.c
> +++ b/drivers/hid/hid-generic.c
> @@ -31,7 +31,13 @@ static int __check_hid_generic(struct device_driver *drv, void *data)
>   	if (hdrv == &hid_generic)
>   		return 0;
>   
> -	return hid_match_device(hdev, hdrv) != NULL;
> +	if (!hid_match_device(hdev, hdrv))
> +		return 0;
> +
> +	if (hdrv->match)
> +		return hdrv->match(hdev, false);
> +
> +	return 1;
>   }
>   
>   static bool hid_generic_match(struct hid_device *hdev,

Re: [PATCH v3 2/4] hwmon: (aquacomputer_d5next) Avoid truncating scaled sensor readings

[Guenter Roeck]

On 9/14/26 16:37, Vas Zayarskiy wrote:
> High Flow Next power is reported in watts. Multiplying the 16-bit value
> by one million can overflow both the signed intermediate and the u32
> cache. Store power in long, perform the conversion in u64, and clamp to
> LONG_MAX. The other existing power conversions fit in a signed 32-bit
> value. Return ENODATA for unavailable power instead of exposing the

-ENODATA

> stored error as a reading.
> 
> Widen the shared current cache to u32. The Aquastream XT calibration can
> exceed 65535 mA; clamp its negative calibration results to zero before

That isn't a calibration. It is a conversion or scaling to hwmon ABI units.

> assigning to the unsigned cache. This also permits devices with scaled
> aggregate currents to store milliamperes directly.
> 
> Assisted-by: LLM sparse
> Signed-off-by: Vas Zayarskiy <contact@vasily.cc>
> ---
>   drivers/hwmon/aquacomputer_d5next.c | 12 ++++++++----
>   1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/hwmon/aquacomputer_d5next.c b/drivers/hwmon/aquacomputer_d5next.c
> index 1ca70e726..9178ac4cb 100644
> --- a/drivers/hwmon/aquacomputer_d5next.c
> +++ b/drivers/hwmon/aquacomputer_d5next.c
> @@ -19,6 +19,7 @@
>   #include <linux/hwmon.h>
>   #include <linux/jiffies.h>
>   #include <linux/ktime.h>
> +#include <linux/limits.h>
>   #include <linux/module.h>
>   #include <linux/seq_file.h>
>   #include <linux/unaligned.h>
> @@ -599,9 +600,9 @@ struct aqc_data {
>   	u32 speed_input_min[1];
>   	u32 speed_input_target[1];
>   	u32 speed_input_max[1];
> -	u32 power_input[8];
> +	long power_input[8];
>   	u16 voltage_input[8];
> -	u16 current_input[8];
> +	u32 current_input[8];
>   
>   	/* Label values */
>   	const char *const *temp_label;
> @@ -976,7 +977,7 @@ static int aqc_legacy_read(struct aqc_data *priv)
>   
>   		/* Calculation derived from linear regression */
>   		sensor_value = get_unaligned_le16(priv->buffer + AQUASTREAMXT_PUMP_CURR_OFFSET);
> -		priv->current_input[0] = DIV_ROUND_CLOSEST(sensor_value * 176, 100) - 52;
> +		priv->current_input[0] = max(DIV_ROUND_CLOSEST(sensor_value * 176, 100) - 52, 0);
>   
>   		sensor_value = get_unaligned_le16(priv->buffer + AQUASTREAMXT_PUMP_VOLTAGE_OFFSET);
>   		priv->voltage_input[0] = DIV_ROUND_CLOSEST(sensor_value * 1000, 61);
> @@ -1070,6 +1071,8 @@ static int aqc_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
>   		}
>   		break;
>   	case hwmon_power:
> +		if (priv->power_input[channel] == -ENODATA)
> +			return -ENODATA;
>   		*val = priv->power_input[channel];
>   		break;
>   	case hwmon_pwm:
> @@ -1426,7 +1429,8 @@ static int aqc_raw_event(struct hid_device *hdev, struct hid_report *report, u8
>   			priv->power_input[0] = -ENODATA;
>   		else
>   			priv->power_input[0] =
> -			    get_unaligned_be16(data + HIGHFLOWNEXT_POWER) * 1000000;
> +			    min_t(u64, get_unaligned_be16(data + HIGHFLOWNEXT_POWER) * 1000000ULL,
> +				  LONG_MAX);
>   
>   		priv->voltage_input[0] = get_unaligned_be16(data + HIGHFLOWNEXT_5V_VOLTAGE) * 10;
>   		priv->voltage_input[1] =

Re: [PATCH v3 1/4] HID: generic: Respect specialized drivers match callbacks

[Vas Zayarskiy]

Hi Guenter,

> That was from 2022 and seems to have been abandoned, making me wonder
> if it is still broken for Logitech devices or if another solution
> was found.

The 2022 ->match() change was proposed to repair the fallback problem exposed by wildcard enrollment of Logitech Bluetooth devices. Maintainers instead reverted that wildcard enrollment in a9d9e46c755a, restoring the previous explicit device matching (https://github.com/torvalds/linux/commit/a9d9e46c755a189ccb44d91b8cf737742a975de8). The way I understand it, the wildcard enrollment was reverted, and that's why the ->match() change simply was not needed anymore.

In the hwmon-next base used for this series, Bluetooth devices are still selected through explicit entries in hidpp_devices[], and hid-logitech-hidpp has no match callback. So the Logitech regression was resolved without adopting the generic matching change.

For AMPINEL, the reason for patch 1 is to leave its separate keyboard interface bound to hid-generic, so it can issue e.g. shutdown commands on abnormal readings. Both interfaces match the hwmon driver's ID table. The existing __check_hid_generic() therefore rejects the keyboard even when the hwmon match callback declines that interface. The hwmon fixes and sensor readings do not depend on this HID change; it is needed by this design to preserve the keyboard's generic binding.

Jiri, Benjamin: is there a preferred way to express this per-interface selection while retaining hid-generic for the keyboard? I would welcome guidance on whether this callback check is the appropriate approach.


Thanks,
Vas




On Tuesday, September 15th, 2026 at 7:31 AM, Guenter Roeck <linux@roeck-us.net> wrote:

> On 9/14/26 16:37, Vas Zayarskiy wrote:
> > A driver can reject an interface in its match callback even when its ID
> > table matches. hid-generic currently yields based on the ID table alone,
> > leaving such an interface without a driver.
> >
> > Consult the callback after matching the ID table so hid-generic can keep
> > interfaces declined by specialized drivers. This allows the AMPINEL
> > hwmon driver to leave the separate keyboard interface to hid-generic.
> >
> > The same callback check was proposed previously for Logitech devices:
> >
> > Link: https://lkml.iu.edu/hypermail/linux/kernel/2212.0/06724.html
> 
> That was from 2022 and seems to have been abandoned, making me wonder
> if it is still broken for Logitech devices or if another solution
> was found.
> 
> Guenter
> 
> >
> > Assisted-by: LLM sparse
> > Signed-off-by: Vas Zayarskiy <contact@vasily.cc>
> > ---
> >   drivers/hid/hid-generic.c | 8 +++++++-
> >   1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/hid/hid-generic.c b/drivers/hid/hid-generic.c
> > index c2de91674..ebc38ae76 100644
> > --- a/drivers/hid/hid-generic.c
> > +++ b/drivers/hid/hid-generic.c
> > @@ -31,7 +31,13 @@ static int __check_hid_generic(struct device_driver *drv, void *data)
> >   	if (hdrv == &hid_generic)
> >   		return 0;
> >
> > -	return hid_match_device(hdev, hdrv) != NULL;
> > +	if (!hid_match_device(hdev, hdrv))
> > +		return 0;
> > +
> > +	if (hdrv->match)
> > +		return hdrv->match(hdev, false);
> > +
> > +	return 1;
> >   }
> >
> >   static bool hid_generic_match(struct hid_device *hdev,
> 
>

Re: [PATCH v3 4/4] hwmon: (aquacomputer_d5next) Add AMPINEL support

[Vas Zayarskiy]

Hi Guenter,

Thanks for the review. I will add the trailing comma and separate the shared report validation and handling of an unknown firmware-version offset from the AMPINEL addition. I also noted the -ENODATA and conversion/scaling wording corrections on patch 2.

> If the match function _is_ needed, the reason needs to be explained, even
> more so since this may impact USB_PRODUCT_ID_AQUAERO.

The requirement is to keep AMPINEL's keyboard interface bound to hid-generic while the hwmon driver handles telemetry. Both interfaces share the same VID/PID and match aqc_table. With the existing __check_hid_generic(), that ID match is enough for hid-generic to decline the keyboard. Returning -ENODEV from aqc_probe(), or rejecting it in aqc_match() alone, does not make it eligible for hid-generic.

The Aquaero collection check rejects its non-telemetry interfaces during probe, but that check alone does not establish keyboard fallback to hid-generic. V3 leaves Aquaero's collection filtering in probe: aqc_match() accepts it subject to the same generic-driver override checks the core already applies when there is no match callback. I do not have Aquaero hardware and have not validated its keyboard behavior.

The matcher test demonstrates the AMPINEL rejection with the original hid-generic logic and passes with patch 1. This is a userspace harness, not a live kernel fallback test. With both revised modules loaded on the physical AMPINEL, the keyboard registration/capabilities survived hwmon reload and suspend/resume, and its capabilities were preserved on reboot. I will make the matching rationale explicit in the next revision.

> FWIW, the raw even can happen prior to this, which will result
> in bad data. This also affetcs other devices supported by this
> driver (and those with fans can crash when it happens).

Could you clarify the path that permits raw_event during probe here? I checked the series base, 46fde107b4e881f179bf02bca69d8a0020561b3c. include/linux/hid.h documents that callbacks are blocked during probe unless the driver calls hid_device_io_start(). In hid-core.c, hid_device_probe() holds driver_input_lock around the driver's probe, and __hid_input_report() returns -EBUSY when it cannot acquire that lock.

aqc_probe() does not call hid_device_io_start(), and neither hid_hw_start() nor hid_hw_open() releases that lock. My reading is that normal USB reports cannot reach aqc_raw_event() until initialization has completed. Is there another path that bypasses this protection? I have not reproduced the race, and would like to understand it before adding a shared initialization fix.

Thanks,
Vas



On Tuesday, September 15th, 2026 at 7:30 AM, Guenter Roeck <linux@roeck-us.net> wrote:

> On 9/14/26 16:37, Vas Zayarskiy wrote:
> > Add read-only total power, total current, input voltage, hotspot
> > temperature and six channel currents for the Aquacomputer AMPINEL.
> >
> > Decode its 544-byte input report through the existing serial-number,
> > temperature and update path, with power, current and voltage handled in
> > the device-specific switch. Store all readings in hwmon units.
> >
> > Match only the USB telemetry interface, leaving the separate keyboard
> > interface to hid-generic. Validate the telemetry collection and report
> > size before opening the device. Channel labels follow report order;
> > configuration and alarms are not supported.
> >
> > Assisted-by: LLM sparse
> > Signed-off-by: Vas Zayarskiy <contact@vasily.cc>
> > ---
> >   Documentation/hwmon/aquacomputer_d5next.rst |  11 ++
> >   drivers/hwmon/Kconfig                       |   4 +-
> >   drivers/hwmon/aquacomputer_d5next.c         | 107 +++++++++++++++++++-
> >   3 files changed, 116 insertions(+), 6 deletions(-)
> >
> > diff --git a/Documentation/hwmon/aquacomputer_d5next.rst b/Documentation/hwmon/aquacomputer_d5next.rst
> > index 49163f387..faf1c51c1 100644
> > --- a/Documentation/hwmon/aquacomputer_d5next.rst
> > +++ b/Documentation/hwmon/aquacomputer_d5next.rst
> > @@ -13,6 +13,7 @@ Supported devices:
> >   * Aquacomputer Quadro fan controller
> >   * Aquacomputer High Flow Next sensor
> >   * Aquacomputer Leakshield leak prevention system
> > +* Aquacomputer AMPINEL power monitor
> >   * Aquacomputer Aquastream XT watercooling pump
> >   * Aquacomputer Aquastream Ultimate watercooling pump
> >   * Aquacomputer Poweradjust 3 fan controller
> > @@ -69,6 +70,16 @@ The Aquastream XT pump exposes temperature readings for the coolant, external se
> >   and fan IC. It also exposes pump and fan speeds (in RPM), voltages, as well as pump
> >   current.
> >
> > +The AMPINEL exposes total power, total current, input voltage, hotspot
> > +temperature and six channel currents. Channel numbers follow report order;
> > +their mapping to physical connector pins has not been verified. The readings
> > +are received approximately twice per second. Only sensor reads are supported;
> > +the driver does not configure protection settings or report alarms.
> > +
> > +For AMPINEL, ``power1_input`` is total power, ``in0_input`` is input voltage,
> > +``temp1_input`` is hotspot temperature, ``curr1_input`` is total current, and
> > +``curr[2-7]_input`` are the six channel currents.
> > +
> >   The Aquastream Ultimate pump exposes coolant temp and an external temp sensor, along
> >   with speed, power, voltage and current of both the pump and optionally connected fan.
> >   It also exposes pressure and flow speed readings.
> > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> > index 3d8b2c958..06cb246ff 100644
> > --- a/drivers/hwmon/Kconfig
> > +++ b/drivers/hwmon/Kconfig
> > @@ -257,14 +257,14 @@ config SENSORS_AHT10
> >   	  will be called aht10.
> >
> >   config SENSORS_AQUACOMPUTER_D5NEXT
> > -	tristate "Aquacomputer D5 Next, Octo, Quadro, Farbwerk, Farbwerk 360, High Flow Next"
> > +	tristate "Aquacomputer D5 Next and other USB devices"
> >   	depends on USB_HID
> >   	select CRC16
> >   	help
> >   	  If you say yes here you get support for sensors and fans of
> >   	  the Aquacomputer D5 Next watercooling pump, Octo and Quadro fan
> >   	  controllers, Farbwerk and Farbwerk 360 RGB controllers, High Flow
> > -	  Next sensor, where available.
> > +	  Next sensor and AMPINEL power monitor, where available.
> >
> >   	  This driver can also be built as a module. If so, the module
> >   	  will be called aquacomputer_d5next.
> > diff --git a/drivers/hwmon/aquacomputer_d5next.c b/drivers/hwmon/aquacomputer_d5next.c
> > index b58f5b5c9..e661c4dfa 100644
> > --- a/drivers/hwmon/aquacomputer_d5next.c
> > +++ b/drivers/hwmon/aquacomputer_d5next.c
> > @@ -2,7 +2,7 @@
> >   /*
> >    * hwmon driver for Aquacomputer devices (D5 Next, Farbwerk, Farbwerk 360, Octo,
> >    * Quadro, High Flow Next, Aquaero, Aquastream Ultimate, Leakshield,
> > - * High Flow USB/MPS Flow family)
> > + * High Flow USB/MPS Flow family, AMPINEL)
> >    *
> >    * Aquacomputer devices send HID reports (with ID 0x01) every second to report
> >    * sensor values, except for devices that communicate through the
> > @@ -23,6 +23,7 @@
> >   #include <linux/module.h>
> >   #include <linux/seq_file.h>
> >   #include <linux/unaligned.h>
> > +#include <linux/usb.h>
> >
> >   #define USB_VENDOR_ID_AQUACOMPUTER	0x0c70
> >   #define USB_PRODUCT_ID_AQUAERO		0xf001
> > @@ -33,6 +34,7 @@
> >   #define USB_PRODUCT_ID_OCTO		0xf011
> >   #define USB_PRODUCT_ID_HIGHFLOWNEXT	0xf012
> >   #define USB_PRODUCT_ID_LEAKSHIELD	0xf014
> > +#define USB_PRODUCT_ID_AMPINEL		0xf015
> >   #define USB_PRODUCT_ID_AQUASTREAMXT	0xf0b6
> >   #define USB_PRODUCT_ID_AQUASTREAMULT	0xf00b
> >   #define USB_PRODUCT_ID_POWERADJUST3	0xf0bd
> > @@ -41,7 +43,7 @@
> >   enum kinds {
> >   	d5next, farbwerk, farbwerk360, octo, quadro,
> >   	highflownext, aquaero, poweradjust3, aquastreamult,
> > -	aquastreamxt, leakshield, highflow
> > +	aquastreamxt, leakshield, highflow, ampinel
> >   };
> >
> >   static const char *const aqc_device_names[] = {
> > @@ -56,7 +58,8 @@ static const char *const aqc_device_names[] = {
> >   	[aquaero] = "aquaero",
> >   	[aquastreamult] = "aquastreamultimate",
> >   	[poweradjust3] = "poweradjust3",
> > -	[highflow] = "highflow" /* Covers MPS Flow devices */
> > +	[highflow] = "highflow", /* Covers MPS Flow devices */
> > +	[ampinel] = "ampinel"
> 
> Add "," at the end to avoid having to change it whenever a new device is added.
> 
> >   };
> >
> >   #define DRIVER_NAME			"aquacomputer_d5next"
> > @@ -111,6 +114,16 @@ static u8 aquaero_secondary_ctrl_report[] = {
> >   #define AQC_FAN_POWER_OFFSET		0x06
> >   #define AQC_FAN_SPEED_OFFSET		0x08
> >
> > +/* AMPINEL offsets include the report ID byte. */
> > +#define AMPINEL_DATA_INTERFACE		1
> > +#define AMPINEL_STATUS_REPORT_SIZE	544
> > +#define AMPINEL_NUM_CHANNELS		6
> > +#define AMPINEL_CHANNEL_CURRENT_START	0x6f
> > +#define AMPINEL_POWER			0xc7
> > +#define AMPINEL_CURRENT			0xc9
> > +#define AMPINEL_VOLTAGE			0xcf
> > +#define AMPINEL_HOTSPOT			0xe1
> > +
> >   /* Specs of the Aquaero fan controllers */
> >   #define AQUAERO_SERIAL_START			0x07
> >   #define AQUAERO_FIRMWARE_VERSION		0x0B
> > @@ -516,6 +529,24 @@ static const char *const label_highflow_speeds[] = {
> >   	"Flow speed [dL/h]"
> >   };
> >
> > +static const char *const label_ampinel_temp[] = {
> > +	"Hotspot"
> > +};
> > +
> > +static const char *const label_ampinel_power[] = {
> > +	"Total power"
> > +};
> > +
> > +static const char *const label_ampinel_voltage[] = {
> > +	"VCC In"
> > +};
> > +
> > +static const char *const label_ampinel_current[] = {
> > +	"Total current",
> > +	"Channel 1", "Channel 2", "Channel 3",
> > +	"Channel 4", "Channel 5", "Channel 6"
> > +};
> > +
> >   struct aqc_fan_structure_offsets {
> >   	u8 voltage;
> >   	u8 curr;
> > @@ -869,6 +900,10 @@ static umode_t aqc_is_visible(const void *data, enum hwmon_sensor_types type, u3
> >   		break;
> >   	case hwmon_power:
> >   		switch (priv->kind) {
> > +		case ampinel:
> > +			if (channel == 0)
> > +				return 0444;
> > +			break;
> >   		case aquastreamult:
> >   			/* Special case to support pump and fan power */
> >   			if (channel < 2)
> > @@ -889,6 +924,10 @@ static umode_t aqc_is_visible(const void *data, enum hwmon_sensor_types type, u3
> >   		break;
> >   	case hwmon_curr:
> >   		switch (priv->kind) {
> > +		case ampinel:
> > +			if (channel < AMPINEL_NUM_CHANNELS + 1)
> > +				return 0444;
> > +			break;
> >   		case aquastreamult:
> >   			/* Special case to support pump and fan current */
> >   			if (channel < 2)
> > @@ -907,6 +946,10 @@ static umode_t aqc_is_visible(const void *data, enum hwmon_sensor_types type, u3
> >   		break;
> >   	case hwmon_in:
> >   		switch (priv->kind) {
> > +		case ampinel:
> > +			if (channel == 0)
> > +				return 0444;
> > +			break;
> >   		case d5next:
> >   			/* Special case to support +5V and +12V voltage sensors */
> >   			if (channel < priv->num_fans + 2)
> > @@ -1336,12 +1379,17 @@ static int aqc_raw_event(struct hid_device *hdev, struct hid_report *report, u8
> >   		return 0;
> >
> >   	priv = hid_get_drvdata(hdev);
> > +	if (priv->kind == ampinel &&
> > +	    (report->type != HID_INPUT_REPORT || size != AMPINEL_STATUS_REPORT_SIZE ||
> > +	     data[0] != STATUS_REPORT_ID))
> > +		return 0;
> >
> 
> This is again a bug fix affecting other controllers as well (report size and
> input type check).
> 
> >   	/* Info provided with every report */
> >   	priv->serial_number[0] = get_unaligned_be16(data + priv->serial_number_start_offset);
> >   	priv->serial_number[1] = get_unaligned_be16(data + priv->serial_number_start_offset +
> >   						    SERIAL_PART_OFFSET);
> > -	priv->firmware_version = get_unaligned_be16(data + priv->firmware_version_offset);
> > +	if (priv->firmware_version_offset)
> > +		priv->firmware_version = get_unaligned_be16(data + priv->firmware_version_offset);
> 
> Unrelated to adding support for ampinel.
> 
> >
> >   	/* Physical temperature sensor readings */
> >   	for (i = 0; i < priv->num_temp_sensors; i++) {
> > @@ -1394,6 +1442,17 @@ static int aqc_raw_event(struct hid_device *hdev, struct hid_report *report, u8
> >
> >   	/* Special-case sensor readings */
> >   	switch (priv->kind) {
> > +	case ampinel:
> > +		priv->power_input[0] =
> > +			min_t(u64, get_unaligned_be16(data + AMPINEL_POWER) * 100000ULL,
> > +			      LONG_MAX);
> > +		priv->current_input[0] = get_unaligned_be16(data + AMPINEL_CURRENT) * 10;
> > +		for (i = 0; i < AMPINEL_NUM_CHANNELS; i++)
> > +			priv->current_input[i + 1] =
> > +				get_unaligned_be16(data + AMPINEL_CHANNEL_CURRENT_START +
> > +						   i * AQC_SENSOR_SIZE);
> > +		priv->voltage_input[0] = get_unaligned_be16(data + AMPINEL_VOLTAGE);
> > +		break;
> >   	case aquaero:
> >   		/* Read calculated virtual temp sensors */
> >   		i = priv->num_temp_sensors + priv->num_virtual_temp_sensors;
> > @@ -1519,8 +1578,26 @@ static void aqc_debugfs_init(struct aqc_data *priv)
> >   		debugfs_create_file("power_cycles", 0444, priv->debugfs, priv, &power_cycles_fops);
> >   }
> >
> > +static bool aqc_match(struct hid_device *hdev, bool ignore_special_driver)
> > +{
> > +	struct usb_interface *intf;
> > +
> > +	if (ignore_special_driver || hdev->quirks & HID_QUIRK_IGNORE_SPECIAL_DRIVER)
> > +		return false;
> > +
> > +	if (hdev->product != USB_PRODUCT_ID_AMPINEL)
> > +		return true;
> > +
> > +	if (!hid_is_usb(hdev))
> > +		return false;
> > +
> > +	intf = to_usb_interface(hdev->dev.parent);
> > +	return intf->cur_altsetting->desc.bInterfaceNumber == AMPINEL_DATA_INTERFACE;
> > +}
> > +
> >   static int aqc_probe(struct hid_device *hdev, const struct hid_device_id *id)
> >   {
> > +	struct hid_report *report;
> >   	struct aqc_data *priv;
> >   	int ret;
> >
> > @@ -1537,6 +1614,14 @@ static int aqc_probe(struct hid_device *hdev, const struct hid_device_id *id)
> >   	if (ret)
> >   		return ret;
> >
> > +	if (hdev->product == USB_PRODUCT_ID_AMPINEL) {
> > +		report = hdev->report_enum[HID_INPUT_REPORT].report_id_hash[STATUS_REPORT_ID];
> > +		if (hdev->maxcollection != 1 ||
> > +		    hdev->collection[0].usage != (HID_UP_MSVENDOR | 1) ||
> > +		    !report || report->size != (AMPINEL_STATUS_REPORT_SIZE - 1) * 8)
> > +			return -ENODEV;
> > +	}
> 
> USB_PRODUCT_ID_AQUAERO has the same or a similar problem and handles it
> in the switch statement below. I do not see a reason to handle this one
> differently (including the need for the match function which doesn't seem
> to be needed for USB_PRODUCT_ID_AQUAERO).
> 
> If the match function _is_ needed, the reason needs to be explained, even
> more so since this may impact USB_PRODUCT_ID_AQUAERO.
> 
> > +
> >   	ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
> >   	if (ret)
> >   		return ret;
> > @@ -1546,6 +1631,15 @@ static int aqc_probe(struct hid_device *hdev, const struct hid_device_id *id)
> >   		goto fail_and_stop;
> >
> >   	switch (hdev->product) {
> > +	case USB_PRODUCT_ID_AMPINEL:
> > +		priv->kind = ampinel;
> > +		priv->num_temp_sensors = 1;
> > +		priv->temp_sensor_start_offset = AMPINEL_HOTSPOT;
> 
> FWIW, the raw even can happen prior to this, which will result
> in bad data. This also affetcs other devices supported by this
> driver (and those with fans can crash when it happens).
> 
> > +		priv->temp_label = label_ampinel_temp;
> > +		priv->power_label = label_ampinel_power;
> > +		priv->voltage_label = label_ampinel_voltage;
> > +		priv->current_label = label_ampinel_current;
> > +		break;
> >   	case USB_PRODUCT_ID_AQUAERO:
> >   		/*
> >   		 * Aquaero presents itself as three HID devices under the same product ID:
> > @@ -1795,6 +1889,9 @@ static int aqc_probe(struct hid_device *hdev, const struct hid_device_id *id)
> >   	}
> >
> >   	switch (priv->kind) {
> > +	case ampinel:
> > +		priv->serial_number_start_offset = AQC_SERIAL_START;
> > +		break;
> >   	case aquaero:
> >   		priv->serial_number_start_offset = AQUAERO_SERIAL_START;
> >   		priv->firmware_version_offset = AQUAERO_FIRMWARE_VERSION;
> > @@ -1890,6 +1987,7 @@ static const struct hid_device_id aqc_table[] = {
> >   	{ HID_USB_DEVICE(USB_VENDOR_ID_AQUACOMPUTER, USB_PRODUCT_ID_QUADRO) },
> >   	{ HID_USB_DEVICE(USB_VENDOR_ID_AQUACOMPUTER, USB_PRODUCT_ID_HIGHFLOWNEXT) },
> >   	{ HID_USB_DEVICE(USB_VENDOR_ID_AQUACOMPUTER, USB_PRODUCT_ID_LEAKSHIELD) },
> > +	{ HID_USB_DEVICE(USB_VENDOR_ID_AQUACOMPUTER, USB_PRODUCT_ID_AMPINEL) },
> >   	{ HID_USB_DEVICE(USB_VENDOR_ID_AQUACOMPUTER, USB_PRODUCT_ID_AQUASTREAMXT) },
> >   	{ HID_USB_DEVICE(USB_VENDOR_ID_AQUACOMPUTER, USB_PRODUCT_ID_AQUASTREAMULT) },
> >   	{ HID_USB_DEVICE(USB_VENDOR_ID_AQUACOMPUTER, USB_PRODUCT_ID_POWERADJUST3) },
> > @@ -1902,6 +2000,7 @@ MODULE_DEVICE_TABLE(hid, aqc_table);
> >   static struct hid_driver aqc_driver = {
> >   	.name = DRIVER_NAME,
> >   	.id_table = aqc_table,
> > +	.match = aqc_match,
> >   	.probe = aqc_probe,
> >   	.remove = aqc_remove,
> >   	.raw_event = aqc_raw_event,
> 
>

Re: [PATCH v3 4/4] hwmon: (aquacomputer_d5next) Add AMPINEL support

[Guenter Roeck]

On 9/14/26 22:29, Vas Zayarskiy wrote:
> Hi Guenter,
> 
> Thanks for the review. I will add the trailing comma and separate the shared report validation and handling of an unknown firmware-version offset from the AMPINEL addition. I also noted the -ENODATA and conversion/scaling wording corrections on patch 2.
> 
>> If the match function _is_ needed, the reason needs to be explained, even
>> more so since this may impact USB_PRODUCT_ID_AQUAERO.
> 
> The requirement is to keep AMPINEL's keyboard interface bound to hid-generic while the hwmon driver handles telemetry. Both interfaces share the same VID/PID and match aqc_table. With the existing __check_hid_generic(), that ID match is enough for hid-generic to decline the keyboard. Returning -ENODEV from aqc_probe(), or rejecting it in aqc_match() alone, does not make it eligible for hid-generic.
> 
> The Aquaero collection check rejects its non-telemetry interfaces during probe, but that check alone does not establish keyboard fallback to hid-generic. V3 leaves Aquaero's collection filtering in probe: aqc_match() accepts it subject to the same generic-driver override checks the core already applies when there is no match callback. I do not have Aquaero hardware and have not validated its keyboard behavior.
> 
> The matcher test demonstrates the AMPINEL rejection with the original hid-generic logic and passes with patch 1. This is a userspace harness, not a live kernel fallback test. With both revised modules loaded on the physical AMPINEL, the keyboard registration/capabilities survived hwmon reload and suspend/resume, and its capabilities were preserved on reboot. I will make the matching rationale explicit in the next revision.
> 
>> FWIW, the raw even can happen prior to this, which will result
>> in bad data. This also affetcs other devices supported by this
>> driver (and those with fans can crash when it happens).
> 
> Could you clarify the path that permits raw_event during probe here? I checked the series base, 46fde107b4e881f179bf02bca69d8a0020561b3c. include/linux/hid.h documents that callbacks are blocked during probe unless the driver calls hid_device_io_start(). In hid-core.c, hid_device_probe() holds driver_input_lock around the driver's probe, and __hid_input_report() returns -EBUSY when it cannot acquire that lock.
> 
> aqc_probe() does not call hid_device_io_start(), and neither hid_hw_start() nor hid_hw_open() releases that lock. My reading is that normal USB reports cannot reach aqc_raw_event() until initialization has completed. Is there another path that bypasses this protection? I have not reproduced the race, and would like to understand it before adding a shared initialization fix.
> 

You are correct. Sorry, I missed the part about callbacks being blocked during probe
unless hid_device_io_start() is called.

Thanks,
Guenter