Return-Path: Sender: Larry Finger Message-ID: <552F2923.2020007@lwfinger.net> Date: Wed, 15 Apr 2015 22:14:43 -0500 From: Larry Finger MIME-Version: 1.0 To: Marcel Holtmann , Daniel Drake CC: "Gustavo F. Padovan" , Johan Hedberg , BlueZ development , Champion Chen , carlo@endlessm.com Subject: Re: [PATCH v7] Bluetooth: btusb: Add Realtek 8723A/8723B/8761A/8821A support References: <1429130874-24831-1-git-send-email-drake@endlessm.com> <3B9B5B7C-9801-4A1D-8585-59DBD156148C@holtmann.org> In-Reply-To: <3B9B5B7C-9801-4A1D-8585-59DBD156148C@holtmann.org> Content-Type: text/plain; charset=windows-1252; format=flowed List-ID: On 04/15/2015 05:26 PM, Marcel Holtmann wrote: > 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. >> >> 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 >> Tested-by: Larry Finger >> --- >> 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 >> #include >> #include >> +#include >> >> #include >> #include >> @@ -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. Marcel, 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. Larry