Bug 63837 - [uhid] [patch] USB: hid_is_collection() only looks for the first item
Summary: [uhid] [patch] USB: hid_is_collection() only looks for the first item
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: usb (show other bugs)
Version: 4.9-STABLE
Hardware: Any Any
: Normal Affects Only Me
Assignee: Mark Linimon
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2004-03-06 13:20 UTC by Nicola Vitale
Modified: 2010-08-28 11:39 UTC (History)
1 user (show)

See Also:


Attachments
hid.c.patch (593 bytes, patch)
2004-03-06 13:20 UTC, Nicola Vitale
no flags Details | Diff
hid-race.patch (375 bytes, patch)
2004-08-09 20:35 UTC, Anish Mistry
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nicola Vitale 2004-03-06 13:20:17 UTC
	The function hid_is_collection, in this file:

__FBSDID("$FreeBSD: src/sys/dev/usb/hid.c,v 1.11.2.7 2004/03/01 00:07:21 julian Exp $");

	only looks for the first item and does not "parse" the whole of
	Report Descriptor, so if the usage, which you are checking, is
	not the first the function always returns 0.

Fix: A possible solution:
How-To-Repeat: 
	For example, using an USB digitizer, modify the ums(4)
	driver, so that it looks for

	        HID_USAGE2(HUP_DIGITIZERS, HUP_DIGITIZER)
	
	(HUP_DIGITIZER==0x01, see HID Usages Tables),
	
	etc, etc,...
Comment 1 Norikatsu Shigemura freebsd_committer freebsd_triage 2004-03-06 15:42:42 UTC
Responsible Changed
From-To: freebsd-bugs->sanpei

Over to USB specialist.
Comment 2 MIHIRA Sanpei Yoshiro freebsd_committer freebsd_triage 2004-03-07 05:45:08 UTC
Responsible Changed
From-To: sanpei->freebsd-bugs

I don't have enough time to handle this changes. 
(We need to check NetBSD changes hid.c rev.1.18, I think)
Comment 3 Anish Mistry 2004-08-09 20:35:13 UTC
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

The attached patch is a bit cleaner and closer with the NetBSD code.  This
really should be commit before the 5-STABLE branch because it'll fix a bunch
of non-working combo usb mice.  Probably is a good canidate for MFCing to
4-STABLE.
---
Anish Mistry
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.4 (FreeBSD)

iD8DBQFBF9H4xqA5ziudZT0RAvc3AKCx5JyRxCP5ZDeZFVp7FMqnVXo8tACfdqk6
U+4IFfL85qsxDqunNV5JfLs==RCpN
-----END PGP SIGNATURE-----
Comment 4 Will Saxon 2004-08-28 01:30:09 UTC
This patch from Anish Mistry works for me. Thanks so much.

-Will
Comment 5 christian.mueller 2004-08-29 11:02:35 UTC
I got a Logitech keyboard/mouse combo to work, with the patch of Mr. 
Mistry. Thanks again. It should be worthy to include the small fix into 
RELENG_5.
Comment 6 Mark Linimon freebsd_committer freebsd_triage 2004-11-04 07:27:10 UTC
Responsible Changed
From-To: freebsd-bugs->freebsd-usb

Reassign to appropriate mailing list.
Comment 7 John Baldwin freebsd_committer freebsd_triage 2004-11-17 14:59:20 UTC
So, I think that the first part of the patch (setting h.report_ID to zero) is 
not needed.  Basically, AFAICT, the effect of the patch is that 
hid_report_size() will now return the ID of the first device of a given kind 
rather than the last such device.  This seems to be a horrible interface 
fwiw, and while this hack might work, I think the real fix is to adopt some 
of the changes NetBSD made with their uhiddev.

-- 
John Baldwin <jhb@FreeBSD.org>  <><  http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve"  =  http://www.FreeBSD.org
Comment 8 Anish Mistry 2004-11-17 15:59:01 UTC
I've talked with Ian, and I'm working on getting this bit inline with that NetBSD has so we can put the fix in, and then work on the full HID import sync.
Comment 9 Nicola Vitale 2004-11-17 16:11:10 UTC
Hello,

> [John Baldwin, 2004-11-17T09:59:20-05::00]
> So, I think that the first part of the patch (setting h.report_ID to zero) is 
> not needed.  Basically, AFAICT, the effect of the patch is that 
> hid_report_size() will now return the ID of the first device of a given kind 
> rather than the last such device.

I'm agree with you. But I can't understand how this patch to hid_report_size()
can magically eliminate the bug in hid_is_collection(), which this PR originally
referred to.
The function hid_is_collection() never calls hid_report_size()
directly or indirectly.

Thanks, ciao.
-- 
Nicola S. Vitale
nivit@email.it
Comment 10 John Baldwin freebsd_committer freebsd_triage 2004-11-17 19:25:55 UTC
On Wednesday 17 November 2004 11:11 am, Nicola Vitale wrote:
> Hello,
>
> > [John Baldwin, 2004-11-17T09:59:20-05::00]
> > So, I think that the first part of the patch (setting h.report_ID to
> > zero) is not needed.  Basically, AFAICT, the effect of the patch is that
> > hid_report_size() will now return the ID of the first device of a given
> > kind rather than the last such device.
>
> I'm agree with you. But I can't understand how this patch to
> hid_report_size() can magically eliminate the bug in hid_is_collection(),
> which this PR originally referred to.
> The function hid_is_collection() never calls hid_report_size()
> directly or indirectly.

I think it affects the IDs that other drivers use (see uhid.c for example) and 
thus they use a "more correct" ID.

-- 
John Baldwin <jhb@FreeBSD.org>  <><  http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve"  =  http://www.FreeBSD.org
Comment 11 doswell 2005-03-30 05:51:02 UTC
Used the second mentioned patch to get a Logitech MX700 duo (wireless
mouse & keyboard with single 'shared' usb connection) working under
FreeBSD 5.3

Thanks,
Hope to see a final solution moved into releases!
Comment 12 ruben 2005-06-05 16:43:07 UTC
Hi, I want to report another success story.

The Logitech cordless desktop I own now works correctly under FreeBSD  
5.4-STABLE with the mentioned patch, love to see it committed to  
RELENG_5 in the near future.
Attaching/detaching works like a charm. Thanks for the fix people!

ukbd0: Logitech USB Receiver, rev 1.10/17.00, addr 2, iclass 3/1
kbd1 at ukbd0
ums0: Logitech USB Receiver, rev 1.10/17.00, addr 2, iclass 3/1
ums0: 7 buttons and Z dir.

Regards,
     Ruben
Comment 13 Mark Linimon freebsd_committer freebsd_triage 2008-02-12 09:34:24 UTC
State Changed
From-To: open->feedback

In the followup to kern/62323, someone mentions that this fix is no 
longer needed for FreeBSD 6.X and beyond.  Can the submitter confirm this?
Comment 14 Mark Linimon freebsd_committer freebsd_triage 2008-02-12 09:35:46 UTC
Responsible Changed
From-To: freebsd-usb->linimon

Track feedback.
Comment 15 Mark Linimon freebsd_committer freebsd_triage 2010-08-28 11:38:33 UTC
State Changed
From-To: feedback->closed

Feedback timeout ( > 2 years).