* Re: [PATCH v7 5/5] ARM: dts: am437x-gp-evm: Update binding for touchscreen size
From: Tony Lindgren @ 2014-06-17 9:50 UTC (permalink / raw)
To: Roger Quadros
Cc: dmitry.torokhov, rydberg, balbi, dmurphy, mugunthanvnm, nsekhar,
linux-input, linux-kernel, linux-omap, Benoit Cousson
In-Reply-To: <1402997507-9736-6-git-send-email-rogerq@ti.com>
* Roger Quadros <rogerq@ti.com> [140617 02:33]:
> Update the bindings for touchscreen size.
>
> CC: Benoit Cousson <bcousson@baylibre.com>
> CC: Tony Lindgren <tony@atomide.com>
> CC: Mugunthan V N <mugunthanvnm@ti.com>
> Signed-off-by: Roger Quadros <rogerq@ti.com>
This too:
Acked-by: Tony Lindgren <tony@atomide.com>
> ---
> arch/arm/boot/dts/am437x-gp-evm.dts | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/boot/dts/am437x-gp-evm.dts b/arch/arm/boot/dts/am437x-gp-evm.dts
> index 003766c..2674132 100644
> --- a/arch/arm/boot/dts/am437x-gp-evm.dts
> +++ b/arch/arm/boot/dts/am437x-gp-evm.dts
> @@ -277,8 +277,8 @@
>
> attb-gpio = <&gpio3 22 GPIO_ACTIVE_HIGH>;
>
> - x-size = <1024>;
> - y-size = <600>;
> + touchscreen-size-x = <1024>;
> + touchscreen-size-y = <600>;
> };
> };
>
> --
> 1.8.3.2
>
^ permalink raw reply
* Re: [PATCH v7 0/5] Input: pixcir_i2c_ts: Add Type-B Multi-touch and DT support
From: Tony Lindgren @ 2014-06-17 9:51 UTC (permalink / raw)
To: Roger Quadros
Cc: dmitry.torokhov, rydberg, balbi, dmurphy, mugunthanvnm, nsekhar,
linux-input, linux-kernel, linux-omap
In-Reply-To: <1402997507-9736-1-git-send-email-rogerq@ti.com>
* Roger Quadros <rogerq@ti.com> [140617 02:33]:
> Hi Dmitry,
>
> These are the pending patches that didn't go through in the 3.16 merge window.
> Please queue them for -next. Thanks.
>
> The series does the following
> - convert to Type-B multi touch protocol
> - support upto 5 fingers with hardware supplied tracking IDs
> - device tree support
>
> Tony,
>
> The last 2 patches fix the touch screen size property in the device tree since
> we now use the unified touch screen properties.
OK I acked those two pathes so Dmitry can take them too
when the driver changes get merged.
Regards,
Tony
^ permalink raw reply
* Re: [PATCH v7 0/5] Input: pixcir_i2c_ts: Add Type-B Multi-touch and DT support
From: Roger Quadros @ 2014-06-17 9:52 UTC (permalink / raw)
To: Tony Lindgren
Cc: dmitry.torokhov, rydberg, balbi, dmurphy, mugunthanvnm, nsekhar,
linux-input, linux-kernel, linux-omap
In-Reply-To: <20140617095132.GR17845@atomide.com>
On 06/17/2014 12:51 PM, Tony Lindgren wrote:
> * Roger Quadros <rogerq@ti.com> [140617 02:33]:
>> Hi Dmitry,
>>
>> These are the pending patches that didn't go through in the 3.16 merge window.
>> Please queue them for -next. Thanks.
>>
>> The series does the following
>> - convert to Type-B multi touch protocol
>> - support upto 5 fingers with hardware supplied tracking IDs
>> - device tree support
>>
>> Tony,
>>
>> The last 2 patches fix the touch screen size property in the device tree since
>> we now use the unified touch screen properties.
>
> OK I acked those two pathes so Dmitry can take them too
> when the driver changes get merged.
>
Thanks Tony.
cheers,
-roger
^ permalink raw reply
* Re: [PATCH v6] leds: USB: HID: Add support for MSI GT683R led panels
From: Johan Hovold @ 2014-06-17 13:46 UTC (permalink / raw)
To: Janne Kanniainen
Cc: jkosina, johan, cooloney, linux-kernel, linux-leds, linux-usb,
linux-input
In-Reply-To: <1402956115-1280-1-git-send-email-janne.kanniainen@gmail.com>
On Tue, Jun 17, 2014 at 01:01:55AM +0300, Janne Kanniainen wrote:
> This driver adds support for USB controlled led panels that exists in
> MSI GT683R laptop
>
> Signed-off-by: Janne Kanniainen <janne.kanniainen@gmail.com>
> ---
> Changes in v2:
> - sorted headers to alphabetic order
> - using devm_kzalloc
> - using BIT(n)
> - using usb_control_msg instead of usb_submit_urb
> - removing unneeded code
>
> Changes in v3:
> - implemented as HID device
> - some cleanups and bug fixes
>
> Changes in v4:
> - more cleanups
> - support for selecting leds
> - suppport for selecting status
>
> Changes in v5:
> - mode attribute documented under Documentation/ABI
> - made array for led_classdev
> - led devices uses now recommended naming scheme
>
> Changes in v6:
> - flush_work added
> - using hid device name instead of hard coded gt683r
> - allocating name buffers with devm_kzalloc
>
> There was a bug with "for", so I fixed it.
Then it really was a v7 and should have been marked as such, right?
> .../ABI/testing/sysfs-class-hid-driver-gt683r | 14 +
> drivers/hid/Kconfig | 16 ++
> drivers/hid/Makefile | 1 +
> drivers/hid/hid-core.c | 1 +
> drivers/hid/hid-gt683r.c | 318 +++++++++++++++++++++
> drivers/hid/hid-ids.h | 2 +-
> drivers/hid/usbhid/hid-quirks.c | 2 +-
> 7 files changed, 352 insertions(+), 2 deletions(-)
> create mode 100644 Documentation/ABI/testing/sysfs-class-hid-driver-gt683r
> create mode 100644 drivers/hid/hid-gt683r.c
>
> diff --git a/Documentation/ABI/testing/sysfs-class-hid-driver-gt683r b/Documentation/ABI/testing/sysfs-class-hid-driver-gt683r
> new file mode 100644
> index 0000000..6d0bd80
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-class-hid-driver-gt683r
> @@ -0,0 +1,14 @@
> +What: /sys/class/hidraw/<hidraw>/device/mode
Perhaps you should name this "leds_mode" or "led_panel_mode", or similar
(only change the attribute name, code is fine otherwise). This is an
attribute of the parent HID device, but it's not really apparent what
just "mode" would mean for such a device.
> +Date: Jun 2014
> +KernelVersion: 3.17
> +Contact: Janne Kanniainen <janne.kanniainen@gmail.com>
> +Description:
> + Set the mode of LEDs
> +
> + 0 - normal
> + 1 - audio
> + 2 - breathing
> +
> + Normal: LEDs are on
Change this to "LEDs are fully on when enabled", or similar?
> + Audio: LEDs brightness depends on sound level
> + Breathing: LEDs brightness varies at human breathing rate
> \ No newline at end of file
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 7af9d0b..b88cabd 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -261,6 +261,22 @@ config HOLTEK_FF
> Say Y here if you have a Holtek On Line Grip based game controller
> and want to have force feedback support for it.
>
> +config HID_GT683R
> + tristate "MSI GT68xR LED support"
> + depends on LEDS_CLASS && USB_HID
> + ---help---
> + Say Y here if you want to enable support for the MSI GT68xR LEDS
> +
> + This driver support following modes:
> + - Normal: LEDs are on
> + - Audio: LEDs brightness depends on sound level
> + - Breathing: LEDs brightness varies at human breathing rate
This might need an update as well.
> +
> + You can also select which leds you want to enable.
You could drop this line, and just say "enable support for the three
MSI" LEDs above?
> +
> + Currently the following devices are know to be supported:
> + - MSI GT683R
> +
> config HID_HUION
> tristate "Huion tablets"
> depends on USB_HID
> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> index fc712dd..7129311 100644
> --- a/drivers/hid/Makefile
> +++ b/drivers/hid/Makefile
> @@ -48,6 +48,7 @@ obj-$(CONFIG_HID_EMS_FF) += hid-emsff.o
> obj-$(CONFIG_HID_ELECOM) += hid-elecom.o
> obj-$(CONFIG_HID_ELO) += hid-elo.o
> obj-$(CONFIG_HID_EZKEY) += hid-ezkey.o
> +obj-$(CONFIG_HID_GT683R) += hid-gt683r.o
> obj-$(CONFIG_HID_GYRATION) += hid-gyration.o
> obj-$(CONFIG_HID_HOLTEK) += hid-holtek-kbd.o
> obj-$(CONFIG_HID_HOLTEK) += hid-holtek-mouse.o
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index da52279..ec88fdb 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -1827,6 +1827,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
> { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_WIRELESS_OPTICAL_DESKTOP_3_0) },
> { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_OFFICE_KB) },
> { HID_USB_DEVICE(USB_VENDOR_ID_MONTEREY, USB_DEVICE_ID_GENIUS_KB29E) },
> + { HID_USB_DEVICE(USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GT683R_LED_PANEL) },
> { HID_USB_DEVICE(USB_VENDOR_ID_NTRIG, USB_DEVICE_ID_NTRIG_TOUCH_SCREEN) },
> { HID_USB_DEVICE(USB_VENDOR_ID_NTRIG, USB_DEVICE_ID_NTRIG_TOUCH_SCREEN_1) },
> { HID_USB_DEVICE(USB_VENDOR_ID_NTRIG, USB_DEVICE_ID_NTRIG_TOUCH_SCREEN_2) },
> diff --git a/drivers/hid/hid-gt683r.c b/drivers/hid/hid-gt683r.c
> new file mode 100644
> index 0000000..1b16250
> --- /dev/null
> +++ b/drivers/hid/hid-gt683r.c
> @@ -0,0 +1,318 @@
> +/*
> + * MSI GT683R led driver
> + *
> + * Copyright (c) 2014 Janne Kanniainen <janne.kanniainen@gmail.com>
> + *
> + * 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; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include <linux/device.h>
> +#include <linux/hid.h>
> +#include <linux/kernel.h>
> +#include <linux/leds.h>
> +#include <linux/module.h>
> +
> +#include "hid-ids.h"
> +
> +#define GT683R_BUFFER_SIZE 8
> +
> +/*
> + * GT683R_LED_OFF: all LEDs are off
> + * GT683R_LED_AUDIO: LEDs brightness depends
> + * on sound level
> + * GT683R_LED_BREATHING: LEDs brightness varies
> + * at human breathing rate
No need to break these two lines (they would still be below 80 cols
wide).
> + * GT683R_LED_NORMAL: LEDs are on
> + */
> +enum gt683r_led_mode {
> + GT683R_LED_OFF = 0,
> + GT683R_LED_AUDIO = 2,
> + GT683R_LED_BREATHING = 3,
> + GT683R_LED_NORMAL = 5
> +};
> +
> +enum gt683r_panels {
> + GT683R_LED_BACK,
> + GT683R_LED_SIDE,
> + GT683R_LED_FRONT,
Perhaps initialise these three explicitly as the order really isn't
arbitrary?
> + GT683R_LED_COUNT,
> +};
> +
> +static const char * const gt683r_panel_names[] = {
> + "back",
> + "side",
> + "front",
> +};
> +
> +struct gt683r_led {
> + struct hid_device *hdev;
> + struct led_classdev led_devs[GT683R_LED_COUNT];
> + struct mutex lock;
> + struct work_struct work;
> + enum led_brightness brightnesses[GT683R_LED_COUNT];
> + enum gt683r_led_mode mode;
> +};
> +
> +static const struct hid_device_id gt683r_led_id[] = {
> + { HID_USB_DEVICE(USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GT683R_LED_PANEL) },
> + { }
> +};
> +
> +static void gt683r_brightness_set(struct led_classdev *led_cdev,
> + enum led_brightness brightness)
> +{
> + int i;
> + struct device *dev = led_cdev->dev->parent;
> + struct hid_device *hdev = container_of(dev, struct hid_device, dev);
> + struct gt683r_led *led = hid_get_drvdata(hdev);
> +
> + for (i = 0; i < GT683R_LED_COUNT; i++) {
> + if (&led->led_devs[i] == led_cdev)
> + break;
> + }
> +
> + if (i < GT683R_LED_COUNT) {
> + led->brightnesses[i] = brightness;
> + schedule_work(&led->work);
> + }
> +}
> +
> +static ssize_t mode_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + u8 sysfs_mode;
> + struct hid_device *hdev =
> + container_of(dev, struct hid_device, dev);
No need to break this (still < 80 cols).
> + struct gt683r_led *led = hid_get_drvdata(hdev);
> +
> + if (led->mode == GT683R_LED_NORMAL)
> + sysfs_mode = 0;
> + else if (led->mode == GT683R_LED_AUDIO)
> + sysfs_mode = 1;
> + else
> + sysfs_mode = 2;
> +
> + return scnprintf(buf, PAGE_SIZE, "%u\n", sysfs_mode);
> +}
> +
> +static ssize_t mode_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + u8 sysfs_mode;
> + struct hid_device *hdev =
> + container_of(dev, struct hid_device, dev);
No need to break this (still < 80 cols).
> + struct gt683r_led *led = hid_get_drvdata(hdev);
> +
> + mutex_lock(&led->lock);
> +
> + if (kstrtou8(buf, 10, &sysfs_mode) || sysfs_mode > 2) {
> + count = -EINVAL;
> + goto fail;
> + }
> +
> + if (sysfs_mode == 0)
> + led->mode = GT683R_LED_NORMAL;
> + else if (sysfs_mode == 1)
> + led->mode = GT683R_LED_AUDIO;
> + else
> + led->mode = GT683R_LED_BREATHING;
> +
> +fail:
> + mutex_unlock(&led->lock);
> +
> + if (count != -EINVAL)
> + schedule_work(&led->work);
> +
> + return count;
> +}
This could be simplified somewhat as the lock only needs to protect the
assignment to led->mode.
(Glad to see you realised I meant kstrouX when I wrote snprintf. ;))
> +
> +static int gt683r_led_snd_msg(struct gt683r_led *led, u8 *msg)
> +{
> + int ret;
> +
> + ret = hid_hw_raw_request(led->hdev, msg[0], msg, GT683R_BUFFER_SIZE,
> + HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
> + if (ret != GT683R_BUFFER_SIZE) {
> + hid_err(led->hdev,
> + "failed to send set report request: %i\n", ret);
> + if (ret < 0)
> + return ret;
> + return -EIO;
> + }
> +
> + return 0;
> +}
> +
> +static int gt683r_leds_set(struct gt683r_led *led, u8 leds)
> +{
> + int ret;
> + u8 *buffer;
> +
> + buffer = kzalloc(GT683R_BUFFER_SIZE, GFP_KERNEL);
> + if (!buffer)
> + return -ENOMEM;
> +
> + buffer[0] = 0x01;
> + buffer[1] = 0x02;
> + buffer[2] = 0x30;
> + buffer[3] = leds;
> + ret = gt683r_led_snd_msg(led, buffer);
> +
> + kfree(buffer);
> + return ret;
> +}
> +
> +static int gt683r_mode_set(struct gt683r_led *led, u8 mode)
> +{
> + int ret;
> + u8 *buffer;
> +
> + buffer = kzalloc(GT683R_BUFFER_SIZE, GFP_KERNEL);
> + if (!buffer)
> + return -ENOMEM;
> +
> + buffer[0] = 0x01;
> + buffer[1] = 0x02;
> + buffer[2] = 0x20;
> + buffer[3] = mode;
> + buffer[4] = 0x01;
> + ret = gt683r_led_snd_msg(led, buffer);
> +
> + kfree(buffer);
> + return ret;
> +}
> +
> +static void gt683r_led_work(struct work_struct *work)
> +{
> + u8 leds = 0, mode;
Try to stick to one definition per line (throughout), especially if
you're initialising as well.
> + struct gt683r_led *led =
> + container_of(work, struct gt683r_led, work);
> +
> + mutex_lock(&led->lock);
> +
> + if (led->brightnesses[GT683R_LED_BACK])
> + leds |= BIT(GT683R_LED_BACK);
> +
> + if (led->brightnesses[GT683R_LED_SIDE])
> + leds |= BIT(GT683R_LED_SIDE);
> +
> + if (led->brightnesses[GT683R_LED_FRONT])
> + leds |= BIT(GT683R_LED_FRONT);
This could of course be implemented as a loop as well, if you want...
> +
> + if (gt683r_leds_set(led, leds))
> + goto fail;
> +
> + if (leds)
> + mode = led->mode;
> + else
> + mode = GT683R_LED_OFF;
> +
> + gt683r_mode_set(led, mode);
> +fail:
> + mutex_unlock(&led->lock);
> +}
> +
> +static DEVICE_ATTR_RW(mode);
> +
> +static int gt683r_led_probe(struct hid_device *hdev,
> + const struct hid_device_id *id)
> +{
> + int ret, i, name_sz;
> + char *name;
> + struct gt683r_led *led;
> +
> + led = devm_kzalloc(&hdev->dev, sizeof(struct gt683r_led), GFP_KERNEL);
> + if (!led)
> + return -ENOMEM;
> +
> + led->mode = GT683R_LED_NORMAL;
> + led->hdev = hdev;
> + hid_set_drvdata(hdev, led);
> +
> + ret = hid_parse(hdev);
> + if (ret) {
> + hid_err(hdev, "hid parsing failed\n");
> + goto fail;
Just return ret here.
> + }
> +
> + ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
> + if (ret) {
> + hid_err(hdev, "hw start failed\n");
> + goto fail;
> + }
> +
> + for (i = 0; i < GT683R_LED_COUNT; i++) {
> + name_sz = strlen(dev_name(&hdev->dev)) +
> + strlen(gt683r_panel_names[i]) + 3;
> +
> + name = devm_kzalloc(&hdev->dev, sizeof(char) * name_sz,
> + GFP_KERNEL);
Must check for allocation failure still.
> +
> + snprintf(name, name_sz, "%s::%s",
> + dev_name(&hdev->dev), gt683r_panel_names[i]);
> + led->led_devs[i].name = name;
> + led->led_devs[i].max_brightness = 1;
> + led->led_devs[i].brightness_set = gt683r_brightness_set;
> + ret = led_classdev_register(&hdev->dev, &led->led_devs[i]);
> + if (ret) {
> + hid_err(hdev, "could not register led device\n");
> + goto fail2;
> + }
> + }
> +
> + ret = device_create_file(&led->hdev->dev,
> + &dev_attr_mode);
No need to break line.
> + if (ret) {
> + hid_err(hdev, "could not make mode attribute file\n");
> + goto fail2;
> + }
> +
> + mutex_init(&led->lock);
> + INIT_WORK(&led->work, gt683r_led_work);
> +
> + return 0;
> +
> +fail2:
> + for (; i >= 0; i--)
You must initialise i to (i - 1) as well (either led_class_devregister
failed or i == GT683R_LED_COUNT here).
> + led_classdev_unregister(&led->led_devs[i]);
> + hid_hw_stop(hdev);
> +fail:
> + return ret;
> +}
> +
> +static void gt683r_led_remove(struct hid_device *hdev)
> +{
> + int i;
> + struct gt683r_led *led = hid_get_drvdata(hdev);
> +
> + for (i = 0; i < GT683R_LED_COUNT; i++)
> + led_classdev_unregister(&led->led_devs[i]);
> + device_remove_file(&hdev->dev,
> + &dev_attr_mode);
No need to break line.
Perhaps do the removal before unregistering to reverse probe.
> + flush_work(&led->work);
> + hid_hw_stop(hdev);
> +}
> +
> +static struct hid_driver gt683r_led_driver = {
> + .probe = gt683r_led_probe,
> + .remove = gt683r_led_remove,
> + .name = "gt683r_led",
> + .id_table = gt683r_led_id,
> +};
> +
> +module_hid_driver(gt683r_led_driver);
> +
> +MODULE_AUTHOR("Janne Kanniainen");
> +MODULE_DESCRIPTION("MSI GT683R led driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 34bb220..3692d37 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -641,7 +641,7 @@
> #define USB_DEVICE_ID_GENIUS_KB29E 0x3004
>
> #define USB_VENDOR_ID_MSI 0x1770
> -#define USB_DEVICE_ID_MSI_GX680R_LED_PANEL 0xff00
> +#define USB_DEVICE_ID_MSI_GT683R_LED_PANEL 0xff00
>
> #define USB_VENDOR_ID_NATIONAL_SEMICONDUCTOR 0x0400
> #define USB_DEVICE_ID_N_S_HARMONY 0xc359
> diff --git a/drivers/hid/usbhid/hid-quirks.c b/drivers/hid/usbhid/hid-quirks.c
> index 8e4ddb3..c640e1d 100644
> --- a/drivers/hid/usbhid/hid-quirks.c
> +++ b/drivers/hid/usbhid/hid-quirks.c
> @@ -73,7 +73,7 @@ static const struct hid_blacklist {
> { USB_VENDOR_ID_FORMOSA, USB_DEVICE_ID_FORMOSA_IR_RECEIVER, HID_QUIRK_NO_INIT_REPORTS },
> { USB_VENDOR_ID_FREESCALE, USB_DEVICE_ID_FREESCALE_MX28, HID_QUIRK_NOGET },
> { USB_VENDOR_ID_MGE, USB_DEVICE_ID_MGE_UPS, HID_QUIRK_NOGET },
> - { USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GX680R_LED_PANEL, HID_QUIRK_NO_INIT_REPORTS },
> + { USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GT683R_LED_PANEL, HID_QUIRK_NO_INIT_REPORTS },
> { USB_VENDOR_ID_NEXIO, USB_DEVICE_ID_NEXIO_MULTITOUCH_PTI0750, HID_QUIRK_NO_INIT_REPORTS },
> { USB_VENDOR_ID_NOVATEK, USB_DEVICE_ID_NOVATEK_MOUSE, HID_QUIRK_NO_INIT_REPORTS },
> { USB_VENDOR_ID_PIXART, USB_DEVICE_ID_PIXART_OPTICAL_TOUCH_SCREEN, HID_QUIRK_NO_INIT_REPORTS },
Johan
^ permalink raw reply
* [PATCH v8] leds: USB: HID: Add support for MSI GT683R led panels
From: Janne Kanniainen @ 2014-06-17 16:41 UTC (permalink / raw)
To: jkosina
Cc: johan, cooloney, linux-kernel, linux-leds, linux-usb, linux-input,
Janne Kanniainen
In-Reply-To: <20140617134602.GB28179@localhost>
This driver adds support for USB controlled led panels that exists in
MSI GT683R laptop
Changes in v2:
- sorted headers to alphabetic order
- using devm_kzalloc
- using BIT(n)
- using usb_control_msg instead of usb_submit_urb
- removing unneeded code
Changes in v3:
- implemented as HID device
- some cleanups and bug fixes
Changes in v4:
- more cleanups
- support for selecting leds
- suppport for selecting status
Changes in v5:
- mode attribute documented under Documentation/ABI
- made array for led_classdev
- led devices uses now recommended naming scheme
Changes in v6:
- flush_work added
- using hid device name instead of hard coded gt683r
- allocating name buffers with devm_kzalloc
Changes in v7:
- buf with for fixed
Changes in v8:
- some cleanups and bugs fixed
Signed-off-by: Janne Kanniainen <janne.kanniainen@gmail.com>
---
.../ABI/testing/sysfs-class-hid-driver-gt683r | 14 +
drivers/hid/Kconfig | 14 +
drivers/hid/Makefile | 1 +
drivers/hid/hid-core.c | 1 +
drivers/hid/hid-gt683r.c | 308 +++++++++++++++++++++
drivers/hid/hid-ids.h | 2 +-
drivers/hid/usbhid/hid-quirks.c | 2 +-
7 files changed, 340 insertions(+), 2 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-class-hid-driver-gt683r
create mode 100644 drivers/hid/hid-gt683r.c
diff --git a/Documentation/ABI/testing/sysfs-class-hid-driver-gt683r b/Documentation/ABI/testing/sysfs-class-hid-driver-gt683r
new file mode 100644
index 0000000..317e9d5
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-hid-driver-gt683r
@@ -0,0 +1,14 @@
+What: /sys/class/hidraw/<hidraw>/device/leds_mode
+Date: Jun 2014
+KernelVersion: 3.17
+Contact: Janne Kanniainen <janne.kanniainen@gmail.com>
+Description:
+ Set the mode of LEDs
+
+ 0 - normal
+ 1 - audio
+ 2 - breathing
+
+ Normal: LEDs are fully on when enabled
+ Audio: LEDs brightness depends on sound level
+ Breathing: LEDs brightness varies at human breathing rate
\ No newline at end of file
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 7af9d0b..e2f4590 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -261,6 +261,20 @@ config HOLTEK_FF
Say Y here if you have a Holtek On Line Grip based game controller
and want to have force feedback support for it.
+config HID_GT683R
+ tristate "MSI GT68xR LED support"
+ depends on LEDS_CLASS && USB_HID
+ ---help---
+ Say Y here if you want to enable support for the three MSI GT68xR LEDs
+
+ This driver support following modes:
+ - Normal: LEDs are fully on when enabled
+ - Audio: LEDs brightness depends on sound level
+ - Breathing: LEDs brightness varies at human breathing rate
+
+ Currently the following devices are know to be supported:
+ - MSI GT683R
+
config HID_HUION
tristate "Huion tablets"
depends on USB_HID
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index fc712dd..7129311 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -48,6 +48,7 @@ obj-$(CONFIG_HID_EMS_FF) += hid-emsff.o
obj-$(CONFIG_HID_ELECOM) += hid-elecom.o
obj-$(CONFIG_HID_ELO) += hid-elo.o
obj-$(CONFIG_HID_EZKEY) += hid-ezkey.o
+obj-$(CONFIG_HID_GT683R) += hid-gt683r.o
obj-$(CONFIG_HID_GYRATION) += hid-gyration.o
obj-$(CONFIG_HID_HOLTEK) += hid-holtek-kbd.o
obj-$(CONFIG_HID_HOLTEK) += hid-holtek-mouse.o
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index da52279..ec88fdb 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1827,6 +1827,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_WIRELESS_OPTICAL_DESKTOP_3_0) },
{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_OFFICE_KB) },
{ HID_USB_DEVICE(USB_VENDOR_ID_MONTEREY, USB_DEVICE_ID_GENIUS_KB29E) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GT683R_LED_PANEL) },
{ HID_USB_DEVICE(USB_VENDOR_ID_NTRIG, USB_DEVICE_ID_NTRIG_TOUCH_SCREEN) },
{ HID_USB_DEVICE(USB_VENDOR_ID_NTRIG, USB_DEVICE_ID_NTRIG_TOUCH_SCREEN_1) },
{ HID_USB_DEVICE(USB_VENDOR_ID_NTRIG, USB_DEVICE_ID_NTRIG_TOUCH_SCREEN_2) },
diff --git a/drivers/hid/hid-gt683r.c b/drivers/hid/hid-gt683r.c
new file mode 100644
index 0000000..19959ac
--- /dev/null
+++ b/drivers/hid/hid-gt683r.c
@@ -0,0 +1,308 @@
+/*
+ * MSI GT683R led driver
+ *
+ * Copyright (c) 2014 Janne Kanniainen <janne.kanniainen@gmail.com>
+ *
+ * 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; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/device.h>
+#include <linux/hid.h>
+#include <linux/kernel.h>
+#include <linux/leds.h>
+#include <linux/module.h>
+
+#include "hid-ids.h"
+
+#define GT683R_BUFFER_SIZE 8
+
+/*
+ * GT683R_LED_OFF: all LEDs are off
+ * GT683R_LED_AUDIO: LEDs brightness depends on sound level
+ * GT683R_LED_BREATHING: LEDs brightness varies at human breathing rate
+ * GT683R_LED_NORMAL: LEDs are fully on when enabled
+ */
+enum gt683r_led_mode {
+ GT683R_LED_OFF = 0,
+ GT683R_LED_AUDIO = 2,
+ GT683R_LED_BREATHING = 3,
+ GT683R_LED_NORMAL = 5
+};
+
+enum gt683r_panels {
+ GT683R_LED_BACK = 0,
+ GT683R_LED_SIDE = 1,
+ GT683R_LED_FRONT = 2,
+ GT683R_LED_COUNT,
+};
+
+static const char * const gt683r_panel_names[] = {
+ "back",
+ "side",
+ "front",
+};
+
+struct gt683r_led {
+ struct hid_device *hdev;
+ struct led_classdev led_devs[GT683R_LED_COUNT];
+ struct mutex lock;
+ struct work_struct work;
+ enum led_brightness brightnesses[GT683R_LED_COUNT];
+ enum gt683r_led_mode mode;
+};
+
+static const struct hid_device_id gt683r_led_id[] = {
+ { HID_USB_DEVICE(USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GT683R_LED_PANEL) },
+ { }
+};
+
+static void gt683r_brightness_set(struct led_classdev *led_cdev,
+ enum led_brightness brightness)
+{
+ int i;
+ struct device *dev = led_cdev->dev->parent;
+ struct hid_device *hdev = container_of(dev, struct hid_device, dev);
+ struct gt683r_led *led = hid_get_drvdata(hdev);
+
+ for (i = 0; i < GT683R_LED_COUNT; i++) {
+ if (&led->led_devs[i] == led_cdev)
+ break;
+ }
+
+ if (i < GT683R_LED_COUNT) {
+ led->brightnesses[i] = brightness;
+ schedule_work(&led->work);
+ }
+}
+
+static ssize_t leds_mode_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ u8 sysfs_mode;
+ struct hid_device *hdev = container_of(dev, struct hid_device, dev);
+ struct gt683r_led *led = hid_get_drvdata(hdev);
+
+ if (led->mode == GT683R_LED_NORMAL)
+ sysfs_mode = 0;
+ else if (led->mode == GT683R_LED_AUDIO)
+ sysfs_mode = 1;
+ else
+ sysfs_mode = 2;
+
+ return scnprintf(buf, PAGE_SIZE, "%u\n", sysfs_mode);
+}
+
+static ssize_t leds_mode_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ u8 sysfs_mode;
+ struct hid_device *hdev = container_of(dev, struct hid_device, dev);
+ struct gt683r_led *led = hid_get_drvdata(hdev);
+
+
+ if (kstrtou8(buf, 10, &sysfs_mode) || sysfs_mode > 2)
+ return -EINVAL;
+
+ mutex_lock(&led->lock);
+
+ if (sysfs_mode == 0)
+ led->mode = GT683R_LED_NORMAL;
+ else if (sysfs_mode == 1)
+ led->mode = GT683R_LED_AUDIO;
+ else
+ led->mode = GT683R_LED_BREATHING;
+
+ mutex_unlock(&led->lock);
+ schedule_work(&led->work);
+
+ return count;
+}
+
+static int gt683r_led_snd_msg(struct gt683r_led *led, u8 *msg)
+{
+ int ret;
+
+ ret = hid_hw_raw_request(led->hdev, msg[0], msg, GT683R_BUFFER_SIZE,
+ HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
+ if (ret != GT683R_BUFFER_SIZE) {
+ hid_err(led->hdev,
+ "failed to send set report request: %i\n", ret);
+ if (ret < 0)
+ return ret;
+ return -EIO;
+ }
+
+ return 0;
+}
+
+static int gt683r_leds_set(struct gt683r_led *led, u8 leds)
+{
+ int ret;
+ u8 *buffer;
+
+ buffer = kzalloc(GT683R_BUFFER_SIZE, GFP_KERNEL);
+ if (!buffer)
+ return -ENOMEM;
+
+ buffer[0] = 0x01;
+ buffer[1] = 0x02;
+ buffer[2] = 0x30;
+ buffer[3] = leds;
+ ret = gt683r_led_snd_msg(led, buffer);
+
+ kfree(buffer);
+ return ret;
+}
+
+static int gt683r_mode_set(struct gt683r_led *led, u8 mode)
+{
+ int ret;
+ u8 *buffer;
+
+ buffer = kzalloc(GT683R_BUFFER_SIZE, GFP_KERNEL);
+ if (!buffer)
+ return -ENOMEM;
+
+ buffer[0] = 0x01;
+ buffer[1] = 0x02;
+ buffer[2] = 0x20;
+ buffer[3] = mode;
+ buffer[4] = 0x01;
+ ret = gt683r_led_snd_msg(led, buffer);
+
+ kfree(buffer);
+ return ret;
+}
+
+static void gt683r_led_work(struct work_struct *work)
+{
+ int i;
+ u8 leds = 0;
+ u8 mode;
+ struct gt683r_led *led = container_of(work, struct gt683r_led, work);
+
+ mutex_lock(&led->lock);
+
+ for (i = 0; i < GT683R_LED_COUNT; i++) {
+ if (led->brightnesses[i])
+ leds |= BIT(i);
+ }
+
+ if (gt683r_leds_set(led, leds))
+ goto fail;
+
+ if (leds)
+ mode = led->mode;
+ else
+ mode = GT683R_LED_OFF;
+
+ gt683r_mode_set(led, mode);
+fail:
+ mutex_unlock(&led->lock);
+}
+
+static DEVICE_ATTR_RW(leds_mode);
+
+static int gt683r_led_probe(struct hid_device *hdev,
+ const struct hid_device_id *id)
+{
+ int i, ret, name_sz;
+ char *name;
+ struct gt683r_led *led;
+
+ led = devm_kzalloc(&hdev->dev, sizeof(struct gt683r_led), GFP_KERNEL);
+ if (!led)
+ return -ENOMEM;
+
+ led->mode = GT683R_LED_NORMAL;
+ led->hdev = hdev;
+ hid_set_drvdata(hdev, led);
+
+ ret = hid_parse(hdev);
+ if (ret) {
+ hid_err(hdev, "hid parsing failed\n");
+ return ret;
+ }
+
+ ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
+ if (ret) {
+ hid_err(hdev, "hw start failed\n");
+ return ret;
+ }
+
+ for (i = 0; i < GT683R_LED_COUNT; i++) {
+ name_sz = strlen(dev_name(&hdev->dev)) +
+ strlen(gt683r_panel_names[i]) + 3;
+
+ name = devm_kzalloc(&hdev->dev, sizeof(char) * name_sz,
+ GFP_KERNEL);
+ if (!name) {
+ ret = -ENOMEM;
+ goto fail;
+ }
+
+ snprintf(name, name_sz, "%s::%s",
+ dev_name(&hdev->dev), gt683r_panel_names[i]);
+ led->led_devs[i].name = name;
+ led->led_devs[i].max_brightness = 1;
+ led->led_devs[i].brightness_set = gt683r_brightness_set;
+ ret = led_classdev_register(&hdev->dev, &led->led_devs[i]);
+ if (ret) {
+ hid_err(hdev, "could not register led device\n");
+ goto fail;
+ }
+ }
+
+ ret = device_create_file(&led->hdev->dev, &dev_attr_leds_mode);
+ if (ret) {
+ hid_err(hdev, "could not make mode attribute file\n");
+ goto fail;
+ }
+
+ mutex_init(&led->lock);
+ INIT_WORK(&led->work, gt683r_led_work);
+
+ return 0;
+
+fail:
+ for (i = i - 1; i >= 0; i--)
+ led_classdev_unregister(&led->led_devs[i]);
+ hid_hw_stop(hdev);
+ return ret;
+}
+
+static void gt683r_led_remove(struct hid_device *hdev)
+{
+ int i;
+ struct gt683r_led *led = hid_get_drvdata(hdev);
+
+ device_remove_file(&hdev->dev, &dev_attr_leds_mode);
+ for (i = 0; i < GT683R_LED_COUNT; i++)
+ led_classdev_unregister(&led->led_devs[i]);
+ flush_work(&led->work);
+ hid_hw_stop(hdev);
+}
+
+static struct hid_driver gt683r_led_driver = {
+ .probe = gt683r_led_probe,
+ .remove = gt683r_led_remove,
+ .name = "gt683r_led",
+ .id_table = gt683r_led_id,
+};
+
+module_hid_driver(gt683r_led_driver);
+
+MODULE_AUTHOR("Janne Kanniainen");
+MODULE_DESCRIPTION("MSI GT683R led driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 34bb220..3692d37 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -641,7 +641,7 @@
#define USB_DEVICE_ID_GENIUS_KB29E 0x3004
#define USB_VENDOR_ID_MSI 0x1770
-#define USB_DEVICE_ID_MSI_GX680R_LED_PANEL 0xff00
+#define USB_DEVICE_ID_MSI_GT683R_LED_PANEL 0xff00
#define USB_VENDOR_ID_NATIONAL_SEMICONDUCTOR 0x0400
#define USB_DEVICE_ID_N_S_HARMONY 0xc359
diff --git a/drivers/hid/usbhid/hid-quirks.c b/drivers/hid/usbhid/hid-quirks.c
index 8e4ddb3..c640e1d 100644
--- a/drivers/hid/usbhid/hid-quirks.c
+++ b/drivers/hid/usbhid/hid-quirks.c
@@ -73,7 +73,7 @@ static const struct hid_blacklist {
{ USB_VENDOR_ID_FORMOSA, USB_DEVICE_ID_FORMOSA_IR_RECEIVER, HID_QUIRK_NO_INIT_REPORTS },
{ USB_VENDOR_ID_FREESCALE, USB_DEVICE_ID_FREESCALE_MX28, HID_QUIRK_NOGET },
{ USB_VENDOR_ID_MGE, USB_DEVICE_ID_MGE_UPS, HID_QUIRK_NOGET },
- { USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GX680R_LED_PANEL, HID_QUIRK_NO_INIT_REPORTS },
+ { USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GT683R_LED_PANEL, HID_QUIRK_NO_INIT_REPORTS },
{ USB_VENDOR_ID_NEXIO, USB_DEVICE_ID_NEXIO_MULTITOUCH_PTI0750, HID_QUIRK_NO_INIT_REPORTS },
{ USB_VENDOR_ID_NOVATEK, USB_DEVICE_ID_NOVATEK_MOUSE, HID_QUIRK_NO_INIT_REPORTS },
{ USB_VENDOR_ID_PIXART, USB_DEVICE_ID_PIXART_OPTICAL_TOUCH_SCREEN, HID_QUIRK_NO_INIT_REPORTS },
--
1.9.2
^ permalink raw reply related
* Re: [PATCH 1/2] touchscreen: sun4i-ts: A10 (sun4i) has double the temperature precision
From: Hans de Goede @ 2014-06-17 17:15 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Maxime Ripard, Tong Zhang, linux-input-u79uwXL29TY76Z2rM5mHXA,
LM Sensors, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <20140616213048.GA422-WlK9ik9hQGAhIp7JRqBPierSzoNAToWh@public.gmane.org>
Hi,
On 06/16/2014 11:30 PM, Dmitry Torokhov wrote:
> Hi Hans,
>
> On Mon, Jun 16, 2014 at 08:24:28PM +0200, Hans de Goede wrote:
>> Testing has revealed that the temperature in the rtp controller of the A10
>> (sun4i) SoC has a resolution of 50 milli degrees / step, where as the
>> A13 (sun5i) and later models have 100 milli degrees / step.
>>
>> Add a new sun5i-a13-ts compatible to differentiate the newer models and
>> set the resolution based on the compatible string.
>>
>> This fixes the temperature reported on the A10 being twice as high as expected.
>
> Should we maybe add explicit temperature steps property so that we won;t
> need to patch again if they decided to change resolution again?
Self NAK, further testing has revealed that it is likely not the resolution which is
different on the A10, but that we need to apply a different offset.
Regards,
Hans
^ permalink raw reply
* Re: [RESEND: PATCH 1/2] mfd: tc3589x: Add device tree bindings
From: Lee Jones @ 2014-06-17 20:23 UTC (permalink / raw)
To: Linus Walleij
Cc: devicetree, Dmitry Torokhov, linux-input, Samuel Ortiz,
linux-kernel, linux-arm-kernel, Mark Rutland
In-Reply-To: <1397028480-15413-1-git-send-email-linus.walleij@linaro.org>
On Wed, 09 Apr 2014, Linus Walleij wrote:
> This defines the device tree bindings for the Toshiba TC3589x
> series of multi-purpose expanders. Only the stuff I can test
> is defined: GPIO and keypad. Others may implement more
> subdevices further down the road.
>
> This is to complement
> commit a435ae1d51e2f18414f2a87219fdbe068231e692
> "mfd: Enable the tc3589x for Device Tree" which left off
> the definition of the device tree bindings.
>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v2->v3:
> - Fix the keys/rows bindings to be u32 rather than
> /bits/ 8, inconsistency noted by Mark Rutland.
> ChangeLog v1->v2:
> - Include a verbose example in the DT bindings.
> - Explain why this is a stand-alone bindings patch.
> ---
> Documentation/devicetree/bindings/mfd/tc3589x.txt | 107 ++++++++++++++++++++++
> 1 file changed, 107 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mfd/tc3589x.txt
Looks about right, been on the list for ages and doesn't add any new
properties. Applied, thanks.
> diff --git a/Documentation/devicetree/bindings/mfd/tc3589x.txt b/Documentation/devicetree/bindings/mfd/tc3589x.txt
> new file mode 100644
> index 000000000000..c6ac5bd2ce51
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/tc3589x.txt
> @@ -0,0 +1,107 @@
> +* Toshiba TC3589x multi-purpose expander
> +
> +The Toshiba TC3589x series are I2C-based MFD devices which may expose the
> +following built-in devices: gpio, keypad, rotator (vibrator), PWM (for
> +e.g. LEDs or vibrators) The included models are:
> +
> +- TC35890
> +- TC35892
> +- TC35893
> +- TC35894
> +- TC35895
> +- TC35896
> +
> +Required properties:
> + - compatible : must be "toshiba,tc35890", "toshiba,tc35892", "toshiba,tc35893",
> + "toshiba,tc35894", "toshiba,tc35895" or "toshiba,tc35896"
> + - reg : I2C address of the device
> + - interrupt-parent : specifies which IRQ controller we're connected to
> + - interrupts : the interrupt on the parent the controller is connected to
> + - interrupt-controller : marks the device node as an interrupt controller
> + - #interrupt-cells : should be <1>, the first cell is the IRQ offset on this
> + TC3589x interrupt controller.
> +
> +Optional nodes:
> +
> +- GPIO
> + This GPIO module inside the TC3589x has 24 (TC35890, TC35892) or 20
> + (other models) GPIO lines.
> + - compatible : must be "toshiba,tc3589x-gpio"
> + - interrupts : interrupt on the parent, which must be the tc3589x MFD device
> + - interrupt-controller : marks the device node as an interrupt controller
> + - #interrupt-cells : should be <2>, the first cell is the IRQ offset on this
> + TC3589x GPIO interrupt controller, the second cell is the interrupt flags
> + in accordance with <dt-bindings/interrupt-controller/irq.h>. The following
> + flags are valid:
> + - IRQ_TYPE_LEVEL_LOW
> + - IRQ_TYPE_LEVEL_HIGH
> + - IRQ_TYPE_EDGE_RISING
> + - IRQ_TYPE_EDGE_FALLING
> + - IRQ_TYPE_EDGE_BOTH
> + - gpio-controller : marks the device node as a GPIO controller
> + - #gpio-cells : should be <2>, the first cell is the GPIO offset on this
> + GPIO controller, the second cell is the flags.
> +
> +- Keypad
> + This keypad is the same on all variants, supporting up to 96 different
> + keys. The linux-specific properties are modeled on those already existing
> + in other input drivers.
> + - compatible : must be "toshiba,tc3589x-keypad"
> + - debounce-delay-ms : debounce interval in milliseconds
> + - keypad,num-rows : number of rows in the matrix, see
> + bindings/input/matrix-keymap.txt
> + - keypad,num-columns : number of columns in the matrix, see
> + bindings/input/matrix-keymap.txt
> + - linux,keymap: the definition can be found in
> + bindings/input/matrix-keymap.txt
> + - linux,no-autorepeat: do no enable autorepeat feature.
> + - linux,wakeup: use any event on keypad as wakeup event.
> +
> +Example:
> +
> +tc35893@44 {
> + compatible = "toshiba,tc35893";
> + reg = <0x44>;
> + interrupt-parent = <&gpio6>;
> + interrupts = <26 IRQ_TYPE_EDGE_RISING>;
> +
> + interrupt-controller;
> + #interrupt-cells = <1>;
> +
> + tc3589x_gpio {
> + compatible = "toshiba,tc3589x-gpio";
> + interrupts = <0>;
> +
> + interrupt-controller;
> + #interrupt-cells = <2>;
> + gpio-controller;
> + #gpio-cells = <2>;
> + };
> + tc3589x_keypad {
> + compatible = "toshiba,tc3589x-keypad";
> + interrupts = <6>;
> + debounce-delay-ms = <4>;
> + keypad,num-columns = <8>;
> + keypad,num-rows = <8>;
> + linux,no-autorepeat;
> + linux,wakeup;
> + linux,keymap = <0x0301006b
> + 0x04010066
> + 0x06040072
> + 0x040200d7
> + 0x0303006a
> + 0x0205000e
> + 0x0607008b
> + 0x0500001c
> + 0x0403000b
> + 0x03040034
> + 0x05020067
> + 0x0305006c
> + 0x040500e7
> + 0x0005009e
> + 0x06020073
> + 0x01030039
> + 0x07060069
> + 0x050500d9>;
> + };
> +};
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH] Input - wacom: put a flag when the led are initialized
From: Ping Cheng @ 2014-06-17 21:14 UTC (permalink / raw)
To: linux-input, linux-kernel@vger.kernel.org
Hi Benjamin,
On Monday, June 16, 2014, Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
>
> Hi Ping,
>
> On Jun 13 2014 or thereabouts, Ping Cheng wrote:
> > Hi Benjamin,
> >
> > On Fri, Jun 13, 2014 at 1:29 PM, Benjamin Tissoires
> > <benjamin.tissoires@redhat.com> wrote:
> > > This solves a bug with the wireless receiver:
> >
> > Your patch does get rid of the crash. But, it does not fix it at the
> > root cause.
>
> True, it fixes the crash, does not get rid of the root cause, but it is
> still required. See later.
>
> >
> > > - at plug, the wireless receiver does not know which Wacom device it is
> > > connected to, so it does not actually creates all the LEDs
> >
> > This is the root cause - LEDs are not created for wireless devices.
> > Neither here, nor later when a real device is connected.
>
> Yep
>
> >
> > > - when the tablet connects, wacom->wacom_wac.features.type is set to the
> > > proper device so that wacom_wac can understand the packets
> >
> > LEDs are not created for any wireless devices since we don't call
> > wacom_initialize_leds() when real tablets are connected.
>
> Yep
>
> >
> > > - when the receiver is unplugged, it detects that a LED should have been
> > > created (based on wacom->wacom_wac.features.type) and tries to remove
> > > it: crash when removing the sysfs group.
> >
> > When receiver is unplugged, it remembers the last tablet that
> > connected to it. If that tablet supports LEDs, wacom_destroy_leds() is
> > called. But, no LEDs were initialized. That's why it crashes.
>
> Yep
>
> >
> > > Side effect, we can now safely call several times wacom_destroy_leds().
> >
> > led_initialized will never be true if we keep wacom_initialize_leds()
> > inside probe().
>
> If you are talking about wireless devices only, yes, this value will
> never be true. That's the purpose of this patch actually :)
>
> >
> > To make initialize_leds() and desctroy_leds() work for wireless
> > devices, we need to move them to wacom_wireless_work() where we know
> > what type of tablet is connected/disconnected.
>
> Unfortunately, this does not work:
> - we *can* create the LEDs sysfs in the wireless work
> - this will prevent the crash
> - the user will think it can control the LEDs
> - actually these LEDs control will do nothing because LEDs handling for
> wireless devices goes through the WL interface, and not the PEN
> interface of the WL receiver.
> - we need to implement a specific led_handling for the wireless receiver
> (which will need to know which type of tablet is connected to it)
> - we still need a way to say that the pen intf which is declared as the
> connected device does not has a LED sysfs.
>
> We could add a quirk to the wacom_wac->features saying that the
> connection is wireless, so there is no LED attached to the interface.
>
> Still, there will be some work to do to properly handle the LED
> configuration from the WL receiver. This work has to be done in the
> kernel, but also in the user space (g-s-d) because now, the led control
> will not be on the pen device, but on a plain usb device without input.
>
> If you prefer, I can add such a quirk. But my only concern is here to
> fix the kernel oops, not to add features which would require more
> testing across different hardware and development on the user space
> side.
>
> >
> > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> >
> > Thank you for your support. But, sorry
> >
> > NAKed-by: Ping Cheng <pingc@wacom.com>
>
> Please reconsider it or validate the quirk approach I mentioned.
I see your point. My concern was once we fixed the crash here, we
would forget to fix the actual problem.
For the time being, let's make sure we can move on. So,
Acked-by: Ping Cheng <pingc@wacom.com>.
Cheers,
Ping
^ permalink raw reply
* Re: [PATCH 08/10] mfd: cros_ec: cleanup: Remove EC wrapper functions
From: Simon Glass @ 2014-06-18 3:42 UTC (permalink / raw)
To: Doug Anderson
Cc: Lee Jones, Andrew Bresticker, Stephen Warren, Olof Johansson,
Sonny Rao, linux-samsung-soc, Javier Martinez Canillas,
Bill Richardson, Wolfram Sang, Mark Brown, Dmitry Torokhov,
Samuel Ortiz, Vincent Palatin, Geert Uytterhoeven,
linux-i2c@vger.kernel.org, lk, linux-input@vger.kernel.org
In-Reply-To: <1402954800-28215-9-git-send-email-dianders@chromium.org>
Hi Doug,
On 16 June 2014 14:39, Doug Anderson <dianders@chromium.org> wrote:
> From: Bill Richardson <wfrichar@chromium.org>
>
> Remove the three wrapper functions that talk to the EC without passing all
> the desired arguments and just use the underlying communication function
> that passes everything in a struct intead.
>
> This is internal code refactoring only. Nothing should change.
>
> Signed-off-by: Bill Richardson <wfrichar@chromium.org>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> ---
> drivers/i2c/busses/i2c-cros-ec-tunnel.c | 15 +++++++++++----
> drivers/input/keyboard/cros_ec_keyb.c | 14 ++++++++++++--
> drivers/mfd/cros_ec.c | 32 --------------------------------
> include/linux/mfd/cros_ec.h | 19 ++++++-------------
> 4 files changed, 29 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-cros-ec-tunnel.c b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
> index 8e7a714..dd07818 100644
> --- a/drivers/i2c/busses/i2c-cros-ec-tunnel.c
> +++ b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
> @@ -183,6 +183,7 @@ static int ec_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg i2c_msgs[],
> u8 *request = NULL;
> u8 *response = NULL;
> int result;
> + struct cros_ec_command msg;
>
> request_len = ec_i2c_count_message(i2c_msgs, num);
> if (request_len < 0) {
> @@ -218,9 +219,15 @@ static int ec_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg i2c_msgs[],
> }
>
> ec_i2c_construct_message(request, i2c_msgs, num, bus_num);
> - result = bus->ec->command_sendrecv(bus->ec, EC_CMD_I2C_PASSTHRU,
> - request, request_len,
> - response, response_len);
> +
> + msg.version = 0;
> + msg.command = EC_CMD_I2C_PASSTHRU;
> + msg.outdata = request;
> + msg.outsize = request_len;
> + msg.indata = response;
> + msg.insize = response_len;
> +
> + result = bus->ec->cmd_xfer(bus->ec, &msg);
> if (result)
> goto exit;
>
> @@ -258,7 +265,7 @@ static int ec_i2c_probe(struct platform_device *pdev)
> u32 remote_bus;
> int err;
>
> - if (!ec->command_sendrecv) {
> + if (!ec->cmd_xfer) {
> dev_err(dev, "Missing sendrecv\n");
> return -EINVAL;
> }
> diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
> index 4083796..dc37b6b 100644
> --- a/drivers/input/keyboard/cros_ec_keyb.c
> +++ b/drivers/input/keyboard/cros_ec_keyb.c
> @@ -191,8 +191,18 @@ static void cros_ec_keyb_close(struct input_dev *dev)
>
> static int cros_ec_keyb_get_state(struct cros_ec_keyb *ckdev, uint8_t *kb_state)
> {
> - return ckdev->ec->command_recv(ckdev->ec, EC_CMD_MKBP_STATE,
> - kb_state, ckdev->cols);
> + int ret;
> + struct cros_ec_command msg = {
> + .version = 0,
> + .command = EC_CMD_MKBP_STATE,
> + .outdata = NULL,
> + .outsize = 0,
> + .indata = kb_state,
> + .insize = ckdev->cols,
> + };
> +
> + ret = ckdev->ec->cmd_xfer(ckdev->ec, &msg);
Do we need ret?
> + return ret;
> }
>
> static int cros_ec_keyb_work(struct notifier_block *nb,
> diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
> index d242714..6dd91e9 100644
> --- a/drivers/mfd/cros_ec.c
> +++ b/drivers/mfd/cros_ec.c
> @@ -44,34 +44,6 @@ int cros_ec_prepare_tx(struct cros_ec_device *ec_dev,
> }
> EXPORT_SYMBOL(cros_ec_prepare_tx);
>
> -static int cros_ec_command_sendrecv(struct cros_ec_device *ec_dev,
> - uint16_t cmd, void *out_buf, int out_len,
> - void *in_buf, int in_len)
> -{
> - struct cros_ec_command msg;
> -
> - msg.version = cmd >> 8;
> - msg.command = cmd & 0xff;
> - msg.outdata = out_buf;
> - msg.outsize = out_len;
> - msg.indata = in_buf;
> - msg.insize = in_len;
> -
> - return ec_dev->cmd_xfer(ec_dev, &msg);
> -}
> -
> -static int cros_ec_command_recv(struct cros_ec_device *ec_dev,
> - uint16_t cmd, void *buf, int buf_len)
> -{
> - return cros_ec_command_sendrecv(ec_dev, cmd, NULL, 0, buf, buf_len);
> -}
> -
> -static int cros_ec_command_send(struct cros_ec_device *ec_dev,
> - uint16_t cmd, void *buf, int buf_len)
> -{
> - return cros_ec_command_sendrecv(ec_dev, cmd, buf, buf_len, NULL, 0);
> -}
> -
> static irqreturn_t ec_irq_thread(int irq, void *data)
> {
> struct cros_ec_device *ec_dev = data;
> @@ -104,10 +76,6 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
>
> BLOCKING_INIT_NOTIFIER_HEAD(&ec_dev->event_notifier);
>
> - ec_dev->command_send = cros_ec_command_send;
> - ec_dev->command_recv = cros_ec_command_recv;
> - ec_dev->command_sendrecv = cros_ec_command_sendrecv;
> -
> if (ec_dev->din_size) {
> ec_dev->din = devm_kzalloc(dev, ec_dev->din_size, GFP_KERNEL);
> if (!ec_dev->din)
> diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
> index 2b0c598..60c0880 100644
> --- a/include/linux/mfd/cros_ec.h
> +++ b/include/linux/mfd/cros_ec.h
> @@ -63,9 +63,10 @@ struct cros_ec_command {
> * @was_wake_device: true if this device was set to wake the system from
> * sleep at the last suspend
> * @event_notifier: interrupt event notifier for transport devices
> - * @command_send: send a command
> - * @command_recv: receive a response
> - * @command_sendrecv: send a command and receive a response
> + * @cmd_xfer: send command to EC and get response
> + * Returns 0 if the communication succeeded, but that doesn't mean the EC
> + * was happy with the command it got. Caller should check msg.result for
> + * the EC's result code.
> *
> * @priv: Private data
> * @irq: Interrupt to use
> @@ -83,7 +84,6 @@ struct cros_ec_command {
> * @parent: pointer to parent device (e.g. i2c or spi device)
> * @wake_enabled: true if this device can wake the system from sleep
> * @lock: one transaction at a time
> - * @cmd_xfer: low-level channel to the EC
> */
> struct cros_ec_device {
>
> @@ -94,13 +94,8 @@ struct cros_ec_device {
> bool was_wake_device;
> struct class *cros_class;
> struct blocking_notifier_head event_notifier;
> - int (*command_send)(struct cros_ec_device *ec,
> - uint16_t cmd, void *out_buf, int out_len);
> - int (*command_recv)(struct cros_ec_device *ec,
> - uint16_t cmd, void *in_buf, int in_len);
> - int (*command_sendrecv)(struct cros_ec_device *ec,
> - uint16_t cmd, void *out_buf, int out_len,
> - void *in_buf, int in_len);
> + int (*cmd_xfer)(struct cros_ec_device *ec,
> + struct cros_ec_command *msg);
>
> /* These are used to implement the platform-specific interface */
> void *priv;
> @@ -112,8 +107,6 @@ struct cros_ec_device {
> struct device *parent;
> bool wake_enabled;
> struct mutex lock;
> - int (*cmd_xfer)(struct cros_ec_device *ec,
> - struct cros_ec_command *msg);
Seems odd - maybe this line move of cmd_xfer() should be in an earlier patch?
Regards,
Simon
^ permalink raw reply
* Re: [PATCH 08/10] mfd: cros_ec: cleanup: Remove EC wrapper functions
From: Doug Anderson @ 2014-06-18 4:27 UTC (permalink / raw)
To: Simon Glass
Cc: Lee Jones, Andrew Bresticker, Stephen Warren, Olof Johansson,
Sonny Rao, linux-samsung-soc, Javier Martinez Canillas,
Bill Richardson, Wolfram Sang, Mark Brown, Dmitry Torokhov,
Samuel Ortiz, Vincent Palatin, Geert Uytterhoeven,
linux-i2c@vger.kernel.org, lk, linux-input@vger.kernel.org
In-Reply-To: <CAPnjgZ3GhvHog5Pb3_DxYSWF0t4mgR=7HBe2KrfKcsJ=zwmFhQ@mail.gmail.com>
Simon,
On Tue, Jun 17, 2014 at 8:42 PM, Simon Glass <sjg@chromium.org> wrote:
>> diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
>> index 4083796..dc37b6b 100644
>> --- a/drivers/input/keyboard/cros_ec_keyb.c
>> +++ b/drivers/input/keyboard/cros_ec_keyb.c
>> @@ -191,8 +191,18 @@ static void cros_ec_keyb_close(struct input_dev *dev)
>>
>> static int cros_ec_keyb_get_state(struct cros_ec_keyb *ckdev, uint8_t *kb_state)
>> {
>> - return ckdev->ec->command_recv(ckdev->ec, EC_CMD_MKBP_STATE,
>> - kb_state, ckdev->cols);
>> + int ret;
>> + struct cros_ec_command msg = {
>> + .version = 0,
>> + .command = EC_CMD_MKBP_STATE,
>> + .outdata = NULL,
>> + .outsize = 0,
>> + .indata = kb_state,
>> + .insize = ckdev->cols,
>> + };
>> +
>> + ret = ckdev->ec->cmd_xfer(ckdev->ec, &msg);
>
> Do we need ret?
No. If you wish I will spin without it. Let me know.
Note that locally this code includes a comment between the above and the return:
/* FIXME: This assumes msg.result == EC_RES_SUCCESS */
Given that this is not a new problem introduced in this code, that it
still hasn't been fixed locally in the ChromeOS tree, and that
generally FIXMEs don't seem to be left in the code upstream, I left it
out.
>> diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
>> index 2b0c598..60c0880 100644
>> --- a/include/linux/mfd/cros_ec.h
>> +++ b/include/linux/mfd/cros_ec.h
>> @@ -63,9 +63,10 @@ struct cros_ec_command {
>> * @was_wake_device: true if this device was set to wake the system from
>> * sleep at the last suspend
>> * @event_notifier: interrupt event notifier for transport devices
>> - * @command_send: send a command
>> - * @command_recv: receive a response
>> - * @command_sendrecv: send a command and receive a response
>> + * @cmd_xfer: send command to EC and get response
>> + * Returns 0 if the communication succeeded, but that doesn't mean the EC
>> + * was happy with the command it got. Caller should check msg.result for
>> + * the EC's result code.
>> *
>> * @priv: Private data
>> * @irq: Interrupt to use
>> @@ -83,7 +84,6 @@ struct cros_ec_command {
>> * @parent: pointer to parent device (e.g. i2c or spi device)
>> * @wake_enabled: true if this device can wake the system from sleep
>> * @lock: one transaction at a time
>> - * @cmd_xfer: low-level channel to the EC
>> */
>> struct cros_ec_device {
>>
>> @@ -94,13 +94,8 @@ struct cros_ec_device {
>> bool was_wake_device;
>> struct class *cros_class;
>> struct blocking_notifier_head event_notifier;
>> - int (*command_send)(struct cros_ec_device *ec,
>> - uint16_t cmd, void *out_buf, int out_len);
>> - int (*command_recv)(struct cros_ec_device *ec,
>> - uint16_t cmd, void *in_buf, int in_len);
>> - int (*command_sendrecv)(struct cros_ec_device *ec,
>> - uint16_t cmd, void *out_buf, int out_len,
>> - void *in_buf, int in_len);
>> + int (*cmd_xfer)(struct cros_ec_device *ec,
>> + struct cros_ec_command *msg);
>>
>> /* These are used to implement the platform-specific interface */
>> void *priv;
>> @@ -112,8 +107,6 @@ struct cros_ec_device {
>> struct device *parent;
>> bool wake_enabled;
>> struct mutex lock;
>> - int (*cmd_xfer)(struct cros_ec_device *ec,
>> - struct cros_ec_command *msg);
>
> Seems odd - maybe this line move of cmd_xfer() should be in an earlier patch?
It got moved from "private" to public. Bill reorganized all the
public stuff at the top and the private at the bottom.
-Doug
^ permalink raw reply
* Re: [PATCH 08/10] mfd: cros_ec: cleanup: Remove EC wrapper functions
From: Simon Glass @ 2014-06-18 5:05 UTC (permalink / raw)
To: Doug Anderson
Cc: Lee Jones, Andrew Bresticker, Stephen Warren, Olof Johansson,
Sonny Rao, linux-samsung-soc, Javier Martinez Canillas,
Bill Richardson, Wolfram Sang, Mark Brown, Dmitry Torokhov,
Samuel Ortiz, Vincent Palatin, Geert Uytterhoeven,
linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, lk,
linux-input-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CAD=FV=Ue9OBkXtii4aKrCbKPd--7ZRN_G0pw2ZtX89mcz_hfzQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hi Doug,
On 17 June 2014 21:27, Doug Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
> Simon,
>
> On Tue, Jun 17, 2014 at 8:42 PM, Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
>>> diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
>>> index 4083796..dc37b6b 100644
>>> --- a/drivers/input/keyboard/cros_ec_keyb.c
>>> +++ b/drivers/input/keyboard/cros_ec_keyb.c
>>> @@ -191,8 +191,18 @@ static void cros_ec_keyb_close(struct input_dev *dev)
>>>
>>> static int cros_ec_keyb_get_state(struct cros_ec_keyb *ckdev, uint8_t *kb_state)
>>> {
>>> - return ckdev->ec->command_recv(ckdev->ec, EC_CMD_MKBP_STATE,
>>> - kb_state, ckdev->cols);
>>> + int ret;
>>> + struct cros_ec_command msg = {
>>> + .version = 0,
>>> + .command = EC_CMD_MKBP_STATE,
>>> + .outdata = NULL,
>>> + .outsize = 0,
>>> + .indata = kb_state,
>>> + .insize = ckdev->cols,
>>> + };
>>> +
>>> + ret = ckdev->ec->cmd_xfer(ckdev->ec, &msg);
>>
>> Do we need ret?
>
> No. If you wish I will spin without it. Let me know.
>
> Note that locally this code includes a comment between the above and the return:
> /* FIXME: This assumes msg.result == EC_RES_SUCCESS */
It's not important to me, and you've explained the other question.
Reviewed-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
^ permalink raw reply
* Re: [PATCH v2] Input: evdev - add event-mask API
From: Peter Hutterer @ 2014-06-18 4:47 UTC (permalink / raw)
To: David Herrmann
Cc: linux-input, Dmitry Torokhov, Benjamin Tissoires,
Lennart Poettering
In-Reply-To: <1402913358-682-1-git-send-email-dh.herrmann@gmail.com>
On Mon, Jun 16, 2014 at 12:09:18PM +0200, David Herrmann wrote:
> Hardware manufacturers group keys in the weirdest way possible. This may
> cause a power-key to be grouped together with normal keyboard keys and
> thus be reported on the same kernel interface.
>
> However, user-space is often only interested in specific sets of events.
> For instance, daemons dealing with system-reboot (like systemd-logind)
> listen for KEY_POWER, but are not interested in any main keyboard keys.
> Usually, power keys are reported via separate interfaces, however,
> some i8042 boards report it in the AT matrix. To avoid waking up those
> system daemons on each key-press, we had two ideas:
> - split off KEY_POWER into a separate interface unconditionally
> - allow masking a specific set of events on evdev FDs
>
> Splitting of KEY_POWER is a rather weird way to deal with this and may
> break backwards-compatibility. It is also specific to KEY_POWER and might
> be required for other stuff, too. Moreover, we might end up with a huge
> set of input-devices just to have them properly split.
>
> Hence, this patchset implements the second idea: An event-mask to specify
> which events you're interested in. Two ioctls allow setting this mask for
> each event-type. If not set, all events are reported. The type==0 entry is
> used same as in EVIOCGBIT to set the actual EV_* mask of masked events.
> This way, you have a two-level filter.
one comment regarding the wording: I'd prefer to use "filtered" or
"discarded" instead of "masked". Take for example the comment below:
"Each event-code is represented by a single bit in the event-mask. If set,
the event is not-masked."
So if the mask is set, the event is not masked, which is rather confusing.
Rewrite this as "if the mask is set, the event is not filtered" and it's a
lot easier to understand.
[...]
> + * EVIOCGMASK - Retrieve current event-mask
> + *
> + * This retrieves the current event-mask for a specific event-type. The
> + * argument must be of type "struct input_mask" and specifies the event-type to
> + * query, the receive buffer and the size of the receive buffer.
> + *
> + * The event-mask is a per-client mask that specifies which events are forwarded
> + * to the client. Each event-code is represented by a single bit in the
> + * event-mask. If set, the event is not-masked. If unset, the event is masked
> + * and will never be queued on the client's receive buffer.
see comment above, funny thing is that until the "If set" it was quite easy
to understand, the rest made it more confusing :)
> + *
> + * This ioctl provides full forward-compatibility. That means, if a kernel is
> + * queried for an unknown event-type or if the receive buffer is larger than the
> + * number of event-codes known to the kernel, the kernel will return all zeroes
> + * for those codes (which means, those codes are masked). This effectively
> + * means, codes unknown to the kernel are always considered hard-masked.
hard-masked is an odd word, using it here with the masked vs filtered
vs zeroes makes the whole thing quite confusing.
> + * If the receive buffer is too small to contain the whole event-mask, a
> + * truncated mask is copied to user-space.
"At maximum, codes_size bytes are copied" says the same and doesn't leave
one wondering where/when the truncation happens.
> + *
> + * This ioctl may fail with ENODEV in case the file is revoked. EFAULT is
> + * returned if the receive-buffer points to invalid memory. EINVAL is returned
> + * if the kernel does not implement the ioctl.
this is nitpicking, but given that ioctls return -1 and set errno, the
documentation should consistently use "may fail with..." instead
of "returns"
So, here's the comments folded in:
EVIOCGMASK - Retrieve current event-mask
This retrieves the current event-mask for a specific event-type. The
argument must be of type "struct input_mask" and specifies the event-type to
query, the receive buffer and the size of the receive buffer.
The event-mask is a per-client mask that specifies which events are forwarded
to the client. Each event-code is represented by a single bit in the
event-mask. If the bit is set, the event is passed to the client normally.
Otherwise, the event is filtered and and will never be queued on the
client's receive buffer.
The default event-mask for a client has all bits set, i.e. all events are
forwarded to the client. If a kernel is queried for an unknown event-type
or if the receive buffer is larger than the number of event-codes known to
the kernel, the kernel returns all zeroes for those codes.
At maximum, codes_size bytes are copied.
This ioctl may fail with ENODEV in case the file is revoked, EFAULT
if the receive-buffer points to invalid memory, or EINVAL if the kernel
does not implement the ioctl.
Otherwise:
Acked-by: Peter Hutterer <peter.hutterer@who-t.net>
Cheers,
Peter
> + */
> +#define EVIOCGMASK _IOR('E', 0x92, struct input_mask) /* Get event-masks */
> +
> +/**
> + * EVIOCSMASK - Set event-mask
> + *
> + * This is the counterpart to EVIOCGMASK. Instead of receiving the current
> + * event-mask, this changes the client's event-mask for a specific type. See
> + * EVIOCGMASK for a description of event-masks and the argument-type.
> + *
> + * This ioctl provides full forward-compatibility. If the passed event-type is
> + * unknown to the kernel, or if the number of codes is bigger than known to the
> + * kernel, the ioctl is still accepted and applied. However, any unknown codes
> + * are left untouched and stay masked. That means, the kernel hard-masks unknown
> + * codes regardless of what the client requests.
> + * If the new mask doesn't cover all known event-codes, all remaining codes are
> + * automatically cleared and thus masked.
> + *
> + * This ioctl may fail with ENODEV in case the file is revoked. EFAULT is
> + * returned if the receive-buffer points to invalid memory. EINVAL is returned
> + * if the kernel does not implement the ioctl.
> + */
> +#define EVIOCSMASK _IOW('E', 0x93, struct input_mask) /* Set event-masks */
> +
> #define EVIOCSCLOCKID _IOW('E', 0xa0, int) /* Set clockid to be used for timestamps */
>
> /*
> --
> 2.0.0
>
^ permalink raw reply
* Re: [PATCH v2] Input: evdev - add event-mask API
From: David Herrmann @ 2014-06-18 5:39 UTC (permalink / raw)
To: Peter Hutterer
Cc: open list:HID CORE LAYER, Dmitry Torokhov, Benjamin Tissoires,
Lennart Poettering
In-Reply-To: <20140618044757.GB23028@jelly.redhat.com>
Hi
On Wed, Jun 18, 2014 at 6:47 AM, Peter Hutterer
<peter.hutterer@who-t.net> wrote:
> On Mon, Jun 16, 2014 at 12:09:18PM +0200, David Herrmann wrote:
>> Hardware manufacturers group keys in the weirdest way possible. This may
>> cause a power-key to be grouped together with normal keyboard keys and
>> thus be reported on the same kernel interface.
>>
>> However, user-space is often only interested in specific sets of events.
>> For instance, daemons dealing with system-reboot (like systemd-logind)
>> listen for KEY_POWER, but are not interested in any main keyboard keys.
>> Usually, power keys are reported via separate interfaces, however,
>> some i8042 boards report it in the AT matrix. To avoid waking up those
>> system daemons on each key-press, we had two ideas:
>> - split off KEY_POWER into a separate interface unconditionally
>> - allow masking a specific set of events on evdev FDs
>>
>> Splitting of KEY_POWER is a rather weird way to deal with this and may
>> break backwards-compatibility. It is also specific to KEY_POWER and might
>> be required for other stuff, too. Moreover, we might end up with a huge
>> set of input-devices just to have them properly split.
>>
>> Hence, this patchset implements the second idea: An event-mask to specify
>> which events you're interested in. Two ioctls allow setting this mask for
>> each event-type. If not set, all events are reported. The type==0 entry is
>> used same as in EVIOCGBIT to set the actual EV_* mask of masked events.
>> This way, you have a two-level filter.
>
> one comment regarding the wording: I'd prefer to use "filtered" or
> "discarded" instead of "masked". Take for example the comment below:
>
> "Each event-code is represented by a single bit in the event-mask. If set,
> the event is not-masked."
>
> So if the mask is set, the event is not masked, which is rather confusing.
> Rewrite this as "if the mask is set, the event is not filtered" and it's a
> lot easier to understand.
>
> [...]
>
>> + * EVIOCGMASK - Retrieve current event-mask
>> + *
>> + * This retrieves the current event-mask for a specific event-type. The
>> + * argument must be of type "struct input_mask" and specifies the event-type to
>> + * query, the receive buffer and the size of the receive buffer.
>> + *
>> + * The event-mask is a per-client mask that specifies which events are forwarded
>> + * to the client. Each event-code is represented by a single bit in the
>> + * event-mask. If set, the event is not-masked. If unset, the event is masked
>> + * and will never be queued on the client's receive buffer.
>
> see comment above, funny thing is that until the "If set" it was quite easy
> to understand, the rest made it more confusing :)
>
>> + *
>> + * This ioctl provides full forward-compatibility. That means, if a kernel is
>> + * queried for an unknown event-type or if the receive buffer is larger than the
>> + * number of event-codes known to the kernel, the kernel will return all zeroes
>> + * for those codes (which means, those codes are masked). This effectively
>> + * means, codes unknown to the kernel are always considered hard-masked.
>
> hard-masked is an odd word, using it here with the masked vs filtered
> vs zeroes makes the whole thing quite confusing.
>
>> + * If the receive buffer is too small to contain the whole event-mask, a
>> + * truncated mask is copied to user-space.
>
> "At maximum, codes_size bytes are copied" says the same and doesn't leave
> one wondering where/when the truncation happens.
>
>> + *
>> + * This ioctl may fail with ENODEV in case the file is revoked. EFAULT is
>> + * returned if the receive-buffer points to invalid memory. EINVAL is returned
>> + * if the kernel does not implement the ioctl.
>
> this is nitpicking, but given that ioctls return -1 and set errno, the
> documentation should consistently use "may fail with..." instead
> of "returns"
I disagree on that one. I document kernel-API and we return negative
error-codes on failure. It's POSIX/glibc that turns this into -1 and
sets errno (and only god knows why that brainfu**ed idea got
standardized..).
However, turning this into "may fail with..." suits both of us, so I'm
fine with it.
Thanks for the re-wording. I amended the changes and also turned
__evdev_is_masked() into __evdev_is_filtered() to make it more
readable.
I will send v3 shortly, thanks!
David
^ permalink raw reply
* [PATCH v3] Input: evdev - add event-mask API
From: David Herrmann @ 2014-06-18 5:42 UTC (permalink / raw)
To: linux-input; +Cc: Dmitry Torokhov, Peter Hutterer, David Herrmann
In-Reply-To: <1402913358-682-1-git-send-email-dh.herrmann@gmail.com>
Hardware manufacturers group keys in the weirdest way possible. This may
cause a power-key to be grouped together with normal keyboard keys and
thus be reported on the same kernel interface.
However, user-space is often only interested in specific sets of events.
For instance, daemons dealing with system-reboot (like systemd-logind)
listen for KEY_POWER, but are not interested in any main keyboard keys.
Usually, power keys are reported via separate interfaces, however,
some i8042 boards report it in the AT matrix. To avoid waking up those
system daemons on each key-press, we had two ideas:
- split off KEY_POWER into a separate interface unconditionally
- allow filtering a specific set of events on evdev FDs
Splitting of KEY_POWER is a rather weird way to deal with this and may
break backwards-compatibility. It is also specific to KEY_POWER and might
be required for other stuff, too. Moreover, we might end up with a huge
set of input-devices just to have them properly split.
Hence, this patchset implements the second idea: An event-mask to specify
which events you're interested in. Two ioctls allow setting this mask for
each event-type. If not set, all events are reported. The type==0 entry is
used same as in EVIOCGBIT to set the actual EV_* mask of filtered events.
This way, you have a two-level filter.
We are heavily forward-compatible to new event-types and event-codes. So
new user-space will be able to run on an old kernel which doesn't know the
given event-codes or event-types.
Acked-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
v3:
- rewording (Peter)
drivers/input/evdev.c | 156 ++++++++++++++++++++++++++++++++++++++++++++-
include/uapi/linux/input.h | 56 ++++++++++++++++
2 files changed, 210 insertions(+), 2 deletions(-)
diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
index fd325ec..6386882 100644
--- a/drivers/input/evdev.c
+++ b/drivers/input/evdev.c
@@ -51,10 +51,130 @@ struct evdev_client {
struct list_head node;
int clkid;
bool revoked;
+ unsigned long *evmasks[EV_CNT];
unsigned int bufsize;
struct input_event buffer[];
};
+static size_t evdev_get_mask_cnt(unsigned int type)
+{
+ static size_t counts[EV_CNT] = {
+ /* EV_SYN==0 is EV_CNT, _not_ SYN_CNT, see EVIOCGBIT */
+ [EV_SYN] = EV_CNT,
+ [EV_KEY] = KEY_CNT,
+ [EV_REL] = REL_CNT,
+ [EV_ABS] = ABS_CNT,
+ [EV_MSC] = MSC_CNT,
+ [EV_SW] = SW_CNT,
+ [EV_LED] = LED_CNT,
+ [EV_SND] = SND_CNT,
+ [EV_FF] = FF_CNT,
+ };
+
+ return (type < EV_CNT) ? counts[type] : 0;
+}
+
+/* must be called with evdev-mutex held */
+static int evdev_set_mask(struct evdev_client *client,
+ unsigned int type,
+ const void __user *codes,
+ u32 codes_size)
+{
+ unsigned long flags, *mask, *oldmask;
+ size_t cnt, size;
+
+ /* unknown masks are simply ignored for forward-compat */
+ cnt = evdev_get_mask_cnt(type);
+ if (!cnt)
+ return 0;
+
+ /* we allow 'codes_size > size' for forward-compat */
+ size = sizeof(unsigned long) * BITS_TO_LONGS(cnt);
+
+ mask = kzalloc(size, GFP_KERNEL);
+ if (!mask)
+ return -ENOMEM;
+
+ if (copy_from_user(mask, codes, min_t(size_t, codes_size, size))) {
+ kfree(mask);
+ return -EFAULT;
+ }
+
+ spin_lock_irqsave(&client->buffer_lock, flags);
+ oldmask = client->evmasks[type];
+ client->evmasks[type] = mask;
+ spin_unlock_irqrestore(&client->buffer_lock, flags);
+
+ kfree(oldmask);
+
+ return 0;
+}
+
+/* must be called with evdev-mutex held */
+static int evdev_get_mask(struct evdev_client *client,
+ unsigned int type,
+ void __user *codes,
+ u32 codes_size)
+{
+ unsigned long *mask;
+ size_t cnt, size, min, i;
+ u8 __user *out;
+
+ /* we allow unknown types and 'codes_size > size' for forward-compat */
+ cnt = evdev_get_mask_cnt(type);
+ size = sizeof(unsigned long) * BITS_TO_LONGS(cnt);
+ min = min_t(size_t, codes_size, size);
+
+ if (cnt > 0) {
+ mask = client->evmasks[type];
+ if (mask) {
+ if (copy_to_user(codes, mask, min))
+ return -EFAULT;
+ } else {
+ /* fake mask with all bits set */
+ out = (u8 __user*)codes;
+ for (i = 0; i < min; ++i) {
+ if (put_user((u8)0xff, out + i))
+ return -EFAULT;
+ }
+ }
+ }
+
+ codes = (u8*)codes + min;
+ codes_size -= min;
+
+ if (codes_size > 0 && clear_user(codes, codes_size))
+ return -EFAULT;
+
+ return 0;
+}
+
+/* requires the buffer lock to be held */
+static bool __evdev_is_filtered(struct evdev_client *client,
+ unsigned int type,
+ unsigned int code)
+{
+ unsigned long *mask;
+ size_t cnt;
+
+ /* EV_SYN and unknown codes are never filtered */
+ if (type == EV_SYN || type >= EV_CNT)
+ return false;
+
+ /* first test whether the type is filtered */
+ mask = client->evmasks[0];
+ if (mask && !test_bit(type, mask))
+ return true;
+
+ /* unknown values are never filtered */
+ cnt = evdev_get_mask_cnt(type);
+ if (!cnt || code >= cnt)
+ return false;
+
+ mask = client->evmasks[type];
+ return mask && !test_bit(code, mask);
+}
+
/* flush queued events of type @type, caller must hold client->buffer_lock */
static void __evdev_flush_queue(struct evdev_client *client, unsigned int type)
{
@@ -177,12 +297,21 @@ static void evdev_pass_values(struct evdev_client *client,
spin_lock(&client->buffer_lock);
for (v = vals; v != vals + count; v++) {
+ if (__evdev_is_filtered(client, v->type, v->code))
+ continue;
+
+ if (v->type == EV_SYN && v->code == SYN_REPORT) {
+ /* drop empty SYN_REPORT */
+ if (client->packet_head == client->head)
+ continue;
+
+ wakeup = true;
+ }
+
event.type = v->type;
event.code = v->code;
event.value = v->value;
__pass_event(client, &event);
- if (v->type == EV_SYN && v->code == SYN_REPORT)
- wakeup = true;
}
spin_unlock(&client->buffer_lock);
@@ -365,6 +494,7 @@ static int evdev_release(struct inode *inode, struct file *file)
{
struct evdev_client *client = file->private_data;
struct evdev *evdev = client->evdev;
+ unsigned int i;
mutex_lock(&evdev->mutex);
evdev_ungrab(evdev, client);
@@ -372,6 +502,9 @@ static int evdev_release(struct inode *inode, struct file *file)
evdev_detach_client(evdev, client);
+ for (i = 0; i < EV_CNT; ++i)
+ kfree(client->evmasks[i]);
+
if (is_vmalloc_addr(client))
vfree(client);
else
@@ -811,6 +944,7 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,
struct evdev *evdev = client->evdev;
struct input_dev *dev = evdev->handle.dev;
struct input_absinfo abs;
+ struct input_mask mask;
struct ff_effect effect;
int __user *ip = (int __user *)p;
unsigned int i, t, u, v;
@@ -872,6 +1006,24 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,
else
return evdev_revoke(evdev, client, file);
+ case EVIOCGMASK:
+ if (copy_from_user(&mask, p, sizeof(mask)))
+ return -EFAULT;
+
+ return evdev_get_mask(client,
+ mask.type,
+ (void*)(long)mask.codes_ptr,
+ mask.codes_size);
+
+ case EVIOCSMASK:
+ if (copy_from_user(&mask, p, sizeof(mask)))
+ return -EFAULT;
+
+ return evdev_set_mask(client,
+ mask.type,
+ (const void*)(long)mask.codes_ptr,
+ mask.codes_size);
+
case EVIOCSCLOCKID:
if (copy_from_user(&i, p, sizeof(unsigned int)))
return -EFAULT;
diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
index 19df18c..f6ace0e 100644
--- a/include/uapi/linux/input.h
+++ b/include/uapi/linux/input.h
@@ -97,6 +97,12 @@ struct input_keymap_entry {
__u8 scancode[32];
};
+struct input_mask {
+ __u32 type;
+ __u32 codes_size;
+ __u64 codes_ptr;
+};
+
#define EVIOCGVERSION _IOR('E', 0x01, int) /* get driver version */
#define EVIOCGID _IOR('E', 0x02, struct input_id) /* get device ID */
#define EVIOCGREP _IOR('E', 0x03, unsigned int[2]) /* get repeat settings */
@@ -154,6 +160,56 @@ struct input_keymap_entry {
#define EVIOCGRAB _IOW('E', 0x90, int) /* Grab/Release device */
#define EVIOCREVOKE _IOW('E', 0x91, int) /* Revoke device access */
+/**
+ * EVIOCGMASK - Retrieve current event-mask
+ *
+ * This retrieves the current event-mask for a specific event-type. The
+ * argument must be of type "struct input_mask" and specifies the event-type to
+ * query, the receive buffer and the size of the receive buffer.
+ *
+ * The event-mask is a per-client mask that specifies which events are forwarded
+ * to the client. Each event-code is represented by a single bit in the
+ * event-mask. If the bit is set, the event is passed to the client normally.
+ * Otherwise, the event is filtered and and will never be queued on the
+ * client's receive buffer.
+ * Event-masks do not affect global state of an input-device. They only affect
+ * the open-file they're applied on. Each open-file (i.e, file-description) can
+ * have a different event-mask.
+ *
+ * The default event-mask for a client has all bits set, i.e. all events are
+ * forwarded to the client. If a kernel is queried for an unknown event-type
+ * or if the receive buffer is larger than the number of event-codes known to
+ * the kernel, the kernel returns all zeroes for those codes.
+ *
+ * At maximum, codes_size bytes are copied.
+ *
+ * This ioctl may fail with ENODEV in case the file is revoked, EFAULT
+ * if the receive-buffer points to invalid memory, or EINVAL if the kernel
+ * does not implement the ioctl.
+ */
+#define EVIOCGMASK _IOR('E', 0x92, struct input_mask) /* Get event-masks */
+
+/**
+ * EVIOCSMASK - Set event-mask
+ *
+ * This is the counterpart to EVIOCGMASK. Instead of receiving the current
+ * event-mask, this changes the client's event-mask for a specific type. See
+ * EVIOCGMASK for a description of event-masks and the argument-type.
+ *
+ * This ioctl provides full forward-compatibility. If the passed event-type is
+ * unknown to the kernel, or if the number of codes is bigger than known to the
+ * kernel, the ioctl is still accepted and applied. However, any unknown codes
+ * are left untouched and stay cleared. That means, the kernel always filters
+ * unknown codes regardless of what the client requests.
+ * If the new mask doesn't cover all known event-codes, all remaining codes are
+ * automatically cleared and thus filtered.
+ *
+ * This ioctl may fail with ENODEV in case the file is revoked. EFAULT is
+ * returned if the receive-buffer points to invalid memory. EINVAL is returned
+ * if the kernel does not implement the ioctl.
+ */
+#define EVIOCSMASK _IOW('E', 0x93, struct input_mask) /* Set event-masks */
+
#define EVIOCSCLOCKID _IOW('E', 0xa0, int) /* Set clockid to be used for timestamps */
/*
--
2.0.0
^ permalink raw reply related
* Re: [PATCH v8] leds: USB: HID: Add support for MSI GT683R led panels
From: Johan Hovold @ 2014-06-18 7:39 UTC (permalink / raw)
To: Janne Kanniainen
Cc: jkosina-AlSwsSmVLrQ, johan-DgEjT+Ai2ygdnm+yROfE0A,
cooloney-Re5JQEeQqe8AvxtiuMwx3w,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-leds-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1403023304-7953-1-git-send-email-janne.kanniainen-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On Tue, Jun 17, 2014 at 07:41:44PM +0300, Janne Kanniainen wrote:
> This driver adds support for USB controlled led panels that exists in
> MSI GT683R laptop
>
> Changes in v2:
> - sorted headers to alphabetic order
> - using devm_kzalloc
> - using BIT(n)
> - using usb_control_msg instead of usb_submit_urb
> - removing unneeded code
> Changes in v3:
> - implemented as HID device
> - some cleanups and bug fixes
>
> Changes in v4:
> - more cleanups
> - support for selecting leds
> - suppport for selecting status
>
> Changes in v5:
> - mode attribute documented under Documentation/ABI
> - made array for led_classdev
> - led devices uses now recommended naming scheme
>
> Changes in v6:
> - flush_work added
> - using hid device name instead of hard coded gt683r
> - allocating name buffers with devm_kzalloc
>
> Changes in v7:
> - buf with for fixed
>
> Changes in v8:
> - some cleanups and bugs fixed
Great job, Janne. I have a few really minor style issues I just noted
and that I point out below, but that's it.
> Signed-off-by: Janne Kanniainen <janne.kanniainen-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> .../ABI/testing/sysfs-class-hid-driver-gt683r | 14 +
> drivers/hid/Kconfig | 14 +
> drivers/hid/Makefile | 1 +
> drivers/hid/hid-core.c | 1 +
> drivers/hid/hid-gt683r.c | 308 +++++++++++++++++++++
> drivers/hid/hid-ids.h | 2 +-
> drivers/hid/usbhid/hid-quirks.c | 2 +-
> 7 files changed, 340 insertions(+), 2 deletions(-)
> create mode 100644 Documentation/ABI/testing/sysfs-class-hid-driver-gt683r
> create mode 100644 drivers/hid/hid-gt683r.c
>
> diff --git a/Documentation/ABI/testing/sysfs-class-hid-driver-gt683r b/Documentation/ABI/testing/sysfs-class-hid-driver-gt683r
> new file mode 100644
> index 0000000..317e9d5
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-class-hid-driver-gt683r
> @@ -0,0 +1,14 @@
> +What: /sys/class/hidraw/<hidraw>/device/leds_mode
> +Date: Jun 2014
> +KernelVersion: 3.17
> +Contact: Janne Kanniainen <janne.kanniainen-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> +Description:
> + Set the mode of LEDs
> +
> + 0 - normal
> + 1 - audio
> + 2 - breathing
> +
> + Normal: LEDs are fully on when enabled
> + Audio: LEDs brightness depends on sound level
> + Breathing: LEDs brightness varies at human breathing rate
> \ No newline at end of file
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 7af9d0b..e2f4590 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -261,6 +261,20 @@ config HOLTEK_FF
> Say Y here if you have a Holtek On Line Grip based game controller
> and want to have force feedback support for it.
>
> +config HID_GT683R
> + tristate "MSI GT68xR LED support"
> + depends on LEDS_CLASS && USB_HID
> + ---help---
> + Say Y here if you want to enable support for the three MSI GT68xR LEDs
> +
> + This driver support following modes:
> + - Normal: LEDs are fully on when enabled
> + - Audio: LEDs brightness depends on sound level
> + - Breathing: LEDs brightness varies at human breathing rate
> +
> + Currently the following devices are know to be supported:
> + - MSI GT683R
> +
> config HID_HUION
> tristate "Huion tablets"
> depends on USB_HID
> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> index fc712dd..7129311 100644
> --- a/drivers/hid/Makefile
> +++ b/drivers/hid/Makefile
> @@ -48,6 +48,7 @@ obj-$(CONFIG_HID_EMS_FF) += hid-emsff.o
> obj-$(CONFIG_HID_ELECOM) += hid-elecom.o
> obj-$(CONFIG_HID_ELO) += hid-elo.o
> obj-$(CONFIG_HID_EZKEY) += hid-ezkey.o
> +obj-$(CONFIG_HID_GT683R) += hid-gt683r.o
> obj-$(CONFIG_HID_GYRATION) += hid-gyration.o
> obj-$(CONFIG_HID_HOLTEK) += hid-holtek-kbd.o
> obj-$(CONFIG_HID_HOLTEK) += hid-holtek-mouse.o
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index da52279..ec88fdb 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -1827,6 +1827,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
> { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_WIRELESS_OPTICAL_DESKTOP_3_0) },
> { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_OFFICE_KB) },
> { HID_USB_DEVICE(USB_VENDOR_ID_MONTEREY, USB_DEVICE_ID_GENIUS_KB29E) },
> + { HID_USB_DEVICE(USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GT683R_LED_PANEL) },
> { HID_USB_DEVICE(USB_VENDOR_ID_NTRIG, USB_DEVICE_ID_NTRIG_TOUCH_SCREEN) },
> { HID_USB_DEVICE(USB_VENDOR_ID_NTRIG, USB_DEVICE_ID_NTRIG_TOUCH_SCREEN_1) },
> { HID_USB_DEVICE(USB_VENDOR_ID_NTRIG, USB_DEVICE_ID_NTRIG_TOUCH_SCREEN_2) },
> diff --git a/drivers/hid/hid-gt683r.c b/drivers/hid/hid-gt683r.c
> new file mode 100644
> index 0000000..19959ac
> --- /dev/null
> +++ b/drivers/hid/hid-gt683r.c
> @@ -0,0 +1,308 @@
> +/*
> + * MSI GT683R led driver
> + *
> + * Copyright (c) 2014 Janne Kanniainen <janne.kanniainen-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> + *
> + * 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; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include <linux/device.h>
> +#include <linux/hid.h>
> +#include <linux/kernel.h>
> +#include <linux/leds.h>
> +#include <linux/module.h>
> +
> +#include "hid-ids.h"
> +
> +#define GT683R_BUFFER_SIZE 8
> +
> +/*
> + * GT683R_LED_OFF: all LEDs are off
> + * GT683R_LED_AUDIO: LEDs brightness depends on sound level
> + * GT683R_LED_BREATHING: LEDs brightness varies at human breathing rate
> + * GT683R_LED_NORMAL: LEDs are fully on when enabled
> + */
> +enum gt683r_led_mode {
> + GT683R_LED_OFF = 0,
> + GT683R_LED_AUDIO = 2,
> + GT683R_LED_BREATHING = 3,
> + GT683R_LED_NORMAL = 5
> +};
> +
> +enum gt683r_panels {
> + GT683R_LED_BACK = 0,
> + GT683R_LED_SIDE = 1,
> + GT683R_LED_FRONT = 2,
> + GT683R_LED_COUNT,
> +};
> +
> +static const char * const gt683r_panel_names[] = {
> + "back",
> + "side",
> + "front",
> +};
> +
> +struct gt683r_led {
> + struct hid_device *hdev;
> + struct led_classdev led_devs[GT683R_LED_COUNT];
> + struct mutex lock;
> + struct work_struct work;
> + enum led_brightness brightnesses[GT683R_LED_COUNT];
> + enum gt683r_led_mode mode;
> +};
> +
> +static const struct hid_device_id gt683r_led_id[] = {
> + { HID_USB_DEVICE(USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GT683R_LED_PANEL) },
> + { }
> +};
> +
> +static void gt683r_brightness_set(struct led_classdev *led_cdev,
> + enum led_brightness brightness)
> +{
> + int i;
> + struct device *dev = led_cdev->dev->parent;
> + struct hid_device *hdev = container_of(dev, struct hid_device, dev);
> + struct gt683r_led *led = hid_get_drvdata(hdev);
> +
> + for (i = 0; i < GT683R_LED_COUNT; i++) {
> + if (&led->led_devs[i] == led_cdev)
I'd prefer to have led_cdev on the left here comparing against the
"constant" structs on the right.
> + break;
> + }
> +
> + if (i < GT683R_LED_COUNT) {
> + led->brightnesses[i] = brightness;
> + schedule_work(&led->work);
> + }
> +}
> +
> +static ssize_t leds_mode_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + u8 sysfs_mode;
> + struct hid_device *hdev = container_of(dev, struct hid_device, dev);
> + struct gt683r_led *led = hid_get_drvdata(hdev);
> +
> + if (led->mode == GT683R_LED_NORMAL)
> + sysfs_mode = 0;
> + else if (led->mode == GT683R_LED_AUDIO)
> + sysfs_mode = 1;
> + else
> + sysfs_mode = 2;
> +
> + return scnprintf(buf, PAGE_SIZE, "%u\n", sysfs_mode);
> +}
> +
> +static ssize_t leds_mode_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + u8 sysfs_mode;
> + struct hid_device *hdev = container_of(dev, struct hid_device, dev);
> + struct gt683r_led *led = hid_get_drvdata(hdev);
> +
> +
> + if (kstrtou8(buf, 10, &sysfs_mode) || sysfs_mode > 2)
> + return -EINVAL;
> +
> + mutex_lock(&led->lock);
> +
> + if (sysfs_mode == 0)
> + led->mode = GT683R_LED_NORMAL;
> + else if (sysfs_mode == 1)
> + led->mode = GT683R_LED_AUDIO;
> + else
> + led->mode = GT683R_LED_BREATHING;
> +
> + mutex_unlock(&led->lock);
> + schedule_work(&led->work);
> +
> + return count;
> +}
> +
> +static int gt683r_led_snd_msg(struct gt683r_led *led, u8 *msg)
> +{
> + int ret;
> +
> + ret = hid_hw_raw_request(led->hdev, msg[0], msg, GT683R_BUFFER_SIZE,
> + HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
> + if (ret != GT683R_BUFFER_SIZE) {
> + hid_err(led->hdev,
> + "failed to send set report request: %i\n", ret);
> + if (ret < 0)
> + return ret;
> + return -EIO;
> + }
> +
> + return 0;
> +}
> +
> +static int gt683r_leds_set(struct gt683r_led *led, u8 leds)
> +{
> + int ret;
> + u8 *buffer;
> +
> + buffer = kzalloc(GT683R_BUFFER_SIZE, GFP_KERNEL);
> + if (!buffer)
> + return -ENOMEM;
> +
> + buffer[0] = 0x01;
> + buffer[1] = 0x02;
> + buffer[2] = 0x30;
> + buffer[3] = leds;
> + ret = gt683r_led_snd_msg(led, buffer);
> +
> + kfree(buffer);
> + return ret;
> +}
> +
> +static int gt683r_mode_set(struct gt683r_led *led, u8 mode)
> +{
> + int ret;
> + u8 *buffer;
> +
> + buffer = kzalloc(GT683R_BUFFER_SIZE, GFP_KERNEL);
> + if (!buffer)
> + return -ENOMEM;
> +
> + buffer[0] = 0x01;
> + buffer[1] = 0x02;
> + buffer[2] = 0x20;
> + buffer[3] = mode;
> + buffer[4] = 0x01;
> + ret = gt683r_led_snd_msg(led, buffer);
> +
> + kfree(buffer);
> + return ret;
> +}
> +
> +static void gt683r_led_work(struct work_struct *work)
> +{
> + int i;
> + u8 leds = 0;
> + u8 mode;
> + struct gt683r_led *led = container_of(work, struct gt683r_led, work);
> +
> + mutex_lock(&led->lock);
> +
> + for (i = 0; i < GT683R_LED_COUNT; i++) {
> + if (led->brightnesses[i])
> + leds |= BIT(i);
> + }
> +
> + if (gt683r_leds_set(led, leds))
> + goto fail;
> +
> + if (leds)
> + mode = led->mode;
> + else
> + mode = GT683R_LED_OFF;
> +
> + gt683r_mode_set(led, mode);
> +fail:
> + mutex_unlock(&led->lock);
> +}
> +
> +static DEVICE_ATTR_RW(leds_mode);
> +
> +static int gt683r_led_probe(struct hid_device *hdev,
> + const struct hid_device_id *id)
> +{
> + int i, ret, name_sz;
One declaration per line.
> + char *name;
> + struct gt683r_led *led;
> +
> + led = devm_kzalloc(&hdev->dev, sizeof(struct gt683r_led), GFP_KERNEL);
Preferred form for passing the size to kmalloc and friends is
sizeof(*led) (in case the type of led ever changes). It's also shorter
and often more readable.
> + if (!led)
> + return -ENOMEM;
> +
> + led->mode = GT683R_LED_NORMAL;
> + led->hdev = hdev;
> + hid_set_drvdata(hdev, led);
> +
> + ret = hid_parse(hdev);
> + if (ret) {
> + hid_err(hdev, "hid parsing failed\n");
> + return ret;
> + }
> +
> + ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
> + if (ret) {
> + hid_err(hdev, "hw start failed\n");
> + return ret;
> + }
> +
> + for (i = 0; i < GT683R_LED_COUNT; i++) {
> + name_sz = strlen(dev_name(&hdev->dev)) +
> + strlen(gt683r_panel_names[i]) + 3;
Try to indent continuation lines at least two tabs further than the
preceding line.
> +
> + name = devm_kzalloc(&hdev->dev, sizeof(char) * name_sz,
> + GFP_KERNEL);
No need to do sizeof(char) which is always 1 (so no need to break this
line either).
> + if (!name) {
> + ret = -ENOMEM;
> + goto fail;
> + }
> +
> + snprintf(name, name_sz, "%s::%s",
> + dev_name(&hdev->dev), gt683r_panel_names[i]);
On more tab here.
> + led->led_devs[i].name = name;
> + led->led_devs[i].max_brightness = 1;
> + led->led_devs[i].brightness_set = gt683r_brightness_set;
> + ret = led_classdev_register(&hdev->dev, &led->led_devs[i]);
> + if (ret) {
> + hid_err(hdev, "could not register led device\n");
> + goto fail;
> + }
> + }
> +
> + ret = device_create_file(&led->hdev->dev, &dev_attr_leds_mode);
> + if (ret) {
> + hid_err(hdev, "could not make mode attribute file\n");
> + goto fail;
> + }
> +
> + mutex_init(&led->lock);
> + INIT_WORK(&led->work, gt683r_led_work);
> +
> + return 0;
> +
> +fail:
> + for (i = i - 1; i >= 0; i--)
> + led_classdev_unregister(&led->led_devs[i]);
> + hid_hw_stop(hdev);
> + return ret;
> +}
> +
> +static void gt683r_led_remove(struct hid_device *hdev)
> +{
> + int i;
> + struct gt683r_led *led = hid_get_drvdata(hdev);
> +
> + device_remove_file(&hdev->dev, &dev_attr_leds_mode);
> + for (i = 0; i < GT683R_LED_COUNT; i++)
> + led_classdev_unregister(&led->led_devs[i]);
> + flush_work(&led->work);
> + hid_hw_stop(hdev);
> +}
> +
> +static struct hid_driver gt683r_led_driver = {
> + .probe = gt683r_led_probe,
> + .remove = gt683r_led_remove,
> + .name = "gt683r_led",
> + .id_table = gt683r_led_id,
> +};
> +
> +module_hid_driver(gt683r_led_driver);
> +
> +MODULE_AUTHOR("Janne Kanniainen");
> +MODULE_DESCRIPTION("MSI GT683R led driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 34bb220..3692d37 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -641,7 +641,7 @@
> #define USB_DEVICE_ID_GENIUS_KB29E 0x3004
>
> #define USB_VENDOR_ID_MSI 0x1770
> -#define USB_DEVICE_ID_MSI_GX680R_LED_PANEL 0xff00
> +#define USB_DEVICE_ID_MSI_GT683R_LED_PANEL 0xff00
>
> #define USB_VENDOR_ID_NATIONAL_SEMICONDUCTOR 0x0400
> #define USB_DEVICE_ID_N_S_HARMONY 0xc359
> diff --git a/drivers/hid/usbhid/hid-quirks.c b/drivers/hid/usbhid/hid-quirks.c
> index 8e4ddb3..c640e1d 100644
> --- a/drivers/hid/usbhid/hid-quirks.c
> +++ b/drivers/hid/usbhid/hid-quirks.c
> @@ -73,7 +73,7 @@ static const struct hid_blacklist {
> { USB_VENDOR_ID_FORMOSA, USB_DEVICE_ID_FORMOSA_IR_RECEIVER, HID_QUIRK_NO_INIT_REPORTS },
> { USB_VENDOR_ID_FREESCALE, USB_DEVICE_ID_FREESCALE_MX28, HID_QUIRK_NOGET },
> { USB_VENDOR_ID_MGE, USB_DEVICE_ID_MGE_UPS, HID_QUIRK_NOGET },
> - { USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GX680R_LED_PANEL, HID_QUIRK_NO_INIT_REPORTS },
> + { USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GT683R_LED_PANEL, HID_QUIRK_NO_INIT_REPORTS },
> { USB_VENDOR_ID_NEXIO, USB_DEVICE_ID_NEXIO_MULTITOUCH_PTI0750, HID_QUIRK_NO_INIT_REPORTS },
> { USB_VENDOR_ID_NOVATEK, USB_DEVICE_ID_NOVATEK_MOUSE, HID_QUIRK_NO_INIT_REPORTS },
> { USB_VENDOR_ID_PIXART, USB_DEVICE_ID_PIXART_OPTICAL_TOUCH_SCREEN, HID_QUIRK_NO_INIT_REPORTS },
Thanks,
Johan
--
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
* Re: [PATCH 08/10] mfd: cros_ec: cleanup: Remove EC wrapper functions
From: Lee Jones @ 2014-06-18 7:40 UTC (permalink / raw)
To: Doug Anderson
Cc: Simon Glass, Andrew Bresticker, Stephen Warren, Olof Johansson,
Sonny Rao, linux-samsung-soc, Javier Martinez Canillas,
Bill Richardson, Wolfram Sang, Mark Brown, Dmitry Torokhov,
Samuel Ortiz, Vincent Palatin, Geert Uytterhoeven,
linux-i2c@vger.kernel.org, lk, linux-input@vger.kernel.org
In-Reply-To: <CAD=FV=Ue9OBkXtii4aKrCbKPd--7ZRN_G0pw2ZtX89mcz_hfzQ@mail.gmail.com>
> >> diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
> >> index 4083796..dc37b6b 100644
> >> --- a/drivers/input/keyboard/cros_ec_keyb.c
> >> +++ b/drivers/input/keyboard/cros_ec_keyb.c
> >> @@ -191,8 +191,18 @@ static void cros_ec_keyb_close(struct input_dev *dev)
> >>
> >> static int cros_ec_keyb_get_state(struct cros_ec_keyb *ckdev, uint8_t *kb_state)
> >> {
> >> - return ckdev->ec->command_recv(ckdev->ec, EC_CMD_MKBP_STATE,
> >> - kb_state, ckdev->cols);
> >> + int ret;
> >> + struct cros_ec_command msg = {
> >> + .version = 0,
> >> + .command = EC_CMD_MKBP_STATE,
> >> + .outdata = NULL,
> >> + .outsize = 0,
> >> + .indata = kb_state,
> >> + .insize = ckdev->cols,
> >> + };
> >> +
> >> + ret = ckdev->ec->cmd_xfer(ckdev->ec, &msg);
> >
> > Do we need ret?
>
> No. If you wish I will spin without it. Let me know.
>
> Note that locally this code includes a comment between the above and the return:
> /* FIXME: This assumes msg.result == EC_RES_SUCCESS */
>
> Given that this is not a new problem introduced in this code, that it
> still hasn't been fixed locally in the ChromeOS tree, and that
> generally FIXMEs don't seem to be left in the code upstream, I left it
> out.
As you're re-spinning anyway, please fix this and add my:
Acked-by: Lee Jones <lee.jones@linaro.org>
> >> diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
> >> index 2b0c598..60c0880 100644
> >> --- a/include/linux/mfd/cros_ec.h
> >> +++ b/include/linux/mfd/cros_ec.h
> >> @@ -63,9 +63,10 @@ struct cros_ec_command {
> >> * @was_wake_device: true if this device was set to wake the system from
> >> * sleep at the last suspend
> >> * @event_notifier: interrupt event notifier for transport devices
> >> - * @command_send: send a command
> >> - * @command_recv: receive a response
> >> - * @command_sendrecv: send a command and receive a response
> >> + * @cmd_xfer: send command to EC and get response
> >> + * Returns 0 if the communication succeeded, but that doesn't mean the EC
> >> + * was happy with the command it got. Caller should check msg.result for
> >> + * the EC's result code.
> >> *
> >> * @priv: Private data
> >> * @irq: Interrupt to use
> >> @@ -83,7 +84,6 @@ struct cros_ec_command {
> >> * @parent: pointer to parent device (e.g. i2c or spi device)
> >> * @wake_enabled: true if this device can wake the system from sleep
> >> * @lock: one transaction at a time
> >> - * @cmd_xfer: low-level channel to the EC
> >> */
> >> struct cros_ec_device {
> >>
> >> @@ -94,13 +94,8 @@ struct cros_ec_device {
> >> bool was_wake_device;
> >> struct class *cros_class;
> >> struct blocking_notifier_head event_notifier;
> >> - int (*command_send)(struct cros_ec_device *ec,
> >> - uint16_t cmd, void *out_buf, int out_len);
> >> - int (*command_recv)(struct cros_ec_device *ec,
> >> - uint16_t cmd, void *in_buf, int in_len);
> >> - int (*command_sendrecv)(struct cros_ec_device *ec,
> >> - uint16_t cmd, void *out_buf, int out_len,
> >> - void *in_buf, int in_len);
> >> + int (*cmd_xfer)(struct cros_ec_device *ec,
> >> + struct cros_ec_command *msg);
> >>
> >> /* These are used to implement the platform-specific interface */
> >> void *priv;
> >> @@ -112,8 +107,6 @@ struct cros_ec_device {
> >> struct device *parent;
> >> bool wake_enabled;
> >> struct mutex lock;
> >> - int (*cmd_xfer)(struct cros_ec_device *ec,
> >> - struct cros_ec_command *msg);
> >
> > Seems odd - maybe this line move of cmd_xfer() should be in an earlier patch?
>
> It got moved from "private" to public. Bill reorganized all the
> public stuff at the top and the private at the bottom.
>
> -Doug
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH v6 3/7] mfd: AXP20x: Add bindings documentation
From: Lee Jones @ 2014-06-18 9:06 UTC (permalink / raw)
To: Boris BREZILLON
Cc: Carlo Caione, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
hdegoede-H+wXaHxf7aLQT0dZR+AlfA, emilio-0Z03zUJReD5OxF6Tv1QG9Q,
wens-jdAy2FN1RRM, sameo-VuQAYsv1563Yd54FQh9/CA,
dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA,
lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, broonie-DgEjT+Ai2ygdnm+yROfE0A
In-Reply-To: <539FED44.1050904-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> On 19/05/2014 21:47, Carlo Caione wrote:
> > Bindings documentation for the AXP20x driver. In this file also
> > sub-nodes are documented.
>
> I think this patch has been forgotten (I didn't find it in linus' tree
> or linux-next tree).
This patch-set became confusing. Some of the patches were applied in
v5 and were still submitted for v6. I need Carlo to rebase on top of
v3.16-rc1+ and re-submit with the Acks he's collected so far.
> > Signed-off-by: Carlo Caione <carlo-KA+7E9HrN00dnm+yROfE0A@public.gmane.org>
> > Acked-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> > ---
> > Documentation/devicetree/bindings/mfd/axp20x.txt | 93 ++++++++++++++++++++++++
> > 1 file changed, 93 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/mfd/axp20x.txt
> >
> > diff --git a/Documentation/devicetree/bindings/mfd/axp20x.txt b/Documentation/devicetree/bindings/mfd/axp20x.txt
> > new file mode 100644
> > index 0000000..cc9e01b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mfd/axp20x.txt
> > @@ -0,0 +1,93 @@
> > +AXP202/AXP209 device tree bindings
> > +
> > +The axp20x family current members :-
> > +axp202 (X-Powers)
> > +axp209 (X-Powers)
> > +
> > +Required properties:
> > +- compatible: "x-powers,axp202" or "x-powers,axp209"
> > +- reg: The I2C slave address for the AXP chip
> > +- interrupt-parent: The parent interrupt controller
> > +- interrupts: Interrupt specifiers for interrupt sources
> > +- interrupt-controller: axp20x has its own internal IRQs
> > +- #interrupt-cells: Should be set to 1
> > +- acin-supply: The input supply for LDO1
> > +- vin2-supply: The input supply for DCDC2
> > +- vin3-supply: The input supply for DCDC3
> > +- ldo24in-supply: The input supply for LDO2, LDO4
> > +- ldo3in-supply: The input supply for LDO3
> > +- ldo5in-supply: The input supply for LDO5
> > +
> > +- regulators: A node that houses a sub-node for each regulator. The regulators are
> > + bound using their name as listed here: dcdc2, dcdc3, ldo1, ldo2,
> > + ldo3, ldo4, ldo5. The bindings details of individual regulator
> > + device can be found in:
> > + Documentation/devicetree/bindings/regulator/regulator.txt with
> > + the exception of x-powers,dcdc-freq
> > +- x-powers,dcdc-freq: defines the work frequency of DC-DC in KHz
> > + (range: 750-1875). Default: 1.5MHz
> > +
> > +Optional properties for DCDCs:
> > +- x-powers,dcdc-workmode: 1 for PWM mode, 0 for AUTO mode
> > + Default: AUTO mode
> > +
> > +Example:
> > +
> > +axp209: pmic@34 {
> > + compatible = "x-powers,axp209";
> > + reg = <0x34>;
> > + interrupt-parent = <&nmi_intc>;
> > + interrupts = <0 8>;
> > +
> > + interrupt-controller;
> > + #interrupt-cells = <1>;
> > +
> > + acin-supply = <&axp_ipsout_reg>;
> > + vin2-supply = <&axp_ipsout_reg>;
> > + vin3-supply = <&axp_ipsout_reg>;
> > + ldo24in-supply = <&axp_ipsout_reg>;
> > + ldo3in-supply = <&axp_ipsout_reg>;
> > + ldo5in-supply = <&axp_ipsout_reg>;
> > +
> > + regulators {
> > + x-powers,dcdc-freq = <1500>;
> > +
> > + axp_vcore_reg: dcdc2 {
> > + regulator-min-microvolt = <700000>;
> > + regulator-max-microvolt = <2275000>;
> > + regulator-always-on;
> > + };
> > +
> > + axp_ddr_reg: dcdc3 {
> > + regulator-min-microvolt = <700000>;
> > + regulator-max-microvolt = <3500000>;
> > + regulator-always-on;
> > + };
> > +
> > + axp_rtc_reg: ldo1 {
> > + regulator-always-on;
> > + };
> > +
> > + axp_analog_reg: ldo2 {
> > + regulator-min-microvolt = <1800000>;
> > + regulator-max-microvolt = <3300000>;
> > + regulator-always-on;
> > + };
> > +
> > + axp_pll_reg: ldo3 {
> > + regulator-min-microvolt = <700000>;
> > + regulator-max-microvolt = <3500000>;
> > + };
> > +
> > + axp_hdmi_reg: ldo4 {
> > + regulator-min-microvolt = <1250000>;
> > + regulator-max-microvolt = <3300000>;
> > + };
> > +
> > + axp_mic_reg: ldo5 {
> > + regulator-min-microvolt = <1800000>;
> > + regulator-max-microvolt = <3300000>;
> > + };
> > + };
> > +};
> > +
>
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* Re: bad handling of HID input reports for a Delcom foot pedal
From: Jiri Kosina @ 2014-06-18 12:13 UTC (permalink / raw)
To: Benjamin Tissoires; +Cc: linux-input, bmills
In-Reply-To: <CAN+gG=GbD-NjJUcSjYfp3rqnG9hHwmxT0ReEYFFDf88v_aS5qw@mail.gmail.com>
On Mon, 19 May 2014, Benjamin Tissoires wrote:
> Hi Jiri,
>
> Please have a look at: https://bugzilla.kernel.org/show_bug.cgi?id=76461
>
> To me, this device sends garbage and should not send the modifiers
> both in a Var input and in an Array input.
>
> I can fix it either by amending the report descriptor of the device
> (one byte to change) or by changing hid-input.c for it to send the
> events in the array processing no matter what was the previous state
> (see below).
>
> I don't know which one to pick because this seems to be a corner case,
> so fixing hid-input might not be the place, but I am afraid of
> removing part of the reports coming from the device (what if the
> modifiers are actually used for something in the report).
>
> Any thoughts on it?
>
> For the record, fixing hid-input would just remove the test in hid_input_field:
>
> if (value[n] >= min && value[n] <= max
> - && field->usage[value[n] - min].hid
> - && search(field->value, value[n], count))
> + && field->usage[value[n] - min].hid)
> hid_process_event(hid, field,
> &field->usage[value[n] - min], 1, interrupt);
> }
>
> (Sorry, gmail, whitespace damaged and everything)
So if modifying the report descriptor is not the option (which is what I
understood from Bryan's mail), I have to say I don't really like this
change in hid_input_field(), as it's really rather "non obvious".
How about just creating a device specific input event quirk for this?
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* [PATCH v9] leds: USB: HID: Add support for MSI GT683R led panels
From: Janne Kanniainen @ 2014-06-18 16:05 UTC (permalink / raw)
To: jkosina
Cc: johan, cooloney, linux-kernel, linux-leds, linux-usb, linux-input,
Janne Kanniainen
In-Reply-To: <20140618073950.GC31163@localhost>
This driver adds support for USB controlled led panels that exists in
MSI GT683R laptop
Signed-off-by: Janne Kanniainen <janne.kanniainen@gmail.com>
---
Changes in v2:
- sorted headers to alphabetic order
- using devm_kzalloc
- using BIT(n)
- using usb_control_msg instead of usb_submit_urb
- removing unneeded code
Changes in v3:
- implemented as HID device
- some cleanups and bug fixes
Changes in v4:
- more cleanups
- support for selecting leds
- suppport for selecting status
Changes in v5:
- mode attribute documented under Documentation/ABI
- made array for led_classdev
- led devices uses now recommended naming scheme
Changes in v6:
- flush_work added
- using hid device name instead of hard coded gt683r
- allocating name buffers with devm_kzalloc
Changes in v7:
- buf with for fixed
Changes in v8:
- some cleanups and bugs fixed
Changes in v9:
- few style issues fixed
.../ABI/testing/sysfs-class-hid-driver-gt683r | 14 +
drivers/hid/Kconfig | 14 +
drivers/hid/Makefile | 1 +
drivers/hid/hid-core.c | 1 +
drivers/hid/hid-gt683r.c | 309 +++++++++++++++++++++
drivers/hid/hid-ids.h | 2 +-
drivers/hid/usbhid/hid-quirks.c | 2 +-
7 files changed, 341 insertions(+), 2 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-class-hid-driver-gt683r
create mode 100644 drivers/hid/hid-gt683r.c
diff --git a/Documentation/ABI/testing/sysfs-class-hid-driver-gt683r b/Documentation/ABI/testing/sysfs-class-hid-driver-gt683r
new file mode 100644
index 0000000..317e9d5
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-hid-driver-gt683r
@@ -0,0 +1,14 @@
+What: /sys/class/hidraw/<hidraw>/device/leds_mode
+Date: Jun 2014
+KernelVersion: 3.17
+Contact: Janne Kanniainen <janne.kanniainen@gmail.com>
+Description:
+ Set the mode of LEDs
+
+ 0 - normal
+ 1 - audio
+ 2 - breathing
+
+ Normal: LEDs are fully on when enabled
+ Audio: LEDs brightness depends on sound level
+ Breathing: LEDs brightness varies at human breathing rate
\ No newline at end of file
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 7af9d0b..e2f4590 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -261,6 +261,20 @@ config HOLTEK_FF
Say Y here if you have a Holtek On Line Grip based game controller
and want to have force feedback support for it.
+config HID_GT683R
+ tristate "MSI GT68xR LED support"
+ depends on LEDS_CLASS && USB_HID
+ ---help---
+ Say Y here if you want to enable support for the three MSI GT68xR LEDs
+
+ This driver support following modes:
+ - Normal: LEDs are fully on when enabled
+ - Audio: LEDs brightness depends on sound level
+ - Breathing: LEDs brightness varies at human breathing rate
+
+ Currently the following devices are know to be supported:
+ - MSI GT683R
+
config HID_HUION
tristate "Huion tablets"
depends on USB_HID
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index fc712dd..7129311 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -48,6 +48,7 @@ obj-$(CONFIG_HID_EMS_FF) += hid-emsff.o
obj-$(CONFIG_HID_ELECOM) += hid-elecom.o
obj-$(CONFIG_HID_ELO) += hid-elo.o
obj-$(CONFIG_HID_EZKEY) += hid-ezkey.o
+obj-$(CONFIG_HID_GT683R) += hid-gt683r.o
obj-$(CONFIG_HID_GYRATION) += hid-gyration.o
obj-$(CONFIG_HID_HOLTEK) += hid-holtek-kbd.o
obj-$(CONFIG_HID_HOLTEK) += hid-holtek-mouse.o
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index da52279..ec88fdb 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1827,6 +1827,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_WIRELESS_OPTICAL_DESKTOP_3_0) },
{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_OFFICE_KB) },
{ HID_USB_DEVICE(USB_VENDOR_ID_MONTEREY, USB_DEVICE_ID_GENIUS_KB29E) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GT683R_LED_PANEL) },
{ HID_USB_DEVICE(USB_VENDOR_ID_NTRIG, USB_DEVICE_ID_NTRIG_TOUCH_SCREEN) },
{ HID_USB_DEVICE(USB_VENDOR_ID_NTRIG, USB_DEVICE_ID_NTRIG_TOUCH_SCREEN_1) },
{ HID_USB_DEVICE(USB_VENDOR_ID_NTRIG, USB_DEVICE_ID_NTRIG_TOUCH_SCREEN_2) },
diff --git a/drivers/hid/hid-gt683r.c b/drivers/hid/hid-gt683r.c
new file mode 100644
index 0000000..077f7a1
--- /dev/null
+++ b/drivers/hid/hid-gt683r.c
@@ -0,0 +1,309 @@
+/*
+ * MSI GT683R led driver
+ *
+ * Copyright (c) 2014 Janne Kanniainen <janne.kanniainen@gmail.com>
+ *
+ * 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; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/device.h>
+#include <linux/hid.h>
+#include <linux/kernel.h>
+#include <linux/leds.h>
+#include <linux/module.h>
+
+#include "hid-ids.h"
+
+#define GT683R_BUFFER_SIZE 8
+
+/*
+ * GT683R_LED_OFF: all LEDs are off
+ * GT683R_LED_AUDIO: LEDs brightness depends on sound level
+ * GT683R_LED_BREATHING: LEDs brightness varies at human breathing rate
+ * GT683R_LED_NORMAL: LEDs are fully on when enabled
+ */
+enum gt683r_led_mode {
+ GT683R_LED_OFF = 0,
+ GT683R_LED_AUDIO = 2,
+ GT683R_LED_BREATHING = 3,
+ GT683R_LED_NORMAL = 5
+};
+
+enum gt683r_panels {
+ GT683R_LED_BACK = 0,
+ GT683R_LED_SIDE = 1,
+ GT683R_LED_FRONT = 2,
+ GT683R_LED_COUNT,
+};
+
+static const char * const gt683r_panel_names[] = {
+ "back",
+ "side",
+ "front",
+};
+
+struct gt683r_led {
+ struct hid_device *hdev;
+ struct led_classdev led_devs[GT683R_LED_COUNT];
+ struct mutex lock;
+ struct work_struct work;
+ enum led_brightness brightnesses[GT683R_LED_COUNT];
+ enum gt683r_led_mode mode;
+};
+
+static const struct hid_device_id gt683r_led_id[] = {
+ { HID_USB_DEVICE(USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GT683R_LED_PANEL) },
+ { }
+};
+
+static void gt683r_brightness_set(struct led_classdev *led_cdev,
+ enum led_brightness brightness)
+{
+ int i;
+ struct device *dev = led_cdev->dev->parent;
+ struct hid_device *hdev = container_of(dev, struct hid_device, dev);
+ struct gt683r_led *led = hid_get_drvdata(hdev);
+
+ for (i = 0; i < GT683R_LED_COUNT; i++) {
+ if (led_cdev == &led->led_devs[i])
+ break;
+ }
+
+ if (i < GT683R_LED_COUNT) {
+ led->brightnesses[i] = brightness;
+ schedule_work(&led->work);
+ }
+}
+
+static ssize_t leds_mode_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ u8 sysfs_mode;
+ struct hid_device *hdev = container_of(dev, struct hid_device, dev);
+ struct gt683r_led *led = hid_get_drvdata(hdev);
+
+ if (led->mode == GT683R_LED_NORMAL)
+ sysfs_mode = 0;
+ else if (led->mode == GT683R_LED_AUDIO)
+ sysfs_mode = 1;
+ else
+ sysfs_mode = 2;
+
+ return scnprintf(buf, PAGE_SIZE, "%u\n", sysfs_mode);
+}
+
+static ssize_t leds_mode_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ u8 sysfs_mode;
+ struct hid_device *hdev = container_of(dev, struct hid_device, dev);
+ struct gt683r_led *led = hid_get_drvdata(hdev);
+
+
+ if (kstrtou8(buf, 10, &sysfs_mode) || sysfs_mode > 2)
+ return -EINVAL;
+
+ mutex_lock(&led->lock);
+
+ if (sysfs_mode == 0)
+ led->mode = GT683R_LED_NORMAL;
+ else if (sysfs_mode == 1)
+ led->mode = GT683R_LED_AUDIO;
+ else
+ led->mode = GT683R_LED_BREATHING;
+
+ mutex_unlock(&led->lock);
+ schedule_work(&led->work);
+
+ return count;
+}
+
+static int gt683r_led_snd_msg(struct gt683r_led *led, u8 *msg)
+{
+ int ret;
+
+ ret = hid_hw_raw_request(led->hdev, msg[0], msg, GT683R_BUFFER_SIZE,
+ HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
+ if (ret != GT683R_BUFFER_SIZE) {
+ hid_err(led->hdev,
+ "failed to send set report request: %i\n", ret);
+ if (ret < 0)
+ return ret;
+ return -EIO;
+ }
+
+ return 0;
+}
+
+static int gt683r_leds_set(struct gt683r_led *led, u8 leds)
+{
+ int ret;
+ u8 *buffer;
+
+ buffer = kzalloc(GT683R_BUFFER_SIZE, GFP_KERNEL);
+ if (!buffer)
+ return -ENOMEM;
+
+ buffer[0] = 0x01;
+ buffer[1] = 0x02;
+ buffer[2] = 0x30;
+ buffer[3] = leds;
+ ret = gt683r_led_snd_msg(led, buffer);
+
+ kfree(buffer);
+ return ret;
+}
+
+static int gt683r_mode_set(struct gt683r_led *led, u8 mode)
+{
+ int ret;
+ u8 *buffer;
+
+ buffer = kzalloc(GT683R_BUFFER_SIZE, GFP_KERNEL);
+ if (!buffer)
+ return -ENOMEM;
+
+ buffer[0] = 0x01;
+ buffer[1] = 0x02;
+ buffer[2] = 0x20;
+ buffer[3] = mode;
+ buffer[4] = 0x01;
+ ret = gt683r_led_snd_msg(led, buffer);
+
+ kfree(buffer);
+ return ret;
+}
+
+static void gt683r_led_work(struct work_struct *work)
+{
+ int i;
+ u8 leds = 0;
+ u8 mode;
+ struct gt683r_led *led = container_of(work, struct gt683r_led, work);
+
+ mutex_lock(&led->lock);
+
+ for (i = 0; i < GT683R_LED_COUNT; i++) {
+ if (led->brightnesses[i])
+ leds |= BIT(i);
+ }
+
+ if (gt683r_leds_set(led, leds))
+ goto fail;
+
+ if (leds)
+ mode = led->mode;
+ else
+ mode = GT683R_LED_OFF;
+
+ gt683r_mode_set(led, mode);
+fail:
+ mutex_unlock(&led->lock);
+}
+
+static DEVICE_ATTR_RW(leds_mode);
+
+static int gt683r_led_probe(struct hid_device *hdev,
+ const struct hid_device_id *id)
+{
+ int i;
+ int ret;
+ int name_sz;
+ char *name;
+ struct gt683r_led *led;
+
+ led = devm_kzalloc(&hdev->dev, sizeof(*led), GFP_KERNEL);
+ if (!led)
+ return -ENOMEM;
+
+ led->mode = GT683R_LED_NORMAL;
+ led->hdev = hdev;
+ hid_set_drvdata(hdev, led);
+
+ ret = hid_parse(hdev);
+ if (ret) {
+ hid_err(hdev, "hid parsing failed\n");
+ return ret;
+ }
+
+ ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
+ if (ret) {
+ hid_err(hdev, "hw start failed\n");
+ return ret;
+ }
+
+ for (i = 0; i < GT683R_LED_COUNT; i++) {
+ name_sz = strlen(dev_name(&hdev->dev)) +
+ strlen(gt683r_panel_names[i]) + 3;
+
+ name = devm_kzalloc(&hdev->dev, name_sz, GFP_KERNEL);
+ if (!name) {
+ ret = -ENOMEM;
+ goto fail;
+ }
+
+ snprintf(name, name_sz, "%s::%s",
+ dev_name(&hdev->dev), gt683r_panel_names[i]);
+ led->led_devs[i].name = name;
+ led->led_devs[i].max_brightness = 1;
+ led->led_devs[i].brightness_set = gt683r_brightness_set;
+ ret = led_classdev_register(&hdev->dev, &led->led_devs[i]);
+ if (ret) {
+ hid_err(hdev, "could not register led device\n");
+ goto fail;
+ }
+ }
+
+ ret = device_create_file(&led->hdev->dev, &dev_attr_leds_mode);
+ if (ret) {
+ hid_err(hdev, "could not make mode attribute file\n");
+ goto fail;
+ }
+
+ mutex_init(&led->lock);
+ INIT_WORK(&led->work, gt683r_led_work);
+
+ return 0;
+
+fail:
+ for (i = i - 1; i >= 0; i--)
+ led_classdev_unregister(&led->led_devs[i]);
+ hid_hw_stop(hdev);
+ return ret;
+}
+
+static void gt683r_led_remove(struct hid_device *hdev)
+{
+ int i;
+ struct gt683r_led *led = hid_get_drvdata(hdev);
+
+ device_remove_file(&hdev->dev, &dev_attr_leds_mode);
+ for (i = 0; i < GT683R_LED_COUNT; i++)
+ led_classdev_unregister(&led->led_devs[i]);
+ flush_work(&led->work);
+ hid_hw_stop(hdev);
+}
+
+static struct hid_driver gt683r_led_driver = {
+ .probe = gt683r_led_probe,
+ .remove = gt683r_led_remove,
+ .name = "gt683r_led",
+ .id_table = gt683r_led_id,
+};
+
+module_hid_driver(gt683r_led_driver);
+
+MODULE_AUTHOR("Janne Kanniainen");
+MODULE_DESCRIPTION("MSI GT683R led driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 34bb220..3692d37 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -641,7 +641,7 @@
#define USB_DEVICE_ID_GENIUS_KB29E 0x3004
#define USB_VENDOR_ID_MSI 0x1770
-#define USB_DEVICE_ID_MSI_GX680R_LED_PANEL 0xff00
+#define USB_DEVICE_ID_MSI_GT683R_LED_PANEL 0xff00
#define USB_VENDOR_ID_NATIONAL_SEMICONDUCTOR 0x0400
#define USB_DEVICE_ID_N_S_HARMONY 0xc359
diff --git a/drivers/hid/usbhid/hid-quirks.c b/drivers/hid/usbhid/hid-quirks.c
index 8e4ddb3..c640e1d 100644
--- a/drivers/hid/usbhid/hid-quirks.c
+++ b/drivers/hid/usbhid/hid-quirks.c
@@ -73,7 +73,7 @@ static const struct hid_blacklist {
{ USB_VENDOR_ID_FORMOSA, USB_DEVICE_ID_FORMOSA_IR_RECEIVER, HID_QUIRK_NO_INIT_REPORTS },
{ USB_VENDOR_ID_FREESCALE, USB_DEVICE_ID_FREESCALE_MX28, HID_QUIRK_NOGET },
{ USB_VENDOR_ID_MGE, USB_DEVICE_ID_MGE_UPS, HID_QUIRK_NOGET },
- { USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GX680R_LED_PANEL, HID_QUIRK_NO_INIT_REPORTS },
+ { USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GT683R_LED_PANEL, HID_QUIRK_NO_INIT_REPORTS },
{ USB_VENDOR_ID_NEXIO, USB_DEVICE_ID_NEXIO_MULTITOUCH_PTI0750, HID_QUIRK_NO_INIT_REPORTS },
{ USB_VENDOR_ID_NOVATEK, USB_DEVICE_ID_NOVATEK_MOUSE, HID_QUIRK_NO_INIT_REPORTS },
{ USB_VENDOR_ID_PIXART, USB_DEVICE_ID_PIXART_OPTICAL_TOUCH_SCREEN, HID_QUIRK_NO_INIT_REPORTS },
--
1.9.2
^ permalink raw reply related
* Re: [PATCH v9] leds: USB: HID: Add support for MSI GT683R led panels
From: Johan Hovold @ 2014-06-18 16:11 UTC (permalink / raw)
To: Janne Kanniainen
Cc: jkosina-AlSwsSmVLrQ, johan-DgEjT+Ai2ygdnm+yROfE0A,
cooloney-Re5JQEeQqe8AvxtiuMwx3w,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-leds-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1403107502-14106-1-git-send-email-janne.kanniainen-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On Wed, Jun 18, 2014 at 07:05:02PM +0300, Janne Kanniainen wrote:
> This driver adds support for USB controlled led panels that exists in
> MSI GT683R laptop
>
> Signed-off-by: Janne Kanniainen <janne.kanniainen-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Reviewed-by: Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Thanks, Janne!
Johan
> ---
> Changes in v2:
> - sorted headers to alphabetic order
> - using devm_kzalloc
> - using BIT(n)
> - using usb_control_msg instead of usb_submit_urb
> - removing unneeded code
> Changes in v3:
> - implemented as HID device
> - some cleanups and bug fixes
>
> Changes in v4:
> - more cleanups
> - support for selecting leds
> - suppport for selecting status
>
> Changes in v5:
> - mode attribute documented under Documentation/ABI
> - made array for led_classdev
> - led devices uses now recommended naming scheme
>
> Changes in v6:
> - flush_work added
> - using hid device name instead of hard coded gt683r
> - allocating name buffers with devm_kzalloc
>
> Changes in v7:
> - buf with for fixed
>
> Changes in v8:
> - some cleanups and bugs fixed
>
> Changes in v9:
> - few style issues fixed
>
> .../ABI/testing/sysfs-class-hid-driver-gt683r | 14 +
> drivers/hid/Kconfig | 14 +
> drivers/hid/Makefile | 1 +
> drivers/hid/hid-core.c | 1 +
> drivers/hid/hid-gt683r.c | 309 +++++++++++++++++++++
> drivers/hid/hid-ids.h | 2 +-
> drivers/hid/usbhid/hid-quirks.c | 2 +-
> 7 files changed, 341 insertions(+), 2 deletions(-)
> create mode 100644 Documentation/ABI/testing/sysfs-class-hid-driver-gt683r
> create mode 100644 drivers/hid/hid-gt683r.c
>
> diff --git a/Documentation/ABI/testing/sysfs-class-hid-driver-gt683r b/Documentation/ABI/testing/sysfs-class-hid-driver-gt683r
> new file mode 100644
> index 0000000..317e9d5
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-class-hid-driver-gt683r
> @@ -0,0 +1,14 @@
> +What: /sys/class/hidraw/<hidraw>/device/leds_mode
> +Date: Jun 2014
> +KernelVersion: 3.17
> +Contact: Janne Kanniainen <janne.kanniainen-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> +Description:
> + Set the mode of LEDs
> +
> + 0 - normal
> + 1 - audio
> + 2 - breathing
> +
> + Normal: LEDs are fully on when enabled
> + Audio: LEDs brightness depends on sound level
> + Breathing: LEDs brightness varies at human breathing rate
> \ No newline at end of file
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 7af9d0b..e2f4590 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -261,6 +261,20 @@ config HOLTEK_FF
> Say Y here if you have a Holtek On Line Grip based game controller
> and want to have force feedback support for it.
>
> +config HID_GT683R
> + tristate "MSI GT68xR LED support"
> + depends on LEDS_CLASS && USB_HID
> + ---help---
> + Say Y here if you want to enable support for the three MSI GT68xR LEDs
> +
> + This driver support following modes:
> + - Normal: LEDs are fully on when enabled
> + - Audio: LEDs brightness depends on sound level
> + - Breathing: LEDs brightness varies at human breathing rate
> +
> + Currently the following devices are know to be supported:
> + - MSI GT683R
> +
> config HID_HUION
> tristate "Huion tablets"
> depends on USB_HID
> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> index fc712dd..7129311 100644
> --- a/drivers/hid/Makefile
> +++ b/drivers/hid/Makefile
> @@ -48,6 +48,7 @@ obj-$(CONFIG_HID_EMS_FF) += hid-emsff.o
> obj-$(CONFIG_HID_ELECOM) += hid-elecom.o
> obj-$(CONFIG_HID_ELO) += hid-elo.o
> obj-$(CONFIG_HID_EZKEY) += hid-ezkey.o
> +obj-$(CONFIG_HID_GT683R) += hid-gt683r.o
> obj-$(CONFIG_HID_GYRATION) += hid-gyration.o
> obj-$(CONFIG_HID_HOLTEK) += hid-holtek-kbd.o
> obj-$(CONFIG_HID_HOLTEK) += hid-holtek-mouse.o
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index da52279..ec88fdb 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -1827,6 +1827,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
> { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_WIRELESS_OPTICAL_DESKTOP_3_0) },
> { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_OFFICE_KB) },
> { HID_USB_DEVICE(USB_VENDOR_ID_MONTEREY, USB_DEVICE_ID_GENIUS_KB29E) },
> + { HID_USB_DEVICE(USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GT683R_LED_PANEL) },
> { HID_USB_DEVICE(USB_VENDOR_ID_NTRIG, USB_DEVICE_ID_NTRIG_TOUCH_SCREEN) },
> { HID_USB_DEVICE(USB_VENDOR_ID_NTRIG, USB_DEVICE_ID_NTRIG_TOUCH_SCREEN_1) },
> { HID_USB_DEVICE(USB_VENDOR_ID_NTRIG, USB_DEVICE_ID_NTRIG_TOUCH_SCREEN_2) },
> diff --git a/drivers/hid/hid-gt683r.c b/drivers/hid/hid-gt683r.c
> new file mode 100644
> index 0000000..077f7a1
> --- /dev/null
> +++ b/drivers/hid/hid-gt683r.c
> @@ -0,0 +1,309 @@
> +/*
> + * MSI GT683R led driver
> + *
> + * Copyright (c) 2014 Janne Kanniainen <janne.kanniainen-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> + *
> + * 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; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include <linux/device.h>
> +#include <linux/hid.h>
> +#include <linux/kernel.h>
> +#include <linux/leds.h>
> +#include <linux/module.h>
> +
> +#include "hid-ids.h"
> +
> +#define GT683R_BUFFER_SIZE 8
> +
> +/*
> + * GT683R_LED_OFF: all LEDs are off
> + * GT683R_LED_AUDIO: LEDs brightness depends on sound level
> + * GT683R_LED_BREATHING: LEDs brightness varies at human breathing rate
> + * GT683R_LED_NORMAL: LEDs are fully on when enabled
> + */
> +enum gt683r_led_mode {
> + GT683R_LED_OFF = 0,
> + GT683R_LED_AUDIO = 2,
> + GT683R_LED_BREATHING = 3,
> + GT683R_LED_NORMAL = 5
> +};
> +
> +enum gt683r_panels {
> + GT683R_LED_BACK = 0,
> + GT683R_LED_SIDE = 1,
> + GT683R_LED_FRONT = 2,
> + GT683R_LED_COUNT,
> +};
> +
> +static const char * const gt683r_panel_names[] = {
> + "back",
> + "side",
> + "front",
> +};
> +
> +struct gt683r_led {
> + struct hid_device *hdev;
> + struct led_classdev led_devs[GT683R_LED_COUNT];
> + struct mutex lock;
> + struct work_struct work;
> + enum led_brightness brightnesses[GT683R_LED_COUNT];
> + enum gt683r_led_mode mode;
> +};
> +
> +static const struct hid_device_id gt683r_led_id[] = {
> + { HID_USB_DEVICE(USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GT683R_LED_PANEL) },
> + { }
> +};
> +
> +static void gt683r_brightness_set(struct led_classdev *led_cdev,
> + enum led_brightness brightness)
> +{
> + int i;
> + struct device *dev = led_cdev->dev->parent;
> + struct hid_device *hdev = container_of(dev, struct hid_device, dev);
> + struct gt683r_led *led = hid_get_drvdata(hdev);
> +
> + for (i = 0; i < GT683R_LED_COUNT; i++) {
> + if (led_cdev == &led->led_devs[i])
> + break;
> + }
> +
> + if (i < GT683R_LED_COUNT) {
> + led->brightnesses[i] = brightness;
> + schedule_work(&led->work);
> + }
> +}
> +
> +static ssize_t leds_mode_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + u8 sysfs_mode;
> + struct hid_device *hdev = container_of(dev, struct hid_device, dev);
> + struct gt683r_led *led = hid_get_drvdata(hdev);
> +
> + if (led->mode == GT683R_LED_NORMAL)
> + sysfs_mode = 0;
> + else if (led->mode == GT683R_LED_AUDIO)
> + sysfs_mode = 1;
> + else
> + sysfs_mode = 2;
> +
> + return scnprintf(buf, PAGE_SIZE, "%u\n", sysfs_mode);
> +}
> +
> +static ssize_t leds_mode_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + u8 sysfs_mode;
> + struct hid_device *hdev = container_of(dev, struct hid_device, dev);
> + struct gt683r_led *led = hid_get_drvdata(hdev);
> +
> +
> + if (kstrtou8(buf, 10, &sysfs_mode) || sysfs_mode > 2)
> + return -EINVAL;
> +
> + mutex_lock(&led->lock);
> +
> + if (sysfs_mode == 0)
> + led->mode = GT683R_LED_NORMAL;
> + else if (sysfs_mode == 1)
> + led->mode = GT683R_LED_AUDIO;
> + else
> + led->mode = GT683R_LED_BREATHING;
> +
> + mutex_unlock(&led->lock);
> + schedule_work(&led->work);
> +
> + return count;
> +}
> +
> +static int gt683r_led_snd_msg(struct gt683r_led *led, u8 *msg)
> +{
> + int ret;
> +
> + ret = hid_hw_raw_request(led->hdev, msg[0], msg, GT683R_BUFFER_SIZE,
> + HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
> + if (ret != GT683R_BUFFER_SIZE) {
> + hid_err(led->hdev,
> + "failed to send set report request: %i\n", ret);
> + if (ret < 0)
> + return ret;
> + return -EIO;
> + }
> +
> + return 0;
> +}
> +
> +static int gt683r_leds_set(struct gt683r_led *led, u8 leds)
> +{
> + int ret;
> + u8 *buffer;
> +
> + buffer = kzalloc(GT683R_BUFFER_SIZE, GFP_KERNEL);
> + if (!buffer)
> + return -ENOMEM;
> +
> + buffer[0] = 0x01;
> + buffer[1] = 0x02;
> + buffer[2] = 0x30;
> + buffer[3] = leds;
> + ret = gt683r_led_snd_msg(led, buffer);
> +
> + kfree(buffer);
> + return ret;
> +}
> +
> +static int gt683r_mode_set(struct gt683r_led *led, u8 mode)
> +{
> + int ret;
> + u8 *buffer;
> +
> + buffer = kzalloc(GT683R_BUFFER_SIZE, GFP_KERNEL);
> + if (!buffer)
> + return -ENOMEM;
> +
> + buffer[0] = 0x01;
> + buffer[1] = 0x02;
> + buffer[2] = 0x20;
> + buffer[3] = mode;
> + buffer[4] = 0x01;
> + ret = gt683r_led_snd_msg(led, buffer);
> +
> + kfree(buffer);
> + return ret;
> +}
> +
> +static void gt683r_led_work(struct work_struct *work)
> +{
> + int i;
> + u8 leds = 0;
> + u8 mode;
> + struct gt683r_led *led = container_of(work, struct gt683r_led, work);
> +
> + mutex_lock(&led->lock);
> +
> + for (i = 0; i < GT683R_LED_COUNT; i++) {
> + if (led->brightnesses[i])
> + leds |= BIT(i);
> + }
> +
> + if (gt683r_leds_set(led, leds))
> + goto fail;
> +
> + if (leds)
> + mode = led->mode;
> + else
> + mode = GT683R_LED_OFF;
> +
> + gt683r_mode_set(led, mode);
> +fail:
> + mutex_unlock(&led->lock);
> +}
> +
> +static DEVICE_ATTR_RW(leds_mode);
> +
> +static int gt683r_led_probe(struct hid_device *hdev,
> + const struct hid_device_id *id)
> +{
> + int i;
> + int ret;
> + int name_sz;
> + char *name;
> + struct gt683r_led *led;
> +
> + led = devm_kzalloc(&hdev->dev, sizeof(*led), GFP_KERNEL);
> + if (!led)
> + return -ENOMEM;
> +
> + led->mode = GT683R_LED_NORMAL;
> + led->hdev = hdev;
> + hid_set_drvdata(hdev, led);
> +
> + ret = hid_parse(hdev);
> + if (ret) {
> + hid_err(hdev, "hid parsing failed\n");
> + return ret;
> + }
> +
> + ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
> + if (ret) {
> + hid_err(hdev, "hw start failed\n");
> + return ret;
> + }
> +
> + for (i = 0; i < GT683R_LED_COUNT; i++) {
> + name_sz = strlen(dev_name(&hdev->dev)) +
> + strlen(gt683r_panel_names[i]) + 3;
> +
> + name = devm_kzalloc(&hdev->dev, name_sz, GFP_KERNEL);
> + if (!name) {
> + ret = -ENOMEM;
> + goto fail;
> + }
> +
> + snprintf(name, name_sz, "%s::%s",
> + dev_name(&hdev->dev), gt683r_panel_names[i]);
> + led->led_devs[i].name = name;
> + led->led_devs[i].max_brightness = 1;
> + led->led_devs[i].brightness_set = gt683r_brightness_set;
> + ret = led_classdev_register(&hdev->dev, &led->led_devs[i]);
> + if (ret) {
> + hid_err(hdev, "could not register led device\n");
> + goto fail;
> + }
> + }
> +
> + ret = device_create_file(&led->hdev->dev, &dev_attr_leds_mode);
> + if (ret) {
> + hid_err(hdev, "could not make mode attribute file\n");
> + goto fail;
> + }
> +
> + mutex_init(&led->lock);
> + INIT_WORK(&led->work, gt683r_led_work);
> +
> + return 0;
> +
> +fail:
> + for (i = i - 1; i >= 0; i--)
> + led_classdev_unregister(&led->led_devs[i]);
> + hid_hw_stop(hdev);
> + return ret;
> +}
> +
> +static void gt683r_led_remove(struct hid_device *hdev)
> +{
> + int i;
> + struct gt683r_led *led = hid_get_drvdata(hdev);
> +
> + device_remove_file(&hdev->dev, &dev_attr_leds_mode);
> + for (i = 0; i < GT683R_LED_COUNT; i++)
> + led_classdev_unregister(&led->led_devs[i]);
> + flush_work(&led->work);
> + hid_hw_stop(hdev);
> +}
> +
> +static struct hid_driver gt683r_led_driver = {
> + .probe = gt683r_led_probe,
> + .remove = gt683r_led_remove,
> + .name = "gt683r_led",
> + .id_table = gt683r_led_id,
> +};
> +
> +module_hid_driver(gt683r_led_driver);
> +
> +MODULE_AUTHOR("Janne Kanniainen");
> +MODULE_DESCRIPTION("MSI GT683R led driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 34bb220..3692d37 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -641,7 +641,7 @@
> #define USB_DEVICE_ID_GENIUS_KB29E 0x3004
>
> #define USB_VENDOR_ID_MSI 0x1770
> -#define USB_DEVICE_ID_MSI_GX680R_LED_PANEL 0xff00
> +#define USB_DEVICE_ID_MSI_GT683R_LED_PANEL 0xff00
>
> #define USB_VENDOR_ID_NATIONAL_SEMICONDUCTOR 0x0400
> #define USB_DEVICE_ID_N_S_HARMONY 0xc359
> diff --git a/drivers/hid/usbhid/hid-quirks.c b/drivers/hid/usbhid/hid-quirks.c
> index 8e4ddb3..c640e1d 100644
> --- a/drivers/hid/usbhid/hid-quirks.c
> +++ b/drivers/hid/usbhid/hid-quirks.c
> @@ -73,7 +73,7 @@ static const struct hid_blacklist {
> { USB_VENDOR_ID_FORMOSA, USB_DEVICE_ID_FORMOSA_IR_RECEIVER, HID_QUIRK_NO_INIT_REPORTS },
> { USB_VENDOR_ID_FREESCALE, USB_DEVICE_ID_FREESCALE_MX28, HID_QUIRK_NOGET },
> { USB_VENDOR_ID_MGE, USB_DEVICE_ID_MGE_UPS, HID_QUIRK_NOGET },
> - { USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GX680R_LED_PANEL, HID_QUIRK_NO_INIT_REPORTS },
> + { USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GT683R_LED_PANEL, HID_QUIRK_NO_INIT_REPORTS },
> { USB_VENDOR_ID_NEXIO, USB_DEVICE_ID_NEXIO_MULTITOUCH_PTI0750, HID_QUIRK_NO_INIT_REPORTS },
> { USB_VENDOR_ID_NOVATEK, USB_DEVICE_ID_NOVATEK_MOUSE, HID_QUIRK_NO_INIT_REPORTS },
> { USB_VENDOR_ID_PIXART, USB_DEVICE_ID_PIXART_OPTICAL_TOUCH_SCREEN, HID_QUIRK_NO_INIT_REPORTS },
--
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
* [PATCH v2 0/10] Batch of cleanup patches for cros_ec
From: Doug Anderson @ 2014-06-18 18:13 UTC (permalink / raw)
To: Lee Jones
Cc: Andrew Bresticker, swarren, olof, Sonny Rao, linux-samsung-soc,
Javier Martinez Canillas, Bill Richardson, sjg, Wolfram Sang,
broonie, Doug Anderson, sameo, dmitry.torokhov, linux-kernel,
geert, linux-i2c, linux-input
This is a batch of cleanup patches picked from the ChromeOS 3.8 kernel
tree and applied to ToT. Most of these patches were authored by Bill
Richardson (CCed). Where appropriate I've squashed patches together,
though I have erred on the side of keeping patches logically distinct
rather than squashing into one big cleanup patch.
There is very little functionality added by this series, but this gets
us closer to how things look in the ChromeOS tree so we can add more
patches atop it. In general I took the oldest patches from our tree
and stopped picking when I got to a reasonable patch size (10
patches). There are about 5 more cleanup patches still in the
ChromeOS tree, then some more major functionality patches.
Note that I didn't take the "cros_ec_dev" userspace inteface, the
"LPC" implementation, the "vboot context" implementation, and patches
relating to exynos5250-spring when picking patches. These bits are
very separate (and big!) and can be added and debated separately after
we've got cleanup in. Whenever patches touched those pieces of the
code I ignored that part of the patch. In general I did take cleanup
code that was intended to make it easier to later add these bits.
I have tested basic functionality of these patches on exynos5250-snow
and exynos5420-peach-pit.
Changes in v2:
- Include example printouts before/after in commit message.
- Removed unneeded "ret" variable.
- Added common function to cros_ec.c
- Changed to dev_dbg() as per http://crosreview.com/66726
- IRQs should be optional => move EC interrupt to keyboard.
Andrew Bresticker (1):
mfd: cros_ec: move EC interrupt to cros_ec_keyb
Bill Richardson (8):
mfd: cros_ec: Fix the comment on cros_ec_remove()
mfd: cros_ec: Allow static din/dout buffers with cros_ec_register()
mfd: cros_ec: Tweak struct cros_ec_device for clarity
mfd: cros_ec: Use struct cros_ec_command to communicate with the EC
mfd: cros_ec: cleanup: remove unused fields from struct cros_ec_device
mfd: cros_ec: cleanup: Remove EC wrapper functions
mfd: cros_ec: Check result code from EC messages
mfd: cros_ec: ec_dev->cmd_xfer() returns number of bytes received from
EC
Simon Glass (1):
mdf: cros_ec: Detect in-progress commands
drivers/i2c/busses/i2c-cros-ec-tunnel.c | 17 +++--
drivers/input/keyboard/cros_ec_keyb.c | 70 ++++++++++++--------
drivers/mfd/cros_ec.c | 97 ++++++++--------------------
drivers/mfd/cros_ec_i2c.c | 37 +++++------
drivers/mfd/cros_ec_spi.c | 36 +++++------
include/linux/mfd/cros_ec.h | 110 +++++++++++++++++---------------
6 files changed, 172 insertions(+), 195 deletions(-)
--
2.0.0.526.g5318336
^ permalink raw reply
* [PATCH v2 07/10] mfd: cros_ec: cleanup: Remove EC wrapper functions
From: Doug Anderson @ 2014-06-18 18:14 UTC (permalink / raw)
To: Lee Jones
Cc: Andrew Bresticker, swarren-3lzwWm7+Weoh9ZMKESR00Q,
olof-nZhT3qVonbNeoWH0uzbU5w, Sonny Rao,
linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
Javier Martinez Canillas, Bill Richardson,
sjg-F7+t8E8rja9g9hUCZPvPmw, Wolfram Sang,
broonie-DgEjT+Ai2ygdnm+yROfE0A, Doug Anderson,
dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w,
sameo-VuQAYsv1563Yd54FQh9/CA, geert-Td1EMuHUCqxL1ZNQvxDV9g,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1403115247-8853-1-git-send-email-dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
From: Bill Richardson <wfrichar-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Remove the three wrapper functions that talk to the EC without passing all
the desired arguments and just use the underlying communication function
that passes everything in a struct intead.
This is internal code refactoring only. Nothing should change.
Signed-off-by: Bill Richardson <wfrichar-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Signed-off-by: Doug Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Acked-by: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Reviewed-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
Changes in v2:
- Removed unneeded "ret" variable.
drivers/i2c/busses/i2c-cros-ec-tunnel.c | 15 +++++++++++----
drivers/input/keyboard/cros_ec_keyb.c | 12 ++++++++++--
drivers/mfd/cros_ec.c | 32 --------------------------------
include/linux/mfd/cros_ec.h | 19 ++++++-------------
4 files changed, 27 insertions(+), 51 deletions(-)
diff --git a/drivers/i2c/busses/i2c-cros-ec-tunnel.c b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
index 8e7a714..dd07818 100644
--- a/drivers/i2c/busses/i2c-cros-ec-tunnel.c
+++ b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
@@ -183,6 +183,7 @@ static int ec_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg i2c_msgs[],
u8 *request = NULL;
u8 *response = NULL;
int result;
+ struct cros_ec_command msg;
request_len = ec_i2c_count_message(i2c_msgs, num);
if (request_len < 0) {
@@ -218,9 +219,15 @@ static int ec_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg i2c_msgs[],
}
ec_i2c_construct_message(request, i2c_msgs, num, bus_num);
- result = bus->ec->command_sendrecv(bus->ec, EC_CMD_I2C_PASSTHRU,
- request, request_len,
- response, response_len);
+
+ msg.version = 0;
+ msg.command = EC_CMD_I2C_PASSTHRU;
+ msg.outdata = request;
+ msg.outsize = request_len;
+ msg.indata = response;
+ msg.insize = response_len;
+
+ result = bus->ec->cmd_xfer(bus->ec, &msg);
if (result)
goto exit;
@@ -258,7 +265,7 @@ static int ec_i2c_probe(struct platform_device *pdev)
u32 remote_bus;
int err;
- if (!ec->command_sendrecv) {
+ if (!ec->cmd_xfer) {
dev_err(dev, "Missing sendrecv\n");
return -EINVAL;
}
diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
index 4083796..b8341ab 100644
--- a/drivers/input/keyboard/cros_ec_keyb.c
+++ b/drivers/input/keyboard/cros_ec_keyb.c
@@ -191,8 +191,16 @@ static void cros_ec_keyb_close(struct input_dev *dev)
static int cros_ec_keyb_get_state(struct cros_ec_keyb *ckdev, uint8_t *kb_state)
{
- return ckdev->ec->command_recv(ckdev->ec, EC_CMD_MKBP_STATE,
- kb_state, ckdev->cols);
+ struct cros_ec_command msg = {
+ .version = 0,
+ .command = EC_CMD_MKBP_STATE,
+ .outdata = NULL,
+ .outsize = 0,
+ .indata = kb_state,
+ .insize = ckdev->cols,
+ };
+
+ return ckdev->ec->cmd_xfer(ckdev->ec, &msg);
}
static int cros_ec_keyb_work(struct notifier_block *nb,
diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
index 49ed8c3..4851ed2 100644
--- a/drivers/mfd/cros_ec.c
+++ b/drivers/mfd/cros_ec.c
@@ -44,34 +44,6 @@ int cros_ec_prepare_tx(struct cros_ec_device *ec_dev,
}
EXPORT_SYMBOL(cros_ec_prepare_tx);
-static int cros_ec_command_sendrecv(struct cros_ec_device *ec_dev,
- uint16_t cmd, void *out_buf, int out_len,
- void *in_buf, int in_len)
-{
- struct cros_ec_command msg;
-
- msg.version = cmd >> 8;
- msg.command = cmd & 0xff;
- msg.outdata = out_buf;
- msg.outsize = out_len;
- msg.indata = in_buf;
- msg.insize = in_len;
-
- return ec_dev->cmd_xfer(ec_dev, &msg);
-}
-
-static int cros_ec_command_recv(struct cros_ec_device *ec_dev,
- uint16_t cmd, void *buf, int buf_len)
-{
- return cros_ec_command_sendrecv(ec_dev, cmd, NULL, 0, buf, buf_len);
-}
-
-static int cros_ec_command_send(struct cros_ec_device *ec_dev,
- uint16_t cmd, void *buf, int buf_len)
-{
- return cros_ec_command_sendrecv(ec_dev, cmd, buf, buf_len, NULL, 0);
-}
-
static irqreturn_t ec_irq_thread(int irq, void *data)
{
struct cros_ec_device *ec_dev = data;
@@ -104,10 +76,6 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
BLOCKING_INIT_NOTIFIER_HEAD(&ec_dev->event_notifier);
- ec_dev->command_send = cros_ec_command_send;
- ec_dev->command_recv = cros_ec_command_recv;
- ec_dev->command_sendrecv = cros_ec_command_sendrecv;
-
if (ec_dev->din_size) {
ec_dev->din = devm_kzalloc(dev, ec_dev->din_size, GFP_KERNEL);
if (!ec_dev->din)
diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
index 2b0c598..60c0880 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -63,9 +63,10 @@ struct cros_ec_command {
* @was_wake_device: true if this device was set to wake the system from
* sleep at the last suspend
* @event_notifier: interrupt event notifier for transport devices
- * @command_send: send a command
- * @command_recv: receive a response
- * @command_sendrecv: send a command and receive a response
+ * @cmd_xfer: send command to EC and get response
+ * Returns 0 if the communication succeeded, but that doesn't mean the EC
+ * was happy with the command it got. Caller should check msg.result for
+ * the EC's result code.
*
* @priv: Private data
* @irq: Interrupt to use
@@ -83,7 +84,6 @@ struct cros_ec_command {
* @parent: pointer to parent device (e.g. i2c or spi device)
* @wake_enabled: true if this device can wake the system from sleep
* @lock: one transaction at a time
- * @cmd_xfer: low-level channel to the EC
*/
struct cros_ec_device {
@@ -94,13 +94,8 @@ struct cros_ec_device {
bool was_wake_device;
struct class *cros_class;
struct blocking_notifier_head event_notifier;
- int (*command_send)(struct cros_ec_device *ec,
- uint16_t cmd, void *out_buf, int out_len);
- int (*command_recv)(struct cros_ec_device *ec,
- uint16_t cmd, void *in_buf, int in_len);
- int (*command_sendrecv)(struct cros_ec_device *ec,
- uint16_t cmd, void *out_buf, int out_len,
- void *in_buf, int in_len);
+ int (*cmd_xfer)(struct cros_ec_device *ec,
+ struct cros_ec_command *msg);
/* These are used to implement the platform-specific interface */
void *priv;
@@ -112,8 +107,6 @@ struct cros_ec_device {
struct device *parent;
bool wake_enabled;
struct mutex lock;
- int (*cmd_xfer)(struct cros_ec_device *ec,
- struct cros_ec_command *msg);
};
/**
--
2.0.0.526.g5318336
^ permalink raw reply related
* [PATCH v2 10/10] mfd: cros_ec: move EC interrupt to cros_ec_keyb
From: Doug Anderson @ 2014-06-18 18:14 UTC (permalink / raw)
To: Lee Jones
Cc: Andrew Bresticker, swarren, olof, Sonny Rao, linux-samsung-soc,
Javier Martinez Canillas, Bill Richardson, sjg, Wolfram Sang,
broonie, Doug Anderson, dmitry.torokhov, sameo, geert,
linux-input, linux-kernel
In-Reply-To: <1403115247-8853-1-git-send-email-dianders@chromium.org>
From: Andrew Bresticker <abrestic@chromium.org>
If we receive EC interrupts after the cros_ec driver has probed, but
before the cros_ec_keyb driver has probed, the cros_ec IRQ handler
will not run the cros_ec_keyb notifier and the EC will leave the IRQ
line asserted. The cros_ec IRQ handler then returns IRQ_HANDLED and
the resulting flood of interrupts causes the machine to hang.
Since the EC interrupt is currently only used for the keyboard, move
the setup and handling of the EC interrupt to the cros_ec_keyb driver.
Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
Signed-off-by: Doug Anderson <dianders@chromium.org>
---
Changes in v2:
- IRQs should be optional => move EC interrupt to keyboard.
drivers/input/keyboard/cros_ec_keyb.c | 58 ++++++++++++++++++++---------------
drivers/mfd/cros_ec.c | 35 +--------------------
include/linux/mfd/cros_ec.h | 2 --
3 files changed, 34 insertions(+), 61 deletions(-)
diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
index b8341ab..791781a 100644
--- a/drivers/input/keyboard/cros_ec_keyb.c
+++ b/drivers/input/keyboard/cros_ec_keyb.c
@@ -24,8 +24,8 @@
#include <linux/module.h>
#include <linux/i2c.h>
#include <linux/input.h>
+#include <linux/interrupt.h>
#include <linux/kernel.h>
-#include <linux/notifier.h>
#include <linux/platform_device.h>
#include <linux/slab.h>
#include <linux/input/matrix_keypad.h>
@@ -42,7 +42,6 @@
* @dev: Device pointer
* @idev: Input device
* @ec: Top level ChromeOS device to use to talk to EC
- * @event_notifier: interrupt event notifier for transport devices
*/
struct cros_ec_keyb {
unsigned int rows;
@@ -55,7 +54,6 @@ struct cros_ec_keyb {
struct device *dev;
struct input_dev *idev;
struct cros_ec_device *ec;
- struct notifier_block notifier;
};
@@ -173,22 +171,6 @@ static void cros_ec_keyb_process(struct cros_ec_keyb *ckdev,
input_sync(ckdev->idev);
}
-static int cros_ec_keyb_open(struct input_dev *dev)
-{
- struct cros_ec_keyb *ckdev = input_get_drvdata(dev);
-
- return blocking_notifier_chain_register(&ckdev->ec->event_notifier,
- &ckdev->notifier);
-}
-
-static void cros_ec_keyb_close(struct input_dev *dev)
-{
- struct cros_ec_keyb *ckdev = input_get_drvdata(dev);
-
- blocking_notifier_chain_unregister(&ckdev->ec->event_notifier,
- &ckdev->notifier);
-}
-
static int cros_ec_keyb_get_state(struct cros_ec_keyb *ckdev, uint8_t *kb_state)
{
struct cros_ec_command msg = {
@@ -203,19 +185,41 @@ static int cros_ec_keyb_get_state(struct cros_ec_keyb *ckdev, uint8_t *kb_state)
return ckdev->ec->cmd_xfer(ckdev->ec, &msg);
}
-static int cros_ec_keyb_work(struct notifier_block *nb,
- unsigned long state, void *_notify)
+static irqreturn_t cros_ec_keyb_irq(int irq, void *data)
{
+ struct cros_ec_keyb *ckdev = data;
+ struct cros_ec_device *ec = ckdev->ec;
int ret;
- struct cros_ec_keyb *ckdev = container_of(nb, struct cros_ec_keyb,
- notifier);
uint8_t kb_state[ckdev->cols];
+ if (device_may_wakeup(ec->dev))
+ pm_wakeup_event(ec->dev, 0);
+
ret = cros_ec_keyb_get_state(ckdev, kb_state);
if (ret >= 0)
cros_ec_keyb_process(ckdev, kb_state, ret);
+ else
+ dev_err(ec->dev, "failed to get keyboard state: %d\n", ret);
- return NOTIFY_DONE;
+ return IRQ_HANDLED;
+}
+
+static int cros_ec_keyb_open(struct input_dev *dev)
+{
+ struct cros_ec_keyb *ckdev = input_get_drvdata(dev);
+ struct cros_ec_device *ec = ckdev->ec;
+
+ return request_threaded_irq(ec->irq, NULL, cros_ec_keyb_irq,
+ IRQF_TRIGGER_LOW | IRQF_ONESHOT,
+ "cros_ec_keyb", ckdev);
+}
+
+static void cros_ec_keyb_close(struct input_dev *dev)
+{
+ struct cros_ec_keyb *ckdev = input_get_drvdata(dev);
+ struct cros_ec_device *ec = ckdev->ec;
+
+ free_irq(ec->irq, ckdev);
}
static int cros_ec_keyb_probe(struct platform_device *pdev)
@@ -246,8 +250,12 @@ static int cros_ec_keyb_probe(struct platform_device *pdev)
if (!idev)
return -ENOMEM;
+ if (!ec->irq) {
+ dev_err(dev, "no EC IRQ specified\n");
+ return -EINVAL;
+ }
+
ckdev->ec = ec;
- ckdev->notifier.notifier_call = cros_ec_keyb_work;
ckdev->dev = dev;
dev_set_drvdata(&pdev->dev, ckdev);
diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
index 83e30c6..4873f9c 100644
--- a/drivers/mfd/cros_ec.c
+++ b/drivers/mfd/cros_ec.c
@@ -62,18 +62,6 @@ int cros_ec_check_result(struct cros_ec_device *ec_dev,
}
EXPORT_SYMBOL(cros_ec_check_result);
-static irqreturn_t ec_irq_thread(int irq, void *data)
-{
- struct cros_ec_device *ec_dev = data;
-
- if (device_may_wakeup(ec_dev->dev))
- pm_wakeup_event(ec_dev->dev, 0);
-
- blocking_notifier_call_chain(&ec_dev->event_notifier, 1, ec_dev);
-
- return IRQ_HANDLED;
-}
-
static const struct mfd_cell cros_devs[] = {
{
.name = "cros-ec-keyb",
@@ -92,8 +80,6 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
struct device *dev = ec_dev->dev;
int err = 0;
- BLOCKING_INIT_NOTIFIER_HEAD(&ec_dev->event_notifier);
-
if (ec_dev->din_size) {
ec_dev->din = devm_kzalloc(dev, ec_dev->din_size, GFP_KERNEL);
if (!ec_dev->din)
@@ -105,42 +91,23 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
return -ENOMEM;
}
- if (!ec_dev->irq) {
- dev_dbg(dev, "no valid IRQ: %d\n", ec_dev->irq);
- return err;
- }
-
- err = request_threaded_irq(ec_dev->irq, NULL, ec_irq_thread,
- IRQF_TRIGGER_LOW | IRQF_ONESHOT,
- "chromeos-ec", ec_dev);
- if (err) {
- dev_err(dev, "request irq %d: error %d\n", ec_dev->irq, err);
- return err;
- }
-
err = mfd_add_devices(dev, 0, cros_devs,
ARRAY_SIZE(cros_devs),
NULL, ec_dev->irq, NULL);
if (err) {
dev_err(dev, "failed to add mfd devices\n");
- goto fail_mfd;
+ return err;
}
dev_info(dev, "Chrome EC device registered\n");
return 0;
-
-fail_mfd:
- free_irq(ec_dev->irq, ec_dev);
-
- return err;
}
EXPORT_SYMBOL(cros_ec_register);
int cros_ec_remove(struct cros_ec_device *ec_dev)
{
mfd_remove_devices(ec_dev->dev);
- free_irq(ec_dev->irq, ec_dev);
return 0;
}
diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
index 0ebf26f..fcbe9d1 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -62,7 +62,6 @@ struct cros_ec_command {
* @dev: Device pointer
* @was_wake_device: true if this device was set to wake the system from
* sleep at the last suspend
- * @event_notifier: interrupt event notifier for transport devices
* @cmd_xfer: send command to EC and get response
* Returns the number of bytes received if the communication succeeded, but
* that doesn't mean the EC was happy with the command. The caller
@@ -93,7 +92,6 @@ struct cros_ec_device {
struct device *dev;
bool was_wake_device;
struct class *cros_class;
- struct blocking_notifier_head event_notifier;
int (*cmd_xfer)(struct cros_ec_device *ec,
struct cros_ec_command *msg);
--
2.0.0.526.g5318336
^ permalink raw reply related
* Re: [PATCH v9] leds: USB: HID: Add support for MSI GT683R led panels
From: Janne Kanniainen @ 2014-06-18 18:41 UTC (permalink / raw)
To: Johan Hovold
Cc: Jiri Kosina, Bryan Wu, linux-kernel, linux-leds, linux-usb,
linux-input
In-Reply-To: <20140618161150.GH31163@localhost>
>> This driver adds support for USB controlled led panels that exists in
>> MSI GT683R laptop
>>
>> Signed-off-by: Janne Kanniainen <janne.kanniainen@gmail.com>
>
> Reviewed-by: Johan Hovold <johan@kernel.org>
>
> Thanks, Janne!
>
> Johan
Thank you for reviewing my patch :) I sure learnt a
lot from you and if I ever send patch in future, it is for sure much
easier (Maybe I get it trough in 8th time ;))
Janne
^ permalink raw reply
* Re: [PATCH v9] leds: USB: HID: Add support for MSI GT683R led panels
From: Johan Hovold @ 2014-06-18 18:46 UTC (permalink / raw)
To: Janne Kanniainen
Cc: Johan Hovold, Jiri Kosina, Bryan Wu, linux-kernel, linux-leds,
linux-usb, linux-input
In-Reply-To: <CAEEjW05Hv_9ABMG+KnGmT9tUmM4McTMyFQDFkq6ad5iLmF2LaQ@mail.gmail.com>
On Wed, Jun 18, 2014 at 09:41:35PM +0300, Janne Kanniainen wrote:
> >> This driver adds support for USB controlled led panels that exists in
> >> MSI GT683R laptop
> >>
> >> Signed-off-by: Janne Kanniainen <janne.kanniainen@gmail.com>
> >
> > Reviewed-by: Johan Hovold <johan@kernel.org>
> >
> > Thanks, Janne!
> >
> > Johan
>
> Thank you for reviewing my patch :) I sure learnt a
> lot from you and if I ever send patch in future, it is for sure much
> easier (Maybe I get it trough in 8th time ;))
My pleasure. It's indeed a very good first patch you have created.
Good luck with the next one! ;)
Johan
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox