Re: [PATCH] Bluetooth: btintel_pcie: fix stale cache in set_dxstate fallback check

[Sergey Lebedev <lsa.uz@pm.me>]

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