Re: [PATCH] Bluetooth: hci_qca: Reset the in-band sleep state on power off

[Vladyslav Tymoshchuk <tvlad744@gmail.com>]

Hi Birk,

On Sat, 10 Oct 2026 11:35:26 +0000, Birk Skyum wrote:
> qca_power_off() cuts the power of the controller but leaves the host
> side of the in-band sleep (IBS) protocol as it was. tx_ibs_state and
> rx_ibs_state are initialised in qca_open() only, so a controller that
> qca_setup() powers on again meets whatever state the last session ended
> in.

I ran into the same bug from another side, on a OnePlus Pad 3 (SM8750,
WCN7880 on UART at 8 Mbaud with IBS), and came to the same fix in the
same place, so here is a second trigger and some test numbers.

The second trigger is HCI_IBS_TX_AWAKE rather than WAKING:
qca_hci_shutdown() deletes the IBS timers and then sends the
pre-shutdown command, which goes through qca_enqueue() and arms
tx_idle_timer again for 2 s. If the device is opened again within those
2 s, the host still believes the controller is awake and sends the
frames after the new setup without an HCI_IBS_WAKE_IND, while the
freshly booted controller starts asleep. The commands right after the
setup get through, the first one after the controller has gone to sleep
breaks the link:

  Bluetooth: hci0: Frame reassembly failed (-84)
  Bluetooth: hci0: Opcode 0x0c56 failed: -110
  Bluetooth: hci0: crash the soc to collect controller dump

It shows up with "btmgmt power off" followed by "power on" within 2 s,
and on every boot of a board without local-bd-address when user space
sets the address with MGMT Set Public Address right after registration
(the unconfigured controller is powered down after its setup and up
again well within the 2 s). The stale idle timer can also queue an
HCI_IBS_SLEEP_IND in the middle of the next setup, which is what
df1e5c51492f was about.

> +	/* A controller that loses power is asleep when it comes back. */
> +	timer_delete(&qca->wake_retrans_timer);
> +	timer_delete(&qca->tx_idle_timer);
> +	qca->tx_ibs_state = HCI_IBS_TX_ASLEEP;
> +	qca->rx_ibs_state = HCI_IBS_RX_ASLEEP;
>  	spin_unlock_irqrestore(&qca->hci_ibs_lock, flags);

I agree with what the Sashiko review pointed out [1]: without _sync a
timer callback that is already waiting for hci_ibs_lock still runs
after the unlock, and ws_awake_device/ws_awake_rx can queue a wake
indication after the reset. As both timer callbacks take hci_ibs_lock,
the _sync variants have to be called outside of it. This is what I
have been testing (on bluetooth-next):

	/* From this point we go into power off state. But serial port may
	 * still be open, stop queueing the IBS data.
	 */
	spin_lock_irqsave(&qca->hci_ibs_lock, flags);
	set_bit(QCA_IBS_DISABLED, &qca->flags);
	spin_unlock_irqrestore(&qca->hci_ibs_lock, flags);

	/* The controller loses the IBS state with its power and comes back
	 * with both sides asleep, as qca_open() assumes. Stop the IBS work and
	 * timers, drop what they queued and start over from there.
	 */
	cancel_work_sync(&qca->ws_awake_device);
	cancel_work_sync(&qca->ws_awake_rx);
	timer_delete_sync(&qca->wake_retrans_timer);
	timer_delete_sync(&qca->tx_idle_timer);

	spin_lock_irqsave(&qca->hci_ibs_lock, flags);
	qca_flush(hu);
	qca->tx_ibs_state = HCI_IBS_TX_ASLEEP;
	qca->rx_ibs_state = HCI_IBS_RX_ASLEEP;
	spin_unlock_irqrestore(&qca->hci_ibs_lock, flags);

All callers of qca_power_off() can sleep, and the work items do not
call it, so there is no wait cycle. The flush moves after the waits,
so it also drops what the work and the timers queued in the meantime.

Sashiko also suggested Fixes: and Cc: stable, with df1e5c51492f. I
would point Fixes: further back:

Fixes: 3e4be65eb82c ("Bluetooth: hci_qca: Add poweroff support during hci down for wcn3990")
Cc: stable@vger.kernel.org

3e4be65eb82c is where the IBS state started to outlive the power of the
controller; the AWAKE case exists since then, and df1e5c51492f made the
WAKING case permanent. Mentioning the AWAKE trigger in the commit
message would help too.

Test numbers, five quick power cycles each (power on within 2 s of the
power off, then a scan): 4 of 5 fail without the change, with commands
timing out ("Opcode 0x2042 failed: -110"), and 0 of 5 with the code
above. With it, user space also sets the address at boot without having
to wait for the 2 s first, and it is now in the kernel I use on this
tablet every day. I have not tried your v1 as posted; I expect it to
cover my case as well most of the time, but I have only tested the form
above.

Would you fold this into a v2? I am happy to test it on this board and
give my Tested-by there. If you would rather not, I can send it as a
separate patch with your Reported-by, whichever you and the
maintainers prefer.

Thanks,
Vladyslav

[1] https://netdev-ai.bots.linux.dev/sashiko/#/patchset/179163212446.46972.2306254853726299632%40pm.me

(I used an AI assistant, Claude, for the analysis and to draft this
mail; the tests ran on my device.)