| Message ID | 20240909205152.2397337-1-luiz.dentz@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v1] Bluetooth: btusb: Fix not handling ZPL/short-transfer | expand |
Dear Luiz, Thank you for your patch. Am 09.09.24 um 22:51 schrieb Luiz Augusto von Dentz: > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > 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.) > 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? > Fixes: 5e23b923da03 ("[Bluetooth] Add generic driver for Bluetooth USB devices") > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > --- > drivers/bluetooth/btusb.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c > index 36a869a57910..83df387aea92 100644 > --- a/drivers/bluetooth/btusb.c > +++ b/drivers/bluetooth/btusb.c > @@ -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) { Kind regards, Paul Kind regards, Paul
Hi Paul, On Tue, Sep 10, 2024 at 3:31 AM Paul Menzel <pmenzel@molgen.mpg.de> wrote: > > Dear Luiz, > > > Thank you for your patch. > > Am 09.09.24 um 22:51 schrieb Luiz Augusto von Dentz: > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > > > 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.) > > > 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. > > Fixes: 5e23b923da03 ("[Bluetooth] Add generic driver for Bluetooth USB devices") > > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > --- > > drivers/bluetooth/btusb.c | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c > > index 36a869a57910..83df387aea92 100644 > > --- a/drivers/bluetooth/btusb.c > > +++ b/drivers/bluetooth/btusb.c > > @@ -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) { > > > Kind regards, > > Paul > > Kind regards, > > Paul
Hi Luiz, I verified this patch and firmware down is passing. > >Hi Paul, > >On Tue, Sep 10, 2024 at 3:31 AM Paul Menzel <pmenzel@molgen.mpg.de> >wrote: >> >> Dear Luiz, >> >> >> Thank you for your patch. >> >> Am 09.09.24 um 22:51 schrieb Luiz Augusto von Dentz: >> > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> >> > >> > 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.) >> >> > 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. > >> > Fixes: 5e23b923da03 ("[Bluetooth] Add generic driver for Bluetooth >> > USB devices") >> > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> Tested-by: Kiran K <kiran.k@intel.com> >> > --- >> > drivers/bluetooth/btusb.c | 5 ++++- >> > 1 file changed, 4 insertions(+), 1 deletion(-) >> > >> > diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c >> > index 36a869a57910..83df387aea92 100644 >> > --- a/drivers/bluetooth/btusb.c >> > +++ b/drivers/bluetooth/btusb.c >> > @@ -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) { >> >> >> Kind regards, >> >> Paul >> >> Kind regards, >> >> Paul > > > >-- >Luiz Augusto von Dentz Thanks, Kiran
Hello: This patch was applied to bluetooth/bluetooth-next.git (master) by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>: On Mon, 9 Sep 2024 16:51:52 -0400 you wrote: > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > 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. > > [...] 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!
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c index 36a869a57910..83df387aea92 100644 --- a/drivers/bluetooth/btusb.c +++ b/drivers/bluetooth/btusb.c @@ -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) {