[PATCH 1/1] HID: ignore afatech 9016

[PATCH 1/1] HID: ignore afatech 9016

[Jiri Slaby]

Let's ignore the device altogether by HID layer. It's handled by
dvb-usb-remote driver properly already.

By now, FULLSPEED_INTERVAL quirk was used. It probably made things
better, but the remote ctrl was still a perfect X killer. This was
the last user of the particular quirk. So remove the quirk as well.

With input going through dvb-usb-remote, the remote works
perfectly.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Jiri Kosina <jkosina@suse.cz>
---
 drivers/hid/usbhid/hid-core.c   |    8 --------
 drivers/hid/usbhid/hid-quirks.c |    2 +-
 include/linux/hid.h             |    1 -
 3 files changed, 1 insertions(+), 10 deletions(-)

diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index e2997a8..36a1561 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -938,14 +938,6 @@ static int usbhid_start(struct hid_device *hid)
 
 		interval = endpoint->bInterval;
 
-		/* Some vendors give fullspeed interval on highspeed devides */
-		if (hid->quirks & HID_QUIRK_FULLSPEED_INTERVAL &&
-		    dev->speed == USB_SPEED_HIGH) {
-			interval = fls(endpoint->bInterval*8);
-			printk(KERN_INFO "%s: Fixing fullspeed to highspeed interval: %d -> %d\n",
-			       hid->name, endpoint->bInterval, interval);
-		}
-
 		/* Change the polling interval of mice. */
 		if (hid->collection->usage == HID_GD_MOUSE && hid_mousepoll_interval > 0)
 			interval = hid_mousepoll_interval;
diff --git a/drivers/hid/usbhid/hid-quirks.c b/drivers/hid/usbhid/hid-quirks.c
index 38773dc..788d9a3 100644
--- a/drivers/hid/usbhid/hid-quirks.c
+++ b/drivers/hid/usbhid/hid-quirks.c
@@ -41,7 +41,7 @@ static const struct hid_blacklist {
 	{ USB_VENDOR_ID_SAITEK, USB_DEVICE_ID_SAITEK_RUMBLEPAD, HID_QUIRK_BADPAD },
 	{ USB_VENDOR_ID_TOPMAX, USB_DEVICE_ID_TOPMAX_COBRAPAD, HID_QUIRK_BADPAD },
 
-	{ USB_VENDOR_ID_AFATECH, USB_DEVICE_ID_AFATECH_AF9016, HID_QUIRK_FULLSPEED_INTERVAL },
+	{ USB_VENDOR_ID_AFATECH, USB_DEVICE_ID_AFATECH_AF9016, HID_QUIRK_IGNORE },
 
 	{ USB_VENDOR_ID_PANTHERLORD, USB_DEVICE_ID_PANTHERLORD_TWIN_USB_JOYSTICK, HID_QUIRK_MULTI_INPUT | HID_QUIRK_SKIP_OUTPUT_REPORTS },
 	{ USB_VENDOR_ID_PLAYDOTCOM, USB_DEVICE_ID_PLAYDOTCOM_EMS_USBII, HID_QUIRK_MULTI_INPUT },
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 8709365..4a33e16 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -311,7 +311,6 @@ struct hid_item {
 #define HID_QUIRK_BADPAD			0x00000020
 #define HID_QUIRK_MULTI_INPUT			0x00000040
 #define HID_QUIRK_SKIP_OUTPUT_REPORTS		0x00010000
-#define HID_QUIRK_FULLSPEED_INTERVAL		0x10000000
 #define HID_QUIRK_NO_INIT_REPORTS		0x20000000
 
 /*
-- 
1.6.5.7

Re: [PATCH 1/1] HID: ignore afatech 9016

[Jiri Kosina]

[ Adding Pekka (the author of the patch that added this quirk for AF9016) to 
  CC ... ]

On Wed, 13 Jan 2010, Jiri Slaby wrote:

> Let's ignore the device altogether by HID layer. It's handled by
> dvb-usb-remote driver properly already.
> 
> By now, FULLSPEED_INTERVAL quirk was used. It probably made things
> better, but the remote ctrl was still a perfect X killer. This was
> the last user of the particular quirk. So remove the quirk as well.
> 
> With input going through dvb-usb-remote, the remote works
> perfectly.
> 
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> Cc: Jiri Kosina <jkosina@suse.cz>
> ---
>  drivers/hid/usbhid/hid-core.c   |    8 --------
>  drivers/hid/usbhid/hid-quirks.c |    2 +-
>  include/linux/hid.h             |    1 -
>  3 files changed, 1 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
> index e2997a8..36a1561 100644
> --- a/drivers/hid/usbhid/hid-core.c
> +++ b/drivers/hid/usbhid/hid-core.c
> @@ -938,14 +938,6 @@ static int usbhid_start(struct hid_device *hid)
>  
>  		interval = endpoint->bInterval;
>  
> -		/* Some vendors give fullspeed interval on highspeed devides */
> -		if (hid->quirks & HID_QUIRK_FULLSPEED_INTERVAL &&
> -		    dev->speed == USB_SPEED_HIGH) {
> -			interval = fls(endpoint->bInterval*8);
> -			printk(KERN_INFO "%s: Fixing fullspeed to highspeed interval: %d -> %d\n",
> -			       hid->name, endpoint->bInterval, interval);
> -		}
> -
>  		/* Change the polling interval of mice. */
>  		if (hid->collection->usage == HID_GD_MOUSE && hid_mousepoll_interval > 0)
>  			interval = hid_mousepoll_interval;
> diff --git a/drivers/hid/usbhid/hid-quirks.c b/drivers/hid/usbhid/hid-quirks.c
> index 38773dc..788d9a3 100644
> --- a/drivers/hid/usbhid/hid-quirks.c
> +++ b/drivers/hid/usbhid/hid-quirks.c
> @@ -41,7 +41,7 @@ static const struct hid_blacklist {
>  	{ USB_VENDOR_ID_SAITEK, USB_DEVICE_ID_SAITEK_RUMBLEPAD, HID_QUIRK_BADPAD },
>  	{ USB_VENDOR_ID_TOPMAX, USB_DEVICE_ID_TOPMAX_COBRAPAD, HID_QUIRK_BADPAD },
>  
> -	{ USB_VENDOR_ID_AFATECH, USB_DEVICE_ID_AFATECH_AF9016, HID_QUIRK_FULLSPEED_INTERVAL },
> +	{ USB_VENDOR_ID_AFATECH, USB_DEVICE_ID_AFATECH_AF9016, HID_QUIRK_IGNORE },

Hmm, why do we keep HID_QUIRK_IGNORE anyway, when we already have generic 
hid_ignore_list[]?

We don't set it for any device in the current codebase any more.

Thanks,x

-- 
Jiri Kosina
SUSE Labs, Novell Inc.

Re: [PATCH 1/1] HID: ignore afatech 9016

[Jiri Slaby]

On 01/13/2010 09:20 PM, Jiri Kosina wrote:
>> diff --git a/drivers/hid/usbhid/hid-quirks.c b/drivers/hid/usbhid/hid-quirks.c
>> index 38773dc..788d9a3 100644
>> --- a/drivers/hid/usbhid/hid-quirks.c
>> +++ b/drivers/hid/usbhid/hid-quirks.c
>> @@ -41,7 +41,7 @@ static const struct hid_blacklist {
>>  	{ USB_VENDOR_ID_SAITEK, USB_DEVICE_ID_SAITEK_RUMBLEPAD, HID_QUIRK_BADPAD },
>>  	{ USB_VENDOR_ID_TOPMAX, USB_DEVICE_ID_TOPMAX_COBRAPAD, HID_QUIRK_BADPAD },
>>  
>> -	{ USB_VENDOR_ID_AFATECH, USB_DEVICE_ID_AFATECH_AF9016, HID_QUIRK_FULLSPEED_INTERVAL },
>> +	{ USB_VENDOR_ID_AFATECH, USB_DEVICE_ID_AFATECH_AF9016, HID_QUIRK_IGNORE },
> 
> Hmm, why do we keep HID_QUIRK_IGNORE anyway, when we already have generic 
> hid_ignore_list[]?

You returned it back because of dynamic quirks...

> We don't set it for any device in the current codebase any more.

Oh yeah, it's hard for people who don't remember code they wrote :).
Will respin. Thanks for the reminder.

-- 
js

[PATCH v2 1/1] HID: ignore afatech 9016

[Jiri Slaby]

Let's ignore the device altogether by the HID layer. It's handled
by dvb-usb-remote driver already.

By now, FULLSPEED_INTERVAL quirk was used. It probably made things
better, but the remote controller was still a perfect X killer.
This was the last user of the particular quirk. So remove the quirk
as well.

With input going through dvb-usb-remote, the remote works
perfectly.

The device is 15a4:9016.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Jiri Kosina <jkosina@suse.cz>
Cc: Pekka Sarnila <sarnila@adit.fi>
---
 drivers/hid/hid-core.c          |    1 +
 drivers/hid/usbhid/hid-core.c   |    8 --------
 drivers/hid/usbhid/hid-quirks.c |    2 --
 include/linux/hid.h             |    1 -
 4 files changed, 1 insertions(+), 11 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 08f8f23..0ae0bfd 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1534,6 +1534,7 @@ static const struct hid_device_id hid_ignore_list[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_ACECAD, USB_DEVICE_ID_ACECAD_FLAIR) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_ACECAD, USB_DEVICE_ID_ACECAD_302) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_ADS_TECH, USB_DEVICE_ID_ADS_TECH_RADIO_SI470X) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_AFATECH, USB_DEVICE_ID_AFATECH_AF9016) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_AIPTEK, USB_DEVICE_ID_AIPTEK_01) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_AIPTEK, USB_DEVICE_ID_AIPTEK_10) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_AIPTEK, USB_DEVICE_ID_AIPTEK_20) },
diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index e2997a8..36a1561 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -938,14 +938,6 @@ static int usbhid_start(struct hid_device *hid)
 
 		interval = endpoint->bInterval;
 
-		/* Some vendors give fullspeed interval on highspeed devides */
-		if (hid->quirks & HID_QUIRK_FULLSPEED_INTERVAL &&
-		    dev->speed == USB_SPEED_HIGH) {
-			interval = fls(endpoint->bInterval*8);
-			printk(KERN_INFO "%s: Fixing fullspeed to highspeed interval: %d -> %d\n",
-			       hid->name, endpoint->bInterval, interval);
-		}
-
 		/* Change the polling interval of mice. */
 		if (hid->collection->usage == HID_GD_MOUSE && hid_mousepoll_interval > 0)
 			interval = hid_mousepoll_interval;
diff --git a/drivers/hid/usbhid/hid-quirks.c b/drivers/hid/usbhid/hid-quirks.c
index 38773dc..f2ae8a7 100644
--- a/drivers/hid/usbhid/hid-quirks.c
+++ b/drivers/hid/usbhid/hid-quirks.c
@@ -41,8 +41,6 @@ static const struct hid_blacklist {
 	{ USB_VENDOR_ID_SAITEK, USB_DEVICE_ID_SAITEK_RUMBLEPAD, HID_QUIRK_BADPAD },
 	{ USB_VENDOR_ID_TOPMAX, USB_DEVICE_ID_TOPMAX_COBRAPAD, HID_QUIRK_BADPAD },
 
-	{ USB_VENDOR_ID_AFATECH, USB_DEVICE_ID_AFATECH_AF9016, HID_QUIRK_FULLSPEED_INTERVAL },
-
 	{ USB_VENDOR_ID_PANTHERLORD, USB_DEVICE_ID_PANTHERLORD_TWIN_USB_JOYSTICK, HID_QUIRK_MULTI_INPUT | HID_QUIRK_SKIP_OUTPUT_REPORTS },
 	{ USB_VENDOR_ID_PLAYDOTCOM, USB_DEVICE_ID_PLAYDOTCOM_EMS_USBII, HID_QUIRK_MULTI_INPUT },
 
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 8709365..4a33e16 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -311,7 +311,6 @@ struct hid_item {
 #define HID_QUIRK_BADPAD			0x00000020
 #define HID_QUIRK_MULTI_INPUT			0x00000040
 #define HID_QUIRK_SKIP_OUTPUT_REPORTS		0x00010000
-#define HID_QUIRK_FULLSPEED_INTERVAL		0x10000000
 #define HID_QUIRK_NO_INIT_REPORTS		0x20000000
 
 /*
-- 
1.6.5.7

Re: [PATCH 1/1] HID: ignore afatech 9016

[Jiri Kosina]

On Wed, 13 Jan 2010, Jiri Slaby wrote:

> >> --- a/drivers/hid/usbhid/hid-quirks.c
> >> +++ b/drivers/hid/usbhid/hid-quirks.c
> >> @@ -41,7 +41,7 @@ static const struct hid_blacklist {
> >>  	{ USB_VENDOR_ID_SAITEK, USB_DEVICE_ID_SAITEK_RUMBLEPAD, HID_QUIRK_BADPAD },
> >>  	{ USB_VENDOR_ID_TOPMAX, USB_DEVICE_ID_TOPMAX_COBRAPAD, HID_QUIRK_BADPAD },
> >>  
> >> -	{ USB_VENDOR_ID_AFATECH, USB_DEVICE_ID_AFATECH_AF9016, HID_QUIRK_FULLSPEED_INTERVAL },
> >> +	{ USB_VENDOR_ID_AFATECH, USB_DEVICE_ID_AFATECH_AF9016, HID_QUIRK_IGNORE },
> > 
> > Hmm, why do we keep HID_QUIRK_IGNORE anyway, when we already have generic 
> > hid_ignore_list[]?
> 
> You returned it back because of dynamic quirks...

Right you are.

> > We don't set it for any device in the current codebase any more.
> 
> Oh yeah, it's hard for people who don't remember code they wrote :).

Oh, right ... happened to me as well, see a few lines above :)

> Will respin. Thanks for the reminder.

Thanks,

-- 
Jiri Kosina
SUSE Labs, Novell Inc.

Re: [PATCH v2 1/1] HID: ignore afatech 9016

[Jiri Kosina]

On Wed, 13 Jan 2010, Jiri Slaby wrote:

> Let's ignore the device altogether by the HID layer. It's handled
> by dvb-usb-remote driver already.
> 
> By now, FULLSPEED_INTERVAL quirk was used. It probably made things
> better, but the remote controller was still a perfect X killer.
> This was the last user of the particular quirk. So remove the quirk
> as well.
> 
> With input going through dvb-usb-remote, the remote works
> perfectly.
> 
> The device is 15a4:9016.

Pekka, did you have chance to verify whether it works fine also with your 
version of the remote, or you still need the FULLSPEED_INTERVAL quirk on 
your side?

Thanks.

> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> Cc: Jiri Kosina <jkosina@suse.cz>
> Cc: Pekka Sarnila <sarnila@adit.fi>
> ---
>  drivers/hid/hid-core.c          |    1 +
>  drivers/hid/usbhid/hid-core.c   |    8 --------
>  drivers/hid/usbhid/hid-quirks.c |    2 --
>  include/linux/hid.h             |    1 -
>  4 files changed, 1 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index 08f8f23..0ae0bfd 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -1534,6 +1534,7 @@ static const struct hid_device_id hid_ignore_list[] = {
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_ACECAD, USB_DEVICE_ID_ACECAD_FLAIR) },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_ACECAD, USB_DEVICE_ID_ACECAD_302) },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_ADS_TECH, USB_DEVICE_ID_ADS_TECH_RADIO_SI470X) },
> +	{ HID_USB_DEVICE(USB_VENDOR_ID_AFATECH, USB_DEVICE_ID_AFATECH_AF9016) },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_AIPTEK, USB_DEVICE_ID_AIPTEK_01) },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_AIPTEK, USB_DEVICE_ID_AIPTEK_10) },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_AIPTEK, USB_DEVICE_ID_AIPTEK_20) },
> diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
> index e2997a8..36a1561 100644
> --- a/drivers/hid/usbhid/hid-core.c
> +++ b/drivers/hid/usbhid/hid-core.c
> @@ -938,14 +938,6 @@ static int usbhid_start(struct hid_device *hid)
>  
>  		interval = endpoint->bInterval;
>  
> -		/* Some vendors give fullspeed interval on highspeed devides */
> -		if (hid->quirks & HID_QUIRK_FULLSPEED_INTERVAL &&
> -		    dev->speed == USB_SPEED_HIGH) {
> -			interval = fls(endpoint->bInterval*8);
> -			printk(KERN_INFO "%s: Fixing fullspeed to highspeed interval: %d -> %d\n",
> -			       hid->name, endpoint->bInterval, interval);
> -		}
> -
>  		/* Change the polling interval of mice. */
>  		if (hid->collection->usage == HID_GD_MOUSE && hid_mousepoll_interval > 0)
>  			interval = hid_mousepoll_interval;
> diff --git a/drivers/hid/usbhid/hid-quirks.c b/drivers/hid/usbhid/hid-quirks.c
> index 38773dc..f2ae8a7 100644
> --- a/drivers/hid/usbhid/hid-quirks.c
> +++ b/drivers/hid/usbhid/hid-quirks.c
> @@ -41,8 +41,6 @@ static const struct hid_blacklist {
>  	{ USB_VENDOR_ID_SAITEK, USB_DEVICE_ID_SAITEK_RUMBLEPAD, HID_QUIRK_BADPAD },
>  	{ USB_VENDOR_ID_TOPMAX, USB_DEVICE_ID_TOPMAX_COBRAPAD, HID_QUIRK_BADPAD },
>  
> -	{ USB_VENDOR_ID_AFATECH, USB_DEVICE_ID_AFATECH_AF9016, HID_QUIRK_FULLSPEED_INTERVAL },
> -
>  	{ USB_VENDOR_ID_PANTHERLORD, USB_DEVICE_ID_PANTHERLORD_TWIN_USB_JOYSTICK, HID_QUIRK_MULTI_INPUT | HID_QUIRK_SKIP_OUTPUT_REPORTS },
>  	{ USB_VENDOR_ID_PLAYDOTCOM, USB_DEVICE_ID_PLAYDOTCOM_EMS_USBII, HID_QUIRK_MULTI_INPUT },
>  
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index 8709365..4a33e16 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -311,7 +311,6 @@ struct hid_item {
>  #define HID_QUIRK_BADPAD			0x00000020
>  #define HID_QUIRK_MULTI_INPUT			0x00000040
>  #define HID_QUIRK_SKIP_OUTPUT_REPORTS		0x00010000
> -#define HID_QUIRK_FULLSPEED_INTERVAL		0x10000000
>  #define HID_QUIRK_NO_INIT_REPORTS		0x20000000
>  
>  /*
> -- 
> 1.6.5.7
> 

-- 
Jiri Kosina
SUSE Labs, Novell Inc.

Re: [PATCH v2 1/1] HID: ignore afatech 9016

[Pekka Sarnila]

Well, as I said I use now different TV-stick. But I have the old one 
somewhere. I'll try to find it and check it.

Pekka

Jiri Kosina wrote:
> On Wed, 13 Jan 2010, Jiri Slaby wrote:
> 
> 
>>Let's ignore the device altogether by the HID layer. It's handled
>>by dvb-usb-remote driver already.
>>
>>By now, FULLSPEED_INTERVAL quirk was used. It probably made things
>>better, but the remote controller was still a perfect X killer.
>>This was the last user of the particular quirk. So remove the quirk
>>as well.
>>
>>With input going through dvb-usb-remote, the remote works
>>perfectly.
>>
>>The device is 15a4:9016.
> 
> 
> Pekka, did you have chance to verify whether it works fine also with your 
> version of the remote, or you still need the FULLSPEED_INTERVAL quirk on 
> your side?
> 
> Thanks.
> 
> 
>>Signed-off-by: Jiri Slaby <jslaby@suse.cz>
>>Cc: Jiri Kosina <jkosina@suse.cz>
>>Cc: Pekka Sarnila <sarnila@adit.fi>
>>---
>> drivers/hid/hid-core.c          |    1 +
>> drivers/hid/usbhid/hid-core.c   |    8 --------
>> drivers/hid/usbhid/hid-quirks.c |    2 --
>> include/linux/hid.h             |    1 -
>> 4 files changed, 1 insertions(+), 11 deletions(-)
>>
>>diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
>>index 08f8f23..0ae0bfd 100644
>>--- a/drivers/hid/hid-core.c
>>+++ b/drivers/hid/hid-core.c
>>@@ -1534,6 +1534,7 @@ static const struct hid_device_id hid_ignore_list[] = {
>> 	{ HID_USB_DEVICE(USB_VENDOR_ID_ACECAD, USB_DEVICE_ID_ACECAD_FLAIR) },
>> 	{ HID_USB_DEVICE(USB_VENDOR_ID_ACECAD, USB_DEVICE_ID_ACECAD_302) },
>> 	{ HID_USB_DEVICE(USB_VENDOR_ID_ADS_TECH, USB_DEVICE_ID_ADS_TECH_RADIO_SI470X) },
>>+	{ HID_USB_DEVICE(USB_VENDOR_ID_AFATECH, USB_DEVICE_ID_AFATECH_AF9016) },
>> 	{ HID_USB_DEVICE(USB_VENDOR_ID_AIPTEK, USB_DEVICE_ID_AIPTEK_01) },
>> 	{ HID_USB_DEVICE(USB_VENDOR_ID_AIPTEK, USB_DEVICE_ID_AIPTEK_10) },
>> 	{ HID_USB_DEVICE(USB_VENDOR_ID_AIPTEK, USB_DEVICE_ID_AIPTEK_20) },
>>diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
>>index e2997a8..36a1561 100644
>>--- a/drivers/hid/usbhid/hid-core.c
>>+++ b/drivers/hid/usbhid/hid-core.c
>>@@ -938,14 +938,6 @@ static int usbhid_start(struct hid_device *hid)
>> 
>> 		interval = endpoint->bInterval;
>> 
>>-		/* Some vendors give fullspeed interval on highspeed devides */
>>-		if (hid->quirks & HID_QUIRK_FULLSPEED_INTERVAL &&
>>-		    dev->speed == USB_SPEED_HIGH) {
>>-			interval = fls(endpoint->bInterval*8);
>>-			printk(KERN_INFO "%s: Fixing fullspeed to highspeed interval: %d -> %d\n",
>>-			       hid->name, endpoint->bInterval, interval);
>>-		}
>>-
>> 		/* Change the polling interval of mice. */
>> 		if (hid->collection->usage == HID_GD_MOUSE && hid_mousepoll_interval > 0)
>> 			interval = hid_mousepoll_interval;
>>diff --git a/drivers/hid/usbhid/hid-quirks.c b/drivers/hid/usbhid/hid-quirks.c
>>index 38773dc..f2ae8a7 100644
>>--- a/drivers/hid/usbhid/hid-quirks.c
>>+++ b/drivers/hid/usbhid/hid-quirks.c
>>@@ -41,8 +41,6 @@ static const struct hid_blacklist {
>> 	{ USB_VENDOR_ID_SAITEK, USB_DEVICE_ID_SAITEK_RUMBLEPAD, HID_QUIRK_BADPAD },
>> 	{ USB_VENDOR_ID_TOPMAX, USB_DEVICE_ID_TOPMAX_COBRAPAD, HID_QUIRK_BADPAD },
>> 
>>-	{ USB_VENDOR_ID_AFATECH, USB_DEVICE_ID_AFATECH_AF9016, HID_QUIRK_FULLSPEED_INTERVAL },
>>-
>> 	{ USB_VENDOR_ID_PANTHERLORD, USB_DEVICE_ID_PANTHERLORD_TWIN_USB_JOYSTICK, HID_QUIRK_MULTI_INPUT | HID_QUIRK_SKIP_OUTPUT_REPORTS },
>> 	{ USB_VENDOR_ID_PLAYDOTCOM, USB_DEVICE_ID_PLAYDOTCOM_EMS_USBII, HID_QUIRK_MULTI_INPUT },
>> 
>>diff --git a/include/linux/hid.h b/include/linux/hid.h
>>index 8709365..4a33e16 100644
>>--- a/include/linux/hid.h
>>+++ b/include/linux/hid.h
>>@@ -311,7 +311,6 @@ struct hid_item {
>> #define HID_QUIRK_BADPAD			0x00000020
>> #define HID_QUIRK_MULTI_INPUT			0x00000040
>> #define HID_QUIRK_SKIP_OUTPUT_REPORTS		0x00010000
>>-#define HID_QUIRK_FULLSPEED_INTERVAL		0x10000000
>> #define HID_QUIRK_NO_INIT_REPORTS		0x20000000
>> 
>> /*
>>-- 
>>1.6.5.7
>>
> 
> 

dvb-usb-remote woes [was: HID: ignore afatech 9016]

[Jiri Slaby]

On 02/01/2010 07:28 PM, Jiri Kosina wrote:
> On Mon, 1 Feb 2010, Pekka Sarnila wrote:
>> Well short answer is: No, it does not work.

Hi, thanks for the input.

>> What I did:
>>
>> I pulled few days ago latest
>>
>>    git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
>>
>> and compiled it. Everything works fine including the tv-stick and the
>> remote. However I get:
>>
>>   <3>af9015: command failed:255
>>   <3>dvb-usb: error while querying for an remote control event.

Yes, I saw this quite recently too. For me it appears when it is booted
up with the stick in. It's still to be fixed.

>> I applied the patch in your mail and recompiled. Remote does not work;
>> af9015.c gets no key presses. Instead that above message still comes.

Yeah, and that's the reason why it doesn't work.

>> Also removing the device or rmmod:ing dvb-usb-af9015.ko causes kernel oops.

Already fixed:
http://patchwork.kernel.org/patch/74095/

>> Analysis:
>>
>> When it works, the remote is recognized by the system as a HID-keyboard,
>> and thus it works as a normal USB-keyboard. dvb-usb-remote.c is not
>> participating at all (though it gives those errors).

Actually it is. But it fails to get status.

>> The remote is visible to the system as a usb interrupt end point.
>> Interrupt endpoint tells the system the polling interval (by endpoint
>> report). From the USB specs on the interval:
>>
>>   The USB System Software will use this information during configuration
>>   to determine a period that can be sustained. The period provided by
>>   the system may be shorter than that desired by the device up to the
>>   shortest period defined by the USB (125 µs microframe or 1 ms frame).
>>   The client software and device can depend only on the fact that the
>>   host will ensure that the time duration between two transaction
>>   attempts with the endpoint will be no longer than the desired period.
>>
>> As I understand it, in plain English it means that the polling interval
>> must not be longer than what the endpoint report says (in case of
>> full-speed endpoint 1 to 255 ms). This device in question, assuming it
>> gives the interval in full-speed mode even it really is high-speed
>> device, gives 16ms interval period.
>>
>> However af9015.c/dvb-usb-remote.c accesses it as if it was a bulk
>> endpoint and with 150ms interval. I did not look further on if this is
>> the only reason or the reason for it not to work. But at least it is in
>> clear contradiction of the USB-specs.

To what statement is it in contradiction? The statement above is for
interrupt transfers.

>> I suspect, to make it work, much of the code from hid-driver should be
>> copied to the device driver.

Which code? I see no code being copied.

>> However I think that would be exactly the
>> wrong way to go. I think the whole idea of making for every device a
>> separated driver ignoring the common code already in the kernel is bad
>> architecture.

Common code does not work well here. Don't you see weird key repetitions
and similar?

>> I have noticed more and more of this type of movement in
>> the driver development resulting the same thing done in hundred
>> different ways when common code could be used. Especially bad I think it
>> is in the cases where universal specification is available (albeit there
>> are many devices that do not follow the usb specs correctly).

In this case I suppose this is why dvb-usb-remote exists.

Maybe it can be handled better by a separate driver (still) as a part of
the HID layer nowadays.

Patrick, Johannes, why was not dvb-usb-remote implemented rather as a
part of the HID layer?

>> Also it is wrong idea that you could/should detect the type of remote
>> controller based on the tv-stick. E.g with Haupauge TV-NOVA nearly any
>> remote works ok (e.g my panasonic tv remote). Every generic ir-receiver
>> sends also the manufacturer and model codes. Remote ir-to-code
>> translates should be based on that (there is a kind of generic layer for
>> that in  linux kernel) and not on the tv-stick model at all (as in the
>> af9015 driver).

Sorry, I don't understand this paragraph. Could you rephrase?

>> BTW I now recall how I got Afateck remote work as should. The driver
>> loads ir-to-code translate table to the stick. I changed the codes to
>> what made more sense.

Why you didn't push this upstream?

>> One problem here is that current HID layer is very sparse in the
>> information it passes on, really only a code. It should also convey the
>> precice id of the device so upper layer would be better able to deal
>> with events. One of my hobbies is flight simulator flying. I use X-plane
>> (excellent program). There is also a linux version. But the linux
>> joystick driver is far from adequate for it (also some joysticks, yokes
>> and pedals or some of their controls didn't work at all). So I use a
>> special kernel for which I rewrote big parts of the HID-driver and
>> input-driver and wrote completely new joystick driver. Now it works
>> quite decently. One problem is the kernel->user joystick driver
>> interface, but it can not be changed since that's what X-plane (and
>> others use). One thing I got to change for that was that HID does less
>> model specific processing and passes more event info to higher layer
>> without breaking the old HID-devices (same with the input layer), so
>> that the joystick layer could do more intelligent processing. I fixed
>> also some bugs and one design omission in the HID driver (e.g. some
>> force feedback joysticks uses that HID-specification, in standard kernel
>> there is some ad-hoc code for that purpose).

Can't be HID bus with a specific driver used instead now?

-- 
js
suse labs

Re: dvb-usb-remote woes [was: HID: ignore afatech 9016]

[Antti Palosaari]

On 02/01/2010 09:42 PM, Jiri Slaby wrote:
> On 02/01/2010 07:28 PM, Jiri Kosina wrote:
>> On Mon, 1 Feb 2010, Pekka Sarnila wrote:
>>> I pulled few days ago latest
>>>
>>>     git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
>>>
>>> and compiled it. Everything works fine including the tv-stick and the
>>> remote. However I get:
>>>
>>>    <3>af9015: command failed:255
>>>    <3>dvb-usb: error while querying for an remote control event.
>
> Yes, I saw this quite recently too. For me it appears when it is booted
> up with the stick in. It's still to be fixed.

I suspect you are using old firmware, 4.65.0.0 probably, that does not 
support remote polling and thus this 255 errors seen.

regards
Antti
-- 
http://palosaari.fi/

Re: dvb-usb-remote woes [was: HID: ignore afatech 9016]

[Jiri Slaby]

On 02/01/2010 10:23 PM, Antti Palosaari wrote:
> On 02/01/2010 09:42 PM, Jiri Slaby wrote:
>> On 02/01/2010 07:28 PM, Jiri Kosina wrote:
>>> On Mon, 1 Feb 2010, Pekka Sarnila wrote:
>>>>    <3>af9015: command failed:255
>>>>    <3>dvb-usb: error while querying for an remote control event.
>>
>> Yes, I saw this quite recently too. For me it appears when it is booted
>> up with the stick in. It's still to be fixed.
> 
> I suspect you are using old firmware, 4.65.0.0 probably, that does not
> support remote polling and thus this 255 errors seen.

For me:
af9013: firmware version:4.95.0

As I wrote, for me it happens iff it is plugged-in while booting. I'll
investigate it further later -- that it a reason why I haven't reported
it yet.

-- 
js
suse labs

Re: dvb-usb-remote woes [was: HID: ignore afatech 9016]

[Pekka Sarnila]

Ok, that was it, I had the old firmware. Now it works with the patch. I 
can now see that the afateck driver does not use the HID 
interface/endpoint (1/83) at all, but vendor specific bulk endpoint (0/81).

The fact that manufacturers have started to more more use the usb vendor 
class instead of the HID class is probably partly a consequence of HID 
specification being poorly designed. Oh, well. More work for driver writers.

Anyway, I'm still of the opinion that the ir-to-code translate should be 
per remote controller not per tv-stick (ideally loaded by kernel from 
the userspace table, and easily modifiable by userspace program).

All in all the remote controller stack is IMHO a real mess: different 
level translates (in kernel and in user space), different user program 
interfaces (keyboard with or without the special keys (many programs 
recognize only the standard keyboard keys), lircd direct, lircd/dcop). 
Really easiest is to modify kernel to get what you want. For average 
user the task of getting remote to do what they want is close to mission 
impossible. Hope someone would do something about this.


Pekka


Antti Palosaari wrote:
> On 02/01/2010 09:42 PM, Jiri Slaby wrote:
> 
>> On 02/01/2010 07:28 PM, Jiri Kosina wrote:
>>
>>> On Mon, 1 Feb 2010, Pekka Sarnila wrote:
>>>
>>>> I pulled few days ago latest
>>>>
>>>>     
>>>> git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
>>>>
>>>> and compiled it. Everything works fine including the tv-stick and the
>>>> remote. However I get:
>>>>
>>>>    <3>af9015: command failed:255
>>>>    <3>dvb-usb: error while querying for an remote control event.
>>
>>
>> Yes, I saw this quite recently too. For me it appears when it is booted
>> up with the stick in. It's still to be fixed.
> 
> 
> I suspect you are using old firmware, 4.65.0.0 probably, that does not 
> support remote polling and thus this 255 errors seen.
> 
> regards
> Antti

Re: dvb-usb-remote woes [was: HID: ignore afatech 9016]

[Pekka Sarnila]

Please, no mail to this address, use only sarnila@adit.fi.

Pekka

Antti Palosaari wrote:
> On 02/01/2010 09:42 PM, Jiri Slaby wrote:
> 
>> On 02/01/2010 07:28 PM, Jiri Kosina wrote:
>>
>>> On Mon, 1 Feb 2010, Pekka Sarnila wrote:
>>>
>>>> I pulled few days ago latest
>>>>
>>>>     
>>>> git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
>>>>
>>>> and compiled it. Everything works fine including the tv-stick and the
>>>> remote. However I get:
>>>>
>>>>    <3>af9015: command failed:255
>>>>    <3>dvb-usb: error while querying for an remote control event.
>>
>>
>> Yes, I saw this quite recently too. For me it appears when it is booted
>> up with the stick in. It's still to be fixed.
> 
> 
> I suspect you are using old firmware, 4.65.0.0 probably, that does not 
> support remote polling and thus this 255 errors seen.
> 
> regards
> Antti

Re: dvb-usb-remote woes [was: HID: ignore afatech 9016]

[Pekka Sarnila]

In the old kernel (2.6.24) it did not work if it was plugged while 
booting. If I recall right then it was due to the order different layers 
are initialized (modules loaded).

Pekka


Jiri Slaby wrote:
> On 02/01/2010 10:23 PM, Antti Palosaari wrote:
> 
>>On 02/01/2010 09:42 PM, Jiri Slaby wrote:
>>
>>>On 02/01/2010 07:28 PM, Jiri Kosina wrote:
>>>
>>>>On Mon, 1 Feb 2010, Pekka Sarnila wrote:
>>>>
>>>>>   <3>af9015: command failed:255
>>>>>   <3>dvb-usb: error while querying for an remote control event.
>>>
>>>Yes, I saw this quite recently too. For me it appears when it is booted
>>>up with the stick in. It's still to be fixed.
>>
>>I suspect you are using old firmware, 4.65.0.0 probably, that does not
>>support remote polling and thus this 255 errors seen.
> 
> 
> For me:
> af9013: firmware version:4.95.0
> 
> As I wrote, for me it happens iff it is plugged-in while booting. I'll
> investigate it further later -- that it a reason why I haven't reported
> it yet.
> 

Re: dvb-usb-remote woes [was: HID: ignore afatech 9016]

[Pekka Sarnila]

Most of the question here are already answered or no more actual, but 
for clarity:

Jiri Slaby wrote:
>>>The remote is visible to the system as a usb interrupt end point.
>>>Interrupt endpoint tells the system the polling interval (by endpoint
>>>report). From the USB specs on the interval:
>>>
>>>  The USB System Software will use this information during configuration
>>>  to determine a period that can be sustained. The period provided by
>>>  the system may be shorter than that desired by the device up to the
>>>  shortest period defined by the USB (125 µs microframe or 1 ms frame).
>>>  The client software and device can depend only on the fact that the
>>>  host will ensure that the time duration between two transaction
>>>  attempts with the endpoint will be no longer than the desired period.
>>>
>>>As I understand it, in plain English it means that the polling interval
>>>must not be longer than what the endpoint report says (in case of
>>>full-speed endpoint 1 to 255 ms). This device in question, assuming it
>>>gives the interval in full-speed mode even it really is high-speed
>>>device, gives 16ms interval period.
>>>
>>>However af9015.c/dvb-usb-remote.c accesses it as if it was a bulk
>>>endpoint and with 150ms interval. I did not look further on if this is
>>>the only reason or the reason for it not to work. But at least it is in
>>>clear contradiction of the USB-specs.
> 
> To what statement is it in contradiction? The statement above is for
> interrupt transfers.

The answer was that there is no contradiction since the 150 ms was for 
the bulk endpoint (0/81). When writing this I thought af9015.c used the 
HID endpoint (1/83), which is interrupt endpoint, as an bulk endpoint.

>>>I suspect, to make it work, much of the code from hid-driver should be
>>>copied to the device driver. 
> 
> Which code? I see no code being copied.

I meant that to make af9015.c to use HID endpoint past HID layer would 
require copying big parts from the HID layer to af9015.c. Again it was 
due to the above misunderstanding on my part.

>>>However I think that would be exactly the
>>>wrong way to go. I think the whole idea of making for every device a
>>>separated driver ignoring the common code already in the kernel is bad
>>>architecture.
> 
> Common code does not work well here. Don't you see weird key repetitions
> and similar?

No, it worked ok for me.

> Patrick, Johannes, why was not dvb-usb-remote implemented rather as a
> part of the HID layer?

Because it's for device specific drivers that uses vendor class endpoint 
instead of the HID class endpoint.

>>>Also it is wrong idea that you could/should detect the type of remote
>>>controller based on the tv-stick. E.g with Haupauge TV-NOVA nearly any
>>>remote works ok (e.g my panasonic tv remote). Every generic ir-receiver
>>>sends also the manufacturer and model codes. Remote ir-to-code
>>>translates should be based on that (there is a kind of generic layer for
>>>that in  linux kernel) and not on the tv-stick model at all (as in the
>>>af9015 driver). 
> 
> Sorry, I don't understand this paragraph. Could you rephrase?

This has been, it seems, the one of the very goals of IR input coding: 
to make remotes other than the ones provided with tv-stick to work with 
that same stick.

>>>BTW I now recall how I got Afateck remote work as should. The driver
>>>loads ir-to-code translate table to the stick. I changed the codes to
>>>what made more sense.
> 
> Why you didn't push this upstream?

Well the driver was at the time not part of the standard kernel 
(downloaded form Matti Palosaaris site). And I put there directly the 
keycodes that worked with command line mplayer. No standard solution.

>>>One problem here is that current HID layer is very sparse in the
>>>information it passes on, really only a code. It should also convey the
>>>precice id of the device so upper layer would be better able to deal
>>>with events. One of my hobbies is flight simulator flying. I use X-plane
>>>(excellent program). There is also a linux version. But the linux
>>>joystick driver is far from adequate for it (also some joysticks, yokes
>>>and pedals or some of their controls didn't work at all). So I use a
>>>special kernel for which I rewrote big parts of the HID-driver and
>>>input-driver and wrote completely new joystick driver. Now it works
>>>quite decently. One problem is the kernel->user joystick driver
>>>interface, but it can not be changed since that's what X-plane (and
>>>others use). One thing I got to change for that was that HID does less
>>>model specific processing and passes more event info to higher layer
>>>without breaking the old HID-devices (same with the input layer), so
>>>that the joystick layer could do more intelligent processing. I fixed
>>>also some bugs and one design omission in the HID driver (e.g. some
>>>force feedback joysticks uses that HID-specification, in standard kernel
>>>there is some ad-hoc code for that purpose).
> 
> Can't be HID bus with a specific driver used instead now?

Well it could, but this way it is much less work and more generic. I use 
many different joysticks, yokes and pedals. And with some generic 
modifications and improvements into generic HID layer and generic input 
layer all worked well. Only joystick layer got to be completely rewritten.

I did never put this upstream because by the time I got my own patches 
integrated to the (new) kernel, the hid/input layer had developed so 
much that the patches could no more be used in the latest kernel. So I 
hand applied them again, and again kernel had moved on, and so on. Also 
to argue for patches that cover several areas and several maintainers is 
difficult, and changing a lot at once is always risky. So I gave up.

If anyone is interested, I could take a look again and see if the 
changes could be argued and applied incrementally instead of one big bunch.

Pekka

Re: dvb-usb-remote woes [was: HID: ignore afatech 9016]

[Jiri Kosina]

On Fri, 5 Feb 2010, Pekka Sarnila wrote:

> > Can't be HID bus with a specific driver used instead now?
> 
> Well it could, but this way it is much less work and more generic. I use many
> different joysticks, yokes and pedals. And with some generic modifications and
> improvements into generic HID layer and generic input layer all worked well.
> Only joystick layer got to be completely rewritten.
> 
> I did never put this upstream because by the time I got my own patches
> integrated to the (new) kernel, the hid/input layer had developed so much that
> the patches could no more be used in the latest kernel. So I hand applied them
> again, and again kernel had moved on, and so on. Also to argue for patches
> that cover several areas and several maintainers is difficult, and changing a
> lot at once is always risky. So I gave up.
> 
> If anyone is interested, I could take a look again and see if the changes
> could be argued and applied incrementally instead of one big bunch.

Hi Pekka,

yes, we are definitely interested (or at least I am).

The major rewrite of the HID core to be full-fledged bus was done exactly 
so that it's easier to add support for new devices, while keeping the main 
code clean.

Even if you have problems porting the drivers to new infrastructure, you 
can always post wha you have -- I believe that we will be able to sort it 
out quickly.

Thanks,

-- 
Jiri Kosina
SUSE Labs, Novell Inc.