[PATCH 0/2] Bluetooth: btintel_pcie: two PM fixes, assembled as one series

[PATCH 0/2] Bluetooth: btintel_pcie: two PM fixes, assembled as one series

[Sergey Lebedev]

Two patches already on this list fix different halves of the same fault, and
each leaves a real failure behind when applied alone. This assembles them into
one series. I am the submitter only - authorship, Fixes: tags and existing
trailers are unchanged.

  1/2  Vladimir V. Kondratyev  - re-read BOOT_STAGE_REG before the fallback
                                 check, so a missed alive interrupt is
                                 survivable
  2/2  Ravindra (Intel)        - fix the PM flow for S0ix, S3 and S4, which
                                 among other things stops .thaw running an FLR

Both carry Fixes: e57362f4911b.

How this came about, so nobody has to take my word for it. Ravindra agreed to
the assembly on 2026-09-08, on the condition that authorship and the Fixes:
tags be preserved and that his patch be rebased on top of Vladimir's; both are
done:

  https://lore.kernel.org/linux-bluetooth/IA1PR11MB786922E0DBE2CDC8FB00C5B6DAB12@IA1PR11MB7869.namprd11.prod.outlook.com/

Vladimir has not replied to the message that said this would go out unless he
objected, and I am not treating that silence as agreement. Vladimir - it is
your patch. If you would rather post the series yourself, or not at all, say
so and I will drop this:

  https://lore.kernel.org/linux-bluetooth/20260908102758.72135-1-lsa.uz@pm.me/

Why one series
==============

They were posted separately and read as alternatives. They are not. The
evidence now comes from two machines, two controller generations, and two
distinct faults:

Surface Pro 11, Lunar Lake, BE201 8086:a876, s2idle. With a fixture that
drops the alive interrupt on demand, Ravindra's change alone leaves the
missed-interrupt case failing exactly as unpatched; Vladimir's re-read fixes
it; together they do not interfere. The same failure was also caught
spontaneously with injection disabled, one run in six.

  https://lore.kernel.org/linux-bluetooth/20260902133836.11786-1-lsa.uz@pm.me/

ThinkPad X9-15p, Panther Lake 8086:e476, hibernation. Ferenc Lengyel reported
hibernation aborting with -EBUSY after the image was already written, and
tested both patches:

  stock                    3 aborts in 10 cycles
  1/2 alone                2 aborts in 5
  1/2 + 2/2                0 aborts in 10

He then instrumented 1/2's re-read and found why 1/2 alone is not enough on
his machine: the register honestly reports the controller is not in D3,
because .thaw has just run an FLR that dropped it to ROM, and firmware has
not reloaded by the time .poweroff asks for D3 some nine seconds later. 2/2
removes that FLR from .thaw and keeps it on .restore. His own caveat, which
he states himself: ten clean cycles against a roughly one-in-three prior
failure rate is Fisher p ~ 0.06 - consistent and matching the mechanism,
not a large sample.

  https://lore.kernel.org/linux-bluetooth/df180a89-b214-41b4-b8ce-c6ed6b12372f@lengyelf.eu/

What the rebase changed
=======================

Nothing but context. 2/2 needed one hand adjustment: its header hunk removes
u8 pm_sx_event from struct btintel_pcie_data, and bluetooth-next has since
gained struct btintel_pcie_mdbgc mdbgc between dbgc and dmp_hdr, so the
three-line context no longer matched. The resulting diffstat is identical to
Ravindra's posting - 64 lines in the .c, 2 in the .h, 44 insertions and 22
deletions - and no reference to pm_sx_event is left anywhere in
drivers/bluetooth.

Built against bluetooth-next at 701ca7188 with W=1: no warnings from either
file. The code is otherwise byte-for-byte what was tested on both machines.

One thing for Paul: you gave a Reviewed-by on v3 of 1/2 at 14:36 UTC on
2026-09-03 and v4 went out at 19:22 without carrying it. I have not added it
back, since it is not mine to move, but you may want to re-give it here.

Originals:
  1/2  https://lore.kernel.org/linux-bluetooth/20260903192245.135310-2-vladimirkondratyev2@gmail.com/
  2/2  https://lore.kernel.org/linux-bluetooth/20260902042840.2432862-1-ravindra@intel.com/

Ravindra (1):
  Bluetooth: btintel_pcie: fix PM flow for S0ix, S3 and S4

Vladimir V. Kondratyev (1):
  Bluetooth: btintel_pcie: fix stale cache in set_dxstate fallback check

 drivers/bluetooth/btintel_pcie.c | 72 ++++++++++++++++++++++----------
 drivers/bluetooth/btintel_pcie.h |  3 +-
 2 files changed, 50 insertions(+), 25 deletions(-)

-- 
2.50.1 (Apple Git-155)

[PATCH 1/2] Bluetooth: btintel_pcie: fix stale cache in set_dxstate fallback check

[Sergey Lebedev]

From: Vladimir V. Kondratyev <vladimirkondratyev2@gmail.com>

btintel_pcie returns -16 (EBUSY) during suspend, causing the entire
suspend operation to abort on Intel Lunar Lake hardware. The system
immediately resumes
after every suspend attempt:
  Bluetooth: hci0: Timeout (200 ms) on alive interrupt for D2 entry,
retry count 0
  Bluetooth: hci0: Timeout (200 ms) on alive interrupt for D2 entry,
retry count 1
  Bluetooth: hci0: Timeout (200 ms) on alive interrupt for D2 entry,
retry count 2
  btintel_pcie 0000:00:14.7: PM: pci_pm_suspend(): btintel_pcie_suspend
[btintel_pcie] returns -16
  btintel_pcie 0000:00:14.7: PM: dpm_run_callback(): pci_pm_suspend
returns -16
  btintel_pcie 0000:00:14.7: PM: failed to suspend async: error -16
  PM: Some devices failed to suspend, or early wake event detected

btintel_pcie_set_dxstate() falls back to checking the controller state via
btintel_pcie_in_d3/d0() when the alive interrupt is missed. However, these
helpers read boot_stage_cache, which is only updated by the interrupt
handler. As such, if the interrupt was missed, the cache is stale and the
fallback check always fails, exhausting all retries and returning -EBUSY,
causing suspend to abort.

The fix involves re-reading the hardware register before the fallback state
check, consistent with btintel_pcie_resume().

Fixes: e57362f4911b ("Bluetooth: btintel_pcie: Add support for _suspend() / _resume()")
Link: https://bugzilla.kernel.org/show_bug.cgi?id=221481
Link: https://lore.kernel.org/linux-bluetooth/20260830151550.44687-1-lsa.uz@pm.me/
Signed-off-by: Vladimir V. Kondratyev <vladimirkondratyev2@gmail.com>
Tested-by: Sergey Lebedev <lsa.uz@pm.me>

Signed-off-by: Sergey Lebedev <lsa.uz@pm.me>
---
 drivers/bluetooth/btintel_pcie.c | 8 +++++---
 drivers/bluetooth/btintel_pcie.h | 1 +
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
index 7a2139a04..361c550b5 100644
--- a/drivers/bluetooth/btintel_pcie.c
+++ b/drivers/bluetooth/btintel_pcie.c
@@ -4191,10 +4191,12 @@ static int btintel_pcie_set_dxstate(struct btintel_pcie_data *data, u32 dxstate)
 					  BTINTEL_PCIE_CSR_MSIX_HW_INT_CAUSES,
 					  BTINTEL_PCIE_MSIX_HW_INT_CAUSES_GP0);
 
-		/* A hardware bug may cause the alive interrupt to be missed.
-		 * Check if the controller reached the expected state and retry
-		 * the operation only if it hasn't.
+		/* A hardware bug may cause the alive interrupt to be missed. Refresh
+		 * boot_stage_cache from hardware, since only the interrupt handler
+		 * updates it. Finally retry only if the state check still fails.
 		 */
+		data->boot_stage_cache = btintel_pcie_rd_reg32(data,
+							       BTINTEL_PCIE_CSR_BOOT_STAGE_REG);
 		if (dxstate == BTINTEL_PCIE_STATE_D0) {
 			if (btintel_pcie_in_d0(data))
 				return 0;
diff --git a/drivers/bluetooth/btintel_pcie.h b/drivers/bluetooth/btintel_pcie.h
index f35f80f80..016795fcb 100644
--- a/drivers/bluetooth/btintel_pcie.h
+++ b/drivers/bluetooth/btintel_pcie.h
@@ -51,6 +51,7 @@
 #define BTINTEL_PCIE_CSR_BOOT_STAGE_DEVICE_HALTED	(BIT(14))
 #define BTINTEL_PCIE_CSR_BOOT_STAGE_MAC_ACCESS_ON	(BIT(16))
 #define BTINTEL_PCIE_CSR_BOOT_STAGE_ALIVE		(BIT(23))
+/* Reflects live D-state. Updated by hardware on every D-state transition. */
 #define BTINTEL_PCIE_CSR_BOOT_STAGE_D3_STATE_READY	(BIT(24))
 
 #define BTINTEL_PCIE_CSR_DOORBELL_MBOX_READ_CONFIRM	(BIT(4))
-- 
2.50.1 (Apple Git-155)

[PATCH 2/2] Bluetooth: btintel_pcie: fix PM flow for S0ix, S3 and S4

[Sergey Lebedev]

From: Ravindra <ravindra@intel.com>

Use pm_suspend_target_state to differentiate S0ix from S3/S4. Set the
controller to D3_HOT for S0ix (PM_SUSPEND_TO_IDLE) and D3_COLD for
S3/S4, freeze and hibernate to prevent post-resume instability.

Register .freeze, .thaw, .poweroff and .restore callbacks for proper
hibernation support. The freeze and poweroff (hibernate) paths set
D3_COLD via btintel_pcie_suspend_late. The thaw path resumes with a
normal D0 transition, while .restore forces FLR-based firmware recovery
after S4 since power is lost. S3 (PM_SUSPEND_MEM) resume also triggers
FLR. S0ix resumes via a normal D0 transition.

Fixes: e57362f4911b ("Bluetooth: btintel_pcie: Add support for _suspend() / _resume()")
Assisted-by: GitHub-Copilot:GPT5
Signed-off-by: Ravindra <ravindra@intel.com>
Assisted-by: Claude:claude-opus-5
Tested-by: Ferenc Lengyel <dev@lengyelf.eu>
Signed-off-by: Sergey Lebedev <lsa.uz@pm.me>
---
 drivers/bluetooth/btintel_pcie.c | 64 ++++++++++++++++++++++----------
 drivers/bluetooth/btintel_pcie.h |  2 -
 2 files changed, 44 insertions(+), 22 deletions(-)

diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
index 361c550b5..e37e710b9 100644
--- a/drivers/bluetooth/btintel_pcie.c
+++ b/drivers/bluetooth/btintel_pcie.c
@@ -16,6 +16,7 @@
 #include <linux/delay.h>
 #include <linux/interrupt.h>
 #include <linux/acpi.h>
+#include <linux/suspend.h>
 
 #include <linux/unaligned.h>
 #include <linux/devcoredump.h>
@@ -4168,11 +4169,16 @@ static void btintel_pcie_coredump(struct device *dev)
 
 static int btintel_pcie_set_dxstate(struct btintel_pcie_data *data, u32 dxstate)
 {
-	int retry = 0, status;
+	int retry = 0;
+	long status;
 	u32 dx_intr_timeout_ms = 200;
 
+	/* Not reset per retry: dxstate is unchanged, so a late interrupt from
+	 * an earlier attempt still confirms the target state.
+	 */
+	data->gp0_received = false;
+
 	do {
-		data->gp0_received = false;
 
 		btintel_pcie_wr_sleep_cntrl(data, dxstate);
 
@@ -4220,18 +4226,23 @@ static int btintel_pcie_suspend_late(struct device *dev, pm_message_t mesg)
 
 	data = pci_get_drvdata(pdev);
 
-	dxstate = (mesg.event == PM_EVENT_SUSPEND ?
-		   BTINTEL_PCIE_STATE_D3_HOT : BTINTEL_PCIE_STATE_D3_COLD);
-
-	data->pm_sx_event = mesg.event;
+	/* S0ix (s2idle) uses D3_HOT; S3, freeze and hibernate use D3_COLD. */
+	if (mesg.event == PM_EVENT_SUSPEND &&
+	    pm_suspend_target_state == PM_SUSPEND_TO_IDLE)
+		dxstate = BTINTEL_PCIE_STATE_D3_HOT;
+	else
+		dxstate = BTINTEL_PCIE_STATE_D3_COLD;
 
 	start = ktime_get();
 
 	/* Refer: 6.4.11.7 -> Platform power management */
 	err = btintel_pcie_set_dxstate(data, dxstate);
 
-	if (err)
+	if (err) {
+		bt_dev_err(data->hdev, "Failed to set dxstate:%u (%d)",
+			   dxstate, err);
 		return err;
+	}
 
 	bt_dev_dbg(data->hdev,
 		   "device entered into d3 state from d0 in %lld us",
@@ -4254,7 +4265,7 @@ static int btintel_pcie_freeze(struct device *dev)
 	return btintel_pcie_suspend_late(dev, PMSG_FREEZE);
 }
 
-static int btintel_pcie_resume(struct device *dev)
+static int btintel_pcie_resume_event(struct device *dev, pm_message_t mesg)
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
 	struct btintel_pcie_data *data;
@@ -4262,19 +4273,15 @@ static int btintel_pcie_resume(struct device *dev)
 	int err;
 
 	data = pci_get_drvdata(pdev);
-	data->gp0_received = false;
 
 	start = ktime_get();
 
-	/* When the system enters S4 (hibernate) mode, bluetooth device loses
-	 * power, which results in the erasure of its loaded firmware.
-	 * Consequently, function level reset (flr) is required on system
-	 * resume to bring the controller back into an operational state by
-	 * initiating a new firmware download.
+	/* S3 and S4 may cut power, erasing the firmware. Force FLR to recover
+	 * instead of a normal D0 transition.
 	 */
-
-	if (data->pm_sx_event == PM_EVENT_FREEZE ||
-	    data->pm_sx_event == PM_EVENT_HIBERNATE) {
+	if (mesg.event == PM_EVENT_RESTORE ||
+	    (mesg.event == PM_EVENT_RESUME &&
+	     pm_suspend_target_state == PM_SUSPEND_MEM)) {
 		set_bit(BTINTEL_PCIE_CORE_HALTED, &data->flags);
 		btintel_pcie_request_reset(data, BTINTEL_PCIE_IOSF_PRR_FLR);
 		return 0;
@@ -4283,7 +4290,9 @@ static int btintel_pcie_resume(struct device *dev)
 	/* Refer: 6.4.11.7 -> Platform power management */
 	err = btintel_pcie_set_dxstate(data, BTINTEL_PCIE_STATE_D0);
 
-	if (err == 0) {
+	if (err) {
+		bt_dev_err(data->hdev, "Failed to set D0 state (%d)", err);
+	} else {
 		bt_dev_dbg(data->hdev,
 			   "device entered into d0 state from d3 in %lld us",
 			   ktime_to_us(ktime_get() - start));
@@ -4308,13 +4317,28 @@ static int btintel_pcie_resume(struct device *dev)
 	return err;
 }
 
+static int btintel_pcie_resume(struct device *dev)
+{
+	return btintel_pcie_resume_event(dev, PMSG_RESUME);
+}
+
+static int btintel_pcie_restore(struct device *dev)
+{
+	return btintel_pcie_resume_event(dev, PMSG_RESTORE);
+}
+
+static int btintel_pcie_thaw(struct device *dev)
+{
+	return btintel_pcie_resume_event(dev, PMSG_THAW);
+}
+
 static const struct dev_pm_ops btintel_pcie_pm_ops = {
 	.suspend = btintel_pcie_suspend,
 	.resume = btintel_pcie_resume,
 	.freeze = btintel_pcie_freeze,
-	.thaw = btintel_pcie_resume,
+	.thaw = btintel_pcie_thaw,
 	.poweroff = btintel_pcie_hibernate,
-	.restore = btintel_pcie_resume,
+	.restore = btintel_pcie_restore,
 };
 
 static struct pci_driver btintel_pcie_driver = {
diff --git a/drivers/bluetooth/btintel_pcie.h b/drivers/bluetooth/btintel_pcie.h
index 016795fcb..3030b4e8b 100644
--- a/drivers/bluetooth/btintel_pcie.h
+++ b/drivers/bluetooth/btintel_pcie.h
@@ -713,7 +713,6 @@ struct btintel_pcie_ini_dump_info {
  * @txq: TX Queue struct
  * @rxq: RX Queue struct
  * @alive_intr_ctxt: Alive interrupt context
- * @pm_sx_event: PM event on which system got suspended
  */
 struct btintel_pcie_data {
 	struct pci_dev	*pdev;
@@ -773,7 +772,6 @@ struct btintel_pcie_data {
 	struct btintel_pcie_dbgc	dbgc;
 	struct btintel_pcie_mdbgc	mdbgc;
 	struct btintel_pcie_dump_header dmp_hdr;
-	u8	pm_sx_event;
 	u32	debug_evt_addr;
 	u32	debug_evt_size;
 	dma_addr_t	debug_table_addr;
-- 
2.50.1 (Apple Git-155)

RE: Bluetooth: btintel_pcie: two PM fixes, assembled as one series

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1158 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/series/1161304/

---Test result---

Test Summary:
CheckPatch                    PASS      1.88 seconds
VerifyFixes                   PASS      0.12 seconds
VerifySignedoff               PASS      0.12 seconds
GitLint                       PASS      0.64 seconds
SubjectPrefix                 PASS      0.23 seconds
BuildKernel                   PASS      27.09 seconds
CheckAllWarning               PASS      30.31 seconds
CheckSparse                   PASS      28.86 seconds
BuildKernel32                 PASS      26.31 seconds
CheckKernelLLVM               SKIP      0.00 seconds
TestRunnerSetup               PASS      479.13 seconds
IncrementalBuild              PASS      27.58 seconds

Details
##############################
Test: CheckKernelLLVM - SKIP
Desc: Build kernel with LLVM + context analysis
Output:
Clang not found


https://github.com/bluez/bluetooth-next/pull/728

---
Regards,
Linux Bluetooth

Re: [PATCH 2/2] Bluetooth: btintel_pcie: fix PM flow for S0ix, S3 and S4

[Luiz Augusto von Dentz]

Hi Sergey,

On Wed, Sep 9, 2026 at 8:34 AM Sergey Lebedev <lsa.uz@pm.me> wrote:
>
> From: Ravindra <ravindra@intel.com>
>
> Use pm_suspend_target_state to differentiate S0ix from S3/S4. Set the
> controller to D3_HOT for S0ix (PM_SUSPEND_TO_IDLE) and D3_COLD for
> S3/S4, freeze and hibernate to prevent post-resume instability.
>
> Register .freeze, .thaw, .poweroff and .restore callbacks for proper
> hibernation support. The freeze and poweroff (hibernate) paths set
> D3_COLD via btintel_pcie_suspend_late. The thaw path resumes with a
> normal D0 transition, while .restore forces FLR-based firmware recovery
> after S4 since power is lost. S3 (PM_SUSPEND_MEM) resume also triggers
> FLR. S0ix resumes via a normal D0 transition.
>
> Fixes: e57362f4911b ("Bluetooth: btintel_pcie: Add support for _suspend() / _resume()")
> Assisted-by: GitHub-Copilot:GPT5
> Signed-off-by: Ravindra <ravindra@intel.com>
> Assisted-by: Claude:claude-opus-5
> Tested-by: Ferenc Lengyel <dev@lengyelf.eu>
> Signed-off-by: Sergey Lebedev <lsa.uz@pm.me>
> ---
>  drivers/bluetooth/btintel_pcie.c | 64 ++++++++++++++++++++++----------
>  drivers/bluetooth/btintel_pcie.h |  2 -
>  2 files changed, 44 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
> index 361c550b5..e37e710b9 100644
> --- a/drivers/bluetooth/btintel_pcie.c
> +++ b/drivers/bluetooth/btintel_pcie.c
> @@ -16,6 +16,7 @@
>  #include <linux/delay.h>
>  #include <linux/interrupt.h>
>  #include <linux/acpi.h>
> +#include <linux/suspend.h>
>
>  #include <linux/unaligned.h>
>  #include <linux/devcoredump.h>
> @@ -4168,11 +4169,16 @@ static void btintel_pcie_coredump(struct device *dev)
>
>  static int btintel_pcie_set_dxstate(struct btintel_pcie_data *data, u32 dxstate)
>  {
> -       int retry = 0, status;
> +       int retry = 0;
> +       long status;
>         u32 dx_intr_timeout_ms = 200;
>
> +       /* Not reset per retry: dxstate is unchanged, so a late interrupt from
> +        * an earlier attempt still confirms the target state.
> +        */
> +       data->gp0_received = false;
> +
>         do {
> -               data->gp0_received = false;
>
>                 btintel_pcie_wr_sleep_cntrl(data, dxstate);
>
> @@ -4220,18 +4226,23 @@ static int btintel_pcie_suspend_late(struct device *dev, pm_message_t mesg)
>
>         data = pci_get_drvdata(pdev);
>
> -       dxstate = (mesg.event == PM_EVENT_SUSPEND ?
> -                  BTINTEL_PCIE_STATE_D3_HOT : BTINTEL_PCIE_STATE_D3_COLD);
> -
> -       data->pm_sx_event = mesg.event;
> +       /* S0ix (s2idle) uses D3_HOT; S3, freeze and hibernate use D3_COLD. */
> +       if (mesg.event == PM_EVENT_SUSPEND &&
> +           pm_suspend_target_state == PM_SUSPEND_TO_IDLE)
> +               dxstate = BTINTEL_PCIE_STATE_D3_HOT;
> +       else
> +               dxstate = BTINTEL_PCIE_STATE_D3_COLD;
>
>         start = ktime_get();
>
>         /* Refer: 6.4.11.7 -> Platform power management */
>         err = btintel_pcie_set_dxstate(data, dxstate);
>
> -       if (err)
> +       if (err) {
> +               bt_dev_err(data->hdev, "Failed to set dxstate:%u (%d)",
> +                          dxstate, err);
>                 return err;
> +       }
>
>         bt_dev_dbg(data->hdev,
>                    "device entered into d3 state from d0 in %lld us",
> @@ -4254,7 +4265,7 @@ static int btintel_pcie_freeze(struct device *dev)
>         return btintel_pcie_suspend_late(dev, PMSG_FREEZE);
>  }
>
> -static int btintel_pcie_resume(struct device *dev)
> +static int btintel_pcie_resume_event(struct device *dev, pm_message_t mesg)
>  {
>         struct pci_dev *pdev = to_pci_dev(dev);
>         struct btintel_pcie_data *data;
> @@ -4262,19 +4273,15 @@ static int btintel_pcie_resume(struct device *dev)
>         int err;
>
>         data = pci_get_drvdata(pdev);
> -       data->gp0_received = false;
>
>         start = ktime_get();
>
> -       /* When the system enters S4 (hibernate) mode, bluetooth device loses
> -        * power, which results in the erasure of its loaded firmware.
> -        * Consequently, function level reset (flr) is required on system
> -        * resume to bring the controller back into an operational state by
> -        * initiating a new firmware download.
> +       /* S3 and S4 may cut power, erasing the firmware. Force FLR to recover
> +        * instead of a normal D0 transition.
>          */
> -
> -       if (data->pm_sx_event == PM_EVENT_FREEZE ||
> -           data->pm_sx_event == PM_EVENT_HIBERNATE) {
> +       if (mesg.event == PM_EVENT_RESTORE ||
> +           (mesg.event == PM_EVENT_RESUME &&
> +            pm_suspend_target_state == PM_SUSPEND_MEM)) {
>                 set_bit(BTINTEL_PCIE_CORE_HALTED, &data->flags);
>                 btintel_pcie_request_reset(data, BTINTEL_PCIE_IOSF_PRR_FLR);
>                 return 0;
> @@ -4283,7 +4290,9 @@ static int btintel_pcie_resume(struct device *dev)
>         /* Refer: 6.4.11.7 -> Platform power management */
>         err = btintel_pcie_set_dxstate(data, BTINTEL_PCIE_STATE_D0);
>
> -       if (err == 0) {
> +       if (err) {
> +               bt_dev_err(data->hdev, "Failed to set D0 state (%d)", err);
> +       } else {
>                 bt_dev_dbg(data->hdev,
>                            "device entered into d0 state from d3 in %lld us",
>                            ktime_to_us(ktime_get() - start));
> @@ -4308,13 +4317,28 @@ static int btintel_pcie_resume(struct device *dev)
>         return err;
>  }
>
> +static int btintel_pcie_resume(struct device *dev)
> +{
> +       return btintel_pcie_resume_event(dev, PMSG_RESUME);
> +}
> +
> +static int btintel_pcie_restore(struct device *dev)
> +{
> +       return btintel_pcie_resume_event(dev, PMSG_RESTORE);
> +}
> +
> +static int btintel_pcie_thaw(struct device *dev)
> +{
> +       return btintel_pcie_resume_event(dev, PMSG_THAW);
> +}
> +
>  static const struct dev_pm_ops btintel_pcie_pm_ops = {
>         .suspend = btintel_pcie_suspend,
>         .resume = btintel_pcie_resume,
>         .freeze = btintel_pcie_freeze,
> -       .thaw = btintel_pcie_resume,
> +       .thaw = btintel_pcie_thaw,
>         .poweroff = btintel_pcie_hibernate,
> -       .restore = btintel_pcie_resume,
> +       .restore = btintel_pcie_restore,
>  };
>
>  static struct pci_driver btintel_pcie_driver = {
> diff --git a/drivers/bluetooth/btintel_pcie.h b/drivers/bluetooth/btintel_pcie.h
> index 016795fcb..3030b4e8b 100644
> --- a/drivers/bluetooth/btintel_pcie.h
> +++ b/drivers/bluetooth/btintel_pcie.h
> @@ -713,7 +713,6 @@ struct btintel_pcie_ini_dump_info {
>   * @txq: TX Queue struct
>   * @rxq: RX Queue struct
>   * @alive_intr_ctxt: Alive interrupt context
> - * @pm_sx_event: PM event on which system got suspended
>   */
>  struct btintel_pcie_data {
>         struct pci_dev  *pdev;
> @@ -773,7 +772,6 @@ struct btintel_pcie_data {
>         struct btintel_pcie_dbgc        dbgc;
>         struct btintel_pcie_mdbgc       mdbgc;
>         struct btintel_pcie_dump_header dmp_hdr;
> -       u8      pm_sx_event;
>         u32     debug_evt_addr;
>         u32     debug_evt_size;
>         dma_addr_t      debug_table_addr;
> --
> 2.50.1 (Apple Git-155)

Sashiko flagged a few things:

https://sashiko.dev/#/patchset/20260909123416.71919-1-lsa.uz%40pm.me

-- 
Luiz Augusto von Dentz

Consent for Assembly

[yCduIhFgkD]

Hi Sergey,

Sorry for the late response.

I consent to the assembling the series.
Also thanks for the extensive testing! You're really rad!

Regards,
Vladimir

Re: [PATCH 2/2] Bluetooth: btintel_pcie: fix PM flow for S0ix, S3 and S4

[Sergey Lebedev]

Thank you for the pointer. That review reached no list at all: not
linux-bluetooth, not devicetree, and not sashiko's own sashiko-reviews list,
which I checked over 31 August to 9 September. It exists only on the site, so
without you going to look we would not have known it was there.

One of its three findings is real, reproducible, and worse than it claims. I
have measured it rather than argued about it, and the fix is below. The other
two are at the end, more briefly.

A missed alive interrupt now costs the controller, not just the suspend
=======================================================================

1/2 makes set_dxstate() return success when the register says the target state
was reached but the interrupt never arrived. That is correct about the
hardware and silent about data->alive_intr_ctxt, which only the interrupt
handler ever moves. So the tracker is left saying D0 after a suspend that
actually reached D3.

On resume the handler then runs with a stale D0 context while the hardware is
already heading to D0, takes neither branch, and sets neither signal_waitq nor
submit_rx. btintel_pcie_reset_ia() and btintel_pcie_start_rx() are the only
things that re-arm the RX rings, and nothing else on the resume path calls
them.

Measured with the same fixture as my 2 September matrix - one alive interrupt
dropped inside the handler, before it touches anything, which is the state a
genuinely missed one leaves:

  no injection      SP11RX: ctxt d3 -> d0, submit_rx=1     device unchanged
  interrupt dropped SP11RX: ctxt d0 -> d0, submit_rx=0     ...then:

    Bluetooth: hci0: Received hw exception interrupt
    Bluetooth: hci0: command 0x0c01 tx timeout
    Bluetooth: hci0: Opcode 0x0c1a failed: -110
    btintel_pcie 0000:00:14.7: resetting

and the controller comes back as a new hci index. So it is not only that RX
stops: the firmware throws an exception, two HCI commands time out, the driver
FLRs it, and every paired device is gone until something re-pairs.

The fix
=======

Do in the fallback what the handler's branch would have done. It mirrors the
handler's own call site, which also ignores start_rx()'s return:

@@ -4204,11 +4204,25 @@ static int btintel_pcie_set_dxstate(struct btintel_pcie_data *data, u32 dxstate)
 		if (dxstate == BTINTEL_PCIE_STATE_D0) {
-			if (btintel_pcie_in_d0(data))
+			if (btintel_pcie_in_d0(data)) {
+				data->alive_intr_ctxt = BTINTEL_PCIE_D0;
+				btintel_pcie_reset_ia(data);
+				btintel_pcie_start_rx(data);
 				return 0;
+			}
 		} else {
-			if (btintel_pcie_in_d3(data))
+			if (btintel_pcie_in_d3(data)) {
+				data->alive_intr_ctxt = BTINTEL_PCIE_D3;
 				return 0;
+			}
 		}

Both halves are needed, and I only know that because setting the context alone
looked like a complete fix until I dropped the interrupt on the way up
instead:

  build                     drop on D3 entry        drop on D0 entry
  as posted                 wedged, FLR, new hci    -
  context only              clean                   wedged, FLR, new hci
  context + RX re-arm       clean, 3 of 3           clean, 3 of 3

Three cycles of each plus three controls: no "hw exception" and no "resetting"
in any of the nine, and the hci index never moved. One caveat about my own
instrument: I also counted HCI events during a scan after each resume, and one
*control* run returned zero with the device plainly healthy, so that counter is
not trustworthy on its own. The exception and reset lines are what never
misfired.

What I propose to do
====================

Fold it into 1/2 and send the series as v2. My reasoning is that a fix for an
unmerged patch in the same series belongs inside it rather than on top, but I
hold that loosely and a separate patch is just as easy if you prefer it for
review.

It changes Vladimir's logic rather than adding to it, so: Vladimir, say if you
would rather carry it yourself and I will hold. Otherwise I will send v2 in a
day or two, unless Luiz would rather see it sooner.

The second finding, which I could not measure
=============================================

Moving data->gp0_received = false out of the retry loop means a late interrupt
from attempt N can satisfy wait_event_timeout() at the top of attempt N+1, and
"if (status) return 0;" has no hardware check behind it - the register re-read
1/2 adds sits on the timeout path only. So the function can report success
having just written wr_sleep_cntrl() and waited for nothing, right after the
previous iteration read the hardware and found it *not* in the target state.

Real by reading, but unmeasured: my fixture drops interrupts and does not delay
them, so I cannot produce a late one. Saying so rather than implying I tested
it.

The third, which is pre-existing and not this series'
=====================================================

set_dxstate() clears the GP0 cause with btintel_pcie_clr_reg_bits(), which is
a read-modify-write. BTINTEL_PCIE_CSR_MSIX_HW_INT_CAUSES looks
write-1-to-clear: the interrupt handler acknowledges it by writing back exactly
what it read. If so the call does the opposite of both halves of its job - it
writes 0 to GP0, which clears nothing, and 1 to whatever else was pending in
that register, retiring HWEXP, GP1 or FWTRIG unserviced.

Neither patch touches that line, so it is not this series' business, but
someone at Intel may want it.

Thanks,
Sergey