diff mbox series

[4/4] Bluetooth: btusb: Signal URB errors as TX failure

Message ID 20211109164113.65981-5-benjamin@sipsolutions.net (mailing list archive)
State Superseded
Headers show
Series Cancel sync commands if a TX failure occurs | expand

Checks

Context Check Description
tedd_an/checkpatch success Checkpatch PASS
tedd_an/gitlint success Gitlint PASS

Commit Message

Benjamin Berg Nov. 9, 2021, 4:41 p.m. UTC
From: Benjamin Berg <bberg@redhat.com>

Call the TX failure handler when transmission of URBs fail. This is done
both for failures to send an URB and also when the interrupt URB used to
retrieve a response fails.

This approach is sufficient to quickly deal with certain errors such as
a device being disconnected while synchronous commands are done during
initialization.

Signed-off-by: Benjamin Berg <bberg@redhat.com>
---
 drivers/bluetooth/btusb.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

Comments

Luiz Augusto von Dentz Nov. 9, 2021, 11:25 p.m. UTC | #1
Hi Benjamin,

On Tue, Nov 9, 2021 at 2:35 PM Benjamin Berg <benjamin@sipsolutions.net> wrote:
>
> From: Benjamin Berg <bberg@redhat.com>
>
> Call the TX failure handler when transmission of URBs fail. This is done
> both for failures to send an URB and also when the interrupt URB used to
> retrieve a response fails.
>
> This approach is sufficient to quickly deal with certain errors such as
> a device being disconnected while synchronous commands are done during
> initialization.
>
> Signed-off-by: Benjamin Berg <bberg@redhat.com>
> ---
>  drivers/bluetooth/btusb.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index 75c83768c257..0c4fe89c6573 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -924,6 +924,8 @@ static void btusb_intr_complete(struct urb *urb)
>                 if (err != -EPERM && err != -ENODEV)
>                         bt_dev_err(hdev, "urb %p failed to resubmit (%d)",
>                                    urb, -err);
> +               if (err != -EPERM)
> +                       hci_tx_error(hdev, -err);
>                 usb_unanchor_urb(urb);
>         }
>  }
> @@ -967,6 +969,8 @@ static int btusb_submit_intr_urb(struct hci_dev *hdev, gfp_t mem_flags)
>                 if (err != -EPERM && err != -ENODEV)
>                         bt_dev_err(hdev, "urb %p submission failed (%d)",
>                                    urb, -err);
> +               if (err != -EPERM)
> +                       hci_tx_error(hdev, -err);
>                 usb_unanchor_urb(urb);
>         }
>
> @@ -1322,10 +1326,12 @@ static void btusb_tx_complete(struct urb *urb)
>         if (!test_bit(HCI_RUNNING, &hdev->flags))
>                 goto done;
>
> -       if (!urb->status)
> +       if (!urb->status) {
>                 hdev->stat.byte_tx += urb->transfer_buffer_length;
> -       else
> +       } else {
> +               hci_tx_error(hdev, -urb->status);
>                 hdev->stat.err_tx++;
> +       }

Looks like we are reusing the btusb_tx_complete for all endpoints but
the likes of hci_tx_error/hci_cmd_sync_cancel only applies to commands
(e.g:  alloc_ctrl_urb), perhaps there is a way to detect if this is
actually a control urb or not so we can skip this for bulk transfers,
or actually if a bulk transfer fails we may actually need to resend
depending if the error is recoverable since the bulk transfers can
actually contain fragments rather than the entire packet, but I'd
leave that for another patch since it is probably not what you are
trying to fix in this set.

>  done:
>         spin_lock_irqsave(&data->txlock, flags);
> @@ -1348,10 +1354,12 @@ static void btusb_isoc_tx_complete(struct urb *urb)
>         if (!test_bit(HCI_RUNNING, &hdev->flags))
>                 goto done;
>
> -       if (!urb->status)
> +       if (!urb->status) {
>                 hdev->stat.byte_tx += urb->transfer_buffer_length;
> -       else
> +       } else {
> +               hci_tx_error(hdev, -urb->status);
>                 hdev->stat.err_tx++;
> +       }
>
>  done:
>         kfree(urb->setup_packet);
> --
> 2.31.1
>
diff mbox series

Patch

diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 75c83768c257..0c4fe89c6573 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -924,6 +924,8 @@  static void btusb_intr_complete(struct urb *urb)
 		if (err != -EPERM && err != -ENODEV)
 			bt_dev_err(hdev, "urb %p failed to resubmit (%d)",
 				   urb, -err);
+		if (err != -EPERM)
+			hci_tx_error(hdev, -err);
 		usb_unanchor_urb(urb);
 	}
 }
@@ -967,6 +969,8 @@  static int btusb_submit_intr_urb(struct hci_dev *hdev, gfp_t mem_flags)
 		if (err != -EPERM && err != -ENODEV)
 			bt_dev_err(hdev, "urb %p submission failed (%d)",
 				   urb, -err);
+		if (err != -EPERM)
+			hci_tx_error(hdev, -err);
 		usb_unanchor_urb(urb);
 	}
 
@@ -1322,10 +1326,12 @@  static void btusb_tx_complete(struct urb *urb)
 	if (!test_bit(HCI_RUNNING, &hdev->flags))
 		goto done;
 
-	if (!urb->status)
+	if (!urb->status) {
 		hdev->stat.byte_tx += urb->transfer_buffer_length;
-	else
+	} else {
+		hci_tx_error(hdev, -urb->status);
 		hdev->stat.err_tx++;
+	}
 
 done:
 	spin_lock_irqsave(&data->txlock, flags);
@@ -1348,10 +1354,12 @@  static void btusb_isoc_tx_complete(struct urb *urb)
 	if (!test_bit(HCI_RUNNING, &hdev->flags))
 		goto done;
 
-	if (!urb->status)
+	if (!urb->status) {
 		hdev->stat.byte_tx += urb->transfer_buffer_length;
-	else
+	} else {
+		hci_tx_error(hdev, -urb->status);
 		hdev->stat.err_tx++;
+	}
 
 done:
 	kfree(urb->setup_packet);