[PATCH v2 3/3] Bluetooth: btmtksdio: call cancel_work_sync() outside of host lock scope

[Sean Wang]

Hi,

On Tue, Jun 16, 2026 at 6:15 AM Sergey Senozhatsky
<senozhatsky at chromium.org> wrote:
>
> cancel_work_sync() should be called outside of host lock scope
> in order to avoid circular locking scenario:
>
> CPU0                                    CPU1
>                                         close()/reset()
>                                         sdio_claim_host()
> txrx_work
>   sdio_claim_host() // sleeps
>                                         cancel_work_sync() // sleeps
>
> In addition, when txrx_work() runs concurrently with close()/reset()
> it better not to re-enable interrupts by testing for BTMTKSDIO_FUNC_ENABLED
> and not BTMTKSDIO_HW_RESET_ACTIVE before C_INT_EN_SET write.  However,
> btmtksdio_close() clears the BTMTKSDIO_FUNC_ENABLED too late (after
> cancel_work_sync() call).  Move BTMTKSDIO_FUNC_ENABLED bit-clear earlier
> so that txrx_work can see concurrent close().
>
> Fixes: 26270bc189ea4 ("Bluetooth: btmtksdio: move interrupt service to work")
> Cc: stable at vger.kernel.org
> Signed-off-by: Sergey Senozhatsky <senozhatsky at chromium.org>
> ---
>  drivers/bluetooth/btmtksdio.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/bluetooth/btmtksdio.c b/drivers/bluetooth/btmtksdio.c
> index d8c8d2857527..207d04cc2282 100644
> --- a/drivers/bluetooth/btmtksdio.c
> +++ b/drivers/bluetooth/btmtksdio.c
> @@ -625,7 +625,9 @@ static void btmtksdio_txrx_work(struct work_struct *work)
>         } while (int_status && time_is_after_jiffies(txrx_timeout));
>
>         /* Enable interrupt */
> -       if (bdev->func->irq_handler)
> +       if (bdev->func->irq_handler &&
> +           test_bit(BTMTKSDIO_FUNC_ENABLED, &bdev->tx_state) &&
> +           !test_bit(BTMTKSDIO_HW_RESET_ACTIVE, &bdev->tx_state))
>                 sdio_writel(bdev->func, C_INT_EN_SET, MTK_REG_CHLPCR, NULL);
>
>         sdio_release_host(bdev->func);
> @@ -741,6 +743,8 @@ static int btmtksdio_close(struct hci_dev *hdev)
>         if (!test_bit(BTMTKSDIO_FUNC_ENABLED, &bdev->tx_state))
>                 return 0;
>
> +       clear_bit(BTMTKSDIO_FUNC_ENABLED, &bdev->tx_state);
> +
>         sdio_claim_host(bdev->func);
>
>         /* Disable interrupt */
> @@ -748,11 +752,12 @@ static int btmtksdio_close(struct hci_dev *hdev)
>
>         sdio_release_irq(bdev->func);
>
> +       sdio_release_host(bdev->func);
>         cancel_work_sync(&bdev->txrx_work);
> +       sdio_claim_host(bdev->func);
>
>         btmtksdio_fw_pmctrl(bdev);
>
> -       clear_bit(BTMTKSDIO_FUNC_ENABLED, &bdev->tx_state);
>         sdio_disable_func(bdev->func);
>
>         sdio_release_host(bdev->func);
> @@ -1295,7 +1300,10 @@ static void btmtksdio_reset(struct hci_dev *hdev)
>
>         sdio_writel(bdev->func, C_INT_EN_CLR, MTK_REG_CHLPCR, NULL);
>         skb_queue_purge(&bdev->txq);
> +
> +       sdio_release_host(bdev->func);
>         cancel_work_sync(&bdev->txrx_work);
> +       sdio_claim_host(bdev->func);
>
>         gpiod_set_value_cansleep(bdev->reset, 1);
>         msleep(100);

The patch looks good to me. Inspired by your patch,
do you think should we add another patch to keep txrx_work out of the
reset window by rejecting TX during reset,
ignoring reset-time interrupts, and making queued workers exit early?

Some code like:

--- a/drivers/bluetooth/btmtksdio.c
+++ b/drivers/bluetooth/btmtksdio.c
@@ -567,6 +567,8 @@ static void btmtksdio_txrx_work(struct work_struct *work)
        pm_runtime_get_sync(bdev->dev);

        sdio_claim_host(bdev->func);
+       if (test_bit(BTMTKSDIO_HW_RESET_ACTIVE, &bdev->tx_state))
+               goto out;

        /* Disable interrupt */
        sdio_writel(bdev->func, C_INT_EN_CLR, MTK_REG_CHLPCR, NULL);
@@ -628,6 +630,7 @@ static void btmtksdio_txrx_work(struct work_struct *work)
            !test_bit(BTMTKSDIO_HW_RESET_ACTIVE, &bdev->tx_state))
                sdio_writel(bdev->func, C_INT_EN_SET, MTK_REG_CHLPCR, NULL);

+out:
        sdio_release_host(bdev->func);

        pm_runtime_put_autosuspend(bdev->dev);
@@ -646,6 +649,9 @@ static void btmtksdio_interrupt(struct sdio_func *func)
        /* Disable interrupt */
        sdio_writel(bdev->func, C_INT_EN_CLR, MTK_REG_CHLPCR, NULL);

+       if (test_bit(BTMTKSDIO_HW_RESET_ACTIVE, &bdev->tx_state))
+               return;
+
        schedule_work(&bdev->txrx_work);
 }

@@ -1250,6 +1256,9 @@ static int btmtksdio_send_frame(struct hci_dev
*hdev, struct sk_buff *skb)
 {
        struct btmtksdio_dev *bdev = hci_get_drvdata(hdev);

+       if (test_bit(BTMTKSDIO_HW_RESET_ACTIVE, &bdev->tx_state))
+               return -EBUSY;
+
        switch (hci_skb_pkt_type(skb)) {
        case HCI_COMMAND_PKT:
                hdev->stat.cmd_tx++;

> --
> 2.54.0.1136.gdb2ca164c4-goog
>
>