* [PATCH] hwmon: (aquacomputer_d5next) Add support for Aquacomputer Farbwerk
@ 2022-04-21 7:27 Aleksa Savic
2022-04-22 7:23 ` Aleksa Savic
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Aleksa Savic @ 2022-04-21 7:27 UTC (permalink / raw)
To: linux-hwmon
Cc: savicaleksa83, Jack Doan, Jean Delvare, Guenter Roeck,
Jonathan Corbet, linux-doc, linux-kernel
Extend aquacomputer_d5next driver to expose hardware temperature sensors
of the Aquacomputer Farbwerk RGB controller, which communicates through
a proprietary USB HID protocol.
Four temperature sensors are available. Additionally, serial number and
firmware version are exposed through debugfs.
Also, add Jack Doan to MAINTAINERS for this driver.
Signed-off-by: Jack Doan <me@jackdoan.com>
Signed-off-by: Aleksa Savic <savicaleksa83@gmail.com>
---
If adding to MAINTAINERS requires a separate commit, I'll send it
separately.
---
Documentation/hwmon/aquacomputer_d5next.rst | 3 +-
MAINTAINERS | 1 +
drivers/hwmon/Kconfig | 5 +--
drivers/hwmon/aquacomputer_d5next.c | 37 ++++++++++++++++++---
4 files changed, 38 insertions(+), 8 deletions(-)
diff --git a/Documentation/hwmon/aquacomputer_d5next.rst b/Documentation/hwmon/aquacomputer_d5next.rst
index e69f718caf5b..717e28226cde 100644
--- a/Documentation/hwmon/aquacomputer_d5next.rst
+++ b/Documentation/hwmon/aquacomputer_d5next.rst
@@ -6,6 +6,7 @@ Kernel driver aquacomputer-d5next
Supported devices:
* Aquacomputer D5 Next watercooling pump
+* Aquacomputer Farbwerk RGB controller
* Aquacomputer Farbwerk 360 RGB controller
* Aquacomputer Octo fan controller
@@ -32,7 +33,7 @@ better suited for userspace tools.
The Octo exposes four temperature sensors and eight PWM controllable fans, along
with their speed (in RPM), power, voltage and current.
-The Farbwerk 360 exposes four temperature sensors. Depending on the device,
+The Farbwerk and Farbwerk 360 expose four temperature sensors. Depending on the device,
not all sysfs and debugfs entries will be available.
Usage notes
diff --git a/MAINTAINERS b/MAINTAINERS
index ea2cd656ee6c..d8e3ca0fbc3a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1389,6 +1389,7 @@ F: drivers/media/i2c/aptina-pll.*
AQUACOMPUTER D5 NEXT PUMP SENSOR DRIVER
M: Aleksa Savic <savicaleksa83@gmail.com>
+M: Jack Doan <me@jackdoan.com>
L: linux-hwmon@vger.kernel.org
S: Maintained
F: Documentation/hwmon/aquacomputer_d5next.rst
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 5beadd8a0932..01d10c9b633a 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -256,12 +256,13 @@ config SENSORS_AHT10
will be called aht10.
config SENSORS_AQUACOMPUTER_D5NEXT
- tristate "Aquacomputer D5 Next, Octo and Farbwerk 360"
+ tristate "Aquacomputer D5 Next, Octo, Farbwerk, Farbwerk 360"
depends on USB_HID
help
If you say yes here you get support for sensors and fans of
the Aquacomputer D5 Next watercooling pump, Octo fan
- controller and Farbwerk 360 RGB controller, where available.
+ controller, Farbwerk and Farbwerk 360 RGB controllers, 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 a464473bc981..7d2e7279abfb 100644
--- a/drivers/hwmon/aquacomputer_d5next.c
+++ b/drivers/hwmon/aquacomputer_d5next.c
@@ -1,11 +1,12 @@
// SPDX-License-Identifier: GPL-2.0+
/*
- * hwmon driver for Aquacomputer devices (D5 Next, Farbwerk 360, Octo)
+ * hwmon driver for Aquacomputer devices (D5 Next, Farbwerk, Farbwerk 360, Octo)
*
* Aquacomputer devices send HID reports (with ID 0x01) every second to report
* sensor values.
*
* Copyright 2021 Aleksa Savic <savicaleksa83@gmail.com>
+ * Copyright 2022 Jack Doan <me@jackdoan.com>
*/
#include <linux/crc16.h>
@@ -19,14 +20,16 @@
#include <asm/unaligned.h>
#define USB_VENDOR_ID_AQUACOMPUTER 0x0c70
+#define USB_PRODUCT_ID_FARBWERK 0xf00a
#define USB_PRODUCT_ID_D5NEXT 0xf00e
#define USB_PRODUCT_ID_FARBWERK360 0xf010
#define USB_PRODUCT_ID_OCTO 0xf011
-enum kinds { d5next, farbwerk360, octo };
+enum kinds { d5next, farbwerk, farbwerk360, octo };
static const char *const aqc_device_names[] = {
[d5next] = "d5next",
+ [farbwerk] = "farbwerk",
[farbwerk360] = "farbwerk360",
[octo] = "octo"
};
@@ -69,6 +72,12 @@ static u8 secondary_ctrl_report[] = {
#define D5NEXT_PUMP_CURRENT 112
#define D5NEXT_FAN_CURRENT 99
+/* Register offsets for the Farbwerk RGB controller */
+#define FARBWERK_NUM_SENSORS 4
+#define FARBWERK_SENSOR_START 0x2f
+#define FARBWERK_SENSOR_SIZE 0x02
+#define FARBWERK_SENSOR_DISCONNECTED 0x7FFF
+
/* Register offsets for the Farbwerk 360 RGB controller */
#define FARBWERK360_NUM_SENSORS 4
#define FARBWERK360_SENSOR_START 0x32
@@ -125,7 +134,7 @@ static const char *const label_d5next_current[] = {
"Fan current"
};
-/* Labels for Farbwerk 360 and Octo temperature sensors */
+/* Labels for Farbwerk, Farbwerk 360 and Octo temperature sensors */
static const char *const label_temp_sensors[] = {
"Sensor 1",
"Sensor 2",
@@ -319,6 +328,7 @@ static umode_t aqc_is_visible(const void *data, enum hwmon_sensor_types type, u3
if (channel == 0)
return 0444;
break;
+ case farbwerk:
case farbwerk360:
case octo:
return 0444;
@@ -551,8 +561,7 @@ static const struct hwmon_chip_info aqc_chip_info = {
.info = aqc_info,
};
-static int aqc_raw_event(struct hid_device *hdev, struct hid_report *report, u8 *data,
- int size)
+static int aqc_raw_event(struct hid_device *hdev, struct hid_report *report, u8 *data, int size)
{
int i, sensor_value;
struct aqc_data *priv;
@@ -587,6 +596,17 @@ static int aqc_raw_event(struct hid_device *hdev, struct hid_report *report, u8
priv->current_input[0] = get_unaligned_be16(data + D5NEXT_PUMP_CURRENT);
priv->current_input[1] = get_unaligned_be16(data + D5NEXT_FAN_CURRENT);
break;
+ case farbwerk:
+ /* Temperature sensor readings */
+ for (i = 0; i < FARBWERK_NUM_SENSORS; i++) {
+ sensor_value = get_unaligned_be16(data + FARBWERK_SENSOR_START +
+ i * FARBWERK_SENSOR_SIZE);
+ if (sensor_value == FARBWERK_SENSOR_DISCONNECTED)
+ priv->temp_input[i] = -ENODATA;
+ else
+ priv->temp_input[i] = sensor_value * 10;
+ }
+ break;
case farbwerk360:
/* Temperature sensor readings */
for (i = 0; i < FARBWERK360_NUM_SENSORS; i++) {
@@ -733,6 +753,11 @@ static int aqc_probe(struct hid_device *hdev, const struct hid_device_id *id)
priv->voltage_label = label_d5next_voltages;
priv->current_label = label_d5next_current;
break;
+ case USB_PRODUCT_ID_FARBWERK:
+ priv->kind = farbwerk;
+
+ priv->temp_label = label_temp_sensors;
+ break;
case USB_PRODUCT_ID_FARBWERK360:
priv->kind = farbwerk360;
@@ -795,6 +820,7 @@ static void aqc_remove(struct hid_device *hdev)
static const struct hid_device_id aqc_table[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_AQUACOMPUTER, USB_PRODUCT_ID_D5NEXT) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_AQUACOMPUTER, USB_PRODUCT_ID_FARBWERK) },
{ HID_USB_DEVICE(USB_VENDOR_ID_AQUACOMPUTER, USB_PRODUCT_ID_FARBWERK360) },
{ HID_USB_DEVICE(USB_VENDOR_ID_AQUACOMPUTER, USB_PRODUCT_ID_OCTO) },
{ }
@@ -826,4 +852,5 @@ module_exit(aqc_exit);
MODULE_LICENSE("GPL");
MODULE_AUTHOR("Aleksa Savic <savicaleksa83@gmail.com>");
+MODULE_AUTHOR("Jack Doan <me@jackdoan.com>");
MODULE_DESCRIPTION("Hwmon driver for Aquacomputer devices");
--
2.35.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] hwmon: (aquacomputer_d5next) Add support for Aquacomputer Farbwerk
2022-04-21 7:27 [PATCH] hwmon: (aquacomputer_d5next) Add support for Aquacomputer Farbwerk Aleksa Savic
@ 2022-04-22 7:23 ` Aleksa Savic
2022-04-22 18:31 ` Guenter Roeck
2022-04-24 1:42 ` Guenter Roeck
2 siblings, 0 replies; 9+ messages in thread
From: Aleksa Savic @ 2022-04-22 7:23 UTC (permalink / raw)
To: linux-hwmon
Cc: Jack Doan, Jean Delvare, Guenter Roeck, Jonathan Corbet,
linux-doc, linux-kernel
Hi,
Just noticed that this patch is missing the "select CRC16" addition
present in hwmon-next. I apologize for that, will fix in v2.
Thanks,
Aleksa
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] hwmon: (aquacomputer_d5next) Add support for Aquacomputer Farbwerk
2022-04-21 7:27 [PATCH] hwmon: (aquacomputer_d5next) Add support for Aquacomputer Farbwerk Aleksa Savic
2022-04-22 7:23 ` Aleksa Savic
@ 2022-04-22 18:31 ` Guenter Roeck
2022-04-23 16:23 ` Aleksa Savic
2022-04-24 1:42 ` Guenter Roeck
2 siblings, 1 reply; 9+ messages in thread
From: Guenter Roeck @ 2022-04-22 18:31 UTC (permalink / raw)
To: Aleksa Savic
Cc: linux-hwmon, Jack Doan, Jean Delvare, Jonathan Corbet, linux-doc,
linux-kernel
On Thu, Apr 21, 2022 at 09:27:42AM +0200, Aleksa Savic wrote:
> Extend aquacomputer_d5next driver to expose hardware temperature sensors
> of the Aquacomputer Farbwerk RGB controller, which communicates through
> a proprietary USB HID protocol.
>
> Four temperature sensors are available. Additionally, serial number and
> firmware version are exposed through debugfs.
>
> Also, add Jack Doan to MAINTAINERS for this driver.
>
> Signed-off-by: Jack Doan <me@jackdoan.com>
> Signed-off-by: Aleksa Savic <savicaleksa83@gmail.com>
> ---
> If adding to MAINTAINERS requires a separate commit, I'll send it
> separately.
> ---
> Documentation/hwmon/aquacomputer_d5next.rst | 3 +-
> MAINTAINERS | 1 +
> drivers/hwmon/Kconfig | 5 +--
> drivers/hwmon/aquacomputer_d5next.c | 37 ++++++++++++++++++---
> 4 files changed, 38 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/hwmon/aquacomputer_d5next.rst b/Documentation/hwmon/aquacomputer_d5next.rst
> index e69f718caf5b..717e28226cde 100644
> --- a/Documentation/hwmon/aquacomputer_d5next.rst
> +++ b/Documentation/hwmon/aquacomputer_d5next.rst
> @@ -6,6 +6,7 @@ Kernel driver aquacomputer-d5next
> Supported devices:
>
> * Aquacomputer D5 Next watercooling pump
> +* Aquacomputer Farbwerk RGB controller
> * Aquacomputer Farbwerk 360 RGB controller
> * Aquacomputer Octo fan controller
>
> @@ -32,7 +33,7 @@ better suited for userspace tools.
> The Octo exposes four temperature sensors and eight PWM controllable fans, along
> with their speed (in RPM), power, voltage and current.
>
> -The Farbwerk 360 exposes four temperature sensors. Depending on the device,
> +The Farbwerk and Farbwerk 360 expose four temperature sensors. Depending on the device,
> not all sysfs and debugfs entries will be available.
>
> Usage notes
> diff --git a/MAINTAINERS b/MAINTAINERS
> index ea2cd656ee6c..d8e3ca0fbc3a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1389,6 +1389,7 @@ F: drivers/media/i2c/aptina-pll.*
>
> AQUACOMPUTER D5 NEXT PUMP SENSOR DRIVER
> M: Aleksa Savic <savicaleksa83@gmail.com>
> +M: Jack Doan <me@jackdoan.com>
> L: linux-hwmon@vger.kernel.org
> S: Maintained
> F: Documentation/hwmon/aquacomputer_d5next.rst
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 5beadd8a0932..01d10c9b633a 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -256,12 +256,13 @@ config SENSORS_AHT10
> will be called aht10.
>
> config SENSORS_AQUACOMPUTER_D5NEXT
> - tristate "Aquacomputer D5 Next, Octo and Farbwerk 360"
> + tristate "Aquacomputer D5 Next, Octo, Farbwerk, Farbwerk 360"
> depends on USB_HID
> help
> If you say yes here you get support for sensors and fans of
> the Aquacomputer D5 Next watercooling pump, Octo fan
> - controller and Farbwerk 360 RGB controller, where available.
> + controller, Farbwerk and Farbwerk 360 RGB controllers, 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 a464473bc981..7d2e7279abfb 100644
> --- a/drivers/hwmon/aquacomputer_d5next.c
> +++ b/drivers/hwmon/aquacomputer_d5next.c
> @@ -1,11 +1,12 @@
> // SPDX-License-Identifier: GPL-2.0+
> /*
> - * hwmon driver for Aquacomputer devices (D5 Next, Farbwerk 360, Octo)
> + * hwmon driver for Aquacomputer devices (D5 Next, Farbwerk, Farbwerk 360, Octo)
> *
> * Aquacomputer devices send HID reports (with ID 0x01) every second to report
> * sensor values.
> *
> * Copyright 2021 Aleksa Savic <savicaleksa83@gmail.com>
> + * Copyright 2022 Jack Doan <me@jackdoan.com>
> */
>
> #include <linux/crc16.h>
> @@ -19,14 +20,16 @@
> #include <asm/unaligned.h>
>
> #define USB_VENDOR_ID_AQUACOMPUTER 0x0c70
> +#define USB_PRODUCT_ID_FARBWERK 0xf00a
> #define USB_PRODUCT_ID_D5NEXT 0xf00e
> #define USB_PRODUCT_ID_FARBWERK360 0xf010
> #define USB_PRODUCT_ID_OCTO 0xf011
>
> -enum kinds { d5next, farbwerk360, octo };
> +enum kinds { d5next, farbwerk, farbwerk360, octo };
>
> static const char *const aqc_device_names[] = {
> [d5next] = "d5next",
> + [farbwerk] = "farbwerk",
> [farbwerk360] = "farbwerk360",
> [octo] = "octo"
> };
> @@ -69,6 +72,12 @@ static u8 secondary_ctrl_report[] = {
> #define D5NEXT_PUMP_CURRENT 112
> #define D5NEXT_FAN_CURRENT 99
>
> +/* Register offsets for the Farbwerk RGB controller */
> +#define FARBWERK_NUM_SENSORS 4
> +#define FARBWERK_SENSOR_START 0x2f
> +#define FARBWERK_SENSOR_SIZE 0x02
> +#define FARBWERK_SENSOR_DISCONNECTED 0x7FFF
> +
> /* Register offsets for the Farbwerk 360 RGB controller */
> #define FARBWERK360_NUM_SENSORS 4
> #define FARBWERK360_SENSOR_START 0x32
> @@ -125,7 +134,7 @@ static const char *const label_d5next_current[] = {
> "Fan current"
> };
>
> -/* Labels for Farbwerk 360 and Octo temperature sensors */
> +/* Labels for Farbwerk, Farbwerk 360 and Octo temperature sensors */
> static const char *const label_temp_sensors[] = {
> "Sensor 1",
> "Sensor 2",
> @@ -319,6 +328,7 @@ static umode_t aqc_is_visible(const void *data, enum hwmon_sensor_types type, u3
> if (channel == 0)
> return 0444;
> break;
> + case farbwerk:
> case farbwerk360:
> case octo:
> return 0444;
> @@ -551,8 +561,7 @@ static const struct hwmon_chip_info aqc_chip_info = {
> .info = aqc_info,
> };
>
> -static int aqc_raw_event(struct hid_device *hdev, struct hid_report *report, u8 *data,
> - int size)
> +static int aqc_raw_event(struct hid_device *hdev, struct hid_report *report, u8 *data, int size)
> {
> int i, sensor_value;
> struct aqc_data *priv;
> @@ -587,6 +596,17 @@ static int aqc_raw_event(struct hid_device *hdev, struct hid_report *report, u8
> priv->current_input[0] = get_unaligned_be16(data + D5NEXT_PUMP_CURRENT);
> priv->current_input[1] = get_unaligned_be16(data + D5NEXT_FAN_CURRENT);
> break;
> + case farbwerk:
> + /* Temperature sensor readings */
> + for (i = 0; i < FARBWERK_NUM_SENSORS; i++) {
> + sensor_value = get_unaligned_be16(data + FARBWERK_SENSOR_START +
> + i * FARBWERK_SENSOR_SIZE);
> + if (sensor_value == FARBWERK_SENSOR_DISCONNECTED)
> + priv->temp_input[i] = -ENODATA;
> + else
> + priv->temp_input[i] = sensor_value * 10;
> + }
> + break;
> case farbwerk360:
> /* Temperature sensor readings */
> for (i = 0; i < FARBWERK360_NUM_SENSORS; i++) {
> @@ -733,6 +753,11 @@ static int aqc_probe(struct hid_device *hdev, const struct hid_device_id *id)
> priv->voltage_label = label_d5next_voltages;
> priv->current_label = label_d5next_current;
> break;
> + case USB_PRODUCT_ID_FARBWERK:
> + priv->kind = farbwerk;
> +
Any special reason for this empty line ? It seems kind of random.
Guenter
> + priv->temp_label = label_temp_sensors;
> + break;
> case USB_PRODUCT_ID_FARBWERK360:
> priv->kind = farbwerk360;
>
> @@ -795,6 +820,7 @@ static void aqc_remove(struct hid_device *hdev)
>
> static const struct hid_device_id aqc_table[] = {
> { HID_USB_DEVICE(USB_VENDOR_ID_AQUACOMPUTER, USB_PRODUCT_ID_D5NEXT) },
> + { HID_USB_DEVICE(USB_VENDOR_ID_AQUACOMPUTER, USB_PRODUCT_ID_FARBWERK) },
> { HID_USB_DEVICE(USB_VENDOR_ID_AQUACOMPUTER, USB_PRODUCT_ID_FARBWERK360) },
> { HID_USB_DEVICE(USB_VENDOR_ID_AQUACOMPUTER, USB_PRODUCT_ID_OCTO) },
> { }
> @@ -826,4 +852,5 @@ module_exit(aqc_exit);
>
> MODULE_LICENSE("GPL");
> MODULE_AUTHOR("Aleksa Savic <savicaleksa83@gmail.com>");
> +MODULE_AUTHOR("Jack Doan <me@jackdoan.com>");
> MODULE_DESCRIPTION("Hwmon driver for Aquacomputer devices");
> --
> 2.35.1
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] hwmon: (aquacomputer_d5next) Add support for Aquacomputer Farbwerk
2022-04-22 18:31 ` Guenter Roeck
@ 2022-04-23 16:23 ` Aleksa Savic
2022-04-23 16:34 ` Guenter Roeck
0 siblings, 1 reply; 9+ messages in thread
From: Aleksa Savic @ 2022-04-23 16:23 UTC (permalink / raw)
To: Guenter Roeck
Cc: linux-hwmon, Jack Doan, Jean Delvare, Jonathan Corbet, linux-doc,
linux-kernel
On Fri, Apr 22, 2022 at 11:31:44AM -0700, Guenter Roeck wrote:
> On Thu, Apr 21, 2022 at 09:27:42AM +0200, Aleksa Savic wrote:
> > Extend aquacomputer_d5next driver to expose hardware temperature sensors
> > of the Aquacomputer Farbwerk RGB controller, which communicates through
> > a proprietary USB HID protocol.
> >
> > Four temperature sensors are available. Additionally, serial number and
> > firmware version are exposed through debugfs.
> >
> > Also, add Jack Doan to MAINTAINERS for this driver.
> >
> > Signed-off-by: Jack Doan <me@jackdoan.com>
> > Signed-off-by: Aleksa Savic <savicaleksa83@gmail.com>
> > ---
> > If adding to MAINTAINERS requires a separate commit, I'll send it
> > separately.
> > ---
> > Documentation/hwmon/aquacomputer_d5next.rst | 3 +-
> > MAINTAINERS | 1 +
> > drivers/hwmon/Kconfig | 5 +--
> > drivers/hwmon/aquacomputer_d5next.c | 37 ++++++++++++++++++---
> > 4 files changed, 38 insertions(+), 8 deletions(-)
> >
> > diff --git a/Documentation/hwmon/aquacomputer_d5next.rst b/Documentation/hwmon/aquacomputer_d5next.rst
> > index e69f718caf5b..717e28226cde 100644
> > --- a/Documentation/hwmon/aquacomputer_d5next.rst
> > +++ b/Documentation/hwmon/aquacomputer_d5next.rst
> > @@ -6,6 +6,7 @@ Kernel driver aquacomputer-d5next
> > Supported devices:
> >
> > * Aquacomputer D5 Next watercooling pump
> > +* Aquacomputer Farbwerk RGB controller
> > * Aquacomputer Farbwerk 360 RGB controller
> > * Aquacomputer Octo fan controller
> >
> > @@ -32,7 +33,7 @@ better suited for userspace tools.
> > The Octo exposes four temperature sensors and eight PWM controllable fans, along
> > with their speed (in RPM), power, voltage and current.
> >
> > -The Farbwerk 360 exposes four temperature sensors. Depending on the device,
> > +The Farbwerk and Farbwerk 360 expose four temperature sensors. Depending on the device,
> > not all sysfs and debugfs entries will be available.
> >
> > Usage notes
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index ea2cd656ee6c..d8e3ca0fbc3a 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -1389,6 +1389,7 @@ F: drivers/media/i2c/aptina-pll.*
> >
> > AQUACOMPUTER D5 NEXT PUMP SENSOR DRIVER
> > M: Aleksa Savic <savicaleksa83@gmail.com>
> > +M: Jack Doan <me@jackdoan.com>
> > L: linux-hwmon@vger.kernel.org
> > S: Maintained
> > F: Documentation/hwmon/aquacomputer_d5next.rst
> > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> > index 5beadd8a0932..01d10c9b633a 100644
> > --- a/drivers/hwmon/Kconfig
> > +++ b/drivers/hwmon/Kconfig
> > @@ -256,12 +256,13 @@ config SENSORS_AHT10
> > will be called aht10.
> >
> > config SENSORS_AQUACOMPUTER_D5NEXT
> > - tristate "Aquacomputer D5 Next, Octo and Farbwerk 360"
> > + tristate "Aquacomputer D5 Next, Octo, Farbwerk, Farbwerk 360"
> > depends on USB_HID
> > help
> > If you say yes here you get support for sensors and fans of
> > the Aquacomputer D5 Next watercooling pump, Octo fan
> > - controller and Farbwerk 360 RGB controller, where available.
> > + controller, Farbwerk and Farbwerk 360 RGB controllers, 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 a464473bc981..7d2e7279abfb 100644
> > --- a/drivers/hwmon/aquacomputer_d5next.c
> > +++ b/drivers/hwmon/aquacomputer_d5next.c
> > @@ -1,11 +1,12 @@
> > // SPDX-License-Identifier: GPL-2.0+
> > /*
> > - * hwmon driver for Aquacomputer devices (D5 Next, Farbwerk 360, Octo)
> > + * hwmon driver for Aquacomputer devices (D5 Next, Farbwerk, Farbwerk 360, Octo)
> > *
> > * Aquacomputer devices send HID reports (with ID 0x01) every second to report
> > * sensor values.
> > *
> > * Copyright 2021 Aleksa Savic <savicaleksa83@gmail.com>
> > + * Copyright 2022 Jack Doan <me@jackdoan.com>
> > */
> >
> > #include <linux/crc16.h>
> > @@ -19,14 +20,16 @@
> > #include <asm/unaligned.h>
> >
> > #define USB_VENDOR_ID_AQUACOMPUTER 0x0c70
> > +#define USB_PRODUCT_ID_FARBWERK 0xf00a
> > #define USB_PRODUCT_ID_D5NEXT 0xf00e
> > #define USB_PRODUCT_ID_FARBWERK360 0xf010
> > #define USB_PRODUCT_ID_OCTO 0xf011
> >
> > -enum kinds { d5next, farbwerk360, octo };
> > +enum kinds { d5next, farbwerk, farbwerk360, octo };
> >
> > static const char *const aqc_device_names[] = {
> > [d5next] = "d5next",
> > + [farbwerk] = "farbwerk",
> > [farbwerk360] = "farbwerk360",
> > [octo] = "octo"
> > };
> > @@ -69,6 +72,12 @@ static u8 secondary_ctrl_report[] = {
> > #define D5NEXT_PUMP_CURRENT 112
> > #define D5NEXT_FAN_CURRENT 99
> >
> > +/* Register offsets for the Farbwerk RGB controller */
> > +#define FARBWERK_NUM_SENSORS 4
> > +#define FARBWERK_SENSOR_START 0x2f
> > +#define FARBWERK_SENSOR_SIZE 0x02
> > +#define FARBWERK_SENSOR_DISCONNECTED 0x7FFF
> > +
> > /* Register offsets for the Farbwerk 360 RGB controller */
> > #define FARBWERK360_NUM_SENSORS 4
> > #define FARBWERK360_SENSOR_START 0x32
> > @@ -125,7 +134,7 @@ static const char *const label_d5next_current[] = {
> > "Fan current"
> > };
> >
> > -/* Labels for Farbwerk 360 and Octo temperature sensors */
> > +/* Labels for Farbwerk, Farbwerk 360 and Octo temperature sensors */
> > static const char *const label_temp_sensors[] = {
> > "Sensor 1",
> > "Sensor 2",
> > @@ -319,6 +328,7 @@ static umode_t aqc_is_visible(const void *data, enum hwmon_sensor_types type, u3
> > if (channel == 0)
> > return 0444;
> > break;
> > + case farbwerk:
> > case farbwerk360:
> > case octo:
> > return 0444;
> > @@ -551,8 +561,7 @@ static const struct hwmon_chip_info aqc_chip_info = {
> > .info = aqc_info,
> > };
> >
> > -static int aqc_raw_event(struct hid_device *hdev, struct hid_report *report, u8 *data,
> > - int size)
> > +static int aqc_raw_event(struct hid_device *hdev, struct hid_report *report, u8 *data, int size)
> > {
> > int i, sensor_value;
> > struct aqc_data *priv;
> > @@ -587,6 +596,17 @@ static int aqc_raw_event(struct hid_device *hdev, struct hid_report *report, u8
> > priv->current_input[0] = get_unaligned_be16(data + D5NEXT_PUMP_CURRENT);
> > priv->current_input[1] = get_unaligned_be16(data + D5NEXT_FAN_CURRENT);
> > break;
> > + case farbwerk:
> > + /* Temperature sensor readings */
> > + for (i = 0; i < FARBWERK_NUM_SENSORS; i++) {
> > + sensor_value = get_unaligned_be16(data + FARBWERK_SENSOR_START +
> > + i * FARBWERK_SENSOR_SIZE);
> > + if (sensor_value == FARBWERK_SENSOR_DISCONNECTED)
> > + priv->temp_input[i] = -ENODATA;
> > + else
> > + priv->temp_input[i] = sensor_value * 10;
> > + }
> > + break;
> > case farbwerk360:
> > /* Temperature sensor readings */
> > for (i = 0; i < FARBWERK360_NUM_SENSORS; i++) {
> > @@ -733,6 +753,11 @@ static int aqc_probe(struct hid_device *hdev, const struct hid_device_id *id)
> > priv->voltage_label = label_d5next_voltages;
> > priv->current_label = label_d5next_current;
> > break;
> > + case USB_PRODUCT_ID_FARBWERK:
> > + priv->kind = farbwerk;
> > +
>
>
> Any special reason for this empty line ? It seems kind of random.
>
> Guenter
>
That's how code in other cases of that switch is formatted (not visible
from this patch directly), setting priv->kind and then the appropriate
labels.
Thanks,
Aleksa
> > + priv->temp_label = label_temp_sensors;
> > + break;
> > case USB_PRODUCT_ID_FARBWERK360:
> > priv->kind = farbwerk360;
> >
> > @@ -795,6 +820,7 @@ static void aqc_remove(struct hid_device *hdev)
> >
> > static const struct hid_device_id aqc_table[] = {
> > { HID_USB_DEVICE(USB_VENDOR_ID_AQUACOMPUTER, USB_PRODUCT_ID_D5NEXT) },
> > + { HID_USB_DEVICE(USB_VENDOR_ID_AQUACOMPUTER, USB_PRODUCT_ID_FARBWERK) },
> > { HID_USB_DEVICE(USB_VENDOR_ID_AQUACOMPUTER, USB_PRODUCT_ID_FARBWERK360) },
> > { HID_USB_DEVICE(USB_VENDOR_ID_AQUACOMPUTER, USB_PRODUCT_ID_OCTO) },
> > { }
> > @@ -826,4 +852,5 @@ module_exit(aqc_exit);
> >
> > MODULE_LICENSE("GPL");
> > MODULE_AUTHOR("Aleksa Savic <savicaleksa83@gmail.com>");
> > +MODULE_AUTHOR("Jack Doan <me@jackdoan.com>");
> > MODULE_DESCRIPTION("Hwmon driver for Aquacomputer devices");
> > --
> > 2.35.1
> >
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] hwmon: (aquacomputer_d5next) Add support for Aquacomputer Farbwerk
2022-04-23 16:23 ` Aleksa Savic
@ 2022-04-23 16:34 ` Guenter Roeck
0 siblings, 0 replies; 9+ messages in thread
From: Guenter Roeck @ 2022-04-23 16:34 UTC (permalink / raw)
To: Aleksa Savic
Cc: linux-hwmon, Jack Doan, Jean Delvare, Jonathan Corbet, linux-doc,
linux-kernel
On Sat, Apr 23, 2022 at 06:23:46PM +0200, Aleksa Savic wrote:
[ ... ]
> > > + case USB_PRODUCT_ID_FARBWERK:
> > > + priv->kind = farbwerk;
> > > +
> >
> >
> > Any special reason for this empty line ? It seems kind of random.
> >
> > Guenter
> >
>
> That's how code in other cases of that switch is formatted (not visible
> from this patch directly), setting priv->kind and then the appropriate
> labels.
>
Ah, ok. Looks odd here, but makes sense.
Thanks,
Guenter
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] hwmon: (aquacomputer_d5next) Add support for Aquacomputer Farbwerk
2022-04-21 7:27 [PATCH] hwmon: (aquacomputer_d5next) Add support for Aquacomputer Farbwerk Aleksa Savic
2022-04-22 7:23 ` Aleksa Savic
2022-04-22 18:31 ` Guenter Roeck
@ 2022-04-24 1:42 ` Guenter Roeck
2022-04-24 3:56 ` Jack Doan
[not found] ` <0bf6576c-c129-f8fb-19c3-e3fca797cfee@jackdoan.com>
2 siblings, 2 replies; 9+ messages in thread
From: Guenter Roeck @ 2022-04-24 1:42 UTC (permalink / raw)
To: Aleksa Savic
Cc: linux-hwmon, Jack Doan, Jean Delvare, Jonathan Corbet, linux-doc,
linux-kernel
On Thu, Apr 21, 2022 at 09:27:42AM +0200, Aleksa Savic wrote:
> Extend aquacomputer_d5next driver to expose hardware temperature sensors
> of the Aquacomputer Farbwerk RGB controller, which communicates through
> a proprietary USB HID protocol.
>
> Four temperature sensors are available. Additionally, serial number and
> firmware version are exposed through debugfs.
>
> Also, add Jack Doan to MAINTAINERS for this driver.
>
> Signed-off-by: Jack Doan <me@jackdoan.com>
> Signed-off-by: Aleksa Savic <savicaleksa83@gmail.com>
> ---
This patch doesn't apply. Please rebase to master and resend.
More comments inline.
> If adding to MAINTAINERS requires a separate commit, I'll send it
> separately.
> ---
> Documentation/hwmon/aquacomputer_d5next.rst | 3 +-
> MAINTAINERS | 1 +
> drivers/hwmon/Kconfig | 5 +--
> drivers/hwmon/aquacomputer_d5next.c | 37 ++++++++++++++++++---
> 4 files changed, 38 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/hwmon/aquacomputer_d5next.rst b/Documentation/hwmon/aquacomputer_d5next.rst
> index e69f718caf5b..717e28226cde 100644
> --- a/Documentation/hwmon/aquacomputer_d5next.rst
> +++ b/Documentation/hwmon/aquacomputer_d5next.rst
> @@ -6,6 +6,7 @@ Kernel driver aquacomputer-d5next
> Supported devices:
>
> * Aquacomputer D5 Next watercooling pump
> +* Aquacomputer Farbwerk RGB controller
> * Aquacomputer Farbwerk 360 RGB controller
> * Aquacomputer Octo fan controller
>
> @@ -32,7 +33,7 @@ better suited for userspace tools.
> The Octo exposes four temperature sensors and eight PWM controllable fans, along
> with their speed (in RPM), power, voltage and current.
>
> -The Farbwerk 360 exposes four temperature sensors. Depending on the device,
> +The Farbwerk and Farbwerk 360 expose four temperature sensors. Depending on the device,
> not all sysfs and debugfs entries will be available.
>
> Usage notes
> diff --git a/MAINTAINERS b/MAINTAINERS
> index ea2cd656ee6c..d8e3ca0fbc3a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1389,6 +1389,7 @@ F: drivers/media/i2c/aptina-pll.*
>
> AQUACOMPUTER D5 NEXT PUMP SENSOR DRIVER
> M: Aleksa Savic <savicaleksa83@gmail.com>
> +M: Jack Doan <me@jackdoan.com>
> L: linux-hwmon@vger.kernel.org
> S: Maintained
> F: Documentation/hwmon/aquacomputer_d5next.rst
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 5beadd8a0932..01d10c9b633a 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -256,12 +256,13 @@ config SENSORS_AHT10
> will be called aht10.
>
> config SENSORS_AQUACOMPUTER_D5NEXT
> - tristate "Aquacomputer D5 Next, Octo and Farbwerk 360"
> + tristate "Aquacomputer D5 Next, Octo, Farbwerk, Farbwerk 360"
> depends on USB_HID
> help
> If you say yes here you get support for sensors and fans of
> the Aquacomputer D5 Next watercooling pump, Octo fan
> - controller and Farbwerk 360 RGB controller, where available.
> + controller, Farbwerk and Farbwerk 360 RGB controllers, 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 a464473bc981..7d2e7279abfb 100644
> --- a/drivers/hwmon/aquacomputer_d5next.c
> +++ b/drivers/hwmon/aquacomputer_d5next.c
> @@ -1,11 +1,12 @@
> // SPDX-License-Identifier: GPL-2.0+
> /*
> - * hwmon driver for Aquacomputer devices (D5 Next, Farbwerk 360, Octo)
> + * hwmon driver for Aquacomputer devices (D5 Next, Farbwerk, Farbwerk 360, Octo)
> *
> * Aquacomputer devices send HID reports (with ID 0x01) every second to report
> * sensor values.
> *
> * Copyright 2021 Aleksa Savic <savicaleksa83@gmail.com>
> + * Copyright 2022 Jack Doan <me@jackdoan.com>
That is a bit aggressive for a few lines of code.
> */
>
> #include <linux/crc16.h>
> @@ -19,14 +20,16 @@
> #include <asm/unaligned.h>
>
> #define USB_VENDOR_ID_AQUACOMPUTER 0x0c70
> +#define USB_PRODUCT_ID_FARBWERK 0xf00a
> #define USB_PRODUCT_ID_D5NEXT 0xf00e
> #define USB_PRODUCT_ID_FARBWERK360 0xf010
> #define USB_PRODUCT_ID_OCTO 0xf011
>
> -enum kinds { d5next, farbwerk360, octo };
> +enum kinds { d5next, farbwerk, farbwerk360, octo };
>
> static const char *const aqc_device_names[] = {
> [d5next] = "d5next",
> + [farbwerk] = "farbwerk",
> [farbwerk360] = "farbwerk360",
> [octo] = "octo"
> };
> @@ -69,6 +72,12 @@ static u8 secondary_ctrl_report[] = {
> #define D5NEXT_PUMP_CURRENT 112
> #define D5NEXT_FAN_CURRENT 99
>
> +/* Register offsets for the Farbwerk RGB controller */
> +#define FARBWERK_NUM_SENSORS 4
> +#define FARBWERK_SENSOR_START 0x2f
> +#define FARBWERK_SENSOR_SIZE 0x02
> +#define FARBWERK_SENSOR_DISCONNECTED 0x7FFF
> +
> /* Register offsets for the Farbwerk 360 RGB controller */
> #define FARBWERK360_NUM_SENSORS 4
> #define FARBWERK360_SENSOR_START 0x32
> @@ -125,7 +134,7 @@ static const char *const label_d5next_current[] = {
> "Fan current"
> };
>
> -/* Labels for Farbwerk 360 and Octo temperature sensors */
> +/* Labels for Farbwerk, Farbwerk 360 and Octo temperature sensors */
> static const char *const label_temp_sensors[] = {
> "Sensor 1",
> "Sensor 2",
> @@ -319,6 +328,7 @@ static umode_t aqc_is_visible(const void *data, enum hwmon_sensor_types type, u3
> if (channel == 0)
> return 0444;
> break;
> + case farbwerk:
> case farbwerk360:
> case octo:
> return 0444;
> @@ -551,8 +561,7 @@ static const struct hwmon_chip_info aqc_chip_info = {
> .info = aqc_info,
> };
>
> -static int aqc_raw_event(struct hid_device *hdev, struct hid_report *report, u8 *data,
> - int size)
> +static int aqc_raw_event(struct hid_device *hdev, struct hid_report *report, u8 *data, int size)
> {
> int i, sensor_value;
> struct aqc_data *priv;
> @@ -587,6 +596,17 @@ static int aqc_raw_event(struct hid_device *hdev, struct hid_report *report, u8
> priv->current_input[0] = get_unaligned_be16(data + D5NEXT_PUMP_CURRENT);
> priv->current_input[1] = get_unaligned_be16(data + D5NEXT_FAN_CURRENT);
> break;
> + case farbwerk:
> + /* Temperature sensor readings */
> + for (i = 0; i < FARBWERK_NUM_SENSORS; i++) {
> + sensor_value = get_unaligned_be16(data + FARBWERK_SENSOR_START +
> + i * FARBWERK_SENSOR_SIZE);
> + if (sensor_value == FARBWERK_SENSOR_DISCONNECTED)
> + priv->temp_input[i] = -ENODATA;
Can the sensor be connected dynamically ? If not, this should be handled
in an is_visible function.
> + else
> + priv->temp_input[i] = sensor_value * 10;
> + }
> + break;
> case farbwerk360:
> /* Temperature sensor readings */
> for (i = 0; i < FARBWERK360_NUM_SENSORS; i++) {
> @@ -733,6 +753,11 @@ static int aqc_probe(struct hid_device *hdev, const struct hid_device_id *id)
> priv->voltage_label = label_d5next_voltages;
> priv->current_label = label_d5next_current;
> break;
> + case USB_PRODUCT_ID_FARBWERK:
> + priv->kind = farbwerk;
> +
> + priv->temp_label = label_temp_sensors;
> + break;
> case USB_PRODUCT_ID_FARBWERK360:
> priv->kind = farbwerk360;
>
> @@ -795,6 +820,7 @@ static void aqc_remove(struct hid_device *hdev)
>
> static const struct hid_device_id aqc_table[] = {
> { HID_USB_DEVICE(USB_VENDOR_ID_AQUACOMPUTER, USB_PRODUCT_ID_D5NEXT) },
> + { HID_USB_DEVICE(USB_VENDOR_ID_AQUACOMPUTER, USB_PRODUCT_ID_FARBWERK) },
> { HID_USB_DEVICE(USB_VENDOR_ID_AQUACOMPUTER, USB_PRODUCT_ID_FARBWERK360) },
> { HID_USB_DEVICE(USB_VENDOR_ID_AQUACOMPUTER, USB_PRODUCT_ID_OCTO) },
> { }
> @@ -826,4 +852,5 @@ module_exit(aqc_exit);
>
> MODULE_LICENSE("GPL");
> MODULE_AUTHOR("Aleksa Savic <savicaleksa83@gmail.com>");
> +MODULE_AUTHOR("Jack Doan <me@jackdoan.com>");
.... as is claiming authorship. I'd be the "author" of hundreds of kernel
files if I would do that. Aleksa signed off on it, so I'll accept it,
but I don't think it is appropriate.
> MODULE_DESCRIPTION("Hwmon driver for Aquacomputer devices");
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] hwmon: (aquacomputer_d5next) Add support for Aquacomputer Farbwerk
2022-04-24 1:42 ` Guenter Roeck
@ 2022-04-24 3:56 ` Jack Doan
[not found] ` <0bf6576c-c129-f8fb-19c3-e3fca797cfee@jackdoan.com>
1 sibling, 0 replies; 9+ messages in thread
From: Jack Doan @ 2022-04-24 3:56 UTC (permalink / raw)
To: Guenter Roeck
Cc: Aleksa Savic, linux-hwmon, Jean Delvare, Jonathan Corbet,
linux-doc, linux-kernel
On Sat, Apr 23, 2022 at 06:42:06PM -0700, Guenter Roeck wrote:
> On Thu, Apr 21, 2022 at 09:27:42AM +0200, Aleksa Savic wrote:
> > Extend aquacomputer_d5next driver to expose hardware temperature sensors
> > of the Aquacomputer Farbwerk RGB controller, which communicates through
> > a proprietary USB HID protocol.
> >
> > Four temperature sensors are available. Additionally, serial number and
> > firmware version are exposed through debugfs.
> >
> > Also, add Jack Doan to MAINTAINERS for this driver.
> >
> > Signed-off-by: Jack Doan <me@jackdoan.com>
> > Signed-off-by: Aleksa Savic <savicaleksa83@gmail.com>
> > ---
>
> This patch doesn't apply. Please rebase to master and resend.
>
Will do!
> More comments inline.
>
> > If adding to MAINTAINERS requires a separate commit, I'll send it
> > separately.
> > ---
> > Documentation/hwmon/aquacomputer_d5next.rst | 3 +-
> > MAINTAINERS | 1 +
> > drivers/hwmon/Kconfig | 5 +--
> > drivers/hwmon/aquacomputer_d5next.c | 37 ++++++++++++++++++---
> > 4 files changed, 38 insertions(+), 8 deletions(-)
> >
> > diff --git a/Documentation/hwmon/aquacomputer_d5next.rst b/Documentation/hwmon/aquacomputer_d5next.rst
> > index e69f718caf5b..717e28226cde 100644
> > --- a/Documentation/hwmon/aquacomputer_d5next.rst
> > +++ b/Documentation/hwmon/aquacomputer_d5next.rst
> > @@ -6,6 +6,7 @@ Kernel driver aquacomputer-d5next
> > Supported devices:
> >
> > * Aquacomputer D5 Next watercooling pump
> > +* Aquacomputer Farbwerk RGB controller
> > * Aquacomputer Farbwerk 360 RGB controller
> > * Aquacomputer Octo fan controller
> >
> > @@ -32,7 +33,7 @@ better suited for userspace tools.
> > The Octo exposes four temperature sensors and eight PWM controllable fans, along
> > with their speed (in RPM), power, voltage and current.
> >
> > -The Farbwerk 360 exposes four temperature sensors. Depending on the device,
> > +The Farbwerk and Farbwerk 360 expose four temperature sensors. Depending on the device,
> > not all sysfs and debugfs entries will be available.
> >
> > Usage notes
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index ea2cd656ee6c..d8e3ca0fbc3a 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -1389,6 +1389,7 @@ F: drivers/media/i2c/aptina-pll.*
> >
> > AQUACOMPUTER D5 NEXT PUMP SENSOR DRIVER
> > M: Aleksa Savic <savicaleksa83@gmail.com>
> > +M: Jack Doan <me@jackdoan.com>
> > L: linux-hwmon@vger.kernel.org
> > S: Maintained
> > F: Documentation/hwmon/aquacomputer_d5next.rst
> > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> > index 5beadd8a0932..01d10c9b633a 100644
> > --- a/drivers/hwmon/Kconfig
> > +++ b/drivers/hwmon/Kconfig
> > @@ -256,12 +256,13 @@ config SENSORS_AHT10
> > will be called aht10.
> >
> > config SENSORS_AQUACOMPUTER_D5NEXT
> > - tristate "Aquacomputer D5 Next, Octo and Farbwerk 360"
> > + tristate "Aquacomputer D5 Next, Octo, Farbwerk, Farbwerk 360"
> > depends on USB_HID
> > help
> > If you say yes here you get support for sensors and fans of
> > the Aquacomputer D5 Next watercooling pump, Octo fan
> > - controller and Farbwerk 360 RGB controller, where available.
> > + controller, Farbwerk and Farbwerk 360 RGB controllers, 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 a464473bc981..7d2e7279abfb 100644
> > --- a/drivers/hwmon/aquacomputer_d5next.c
> > +++ b/drivers/hwmon/aquacomputer_d5next.c
> > @@ -1,11 +1,12 @@
> > // SPDX-License-Identifier: GPL-2.0+
> > /*
> > - * hwmon driver for Aquacomputer devices (D5 Next, Farbwerk 360, Octo)
> > + * hwmon driver for Aquacomputer devices (D5 Next, Farbwerk, Farbwerk 360, Octo)
> > *
> > * Aquacomputer devices send HID reports (with ID 0x01) every second to report
> > * sensor values.
> > *
> > * Copyright 2021 Aleksa Savic <savicaleksa83@gmail.com>
> > + * Copyright 2022 Jack Doan <me@jackdoan.com>
>
> That is a bit aggressive for a few lines of code.
Addressed below.
>
> > */
> >
> > #include <linux/crc16.h>
> > @@ -19,14 +20,16 @@
> > #include <asm/unaligned.h>
> >
> > #define USB_VENDOR_ID_AQUACOMPUTER 0x0c70
> > +#define USB_PRODUCT_ID_FARBWERK 0xf00a
> > #define USB_PRODUCT_ID_D5NEXT 0xf00e
> > #define USB_PRODUCT_ID_FARBWERK360 0xf010
> > #define USB_PRODUCT_ID_OCTO 0xf011
> >
> > -enum kinds { d5next, farbwerk360, octo };
> > +enum kinds { d5next, farbwerk, farbwerk360, octo };
> >
> > static const char *const aqc_device_names[] = {
> > [d5next] = "d5next",
> > + [farbwerk] = "farbwerk",
> > [farbwerk360] = "farbwerk360",
> > [octo] = "octo"
> > };
> > @@ -69,6 +72,12 @@ static u8 secondary_ctrl_report[] = {
> > #define D5NEXT_PUMP_CURRENT 112
> > #define D5NEXT_FAN_CURRENT 99
> >
> > +/* Register offsets for the Farbwerk RGB controller */
> > +#define FARBWERK_NUM_SENSORS 4
> > +#define FARBWERK_SENSOR_START 0x2f
> > +#define FARBWERK_SENSOR_SIZE 0x02
> > +#define FARBWERK_SENSOR_DISCONNECTED 0x7FFF
> > +
> > /* Register offsets for the Farbwerk 360 RGB controller */
> > #define FARBWERK360_NUM_SENSORS 4
> > #define FARBWERK360_SENSOR_START 0x32
> > @@ -125,7 +134,7 @@ static const char *const label_d5next_current[] = {
> > "Fan current"
> > };
> >
> > -/* Labels for Farbwerk 360 and Octo temperature sensors */
> > +/* Labels for Farbwerk, Farbwerk 360 and Octo temperature sensors */
> > static const char *const label_temp_sensors[] = {
> > "Sensor 1",
> > "Sensor 2",
> > @@ -319,6 +328,7 @@ static umode_t aqc_is_visible(const void *data, enum hwmon_sensor_types type, u3
> > if (channel == 0)
> > return 0444;
> > break;
> > + case farbwerk:
> > case farbwerk360:
> > case octo:
> > return 0444;
> > @@ -551,8 +561,7 @@ static const struct hwmon_chip_info aqc_chip_info = {
> > .info = aqc_info,
> > };
> >
> > -static int aqc_raw_event(struct hid_device *hdev, struct hid_report *report, u8 *data,
> > - int size)
> > +static int aqc_raw_event(struct hid_device *hdev, struct hid_report *report, u8 *data, int size)
> > {
> > int i, sensor_value;
> > struct aqc_data *priv;
> > @@ -587,6 +596,17 @@ static int aqc_raw_event(struct hid_device *hdev, struct hid_report *report, u8
> > priv->current_input[0] = get_unaligned_be16(data + D5NEXT_PUMP_CURRENT);
> > priv->current_input[1] = get_unaligned_be16(data + D5NEXT_FAN_CURRENT);
> > break;
> > + case farbwerk:
> > + /* Temperature sensor readings */
> > + for (i = 0; i < FARBWERK_NUM_SENSORS; i++) {
> > + sensor_value = get_unaligned_be16(data + FARBWERK_SENSOR_START +
> > + i * FARBWERK_SENSOR_SIZE);
> > + if (sensor_value == FARBWERK_SENSOR_DISCONNECTED)
> > + priv->temp_input[i] = -ENODATA;
>
> Can the sensor be connected dynamically ? If not, this should be handled
> in an is_visible function.
>
It's a somewhat unlikely use-case, but yes, they can be. The sensors are NTC thermistors that connect to pin-headers. Do we have a better way of representing "open circuit" than -ENODATA?
> > + else
> > + priv->temp_input[i] = sensor_value * 10;
> > + }
> > + break;
> > case farbwerk360:
> > /* Temperature sensor readings */
> > for (i = 0; i < FARBWERK360_NUM_SENSORS; i++) {
> > @@ -733,6 +753,11 @@ static int aqc_probe(struct hid_device *hdev, const struct hid_device_id *id)
> > priv->voltage_label = label_d5next_voltages;
> > priv->current_label = label_d5next_current;
> > break;
> > + case USB_PRODUCT_ID_FARBWERK:
> > + priv->kind = farbwerk;
> > +
> > + priv->temp_label = label_temp_sensors;
> > + break;
> > case USB_PRODUCT_ID_FARBWERK360:
> > priv->kind = farbwerk360;
> >
> > @@ -795,6 +820,7 @@ static void aqc_remove(struct hid_device *hdev)
> >
> > static const struct hid_device_id aqc_table[] = {
> > { HID_USB_DEVICE(USB_VENDOR_ID_AQUACOMPUTER, USB_PRODUCT_ID_D5NEXT) },
> > + { HID_USB_DEVICE(USB_VENDOR_ID_AQUACOMPUTER, USB_PRODUCT_ID_FARBWERK) },
> > { HID_USB_DEVICE(USB_VENDOR_ID_AQUACOMPUTER, USB_PRODUCT_ID_FARBWERK360) },
> > { HID_USB_DEVICE(USB_VENDOR_ID_AQUACOMPUTER, USB_PRODUCT_ID_OCTO) },
> > { }
> > @@ -826,4 +852,5 @@ module_exit(aqc_exit);
> >
> > MODULE_LICENSE("GPL");
> > MODULE_AUTHOR("Aleksa Savic <savicaleksa83@gmail.com>");
> > +MODULE_AUTHOR("Jack Doan <me@jackdoan.com>");
>
> .... as is claiming authorship. I'd be the "author" of hundreds of kernel
> files if I would do that. Aleksa signed off on it, so I'll accept it,
> but I don't think it is appropriate.
In the context of just this patch, yes, I agree. But, I felt it was warranted based on my previous contributions. I did a good bit of the reverse-engineering that made writing settings to these devices possible, and a lot of the previously submitted code for writing the setting changes is mine as well.
I didn't mean to make an inappropriate request though! If you'd like, I can submit a version of the patch without these lines.
Also I'm really sorry if you get this email twice. Thunderbird seems to have snuck HTML into my emails and the mailinglist rejected my first reply. I've switched to mutt to prevent this.
>
> > MODULE_DESCRIPTION("Hwmon driver for Aquacomputer devices");
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] hwmon: (aquacomputer_d5next) Add support for Aquacomputer Farbwerk
[not found] ` <0bf6576c-c129-f8fb-19c3-e3fca797cfee@jackdoan.com>
@ 2022-04-24 5:01 ` Guenter Roeck
2022-04-24 5:16 ` Aleksa Savic
0 siblings, 1 reply; 9+ messages in thread
From: Guenter Roeck @ 2022-04-24 5:01 UTC (permalink / raw)
To: Jack Doan, Aleksa Savic
Cc: linux-hwmon, Jean Delvare, Jonathan Corbet, linux-doc, linux-kernel
On 4/23/22 20:47, Jack Doan wrote:
> On 4/23/22 20:42, Guenter Roeck wrote:
>
>> On Thu, Apr 21, 2022 at 09:27:42AM +0200, Aleksa Savic wrote:
>>> Extend aquacomputer_d5next driver to expose hardware temperature sensors
>>> of the Aquacomputer Farbwerk RGB controller, which communicates through
>>> a proprietary USB HID protocol.
>>>
>>> Four temperature sensors are available. Additionally, serial number and
>>> firmware version are exposed through debugfs.
>>>
>>> Also, add Jack Doan to MAINTAINERS for this driver.
>>>
>>> Signed-off-by: Jack Doan<me@jackdoan.com>
>>> Signed-off-by: Aleksa Savic<savicaleksa83@gmail.com>
>>> ---
>> This patch doesn't apply. Please rebase to master and resend.
> Will do!
>> More comments inline.
>>
>>> If adding to MAINTAINERS requires a separate commit, I'll send it
>>> separately.
>>> ---
>>> Documentation/hwmon/aquacomputer_d5next.rst | 3 +-
>>> MAINTAINERS | 1 +
>>> drivers/hwmon/Kconfig | 5 +--
>>> drivers/hwmon/aquacomputer_d5next.c | 37 ++++++++++++++++++---
>>> 4 files changed, 38 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/Documentation/hwmon/aquacomputer_d5next.rst b/Documentation/hwmon/aquacomputer_d5next.rst
>>> index e69f718caf5b..717e28226cde 100644
>>> --- a/Documentation/hwmon/aquacomputer_d5next.rst
>>> +++ b/Documentation/hwmon/aquacomputer_d5next.rst
>>> @@ -6,6 +6,7 @@ Kernel driver aquacomputer-d5next
>>> Supported devices:
>>>
>>> * Aquacomputer D5 Next watercooling pump
>>> +* Aquacomputer Farbwerk RGB controller
>>> * Aquacomputer Farbwerk 360 RGB controller
>>> * Aquacomputer Octo fan controller
>>>
>>> @@ -32,7 +33,7 @@ better suited for userspace tools.
>>> The Octo exposes four temperature sensors and eight PWM controllable fans, along
>>> with their speed (in RPM), power, voltage and current.
>>>
>>> -The Farbwerk 360 exposes four temperature sensors. Depending on the device,
>>> +The Farbwerk and Farbwerk 360 expose four temperature sensors. Depending on the device,
>>> not all sysfs and debugfs entries will be available.
>>>
>>> Usage notes
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index ea2cd656ee6c..d8e3ca0fbc3a 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -1389,6 +1389,7 @@ F: drivers/media/i2c/aptina-pll.*
>>>
>>> AQUACOMPUTER D5 NEXT PUMP SENSOR DRIVER
>>> M: Aleksa Savic<savicaleksa83@gmail.com>
>>> +M: Jack Doan<me@jackdoan.com>
>>> L: linux-hwmon@vger.kernel.org
>>> S: Maintained
>>> F: Documentation/hwmon/aquacomputer_d5next.rst
>>> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
>>> index 5beadd8a0932..01d10c9b633a 100644
>>> --- a/drivers/hwmon/Kconfig
>>> +++ b/drivers/hwmon/Kconfig
>>> @@ -256,12 +256,13 @@ config SENSORS_AHT10
>>> will be called aht10.
>>>
>>> config SENSORS_AQUACOMPUTER_D5NEXT
>>> - tristate "Aquacomputer D5 Next, Octo and Farbwerk 360"
>>> + tristate "Aquacomputer D5 Next, Octo, Farbwerk, Farbwerk 360"
>>> depends on USB_HID
>>> help
>>> If you say yes here you get support for sensors and fans of
>>> the Aquacomputer D5 Next watercooling pump, Octo fan
>>> - controller and Farbwerk 360 RGB controller, where available.
>>> + controller, Farbwerk and Farbwerk 360 RGB controllers, 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 a464473bc981..7d2e7279abfb 100644
>>> --- a/drivers/hwmon/aquacomputer_d5next.c
>>> +++ b/drivers/hwmon/aquacomputer_d5next.c
>>> @@ -1,11 +1,12 @@
>>> // SPDX-License-Identifier: GPL-2.0+
>>> /*
>>> - * hwmon driver for Aquacomputer devices (D5 Next, Farbwerk 360, Octo)
>>> + * hwmon driver for Aquacomputer devices (D5 Next, Farbwerk, Farbwerk 360, Octo)
>>> *
>>> * Aquacomputer devices send HID reports (with ID 0x01) every second to report
>>> * sensor values.
>>> *
>>> * Copyright 2021 Aleksa Savic<savicaleksa83@gmail.com>
>>> + * Copyright 2022 Jack Doan<me@jackdoan.com>
>> That is a bit aggressive for a few lines of code.
> Addressed below.
>>> */
>>>
>>> #include <linux/crc16.h>
>>> @@ -19,14 +20,16 @@
>>> #include <asm/unaligned.h>
>>>
>>> #define USB_VENDOR_ID_AQUACOMPUTER 0x0c70
>>> +#define USB_PRODUCT_ID_FARBWERK 0xf00a
>>> #define USB_PRODUCT_ID_D5NEXT 0xf00e
>>> #define USB_PRODUCT_ID_FARBWERK360 0xf010
>>> #define USB_PRODUCT_ID_OCTO 0xf011
>>>
>>> -enum kinds { d5next, farbwerk360, octo };
>>> +enum kinds { d5next, farbwerk, farbwerk360, octo };
>>>
>>> static const char *const aqc_device_names[] = {
>>> [d5next] = "d5next",
>>> + [farbwerk] = "farbwerk",
>>> [farbwerk360] = "farbwerk360",
>>> [octo] = "octo"
>>> };
>>> @@ -69,6 +72,12 @@ static u8 secondary_ctrl_report[] = {
>>> #define D5NEXT_PUMP_CURRENT 112
>>> #define D5NEXT_FAN_CURRENT 99
>>>
>>> +/* Register offsets for the Farbwerk RGB controller */
>>> +#define FARBWERK_NUM_SENSORS 4
>>> +#define FARBWERK_SENSOR_START 0x2f
>>> +#define FARBWERK_SENSOR_SIZE 0x02
>>> +#define FARBWERK_SENSOR_DISCONNECTED 0x7FFF
>>> +
>>> /* Register offsets for the Farbwerk 360 RGB controller */
>>> #define FARBWERK360_NUM_SENSORS 4
>>> #define FARBWERK360_SENSOR_START 0x32
>>> @@ -125,7 +134,7 @@ static const char *const label_d5next_current[] = {
>>> "Fan current"
>>> };
>>>
>>> -/* Labels for Farbwerk 360 and Octo temperature sensors */
>>> +/* Labels for Farbwerk, Farbwerk 360 and Octo temperature sensors */
>>> static const char *const label_temp_sensors[] = {
>>> "Sensor 1",
>>> "Sensor 2",
>>> @@ -319,6 +328,7 @@ static umode_t aqc_is_visible(const void *data, enum hwmon_sensor_types type, u3
>>> if (channel == 0)
>>> return 0444;
>>> break;
>>> + case farbwerk:
>>> case farbwerk360:
>>> case octo:
>>> return 0444;
>>> @@ -551,8 +561,7 @@ static const struct hwmon_chip_info aqc_chip_info = {
>>> .info = aqc_info,
>>> };
>>>
>>> -static int aqc_raw_event(struct hid_device *hdev, struct hid_report *report, u8 *data,
>>> - int size)
>>> +static int aqc_raw_event(struct hid_device *hdev, struct hid_report *report, u8 *data, int size)
>>> {
>>> int i, sensor_value;
>>> struct aqc_data *priv;
>>> @@ -587,6 +596,17 @@ static int aqc_raw_event(struct hid_device *hdev, struct hid_report *report, u8
>>> priv->current_input[0] = get_unaligned_be16(data + D5NEXT_PUMP_CURRENT);
>>> priv->current_input[1] = get_unaligned_be16(data + D5NEXT_FAN_CURRENT);
>>> break;
>>> + case farbwerk:
>>> + /* Temperature sensor readings */
>>> + for (i = 0; i < FARBWERK_NUM_SENSORS; i++) {
>>> + sensor_value = get_unaligned_be16(data + FARBWERK_SENSOR_START +
>>> + i * FARBWERK_SENSOR_SIZE);
>>> + if (sensor_value == FARBWERK_SENSOR_DISCONNECTED)
>>> + priv->temp_input[i] = -ENODATA;
>> Can the sensor be connected dynamically ? If not, this should be handled
>> in an is_visible function.
> It's a somewhat unlikely use-case, but yes, they can be. The sensors are NTC thermistors that connect to pin-headers. Do we have a better way of representing "open circuit" than -ENODATA?
Seems to be a bit theoretic to assume that someone would play with
connecting and disconnecting cables while the system is running.
Have you tried to do that ? It would be interesting to know if
the controller handles that situation. But don't blame me
if you fry your system :-)
Anyway, there could be a tempX_fault attribute which would return 1
in that case. Not sure if that makes more sense, though.
>>> + else
>>> + priv->temp_input[i] = sensor_value * 10;
>>> + }
>>> + break;
>>> case farbwerk360:
>>> /* Temperature sensor readings */
>>> for (i = 0; i < FARBWERK360_NUM_SENSORS; i++) {
>>> @@ -733,6 +753,11 @@ static int aqc_probe(struct hid_device *hdev, const struct hid_device_id *id)
>>> priv->voltage_label = label_d5next_voltages;
>>> priv->current_label = label_d5next_current;
>>> break;
>>> + case USB_PRODUCT_ID_FARBWERK:
>>> + priv->kind = farbwerk;
>>> +
>>> + priv->temp_label = label_temp_sensors;
>>> + break;
>>> case USB_PRODUCT_ID_FARBWERK360:
>>> priv->kind = farbwerk360;
>>>
>>> @@ -795,6 +820,7 @@ static void aqc_remove(struct hid_device *hdev)
>>>
>>> static const struct hid_device_id aqc_table[] = {
>>> { HID_USB_DEVICE(USB_VENDOR_ID_AQUACOMPUTER, USB_PRODUCT_ID_D5NEXT) },
>>> + { HID_USB_DEVICE(USB_VENDOR_ID_AQUACOMPUTER, USB_PRODUCT_ID_FARBWERK) },
>>> { HID_USB_DEVICE(USB_VENDOR_ID_AQUACOMPUTER, USB_PRODUCT_ID_FARBWERK360) },
>>> { HID_USB_DEVICE(USB_VENDOR_ID_AQUACOMPUTER, USB_PRODUCT_ID_OCTO) },
>>> { }
>>> @@ -826,4 +852,5 @@ module_exit(aqc_exit);
>>>
>>> MODULE_LICENSE("GPL");
>>> MODULE_AUTHOR("Aleksa Savic<savicaleksa83@gmail.com>");
>>> +MODULE_AUTHOR("Jack Doan<me@jackdoan.com>");
>> .... as is claiming authorship. I'd be the "author" of hundreds of kernel
>> files if I would do that. Aleksa signed off on it, so I'll accept it,
>> but I don't think it is appropriate.
> In the context of just this patch, yes, I agree. But, I felt it was warranted based on my previous contributions. I did a good bit of the reverse-engineering that made writing settings to these devices possible, and a lot of the previously submitted code for writing the setting changes is mine as well.
>
> I didn't mean to make an inappropriate request though! If you'd like, I can submit a version of the patch without these lines.
No, I think that makes sense as long as Aleksa is ok with it.
Guenter
>>> MODULE_DESCRIPTION("Hwmon driver for Aquacomputer devices");
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] hwmon: (aquacomputer_d5next) Add support for Aquacomputer Farbwerk
2022-04-24 5:01 ` Guenter Roeck
@ 2022-04-24 5:16 ` Aleksa Savic
0 siblings, 0 replies; 9+ messages in thread
From: Aleksa Savic @ 2022-04-24 5:16 UTC (permalink / raw)
To: Guenter Roeck
Cc: Jack Doan, linux-hwmon, Jean Delvare, Jonathan Corbet, linux-doc,
linux-kernel
On Sat, Apr 23, 2022 at 10:01:50PM -0700, Guenter Roeck wrote:
> On 4/23/22 20:47, Jack Doan wrote:
> > On 4/23/22 20:42, Guenter Roeck wrote:
> >
> > > On Thu, Apr 21, 2022 at 09:27:42AM +0200, Aleksa Savic wrote:
> > > > Extend aquacomputer_d5next driver to expose hardware temperature sensors
> > > > of the Aquacomputer Farbwerk RGB controller, which communicates through
> > > > a proprietary USB HID protocol.
> > > >
> > > > Four temperature sensors are available. Additionally, serial number and
> > > > firmware version are exposed through debugfs.
> > > >
> > > > Also, add Jack Doan to MAINTAINERS for this driver.
> > > >
> > > > Signed-off-by: Jack Doan<me@jackdoan.com>
> > > > Signed-off-by: Aleksa Savic<savicaleksa83@gmail.com>
> > > > ---
> > > This patch doesn't apply. Please rebase to master and resend.
> > Will do!
> > > More comments inline.
> > >
> > > > If adding to MAINTAINERS requires a separate commit, I'll send it
> > > > separately.
> > > > ---
> > > > Documentation/hwmon/aquacomputer_d5next.rst | 3 +-
> > > > MAINTAINERS | 1 +
> > > > drivers/hwmon/Kconfig | 5 +--
> > > > drivers/hwmon/aquacomputer_d5next.c | 37 ++++++++++++++++++---
> > > > 4 files changed, 38 insertions(+), 8 deletions(-)
> > > >
> > > > diff --git a/Documentation/hwmon/aquacomputer_d5next.rst b/Documentation/hwmon/aquacomputer_d5next.rst
> > > > index e69f718caf5b..717e28226cde 100644
> > > > --- a/Documentation/hwmon/aquacomputer_d5next.rst
> > > > +++ b/Documentation/hwmon/aquacomputer_d5next.rst
> > > > @@ -6,6 +6,7 @@ Kernel driver aquacomputer-d5next
> > > > Supported devices:
> > > > * Aquacomputer D5 Next watercooling pump
> > > > +* Aquacomputer Farbwerk RGB controller
> > > > * Aquacomputer Farbwerk 360 RGB controller
> > > > * Aquacomputer Octo fan controller
> > > > @@ -32,7 +33,7 @@ better suited for userspace tools.
> > > > The Octo exposes four temperature sensors and eight PWM controllable fans, along
> > > > with their speed (in RPM), power, voltage and current.
> > > > -The Farbwerk 360 exposes four temperature sensors. Depending on the device,
> > > > +The Farbwerk and Farbwerk 360 expose four temperature sensors. Depending on the device,
> > > > not all sysfs and debugfs entries will be available.
> > > > Usage notes
> > > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > > index ea2cd656ee6c..d8e3ca0fbc3a 100644
> > > > --- a/MAINTAINERS
> > > > +++ b/MAINTAINERS
> > > > @@ -1389,6 +1389,7 @@ F: drivers/media/i2c/aptina-pll.*
> > > > AQUACOMPUTER D5 NEXT PUMP SENSOR DRIVER
> > > > M: Aleksa Savic<savicaleksa83@gmail.com>
> > > > +M: Jack Doan<me@jackdoan.com>
> > > > L: linux-hwmon@vger.kernel.org
> > > > S: Maintained
> > > > F: Documentation/hwmon/aquacomputer_d5next.rst
> > > > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> > > > index 5beadd8a0932..01d10c9b633a 100644
> > > > --- a/drivers/hwmon/Kconfig
> > > > +++ b/drivers/hwmon/Kconfig
> > > > @@ -256,12 +256,13 @@ config SENSORS_AHT10
> > > > will be called aht10.
> > > > config SENSORS_AQUACOMPUTER_D5NEXT
> > > > - tristate "Aquacomputer D5 Next, Octo and Farbwerk 360"
> > > > + tristate "Aquacomputer D5 Next, Octo, Farbwerk, Farbwerk 360"
> > > > depends on USB_HID
> > > > help
> > > > If you say yes here you get support for sensors and fans of
> > > > the Aquacomputer D5 Next watercooling pump, Octo fan
> > > > - controller and Farbwerk 360 RGB controller, where available.
> > > > + controller, Farbwerk and Farbwerk 360 RGB controllers, 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 a464473bc981..7d2e7279abfb 100644
> > > > --- a/drivers/hwmon/aquacomputer_d5next.c
> > > > +++ b/drivers/hwmon/aquacomputer_d5next.c
> > > > @@ -1,11 +1,12 @@
> > > > // SPDX-License-Identifier: GPL-2.0+
> > > > /*
> > > > - * hwmon driver for Aquacomputer devices (D5 Next, Farbwerk 360, Octo)
> > > > + * hwmon driver for Aquacomputer devices (D5 Next, Farbwerk, Farbwerk 360, Octo)
> > > > *
> > > > * Aquacomputer devices send HID reports (with ID 0x01) every second to report
> > > > * sensor values.
> > > > *
> > > > * Copyright 2021 Aleksa Savic<savicaleksa83@gmail.com>
> > > > + * Copyright 2022 Jack Doan<me@jackdoan.com>
> > > That is a bit aggressive for a few lines of code.
> > Addressed below.
> > > > */
> > > > #include <linux/crc16.h>
> > > > @@ -19,14 +20,16 @@
> > > > #include <asm/unaligned.h>
> > > > #define USB_VENDOR_ID_AQUACOMPUTER 0x0c70
> > > > +#define USB_PRODUCT_ID_FARBWERK 0xf00a
> > > > #define USB_PRODUCT_ID_D5NEXT 0xf00e
> > > > #define USB_PRODUCT_ID_FARBWERK360 0xf010
> > > > #define USB_PRODUCT_ID_OCTO 0xf011
> > > > -enum kinds { d5next, farbwerk360, octo };
> > > > +enum kinds { d5next, farbwerk, farbwerk360, octo };
> > > > static const char *const aqc_device_names[] = {
> > > > [d5next] = "d5next",
> > > > + [farbwerk] = "farbwerk",
> > > > [farbwerk360] = "farbwerk360",
> > > > [octo] = "octo"
> > > > };
> > > > @@ -69,6 +72,12 @@ static u8 secondary_ctrl_report[] = {
> > > > #define D5NEXT_PUMP_CURRENT 112
> > > > #define D5NEXT_FAN_CURRENT 99
> > > > +/* Register offsets for the Farbwerk RGB controller */
> > > > +#define FARBWERK_NUM_SENSORS 4
> > > > +#define FARBWERK_SENSOR_START 0x2f
> > > > +#define FARBWERK_SENSOR_SIZE 0x02
> > > > +#define FARBWERK_SENSOR_DISCONNECTED 0x7FFF
> > > > +
> > > > /* Register offsets for the Farbwerk 360 RGB controller */
> > > > #define FARBWERK360_NUM_SENSORS 4
> > > > #define FARBWERK360_SENSOR_START 0x32
> > > > @@ -125,7 +134,7 @@ static const char *const label_d5next_current[] = {
> > > > "Fan current"
> > > > };
> > > > -/* Labels for Farbwerk 360 and Octo temperature sensors */
> > > > +/* Labels for Farbwerk, Farbwerk 360 and Octo temperature sensors */
> > > > static const char *const label_temp_sensors[] = {
> > > > "Sensor 1",
> > > > "Sensor 2",
> > > > @@ -319,6 +328,7 @@ static umode_t aqc_is_visible(const void *data, enum hwmon_sensor_types type, u3
> > > > if (channel == 0)
> > > > return 0444;
> > > > break;
> > > > + case farbwerk:
> > > > case farbwerk360:
> > > > case octo:
> > > > return 0444;
> > > > @@ -551,8 +561,7 @@ static const struct hwmon_chip_info aqc_chip_info = {
> > > > .info = aqc_info,
> > > > };
> > > > -static int aqc_raw_event(struct hid_device *hdev, struct hid_report *report, u8 *data,
> > > > - int size)
> > > > +static int aqc_raw_event(struct hid_device *hdev, struct hid_report *report, u8 *data, int size)
> > > > {
> > > > int i, sensor_value;
> > > > struct aqc_data *priv;
> > > > @@ -587,6 +596,17 @@ static int aqc_raw_event(struct hid_device *hdev, struct hid_report *report, u8
> > > > priv->current_input[0] = get_unaligned_be16(data + D5NEXT_PUMP_CURRENT);
> > > > priv->current_input[1] = get_unaligned_be16(data + D5NEXT_FAN_CURRENT);
> > > > break;
> > > > + case farbwerk:
> > > > + /* Temperature sensor readings */
> > > > + for (i = 0; i < FARBWERK_NUM_SENSORS; i++) {
> > > > + sensor_value = get_unaligned_be16(data + FARBWERK_SENSOR_START +
> > > > + i * FARBWERK_SENSOR_SIZE);
> > > > + if (sensor_value == FARBWERK_SENSOR_DISCONNECTED)
> > > > + priv->temp_input[i] = -ENODATA;
> > > Can the sensor be connected dynamically ? If not, this should be handled
> > > in an is_visible function.
> > It's a somewhat unlikely use-case, but yes, they can be. The sensors are NTC thermistors that connect to pin-headers. Do we have a better way of representing "open circuit" than -ENODATA?
>
> Seems to be a bit theoretic to assume that someone would play with
> connecting and disconnecting cables while the system is running.
> Have you tried to do that ? It would be interesting to know if
> the controller handles that situation. But don't blame me
> if you fry your system :-)
>
> Anyway, there could be a tempX_fault attribute which would return 1
> in that case. Not sure if that makes more sense, though.
They handle hot plugging just fine, that's how I initially tested if I
got the offsets right wihout having to reboot. Same goes for PWM inputs
on devices that have them.
> > > > + else
> > > > + priv->temp_input[i] = sensor_value * 10;
> > > > + }
> > > > + break;
> > > > case farbwerk360:
> > > > /* Temperature sensor readings */
> > > > for (i = 0; i < FARBWERK360_NUM_SENSORS; i++) {
> > > > @@ -733,6 +753,11 @@ static int aqc_probe(struct hid_device *hdev, const struct hid_device_id *id)
> > > > priv->voltage_label = label_d5next_voltages;
> > > > priv->current_label = label_d5next_current;
> > > > break;
> > > > + case USB_PRODUCT_ID_FARBWERK:
> > > > + priv->kind = farbwerk;
> > > > +
> > > > + priv->temp_label = label_temp_sensors;
> > > > + break;
> > > > case USB_PRODUCT_ID_FARBWERK360:
> > > > priv->kind = farbwerk360;
> > > > @@ -795,6 +820,7 @@ static void aqc_remove(struct hid_device *hdev)
> > > > static const struct hid_device_id aqc_table[] = {
> > > > { HID_USB_DEVICE(USB_VENDOR_ID_AQUACOMPUTER, USB_PRODUCT_ID_D5NEXT) },
> > > > + { HID_USB_DEVICE(USB_VENDOR_ID_AQUACOMPUTER, USB_PRODUCT_ID_FARBWERK) },
> > > > { HID_USB_DEVICE(USB_VENDOR_ID_AQUACOMPUTER, USB_PRODUCT_ID_FARBWERK360) },
> > > > { HID_USB_DEVICE(USB_VENDOR_ID_AQUACOMPUTER, USB_PRODUCT_ID_OCTO) },
> > > > { }
> > > > @@ -826,4 +852,5 @@ module_exit(aqc_exit);
> > > > MODULE_LICENSE("GPL");
> > > > MODULE_AUTHOR("Aleksa Savic<savicaleksa83@gmail.com>");
> > > > +MODULE_AUTHOR("Jack Doan<me@jackdoan.com>");
> > > .... as is claiming authorship. I'd be the "author" of hundreds of kernel
> > > files if I would do that. Aleksa signed off on it, so I'll accept it,
> > > but I don't think it is appropriate.
> > In the context of just this patch, yes, I agree. But, I felt it was warranted based on my previous contributions. I did a good bit of the reverse-engineering that made writing settings to these devices possible, and a lot of the previously submitted code for writing the setting changes is mine as well.
> >
> > I didn't mean to make an inappropriate request though! If you'd like, I can submit a version of the patch without these lines.
>
> No, I think that makes sense as long as Aleksa is ok with it.
>
> Guenter
Yes, I'm OK with adding Jack since he's the main author of this and some bigger patches
that we're working on at the Github repo, such as adding support for Quadro and setting
fan/pump curves directly on the D5 Next pump (which I'd say took a great deal of
reverse engineering, at least from my POV).
Thanks,
Aleksa
> > > > MODULE_DESCRIPTION("Hwmon driver for Aquacomputer devices");
>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-04-24 5:16 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-21 7:27 [PATCH] hwmon: (aquacomputer_d5next) Add support for Aquacomputer Farbwerk Aleksa Savic
2022-04-22 7:23 ` Aleksa Savic
2022-04-22 18:31 ` Guenter Roeck
2022-04-23 16:23 ` Aleksa Savic
2022-04-23 16:34 ` Guenter Roeck
2022-04-24 1:42 ` Guenter Roeck
2022-04-24 3:56 ` Jack Doan
[not found] ` <0bf6576c-c129-f8fb-19c3-e3fca797cfee@jackdoan.com>
2022-04-24 5:01 ` Guenter Roeck
2022-04-24 5:16 ` Aleksa Savic
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).