Re: [PATCH v7] Bluetooth: btusb: Add Realtek 8723A/8723B/8761A/8821A support

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

 



Hi Larry,

>>> 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.
>>> 
>>> This has also been tested on RTL8723AE and RTL8821AE. Support for
>>> RTL8761A has also been added, but that is untested.
>>> 
>>> Signed-off-by: Daniel Drake <drake@xxxxxxxxxxxx>
>>> Tested-by: Larry Finger <Larry.Finger@xxxxxxxxxxxx>
>>> ---
>>> drivers/bluetooth/btusb.c | 413 ++++++++++++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 413 insertions(+)
>> 
>> overall this looks pretty good. I like it. So we just need to do some minor modifications before we can merge it.
>> 
>>> 
>>> v7:
>>> - Rebase on bluetooth-next
>>> - Add suspend/resume support
>>> 
>>> v6:
>>> - Really add firmware log message.
>>> 
>>> v5:
>>> - log firmware filename before loading it
>>> - this has passed testing from Larry and friends
>>> 
>>> v4:
>>> - Simplify ID matching: match just the Realtek vendor, and the non-Realtek
>>>   device IDs. Specific chip detection is then done with READ_LOCAL_VERSION.
>>> - Endianness fixes
>>> - Addressed other review comments
>>> 
>>> v3:
>>> - 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 de7b236..1ca7e99 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>
>>> @@ -57,6 +58,7 @@ static struct usb_driver btusb_driver;
>>> #define BTUSB_AMP		0x4000
>>> #define BTUSB_QCA_ROME		0x8000
>>> #define BTUSB_BCM_APPLE		0x10000
>>> +#define BTUSB_REALTEK		0x20000
>>> 
>>> static const struct usb_device_id btusb_table[] = {
>>> 	/* Generic Bluetooth USB device */
>>> @@ -288,6 +290,28 @@ static const struct usb_device_id blacklist_table[] = {
>>> 	{ USB_VENDOR_AND_INTERFACE_INFO(0x8087, 0xe0, 0x01, 0x01),
>>> 	  .driver_info = BTUSB_IGNORE },
>>> 
>>> +	/* Realtek Bluetooth devices */
>>> +	{ USB_VENDOR_AND_INTERFACE_INFO(0x0bda, 0xe0, 0x01, 0x01),
>>> +	  .driver_info = BTUSB_REALTEK },
>>> +
>>> +	/* Additional Realtek 8723AE Bluetooth devices */
>>> +	{ USB_DEVICE(0x0930, 0x021d), .driver_info = BTUSB_REALTEK },
>>> +	{ USB_DEVICE(0x13d3, 0x3394), .driver_info = BTUSB_REALTEK },
>>> +
>>> +	/* Additional Realtek 8723BE Bluetooth devices */
>>> +	{ USB_DEVICE(0x0489, 0xe085), .driver_info = BTUSB_REALTEK },
>>> +	{ USB_DEVICE(0x0489, 0xe08b), .driver_info = BTUSB_REALTEK },
>>> +	{ USB_DEVICE(0x13d3, 0x3410), .driver_info = BTUSB_REALTEK },
>>> +	{ USB_DEVICE(0x13d3, 0x3416), .driver_info = BTUSB_REALTEK },
>>> +	{ USB_DEVICE(0x13d3, 0x3459), .driver_info = BTUSB_REALTEK },
>>> +
>>> +	/* Additional Realtek 8821AE Bluetooth devices */
>>> +	{ USB_DEVICE(0x0b05, 0x17dc), .driver_info = BTUSB_REALTEK },
>>> +	{ USB_DEVICE(0x13d3, 0x3414), .driver_info = BTUSB_REALTEK },
>>> +	{ USB_DEVICE(0x13d3, 0x3458), .driver_info = BTUSB_REALTEK },
>>> +	{ USB_DEVICE(0x13d3, 0x3461), .driver_info = BTUSB_REALTEK },
>>> +	{ USB_DEVICE(0x13d3, 0x3462), .driver_info = BTUSB_REALTEK },
>>> +
>>> 	{ }	/* Terminating entry */
>>> };
>>> 
>>> @@ -310,6 +334,7 @@ struct btusb_data {
>>> 	struct usb_interface *intf;
>>> 	struct usb_interface *isoc;
>>> 
>>> +	unsigned long driver_info;
>>> 	unsigned long flags;
>>> 
>>> 	struct work_struct work;
>>> @@ -1345,6 +1370,376 @@ static int btusb_setup_csr(struct hci_dev *hdev)
>>> 	return ret;
>>> }
>>> 
>>> +#define RTL_FRAG_LEN 252
>>> +
>>> +struct rtl_download_cmd {
>>> +	__u8 index;
>>> +	__u8 data[RTL_FRAG_LEN];
>>> +} __packed;
>>> +
>>> +struct rtl_download_response {
>>> +	__u8 status;
>>> +	__u8 index;
>>> +} __packed;
>>> +
>>> +struct rtl_rom_version_evt {
>>> +	__u8 status;
>>> +	__u8 version;
>>> +} __packed;
>>> +
>>> +struct rtl_epatch_header {
>>> +	__u8 signature[8];
>>> +	__le32 fw_version;
>>> +	__le16 num_patches;
>>> +} __packed;
>>> +
>>> +#define RTL_EPATCH_SIGNATURE	"Realtech"
>>> +#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));
>> 
>> I would like to do BT_ERR("%: ..", hdev->name, ..) here for all commands. And this should apply to all BT_ERR.
>> 
>>> +		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);
>> 
>> Please turn this into a BT_INFO("%s: ..", hdev->name, ..) style information print. I like to standardize this a little for all drivers.
>> 
>>> +	if (!rom_version->status)
>>> +		r = rom_version->version;
>>> +	else
>>> +		r = bt_to_errno(rom_version->status);
>>> +
>>> +	kfree_skb(skb);
>>> +	return r;
>>> +}
>> 
>> So I had a similar detail when trying to convert Atheros UART support to a kernel driver. I would prefer if we keep int return value for pure status and not overload it with other meanings.
>> 
>> static int rtl_read_rom_version(struct hci_dev *hdev, u8 *version)
>> {
>> }
>> 
>>> +
>>> +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, num_patches;
>>> +	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);
>> 
>> Please not do this twice. Lets print this as BT_INFO in rtl_read_rom_version and be done with it.
>> 
>>> +
>>> +	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;
>>> +	}
>>> +
>>> +	num_patches = le16_to_cpu(epatch_info->num_patches);
>>> +	BT_DBG("fw_version=%x, num_patches=%d",
>>> +	       le32_to_cpu(epatch_info->fw_version), 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 * 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) * num_patches);
>>> +	patch_offset_base = patch_length_base + (sizeof(u16) * num_patches);
>>> +	for (i = 0; i < 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);
>>> +	const struct firmware *fw;
>>> +	int ret;
>>> +
>>> +	BT_INFO("%s: rtl: loading rtl_bt/rtl8723a_fw.bin", hdev->name);
>>> +	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)) {
>>> +		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, u16 lmp_subver,
>>> +				const char *fw_name)
>>> +{
>>> +	struct btusb_data *data = dev_get_drvdata(&hdev->dev);
>>> +	struct usb_device *udev = interface_to_usbdev(data->intf);
>>> +	unsigned char *fw_data;
>>> +	const struct firmware *fw;
>>> +	int ret;
>>> +
>>> +	BT_INFO("%s: rtl: loading %s", hdev->name, fw_name);
>>> +	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;
>>> +}
>>> +
>>> +static int btusb_setup_realtek(struct hci_dev *hdev)
>>> +{
>>> +	struct sk_buff *skb;
>>> +	struct hci_rp_read_local_version *resp;
>>> +	u16 lmp_subver;
>>> +
>>> +	skb = btusb_read_local_version(hdev);
>>> +	if (IS_ERR(skb))
>>> +		return -PTR_ERR(skb);
>>> +
>>> +	resp = (struct hci_rp_read_local_version *)skb->data;
>>> +	BT_INFO("%s: rtl: examining hci_ver=%02x hci_rev=%04x lmp_ver=%02x "
>>> +		"lmp_subver=%04x", hdev->name, resp->hci_ver, resp->hci_rev,
>>> +		resp->lmp_ver, resp->lmp_subver);
>>> +
>>> +	lmp_subver = le16_to_cpu(resp->lmp_subver);
>>> +	kfree_skb(skb);
>>> +
>>> +	/* Match a set of subver values that correspond to stock firmware,
>>> +	 * which is not compatible with standard btusb.
>>> +	 * If matched, upload an alternative firmware that does conform to
>>> +	 * standard btusb. Once that firmware is uploaded, the subver changes
>>> +	 * to a different value.
>>> +	 */
>>> +	switch (lmp_subver) {
>>> +	case RTL_ROM_LMP_8723A:
>>> +	case RTL_ROM_LMP_3499:
>>> +		return btusb_setup_rtl8723a(hdev);
>>> +	case RTL_ROM_LMP_8723B:
>>> +		return btusb_setup_rtl8723b(hdev, lmp_subver,
>>> +					    "rtl_bt/rtl8723b_fw.bin");
>>> +	case RTL_ROM_LMP_8821A:
>>> +		return btusb_setup_rtl8723b(hdev, lmp_subver,
>>> +					    "rtl_bt/rtl8821a_fw.bin");
>>> +	case RTL_ROM_LMP_8761A:
>>> +		return btusb_setup_rtl8723b(hdev, lmp_subver,
>>> +					    "rtl_bt/rtl8761a_fw.bin");
>>> +	default:
>>> +		BT_INFO("rtl: assuming no firmware upload needed.");
>>> +		return 0;
>>> +	}
>>> +}
>>> +
>> 
>> Since this is not your fault, I am willing to merge this change (after minor cleanup) and then convert it over to move the vendor HCI commands into btrtl.ko separate module.
>> 
>> I have already done this for btbcm.ko and btintel.ko and I will do this with other vendor details as well. So this is the strategy that we are heading towards. As I said, this is something that came along and is a new requirement and I am willing to do fix this up for you after merging the patch. So nothing to worry about. It is just a heads up.
>> 
>>> static const struct firmware *btusb_setup_intel_get_fw(struct hci_dev *hdev,
>>> 						       struct intel_version *ver)
>>> {
>>> @@ -2682,6 +3077,7 @@ static int btusb_probe(struct usb_interface *intf,
>>> 
>>> 	data->udev = interface_to_usbdev(intf);
>>> 	data->intf = intf;
>>> +	data->driver_info = id->driver_info;
>>> 
>>> 	INIT_WORK(&data->work, btusb_work);
>>> 	INIT_WORK(&data->waker, btusb_waker);
>>> @@ -2776,6 +3172,9 @@ static int btusb_probe(struct usb_interface *intf,
>>> 		hdev->set_bdaddr = btusb_set_bdaddr_ath3012;
>>> 	}
>>> 
>>> +	if (id->driver_info & BTUSB_REALTEK)
>>> +		hdev->setup = btusb_setup_realtek;
>>> +
>>> 	if (id->driver_info & BTUSB_AMP) {
>>> 		/* AMP controllers do not support SCO packets */
>>> 		data->isoc = NULL;
>>> @@ -2906,6 +3305,20 @@ static int btusb_suspend(struct usb_interface *intf, pm_message_t message)
>>> 	btusb_stop_traffic(data);
>>> 	usb_kill_anchored_urbs(&data->tx_anchor);
>>> 
>>> +	/* For an ordinary suspend (where you would expect the device to be
>>> +	 * powered down or put in low-power mode), Realtek devices lose their
>>> +	 * updated firmware, but the hub doesn't notice any status change.
>>> +	 * Explicitly request a reset on resume.
>>> +	 *
>>> +	 * For the case where the device is powered during suspend, assume
>>> +	 * this is not needed (like the vendor code). In my testing of this
>>> +	 * case, the device fails to remain powered during suspend and hence is
>>> +	 * automatically reset during resume.
>>> +	 */
>>> +	if (data->driver_info & BTUSB_REALTEK &&
>>> +	    !device_may_wakeup(&data->udev->dev))
>>> +		data->udev->reset_resume = 1;
>>> +
>> 
>> This needs to be a separate patch on top of adding basic support for Realtek devices. So please split this out.
>> 
>> And while you do, please create a data->flags for it and not copy the driver_info into the btusb_data structure. Lets define a common flag indicating the need for reset_resume = 1. And then only if the Realtek hardware has been detected, this flag gets set.
> 
> I know you are busy, but I would like to know why it takes so long to review patches for btusb. After testing the driver with V5 of the patch, I placed the modified btusb.c in a GitHub repo. As that was during the 3.9-rcX series, I expected these changes to be in kernel 4.1, and I expressed that expectation to the users of that repo. Now, I find that the patch will be available in 4.2 at the earliest.
> 
> Please remember that there are many users of Realtek devices that are waiting for the standard btusb driver in the kernel to support their device.

I think that I have reviewed all patches pretty much on the spot. If you think that I missed a patch, then someone needs to ping me. You see the amount emails that go through this mailing list. And that is just a tiny portion of my daily emails.

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