Hi Marcel, > -----Original Message----- > From: Marcel Holtmann [mailto:marcel@xxxxxxxxxxxx] > Sent: 2011年11月14日 17:32 > To: Yao, Costa > Cc: padovan@xxxxxxxxxxxxxx; linux-bluetooth@xxxxxxxxxxxxxxx > Subject: Re: [PATCH] Bluetooth: btusb: Use USB_INTERFACE_INFO to do device > matching > > Hi Costa, > > > 1 Use USB_INTERFACE_INFO to do device matching, other than > USB_DEVICE_INFO. > > 2 And we try to find the first interface containing the interrupt endpoint. > > > > Signed-off-by: Costa Yao <cqyao@xxxxxxxxxxxxxxxx> > > --- > > drivers/bluetooth/btusb.c | 16 +++++++++++----- > > 1 files changed, 11 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c > > index 9db2476..80d7e2d 100644 > > --- a/drivers/bluetooth/btusb.c > > +++ b/drivers/bluetooth/btusb.c > > @@ -58,7 +58,7 @@ static struct usb_driver btusb_driver; > > > > static struct usb_device_id btusb_table[] = { > > /* Generic Bluetooth USB device */ > > - { USB_DEVICE_INFO(0xe0, 0x01, 0x01) }, > > + { USB_INTERFACE_INFO(0xe0, 0x01, 0x01) }, > > > > /* Broadcom SoftSailing reporting vendor specific */ > > { USB_DEVICE(0x05ac, 0x21e1) }, > > this actually means that some of the USB_DEVICE entries can be removed. > Please go through our commit logs for btusb.c and figure out which ones can be > removed and send a separate patch for that. I have gone through commits about the adding of USB_DEVICE entry, and found as follow: For Broadcom SoftSailing reporting vendor specific entry, no detailed commit description. For Apple MacBookPro7,1 entry, its interface class is 0xff, so cannot be removed. For Apple iMac11,1 entry, its interface class is 0xff, so cannot be removed. For Apple MacBookPro6,2 entry, its interface class is 0xff, so cannot be removed. For Apple MacBookAir3,1, MacBookAir3,2 entry, its interface class is 0xff, so cannot be removed. For Apple MacBookAir4,1 entry, no detailed commit description. For Apple MacBookPro8,2 entry, no detailed commit description. For Apple MacMini5,1, no detailed commit description. For Ericsson entry, no commit description. For Canyon CN-BTU1 entry, no detailed commit description. For BCM20702A0 entry, its interface class is 0xff, so it cannot be removed. For "AVM BlueFRITZ", "Bluetooth Ulttraport Module from IBM", and "ALPS Modules with non-standard id" entries, no related commit description. > > @@ -915,9 +915,14 @@ static int btusb_probe(struct usb_interface > > *intf, > > > > BT_DBG("intf %p id %p", intf, id); > > > > - /* interface numbers are hardcoded in the spec */ > > - if (intf->cur_altsetting->desc.bInterfaceNumber != 0) > > + for (i = 0; i < intf->cur_altsetting->desc.bNumEndpoints; i++) { > > + ep_desc = &intf->cur_altsetting->endpoint[i].desc; > > + > > + if (usb_endpoint_is_int_in(ep_desc)) > > + break; > > + > > return -ENODEV; > > + } > > There is no point in walking the endpoint list twice in the probe function. Store > possible candidates for int_in, bulk_out and bulk_in descriptors so that later on > we can just assign them to our data. > > Then we can just check if this interface has int and proper bulk endpoints > available. And in error case return ENODEV. > > Actually thinking about it, we already do that, so just removing the hardcoded > check for desc.bInterfaceNumber != 0 should do the trick. > > > if (!id->driver_info) { > > const struct usb_device_id *match; > > @@ -1014,8 +1019,9 @@ static int btusb_probe(struct usb_interface > > *intf, > > > > hdev->owner = THIS_MODULE; > > > > - /* Interface numbers are hardcoded in the specification */ > > - data->isoc = usb_ifnum_to_if(data->udev, 1); > > + > > + data->isoc = usb_ifnum_to_if(data->udev, > > + 1+intf->cur_altsetting->desc.bInterfaceNumber); > > And here you need to to check all interfaces. Just assuming it is +1 might not be > good enough. Or at least put a proper comment in here that we expect the > ISOC interface to be +1. And then don't do 1+, I prefer to have > bInterfaceNumber + 1. > > Regards > > Marcel > ��.n��������+%������w��{.n�����{��� �^n�r������&��z�ޗ�zf���h���~����������_��+v���)ߣ�
