* Re: [PATCH] HID: fix data access in implement()
From: Jiri Kosina @ 2013-07-10 17:49 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Bruno Prémont, Gustavo Padovan, Adam Kropelin,
Grant Grundler, linux-input
In-Reply-To: <51DD6BB8.9050206@gmail.com>
On Wed, 10 Jul 2013, Benjamin Tissoires wrote:
> > implement() is setting bytes in LE data stream. In case the data
> > is not aligned to 64bits, it reads past the allocated buffer. It
> > doesn't really change any value there (it's properly bitmasked), but
> > in case that this read past the boundary hits a page boundary, pagefault
> > happens when accessing 64bits of 'x' in implement(), and kernel oopses.
> >
> > This happens much more often when numbered reports are in use, as the
> > initial 8bit skip in the buffer makes the whole process work on values
> > which are not aligned to 64bits.
> >
> > This problem dates back to attempts in 2005 and 2006 to make implement()
> > and extract() as generic as possible, and even back then the problem
> > was realized by Adam Kroperlin, but falsely assumed to be impossible
> > to cause any harm:
> >
> > http://www.mail-archive.com/linux-usb-devel@lists.sourceforge.net/msg47690.html
> >
> > I have made several attempts at fixing it "on the spot" directly in
> > implement(), but the results were horrible; the special casing for processing
> > last 64bit chunk and switching to different math makes it unreadable mess.
> >
> > I therefore took a path to allocate a few bytes more which will never make
> > it into final report, but are there as a cushion for all the 64bit math
> > operations happening in implement() and extract().
> >
> > All callers of hid_output_report() are converted at the same time to allocate
> > the buffer by newly introduced hid_alloc_report_buf() helper.
> >
> > Signed-off-by: Jiri Kosina <jkosina@suse.cz>
> > ---
>
> Thanks for that. It should (I hope) fix the bugs we are seeing from time
> to times under Fedora and that I was not able to answer:
> https://bugzilla.redhat.com/show_bug.cgi?id=965280
> https://bugzilla.redhat.com/show_bug.cgi?id=927488
> https://bugzilla.redhat.com/show_bug.cgi?id=881504
965280 and 881504 absolutely look like an incarnation of this very
problem. The faulting address is pagesize aligned (i.e. the loop just
crossed the boundary), and the faulting code is the
put_unaligned_access_le64().
Exactly the same as the report I had.
927488 seems like a different bug to me -- not an address on page
boundary, and we're not faulting in implement().
> I have a small remark for usbhid:
>
> > drivers/hid/hid-core.c | 19 ++++++++++++++++++-
> > drivers/hid/hid-logitech-dj.c | 12 ++++++++++--
> > drivers/hid/hid-picolcd_debugfs.c | 10 +++++++++-
> > drivers/hid/usbhid/hid-core.c | 4 ++--
> > include/linux/hid.h | 1 +
> > net/bluetooth/hidp/core.c | 14 +++++++++-----
> > 6 files changed, 49 insertions(+), 11 deletions(-)
>
> [snipped]
>
> > diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
> > index 9941828..2b0b96daf 100644
> > --- a/drivers/hid/usbhid/hid-core.c
> > +++ b/drivers/hid/usbhid/hid-core.c
> > @@ -546,7 +546,7 @@ static void __usbhid_submit_report(struct hid_device *hid, struct hid_report *re
> > return;
> > }
> >
> > - usbhid->out[usbhid->outhead].raw_report = kmalloc(len, GFP_ATOMIC);
> > + usbhid->out[usbhid->outhead].raw_report = hid_alloc_report_buf(report, GFP_ATOMIC);
> > if (!usbhid->out[usbhid->outhead].raw_report) {
> > hid_warn(hid, "output queueing failed\n");
> > return;
> > @@ -595,7 +595,7 @@ static void __usbhid_submit_report(struct hid_device *hid, struct hid_report *re
> > }
> >
> > if (dir == USB_DIR_OUT) {
> > - usbhid->ctrl[usbhid->ctrlhead].raw_report = kmalloc(len, GFP_ATOMIC);
> > + usbhid->ctrl[usbhid->ctrlhead].raw_report = hid_alloc_report_buf(report, GFP_ATOMIC);
>
> line 538: int len = ((report->size - 1) >> 3) + 1 + (report->id > 0);
> is not used anywhere after applying the patch. It can be dropped.
Good catch.
> Besides the picolcd problems spotted by Bruno:
> Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH 22/51] Input: atmel_mxt_ts - Add shutdown function
From: Dmitry Torokhov @ 2013-07-10 16:55 UTC (permalink / raw)
To: Nick Dyer
Cc: Daniel Kurtz, Henrik Rydberg, Joonyoung Shim, Alan Bowens,
linux-input, linux-kernel, Peter Meerwald, Benson Leung,
Olof Johansson
In-Reply-To: <51DA8CB6.2010109@itdev.co.uk>
On Mon, Jul 08, 2013 at 10:56:06AM +0100, Nick Dyer wrote:
> Dmitry Torokhov wrote:
> > On Thu, Jun 27, 2013 at 01:48:57PM +0100, Nick Dyer wrote:
> >> Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
> >> Acked-by: Benson Leung <bleung@chromium.org>
> >
> > Why is this needed?
>
> The patch disables the interrupt handler on shutdown.
>
> One of our customers reported a bug caused by input events being generated
> during shutdown (for example if the user was touching the device whilst it
> was turning off), which was solved by putting in this change.
What kind of bug? Could you please be more precise?
>
> However, now you've drawn my attention to it again, it seems to me that
> probably a better thing for us to be doing would be to power off the
> touchscreen controller here, and let the interrupt disable be handled by
> core code - do you agree?
We'd be powering off everything in a moment anyway, no? Or is there a
concern that the device will stay powered up even if the system is in off
state?
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 26/51] Input: atmel_mxt_ts - Move input device init into separate function
From: Dmitry Torokhov @ 2013-07-10 16:53 UTC (permalink / raw)
To: Nick Dyer
Cc: Daniel Kurtz, Henrik Rydberg, Joonyoung Shim, Alan Bowens,
linux-input, linux-kernel, Peter Meerwald, Benson Leung,
Olof Johansson
In-Reply-To: <51DA894D.3050208@itdev.co.uk>
On Mon, Jul 08, 2013 at 10:41:33AM +0100, Nick Dyer wrote:
> Dmitry Torokhov wrote:
> > On Thu, Jun 27, 2013 at 01:49:01PM +0100, Nick Dyer wrote:
> >> Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
> >
> > So before we allocated input device before requesting IRQ, now we fo it
> > afterwards so there is moment where the interrupt is requested and not
> > disabled and input device is not allocated yet. Is it possible for
> > interrupt to happen at that moment?
>
> Yes, and it will be handled correctly, there are guards in the correct
> places to ensure the input device will not be used before being registered.
>
> It is registered at this point since there are several paths that might
> need the interrupt handler (for example, to handle flash if device is in
> failed state, or to upload configuration if necessary).
OK, thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 2/2] Input: Adding DT support for keyreset tuneables
From: Dmitry Torokhov @ 2013-07-10 16:52 UTC (permalink / raw)
To: Grant Likely
Cc: Mathieu Poirier, devicetree-discuss, john.stultz, kernel-team,
linux-input
In-Reply-To: <20130710151457.D0B863E1168@localhost>
On Wed, Jul 10, 2013 at 04:14:57PM +0100, Grant Likely wrote:
> On Fri, 28 Jun 2013 07:19:06 -0600, Mathieu Poirier <mathieu.poirier@linaro.org> wrote:
> > On 13-06-28 12:09 AM, Dmitry Torokhov wrote:
> > >>>> I do not agree. We want the binding to be generic and not tied
> > >>>> specifically to the keyreset functionality. As such 'input-keyset' or
> > >>>> 'input-keychord' are more appropriate.
> > >>>
> > >>> The binding is defined specifically for sysrq and specifically to
> > >>> perform reset action.
> > >>
> > >> Yes for now but as the examples in the binding show, it is easy to
> > >> envision how other drivers could use it.
> > >
> > > I think you over-complicate things here. Unlike matrix-keypad binding,
> > > where you have a common parsing code, here we have an individual driver.
> > > I really do not see anyone else using such sequences or chords as such
> > > processing should be done in userspace. Sysrq is quite an exception.
> >
> > To be honest I don't have a very strong opinion on the binding. I made
> > it as generic as possible on the guidance of the DT people. Let's see
> > what they think of it.
>
> Hi Mathieu,
>
> As per our conversation just now at Connect, the binding should probably
> look like this:
>
> Sysrq keyset binding:
>
> The /chosen node can contain a linux,input-keyset-sysrq child node to
> define a set of keys that will generate a sysrq when pressed together.
Hmm, we would have only one such node, /sysrq, or /linux,sysrq,
whatever. The sysrq setting is system-wide and applicable to all
devices. Given that it is used only on mobile, where there not that
many input devices (a few keys and touchscreen) I do not believe we
should consider adding per-device settings.
>
> Required properties:
> keyset: array of keycodes
Please, let's call it 'key-reset-seq', because it is exactly the reset
sequence. There won't be any additional sequences or chords as those
should be handled in userspace, sysrq is a special case here.
> timeout-ms: duration keys must be pressed together in microseconds
> before generating a sysrq
>
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 2/2] Input: Adding DT support for keyreset tuneables
From: Grant Likely @ 2013-07-10 15:14 UTC (permalink / raw)
To: Mathieu Poirier, Dmitry Torokhov
Cc: devicetree-discuss, john.stultz, kernel-team, linux-input
In-Reply-To: <51CD8D4A.10207@linaro.org>
On Fri, 28 Jun 2013 07:19:06 -0600, Mathieu Poirier <mathieu.poirier@linaro.org> wrote:
> On 13-06-28 12:09 AM, Dmitry Torokhov wrote:
> >>>> I do not agree. We want the binding to be generic and not tied
> >>>> specifically to the keyreset functionality. As such 'input-keyset' or
> >>>> 'input-keychord' are more appropriate.
> >>>
> >>> The binding is defined specifically for sysrq and specifically to
> >>> perform reset action.
> >>
> >> Yes for now but as the examples in the binding show, it is easy to
> >> envision how other drivers could use it.
> >
> > I think you over-complicate things here. Unlike matrix-keypad binding,
> > where you have a common parsing code, here we have an individual driver.
> > I really do not see anyone else using such sequences or chords as such
> > processing should be done in userspace. Sysrq is quite an exception.
>
> To be honest I don't have a very strong opinion on the binding. I made
> it as generic as possible on the guidance of the DT people. Let's see
> what they think of it.
Hi Mathieu,
As per our conversation just now at Connect, the binding should probably
look like this:
Sysrq keyset binding:
The /chosen node can contain a linux,input-keyset-sysrq child node to
define a set of keys that will generate a sysrq when pressed together.
Required properties:
keyset: array of keycodes
timeout-ms: duration keys must be pressed together in microseconds
before generating a sysrq
g.
^ permalink raw reply
* Re: [PATCH] HID: fix data access in implement()
From: Benjamin Tissoires @ 2013-07-10 14:12 UTC (permalink / raw)
To: Jiri Kosina
Cc: Bruno Prémont, Gustavo Padovan, Adam Kropelin,
Grant Grundler, linux-input
In-Reply-To: <alpine.LNX.2.00.1307092042190.26857@pobox.suse.cz>
Hi Jiri,
On 07/09/2013 08:44 PM, Jiri Kosina wrote:
> implement() is setting bytes in LE data stream. In case the data
> is not aligned to 64bits, it reads past the allocated buffer. It
> doesn't really change any value there (it's properly bitmasked), but
> in case that this read past the boundary hits a page boundary, pagefault
> happens when accessing 64bits of 'x' in implement(), and kernel oopses.
>
> This happens much more often when numbered reports are in use, as the
> initial 8bit skip in the buffer makes the whole process work on values
> which are not aligned to 64bits.
>
> This problem dates back to attempts in 2005 and 2006 to make implement()
> and extract() as generic as possible, and even back then the problem
> was realized by Adam Kroperlin, but falsely assumed to be impossible
> to cause any harm:
>
> http://www.mail-archive.com/linux-usb-devel@lists.sourceforge.net/msg47690.html
>
> I have made several attempts at fixing it "on the spot" directly in
> implement(), but the results were horrible; the special casing for processing
> last 64bit chunk and switching to different math makes it unreadable mess.
>
> I therefore took a path to allocate a few bytes more which will never make
> it into final report, but are there as a cushion for all the 64bit math
> operations happening in implement() and extract().
>
> All callers of hid_output_report() are converted at the same time to allocate
> the buffer by newly introduced hid_alloc_report_buf() helper.
>
> Signed-off-by: Jiri Kosina <jkosina@suse.cz>
> ---
Thanks for that. It should (I hope) fix the bugs we are seeing from time
to times under Fedora and that I was not able to answer:
https://bugzilla.redhat.com/show_bug.cgi?id=965280
https://bugzilla.redhat.com/show_bug.cgi?id=927488
https://bugzilla.redhat.com/show_bug.cgi?id=881504
I have a small remark for usbhid:
> drivers/hid/hid-core.c | 19 ++++++++++++++++++-
> drivers/hid/hid-logitech-dj.c | 12 ++++++++++--
> drivers/hid/hid-picolcd_debugfs.c | 10 +++++++++-
> drivers/hid/usbhid/hid-core.c | 4 ++--
> include/linux/hid.h | 1 +
> net/bluetooth/hidp/core.c | 14 +++++++++-----
> 6 files changed, 49 insertions(+), 11 deletions(-)
[snipped]
> diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
> index 9941828..2b0b96daf 100644
> --- a/drivers/hid/usbhid/hid-core.c
> +++ b/drivers/hid/usbhid/hid-core.c
> @@ -546,7 +546,7 @@ static void __usbhid_submit_report(struct hid_device *hid, struct hid_report *re
> return;
> }
>
> - usbhid->out[usbhid->outhead].raw_report = kmalloc(len, GFP_ATOMIC);
> + usbhid->out[usbhid->outhead].raw_report = hid_alloc_report_buf(report, GFP_ATOMIC);
> if (!usbhid->out[usbhid->outhead].raw_report) {
> hid_warn(hid, "output queueing failed\n");
> return;
> @@ -595,7 +595,7 @@ static void __usbhid_submit_report(struct hid_device *hid, struct hid_report *re
> }
>
> if (dir == USB_DIR_OUT) {
> - usbhid->ctrl[usbhid->ctrlhead].raw_report = kmalloc(len, GFP_ATOMIC);
> + usbhid->ctrl[usbhid->ctrlhead].raw_report = hid_alloc_report_buf(report, GFP_ATOMIC);
line 538: int len = ((report->size - 1) >> 3) + 1 + (report->id > 0);
is not used anywhere after applying the patch. It can be dropped.
Besides the picolcd problems spotted by Bruno:
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cheers,
Benjamin
^ permalink raw reply
* Re: [PATCH] HID: fix data access in implement()
From: Bruno Prémont @ 2013-07-10 12:11 UTC (permalink / raw)
To: Jiri Kosina; +Cc: Gustavo Padovan, Adam Kropelin, Grant Grundler, linux-input
In-Reply-To: <alpine.LNX.2.00.1307092042190.26857@pobox.suse.cz>
Hi Jiri,
Some notes regarding picoLCD part below.
On Tue, 09 July 2013 Jiri Kosina <jkosina@suse.cz> wrote:
> implement() is setting bytes in LE data stream. In case the data
> is not aligned to 64bits, it reads past the allocated buffer. It
> doesn't really change any value there (it's properly bitmasked), but
> in case that this read past the boundary hits a page boundary, pagefault
> happens when accessing 64bits of 'x' in implement(), and kernel oopses.
>
> This happens much more often when numbered reports are in use, as the
> initial 8bit skip in the buffer makes the whole process work on values
> which are not aligned to 64bits.
>
> This problem dates back to attempts in 2005 and 2006 to make implement()
> and extract() as generic as possible, and even back then the problem
> was realized by Adam Kroperlin, but falsely assumed to be impossible
> to cause any harm:
>
> http://www.mail-archive.com/linux-usb-devel@lists.sourceforge.net/msg47690.html
>
> I have made several attempts at fixing it "on the spot" directly in
> implement(), but the results were horrible; the special casing for processing
> last 64bit chunk and switching to different math makes it unreadable mess.
>
> I therefore took a path to allocate a few bytes more which will never make
> it into final report, but are there as a cushion for all the 64bit math
> operations happening in implement() and extract().
>
> All callers of hid_output_report() are converted at the same time to allocate
> the buffer by newly introduced hid_alloc_report_buf() helper.
>
> Signed-off-by: Jiri Kosina <jkosina@suse.cz>
> ---
> drivers/hid/hid-core.c | 19 ++++++++++++++++++-
> drivers/hid/hid-logitech-dj.c | 12 ++++++++++--
> drivers/hid/hid-picolcd_debugfs.c | 10 +++++++++-
> drivers/hid/usbhid/hid-core.c | 4 ++--
> include/linux/hid.h | 1 +
> net/bluetooth/hidp/core.c | 14 +++++++++-----
> 6 files changed, 49 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/hid/hid-picolcd_debugfs.c b/drivers/hid/hid-picolcd_debugfs.c
> index 59ab8e1..784a17c 100644
> --- a/drivers/hid/hid-picolcd_debugfs.c
> +++ b/drivers/hid/hid-picolcd_debugfs.c
> @@ -394,7 +394,7 @@ static void dump_buff_as_hex(char *dst, size_t dst_sz, const u8 *data,
> void picolcd_debug_out_report(struct picolcd_data *data,
> struct hid_device *hdev, struct hid_report *report)
> {
> - u8 raw_data[70];
> + u8 *raw_data;
> int raw_size = (report->size >> 3) + 1;
> char *buff;
> #define BUFF_SZ 256
> @@ -407,11 +407,18 @@ void picolcd_debug_out_report(struct picolcd_data *data,
> if (!buff)
> return;
>
> + raw_data = hid_alloc_report_buf(report, GFP_ATOMIC);
> + if (!raw_data) {
> + kfree(buff);
> + return;
> + }
> +
> snprintf(buff, BUFF_SZ, "\nout report %d (size %d) = ",
> report->id, raw_size);
> hid_debug_event(hdev, buff);
> if (raw_size + 5 > sizeof(raw_data)) {
As you change raw_data from u8[] to u8* the sizeof(raw_data)
will not do the right thing.
In addition the whole test (if it were correct) is not needed anymore as
hid_alloc_report_buf() should always return a buffer of sufficient size.
So just drop the test:
- if (raw_size + 5 > sizeof(raw_data)) {
- kfree(buff);
- hid_debug_event(hdev, " TOO BIG\n");
- return;
- } else {
- raw_data[0] = report->id;
- hid_output_report(report, raw_data);
- dump_buff_as_hex(buff, BUFF_SZ, raw_data, raw_size);
- hid_debug_event(hdev, buff);
- }
+ raw_data[0] = report->id;
+ hid_output_report(report, raw_data);
+ dump_buff_as_hex(buff, BUFF_SZ, raw_data, raw_size);
+ hid_debug_event(hdev, buff);
> kfree(buff);
> + kfree(raw_data);
> hid_debug_event(hdev, " TOO BIG\n");
> return;
> } else {
> @@ -644,6 +651,7 @@ void picolcd_debug_out_report(struct picolcd_data *data,
> break;
> }
> wake_up_interruptible(&hdev->debug_wait);
> + kfree(raw_data);
> kfree(buff);
> }
>
Bruno
^ permalink raw reply
* Re: Logitech M705: Thumb button (was Re: [PATCH v4] HID: Add full support for Logitech Unifying receivers)
From: Jiri Kosina @ 2013-07-09 20:41 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Nestor Lopez Casado, Markus Trippelsdorf, Dmitry Torokhov,
Richard Titmuss, Olivier Gay, open list:HID CORE LAYER,
linux-kernel@vger.kernel.org, Jean-Daniel Zanone,
František Fuka
In-Reply-To: <CAN+gG=HNSzg_W-26vMb2seWNU+QcO3+wv4Z1AmZ36LqPhbzdEg@mail.gmail.com>
On Tue, 9 Jul 2013, Benjamin Tissoires wrote:
> For those who wants to try, I have set up a project [1] to implement
> part of the spec Nestor released. I implemented various features: list
> devices paired to a receiver, pair/unpair a device, and toggle the
> button 6 on M705 mouse with the faulty firmware. When switching mode for
> the M705, as Nestor said, we lose the AC panning ability until the
> correct remapping is done in the kernel.
>
> Cheers,
> Benjamin
>
> [1] https://github.com/bentiss/uLogitech
Thanks for your work on this, Benjamin.
I have a followup question -- why not do this in the kernel completely
though? I have actually been asked by Linus directly (so we'd rather do
that :) ) to include pairing trigger as a sysfs toggle instead of having
to use userspace utilitites, and it actually makes sense to me.
Are you aware of any particular reason to not have all that's provided by
uLogitech in the kernel?
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH 1/2] HID: Add driver for Holtek gaming mouse 04d9:a067
From: Christian Ohm @ 2013-07-09 19:24 UTC (permalink / raw)
To: Jiri Kosina; +Cc: Christian Ohm, linux-input, linux-kernel
In-Reply-To: <alpine.LNX.2.00.1305281219490.3914@pobox.suse.cz>
On Tuesday, 28 May 2013 at 12:20, Jiri Kosina wrote:
> I have applied your patches and fixed that omission.
I see the driver made it into Linus's tree. Thanks for applying, and the
fixes!
Best regards,
Christian Ohm
^ permalink raw reply
* Re: [PATCH] HID: fix data access in implement()
From: Gustavo Padovan @ 2013-07-09 18:59 UTC (permalink / raw)
To: Jiri Kosina
Cc: Bruno Prémont, Adam Kropelin, Grant Grundler, linux-input
In-Reply-To: <alpine.LNX.2.00.1307092042190.26857@pobox.suse.cz>
Hi Jiri,
* Jiri Kosina <jkosina@suse.cz> [2013-07-09 20:44:27 +0200]:
> implement() is setting bytes in LE data stream. In case the data
> is not aligned to 64bits, it reads past the allocated buffer. It
> doesn't really change any value there (it's properly bitmasked), but
> in case that this read past the boundary hits a page boundary, pagefault
> happens when accessing 64bits of 'x' in implement(), and kernel oopses.
>
> This happens much more often when numbered reports are in use, as the
> initial 8bit skip in the buffer makes the whole process work on values
> which are not aligned to 64bits.
>
> This problem dates back to attempts in 2005 and 2006 to make implement()
> and extract() as generic as possible, and even back then the problem
> was realized by Adam Kroperlin, but falsely assumed to be impossible
> to cause any harm:
>
> http://www.mail-archive.com/linux-usb-devel@lists.sourceforge.net/msg47690.html
>
> I have made several attempts at fixing it "on the spot" directly in
> implement(), but the results were horrible; the special casing for processing
> last 64bit chunk and switching to different math makes it unreadable mess.
>
> I therefore took a path to allocate a few bytes more which will never make
> it into final report, but are there as a cushion for all the 64bit math
> operations happening in implement() and extract().
>
> All callers of hid_output_report() are converted at the same time to allocate
> the buffer by newly introduced hid_alloc_report_buf() helper.
>
> Signed-off-by: Jiri Kosina <jkosina@suse.cz>
> ---
> drivers/hid/hid-core.c | 19 ++++++++++++++++++-
> drivers/hid/hid-logitech-dj.c | 12 ++++++++++--
> drivers/hid/hid-picolcd_debugfs.c | 10 +++++++++-
> drivers/hid/usbhid/hid-core.c | 4 ++--
> include/linux/hid.h | 1 +
> net/bluetooth/hidp/core.c | 14 +++++++++-----
> 6 files changed, 49 insertions(+), 11 deletions(-)
For the bluetooth part:
Acked-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
Gustavo
^ permalink raw reply
* [PATCH] HID: fix data access in implement()
From: Jiri Kosina @ 2013-07-09 18:44 UTC (permalink / raw)
To: Bruno Prémont, Gustavo Padovan, Adam Kropelin,
Grant Grundler
Cc: linux-input
implement() is setting bytes in LE data stream. In case the data
is not aligned to 64bits, it reads past the allocated buffer. It
doesn't really change any value there (it's properly bitmasked), but
in case that this read past the boundary hits a page boundary, pagefault
happens when accessing 64bits of 'x' in implement(), and kernel oopses.
This happens much more often when numbered reports are in use, as the
initial 8bit skip in the buffer makes the whole process work on values
which are not aligned to 64bits.
This problem dates back to attempts in 2005 and 2006 to make implement()
and extract() as generic as possible, and even back then the problem
was realized by Adam Kroperlin, but falsely assumed to be impossible
to cause any harm:
http://www.mail-archive.com/linux-usb-devel@lists.sourceforge.net/msg47690.html
I have made several attempts at fixing it "on the spot" directly in
implement(), but the results were horrible; the special casing for processing
last 64bit chunk and switching to different math makes it unreadable mess.
I therefore took a path to allocate a few bytes more which will never make
it into final report, but are there as a cushion for all the 64bit math
operations happening in implement() and extract().
All callers of hid_output_report() are converted at the same time to allocate
the buffer by newly introduced hid_alloc_report_buf() helper.
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
---
drivers/hid/hid-core.c | 19 ++++++++++++++++++-
drivers/hid/hid-logitech-dj.c | 12 ++++++++++--
drivers/hid/hid-picolcd_debugfs.c | 10 +++++++++-
drivers/hid/usbhid/hid-core.c | 4 ++--
include/linux/hid.h | 1 +
net/bluetooth/hidp/core.c | 14 +++++++++-----
6 files changed, 49 insertions(+), 11 deletions(-)
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 264f550..987279b 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1128,7 +1128,8 @@ static void hid_output_field(const struct hid_device *hid,
}
/*
- * Create a report.
+ * Create a report. 'data' has to be allocated using
+ * hid_alloc_report_buf() so that it has proper size.
*/
void hid_output_report(struct hid_report *report, __u8 *data)
@@ -1145,6 +1146,22 @@ void hid_output_report(struct hid_report *report, __u8 *data)
EXPORT_SYMBOL_GPL(hid_output_report);
/*
+ * Allocator for buffer that is going to be passed to hid_output_report()
+ */
+u8 *hid_alloc_report_buf(struct hid_report *report, gfp_t flags)
+{
+ /*
+ * 7 extra bytes are necessary to achieve proper functionality
+ * of implement() working on 8 byte chunks
+ */
+
+ int len = ((report->size - 1) >> 3) + 1 + (report->id > 0) + 7;
+
+ return kmalloc(len, flags);
+}
+EXPORT_SYMBOL_GPL(hid_alloc_report_buf);
+
+/*
* Set a field value. The report this field belongs to has to be
* created and transferred to the device, to set this value in the
* device.
diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index 5207591a..4d79273 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -574,7 +574,7 @@ static int logi_dj_ll_input_event(struct input_dev *dev, unsigned int type,
struct hid_field *field;
struct hid_report *report;
- unsigned char data[8];
+ unsigned char *data;
int offset;
dbg_hid("%s: %s, type:%d | code:%d | value:%d\n",
@@ -590,6 +590,13 @@ static int logi_dj_ll_input_event(struct input_dev *dev, unsigned int type,
return -1;
}
hid_set_field(field, offset, value);
+
+ data = hid_alloc_report_buf(field->report, GFP_KERNEL);
+ if (!data) {
+ dev_warn(&dev->dev, "failed to allocate report buf memory\n");
+ return -1;
+ }
+
hid_output_report(field->report, &data[0]);
output_report_enum = &dj_rcv_hiddev->report_enum[HID_OUTPUT_REPORT];
@@ -600,8 +607,9 @@ static int logi_dj_ll_input_event(struct input_dev *dev, unsigned int type,
hid_hw_request(dj_rcv_hiddev, report, HID_REQ_SET_REPORT);
- return 0;
+ kfree(data);
+ return 0;
}
static int logi_dj_ll_start(struct hid_device *hid)
diff --git a/drivers/hid/hid-picolcd_debugfs.c b/drivers/hid/hid-picolcd_debugfs.c
index 59ab8e1..784a17c 100644
--- a/drivers/hid/hid-picolcd_debugfs.c
+++ b/drivers/hid/hid-picolcd_debugfs.c
@@ -394,7 +394,7 @@ static void dump_buff_as_hex(char *dst, size_t dst_sz, const u8 *data,
void picolcd_debug_out_report(struct picolcd_data *data,
struct hid_device *hdev, struct hid_report *report)
{
- u8 raw_data[70];
+ u8 *raw_data;
int raw_size = (report->size >> 3) + 1;
char *buff;
#define BUFF_SZ 256
@@ -407,11 +407,18 @@ void picolcd_debug_out_report(struct picolcd_data *data,
if (!buff)
return;
+ raw_data = hid_alloc_report_buf(report, GFP_ATOMIC);
+ if (!raw_data) {
+ kfree(buff);
+ return;
+ }
+
snprintf(buff, BUFF_SZ, "\nout report %d (size %d) = ",
report->id, raw_size);
hid_debug_event(hdev, buff);
if (raw_size + 5 > sizeof(raw_data)) {
kfree(buff);
+ kfree(raw_data);
hid_debug_event(hdev, " TOO BIG\n");
return;
} else {
@@ -644,6 +651,7 @@ void picolcd_debug_out_report(struct picolcd_data *data,
break;
}
wake_up_interruptible(&hdev->debug_wait);
+ kfree(raw_data);
kfree(buff);
}
diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index 9941828..2b0b96daf 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -546,7 +546,7 @@ static void __usbhid_submit_report(struct hid_device *hid, struct hid_report *re
return;
}
- usbhid->out[usbhid->outhead].raw_report = kmalloc(len, GFP_ATOMIC);
+ usbhid->out[usbhid->outhead].raw_report = hid_alloc_report_buf(report, GFP_ATOMIC);
if (!usbhid->out[usbhid->outhead].raw_report) {
hid_warn(hid, "output queueing failed\n");
return;
@@ -595,7 +595,7 @@ static void __usbhid_submit_report(struct hid_device *hid, struct hid_report *re
}
if (dir == USB_DIR_OUT) {
- usbhid->ctrl[usbhid->ctrlhead].raw_report = kmalloc(len, GFP_ATOMIC);
+ usbhid->ctrl[usbhid->ctrlhead].raw_report = hid_alloc_report_buf(report, GFP_ATOMIC);
if (!usbhid->ctrl[usbhid->ctrlhead].raw_report) {
hid_warn(hid, "control queueing failed\n");
return;
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 0c48991..acccdf4 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -744,6 +744,7 @@ struct hid_field *hidinput_get_led_field(struct hid_device *hid);
unsigned int hidinput_count_leds(struct hid_device *hid);
__s32 hidinput_calc_abs_res(const struct hid_field *field, __u16 code);
void hid_output_report(struct hid_report *report, __u8 *data);
+u8 *hid_alloc_report_buf(struct hid_report *report, gfp_t flags);
struct hid_device *hid_allocate_device(void);
struct hid_report *hid_register_report(struct hid_device *device, unsigned type, unsigned id);
int hid_parse_report(struct hid_device *hid, __u8 *start, unsigned size);
diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index 940f5ac..41f154d 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -231,17 +231,21 @@ static void hidp_input_report(struct hidp_session *session, struct sk_buff *skb)
static int hidp_send_report(struct hidp_session *session, struct hid_report *report)
{
- unsigned char buf[32], hdr;
- int rsize;
+ unsigned char hdr;
+ u8 *buf;
+ int rsize, ret;
- rsize = ((report->size - 1) >> 3) + 1 + (report->id > 0);
- if (rsize > sizeof(buf))
+ buf = hid_alloc_report_buf(report, GFP_ATOMIC);
+ if (!buf)
return -EIO;
hid_output_report(report, buf);
hdr = HIDP_TRANS_DATA | HIDP_DATA_RTYPE_OUPUT;
- return hidp_send_intr_message(session, hdr, buf, rsize);
+ ret = hidp_send_intr_message(session, hdr, buf, rsize);
+
+ kfree(buf);
+ return ret;
}
static int hidp_get_raw_report(struct hid_device *hid,
--
1.7.3.1
^ permalink raw reply related
* Re: My logitech wireless keyboard and mouse stop working after 3.10 kernel
From: wujun zhou @ 2013-07-09 18:27 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Jiri Kosina, linux-input, Andrew de los Reyes,
Nestor Lopez Casado, Linus Torvalds
In-Reply-To: <51DC39CB.9070508@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 5290 bytes --]
Sorry it still doesn't work all the time. 90% time it works. But
sometime(very rare), I still get -EPIPE in packet 49.
In case it matters:
My motherboard is a GIgabyte GA-Z87-UD4H, and all the usb port is
connected to the USB3.0 controller in the intel Z87 chipset
lspci -vv
00:14.0 USB controller: Intel Corporation Lynx Point USB xHCI Host
Controller (rev 04) (prog-if 30 [XHCI])
Subsystem: Giga-byte Technology Device 5007
Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop-
ParErr- Stepping- SERR- FastB2B- DisINTx+
Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium
>TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
Latency: 0
Interrupt: pin A routed to IRQ 44
Region 0: Memory at f0520000 (64-bit, non-prefetchable) [size=64K]
Capabilities: [70] Power Management version 2
Flags: PMEClk- DSI- D1- D2- AuxCurrent=375mA
PME(D0-,D1-,D2-,D3hot+,D3cold+)
Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=0 PME-
Capabilities: [80] MSI: Enable+ Count=1/8 Maskable- 64bit+
Address: 00000000feeff00c Data: 41d1
Kernel driver in use: xhci_hcd
lsusb
/: Bus 04.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/6p, 5000M
|__ Port 5: Dev 2, If 0, Class=Hub, Driver=hub/4p, 5000M
|__ Port 6: Dev 3, If 0, Class=Hub, Driver=hub/4p, 5000M
/: Bus 03.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/14p, 480M
|__ Port 9: Dev 4, If 0, Class=Hub, Driver=hub/4p, 480M
|__ Port 10: Dev 5, If 0, Class=Hub, Driver=hub/4p, 480M
|__ Port 4: Dev 8, If 0, Class=Human Interface Device,
Driver=usbhid, 12M
|__ Port 4: Dev 8, If 1, Class=Human Interface Device,
Driver=usbhid, 12M
|__ Port 4: Dev 8, If 2, Class=Human Interface Device,
Driver=usbhid, 12M
/: Bus 02.Port 1: Dev 1, Class=root_hub, Driver=ehci-pci/2p, 480M
|__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/8p, 480M
/: Bus 01.Port 1: Dev 1, Class=root_hub, Driver=ehci-pci/2p, 480M
|__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/6p, 480M
On Tue, Jul 9, 2013 at 9:26 AM, Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
> On 07/09/2013 06:44 AM, wujun zhou wrote:
>> Hello Jiri,
>> Thanks for the quick reply.
>>
>>> Does the keyboard/mouse get properly discovered if you unplug and replug
>>> the receiver?
>> No. It is still not working after replug or reboot
>>
>>> What if you revert back to the 59626408 workaround (i.e. revert
>>> a9dd22b730 and 8af6c0883)?
>> It works after revert 8af6c08830b1ae114d1a8b548b1f8b056e068887.
>>
>>> so it's either rather timing sensitive, or device-specific.
>> The problem is indeed timing sensitive. Yesterday, after I inserted
>> some dbg_hid() for debuging, the problem just gone, very strange.
>>
>>
>> I think for my case, I find the reason of the problem. My device do
>> not report the expected REPORT_TYPE_NOTIF_DEVICE_PAIRED(0x41). (Not
>> discarded by the hid-core as mentioned in 8af6c0883, the hardware
>> itself has not sent the packet.)
>> Instead, the device reports a
>> REPORT_TYPE_NOTIF_CONNECTION_STATUS(0x42) event, with
>> dj_report->report_params[CONNECTION_STATUS_PARAM_STATUS]=0, the first
>> time I move the mouse.
>> And then report the normal activities of the mouse. (details about the
>> usb packets can be found in the attachment.)
>>
>
> Thanks for the logs. Your analysis is nearly good :) the problem comes
> from the frame 49 and not the 50 in your pcap logs. When calling the
> command REPORT_TYPE_CMD_GET_PAIRED_DEVICES (20 ff 81), we get an error
> -EPIPE. That means that the receiver never got the request for him to
> send the REPORT_TYPE_NOTIF_DEVICE_PAIRED(0x41).
>
> So this bug is definitively linked to the bugs closed by commit
> dcd9006b1b053c7b1cebe81333261d4fd492ffeb. There is something different
> with USB 3 hubs that triggers this -EPIPE. The urb is still not ready
> while it should.
>
> I am trying a long shot here, but could you please give a test to the
> following patch?
>
> Cheers,
> Benjamin
>
>
> From d73c86cff1be22e264fd857ebdc1fd16d07edaad Mon Sep 17 00:00:00 2001
> From: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> Date: Tue, 9 Jul 2013 18:22:29 +0200
> Subject: [PATCH] HID: logitech-dj: wait for URB completion before asking
> paired devices
>
> On some USB3 boards, once the receiver is switched to the DJ mode, if
> we try to directly call another SET_REPORT request, we get an error
> -EPIPE.
>
> Waiting for the command to complete fixes this.
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---
> drivers/hid/hid-logitech-dj.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
> index 5207591a..9a6ccf8 100644
> --- a/drivers/hid/hid-logitech-dj.c
> +++ b/drivers/hid/hid-logitech-dj.c
> @@ -766,6 +766,9 @@ static int logi_dj_probe(struct hid_device *hdev,
> /* Allow incoming packets to arrive: */
> hid_device_io_start(hdev);
>
> + /* wait for the receiver to accept other commands */
> + hid_hw_wait(hdev);
> +
> retval = logi_dj_recv_query_paired_devices(djrcv_dev);
> if (retval < 0) {
> dev_err(&hdev->dev, "%s:logi_dj_recv_query_paired_devices "
> --
> 1.8.3.1
>
>
[-- Attachment #2: logitech_with_patch.pcapng.gz --]
[-- Type: application/x-gzip, Size: 1658 bytes --]
^ permalink raw reply
* Re: My logitech wireless keyboard and mouse stop working after 3.10 kernel
From: Benjamin Tissoires @ 2013-07-09 16:26 UTC (permalink / raw)
To: wujun zhou
Cc: Jiri Kosina, linux-input, Andrew de los Reyes,
Nestor Lopez Casado, Linus Torvalds
In-Reply-To: <CAPf1XsMECGhdXLLm8zw9s13pOKDrWQV2aMp-vWSVrrDNvTYv5g@mail.gmail.com>
On 07/09/2013 06:44 AM, wujun zhou wrote:
> Hello Jiri,
> Thanks for the quick reply.
>
>> Does the keyboard/mouse get properly discovered if you unplug and replug
>> the receiver?
> No. It is still not working after replug or reboot
>
>> What if you revert back to the 59626408 workaround (i.e. revert
>> a9dd22b730 and 8af6c0883)?
> It works after revert 8af6c08830b1ae114d1a8b548b1f8b056e068887.
>
>> so it's either rather timing sensitive, or device-specific.
> The problem is indeed timing sensitive. Yesterday, after I inserted
> some dbg_hid() for debuging, the problem just gone, very strange.
>
>
> I think for my case, I find the reason of the problem. My device do
> not report the expected REPORT_TYPE_NOTIF_DEVICE_PAIRED(0x41). (Not
> discarded by the hid-core as mentioned in 8af6c0883, the hardware
> itself has not sent the packet.)
> Instead, the device reports a
> REPORT_TYPE_NOTIF_CONNECTION_STATUS(0x42) event, with
> dj_report->report_params[CONNECTION_STATUS_PARAM_STATUS]=0, the first
> time I move the mouse.
> And then report the normal activities of the mouse. (details about the
> usb packets can be found in the attachment.)
>
Thanks for the logs. Your analysis is nearly good :) the problem comes
from the frame 49 and not the 50 in your pcap logs. When calling the
command REPORT_TYPE_CMD_GET_PAIRED_DEVICES (20 ff 81), we get an error
-EPIPE. That means that the receiver never got the request for him to
send the REPORT_TYPE_NOTIF_DEVICE_PAIRED(0x41).
So this bug is definitively linked to the bugs closed by commit
dcd9006b1b053c7b1cebe81333261d4fd492ffeb. There is something different
with USB 3 hubs that triggers this -EPIPE. The urb is still not ready
while it should.
I am trying a long shot here, but could you please give a test to the
following patch?
Cheers,
Benjamin
>From d73c86cff1be22e264fd857ebdc1fd16d07edaad Mon Sep 17 00:00:00 2001
From: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Date: Tue, 9 Jul 2013 18:22:29 +0200
Subject: [PATCH] HID: logitech-dj: wait for URB completion before asking
paired devices
On some USB3 boards, once the receiver is switched to the DJ mode, if
we try to directly call another SET_REPORT request, we get an error
-EPIPE.
Waiting for the command to complete fixes this.
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
drivers/hid/hid-logitech-dj.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index 5207591a..9a6ccf8 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -766,6 +766,9 @@ static int logi_dj_probe(struct hid_device *hdev,
/* Allow incoming packets to arrive: */
hid_device_io_start(hdev);
+ /* wait for the receiver to accept other commands */
+ hid_hw_wait(hdev);
+
retval = logi_dj_recv_query_paired_devices(djrcv_dev);
if (retval < 0) {
dev_err(&hdev->dev, "%s:logi_dj_recv_query_paired_devices "
--
1.8.3.1
^ permalink raw reply related
* Re: Logitech M705: Thumb button (was Re: [PATCH v4] HID: Add full support for Logitech Unifying receivers)
From: Benjamin Tissoires @ 2013-07-09 13:40 UTC (permalink / raw)
To: Nestor Lopez Casado
Cc: Jiri Kosina, Markus Trippelsdorf, Dmitry Torokhov,
Richard Titmuss, Olivier Gay, open list:HID CORE LAYER,
linux-kernel@vger.kernel.org, Jean-Daniel Zanone,
František Fuka
In-Reply-To: <CAE7qMrosUkhF3Yak11Mh81NcGhYduUKRoHiUwH4RbTN1KXqi8A@mail.gmail.com>
Hi Nestor,
On Wed, Jul 3, 2013 at 12:46 PM, Nestor Lopez Casado
<nlopezcasad@logitech.com> wrote:
> Hi all,
>
> Sorry it took me some time to answer.
>
> I extended the Logitech HIDPP10 specification here:
> https://drive.google.com/folderview?id=0BxbRzx7vEV7eWmgwazJ3NUFfQ28&usp=sharing
Thanks :)
> [...]
>> Let me resurrect this 2 years old thread, as I have received multiple
>> reports since then, stating that some revisions of M705 don't have working
>> thumb button.
>>
>> Question mostly to the Logitech guys: Nestor, Olivier, could you please
>> provide us with the sequence that needs to be sent to (certain revisisons
>> of) M705 to let enter the debug mode, making the thumb button actually
>> emit a USB message?
>
>
> There are two versions of FW in the field:
>
> 1) Version 17.01.Build 0017.
> This version reports the thumb button by default as HID button 6.
>
> 2) Version 17.00.Build 0015
> This version does not report the thumb button by default.
>
> It is possible to enable the M705 with FW 17.00.Build 0015 to send the
> thumb button as HID button 6 via HIDPP10 command. But this will have a
> (undesirable ?) side effect.
> Once the button 6 is enabled, the buttons aside to the wheel (tilt
> buttons) will no longer be reported as AC pan HID usage, they will be
> reported as buttons 7 & 8. This might disrupt some users that rely on
> the mouse AC pan feature.
This side effect can be solved by handling the magic sequence in the
kernel, and do the mapping in the kernel also.
This would need to add a special driver for the M705 though. The good
point is that both the firmware behave the same way when they are
switched to the special mode, meaning that we don't need to check the
build number and do the switch as soon as we see the M705 PID.
>
> To enable HID button 6 reporting from thumb button we need to clear
> the bit 1 (Special button function) in register 0x01 on the M705
> READ register: 10 IDX 81 01 r0 r1 r2
> Clear Bit1: r0 &= 0xFD;
> Write back register: 10 IDX 80 01 r0 r1 r2
>
> IDX is the device index on the Unifying receiver.
>
> Note that if the command is sent to all M705 devices, users that have
> the 17.01.Build 0017 will lose the Ac pan and will gain nothing.
>
For those who wants to try, I have set up a project [1] to implement
part of the spec Nestor released.
I implemented various features: list devices paired to a receiver,
pair/unpair a device, and toggle the button 6 on M705 mouse with the
faulty firmware.
When switching mode for the M705, as Nestor said, we lose the AC
panning ability until the correct remapping is done in the kernel.
Cheers,
Benjamin
[1] https://github.com/bentiss/uLogitech
^ permalink raw reply
* Re: Input: cyttsp4 - SPI driver for Cypress TMA4XX touchscreen devices
From: Ferruh Yigit @ 2013-07-09 7:12 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Javier Martinez Canillas, Dmitry Torokhov,
linux-input@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <CAMuHMdXS=35MVinOPNRKi=bAzVpqvL-QEm+QsciO1Xe-BYzc4Q@mail.gmail.com>
On 07/07/2013 10:15 PM, Geert Uytterhoeven wrote:
> On Fri, Jul 5, 2013 at 1:51 AM, Linux Kernel Mailing List
> <linux-kernel@vger.kernel.org> wrote:
>> +++ b/drivers/input/touchscreen/cyttsp4_spi.c
>
>> +static int cyttsp_spi_xfer(struct device *dev, u8 *xfer_buf,
>> + u8 op, u8 reg, u8 *buf, int length)
>> +{
>
>> + if (reg > 255)
>
> As "reg" is "u8", this is never true:
>
> drivers/input/touchscreen/cyttsp4_spi.c: In function ‘cyttsp_spi_xfer’:
> drivers/input/touchscreen/cyttsp4_spi.c:66: warning: comparison is
> always false due to limited range of data type
>
>> + wr_buf[0] = op + CY_SPI_A8_BIT;
>> + else
>> + wr_buf[0] = op;
>
> Can the if-clause and the first branch just be removed, or is there a real bug
> involved (e.g. wrong type for "reg")?
Yes there was a bug here, and already sent a patch for this, please
check https://patchwork.kernel.org/patch/2820561/
thanks,
ferruh
This message and any attachments may contain Cypress (or its subsidiaries) confidential information. If it has been received in error, please advise the sender and immediately delete this message.
^ permalink raw reply
* Re: My logitech wireless keyboard and mouse stop working after 3.10 kernel
From: wujun zhou @ 2013-07-09 4:44 UTC (permalink / raw)
To: Jiri Kosina
Cc: linux-input, Benjamin Tissoires, Andrew de los Reyes,
Nestor Lopez Casado
In-Reply-To: <alpine.LNX.2.00.1307071153210.26857@pobox.suse.cz>
[-- Attachment #1: Type: text/plain, Size: 5338 bytes --]
Hello Jiri,
Thanks for the quick reply.
> Does the keyboard/mouse get properly discovered if you unplug and replug
> the receiver?
No. It is still not working after replug or reboot
> What if you revert back to the 59626408 workaround (i.e. revert
> a9dd22b730 and 8af6c0883)?
It works after revert 8af6c08830b1ae114d1a8b548b1f8b056e068887.
> so it's either rather timing sensitive, or device-specific.
The problem is indeed timing sensitive. Yesterday, after I inserted
some dbg_hid() for debuging, the problem just gone, very strange.
I think for my case, I find the reason of the problem. My device do
not report the expected REPORT_TYPE_NOTIF_DEVICE_PAIRED(0x41). (Not
discarded by the hid-core as mentioned in 8af6c0883, the hardware
itself has not sent the packet.)
Instead, the device reports a
REPORT_TYPE_NOTIF_CONNECTION_STATUS(0x42) event, with
dj_report->report_params[CONNECTION_STATUS_PARAM_STATUS]=0, the first
time I move the mouse.
And then report the normal activities of the mouse. (details about the
usb packets can be found in the attachment.)
In the current code, function
drivers/hid/hid-logitech-dj.c:logi_dj_raw_event (Line 629) will
discard that notification event silently.
switch (dj_report->report_type){
...
case REPORT_TYPE_NOTIF_CONNECTION_STATUS:
if (dj_report->report_params[CONNECTION_STATUS_PARAM_STATUS] ==
STATUS_LINKLOSS) { /* In
the case of the notification event, the STATUS is 0
(STATUS_LINKLOSS==1)*/
logi_dj_recv_forward_null_report(djrcv_dev, dj_report);
}
break;
...
When revert the 8af6c08830b1ae114d1a8b548b1f8b056e068887 (Revert "HID:
Fix logitech-dj: missing Unifying device issue")
The mouse's normal report (report_type==2) will trigger the
logi_dj_recv_forward_report() function and the function will enqueue
that report and schedule the tasklet delayedwork_callback.
The event then fall to the default case of the dj_report.report_type
switch. Which will trigger the
logi_dj_recv_query_paired_devices(djrcv_dev) to ask a new report. And
the device will send back the expected REPORT_TYPE_NOTIF_DEVICE_PAIRED
packet after this.
The attachment logitech_not_work.pcapng.gz and logitech_work.pcapng.gz
are the usb packets grabbed by wireshark.
The logitech_not_work uses the v3.10 driver and the logitech_work uses
v3.10 and revert the 8af6c08830b
The strange one is the package 50. I think we expect a
REPORT_TYPE_NOTIF_DEVICE_PAIRED(0x41) event (and the device report
that event sometimes after I inserted some dbg_hid(), which may slow
things down a little bit.)
But I get this in my computer: (I am using intel i7 4770 CPU)
20:01:42:00:00:00:00:00:00:00:00:00:00:00:00
The third field is REPORT_TYPE_NOTIF_CONNECTION_STATUS (0x42 [66]).
And after this, the mouse report the move events, and we do not
receive any REPORT_TYPE_NOTIF_DEVICE_PAIRED event without another
SET_REPORT request.
The logitech_not_work_dmesg.txt and logitech_work_dmesg.txt are dmsgs
with hid.debug set to 2 (The dmesg and the above captured packets are
from different session, so there may be different)
Besides, I change the dbg_hid() statement inside logi_dj_raw_event to
print the type of the event(And I also move that statement after the
switch statement to get the type)
For the logitech_not_work_dmesg.txt(v3.10), there are a lot of events like this:
Jul 08 14:21:05 g kernel: drivers/hid/hid-logitech-dj.c:
logi_dj_raw_event, size:15,type:66
Jul 08 14:21:05 g kernel: drivers/hid/hid-logitech-dj.c:
djrcv_dev->paired_dj_devices[dj_report->device_index] is NULL, index 1
Jul 08 14:21:05 g kernel: drivers/hid/hid-logitech-dj.c:
logi_dj_raw_event, size:15,type:2
Jul 08 14:21:05 g kernel: drivers/hid/hid-logitech-dj.c:
djrcv_dev->paired_dj_devices[dj_report->device_index] is NULL, index 1
Jul 08 14:21:05 g kernel: drivers/hid/hid-logitech-dj.c:
logi_dj_raw_event, size:15,type:2
Jul 08 14:21:05 g kernel: drivers/hid/hid-logitech-dj.c:
djrcv_dev->paired_dj_devices[dj_report->device_index] is NULL, index 1
Jul 08 14:21:05 g kernel: drivers/hid/hid-logitech-dj.c:
logi_dj_raw_event, size:15,type:2
...
For logitech_work_dmesg.txt(with the revert of 8af6c08830b):
Jul 08 14:37:07 g kernel: drivers/hid/usbhid/hid-core.c: submitting
ctrl urb: Set_Report wValue=0x0220 wIndex=0x0002 wLength=15
Jul 08 14:37:22 g kernel: drivers/hid/hid-logitech-dj.c:
logi_dj_raw_event, size:15,type:66
Jul 08 14:37:22 g kernel: drivers/hid/hid-logitech-dj.c:
djrcv_dev->paired_dj_devices[dj_report->device_index] is NULL, index 1
Jul 08 14:37:22 g kernel: drivers/hid/hid-logitech-dj.c:
logi_dj_raw_event, size:15,type:2
Jul 08 14:37:22 g kernel: drivers/hid/hid-logitech-dj.c: delayedwork_callback
Jul 08 14:37:22 g kernel: drivers/hid/usbhid/hid-core.c: submitting
ctrl urb: Set_Report wValue=0x0220 wIndex=0x0002 wLength=15
Jul 08 14:37:22 g kernel: drivers/hid/hid-logitech-dj.c:
logi_dj_raw_event, size:15,type:65
The type:2 report trigger the Set_Report packet and the new status is
reported. Please pay special attention to the time of the type
66(0x42) packet, it happened 15 seconds after the last set_report. (It
is trigger when I move the mouse)
The simple_unifying_receive_path is a fix working on my computer. But
my knowledge of the HID subsystem is very limited. Thus, this patch is
only for demonstration purpose.
Best,
Wujun Zhou
[-- Attachment #2: logitech_not_work_dmesg.txt --]
[-- Type: text/plain, Size: 4199 bytes --]
Jul 08 14:21:04 g kernel: usb 3-10.3: new full-speed USB device number 13 using xhci_hcd
Jul 08 14:21:04 g kernel: usb 3-10.3: ep0 maxpacket = 8
Jul 08 14:21:04 g kernel: usb 3-10.3: Successful evaluate context command
Jul 08 14:21:04 g kernel: usb 3-10.3: skipped 1 descriptor after interface
Jul 08 14:21:04 g kernel: usb 3-10.3: skipped 1 descriptor after interface
Jul 08 14:21:04 g kernel: usb 3-10.3: skipped 1 descriptor after interface
Jul 08 14:21:04 g kernel: usb 3-10.3: default language 0x0409
Jul 08 14:21:04 g kernel: usb 3-10.3: udev 13, busnum 3, minor = 268
Jul 08 14:21:04 g kernel: usb 3-10.3: New USB device found, idVendor=046d, idProduct=c52b
Jul 08 14:21:04 g kernel: usb 3-10.3: New USB device strings: Mfr=1, Product=2, SerialNumber=0
Jul 08 14:21:04 g kernel: usb 3-10.3: Product: USB Receiver
Jul 08 14:21:04 g kernel: usb 3-10.3: Manufacturer: Logitech
Jul 08 14:21:04 g kernel: usb 3-10.3: usb_probe_device
Jul 08 14:21:04 g kernel: usb 3-10.3: configuration #1 chosen from 1 choice
Jul 08 14:21:04 g kernel: usb 3-10.3: Successful Endpoint Configure command
Jul 08 14:21:04 g kernel: usb 3-10.3: adding 3-10.3:1.0 (config #1, interface 0)
Jul 08 14:21:04 g kernel: usbhid 3-10.3:1.0: usb_probe_interface
Jul 08 14:21:04 g kernel: usbhid 3-10.3:1.0: usb_probe_interface - got id
Jul 08 14:21:04 g kernel: drivers/hid/usbhid/hid-core.c: HID probe called for ifnum 0
Jul 08 14:21:04 g kernel: drivers/hid/hid-logitech-dj.c: logi_dj_probe called for ifnum 0
Jul 08 14:21:04 g kernel: drivers/hid/hid-logitech-dj.c: logi_dj_probe: ignoring ifnum 0
Jul 08 14:21:04 g kernel: usb 3-10.3: adding 3-10.3:1.1 (config #1, interface 1)
Jul 08 14:21:04 g kernel: usbhid 3-10.3:1.1: usb_probe_interface
Jul 08 14:21:04 g kernel: usbhid 3-10.3:1.1: usb_probe_interface - got id
Jul 08 14:21:04 g kernel: drivers/hid/usbhid/hid-core.c: HID probe called for ifnum 1
Jul 08 14:21:04 g kernel: drivers/hid/hid-logitech-dj.c: logi_dj_probe called for ifnum 1
Jul 08 14:21:04 g kernel: drivers/hid/hid-logitech-dj.c: logi_dj_probe: ignoring ifnum 1
Jul 08 14:21:04 g kernel: usb 3-10.3: adding 3-10.3:1.2 (config #1, interface 2)
Jul 08 14:21:04 g kernel: usbhid 3-10.3:1.2: usb_probe_interface
Jul 08 14:21:04 g kernel: usbhid 3-10.3:1.2: usb_probe_interface - got id
Jul 08 14:21:04 g kernel: drivers/hid/usbhid/hid-core.c: HID probe called for ifnum 2
Jul 08 14:21:04 g kernel: drivers/hid/hid-logitech-dj.c: logi_dj_probe called for ifnum 2
Jul 08 14:21:04 g kernel: drivers/hid/usbhid/hid-core.c: submitting ctrl urb: Get_Report wValue=0x0110 wIndex=0x0002 wLength=7
Jul 08 14:21:04 g kernel: drivers/hid/usbhid/hid-core.c: submitting ctrl urb: Get_Report wValue=0x0111 wIndex=0x0002 wLength=20
Jul 08 14:21:04 g kernel: drivers/hid/usbhid/hid-core.c: submitting ctrl urb: Get_Report wValue=0x0120 wIndex=0x0002 wLength=15
Jul 08 14:21:04 g kernel: drivers/hid/usbhid/hid-core.c: submitting ctrl urb: Get_Report wValue=0x0121 wIndex=0x0002 wLength=32
Jul 08 14:21:04 g kernel: usbhid 3-10.3:1.2: looking for a minor, starting at 96
Jul 08 14:21:04 g kernel: logitech-djreceiver 0003:046D:C52B.0029: hiddev0,hidraw3: USB HID v1.11 Device [Logitech USB Receiver] on usb-0000:00:14
Jul 08 14:21:04 g kernel: drivers/hid/usbhid/hid-core.c: submitting ctrl urb: Set_Report wValue=0x0220 wIndex=0x0002 wLength=15
Jul 08 14:21:04 g kernel: drivers/hid/usbhid/hid-core.c: submitting ctrl urb: Set_Report wValue=0x0220 wIndex=0x0002 wLength=15
Jul 08 14:21:05 g kernel: drivers/hid/hid-logitech-dj.c: logi_dj_raw_event, size:15,type:66
Jul 08 14:21:05 g kernel: drivers/hid/hid-logitech-dj.c: djrcv_dev->paired_dj_devices[dj_report->device_index] is NULL, index 1
Jul 08 14:21:05 g kernel: drivers/hid/hid-logitech-dj.c: logi_dj_raw_event, size:15,type:2
Jul 08 14:21:05 g kernel: drivers/hid/hid-logitech-dj.c: djrcv_dev->paired_dj_devices[dj_report->device_index] is NULL, index 1
Jul 08 14:21:05 g kernel: drivers/hid/hid-logitech-dj.c: logi_dj_raw_event, size:15,type:2
Jul 08 14:21:05 g kernel: drivers/hid/hid-logitech-dj.c: djrcv_dev->paired_dj_devices[dj_report->device_index] is NULL, index 1
Jul 08 14:21:05 g kernel: drivers/hid/hid-logitech-dj.c: logi_dj_raw_event, size:15,type:2
[-- Attachment #3: logitech_not_work.pcapng.gz --]
[-- Type: application/x-gzip, Size: 1682 bytes --]
[-- Attachment #4: logitech_work.pcapng.gz --]
[-- Type: application/x-gzip, Size: 2393 bytes --]
[-- Attachment #5: logitech_work_dmesg.txt --]
[-- Type: text/plain, Size: 7199 bytes --]
Jul 08 14:37:07 g kernel: hub 3-10:1.0: state 7 ports 4 chg 0000 evt 0010
Jul 08 14:37:07 g kernel: hub 3-10:1.0: port 4, status 0101, change 0001, 12 Mb/s
Jul 08 14:37:07 g kernel: hub 3-10:1.0: debounce: port 4: total 100ms stable 100ms status 0x101
Jul 08 14:37:07 g kernel: usb 3-10.4: new full-speed USB device number 15 using xhci_hcd
Jul 08 14:37:07 g kernel: usb 3-10.4: ep0 maxpacket = 8
Jul 08 14:37:07 g kernel: usb 3-10.4: Successful evaluate context command
Jul 08 14:37:07 g kernel: usb 3-10.4: skipped 1 descriptor after interface
Jul 08 14:37:07 g kernel: usb 3-10.4: skipped 1 descriptor after interface
Jul 08 14:37:07 g kernel: usb 3-10.4: skipped 1 descriptor after interface
Jul 08 14:37:07 g kernel: usb 3-10.4: default language 0x0409
Jul 08 14:37:07 g kernel: usb 3-10.4: udev 15, busnum 3, minor = 270
Jul 08 14:37:07 g kernel: usb 3-10.4: New USB device found, idVendor=046d, idProduct=c52b
Jul 08 14:37:07 g kernel: usb 3-10.4: New USB device strings: Mfr=1, Product=2, SerialNumber=0
Jul 08 14:37:07 g kernel: usb 3-10.4: Product: USB Receiver
Jul 08 14:37:07 g kernel: usb 3-10.4: Manufacturer: Logitech
Jul 08 14:37:07 g kernel: usb 3-10.4: usb_probe_device
Jul 08 14:37:07 g kernel: usb 3-10.4: configuration #1 chosen from 1 choice
Jul 08 14:37:07 g kernel: usb 3-10.4: Successful Endpoint Configure command
Jul 08 14:37:07 g kernel: usb 3-10.4: adding 3-10.4:1.0 (config #1, interface 0)
Jul 08 14:37:07 g kernel: usbhid 3-10.4:1.0: usb_probe_interface
Jul 08 14:37:07 g kernel: usbhid 3-10.4:1.0: usb_probe_interface - got id
Jul 08 14:37:07 g kernel: drivers/hid/usbhid/hid-core.c: HID probe called for ifnum 0
Jul 08 14:37:07 g kernel: drivers/hid/hid-logitech-dj.c: logi_dj_probe called for ifnum 0
Jul 08 14:37:07 g kernel: drivers/hid/hid-logitech-dj.c: logi_dj_probe: ignoring ifnum 0
Jul 08 14:37:07 g kernel: usb 3-10.4: adding 3-10.4:1.1 (config #1, interface 1)
Jul 08 14:37:07 g kernel: usbhid 3-10.4:1.1: usb_probe_interface
Jul 08 14:37:07 g kernel: usbhid 3-10.4:1.1: usb_probe_interface - got id
Jul 08 14:37:07 g kernel: drivers/hid/usbhid/hid-core.c: HID probe called for ifnum 1
Jul 08 14:37:07 g kernel: drivers/hid/hid-logitech-dj.c: logi_dj_probe called for ifnum 1
Jul 08 14:37:07 g kernel: drivers/hid/hid-logitech-dj.c: logi_dj_probe: ignoring ifnum 1
Jul 08 14:37:07 g kernel: usb 3-10.4: adding 3-10.4:1.2 (config #1, interface 2)
Jul 08 14:37:07 g kernel: usbhid 3-10.4:1.2: usb_probe_interface
Jul 08 14:37:07 g kernel: usbhid 3-10.4:1.2: usb_probe_interface - got id
Jul 08 14:37:07 g kernel: drivers/hid/usbhid/hid-core.c: HID probe called for ifnum 2
Jul 08 14:37:07 g kernel: drivers/hid/hid-logitech-dj.c: logi_dj_probe called for ifnum 2
Jul 08 14:37:07 g kernel: drivers/hid/usbhid/hid-core.c: submitting ctrl urb: Get_Report wValue=0x0110 wIndex=0x0002 wLength=7
Jul 08 14:37:07 g kernel: drivers/hid/usbhid/hid-core.c: submitting ctrl urb: Get_Report wValue=0x0111 wIndex=0x0002 wLength=20
Jul 08 14:37:07 g kernel: drivers/hid/usbhid/hid-core.c: submitting ctrl urb: Get_Report wValue=0x0120 wIndex=0x0002 wLength=15
Jul 08 14:37:07 g kernel: drivers/hid/usbhid/hid-core.c: submitting ctrl urb: Get_Report wValue=0x0121 wIndex=0x0002 wLength=32
Jul 08 14:37:07 g kernel: usbhid 3-10.4:1.2: looking for a minor, starting at 96
Jul 08 14:37:07 g kernel: logitech-djreceiver 0003:046D:C52B.0031: hiddev0,hidraw3: USB HID v1.11 Device [Logitech USB Receiver] on usb-0000:00:14.0-10.4/input2
Jul 08 14:37:07 g kernel: drivers/hid/usbhid/hid-core.c: submitting ctrl urb: Set_Report wValue=0x0220 wIndex=0x0002 wLength=15
Jul 08 14:37:07 g kernel: drivers/hid/usbhid/hid-core.c: submitting ctrl urb: Set_Report wValue=0x0220 wIndex=0x0002 wLength=15
Jul 08 14:37:22 g kernel: drivers/hid/hid-logitech-dj.c: logi_dj_raw_event, size:15,type:66
Jul 08 14:37:22 g kernel: drivers/hid/hid-logitech-dj.c: djrcv_dev->paired_dj_devices[dj_report->device_index] is NULL, index 1
Jul 08 14:37:22 g kernel: drivers/hid/hid-logitech-dj.c: logi_dj_raw_event, size:15,type:2
Jul 08 14:37:22 g kernel: drivers/hid/hid-logitech-dj.c: delayedwork_callback
Jul 08 14:37:22 g kernel: drivers/hid/usbhid/hid-core.c: submitting ctrl urb: Set_Report wValue=0x0220 wIndex=0x0002 wLength=15
Jul 08 14:37:22 g kernel: drivers/hid/hid-logitech-dj.c: logi_dj_raw_event, size:15,type:65
Jul 08 14:37:22 g kernel: drivers/hid/hid-logitech-dj.c: delayedwork_callback
Jul 08 14:37:22 g kernel: drivers/hid/hid-logitech-dj.c: logi_dj_ll_parse
Jul 08 14:37:22 g kernel: drivers/hid/hid-logitech-dj.c: logi_dj_ll_parse: sending a mouse descriptor, reports_supported: 4
Jul 08 14:37:22 g kernel: drivers/hid/hid-logitech-dj.c: logi_dj_ll_start
Jul 08 14:37:22 g kernel: input: Logitech Unifying Device. Wireless PID:101b as /devices/pci0000:00/0000:00:14.0/usb3/3-10/3-10.4/3-10.4:1.2/0003:046D:C52B.0031/input/input22
Jul 08 14:37:22 g kernel: logitech-djdevice 0003:046D:C52B.0032: input,hidraw4: USB HID v1.11 Mouse [Logitech Unifying Device. Wireless PID:101b] on usb-0000:00:14.0-10.4:1
Jul 08 14:37:22 g kernel: drivers/hid/hid-logitech-dj.c: logi_dj_ll_open:usb-0000:00:14.0-10.4:1
Jul 08 14:37:22 g kernel: drivers/hid/hid-logitech-dj.c: logi_dj_raw_event, size:15,type:65
Jul 08 14:37:22 g kernel: drivers/hid/hid-logitech-dj.c: delayedwork_callback
Jul 08 14:37:22 g kernel: drivers/hid/hid-logitech-dj.c: logi_dj_ll_parse
Jul 08 14:37:22 g kernel: drivers/hid/hid-logitech-dj.c: logi_dj_ll_parse: sending a kbd descriptor, reports_supported: 401a
Jul 08 14:37:22 g kernel: drivers/hid/hid-logitech-dj.c: logi_dj_ll_parse: sending a multimedia report descriptor: 401a
Jul 08 14:37:22 g kernel: drivers/hid/hid-logitech-dj.c: logi_dj_ll_parse: sending a power keys report descriptor: 401a
Jul 08 14:37:22 g kernel: drivers/hid/hid-logitech-dj.c: logi_dj_ll_parse: need to send kbd leds report descriptor: 401a
Jul 08 14:37:22 g kernel: drivers/hid/hid-logitech-dj.c: logi_dj_ll_start
Jul 08 14:37:22 g kernel: input: Logitech Unifying Device. Wireless PID:4003 as /devices/pci0000:00/0000:00:14.0/usb3/3-10/3-10.4/3-10.4:1.2/0003:046D:C52B.0031/input/input23
Jul 08 14:37:22 g kernel: drivers/hid/hid-logitech-dj.c: logi_dj_ll_open:usb-0000:00:14.0-10.4:2
Jul 08 14:37:22 g kernel: logitech-djdevice 0003:046D:C52B.0033: input,hidraw5: USB HID v1.11 Keyboard [Logitech Unifying Device. Wireless PID:4003] on usb-0000:00:14.0-10.4:2
Jul 08 14:37:22 g kernel: drivers/hid/hid-logitech-dj.c: logi_dj_raw_event, size:15,type:65
Jul 08 14:37:22 g kernel: drivers/hid/hid-logitech-dj.c: delayedwork_callback
Jul 08 14:37:22 g kernel: drivers/hid/hid-logitech-dj.c: logi_dj_recv_add_djhid_device: device list is empty
Jul 08 14:37:22 g kernel: drivers/hid/hid-logitech-dj.c: logi_dj_raw_event, size:15,type:2
Jul 08 14:37:22 g kernel: drivers/hid/hid-logitech-dj.c: logi_dj_raw_event, size:15,type:2
Jul 08 14:37:22 g kernel: drivers/hid/hid-logitech-dj.c: logi_dj_raw_event, size:7,type:143
Jul 08 14:37:22 g kernel: drivers/hid/hid-logitech-dj.c: logi_dj_raw_event, size:15,type:2
Jul 08 14:37:22 g kernel: drivers/hid/hid-logitech-dj.c: logi_dj_raw_event, size:7,type:143
Jul 08 14:37:22 g kernel: drivers/hid/hid-logitech-dj.c: logi_dj_raw_event, size:15,type:2
[-- Attachment #6: simple_unifying_receiver_fix.patch --]
[-- Type: application/octet-stream, Size: 1936 bytes --]
From e39bdcd6da4de815f85e5aa2ceb19fe769d53688 Mon Sep 17 00:00:00 2001
From: Wujun Zhou <zwj.echo@gmail.com>
Date: Mon, 8 Jul 2013 20:54:49 -0700
Subject: [PATCH] HID: Fix logitech-dj: missing Unifying pair notification
---
drivers/hid/hid-logitech-dj.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index 5207591a..1a332b2 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -192,6 +192,7 @@ static struct hid_ll_driver logi_dj_ll_driver;
static int logi_dj_output_hidraw_report(struct hid_device *hid, u8 * buf,
size_t count,
unsigned char report_type);
+static int logi_dj_recv_query_paired_devices(struct dj_receiver_dev *djrcv_dev);
static void logi_dj_recv_destroy_djhid_device(struct dj_receiver_dev *djrcv_dev,
struct dj_report *dj_report)
@@ -305,6 +306,7 @@ static void delayedwork_callback(struct work_struct *work)
struct dj_report dj_report;
unsigned long flags;
int count;
+ int retval;
dbg_hid("%s\n", __func__);
@@ -336,6 +338,14 @@ static void delayedwork_callback(struct work_struct *work)
case REPORT_TYPE_NOTIF_DEVICE_UNPAIRED:
logi_dj_recv_destroy_djhid_device(djrcv_dev, &dj_report);
break;
+ case REPORT_TYPE_NOTIF_CONNECTION_STATUS:
+ retval = logi_dj_recv_query_paired_devices(djrcv_dev);
+ if (retval) {
+ dev_err(&djrcv_dev->hdev->dev,
+ "%s:logi_dj_recv_query_paired_devices "
+ "error:%d\n", __func__, retval);
+ }
+ break;
default:
dbg_hid("%s: unexpected report type\n", __func__);
}
@@ -672,6 +682,8 @@ static int logi_dj_raw_event(struct hid_device *hdev,
if (dj_report->report_params[CONNECTION_STATUS_PARAM_STATUS] ==
STATUS_LINKLOSS) {
logi_dj_recv_forward_null_report(djrcv_dev, dj_report);
+ } else {
+ logi_dj_recv_queue_notification(djrcv_dev, dj_report);
}
break;
default:
--
1.8.1.5
^ permalink raw reply related
* Re: [PATCH 22/51] Input: atmel_mxt_ts - Add shutdown function
From: Nick Dyer @ 2013-07-08 9:56 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Daniel Kurtz, Henrik Rydberg, Joonyoung Shim, Alan Bowens,
linux-input, linux-kernel, Peter Meerwald, Benson Leung,
Olof Johansson
In-Reply-To: <20130707052934.GB15453@core.coreip.homeip.net>
Dmitry Torokhov wrote:
> On Thu, Jun 27, 2013 at 01:48:57PM +0100, Nick Dyer wrote:
>> Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
>> Acked-by: Benson Leung <bleung@chromium.org>
>
> Why is this needed?
The patch disables the interrupt handler on shutdown.
One of our customers reported a bug caused by input events being generated
during shutdown (for example if the user was touching the device whilst it
was turning off), which was solved by putting in this change.
However, now you've drawn my attention to it again, it seems to me that
probably a better thing for us to be doing would be to power off the
touchscreen controller here, and let the interrupt disable be handled by
core code - do you agree?
^ permalink raw reply
* Re: [PATCH 26/51] Input: atmel_mxt_ts - Move input device init into separate function
From: Nick Dyer @ 2013-07-08 9:41 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Daniel Kurtz, Henrik Rydberg, Joonyoung Shim, Alan Bowens,
linux-input, linux-kernel, Peter Meerwald, Benson Leung,
Olof Johansson
In-Reply-To: <20130707053407.GC15453@core.coreip.homeip.net>
Dmitry Torokhov wrote:
> On Thu, Jun 27, 2013 at 01:49:01PM +0100, Nick Dyer wrote:
>> Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
>
> So before we allocated input device before requesting IRQ, now we fo it
> afterwards so there is moment where the interrupt is requested and not
> disabled and input device is not allocated yet. Is it possible for
> interrupt to happen at that moment?
Yes, and it will be handled correctly, there are guards in the correct
places to ensure the input device will not be used before being registered.
It is registered at this point since there are several paths that might
need the interrupt handler (for example, to handle flash if device is in
failed state, or to upload configuration if necessary).
^ permalink raw reply
* Re: [PATCH 3/3] input: mc13783: Add DT probe support
From: Markus Pargmann @ 2013-07-08 7:13 UTC (permalink / raw)
To: Alexander Shiyan
Cc: linux-arm-kernel, linux-input, Sascha Hauer, Grant Likely,
Rob Herring, Dmitry Torokhov, Shawn Guo
In-Reply-To: <1373086680-31444-3-git-send-email-shc_work@mail.ru>
Hi,
On Sat, Jul 06, 2013 at 08:58:00AM +0400, Alexander Shiyan wrote:
> Patch adds DT support for MC13783/MC13892 PMICs.
>
> Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> ---
> Documentation/devicetree/bindings/mfd/mc13xxx.txt | 12 +++++
> drivers/input/misc/mc13783-pwrbutton.c | 59 ++++++++++++++++++-----
> 2 files changed, 58 insertions(+), 13 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mfd/mc13xxx.txt b/Documentation/devicetree/bindings/mfd/mc13xxx.txt
> index abd9e3c..967bed6 100644
> --- a/Documentation/devicetree/bindings/mfd/mc13xxx.txt
> +++ b/Documentation/devicetree/bindings/mfd/mc13xxx.txt
> @@ -10,6 +10,11 @@ Optional properties:
> - fsl,mc13xxx-uses-touch : Indicate the touchscreen controller is being used
>
> Sub-nodes:
> +- buttons : Contain power button nodes. Each button should be declared as
> + "btn<num>" and contain code in "linux,code" property. Optional properties:
gpio_keys also defines button node bindings. Why don't you use the same
name format (button@<num>)?
> + active-high : Change active button level from 0 to 1.
> + enable-reset: Performs hadware reset through PMIC.
> + debounce : Debounce value which will be taken from PMIC datasheet.
> - regulators : Contain the regulator nodes. The regulators are bound using
> their names as listed below with their registers and bits for enabling.
>
> @@ -89,6 +94,13 @@ ecspi@70010000 { /* ECSPI1 */
> interrupt-parent = <&gpio0>;
> interrupts = <8>;
>
> + buttons {
> + btn1 {
> + linux,code = <0x1f>;
> + debounce = <1>;
> + };
> + };
> +
> regulators {
> sw1_reg: mc13892__sw1 {
> regulator-min-microvolt = <600000>;
> diff --git a/drivers/input/misc/mc13783-pwrbutton.c b/drivers/input/misc/mc13783-pwrbutton.c
> index 855d576..05bb570 100644
> --- a/drivers/input/misc/mc13783-pwrbutton.c
> +++ b/drivers/input/misc/mc13783-pwrbutton.c
> @@ -24,6 +24,7 @@
> #include <linux/kernel.h>
> #include <linux/errno.h>
> #include <linux/input.h>
> +#include <linux/of.h>
> #include <linux/platform_device.h>
> #include <linux/mfd/mc13783.h>
> #include <linux/mfd/mc13892.h>
> @@ -121,21 +122,28 @@ static int __init mc13xxx_pwrbutton_probe(struct platform_device *pdev)
> struct mc13xxx *mc13xxx = dev_get_drvdata(pdev->dev.parent);
> struct mc13xxx_pwrb_devtype *devtype =
> (struct mc13xxx_pwrb_devtype *)pdev->id_entry->driver_data;
> + struct device_node *parent, *child;
> struct mc13xxx_pwrb *priv;
> int i, reg = 0, ret = -EINVAL;
>
> - if (!pdata) {
> + of_node_get(pdev->dev.parent->of_node);
> + parent = of_find_node_by_name(pdev->dev.parent->of_node, "buttons");
> + if (!pdata && !parent) {
> dev_err(&pdev->dev, "Missing platform data\n");
> return -ENODEV;
> }
>
> priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> - if (!priv)
> - return -ENOMEM;
> + if (!priv) {
> + ret = -ENOMEM;
> + goto out_node_put;
> + }
>
> priv->input = devm_input_allocate_device(&pdev->dev);
> - if (!priv->input)
> - return -ENOMEM;
> + if (!priv->input) {
> + ret = -ENOMEM;
> + goto out_node_put;
> + }
>
> priv->mc13xxx = mc13xxx;
> priv->devtype = devtype;
> @@ -143,15 +151,36 @@ static int __init mc13xxx_pwrbutton_probe(struct platform_device *pdev)
>
> for (i = 0; i < MAX13XXX_NUM_BUTTONS; i++) {
> u16 code, invert, reset, debounce;
> + const __be32 *prop;
> + char childname[5];
>
> - if (!(pdata->buttons[i].flags & MC13XXX_BUTTON_ENABLE))
> - continue;
> - code = pdata->buttons[i].keycode;
> - invert = !!(pdata->buttons[i].flags &
> - MC13XXX_BUTTON_POL_INVERT);
> - reset = !!(pdata->buttons[i].flags &
> - MC13XXX_BUTTON_RESET_EN);
> - debounce = pdata->buttons[i].flags;
> + if (parent) {
> + sprintf(childname, "btn%i", i + 1);
> + child = of_get_child_by_name(parent, childname);
> + if (!child)
> + continue;
> + prop = of_get_property(child, "linux,code", NULL);
> + if (prop)
> + code = be32_to_cpu(*prop) & 0xffff;
> + else {
> + dev_err(&pdev->dev,
> + "Button %i: Missing key code\n", i + 1);
> + continue;
> + }
> + invert = !!of_get_property(child, "active-high", NULL);
> + reset = !!of_get_property(child, "enable-reset", NULL);
You can use of_property_read_bool here.
Regards,
Markus
> + prop = of_get_property(child, "debounce", NULL);
> + debounce = prop ? be32_to_cpu(*prop) : 0;
> + } else {
> + if (!(pdata->buttons[i].flags & MC13XXX_BUTTON_ENABLE))
> + continue;
> + code = pdata->buttons[i].keycode;
> + invert = !!(pdata->buttons[i].flags &
> + MC13XXX_BUTTON_POL_INVERT);
> + reset = !!(pdata->buttons[i].flags &
> + MC13XXX_BUTTON_RESET_EN);
> + debounce = pdata->buttons[i].flags;
> + }
>
> priv->btn_code[i] = code;
> if (code != KEY_RESERVED)
> @@ -186,6 +215,10 @@ static int __init mc13xxx_pwrbutton_probe(struct platform_device *pdev)
> if (ret)
> dev_err(&pdev->dev, "Can't register input device: %i\n", ret);
>
> +out_node_put:
> + if (parent)
> + of_node_put(parent);
> +
> return ret;
> }
>
> --
> 1.8.1.5
>
>
^ permalink raw reply
* Re: Input: cyttsp4 - SPI driver for Cypress TMA4XX touchscreen devices
From: Geert Uytterhoeven @ 2013-07-07 19:15 UTC (permalink / raw)
To: Ferruh Yigit, Javier Martinez Canillas, Dmitry Torokhov
Cc: linux-input@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20130704235117.733726609A1@gitolite.kernel.org>
On Fri, Jul 5, 2013 at 1:51 AM, Linux Kernel Mailing List
<linux-kernel@vger.kernel.org> wrote:
> +++ b/drivers/input/touchscreen/cyttsp4_spi.c
> +static int cyttsp_spi_xfer(struct device *dev, u8 *xfer_buf,
> + u8 op, u8 reg, u8 *buf, int length)
> +{
> + if (reg > 255)
As "reg" is "u8", this is never true:
drivers/input/touchscreen/cyttsp4_spi.c: In function ‘cyttsp_spi_xfer’:
drivers/input/touchscreen/cyttsp4_spi.c:66: warning: comparison is
always false due to limited range of data type
> + wr_buf[0] = op + CY_SPI_A8_BIT;
> + else
> + wr_buf[0] = op;
Can the if-clause and the first branch just be removed, or is there a real bug
involved (e.g. wrong type for "reg")?
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
--
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] Input: cyttsp4 - Kill defined but not used compiler warnings
From: Geert Uytterhoeven @ 2013-07-07 19:08 UTC (permalink / raw)
To: Ferruh Yigit, Javier Martinez Canillas, Dmitry Torokhov
Cc: linux-input, linux-kernel, Geert Uytterhoeven
If both CONFIG_PM_SLEEP and CONFIG_PM_RUNTIME are unset:
drivers/input/touchscreen/cyttsp4_core.c:1556: warning: ‘cyttsp4_core_sleep’ defined but not used
drivers/input/touchscreen/cyttsp4_core.c:1634: warning: ‘cyttsp4_core_wake’ defined but not used
Move cyttsp4_core_sleep(), cyttsp4_core_wake(), and cyttsp4_core_wake_()
(which is called from cyttsp4_core_wake() only) inside the existing section
protected by #if defined(CONFIG_PM_SLEEP) || defined(CONFIG_PM_RUNTIME)
Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
drivers/input/touchscreen/cyttsp4_core.c | 200 +++++++++++++++---------------
1 file changed, 100 insertions(+), 100 deletions(-)
diff --git a/drivers/input/touchscreen/cyttsp4_core.c b/drivers/input/touchscreen/cyttsp4_core.c
index edcf799..c49161c2 100644
--- a/drivers/input/touchscreen/cyttsp4_core.c
+++ b/drivers/input/touchscreen/cyttsp4_core.c
@@ -1552,106 +1552,6 @@ exit:
return rc;
}
-static int cyttsp4_core_sleep(struct cyttsp4 *cd)
-{
- int rc;
-
- rc = cyttsp4_request_exclusive(cd, cd->dev,
- CY_CORE_SLEEP_REQUEST_EXCLUSIVE_TIMEOUT);
- if (rc < 0) {
- dev_err(cd->dev, "%s: fail get exclusive ex=%p own=%p\n",
- __func__, cd->exclusive_dev, cd->dev);
- return 0;
- }
-
- rc = cyttsp4_core_sleep_(cd);
-
- if (cyttsp4_release_exclusive(cd, cd->dev) < 0)
- dev_err(cd->dev, "%s: fail to release exclusive\n", __func__);
- else
- dev_vdbg(cd->dev, "%s: pass release exclusive\n", __func__);
-
- return rc;
-}
-
-static int cyttsp4_core_wake_(struct cyttsp4 *cd)
-{
- struct device *dev = cd->dev;
- int rc;
- u8 mode;
- int t;
-
- /* Already woken? */
- mutex_lock(&cd->system_lock);
- if (cd->sleep_state == SS_SLEEP_OFF) {
- mutex_unlock(&cd->system_lock);
- return 0;
- }
- cd->int_status &= ~CY_INT_IGNORE;
- cd->int_status |= CY_INT_AWAKE;
- cd->sleep_state = SS_WAKING;
-
- if (cd->cpdata->power) {
- dev_dbg(dev, "%s: Power up HW\n", __func__);
- rc = cd->cpdata->power(cd->cpdata, 1, dev, &cd->ignore_irq);
- } else {
- dev_dbg(dev, "%s: No power function\n", __func__);
- rc = -ENOSYS;
- }
- if (rc < 0) {
- dev_err(dev, "%s: HW Power up fails r=%d\n",
- __func__, rc);
-
- /* Initiate a read transaction to wake up */
- cyttsp4_adap_read(cd, CY_REG_BASE, sizeof(mode), &mode);
- } else
- dev_vdbg(cd->dev, "%s: HW power up succeeds\n",
- __func__);
- mutex_unlock(&cd->system_lock);
-
- t = wait_event_timeout(cd->wait_q,
- (cd->int_status & CY_INT_AWAKE) == 0,
- msecs_to_jiffies(CY_CORE_WAKEUP_TIMEOUT));
- if (IS_TMO(t)) {
- dev_err(dev, "%s: TMO waiting for wakeup\n", __func__);
- mutex_lock(&cd->system_lock);
- cd->int_status &= ~CY_INT_AWAKE;
- /* Try starting up */
- cyttsp4_queue_startup_(cd);
- mutex_unlock(&cd->system_lock);
- }
-
- mutex_lock(&cd->system_lock);
- cd->sleep_state = SS_SLEEP_OFF;
- mutex_unlock(&cd->system_lock);
-
- cyttsp4_start_wd_timer(cd);
-
- return 0;
-}
-
-static int cyttsp4_core_wake(struct cyttsp4 *cd)
-{
- int rc;
-
- rc = cyttsp4_request_exclusive(cd, cd->dev,
- CY_CORE_REQUEST_EXCLUSIVE_TIMEOUT);
- if (rc < 0) {
- dev_err(cd->dev, "%s: fail get exclusive ex=%p own=%p\n",
- __func__, cd->exclusive_dev, cd->dev);
- return 0;
- }
-
- rc = cyttsp4_core_wake_(cd);
-
- if (cyttsp4_release_exclusive(cd, cd->dev) < 0)
- dev_err(cd->dev, "%s: fail to release exclusive\n", __func__);
- else
- dev_vdbg(cd->dev, "%s: pass release exclusive\n", __func__);
-
- return rc;
-}
-
static int cyttsp4_startup_(struct cyttsp4 *cd)
{
int retry = CY_CORE_STARTUP_RETRY_COUNT;
@@ -1821,6 +1721,106 @@ static void cyttsp4_free_si_ptrs(struct cyttsp4 *cd)
}
#if defined(CONFIG_PM_SLEEP) || defined(CONFIG_PM_RUNTIME)
+static int cyttsp4_core_sleep(struct cyttsp4 *cd)
+{
+ int rc;
+
+ rc = cyttsp4_request_exclusive(cd, cd->dev,
+ CY_CORE_SLEEP_REQUEST_EXCLUSIVE_TIMEOUT);
+ if (rc < 0) {
+ dev_err(cd->dev, "%s: fail get exclusive ex=%p own=%p\n",
+ __func__, cd->exclusive_dev, cd->dev);
+ return 0;
+ }
+
+ rc = cyttsp4_core_sleep_(cd);
+
+ if (cyttsp4_release_exclusive(cd, cd->dev) < 0)
+ dev_err(cd->dev, "%s: fail to release exclusive\n", __func__);
+ else
+ dev_vdbg(cd->dev, "%s: pass release exclusive\n", __func__);
+
+ return rc;
+}
+
+static int cyttsp4_core_wake_(struct cyttsp4 *cd)
+{
+ struct device *dev = cd->dev;
+ int rc;
+ u8 mode;
+ int t;
+
+ /* Already woken? */
+ mutex_lock(&cd->system_lock);
+ if (cd->sleep_state == SS_SLEEP_OFF) {
+ mutex_unlock(&cd->system_lock);
+ return 0;
+ }
+ cd->int_status &= ~CY_INT_IGNORE;
+ cd->int_status |= CY_INT_AWAKE;
+ cd->sleep_state = SS_WAKING;
+
+ if (cd->cpdata->power) {
+ dev_dbg(dev, "%s: Power up HW\n", __func__);
+ rc = cd->cpdata->power(cd->cpdata, 1, dev, &cd->ignore_irq);
+ } else {
+ dev_dbg(dev, "%s: No power function\n", __func__);
+ rc = -ENOSYS;
+ }
+ if (rc < 0) {
+ dev_err(dev, "%s: HW Power up fails r=%d\n",
+ __func__, rc);
+
+ /* Initiate a read transaction to wake up */
+ cyttsp4_adap_read(cd, CY_REG_BASE, sizeof(mode), &mode);
+ } else
+ dev_vdbg(cd->dev, "%s: HW power up succeeds\n",
+ __func__);
+ mutex_unlock(&cd->system_lock);
+
+ t = wait_event_timeout(cd->wait_q,
+ (cd->int_status & CY_INT_AWAKE) == 0,
+ msecs_to_jiffies(CY_CORE_WAKEUP_TIMEOUT));
+ if (IS_TMO(t)) {
+ dev_err(dev, "%s: TMO waiting for wakeup\n", __func__);
+ mutex_lock(&cd->system_lock);
+ cd->int_status &= ~CY_INT_AWAKE;
+ /* Try starting up */
+ cyttsp4_queue_startup_(cd);
+ mutex_unlock(&cd->system_lock);
+ }
+
+ mutex_lock(&cd->system_lock);
+ cd->sleep_state = SS_SLEEP_OFF;
+ mutex_unlock(&cd->system_lock);
+
+ cyttsp4_start_wd_timer(cd);
+
+ return 0;
+}
+
+static int cyttsp4_core_wake(struct cyttsp4 *cd)
+{
+ int rc;
+
+ rc = cyttsp4_request_exclusive(cd, cd->dev,
+ CY_CORE_REQUEST_EXCLUSIVE_TIMEOUT);
+ if (rc < 0) {
+ dev_err(cd->dev, "%s: fail get exclusive ex=%p own=%p\n",
+ __func__, cd->exclusive_dev, cd->dev);
+ return 0;
+ }
+
+ rc = cyttsp4_core_wake_(cd);
+
+ if (cyttsp4_release_exclusive(cd, cd->dev) < 0)
+ dev_err(cd->dev, "%s: fail to release exclusive\n", __func__);
+ else
+ dev_vdbg(cd->dev, "%s: pass release exclusive\n", __func__);
+
+ return rc;
+}
+
static int cyttsp4_core_suspend(struct device *dev)
{
struct cyttsp4 *cd = dev_get_drvdata(dev);
--
1.7.9.5
--
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 related
* [PATCH] HID : core : fix hid delimiter local tag parsing.
From: pchavent @ 2013-07-07 16:35 UTC (permalink / raw)
To: jkosina, linux-input; +Cc: paul.chavent
[-- Attachment #1: Type: text/plain, Size: 318 bytes --]
Hi.
When i plug an USB HID device with the DELIMITER tag in its report
descriptor, the device doesn't load.
The problem comes from the "return 1" in the DELIMITER switch of the
local tags parsing.
I join a trivial patch that return no error when encounter the
DELIMITER tag.
Please review it.
Regards.
Paul.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-HID-core-fix-hid-delimiter-local-tag-parsing.patch --]
[-- Type: text/x-c; name=0001-HID-core-fix-hid-delimiter-local-tag-parsing.patch, Size: 674 bytes --]
From d34e7be36d6c392578ac9c81614325f9bff9de93 Mon Sep 17 00:00:00 2001
From: Paul Chavent <paul.chavent@onera.fr>
Date: Sun, 7 Jul 2013 17:43:56 +0200
Subject: [PATCH] HID : core : fix hid delimiter local tag parsing.
---
drivers/hid/hid-core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 36668d1..1e23ae9 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -450,7 +450,7 @@ static int hid_parser_local(struct hid_parser *parser, struct hid_item *item)
}
parser->local.delimiter_depth--;
}
- return 1;
+ return 0;
case HID_LOCAL_ITEM_TAG_USAGE:
--
1.7.12.1
^ permalink raw reply related
* Re: My logitech wireless keyboard and mouse stop working after 3.10 kernel
From: Jiri Kosina @ 2013-07-07 9:55 UTC (permalink / raw)
To: wujun zhou
Cc: linux-input, Benjamin Tissoires, Andrew de los Reyes,
Nestor Lopez Casado
In-Reply-To: <CAPf1XsNNqBajqi=SZ=_gcB9dQ1RSAvMnGn981gnN7JqqdThjhQ@mail.gmail.com>
On Sat, 6 Jul 2013, wujun zhou wrote:
> Dear maintainers,
>
> After I switch to the 3.10 kernel, my logitech wireless keyboard and mouse
> stop working.
>
> The keyboard and mouse are connected to the desktop though logitech
> unifying receiver. This little problem almost makes my desktop totally
> unusable. Could someone check and fix the problem? Thanks
>
> Keywords: hid, input, logitech unifying receiver
> Version: Linux version 3.10.0 (gcc version 4.7.3 (Gentoo 4.7.3 p1.0,
> pie-0.5.5) ) #6 SMP Sat Jul 6 21:29:14 PDT 2013
[ adding some CCs ]
Hi,
you are not the first one reporting problems getting worse with later
kernels with the unifying receiver.
Does the keyboard/mouse get properly discovered if you unplug and replug
the receiver? Linus reported that sometimes this is the case for him as
well.
Quoting my response to him two days ago:
===
we have had such reports before, but commit a9dd22b730 (and subsequent
revert of workaround fone by 59626408) was supposed to fix exactly that.
Any idea when this started happening to you?
What if you revert back to the 59626408 workaround (i.e. revert
a9dd22b730 and 8af6c0883)?
I have a couple of unifying receivers myself, but was never able to see
this problem ... so it's either rather timing sensitive, or
device-specific.
===
>
> In case the model matters:
> My mouse: logitech M705
> Keyboard: logitech K270
> The logitech unifying receiver came with one of them. (Sorry, I can't
> remember exactly which one it belong to)
>
> The driver stop working after this commit
> 4f5a81042909fed6977881f22c024aa3582cfcca. The previous version
> 83a44ac8bf4a8e6cbbf0c00ff281a482778f708a works. And I think it is caused by
> the changes to drivers/hid/hid-logitech-dj.c
>
> I get the following dmesg and /proc/bus/input/devices using 3.10 kernel
> [ 5.523741] logitech-djreceiver 0003:046D:C52B.0005: hiddev0,hidraw2:
> USB HID v1.11 Device [Logitech USB Receiver] on usb-0000:00:14.0-9.2/input2
>
> $ cat /proc/bus/input/devices
> ...
> I: Bus=0019 Vendor=0000 Product=0006 Version=0000
> N: Name="Video Bus"
> P: Phys=LNXVIDEO/video/input0
> S: Sysfs=/devices/LNXSYSTM:00/device:00/PNP0A08:00/LNXVIDEO:00/input/input2
> U: Uniq=
> H: Handlers=kbd event2
> B: PROP=0
> B: EV=3
> B: KEY=3e000b00000000 0 0 0
>
> The kernel find the receiver but not the connected keyboard and mouse.
>
>
> The dmesg and /proc/bus/input/devices when everything works:
> commit 83a44ac8bf4a8e6cbbf0c00ff281a482778f708a
>
> [ 7.479464] logitech-djreceiver 0003:046D:C52B.0003: hiddev0,hidraw0:
> USB HID v1.11 Device [Logitech USB Receiver] on usb-0000:00:14.0-9.2/input2
> [ 33.072330] input: Logitech Unifying Device. Wireless PID:101b as
> /devices/pci0000:00/0000:00:14.0/usb3/3-9/3-9.2/3-9.2:1.2/0003:046D:C52B.0003/input/input3
> [ 33.072550] logitech-djdevice 0003:046D:C52B.0004: input,hidraw1: USB
> HID v1.11 Mouse [Logitech Unifying Device. Wireless PID:101b] on
> usb-0000:00:14.0-9.2:1
> [ 33.074266] input: Logitech Unifying Device. Wireless PID:4003 as
> /devices/pci0000:00/0000:00:14.0/usb3/3-9/3-9.2/3-9.2:1.2/0003:046D:C52B.0003/input/input4
> [ 33.074380] logitech-djdevice 0003:046D:C52B.0005: input,hidraw2: USB
> HID v1.11 Keyboard [Logitech Unifying Device. Wireless PID:4003] on
> usb-0000:00:14.0-9.2:2
>
>
> $ cat /proc/bus/input/devices
> I: Bus=0019 Vendor=0000 Product=0001 Version=0000
> N: Name="Power Button"
> P: Phys=PNP0C0C/button/input0
> S: Sysfs=/devices/LNXSYSTM:00/device:00/PNP0C0C:00/input/input0
> U: Uniq=
> H: Handlers=kbd event0
> B: PROP=0
> B: EV=3
> B: KEY=10000000000000 0
>
> I: Bus=0019 Vendor=0000 Product=0001 Version=0000
> N: Name="Power Button"
> P: Phys=LNXPWRBN/button/input0
> S: Sysfs=/devices/LNXSYSTM:00/LNXPWRBN:00/input/input1
> U: Uniq=
> H: Handlers=kbd event1
> B: PROP=0
> B: EV=3
> B: KEY=10000000000000 0
>
> I: Bus=0019 Vendor=0000 Product=0006 Version=0000
> N: Name="Video Bus"
> P: Phys=LNXVIDEO/video/input0
> S: Sysfs=/devices/LNXSYSTM:00/device:00/PNP0A08:00/LNXVIDEO:00/input/input2
> U: Uniq=
> H: Handlers=kbd event2
> B: PROP=0
> B: EV=3
> B: KEY=3e000b00000000 0 0 0
>
> I: Bus=0003 Vendor=046d Product=c52b Version=0111
> N: Name="Logitech Unifying Device. Wireless PID:101b"
> P: Phys=usb-0000:00:14.0-9.2:1
> S:
> Sysfs=/devices/pci0000:00/0000:00:14.0/usb3/3-9/3-9.2/3-9.2:1.2/0003:046D:C52B.0003/input/input3
> U: Uniq=
> H: Handlers=mouse0 event3
> B: PROP=0
> B: EV=17
> B: KEY=ffff0000 0 0 0 0
> B: REL=143
> B: MSC=10
>
> I: Bus=0003 Vendor=046d Product=c52b Version=0111
> N: Name="Logitech Unifying Device. Wireless PID:4003"
> P: Phys=usb-0000:00:14.0-9.2:2
> S:
> Sysfs=/devices/pci0000:00/0000:00:14.0/usb3/3-9/3-9.2/3-9.2:1.2/0003:046D:C52B.0003/input/input4
> U: Uniq=
> H: Handlers=sysrq kbd event4
> B: PROP=0
> B: EV=12001f
> B: KEY=4837fff072ff32d bf54444600000000 1 30f908b17c007 ffff7bfad941dfff
> febeffdfffefffff fffffffffffffffe
> B: REL=40
> B: ABS=100000000
> B: MSC=10
> B: LED=1f
>
> Please let me know if you need more debug info. Thanks.
>
> Best,
> Wujun Zhou
>
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* My logitech wireless keyboard and mouse stop working after kernel v3.10
From: wujun zhou @ 2013-07-07 7:54 UTC (permalink / raw)
To: linux-input
Dear maintainers,
After I switch to the 3.10 kernel, my logitech wireless keyboard and
mouse stop working.
The keyboard and mouse are connected to the desktop though logitech
unifying receiver. This little problem almost makes my desktop totally
unusable. Could someone check and fix the problem? Thanks
Keywords: hid, input, logitech unifying receiver
Version: Linux version 3.10.0 (gcc version 4.7.3 (Gentoo 4.7.3 p1.0,
pie-0.5.5) ) #6 SMP Sat Jul 6 21:29:14 PDT 2013
In case the model matters:
My mouse: logitech M705
Keyboard: logitech K270
The logitech unifying receiver came with one of them. (Sorry, I can't
remember exactly which one it belong to)
The driver stop working after this commit
4f5a81042909fed6977881f22c024aa3582cfcca. The previous version
83a44ac8bf4a8e6cbbf0c00ff281a482778f708a works. And I think it is
caused by the changes to drivers/hid/hid-logitech-dj.c
I get the following dmesg and /proc/bus/input/devices using 3.10 kernel
[ 5.523741] logitech-djreceiver 0003:046D:C52B.0005:
hiddev0,hidraw2: USB HID v1.11 Device [Logitech USB Receiver] on
usb-0000:00:14.0-9.2/input2
$ cat /proc/bus/input/devices
...
I: Bus=0019 Vendor=0000 Product=0006 Version=0000
N: Name="Video Bus"
P: Phys=LNXVIDEO/video/input0
S: Sysfs=/devices/LNXSYSTM:00/device:00/PNP0A08:00/LNXVIDEO:00/input/input2
U: Uniq=
H: Handlers=kbd event2
B: PROP=0
B: EV=3
B: KEY=3e000b00000000 0 0 0
It seems that the kernel find the receiver but not the connected
keyboard and mouse.
The dmesg and /proc/bus/input/devices when everything works:
commit 83a44ac8bf4a8e6cbbf0c00ff281a482778f708a
[ 7.479464] logitech-djreceiver 0003:046D:C52B.0003:
hiddev0,hidraw0: USB HID v1.11 Device [Logitech USB Receiver] on
usb-0000:00:14.0-9.2/input2
[ 33.072330] input: Logitech Unifying Device. Wireless PID:101b as
/devices/pci0000:00/0000:00:14.0/usb3/3-9/3-9.2/3-9.2:1.2/0003:046D:C52B.0003/input/input3
[ 33.072550] logitech-djdevice 0003:046D:C52B.0004: input,hidraw1:
USB HID v1.11 Mouse [Logitech Unifying Device. Wireless PID:101b] on
usb-0000:00:14.0-9.2:1
[ 33.074266] input: Logitech Unifying Device. Wireless PID:4003 as
/devices/pci0000:00/0000:00:14.0/usb3/3-9/3-9.2/3-9.2:1.2/0003:046D:C52B.0003/input/input4
[ 33.074380] logitech-djdevice 0003:046D:C52B.0005: input,hidraw2:
USB HID v1.11 Keyboard [Logitech Unifying Device. Wireless PID:4003]
on usb-0000:00:14.0-9.2:2
$ cat /proc/bus/input/devices
I: Bus=0019 Vendor=0000 Product=0001 Version=0000
N: Name="Power Button"
P: Phys=PNP0C0C/button/input0
S: Sysfs=/devices/LNXSYSTM:00/device:00/PNP0C0C:00/input/input0
U: Uniq=
H: Handlers=kbd event0
B: PROP=0
B: EV=3
B: KEY=10000000000000 0
I: Bus=0019 Vendor=0000 Product=0001 Version=0000
N: Name="Power Button"
P: Phys=LNXPWRBN/button/input0
S: Sysfs=/devices/LNXSYSTM:00/LNXPWRBN:00/input/input1
U: Uniq=
H: Handlers=kbd event1
B: PROP=0
B: EV=3
B: KEY=10000000000000 0
I: Bus=0019 Vendor=0000 Product=0006 Version=0000
N: Name="Video Bus"
P: Phys=LNXVIDEO/video/input0
S: Sysfs=/devices/LNXSYSTM:00/device:00/PNP0A08:00/LNXVIDEO:00/input/input2
U: Uniq=
H: Handlers=kbd event2
B: PROP=0
B: EV=3
B: KEY=3e000b00000000 0 0 0
I: Bus=0003 Vendor=046d Product=c52b Version=0111
N: Name="Logitech Unifying Device. Wireless PID:101b"
P: Phys=usb-0000:00:14.0-9.2:1
S: Sysfs=/devices/pci0000:00/0000:00:14.0/usb3/3-9/3-9.2/3-9.2:1.2/0003:046D:C52B.0003/input/input3
U: Uniq=
H: Handlers=mouse0 event3
B: PROP=0
B: EV=17
B: KEY=ffff0000 0 0 0 0
B: REL=143
B: MSC=10
I: Bus=0003 Vendor=046d Product=c52b Version=0111
N: Name="Logitech Unifying Device. Wireless PID:4003"
P: Phys=usb-0000:00:14.0-9.2:2
S: Sysfs=/devices/pci0000:00/0000:00:14.0/usb3/3-9/3-9.2/3-9.2:1.2/0003:046D:C52B.0003/input/input4
U: Uniq=
H: Handlers=sysrq kbd event4
B: PROP=0
B: EV=12001f
B: KEY=4837fff072ff32d bf54444600000000 1 30f908b17c007
ffff7bfad941dfff febeffdfffefffff fffffffffffffffe
B: REL=40
B: ABS=100000000
B: MSC=10
B: LED=1f
I think the following bug report is also related to this.
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1194649
Please let me know if you need more debug info. Thanks.
Best,
Wujun Zhou
^ permalink raw reply
* Re: [PATCH 26/51] Input: atmel_mxt_ts - Move input device init into separate function
From: Dmitry Torokhov @ 2013-07-07 5:34 UTC (permalink / raw)
To: Nick Dyer
Cc: Daniel Kurtz, Henrik Rydberg, Joonyoung Shim, Alan Bowens,
linux-input, linux-kernel, Peter Meerwald, Benson Leung,
Olof Johansson
In-Reply-To: <1372337366-9286-27-git-send-email-nick.dyer@itdev.co.uk>
On Thu, Jun 27, 2013 at 01:49:01PM +0100, Nick Dyer wrote:
> Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
So before we allocated input device before requesting IRQ, now we fo it
afterwards so there is moment where the interrupt is requested and not
disabled and input device is not allocated yet. Is it possible for
interrupt to happen at that moment?
Thanks.
> ---
> drivers/input/touchscreen/atmel_mxt_ts.c | 129 +++++++++++++++++-------------
> 1 file changed, 75 insertions(+), 54 deletions(-)
>
> diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
> index 8632133..030ebc5 100644
> --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> @@ -1728,73 +1728,39 @@ static int mxt_handle_pdata(struct mxt_data *data)
> return 0;
> }
>
> -static int mxt_probe(struct i2c_client *client,
> - const struct i2c_device_id *id)
> +static int mxt_initialize_t9_input_device(struct mxt_data *data)
> {
> - struct mxt_data *data;
> + struct device *dev = &data->client->dev;
> + const struct mxt_platform_data *pdata = data->pdata;
> struct input_dev *input_dev;
> int error;
> unsigned int num_mt_slots;
> unsigned int mt_flags = 0;
> int i;
>
> - data = kzalloc(sizeof(struct mxt_data), GFP_KERNEL);
> input_dev = input_allocate_device();
> - if (!data || !input_dev) {
> - dev_err(&client->dev, "Failed to allocate memory\n");
> - error = -ENOMEM;
> - goto err_free_mem;
> + if (!input_dev) {
> + dev_err(dev, "Failed to allocate memory\n");
> + return -ENOMEM;
> }
>
> input_dev->name = "Atmel maXTouch Touchscreen";
> - snprintf(data->phys, sizeof(data->phys), "i2c-%u-%04x/input0",
> - client->adapter->nr, client->addr);
> -
> input_dev->phys = data->phys;
> -
> input_dev->id.bustype = BUS_I2C;
> - input_dev->dev.parent = &client->dev;
> + input_dev->dev.parent = dev;
> input_dev->open = mxt_input_open;
> input_dev->close = mxt_input_close;
>
> - data->client = client;
> - data->input_dev = input_dev;
> - data->irq = client->irq;
> - i2c_set_clientdata(client, data);
> -
> - error = mxt_handle_pdata(data);
> - if (error)
> - goto err_free_mem;
> -
> - init_completion(&data->bl_completion);
> - init_completion(&data->reset_completion);
> - init_completion(&data->crc_completion);
> -
> - error = request_threaded_irq(data->irq, NULL, mxt_interrupt,
> - data->pdata->irqflags | IRQF_ONESHOT,
> - client->name, data);
> - if (error) {
> - dev_err(&client->dev, "Failed to register interrupt\n");
> - goto err_free_pdata;
> - }
> -
> - disable_irq(client->irq);
> -
> - error = mxt_initialize(data);
> - if (error)
> - goto err_free_irq;
> -
> __set_bit(EV_ABS, input_dev->evbit);
> - __set_bit(EV_KEY, input_dev->evbit);
> - __set_bit(BTN_TOUCH, input_dev->keybit);
> + input_set_capability(input_dev, EV_KEY, BTN_TOUCH);
>
> - if (data->pdata->t19_num_keys) {
> + if (pdata->t19_num_keys) {
> __set_bit(INPUT_PROP_BUTTONPAD, input_dev->propbit);
>
> - for (i = 0; i < data->pdata->t19_num_keys; i++)
> - if (data->pdata->t19_keymap[i] != KEY_RESERVED)
> + for (i = 0; i < pdata->t19_num_keys; i++)
> + if (pdata->t19_keymap[i] != KEY_RESERVED)
> input_set_capability(input_dev, EV_KEY,
> - data->pdata->t19_keymap[i]);
> + pdata->t19_keymap[i]);
>
> mt_flags |= INPUT_MT_POINTER;
>
> @@ -1819,8 +1785,11 @@ static int mxt_probe(struct i2c_client *client,
> /* For multi touch */
> num_mt_slots = data->T9_reportid_max - data->T9_reportid_min + 1;
> error = input_mt_init_slots(input_dev, num_mt_slots, mt_flags);
> - if (error)
> - goto err_free_object;
> + if (error) {
> + dev_err(dev, "Error %d initialising slots\n", error);
> + goto err_free_mem;
> + }
> +
> input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR,
> 0, MXT_MAX_AREA, 0, 0);
> input_set_abs_params(input_dev, ABS_MT_POSITION_X,
> @@ -1834,11 +1803,64 @@ static int mxt_probe(struct i2c_client *client,
>
> error = input_register_device(input_dev);
> if (error) {
> - dev_err(&client->dev, "Error %d registering input device\n",
> - error);
> - goto err_free_object;
> + dev_err(dev, "Error %d registering input device\n", error);
> + goto err_free_mem;
> }
>
> + data->input_dev = input_dev;
> +
> + return 0;
> +
> +err_free_mem:
> + input_free_device(input_dev);
> + return error;
> +}
> +
> +static int mxt_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct mxt_data *data;
> + int error;
> +
> + data = kzalloc(sizeof(struct mxt_data), GFP_KERNEL);
> + if (!data) {
> + dev_err(&client->dev, "Failed to allocate memory\n");
> + return -ENOMEM;
> + }
> +
> + snprintf(data->phys, sizeof(data->phys), "i2c-%u-%04x/input0",
> + client->adapter->nr, client->addr);
> +
> + data->client = client;
> + data->irq = client->irq;
> + i2c_set_clientdata(client, data);
> +
> + error = mxt_handle_pdata(data);
> + if (error)
> + goto err_free_mem;
> +
> + init_completion(&data->bl_completion);
> + init_completion(&data->reset_completion);
> + init_completion(&data->crc_completion);
> +
> + error = request_threaded_irq(data->irq, NULL, mxt_interrupt,
> + data->pdata->irqflags | IRQF_ONESHOT,
> + client->name, data);
> + if (error) {
> + dev_err(&client->dev, "Failed to register interrupt\n");
> + goto err_free_pdata;
> + }
> +
> + disable_irq(data->irq);
> +
> + error = mxt_initialize(data);
> + if (error)
> + goto err_free_irq;
> +
> + error = mxt_initialize_t9_input_device(data);
> + if (error)
> + goto err_free_object;
> +
> error = sysfs_create_group(&client->dev.kobj, &mxt_attr_group);
> if (error) {
> dev_err(&client->dev, "Failure %d creating sysfs group\n",
> @@ -1849,8 +1871,8 @@ static int mxt_probe(struct i2c_client *client,
> return 0;
>
> err_unregister_device:
> - input_unregister_device(input_dev);
> - input_dev = NULL;
> + input_unregister_device(data->input_dev);
> + data->input_dev = NULL;
> err_free_object:
> kfree(data->object_table);
> err_free_irq:
> @@ -1859,7 +1881,6 @@ err_free_pdata:
> if (!dev_get_platdata(&data->client->dev))
> kfree(data->pdata);
> err_free_mem:
> - input_free_device(input_dev);
> kfree(data);
> return error;
> }
> --
> 1.7.10.4
>
--
Dmitry
^ 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