All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] Bluetooth: btusb: Add Realtek 8723A/8723B/8761A/8821A support
@ 2015-02-09 22:59 Daniel Drake
  2015-02-10  0:06 ` Larry Finger
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Daniel Drake @ 2015-02-09 22:59 UTC (permalink / raw)
  To: marcel, gustavo, johan.hedberg
  Cc: linux-bluetooth, larry.finger, champion_chen

Realtek ship a variety of bluetooth USB devices that identify
themselves with standard USB Bluetooth device class values, but
require a special driver to actually work. Without that driver,
you never get any scan results.

More recently however, Realtek appear to have wisened up and simply
posted a firmware update that makes these devices comply with
normal btusb protocols. The firmware needs to be uploaded on each boot.

Based on Realtek code from https://github.com/lwfinger/rtl8723au_bt
('new' branch).

This enables bluetooth support in the Gigabyte Brix GB-BXBT-2807 which
has this RTL8723BE USB device:

T:  Bus=01 Lev=01 Prnt=01 Port=01 Cnt=02 Dev#=  3 Spd=12   MxCh= 0
D:  Ver= 2.10 Cls=e0(wlcon) Sub=01 Prot=01 MxPS=64 #Cfgs=  1
P:  Vendor=13d3 ProdID=3410 Rev= 2.00
S:  Manufacturer=Realtek
S:  Product=Bluetooth Radio
S:  SerialNumber=00e04c000001
C:* #Ifs= 2 Cfg#= 1 Atr=e0 MxPwr=500mA
I:* If#= 0 Alt= 0 #EPs= 3 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=81(I) Atr=03(Int.) MxPS=  16 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

There is no change to the USB descriptor after firmware update,
however the version read by HCI_OP_READ_LOCAL_VERSION changes from
0x8723 to 0x3083.

I also brought in support for the 8821A/8761A devices, which use
the same firmware uploading mechanism, plus the 8723A devices,
which use a more simplistic firmware format. I have not tested
with these additional devices.

Signed-off-by: Daniel Drake <drake@endlessm.com>
---
 drivers/bluetooth/btusb.c | 432 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 432 insertions(+)

Depends on:
 Bluetooth: btusb: Add helper for READ_LOCAL_VERSION command

v3: Sorry for the long delay in getting back to this!
 - Removed support for devices where we don't have firmware
 - Divide 8723A/8723B codepaths based on driver_info constant
 - Added more device IDs from latest Realtek code
 - Addressed minor review comments
 - Rename RTK --> RTL

v2:
 - share main blacklist table with other devices
 - epatch table parsing endian/alignment fixes
 - BT_INFO message to inform user
 - added missing kmalloc error check
 - fixed skb leak
 - style fixes

diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 3096308..0256ab2 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -24,6 +24,7 @@
 #include <linux/module.h>
 #include <linux/usb.h>
 #include <linux/firmware.h>
+#include <asm/unaligned.h>
 
 #include <net/bluetooth/bluetooth.h>
 #include <net/bluetooth/hci_core.h>
@@ -52,6 +53,8 @@ static struct usb_driver btusb_driver;
 #define BTUSB_SWAVE		0x1000
 #define BTUSB_INTEL_NEW		0x2000
 #define BTUSB_AMP		0x4000
+#define BTUSB_RTL8723A		0x8000
+#define BTUSB_RTL8723B		0x10000
 
 static const struct usb_device_id btusb_table[] = {
 	/* Generic Bluetooth USB device */
@@ -280,6 +283,58 @@ static const struct usb_device_id blacklist_table[] = {
 	{ USB_VENDOR_AND_INTERFACE_INFO(0x8087, 0xe0, 0x01, 0x01),
 	  .driver_info = BTUSB_IGNORE },
 
+	/* Realtek 8723AE Bluetooth devices */
+	{ USB_DEVICE(0x0930, 0x021d), .driver_info = BTUSB_RTL8723A },
+	{ USB_DEVICE(0x0bda, 0x0723), .driver_info = BTUSB_RTL8723A },
+	{ USB_DEVICE(0x0bda, 0x8723), .driver_info = BTUSB_RTL8723A },
+	{ USB_DEVICE(0x13d3, 0x3394), .driver_info = BTUSB_RTL8723A },
+
+	/* Realtek 8723AU Bluetooth devices */
+	{ USB_DEVICE(0x0bda, 0x0724), .driver_info = BTUSB_RTL8723A },
+	{ USB_DEVICE(0x0bda, 0x1724), .driver_info = BTUSB_RTL8723A },
+	{ USB_DEVICE(0x0bda, 0x8725), .driver_info = BTUSB_RTL8723A },
+	{ USB_DEVICE(0x0bda, 0x872a), .driver_info = BTUSB_RTL8723A },
+	{ USB_DEVICE(0x0bda, 0x872b), .driver_info = BTUSB_RTL8723A },
+	{ USB_DEVICE(0x0bda, 0xa724), .driver_info = BTUSB_RTL8723A },
+
+	/* Realtek 8723BE Bluetooth devices */
+	{ USB_DEVICE(0x0489, 0xe085), .driver_info = BTUSB_RTL8723B },
+	{ USB_DEVICE(0x0489, 0xe08b), .driver_info = BTUSB_RTL8723B },
+	{ USB_DEVICE(0x0bda, 0xb001), .driver_info = BTUSB_RTL8723B },
+	{ USB_DEVICE(0x0bda, 0xb002), .driver_info = BTUSB_RTL8723B },
+	{ USB_DEVICE(0x0bda, 0xb003), .driver_info = BTUSB_RTL8723B },
+	{ USB_DEVICE(0x0bda, 0xb004), .driver_info = BTUSB_RTL8723B },
+	{ USB_DEVICE(0x0bda, 0xb005), .driver_info = BTUSB_RTL8723B },
+	{ USB_DEVICE(0x0bda, 0xb723), .driver_info = BTUSB_RTL8723B },
+	{ USB_DEVICE(0x0bda, 0xb728), .driver_info = BTUSB_RTL8723B },
+	{ USB_DEVICE(0x0bda, 0xb72b), .driver_info = BTUSB_RTL8723B },
+	{ USB_DEVICE(0x13d3, 0x3410), .driver_info = BTUSB_RTL8723B },
+	{ USB_DEVICE(0x13d3, 0x3416), .driver_info = BTUSB_RTL8723B },
+	{ USB_DEVICE(0x13d3, 0x3459), .driver_info = BTUSB_RTL8723B },
+
+	/* Realtek 8723BU Bluetooth devices */
+	{ USB_DEVICE(0x0bda, 0xb720), .driver_info = BTUSB_RTL8723B },
+	{ USB_DEVICE(0x0bda, 0xb72a), .driver_info = BTUSB_RTL8723B },
+
+	/* Realtek 8821AE Bluetooth devices */
+	{ USB_DEVICE(0x0b05, 0x17dc), .driver_info = BTUSB_RTL8723B },
+	{ USB_DEVICE(0x0bda, 0x0821), .driver_info = BTUSB_RTL8723B },
+	{ USB_DEVICE(0x0bda, 0x8821), .driver_info = BTUSB_RTL8723B },
+	{ USB_DEVICE(0x13d3, 0x3414), .driver_info = BTUSB_RTL8723B },
+	{ USB_DEVICE(0x13d3, 0x3458), .driver_info = BTUSB_RTL8723B },
+	{ USB_DEVICE(0x13d3, 0x3461), .driver_info = BTUSB_RTL8723B },
+	{ USB_DEVICE(0x13d3, 0x3462), .driver_info = BTUSB_RTL8723B },
+
+	/* Realtek 8821AU Bluetooth devices */
+	{ USB_DEVICE(0x0bda, 0x0823), .driver_info = BTUSB_RTL8723B },
+
+	/* Realtek 8761AU Bluetooth devices */
+	{ USB_DEVICE(0x0bda, 0x8760), .driver_info = BTUSB_RTL8723B },
+	{ USB_DEVICE(0x0bda, 0x8761), .driver_info = BTUSB_RTL8723B },
+	{ USB_DEVICE(0x0bda, 0x8a60), .driver_info = BTUSB_RTL8723B },
+	{ USB_DEVICE(0x0bda, 0xa761), .driver_info = BTUSB_RTL8723B },
+	{ USB_DEVICE(0x0bda, 0xb761), .driver_info = BTUSB_RTL8723B },
+
 	{ }	/* Terminating entry */
 };
 
@@ -1341,6 +1396,377 @@ static int btusb_setup_csr(struct hci_dev *hdev)
 	return ret;
 }
 
+#define RTL_FRAG_LEN 252
+
+struct rtl_download_cmd {
+	uint8_t index;
+	uint8_t data[RTL_FRAG_LEN];
+} __packed;
+
+struct rtl_download_response {
+	uint8_t status;
+	uint8_t index;
+} __packed;
+
+struct rtl_rom_version_evt {
+	uint8_t status;
+	uint8_t version;
+} __packed;
+
+struct rtl_epatch_header {
+	uint8_t signature[8];
+	uint32_t fw_version;
+	uint16_t num_patches;
+} __packed;
+
+static const uint8_t RTL_EPATCH_SIGNATURE[] = {
+	0x52, 0x65, 0x61, 0x6C, 0x74, 0x65, 0x63, 0x68
+};
+
+#define RTL_ROM_LMP_3499	0x3499
+#define RTL_ROM_LMP_8723A	0x1200
+#define RTL_ROM_LMP_8723B	0x8723
+#define RTL_ROM_LMP_8821A	0x8821
+#define RTL_ROM_LMP_8761A	0x8761
+
+static int rtl_read_rom_version(struct hci_dev *hdev)
+{
+	struct rtl_rom_version_evt *rom_version;
+	struct sk_buff *skb;
+	int r;
+
+	/* Read RTL ROM version command */
+	skb = __hci_cmd_sync(hdev, 0xfc6d, 0, NULL, HCI_INIT_TIMEOUT);
+	if (IS_ERR(skb)) {
+		BT_ERR("Read ROM version failed (%ld)", PTR_ERR(skb));
+		return PTR_ERR(skb);
+	}
+
+	if (skb->len != sizeof(*rom_version)) {
+		BT_ERR("RTL version event length mismatch");
+		kfree_skb(skb);
+		return -EIO;
+	}
+
+	rom_version = (struct rtl_rom_version_evt *) skb->data;
+	BT_DBG("rom_version status=%x version=%x",
+	       rom_version->status, rom_version->version);
+	if (rom_version->status)
+		r = 0;
+	else
+		r = rom_version->version;
+
+	kfree_skb(skb);
+	return r;
+}
+
+static int rtl8723b_parse_firmware(struct hci_dev *hdev, u16 lmp_subver,
+				   const struct firmware *fw,
+				   unsigned char **_buf)
+{
+	const uint8_t extension_sig[] = { 0x51, 0x04, 0xfd, 0x77 };
+	struct rtl_epatch_header *epatch_info;
+	unsigned char *buf;
+	int i, len, rom_version;
+	size_t min_size;
+	uint8_t opcode, length, data;
+	int project_id = -1;
+	const unsigned char *fwptr, *chip_id_base;
+	const unsigned char *patch_length_base, *patch_offset_base;
+	u32 patch_offset = 0;
+	u16 patch_length;
+	const uint16_t project_id_to_lmp_subver[] = {
+		RTL_ROM_LMP_8723A,
+		RTL_ROM_LMP_8723B,
+		RTL_ROM_LMP_8821A,
+		RTL_ROM_LMP_8761A
+	};
+
+	rom_version = rtl_read_rom_version(hdev);
+	if (rom_version < 0)
+		return rom_version;
+
+	BT_DBG("lmp_subver=%x rom_version=%x", lmp_subver, rom_version);
+
+	min_size = sizeof(struct rtl_epatch_header) + sizeof(extension_sig) + 3;
+	if (fw->size < min_size)
+		return -EINVAL;
+
+	fwptr = fw->data + fw->size - sizeof(extension_sig);
+	if (memcmp(fwptr, extension_sig, sizeof(extension_sig)) != 0) {
+		BT_ERR("extension section signature mismatch");
+		return -EINVAL;
+	}
+
+	/* Loop from the end of the firmware parsing instructions, until
+	 * we find an instruction that identifies the "project ID" for the
+	 * hardware supported by this firwmare file.
+	 * Once we have that, we double-check that that project_id is suitable
+	 * for the hardware we are working with. */
+	while (fwptr >= fw->data + (sizeof(struct rtl_epatch_header) + 3)) {
+		opcode = *--fwptr;
+		length = *--fwptr;
+		data = *--fwptr;
+		BT_DBG("check op=%x len=%x data=%x", opcode, length, data);
+
+		if (opcode == 0xff) /* EOF */
+			break;
+
+		if (length == 0) {
+			BT_ERR("found instruction with length 0");
+			return -EINVAL;
+		}
+
+		if (opcode == 0 && length == 1) {
+			project_id = data;
+			break;
+		}
+
+		fwptr -= length;
+	}
+
+	if (project_id < 0) {
+		BT_ERR("failed to find version instruction");
+		return -EINVAL;
+	}
+
+	if (project_id > ARRAY_SIZE(project_id_to_lmp_subver)) {
+		BT_ERR("unknown project id %d", project_id);
+		return -EINVAL;
+	}
+
+	if (lmp_subver != project_id_to_lmp_subver[project_id]) {
+		BT_ERR("firmware is for %x but this is a %x",
+		       project_id_to_lmp_subver[project_id], lmp_subver);
+		return -EINVAL;
+	}
+
+	epatch_info = (struct rtl_epatch_header *) fw->data;
+	if (memcmp(epatch_info->signature, RTL_EPATCH_SIGNATURE, 8) != 0) {
+		BT_ERR("bad EPATCH signature");
+		return -EINVAL;
+	}
+
+	BT_DBG("fw_version=%x, num_patches=%d",
+	       epatch_info->fw_version, epatch_info->num_patches);
+
+	/* After the rtl_epatch_header there is a funky patch metadata section.
+	 * Assuming 2 patches, the layout is:
+	 * ChipID1 ChipID2 PatchLength1 PatchLength2 PatchOffset1 PatchOffset2
+	 *
+	 * Find the right patch for this chip. */
+	min_size += 8 * epatch_info->num_patches;
+	if (fw->size < min_size)
+		return -EINVAL;
+
+	chip_id_base = fw->data + sizeof(struct rtl_epatch_header);
+	patch_length_base = chip_id_base +
+			    (sizeof(u16) * epatch_info->num_patches);
+	patch_offset_base = patch_length_base +
+			    (sizeof(u16) * epatch_info->num_patches);
+	for (i = 0; i < epatch_info->num_patches; i++) {
+		u16 chip_id = get_unaligned_le16(chip_id_base +
+						 (i * sizeof(u16)));
+		if (chip_id == rom_version + 1) {
+			patch_length = get_unaligned_le16(patch_length_base +
+							  (i * sizeof(u16)));
+			patch_offset = get_unaligned_le32(patch_offset_base +
+							  (i * sizeof(u32)));
+			break;
+		}
+	}
+
+	if (!patch_offset) {
+		BT_ERR("didn't find patch for chip id %d", rom_version);
+		return -EINVAL;
+	}
+
+	BT_DBG("length=%x offset=%x index %d", patch_length, patch_offset, i);
+	min_size = patch_offset + patch_length;
+	if (fw->size < min_size)
+		return -EINVAL;
+
+	/* Copy the firmware into a new buffer and write the version at
+	 * the end. */
+	len = patch_length;
+	buf = kmemdup(fw->data + patch_offset, patch_length, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	memcpy(buf + patch_length - 4, &epatch_info->fw_version, 4);
+
+	*_buf = buf;
+	return len;
+}
+
+static int rtl_download_firmware(struct hci_dev *hdev,
+				 const unsigned char *data, int fw_len)
+{
+	struct rtl_download_cmd *dl_cmd;
+	int frag_num = fw_len / RTL_FRAG_LEN + 1;
+	int frag_len = RTL_FRAG_LEN;
+	int ret = 0;
+	int i;
+
+	dl_cmd = kmalloc(sizeof(struct rtl_download_cmd), GFP_KERNEL);
+	if (!dl_cmd)
+		return -ENOMEM;
+
+	for (i = 0; i < frag_num; i++) {
+		struct rtl_download_response *dl_resp;
+		struct sk_buff *skb;
+
+		BT_DBG("download fw (%d/%d)", i, frag_num);
+		dl_cmd->index = i;
+		if (i == (frag_num - 1)) {
+			dl_cmd->index |= 0x80; /* data end */
+			frag_len = fw_len % RTL_FRAG_LEN;
+		}
+		memcpy(dl_cmd->data, data, frag_len);
+
+		/* Send download command */
+		skb = __hci_cmd_sync(hdev, 0xfc20, frag_len + 1, dl_cmd,
+				     HCI_INIT_TIMEOUT);
+		if (IS_ERR(skb)) {
+			BT_ERR("download fw command failed (%ld)",
+			       PTR_ERR(skb));
+			ret = -PTR_ERR(skb);
+			goto out;
+		}
+
+		if (skb->len != sizeof(*dl_resp)) {
+			BT_ERR("download fw event length mismatch");
+			kfree_skb(skb);
+			ret = -EIO;
+			goto out;
+		}
+
+		dl_resp = (struct rtl_download_response *) skb->data;
+		if (dl_resp->status != 0) {
+			kfree_skb(skb);
+			ret = bt_to_errno(dl_resp->status);
+			goto out;
+		}
+
+		kfree_skb(skb);
+		data += RTL_FRAG_LEN;
+	}
+
+out:
+	kfree(dl_cmd);
+	return ret;
+}
+
+static int btusb_setup_rtl8723a(struct hci_dev *hdev)
+{
+	struct btusb_data *data = dev_get_drvdata(&hdev->dev);
+	struct usb_device *udev = interface_to_usbdev(data->intf);
+	struct sk_buff *skb;
+	struct hci_rp_read_local_version *resp;
+	const struct firmware *fw;
+	int ret;
+
+	skb = btusb_read_local_version(hdev);
+	if (IS_ERR(skb))
+		return -PTR_ERR(skb);
+
+	resp = (struct hci_rp_read_local_version *) skb->data;
+	if (resp->lmp_subver != RTL_ROM_LMP_8723A
+			&& resp->lmp_subver != RTL_ROM_LMP_3499) {
+		BT_INFO("rtl8723a: assuming no firmware upload needed.");
+		kfree_skb(skb);
+		return 0;
+	}
+
+	kfree_skb(skb);
+
+	ret = request_firmware(&fw, "rtl_bt/rtl8723a_fw.bin", &udev->dev);
+	if (ret < 0) {
+		BT_ERR("Failed to load rtl_bt/rtl8723a_fw.bin");
+		return ret;
+	}
+
+	if (fw->size < 8) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	/* Check that the firmware doesn't have the epatch signature
+	 * (which is only for RTL8723B and newer). */
+	if (memcmp(fw->data, RTL_EPATCH_SIGNATURE, 8) == 0) {
+		BT_ERR("RTL8723A: unexpected EPATCH signature!");
+		ret = -EINVAL;
+		goto out;
+	}
+
+	ret = rtl_download_firmware(hdev, fw->data, fw->size);
+
+out:
+	release_firmware(fw);
+	return ret;
+}
+
+static int btusb_setup_rtl8723b(struct hci_dev *hdev)
+{
+	struct btusb_data *data = dev_get_drvdata(&hdev->dev);
+	struct usb_device *udev = interface_to_usbdev(data->intf);
+	const char *fw_name;
+	unsigned char *fw_data;
+	u16 lmp_subver;
+	const struct firmware *fw;
+	struct hci_rp_read_local_version *resp;
+	struct sk_buff *skb;
+	int ret;
+
+	skb = btusb_read_local_version(hdev);
+	if (IS_ERR(skb))
+		return -PTR_ERR(skb);
+
+	resp = (struct hci_rp_read_local_version *) skb->data;
+	if (resp->status) {
+		BT_ERR("rtl fw version event failed (%02x)", resp->status);
+		kfree_skb(skb);
+		return bt_to_errno(resp->status);
+	}
+
+	lmp_subver = resp->lmp_subver;
+	kfree_skb(skb);
+
+	switch (lmp_subver) {
+	case RTL_ROM_LMP_8723B:
+		fw_name = "rtl_bt/rtl8723b_fw.bin";
+		break;
+	case RTL_ROM_LMP_8821A:
+		fw_name = "rtl_bt/rtl8821a_fw.bin";
+		break;
+	case RTL_ROM_LMP_8761A:
+		fw_name = "rtl_bt/rtl8761a_fw.bin";
+		break;
+	default:
+		BT_INFO("rtl8723b: assuming no firmware upload needed.");
+		return 0;
+	}
+
+	ret = request_firmware(&fw, fw_name, &udev->dev);
+	if (ret < 0) {
+		BT_ERR("Failed to load %s", fw_name);
+		return ret;
+	}
+
+	ret = rtl8723b_parse_firmware(hdev, lmp_subver, fw, &fw_data);
+	if (ret < 0)
+		goto out;
+
+	ret = rtl_download_firmware(hdev, fw_data, ret);
+	kfree(fw_data);
+	if (ret < 0)
+		goto out;
+
+out:
+	release_firmware(fw);
+	return ret;
+}
+
 struct intel_version {
 	u8 status;
 	u8 hw_platform;
@@ -2732,6 +3158,12 @@ static int btusb_probe(struct usb_interface *intf,
 		set_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks);
 	}
 
+	if (id->driver_info & BTUSB_RTL8723A)
+		hdev->setup = btusb_setup_rtl8723a;
+
+	if (id->driver_info & BTUSB_RTL8723B)
+		hdev->setup = btusb_setup_rtl8723b;
+
 	if (id->driver_info & BTUSB_AMP) {
 		/* AMP controllers do not support SCO packets */
 		data->isoc = NULL;
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v3] Bluetooth: btusb: Add Realtek 8723A/8723B/8761A/8821A support
  2015-02-09 22:59 [PATCH v3] Bluetooth: btusb: Add Realtek 8723A/8723B/8761A/8821A support Daniel Drake
@ 2015-02-10  0:06 ` Larry Finger
  2015-02-10 17:33 ` Larry Finger
  2015-02-12 20:26 ` Marcel Holtmann
  2 siblings, 0 replies; 7+ messages in thread
From: Larry Finger @ 2015-02-10  0:06 UTC (permalink / raw)
  To: Daniel Drake, marcel, gustavo, johan.hedberg
  Cc: linux-bluetooth, champion_chen

On 02/09/2015 04:59 PM, Daniel Drake wrote:
> Realtek ship a variety of bluetooth USB devices that identify
> themselves with standard USB Bluetooth device class values, but
> require a special driver to actually work. Without that driver,
> you never get any scan results.
>
> More recently however, Realtek appear to have wisened up and simply
> posted a firmware update that makes these devices comply with
> normal btusb protocols. The firmware needs to be uploaded on each boot.
>
> Based on Realtek code from https://github.com/lwfinger/rtl8723au_bt
> ('new' branch).

Daniel,

I am very happy to see this patch. I have not yet tested it, or even read it 
carefully. One thing to note is that users have been a lot more successful with 
the 'troy' branch of that repo. Again, I have not really looked at the differences.

I do have firmware for the BT part of the RTL8192EE (have hardware) and 
RTL8192EU (no hardware). These devices should probably be added as well. If you 
prefer, your patch can leave those devices out, and I can add them later.

I have samples of many of the Realtek BT devices. After testing, I will let you 
know of further comments.

Larry



>
> This enables bluetooth support in the Gigabyte Brix GB-BXBT-2807 which
> has this RTL8723BE USB device:
>
> T:  Bus=01 Lev=01 Prnt=01 Port=01 Cnt=02 Dev#=  3 Spd=12   MxCh= 0
> D:  Ver= 2.10 Cls=e0(wlcon) Sub=01 Prot=01 MxPS=64 #Cfgs=  1
> P:  Vendor=13d3 ProdID=3410 Rev= 2.00
> S:  Manufacturer=Realtek
> S:  Product=Bluetooth Radio
> S:  SerialNumber=00e04c000001
> C:* #Ifs= 2 Cfg#= 1 Atr=e0 MxPwr=500mA
> I:* If#= 0 Alt= 0 #EPs= 3 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
> E:  Ad=81(I) Atr=03(Int.) MxPS=  16 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
>
> There is no change to the USB descriptor after firmware update,
> however the version read by HCI_OP_READ_LOCAL_VERSION changes from
> 0x8723 to 0x3083.
>
> I also brought in support for the 8821A/8761A devices, which use
> the same firmware uploading mechanism, plus the 8723A devices,
> which use a more simplistic firmware format. I have not tested
> with these additional devices.
>
> Signed-off-by: Daniel Drake <drake@endlessm.com>
> ---
>   drivers/bluetooth/btusb.c | 432 ++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 432 insertions(+)
>
> Depends on:
>   Bluetooth: btusb: Add helper for READ_LOCAL_VERSION command
>
> v3: Sorry for the long delay in getting back to this!
>   - Removed support for devices where we don't have firmware
>   - Divide 8723A/8723B codepaths based on driver_info constant
>   - Added more device IDs from latest Realtek code
>   - Addressed minor review comments
>   - Rename RTK --> RTL
>
> v2:
>   - share main blacklist table with other devices
>   - epatch table parsing endian/alignment fixes
>   - BT_INFO message to inform user
>   - added missing kmalloc error check
>   - fixed skb leak
>   - style fixes
>
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index 3096308..0256ab2 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -24,6 +24,7 @@
>   #include <linux/module.h>
>   #include <linux/usb.h>
>   #include <linux/firmware.h>
> +#include <asm/unaligned.h>
>
>   #include <net/bluetooth/bluetooth.h>
>   #include <net/bluetooth/hci_core.h>
> @@ -52,6 +53,8 @@ static struct usb_driver btusb_driver;
>   #define BTUSB_SWAVE		0x1000
>   #define BTUSB_INTEL_NEW		0x2000
>   #define BTUSB_AMP		0x4000
> +#define BTUSB_RTL8723A		0x8000
> +#define BTUSB_RTL8723B		0x10000
>
>   static const struct usb_device_id btusb_table[] = {
>   	/* Generic Bluetooth USB device */
> @@ -280,6 +283,58 @@ static const struct usb_device_id blacklist_table[] = {
>   	{ USB_VENDOR_AND_INTERFACE_INFO(0x8087, 0xe0, 0x01, 0x01),
>   	  .driver_info = BTUSB_IGNORE },
>
> +	/* Realtek 8723AE Bluetooth devices */
> +	{ USB_DEVICE(0x0930, 0x021d), .driver_info = BTUSB_RTL8723A },
> +	{ USB_DEVICE(0x0bda, 0x0723), .driver_info = BTUSB_RTL8723A },
> +	{ USB_DEVICE(0x0bda, 0x8723), .driver_info = BTUSB_RTL8723A },
> +	{ USB_DEVICE(0x13d3, 0x3394), .driver_info = BTUSB_RTL8723A },
> +
> +	/* Realtek 8723AU Bluetooth devices */
> +	{ USB_DEVICE(0x0bda, 0x0724), .driver_info = BTUSB_RTL8723A },
> +	{ USB_DEVICE(0x0bda, 0x1724), .driver_info = BTUSB_RTL8723A },
> +	{ USB_DEVICE(0x0bda, 0x8725), .driver_info = BTUSB_RTL8723A },
> +	{ USB_DEVICE(0x0bda, 0x872a), .driver_info = BTUSB_RTL8723A },
> +	{ USB_DEVICE(0x0bda, 0x872b), .driver_info = BTUSB_RTL8723A },
> +	{ USB_DEVICE(0x0bda, 0xa724), .driver_info = BTUSB_RTL8723A },
> +
> +	/* Realtek 8723BE Bluetooth devices */
> +	{ USB_DEVICE(0x0489, 0xe085), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x0489, 0xe08b), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x0bda, 0xb001), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x0bda, 0xb002), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x0bda, 0xb003), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x0bda, 0xb004), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x0bda, 0xb005), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x0bda, 0xb723), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x0bda, 0xb728), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x0bda, 0xb72b), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x13d3, 0x3410), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x13d3, 0x3416), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x13d3, 0x3459), .driver_info = BTUSB_RTL8723B },
> +
> +	/* Realtek 8723BU Bluetooth devices */
> +	{ USB_DEVICE(0x0bda, 0xb720), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x0bda, 0xb72a), .driver_info = BTUSB_RTL8723B },
> +
> +	/* Realtek 8821AE Bluetooth devices */
> +	{ USB_DEVICE(0x0b05, 0x17dc), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x0bda, 0x0821), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x0bda, 0x8821), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x13d3, 0x3414), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x13d3, 0x3458), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x13d3, 0x3461), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x13d3, 0x3462), .driver_info = BTUSB_RTL8723B },
> +
> +	/* Realtek 8821AU Bluetooth devices */
> +	{ USB_DEVICE(0x0bda, 0x0823), .driver_info = BTUSB_RTL8723B },
> +
> +	/* Realtek 8761AU Bluetooth devices */
> +	{ USB_DEVICE(0x0bda, 0x8760), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x0bda, 0x8761), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x0bda, 0x8a60), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x0bda, 0xa761), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x0bda, 0xb761), .driver_info = BTUSB_RTL8723B },
> +
>   	{ }	/* Terminating entry */
>   };
>
> @@ -1341,6 +1396,377 @@ static int btusb_setup_csr(struct hci_dev *hdev)
>   	return ret;
>   }
>
> +#define RTL_FRAG_LEN 252
> +
> +struct rtl_download_cmd {
> +	uint8_t index;
> +	uint8_t data[RTL_FRAG_LEN];
> +} __packed;
> +
> +struct rtl_download_response {
> +	uint8_t status;
> +	uint8_t index;
> +} __packed;
> +
> +struct rtl_rom_version_evt {
> +	uint8_t status;
> +	uint8_t version;
> +} __packed;
> +
> +struct rtl_epatch_header {
> +	uint8_t signature[8];
> +	uint32_t fw_version;
> +	uint16_t num_patches;
> +} __packed;
> +
> +static const uint8_t RTL_EPATCH_SIGNATURE[] = {
> +	0x52, 0x65, 0x61, 0x6C, 0x74, 0x65, 0x63, 0x68
> +};
> +
> +#define RTL_ROM_LMP_3499	0x3499
> +#define RTL_ROM_LMP_8723A	0x1200
> +#define RTL_ROM_LMP_8723B	0x8723
> +#define RTL_ROM_LMP_8821A	0x8821
> +#define RTL_ROM_LMP_8761A	0x8761
> +
> +static int rtl_read_rom_version(struct hci_dev *hdev)
> +{
> +	struct rtl_rom_version_evt *rom_version;
> +	struct sk_buff *skb;
> +	int r;
> +
> +	/* Read RTL ROM version command */
> +	skb = __hci_cmd_sync(hdev, 0xfc6d, 0, NULL, HCI_INIT_TIMEOUT);
> +	if (IS_ERR(skb)) {
> +		BT_ERR("Read ROM version failed (%ld)", PTR_ERR(skb));
> +		return PTR_ERR(skb);
> +	}
> +
> +	if (skb->len != sizeof(*rom_version)) {
> +		BT_ERR("RTL version event length mismatch");
> +		kfree_skb(skb);
> +		return -EIO;
> +	}
> +
> +	rom_version = (struct rtl_rom_version_evt *) skb->data;
> +	BT_DBG("rom_version status=%x version=%x",
> +	       rom_version->status, rom_version->version);
> +	if (rom_version->status)
> +		r = 0;
> +	else
> +		r = rom_version->version;
> +
> +	kfree_skb(skb);
> +	return r;
> +}
> +
> +static int rtl8723b_parse_firmware(struct hci_dev *hdev, u16 lmp_subver,
> +				   const struct firmware *fw,
> +				   unsigned char **_buf)
> +{
> +	const uint8_t extension_sig[] = { 0x51, 0x04, 0xfd, 0x77 };
> +	struct rtl_epatch_header *epatch_info;
> +	unsigned char *buf;
> +	int i, len, rom_version;
> +	size_t min_size;
> +	uint8_t opcode, length, data;
> +	int project_id = -1;
> +	const unsigned char *fwptr, *chip_id_base;
> +	const unsigned char *patch_length_base, *patch_offset_base;
> +	u32 patch_offset = 0;
> +	u16 patch_length;
> +	const uint16_t project_id_to_lmp_subver[] = {
> +		RTL_ROM_LMP_8723A,
> +		RTL_ROM_LMP_8723B,
> +		RTL_ROM_LMP_8821A,
> +		RTL_ROM_LMP_8761A
> +	};
> +
> +	rom_version = rtl_read_rom_version(hdev);
> +	if (rom_version < 0)
> +		return rom_version;
> +
> +	BT_DBG("lmp_subver=%x rom_version=%x", lmp_subver, rom_version);
> +
> +	min_size = sizeof(struct rtl_epatch_header) + sizeof(extension_sig) + 3;
> +	if (fw->size < min_size)
> +		return -EINVAL;
> +
> +	fwptr = fw->data + fw->size - sizeof(extension_sig);
> +	if (memcmp(fwptr, extension_sig, sizeof(extension_sig)) != 0) {
> +		BT_ERR("extension section signature mismatch");
> +		return -EINVAL;
> +	}
> +
> +	/* Loop from the end of the firmware parsing instructions, until
> +	 * we find an instruction that identifies the "project ID" for the
> +	 * hardware supported by this firwmare file.
> +	 * Once we have that, we double-check that that project_id is suitable
> +	 * for the hardware we are working with. */
> +	while (fwptr >= fw->data + (sizeof(struct rtl_epatch_header) + 3)) {
> +		opcode = *--fwptr;
> +		length = *--fwptr;
> +		data = *--fwptr;
> +		BT_DBG("check op=%x len=%x data=%x", opcode, length, data);
> +
> +		if (opcode == 0xff) /* EOF */
> +			break;
> +
> +		if (length == 0) {
> +			BT_ERR("found instruction with length 0");
> +			return -EINVAL;
> +		}
> +
> +		if (opcode == 0 && length == 1) {
> +			project_id = data;
> +			break;
> +		}
> +
> +		fwptr -= length;
> +	}
> +
> +	if (project_id < 0) {
> +		BT_ERR("failed to find version instruction");
> +		return -EINVAL;
> +	}
> +
> +	if (project_id > ARRAY_SIZE(project_id_to_lmp_subver)) {
> +		BT_ERR("unknown project id %d", project_id);
> +		return -EINVAL;
> +	}
> +
> +	if (lmp_subver != project_id_to_lmp_subver[project_id]) {
> +		BT_ERR("firmware is for %x but this is a %x",
> +		       project_id_to_lmp_subver[project_id], lmp_subver);
> +		return -EINVAL;
> +	}
> +
> +	epatch_info = (struct rtl_epatch_header *) fw->data;
> +	if (memcmp(epatch_info->signature, RTL_EPATCH_SIGNATURE, 8) != 0) {
> +		BT_ERR("bad EPATCH signature");
> +		return -EINVAL;
> +	}
> +
> +	BT_DBG("fw_version=%x, num_patches=%d",
> +	       epatch_info->fw_version, epatch_info->num_patches);
> +
> +	/* After the rtl_epatch_header there is a funky patch metadata section.
> +	 * Assuming 2 patches, the layout is:
> +	 * ChipID1 ChipID2 PatchLength1 PatchLength2 PatchOffset1 PatchOffset2
> +	 *
> +	 * Find the right patch for this chip. */
> +	min_size += 8 * epatch_info->num_patches;
> +	if (fw->size < min_size)
> +		return -EINVAL;
> +
> +	chip_id_base = fw->data + sizeof(struct rtl_epatch_header);
> +	patch_length_base = chip_id_base +
> +			    (sizeof(u16) * epatch_info->num_patches);
> +	patch_offset_base = patch_length_base +
> +			    (sizeof(u16) * epatch_info->num_patches);
> +	for (i = 0; i < epatch_info->num_patches; i++) {
> +		u16 chip_id = get_unaligned_le16(chip_id_base +
> +						 (i * sizeof(u16)));
> +		if (chip_id == rom_version + 1) {
> +			patch_length = get_unaligned_le16(patch_length_base +
> +							  (i * sizeof(u16)));
> +			patch_offset = get_unaligned_le32(patch_offset_base +
> +							  (i * sizeof(u32)));
> +			break;
> +		}
> +	}
> +
> +	if (!patch_offset) {
> +		BT_ERR("didn't find patch for chip id %d", rom_version);
> +		return -EINVAL;
> +	}
> +
> +	BT_DBG("length=%x offset=%x index %d", patch_length, patch_offset, i);
> +	min_size = patch_offset + patch_length;
> +	if (fw->size < min_size)
> +		return -EINVAL;
> +
> +	/* Copy the firmware into a new buffer and write the version at
> +	 * the end. */
> +	len = patch_length;
> +	buf = kmemdup(fw->data + patch_offset, patch_length, GFP_KERNEL);
> +	if (!buf)
> +		return -ENOMEM;
> +
> +	memcpy(buf + patch_length - 4, &epatch_info->fw_version, 4);
> +
> +	*_buf = buf;
> +	return len;
> +}
> +
> +static int rtl_download_firmware(struct hci_dev *hdev,
> +				 const unsigned char *data, int fw_len)
> +{
> +	struct rtl_download_cmd *dl_cmd;
> +	int frag_num = fw_len / RTL_FRAG_LEN + 1;
> +	int frag_len = RTL_FRAG_LEN;
> +	int ret = 0;
> +	int i;
> +
> +	dl_cmd = kmalloc(sizeof(struct rtl_download_cmd), GFP_KERNEL);
> +	if (!dl_cmd)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < frag_num; i++) {
> +		struct rtl_download_response *dl_resp;
> +		struct sk_buff *skb;
> +
> +		BT_DBG("download fw (%d/%d)", i, frag_num);
> +		dl_cmd->index = i;
> +		if (i == (frag_num - 1)) {
> +			dl_cmd->index |= 0x80; /* data end */
> +			frag_len = fw_len % RTL_FRAG_LEN;
> +		}
> +		memcpy(dl_cmd->data, data, frag_len);
> +
> +		/* Send download command */
> +		skb = __hci_cmd_sync(hdev, 0xfc20, frag_len + 1, dl_cmd,
> +				     HCI_INIT_TIMEOUT);
> +		if (IS_ERR(skb)) {
> +			BT_ERR("download fw command failed (%ld)",
> +			       PTR_ERR(skb));
> +			ret = -PTR_ERR(skb);
> +			goto out;
> +		}
> +
> +		if (skb->len != sizeof(*dl_resp)) {
> +			BT_ERR("download fw event length mismatch");
> +			kfree_skb(skb);
> +			ret = -EIO;
> +			goto out;
> +		}
> +
> +		dl_resp = (struct rtl_download_response *) skb->data;
> +		if (dl_resp->status != 0) {
> +			kfree_skb(skb);
> +			ret = bt_to_errno(dl_resp->status);
> +			goto out;
> +		}
> +
> +		kfree_skb(skb);
> +		data += RTL_FRAG_LEN;
> +	}
> +
> +out:
> +	kfree(dl_cmd);
> +	return ret;
> +}
> +
> +static int btusb_setup_rtl8723a(struct hci_dev *hdev)
> +{
> +	struct btusb_data *data = dev_get_drvdata(&hdev->dev);
> +	struct usb_device *udev = interface_to_usbdev(data->intf);
> +	struct sk_buff *skb;
> +	struct hci_rp_read_local_version *resp;
> +	const struct firmware *fw;
> +	int ret;
> +
> +	skb = btusb_read_local_version(hdev);
> +	if (IS_ERR(skb))
> +		return -PTR_ERR(skb);
> +
> +	resp = (struct hci_rp_read_local_version *) skb->data;
> +	if (resp->lmp_subver != RTL_ROM_LMP_8723A
> +			&& resp->lmp_subver != RTL_ROM_LMP_3499) {
> +		BT_INFO("rtl8723a: assuming no firmware upload needed.");
> +		kfree_skb(skb);
> +		return 0;
> +	}
> +
> +	kfree_skb(skb);
> +
> +	ret = request_firmware(&fw, "rtl_bt/rtl8723a_fw.bin", &udev->dev);
> +	if (ret < 0) {
> +		BT_ERR("Failed to load rtl_bt/rtl8723a_fw.bin");
> +		return ret;
> +	}
> +
> +	if (fw->size < 8) {
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	/* Check that the firmware doesn't have the epatch signature
> +	 * (which is only for RTL8723B and newer). */
> +	if (memcmp(fw->data, RTL_EPATCH_SIGNATURE, 8) == 0) {
> +		BT_ERR("RTL8723A: unexpected EPATCH signature!");
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	ret = rtl_download_firmware(hdev, fw->data, fw->size);
> +
> +out:
> +	release_firmware(fw);
> +	return ret;
> +}
> +
> +static int btusb_setup_rtl8723b(struct hci_dev *hdev)
> +{
> +	struct btusb_data *data = dev_get_drvdata(&hdev->dev);
> +	struct usb_device *udev = interface_to_usbdev(data->intf);
> +	const char *fw_name;
> +	unsigned char *fw_data;
> +	u16 lmp_subver;
> +	const struct firmware *fw;
> +	struct hci_rp_read_local_version *resp;
> +	struct sk_buff *skb;
> +	int ret;
> +
> +	skb = btusb_read_local_version(hdev);
> +	if (IS_ERR(skb))
> +		return -PTR_ERR(skb);
> +
> +	resp = (struct hci_rp_read_local_version *) skb->data;
> +	if (resp->status) {
> +		BT_ERR("rtl fw version event failed (%02x)", resp->status);
> +		kfree_skb(skb);
> +		return bt_to_errno(resp->status);
> +	}
> +
> +	lmp_subver = resp->lmp_subver;
> +	kfree_skb(skb);
> +
> +	switch (lmp_subver) {
> +	case RTL_ROM_LMP_8723B:
> +		fw_name = "rtl_bt/rtl8723b_fw.bin";
> +		break;
> +	case RTL_ROM_LMP_8821A:
> +		fw_name = "rtl_bt/rtl8821a_fw.bin";
> +		break;
> +	case RTL_ROM_LMP_8761A:
> +		fw_name = "rtl_bt/rtl8761a_fw.bin";
> +		break;
> +	default:
> +		BT_INFO("rtl8723b: assuming no firmware upload needed.");
> +		return 0;
> +	}
> +
> +	ret = request_firmware(&fw, fw_name, &udev->dev);
> +	if (ret < 0) {
> +		BT_ERR("Failed to load %s", fw_name);
> +		return ret;
> +	}
> +
> +	ret = rtl8723b_parse_firmware(hdev, lmp_subver, fw, &fw_data);
> +	if (ret < 0)
> +		goto out;
> +
> +	ret = rtl_download_firmware(hdev, fw_data, ret);
> +	kfree(fw_data);
> +	if (ret < 0)
> +		goto out;
> +
> +out:
> +	release_firmware(fw);
> +	return ret;
> +}
> +
>   struct intel_version {
>   	u8 status;
>   	u8 hw_platform;
> @@ -2732,6 +3158,12 @@ static int btusb_probe(struct usb_interface *intf,
>   		set_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks);
>   	}
>
> +	if (id->driver_info & BTUSB_RTL8723A)
> +		hdev->setup = btusb_setup_rtl8723a;
> +
> +	if (id->driver_info & BTUSB_RTL8723B)
> +		hdev->setup = btusb_setup_rtl8723b;
> +
>   	if (id->driver_info & BTUSB_AMP) {
>   		/* AMP controllers do not support SCO packets */
>   		data->isoc = NULL;
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3] Bluetooth: btusb: Add Realtek 8723A/8723B/8761A/8821A support
  2015-02-09 22:59 [PATCH v3] Bluetooth: btusb: Add Realtek 8723A/8723B/8761A/8821A support Daniel Drake
  2015-02-10  0:06 ` Larry Finger
@ 2015-02-10 17:33 ` Larry Finger
  2015-02-11  1:01   ` Daniel Drake
  2015-02-12 20:26 ` Marcel Holtmann
  2 siblings, 1 reply; 7+ messages in thread
From: Larry Finger @ 2015-02-10 17:33 UTC (permalink / raw)
  To: Daniel Drake, marcel, gustavo, johan.hedberg
  Cc: linux-bluetooth, champion_chen

On 02/09/2015 04:59 PM, Daniel Drake wrote:
> Realtek ship a variety of bluetooth USB devices that identify
> themselves with standard USB Bluetooth device class values, but
> require a special driver to actually work. Without that driver,
> you never get any scan results.
>
> More recently however, Realtek appear to have wisened up and simply
> posted a firmware update that makes these devices comply with
> normal btusb protocols. The firmware needs to be uploaded on each boot.
>
> Based on Realtek code from https://github.com/lwfinger/rtl8723au_bt
> ('new' branch).
>
> This enables bluetooth support in the Gigabyte Brix GB-BXBT-2807 which
> has this RTL8723BE USB device:
>
> T:  Bus=01 Lev=01 Prnt=01 Port=01 Cnt=02 Dev#=  3 Spd=12   MxCh= 0
> D:  Ver= 2.10 Cls=e0(wlcon) Sub=01 Prot=01 MxPS=64 #Cfgs=  1
> P:  Vendor=13d3 ProdID=3410 Rev= 2.00
> S:  Manufacturer=Realtek
> S:  Product=Bluetooth Radio
> S:  SerialNumber=00e04c000001
> C:* #Ifs= 2 Cfg#= 1 Atr=e0 MxPwr=500mA
> I:* If#= 0 Alt= 0 #EPs= 3 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
> E:  Ad=81(I) Atr=03(Int.) MxPS=  16 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
>
> There is no change to the USB descriptor after firmware update,
> however the version read by HCI_OP_READ_LOCAL_VERSION changes from
> 0x8723 to 0x3083.
>
> I also brought in support for the 8821A/8761A devices, which use
> the same firmware uploading mechanism, plus the 8723A devices,
> which use a more simplistic firmware format. I have not tested
> with these additional devices.
>
> Signed-off-by: Daniel Drake <drake@endlessm.com>
> ---
>   drivers/bluetooth/btusb.c | 432 ++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 432 insertions(+)
>
> Depends on:
>   Bluetooth: btusb: Add helper for READ_LOCAL_VERSION command
>
> v3: Sorry for the long delay in getting back to this!
>   - Removed support for devices where we don't have firmware
>   - Divide 8723A/8723B codepaths based on driver_info constant
>   - Added more device IDs from latest Realtek code
>   - Addressed minor review comments
>   - Rename RTK --> RTL
>
> v2:
>   - share main blacklist table with other devices
>   - epatch table parsing endian/alignment fixes
>   - BT_INFO message to inform user
>   - added missing kmalloc error check
>   - fixed skb leak
>   - style fixes
>
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index 3096308..0256ab2 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -24,6 +24,7 @@
>   #include <linux/module.h>
>   #include <linux/usb.h>
>   #include <linux/firmware.h>
> +#include <asm/unaligned.h>
>
>   #include <net/bluetooth/bluetooth.h>
>   #include <net/bluetooth/hci_core.h>
> @@ -52,6 +53,8 @@ static struct usb_driver btusb_driver;
>   #define BTUSB_SWAVE		0x1000
>   #define BTUSB_INTEL_NEW		0x2000
>   #define BTUSB_AMP		0x4000
> +#define BTUSB_RTL8723A		0x8000
> +#define BTUSB_RTL8723B		0x10000
>
>   static const struct usb_device_id btusb_table[] = {
>   	/* Generic Bluetooth USB device */
> @@ -280,6 +283,58 @@ static const struct usb_device_id blacklist_table[] = {
>   	{ USB_VENDOR_AND_INTERFACE_INFO(0x8087, 0xe0, 0x01, 0x01),
>   	  .driver_info = BTUSB_IGNORE },
>
> +	/* Realtek 8723AE Bluetooth devices */
> +	{ USB_DEVICE(0x0930, 0x021d), .driver_info = BTUSB_RTL8723A },
> +	{ USB_DEVICE(0x0bda, 0x0723), .driver_info = BTUSB_RTL8723A },
> +	{ USB_DEVICE(0x0bda, 0x8723), .driver_info = BTUSB_RTL8723A },
> +	{ USB_DEVICE(0x13d3, 0x3394), .driver_info = BTUSB_RTL8723A },
> +
> +	/* Realtek 8723AU Bluetooth devices */
> +	{ USB_DEVICE(0x0bda, 0x0724), .driver_info = BTUSB_RTL8723A },
> +	{ USB_DEVICE(0x0bda, 0x1724), .driver_info = BTUSB_RTL8723A },
> +	{ USB_DEVICE(0x0bda, 0x8725), .driver_info = BTUSB_RTL8723A },
> +	{ USB_DEVICE(0x0bda, 0x872a), .driver_info = BTUSB_RTL8723A },
> +	{ USB_DEVICE(0x0bda, 0x872b), .driver_info = BTUSB_RTL8723A },
> +	{ USB_DEVICE(0x0bda, 0xa724), .driver_info = BTUSB_RTL8723A },
> +
> +	/* Realtek 8723BE Bluetooth devices */
> +	{ USB_DEVICE(0x0489, 0xe085), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x0489, 0xe08b), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x0bda, 0xb001), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x0bda, 0xb002), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x0bda, 0xb003), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x0bda, 0xb004), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x0bda, 0xb005), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x0bda, 0xb723), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x0bda, 0xb728), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x0bda, 0xb72b), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x13d3, 0x3410), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x13d3, 0x3416), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x13d3, 0x3459), .driver_info = BTUSB_RTL8723B },
> +
> +	/* Realtek 8723BU Bluetooth devices */
> +	{ USB_DEVICE(0x0bda, 0xb720), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x0bda, 0xb72a), .driver_info = BTUSB_RTL8723B },
> +
> +	/* Realtek 8821AE Bluetooth devices */
> +	{ USB_DEVICE(0x0b05, 0x17dc), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x0bda, 0x0821), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x0bda, 0x8821), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x13d3, 0x3414), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x13d3, 0x3458), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x13d3, 0x3461), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x13d3, 0x3462), .driver_info = BTUSB_RTL8723B },
> +
> +	/* Realtek 8821AU Bluetooth devices */
> +	{ USB_DEVICE(0x0bda, 0x0823), .driver_info = BTUSB_RTL8723B },
> +
> +	/* Realtek 8761AU Bluetooth devices */
> +	{ USB_DEVICE(0x0bda, 0x8760), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x0bda, 0x8761), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x0bda, 0x8a60), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x0bda, 0xa761), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x0bda, 0xb761), .driver_info = BTUSB_RTL8723B },
> +
>   	{ }	/* Terminating entry */
>   };
>
> @@ -1341,6 +1396,377 @@ static int btusb_setup_csr(struct hci_dev *hdev)
>   	return ret;
>   }
>
> +#define RTL_FRAG_LEN 252
> +
> +struct rtl_download_cmd {
> +	uint8_t index;
> +	uint8_t data[RTL_FRAG_LEN];
> +} __packed;
> +
> +struct rtl_download_response {
> +	uint8_t status;
> +	uint8_t index;
> +} __packed;
> +
> +struct rtl_rom_version_evt {
> +	uint8_t status;
> +	uint8_t version;
> +} __packed;
> +
> +struct rtl_epatch_header {
> +	uint8_t signature[8];
> +	uint32_t fw_version;
> +	uint16_t num_patches;
> +} __packed;
> +
> +static const uint8_t RTL_EPATCH_SIGNATURE[] = {
> +	0x52, 0x65, 0x61, 0x6C, 0x74, 0x65, 0x63, 0x68
> +};
> +
> +#define RTL_ROM_LMP_3499	0x3499
> +#define RTL_ROM_LMP_8723A	0x1200
> +#define RTL_ROM_LMP_8723B	0x8723
> +#define RTL_ROM_LMP_8821A	0x8821
> +#define RTL_ROM_LMP_8761A	0x8761
> +
> +static int rtl_read_rom_version(struct hci_dev *hdev)
> +{
> +	struct rtl_rom_version_evt *rom_version;
> +	struct sk_buff *skb;
> +	int r;
> +
> +	/* Read RTL ROM version command */
> +	skb = __hci_cmd_sync(hdev, 0xfc6d, 0, NULL, HCI_INIT_TIMEOUT);
> +	if (IS_ERR(skb)) {
> +		BT_ERR("Read ROM version failed (%ld)", PTR_ERR(skb));
> +		return PTR_ERR(skb);
> +	}
> +
> +	if (skb->len != sizeof(*rom_version)) {
> +		BT_ERR("RTL version event length mismatch");
> +		kfree_skb(skb);
> +		return -EIO;
> +	}
> +
> +	rom_version = (struct rtl_rom_version_evt *) skb->data;
> +	BT_DBG("rom_version status=%x version=%x",
> +	       rom_version->status, rom_version->version);
> +	if (rom_version->status)
> +		r = 0;
> +	else
> +		r = rom_version->version;
> +
> +	kfree_skb(skb);
> +	return r;
> +}
> +
> +static int rtl8723b_parse_firmware(struct hci_dev *hdev, u16 lmp_subver,
> +				   const struct firmware *fw,
> +				   unsigned char **_buf)
> +{
> +	const uint8_t extension_sig[] = { 0x51, 0x04, 0xfd, 0x77 };
> +	struct rtl_epatch_header *epatch_info;
> +	unsigned char *buf;
> +	int i, len, rom_version;
> +	size_t min_size;
> +	uint8_t opcode, length, data;
> +	int project_id = -1;
> +	const unsigned char *fwptr, *chip_id_base;
> +	const unsigned char *patch_length_base, *patch_offset_base;
> +	u32 patch_offset = 0;
> +	u16 patch_length;
> +	const uint16_t project_id_to_lmp_subver[] = {
> +		RTL_ROM_LMP_8723A,
> +		RTL_ROM_LMP_8723B,
> +		RTL_ROM_LMP_8821A,
> +		RTL_ROM_LMP_8761A
> +	};
> +
> +	rom_version = rtl_read_rom_version(hdev);
> +	if (rom_version < 0)
> +		return rom_version;
> +
> +	BT_DBG("lmp_subver=%x rom_version=%x", lmp_subver, rom_version);
> +
> +	min_size = sizeof(struct rtl_epatch_header) + sizeof(extension_sig) + 3;
> +	if (fw->size < min_size)
> +		return -EINVAL;
> +
> +	fwptr = fw->data + fw->size - sizeof(extension_sig);
> +	if (memcmp(fwptr, extension_sig, sizeof(extension_sig)) != 0) {
> +		BT_ERR("extension section signature mismatch");
> +		return -EINVAL;
> +	}
> +
> +	/* Loop from the end of the firmware parsing instructions, until
> +	 * we find an instruction that identifies the "project ID" for the
> +	 * hardware supported by this firwmare file.
> +	 * Once we have that, we double-check that that project_id is suitable
> +	 * for the hardware we are working with. */
> +	while (fwptr >= fw->data + (sizeof(struct rtl_epatch_header) + 3)) {
> +		opcode = *--fwptr;
> +		length = *--fwptr;
> +		data = *--fwptr;
> +		BT_DBG("check op=%x len=%x data=%x", opcode, length, data);
> +
> +		if (opcode == 0xff) /* EOF */
> +			break;
> +
> +		if (length == 0) {
> +			BT_ERR("found instruction with length 0");
> +			return -EINVAL;
> +		}
> +
> +		if (opcode == 0 && length == 1) {
> +			project_id = data;
> +			break;
> +		}
> +
> +		fwptr -= length;
> +	}
> +
> +	if (project_id < 0) {
> +		BT_ERR("failed to find version instruction");
> +		return -EINVAL;
> +	}
> +
> +	if (project_id > ARRAY_SIZE(project_id_to_lmp_subver)) {
> +		BT_ERR("unknown project id %d", project_id);
> +		return -EINVAL;
> +	}
> +
> +	if (lmp_subver != project_id_to_lmp_subver[project_id]) {
> +		BT_ERR("firmware is for %x but this is a %x",
> +		       project_id_to_lmp_subver[project_id], lmp_subver);
> +		return -EINVAL;
> +	}
> +
> +	epatch_info = (struct rtl_epatch_header *) fw->data;
> +	if (memcmp(epatch_info->signature, RTL_EPATCH_SIGNATURE, 8) != 0) {
> +		BT_ERR("bad EPATCH signature");
> +		return -EINVAL;
> +	}
> +
> +	BT_DBG("fw_version=%x, num_patches=%d",
> +	       epatch_info->fw_version, epatch_info->num_patches);
> +
> +	/* After the rtl_epatch_header there is a funky patch metadata section.
> +	 * Assuming 2 patches, the layout is:
> +	 * ChipID1 ChipID2 PatchLength1 PatchLength2 PatchOffset1 PatchOffset2
> +	 *
> +	 * Find the right patch for this chip. */
> +	min_size += 8 * epatch_info->num_patches;
> +	if (fw->size < min_size)
> +		return -EINVAL;
> +
> +	chip_id_base = fw->data + sizeof(struct rtl_epatch_header);
> +	patch_length_base = chip_id_base +
> +			    (sizeof(u16) * epatch_info->num_patches);
> +	patch_offset_base = patch_length_base +
> +			    (sizeof(u16) * epatch_info->num_patches);
> +	for (i = 0; i < epatch_info->num_patches; i++) {
> +		u16 chip_id = get_unaligned_le16(chip_id_base +
> +						 (i * sizeof(u16)));
> +		if (chip_id == rom_version + 1) {
> +			patch_length = get_unaligned_le16(patch_length_base +
> +							  (i * sizeof(u16)));
> +			patch_offset = get_unaligned_le32(patch_offset_base +
> +							  (i * sizeof(u32)));
> +			break;
> +		}
> +	}
> +
> +	if (!patch_offset) {
> +		BT_ERR("didn't find patch for chip id %d", rom_version);
> +		return -EINVAL;
> +	}
> +
> +	BT_DBG("length=%x offset=%x index %d", patch_length, patch_offset, i);
> +	min_size = patch_offset + patch_length;
> +	if (fw->size < min_size)
> +		return -EINVAL;
> +
> +	/* Copy the firmware into a new buffer and write the version at
> +	 * the end. */
> +	len = patch_length;
> +	buf = kmemdup(fw->data + patch_offset, patch_length, GFP_KERNEL);
> +	if (!buf)
> +		return -ENOMEM;
> +
> +	memcpy(buf + patch_length - 4, &epatch_info->fw_version, 4);
> +
> +	*_buf = buf;
> +	return len;
> +}
> +
> +static int rtl_download_firmware(struct hci_dev *hdev,
> +				 const unsigned char *data, int fw_len)
> +{
> +	struct rtl_download_cmd *dl_cmd;
> +	int frag_num = fw_len / RTL_FRAG_LEN + 1;
> +	int frag_len = RTL_FRAG_LEN;
> +	int ret = 0;
> +	int i;
> +
> +	dl_cmd = kmalloc(sizeof(struct rtl_download_cmd), GFP_KERNEL);
> +	if (!dl_cmd)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < frag_num; i++) {
> +		struct rtl_download_response *dl_resp;
> +		struct sk_buff *skb;
> +
> +		BT_DBG("download fw (%d/%d)", i, frag_num);
> +		dl_cmd->index = i;
> +		if (i == (frag_num - 1)) {
> +			dl_cmd->index |= 0x80; /* data end */
> +			frag_len = fw_len % RTL_FRAG_LEN;
> +		}
> +		memcpy(dl_cmd->data, data, frag_len);
> +
> +		/* Send download command */
> +		skb = __hci_cmd_sync(hdev, 0xfc20, frag_len + 1, dl_cmd,
> +				     HCI_INIT_TIMEOUT);
> +		if (IS_ERR(skb)) {
> +			BT_ERR("download fw command failed (%ld)",
> +			       PTR_ERR(skb));
> +			ret = -PTR_ERR(skb);
> +			goto out;
> +		}
> +
> +		if (skb->len != sizeof(*dl_resp)) {
> +			BT_ERR("download fw event length mismatch");
> +			kfree_skb(skb);
> +			ret = -EIO;
> +			goto out;
> +		}
> +
> +		dl_resp = (struct rtl_download_response *) skb->data;
> +		if (dl_resp->status != 0) {
> +			kfree_skb(skb);
> +			ret = bt_to_errno(dl_resp->status);
> +			goto out;
> +		}
> +
> +		kfree_skb(skb);
> +		data += RTL_FRAG_LEN;
> +	}
> +
> +out:
> +	kfree(dl_cmd);
> +	return ret;
> +}
> +
> +static int btusb_setup_rtl8723a(struct hci_dev *hdev)
> +{
> +	struct btusb_data *data = dev_get_drvdata(&hdev->dev);
> +	struct usb_device *udev = interface_to_usbdev(data->intf);
> +	struct sk_buff *skb;
> +	struct hci_rp_read_local_version *resp;
> +	const struct firmware *fw;
> +	int ret;
> +
> +	skb = btusb_read_local_version(hdev);
> +	if (IS_ERR(skb))
> +		return -PTR_ERR(skb);
> +
> +	resp = (struct hci_rp_read_local_version *) skb->data;
> +	if (resp->lmp_subver != RTL_ROM_LMP_8723A
> +			&& resp->lmp_subver != RTL_ROM_LMP_3499) {
> +		BT_INFO("rtl8723a: assuming no firmware upload needed.");
> +		kfree_skb(skb);
> +		return 0;
> +	}
> +
> +	kfree_skb(skb);
> +
> +	ret = request_firmware(&fw, "rtl_bt/rtl8723a_fw.bin", &udev->dev);
> +	if (ret < 0) {
> +		BT_ERR("Failed to load rtl_bt/rtl8723a_fw.bin");
> +		return ret;
> +	}
> +
> +	if (fw->size < 8) {
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	/* Check that the firmware doesn't have the epatch signature
> +	 * (which is only for RTL8723B and newer). */
> +	if (memcmp(fw->data, RTL_EPATCH_SIGNATURE, 8) == 0) {
> +		BT_ERR("RTL8723A: unexpected EPATCH signature!");
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	ret = rtl_download_firmware(hdev, fw->data, fw->size);
> +
> +out:
> +	release_firmware(fw);
> +	return ret;
> +}
> +
> +static int btusb_setup_rtl8723b(struct hci_dev *hdev)
> +{
> +	struct btusb_data *data = dev_get_drvdata(&hdev->dev);
> +	struct usb_device *udev = interface_to_usbdev(data->intf);
> +	const char *fw_name;
> +	unsigned char *fw_data;
> +	u16 lmp_subver;
> +	const struct firmware *fw;
> +	struct hci_rp_read_local_version *resp;
> +	struct sk_buff *skb;
> +	int ret;
> +
> +	skb = btusb_read_local_version(hdev);
> +	if (IS_ERR(skb))
> +		return -PTR_ERR(skb);
> +
> +	resp = (struct hci_rp_read_local_version *) skb->data;
> +	if (resp->status) {
> +		BT_ERR("rtl fw version event failed (%02x)", resp->status);
> +		kfree_skb(skb);
> +		return bt_to_errno(resp->status);
> +	}
> +
> +	lmp_subver = resp->lmp_subver;
> +	kfree_skb(skb);
> +
> +	switch (lmp_subver) {
> +	case RTL_ROM_LMP_8723B:
> +		fw_name = "rtl_bt/rtl8723b_fw.bin";
> +		break;
> +	case RTL_ROM_LMP_8821A:
> +		fw_name = "rtl_bt/rtl8821a_fw.bin";
> +		break;
> +	case RTL_ROM_LMP_8761A:
> +		fw_name = "rtl_bt/rtl8761a_fw.bin";
> +		break;
> +	default:
> +		BT_INFO("rtl8723b: assuming no firmware upload needed.");
> +		return 0;
> +	}
> +
> +	ret = request_firmware(&fw, fw_name, &udev->dev);
> +	if (ret < 0) {
> +		BT_ERR("Failed to load %s", fw_name);
> +		return ret;
> +	}
> +
> +	ret = rtl8723b_parse_firmware(hdev, lmp_subver, fw, &fw_data);
> +	if (ret < 0)
> +		goto out;
> +
> +	ret = rtl_download_firmware(hdev, fw_data, ret);
> +	kfree(fw_data);
> +	if (ret < 0)
> +		goto out;
> +
> +out:
> +	release_firmware(fw);
> +	return ret;
> +}
> +
>   struct intel_version {
>   	u8 status;
>   	u8 hw_platform;
> @@ -2732,6 +3158,12 @@ static int btusb_probe(struct usb_interface *intf,
>   		set_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks);
>   	}
>
> +	if (id->driver_info & BTUSB_RTL8723A)
> +		hdev->setup = btusb_setup_rtl8723a;
> +
> +	if (id->driver_info & BTUSB_RTL8723B)
> +		hdev->setup = btusb_setup_rtl8723b;
> +
>   	if (id->driver_info & BTUSB_AMP) {
>   		/* AMP controllers do not support SCO packets */
>   		data->isoc = NULL;
>

Daniel,

I have tested the patched driver on RTL8723AE, RTL8723BE, and RTL8821AE. I have 
no BT devices for extended testing, but all 3 could pair with my cell phone and 
discover a number of BT devices such as my TV, etc. I have an RTL8723AU that is 
part of a Radxa Rock SBC, but I have not yet tested it.

As you noted, there are some additional devices that are not included. I have 
firmware for them, but I think leaving them out for the moment is best. For 
completeness, these are the RTL8192EU (0bda:818b and 0bda:818c), the RTL8192EE 
(0bda:8760 and 0bda:8761), and the RTL8812AE (0bda:8a60). My RTL8192EE devices 
do not have BT - such a chip may not exist in the wild, and I have no RTL8812AE 
or RTL8192EU chips. All of these supposedly have an lmp subversion of 0x8761, 
thus the case section for RTL_ROM_LMP_8761A in btusb_setup_rtl8723b() will need 
a second switch statement to select the correct firmware in case those devices 
need to be added. That will be trivial when and if the need arises.

Surely the driver needs to distinguish between the RTL8723A and the others. 
There is no problem with using RTL8723B for the other description, but you 
should probably add a comment to indicate that RTL8821A is also included.

To get some additional testing, I will create a new branch for the BT repo at 
GitHub.

AFAIK, the firmware should be redistributable under the conditions of 
"LICENCE.rtlwifi_firmware.txt" from the linux-firmware repo. Are you planning to 
submit the firmware to that repo, or should I do it?

If you wish, add a "Tested-by: Larry Finger <Larry.Finger@lwfinger.net>".

Larry

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3] Bluetooth: btusb: Add Realtek 8723A/8723B/8761A/8821A support
  2015-02-10 17:33 ` Larry Finger
@ 2015-02-11  1:01   ` Daniel Drake
  0 siblings, 0 replies; 7+ messages in thread
From: Daniel Drake @ 2015-02-11  1:01 UTC (permalink / raw)
  To: Larry Finger
  Cc: Marcel Holtmann, Gustavo F. Padovan, Johan Hedberg,
	Linux Bluetooth mailing list, 陈艳萍

On Tue, Feb 10, 2015 at 11:33 AM, Larry Finger
<Larry.Finger@lwfinger.net> wrote:
> I have tested the patched driver on RTL8723AE, RTL8723BE, and RTL8821AE. I
> have no BT devices for extended testing, but all 3 could pair with my cell
> phone and discover a number of BT devices such as my TV, etc. I have an
> RTL8723AU that is part of a Radxa Rock SBC, but I have not yet tested it.

Thanks for testing!

> As you noted, there are some additional devices that are not included. I
> have firmware for them, but I think leaving them out for the moment is best.

I agree, those can happen in later patches.

I'll send a followup patch with the added comment and your tested-by,
but first I'll wait a couple more days to see if there is more
feedback.

> AFAIK, the firmware should be redistributable under the conditions of
> "LICENCE.rtlwifi_firmware.txt" from the linux-firmware repo. Are you
> planning to submit the firmware to that repo, or should I do it?

I'll do that too. Can you confirm which are the latest firmware files?
For example in the 'new' branch there seem to be 2 different firmwares
for rtl8723b at least.

Thanks
Daniel

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3] Bluetooth: btusb: Add Realtek 8723A/8723B/8761A/8821A support
  2015-02-09 22:59 [PATCH v3] Bluetooth: btusb: Add Realtek 8723A/8723B/8761A/8821A support Daniel Drake
  2015-02-10  0:06 ` Larry Finger
  2015-02-10 17:33 ` Larry Finger
@ 2015-02-12 20:26 ` Marcel Holtmann
  2015-02-12 20:44   ` Daniel Drake
  2 siblings, 1 reply; 7+ messages in thread
From: Marcel Holtmann @ 2015-02-12 20:26 UTC (permalink / raw)
  To: Daniel Drake
  Cc: Gustavo F. Padovan, Johan Hedberg, linux-bluetooth, larry.finger,
	champion_chen

Hi Daniel,

> Realtek ship a variety of bluetooth USB devices that identify
> themselves with standard USB Bluetooth device class values, but
> require a special driver to actually work. Without that driver,
> you never get any scan results.
> 
> More recently however, Realtek appear to have wisened up and simply
> posted a firmware update that makes these devices comply with
> normal btusb protocols. The firmware needs to be uploaded on each boot.
> 
> Based on Realtek code from https://github.com/lwfinger/rtl8723au_bt
> ('new' branch).
> 
> This enables bluetooth support in the Gigabyte Brix GB-BXBT-2807 which
> has this RTL8723BE USB device:
> 
> T:  Bus=01 Lev=01 Prnt=01 Port=01 Cnt=02 Dev#=  3 Spd=12   MxCh= 0
> D:  Ver= 2.10 Cls=e0(wlcon) Sub=01 Prot=01 MxPS=64 #Cfgs=  1
> P:  Vendor=13d3 ProdID=3410 Rev= 2.00
> S:  Manufacturer=Realtek
> S:  Product=Bluetooth Radio
> S:  SerialNumber=00e04c000001
> C:* #Ifs= 2 Cfg#= 1 Atr=e0 MxPwr=500mA
> I:* If#= 0 Alt= 0 #EPs= 3 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
> E:  Ad=81(I) Atr=03(Int.) MxPS=  16 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
> 
> There is no change to the USB descriptor after firmware update,
> however the version read by HCI_OP_READ_LOCAL_VERSION changes from
> 0x8723 to 0x3083.
> 
> I also brought in support for the 8821A/8761A devices, which use
> the same firmware uploading mechanism, plus the 8723A devices,
> which use a more simplistic firmware format. I have not tested
> with these additional devices.
> 
> Signed-off-by: Daniel Drake <drake@endlessm.com>
> ---
> drivers/bluetooth/btusb.c | 432 ++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 432 insertions(+)
> 
> Depends on:
> Bluetooth: btusb: Add helper for READ_LOCAL_VERSION command
> 
> v3: Sorry for the long delay in getting back to this!
> - Removed support for devices where we don't have firmware
> - Divide 8723A/8723B codepaths based on driver_info constant
> - Added more device IDs from latest Realtek code
> - Addressed minor review comments
> - Rename RTK --> RTL
> 
> v2:
> - share main blacklist table with other devices
> - epatch table parsing endian/alignment fixes
> - BT_INFO message to inform user
> - added missing kmalloc error check
> - fixed skb leak
> - style fixes
> 
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index 3096308..0256ab2 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -24,6 +24,7 @@
> #include <linux/module.h>
> #include <linux/usb.h>
> #include <linux/firmware.h>
> +#include <asm/unaligned.h>
> 
> #include <net/bluetooth/bluetooth.h>
> #include <net/bluetooth/hci_core.h>
> @@ -52,6 +53,8 @@ static struct usb_driver btusb_driver;
> #define BTUSB_SWAVE		0x1000
> #define BTUSB_INTEL_NEW		0x2000
> #define BTUSB_AMP		0x4000
> +#define BTUSB_RTL8723A		0x8000
> +#define BTUSB_RTL8723B		0x10000
> 
> static const struct usb_device_id btusb_table[] = {
> 	/* Generic Bluetooth USB device */
> @@ -280,6 +283,58 @@ static const struct usb_device_id blacklist_table[] = {
> 	{ USB_VENDOR_AND_INTERFACE_INFO(0x8087, 0xe0, 0x01, 0x01),
> 	  .driver_info = BTUSB_IGNORE },
> 
> +	/* Realtek 8723AE Bluetooth devices */
> +	{ USB_DEVICE(0x0930, 0x021d), .driver_info = BTUSB_RTL8723A },
> +	{ USB_DEVICE(0x0bda, 0x0723), .driver_info = BTUSB_RTL8723A },
> +	{ USB_DEVICE(0x0bda, 0x8723), .driver_info = BTUSB_RTL8723A },
> +	{ USB_DEVICE(0x13d3, 0x3394), .driver_info = BTUSB_RTL8723A },
> +
> +	/* Realtek 8723AU Bluetooth devices */
> +	{ USB_DEVICE(0x0bda, 0x0724), .driver_info = BTUSB_RTL8723A },
> +	{ USB_DEVICE(0x0bda, 0x1724), .driver_info = BTUSB_RTL8723A },
> +	{ USB_DEVICE(0x0bda, 0x8725), .driver_info = BTUSB_RTL8723A },
> +	{ USB_DEVICE(0x0bda, 0x872a), .driver_info = BTUSB_RTL8723A },
> +	{ USB_DEVICE(0x0bda, 0x872b), .driver_info = BTUSB_RTL8723A },
> +	{ USB_DEVICE(0x0bda, 0xa724), .driver_info = BTUSB_RTL8723A },
> +
> +	/* Realtek 8723BE Bluetooth devices */
> +	{ USB_DEVICE(0x0489, 0xe085), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x0489, 0xe08b), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x0bda, 0xb001), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x0bda, 0xb002), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x0bda, 0xb003), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x0bda, 0xb004), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x0bda, 0xb005), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x0bda, 0xb723), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x0bda, 0xb728), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x0bda, 0xb72b), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x13d3, 0x3410), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x13d3, 0x3416), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x13d3, 0x3459), .driver_info = BTUSB_RTL8723B },
> +
> +	/* Realtek 8723BU Bluetooth devices */
> +	{ USB_DEVICE(0x0bda, 0xb720), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x0bda, 0xb72a), .driver_info = BTUSB_RTL8723B },
> +
> +	/* Realtek 8821AE Bluetooth devices */
> +	{ USB_DEVICE(0x0b05, 0x17dc), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x0bda, 0x0821), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x0bda, 0x8821), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x13d3, 0x3414), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x13d3, 0x3458), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x13d3, 0x3461), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x13d3, 0x3462), .driver_info = BTUSB_RTL8723B },
> +
> +	/* Realtek 8821AU Bluetooth devices */
> +	{ USB_DEVICE(0x0bda, 0x0823), .driver_info = BTUSB_RTL8723B },
> +
> +	/* Realtek 8761AU Bluetooth devices */
> +	{ USB_DEVICE(0x0bda, 0x8760), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x0bda, 0x8761), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x0bda, 0x8a60), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x0bda, 0xa761), .driver_info = BTUSB_RTL8723B },
> +	{ USB_DEVICE(0x0bda, 0xb761), .driver_info = BTUSB_RTL8723B },
> +

do we need all of this? Or can we just issue a HCI command and figure which chipset it is. I remember we had some discussion on this. I just do not remember if it was possible or not.

> 	{ }	/* Terminating entry */
> };
> 
> @@ -1341,6 +1396,377 @@ static int btusb_setup_csr(struct hci_dev *hdev)
> 	return ret;
> }
> 
> +#define RTL_FRAG_LEN 252
> +
> +struct rtl_download_cmd {
> +	uint8_t index;
> +	uint8_t data[RTL_FRAG_LEN];
> +} __packed;

Why is this uint8_t and not __u8 like we do with the other structures.

> +
> +struct rtl_download_response {
> +	uint8_t status;
> +	uint8_t index;
> +} __packed;
> +
> +struct rtl_rom_version_evt {
> +	uint8_t status;
> +	uint8_t version;
> +} __packed;
> +
> +struct rtl_epatch_header {
> +	uint8_t signature[8];
> +	uint32_t fw_version;
> +	uint16_t num_patches;

__le32 and __le16 here please. Or the big endian variants in case this is in big endian.

> +} __packed;
> +
> +static const uint8_t RTL_EPATCH_SIGNATURE[] = {
> +	0x52, 0x65, 0x61, 0x6C, 0x74, 0x65, 0x63, 0x68
> +};

Why is this RTL_EPATCH_ uppercase. I prefer only to have defines in uppercase and not const arrays. However you could define it as string array.

You could do "\x52\x65.." and so on to keep it as define.

> +
> +#define RTL_ROM_LMP_3499	0x3499
> +#define RTL_ROM_LMP_8723A	0x1200
> +#define RTL_ROM_LMP_8723B	0x8723
> +#define RTL_ROM_LMP_8821A	0x8821
> +#define RTL_ROM_LMP_8761A	0x8761
> +
> +static int rtl_read_rom_version(struct hci_dev *hdev)
> +{
> +	struct rtl_rom_version_evt *rom_version;
> +	struct sk_buff *skb;
> +	int r;
> +
> +	/* Read RTL ROM version command */
> +	skb = __hci_cmd_sync(hdev, 0xfc6d, 0, NULL, HCI_INIT_TIMEOUT);
> +	if (IS_ERR(skb)) {
> +		BT_ERR("Read ROM version failed (%ld)", PTR_ERR(skb));
> +		return PTR_ERR(skb);
> +	}
> +
> +	if (skb->len != sizeof(*rom_version)) {
> +		BT_ERR("RTL version event length mismatch");
> +		kfree_skb(skb);
> +		return -EIO;
> +	}
> +
> +	rom_version = (struct rtl_rom_version_evt *) skb->data;
> +	BT_DBG("rom_version status=%x version=%x",
> +	       rom_version->status, rom_version->version);
> +	if (rom_version->status)
> +		r = 0;
> +	else
> +		r = rom_version->version;

I wonder why not use a negative return value. Also lets use something clearer than just r as variable name.

> +
> +	kfree_skb(skb);
> +	return r;
> +}
> +
> +static int rtl8723b_parse_firmware(struct hci_dev *hdev, u16 lmp_subver,
> +				   const struct firmware *fw,
> +				   unsigned char **_buf)
> +{
> +	const uint8_t extension_sig[] = { 0x51, 0x04, 0xfd, 0x77 };
> +	struct rtl_epatch_header *epatch_info;
> +	unsigned char *buf;
> +	int i, len, rom_version;
> +	size_t min_size;
> +	uint8_t opcode, length, data;
> +	int project_id = -1;
> +	const unsigned char *fwptr, *chip_id_base;
> +	const unsigned char *patch_length_base, *patch_offset_base;
> +	u32 patch_offset = 0;
> +	u16 patch_length;
> +	const uint16_t project_id_to_lmp_subver[] = {
> +		RTL_ROM_LMP_8723A,
> +		RTL_ROM_LMP_8723B,
> +		RTL_ROM_LMP_8821A,
> +		RTL_ROM_LMP_8761A
> +	};
> +
> +	rom_version = rtl_read_rom_version(hdev);
> +	if (rom_version < 0)
> +		return rom_version;
> +
> +	BT_DBG("lmp_subver=%x rom_version=%x", lmp_subver, rom_version);
> +
> +	min_size = sizeof(struct rtl_epatch_header) + sizeof(extension_sig) + 3;
> +	if (fw->size < min_size)
> +		return -EINVAL;
> +
> +	fwptr = fw->data + fw->size - sizeof(extension_sig);
> +	if (memcmp(fwptr, extension_sig, sizeof(extension_sig)) != 0) {

	if (memcmp())

> +		BT_ERR("extension section signature mismatch");
> +		return -EINVAL;
> +	}
> +
> +	/* Loop from the end of the firmware parsing instructions, until
> +	 * we find an instruction that identifies the "project ID" for the
> +	 * hardware supported by this firwmare file.
> +	 * Once we have that, we double-check that that project_id is suitable
> +	 * for the hardware we are working with. */

	*/ on extra line.

Please keep the coding style for the network subsystem in line.

> +	while (fwptr >= fw->data + (sizeof(struct rtl_epatch_header) + 3)) {
> +		opcode = *--fwptr;
> +		length = *--fwptr;
> +		data = *--fwptr;

Extra empty line before debug statement.

> +		BT_DBG("check op=%x len=%x data=%x", opcode, length, data);
> +
> +		if (opcode == 0xff) /* EOF */
> +			break;
> +
> +		if (length == 0) {
> +			BT_ERR("found instruction with length 0");
> +			return -EINVAL;
> +		}
> +
> +		if (opcode == 0 && length == 1) {
> +			project_id = data;
> +			break;
> +		}
> +
> +		fwptr -= length;
> +	}
> +
> +	if (project_id < 0) {
> +		BT_ERR("failed to find version instruction");
> +		return -EINVAL;
> +	}
> +
> +	if (project_id > ARRAY_SIZE(project_id_to_lmp_subver)) {
> +		BT_ERR("unknown project id %d", project_id);
> +		return -EINVAL;
> +	}
> +
> +	if (lmp_subver != project_id_to_lmp_subver[project_id]) {
> +		BT_ERR("firmware is for %x but this is a %x",
> +		       project_id_to_lmp_subver[project_id], lmp_subver);
> +		return -EINVAL;
> +	}
> +
> +	epatch_info = (struct rtl_epatch_header *) fw->data;
> +	if (memcmp(epatch_info->signature, RTL_EPATCH_SIGNATURE, 8) != 0) {

	if (memcmp())

> +		BT_ERR("bad EPATCH signature");
> +		return -EINVAL;
> +	}
> +
> +	BT_DBG("fw_version=%x, num_patches=%d",
> +	       epatch_info->fw_version, epatch_info->num_patches);
> +
> +	/* After the rtl_epatch_header there is a funky patch metadata section.
> +	 * Assuming 2 patches, the layout is:
> +	 * ChipID1 ChipID2 PatchLength1 PatchLength2 PatchOffset1 PatchOffset2
> +	 *
> +	 * Find the right patch for this chip. */
> +	min_size += 8 * epatch_info->num_patches;
> +	if (fw->size < min_size)
> +		return -EINVAL;
> +
> +	chip_id_base = fw->data + sizeof(struct rtl_epatch_header);
> +	patch_length_base = chip_id_base +
> +			    (sizeof(u16) * epatch_info->num_patches);
> +	patch_offset_base = patch_length_base +
> +			    (sizeof(u16) * epatch_info->num_patches);
> +	for (i = 0; i < epatch_info->num_patches; i++) {
> +		u16 chip_id = get_unaligned_le16(chip_id_base +
> +						 (i * sizeof(u16)));
> +		if (chip_id == rom_version + 1) {
> +			patch_length = get_unaligned_le16(patch_length_base +
> +							  (i * sizeof(u16)));
> +			patch_offset = get_unaligned_le32(patch_offset_base +
> +							  (i * sizeof(u32)));
> +			break;
> +		}
> +	}
> +
> +	if (!patch_offset) {
> +		BT_ERR("didn't find patch for chip id %d", rom_version);
> +		return -EINVAL;
> +	}
> +
> +	BT_DBG("length=%x offset=%x index %d", patch_length, patch_offset, i);
> +	min_size = patch_offset + patch_length;
> +	if (fw->size < min_size)
> +		return -EINVAL;
> +
> +	/* Copy the firmware into a new buffer and write the version at
> +	 * the end. */
> +	len = patch_length;
> +	buf = kmemdup(fw->data + patch_offset, patch_length, GFP_KERNEL);
> +	if (!buf)
> +		return -ENOMEM;
> +
> +	memcpy(buf + patch_length - 4, &epatch_info->fw_version, 4);
> +
> +	*_buf = buf;
> +	return len;
> +}
> +
> +static int rtl_download_firmware(struct hci_dev *hdev,
> +				 const unsigned char *data, int fw_len)
> +{
> +	struct rtl_download_cmd *dl_cmd;
> +	int frag_num = fw_len / RTL_FRAG_LEN + 1;
> +	int frag_len = RTL_FRAG_LEN;
> +	int ret = 0;
> +	int i;
> +
> +	dl_cmd = kmalloc(sizeof(struct rtl_download_cmd), GFP_KERNEL);
> +	if (!dl_cmd)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < frag_num; i++) {
> +		struct rtl_download_response *dl_resp;
> +		struct sk_buff *skb;
> +
> +		BT_DBG("download fw (%d/%d)", i, frag_num);

Extra empty line here after the debug statement.

> +		dl_cmd->index = i;
> +		if (i == (frag_num - 1)) {
> +			dl_cmd->index |= 0x80; /* data end */
> +			frag_len = fw_len % RTL_FRAG_LEN;
> +		}
> +		memcpy(dl_cmd->data, data, frag_len);
> +
> +		/* Send download command */
> +		skb = __hci_cmd_sync(hdev, 0xfc20, frag_len + 1, dl_cmd,
> +				     HCI_INIT_TIMEOUT);
> +		if (IS_ERR(skb)) {
> +			BT_ERR("download fw command failed (%ld)",
> +			       PTR_ERR(skb));
> +			ret = -PTR_ERR(skb);
> +			goto out;
> +		}
> +
> +		if (skb->len != sizeof(*dl_resp)) {
> +			BT_ERR("download fw event length mismatch");
> +			kfree_skb(skb);
> +			ret = -EIO;
> +			goto out;
> +		}
> +
> +		dl_resp = (struct rtl_download_response *) skb->data;

It is not consistent in the current driver, but *)skb->data please. No extra space.

> +		if (dl_resp->status != 0) {
> +			kfree_skb(skb);
> +			ret = bt_to_errno(dl_resp->status);
> +			goto out;
> +		}
> +
> +		kfree_skb(skb);
> +		data += RTL_FRAG_LEN;
> +	}
> +
> +out:
> +	kfree(dl_cmd);
> +	return ret;
> +}
> +
> +static int btusb_setup_rtl8723a(struct hci_dev *hdev)
> +{
> +	struct btusb_data *data = dev_get_drvdata(&hdev->dev);
> +	struct usb_device *udev = interface_to_usbdev(data->intf);
> +	struct sk_buff *skb;
> +	struct hci_rp_read_local_version *resp;
> +	const struct firmware *fw;
> +	int ret;
> +
> +	skb = btusb_read_local_version(hdev);
> +	if (IS_ERR(skb))
> +		return -PTR_ERR(skb);
> +
> +	resp = (struct hci_rp_read_local_version *) skb->data;
> +	if (resp->lmp_subver != RTL_ROM_LMP_8723A
> +			&& resp->lmp_subver != RTL_ROM_LMP_3499) {

Wrong coding style for indentation.

> +		BT_INFO("rtl8723a: assuming no firmware upload needed.");
> +		kfree_skb(skb);
> +		return 0;
> +	}
> +
> +	kfree_skb(skb);
> +
> +	ret = request_firmware(&fw, "rtl_bt/rtl8723a_fw.bin", &udev->dev);
> +	if (ret < 0) {
> +		BT_ERR("Failed to load rtl_bt/rtl8723a_fw.bin");
> +		return ret;
> +	}
> +
> +	if (fw->size < 8) {
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	/* Check that the firmware doesn't have the epatch signature
> +	 * (which is only for RTL8723B and newer). */
> +	if (memcmp(fw->data, RTL_EPATCH_SIGNATURE, 8) == 0) {

Use !memcmp here.

> +		BT_ERR("RTL8723A: unexpected EPATCH signature!");
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	ret = rtl_download_firmware(hdev, fw->data, fw->size);
> +
> +out:
> +	release_firmware(fw);
> +	return ret;
> +}
> +
> +static int btusb_setup_rtl8723b(struct hci_dev *hdev)
> +{
> +	struct btusb_data *data = dev_get_drvdata(&hdev->dev);
> +	struct usb_device *udev = interface_to_usbdev(data->intf);
> +	const char *fw_name;
> +	unsigned char *fw_data;
> +	u16 lmp_subver;
> +	const struct firmware *fw;
> +	struct hci_rp_read_local_version *resp;
> +	struct sk_buff *skb;
> +	int ret;
> +
> +	skb = btusb_read_local_version(hdev);
> +	if (IS_ERR(skb))
> +		return -PTR_ERR(skb);
> +
> +	resp = (struct hci_rp_read_local_version *) skb->data;
> +	if (resp->status) {
> +		BT_ERR("rtl fw version event failed (%02x)", resp->status);
> +		kfree_skb(skb);
> +		return bt_to_errno(resp->status);
> +	}
> +
> +	lmp_subver = resp->lmp_subver;

This is missing endian conversion. My advice is to actually run make C=2.

> +	kfree_skb(skb);
> +
> +	switch (lmp_subver) {
> +	case RTL_ROM_LMP_8723B:
> +		fw_name = "rtl_bt/rtl8723b_fw.bin";
> +		break;
> +	case RTL_ROM_LMP_8821A:
> +		fw_name = "rtl_bt/rtl8821a_fw.bin";
> +		break;
> +	case RTL_ROM_LMP_8761A:
> +		fw_name = "rtl_bt/rtl8761a_fw.bin";
> +		break;
> +	default:
> +		BT_INFO("rtl8723b: assuming no firmware upload needed.");
> +		return 0;
> +	}
> +
> +	ret = request_firmware(&fw, fw_name, &udev->dev);
> +	if (ret < 0) {
> +		BT_ERR("Failed to load %s", fw_name);
> +		return ret;
> +	}
> +
> +	ret = rtl8723b_parse_firmware(hdev, lmp_subver, fw, &fw_data);
> +	if (ret < 0)
> +		goto out;
> +
> +	ret = rtl_download_firmware(hdev, fw_data, ret);
> +	kfree(fw_data);
> +	if (ret < 0)
> +		goto out;
> +
> +out:
> +	release_firmware(fw);
> +	return ret;
> +}
> +
> struct intel_version {
> 	u8 status;
> 	u8 hw_platform;
> @@ -2732,6 +3158,12 @@ static int btusb_probe(struct usb_interface *intf,
> 		set_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks);
> 	}
> 
> +	if (id->driver_info & BTUSB_RTL8723A)
> +		hdev->setup = btusb_setup_rtl8723a;
> +
> +	if (id->driver_info & BTUSB_RTL8723B)
> +		hdev->setup = btusb_setup_rtl8723b;
> +

I would really prefer to be able to detect this with a HCI command and not have a long list of chipsets.

> 	if (id->driver_info & BTUSB_AMP) {
> 		/* AMP controllers do not support SCO packets */
> 		data->isoc = NULL;

Regards

Marcel


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3] Bluetooth: btusb: Add Realtek 8723A/8723B/8761A/8821A support
  2015-02-12 20:26 ` Marcel Holtmann
@ 2015-02-12 20:44   ` Daniel Drake
  2015-02-12 20:56     ` Marcel Holtmann
  0 siblings, 1 reply; 7+ messages in thread
From: Daniel Drake @ 2015-02-12 20:44 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Gustavo F. Padovan, Johan Hedberg, Linux Bluetooth mailing list,
	Larry Finger, 陈艳萍

Hi Marcel,

Thanks for the feedback! It all makes sense, I just need clarification
on one part.

On Thu, Feb 12, 2015 at 2:26 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
>> +     if (id->driver_info & BTUSB_RTL8723A)
>> +             hdev->setup = btusb_setup_rtl8723a;
>> +
>> +     if (id->driver_info & BTUSB_RTL8723B)
>> +             hdev->setup = btusb_setup_rtl8723b;
>> +
>
> I would really prefer to be able to detect this with a HCI command and not have a long list of chipsets.

Can you explain a little more what you mean here? I should have just 1
entry point for both firmware types, where a HCI command is run, then
it dispatches the appropriate firmware loading code?

I thought by doing it like I did in this patch, I was addressing one
of your comments on the v2 patch, where you said:

"You do know that you can add individual hdev->setup function for each
different firmware loading mechanism. They do not need to be all the
same. I get the feeling you are trying too hard to push it all into a
single entry point."
from http://www.spinics.net/lists/linux-bluetooth/msg50141.html

Thanks
Daniel

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3] Bluetooth: btusb: Add Realtek 8723A/8723B/8761A/8821A support
  2015-02-12 20:44   ` Daniel Drake
@ 2015-02-12 20:56     ` Marcel Holtmann
  0 siblings, 0 replies; 7+ messages in thread
From: Marcel Holtmann @ 2015-02-12 20:56 UTC (permalink / raw)
  To: Daniel Drake
  Cc: Gustavo F. Padovan, Johan Hedberg, Linux Bluetooth mailing list,
	Larry Finger, 陈艳萍

Hi Daniel,

> Thanks for the feedback! It all makes sense, I just need clarification
> on one part.
> 
> On Thu, Feb 12, 2015 at 2:26 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
>>> +     if (id->driver_info & BTUSB_RTL8723A)
>>> +             hdev->setup = btusb_setup_rtl8723a;
>>> +
>>> +     if (id->driver_info & BTUSB_RTL8723B)
>>> +             hdev->setup = btusb_setup_rtl8723b;
>>> +
>> 
>> I would really prefer to be able to detect this with a HCI command and not have a long list of chipsets.
> 
> Can you explain a little more what you mean here? I should have just 1
> entry point for both firmware types, where a HCI command is run, then
> it dispatches the appropriate firmware loading code?
> 
> I thought by doing it like I did in this patch, I was addressing one
> of your comments on the v2 patch, where you said:
> 
> "You do know that you can add individual hdev->setup function for each
> different firmware loading mechanism. They do not need to be all the
> same. I get the feeling you are trying too hard to push it all into a
> single entry point."
> from http://www.spinics.net/lists/linux-bluetooth/msg50141.html

it might be the only way to do it. However the blacklist list is pretty long and if we have any chance to use USB_VENDOR_AND_INTERFACE_INFO to shrink that would be great. If not, then we have to deal with it.

Regards

Marcel


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2015-02-12 20:56 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-02-09 22:59 [PATCH v3] Bluetooth: btusb: Add Realtek 8723A/8723B/8761A/8821A support Daniel Drake
2015-02-10  0:06 ` Larry Finger
2015-02-10 17:33 ` Larry Finger
2015-02-11  1:01   ` Daniel Drake
2015-02-12 20:26 ` Marcel Holtmann
2015-02-12 20:44   ` Daniel Drake
2015-02-12 20:56     ` Marcel Holtmann

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.