[PATCH] Fix SCO on Broadcom Bluetooth adapters

[PATCH] Fix SCO on Broadcom Bluetooth adapters

[Olivier Galibert]

Broadcom USB Bluetooth adapters report a maximum of zero SCO packets
in-flight, killing SCO.  Use a reasonable count instead in that case.

Signed-off-by: Olivier Galibert <galibert@pobox.com>

---

I don't think that could be reasonably done as a quirk.  Simple
examination of the .inf coming with the windows driver shows that 100+
different models may be having this problem.  Also, it can't break
already working adapters, so why bother.

 net/bluetooth/hci_event.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -324,6 +324,13 @@ static void hci_cc_info_param(struct hci
 		hdev->acl_pkts = hdev->acl_cnt = __le16_to_cpu(bs->acl_max_pkt);
 		hdev->sco_pkts = hdev->sco_cnt = __le16_to_cpu(bs->sco_max_pkt);
 
+		/* Some buggy USB bluetooth adapters, Broadcom in
+		   particular, answer zero as the max number of sco
+		   packets in flight.  Use a reasonable value
+		   instead */
+		if (hdev->sco_pkts == 0)
+			hdev->sco_pkts = hdev->sco_cnt = 8
+
 		BT_DBG("%s mtu: acl %d, sco %d max_pkt: acl %d, sco %d", hdev->name,
 			hdev->acl_mtu, hdev->sco_mtu, hdev->acl_pkts, hdev->sco_pkts);
 		break;

Re: [PATCH] Fix SCO on Broadcom Bluetooth adapters

[Pavel Machek]

On Út 14-03-06 12:12:48, Olivier Galibert wrote:
> Broadcom USB Bluetooth adapters report a maximum of zero SCO packets
> in-flight, killing SCO.  Use a reasonable count instead in that
> case.

Printk("broken Broadcom USB detected, working around"), I'd say. Then
you can also remove the comment :-). Maybe 4 is reasonable value for
some really broken thing, or something...
								
> Signed-off-by: Olivier Galibert <galibert@pobox.com>

								Pavel

> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -324,6 +324,13 @@ static void hci_cc_info_param(struct hci
>  		hdev->acl_pkts = hdev->acl_cnt = __le16_to_cpu(bs->acl_max_pkt);
>  		hdev->sco_pkts = hdev->sco_cnt = __le16_to_cpu(bs->sco_max_pkt);
>  
> +		/* Some buggy USB bluetooth adapters, Broadcom in
> +		   particular, answer zero as the max number of sco
> +		   packets in flight.  Use a reasonable value
> +		   instead */
> +		if (hdev->sco_pkts == 0)
> +			hdev->sco_pkts = hdev->sco_cnt = 8
> +
>  		BT_DBG("%s mtu: acl %d, sco %d max_pkt: acl %d, sco %d", hdev->name,
>  			hdev->acl_mtu, hdev->sco_mtu, hdev->acl_pkts, hdev->sco_pkts);
>  		break;


-- 
189:    private string AtomSTSZ = "stsz";

Re: [PATCH] Fix SCO on Broadcom Bluetooth adapters

[Marcel Holtmann]

Hi Olivier,

> Broadcom USB Bluetooth adapters report a maximum of zero SCO packets
> in-flight, killing SCO.  Use a reasonable count instead in that case.
> 
> Signed-off-by: Olivier Galibert <galibert@pobox.com>
> 
> ---
> 
> I don't think that could be reasonably done as a quirk.  Simple
> examination of the .inf coming with the windows driver shows that 100+
> different models may be having this problem.  Also, it can't break
> already working adapters, so why bother.

your patch might break devices where this value is chosen on purpose, so
it is not acceptable and must be done with a quirk. Another reason is
that I don't allow any stupid vendor specific workarounds inside the
Bluetooth core unless they are implemented as quirks. The core has no
vendor information at all.

Regards

Marcel

Re: [PATCH] Fix SCO on Broadcom Bluetooth adapters

[Olivier Galibert]

On Tue, Mar 14, 2006 at 02:49:03PM +0100, Marcel Holtmann wrote:
> your patch might break devices where this value is chosen on purpose

Please do tell me how one is supposed to do sco communications when
this value is chosen on purpose and I'll be happy to implement that.
I have found no clue about that in the bluetooth specs.

Meanwhile, I'll implement it with a quirk, and I can guarantee that
you're not going to like the result.  I have a list of 104 (iirc)
device ids that may need it, and given that hardware is supposed to
work they will have to be added until proven otherwise.

And it won't fix the uart versions either.

  OG.

Re: [PATCH] Fix SCO on Broadcom Bluetooth adapters

[Alan Cox]

On Maw, 2006-03-14 at 15:08 +0100, Olivier Galibert wrote:
> Meanwhile, I'll implement it with a quirk, and I can guarantee that
> you're not going to like the result.  I have a list of 104 (iirc)
> device ids that may need it, and given that hardware is supposed to
> work they will have to be added until proven otherwise.

Just add a wildcard quirk so your fixup is called for every single
device and then does the check. Keeps the fix out of the core and
clearly documented while keeping the list short.

Re: [PATCH] Fix SCO on Broadcom Bluetooth adapters

[Olivier Galibert]

On Tue, Mar 14, 2006 at 10:27:56PM +0000, Alan Cox wrote:
> Just add a wildcard quirk so your fixup is called for every single
> device and then does the check. Keeps the fix out of the core and
> clearly documented while keeping the list short.

Errr, ok, I'm all for doing that but I'm not sure what "that" means.
You want the test/fix to be put in a function somewhere to be defined
and hci_event to call it where I put the if initially?

  OG.

PS: It's only 51 after all.  For now, that is, since we know there
    will be new ones with the same problem.

Re: [PATCH] Fix SCO on Broadcom Bluetooth adapters

[Randy.Dunlap]

On Tue, 14 Mar 2006 12:12:48 +0100 Olivier Galibert wrote:

> Broadcom USB Bluetooth adapters report a maximum of zero SCO packets
> in-flight, killing SCO.  Use a reasonable count instead in that case.
> 
> Signed-off-by: Olivier Galibert <galibert@pobox.com>
> 
> ---
> 
> I don't think that could be reasonably done as a quirk.  Simple
> examination of the .inf coming with the windows driver shows that 100+
> different models may be having this problem.  Also, it can't break
> already working adapters, so why bother.
> 
>  net/bluetooth/hci_event.c |    7 +++++++
>  1 files changed, 7 insertions(+), 0 deletions(-)
> 
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -324,6 +324,13 @@ static void hci_cc_info_param(struct hci
>  		hdev->acl_pkts = hdev->acl_cnt = __le16_to_cpu(bs->acl_max_pkt);
>  		hdev->sco_pkts = hdev->sco_cnt = __le16_to_cpu(bs->sco_max_pkt);
>  
> +		/* Some buggy USB bluetooth adapters, Broadcom in
> +		   particular, answer zero as the max number of sco
> +		   packets in flight.  Use a reasonable value
> +		   instead */
> +		if (hdev->sco_pkts == 0)
> +			hdev->sco_pkts = hdev->sco_cnt = 8

missing trailing ';'

---
~Randy