[PATCH] Input: usbtouchscreen - initialize eGalax devices

[PATCH] Input: usbtouchscreen - initialize eGalax devices

[Forest Bond]

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,
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>
---
 drivers/input/touchscreen/usbtouchscreen.c |   25 +++++++++++++++++++++++++
 1 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/drivers/input/touchscreen/usbtouchscreen.c b/drivers/input/touchscreen/usbtouchscreen.c
index e32709e..2ce5308 100644
--- a/drivers/input/touchscreen/usbtouchscreen.c
+++ b/drivers/input/touchscreen/usbtouchscreen.c
@@ -304,6 +304,30 @@ 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;
+	struct usb_device *udev = interface_to_usbdev(usbtouch->interface);
+
+	/* An eGalax diagnostic packet kicks the device into using the right
+	 * protocol. */
+	for (i = 0; i < 3; i++) {
+		/* Send a "check active" packet. The response will be read
+		 * later and ignored. */
+		ret = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
+				      0,
+				      USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
+				      0, 0, "\x0A\x01A", 0,
+				      USB_CTRL_SET_TIMEOUT);
+		if (ret >= 0)
+			break;
+		if (ret != -EPIPE)
+			return ret;
+	}
+
+	return 0;
+}
+
 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 +1080,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

Re: [PATCH] Input: usbtouchscreen - initialize eGalax devices

[Dmitry Torokhov]

On Fri, Aug 31, 2012 at 09:56:58AM -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,
> 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, thanks Forest.

> ---
>  drivers/input/touchscreen/usbtouchscreen.c |   25 +++++++++++++++++++++++++
>  1 files changed, 25 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/usbtouchscreen.c b/drivers/input/touchscreen/usbtouchscreen.c
> index e32709e..2ce5308 100644
> --- a/drivers/input/touchscreen/usbtouchscreen.c
> +++ b/drivers/input/touchscreen/usbtouchscreen.c
> @@ -304,6 +304,30 @@ 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;
> +	struct usb_device *udev = interface_to_usbdev(usbtouch->interface);
> +
> +	/* An eGalax diagnostic packet kicks the device into using the right
> +	 * protocol. */
> +	for (i = 0; i < 3; i++) {
> +		/* Send a "check active" packet. The response will be read
> +		 * later and ignored. */
> +		ret = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
> +				      0,
> +				      USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
> +				      0, 0, "\x0A\x01A", 0,
> +				      USB_CTRL_SET_TIMEOUT);
> +		if (ret >= 0)
> +			break;
> +		if (ret != -EPIPE)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
>  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 +1080,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

-- 
Dmitry

Re: [PATCH] Input: usbtouchscreen - initialize eGalax devices

[Sergei Shtylyov]

Hello.

On 08/31/2012 05:56 PM, 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,

   Please also specify that commit's summary ion parens.

> 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>
> ---
>  drivers/input/touchscreen/usbtouchscreen.c |   25 +++++++++++++++++++++++++
>  1 files changed, 25 insertions(+), 0 deletions(-)

> diff --git a/drivers/input/touchscreen/usbtouchscreen.c b/drivers/input/touchscreen/usbtouchscreen.c
> index e32709e..2ce5308 100644
> --- a/drivers/input/touchscreen/usbtouchscreen.c
> +++ b/drivers/input/touchscreen/usbtouchscreen.c
> @@ -304,6 +304,30 @@ 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;
> +	struct usb_device *udev = interface_to_usbdev(usbtouch->interface);
> +
> +	/* An eGalax diagnostic packet kicks the device into using the right
> +	 * protocol. */

   The preferred multi-line comment style is:

/*
 * bla
 * bla
 */

> +	for (i = 0; i < 3; i++) {
> +		/* Send a "check active" packet. The response will be read
> +		 * later and ignored. */
> +		ret = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
> +				      0,
> +				      USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
> +				      0, 0, "\x0A\x01A", 0,

   You probably can't send data from the .const section (as well as off the
stack) -- they can be DMA'ed and there'll be issues with cache consistency on
non-x86 arches. You should allocate the data with kmalloc().
   Although, on the second thought, maybe I'm wrong in this case... not really
sure about sending -- receiving (to the .data section) could certainly be harmful...

WBR, Sergei

Re: [PATCH] Input: usbtouchscreen - initialize eGalax devices

[Dmitry Torokhov]

On Fri, Aug 31, 2012 at 10:50:38PM +0400, Sergei Shtylyov wrote:
> Hello.
> 
> On 08/31/2012 05:56 PM, 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,
> 
>    Please also specify that commit's summary ion parens.
> 
> > 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>
> > ---
> >  drivers/input/touchscreen/usbtouchscreen.c |   25 +++++++++++++++++++++++++
> >  1 files changed, 25 insertions(+), 0 deletions(-)
> 
> > diff --git a/drivers/input/touchscreen/usbtouchscreen.c b/drivers/input/touchscreen/usbtouchscreen.c
> > index e32709e..2ce5308 100644
> > --- a/drivers/input/touchscreen/usbtouchscreen.c
> > +++ b/drivers/input/touchscreen/usbtouchscreen.c
> > @@ -304,6 +304,30 @@ 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;
> > +	struct usb_device *udev = interface_to_usbdev(usbtouch->interface);
> > +
> > +	/* An eGalax diagnostic packet kicks the device into using the right
> > +	 * protocol. */
> 
>    The preferred multi-line comment style is:
> 
> /*
>  * bla
>  * bla
>  */
> 
> > +	for (i = 0; i < 3; i++) {
> > +		/* Send a "check active" packet. The response will be read
> > +		 * later and ignored. */
> > +		ret = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
> > +				      0,
> > +				      USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
> > +				      0, 0, "\x0A\x01A", 0,
> 
>    You probably can't send data from the .const section (as well as off the
> stack) -- they can be DMA'ed and there'll be issues with cache consistency on
> non-x86 arches. You should allocate the data with kmalloc().
>    Although, on the second thought, maybe I'm wrong in this case... not really
> sure about sending -- receiving (to the .data section) could certainly be harmful...

Hmm, do we actually send anything here? The "size" passed to
usb_control_msg() is 0 so I don't think we use that data at all...

Thanks.

-- 
Dmitry

Re: [PATCH] Input: usbtouchscreen - initialize eGalax devices

[Alan Stern]

On Fri, 31 Aug 2012, Dmitry Torokhov wrote:

> > > +		/* Send a "check active" packet. The response will be read
> > > +		 * later and ignored. */
> > > +		ret = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
> > > +				      0,
> > > +				      USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
> > > +				      0, 0, "\x0A\x01A", 0,
> > 
> >    You probably can't send data from the .const section (as well as off the
> > stack) -- they can be DMA'ed and there'll be issues with cache consistency on
> > non-x86 arches. You should allocate the data with kmalloc().
> >    Although, on the second thought, maybe I'm wrong in this case... not really
> > sure about sending -- receiving (to the .data section) could certainly be harmful...
> 
> Hmm, do we actually send anything here? The "size" passed to
> usb_control_msg() is 0 so I don't think we use that data at all...

Good point.  Perhaps the 0 is a typo, in which case data does get sent
and the buffer must be kmalloc'ed.  If the 0 is correct then the buffer
should be NULL, not "\x0A\x01A" (and what's the purpose of the leading
'0' in the second byte?).

In addition, although the bRequestType specifies USB_DIR_OUT, the pipe
value is usb_rcvctrlpipe.  Is this transfer meant to be IN or OUT?

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Input: usbtouchscreen - initialize eGalax devices

[Forest Bond]

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

Hi,

On Fri, Aug 31, 2012 at 04:04:58PM -0400, Alan Stern wrote:
> On Fri, 31 Aug 2012, Dmitry Torokhov wrote:
> 
> > > > +		/* Send a "check active" packet. The response will be read
> > > > +		 * later and ignored. */
> > > > +		ret = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
> > > > +				      0,
> > > > +				      USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
> > > > +				      0, 0, "\x0A\x01A", 0,
> > > 
> > >    You probably can't send data from the .const section (as well as off the
> > > stack) -- they can be DMA'ed and there'll be issues with cache consistency on
> > > non-x86 arches. You should allocate the data with kmalloc().
> > >    Although, on the second thought, maybe I'm wrong in this case... not really
> > > sure about sending -- receiving (to the .data section) could certainly be harmful...
> > 
> > Hmm, do we actually send anything here? The "size" passed to
> > usb_control_msg() is 0 so I don't think we use that data at all...
> 
> Good point.  Perhaps the 0 is a typo, in which case data does get sent
> and the buffer must be kmalloc'ed.  If the 0 is correct then the buffer
> should be NULL, not "\x0A\x01A" (and what's the purpose of the leading
> '0' in the second byte?).
> 
> In addition, although the bRequestType specifies USB_DIR_OUT, the pipe
> value is usb_rcvctrlpipe.  Is this transfer meant to be IN or OUT?

The primary issue is that I'm a USB newb working on a deadline. ;)

My intention with this call was to send those three bytes out to the device.  I
tested this in Python using a libusb binding.  Oddly enough, the code as written
does in fact work in the sense that the device talks to the driver correctly
after initialization (and without it it doesn't).

Anyway, thanks for the review and sorry for being a bonehead.  I'll resend.

Thanks,
Forest
-- 
Forest Bond
http://www.alittletooquiet.net
http://www.rapidrollout.com

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

Re: [PATCH] Input: usbtouchscreen - initialize eGalax devices

[Forest Bond]

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

Hi,

On Fri, Aug 31, 2012 at 04:04:58PM -0400, Alan Stern wrote:
> On Fri, 31 Aug 2012, Dmitry Torokhov wrote:
> 
> > > > +		/* Send a "check active" packet. The response will be read
> > > > +		 * later and ignored. */
> > > > +		ret = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
> > > > +				      0,
> > > > +				      USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
> > > > +				      0, 0, "\x0A\x01A", 0,
> > > 
> > >    You probably can't send data from the .const section (as well as off the
> > > stack) -- they can be DMA'ed and there'll be issues with cache consistency on
> > > non-x86 arches. You should allocate the data with kmalloc().
> > >    Although, on the second thought, maybe I'm wrong in this case... not really
> > > sure about sending -- receiving (to the .data section) could certainly be harmful...
> > 
> > Hmm, do we actually send anything here? The "size" passed to
> > usb_control_msg() is 0 so I don't think we use that data at all...
> 
> Good point.  Perhaps the 0 is a typo, in which case data does get sent
> and the buffer must be kmalloc'ed.  If the 0 is correct then the buffer
> should be NULL, not "\x0A\x01A" (and what's the purpose of the leading
> '0' in the second byte?).
> 
> In addition, although the bRequestType specifies USB_DIR_OUT, the pipe
> value is usb_rcvctrlpipe.  Is this transfer meant to be IN or OUT?

Thanks again to all for the review.  My theory for why the previous patch worked
in spite of its wrongness is that the device actually switches modes when it
receives a control message with USB_TYPE_VENDOR even though the documentation
suggests an actual diagnostic packet must be received.

Does this (untested) patch look more reasonable?


diff --git a/drivers/input/touchscreen/usbtouchscreen.c b/drivers/input/touchscreen/usbtouchscreen.c
index e32709e..64b4b17 100644
--- a/drivers/input/touchscreen/usbtouchscreen.c
+++ b/drivers/input/touchscreen/usbtouchscreen.c
@@ -304,6 +304,41 @@ 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);
+	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 +1091,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
 

Thanks,
Forest
-- 
Forest Bond
http://www.alittletooquiet.net
http://www.rapidrollout.com

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

Re: [PATCH] Input: usbtouchscreen - initialize eGalax devices

[Dmitry Torokhov]

On Fri, Aug 31, 2012 at 06:53:53PM -0400, Forest Bond wrote:
> Hi,
> 
> On Fri, Aug 31, 2012 at 04:04:58PM -0400, Alan Stern wrote:
> > On Fri, 31 Aug 2012, Dmitry Torokhov wrote:
> > 
> > > > > +		/* Send a "check active" packet. The response will be read
> > > > > +		 * later and ignored. */
> > > > > +		ret = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
> > > > > +				      0,
> > > > > +				      USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
> > > > > +				      0, 0, "\x0A\x01A", 0,
> > > > 
> > > >    You probably can't send data from the .const section (as well as off the
> > > > stack) -- they can be DMA'ed and there'll be issues with cache consistency on
> > > > non-x86 arches. You should allocate the data with kmalloc().
> > > >    Although, on the second thought, maybe I'm wrong in this case... not really
> > > > sure about sending -- receiving (to the .data section) could certainly be harmful...
> > > 
> > > Hmm, do we actually send anything here? The "size" passed to
> > > usb_control_msg() is 0 so I don't think we use that data at all...
> > 
> > Good point.  Perhaps the 0 is a typo, in which case data does get sent
> > and the buffer must be kmalloc'ed.  If the 0 is correct then the buffer
> > should be NULL, not "\x0A\x01A" (and what's the purpose of the leading
> > '0' in the second byte?).
> > 
> > In addition, although the bRequestType specifies USB_DIR_OUT, the pipe
> > value is usb_rcvctrlpipe.  Is this transfer meant to be IN or OUT?
> 
> Thanks again to all for the review.  My theory for why the previous patch worked
> in spite of its wrongness is that the device actually switches modes when it
> receives a control message with USB_TYPE_VENDOR even though the documentation
> suggests an actual diagnostic packet must be received.
> 
> Does this (untested) patch look more reasonable?

Yes, but we still need it tested, please.

> 
> 
> diff --git a/drivers/input/touchscreen/usbtouchscreen.c b/drivers/input/touchscreen/usbtouchscreen.c
> index e32709e..64b4b17 100644
> --- a/drivers/input/touchscreen/usbtouchscreen.c
> +++ b/drivers/input/touchscreen/usbtouchscreen.c
> @@ -304,6 +304,41 @@ 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);
> +	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 +1091,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
>  
> 
> Thanks,
> Forest
> -- 
> Forest Bond
> http://www.alittletooquiet.net
> http://www.rapidrollout.com



-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Input: usbtouchscreen - initialize eGalax devices

[Forest Bond]

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

Hi Dmitry,

On Fri, Aug 31, 2012 at 04:10:47PM -0700, Dmitry Torokhov wrote:
> On Fri, Aug 31, 2012 at 06:53:53PM -0400, Forest Bond wrote:
> > Hi,
> > 
> > On Fri, Aug 31, 2012 at 04:04:58PM -0400, Alan Stern wrote:
> > > On Fri, 31 Aug 2012, Dmitry Torokhov wrote:
> > > 
> > > > > > +		/* Send a "check active" packet. The response will be read
> > > > > > +		 * later and ignored. */
> > > > > > +		ret = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
> > > > > > +				      0,
> > > > > > +				      USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
> > > > > > +				      0, 0, "\x0A\x01A", 0,
> > > > > 
> > > > >    You probably can't send data from the .const section (as well as off the
> > > > > stack) -- they can be DMA'ed and there'll be issues with cache consistency on
> > > > > non-x86 arches. You should allocate the data with kmalloc().
> > > > >    Although, on the second thought, maybe I'm wrong in this case... not really
> > > > > sure about sending -- receiving (to the .data section) could certainly be harmful...
> > > > 
> > > > Hmm, do we actually send anything here? The "size" passed to
> > > > usb_control_msg() is 0 so I don't think we use that data at all...
> > > 
> > > Good point.  Perhaps the 0 is a typo, in which case data does get sent
> > > and the buffer must be kmalloc'ed.  If the 0 is correct then the buffer
> > > should be NULL, not "\x0A\x01A" (and what's the purpose of the leading
> > > '0' in the second byte?).
> > > 
> > > In addition, although the bRequestType specifies USB_DIR_OUT, the pipe
> > > value is usb_rcvctrlpipe.  Is this transfer meant to be IN or OUT?
> > 
> > Thanks again to all for the review.  My theory for why the previous patch worked
> > in spite of its wrongness is that the device actually switches modes when it
> > receives a control message with USB_TYPE_VENDOR even though the documentation
> > suggests an actual diagnostic packet must be received.
> > 
> > Does this (untested) patch look more reasonable?
> 
> Yes, but we still need it tested, please.

Great, I'll test it later this evening when I am back in front of the hardware
and follow up with a properly submitted patch.

Thanks again,
Forest
-- 
Forest Bond
http://www.alittletooquiet.net
http://www.rapidrollout.com

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

[PATCH resend] Input: usbtouchscreen - initialize eGalax devices

[Forest Bond]

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>
---
 drivers/input/touchscreen/usbtouchscreen.c |   36 ++++++++++++++++++++++++++++
 1 files changed, 36 insertions(+), 0 deletions(-)

diff --git a/drivers/input/touchscreen/usbtouchscreen.c b/drivers/input/touchscreen/usbtouchscreen.c
index e32709e..64b4b17 100644
--- a/drivers/input/touchscreen/usbtouchscreen.c
+++ b/drivers/input/touchscreen/usbtouchscreen.c
@@ -304,6 +304,41 @@ 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);
+	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 +1091,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

Re: [PATCH resend] Input: usbtouchscreen - initialize eGalax devices

[Sergey Vlasov]

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

On Fri, Aug 31, 2012 at 08:37:26PM -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>
> ---
>  drivers/input/touchscreen/usbtouchscreen.c |   36 ++++++++++++++++++++++++++++
>  1 files changed, 36 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/usbtouchscreen.c b/drivers/input/touchscreen/usbtouchscreen.c
> index e32709e..64b4b17 100644
> --- a/drivers/input/touchscreen/usbtouchscreen.c
> +++ b/drivers/input/touchscreen/usbtouchscreen.c
> @@ -304,6 +304,41 @@ 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;
> +}

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

[PATCH resend2] Input: usbtouchscreen - initialize eGalax devices

[Forest Bond]

From: Forest Bond <forest.bond-XbHmxybg72Wcj+tRJgl41g@public.gmane.org>

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-XbHmxybg72Wcj+tRJgl41g@public.gmane.org>
---
 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
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH resend2] Input: usbtouchscreen - initialize eGalax devices

[Dmitry Torokhov]

On Mon, Sep 03, 2012 at 01:33:50PM -0400, Forest Bond wrote:
> From: Forest Bond <forest.bond-XbHmxybg72Wcj+tRJgl41g@public.gmane.org>
> 
> 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-XbHmxybg72Wcj+tRJgl41g@public.gmane.org>

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

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH resend2] Input: usbtouchscreen - initialize eGalax devices

[Forest Bond]

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

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-XbHmxybg72Wcj+tRJgl41g@public.gmane.org>
> > 
> > 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-XbHmxybg72Wcj+tRJgl41g@public.gmane.org>
> 
> 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
-- 
Forest Bond
http://www.alittletooquiet.net
http://www.rapidrollout.com

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

Re: [PATCH resend2] Input: usbtouchscreen - initialize eGalax devices

[Dmitry Torokhov]

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!

-- 
Dmitry

Re: [PATCH resend2] Input: usbtouchscreen - initialize eGalax devices

[Jiri Kosina]

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,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH resend2] Input: usbtouchscreen - initialize eGalax devices

[Forest Bond]

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

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
-- 
Forest Bond
http://www.alittletooquiet.net
http://www.rapidrollout.com

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

Re: [PATCH resend2] Input: usbtouchscreen - initialize eGalax devices

[Jiri Kosina]

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?

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH resend2] Input: usbtouchscreen - initialize eGalax devices

[Forest Bond]

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

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
-- 
Forest Bond
http://www.alittletooquiet.net
http://www.rapidrollout.com

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