Am 27.05.2016 um 23:45 schrieb Benjamin Tissoires:
> On May 27 2016 or thereabouts, Heiner Kallweit wrote:
>> The Riso Kagaku Webmail Notifier (and its clones) is supported as part of
>> usb/misc/usbled driver currently. This patch migrates the driver for this
>> device to the HID subsystem.
>>
>> 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
>>
>> I know at least one cheap clone coming with green and blue LED switched.
>> There's a module paramater to deal with this.
>>
>> There might be users of the current module, therefore so far allow
>> compilation of the new driver only if the old one is disabled.
>>
>> Signed-off-by: Heiner Kallweit <[email protected]>
>> ---
>> drivers/hid/Kconfig | 9 +++
>> drivers/hid/Makefile | 1 +
>> drivers/hid/hid-core.c | 2 +-
>> drivers/hid/hid-rika.c | 171
>> +++++++++++++++++++++++++++++++++++++++++++++++++
>> 4 files changed, 182 insertions(+), 1 deletion(-)
>> create mode 100644 drivers/hid/hid-rika.c
>>
>> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
>> index 5646ca4..5ba75de 100644
>> --- a/drivers/hid/Kconfig
>> +++ b/drivers/hid/Kconfig
>> @@ -784,6 +784,15 @@ config HID_HYPERV_MOUSE
>> ---help---
>> Select this option to enable the Hyper-V mouse driver.
>>
>> +config HID_RIKA
>> + tristate "Riso Kaguku Webmail Notifier USB LED"
>> + depends on HIDRAW
>> + depends on LEDS_CLASS
>> + depends on USB_LED = 'n'
>> + ---help---
>> + Support for the Riso Kagaku Webmail Notifier. This driver registers
>> + a LED class instance for red, green, and blue color component.
>> +
>> config HID_SMARTJOYPLUS
>> tristate "SmartJoy PLUS PS2/USB adapter support"
>> depends on HID
>> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
>> index a2fb562..406f6d0 100644
>> --- a/drivers/hid/Makefile
>> +++ b/drivers/hid/Makefile
>> @@ -91,6 +91,7 @@ obj-$(CONFIG_HID_STEELSERIES) += hid-steelseries.o
>> obj-$(CONFIG_HID_SUNPLUS) += hid-sunplus.o
>> obj-$(CONFIG_HID_GREENASIA) += hid-gaff.o
>> obj-$(CONFIG_HID_THINGM) += hid-thingm.o
>> +obj-$(CONFIG_HID_RIKA) += hid-rika.o
>> obj-$(CONFIG_HID_THRUSTMASTER) += hid-tmff.o
>> obj-$(CONFIG_HID_TIVO) += hid-tivo.o
>> obj-$(CONFIG_HID_TOPSEED) += hid-topseed.o
>> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
>> index 8ea3a26..db929dc 100644
>> --- a/drivers/hid/hid-core.c
>> +++ b/drivers/hid/hid-core.c
>> @@ -2008,6 +2008,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) },
>> @@ -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) },
>
> Hi Heiner,
>
> Just a quick remark before an actual review. If you remove
> unconditionally the driver from hid_ignore_list, the usb driver will
> never get a chance to be bound to it as hid core will keep a reference
> to it.
>
> The solution is either to #ifdef this line or to port the full USB
> driver into a HID one (and so remove the USB one).
Right, thanks for the hint. For now I would enclose this line in
#if IS_ENABLED(CONFIG_USB_LED)
There might be users of the current USB driver (although the proprietary
sysfs interface isn't documented under Documentation/ABI) and it might
be a good idea to give them some time to migrate before removing the old
USB driver.
Before submitting a v2 of the patch I'll wait for more review feedback.
Rgds, Heiner
>
> Cheers,
> Benjamin
>
>> { }
>> };
>>
>> diff --git a/drivers/hid/hid-rika.c b/drivers/hid/hid-rika.c
>> new file mode 100644
>> index 0000000..d4d2087
>> --- /dev/null
>> +++ b/drivers/hid/hid-rika.c
>> @@ -0,0 +1,171 @@
>> +/*
>> + * Riso Kagaku Webmail Notifier USB RGB LED driver
>> + *
>> + * Copyright 2016 Heiner Kallweit <[email protected]>
>> + * 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"
>> +
>> +#define REPORT_SIZE 6
>> +
>> +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 rika_led {
>> + struct led_classdev cdev;
>> + struct rika_device *rdev;
>> + char name[32];
>> +};
>> +
>> +struct rika_device {
>> + struct rika_led red;
>> + struct rika_led green;
>> + struct rika_led blue;
>> + struct hid_device *hdev;
>> + struct mutex lock;
>> +};
>> +
>> +#define to_rika_led(arg) container_of(arg, struct rika_led, cdev)
>> +
>> +static bool switch_green_blue;
>> +module_param(switch_green_blue, bool, 0);
>> +MODULE_PARM_DESC(switch_green_blue, "switch green and blue RGB component");
>> +
>> +static u8 rika_index(struct rika_device *rdev)
>> +{
>> + enum led_brightness r, g, b;
>> +
>> + r = rdev->red.cdev.brightness;
>> + g = rdev->green.cdev.brightness;
>> + b = rdev->blue.cdev.brightness;
>> +
>> + if (switch_green_blue)
>> + return RISO_KAGAKU_IX(r, b, g);
>> + else
>> + return RISO_KAGAKU_IX(r, g, b);
>> +}
>> +
>> +static int rika_write_color(struct led_classdev *cdev, enum led_brightness
>> br)
>> +{
>> + struct rika_led *rled = to_rika_led(cdev);
>> + struct rika_device *rdev = rled->rdev;
>> + __u8 buf[REPORT_SIZE] = {};
>> + int ret;
>> +
>> + buf[1] = rika_index(rdev);
>> +
>> + mutex_lock(&rdev->lock);
>> + ret = hid_hw_output_report(rdev->hdev, buf, REPORT_SIZE);
>> + mutex_unlock(&rdev->lock);
>> +
>> + if (ret < 0)
>> + return ret;
>> +
>> + return (ret == REPORT_SIZE) ? 0 : -EMSGSIZE;
>> +}
>> +
>> +static int rika_init_led(struct rika_led *led, const char *color_name,
>> + struct rika_device *rdev, int minor)
>> +{
>> + snprintf(led->name, sizeof(led->name), "rika%d:%s", minor, color_name);
>> + led->cdev.name = led->name;
>> + led->cdev.max_brightness = 1;
>> + led->cdev.brightness_set_blocking = rika_write_color;
>> + led->cdev.flags = LED_HW_PLUGGABLE;
>> + led->rdev = rdev;
>> +
>> + return devm_led_classdev_register(&rdev->hdev->dev, &led->cdev);
>> +}
>> +
>> +static int rika_init_rgb(struct rika_device *rdev, int minor)
>> +{
>> + int ret;
>> +
>> + /* Register the red diode */
>> + ret = rika_init_led(&rdev->red, "red", rdev, minor);
>> + if (ret)
>> + return ret;
>> +
>> + /* Register the green diode */
>> + ret = rika_init_led(&rdev->green, "green", rdev, minor);
>> + if (ret)
>> + return ret;
>> +
>> + /* Register the blue diode */
>> + return rika_init_led(&rdev->blue, "blue", rdev, minor);
>> +}
>> +
>> +static int rika_probe(struct hid_device *hdev, const struct hid_device_id
>> *id)
>> +{
>> + struct rika_device *rdev;
>> + int ret, minor;
>> +
>> + rdev = devm_kzalloc(&hdev->dev, sizeof(*rdev), GFP_KERNEL);
>> + if (!rdev)
>> + return -ENOMEM;
>> +
>> + ret = hid_parse(hdev);
>> + if (ret)
>> + return ret;
>> +
>> + rdev->hdev = hdev;
>> + mutex_init(&rdev->lock);
>> +
>> + ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
>> + if (ret)
>> + return ret;
>> +
>> + minor = ((struct hidraw *) hdev->hidraw)->minor;
>> +
>> + ret = rika_init_rgb(rdev, minor);
>> + if (ret) {
>> + hid_hw_stop(hdev);
>> + return ret;
>> + }
>> +
>> + dev_info(&hdev->dev, "RiKa Webmail Notifier %d initialized\n", minor);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct hid_device_id rika_table[] = {
>> + { HID_USB_DEVICE(USB_VENDOR_ID_RISO_KAGAKU,
>> + USB_DEVICE_ID_RI_KA_WEBMAIL) },
>> + { }
>> +};
>> +MODULE_DEVICE_TABLE(hid, rika_table);
>> +
>> +static struct hid_driver rika_driver = {
>> + .name = "rika",
>> + .probe = rika_probe,
>> + .id_table = rika_table,
>> +};
>> +
>> +module_hid_driver(rika_driver);
>> +
>> +MODULE_LICENSE("GPL");
>> +MODULE_AUTHOR("Heiner Kallweit <[email protected]>");
>> +MODULE_DESCRIPTION("Riso Kagaku Webmail Notifier USB RGB LED driver");
>> --
>> 2.8.2
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
