Patchwork [Quantal,v2] UBUNTU: SAUCE: Bluetooth: use hci_send_cmd instead of usb_control_msg

login
register
mail settings
Submitter Jesse Sung
Date Sept. 6, 2013, 8:47 a.m.
Message ID <1378457253-32652-1-git-send-email-jesse.sung@canonical.com>
Download mbox | patch
Permalink /patch/273123/
State New
Headers show

Comments

Jesse Sung - Sept. 6, 2013, 8:47 a.m.
From: Wen-chien Jesse Sung <jesse.sung@canonical.com>

BugLink: https://launchpad.net/bugs/1065400

Calling usb_control_msg directly to send firmware file does not work
with new platforms. Use hci_send_cmd instead.

Signed-off-by: Wen-chien Jesse Sung <jesse.sung@canonical.com>
---
 drivers/bluetooth/btusb.c | 60 +++++++++++++++++++++++------------------------
 net/bluetooth/hci_core.c  |  1 +
 2 files changed, 31 insertions(+), 30 deletions(-)
Tim Gardner - Sept. 6, 2013, 7:38 p.m.

Andy Whitcroft - Sept. 9, 2013, 9:39 a.m.
On Fri, Sep 06, 2013 at 04:47:33PM +0800, Jesse Sung wrote:
> From: Wen-chien Jesse Sung <jesse.sung@canonical.com>
> 
> BugLink: https://launchpad.net/bugs/1065400
> 
> Calling usb_control_msg directly to send firmware file does not work
> with new platforms. Use hci_send_cmd instead.
> 
> Signed-off-by: Wen-chien Jesse Sung <jesse.sung@canonical.com>
> ---
>  drivers/bluetooth/btusb.c | 60 +++++++++++++++++++++++------------------------
>  net/bluetooth/hci_core.c  |  1 +
>  2 files changed, 31 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index db16ce3..1f77d40 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -57,7 +57,7 @@ static struct usb_driver btusb_driver;
>  #define BTUSB_BROKEN_ISOC	0x20
>  #define BTUSB_WRONG_SCO_MTU	0x40
>  #define BTUSB_ATH3012		0x80
> -#define BTUSB_BCM_PATCHRAM	0x100
> +#define BTUSB_BCM_PATCHRAM	0x800
>  
>  static struct usb_device_id btusb_table[] = {
>  	/* Generic Bluetooth USB device */
> @@ -234,14 +234,13 @@ static struct usb_device_id blacklist_table[] = {
>  #define BTUSB_ISOC_RUNNING	2
>  #define BTUSB_SUSPENDING	3
>  #define BTUSB_DID_ISO_RESUME	4
> -#define BTUSB_FIRMWARE_DONE	5
> +#define BTUSB_FIRMWARE_DONE	7
>  
>  struct btusb_data {
>  	struct hci_dev       *hdev;
>  	struct usb_device    *udev;
>  	struct usb_interface *intf;
>  	struct usb_interface *isoc;
> -	const struct usb_device_id *id;
>  
>  	spinlock_t lock;
>  
> @@ -946,24 +945,20 @@ static void btusb_waker(struct work_struct *work)
>  	usb_autopm_put_interface(data->intf);
>  }
>  
> -#define PATCHRAM_TIMEOUT	1000
>  #define PATCHRAM_NAME_LEN	20
> +#define PATCHRAM_BUF_SIZE	(256 + 4)
>  
>  static void btusb_load_firmware(struct hci_dev *hdev)
>  {
>  	struct btusb_data *data = hci_get_drvdata(hdev);
>  	struct usb_device *udev = data->udev;
> -	const struct usb_device_id *id = data->id;
>  	size_t pos = 0;
>  	int err = 0;
>  	char filename[PATCHRAM_NAME_LEN];
>  	const struct firmware *fw;
> +	u8 *buf = NULL;
> +	u8 val = 0;
>  
> -	unsigned char reset_cmd[] = { 0x03, 0x0c, 0x00 };
> -	unsigned char download_cmd[] = { 0x2e, 0xfc, 0x00 };
> -
> -	if (!(id->driver_info & BTUSB_BCM_PATCHRAM))
> -		return;
>  	if (test_and_set_bit(BTUSB_FIRMWARE_DONE, &data->flags))
>  		return;
>  
> @@ -975,38 +970,43 @@ static void btusb_load_firmware(struct hci_dev *hdev)
>  		return;
>  	}
>  
> -	if (usb_control_msg(udev, usb_sndctrlpipe(udev, 0), 0, USB_TYPE_CLASS, 0, 0,
> -		reset_cmd, sizeof(reset_cmd), PATCHRAM_TIMEOUT) < 0) {
> -		err = -1;
> +	buf = kmalloc(PATCHRAM_BUF_SIZE, GFP_KERNEL);
> +	if (!buf)
>  		goto out;
> -	}
> -	msleep(300);
>  
> -	if (usb_control_msg(udev, usb_sndctrlpipe(udev, 0), 0, USB_TYPE_CLASS, 0, 0,
> -		download_cmd, sizeof(download_cmd), PATCHRAM_TIMEOUT) < 0) {
> -		err = -1;
> +	err= hci_send_cmd(hdev, 0x0c03, 1, &val);
> +	if (err)
>  		goto out;
> -	}
>  	msleep(300);
>  
> +	err = hci_send_cmd(hdev, 0xfc2e, 1, &val);
> +	if (err)
> +		goto out;
> +	msleep(1000);

Gah I assume it is "normal" to just use these gratuitous hex numbers in
this kind of code?

> +
>  	while (pos < fw->size) {
>  		size_t len;
>  		len = fw->data[pos + 2] + 3;
> -		if ((pos + len > fw->size) ||
> -			(usb_control_msg(udev, usb_sndctrlpipe(udev, 0), 0,
> -			USB_TYPE_CLASS, 0, 0, (void *)fw->data + pos, len,
> -			PATCHRAM_TIMEOUT) < 0)) {
> -			err = -1;
> +		if (pos + len > fw->size) {
> +			err = -EINVAL;
>  			goto out;
>  		}
> +		memcpy(buf, fw->data + pos, len);
> +		err = hci_send_cmd(hdev, le16_to_cpu(*(u16*)buf),
> +					*(buf + sizeof(u16)), buf + sizeof(u16) + 1);
> +		if (err)
> +			goto out;
>  		pos += len;
>  	}
>  
> -	err = (usb_control_msg(udev, usb_sndctrlpipe(udev, 0), 0, USB_TYPE_CLASS, 0, 0,
> -		reset_cmd, sizeof(reset_cmd), PATCHRAM_TIMEOUT) < 0);
> +	err = hci_send_cmd(hdev, 0x0c03, 1, &val);
>  out:
>  	if (err)
> -		BT_INFO("fail to load firmware, may not work correctly");
> +		BT_INFO("fail to load firmware");
> +	else
> +		BT_INFO("firmware loaded");
> +	if (buf)
> +		kfree(buf);
>  	release_firmware(fw);
>  }
>  
> @@ -1097,8 +1097,6 @@ static int btusb_probe(struct usb_interface *intf,
>  	init_usb_anchor(&data->isoc_anchor);
>  	init_usb_anchor(&data->deferred);
>  
> -	data->id = id;
> -
>  	hdev = hci_alloc_dev();
>  	if (!hdev) {
>  		kfree(data);
> @@ -1117,7 +1115,9 @@ static int btusb_probe(struct usb_interface *intf,
>  	hdev->flush    = btusb_flush;
>  	hdev->send     = btusb_send_frame;
>  	hdev->notify   = btusb_notify;
> -	hdev->load_firmware = btusb_load_firmware;
> +
> +	if (id->driver_info & BTUSB_BCM_PATCHRAM)
> +		hdev->load_firmware = btusb_load_firmware;
>  
>  	/* Interface numbers are hardcoded in the specification */
>  	data->isoc = usb_ifnum_to_if(data->udev, 1);
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 4e53850..d5298c6 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -2134,6 +2134,7 @@ int hci_send_cmd(struct hci_dev *hdev, __u16 opcode, __u32 plen, void *param)
>  
>  	return 0;
>  }
> +EXPORT_SYMBOL(hci_send_cmd);
>  
>  /* Get data from the previously sent command */
>  void *hci_sent_cmd_data(struct hci_dev *hdev, __u16 opcode)

On the assumption that this has been tested with old and new devices and
with those without PATCHRAM requirements.  I guess:

Acked-by: Andy Whitcroft <apw@canonical.com>

-apw
Andy Whitcroft - Sept. 9, 2013, 9:43 a.m.
Applied to Quantal.

-apw
Andy Whitcroft - Sept. 9, 2013, 9:47 a.m.
Applied to Quantal.

-apw

Patch

diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index db16ce3..1f77d40 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -57,7 +57,7 @@  static struct usb_driver btusb_driver;
 #define BTUSB_BROKEN_ISOC	0x20
 #define BTUSB_WRONG_SCO_MTU	0x40
 #define BTUSB_ATH3012		0x80
-#define BTUSB_BCM_PATCHRAM	0x100
+#define BTUSB_BCM_PATCHRAM	0x800
 
 static struct usb_device_id btusb_table[] = {
 	/* Generic Bluetooth USB device */
@@ -234,14 +234,13 @@  static struct usb_device_id blacklist_table[] = {
 #define BTUSB_ISOC_RUNNING	2
 #define BTUSB_SUSPENDING	3
 #define BTUSB_DID_ISO_RESUME	4
-#define BTUSB_FIRMWARE_DONE	5
+#define BTUSB_FIRMWARE_DONE	7
 
 struct btusb_data {
 	struct hci_dev       *hdev;
 	struct usb_device    *udev;
 	struct usb_interface *intf;
 	struct usb_interface *isoc;
-	const struct usb_device_id *id;
 
 	spinlock_t lock;
 
@@ -946,24 +945,20 @@  static void btusb_waker(struct work_struct *work)
 	usb_autopm_put_interface(data->intf);
 }
 
-#define PATCHRAM_TIMEOUT	1000
 #define PATCHRAM_NAME_LEN	20
+#define PATCHRAM_BUF_SIZE	(256 + 4)
 
 static void btusb_load_firmware(struct hci_dev *hdev)
 {
 	struct btusb_data *data = hci_get_drvdata(hdev);
 	struct usb_device *udev = data->udev;
-	const struct usb_device_id *id = data->id;
 	size_t pos = 0;
 	int err = 0;
 	char filename[PATCHRAM_NAME_LEN];
 	const struct firmware *fw;
+	u8 *buf = NULL;
+	u8 val = 0;
 
-	unsigned char reset_cmd[] = { 0x03, 0x0c, 0x00 };
-	unsigned char download_cmd[] = { 0x2e, 0xfc, 0x00 };
-
-	if (!(id->driver_info & BTUSB_BCM_PATCHRAM))
-		return;
 	if (test_and_set_bit(BTUSB_FIRMWARE_DONE, &data->flags))
 		return;
 
@@ -975,38 +970,43 @@  static void btusb_load_firmware(struct hci_dev *hdev)
 		return;
 	}
 
-	if (usb_control_msg(udev, usb_sndctrlpipe(udev, 0), 0, USB_TYPE_CLASS, 0, 0,
-		reset_cmd, sizeof(reset_cmd), PATCHRAM_TIMEOUT) < 0) {
-		err = -1;
+	buf = kmalloc(PATCHRAM_BUF_SIZE, GFP_KERNEL);
+	if (!buf)
 		goto out;
-	}
-	msleep(300);
 
-	if (usb_control_msg(udev, usb_sndctrlpipe(udev, 0), 0, USB_TYPE_CLASS, 0, 0,
-		download_cmd, sizeof(download_cmd), PATCHRAM_TIMEOUT) < 0) {
-		err = -1;
+	err= hci_send_cmd(hdev, 0x0c03, 1, &val);
+	if (err)
 		goto out;
-	}
 	msleep(300);
 
+	err = hci_send_cmd(hdev, 0xfc2e, 1, &val);
+	if (err)
+		goto out;
+	msleep(1000);
+
 	while (pos < fw->size) {
 		size_t len;
 		len = fw->data[pos + 2] + 3;
-		if ((pos + len > fw->size) ||
-			(usb_control_msg(udev, usb_sndctrlpipe(udev, 0), 0,
-			USB_TYPE_CLASS, 0, 0, (void *)fw->data + pos, len,
-			PATCHRAM_TIMEOUT) < 0)) {
-			err = -1;
+		if (pos + len > fw->size) {
+			err = -EINVAL;
 			goto out;
 		}
+		memcpy(buf, fw->data + pos, len);
+		err = hci_send_cmd(hdev, le16_to_cpu(*(u16*)buf),
+					*(buf + sizeof(u16)), buf + sizeof(u16) + 1);
+		if (err)
+			goto out;
 		pos += len;
 	}
 
-	err = (usb_control_msg(udev, usb_sndctrlpipe(udev, 0), 0, USB_TYPE_CLASS, 0, 0,
-		reset_cmd, sizeof(reset_cmd), PATCHRAM_TIMEOUT) < 0);
+	err = hci_send_cmd(hdev, 0x0c03, 1, &val);
 out:
 	if (err)
-		BT_INFO("fail to load firmware, may not work correctly");
+		BT_INFO("fail to load firmware");
+	else
+		BT_INFO("firmware loaded");
+	if (buf)
+		kfree(buf);
 	release_firmware(fw);
 }
 
@@ -1097,8 +1097,6 @@  static int btusb_probe(struct usb_interface *intf,
 	init_usb_anchor(&data->isoc_anchor);
 	init_usb_anchor(&data->deferred);
 
-	data->id = id;
-
 	hdev = hci_alloc_dev();
 	if (!hdev) {
 		kfree(data);
@@ -1117,7 +1115,9 @@  static int btusb_probe(struct usb_interface *intf,
 	hdev->flush    = btusb_flush;
 	hdev->send     = btusb_send_frame;
 	hdev->notify   = btusb_notify;
-	hdev->load_firmware = btusb_load_firmware;
+
+	if (id->driver_info & BTUSB_BCM_PATCHRAM)
+		hdev->load_firmware = btusb_load_firmware;
 
 	/* Interface numbers are hardcoded in the specification */
 	data->isoc = usb_ifnum_to_if(data->udev, 1);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 4e53850..d5298c6 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2134,6 +2134,7 @@  int hci_send_cmd(struct hci_dev *hdev, __u16 opcode, __u32 plen, void *param)
 
 	return 0;
 }
+EXPORT_SYMBOL(hci_send_cmd);
 
 /* Get data from the previously sent command */
 void *hci_sent_cmd_data(struct hci_dev *hdev, __u16 opcode)