* [PATCH v2] Bluetooth: btintel_pcie: fix PM flow for S0ix, S3 and S4
@ 2026-09-02 4:28 Ravindra
2026-09-02 7:01 ` Paul Menzel
0 siblings, 1 reply; 4+ messages in thread
From: Ravindra @ 2026-09-02 4:28 UTC (permalink / raw)
To: linux-bluetooth
Cc: ravishankar.srivatsa, chethan.tumkur.narayan, kiran.k, Ravindra
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>
---
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 30923eaabed7..90d4a3596f80 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>
@@ -3515,11 +3516,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);
@@ -3565,18 +3571,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",
@@ -3599,7 +3610,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;
@@ -3607,19 +3618,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;
@@ -3628,7 +3635,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));
@@ -3653,13 +3662,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 9baa214d9bbe..290679f45652 100644
--- a/drivers/bluetooth/btintel_pcie.h
+++ b/drivers/bluetooth/btintel_pcie.h
@@ -541,7 +541,6 @@ struct btintel_pcie_dump_header {
* @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;
@@ -600,7 +599,6 @@ struct btintel_pcie_data {
enum btintel_pcie_reset_type reset_type;
struct btintel_pcie_dbgc dbgc;
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.43.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] Bluetooth: btintel_pcie: fix PM flow for S0ix, S3 and S4
2026-09-02 4:28 Ravindra
@ 2026-09-02 7:01 ` Paul Menzel
0 siblings, 0 replies; 4+ messages in thread
From: Paul Menzel @ 2026-09-02 7:01 UTC (permalink / raw)
To: Ravindra
Cc: linux-bluetooth, ravishankar.srivatsa, chethan.tumkur.narayan,
kiran.k
Dear Ravindra,
Thank you for your patch.
Am 02.09.26 um 06:28 schrieb Ravindra:
Please start by describing the problem.
> 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.
Please add how you tested this.
> Fixes: e57362f4911b ("Bluetooth: btintel_pcie: Add support for _suspend() / _resume()")
> Assisted-by: GitHub-Copilot:GPT5
> Signed-off-by: Ravindra <ravindra@intel.com>
> ---
> 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 30923eaabed7..90d4a3596f80 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>
> @@ -3515,11 +3516,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);
>
gemini/gemini-3.1-pro-preview comments [1]:
> Can a late interrupt from a previous retry attempt cause wait_event_timeout()
> to return immediately?
> If a previous attempt times out, the loop will issue a new
> btintel_pcie_wr_sleep_cntrl() call. If a dangling interrupt from the first
> attempt fires concurrently, it will set data->gp0_received to true. This
> would spuriously satisfy the wait condition while the hardware is actively
> processing the second transition, potentially causing the driver to access
> the device before it is fully initialized.
> @@ -3565,18 +3571,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",
> @@ -3599,7 +3610,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;
> @@ -3607,19 +3618,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;
> @@ -3628,7 +3635,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));
> @@ -3653,13 +3662,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 9baa214d9bbe..290679f45652 100644
> --- a/drivers/bluetooth/btintel_pcie.h
> +++ b/drivers/bluetooth/btintel_pcie.h
> @@ -541,7 +541,6 @@ struct btintel_pcie_dump_header {
> * @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;
> @@ -600,7 +599,6 @@ struct btintel_pcie_data {
> enum btintel_pcie_reset_type reset_type;
> struct btintel_pcie_dbgc dbgc;
> 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;
Kind regards,
Paul
[1]:
https://sashiko.dev/#/patchset/20260902042840.2432862-1-ravindra%40intel.com
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] Bluetooth: btintel_pcie: fix PM flow for S0ix, S3 and S4
@ 2026-09-02 9:10 Sergey Lebedev
2026-09-02 13:38 ` Sergey Lebedev
0 siblings, 1 reply; 4+ messages in thread
From: Sergey Lebedev @ 2026-09-02 9:10 UTC (permalink / raw)
To: Ravindra
Cc: Vladimir V . Kondratyev, Paul Menzel, Kiran K,
Chandrashekar Devegowda, Ravishankar Srivatsa,
Chethan Tumkur Narayan, Marcel Holtmann, Luiz Augusto von Dentz,
linux-bluetooth
Ravindra,
Paul Menzel pointed me at this patch from the thread on Vladimir
Kondratyev's stale-cache fix, since both change
btintel_pcie_set_dxstate(). They overlap textually and, more usefully,
they fix different halves of the same failure. I have the hardware and a
fixture for one of those halves, so here is what I can add.
Your change moves the flag out of the retry loop:
+ data->gp0_received = false;
+
do {
- data->gp0_received = false;
That recovers the case where the alive interrupt is merely *late* — it
arrives while a later retry is waiting, and previously the per-iteration
reset threw it away. The comment you added states the invariant well.
What it does not cover is the interrupt being missed outright. When that
happens gp0_received never becomes true, the loop falls through to
if (dxstate == BTINTEL_PCIE_STATE_D0) {
if (btintel_pcie_in_d0(data))
and those helpers read data->boot_stage_cache, which only the interrupt
handler ever writes. No interrupt, no refresh: the fallback is asked to
decide using the value from before the transition, so it reports the old
state, every retry is exhausted, and -EBUSY aborts the suspend. Vladimir's
patch re-reads BTINTEL_PCIE_CSR_BOOT_STAGE_REG immediately before that
check, which is what btintel_pcie_resume() already does.
So the two are complementary rather than competing: yours makes a late
interrupt count, his makes a missing one survivable.
I can be concrete about the second half because I reproduced it
deliberately. Surface Pro 11 (Intel, Lunar Lake), Intel BE201, 8086:a876
rev 10. The driver was built out of tree with one debug-only parameter
that returns from btintel_pcie_msix_gp0_handler() before the
boot_stage_cache refresh, and only while alive_intr_ctxt is
BTINTEL_PCIE_D0 — the exact state a genuinely missed alive interrupt
leaves behind, with the controller still reaching D3.
Under that fixture, moving the flag out of the loop does not help: there
is no late interrupt to catch, and the cache stays stale. Three timeouts,
-EBUSY, suspend aborted. With the re-read, one timeout and the system
suspends. Method and logs are in
Message-ID <20260830151550.44687-1-lsa.uz@pm.me>.
The failure is not theoretical here. Before any workaround it aborted 2 of
the 3 suspends I attempted, and one device returning -EBUSY stops the
whole system from sleeping.
Whichever of the two lands first, the other will need a rebase — the
hunks touch adjacent lines. If it helps, I am happy to test them applied
together against my fixture and on ordinary suspend cycles, and to report
per-patch rather than as a combined result.
If you and Vladimir would prefer these as one series rather than two
independent patches, I am happy to do the assembly work — rebased, with
both of you as the authors of your respective halves and the Fixes: tags
kept separate, since they are separate bugs with separate reverts. I did
not want to send that uninvited over two patches that are already on the
list; say the word and it is a short job.
One question on your patch, from someone who does not have S4 on this
machine to check: .thaw now takes the normal D0 path, while .restore
forces FLR. After PM_EVENT_FREEZE the controller was put into D3_COLD by
btintel_pcie_suspend_late(). Is D3_COLD guaranteed to preserve firmware
across a freeze/thaw on this part, or does thaw also need the FLR path?
Thanks,
Sergey
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] Bluetooth: btintel_pcie: fix PM flow for S0ix, S3 and S4
2026-09-02 9:10 [PATCH v2] Bluetooth: btintel_pcie: fix PM flow for S0ix, S3 and S4 Sergey Lebedev
@ 2026-09-02 13:38 ` Sergey Lebedev
0 siblings, 0 replies; 4+ messages in thread
From: Sergey Lebedev @ 2026-09-02 13:38 UTC (permalink / raw)
To: Ravindra
Cc: Vladimir V . Kondratyev, Paul Menzel, Kiran K,
Chandrashekar Devegowda, Marcel Holtmann, Luiz Augusto von Dentz,
linux-bluetooth
I said in my earlier mail that these two patches fix different halves of
one failure, and offered to test them. Here is the measurement rather
than the argument.
Hardware: Surface Pro 11 (Intel, Lunar Lake), Intel BE201, 8086:a876
rev 10, kernel 7.0.0-30, s2idle. Four builds of btintel_pcie from the
same source, differing only in which hunk is present, each carrying one
debug-only module parameter that returns from
btintel_pcie_msix_gp0_handler() before the boot_stage_cache refresh and
only while alive_intr_ctxt is BTINTEL_PCIE_D0. That is a missed alive
interrupt: gp0_received never set, cache never refreshed, controller
still reaching D3. Suspend triggered with rtcwake -m freeze -s 45.
build result
---------------------------------------------------------------
neither hunk 3 timeouts, -EBUSY, suspend aborted
Ravindra's hunk only 3 timeouts, -EBUSY, suspend aborted
Vladimir's hunk only 1 timeout, suspended and resumed
both hunks 1 timeout, suspended and resumed
Unpatched and with your hunk alone, identical:
PM: suspend entry (s2idle)
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: failed to suspend async: error -16
PM: Some devices failed to suspend, or early wake event detected
PM: suspend exit <- 1.7 s after entry; it never slept
With the re-read present, alone or alongside your hunk:
PM: suspend entry (s2idle)
Bluetooth: hci0: Timeout (200 ms) on alive interrupt for D2 entry, retry count 0
PM: suspend exit <- 48.8 s after entry; it slept the full period
This is not a criticism of your patch: the fixture removes the interrupt
entirely, so there is no late interrupt for your hunk to rescue, and it
cannot help by construction. It measures the half it does not cover, and
confirms the two do not interfere when applied together.
The single-run table above had one oddity - the both-hunks run slept 8.6 s
instead of the full 45 - so I repeated it rather than leave a guess in the
archive. Eight further cycles, no failures anywhere:
both hunks 48.0 48.6 50.8 48.6 50.7 s
re-read only 48.6 46.6 50.8 s
The 8.6 s did not recur. It was a stray wake, not a property of the
combination.
Those runs turned up something better than the caveat they removed. The
retry count varies between 1 and 3 from run to run, and the outcome does
not: even when all three retries time out, the re-read then observes D3
and set_dxstate() returns 0. One of the re-read-only runs did exactly
that - three timeouts, no -EBUSY, slept 46.6 s - which is the fallback
doing the whole job with the retry loop having contributed nothing.
One incidental confirmation, since Paul asked in the other thread whether
a register called *boot stage* really changes after boot. Tracing the
fallback path prints what it actually reads at D3 entry:
SP11TRACE: retry=0 dxstate=2 gp0_received=0 boot_stage=0x61710007
Bit 24 - BTINTEL_PCIE_CSR_BOOT_STAGE_D3_STATE_READY - is set, on hardware,
long after boot, which is why the re-read is able to answer the question
the cached value cannot.
One caveat I cannot remove. I have no fixture for the late-interrupt case
your hunk addresses, so I have measured only that it does no harm here,
not that it works. That needs a different fixture - delaying the interrupt
rather than dropping it - and I have not built one.
Raw log and the four builds are available if anyone wants them.
While instrumenting this I caught the failure happening on its own, with the
injection disabled, which I had not managed before. One run in six:
PM: suspend entry (s2idle)
Timeout (200 ms) on alive interrupt for D2 entry, retry count 0
SP11TRACE: retry=0 dxstate=2 gp0_received=0 boot_stage=0x61511007
Timeout (200 ms) on alive interrupt for D0 entry, retry count 0
SP11TRACE: retry=0 dxstate=0 gp0_received=0 boot_stage=0xa0db1007
Timeout (3000 ms) on alive interrupt, alive context: intel_reset1
The interrupt went missing in both directions in the same cycle, and the
re-read rescued both: bit 24 set at D3 entry, clear at D0 entry, each time
matching the state the controller had actually reached. No -EBUSY. So this
is not only a fixture artefact - the hardware does drop it, and the register
is right when the interrupt is not.
That leads to a question rather than a patch, for whoever owns this code.
Both patches keep the interrupt as the primary signal and the register as a
fallback. Given that the driver's own comment calls the missed interrupt a
hardware bug, and given the register answers correctly in both directions,
would it not be sounder to invert that - poll BOOT_STAGE_REG for the target
bit, and treat the interrupt as an early-exit optimisation rather than the
thing being waited on?
The file already does exactly this shape for the reset path:
do {
reg = btintel_pcie_rd_reg32(data, BTINTEL_PCIE_CSR_FUNC_CTRL_REG);
if (reg & BTINTEL_PCIE_CSR_FUNC_CTRL_BUS_MASTER_STS)
break;
usleep_range(10000, 12000);
} while (--retry > 0);
and btintel_pcie.c declares POLL_INTERVAL_US, which nothing uses - so the
intent seems to have existed at some point.
The practical difference is not only correctness. Today a missed interrupt
costs up to 600 ms of waiting per transition before the fallback is even
consulted; a poll would return as soon as the bit flips. And the stale-cache
class of bug disappears rather than being caught.
I am not proposing this as a patch over yours - you two own this code and I
have one machine. But if either of you thinks the shape is right, I have the
hardware, the fixture and now a spontaneous reproduction to test it against.
Vladimir has said he is content either way on the series question, so it
rests with you and the maintainers. My offer to do the assembly stands.
Thanks,
Sergey
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-09-02 13:38 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-02 9:10 [PATCH v2] Bluetooth: btintel_pcie: fix PM flow for S0ix, S3 and S4 Sergey Lebedev
2026-09-02 13:38 ` Sergey Lebedev
-- strict thread matches above, loose matches on Subject: below --
2026-09-02 4:28 Ravindra
2026-09-02 7:01 ` Paul Menzel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox