Thread (86 messages) 86 messages, 7 authors, 2021-12-21

Re: [PATCHv2] Bluetooth: quirk disabling LE Read Transmit Power

From: Aditya Garg <hidden>
Date: 2021-11-29 07:14:59
Also in: lkml, regressions

So I have finally managed to add quirks in btbcm on the basis on DMI data. This shall be advantageous in the situation when the user shall be using a USB adapter so that the quirk gets ineffective for the adapter.
quoted hunk ↗ jump to hunk
On 26-Nov-2021, at 8:45 PM, Aditya Garg [off-list ref] wrote:


quoted
On 25-Nov-2021, at 5:56 PM, Thorsten Leemhuis [off-list ref] wrote:

Hi, this is your Linux kernel regression tracker speaking again.

On 19.11.21 17:59, Aditya Garg wrote:
quoted
quoted
On 18-Nov-2021, at 12:31 AM, Marcel Holtmann [off-list ref] wrote:
quoted
quoted
So if this just affects two macs, why can't the fix be realized as a
quirk that is only enabled on those two systems? Or are they impossible
to detect clearly via DMI data or something like that?
I think we should be able to quirk based off the acpi _CID "apple-uart-blth"
or _HID "BCM2E7C". Marcel suggested quirking based of the acpi table here
https://lore.kernel.org/linux-bluetooth/1D2217A9-EA73-4D93-8D0B-5BC2718D4788@holtmann.org/ (local)

This would catch some unaffected Macs, but they don't support the LE Read
Transmit Power command anyway (the affected macs were released after it
was added to the Bluetooth spec, while the unaffected Macs were released
before it was added to the spec, and thus don't support it).

I'm not sure how to go about applying a quirk based off this, there are
quirks in drivers/bluetooth/hci_bcm.c (no_early_set_baudrate and
drive_rts_on_open), but they don't seem to be based off acpi ids.

It might be simpler to make it ignore the Unknown Command error, like
in this patch https://lore.kernel.org/linux-bluetooth/CABBYNZLjSfcG_KqTEbL6NOSvHhA5-b1t_S=3FQP4=GwW21kuzg@mail.gmail.com/ (local)
however that only applies on bluetooth-next and needed the status it
checks for to be -56, not 0x01.
so we abstain from try-and-error sending of commands. The Bluetooth spec
has a list of supported commands that a host can query for a reason. This
is really broken behavior of the controller and needs to be pointed out as
such.
Well all I can do is provide you any logs or information I can. But we do really wish to get this regression fixed soon.
quoted
The question is just how we quirk it.
This thread once again looks stalled and smells a lot like "everyone
agrees that his should be fixed, but afaics nobody submitted a fix or
committed to work on one". Please speak up if my impression is wrong, as
this is a regression and thus needs to be fixed, ideally quickly. Part
of my job is to make that happen and thus remind developers and
maintainers about this until we have a fix.
On the basis of DMI data, I have made this patch to disable read transmit power on 16,1. I have tested this on my 16,1 successfully. Still consider this as a draft as more models have to be added. I am sending this to get the approval of the maintainers whether this quirk is acceptable or not. If yes, I shall send the final patch.

From 3dab2e1e9e0b266574f5f010efc6680417fb0c61 Mon Sep 17 00:00:00 2001
From: Aditya Garg <redacted>
Date: Fri, 26 Nov 2021 18:28:46 +0530
Subject: [PATCH] Add quirk to disable read transmit power on MacBook Pro 16
inch, 2019

---
net/bluetooth/hci_core.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 8d33aa64846b1c..d11064cb3666ef 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -32,6 +32,7 @@
#include <linux/property.h>
#include <linux/suspend.h>
#include <linux/wait.h>
+#include <linux/dmi.h>
#include <asm/unaligned.h>

#include <net/bluetooth/bluetooth.h>
@@ -461,9 +462,23 @@ static void hci_set_event_mask_page_2(struct hci_request *req)
			    sizeof(events), events);
}

+static const struct dmi_system_id fix_up_apple_bluetooth[] = {
+	{
+		/* Match for Apple MacBook Pro 16 inch, 2019 which needs
+		 * read transmit power to be disabled
+		 */
+		.matches = {
+			DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+			DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,1"),
+		},
+	},
+	{ }
+};
+
static int hci_init3_req(struct hci_request *req, unsigned long opt)
{
	struct hci_dev *hdev = req->hdev;
+	const struct dmi_system_id *dmi_id;
	u8 p;

	hci_setup_event_mask(req);
@@ -619,7 +634,8 @@ static int hci_init3_req(struct hci_request *req, unsigned long opt)
			hci_req_add(req, HCI_OP_LE_READ_ADV_TX_POWER, 0, NULL);
		}

-		if (hdev->commands[38] & 0x80) {
+		dmi_id = dmi_first_match(fix_up_apple_bluetooth);
+		if (hdev->commands[38] & 0x80 && (!dmi_id)) {
			/* Read LE Min/Max Tx Power*/
			hci_req_add(req, HCI_OP_LE_READ_TRANSMIT_POWER,
				    0, NULL);
quoted
Ciao, Thorsten

#regzbot title bluetooth: "Query LE tx power on startup" broke Bluetooth
on MacBookPro16,1
#regzbot poke
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help