[PATCH 0/2] Bluetooth: hci_qca: Add WCN clock enable/disable support for Shikra

[PATCH 0/2] Bluetooth: hci_qca: Add WCN clock enable/disable support for Shikra

[Yepuri Siddu]

clk_smd_rpm_handoff() votes both active and sleep RPM resource states for
every clock, keeping them non-zero until a consumer takes over. On Shikra,
the BT WCN clock was surviving on these proxy votes. With proxy vote
removal on QCM2290 [1], the sleep vote is no longer held, which means BT
must explicitly vote for its WCN clock during enable and release it during
disable to avoid keeping the clock active in idle.

This series adds WCN clock management to the hci_qca driver for the
pwrseq-based power path and enables it for Shikra boards. Targets that
do not define a clock in DTS are unaffected.

Validations:
- BT enable/disable verified on Shikra CQM, CQS and IQS EVK boards.
- Confirmed WCN clock voted on BT enable and released on BT disable.

As RPMCC proxy votes are being removed for QCM2290 [1], BT can no longer
rely on them to keep the WCN clock active. This series ensures BT
explicitly enables the WCN clock on power-on and disables it on power-off.

[1] https://lore.kernel.org/all/20260910-clk-smd-rpm-skip-proxy-v1-1-1cb5694a99d9@oss.qualcomm.com/

Signed-off-by: Yepuri Siddu <yepuri.siddu@oss.qualcomm.com>
---
Yepuri Siddu (2):
      Bluetooth: hci_qca: Add WCN clock management for pwrseq-based power path
      arm64: dts: qcom: shikra: Add WCN clock to Bluetooth node

 arch/arm64/boot/dts/qcom/shikra-cqm-som.dtsi |  2 ++
 arch/arm64/boot/dts/qcom/shikra-iqs-som.dtsi |  2 ++
 drivers/bluetooth/hci_qca.c                  | 23 ++++++++++++++++++++---
 3 files changed, 24 insertions(+), 3 deletions(-)
---
base-commit: 165768bb70265b5c38cf0b73fafd75be235f8b14
change-id: 20260925-bt-wcn-clk-enable-9de068334aa1

Best regards,
--  
Yepuri Siddu <yepuri.siddu@oss.qualcomm.com>

[PATCH 1/2] Bluetooth: hci_qca: Add WCN clock management for pwrseq-based power path

[Yepuri Siddu]

RPMCC previously kept WCN clocks enabled via proxy votes, so the pwrseq
power path in hci_qca did not need to explicitly manage the clock.
With proxy vote removal, each consumer must explicitly enable and disable
its required clocks.

Extend the pwrseq-based power path to acquire and manage an optional
WCN clock. In qca_serdev_probe(), acquire the clock using
devm_clk_get_optional(). In qca_regulator_enable(), enable the clock
after a successful pwrseq_enable() with proper rollback on failure.
In qca_power_off(), disable the clock before pwrseq_disable().

Targets that do not define a clock in DTS are unaffected since
devm_clk_get_optional() returns NULL and all clock operations are
guarded accordingly.

Signed-off-by: Yepuri Siddu <yepuri.siddu@oss.qualcomm.com>
---
 drivers/bluetooth/hci_qca.c | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index 7089e9b639b2..489a059e519a 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -2265,6 +2265,8 @@ static void qca_power_off(struct hci_uart *hu)
 	}
 
 	if (power && power->pwrseq) {
+		if (qcadev->susclk)
+			clk_disable_unprepare(qcadev->susclk);
 		pwrseq_disable(power->pwrseq);
 		set_bit(QCA_BT_OFF, &qca->flags);
 		return;
@@ -2324,8 +2326,17 @@ static int qca_regulator_enable(struct qca_serdev *qcadev)
 	struct qca_power *power = qcadev->bt_power;
 	int ret;
 
-	if (power->pwrseq)
-		return pwrseq_enable(power->pwrseq);
+	if (power->pwrseq) {
+		ret = pwrseq_enable(power->pwrseq);
+		if (ret)
+			return ret;
+		if (qcadev->susclk) {
+			ret = clk_prepare_enable(qcadev->susclk);
+			if (ret)
+				pwrseq_disable(power->pwrseq);
+		}
+		return ret;
+	}
 
 	/* Already enabled */
 	if (power->vregs_on)
@@ -2472,8 +2483,14 @@ static int qca_serdev_probe(struct serdev_device *serdev)
 			 */
 			if (IS_ERR(qcadev->bt_power->pwrseq))
 				qcadev->bt_power->pwrseq = NULL;
-			else
+			else {
+				qcadev->susclk = devm_clk_get_optional(&serdev->dev, NULL);
+				if (IS_ERR(qcadev->susclk)) {
+					dev_err(&serdev->dev, "failed to acquire clk\n");
+					return PTR_ERR(qcadev->susclk);
+				}
 				break;
+			}
 		}
 
 		qcadev->bt_power->dev = &serdev->dev;

-- 
2.34.1

Re: [PATCH 1/2] Bluetooth: hci_qca: Add WCN clock management for pwrseq-based power path

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] bluetooth: hci_qca: unbalanced clock disable in error paths
--

commit b40a50081c6337711ee58ffef1a77bff3162c4ae
Author: Yepuri Siddu <yepuri.siddu@oss.qualcomm.com>

Bluetooth: hci_qca: Add WCN clock management for pwrseq-based power path

This commit extends the pwrseq-based power path in the hci_qca driver to
acquire and manage an optional WCN clock. It updates qca_serdev_probe() to
acquire the clock, and modifies qca_regulator_enable() and qca_power_off()
to manage the clock state alongside the power sequencer.

> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
> index 7089e9b639b2a..489a059e519a6 100644
> --- a/drivers/bluetooth/hci_qca.c
> +++ b/drivers/bluetooth/hci_qca.c
> @@ -2265,6 +2265,8 @@ static void qca_power_off(struct hci_uart *hu)
>  	}
>  
>  	if (power && power->pwrseq) {
> +		if (qcadev->susclk)
> +			clk_disable_unprepare(qcadev->susclk);
>  		pwrseq_disable(power->pwrseq);

[Severity: High]
Does this code unconditionally disable the clock and power sequencer without
checking if they were actually enabled?

Unlike the regular regulator path which uses power->vregs_on for state
tracking, this pwrseq teardown lacks state tracking. This causes unbalanced
disables in two paths:

1) The module is unloaded without the HCI interface ever being brought up.
qca_close() calls qca_power_off() (since QCA_BT_OFF is initialized to 0),
which unconditionally disables the never-enabled clock and pwrseq.

2) In qca_setup(), if qca_power_on() fails (e.g., clk_prepare_enable()
fails), the error handler calls qca_power_off() which performs an unconditional
teardown despite the rollback already executed in qca_regulator_enable().

This can trigger a WARN in the clock framework due to an unbalanced
clk_disable_unprepare() and cause an unbalanced pwrseq_disable(), which crashes
the kernel if panic_on_warn is set. This is reachable by a local privileged user
loading and unloading the driver, or automatically on hardware initialization
failure.

>  		set_bit(QCA_BT_OFF, &qca->flags);
>  		return;
>  	}

[ ... ]

> @@ -2324,8 +2326,17 @@ static int qca_regulator_enable(struct qca_serdev *qcadev)
>  	struct qca_power *power = qcadev->bt_power;
>  	int ret;
>  
> -	if (power->pwrseq)
> -		return pwrseq_enable(power->pwrseq);
> +	if (power->pwrseq) {
> +		ret = pwrseq_enable(power->pwrseq);
> +		if (ret)
> +			return ret;
> +		if (qcadev->susclk) {
> +			ret = clk_prepare_enable(qcadev->susclk);
> +			if (ret)
> +				pwrseq_disable(power->pwrseq);

[Severity: High]
Does this rollback leave the power sequencer and clock in an untracked state?

If clk_prepare_enable() fails, pwrseq_disable() is correctly called to roll
back the sequence, returning the error to the caller. However, because there
is no tracking to indicate the components are disabled, the error handler
(which calls qca_power_off()) will attempt to disable them a second time,
triggering the unbalanced disables mentioned above.

> +		}
> +		return ret;
> +	}
>  
>  	/* Already enabled */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260925-bt-wcn-clk-enable-v1-0-30bee88c4de4@oss.qualcomm.com?part=1

Re: [PATCH 1/2] Bluetooth: hci_qca: Add WCN clock management for pwrseq-based power path

[Loic Poulain]

On Fri, Sep 25, 2026 at 8:21 AM Yepuri Siddu
<yepuri.siddu@oss.qualcomm.com> wrote:
>
> RPMCC previously kept WCN clocks enabled via proxy votes, so the pwrseq
> power path in hci_qca did not need to explicitly manage the clock.
> With proxy vote removal, each consumer must explicitly enable and disable
> its required clocks.
>
> Extend the pwrseq-based power path to acquire and manage an optional
> WCN clock. In qca_serdev_probe(), acquire the clock using
> devm_clk_get_optional(). In qca_regulator_enable(), enable the clock
> after a successful pwrseq_enable() with proper rollback on failure.
> In qca_power_off(), disable the clock before pwrseq_disable().
>
> Targets that do not define a clock in DTS are unaffected since
> devm_clk_get_optional() returns NULL and all clock operations are
> guarded accordingly.
>
> Signed-off-by: Yepuri Siddu <yepuri.siddu@oss.qualcomm.com>
> ---
>  drivers/bluetooth/hci_qca.c | 23 ++++++++++++++++++++---
>  1 file changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
> index 7089e9b639b2..489a059e519a 100644
> --- a/drivers/bluetooth/hci_qca.c
> +++ b/drivers/bluetooth/hci_qca.c
> @@ -2265,6 +2265,8 @@ static void qca_power_off(struct hci_uart *hu)
>         }
>
>         if (power && power->pwrseq) {
> +               if (qcadev->susclk)
> +                       clk_disable_unprepare(qcadev->susclk);
>                 pwrseq_disable(power->pwrseq);
>                 set_bit(QCA_BT_OFF, &qca->flags);
>                 return;
> @@ -2324,8 +2326,17 @@ static int qca_regulator_enable(struct qca_serdev *qcadev)
>         struct qca_power *power = qcadev->bt_power;
>         int ret;
>
> -       if (power->pwrseq)
> -               return pwrseq_enable(power->pwrseq);
> +       if (power->pwrseq) {
> +               ret = pwrseq_enable(power->pwrseq);
> +               if (ret)
> +                       return ret;
> +               if (qcadev->susclk) {
> +                       ret = clk_prepare_enable(qcadev->susclk);

Why is susclk guarded by the pwrseq? They seem unrelated to me.

> +                       if (ret)
> +                               pwrseq_disable(power->pwrseq);
> +               }
> +               return ret;
> +       }
>
>         /* Already enabled */
>         if (power->vregs_on)
> @@ -2472,8 +2483,14 @@ static int qca_serdev_probe(struct serdev_device *serdev)
>                          */
>                         if (IS_ERR(qcadev->bt_power->pwrseq))
>                                 qcadev->bt_power->pwrseq = NULL;
> -                       else
> +                       else {
> +                               qcadev->susclk = devm_clk_get_optional(&serdev->dev, NULL);
> +                               if (IS_ERR(qcadev->susclk)) {
> +                                       dev_err(&serdev->dev, "failed to acquire clk\n");
> +                                       return PTR_ERR(qcadev->susclk);
> +                               }
>                                 break;
> +                       }
>                 }
>
>                 qcadev->bt_power->dev = &serdev->dev;
>
> --
> 2.34.1
>
>

Re: [PATCH 1/2] Bluetooth: hci_qca: Add WCN clock management for pwrseq-based power path

[Yepuri Siddu]

On 9/29/2026 3:33 PM, Loic Poulain wrote:
> On Fri, Sep 25, 2026 at 8:21 AM Yepuri Siddu
> <yepuri.siddu@oss.qualcomm.com> wrote:
>>
>> RPMCC previously kept WCN clocks enabled via proxy votes, so the pwrseq
>> power path in hci_qca did not need to explicitly manage the clock.
>> With proxy vote removal, each consumer must explicitly enable and disable
>> its required clocks.
>>
>> Extend the pwrseq-based power path to acquire and manage an optional
>> WCN clock. In qca_serdev_probe(), acquire the clock using
>> devm_clk_get_optional(). In qca_regulator_enable(), enable the clock
>> after a successful pwrseq_enable() with proper rollback on failure.
>> In qca_power_off(), disable the clock before pwrseq_disable().
>>
>> Targets that do not define a clock in DTS are unaffected since
>> devm_clk_get_optional() returns NULL and all clock operations are
>> guarded accordingly.
>>
>> Signed-off-by: Yepuri Siddu <yepuri.siddu@oss.qualcomm.com>
>> ---
>>   drivers/bluetooth/hci_qca.c | 23 ++++++++++++++++++++---
>>   1 file changed, 20 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
>> index 7089e9b639b2..489a059e519a 100644
>> --- a/drivers/bluetooth/hci_qca.c
>> +++ b/drivers/bluetooth/hci_qca.c
>> @@ -2265,6 +2265,8 @@ static void qca_power_off(struct hci_uart *hu)
>>          }
>>
>>          if (power && power->pwrseq) {
>> +               if (qcadev->susclk)
>> +                       clk_disable_unprepare(qcadev->susclk);
>>                  pwrseq_disable(power->pwrseq);
>>                  set_bit(QCA_BT_OFF, &qca->flags);
>>                  return;
>> @@ -2324,8 +2326,17 @@ static int qca_regulator_enable(struct qca_serdev *qcadev)
>>          struct qca_power *power = qcadev->bt_power;
>>          int ret;
>>
>> -       if (power->pwrseq)
>> -               return pwrseq_enable(power->pwrseq);
>> +       if (power->pwrseq) {
>> +               ret = pwrseq_enable(power->pwrseq);
>> +               if (ret)
>> +                       return ret;
>> +               if (qcadev->susclk) {
>> +                       ret = clk_prepare_enable(qcadev->susclk);
> 
> Why is susclk guarded by the pwrseq? They seem unrelated to me.

You are right. On further thought, the clock belongs to the WCN hardware
and its power sequencing is already handled by the wcn3988-pmu driver.
So the hci_qca driver does not need to manage it at all.

In v2 we dropped the hci_qca changes and moved the clock property to the
wcn3988-pmu node in DTS instead:
https://lore.kernel.org/all/20260929-bt-wcn-clk-enable-v2-1-7a90902f3df5@oss.qualcomm.com/

Thanks,
Siddu

[PATCH 2/2] arm64: dts: qcom: shikra: Add WCN clock to Bluetooth node

[Yepuri Siddu]

With RPMCC proxy vote removal, the Bluetooth node must explicitly
declare its WCN clock dependency so the hci_qca driver can vote for
it during BT power on/off via the pwrseq path.

CQM SOM uses RPM_SMD_RF_CLK2 while IQS SOM uses RPM_SMD_RF_CLK1,
reflecting the WCN clock assignments on each board variant.

Signed-off-by: Yepuri Siddu <yepuri.siddu@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/shikra-cqm-som.dtsi | 2 ++
 arch/arm64/boot/dts/qcom/shikra-iqs-som.dtsi | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/shikra-cqm-som.dtsi b/arch/arm64/boot/dts/qcom/shikra-cqm-som.dtsi
index 8ac42ff625a0..4e03e459c7c1 100644
--- a/arch/arm64/boot/dts/qcom/shikra-cqm-som.dtsi
+++ b/arch/arm64/boot/dts/qcom/shikra-cqm-som.dtsi
@@ -220,6 +220,8 @@ bluetooth {
 		vddxo-supply = <&vreg_pmu_xo>;
 		vddrf-supply = <&vreg_pmu_rf>;
 		vddch0-supply = <&vreg_pmu_ch0>;
+
+		clocks = <&rpmcc RPM_SMD_RF_CLK2>;
 	};
 };
 
diff --git a/arch/arm64/boot/dts/qcom/shikra-iqs-som.dtsi b/arch/arm64/boot/dts/qcom/shikra-iqs-som.dtsi
index 4ff97945274d..227283ab74a8 100644
--- a/arch/arm64/boot/dts/qcom/shikra-iqs-som.dtsi
+++ b/arch/arm64/boot/dts/qcom/shikra-iqs-som.dtsi
@@ -242,6 +242,8 @@ bluetooth {
 		vddxo-supply = <&vreg_pmu_xo>;
 		vddrf-supply = <&vreg_pmu_rf>;
 		vddch0-supply = <&vreg_pmu_ch0>;
+
+		clocks = <&rpmcc RPM_SMD_RF_CLK1>;
 	};
 };
 

-- 
2.34.1

Re: [PATCH 2/2] arm64: dts: qcom: shikra: Add WCN clock to Bluetooth node

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] dt-bindings: incorrect WCN clock placement breaks WiFi power sequencing
- [High] bluetooth: hci_qca: unbalanced clock and pwrseq disable on error path
--

commit 508a8593da851a2bfa09fc351881cb48702acb33
Author: Yepuri Siddu <yepuri.siddu@oss.qualcomm.com>

arm64: dts: qcom: shikra: Add WCN clock to Bluetooth node

This commit explicitly declares the WCN clock dependency in the Bluetooth node
for the shikra CQM and IQS SOM boards. It is intended to allow the hci_qca
driver to vote for the clock during Bluetooth power sequencing.

> diff --git a/arch/arm64/boot/dts/qcom/shikra-cqm-som.dtsi b/arch/arm64/boot/dts/qcom/shikra-cqm-som.dtsi
> index 8ac42ff625a0e..4e03e459c7c16 100644
> --- a/arch/arm64/boot/dts/qcom/shikra-cqm-som.dtsi
> +++ b/arch/arm64/boot/dts/qcom/shikra-cqm-som.dtsi
> @@ -220,6 +220,8 @@ bluetooth {
>  		vddxo-supply = <&vreg_pmu_xo>;
>  		vddrf-supply = <&vreg_pmu_rf>;
>  		vddch0-supply = <&vreg_pmu_ch0>;
> +
> +		clocks = <&rpmcc RPM_SMD_RF_CLK2>;

[Severity: High]
Does this code place the WCN clock in the correct node?

The WCN PMU driver (drivers/power/sequencing/pwrseq-qcom-wcn.c) manages the
shared WCN RF clock for both Bluetooth and WLAN. It retrieves the clock from
its own PMU device node via devm_clk_get_optional(dev, NULL).

If the main RF clock is placed inside the bluetooth node instead of the
wcn3988-pmu node, the PMU driver's ctx->clk evaluates to NULL.

If WiFi is powered on while Bluetooth is off, will the PMU driver attempt to
enable the clock but do nothing, leaving the hardware clock disabled and
causing the WiFi subsystem to silently fail or crash?

>  	};
>  };
>  
> diff --git a/arch/arm64/boot/dts/qcom/shikra-iqs-som.dtsi b/arch/arm64/boot/dts/qcom/shikra-iqs-som.dtsi
> index 4ff97945274dd..227283ab74a81 100644
> --- a/arch/arm64/boot/dts/qcom/shikra-iqs-som.dtsi
> +++ b/arch/arm64/boot/dts/qcom/shikra-iqs-som.dtsi
> @@ -242,6 +242,8 @@ bluetooth {
>  		vddxo-supply = <&vreg_pmu_xo>;
>  		vddrf-supply = <&vreg_pmu_rf>;
>  		vddch0-supply = <&vreg_pmu_ch0>;
> +
> +		clocks = <&rpmcc RPM_SMD_RF_CLK1>;

[Severity: High]
Is the WCN clock placement correct here?

Similar to the CQM SOM, if the clock is placed in the bluetooth node instead
of the wcn3988-pmu node, will this deprive the PMU power sequencer of its
clock reference and break WiFi power sequencing?

>  	};
>  };

I also have a question regarding the pwrseq path mentioned in the commit
message.

drivers/bluetooth/hci_qca.c:qca_regulator_enable() {
    ...
		if (qcadev->susclk) {
			ret = clk_prepare_enable(qcadev->susclk);
			if (ret)
				pwrseq_disable(power->pwrseq);
		}
		return ret;
}

[Severity: High]
If clk_prepare_enable(qcadev->susclk) fails here, pwrseq_disable() is called
and the error is returned. This error propagates up through qca_power_on() to
qca_setup(). Upon failure, qca_setup() jumps to the out label and invokes
qca_power_off():

drivers/bluetooth/hci_qca.c:qca_power_off() {
    ...
	if (power && power->pwrseq) {
		if (qcadev->susclk)
			clk_disable_unprepare(qcadev->susclk);
		pwrseq_disable(power->pwrseq);
		set_bit(QCA_BT_OFF, &qca->flags);
		return;
	}
    ...
}

Because qca_power_off() lacks state tracking for the pwrseq path, will this
unconditionally execute clk_disable_unprepare(qcadev->susclk) and
pwrseq_disable(power->pwrseq) again?

Could this unbalanced clock unprepare trigger a kernel WARN, and could the
double pwrseq_disable() cause a reference count underflow, potentially leading
to crashes or resource corruption?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260925-bt-wcn-clk-enable-v1-0-30bee88c4de4@oss.qualcomm.com?part=2