Re: [PATCH v2 2/2] Bluetooth: btintel_pcie: Fix Alive Context State Handling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Kiran,

On Mon, Jul 14, 2025 at 10:36 PM Kiran K <kiran.k@xxxxxxxxx> wrote:
>
> The firmware raises an alive interrupt upon sending the HCI_RESET or
> BTINTEL_HCI_OP_RESET command. As part of handling the reset command,
> firmware initializes the hardware and data path and raises the alive
> interrupt. Upon receiving the alive interrupt, the driver must enable
> the data path and grant RX buffers to the firmware before sending any
> commands.
>
> The alive context maintained in the driver must be updated before
> sending BTINTEL_HCI_OP_RESET or HCI_OP_RESET to prevent a potential race
> condition where the context is also updated in the threaded IRQ.
>
> The issue was observed in a stress reboot usecase (1/25) using "sudo
> reboot" command where the firmware download was failing as the driver
> was not granting RX buffer to firmware due to race condition.
>
> Bluetooth: hci0: API lock is disabled
> Bluetooth: hci0: Debug lock is disabled
> Bluetooth: hci0: Minimum firmware build 1 week 10 2014
> Bluetooth: hci0: Bootloader timestamp 2023.43 buildtype 1 build 11631
> Bluetooth: hci0: Found device firmware: intel/ibt-00a0-00a1-iml.sfi
> Bluetooth: hci0: Boot Address: 0xb0301000
> Bluetooth: hci0: Firmware Version: 167-12.25
> Bluetooth: hci0: Waiting for firmware download to complete
> Bluetooth: hci0: Firmware loaded in 99902 usecs
> Bluetooth: hci0: Alive context: fw_dl old_boot_stage: 0xa0db0003
>            new_boot_stage: 0xa0db0003
> Bluetooth: hci0: sent cmd: 0xfc01 alive context changed:
>            fw_dl  ->  intel_reset1
> Bluetooth: hci0: Waiting for device to boot
> Bluetooth: hci0: Device boot timeout
> Bluetooth: hci0: Firmware download retry count: 1
>
> Fixes: 05c200c8f029 ("Bluetooth: btintel_pcie: Add handshake between driver and firmware")
> Signed-off-by: Kiran K <kiran.k@xxxxxxxxx>
> Signed-off-by: Sai Teja Aluvala <aluvala.sai.teja@xxxxxxxxx>
> ---
>  drivers/bluetooth/btintel_pcie.c | 28 +++++++++++++++-------------
>  1 file changed, 15 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
> index a17c438784ae..6680ef3207ed 100644
> --- a/drivers/bluetooth/btintel_pcie.c
> +++ b/drivers/bluetooth/btintel_pcie.c
> @@ -1988,12 +1988,6 @@ static int btintel_pcie_send_frame(struct hci_dev *hdev,
>                                 btintel_pcie_inject_cmd_complete(hdev, opcode);
>                 }
>
> -               /* Firmware raises alive interrupt on HCI_OP_RESET or
> -                * BTINTEL_HCI_OP_RESET
> -                */
> -               if (opcode == HCI_OP_RESET || opcode == BTINTEL_HCI_OP_RESET)
> -                       data->gp0_received = false;
> -
>                 hdev->stat.cmd_tx++;
>                 break;
>         case HCI_ACLDATA_PKT:
> @@ -2014,6 +2008,21 @@ static int btintel_pcie_send_frame(struct hci_dev *hdev,
>         memcpy(skb_push(skb, BTINTEL_PCIE_HCI_TYPE_LEN), &type,
>                BTINTEL_PCIE_HCI_TYPE_LEN);
>
> +       if (type == BTINTEL_PCIE_HCI_CMD_PKT &&
> +           (opcode == HCI_OP_RESET || opcode == BTINTEL_HCI_OP_RESET)) {
> +               /* Firmware raises alive interrupt on HCI_OP_RESET or
> +                * BTINTEL_HCI_OP_RESET
> +                */
> +               data->gp0_received = false;
> +               old_ctxt = data->alive_intr_ctxt;
> +               data->alive_intr_ctxt =
> +                       (opcode == 0xfc01 ? BTINTEL_PCIE_INTEL_HCI_RESET1 :
> +                                       BTINTEL_PCIE_HCI_RESET);
> +               bt_dev_dbg(data->hdev, "sending cmd: 0x%4.4x alive context changed: %s -> %s",
> +                               opcode, btintel_pcie_alivectxt_state2str(old_ctxt),
> +                               btintel_pcie_alivectxt_state2str(data->alive_intr_ctxt));
> +       }
> +
>         ret = btintel_pcie_send_sync(data, skb);
>         if (ret) {
>                 hdev->stat.err_tx++;
> @@ -2023,13 +2032,6 @@ static int btintel_pcie_send_frame(struct hci_dev *hdev,
>
>         if (type == BTINTEL_PCIE_HCI_CMD_PKT &&
>             (opcode == HCI_OP_RESET || opcode == BTINTEL_HCI_OP_RESET)) {
> -               old_ctxt = data->alive_intr_ctxt;
> -               data->alive_intr_ctxt =
> -                       (opcode == BTINTEL_HCI_OP_RESET ? BTINTEL_PCIE_INTEL_HCI_RESET1 :
> -                               BTINTEL_PCIE_HCI_RESET);
> -               bt_dev_dbg(data->hdev, "sent cmd: 0x%4.4x alive context changed: %s  ->  %s",
> -                          opcode, btintel_pcie_alivectxt_state2str(old_ctxt),
> -                          btintel_pcie_alivectxt_state2str(data->alive_intr_ctxt));
>                 ret = wait_event_timeout(data->gp0_wait_q,
>                                          data->gp0_received,
>                                          msecs_to_jiffies(BTINTEL_DEFAULT_INTR_TIMEOUT_MS));

Looks like the comment about moving every wait_event_timeout into
btintel_pcie_send_sync has not been addressed, or there is a valid
reason why we are handling this as post processing of
btintel_pcie_send_sync?

> --
> 2.43.0
>
>


-- 
Luiz Augusto von Dentz





[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux