Login | Register For Free | Help
Search for: (Advanced)

Mailing List Archive: Linux: Kernel

[PATCH v3 0/6] hid: Introduce device groups

 

 

Linux kernel RSS feed   Index | Next | Previous | View Threaded


rydberg at euromail

Apr 23, 2012, 3:07 AM

Post #1 of 21 (221 views)
Permalink
[PATCH v3 0/6] hid: Introduce device groups

Hi Jiri,

Here is the third version of the extension to the device-driver
matching mechanism. AFAICT, there are no outstanding issues. I think
we are getting close. :-)

Building on the report fixup patches, this patchset brings a bit of
transparency to the hid bus, by the addition of device groups. This
allows a more versatile handling of hid drivers in userland, and
simplifies the logic in the kernel.

In particular, there can be one generic module per device group, and
those modules are handled automatically by udev. Dynamic rebinding of
drivers is fully supported. For instance, to load a special
out-of-tree driver instead of the a generic one, simply unbind the
device and load the new module. One can also keep _all_ generic
drivers as modules, significantly simplifying the process of adding
and testing new features.

And, of course, auto-loading of new multitouch drivers works, since
that was what triggered this patchset to begin with. :-)

The second patch contains a simple descriptor scanner that is
new. The last patch unifies the generic drivers into a single,
loadable module.

Enjoy,
Henrik

Henrik Rydberg (6):
hid: Add device group to modalias
hid: Scan the device for group info before adding it
hid: Allow bus wildcard matching
hid: Create a generic device group
hid-multitouch: Switch to device groups
hid: Create a common generic driver

drivers/hid/Kconfig | 12 +++
drivers/hid/Makefile | 2 +
drivers/hid/hid-core.c | 172 ++++++++++++++++++++-------------------
drivers/hid/hid-generic.c | 53 ++++++++++++
drivers/hid/hid-input.c | 11 ---
drivers/hid/hid-multitouch.c | 3 +-
drivers/hid/usbhid/hid-core.c | 16 ----
include/linux/hid.h | 20 +++--
include/linux/mod_devicetable.h | 4 +-
net/bluetooth/hidp/core.c | 27 +-----
scripts/mod/file2alias.c | 5 +-
11 files changed, 180 insertions(+), 145 deletions(-)
create mode 100644 drivers/hid/hid-generic.c

--
1.7.10
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


benjamin.tissoires at gmail

Apr 23, 2012, 3:51 AM

Post #2 of 21 (221 views)
Permalink
Re: [PATCH v3 0/6] hid: Introduce device groups [In reply to]

Hi Henrik,

Thanks for handling this. I still won't have the time to review this
patch set before early may as I'm out of my office till then. Also
sorry for not answering the previous mails, I did not had a reliable
internet connection during my holidays.

Thanks,
Benjamin

Le 23 avr. 2012 12:03, "Henrik Rydberg" <rydberg [at] euromail> a écrit :
>
> Hi Jiri.
>
> Here is the third version of the extension to the device-driver
> matching mechanism. AFAICT, there are no outstanding issues. I think
> we are getting close. :-)
>
> Building on the report fixup patches, this patchset brings a bit of
> transparency to the hid bus, by the addition of device groups. This
> allows a more versatile handling of hid drivers in userland, and
> simplifies the logic in the kernel.
>
> In particular, there can be one generic module per device group, and
> those modules are handled automatically by udev. Dynamic rebinding of
> drivers is fully supported. For instance, to load a special
> out-of-tree driver instead of the a generic one, simply unbind the
> device and load the new module. One can also keep _all_ generic
> drivers as modules, significantly simplifying the process of adding
> and testing new features.
>
> And, of course, auto-loading of new multitouch drivers works, since
> that was what triggered this patchset to begin with. :-)
>
> The second patch contains a simple descriptor scanner that is
> new. The last patch unifies the generic drivers into a single,
> loadable module.
>
> Enjoy,
> Henrik
>
> Henrik Rydberg (6):
> hid: Add device group to modalias
> hid: Scan the device for group info before adding it
> hid: Allow bus wildcard matching
> hid: Create a generic device group
> hid-multitouch: Switch to device groups
> hid: Create a common generic driver
>
> drivers/hid/Kconfig | 12 +++
> drivers/hid/Makefile | 2 +
> drivers/hid/hid-core.c | 172 ++++++++++++++++++++-------------------
> drivers/hid/hid-generic.c | 53 ++++++++++++
> drivers/hid/hid-input.c | 11 ---
> drivers/hid/hid-multitouch.c | 3 +-
> drivers/hid/usbhid/hid-core.c | 16 ----
> include/linux/hid.h | 20 +++--
> include/linux/mod_devicetable.h | 4 +-
> net/bluetooth/hidp/core.c | 27 +-----
> scripts/mod/file2alias.c | 5 +-
> 11 files changed, 180 insertions(+), 145 deletions(-)
> create mode 100644 drivers/hid/hid-generic.c
>
> --
> 1.7.10
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo [at] vger
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


benjamin.tissoires at gmail

Apr 23, 2012, 8:21 AM

Post #3 of 21 (222 views)
Permalink
Re: [PATCH v3 0/6] hid: Introduce device groups [In reply to]

Hi Henrik,

Thanks for handling this. I still won't have the time to review this
patch set before early may as I'm out of my office. Also sorry for not
answering the previous mails, I did not had a reliable internet
connection during my holidays.

Thanks,
Benjamin

Le 23 avr. 2012 12:03, "Henrik Rydberg" <rydberg [at] euromail> a écrit :
>
> Hi Jiri.
>
> Here is the third version of the extension to the device-driver
> matching mechanism. AFAICT, there are no outstanding issues. I think
> we are getting close. :-)
>
> Building on the report fixup patches, this patchset brings a bit of
> transparency to the hid bus, by the addition of device groups.  This
> allows a more versatile handling of hid drivers in userland, and
> simplifies the logic in the kernel.
>
> In particular, there can be one generic module per device group, and
> those modules are handled automatically by udev. Dynamic rebinding of
> drivers is fully supported. For instance, to load a special
> out-of-tree driver instead of the a generic one, simply unbind the
> device and load the new module. One can also keep _all_ generic
> drivers as modules, significantly simplifying the process of adding
> and testing new features.
>
> And, of course, auto-loading of new multitouch drivers works, since
> that was what triggered this patchset to begin with. :-)
>
> The second patch contains a simple descriptor scanner that is
> new. The last patch unifies the generic drivers into a single,
> loadable module.
>
> Enjoy,
> Henrik
>
> Henrik Rydberg (6):
>  hid: Add device group to modalias
>  hid: Scan the device for group info before adding it
>  hid: Allow bus wildcard matching
>  hid: Create a generic device group
>  hid-multitouch: Switch to device groups
>  hid: Create a common generic driver
>
>  drivers/hid/Kconfig             |   12 +++
>  drivers/hid/Makefile            |    2 +
>  drivers/hid/hid-core.c          |  172 ++++++++++++++++++++-------------------
>  drivers/hid/hid-generic.c       |   53 ++++++++++++
>  drivers/hid/hid-input.c         |   11 ---
>  drivers/hid/hid-multitouch.c    |    3 +-
>  drivers/hid/usbhid/hid-core.c   |   16 ----
>  include/linux/hid.h             |   20 +++--
>  include/linux/mod_devicetable.h |    4 +-
>  net/bluetooth/hidp/core.c       |   27 +-----
>  scripts/mod/file2alias.c        |    5 +-
>  11 files changed, 180 insertions(+), 145 deletions(-)
>  create mode 100644 drivers/hid/hid-generic.c
>
> --
> 1.7.10
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo [at] vger
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


jkosina at suse

Apr 30, 2012, 1:35 AM

Post #4 of 21 (218 views)
Permalink
Re: [PATCH v3 0/6] hid: Introduce device groups [In reply to]

On Mon, 23 Apr 2012, Henrik Rydberg wrote:

> Hi Jiri,
>
> Here is the third version of the extension to the device-driver
> matching mechanism. AFAICT, there are no outstanding issues. I think
> we are getting close. :-)

I think so, and overall I like the aproach and the implementation. Thanks!

Just got the stacktrace on [1] though when running kernel with this
patchset. The trace popped during shutdown, and the machine froze
completely; I didn't have any kind of external console connected, so
unfortunately I don't have the beginning of the whole thing.

I haven't been able to reproduce it so far. This was after several
"parallel" plug/remove cycles of multiple HID devices driven by multiple
different drivers.

I haven't performed any analysis what this might be yet.

[1] http://www.jikos.cz/jikos/junk/autoloading-trace.jpg


--
Jiri Kosina
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


jkosina at suse

Apr 30, 2012, 1:44 AM

Post #5 of 21 (220 views)
Permalink
Re: [PATCH v3 0/6] hid: Introduce device groups [In reply to]

On Mon, 30 Apr 2012, Jiri Kosina wrote:

> > Here is the third version of the extension to the device-driver
> > matching mechanism. AFAICT, there are no outstanding issues. I think
> > we are getting close. :-)
>
> I think so, and overall I like the aproach and the implementation. Thanks!
>
> Just got the stacktrace on [1] though when running kernel with this
> patchset. The trace popped during shutdown, and the machine froze
> completely; I didn't have any kind of external console connected, so
> unfortunately I don't have the beginning of the whole thing.
>
> I haven't been able to reproduce it so far. This was after several
> "parallel" plug/remove cycles of multiple HID devices driven by multiple
> different drivers.
>
> I haven't performed any analysis what this might be yet.
>
> [1] http://www.jikos.cz/jikos/junk/autoloading-trace.jpg

It actually seems to be spinlock lockup (due to the NMI trigger being
apparent at the very first line) on kbd_event_lock ...

--
Jiri Kosina
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


rydberg at euromail

Apr 30, 2012, 4:53 AM

Post #6 of 21 (218 views)
Permalink
Re: [PATCH v3 0/6] hid: Introduce device groups [In reply to]

Hi Jiri,

> > Just got the stacktrace on [1] though when running kernel with this
> > patchset. The trace popped during shutdown, and the machine froze
> > completely; I didn't have any kind of external console connected, so
> > unfortunately I don't have the beginning of the whole thing.
> >
> > I haven't been able to reproduce it so far. This was after several
> > "parallel" plug/remove cycles of multiple HID devices driven by multiple
> > different drivers.
> >
> > I haven't performed any analysis what this might be yet.
> >
> > [1] http://www.jikos.cz/jikos/junk/autoloading-trace.jpg
>
> It actually seems to be spinlock lockup (due to the NMI trigger being
> apparent at the very first line) on kbd_event_lock ...

Ah, yes. I take it you are talking about tty/vt/keyboard.c. So some
random keypress during shutdown triggers the event, which eventually
reaches input_pass_event(). From there on, the trace stays in the
mentioned driver. First kbd_event() gets called, which takes the lock
and goes on to, in turn, call kbd_keycode(), k_handler[2]() ==
k_spec(), fn_handler[9]() == fn_hold(), which goes on to call
stop_tty(). This function comes back to the driver, via con_stop(), as
vt_kbd_con_stop(), which in turn takes the same lock. So unless the
teardown of something in hid affects the choices made in the tty
driver, it appears this is a different problem. Or?

Henrik
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


jkosina at suse

Apr 30, 2012, 4:55 AM

Post #7 of 21 (216 views)
Permalink
Re: [PATCH v3 0/6] hid: Introduce device groups [In reply to]

On Mon, 30 Apr 2012, Henrik Rydberg wrote:

> > > Just got the stacktrace on [1] though when running kernel with this
> > > patchset. The trace popped during shutdown, and the machine froze
> > > completely; I didn't have any kind of external console connected, so
> > > unfortunately I don't have the beginning of the whole thing.
> > >
> > > I haven't been able to reproduce it so far. This was after several
> > > "parallel" plug/remove cycles of multiple HID devices driven by multiple
> > > different drivers.
> > >
> > > I haven't performed any analysis what this might be yet.
> > >
> > > [1] http://www.jikos.cz/jikos/junk/autoloading-trace.jpg
> >
> > It actually seems to be spinlock lockup (due to the NMI trigger being
> > apparent at the very first line) on kbd_event_lock ...
>
> Ah, yes. I take it you are talking about tty/vt/keyboard.c. So some
> random keypress during shutdown triggers the event, which eventually
> reaches input_pass_event(). From there on, the trace stays in the
> mentioned driver. First kbd_event() gets called, which takes the lock
> and goes on to, in turn, call kbd_keycode(), k_handler[2]() ==
> k_spec(), fn_handler[9]() == fn_hold(), which goes on to call
> stop_tty(). This function comes back to the driver, via con_stop(), as
> vt_kbd_con_stop(), which in turn takes the same lock. So unless the
> teardown of something in hid affects the choices made in the tty
> driver, it appears this is a different problem. Or?

I just came to the same conclusion a few minutes ago ... i.e. this is
likely unrelated to the patchset and I just triggered it by pure
coincidence on the patched kernel.

I will keep looking into it a little bit more. Dmitry, any immediate ideas
by any chance?

Otherwise the series seems indeed fine and if I don't come across anything
substantial once I am done with the review, I am considering pushing it
for -next (I still have to look at quirks propagation Nikolai pointed
out).

Thanks,

--
Jiri Kosina
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


jkosina at suse

May 1, 2012, 4:03 AM

Post #8 of 21 (209 views)
Permalink
Re: [PATCH v3 0/6] hid: Introduce device groups [In reply to]

On Mon, 23 Apr 2012, Henrik Rydberg wrote:

> Hi Jiri,
>
> Here is the third version of the extension to the device-driver
> matching mechanism. AFAICT, there are no outstanding issues. I think
> we are getting close. :-)

I have now updated the 'might-rebase/autoloading' branch of git.hid tree
(this is a rebasing branch, so simple pull will not work for you as a
fast-forward, you'll have to delete and repull the branch).

It's currently in a shape I would be okay to pushing out for -next and
scheduling for next merge window, and I will do so tomorrow if nothing
else pops up during testing or reviews.

Thanks everybody, especially Henrik of course, for excellent work on this.

--
Jiri Kosina
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


jkosina at suse

May 1, 2012, 5:23 AM

Post #9 of 21 (207 views)
Permalink
Re: [PATCH v3 0/6] hid: Introduce device groups [In reply to]

On Mon, 30 Apr 2012, Jiri Kosina wrote:

> > Ah, yes. I take it you are talking about tty/vt/keyboard.c. So some
> > random keypress during shutdown triggers the event, which eventually
> > reaches input_pass_event(). From there on, the trace stays in the
> > mentioned driver. First kbd_event() gets called, which takes the lock
> > and goes on to, in turn, call kbd_keycode(), k_handler[2]() ==
> > k_spec(), fn_handler[9]() == fn_hold(), which goes on to call
> > stop_tty(). This function comes back to the driver, via con_stop(), as
> > vt_kbd_con_stop(), which in turn takes the same lock. So unless the
> > teardown of something in hid affects the choices made in the tty
> > driver, it appears this is a different problem. Or?
>
> I just came to the same conclusion a few minutes ago ... i.e. this is
> likely unrelated to the patchset and I just triggered it by pure
> coincidence on the patched kernel.
>
> I will keep looking into it a little bit more. Dmitry, any immediate ideas
> by any chance?

Okay, someone was able to get lockdep complain about this and Alan is
apparently on it already:

https://lkml.org/lkml/2012/5/1/69

So nothing to be worried about.

--
Jiri Kosina
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


rydberg at euromail

May 1, 2012, 10:46 PM

Post #10 of 21 (203 views)
Permalink
Re: [PATCH v3 0/6] hid: Introduce device groups [In reply to]

Hi Jiri,

> I have now updated the 'might-rebase/autoloading' branch of git.hid tree
> (this is a rebasing branch, so simple pull will not work for you as a
> fast-forward, you'll have to delete and repull the branch).
>
> It's currently in a shape I would be okay to pushing out for -next and
> scheduling for next merge window, and I will do so tomorrow if nothing
> else pops up during testing or reviews.

The branch looks good in this end.

Thanks,
Henrik
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


benjamin.tissoires at gmail

May 2, 2012, 1:33 AM

Post #11 of 21 (205 views)
Permalink
Re: [PATCH v3 0/6] hid: Introduce device groups [In reply to]

On Tue, May 1, 2012 at 1:03 PM, Jiri Kosina <jkosina [at] suse> wrote:
> On Mon, 23 Apr 2012, Henrik Rydberg wrote:
>
>> Hi Jiri,
>>
>> Here is the third version of the extension to the device-driver
>> matching mechanism. AFAICT, there are no outstanding issues. I think
>> we are getting close. :-)
>
> I have now updated the 'might-rebase/autoloading' branch of git.hid tree
> (this is a rebasing branch, so simple pull will not work for you as a
> fast-forward, you'll have to delete and repull the branch).
>
> It's currently in a shape I would be okay to pushing out for -next and
> scheduling for next merge window, and I will do so tomorrow if nothing
> else pops up during testing or reviews.

Hi Jiri,

Well, I'm back in my lab this morning, and I'm testing your branch
against the different devices we have here. I'm observing a strange
problem: when connecting some devices, the first touch is always at
0,0. If I rmmod and insmod hid_multitouch it seems to work after....

I'm investigating this bug now. I'll tell you when I'm done. I still
don't now if it's related to Henrik's changes or 3.4-rc4.

Cheers,
Benjamin


>
> Thanks everybody, especially Henrik of course, for excellent work on this.
>
> --
> Jiri Kosina
> SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


jkosina at suse

May 2, 2012, 1:45 AM

Post #12 of 21 (202 views)
Permalink
Re: [PATCH v3 0/6] hid: Introduce device groups [In reply to]

On Wed, 2 May 2012, Benjamin Tissoires wrote:

> > It's currently in a shape I would be okay to pushing out for -next and
> > scheduling for next merge window, and I will do so tomorrow if nothing
> > else pops up during testing or reviews.
>
> Hi Jiri,
>
> Well, I'm back in my lab this morning, and I'm testing your branch
> against the different devices we have here. I'm observing a strange
> problem: when connecting some devices, the first touch is always at
> 0,0. If I rmmod and insmod hid_multitouch it seems to work after....
>
> I'm investigating this bug now. I'll tell you when I'm done. I still
> don't now if it's related to Henrik's changes or 3.4-rc4.

I see, thanks a lot for the heads-up, I will be waiting for your analysis
of this.

I have already renamed the branch to 'device-groups' and merged it into
branch that goes into next, so please send me any followup patches on top
of that queue now.

Thanks,

--
Jiri Kosina
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


benjamin.tissoires at gmail

May 2, 2012, 3:55 AM

Post #13 of 21 (202 views)
Permalink
Re: [PATCH v3 0/6] hid: Introduce device groups [In reply to]

On Wed, May 2, 2012 at 10:45 AM, Jiri Kosina <jkosina [at] suse> wrote:
> On Wed, 2 May 2012, Benjamin Tissoires wrote:
>
>> > It's currently in a shape I would be okay to pushing out for -next and
>> > scheduling for next merge window, and I will do so tomorrow if nothing
>> > else pops up during testing or reviews.
>>
>> Hi Jiri,
>>
>> Well, I'm back in my lab this morning, and I'm testing your branch
>> against the different devices we have here. I'm observing a strange
>> problem: when connecting some devices, the first touch is always at
>> 0,0. If I rmmod and insmod hid_multitouch it seems to work after....
>>
>> I'm investigating this bug now. I'll tell you when I'm done. I still
>> don't now if it's related to Henrik's changes or 3.4-rc4.
>
> I see, thanks a lot for the heads-up, I will be waiting for your analysis
> of this.
>
> I have already renamed the branch to 'device-groups' and merged it into
> branch that goes into next, so please send me any followup patches on top
> of that queue now.

Ok, I think I understood: nothing related to Henrik's changes. The bug
is also present in the upstream kernel.
In the function set_last_slot_field, I was doing a:
test_bit(usage->hid, hi->input->absbit);

for instance, 0x10030 (X hid field) in the absbit bit field which
contains only ABS_CNT (0x40) bits.... That's why it was a random bug.
Now I understood why Ubuntu complained about this test ;-)

I won't have the time to submit the patch right now, I have to give a
lecture this afternoon. Maybe this evening or tomorrow.

Cheers,
Benjamin

>
> Thanks,
>
> --
> Jiri Kosina
> SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


benjamin.tissoires at gmail

May 3, 2012, 3:06 AM

Post #14 of 21 (194 views)
Permalink
Re: [PATCH v3 0/6] hid: Introduce device groups [In reply to]

On Wed, May 2, 2012 at 12:55 PM, Benjamin Tissoires
<benjamin.tissoires [at] gmail> wrote:
> On Wed, May 2, 2012 at 10:45 AM, Jiri Kosina <jkosina [at] suse> wrote:
>> On Wed, 2 May 2012, Benjamin Tissoires wrote:
>>
>>> > It's currently in a shape I would be okay to pushing out for -next and
>>> > scheduling for next merge window, and I will do so tomorrow if nothing
>>> > else pops up during testing or reviews.
>>>
>>> Hi Jiri,
>>>
>>> Well, I'm back in my lab this morning, and I'm testing your branch
>>> against the different devices we have here. I'm observing a strange
>>> problem: when connecting some devices, the first touch is always at
>>> 0,0. If I rmmod and insmod hid_multitouch it seems to work after....
>>>
>>> I'm investigating this bug now. I'll tell you when I'm done. I still
>>> don't now if it's related to Henrik's changes or 3.4-rc4.
>>
>> I see, thanks a lot for the heads-up, I will be waiting for your analysis
>> of this.
>>
>> I have already renamed the branch to 'device-groups' and merged it into
>> branch that goes into next, so please send me any followup patches on top
>> of that queue now.
>
> Ok, I think I understood: nothing related to Henrik's changes. The bug
> is also present in the upstream kernel.
> In the function set_last_slot_field, I was doing a:
> test_bit(usage->hid, hi->input->absbit);
>
> for instance, 0x10030 (X hid field) in the absbit bit field which
> contains only ABS_CNT (0x40) bits.... That's why it was a random bug.
> Now I understood why Ubuntu complained about this test ;-)
>
> I won't have the time to submit the patch right now, I have to give a
> lecture this afternoon. Maybe this evening or tomorrow.
>
> Cheers,
> Benjamin

Hi guys,

I'm currently on the bug fix I told you earlier. However, I found a
more problematic bug in the hid_groups functionality.

Some device, like the Perixx peripad, present several interfaces
(mouse, keyboard and multitouch).
The hid groups functionality detects the HID field Contact ID, and
then forwards all interfaces to hid-multitouch. The point is that
hid-multitouch does not know how to handle mice and keyboards, and
then fails handling the interfaces of the device.

This particular device is then fully broken (as anyone pumped the events).
I also noticed the same problem (but less problematic) with cypress
panels: it presents different vendor interfaces and they are handled
by hid-multitouch.

Sorry for spotting this that late,
Benjamin


>
>>
>> Thanks,
>>
>> --
>> Jiri Kosina
>> SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


rydberg at euromail

May 3, 2012, 5:23 AM

Post #15 of 21 (195 views)
Permalink
Re: [PATCH v3 0/6] hid: Introduce device groups [In reply to]

> I'm currently on the bug fix I told you earlier. However, I found a
> more problematic bug in the hid_groups functionality.
>
> Some device, like the Perixx peripad, present several interfaces
> (mouse, keyboard and multitouch).
> The hid groups functionality detects the HID field Contact ID, and
> then forwards all interfaces to hid-multitouch. The point is that
> hid-multitouch does not know how to handle mice and keyboards, and
> then fails handling the interfaces of the device.

I am a bit unclear as to which devices this applies to, but I see two
possible solutions:

1) Add the devices in question back to the have_special_drivers list.

2) Add the interface type to the group descision, which should
probably be done anyway. I have a patch in the pipe that, will send it
later today.

> This particular device is then fully broken (as anyone pumped the events).
> I also noticed the same problem (but less problematic) with cypress
> panels: it presents different vendor interfaces and they are handled
> by hid-multitouch.

It would be great if you could test soution 1) before on a device -
something seems wrong if those interfaces were handled by hid-generic
before, but before getting the logic straight, it does not hurt to
try. :-)

Thanks,
Henrik
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


benjamin.tissoires at gmail

May 3, 2012, 5:45 AM

Post #16 of 21 (195 views)
Permalink
Re: [PATCH v3 0/6] hid: Introduce device groups [In reply to]

On Thu, May 3, 2012 at 2:23 PM, Henrik Rydberg <rydberg [at] euromail> wrote:
>> I'm currently on the bug fix I told you earlier. However, I found a
>> more problematic bug in the hid_groups functionality.
>>
>> Some device, like the Perixx peripad, present several interfaces
>> (mouse, keyboard and multitouch).
>> The hid groups functionality detects the HID field Contact ID, and
>> then forwards all interfaces to hid-multitouch. The point is that
>> hid-multitouch does not know how to handle mice and keyboards, and
>> then fails handling the interfaces of the device.
>
> I am a bit unclear as to which devices this applies to, but I see two
> possible solutions:
>
> 1) Add the devices in question back to the have_special_drivers list.

Well... The device presents valid mouse and keyboard interface that
should be handled by hid-generic.
The behavior of this particular device is the following:
- when 1 finger is in use, then it sends events over the mouse interface
- when 2 fingers are present, it sends events over the multitouch interface
- when you physically trigger the switch mode button, a keyboard
appears and it sends key events over the keyboard interface, and
eventually mouse events if you press the "mouse" key.... ;-)

This crap is all inherited by the fact that Microsoft do not want to
handle indirect touch, and the device maker found this solution to
counter this.

To sum up, adding it to the have_special_drivers driver list won't
work as we need part of the device to be handled by hid-generic.

>
> 2) Add the interface type to the group descision, which should
> probably be done anyway. I have a patch in the pipe that, will send it
> later today.

A simpler solution consists in adding the macros HID_USB_MT_DEVICE(v,
p) and HID_BLUETOOTH_MT_DEVICE(v, p) as you had introduced in a
earlier patch (I don't know why it disappeared).

The problem came out because:
- hid-multitouch registered the triplet BUS_USB / VID / PID.
- For each interface, it asks udev (or the kernel) which driver to
use, and whatever .group was, it was always hid-multitouch that came
out.

So it's just safer to specify the group for all multitouch devices.

Cheers,
Benjamin

>
>> This particular device is then fully broken (as anyone pumped the events).
>> I also noticed the same problem (but less problematic) with cypress
>> panels: it presents different vendor interfaces and they are handled
>> by hid-multitouch.
>
> It would be great if you could test soution 1) before on a device -
> something seems wrong if those interfaces were handled by hid-generic
> before, but before getting the logic straight, it does not hurt to
> try. :-)
>
> Thanks,
> Henrik
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


rydberg at euromail

May 3, 2012, 6:19 AM

Post #17 of 21 (197 views)
Permalink
Re: [PATCH v3 0/6] hid: Introduce device groups [In reply to]

> > 1) Add the devices in question back to the have_special_drivers list.
>
> Well... The device presents valid mouse and keyboard interface that
> should be handled by hid-generic.
> The behavior of this particular device is the following:
> - when 1 finger is in use, then it sends events over the mouse interface
> - when 2 fingers are present, it sends events over the multitouch interface
> - when you physically trigger the switch mode button, a keyboard
> appears and it sends key events over the keyboard interface, and
> eventually mouse events if you press the "mouse" key.... ;-)
>
> This crap is all inherited by the fact that Microsoft do not want to
> handle indirect touch, and the device maker found this solution to
> counter this.
>
> To sum up, adding it to the have_special_drivers driver list won't
> work as we need part of the device to be handled by hid-generic.

So was this particular device never listed in have_special_drivers?

> > 2) Add the interface type to the group descision, which should
> > probably be done anyway. I have a patch in the pipe that, will send it
> > later today.
>
> A simpler solution consists in adding the macros HID_USB_MT_DEVICE(v,
> p) and HID_BLUETOOTH_MT_DEVICE(v, p) as you had introduced in a
> earlier patch (I don't know why it disappeared).

No, the specific entries in the hid-multitouch device list matches any
group, so those defines were simplified away in the second version.

> The problem came out because:
> - hid-multitouch registered the triplet BUS_USB / VID / PID.
> - For each interface, it asks udev (or the kernel) which driver to
> use, and whatever .group was, it was always hid-multitouch that came
> out.
>
> So it's just safer to specify the group for all multitouch devices.

This is still confusing. I thought the real problem was that the
non-mt interfaces do not match hid-generic. Solution 2) should take
care of that. What I don't understand is how those other interfaces
came to be handled by hid-generic before this patch, unless this
device was never listed in have_special_driver.

Are we talking about USB_DEVICE_ID_TOPSEED2_PERIPAD_701 here?

Henrik
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


benjamin.tissoires at gmail

May 3, 2012, 6:37 AM

Post #18 of 21 (197 views)
Permalink
Re: [PATCH v3 0/6] hid: Introduce device groups [In reply to]

On Thu, May 3, 2012 at 3:19 PM, Henrik Rydberg <rydberg [at] euromail> wrote:
>> > 1) Add the devices in question back to the have_special_drivers list.
>>
>> Well... The device presents valid mouse and keyboard interface that
>> should be handled by hid-generic.
>> The behavior of this particular device is the following:
>> - when 1 finger is in use, then it sends events over the mouse interface
>> - when 2 fingers are present, it sends events over the multitouch interface
>> - when you physically trigger the switch mode button, a keyboard
>> appears and it sends key events over the keyboard interface, and
>> eventually mouse events if you press the "mouse" key.... ;-)
>>
>> This crap is all inherited by the fact that Microsoft do not want to
>> handle indirect touch, and the device maker found this solution to
>> counter this.
>>
>> To sum up, adding it to the have_special_drivers driver list won't
>> work as we need part of the device to be handled by hid-generic.
>
> So was this particular device never listed in have_special_drivers?

No, and that's the way it should (not being part of have_special_driver).

>
>> > 2) Add the interface type to the group descision, which should
>> > probably be done anyway. I have a patch in the pipe that, will send it
>> > later today.
>>
>> A simpler solution consists in adding the macros HID_USB_MT_DEVICE(v,
>> p) and HID_BLUETOOTH_MT_DEVICE(v, p) as you had introduced in a
>> earlier patch (I don't know why it disappeared).
>
> No, the specific entries in the hid-multitouch device list matches any
> group, so those defines were simplified away in the second version.

disagree: a device can present several interface (because it has
several "devices") and only those presenting Contact ID can and should
be handled by hid-multitouch.

Cypress for instance presents one interface for the multitouch layer,
and one other for specific controls that are seen as a keyboard.
However, in this particular case, I'm not sure we want to show this
interface to the end user.... ;-)

>
>> The problem came out because:
>> - hid-multitouch registered the triplet BUS_USB / VID / PID.
>> - For each interface, it asks udev (or the kernel) which driver to
>> use, and whatever .group was, it was always hid-multitouch that came
>> out.
>>
>> So it's just safer to specify the group for all multitouch devices.
>
> This is still confusing. I thought the real problem was that the
> non-mt interfaces do not match hid-generic. Solution 2) should take
> care of that. What I don't understand is how those other interfaces
> came to be handled by hid-generic before this patch, unless this
> device was never listed in have_special_driver.

The think is that they do match hid-generic (they get the group
HID_GROUP_GENERIC).
However they also match hid-multitouch (as hid-multitouch does not ask
for a particular group). So, if hid-multitouch is loaded __before__
hid-generic, it will be given the device whatever the match with
hid-generic.

And again, yes it was never listed in have_special_driver.

>
> Are we talking about USB_DEVICE_ID_TOPSEED2_PERIPAD_701 here?

yep


- For consistency, I'd rather specifying the group for any devices.
This because hid-multitouch can not handle other interfaces than
multitouch one. Though the catchall is interesting in the sense that
it may help us to hide unwanted interfaces.

- For backward compatibility, we should adapt each device (currently,
I only spotted this particular one) to decide if we need to catch the
group or not.

Jiri, any thought?

Benjamin

>
> Henrik
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


rydberg at euromail

May 3, 2012, 6:54 AM

Post #19 of 21 (195 views)
Permalink
Re: [PATCH v3 0/6] hid: Introduce device groups [In reply to]

> >> > 2) Add the interface type to the group descision, which should
> >> > probably be done anyway. I have a patch in the pipe that, will send it
> >> > later today.
> >>
> >> A simpler solution consists in adding the macros HID_USB_MT_DEVICE(v,
> >> p) and HID_BLUETOOTH_MT_DEVICE(v, p) as you had introduced in a
> >> earlier patch (I don't know why it disappeared).
> >
> > No, the specific entries in the hid-multitouch device list matches any
> > group, so those defines were simplified away in the second version.
>
> disagree: a device can present several interface (because it has
> several "devices") and only those presenting Contact ID can and should
> be handled by hid-multitouch.

Obviously this is only a problem for the devices with mixed
interfaces, but for those, solution 2) together with specifying the
group as you suggest should work. We can definitely change all devices
in the list, it just was not necessary before (or so I thought).

> The think is that they do match hid-generic (they get the group
> HID_GROUP_GENERIC).
> However they also match hid-multitouch (as hid-multitouch does not ask
> for a particular group). So, if hid-multitouch is loaded __before__
> hid-generic, it will be given the device whatever the match with
> hid-generic.

I suppose what you describe here is how it was working before the
device groups. Ok, I see what to do now. I will be back shortly with a
patch which should make USB_DEVICE_ID_TOPSEED2_PERIPAD_701 work, and
let's take it from there.

Thanks for testing,
Henrik
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


benjamin.tissoires at gmail

May 3, 2012, 7:04 AM

Post #20 of 21 (195 views)
Permalink
Re: [PATCH v3 0/6] hid: Introduce device groups [In reply to]

On Thu, May 3, 2012 at 3:54 PM, Henrik Rydberg <rydberg [at] euromail> wrote:
>> >> > 2) Add the interface type to the group descision, which should
>> >> > probably be done anyway. I have a patch in the pipe that, will send it
>> >> > later today.
>> >>
>> >> A simpler solution consists in adding the macros HID_USB_MT_DEVICE(v,
>> >> p) and HID_BLUETOOTH_MT_DEVICE(v, p) as you had introduced in a
>> >> earlier patch (I don't know why it disappeared).
>> >
>> > No, the specific entries in the hid-multitouch device list matches any
>> > group, so those defines were simplified away in the second version.
>>
>> disagree: a device can present several interface (because it has
>> several "devices") and only those presenting Contact ID can and should
>> be handled by hid-multitouch.
>
> Obviously this is only a problem for the devices with mixed
> interfaces, but for those, solution 2) together with specifying the
> group as you suggest should work. We can definitely change all devices
> in the list, it just was not necessary before (or so I thought).
>
>> The think is that they do match hid-generic (they get the group
>> HID_GROUP_GENERIC).
>> However they also match hid-multitouch (as hid-multitouch does not ask
>> for a particular group). So, if hid-multitouch is loaded __before__
>> hid-generic, it will be given the device whatever the match with
>> hid-generic.
>
> I suppose what you describe here is how it was working before the
> device groups.

Just to be clear: no, this last paragraph occurs after device groups.
Before, it was working as hid-generic catch the device first, then
released it while looking at the report descriptors. Now, it's purely
dependent on which driver is loaded first for this particaular case.

Benjamin

> Ok, I see what to do now. I will be back shortly with a
> patch which should make USB_DEVICE_ID_TOPSEED2_PERIPAD_701 work, and
> let's take it from there.
>
> Thanks for testing,
> Henrik
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


rydberg at euromail

May 3, 2012, 7:16 AM

Post #21 of 21 (197 views)
Permalink
Re: [PATCH v3 0/6] hid: Introduce device groups [In reply to]

> > I suppose what you describe here is how it was working before the
> > device groups.
>
> Just to be clear: no, this last paragraph occurs after device groups.
> Before, it was working as hid-generic catch the device first, then
> released it while looking at the report descriptors. Now, it's purely
> dependent on which driver is loaded first for this particaular case.

Thanks for the clarification, and I agree. It also means you are
completely right, it will suffice to change those devices from the
wildcard group to the multitouch group. I don't mind switching them
all.

Thanks,
Henrik
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Linux kernel RSS feed   Index | Next | Previous | View Threaded
 
 


Interested in having your list archived? Contact Gossamer Threads
 
  Web Applications & Managed Hosting Powered by Gossamer Threads Inc.