Skip to contentSkip to conversation

Bluetooth: btusb: Fix not handling ZPL/short-transfer

PATCH Applied Luiz Augusto von Dentz 4 participants 5 messages Sep 9, 2024 – Sep 12, 2024

Subsystems (2)
USB Bluetooth drivers
lore
Share & tools
lore

Luiz Augusto von Dentz

author maintainer
#0 lore raw

PATCH 1/1 Bluetooth: btusb: Fix not handling ZPL/short-transfer

From: Luiz Augusto von Dentz <[email protected]>

Requesting transfers of the exact same size of wMaxPacketSize may result in ZPL/short-transfer since the USB stack cannot handle it as we are limiting the buffer size to be the same as wMaxPacketSize.

Also, in terms of throughput this change has the same effect to interrupt endpoint as 290ba200815f "Bluetooth: Improve USB driver throughput by increasing the frame size" had for the bulk endpoint, so users of the advertisement bearer (e.g. BT Mesh) may benefit from this change.

Fixes 5e23b923da03 ("[Bluetooth] Add generic driver for Bluetooth USB devices")
Signed-off-by Luiz Augusto von Dentz <[email protected]>
---
 drivers/bluetooth/btusb.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
drivers/bluetooth/btusb.c +4 -1
index 36a869a57910..83df387aea92 100644
@@ -1341,7 +1341,10 @@ static int btusb_submit_intr_urb(struct hci_dev *hdev, gfp_t mem_flags)
 	if (!urb)
 		return -ENOMEM;
 
-	size = le16_to_cpu(data->intr_ep->wMaxPacketSize);
+	/* Use maximum HCI Event size so the USB stack handles
+	 * ZPL/short-transfer automatically.
+	 */
+	size = HCI_MAX_EVENT_SIZE;
 
 	buf = kmalloc(size, mem_flags);
 	if (!buf) {
Signature · 1 line
2.46.0
#1 lore raw

Dear Luiz,

Thank you for your patch.

Am 09.09.24 um 22:51 schrieb Luiz Augusto von Dentz:

Quoted 8 lines

From: Luiz Augusto von Dentz <[email protected]>

Requesting transfers of the exact same size of wMaxPacketSize may result in ZPL/short-transfer since the USB stack cannot handle it as we are limiting the buffer size to be the same as wMaxPacketSize.

Also, in terms of throughput this change has the same effect to interrupt endpoint as 290ba200815f "Bluetooth: Improve USB driver throughput

(*interrupt* would fit on the line above.)

Quoted 2 lines

by increasing the frame size" had for the bulk endpoint, so users of the advertisement bearer (e.g. BT Mesh) may benefit from this change.

Do you have a benchmark script, that can be run?

Quoted 12 lines
Fixes 5e23b923da03 ("[Bluetooth] Add generic driver for Bluetooth USB devices")
Signed-off-by Luiz Augusto von Dentz <[email protected]>
---
  drivers/bluetooth/btusb.c | 5 ++++-
  1 file changed, 4 insertions(+), 1 deletion(-)
drivers/bluetooth/btusb.c +0 -0
index 36a869a57910..83df387aea92 100644
@@ -1341,7 +1341,10 @@ static int btusb_submit_intr_urb(struct hci_dev *hdev, gfp_t mem_flags)
  	if (!urb)

drivers/bluetooth/btusb.c static int btusb_submit_intr_urb(struct hci_dev *hdev, gfp_t mem_flags) · line 1350

Quoted 10 lines
drivers/bluetooth/btusb.c +4 -1
  		return -ENOMEM;
 
-	size = le16_to_cpu(data->intr_ep->wMaxPacketSize);
+	/* Use maximum HCI Event size so the USB stack handles
+	 * ZPL/short-transfer automatically.
+	 */
+	size = HCI_MAX_EVENT_SIZE;
 
  	buf = kmalloc(size, mem_flags);
  	if (!buf) {

Kind regards,

Paul

Kind regards,

Paul

author maintainer
#2 lore raw

Hi Paul,

Quoted 16 lines · Paul Menzel

Dear Luiz,

Thank you for your patch.

Am 09.09.24 um 22:51 schrieb Luiz Augusto von Dentz:

Quoted 8 lines

From: Luiz Augusto von Dentz <[email protected]>

Requesting transfers of the exact same size of wMaxPacketSize may result in ZPL/short-transfer since the USB stack cannot handle it as we are limiting the buffer size to be the same as wMaxPacketSize.

Also, in terms of throughput this change has the same effect to interrupt endpoint as 290ba200815f "Bluetooth: Improve USB driver throughput

Quoted 6 lines

(*interrupt* would fit on the line above.)

Quoted 2 lines

by increasing the frame size" had for the bulk endpoint, so users of the advertisement bearer (e.g. BT Mesh) may benefit from this change.

Do you have a benchmark script, that can be run?

It is a **may** not a might and not the main objective of the change.

Quoted 31 lines
Quoted 12 lines
Fixes 5e23b923da03 ("[Bluetooth] Add generic driver for Bluetooth USB devices")
Signed-off-by Luiz Augusto von Dentz <[email protected]>
---
  drivers/bluetooth/btusb.c | 5 ++++-
  1 file changed, 4 insertions(+), 1 deletion(-)
drivers/bluetooth/btusb.c +0 -0
index 36a869a57910..83df387aea92 100644
@@ -1341,7 +1341,10 @@ static int btusb_submit_intr_urb(struct hci_dev *hdev, gfp_t mem_flags)
      if (!urb)

drivers/bluetooth/btusb.c static int btusb_submit_intr_urb(struct hci_dev *hdev, gfp_t mem_flags) · line 1350

Quoted 10 lines
drivers/bluetooth/btusb.c +4 -1
              return -ENOMEM;
 
-     size = le16_to_cpu(data->intr_ep->wMaxPacketSize);
+     /* Use maximum HCI Event size so the USB stack handles
+      * ZPL/short-transfer automatically.
+      */
+     size = HCI_MAX_EVENT_SIZE;
 
      buf = kmalloc(size, mem_flags);
      if (!buf) {

Kind regards,

Paul

Kind regards,

Paul

Signature · 1 line
Luiz Augusto von Dentz
Tested-by
#3 lore raw

Hi Luiz,

I verified this patch and firmware down is passing.

Quoted 22 lines

Hi Paul,

Quoted 17 lines · Paul Menzel

Dear Luiz,

Thank you for your patch.

Am 09.09.24 um 22:51 schrieb Luiz Augusto von Dentz:

Quoted 9 lines

From: Luiz Augusto von Dentz <[email protected]>

Requesting transfers of the exact same size of wMaxPacketSize may result in ZPL/short-transfer since the USB stack cannot handle it as we are limiting the buffer size to be the same as wMaxPacketSize.

Also, in terms of throughput this change has the same effect to interrupt endpoint as 290ba200815f "Bluetooth: Improve USB driver throughput

Quoted 12 lines
Quoted 6 lines

(*interrupt* would fit on the line above.)

Quoted 2 lines

by increasing the frame size" had for the bulk endpoint, so users of the advertisement bearer (e.g. BT Mesh) may benefit from this change.

Do you have a benchmark script, that can be run?

It is a **may** not a might and not the main objective of the change.

Quoted 3 lines
Quoted 3 lines
Fixes 5e23b923da03 ("[Bluetooth] Add generic driver for Bluetooth

USB devices")

Signed-off-by Luiz Augusto von Dentz <[email protected]>
Tested-by Kiran K <[email protected]>
Quoted 27 lines
Quoted 9 lines

drivers/bluetooth/btusb.c static int btusb_submit_intr_urb(struct hci_dev

Quoted 9 lines
---
  drivers/bluetooth/btusb.c | 5 ++++-
  1 file changed, 4 insertions(+), 1 deletion(-)
drivers/bluetooth/btusb.c +0 -0
index 36a869a57910..83df387aea92 100644
@@ -1341,7 +1341,10 @@ static int btusb_submit_intr_urb(struct hci_dev

*hdev, gfp_t mem_flags)

Quoted 17 lines

drivers/bluetooth/btusb.c static int btusb_submit_intr_urb(struct hci_dev *hdev, gfp_t mem_flags) · line 1350

Quoted 11 lines
drivers/bluetooth/btusb.c +4 -1
      if (!urb)
              return -ENOMEM;
 
-     size = le16_to_cpu(data->intr_ep->wMaxPacketSize);
+     /* Use maximum HCI Event size so the USB stack handles
+      * ZPL/short-transfer automatically.
+      */
+     size = HCI_MAX_EVENT_SIZE;
 
      buf = kmalloc(size, mem_flags);
      if (!buf) {

Kind regards,

Paul

Quoted 8 lines
Quoted 3 lines

Kind regards,

Paul

--
Luiz Augusto von Dentz

Thanks,
Kiran

CI Applied
#4 lore raw
In reply to Luiz Augusto von Dentz
This patch was applied to bluetooth/bluetooth-next.git (master) by Luiz Augusto von Dentz <[email protected]>:

Hello:

This patch was applied to bluetooth/bluetooth-next.git (master) by Luiz Augusto von Dentz <[email protected]>:

Quoted 6 lines · On Mon, 9 Sep 2024 16:51:52 -0400 you wrote:

From: Luiz Augusto von Dentz <[email protected]>

Requesting transfers of the exact same size of wMaxPacketSize may result in ZPL/short-transfer since the USB stack cannot handle it as we are limiting the buffer size to be the same as wMaxPacketSize.

Quoted 6 lines

Also, in terms of throughput this change has the same effect to interrupt endpoint as 290ba200815f "Bluetooth: Improve USB driver throughput by increasing the frame size" had for the bulk endpoint, so users of the advertisement bearer (e.g. BT Mesh) may benefit from this change.

[...]

Here is the summary with links:
- [v1] Bluetooth: btusb: Fix not handling ZPL/short-transfer

    https://git.kernel.org/bluetooth/bluetooth-next/c/7b05933340f4

You are awesome, thank you!

Signature · 2 lines
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html