Re: [PATCH v7 00/12] Bluetooth: btintel: Refactoring setup routines

From: Marcel Holtmann <marcel@holtmann.org>
To: Tedd Ho-Jeong An <hj.tedd.an@gmail.com>
Cc: BlueZ <linux-bluetooth@vger.kernel.org>,
	Tedd Ho-Jeong An <tedd.an@intel.com>
Subject: Re: [PATCH v7 00/12] Bluetooth: btintel: Refactoring setup routines
Date: Thu, 5 Aug 2021 16:05:28 +0200	[thread overview]
Message-ID: <D943EB26-16E4-415F-9AEC-88D9DF4EC6E7@holtmann.org> (raw)
In-Reply-To: <20210805003219.23221-1-hj.tedd.an@gmail.com>

Hi Tedd,

> Changes in v7:
> - Rebase the patchsets to the tip of the bluetooth-next
>  6eefec4a0 ("Bluetooth: Add additional Bluetooth part for Realtek 8852AE")
> 
> - Some basic testing were done with the following Intel devices to verify
>  the firmware loading and some profiles (LE HID, A2DP, HFP)
> 	WP2_B3
> 	WP2_B5
> 	StP
> 	SdP
> 	SfP
> 	WsP
> 	ThP
> 	CcP
> 	TyP
> 
> 
> This patch set refactors the multiple setup routines for various Intel devices
> to a combined single entry. Here are the highlight of the changes:
> 
> 1. Updated hci_alloc_dev() to allocate the hdev object with an extra buffer
>   for the private data. btintel introduces the btintel_data struct and
>   store it to the private data in hdev object.
> 
> 2. Added a single entry for setup and shutdown and uses the
>   HCI_Intel_Read_Version command to identify the device, instead of
>   relying on the USB VID and PID. The new HCI_Intel_Read_Version command
>   is used for both legacy ROM, legacy Bootloader and TLV based bootloader.
> 
>   Also, btintel_configure_setup() is added to setup the most of hdev
>   callbacks, unless it is a transport specific functions.
> 
>   After identifying the device, it calls the corresponding setup routines.
>   These routines were copied from btusb to btintel and changes are none or
>   very minimal.
> 
> 3. Keep the state of bootloader in btintel object. The bootloader state
>   is agnostic to the transport type, so btintel uses the btintel_data
>   to keep track of the state in the private data section in hdev.
> 
>   Also, added macros to set/clear/test flags to simplify the code.
> 
> 5. Cleaned up the exported functions and make it static as much as possible
>   if not necessary.
> 
> 6. From the JfP/ThP, the operational firmware support the new TLV based
>   HCI_Intel_Read_Version command, which confues the usage during the
>   setup routine. So, the check for firmware variant of those legacy
>   bootloader sku is added to use the legacy bootloader setup call.
> 
> 7. All of HCI quirks for Intel devices are moved in the setup routines.
>   There are several HCI quirks for Intel devices and some of them are
>   for all Intel devices and some of them are for a specific devices.
> 
>   The flag for HCI quirks are removed from the .driver_info, and applying
>   HCI quirks are done in combined setup routine depends on the hw_variant.
> 
> 8. Combined the setting the MSFT extension support in the combined setup
>   routine now depends on the hw_variant.
> 
> 
> Tedd Ho-Jeong An (12):
>  Bluetooth: Add support hdev to allocate private data
>  Bluetooth: btintel: Add combined setup and shutdown functions
>  Bluetooth: btintel: Refactoring setup routine for legacy ROM sku
>  Bluetooth: btintel: Add btintel data struct
>  Bluetooth: btintel: Fix the first HCI command not work with ROM device
>  Bluetooth: btintel: Fix the LED is not turning off immediately
>  Bluetooth: btintel: Add combined set_diag functions
>  Bluetooth: btintel: Refactoring setup routine for bootloader devices
>  Bluetooth: btintel: Move hci quirks to setup routine
>  Bluetooth: btintel: Clean the exported function to static
>  Bluetooth: btintel: Fix the legacy bootloader returns tlv based
>    version
>  Bluetooth: btintel: Combine setting up MSFT extension
> 
> drivers/bluetooth/btintel.c      | 1314 ++++++++++++++++++++++++++++--
> drivers/bluetooth/btintel.h      |  119 ++-
> drivers/bluetooth/btusb.c        | 1128 +------------------------
> include/net/bluetooth/hci_core.h |   13 +-
> net/bluetooth/hci_core.c         |   13 +-
> 5 files changed, 1364 insertions(+), 1223 deletions(-)

all 12 patches have been applied to bluetooth-next tree.

Regards

Marcel