| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20170410063907.GA20349@mail.corp.redhat.com>
Date: Mon, 10 Apr 2017 08:39:07 +0200
From: Benjamin Tissoires <benjamin.tissoires@...hat.com>
To: Carlo Caione <carlo@...one.org>
Cc: jikos@...nel.org, linux-input@...r.kernel.org,
linux-kernel@...r.kernel.org, linux@...lessm.com,
Carlo Caione <carlo@...lessm.com>
Subject: Re: [PATCH v3] HID: asus: support backlight on USB keyboards
On Apr 06 2017 or thereabouts, Carlo Caione wrote:
> From: Carlo Caione <carlo@...lessm.com>
>
> The latest USB keyboards shipped on several ASUS laptop models
> (including ROG laptop models such as GL702VMK) have the keyboards
> backlight controlled by the keyboard firmware.
>
> The firmware implements at least 3 different commands:
> - Init command (to use when the system starts)
> - Configuration command (to get keyboard status/information)
> - Backlight level control (to change the level of the keyboard light)
>
> With this patch we create the usual 'asus::kbd_backlight' led class
> entry to control the keyboard backlight.
>
> Signed-off-by: Carlo Caione <carlo@...lessm.com>
> ---
> Changelog:
>
> v2:
> - Code refactoring
> - s/sysfs/led class
> - No error messages on kzalloc/kmemdup failing
> - Fixed race condition when removing device
> - Used devm_* when possible
> - Added LEDS_CLASS dependency
>
> v3:
> - s/asus_kbd_get_report/asus_kbd_set_report
> - Added cancel_work_sync()
> - Return error from asus_kbd_register_leds()
> - Do not leak drvdata->kbd_backlight on devm_led_classdev_register()
> failure
> - Coding style
> ---
> drivers/hid/Kconfig | 1 +
> drivers/hid/hid-asus.c | 184 ++++++++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 184 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index eb1846a..dec5f3e 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -136,6 +136,7 @@ config HID_APPLEIR
>
> config HID_ASUS
> tristate "Asus"
> + depends on LEDS_CLASS
> ---help---
> Support for Asus notebook built-in keyboard and touchpad via i2c, and
> the Asus Republic of Gamers laptop keyboard special keys.
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index bacba97..e78c7de 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -40,8 +40,12 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
>
> #define FEATURE_REPORT_ID 0x0d
> #define INPUT_REPORT_ID 0x5d
> +#define FEATURE_KBD_REPORT_ID 0x5a
>
> #define INPUT_REPORT_SIZE 28
> +#define FEATURE_KBD_REPORT_SIZE 16
> +
> +#define SUPPORT_KBD_BACKLIGHT BIT(0)
>
> #define MAX_CONTACTS 5
>
> @@ -64,6 +68,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
> #define QUIRK_SKIP_INPUT_MAPPING BIT(2)
> #define QUIRK_IS_MULTITOUCH BIT(3)
> #define QUIRK_NO_CONSUMER_USAGES BIT(4)
> +#define QUIRK_USE_KBD_BACKLIGHT BIT(5)
>
> #define I2C_KEYBOARD_QUIRKS (QUIRK_FIX_NOTEBOOK_REPORT | \
> QUIRK_NO_INIT_REPORTS | \
> @@ -74,9 +79,19 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
>
> #define TRKID_SGN ((TRKID_MAX + 1) >> 1)
>
> +struct asus_kbd_leds {
> + struct led_classdev cdev;
> + struct hid_device *hdev;
> + struct work_struct work;
> + unsigned int brightness;
> + bool removed;
> +};
> +
> struct asus_drvdata {
> unsigned long quirks;
> struct input_dev *input;
> + struct asus_kbd_leds *kbd_backlight;
> + bool enable_backlight;
> };
>
> static void asus_report_contact_down(struct input_dev *input,
> @@ -171,6 +186,149 @@ static int asus_raw_event(struct hid_device *hdev,
> return 0;
> }
>
> +static int asus_kbd_set_report(struct hid_device *hdev, u8 *buf, size_t buf_size)
> +{
> + unsigned char *dmabuf;
> + int ret;
> +
> + dmabuf = kmemdup(buf, buf_size, GFP_KERNEL);
> + if (!dmabuf)
> + return -ENOMEM;
> +
> + ret = hid_hw_raw_request(hdev, FEATURE_KBD_REPORT_ID, dmabuf,
> + buf_size, HID_FEATURE_REPORT,
> + HID_REQ_SET_REPORT);
> + kfree(dmabuf);
> +
> + return ret;
> +}
> +
> +static int asus_kbd_init(struct hid_device *hdev)
> +{
> + u8 buf[] = { FEATURE_KBD_REPORT_ID, 0x41, 0x53, 0x55, 0x53, 0x20, 0x54,
> + 0x65, 0x63, 0x68, 0x2e, 0x49, 0x6e, 0x63, 0x2e, 0x00 };
> + int ret;
> +
> + ret = asus_kbd_set_report(hdev, buf, sizeof(buf));
> + if (ret < 0)
> + hid_err(hdev, "Asus failed to send init command: %d\n", ret);
> +
> + return ret;
> +}
> +
> +static int asus_kbd_get_functions(struct hid_device *hdev,
> + unsigned char *kbd_func)
> +{
> + u8 buf[] = { FEATURE_KBD_REPORT_ID, 0x05, 0x20, 0x31, 0x00, 0x08 };
> + u8 *readbuf;
> + int ret;
> +
> + ret = asus_kbd_set_report(hdev, buf, sizeof(buf));
> + if (ret < 0) {
> + hid_err(hdev, "Asus failed to send configuration command: %d\n", ret);
> + return ret;
> + }
> +
> + readbuf = kzalloc(FEATURE_KBD_REPORT_SIZE, GFP_KERNEL);
> + if (!readbuf)
> + return -ENOMEM;
> +
> + ret = hid_hw_raw_request(hdev, FEATURE_KBD_REPORT_ID, readbuf,
> + FEATURE_KBD_REPORT_SIZE, HID_FEATURE_REPORT,
> + HID_REQ_GET_REPORT);
> + if (ret < 0) {
> + hid_err(hdev, "Asus failed to request functions: %d\n", ret);
> + kfree(readbuf);
> + return ret;
> + }
> +
> + *kbd_func = readbuf[6];
> +
> + kfree(readbuf);
> + return ret;
> +}
> +
> +static void asus_kbd_backlight_set(struct led_classdev *led_cdev,
> + enum led_brightness brightness)
> +{
> + struct asus_kbd_leds *led = container_of(led_cdev, struct asus_kbd_leds,
> + cdev);
> + if (led->brightness == brightness)
> + return;
> +
> + led->brightness = brightness;
> + schedule_work(&led->work);
> +}
> +
> +static enum led_brightness asus_kbd_backlight_get(struct led_classdev *led_cdev)
> +{
> + struct asus_kbd_leds *led = container_of(led_cdev, struct asus_kbd_leds,
> + cdev);
> +
> + return led->brightness;
> +}
> +
> +static void asus_kbd_backlight_work(struct work_struct *work)
> +{
> + struct asus_kbd_leds *led = container_of(work, struct asus_kbd_leds, work);
> + u8 buf[] = { FEATURE_KBD_REPORT_ID, 0xba, 0xc5, 0xc4, 0x00 };
> + int ret;
> +
> + if (led->removed)
> + return;
> +
> + buf[4] = led->brightness;
> +
> + ret = asus_kbd_set_report(led->hdev, buf, sizeof(buf));
> + if (ret < 0)
> + hid_err(led->hdev, "Asus failed to set keyboard backlight: %d\n", ret);
> +}
> +
> +static int asus_kbd_register_leds(struct hid_device *hdev)
> +{
> + struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
> + unsigned char kbd_func;
> + int ret;
> +
> + /* Initialize keyboard */
> + ret = asus_kbd_init(hdev);
> + if (ret < 0)
> + return ret;
> +
> + /* Get keyboard functions */
> + ret = asus_kbd_get_functions(hdev, &kbd_func);
> + if (ret < 0)
> + return ret;
> +
> + /* Check for backlight support */
> + if (!(kbd_func & SUPPORT_KBD_BACKLIGHT))
> + return -ENODEV;
> +
> + drvdata->kbd_backlight = devm_kzalloc(&hdev->dev,
> + sizeof(struct asus_kbd_leds),
> + GFP_KERNEL);
> + if (!drvdata->kbd_backlight)
> + return -ENOMEM;
> +
> + drvdata->kbd_backlight->removed = false;
> + drvdata->kbd_backlight->brightness = 0;
> + drvdata->kbd_backlight->hdev = hdev;
> + drvdata->kbd_backlight->cdev.name = "asus::kbd_backlight";
> + drvdata->kbd_backlight->cdev.max_brightness = 3;
> + drvdata->kbd_backlight->cdev.brightness_set = asus_kbd_backlight_set;
> + drvdata->kbd_backlight->cdev.brightness_get = asus_kbd_backlight_get;
> + INIT_WORK(&drvdata->kbd_backlight->work, asus_kbd_backlight_work);
> +
> + ret = devm_led_classdev_register(&hdev->dev, &drvdata->kbd_backlight->cdev);
> + if (ret < 0) {
> + /* No need to have this still around */
> + devm_kfree(&hdev->dev, drvdata->kbd_backlight);
> + cancel_work_sync(&drvdata->kbd_backlight->work);
Small nitpick, you don't need to call cancel_work_sync() here, nobody
could have called a worker. But OTOH, it doesn't hurt.
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@...hat.com>
Cheers,
Benjamin
> + }
> +
> + return ret;
> +}
> +
> static int asus_input_configured(struct hid_device *hdev, struct hid_input *hi)
> {
> struct input_dev *input = hi->input;
> @@ -198,6 +356,9 @@ static int asus_input_configured(struct hid_device *hdev, struct hid_input *hi)
>
> drvdata->input = input;
>
> + if (drvdata->enable_backlight && asus_kbd_register_leds(hdev))
> + hid_warn(hdev, "Failed to initialize backlight.\n");
> +
> return 0;
> }
>
> @@ -248,6 +409,16 @@ static int asus_input_mapping(struct hid_device *hdev,
> * as some make the keyboard appear as a pointer device. */
> return -1;
> }
> +
> + /*
> + * Check and enable backlight only on devices with UsagePage ==
> + * 0xff31 to avoid initializing the keyboard firmware multiple
> + * times on devices with multiple HID descriptors but same
> + * PID/VID.
> + */
> + if (drvdata->quirks & QUIRK_USE_KBD_BACKLIGHT)
> + drvdata->enable_backlight = true;
> +
> return 1;
> }
>
> @@ -358,6 +529,16 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
> return ret;
> }
>
> +static void asus_remove(struct hid_device *hdev)
> +{
> + struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
> +
> + if (drvdata->kbd_backlight) {
> + drvdata->kbd_backlight->removed = true;
> + cancel_work_sync(&drvdata->kbd_backlight->work);
> + }
> +}
> +
> static __u8 *asus_report_fixup(struct hid_device *hdev, __u8 *rdesc,
> unsigned int *rsize)
> {
> @@ -379,7 +560,7 @@ static const struct hid_device_id asus_devices[] = {
> { HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
> USB_DEVICE_ID_ASUSTEK_ROG_KEYBOARD1) },
> { HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
> - USB_DEVICE_ID_ASUSTEK_ROG_KEYBOARD2) },
> + USB_DEVICE_ID_ASUSTEK_ROG_KEYBOARD2), QUIRK_USE_KBD_BACKLIGHT },
> { }
> };
> MODULE_DEVICE_TABLE(hid, asus_devices);
> @@ -389,6 +570,7 @@ static struct hid_driver asus_driver = {
> .id_table = asus_devices,
> .report_fixup = asus_report_fixup,
> .probe = asus_probe,
> + .remove = asus_remove,
> .input_mapping = asus_input_mapping,
> .input_configured = asus_input_configured,
> #ifdef CONFIG_PM
> --
> 2.9.3
>
Powered by blists - more mailing lists