Dear Kiran,
Thank you for your reply.
Am 08.07.25 um 14:30 schrieb K, Kiran:
Subject: Re: [PATCH v1 2/2] Bluetooth: btintel_pcie: Fix alive context state handling
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.)
Ack. Unfortunately 'interrpt' was missing in my codespell
dictionary. I have updated the same. Thanks.
hunspell or a/ispell should find it by defautl.
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?
Yes. Issue was reproduced in stress reboot scenario although reproduction rate is less - 1/25.
It’d be great if you added that to the commit message, and what
scripts/commands you run for this stress reboot scenario.
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.
Ack.
There is one more comment below.
---
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
