* [PATCH v2 2/3] HID: logitech-hidpp: check WTP report length
From: Peter Wu @ 2014-12-16 15:55 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Nestor Lopez Casado, linux-input, linux-kernel
In-Reply-To: <1418745323-17133-1-git-send-email-peter@lekensteyn.nl>
Malicious USB devices can send bogus reports smaller than the expected
buffer size. Ensure that the length for WTP reports is valid to avoid
reading out of bounds.
Signed-off-by: Peter Wu <peter@lekensteyn.nl>
---
v1: patch 2/3 HID: logitech-{dj,hidpp}: check report length
v2: splitted original report length check patch
---
drivers/hid/hid-logitech-hidpp.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index b32f751..2f1b0ac 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -805,6 +805,11 @@ static int wtp_raw_event(struct hid_device *hdev, u8 *data, int size)
switch (data[0]) {
case 0x02:
+ if (size < 2) {
+ hid_err(hdev, "Received HID report of bad size (%d)",
+ size);
+ return 1;
+ }
if (hidpp->quirks & HIDPP_QUIRK_WTP_PHYSICAL_BUTTONS) {
input_event(wd->input, EV_KEY, BTN_LEFT,
!!(data[1] & 0x01));
@@ -818,6 +823,7 @@ static int wtp_raw_event(struct hid_device *hdev, u8 *data, int size)
return wtp_mouse_raw_xy_event(hidpp, &data[7]);
}
case REPORT_ID_HIDPP_LONG:
+ /* size is already checked in hidpp_raw_event. */
if ((report->fap.feature_index != wd->mt_feature_index) ||
(report->fap.funcindex_clientid != EVENT_TOUCHPAD_RAW_XY))
return 1;
--
2.1.3
^ permalink raw reply related
* Re: [PATCH 0/2] Input: atmel_mxt_ts - Add support for T100 multi-touch
From: Nick Dyer @ 2014-12-16 16:16 UTC (permalink / raw)
To: Javier Martinez Canillas, Dmitry Torokhov
Cc: Henrik Rydberg, Sjoerd Simons, Doug Anderson, Olof Johansson,
Yufeng Shen, Benson Leung, linux-input, linux-samsung-soc
In-Reply-To: <1418639947-28765-1-git-send-email-javier.martinez@collabora.co.uk>
On 15/12/14 10:39, Javier Martinez Canillas wrote:
> There are different multi-touch protocols used by Atmel maXTouch touchscreen
> chips. Old chips use the T9 multi-touch object type while newer chips use the
> T100 multi-touch object type. The Atmel touch driver currently only supports
> the former so this series add support for the later.
>
> The series is composed of the following patches:
>
> Nick Dyer (1):
> Input: atmel_mxt_ts - Implement support for T100 touch object
Hi Dmitry-
I posted an updated version of this patch on 9th December and would prefer
you apply that rather than this version, since it contains some useful
additional work.
A link to it:
http://www.spinics.net/lists/kernel/msg1885519.html
cheers
Nick
^ permalink raw reply
* Re: [PATCH 0/2] Input: atmel_mxt_ts - Add support for T100 multi-touch
From: Javier Martinez Canillas @ 2014-12-16 16:34 UTC (permalink / raw)
To: Nick Dyer, Dmitry Torokhov
Cc: Henrik Rydberg, Sjoerd Simons, Doug Anderson, Olof Johansson,
Yufeng Shen, Benson Leung, linux-input, linux-samsung-soc
In-Reply-To: <54905AD7.4070205@itdev.co.uk>
Hello Nick,
On 12/16/2014 05:16 PM, Nick Dyer wrote:
> On 15/12/14 10:39, Javier Martinez Canillas wrote:
>> There are different multi-touch protocols used by Atmel maXTouch touchscreen
>> chips. Old chips use the T9 multi-touch object type while newer chips use the
>> T100 multi-touch object type. The Atmel touch driver currently only supports
>> the former so this series add support for the later.
>>
>> The series is composed of the following patches:
>>
>> Nick Dyer (1):
>> Input: atmel_mxt_ts - Implement support for T100 touch object
>
> Hi Dmitry-
>
> I posted an updated version of this patch on 9th December and would prefer
> you apply that rather than this version, since it contains some useful
> additional work.
>
> A link to it:
> http://www.spinics.net/lists/kernel/msg1885519.html
>
Great, I didn't know about your patch since I was not in the cc list.
Thanks for posting and feel free to discard these patches.
Best regards,
Javier
^ permalink raw reply
* Re: [PATCH 0/2] Input: atmel_mxt_ts - Add support for T100 multi-touch
From: Nick Dyer @ 2014-12-16 16:39 UTC (permalink / raw)
To: Javier Martinez Canillas
Cc: Dmitry Torokhov, Henrik Rydberg, Sjoerd Simons, Doug Anderson,
Olof Johansson, Yufeng Shen, Benson Leung, linux-input,
linux-samsung-soc
In-Reply-To: <54905F00.4020107@collabora.co.uk>
On 16/12/14 16:34, Javier Martinez Canillas wrote:
> On 12/16/2014 05:16 PM, Nick Dyer wrote:
>>> Nick Dyer (1):
>>> Input: atmel_mxt_ts - Implement support for T100 touch object
>> I posted an updated version of this patch on 9th December and would prefer
>> you apply that rather than this version, since it contains some useful
>> additional work.
>>
>> A link to it:
>> http://www.spinics.net/lists/kernel/msg1885519.html
>
> Great, I didn't know about your patch since I was not in the cc list.
>
> Thanks for posting and feel free to discard these patches.
No worries! Do you want me to put you on CC in future?
I will add a todo to pick up your second change "Split out touchpad
initialisation logic" into patches I have waiting for upstream, it is a
sensible refactoring I think.
^ permalink raw reply
* Re: [PATCH 0/2] Input: atmel_mxt_ts - Add support for T100 multi-touch
From: Javier Martinez Canillas @ 2014-12-16 16:49 UTC (permalink / raw)
To: Nick Dyer
Cc: Dmitry Torokhov, Henrik Rydberg, Sjoerd Simons, Doug Anderson,
Olof Johansson, Yufeng Shen, Benson Leung, linux-input,
linux-samsung-soc
In-Reply-To: <54906031.8020309@itdev.co.uk>
Hello Nick,
On 12/16/2014 05:39 PM, Nick Dyer wrote:
> On 16/12/14 16:34, Javier Martinez Canillas wrote:
>> On 12/16/2014 05:16 PM, Nick Dyer wrote:
>>>> Nick Dyer (1):
>>>> Input: atmel_mxt_ts - Implement support for T100 touch object
>>> I posted an updated version of this patch on 9th December and would prefer
>>> you apply that rather than this version, since it contains some useful
>>> additional work.
>>>
>>> A link to it:
>>> http://www.spinics.net/lists/kernel/msg1885519.html
>>
>> Great, I didn't know about your patch since I was not in the cc list.
>>
>> Thanks for posting and feel free to discard these patches.
>
> No worries! Do you want me to put you on CC in future?
>
Yes, please cc me and also to Sjoerd since we are working on machines
that have Atmel maxtouch chips.
> I will add a todo to pick up your second change "Split out touchpad
> initialisation logic" into patches I have waiting for upstream, it is a
> sensible refactoring I think.
>
Awesome, what do you think about the change to have a common input device
initialization function that I squashed in your original patch?
mxt_initialize_t100_input_device() and mxt_initialize_t9_input_device()
are very similar so I think that is a sensible refactoring as well.
If you agree with the change I can post it on top of your patch once it
lands in mainline.
Best regards,
Javier
^ permalink raw reply
* [PATCH] Input: Add support for CLOCK_BOOTTIME
From: Aniroop Mathur @ 2014-12-16 16:56 UTC (permalink / raw)
To: dmitry.torokhov, dtor, linux-input; +Cc: aniroop.mathur, a.mathur
This patch adds support for CLOCK_BOOTTIME for input event timestamp.
CLOCK_BOOTTIME includes suspend time, so it would allow aplications
to get correct time difference between two events even when system
resumes from suspend state.
Signed-off-by: Aniroop Mathur <a.mathur@samsung.com>
---
drivers/input/evdev.c | 33 +++++++++++++++++++++------------
1 file changed, 21 insertions(+), 12 deletions(-)
diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
index de05545..7825794 100644
--- a/drivers/input/evdev.c
+++ b/drivers/input/evdev.c
@@ -28,6 +28,13 @@
#include <linux/cdev.h>
#include "input-compat.h"
+enum clock_type {
+ REAL = 0,
+ MONO,
+ BOOT,
+ CLK_MAX
+};
+
struct evdev {
int open;
struct input_handle handle;
@@ -108,8 +115,9 @@ static void evdev_queue_syn_dropped(struct evdev_client *client)
struct input_event ev;
ktime_t time;
- time = (client->clkid == CLOCK_MONOTONIC) ?
- ktime_get() : ktime_get_real();
+ time = (client->clkid == CLOCK_REALTIME) ?
+ ktime_get_real() : (client->clkid == CLOCK_MONOTONIC) ?
+ ktime_get() : ktime_get_boottime();
ev.time = ktime_to_timeval(time);
ev.type = EV_SYN;
@@ -159,7 +167,7 @@ static void __pass_event(struct evdev_client *client,
static void evdev_pass_values(struct evdev_client *client,
const struct input_value *vals, unsigned int count,
- ktime_t mono, ktime_t real)
+ ktime_t *ev_time)
{
struct evdev *evdev = client->evdev;
const struct input_value *v;
@@ -169,8 +177,9 @@ static void evdev_pass_values(struct evdev_client *client,
if (client->revoked)
return;
- event.time = ktime_to_timeval(client->clkid == CLOCK_MONOTONIC ?
- mono : real);
+ event.time = ktime_to_timeval(client->clkid == CLOCK_REALTIME ?
+ ev_time[REAL] : client->clkid == CLOCK_MONOTONIC ?
+ ev_time[MONO] : ev_time[BOOT]);
/* Interrupts are disabled, just acquire the lock. */
spin_lock(&client->buffer_lock);
@@ -198,21 +207,21 @@ static void evdev_events(struct input_handle *handle,
{
struct evdev *evdev = handle->private;
struct evdev_client *client;
- ktime_t time_mono, time_real;
+ ktime_t ev_time[CLK_MAX];
- time_mono = ktime_get();
- time_real = ktime_mono_to_real(time_mono);
+ ev_time[MONO] = ktime_get();
+ ev_time[REAL] = ktime_mono_to_real(ev_time[MONO]);
+ ev_time[BOOT] = ktime_get_boottime();
rcu_read_lock();
client = rcu_dereference(evdev->grab);
if (client)
- evdev_pass_values(client, vals, count, time_mono, time_real);
+ evdev_pass_values(client, vals, count, ev_time);
else
list_for_each_entry_rcu(client, &evdev->client_list, node)
- evdev_pass_values(client, vals, count,
- time_mono, time_real);
+ evdev_pass_values(client, vals, count, ev_time);
rcu_read_unlock();
}
@@ -874,7 +883,7 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,
case EVIOCSCLOCKID:
if (copy_from_user(&i, p, sizeof(unsigned int)))
return -EFAULT;
- if (i != CLOCK_MONOTONIC && i != CLOCK_REALTIME)
+ if (i != CLOCK_MONOTONIC && i != CLOCK_REALTIME && i != CLOCK_BOOTTIME)
return -EINVAL;
client->clkid = i;
return 0;
--
1.9.1
^ permalink raw reply related
* Re: [PATCH 0/2] Input: atmel_mxt_ts - Add support for T100 multi-touch
From: Nick Dyer @ 2014-12-16 17:07 UTC (permalink / raw)
To: Javier Martinez Canillas
Cc: Dmitry Torokhov, Henrik Rydberg, Sjoerd Simons, Doug Anderson,
Olof Johansson, Yufeng Shen, Benson Leung, linux-input,
linux-samsung-soc
In-Reply-To: <54906285.3060907@collabora.co.uk>
On 16/12/14 16:49, Javier Martinez Canillas wrote:
> Awesome, what do you think about the change to have a common input device
> initialization function that I squashed in your original patch?
> mxt_initialize_t100_input_device() and mxt_initialize_t9_input_device()
> are very similar so I think that is a sensible refactoring as well.
>
> If you agree with the change I can post it on top of your patch once it
> lands in mainline.
I had been keeping them separate on the basis that we don't want changes to
support new T100 features to cause regressions in the old T9 handling. But
there is a fair amount of duplication as you say, probably worth addressing.
FWIW I have a queue of stuff that might be considered higher priority, the
next 15-patch set would be up to "add regulator control support":
https://github.com/ndyer/linux/compare/dtor:next...for-dtor
It does cause me some issues to merge upstream refactorings past that lot...
^ permalink raw reply
* [git pull] Input updates for 3.19-rc0
From: Dmitry Torokhov @ 2014-12-16 17:32 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-kernel, linux-input, Jiri Kosina
Hi Linus,
Please pull from:
git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git for-linus
or
master.kernel.org:/pub/scm/linux/kernel/git/dtor/input.git for-linus
to receive updates for the input subsystem. You will get 2 new drivers
ofr Elan hardware (for I2C touchpad and touchscreen found in several
Chromebooks and other devices), a driver for Goodix touch panel, and
small fixes to Cypress I2C trackpad and other input drivers. Also we
switched to use __maybe_unused instead of gating suspend/resume code
with #ifdef guards to get better compile coverage.
Changelog:
---------
Alexander Stein (1):
Input: gpio_keys - add device tree support for interrupt only keys
Aniroop Mathur (2):
Input: initialize input_no to -1 to avoid subtraction
Input: initialize device counter variables with -1
Bastien Nocera (1):
Input: add driver for the Goodix touchpanel
Chen Gang (1):
Input: lifebook - use "static inline" instead of "inline" in lifebook.h
Dan Carpenter (1):
Input: lm8323 - missing error check in lm8323_set_disable()
Dmitry Torokhov (1):
Input: gpio_keys - fix warning regarding uninitialized 'irq' variable
Dudley Du (3):
Input: cyapa - switch to using managed resources
Input: cyapa - fix resuming the device
Input: cyapa - use 'error' for error codes
Duson Lin (1):
Input: add driver for Elan I2C/SMbus touchpad
Fabio Estevam (1):
Input: gpio_keys - fix warning regarding uninitialized 'button' variable
Geert Uytterhoeven (2):
Input: amikbd - fix build if !CONFIG_HW_CONSOLE
Input: amikbd - allocate temporary keymap buffer on the stack
Jingoo Han (3):
Input: misc - use __maybe_unused instead of ifdef around suspend/resume
Input: mouse - use __maybe_unused instead of ifdef around suspend/resume
Input: touchscreen - use __maybe_unused instead of ifdef around suspend/resume
Matt Ranostay (3):
Input: rename cap1106 driver to cap11xx
Input: cap11xx - add support for various cap11xx devices
Input: cap11xx - support for irq-active-high option
Pramod Gurav (3):
Input: mpr121 - switch to using managed resources
Input: pxa27x_keypad - switch to using managed resources
Input: lpc32xx-keys - switch to using managed resources
Scott Liu (1):
Input: add support for Elan eKTH I2C touchscreens
Stefan Brüns (1):
Input: atkbd - correct MSC_SCAN events for force_release keys
Tobias Klauser (1):
Input: altera_ps2 - switch to using managed resources
Todor Minchev (1):
Input: i8042 - do not try to load on Intel NUC D54250WYK
Diffstat:
--------
.../bindings/input/{cap1106.txt => cap11xx.txt} | 26 +-
.../devicetree/bindings/input/elan_i2c.txt | 34 +
.../devicetree/bindings/input/elants_i2c.txt | 33 +
.../devicetree/bindings/input/gpio-keys.txt | 10 +-
.../devicetree/bindings/vendor-prefixes.txt | 1 +
MAINTAINERS | 6 +
drivers/input/gameport/gameport.c | 4 +-
drivers/input/input.c | 4 +-
drivers/input/joystick/xpad.c | 8 +-
drivers/input/keyboard/Kconfig | 8 +-
drivers/input/keyboard/Makefile | 2 +-
drivers/input/keyboard/amikbd.c | 47 +-
drivers/input/keyboard/atkbd.c | 6 +-
drivers/input/keyboard/cap1106.c | 341 ------
drivers/input/keyboard/cap11xx.c | 376 ++++++
drivers/input/keyboard/gpio_keys.c | 37 +-
drivers/input/keyboard/lm8323.c | 2 +
drivers/input/keyboard/lpc32xx-keys.c | 92 +-
drivers/input/keyboard/mpr121_touchkey.c | 42 +-
drivers/input/keyboard/pxa27x_keypad.c | 84 +-
drivers/input/misc/88pm860x_onkey.c | 6 +-
drivers/input/misc/ad714x-i2c.c | 6 +-
drivers/input/misc/ad714x-spi.c | 6 +-
drivers/input/misc/adxl34x-i2c.c | 6 +-
drivers/input/misc/adxl34x-spi.c | 6 +-
drivers/input/misc/drv260x.c | 6 +-
drivers/input/misc/drv2667.c | 6 +-
drivers/input/misc/gp2ap002a00f.c | 6 +-
drivers/input/misc/ims-pcu.c | 4 +-
drivers/input/misc/kxtj9.c | 6 +-
drivers/input/misc/max77693-haptic.c | 6 +-
drivers/input/misc/max8925_onkey.c | 6 +-
drivers/input/misc/max8997_haptic.c | 4 +-
drivers/input/misc/palmas-pwrbutton.c | 6 +-
drivers/input/misc/pm8xxx-vibrator.c | 4 +-
drivers/input/misc/pmic8xxx-pwrkey.c | 6 +-
drivers/input/misc/pwm-beeper.c | 6 +-
drivers/input/misc/sirfsoc-onkey.c | 4 +-
drivers/input/misc/twl4030-vibra.c | 6 +-
drivers/input/misc/twl6040-vibra.c | 4 +-
drivers/input/mouse/Kconfig | 30 +
drivers/input/mouse/Makefile | 5 +
drivers/input/mouse/cyapa.c | 289 +++--
drivers/input/mouse/elan_i2c.h | 86 ++
drivers/input/mouse/elan_i2c_core.c | 1137 +++++++++++++++++
drivers/input/mouse/elan_i2c_i2c.c | 611 ++++++++++
drivers/input/mouse/elan_i2c_smbus.c | 514 ++++++++
drivers/input/mouse/lifebook.h | 6 +-
drivers/input/mouse/navpoint.c | 6 +-
drivers/input/mouse/synaptics_i2c.c | 6 +-
drivers/input/serio/altera_ps2.c | 81 +-
drivers/input/serio/i8042-x86ia64io.h | 10 +
drivers/input/serio/serio.c | 4 +-
drivers/input/serio/serio_raw.c | 4 +-
drivers/input/touchscreen/Kconfig | 25 +
drivers/input/touchscreen/Makefile | 2 +
drivers/input/touchscreen/ad7877.c | 6 +-
drivers/input/touchscreen/ad7879.c | 6 +-
drivers/input/touchscreen/ads7846.c | 6 +-
drivers/input/touchscreen/atmel_mxt_ts.c | 6 +-
drivers/input/touchscreen/auo-pixcir-ts.c | 6 +-
drivers/input/touchscreen/cy8ctmg110_ts.c | 6 +-
drivers/input/touchscreen/cyttsp_core.c | 7 +-
drivers/input/touchscreen/edt-ft5x06.c | 6 +-
drivers/input/touchscreen/eeti_ts.c | 6 +-
drivers/input/touchscreen/egalax_ts.c | 6 +-
drivers/input/touchscreen/elants_i2c.c | 1271 ++++++++++++++++++++
drivers/input/touchscreen/goodix.c | 395 ++++++
drivers/input/touchscreen/ili210x.c | 6 +-
drivers/input/touchscreen/ipaq-micro-ts.c | 6 +-
drivers/input/touchscreen/mms114.c | 6 +-
drivers/input/touchscreen/pixcir_i2c_ts.c | 6 +-
drivers/input/touchscreen/st1232.c | 7 +-
drivers/input/touchscreen/tsc2005.c | 6 +-
drivers/input/touchscreen/ucb1400_ts.c | 6 +-
drivers/input/touchscreen/wacom_i2c.c | 6 +-
drivers/input/touchscreen/zforce_ts.c | 6 +-
77 files changed, 4949 insertions(+), 922 deletions(-)
rename Documentation/devicetree/bindings/input/{cap1106.txt => cap11xx.txt} (64%)
create mode 100644 Documentation/devicetree/bindings/input/elan_i2c.txt
create mode 100644 Documentation/devicetree/bindings/input/elants_i2c.txt
delete mode 100644 drivers/input/keyboard/cap1106.c
create mode 100644 drivers/input/keyboard/cap11xx.c
create mode 100644 drivers/input/mouse/elan_i2c.h
create mode 100644 drivers/input/mouse/elan_i2c_core.c
create mode 100644 drivers/input/mouse/elan_i2c_i2c.c
create mode 100644 drivers/input/mouse/elan_i2c_smbus.c
create mode 100644 drivers/input/touchscreen/elants_i2c.c
create mode 100644 drivers/input/touchscreen/goodix.c
--
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 0/2] Input: atmel_mxt_ts - Add support for T100 multi-touch
From: Javier Martinez Canillas @ 2014-12-16 17:46 UTC (permalink / raw)
To: Nick Dyer
Cc: Dmitry Torokhov, Henrik Rydberg, Sjoerd Simons, Doug Anderson,
Olof Johansson, Yufeng Shen, Benson Leung, linux-input,
linux-samsung-soc
In-Reply-To: <549066D4.7000302@itdev.co.uk>
Hello Nick,
On 12/16/2014 06:07 PM, Nick Dyer wrote:
> On 16/12/14 16:49, Javier Martinez Canillas wrote:
>> Awesome, what do you think about the change to have a common input device
>> initialization function that I squashed in your original patch?
>> mxt_initialize_t100_input_device() and mxt_initialize_t9_input_device()
>> are very similar so I think that is a sensible refactoring as well.
>>
>> If you agree with the change I can post it on top of your patch once it
>> lands in mainline.
>
> I had been keeping them separate on the basis that we don't want changes to
> support new T100 features to cause regressions in the old T9 handling. But
> there is a fair amount of duplication as you say, probably worth addressing.
>
Yeah but if there are regressions I think we should address those instead of
duplicating code, to make the driver more maintainable in the long term.
> FWIW I have a queue of stuff that might be considered higher priority, the
> next 15-patch set would be up to "add regulator control support":
> https://github.com/ndyer/linux/compare/dtor:next...for-dtor
>
> It does cause me some issues to merge upstream refactorings past that lot...
>
Sure, I can hold any refactoring patches until you have pushed all your
queue so you don't have to resolve conflicts.
Just let me know when is a good time to push the refactoring changes.
Thanks a lot for your help and best regards,
Javier
^ permalink raw reply
* Re: [PATCH v15 09/12] input: cyapa: add gen5 trackpad device firmware update function support
From: Benson Leung @ 2014-12-16 20:24 UTC (permalink / raw)
To: Jeremiah Mahler, Dudley Du, Dmitry Torokhov, Henrik Rydberg,
Benson Leung, linux-input@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20141216135639.GA1238@hudson.localdomain>
On Tue, Dec 16, 2014 at 5:56 AM, Jeremiah Mahler <jmmahler@gmail.com> wrote:
> On Mon, Dec 15, 2014 at 02:23:20PM +0800, Dudley Du wrote:
>> Add firmware image update function supported for gen5 trackpad device,
>> it can be used through sysfs update_fw interface.
>> TEST=test on Chromebooks.
>>
>> Signed-off-by: Dudley Du <dudley.dulixin@gmail.com>
>> ---
>> drivers/input/mouse/Kconfig | 1 +
>> drivers/input/mouse/cyapa_gen5.c | 292 ++++++++++++++++++++++++++++++++++++++-
>> 2 files changed, 292 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/input/mouse/Kconfig b/drivers/input/mouse/Kconfig
>> index d8b46b0..728490e 100644
>> --- a/drivers/input/mouse/Kconfig
>> +++ b/drivers/input/mouse/Kconfig
>> @@ -206,6 +206,7 @@ config MOUSE_BCM5974
>> config MOUSE_CYAPA
>> tristate "Cypress APA I2C Trackpad support"
>> depends on I2C
>> + select CRC_ITU_T
>> help
>
> Just found out that if I2C_DESIGNWARE_PCI isn't enabled the touchpad
> won't work. Verify this on your machines. Then perhaps add a depends
> for I2C_DESIGNWARE_PCI instead of I2C since it would include the former.
This isn't strictly true on all devices, though. This is true on
DESIGNWARE_PCI based devices like the Acer C720 and the HP Chromebook
14, but on other platforms that use Cypress trackpads, such as ARM
platforms like the Samsung Chromebook Series 3 DESIGNWARE_PCI is not
required, and will just result in a driver that's never used being
built.
The specific I2C bus that's being used here shouldn't matter here...
that's more of a platform issue. In the case with Chromebooks, it
might make sense to change drivers/platform/chrome/Kconfig so that
CHROMEOS_LAPTOP depends on I2C_DESIGNWARE_PCI, maybe.
--
Benson Leung
Software Engineer, Chrome OS
Google Inc.
bleung@google.com
^ permalink raw reply
* [PATCH 0/2] HID: lenovo: Extra features for compact keyboards
From: Jamie Lentin @ 2014-12-16 21:26 UTC (permalink / raw)
To: Jiri Kosina; +Cc: linux-input, linux-kernel, Jamie Lentin
Another set of patches for the Lenovo Compact Keyboards, enabling
sensitivity control and improving scroll-wheel emulation.
This applies to 3.18, once previous fixups[0] have been applied. Tested
with USB and Bluetooth versions of the keyboard.
Cheers,
[0] https://lkml.org/lkml/2014/11/9/25
Jamie Lentin (2):
HID: lenovo: Add sensitivity control to compact keyboards
HID: lenovo: Use native middle-button mode for compact keyboards
drivers/hid/hid-lenovo.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 78 insertions(+), 1 deletion(-)
--
2.1.3
^ permalink raw reply
* [PATCH 2/2] HID: lenovo: Use native middle-button mode for compact keyboards
From: Jamie Lentin @ 2014-12-16 21:26 UTC (permalink / raw)
To: Jiri Kosina; +Cc: linux-input, linux-kernel, Jamie Lentin
In-Reply-To: <1418765206-30885-1-git-send-email-jm@lentin.co.uk>
By default the middle button is in a compatibility mode, and generates
standard wheel events when dragging with the middle trackpoint button.
Unfortunately this is buggy:
* The middle button comes up before starting wheel events, causing a middle
click on whatever the mouse cursor was sitting on
* The USB keyboard always generates the "native" horizontal wheel event,
regardless of mode.
Instead, enable the "native" mode the Windows driver uses, and add support
for the custom events this generates. This fixes the USB keyboard wheel
events, and the middle-click up event comes after the wheel events.
Signed-off-by: Jamie Lentin <jm@lentin.co.uk>
---
drivers/hid/hid-lenovo.c | 37 +++++++++++++++++++++++++++++++++++++
1 file changed, 37 insertions(+)
diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
index 0f35a76..c4c3f09 100644
--- a/drivers/hid/hid-lenovo.c
+++ b/drivers/hid/hid-lenovo.c
@@ -92,6 +92,38 @@ static int lenovo_input_mapping_cptkbd(struct hid_device *hdev,
case 0x00fa: /* Fn-Esc: Fn-lock toggle */
map_key_clear(KEY_FN_ESC);
return 1;
+ case 0x00fb: /* Middle mouse button (in native mode) */
+ map_key_clear(BTN_MIDDLE);
+ return 1;
+ }
+ }
+
+ /* Compatibility middle/wheel mappings should be ignored */
+ if (usage->hid == HID_GD_WHEEL)
+ return -1;
+ if ((usage->hid & HID_USAGE_PAGE) == HID_UP_BUTTON &&
+ (usage->hid & HID_USAGE) == 0x003)
+ return -1;
+ if ((usage->hid & HID_USAGE_PAGE) == HID_UP_CONSUMER &&
+ (usage->hid & HID_USAGE) == 0x238)
+ return -1;
+
+ /* Map wheel emulation reports: 0xffa1 = USB, 0xff10 = BT */
+ if ((usage->hid & HID_USAGE_PAGE) == 0xff100000 ||
+ (usage->hid & HID_USAGE_PAGE) == 0xffa10000) {
+ field->flags |= HID_MAIN_ITEM_RELATIVE | HID_MAIN_ITEM_VARIABLE;
+ field->logical_minimum = -127;
+ field->logical_maximum = 127;
+
+ switch (usage->hid & HID_USAGE) {
+ case 0x0000:
+ hid_map_usage(hi, usage, bit, max, EV_REL, 0x06);
+ return 1;
+ case 0x0001:
+ hid_map_usage(hi, usage, bit, max, EV_REL, 0x08);
+ return 1;
+ default:
+ return -1;
}
}
@@ -633,6 +665,11 @@ static int lenovo_probe_cptkbd(struct hid_device *hdev)
if (ret)
hid_warn(hdev, "Failed to switch F7/9/11 mode: %d\n", ret);
+ /* Switch middle button to native mode */
+ ret = lenovo_send_cmd_cptkbd(hdev, 0x09, 0x01);
+ if (ret)
+ hid_warn(hdev, "Failed to switch middle button: %d\n", ret);
+
/* Set keyboard settings to known state */
cptkbd_data->fn_lock = true;
cptkbd_data->sensitivity = 0x05;
--
2.1.3
^ permalink raw reply related
* Re: [PATCH v2 1/3] HID: logitech-dj: check report length
From: Benjamin Tissoires @ 2014-12-16 21:55 UTC (permalink / raw)
To: Peter Wu
Cc: Jiri Kosina, Benjamin Tissoires, Nestor Lopez Casado, linux-input,
linux-kernel@vger.kernel.org
In-Reply-To: <1418745323-17133-1-git-send-email-peter@lekensteyn.nl>
On Tue, Dec 16, 2014 at 10:55 AM, Peter Wu <peter@lekensteyn.nl> wrote:
> Malicious USB devices can send bogus reports smaller than the expected
> buffer size. Ensure that the length is valid to avoid reading out of
> bounds.
>
> Signed-off-by: Peter Wu <peter@lekensteyn.nl>
> ---
> v1: patch 2/3 HID: logitech-{dj,hidpp}: check report length
> v2: splitted original report length check patch
> ---
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cheers,
Benjamin
> drivers/hid/hid-logitech-dj.c | 16 +++++++++++++++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
> index c917ab6..5bc6d80 100644
> --- a/drivers/hid/hid-logitech-dj.c
> +++ b/drivers/hid/hid-logitech-dj.c
> @@ -962,10 +962,24 @@ static int logi_dj_raw_event(struct hid_device *hdev,
>
> switch (data[0]) {
> case REPORT_ID_DJ_SHORT:
> + if (size != DJREPORT_SHORT_LENGTH) {
> + dev_err(&hdev->dev, "DJ report of bad size (%d)", size);
> + return false;
> + }
> return logi_dj_dj_event(hdev, report, data, size);
> case REPORT_ID_HIDPP_SHORT:
> - /* intentional fallthrough */
> + if (size != HIDPP_REPORT_SHORT_LENGTH) {
> + dev_err(&hdev->dev,
> + "Short HID++ report of bad size (%d)", size);
> + return false;
> + }
> + return logi_dj_hidpp_event(hdev, report, data, size);
> case REPORT_ID_HIDPP_LONG:
> + if (size != HIDPP_REPORT_LONG_LENGTH) {
> + dev_err(&hdev->dev,
> + "Long HID++ report of bad size (%d)", size);
> + return false;
> + }
> return logi_dj_hidpp_event(hdev, report, data, size);
> }
>
> --
> 2.1.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 2/3] HID: logitech-hidpp: check WTP report length
From: Benjamin Tissoires @ 2014-12-16 21:56 UTC (permalink / raw)
To: Peter Wu
Cc: Jiri Kosina, Benjamin Tissoires, Nestor Lopez Casado, linux-input,
linux-kernel@vger.kernel.org
In-Reply-To: <1418745323-17133-2-git-send-email-peter@lekensteyn.nl>
On Tue, Dec 16, 2014 at 10:55 AM, Peter Wu <peter@lekensteyn.nl> wrote:
> Malicious USB devices can send bogus reports smaller than the expected
> buffer size. Ensure that the length for WTP reports is valid to avoid
> reading out of bounds.
>
> Signed-off-by: Peter Wu <peter@lekensteyn.nl>
> ---
> v1: patch 2/3 HID: logitech-{dj,hidpp}: check report length
> v2: splitted original report length check patch
> ---
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cheers,
Benjamin
> drivers/hid/hid-logitech-hidpp.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> index b32f751..2f1b0ac 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
> @@ -805,6 +805,11 @@ static int wtp_raw_event(struct hid_device *hdev, u8 *data, int size)
>
> switch (data[0]) {
> case 0x02:
> + if (size < 2) {
> + hid_err(hdev, "Received HID report of bad size (%d)",
> + size);
> + return 1;
> + }
> if (hidpp->quirks & HIDPP_QUIRK_WTP_PHYSICAL_BUTTONS) {
> input_event(wd->input, EV_KEY, BTN_LEFT,
> !!(data[1] & 0x01));
> @@ -818,6 +823,7 @@ static int wtp_raw_event(struct hid_device *hdev, u8 *data, int size)
> return wtp_mouse_raw_xy_event(hidpp, &data[7]);
> }
> case REPORT_ID_HIDPP_LONG:
> + /* size is already checked in hidpp_raw_event. */
> if ((report->fap.feature_index != wd->mt_feature_index) ||
> (report->fap.funcindex_clientid != EVENT_TOUCHPAD_RAW_XY))
> return 1;
> --
> 2.1.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 3/3] HID: logitech-hidpp: separate HID++ from WTP processing
From: Benjamin Tissoires @ 2014-12-16 22:00 UTC (permalink / raw)
To: Peter Wu
Cc: Jiri Kosina, Benjamin Tissoires, Nestor Lopez Casado, linux-input,
linux-kernel@vger.kernel.org
In-Reply-To: <1418745323-17133-3-git-send-email-peter@lekensteyn.nl>
On Tue, Dec 16, 2014 at 10:55 AM, Peter Wu <peter@lekensteyn.nl> wrote:
> Previously wtp_raw_event would be called through
> hidpp_raw_hidpp_event (for the touchpad report) and hidpp_raw_event
> (for the mouse report).
>
> This patch removes one calling surface, making a clearer distinction
> between "generic HID++ processing" (matching internal reports) and
> device-specific event processing.
>
> Suggested-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> Signed-off-by: Peter Wu <peter@lekensteyn.nl>
> ---
> v1: patch 2/3 HID: logitech-{dj,hidpp}: check report length
> v2: splitted original report length check patch. Restructured code.
> ---
The patch is good per-se (and I tested the whole series BTW).
I just have a minor cosmetic comment.
If you feel like sending a v3, go ahead, otherwise, this patch is
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> drivers/hid/hid-logitech-hidpp.c | 18 ++++++++++++------
> 1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> index 2f1b0ac..3dcd59c 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
> @@ -942,7 +942,7 @@ static int hidpp_raw_hidpp_event(struct hidpp_device *hidpp, u8 *data,
>
> /*
> * If the mutex is locked then we have a pending answer from a
> - * previoulsly sent command
> + * previously sent command.
> */
> if (unlikely(mutex_is_locked(&hidpp->send_mutex))) {
> /*
> @@ -973,9 +973,6 @@ static int hidpp_raw_hidpp_event(struct hidpp_device *hidpp, u8 *data,
> return 1;
> }
>
> - if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)
> - return wtp_raw_event(hidpp->hid_dev, data, size);
> -
> return 0;
> }
>
> @@ -983,7 +980,9 @@ static int hidpp_raw_event(struct hid_device *hdev, struct hid_report *report,
> u8 *data, int size)
> {
> struct hidpp_device *hidpp = hid_get_drvdata(hdev);
> + int r = 0;
I'd rather have this variable named "ret".
2 reasons: that's how it is across the file, and I dislike variable
names with only one letter (except i, j, k, n, where this is obvious).
Cheers,
Benjamin
>
> + /* Generic HID++ processing. */
> switch (data[0]) {
> case REPORT_ID_HIDPP_LONG:
> if (size != HIDPP_REPORT_LONG_LENGTH) {
> @@ -991,16 +990,23 @@ static int hidpp_raw_event(struct hid_device *hdev, struct hid_report *report,
> size);
> return 1;
> }
> - return hidpp_raw_hidpp_event(hidpp, data, size);
> + r = hidpp_raw_hidpp_event(hidpp, data, size);
> + break;
> case REPORT_ID_HIDPP_SHORT:
> if (size != HIDPP_REPORT_SHORT_LENGTH) {
> hid_err(hdev, "received hid++ report of bad size (%d)",
> size);
> return 1;
> }
> - return hidpp_raw_hidpp_event(hidpp, data, size);
> + r = hidpp_raw_hidpp_event(hidpp, data, size);
> + break;
> }
>
> + /* If no report is available for further processing, skip calling
> + * raw_event of subclasses. */
> + if (r != 0)
> + return r;
> +
> if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)
> return wtp_raw_event(hdev, data, size);
>
> --
> 2.1.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH 1/2] HID: lenovo: Add sensitivity control to compact keyboards
From: Jamie Lentin @ 2014-12-16 21:26 UTC (permalink / raw)
To: Jiri Kosina; +Cc: linux-input, linux-kernel, Jamie Lentin
In-Reply-To: <1418765206-30885-1-git-send-email-jm@lentin.co.uk>
The trackpoint sensitivity can also be controlled, expose this via
sysfs.
Signed-off-by: Jamie Lentin <jm@lentin.co.uk>
---
drivers/hid/hid-lenovo.c | 42 +++++++++++++++++++++++++++++++++++++++++-
1 file changed, 41 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
index 4c55f4d..0f35a76 100644
--- a/drivers/hid/hid-lenovo.c
+++ b/drivers/hid/hid-lenovo.c
@@ -38,6 +38,7 @@ struct lenovo_drvdata_tpkbd {
struct lenovo_drvdata_cptkbd {
bool fn_lock;
+ int sensitivity;
};
#define map_key_clear(c) hid_map_usage_clear(hi, usage, bit, max, EV_KEY, (c))
@@ -145,6 +146,7 @@ static void lenovo_features_set_cptkbd(struct hid_device *hdev)
struct lenovo_drvdata_cptkbd *cptkbd_data = hid_get_drvdata(hdev);
ret = lenovo_send_cmd_cptkbd(hdev, 0x05, cptkbd_data->fn_lock);
+ ret = lenovo_send_cmd_cptkbd(hdev, 0x02, cptkbd_data->sensitivity);
if (ret)
hid_err(hdev, "Fn-lock setting failed: %d\n", ret);
}
@@ -179,13 +181,50 @@ static ssize_t attr_fn_lock_store_cptkbd(struct device *dev,
return count;
}
+static ssize_t attr_sensitivity_show_cptkbd(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct hid_device *hdev = container_of(dev, struct hid_device, dev);
+ struct lenovo_drvdata_cptkbd *cptkbd_data = hid_get_drvdata(hdev);
+
+ return snprintf(buf, PAGE_SIZE, "%u\n",
+ cptkbd_data->sensitivity);
+}
+
+static ssize_t attr_sensitivity_store_cptkbd(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf,
+ size_t count)
+{
+ struct hid_device *hdev = container_of(dev, struct hid_device, dev);
+ struct lenovo_drvdata_cptkbd *cptkbd_data = hid_get_drvdata(hdev);
+ int value;
+
+ if (kstrtoint(buf, 10, &value) || value < 1 || value > 255)
+ return -EINVAL;
+
+ cptkbd_data->sensitivity = value;
+ lenovo_features_set_cptkbd(hdev);
+
+ return count;
+}
+
+
static struct device_attribute dev_attr_fn_lock_cptkbd =
__ATTR(fn_lock, S_IWUSR | S_IRUGO,
attr_fn_lock_show_cptkbd,
attr_fn_lock_store_cptkbd);
+static struct device_attribute dev_attr_sensitivity_cptkbd =
+ __ATTR(sensitivity, S_IWUSR | S_IRUGO,
+ attr_sensitivity_show_cptkbd,
+ attr_sensitivity_store_cptkbd);
+
+
static struct attribute *lenovo_attributes_cptkbd[] = {
&dev_attr_fn_lock_cptkbd.attr,
+ &dev_attr_sensitivity_cptkbd.attr,
NULL
};
@@ -594,8 +633,9 @@ static int lenovo_probe_cptkbd(struct hid_device *hdev)
if (ret)
hid_warn(hdev, "Failed to switch F7/9/11 mode: %d\n", ret);
- /* Turn Fn-Lock on by default */
+ /* Set keyboard settings to known state */
cptkbd_data->fn_lock = true;
+ cptkbd_data->sensitivity = 0x05;
lenovo_features_set_cptkbd(hdev);
ret = sysfs_create_group(&hdev->dev.kobj, &lenovo_attr_group_cptkbd);
--
2.1.3
^ permalink raw reply related
* [PATCH 2/2] HID: logitech-hidpp: store the name of the device in struct hidpp
From: Benjamin Tissoires @ 2014-12-16 22:06 UTC (permalink / raw)
To: Jiri Kosina, Peter Wu, Nestor Lopez Casado
Cc: Peter Hutterer, linux-input, linux-kernel
In-Reply-To: <1418767562-4136-1-git-send-email-benjamin.tissoires@redhat.com>
If a disconnect occurs while getting the actual name of the device
(which can take several HID transactions), the name of the device will
be the hid name, provided by the Unifying Receiver.
This means that in some cases, the user space will see a different
name that what it usually sees when there is no disconnect.
We should store the name of the device in the struct hidpp. That way,
if a disconnect occurs while we are accessing the name,
hidpp_connect_event() can fail, and the input node is not created.
The input node will be created only if we have a connection which
lasts long enough to retrieve all the requested information:
name, protocol, and specific configuration.
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
drivers/hid/hid-logitech-hidpp.c | 31 ++++++++++++++++++++-----------
1 file changed, 20 insertions(+), 11 deletions(-)
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index c0fb5fe..4bc8714 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -89,6 +89,7 @@ struct hidpp_device {
struct hid_device *hid_dev;
struct mutex send_mutex;
void *send_receive_buf;
+ char *name;
wait_queue_head_t wait;
bool answer_available;
u8 protocol_major;
@@ -1087,6 +1088,7 @@ static void hidpp_input_close(struct input_dev *dev)
static struct input_dev *hidpp_allocate_input(struct hid_device *hdev)
{
struct input_dev *input_dev = devm_input_allocate_device(&hdev->dev);
+ struct hidpp_device *hidpp = hid_get_drvdata(hdev);
if (!input_dev)
return NULL;
@@ -1095,7 +1097,7 @@ static struct input_dev *hidpp_allocate_input(struct hid_device *hdev)
input_dev->open = hidpp_input_open;
input_dev->close = hidpp_input_close;
- input_dev->name = hdev->name;
+ input_dev->name = hidpp->name;
input_dev->phys = hdev->phys;
input_dev->uniq = hdev->uniq;
input_dev->id.bustype = hdev->bus;
@@ -1137,22 +1139,28 @@ static void hidpp_connect_event(struct hidpp_device *hidpp)
hid_info(hdev, "HID++ %u.%u device connected.\n",
hidpp->protocol_major, hidpp->protocol_minor);
+ if (!hidpp->name || hidpp->name == hdev->name) {
+ name = hidpp_get_device_name(hidpp);
+ if (!name) {
+ hid_err(hdev,
+ "unable to retrieve the name of the device");
+ return;
+ }
+
+ devm_name = devm_kasprintf(&hdev->dev, GFP_KERNEL, "%s", name);
+ kfree(name);
+ if (!devm_name)
+ return;
+
+ hidpp->name = devm_name;
+ }
+
input = hidpp_allocate_input(hdev);
if (!input) {
hid_err(hdev, "cannot allocate new input device: %d\n", ret);
return;
}
- name = hidpp_get_device_name(hidpp);
- if (!name) {
- hid_err(hdev, "unable to retrieve the name of the device");
- } else {
- devm_name = devm_kasprintf(&hdev->dev, GFP_KERNEL, "%s", name);
- if (devm_name)
- input->name = devm_name;
- kfree(name);
- }
-
hidpp_populate_input(hidpp, input, false);
ret = input_register_device(input);
@@ -1175,6 +1183,7 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
return -ENOMEM;
hidpp->hid_dev = hdev;
+ hidpp->name = hdev->name;
hid_set_drvdata(hdev, hidpp);
hidpp->quirks = id->driver_data;
--
2.1.0
^ permalink raw reply related
* [PATCH 1/2] HID: logitech-hidpp: bail out if wtp_connect fails
From: Benjamin Tissoires @ 2014-12-16 22:06 UTC (permalink / raw)
To: Jiri Kosina, Peter Wu, Nestor Lopez Casado
Cc: Peter Hutterer, linux-input, linux-kernel
If wtp_connect() fails, that means most of the time that the device has
been disconnected. Subsequent attempts to contact the device will fail
too, so it's simpler to bail out earlier.
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
drivers/hid/hid-logitech-hidpp.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index d008d71..c0fb5fe 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -914,24 +914,24 @@ static int wtp_allocate(struct hid_device *hdev, const struct hid_device_id *id)
return 0;
};
-static void wtp_connect(struct hid_device *hdev, bool connected)
+static int wtp_connect(struct hid_device *hdev, bool connected)
{
struct hidpp_device *hidpp = hid_get_drvdata(hdev);
struct wtp_data *wd = hidpp->private_data;
int ret;
if (!connected)
- return;
+ return 0;
if (!wd->x_size) {
ret = wtp_get_config(hidpp);
if (ret) {
hid_err(hdev, "Can not get wtp config: %d\n", ret);
- return;
+ return ret;
}
}
- hidpp_touchpad_set_raw_report_state(hidpp, wd->mt_feature_index,
+ return hidpp_touchpad_set_raw_report_state(hidpp, wd->mt_feature_index,
true, true);
}
@@ -1115,8 +1115,11 @@ static void hidpp_connect_event(struct hidpp_device *hidpp)
struct input_dev *input;
char *name, *devm_name;
- if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)
- wtp_connect(hdev, connected);
+ if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP) {
+ ret = wtp_connect(hdev, connected);
+ if (ret)
+ return;
+ }
if (!connected || hidpp->delayed_input)
return;
--
2.1.0
^ permalink raw reply related
* Re: [PATCH 1/2] HID: logitech-hidpp: bail out if wtp_connect fails
From: Benjamin Tissoires @ 2014-12-16 22:13 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Jiri Kosina, Peter Wu, Nestor Lopez Casado, linux-input,
linux-kernel@vger.kernel.org
In-Reply-To: <1418767562-4136-1-git-send-email-benjamin.tissoires@redhat.com>
On Tue, Dec 16, 2014 at 5:06 PM, Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
> If wtp_connect() fails, that means most of the time that the device has
> been disconnected. Subsequent attempts to contact the device will fail
> too, so it's simpler to bail out earlier.
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---
Jiri, Peter,
the logitech patches are queuing up really fast.
To keep track of them, I made a bundle on patchwork:
https://patchwork.kernel.org/bundle/bentiss/hid-logitech-hidpp/
(/me discovered a new tool to play with)
Right now, the patch "HID: logitech-hidpp: detect HID++ 2.0 errors
too" is waiting for Logitech's approval, and the 2 of this series need
review.
Peter, please tell me if I missed one patch.
Cheers,
Benjamin
> drivers/hid/hid-logitech-hidpp.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> index d008d71..c0fb5fe 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
> @@ -914,24 +914,24 @@ static int wtp_allocate(struct hid_device *hdev, const struct hid_device_id *id)
> return 0;
> };
>
> -static void wtp_connect(struct hid_device *hdev, bool connected)
> +static int wtp_connect(struct hid_device *hdev, bool connected)
> {
> struct hidpp_device *hidpp = hid_get_drvdata(hdev);
> struct wtp_data *wd = hidpp->private_data;
> int ret;
>
> if (!connected)
> - return;
> + return 0;
>
> if (!wd->x_size) {
> ret = wtp_get_config(hidpp);
> if (ret) {
> hid_err(hdev, "Can not get wtp config: %d\n", ret);
> - return;
> + return ret;
> }
> }
>
> - hidpp_touchpad_set_raw_report_state(hidpp, wd->mt_feature_index,
> + return hidpp_touchpad_set_raw_report_state(hidpp, wd->mt_feature_index,
> true, true);
> }
>
> @@ -1115,8 +1115,11 @@ static void hidpp_connect_event(struct hidpp_device *hidpp)
> struct input_dev *input;
> char *name, *devm_name;
>
> - if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)
> - wtp_connect(hdev, connected);
> + if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP) {
> + ret = wtp_connect(hdev, connected);
> + if (ret)
> + return;
> + }
>
> if (!connected || hidpp->delayed_input)
> return;
> --
> 2.1.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH 3/3] HID: logitech-hidpp: separate HID++ from WTP processing
From: Peter Wu @ 2014-12-16 23:23 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Nestor Lopez Casado, linux-input, linux-kernel
In-Reply-To: <CAN+gG=GhJx38LDwAGoPA1BpYOmjS8r_xnYPXWOgB66VLwmv5kg@mail.gmail.com>
Previously wtp_raw_event would be called through
hidpp_raw_hidpp_event (for the touchpad report) and hidpp_raw_event
(for the mouse report).
This patch removes one calling surface, making a clearer distinction
between "generic HID++ processing" (matching internal reports) and
device-specific event processing.
Suggested-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Signed-off-by: Peter Wu <peter@lekensteyn.nl>
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
v1: patch 2/3 HID: logitech-{dj,hidpp}: check report length
v2: splitted original report length check patch. Restructured code.
v3: renamed var r to ret for consistency, added R-b of Benjamin
Hi Benjamin,
I am in a good mood now, so here is v3!
IMO 'r' as 'ret' is common just like 'i' is short for 'index', but for
consistency sake I'll use ret here ;)
Kind regards,
Peter
---
drivers/hid/hid-logitech-hidpp.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 2f1b0ac..18af2de 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -942,7 +942,7 @@ static int hidpp_raw_hidpp_event(struct hidpp_device *hidpp, u8 *data,
/*
* If the mutex is locked then we have a pending answer from a
- * previoulsly sent command
+ * previously sent command.
*/
if (unlikely(mutex_is_locked(&hidpp->send_mutex))) {
/*
@@ -973,9 +973,6 @@ static int hidpp_raw_hidpp_event(struct hidpp_device *hidpp, u8 *data,
return 1;
}
- if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)
- return wtp_raw_event(hidpp->hid_dev, data, size);
-
return 0;
}
@@ -983,7 +980,9 @@ static int hidpp_raw_event(struct hid_device *hdev, struct hid_report *report,
u8 *data, int size)
{
struct hidpp_device *hidpp = hid_get_drvdata(hdev);
+ int ret = 0;
+ /* Generic HID++ processing. */
switch (data[0]) {
case REPORT_ID_HIDPP_LONG:
if (size != HIDPP_REPORT_LONG_LENGTH) {
@@ -991,16 +990,23 @@ static int hidpp_raw_event(struct hid_device *hdev, struct hid_report *report,
size);
return 1;
}
- return hidpp_raw_hidpp_event(hidpp, data, size);
+ ret = hidpp_raw_hidpp_event(hidpp, data, size);
+ break;
case REPORT_ID_HIDPP_SHORT:
if (size != HIDPP_REPORT_SHORT_LENGTH) {
hid_err(hdev, "received hid++ report of bad size (%d)",
size);
return 1;
}
- return hidpp_raw_hidpp_event(hidpp, data, size);
+ ret = hidpp_raw_hidpp_event(hidpp, data, size);
+ break;
}
+ /* If no report is available for further processing, skip calling
+ * raw_event of subclasses. */
+ if (ret != 0)
+ return ret;
+
if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)
return wtp_raw_event(hdev, data, size);
--
2.1.3
^ permalink raw reply related
* Re: [PATCH 1/2] HID: logitech-hidpp: bail out if wtp_connect fails
From: Peter Wu @ 2014-12-16 23:33 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Jiri Kosina, Nestor Lopez Casado, Peter Hutterer, linux-input,
linux-kernel
In-Reply-To: <1418767562-4136-1-git-send-email-benjamin.tissoires@redhat.com>
On Tuesday 16 December 2014 17:06:01 Benjamin Tissoires wrote:
> If wtp_connect() fails, that means most of the time that the device has
> been disconnected. Subsequent attempts to contact the device will fail
> too, so it's simpler to bail out earlier.
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---
> drivers/hid/hid-logitech-hidpp.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> index d008d71..c0fb5fe 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
> @@ -914,24 +914,24 @@ static int wtp_allocate(struct hid_device *hdev, const struct hid_device_id *id)
> return 0;
> };
>
> -static void wtp_connect(struct hid_device *hdev, bool connected)
> +static int wtp_connect(struct hid_device *hdev, bool connected)
> {
> struct hidpp_device *hidpp = hid_get_drvdata(hdev);
> struct wtp_data *wd = hidpp->private_data;
> int ret;
>
> if (!connected)
> - return;
> + return 0;
"0" is success, what about -1 or -ENODEV here to signal an error? The
following line (in hidpp_connect_event) returns on !connected, but that
is no reason to return 0 here.
("No connection" sounds like an error condition to me as "[wtp_]connect"
cannot do something meaningful.)
Whether or not you change the return value is up to you. This patch is
Reviewed-by: Peter Wu <peter@lekensteyn.nl>
Kind regards,
Peter
> if (!wd->x_size) {
> ret = wtp_get_config(hidpp);
> if (ret) {
> hid_err(hdev, "Can not get wtp config: %d\n", ret);
> - return;
> + return ret;
> }
> }
>
> - hidpp_touchpad_set_raw_report_state(hidpp, wd->mt_feature_index,
> + return hidpp_touchpad_set_raw_report_state(hidpp, wd->mt_feature_index,
> true, true);
> }
>
> @@ -1115,8 +1115,11 @@ static void hidpp_connect_event(struct hidpp_device *hidpp)
> struct input_dev *input;
> char *name, *devm_name;
>
> - if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)
> - wtp_connect(hdev, connected);
> + if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP) {
> + ret = wtp_connect(hdev, connected);
> + if (ret)
> + return;
> + }
>
> if (!connected || hidpp->delayed_input)
> return;
>
^ permalink raw reply
* Re: PROBLEM: [HP Stream Notebook - 11-d010nr] clickpad malfunctions after performing a hardware click
From: Peter Hutterer @ 2014-12-17 1:09 UTC (permalink / raw)
To: Amordea Whiteoak; +Cc: Benjamin Tissoires, linux-input, Andrew Duggan
In-Reply-To: <CALmnu0a_HmgJXa9aYbCpyRhAb5h_FrLyhVUeeojMJa0tPUTHTg@mail.gmail.com>
On Mon, Dec 15, 2014 at 03:05:04PM -0500, Amordea Whiteoak wrote:
> > Strictly speaking, there is no right click button on clickpads. We
> > emulate one with the software buttons in Xorg.
> > IIRC, there were some "workarounds" in your original bugs which
> > consist in disabling the software buttons area. Can you make sure that
> > there is no left over from this?
>
> I learn new things about clickpads every day. :)
>
> To my knowledge, those configurations changes should reset upon reboot
> and I never committed any of those lines to any of my startup files.
> So we should be good there, I think. Just in case, I am enclosing the
> output of my synclient -l, which should be the default configuration.
there are at least two options in there that are non-default, so maybe other
options are different too. an xorg.log would be more useful here, or make
sure that you don't have any custom configs in /etc/X11/xorg.conf.d/
or an xorg.conf
> > If this still doesn't work, we wil need to check that the X and Y
> > ranges reported by your clickpad are accurate and that the software
> > buttons are properly activated.
>
> I don't know the testing methodology to use to provide positioning
> data for my trackpad, but let me know if you need that and how you
> want me to get it and I will be happy to provide.
Two options:
* use evtest or evemu-record to record the data and look at the output, or
simpler
* use libevdev's touchpad-edge-detector tool to scope out the min/max ranges
of your touchpad
Cheers,
Peter
^ permalink raw reply
* Re: [PATCH 2/2] HID: logitech-hidpp: store the name of the device in struct hidpp
From: Peter Wu @ 2014-12-17 1:18 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Jiri Kosina, Nestor Lopez Casado, Peter Hutterer, linux-input,
linux-kernel
In-Reply-To: <1418767562-4136-2-git-send-email-benjamin.tissoires@redhat.com>
On Tuesday 16 December 2014 17:06:02 Benjamin Tissoires wrote:
> If a disconnect occurs while getting the actual name of the device
> (which can take several HID transactions), the name of the device will
> be the hid name, provided by the Unifying Receiver.
> This means that in some cases, the user space will see a different
> name that what it usually sees when there is no disconnect.
>
> We should store the name of the device in the struct hidpp. That way,
> if a disconnect occurs while we are accessing the name,
> hidpp_connect_event() can fail, and the input node is not created.
>
> The input node will be created only if we have a connection which
> lasts long enough to retrieve all the requested information:
> name, protocol, and specific configuration.
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---
> drivers/hid/hid-logitech-hidpp.c | 31 ++++++++++++++++++++-----------
> 1 file changed, 20 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> index c0fb5fe..4bc8714 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
> @@ -89,6 +89,7 @@ struct hidpp_device {
> struct hid_device *hid_dev;
> struct mutex send_mutex;
> void *send_receive_buf;
> + char *name;
> wait_queue_head_t wait;
> bool answer_available;
> u8 protocol_major;
> @@ -1087,6 +1088,7 @@ static void hidpp_input_close(struct input_dev *dev)
> static struct input_dev *hidpp_allocate_input(struct hid_device *hdev)
> {
> struct input_dev *input_dev = devm_input_allocate_device(&hdev->dev);
> + struct hidpp_device *hidpp = hid_get_drvdata(hdev);
>
> if (!input_dev)
> return NULL;
> @@ -1095,7 +1097,7 @@ static struct input_dev *hidpp_allocate_input(struct hid_device *hdev)
> input_dev->open = hidpp_input_open;
> input_dev->close = hidpp_input_close;
>
> - input_dev->name = hdev->name;
> + input_dev->name = hidpp->name;
> input_dev->phys = hdev->phys;
> input_dev->uniq = hdev->uniq;
> input_dev->id.bustype = hdev->bus;
> @@ -1137,22 +1139,28 @@ static void hidpp_connect_event(struct hidpp_device *hidpp)
> hid_info(hdev, "HID++ %u.%u device connected.\n",
> hidpp->protocol_major, hidpp->protocol_minor);
>
> + if (!hidpp->name || hidpp->name == hdev->name) {
Hm, is it ever possible that hidpp->name is NULL? probe sets the pointer
to an array (hdev->name). Defence in depth I guess, but perhaps a
comment could clarify this?
Otherwise the changes look OK. I have tested this situation:
1. Insert receiver
2. Return a HID++ version for the WTP.
3. Return -9 (ResourceError) for the device name feature request (via
the QEMU emulation).
4. Observe that this fails.
So maybe you could add a comment for the above and add these tags:
Reviewed-by: Peter Wu <peter@lekensteyn.nl>
Tested-by: Peter Wu <peter@lekensteyn.nl>
Kind regards,
Peter
> + name = hidpp_get_device_name(hidpp);
> + if (!name) {
> + hid_err(hdev,
> + "unable to retrieve the name of the device");
> + return;
> + }
> +
> + devm_name = devm_kasprintf(&hdev->dev, GFP_KERNEL, "%s", name);
> + kfree(name);
> + if (!devm_name)
> + return;
> +
> + hidpp->name = devm_name;
> + }
> +
> input = hidpp_allocate_input(hdev);
> if (!input) {
> hid_err(hdev, "cannot allocate new input device: %d\n", ret);
> return;
> }
>
> - name = hidpp_get_device_name(hidpp);
> - if (!name) {
> - hid_err(hdev, "unable to retrieve the name of the device");
> - } else {
> - devm_name = devm_kasprintf(&hdev->dev, GFP_KERNEL, "%s", name);
> - if (devm_name)
> - input->name = devm_name;
> - kfree(name);
> - }
> -
> hidpp_populate_input(hidpp, input, false);
>
> ret = input_register_device(input);
> @@ -1175,6 +1183,7 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
> return -ENOMEM;
>
> hidpp->hid_dev = hdev;
> + hidpp->name = hdev->name;
> hid_set_drvdata(hdev, hidpp);
>
> hidpp->quirks = id->driver_data;
>
^ permalink raw reply
* Re: [PATCH 1/2] HID: logitech-hidpp: bail out if wtp_connect fails
From: Peter Wu @ 2014-12-17 1:28 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Benjamin Tissoires, Jiri Kosina, Nestor Lopez Casado, linux-input,
linux-kernel@vger.kernel.org
In-Reply-To: <CAN+gG=F0HhzXQhsaHTEb5Hy2CVTsoOSopCt4grXFnSKt+tPsWw@mail.gmail.com>
Hi Benjamin,
On Tuesday 16 December 2014 17:13:05 Benjamin Tissoires wrote:
> the logitech patches are queuing up really fast.
> To keep track of them, I made a bundle on patchwork:
> https://patchwork.kernel.org/bundle/bentiss/hid-logitech-hidpp/
> (/me discovered a new tool to play with)
>
> Right now, the patch "HID: logitech-hidpp: detect HID++ 2.0 errors
> too" is waiting for Logitech's approval, and the 2 of this series need
> review.
>
> Peter, please tell me if I missed one patch.
Nice, it would be even better if a bundle could be bookmarked, or if a
group could set review flags in this bundle :-)
There are no missing patches from my side. All changes (based on
jikos/hid, branch for-next) are at
https://git.lekensteyn.nl/peter/linux/log/?h=logitech-hidpp
and are tested in QEMU with an emulated device and a real device (with
T400/T650/M525 paired).
I noticed that all devices would immediately get an input device (even
if they were off), except for the T650. This apparently happens because
the touchpad configuration cannot be retrieved or when the touchpad
cannot be put in raw reporting mode. I cannot think of something to
"fix" this though.
--
Kind regards,
Peter
https://lekensteyn.nl
^ permalink raw reply
* Re: [PATCH 1/2] HID: logitech-hidpp: bail out if wtp_connect fails
From: Peter Wu @ 2014-12-17 1:32 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Jiri Kosina, Nestor Lopez Casado, Peter Hutterer, linux-input,
linux-kernel
In-Reply-To: <1771836.oYfhfEolNx@al>
Sorry for the rapid mail, I forgot to mention something.
wtp_connect won't work on non-HID++ devices. What about moving it down,
between the generic routines (reading protocol and name) and
hidpp_allocate_input? Then the connected parameter can also be dropped.
Kind regards,
Peter
On Wednesday 17 December 2014 00:33:55 Peter Wu wrote:
> On Tuesday 16 December 2014 17:06:01 Benjamin Tissoires wrote:
> > If wtp_connect() fails, that means most of the time that the device has
> > been disconnected. Subsequent attempts to contact the device will fail
> > too, so it's simpler to bail out earlier.
> >
> > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> > ---
> > drivers/hid/hid-logitech-hidpp.c | 15 +++++++++------
> > 1 file changed, 9 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> > index d008d71..c0fb5fe 100644
> > --- a/drivers/hid/hid-logitech-hidpp.c
> > +++ b/drivers/hid/hid-logitech-hidpp.c
> > @@ -914,24 +914,24 @@ static int wtp_allocate(struct hid_device *hdev, const struct hid_device_id *id)
> > return 0;
> > };
> >
> > -static void wtp_connect(struct hid_device *hdev, bool connected)
> > +static int wtp_connect(struct hid_device *hdev, bool connected)
> > {
> > struct hidpp_device *hidpp = hid_get_drvdata(hdev);
> > struct wtp_data *wd = hidpp->private_data;
> > int ret;
> >
> > if (!connected)
> > - return;
> > + return 0;
>
> "0" is success, what about -1 or -ENODEV here to signal an error? The
> following line (in hidpp_connect_event) returns on !connected, but that
> is no reason to return 0 here.
>
> ("No connection" sounds like an error condition to me as "[wtp_]connect"
> cannot do something meaningful.)
>
> Whether or not you change the return value is up to you. This patch is
> Reviewed-by: Peter Wu <peter@lekensteyn.nl>
>
> Kind regards,
> Peter
>
> > if (!wd->x_size) {
> > ret = wtp_get_config(hidpp);
> > if (ret) {
> > hid_err(hdev, "Can not get wtp config: %d\n", ret);
> > - return;
> > + return ret;
> > }
> > }
> >
> > - hidpp_touchpad_set_raw_report_state(hidpp, wd->mt_feature_index,
> > + return hidpp_touchpad_set_raw_report_state(hidpp, wd->mt_feature_index,
> > true, true);
> > }
> >
> > @@ -1115,8 +1115,11 @@ static void hidpp_connect_event(struct hidpp_device *hidpp)
> > struct input_dev *input;
> > char *name, *devm_name;
> >
> > - if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)
> > - wtp_connect(hdev, connected);
> > + if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP) {
> > + ret = wtp_connect(hdev, connected);
> > + if (ret)
> > + return;
> > + }
> >
> > if (!connected || hidpp->delayed_input)
> > return;
> >
^ 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