* [PATCH 1/2] hid: migrate USB LED driver from usb misc to hid
@ 2016-06-17 6:07 Heiner Kallweit
[not found] ` <8413318b-cd83-764e-808a-0c060a15a757-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
0 siblings, 1 reply; 3+ messages in thread
From: Heiner Kallweit @ 2016-06-17 6:07 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Greg Kroah-Hartman, Jiri Kosina, linux-input,
Linux USB Mailing List
This patch migrates the USB LED driver to the HID subsystem.
Supported are Dream Cheeky Webmail Notifier / Friends Alert
and Riso Kagaku Webmail Notifier.
Benefits:
- Avoid using USB low-level calls and use the HID subsystem instead
(as this device provides a USB HID interface)
- Use standard LED subsystem instead of proprietary sysfs entries,
this allows e.g. to use the device with features like triggers
Successfully tested with a Dream Cheeky Webmail Notifier and a
Riso Kagaku Webmail Notifier compatible device.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/hid/Kconfig | 12 ++
drivers/hid/Makefile | 1 +
drivers/hid/hid-core.c | 6 +-
drivers/hid/hid-ids.h | 2 +
drivers/hid/hid-usb-led.c | 276 ++++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 294 insertions(+), 3 deletions(-)
create mode 100644 drivers/hid/hid-usb-led.c
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 5646ca4..adc9ce6 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -841,6 +841,18 @@ config THRUSTMASTER_FF
a THRUSTMASTER Dual Trigger 3-in-1 or a THRUSTMASTER Ferrari GT
Rumble Force or Force Feedback Wheel.
+config HID_USB_LED
+ tristate "Simple USB RGB LED support"
+ depends on HID
+ depends on LEDS_CLASS
+ ---help---
+ Support for simple USB RGB LED devices. Currently supported are the
+ Riso Kagaku Webmail Notifier and the Dream Cheeky Webmail Notifier
+ and Friends Alert.
+
+ To compile this driver as a module, choose M here: the
+ module will be called hid-usb-led.
+
config HID_WACOM
tristate "Wacom Intuos/Graphire tablet support (USB)"
depends on HID
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index a2fb562..3014bdd 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -96,6 +96,7 @@ obj-$(CONFIG_HID_TIVO) += hid-tivo.o
obj-$(CONFIG_HID_TOPSEED) += hid-topseed.o
obj-$(CONFIG_HID_TWINHAN) += hid-twinhan.o
obj-$(CONFIG_HID_UCLOGIC) += hid-uclogic.o
+obj-$(CONFIG_HID_USB_LED) += hid-usb-led.o
obj-$(CONFIG_HID_XINMO) += hid-xinmo.o
obj-$(CONFIG_HID_ZEROPLUS) += hid-zpff.o
obj-$(CONFIG_HID_ZYDACRON) += hid-zydacron.o
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 8ea3a26..eb674ce 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1879,6 +1879,8 @@ static const struct hid_device_id hid_have_special_driver[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_CYPRESS, USB_DEVICE_ID_CYPRESS_MOUSE) },
{ HID_USB_DEVICE(USB_VENDOR_ID_DRAGONRISE, 0x0006) },
{ HID_USB_DEVICE(USB_VENDOR_ID_DRAGONRISE, 0x0011) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_DREAM_CHEEKY, USB_DEVICE_ID_DREAM_CHEEKY_WN) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_DREAM_CHEEKY, USB_DEVICE_ID_DREAM_CHEEKY_FA) },
{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_ELECOM, USB_DEVICE_ID_ELECOM_BM084) },
{ HID_USB_DEVICE(USB_VENDOR_ID_ELO, 0x0009) },
{ HID_USB_DEVICE(USB_VENDOR_ID_ELO, 0x0030) },
@@ -2008,6 +2010,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_PETALYNX, USB_DEVICE_ID_PETALYNX_MAXTER_REMOTE) },
{ HID_USB_DEVICE(USB_VENDOR_ID_PLANTRONICS, HID_ANY_ID) },
{ HID_USB_DEVICE(USB_VENDOR_ID_PRIMAX, USB_DEVICE_ID_PRIMAX_KEYBOARD) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_RISO_KAGAKU, USB_DEVICE_ID_RI_KA_WEBMAIL) },
#if IS_ENABLED(CONFIG_HID_ROCCAT)
{ HID_USB_DEVICE(USB_VENDOR_ID_ROCCAT, USB_DEVICE_ID_ROCCAT_ARVO) },
{ HID_USB_DEVICE(USB_VENDOR_ID_ROCCAT, USB_DEVICE_ID_ROCCAT_ISKU) },
@@ -2348,8 +2351,6 @@ static const struct hid_device_id hid_ignore_list[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_DEALEXTREAME, USB_DEVICE_ID_DEALEXTREAME_RADIO_SI4701) },
{ HID_USB_DEVICE(USB_VENDOR_ID_DELORME, USB_DEVICE_ID_DELORME_EARTHMATE) },
{ HID_USB_DEVICE(USB_VENDOR_ID_DELORME, USB_DEVICE_ID_DELORME_EM_LT20) },
- { HID_USB_DEVICE(USB_VENDOR_ID_DREAM_CHEEKY, 0x0004) },
- { HID_USB_DEVICE(USB_VENDOR_ID_DREAM_CHEEKY, 0x000a) },
{ HID_I2C_DEVICE(USB_VENDOR_ID_ELAN, 0x0400) },
{ HID_I2C_DEVICE(USB_VENDOR_ID_ELAN, 0x0401) },
{ HID_USB_DEVICE(USB_VENDOR_ID_ESSENTIAL_REALITY, USB_DEVICE_ID_ESSENTIAL_REALITY_P5) },
@@ -2486,7 +2487,6 @@ static const struct hid_device_id hid_ignore_list[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_SYNAPTICS, USB_DEVICE_ID_SYNAPTICS_DPAD) },
#endif
{ HID_USB_DEVICE(USB_VENDOR_ID_YEALINK, USB_DEVICE_ID_YEALINK_P1K_P4K_B2K) },
- { HID_USB_DEVICE(USB_VENDOR_ID_RISO_KAGAKU, USB_DEVICE_ID_RI_KA_WEBMAIL) },
{ }
};
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 3eec09a..e104aba 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -334,6 +334,8 @@
#define USB_DEVICE_ID_ELECOM_BM084 0x0061
#define USB_VENDOR_ID_DREAM_CHEEKY 0x1d34
+#define USB_DEVICE_ID_DREAM_CHEEKY_WN 0x0004
+#define USB_DEVICE_ID_DREAM_CHEEKY_FA 0x000a
#define USB_VENDOR_ID_ELITEGROUP 0x03fc
#define USB_DEVICE_ID_ELITEGROUP_05D8 0x05d8
diff --git a/drivers/hid/hid-usb-led.c b/drivers/hid/hid-usb-led.c
new file mode 100644
index 0000000..c7f5a62
--- /dev/null
+++ b/drivers/hid/hid-usb-led.c
@@ -0,0 +1,276 @@
+/*
+ * Simple USB RGB LED driver
+ *
+ * Copyright 2016 Heiner Kallweit <hkallweit1@gmail.com>
+ * Based on drivers/hid/hid-thingm.c and
+ * drivers/usb/misc/usbled.c
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation, version 2.
+ */
+
+#include <linux/hid.h>
+#include <linux/hidraw.h>
+#include <linux/leds.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+
+#include "hid-ids.h"
+
+enum led_report_type {
+ RAW_REQUEST,
+ OUTPUT_REPORT
+};
+
+static unsigned const char riso_kagaku_tbl[] = {
+/* R+2G+4B -> riso kagaku color index */
+ [0] = 0, /* black */
+ [1] = 2, /* red */
+ [2] = 1, /* green */
+ [3] = 5, /* yellow */
+ [4] = 3, /* blue */
+ [5] = 6, /* magenta */
+ [6] = 4, /* cyan */
+ [7] = 7 /* white */
+};
+
+#define RISO_KAGAKU_IX(r, g, b) riso_kagaku_tbl[((r)?1:0)+((g)?2:0)+((b)?4:0)]
+
+struct usbled_device;
+
+struct usbled_type {
+ const char *name;
+ const char *short_name;
+ enum led_brightness max_brightness;
+ size_t report_size;
+ enum led_report_type report_type;
+ u8 report_id;
+ int (*init)(struct usbled_device *udev);
+ int (*write)(struct led_classdev *cdev, enum led_brightness br);
+};
+
+struct usbled_led {
+ struct led_classdev cdev;
+ struct usbled_device *udev;
+ char name[32];
+};
+
+struct usbled_device {
+ const struct usbled_type *type;
+ struct usbled_led red;
+ struct usbled_led green;
+ struct usbled_led blue;
+ struct hid_device *hdev;
+ struct mutex lock;
+};
+
+#define MAX_REPORT_SIZE 16
+
+#define to_usbled_led(arg) container_of(arg, struct usbled_led, cdev)
+
+static bool riso_kagaku_switch_green_blue;
+module_param(riso_kagaku_switch_green_blue, bool, S_IRUGO | S_IWUSR);
+MODULE_PARM_DESC(riso_kagaku_switch_green_blue,
+ "switch green and blue RGB component for Riso Kagaku devices");
+
+static int usbled_send(struct usbled_device *udev, __u8 *buf)
+{
+ int ret;
+
+ buf[0] = udev->type->report_id;
+
+ mutex_lock(&udev->lock);
+
+ if (udev->type->report_type == RAW_REQUEST)
+ ret = hid_hw_raw_request(udev->hdev, buf[0], buf,
+ udev->type->report_size,
+ HID_FEATURE_REPORT,
+ HID_REQ_SET_REPORT);
+ else if (udev->type->report_type == OUTPUT_REPORT)
+ ret = hid_hw_output_report(udev->hdev, buf,
+ udev->type->report_size);
+ else
+ ret = -EINVAL;
+
+ mutex_unlock(&udev->lock);
+
+ if (ret < 0)
+ return ret;
+
+ return ret == udev->type->report_size ? 0 : -EMSGSIZE;
+}
+
+static u8 riso_kagaku_index(struct usbled_device *udev)
+{
+ enum led_brightness r, g, b;
+
+ r = udev->red.cdev.brightness;
+ g = udev->green.cdev.brightness;
+ b = udev->blue.cdev.brightness;
+
+ if (riso_kagaku_switch_green_blue)
+ return RISO_KAGAKU_IX(r, b, g);
+ else
+ return RISO_KAGAKU_IX(r, g, b);
+}
+
+static int riso_kagaku_write(struct led_classdev *cdev, enum led_brightness br)
+{
+ struct usbled_led *uled = to_usbled_led(cdev);
+ struct usbled_device *udev = uled->udev;
+ __u8 buf[MAX_REPORT_SIZE] = {};
+
+ buf[1] = riso_kagaku_index(udev);
+
+ return usbled_send(udev, buf);
+}
+
+static const struct usbled_type usbled_riso_kagaku = {
+ .name = "Riso Kagaku Webmail Notifier",
+ .short_name = "riso_kagaku",
+ .max_brightness = 1,
+ .report_size = 6,
+ .report_type = OUTPUT_REPORT,
+ .report_id = 0,
+ .write = riso_kagaku_write,
+};
+
+static int dream_cheeky_write(struct led_classdev *cdev, enum led_brightness br)
+{
+ struct usbled_led *uled = to_usbled_led(cdev);
+ struct usbled_device *udev = uled->udev;
+ __u8 buf[MAX_REPORT_SIZE] = {};
+
+ buf[1] = udev->red.cdev.brightness;
+ buf[2] = udev->green.cdev.brightness;
+ buf[3] = udev->blue.cdev.brightness;
+ buf[7] = 0x1a;
+ buf[8] = 0x05;
+
+ return usbled_send(udev, buf);
+}
+
+static int dream_cheeky_init(struct usbled_device *udev)
+{
+ __u8 buf[MAX_REPORT_SIZE] = {};
+
+ /* Dream Cheeky magic */
+ buf[1] = 0x1f;
+ buf[2] = 0x02;
+ buf[4] = 0x5f;
+ buf[7] = 0x1a;
+ buf[8] = 0x03;
+
+ return usbled_send(udev, buf);
+}
+
+static const struct usbled_type usbled_dream_cheeky = {
+ .name = "Dream Cheeky Webmail Notifier",
+ .short_name = "dream_cheeky",
+ .max_brightness = 31,
+ .report_size = 9,
+ .report_type = RAW_REQUEST,
+ .report_id = 0,
+ .init = dream_cheeky_init,
+ .write = dream_cheeky_write,
+};
+
+static int usbled_init_led(struct usbled_led *led, const char *color_name,
+ struct usbled_device *udev, unsigned int minor)
+{
+ snprintf(led->name, sizeof(led->name), "%s%u:%s",
+ udev->type->short_name, minor, color_name);
+ led->cdev.name = led->name;
+ led->cdev.max_brightness = udev->type->max_brightness;
+ led->cdev.brightness_set_blocking = udev->type->write;
+ led->cdev.flags = LED_HW_PLUGGABLE;
+ led->udev = udev;
+
+ return devm_led_classdev_register(&udev->hdev->dev, &led->cdev);
+}
+
+static int usbled_init_rgb(struct usbled_device *udev, unsigned int minor)
+{
+ int ret;
+
+ /* Register the red diode */
+ ret = usbled_init_led(&udev->red, "red", udev, minor);
+ if (ret)
+ return ret;
+
+ /* Register the green diode */
+ ret = usbled_init_led(&udev->green, "green", udev, minor);
+ if (ret)
+ return ret;
+
+ /* Register the blue diode */
+ return usbled_init_led(&udev->blue, "blue", udev, minor);
+}
+
+static int usbled_probe(struct hid_device *hdev, const struct hid_device_id *id)
+{
+ struct usbled_device *udev;
+ unsigned int minor;
+ int ret;
+
+ udev = devm_kzalloc(&hdev->dev, sizeof(*udev), GFP_KERNEL);
+ if (!udev)
+ return -ENOMEM;
+
+ ret = hid_parse(hdev);
+ if (ret)
+ return ret;
+
+ udev->hdev = hdev;
+ udev->type = (const struct usbled_type *)id->driver_data;
+ mutex_init(&udev->lock);
+
+ if (udev->type->init) {
+ ret = udev->type->init(udev);
+ if (ret)
+ return ret;
+ }
+
+ ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
+ if (ret)
+ return ret;
+
+ minor = ((struct hidraw *) hdev->hidraw)->minor;
+
+ ret = usbled_init_rgb(udev, minor);
+ if (ret) {
+ hid_hw_stop(hdev);
+ return ret;
+ }
+
+ dev_info(&hdev->dev, "%s initialized\n", udev->type->name);
+
+ return 0;
+}
+
+static const struct hid_device_id usbled_table[] = {
+ { HID_USB_DEVICE(USB_VENDOR_ID_RISO_KAGAKU,
+ USB_DEVICE_ID_RI_KA_WEBMAIL),
+ .driver_data = (kernel_ulong_t)&usbled_riso_kagaku },
+ { HID_USB_DEVICE(USB_VENDOR_ID_DREAM_CHEEKY,
+ USB_DEVICE_ID_DREAM_CHEEKY_WN),
+ .driver_data = (kernel_ulong_t)&usbled_dream_cheeky },
+ { HID_USB_DEVICE(USB_VENDOR_ID_DREAM_CHEEKY,
+ USB_DEVICE_ID_DREAM_CHEEKY_FA),
+ .driver_data = (kernel_ulong_t)&usbled_dream_cheeky },
+ { }
+};
+MODULE_DEVICE_TABLE(hid, usbled_table);
+
+static struct hid_driver usbled_driver = {
+ .name = "usb-led",
+ .probe = usbled_probe,
+ .id_table = usbled_table,
+};
+
+module_hid_driver(usbled_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Heiner Kallweit <hkallweit1@gmail.com>");
+MODULE_DESCRIPTION("Simple USB RGB LED driver");
--
2.8.3
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH 1/2] hid: migrate USB LED driver from usb misc to hid
[not found] ` <8413318b-cd83-764e-808a-0c060a15a757-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2016-06-17 8:53 ` Benjamin Tissoires
2016-06-17 17:28 ` Heiner Kallweit
0 siblings, 1 reply; 3+ messages in thread
From: Benjamin Tissoires @ 2016-06-17 8:53 UTC (permalink / raw)
To: Heiner Kallweit
Cc: Greg Kroah-Hartman, Jiri Kosina,
linux-input-u79uwXL29TY76Z2rM5mHXA, Linux USB Mailing List
Hi Heiner,
On Jun 17 2016 or thereabouts, Heiner Kallweit wrote:
> This patch migrates the USB LED driver to the HID subsystem.
> Supported are Dream Cheeky Webmail Notifier / Friends Alert
> and Riso Kagaku Webmail Notifier.
>
> Benefits:
> - Avoid using USB low-level calls and use the HID subsystem instead
> (as this device provides a USB HID interface)
> - Use standard LED subsystem instead of proprietary sysfs entries,
> this allows e.g. to use the device with features like triggers
>
> Successfully tested with a Dream Cheeky Webmail Notifier and a
> Riso Kagaku Webmail Notifier compatible device.
>
> Signed-off-by: Heiner Kallweit <hkallweit1-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Looks good, though I have a few comments (sorry, again...)
> ---
> drivers/hid/Kconfig | 12 ++
> drivers/hid/Makefile | 1 +
> drivers/hid/hid-core.c | 6 +-
> drivers/hid/hid-ids.h | 2 +
> drivers/hid/hid-usb-led.c | 276 ++++++++++++++++++++++++++++++++++++++++++++++
There is nothing USB related now. How about just calling it hid-led.c?
> 5 files changed, 294 insertions(+), 3 deletions(-)
> create mode 100644 drivers/hid/hid-usb-led.c
>
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 5646ca4..adc9ce6 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -841,6 +841,18 @@ config THRUSTMASTER_FF
> a THRUSTMASTER Dual Trigger 3-in-1 or a THRUSTMASTER Ferrari GT
> Rumble Force or Force Feedback Wheel.
>
> +config HID_USB_LED
> + tristate "Simple USB RGB LED support"
> + depends on HID
> + depends on LEDS_CLASS
> + ---help---
> + Support for simple USB RGB LED devices. Currently supported are the
> + Riso Kagaku Webmail Notifier and the Dream Cheeky Webmail Notifier
> + and Friends Alert.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called hid-usb-led.
> +
> config HID_WACOM
> tristate "Wacom Intuos/Graphire tablet support (USB)"
> depends on HID
> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> index a2fb562..3014bdd 100644
> --- a/drivers/hid/Makefile
> +++ b/drivers/hid/Makefile
> @@ -96,6 +96,7 @@ obj-$(CONFIG_HID_TIVO) += hid-tivo.o
> obj-$(CONFIG_HID_TOPSEED) += hid-topseed.o
> obj-$(CONFIG_HID_TWINHAN) += hid-twinhan.o
> obj-$(CONFIG_HID_UCLOGIC) += hid-uclogic.o
> +obj-$(CONFIG_HID_USB_LED) += hid-usb-led.o
> obj-$(CONFIG_HID_XINMO) += hid-xinmo.o
> obj-$(CONFIG_HID_ZEROPLUS) += hid-zpff.o
> obj-$(CONFIG_HID_ZYDACRON) += hid-zydacron.o
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index 8ea3a26..eb674ce 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -1879,6 +1879,8 @@ static const struct hid_device_id hid_have_special_driver[] = {
> { HID_USB_DEVICE(USB_VENDOR_ID_CYPRESS, USB_DEVICE_ID_CYPRESS_MOUSE) },
> { HID_USB_DEVICE(USB_VENDOR_ID_DRAGONRISE, 0x0006) },
> { HID_USB_DEVICE(USB_VENDOR_ID_DRAGONRISE, 0x0011) },
> + { HID_USB_DEVICE(USB_VENDOR_ID_DREAM_CHEEKY, USB_DEVICE_ID_DREAM_CHEEKY_WN) },
> + { HID_USB_DEVICE(USB_VENDOR_ID_DREAM_CHEEKY, USB_DEVICE_ID_DREAM_CHEEKY_FA) },
> { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_ELECOM, USB_DEVICE_ID_ELECOM_BM084) },
> { HID_USB_DEVICE(USB_VENDOR_ID_ELO, 0x0009) },
> { HID_USB_DEVICE(USB_VENDOR_ID_ELO, 0x0030) },
> @@ -2008,6 +2010,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
> { HID_USB_DEVICE(USB_VENDOR_ID_PETALYNX, USB_DEVICE_ID_PETALYNX_MAXTER_REMOTE) },
> { HID_USB_DEVICE(USB_VENDOR_ID_PLANTRONICS, HID_ANY_ID) },
> { HID_USB_DEVICE(USB_VENDOR_ID_PRIMAX, USB_DEVICE_ID_PRIMAX_KEYBOARD) },
> + { HID_USB_DEVICE(USB_VENDOR_ID_RISO_KAGAKU, USB_DEVICE_ID_RI_KA_WEBMAIL) },
> #if IS_ENABLED(CONFIG_HID_ROCCAT)
> { HID_USB_DEVICE(USB_VENDOR_ID_ROCCAT, USB_DEVICE_ID_ROCCAT_ARVO) },
> { HID_USB_DEVICE(USB_VENDOR_ID_ROCCAT, USB_DEVICE_ID_ROCCAT_ISKU) },
> @@ -2348,8 +2351,6 @@ static const struct hid_device_id hid_ignore_list[] = {
> { HID_USB_DEVICE(USB_VENDOR_ID_DEALEXTREAME, USB_DEVICE_ID_DEALEXTREAME_RADIO_SI4701) },
> { HID_USB_DEVICE(USB_VENDOR_ID_DELORME, USB_DEVICE_ID_DELORME_EARTHMATE) },
> { HID_USB_DEVICE(USB_VENDOR_ID_DELORME, USB_DEVICE_ID_DELORME_EM_LT20) },
> - { HID_USB_DEVICE(USB_VENDOR_ID_DREAM_CHEEKY, 0x0004) },
> - { HID_USB_DEVICE(USB_VENDOR_ID_DREAM_CHEEKY, 0x000a) },
> { HID_I2C_DEVICE(USB_VENDOR_ID_ELAN, 0x0400) },
> { HID_I2C_DEVICE(USB_VENDOR_ID_ELAN, 0x0401) },
> { HID_USB_DEVICE(USB_VENDOR_ID_ESSENTIAL_REALITY, USB_DEVICE_ID_ESSENTIAL_REALITY_P5) },
> @@ -2486,7 +2487,6 @@ static const struct hid_device_id hid_ignore_list[] = {
> { HID_USB_DEVICE(USB_VENDOR_ID_SYNAPTICS, USB_DEVICE_ID_SYNAPTICS_DPAD) },
> #endif
> { HID_USB_DEVICE(USB_VENDOR_ID_YEALINK, USB_DEVICE_ID_YEALINK_P1K_P4K_B2K) },
> - { HID_USB_DEVICE(USB_VENDOR_ID_RISO_KAGAKU, USB_DEVICE_ID_RI_KA_WEBMAIL) },
> { }
> };
>
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 3eec09a..e104aba 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -334,6 +334,8 @@
> #define USB_DEVICE_ID_ELECOM_BM084 0x0061
>
> #define USB_VENDOR_ID_DREAM_CHEEKY 0x1d34
> +#define USB_DEVICE_ID_DREAM_CHEEKY_WN 0x0004
> +#define USB_DEVICE_ID_DREAM_CHEEKY_FA 0x000a
>
> #define USB_VENDOR_ID_ELITEGROUP 0x03fc
> #define USB_DEVICE_ID_ELITEGROUP_05D8 0x05d8
> diff --git a/drivers/hid/hid-usb-led.c b/drivers/hid/hid-usb-led.c
> new file mode 100644
> index 0000000..c7f5a62
> --- /dev/null
> +++ b/drivers/hid/hid-usb-led.c
> @@ -0,0 +1,276 @@
> +/*
> + * Simple USB RGB LED driver
> + *
> + * Copyright 2016 Heiner Kallweit <hkallweit1-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> + * Based on drivers/hid/hid-thingm.c and
> + * drivers/usb/misc/usbled.c
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation, version 2.
> + */
> +
> +#include <linux/hid.h>
> +#include <linux/hidraw.h>
> +#include <linux/leds.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +
> +#include "hid-ids.h"
> +
> +enum led_report_type {
> + RAW_REQUEST,
> + OUTPUT_REPORT
> +};
> +
> +static unsigned const char riso_kagaku_tbl[] = {
> +/* R+2G+4B -> riso kagaku color index */
> + [0] = 0, /* black */
> + [1] = 2, /* red */
> + [2] = 1, /* green */
> + [3] = 5, /* yellow */
> + [4] = 3, /* blue */
> + [5] = 6, /* magenta */
> + [6] = 4, /* cyan */
> + [7] = 7 /* white */
> +};
> +
> +#define RISO_KAGAKU_IX(r, g, b) riso_kagaku_tbl[((r)?1:0)+((g)?2:0)+((b)?4:0)]
> +
> +struct usbled_device;
I'd say remove all "usb" strings from the driver and use hled or hidled instead
of usbled.
> +
> +struct usbled_type {
> + const char *name;
> + const char *short_name;
> + enum led_brightness max_brightness;
> + size_t report_size;
> + enum led_report_type report_type;
> + u8 report_id;
> + int (*init)(struct usbled_device *udev);
> + int (*write)(struct led_classdev *cdev, enum led_brightness br);
> +};
> +
> +struct usbled_led {
> + struct led_classdev cdev;
> + struct usbled_device *udev;
> + char name[32];
> +};
> +
> +struct usbled_device {
> + const struct usbled_type *type;
> + struct usbled_led red;
> + struct usbled_led green;
> + struct usbled_led blue;
> + struct hid_device *hdev;
> + struct mutex lock;
> +};
> +
> +#define MAX_REPORT_SIZE 16
> +
> +#define to_usbled_led(arg) container_of(arg, struct usbled_led, cdev)
> +
> +static bool riso_kagaku_switch_green_blue;
> +module_param(riso_kagaku_switch_green_blue, bool, S_IRUGO | S_IWUSR);
> +MODULE_PARM_DESC(riso_kagaku_switch_green_blue,
> + "switch green and blue RGB component for Riso Kagaku devices");
> +
> +static int usbled_send(struct usbled_device *udev, __u8 *buf)
> +{
> + int ret;
> +
> + buf[0] = udev->type->report_id;
> +
> + mutex_lock(&udev->lock);
> +
> + if (udev->type->report_type == RAW_REQUEST)
> + ret = hid_hw_raw_request(udev->hdev, buf[0], buf,
> + udev->type->report_size,
> + HID_FEATURE_REPORT,
> + HID_REQ_SET_REPORT);
> + else if (udev->type->report_type == OUTPUT_REPORT)
> + ret = hid_hw_output_report(udev->hdev, buf,
> + udev->type->report_size);
> + else
> + ret = -EINVAL;
> +
> + mutex_unlock(&udev->lock);
> +
> + if (ret < 0)
> + return ret;
> +
> + return ret == udev->type->report_size ? 0 : -EMSGSIZE;
> +}
> +
> +static u8 riso_kagaku_index(struct usbled_device *udev)
> +{
> + enum led_brightness r, g, b;
> +
> + r = udev->red.cdev.brightness;
> + g = udev->green.cdev.brightness;
> + b = udev->blue.cdev.brightness;
> +
> + if (riso_kagaku_switch_green_blue)
> + return RISO_KAGAKU_IX(r, b, g);
> + else
> + return RISO_KAGAKU_IX(r, g, b);
> +}
> +
> +static int riso_kagaku_write(struct led_classdev *cdev, enum led_brightness br)
> +{
> + struct usbled_led *uled = to_usbled_led(cdev);
> + struct usbled_device *udev = uled->udev;
> + __u8 buf[MAX_REPORT_SIZE] = {};
> +
> + buf[1] = riso_kagaku_index(udev);
> +
> + return usbled_send(udev, buf);
> +}
> +
> +static const struct usbled_type usbled_riso_kagaku = {
> + .name = "Riso Kagaku Webmail Notifier",
> + .short_name = "riso_kagaku",
> + .max_brightness = 1,
> + .report_size = 6,
> + .report_type = OUTPUT_REPORT,
> + .report_id = 0,
> + .write = riso_kagaku_write,
> +};
> +
> +static int dream_cheeky_write(struct led_classdev *cdev, enum led_brightness br)
> +{
> + struct usbled_led *uled = to_usbled_led(cdev);
> + struct usbled_device *udev = uled->udev;
> + __u8 buf[MAX_REPORT_SIZE] = {};
> +
> + buf[1] = udev->red.cdev.brightness;
> + buf[2] = udev->green.cdev.brightness;
> + buf[3] = udev->blue.cdev.brightness;
> + buf[7] = 0x1a;
> + buf[8] = 0x05;
> +
> + return usbled_send(udev, buf);
> +}
> +
> +static int dream_cheeky_init(struct usbled_device *udev)
> +{
> + __u8 buf[MAX_REPORT_SIZE] = {};
> +
> + /* Dream Cheeky magic */
> + buf[1] = 0x1f;
> + buf[2] = 0x02;
> + buf[4] = 0x5f;
> + buf[7] = 0x1a;
> + buf[8] = 0x03;
> +
> + return usbled_send(udev, buf);
> +}
> +
> +static const struct usbled_type usbled_dream_cheeky = {
> + .name = "Dream Cheeky Webmail Notifier",
> + .short_name = "dream_cheeky",
> + .max_brightness = 31,
> + .report_size = 9,
> + .report_type = RAW_REQUEST,
> + .report_id = 0,
> + .init = dream_cheeky_init,
> + .write = dream_cheeky_write,
> +};
> +
> +static int usbled_init_led(struct usbled_led *led, const char *color_name,
> + struct usbled_device *udev, unsigned int minor)
> +{
> + snprintf(led->name, sizeof(led->name), "%s%u:%s",
> + udev->type->short_name, minor, color_name);
> + led->cdev.name = led->name;
> + led->cdev.max_brightness = udev->type->max_brightness;
> + led->cdev.brightness_set_blocking = udev->type->write;
> + led->cdev.flags = LED_HW_PLUGGABLE;
> + led->udev = udev;
> +
> + return devm_led_classdev_register(&udev->hdev->dev, &led->cdev);
> +}
> +
> +static int usbled_init_rgb(struct usbled_device *udev, unsigned int minor)
> +{
> + int ret;
> +
> + /* Register the red diode */
> + ret = usbled_init_led(&udev->red, "red", udev, minor);
> + if (ret)
> + return ret;
> +
> + /* Register the green diode */
> + ret = usbled_init_led(&udev->green, "green", udev, minor);
> + if (ret)
> + return ret;
> +
> + /* Register the blue diode */
> + return usbled_init_led(&udev->blue, "blue", udev, minor);
> +}
> +
> +static int usbled_probe(struct hid_device *hdev, const struct hid_device_id *id)
> +{
> + struct usbled_device *udev;
> + unsigned int minor;
> + int ret;
> +
> + udev = devm_kzalloc(&hdev->dev, sizeof(*udev), GFP_KERNEL);
> + if (!udev)
> + return -ENOMEM;
> +
> + ret = hid_parse(hdev);
> + if (ret)
> + return ret;
> +
> + udev->hdev = hdev;
> + udev->type = (const struct usbled_type *)id->driver_data;
I don't think it is a good idea to have an internal pointer in the
driver_data. Users can bind a new device to your driver through the
sysfs (/sys/bus/hid/drivers/hid-usb-led/bind) and having a pointer is
not easy for them.
I'd rather use a plain enum or simply decide about the type based on the
vendor id.
> + mutex_init(&udev->lock);
> +
> + if (udev->type->init) {
udev->type can be null (if manually bound from userspace as mentioned
above)
> + ret = udev->type->init(udev);
> + if (ret)
> + return ret;
> + }
> +
> + ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
> + if (ret)
> + return ret;
> +
> + minor = ((struct hidraw *) hdev->hidraw)->minor;
> +
> + ret = usbled_init_rgb(udev, minor);
> + if (ret) {
> + hid_hw_stop(hdev);
> + return ret;
> + }
> +
> + dev_info(&hdev->dev, "%s initialized\n", udev->type->name);
use hid_info(hdev, "initialized"); (not sure the name is that important
in this info message, but it's up to you).
Cheers,
Benjamin
> +
> + return 0;
> +}
> +
> +static const struct hid_device_id usbled_table[] = {
> + { HID_USB_DEVICE(USB_VENDOR_ID_RISO_KAGAKU,
> + USB_DEVICE_ID_RI_KA_WEBMAIL),
> + .driver_data = (kernel_ulong_t)&usbled_riso_kagaku },
> + { HID_USB_DEVICE(USB_VENDOR_ID_DREAM_CHEEKY,
> + USB_DEVICE_ID_DREAM_CHEEKY_WN),
> + .driver_data = (kernel_ulong_t)&usbled_dream_cheeky },
> + { HID_USB_DEVICE(USB_VENDOR_ID_DREAM_CHEEKY,
> + USB_DEVICE_ID_DREAM_CHEEKY_FA),
> + .driver_data = (kernel_ulong_t)&usbled_dream_cheeky },
> + { }
> +};
> +MODULE_DEVICE_TABLE(hid, usbled_table);
> +
> +static struct hid_driver usbled_driver = {
> + .name = "usb-led",
> + .probe = usbled_probe,
> + .id_table = usbled_table,
> +};
> +
> +module_hid_driver(usbled_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Heiner Kallweit <hkallweit1-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>");
> +MODULE_DESCRIPTION("Simple USB RGB LED driver");
> --
> 2.8.3
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 1/2] hid: migrate USB LED driver from usb misc to hid
2016-06-17 8:53 ` Benjamin Tissoires
@ 2016-06-17 17:28 ` Heiner Kallweit
0 siblings, 0 replies; 3+ messages in thread
From: Heiner Kallweit @ 2016-06-17 17:28 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Greg Kroah-Hartman, Jiri Kosina, linux-input,
Linux USB Mailing List
Am 17.06.2016 um 10:53 schrieb Benjamin Tissoires:
> Hi Heiner,
>
> On Jun 17 2016 or thereabouts, Heiner Kallweit wrote:
>> This patch migrates the USB LED driver to the HID subsystem.
>> Supported are Dream Cheeky Webmail Notifier / Friends Alert
>> and Riso Kagaku Webmail Notifier.
>>
>> Benefits:
>> - Avoid using USB low-level calls and use the HID subsystem instead
>> (as this device provides a USB HID interface)
>> - Use standard LED subsystem instead of proprietary sysfs entries,
>> this allows e.g. to use the device with features like triggers
>>
>> Successfully tested with a Dream Cheeky Webmail Notifier and a
>> Riso Kagaku Webmail Notifier compatible device.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>
> Looks good, though I have a few comments (sorry, again...)
>
>> ---
>> drivers/hid/Kconfig | 12 ++
>> drivers/hid/Makefile | 1 +
>> drivers/hid/hid-core.c | 6 +-
>> drivers/hid/hid-ids.h | 2 +
>> drivers/hid/hid-usb-led.c | 276 ++++++++++++++++++++++++++++++++++++++++++++++
>
> There is nothing USB related now. How about just calling it hid-led.c?
>
Sounds reasonable ..
>> 5 files changed, 294 insertions(+), 3 deletions(-)
>> create mode 100644 drivers/hid/hid-usb-led.c
>>
>> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
>> index 5646ca4..adc9ce6 100644
>> --- a/drivers/hid/Kconfig
>> +++ b/drivers/hid/Kconfig
>> @@ -841,6 +841,18 @@ config THRUSTMASTER_FF
>> a THRUSTMASTER Dual Trigger 3-in-1 or a THRUSTMASTER Ferrari GT
>> Rumble Force or Force Feedback Wheel.
>>
>> +config HID_USB_LED
>> + tristate "Simple USB RGB LED support"
>> + depends on HID
>> + depends on LEDS_CLASS
>> + ---help---
>> + Support for simple USB RGB LED devices. Currently supported are the
>> + Riso Kagaku Webmail Notifier and the Dream Cheeky Webmail Notifier
>> + and Friends Alert.
>> +
>> + To compile this driver as a module, choose M here: the
>> + module will be called hid-usb-led.
>> +
>> config HID_WACOM
>> tristate "Wacom Intuos/Graphire tablet support (USB)"
>> depends on HID
>> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
>> index a2fb562..3014bdd 100644
>> --- a/drivers/hid/Makefile
>> +++ b/drivers/hid/Makefile
>> @@ -96,6 +96,7 @@ obj-$(CONFIG_HID_TIVO) += hid-tivo.o
>> obj-$(CONFIG_HID_TOPSEED) += hid-topseed.o
>> obj-$(CONFIG_HID_TWINHAN) += hid-twinhan.o
>> obj-$(CONFIG_HID_UCLOGIC) += hid-uclogic.o
>> +obj-$(CONFIG_HID_USB_LED) += hid-usb-led.o
>> obj-$(CONFIG_HID_XINMO) += hid-xinmo.o
>> obj-$(CONFIG_HID_ZEROPLUS) += hid-zpff.o
>> obj-$(CONFIG_HID_ZYDACRON) += hid-zydacron.o
>> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
>> index 8ea3a26..eb674ce 100644
>> --- a/drivers/hid/hid-core.c
>> +++ b/drivers/hid/hid-core.c
>> @@ -1879,6 +1879,8 @@ static const struct hid_device_id hid_have_special_driver[] = {
>> { HID_USB_DEVICE(USB_VENDOR_ID_CYPRESS, USB_DEVICE_ID_CYPRESS_MOUSE) },
>> { HID_USB_DEVICE(USB_VENDOR_ID_DRAGONRISE, 0x0006) },
>> { HID_USB_DEVICE(USB_VENDOR_ID_DRAGONRISE, 0x0011) },
>> + { HID_USB_DEVICE(USB_VENDOR_ID_DREAM_CHEEKY, USB_DEVICE_ID_DREAM_CHEEKY_WN) },
>> + { HID_USB_DEVICE(USB_VENDOR_ID_DREAM_CHEEKY, USB_DEVICE_ID_DREAM_CHEEKY_FA) },
>> { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_ELECOM, USB_DEVICE_ID_ELECOM_BM084) },
>> { HID_USB_DEVICE(USB_VENDOR_ID_ELO, 0x0009) },
>> { HID_USB_DEVICE(USB_VENDOR_ID_ELO, 0x0030) },
>> @@ -2008,6 +2010,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
>> { HID_USB_DEVICE(USB_VENDOR_ID_PETALYNX, USB_DEVICE_ID_PETALYNX_MAXTER_REMOTE) },
>> { HID_USB_DEVICE(USB_VENDOR_ID_PLANTRONICS, HID_ANY_ID) },
>> { HID_USB_DEVICE(USB_VENDOR_ID_PRIMAX, USB_DEVICE_ID_PRIMAX_KEYBOARD) },
>> + { HID_USB_DEVICE(USB_VENDOR_ID_RISO_KAGAKU, USB_DEVICE_ID_RI_KA_WEBMAIL) },
>> #if IS_ENABLED(CONFIG_HID_ROCCAT)
>> { HID_USB_DEVICE(USB_VENDOR_ID_ROCCAT, USB_DEVICE_ID_ROCCAT_ARVO) },
>> { HID_USB_DEVICE(USB_VENDOR_ID_ROCCAT, USB_DEVICE_ID_ROCCAT_ISKU) },
>> @@ -2348,8 +2351,6 @@ static const struct hid_device_id hid_ignore_list[] = {
>> { HID_USB_DEVICE(USB_VENDOR_ID_DEALEXTREAME, USB_DEVICE_ID_DEALEXTREAME_RADIO_SI4701) },
>> { HID_USB_DEVICE(USB_VENDOR_ID_DELORME, USB_DEVICE_ID_DELORME_EARTHMATE) },
>> { HID_USB_DEVICE(USB_VENDOR_ID_DELORME, USB_DEVICE_ID_DELORME_EM_LT20) },
>> - { HID_USB_DEVICE(USB_VENDOR_ID_DREAM_CHEEKY, 0x0004) },
>> - { HID_USB_DEVICE(USB_VENDOR_ID_DREAM_CHEEKY, 0x000a) },
>> { HID_I2C_DEVICE(USB_VENDOR_ID_ELAN, 0x0400) },
>> { HID_I2C_DEVICE(USB_VENDOR_ID_ELAN, 0x0401) },
>> { HID_USB_DEVICE(USB_VENDOR_ID_ESSENTIAL_REALITY, USB_DEVICE_ID_ESSENTIAL_REALITY_P5) },
>> @@ -2486,7 +2487,6 @@ static const struct hid_device_id hid_ignore_list[] = {
>> { HID_USB_DEVICE(USB_VENDOR_ID_SYNAPTICS, USB_DEVICE_ID_SYNAPTICS_DPAD) },
>> #endif
>> { HID_USB_DEVICE(USB_VENDOR_ID_YEALINK, USB_DEVICE_ID_YEALINK_P1K_P4K_B2K) },
>> - { HID_USB_DEVICE(USB_VENDOR_ID_RISO_KAGAKU, USB_DEVICE_ID_RI_KA_WEBMAIL) },
>> { }
>> };
>>
>> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
>> index 3eec09a..e104aba 100644
>> --- a/drivers/hid/hid-ids.h
>> +++ b/drivers/hid/hid-ids.h
>> @@ -334,6 +334,8 @@
>> #define USB_DEVICE_ID_ELECOM_BM084 0x0061
>>
>> #define USB_VENDOR_ID_DREAM_CHEEKY 0x1d34
>> +#define USB_DEVICE_ID_DREAM_CHEEKY_WN 0x0004
>> +#define USB_DEVICE_ID_DREAM_CHEEKY_FA 0x000a
>>
>> #define USB_VENDOR_ID_ELITEGROUP 0x03fc
>> #define USB_DEVICE_ID_ELITEGROUP_05D8 0x05d8
>> diff --git a/drivers/hid/hid-usb-led.c b/drivers/hid/hid-usb-led.c
>> new file mode 100644
>> index 0000000..c7f5a62
>> --- /dev/null
>> +++ b/drivers/hid/hid-usb-led.c
>> @@ -0,0 +1,276 @@
>> +/*
>> + * Simple USB RGB LED driver
>> + *
>> + * Copyright 2016 Heiner Kallweit <hkallweit1@gmail.com>
>> + * Based on drivers/hid/hid-thingm.c and
>> + * drivers/usb/misc/usbled.c
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License as
>> + * published by the Free Software Foundation, version 2.
>> + */
>> +
>> +#include <linux/hid.h>
>> +#include <linux/hidraw.h>
>> +#include <linux/leds.h>
>> +#include <linux/module.h>
>> +#include <linux/mutex.h>
>> +
>> +#include "hid-ids.h"
>> +
>> +enum led_report_type {
>> + RAW_REQUEST,
>> + OUTPUT_REPORT
>> +};
>> +
>> +static unsigned const char riso_kagaku_tbl[] = {
>> +/* R+2G+4B -> riso kagaku color index */
>> + [0] = 0, /* black */
>> + [1] = 2, /* red */
>> + [2] = 1, /* green */
>> + [3] = 5, /* yellow */
>> + [4] = 3, /* blue */
>> + [5] = 6, /* magenta */
>> + [6] = 4, /* cyan */
>> + [7] = 7 /* white */
>> +};
>> +
>> +#define RISO_KAGAKU_IX(r, g, b) riso_kagaku_tbl[((r)?1:0)+((g)?2:0)+((b)?4:0)]
>> +
>> +struct usbled_device;
>
> I'd say remove all "usb" strings from the driver and use hled or hidled instead
> of usbled.
>
>> +
>> +struct usbled_type {
>> + const char *name;
>> + const char *short_name;
>> + enum led_brightness max_brightness;
>> + size_t report_size;
>> + enum led_report_type report_type;
>> + u8 report_id;
>> + int (*init)(struct usbled_device *udev);
>> + int (*write)(struct led_classdev *cdev, enum led_brightness br);
>> +};
>> +
>> +struct usbled_led {
>> + struct led_classdev cdev;
>> + struct usbled_device *udev;
>> + char name[32];
>> +};
>> +
>> +struct usbled_device {
>> + const struct usbled_type *type;
>> + struct usbled_led red;
>> + struct usbled_led green;
>> + struct usbled_led blue;
>> + struct hid_device *hdev;
>> + struct mutex lock;
>> +};
>> +
>> +#define MAX_REPORT_SIZE 16
>> +
>> +#define to_usbled_led(arg) container_of(arg, struct usbled_led, cdev)
>> +
>> +static bool riso_kagaku_switch_green_blue;
>> +module_param(riso_kagaku_switch_green_blue, bool, S_IRUGO | S_IWUSR);
>> +MODULE_PARM_DESC(riso_kagaku_switch_green_blue,
>> + "switch green and blue RGB component for Riso Kagaku devices");
>> +
>> +static int usbled_send(struct usbled_device *udev, __u8 *buf)
>> +{
>> + int ret;
>> +
>> + buf[0] = udev->type->report_id;
>> +
>> + mutex_lock(&udev->lock);
>> +
>> + if (udev->type->report_type == RAW_REQUEST)
>> + ret = hid_hw_raw_request(udev->hdev, buf[0], buf,
>> + udev->type->report_size,
>> + HID_FEATURE_REPORT,
>> + HID_REQ_SET_REPORT);
>> + else if (udev->type->report_type == OUTPUT_REPORT)
>> + ret = hid_hw_output_report(udev->hdev, buf,
>> + udev->type->report_size);
>> + else
>> + ret = -EINVAL;
>> +
>> + mutex_unlock(&udev->lock);
>> +
>> + if (ret < 0)
>> + return ret;
>> +
>> + return ret == udev->type->report_size ? 0 : -EMSGSIZE;
>> +}
>> +
>> +static u8 riso_kagaku_index(struct usbled_device *udev)
>> +{
>> + enum led_brightness r, g, b;
>> +
>> + r = udev->red.cdev.brightness;
>> + g = udev->green.cdev.brightness;
>> + b = udev->blue.cdev.brightness;
>> +
>> + if (riso_kagaku_switch_green_blue)
>> + return RISO_KAGAKU_IX(r, b, g);
>> + else
>> + return RISO_KAGAKU_IX(r, g, b);
>> +}
>> +
>> +static int riso_kagaku_write(struct led_classdev *cdev, enum led_brightness br)
>> +{
>> + struct usbled_led *uled = to_usbled_led(cdev);
>> + struct usbled_device *udev = uled->udev;
>> + __u8 buf[MAX_REPORT_SIZE] = {};
>> +
>> + buf[1] = riso_kagaku_index(udev);
>> +
>> + return usbled_send(udev, buf);
>> +}
>> +
>> +static const struct usbled_type usbled_riso_kagaku = {
>> + .name = "Riso Kagaku Webmail Notifier",
>> + .short_name = "riso_kagaku",
>> + .max_brightness = 1,
>> + .report_size = 6,
>> + .report_type = OUTPUT_REPORT,
>> + .report_id = 0,
>> + .write = riso_kagaku_write,
>> +};
>> +
>> +static int dream_cheeky_write(struct led_classdev *cdev, enum led_brightness br)
>> +{
>> + struct usbled_led *uled = to_usbled_led(cdev);
>> + struct usbled_device *udev = uled->udev;
>> + __u8 buf[MAX_REPORT_SIZE] = {};
>> +
>> + buf[1] = udev->red.cdev.brightness;
>> + buf[2] = udev->green.cdev.brightness;
>> + buf[3] = udev->blue.cdev.brightness;
>> + buf[7] = 0x1a;
>> + buf[8] = 0x05;
>> +
>> + return usbled_send(udev, buf);
>> +}
>> +
>> +static int dream_cheeky_init(struct usbled_device *udev)
>> +{
>> + __u8 buf[MAX_REPORT_SIZE] = {};
>> +
>> + /* Dream Cheeky magic */
>> + buf[1] = 0x1f;
>> + buf[2] = 0x02;
>> + buf[4] = 0x5f;
>> + buf[7] = 0x1a;
>> + buf[8] = 0x03;
>> +
>> + return usbled_send(udev, buf);
>> +}
>> +
>> +static const struct usbled_type usbled_dream_cheeky = {
>> + .name = "Dream Cheeky Webmail Notifier",
>> + .short_name = "dream_cheeky",
>> + .max_brightness = 31,
>> + .report_size = 9,
>> + .report_type = RAW_REQUEST,
>> + .report_id = 0,
>> + .init = dream_cheeky_init,
>> + .write = dream_cheeky_write,
>> +};
>> +
>> +static int usbled_init_led(struct usbled_led *led, const char *color_name,
>> + struct usbled_device *udev, unsigned int minor)
>> +{
>> + snprintf(led->name, sizeof(led->name), "%s%u:%s",
>> + udev->type->short_name, minor, color_name);
>> + led->cdev.name = led->name;
>> + led->cdev.max_brightness = udev->type->max_brightness;
>> + led->cdev.brightness_set_blocking = udev->type->write;
>> + led->cdev.flags = LED_HW_PLUGGABLE;
>> + led->udev = udev;
>> +
>> + return devm_led_classdev_register(&udev->hdev->dev, &led->cdev);
>> +}
>> +
>> +static int usbled_init_rgb(struct usbled_device *udev, unsigned int minor)
>> +{
>> + int ret;
>> +
>> + /* Register the red diode */
>> + ret = usbled_init_led(&udev->red, "red", udev, minor);
>> + if (ret)
>> + return ret;
>> +
>> + /* Register the green diode */
>> + ret = usbled_init_led(&udev->green, "green", udev, minor);
>> + if (ret)
>> + return ret;
>> +
>> + /* Register the blue diode */
>> + return usbled_init_led(&udev->blue, "blue", udev, minor);
>> +}
>> +
>> +static int usbled_probe(struct hid_device *hdev, const struct hid_device_id *id)
>> +{
>> + struct usbled_device *udev;
>> + unsigned int minor;
>> + int ret;
>> +
>> + udev = devm_kzalloc(&hdev->dev, sizeof(*udev), GFP_KERNEL);
>> + if (!udev)
>> + return -ENOMEM;
>> +
>> + ret = hid_parse(hdev);
>> + if (ret)
>> + return ret;
>> +
>> + udev->hdev = hdev;
>> + udev->type = (const struct usbled_type *)id->driver_data;
>
> I don't think it is a good idea to have an internal pointer in the
> driver_data. Users can bind a new device to your driver through the
> sysfs (/sys/bus/hid/drivers/hid-usb-led/bind) and having a pointer is
> not easy for them.
>
> I'd rather use a plain enum or simply decide about the type based on the
> vendor id.
>
Uh, I wasn't aware of this sysfs feature. I think then I'll go with the enum.
This allows to use the driver also with devices from other manufacturers which
by chance use the same protocol as one of the supported devices.
>> + mutex_init(&udev->lock);
>> +
>> + if (udev->type->init) {
>
> udev->type can be null (if manually bound from userspace as mentioned
> above)
>
Yes, due to the manual bind feature I have to check for it.
>> + ret = udev->type->init(udev);
>> + if (ret)
>> + return ret;
>> + }
>> +
>> + ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
>> + if (ret)
>> + return ret;
>> +
>> + minor = ((struct hidraw *) hdev->hidraw)->minor;
>> +
>> + ret = usbled_init_rgb(udev, minor);
>> + if (ret) {
>> + hid_hw_stop(hdev);
>> + return ret;
>> + }
>> +
>> + dev_info(&hdev->dev, "%s initialized\n", udev->type->name);
>
> use hid_info(hdev, "initialized"); (not sure the name is that important
> in this info message, but it's up to you).
>
OK
> Cheers,
> Benjamin
>
>> +
>> + return 0;
>> +}
>> +
>> +static const struct hid_device_id usbled_table[] = {
>> + { HID_USB_DEVICE(USB_VENDOR_ID_RISO_KAGAKU,
>> + USB_DEVICE_ID_RI_KA_WEBMAIL),
>> + .driver_data = (kernel_ulong_t)&usbled_riso_kagaku },
>> + { HID_USB_DEVICE(USB_VENDOR_ID_DREAM_CHEEKY,
>> + USB_DEVICE_ID_DREAM_CHEEKY_WN),
>> + .driver_data = (kernel_ulong_t)&usbled_dream_cheeky },
>> + { HID_USB_DEVICE(USB_VENDOR_ID_DREAM_CHEEKY,
>> + USB_DEVICE_ID_DREAM_CHEEKY_FA),
>> + .driver_data = (kernel_ulong_t)&usbled_dream_cheeky },
>> + { }
>> +};
>> +MODULE_DEVICE_TABLE(hid, usbled_table);
>> +
>> +static struct hid_driver usbled_driver = {
>> + .name = "usb-led",
>> + .probe = usbled_probe,
>> + .id_table = usbled_table,
>> +};
>> +
>> +module_hid_driver(usbled_driver);
>> +
>> +MODULE_LICENSE("GPL");
>> +MODULE_AUTHOR("Heiner Kallweit <hkallweit1@gmail.com>");
>> +MODULE_DESCRIPTION("Simple USB RGB LED driver");
>> --
>> 2.8.3
>>
>>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-06-17 17:29 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-06-17 6:07 [PATCH 1/2] hid: migrate USB LED driver from usb misc to hid Heiner Kallweit
[not found] ` <8413318b-cd83-764e-808a-0c060a15a757-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2016-06-17 8:53 ` Benjamin Tissoires
2016-06-17 17:28 ` Heiner Kallweit
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).