Messages in this thread |  | | Date | Wed, 02 Sep 2026 07:24:30 +0000 | | From | Sergey Lebedev <> | | Subject | Re: [PATCH] Bluetooth: btintel_pcie: fix stale cache in set_dxstate fallback check |
| |
Paul,
Two of your three points I can answer from the hardware, since the Tested-by on this patch is mine.
> Can you reproduce this on real hardware?
Yes. Surface Pro 11 (Intel, Lunar Lake), Intel BE201, 8086:a876 rev 10. It first appeared in ordinary use, not in a test:
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 returns -16 PM: Some devices failed to suspend, or early wake event detected
One device returning -EBUSY aborts the whole system suspend, so the machine does not sleep at all.
I have to be honest about the rate rather than useful: it failed on 2 of the 3 suspends I attempted before installing a workaround, and later, on a newer kernel with the workaround removed, it succeeded 7 times out of 7 with no btintel_pcie change in between. So the spontaneous failure is not something I can produce on demand.
What I could do was exercise the exact path deterministically. I built the driver 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. That leaves the cache stale and gp0_received false — precisely the state a genuinely missed alive interrupt leaves behind — while the controller still reaches D3.
Without the patch, that reproduces the production failure exactly: three timeouts, -EBUSY, suspend aborted. With the patch and nothing else changed: one timeout, the re-read observes D3, set_dxstate() returns 0, and the system suspends and resumes normally.
Full method and logs are in my earlier message on this thread, Message-ID <20260830151550.44687-1-lsa.uz@pm.me>.
> To me it sounds strange, that a register name *boot stage* should > change after the boot. ;-)
It is a fair reaction to the name, but the register genuinely carries live power state. From the driver's own header:
#define BTINTEL_PCIE_CSR_BOOT_STAGE_D3_STATE_READY (BIT(24))
and both helpers test that one bit and nothing else:
static bool btintel_pcie_in_d3(struct btintel_pcie_data *data) { return data->boot_stage_cache & BTINTEL_PCIE_CSR_BOOT_STAGE_D3_STATE_READY; }
static bool btintel_pcie_in_d0(struct btintel_pcie_data *data) { return !(data->boot_stage_cache & BTINTEL_PCIE_CSR_BOOT_STAGE_D3_STATE_READY); }
So bit 24 is updated by the controller on every D-state transition, long after boot. The name describes the register's original purpose rather than everything Intel later put in it. btintel_pcie_resume() already re-reads it for exactly this reason, which is what makes the patch's change consistent rather than novel.
The re-flow is Vladimir's call and I have no opinion on it. If a v3 comes out of this, I still have the hardware and the test fixture and will re-run both halves against it.
Thanks, Sergey
|  |