| Message ID | b86543908684cc6cd9afaf4de10fac7af1a49665.camel@iki.fi (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] Bluetooth: btusb: check conditions before enabling USB ALT 3 for WBS | expand |
Hi Pauli, > Some USB BT adapters don't satisfy the MTU requirement mentioned in > commit e848dbd364ac ("Bluetooth: btusb: Add support USB ALT 3 for WBS") > and have ALT 3 setting that produces no/garbled audio. Some adapters > with larger MTU were also reported to have problems with ALT 3. > > Add a flag and check it and MTU before selecting ALT 3, falling back to > ALT 1. Enable the flag for Realtek, restoring the previous behavior for > non-Realtek devices. > > Tested with USB adapters (mtu<72, no/garbled sound with ALT3, ALT1 > works) BCM20702A1 0b05:17cb, CSR8510A10 0a12:0001, and (mtu>=72, ALT3 > works) RTL8761BU 0bda:8771, Intel AX200 8087:0029 (after disabling > ALT6). Also got reports for (mtu>=72, ALT 3 reported to produce bad > audio) Intel 8087:0a2b. > > Signed-off-by: Pauli Virtanen <pav@iki.fi> > Fixes: e848dbd364ac ("Bluetooth: btusb: Add support USB ALT 3 for WBS") before I will apply this, I need Tested-by or Ack-by people that confirm that this fixes their issues now. > --- > > Changes in v2: > - Explain magic number 72 in a comment; didn't add the table for them, > because it's not used elsewhere and we need just one number from it. > - Add flag for ALT3 support, restoring the behavior > for non-Realtek devices the same as before e848dbd364ac, due to > the problems reported on an Intel adapter. Don't have the device > myself. > > drivers/bluetooth/btusb.c | 22 ++++++++++++++-------- > 1 file changed, 14 insertions(+), 8 deletions(-) > > diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c > index 6d23308119d1..5cec719f6cba 100644 > --- a/drivers/bluetooth/btusb.c > +++ b/drivers/bluetooth/btusb.c > @@ -516,6 +516,7 @@ static const struct dmi_system_id btusb_needs_reset_resume_table[] = { > #define BTUSB_HW_RESET_ACTIVE 12 > #define BTUSB_TX_WAIT_VND_EVT 13 > #define BTUSB_WAKEUP_DISABLE 14 > +#define BTUSB_ALT3_OK_FOR_WBS 15 Rename this to BTUSB_USE_ALT3_FOR_WBS. > > struct btusb_data { > struct hci_dev *hdev; > @@ -1748,16 +1749,20 @@ static void btusb_work(struct work_struct *work) > /* Bluetooth USB spec recommends alt 6 (63 bytes), but > * many adapters do not support it. Alt 1 appears to > * work for all adapters that do not have alt 6, and > - * which work with WBS at all. > + * which work with WBS at all. Some devices prefer > + * alt 3 (HCI payload >= 60 Bytes let air packet > + * data satisfy 60 bytes), requiring > + * MTU >= 3 (packets) * 25 (size) - 3 (headers) = 72 > + * see also Core spec 5, vol 4, B 2.1.1 & Table 2.1. > */ > - new_alts = btusb_find_altsetting(data, 6) ? 6 : 1; > - /* Because mSBC frames do not need to be aligned to the > - * SCO packet boundary. If support the Alt 3, use the > - * Alt 3 for HCI payload >= 60 Bytes let air packet > - * data satisfy 60 bytes. > - */ > - if (new_alts == 1 && btusb_find_altsetting(data, 3)) > + if (btusb_find_altsetting(data, 6)) > + new_alts = 6; > + else if (test_bit(BTUSB_ALT3_OK_FOR_WBS, &data->flags) && > + hdev->sco_mtu >= 72 && > + btusb_find_altsetting(data, 3)) This is whitespace damaged. > new_alts = 3; > + else > + new_alts = 1; > } > > if (btusb_switch_alt_setting(hdev, new_alts) < 0) > @@ -4733,6 +4738,7 @@ static int btusb_probe(struct usb_interface *intf, > * (DEVICE_REMOTE_WAKEUP) > */ > set_bit(BTUSB_WAKEUP_DISABLE, &data->flags); > + set_bit(BTUSB_ALT3_OK_FOR_WBS, &data->flags); > } Regards Marcel
> > Some USB BT adapters don't satisfy the MTU requirement mentioned in > > commit e848dbd364ac ("Bluetooth: btusb: Add support USB ALT 3 for WBS") > > and have ALT 3 setting that produces no/garbled audio. Some adapters > > with larger MTU were also reported to have problems with ALT 3. > > > > Add a flag and check it and MTU before selecting ALT 3, falling back to > > ALT 1. Enable the flag for Realtek, restoring the previous behavior for > > non-Realtek devices. > > > > Tested with USB adapters (mtu<72, no/garbled sound with ALT3, ALT1 > > works) BCM20702A1 0b05:17cb, CSR8510A10 0a12:0001, and (mtu>=72, ALT3 > > works) RTL8761BU 0bda:8771, Intel AX200 8087:0029 (after disabling > > ALT6). Also got reports for (mtu>=72, ALT 3 reported to produce bad > > audio) Intel 8087:0a2b. > > > > Signed-off-by: Pauli Virtanen <pav@iki.fi> > > Fixes: e848dbd364ac ("Bluetooth: btusb: Add support USB ALT 3 for WBS") > > before I will apply this, I need Tested-by or Ack-by people that confirm that this fixes their issues now. For 8087:0a2b (with broken mSBC audio since e848dbd364ac): Tested-by: Michał Kępień <kernel@kempniu.pl> It would be nice to get this into the stable tree as e848dbd364ac got there in v5.13.3 (as 15407b14e27b) and v5.10.51 (as 05298f1733c6). Thanks!
Hello Marcel, On Fri, Jul 23, 2021 at 02:19:09PM +0200, Marcel Holtmann wrote: > Hi Pauli, > > > Some USB BT adapters don't satisfy the MTU requirement mentioned in > > commit e848dbd364ac ("Bluetooth: btusb: Add support USB ALT 3 for WBS") > > and have ALT 3 setting that produces no/garbled audio. Some adapters > > with larger MTU were also reported to have problems with ALT 3. > > > > Add a flag and check it and MTU before selecting ALT 3, falling back to > > ALT 1. Enable the flag for Realtek, restoring the previous behavior for > > non-Realtek devices. > > > > Tested with USB adapters (mtu<72, no/garbled sound with ALT3, ALT1 > > works) BCM20702A1 0b05:17cb, CSR8510A10 0a12:0001, and (mtu>=72, ALT3 > > works) RTL8761BU 0bda:8771, Intel AX200 8087:0029 (after disabling > > ALT6). Also got reports for (mtu>=72, ALT 3 reported to produce bad > > audio) Intel 8087:0a2b. > > > > Signed-off-by: Pauli Virtanen <pav@iki.fi> > > Fixes: e848dbd364ac ("Bluetooth: btusb: Add support USB ALT 3 for WBS") > > before I will apply this, I need Tested-by or Ack-by people that confirm that this fixes their issues now. > Is v3 ok/enough? It has one Tested-by. It would probably be good to send v4 anyway with CC stable@kernel.org .. Thanks, -- Pasi > > > --- > > > > Changes in v2: > > - Explain magic number 72 in a comment; didn't add the table for them, > > because it's not used elsewhere and we need just one number from it. > > - Add flag for ALT3 support, restoring the behavior > > for non-Realtek devices the same as before e848dbd364ac, due to > > the problems reported on an Intel adapter. Don't have the device > > myself. > > > > drivers/bluetooth/btusb.c | 22 ++++++++++++++-------- > > 1 file changed, 14 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c > > index 6d23308119d1..5cec719f6cba 100644 > > --- a/drivers/bluetooth/btusb.c > > +++ b/drivers/bluetooth/btusb.c > > @@ -516,6 +516,7 @@ static const struct dmi_system_id btusb_needs_reset_resume_table[] = { > > #define BTUSB_HW_RESET_ACTIVE 12 > > #define BTUSB_TX_WAIT_VND_EVT 13 > > #define BTUSB_WAKEUP_DISABLE 14 > > +#define BTUSB_ALT3_OK_FOR_WBS 15 > > Rename this to BTUSB_USE_ALT3_FOR_WBS. > > > > > struct btusb_data { > > struct hci_dev *hdev; > > @@ -1748,16 +1749,20 @@ static void btusb_work(struct work_struct *work) > > /* Bluetooth USB spec recommends alt 6 (63 bytes), but > > * many adapters do not support it. Alt 1 appears to > > * work for all adapters that do not have alt 6, and > > - * which work with WBS at all. > > + * which work with WBS at all. Some devices prefer > > + * alt 3 (HCI payload >= 60 Bytes let air packet > > + * data satisfy 60 bytes), requiring > > + * MTU >= 3 (packets) * 25 (size) - 3 (headers) = 72 > > + * see also Core spec 5, vol 4, B 2.1.1 & Table 2.1. > > */ > > - new_alts = btusb_find_altsetting(data, 6) ? 6 : 1; > > - /* Because mSBC frames do not need to be aligned to the > > - * SCO packet boundary. If support the Alt 3, use the > > - * Alt 3 for HCI payload >= 60 Bytes let air packet > > - * data satisfy 60 bytes. > > - */ > > - if (new_alts == 1 && btusb_find_altsetting(data, 3)) > > + if (btusb_find_altsetting(data, 6)) > > + new_alts = 6; > > + else if (test_bit(BTUSB_ALT3_OK_FOR_WBS, &data->flags) && > > + hdev->sco_mtu >= 72 && > > + btusb_find_altsetting(data, 3)) > > This is whitespace damaged. > > > new_alts = 3; > > + else > > + new_alts = 1; > > } > > > > if (btusb_switch_alt_setting(hdev, new_alts) < 0) > > @@ -4733,6 +4738,7 @@ static int btusb_probe(struct usb_interface *intf, > > * (DEVICE_REMOTE_WAKEUP) > > */ > > set_bit(BTUSB_WAKEUP_DISABLE, &data->flags); > > + set_bit(BTUSB_ALT3_OK_FOR_WBS, &data->flags); > > } > > Regards > > Marcel >
Hi Pasi, Pauli, On Tue, Aug 10, 2021 at 9:50 AM Pasi Kärkkäinen <pasik@iki.fi> wrote: > > Hello Marcel, > > On Fri, Jul 23, 2021 at 02:19:09PM +0200, Marcel Holtmann wrote: > > Hi Pauli, > > > > > Some USB BT adapters don't satisfy the MTU requirement mentioned in > > > commit e848dbd364ac ("Bluetooth: btusb: Add support USB ALT 3 for WBS") > > > and have ALT 3 setting that produces no/garbled audio. Some adapters > > > with larger MTU were also reported to have problems with ALT 3. > > > > > > Add a flag and check it and MTU before selecting ALT 3, falling back to > > > ALT 1. Enable the flag for Realtek, restoring the previous behavior for > > > non-Realtek devices. > > > > > > Tested with USB adapters (mtu<72, no/garbled sound with ALT3, ALT1 > > > works) BCM20702A1 0b05:17cb, CSR8510A10 0a12:0001, and (mtu>=72, ALT3 > > > works) RTL8761BU 0bda:8771, Intel AX200 8087:0029 (after disabling > > > ALT6). Also got reports for (mtu>=72, ALT 3 reported to produce bad > > > audio) Intel 8087:0a2b. > > > > > > Signed-off-by: Pauli Virtanen <pav@iki.fi> > > > Fixes: e848dbd364ac ("Bluetooth: btusb: Add support USB ALT 3 for WBS") > > > > before I will apply this, I need Tested-by or Ack-by people that confirm that this fixes their issues now. > > > > Is v3 ok/enough? It has one Tested-by. > It would probably be good to send v4 anyway with CC stable@kernel.org .. > > > Thanks, > > -- Pasi > > > > > > --- > > > > > > Changes in v2: > > > - Explain magic number 72 in a comment; didn't add the table for them, > > > because it's not used elsewhere and we need just one number from it. > > > - Add flag for ALT3 support, restoring the behavior > > > for non-Realtek devices the same as before e848dbd364ac, due to > > > the problems reported on an Intel adapter. Don't have the device > > > myself. > > > > > > drivers/bluetooth/btusb.c | 22 ++++++++++++++-------- > > > 1 file changed, 14 insertions(+), 8 deletions(-) > > > > > > diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c > > > index 6d23308119d1..5cec719f6cba 100644 > > > --- a/drivers/bluetooth/btusb.c > > > +++ b/drivers/bluetooth/btusb.c > > > @@ -516,6 +516,7 @@ static const struct dmi_system_id btusb_needs_reset_resume_table[] = { > > > #define BTUSB_HW_RESET_ACTIVE 12 > > > #define BTUSB_TX_WAIT_VND_EVT 13 > > > #define BTUSB_WAKEUP_DISABLE 14 > > > +#define BTUSB_ALT3_OK_FOR_WBS 15 > > > > Rename this to BTUSB_USE_ALT3_FOR_WBS. > > > > > > > > struct btusb_data { > > > struct hci_dev *hdev; > > > @@ -1748,16 +1749,20 @@ static void btusb_work(struct work_struct *work) > > > /* Bluetooth USB spec recommends alt 6 (63 bytes), but > > > * many adapters do not support it. Alt 1 appears to > > > * work for all adapters that do not have alt 6, and > > > - * which work with WBS at all. > > > + * which work with WBS at all. Some devices prefer > > > + * alt 3 (HCI payload >= 60 Bytes let air packet > > > + * data satisfy 60 bytes), requiring > > > + * MTU >= 3 (packets) * 25 (size) - 3 (headers) = 72 > > > + * see also Core spec 5, vol 4, B 2.1.1 & Table 2.1. > > > */ > > > - new_alts = btusb_find_altsetting(data, 6) ? 6 : 1; > > > - /* Because mSBC frames do not need to be aligned to the > > > - * SCO packet boundary. If support the Alt 3, use the > > > - * Alt 3 for HCI payload >= 60 Bytes let air packet > > > - * data satisfy 60 bytes. > > > - */ > > > - if (new_alts == 1 && btusb_find_altsetting(data, 3)) > > > + if (btusb_find_altsetting(data, 6)) > > > + new_alts = 6; > > > + else if (test_bit(BTUSB_ALT3_OK_FOR_WBS, &data->flags) && > > > + hdev->sco_mtu >= 72 && > > > + btusb_find_altsetting(data, 3)) > > > > This is whitespace damaged. Ive fixed this and made it fit in 80 columns by reordering the checks. > > > > > new_alts = 3; > > > + else > > > + new_alts = 1; > > > } > > > > > > if (btusb_switch_alt_setting(hdev, new_alts) < 0) > > > @@ -4733,6 +4738,7 @@ static int btusb_probe(struct usb_interface *intf, > > > * (DEVICE_REMOTE_WAKEUP) > > > */ > > > set_bit(BTUSB_WAKEUP_DISABLE, &data->flags); > > > + set_bit(BTUSB_ALT3_OK_FOR_WBS, &data->flags); > > > } > > > > Regards > > > > Marcel Applied, thanks.
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c index 6d23308119d1..5cec719f6cba 100644 --- a/drivers/bluetooth/btusb.c +++ b/drivers/bluetooth/btusb.c @@ -516,6 +516,7 @@ static const struct dmi_system_id btusb_needs_reset_resume_table[] = { #define BTUSB_HW_RESET_ACTIVE 12 #define BTUSB_TX_WAIT_VND_EVT 13 #define BTUSB_WAKEUP_DISABLE 14 +#define BTUSB_ALT3_OK_FOR_WBS 15 struct btusb_data { struct hci_dev *hdev; @@ -1748,16 +1749,20 @@ static void btusb_work(struct work_struct *work) /* Bluetooth USB spec recommends alt 6 (63 bytes), but * many adapters do not support it. Alt 1 appears to * work for all adapters that do not have alt 6, and - * which work with WBS at all. + * which work with WBS at all. Some devices prefer + * alt 3 (HCI payload >= 60 Bytes let air packet + * data satisfy 60 bytes), requiring + * MTU >= 3 (packets) * 25 (size) - 3 (headers) = 72 + * see also Core spec 5, vol 4, B 2.1.1 & Table 2.1. */ - new_alts = btusb_find_altsetting(data, 6) ? 6 : 1; - /* Because mSBC frames do not need to be aligned to the - * SCO packet boundary. If support the Alt 3, use the - * Alt 3 for HCI payload >= 60 Bytes let air packet - * data satisfy 60 bytes. - */ - if (new_alts == 1 && btusb_find_altsetting(data, 3)) + if (btusb_find_altsetting(data, 6)) + new_alts = 6; + else if (test_bit(BTUSB_ALT3_OK_FOR_WBS, &data->flags) && + hdev->sco_mtu >= 72 && + btusb_find_altsetting(data, 3)) new_alts = 3; + else + new_alts = 1; } if (btusb_switch_alt_setting(hdev, new_alts) < 0) @@ -4733,6 +4738,7 @@ static int btusb_probe(struct usb_interface *intf, * (DEVICE_REMOTE_WAKEUP) */ set_bit(BTUSB_WAKEUP_DISABLE, &data->flags); + set_bit(BTUSB_ALT3_OK_FOR_WBS, &data->flags); } if (!reset)
Some USB BT adapters don't satisfy the MTU requirement mentioned in commit e848dbd364ac ("Bluetooth: btusb: Add support USB ALT 3 for WBS") and have ALT 3 setting that produces no/garbled audio. Some adapters with larger MTU were also reported to have problems with ALT 3. Add a flag and check it and MTU before selecting ALT 3, falling back to ALT 1. Enable the flag for Realtek, restoring the previous behavior for non-Realtek devices. Tested with USB adapters (mtu<72, no/garbled sound with ALT3, ALT1 works) BCM20702A1 0b05:17cb, CSR8510A10 0a12:0001, and (mtu>=72, ALT3 works) RTL8761BU 0bda:8771, Intel AX200 8087:0029 (after disabling ALT6). Also got reports for (mtu>=72, ALT 3 reported to produce bad audio) Intel 8087:0a2b. Signed-off-by: Pauli Virtanen <pav@iki.fi> Fixes: e848dbd364ac ("Bluetooth: btusb: Add support USB ALT 3 for WBS") --- Changes in v2: - Explain magic number 72 in a comment; didn't add the table for them, because it's not used elsewhere and we need just one number from it. - Add flag for ALT3 support, restoring the behavior for non-Realtek devices the same as before e848dbd364ac, due to the problems reported on an Intel adapter. Don't have the device myself. drivers/bluetooth/btusb.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-)