* [PATCH] HID: uhid: allocate static minor
From: David Herrmann @ 2013-09-09 16:33 UTC (permalink / raw)
To: linux-input
Cc: Tom Gundersen, Kay Sievers, Marcel Holtmann, linux-kernel,
David Herrmann
udev has this nice feature of creating "dead" /dev/<node> device-nodes if
it finds a devnode:<node> modalias. Once the node is accessed, the kernel
automatically loads the module that provides the node. However, this
requires udev to know the major:minor code to use for the node. This
feature was introduced by:
commit 578454ff7eab61d13a26b568f99a89a2c9edc881
Author: Kay Sievers <kay.sievers@vrfy.org>
Date: Thu May 20 18:07:20 2010 +0200
driver core: add devname module aliases to allow module on-demand auto-loading
However, uhid uses dynamic minor numbers so this doesn't actually work. We
need to load uhid to know which minor it's going to use.
Hence, allocate a static minor (just like uinput does) and we're good
to go.
Reported-by: Tom Gundersen <teg@jklm.no>
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
drivers/hid/uhid.c | 3 ++-
include/linux/miscdevice.h | 1 +
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/uhid.c b/drivers/hid/uhid.c
index 5bf2fb7..93b00d7 100644
--- a/drivers/hid/uhid.c
+++ b/drivers/hid/uhid.c
@@ -615,7 +615,7 @@ static const struct file_operations uhid_fops = {
static struct miscdevice uhid_misc = {
.fops = &uhid_fops,
- .minor = MISC_DYNAMIC_MINOR,
+ .minor = UHID_MINOR,
.name = UHID_NAME,
};
@@ -634,4 +634,5 @@ module_exit(uhid_exit);
MODULE_LICENSE("GPL");
MODULE_AUTHOR("David Herrmann <dh.herrmann@gmail.com>");
MODULE_DESCRIPTION("User-space I/O driver support for HID subsystem");
+MODULE_ALIAS_MISCDEV(UHID_MINOR);
MODULE_ALIAS("devname:" UHID_NAME);
diff --git a/include/linux/miscdevice.h b/include/linux/miscdevice.h
index 09c2300..cb35835 100644
--- a/include/linux/miscdevice.h
+++ b/include/linux/miscdevice.h
@@ -45,6 +45,7 @@
#define MAPPER_CTRL_MINOR 236
#define LOOP_CTRL_MINOR 237
#define VHOST_NET_MINOR 238
+#define UHID_MINOR 239
#define MISC_DYNAMIC_MINOR 255
struct device;
--
1.8.4
^ permalink raw reply related
* Re: [BUG?] HID: uhid: add devname module alias
From: David Herrmann @ 2013-09-09 16:38 UTC (permalink / raw)
To: Tom Gundersen
Cc: Marcel Holtmann, Jiri Kosina, Kay Sievers,
linux-input@vger.kernel.org, LKML
In-Reply-To: <CAG-2HqXVR5rFCa0XGmFpJcPWgPBKKaELgmFxhWKxxOYQRc-m2A@mail.gmail.com>
Hi Tom
On Mon, Sep 9, 2013 at 5:43 PM, Tom Gundersen <teg@jklm.no> wrote:
> Hi Marcel,
>
> The above commit (60cbd53 in mainline) doesn't appear to work for me.
> I.e., depmod does not create an entry in modules.devname and hence no
> device node is created on boot.
>
> If I understand correctly, you'd also need to create the correct
> "char-major-<MAJOR>-<MINOR>" alias (which I don't think you can do as
> long as you are using MISC_DYNAMIC_MINOR).
>
> Or am I missing something?
Nope, that's correct as discussed on IRC. Patch pending as:
Subject: [PATCH] HID: uhid: allocate static minor
Message-Id: <1378744434-10883-1-git-send-email-dh.herrmann@gmail.com>
Thanks
David
^ permalink raw reply
* Re: [PATCH] HID: uhid: allocate static minor
From: Tom Gundersen @ 2013-09-09 16:51 UTC (permalink / raw)
To: David Herrmann
Cc: linux-input@vger.kernel.org, Kay Sievers, Marcel Holtmann, LKML
In-Reply-To: <1378744434-10883-1-git-send-email-dh.herrmann@gmail.com>
On Mon, Sep 9, 2013 at 6:33 PM, David Herrmann <dh.herrmann@gmail.com> wrote:
> udev has this nice feature of creating "dead" /dev/<node> device-nodes if
> it finds a devnode:<node> modalias. Once the node is accessed, the kernel
> automatically loads the module that provides the node. However, this
> requires udev to know the major:minor code to use for the node. This
> feature was introduced by:
>
> commit 578454ff7eab61d13a26b568f99a89a2c9edc881
> Author: Kay Sievers <kay.sievers@vrfy.org>
> Date: Thu May 20 18:07:20 2010 +0200
>
> driver core: add devname module aliases to allow module on-demand auto-loading
>
> However, uhid uses dynamic minor numbers so this doesn't actually work. We
> need to load uhid to know which minor it's going to use.
>
> Hence, allocate a static minor (just like uinput does) and we're good
> to go.
>
> Reported-by: Tom Gundersen <teg@jklm.no>
Thanks, this works (I only tested the creation of the device node, not
the actual module loading).
> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
> ---
> drivers/hid/uhid.c | 3 ++-
> include/linux/miscdevice.h | 1 +
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hid/uhid.c b/drivers/hid/uhid.c
> index 5bf2fb7..93b00d7 100644
> --- a/drivers/hid/uhid.c
> +++ b/drivers/hid/uhid.c
> @@ -615,7 +615,7 @@ static const struct file_operations uhid_fops = {
>
> static struct miscdevice uhid_misc = {
> .fops = &uhid_fops,
> - .minor = MISC_DYNAMIC_MINOR,
> + .minor = UHID_MINOR,
> .name = UHID_NAME,
> };
>
> @@ -634,4 +634,5 @@ module_exit(uhid_exit);
> MODULE_LICENSE("GPL");
> MODULE_AUTHOR("David Herrmann <dh.herrmann@gmail.com>");
> MODULE_DESCRIPTION("User-space I/O driver support for HID subsystem");
> +MODULE_ALIAS_MISCDEV(UHID_MINOR);
> MODULE_ALIAS("devname:" UHID_NAME);
> diff --git a/include/linux/miscdevice.h b/include/linux/miscdevice.h
> index 09c2300..cb35835 100644
> --- a/include/linux/miscdevice.h
> +++ b/include/linux/miscdevice.h
> @@ -45,6 +45,7 @@
> #define MAPPER_CTRL_MINOR 236
> #define LOOP_CTRL_MINOR 237
> #define VHOST_NET_MINOR 238
> +#define UHID_MINOR 239
> #define MISC_DYNAMIC_MINOR 255
>
> struct device;
> --
> 1.8.4
>
^ permalink raw reply
* Re: [PATCH v2 0/7] HID: validate report details
From: Kees Cook @ 2013-09-09 21:48 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: linux-input, Benjamin Tissoires, Jiri Kosina, Henrik Rydberg
In-Reply-To: <CAN+gG=HrA7LcAS=kyVHjK_TPo83kd80aZJfv78CJ+01a+UFKDA@mail.gmail.com>
On Mon, Sep 9, 2013 at 6:48 AM, Benjamin Tissoires
<benjamin.tissoires@gmail.com> wrote:
> On Wed, Sep 4, 2013 at 6:37 PM, Kees Cook <keescook@chromium.org> wrote:
>> These patches introduce a validation function for HID devices that do
>> direct report value accesses, solving a number of heap smashing flaws.
>>
>> This version changes to using an field-index-based checker for the new
>> "hid_validate_values()" which requires callers to loop across fields if
>> they use more than one field.
>
> I am globally happy with the patch series.
> I have some concerns about patches 4 6 and 7, but the other can be
> applied right now.
>
> Kees, if you want to switch to something else, I can handle the v3 for
> these three patches: I have some logitech-dj devices and a tester for
> the lenovo one.
That would be fantastic, thank you. I don't have any tools to test
these in the "expected" case. :)
-Kees
--
Kees Cook
Chrome OS Security
^ permalink raw reply
* [PATCH] Input: synaptics_usb - Scale stick axes evenly
From: Andrew Deason @ 2013-09-10 3:30 UTC (permalink / raw)
To: linux-input
Currently, we report relative x and y coordinates when receiving data
from the pointing stick. We try to divide each value by 16, since the
raw values are very large, and send the pointer flying across the
screen. Currently we try to do this by shifting the relative
coordinates for the pointing stick over by 4 bits by doing, e.g.:
x = (s16)(be16_to_cpup((__be16 *)&synusb->data[2]) << 3) >> 7;
The raw value from usb is 13-bits wide, so I assume that we shift left
by 3 then right by 7 in order to effectively shift right by 4 while
preserving sign.
However, this is inconsistent with different directions. For example,
an x value of 1 would indicate movement to the right by 1 unit, and
shifting right by 4 reduces this value to 0. But a value of -1
(movement left by 1 unit) is 0x1fff, and running this through the
above code yields a result of -1, not 0.
The result of this is that very tiny movements in the right and up
directions result in no movement, but very tiny movements in the left
and down directions do result in movement, resulting in a confusing
user experience.
So instead of all of this, just figure out what the raw value is, and
divide it by 16 explicitly.
Signed-off-by: Andrew Deason <adeason@dson.org>
---
To be clear, I'm still finishing a larger change to how we report stick
movement, per
<http://permalink.gmane.org/gmane.linux.kernel.input/31777>. But in the
meantime, this simple change removes one of the more aggravating
characteristics of the existing code.
---
drivers/input/mouse/synaptics_usb.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/input/mouse/synaptics_usb.c b/drivers/input/mouse/synaptics_usb.c
index 64cf34e..2c4a3c1 100644
--- a/drivers/input/mouse/synaptics_usb.c
+++ b/drivers/input/mouse/synaptics_usb.c
@@ -108,8 +108,10 @@ static void synusb_report_stick(struct synusb *synusb)
unsigned int pressure;
pressure = synusb->data[6];
- x = (s16)(be16_to_cpup((__be16 *)&synusb->data[2]) << 3) >> 7;
- y = (s16)(be16_to_cpup((__be16 *)&synusb->data[4]) << 3) >> 7;
+ x = (s16)(be16_to_cpup((__be16 *)&synusb->data[2]) << 3) >> 3;
+ y = (s16)(be16_to_cpup((__be16 *)&synusb->data[4]) << 3) >> 3;
+ x /= 16;
+ y /= 16;
if (pressure > 0) {
input_report_rel(input_dev, REL_X, x);
--
1.8.4.rc3
^ permalink raw reply related
* Webmail Account Warning.
From: Administrator:: Account Warning. @ 2013-09-10 10:54 UTC (permalink / raw)
Administrator Webmail Account Warning.
This mail is from All Administrator Webmail Account; we wish to bring to
your notice the Condition of your email account.
We have just noticed that you have exceeded your email Database limit of
500 MB quota and your email IP is causing conflict because it is been
accessed in different server location. You need to Upgrade and expand your
email quota limit before you can continue to use your email.
Update your email quota limit to 2.6 GB, use the below web link:
http://accountwarning.yolasite.com/contact-us.php
Failure to do this will result to email deactivation within 24 hours
Thank you for your understanding.
Copyright 2013 Help Desk
Administrator Webmail Support.
^ permalink raw reply
* Re: [PATCH 4/7] HID: steelseries: validate output report details
From: Benjamin Tissoires @ 2013-09-10 11:45 UTC (permalink / raw)
To: Kees Cook; +Cc: linux-input, Benjamin Tissoires, Jiri Kosina, Henrik Rydberg
In-Reply-To: <CAN+gG=H6Pib-kzWKPH_Hj_vcw7q_agHZ7_Qi-pEde9tbTB4EkA@mail.gmail.com>
On Mon, Sep 9, 2013 at 3:02 PM, Benjamin Tissoires
<benjamin.tissoires@gmail.com> wrote:
> On Wed, Sep 4, 2013 at 6:37 PM, Kees Cook <keescook@chromium.org> wrote:
>> A HID device could send a malicious output report that would cause the
>> steelseries HID driver to write beyond the output report allocation
>> during initialization, causing a heap overflow:
>>
>> [ 167.981534] usb 1-1: New USB device found, idVendor=1038, idProduct=1410
>> ...
>> [ 182.050547] BUG kmalloc-256 (Tainted: G W ): Redzone overwritten
>>
>> CVE-2013-2891
>>
>> Signed-off-by: Kees Cook <keescook@chromium.org>
>> Cc: stable@kernel.org
>> ---
>> drivers/hid/hid-steelseries.c | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/hid/hid-steelseries.c b/drivers/hid/hid-steelseries.c
>> index d164911..29f328f 100644
>> --- a/drivers/hid/hid-steelseries.c
>> +++ b/drivers/hid/hid-steelseries.c
>> @@ -249,6 +249,11 @@ static int steelseries_srws1_probe(struct hid_device *hdev,
>> goto err_free;
>> }
>>
>> + if (!hid_validate_values(hdev, HID_OUTPUT_REPORT, 0, 0, 16)) {
>> + ret = -ENODEV;
>> + goto err_free;
>
> There is a problem here in case of a failure:
> hid_parse() allocates a lot of memory and sets the flag
> HID_STAT_PARSED, but we are leaving the probe() without clearing all
> this stuff.
> In hid_parse(), when a failure is detected, we call
> hid_close_report(), so we also should close the report in the same way
> here (but hid_close_report() is a static function).
>
Sorry, my bad. hid_close_report() is called upon ->probe() failure. So
this is perfectly good.
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cheers,
Benjamin
^ permalink raw reply
* 答复: [PATCH 2/2] Input: ALPS Touchpad- Improve the performance of alps v5-protocol's touchpad
From: yunkang.tang @ 2013-09-10 11:50 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input@vger.kernel.org
In-Reply-To: <20130909162734.GC25107@core.coreip.homeip.net>
Hi Dmitry,
>Please do not separate changes in header files form changes in .c files
>because patches need to be complete. If I were to apply this patch alone
>it would break the compilation.
Thanks for your expanation. I got that.
Thanks
Best Regards
------------------------------------------------------
Tommy
On Thu, Sep 05, 2013 at 05:50:55AM +0000, yunkang.tang@cn.alps.com wrote:
> <Change list>
> - Add the macro definition for v5 device.
>
> --- linux-3.11/drivers/input/mouse/alps.h.orig 2013-09-04 19:51:33.837135870 +0800
> +++ linux-3.11/drivers/input/mouse/alps.h 2013-09-05 21:01:57.074314890 +0800
> @@ -18,6 +18,10 @@
> #define ALPS_PROTO_V4 4
> #define ALPS_PROTO_V5 5
>
> +#define DOLPHIN_COUNT_PER_ELECTRODE 64
> +#define DOLPHIN_PROFILE_XOFFSET 8 /* [DOLPHIN] The number of the x-electrode offset value */
> +#define DOLPHIN_PROFILE_YOFFSET 1 /* [DOLPHIN] The number of the y-electrode offset value */
> +
> /**
> * struct alps_model_info - touchpad ID table
> * @signature: E7 response string to match.
> @@ -69,7 +73,7 @@ struct alps_nibble_commands {
> * @y: Y position for ST.
> * @z: Z position for ST.
> * @first_mp: Packet is the first of a multi-packet report.
> - * @is_mp: Packet is part of a multi-packet report.
> + * @is_mp: Packet is the last of a multi-packet report.
> * @left: Left touchpad button is active.
> * @right: Right touchpad button is active.
> * @middle: Middle touchpad button is active.
> @@ -145,7 +149,7 @@ struct alps_data {
>
> int (*hw_init)(struct psmouse *psmouse);
> void (*process_packet)(struct psmouse *psmouse);
> - void (*decode_fields)(struct alps_fields *f, unsigned char *p);
> + void (*decode_fields)(struct alps_fields *f, unsigned char *p, struct psmouse *psmouse);
> void (*set_abs_params)(struct alps_data *priv, struct input_dev *dev1);
>
> int prev_fin;
Please do not separate changes in header files form changes in .c files
because patches need to be complete. If I were to apply this patch alone
it would break the compilation.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: Atmel updates to atmel_mxt_ts touch controller driver - v6
From: Nick Dyer @ 2013-09-10 13:58 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: rydberg, Daniel Kurtz, Joonyoung Shim, Alan Bowens, linux-input,
linux-kernel, Peter Meerwald, Benson Leung, Olof Johansson
In-Reply-To: <20130815160734.GB12196@core.coreip.homeip.net>
Dmitry Torokhov wrote:
> On Thu, Aug 15, 2013 at 04:55:57PM +0100, Nick Dyer wrote:
>> rydberg@euromail.se wrote:
>>> First: thanks for the patches and you work on this driver.
>>
>> Thank you for your time in looking at these changes.
>>
>>> Now, I don't swear much, but I would like to emphasize line 161 of
>>> Documentation/SubmittingPatches:
>>>
>>> **Do not send more than 15 patches at once to the vger mailing lists!!!***
>>>
>>> One reason that should be obvious by now is that your work will be
>>> attended to much quicker. One may think that it is more efficient to
>>> send the whole backlog at once, but in fact, the time it takes to get
>>> a patchset accepted is inversely proportional to the length of the
>>> patchset. So please, keep it small and simple next time,
>>
>> Apologies. I will split this lot up into several smaller series of patches.
>
> Wait, I am in the process of applying it actually...
I wonder, have you made any progress with this? Is there any way I can assist?
Thanks
--
Nick Dyer
^ permalink raw reply
* [PATCH 3.12-rc1] USB: input: cm109.c: Convert high volume dev_err() to dev_err_ratelimited()
From: Tim Gardner @ 2013-09-10 16:23 UTC (permalink / raw)
To: linux-input, linux-kernel; +Cc: Tim Gardner, Dmitry Torokhov
BugLink: http://bugs.launchpad.net/bugs/1222850
This input device can get into a state that produces a high
volume of device status errors. Attempt to throttle these
error messages such that the kernel log is not flooded.
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
---
drivers/input/misc/cm109.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/drivers/input/misc/cm109.c b/drivers/input/misc/cm109.c
index 082684e..8ec72bc 100644
--- a/drivers/input/misc/cm109.c
+++ b/drivers/input/misc/cm109.c
@@ -351,7 +351,8 @@ static void cm109_urb_irq_callback(struct urb *urb)
if (status) {
if (status == -ESHUTDOWN)
return;
- dev_err(&dev->intf->dev, "%s: urb status %d\n", __func__, status);
+ dev_err_ratelimited(&dev->intf->dev, "%s: urb status %d\n",
+ __func__, status);
}
/* Special keys */
@@ -419,7 +420,8 @@ static void cm109_urb_ctl_callback(struct urb *urb)
dev->ctl_data->byte[3]);
if (status)
- dev_err(&dev->intf->dev, "%s: urb status %d\n", __func__, status);
+ dev_err_ratelimited(&dev->intf->dev, "%s: urb status %d\n",
+ __func__, status);
spin_lock(&dev->ctl_submit_lock);
@@ -436,7 +438,7 @@ static void cm109_urb_ctl_callback(struct urb *urb)
dev->irq_urb_pending = 1;
error = usb_submit_urb(dev->urb_irq, GFP_ATOMIC);
if (error)
- dev_err(&dev->intf->dev,
+ dev_err_ratelimited(&dev->intf->dev,
"%s: usb_submit_urb (urb_irq) failed %d\n",
__func__, error);
}
@@ -480,7 +482,7 @@ static void cm109_toggle_buzzer_sync(struct cm109_dev *dev, int on)
dev->ctl_data,
USB_PKT_LEN, USB_CTRL_SET_TIMEOUT);
if (error < 0 && error != -EINTR)
- dev_err(&dev->intf->dev, "%s: usb_control_msg() failed %d\n",
+ dev_err_ratelimited(&dev->intf->dev, "%s: usb_control_msg() failed %d\n",
__func__, error);
}
@@ -542,8 +544,9 @@ static int cm109_input_open(struct input_dev *idev)
error = usb_submit_urb(dev->urb_ctl, GFP_KERNEL);
if (error)
- dev_err(&dev->intf->dev, "%s: usb_submit_urb (urb_ctl) failed %d\n",
- __func__, error);
+ dev_err_ratelimited(&dev->intf->dev,
+ "%s: usb_submit_urb (urb_ctl) failed %d\n", __func__,
+ error);
else
dev->open = 1;
@@ -716,7 +719,7 @@ static int cm109_usb_probe(struct usb_interface *intf,
pipe = usb_rcvintpipe(udev, endpoint->bEndpointAddress);
ret = usb_maxpacket(udev, pipe, usb_pipeout(pipe));
if (ret != USB_PKT_LEN)
- dev_err(&intf->dev, "invalid payload size %d, expected %d\n",
+ dev_err_ratelimited(&intf->dev, "invalid payload size %d, expected %d\n",
ret, USB_PKT_LEN);
/* initialise irq urb */
--
1.7.9.5
^ permalink raw reply related
* [PATCH 1/3] HID: Delay opening HID device
From: Srinivas Pandruvada @ 2013-09-10 20:03 UTC (permalink / raw)
To: linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-iio-u79uwXL29TY76Z2rM5mHXA
Cc: jc23-KWPb1pKIrIJaa/9Udqfwiw, jkosina-AlSwsSmVLrQ,
holler-SXC+2es9fhnfWeYVQQPykw, Srinivas Pandruvada
Don't call hid_open_device till there is actually an user. This saves
power by not opening underlying transport for HID. Also close device
if there are no active mfd client using HID sensor hub.
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
drivers/hid/hid-sensor-hub.c | 45 ++++++++++++++++++++++++++++++++----------
include/linux/hid-sensor-hub.h | 18 +++++++++++++++++
2 files changed, 53 insertions(+), 10 deletions(-)
diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
index 10e1581..8effdee 100644
--- a/drivers/hid/hid-sensor-hub.c
+++ b/drivers/hid/hid-sensor-hub.c
@@ -465,6 +465,39 @@ static int sensor_hub_raw_event(struct hid_device *hdev,
return 1;
}
+int sensor_hub_device_open(struct hid_sensor_hub_device *hsdev)
+{
+ int ret;
+ struct sensor_hub_data *data = hid_get_drvdata(hsdev->hdev);
+
+ mutex_lock(&data->mutex);
+ if (!hsdev->ref_cnt) {
+ ret = hid_hw_open(hsdev->hdev);
+ if (ret) {
+ hid_err(hsdev->hdev, "failed to open hid device\n");
+ mutex_unlock(&data->mutex);
+ return ret;
+ }
+ }
+ hsdev->ref_cnt++;
+ mutex_unlock(&data->mutex);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(sensor_hub_device_open);
+
+void sensor_hub_device_close(struct hid_sensor_hub_device *hsdev)
+{
+ struct sensor_hub_data *data = hid_get_drvdata(hsdev->hdev);
+
+ mutex_lock(&data->mutex);
+ hsdev->ref_cnt--;
+ if (!hsdev->ref_cnt)
+ hid_hw_close(hsdev->hdev);
+ mutex_unlock(&data->mutex);
+}
+EXPORT_SYMBOL_GPL(sensor_hub_device_close);
+
static int sensor_hub_probe(struct hid_device *hdev,
const struct hid_device_id *id)
{
@@ -506,12 +539,6 @@ static int sensor_hub_probe(struct hid_device *hdev,
hid_err(hdev, "hw start failed\n");
return ret;
}
- ret = hid_hw_open(hdev);
- if (ret) {
- hid_err(hdev, "failed to open input interrupt pipe\n");
- goto err_stop_hw;
- }
-
INIT_LIST_HEAD(&sd->dyn_callback_list);
sd->hid_sensor_client_cnt = 0;
report_enum = &hdev->report_enum[HID_INPUT_REPORT];
@@ -520,7 +547,7 @@ static int sensor_hub_probe(struct hid_device *hdev,
if (dev_cnt > HID_MAX_PHY_DEVICES) {
hid_err(hdev, "Invalid Physical device count\n");
ret = -EINVAL;
- goto err_close;
+ goto err_stop_hw;
}
sd->hid_sensor_hub_client_devs = kzalloc(dev_cnt *
sizeof(struct mfd_cell),
@@ -528,7 +555,7 @@ static int sensor_hub_probe(struct hid_device *hdev,
if (sd->hid_sensor_hub_client_devs == NULL) {
hid_err(hdev, "Failed to allocate memory for mfd cells\n");
ret = -ENOMEM;
- goto err_close;
+ goto err_stop_hw;
}
list_for_each_entry(report, &report_enum->report_list, list) {
hid_dbg(hdev, "Report id:%x\n", report->id);
@@ -565,8 +592,6 @@ err_free_names:
for (i = 0; i < sd->hid_sensor_client_cnt ; ++i)
kfree(sd->hid_sensor_hub_client_devs[i].name);
kfree(sd->hid_sensor_hub_client_devs);
-err_close:
- hid_hw_close(hdev);
err_stop_hw:
hid_hw_stop(hdev);
diff --git a/include/linux/hid-sensor-hub.h b/include/linux/hid-sensor-hub.h
index 32ba451..a265af2 100644
--- a/include/linux/hid-sensor-hub.h
+++ b/include/linux/hid-sensor-hub.h
@@ -47,11 +47,13 @@ struct hid_sensor_hub_attribute_info {
* @hdev: Stores the hid instance.
* @vendor_id: Vendor id of hub device.
* @product_id: Product id of hub device.
+ * @ref_cnt: Number of MFD clients have opened this device
*/
struct hid_sensor_hub_device {
struct hid_device *hdev;
u32 vendor_id;
u32 product_id;
+ int ref_cnt;
};
/**
@@ -74,6 +76,22 @@ struct hid_sensor_hub_callbacks {
void *priv);
};
+/**
+* sensor_hub_device_open() - Open hub device
+* @hsdev: Hub device instance.
+*
+* Used to open hid device for sensor hub.
+*/
+int sensor_hub_device_open(struct hid_sensor_hub_device *hsdev);
+
+/**
+* sensor_hub_device_clode() - Close hub device
+* @hsdev: Hub device instance.
+*
+* Used to clode hid device for sensor hub.
+*/
+void sensor_hub_device_close(struct hid_sensor_hub_device *hsdev);
+
/* Registration functions */
/**
--
1.8.3.1
^ permalink raw reply related
* [PATCH 2/3] IIO: call sensor hub open close function
From: Srinivas Pandruvada @ 2013-09-10 20:03 UTC (permalink / raw)
To: linux-input, linux-iio; +Cc: jc23, jkosina, holler, Srinivas Pandruvada
In-Reply-To: <1378843432-5113-1-git-send-email-srinivas.pandruvada@linux.intel.com>
Call hid_sensor_hub_device_open when user space opens device and call
hid_sensor_hub_device_close when device is closed. This helps in
saving power.
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
drivers/iio/common/hid-sensors/hid-sensor-trigger.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
index 87419c4..b6e77e0 100644
--- a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
+++ b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
@@ -34,6 +34,12 @@ static int hid_sensor_data_rdy_trigger_set_state(struct iio_trigger *trig,
struct hid_sensor_common *st = iio_trigger_get_drvdata(trig);
int state_val;
+ if (state) {
+ if (sensor_hub_device_open(st->hsdev))
+ return -EIO;
+ } else
+ sensor_hub_device_close(st->hsdev);
+
state_val = state ? 1 : 0;
if (IS_ENABLED(CONFIG_HID_SENSOR_ENUM_BASE_QUIRKS))
++state_val;
--
1.8.3.1
^ permalink raw reply related
* [PATCH 3/3] HID RTC: Open sensor hub open close
From: Srinivas Pandruvada @ 2013-09-10 20:03 UTC (permalink / raw)
To: linux-input, linux-iio; +Cc: jc23, jkosina, holler, Srinivas Pandruvada
In-Reply-To: <1378843432-5113-1-git-send-email-srinivas.pandruvada@linux.intel.com>
Open sensor hub when module is loaded and close when module is removed.
This helps saving power by opening HID transport only when there is an
user.
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
drivers/rtc/rtc-hid-sensor-time.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/rtc/rtc-hid-sensor-time.c b/drivers/rtc/rtc-hid-sensor-time.c
index 7273b01..1fe170c 100644
--- a/drivers/rtc/rtc-hid-sensor-time.c
+++ b/drivers/rtc/rtc-hid-sensor-time.c
@@ -279,15 +279,28 @@ static int hid_time_probe(struct platform_device *pdev)
return ret;
}
+ ret = sensor_hub_device_open(hsdev);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to open sensor hub device!\n");
+ goto err_open;
+ }
+
time_state->rtc = devm_rtc_device_register(&pdev->dev,
"hid-sensor-time", &hid_time_rtc_ops,
THIS_MODULE);
if (IS_ERR(time_state->rtc)) {
dev_err(&pdev->dev, "rtc device register failed!\n");
- return PTR_ERR(time_state->rtc);
+ ret = PTR_ERR(time_state->rtc);
+ goto err_rtc;
}
+ return 0;
+
+err_rtc:
+ sensor_hub_device_close(hsdev);
+err_open:
+ sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_TIME);
return ret;
}
@@ -295,6 +308,7 @@ static int hid_time_remove(struct platform_device *pdev)
{
struct hid_sensor_hub_device *hsdev = pdev->dev.platform_data;
+ sensor_hub_device_close(hsdev);
sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_TIME);
return 0;
--
1.8.3.1
^ permalink raw reply related
* [PATCH v2] Input: wacom - add support for three new Intuos Pro devices
From: Ping Cheng @ 2013-09-10 21:57 UTC (permalink / raw)
To: linux-input; +Cc: dmitry.torokhov, chris, Ping Cheng
Acked-by: Chris Bagwell <chris@cnpbagwell.com>
Signed-off-by: Ping Cheng <pingc@wacom.com>
---
v2: Updated with Chris' comment (missed model specific bits in v1).
---
drivers/input/tablet/wacom_sys.c | 10 +++++--
drivers/input/tablet/wacom_wac.c | 58 ++++++++++++++++++++++++++++++----------
drivers/input/tablet/wacom_wac.h | 3 +++
3 files changed, 55 insertions(+), 16 deletions(-)
diff --git a/drivers/input/tablet/wacom_sys.c b/drivers/input/tablet/wacom_sys.c
index 79b69ea..cec3f9a 100644
--- a/drivers/input/tablet/wacom_sys.c
+++ b/drivers/input/tablet/wacom_sys.c
@@ -719,7 +719,7 @@ static int wacom_led_control(struct wacom *wacom)
return -ENOMEM;
if (wacom->wacom_wac.features.type >= INTUOS5S &&
- wacom->wacom_wac.features.type <= INTUOS5L) {
+ wacom->wacom_wac.features.type <= INTUOSPL) {
/*
* Touch Ring and crop mark LED luminance may take on
* one of four values:
@@ -981,6 +981,9 @@ static int wacom_initialize_leds(struct wacom *wacom)
case INTUOS5S:
case INTUOS5:
case INTUOS5L:
+ case INTUOSPS:
+ case INTUOSPM:
+ case INTUOSPL:
wacom->led.select[0] = 0;
wacom->led.select[1] = 0;
wacom->led.llv = 32;
@@ -1024,6 +1027,9 @@ static void wacom_destroy_leds(struct wacom *wacom)
case INTUOS5S:
case INTUOS5:
case INTUOS5L:
+ case INTUOSPS:
+ case INTUOSPM:
+ case INTUOSPL:
sysfs_remove_group(&wacom->intf->dev.kobj,
&intuos5_led_attr_group);
break;
@@ -1298,7 +1304,7 @@ static int wacom_probe(struct usb_interface *intf, const struct usb_device_id *i
* HID descriptor. If this is the touch interface (wMaxPacketSize
* of WACOM_PKGLEN_BBTOUCH3), override the table values.
*/
- if (features->type >= INTUOS5S && features->type <= INTUOS5L) {
+ if (features->type >= INTUOS5S && features->type <= INTUOSPL) {
if (endpoint->wMaxPacketSize == WACOM_PKGLEN_BBTOUCH3) {
features->device_type = BTN_TOOL_FINGER;
features->pktlen = WACOM_PKGLEN_BBTOUCH3;
diff --git a/drivers/input/tablet/wacom_wac.c b/drivers/input/tablet/wacom_wac.c
index 541197b..dc96274 100644
--- a/drivers/input/tablet/wacom_wac.c
+++ b/drivers/input/tablet/wacom_wac.c
@@ -619,14 +619,14 @@ static int wacom_intuos_irq(struct wacom_wac *wacom)
} else {
input_report_abs(input, ABS_MISC, 0);
}
- } else if (features->type >= INTUOS5S && features->type <= INTUOS5L) {
+ } else if (features->type >= INTUOS5S && features->type <= INTUOSPL) {
int i;
/* Touch ring mode switch has no capacitive sensor */
input_report_key(input, BTN_0, (data[3] & 0x01));
/*
- * ExpressKeys on Intuos5 have a capacitive sensor in
+ * ExpressKeys on Intuos5/Intuos Pro have a capacitive sensor in
* addition to the mechanical switch. Switch data is
* stored in data[4], capacitive data in data[5].
*/
@@ -714,7 +714,9 @@ static int wacom_intuos_irq(struct wacom_wac *wacom)
features->type == INTUOS4 ||
features->type == INTUOS4S ||
features->type == INTUOS5 ||
- features->type == INTUOS5S)) {
+ features->type == INTUOS5S ||
+ features->type == INTUOSPM ||
+ features->type == INTUOSPS)) {
return 0;
}
@@ -767,8 +769,7 @@ static int wacom_intuos_irq(struct wacom_wac *wacom)
} else if (wacom->tool[idx] == BTN_TOOL_MOUSE) {
/* I4 mouse */
- if ((features->type >= INTUOS4S && features->type <= INTUOS4L) ||
- (features->type >= INTUOS5S && features->type <= INTUOS5L)) {
+ if (features->type >= INTUOS4S && features->type <= INTUOSPL) {
input_report_key(input, BTN_LEFT, data[6] & 0x01);
input_report_key(input, BTN_MIDDLE, data[6] & 0x02);
input_report_key(input, BTN_RIGHT, data[6] & 0x04);
@@ -795,7 +796,8 @@ static int wacom_intuos_irq(struct wacom_wac *wacom)
}
}
} else if ((features->type < INTUOS3S || features->type == INTUOS3L ||
- features->type == INTUOS4L || features->type == INTUOS5L) &&
+ features->type == INTUOS4L || features->type == INTUOS5L ||
+ features->type == INTUOSPL) &&
wacom->tool[idx] == BTN_TOOL_LENS) {
/* Lens cursor packets */
input_report_key(input, BTN_LEFT, data[8] & 0x01);
@@ -1105,6 +1107,7 @@ static int wacom_bpt_touch(struct wacom_wac *wacom)
static void wacom_bpt3_touch_msg(struct wacom_wac *wacom, unsigned char *data)
{
+ struct wacom_features *features = &wacom->features;
struct input_dev *input = wacom->input;
bool touch = data[1] & 0x80;
int slot = input_mt_get_slot_by_key(input, data[0]);
@@ -1120,14 +1123,20 @@ static void wacom_bpt3_touch_msg(struct wacom_wac *wacom, unsigned char *data)
if (touch) {
int x = (data[2] << 4) | (data[4] >> 4);
int y = (data[3] << 4) | (data[4] & 0x0f);
- int a = data[5];
+ int width, height;
- // "a" is a scaled-down area which we assume is roughly
- // circular and which can be described as: a=(pi*r^2)/C.
- int x_res = input_abs_get_res(input, ABS_X);
- int y_res = input_abs_get_res(input, ABS_Y);
- int width = 2 * int_sqrt(a * WACOM_CONTACT_AREA_SCALE);
- int height = width * y_res / x_res;
+ if (features->type >= INTUOSPS && features->type <= INTUOSPL) {
+ width = data[5];
+ height = data[6];
+ } else {
+ int a = data[5];
+ // "a" is a scaled-down area which we assume is roughly
+ // circular and which can be described as: a=(pi*r^2)/C.
+ int x_res = input_abs_get_res(input, ABS_X);
+ int y_res = input_abs_get_res(input, ABS_Y);
+ width = 2 * int_sqrt(a * WACOM_CONTACT_AREA_SCALE);
+ height = width * y_res / x_res;
+ }
input_report_abs(input, ABS_MT_POSITION_X, x);
input_report_abs(input, ABS_MT_POSITION_Y, y);
@@ -1335,6 +1344,9 @@ void wacom_wac_irq(struct wacom_wac *wacom_wac, size_t len)
case INTUOS5S:
case INTUOS5:
case INTUOS5L:
+ case INTUOSPS:
+ case INTUOSPM:
+ case INTUOSPL:
if (len == WACOM_PKGLEN_BBTOUCH3)
sync = wacom_bpt3_touch(wacom_wac);
else
@@ -1418,7 +1430,7 @@ void wacom_setup_device_quirks(struct wacom_features *features)
/* these device have multiple inputs */
if (features->type >= WIRELESS ||
- (features->type >= INTUOS5S && features->type <= INTUOS5L) ||
+ (features->type >= INTUOS5S && features->type <= INTUOSPL) ||
(features->oVid && features->oPid))
features->quirks |= WACOM_QUIRK_MULTI_INPUT;
@@ -1625,6 +1637,8 @@ int wacom_setup_input_capabilities(struct input_dev *input_dev,
case INTUOS5:
case INTUOS5L:
+ case INTUOSPM:
+ case INTUOSPL:
if (features->device_type == BTN_TOOL_PEN) {
__set_bit(BTN_7, input_dev->keybit);
__set_bit(BTN_8, input_dev->keybit);
@@ -1632,6 +1646,7 @@ int wacom_setup_input_capabilities(struct input_dev *input_dev,
/* fall through */
case INTUOS5S:
+ case INTUOSPS:
__set_bit(INPUT_PROP_POINTER, input_dev->propbit);
if (features->device_type == BTN_TOOL_PEN) {
@@ -1950,6 +1965,18 @@ static const struct wacom_features wacom_features_0x29 =
static const struct wacom_features wacom_features_0x2A =
{ "Wacom Intuos5 M", WACOM_PKGLEN_INTUOS, 44704, 27940, 2047,
63, INTUOS5, WACOM_INTUOS3_RES, WACOM_INTUOS3_RES };
+static const struct wacom_features wacom_features_0x314 =
+ { "Wacom Intuos Pro S", WACOM_PKGLEN_INTUOS, 31496, 19685, 2047,
+ 63, INTUOSPS, WACOM_INTUOS3_RES, WACOM_INTUOS3_RES,
+ .touch_max = 16 };
+static const struct wacom_features wacom_features_0x315 =
+ { "Wacom Intuos Pro M", WACOM_PKGLEN_INTUOS, 44704, 27940, 2047,
+ 63, INTUOSPM, WACOM_INTUOS3_RES, WACOM_INTUOS3_RES,
+ .touch_max = 16 };
+static const struct wacom_features wacom_features_0x317 =
+ { "Wacom Intuos Pro L", WACOM_PKGLEN_INTUOS, 65024, 40640, 2047,
+ 63, INTUOSPL, WACOM_INTUOS3_RES, WACOM_INTUOS3_RES,
+ .touch_max = 16 };
static const struct wacom_features wacom_features_0xF4 =
{ "Wacom Cintiq 24HD", WACOM_PKGLEN_INTUOS, 104480, 65600, 2047,
63, WACOM_24HD, WACOM_INTUOS3_RES, WACOM_INTUOS3_RES };
@@ -2241,6 +2268,9 @@ const struct usb_device_id wacom_ids[] = {
{ USB_DEVICE_WACOM(0x101) },
{ USB_DEVICE_WACOM(0x10D) },
{ USB_DEVICE_WACOM(0x304) },
+ { USB_DEVICE_DETAILED(0x314, USB_CLASS_HID, 0, 0) },
+ { USB_DEVICE_DETAILED(0x315, USB_CLASS_HID, 0, 0) },
+ { USB_DEVICE_DETAILED(0x317, USB_CLASS_HID, 0, 0) },
{ USB_DEVICE_WACOM(0x4001) },
{ USB_DEVICE_WACOM(0x47) },
{ USB_DEVICE_WACOM(0xF4) },
diff --git a/drivers/input/tablet/wacom_wac.h b/drivers/input/tablet/wacom_wac.h
index dfc9e08..d6dec58 100644
--- a/drivers/input/tablet/wacom_wac.h
+++ b/drivers/input/tablet/wacom_wac.h
@@ -76,6 +76,9 @@ enum {
INTUOS5S,
INTUOS5,
INTUOS5L,
+ INTUOSPS,
+ INTUOSPM,
+ INTUOSPL,
WACOM_21UX2,
WACOM_22HD,
DTK,
--
1.8.1.2
^ permalink raw reply related
* [PATCH 2/2] uinput: Use input_inject_event in uinput_inject_event
From: Ryan Mallon @ 2013-09-10 23:32 UTC (permalink / raw)
To: dmitry.torokhov, rydberg; +Cc: carl, linux-input, linux-kernel, Ryan Mallon
In-Reply-To: <1378855973-11595-1-git-send-email-rmallon@gmail.com>
Call input_inject_event rather than input_event in uinput_inject_event. This
mirrors the behaviour of evdev_write. input_inject_event will ignore the
injected event if the uinput input device has been grabbed for exclusive
access by a handler other than uinput.
Signed-off-by: Ryan Mallon <rmallon@gmail.com>
---
drivers/input/misc/uinput.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
index 6aea346..0962ecb 100644
--- a/drivers/input/misc/uinput.c
+++ b/drivers/input/misc/uinput.c
@@ -443,7 +443,7 @@ static ssize_t uinput_inject_event(struct uinput_device *udev,
if (input_event_from_user(buffer + bytes, &ev))
return -EFAULT;
- input_event(udev->dev, ev.type, ev.code, ev.value);
+ input_inject_event(udev->dev, ev.type, ev.code, ev.value);
bytes += input_event_size();
}
--
1.7.9.7
^ permalink raw reply related
* [PATCH 1/2] uinput: Support injecting multiple events in one write() call
From: Ryan Mallon @ 2013-09-10 23:32 UTC (permalink / raw)
To: dmitry.torokhov, rydberg; +Cc: carl, linux-input, linux-kernel, Ryan Mallon
Rework the code in uinput_inject_event so that it matches the code in
evdev_write and allows injecting more than one event, or zero events.
Signed-off-by: Ryan Mallon <rmallon@gmail.com>
---
drivers/input/misc/uinput.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
index a0a4bba..6aea346 100644
--- a/drivers/input/misc/uinput.c
+++ b/drivers/input/misc/uinput.c
@@ -434,16 +434,20 @@ static ssize_t uinput_inject_event(struct uinput_device *udev,
const char __user *buffer, size_t count)
{
struct input_event ev;
+ size_t bytes = 0;
- if (count < input_event_size())
+ if (count != 0 && count < input_event_size())
return -EINVAL;
- if (input_event_from_user(buffer, &ev))
- return -EFAULT;
+ while (bytes + input_event_size() <= count) {
+ if (input_event_from_user(buffer + bytes, &ev))
+ return -EFAULT;
- input_event(udev->dev, ev.type, ev.code, ev.value);
+ input_event(udev->dev, ev.type, ev.code, ev.value);
+ bytes += input_event_size();
+ }
- return input_event_size();
+ return bytes;
}
static ssize_t uinput_write(struct file *file, const char __user *buffer,
--
1.7.9.7
^ permalink raw reply related
* Re: [PATCH 2/2] uinput: Use input_inject_event in uinput_inject_event
From: Dmitry Torokhov @ 2013-09-11 0:12 UTC (permalink / raw)
To: Ryan Mallon; +Cc: rydberg, carl, linux-input, linux-kernel
In-Reply-To: <1378855973-11595-2-git-send-email-rmallon@gmail.com>
Hi Ryan,
On Wed, Sep 11, 2013 at 09:32:53AM +1000, Ryan Mallon wrote:
> Call input_inject_event rather than input_event in uinput_inject_event. This
> mirrors the behaviour of evdev_write. input_inject_event will ignore the
> injected event if the uinput input device has been grabbed for exclusive
> access by a handler other than uinput.
uinput is not an input handler but input driver and therefore should
continue using input_event().
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 1/2] uinput: Support injecting multiple events in one write() call
From: Dmitry Torokhov @ 2013-09-11 0:14 UTC (permalink / raw)
To: Ryan Mallon; +Cc: rydberg, carl, linux-input, linux-kernel
In-Reply-To: <1378855973-11595-1-git-send-email-rmallon@gmail.com>
Hi Ryan,
On Wed, Sep 11, 2013 at 09:32:52AM +1000, Ryan Mallon wrote:
> Rework the code in uinput_inject_event so that it matches the code in
> evdev_write and allows injecting more than one event, or zero events.
>
> Signed-off-by: Ryan Mallon <rmallon@gmail.com>
> ---
> drivers/input/misc/uinput.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
> index a0a4bba..6aea346 100644
> --- a/drivers/input/misc/uinput.c
> +++ b/drivers/input/misc/uinput.c
> @@ -434,16 +434,20 @@ static ssize_t uinput_inject_event(struct uinput_device *udev,
> const char __user *buffer, size_t count)
> {
> struct input_event ev;
> + size_t bytes = 0;
>
> - if (count < input_event_size())
> + if (count != 0 && count < input_event_size())
> return -EINVAL;
>
> - if (input_event_from_user(buffer, &ev))
> - return -EFAULT;
> + while (bytes + input_event_size() <= count) {
> + if (input_event_from_user(buffer + bytes, &ev))
> + return -EFAULT;
Unless the failure is in the very first packet we should tell the user
how many bytes we have transferred successfully instead of returning
-EFAULT.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 2/2] uinput: Use input_inject_event in uinput_inject_event
From: Ryan Mallon @ 2013-09-11 0:26 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: rydberg, carl, linux-input, linux-kernel
In-Reply-To: <20130911001235.GA5840@core.coreip.homeip.net>
On 11/09/13 10:12, Dmitry Torokhov wrote:
> Hi Ryan,
>
> On Wed, Sep 11, 2013 at 09:32:53AM +1000, Ryan Mallon wrote:
>> Call input_inject_event rather than input_event in uinput_inject_event. This
>> mirrors the behaviour of evdev_write. input_inject_event will ignore the
>> injected event if the uinput input device has been grabbed for exclusive
>> access by a handler other than uinput.
>
> uinput is not an input handler but input driver and therefore should
> continue using input_event().
Ah, I wasn't sure about this, which is why I broke it out into a
separate patch. Sorry for the noise.
~Ryan
^ permalink raw reply
* Re: [PATCH 1/2] uinput: Support injecting multiple events in one write() call
From: Ryan Mallon @ 2013-09-11 0:29 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: rydberg, carl, linux-input, linux-kernel
In-Reply-To: <20130911001428.GB5840@core.coreip.homeip.net>
On 11/09/13 10:14, Dmitry Torokhov wrote:
> Hi Ryan,
>
> On Wed, Sep 11, 2013 at 09:32:52AM +1000, Ryan Mallon wrote:
>> Rework the code in uinput_inject_event so that it matches the code in
>> evdev_write and allows injecting more than one event, or zero events.
>>
>> Signed-off-by: Ryan Mallon <rmallon@gmail.com>
>> ---
>> drivers/input/misc/uinput.c | 14 +++++++++-----
>> 1 file changed, 9 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
>> index a0a4bba..6aea346 100644
>> --- a/drivers/input/misc/uinput.c
>> +++ b/drivers/input/misc/uinput.c
>> @@ -434,16 +434,20 @@ static ssize_t uinput_inject_event(struct uinput_device *udev,
>> const char __user *buffer, size_t count)
>> {
>> struct input_event ev;
>> + size_t bytes = 0;
>>
>> - if (count < input_event_size())
>> + if (count != 0 && count < input_event_size())
>> return -EINVAL;
>>
>> - if (input_event_from_user(buffer, &ev))
>> - return -EFAULT;
>> + while (bytes + input_event_size() <= count) {
>> + if (input_event_from_user(buffer + bytes, &ev))
>> + return -EFAULT;
> Unless the failure is in the very first packet we should tell the user
> how many bytes we have transferred successfully instead of returning
> -EFAULT.
Good point. evdev is also broken in this regard, I'll send a follow-up
patch for that once you are happy with this one. Updated patch is below:
~Ryan
---
diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
index a0a4bba..92ca2c5 100644
--- a/drivers/input/misc/uinput.c
+++ b/drivers/input/misc/uinput.c
@@ -434,16 +434,20 @@ static ssize_t uinput_inject_event(struct uinput_device *u
const char __user *buffer, size_t count)
{
struct input_event ev;
+ size_t bytes = 0;
- if (count < input_event_size())
+ if (count != 0 && count < input_event_size())
return -EINVAL;
- if (input_event_from_user(buffer, &ev))
- return -EFAULT;
+ while (bytes + input_event_size() <= count) {
+ if (input_event_from_user(buffer + bytes, &ev))
+ return bytes ?: -EFAULT;
- input_event(udev->dev, ev.type, ev.code, ev.value);
+ input_event(udev->dev, ev.type, ev.code, ev.value);
+ bytes += input_event_size();
+ }
- return input_event_size();
+ return bytes;
}
static ssize_t uinput_write(struct file *file, const char __user *buffer,
^ permalink raw reply related
* Re: [PATCH 2/2] iio: ti_am335x_adc: Add continuous sampling support
From: Zubair Lutfullah : @ 2013-09-11 16:02 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Zubair Lutfullah, dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
bigeasy-hfZtesqFncYOwBW4kG4KsQ,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r
In-Reply-To: <522C7EDB.7090306-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
On Sun, Sep 08, 2013 at 02:42:51PM +0100, Jonathan Cameron wrote:
> On 09/01/13 12:17, Zubair Lutfullah wrote:
Thanks for the review. Fitting this driver in the usual trigger ABI
is a bad workaround indeed.
I am still a bit unclear regarding the implementation.
Questions and replies inline.
I hope I find the time to pursue the approach you suggest.
Thanks
ZubairLK
> > #include <linux/of.h>
> Ouch, how did that slip in here. Dropping this should have been
> in a separate patch as it doesn't really have anything to do with
> the rest of this series.
>
> > -#include <linux/iio/machine.h>
Squashed it in.. I'll do a cleanup patch separately.
> > #include <linux/iio/driver.h>
> > +static void tiadc_step_config(struct iio_dev *indio_dev)
> > +{
> > + struct tiadc_device *adc_dev = iio_priv(indio_dev);
> > unsigned int stepconfig;
> > - int i, steps;
> > + int i, steps, chan;
> Whilst I agree that pulling chan up here is slightly cleaner, technically that
> is a bit of unconnected cleanup and should have been in a precursor patch.
> (though don't bother doing so now!)
Again. Squashed in..
> > +
> > +static irqreturn_t tiadc_irq(int irq, void *private)
> > +{
...
> > + return IRQ_HANDLED;
> > + } else if (status & IRQENB_FIFO1THRES) {
> > + /* Trigger to push FIFO data to iio buffer */
> > + tiadc_writel(adc_dev, REG_IRQCLR, IRQENB_FIFO1THRES);
>
> So this is triggering at the threshold. Thus it isn't acting as a conventional
> trigger in the IIO sense at all. It is of no real use to anything outside
> this driver, so at the very least add the two callbacks to prevent this
> driver using any other trigger and the trigger being used by any other device.
> However, I'm really not happy with this solution.
>
> Take a look at running with no trigger at all and calling the buffer
> filling more directly. The issue is that if we have a trigger now it will
> become part of the userspace interface and become a pain to drop later.
>
> It is valid here I think to use the INDIO_BUFFER_HARDWARE mode (which skips
> the checks for a trigger) then just don't call iio_triggered_buffer_post_enable
> or iio_triggered_buffer_predisable. Then make this a threaded interrupt and
> put your tiadc_trigger_h as the thread. Finally just return IRQ_WAKE_THREAD
> in this case. You'll have to pull a few bits out of
> industrialio-triggered-buffer.c to create the kfifo etc, but no more than
> a few lines given the pollfunc won't exist.
If I understand correctly.
buffer setup ops for postenable and predisable have
return iio_triggered_buffer_*(indio_dev)
calls at the end. I return 0 instead.
Insead of iio_triggered_buffer setup. I only use iio_kfifo_alloc
(what size fifo is made? :s )
And do
indio_dev->modes |= INDIO_BUFFER_HARDWARE;
Then I use request_threaded_irq(*). The hardware IRQ line for the fifos
So the HW irq handler runs in interrupt context which wakes the threaded handler
which pushes samples to the iio buffer.
Shared IRQ line. ADC uses request_threaded_irq(). TSC uses request_irq()
ADC returns IRQ_WAKE_THREAD. But the TSC side returns NONE/HANDLED.
Does this change to a threaded irq affect the TSC side of things?
I just received an ack by Dmitry for that patch..
>
> Actually, might be cleaner to move the flush case into the thread as well
> and just have a single check in this top half of the handler. That bit
> is up to you given it is really a question of how long it take and hence
> if it is acceptable in interrupt context.
Sorry. I don't understand this paragraph.. Could you please elaborate?
The flushing fifo before/after enabling/disabling the buffer part?
I explained the reason behind the flushes
>
> > + iio_trigger_poll(indio_dev->trig, iio_get_time_ns());
> > + return IRQ_HANDLED;
> > + } else
> > + return IRQ_NONE;
> > +
> > +}
> > +
> > +static irqreturn_t tiadc_trigger_h(int irq, void *p)
> > +{
...
>
> > + fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
> > + for (k = 0; k < fifo1count; k = k + i) {
> As mentioned below, why not use 16 bit storage in the kfifo?
32 bit storage is because the FIFO is 32 bit in hardware.
It returns 12 bits data. some reserve bits. And 4 bit channel id.
The channel id is used to separate which channel data which.
Need the full 32 bits..
I've added a comment. Not here. but when the storage_bits = 32 is
set.
...
> > +static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
...
> Why would there be data in the fifo? Seems to me that flushing
> after disabling it and before enabling it should not both be
> necessary? Is this just a case of playing safe?
>
> > + /* Flush FIFO before starting sampling */
> > + fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
> > + for (i = 0; i < fifo1count; i++)
> > + read = tiadc_readl(adc_dev, REG_FIFO1);
Pre-flush was needed because TSC events would cause some
redundant ADC samples in the FIFO. Fixes have been sent to input.
Shouldn't happen now.
Post-flush is playing safe. There 'might' be a chance that
another sample comes in the time it takes for fifocount to be read
And that number of samples read from fifo. and then adc disabled.
> > +static int tiadc_buffer_postdisable(struct iio_dev *indio_dev)
> > +{
> > + struct tiadc_device *adc_dev = iio_priv(indio_dev);
> > +
> > + tiadc_step_config(indio_dev);
> > + kfree(adc_dev->data);
> blank line here please.
ok
> > + return 0;
> > +}
> > +
> > +static const struct iio_buffer_setup_ops tiadc_buffer_setup_ops = {
> > + .preenable = &tiadc_buffer_preenable,
> > + .postenable = &tiadc_buffer_postenable,
> > + .predisable = &tiadc_buffer_predisable,
> > + .postdisable = &tiadc_buffer_postdisable,
> > +};
> > +
> > +static const struct iio_trigger_ops tiadc_trigger_ops = {
> > + .owner = THIS_MODULE,
> > +};
> > +
>
> If you want a utility function for allocation, convention would
> say have one for freeing as well so that the code is obviously
> balanced and easy to check. Also this registers the buffer as
> well so the name should probably reflect that.
>
Noted.
> > +static int tiadc_iio_allocate_trigger(struct iio_dev *indio_dev)
> > +{
> > + struct tiadc_device *adc_dev = iio_priv(indio_dev);
> > + struct iio_trigger *trig = adc_dev->trig;
> > + int ret;
> > +
> devm_iio_trigger_alloc is now available and will simplify things a little.
>
Trigger related stuff is redundant anyways as moving away from trigger..
> > + trig = iio_trigger_alloc("%s-dev%d", indio_dev->name, indio_dev->id);
> > @@ -120,6 +295,7 @@ static int tiadc_channel_init(struct iio_dev *indio_dev, int channels)
> > chan->scan_type.realbits = 12;
> > chan->scan_type.storagebits = 32;
> I never noticed this before but putting 12 bit values in 32 bit storage
> does seem rather wasteful. Far as I can see above there is no real
> reason for doing so. If there is, then please add a comment justifying this
> choice!
>
Answered earlier. I added a comment here explaining.
> > @@ -142,11 +318,14 @@ static int tiadc_read_raw(struct iio_dev *indio_dev,
...
> > * Hence we need to flush out this data.
> > */
> Is the comment above still valid? I'd guess it referred to this code
> which has now gone.
> >
Its valid when the comment is read completely. All ADC channels
are sampled even if only one is read. The one needed is returned
to userspace. The rest of the fifo is flushed.
Not the ideal way. But I'm focusing on continuous sampling mode at the moment
> > adc_dev->channels = channels;
>
> Use the devm interfaces and there is no need to keep a copy
> of the irq number about.
>
Noted.
...
> >
> devm_request_irq will simplify error handling a little.
>
Indeed error handling gets confusing. I'll update.
...
> > + iio_trigger_free(adc_dev->trig);
> There is a devm trigger allocation function now. Using that
> would make things a little cleaner.
Noted
>
> > + free_irq(adc_dev->irq, indio_dev);
> Best to use the devm interfaces for irq handling unless
> there is a good reason to not do so (cleaner code ;)
>
> > iio_device_unregister(indio_dev);
>
Noted.
> We would normally expect this function to run in exact
> reverse order of the probe. That is not the case here
> so please reorder things so it is.
> > + iio_buffer_unregister(indio_dev);
I'll look into it.
> > tiadc_channels_remove(indio_dev);
> >
> > step_en = get_adc_step_mask(adc_dev);
> > @@ -298,10 +495,16 @@ static int tiadc_resume(struct device *dev)
> >
> > /* Make sure ADC is powered up */
> This comment doesn't seem to me to be true any more.
> > restore = tiadc_readl(adc_dev, REG_CTRL);
> > - restore &= ~(CNTRLREG_POWERDOWN);
> Some coments here would be good to explain the sequence that
> is going on.
>
> I'm guessing a bit, but looks like you turn off the touch screen
> but leave everything else alone, then set the threshold then
> reenable the touch screen before powering up the screen?
>
To be honest. I don't even remember.
It might have been a patch I squashed in.
does not seem to be related to the continuous sampling I guess..
I should remove this and send it separately.
> > + restore &= ~(CNTRLREG_TSCSSENB);
...
> > tiadc_writel(adc_dev, REG_CTRL, restore);
> > return 0;
> > }
> > diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
> > index db1791b..a372ebf 100644
> > --- a/include/linux/mfd/ti_am335x_tscadc.h
> > +++ b/include/linux/mfd/ti_am335x_tscadc.h
> > @@ -46,17 +46,25 @@
> > /* adc device */
> > struct adc_device *adc;
> > +
> > + /* Context save */
> > + unsigned int irqstat;
> Where are these used?
> > + unsigned int ctrl;
> > };
Oops. Don't think they are used anywhere.
Thanks for pointing it out.
> >
> > static inline struct ti_tscadc_dev *ti_tscadc_dev_get(struct platform_device *p)
> >
Thanks
ZubairLK
^ permalink raw reply
* Re: [PATCH 1/2] input: ti_am335x_tsc: Enable shared IRQ for TSC
From: Zubair Lutfullah : @ 2013-09-11 16:08 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Jonathan Cameron, Zubair Lutfullah,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
bigeasy-hfZtesqFncYOwBW4kG4KsQ,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r
In-Reply-To: <20130909161230.GA25107-WlK9ik9hQGAhIp7JRqBPierSzoNAToWh@public.gmane.org>
On Mon, Sep 09, 2013 at 09:12:30AM -0700, Dmitry Torokhov wrote:
> On Sun, Sep 08, 2013 at 12:29:26PM +0100, Jonathan Cameron wrote:
> > On 09/01/13 12:17, Zubair Lutfullah wrote:
> > > Enable shared IRQ to allow ADC to share IRQ line from
> > > parent MFD core. Only FIFO0 IRQs are for TSC and handled
> > > on the TSC side.
> > >
> > > Step mask would be updated from cached variable only previously.
> > > In rare cases when both TSC and ADC are used, the cached
> > > variable gets mixed up.
> > > The step mask is written with the required mask every time.
> > >
> > > Rachna Patil (TI) laid ground work for shared IRQ.
> > >
> > > Signed-off-by: Zubair Lutfullah <zubair.lutfullah-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > Whilst I would have prefered an mfd under these drivers and elegant handling
> > of the interrupts, I guess if this works it is at least not terribly invasive.
> >
> > However, this does need an Ack from Dmitry before I can take it (or for
> > Dmitry to take it himself?)
>
> I completely agree with Jonathan, more elegant handling would be nice
> but current one will do for now.
>
> Since most of the work on the driver is going through IIO tree I think
> it would make sense for this patch to go through it as well.
>
> Acked-by: Dmitry Torokhov <dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Thank-you.
ZubairLK
^ permalink raw reply
* [PATCH v3 02/10] HID: zeroplus: validate output report details
From: Benjamin Tissoires @ 2013-09-11 19:56 UTC (permalink / raw)
To: Benjamin Tissoires, Kees Cook, Henrik Rydberg, Jiri Kosina,
linux-input, linux-kernel
In-Reply-To: <1378929419-6269-1-git-send-email-benjamin.tissoires@redhat.com>
From: Kees Cook <keescook@chromium.org>
The zeroplus HID driver was not checking the size of allocated values
in fields it used. A HID device could send a malicious output report
that would cause the driver to write beyond the output report allocation
during initialization, causing a heap overflow:
[ 1442.728680] usb 1-1: New USB device found, idVendor=0c12, idProduct=0005
...
[ 1466.243173] BUG kmalloc-192 (Tainted: G W ): Redzone overwritten
CVE-2013-2889
Signed-off-by: Kees Cook <keescook@chromium.org>
Cc: stable@vger.kernel.org
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
v3:
- no changes
drivers/hid/hid-zpff.c | 18 +++++-------------
1 file changed, 5 insertions(+), 13 deletions(-)
diff --git a/drivers/hid/hid-zpff.c b/drivers/hid/hid-zpff.c
index 6ec28a3..a29756c 100644
--- a/drivers/hid/hid-zpff.c
+++ b/drivers/hid/hid-zpff.c
@@ -68,21 +68,13 @@ static int zpff_init(struct hid_device *hid)
struct hid_report *report;
struct hid_input *hidinput = list_entry(hid->inputs.next,
struct hid_input, list);
- struct list_head *report_list =
- &hid->report_enum[HID_OUTPUT_REPORT].report_list;
struct input_dev *dev = hidinput->input;
- int error;
+ int i, error;
- if (list_empty(report_list)) {
- hid_err(hid, "no output report found\n");
- return -ENODEV;
- }
-
- report = list_entry(report_list->next, struct hid_report, list);
-
- if (report->maxfield < 4) {
- hid_err(hid, "not enough fields in report\n");
- return -ENODEV;
+ for (i = 0; i < 4; i++) {
+ report = hid_validate_values(hid, HID_OUTPUT_REPORT, 0, i, 1);
+ if (!report)
+ return -ENODEV;
}
zpff = kzalloc(sizeof(struct zpff_device), GFP_KERNEL);
--
1.8.3.1
^ permalink raw reply related
* [PATCH v3 03/10] HID: sony: validate HID output report details
From: Benjamin Tissoires @ 2013-09-11 19:56 UTC (permalink / raw)
To: Benjamin Tissoires, Kees Cook, Henrik Rydberg, Jiri Kosina,
linux-input, linux-kernel
In-Reply-To: <1378929419-6269-1-git-send-email-benjamin.tissoires@redhat.com>
From: Kees Cook <keescook@chromium.org>
This driver must validate the availability of the HID output report and
its size before it can write LED states via buzz_set_leds(). This stops
a heap overflow that is possible if a device provides a malicious HID
output report:
[ 108.171280] usb 1-1: New USB device found, idVendor=054c, idProduct=0002
...
[ 117.507877] BUG kmalloc-192 (Not tainted): Redzone overwritten
CVE-2013-2890
Signed-off-by: Kees Cook <keescook@chromium.org>
Cc: stable@vger.kernel.org
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
v3:
- no changes
drivers/hid/hid-sony.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 30dbb6b..b18320d 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -537,6 +537,10 @@ static int buzz_init(struct hid_device *hdev)
drv_data = hid_get_drvdata(hdev);
BUG_ON(!(drv_data->quirks & BUZZ_CONTROLLER));
+ /* Validate expected report characteristics. */
+ if (!hid_validate_values(hdev, HID_OUTPUT_REPORT, 0, 0, 7))
+ return -ENODEV;
+
buzz = kzalloc(sizeof(*buzz), GFP_KERNEL);
if (!buzz) {
hid_err(hdev, "Insufficient memory, cannot allocate driver data\n");
--
1.8.3.1
^ permalink raw reply related
* [PATCH v3 04/10] HID: steelseries: validate output report details
From: Benjamin Tissoires @ 2013-09-11 19:56 UTC (permalink / raw)
To: Benjamin Tissoires, Kees Cook, Henrik Rydberg, Jiri Kosina,
linux-input, linux-kernel
In-Reply-To: <1378929419-6269-1-git-send-email-benjamin.tissoires@redhat.com>
From: Kees Cook <keescook@chromium.org>
A HID device could send a malicious output report that would cause the
steelseries HID driver to write beyond the output report allocation
during initialization, causing a heap overflow:
[ 167.981534] usb 1-1: New USB device found, idVendor=1038, idProduct=1410
...
[ 182.050547] BUG kmalloc-256 (Tainted: G W ): Redzone overwritten
CVE-2013-2891
Signed-off-by: Kees Cook <keescook@chromium.org>
Cc: stable@vger.kernel.org
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
v3:
- no changes
drivers/hid/hid-steelseries.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/hid/hid-steelseries.c b/drivers/hid/hid-steelseries.c
index d164911..29f328f 100644
--- a/drivers/hid/hid-steelseries.c
+++ b/drivers/hid/hid-steelseries.c
@@ -249,6 +249,11 @@ static int steelseries_srws1_probe(struct hid_device *hdev,
goto err_free;
}
+ if (!hid_validate_values(hdev, HID_OUTPUT_REPORT, 0, 0, 16)) {
+ ret = -ENODEV;
+ goto err_free;
+ }
+
ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
if (ret) {
hid_err(hdev, "hw start failed\n");
--
1.8.3.1
^ permalink raw reply related
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