| Message ID | 20120903173349.GA18666@alittletooquiet.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Sep 03, 2012 at 01:33:50PM -0400, Forest Bond wrote: > From: Forest Bond <forest.bond@rapidrollout.com> > > Certain eGalax devices expose an interface with class HID and protocol > None. Some work with usbhid and some work with usbtouchscreen, but > there is no easy way to differentiate. Sending an eGalax diagnostic > packet seems to kick them all into using the right protocol for > usbtouchscreen, so we can continue to bind them all there (as opposed to > handing some off to usbhid). > > This fixes a regression for devices that were claimed by (and worked > with) usbhid prior to commit 139ebe8dc80dd74cb2ac9f5603d18fbf5cff049f > ("Input: usbtouchscreen - fix eGalax HID ignoring"), which made > usbtouchscreen claim them instead. With this patch they will still be > claimed by usbtouchscreen, but they will actually report events > usbtouchscreen can understand. Note that these devices will be limited > to the usbtouchscreen feature set so e.g. dual touch features are not > supported. > > I have the distinct pleasure of needing to support devices of both types > and have tested accordingly. > > Signed-off-by: Forest Bond <forest.bond@rapidrollout.com> Applied, thank you Forest. > --- > drivers/input/touchscreen/usbtouchscreen.c | 39 ++++++++++++++++++++++++++++ > 1 files changed, 39 insertions(+), 0 deletions(-) > > diff --git a/drivers/input/touchscreen/usbtouchscreen.c b/drivers/input/touchscreen/usbtouchscreen.c > index e32709e..c5f4dc0 100644 > --- a/drivers/input/touchscreen/usbtouchscreen.c > +++ b/drivers/input/touchscreen/usbtouchscreen.c > @@ -304,6 +304,44 @@ static int e2i_read_data(struct usbtouch_usb *dev, unsigned char *pkt) > #define EGALAX_PKT_TYPE_REPT 0x80 > #define EGALAX_PKT_TYPE_DIAG 0x0A > > +static int egalax_init(struct usbtouch_usb *usbtouch) > +{ > + int ret, i; > + unsigned char *buf; > + struct usb_device *udev = interface_to_usbdev(usbtouch->interface); > + > + /* An eGalax diagnostic packet kicks the device into using the right > + * protocol. We send a "check active" packet. The response will be > + * read later and ignored. > + */ > + > + buf = kmalloc(3, GFP_KERNEL); > + if (!buf) > + return -ENOMEM; > + > + buf[0] = EGALAX_PKT_TYPE_DIAG; > + buf[1] = 1; /* length */ > + buf[2] = 'A'; /* command - check active */ > + > + for (i = 0; i < 3; i++) { > + ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), > + 0, > + USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE, > + 0, 0, buf, 3, > + USB_CTRL_SET_TIMEOUT); > + if (ret >= 0) { > + ret = 0; > + break; > + } > + if (ret != -EPIPE) > + break; > + } > + > + kfree(buf); > + > + return ret; > +} > + > static int egalax_read_data(struct usbtouch_usb *dev, unsigned char *pkt) > { > if ((pkt[0] & EGALAX_PKT_TYPE_MASK) != EGALAX_PKT_TYPE_REPT) > @@ -1056,6 +1094,7 @@ static struct usbtouch_device_info usbtouch_dev_info[] = { > .process_pkt = usbtouch_process_multi, > .get_pkt_len = egalax_get_pkt_len, > .read_data = egalax_read_data, > + .init = egalax_init, > }, > #endif > > -- > 1.7.0.4
Hi Dmitry, On Tue, Sep 04, 2012 at 11:07:04PM -0700, Dmitry Torokhov wrote: > On Mon, Sep 03, 2012 at 01:33:50PM -0400, Forest Bond wrote: > > From: Forest Bond <forest.bond@rapidrollout.com> > > > > Certain eGalax devices expose an interface with class HID and protocol > > None. Some work with usbhid and some work with usbtouchscreen, but > > there is no easy way to differentiate. Sending an eGalax diagnostic > > packet seems to kick them all into using the right protocol for > > usbtouchscreen, so we can continue to bind them all there (as opposed to > > handing some off to usbhid). > > > > This fixes a regression for devices that were claimed by (and worked > > with) usbhid prior to commit 139ebe8dc80dd74cb2ac9f5603d18fbf5cff049f > > ("Input: usbtouchscreen - fix eGalax HID ignoring"), which made > > usbtouchscreen claim them instead. With this patch they will still be > > claimed by usbtouchscreen, but they will actually report events > > usbtouchscreen can understand. Note that these devices will be limited > > to the usbtouchscreen feature set so e.g. dual touch features are not > > supported. > > > > I have the distinct pleasure of needing to support devices of both types > > and have tested accordingly. > > > > Signed-off-by: Forest Bond <forest.bond@rapidrollout.com> > > Applied, thank you Forest. Great, thanks a lot. The other piece to this puzzle is that usbhid should blacklist these devices to avoid binding if it happens to be loaded before usbtouchscreen. To do this, usbhid needs to be able to blacklist devices based on interface protocol (right now it only supports blacklist on VID + PID). Would you accept a patch set that implements this? Thanks, Forest
On Friday, September 07, 2012 04:42:32 PM Forest Bond wrote: > Hi Dmitry, > > On Tue, Sep 04, 2012 at 11:07:04PM -0700, Dmitry Torokhov wrote: > > On Mon, Sep 03, 2012 at 01:33:50PM -0400, Forest Bond wrote: > > > From: Forest Bond <forest.bond@rapidrollout.com> > > > > > > Certain eGalax devices expose an interface with class HID and protocol > > > None. Some work with usbhid and some work with usbtouchscreen, but > > > there is no easy way to differentiate. Sending an eGalax diagnostic > > > packet seems to kick them all into using the right protocol for > > > usbtouchscreen, so we can continue to bind them all there (as opposed to > > > handing some off to usbhid). > > > > > > This fixes a regression for devices that were claimed by (and worked > > > with) usbhid prior to commit 139ebe8dc80dd74cb2ac9f5603d18fbf5cff049f > > > ("Input: usbtouchscreen - fix eGalax HID ignoring"), which made > > > usbtouchscreen claim them instead. With this patch they will still be > > > claimed by usbtouchscreen, but they will actually report events > > > usbtouchscreen can understand. Note that these devices will be limited > > > to the usbtouchscreen feature set so e.g. dual touch features are not > > > supported. > > > > > > I have the distinct pleasure of needing to support devices of both types > > > and have tested accordingly. > > > > > > Signed-off-by: Forest Bond <forest.bond@rapidrollout.com> > > > > Applied, thank you Forest. > > Great, thanks a lot. > > The other piece to this puzzle is that usbhid should blacklist these devices > to avoid binding if it happens to be loaded before usbtouchscreen. To do > this, usbhid needs to be able to blacklist devices based on interface > protocol (right now it only supports blacklist on VID + PID). > > Would you accept a patch set that implements this? Juri, this question is really for you... Thanks!
On Mon, 10 Sep 2012, Dmitry Torokhov wrote: > > The other piece to this puzzle is that usbhid should blacklist these devices > > to avoid binding if it happens to be loaded before usbtouchscreen. To do > > this, usbhid needs to be able to blacklist devices based on interface > > protocol (right now it only supports blacklist on VID + PID). > > > > Would you accept a patch set that implements this? > > Juri, this question is really for you... Generally, I am not objecting to this idea in general. What is the particular use case here, so that it's needed? Thanks,
Hi Jiri, Thanks for your message. Pretty good timing, actually, as I was just about to pick this back up. ;) On Thu, Nov 01, 2012 at 11:38:19AM +0100, Jiri Kosina wrote: > On Mon, 10 Sep 2012, Dmitry Torokhov wrote: > > > > The other piece to this puzzle is that usbhid should blacklist these devices > > > to avoid binding if it happens to be loaded before usbtouchscreen. To do > > > this, usbhid needs to be able to blacklist devices based on interface > > > protocol (right now it only supports blacklist on VID + PID). > > > > > > Would you accept a patch set that implements this? > > > > Juri, this question is really for you... > > Generally, I am not objecting to this idea in general. > > What is the particular use case here, so that it's needed? We have some eGalax devices with class HID and protocol None that both usbhid and usbtouchscreen will bind to, but we only want them bound to usbtouchscreen. Some do in fact work with usbhid, but not all of them do. OTOH they all work with usbtouchscreen as of commit 037a833ed05a86d01ea27a2c32043b86c549be1b. We want to blacklist these devices in usbhid to avoid binding to it if it is loaded first. But usbhid should continue to handle eGalax devices with class HID and protocol other than None (e.g. Mouse). They all have the same vendor and product IDs, so we need to be able to blacklist on (VID, PID, protocol) instead of just (VID, PID). Does that make sense? Thanks, Forest
On Fri, 2 Nov 2012, Forest Bond wrote: > We have some eGalax devices with class HID and protocol None that both > usbhid and usbtouchscreen will bind to, but we only want them bound to > usbtouchscreen. Some do in fact work with usbhid, but not all of them > do. OTOH they all work with usbtouchscreen as of commit > 037a833ed05a86d01ea27a2c32043b86c549be1b. > > We want to blacklist these devices in usbhid to avoid binding to it if > it is loaded first. But usbhid should continue to handle eGalax devices > with class HID and protocol other than None (e.g. Mouse). They all have > the same vendor and product IDs, so we need to be able to blacklist on > (VID, PID, protocol) instead of just (VID, PID). I see, thanks for the explanation. As this is the first time we are having this problem, I'd just propose to put just another switch into hid_ignore() for USB_VENDOR_ID_DWAV, and looking at hdev->type there to see whether we should ignore the device or not. If it turns, over time, to be a more general problem for multiple devices, we'll just introduce the more general blacklist matching then. How does that sound?
Hi Jiri, On Mon, Nov 05, 2012 at 03:19:34PM +0100, Jiri Kosina wrote: > On Fri, 2 Nov 2012, Forest Bond wrote: > > > We have some eGalax devices with class HID and protocol None that both > > usbhid and usbtouchscreen will bind to, but we only want them bound to > > usbtouchscreen. Some do in fact work with usbhid, but not all of them > > do. OTOH they all work with usbtouchscreen as of commit > > 037a833ed05a86d01ea27a2c32043b86c549be1b. > > > > We want to blacklist these devices in usbhid to avoid binding to it if > > it is loaded first. But usbhid should continue to handle eGalax devices > > with class HID and protocol other than None (e.g. Mouse). They all have > > the same vendor and product IDs, so we need to be able to blacklist on > > (VID, PID, protocol) instead of just (VID, PID). > > I see, thanks for the explanation. > > As this is the first time we are having this problem, I'd just propose to > put just another switch into hid_ignore() for USB_VENDOR_ID_DWAV, and > looking at hdev->type there to see whether we should ignore the device or > not. > > If it turns, over time, to be a more general problem for multiple devices, > we'll just introduce the more general blacklist matching then. > > How does that sound? Makes sense to me. I didn't notice the checks in hid_ignore until now. I'll send a patch in shortly. Thanks, Forest
diff --git a/drivers/input/touchscreen/usbtouchscreen.c b/drivers/input/touchscreen/usbtouchscreen.c index e32709e..c5f4dc0 100644 --- a/drivers/input/touchscreen/usbtouchscreen.c +++ b/drivers/input/touchscreen/usbtouchscreen.c @@ -304,6 +304,44 @@ static int e2i_read_data(struct usbtouch_usb *dev, unsigned char *pkt) #define EGALAX_PKT_TYPE_REPT 0x80 #define EGALAX_PKT_TYPE_DIAG 0x0A +static int egalax_init(struct usbtouch_usb *usbtouch) +{ + int ret, i; + unsigned char *buf; + struct usb_device *udev = interface_to_usbdev(usbtouch->interface); + + /* An eGalax diagnostic packet kicks the device into using the right + * protocol. We send a "check active" packet. The response will be + * read later and ignored. + */ + + buf = kmalloc(3, GFP_KERNEL); + if (!buf) + return -ENOMEM; + + buf[0] = EGALAX_PKT_TYPE_DIAG; + buf[1] = 1; /* length */ + buf[2] = 'A'; /* command - check active */ + + for (i = 0; i < 3; i++) { + ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), + 0, + USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE, + 0, 0, buf, 3, + USB_CTRL_SET_TIMEOUT); + if (ret >= 0) { + ret = 0; + break; + } + if (ret != -EPIPE) + break; + } + + kfree(buf); + + return ret; +} + static int egalax_read_data(struct usbtouch_usb *dev, unsigned char *pkt) { if ((pkt[0] & EGALAX_PKT_TYPE_MASK) != EGALAX_PKT_TYPE_REPT) @@ -1056,6 +1094,7 @@ static struct usbtouch_device_info usbtouch_dev_info[] = { .process_pkt = usbtouch_process_multi, .get_pkt_len = egalax_get_pkt_len, .read_data = egalax_read_data, + .init = egalax_init, }, #endif