[PATCH 0/4] Bluetooth: qca: Fix WCN3950 power-off

[PATCH 0/4] Bluetooth: qca: Fix WCN3950 power-off

[Michael McAllister]

Hi,

Turning Bluetooth off on my Moto G10 leaves all four WCN3950 regulator
votes enabled. This series fixes the shared UART cleanup and shutdown
ordering first, then adds WCN3950 to the existing power-off paths.

Thanks,
Michael

Michael McAllister (4):
  Bluetooth: qca: Restore flow control after a failed power pulse
  Bluetooth: hci_serdev: Track whether the serial port is open
  Bluetooth: qca: Power off after unregistering the HCI device
  Bluetooth: qca: Power off WCN3950

 drivers/bluetooth/hci_mrvl.c   |  4 ++--
 drivers/bluetooth/hci_qca.c    | 38 ++++++++++------------------------
 drivers/bluetooth/hci_serdev.c | 37 ++++++++++++++++++++++++++-------
 drivers/bluetooth/hci_uart.h   |  3 +++
 4 files changed, 46 insertions(+), 36 deletions(-)


base-commit: 1081a6ef8f1e0661fd52eb3f878691b0b96cdff9

[PATCH 2/4] Bluetooth: hci_serdev: Track whether the serial port is open

[Michael McAllister]

QCA setup temporarily closes the serial port while enabling the supplies.
If enabling the supplies or reopening the port fails, PROTO_READY stays
set. Error cleanup can then flush or close an already-closed port.

Track the open state separately from protocol readiness. Use the same
open and close helpers in the transport and the QCA and Marvell drivers,
so setup retries do not close the port twice. Skip the serial flush when
the port is closed, while still flushing the protocol queues.

Close the port during unregister according to its actual open state.

Signed-off-by: Michael McAllister <me@michaelmcallister.org>
---
 drivers/bluetooth/hci_mrvl.c   |  4 ++--
 drivers/bluetooth/hci_qca.c    | 12 +++++------
 drivers/bluetooth/hci_serdev.c | 37 +++++++++++++++++++++++++++-------
 drivers/bluetooth/hci_uart.h   |  3 +++
 4 files changed, 41 insertions(+), 15 deletions(-)

diff --git a/drivers/bluetooth/hci_mrvl.c b/drivers/bluetooth/hci_mrvl.c
index 5798a8db0..38d9add89 100644
--- a/drivers/bluetooth/hci_mrvl.c
+++ b/drivers/bluetooth/hci_mrvl.c
@@ -76,7 +76,7 @@ static int mrvl_open(struct hci_uart *hu)
 	hu->priv = mrvl;
 
 	if (hu->serdev) {
-		ret = serdev_device_open(hu->serdev);
+		ret = hci_uart_serdev_open(hu);
 		if (ret)
 			goto err;
 	}
@@ -95,7 +95,7 @@ static int mrvl_close(struct hci_uart *hu)
 	BT_DBG("hu %p", hu);
 
 	if (hu->serdev)
-		serdev_device_close(hu->serdev);
+		hci_uart_serdev_close(hu);
 
 	skb_queue_purge(&mrvl->txq);
 	skb_queue_purge(&mrvl->rawq);
diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index 8705c4543..3d9e6cd15 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -1755,8 +1755,8 @@ static int qca_port_reopen(struct hci_uart *hu)
 	 * Without this, we will have RTS and CTS synchronization
 	 * issues.
 	 */
-	serdev_device_close(hu->serdev);
-	ret = serdev_device_open(hu->serdev);
+	hci_uart_serdev_close(hu);
+	ret = hci_uart_serdev_open(hu);
 	if (ret) {
 		bt_dev_err(hu->hdev, "failed to open port");
 		return ret;
@@ -1780,12 +1780,12 @@ static int qca_regulator_init(struct hci_uart *hu)
 	qcadev = serdev_device_get_drvdata(hu->serdev);
 
 	if (!qcadev->bt_power->vregs_on) {
-		serdev_device_close(hu->serdev);
+		hci_uart_serdev_close(hu);
 		ret = qca_regulator_enable(qcadev);
 		if (ret)
 			return ret;
 
-		ret = serdev_device_open(hu->serdev);
+		ret = hci_uart_serdev_open(hu);
 		if (ret) {
 			bt_dev_err(hu->hdev, "failed to open port");
 			return ret;
@@ -2061,8 +2061,8 @@ static int qca_setup(struct hci_uart *hu)
 		if (retries < MAX_INIT_RETRIES) {
 			bt_dev_warn(hdev, "Retry BT power ON:%d", retries);
 			if (hu->serdev) {
-				serdev_device_close(hu->serdev);
-				ret = serdev_device_open(hu->serdev);
+				hci_uart_serdev_close(hu);
+				ret = hci_uart_serdev_open(hu);
 				if (ret) {
 					bt_dev_err(hdev, "failed to open port");
 					return ret;
diff --git a/drivers/bluetooth/hci_serdev.c b/drivers/bluetooth/hci_serdev.c
index 48ba017e2..05aa490a9 100644
--- a/drivers/bluetooth/hci_serdev.c
+++ b/drivers/bluetooth/hci_serdev.c
@@ -21,6 +21,29 @@
 
 #include "hci_uart.h"
 
+int hci_uart_serdev_open(struct hci_uart *hu)
+{
+	int err;
+
+	if (hu->serdev_open)
+		return 0;
+
+	err = serdev_device_open(hu->serdev);
+	if (!err)
+		hu->serdev_open = true;
+
+	return err;
+}
+
+void hci_uart_serdev_close(struct hci_uart *hu)
+{
+	if (!hu->serdev_open)
+		return;
+
+	hu->serdev_open = false;
+	serdev_device_close(hu->serdev);
+}
+
 static inline void hci_uart_tx_complete(struct hci_uart *hu, int pkt_type)
 {
 	struct hci_dev *hdev = hu->hdev;
@@ -102,7 +125,8 @@ static int hci_uart_flush(struct hci_dev *hdev)
 	}
 
 	/* Flush any pending characters in the driver and discipline. */
-	serdev_device_write_flush(hu->serdev);
+	if (hu->serdev_open)
+		serdev_device_write_flush(hu->serdev);
 
 	if (test_bit(HCI_UART_PROTO_READY, &hu->flags))
 		hu->proto->flush(hu);
@@ -123,7 +147,7 @@ static int hci_uart_open(struct hci_dev *hdev)
 	 * BT OFF. Upon next BT ON UART port should be opened.
 	 */
 	if (!test_bit(HCI_UART_PROTO_READY, &hu->flags)) {
-		err = serdev_device_open(hu->serdev);
+		err = hci_uart_serdev_open(hu);
 		if (err)
 			return err;
 		set_bit(HCI_UART_PROTO_READY, &hu->flags);
@@ -154,7 +178,7 @@ static int hci_uart_close(struct hci_dev *hdev)
 	 */
 	if (hci_test_quirk(hdev, HCI_QUIRK_NON_PERSISTENT_SETUP)) {
 		clear_bit(HCI_UART_PROTO_READY, &hu->flags);
-		serdev_device_close(hu->serdev);
+		hci_uart_serdev_close(hu);
 	}
 
 	return 0;
@@ -314,7 +338,7 @@ int hci_uart_register_device_priv(struct hci_uart *hu,
 	if (percpu_init_rwsem(&hu->proto_lock))
 		return -ENOMEM;
 
-	err = serdev_device_open(hu->serdev);
+	err = hci_uart_serdev_open(hu);
 	if (err)
 		goto err_rwsem;
 
@@ -385,7 +409,7 @@ int hci_uart_register_device_priv(struct hci_uart *hu,
 	clear_bit(HCI_UART_PROTO_READY, &hu->flags);
 	p->close(hu);
 err_open:
-	serdev_device_close(hu->serdev);
+	hci_uart_serdev_close(hu);
 err_rwsem:
 	percpu_free_rwsem(&hu->proto_lock);
 	return err;
@@ -438,8 +462,7 @@ void hci_uart_unregister_device(struct hci_uart *hu)
 	 */
 	hu->proto->close(hu);
 
-	if (proto_ready)
-		serdev_device_close(hu->serdev);
+	hci_uart_serdev_close(hu);
 
 	hci_free_dev(hdev);
 
diff --git a/drivers/bluetooth/hci_uart.h b/drivers/bluetooth/hci_uart.h
index 7fbe8dffa..2f8e346f3 100644
--- a/drivers/bluetooth/hci_uart.h
+++ b/drivers/bluetooth/hci_uart.h
@@ -66,6 +66,7 @@ struct hci_uart_proto {
 struct hci_uart {
 	struct tty_struct	*tty;
 	struct serdev_device	*serdev;
+	bool			serdev_open;
 	struct hci_dev		*hdev;
 	unsigned long		flags;
 	unsigned long		hdev_flags;
@@ -112,6 +113,8 @@ static inline int hci_uart_register_device(struct hci_uart *hu,
 }
 
 void hci_uart_unregister_device(struct hci_uart *hu);
+int hci_uart_serdev_open(struct hci_uart *hu);
+void hci_uart_serdev_close(struct hci_uart *hu);
 
 int hci_uart_tx_wakeup(struct hci_uart *hu);
 int hci_uart_wait_until_sent(struct hci_uart *hu);

RE: Bluetooth: qca: Fix WCN3950 power-off

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1302 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/1195776/

---Test result---

Test Summary:
CheckPatch                    PASS      3.03 seconds
VerifyFixes                   PASS      0.07 seconds
VerifySignedoff               PASS      0.07 seconds
GitLint                       PASS      0.93 seconds
SubjectPrefix                 PASS      0.27 seconds
BuildKernel                   PASS      32.82 seconds
CheckAllWarning               PASS      36.55 seconds
CheckSparse                   WARNING   40.78 seconds
BuildKernel32                 PASS      35.33 seconds
CheckKernelLLVM               PASS      36.31 seconds
TestRunnerSetup               PASS      768.31 seconds
IncrementalBuild              PASS      37.75 seconds

Details
##############################
Test: CheckSparse - WARNING
Desc: Run sparse tool with linux kernel
Output:
drivers/bluetooth/hci_mrvl.c:170:23: warning: restricted __le16 degrades to integerdrivers/bluetooth/hci_mrvl.c:203:23: warning: restricted __le16 degrades to integer


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

---
Regards,
Linux Bluetooth

Re: [PATCH 2/4] Bluetooth: hci_serdev: Track whether the serial port is open

[Markus Probst]

[-- Attachment #1: Type: text/plain, Size: 7741 bytes --]

On Wed, 2026-10-28 at 22:34 +0000, Michael McAllister wrote:
> QCA setup temporarily closes the serial port while enabling the supplies.
> If enabling the supplies or reopening the port fails, PROTO_READY stays
> set. Error cleanup can then flush or close an already-closed port.
> 
> Track the open state separately from protocol readiness. Use the same
> open and close helpers in the transport and the QCA and Marvell drivers,
> so setup retries do not close the port twice. Skip the serial flush when
> the port is closed, while still flushing the protocol queues.
> 
> Close the port during unregister according to its actual open state.
> 
> Signed-off-by: Michael McAllister <me@michaelmcallister.org>
> ---
>  drivers/bluetooth/hci_mrvl.c   |  4 ++--
>  drivers/bluetooth/hci_qca.c    | 12 +++++------
>  drivers/bluetooth/hci_serdev.c | 37 +++++++++++++++++++++++++++-------
>  drivers/bluetooth/hci_uart.h   |  3 +++
>  4 files changed, 41 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/bluetooth/hci_mrvl.c b/drivers/bluetooth/hci_mrvl.c
> index 5798a8db0..38d9add89 100644
> --- a/drivers/bluetooth/hci_mrvl.c
> +++ b/drivers/bluetooth/hci_mrvl.c
> @@ -76,7 +76,7 @@ static int mrvl_open(struct hci_uart *hu)
>  	hu->priv = mrvl;
>  
>  	if (hu->serdev) {
> -		ret = serdev_device_open(hu->serdev);
> +		ret = hci_uart_serdev_open(hu);
Hi,

Why is `serdev_device_open`/`hci_uart_serdev_open` even called here?

According to Sashiko [1], `hci_uart_register_device` already calls
`serdev_device_open` and directly after that `p->open` (effectively
`mvrl_open` and thus another `serdev_device_open`). If Sashiko is right
(which appears to be the case at first look), the device is always
opened twice on probe.

From what I can tell, opening it twice is *undefined behaviour*. This
patch adds a check if the device is open, before calling
`serdev_device_open`. This solves the issue (possibly by accident), but
does not explain why it exists in the first place.

My patch [2] would instead return the error -EALREADY, which could
prevent it to probe entirely. It would also allow calling
`serdev_device_close` on a already closed serdev device, which makes
the use in the commit message obsolete.

Thanks
- Markus Probst

[1]
https://lore.kernel.org/linux-serial/sashiko-outbox-211656@kernel.org/
[2]
https://lore.kernel.org/linux-serial/20261112-rust_serdev_probe_refactor-v6-1-3568ed56264c@posteo.de/


>  		if (ret)
>  			goto err;
>  	}
> @@ -95,7 +95,7 @@ static int mrvl_close(struct hci_uart *hu)
>  	BT_DBG("hu %p", hu);
>  
>  	if (hu->serdev)
> -		serdev_device_close(hu->serdev);
> +		hci_uart_serdev_close(hu);
>  
>  	skb_queue_purge(&mrvl->txq);
>  	skb_queue_purge(&mrvl->rawq);
> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
> index 8705c4543..3d9e6cd15 100644
> --- a/drivers/bluetooth/hci_qca.c
> +++ b/drivers/bluetooth/hci_qca.c
> @@ -1755,8 +1755,8 @@ static int qca_port_reopen(struct hci_uart *hu)
>  	 * Without this, we will have RTS and CTS synchronization
>  	 * issues.
>  	 */
> -	serdev_device_close(hu->serdev);
> -	ret = serdev_device_open(hu->serdev);
> +	hci_uart_serdev_close(hu);
> +	ret = hci_uart_serdev_open(hu);
>  	if (ret) {
>  		bt_dev_err(hu->hdev, "failed to open port");
>  		return ret;
> @@ -1780,12 +1780,12 @@ static int qca_regulator_init(struct hci_uart *hu)
>  	qcadev = serdev_device_get_drvdata(hu->serdev);
>  
>  	if (!qcadev->bt_power->vregs_on) {
> -		serdev_device_close(hu->serdev);
> +		hci_uart_serdev_close(hu);
>  		ret = qca_regulator_enable(qcadev);
>  		if (ret)
>  			return ret;
>  
> -		ret = serdev_device_open(hu->serdev);
> +		ret = hci_uart_serdev_open(hu);
>  		if (ret) {
>  			bt_dev_err(hu->hdev, "failed to open port");
>  			return ret;
> @@ -2061,8 +2061,8 @@ static int qca_setup(struct hci_uart *hu)
>  		if (retries < MAX_INIT_RETRIES) {
>  			bt_dev_warn(hdev, "Retry BT power ON:%d", retries);
>  			if (hu->serdev) {
> -				serdev_device_close(hu->serdev);
> -				ret = serdev_device_open(hu->serdev);
> +				hci_uart_serdev_close(hu);
> +				ret = hci_uart_serdev_open(hu);
>  				if (ret) {
>  					bt_dev_err(hdev, "failed to open port");
>  					return ret;
> diff --git a/drivers/bluetooth/hci_serdev.c b/drivers/bluetooth/hci_serdev.c
> index 48ba017e2..05aa490a9 100644
> --- a/drivers/bluetooth/hci_serdev.c
> +++ b/drivers/bluetooth/hci_serdev.c
> @@ -21,6 +21,29 @@
>  
>  #include "hci_uart.h"
>  
> +int hci_uart_serdev_open(struct hci_uart *hu)
> +{
> +	int err;
> +
> +	if (hu->serdev_open)
> +		return 0;
> +
> +	err = serdev_device_open(hu->serdev);
> +	if (!err)
> +		hu->serdev_open = true;
> +
> +	return err;
> +}
> +
> +void hci_uart_serdev_close(struct hci_uart *hu)
> +{
> +	if (!hu->serdev_open)
> +		return;
> +
> +	hu->serdev_open = false;
> +	serdev_device_close(hu->serdev);
> +}
> +
>  static inline void hci_uart_tx_complete(struct hci_uart *hu, int pkt_type)
>  {
>  	struct hci_dev *hdev = hu->hdev;
> @@ -102,7 +125,8 @@ static int hci_uart_flush(struct hci_dev *hdev)
>  	}
>  
>  	/* Flush any pending characters in the driver and discipline. */
> -	serdev_device_write_flush(hu->serdev);
> +	if (hu->serdev_open)
> +		serdev_device_write_flush(hu->serdev);
>  
>  	if (test_bit(HCI_UART_PROTO_READY, &hu->flags))
>  		hu->proto->flush(hu);
> @@ -123,7 +147,7 @@ static int hci_uart_open(struct hci_dev *hdev)
>  	 * BT OFF. Upon next BT ON UART port should be opened.
>  	 */
>  	if (!test_bit(HCI_UART_PROTO_READY, &hu->flags)) {
> -		err = serdev_device_open(hu->serdev);
> +		err = hci_uart_serdev_open(hu);
>  		if (err)
>  			return err;
>  		set_bit(HCI_UART_PROTO_READY, &hu->flags);
> @@ -154,7 +178,7 @@ static int hci_uart_close(struct hci_dev *hdev)
>  	 */
>  	if (hci_test_quirk(hdev, HCI_QUIRK_NON_PERSISTENT_SETUP)) {
>  		clear_bit(HCI_UART_PROTO_READY, &hu->flags);
> -		serdev_device_close(hu->serdev);
> +		hci_uart_serdev_close(hu);
>  	}
>  
>  	return 0;
> @@ -314,7 +338,7 @@ int hci_uart_register_device_priv(struct hci_uart *hu,
>  	if (percpu_init_rwsem(&hu->proto_lock))
>  		return -ENOMEM;
>  
> -	err = serdev_device_open(hu->serdev);
> +	err = hci_uart_serdev_open(hu);
>  	if (err)
>  		goto err_rwsem;
>  
> @@ -385,7 +409,7 @@ int hci_uart_register_device_priv(struct hci_uart *hu,
>  	clear_bit(HCI_UART_PROTO_READY, &hu->flags);
>  	p->close(hu);
>  err_open:
> -	serdev_device_close(hu->serdev);
> +	hci_uart_serdev_close(hu);
>  err_rwsem:
>  	percpu_free_rwsem(&hu->proto_lock);
>  	return err;
> @@ -438,8 +462,7 @@ void hci_uart_unregister_device(struct hci_uart *hu)
>  	 */
>  	hu->proto->close(hu);
>  
> -	if (proto_ready)
> -		serdev_device_close(hu->serdev);
> +	hci_uart_serdev_close(hu);
>  
>  	hci_free_dev(hdev);
>  
> diff --git a/drivers/bluetooth/hci_uart.h b/drivers/bluetooth/hci_uart.h
> index 7fbe8dffa..2f8e346f3 100644
> --- a/drivers/bluetooth/hci_uart.h
> +++ b/drivers/bluetooth/hci_uart.h
> @@ -66,6 +66,7 @@ struct hci_uart_proto {
>  struct hci_uart {
>  	struct tty_struct	*tty;
>  	struct serdev_device	*serdev;
> +	bool			serdev_open;
>  	struct hci_dev		*hdev;
>  	unsigned long		flags;
>  	unsigned long		hdev_flags;
> @@ -112,6 +113,8 @@ static inline int hci_uart_register_device(struct hci_uart *hu,
>  }
>  
>  void hci_uart_unregister_device(struct hci_uart *hu);
> +int hci_uart_serdev_open(struct hci_uart *hu);
> +void hci_uart_serdev_close(struct hci_uart *hu);
>  
>  int hci_uart_tx_wakeup(struct hci_uart *hu);
>  int hci_uart_wait_until_sent(struct hci_uart *hu);

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 870 bytes --]

[PATCH 0/1] Bluetooth: hci_mrvl: Fix duplicate serdev open/close

[Michael McAllister]

Hi Markus, thanks for flagging this!

You're right, hci_serdev already opens and closes the port, so the extra
calls in the Marvell driver aren't needed. This removes them as a
separate fix.

I've build-tested it and checked that the duplicate-open reproducer
passes with the fix, but I don't have Marvell hardware to test on.

Thanks,
Michael

[PATCH 1/1] Bluetooth: hci_mrvl: Leave the serdev port lifetime to hci_serdev

[Michael McAllister]

hci_uart_register_device_priv() opens the port before calling mrvl_open(),
so Marvell opens it twice. Likewise, mrvl_close() closes it before the
transport closes it again during unregister or registration error cleanup.

Remove the extra calls and let hci_serdev own the port lifetime. This also
avoids failing probe when serdev rejects a second open with -EALREADY.

Fixes: be70e5e774a6 ("Bluetooth: hci_mrvl: Add serdev support")
Reported-by: Markus Probst <markus.probst@posteo.de>
Closes: https://lore.kernel.org/341e56b1ff7d8a5b109e26e30dfed61faa54d603.camel@posteo.de/
Assisted-by: LLM
Signed-off-by: Michael McAllister <me@michaelmcallister.org>
---
 drivers/bluetooth/hci_mrvl.c | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/drivers/bluetooth/hci_mrvl.c b/drivers/bluetooth/hci_mrvl.c
index 5798a8db0..def19907c 100644
--- a/drivers/bluetooth/hci_mrvl.c
+++ b/drivers/bluetooth/hci_mrvl.c
@@ -57,7 +57,6 @@ struct hci_mrvl_pkt {
 static int mrvl_open(struct hci_uart *hu)
 {
 	struct mrvl_data *mrvl;
-	int ret;
 
 	BT_DBG("hu %p", hu);
 
@@ -75,17 +74,7 @@ static int mrvl_open(struct hci_uart *hu)
 
 	hu->priv = mrvl;
 
-	if (hu->serdev) {
-		ret = serdev_device_open(hu->serdev);
-		if (ret)
-			goto err;
-	}
-
 	return 0;
-err:
-	kfree(mrvl);
-
-	return ret;
 }
 
 static int mrvl_close(struct hci_uart *hu)
@@ -94,9 +83,6 @@ static int mrvl_close(struct hci_uart *hu)
 
 	BT_DBG("hu %p", hu);
 
-	if (hu->serdev)
-		serdev_device_close(hu->serdev);
-
 	skb_queue_purge(&mrvl->txq);
 	skb_queue_purge(&mrvl->rawq);
 	kfree_skb(mrvl->rx_skb);
-- 
2.39.5

RE: Bluetooth: hci_mrvl: Fix duplicate serdev open/close

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 2077 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/1211439/

---Test result---

Test Summary:
CheckPatch                    FAIL      0.66 seconds
VerifyFixes                   PASS      0.13 seconds
VerifySignedoff               PASS      0.13 seconds
GitLint                       PASS      0.32 seconds
SubjectPrefix                 PASS      0.13 seconds
BuildKernel                   PASS      23.28 seconds
CheckAllWarning               PASS      26.65 seconds
CheckSparse                   WARNING   28.66 seconds
BuildKernel32                 PASS      23.36 seconds
CheckKernelLLVM               PASS      24.29 seconds
TestRunnerSetup               PASS      550.59 seconds
IncrementalBuild              PASS      22.06 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
[1/1] Bluetooth: hci_mrvl: Leave the serdev port lifetime to hci_serdev
WARNING: Non-standard signature: Assisted-by:
#94: 
Assisted-by: LLM

ERROR: Unrecognized email address: 'LLM'
#94: 
Assisted-by: LLM

total: 1 errors, 1 warnings, 33 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

/home/runner/work/bluetooth-next/bluetooth-next/src/patch/14984056.patch has style problems, please review.

NOTE: Ignored message types: UNKNOWN_COMMIT_ID

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.


##############################
Test: CheckSparse - WARNING
Desc: Run sparse tool with linux kernel
Output:
drivers/bluetooth/hci_mrvl.c:156:23: warning: restricted __le16 degrades to integerdrivers/bluetooth/hci_mrvl.c:189:23: warning: restricted __le16 degrades to integer


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

---
Regards,
Linux Bluetooth

Re: [PATCH 0/1] Bluetooth: hci_mrvl: Fix duplicate serdev open/close

[Markus Probst]

[-- Attachment #1: Type: text/plain, Size: 769 bytes --]

On Sat, 2026-11-14 at 16:15 +0000, Michael McAllister wrote:
> Hi Markus, thanks for flagging this!
> 
> You're right, hci_serdev already opens and closes the port, so the extra
> calls in the Marvell driver aren't needed. This removes them as a
> separate fix.
> 
> I've build-tested it and checked that the duplicate-open reproducer
> passes with the fix, but I don't have Marvell hardware to test on.
> 
> Thanks,
> Michael

Hi,

Thanks for the patch!

I added some more people from the git history (incl. from existing
device trees that have this hardware) to CC here in the hope that they
can test it.

If nobody tests it, there is still be linux-next. But its generally
better if it gets tested once before that.

Thanks
- Markus Probst

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 870 bytes --]

[PATCH 1/4] Bluetooth: qca: Restore flow control after a failed power pulse

[Michael McAllister]

qca_send_power_pulse() disables hardware flow control before writing to
the UART. If the write fails, it returns without restoring flow control.

Restore flow control on the error path as on success.

Fixes: f9558270b85c ("Bluetooth: hci_qca: use wait_until_sent() for power pulses")
Signed-off-by: Michael McAllister <me@michaelmcallister.org>
---
 drivers/bluetooth/hci_qca.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index 7089e9b63..8705c4543 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -1424,6 +1424,7 @@ static int qca_send_power_pulse(struct hci_uart *hu, bool on)
 	ret = serdev_device_write_buf(hu->serdev, &cmd, sizeof(cmd));
 	if (ret < 0) {
 		bt_dev_err(hu->hdev, "failed to send power pulse %02x", cmd);
+		hci_uart_set_flow_control(hu, false);
 		return ret;
 	}
 

[PATCH 4/4] Bluetooth: qca: Power off WCN3950

[Michael McAllister]

Turning Bluetooth off on the Moto G10 leaves the WCN3950 regulator
votes enabled. qca_power_off() handles the other WCN39xx controllers
but omits WCN3950 from both switches.

Add it to the existing power-off pulse and regulator-disable paths.

Fixes: d5712c511cb3 ("Bluetooth: qca: add WCN3950 support")
Signed-off-by: Michael McAllister <me@michaelmcallister.org>
---
 drivers/bluetooth/hci_qca.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index 3d688ab75..540014182 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -2249,6 +2249,7 @@ static void qca_power_off(struct hci_uart *hu)
 	power = qcadev->bt_power;
 
 	switch (soc_type) {
+	case QCA_WCN3950:
 	case QCA_WCN3988:
 	case QCA_WCN3990:
 	case QCA_WCN3991:
@@ -2269,6 +2270,7 @@ static void qca_power_off(struct hci_uart *hu)
         }
 
 	switch (soc_type) {
+	case QCA_WCN3950:
 	case QCA_WCN3988:
 	case QCA_WCN3990:
 	case QCA_WCN3991:

[PATCH 3/4] Bluetooth: qca: Power off after unregistering the HCI device

[Michael McAllister]

qca_serdev_remove() powers off the controller while HCI setup and
transmission can still be running. Leave power-off to qca_close(), which
runs after HCI unregistration and write-work cancellation, while the
serial transport is still available.

Use the serial port's open state for the power-pulse check. PROTO_READY
is already clear during protocol cleanup, even when the port is open.
The open-state check also skips serial I/O after a failed setup reopen.

Mark a newly allocated serdev controller as off until power-on is tried,
so registration failures do not send a power pulse before HCI allocation.

Signed-off-by: Michael McAllister <me@michaelmcallister.org>
---
 drivers/bluetooth/hci_qca.c | 23 ++---------------------
 1 file changed, 2 insertions(+), 21 deletions(-)

diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index 3d9e6cd15..3d688ab75 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -619,6 +619,7 @@ static int qca_open(struct hci_uart *hu)
 	hu->priv = qca;
 
 	if (hu->serdev) {
+		set_bit(QCA_BT_OFF, &qca->flags);
 		qcadev = serdev_device_get_drvdata(hu->serdev);
 
 		switch (qcadev->btsoc_type) {
@@ -2252,11 +2253,7 @@ static void qca_power_off(struct hci_uart *hu)
 	case QCA_WCN3990:
 	case QCA_WCN3991:
 	case QCA_WCN3998:
-		/* Both of these write to the serial port which may have
-		 * already been closed by hci_uart_close(), which closes
-		 * the port if HCI_QUIRK_NON_PERSISTENT_SETUP is set.
-		 */
-		if (test_bit(HCI_UART_PROTO_READY, &hu->flags)) {
+		if (hu->serdev_open) {
 			host_set_baudrate(hu, 2400);
 			qca_send_power_pulse(hu, false);
 		}
@@ -2580,22 +2577,6 @@ static int qca_serdev_probe(struct serdev_device *serdev)
 static void qca_serdev_remove(struct serdev_device *serdev)
 {
 	struct qca_serdev *qcadev = serdev_device_get_drvdata(serdev);
-	struct qca_power *power = qcadev->bt_power;
-
-	switch (qcadev->btsoc_type) {
-	case QCA_WCN3988:
-	case QCA_WCN3990:
-	case QCA_WCN3991:
-	case QCA_WCN3998:
-	case QCA_WCN6750:
-	case QCA_WCN6855:
-	case QCA_WCN7850:
-		if (power->vregs_on)
-			qca_power_off(&qcadev->serdev_hu);
-		break;
-	default:
-		break;
-	}
 
 	hci_uart_unregister_device(&qcadev->serdev_hu);
 }

[PATCH v2 0/4] Bluetooth: qca: Power off WCN3950

[Michael McAllister]

Hi,

Thanks for the review. I've split this into four patches, with the
Marvell fix kept separate. I've tested a backport on my Moto G10,
including power cycling and unbind/rebind with Bluetooth both on and
off.

Thanks,
Michael

---
base-commit: da3cc02587c629420603df4395179ec82955c941
prerequisite-patch-id: 517b9cfef6a60777b0869a8f46d4c07be7f77c13

[PATCH v2 2/4] Bluetooth: qca: Power off after unregistering the HCI device

[Michael McAllister]

qca_serdev_remove() powers off while HCI setup and UART writes can still
be running. Leave power-off to qca_close(), after HCI unregistration and
write-work cancellation, while the serial transport is still available.

Check the actual port state before sending the power-off pulse, since
PROTO_READY is already clear during protocol cleanup. Start serdev
controllers in QCA_BT_OFF so registration failure does not send a pulse
before HCI allocation or power-on.

Drain HCI setup before clearing protocol readiness, so a setup reopen
cannot re-enable UART write work after cancellation. Leave HCI flush
and close callbacks to the transport teardown during unregister; the
port stays open until protocol close has sent the power-off pulse.

Flush controller TX after stopping UART work, before protocol and port
close, so previously accepted bytes are discarded rather than drained.

Signed-off-by: Michael McAllister <me@michaelmcallister.org>
---
 drivers/bluetooth/hci_qca.c    | 24 ++----------------------
 drivers/bluetooth/hci_serdev.c | 18 ++++++++++++++----
 2 files changed, 16 insertions(+), 26 deletions(-)

diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index 28e84fc24..ddbeb0787 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -620,6 +620,7 @@ static int qca_open(struct hci_uart *hu)
 	hu->priv = qca;
 
 	if (hu->serdev) {
+		set_bit(QCA_BT_OFF, &qca->flags);
 		qcadev = serdev_device_get_drvdata(hu->serdev);
 
 		switch (qcadev->btsoc_type) {
@@ -2285,11 +2286,7 @@ static void qca_power_off(struct hci_uart *hu)
 	case QCA_WCN3990:
 	case QCA_WCN3991:
 	case QCA_WCN3998:
-		/* Both of these write to the serial port which may have
-		 * already been closed by hci_uart_close(), which closes
-		 * the port if HCI_QUIRK_NON_PERSISTENT_SETUP is set.
-		 */
-		if (test_bit(HCI_UART_PROTO_READY, &hu->flags)) {
+		if (hu->serdev_open) {
 			host_set_baudrate(hu, 2400);
 			qca_send_power_pulse(hu, false);
 		}
@@ -2618,23 +2615,6 @@ static int qca_serdev_probe(struct serdev_device *serdev)
 static void qca_serdev_remove(struct serdev_device *serdev)
 {
 	struct qca_serdev *qcadev = serdev_device_get_drvdata(serdev);
-	struct qca_power *power = qcadev->bt_power;
-
-	switch (qcadev->btsoc_type) {
-	case QCA_WCN3988:
-	case QCA_WCN3990:
-	case QCA_WCN3991:
-	case QCA_WCN3998:
-	case QCA_WCN6750:
-	case QCA_QCA2066:
-	case QCA_WCN6855:
-	case QCA_WCN7850:
-		if (power->vregs_on)
-			qca_power_off(&qcadev->serdev_hu);
-		break;
-	default:
-		break;
-	}
 
 	hci_uart_unregister_device(&qcadev->serdev_hu);
 }
diff --git a/drivers/bluetooth/hci_serdev.c b/drivers/bluetooth/hci_serdev.c
index 3d10d8d75..e279a7d61 100644
--- a/drivers/bluetooth/hci_serdev.c
+++ b/drivers/bluetooth/hci_serdev.c
@@ -120,6 +120,9 @@ static int hci_uart_flush(struct hci_dev *hdev)
 
 	BT_DBG("hdev %p serdev %p", hdev, hu->serdev);
 
+	if (hci_dev_test_flag(hdev, HCI_UNREGISTER))
+		return 0;
+
 	if (hu->tx_skb) {
 		kfree_skb(hu->tx_skb); hu->tx_skb = NULL;
 	}
@@ -167,6 +170,9 @@ static int hci_uart_close(struct hci_dev *hdev)
 
 	BT_DBG("hdev %p", hdev);
 
+	if (hci_dev_test_flag(hdev, HCI_UNREGISTER))
+		return 0;
+
 	if (!test_bit(HCI_UART_PROTO_READY, &hu->flags))
 		return 0;
 
@@ -425,6 +431,10 @@ void hci_uart_unregister_device(struct hci_uart *hu)
 	/* Wait for init_ready to finish to prevent registration races */
 	cancel_work_sync(&hu->init_ready);
 
+	/* Drain HCI setup before clearing protocol readiness. */
+	if (test_bit(HCI_UART_REGISTERED, &hu->flags))
+		hci_unregister_dev(hdev);
+
 	proto_ready = test_bit(HCI_UART_PROTO_READY, &hu->flags);
 	if (proto_ready) {
 		/* Clear HCI_UART_PROTO_READY under the write lock so a
@@ -446,16 +456,16 @@ void hci_uart_unregister_device(struct hci_uart *hu)
 	cancel_work_sync(&hu->write_work);
 
 	/* Free any partially transmitted frame left over by write_work now
-	 * that the transmit path is fully quiesced. hci_uart_close() would
-	 * skip hci_uart_flush() because HCI_UART_PROTO_READY is cleared.
+	 * that the transmit path is fully quiesced. The HCI close and flush
+	 * callbacks leave transport teardown to us during unregister.
 	 */
 	if (hu->tx_skb) {
 		kfree_skb(hu->tx_skb);
 		hu->tx_skb = NULL;
 	}
 
-	if (test_bit(HCI_UART_REGISTERED, &hu->flags))
-		hci_unregister_dev(hdev);
+	if (hu->serdev_open)
+		serdev_device_write_flush(hu->serdev);
 
 	/* Close the protocol before freeing hdev (intrinsically purges queues).
 	 * Some protocol close handlers (e.g. qca_close) may still access the
-- 
2.39.5

[PATCH v2 3/4] Bluetooth: qca: Handle failed power-pulse writes

[Michael McAllister]

A failed UART write leaves hardware flow control disabled. A zero-byte
write is also treated as success even though no pulse was queued.

Require the one-byte pulse to be accepted and restore flow control on
the error path, returning -EIO for a zero-byte write.

Fixes: f9558270b85c ("Bluetooth: hci_qca: use wait_until_sent() for power pulses")

Signed-off-by: Michael McAllister <me@michaelmcallister.org>
---
 drivers/bluetooth/hci_qca.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index ddbeb0787..6642af781 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -1430,9 +1430,10 @@ static int qca_send_power_pulse(struct hci_uart *hu, bool on)
 	serdev_device_write_flush(hu->serdev);
 	hci_uart_set_flow_control(hu, true);
 	ret = serdev_device_write_buf(hu->serdev, &cmd, sizeof(cmd));
-	if (ret < 0) {
+	if (ret != sizeof(cmd)) {
 		bt_dev_err(hu->hdev, "failed to send power pulse %02x", cmd);
-		return ret;
+		hci_uart_set_flow_control(hu, false);
+		return ret < 0 ? ret : -EIO;
 	}
 
 	serdev_device_wait_until_sent(hu->serdev, timeout);
-- 
2.39.5

[PATCH v2 1/4] Bluetooth: hci_serdev: Track whether the serial port is open

[Michael McAllister]

HCI_UART_PROTO_READY is cleared before protocol close during unregister,
even though the serial port stays open. QCA needs to distinguish that
case from a port left closed by a failed setup reopen before sending
its power-off pulse.

Track the port state in the transport and QCA open/close helpers. Keep
the existing PROTO_READY handling for setup failures and use the actual
open state when closing the port during unregister.

Signed-off-by: Michael McAllister <me@michaelmcallister.org>
---
 drivers/bluetooth/hci_qca.c    |  4 ++--
 drivers/bluetooth/hci_serdev.c | 34 ++++++++++++++++++++++++++++------
 drivers/bluetooth/hci_uart.h   |  3 +++
 3 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index d693e2842..28e84fc24 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -1762,14 +1762,14 @@ static void qca_port_close(struct hci_uart *hu)
 	 * hci_uart_close() or hci_uart_unregister_device().
 	 */
 	if (test_and_clear_bit(HCI_UART_PROTO_READY, &hu->flags))
-		serdev_device_close(hu->serdev);
+		hci_uart_serdev_close(hu);
 }
 
 static int qca_port_open(struct hci_uart *hu)
 {
 	int ret;
 
-	ret = serdev_device_open(hu->serdev);
+	ret = hci_uart_serdev_open(hu);
 	if (!ret)
 		set_bit(HCI_UART_PROTO_READY, &hu->flags);
 
diff --git a/drivers/bluetooth/hci_serdev.c b/drivers/bluetooth/hci_serdev.c
index d2c48df34..3d10d8d75 100644
--- a/drivers/bluetooth/hci_serdev.c
+++ b/drivers/bluetooth/hci_serdev.c
@@ -21,6 +21,29 @@
 
 #include "hci_uart.h"
 
+int hci_uart_serdev_open(struct hci_uart *hu)
+{
+	int err;
+
+	if (hu->serdev_open)
+		return 0;
+
+	err = serdev_device_open(hu->serdev);
+	if (!err)
+		hu->serdev_open = true;
+
+	return err;
+}
+
+void hci_uart_serdev_close(struct hci_uart *hu)
+{
+	if (!hu->serdev_open)
+		return;
+
+	hu->serdev_open = false;
+	serdev_device_close(hu->serdev);
+}
+
 static inline void hci_uart_tx_complete(struct hci_uart *hu, int pkt_type)
 {
 	struct hci_dev *hdev = hu->hdev;
@@ -125,7 +148,7 @@ static int hci_uart_open(struct hci_dev *hdev)
 	 * BT OFF. Upon next BT ON UART port should be opened.
 	 */
 	if (!test_bit(HCI_UART_PROTO_READY, &hu->flags)) {
-		err = serdev_device_open(hu->serdev);
+		err = hci_uart_serdev_open(hu);
 		if (err)
 			return err;
 		set_bit(HCI_UART_PROTO_READY, &hu->flags);
@@ -156,7 +179,7 @@ static int hci_uart_close(struct hci_dev *hdev)
 	 */
 	if (hci_test_quirk(hdev, HCI_QUIRK_NON_PERSISTENT_SETUP)) {
 		clear_bit(HCI_UART_PROTO_READY, &hu->flags);
-		serdev_device_close(hu->serdev);
+		hci_uart_serdev_close(hu);
 	}
 
 	return 0;
@@ -316,7 +339,7 @@ int hci_uart_register_device_priv(struct hci_uart *hu,
 	if (percpu_init_rwsem(&hu->proto_lock))
 		return -ENOMEM;
 
-	err = serdev_device_open(hu->serdev);
+	err = hci_uart_serdev_open(hu);
 	if (err)
 		goto err_rwsem;
 
@@ -387,7 +410,7 @@ int hci_uart_register_device_priv(struct hci_uart *hu,
 	clear_bit(HCI_UART_PROTO_READY, &hu->flags);
 	p->close(hu);
 err_open:
-	serdev_device_close(hu->serdev);
+	hci_uart_serdev_close(hu);
 err_rwsem:
 	percpu_free_rwsem(&hu->proto_lock);
 	return err;
@@ -440,8 +463,7 @@ void hci_uart_unregister_device(struct hci_uart *hu)
 	 */
 	hu->proto->close(hu);
 
-	if (proto_ready)
-		serdev_device_close(hu->serdev);
+	hci_uart_serdev_close(hu);
 
 	hci_free_dev(hdev);
 
diff --git a/drivers/bluetooth/hci_uart.h b/drivers/bluetooth/hci_uart.h
index 7fbe8dffa..2f8e346f3 100644
--- a/drivers/bluetooth/hci_uart.h
+++ b/drivers/bluetooth/hci_uart.h
@@ -66,6 +66,7 @@ struct hci_uart_proto {
 struct hci_uart {
 	struct tty_struct	*tty;
 	struct serdev_device	*serdev;
+	bool			serdev_open;
 	struct hci_dev		*hdev;
 	unsigned long		flags;
 	unsigned long		hdev_flags;
@@ -112,6 +113,8 @@ static inline int hci_uart_register_device(struct hci_uart *hu,
 }
 
 void hci_uart_unregister_device(struct hci_uart *hu);
+int hci_uart_serdev_open(struct hci_uart *hu);
+void hci_uart_serdev_close(struct hci_uart *hu);
 
 int hci_uart_tx_wakeup(struct hci_uart *hu);
 int hci_uart_wait_until_sent(struct hci_uart *hu);
-- 
2.39.5

RE: [v2,1/4] Bluetooth: hci_serdev: Track whether the serial port is open

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1020 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/1211540/

---Test result---

Test Summary:
CheckPatch                    PASS      3.32 seconds
VerifyFixes                   PASS      0.13 seconds
VerifySignedoff               PASS      0.13 seconds
GitLint                       PASS      1.33 seconds
SubjectPrefix                 PASS      0.51 seconds
BuildKernel                   PASS      24.01 seconds
CheckAllWarning               PASS      27.63 seconds
CheckSparse                   PASS      30.16 seconds
BuildKernel32                 PASS      24.23 seconds
CheckKernelLLVM               PASS      24.51 seconds
TestRunnerSetup               PASS      556.91 seconds
IncrementalBuild              PASS      28.40 seconds



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

---
Regards,
Linux Bluetooth

[PATCH v2 4/4] Bluetooth: qca: Power off WCN3950

[Michael McAllister]

Turning Bluetooth off on the Moto G10 leaves the WCN3950 regulator
votes enabled. qca_power_off() handles the other WCN39xx controllers
but omits WCN3950 from both switches.

Add it to the existing power-off pulse and regulator-disable paths.

Fixes: d5712c511cb3 ("Bluetooth: qca: add WCN3950 support")

Signed-off-by: Michael McAllister <me@michaelmcallister.org>
---
 drivers/bluetooth/hci_qca.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index 6642af781..bd8cfe95c 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -2283,6 +2283,7 @@ static void qca_power_off(struct hci_uart *hu)
 	power = qcadev->bt_power;
 
 	switch (soc_type) {
+	case QCA_WCN3950:
 	case QCA_WCN3988:
 	case QCA_WCN3990:
 	case QCA_WCN3991:
@@ -2303,6 +2304,7 @@ static void qca_power_off(struct hci_uart *hu)
         }
 
 	switch (soc_type) {
+	case QCA_WCN3950:
 	case QCA_WCN3988:
 	case QCA_WCN3990:
 	case QCA_WCN3991:
-- 
2.39.5