Bluetooth: Add support for Intel Bluetooth device [8087:07dc]

PATCH Superseded Tedd Ho-Jeong An 3 participants 7 messages Apr 17, 2013 – Apr 18, 2013 Bluetooth drivers

lore

Share & tools

lore

Tedd Ho-Jeong An

author

2013-04-17 18:47 UTC #0 lore raw

PATCH v4 1/1 Bluetooth: Add support for Intel Bluetooth device [8087:07dc]

From: Tedd Ho-Jeong An <[email protected]>

This patch adds support for Intel Bluetooth device by adding btusb_setup_intel() routine that update the device with ROM patch.

T:  Bus=02 Lev=02 Prnt=02 Port=00 Cnt=01 Dev#=  4 Spd=12   MxCh= 0
D:  Ver= 2.00 Cls=e0(wlcon) Sub=01 Prot=01 MxPS=64 #Cfgs=  1
P:  Vendor=8087 ProdID=07dc Rev= 0.01
C:* #Ifs= 2 Cfg#= 1 Atr=e0 MxPwr=100mA
I:* If#= 0 Alt= 0 #EPs= 3 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=81(I) Atr=03(Int.) MxPS=  64 Ivl=1ms
E:  Ad=02(O) Atr=02(Bulk) MxPS=  64 Ivl=0ms
E:  Ad=82(I) Atr=02(Bulk) MxPS=  64 Ivl=0ms
I:* If#= 1 Alt= 0 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=03(O) Atr=01(Isoc) MxPS=   0 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=   0 Ivl=1ms
I:  If#= 1 Alt= 1 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=03(O) Atr=01(Isoc) MxPS=   9 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=   9 Ivl=1ms
I:  If#= 1 Alt= 2 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=03(O) Atr=01(Isoc) MxPS=  17 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=  17 Ivl=1ms
I:  If#= 1 Alt= 3 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=03(O) Atr=01(Isoc) MxPS=  25 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=  25 Ivl=1ms
I:  If#= 1 Alt= 4 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=03(O) Atr=01(Isoc) MxPS=  33 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=  33 Ivl=1ms
I:  If#= 1 Alt= 5 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=03(O) Atr=01(Isoc) MxPS=  49 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=  49 Ivl=1ms

Signed-off-by Tedd Ho-Jeong An <[email protected]>

---
v4 changes
- refactored patching routin to separate function
- changed varaible type for status to u8
- corrected minor format issues

 drivers/bluetooth/btusb.c |  366 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 366 insertions(+)

drivers/bluetooth/btusb.c +366 -0

index 3d684d2..52c0ec0 100644
@@ -23,6 +23,7 @@
 
 #include <linux/module.h>
 #include <linux/usb.h>
+#include <linux/firmware.h>
 
 #include <net/bluetooth/bluetooth.h>
 #include <net/bluetooth/hci_core.h>

@@ -47,6 +48,7 @@ static struct usb_driver btusb_driver;
 #define BTUSB_BROKEN_ISOC	0x20
 #define BTUSB_WRONG_SCO_MTU	0x40
 #define BTUSB_ATH3012		0x80
+#define BTUSB_INTEL		0x100
 
 static struct usb_device_id btusb_table[] = {
 	/* Generic Bluetooth USB device */

@@ -207,6 +209,9 @@ static struct usb_device_id blacklist_table[] = {
 	/* Frontline ComProbe Bluetooth Sniffer */
 	{ USB_DEVICE(0x16d3, 0x0002), .driver_info = BTUSB_SNIFFER },
 
+	/* Intel Bluetooth device */
+	{ USB_DEVICE(0x8087, 0x07dc), .driver_info = BTUSB_INTEL },
+
 	{ }	/* Terminating entry */
 };

@@ -943,6 +948,364 @@ static int btusb_setup_bcm92035(struct hci_dev *hdev)
 	return 0;
 }
 
+struct intel_version {
+	u8	status;
+	u8	hw_platform;
+	u8	hw_variant;
+	u8	hw_revision;
+	u8	fw_variant;
+	u8	fw_revision;
+	u8	fw_build_num;
+	u8	fw_build_ww;
+	u8	fw_build_yy;
+	u8	fw_patch_num;
+} __packed;
+
+static const struct firmware *btusb_setup_intel_get_fw(struct hci_dev *hdev,
+						struct intel_version *ver)
+{
+	const struct firmware *fw;
+	char fwname[64];
+	int ret;
+
+	snprintf(fwname, sizeof(fwname),
+		 "intel/ibt-hw-%x.%x.%x-fw-%x.%x.%x.%x.%x.bseq",
+		 ver->hw_platform, ver->hw_variant, ver->hw_revision,
+		 ver->fw_variant,  ver->fw_revision, ver->fw_build_num,
+		 ver->fw_build_ww, ver->fw_build_yy);
+
+	ret = request_firmware(&fw, fwname, &hdev->dev);
+	if (ret < 0) {
+		if (ret == -EINVAL) {
+			BT_ERR("%s Intel firmware file request failed (%d)",
+			       hdev->name, ret);
+			return NULL;
+		}
+
+		BT_ERR("%s failed to open Intel firmware file: %s(%d)",
+		       hdev->name, fwname, ret);
+
+		snprintf(fwname, sizeof(fwname), "intel/ibt-default-%x.%x.bseq",
+			 ver->hw_platform, ver->hw_variant);
+		if (request_firmware(&fw, fwname, &hdev->dev) < 0) {
+			BT_ERR("%s failed to open default Intel fw file: %s",
+			       hdev->name, fwname);
+			return NULL;
+		}
+	}
+
+	BT_INFO("%s Intel Bluetooth firmware file: %s", hdev->name, fwname);
+
+	return fw;
+}
+
+static int btusb_setup_intel_patching(struct hci_dev *hdev,
+				      const struct firmware *fw,
+				      const u8 **curr_fw_ptr,
+				      int *disable_patch)
+{
+	struct sk_buff *skb;
+	struct hci_command_hdr *cmd;
+	const u8 *cmd_param;
+	struct hci_event_hdr *evt = NULL;
+	const u8 *evt_param = NULL;
+	const u8 *fw_ptr = *curr_fw_ptr;
+	int remain = fw->size - (fw_ptr - fw->data);
+
+	/* Read command */
+	if (remain > HCI_COMMAND_HDR_SIZE &&
+	    fw_ptr[0] != 0x01) {
+		BT_ERR("%s Intel fw corrupted: invalid cmd read", hdev->name);
+		return -1;
+	}
+	fw_ptr++;
+	remain--;
+
+	cmd = (struct hci_command_hdr *)fw_ptr;
+	fw_ptr += sizeof(*cmd);
+	remain -= sizeof(*cmd);
+
+	/* checking the length */
+	if (remain < cmd->plen) {
+		BT_ERR("%s Intel fw corrupted: invalid cmd len", hdev->name);
+		return -1;
+	}
+
+	/* If there is a command that loads a patch in the firmware
+	 * file, then enable the patch upon success, otherwise just
+	 * disable the manufacturer mode, for example patch activation
+	 * is not required when the default firmware patch file is used
+	 * because there are no patch data to load.
+	 */
+	if (*disable_patch && cmd->opcode == 0xfc8e)
+		*disable_patch = 0;
+
+	cmd_param = fw_ptr;
+	fw_ptr += cmd->plen;
+	remain -= cmd->plen;
+
+	/* Read event */
+	while (remain > HCI_EVENT_HDR_SIZE && fw_ptr[0] == 0x02) {
+		fw_ptr++;
+		remain--;
+
+		evt = (struct hci_event_hdr *)fw_ptr;
+		fw_ptr += sizeof(*evt);
+		remain -= sizeof(*evt);
+
+		if (remain < evt->plen) {
+			BT_ERR("%s Intel fw corrupted: invalid evt len",
+			       hdev->name);
+			return -1;
+		}
+
+		evt_param = fw_ptr;
+		fw_ptr += evt->plen;
+		remain -= evt->plen;
+	}
+
+	if (!evt || !evt_param || remain < 0) {
+		BT_ERR("%s Intel fw corrupted: invalid evt read", hdev->name);
+		return -1;
+	}
+
+	skb = __hci_cmd_sync_ev(hdev, le16_to_cpu(cmd->opcode),
+				cmd->plen, (void *)cmd_param, evt->evt,
+				HCI_INIT_TIMEOUT);
+	if (IS_ERR(skb)) {
+		BT_ERR("%s sending Intel patch command failed (%ld)",
+		       hdev->name, PTR_ERR(skb));
+		return -1;
+	}
+
+	/* Checking the returned event */
+	if (skb->len != evt->plen) {
+		BT_ERR("%s mismatch event length", hdev->name);
+		kfree_skb(skb);
+		return -1;
+	}
+
+	if (memcmp(skb->data, evt_param, evt->plen)) {
+		BT_ERR("%s mismatch event parameter", hdev->name);
+		kfree_skb(skb);
+		return -1;
+	}
+	kfree_skb(skb);
+
+	*curr_fw_ptr = fw_ptr;
+
+	return 0;
+}
+
+static int btusb_setup_intel(struct hci_dev *hdev)
+{
+	struct sk_buff *skb;
+	const struct firmware *fw;
+	const u8 *fw_ptr;
+	int disable_patch;
+	struct intel_version *ver;
+
+	u8 mfg_enable[] = { 0x01, 0x00 };
+	u8 mfg_disable[] = { 0x00, 0x00 };
+	u8 mfg_reset_deactivate[] = { 0x00, 0x01 };
+	u8 mfg_reset_activate[] = { 0x00, 0x02 };
+
+	BT_DBG("%s", hdev->name);
+
+	/* The controller has a bug with the first HCI command send to it
+	 * returning number of completed commands as zero. This would stall the
+	 * command processing in the Bluetooth core
+	 *
+	 * As a workaround, send HCI Reset command first which will reset the
+	 * number of completed commands and allow normal command processing
+	 * from now on
+	 */
+	skb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL, HCI_INIT_TIMEOUT);
+	if (IS_ERR(skb)) {
+		BT_ERR("%s sending initial HCI reset command failed (%ld)",
+		       hdev->name, PTR_ERR(skb));
+		return -PTR_ERR(skb);
+	}
+	kfree_skb(skb);
+
+	/* Read Intel specific controller version first to allow selection of
+	 * which firmware file to load.
+	 *
+	 * The returned information are hardware variant and revision plus
+	 * firmware variant, revision and build number.
+	 */
+	skb = __hci_cmd_sync(hdev, 0xfc05, 0, NULL, HCI_INIT_TIMEOUT);
+	if (IS_ERR(skb)) {
+		BT_ERR("%s reading Intel fw version command failed (%ld)",
+		       hdev->name, PTR_ERR(skb));
+		return -PTR_ERR(skb);
+	}
+
+	if (skb->len != sizeof(*ver)) {
+		BT_ERR("%s Intel version event length mismatch", hdev->name);
+		kfree_skb(skb);
+		return 0;
+	}
+
+	ver = (struct intel_version *)skb->data;
+	/* was command successful */
+	if (ver->status) {
+		BT_ERR("%s Intel fw version event failed (%02x)", hdev->name,
+		       ver->status);
+		kfree_skb(skb);
+		return 0;
+	}
+
+	BT_INFO("%s read Intel version: %02x%02x%02x%02x%02x%02x%02x%02x%02x",
+		hdev->name, ver->hw_platform, ver->hw_variant,
+		ver->hw_revision, ver->fw_variant,  ver->fw_revision,
+		ver->fw_build_num, ver->fw_build_ww, ver->fw_build_yy,
+		ver->fw_patch_num);
+
+	/* is the device already patched */
+	if (ver->fw_patch_num) {
+		BT_INFO("%s Intel device is already patched. patch num: %02x",
+			hdev->name, ver->fw_patch_num);
+		kfree_skb(skb);
+		return 0;
+	}
+
+	/* Opens the firmware patch file based on the firmware version read
+	 * from the controller. If it failed to open the matching firmware
+	 * patch file, it tries to open the default firmware patch file.
+	 * If no patch file, allow the device to operate without a patch.
+	 */
+	fw = btusb_setup_intel_get_fw(hdev, ver);
+	if (!fw) {
+		kfree_skb(skb);
+		return 0;
+	}
+	fw_ptr = fw->data;
+
+	/* This Intel specific command enables the manufacturer mode of the
+	 * controller.
+	 *
+	 * Only while this mode is enabled, the driver can download the
+	 * firmware patch data and configuration parameters.
+	 */
+	skb = __hci_cmd_sync(hdev, 0xfc11, 2, mfg_enable, HCI_INIT_TIMEOUT);
+	if (IS_ERR(skb)) {
+		BT_ERR("%s entering Intel manufacturer mode failed (%ld)",
+		       hdev->name, PTR_ERR(skb));
+		release_firmware(fw);
+		return -PTR_ERR(skb);
+	}
+
+	if (skb->data[0]) {
+		u8 evt_status = skb->data[0];
+		BT_ERR("%s enable Intel manufacturer mode event failed (%02x)",
+		       hdev->name, evt_status);
+		kfree_skb(skb);
+		release_firmware(fw);
+		return -bt_to_errno(evt_status);
+	}
+	kfree_skb(skb);
+
+	disable_patch = 1;
+
+	/* The firmware data file consists of list of Intel specific HCI
+	 * commands and its expected events. The first byte indicates the
+	 * type of the message, either HCI command or HCI event.
+	 *
+	 * It reads the command and its expected event from the firmware file,
+	 * and send to the controller. Once __hci_cmd_sync_ev() returns,
+	 * the returned event is compared with the event read from the firmware
+	 * file and it will continue until all the messages are downloaded to
+	 * the controller.
+	 *
+	 * Once the firmware patching is completed successfully,
+	 * the manufacturer mode is disabled with reset and activating the
+	 * downloaded patch.
+	 *
+	 * If the firmware patching is failed, the manufacturer mode is
+	 * disabled with reset and deactivating the patch.
+	 *
+	 * If the default patch file is used, no reset is done when disabling
+	 * the manufacturer.
+	 */
+	while (1) {
+		int ret;
+
+		ret = btusb_setup_intel_patching(hdev, fw, &fw_ptr,
+						 &disable_patch);
+		if (ret < 0)
+			goto exit_mfg_deactivate;
+
+		/* Checking if EOF */
+		if (fw->size == fw_ptr - fw->data) {
+			if (disable_patch)
+				goto exit_mfg_disable;
+			else
+				goto exit_mfg_activate;
+		} else if (fw->size < fw_ptr - fw->data) {
+			BT_ERR("%s inconsistent patch read size", hdev->name);
+			goto exit_mfg_deactivate;
+		}
+	}
+
+exit_mfg_activate:
+	release_firmware(fw);
+
+	/* Reset is done when disabling the manufacturer mode and activate
+	 * the downloaded firmware patches
+	 */
+	skb = __hci_cmd_sync(hdev, 0xfc11, 2, mfg_reset_activate,
+			     HCI_INIT_TIMEOUT);
+	if (IS_ERR(skb)) {
+		BT_ERR("%s exiting Intel manufacturer mode failed (%ld)",
+		       hdev->name, PTR_ERR(skb));
+		return -PTR_ERR(skb);
+	}
+	kfree_skb(skb);
+
+	BT_INFO("%s Intel Bluetooth firmware patch completed and activated",
+		hdev->name);
+
+	return 0;
+
+exit_mfg_deactivate:
+	release_firmware(fw);
+
+	/* Reset is done when disabling the manufacturer mode and deactivate
+	 * the downloaded firmware patches.
+	 */
+	skb = __hci_cmd_sync(hdev, 0xfc11, 2, mfg_reset_deactivate,
+			     HCI_INIT_TIMEOUT);
+	if (IS_ERR(skb)) {
+		BT_ERR("%s exiting Intel manufacturer mode failed (%ld)",
+		       hdev->name, PTR_ERR(skb));
+		return -PTR_ERR(skb);
+	}
+	kfree_skb(skb);
+
+	BT_INFO("%s Intel Bluetooth firmware patch completed and deactivated",
+		hdev->name);
+
+	return 0;
+
+exit_mfg_disable:
+	release_firmware(fw);
+
+	/* No reset is done when disabling the manufacturer mode */
+	skb = __hci_cmd_sync(hdev, 0xfc11, 2, mfg_disable,
+			     HCI_INIT_TIMEOUT);
+	if (IS_ERR(skb)) {
+		BT_ERR("%s exiting Intel manufacturer mode failed (%ld)",
+		       hdev->name, PTR_ERR(skb));
+		return -PTR_ERR(skb);
+	}
+	kfree_skb(skb);
+
+	BT_INFO("%s Intel Bluetooth firmware patch completed", hdev->name);
+
+	return 0;
+}
+
 static int btusb_probe(struct usb_interface *intf,
 				const struct usb_device_id *id)
 {

@@ -1048,6 +1411,9 @@ static int btusb_probe(struct usb_interface *intf,
 	if (id->driver_info & BTUSB_BCM92035)
 		hdev->setup = btusb_setup_bcm92035;
 
+	if (id->driver_info & BTUSB_INTEL)
+		hdev->setup = btusb_setup_intel;
+
 	/* Interface numbers are hardcoded in the specification */
 	data->isoc = usb_ifnum_to_if(data->udev, 1);

Signature · 1 line

1.7.9.5

Gustavo Padovan

2013-04-18 04:12 UTC #1 lore raw

Hi Tedd,

Quoted 128 lines · Tedd Ho-Jeong An

From: Tedd Ho-Jeong An <[email protected]>

This patch adds support for Intel Bluetooth device by adding btusb_setup_intel() routine that update the device with ROM patch.

T:  Bus=02 Lev=02 Prnt=02 Port=00 Cnt=01 Dev#=  4 Spd=12   MxCh= 0
D:  Ver= 2.00 Cls=e0(wlcon) Sub=01 Prot=01 MxPS=64 #Cfgs=  1
P:  Vendor=8087 ProdID=07dc Rev= 0.01
C:* #Ifs= 2 Cfg#= 1 Atr=e0 MxPwr=100mA
I:* If#= 0 Alt= 0 #EPs= 3 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=81(I) Atr=03(Int.) MxPS=  64 Ivl=1ms
E:  Ad=02(O) Atr=02(Bulk) MxPS=  64 Ivl=0ms
E:  Ad=82(I) Atr=02(Bulk) MxPS=  64 Ivl=0ms
I:* If#= 1 Alt= 0 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=03(O) Atr=01(Isoc) MxPS=   0 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=   0 Ivl=1ms
I:  If#= 1 Alt= 1 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=03(O) Atr=01(Isoc) MxPS=   9 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=   9 Ivl=1ms
I:  If#= 1 Alt= 2 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=03(O) Atr=01(Isoc) MxPS=  17 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=  17 Ivl=1ms
I:  If#= 1 Alt= 3 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=03(O) Atr=01(Isoc) MxPS=  25 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=  25 Ivl=1ms
I:  If#= 1 Alt= 4 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=03(O) Atr=01(Isoc) MxPS=  33 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=  33 Ivl=1ms
I:  If#= 1 Alt= 5 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=03(O) Atr=01(Isoc) MxPS=  49 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=  49 Ivl=1ms

Signed-off-by Tedd Ho-Jeong An <[email protected]>

---
v4 changes
- refactored patching routin to separate function
- changed varaible type for status to u8
- corrected minor format issues

 drivers/bluetooth/btusb.c |  366 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 366 insertions(+)

drivers/bluetooth/btusb.c +57 -0

index 3d684d2..52c0ec0 100644
@@ -23,6 +23,7 @@
 
 #include <linux/module.h>
 #include <linux/usb.h>
+#include <linux/firmware.h>
 
 #include <net/bluetooth/bluetooth.h>
 #include <net/bluetooth/hci_core.h>

@@ -47,6 +48,7 @@ static struct usb_driver btusb_driver;
 #define BTUSB_BROKEN_ISOC	0x20
 #define BTUSB_WRONG_SCO_MTU	0x40
 #define BTUSB_ATH3012		0x80
+#define BTUSB_INTEL		0x100
 
 static struct usb_device_id btusb_table[] = {
 	/* Generic Bluetooth USB device */

@@ -207,6 +209,9 @@ static struct usb_device_id blacklist_table[] = {
 	/* Frontline ComProbe Bluetooth Sniffer */
 	{ USB_DEVICE(0x16d3, 0x0002), .driver_info = BTUSB_SNIFFER },
 
+	/* Intel Bluetooth device */
+	{ USB_DEVICE(0x8087, 0x07dc), .driver_info = BTUSB_INTEL },
+
 	{ }	/* Terminating entry */
 };

@@ -943,6 +948,364 @@ static int btusb_setup_bcm92035(struct hci_dev *hdev)
 	return 0;
 }
 
+struct intel_version {
+	u8	status;
+	u8	hw_platform;
+	u8	hw_variant;
+	u8	hw_revision;
+	u8	fw_variant;
+	u8	fw_revision;
+	u8	fw_build_num;
+	u8	fw_build_ww;
+	u8	fw_build_yy;
+	u8	fw_patch_num;
+} __packed;
+
+static const struct firmware *btusb_setup_intel_get_fw(struct hci_dev *hdev,
+						struct intel_version *ver)
+{
+	const struct firmware *fw;
+	char fwname[64];
+	int ret;
+
+	snprintf(fwname, sizeof(fwname),
+		 "intel/ibt-hw-%x.%x.%x-fw-%x.%x.%x.%x.%x.bseq",
+		 ver->hw_platform, ver->hw_variant, ver->hw_revision,
+		 ver->fw_variant,  ver->fw_revision, ver->fw_build_num,
+		 ver->fw_build_ww, ver->fw_build_yy);
+
+	ret = request_firmware(&fw, fwname, &hdev->dev);
+	if (ret < 0) {
+		if (ret == -EINVAL) {
+			BT_ERR("%s Intel firmware file request failed (%d)",
+			       hdev->name, ret);
+			return NULL;
+		}
+
+		BT_ERR("%s failed to open Intel firmware file: %s(%d)",
+		       hdev->name, fwname, ret);
+
+		snprintf(fwname, sizeof(fwname), "intel/ibt-default-%x.%x.bseq",
+			 ver->hw_platform, ver->hw_variant);
+		if (request_firmware(&fw, fwname, &hdev->dev) < 0) {
+			BT_ERR("%s failed to open default Intel fw file: %s",
+			       hdev->name, fwname);
+			return NULL;
+		}
+	}
+
+	BT_INFO("%s Intel Bluetooth firmware file: %s", hdev->name, fwname);
+
+	return fw;
+}
+
+static int btusb_setup_intel_patching(struct hci_dev *hdev,

drivers/bluetooth/btusb.c static int btusb_setup_bcm92035(struct hci_dev *hdev) · line 1012

Quoted 10 lines

drivers/bluetooth/btusb.c +10 -0

+				      const struct firmware *fw,
+				      const u8 **curr_fw_ptr,
+				      int *disable_patch)
+{
+	struct sk_buff *skb;
+	struct hci_command_hdr *cmd;
+	const u8 *cmd_param;
+	struct hci_event_hdr *evt = NULL;
+	const u8 *evt_param = NULL;
+	const u8 *fw_ptr = *curr_fw_ptr;

Why do you need an extra var to keep fw_ptr? can't you just use curr_fw_ptr directly?

drivers/bluetooth/btusb.c static int btusb_setup_bcm92035(struct hci_dev *hdev) · line 1017

Quoted 5 lines

drivers/bluetooth/btusb.c +5 -0

+	int remain = fw->size - (fw_ptr - fw->data);
+
+	/* Read command */
+	if (remain > HCI_COMMAND_HDR_SIZE &&
+	    fw_ptr[0] != 0x01) {

Apparently you don't need to break line on the if.

drivers/bluetooth/btusb.c static int btusb_setup_bcm92035(struct hci_dev *hdev) · line 1019

Quoted 2 lines

drivers/bluetooth/btusb.c +2 -0

+		BT_ERR("%s Intel fw corrupted: invalid cmd read", hdev->name);
+		return -1;

Please return a proper error code.

drivers/bluetooth/btusb.c static int btusb_setup_bcm92035(struct hci_dev *hdev) · line 1031

Quoted 12 lines

drivers/bluetooth/btusb.c +12 -0

+	}
+	fw_ptr++;
+	remain--;
+
+	cmd = (struct hci_command_hdr *)fw_ptr;
+	fw_ptr += sizeof(*cmd);
+	remain -= sizeof(*cmd);
+
+	/* checking the length */
+	if (remain < cmd->plen) {
+		BT_ERR("%s Intel fw corrupted: invalid cmd len", hdev->name);
+		return -1;

Same here and all other places you return -1

Quoted 74 lines

drivers/bluetooth/btusb.c +74 -0

+	}
+
+	/* If there is a command that loads a patch in the firmware
+	 * file, then enable the patch upon success, otherwise just
+	 * disable the manufacturer mode, for example patch activation
+	 * is not required when the default firmware patch file is used
+	 * because there are no patch data to load.
+	 */
+	if (*disable_patch && cmd->opcode == 0xfc8e)
+		*disable_patch = 0;
+
+	cmd_param = fw_ptr;
+	fw_ptr += cmd->plen;
+	remain -= cmd->plen;
+
+	/* Read event */
+	while (remain > HCI_EVENT_HDR_SIZE && fw_ptr[0] == 0x02) {
+		fw_ptr++;
+		remain--;
+
+		evt = (struct hci_event_hdr *)fw_ptr;
+		fw_ptr += sizeof(*evt);
+		remain -= sizeof(*evt);
+
+		if (remain < evt->plen) {
+			BT_ERR("%s Intel fw corrupted: invalid evt len",
+			       hdev->name);
+			return -1;
+		}
+
+		evt_param = fw_ptr;
+		fw_ptr += evt->plen;
+		remain -= evt->plen;
+	}
+
+	if (!evt || !evt_param || remain < 0) {
+		BT_ERR("%s Intel fw corrupted: invalid evt read", hdev->name);
+		return -1;
+	}
+
+	skb = __hci_cmd_sync_ev(hdev, le16_to_cpu(cmd->opcode),
+				cmd->plen, (void *)cmd_param, evt->evt,
+				HCI_INIT_TIMEOUT);
+	if (IS_ERR(skb)) {
+		BT_ERR("%s sending Intel patch command failed (%ld)",
+		       hdev->name, PTR_ERR(skb));
+		return -1;
+	}
+
+	/* Checking the returned event */
+	if (skb->len != evt->plen) {
+		BT_ERR("%s mismatch event length", hdev->name);
+		kfree_skb(skb);
+		return -1;
+	}
+
+	if (memcmp(skb->data, evt_param, evt->plen)) {
+		BT_ERR("%s mismatch event parameter", hdev->name);
+		kfree_skb(skb);
+		return -1;
+	}
+	kfree_skb(skb);
+
+	*curr_fw_ptr = fw_ptr;
+
+	return 0;
+}
+
+static int btusb_setup_intel(struct hci_dev *hdev)
+{
+	struct sk_buff *skb;
+	const struct firmware *fw;
+	const u8 *fw_ptr;
+	int disable_patch;

drivers/bluetooth/btusb.c static int btusb_setup_bcm92035(struct hci_dev *hdev) · line 1115

Quoted 10 lines

drivers/bluetooth/btusb.c +10 -0

+	struct intel_version *ver;
+
+	u8 mfg_enable[] = { 0x01, 0x00 };
+	u8 mfg_disable[] = { 0x00, 0x00 };
+	u8 mfg_reset_deactivate[] = { 0x00, 0x01 };
+	u8 mfg_reset_activate[] = { 0x00, 0x02 };
+
+	BT_DBG("%s", hdev->name);
+
+	/* The controller has a bug with the first HCI command send to it

s/send/sent/

Quoted 12 lines

drivers/bluetooth/btusb.c +12 -0

+	 * returning number of completed commands as zero. This would stall the
+	 * command processing in the Bluetooth core
+	 *
+	 * As a workaround, send HCI Reset command first which will reset the
+	 * number of completed commands and allow normal command processing
+	 * from now on
+	 */
+	skb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL, HCI_INIT_TIMEOUT);
+	if (IS_ERR(skb)) {
+		BT_ERR("%s sending initial HCI reset command failed (%ld)",
+		       hdev->name, PTR_ERR(skb));
+		return -PTR_ERR(skb);

drivers/bluetooth/btusb.c static int btusb_setup_bcm92035(struct hci_dev *hdev) · line 1137

Quoted 10 lines

drivers/bluetooth/btusb.c +10 -0

+	}
+	kfree_skb(skb);
+
+	/* Read Intel specific controller version first to allow selection of
+	 * which firmware file to load.
+	 *
+	 * The returned information are hardware variant and revision plus
+	 * firmware variant, revision and build number.
+	 */
+	skb = __hci_cmd_sync(hdev, 0xfc05, 0, NULL, HCI_INIT_TIMEOUT);

Could you turn 0xfc05 into a macro that we can actually understand

drivers/bluetooth/btusb.c static int btusb_setup_bcm92035(struct hci_dev *hdev) · line 1147

Quoted 10 lines

drivers/bluetooth/btusb.c +10 -0

+	if (IS_ERR(skb)) {
+		BT_ERR("%s reading Intel fw version command failed (%ld)",
+		       hdev->name, PTR_ERR(skb));
+		return -PTR_ERR(skb);
+	}
+
+	if (skb->len != sizeof(*ver)) {
+		BT_ERR("%s Intel version event length mismatch", hdev->name);
+		kfree_skb(skb);
+		return 0;

So why you return 0 here?

drivers/bluetooth/btusb.c static int btusb_setup_bcm92035(struct hci_dev *hdev) · line 1150

Quoted 3 lines

drivers/bluetooth/btusb.c +3 -0
+	}
+
+	ver = (struct intel_version *)skb->data;

Is the cast really necessary?

drivers/bluetooth/btusb.c static int btusb_setup_bcm92035(struct hci_dev *hdev) · line 1151

Quoted 1 line

drivers/bluetooth/btusb.c +1 -0
+	/* was command successful */

I would just drop this comment. The 'if' below already tell you that.

drivers/bluetooth/btusb.c static int btusb_setup_bcm92035(struct hci_dev *hdev) · line 1156

Quoted 5 lines

drivers/bluetooth/btusb.c +5 -0

+	if (ver->status) {
+		BT_ERR("%s Intel fw version event failed (%02x)", hdev->name,
+		       ver->status);
+		kfree_skb(skb);
+		return 0;

Here again. Why return 0? Check other places in the function.

drivers/bluetooth/btusb.c static int btusb_setup_bcm92035(struct hci_dev *hdev) · line 1159

Quoted 3 lines

drivers/bluetooth/btusb.c +3 -0

+	}
+
+	BT_INFO("%s read Intel version: %02x%02x%02x%02x%02x%02x%02x%02x%02x",

Add a ":" after the %s for hdev->name

Quoted 25 lines

drivers/bluetooth/btusb.c +25 -0

+		hdev->name, ver->hw_platform, ver->hw_variant,
+		ver->hw_revision, ver->fw_variant,  ver->fw_revision,
+		ver->fw_build_num, ver->fw_build_ww, ver->fw_build_yy,
+		ver->fw_patch_num);
+
+	/* is the device already patched */
+	if (ver->fw_patch_num) {
+		BT_INFO("%s Intel device is already patched. patch num: %02x",
+			hdev->name, ver->fw_patch_num);
+		kfree_skb(skb);
+		return 0;
+	}
+
+	/* Opens the firmware patch file based on the firmware version read
+	 * from the controller. If it failed to open the matching firmware
+	 * patch file, it tries to open the default firmware patch file.
+	 * If no patch file, allow the device to operate without a patch.
+	 */
+	fw = btusb_setup_intel_get_fw(hdev, ver);
+	if (!fw) {
+		kfree_skb(skb);
+		return 0;
+	}
+	fw_ptr = fw->data;
+

drivers/bluetooth/btusb.c static int btusb_setup_bcm92035(struct hci_dev *hdev) · line 1194

Quoted 10 lines

drivers/bluetooth/btusb.c +10 -0

+	/* This Intel specific command enables the manufacturer mode of the
+	 * controller.
+	 *
+	 * Only while this mode is enabled, the driver can download the
+	 * firmware patch data and configuration parameters.
+	 */
+	skb = __hci_cmd_sync(hdev, 0xfc11, 2, mfg_enable, HCI_INIT_TIMEOUT);
+	if (IS_ERR(skb)) {
+		BT_ERR("%s entering Intel manufacturer mode failed (%ld)",
+		       hdev->name, PTR_ERR(skb));

Same here ":" after %s.

Quoted 21 lines

drivers/bluetooth/btusb.c +21 -0

+		release_firmware(fw);
+		return -PTR_ERR(skb);
+	}
+
+	if (skb->data[0]) {
+		u8 evt_status = skb->data[0];
+		BT_ERR("%s enable Intel manufacturer mode event failed (%02x)",
+		       hdev->name, evt_status);
+		kfree_skb(skb);
+		release_firmware(fw);
+		return -bt_to_errno(evt_status);
+	}
+	kfree_skb(skb);
+
+	disable_patch = 1;
+
+	/* The firmware data file consists of list of Intel specific HCI
+	 * commands and its expected events. The first byte indicates the
+	 * type of the message, either HCI command or HCI event.
+	 *
+	 * It reads the command and its expected event from the firmware file,

drivers/bluetooth/btusb.c static int btusb_setup_bcm92035(struct hci_dev *hdev) · line 1225

Quoted 10 lines

drivers/bluetooth/btusb.c +10 -0

+	 * and send to the controller. Once __hci_cmd_sync_ev() returns,
+	 * the returned event is compared with the event read from the firmware
+	 * file and it will continue until all the messages are downloaded to
+	 * the controller.
+	 *
+	 * Once the firmware patching is completed successfully,
+	 * the manufacturer mode is disabled with reset and activating the
+	 * downloaded patch.
+	 *
+	 * If the firmware patching is failed, the manufacturer mode is

s/is failed/fails/

Quoted 12 lines

drivers/bluetooth/btusb.c +12 -0

+	 * disabled with reset and deactivating the patch.
+	 *
+	 * If the default patch file is used, no reset is done when disabling
+	 * the manufacturer.
+	 */
+	while (1) {
+		int ret;
+
+		ret = btusb_setup_intel_patching(hdev, fw, &fw_ptr,
+						 &disable_patch);
+		if (ret < 0)
+			goto exit_mfg_deactivate;

drivers/bluetooth/btusb.c static int btusb_setup_bcm92035(struct hci_dev *hdev) · line 1247

Quoted 10 lines

drivers/bluetooth/btusb.c +10 -0

+
+		/* Checking if EOF */
+		if (fw->size == fw_ptr - fw->data) {
+			if (disable_patch)
+				goto exit_mfg_disable;
+			else
+				goto exit_mfg_activate;
+		} else if (fw->size < fw_ptr - fw->data) {
+			BT_ERR("%s inconsistent patch read size", hdev->name);
+			goto exit_mfg_deactivate;

It seems that to make this if true 'remain' has to be < 0 in the other function and if remain is < 0 the we actually never see this if since the return of btusb_setup_intel_patching() is < 0 and we goto exit_mfg_deactivate above.

Gustavo

Tedd Ho-Jeong An

author

2013-04-18 16:10 UTC #2 lore raw

Hi Gustavo

Quoted 137 lines · Gustavo Padovan

Hi Tedd,

Quoted 133 lines · Tedd Ho-Jeong An

From: Tedd Ho-Jeong An <[email protected]>

This patch adds support for Intel Bluetooth device by adding btusb_setup_intel() routine that update the device with ROM patch.

T:  Bus=02 Lev=02 Prnt=02 Port=00 Cnt=01 Dev#=  4 Spd=12   MxCh= 0
D:  Ver= 2.00 Cls=e0(wlcon) Sub=01 Prot=01 MxPS=64 #Cfgs=  1
P:  Vendor=8087 ProdID=07dc Rev= 0.01
C:* #Ifs= 2 Cfg#= 1 Atr=e0 MxPwr=100mA
I:* If#= 0 Alt= 0 #EPs= 3 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=81(I) Atr=03(Int.) MxPS=  64 Ivl=1ms
E:  Ad=02(O) Atr=02(Bulk) MxPS=  64 Ivl=0ms
E:  Ad=82(I) Atr=02(Bulk) MxPS=  64 Ivl=0ms
I:* If#= 1 Alt= 0 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=03(O) Atr=01(Isoc) MxPS=   0 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=   0 Ivl=1ms
I:  If#= 1 Alt= 1 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=03(O) Atr=01(Isoc) MxPS=   9 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=   9 Ivl=1ms
I:  If#= 1 Alt= 2 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=03(O) Atr=01(Isoc) MxPS=  17 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=  17 Ivl=1ms
I:  If#= 1 Alt= 3 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=03(O) Atr=01(Isoc) MxPS=  25 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=  25 Ivl=1ms
I:  If#= 1 Alt= 4 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=03(O) Atr=01(Isoc) MxPS=  33 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=  33 Ivl=1ms
I:  If#= 1 Alt= 5 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=03(O) Atr=01(Isoc) MxPS=  49 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=  49 Ivl=1ms

Signed-off-by Tedd Ho-Jeong An <[email protected]>

---
v4 changes
- refactored patching routin to separate function
- changed varaible type for status to u8
- corrected minor format issues

 drivers/bluetooth/btusb.c |  366 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 366 insertions(+)

drivers/bluetooth/btusb.c +62 -0

index 3d684d2..52c0ec0 100644
@@ -23,6 +23,7 @@
 
 #include <linux/module.h>
 #include <linux/usb.h>
+#include <linux/firmware.h>
 
 #include <net/bluetooth/bluetooth.h>
 #include <net/bluetooth/hci_core.h>

@@ -47,6 +48,7 @@ static struct usb_driver btusb_driver;
 #define BTUSB_BROKEN_ISOC	0x20
 #define BTUSB_WRONG_SCO_MTU	0x40
 #define BTUSB_ATH3012		0x80
+#define BTUSB_INTEL		0x100
 
 static struct usb_device_id btusb_table[] = {
 	/* Generic Bluetooth USB device */

@@ -207,6 +209,9 @@ static struct usb_device_id blacklist_table[] = {
 	/* Frontline ComProbe Bluetooth Sniffer */
 	{ USB_DEVICE(0x16d3, 0x0002), .driver_info = BTUSB_SNIFFER },
 
+	/* Intel Bluetooth device */
+	{ USB_DEVICE(0x8087, 0x07dc), .driver_info = BTUSB_INTEL },
+
 	{ }	/* Terminating entry */
 };

@@ -943,6 +948,364 @@ static int btusb_setup_bcm92035(struct hci_dev *hdev)
 	return 0;
 }
 
+struct intel_version {
+	u8	status;
+	u8	hw_platform;
+	u8	hw_variant;
+	u8	hw_revision;
+	u8	fw_variant;
+	u8	fw_revision;
+	u8	fw_build_num;
+	u8	fw_build_ww;
+	u8	fw_build_yy;
+	u8	fw_patch_num;
+} __packed;
+
+static const struct firmware *btusb_setup_intel_get_fw(struct hci_dev *hdev,
+						struct intel_version *ver)
+{
+	const struct firmware *fw;
+	char fwname[64];
+	int ret;
+
+	snprintf(fwname, sizeof(fwname),
+		 "intel/ibt-hw-%x.%x.%x-fw-%x.%x.%x.%x.%x.bseq",
+		 ver->hw_platform, ver->hw_variant, ver->hw_revision,
+		 ver->fw_variant,  ver->fw_revision, ver->fw_build_num,
+		 ver->fw_build_ww, ver->fw_build_yy);
+
+	ret = request_firmware(&fw, fwname, &hdev->dev);
+	if (ret < 0) {
+		if (ret == -EINVAL) {
+			BT_ERR("%s Intel firmware file request failed (%d)",
+			       hdev->name, ret);
+			return NULL;
+		}
+
+		BT_ERR("%s failed to open Intel firmware file: %s(%d)",
+		       hdev->name, fwname, ret);
+
+		snprintf(fwname, sizeof(fwname), "intel/ibt-default-%x.%x.bseq",
+			 ver->hw_platform, ver->hw_variant);
+		if (request_firmware(&fw, fwname, &hdev->dev) < 0) {
+			BT_ERR("%s failed to open default Intel fw file: %s",
+			       hdev->name, fwname);
+			return NULL;
+		}
+	}
+
+	BT_INFO("%s Intel Bluetooth firmware file: %s", hdev->name, fwname);
+
+	return fw;
+}
+
+static int btusb_setup_intel_patching(struct hci_dev *hdev,
+				      const struct firmware *fw,
+				      const u8 **curr_fw_ptr,
+				      int *disable_patch)
+{
+	struct sk_buff *skb;

Quoted 8 lines

drivers/bluetooth/btusb.c static int btusb_setup_bcm92035(struct hci_dev *hdev) · line 1012
Quoted 5 lines
drivers/bluetooth/btusb.c +5 -0
+	struct hci_command_hdr *cmd;
+	const u8 *cmd_param;
+	struct hci_event_hdr *evt = NULL;
+	const u8 *evt_param = NULL;
+	const u8 *fw_ptr = *curr_fw_ptr;
Why do you need an extra var to keep fw_ptr? can't you just use curr_fw_ptr directly?

Yes. I can. I will make a change.

Quoted 8 lines

drivers/bluetooth/btusb.c static int btusb_setup_bcm92035(struct hci_dev *hdev) · line 1017
Quoted 6 lines
drivers/bluetooth/btusb.c +5 -0
+	int remain = fw->size - (fw_ptr - fw->data);
+
+	/* Read command */
+	if (remain > HCI_COMMAND_HDR_SIZE &&
+	    fw_ptr[0] != 0x01) {
Apparently you don't need to break line on the if.

ACK.

Quoted 5 lines

drivers/bluetooth/btusb.c static int btusb_setup_bcm92035(struct hci_dev *hdev) · line 1019
Quoted 3 lines
drivers/bluetooth/btusb.c +2 -0
+		BT_ERR("%s Intel fw corrupted: invalid cmd read", hdev->name);
+		return -1;
Please return a proper error code.

ACK.

Quoted 15 lines

drivers/bluetooth/btusb.c static int btusb_setup_bcm92035(struct hci_dev *hdev) · line 1031
Quoted 13 lines
drivers/bluetooth/btusb.c +12 -0
+	}
+	fw_ptr++;
+	remain--;
+
+	cmd = (struct hci_command_hdr *)fw_ptr;
+	fw_ptr += sizeof(*cmd);
+	remain -= sizeof(*cmd);
+
+	/* checking the length */
+	if (remain < cmd->plen) {
+		BT_ERR("%s Intel fw corrupted: invalid cmd len", hdev->name);
+		return -1;
Same here and all other places you return -1

ACK.

Quoted 79 lines

drivers/bluetooth/btusb.c +78 -0

+	}
+
+	/* If there is a command that loads a patch in the firmware
+	 * file, then enable the patch upon success, otherwise just
+	 * disable the manufacturer mode, for example patch activation
+	 * is not required when the default firmware patch file is used
+	 * because there are no patch data to load.
+	 */
+	if (*disable_patch && cmd->opcode == 0xfc8e)
+		*disable_patch = 0;
+
+	cmd_param = fw_ptr;
+	fw_ptr += cmd->plen;
+	remain -= cmd->plen;
+
+	/* Read event */
+	while (remain > HCI_EVENT_HDR_SIZE && fw_ptr[0] == 0x02) {
+		fw_ptr++;
+		remain--;
+
+		evt = (struct hci_event_hdr *)fw_ptr;
+		fw_ptr += sizeof(*evt);
+		remain -= sizeof(*evt);
+
+		if (remain < evt->plen) {
+			BT_ERR("%s Intel fw corrupted: invalid evt len",
+			       hdev->name);
+			return -1;
+		}
+
+		evt_param = fw_ptr;
+		fw_ptr += evt->plen;
+		remain -= evt->plen;
+	}
+
+	if (!evt || !evt_param || remain < 0) {
+		BT_ERR("%s Intel fw corrupted: invalid evt read", hdev->name);
+		return -1;
+	}
+
+	skb = __hci_cmd_sync_ev(hdev, le16_to_cpu(cmd->opcode),
+				cmd->plen, (void *)cmd_param, evt->evt,
+				HCI_INIT_TIMEOUT);
+	if (IS_ERR(skb)) {
+		BT_ERR("%s sending Intel patch command failed (%ld)",
+		       hdev->name, PTR_ERR(skb));
+		return -1;
+	}
+
+	/* Checking the returned event */
+	if (skb->len != evt->plen) {
+		BT_ERR("%s mismatch event length", hdev->name);
+		kfree_skb(skb);
+		return -1;
+	}
+
+	if (memcmp(skb->data, evt_param, evt->plen)) {
+		BT_ERR("%s mismatch event parameter", hdev->name);
+		kfree_skb(skb);
+		return -1;
+	}
+	kfree_skb(skb);
+
+	*curr_fw_ptr = fw_ptr;
+
+	return 0;
+}
+
+static int btusb_setup_intel(struct hci_dev *hdev)
+{
+	struct sk_buff *skb;
+	const struct firmware *fw;
+	const u8 *fw_ptr;
+	int disable_patch;
+	struct intel_version *ver;
+
+	u8 mfg_enable[] = { 0x01, 0x00 };
+	u8 mfg_disable[] = { 0x00, 0x00 };

Quoted 8 lines

drivers/bluetooth/btusb.c static int btusb_setup_bcm92035(struct hci_dev *hdev) · line 1115
Quoted 6 lines
drivers/bluetooth/btusb.c +6 -0
+	u8 mfg_reset_deactivate[] = { 0x00, 0x01 };
+	u8 mfg_reset_activate[] = { 0x00, 0x02 };
+
+	BT_DBG("%s", hdev->name);
+
+	/* The controller has a bug with the first HCI command send to it
s/send/sent/

ACK.

Quoted 17 lines

drivers/bluetooth/btusb.c +16 -0

+	 * returning number of completed commands as zero. This would stall the
+	 * command processing in the Bluetooth core
+	 *
+	 * As a workaround, send HCI Reset command first which will reset the
+	 * number of completed commands and allow normal command processing
+	 * from now on
+	 */
+	skb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL, HCI_INIT_TIMEOUT);
+	if (IS_ERR(skb)) {
+		BT_ERR("%s sending initial HCI reset command failed (%ld)",
+		       hdev->name, PTR_ERR(skb));
+		return -PTR_ERR(skb);
+	}
+	kfree_skb(skb);
+
+	/* Read Intel specific controller version first to allow selection of

Quoted 8 lines

drivers/bluetooth/btusb.c static int btusb_setup_bcm92035(struct hci_dev *hdev) · line 1137
Quoted 6 lines
drivers/bluetooth/btusb.c +6 -0
+	 * which firmware file to load.
+	 *
+	 * The returned information are hardware variant and revision plus
+	 * firmware variant, revision and build number.
+	 */
+	skb = __hci_cmd_sync(hdev, 0xfc05, 0, NULL, HCI_INIT_TIMEOUT);
Could you turn 0xfc05 into a macro that we can actually understand

My older version of patch used macro but I changed it later due to a comments from others. I will make a change back to use macro. I think that will make the code cleaner.

Quoted 13 lines

drivers/bluetooth/btusb.c static int btusb_setup_bcm92035(struct hci_dev *hdev) · line 1147
Quoted 11 lines
drivers/bluetooth/btusb.c +10 -0
+	if (IS_ERR(skb)) {
+		BT_ERR("%s reading Intel fw version command failed (%ld)",
+		       hdev->name, PTR_ERR(skb));
+		return -PTR_ERR(skb);
+	}
+
+	if (skb->len != sizeof(*ver)) {
+		BT_ERR("%s Intel version event length mismatch", hdev->name);
+		kfree_skb(skb);
+		return 0;
So why you return 0 here?

Orignially, I thought it would be ok to continue the initialization (BT init) even if it received invalid length or status. Actually, if it receives these errors, there must be something wrong in the device and should return with error.

I will make a change to return with proper error code.

However, please understand that there are a certain cases that I need to return 0 even if the fw patching is not completed.

I will carefully review those return case.

Quoted 10 lines

drivers/bluetooth/btusb.c static int btusb_setup_bcm92035(struct hci_dev *hdev) · line 1150
Quoted 4 lines
drivers/bluetooth/btusb.c +3 -0
+	}
+
+	ver = (struct intel_version *)skb->data;
Is the cast really necessary?
drivers/bluetooth/btusb.c static int btusb_setup_bcm92035(struct hci_dev *hdev) · line 1151
Quoted 1 line
drivers/bluetooth/btusb.c +1 -0
+	/* was command successful */
I would just drop this comment. The 'if' below already tell you that.

ACK

Quoted 8 lines

drivers/bluetooth/btusb.c static int btusb_setup_bcm92035(struct hci_dev *hdev) · line 1156
Quoted 6 lines
drivers/bluetooth/btusb.c +5 -0
+	if (ver->status) {
+		BT_ERR("%s Intel fw version event failed (%02x)", hdev->name,
+		       ver->status);
+		kfree_skb(skb);
+		return 0;
Here again. Why return 0? Check other places in the function.

Same as above.

Quoted 6 lines

drivers/bluetooth/btusb.c static int btusb_setup_bcm92035(struct hci_dev *hdev) · line 1159
Quoted 4 lines
drivers/bluetooth/btusb.c +3 -0
+	}
+
+	BT_INFO("%s read Intel version: %02x%02x%02x%02x%02x%02x%02x%02x%02x",
Add a ":" after the %s for hdev->name

ACK

Quoted 30 lines

drivers/bluetooth/btusb.c +29 -0

+		hdev->name, ver->hw_platform, ver->hw_variant,
+		ver->hw_revision, ver->fw_variant,  ver->fw_revision,
+		ver->fw_build_num, ver->fw_build_ww, ver->fw_build_yy,
+		ver->fw_patch_num);
+
+	/* is the device already patched */
+	if (ver->fw_patch_num) {
+		BT_INFO("%s Intel device is already patched. patch num: %02x",
+			hdev->name, ver->fw_patch_num);
+		kfree_skb(skb);
+		return 0;
+	}
+
+	/* Opens the firmware patch file based on the firmware version read
+	 * from the controller. If it failed to open the matching firmware
+	 * patch file, it tries to open the default firmware patch file.
+	 * If no patch file, allow the device to operate without a patch.
+	 */
+	fw = btusb_setup_intel_get_fw(hdev, ver);
+	if (!fw) {
+		kfree_skb(skb);
+		return 0;
+	}
+	fw_ptr = fw->data;
+
+	/* This Intel specific command enables the manufacturer mode of the
+	 * controller.
+	 *
+	 * Only while this mode is enabled, the driver can download the

Quoted 8 lines

drivers/bluetooth/btusb.c static int btusb_setup_bcm92035(struct hci_dev *hdev) · line 1194
Quoted 6 lines
drivers/bluetooth/btusb.c +6 -0
+	 * firmware patch data and configuration parameters.
+	 */
+	skb = __hci_cmd_sync(hdev, 0xfc11, 2, mfg_enable, HCI_INIT_TIMEOUT);
+	if (IS_ERR(skb)) {
+		BT_ERR("%s entering Intel manufacturer mode failed (%ld)",
+		       hdev->name, PTR_ERR(skb));
Same here ":" after %s.

ACK

Quoted 26 lines

drivers/bluetooth/btusb.c +25 -0

+		release_firmware(fw);
+		return -PTR_ERR(skb);
+	}
+
+	if (skb->data[0]) {
+		u8 evt_status = skb->data[0];
+		BT_ERR("%s enable Intel manufacturer mode event failed (%02x)",
+		       hdev->name, evt_status);
+		kfree_skb(skb);
+		release_firmware(fw);
+		return -bt_to_errno(evt_status);
+	}
+	kfree_skb(skb);
+
+	disable_patch = 1;
+
+	/* The firmware data file consists of list of Intel specific HCI
+	 * commands and its expected events. The first byte indicates the
+	 * type of the message, either HCI command or HCI event.
+	 *
+	 * It reads the command and its expected event from the firmware file,
+	 * and send to the controller. Once __hci_cmd_sync_ev() returns,
+	 * the returned event is compared with the event read from the firmware
+	 * file and it will continue until all the messages are downloaded to
+	 * the controller.

Quoted 8 lines

drivers/bluetooth/btusb.c static int btusb_setup_bcm92035(struct hci_dev *hdev) · line 1225
Quoted 6 lines
drivers/bluetooth/btusb.c +6 -0
+	 *
+	 * Once the firmware patching is completed successfully,
+	 * the manufacturer mode is disabled with reset and activating the
+	 * downloaded patch.
+	 *
+	 * If the firmware patching is failed, the manufacturer mode is
s/is failed/fails/

ACK

Quoted 20 lines

drivers/bluetooth/btusb.c +19 -0

+	 * disabled with reset and deactivating the patch.
+	 *
+	 * If the default patch file is used, no reset is done when disabling
+	 * the manufacturer.
+	 */
+	while (1) {
+		int ret;
+
+		ret = btusb_setup_intel_patching(hdev, fw, &fw_ptr,
+						 &disable_patch);
+		if (ret < 0)
+			goto exit_mfg_deactivate;
+
+		/* Checking if EOF */
+		if (fw->size == fw_ptr - fw->data) {
+			if (disable_patch)
+				goto exit_mfg_disable;
+			else
+				goto exit_mfg_activate;

Quoted 9 lines

drivers/bluetooth/btusb.c static int btusb_setup_bcm92035(struct hci_dev *hdev) · line 1247
Quoted 3 lines
drivers/bluetooth/btusb.c +3 -0
+		} else if (fw->size < fw_ptr - fw->data) {
+			BT_ERR("%s inconsistent patch read size", hdev->name);
+			goto exit_mfg_deactivate;
It seems that to make this if true 'remain' has to be < 0 in the other function and if remain is < 0 the we actually never see this if since the return of btusb_setup_intel_patching() is < 0 and we goto exit_mfg_deactivate above.

I was a little coufused at first but I got your point. Yes, you are right. this 'else if' will never be hit. Good point and I will remove it.

Quoted 1 line

Gustavo

Signature · 3 lines

Regards
Tedd Ho-Jeong An
Intel Corporation

Gustavo Padovan

2013-04-18 16:18 UTC #3 lore raw

Hi Tedd,

Quoted 321 lines · Tedd Ho-Jeong An

Hi Gustavo

Quoted 137 lines · Gustavo Padovan

Hi Tedd,

Quoted 133 lines · Tedd Ho-Jeong An

From: Tedd Ho-Jeong An <[email protected]>

This patch adds support for Intel Bluetooth device by adding btusb_setup_intel() routine that update the device with ROM patch.

T:  Bus=02 Lev=02 Prnt=02 Port=00 Cnt=01 Dev#=  4 Spd=12   MxCh= 0
D:  Ver= 2.00 Cls=e0(wlcon) Sub=01 Prot=01 MxPS=64 #Cfgs=  1
P:  Vendor=8087 ProdID=07dc Rev= 0.01
C:* #Ifs= 2 Cfg#= 1 Atr=e0 MxPwr=100mA
I:* If#= 0 Alt= 0 #EPs= 3 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=81(I) Atr=03(Int.) MxPS=  64 Ivl=1ms
E:  Ad=02(O) Atr=02(Bulk) MxPS=  64 Ivl=0ms
E:  Ad=82(I) Atr=02(Bulk) MxPS=  64 Ivl=0ms
I:* If#= 1 Alt= 0 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=03(O) Atr=01(Isoc) MxPS=   0 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=   0 Ivl=1ms
I:  If#= 1 Alt= 1 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=03(O) Atr=01(Isoc) MxPS=   9 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=   9 Ivl=1ms
I:  If#= 1 Alt= 2 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=03(O) Atr=01(Isoc) MxPS=  17 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=  17 Ivl=1ms
I:  If#= 1 Alt= 3 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=03(O) Atr=01(Isoc) MxPS=  25 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=  25 Ivl=1ms
I:  If#= 1 Alt= 4 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=03(O) Atr=01(Isoc) MxPS=  33 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=  33 Ivl=1ms
I:  If#= 1 Alt= 5 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=03(O) Atr=01(Isoc) MxPS=  49 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=  49 Ivl=1ms

Signed-off-by Tedd Ho-Jeong An <[email protected]>

---
v4 changes
- refactored patching routin to separate function
- changed varaible type for status to u8
- corrected minor format issues

 drivers/bluetooth/btusb.c |  366 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 366 insertions(+)

drivers/bluetooth/btusb.c +62 -0

index 3d684d2..52c0ec0 100644
@@ -23,6 +23,7 @@
 
 #include <linux/module.h>
 #include <linux/usb.h>
+#include <linux/firmware.h>
 
 #include <net/bluetooth/bluetooth.h>
 #include <net/bluetooth/hci_core.h>

@@ -47,6 +48,7 @@ static struct usb_driver btusb_driver;
 #define BTUSB_BROKEN_ISOC	0x20
 #define BTUSB_WRONG_SCO_MTU	0x40
 #define BTUSB_ATH3012		0x80
+#define BTUSB_INTEL		0x100
 
 static struct usb_device_id btusb_table[] = {
 	/* Generic Bluetooth USB device */

@@ -207,6 +209,9 @@ static struct usb_device_id blacklist_table[] = {
 	/* Frontline ComProbe Bluetooth Sniffer */
 	{ USB_DEVICE(0x16d3, 0x0002), .driver_info = BTUSB_SNIFFER },
 
+	/* Intel Bluetooth device */
+	{ USB_DEVICE(0x8087, 0x07dc), .driver_info = BTUSB_INTEL },
+
 	{ }	/* Terminating entry */
 };

@@ -943,6 +948,364 @@ static int btusb_setup_bcm92035(struct hci_dev *hdev)
 	return 0;
 }
 
+struct intel_version {
+	u8	status;
+	u8	hw_platform;
+	u8	hw_variant;
+	u8	hw_revision;
+	u8	fw_variant;
+	u8	fw_revision;
+	u8	fw_build_num;
+	u8	fw_build_ww;
+	u8	fw_build_yy;
+	u8	fw_patch_num;
+} __packed;
+
+static const struct firmware *btusb_setup_intel_get_fw(struct hci_dev *hdev,
+						struct intel_version *ver)
+{
+	const struct firmware *fw;
+	char fwname[64];
+	int ret;
+
+	snprintf(fwname, sizeof(fwname),
+		 "intel/ibt-hw-%x.%x.%x-fw-%x.%x.%x.%x.%x.bseq",
+		 ver->hw_platform, ver->hw_variant, ver->hw_revision,
+		 ver->fw_variant,  ver->fw_revision, ver->fw_build_num,
+		 ver->fw_build_ww, ver->fw_build_yy);
+
+	ret = request_firmware(&fw, fwname, &hdev->dev);
+	if (ret < 0) {
+		if (ret == -EINVAL) {
+			BT_ERR("%s Intel firmware file request failed (%d)",
+			       hdev->name, ret);
+			return NULL;
+		}
+
+		BT_ERR("%s failed to open Intel firmware file: %s(%d)",
+		       hdev->name, fwname, ret);
+
+		snprintf(fwname, sizeof(fwname), "intel/ibt-default-%x.%x.bseq",
+			 ver->hw_platform, ver->hw_variant);
+		if (request_firmware(&fw, fwname, &hdev->dev) < 0) {
+			BT_ERR("%s failed to open default Intel fw file: %s",
+			       hdev->name, fwname);
+			return NULL;
+		}
+	}
+
+	BT_INFO("%s Intel Bluetooth firmware file: %s", hdev->name, fwname);
+
+	return fw;
+}
+
+static int btusb_setup_intel_patching(struct hci_dev *hdev,
+				      const struct firmware *fw,
+				      const u8 **curr_fw_ptr,
+				      int *disable_patch)
+{
+	struct sk_buff *skb;

Quoted 8 lines

drivers/bluetooth/btusb.c static int btusb_setup_bcm92035(struct hci_dev *hdev) · line 1012
Quoted 5 lines
drivers/bluetooth/btusb.c +5 -0
+	struct hci_command_hdr *cmd;
+	const u8 *cmd_param;
+	struct hci_event_hdr *evt = NULL;
+	const u8 *evt_param = NULL;
+	const u8 *fw_ptr = *curr_fw_ptr;
Why do you need an extra var to keep fw_ptr? can't you just use curr_fw_ptr directly?

Yes. I can. I will make a change.

Quoted 8 lines

drivers/bluetooth/btusb.c static int btusb_setup_bcm92035(struct hci_dev *hdev) · line 1017
Quoted 6 lines
drivers/bluetooth/btusb.c +5 -0
+	int remain = fw->size - (fw_ptr - fw->data);
+
+	/* Read command */
+	if (remain > HCI_COMMAND_HDR_SIZE &&
+	    fw_ptr[0] != 0x01) {
Apparently you don't need to break line on the if.

ACK.

Quoted 5 lines

drivers/bluetooth/btusb.c static int btusb_setup_bcm92035(struct hci_dev *hdev) · line 1019
Quoted 3 lines
drivers/bluetooth/btusb.c +2 -0
+		BT_ERR("%s Intel fw corrupted: invalid cmd read", hdev->name);
+		return -1;
Please return a proper error code.

ACK.

Quoted 15 lines

drivers/bluetooth/btusb.c static int btusb_setup_bcm92035(struct hci_dev *hdev) · line 1031
Quoted 13 lines
drivers/bluetooth/btusb.c +12 -0
+	}
+	fw_ptr++;
+	remain--;
+
+	cmd = (struct hci_command_hdr *)fw_ptr;
+	fw_ptr += sizeof(*cmd);
+	remain -= sizeof(*cmd);
+
+	/* checking the length */
+	if (remain < cmd->plen) {
+		BT_ERR("%s Intel fw corrupted: invalid cmd len", hdev->name);
+		return -1;
Same here and all other places you return -1

ACK.

Quoted 79 lines

drivers/bluetooth/btusb.c +78 -0

+	}
+
+	/* If there is a command that loads a patch in the firmware
+	 * file, then enable the patch upon success, otherwise just
+	 * disable the manufacturer mode, for example patch activation
+	 * is not required when the default firmware patch file is used
+	 * because there are no patch data to load.
+	 */
+	if (*disable_patch && cmd->opcode == 0xfc8e)
+		*disable_patch = 0;
+
+	cmd_param = fw_ptr;
+	fw_ptr += cmd->plen;
+	remain -= cmd->plen;
+
+	/* Read event */
+	while (remain > HCI_EVENT_HDR_SIZE && fw_ptr[0] == 0x02) {
+		fw_ptr++;
+		remain--;
+
+		evt = (struct hci_event_hdr *)fw_ptr;
+		fw_ptr += sizeof(*evt);
+		remain -= sizeof(*evt);
+
+		if (remain < evt->plen) {
+			BT_ERR("%s Intel fw corrupted: invalid evt len",
+			       hdev->name);
+			return -1;
+		}
+
+		evt_param = fw_ptr;
+		fw_ptr += evt->plen;
+		remain -= evt->plen;
+	}
+
+	if (!evt || !evt_param || remain < 0) {
+		BT_ERR("%s Intel fw corrupted: invalid evt read", hdev->name);
+		return -1;
+	}
+
+	skb = __hci_cmd_sync_ev(hdev, le16_to_cpu(cmd->opcode),
+				cmd->plen, (void *)cmd_param, evt->evt,
+				HCI_INIT_TIMEOUT);
+	if (IS_ERR(skb)) {
+		BT_ERR("%s sending Intel patch command failed (%ld)",
+		       hdev->name, PTR_ERR(skb));
+		return -1;
+	}
+
+	/* Checking the returned event */
+	if (skb->len != evt->plen) {
+		BT_ERR("%s mismatch event length", hdev->name);
+		kfree_skb(skb);
+		return -1;
+	}
+
+	if (memcmp(skb->data, evt_param, evt->plen)) {
+		BT_ERR("%s mismatch event parameter", hdev->name);
+		kfree_skb(skb);
+		return -1;
+	}
+	kfree_skb(skb);
+
+	*curr_fw_ptr = fw_ptr;
+
+	return 0;
+}
+
+static int btusb_setup_intel(struct hci_dev *hdev)
+{
+	struct sk_buff *skb;
+	const struct firmware *fw;
+	const u8 *fw_ptr;
+	int disable_patch;
+	struct intel_version *ver;
+
+	u8 mfg_enable[] = { 0x01, 0x00 };
+	u8 mfg_disable[] = { 0x00, 0x00 };

Quoted 8 lines

drivers/bluetooth/btusb.c static int btusb_setup_bcm92035(struct hci_dev *hdev) · line 1115
Quoted 6 lines
drivers/bluetooth/btusb.c +6 -0
+	u8 mfg_reset_deactivate[] = { 0x00, 0x01 };
+	u8 mfg_reset_activate[] = { 0x00, 0x02 };
+
+	BT_DBG("%s", hdev->name);
+
+	/* The controller has a bug with the first HCI command send to it
s/send/sent/

ACK.

Quoted 17 lines

drivers/bluetooth/btusb.c +16 -0

+	 * returning number of completed commands as zero. This would stall the
+	 * command processing in the Bluetooth core
+	 *
+	 * As a workaround, send HCI Reset command first which will reset the
+	 * number of completed commands and allow normal command processing
+	 * from now on
+	 */
+	skb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL, HCI_INIT_TIMEOUT);
+	if (IS_ERR(skb)) {
+		BT_ERR("%s sending initial HCI reset command failed (%ld)",
+		       hdev->name, PTR_ERR(skb));
+		return -PTR_ERR(skb);
+	}
+	kfree_skb(skb);
+
+	/* Read Intel specific controller version first to allow selection of

Quoted 8 lines

drivers/bluetooth/btusb.c static int btusb_setup_bcm92035(struct hci_dev *hdev) · line 1137
Quoted 6 lines
drivers/bluetooth/btusb.c +6 -0
+	 * which firmware file to load.
+	 *
+	 * The returned information are hardware variant and revision plus
+	 * firmware variant, revision and build number.
+	 */
+	skb = __hci_cmd_sync(hdev, 0xfc05, 0, NULL, HCI_INIT_TIMEOUT);
Could you turn 0xfc05 into a macro that we can actually understand

My older version of patch used macro but I changed it later due to a comments from others. I will make a change back to use macro. I think that will make the code cleaner.

Quoted 13 lines

drivers/bluetooth/btusb.c static int btusb_setup_bcm92035(struct hci_dev *hdev) · line 1147
Quoted 11 lines
drivers/bluetooth/btusb.c +10 -0
+	if (IS_ERR(skb)) {
+		BT_ERR("%s reading Intel fw version command failed (%ld)",
+		       hdev->name, PTR_ERR(skb));
+		return -PTR_ERR(skb);
+	}
+
+	if (skb->len != sizeof(*ver)) {
+		BT_ERR("%s Intel version event length mismatch", hdev->name);
+		kfree_skb(skb);
+		return 0;
So why you return 0 here?

Quoted 6 lines

Orignially, I thought it would be ok to continue the initialization (BT init) even if it received invalid length or status. Actually, if it receives these errors, there must be something wrong in the device and should return with error.
I will make a change to return with proper error code.
However, please understand that there are a certain cases that I need to return 0 even if the fw patching is not completed.

I saw that, so please make sure to return error in he places where we really have a failure.

Gustavo

Marcel Holtmann

maintainer

2013-04-18 16:23 UTC #4 lore raw

In reply to Tedd Ho-Jeong An

Hi Tedd,

Quoted 295 lines

Quoted 131 lines

This patch adds support for Intel Bluetooth device by adding btusb_setup_intel() routine that update the device with ROM patch.

T:  Bus=02 Lev=02 Prnt=02 Port=00 Cnt=01 Dev#=  4 Spd=12   MxCh= 0
D:  Ver= 2.00 Cls=e0(wlcon) Sub=01 Prot=01 MxPS=64 #Cfgs=  1
P:  Vendor=8087 ProdID=07dc Rev= 0.01
C:* #Ifs= 2 Cfg#= 1 Atr=e0 MxPwr=100mA
I:* If#= 0 Alt= 0 #EPs= 3 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=81(I) Atr=03(Int.) MxPS=  64 Ivl=1ms
E:  Ad=02(O) Atr=02(Bulk) MxPS=  64 Ivl=0ms
E:  Ad=82(I) Atr=02(Bulk) MxPS=  64 Ivl=0ms
I:* If#= 1 Alt= 0 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=03(O) Atr=01(Isoc) MxPS=   0 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=   0 Ivl=1ms
I:  If#= 1 Alt= 1 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=03(O) Atr=01(Isoc) MxPS=   9 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=   9 Ivl=1ms
I:  If#= 1 Alt= 2 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=03(O) Atr=01(Isoc) MxPS=  17 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=  17 Ivl=1ms
I:  If#= 1 Alt= 3 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=03(O) Atr=01(Isoc) MxPS=  25 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=  25 Ivl=1ms
I:  If#= 1 Alt= 4 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=03(O) Atr=01(Isoc) MxPS=  33 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=  33 Ivl=1ms
I:  If#= 1 Alt= 5 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=03(O) Atr=01(Isoc) MxPS=  49 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=  49 Ivl=1ms

Signed-off-by Tedd Ho-Jeong An <[email protected]>

---
v4 changes
- refactored patching routin to separate function
- changed varaible type for status to u8
- corrected minor format issues

drivers/bluetooth/btusb.c | 366 +++++++++++++++++++++++++++++++++++++++++++++

1 file changed, 366 insertions(+)

drivers/bluetooth/btusb.c +62 -0

index 3d684d2..52c0ec0 100644
@@ -23,6 +23,7 @@
 
 #include <linux/module.h>
 #include <linux/usb.h>
+#include <linux/firmware.h>
 
 #include <net/bluetooth/bluetooth.h>
 #include <net/bluetooth/hci_core.h>

@@ -47,6 +48,7 @@ static struct usb_driver btusb_driver;
 #define BTUSB_BROKEN_ISOC	0x20
 #define BTUSB_WRONG_SCO_MTU	0x40
 #define BTUSB_ATH3012		0x80
+#define BTUSB_INTEL		0x100
 
 static struct usb_device_id btusb_table[] = {
 	/* Generic Bluetooth USB device */

@@ -207,6 +209,9 @@ static struct usb_device_id blacklist_table[] = {
 	/* Frontline ComProbe Bluetooth Sniffer */
 	{ USB_DEVICE(0x16d3, 0x0002), .driver_info = BTUSB_SNIFFER },
 
+	/* Intel Bluetooth device */
+	{ USB_DEVICE(0x8087, 0x07dc), .driver_info = BTUSB_INTEL },
+
 	{ }	/* Terminating entry */
 };

@@ -943,6 +948,364 @@ static int btusb_setup_bcm92035(struct hci_dev *hdev)
 	return 0;
 }
 
+struct intel_version {
+	u8	status;
+	u8	hw_platform;
+	u8	hw_variant;
+	u8	hw_revision;
+	u8	fw_variant;
+	u8	fw_revision;
+	u8	fw_build_num;
+	u8	fw_build_ww;
+	u8	fw_build_yy;
+	u8	fw_patch_num;
+} __packed;
+
+static const struct firmware *btusb_setup_intel_get_fw(struct hci_dev *hdev,
+						struct intel_version *ver)
+{
+	const struct firmware *fw;
+	char fwname[64];
+	int ret;
+
+	snprintf(fwname, sizeof(fwname),
+		 "intel/ibt-hw-%x.%x.%x-fw-%x.%x.%x.%x.%x.bseq",
+		 ver->hw_platform, ver->hw_variant, ver->hw_revision,
+		 ver->fw_variant,  ver->fw_revision, ver->fw_build_num,
+		 ver->fw_build_ww, ver->fw_build_yy);
+
+	ret = request_firmware(&fw, fwname, &hdev->dev);
+	if (ret < 0) {
+		if (ret == -EINVAL) {
+			BT_ERR("%s Intel firmware file request failed (%d)",
+			       hdev->name, ret);
+			return NULL;
+		}
+
+		BT_ERR("%s failed to open Intel firmware file: %s(%d)",
+		       hdev->name, fwname, ret);
+
+		snprintf(fwname, sizeof(fwname), "intel/ibt-default-%x.%x.bseq",
+			 ver->hw_platform, ver->hw_variant);
+		if (request_firmware(&fw, fwname, &hdev->dev) < 0) {
+			BT_ERR("%s failed to open default Intel fw file: %s",
+			       hdev->name, fwname);
+			return NULL;
+		}
+	}
+
+	BT_INFO("%s Intel Bluetooth firmware file: %s", hdev->name, fwname);
+
+	return fw;
+}
+
+static int btusb_setup_intel_patching(struct hci_dev *hdev,
+				      const struct firmware *fw,
+				      const u8 **curr_fw_ptr,
+				      int *disable_patch)
+{
+	struct sk_buff *skb;

Quoted 8 lines

drivers/bluetooth/btusb.c static int btusb_setup_bcm92035(struct hci_dev *hdev) · line 1012
Quoted 5 lines
drivers/bluetooth/btusb.c +5 -0
+	struct hci_command_hdr *cmd;
+	const u8 *cmd_param;
+	struct hci_event_hdr *evt = NULL;
+	const u8 *evt_param = NULL;
+	const u8 *fw_ptr = *curr_fw_ptr;
Why do you need an extra var to keep fw_ptr? can't you just use curr_fw_ptr directly?

Yes. I can. I will make a change.

Quoted 8 lines

drivers/bluetooth/btusb.c static int btusb_setup_bcm92035(struct hci_dev *hdev) · line 1017
Quoted 6 lines
drivers/bluetooth/btusb.c +5 -0
+	int remain = fw->size - (fw_ptr - fw->data);
+
+	/* Read command */
+	if (remain > HCI_COMMAND_HDR_SIZE &&
+	    fw_ptr[0] != 0x01) {
Apparently you don't need to break line on the if.

ACK.

Quoted 5 lines

drivers/bluetooth/btusb.c static int btusb_setup_bcm92035(struct hci_dev *hdev) · line 1019
Quoted 3 lines
drivers/bluetooth/btusb.c +2 -0
+		BT_ERR("%s Intel fw corrupted: invalid cmd read", hdev->name);
+		return -1;
Please return a proper error code.

ACK.

Quoted 15 lines

drivers/bluetooth/btusb.c static int btusb_setup_bcm92035(struct hci_dev *hdev) · line 1031
Quoted 13 lines
drivers/bluetooth/btusb.c +12 -0
+	}
+	fw_ptr++;
+	remain--;
+
+	cmd = (struct hci_command_hdr *)fw_ptr;
+	fw_ptr += sizeof(*cmd);
+	remain -= sizeof(*cmd);
+
+	/* checking the length */
+	if (remain < cmd->plen) {
+		BT_ERR("%s Intel fw corrupted: invalid cmd len", hdev->name);
+		return -1;
Same here and all other places you return -1

ACK.

Quoted 79 lines

drivers/bluetooth/btusb.c +78 -0

+	}
+
+	/* If there is a command that loads a patch in the firmware
+	 * file, then enable the patch upon success, otherwise just
+	 * disable the manufacturer mode, for example patch activation
+	 * is not required when the default firmware patch file is used
+	 * because there are no patch data to load.
+	 */
+	if (*disable_patch && cmd->opcode == 0xfc8e)
+		*disable_patch = 0;
+
+	cmd_param = fw_ptr;
+	fw_ptr += cmd->plen;
+	remain -= cmd->plen;
+
+	/* Read event */
+	while (remain > HCI_EVENT_HDR_SIZE && fw_ptr[0] == 0x02) {
+		fw_ptr++;
+		remain--;
+
+		evt = (struct hci_event_hdr *)fw_ptr;
+		fw_ptr += sizeof(*evt);
+		remain -= sizeof(*evt);
+
+		if (remain < evt->plen) {
+			BT_ERR("%s Intel fw corrupted: invalid evt len",
+			       hdev->name);
+			return -1;
+		}
+
+		evt_param = fw_ptr;
+		fw_ptr += evt->plen;
+		remain -= evt->plen;
+	}
+
+	if (!evt || !evt_param || remain < 0) {
+		BT_ERR("%s Intel fw corrupted: invalid evt read", hdev->name);
+		return -1;
+	}
+
+	skb = __hci_cmd_sync_ev(hdev, le16_to_cpu(cmd->opcode),
+				cmd->plen, (void *)cmd_param, evt->evt,
+				HCI_INIT_TIMEOUT);
+	if (IS_ERR(skb)) {
+		BT_ERR("%s sending Intel patch command failed (%ld)",
+		       hdev->name, PTR_ERR(skb));
+		return -1;
+	}
+
+	/* Checking the returned event */
+	if (skb->len != evt->plen) {
+		BT_ERR("%s mismatch event length", hdev->name);
+		kfree_skb(skb);
+		return -1;
+	}
+
+	if (memcmp(skb->data, evt_param, evt->plen)) {
+		BT_ERR("%s mismatch event parameter", hdev->name);
+		kfree_skb(skb);
+		return -1;
+	}
+	kfree_skb(skb);
+
+	*curr_fw_ptr = fw_ptr;
+
+	return 0;
+}
+
+static int btusb_setup_intel(struct hci_dev *hdev)
+{
+	struct sk_buff *skb;
+	const struct firmware *fw;
+	const u8 *fw_ptr;
+	int disable_patch;
+	struct intel_version *ver;
+
+	u8 mfg_enable[] = { 0x01, 0x00 };
+	u8 mfg_disable[] = { 0x00, 0x00 };

Quoted 8 lines

drivers/bluetooth/btusb.c static int btusb_setup_bcm92035(struct hci_dev *hdev) · line 1115
Quoted 6 lines
drivers/bluetooth/btusb.c +6 -0
+	u8 mfg_reset_deactivate[] = { 0x00, 0x01 };
+	u8 mfg_reset_activate[] = { 0x00, 0x02 };
+
+	BT_DBG("%s", hdev->name);
+
+	/* The controller has a bug with the first HCI command send to it
s/send/sent/

ACK.

Quoted 17 lines

drivers/bluetooth/btusb.c +16 -0

+	 * returning number of completed commands as zero. This would stall the
+	 * command processing in the Bluetooth core
+	 *
+	 * As a workaround, send HCI Reset command first which will reset the
+	 * number of completed commands and allow normal command processing
+	 * from now on
+	 */
+	skb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL, HCI_INIT_TIMEOUT);
+	if (IS_ERR(skb)) {
+		BT_ERR("%s sending initial HCI reset command failed (%ld)",
+		       hdev->name, PTR_ERR(skb));
+		return -PTR_ERR(skb);
+	}
+	kfree_skb(skb);
+
+	/* Read Intel specific controller version first to allow selection of

Quoted 8 lines

drivers/bluetooth/btusb.c static int btusb_setup_bcm92035(struct hci_dev *hdev) · line 1137
Quoted 6 lines
drivers/bluetooth/btusb.c +6 -0
+	 * which firmware file to load.
+	 *
+	 * The returned information are hardware variant and revision plus
+	 * firmware variant, revision and build number.
+	 */
+	skb = __hci_cmd_sync(hdev, 0xfc05, 0, NULL, HCI_INIT_TIMEOUT);
Could you turn 0xfc05 into a macro that we can actually understand

Quoted 2 lines

My older version of patch used macro but I changed it later due to a comments from others. I will make a change back to use macro. I think that will make the code cleaner.

please leave this as 0xfc05. The comment above is enough. That is why I asked for the comment to put above to explain what this command does.

There is no real point in creating a macro for this. We will not be releasing the vendor command specification any time soon. So even if you call this HCI_INTEL_FOO it has no extra meaning to anybody reading this. The comment above is way more helpful for anybody who has to review this code then a made up macro.

And in this specific cases, the macro forces a line split and makes the code even less readable without adding any extra value for the reader.

Gustavo, feel free to check my earlier reviews.

Regards

Marcel

Gustavo Padovan

2013-04-18 16:28 UTC #5 lore raw

Hi Tedd,

Quoted 297 lines · Marcel Holtmann

Hi Tedd,

Quoted 295 lines

Quoted 131 lines

This patch adds support for Intel Bluetooth device by adding btusb_setup_intel() routine that update the device with ROM patch.

T:  Bus=02 Lev=02 Prnt=02 Port=00 Cnt=01 Dev#=  4 Spd=12   MxCh= 0
D:  Ver= 2.00 Cls=e0(wlcon) Sub=01 Prot=01 MxPS=64 #Cfgs=  1
P:  Vendor=8087 ProdID=07dc Rev= 0.01
C:* #Ifs= 2 Cfg#= 1 Atr=e0 MxPwr=100mA
I:* If#= 0 Alt= 0 #EPs= 3 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=81(I) Atr=03(Int.) MxPS=  64 Ivl=1ms
E:  Ad=02(O) Atr=02(Bulk) MxPS=  64 Ivl=0ms
E:  Ad=82(I) Atr=02(Bulk) MxPS=  64 Ivl=0ms
I:* If#= 1 Alt= 0 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=03(O) Atr=01(Isoc) MxPS=   0 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=   0 Ivl=1ms
I:  If#= 1 Alt= 1 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=03(O) Atr=01(Isoc) MxPS=   9 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=   9 Ivl=1ms
I:  If#= 1 Alt= 2 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=03(O) Atr=01(Isoc) MxPS=  17 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=  17 Ivl=1ms
I:  If#= 1 Alt= 3 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=03(O) Atr=01(Isoc) MxPS=  25 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=  25 Ivl=1ms
I:  If#= 1 Alt= 4 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=03(O) Atr=01(Isoc) MxPS=  33 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=  33 Ivl=1ms
I:  If#= 1 Alt= 5 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=03(O) Atr=01(Isoc) MxPS=  49 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=  49 Ivl=1ms

Signed-off-by Tedd Ho-Jeong An <[email protected]>

---
v4 changes
- refactored patching routin to separate function
- changed varaible type for status to u8
- corrected minor format issues

drivers/bluetooth/btusb.c | 366 +++++++++++++++++++++++++++++++++++++++++++++

1 file changed, 366 insertions(+)

drivers/bluetooth/btusb.c +62 -0

index 3d684d2..52c0ec0 100644
@@ -23,6 +23,7 @@
 
 #include <linux/module.h>
 #include <linux/usb.h>
+#include <linux/firmware.h>
 
 #include <net/bluetooth/bluetooth.h>
 #include <net/bluetooth/hci_core.h>

@@ -47,6 +48,7 @@ static struct usb_driver btusb_driver;
 #define BTUSB_BROKEN_ISOC	0x20
 #define BTUSB_WRONG_SCO_MTU	0x40
 #define BTUSB_ATH3012		0x80
+#define BTUSB_INTEL		0x100
 
 static struct usb_device_id btusb_table[] = {
 	/* Generic Bluetooth USB device */

@@ -207,6 +209,9 @@ static struct usb_device_id blacklist_table[] = {
 	/* Frontline ComProbe Bluetooth Sniffer */
 	{ USB_DEVICE(0x16d3, 0x0002), .driver_info = BTUSB_SNIFFER },
 
+	/* Intel Bluetooth device */
+	{ USB_DEVICE(0x8087, 0x07dc), .driver_info = BTUSB_INTEL },
+
 	{ }	/* Terminating entry */
 };

@@ -943,6 +948,364 @@ static int btusb_setup_bcm92035(struct hci_dev *hdev)
 	return 0;
 }
 
+struct intel_version {
+	u8	status;
+	u8	hw_platform;
+	u8	hw_variant;
+	u8	hw_revision;
+	u8	fw_variant;
+	u8	fw_revision;
+	u8	fw_build_num;
+	u8	fw_build_ww;
+	u8	fw_build_yy;
+	u8	fw_patch_num;
+} __packed;
+
+static const struct firmware *btusb_setup_intel_get_fw(struct hci_dev *hdev,
+						struct intel_version *ver)
+{
+	const struct firmware *fw;
+	char fwname[64];
+	int ret;
+
+	snprintf(fwname, sizeof(fwname),
+		 "intel/ibt-hw-%x.%x.%x-fw-%x.%x.%x.%x.%x.bseq",
+		 ver->hw_platform, ver->hw_variant, ver->hw_revision,
+		 ver->fw_variant,  ver->fw_revision, ver->fw_build_num,
+		 ver->fw_build_ww, ver->fw_build_yy);
+
+	ret = request_firmware(&fw, fwname, &hdev->dev);
+	if (ret < 0) {
+		if (ret == -EINVAL) {
+			BT_ERR("%s Intel firmware file request failed (%d)",
+			       hdev->name, ret);
+			return NULL;
+		}
+
+		BT_ERR("%s failed to open Intel firmware file: %s(%d)",
+		       hdev->name, fwname, ret);
+
+		snprintf(fwname, sizeof(fwname), "intel/ibt-default-%x.%x.bseq",
+			 ver->hw_platform, ver->hw_variant);
+		if (request_firmware(&fw, fwname, &hdev->dev) < 0) {
+			BT_ERR("%s failed to open default Intel fw file: %s",
+			       hdev->name, fwname);
+			return NULL;
+		}
+	}
+
+	BT_INFO("%s Intel Bluetooth firmware file: %s", hdev->name, fwname);
+
+	return fw;
+}
+
+static int btusb_setup_intel_patching(struct hci_dev *hdev,
+				      const struct firmware *fw,
+				      const u8 **curr_fw_ptr,
+				      int *disable_patch)
+{
+	struct sk_buff *skb;

Quoted 8 lines

drivers/bluetooth/btusb.c static int btusb_setup_bcm92035(struct hci_dev *hdev) · line 1012
Quoted 5 lines
drivers/bluetooth/btusb.c +5 -0
+	struct hci_command_hdr *cmd;
+	const u8 *cmd_param;
+	struct hci_event_hdr *evt = NULL;
+	const u8 *evt_param = NULL;
+	const u8 *fw_ptr = *curr_fw_ptr;
Why do you need an extra var to keep fw_ptr? can't you just use curr_fw_ptr directly?

Yes. I can. I will make a change.

Quoted 8 lines

drivers/bluetooth/btusb.c static int btusb_setup_bcm92035(struct hci_dev *hdev) · line 1017
Quoted 6 lines
drivers/bluetooth/btusb.c +5 -0
+	int remain = fw->size - (fw_ptr - fw->data);
+
+	/* Read command */
+	if (remain > HCI_COMMAND_HDR_SIZE &&
+	    fw_ptr[0] != 0x01) {
Apparently you don't need to break line on the if.

ACK.

Quoted 5 lines

drivers/bluetooth/btusb.c static int btusb_setup_bcm92035(struct hci_dev *hdev) · line 1019
Quoted 3 lines
drivers/bluetooth/btusb.c +2 -0
+		BT_ERR("%s Intel fw corrupted: invalid cmd read", hdev->name);
+		return -1;
Please return a proper error code.

ACK.

Quoted 15 lines

drivers/bluetooth/btusb.c static int btusb_setup_bcm92035(struct hci_dev *hdev) · line 1031
Quoted 13 lines
drivers/bluetooth/btusb.c +12 -0
+	}
+	fw_ptr++;
+	remain--;
+
+	cmd = (struct hci_command_hdr *)fw_ptr;
+	fw_ptr += sizeof(*cmd);
+	remain -= sizeof(*cmd);
+
+	/* checking the length */
+	if (remain < cmd->plen) {
+		BT_ERR("%s Intel fw corrupted: invalid cmd len", hdev->name);
+		return -1;
Same here and all other places you return -1

ACK.

Quoted 79 lines

drivers/bluetooth/btusb.c +78 -0

+	}
+
+	/* If there is a command that loads a patch in the firmware
+	 * file, then enable the patch upon success, otherwise just
+	 * disable the manufacturer mode, for example patch activation
+	 * is not required when the default firmware patch file is used
+	 * because there are no patch data to load.
+	 */
+	if (*disable_patch && cmd->opcode == 0xfc8e)
+		*disable_patch = 0;
+
+	cmd_param = fw_ptr;
+	fw_ptr += cmd->plen;
+	remain -= cmd->plen;
+
+	/* Read event */
+	while (remain > HCI_EVENT_HDR_SIZE && fw_ptr[0] == 0x02) {
+		fw_ptr++;
+		remain--;
+
+		evt = (struct hci_event_hdr *)fw_ptr;
+		fw_ptr += sizeof(*evt);
+		remain -= sizeof(*evt);
+
+		if (remain < evt->plen) {
+			BT_ERR("%s Intel fw corrupted: invalid evt len",
+			       hdev->name);
+			return -1;
+		}
+
+		evt_param = fw_ptr;
+		fw_ptr += evt->plen;
+		remain -= evt->plen;
+	}
+
+	if (!evt || !evt_param || remain < 0) {
+		BT_ERR("%s Intel fw corrupted: invalid evt read", hdev->name);
+		return -1;
+	}
+
+	skb = __hci_cmd_sync_ev(hdev, le16_to_cpu(cmd->opcode),
+				cmd->plen, (void *)cmd_param, evt->evt,
+				HCI_INIT_TIMEOUT);
+	if (IS_ERR(skb)) {
+		BT_ERR("%s sending Intel patch command failed (%ld)",
+		       hdev->name, PTR_ERR(skb));
+		return -1;
+	}
+
+	/* Checking the returned event */
+	if (skb->len != evt->plen) {
+		BT_ERR("%s mismatch event length", hdev->name);
+		kfree_skb(skb);
+		return -1;
+	}
+
+	if (memcmp(skb->data, evt_param, evt->plen)) {
+		BT_ERR("%s mismatch event parameter", hdev->name);
+		kfree_skb(skb);
+		return -1;
+	}
+	kfree_skb(skb);
+
+	*curr_fw_ptr = fw_ptr;
+
+	return 0;
+}
+
+static int btusb_setup_intel(struct hci_dev *hdev)
+{
+	struct sk_buff *skb;
+	const struct firmware *fw;
+	const u8 *fw_ptr;
+	int disable_patch;
+	struct intel_version *ver;
+
+	u8 mfg_enable[] = { 0x01, 0x00 };
+	u8 mfg_disable[] = { 0x00, 0x00 };

Quoted 8 lines

drivers/bluetooth/btusb.c static int btusb_setup_bcm92035(struct hci_dev *hdev) · line 1115
Quoted 6 lines
drivers/bluetooth/btusb.c +6 -0
+	u8 mfg_reset_deactivate[] = { 0x00, 0x01 };
+	u8 mfg_reset_activate[] = { 0x00, 0x02 };
+
+	BT_DBG("%s", hdev->name);
+
+	/* The controller has a bug with the first HCI command send to it
s/send/sent/

ACK.

Quoted 25 lines

Quoted 13 lines

drivers/bluetooth/btusb.c +12 -0

+	 * returning number of completed commands as zero. This would stall the
+	 * command processing in the Bluetooth core
+	 *
+	 * As a workaround, send HCI Reset command first which will reset the
+	 * number of completed commands and allow normal command processing
+	 * from now on
+	 */
+	skb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL, HCI_INIT_TIMEOUT);
+	if (IS_ERR(skb)) {
+		BT_ERR("%s sending initial HCI reset command failed (%ld)",
+		       hdev->name, PTR_ERR(skb));
+		return -PTR_ERR(skb);

drivers/bluetooth/btusb.c static int btusb_setup_bcm92035(struct hci_dev *hdev) · line 1137

Quoted 10 lines

drivers/bluetooth/btusb.c +10 -0

+	}
+	kfree_skb(skb);
+
+	/* Read Intel specific controller version first to allow selection of
+	 * which firmware file to load.
+	 *
+	 * The returned information are hardware variant and revision plus
+	 * firmware variant, revision and build number.
+	 */
+	skb = __hci_cmd_sync(hdev, 0xfc05, 0, NULL, HCI_INIT_TIMEOUT);

Could you turn 0xfc05 into a macro that we can actually understand

Quoted 8 lines

Quoted 2 lines
My older version of patch used macro but I changed it later due to a comments from others. I will make a change back to use macro. I think that will make the code cleaner.
please leave this as 0xfc05. The comment above is enough. That is why I asked for the comment to put above to explain what this command does.
There is no real point in creating a macro for this. We will not be releasing the vendor command specification any time soon. So even if you call this HCI_INTEL_FOO it has no extra meaning to anybody reading this. The comment above is way more helpful for anybody who has to review this code then a made up macro.
And in this specific cases, the macro forces a line split and makes the code even less readable without adding any extra value for the reader.

Okay, so please go with this approach, you can let the cmds code directly in the code.

Gustavo

Tedd Ho-Jeong An

author

2013-04-18 18:29 UTC #6 lore raw

Hi, Gustavo.

I just sent out the V5 after updating with your comments.

Tedd

Quoted 313 lines · Gustavo Padovan

Hi Tedd,

Quoted 297 lines · Marcel Holtmann

Hi Tedd,

Quoted 295 lines

Quoted 131 lines

This patch adds support for Intel Bluetooth device by adding btusb_setup_intel() routine that update the device with ROM patch.

T:  Bus=02 Lev=02 Prnt=02 Port=00 Cnt=01 Dev#=  4 Spd=12   MxCh= 0
D:  Ver= 2.00 Cls=e0(wlcon) Sub=01 Prot=01 MxPS=64 #Cfgs=  1
P:  Vendor=8087 ProdID=07dc Rev= 0.01
C:* #Ifs= 2 Cfg#= 1 Atr=e0 MxPwr=100mA
I:* If#= 0 Alt= 0 #EPs= 3 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=81(I) Atr=03(Int.) MxPS=  64 Ivl=1ms
E:  Ad=02(O) Atr=02(Bulk) MxPS=  64 Ivl=0ms
E:  Ad=82(I) Atr=02(Bulk) MxPS=  64 Ivl=0ms
I:* If#= 1 Alt= 0 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=03(O) Atr=01(Isoc) MxPS=   0 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=   0 Ivl=1ms
I:  If#= 1 Alt= 1 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=03(O) Atr=01(Isoc) MxPS=   9 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=   9 Ivl=1ms
I:  If#= 1 Alt= 2 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=03(O) Atr=01(Isoc) MxPS=  17 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=  17 Ivl=1ms
I:  If#= 1 Alt= 3 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=03(O) Atr=01(Isoc) MxPS=  25 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=  25 Ivl=1ms
I:  If#= 1 Alt= 4 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=03(O) Atr=01(Isoc) MxPS=  33 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=  33 Ivl=1ms
I:  If#= 1 Alt= 5 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=03(O) Atr=01(Isoc) MxPS=  49 Ivl=1ms
E:  Ad=83(I) Atr=01(Isoc) MxPS=  49 Ivl=1ms

Signed-off-by Tedd Ho-Jeong An <[email protected]>

---
v4 changes
- refactored patching routin to separate function
- changed varaible type for status to u8
- corrected minor format issues

drivers/bluetooth/btusb.c | 366 +++++++++++++++++++++++++++++++++++++++++++++

1 file changed, 366 insertions(+)

drivers/bluetooth/btusb.c +62 -0

index 3d684d2..52c0ec0 100644
@@ -23,6 +23,7 @@
 
 #include <linux/module.h>
 #include <linux/usb.h>
+#include <linux/firmware.h>
 
 #include <net/bluetooth/bluetooth.h>
 #include <net/bluetooth/hci_core.h>

@@ -47,6 +48,7 @@ static struct usb_driver btusb_driver;
 #define BTUSB_BROKEN_ISOC	0x20
 #define BTUSB_WRONG_SCO_MTU	0x40
 #define BTUSB_ATH3012		0x80
+#define BTUSB_INTEL		0x100
 
 static struct usb_device_id btusb_table[] = {
 	/* Generic Bluetooth USB device */

@@ -207,6 +209,9 @@ static struct usb_device_id blacklist_table[] = {
 	/* Frontline ComProbe Bluetooth Sniffer */
 	{ USB_DEVICE(0x16d3, 0x0002), .driver_info = BTUSB_SNIFFER },
 
+	/* Intel Bluetooth device */
+	{ USB_DEVICE(0x8087, 0x07dc), .driver_info = BTUSB_INTEL },
+
 	{ }	/* Terminating entry */
 };

@@ -943,6 +948,364 @@ static int btusb_setup_bcm92035(struct hci_dev *hdev)
 	return 0;
 }
 
+struct intel_version {
+	u8	status;
+	u8	hw_platform;
+	u8	hw_variant;
+	u8	hw_revision;
+	u8	fw_variant;
+	u8	fw_revision;
+	u8	fw_build_num;
+	u8	fw_build_ww;
+	u8	fw_build_yy;
+	u8	fw_patch_num;
+} __packed;
+
+static const struct firmware *btusb_setup_intel_get_fw(struct hci_dev *hdev,
+						struct intel_version *ver)
+{
+	const struct firmware *fw;
+	char fwname[64];
+	int ret;
+
+	snprintf(fwname, sizeof(fwname),
+		 "intel/ibt-hw-%x.%x.%x-fw-%x.%x.%x.%x.%x.bseq",
+		 ver->hw_platform, ver->hw_variant, ver->hw_revision,
+		 ver->fw_variant,  ver->fw_revision, ver->fw_build_num,
+		 ver->fw_build_ww, ver->fw_build_yy);
+
+	ret = request_firmware(&fw, fwname, &hdev->dev);
+	if (ret < 0) {
+		if (ret == -EINVAL) {
+			BT_ERR("%s Intel firmware file request failed (%d)",
+			       hdev->name, ret);
+			return NULL;
+		}
+
+		BT_ERR("%s failed to open Intel firmware file: %s(%d)",
+		       hdev->name, fwname, ret);
+
+		snprintf(fwname, sizeof(fwname), "intel/ibt-default-%x.%x.bseq",
+			 ver->hw_platform, ver->hw_variant);
+		if (request_firmware(&fw, fwname, &hdev->dev) < 0) {
+			BT_ERR("%s failed to open default Intel fw file: %s",
+			       hdev->name, fwname);
+			return NULL;
+		}
+	}
+
+	BT_INFO("%s Intel Bluetooth firmware file: %s", hdev->name, fwname);
+
+	return fw;
+}
+
+static int btusb_setup_intel_patching(struct hci_dev *hdev,
+				      const struct firmware *fw,
+				      const u8 **curr_fw_ptr,
+				      int *disable_patch)
+{
+	struct sk_buff *skb;

Quoted 8 lines

drivers/bluetooth/btusb.c static int btusb_setup_bcm92035(struct hci_dev *hdev) · line 1012
Quoted 5 lines
drivers/bluetooth/btusb.c +5 -0
+	struct hci_command_hdr *cmd;
+	const u8 *cmd_param;
+	struct hci_event_hdr *evt = NULL;
+	const u8 *evt_param = NULL;
+	const u8 *fw_ptr = *curr_fw_ptr;
Why do you need an extra var to keep fw_ptr? can't you just use curr_fw_ptr directly?

Yes. I can. I will make a change.

Quoted 8 lines

drivers/bluetooth/btusb.c static int btusb_setup_bcm92035(struct hci_dev *hdev) · line 1017
Quoted 6 lines
drivers/bluetooth/btusb.c +5 -0
+	int remain = fw->size - (fw_ptr - fw->data);
+
+	/* Read command */
+	if (remain > HCI_COMMAND_HDR_SIZE &&
+	    fw_ptr[0] != 0x01) {
Apparently you don't need to break line on the if.

ACK.

Quoted 5 lines

drivers/bluetooth/btusb.c static int btusb_setup_bcm92035(struct hci_dev *hdev) · line 1019
Quoted 3 lines
drivers/bluetooth/btusb.c +2 -0
+		BT_ERR("%s Intel fw corrupted: invalid cmd read", hdev->name);
+		return -1;
Please return a proper error code.

ACK.

Quoted 15 lines

drivers/bluetooth/btusb.c static int btusb_setup_bcm92035(struct hci_dev *hdev) · line 1031
Quoted 13 lines
drivers/bluetooth/btusb.c +12 -0
+	}
+	fw_ptr++;
+	remain--;
+
+	cmd = (struct hci_command_hdr *)fw_ptr;
+	fw_ptr += sizeof(*cmd);
+	remain -= sizeof(*cmd);
+
+	/* checking the length */
+	if (remain < cmd->plen) {
+		BT_ERR("%s Intel fw corrupted: invalid cmd len", hdev->name);
+		return -1;
Same here and all other places you return -1

ACK.

Quoted 79 lines

drivers/bluetooth/btusb.c +78 -0

+	}
+
+	/* If there is a command that loads a patch in the firmware
+	 * file, then enable the patch upon success, otherwise just
+	 * disable the manufacturer mode, for example patch activation
+	 * is not required when the default firmware patch file is used
+	 * because there are no patch data to load.
+	 */
+	if (*disable_patch && cmd->opcode == 0xfc8e)
+		*disable_patch = 0;
+
+	cmd_param = fw_ptr;
+	fw_ptr += cmd->plen;
+	remain -= cmd->plen;
+
+	/* Read event */
+	while (remain > HCI_EVENT_HDR_SIZE && fw_ptr[0] == 0x02) {
+		fw_ptr++;
+		remain--;
+
+		evt = (struct hci_event_hdr *)fw_ptr;
+		fw_ptr += sizeof(*evt);
+		remain -= sizeof(*evt);
+
+		if (remain < evt->plen) {
+			BT_ERR("%s Intel fw corrupted: invalid evt len",
+			       hdev->name);
+			return -1;
+		}
+
+		evt_param = fw_ptr;
+		fw_ptr += evt->plen;
+		remain -= evt->plen;
+	}
+
+	if (!evt || !evt_param || remain < 0) {
+		BT_ERR("%s Intel fw corrupted: invalid evt read", hdev->name);
+		return -1;
+	}
+
+	skb = __hci_cmd_sync_ev(hdev, le16_to_cpu(cmd->opcode),
+				cmd->plen, (void *)cmd_param, evt->evt,
+				HCI_INIT_TIMEOUT);
+	if (IS_ERR(skb)) {
+		BT_ERR("%s sending Intel patch command failed (%ld)",
+		       hdev->name, PTR_ERR(skb));
+		return -1;
+	}
+
+	/* Checking the returned event */
+	if (skb->len != evt->plen) {
+		BT_ERR("%s mismatch event length", hdev->name);
+		kfree_skb(skb);
+		return -1;
+	}
+
+	if (memcmp(skb->data, evt_param, evt->plen)) {
+		BT_ERR("%s mismatch event parameter", hdev->name);
+		kfree_skb(skb);
+		return -1;
+	}
+	kfree_skb(skb);
+
+	*curr_fw_ptr = fw_ptr;
+
+	return 0;
+}
+
+static int btusb_setup_intel(struct hci_dev *hdev)
+{
+	struct sk_buff *skb;
+	const struct firmware *fw;
+	const u8 *fw_ptr;
+	int disable_patch;
+	struct intel_version *ver;
+
+	u8 mfg_enable[] = { 0x01, 0x00 };
+	u8 mfg_disable[] = { 0x00, 0x00 };

Quoted 8 lines

drivers/bluetooth/btusb.c static int btusb_setup_bcm92035(struct hci_dev *hdev) · line 1115
Quoted 6 lines
drivers/bluetooth/btusb.c +6 -0
+	u8 mfg_reset_deactivate[] = { 0x00, 0x01 };
+	u8 mfg_reset_activate[] = { 0x00, 0x02 };
+
+	BT_DBG("%s", hdev->name);
+
+	/* The controller has a bug with the first HCI command send to it
s/send/sent/

ACK.

Quoted 25 lines

Quoted 13 lines

drivers/bluetooth/btusb.c +12 -0

+	 * returning number of completed commands as zero. This would stall the
+	 * command processing in the Bluetooth core
+	 *
+	 * As a workaround, send HCI Reset command first which will reset the
+	 * number of completed commands and allow normal command processing
+	 * from now on
+	 */
+	skb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL, HCI_INIT_TIMEOUT);
+	if (IS_ERR(skb)) {
+		BT_ERR("%s sending initial HCI reset command failed (%ld)",
+		       hdev->name, PTR_ERR(skb));
+		return -PTR_ERR(skb);

drivers/bluetooth/btusb.c static int btusb_setup_bcm92035(struct hci_dev *hdev) · line 1137

Quoted 10 lines

drivers/bluetooth/btusb.c +10 -0

+	}
+	kfree_skb(skb);
+
+	/* Read Intel specific controller version first to allow selection of
+	 * which firmware file to load.
+	 *
+	 * The returned information are hardware variant and revision plus
+	 * firmware variant, revision and build number.
+	 */
+	skb = __hci_cmd_sync(hdev, 0xfc05, 0, NULL, HCI_INIT_TIMEOUT);

Could you turn 0xfc05 into a macro that we can actually understand

Quoted 8 lines

Quoted 2 lines
My older version of patch used macro but I changed it later due to a comments from others. I will make a change back to use macro. I think that will make the code cleaner.
please leave this as 0xfc05. The comment above is enough. That is why I asked for the comment to put above to explain what this command does.
There is no real point in creating a macro for this. We will not be releasing the vendor command specification any time soon. So even if you call this HCI_INTEL_FOO it has no extra meaning to anybody reading this. The comment above is way more helpful for anybody who has to review this code then a made up macro.
And in this specific cases, the macro forces a line split and makes the code even less readable without adding any extra value for the reader.

Okay, so please go with this approach, you can let the cmds code directly in the code.

Gustavo

Signature · 3 lines

Regards
Tedd Ho-Jeong An
Intel Corporation