Re: [PATCH v2 1/2] Bluetooth: btusb: mediatek: Fix leaked runtime PM reference in reset

Jiajia Liu <[email protected]>
Newsgroups org.infradead.lists.linux-mediatek,org.infradead.lists.linux-arm-kernel,org.kernel.vger.linux-bluetooth,org.kernel.vger.linux-kernel
Message-ID <aobQUR1NXk5vHBrg@nature>
On Wed, Aug 19, 2026 at 12:35:38PM -0400, Luiz Augusto von Dentz wrote:
> Hi Jiajia,
> 
> On Wed, Aug 19, 2026 at 5:52 AM Jiajia Liu <[email protected]> wrote:
> >
> > MT7925 on HP Pro Mini 260 sometimes timed out during reloading driver
> > and reset usb device. btusb_suspend is not called again after closing
> > bluetooth interface.
> >
> >  usbcore: registered new interface driver btusb
> >  Bluetooth: hci0: HW/SW Version: 0x00000000, Build Time: 20260605184935
> >  Bluetooth: hci0: Execution of wmt command timed out
> >  Bluetooth: hci0: Failed to send wmt patch dwnld (-110)
> >  Bluetooth: hci0: Failed to set up firmware (-110)
> >  usb 3-10: reset high-speed USB device number 4 using xhci_hcd
> >  Bluetooth: hci0: HW/SW Version: 0x00000000, Build Time: 20260605184935
> >  Bluetooth: hci0: Device setup in 1856545 usecs
> >  Bluetooth: hci0: AOSP extensions version v1.00
> >  Bluetooth: hci0: AOSP quality report is supported
> >  Bluetooth: MGMT ver 1.23
> >
> > btusb_mtk_reset calls usb_autopm_get_interface to resume the device
> > before driving the hardware reset, but never calls the matching
> > usb_autopm_put_interface. Every hardware reset therefore leaks a PM
> > usage reference of the interface, preventing the device from being
> > runtime suspended again until it is unbound.
> >
> > Add BTUSB_USB_RESET_ACTIVE flag and set it before usb_queue_reset_device.
> > Release the PM reference in btusb_disconnect if this flag is set.
> >
> > Fixes: 25b6d7593a3a ("Bluetooth: btmtk: introduce btmtk reset work")
> > Assisted-by: Claude:qwen3.8-max
> > Signed-off-by: Jiajia Liu <[email protected]>
> > ---
> >
> > Changes in v2:
> > - Fix the race window (sashiko)
> >   Add and set BTUSB_USB_RESET_ACTIVE flag before usb_queue_reset_device.
> >   Release PM reference in btusb_disconnect if this flag is set.
> >
> > Changes in v1:
> > - add usb_autopm_put_interface after usb_queue_reset_device
> >
> > ---
> >  drivers/bluetooth/btusb.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> > index 184e95c1625e..45a726bed0dd 100644
> > --- a/drivers/bluetooth/btusb.c
> > +++ b/drivers/bluetooth/btusb.c
> > @@ -957,6 +957,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_USB_RESET_ACTIVE 18
> >
> >  struct btusb_data {
> >         struct hci_dev       *hdev;
> > @@ -2915,6 +2916,7 @@ static int btusb_mtk_reset(struct hci_dev *hdev, void *rst_data)
> >
> >         err = btmtk_usb_subsys_reset(hdev, btmtk_data->dev_id);
> >
> > +       set_bit(BTUSB_USB_RESET_ACTIVE, &data->flags);
> >         usb_queue_reset_device(data->intf);
> >         clear_bit(BTMTK_HW_RESET_ACTIVE, &btmtk_data->flags);
> >
> > @@ -4497,6 +4499,9 @@ static void btusb_disconnect(struct usb_interface *intf)
> >         if (data->reset_gpio)
> >                 gpiod_put(data->reset_gpio);
> >
> > +       if (test_bit(BTUSB_USB_RESET_ACTIVE, &data->flags))
> > +               usb_autopm_put_interface_no_suspend(data->intf);
> 
> Should this be test_and_clear_bit rather then just test_bit, otherwise
> it doesn't seem to be ever cleared? Sashiko also complains about the
> handling of multiple resets (is that really possible though?):

Will use test_and_clear_bit.

> 
> https://sashiko.dev/#/patchset/ec23dae6c247005e8eccd312d326a626163ea491.1787132512.git.liujiajia%40kylinos.cn
> 
> Or perhaps we should check if BTUSB_USB_RESET_ACTIVE (Id rename it to
> just BTUSB_RESET) and then don't queue a second time which perhaps is
> what sashiko is talking about when it refers to reference.

Sashiko concerns other PM usage references will leak if there are multiple
resets, since btusb_disconnect only drop one reference.

I am trying to use test_and_set_bit before usb_queue_reset_device. If flag
BTUSB_RESET_ACTIVE is already set, drop the reference it gets.

 @@ -2915,6 +2918,9 @@ static int btusb_mtk_reset(struct hci_dev *hdev, void *rst_data)
 
         err = btmtk_usb_subsys_reset(hdev, btmtk_data->dev_id);
 
 +       if (test_and_set_bit(BTUSB_RESET_ACTIVE, &data->flags))
 +               usb_autopm_put_interface_no_suspend(data->intf);
 +
         usb_queue_reset_device(data->intf);
         clear_bit(BTMTK_HW_RESET_ACTIVE, &btmtk_data->flags);
 
 @@ -4497,6 +4503,9 @@ static void btusb_disconnect(struct usb_interface *intf)
         if (data->reset_gpio)
                 gpiod_put(data->reset_gpio);
 
 +       if (test_and_clear_bit(BTUSB_RESET_ACTIVE, &data->flags))
 +               usb_autopm_put_interface_no_suspend(data->intf);

And add btmtk_reset_sync before btmtk_setup_firmware_79xx to construct two
calls of btusb_mtk_reset for testing.

 [29641.975493] usbcore: deregistering interface driver btusb
 [29641.997490] btusb_close: hci0
 [29642.334308] usbcore: registered new interface driver btusb
 [29642.334794] called btmtk_reset_sync
 [29642.335654] Bluetooth: hci0: HW/SW Version: 0x00000000, Build Time: 20260605184935
 [29643.559484] Bluetooth: hci0: adv larger than maximum supported
 [29643.569495] Bluetooth: hci0: adv larger than maximum supported
 [29647.472831] Bluetooth: hci0: adv larger than maximum supported
 [29648.920846] Bluetooth: hci0: adv larger than maximum supported
 [29652.577442] Bluetooth: hci0: Execution of wmt command timed out
 [29652.577450] Bluetooth: hci0: Failed to send wmt patch dwnld (-110)
 [29652.577468] Bluetooth: hci0: Failed to set up firmware (-110)
 [29652.577470] Bluetooth: hci0: HCI Enhanced Setup Synchronous Connection command is advertised, but not supported.
 [29652.577475] btusb_close: hci0

Detect two calls.

 [29652.786083] btusb_mtk_reset: already set BTUSB_RESET_ACTIVE
 [29652.922030] usb 3-10: reset high-speed USB device number 4 using xhci_hcd
 [29653.069414] Bluetooth: hci0: HW/SW Version: 0x00000000, Build Time: 20260605184935
 [29653.086161] Bluetooth: hci0: urb 00000000b24765f3 failed to resubmit (2)
 [29655.108547] Bluetooth: hci0: Device setup in 1993116 usecs
 [29655.108554] Bluetooth: hci0: HCI Enhanced Setup Synchronous Connection command is advertised, but not supported.
 [29657.121635] Bluetooth: hci0: Opcode 0x0c03 failed: -110
 [29659.137668] Bluetooth: hci0: Failed to read MSFT supported features (-110)
 [29661.153775] Bluetooth: hci0: AOSP get vendor capabilities (-110)
 [29661.154635] btusb_close: hci0
 [29661.290132] usb 3-10: reset high-speed USB device number 4 using xhci_hcd
 [29661.432184] Bluetooth: hci0: HW/SW Version: 0x00000000, Build Time: 20260605184935
 [29661.934878] Bluetooth: hci0: Device setup in 492744 usecs
 [29661.934887] Bluetooth: hci0: HCI Enhanced Setup Synchronous Connection command is advertised, but not supported.
 [29662.020980] Bluetooth: hci0: AOSP extensions version v1.00
 [29662.021001] Bluetooth: hci0: AOSP quality report is supported
 [29662.021474] Bluetooth: MGMT ver 1.23

Close bluetooth on control panel. btusb_suspend is called after two seconds.

 [29674.033188] btusb_close: hci0
 [29676.530032] btusb_suspend: intf 0000000016fc024b
 [29676.530053] btusb_suspend: intf 000000009b822b1b

> 
> >         if (intf == data->intf) {
> >                 if (data->isoc)
> >                         usb_driver_release_interface(&btusb_driver, data->isoc);
> > --
> > 2.55.0
> >
> 
> 
> -- 
> Luiz Augusto von Dentz
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.