[PATCH] bluetooth: add quirk using packet size 60

Hilda Wu posted 1 patch 3 weeks, 4 days ago
There is a newer version of this series
drivers/bluetooth/btrtl.c |  3 ++
drivers/bluetooth/btrtl.h |  1 +
drivers/bluetooth/btusb.c | 89 ++++++++++++++++++++++++++++++---------
3 files changed, 73 insertions(+), 20 deletions(-)
[PATCH] bluetooth: add quirk using packet size 60
Posted by Hilda Wu 3 weeks, 4 days ago
The RTL8852BE-VT supports USB alternate setting 6.
However, its descriptor does not report this capability to the host.
Therefore, a quirk is needed to bypass the RTL8852BE-VT's descriptor
and allow it to use USB ALT 6 directly.

Signed-off-by: Hilda Wu <hildawu@realtek.com>
---
 drivers/bluetooth/btrtl.c |  3 ++
 drivers/bluetooth/btrtl.h |  1 +
 drivers/bluetooth/btusb.c | 89 ++++++++++++++++++++++++++++++---------
 3 files changed, 73 insertions(+), 20 deletions(-)

diff --git a/drivers/bluetooth/btrtl.c b/drivers/bluetooth/btrtl.c
index 0bcb44cf7b31..b75f0b36a09b 100644
--- a/drivers/bluetooth/btrtl.c
+++ b/drivers/bluetooth/btrtl.c
@@ -1312,6 +1312,9 @@ void btrtl_set_quirks(struct hci_dev *hdev, struct btrtl_device_info *btrtl_dev)
 		    btrtl_dev->project_id == CHIP_ID_8852C)
 			set_bit(HCI_QUIRK_USE_MSFT_EXT_ADDRESS_FILTER, &hdev->quirks);
 
+		if (btrtl_dev->project_id == CHIP_ID_8852BT)
+			btrealtek_set_flag(hdev, REALTEK_ALT6_FORCE);
+
 		hci_set_aosp_capable(hdev);
 		break;
 	default:
diff --git a/drivers/bluetooth/btrtl.h b/drivers/bluetooth/btrtl.h
index a2d9d34f9fb0..ffec2fca88ec 100644
--- a/drivers/bluetooth/btrtl.h
+++ b/drivers/bluetooth/btrtl.h
@@ -105,6 +105,7 @@ struct rtl_vendor_cmd {
 
 enum {
 	REALTEK_ALT6_CONTINUOUS_TX_CHIP,
+	REALTEK_ALT6_FORCE,
 
 	__REALTEK_NUM_FLAGS,
 };
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 514d593923ad..eca0b173232e 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -804,6 +804,7 @@ struct qca_dump_info {
 #define BTUSB_USE_ALT3_FOR_WBS	15
 #define BTUSB_ALT6_CONTINUOUS_TX	16
 #define BTUSB_HW_SSR_ACTIVE	17
+#define BTUSB_ALT_CHANGED	18
 
 struct btusb_data {
 	struct hci_dev       *hdev;
@@ -2130,16 +2131,61 @@ static void btusb_notify(struct hci_dev *hdev, unsigned int evt)
 	}
 }
 
+static struct usb_host_interface *btusb_find_altsetting(struct btusb_data *data,
+							int alt)
+{
+	struct usb_interface *intf = data->isoc;
+	int i;
+
+	BT_DBG("Looking for Alt no :%d", alt);
+
+	if (!intf)
+		return NULL;
+
+	for (i = 0; i < intf->num_altsetting; i++) {
+		if (intf->altsetting[i].desc.bAlternateSetting == alt)
+			return &intf->altsetting[i];
+	}
+
+	return NULL;
+}
+
 static inline int __set_isoc_interface(struct hci_dev *hdev, int altsetting)
 {
 	struct btusb_data *data = hci_get_drvdata(hdev);
 	struct usb_interface *intf = data->isoc;
 	struct usb_endpoint_descriptor *ep_desc;
+	struct usb_host_interface *alt;
 	int i, err;
 
 	if (!data->isoc)
 		return -ENODEV;
 
+	/* For some Realtek chips, they actually have the altsetting 6, but its
+	 * altsetting descriptor is not exposed. We can activate altsetting 6 by
+	 * replacing the altsetting 5.
+	 */
+	if (altsetting == 6 && !btusb_find_altsetting(data, 6) &&
+	    btrealtek_test_flag(hdev, REALTEK_ALT6_FORCE)) {
+		alt = NULL;
+		for (i = 0; i < intf->num_altsetting; i++) {
+			if (intf->altsetting[i].desc.bAlternateSetting == 5) {
+				alt = &intf->altsetting[i];
+				break;
+			}
+		}
+		if (alt) {
+			for (i = 0; i < alt->desc.bNumEndpoints; i++) {
+				ep_desc = &alt->endpoint[i].desc;
+				if (usb_endpoint_is_isoc_out(ep_desc) ||
+				    usb_endpoint_is_isoc_in(ep_desc))
+					ep_desc->wMaxPacketSize = 63;
+			}
+			alt->desc.bAlternateSetting = 6;
+			set_bit(BTUSB_ALT_CHANGED, &data->flags);
+		}
+	}
+
 	err = usb_set_interface(data->udev, data->isoc_ifnum, altsetting);
 	if (err < 0) {
 		bt_dev_err(hdev, "setting interface failed (%d)", -err);
@@ -2151,6 +2197,27 @@ static inline int __set_isoc_interface(struct hci_dev *hdev, int altsetting)
 	data->isoc_tx_ep = NULL;
 	data->isoc_rx_ep = NULL;
 
+	/* Recover alt 5 desc if alt 0 is set. */
+	if (!altsetting && test_bit(BTUSB_ALT_CHANGED, &data->flags)) {
+		alt = NULL;
+		for (i = 0; i < intf->num_altsetting; i++) {
+			if (intf->altsetting[i].desc.bAlternateSetting == 6) {
+				alt = &intf->altsetting[i];
+				break;
+			}
+		}
+		if (alt) {
+			for (i = 0; i < alt->desc.bNumEndpoints; i++) {
+				ep_desc = &alt->endpoint[i].desc;
+				if (usb_endpoint_is_isoc_out(ep_desc) ||
+				    usb_endpoint_is_isoc_in(ep_desc))
+					ep_desc->wMaxPacketSize = 49;
+			}
+			alt->desc.bAlternateSetting = 5;
+			clear_bit(BTUSB_ALT_CHANGED, &data->flags);
+		}
+	}
+
 	for (i = 0; i < intf->cur_altsetting->desc.bNumEndpoints; i++) {
 		ep_desc = &intf->cur_altsetting->endpoint[i].desc;
 
@@ -2213,25 +2280,6 @@ static int btusb_switch_alt_setting(struct hci_dev *hdev, int new_alts)
 	return 0;
 }
 
-static struct usb_host_interface *btusb_find_altsetting(struct btusb_data *data,
-							int alt)
-{
-	struct usb_interface *intf = data->isoc;
-	int i;
-
-	BT_DBG("Looking for Alt no :%d", alt);
-
-	if (!intf)
-		return NULL;
-
-	for (i = 0; i < intf->num_altsetting; i++) {
-		if (intf->altsetting[i].desc.bAlternateSetting == alt)
-			return &intf->altsetting[i];
-	}
-
-	return NULL;
-}
-
 static void btusb_work(struct work_struct *work)
 {
 	struct btusb_data *data = container_of(work, struct btusb_data, work);
@@ -2269,7 +2317,8 @@ static void btusb_work(struct work_struct *work)
 			 * MTU >= 3 (packets) * 25 (size) - 3 (headers) = 72
 			 * see also Core spec 5, vol 4, B 2.1.1 & Table 2.1.
 			 */
-			if (btusb_find_altsetting(data, 6))
+			if (btusb_find_altsetting(data, 6) ||
+			    btrealtek_test_flag(hdev, REALTEK_ALT6_FORCE))
 				new_alts = 6;
 			else if (btusb_find_altsetting(data, 3) &&
 				 hdev->sco_mtu >= 72 &&
-- 
2.34.1
Re: [PATCH] bluetooth: add quirk using packet size 60
Posted by Luiz Augusto von Dentz 3 weeks, 3 days ago
Hi Hilda,

On Wed, Oct 30, 2024 at 6:08 AM Hilda Wu <hildawu@realtek.com> wrote:
>
> The RTL8852BE-VT supports USB alternate setting 6.
> However, its descriptor does not report this capability to the host.
> Therefore, a quirk is needed to bypass the RTL8852BE-VT's descriptor
> and allow it to use USB ALT 6 directly.

It is getting very hackish trying to fixup the descriptor in software,
isn't it possible to update the USB descriptors via firmware update?
Not to mention you didn't include any logs of how this was tested, I
suppose this is for HFP/SCO wideband (using msbc), is that just plain
broken right now?

> Signed-off-by: Hilda Wu <hildawu@realtek.com>
> ---
>  drivers/bluetooth/btrtl.c |  3 ++
>  drivers/bluetooth/btrtl.h |  1 +
>  drivers/bluetooth/btusb.c | 89 ++++++++++++++++++++++++++++++---------
>  3 files changed, 73 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/bluetooth/btrtl.c b/drivers/bluetooth/btrtl.c
> index 0bcb44cf7b31..b75f0b36a09b 100644
> --- a/drivers/bluetooth/btrtl.c
> +++ b/drivers/bluetooth/btrtl.c
> @@ -1312,6 +1312,9 @@ void btrtl_set_quirks(struct hci_dev *hdev, struct btrtl_device_info *btrtl_dev)
>                     btrtl_dev->project_id == CHIP_ID_8852C)
>                         set_bit(HCI_QUIRK_USE_MSFT_EXT_ADDRESS_FILTER, &hdev->quirks);
>
> +               if (btrtl_dev->project_id == CHIP_ID_8852BT)
> +                       btrealtek_set_flag(hdev, REALTEK_ALT6_FORCE);
> +
>                 hci_set_aosp_capable(hdev);
>                 break;
>         default:
> diff --git a/drivers/bluetooth/btrtl.h b/drivers/bluetooth/btrtl.h
> index a2d9d34f9fb0..ffec2fca88ec 100644
> --- a/drivers/bluetooth/btrtl.h
> +++ b/drivers/bluetooth/btrtl.h
> @@ -105,6 +105,7 @@ struct rtl_vendor_cmd {
>
>  enum {
>         REALTEK_ALT6_CONTINUOUS_TX_CHIP,
> +       REALTEK_ALT6_FORCE,
>
>         __REALTEK_NUM_FLAGS,
>  };
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index 514d593923ad..eca0b173232e 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -804,6 +804,7 @@ struct qca_dump_info {
>  #define BTUSB_USE_ALT3_FOR_WBS 15
>  #define BTUSB_ALT6_CONTINUOUS_TX       16
>  #define BTUSB_HW_SSR_ACTIVE    17
> +#define BTUSB_ALT_CHANGED      18
>
>  struct btusb_data {
>         struct hci_dev       *hdev;
> @@ -2130,16 +2131,61 @@ static void btusb_notify(struct hci_dev *hdev, unsigned int evt)
>         }
>  }
>
> +static struct usb_host_interface *btusb_find_altsetting(struct btusb_data *data,
> +                                                       int alt)
> +{
> +       struct usb_interface *intf = data->isoc;
> +       int i;
> +
> +       BT_DBG("Looking for Alt no :%d", alt);
> +
> +       if (!intf)
> +               return NULL;
> +
> +       for (i = 0; i < intf->num_altsetting; i++) {
> +               if (intf->altsetting[i].desc.bAlternateSetting == alt)
> +                       return &intf->altsetting[i];
> +       }
> +
> +       return NULL;
> +}
> +
>  static inline int __set_isoc_interface(struct hci_dev *hdev, int altsetting)
>  {
>         struct btusb_data *data = hci_get_drvdata(hdev);
>         struct usb_interface *intf = data->isoc;
>         struct usb_endpoint_descriptor *ep_desc;
> +       struct usb_host_interface *alt;
>         int i, err;
>
>         if (!data->isoc)
>                 return -ENODEV;
>
> +       /* For some Realtek chips, they actually have the altsetting 6, but its
> +        * altsetting descriptor is not exposed. We can activate altsetting 6 by
> +        * replacing the altsetting 5.
> +        */
> +       if (altsetting == 6 && !btusb_find_altsetting(data, 6) &&
> +           btrealtek_test_flag(hdev, REALTEK_ALT6_FORCE)) {
> +               alt = NULL;
> +               for (i = 0; i < intf->num_altsetting; i++) {
> +                       if (intf->altsetting[i].desc.bAlternateSetting == 5) {
> +                               alt = &intf->altsetting[i];
> +                               break;
> +                       }
> +               }

I believe you can replace the code above with btusb_find_altsetting so
please use that instead of duplicating its logic.

> +               if (alt) {
> +                       for (i = 0; i < alt->desc.bNumEndpoints; i++) {
> +                               ep_desc = &alt->endpoint[i].desc;
> +                               if (usb_endpoint_is_isoc_out(ep_desc) ||
> +                                   usb_endpoint_is_isoc_in(ep_desc))
> +                                       ep_desc->wMaxPacketSize = 63;
> +                       }
> +                       alt->desc.bAlternateSetting = 6;
> +                       set_bit(BTUSB_ALT_CHANGED, &data->flags);
> +               }
> +       }
> +
>         err = usb_set_interface(data->udev, data->isoc_ifnum, altsetting);
>         if (err < 0) {
>                 bt_dev_err(hdev, "setting interface failed (%d)", -err);
> @@ -2151,6 +2197,27 @@ static inline int __set_isoc_interface(struct hci_dev *hdev, int altsetting)
>         data->isoc_tx_ep = NULL;
>         data->isoc_rx_ep = NULL;
>
> +       /* Recover alt 5 desc if alt 0 is set. */
> +       if (!altsetting && test_bit(BTUSB_ALT_CHANGED, &data->flags)) {
> +               alt = NULL;
> +               for (i = 0; i < intf->num_altsetting; i++) {
> +                       if (intf->altsetting[i].desc.bAlternateSetting == 6) {
> +                               alt = &intf->altsetting[i];
> +                               break;
> +                       }
> +               }

Ditto.

> +               if (alt) {
> +                       for (i = 0; i < alt->desc.bNumEndpoints; i++) {
> +                               ep_desc = &alt->endpoint[i].desc;
> +                               if (usb_endpoint_is_isoc_out(ep_desc) ||
> +                                   usb_endpoint_is_isoc_in(ep_desc))
> +                                       ep_desc->wMaxPacketSize = 49;
> +                       }
> +                       alt->desc.bAlternateSetting = 5;
> +                       clear_bit(BTUSB_ALT_CHANGED, &data->flags);
> +               }
> +       }
> +
>         for (i = 0; i < intf->cur_altsetting->desc.bNumEndpoints; i++) {
>                 ep_desc = &intf->cur_altsetting->endpoint[i].desc;
>
> @@ -2213,25 +2280,6 @@ static int btusb_switch_alt_setting(struct hci_dev *hdev, int new_alts)
>         return 0;
>  }
>
> -static struct usb_host_interface *btusb_find_altsetting(struct btusb_data *data,
> -                                                       int alt)
> -{
> -       struct usb_interface *intf = data->isoc;
> -       int i;
> -
> -       BT_DBG("Looking for Alt no :%d", alt);
> -
> -       if (!intf)
> -               return NULL;
> -
> -       for (i = 0; i < intf->num_altsetting; i++) {
> -               if (intf->altsetting[i].desc.bAlternateSetting == alt)
> -                       return &intf->altsetting[i];
> -       }
> -
> -       return NULL;
> -}
> -
>  static void btusb_work(struct work_struct *work)
>  {
>         struct btusb_data *data = container_of(work, struct btusb_data, work);
> @@ -2269,7 +2317,8 @@ static void btusb_work(struct work_struct *work)
>                          * MTU >= 3 (packets) * 25 (size) - 3 (headers) = 72
>                          * see also Core spec 5, vol 4, B 2.1.1 & Table 2.1.
>                          */
> -                       if (btusb_find_altsetting(data, 6))
> +                       if (btusb_find_altsetting(data, 6) ||
> +                           btrealtek_test_flag(hdev, REALTEK_ALT6_FORCE))
>                                 new_alts = 6;
>                         else if (btusb_find_altsetting(data, 3) &&
>                                  hdev->sco_mtu >= 72 &&
> --
> 2.34.1
>


-- 
Luiz Augusto von Dentz
RE: [PATCH] bluetooth: add quirk using packet size 60
Posted by Hilda Wu 6 days, 15 hours ago
Hi Luiz,
Thanks for your feedback.
I just send the V2. 
Please see the reply below. Thank you

-----Original Message-----
From: Luiz Augusto von Dentz <luiz.dentz@gmail.com> 
Sent: Friday, November 1, 2024 4:47 AM
To: Hilda Wu <hildawu@realtek.com>
Cc: marcel@holtmann.org; linux-bluetooth@vger.kernel.org; linux-kernel@vger.kernel.org; Max Chou <max.chou@realtek.com>; alex_lu@realsil.com.cn; KidmanLee <kidman@realtek.com>
Subject: Re: [PATCH] bluetooth: add quirk using packet size 60


External mail.



Hi Hilda,

On Wed, Oct 30, 2024 at 6:08 AM Hilda Wu <hildawu@realtek.com> wrote:
>
> The RTL8852BE-VT supports USB alternate setting 6.
> However, its descriptor does not report this capability to the host.
> Therefore, a quirk is needed to bypass the RTL8852BE-VT's descriptor 
> and allow it to use USB ALT 6 directly.

It is getting very hackish trying to fixup the descriptor in software, isn't it possible to update the USB descriptors via firmware update?
Not to mention you didn't include any logs of how this was tested, I suppose this is for HFP/SCO wideband (using msbc), is that just plain broken right now?

Since there is currently no way for update the USB descriptors even if it is firmware. That’s why we do it through the driver.
This is for HFP/SCO wideband (using msbc), the broken state is other USB ALT will be used and there will be no sound.

> Signed-off-by: Hilda Wu <hildawu@realtek.com>
> ---
>  drivers/bluetooth/btrtl.c |  3 ++
>  drivers/bluetooth/btrtl.h |  1 +
>  drivers/bluetooth/btusb.c | 89 
> ++++++++++++++++++++++++++++++---------
>  3 files changed, 73 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/bluetooth/btrtl.c b/drivers/bluetooth/btrtl.c 
> index 0bcb44cf7b31..b75f0b36a09b 100644
> --- a/drivers/bluetooth/btrtl.c
> +++ b/drivers/bluetooth/btrtl.c
> @@ -1312,6 +1312,9 @@ void btrtl_set_quirks(struct hci_dev *hdev, struct btrtl_device_info *btrtl_dev)
>                     btrtl_dev->project_id == CHIP_ID_8852C)
>                         set_bit(HCI_QUIRK_USE_MSFT_EXT_ADDRESS_FILTER, 
> &hdev->quirks);
>
> +               if (btrtl_dev->project_id == CHIP_ID_8852BT)
> +                       btrealtek_set_flag(hdev, REALTEK_ALT6_FORCE);
> +
>                 hci_set_aosp_capable(hdev);
>                 break;
>         default:
> diff --git a/drivers/bluetooth/btrtl.h b/drivers/bluetooth/btrtl.h 
> index a2d9d34f9fb0..ffec2fca88ec 100644
> --- a/drivers/bluetooth/btrtl.h
> +++ b/drivers/bluetooth/btrtl.h
> @@ -105,6 +105,7 @@ struct rtl_vendor_cmd {
>
>  enum {
>         REALTEK_ALT6_CONTINUOUS_TX_CHIP,
> +       REALTEK_ALT6_FORCE,
>
>         __REALTEK_NUM_FLAGS,
>  };
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c 
> index 514d593923ad..eca0b173232e 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -804,6 +804,7 @@ struct qca_dump_info {  #define 
> BTUSB_USE_ALT3_FOR_WBS 15
>  #define BTUSB_ALT6_CONTINUOUS_TX       16
>  #define BTUSB_HW_SSR_ACTIVE    17
> +#define BTUSB_ALT_CHANGED      18
>
>  struct btusb_data {
>         struct hci_dev       *hdev;
> @@ -2130,16 +2131,61 @@ static void btusb_notify(struct hci_dev *hdev, unsigned int evt)
>         }
>  }
>
> +static struct usb_host_interface *btusb_find_altsetting(struct btusb_data *data,
> +                                                       int alt) {
> +       struct usb_interface *intf = data->isoc;
> +       int i;
> +
> +       BT_DBG("Looking for Alt no :%d", alt);
> +
> +       if (!intf)
> +               return NULL;
> +
> +       for (i = 0; i < intf->num_altsetting; i++) {
> +               if (intf->altsetting[i].desc.bAlternateSetting == alt)
> +                       return &intf->altsetting[i];
> +       }
> +
> +       return NULL;
> +}
> +
>  static inline int __set_isoc_interface(struct hci_dev *hdev, int 
> altsetting)  {
>         struct btusb_data *data = hci_get_drvdata(hdev);
>         struct usb_interface *intf = data->isoc;
>         struct usb_endpoint_descriptor *ep_desc;
> +       struct usb_host_interface *alt;
>         int i, err;
>
>         if (!data->isoc)
>                 return -ENODEV;
>
> +       /* For some Realtek chips, they actually have the altsetting 6, but its
> +        * altsetting descriptor is not exposed. We can activate altsetting 6 by
> +        * replacing the altsetting 5.
> +        */
> +       if (altsetting == 6 && !btusb_find_altsetting(data, 6) &&
> +           btrealtek_test_flag(hdev, REALTEK_ALT6_FORCE)) {
> +               alt = NULL;
> +               for (i = 0; i < intf->num_altsetting; i++) {
> +                       if (intf->altsetting[i].desc.bAlternateSetting == 5) {
> +                               alt = &intf->altsetting[i];
> +                               break;
> +                       }
> +               }

I believe you can replace the code above with btusb_find_altsetting so please use that instead of duplicating its logic.

Adjusted in V2.

> +               if (alt) {
> +                       for (i = 0; i < alt->desc.bNumEndpoints; i++) {
> +                               ep_desc = &alt->endpoint[i].desc;
> +                               if (usb_endpoint_is_isoc_out(ep_desc) ||
> +                                   usb_endpoint_is_isoc_in(ep_desc))
> +                                       ep_desc->wMaxPacketSize = 63;
> +                       }
> +                       alt->desc.bAlternateSetting = 6;
> +                       set_bit(BTUSB_ALT_CHANGED, &data->flags);
> +               }
> +       }
> +
>         err = usb_set_interface(data->udev, data->isoc_ifnum, altsetting);
>         if (err < 0) {
>                 bt_dev_err(hdev, "setting interface failed (%d)", 
> -err); @@ -2151,6 +2197,27 @@ static inline int __set_isoc_interface(struct hci_dev *hdev, int altsetting)
>         data->isoc_tx_ep = NULL;
>         data->isoc_rx_ep = NULL;
>
> +       /* Recover alt 5 desc if alt 0 is set. */
> +       if (!altsetting && test_bit(BTUSB_ALT_CHANGED, &data->flags)) {
> +               alt = NULL;
> +               for (i = 0; i < intf->num_altsetting; i++) {
> +                       if (intf->altsetting[i].desc.bAlternateSetting == 6) {
> +                               alt = &intf->altsetting[i];
> +                               break;
> +                       }
> +               }

Ditto.

Adjusted in V2.

> +               if (alt) {
> +                       for (i = 0; i < alt->desc.bNumEndpoints; i++) {
> +                               ep_desc = &alt->endpoint[i].desc;
> +                               if (usb_endpoint_is_isoc_out(ep_desc) ||
> +                                   usb_endpoint_is_isoc_in(ep_desc))
> +                                       ep_desc->wMaxPacketSize = 49;
> +                       }
> +                       alt->desc.bAlternateSetting = 5;
> +                       clear_bit(BTUSB_ALT_CHANGED, &data->flags);
> +               }
> +       }
> +
>         for (i = 0; i < intf->cur_altsetting->desc.bNumEndpoints; i++) {
>                 ep_desc = &intf->cur_altsetting->endpoint[i].desc;
>
> @@ -2213,25 +2280,6 @@ static int btusb_switch_alt_setting(struct hci_dev *hdev, int new_alts)
>         return 0;
>  }
>
> -static struct usb_host_interface *btusb_find_altsetting(struct btusb_data *data,
> -                                                       int alt)
> -{
> -       struct usb_interface *intf = data->isoc;
> -       int i;
> -
> -       BT_DBG("Looking for Alt no :%d", alt);
> -
> -       if (!intf)
> -               return NULL;
> -
> -       for (i = 0; i < intf->num_altsetting; i++) {
> -               if (intf->altsetting[i].desc.bAlternateSetting == alt)
> -                       return &intf->altsetting[i];
> -       }
> -
> -       return NULL;
> -}
> -
>  static void btusb_work(struct work_struct *work)  {
>         struct btusb_data *data = container_of(work, struct 
> btusb_data, work); @@ -2269,7 +2317,8 @@ static void btusb_work(struct work_struct *work)
>                          * MTU >= 3 (packets) * 25 (size) - 3 (headers) = 72
>                          * see also Core spec 5, vol 4, B 2.1.1 & Table 2.1.
>                          */
> -                       if (btusb_find_altsetting(data, 6))
> +                       if (btusb_find_altsetting(data, 6) ||
> +                           btrealtek_test_flag(hdev, 
> + REALTEK_ALT6_FORCE))
>                                 new_alts = 6;
>                         else if (btusb_find_altsetting(data, 3) &&
>                                  hdev->sco_mtu >= 72 &&
> --
> 2.34.1
>


--
Luiz Augusto von Dentz
Re: [PATCH] bluetooth: add quirk using packet size 60
Posted by kernel test robot 3 weeks, 3 days ago
Hi Hilda,

kernel test robot noticed the following build warnings:

[auto build test WARNING on bluetooth/master]
[also build test WARNING on bluetooth-next/master linus/master v6.12-rc5 next-20241031]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Hilda-Wu/bluetooth-add-quirk-using-packet-size-60/20241030-181008
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth.git master
patch link:    https://lore.kernel.org/r/20241030100804.2743115-1-hildawu%40realtek.com
patch subject: [PATCH] bluetooth: add quirk using packet size 60
config: x86_64-randconfig-123-20241031 (https://download.01.org/0day-ci/archive/20241031/202410312046.r3WbTClD-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241031/202410312046.r3WbTClD-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410312046.r3WbTClD-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/bluetooth/btusb.c:2156:65: sparse: sparse: incorrect type in assignment (different base types) @@     expected restricted __le16 [usertype] wMaxPacketSize @@     got int @@
   drivers/bluetooth/btusb.c:2156:65: sparse:     expected restricted __le16 [usertype] wMaxPacketSize
   drivers/bluetooth/btusb.c:2156:65: sparse:     got int
   drivers/bluetooth/btusb.c:2188:65: sparse: sparse: incorrect type in assignment (different base types) @@     expected restricted __le16 [usertype] wMaxPacketSize @@     got int @@
   drivers/bluetooth/btusb.c:2188:65: sparse:     expected restricted __le16 [usertype] wMaxPacketSize
   drivers/bluetooth/btusb.c:2188:65: sparse:     got int
>> drivers/bluetooth/btusb.c:2156:65: sparse: sparse: incorrect type in assignment (different base types) @@     expected restricted __le16 [usertype] wMaxPacketSize @@     got int @@
   drivers/bluetooth/btusb.c:2156:65: sparse:     expected restricted __le16 [usertype] wMaxPacketSize
   drivers/bluetooth/btusb.c:2156:65: sparse:     got int
   drivers/bluetooth/btusb.c:2188:65: sparse: sparse: incorrect type in assignment (different base types) @@     expected restricted __le16 [usertype] wMaxPacketSize @@     got int @@
   drivers/bluetooth/btusb.c:2188:65: sparse:     expected restricted __le16 [usertype] wMaxPacketSize
   drivers/bluetooth/btusb.c:2188:65: sparse:     got int

vim +2156 drivers/bluetooth/btusb.c

  2126	
  2127	static inline int __set_isoc_interface(struct hci_dev *hdev, int altsetting)
  2128	{
  2129		struct btusb_data *data = hci_get_drvdata(hdev);
  2130		struct usb_interface *intf = data->isoc;
  2131		struct usb_endpoint_descriptor *ep_desc;
  2132		struct usb_host_interface *alt;
  2133		int i, err;
  2134	
  2135		if (!data->isoc)
  2136			return -ENODEV;
  2137	
  2138		/* For some Realtek chips, they actually have the altsetting 6, but its
  2139		 * altsetting descriptor is not exposed. We can activate altsetting 6 by
  2140		 * replacing the altsetting 5.
  2141		 */
  2142		if (altsetting == 6 && !btusb_find_altsetting(data, 6) &&
  2143		    btrealtek_test_flag(hdev, REALTEK_ALT6_FORCE)) {
  2144			alt = NULL;
  2145			for (i = 0; i < intf->num_altsetting; i++) {
  2146				if (intf->altsetting[i].desc.bAlternateSetting == 5) {
  2147					alt = &intf->altsetting[i];
  2148					break;
  2149				}
  2150			}
  2151			if (alt) {
  2152				for (i = 0; i < alt->desc.bNumEndpoints; i++) {
  2153					ep_desc = &alt->endpoint[i].desc;
  2154					if (usb_endpoint_is_isoc_out(ep_desc) ||
  2155					    usb_endpoint_is_isoc_in(ep_desc))
> 2156						ep_desc->wMaxPacketSize = 63;
  2157				}
  2158				alt->desc.bAlternateSetting = 6;
  2159				set_bit(BTUSB_ALT_CHANGED, &data->flags);
  2160			}
  2161		}
  2162	
  2163		err = usb_set_interface(data->udev, data->isoc_ifnum, altsetting);
  2164		if (err < 0) {
  2165			bt_dev_err(hdev, "setting interface failed (%d)", -err);
  2166			return err;
  2167		}
  2168	
  2169		data->isoc_altsetting = altsetting;
  2170	
  2171		data->isoc_tx_ep = NULL;
  2172		data->isoc_rx_ep = NULL;
  2173	
  2174		/* Recover alt 5 desc if alt 0 is set. */
  2175		if (!altsetting && test_bit(BTUSB_ALT_CHANGED, &data->flags)) {
  2176			alt = NULL;
  2177			for (i = 0; i < intf->num_altsetting; i++) {
  2178				if (intf->altsetting[i].desc.bAlternateSetting == 6) {
  2179					alt = &intf->altsetting[i];
  2180					break;
  2181				}
  2182			}
  2183			if (alt) {
  2184				for (i = 0; i < alt->desc.bNumEndpoints; i++) {
  2185					ep_desc = &alt->endpoint[i].desc;
  2186					if (usb_endpoint_is_isoc_out(ep_desc) ||
  2187					    usb_endpoint_is_isoc_in(ep_desc))
  2188						ep_desc->wMaxPacketSize = 49;
  2189				}
  2190				alt->desc.bAlternateSetting = 5;
  2191				clear_bit(BTUSB_ALT_CHANGED, &data->flags);
  2192			}
  2193		}
  2194	
  2195		for (i = 0; i < intf->cur_altsetting->desc.bNumEndpoints; i++) {
  2196			ep_desc = &intf->cur_altsetting->endpoint[i].desc;
  2197	
  2198			if (!data->isoc_tx_ep && usb_endpoint_is_isoc_out(ep_desc)) {
  2199				data->isoc_tx_ep = ep_desc;
  2200				continue;
  2201			}
  2202	
  2203			if (!data->isoc_rx_ep && usb_endpoint_is_isoc_in(ep_desc)) {
  2204				data->isoc_rx_ep = ep_desc;
  2205				continue;
  2206			}
  2207		}
  2208	
  2209		if (!data->isoc_tx_ep || !data->isoc_rx_ep) {
  2210			bt_dev_err(hdev, "invalid SCO descriptors");
  2211			return -ENODEV;
  2212		}
  2213	
  2214		return 0;
  2215	}
  2216	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki