[PATCH 0/4] hwmon: Add WITRN USB tester driver

[PATCH 0/4] hwmon: Add WITRN USB tester driver

[Rong Zhang]

WITRN produces a series of devices to monitor power characteristics of
USB connections and display those on a on-device display. Most of them
contain an additional port which exposes the measurements via USB HID.

These devices report sensor values in IEEE-754 float (binary32) format.
The driver must perform floating-point number to integer conversions to
provide hwmon channels. Meanwhile, they also report accumulative float
values, and simple division or multiplication turns them into useful
hwmon channels.

Patch 1 adds label support for 64-bit energy attributes, as the driver
needs it.

Patch 2 adds a helper module for floating-point to integer conversions,
so that the conversion, multification and division methods can be used
in this driver as well as other drivers (I am also working on another
USB tester driver that needs it).

Patch 3 adds a barebone HID driver for WITRN K2.

Patch 4 adds hwmon channels and attributes to the driver.

Signed-off-by: Rong Zhang <i@rong.moe>
---
Rong Zhang (4):
      hwmon: Add label support for 64-bit energy attributes
      hwmon: New helper module for floating-point to integer conversions
      hwmon: Add barebone HID driver for WITRN
      hwmon: (witrn) Add monitoring support

 Documentation/hwmon/index.rst |   1 +
 Documentation/hwmon/witrn.rst |  53 ++++
 MAINTAINERS                   |   7 +
 drivers/hwmon/Kconfig         |  14 +
 drivers/hwmon/Makefile        |   2 +
 drivers/hwmon/hwmon-fp.c      | 262 ++++++++++++++++
 drivers/hwmon/hwmon-fp.h      | 212 +++++++++++++
 drivers/hwmon/hwmon.c         |   1 +
 drivers/hwmon/witrn.c         | 691 ++++++++++++++++++++++++++++++++++++++++++
 9 files changed, 1243 insertions(+)
---
base-commit: 0138af2472dfdef0d56fc4697416eaa0ff2589bd
change-id: 20260327-b4-hwmon-witrn-a629b9040250

Thanks,
Rong

[PATCH 1/4] hwmon: Add label support for 64-bit energy attributes

[Rong Zhang]

Since commit 0bcd01f757bc ("hwmon: Introduce 64-bit energy attribute
support"), devices can report 64-bit energy values by selecting the
sensor type "energy64". However, such sensors can't report their labels
since is_string_attr() was not updated to match it.

Add label support for 64-bit energy attributes by updating
is_string_attr() to match hwmon_energy64 in addition to hwmon_energy.

Signed-off-by: Rong Zhang <i@rong.moe>
---
 drivers/hwmon/hwmon.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c
index 9695dca62a7e..6812d1fd7c28 100644
--- a/drivers/hwmon/hwmon.c
+++ b/drivers/hwmon/hwmon.c
@@ -505,6 +505,7 @@ static bool is_string_attr(enum hwmon_sensor_types type, u32 attr)
 	       (type == hwmon_curr && attr == hwmon_curr_label) ||
 	       (type == hwmon_power && attr == hwmon_power_label) ||
 	       (type == hwmon_energy && attr == hwmon_energy_label) ||
+	       (type == hwmon_energy64 && attr == hwmon_energy_label) ||
 	       (type == hwmon_humidity && attr == hwmon_humidity_label) ||
 	       (type == hwmon_fan && attr == hwmon_fan_label);
 }

-- 
2.53.0

[PATCH 2/4] hwmon: New helper module for floating-point to integer conversions

[Rong Zhang]

Some devices report sensor values in IEEE-754 float (binary32) format.
Their drivers must perform floating-point number to integer conversions
to provide hwmon channels. Meanwhile, some of these devices also report
accumulative float values, and simple division or multiplication turns
them into useful hwmon channel.

Add a new helper module called "hwmon-fp" with float-to-s64/long
conversion, multification and division methods, so that these devices
can be supported painlessly.

Signed-off-by: Rong Zhang <i@rong.moe>
---
 drivers/hwmon/Kconfig    |   3 +
 drivers/hwmon/Makefile   |   1 +
 drivers/hwmon/hwmon-fp.c | 262 +++++++++++++++++++++++++++++++++++++++++++++++
 drivers/hwmon/hwmon-fp.h | 212 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 478 insertions(+)

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 328867242cb3..7ad909033e79 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -25,6 +25,9 @@ menuconfig HWMON
 
 if HWMON
 
+config HWMON_FP
+	tristate
+
 config HWMON_VID
 	tristate
 
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 5833c807c688..6dba69f712be 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -4,6 +4,7 @@
 #
 
 obj-$(CONFIG_HWMON)		+= hwmon.o
+obj-$(CONFIG_HWMON_FP)		+= hwmon-fp.o
 obj-$(CONFIG_HWMON_VID)		+= hwmon-vid.o
 
 # ACPI drivers
diff --git a/drivers/hwmon/hwmon-fp.c b/drivers/hwmon/hwmon-fp.c
new file mode 100644
index 000000000000..2ad636129a0c
--- /dev/null
+++ b/drivers/hwmon/hwmon-fp.c
@@ -0,0 +1,262 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Floating-point number to integer conversions
+ *
+ * Currently, only float (binary32) is supported.
+ *
+ * Copyright (c) 2026 Rong Zhang <i@rong.moe>
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/bits.h>
+#include <linux/bitfield.h>
+#include <linux/bitops.h>
+#include <linux/compiler.h>
+#include <linux/types.h>
+#include <linux/limits.h>
+#include <linux/minmax.h>
+#include <linux/module.h>
+
+#include "hwmon-fp.h"
+
+#define FLOAT_SIGN_MASK			BIT(31)
+#define FLOAT_EXPONENT_MASK		GENMASK(30, 23)
+#define FLOAT_MANTISSA_MASK		GENMASK(22, 0)
+#define FLOAT_EXPONENT_OFFSET		127
+#define FLOAT_FRACTION_Q		23
+#define FLOAT_IMPLICIT_BIT		BIT(23)
+
+#define HWMON_FP_SCALE_MAGNIFY_SHIFT_L	6
+
+struct float_struct {
+	u32 significand;
+	s16 shift_r;		/* = Q - exponent */
+	bool sign;
+	bool inf;		/* See below. */
+};
+
+/*
+ * The sign of a floating-point number carries significant information,
+ * return a saturated value for infinity so its sign is retained.
+ */
+static inline int hwmon_fp_infinity_to_s64(bool sign, s64 *val)
+{
+	*val = sign ? S64_MIN : S64_MAX;
+	return 0;
+}
+
+static int hwmon_fp_parse_float(u32 flt, struct float_struct *fs)
+{
+	u32 mantissa = FIELD_GET(FLOAT_MANTISSA_MASK, flt);
+	u8 exponent = FIELD_GET(FLOAT_EXPONENT_MASK, flt);
+	bool sign = FIELD_GET(FLOAT_SIGN_MASK, flt);
+
+	if (unlikely(exponent == FLOAT_EXPONENT_MASK >> FLOAT_FRACTION_Q)) {
+		if (mantissa != 0) /* NaN */
+			return -EINVAL;
+
+		/* Infinity */
+		fs->significand = HWMON_FP_FLOAT_SIGNIFICAND_MAX; /* Distinguish from fs(zero). */
+		fs->shift_r = 0;
+		fs->sign = sign;
+		fs->inf = true;
+
+		return 0;
+	}
+
+	fs->sign = sign;
+	fs->inf = false;
+
+	if (likely(exponent != 0)) {
+		/* Normal */
+		fs->significand = (FLOAT_IMPLICIT_BIT | mantissa);
+		fs->shift_r = FLOAT_FRACTION_Q - (exponent - FLOAT_EXPONENT_OFFSET);
+	} else if (unlikely(mantissa != 0)) {	/* exponent == 0 && mantissa != 0 */
+		/* Subnormal */
+		fs->significand = mantissa;
+		fs->shift_r = FLOAT_FRACTION_Q - (1 - FLOAT_EXPONENT_OFFSET);
+	} else {				/* exponent == 0 && mantissa == 0 */
+		/* Zero */
+		fs->significand = 0; /* Only fs(zero) has fs->significand == 0. */
+		fs->shift_r = 0;
+	}
+
+	return 0;
+}
+
+static int hwmon_fp_raw_to_s64(u64 significand, int shift_r, bool sign, s64 *val)
+{
+	u64 temp;
+
+	if (unlikely(shift_r >= 64) || significand == 0) {
+		*val = 0;
+		return 0;
+	}
+
+	if (shift_r < 0) {
+		/*
+		 * Left shift:
+		 *
+		 *   (significand * 2^-Q) * 2^exponent
+		 * = significand * 2^(exponent - Q)
+		 * = significand * 2^-shift_r
+		 * = significand << -shift_r
+		 */
+		shift_r = -shift_r;
+		temp = significand << shift_r;
+
+		if (unlikely(temp >> shift_r != significand))
+			return hwmon_fp_infinity_to_s64(sign, val);
+	} else if (shift_r == 0) {
+		temp = significand;
+	} else { /* shift_r > 0 */
+		/*
+		 * Right shift:
+		 *
+		 *   (significand * 2^-Q) * 2^exponent
+		 * = significand / 2^(Q - exponent)
+		 * = significand / 2^shift_r
+		 * = significand >> shift_r
+		 */
+		temp = significand >> shift_r;
+
+		/* Round to nearest. */
+		temp += !!(significand & BIT_U64(shift_r - 1));
+	}
+
+	if (unlikely((s64)temp < 0))
+		return hwmon_fp_infinity_to_s64(sign, val);
+
+	*val = (sign ? -1 : 1) * (s64)temp;
+	return 0;
+}
+
+static int __hwmon_fp_float_to_s64_unsafe(const struct float_struct *fs, u32 scale, s64 *val)
+{
+	if (unlikely(fs->inf))
+		return hwmon_fp_infinity_to_s64(fs->sign, val);
+
+	return hwmon_fp_raw_to_s64((u64)scale * (u64)fs->significand,
+				   fs->shift_r, fs->sign, val);
+}
+
+int hwmon_fp_float_to_s64_unsafe(u32 flt, u32 scale, s64 *val)
+{
+	struct float_struct fs;
+	int ret;
+
+	ret = hwmon_fp_parse_float(flt, &fs);
+	if (ret)
+		return ret;
+
+	return __hwmon_fp_float_to_s64_unsafe(&fs, scale, val);
+}
+EXPORT_SYMBOL_GPL(hwmon_fp_float_to_s64_unsafe);
+
+static int __hwmon_fp_mul_to_s64_unsafe(const struct float_struct *fs1,
+					const struct float_struct *fs2,
+					u32 scale_ntz, ulong scale_ctz, s64 *val)
+{
+	bool sign = fs1->sign ^ fs2->sign;
+	u64 scaled_significand;
+	int shift_r;
+
+	if (unlikely((fs1->inf && fs2->significand == 0) || (fs1->significand == 0 && fs2->inf)))
+		return -EINVAL;
+
+	if (unlikely(fs1->inf || fs2->inf))
+		return hwmon_fp_infinity_to_s64(sign, val);
+
+	if (fs1->significand == 0 || fs2->significand == 0) {
+		*val = 0;
+		return 0;
+	}
+
+	/*
+	 *   scale_ntz * 2^scale_ctz * significand1 * 2^-shift_r1 * significand2 * 2^-shift_r2
+	 * = scale_ntz * significand1 * significand2 * 2^-(shift_r1 + shift_r2 - scale_ctz)
+	 * = (scale_ntz * significand1 * significand2) >> (shift_r1 + shift_r2 - scale_ctz)
+	 */
+	scaled_significand = (u64)scale_ntz * (u64)fs1->significand * (u64)fs2->significand;
+	shift_r = fs1->shift_r + fs2->shift_r - scale_ctz;
+
+	return hwmon_fp_raw_to_s64(scaled_significand, shift_r, sign, val);
+}
+
+int hwmon_fp_mul_to_s64_unsafe(u32 flt1, u32 flt2, u32 scale, ulong scale_ctz, s64 *val)
+{
+	struct float_struct fs1, fs2;
+	int ret;
+
+	ret = hwmon_fp_parse_float(flt1, &fs1) || hwmon_fp_parse_float(flt2, &fs2);
+	if (ret)
+		return ret;
+
+	return __hwmon_fp_mul_to_s64_unsafe(&fs1, &fs2, scale, scale_ctz, val);
+}
+EXPORT_SYMBOL_GPL(hwmon_fp_mul_to_s64_unsafe);
+
+static int __hwmon_fp_div_to_s64_unsafe(const struct float_struct *fs1,
+					const struct float_struct *fs2,
+					u32 scale, bool div0_ok, s64 *val)
+{
+	bool sign = fs1->sign ^ fs2->sign;
+	u64 scaled_significand;
+	int shift_r;
+
+	if (unlikely(fs1->inf && fs2->inf))
+		return -EINVAL;
+
+	if (fs2->significand == 0) {
+		if (div0_ok) {
+			*val = 0;
+			return 0;
+		}
+		return -EINVAL;
+	}
+
+	if (unlikely(fs1->inf))
+		return hwmon_fp_infinity_to_s64(sign, val);
+
+	if (unlikely(fs2->inf) || fs1->significand == 0) {
+		*val = 0;
+		return 0;
+	}
+
+	/*
+	 * Make the dividend as large as possible to improve accuracy, otherwise
+	 * the divide-and-right-shift procedure may produce an inaccurate result.
+	 *
+	 *   scale * (significand1 * 2^-shift_r1) / (significand2 * 2^-shift_r2)
+	 * = scale * 2^6 * 2^-6 * (significand1 * 2^-shift_r1) / (significand2 * 2^-shift_r2)
+	 * = (((scale * 2^6) * significand1) / significand2) * 2^-(shift_r1 - shift_r2 + 6)
+	 * = (((scale << 6) * significand1) / significand2) >> (shift_r1 - shift_r2 + 6)
+	 *
+	 * This will never overflow: (2^32 - 1) * 2^6 * (2^24 - 1) < (2^62 - 1).
+	 */
+	scaled_significand = ((u64)scale << HWMON_FP_SCALE_MAGNIFY_SHIFT_L) * (u64)fs1->significand;
+	scaled_significand =
+		(scaled_significand + (u64)fs2->significand / 2) / (u64)fs2->significand;
+
+	shift_r = fs1->shift_r - fs2->shift_r + HWMON_FP_SCALE_MAGNIFY_SHIFT_L;
+
+	return hwmon_fp_raw_to_s64(scaled_significand, shift_r, sign, val);
+}
+
+int hwmon_fp_div_to_s64_unsafe(u32 flt1, u32 flt2, u32 scale, bool div0_ok, s64 *val)
+{
+	struct float_struct fs1, fs2;
+	int ret;
+
+	ret = hwmon_fp_parse_float(flt1, &fs1) || hwmon_fp_parse_float(flt2, &fs2);
+	if (ret)
+		return ret;
+
+	return __hwmon_fp_div_to_s64_unsafe(&fs1, &fs2, scale, div0_ok, val);
+}
+EXPORT_SYMBOL_GPL(hwmon_fp_div_to_s64_unsafe);
+
+MODULE_AUTHOR("Rong Zhang <i@rong.moe>");
+MODULE_DESCRIPTION("hwmon floating-point number to integer conversions");
+MODULE_LICENSE("GPL");
diff --git a/drivers/hwmon/hwmon-fp.h b/drivers/hwmon/hwmon-fp.h
new file mode 100644
index 000000000000..55d6a5021535
--- /dev/null
+++ b/drivers/hwmon/hwmon-fp.h
@@ -0,0 +1,212 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Floating-point number to integer conversions
+ *
+ * Copyright (c) 2026 Rong Zhang <i@rong.moe>
+ */
+
+#ifndef __HWMON_FP_H__
+#define __HWMON_FP_H__
+
+#include <asm/bitsperlong.h>
+#include <linux/bug.h>
+#include <linux/build_bug.h>
+#include <linux/compiler.h>
+#include <linux/limits.h>
+#include <linux/types.h>
+#include <linux/units.h>
+
+int hwmon_fp_float_to_s64_unsafe(u32 flt, u32 scale, s64 *val);
+int hwmon_fp_mul_to_s64_unsafe(u32 flt1, u32 flt2, u32 scale_ntz, ulong scale_ctz, s64 *val);
+int hwmon_fp_div_to_s64_unsafe(u32 flt1, u32 flt2, u32 scale, bool div0_ok, s64 *val);
+
+#define HWMON_FP_FLOAT_SIGNIFICAND_BITS	24
+#define HWMON_FP_FLOAT_SIGNIFICAND_MAX	BIT(HWMON_FP_FLOAT_SIGNIFICAND_BITS)
+
+#define HWMON_FP_MUL_SCALE_MAX		BIT(64 - HWMON_FP_FLOAT_SIGNIFICAND_BITS * 2)
+
+static inline int __hwmon_fp_check_scale(u32 scale)
+{
+	return WARN_ON(scale <= 0) ? -EINVAL : 0;
+}
+
+#define hwmon_fp_check_scale(scale)				\
+	__builtin_choose_expr(__is_constexpr(scale),		\
+			      BUILD_BUG_ON_ZERO((scale) <= 0),	\
+			      __hwmon_fp_check_scale(scale))
+
+#define HWMON_FP_SCALE_IN	MILLI	/* millivolt		 (mV) */
+#define HWMON_FP_SCALE_TEMP	MILLI	/* millidegree Celsius	(m°C) */
+#define HWMON_FP_SCALE_CURR	MILLI	/* milliampere		 (mA) */
+#define HWMON_FP_SCALE_POWER	MICRO	/* microWatt		 (uW) */
+#define HWMON_FP_SCALE_ENERGY	MICRO	/* microJoule		 (uJ) */
+
+/**
+ * hwmon_fp_float_to_s64() - Convert a float (binary32) number into a signed
+ * 64-bit integer.
+ * @flt:	Float (binary32) number.
+ * @scale:	Scale factor.
+ * @val:	Pointer to store the converted value.
+ *
+ * Special case:
+ *             inf -> S64_MAX or S64_MIN
+ *             NaN -> -EINVAL;
+ *      (overflow) -> S64_MAX or S64_MIN
+ *     (underflow) -> 0
+ *
+ * Return: 0 on success, or an error code.
+ */
+#define hwmon_fp_float_to_s64(flt, scale, val)		\
+	(hwmon_fp_check_scale(scale) ||			\
+	 hwmon_fp_float_to_s64_unsafe(flt, scale, val))
+
+/*
+ * Handling multification is very tricky, as large scale factors must not lead
+ * to overflow. Fortunately, cutting off all trailing zeros and restoring them
+ * while right shifting is enough reduce the scale factor used in
+ * multiplication to a small enough value.
+ */
+static inline int __hwmon_fp_mul_to_s64(u32 flt1, u32 flt2, u32 scale, s64 *val)
+{
+	ulong scale_ctz;
+
+	if (WARN_ON(scale <= 0))
+		return -EINVAL;
+
+	scale_ctz = __ffs(scale);
+	scale >>= scale_ctz;
+
+	if (WARN_ON(scale >= HWMON_FP_MUL_SCALE_MAX))
+		return -EINVAL;
+
+	return hwmon_fp_mul_to_s64_unsafe(flt1, flt2, scale, scale_ctz, val);
+}
+
+#define __hwmon_fp_mul_to_s64_const(flt1, flt2, scale, val)			\
+({										\
+	u32 _scale_ntz = (scale);						\
+	ulong _scale_ctz;							\
+										\
+	BUILD_BUG_ON(_scale_ntz <= 0);						\
+										\
+	_scale_ctz = __builtin_ctzl(_scale_ntz);				\
+	_scale_ntz >>= _scale_ctz;						\
+										\
+	BUILD_BUG_ON(_scale_ntz >= HWMON_FP_MUL_SCALE_MAX);			\
+										\
+	hwmon_fp_mul_to_s64_unsafe(flt1, flt2, _scale_ntz, _scale_ctz, val);	\
+})
+
+/**
+ * hwmon_fp_mul_to_s64() - Multiply two float (binary32) numbers and convert the
+ * product into a signed 64-bit integer.
+ * @flt1:	Multiplicand stored in float (binary32) format.
+ * @flt2:	Multiplier stored in float (binary32) format.
+ * @scale:	Scale factor.
+ * @val:	Pointer to store the product.
+ *
+ * Calculate @scale * @flt1 * @flt2.
+ *
+ * Special case:
+ *     0 * inf -> -EINVAL
+ *     x * inf -> S64_MAX or S64_MIN
+ *     x *   0 -> 0
+ *
+ * Return: 0 on success, or an error code.
+ */
+#define hwmon_fp_mul_to_s64(flt1, flt2, scale, val)			\
+	__builtin_choose_expr(__is_constexpr(scale),			\
+		__hwmon_fp_mul_to_s64_const(flt1, flt2, scale, val),	\
+		__hwmon_fp_mul_to_s64(flt1, flt2, scale, val))
+
+/**
+ * hwmon_fp_div_to_s64() - Divide two float (binary32) numbers and convert the
+ * quotient into a signed 64-bit integer.
+ * @flt1:	Dividend stored in float (binary32) format.
+ * @flt2:	Divisor stored in float (binary32) format.
+ * @scale:	Scale factor.
+ * @div0_ok:	If true, return 0 when @flt2 is 0. Otherwise, -EINVAL is returned.
+ * @val:	Pointer to store the quotient.
+ *
+ * Calculate @scale * (@flt1 / @flt2).
+ *
+ * Special case:
+ *     inf / inf -> -EINVAL
+ *     inf /   x -> S64_MAX or S64_MIN
+ *       x /   0 -> See div0_ok
+ *       x / inf -> 0
+ *       0 /   x -> 0
+ *
+ * Return: 0 on success, or an error code.
+ */
+#define hwmon_fp_div_to_s64(flt1, flt2, scale, div0_ok, val)		\
+	(hwmon_fp_check_scale(scale) ||					\
+	 hwmon_fp_div_to_s64_unsafe(flt1, flt2, scale, div0_ok, val))
+
+#if BITS_PER_LONG == 64
+
+static_assert(sizeof(long) == sizeof(s64));
+
+static inline long hwmon_fp_s64_to_long(s64 val64)
+{
+	return val64;
+}
+
+# define hwmon_fp_float_to_long(flt, scale, val) \
+	hwmon_fp_float_to_s64(flt, scale, (s64 *)val)
+# define hwmon_fp_div_to_long(flt1, flt2, scale, div0_ok, val) \
+	hwmon_fp_div_to_s64(flt1, flt2, scale, div0_ok, (s64 *)val)
+# define hwmon_fp_mul_to_long(flt1, flt2, scale, val) \
+	hwmon_fp_mul_to_s64(flt1, flt2, scale, (s64 *)val)
+
+#else /* BITS_PER_LONG == 64 */
+
+static inline long hwmon_fp_s64_to_long(s64 val64)
+{
+	if (unlikely(val64 > LONG_MAX))
+		return LONG_MAX;
+	else if (unlikely(val64 < LONG_MIN))
+		return LONG_MIN;
+	else
+		return val64;
+}
+
+# define hwmon_fp_float_to_long(flt, scale, val)		\
+({								\
+	s64 _val64;						\
+	int _ret;						\
+								\
+	_ret = hwmon_fp_float_to_s64(flt, scale, &_val64);	\
+	if (!_ret)						\
+		*(val) = hwmon_fp_s64_to_long(_val64);		\
+								\
+	_ret;							\
+})
+
+# define hwmon_fp_div_to_long(flt1, flt2, scale, div0_ok, val)			\
+({										\
+	s64 _val64;								\
+	int _ret;								\
+										\
+	_ret = hwmon_fp_div_to_s64(flt1, flt2, scale, div0_ok, &_val64);	\
+	if (!_ret)								\
+		*(val) = hwmon_fp_s64_to_long(_val64);				\
+										\
+	_ret;									\
+})
+
+# define hwmon_fp_mul_to_long(flt1, flt2, scale, val)		\
+({								\
+	s64 _val64;						\
+	int _ret;						\
+								\
+	_ret = hwmon_fp_mul_to_s64(flt1, flt2, scale, &_val64);	\
+	if (!_ret)						\
+		*(val) = hwmon_fp_s64_to_long(_val64);		\
+								\
+	_ret;							\
+})
+
+#endif /* BITS_PER_LONG == 64 */
+
+#endif /* __HWMON_FP_H__ */

-- 
2.53.0

[PATCH 3/4] hwmon: Add barebone HID driver for WITRN

[Rong Zhang]

WITRN produces a series of devices to monitor power characteristics of
USB connections and display those on a on-device display. Most of them
contain an additional port which exposes the measurements via USB HID.

Add a barebone HID driver to collect these measurements. The monitoring
support can be implemented later.

Developed and tested with WITRN K2 USB-C tester.

Signed-off-by: Rong Zhang <i@rong.moe>
---
 Documentation/hwmon/index.rst |   1 +
 Documentation/hwmon/witrn.rst |  23 ++++
 MAINTAINERS                   |   7 ++
 drivers/hwmon/Kconfig         |  10 ++
 drivers/hwmon/Makefile        |   1 +
 drivers/hwmon/witrn.c         | 268 ++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 310 insertions(+)

diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
index b2ca8513cfcd..f1f2b599c76b 100644
--- a/Documentation/hwmon/index.rst
+++ b/Documentation/hwmon/index.rst
@@ -276,6 +276,7 @@ Hardware Monitoring Kernel Drivers
    w83795
    w83l785ts
    w83l786ng
+   witrn
    wm831x
    wm8350
    xgene-hwmon
diff --git a/Documentation/hwmon/witrn.rst b/Documentation/hwmon/witrn.rst
new file mode 100644
index 000000000000..e64c527928d0
--- /dev/null
+++ b/Documentation/hwmon/witrn.rst
@@ -0,0 +1,23 @@
+.. SPDX-License-Identifier: GPL-2.0+
+
+Kernel driver witrn
+====================
+
+Supported chips:
+
+  * WITRN K2
+
+    Prefix: 'witrn'
+
+    Addresses scanned: -
+
+Author:
+
+  - Rong Zhang <i@rong.moe>
+
+Description
+-----------
+
+This driver implements support for the WITRN USB tester family.
+
+The device communicates with the custom protocol over USB HID.
diff --git a/MAINTAINERS b/MAINTAINERS
index 0481aca2286c..18a1077d38e7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -28444,6 +28444,13 @@ M:	Miloslav Trmac <mitr@volny.cz>
 S:	Maintained
 F:	drivers/input/misc/wistron_btns.c
 
+WITRN USB TESTER HARDWARE MONITOR DRIVER
+M:	Rong Zhang <i@rong.moe>
+L:	linux-hwmon@vger.kernel.org
+S:	Maintained
+F:	Documentation/hwmon/witrn.rst
+F:	drivers/hwmon/witrn.c
+
 WMI BINARY MOF DRIVER
 M:	Armin Wolf <W_Armin@gmx.de>
 R:	Thomas Weißschuh <linux@weissschuh.net>
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 7ad909033e79..746184608f81 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -2629,6 +2629,16 @@ config SENSORS_W83627EHF
 	  This driver can also be built as a module. If so, the module
 	  will be called w83627ehf.
 
+config SENSORS_WITRN
+	tristate "WITRN USB tester"
+	depends on USB_HID
+	help
+	  If you say yes here you get support for WITRN USB charging
+	  testers.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called witrn.
+
 config SENSORS_WM831X
 	tristate "WM831x PMICs"
 	depends on MFD_WM831X
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 6dba69f712be..f87eb1710974 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -243,6 +243,7 @@ obj-$(CONFIG_SENSORS_VT8231)	+= vt8231.o
 obj-$(CONFIG_SENSORS_W83627EHF)	+= w83627ehf.o
 obj-$(CONFIG_SENSORS_W83L785TS)	+= w83l785ts.o
 obj-$(CONFIG_SENSORS_W83L786NG)	+= w83l786ng.o
+obj-$(CONFIG_SENSORS_WITRN)	+= witrn.o
 obj-$(CONFIG_SENSORS_WM831X)	+= wm831x-hwmon.o
 obj-$(CONFIG_SENSORS_WM8350)	+= wm8350-hwmon.o
 obj-$(CONFIG_SENSORS_XGENE)	+= xgene-hwmon.o
diff --git a/drivers/hwmon/witrn.c b/drivers/hwmon/witrn.c
new file mode 100644
index 000000000000..e8713da6de5a
--- /dev/null
+++ b/drivers/hwmon/witrn.c
@@ -0,0 +1,268 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * witrn - Driver for WITRN USB charging testers
+ *
+ * Copyright (C) 2026 Rong Zhang <i@rong.moe>
+ */
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/bitops.h>
+#include <linux/completion.h>
+#include <linux/cleanup.h>
+#include <linux/device.h>
+#include <linux/hid.h>
+#include <linux/jiffies.h>
+#include <linux/limits.h>
+#include <linux/module.h>
+#include <linux/spinlock.h>
+#include <linux/types.h>
+#include <linux/units.h>
+#include <linux/workqueue.h>
+
+#define DRIVER_NAME		"witrn"
+#define WITRN_EP_CMD_OUT	0x01
+#define WITRN_EP_DATA_IN	0x81
+
+#define WITRN_REPORT_SZ		64
+
+/* flags */
+#define WITRN_HID_OPENED	0
+
+/*
+ * The device sends reports every 10ms (100Hz!) once it's opened, which is
+ * really annoying and produces a lot of irq noise.
+ *
+ * Unfortunately, the device doesn't provide any command to start/stop reporting
+ * on demand -- it simply spams reports blindly. The only way to stop reporting
+ * is to close the HID device (i.e., to stop IN URB (re)submission).
+ *
+ * Let's close the HID device if the device has not been accessed for a while.
+ */
+#define PAUSE_TIMEOUT		secs_to_jiffies(8)
+#define UP_TO_DATE_TIMEOUT	msecs_to_jiffies(100)
+
+enum witrn_report_type {
+	WITRN_PD		= 0xfe,
+	WITRN_SENSOR		= 0xff,
+};
+
+struct witrn_sensor {
+	__le16	record_threshold;	/* mA */
+	__le32	record_charge;		/* Ah (float) */
+	__le32	record_energy;		/* Wh (float) */
+	__le32	record_time;		/* s */
+	__le32	uptime;			/* s */
+	__le32	vdp;			/* V (float) */
+	__le32	vdm;			/* V (float) */
+	u8	__unknown[4];
+	__le32	temp_ntc;		/* Celsius (float) */
+	__le32	vbus;			/* V (float) */
+	__le32	ibus;			/* A (float) */
+	u8	record_group;		/* 0: group 1 on device, ... */
+	u8	vcc1;			/* dV */
+	u8	vcc2;			/* dV */
+} __packed;
+
+struct witrn_report {
+	u8	report_type;
+	u8	__unknown_0[11];
+
+	struct witrn_sensor sensor;
+
+	u8	__unknown_1[7];
+} __packed;
+static_assert(sizeof(struct witrn_report) == WITRN_REPORT_SZ);
+
+struct witrn_priv {
+	struct hid_device *hdev;
+
+	struct work_struct pause_work;
+
+	unsigned long flags;
+
+	spinlock_t lock; /* Protects members below */
+
+	struct completion completion;
+	unsigned long last_update; /* jiffies */
+	unsigned long last_access; /* jiffies */
+
+	struct witrn_sensor sensor;
+};
+
+static inline bool sensor_is_outdated(struct witrn_priv *priv)
+{
+	return time_after(jiffies, priv->last_update + UP_TO_DATE_TIMEOUT);
+}
+
+static inline bool hwmon_is_inactive(struct witrn_priv *priv)
+{
+	return time_after(jiffies, priv->last_access + PAUSE_TIMEOUT);
+}
+
+/* ======== HID ======== */
+
+static int witrn_open_hid(struct witrn_priv *priv)
+{
+	int ret;
+
+	if (test_and_set_bit(WITRN_HID_OPENED, &priv->flags))
+		return 0; /* Already opened */
+
+	hid_dbg(priv->hdev, "opening hid hw\n");
+
+	ret = hid_hw_open(priv->hdev);
+	if (ret) {
+		hid_err(priv->hdev, "hid hw open failed with %d\n", ret);
+		clear_bit(WITRN_HID_OPENED, &priv->flags);
+	}
+
+	return ret;
+}
+
+static void witrn_close_hid(struct witrn_priv *priv)
+{
+	if (!test_and_clear_bit(WITRN_HID_OPENED, &priv->flags))
+		return; /* Already closed */
+
+	hid_dbg(priv->hdev, "closing hid hw\n");
+
+	hid_hw_close(priv->hdev);
+}
+
+static void witrn_pause_hid(struct work_struct *work)
+{
+	struct witrn_priv *priv = container_of(work, struct witrn_priv, pause_work);
+
+	scoped_guard(spinlock, &priv->lock) {
+		/* Double check. Condition may change after being scheduled. */
+		if (!hwmon_is_inactive(priv))
+			return;
+	}
+
+	witrn_close_hid(priv);
+}
+
+static int witrn_raw_event(struct hid_device *hdev, struct hid_report *report,
+			   u8 *data, int size)
+{
+	struct witrn_priv *priv = hid_get_drvdata(hdev);
+	const struct witrn_report *wreport;
+	bool do_pause = false;
+
+	/* HIDRAW has opened the device while we are pausing. */
+	if (!test_bit(WITRN_HID_OPENED, &priv->flags))
+		return 0;
+
+	if (size < WITRN_REPORT_SZ) {
+		hid_dbg(hdev, "report size mismatch: %d < %d\n", size, WITRN_REPORT_SZ);
+		return 0;
+	}
+
+	wreport = (const struct witrn_report *)data;
+	if (wreport->report_type != WITRN_SENSOR) {
+		hid_dbg(hdev, "report ignored with type 0x%02x", wreport->report_type);
+		return 0;
+	}
+
+	scoped_guard(spinlock, &priv->lock) {
+		priv->last_update = jiffies;
+		do_pause = hwmon_is_inactive(priv);
+
+		memcpy(&priv->sensor, &wreport->sensor, sizeof(wreport->sensor));
+		complete(&priv->completion);
+	}
+
+	if (do_pause)
+		schedule_work(&priv->pause_work);
+
+	return 0;
+}
+
+static int witrn_probe(struct hid_device *hdev, const struct hid_device_id *id)
+{
+	struct device *parent = &hdev->dev;
+	struct witrn_priv *priv;
+	int ret;
+
+	priv = devm_kzalloc(parent, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->hdev = hdev;
+	hid_set_drvdata(hdev, priv);
+
+	ret = hid_parse(hdev);
+	if (ret) {
+		hid_err(hdev, "hid parse failed with %d\n", ret);
+		return ret;
+	}
+
+	/* Enable HIDRAW so existing user-space tools can continue to work. */
+	ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
+	if (ret) {
+		hid_err(hdev, "hid hw start failed with %d\n", ret);
+		return ret;
+	}
+
+	spin_lock_init(&priv->lock);
+	init_completion(&priv->completion);
+
+	INIT_WORK(&priv->pause_work, witrn_pause_hid);
+
+	priv->last_access = jiffies;
+	priv->last_update = priv->last_access - UP_TO_DATE_TIMEOUT - 1;
+	clear_bit(WITRN_HID_OPENED, &priv->flags);
+
+	ret = witrn_open_hid(priv);
+	if (ret) {
+		hid_hw_stop(hdev);
+		return ret;
+	}
+
+	return 0;
+}
+
+static void witrn_remove(struct hid_device *hdev)
+{
+	struct witrn_priv *priv = hid_get_drvdata(hdev);
+
+	witrn_close_hid(priv);
+
+	/* Cancel it after closing HID so that it won't be rescheduled. */
+	cancel_work_sync(&priv->pause_work);
+
+	hid_hw_stop(hdev);
+}
+
+static const struct hid_device_id witrn_id_table[] = {
+	{ HID_USB_DEVICE(0x0716, 0x5060) },	/* WITRN K2 USB-C tester */
+	{ }
+};
+
+MODULE_DEVICE_TABLE(hid, witrn_id_table);
+
+static struct hid_driver witrn_driver = {
+	.name = DRIVER_NAME,
+	.id_table = witrn_id_table,
+	.probe = witrn_probe,
+	.remove = witrn_remove,
+	.raw_event = witrn_raw_event,
+};
+
+static int __init witrn_init(void)
+{
+	return hid_register_driver(&witrn_driver);
+}
+
+static void __exit witrn_exit(void)
+{
+	hid_unregister_driver(&witrn_driver);
+}
+
+/* When compiled into the kernel, initialize after the HID bus */
+late_initcall(witrn_init);
+module_exit(witrn_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Rong Zhang <i@rong.moe>");
+MODULE_DESCRIPTION("WITRN USB tester driver");

-- 
2.53.0

[PATCH 4/4] hwmon: (witrn) Add monitoring support

[Rong Zhang]

With sensor data collected, they can be exported to userspace via hwmon.

Register hwmon driver for witrn, with appropriate channels and
attributes.

Developed and tested with WITRN K2 USB-C tester. Theoretically this
driver should work properly on other models. They can be added to the
HID match table too if someone tests the driver with them.

Signed-off-by: Rong Zhang <i@rong.moe>
---
 Documentation/hwmon/witrn.rst |  30 +++
 drivers/hwmon/Kconfig         |   1 +
 drivers/hwmon/witrn.c         | 423 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 454 insertions(+)

diff --git a/Documentation/hwmon/witrn.rst b/Documentation/hwmon/witrn.rst
index e64c527928d0..f13323fdd9d9 100644
--- a/Documentation/hwmon/witrn.rst
+++ b/Documentation/hwmon/witrn.rst
@@ -21,3 +21,33 @@ Description
 This driver implements support for the WITRN USB tester family.
 
 The device communicates with the custom protocol over USB HID.
+
+As current can flow in both directions through the tester the sign of the
+channel "curr1_input" (label "IBUS") indicates the direction.
+
+Sysfs entries
+-------------
+
+  ===============  ==========  ==============================================================
+  Name             Label       Description
+  ===============  ==========  ==============================================================
+  in0_input        VBUS        Measured VBUS voltage (mV)
+  in0_average      VBUS        Calculated average VBUS voltage (mV)
+  in1_input        D+          Measured D+ voltage (mV)
+  in2_input        D-          Measured D- voltage (mV)
+  in3_input        CC1         Measured CC1 voltage (mV)
+  in4_input        CC2         Measured CC2 voltage (mV)
+  cur1_input       IBUS        Measured VBUS current (mA)
+  curr1_average    IBUS        Calculated average VBUS current (mA)
+  curr1_rated_min  IBUS        Stop accumulating (recording) below this VBUS current (mA)
+  power1_input     PBUS        Calculated VBUS power (uW)
+  power1_average   PBUS        Calculated average VBUS power (uW)
+  energy1_input    EBUS        Accumulated VBUS energy (uJ)
+  charge1_input    CBUS        Accumulated VBUS charge (mC)
+  temp1_input      Thermistor  Measured thermistor temperature (m°C), -EXDEV if not connected
+  record_group                 ID of the record group for accumulative values
+  record_time                  Accumulated time for recording (s), see also curr1_rated_min
+  uptime                       Accumulated time since the device has been powered on (s)
+  ===============  ==========  ==============================================================
+
+All entries are readonly.
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 746184608f81..c8b5144707a1 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -2632,6 +2632,7 @@ config SENSORS_W83627EHF
 config SENSORS_WITRN
 	tristate "WITRN USB tester"
 	depends on USB_HID
+	select HWMON_FP
 	help
 	  If you say yes here you get support for WITRN USB charging
 	  testers.
diff --git a/drivers/hwmon/witrn.c b/drivers/hwmon/witrn.c
index e8713da6de5a..f43bdbf13435 100644
--- a/drivers/hwmon/witrn.c
+++ b/drivers/hwmon/witrn.c
@@ -11,14 +11,19 @@
 #include <linux/cleanup.h>
 #include <linux/device.h>
 #include <linux/hid.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
 #include <linux/jiffies.h>
 #include <linux/limits.h>
 #include <linux/module.h>
 #include <linux/spinlock.h>
+#include <linux/sysfs.h>
 #include <linux/types.h>
 #include <linux/units.h>
 #include <linux/workqueue.h>
 
+#include "hwmon-fp.h"
+
 #define DRIVER_NAME		"witrn"
 #define WITRN_EP_CMD_OUT	0x01
 #define WITRN_EP_DATA_IN	0x81
@@ -74,6 +79,7 @@ struct witrn_report {
 static_assert(sizeof(struct witrn_report) == WITRN_REPORT_SZ);
 
 struct witrn_priv {
+	struct device *hwmon_dev;
 	struct hid_device *hdev;
 
 	struct work_struct pause_work;
@@ -178,6 +184,413 @@ static int witrn_raw_event(struct hid_device *hdev, struct hid_report *report,
 	return 0;
 }
 
+/* ======== HWMON ======== */
+
+static int witrn_collect_sensor(struct witrn_priv *priv, struct witrn_sensor *sensor)
+{
+	int ret;
+
+	scoped_guard(spinlock, &priv->lock) {
+		priv->last_access = jiffies;
+
+		if (!sensor_is_outdated(priv)) {
+			memcpy(sensor, &priv->sensor, sizeof(priv->sensor));
+			return 0;
+		}
+
+		reinit_completion(&priv->completion);
+	}
+
+	ret = witrn_open_hid(priv);
+	if (ret)
+		return ret;
+
+	ret = wait_for_completion_interruptible_timeout(&priv->completion,
+							UP_TO_DATE_TIMEOUT);
+	if (ret == 0)
+		return -ETIMEDOUT;
+	else if (ret < 0)
+		return ret;
+
+	scoped_guard(spinlock, &priv->lock)
+		memcpy(sensor, &priv->sensor, sizeof(priv->sensor));
+
+	return 0;
+}
+
+#define SECS_PER_HOUR		3600ULL
+#define WITRN_SCALE_IN_VCC	(HWMON_FP_SCALE_IN / DECI)		/* dV to mV */
+#define WITRN_SCALE_CHARGE	(HWMON_FP_SCALE_CURR * SECS_PER_HOUR)	/* Ah to mC(mAs) */
+#define WITRN_SCALE_ENERGY	(HWMON_FP_SCALE_ENERGY * SECS_PER_HOUR)	/* Wh to uJ(uWs) */
+
+static int witrn_read_in(const struct witrn_sensor *sensor, u32 attr, int channel, long *val)
+{
+	switch (attr) {
+	case hwmon_in_input:
+		switch (channel) {
+		case 0:
+			return hwmon_fp_float_to_long(le32_to_cpu(sensor->vbus),
+						      HWMON_FP_SCALE_IN, val);
+		case 1:
+			return hwmon_fp_float_to_long(le32_to_cpu(sensor->vdp),
+						      HWMON_FP_SCALE_IN, val);
+		case 2:
+			return hwmon_fp_float_to_long(le32_to_cpu(sensor->vdm),
+						      HWMON_FP_SCALE_IN, val);
+		case 3:
+			*val = sensor->vcc1 * WITRN_SCALE_IN_VCC;
+			return 0;
+		case 4:
+			*val = sensor->vcc2 * WITRN_SCALE_IN_VCC;
+			return 0;
+		default:
+			return -EOPNOTSUPP;
+		}
+	case hwmon_in_average:
+		switch (channel) {
+		case 0:
+			return hwmon_fp_div_to_long(le32_to_cpu(sensor->record_energy),
+						    le32_to_cpu(sensor->record_charge),
+						    HWMON_FP_SCALE_IN, true, val);
+		default:
+			return -EOPNOTSUPP;
+		}
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static int witrn_read_curr(const struct witrn_sensor *sensor, u32 attr, int channel, long *val)
+{
+	int ret;
+
+	switch (attr) {
+	case hwmon_curr_input:
+		switch (channel) {
+		case 0:
+			return hwmon_fp_float_to_long(le32_to_cpu(sensor->ibus),
+						      HWMON_FP_SCALE_CURR, val);
+		default:
+			return -EOPNOTSUPP;
+		}
+	case hwmon_curr_average:
+		switch (channel) {
+		case 0: {
+			s64 record_time = le32_to_cpu(sensor->record_time);
+			s64 capacity; /* mC(mAs) */
+
+			if (record_time == 0) {
+				*val = 0;
+				return 0;
+			}
+
+			ret = hwmon_fp_float_to_s64(le32_to_cpu(sensor->record_charge),
+						    WITRN_SCALE_CHARGE, &capacity);
+			if (ret)
+				return ret;
+
+			/* mC(mAs) / s = mA */
+			*val = hwmon_fp_s64_to_long(capacity / record_time);
+			return 0;
+		}
+		default:
+			return -EOPNOTSUPP;
+		}
+	case hwmon_curr_rated_min:
+		switch (channel) {
+		case 0:
+			*val = le16_to_cpu(sensor->record_threshold); /* already in mA */
+			return 0;
+		default:
+			return -EOPNOTSUPP;
+		}
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static int witrn_read_power(const struct witrn_sensor *sensor, u32 attr, int channel, long *val)
+{
+	int ret;
+
+	switch (attr) {
+	case hwmon_power_input:
+		switch (channel) {
+		case 0:
+			/*
+			 * The device provides 1e-5 precision.
+			 *
+			 * Though userspace programs can calculate (VBUS * IBUS)
+			 * themselves, this channel is provided for convenience
+			 * and accuracy.
+			 *
+			 * E.g., when VBUS = 5.00049V and IBUS = 0.50049A,
+			 * userspace calculates 5.000V * 0.500A = 2.500000W,
+			 * while this channel reports 2.502695W.
+			 */
+			return hwmon_fp_mul_to_long(le32_to_cpu(sensor->vbus),
+						    le32_to_cpu(sensor->ibus),
+						    HWMON_FP_SCALE_POWER, val);
+		default:
+			return -EOPNOTSUPP;
+		}
+	case hwmon_power_average:
+		switch (channel) {
+		case 0: {
+			s64 record_time = le32_to_cpu(sensor->record_time);
+			s64 energy; /* uJ(uWs) */
+
+			if (record_time == 0) {
+				*val = 0;
+				return 0;
+			}
+
+			ret = hwmon_fp_float_to_s64(le32_to_cpu(sensor->record_energy),
+						    WITRN_SCALE_ENERGY, &energy);
+			if (ret)
+				return ret;
+
+			/* uJ(uWs) / s = uW */
+			*val = hwmon_fp_s64_to_long(energy / record_time);
+			return 0;
+		}
+		default:
+			return -EOPNOTSUPP;
+		}
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static int witrn_read_temp(const struct witrn_sensor *sensor, u32 attr, int channel, long *val)
+{
+	int ret;
+
+	switch (attr) {
+	case hwmon_temp_input:
+		switch (channel) {
+		case 0:
+			ret = hwmon_fp_float_to_long(le32_to_cpu(sensor->temp_ntc),
+						     HWMON_FP_SCALE_TEMP, val);
+
+			/*
+			 * The thermistor (NTC, B=3435, T0=25°C, R0=10kohm) is an optional
+			 * addon. When it's missing, an extremely cold temperature
+			 * (-50°C - -80°C) is reported as the device deduced a very large
+			 * resistance value (~500Kohm - ~5Mohm).
+			 *
+			 * We choose -40°C (~250kohm) as the threshold to determine whether
+			 * the thermistor is connected.
+			 *
+			 * The addon can be connected to the device after the device being
+			 * connected to the PC, so we can't use is_visible to hide it.
+			 */
+			if (!ret && *val < -40L * (long)HWMON_FP_SCALE_TEMP)
+				return -EXDEV;
+
+			return ret;
+		default:
+			return -EOPNOTSUPP;
+		}
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static int witrn_read_energy(const struct witrn_sensor *sensor, u32 attr, int channel, s64 *val)
+{
+	switch (attr) {
+	case hwmon_energy_input:
+		switch (channel) {
+		case 0:
+			return hwmon_fp_float_to_s64(le32_to_cpu(sensor->record_energy),
+						     WITRN_SCALE_ENERGY, val);
+		default:
+			return -EOPNOTSUPP;
+		}
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static int witrn_read(struct device *dev, enum hwmon_sensor_types type,
+		      u32 attr, int channel, long *val)
+{
+	struct witrn_priv *priv = dev_get_drvdata(dev);
+	struct witrn_sensor sensor;
+	int ret;
+
+	ret = witrn_collect_sensor(priv, &sensor);
+	if (ret)
+		return ret;
+
+	switch (type) {
+	case hwmon_in:
+		return witrn_read_in(&sensor, attr, channel, val);
+	case hwmon_curr:
+		return witrn_read_curr(&sensor, attr, channel, val);
+	case hwmon_power:
+		return witrn_read_power(&sensor, attr, channel, val);
+	case hwmon_temp:
+		return witrn_read_temp(&sensor, attr, channel, val);
+	case hwmon_energy64:
+		return witrn_read_energy(&sensor, attr, channel, (s64 *)val);
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static int witrn_read_string(struct device *dev, enum hwmon_sensor_types type,
+			     u32 attr, int channel, const char **str)
+{
+	static const char * const in_labels[] = {
+		"VBUS",
+		"D+",
+		"D-",
+		"CC1",
+		"CC2",
+	};
+	static const char * const curr_labels[] = {
+		"IBUS", /* VBUS current */
+	};
+	static const char * const power_labels[] = {
+		"PBUS", /* VBUS power */
+	};
+	static const char * const energy_labels[] = {
+		"EBUS", /* VBUS energy */
+	};
+	static const char * const temp_labels[] = {
+		"Thermistor",
+	};
+
+	if (type == hwmon_in && attr == hwmon_in_label &&
+	    channel < ARRAY_SIZE(in_labels)) {
+		*str = in_labels[channel];
+	} else if (type == hwmon_curr && attr == hwmon_curr_label &&
+		   channel < ARRAY_SIZE(curr_labels)) {
+		*str = curr_labels[channel];
+	} else if (type == hwmon_power && attr == hwmon_power_label &&
+		   channel < ARRAY_SIZE(power_labels)) {
+		*str = power_labels[channel];
+	} else if (type == hwmon_energy64 && attr == hwmon_energy_label &&
+		   channel < ARRAY_SIZE(energy_labels)) {
+		*str = energy_labels[channel];
+	} else if (type == hwmon_temp && attr == hwmon_temp_label &&
+		   channel < ARRAY_SIZE(temp_labels)) {
+		*str = temp_labels[channel];
+	} else {
+		return -EOPNOTSUPP;
+	}
+
+	return 0;
+}
+
+static const struct hwmon_channel_info *const witrn_info[] = {
+	HWMON_CHANNEL_INFO(in,
+			   HWMON_I_INPUT | HWMON_I_LABEL | HWMON_I_AVERAGE,
+			   HWMON_I_INPUT | HWMON_I_LABEL,
+			   HWMON_I_INPUT | HWMON_I_LABEL,
+			   HWMON_I_INPUT | HWMON_I_LABEL,
+			   HWMON_I_INPUT | HWMON_I_LABEL),
+	HWMON_CHANNEL_INFO(curr,
+			   HWMON_C_INPUT | HWMON_C_LABEL | HWMON_C_AVERAGE | HWMON_C_RATED_MIN),
+	HWMON_CHANNEL_INFO(power,
+			   HWMON_P_INPUT | HWMON_P_LABEL | HWMON_P_AVERAGE),
+	HWMON_CHANNEL_INFO(energy64,
+			   HWMON_E_INPUT | HWMON_E_LABEL),
+	HWMON_CHANNEL_INFO(temp,
+			   HWMON_T_INPUT | HWMON_T_LABEL),
+	NULL
+};
+
+static const struct hwmon_ops witrn_hwmon_ops = {
+	.visible = 0444, /* Nothing is tunable from PC :-( */
+	.read = witrn_read,
+	.read_string = witrn_read_string,
+};
+
+static const struct hwmon_chip_info witrn_chip_info = {
+	.ops = &witrn_hwmon_ops,
+	.info = witrn_info,
+};
+
+enum witrn_attr_channel {
+	ATTR_CHARGE,
+	ATTR_RECORD_GROUP,
+	ATTR_RECORD_TIME,
+	ATTR_UPTIME,
+};
+
+static ssize_t witrn_attr_show(struct device *dev, struct device_attribute *attr,
+			       char *buf)
+{
+	enum witrn_attr_channel channel = to_sensor_dev_attr(attr)->index;
+	struct witrn_priv *priv = dev_get_drvdata(dev);
+	struct witrn_sensor sensor;
+	int ret;
+	s64 val;
+
+	ret = witrn_collect_sensor(priv, &sensor);
+	if (ret)
+		return ret;
+
+	switch (channel) {
+	case ATTR_CHARGE:
+		ret = hwmon_fp_float_to_s64(le32_to_cpu(sensor.record_charge),
+					    WITRN_SCALE_CHARGE, &val);
+		if (ret)
+			return ret;
+		break;
+	case ATTR_RECORD_GROUP:
+		/* +1 to match the index displayed on the meter. */
+		val = sensor.record_group + 1;
+		break;
+	case ATTR_RECORD_TIME:
+		val = le32_to_cpu(sensor.record_time);
+		break;
+	case ATTR_UPTIME:
+		val = le32_to_cpu(sensor.uptime);
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	return sysfs_emit(buf, "%lld\n", val);
+}
+
+static ssize_t witrn_attr_label_show(struct device *dev, struct device_attribute *attr,
+				     char *buf)
+{
+	enum witrn_attr_channel channel = to_sensor_dev_attr(attr)->index;
+	const char *str;
+
+	switch (channel) {
+	case ATTR_CHARGE:
+		str = "CBUS"; /* VBUS charge */
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	return sysfs_emit(buf, "%s\n", str);
+}
+
+static SENSOR_DEVICE_ATTR_RO(charge1_input, witrn_attr, ATTR_CHARGE);
+static SENSOR_DEVICE_ATTR_RO(charge1_label, witrn_attr_label, ATTR_CHARGE);
+static SENSOR_DEVICE_ATTR_RO(record_group, witrn_attr, ATTR_RECORD_GROUP);
+static SENSOR_DEVICE_ATTR_RO(record_time, witrn_attr, ATTR_RECORD_TIME);
+static SENSOR_DEVICE_ATTR_RO(uptime, witrn_attr, ATTR_UPTIME);
+
+static struct attribute *witrn_attrs[] = {
+	&sensor_dev_attr_charge1_input.dev_attr.attr,
+	&sensor_dev_attr_charge1_label.dev_attr.attr,
+	&sensor_dev_attr_record_group.dev_attr.attr,
+	&sensor_dev_attr_record_time.dev_attr.attr,
+	&sensor_dev_attr_uptime.dev_attr.attr,
+	NULL
+};
+ATTRIBUTE_GROUPS(witrn);
+
 static int witrn_probe(struct hid_device *hdev, const struct hid_device_id *id)
 {
 	struct device *parent = &hdev->dev;
@@ -219,6 +632,14 @@ static int witrn_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		return ret;
 	}
 
+	priv->hwmon_dev = hwmon_device_register_with_info(parent, DRIVER_NAME, priv,
+							  &witrn_chip_info, witrn_groups);
+	if (IS_ERR(priv->hwmon_dev)) {
+		witrn_close_hid(priv);
+		hid_hw_stop(hdev);
+		return PTR_ERR(priv->hwmon_dev);
+	}
+
 	return 0;
 }
 
@@ -226,6 +647,8 @@ static void witrn_remove(struct hid_device *hdev)
 {
 	struct witrn_priv *priv = hid_get_drvdata(hdev);
 
+	hwmon_device_unregister(priv->hwmon_dev);
+
 	witrn_close_hid(priv);
 
 	/* Cancel it after closing HID so that it won't be rescheduled. */

-- 
2.53.0

Re: [PATCH 1/4] hwmon: Add label support for 64-bit energy attributes

[Guenter Roeck]

On Fri, Mar 27, 2026 at 03:19:50AM +0800, Rong Zhang wrote:
> Since commit 0bcd01f757bc ("hwmon: Introduce 64-bit energy attribute
> support"), devices can report 64-bit energy values by selecting the
> sensor type "energy64". However, such sensors can't report their labels
> since is_string_attr() was not updated to match it.
> 
> Add label support for 64-bit energy attributes by updating
> is_string_attr() to match hwmon_energy64 in addition to hwmon_energy.
> 
> Signed-off-by: Rong Zhang <i@rong.moe>

Applied.

Thanks,
Guenter

Re: [PATCH 0/4] hwmon: Add WITRN USB tester driver

[Guenter Roeck]

On 3/26/26 12:19, Rong Zhang wrote:
> WITRN produces a series of devices to monitor power characteristics of
> USB connections and display those on a on-device display. Most of them
> contain an additional port which exposes the measurements via USB HID.
> 
> These devices report sensor values in IEEE-754 float (binary32) format.
> The driver must perform floating-point number to integer conversions to
> provide hwmon channels. Meanwhile, they also report accumulative float
> values, and simple division or multiplication turns them into useful
> hwmon channels.
> 
> Patch 1 adds label support for 64-bit energy attributes, as the driver
> needs it.
> 
> Patch 2 adds a helper module for floating-point to integer conversions,
> so that the conversion, multification and division methods can be used
> in this driver as well as other drivers (I am also working on another
> USB tester driver that needs it).
> 
> Patch 3 adds a barebone HID driver for WITRN K2.
> 
> Patch 4 adds hwmon channels and attributes to the driver.
> 
> Signed-off-by: Rong Zhang <i@rong.moe>
> ---
> Rong Zhang (4):
>        hwmon: Add label support for 64-bit energy attributes
>        hwmon: New helper module for floating-point to integer conversions

Nack. This is not a hwmon problem and should reside in a driver or in lib/
(if it is needed by multiple drivers).

>        hwmon: Add barebone HID driver for WITRN

Nack. This is the wrong place for such a driver. It should reside somewhere
in drivers/usb, or maybe in drivers/misc/.

>        hwmon: (witrn) Add monitoring support

This should be implemented as auxiliary driver.

Sashiko has a lot of feedback that you might want to address before
resubmitting.

https://sashiko.dev/#/patchset/20260327-b4-hwmon-witrn-v1-0-8d2f1896c045%40rong.moe

Thanks,
Guenter

> 
>   Documentation/hwmon/index.rst |   1 +
>   Documentation/hwmon/witrn.rst |  53 ++++
>   MAINTAINERS                   |   7 +
>   drivers/hwmon/Kconfig         |  14 +
>   drivers/hwmon/Makefile        |   2 +
>   drivers/hwmon/hwmon-fp.c      | 262 ++++++++++++++++
>   drivers/hwmon/hwmon-fp.h      | 212 +++++++++++++
>   drivers/hwmon/hwmon.c         |   1 +
>   drivers/hwmon/witrn.c         | 691 ++++++++++++++++++++++++++++++++++++++++++
>   9 files changed, 1243 insertions(+)
> ---
> base-commit: 0138af2472dfdef0d56fc4697416eaa0ff2589bd
> change-id: 20260327-b4-hwmon-witrn-a629b9040250
> 
> Thanks,
> Rong
> 

Re: [PATCH 0/4] hwmon: Add WITRN USB tester driver

[Rong Zhang]

Hi Guenter,

Thanks a lot for your review and applying patch 1 :-)

On Thu, 2026-03-26 at 17:05 -0700, Guenter Roeck wrote:
> On 3/26/26 12:19, Rong Zhang wrote:
> > WITRN produces a series of devices to monitor power characteristics of
> > USB connections and display those on a on-device display. Most of them
> > contain an additional port which exposes the measurements via USB HID.
> > 
> > These devices report sensor values in IEEE-754 float (binary32) format.
> > The driver must perform floating-point number to integer conversions to
> > provide hwmon channels. Meanwhile, they also report accumulative float
> > values, and simple division or multiplication turns them into useful
> > hwmon channels.
> > 
> > Patch 1 adds label support for 64-bit energy attributes, as the driver
> > needs it.
> > 
> > Patch 2 adds a helper module for floating-point to integer conversions,
> > so that the conversion, multification and division methods can be used
> > in this driver as well as other drivers (I am also working on another
> > USB tester driver that needs it).
> > 
> > Patch 3 adds a barebone HID driver for WITRN K2.
> > 
> > Patch 4 adds hwmon channels and attributes to the driver.
> > 
> > Signed-off-by: Rong Zhang <i@rong.moe>
> > ---
> > Rong Zhang (4):
> >        hwmon: Add label support for 64-bit energy attributes
> >        hwmon: New helper module for floating-point to integer conversions
> 
> Nack. This is not a hwmon problem and should reside in a driver or in lib/
> (if it is needed by multiple drivers).

Makes sense. I will try.

> 
> >        hwmon: Add barebone HID driver for WITRN
> 
> Nack. This is the wrong place for such a driver. It should reside somewhere
> in drivers/usb, or maybe in drivers/misc/.

Hmm, I decided to place it here because:

- It's not a hid_ll_driver but a dumb hid_driver, i.e., does no low
level things but just receives hid event from the HID core. It doesn't
even send any HID report to the device.

- There has been numerous hid_driver in drivers/hwmon/.

- There has been a similar USB tester driver in drivers/hwmon/, i.e.,
powerz. That's the major reason of my decision.

Could you kindly explain what kinds of driver can be accepted into
drivers/hwmon/?

> 
> >        hwmon: (witrn) Add monitoring support
> 
> This should be implemented as auxiliary driver.

Could you kindly elaborate? Did you mean that if the device supports
multiple functionalities they should be implemented as multiple
auxiliary drivers in different subsystems?

FYI, the USB tester doesn't provide any other meaningful feature that
fits other subsystems. The tester only provides two features through USB
HID: power measurements (this series), and raw PD packets sniffing.

As for the latter, the USB tester can sniff raw PD packets between the
source and sink if enabled in the device menu. It doesn't provide the
parsed result for packets, and the PC cannot ask the tester to send PD
packets. This doesn't fit UCSI at all, as a UCSI device operates at a
higher level and must accept commands. AFAIK such a dumb sniffer won't
fit any subsystem in the kernel. Hence, the only thing fits a subsystem
is its power measurements.

All measurements supported by the official utility for Windows can be
found in `struct witrn_sensor'. Other than that, all extra features
provided by the utility are implemented in software and I didn't see any
extra USB packets other than querying the serial number [1] when I was
randomly messing around with the utility [2].

I separated patch 3 and 4 just for easier review. If you are not in
favor of such a style, I will squash them.

[1]: In fact, the utility directly asks the device to return the content
on several specific ROM/RAM addresses, and then the utility calculates
the serial number with an unknown algorithm. Reading a ROM/RAM address
seems to be the only command that the device accepts from the USB host.

[2]: Yeah, their utility does not support PD packet capturing or
parsing. It seems that the manufacturer provides the PD sniffing feature
as is and expects users to capture it via hidraw or libusb and parse it
themselves.

> 
> Sashiko has a lot of feedback that you might want to address before
> resubmitting.
> 
> https://sashiko.dev/#/patchset/20260327-b4-hwmon-witrn-v1-0-8d2f1896c045%40rong.moe

Sashiko's feedback helps a lot. Thanks.

Thanks,
Rong

> 
> Thanks,
> Guenter
> 
> > 
> >   Documentation/hwmon/index.rst |   1 +
> >   Documentation/hwmon/witrn.rst |  53 ++++
> >   MAINTAINERS                   |   7 +
> >   drivers/hwmon/Kconfig         |  14 +
> >   drivers/hwmon/Makefile        |   2 +
> >   drivers/hwmon/hwmon-fp.c      | 262 ++++++++++++++++
> >   drivers/hwmon/hwmon-fp.h      | 212 +++++++++++++
> >   drivers/hwmon/hwmon.c         |   1 +
> >   drivers/hwmon/witrn.c         | 691 ++++++++++++++++++++++++++++++++++++++++++
> >   9 files changed, 1243 insertions(+)
> > ---
> > base-commit: 0138af2472dfdef0d56fc4697416eaa0ff2589bd
> > change-id: 20260327-b4-hwmon-witrn-a629b9040250
> > 
> > Thanks,
> > Rong
> > 

Re: [PATCH 0/4] hwmon: Add WITRN USB tester driver

[Guenter Roeck]

On 3/27/26 05:01, Rong Zhang wrote:
> Hi Guenter,
> 
> Thanks a lot for your review and applying patch 1 :-)
> 
> On Thu, 2026-03-26 at 17:05 -0700, Guenter Roeck wrote:
>> On 3/26/26 12:19, Rong Zhang wrote:
>>> WITRN produces a series of devices to monitor power characteristics of
>>> USB connections and display those on a on-device display. Most of them
>>> contain an additional port which exposes the measurements via USB HID.
>>>
>>> These devices report sensor values in IEEE-754 float (binary32) format.
>>> The driver must perform floating-point number to integer conversions to
>>> provide hwmon channels. Meanwhile, they also report accumulative float
>>> values, and simple division or multiplication turns them into useful
>>> hwmon channels.
>>>
>>> Patch 1 adds label support for 64-bit energy attributes, as the driver
>>> needs it.
>>>
>>> Patch 2 adds a helper module for floating-point to integer conversions,
>>> so that the conversion, multification and division methods can be used
>>> in this driver as well as other drivers (I am also working on another
>>> USB tester driver that needs it).
>>>
>>> Patch 3 adds a barebone HID driver for WITRN K2.
>>>
>>> Patch 4 adds hwmon channels and attributes to the driver.
>>>
>>> Signed-off-by: Rong Zhang <i@rong.moe>
>>> ---
>>> Rong Zhang (4):
>>>         hwmon: Add label support for 64-bit energy attributes
>>>         hwmon: New helper module for floating-point to integer conversions
>>
>> Nack. This is not a hwmon problem and should reside in a driver or in lib/
>> (if it is needed by multiple drivers).
> 
> Makes sense. I will try.
> 
>>
>>>         hwmon: Add barebone HID driver for WITRN
>>
>> Nack. This is the wrong place for such a driver. It should reside somewhere
>> in drivers/usb, or maybe in drivers/misc/.
> 
> Hmm, I decided to place it here because:
> 
> - It's not a hid_ll_driver but a dumb hid_driver, i.e., does no low
> level things but just receives hid event from the HID core. It doesn't
> even send any HID report to the device.
> 
> - There has been numerous hid_driver in drivers/hwmon/.
> 
> - There has been a similar USB tester driver in drivers/hwmon/, i.e.,
> powerz. That's the major reason of my decision.
> 

powerz is a pure hwmon driver, nothing else. It does not claim to be a
"pure hid driver". If your driver _only_ provides a hwmon ABI, it would
be acceptable. But then this and the next patch should be one patch,
and it should only provide the hwmon ABI, nothing else (except maybe
debugfs entries or sysfs entries attached directly to the HID device
to display information such as serial number etc). Reading ROM/RAM addresses,
as mentioned below, would be outside that scope.

The entire powerz driver is 269 lines of code. Your driver has well above
1,000 LOC. Your code has separate source files plus an include file.
That suggests that it does more than just reporting hardware monitoring
attributes.

I have not looked further into the code itself. My response is based purely
on the subjects and code organization, which suggests that this is a HID
driver with attached hardware monitoring.

I am not sure I understand what all that would have to do with UCSI. UCSI
support is implemented in drivers/usb/typec/ucsi. Anything associated
with that protocol should be implemented there if it is part of the
protocol.

> Could you kindly explain what kinds of driver can be accepted into
> drivers/hwmon/?
> 
>>
>>>         hwmon: (witrn) Add monitoring support
>>
>> This should be implemented as auxiliary driver.
> 
> Could you kindly elaborate? Did you mean that if the device supports
> multiple functionalities they should be implemented as multiple
> auxiliary drivers in different subsystems?
> 

Correct. Your series suggests that this would be the case.

> FYI, the USB tester doesn't provide any other meaningful feature that
> fits other subsystems. The tester only provides two features through USB
> HID: power measurements (this series), and raw PD packets sniffing.
> 

Again, support for raw PD packets sniffing would be outside the scope
of the hardware monitoring subsystem.

Thanks,
Guenter

> As for the latter, the USB tester can sniff raw PD packets between the
> source and sink if enabled in the device menu. It doesn't provide the
> parsed result for packets, and the PC cannot ask the tester to send PD
> packets. This doesn't fit UCSI at all, as a UCSI device operates at a
> higher level and must accept commands. AFAIK such a dumb sniffer won't
> fit any subsystem in the kernel. Hence, the only thing fits a subsystem
> is its power measurements.
> 
> All measurements supported by the official utility for Windows can be
> found in `struct witrn_sensor'. Other than that, all extra features
> provided by the utility are implemented in software and I didn't see any
> extra USB packets other than querying the serial number [1] when I was
> randomly messing around with the utility [2].
> 
> I separated patch 3 and 4 just for easier review. If you are not in
> favor of such a style, I will squash them.
> 
> [1]: In fact, the utility directly asks the device to return the content
> on several specific ROM/RAM addresses, and then the utility calculates
> the serial number with an unknown algorithm. Reading a ROM/RAM address
> seems to be the only command that the device accepts from the USB host.
> 
> [2]: Yeah, their utility does not support PD packet capturing or
> parsing. It seems that the manufacturer provides the PD sniffing feature
> as is and expects users to capture it via hidraw or libusb and parse it
> themselves.
> 
>>
>> Sashiko has a lot of feedback that you might want to address before
>> resubmitting.
>>
>> https://sashiko.dev/#/patchset/20260327-b4-hwmon-witrn-v1-0-8d2f1896c045%40rong.moe
> 
> Sashiko's feedback helps a lot. Thanks.
> 
> Thanks,
> Rong
> 
>>
>> Thanks,
>> Guenter
>>
>>>
>>>    Documentation/hwmon/index.rst |   1 +
>>>    Documentation/hwmon/witrn.rst |  53 ++++
>>>    MAINTAINERS                   |   7 +
>>>    drivers/hwmon/Kconfig         |  14 +
>>>    drivers/hwmon/Makefile        |   2 +
>>>    drivers/hwmon/hwmon-fp.c      | 262 ++++++++++++++++
>>>    drivers/hwmon/hwmon-fp.h      | 212 +++++++++++++
>>>    drivers/hwmon/hwmon.c         |   1 +
>>>    drivers/hwmon/witrn.c         | 691 ++++++++++++++++++++++++++++++++++++++++++
>>>    9 files changed, 1243 insertions(+)
>>> ---
>>> base-commit: 0138af2472dfdef0d56fc4697416eaa0ff2589bd
>>> change-id: 20260327-b4-hwmon-witrn-a629b9040250
>>>
>>> Thanks,
>>> Rong
>>>

Re: [PATCH 0/4] hwmon: Add WITRN USB tester driver

[Rong Zhang]

Hi Guenter,

Thanks for your detailed elaboration. It's very helpful.

On Fri, 2026-03-27 at 08:42 -0700, Guenter Roeck wrote:
> On 3/27/26 05:01, Rong Zhang wrote:
> > Hi Guenter,
> > 
> > Thanks a lot for your review and applying patch 1 :-)
> > 
> > On Thu, 2026-03-26 at 17:05 -0700, Guenter Roeck wrote:
> > > On 3/26/26 12:19, Rong Zhang wrote:
> > > > WITRN produces a series of devices to monitor power characteristics of
> > > > USB connections and display those on a on-device display. Most of them
> > > > contain an additional port which exposes the measurements via USB HID.
> > > > 
> > > > These devices report sensor values in IEEE-754 float (binary32) format.
> > > > The driver must perform floating-point number to integer conversions to
> > > > provide hwmon channels. Meanwhile, they also report accumulative float
> > > > values, and simple division or multiplication turns them into useful
> > > > hwmon channels.
> > > > 
> > > > Patch 1 adds label support for 64-bit energy attributes, as the driver
> > > > needs it.
> > > > 
> > > > Patch 2 adds a helper module for floating-point to integer conversions,
> > > > so that the conversion, multification and division methods can be used
> > > > in this driver as well as other drivers (I am also working on another
> > > > USB tester driver that needs it).
> > > > 
> > > > Patch 3 adds a barebone HID driver for WITRN K2.
> > > > 
> > > > Patch 4 adds hwmon channels and attributes to the driver.
> > > > 
> > > > Signed-off-by: Rong Zhang <i@rong.moe>
> > > > ---
> > > > Rong Zhang (4):
> > > >         hwmon: Add label support for 64-bit energy attributes
> > > >         hwmon: New helper module for floating-point to integer conversions
> > > 
> > > Nack. This is not a hwmon problem and should reside in a driver or in lib/
> > > (if it is needed by multiple drivers).
> > 
> > Makes sense. I will try.
> > 
> > > 
> > > >         hwmon: Add barebone HID driver for WITRN
> > > 
> > > Nack. This is the wrong place for such a driver. It should reside somewhere
> > > in drivers/usb, or maybe in drivers/misc/.
> > 
> > Hmm, I decided to place it here because:
> > 
> > - It's not a hid_ll_driver but a dumb hid_driver, i.e., does no low
> > level things but just receives hid event from the HID core. It doesn't
> > even send any HID report to the device.
> > 
> > - There has been numerous hid_driver in drivers/hwmon/.
> > 
> > - There has been a similar USB tester driver in drivers/hwmon/, i.e.,
> > powerz. That's the major reason of my decision.
> > 
> 
> powerz is a pure hwmon driver, nothing else. It does not claim to be a
> "pure hid driver". If your driver _only_ provides a hwmon ABI, it would
> be acceptable. But then this and the next patch should be one patch,
> and it should only provide the hwmon ABI, nothing else (except maybe
> debugfs entries or sysfs entries attached directly to the HID device
> to display information such as serial number etc). 
> 

Understood. I will squash the two patches into a single patch, as it
will only provide hwmon ABI.

> Reading ROM/RAM addresses,
> as mentioned below, would be outside that scope.

Thanks for clarification. I am not going to support this, as dancing
with undocumented ROM/RAM access is too dangerous.

> 
> The entire powerz driver is 269 lines of code. Your driver has well above
> 1,000 LOC. 
> 

The witrn driver itself is less than 700 LOC. It's longer because of
more channels and the need to call floating-point conversion and
arithmetic methods.

Other LOC mostly locates at the floating-point conversions and
arithmetic module. Since I will turn it into a generic lib, it won't be
a part of the witrn driver when I resubmit it. Thanks for the
suggestion.

> Your code has separate source files plus an include file.

The include file is for the floating-point conversion and arithmetic
module.

> That suggests that it does more than just reporting hardware monitoring
> attributes.
> 
> I have not looked further into the code itself. My response is based purely
> on the subjects and code organization, which suggests that this is a HID
> driver with attached hardware monitoring.

I should have explained the structure of the series more detailedly.
Sorry for causing the misunderstanding.

> 
> I am not sure I understand what all that would have to do with UCSI. UCSI
> support is implemented in drivers/usb/typec/ucsi. Anything associated
> with that protocol should be implemented there if it is part of the
> protocol.

Thanks for clarification. I think the feature has nothing to do with
UCSI because it's a passive dumb raw packet sniffer.

> 
> > Could you kindly explain what kinds of driver can be accepted into
> > drivers/hwmon/?
> > 
> > > 
> > > >         hwmon: (witrn) Add monitoring support
> > > 
> > > This should be implemented as auxiliary driver.
> > 
> > Could you kindly elaborate? Did you mean that if the device supports
> > multiple functionalities they should be implemented as multiple
> > auxiliary drivers in different subsystems?
> > 
> 
> Correct. Your series suggests that this would be the case.

Sorry for causing the misunderstanding again.

> 
> > FYI, the USB tester doesn't provide any other meaningful feature that
> > fits other subsystems. The tester only provides two features through USB
> > HID: power measurements (this series), and raw PD packets sniffing.
> > 
> 
> Again, support for raw PD packets sniffing would be outside the scope
> of the hardware monitoring subsystem.

Agreed. I am not going to support it in any other subsystem either.
Users should use hidraw and parse everything themselves if they need it.

Thanks,
Rong

> 
> Thanks,
> Guenter
> 
> > As for the latter, the USB tester can sniff raw PD packets between the
> > source and sink if enabled in the device menu. It doesn't provide the
> > parsed result for packets, and the PC cannot ask the tester to send PD
> > packets. This doesn't fit UCSI at all, as a UCSI device operates at a
> > higher level and must accept commands. AFAIK such a dumb sniffer won't
> > fit any subsystem in the kernel. Hence, the only thing fits a subsystem
> > is its power measurements.
> > 
> > All measurements supported by the official utility for Windows can be
> > found in `struct witrn_sensor'. Other than that, all extra features
> > provided by the utility are implemented in software and I didn't see any
> > extra USB packets other than querying the serial number [1] when I was
> > randomly messing around with the utility [2].
> > 
> > I separated patch 3 and 4 just for easier review. If you are not in
> > favor of such a style, I will squash them.
> > 
> > [1]: In fact, the utility directly asks the device to return the content
> > on several specific ROM/RAM addresses, and then the utility calculates
> > the serial number with an unknown algorithm. Reading a ROM/RAM address
> > seems to be the only command that the device accepts from the USB host.
> > 
> > [2]: Yeah, their utility does not support PD packet capturing or
> > parsing. It seems that the manufacturer provides the PD sniffing feature
> > as is and expects users to capture it via hidraw or libusb and parse it
> > themselves.
> > 
> > > 
> > > Sashiko has a lot of feedback that you might want to address before
> > > resubmitting.
> > > 
> > > https://sashiko.dev/#/patchset/20260327-b4-hwmon-witrn-v1-0-8d2f1896c045%40rong.moe
> > 
> > Sashiko's feedback helps a lot. Thanks.
> > 
> > Thanks,
> > Rong
> > 
> > > 
> > > Thanks,
> > > Guenter
> > > 
> > > > 
> > > >    Documentation/hwmon/index.rst |   1 +
> > > >    Documentation/hwmon/witrn.rst |  53 ++++
> > > >    MAINTAINERS                   |   7 +
> > > >    drivers/hwmon/Kconfig         |  14 +
> > > >    drivers/hwmon/Makefile        |   2 +
> > > >    drivers/hwmon/hwmon-fp.c      | 262 ++++++++++++++++
> > > >    drivers/hwmon/hwmon-fp.h      | 212 +++++++++++++
> > > >    drivers/hwmon/hwmon.c         |   1 +
> > > >    drivers/hwmon/witrn.c         | 691 ++++++++++++++++++++++++++++++++++++++++++
> > > >    9 files changed, 1243 insertions(+)
> > > > ---
> > > > base-commit: 0138af2472dfdef0d56fc4697416eaa0ff2589bd
> > > > change-id: 20260327-b4-hwmon-witrn-a629b9040250
> > > > 
> > > > Thanks,
> > > > Rong
> > > > 

Re: [PATCH 4/4] hwmon: (witrn) Add monitoring support

[kernel test robot]

Hi Rong,

kernel test robot noticed the following build errors:

[auto build test ERROR on 0138af2472dfdef0d56fc4697416eaa0ff2589bd]

url:    https://github.com/intel-lab-lkp/linux/commits/Rong-Zhang/hwmon-Add-label-support-for-64-bit-energy-attributes/20260329-030139
base:   0138af2472dfdef0d56fc4697416eaa0ff2589bd
patch link:    https://lore.kernel.org/r/20260327-b4-hwmon-witrn-v1-4-8d2f1896c045%40rong.moe
patch subject: [PATCH 4/4] hwmon: (witrn) Add monitoring support
config: nios2-allmodconfig (https://download.01.org/0day-ci/archive/20260329/202603291947.CoxzmTCo-lkp@intel.com/config)
compiler: nios2-linux-gcc (GCC) 11.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260329/202603291947.CoxzmTCo-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202603291947.CoxzmTCo-lkp@intel.com/

All errors (new ones prefixed by >>, old ones prefixed by <<):

ERROR: modpost: "__udivdi3" [drivers/hwmon/hwmon-fp.ko] undefined!
>> ERROR: modpost: "__divdi3" [drivers/hwmon/hwmon-fp.ko] undefined!
ERROR: modpost: "__divdi3" [drivers/hwmon/witrn.ko] undefined!

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 4/4] hwmon: (witrn) Add monitoring support

[kernel test robot]

Hi Rong,

kernel test robot noticed the following build errors:

[auto build test ERROR on 0138af2472dfdef0d56fc4697416eaa0ff2589bd]

url:    https://github.com/intel-lab-lkp/linux/commits/Rong-Zhang/hwmon-Add-label-support-for-64-bit-energy-attributes/20260329-030139
base:   0138af2472dfdef0d56fc4697416eaa0ff2589bd
patch link:    https://lore.kernel.org/r/20260327-b4-hwmon-witrn-v1-4-8d2f1896c045%40rong.moe
patch subject: [PATCH 4/4] hwmon: (witrn) Add monitoring support
config: csky-allmodconfig (https://download.01.org/0day-ci/archive/20260329/202603292058.Vszdl5uf-lkp@intel.com/config)
compiler: csky-linux-gcc (GCC) 15.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260329/202603292058.Vszdl5uf-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202603292058.Vszdl5uf-lkp@intel.com/

All errors (new ones prefixed by >>, old ones prefixed by <<):

>> ERROR: modpost: "__udivdi3" [drivers/hwmon/hwmon-fp.ko] undefined!
>> ERROR: modpost: "__divdi3" [drivers/hwmon/witrn.ko] undefined!

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki