2022-02-21 14:31:04

by Aleksa Savic

[permalink] [raw]
Subject: [PATCH] hwmon: add driver for Aquacomputer Farbwerk 360

This driver exposes hardware temperature sensors of the Aquacomputer
Farbwerk 360 RGB controller, which communicates through a proprietary
USB HID protocol.

Four temperature sensors are available. If a sensor is not connected,
it will report zeroes. Additionally, serial number and firmware version
are exposed through debugfs.

This driver has been tested on x86_64.

Signed-off-by: Aleksa Savic <[email protected]>
---
.../hwmon/aquacomputer_farbwerk360.rst | 40 +++
Documentation/hwmon/index.rst | 1 +
MAINTAINERS | 7 +
drivers/hwmon/Kconfig | 10 +
drivers/hwmon/Makefile | 1 +
drivers/hwmon/aquacomputer_farbwerk360.c | 261 ++++++++++++++++++
6 files changed, 320 insertions(+)
create mode 100644 Documentation/hwmon/aquacomputer_farbwerk360.rst
create mode 100644 drivers/hwmon/aquacomputer_farbwerk360.c

diff --git a/Documentation/hwmon/aquacomputer_farbwerk360.rst b/Documentation/hwmon/aquacomputer_farbwerk360.rst
new file mode 100644
index 000000000000..cebaffccd818
--- /dev/null
+++ b/Documentation/hwmon/aquacomputer_farbwerk360.rst
@@ -0,0 +1,40 @@
+.. SPDX-License-Identifier: GPL-2.0-or-later
+
+Kernel driver aquacomputer_farbwerk360
+======================================
+
+Supported devices:
+
+* Aquacomputer Farbwerk 360 RGB controller
+
+Author: Aleksa Savic
+
+Description
+-----------
+
+This driver exposes hardware temperature sensors of the Aquacomputer Farbwerk 360
+RGB controller, which communicates through a proprietary USB HID protocol.
+
+Four temperature sensors are available. If a sensor is not connected, it will report
+zeroes. Additionally, serial number and firmware version are exposed through debugfs.
+
+Usage notes
+-----------
+
+Farbwerk 360 communicates via HID reports. The driver is loaded automatically by
+the kernel and supports hotswapping.
+
+Sysfs entries
+-------------
+
+=============== ==============================================
+temp[1-4]_input Measured temperature (in millidegrees Celsius)
+=============== ==============================================
+
+Debugfs entries
+---------------
+
+================ ===============================================
+serial_number Serial number of the pump
+firmware_version Version of installed firmware
+================ ===============================================
diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
index df20022c741f..42b7369340ba 100644
--- a/Documentation/hwmon/index.rst
+++ b/Documentation/hwmon/index.rst
@@ -40,6 +40,7 @@ Hardware Monitoring Kernel Drivers
aht10
amc6821
aquacomputer_d5next
+ aquacomputer_farbwerk360
asb100
asc7621
aspeed-pwm-tacho
diff --git a/MAINTAINERS b/MAINTAINERS
index bd86ed9fbc79..fb8b8d7aebbc 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1386,6 +1386,13 @@ S: Maintained
F: Documentation/hwmon/aquacomputer_d5next.rst
F: drivers/hwmon/aquacomputer_d5next.c

+AQUACOMPUTER FARBWERK 360 RGB CONTROLLER SENSOR DRIVER
+M: Aleksa Savic <[email protected]>
+L: [email protected]
+S: Maintained
+F: Documentation/hwmon/aquacomputer_farbwerk360.rst
+F: drivers/hwmon/aquacomputer_farbwerk360.c
+
AQUANTIA ETHERNET DRIVER (atlantic)
M: Igor Russkikh <[email protected]>
L: [email protected]
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 8df25f1079ba..e1ca5e1e6ab0 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -264,6 +264,16 @@ config SENSORS_AQUACOMPUTER_D5NEXT
This driver can also be built as a module. If so, the module
will be called aquacomputer_d5next.

+config SENSORS_AQUACOMPUTER_FARBWERK360
+ tristate "Aquacomputer Farbwerk 360 RGB controller"
+ depends on USB_HID
+ help
+ If you say yes here you get support for temperature sensors provided by
+ the Aquacomputer Farbwerk 360 RGB controller.
+
+ This driver can also be built as a module. If so, the module
+ will be called aquacomputer_farbwerk360.
+
config SENSORS_AS370
tristate "Synaptics AS370 SoC hardware monitoring driver"
help
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 185f946d698b..1c1556a53f6d 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -49,6 +49,7 @@ obj-$(CONFIG_SENSORS_ADT7475) += adt7475.o
obj-$(CONFIG_SENSORS_AHT10) += aht10.o
obj-$(CONFIG_SENSORS_APPLESMC) += applesmc.o
obj-$(CONFIG_SENSORS_AQUACOMPUTER_D5NEXT) += aquacomputer_d5next.o
+obj-$(CONFIG_SENSORS_AQUACOMPUTER_FARBWERK360) += aquacomputer_farbwerk360.o
obj-$(CONFIG_SENSORS_ARM_SCMI) += scmi-hwmon.o
obj-$(CONFIG_SENSORS_ARM_SCPI) += scpi-hwmon.o
obj-$(CONFIG_SENSORS_AS370) += as370-hwmon.o
diff --git a/drivers/hwmon/aquacomputer_farbwerk360.c b/drivers/hwmon/aquacomputer_farbwerk360.c
new file mode 100644
index 000000000000..14b760a2c8a8
--- /dev/null
+++ b/drivers/hwmon/aquacomputer_farbwerk360.c
@@ -0,0 +1,261 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * hwmon driver for Aquacomputer Farbwerk 360 (RGB controller)
+ *
+ * The Farbwerk 360 sends HID reports (with ID 0x01) every second to report sensor values
+ * of up to four connected temperature sensors.
+ *
+ * Copyright 2022 Aleksa Savic <[email protected]>
+ */
+
+#include <asm/unaligned.h>
+#include <linux/debugfs.h>
+#include <linux/hid.h>
+#include <linux/hwmon.h>
+#include <linux/jiffies.h>
+#include <linux/module.h>
+#include <linux/seq_file.h>
+
+#define DRIVER_NAME "aquacomputer_farbwerk360"
+
+#define STATUS_REPORT_ID 0x01
+#define STATUS_UPDATE_INTERVAL (2 * HZ) /* In seconds */
+
+/* Register offsets */
+#define SERIAL_FIRST_PART 0x03
+#define SERIAL_SECOND_PART 0x05
+#define FIRMWARE_VERSION 0x0D
+
+#define NUM_SENSORS 4
+#define SENSOR_START 0x32
+#define SENSOR_SIZE 0x02
+#define SENSOR_DISCONNECTED 0x7FFF
+
+static const char *const label_temps[] = { "Sensor 1", "Sensor 2", "Sensor 3", "Sensor 4" };
+
+struct farbwerk360_data {
+ struct hid_device *hdev;
+ struct device *hwmon_dev;
+ struct dentry *debugfs;
+ s32 temp_input[4];
+ u32 serial_number[2];
+ u16 firmware_version;
+ unsigned long updated;
+};
+
+static umode_t farbwerk360_is_visible(const void *data, enum hwmon_sensor_types type, u32 attr,
+ int channel)
+{
+ return 0444;
+}
+
+static int farbwerk360_read(struct device *dev, enum hwmon_sensor_types type, u32 attr, int channel,
+ long *val)
+{
+ struct farbwerk360_data *priv = dev_get_drvdata(dev);
+
+ if (time_after(jiffies, priv->updated + STATUS_UPDATE_INTERVAL))
+ return -ENODATA;
+
+ switch (type) {
+ case hwmon_temp:
+ *val = priv->temp_input[channel];
+ break;
+ default:
+ return -EOPNOTSUPP;
+ }
+
+ return 0;
+}
+
+static int farbwerk360_read_string(struct device *dev, enum hwmon_sensor_types type, u32 attr,
+ int channel, const char **str)
+{
+ switch (type) {
+ case hwmon_temp:
+ *str = label_temps[channel];
+ break;
+ default:
+ return -EOPNOTSUPP;
+ }
+
+ return 0;
+}
+
+static const struct hwmon_ops farbwerk360_hwmon_ops = {
+ .is_visible = farbwerk360_is_visible,
+ .read = farbwerk360_read,
+ .read_string = farbwerk360_read_string,
+};
+
+static const struct hwmon_channel_info *farbwerk360_info[] = {
+ HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT | HWMON_T_LABEL, HWMON_T_INPUT | HWMON_T_LABEL,
+ HWMON_T_INPUT | HWMON_T_LABEL, HWMON_T_INPUT | HWMON_T_LABEL),
+ NULL
+};
+
+static const struct hwmon_chip_info farbwerk360_chip_info = {
+ .ops = &farbwerk360_hwmon_ops,
+ .info = farbwerk360_info,
+};
+
+static int farbwerk360_raw_event(struct hid_device *hdev, struct hid_report *report, u8 *data,
+ int size)
+{
+ int i, sensor_value;
+ struct farbwerk360_data *priv;
+
+ if (report->id != STATUS_REPORT_ID)
+ return 0;
+
+ priv = hid_get_drvdata(hdev);
+
+ /* Info provided with every report */
+ priv->serial_number[0] = get_unaligned_be16(data + SERIAL_FIRST_PART);
+ priv->serial_number[1] = get_unaligned_be16(data + SERIAL_SECOND_PART);
+
+ priv->firmware_version = get_unaligned_be16(data + FIRMWARE_VERSION);
+
+ /* Temperature sensor readings */
+ for (i = 0; i < NUM_SENSORS; i++) {
+ sensor_value = get_unaligned_be16(data + SENSOR_START + i * SENSOR_SIZE);
+ if (sensor_value == SENSOR_DISCONNECTED)
+ sensor_value = 0;
+
+ priv->temp_input[i] = sensor_value * 10;
+ }
+
+ priv->updated = jiffies;
+
+ return 0;
+}
+
+#ifdef CONFIG_DEBUG_FS
+
+static int serial_number_show(struct seq_file *seqf, void *unused)
+{
+ struct farbwerk360_data *priv = seqf->private;
+
+ seq_printf(seqf, "%05u-%05u\n", priv->serial_number[0], priv->serial_number[1]);
+
+ return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(serial_number);
+
+static int firmware_version_show(struct seq_file *seqf, void *unused)
+{
+ struct farbwerk360_data *priv = seqf->private;
+
+ seq_printf(seqf, "%u\n", priv->firmware_version);
+
+ return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(firmware_version);
+
+static void farbwerk360_debugfs_init(struct farbwerk360_data *priv)
+{
+ char name[32];
+
+ scnprintf(name, sizeof(name), "%s-%s", DRIVER_NAME, dev_name(&priv->hdev->dev));
+
+ priv->debugfs = debugfs_create_dir(name, NULL);
+ debugfs_create_file("serial_number", 0444, priv->debugfs, priv, &serial_number_fops);
+ debugfs_create_file("firmware_version", 0444, priv->debugfs, priv, &firmware_version_fops);
+}
+
+#else
+
+static void farbwerk360_debugfs_init(struct farbwerk360_data *priv)
+{
+}
+
+#endif
+
+static int farbwerk360_probe(struct hid_device *hdev, const struct hid_device_id *id)
+{
+ int ret;
+ struct farbwerk360_data *priv;
+
+ priv = devm_kzalloc(&hdev->dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->hdev = hdev;
+ hid_set_drvdata(hdev, priv);
+
+ priv->updated = jiffies - STATUS_UPDATE_INTERVAL;
+
+ ret = hid_parse(hdev);
+ if (ret)
+ return ret;
+
+ ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
+ if (ret)
+ return ret;
+
+ ret = hid_hw_open(hdev);
+ if (ret)
+ goto fail_and_stop;
+
+ priv->hwmon_dev = hwmon_device_register_with_info(&hdev->dev, "farbwerk360", priv,
+ &farbwerk360_chip_info, NULL);
+
+ if (IS_ERR(priv->hwmon_dev)) {
+ ret = PTR_ERR(priv->hwmon_dev);
+ goto fail_and_close;
+ }
+
+ farbwerk360_debugfs_init(priv);
+
+ return 0;
+
+fail_and_close:
+ hid_hw_close(hdev);
+fail_and_stop:
+ hid_hw_stop(hdev);
+ return ret;
+}
+
+static void farbwerk360_remove(struct hid_device *hdev)
+{
+ struct farbwerk360_data *priv = hid_get_drvdata(hdev);
+
+ debugfs_remove_recursive(priv->debugfs);
+ hwmon_device_unregister(priv->hwmon_dev);
+
+ hid_hw_close(hdev);
+ hid_hw_stop(hdev);
+}
+
+static const struct hid_device_id farbwerk360_table[] = {
+ { HID_USB_DEVICE(0x0c70, 0xf010) }, /* Aquacomputer Farbwerk 360 */
+ {},
+};
+
+MODULE_DEVICE_TABLE(hid, farbwerk360_table);
+
+static struct hid_driver farbwerk360_driver = {
+ .name = DRIVER_NAME,
+ .id_table = farbwerk360_table,
+ .probe = farbwerk360_probe,
+ .remove = farbwerk360_remove,
+ .raw_event = farbwerk360_raw_event,
+};
+
+static int __init farbwerk360_init(void)
+{
+ return hid_register_driver(&farbwerk360_driver);
+}
+
+static void __exit farbwerk360_exit(void)
+{
+ hid_unregister_driver(&farbwerk360_driver);
+}
+
+/* Request to initialize after the HID bus to ensure it's not being loaded before */
+late_initcall(farbwerk360_init);
+module_exit(farbwerk360_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Aleksa Savic <[email protected]>");
+MODULE_DESCRIPTION("Hwmon driver for Aquacomputer Farbwerk 360");
--
2.35.1


2022-02-24 21:24:36

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH] hwmon: add driver for Aquacomputer Farbwerk 360

Hi,

On 2/21/22 02:21, Aleksa Savic wrote:
> This driver exposes hardware temperature sensors of the Aquacomputer
> Farbwerk 360 RGB controller, which communicates through a proprietary
> USB HID protocol.
>
> Four temperature sensors are available. If a sensor is not connected,
> it will report zeroes. Additionally, serial number and firmware version
> are exposed through debugfs.
>
> This driver has been tested on x86_64.
>
> Signed-off-by: Aleksa Savic <[email protected]>

Unless I amm missing something, this driver is quite similar to the
d5next driver (drivers/hwmon/aquacomputer_d5next.c), except that there
are 4 instead of 1 temperature sensor, the sensor data is in a different place,
and there is _only_ temperature data. It should be quite straightforward
to merge the two drivers into one, so please do that.

Additional comment inline.

Thanks,
Guenter

> ---
> .../hwmon/aquacomputer_farbwerk360.rst | 40 +++
> Documentation/hwmon/index.rst | 1 +
> MAINTAINERS | 7 +
> drivers/hwmon/Kconfig | 10 +
> drivers/hwmon/Makefile | 1 +
> drivers/hwmon/aquacomputer_farbwerk360.c | 261 ++++++++++++++++++
> 6 files changed, 320 insertions(+)
> create mode 100644 Documentation/hwmon/aquacomputer_farbwerk360.rst
> create mode 100644 drivers/hwmon/aquacomputer_farbwerk360.c
>
> diff --git a/Documentation/hwmon/aquacomputer_farbwerk360.rst b/Documentation/hwmon/aquacomputer_farbwerk360.rst
> new file mode 100644
> index 000000000000..cebaffccd818
> --- /dev/null
> +++ b/Documentation/hwmon/aquacomputer_farbwerk360.rst
> @@ -0,0 +1,40 @@
> +.. SPDX-License-Identifier: GPL-2.0-or-later
> +
> +Kernel driver aquacomputer_farbwerk360
> +======================================
> +
> +Supported devices:
> +
> +* Aquacomputer Farbwerk 360 RGB controller
> +
> +Author: Aleksa Savic
> +
> +Description
> +-----------
> +
> +This driver exposes hardware temperature sensors of the Aquacomputer Farbwerk 360
> +RGB controller, which communicates through a proprietary USB HID protocol.
> +
> +Four temperature sensors are available. If a sensor is not connected, it will report
> +zeroes. Additionally, serial number and firmware version are exposed through debugfs.
> +
> +Usage notes
> +-----------
> +
> +Farbwerk 360 communicates via HID reports. The driver is loaded automatically by
> +the kernel and supports hotswapping.
> +
> +Sysfs entries
> +-------------
> +
> +=============== ==============================================
> +temp[1-4]_input Measured temperature (in millidegrees Celsius)
> +=============== ==============================================
> +
> +Debugfs entries
> +---------------
> +
> +================ ===============================================
> +serial_number Serial number of the pump
> +firmware_version Version of installed firmware
> +================ ===============================================
> diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
> index df20022c741f..42b7369340ba 100644
> --- a/Documentation/hwmon/index.rst
> +++ b/Documentation/hwmon/index.rst
> @@ -40,6 +40,7 @@ Hardware Monitoring Kernel Drivers
> aht10
> amc6821
> aquacomputer_d5next
> + aquacomputer_farbwerk360
> asb100
> asc7621
> aspeed-pwm-tacho
> diff --git a/MAINTAINERS b/MAINTAINERS
> index bd86ed9fbc79..fb8b8d7aebbc 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1386,6 +1386,13 @@ S: Maintained
> F: Documentation/hwmon/aquacomputer_d5next.rst
> F: drivers/hwmon/aquacomputer_d5next.c
>
> +AQUACOMPUTER FARBWERK 360 RGB CONTROLLER SENSOR DRIVER
> +M: Aleksa Savic <[email protected]>
> +L: [email protected]
> +S: Maintained
> +F: Documentation/hwmon/aquacomputer_farbwerk360.rst
> +F: drivers/hwmon/aquacomputer_farbwerk360.c
> +
> AQUANTIA ETHERNET DRIVER (atlantic)
> M: Igor Russkikh <[email protected]>
> L: [email protected]
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 8df25f1079ba..e1ca5e1e6ab0 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -264,6 +264,16 @@ config SENSORS_AQUACOMPUTER_D5NEXT
> This driver can also be built as a module. If so, the module
> will be called aquacomputer_d5next.
>
> +config SENSORS_AQUACOMPUTER_FARBWERK360
> + tristate "Aquacomputer Farbwerk 360 RGB controller"
> + depends on USB_HID
> + help
> + If you say yes here you get support for temperature sensors provided by
> + the Aquacomputer Farbwerk 360 RGB controller.
> +
> + This driver can also be built as a module. If so, the module
> + will be called aquacomputer_farbwerk360.
> +
> config SENSORS_AS370
> tristate "Synaptics AS370 SoC hardware monitoring driver"
> help
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index 185f946d698b..1c1556a53f6d 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -49,6 +49,7 @@ obj-$(CONFIG_SENSORS_ADT7475) += adt7475.o
> obj-$(CONFIG_SENSORS_AHT10) += aht10.o
> obj-$(CONFIG_SENSORS_APPLESMC) += applesmc.o
> obj-$(CONFIG_SENSORS_AQUACOMPUTER_D5NEXT) += aquacomputer_d5next.o
> +obj-$(CONFIG_SENSORS_AQUACOMPUTER_FARBWERK360) += aquacomputer_farbwerk360.o
> obj-$(CONFIG_SENSORS_ARM_SCMI) += scmi-hwmon.o
> obj-$(CONFIG_SENSORS_ARM_SCPI) += scpi-hwmon.o
> obj-$(CONFIG_SENSORS_AS370) += as370-hwmon.o
> diff --git a/drivers/hwmon/aquacomputer_farbwerk360.c b/drivers/hwmon/aquacomputer_farbwerk360.c
> new file mode 100644
> index 000000000000..14b760a2c8a8
> --- /dev/null
> +++ b/drivers/hwmon/aquacomputer_farbwerk360.c
> @@ -0,0 +1,261 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * hwmon driver for Aquacomputer Farbwerk 360 (RGB controller)
> + *
> + * The Farbwerk 360 sends HID reports (with ID 0x01) every second to report sensor values
> + * of up to four connected temperature sensors.
> + *
> + * Copyright 2022 Aleksa Savic <[email protected]>
> + */
> +
> +#include <asm/unaligned.h>

asm includes should be last (ah, my bad, this is from the d5 driver.
but still, just for the future).

> +#include <linux/debugfs.h>
> +#include <linux/hid.h>
> +#include <linux/hwmon.h>
> +#include <linux/jiffies.h>
> +#include <linux/module.h>
> +#include <linux/seq_file.h>
> +
> +#define DRIVER_NAME "aquacomputer_farbwerk360"
> +
> +#define STATUS_REPORT_ID 0x01
> +#define STATUS_UPDATE_INTERVAL (2 * HZ) /* In seconds */
> +
> +/* Register offsets */
> +#define SERIAL_FIRST_PART 0x03
> +#define SERIAL_SECOND_PART 0x05
> +#define FIRMWARE_VERSION 0x0D
> +
> +#define NUM_SENSORS 4
> +#define SENSOR_START 0x32
> +#define SENSOR_SIZE 0x02
> +#define SENSOR_DISCONNECTED 0x7FFF
> +
> +static const char *const label_temps[] = { "Sensor 1", "Sensor 2", "Sensor 3", "Sensor 4" };
> +
> +struct farbwerk360_data {
> + struct hid_device *hdev;
> + struct device *hwmon_dev;
> + struct dentry *debugfs;
> + s32 temp_input[4];
> + u32 serial_number[2];
> + u16 firmware_version;
> + unsigned long updated;
> +};
> +
> +static umode_t farbwerk360_is_visible(const void *data, enum hwmon_sensor_types type, u32 attr,
> + int channel)
> +{
> + return 0444;
> +}
> +
> +static int farbwerk360_read(struct device *dev, enum hwmon_sensor_types type, u32 attr, int channel,
> + long *val)
> +{
> + struct farbwerk360_data *priv = dev_get_drvdata(dev);
> +
> + if (time_after(jiffies, priv->updated + STATUS_UPDATE_INTERVAL))
> + return -ENODATA;
> +
> + switch (type) {
> + case hwmon_temp:
> + *val = priv->temp_input[channel];
> + break;
> + default:
> + return -EOPNOTSUPP;
> + }
> +
> + return 0;
> +}
> +
> +static int farbwerk360_read_string(struct device *dev, enum hwmon_sensor_types type, u32 attr,
> + int channel, const char **str)
> +{
> + switch (type) {
> + case hwmon_temp:
> + *str = label_temps[channel];
> + break;
> + default:
> + return -EOPNOTSUPP;
> + }
> +
> + return 0;
> +}
> +
> +static const struct hwmon_ops farbwerk360_hwmon_ops = {
> + .is_visible = farbwerk360_is_visible,
> + .read = farbwerk360_read,
> + .read_string = farbwerk360_read_string,
> +};
> +
> +static const struct hwmon_channel_info *farbwerk360_info[] = {
> + HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT | HWMON_T_LABEL, HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_INPUT | HWMON_T_LABEL, HWMON_T_INPUT | HWMON_T_LABEL),
> + NULL
> +};
> +
> +static const struct hwmon_chip_info farbwerk360_chip_info = {
> + .ops = &farbwerk360_hwmon_ops,
> + .info = farbwerk360_info,
> +};
> +
> +static int farbwerk360_raw_event(struct hid_device *hdev, struct hid_report *report, u8 *data,
> + int size)
> +{
> + int i, sensor_value;
> + struct farbwerk360_data *priv;
> +
> + if (report->id != STATUS_REPORT_ID)
> + return 0;
> +
> + priv = hid_get_drvdata(hdev);
> +
> + /* Info provided with every report */
> + priv->serial_number[0] = get_unaligned_be16(data + SERIAL_FIRST_PART);
> + priv->serial_number[1] = get_unaligned_be16(data + SERIAL_SECOND_PART);
> +
> + priv->firmware_version = get_unaligned_be16(data + FIRMWARE_VERSION);
> +
> + /* Temperature sensor readings */
> + for (i = 0; i < NUM_SENSORS; i++) {
> + sensor_value = get_unaligned_be16(data + SENSOR_START + i * SENSOR_SIZE);
> + if (sensor_value == SENSOR_DISCONNECTED)
> + sensor_value = 0;

If it is known that a sensor is disconnected, it would make
more sense to report -ENODATA when it is read instead of a
temperature of 0 degrees.

> +
> + priv->temp_input[i] = sensor_value * 10;
> + }
> +
> + priv->updated = jiffies;
> +
> + return 0;
> +}
> +
> +#ifdef CONFIG_DEBUG_FS
> +
> +static int serial_number_show(struct seq_file *seqf, void *unused)
> +{
> + struct farbwerk360_data *priv = seqf->private;
> +
> + seq_printf(seqf, "%05u-%05u\n", priv->serial_number[0], priv->serial_number[1]);
> +
> + return 0;
> +}
> +DEFINE_SHOW_ATTRIBUTE(serial_number);
> +
> +static int firmware_version_show(struct seq_file *seqf, void *unused)
> +{
> + struct farbwerk360_data *priv = seqf->private;
> +
> + seq_printf(seqf, "%u\n", priv->firmware_version);
> +
> + return 0;
> +}
> +DEFINE_SHOW_ATTRIBUTE(firmware_version);
> +
> +static void farbwerk360_debugfs_init(struct farbwerk360_data *priv)
> +{
> + char name[32];
> +
> + scnprintf(name, sizeof(name), "%s-%s", DRIVER_NAME, dev_name(&priv->hdev->dev));
> +
> + priv->debugfs = debugfs_create_dir(name, NULL);
> + debugfs_create_file("serial_number", 0444, priv->debugfs, priv, &serial_number_fops);
> + debugfs_create_file("firmware_version", 0444, priv->debugfs, priv, &firmware_version_fops);
> +}
> +
> +#else
> +
> +static void farbwerk360_debugfs_init(struct farbwerk360_data *priv)
> +{
> +}
> +
> +#endif
> +
> +static int farbwerk360_probe(struct hid_device *hdev, const struct hid_device_id *id)
> +{
> + int ret;
> + struct farbwerk360_data *priv;
> +
> + priv = devm_kzalloc(&hdev->dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + priv->hdev = hdev;
> + hid_set_drvdata(hdev, priv);
> +
> + priv->updated = jiffies - STATUS_UPDATE_INTERVAL;
> +
> + ret = hid_parse(hdev);
> + if (ret)
> + return ret;
> +
> + ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
> + if (ret)
> + return ret;
> +
> + ret = hid_hw_open(hdev);
> + if (ret)
> + goto fail_and_stop;
> +
> + priv->hwmon_dev = hwmon_device_register_with_info(&hdev->dev, "farbwerk360", priv,
> + &farbwerk360_chip_info, NULL);
> +
> + if (IS_ERR(priv->hwmon_dev)) {
> + ret = PTR_ERR(priv->hwmon_dev);
> + goto fail_and_close;
> + }
> +
> + farbwerk360_debugfs_init(priv);
> +
> + return 0;
> +
> +fail_and_close:
> + hid_hw_close(hdev);
> +fail_and_stop:
> + hid_hw_stop(hdev);
> + return ret;
> +}
> +
> +static void farbwerk360_remove(struct hid_device *hdev)
> +{
> + struct farbwerk360_data *priv = hid_get_drvdata(hdev);
> +
> + debugfs_remove_recursive(priv->debugfs);
> + hwmon_device_unregister(priv->hwmon_dev);
> +
> + hid_hw_close(hdev);
> + hid_hw_stop(hdev);
> +}
> +
> +static const struct hid_device_id farbwerk360_table[] = {
> + { HID_USB_DEVICE(0x0c70, 0xf010) }, /* Aquacomputer Farbwerk 360 */
> + {},
> +};
> +
> +MODULE_DEVICE_TABLE(hid, farbwerk360_table);
> +
> +static struct hid_driver farbwerk360_driver = {
> + .name = DRIVER_NAME,
> + .id_table = farbwerk360_table,
> + .probe = farbwerk360_probe,
> + .remove = farbwerk360_remove,
> + .raw_event = farbwerk360_raw_event,
> +};
> +
> +static int __init farbwerk360_init(void)
> +{
> + return hid_register_driver(&farbwerk360_driver);
> +}
> +
> +static void __exit farbwerk360_exit(void)
> +{
> + hid_unregister_driver(&farbwerk360_driver);
> +}
> +
> +/* Request to initialize after the HID bus to ensure it's not being loaded before */
> +late_initcall(farbwerk360_init);
> +module_exit(farbwerk360_exit);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Aleksa Savic <[email protected]>");
> +MODULE_DESCRIPTION("Hwmon driver for Aquacomputer Farbwerk 360");

2022-02-26 17:03:05

by Aleksa Savic

[permalink] [raw]
Subject: Re: [PATCH] hwmon: add driver for Aquacomputer Farbwerk 360

Hi,

On Thu, Feb 24, 2022 at 11:57:47AM -0800, Guenter Roeck wrote:
> Hi,
>
> On 2/21/22 02:21, Aleksa Savic wrote:
> > This driver exposes hardware temperature sensors of the Aquacomputer
> > Farbwerk 360 RGB controller, which communicates through a proprietary
> > USB HID protocol.
> >
> > Four temperature sensors are available. If a sensor is not connected,
> > it will report zeroes. Additionally, serial number and firmware version
> > are exposed through debugfs.
> >
> > This driver has been tested on x86_64.
> >
> > Signed-off-by: Aleksa Savic <[email protected]>
>
> Unless I amm missing something, this driver is quite similar to the
> d5next driver (drivers/hwmon/aquacomputer_d5next.c), except that there
> are 4 instead of 1 temperature sensor, the sensor data is in a different place,
> and there is _only_ temperature data. It should be quite straightforward
> to merge the two drivers into one, so please do that.
>
> Additional comment inline.
>
> Thanks,
> Guenter

Yes, it's very similar, I based this one on that. I'll send a patch for
the original driver. I also have code ready for Aquacomputer Octo (8 fans, 4
temperature sensors, very similar to all this as well) with PWM write control
support for the fans. And there is also WIP code for PWM and curve support for
the D5 Next from contributors on my github repo, though we're working on that.

Considering that the driver would then contain support for more devices (the D5
Next and the Farbwerk 360 for now), can it be renamed to just aquacomputer or
something similar?

For the inline comments, understood, I'll resolve them in the next patch.

Thanks,
Aleksa

> > ---
> > .../hwmon/aquacomputer_farbwerk360.rst | 40 +++
> > Documentation/hwmon/index.rst | 1 +
> > MAINTAINERS | 7 +
> > drivers/hwmon/Kconfig | 10 +
> > drivers/hwmon/Makefile | 1 +
> > drivers/hwmon/aquacomputer_farbwerk360.c | 261 ++++++++++++++++++
> > 6 files changed, 320 insertions(+)
> > create mode 100644 Documentation/hwmon/aquacomputer_farbwerk360.rst
> > create mode 100644 drivers/hwmon/aquacomputer_farbwerk360.c
> >
> > diff --git a/Documentation/hwmon/aquacomputer_farbwerk360.rst b/Documentation/hwmon/aquacomputer_farbwerk360.rst
> > new file mode 100644
> > index 000000000000..cebaffccd818
> > --- /dev/null
> > +++ b/Documentation/hwmon/aquacomputer_farbwerk360.rst
> > @@ -0,0 +1,40 @@
> > +.. SPDX-License-Identifier: GPL-2.0-or-later
> > +
> > +Kernel driver aquacomputer_farbwerk360
> > +======================================
> > +
> > +Supported devices:
> > +
> > +* Aquacomputer Farbwerk 360 RGB controller
> > +
> > +Author: Aleksa Savic
> > +
> > +Description
> > +-----------
> > +
> > +This driver exposes hardware temperature sensors of the Aquacomputer Farbwerk 360
> > +RGB controller, which communicates through a proprietary USB HID protocol.
> > +
> > +Four temperature sensors are available. If a sensor is not connected, it will report
> > +zeroes. Additionally, serial number and firmware version are exposed through debugfs.
> > +
> > +Usage notes
> > +-----------
> > +
> > +Farbwerk 360 communicates via HID reports. The driver is loaded automatically by
> > +the kernel and supports hotswapping.
> > +
> > +Sysfs entries
> > +-------------
> > +
> > +=============== ==============================================
> > +temp[1-4]_input Measured temperature (in millidegrees Celsius)
> > +=============== ==============================================
> > +
> > +Debugfs entries
> > +---------------
> > +
> > +================ ===============================================
> > +serial_number Serial number of the pump
> > +firmware_version Version of installed firmware
> > +================ ===============================================
> > diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
> > index df20022c741f..42b7369340ba 100644
> > --- a/Documentation/hwmon/index.rst
> > +++ b/Documentation/hwmon/index.rst
> > @@ -40,6 +40,7 @@ Hardware Monitoring Kernel Drivers
> > aht10
> > amc6821
> > aquacomputer_d5next
> > + aquacomputer_farbwerk360
> > asb100
> > asc7621
> > aspeed-pwm-tacho
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index bd86ed9fbc79..fb8b8d7aebbc 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -1386,6 +1386,13 @@ S: Maintained
> > F: Documentation/hwmon/aquacomputer_d5next.rst
> > F: drivers/hwmon/aquacomputer_d5next.c
> > +AQUACOMPUTER FARBWERK 360 RGB CONTROLLER SENSOR DRIVER
> > +M: Aleksa Savic <[email protected]>
> > +L: [email protected]
> > +S: Maintained
> > +F: Documentation/hwmon/aquacomputer_farbwerk360.rst
> > +F: drivers/hwmon/aquacomputer_farbwerk360.c
> > +
> > AQUANTIA ETHERNET DRIVER (atlantic)
> > M: Igor Russkikh <[email protected]>
> > L: [email protected]
> > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> > index 8df25f1079ba..e1ca5e1e6ab0 100644
> > --- a/drivers/hwmon/Kconfig
> > +++ b/drivers/hwmon/Kconfig
> > @@ -264,6 +264,16 @@ config SENSORS_AQUACOMPUTER_D5NEXT
> > This driver can also be built as a module. If so, the module
> > will be called aquacomputer_d5next.
> > +config SENSORS_AQUACOMPUTER_FARBWERK360
> > + tristate "Aquacomputer Farbwerk 360 RGB controller"
> > + depends on USB_HID
> > + help
> > + If you say yes here you get support for temperature sensors provided by
> > + the Aquacomputer Farbwerk 360 RGB controller.
> > +
> > + This driver can also be built as a module. If so, the module
> > + will be called aquacomputer_farbwerk360.
> > +
> > config SENSORS_AS370
> > tristate "Synaptics AS370 SoC hardware monitoring driver"
> > help
> > diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> > index 185f946d698b..1c1556a53f6d 100644
> > --- a/drivers/hwmon/Makefile
> > +++ b/drivers/hwmon/Makefile
> > @@ -49,6 +49,7 @@ obj-$(CONFIG_SENSORS_ADT7475) += adt7475.o
> > obj-$(CONFIG_SENSORS_AHT10) += aht10.o
> > obj-$(CONFIG_SENSORS_APPLESMC) += applesmc.o
> > obj-$(CONFIG_SENSORS_AQUACOMPUTER_D5NEXT) += aquacomputer_d5next.o
> > +obj-$(CONFIG_SENSORS_AQUACOMPUTER_FARBWERK360) += aquacomputer_farbwerk360.o
> > obj-$(CONFIG_SENSORS_ARM_SCMI) += scmi-hwmon.o
> > obj-$(CONFIG_SENSORS_ARM_SCPI) += scpi-hwmon.o
> > obj-$(CONFIG_SENSORS_AS370) += as370-hwmon.o
> > diff --git a/drivers/hwmon/aquacomputer_farbwerk360.c b/drivers/hwmon/aquacomputer_farbwerk360.c
> > new file mode 100644
> > index 000000000000..14b760a2c8a8
> > --- /dev/null
> > +++ b/drivers/hwmon/aquacomputer_farbwerk360.c
> > @@ -0,0 +1,261 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * hwmon driver for Aquacomputer Farbwerk 360 (RGB controller)
> > + *
> > + * The Farbwerk 360 sends HID reports (with ID 0x01) every second to report sensor values
> > + * of up to four connected temperature sensors.
> > + *
> > + * Copyright 2022 Aleksa Savic <[email protected]>
> > + */
> > +
> > +#include <asm/unaligned.h>
>
> asm includes should be last (ah, my bad, this is from the d5 driver.
> but still, just for the future).
>
> > +#include <linux/debugfs.h>
> > +#include <linux/hid.h>
> > +#include <linux/hwmon.h>
> > +#include <linux/jiffies.h>
> > +#include <linux/module.h>
> > +#include <linux/seq_file.h>
> > +
> > +#define DRIVER_NAME "aquacomputer_farbwerk360"
> > +
> > +#define STATUS_REPORT_ID 0x01
> > +#define STATUS_UPDATE_INTERVAL (2 * HZ) /* In seconds */
> > +
> > +/* Register offsets */
> > +#define SERIAL_FIRST_PART 0x03
> > +#define SERIAL_SECOND_PART 0x05
> > +#define FIRMWARE_VERSION 0x0D
> > +
> > +#define NUM_SENSORS 4
> > +#define SENSOR_START 0x32
> > +#define SENSOR_SIZE 0x02
> > +#define SENSOR_DISCONNECTED 0x7FFF
> > +
> > +static const char *const label_temps[] = { "Sensor 1", "Sensor 2", "Sensor 3", "Sensor 4" };
> > +
> > +struct farbwerk360_data {
> > + struct hid_device *hdev;
> > + struct device *hwmon_dev;
> > + struct dentry *debugfs;
> > + s32 temp_input[4];
> > + u32 serial_number[2];
> > + u16 firmware_version;
> > + unsigned long updated;
> > +};
> > +
> > +static umode_t farbwerk360_is_visible(const void *data, enum hwmon_sensor_types type, u32 attr,
> > + int channel)
> > +{
> > + return 0444;
> > +}
> > +
> > +static int farbwerk360_read(struct device *dev, enum hwmon_sensor_types type, u32 attr, int channel,
> > + long *val)
> > +{
> > + struct farbwerk360_data *priv = dev_get_drvdata(dev);
> > +
> > + if (time_after(jiffies, priv->updated + STATUS_UPDATE_INTERVAL))
> > + return -ENODATA;
> > +
> > + switch (type) {
> > + case hwmon_temp:
> > + *val = priv->temp_input[channel];
> > + break;
> > + default:
> > + return -EOPNOTSUPP;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int farbwerk360_read_string(struct device *dev, enum hwmon_sensor_types type, u32 attr,
> > + int channel, const char **str)
> > +{
> > + switch (type) {
> > + case hwmon_temp:
> > + *str = label_temps[channel];
> > + break;
> > + default:
> > + return -EOPNOTSUPP;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static const struct hwmon_ops farbwerk360_hwmon_ops = {
> > + .is_visible = farbwerk360_is_visible,
> > + .read = farbwerk360_read,
> > + .read_string = farbwerk360_read_string,
> > +};
> > +
> > +static const struct hwmon_channel_info *farbwerk360_info[] = {
> > + HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT | HWMON_T_LABEL, HWMON_T_INPUT | HWMON_T_LABEL,
> > + HWMON_T_INPUT | HWMON_T_LABEL, HWMON_T_INPUT | HWMON_T_LABEL),
> > + NULL
> > +};
> > +
> > +static const struct hwmon_chip_info farbwerk360_chip_info = {
> > + .ops = &farbwerk360_hwmon_ops,
> > + .info = farbwerk360_info,
> > +};
> > +
> > +static int farbwerk360_raw_event(struct hid_device *hdev, struct hid_report *report, u8 *data,
> > + int size)
> > +{
> > + int i, sensor_value;
> > + struct farbwerk360_data *priv;
> > +
> > + if (report->id != STATUS_REPORT_ID)
> > + return 0;
> > +
> > + priv = hid_get_drvdata(hdev);
> > +
> > + /* Info provided with every report */
> > + priv->serial_number[0] = get_unaligned_be16(data + SERIAL_FIRST_PART);
> > + priv->serial_number[1] = get_unaligned_be16(data + SERIAL_SECOND_PART);
> > +
> > + priv->firmware_version = get_unaligned_be16(data + FIRMWARE_VERSION);
> > +
> > + /* Temperature sensor readings */
> > + for (i = 0; i < NUM_SENSORS; i++) {
> > + sensor_value = get_unaligned_be16(data + SENSOR_START + i * SENSOR_SIZE);
> > + if (sensor_value == SENSOR_DISCONNECTED)
> > + sensor_value = 0;
>
> If it is known that a sensor is disconnected, it would make
> more sense to report -ENODATA when it is read instead of a
> temperature of 0 degrees.
>
> > +
> > + priv->temp_input[i] = sensor_value * 10;
> > + }
> > +
> > + priv->updated = jiffies;
> > +
> > + return 0;
> > +}
> > +
> > +#ifdef CONFIG_DEBUG_FS
> > +
> > +static int serial_number_show(struct seq_file *seqf, void *unused)
> > +{
> > + struct farbwerk360_data *priv = seqf->private;
> > +
> > + seq_printf(seqf, "%05u-%05u\n", priv->serial_number[0], priv->serial_number[1]);
> > +
> > + return 0;
> > +}
> > +DEFINE_SHOW_ATTRIBUTE(serial_number);
> > +
> > +static int firmware_version_show(struct seq_file *seqf, void *unused)
> > +{
> > + struct farbwerk360_data *priv = seqf->private;
> > +
> > + seq_printf(seqf, "%u\n", priv->firmware_version);
> > +
> > + return 0;
> > +}
> > +DEFINE_SHOW_ATTRIBUTE(firmware_version);
> > +
> > +static void farbwerk360_debugfs_init(struct farbwerk360_data *priv)
> > +{
> > + char name[32];
> > +
> > + scnprintf(name, sizeof(name), "%s-%s", DRIVER_NAME, dev_name(&priv->hdev->dev));
> > +
> > + priv->debugfs = debugfs_create_dir(name, NULL);
> > + debugfs_create_file("serial_number", 0444, priv->debugfs, priv, &serial_number_fops);
> > + debugfs_create_file("firmware_version", 0444, priv->debugfs, priv, &firmware_version_fops);
> > +}
> > +
> > +#else
> > +
> > +static void farbwerk360_debugfs_init(struct farbwerk360_data *priv)
> > +{
> > +}
> > +
> > +#endif
> > +
> > +static int farbwerk360_probe(struct hid_device *hdev, const struct hid_device_id *id)
> > +{
> > + int ret;
> > + struct farbwerk360_data *priv;
> > +
> > + priv = devm_kzalloc(&hdev->dev, sizeof(*priv), GFP_KERNEL);
> > + if (!priv)
> > + return -ENOMEM;
> > +
> > + priv->hdev = hdev;
> > + hid_set_drvdata(hdev, priv);
> > +
> > + priv->updated = jiffies - STATUS_UPDATE_INTERVAL;
> > +
> > + ret = hid_parse(hdev);
> > + if (ret)
> > + return ret;
> > +
> > + ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
> > + if (ret)
> > + return ret;
> > +
> > + ret = hid_hw_open(hdev);
> > + if (ret)
> > + goto fail_and_stop;
> > +
> > + priv->hwmon_dev = hwmon_device_register_with_info(&hdev->dev, "farbwerk360", priv,
> > + &farbwerk360_chip_info, NULL);
> > +
> > + if (IS_ERR(priv->hwmon_dev)) {
> > + ret = PTR_ERR(priv->hwmon_dev);
> > + goto fail_and_close;
> > + }
> > +
> > + farbwerk360_debugfs_init(priv);
> > +
> > + return 0;
> > +
> > +fail_and_close:
> > + hid_hw_close(hdev);
> > +fail_and_stop:
> > + hid_hw_stop(hdev);
> > + return ret;
> > +}
> > +
> > +static void farbwerk360_remove(struct hid_device *hdev)
> > +{
> > + struct farbwerk360_data *priv = hid_get_drvdata(hdev);
> > +
> > + debugfs_remove_recursive(priv->debugfs);
> > + hwmon_device_unregister(priv->hwmon_dev);
> > +
> > + hid_hw_close(hdev);
> > + hid_hw_stop(hdev);
> > +}
> > +
> > +static const struct hid_device_id farbwerk360_table[] = {
> > + { HID_USB_DEVICE(0x0c70, 0xf010) }, /* Aquacomputer Farbwerk 360 */
> > + {},
> > +};
> > +
> > +MODULE_DEVICE_TABLE(hid, farbwerk360_table);
> > +
> > +static struct hid_driver farbwerk360_driver = {
> > + .name = DRIVER_NAME,
> > + .id_table = farbwerk360_table,
> > + .probe = farbwerk360_probe,
> > + .remove = farbwerk360_remove,
> > + .raw_event = farbwerk360_raw_event,
> > +};
> > +
> > +static int __init farbwerk360_init(void)
> > +{
> > + return hid_register_driver(&farbwerk360_driver);
> > +}
> > +
> > +static void __exit farbwerk360_exit(void)
> > +{
> > + hid_unregister_driver(&farbwerk360_driver);
> > +}
> > +
> > +/* Request to initialize after the HID bus to ensure it's not being loaded before */
> > +late_initcall(farbwerk360_init);
> > +module_exit(farbwerk360_exit);
> > +
> > +MODULE_LICENSE("GPL");
> > +MODULE_AUTHOR("Aleksa Savic <[email protected]>");
> > +MODULE_DESCRIPTION("Hwmon driver for Aquacomputer Farbwerk 360");
>

2022-02-26 17:51:04

by Aleksa Savic

[permalink] [raw]
Subject: Re: [PATCH] hwmon: add driver for Aquacomputer Farbwerk 360

On Sat, Feb 26, 2022 at 07:31:34AM -0800, Guenter Roeck wrote:
> On 2/26/22 07:09, Aleksa Savic wrote:
> > Hi,
> >
> > On Thu, Feb 24, 2022 at 11:57:47AM -0800, Guenter Roeck wrote:
> > > Hi,
> > >
> > > On 2/21/22 02:21, Aleksa Savic wrote:
> > > > This driver exposes hardware temperature sensors of the Aquacomputer
> > > > Farbwerk 360 RGB controller, which communicates through a proprietary
> > > > USB HID protocol.
> > > >
> > > > Four temperature sensors are available. If a sensor is not connected,
> > > > it will report zeroes. Additionally, serial number and firmware version
> > > > are exposed through debugfs.
> > > >
> > > > This driver has been tested on x86_64.
> > > >
> > > > Signed-off-by: Aleksa Savic <[email protected]>
> > >
> > > Unless I amm missing something, this driver is quite similar to the
> > > d5next driver (drivers/hwmon/aquacomputer_d5next.c), except that there
> > > are 4 instead of 1 temperature sensor, the sensor data is in a different place,
> > > and there is _only_ temperature data. It should be quite straightforward
> > > to merge the two drivers into one, so please do that.
> > >
> > > Additional comment inline.
> > >
> > > Thanks,
> > > Guenter
> >
> > Yes, it's very similar, I based this one on that. I'll send a patch for
> > the original driver. I also have code ready for Aquacomputer Octo (8 fans, 4
> > temperature sensors, very similar to all this as well) with PWM write control
> > support for the fans. And there is also WIP code for PWM and curve support for
> > the D5 Next from contributors on my github repo, though we're working on that.
> >
> > Considering that the driver would then contain support for more devices (the D5
> > Next and the Farbwerk 360 for now), can it be renamed to just aquacomputer or
> > something similar?
> >
>
> We don't normally do that (because the Kconfig symbol is already established)
> and just name the driver for the first chip (or, in this case, controller)
> it supports.
>
> Guenter

Got it.

Thanks,
Aleksa

2022-02-26 18:43:05

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH] hwmon: add driver for Aquacomputer Farbwerk 360

On 2/26/22 07:09, Aleksa Savic wrote:
> Hi,
>
> On Thu, Feb 24, 2022 at 11:57:47AM -0800, Guenter Roeck wrote:
>> Hi,
>>
>> On 2/21/22 02:21, Aleksa Savic wrote:
>>> This driver exposes hardware temperature sensors of the Aquacomputer
>>> Farbwerk 360 RGB controller, which communicates through a proprietary
>>> USB HID protocol.
>>>
>>> Four temperature sensors are available. If a sensor is not connected,
>>> it will report zeroes. Additionally, serial number and firmware version
>>> are exposed through debugfs.
>>>
>>> This driver has been tested on x86_64.
>>>
>>> Signed-off-by: Aleksa Savic <[email protected]>
>>
>> Unless I amm missing something, this driver is quite similar to the
>> d5next driver (drivers/hwmon/aquacomputer_d5next.c), except that there
>> are 4 instead of 1 temperature sensor, the sensor data is in a different place,
>> and there is _only_ temperature data. It should be quite straightforward
>> to merge the two drivers into one, so please do that.
>>
>> Additional comment inline.
>>
>> Thanks,
>> Guenter
>
> Yes, it's very similar, I based this one on that. I'll send a patch for
> the original driver. I also have code ready for Aquacomputer Octo (8 fans, 4
> temperature sensors, very similar to all this as well) with PWM write control
> support for the fans. And there is also WIP code for PWM and curve support for
> the D5 Next from contributors on my github repo, though we're working on that.
>
> Considering that the driver would then contain support for more devices (the D5
> Next and the Farbwerk 360 for now), can it be renamed to just aquacomputer or
> something similar?
>

We don't normally do that (because the Kconfig symbol is already established)
and just name the driver for the first chip (or, in this case, controller)
it supports.

Guenter