diff mbox series

[1/6] skeleton for asetek gen 6 driver

Message ID 20200713193227.189751-1-jaap.aarts1@gmail.com (mailing list archive)
State New, archived
Headers show
Series [1/6] skeleton for asetek gen 6 driver | expand

Commit Message

jaap aarts July 13, 2020, 7:32 p.m. UTC
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

Comments

Guenter Roeck July 14, 2020, 4:59 a.m. UTC | #1
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
Guenter Roeck July 14, 2020, 5:19 a.m. UTC | #2
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
Greg Kroah-Hartman July 14, 2020, 5:54 a.m. UTC | #3
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
jaap aarts July 14, 2020, 10:03 a.m. UTC | #4
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
Guenter Roeck July 14, 2020, 5:30 p.m. UTC | #5
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
>
jaap aarts July 14, 2020, 5:43 p.m. UTC | #6
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
> >
>
diff mbox series

Patch

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");