I have no idea what I'm doing

PATCH In review Chris Mason 2 participants 8 messages Jul 14, 2015 Bluetooth drivers

lore

Share & tools

lore

Chris Mason

author

2015-07-14 19:16 UTC #0 lore raw

PATCH 1/1 I have no idea what I'm doing

Hi Marcel,

I upgraded my macbook from 4.0.2 to 4.1.2, and now I get messages similar to this bugzilla:

https://bugzilla.kernel.org/show_bug.cgi?id=100651

and this one:

https://bugzilla.kernel.org/show_bug.cgi?id=101171

[21552.711888] Bluetooth: hci0: BCM: Read verbose config info failed (-16)
[21728.019676] Bluetooth: hci0: BCM: Read verbose config info failed (-16)

A little poking around shows you recently added this check, and from what I can tell with grep and friends the btbcm_read_verbose_config() function is completely new? I don't see similar reads being done by older kernels.

So, I pushed some code around. It does compile, but printk proves that only the second hunk is happening on my box, so testing is somewhat light.

With this applied, I'm able to use bluetooth again.

Signed-off-by Chris Mason <[email protected]>

drivers/bluetooth/btbcm.c +9 ~~-11~~

index 4bba866..90e7099 100644
@@ -294,11 +294,10 @@ int btbcm_setup_patchram(struct hci_dev *hdev)
 
 	/* Read Verbose Config Version Info */
 	skb = btbcm_read_verbose_config(hdev);
-	if (IS_ERR(skb))
-		return PTR_ERR(skb);
-
-	BT_INFO("%s: BCM: chip id %u", hdev->name, skb->data[1]);
-	kfree_skb(skb);
+	if (!IS_ERR(skb)) {
+		BT_INFO("%s: BCM: chip id %u", hdev->name, skb->data[1]);
+		kfree_skb(skb);
+	}
 
 	switch ((rev & 0xf000) >> 12) {
 	case 0:

@@ -378,12 +377,11 @@ int btbcm_setup_apple(struct hci_dev *hdev)
 
 	/* Read Verbose Config Version Info */
 	skb = btbcm_read_verbose_config(hdev);
-	if (IS_ERR(skb))
-		return PTR_ERR(skb);
-
-	BT_INFO("%s: BCM: chip id %u build %4.4u", hdev->name, skb->data[1],
-		get_unaligned_le16(skb->data + 5));
-	kfree_skb(skb);
+	if (!IS_ERR(skb)) {
+		BT_INFO("%s: BCM: chip id %u build %4.4u", hdev->name, skb->data[1],
+			get_unaligned_le16(skb->data + 5));
+		kfree_skb(skb);
+	}
 
 	set_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks);

Marcel Holtmann

maintainer

2015-07-14 19:49 UTC #1 lore raw

Hi Chris,

Quoted 32 lines

I upgraded my macbook from 4.0.2 to 4.1.2, and now I get messages similar to this bugzilla:
https://bugzilla.kernel.org/show_bug.cgi?id=100651
and this one:
https://bugzilla.kernel.org/show_bug.cgi?id=101171
[21552.711888] Bluetooth: hci0: BCM: Read verbose config info failed (-16)
[21728.019676] Bluetooth: hci0: BCM: Read verbose config info failed (-16)
A little poking around shows you recently added this check, and from what I can tell with grep and friends the btbcm_read_verbose_config() function is completely new? I don't see similar reads being done by older kernels.
So, I pushed some code around. It does compile, but printk proves that only the second hunk is happening on my box, so testing is somewhat light.
With this applied, I'm able to use bluetooth again.
Signed-off-by Chris Mason <[email protected]>
drivers/bluetooth/btbcm.c +0 -0
index 4bba866..90e7099 100644
@@ -294,11 +294,10 @@ int btbcm_setup_patchram(struct hci_dev *hdev)
 
 	/* Read Verbose Config Version Info */

drivers/bluetooth/btbcm.c int btbcm_setup_patchram(struct hci_dev *hdev) · line 300

Quoted 10 lines

drivers/bluetooth/btbcm.c +4 -5

 	skb = btbcm_read_verbose_config(hdev);
-	if (IS_ERR(skb))
-		return PTR_ERR(skb);
-
-	BT_INFO("%s: BCM: chip id %u", hdev->name, skb->data[1]);
-	kfree_skb(skb);
+	if (!IS_ERR(skb)) {
+		BT_INFO("%s: BCM: chip id %u", hdev->name, skb->data[1]);
+		kfree_skb(skb);
+	}

leave this one out of it. Since all Broadcom chips do actually support this as far as I can tell. We really want to fail this one since something went wrong in that case.

Quoted 10 lines

drivers/bluetooth/btbcm.c +0 -3

 	switch ((rev & 0xf000) >> 12) {
 	case 0:

@@ -378,12 +377,11 @@ int btbcm_setup_apple(struct hci_dev *hdev)
 
 	/* Read Verbose Config Version Info */
 	skb = btbcm_read_verbose_config(hdev);
-	if (IS_ERR(skb))
-		return PTR_ERR(skb);
-

drivers/bluetooth/btbcm.c int btbcm_setup_apple(struct hci_dev *hdev) · line 386

Quoted 10 lines

drivers/bluetooth/btbcm.c +5 -3

-	BT_INFO("%s: BCM: chip id %u build %4.4u", hdev->name, skb->data[1],
-		get_unaligned_le16(skb->data + 5));
-	kfree_skb(skb);
+	if (!IS_ERR(skb)) {
+		BT_INFO("%s: BCM: chip id %u build %4.4u", hdev->name, skb->data[1],
+			get_unaligned_le16(skb->data + 5));
+		kfree_skb(skb);
+	}
 
 	set_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks);

This hunk is fine. I would accept a patch for this. I tested 8 Apple Bluetooth modules and they were all fine with this command. It seems there is this one chip in this one MacBook that is not.

Actually can you run something like "hciconfig hci0 version" and see if it really is a Broadcom chip in there or if Apple switched vendors and we are accidentally assuming it is Broadcom while in reality it is not. If it is not then the quirk might actually not apply either anymore.

Regards

Marcel

Chris Mason

author

2015-07-14 19:59 UTC #2 lore raw

Quoted 33 lines · Marcel Holtmann

Hi Chris,
Quoted 31 lines
I upgraded my macbook from 4.0.2 to 4.1.2, and now I get messages similar to this bugzilla:
https://bugzilla.kernel.org/show_bug.cgi?id=100651
and this one:
https://bugzilla.kernel.org/show_bug.cgi?id=101171
[21552.711888] Bluetooth: hci0: BCM: Read verbose config info failed (-16)
[21728.019676] Bluetooth: hci0: BCM: Read verbose config info failed (-16)
A little poking around shows you recently added this check, and from what I can tell with grep and friends the btbcm_read_verbose_config() function is completely new? I don't see similar reads being done by older kernels.
So, I pushed some code around. It does compile, but printk proves that only the second hunk is happening on my box, so testing is somewhat light.
With this applied, I'm able to use bluetooth again.
Signed-off-by Chris Mason <[email protected]>
drivers/bluetooth/btbcm.c +0 -0
index 4bba866..90e7099 100644
@@ -294,11 +294,10 @@ int btbcm_setup_patchram(struct hci_dev *hdev)
 

Quoted 15 lines

drivers/bluetooth/btbcm.c int btbcm_setup_patchram(struct hci_dev *hdev) · line 300
Quoted 11 lines
drivers/bluetooth/btbcm.c +4 -5
 	/* Read Verbose Config Version Info */
 	skb = btbcm_read_verbose_config(hdev);
-	if (IS_ERR(skb))
-		return PTR_ERR(skb);
-
-	BT_INFO("%s: BCM: chip id %u", hdev->name, skb->data[1]);
-	kfree_skb(skb);
+	if (!IS_ERR(skb)) {
+		BT_INFO("%s: BCM: chip id %u", hdev->name, skb->data[1]);
+		kfree_skb(skb);
+	}
leave this one out of it. Since all Broadcom chips do actually support this as far as I can tell. We really want to fail this one since something went wrong in that case.

ack. Both of those bugzillas look like macs, so they shouldn't need this hunk either.

Quoted 27 lines

Quoted 11 lines
drivers/bluetooth/btbcm.c +0 -3
 	switch ((rev & 0xf000) >> 12) {
 	case 0:
@@ -378,12 +377,11 @@ int btbcm_setup_apple(struct hci_dev *hdev)
 
 	/* Read Verbose Config Version Info */
 	skb = btbcm_read_verbose_config(hdev);
-	if (IS_ERR(skb))
-		return PTR_ERR(skb);
-
drivers/bluetooth/btbcm.c int btbcm_setup_apple(struct hci_dev *hdev) · line 386
Quoted 10 lines
drivers/bluetooth/btbcm.c +5 -3
-	BT_INFO("%s: BCM: chip id %u build %4.4u", hdev->name, skb->data[1],
-		get_unaligned_le16(skb->data + 5));
-	kfree_skb(skb);
+	if (!IS_ERR(skb)) {
+		BT_INFO("%s: BCM: chip id %u build %4.4u", hdev->name, skb->data[1],
+			get_unaligned_le16(skb->data + 5));
+		kfree_skb(skb);
+	}
 
 	set_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks);
This hunk is fine. I would accept a patch for this. I tested 8 Apple Bluetooth modules and they were all fine with this command. It seems there is this one chip in this one MacBook that is not.

Quoted 5 lines

Actually can you run something like "hciconfig hci0 version" and see if it really is a Broadcom chip in there or if Apple switched vendors and we are accidentally assuming it is Broadcom while in reality it is not. If it is not then the quirk might actually not apply either anymore.

mason@ret ~> hciconfig hci0 version
hci0: Type: BR/EDR Bus: USB

	BD Address: 60:03:08:8D:0D:A9  ACL MTU: 1021:8  SCO MTU: 64:1
	HCI Version: 4.0 (0x6)  Revision: 0x21ae
	LMP Version: 4.0 (0x6)  Subversion: 0x414e
	Manufacturer: Broadcom Corporation (15)

One of the threads I saw when I was googling claimed it worked if you rebooted from 4.0 into 4.1 but not 4.1 into 4.1.

-chris

Marcel Holtmann

maintainer

2015-07-14 20:49 UTC #3 lore raw

Hi Chris,

Quoted 83 lines

Quoted 31 lines
Quoted 31 lines
I upgraded my macbook from 4.0.2 to 4.1.2, and now I get messages similar to this bugzilla:
https://bugzilla.kernel.org/show_bug.cgi?id=100651
and this one:
https://bugzilla.kernel.org/show_bug.cgi?id=101171
[21552.711888] Bluetooth: hci0: BCM: Read verbose config info failed (-16)
[21728.019676] Bluetooth: hci0: BCM: Read verbose config info failed (-16)
A little poking around shows you recently added this check, and from what I can tell with grep and friends the btbcm_read_verbose_config() function is completely new? I don't see similar reads being done by older kernels.
So, I pushed some code around. It does compile, but printk proves that only the second hunk is happening on my box, so testing is somewhat light.
With this applied, I'm able to use bluetooth again.
Signed-off-by Chris Mason <[email protected]>
drivers/bluetooth/btbcm.c +0 -0
index 4bba866..90e7099 100644
@@ -294,11 +294,10 @@ int btbcm_setup_patchram(struct hci_dev *hdev)
 
Quoted 15 lines
drivers/bluetooth/btbcm.c int btbcm_setup_patchram(struct hci_dev *hdev) · line 300
Quoted 11 lines
drivers/bluetooth/btbcm.c +4 -5
 	/* Read Verbose Config Version Info */
 	skb = btbcm_read_verbose_config(hdev);
-	if (IS_ERR(skb))
-		return PTR_ERR(skb);
-
-	BT_INFO("%s: BCM: chip id %u", hdev->name, skb->data[1]);
-	kfree_skb(skb);
+	if (!IS_ERR(skb)) {
+		BT_INFO("%s: BCM: chip id %u", hdev->name, skb->data[1]);
+		kfree_skb(skb);
+	}
leave this one out of it. Since all Broadcom chips do actually support this as far as I can tell. We really want to fail this one since something went wrong in that case.
ack. Both of those bugzillas look like macs, so they shouldn't need this hunk either.
Quoted 27 lines
Quoted 11 lines
drivers/bluetooth/btbcm.c +0 -3
 	switch ((rev & 0xf000) >> 12) {
 	case 0:
@@ -378,12 +377,11 @@ int btbcm_setup_apple(struct hci_dev *hdev)
 
 	/* Read Verbose Config Version Info */
 	skb = btbcm_read_verbose_config(hdev);
-	if (IS_ERR(skb))
-		return PTR_ERR(skb);
-
drivers/bluetooth/btbcm.c int btbcm_setup_apple(struct hci_dev *hdev) · line 386
Quoted 10 lines
drivers/bluetooth/btbcm.c +5 -3
-	BT_INFO("%s: BCM: chip id %u build %4.4u", hdev->name, skb->data[1],
-		get_unaligned_le16(skb->data + 5));
-	kfree_skb(skb);
+	if (!IS_ERR(skb)) {
+		BT_INFO("%s: BCM: chip id %u build %4.4u", hdev->name, skb->data[1],
+			get_unaligned_le16(skb->data + 5));
+		kfree_skb(skb);
+	}
 
 	set_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks);
This hunk is fine. I would accept a patch for this. I tested 8 Apple Bluetooth modules and they were all fine with this command. It seems there is this one chip in this one MacBook that is not.
Quoted 5 lines
Actually can you run something like "hciconfig hci0 version" and see if it really is a Broadcom chip in there or if Apple switched vendors and we are accidentally assuming it is Broadcom while in reality it is not. If it is not then the quirk might actually not apply either anymore.

Quoted 6 lines

mason@ret ~> hciconfig hci0 version
hci0: Type: BR/EDR Bus: USB

	BD Address: 60:03:08:8D:0D:A9  ACL MTU: 1021:8  SCO MTU: 64:1
	HCI Version: 4.0 (0x6)  Revision: 0x21ae
	LMP Version: 4.0 (0x6)  Subversion: 0x414e
	Manufacturer: Broadcom Corporation (15)

so it is a Broadcom. What MacBook is this anyway?

Quoted 2 lines

One of the threads I saw when I was googling claimed it worked if you rebooted from 4.0 into 4.1 but not 4.1 into 4.1.

I think that is just someone getting confused. It should always fail if you boot 4.1. No matter what you had running before.

Regards

Marcel

Chris Mason

author

2015-07-14 20:53 UTC #4 lore raw

Quoted 8 lines · Marcel Holtmann

Hi Chris,
Quoted 7 lines
Quoted 6 lines
Actually can you run something like "hciconfig hci0 version" and see if it really is a Broadcom chip in there or if Apple switched vendors and we are accidentally assuming it is Broadcom while in reality it is not. If it is not then the quirk might actually not apply either anymore.

Quoted 8 lines

Quoted 6 lines
mason@ret ~> hciconfig hci0 version
hci0: Type: BR/EDR Bus: USB
	BD Address: 60:03:08:8D:0D:A9  ACL MTU: 1021:8  SCO MTU: 64:1
	HCI Version: 4.0 (0x6)  Revision: 0x21ae
	LMP Version: 4.0 (0x6)  Subversion: 0x414e
	Manufacturer: Broadcom Corporation (15)
so it is a Broadcom. What MacBook is this anyway?

macbookpro 13" from 2013 (newer model in '13)

Quoted 7 lines

Quoted 3 lines
One of the threads I saw when I was googling claimed it worked if you rebooted from 4.0 into 4.1 but not 4.1 into 4.1.
I think that is just someone getting confused. It should always fail if you boot 4.1. No matter what you had running before.

That's my experience.

-chris

Marcel Holtmann

maintainer

2015-07-14 20:58 UTC #5 lore raw

Hi Chris,

Quoted 13 lines

Quoted 13 lines
Quoted 12 lines
Quoted 5 lines
Actually can you run something like "hciconfig hci0 version" and see if it really is a Broadcom chip in there or if Apple switched vendors and we are accidentally assuming it is Broadcom while in reality it is not. If it is not then the quirk might actually not apply either anymore.
mason@ret ~> hciconfig hci0 version
hci0: Type: BR/EDR Bus: USB
	BD Address: 60:03:08:8D:0D:A9  ACL MTU: 1021:8  SCO MTU: 64:1
	HCI Version: 4.0 (0x6)  Revision: 0x21ae
	LMP Version: 4.0 (0x6)  Subversion: 0x414e
	Manufacturer: Broadcom Corporation (15)

Quoted 3 lines

Quoted 1 line
so it is a Broadcom. What MacBook is this anyway?
macbookpro 13" from 2013 (newer model in '13)

besides having that Bluetooth module for testing, I also have that actual Macbook. And I did test this. It worked fine for me. Maybe some OS X update messed with the firmware. Worth while checking.

Regards

Marcel

Chris Mason

author

2015-07-14 21:01 UTC #6 lore raw

Quoted 15 lines · Marcel Holtmann

Hi Chris,
Quoted 13 lines
Quoted 13 lines
Quoted 12 lines
Quoted 5 lines
Actually can you run something like "hciconfig hci0 version" and see if it really is a Broadcom chip in there or if Apple switched vendors and we are accidentally assuming it is Broadcom while in reality it is not. If it is not then the quirk might actually not apply either anymore.
mason@ret ~> hciconfig hci0 version
hci0: Type: BR/EDR Bus: USB
	BD Address: 60:03:08:8D:0D:A9  ACL MTU: 1021:8  SCO MTU: 64:1
	HCI Version: 4.0 (0x6)  Revision: 0x21ae
	LMP Version: 4.0 (0x6)  Subversion: 0x414e
	Manufacturer: Broadcom Corporation (15)

Quoted 5 lines

Quoted 3 lines
Quoted 1 line
so it is a Broadcom. What MacBook is this anyway?
macbookpro 13" from 2013 (newer model in '13)
besides having that Bluetooth module for testing, I also have that actual Macbook. And I did test this. It worked fine for me. Maybe some OS X update messed with the firmware. Worth while checking.

This only runs linux, I don't dual boot.

-chris

Marcel Holtmann

maintainer

2015-07-14 21:11 UTC #7 lore raw

Hi Chris,

Quoted 13 lines

Quoted 13 lines
Quoted 13 lines
Quoted 13 lines
Quoted 12 lines
Quoted 5 lines
Actually can you run something like "hciconfig hci0 version" and see if it really is a Broadcom chip in there or if Apple switched vendors and we are accidentally assuming it is Broadcom while in reality it is not. If it is not then the quirk might actually not apply either anymore.
mason@ret ~> hciconfig hci0 version
hci0: Type: BR/EDR Bus: USB
	BD Address: 60:03:08:8D:0D:A9  ACL MTU: 1021:8  SCO MTU: 64:1
	HCI Version: 4.0 (0x6)  Revision: 0x21ae
	LMP Version: 4.0 (0x6)  Subversion: 0x414e
	Manufacturer: Broadcom Corporation (15)

Quoted 7 lines

Quoted 5 lines
Quoted 3 lines
Quoted 1 line
so it is a Broadcom. What MacBook is this anyway?
macbookpro 13" from 2013 (newer model in '13)
besides having that Bluetooth module for testing, I also have that actual Macbook. And I did test this. It worked fine for me. Maybe some OS X update messed with the firmware. Worth while checking.
This only runs linux, I don't dual boot.

if you never ran any OS X or OS X update, then this might have been a bogus firmware when the hardware shipped and by now they have all been updated. That would make some sense at least.

Regards

Marcel