Patchwork [Raring,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 <1378457229-30385-1-git-send-email-jesse.sung@canonical.com>
Download mbox | patch
Permalink /patch/273122/
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 | 58 +++++++++++++++++++++++------------------------
 net/bluetooth/hci_core.c  |  1 +
 2 files changed, 29 insertions(+), 30 deletions(-)
Andy Whitcroft - Sept. 6, 2013, 10:49 a.m.
On Fri, Sep 06, 2013 at 04:47:09PM +0800, Jesse Sung wrote:
> From: Wen-chien Jesse Sung <jesse.sung@canonical.com>
> 
> BugLink: https://launchpad.net/bugs/1065400

This bug looks to have been 'closed' with fixed released before, does
this imply that those fixes did not fix things, or that this is a new
problem introduced by those, or a separate but related problem.  I am
wondering if we should be leaving the old bug fixed as it did fix things
for the original people there:

    "Thank you Jesse very much!, my BT is working again :-)"

and making a new one as the continuation for the new issues?  Thoughts?

> 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>

This patch, is this a combination backport of something specific or new
code from yourself?  I think the attribution above imples it is yours
alone?  Just want to confirm.  If it is a backport of something then it
would be good to know what for comparison.

> ---
>  drivers/bluetooth/btusb.c | 58 +++++++++++++++++++++++------------------------
>  net/bluetooth/hci_core.c  |  1 +
>  2 files changed, 29 insertions(+), 30 deletions(-)

-apw
Jesse Sung - Sept. 6, 2013, 4:47 p.m.
2013/9/6 Andy Whitcroft <apw@canonical.com>:
> On Fri, Sep 06, 2013 at 04:47:09PM +0800, Jesse Sung wrote:
>> From: Wen-chien Jesse Sung <jesse.sung@canonical.com>
>>
>> BugLink: https://launchpad.net/bugs/1065400
>
> This bug looks to have been 'closed' with fixed released before, does
> this imply that those fixes did not fix things, or that this is a new
> problem introduced by those, or a separate but related problem.  I am
> wondering if we should be leaving the old bug fixed as it did fix things
> for the original people there:
>
>     "Thank you Jesse very much!, my BT is working again :-)"
>
> and making a new one as the continuation for the new issues?  Thoughts?

The previous fixes does not work on new platforms (eg. lynx poing lp).
That's why this bug is 'fix released' previously. Those fixes did work then.

Dunno whether reusing this bug or creating a new one is better for
this situation.
I'd like to keep all these patchram related info together, so I'm reusing it.
If using a new bug is preferred, please let me know, I'll create one.

>> 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>
>
> This patch, is this a combination backport of something specific or new
> code from yourself?  I think the attribution above imples it is yours
> alone?  Just want to confirm.  If it is a backport of something then it
> would be good to know what for comparison.

No, it's not a backport. Just as what the only SOB line implies, it is
mine alone. :-)

Thanks,
Jesse
Tim Gardner - Sept. 6, 2013, 7:38 p.m.

Andy Whitcroft - Sept. 9, 2013, 9:40 a.m.
On Fri, Sep 06, 2013 at 04:47:09PM +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 | 58 +++++++++++++++++++++++------------------------
>  net/bluetooth/hci_core.c  |  1 +
>  2 files changed, 29 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index 5be9002..146198d 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -49,7 +49,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 */
> @@ -226,14 +226,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;
>  
> @@ -938,24 +937,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;
>  
> @@ -967,40 +962,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);
>  }
>  
> @@ -1089,8 +1087,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)
>  		return -ENOMEM;
> @@ -1107,7 +1103,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 843f46f..927e55e 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -2193,6 +2193,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)

"ditto"

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

-apw
Andy Whitcroft - Sept. 9, 2013, 9:44 a.m.
Applied to Raring.

-apw

Patch

diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 5be9002..146198d 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -49,7 +49,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 */
@@ -226,14 +226,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;
 
@@ -938,24 +937,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;
 
@@ -967,40 +962,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);
 }
 
@@ -1089,8 +1087,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)
 		return -ENOMEM;
@@ -1107,7 +1103,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 843f46f..927e55e 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2193,6 +2193,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)