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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Tedd,

> 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 <tedd.an@xxxxxxxxx>
> ---
> changes v6
> - Added more comments
> - renamed the default firmware file
> - simplified the patching completion routine
> - print opcode if event fails
> - use sizeof() instead of raw number for parameter length
> - minor format fix
> 
> drivers/bluetooth/btusb.c |  379 +++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 379 insertions(+)
> 
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index 3d684d2..c0087cb 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -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,377 @@ 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;

I just realized that you might not need to use tabs here to align the names. You can just use one space in between. However here I am not even sure what the official net/ coding style is.

> +
> +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);
> +
> +		/* If the correct firmware patch file is not found, use the
> +		 * default firmware patch file instead
> +		 */
> +		snprintf(fwname, sizeof(fwname), "intel/ibt-hw-%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 **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;
> +	int remain = fw->size - (*fw_ptr - fw->data);
> +
> +	/* The first byte indicates the types of the patch command or event.
> +	 * 0x01 means HCI command and 0x02 is HCI event. If the first bytes
> +	 * in the current firmware buffer doesn't start with 0x01 or
> +	 * the size of remain buffer is smaller than HCI command header,
> +	 * the firmware file is corrupted and it should stop the patching
> +	 * process.
> +	 */
> +	if (remain > HCI_COMMAND_HDR_SIZE && *fw_ptr[0] != 0x01) {
> +		BT_ERR("%s Intel fw corrupted: invalid cmd read", hdev->name);
> +		return -EINVAL;
> +	}
> +	(*fw_ptr)++;
> +	remain--;
> +
> +	cmd = (struct hci_command_hdr *)(*fw_ptr);
> +	*fw_ptr += sizeof(*cmd);
> +	remain -= sizeof(*cmd);
> +
> +	/* Ensure that the remain firmware data is long enough than the length
> +	 * of command parameter. If not, the firmware file is corrupted.
> +	 */
> +	if (remain < cmd->plen) {
> +		BT_ERR("%s Intel fw corrupted: invalid cmd len", hdev->name);
> +		return -EFAULT;
> +	}
> +
> +	/* 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 && __constant_le16_to_cpu(cmd->opcode) == 0xfc8e)

This has to be the other way around.

		cmd->opcode == __constant_cpu_to_le16(0xfc8e)

Or you need to use le16_to_cpu(cmd->opcode) like you do below. It is just an optimization to use the constant version on a real constant.

> +		*disable_patch = 0;
> +
> +	cmd_param = *fw_ptr;
> +	*fw_ptr += cmd->plen;
> +	remain -= cmd->plen;
> +
> +	/* This reads the expected events when the above command is sent to the
> +	 * device. Some vendor commands expects more than one events, for
> +	 * example command status event followed by vendor specific event.
> +	 * For this case, it only keeps the last expected event. so the command
> +	 * can be sent with __hci_cmd_sync_ev() which returns the sk_buff of
> +	 * last expected 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 -EFAULT;
> +		}
> +
> +		evt_param = *fw_ptr;
> +		*fw_ptr += evt->plen;
> +		remain -= evt->plen;
> +	}
> +
> +	/* Every HCI commands in the firmware file has its correspond event.
> +	 * If event is not found or remain is smaller than zero, the firmware
> +	 * file is corrupted.
> +	 */
> +	if (!evt || !evt_param || remain < 0) {
> +		BT_ERR("%s Intel fw corrupted: invalid evt read", hdev->name);
> +		return -EFAULT;
> +	}
> +
> +	skb = __hci_cmd_sync_ev(hdev, le16_to_cpu(cmd->opcode), cmd->plen,
> +				(void *)cmd_param, evt->evt, HCI_INIT_TIMEOUT);

Is this (void *) casting really needed. The parameter is void already. So it should not be needed at a all. If it throws a compiler warning please let us know which one. Same goes for the others where you cast.

> +	if (IS_ERR(skb)) {
> +		BT_ERR("%s sending Intel patch command (0x%4.4x) failed (%ld)",
> +		       hdev->name, cmd->opcode, PTR_ERR(skb));
> +		return -PTR_ERR(skb);
> +	}
> +
> +	/* It ensures that the returned event matches the event data read from
> +	 * the firmware file. At fist, it checks the length and then
> +	 * the contents of the event.
> +	 */
> +	if (skb->len != evt->plen) {
> +		BT_ERR("%s mismatch event length (opcode 0x%4.4x)", hdev->name,
> +		       le16_to_cpu(cmd->opcode));
> +		kfree_skb(skb);
> +		return -EFAULT;
> +	}
> +
> +	if (memcmp(skb->data, evt_param, evt->plen)) {
> +		BT_ERR("%s mismatch event parameter (opcode 0x%4.4x)",
> +		       hdev->name, le16_to_cpu(cmd->opcode));
> +		kfree_skb(skb);
> +		return -EFAULT;
> +	}
> +	kfree_skb(skb);
> +
> +	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 };

Since the patch from Johan got applied to bluetooth-next, please make these const.

> +
> +	BT_DBG("%s", hdev->name);
> +
> +	/* The controller has a bug with the first HCI command sent 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 -EIO;
> +	}
> +
> +	ver = (struct intel_version *)skb->data;
> +	if (ver->status) {
> +		BT_ERR("%s Intel fw version event failed (%02x)", hdev->name,
> +		       ver->status);
> +		kfree_skb(skb);
> +		return -bt_to_errno(ver->status);
> +	}
> +
> +	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);
> +
> +	/* fw_patch_num indicates the version of patch the device currently
> +	 * have. If there is no patch data in the device, it is always 0x00.
> +	 * So, if it is other than 0x00, no need to patch the deivce again.
> +	 */
> +	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 fails to open the matching firmware
> +	 * patch file, it tries to open the default firmware patch file.
> +	 * If no patch file is found, 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 fails, 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 (fw->size > fw_ptr - fw->data) {
> +		int ret;
> +
> +		ret = btusb_setup_intel_patching(hdev, fw, &fw_ptr,
> +						 &disable_patch);
> +		if (ret < 0)
> +			goto exit_mfg_deactivate;
> +	}
> +
> +	release_firmware(fw);
> +
> +	if (disable_patch)
> +		goto exit_mfg_disable;
> +
> +	/* Patching completed successfully and disable the manufacturer mode
> +	 * with reset and activate the downloaded firmware patches.
> +	 */
> +	skb = __hci_cmd_sync(hdev, 0xfc11, sizeof(mfg_reset_activate),
> +			     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_disable:
> +

Remove this empty line above.

> +	/* Disable the manufacturer mode without reset */
> +	skb = __hci_cmd_sync(hdev, 0xfc11, sizeof(mfg_disable), 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;
> +
> +exit_mfg_deactivate:
> +	release_firmware(fw);
> +
> +	/* Patching failed. Disable the manufacturer mode with reset and
> +	 * deactivate the downloaded firmware patches.
> +	 */
> +	skb = __hci_cmd_sync(hdev, 0xfc11, sizeof(mfg_reset_deactivate),
> +			     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;
> +}
> +
> static int btusb_probe(struct usb_interface *intf,
> 				const struct usb_device_id *id)
> {
> @@ -1048,6 +1424,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);

Regards

Marcel

--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux