[PATCH 1/3] Bluetooth: btusb: Make use of of BIT macro to declare flags

[PATCH 1/3] Bluetooth: btusb: Make use of of BIT macro to declare flags

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This makes use of BIT macro when declaring BTUSB flags which makes it
understand how many bits are in use and many are left.

Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
 drivers/bluetooth/btusb.c | 54 +++++++++++++++++++--------------------
 1 file changed, 27 insertions(+), 27 deletions(-)

diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 558151f2f150..34d008380fdb 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -36,33 +36,33 @@ static bool reset = true;
 
 static struct usb_driver btusb_driver;
 
-#define BTUSB_IGNORE		0x01
-#define BTUSB_DIGIANSWER	0x02
-#define BTUSB_CSR		0x04
-#define BTUSB_SNIFFER		0x08
-#define BTUSB_BCM92035		0x10
-#define BTUSB_BROKEN_ISOC	0x20
-#define BTUSB_WRONG_SCO_MTU	0x40
-#define BTUSB_ATH3012		0x80
-#define BTUSB_INTEL_COMBINED	0x100
-#define BTUSB_INTEL_BOOT	0x200
-#define BTUSB_BCM_PATCHRAM	0x400
-#define BTUSB_MARVELL		0x800
-#define BTUSB_SWAVE		0x1000
-#define BTUSB_AMP		0x4000
-#define BTUSB_QCA_ROME		0x8000
-#define BTUSB_BCM_APPLE		0x10000
-#define BTUSB_REALTEK		0x20000
-#define BTUSB_BCM2045		0x40000
-#define BTUSB_IFNUM_2		0x80000
-#define BTUSB_CW6622		0x100000
-#define BTUSB_MEDIATEK		0x200000
-#define BTUSB_WIDEBAND_SPEECH	0x400000
-#define BTUSB_VALID_LE_STATES   0x800000
-#define BTUSB_QCA_WCN6855	0x1000000
-#define BTUSB_INTEL_BROKEN_SHUTDOWN_LED	0x2000000
-#define BTUSB_INTEL_BROKEN_INITIAL_NCMD 0x4000000
-#define BTUSB_INTEL_NO_WBS_SUPPORT	0x8000000
+#define BTUSB_IGNORE			BIT(0)
+#define BTUSB_DIGIANSWER		BIT(1)
+#define BTUSB_CSR			BIT(2)
+#define BTUSB_SNIFFER			BIT(3)
+#define BTUSB_BCM92035			BIT(4)
+#define BTUSB_BROKEN_ISOC		BIT(5)
+#define BTUSB_WRONG_SCO_MTU		BIT(6)
+#define BTUSB_ATH3012			BIT(7)
+#define BTUSB_INTEL_COMBINED		BIT(8)
+#define BTUSB_INTEL_BOOT		BIT(9)
+#define BTUSB_BCM_PATCHRAM		BIT(10)
+#define BTUSB_MARVELL			BIT(11)
+#define BTUSB_SWAVE			BIT(12)
+#define BTUSB_AMP			BIT(13)
+#define BTUSB_QCA_ROME			BIT(14)
+#define BTUSB_BCM_APPLE			BIT(15)
+#define BTUSB_REALTEK			BIT(16)
+#define BTUSB_BCM2045			BIT(17)
+#define BTUSB_IFNUM_2			BIT(18)
+#define BTUSB_CW6622			BIT(19)
+#define BTUSB_MEDIATEK			BIT(20)
+#define BTUSB_WIDEBAND_SPEECH		BIT(21)
+#define BTUSB_VALID_LE_STATES		BIT(22)
+#define BTUSB_QCA_WCN6855		BIT(23)
+#define BTUSB_INTEL_BROKEN_SHUTDOWN_LED	BIT(24)
+#define BTUSB_INTEL_BROKEN_INITIAL_NCMD BIT(25)
+#define BTUSB_INTEL_NO_WBS_SUPPORT	BIT(26)
 
 static const struct usb_device_id btusb_table[] = {
 	/* Generic Bluetooth USB device */
-- 
2.35.1

[PATCH 2/3] Bluetooth: HCI: Add HCI_QUIRK_BROKEN_ENHANCED_SETUP_SCO quirk

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This adds HCI_QUIRK_BROKEN_ENHANCED_SETUP_SCO quirk which can be used
to mark HCI_Enhanced_Setup_Synchronous_Connection as broken even if its
support command bit are set since some controller report it as supported
but the command don't work properly with some configurations
(e.g. BT_VOICE_TRANSPARENT/mSBC).

Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
 include/net/bluetooth/hci.h      | 9 +++++++++
 include/net/bluetooth/hci_core.h | 8 ++++++--
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 35c073d44ec5..a4da339aab07 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -255,6 +255,15 @@ enum {
 	 * during the hdev->setup vendor callback.
 	 */
 	HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER,
+
+	/*
+	 * When this quirk is set, disables the use of
+	 * HCI_OP_ENHANCED_SETUP_SYNC_CONN command to setup SCO connections.
+	 *
+	 * This quirk can be set before hci_register_dev is called or
+	 * during the hdev->setup vendor callback.
+	 */
+	HCI_QUIRK_BROKEN_ENHANCED_SETUP_SCO,
 };
 
 /* HCI device flags */
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index d5377740e99c..7a9795783850 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1492,8 +1492,12 @@ void hci_conn_del_sysfs(struct hci_conn *conn);
 #define privacy_mode_capable(dev) (use_ll_privacy(dev) && \
 				   (hdev->commands[39] & 0x04))
 
-/* Use enhanced synchronous connection if command is supported */
-#define enhanced_sco_capable(dev) ((dev)->commands[29] & 0x08)
+/* Use enhanced synchronous connection if command is supported and its quirk
+ * has not been set.
+ */
+#define enhanced_sco_capable(dev) (((dev)->commands[29] & 0x08) && \
+				   !test_bit(HCI_QUIRK_BROKEN_ENHANCED_SETUP_SCO, \
+					     &(dev)->quirks))
 
 /* Use ext scanning if set ext scan param and ext scan enable is supported */
 #define use_ext_scan(dev) (((dev)->commands[37] & 0x20) && \
-- 
2.35.1

Re: [PATCH 2/3] Bluetooth: HCI: Add HCI_QUIRK_BROKEN_ENHANCED_SETUP_SCO quirk

[Marcel Holtmann]

Hi Luiz,

> This adds HCI_QUIRK_BROKEN_ENHANCED_SETUP_SCO quirk which can be used
> to mark HCI_Enhanced_Setup_Synchronous_Connection as broken even if its
> support command bit are set since some controller report it as supported
> but the command don't work properly with some configurations
> (e.g. BT_VOICE_TRANSPARENT/mSBC).
> 
> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> ---
> include/net/bluetooth/hci.h      | 9 +++++++++
> include/net/bluetooth/hci_core.h | 8 ++++++--
> 2 files changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index 35c073d44ec5..a4da339aab07 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -255,6 +255,15 @@ enum {
> 	 * during the hdev->setup vendor callback.
> 	 */
> 	HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER,
> +
> +	/*
> +	 * When this quirk is set, disables the use of
> +	 * HCI_OP_ENHANCED_SETUP_SYNC_CONN command to setup SCO connections.

it is SCO and eSCO connections.

> +	 *
> +	 * This quirk can be set before hci_register_dev is called or
> +	 * during the hdev->setup vendor callback.
> +	 */
> +	HCI_QUIRK_BROKEN_ENHANCED_SETUP_SCO,
> };

s/_SCO/_SYNC/

> 
> /* HCI device flags */
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index d5377740e99c..7a9795783850 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -1492,8 +1492,12 @@ void hci_conn_del_sysfs(struct hci_conn *conn);
> #define privacy_mode_capable(dev) (use_ll_privacy(dev) && \
> 				   (hdev->commands[39] & 0x04))
> 
> -/* Use enhanced synchronous connection if command is supported */
> -#define enhanced_sco_capable(dev) ((dev)->commands[29] & 0x08)
> +/* Use enhanced synchronous connection if command is supported and its quirk
> + * has not been set.
> + */
> +#define enhanced_sco_capable(dev) (((dev)->commands[29] & 0x08) && \
> +				   !test_bit(HCI_QUIRK_BROKEN_ENHANCED_SETUP_SCO, \
> +					     &(dev)->quirks))

Hmmm. So we got that naming wrong already. It should have really read enhanced_setup_sync_capable().

And frankly now I would actually rename it in the context that we limit it other than commands/feature bits. So use use_enhanced_setup_sync() in the code now.

Regards

Marcel

[PATCH 3/3] Bluetooth: btusb: Add BTUSB_BROKEN_ENHANCED_SETUP_SCO flag

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This adds BTUSB_BROKEN_ENHANCED_SETUP_SCO flag which can be used to set
HCI_QUIRK_BROKEN_ENHANCED_SETUP_SCO disabling the use of
HCI_OP_ENHANCED_SETUP_SYNC_CONN command.

Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
 drivers/bluetooth/btusb.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 34d008380fdb..d09a6a712632 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -63,6 +63,7 @@ static struct usb_driver btusb_driver;
 #define BTUSB_INTEL_BROKEN_SHUTDOWN_LED	BIT(24)
 #define BTUSB_INTEL_BROKEN_INITIAL_NCMD BIT(25)
 #define BTUSB_INTEL_NO_WBS_SUPPORT	BIT(26)
+#define BTUSB_BROKEN_ENHANCED_SETUP_SCO	BIT(27)
 
 static const struct usb_device_id btusb_table[] = {
 	/* Generic Bluetooth USB device */
@@ -3848,6 +3849,9 @@ static int btusb_probe(struct usb_interface *intf,
 		set_bit(HCI_QUIRK_RESET_ON_CLOSE, &hdev->quirks);
 	}
 
+	if (id->driver_info & BTUSB_BROKEN_ENHANCED_SETUP_SCO)
+		set_bit(HCI_QUIRK_BROKEN_ENHANCED_SETUP_SCO, &hdev->quirks);
+
 	if (id->driver_info & BTUSB_CSR) {
 		struct usb_device *udev = data->udev;
 		u16 bcdDevice = le16_to_cpu(udev->descriptor.bcdDevice);
-- 
2.35.1

Re: [PATCH 3/3] Bluetooth: btusb: Add BTUSB_BROKEN_ENHANCED_SETUP_SCO flag

[Marcel Holtmann]

Hi Luiz,

> This adds BTUSB_BROKEN_ENHANCED_SETUP_SCO flag which can be used to set
> HCI_QUIRK_BROKEN_ENHANCED_SETUP_SCO disabling the use of
> HCI_OP_ENHANCED_SETUP_SYNC_CONN command.
> 
> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> ---
> drivers/bluetooth/btusb.c | 4 ++++
> 1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index 34d008380fdb..d09a6a712632 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -63,6 +63,7 @@ static struct usb_driver btusb_driver;
> #define BTUSB_INTEL_BROKEN_SHUTDOWN_LED	BIT(24)
> #define BTUSB_INTEL_BROKEN_INITIAL_NCMD BIT(25)
> #define BTUSB_INTEL_NO_WBS_SUPPORT	BIT(26)
> +#define BTUSB_BROKEN_ENHANCED_SETUP_SCO	BIT(27)
> 
> static const struct usb_device_id btusb_table[] = {
> 	/* Generic Bluetooth USB device */
> @@ -3848,6 +3849,9 @@ static int btusb_probe(struct usb_interface *intf,
> 		set_bit(HCI_QUIRK_RESET_ON_CLOSE, &hdev->quirks);
> 	}
> 
> +	if (id->driver_info & BTUSB_BROKEN_ENHANCED_SETUP_SCO)
> +		set_bit(HCI_QUIRK_BROKEN_ENHANCED_SETUP_SCO, &hdev->quirks);
> +
> 	if (id->driver_info & BTUSB_CSR) {
> 		struct usb_device *udev = data->udev;
> 		u16 bcdDevice = le16_to_cpu(udev->descriptor.bcdDevice);

I am only adding this if I see the need to do this via USB VID/PID. Most drivers should make the decision on the vendor setup callback. Or FIX their broken firmware.

Regards

Marcel

RE: [1/3] Bluetooth: btusb: Make use of of BIT macro to declare flags

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1377 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=620130

---Test result---

Test Summary:
CheckPatch                    PASS      2.97 seconds
GitLint                       PASS      1.62 seconds
SubjectPrefix                 PASS      1.26 seconds
BuildKernel                   PASS      35.42 seconds
BuildKernel32                 PASS      26.94 seconds
Incremental Build with patchesPASS      72.59 seconds
TestRunner: Setup             PASS      479.12 seconds
TestRunner: l2cap-tester      PASS      13.62 seconds
TestRunner: bnep-tester       PASS      6.17 seconds
TestRunner: mgmt-tester       PASS      104.98 seconds
TestRunner: rfcomm-tester     FAIL      7.53 seconds
TestRunner: sco-tester        PASS      7.74 seconds
TestRunner: smp-tester        PASS      7.62 seconds
TestRunner: userchan-tester   PASS      6.40 seconds

Details
##############################
Test: TestRunner: rfcomm-tester - FAIL - 7.53 seconds
Run test-runner with rfcomm-tester
Total: 10, Passed: 9 (90.0%), Failed: 1, Not Run: 0

Failed Test Cases
Basic RFCOMM Socket Client - Write 32k Success       Failed       0.153 seconds



---
Regards,
Linux Bluetooth

Re: [PATCH 1/3] Bluetooth: btusb: Make use of of BIT macro to declare flags

[Marcel Holtmann]

Hi Luiz,

> This makes use of BIT macro when declaring BTUSB flags which makes it
> understand how many bits are in use and many are left.
> 
> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> ---
> drivers/bluetooth/btusb.c | 54 +++++++++++++++++++--------------------
> 1 file changed, 27 insertions(+), 27 deletions(-)

patch has been applied to bluetooth-next tree.

Regards

Marcel