Dear Kiran, dear Sai,
Thank you for the patch.
Am 07.07.25 um 05:46 schrieb Kiran K:
Firmware raises alive interrpt on sending 0xfc01 command. Alive context
interr*u*pt
(I would have hoped, two developers would spot such a typo, a spell
checker also highlights.)
maintained in driver needs to be updated before sending 0xfc01 (Intel
Reset) or 0x03c0 (HCI Reset) to avoid the potential race condition where
the context is also updated in threaded irq.
Do you have a reproducer for the issue?
Signed-off-by: Kiran K <kiran.k@xxxxxxxxx>
Signed-off-by: Sai Teja Aluvala <aluvala.sai.teja@xxxxxxxxx>
Fixes: 05c200c8f029 ("Bluetooth: btintel_pcie: Add handshake between driver and firmware")
I’d add the Fixes tag before the Signed-off-by lines.
---
drivers/bluetooth/btintel_pcie.c | 25 ++++++++++++++-----------
1 file changed, 14 insertions(+), 11 deletions(-)
diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
index f893ad6fc87a..d29103b102e4 100644
--- a/drivers/bluetooth/btintel_pcie.c
+++ b/drivers/bluetooth/btintel_pcie.c
@@ -1988,10 +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 0xfc01*/
- if (opcode == HCI_OP_RESET || opcode == 0xfc01)
- data->gp0_received = false;
-
hdev->stat.cmd_tx++;
break;
case HCI_ACLDATA_PKT:
@@ -2012,6 +2008,20 @@ 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) {
+ /* Firmware raises alive interrupt on HCI_OP_RESET or 0xfc01*/
+ if (opcode == HCI_OP_RESET || opcode == 0xfc01) {
Why not keep the form of just one if statement with && in the condition?
as below?
+ 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++;
@@ -2021,13 +2031,6 @@ static int btintel_pcie_send_frame(struct hci_dev *hdev,
if (type == BTINTEL_PCIE_HCI_CMD_PKT &&
(opcode == HCI_OP_RESET || opcode == 0xfc01)) {
- 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, "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));
Kind regards,
Paul
