public inbox for linux-bluetooth@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/2] Bluetooth: btintel_pcie: Make driver wait for alive interrupt
@ 2025-07-15  2:52 Kiran K
  2025-07-15  2:52 ` [PATCH v2 2/2] Bluetooth: btintel_pcie: Fix Alive Context State Handling Kiran K
  2025-07-15  3:34 ` [v2,1/2] Bluetooth: btintel_pcie: Make driver wait for alive interrupt bluez.test.bot
  0 siblings, 2 replies; 5+ messages in thread
From: Kiran K @ 2025-07-15  2:52 UTC (permalink / raw)
  To: linux-bluetooth
  Cc: ravishankar.srivatsa, chethan.tumkur.narayan,
	chandrashekar.devegowda, aluvala.sai.teja, Kiran K

The firmware raises an alive interrupt upon receiving the HCI_RESET or
BTINTEL_HCI_OP_RESET (Intel reset - 0xfc01) command. This change fixes
the driver to properly wait for the alive interrupt to avoid driver
sending commands to firmware before it is ready to process.

For details on the handshake between the driver and firmware, refer to
commit 05c200c8f029 ("Bluetooth: btintel_pcie: Add handshake between
driver and firmware").

As the driver needs to handle two interrupts for HCI_OP_RESET and
BTINTEL_HCI_OP_RESET, the firmware ensures that the TX completion
interrupt is always followed by the alive interrupt.

Fixes: 05c200c8f029 ("Bluetooth: btintel_pcie: Add handshake between driver and firmware")
Signed-off-by: Sai Teja Aluvala <aluvala.sai.teja@intel.com>
Signed-off-by: Kiran K <kiran.k@intel.com>
---
 drivers/bluetooth/btintel_pcie.c | 30 +++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
index 60528bdc4316..a17c438784ae 100644
--- a/drivers/bluetooth/btintel_pcie.c
+++ b/drivers/bluetooth/btintel_pcie.c
@@ -947,11 +947,13 @@ static void btintel_pcie_msix_gp0_handler(struct btintel_pcie_data *data)
 	case BTINTEL_PCIE_INTEL_HCI_RESET1:
 		if (btintel_pcie_in_op(data)) {
 			submit_rx = true;
+			signal_waitq = true;
 			break;
 		}
 
 		if (btintel_pcie_in_iml(data)) {
 			submit_rx = true;
+			signal_waitq = true;
 			data->alive_intr_ctxt = BTINTEL_PCIE_FW_DL;
 			break;
 		}
@@ -1985,8 +1987,11 @@ static int btintel_pcie_send_frame(struct hci_dev *hdev,
 			if (opcode == BTINTEL_HCI_OP_RESET)
 				btintel_pcie_inject_cmd_complete(hdev, opcode);
 		}
-		/* Firmware raises alive interrupt on HCI_OP_RESET */
-		if (opcode == HCI_OP_RESET)
+
+		/* 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++;
@@ -2025,17 +2030,16 @@ static int btintel_pcie_send_frame(struct hci_dev *hdev,
 		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));
-		if (opcode == HCI_OP_RESET) {
-			ret = wait_event_timeout(data->gp0_wait_q,
-						 data->gp0_received,
-						 msecs_to_jiffies(BTINTEL_DEFAULT_INTR_TIMEOUT_MS));
-			if (!ret) {
-				hdev->stat.err_tx++;
-				bt_dev_err(hdev, "No alive interrupt received for %s",
-					   btintel_pcie_alivectxt_state2str(data->alive_intr_ctxt));
-				ret = -ETIME;
-				goto exit_error;
-			}
+		ret = wait_event_timeout(data->gp0_wait_q,
+					 data->gp0_received,
+					 msecs_to_jiffies(BTINTEL_DEFAULT_INTR_TIMEOUT_MS));
+		if (!ret) {
+			hdev->stat.err_tx++;
+			bt_dev_err(hdev, "Timeout on alive interrupt (%u ms). Alive context: %s",
+				   BTINTEL_DEFAULT_INTR_TIMEOUT_MS,
+				   btintel_pcie_alivectxt_state2str(data->alive_intr_ctxt));
+			ret = -ETIME;
+			goto exit_error;
 		}
 	}
 	hdev->stat.byte_tx += skb->len;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v2 2/2] Bluetooth: btintel_pcie: Fix Alive Context State Handling
  2025-07-15  2:52 [PATCH v2 1/2] Bluetooth: btintel_pcie: Make driver wait for alive interrupt Kiran K
@ 2025-07-15  2:52 ` Kiran K
  2025-07-16 18:57   ` Luiz Augusto von Dentz
  2025-07-15  3:34 ` [v2,1/2] Bluetooth: btintel_pcie: Make driver wait for alive interrupt bluez.test.bot
  1 sibling, 1 reply; 5+ messages in thread
From: Kiran K @ 2025-07-15  2:52 UTC (permalink / raw)
  To: linux-bluetooth
  Cc: ravishankar.srivatsa, chethan.tumkur.narayan,
	chandrashekar.devegowda, aluvala.sai.teja, Kiran K

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@intel.com>
Signed-off-by: Sai Teja Aluvala <aluvala.sai.teja@intel.com>
---
 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));
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* RE: [v2,1/2] Bluetooth: btintel_pcie: Make driver wait for alive interrupt
  2025-07-15  2:52 [PATCH v2 1/2] Bluetooth: btintel_pcie: Make driver wait for alive interrupt Kiran K
  2025-07-15  2:52 ` [PATCH v2 2/2] Bluetooth: btintel_pcie: Fix Alive Context State Handling Kiran K
@ 2025-07-15  3:34 ` bluez.test.bot
  1 sibling, 0 replies; 5+ messages in thread
From: bluez.test.bot @ 2025-07-15  3:34 UTC (permalink / raw)
  To: linux-bluetooth, kiran.k

[-- Attachment #1: Type: text/plain, Size: 7371 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=982290

---Test result---

Test Summary:
CheckPatch                    PENDING   0.29 seconds
GitLint                       PENDING   0.21 seconds
SubjectPrefix                 PASS      0.26 seconds
BuildKernel                   FAIL      18.37 seconds
CheckAllWarning               FAIL      20.70 seconds
CheckSparse                   FAIL      22.66 seconds
BuildKernel32                 FAIL      18.33 seconds
TestRunnerSetup               PASS      464.45 seconds
TestRunner_l2cap-tester       PASS      25.06 seconds
TestRunner_iso-tester         PASS      36.35 seconds
TestRunner_bnep-tester        PASS      5.99 seconds
TestRunner_mgmt-tester        FAIL      129.33 seconds
TestRunner_rfcomm-tester      PASS      9.46 seconds
TestRunner_sco-tester         PASS      14.85 seconds
TestRunner_ioctl-tester       PASS      9.92 seconds
TestRunner_mesh-tester        FAIL      13.45 seconds
TestRunner_smp-tester         PASS      8.55 seconds
TestRunner_userchan-tester    PASS      6.32 seconds
IncrementalBuild              PENDING   0.53 seconds

Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:

##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:

##############################
Test: BuildKernel - FAIL
Desc: Build Kernel for Bluetooth
Output:

drivers/bluetooth/hci_ll.c: In function ‘ll_setup’:
drivers/bluetooth/hci_ll.c:652:46: error: ‘struct hci_dev’ has no member named ‘quirks’
  652 |   set_bit(HCI_QUIRK_INVALID_BDADDR, &hu->hdev->quirks);
      |                                              ^~
drivers/bluetooth/hci_ll.c:656:47: error: ‘struct hci_dev’ has no member named ‘quirks’
  656 |    set_bit(HCI_QUIRK_INVALID_BDADDR, &hu->hdev->quirks);
      |                                               ^~
make[4]: *** [scripts/Makefile.build:203: drivers/bluetooth/hci_ll.o] Error 1
make[3]: *** [scripts/Makefile.build:461: drivers/bluetooth] Error 2
make[2]: *** [scripts/Makefile.build:461: drivers] Error 2
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/github/workspace/src/src/Makefile:2003: .] Error 2
make: *** [Makefile:248: __sub-make] Error 2
##############################
Test: CheckAllWarning - FAIL
Desc: Run linux kernel with all warning enabled
Output:

drivers/bluetooth/hci_ll.c: In function ‘ll_setup’:
drivers/bluetooth/hci_ll.c:652:46: error: ‘struct hci_dev’ has no member named ‘quirks’
  652 |   set_bit(HCI_QUIRK_INVALID_BDADDR, &hu->hdev->quirks);
      |                                              ^~
drivers/bluetooth/hci_ll.c:656:47: error: ‘struct hci_dev’ has no member named ‘quirks’
  656 |    set_bit(HCI_QUIRK_INVALID_BDADDR, &hu->hdev->quirks);
      |                                               ^~
make[4]: *** [scripts/Makefile.build:203: drivers/bluetooth/hci_ll.o] Error 1
make[4]: *** Waiting for unfinished jobs....
make[3]: *** [scripts/Makefile.build:461: drivers/bluetooth] Error 2
make[2]: *** [scripts/Makefile.build:461: drivers] Error 2
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/github/workspace/src/src/Makefile:2003: .] Error 2
make: *** [Makefile:248: __sub-make] Error 2
##############################
Test: CheckSparse - FAIL
Desc: Run sparse tool with linux kernel
Output:

net/bluetooth/af_bluetooth.c:248:25: warning: context imbalance in 'bt_accept_enqueue' - different lock contexts for basic block
net/bluetooth/hci_core.c:85:9: warning: context imbalance in '__hci_dev_get' - different lock contexts for basic block
net/bluetooth/hci_core.c: note: in included file (through include/linux/notifier.h, include/linux/memory_hotplug.h, include/linux/mmzone.h, include/linux/gfp.h, include/linux/xarray.h, include/linux/radix-tree.h, ...):
./include/linux/srcu.h:400:9: warning: context imbalance in 'hci_dev_put_srcu' - unexpected unlock
drivers/bluetooth/hci_ll.c: In function ‘ll_setup’:
drivers/bluetooth/hci_ll.c:652:46: error: ‘struct hci_dev’ has no member named ‘quirks’
  652 |   set_bit(HCI_QUIRK_INVALID_BDADDR, &hu->hdev->quirks);
      |                                              ^~
drivers/bluetooth/hci_ll.c:656:47: error: ‘struct hci_dev’ has no member named ‘quirks’
  656 |    set_bit(HCI_QUIRK_INVALID_BDADDR, &hu->hdev->quirks);
      |                                               ^~
make[4]: *** [scripts/Makefile.build:203: drivers/bluetooth/hci_ll.o] Error 1
make[4]: *** Waiting for unfinished jobs....
make[3]: *** [scripts/Makefile.build:461: drivers/bluetooth] Error 2
make[2]: *** [scripts/Makefile.build:461: drivers] Error 2
make[2]: *** Waiting for unfinished jobs....
net/bluetooth/hci_event.c: note: in included file (through include/net/bluetooth/hci_core.h):
./include/net/bluetooth/hci.h:2655:47: warning: array of flexible structures
./include/net/bluetooth/hci.h:2741:43: warning: array of flexible structures
net/bluetooth/hci_codec.c: note: in included file:
./include/net/bluetooth/hci_core.h:149:35: warning: array of flexible structures
net/bluetooth/sco.c: note: in included file:
./include/net/bluetooth/hci_core.h:149:35: warning: array of flexible structures
make[1]: *** [/github/workspace/src/src/Makefile:2003: .] Error 2
make: *** [Makefile:248: __sub-make] Error 2
##############################
Test: BuildKernel32 - FAIL
Desc: Build 32bit Kernel for Bluetooth
Output:

drivers/bluetooth/hci_ll.c: In function ‘ll_setup’:
drivers/bluetooth/hci_ll.c:652:46: error: ‘struct hci_dev’ has no member named ‘quirks’
  652 |   set_bit(HCI_QUIRK_INVALID_BDADDR, &hu->hdev->quirks);
      |                                              ^~
drivers/bluetooth/hci_ll.c:656:47: error: ‘struct hci_dev’ has no member named ‘quirks’
  656 |    set_bit(HCI_QUIRK_INVALID_BDADDR, &hu->hdev->quirks);
      |                                               ^~
make[4]: *** [scripts/Makefile.build:203: drivers/bluetooth/hci_ll.o] Error 1
make[3]: *** [scripts/Makefile.build:461: drivers/bluetooth] Error 2
make[2]: *** [scripts/Makefile.build:461: drivers] Error 2
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/github/workspace/src/src/Makefile:2003: .] Error 2
make: *** [Makefile:248: __sub-make] Error 2
##############################
Test: TestRunner_mgmt-tester - FAIL
Desc: Run mgmt-tester with test-runner
Output:
Total: 490, Passed: 484 (98.8%), Failed: 2, Not Run: 4

Failed Test Cases
LL Privacy - Add Device 3 (AL is full)               Failed       0.216 seconds
LL Privacy - Set Flags 3 (2 Devices to RL)           Failed       0.211 seconds
##############################
Test: TestRunner_mesh-tester - FAIL
Desc: Run mesh-tester with test-runner
Output:
Total: 10, Passed: 8 (80.0%), Failed: 2, Not Run: 0

Failed Test Cases
Mesh - Send cancel - 1                               Timed out    2.147 seconds
Mesh - Send cancel - 2                               Timed out    1.996 seconds
##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:



---
Regards,
Linux Bluetooth


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 2/2] Bluetooth: btintel_pcie: Fix Alive Context State Handling
  2025-07-15  2:52 ` [PATCH v2 2/2] Bluetooth: btintel_pcie: Fix Alive Context State Handling Kiran K
@ 2025-07-16 18:57   ` Luiz Augusto von Dentz
  2025-07-21  9:03     ` K, Kiran
  0 siblings, 1 reply; 5+ messages in thread
From: Luiz Augusto von Dentz @ 2025-07-16 18:57 UTC (permalink / raw)
  To: Kiran K
  Cc: linux-bluetooth, ravishankar.srivatsa, chethan.tumkur.narayan,
	chandrashekar.devegowda, aluvala.sai.teja

Hi Kiran,

On Mon, Jul 14, 2025 at 10:36 PM Kiran K <kiran.k@intel.com> 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@intel.com>
> Signed-off-by: Sai Teja Aluvala <aluvala.sai.teja@intel.com>
> ---
>  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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH v2 2/2] Bluetooth: btintel_pcie: Fix Alive Context State Handling
  2025-07-16 18:57   ` Luiz Augusto von Dentz
@ 2025-07-21  9:03     ` K, Kiran
  0 siblings, 0 replies; 5+ messages in thread
From: K, Kiran @ 2025-07-21  9:03 UTC (permalink / raw)
  To: Luiz Augusto von Dentz
  Cc: linux-bluetooth@vger.kernel.org, Srivatsa, Ravishankar,
	Tumkur Narayan, Chethan, Devegowda, Chandrashekar,
	Aluvala Sai Teja

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@intel.com> 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@intel.com>
>> Signed-off-by: Sai Teja Aluvala <aluvala.sai.teja@intel.com>
>> ---
>>  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


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-07-21  9:04 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-15  2:52 [PATCH v2 1/2] Bluetooth: btintel_pcie: Make driver wait for alive interrupt Kiran K
2025-07-15  2:52 ` [PATCH v2 2/2] Bluetooth: btintel_pcie: Fix Alive Context State Handling Kiran K
2025-07-16 18:57   ` Luiz Augusto von Dentz
2025-07-21  9:03     ` K, Kiran
2025-07-15  3:34 ` [v2,1/2] Bluetooth: btintel_pcie: Make driver wait for alive interrupt bluez.test.bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox