linux-hwmon.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/6] skeleton for asetek gen 6 driver
@ 2020-07-13 19:32 jaap aarts
  2020-07-13 19:32 ` [PATCH 2/6] fan support " jaap aarts
                   ` (7 more replies)
  0 siblings, 8 replies; 12+ messages in thread
From: jaap aarts @ 2020-07-13 19:32 UTC (permalink / raw)
  To: Jean Delvare, Guenter Roeck, linux-hwmon, linux-usb
  Cc: jaap aarts, Jaap Aarts

Signed-off-by: Jaap Aarts <jaap.aarts1@example.com>
---
 drivers/hwmon/Kconfig       |   6 ++
 drivers/hwmon/Makefile      |   1 +
 drivers/hwmon/asetek_gen6.c | 186 ++++++++++++++++++++++++++++++++++++
 3 files changed, 193 insertions(+)
 create mode 100644 drivers/hwmon/asetek_gen6.c

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 288ae9f63588..521a9e0c88ca 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -378,6 +378,12 @@ config SENSORS_ARM_SCPI
 	  and power sensors available on ARM Ltd's SCP based platforms. The
 	  actual number and type of sensors exported depend on the platform.
 
+config SENSORS_ASETEK_GEN6
+	tristate "Asetek generation 6 driver"
+	help
+	  If you say yes here you get support for asetek generation 6 boards
+	  found on most AIO liquid coolers with an asetek pump.
+
 config SENSORS_ASB100
 	tristate "Asus ASB100 Bach"
 	depends on X86 && I2C
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 3e32c21f5efe..9683d2aae2b2 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -20,6 +20,7 @@ obj-$(CONFIG_SENSORS_W83793)	+= w83793.o
 obj-$(CONFIG_SENSORS_W83795)	+= w83795.o
 obj-$(CONFIG_SENSORS_W83781D)	+= w83781d.o
 obj-$(CONFIG_SENSORS_W83791D)	+= w83791d.o
+obj-$(CONFIG_SENSORS_ASETEK_GEN6)	+= asetek_gen6.o
 
 obj-$(CONFIG_SENSORS_AB8500)	+= abx500.o ab8500.o
 obj-$(CONFIG_SENSORS_ABITUGURU)	+= abituguru.o
diff --git a/drivers/hwmon/asetek_gen6.c b/drivers/hwmon/asetek_gen6.c
new file mode 100644
index 000000000000..4d530a4cb71d
--- /dev/null
+++ b/drivers/hwmon/asetek_gen6.c
@@ -0,0 +1,186 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/* 
+ * A hwmon driver for most asetek gen 6 all-in-one liquid coolers.
+ * Copyright (c) Jaap Aarts 2020
+ * 
+ * Protocol reverse engineered by audiohacked
+ * https://github.com/audiohacked/OpenCorsairLink
+ */
+
+/*
+ * Supports following chips: 
+ * driver h100i platinum
+ * 
+ * Other products should work with this driver but no testing has been done.
+ * 
+ * Note: platinum is the codename name for pro within driver
+ * 
+ * Note: fan curve controll has not been implemented
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/usb.h>
+#include <linux/slab.h>
+#include <linux/mutex.h>
+#include <linux/errno.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+
+struct driver {
+	struct usb_device *udev;
+
+	char *bulk_out_buffer;
+	char *bulk_in_buffer;
+	size_t bulk_out_size;
+	size_t bulk_in_size;
+	char bulk_in_endpointAddr;
+	char bulk_out_endpointAddr;
+
+	struct usb_interface *interface; /* the interface for this device */
+	struct mutex io_mutex; /* synchronize I/O with disconnect */
+	struct semaphore
+		limit_sem; /* limiting the number of writes in progress */
+
+	struct kref kref;
+};
+
+/* devices that work with this driver */
+static const struct usb_device_id astk_table[] = {
+	{ USB_DEVICE(0x1b1c, 0x0c15) },
+	{} /* Terminating entry */
+};
+
+MODULE_DEVICE_TABLE(usb, astk_table);
+
+int init_device(struct usb_device *udev)
+{
+	int retval;
+	retval = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), 0x00, 0x40,
+				 0xffff, 0x0000, 0, 0, 0);
+	//this always returns error
+	if (retval != 0)
+		;
+
+	retval = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), 0x02, 0x40,
+				 0x0002, 0x0000, 0, 0, 0);
+	if (retval != 0)
+		return retval;
+
+	return 0;
+}
+int deinit_device(struct usb_device *udev)
+{
+	int retval;
+	retval = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), 0x02, 0x40,
+				 0x0004, 0x0000, 0, 0, 0);
+	if (retval != 0)
+		return retval;
+
+	return 0;
+}
+
+static void astk_delete(struct kref *kref)
+{
+	struct driver *dev = container_of(kref, struct driver, kref);
+
+	usb_put_intf(dev->interface);
+	usb_put_dev(dev->udev);
+	kfree(dev->bulk_in_buffer);
+	kfree(dev->bulk_out_buffer);
+	kfree(dev);
+}
+
+static int astk_probe(struct usb_interface *interface,
+		      const struct usb_device_id *id)
+{
+	struct driver *dev;
+	int retval = 0;
+	struct usb_endpoint_descriptor *bulk_in, *bulk_out;
+
+	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
+	if (!dev) {
+		retval = -ENOMEM;
+		goto exit;
+	}
+
+	retval = usb_find_common_endpoints(interface->cur_altsetting, &bulk_in,
+					   &bulk_out, NULL, NULL);
+	if (retval != 0)
+		goto exit;
+
+	dev->udev = usb_get_dev(interface_to_usbdev(interface));
+	dev->interface = usb_get_intf(interface);
+
+	/* set up the endpoint information */
+	/* use only the first bulk-in and bulk-out endpoints */
+	dev->bulk_in_size = usb_endpoint_maxp(bulk_in);
+	dev->bulk_in_buffer = kmalloc(dev->bulk_in_size, GFP_KERNEL);
+	dev->bulk_in_endpointAddr = bulk_in->bEndpointAddress;
+	dev->bulk_out_size = usb_endpoint_maxp(bulk_out);
+	dev->bulk_out_buffer = kmalloc(dev->bulk_out_size, GFP_KERNEL);
+	dev->bulk_out_endpointAddr = bulk_out->bEndpointAddress;
+
+	//hwmon_init(dev);
+	retval = init_device(dev->udev);
+	if (retval) {
+		dev_err(&interface->dev, "Failled initialising this device.\n");
+		goto exit;
+	}
+
+	usb_set_intfdata(interface, dev);
+	kref_init(&dev->kref);
+	mutex_init(&dev->io_mutex);
+	sema_init(&dev->limit_sem, 8);
+exit:
+	return retval;
+}
+
+static void astk_disconnect(struct usb_interface *interface)
+{
+	struct driver *dev = usb_get_intfdata(interface);
+	//hwmon_deinit(dev);
+	usb_set_intfdata(interface, NULL);
+	kref_put(&dev->kref, astk_delete);
+	deinit_device(dev->udev);
+	/* let the user know what node this device is now attached to */
+}
+static int astk_resume(struct usb_interface *intf)
+{
+	return 0;
+}
+
+static int astk_suspend(struct usb_interface *intf, pm_message_t message)
+{
+	return 0;
+}
+
+static struct usb_driver astk_driver = {
+	.name = "Asetek gen6 driver",
+	.id_table = astk_table,
+	.probe = astk_probe,
+	.disconnect = astk_disconnect,
+	.resume = astk_resume,
+	.suspend = astk_suspend,
+	.supports_autosuspend = 1,
+};
+
+static int __init astk_init(void)
+{
+	int ret = -1;
+	ret = usb_register(&astk_driver);
+
+	return ret;
+}
+
+static void __exit astk_exit(void)
+{
+	usb_deregister(&astk_driver);
+}
+
+module_init(astk_init);
+module_exit(astk_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Jaap Aarts <jaap.aarts1@gmail.com>");
+MODULE_DESCRIPTION("Asetek gen6 driver");
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/6] fan support for asetek gen 6 driver
  2020-07-13 19:32 [PATCH 1/6] skeleton for asetek gen 6 driver jaap aarts
@ 2020-07-13 19:32 ` jaap aarts
  2020-07-13 19:32 ` [PATCH 3/6] rpm " jaap aarts
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: jaap aarts @ 2020-07-13 19:32 UTC (permalink / raw)
  To: Jean Delvare, Guenter Roeck, linux-hwmon, linux-usb
  Cc: jaap aarts, Jaap Aarts

Signed-off-by: Jaap Aarts <jaap.aarts1@example.com>
---
 drivers/hwmon/asetek_gen6.c | 315 +++++++++++++++++++++++++++++++++++-
 1 file changed, 312 insertions(+), 3 deletions(-)

diff --git a/drivers/hwmon/asetek_gen6.c b/drivers/hwmon/asetek_gen6.c
index 4d530a4cb71d..b82a678717ab 100644
--- a/drivers/hwmon/asetek_gen6.c
+++ b/drivers/hwmon/asetek_gen6.c
@@ -4,7 +4,7 @@
  * Copyright (c) Jaap Aarts 2020
  * 
  * Protocol reverse engineered by audiohacked
- * https://github.com/audiohacked/OpenCorsairLink
+ * https://github.com/audiohacked/OpendriverLink
  */
 
 /*
@@ -45,6 +45,315 @@ struct driver {
 	struct kref kref;
 };
 
+struct curve_point {
+	uint8_t temp;
+	uint8_t pwm;
+};
+
+struct fan_hwmon_data {
+	char fan_channel;
+	long fan_target;
+	unsigned char fan_pwm_target;
+	long mode;
+	struct curve_point curve[7];
+};
+
+struct hwmon_data {
+	struct driver *dev;
+	int channel_count;
+	void **channel_data;
+	int fan_1_index;
+};
+
+struct curve_point default_curve[] = {
+	{
+		.temp = 0x10,
+		.pwm = 0x19,
+	},
+	{
+		.temp = 0x14,
+		.pwm = 0x19,
+	},
+	{
+		.temp = 0x20,
+		.pwm = 0x27,
+	},
+	{
+		.temp = 0x28,
+		.pwm = 0x32,
+	},
+	{
+		.temp = 0x32,
+		.pwm = 0x4b,
+	},
+	{
+		.temp = 0x37,
+		.pwm = 0x5a,
+	},
+	{
+		.temp = 0x3c,
+		.pwm = 0x64,
+	},
+
+};
+
+static const char SUCCESS[2] = { 0x12, 0x34 };
+
+#define SUCCES_LENGTH 3
+
+static bool check_succes(char command, char ret[SUCCES_LENGTH])
+{
+	char success[SUCCES_LENGTH] = { command };
+	strncpy(&success[1], SUCCESS, SUCCES_LENGTH - 1);
+	return strncmp(ret, success, SUCCES_LENGTH) == 0;
+}
+
+int set_fan_target_rpm(struct driver *cdev, struct fan_hwmon_data *fan_data,
+		       long val)
+{
+	int retval;
+	int wrote;
+	int sndpipe = usb_sndbulkpipe(cdev->udev, cdev->bulk_out_endpointAddr);
+	int rcvpipe = usb_rcvbulkpipe(cdev->udev, cdev->bulk_in_endpointAddr);
+
+	char *send_buf = cdev->bulk_out_buffer;
+	char *recv_buf = cdev->bulk_in_buffer;
+
+	fan_data->fan_target = val;
+	fan_data->fan_pwm_target = 0;
+
+	send_buf[0] = 0x43;
+	send_buf[1] = fan_data->fan_channel;
+	send_buf[2] = (fan_data->fan_target >> 8);
+	send_buf[3] = fan_data->fan_target;
+
+	retval = usb_bulk_msg(cdev->udev, sndpipe, send_buf, 4, &wrote, 100);
+	if (retval != 0)
+		return retval;
+
+	retval = usb_bulk_msg(cdev->udev, rcvpipe, recv_buf, 6, &wrote, 100000);
+	if (retval != 0)
+		return retval;
+
+	//no error
+	if (!check_succes(send_buf[0], recv_buf) ||
+	    recv_buf[3] != fan_data->fan_channel)
+		printk(KERN_INFO "[*] Failled setting fan rpm %d,%d,%d/%d\n",
+		       recv_buf[0], recv_buf[1], recv_buf[2], recv_buf[3]);
+	return 0;
+}
+void get_fan_target_rpm(struct fan_hwmon_data *fan_data, long *val)
+{
+	*val = fan_data->fan_target;
+}
+int get_fan_current_rpm(struct driver *cdev, struct fan_hwmon_data *fan_data,
+			long *val)
+{
+	int retval;
+	int wrote;
+	int sndpipe = usb_sndbulkpipe(cdev->udev, cdev->bulk_out_endpointAddr);
+	int rcvpipe = usb_rcvbulkpipe(cdev->udev, cdev->bulk_in_endpointAddr);
+
+	char *send_buf = cdev->bulk_out_buffer;
+	char *recv_buf = cdev->bulk_in_buffer;
+
+	send_buf[0] = 0x41;
+	send_buf[1] = fan_data->fan_channel;
+
+	retval = usb_bulk_msg(cdev->udev, sndpipe, send_buf, 2, &wrote, 100);
+	if (retval != 0)
+		return retval;
+
+	retval = usb_bulk_msg(cdev->udev, rcvpipe, recv_buf, 6, &wrote, 100);
+	if (retval != 0)
+		return retval;
+
+	if (!check_succes(0x41, recv_buf) ||
+	    recv_buf[3] != fan_data->fan_channel)
+		printk(KERN_INFO "[*] Failled retreiving fan rmp %d,%d,%d/%d\n",
+		       recv_buf[0], recv_buf[1], recv_buf[2], recv_buf[3]);
+
+	*val = (((uint8_t)recv_buf[4]) << 8) + (uint8_t)recv_buf[5];
+	return 0;
+}
+
+umode_t is_visible_func(const void *d, enum hwmon_sensor_types type, u32 attr,
+			int channel)
+{
+	switch (type) {
+	case hwmon_fan:
+		switch (attr) {
+		case hwmon_fan_input:
+			return 0444;
+			break;
+		case hwmon_fan_target:
+			return 0644;
+			break;
+		case hwmon_fan_min:
+			return 0444;
+			break;
+		default:
+			break;
+		}
+		break;
+	default:
+		break;
+	}
+	return 0;
+}
+
+static int write_func(struct device *dev, enum hwmon_sensor_types type,
+		      u32 attr, int channel, long val)
+{
+	struct hwmon_data *data = dev_get_drvdata(dev);
+	struct driver *cdev = data->dev;
+	struct fan_hwmon_data *fan_data;
+	int retval = 0;
+
+	switch (type) {
+	case hwmon_fan:;
+		switch (attr) {
+		case hwmon_fan_target:
+			fan_data = data->channel_data[channel];
+
+			retval = usb_autopm_get_interface(cdev->interface);
+			if (retval)
+				goto exit;
+
+			if (down_trylock(&cdev->limit_sem)) {
+				retval = -EAGAIN;
+				goto cleanup_interface;
+			}
+
+			retval = set_fan_target_rpm(cdev, fan_data, val);
+			if (retval)
+				goto cleanup;
+
+			break;
+		default:
+			return -EINVAL;
+		}
+		break;
+	default:
+		return -EINVAL;
+	}
+
+cleanup:
+	up(&cdev->limit_sem);
+cleanup_interface:
+	usb_autopm_put_interface(cdev->interface);
+exit:
+	return retval;
+}
+int read_func(struct device *dev, enum hwmon_sensor_types type, u32 attr,
+	      int channel, long *val)
+{
+	struct hwmon_data *data = dev_get_drvdata(dev);
+	struct driver *cdev = data->dev;
+	struct fan_hwmon_data *fan_data;
+	int retval = 0;
+	if (channel >= data->channel_count)
+		return -EAGAIN;
+
+	switch (type) {
+	case hwmon_fan:;
+		switch (attr) {
+		case hwmon_fan_input:
+			fan_data = data->channel_data[channel];
+
+			retval = usb_autopm_get_interface(cdev->interface);
+			if (retval)
+				goto exit;
+
+			if (down_trylock(&cdev->limit_sem)) {
+				retval = -EAGAIN;
+				goto cleanup_interface;
+			}
+
+			retval = get_fan_current_rpm(cdev, fan_data, val);
+			if (retval)
+				goto cleanup;
+
+			break;
+		case hwmon_fan_target:
+			fan_data = data->channel_data[channel];
+
+			get_fan_target_rpm(fan_data, val);
+			break;
+		case hwmon_fan_min:
+			*val = 200;
+			break;
+		default:
+			return -EINVAL;
+		}
+		break;
+	default:
+		return -EINVAL;
+	}
+
+cleanup:
+	up(&cdev->limit_sem);
+cleanup_interface:
+	usb_autopm_put_interface(cdev->interface);
+exit:
+	return retval;
+}
+static const struct hwmon_channel_info *dual_fan[] = {
+	HWMON_CHANNEL_INFO(fan, HWMON_F_INPUT | HWMON_F_TARGET,
+			   HWMON_F_INPUT | HWMON_F_TARGET),
+	NULL
+};
+
+static const struct hwmon_ops asetek_6_ops = {
+	.is_visible = is_visible_func,
+	.read = read_func,
+	.write = write_func,
+};
+
+static const struct hwmon_chip_info lm75_chip_info = {
+	.ops = &asetek_6_ops,
+	.info = dual_fan,
+};
+
+void hwmon_init(struct driver *dev)
+{
+	struct device *hwmon_dev;
+	struct hwmon_data *data = devm_kzalloc(
+		&dev->udev->dev, sizeof(struct hwmon_data), GFP_KERNEL);
+	struct hwmon_chip_info *hwmon_info = devm_kzalloc(
+		&dev->udev->dev, sizeof(struct hwmon_chip_info), GFP_KERNEL);
+	struct fan_hwmon_data *fan1 = devm_kzalloc(
+		&dev->udev->dev, sizeof(struct fan_hwmon_data), GFP_KERNEL);
+	struct fan_hwmon_data *fan2 = devm_kzalloc(
+		&dev->udev->dev, sizeof(struct fan_hwmon_data), GFP_KERNEL);
+	data->channel_count = 2; //amount of fans
+	data->channel_data =
+		devm_kzalloc(&dev->udev->dev,
+			     sizeof(char *) * data->channel_count, GFP_KERNEL);
+
+	hwmon_info->ops = &asetek_6_ops;
+	hwmon_info->info = dual_fan;
+
+	fan1->fan_channel = 0;
+	fan1->mode = 2;
+	fan2->fan_channel = 1;
+	fan2->mode = 2;
+
+	data->fan_1_index = 0;
+	data->dev = dev;
+	data->channel_data[0] = fan1;
+	data->channel_data[1] = fan2;
+
+	hwmon_dev = devm_hwmon_device_register_with_info(
+		&dev->udev->dev, "driver_fan", data, hwmon_info, NULL);
+	printk(KERN_INFO "[*] Setup hwmon\n");
+}
+
+void hwmon_deinit(struct driver *dev)
+{
+	hwmon_device_unregister(&dev->udev->dev);
+	printk(KERN_INFO "[*] HWMON DISCONNECT\n");
+}
 /* devices that work with this driver */
 static const struct usb_device_id astk_table[] = {
 	{ USB_DEVICE(0x1b1c, 0x0c15) },
@@ -121,7 +430,7 @@ static int astk_probe(struct usb_interface *interface,
 	dev->bulk_out_buffer = kmalloc(dev->bulk_out_size, GFP_KERNEL);
 	dev->bulk_out_endpointAddr = bulk_out->bEndpointAddress;
 
-	//hwmon_init(dev);
+	hwmon_init(dev);
 	retval = init_device(dev->udev);
 	if (retval) {
 		dev_err(&interface->dev, "Failled initialising this device.\n");
@@ -139,7 +448,7 @@ static int astk_probe(struct usb_interface *interface,
 static void astk_disconnect(struct usb_interface *interface)
 {
 	struct driver *dev = usb_get_intfdata(interface);
-	//hwmon_deinit(dev);
+	hwmon_deinit(dev);
 	usb_set_intfdata(interface, NULL);
 	kref_put(&dev->kref, astk_delete);
 	deinit_device(dev->udev);
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/6] rpm support for asetek gen 6 driver
  2020-07-13 19:32 [PATCH 1/6] skeleton for asetek gen 6 driver jaap aarts
  2020-07-13 19:32 ` [PATCH 2/6] fan support " jaap aarts
@ 2020-07-13 19:32 ` jaap aarts
  2020-07-13 19:32 ` [PATCH 4/6] automatic fan count detection jaap aarts
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: jaap aarts @ 2020-07-13 19:32 UTC (permalink / raw)
  To: Jean Delvare, Guenter Roeck, linux-hwmon, linux-usb
  Cc: jaap aarts, Jaap Aarts

Signed-off-by: Jaap Aarts <jaap.aarts1@example.com>
---
 drivers/hwmon/asetek_gen6.c | 190 ++++++++++++++++++++++++++++++++++--
 1 file changed, 182 insertions(+), 8 deletions(-)

diff --git a/drivers/hwmon/asetek_gen6.c b/drivers/hwmon/asetek_gen6.c
index b82a678717ab..5c3bd456f881 100644
--- a/drivers/hwmon/asetek_gen6.c
+++ b/drivers/hwmon/asetek_gen6.c
@@ -108,6 +108,50 @@ static bool check_succes(char command, char ret[SUCCES_LENGTH])
 	return strncmp(ret, success, SUCCES_LENGTH) == 0;
 }
 
+int set_fan_rpm_curve(struct driver *cdev, struct fan_hwmon_data *fan_data,
+		      struct curve_point point[7])
+{
+	int retval;
+	int wrote;
+	int sndpipe = usb_sndbulkpipe(cdev->udev, cdev->bulk_out_endpointAddr);
+	int rcvpipe = usb_rcvbulkpipe(cdev->udev, cdev->bulk_in_endpointAddr);
+
+	char *send_buf = cdev->bulk_out_buffer;
+	char *recv_buf = cdev->bulk_in_buffer;
+	memcpy(fan_data->curve, point, sizeof(fan_data->curve));
+
+	send_buf[0] = 0x40;
+	send_buf[1] = fan_data->fan_channel;
+	send_buf[2] = point[0].temp;
+	send_buf[3] = point[1].temp;
+	send_buf[4] = point[2].temp;
+	send_buf[5] = point[3].temp;
+	send_buf[6] = point[4].temp;
+	send_buf[7] = point[5].temp;
+	send_buf[8] = point[6].temp;
+	send_buf[9] = point[0].pwm;
+	send_buf[10] = point[1].pwm;
+	send_buf[11] = point[2].pwm;
+	send_buf[12] = point[3].pwm;
+	send_buf[13] = point[4].pwm;
+	send_buf[14] = point[5].pwm;
+	send_buf[15] = point[6].pwm;
+
+	retval = usb_bulk_msg(cdev->udev, sndpipe, send_buf, 16, &wrote, 100);
+	if (retval != 0)
+		return retval;
+
+	retval = usb_bulk_msg(cdev->udev, rcvpipe, recv_buf, 4, &wrote, 100);
+	if (retval != 0)
+		return retval;
+
+	if (!check_succes(
+		    send_buf[0],
+		    recv_buf) /* || recv_buf[3] != fan_data->fan_channel */)
+		printk(KERN_INFO "[*] Failled setting fan curve %d,%d,%d/%d\n",
+		       recv_buf[0], recv_buf[1], recv_buf[2], recv_buf[3]);
+	return 0;
+}
 int set_fan_target_rpm(struct driver *cdev, struct fan_hwmon_data *fan_data,
 		       long val)
 {
@@ -177,6 +221,40 @@ int get_fan_current_rpm(struct driver *cdev, struct fan_hwmon_data *fan_data,
 	return 0;
 }
 
+int set_fan_target_pwm(struct driver *cdev, struct fan_hwmon_data *fan_data,
+		       long val)
+{
+	int retval;
+	int wrote;
+	int sndpipe = usb_sndbulkpipe(cdev->udev, cdev->bulk_out_endpointAddr);
+	int rcvpipe = usb_rcvbulkpipe(cdev->udev, cdev->bulk_in_endpointAddr);
+
+	unsigned char *send_buf = cdev->bulk_out_buffer;
+	unsigned char *recv_buf = cdev->bulk_in_buffer;
+
+	fan_data->fan_pwm_target = val;
+	fan_data->fan_target = 0;
+
+	send_buf[0] = 0x42;
+	send_buf[1] = fan_data->fan_channel;
+	send_buf[3] = fan_data->fan_pwm_target;
+
+	retval = usb_bulk_msg(cdev->udev, sndpipe, send_buf, 4, &wrote, 100);
+	if (retval != 0)
+		return retval;
+
+	retval = usb_bulk_msg(cdev->udev, rcvpipe, recv_buf, 6, &wrote, 100000);
+	if (retval != 0)
+		return retval;
+
+	//no error
+	if (!check_succes(send_buf[0], recv_buf) ||
+	    recv_buf[3] != fan_data->fan_channel)
+		printk(KERN_INFO "[*] Failled setting fan pwm %d,%d,%d/%d\n",
+		       recv_buf[0], recv_buf[1], recv_buf[2], recv_buf[3]);
+	return 0;
+}
+
 umode_t is_visible_func(const void *d, enum hwmon_sensor_types type, u32 attr,
 			int channel)
 {
@@ -196,6 +274,19 @@ umode_t is_visible_func(const void *d, enum hwmon_sensor_types type, u32 attr,
 			break;
 		}
 		break;
+	case hwmon_pwm:
+		switch (attr) {
+		case hwmon_pwm_input:
+			return 0200;
+			break;
+		case hwmon_pwm_mode:
+			return 0644;
+			break;
+		default:
+			break;
+		}
+		break;
+
 	default:
 		break;
 	}
@@ -214,7 +305,11 @@ static int write_func(struct device *dev, enum hwmon_sensor_types type,
 	case hwmon_fan:;
 		switch (attr) {
 		case hwmon_fan_target:
+
 			fan_data = data->channel_data[channel];
+			if (fan_data->mode != 1) {
+				return -EINVAL;
+			}
 
 			retval = usb_autopm_get_interface(cdev->interface);
 			if (retval)
@@ -229,11 +324,70 @@ static int write_func(struct device *dev, enum hwmon_sensor_types type,
 			if (retval)
 				goto cleanup;
 
-			break;
+			goto exit;
 		default:
 			return -EINVAL;
 		}
-		break;
+		goto exit;
+	case hwmon_pwm:;
+
+		switch (attr) {
+		case hwmon_pwm_input:
+			fan_data = data->channel_data[channel];
+			if (fan_data->mode != 1) {
+				return -EINVAL;
+			}
+
+			retval = usb_autopm_get_interface(cdev->interface);
+			if (retval)
+				goto exit;
+
+			if (down_trylock(&cdev->limit_sem)) {
+				retval = -EAGAIN;
+				goto cleanup_interface;
+			}
+
+			retval = set_fan_target_pwm(cdev, fan_data, val);
+			if (retval)
+				return retval;
+
+			goto cleanup;
+		case hwmon_pwm_mode:
+			fan_data = data->channel_data[channel];
+
+			retval = usb_autopm_get_interface(cdev->interface);
+			if (retval)
+				goto exit;
+
+			if (down_trylock(&cdev->limit_sem)) {
+				retval = -EAGAIN;
+				goto cleanup_interface;
+			}
+			fan_data->mode = val;
+
+			if (val == 0) {
+				set_fan_rpm_curve(cdev, fan_data,
+						  default_curve);
+			} else if (val == 1) {
+				if (fan_data->fan_target != 0) {
+					retval = set_fan_target_rpm(
+						cdev, fan_data,
+						fan_data->fan_target);
+					if (retval)
+						goto cleanup;
+				} else if (fan_data->fan_pwm_target != 0) {
+					retval = set_fan_target_pwm(
+						cdev, fan_data,
+						fan_data->fan_pwm_target);
+					if (retval)
+						goto cleanup;
+				}
+			}
+			goto cleanup;
+		default:
+			return -EINVAL;
+		}
+		goto exit;
 	default:
 		return -EINVAL;
 	}
@@ -274,19 +428,36 @@ int read_func(struct device *dev, enum hwmon_sensor_types type, u32 attr,
 			if (retval)
 				goto cleanup;
 
-			break;
+			goto cleanup;
 		case hwmon_fan_target:
 			fan_data = data->channel_data[channel];
+			if (fan_data->mode != 1) {
+				*val = 0;
+				goto exit;
+			}
 
 			get_fan_target_rpm(fan_data, val);
-			break;
+			goto exit;
 		case hwmon_fan_min:
 			*val = 200;
-			break;
+			goto exit;
+
 		default:
 			return -EINVAL;
 		}
-		break;
+		goto exit;
+
+	case hwmon_pwm:
+		switch (attr) {
+		case hwmon_pwm_mode:
+			fan_data = data->channel_data[channel];
+			*val = fan_data->mode;
+			goto exit;
+		default:
+			return -EINVAL;
+		}
+		goto exit;
+
 	default:
 		return -EINVAL;
 	}
@@ -299,8 +470,11 @@ int read_func(struct device *dev, enum hwmon_sensor_types type, u32 attr,
 	return retval;
 }
 static const struct hwmon_channel_info *dual_fan[] = {
-	HWMON_CHANNEL_INFO(fan, HWMON_F_INPUT | HWMON_F_TARGET,
-			   HWMON_F_INPUT | HWMON_F_TARGET),
+	HWMON_CHANNEL_INFO(fan, HWMON_F_INPUT | HWMON_F_TARGET | HWMON_F_MIN,
+			   HWMON_F_INPUT | HWMON_F_TARGET | HWMON_F_MIN),
+	HWMON_CHANNEL_INFO(pwm, HWMON_PWM_INPUT | HWMON_PWM_MODE,
+			   HWMON_PWM_INPUT | HWMON_PWM_MODE),
+
 	NULL
 };
 
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 4/6] automatic fan count detection
  2020-07-13 19:32 [PATCH 1/6] skeleton for asetek gen 6 driver jaap aarts
  2020-07-13 19:32 ` [PATCH 2/6] fan support " jaap aarts
  2020-07-13 19:32 ` [PATCH 3/6] rpm " jaap aarts
@ 2020-07-13 19:32 ` jaap aarts
  2020-07-13 19:32 ` [PATCH 5/6] allow setting enable instead of mode, and support enable 2/3/4 jaap aarts
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: jaap aarts @ 2020-07-13 19:32 UTC (permalink / raw)
  To: Jean Delvare, Guenter Roeck, linux-hwmon, linux-usb
  Cc: jaap aarts, Jaap Aarts

Signed-off-by: Jaap Aarts <jaap.aarts1@example.com>
---
 drivers/hwmon/asetek_gen6.c | 146 ++++++++++++++++++++++++------------
 1 file changed, 96 insertions(+), 50 deletions(-)

diff --git a/drivers/hwmon/asetek_gen6.c b/drivers/hwmon/asetek_gen6.c
index 5c3bd456f881..d657ecc78908 100644
--- a/drivers/hwmon/asetek_gen6.c
+++ b/drivers/hwmon/asetek_gen6.c
@@ -9,11 +9,11 @@
 
 /*
  * Supports following chips: 
- * driver h100i platinum
+ * h100i platinum
  * 
  * Other products should work with this driver but no testing has been done.
  * 
- * Note: platinum is the codename name for pro within driver
+ * Note: platinum is the codename name for pro within driver, so h100i platinum = h1ooi pro
  * 
  * Note: fan curve controll has not been implemented
  */
@@ -50,7 +50,7 @@ struct curve_point {
 	uint8_t pwm;
 };
 
-struct fan_hwmon_data {
+struct hwmon_fan_data {
 	char fan_channel;
 	long fan_target;
 	unsigned char fan_pwm_target;
@@ -62,7 +62,6 @@ struct hwmon_data {
 	struct driver *dev;
 	int channel_count;
 	void **channel_data;
-	int fan_1_index;
 };
 
 struct curve_point default_curve[] = {
@@ -105,10 +104,10 @@ static bool check_succes(char command, char ret[SUCCES_LENGTH])
 {
 	char success[SUCCES_LENGTH] = { command };
 	strncpy(&success[1], SUCCESS, SUCCES_LENGTH - 1);
-	return strncmp(ret, success, SUCCES_LENGTH) == 0;
+	return strncmp(ret, success, SUCCES_LENGTH - 1) == 0;
 }
 
-int set_fan_rpm_curve(struct driver *cdev, struct fan_hwmon_data *fan_data,
+int set_fan_rpm_curve(struct driver *cdev, struct hwmon_fan_data *fan_data,
 		      struct curve_point point[7])
 {
 	int retval;
@@ -145,14 +144,12 @@ int set_fan_rpm_curve(struct driver *cdev, struct fan_hwmon_data *fan_data,
 	if (retval != 0)
 		return retval;
 
-	if (!check_succes(
-		    send_buf[0],
-		    recv_buf) /* || recv_buf[3] != fan_data->fan_channel */)
+	if (!check_succes(send_buf[0], recv_buf))
 		printk(KERN_INFO "[*] Failled setting fan curve %d,%d,%d/%d\n",
 		       recv_buf[0], recv_buf[1], recv_buf[2], recv_buf[3]);
 	return 0;
 }
-int set_fan_target_rpm(struct driver *cdev, struct fan_hwmon_data *fan_data,
+int set_fan_target_rpm(struct driver *cdev, struct hwmon_fan_data *fan_data,
 		       long val)
 {
 	int retval;
@@ -180,17 +177,16 @@ int set_fan_target_rpm(struct driver *cdev, struct fan_hwmon_data *fan_data,
 		return retval;
 
 	//no error
-	if (!check_succes(send_buf[0], recv_buf) ||
-	    recv_buf[3] != fan_data->fan_channel)
+	if (!check_succes(send_buf[0], recv_buf))
 		printk(KERN_INFO "[*] Failled setting fan rpm %d,%d,%d/%d\n",
 		       recv_buf[0], recv_buf[1], recv_buf[2], recv_buf[3]);
 	return 0;
 }
-void get_fan_target_rpm(struct fan_hwmon_data *fan_data, long *val)
+void get_fan_target_rpm(struct hwmon_fan_data *fan_data, long *val)
 {
 	*val = fan_data->fan_target;
 }
-int get_fan_current_rpm(struct driver *cdev, struct fan_hwmon_data *fan_data,
+int get_fan_current_rpm(struct driver *cdev, struct hwmon_fan_data *fan_data,
 			long *val)
 {
 	int retval;
@@ -221,7 +217,7 @@ int get_fan_current_rpm(struct driver *cdev, struct fan_hwmon_data *fan_data,
 	return 0;
 }
 
-int set_fan_target_pwm(struct driver *cdev, struct fan_hwmon_data *fan_data,
+int set_fan_target_pwm(struct driver *cdev, struct hwmon_fan_data *fan_data,
 		       long val)
 {
 	int retval;
@@ -248,8 +244,7 @@ int set_fan_target_pwm(struct driver *cdev, struct fan_hwmon_data *fan_data,
 		return retval;
 
 	//no error
-	if (!check_succes(send_buf[0], recv_buf) ||
-	    recv_buf[3] != fan_data->fan_channel)
+	if (!check_succes(send_buf[0], recv_buf))
 		printk(KERN_INFO "[*] Failled setting fan pwm %d,%d,%d/%d\n",
 		       recv_buf[0], recv_buf[1], recv_buf[2], recv_buf[3]);
 	return 0;
@@ -298,7 +293,7 @@ static int write_func(struct device *dev, enum hwmon_sensor_types type,
 {
 	struct hwmon_data *data = dev_get_drvdata(dev);
 	struct driver *cdev = data->dev;
-	struct fan_hwmon_data *fan_data;
+	struct hwmon_fan_data *fan_data;
 	int retval = 0;
 
 	switch (type) {
@@ -404,7 +399,7 @@ int read_func(struct device *dev, enum hwmon_sensor_types type, u32 attr,
 {
 	struct hwmon_data *data = dev_get_drvdata(dev);
 	struct driver *cdev = data->dev;
-	struct fan_hwmon_data *fan_data;
+	struct hwmon_fan_data *fan_data;
 	int retval = 0;
 	if (channel >= data->channel_count)
 		return -EAGAIN;
@@ -469,14 +464,8 @@ int read_func(struct device *dev, enum hwmon_sensor_types type, u32 attr,
 exit:
 	return retval;
 }
-static const struct hwmon_channel_info *dual_fan[] = {
-	HWMON_CHANNEL_INFO(fan, HWMON_F_INPUT | HWMON_F_TARGET | HWMON_F_MIN,
-			   HWMON_F_INPUT | HWMON_F_TARGET | HWMON_F_MIN),
-	HWMON_CHANNEL_INFO(pwm, HWMON_PWM_INPUT | HWMON_PWM_MODE,
-			   HWMON_PWM_INPUT | HWMON_PWM_MODE),
-
-	NULL
-};
+#define fan_config HWMON_F_INPUT | HWMON_F_TARGET | HWMON_F_MIN
+#define pwm_config HWMON_PWM_INPUT | HWMON_PWM_MODE
 
 static const struct hwmon_ops asetek_6_ops = {
 	.is_visible = is_visible_func,
@@ -484,40 +473,93 @@ static const struct hwmon_ops asetek_6_ops = {
 	.write = write_func,
 };
 
-static const struct hwmon_chip_info lm75_chip_info = {
-	.ops = &asetek_6_ops,
-	.info = dual_fan,
-};
+bool does_fan_exist(struct driver *cdev, int channel)
+{
+	int retval;
+	int wrote;
+	int sndpipe = usb_sndbulkpipe(cdev->udev, cdev->bulk_out_endpointAddr);
+	int rcvpipe = usb_rcvbulkpipe(cdev->udev, cdev->bulk_in_endpointAddr);
+
+	char *send_buf = cdev->bulk_out_buffer;
+	char *recv_buf = cdev->bulk_in_buffer;
+
+	send_buf[0] = 0x43;
+	send_buf[1] = channel;
+	send_buf[2] = (600 >> 8);
+	send_buf[3] = 600;
+
+	retval = usb_bulk_msg(cdev->udev, sndpipe, send_buf, 4, &wrote, 100);
+	if (retval != 0)
+		return false;
+
+	retval = usb_bulk_msg(cdev->udev, rcvpipe, recv_buf, 6, &wrote, 100000);
+	if (retval != 0)
+		return false;
+
+	if (!check_succes(send_buf[0], recv_buf))
+		return false;
+	return true;
+}
+
+int get_fan_count(struct driver *dev)
+{
+	int fan;
+	for (fan = 0; does_fan_exist(dev, fan); fan += 1) {
+	}
+	return fan;
+}
 
 void hwmon_init(struct driver *dev)
 {
+	int fan_id;
 	struct device *hwmon_dev;
+	struct hwmon_fan_data *fan;
 	struct hwmon_data *data = devm_kzalloc(
 		&dev->udev->dev, sizeof(struct hwmon_data), GFP_KERNEL);
 	struct hwmon_chip_info *hwmon_info = devm_kzalloc(
 		&dev->udev->dev, sizeof(struct hwmon_chip_info), GFP_KERNEL);
-	struct fan_hwmon_data *fan1 = devm_kzalloc(
-		&dev->udev->dev, sizeof(struct fan_hwmon_data), GFP_KERNEL);
-	struct fan_hwmon_data *fan2 = devm_kzalloc(
-		&dev->udev->dev, sizeof(struct fan_hwmon_data), GFP_KERNEL);
-	data->channel_count = 2; //amount of fans
+	struct hwmon_channel_info **aio_info =
+		devm_kzalloc(&dev->udev->dev,
+			     sizeof(struct hwmon_channel_info *) * 2,
+			     GFP_KERNEL); //2==amount of channel infos.
+	u32 *fans_config = devm_kzalloc(&dev->udev->dev,
+					sizeof(u32) * (data->channel_count + 1),
+					GFP_KERNEL);
+	u32 *pwms_config = devm_kzalloc(&dev->udev->dev,
+					sizeof(u32) * (data->channel_count + 1),
+					GFP_KERNEL);
+
+	data->channel_count = get_fan_count(dev); //amount of fans
 	data->channel_data =
 		devm_kzalloc(&dev->udev->dev,
 			     sizeof(char *) * data->channel_count, GFP_KERNEL);
 
-	hwmon_info->ops = &asetek_6_ops;
-	hwmon_info->info = dual_fan;
+	for (fan_id = 0; fan_id <= data->channel_count; fan_id++) {
+		fan = devm_kzalloc(&dev->udev->dev,
+				   sizeof(struct hwmon_fan_data), GFP_KERNEL);
+		fan->fan_channel = fan_id;
+		fan->mode = 2;
+		data->channel_data[fan_id] = fan;
+		fans_config[fan_id] = fan_config;
+		pwms_config[fan_id] = pwm_config;
+	}
+	fans_config[data->channel_count] = 0;
+	pwms_config[data->channel_count] = 0;
 
-	fan1->fan_channel = 0;
-	fan1->mode = 2;
-	fan2->fan_channel = 1;
-	fan2->mode = 2;
+	aio_info[0] = devm_kzalloc(
+		&dev->udev->dev, sizeof(struct hwmon_channel_info), GFP_KERNEL);
+	aio_info[0]->type = hwmon_fan;
+	aio_info[0]->config = fans_config;
 
-	data->fan_1_index = 0;
-	data->dev = dev;
-	data->channel_data[0] = fan1;
-	data->channel_data[1] = fan2;
+	aio_info[1] = devm_kzalloc(
+		&dev->udev->dev, sizeof(struct hwmon_channel_info), GFP_KERNEL);
+	aio_info[1]->type = hwmon_pwm;
+	aio_info[1]->config = pwms_config;
+
+	hwmon_info->ops = &asetek_6_ops;
+	hwmon_info->info = (const struct hwmon_channel_info **)aio_info;
 
+	data->dev = dev;
 	hwmon_dev = devm_hwmon_device_register_with_info(
 		&dev->udev->dev, "driver_fan", data, hwmon_info, NULL);
 	printk(KERN_INFO "[*] Setup hwmon\n");
@@ -528,10 +570,14 @@ void hwmon_deinit(struct driver *dev)
 	hwmon_device_unregister(&dev->udev->dev);
 	printk(KERN_INFO "[*] HWMON DISCONNECT\n");
 }
-/* devices that work with this driver */
+
+/*
+	Devices that work with this driver.
+	More devices should work, however none have been tested.
+*/
 static const struct usb_device_id astk_table[] = {
 	{ USB_DEVICE(0x1b1c, 0x0c15) },
-	{} /* Terminating entry */
+	{},
 };
 
 MODULE_DEVICE_TABLE(usb, astk_table);
@@ -604,13 +650,14 @@ static int astk_probe(struct usb_interface *interface,
 	dev->bulk_out_buffer = kmalloc(dev->bulk_out_size, GFP_KERNEL);
 	dev->bulk_out_endpointAddr = bulk_out->bEndpointAddress;
 
-	hwmon_init(dev);
 	retval = init_device(dev->udev);
 	if (retval) {
 		dev_err(&interface->dev, "Failled initialising this device.\n");
 		goto exit;
 	}
 
+	hwmon_init(dev);
+
 	usb_set_intfdata(interface, dev);
 	kref_init(&dev->kref);
 	mutex_init(&dev->io_mutex);
@@ -626,7 +673,6 @@ static void astk_disconnect(struct usb_interface *interface)
 	usb_set_intfdata(interface, NULL);
 	kref_put(&dev->kref, astk_delete);
 	deinit_device(dev->udev);
-	/* let the user know what node this device is now attached to */
 }
 static int astk_resume(struct usb_interface *intf)
 {
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 5/6] allow setting enable instead of mode, and support enable 2/3/4
  2020-07-13 19:32 [PATCH 1/6] skeleton for asetek gen 6 driver jaap aarts
                   ` (2 preceding siblings ...)
  2020-07-13 19:32 ` [PATCH 4/6] automatic fan count detection jaap aarts
@ 2020-07-13 19:32 ` jaap aarts
  2020-07-13 19:32 ` [PATCH 6/6] added some comments and fixed some styling jaap aarts
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: jaap aarts @ 2020-07-13 19:32 UTC (permalink / raw)
  To: Jean Delvare, Guenter Roeck, linux-hwmon, linux-usb
  Cc: jaap aarts, Jaap Aarts

Signed-off-by: Jaap Aarts <jaap.aarts1@example.com>
---
 drivers/hwmon/asetek_gen6.c | 110 ++++++++++++++++++++++++++++++------
 1 file changed, 93 insertions(+), 17 deletions(-)

diff --git a/drivers/hwmon/asetek_gen6.c b/drivers/hwmon/asetek_gen6.c
index d657ecc78908..19f50d5cd179 100644
--- a/drivers/hwmon/asetek_gen6.c
+++ b/drivers/hwmon/asetek_gen6.c
@@ -64,38 +64,100 @@ struct hwmon_data {
 	void **channel_data;
 };
 
-struct curve_point default_curve[] = {
+struct curve_point quiet_curve[] = {
 	{
-		.temp = 0x10,
-		.pwm = 0x19,
+		.temp = 0x1F,
+		.pwm = 0x15,
 	},
 	{
-		.temp = 0x14,
-		.pwm = 0x19,
+		.temp = 0x21,
+		.pwm = 0x1E,
+	},
+	{
+		.temp = 0x24,
+		.pwm = 0x25,
+	},
+	{
+		.temp = 0x27,
+		.pwm = 0x2D,
+	},
+	{
+		.temp = 0x29,
+		.pwm = 0x38,
+	},
+	{
+		.temp = 0x2C,
+		.pwm = 0x4A,
+	},
+	{
+		.temp = 0x2F,
+		.pwm = 0x64,
+	},
+};
+
+struct curve_point balanced_curve[] = {
+	{
+		.temp = 0x1C,
+		.pwm = 0x15,
+	},
+	{
+		.temp = 0x1E,
+		.pwm = 0x1B,
 	},
 	{
 		.temp = 0x20,
-		.pwm = 0x27,
+		.pwm = 0x23,
 	},
 	{
-		.temp = 0x28,
-		.pwm = 0x32,
+		.temp = 0x22,
+		.pwm = 0x28,
 	},
 	{
-		.temp = 0x32,
-		.pwm = 0x4b,
+		.temp = 0x24,
+		.pwm = 0x32,
 	},
 	{
-		.temp = 0x37,
-		.pwm = 0x5a,
+		.temp = 0x27,
+		.pwm = 0x48,
 	},
 	{
-		.temp = 0x3c,
+		.temp = 0x29,
 		.pwm = 0x64,
 	},
+};
 
+struct curve_point extreme_curve[] = {
+	{
+		.temp = 0x19,
+		.pwm = 0x28,
+	},
+	{
+		.temp = 0x1B,
+		.pwm = 0x2E,
+	},
+	{
+		.temp = 0x1D,
+		.pwm = 0x37,
+	},
+	{
+		.temp = 0x1E,
+		.pwm = 0x41,
+	},
+	{
+		.temp = 0x1F,
+		.pwm = 0x4C,
+	},
+	{
+		.temp = 0x20,
+		.pwm = 0x56,
+	},
+	{
+		.temp = 0x21,
+		.pwm = 0x64,
+	},
 };
 
+#define default_curve quiet_curve
 static const char SUCCESS[2] = { 0x12, 0x34 };
 
 #define SUCCES_LENGTH 3
@@ -347,7 +409,7 @@ static int write_func(struct device *dev, enum hwmon_sensor_types type,
 				return retval;
 
 			goto cleanup;
-		case hwmon_pwm_mode:
+		case hwmon_pwm_enable:
 			fan_data = data->channel_data[channel];
 
 			retval = usb_autopm_get_interface(cdev->interface);
@@ -360,10 +422,12 @@ static int write_func(struct device *dev, enum hwmon_sensor_types type,
 			}
 			fan_data->mode = val;
 
-			if (val == 0) {
+			switch (val) {
+			case 0:
 				set_fan_rpm_curve(cdev, fan_data,
 						  default_curve);
-			} else if (val == 1) {
+				break;
+			case 1:
 				if (fan_data->fan_target != 0) {
 					retval = set_fan_target_rpm(
 						cdev, fan_data,
@@ -377,6 +441,18 @@ static int write_func(struct device *dev, enum hwmon_sensor_types type,
 					if (retval)
 						goto cleanup;
 				}
+				break;
+			case 2:
+				set_fan_rpm_curve(cdev, fan_data, quiet_curve);
+				break;
+			case 3:
+				set_fan_rpm_curve(cdev, fan_data,
+						  balanced_curve);
+				break;
+			case 4:
+				set_fan_rpm_curve(cdev, fan_data,
+						  extreme_curve);
+				break;
 			}
 			goto cleanup;
 		default:
@@ -465,7 +541,7 @@ int read_func(struct device *dev, enum hwmon_sensor_types type, u32 attr,
 	return retval;
 }
 #define fan_config HWMON_F_INPUT | HWMON_F_TARGET | HWMON_F_MIN
-#define pwm_config HWMON_PWM_INPUT | HWMON_PWM_MODE
+#define pwm_config HWMON_PWM_INPUT | HWMON_PWM_ENABLE
 
 static const struct hwmon_ops asetek_6_ops = {
 	.is_visible = is_visible_func,
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 6/6] added some comments and fixed some styling
  2020-07-13 19:32 [PATCH 1/6] skeleton for asetek gen 6 driver jaap aarts
                   ` (3 preceding siblings ...)
  2020-07-13 19:32 ` [PATCH 5/6] allow setting enable instead of mode, and support enable 2/3/4 jaap aarts
@ 2020-07-13 19:32 ` jaap aarts
  2020-07-14  4:59 ` [PATCH 1/6] skeleton for asetek gen 6 driver Guenter Roeck
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: jaap aarts @ 2020-07-13 19:32 UTC (permalink / raw)
  To: Jean Delvare, Guenter Roeck, linux-hwmon, linux-usb
  Cc: jaap aarts, Jaap Aarts

Signed-off-by: Jaap Aarts <jaap.aarts1@example.com>
---
 ...driver.patch.EXPERIMENTAL-checkpatch-fixes | 239 ++++++++++++++++++
 drivers/hwmon/asetek_gen6.c                   |  31 ++-
 2 files changed, 260 insertions(+), 10 deletions(-)
 create mode 100644 0001-skeleton-for-asetek-gen-6-driver.patch.EXPERIMENTAL-checkpatch-fixes

diff --git a/0001-skeleton-for-asetek-gen-6-driver.patch.EXPERIMENTAL-checkpatch-fixes b/0001-skeleton-for-asetek-gen-6-driver.patch.EXPERIMENTAL-checkpatch-fixes
new file mode 100644
index 000000000000..6f47a064072b
--- /dev/null
+++ b/0001-skeleton-for-asetek-gen-6-driver.patch.EXPERIMENTAL-checkpatch-fixes
@@ -0,0 +1,239 @@
+From 84c7d189d0f4227f7b249a73c2c1bd26ed4f46a6 Mon Sep 17 00:00:00 2001
+From: jaap aarts <jaap.aarts1@gmail.com>
+Date: Tue, 30 Jun 2020 13:46:14 +0200
+Subject: [PATCH 1/5] skeleton for asetek gen 6 driver
+
+---
+ drivers/hwmon/Kconfig       |   6 ++
+ drivers/hwmon/Makefile      |   1 +
+ drivers/hwmon/asetek_gen6.c | 186 ++++++++++++++++++++++++++++++++++++
+ 3 files changed, 193 insertions(+)
+ create mode 100644 drivers/hwmon/asetek_gen6.c
+
+diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
+index 288ae9f63588..521a9e0c88ca 100644
+--- a/drivers/hwmon/Kconfig
++++ b/drivers/hwmon/Kconfig
+@@ -378,6 +378,12 @@ config SENSORS_ARM_SCPI
+ 	  and power sensors available on ARM Ltd's SCP based platforms. The
+ 	  actual number and type of sensors exported depend on the platform.
+ 
++config SENSORS_ASETEK_GEN6
++	tristate "Asetek generation 6 driver"
++	help
++	  If you say yes here you get support for asetek generation 6 boards
++	  found on most AIO liquid coolers with an asetek pump.
++
+ config SENSORS_ASB100
+ 	tristate "Asus ASB100 Bach"
+ 	depends on X86 && I2C
+diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
+index 3e32c21f5efe..9683d2aae2b2 100644
+--- a/drivers/hwmon/Makefile
++++ b/drivers/hwmon/Makefile
+@@ -20,6 +20,7 @@ obj-$(CONFIG_SENSORS_W83793)	+= w83793.o
+ obj-$(CONFIG_SENSORS_W83795)	+= w83795.o
+ obj-$(CONFIG_SENSORS_W83781D)	+= w83781d.o
+ obj-$(CONFIG_SENSORS_W83791D)	+= w83791d.o
++obj-$(CONFIG_SENSORS_ASETEK_GEN6)	+= asetek_gen6.o
+ 
+ obj-$(CONFIG_SENSORS_AB8500)	+= abx500.o ab8500.o
+ obj-$(CONFIG_SENSORS_ABITUGURU)	+= abituguru.o
+diff --git a/drivers/hwmon/asetek_gen6.c b/drivers/hwmon/asetek_gen6.c
+new file mode 100644
+index 000000000000..4d530a4cb71d
+--- /dev/null
++++ b/drivers/hwmon/asetek_gen6.c
+@@ -0,0 +4,189 @@
++// SPDX-License-Identifier: GPL-2.0-or-later
++/*
++ * A hwmon driver for most asetek gen 6 all-in-one liquid coolers.
++ * Copyright (c) Jaap Aarts 2020
++ *
++ * Protocol reverse engineered by audiohacked
++ * https://github.com/audiohacked/OpenCorsairLink
++ */
++
++/*
++ * Supports following chips:
++ * driver h100i platinum
++ *
++ * Other products should work with this driver but no testing has been done.
++ *
++ * Note: platinum is the codename name for pro within driver
++ *
++ * Note: fan curve control has not been implemented
++ */
++
++#include <linux/module.h>
++#include <linux/kernel.h>
++#include <linux/usb.h>
++#include <linux/slab.h>
++#include <linux/mutex.h>
++#include <linux/errno.h>
++#include <linux/hwmon.h>
++#include <linux/hwmon-sysfs.h>
++
++struct driver {
++	struct usb_device *udev;
++
++	char *bulk_out_buffer;
++	char *bulk_in_buffer;
++	size_t bulk_out_size;
++	size_t bulk_in_size;
++	char bulk_in_endpointAddr;
++	char bulk_out_endpointAddr;
++
++	struct usb_interface *interface; /* the interface for this device */
++	struct mutex io_mutex; /* synchronize I/O with disconnect */
++	struct semaphore
++		limit_sem; /* limiting the number of writes in progress */
++
++	struct kref kref;
++};
++
++/* devices that work with this driver */
++static const struct usb_device_id astk_table[] = {
++	{ USB_DEVICE(0x1b1c, 0x0c15) },
++	{} /* Terminating entry */
++};
++
++MODULE_DEVICE_TABLE(usb, astk_table);
++
++int init_device(struct usb_device *udev)
++{
++	int retval;
++
++	retval = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), 0x00, 0x40,
++				 0xffff, 0x0000, 0, 0, 0);
++	//this always returns error
++	if (retval != 0)
++		;
++
++	retval = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), 0x02, 0x40,
++				 0x0002, 0x0000, 0, 0, 0);
++	if (retval != 0)
++		return retval;
++
++	return 0;
++}
++int deinit_device(struct usb_device *udev)
++{
++	int retval;
++
++	retval = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), 0x02, 0x40,
++				 0x0004, 0x0000, 0, 0, 0);
++	if (retval != 0)
++		return retval;
++
++	return 0;
++}
++
++static void astk_delete(struct kref *kref)
++{
++	struct driver *dev = container_of(kref, struct driver, kref);
++
++	usb_put_intf(dev->interface);
++	usb_put_dev(dev->udev);
++	kfree(dev->bulk_in_buffer);
++	kfree(dev->bulk_out_buffer);
++	kfree(dev);
++}
++
++static int astk_probe(struct usb_interface *interface,
++		      const struct usb_device_id *id)
++{
++	struct driver *dev;
++	int retval = 0;
++	struct usb_endpoint_descriptor *bulk_in, *bulk_out;
++
++	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
++	if (!dev) {
++		retval = -ENOMEM;
++		goto exit;
++	}
++
++	retval = usb_find_common_endpoints(interface->cur_altsetting, &bulk_in,
++					   &bulk_out, NULL, NULL);
++	if (retval != 0)
++		goto exit;
++
++	dev->udev = usb_get_dev(interface_to_usbdev(interface));
++	dev->interface = usb_get_intf(interface);
++
++	/* set up the endpoint information */
++	/* use only the first bulk-in and bulk-out endpoints */
++	dev->bulk_in_size = usb_endpoint_maxp(bulk_in);
++	dev->bulk_in_buffer = kmalloc(dev->bulk_in_size, GFP_KERNEL);
++	dev->bulk_in_endpointAddr = bulk_in->bEndpointAddress;
++	dev->bulk_out_size = usb_endpoint_maxp(bulk_out);
++	dev->bulk_out_buffer = kmalloc(dev->bulk_out_size, GFP_KERNEL);
++	dev->bulk_out_endpointAddr = bulk_out->bEndpointAddress;
++
++	//hwmon_init(dev);
++	retval = init_device(dev->udev);
++	if (retval) {
++		dev_err(&interface->dev, "Failled initialising this device.\n");
++		goto exit;
++	}
++
++	usb_set_intfdata(interface, dev);
++	kref_init(&dev->kref);
++	mutex_init(&dev->io_mutex);
++	sema_init(&dev->limit_sem, 8);
++exit:
++	return retval;
++}
++
++static void astk_disconnect(struct usb_interface *interface)
++{
++	struct driver *dev = usb_get_intfdata(interface);
++	//hwmon_deinit(dev);
++	usb_set_intfdata(interface, NULL);
++	kref_put(&dev->kref, astk_delete);
++	deinit_device(dev->udev);
++	/* let the user know what node this device is now attached to */
++}
++static int astk_resume(struct usb_interface *intf)
++{
++	return 0;
++}
++
++static int astk_suspend(struct usb_interface *intf, pm_message_t message)
++{
++	return 0;
++}
++
++static struct usb_driver astk_driver = {
++	.name = "Asetek gen6 driver",
++	.id_table = astk_table,
++	.probe = astk_probe,
++	.disconnect = astk_disconnect,
++	.resume = astk_resume,
++	.suspend = astk_suspend,
++	.supports_autosuspend = 1,
++};
++
++static int __init astk_init(void)
++{
++	int ret = -1;
++
++	ret = usb_register(&astk_driver);
++
++	return ret;
++}
++
++static void __exit astk_exit(void)
++{
++	usb_deregister(&astk_driver);
++}
++
++module_init(astk_init);
++module_exit(astk_exit);
++
++MODULE_LICENSE("GPL");
++MODULE_AUTHOR("Jaap Aarts <jaap.aarts1@gmail.com>");
++MODULE_DESCRIPTION("Asetek gen6 driver");
+-- 
+2.27.0
+
diff --git a/drivers/hwmon/asetek_gen6.c b/drivers/hwmon/asetek_gen6.c
index 19f50d5cd179..7fc31d4b5743 100644
--- a/drivers/hwmon/asetek_gen6.c
+++ b/drivers/hwmon/asetek_gen6.c
@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
-/* 
+/*
  * A hwmon driver for most asetek gen 6 all-in-one liquid coolers.
  * Copyright (c) Jaap Aarts 2020
  * 
@@ -15,7 +15,7 @@
  * 
  * Note: platinum is the codename name for pro within driver, so h100i platinum = h1ooi pro
  * 
- * Note: fan curve controll has not been implemented
+ * Note: fan curve control has not been implemented
  */
 
 #include <linux/module.h>
@@ -158,6 +158,7 @@ struct curve_point extreme_curve[] = {
 };
 
 #define default_curve quiet_curve
+
 static const char SUCCESS[2] = { 0x12, 0x34 };
 
 #define SUCCES_LENGTH 3
@@ -211,6 +212,7 @@ int set_fan_rpm_curve(struct driver *cdev, struct hwmon_fan_data *fan_data,
 		       recv_buf[0], recv_buf[1], recv_buf[2], recv_buf[3]);
 	return 0;
 }
+
 int set_fan_target_rpm(struct driver *cdev, struct hwmon_fan_data *fan_data,
 		       long val)
 {
@@ -244,10 +246,12 @@ int set_fan_target_rpm(struct driver *cdev, struct hwmon_fan_data *fan_data,
 		       recv_buf[0], recv_buf[1], recv_buf[2], recv_buf[3]);
 	return 0;
 }
+
 void get_fan_target_rpm(struct hwmon_fan_data *fan_data, long *val)
 {
 	*val = fan_data->fan_target;
 }
+
 int get_fan_current_rpm(struct driver *cdev, struct hwmon_fan_data *fan_data,
 			long *val)
 {
@@ -272,7 +276,7 @@ int get_fan_current_rpm(struct driver *cdev, struct hwmon_fan_data *fan_data,
 
 	if (!check_succes(0x41, recv_buf) ||
 	    recv_buf[3] != fan_data->fan_channel)
-		printk(KERN_INFO "[*] Failled retreiving fan rmp %d,%d,%d/%d\n",
+		printk(KERN_INFO "[*] Failled retrieving fan rmp %d,%d,%d/%d\n",
 		       recv_buf[0], recv_buf[1], recv_buf[2], recv_buf[3]);
 
 	*val = (((uint8_t)recv_buf[4]) << 8) + (uint8_t)recv_buf[5];
@@ -470,6 +474,7 @@ static int write_func(struct device *dev, enum hwmon_sensor_types type,
 exit:
 	return retval;
 }
+
 int read_func(struct device *dev, enum hwmon_sensor_types type, u32 attr,
 	      int channel, long *val)
 {
@@ -540,8 +545,9 @@ int read_func(struct device *dev, enum hwmon_sensor_types type, u32 attr,
 exit:
 	return retval;
 }
-#define fan_config HWMON_F_INPUT | HWMON_F_TARGET | HWMON_F_MIN
-#define pwm_config HWMON_PWM_INPUT | HWMON_PWM_ENABLE
+
+#define fan_config (HWMON_F_INPUT | HWMON_F_TARGET | HWMON_F_MIN)
+#define pwm_config (HWMON_PWM_INPUT | HWMON_PWM_ENABLE)
 
 static const struct hwmon_ops asetek_6_ops = {
 	.is_visible = is_visible_func,
@@ -580,8 +586,8 @@ bool does_fan_exist(struct driver *cdev, int channel)
 int get_fan_count(struct driver *dev)
 {
 	int fan;
-	for (fan = 0; does_fan_exist(dev, fan); fan += 1) {
-	}
+	for (fan = 0; does_fan_exist(dev, fan); fan += 1)
+		;
 	return fan;
 }
 
@@ -594,10 +600,13 @@ void hwmon_init(struct driver *dev)
 		&dev->udev->dev, sizeof(struct hwmon_data), GFP_KERNEL);
 	struct hwmon_chip_info *hwmon_info = devm_kzalloc(
 		&dev->udev->dev, sizeof(struct hwmon_chip_info), GFP_KERNEL);
+	//Allocate the info table
 	struct hwmon_channel_info **aio_info =
 		devm_kzalloc(&dev->udev->dev,
 			     sizeof(struct hwmon_channel_info *) * 2,
-			     GFP_KERNEL); //2==amount of channel infos.
+			     GFP_KERNEL); //2 for each channel info.
+
+	//Allocate the fan and PWM configuration
 	u32 *fans_config = devm_kzalloc(&dev->udev->dev,
 					sizeof(u32) * (data->channel_count + 1),
 					GFP_KERNEL);
@@ -610,6 +619,7 @@ void hwmon_init(struct driver *dev)
 		devm_kzalloc(&dev->udev->dev,
 			     sizeof(char *) * data->channel_count, GFP_KERNEL);
 
+	//For each fan create a data channel a fan config entry and a pwm config entry
 	for (fan_id = 0; fan_id <= data->channel_count; fan_id++) {
 		fan = devm_kzalloc(&dev->udev->dev,
 				   sizeof(struct hwmon_fan_data), GFP_KERNEL);
@@ -648,8 +658,8 @@ void hwmon_deinit(struct driver *dev)
 }
 
 /*
-	Devices that work with this driver.
-	More devices should work, however none have been tested.
+ * Devices that work with this driver.
+ * More devices should work, however none have been tested.
 */
 static const struct usb_device_id astk_table[] = {
 	{ USB_DEVICE(0x1b1c, 0x0c15) },
@@ -674,6 +684,7 @@ int init_device(struct usb_device *udev)
 
 	return 0;
 }
+
 int deinit_device(struct usb_device *udev)
 {
 	int retval;
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/6] skeleton for asetek gen 6 driver
  2020-07-13 19:32 [PATCH 1/6] skeleton for asetek gen 6 driver jaap aarts
                   ` (4 preceding siblings ...)
  2020-07-13 19:32 ` [PATCH 6/6] added some comments and fixed some styling jaap aarts
@ 2020-07-14  4:59 ` Guenter Roeck
  2020-07-14 10:03   ` jaap aarts
  2020-07-14  5:19 ` Guenter Roeck
  2020-07-14  5:54 ` Greg KH
  7 siblings, 1 reply; 12+ messages in thread
From: Guenter Roeck @ 2020-07-14  4:59 UTC (permalink / raw)
  To: jaap aarts, Jean Delvare, linux-hwmon, linux-usb; +Cc: Jaap Aarts

On 7/13/20 12:32 PM, jaap aarts wrote:
> Signed-off-by: Jaap Aarts <jaap.aarts1@example.com>

I am not going to review code which is later changed in the
same patch series. Please combine all patches into one.

Guenter

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/6] skeleton for asetek gen 6 driver
  2020-07-13 19:32 [PATCH 1/6] skeleton for asetek gen 6 driver jaap aarts
                   ` (5 preceding siblings ...)
  2020-07-14  4:59 ` [PATCH 1/6] skeleton for asetek gen 6 driver Guenter Roeck
@ 2020-07-14  5:19 ` Guenter Roeck
  2020-07-14  5:54 ` Greg KH
  7 siblings, 0 replies; 12+ messages in thread
From: Guenter Roeck @ 2020-07-14  5:19 UTC (permalink / raw)
  To: jaap aarts, Jean Delvare, linux-hwmon, linux-usb; +Cc: Jaap Aarts

On 7/13/20 12:32 PM, jaap aarts wrote:
> Signed-off-by: Jaap Aarts <jaap.aarts1@example.com>

That e-mail address does not exist.

Guenter

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/6] skeleton for asetek gen 6 driver
  2020-07-13 19:32 [PATCH 1/6] skeleton for asetek gen 6 driver jaap aarts
                   ` (6 preceding siblings ...)
  2020-07-14  5:19 ` Guenter Roeck
@ 2020-07-14  5:54 ` Greg KH
  7 siblings, 0 replies; 12+ messages in thread
From: Greg KH @ 2020-07-14  5:54 UTC (permalink / raw)
  To: jaap aarts
  Cc: Jean Delvare, Guenter Roeck, linux-hwmon, linux-usb, Jaap Aarts

On Mon, Jul 13, 2020 at 09:32:22PM +0200, jaap aarts wrote:
> Signed-off-by: Jaap Aarts <jaap.aarts1@example.com>
> ---

I know I don't accept patches without any changelog text, and odds are,
most other maintainers do not either :(

Also, please fix up the subject lines of your patches to match the
subsystem it is being sent to.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/6] skeleton for asetek gen 6 driver
  2020-07-14  4:59 ` [PATCH 1/6] skeleton for asetek gen 6 driver Guenter Roeck
@ 2020-07-14 10:03   ` jaap aarts
  2020-07-14 17:30     ` Guenter Roeck
  0 siblings, 1 reply; 12+ messages in thread
From: jaap aarts @ 2020-07-14 10:03 UTC (permalink / raw)
  To: Guenter Roeck; +Cc: Jean Delvare, linux-hwmon, linux-usb, Jaap Aarts

On Tue, 14 Jul 2020 at 06:59, Guenter Roeck <linux@roeck-us.net> wrote:
>
> On 7/13/20 12:32 PM, jaap aarts wrote:
> > Signed-off-by: Jaap Aarts <jaap.aarts1@example.com>
>
> I am not going to review code which is later changed in the
> same patch series. Please combine all patches into one.
>
> Guenter

Thanks for the feedback, most guides state you should
split up your changes into smaller patches if they get very big.
Maybe I misunderstood the intent of that?
Anyways I combined all patches, fixed my signoff line, added
a changelog and fixed my subject line.

Thanks,

Jaap Aarts

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/6] skeleton for asetek gen 6 driver
  2020-07-14 10:03   ` jaap aarts
@ 2020-07-14 17:30     ` Guenter Roeck
  2020-07-14 17:43       ` jaap aarts
  0 siblings, 1 reply; 12+ messages in thread
From: Guenter Roeck @ 2020-07-14 17:30 UTC (permalink / raw)
  To: jaap aarts; +Cc: Jean Delvare, linux-hwmon, linux-usb, Jaap Aarts

On 7/14/20 3:03 AM, jaap aarts wrote:
> On Tue, 14 Jul 2020 at 06:59, Guenter Roeck <linux@roeck-us.net> wrote:
>>
>> On 7/13/20 12:32 PM, jaap aarts wrote:
>>> Signed-off-by: Jaap Aarts <jaap.aarts1@example.com>
>>
>> I am not going to review code which is later changed in the
>> same patch series. Please combine all patches into one.
>>
>> Guenter
> 
> Thanks for the feedback, most guides state you should
> split up your changes into smaller patches if they get very big.

Yes, but not if the later patches change the initial ones. this
is not the case here. Your first patch doesn't even register a
hwmon device. The last two patches change previous patches, meaning
I would just waste my time reviewing patches 1..4. Worse, all those
style issues fixed in the last patch would create so much noise that
I might miss real issues.

A split would have been fine if the first patch introduced working
code, then subsequent patches built on it without replacing previously
introduced code.

Guenter

> Maybe I misunderstood the intent of that?
> Anyways I combined all patches, fixed my signoff line, added
> a changelog and fixed my subject line.
> 
> Thanks,
> 
> Jaap Aarts
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/6] skeleton for asetek gen 6 driver
  2020-07-14 17:30     ` Guenter Roeck
@ 2020-07-14 17:43       ` jaap aarts
  0 siblings, 0 replies; 12+ messages in thread
From: jaap aarts @ 2020-07-14 17:43 UTC (permalink / raw)
  To: Guenter Roeck; +Cc: Jean Delvare, linux-hwmon, linux-usb, Jaap Aarts

On Tue, 14 Jul 2020 at 19:31, Guenter Roeck <linux@roeck-us.net> wrote:
>
> On 7/14/20 3:03 AM, jaap aarts wrote:
> > On Tue, 14 Jul 2020 at 06:59, Guenter Roeck <linux@roeck-us.net> wrote:
> >>
> >> On 7/13/20 12:32 PM, jaap aarts wrote:
> >>> Signed-off-by: Jaap Aarts <jaap.aarts1@example.com>
> >>
> >> I am not going to review code which is later changed in the
> >> same patch series. Please combine all patches into one.
> >>
> >> Guenter
> >
> > Thanks for the feedback, most guides state you should
> > split up your changes into smaller patches if they get very big.
>
> Yes, but not if the later patches change the initial ones. this
> is not the case here. Your first patch doesn't even register a
> hwmon device. The last two patches change previous patches, meaning
> I would just waste my time reviewing patches 1..4. Worse, all those
> style issues fixed in the last patch would create so much noise that
> I might miss real issues.

Ok thanks, for future patches I will keep this in mind. If this driver gets in
I will be adding other features. I will keep this in mind when submitting
those.
I send it as one patch,  it's quite big, I will split my patches more
appropriately next time.

> A split would have been fine if the first patch introduced working
> code, then subsequent patches built on it without replacing previously
> introduced code.
>
> Guenter
>
> > Maybe I misunderstood the intent of that?
> > Anyways I combined all patches, fixed my signoff line, added
> > a changelog and fixed my subject line.
> >
> > Thanks,
> >
> > Jaap Aarts
> >
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2020-07-14 17:43 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-13 19:32 [PATCH 1/6] skeleton for asetek gen 6 driver jaap aarts
2020-07-13 19:32 ` [PATCH 2/6] fan support " jaap aarts
2020-07-13 19:32 ` [PATCH 3/6] rpm " jaap aarts
2020-07-13 19:32 ` [PATCH 4/6] automatic fan count detection jaap aarts
2020-07-13 19:32 ` [PATCH 5/6] allow setting enable instead of mode, and support enable 2/3/4 jaap aarts
2020-07-13 19:32 ` [PATCH 6/6] added some comments and fixed some styling jaap aarts
2020-07-14  4:59 ` [PATCH 1/6] skeleton for asetek gen 6 driver Guenter Roeck
2020-07-14 10:03   ` jaap aarts
2020-07-14 17:30     ` Guenter Roeck
2020-07-14 17:43       ` jaap aarts
2020-07-14  5:19 ` Guenter Roeck
2020-07-14  5:54 ` Greg KH

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).