This changes corsair-cpro to a hid driver using hid reports.

Signed-off-by: Marius Zachmann <m...@mariuszachmann.de>

---
Changes from v1
- add comment at the beginning about using hid and hidraw
- use hwmon_device_register_with_info and hwmon_device_unregister
- use a define for timeout

---
 drivers/hid/hid-quirks.c     |   2 -
 drivers/hwmon/Kconfig        |   2 +-
 drivers/hwmon/corsair-cpro.c | 124 ++++++++++++++++++++++++++---------
 3 files changed, 95 insertions(+), 35 deletions(-)

diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
index 7b7bc7737c53..ca8b5c261c7c 100644
--- a/drivers/hid/hid-quirks.c
+++ b/drivers/hid/hid-quirks.c
@@ -699,8 +699,6 @@ static const struct hid_device_id hid_ignore_list[] = {
        { HID_USB_DEVICE(USB_VENDOR_ID_AXENTIA, USB_DEVICE_ID_AXENTIA_FM_RADIO) 
},
        { HID_USB_DEVICE(USB_VENDOR_ID_BERKSHIRE, USB_DEVICE_ID_BERKSHIRE_PCWD) 
},
        { HID_USB_DEVICE(USB_VENDOR_ID_CIDC, 0x0103) },
-       { HID_USB_DEVICE(USB_VENDOR_ID_CORSAIR, 0x0c10) },
-       { HID_USB_DEVICE(USB_VENDOR_ID_CORSAIR, 0x1d00) },
        { HID_USB_DEVICE(USB_VENDOR_ID_CYGNAL, 
USB_DEVICE_ID_CYGNAL_RADIO_SI470X) },
        { HID_USB_DEVICE(USB_VENDOR_ID_CYGNAL, 
USB_DEVICE_ID_CYGNAL_RADIO_SI4713) },
        { HID_USB_DEVICE(USB_VENDOR_ID_CMEDIA, USB_DEVICE_ID_CM109) },
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 8b046a5dfa40..c603d8c8e3d2 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -441,7 +441,7 @@ config SENSORS_BT1_PVT_ALARMS

 config SENSORS_CORSAIR_CPRO
        tristate "Corsair Commander Pro controller"
-       depends on USB
+       depends on HID
        help
          If you say yes here you get support for the Corsair Commander Pro
          controller.
diff --git a/drivers/hwmon/corsair-cpro.c b/drivers/hwmon/corsair-cpro.c
index fe625190e3a1..ba378574b073 100644
--- a/drivers/hwmon/corsair-cpro.c
+++ b/drivers/hwmon/corsair-cpro.c
@@ -2,16 +2,21 @@
 /*
  * corsair-cpro.c - Linux driver for Corsair Commander Pro
  * Copyright (C) 2020 Marius Zachmann <m...@mariuszachmann.de>
+ *
+ * This driver uses hid reports to communicate with the device to allow hidraw 
userspace drivers
+ * still being used. The device does not use report ids. When using hidraw and 
this driver
+ * simultaniously, reports could be switched.
  */

 #include <linux/bitops.h>
+#include <linux/completion.h>
+#include <linux/hid.h>
 #include <linux/hwmon.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/slab.h>
 #include <linux/types.h>
-#include <linux/usb.h>

 #define USB_VENDOR_ID_CORSAIR                  0x1b1c
 #define USB_PRODUCT_ID_CORSAIR_COMMANDERPRO    0x0c10
@@ -20,6 +25,7 @@
 #define OUT_BUFFER_SIZE                63
 #define IN_BUFFER_SIZE         16
 #define LABEL_LENGTH           11
+#define REQ_TIMEOUT            300

 #define CTL_GET_TMP_CNCT       0x10    /*
                                         * returns in bytes 1-4 for each temp 
sensor:
@@ -62,7 +68,9 @@
 #define NUM_TEMP_SENSORS       4

 struct ccp_device {
-       struct usb_device *udev;
+       struct hid_device *hdev;
+       struct device *hwmon_dev;
+       struct completion wait_input_report;
        struct mutex mutex; /* whenever buffer is used, lock before 
send_usb_cmd */
        u8 *buffer;
        int pwm[6];
@@ -75,7 +83,7 @@ struct ccp_device {
 /* send command, check for error in response, response in ccp->buffer */
 static int send_usb_cmd(struct ccp_device *ccp, u8 command, u8 byte1, u8 
byte2, u8 byte3)
 {
-       int actual_length;
+       unsigned long t;
        int ret;

        memset(ccp->buffer, 0x00, OUT_BUFFER_SIZE);
@@ -84,26 +92,39 @@ static int send_usb_cmd(struct ccp_device *ccp, u8 command, 
u8 byte1, u8 byte2,
        ccp->buffer[2] = byte2;
        ccp->buffer[3] = byte3;

-       ret = usb_bulk_msg(ccp->udev, usb_sndintpipe(ccp->udev, 2), 
ccp->buffer, OUT_BUFFER_SIZE,
-                          &actual_length, 1000);
-       if (ret)
-               return ret;
+       reinit_completion(&ccp->wait_input_report);

-       /* response needs to be received every time */
-       ret = usb_bulk_msg(ccp->udev, usb_rcvintpipe(ccp->udev, 1), 
ccp->buffer, IN_BUFFER_SIZE,
-                          &actual_length, 1000);
-       if (ret)
+       ret = hid_hw_output_report(ccp->hdev, ccp->buffer, OUT_BUFFER_SIZE);
+       if (ret < 0)
                return ret;

+       t = wait_for_completion_timeout(&ccp->wait_input_report, 
msecs_to_jiffies(REQ_TIMEOUT));
+       if (!t)
+               return -ETIMEDOUT;
+
        /* first byte of response is error code */
        if (ccp->buffer[0] != 0x00) {
-               dev_dbg(&ccp->udev->dev, "device response error: %d", 
ccp->buffer[0]);
+               hid_dbg(ccp->hdev, "device response error: %d", ccp->buffer[0]);
                return -EIO;
        }

        return 0;
 }

+static int ccp_raw_event(struct hid_device *hdev, struct hid_report *report, 
u8 *data, int size)
+{
+       struct ccp_device *ccp = hid_get_drvdata(hdev);
+
+       /* only copy buffer when requested */
+       if (completion_done(&ccp->wait_input_report))
+               return 0;
+
+       memcpy(ccp->buffer, data, min(IN_BUFFER_SIZE, size));
+       complete(&ccp->wait_input_report);
+
+       return 0;
+}
+
 /* requests and returns single data values depending on channel */
 static int get_data(struct ccp_device *ccp, int command, int channel)
 {
@@ -437,57 +458,98 @@ static int get_temp_cnct(struct ccp_device *ccp)
        return 0;
 }

-static int ccp_probe(struct usb_interface *intf, const struct usb_device_id 
*id)
+static int ccp_probe(struct hid_device *hdev, const struct hid_device_id *id)
 {
-       struct device *hwmon_dev;
        struct ccp_device *ccp;
        int ret;

-       ccp = devm_kzalloc(&intf->dev, sizeof(*ccp), GFP_KERNEL);
+       ccp = devm_kzalloc(&hdev->dev, sizeof(*ccp), GFP_KERNEL);
        if (!ccp)
                return -ENOMEM;

-       ccp->buffer = devm_kmalloc(&intf->dev, OUT_BUFFER_SIZE, GFP_KERNEL);
+       ccp->buffer = devm_kmalloc(&hdev->dev, OUT_BUFFER_SIZE, GFP_KERNEL);
        if (!ccp->buffer)
                return -ENOMEM;

+       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 out_hw_stop;
+
+       ccp->hdev = hdev;
+       hid_set_drvdata(hdev, ccp);
        mutex_init(&ccp->mutex);
+       init_completion(&ccp->wait_input_report);

-       ccp->udev = interface_to_usbdev(intf);
+       hid_device_io_start(hdev);

        /* temp and fan connection status only updates when device is powered 
on */
        ret = get_temp_cnct(ccp);
        if (ret)
-               return ret;
+               goto out_hw_close;

        ret = get_fan_cnct(ccp);
        if (ret)
-               return ret;
+               goto out_hw_close;
+       ccp->hwmon_dev = hwmon_device_register_with_info(&hdev->dev, 
"corsaircpro",
+                                                        ccp, &ccp_chip_info, 
0);
+       if (IS_ERR(ccp->hwmon_dev)) {
+               ret = PTR_ERR(ccp->hwmon_dev);
+               goto out_hw_close;
+       }

-       hwmon_dev = devm_hwmon_device_register_with_info(&intf->dev, 
"corsaircpro", ccp,
-                                                        &ccp_chip_info, 0);
+       return 0;

-       return PTR_ERR_OR_ZERO(hwmon_dev);
+out_hw_close:
+       hid_hw_close(hdev);
+out_hw_stop:
+       hid_hw_stop(hdev);
+       return ret;
 }

-static void ccp_disconnect(struct usb_interface *intf)
+static void ccp_remove(struct hid_device *hdev)
 {
+       struct ccp_device *ccp = hid_get_drvdata(hdev);
+
+       hwmon_device_unregister(ccp->hwmon_dev);
+       hid_hw_close(hdev);
+       hid_hw_stop(hdev);
 }

-static const struct usb_device_id ccp_devices[] = {
-       { USB_DEVICE(USB_VENDOR_ID_CORSAIR, 
USB_PRODUCT_ID_CORSAIR_COMMANDERPRO) },
-       { USB_DEVICE(USB_VENDOR_ID_CORSAIR, USB_PRODUCT_ID_CORSAIR_1000D) },
+static const struct hid_device_id ccp_devices[] = {
+       { HID_USB_DEVICE(USB_VENDOR_ID_CORSAIR, 
USB_PRODUCT_ID_CORSAIR_COMMANDERPRO) },
+       { HID_USB_DEVICE(USB_VENDOR_ID_CORSAIR, USB_PRODUCT_ID_CORSAIR_1000D) },
        { }
 };

-static struct usb_driver ccp_driver = {
+static struct hid_driver ccp_driver = {
        .name = "corsair-cpro",
+       .id_table = ccp_devices,
        .probe = ccp_probe,
-       .disconnect = ccp_disconnect,
-       .id_table = ccp_devices
+       .remove = ccp_remove,
+       .raw_event = ccp_raw_event,
 };

-MODULE_DEVICE_TABLE(usb, ccp_devices);
+MODULE_DEVICE_TABLE(hid, ccp_devices);
 MODULE_LICENSE("GPL");

-module_usb_driver(ccp_driver);
+static int __init ccp_init(void)
+{
+       return hid_register_driver(&ccp_driver);
+}
+
+static void __exit ccp_exit(void)
+{
+       hid_unregister_driver(&ccp_driver);
+}
+
+/* make sure it is loaded after hid */
+late_initcall(ccp_init);
+module_exit(ccp_exit);
--
2.27.0

Reply via email to