Linux Input/HID development
 help / color / mirror / Atom feed
* Behaviour of input-polldev?
From: Florian Echtler @ 2013-09-03 11:39 UTC (permalink / raw)
  To: linux-input

[-- Attachment #1: Type: text/plain, Size: 528 bytes --]

Hello everyone,

I'm in the process of writing an input driver for the Microsoft
Pixelsense (formerly Surface 2.0). The device only has bulk endpoints
and consequently needs to be polled regularly. My beta driver uses
input-polldev for this, and it appears to work nicely, however, I
couldn't find out how input-polldev behaves if one polling cycle takes
longer than expected and a new poll would already be triggered while the
first one is still running?

Thanks & BR, Florian
-- 
SENT FROM MY DEC VT50 TERMINAL


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply

* Re: [PATCH] input: allow SERIO=m even without EXPERT=y
From: Tom Gundersen @ 2013-09-02 21:46 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: richard -rw- weinberger, linux-input@vger.kernel.org, LKML,
	Dmitry Torokhov
In-Reply-To: <CAMuHMdU1Dt2KOJny78d2DdprHwF5cOQEMGDm7rVDMRyL1jTAsA@mail.gmail.com>

On Mon, Sep 2, 2013 at 11:30 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Mon, Sep 2, 2013 at 11:01 PM, richard -rw- weinberger
> <richard.weinberger@gmail.com> wrote:
>> On Mon, Sep 2, 2013 at 8:47 PM, Tom Gundersen <teg@jklm.no> wrote:
>>> There is plenty of consumer hardware (e.g., mac books) that does not use
>>> AT keyboards or PS/2 mice. It therefore makes sense for distro kernels
>>> to build the related drivers as modules to avoid loading them on hardware
>>> that does not need them. As such, these options should no longer be protected
>>> by EXPERT.
>>>
>>> Moreover, building these drivers as modules gets rid of the following ugly
>>> error during boot:
>>>
>>> [    2.337745] i8042: PNP: No PS/2 controller found. Probing ports directly.
>>> [    3.439537] i8042: No controller found
>>>
>>> Signed-off-by: Tom Gundersen <teg@jklm.no>
>>> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>>> ---
>>>  drivers/input/keyboard/Kconfig | 4 ++--
>>>  drivers/input/serio/Kconfig    | 6 +++---
>>>  2 files changed, 5 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
>>> index 269d4c3..2d31cec 100644
>>> --- a/drivers/input/keyboard/Kconfig
>>> +++ b/drivers/input/keyboard/Kconfig
>>> @@ -2,7 +2,7 @@
>>>  # Input core configuration
>>>  #
>>>  menuconfig INPUT_KEYBOARD
>>> -       bool "Keyboards" if EXPERT || !X86
>>> +       bool "Keyboards"
>>
>> Maybe this a stupid question, but why are you dropping the "!X86" too?
>
> Because the "if EXPERT" only mattered for X86.
>
> Apply De Morgan and it read like "do not ask the question if !EXPERT && X86".

Precisely.

-t

^ permalink raw reply

* Re: [PATCH] input: allow SERIO=m even without EXPERT=y
From: Geert Uytterhoeven @ 2013-09-02 21:30 UTC (permalink / raw)
  To: richard -rw- weinberger
  Cc: Tom Gundersen, linux-input@vger.kernel.org, LKML, Dmitry Torokhov
In-Reply-To: <CAFLxGvy+oBg-Qbd_8wL-86ODpB9fand2aVcyVcRRLarRe40wSw@mail.gmail.com>

On Mon, Sep 2, 2013 at 11:01 PM, richard -rw- weinberger
<richard.weinberger@gmail.com> wrote:
> On Mon, Sep 2, 2013 at 8:47 PM, Tom Gundersen <teg@jklm.no> wrote:
>> There is plenty of consumer hardware (e.g., mac books) that does not use
>> AT keyboards or PS/2 mice. It therefore makes sense for distro kernels
>> to build the related drivers as modules to avoid loading them on hardware
>> that does not need them. As such, these options should no longer be protected
>> by EXPERT.
>>
>> Moreover, building these drivers as modules gets rid of the following ugly
>> error during boot:
>>
>> [    2.337745] i8042: PNP: No PS/2 controller found. Probing ports directly.
>> [    3.439537] i8042: No controller found
>>
>> Signed-off-by: Tom Gundersen <teg@jklm.no>
>> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>> ---
>>  drivers/input/keyboard/Kconfig | 4 ++--
>>  drivers/input/serio/Kconfig    | 6 +++---
>>  2 files changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
>> index 269d4c3..2d31cec 100644
>> --- a/drivers/input/keyboard/Kconfig
>> +++ b/drivers/input/keyboard/Kconfig
>> @@ -2,7 +2,7 @@
>>  # Input core configuration
>>  #
>>  menuconfig INPUT_KEYBOARD
>> -       bool "Keyboards" if EXPERT || !X86
>> +       bool "Keyboards"
>
> Maybe this a stupid question, but why are you dropping the "!X86" too?

Because the "if EXPERT" only mattered for X86.

Apply De Morgan and it read like "do not ask the question if !EXPERT && X86".

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH] input: allow SERIO=m even without EXPERT=y
From: richard -rw- weinberger @ 2013-09-02 21:01 UTC (permalink / raw)
  To: Tom Gundersen; +Cc: linux-input@vger.kernel.org, LKML, Dmitry Torokhov
In-Reply-To: <1378147630-12471-1-git-send-email-teg@jklm.no>

On Mon, Sep 2, 2013 at 8:47 PM, Tom Gundersen <teg@jklm.no> wrote:
> There is plenty of consumer hardware (e.g., mac books) that does not use
> AT keyboards or PS/2 mice. It therefore makes sense for distro kernels
> to build the related drivers as modules to avoid loading them on hardware
> that does not need them. As such, these options should no longer be protected
> by EXPERT.
>
> Moreover, building these drivers as modules gets rid of the following ugly
> error during boot:
>
> [    2.337745] i8042: PNP: No PS/2 controller found. Probing ports directly.
> [    3.439537] i8042: No controller found
>
> Signed-off-by: Tom Gundersen <teg@jklm.no>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>  drivers/input/keyboard/Kconfig | 4 ++--
>  drivers/input/serio/Kconfig    | 6 +++---
>  2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
> index 269d4c3..2d31cec 100644
> --- a/drivers/input/keyboard/Kconfig
> +++ b/drivers/input/keyboard/Kconfig
> @@ -2,7 +2,7 @@
>  # Input core configuration
>  #
>  menuconfig INPUT_KEYBOARD
> -       bool "Keyboards" if EXPERT || !X86
> +       bool "Keyboards"

Maybe this a stupid question, but why are you dropping the "!X86" too?

>         default y
>         help
>           Say Y here, and a list of supported keyboards will be displayed.
> @@ -67,7 +67,7 @@ config KEYBOARD_ATARI
>           module will be called atakbd.
>
>  config KEYBOARD_ATKBD
> -       tristate "AT keyboard" if EXPERT || !X86
> +       tristate "AT keyboard"
>         default y
>         select SERIO
>         select SERIO_LIBPS2
> diff --git a/drivers/input/serio/Kconfig b/drivers/input/serio/Kconfig
> index 1e691a3..39c869d 100644
> --- a/drivers/input/serio/Kconfig
> +++ b/drivers/input/serio/Kconfig
> @@ -2,7 +2,7 @@
>  # Input core configuration
>  #
>  config SERIO
> -       tristate "Serial I/O support" if EXPERT || !X86
> +       tristate "Serial I/O support"
>         default y
>         help
>           Say Yes here if you have any input device that uses serial I/O to
> @@ -19,7 +19,7 @@ config SERIO
>  if SERIO
>
>  config SERIO_I8042
> -       tristate "i8042 PC Keyboard controller" if EXPERT || !X86
> +       tristate "i8042 PC Keyboard controller"
>         default y
>         depends on !PARISC && (!ARM || ARCH_SHARK || FOOTBRIDGE_HOST) && \
>                    (!SUPERH || SH_CAYMAN) && !M68K && !BLACKFIN && !S390 && \
> @@ -170,7 +170,7 @@ config SERIO_MACEPS2
>           module will be called maceps2.
>
>  config SERIO_LIBPS2
> -       tristate "PS/2 driver library" if EXPERT
> +       tristate "PS/2 driver library"
>         depends on SERIO_I8042 || SERIO_I8042=n
>         help
>           Say Y here if you are using a driver for device connected
> --
> 1.8.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/



-- 
Thanks,
//richard

^ permalink raw reply

* [PATCH] input: allow SERIO=m even without EXPERT=y
From: Tom Gundersen @ 2013-09-02 18:47 UTC (permalink / raw)
  To: linux-input; +Cc: linux-kernel, Tom Gundersen, Dmitry Torokhov

There is plenty of consumer hardware (e.g., mac books) that does not use
AT keyboards or PS/2 mice. It therefore makes sense for distro kernels
to build the related drivers as modules to avoid loading them on hardware
that does not need them. As such, these options should no longer be protected
by EXPERT.

Moreover, building these drivers as modules gets rid of the following ugly
error during boot:

[    2.337745] i8042: PNP: No PS/2 controller found. Probing ports directly.
[    3.439537] i8042: No controller found

Signed-off-by: Tom Gundersen <teg@jklm.no>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/keyboard/Kconfig | 4 ++--
 drivers/input/serio/Kconfig    | 6 +++---
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index 269d4c3..2d31cec 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -2,7 +2,7 @@
 # Input core configuration
 #
 menuconfig INPUT_KEYBOARD
-	bool "Keyboards" if EXPERT || !X86
+	bool "Keyboards"
 	default y
 	help
 	  Say Y here, and a list of supported keyboards will be displayed.
@@ -67,7 +67,7 @@ config KEYBOARD_ATARI
 	  module will be called atakbd.
 
 config KEYBOARD_ATKBD
-	tristate "AT keyboard" if EXPERT || !X86
+	tristate "AT keyboard"
 	default y
 	select SERIO
 	select SERIO_LIBPS2
diff --git a/drivers/input/serio/Kconfig b/drivers/input/serio/Kconfig
index 1e691a3..39c869d 100644
--- a/drivers/input/serio/Kconfig
+++ b/drivers/input/serio/Kconfig
@@ -2,7 +2,7 @@
 # Input core configuration
 #
 config SERIO
-	tristate "Serial I/O support" if EXPERT || !X86
+	tristate "Serial I/O support"
 	default y
 	help
 	  Say Yes here if you have any input device that uses serial I/O to
@@ -19,7 +19,7 @@ config SERIO
 if SERIO
 
 config SERIO_I8042
-	tristate "i8042 PC Keyboard controller" if EXPERT || !X86
+	tristate "i8042 PC Keyboard controller"
 	default y
 	depends on !PARISC && (!ARM || ARCH_SHARK || FOOTBRIDGE_HOST) && \
 		   (!SUPERH || SH_CAYMAN) && !M68K && !BLACKFIN && !S390 && \
@@ -170,7 +170,7 @@ config SERIO_MACEPS2
 	  module will be called maceps2.
 
 config SERIO_LIBPS2
-	tristate "PS/2 driver library" if EXPERT
+	tristate "PS/2 driver library"
 	depends on SERIO_I8042 || SERIO_I8042=n
 	help
 	  Say Y here if you are using a driver for device connected
-- 
1.8.4


^ permalink raw reply related

* Re: [PATCH] hid: uhid: improve uhid example client
From: David Herrmann @ 2013-09-02 13:21 UTC (permalink / raw)
  To: Benjamin Tissoires; +Cc: linux-input, Jiri Kosina, Anderson Lizardo
In-Reply-To: <CAN+gG=GbY9mQGQhRZcf9Osc1q5YjwpGVExhH6pAN9X0urcRv0w@mail.gmail.com>

Hi

On Mon, Sep 2, 2013 at 3:16 PM, Benjamin Tissoires
<benjamin.tissoires@gmail.com> wrote:
> Hi David,
>
> On Sun, Sep 1, 2013 at 11:45 PM, David Herrmann <dh.herrmann@gmail.com> wrote:
>> This extends the uhid example client. It properly documents the built-in
>> report-descriptor an adds explicit report-numbers.
>>
>> Furthermore, LED output reports are added to utilize the new UHID output
>> reports of the kernel. Support for 3 basic LEDs is added and a small
>> report-parser to print debug messages if output reports were received.
>>
>> To test this, simply write the EV_LED+LED_CAPSL+1 event to the evdev
>> device-node of the uhid-device and the kernel will forward it to your uhid
>> client.
>>
>> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
>> ---
>> Hi Jiri
>>
>> This is actually the first time that I wrote an HID report-descriptor. The old
>> 3-button mouse was just copied from a real mouse. My descriptor works perfectly
>> well but I am kind of scared I am no longer allowed to make fun of firmware
>> authors..
>
> If you are affraid of legal issues, you should consider using the
> keyboard report descriptor given as an example on the page 69
> (Appendix E.6) of the USB HID specification 1.1.

Not afraid of legal issues at all. I reviewed it thoroughly,
documented the new one and I added my own extensions (like WHEEL). So
it is no longer based on the old one. But thanks for the usb-spec
hint, haven't looked there, yet.

> Other than that, I had also to deal with raw reports and report
> descriptors, so I have written some tools to decode them on:
> https://github.com/bentiss/hid-replay/tree/master/tools
> I am expecting in input the hid-replay file format, which you can
> retrieve an example from hid-recorder, and from the man.
>
> It's not 100% accurate (I had corner cases for some keyboards), but
> it's doing the decoding job quite properly.
> I am also thinking of a way to "reencode" it more efficiently from the
> human readable version, but this is not done yet.

Cool! Encoding-support is actually what I need, but the decoding is
nice to verify correctness.

>>
>> Anyway, I extended the example to have a test-case for uhid output reports and
>> it all works as expected. I got asked for some help on this so I wrote up this
>> patch to push it mainline.
>
> Great thanks!
>
> On overall, the example looks sane, but I will not be able to do an
> exhaustive review before Friday. So Jiri, I think you can take it
> unless David decides to change the report descriptors with the one I
> mentioned earlier.

No intention to modify it. Looks all good to me now.

Thanks!
David

^ permalink raw reply

* Re: [PATCH] hid: uhid: improve uhid example client
From: Benjamin Tissoires @ 2013-09-02 13:16 UTC (permalink / raw)
  To: David Herrmann; +Cc: linux-input, Jiri Kosina, Anderson Lizardo
In-Reply-To: <1378071918-1152-1-git-send-email-dh.herrmann@gmail.com>

Hi David,

On Sun, Sep 1, 2013 at 11:45 PM, David Herrmann <dh.herrmann@gmail.com> wrote:
> This extends the uhid example client. It properly documents the built-in
> report-descriptor an adds explicit report-numbers.
>
> Furthermore, LED output reports are added to utilize the new UHID output
> reports of the kernel. Support for 3 basic LEDs is added and a small
> report-parser to print debug messages if output reports were received.
>
> To test this, simply write the EV_LED+LED_CAPSL+1 event to the evdev
> device-node of the uhid-device and the kernel will forward it to your uhid
> client.
>
> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
> ---
> Hi Jiri
>
> This is actually the first time that I wrote an HID report-descriptor. The old
> 3-button mouse was just copied from a real mouse. My descriptor works perfectly
> well but I am kind of scared I am no longer allowed to make fun of firmware
> authors..

If you are affraid of legal issues, you should consider using the
keyboard report descriptor given as an example on the page 69
(Appendix E.6) of the USB HID specification 1.1.

Other than that, I had also to deal with raw reports and report
descriptors, so I have written some tools to decode them on:
https://github.com/bentiss/hid-replay/tree/master/tools
I am expecting in input the hid-replay file format, which you can
retrieve an example from hid-recorder, and from the man.

It's not 100% accurate (I had corner cases for some keyboards), but
it's doing the decoding job quite properly.
I am also thinking of a way to "reencode" it more efficiently from the
human readable version, but this is not done yet.

>
> Anyway, I extended the example to have a test-case for uhid output reports and
> it all works as expected. I got asked for some help on this so I wrote up this
> patch to push it mainline.

Great thanks!

On overall, the example looks sane, but I will not be able to do an
exhaustive review before Friday. So Jiri, I think you can take it
unless David decides to change the report descriptors with the one I
mentioned earlier.

Cheers,
Benjamin

>
> Cheers
> David
>
> #include <assert.h>
> #include <linux/input.h>
> #include <fcntl.h>
> #include <unistd.h>
> #include <string.h>
> #include <stdlib.h>
>
> int main(int argc, char **argv)
> {
>         int fd, r;
>         struct input_event ev;
>
>         assert(argc >= 3);
>
>         fd = open(argv[1], O_RDWR | O_CLOEXEC);
>         assert(fd >= 0);
>
>         memset(&ev, 0, sizeof(ev));
>         ev.type = EV_LED;
>         ev.code = LED_CAPSL;
>         ev.value = atoi(argv[2]);
>
>         r = write(fd, &ev, sizeof(ev));
>         assert(r == sizeof(ev));
>
>         close(fd);
>
>         return 0;
> }
>
>  samples/uhid/uhid-example.c | 123 +++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 103 insertions(+), 20 deletions(-)
>
> diff --git a/samples/uhid/uhid-example.c b/samples/uhid/uhid-example.c
> index 03ce3c0..7d58a4b 100644
> --- a/samples/uhid/uhid-example.c
> +++ b/samples/uhid/uhid-example.c
> @@ -1,14 +1,15 @@
>  /*
>   * UHID Example
>   *
> - * Copyright (c) 2012 David Herrmann <dh.herrmann@googlemail.com>
> + * Copyright (c) 2012-2013 David Herrmann <dh.herrmann@gmail.com>
>   *
>   * The code may be used by anyone for any purpose,
>   * and can serve as a starting point for developing
>   * applications using uhid.
>   */
>
> -/* UHID Example
> +/*
> + * UHID Example
>   * This example emulates a basic 3 buttons mouse with wheel over UHID. Run this
>   * program as root and then use the following keys to control the mouse:
>   *   q: Quit the application
> @@ -22,6 +23,11 @@
>   *   r: Move wheel up
>   *   f: Move wheel down
>   *
> + * Additionally to 3 button mouse, 3 keyboard LEDs are also supported (LED_NUML,
> + * LED_CAPSL and LED_SCROLLL). The device doesn't generate any related keyboard
> + * events, though. You need to manually write the EV_LED/LED_XY/1 activation
> + * input event to the evdev device to see it being sent to this device.
> + *
>   * If uhid is not available as /dev/uhid, then you can pass a different path as
>   * first argument.
>   * If <linux/uhid.h> is not installed in /usr, then compile this with:
> @@ -41,11 +47,12 @@
>  #include <unistd.h>
>  #include <linux/uhid.h>
>
> -/* HID Report Desciptor
> - * We emulate a basic 3 button mouse with wheel. This is the report-descriptor
> - * as the kernel will parse it:
> +/*
> + * HID Report Desciptor
> + * We emulate a basic 3 button mouse with wheel and 3 keyboard LEDs. This is
> + * the report-descriptor as the kernel will parse it:
>   *
> - * INPUT[INPUT]
> + * INPUT(1)[INPUT]
>   *   Field(0)
>   *     Physical(GenericDesktop.Pointer)
>   *     Application(GenericDesktop.Mouse)
> @@ -72,6 +79,19 @@
>   *     Report Count(3)
>   *     Report Offset(8)
>   *     Flags( Variable Relative )
> + * OUTPUT(2)[OUTPUT]
> + *   Field(0)
> + *     Application(GenericDesktop.Keyboard)
> + *     Usage(3)
> + *       LED.NumLock
> + *       LED.CapsLock
> + *       LED.ScrollLock
> + *     Logical Minimum(0)
> + *     Logical Maximum(1)
> + *     Report Size(1)
> + *     Report Count(3)
> + *     Report Offset(0)
> + *     Flags( Variable Absolute )
>   *
>   * This is the mapping that we expect:
>   *   Button.0001 ---> Key.LeftBtn
> @@ -80,19 +100,59 @@
>   *   GenericDesktop.X ---> Relative.X
>   *   GenericDesktop.Y ---> Relative.Y
>   *   GenericDesktop.Wheel ---> Relative.Wheel
> + *   LED.NumLock ---> LED.NumLock
> + *   LED.CapsLock ---> LED.CapsLock
> + *   LED.ScrollLock ---> LED.ScrollLock
>   *
>   * This information can be verified by reading /sys/kernel/debug/hid/<dev>/rdesc
>   * This file should print the same information as showed above.
>   */
>
>  static unsigned char rdesc[] = {
> -       0x05, 0x01, 0x09, 0x02, 0xa1, 0x01, 0x09, 0x01,
> -       0xa1, 0x00, 0x05, 0x09, 0x19, 0x01, 0x29, 0x03,
> -       0x15, 0x00, 0x25, 0x01, 0x95, 0x03, 0x75, 0x01,
> -       0x81, 0x02, 0x95, 0x01, 0x75, 0x05, 0x81, 0x01,
> -       0x05, 0x01, 0x09, 0x30, 0x09, 0x31, 0x09, 0x38,
> -       0x15, 0x80, 0x25, 0x7f, 0x75, 0x08, 0x95, 0x03,
> -       0x81, 0x06, 0xc0, 0xc0,
> +       0x05, 0x01,     /* USAGE_PAGE (Generic Desktop) */
> +       0x09, 0x02,     /* USAGE (Mouse) */
> +       0xa1, 0x01,     /* COLLECTION (Application) */
> +       0x09, 0x01,             /* USAGE (Pointer) */
> +       0xa1, 0x00,             /* COLLECTION (Physical) */
> +       0x85, 0x01,                     /* REPORT_ID (1) */
> +       0x05, 0x09,                     /* USAGE_PAGE (Button) */
> +       0x19, 0x01,                     /* USAGE_MINIMUM (Button 1) */
> +       0x29, 0x03,                     /* USAGE_MAXIMUM (Button 3) */
> +       0x15, 0x00,                     /* LOGICAL_MINIMUM (0) */
> +       0x25, 0x01,                     /* LOGICAL_MAXIMUM (1) */
> +       0x95, 0x03,                     /* REPORT_COUNT (3) */
> +       0x75, 0x01,                     /* REPORT_SIZE (1) */
> +       0x81, 0x02,                     /* INPUT (Data,Var,Abs) */
> +       0x95, 0x01,                     /* REPORT_COUNT (1) */
> +       0x75, 0x05,                     /* REPORT_SIZE (5) */
> +       0x81, 0x01,                     /* INPUT (Cnst,Var,Abs) */
> +       0x05, 0x01,                     /* USAGE_PAGE (Generic Desktop) */
> +       0x09, 0x30,                     /* USAGE (X) */
> +       0x09, 0x31,                     /* USAGE (Y) */
> +       0x09, 0x38,                     /* USAGE (WHEEL) */
> +       0x15, 0x81,                     /* LOGICAL_MINIMUM (-127) */
> +       0x25, 0x7f,                     /* LOGICAL_MAXIMUM (127) */
> +       0x75, 0x08,                     /* REPORT_SIZE (8) */
> +       0x95, 0x03,                     /* REPORT_COUNT (3) */
> +       0x81, 0x06,                     /* INPUT (Data,Var,Rel) */
> +       0xc0,                   /* END_COLLECTION */
> +       0xc0,           /* END_COLLECTION */
> +       0x05, 0x01,     /* USAGE_PAGE (Generic Desktop) */
> +       0x09, 0x06,     /* USAGE (Keyboard) */
> +       0xa1, 0x01,     /* COLLECTION (Application) */
> +       0x85, 0x02,             /* REPORT_ID (2) */
> +       0x05, 0x08,             /* USAGE_PAGE (Led) */
> +       0x19, 0x01,             /* USAGE_MINIMUM (1) */
> +       0x29, 0x03,             /* USAGE_MAXIMUM (3) */
> +       0x15, 0x00,             /* LOGICAL_MINIMUM (0) */
> +       0x25, 0x01,             /* LOGICAL_MAXIMUM (1) */
> +       0x95, 0x03,             /* REPORT_COUNT (3) */
> +       0x75, 0x01,             /* REPORT_SIZE (1) */
> +       0x91, 0x02,             /* Output (Data,Var,Abs) */
> +       0x95, 0x01,             /* REPORT_COUNT (1) */
> +       0x75, 0x05,             /* REPORT_SIZE (5) */
> +       0x91, 0x01,             /* Output (Cnst,Var,Abs) */
> +       0xc0,           /* END_COLLECTION */
>  };
>
>  static int uhid_write(int fd, const struct uhid_event *ev)
> @@ -140,6 +200,27 @@ static void destroy(int fd)
>         uhid_write(fd, &ev);
>  }
>
> +/* This parses raw output reports sent by the kernel to the device. A normal
> + * uhid program shouldn't do this but instead just forward the raw report.
> + * However, for ducomentational purposes, we try to detect LED events here and
> + * print debug messages for it. */
> +static void handle_output(struct uhid_event *ev)
> +{
> +       /* LED messages are adverised via OUTPUT reports; ignore the rest */
> +       if (ev->u.output.rtype != UHID_OUTPUT_REPORT)
> +               return;
> +       /* LED reports have length 2 bytes */
> +       if (ev->u.output.size != 2)
> +               return;
> +       /* first byte is report-id which is 0x02 for LEDs in our rdesc */
> +       if (ev->u.output.data[0] != 0x2)
> +               return;
> +
> +       /* print flags payload */
> +       fprintf(stderr, "LED output report received with flags %x\n",
> +               ev->u.output.data[1]);
> +}
> +
>  static int event(int fd)
>  {
>         struct uhid_event ev;
> @@ -174,6 +255,7 @@ static int event(int fd)
>                 break;
>         case UHID_OUTPUT:
>                 fprintf(stderr, "UHID_OUTPUT from uhid-dev\n");
> +               handle_output(&ev);
>                 break;
>         case UHID_OUTPUT_EV:
>                 fprintf(stderr, "UHID_OUTPUT_EV from uhid-dev\n");
> @@ -198,18 +280,19 @@ static int send_event(int fd)
>
>         memset(&ev, 0, sizeof(ev));
>         ev.type = UHID_INPUT;
> -       ev.u.input.size = 4;
> +       ev.u.input.size = 5;
>
> +       ev.u.input.data[0] = 0x1;
>         if (btn1_down)
> -               ev.u.input.data[0] |= 0x1;
> +               ev.u.input.data[1] |= 0x1;
>         if (btn2_down)
> -               ev.u.input.data[0] |= 0x2;
> +               ev.u.input.data[1] |= 0x2;
>         if (btn3_down)
> -               ev.u.input.data[0] |= 0x4;
> +               ev.u.input.data[1] |= 0x4;
>
> -       ev.u.input.data[1] = abs_hor;
> -       ev.u.input.data[2] = abs_ver;
> -       ev.u.input.data[3] = wheel;
> +       ev.u.input.data[2] = abs_hor;
> +       ev.u.input.data[3] = abs_ver;
> +       ev.u.input.data[4] = wheel;
>
>         return uhid_write(fd, &ev);
>  }
> --
> 1.8.4
>
> --
> 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 11/14] HID: multitouch: validate feature report details
From: Benjamin Tissoires @ 2013-09-02 12:56 UTC (permalink / raw)
  To: Kees Cook; +Cc: Benjamin Tissoires, Jiri Kosina, linux-input, Henrik Rydberg
In-Reply-To: <CAGXu5jKAob5s1vs5s974QsJWdje2rQgyz=PVACxW3aWiJWiEEA@mail.gmail.com>

On Fri, Aug 30, 2013 at 8:27 PM, Kees Cook <keescook@chromium.org> wrote:
> On Fri, Aug 30, 2013 at 8:27 AM, Benjamin Tissoires
> <benjamin.tissoires@gmail.com> wrote:
>> On Thu, Aug 29, 2013 at 9:41 PM, Kees Cook <keescook@chromium.org> wrote:
>>> On Thu, Aug 29, 2013 at 1:59 AM, Benjamin Tissoires
>>> <benjamin.tissoires@redhat.com> wrote:
>>>> Hi Kees,
>>>>
>>>> I would be curious to have the HID report descriptors (maybe off list)
>>>> to understand how things can be that bad.
>>>
>>> Certainly! I'll send them your way. I did have to get pretty creative
>>> to tickle these conditions.
>>>
>>>> On overall, I'd prefer all those checks to be in hid-core so that we
>>>> have the guarantee that we don't have to open a new CVE each time a
>>>> specific hid driver do not check for these ranges.
>>>
>>> I pondered doing this, but it seemed like something that needed wider
>>> discussion, so I thought I'd start with just the dump of fixes. It
>>> seems like the entire HID report interface should use access functions
>>> to enforce range checking -- perhaps further enforced by making the
>>> structure opaque to the drivers.
>>
>> The problem with access functions with range checking is when they are
>> used at each input report. This will overload the kernel processing
>> for static checks.
>>
>>>
>>>> More specific comments inlined:
>>>>
>>>> On 28/08/13 22:31, Jiri Kosina wrote:
>>>>> From: Kees Cook <keescook@chromium.org>
>>>>>
>>>>> When working on report indexes, always validate that they are in bounds.
>>>>> Without this, a HID device could report a malicious feature report that
>>>>> could trick the driver into a heap overflow:
>>>>>
>>>>> [  634.885003] usb 1-1: New USB device found, idVendor=0596, idProduct=0500
>>>>> ...
>>>>> [  676.469629] BUG kmalloc-192 (Tainted: G        W   ): Redzone overwritten
>>>>>
>>>>> CVE-2013-2897
>>>>>
>>>>> Signed-off-by: Kees Cook <keescook@chromium.org>
>>>>> Cc: stable@kernel.org
>>>>> ---
>>>>>  drivers/hid/hid-multitouch.c |   25 ++++++++++++++++++++-----
>>>>>  1 file changed, 20 insertions(+), 5 deletions(-)
>>>>>
>>>>> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
>>>>> index cb0e361..2aa275e 100644
>>>>> --- a/drivers/hid/hid-multitouch.c
>>>>> +++ b/drivers/hid/hid-multitouch.c
>>>>> @@ -330,9 +330,18 @@ static void mt_feature_mapping(struct hid_device *hdev,
>>>>>                               break;
>>>>>                       }
>>>>>               }
>>>>> +             /* Ignore if value index is out of bounds. */
>>>>> +             if (td->inputmode_index < 0 ||
>>>>
>>>> td->inputmode_index can not be less than 0
>>>
>>> Well, it certainly _shouldn't_ be less than zero. :) However, it is
>>> defined as s8, and gets set from an int:
>>>
>>>                 for (i=0; i < field->maxusage; i++) {
>>>                         if (field->usage[i].hid == usage->hid) {
>>>                                 td->inputmode_index = i;
>>>                                 break;
>>>                         }
>>>                 }
>>>
>>> Both "i" and "maxusage" are int, and I can generate a large maxusage
>>> and usage array where the first matching hid equality happens when i
>>> is >127, causing inputmode_index to wrap.
>>
>> ouch. Sorry. This piece of code (the current code) is junk. We should
>> switch to usage->usage_index and change from __s8 to __s16 the
>> declaration ASAP.
>>
>>>
>>>>> +                 td->inputmode_index >= field->report_count) {
>>>>
>>>> if this is really required, we could just change the for loop above to
>>>> go from 0 to field->report_count instead.
>>>
>>> That's certainly true, but since usage count and report count are not
>>> directly associated, I don't know if there are devices that will freak
>>> out with this restriction.
>>
>> Actually, I just again another long time understanding all the mess of
>> having usage count and report count different in hid-core.
>>
>> I think it is a bug at some point (but it looks like I tend to be
>> wrong on a lot of things recently :-P ), because when you look at the
>> actual code of hid_input_field() in hid-core.c, we never go further
>> than field->report_count when dealing with input report.
>> So my guess is that we are declaring extra usages that are never used
>> to parse incoming data.
>>
>>>
>>>> However, I think we could just rely on usage->usage_index to get the
>>>> actual index.
>>>> I'll do more tests today and tell you later.
>>>>
>>>>> +                     dev_err(&hdev->dev, "HID_DG_INPUTMODE out of range\n");
>>>>> +                     td->inputmode = -1;
>>>>> +             }
>>>>>
>>>>>               break;
>>>>>       case HID_DG_CONTACTMAX:
>>>>> +             /* Ignore if value count is out of bounds. */
>>>>> +             if (field->report_count < 1)
>>>>> +                     break;
>>>>
>>>> If you can trigger this, I would say that the fix should be in hid-core,
>>>> not in every driver. A null report_count should not be allowed by the
>>>> HID protocol.
>>>
>>> Again, I have no problem with that idea, but there seem to be lots of
>>> general cases where this may not be possible (i.e. a HID with 0 OUTPUT
>>> or FEATURE reports). I didn't see a sensible way to approach this in
>>> core without declaring a "I require $foo many OUTPUT reports, $bar
>>> many INPUT, and $baz many FEATURE" for each driver. I instead opted
>>> for the report validation function instead.
>>>
>>
>> I think we can just add this check in both hidinput_configure_usage()
>> and report_features() (both in hid-input.c) so that we don't call the
>> *_mapping() callbacks with non sense fields.
>> This way, the specific hid drivers will know only the correct fields,
>> and don't need to check this. So patches 9, 11 and 13 can be amended.
>>
>> It looks to me that you mismatched field->report_count and the actual
>> report_count in your answer: field->report_count is the number of
>> consecutive fields of field->report_size that are present for the
>> parsing of the report. It has nothing to do with the total report
>> count.
>>
>>>>>               td->maxcontact_report_id = field->report->id;
>>>>>               td->maxcontacts = field->value[0];
>>>>>               if (!td->maxcontacts &&
>>>>> @@ -743,15 +752,21 @@ static void mt_touch_report(struct hid_device *hid, struct hid_report *report)
>>>>>       unsigned count;
>>>>>       int r, n;
>>>>>
>>>>> +     if (report->maxfield == 0)
>>>>> +             return;
>>>>> +
>>>>>       /*
>>>>>        * Includes multi-packet support where subsequent
>>>>>        * packets are sent with zero contactcount.
>>>>>        */
>>>>> -     if (td->cc_index >= 0) {
>>>>> -             struct hid_field *field = report->field[td->cc_index];
>>>>> -             int value = field->value[td->cc_value_index];
>>>>> -             if (value)
>>>>> -                     td->num_expected = value;
>>>>> +     if (td->cc_index >= 0 && td->cc_index < report->maxfield) {
>>>>> +             field = report->field[td->cc_index];
>>>>
>>>> looks like we previously overwrote the definition of field :(
>>>>
>>>>> +             if (td->cc_value_index >= 0 &&
>>>>> +                 td->cc_value_index < field->report_count) {
>>>>> +                     int value = field->value[td->cc_value_index];
>>>>> +                     if (value)
>>>>> +                             td->num_expected = value;
>>>>> +             }
>>>>
>>>> I can not see why td->cc_index and td->cc_value_index could have bad
>>>> values. They are initially created by hid-core, and are not provided by
>>>> the device.
>>>> Anyway, if you are able to produce bad values with them, I'd rather have
>>>> those checks during the assignment of td->cc_index (in
>>>> mt_touch_input_mapping()), instead of checking them for each report.
>>>
>>> Well, the problem comes again from there not being a hard link between
>>> the usage array and the value array:
>>>
>>>                         td->cc_value_index = usage->usage_index;
>>> ...
>>>                         int value = field->value[td->cc_value_index];
>>>
>>> And all of this would be irrelevant if there was an access function
>>> for field->value[].
>>
>> True, with the current implementation, td->cc_value_index can be
>> greater than field->report_count. Still, moving this check in
>> mt_touch_input_mapping() will allow us to make it only once.
>>
>>>
>>> Thanks for the review!
>>
>> you are welcome :)
>
> Okay, so, where does the whole patch series stand? It seems like the
> checks are all worth adding; and my fixes are, I think, "minimal" so
> having them go into the tree (so that they land in stable) seems like

Hi Kees,

well, sorry for that, but I don't want to see this specific patch in
stable (11/14). The blocking problem I see with this one is that it is
doing the checks in mt_touch_report() at each incoming report, which
will overload the processing without a real need as this check can be
done while setting the device up.
And having it in stable means that we will have to fix them right
after, so I'm not particularly kind of it.

For Fedora, for instance, we already have included the whole patch
series to fix the CVE, and I'm sure other distributions will do the
same. So I would say that the CVE is not particularly a problem right
now for main users. For upstream, I would say it is a different
matter.

Anyway, the final decision will be taken by Jiri, I am just expressing
my point of view.


> a good idea. The work beyond that could be done on top of the existing
> patches? There seems to be a lot of redesign ideas, but those probably
> aren't so great for stable, and I'm probably not the best person to
> write them, since everything I know about HID code I learned two weeks
> ago. :)

There are not so many changes I asked, but it's clear that I would
prefer doing some regressions tests before having them in stable. The
problem I have now is that I am taking this week 4 days off with
little to no internet connection. So I will not be able to do the
proper testing before Friday.

>
> Given the 12 flaws, what do you see as the best way forward?

If Jiri thinks we can wait one more week for some of these patches (I
know that the most critical one, 1/14 is already on its way to Linus),
then I can handle the v2, but not before Friday. Again, I think the
current patch series is perfectly fine for distributions as they can
add/remove things more easily than we can do in master.

Cheers,
Bnejamin

^ permalink raw reply

* [PATCH] HID: move HID_REPORT_TYPES closer to the report-definitions
From: David Herrmann @ 2013-09-02 12:00 UTC (permalink / raw)
  To: linux-input; +Cc: Jiri Kosina, David Herrmann

HID_REPORT_TYPES defines the number of available report-types. Move it
closer to the actualy definition of the report-types so we can see the
relation more clearly (and hopefully will never forget to update it).

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 include/linux/hid.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/hid.h b/include/linux/hid.h
index 650dc29..383b9d6 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -252,6 +252,8 @@ struct hid_item {
 #define HID_OUTPUT_REPORT	1
 #define HID_FEATURE_REPORT	2
 
+#define HID_REPORT_TYPES	3
+
 /*
  * HID connect requests
  */
@@ -401,8 +403,6 @@ struct hid_report_enum {
 	struct hid_report *report_id_hash[256];
 };
 
-#define HID_REPORT_TYPES 3
-
 #define HID_MIN_BUFFER_SIZE	64		/* make sure there is at least a packet size of space */
 #define HID_MAX_BUFFER_SIZE	4096		/* 4kb */
 #define HID_CONTROL_FIFO_SIZE	256		/* to init devices with >100 reports */
-- 
1.8.4


^ permalink raw reply related

* Re: [PATCH 1/3] Input: introduce BTN/ABS bits for drums and guitars
From: Jiri Kosina @ 2013-09-02 11:41 UTC (permalink / raw)
  To: David Herrmann; +Cc: linux-input, Nicolas Adenis-Lamarre, Dmitry Torokhov
In-Reply-To: <1377537288-19289-2-git-send-email-dh.herrmann@gmail.com>

On Mon, 26 Aug 2013, David Herrmann wrote:

> There are a bunch of guitar and drums devices out there that all report
> similar data. To avoid reporting this as BTN_MISC or ABS_MISC, we
> allocate some proper namespace for them. Note that most of these devices
> are toys and we cannot report any sophisticated physics via this API.
> 
> I did some google-images research and tried to provide definitions that
> work with all common devices. That's why I went with 4 toms, 4 cymbals,
> one bass, one hi-hat. I haven't seen other drums and I doubt that we need
> any additions to that. Anyway, the naming-scheme is intentionally done in
> an extensible way.
> 
> For guitars, we support 5 frets (normally aligned vertically, compared to
> the real horizontal layouts), a single strum-bar with up/down directions,
> an optional fret-board and a whammy-bar.
> 
> Most of the devices provide pressure values so I went with ABS_* bits. If
> we ever support devices which only provide digital input, we have to
> decide whether to emulate pressure data or add additional BTN_* bits.
> 
> If someone is not familiar with these devices, here are two pictures which
> provide almost all introduced interfaces (or try the given keywords
> with a google-image search):
>   Guitar: ("guitar hero world tour guitar")
>     http://images1.wikia.nocookie.net/__cb20120911023442/applezone/es/images/f/f9/Wii_Guitar.jpg
>   Drums: ("guitar hero drums")
>     http://oyster.ignimgs.com/franchises/images/03/55/35526_band-hero-drum-set-hands-on-20090929040735768.jpg
> 
> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>

Hi,

I have reviewed and like the implementation of 2/3 and 3/3, but I of 
course would like to have Ack from Dmitry for this, so that I could take 
it through my tree together with the rest of the patchset.

Dmitry, pretty please?

Thanks.

> ---
>  include/linux/mod_devicetable.h |  2 +-
>  include/uapi/linux/input.h      | 25 +++++++++++++++++++++++--
>  2 files changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
> index 45e9214..329aa30 100644
> --- a/include/linux/mod_devicetable.h
> +++ b/include/linux/mod_devicetable.h
> @@ -277,7 +277,7 @@ struct pcmcia_device_id {
>  #define INPUT_DEVICE_ID_KEY_MIN_INTERESTING	0x71
>  #define INPUT_DEVICE_ID_KEY_MAX		0x2ff
>  #define INPUT_DEVICE_ID_REL_MAX		0x0f
> -#define INPUT_DEVICE_ID_ABS_MAX		0x3f
> +#define INPUT_DEVICE_ID_ABS_MAX		0x4f
>  #define INPUT_DEVICE_ID_MSC_MAX		0x07
>  #define INPUT_DEVICE_ID_LED_MAX		0x0f
>  #define INPUT_DEVICE_ID_SND_MAX		0x07
> diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
> index d584047..76457ee 100644
> --- a/include/uapi/linux/input.h
> +++ b/include/uapi/linux/input.h
> @@ -716,6 +716,14 @@ struct input_keymap_entry {
>  #define BTN_DPAD_LEFT		0x222
>  #define BTN_DPAD_RIGHT		0x223
>  
> +#define BTN_FRET_FAR_UP		0x224
> +#define BTN_FRET_UP		0x225
> +#define BTN_FRET_MID		0x226
> +#define BTN_FRET_LOW		0x227
> +#define BTN_FRET_FAR_LOW	0x228
> +#define BTN_STRUM_BAR_UP	0x229
> +#define BTN_STRUM_BAR_DOWN	0x22a
> +
>  #define BTN_TRIGGER_HAPPY		0x2c0
>  #define BTN_TRIGGER_HAPPY1		0x2c0
>  #define BTN_TRIGGER_HAPPY2		0x2c1
> @@ -829,8 +837,21 @@ struct input_keymap_entry {
>  #define ABS_MT_TOOL_X		0x3c	/* Center X tool position */
>  #define ABS_MT_TOOL_Y		0x3d	/* Center Y tool position */
>  
> -
> -#define ABS_MAX			0x3f
> +/* Drums and guitars (mostly toys) */
> +#define ABS_TOM_FAR_LEFT	0x40
> +#define ABS_TOM_LEFT		0x41
> +#define ABS_TOM_RIGHT		0x42
> +#define ABS_TOM_FAR_RIGHT	0x43
> +#define ABS_CYMBAL_FAR_LEFT	0x44
> +#define ABS_CYMBAL_LEFT		0x45
> +#define ABS_CYMBAL_RIGHT	0x46
> +#define ABS_CYMBAL_FAR_RIGHT	0x47
> +#define ABS_BASS		0x48
> +#define ABS_HI_HAT		0x49
> +#define ABS_FRET_BOARD		0x4a	/* Guitar fret board, vertical pos */
> +#define ABS_WHAMMY_BAR		0x4b	/* Guitar whammy bar (or vibrato) */
> +
> +#define ABS_MAX			0x4f
>  #define ABS_CNT			(ABS_MAX+1)
>  
>  /*
> -- 
> 1.8.4
> 

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: [PATCH] HID: roccat: Added support for KonePureOptical v2
From: Jiri Kosina @ 2013-09-02 11:39 UTC (permalink / raw)
  To: Stefan Achatz; +Cc: linux-input, linux-kernel
In-Reply-To: <1377864607.2406.1.camel@neuromancer.tessier-ashpool>

On Fri, 30 Aug 2013, Stefan Achatz wrote:

> KonePureOptical is a KonePure with different sensor.
> 
> Signed-off-by: Stefan Achatz <erazor_de@users.sourceforge.net>

Applied, thanks.

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: HID: picolcd: Prevent NULL pointer dereference on _remove()
From: Jiri Kosina @ 2013-09-02 11:37 UTC (permalink / raw)
  To: Bruno Prémont; +Cc: linux-input
In-Reply-To: <20130831140748.3353e7bc@neptune.home>

On Sat, 31 Aug 2013, Bruno Prémont wrote:

> When picolcd is switched into bootloader mode (for FW flashing) make
> sure not to try to dereference NULL-pointers of feature-devices during
> unplug/unbind.
> 
> This fixes following BUG:
>   BUG: unable to handle kernel NULL pointer dereference at 00000298
>   IP: [<f811f56b>] picolcd_exit_framebuffer+0x1b/0x80 [hid_picolcd]
>   *pde = 00000000
>   Oops: 0000 [#1]
>   Modules linked in: hid_picolcd syscopyarea sysfillrect sysimgblt fb_sys_fops
>   CPU: 0 PID: 15 Comm: khubd Not tainted 3.11.0-rc7-00002-g50d62d4 #2
>   EIP: 0060:[<f811f56b>] EFLAGS: 00010292 CPU: 0
>   EIP is at picolcd_exit_framebuffer+0x1b/0x80 [hid_picolcd]
>   Call Trace:
>    [<f811d1ab>] picolcd_remove+0xcb/0x120 [hid_picolcd]
>    [<c1469b09>] hid_device_remove+0x59/0xc0
>    [<c13464ca>] __device_release_driver+0x5a/0xb0
>    [<c134653f>] device_release_driver+0x1f/0x30
>    [<c134603d>] bus_remove_device+0x9d/0xd0
>    [<c13439a5>] device_del+0xd5/0x150
>    [<c14696a4>] hid_destroy_device+0x24/0x60
>    [<c1474cbb>] usbhid_disconnect+0x1b/0x40
>    ...
> 
> Signed-off-by: Bruno Prémont <bonbons@linux-vserver.org>
> Cc: stable@kernel.org
> ---
>  drivers/hid/hid-picolcd_cir.c | 3 ++-
>  drivers/hid/hid-picolcd_fb.c  | 6 +++++-
>  2 files changed, 7 insertions(+), 2 deletions(-)

Applied, thanks Bruno.

-- 
Jiri Kosina
SUSE Labs
--
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] HID: usbhid: quirk for N-Trig DuoSense Touch Screen
From: Jiri Kosina @ 2013-09-02 11:36 UTC (permalink / raw)
  To: Vasily Titskiy; +Cc: linux-kernel, linux-input
In-Reply-To: <CABSveYikpdAy9rUDfVrL=jAj7OCqFXyPd7GsvjTnhH3K=u--Zg@mail.gmail.com>

On Fri, 30 Aug 2013, Vasily Titskiy wrote:

> The DuoSense touchscreen device causes a 10 second timeout. This fix
> removes the delay.
> 
> Signed-off-by: Vasily Titskiy <qehgt0@gmail.com>
> ---
>  drivers/hid/hid-ids.h           |    1 +
>  drivers/hid/usbhid/hid-quirks.c |    1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 2168885..376170e 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -646,6 +646,7 @@
>  #define USB_DEVICE_ID_NTRIG_TOUCH_SCREEN_16   0x0012
>  #define USB_DEVICE_ID_NTRIG_TOUCH_SCREEN_17   0x0013
>  #define USB_DEVICE_ID_NTRIG_TOUCH_SCREEN_18   0x0014
> +#define USB_DEVICE_ID_NTRIG_DUOSENSE 0x1500
> 
>  #define USB_VENDOR_ID_ONTRAK 0x0a07
>  #define USB_DEVICE_ID_ONTRAK_ADU100 0x0064
> diff --git a/drivers/hid/usbhid/hid-quirks.c b/drivers/hid/usbhid/hid-quirks.c
> index 19b8360..1671b47 100644
> --- a/drivers/hid/usbhid/hid-quirks.c
> +++ b/drivers/hid/usbhid/hid-quirks.c
> @@ -109,6 +109,7 @@ static const struct hid_blacklist {
>   { USB_VENDOR_ID_SIGMA_MICRO, USB_DEVICE_ID_SIGMA_MICRO_KEYBOARD,
> HID_QUIRK_NO_INIT_REPORTS },
>   { USB_VENDOR_ID_KYE, USB_DEVICE_ID_KYE_MOUSEPEN_I608X,
> HID_QUIRK_MULTI_INPUT },
>   { USB_VENDOR_ID_KYE, USB_DEVICE_ID_KYE_EASYPEN_M610X, HID_QUIRK_MULTI_INPUT },
> + { USB_VENDOR_ID_NTRIG, USB_DEVICE_ID_NTRIG_DUOSENSE,
> HID_QUIRK_NO_INIT_REPORTS },
>   { 0, 0 }
>  };

Vasily,

your mail client is line-wrapping text, which causes corrupted patches.

I have fixed it up and applied, but please keep that in mind for your 
future submissions.

Thanks,

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: [PATCH] HID: Correct the USB IDs for the new Macbook Air 6
From: Jiri Kosina @ 2013-09-02 11:30 UTC (permalink / raw)
  To: Henrik Rydberg; +Cc: Dmitry Torokhov, linux-input, linux-kernel
In-Reply-To: <1378042304-1400-1-git-send-email-rydberg@euromail.se>

On Sun, 1 Sep 2013, Henrik Rydberg wrote:

> A recent patch (9d9a04ee) added support for the new machine, but got
> the sequence of USB ids wrong. Reports from both Ian and Linus T show
> that the 0x0291 id is for ISO, not ANSI, which should have the missing
> number 0x0290. This patchs moves the three numbers accordingly, fixing
> the problem.
> 
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Reported-and-tested-by: Ian Munsie <darkstarsword@gmail.com>
> Tested-by: Linus G Thiel <linus@hanssonlarsson.se>
> Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
> ---
> Hi Jiri, Dmitry,
> 
> it looks like this change has been sufficiently tested now, in
> addition to making perfect sense. Jiri, would you mind taking it, if
> Dmitry approves?

Absolutely ... waiting for Dmitry's Ack.

Thanks,

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: [PATCH] HID: uhid: add devname module alias
From: Jiri Kosina @ 2013-09-02 11:28 UTC (permalink / raw)
  To: David Herrmann; +Cc: Marcel Holtmann, David Herrmann, open list:HID CORE LAYER
In-Reply-To: <CANq1E4SMMY+aO3DfmKCWST9GUc0Lwugy9iB1TZq3Mg_QtjEAWw@mail.gmail.com>

On Sun, 1 Sep 2013, David Herrmann wrote:

> > For simple device node creation, add the devname module alias.
> 
> Yepp, looks good. uinput already has it, so:
> Reviewed-by: David Herrmann <dh.herrmann@gmail.com>

Applied, thanks.

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: [PATCH 4/6] si4713 : HID blacklist Si4713 USB development board
From: Jiri Kosina @ 2013-09-02  9:23 UTC (permalink / raw)
  To: Dinesh Ram; +Cc: linux-media, dinesh.ram, linux-input
In-Reply-To: <228d4c6c3c411f4f5aad408d5bff88bb09a82e4e.1377861337.git.dinram@cisco.com>

On Fri, 30 Aug 2013, Dinesh Ram wrote:

> The Si4713 development board contains a Si4713 FM transmitter chip
> and is handled by the radio-usb-si4713 driver.
> The board reports itself as (10c4:8244) Cygnal Integrated Products, Inc.
> and misidentifies itself as a HID device in its USB interface descriptor.
> This patch ignores this device as an HID device and hence loads the custom driver.
> 
> Signed-off-by: Dinesh Ram <dinram@cisco.com>
> 
> Cc: Jiri Kosina <jkosina@suse.cz>
> Cc: linux-input@vger.kernel.org
> ---
>  drivers/hid/hid-core.c | 1 +
>  drivers/hid/hid-ids.h  | 2 ++
>  2 files changed, 3 insertions(+)
> 
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index 36668d1..109510f 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -1977,6 +1977,7 @@ static const struct hid_device_id hid_ignore_list[] = {
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_BERKSHIRE, USB_DEVICE_ID_BERKSHIRE_PCWD) },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_CIDC, 0x0103) },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_CYGNAL, USB_DEVICE_ID_CYGNAL_RADIO_SI470X) },
> +	{ HID_USB_DEVICE(USB_VENDOR_ID_CYGNAL, USB_DEVICE_ID_CYGNAL_RADIO_SI4713) },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_CMEDIA, USB_DEVICE_ID_CM109) },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_CYPRESS, USB_DEVICE_ID_CYPRESS_HIDCOM) },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_CYPRESS, USB_DEVICE_ID_CYPRESS_ULTRAMOUSE) },
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index ffe4c7a..2a38726 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -241,6 +241,8 @@
>  #define USB_VENDOR_ID_CYGNAL		0x10c4
>  #define USB_DEVICE_ID_CYGNAL_RADIO_SI470X	0x818a
>  
> +#define USB_DEVICE_ID_CYGNAL_RADIO_SI4713       0x8244
> +
>  #define USB_VENDOR_ID_CYPRESS		0x04b4
>  #define USB_DEVICE_ID_CYPRESS_MOUSE	0x0001
>  #define USB_DEVICE_ID_CYPRESS_HIDCOM	0x5500

Acked-by: Jiri Kosina <jkosina@suse.cz>

(under the assumption that this goes in together with the custom driver).

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* [PATCH] hid: uhid: improve uhid example client
From: David Herrmann @ 2013-09-01 21:45 UTC (permalink / raw)
  To: linux-input; +Cc: Jiri Kosina, Anderson Lizardo, David Herrmann

This extends the uhid example client. It properly documents the built-in
report-descriptor an adds explicit report-numbers.

Furthermore, LED output reports are added to utilize the new UHID output
reports of the kernel. Support for 3 basic LEDs is added and a small
report-parser to print debug messages if output reports were received.

To test this, simply write the EV_LED+LED_CAPSL+1 event to the evdev
device-node of the uhid-device and the kernel will forward it to your uhid
client.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
Hi Jiri

This is actually the first time that I wrote an HID report-descriptor. The old
3-button mouse was just copied from a real mouse. My descriptor works perfectly
well but I am kind of scared I am no longer allowed to make fun of firmware
authors..

Anyway, I extended the example to have a test-case for uhid output reports and
it all works as expected. I got asked for some help on this so I wrote up this
patch to push it mainline.

Cheers
David

#include <assert.h>
#include <linux/input.h>
#include <fcntl.h>
#include <unistd.h>
#include <string.h>
#include <stdlib.h>

int main(int argc, char **argv)
{
        int fd, r;
        struct input_event ev;

        assert(argc >= 3);

        fd = open(argv[1], O_RDWR | O_CLOEXEC);
        assert(fd >= 0);

        memset(&ev, 0, sizeof(ev));
        ev.type = EV_LED;
        ev.code = LED_CAPSL;
        ev.value = atoi(argv[2]);

        r = write(fd, &ev, sizeof(ev));
        assert(r == sizeof(ev));

        close(fd);

        return 0;
}

 samples/uhid/uhid-example.c | 123 +++++++++++++++++++++++++++++++++++++-------
 1 file changed, 103 insertions(+), 20 deletions(-)

diff --git a/samples/uhid/uhid-example.c b/samples/uhid/uhid-example.c
index 03ce3c0..7d58a4b 100644
--- a/samples/uhid/uhid-example.c
+++ b/samples/uhid/uhid-example.c
@@ -1,14 +1,15 @@
 /*
  * UHID Example
  *
- * Copyright (c) 2012 David Herrmann <dh.herrmann@googlemail.com>
+ * Copyright (c) 2012-2013 David Herrmann <dh.herrmann@gmail.com>
  *
  * The code may be used by anyone for any purpose,
  * and can serve as a starting point for developing
  * applications using uhid.
  */
 
-/* UHID Example
+/*
+ * UHID Example
  * This example emulates a basic 3 buttons mouse with wheel over UHID. Run this
  * program as root and then use the following keys to control the mouse:
  *   q: Quit the application
@@ -22,6 +23,11 @@
  *   r: Move wheel up
  *   f: Move wheel down
  *
+ * Additionally to 3 button mouse, 3 keyboard LEDs are also supported (LED_NUML,
+ * LED_CAPSL and LED_SCROLLL). The device doesn't generate any related keyboard
+ * events, though. You need to manually write the EV_LED/LED_XY/1 activation
+ * input event to the evdev device to see it being sent to this device.
+ *
  * If uhid is not available as /dev/uhid, then you can pass a different path as
  * first argument.
  * If <linux/uhid.h> is not installed in /usr, then compile this with:
@@ -41,11 +47,12 @@
 #include <unistd.h>
 #include <linux/uhid.h>
 
-/* HID Report Desciptor
- * We emulate a basic 3 button mouse with wheel. This is the report-descriptor
- * as the kernel will parse it:
+/*
+ * HID Report Desciptor
+ * We emulate a basic 3 button mouse with wheel and 3 keyboard LEDs. This is
+ * the report-descriptor as the kernel will parse it:
  *
- * INPUT[INPUT]
+ * INPUT(1)[INPUT]
  *   Field(0)
  *     Physical(GenericDesktop.Pointer)
  *     Application(GenericDesktop.Mouse)
@@ -72,6 +79,19 @@
  *     Report Count(3)
  *     Report Offset(8)
  *     Flags( Variable Relative )
+ * OUTPUT(2)[OUTPUT]
+ *   Field(0)
+ *     Application(GenericDesktop.Keyboard)
+ *     Usage(3)
+ *       LED.NumLock
+ *       LED.CapsLock
+ *       LED.ScrollLock
+ *     Logical Minimum(0)
+ *     Logical Maximum(1)
+ *     Report Size(1)
+ *     Report Count(3)
+ *     Report Offset(0)
+ *     Flags( Variable Absolute )
  *
  * This is the mapping that we expect:
  *   Button.0001 ---> Key.LeftBtn
@@ -80,19 +100,59 @@
  *   GenericDesktop.X ---> Relative.X
  *   GenericDesktop.Y ---> Relative.Y
  *   GenericDesktop.Wheel ---> Relative.Wheel
+ *   LED.NumLock ---> LED.NumLock
+ *   LED.CapsLock ---> LED.CapsLock
+ *   LED.ScrollLock ---> LED.ScrollLock
  *
  * This information can be verified by reading /sys/kernel/debug/hid/<dev>/rdesc
  * This file should print the same information as showed above.
  */
 
 static unsigned char rdesc[] = {
-	0x05, 0x01, 0x09, 0x02, 0xa1, 0x01, 0x09, 0x01,
-	0xa1, 0x00, 0x05, 0x09, 0x19, 0x01, 0x29, 0x03,
-	0x15, 0x00, 0x25, 0x01, 0x95, 0x03, 0x75, 0x01,
-	0x81, 0x02, 0x95, 0x01, 0x75, 0x05, 0x81, 0x01,
-	0x05, 0x01, 0x09, 0x30, 0x09, 0x31, 0x09, 0x38,
-	0x15, 0x80, 0x25, 0x7f, 0x75, 0x08, 0x95, 0x03,
-	0x81, 0x06, 0xc0, 0xc0,
+	0x05, 0x01,	/* USAGE_PAGE (Generic Desktop) */
+	0x09, 0x02,	/* USAGE (Mouse) */
+	0xa1, 0x01,	/* COLLECTION (Application) */
+	0x09, 0x01,		/* USAGE (Pointer) */
+	0xa1, 0x00,		/* COLLECTION (Physical) */
+	0x85, 0x01,			/* REPORT_ID (1) */
+	0x05, 0x09,			/* USAGE_PAGE (Button) */
+	0x19, 0x01,			/* USAGE_MINIMUM (Button 1) */
+	0x29, 0x03,			/* USAGE_MAXIMUM (Button 3) */
+	0x15, 0x00,			/* LOGICAL_MINIMUM (0) */
+	0x25, 0x01,			/* LOGICAL_MAXIMUM (1) */
+	0x95, 0x03,			/* REPORT_COUNT (3) */
+	0x75, 0x01,			/* REPORT_SIZE (1) */
+	0x81, 0x02,			/* INPUT (Data,Var,Abs) */
+	0x95, 0x01,			/* REPORT_COUNT (1) */
+	0x75, 0x05,			/* REPORT_SIZE (5) */
+	0x81, 0x01,			/* INPUT (Cnst,Var,Abs) */
+	0x05, 0x01,			/* USAGE_PAGE (Generic Desktop) */
+	0x09, 0x30,			/* USAGE (X) */
+	0x09, 0x31,			/* USAGE (Y) */
+	0x09, 0x38,			/* USAGE (WHEEL) */
+	0x15, 0x81,			/* LOGICAL_MINIMUM (-127) */
+	0x25, 0x7f,			/* LOGICAL_MAXIMUM (127) */
+	0x75, 0x08,			/* REPORT_SIZE (8) */
+	0x95, 0x03,			/* REPORT_COUNT (3) */
+	0x81, 0x06,			/* INPUT (Data,Var,Rel) */
+	0xc0,			/* END_COLLECTION */
+	0xc0,		/* END_COLLECTION */
+	0x05, 0x01,	/* USAGE_PAGE (Generic Desktop) */
+	0x09, 0x06,	/* USAGE (Keyboard) */
+	0xa1, 0x01,	/* COLLECTION (Application) */
+	0x85, 0x02,		/* REPORT_ID (2) */
+	0x05, 0x08,		/* USAGE_PAGE (Led) */
+	0x19, 0x01,		/* USAGE_MINIMUM (1) */
+	0x29, 0x03,		/* USAGE_MAXIMUM (3) */
+	0x15, 0x00,		/* LOGICAL_MINIMUM (0) */
+	0x25, 0x01,		/* LOGICAL_MAXIMUM (1) */
+	0x95, 0x03,		/* REPORT_COUNT (3) */
+	0x75, 0x01,		/* REPORT_SIZE (1) */
+	0x91, 0x02,		/* Output (Data,Var,Abs) */
+	0x95, 0x01,		/* REPORT_COUNT (1) */
+	0x75, 0x05,		/* REPORT_SIZE (5) */
+	0x91, 0x01,		/* Output (Cnst,Var,Abs) */
+	0xc0,		/* END_COLLECTION */
 };
 
 static int uhid_write(int fd, const struct uhid_event *ev)
@@ -140,6 +200,27 @@ static void destroy(int fd)
 	uhid_write(fd, &ev);
 }
 
+/* This parses raw output reports sent by the kernel to the device. A normal
+ * uhid program shouldn't do this but instead just forward the raw report.
+ * However, for ducomentational purposes, we try to detect LED events here and
+ * print debug messages for it. */
+static void handle_output(struct uhid_event *ev)
+{
+	/* LED messages are adverised via OUTPUT reports; ignore the rest */
+	if (ev->u.output.rtype != UHID_OUTPUT_REPORT)
+		return;
+	/* LED reports have length 2 bytes */
+	if (ev->u.output.size != 2)
+		return;
+	/* first byte is report-id which is 0x02 for LEDs in our rdesc */
+	if (ev->u.output.data[0] != 0x2)
+		return;
+
+	/* print flags payload */
+	fprintf(stderr, "LED output report received with flags %x\n",
+		ev->u.output.data[1]);
+}
+
 static int event(int fd)
 {
 	struct uhid_event ev;
@@ -174,6 +255,7 @@ static int event(int fd)
 		break;
 	case UHID_OUTPUT:
 		fprintf(stderr, "UHID_OUTPUT from uhid-dev\n");
+		handle_output(&ev);
 		break;
 	case UHID_OUTPUT_EV:
 		fprintf(stderr, "UHID_OUTPUT_EV from uhid-dev\n");
@@ -198,18 +280,19 @@ static int send_event(int fd)
 
 	memset(&ev, 0, sizeof(ev));
 	ev.type = UHID_INPUT;
-	ev.u.input.size = 4;
+	ev.u.input.size = 5;
 
+	ev.u.input.data[0] = 0x1;
 	if (btn1_down)
-		ev.u.input.data[0] |= 0x1;
+		ev.u.input.data[1] |= 0x1;
 	if (btn2_down)
-		ev.u.input.data[0] |= 0x2;
+		ev.u.input.data[1] |= 0x2;
 	if (btn3_down)
-		ev.u.input.data[0] |= 0x4;
+		ev.u.input.data[1] |= 0x4;
 
-	ev.u.input.data[1] = abs_hor;
-	ev.u.input.data[2] = abs_ver;
-	ev.u.input.data[3] = wheel;
+	ev.u.input.data[2] = abs_hor;
+	ev.u.input.data[3] = abs_ver;
+	ev.u.input.data[4] = wheel;
 
 	return uhid_write(fd, &ev);
 }
-- 
1.8.4


^ permalink raw reply related

* Re: [PATCH] HID: uhid: add devname module alias
From: David Herrmann @ 2013-09-01 18:36 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: David Herrmann, Jiri Kosina, open list:HID CORE LAYER
In-Reply-To: <1378058566-5207-1-git-send-email-marcel@holtmann.org>

Hi

On Sun, Sep 1, 2013 at 8:02 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
> For simple device node creation, add the devname module alias.

Yepp, looks good. uinput already has it, so:
Reviewed-by: David Herrmann <dh.herrmann@gmail.com>

Thanks
David

> Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
> ---
>  drivers/hid/uhid.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/hid/uhid.c b/drivers/hid/uhid.c
> index fc307e0..9ab7dfc 100644
> --- a/drivers/hid/uhid.c
> +++ b/drivers/hid/uhid.c
> @@ -659,3 +659,4 @@ 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("devname:" UHID_NAME);
> --
> 1.8.3.1
>

^ permalink raw reply

* [PATCH] HID: uhid: add devname module alias
From: Marcel Holtmann @ 2013-09-01 18:02 UTC (permalink / raw)
  To: dh.herrmann, jkosina; +Cc: linux-input

For simple device node creation, add the devname module alias.

Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
---
 drivers/hid/uhid.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/hid/uhid.c b/drivers/hid/uhid.c
index fc307e0..9ab7dfc 100644
--- a/drivers/hid/uhid.c
+++ b/drivers/hid/uhid.c
@@ -659,3 +659,4 @@ 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("devname:" UHID_NAME);
-- 
1.8.3.1


^ permalink raw reply related

* Re: [PATCH 2/3] driver: input: twl4030-vibra: fix missing of_node_put
From: Peter Ujfalusi @ 2013-09-01 17:01 UTC (permalink / raw)
  To: Libo Chen; +Cc: dmitry.torokhov, Bill Pemberton, broonie, linux-input, LKML
In-Reply-To: <522190ED.6020909@huawei.com>

On 08/31/2013 09:45 AM, Libo Chen wrote:
> 
> decrease node device_node refcount after task completion
> 
> Signed-off-by: Libo Chen <libo.chen@huawei.com>

Acked-by: Peter Ujfalusi <peter.ujfalusi@ti.com>

> ---
>  drivers/input/misc/twl4030-vibra.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/input/misc/twl4030-vibra.c b/drivers/input/misc/twl4030-vibra.c
> index 68a5f33..b8d1526 100644
> --- a/drivers/input/misc/twl4030-vibra.c
> +++ b/drivers/input/misc/twl4030-vibra.c
> @@ -185,8 +185,10 @@ static bool twl4030_vibra_check_coexist(struct twl4030_vibra_data *pdata,
>  	if (pdata && pdata->coexist)
>  		return true;
> 
> -	if (of_find_node_by_name(node, "codec"))
> +	if (of_find_node_by_name(node, "codec")) {
> +		of_node_put(node);
>  		return true;
> +	}
> 
>  	return false;
>  }
> 


-- 
Péter
--
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 3/3] driver: input: twl6040-vibra: fix missing of_node_put
From: Peter Ujfalusi @ 2013-09-01 16:59 UTC (permalink / raw)
  To: Libo Chen
  Cc: dmitry.torokhov, Bill Pemberton, broonie, javier, linux-input,
	LKML, Li Zefan, zhangwei(Jovi)
In-Reply-To: <522190F1.3010103@huawei.com>

On 08/31/2013 09:45 AM, Libo Chen wrote:
> 
> decrease twl6040_core_node device_node refcount after task completion
> 
> There are two ways to implement the function of_node_put through
> the marco CONFIG_OF_DYNAMIC, so it is save to call directly.
> 
> Signed-off-by: Libo Chen <libo.chen@huawei.com>

Acked-by: Peter Ujfalusi <peter.ujfalusi@ti.com>

> ---
>  drivers/input/misc/twl6040-vibra.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/input/misc/twl6040-vibra.c b/drivers/input/misc/twl6040-vibra.c
> index 0c2dfc8..f16193a 100644
> --- a/drivers/input/misc/twl6040-vibra.c
> +++ b/drivers/input/misc/twl6040-vibra.c
> @@ -271,12 +271,14 @@ static int twl6040_vibra_probe(struct platform_device *pdev)
>  #endif
> 
>  	if (!pdata && !twl6040_core_node) {
> +		of_node_put(twl6040_core_node);
>  		dev_err(&pdev->dev, "platform_data not available\n");
>  		return -EINVAL;
>  	}
> 
>  	info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
>  	if (!info) {
> +		of_node_put(twl6040_core_node);
>  		dev_err(&pdev->dev, "couldn't allocate memory\n");
>  		return -ENOMEM;
>  	}
> @@ -304,6 +306,7 @@ static int twl6040_vibra_probe(struct platform_device *pdev)
>  				     &vddvibl_uV);
>  		of_property_read_u32(twl6040_core_node, "ti,vddvibr-uV",
>  				     &vddvibr_uV);
> +		of_node_put(twl6040_core_node);
>  	}
> 
>  	if ((!info->vibldrv_res && !info->viblmotor_res) ||
> 


-- 
Péter
--
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 v3] Input: evdev - add EVIOCREVOKE ioctl
From: David Herrmann @ 2013-09-01 14:07 UTC (permalink / raw)
  To: linux-input; +Cc: Dmitry Torokhov, David Herrmann
In-Reply-To: <1377603589-25501-1-git-send-email-dh.herrmann@gmail.com>

If we have multiple sessions on a system, we normally don't want
background sessions to read input events. Otherwise, it could capture
passwords and more entered by the user on the foreground session. This is
a real world problem as the recent XMir development showed:
  http://mjg59.dreamwidth.org/27327.html

We currently rely on sessions to release input devices when being
deactivated. This relies on trust across sessions. But that's not given on
usual systems. We therefore need a way to control which processes have
access to input devices.

With VTs the kernel simply routed them through the active /dev/ttyX. This
is not possible with evdev devices, though. Moreover, we want to avoid
routing input-devices through some dispatcher-daemon in userspace (which
would add some latency).

This patch introduces EVIOCREVOKE. If called on an evdev fd, this revokes
device-access irrecoverably for that *single* open-file. Hence, once you
call EVIOCREVOKE on any dup()ed fd, all fds for that open-file will be
rather useless now (but still valid compared to close()!). This allows us
to pass fds directly to session-processes from a trusted source. The
source keeps a dup()ed fd and revokes access once the session-process is
no longer active.
Compared to the EVIOCMUTE proposal, we can avoid the CAP_SYS_ADMIN
restriction now as there is no way to revive the fd again. Hence, a user
is free to call EVIOCREVOKE themself to kill the fd.

Additionally, this ioctl allows multi-layer access-control (again compared
to EVIOCMUTE which was limited to one layer via CAP_SYS_ADMIN). A middle
layer can simply request a new open-file from the layer above and pass it
to the layer below. Now each layer can call EVIOCREVOKE on the fds to
revoke access for all layers below, at the expense of one fd per layer.

There's already ongoing experimental user-space work which demonstrates
how it can be used:
  http://lists.freedesktop.org/archives/systemd-devel/2013-August/012897.html

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
Changes from v2:
 - Handle client->revoked like evdev->exist to avoid special-casing "revoked"

 drivers/input/evdev.c      | 33 +++++++++++++++++++++++++++++----
 include/uapi/linux/input.h |  1 +
 2 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
index d2b34fb..cf6bdff 100644
--- a/drivers/input/evdev.c
+++ b/drivers/input/evdev.c
@@ -48,6 +48,7 @@ struct evdev_client {
 	struct evdev *evdev;
 	struct list_head node;
 	int clkid;
+	bool revoked;
 	unsigned int bufsize;
 	struct input_event buffer[];
 };
@@ -164,6 +165,9 @@ static void evdev_pass_values(struct evdev_client *client,
 	struct input_event event;
 	bool wakeup = false;
 
+	if (client->revoked)
+		return;
+
 	event.time = ktime_to_timeval(client->clkid == CLOCK_MONOTONIC ?
 				      mono : real);
 
@@ -429,7 +433,7 @@ static ssize_t evdev_write(struct file *file, const char __user *buffer,
 	if (retval)
 		return retval;
 
-	if (!evdev->exist) {
+	if (!evdev->exist || client->revoked) {
 		retval = -ENODEV;
 		goto out;
 	}
@@ -511,7 +515,7 @@ static ssize_t evdev_read(struct file *file, char __user *buffer,
 		if (!(file->f_flags & O_NONBLOCK)) {
 			error = wait_event_interruptible(evdev->wait,
 					client->packet_head != client->tail ||
-					!evdev->exist);
+					!evdev->exist || client->revoked);
 			if (error)
 				return error;
 		}
@@ -529,7 +533,11 @@ static unsigned int evdev_poll(struct file *file, poll_table *wait)
 
 	poll_wait(file, &evdev->wait, wait);
 
-	mask = evdev->exist ? POLLOUT | POLLWRNORM : POLLHUP | POLLERR;
+	if (evdev->exist && !client->revoked)
+		mask = POLLOUT | POLLWRNORM;
+	else
+		mask = POLLHUP | POLLERR;
+
 	if (client->packet_head != client->tail)
 		mask |= POLLIN | POLLRDNORM;
 
@@ -795,6 +803,17 @@ static int evdev_handle_mt_request(struct input_dev *dev,
 	return 0;
 }
 
+static int evdev_revoke(struct evdev *evdev, struct evdev_client *client,
+			struct file *file)
+{
+	client->revoked = true;
+	evdev_ungrab(evdev, client);
+	input_flush_device(&evdev->handle, file);
+	wake_up_interruptible(&evdev->wait);
+
+	return 0;
+}
+
 static long evdev_do_ioctl(struct file *file, unsigned int cmd,
 			   void __user *p, int compat_mode)
 {
@@ -857,6 +876,12 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,
 		else
 			return evdev_ungrab(evdev, client);
 
+	case EVIOCREVOKE:
+		if (p)
+			return -EINVAL;
+		else
+			return evdev_revoke(evdev, client, file);
+
 	case EVIOCSCLOCKID:
 		if (copy_from_user(&i, p, sizeof(unsigned int)))
 			return -EFAULT;
@@ -1002,7 +1027,7 @@ static long evdev_ioctl_handler(struct file *file, unsigned int cmd,
 	if (retval)
 		return retval;
 
-	if (!evdev->exist) {
+	if (!evdev->exist || client->revoked) {
 		retval = -ENODEV;
 		goto out;
 	}
diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
index 2fb6fae..d61c61c 100644
--- a/include/uapi/linux/input.h
+++ b/include/uapi/linux/input.h
@@ -152,6 +152,7 @@ struct input_keymap_entry {
 #define EVIOCGEFFECTS		_IOR('E', 0x84, int)			/* Report number of effects playable at the same time */
 
 #define EVIOCGRAB		_IOW('E', 0x90, int)			/* Grab/Release device */
+#define EVIOCREVOKE		_IOW('E', 0x91, int)			/* Revoke device access */
 
 #define EVIOCSCLOCKID		_IOW('E', 0xa0, int)			/* Set clockid to be used for timestamps */
 
-- 
1.8.4


^ permalink raw reply related

* [PATCH] HID: Correct the USB IDs for the new Macbook Air 6
From: Henrik Rydberg @ 2013-09-01 13:31 UTC (permalink / raw)
  To: Jiri Kosina; +Cc: Dmitry Torokhov, linux-input, linux-kernel, Henrik Rydberg

A recent patch (9d9a04ee) added support for the new machine, but got
the sequence of USB ids wrong. Reports from both Ian and Linus T show
that the 0x0291 id is for ISO, not ANSI, which should have the missing
number 0x0290. This patchs moves the three numbers accordingly, fixing
the problem.

Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Reported-and-tested-by: Ian Munsie <darkstarsword@gmail.com>
Tested-by: Linus G Thiel <linus@hanssonlarsson.se>
Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
Hi Jiri, Dmitry,

it looks like this change has been sufficiently tested now, in
addition to making perfect sense. Jiri, would you mind taking it, if
Dmitry approves?

The patch should go to stable as well at some point, but the new order
makes me not dare adding a Cc... ;-)

Cheers,
Henrik

 drivers/hid/hid-ids.h         | 6 +++---
 drivers/input/mouse/bcm5974.c | 6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index ffe4c7a..0b7ac21 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -135,9 +135,9 @@
 #define USB_DEVICE_ID_APPLE_ALU_WIRELESS_2009_JIS   0x023b
 #define USB_DEVICE_ID_APPLE_ALU_WIRELESS_2011_ANSI  0x0255
 #define USB_DEVICE_ID_APPLE_ALU_WIRELESS_2011_ISO   0x0256
-#define USB_DEVICE_ID_APPLE_WELLSPRING8_ANSI	0x0291
-#define USB_DEVICE_ID_APPLE_WELLSPRING8_ISO	0x0292
-#define USB_DEVICE_ID_APPLE_WELLSPRING8_JIS	0x0293
+#define USB_DEVICE_ID_APPLE_WELLSPRING8_ANSI	0x0290
+#define USB_DEVICE_ID_APPLE_WELLSPRING8_ISO	0x0291
+#define USB_DEVICE_ID_APPLE_WELLSPRING8_JIS	0x0292
 #define USB_DEVICE_ID_APPLE_FOUNTAIN_TP_ONLY	0x030a
 #define USB_DEVICE_ID_APPLE_GEYSER1_TP_ONLY	0x030b
 #define USB_DEVICE_ID_APPLE_IRCONTROL	0x8240
diff --git a/drivers/input/mouse/bcm5974.c b/drivers/input/mouse/bcm5974.c
index 4ef4d5e..a73f961 100644
--- a/drivers/input/mouse/bcm5974.c
+++ b/drivers/input/mouse/bcm5974.c
@@ -89,9 +89,9 @@
 #define USB_DEVICE_ID_APPLE_WELLSPRING7A_ISO	0x025a
 #define USB_DEVICE_ID_APPLE_WELLSPRING7A_JIS	0x025b
 /* MacbookAir6,2 (unibody, June 2013) */
-#define USB_DEVICE_ID_APPLE_WELLSPRING8_ANSI	0x0291
-#define USB_DEVICE_ID_APPLE_WELLSPRING8_ISO	0x0292
-#define USB_DEVICE_ID_APPLE_WELLSPRING8_JIS	0x0293
+#define USB_DEVICE_ID_APPLE_WELLSPRING8_ANSI	0x0290
+#define USB_DEVICE_ID_APPLE_WELLSPRING8_ISO	0x0291
+#define USB_DEVICE_ID_APPLE_WELLSPRING8_JIS	0x0292
 
 #define BCM5974_DEVICE(prod) {					\
 	.match_flags = (USB_DEVICE_ID_MATCH_DEVICE |		\
-- 
1.8.3.4


^ permalink raw reply related

* Re: [PATCH 1/2] HID: apple: Add another device ID for the mid-2013 Macbook Air
From: Henrik Rydberg @ 2013-09-01 12:59 UTC (permalink / raw)
  To: Linus G Thiel
  Cc: Ian Munsie, Brad Ford, Dmitry Torokhov, Jiri Kosina, linux-input,
	linux-kernel
In-Reply-To: <CAG4XgJTX13d9-2X7g7Dpi+O6nFhsufAijAnVpAczwrkiLrrZng@mail.gmail.com>

On Mon, Aug 26, 2013 at 02:51:32PM +0200, Linus G Thiel wrote:
> I'm on an ISO layout, this is what makes sense to me and it also works fine
> on my laptop (6,2 with Swedish layout).
> 
> 
> On Mon, Aug 26, 2013 at 3:50 AM, Ian Munsie <darkstarsword@gmail.com> wrote:
> 
> > > Brad, Linus, does the above patch work for you as well as for Ian?
> >
> > That would work fine for me, but I guess we need confirmation from
> > someone with the ISO or JIS layouts.
> >
> > Cheers,
> > -Ian

Thanks for the tests, we will assume it works for Brad as well. I will send a new patch.

Henrik

^ permalink raw reply

* Re: [PATCH] Input: add driver for Neonode zForce based touchscreens
From: Henrik Rydberg @ 2013-09-01 12:57 UTC (permalink / raw)
  To: Heiko Stübner; +Cc: Dmitry Torokhov, linux-kernel, linux-input
In-Reply-To: <201308161359.40134.heiko@sntech.de>

Hi Heiko,

> This adds a driver for touchscreens using the zforce infrared
> technology from Neonode connected via i2c to the host system.
> 
> It supports multitouch with up to two fingers and tracking of the
> contacts in hardware.
> 
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>

Thanks for the driver. Please find some comments below.

> +static int zforce_touch_event(struct zforce_ts *ts, u8 *payload)
> +{
> +	struct i2c_client *client = ts->client;
> +	struct zforce_point point[ZFORCE_REPORT_POINTS];

You do not really need the array here, do you? A single member should
suffice, stack is precious.

> +	const struct zforce_ts_platdata *pdata = client->dev.platform_data;
> +	int count, i;
> +
> +	count = payload[0];
> +	if (count > ZFORCE_REPORT_POINTS) {
> +		dev_warn(&client->dev, "to many coordinates %d, expected max %d\n",
> +			 count, ZFORCE_REPORT_POINTS);
> +		count = ZFORCE_REPORT_POINTS;
> +	}
> +
> +	for (i = 0; i < count; i++) {
> +		point[i].coord_x =
> +			payload[9 * i + 2] << 8 | payload[9 * i + 1];
> +		point[i].coord_y =
> +			payload[9 * i + 4] << 8 | payload[9 * i + 3];
> +
> +		if (point[i].coord_x > pdata->x_max ||
> +		    point[i].coord_y > pdata->y_max) {
> +			dev_warn(&client->dev, "coordinates (%d,%d) invalid\n",
> +				point[i].coord_x, point[i].coord_y);
> +			point[i].coord_x = point[i].coord_y = 0;
> +		}
> +
> +		point[i].state = payload[9 * i + 5] & 0x03;
> +		point[i].id = (payload[9 * i + 5] & 0xfc) >> 2;
> +
> +		/* determine touch major, minor and orientation */
> +		point[i].area_major = max(payload[9 * i + 6],
> +					  payload[9 * i + 7]);
> +		point[i].area_minor = min(payload[9 * i + 6],
> +					  payload[9 * i + 7]);
> +		point[i].orientation = payload[9 * i + 6] > payload[9 * i + 7];
> +
> +		point[i].pressure = payload[9 * i + 8];
> +		point[i].prblty = payload[9 * i + 9];
> +	}
> +
> +	for (i = 0; i < count; i++) {

just continue the loop here (or move the conversion out as a function).

> +		dev_dbg(&client->dev, "point %d/%d: state %d, id %d, pressure %d, prblty %d, x %d, y %d, amajor %d, aminor %d, ori %d\n",
> +			i, count, point[i].state, point[i].id,
> +			point[i].pressure, point[i].prblty,
> +			point[i].coord_x, point[i].coord_y,
> +			point[i].area_major, point[i].area_minor,
> +			point[i].orientation);
> +
> +		/* the zforce id starts with "1", so needs to be decreased */
> +		input_mt_slot(ts->input, point[i].id - 1);
> +
> +		input_mt_report_slot_state(ts->input, MT_TOOL_FINGER,
> +						point[i].state != STATE_UP);
> +
> +		if (point[i].state != STATE_UP) {
> +			input_report_abs(ts->input, ABS_MT_POSITION_X,
> +					 point[i].coord_x);
> +			input_report_abs(ts->input, ABS_MT_POSITION_Y,
> +					 point[i].coord_y);
> +			input_report_abs(ts->input, ABS_MT_TOUCH_MAJOR,
> +					 point[i].area_major);
> +			input_report_abs(ts->input, ABS_MT_TOUCH_MINOR,
> +					 point[i].area_minor);
> +			input_report_abs(ts->input, ABS_MT_ORIENTATION,
> +					 point[i].orientation);
> +		}
> +	}
> +
> +	if (point[0].state != STATE_UP) {
> +		input_report_abs(ts->input, ABS_X, point[0].coord_x);
> +		input_report_abs(ts->input, ABS_Y, point[0].coord_y);
> +	}
> +
> +	input_report_key(ts->input, BTN_TOUCH, point[0].state != STATE_UP);

You can use input_mt_sync() here instead of the single-touch stuff.

> +
> +	input_sync(ts->input);
> +
> +	return 0;
> +}
> +
> +static int zforce_probe(struct i2c_client *client,
> +			const struct i2c_device_id *id)
> +{
> +	const struct zforce_ts_platdata *pdata = client->dev.platform_data;
> +	struct zforce_ts *ts;
> +	struct input_dev *input_dev;
> +	int ret;
> +
> +	if (!pdata)
> +		return -EINVAL;
> +
> +	ts = devm_kzalloc(&client->dev, sizeof(struct zforce_ts), GFP_KERNEL);
> +	if (!ts)
> +		return -ENOMEM;
> +
> +	ret = devm_gpio_request_one(&client->dev, pdata->gpio_int, GPIOF_IN,
> +				    "zforce_ts_int");
> +	if (ret) {
> +		dev_err(&client->dev, "request of gpio %d failed, %d\n",
> +			pdata->gpio_int, ret);
> +		return ret;
> +	}
> +
> +	ret = devm_gpio_request_one(&client->dev, pdata->gpio_rst,
> +				    GPIOF_OUT_INIT_LOW, "zforce_ts_rst");
> +	if (ret) {
> +		dev_err(&client->dev, "request of gpio %d failed, %d\n",
> +			pdata->gpio_rst, ret);
> +		return ret;
> +	}
> +
> +	ret = devm_add_action(&client->dev, zforce_reset, ts);
> +	if (ret) {
> +		dev_err(&client->dev, "failed to register reset action, %d\n",
> +			ret);
> +		return ret;
> +	}
> +
> +	msleep(20);
> +
> +	snprintf(ts->phys, sizeof(ts->phys),
> +		 "%s/input0", dev_name(&client->dev));
> +
> +	input_dev = devm_input_allocate_device(&client->dev);
> +	if (!input_dev) {
> +		dev_err(&client->dev, "could not allocate input device\n");
> +		return -ENOMEM;
> +	}
> +
> +	mutex_init(&ts->access_mutex);
> +	mutex_init(&ts->command_mutex);
> +
> +	ts->pdata = pdata;
> +	ts->client = client;
> +	ts->input = input_dev;
> +
> +	input_dev->name = "Neonode zForce touchscreen";
> +	input_dev->phys = ts->phys;
> +	input_dev->id.bustype = BUS_I2C;
> +	input_dev->dev.parent = &client->dev;
> +
> +	input_dev->open = zforce_input_open;
> +	input_dev->close = zforce_input_close;
> +
> +	set_bit(EV_KEY, input_dev->evbit);
> +	set_bit(EV_SYN, input_dev->evbit);
> +	set_bit(EV_ABS, input_dev->evbit);
> +	set_bit(BTN_TOUCH, input_dev->keybit);

BTN_TOUCH can be skipped,

> +
> +	/* For single touch */
> +	input_set_abs_params(input_dev, ABS_X, 0, pdata->x_max, 0, 0);
> +	input_set_abs_params(input_dev, ABS_Y, 0, pdata->y_max, 0, 0);

and these too,

> +
> +	/* For multi touch */
> +	input_mt_init_slots(input_dev, ZFORCE_REPORT_POINTS, 0);

if you add INPUT_MT_DIRECT as argument here.

> +	input_set_abs_params(input_dev, ABS_MT_POSITION_X, 0,
> +			     pdata->x_max, 0, 0);
> +	input_set_abs_params(input_dev, ABS_MT_POSITION_Y, 0,
> +			     pdata->y_max, 0, 0);
> +
> +	input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR, 0,
> +			     ZFORCE_MAX_AREA, 0, 0);
> +	input_set_abs_params(input_dev, ABS_MT_TOUCH_MINOR, 0,
> +			     ZFORCE_MAX_AREA, 0, 0);
> +	input_set_abs_params(input_dev, ABS_MT_ORIENTATION, 0, 1, 0, 0);
> +
> +	input_set_drvdata(ts->input, ts);
> +
> +	init_completion(&ts->command_done);
> +
> +	/* The zforce pulls the interrupt low when it has data ready.
> +	 * After it is triggered the isr thread runs until all the available
> +	 * packets have been read and the interrupt is high again.
> +	 * Therefore we can trigger the interrupt anytime it is low and do
> +	 * not need to limit it to the interrupt edge.
> +	 */
> +	ret = devm_request_threaded_irq(&client->dev, client->irq, NULL,
> +					zforce_interrupt,
> +					IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> +					input_dev->name, ts);
> +	if (ret) {
> +		dev_err(&client->dev, "irq %d request failed\n", client->irq);
> +		return ret;
> +	}
> +
> +	i2c_set_clientdata(client, ts);
> +
> +	/* let the controller boot */
> +	gpio_set_value(pdata->gpio_rst, 1);
> +
> +	ts->command_waiting = NOTIFICATION_BOOTCOMPLETE;
> +	if (wait_for_completion_timeout(&ts->command_done, WAIT_TIMEOUT) == 0)
> +		dev_warn(&client->dev, "bootcomplete timed out\n");
> +
> +	/* need to start device to get version information */
> +	ret = zforce_command_wait(ts, COMMAND_INITIALIZE);
> +	if (ret) {
> +		dev_err(&client->dev, "unable to initialize, %d\n", ret);
> +		return ret;
> +	}
> +
> +	/* this gets the firmware version among other informations */
> +	ret = zforce_command_wait(ts, COMMAND_STATUS);
> +	if (ret < 0) {
> +		dev_err(&client->dev, "couldn't get status, %d\n", ret);
> +		zforce_stop(ts);
> +		return ret;
> +	}
> +
> +	/* stop device and put it into sleep until it is opened */
> +	ret = zforce_stop(ts);
> +	if (ret < 0)
> +		return ret;
> +
> +	device_set_wakeup_capable(&client->dev, true);
> +
> +	ret = input_register_device(input_dev);
> +	if (ret) {
> +		dev_err(&client->dev, "could not register input device, %d\n",
> +			ret);
> +		return ret;
> +	}
> +
> +	return 0;
> +}

Thanks,
Henrik

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox