From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id C7D9EC2D0E8 for ; Thu, 26 Mar 2020 06:12:56 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id AACDF20772 for ; Thu, 26 Mar 2020 06:12:56 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727575AbgCZGMy convert rfc822-to-8bit (ORCPT ); Thu, 26 Mar 2020 02:12:54 -0400 Received: from coyote.holtmann.net ([212.227.132.17]:57475 "EHLO mail.holtmann.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725854AbgCZGMy (ORCPT ); Thu, 26 Mar 2020 02:12:54 -0400 Received: from marcel-macpro.fritz.box (p4FEFC5A7.dip0.t-ipconnect.de [79.239.197.167]) by mail.holtmann.org (Postfix) with ESMTPSA id 42F18CECDA; Thu, 26 Mar 2020 07:22:24 +0100 (CET) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 13.4 \(3608.80.23.2.2\)) Subject: Re: [PATCH v2 1/2] Bluetooth: btusb: Indicate Microsoft vendor extension for Intel 9460/9560 and 9160/9260 From: Marcel Holtmann In-Reply-To: Date: Thu, 26 Mar 2020 07:12:52 +0100 Cc: Bluetooth Kernel Mailing List , Luiz Augusto von Dentz , Alain Michaud , "David S. Miller" , Jakub Kicinski , Johan Hedberg , LKML , netdev Content-Transfer-Encoding: 8BIT Message-Id: References: <20200325070336.1097-1-mcchou@chromium.org> <20200325000332.v2.1.I0e975833a6789e8acc74be7756cd54afde6ba98c@changeid> <72699110-843A-4382-8FF1-20C5D4D557A2@holtmann.org> To: Miao-chen Chou X-Mailer: Apple Mail (2.3608.80.23.2.2) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Hi Miao-chen, >>>>> This adds a bit mask of driver_info for Microsoft vendor extension and >>>>> indicates the support for Intel 9460/9560 and 9160/9260. See >>>>> https://docs.microsoft.com/en-us/windows-hardware/drivers/bluetooth/ >>>>> microsoft-defined-bluetooth-hci-commands-and-events for more information >>>>> about the extension. This was verified with Intel ThunderPeak BT controller >>>>> where msft_vnd_ext_opcode is 0xFC1E. >>>>> >>>>> Signed-off-by: Miao-chen Chou >>>>> --- >>>>> >>>>> Changes in v2: >>>>> - Define struct msft_vnd_ext and add a field of this type to struct >>>>> hci_dev to facilitate the support of Microsoft vendor extension. >>>>> >>>>> drivers/bluetooth/btusb.c | 14 ++++++++++++-- >>>>> include/net/bluetooth/hci_core.h | 6 ++++++ >>>>> 2 files changed, 18 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c >>>>> index 3bdec42c9612..4c49f394f174 100644 >>>>> --- a/drivers/bluetooth/btusb.c >>>>> +++ b/drivers/bluetooth/btusb.c >>>>> @@ -58,6 +58,7 @@ static struct usb_driver btusb_driver; >>>>> #define BTUSB_CW6622 0x100000 >>>>> #define BTUSB_MEDIATEK 0x200000 >>>>> #define BTUSB_WIDEBAND_SPEECH 0x400000 >>>>> +#define BTUSB_MSFT_VND_EXT 0x800000 >>>>> >>>>> static const struct usb_device_id btusb_table[] = { >>>>> /* Generic Bluetooth USB device */ >>>>> @@ -335,7 +336,8 @@ static const struct usb_device_id blacklist_table[] = { >>>>> >>>>> /* Intel Bluetooth devices */ >>>>> { USB_DEVICE(0x8087, 0x0025), .driver_info = BTUSB_INTEL_NEW | >>>>> - BTUSB_WIDEBAND_SPEECH }, >>>>> + BTUSB_WIDEBAND_SPEECH | >>>>> + BTUSB_MSFT_VND_EXT }, >>>>> { USB_DEVICE(0x8087, 0x0026), .driver_info = BTUSB_INTEL_NEW | >>>>> BTUSB_WIDEBAND_SPEECH }, >>>>> { USB_DEVICE(0x8087, 0x0029), .driver_info = BTUSB_INTEL_NEW | >>>>> @@ -348,7 +350,8 @@ static const struct usb_device_id blacklist_table[] = { >>>>> { USB_DEVICE(0x8087, 0x0aa7), .driver_info = BTUSB_INTEL | >>>>> BTUSB_WIDEBAND_SPEECH }, >>>>> { USB_DEVICE(0x8087, 0x0aaa), .driver_info = BTUSB_INTEL_NEW | >>>>> - BTUSB_WIDEBAND_SPEECH }, >>>>> + BTUSB_WIDEBAND_SPEECH | >>>>> + BTUSB_MSFT_VND_EXT }, >>>>> >>>>> /* Other Intel Bluetooth devices */ >>>>> { USB_VENDOR_AND_INTERFACE_INFO(0x8087, 0xe0, 0x01, 0x01), >>>>> @@ -3734,6 +3737,8 @@ static int btusb_probe(struct usb_interface *intf, >>>>> hdev->send = btusb_send_frame; >>>>> hdev->notify = btusb_notify; >>>>> >>>>> + hdev->msft_ext.opcode = HCI_OP_NOP; >>>>> + >>>> >>>> do this in the hci_alloc_dev procedure for every driver. This doesn’t belong in the driver. >>> Thanks for the note, I will address this. >>>> >>>>> #ifdef CONFIG_PM >>>>> err = btusb_config_oob_wake(hdev); >>>>> if (err) >>>>> @@ -3800,6 +3805,11 @@ static int btusb_probe(struct usb_interface *intf, >>>>> set_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks); >>>>> set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks); >>>>> set_bit(HCI_QUIRK_NON_PERSISTENT_DIAG, &hdev->quirks); >>>>> + >>>>> + if (id->driver_info & BTUSB_MSFT_VND_EXT && >>>>> + (id->idProduct == 0x0025 || id->idProduct == 0x0aaa)) { >>>> >>>> Please scrap this extra check. You already selected out the PID with the blacklist_table. In addition, I do not want to add a PID in two places in the driver. >>> If we scrap the check around idProduct, how do we tell two controllers >>> apart if they use different opcode for Microsoft vendor extension? >> >> for Intel controllers this is highly unlikely. If we really decide to change the opcode in newer firmware versions, we then deal with it at that point. >> >> However for Intel controllers I have the feeling that we better do it after the Read the Intel version information and then do it based on hardware revision and firmware version. > I would agree with you given that the FW loaded for the same HW can > differ, and different FW version may have different configuration in > terms of the use of extensions. But it's not clear to me how we can > tell whether an extension is supported based on a version number. Is > there any implication on the support of an extension given a FW > version (e.g. any FW version greater than 10 would support MSFT > extension)? that is for us to figure out. I will get back to you on that. Regards Marcel