Hi Luiz,
>Subject: Re: [PATCH v2 2/2] Bluetooth: btintel_pcie: Fix Alive Context State
>Handling
>
>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?
Ack. I agree with your comment. It makes logical sense to move wait_event_timeout() call for alive interrupt inside btintel_pcie_send_sync().
>
>> --
>> 2.43.0
>>
>>
>
>
>--
>Luiz Augusto von Dentz
Thanks,
Kiran
