diff mbox series

[v5,1/2] Bluetooth: btusb: use usb vid/pid for initializing hal callbacks

Message ID 1601712179-13540-1-git-send-email-kiran.k@intel.com
State New
Headers show
Series [v5,1/2] Bluetooth: btusb: use usb vid/pid for initializing hal callbacks | expand

Commit Message

Kiran K Oct. 3, 2020, 8:02 a.m. UTC
For Intel controllers, use vid/pid for initalizing hardware abstraction
layer callbacks to avoid defining new quirk flags for new products.

Signed-off-by: Kiran K <kiran.k@intel.com>
---
Changes in v5:
* Use usb vid/pid combination to identify controller type istead of using flags

Changes in previous versions:
None. This is a new patch created part of v5

 drivers/bluetooth/btusb.c | 79 ++++++++++++++++++++++++++++-------------------
 1 file changed, 48 insertions(+), 31 deletions(-)

Comments

Marcel Holtmann Oct. 3, 2020, 8:27 a.m. UTC | #1
Hi Kiran,

> For Intel controllers, use vid/pid for initalizing hardware abstraction

> layer callbacks to avoid defining new quirk flags for new products.

> 

> Signed-off-by: Kiran K <kiran.k@intel.com>

> ---

> Changes in v5:

> * Use usb vid/pid combination to identify controller type istead of using flags

> 

> Changes in previous versions:

> None. This is a new patch created part of v5

> 

> drivers/bluetooth/btusb.c | 79 ++++++++++++++++++++++++++++-------------------

> 1 file changed, 48 insertions(+), 31 deletions(-)

> 

> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c

> index 1005b6e..e2cd78d 100644

> --- a/drivers/bluetooth/btusb.c

> +++ b/drivers/bluetooth/btusb.c

> @@ -3969,13 +3969,21 @@ static int btusb_probe(struct usb_interface *intf,

> 	init_usb_anchor(&data->ctrl_anchor);

> 	spin_lock_init(&data->rxlock);

> 

> -	if (id->driver_info & BTUSB_INTEL_NEW) {

> -		data->recv_event = btusb_recv_event_intel;

> -		data->recv_bulk = btusb_recv_bulk_intel;

> -		set_bit(BTUSB_BOOTLOADER, &data->flags);

> -	} else {

> -		data->recv_event = hci_recv_frame;

> -		data->recv_bulk = btusb_recv_bulk;

> +	data->recv_event = hci_recv_frame;

> +	data->recv_bulk = btusb_recv_bulk;

> +

> +	if (id->idVendor == 0x8087) {

> +		switch (id->idProduct) {

> +		case 0x0025:

> +		case 0x0026:

> +		case 0x0029:

> +		case 0x0a2b:

> +		case 0x0aaa:

> +			data->recv_event = btusb_recv_event_intel;

> +			data->recv_bulk = btusb_recv_bulk_intel;

> +			set_bit(BTUSB_BOOTLOADER, &data->flags);

> +			break;

> +		}

> 	}


please don’t do this. This is exactly what I didn’t want. Listing PID in the device table and then also again in the probe callback is bound to be error prone.

Regards

Marcel
Kiran K Oct. 3, 2020, 1:17 p.m. UTC | #2
Hi Marcel,

> -----Original Message-----

> From: Marcel Holtmann <marcel@holtmann.org>

> Sent: Saturday, October 3, 2020 1:57 PM

> To: Kiran K <kiraank@gmail.com>

> Cc: linux-bluetooth <linux-bluetooth@vger.kernel.org>; Narasimman, Sathish

> <sathish.narasimman@intel.com>; Tumkur Narayan, Chethan

> <chethan.tumkur.narayan@intel.com>; Srivatsa, Ravishankar

> <ravishankar.srivatsa@intel.com>; K, Kiran <kiran.k@intel.com>

> Subject: Re: [PATCH v5 1/2] Bluetooth: btusb: use usb vid/pid for initializing

> hal callbacks

> 

> Hi Kiran,

> 

> > For Intel controllers, use vid/pid for initalizing hardware

> > abstraction layer callbacks to avoid defining new quirk flags for new

> products.

> >

> > Signed-off-by: Kiran K <kiran.k@intel.com>

> > ---

> > Changes in v5:

> > * Use usb vid/pid combination to identify controller type istead of

> > using flags

> >

> > Changes in previous versions:

> > None. This is a new patch created part of v5

> >

> > drivers/bluetooth/btusb.c | 79

> > ++++++++++++++++++++++++++++-------------------

> > 1 file changed, 48 insertions(+), 31 deletions(-)

> >

> > diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c

> > index 1005b6e..e2cd78d 100644

> > --- a/drivers/bluetooth/btusb.c

> > +++ b/drivers/bluetooth/btusb.c

> > @@ -3969,13 +3969,21 @@ static int btusb_probe(struct usb_interface

> *intf,

> > 	init_usb_anchor(&data->ctrl_anchor);

> > 	spin_lock_init(&data->rxlock);

> >

> > -	if (id->driver_info & BTUSB_INTEL_NEW) {

> > -		data->recv_event = btusb_recv_event_intel;

> > -		data->recv_bulk = btusb_recv_bulk_intel;

> > -		set_bit(BTUSB_BOOTLOADER, &data->flags);

> > -	} else {

> > -		data->recv_event = hci_recv_frame;

> > -		data->recv_bulk = btusb_recv_bulk;

> > +	data->recv_event = hci_recv_frame;

> > +	data->recv_bulk = btusb_recv_bulk;

> > +

> > +	if (id->idVendor == 0x8087) {

> > +		switch (id->idProduct) {

> > +		case 0x0025:

> > +		case 0x0026:

> > +		case 0x0029:

> > +		case 0x0a2b:

> > +		case 0x0aaa:

> > +			data->recv_event = btusb_recv_event_intel;

> > +			data->recv_bulk = btusb_recv_bulk_intel;

> > +			set_bit(BTUSB_BOOTLOADER, &data->flags);

> > +			break;

> > +		}

> > 	}

> 

> please don’t do this. This is exactly what I didn’t want. Listing PID in the

> device table and then also again in the probe callback is bound to be error

> prone.

> 


Got your point. I will send the fix with BTUSB_INTEL_NEWGEN as discussed earlier.  Thanks.

> Regards

> 

> Marcel


Regards,
Kiran
diff mbox series

Patch

diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 1005b6e..e2cd78d 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -3969,13 +3969,21 @@  static int btusb_probe(struct usb_interface *intf,
 	init_usb_anchor(&data->ctrl_anchor);
 	spin_lock_init(&data->rxlock);
 
-	if (id->driver_info & BTUSB_INTEL_NEW) {
-		data->recv_event = btusb_recv_event_intel;
-		data->recv_bulk = btusb_recv_bulk_intel;
-		set_bit(BTUSB_BOOTLOADER, &data->flags);
-	} else {
-		data->recv_event = hci_recv_frame;
-		data->recv_bulk = btusb_recv_bulk;
+	data->recv_event = hci_recv_frame;
+	data->recv_bulk = btusb_recv_bulk;
+
+	if (id->idVendor == 0x8087) {
+		switch (id->idProduct) {
+		case 0x0025:
+		case 0x0026:
+		case 0x0029:
+		case 0x0a2b:
+		case 0x0aaa:
+			data->recv_event = btusb_recv_event_intel;
+			data->recv_bulk = btusb_recv_bulk_intel;
+			set_bit(BTUSB_BOOTLOADER, &data->flags);
+			break;
+		}
 	}
 
 	hdev = hci_alloc_dev();
@@ -4052,30 +4060,39 @@  static int btusb_probe(struct usb_interface *intf,
 		data->diag = usb_ifnum_to_if(data->udev, ifnum_base + 2);
 	}
 
-	if (id->driver_info & BTUSB_INTEL) {
-		hdev->manufacturer = 2;
-		hdev->setup = btusb_setup_intel;
-		hdev->shutdown = btusb_shutdown_intel;
-		hdev->set_diag = btintel_set_diag_mfg;
-		hdev->set_bdaddr = btintel_set_bdaddr;
-		hdev->cmd_timeout = btusb_intel_cmd_timeout;
-		set_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks);
-		set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks);
-		set_bit(HCI_QUIRK_NON_PERSISTENT_DIAG, &hdev->quirks);
-	}
-
-	if (id->driver_info & BTUSB_INTEL_NEW) {
-		hdev->manufacturer = 2;
-		hdev->send = btusb_send_frame_intel;
-		hdev->setup = btusb_setup_intel_new;
-		hdev->shutdown = btusb_shutdown_intel_new;
-		hdev->hw_error = btintel_hw_error;
-		hdev->set_diag = btintel_set_diag;
-		hdev->set_bdaddr = btintel_set_bdaddr;
-		hdev->cmd_timeout = btusb_intel_cmd_timeout;
-		set_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks);
-		set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks);
-		set_bit(HCI_QUIRK_NON_PERSISTENT_DIAG, &hdev->quirks);
+	if (id->idVendor == 0x8087) {
+		switch (id->idProduct) {
+		case 0x07dc:
+		case 0x0a2a:
+		case 0x0aa7:
+			hdev->manufacturer = 2;
+			hdev->setup = btusb_setup_intel;
+			hdev->shutdown = btusb_shutdown_intel;
+			hdev->set_diag = btintel_set_diag_mfg;
+			hdev->set_bdaddr = btintel_set_bdaddr;
+			hdev->cmd_timeout = btusb_intel_cmd_timeout;
+			set_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks);
+			set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks);
+			set_bit(HCI_QUIRK_NON_PERSISTENT_DIAG, &hdev->quirks);
+			break;
+		case 0x0025:
+		case 0x0026:
+		case 0x0029:
+		case 0x0a2b:
+		case 0x0aaa:
+			hdev->manufacturer = 2;
+			hdev->send = btusb_send_frame_intel;
+			hdev->setup = btusb_setup_intel_new;
+			hdev->shutdown = btusb_shutdown_intel_new;
+			hdev->hw_error = btintel_hw_error;
+			hdev->set_diag = btintel_set_diag;
+			hdev->set_bdaddr = btintel_set_bdaddr;
+			hdev->cmd_timeout = btusb_intel_cmd_timeout;
+			set_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks);
+			set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks);
+			set_bit(HCI_QUIRK_NON_PERSISTENT_DIAG, &hdev->quirks);
+			break;
+		}
 	}
 
 	if (id->driver_info & BTUSB_MARVELL)