[PATCH v1] driver: bluetooth: hci_qca:fix unable to load the BT driver

[PATCH v1] driver: bluetooth: hci_qca:fix unable to load the BT driver

[Shuai Zhang]

Some modules have BT_EN enabled via a hardware pull-up,
meaning it is not defined in the DTS and is not controlled
through the power sequence. In such cases, fall through
to follow the legacy flow.

Signed-off-by: Shuai Zhang <quic_shuaz@quicinc.com>
---
 drivers/bluetooth/hci_qca.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index a2dc39c00..976ec88a0 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -2392,10 +2392,17 @@ static int qca_serdev_probe(struct serdev_device *serdev)
 			 */
 			qcadev->bt_power->pwrseq = devm_pwrseq_get(&serdev->dev,
 								   "bluetooth");
-			if (IS_ERR(qcadev->bt_power->pwrseq))
-				return PTR_ERR(qcadev->bt_power->pwrseq);
 
-			break;
+			/*
+			 * Some modules have BT_EN enabled via a hardware pull-up,
+			 * meaning it is not defined in the DTS and is not controlled
+			 * through the power sequence. In such cases, fall through
+			 * to follow the legacy flow.
+			 */
+			if (IS_ERR(qcadev->bt_power->pwrseq))
+				qcadev->bt_power->pwrseq = NULL;
+			else
+				break;
 		}
 		fallthrough;
 	case QCA_WCN3950:
-- 
2.34.1

RE: [v1] driver: bluetooth: hci_qca:fix unable to load the BT driver

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1848 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/project/bluetooth/list/?series=969790

---Test result---

Test Summary:
CheckPatch                    PENDING   0.30 seconds
GitLint                       PENDING   0.26 seconds
SubjectPrefix                 FAIL      0.55 seconds
BuildKernel                   PASS      25.08 seconds
CheckAllWarning               PASS      27.60 seconds
CheckSparse                   PASS      31.08 seconds
BuildKernel32                 PASS      24.54 seconds
TestRunnerSetup               PASS      458.09 seconds
TestRunner_l2cap-tester       PASS      25.22 seconds
TestRunner_iso-tester         PASS      33.74 seconds
TestRunner_bnep-tester        PASS      5.86 seconds
TestRunner_mgmt-tester        PASS      133.61 seconds
TestRunner_rfcomm-tester      PASS      9.28 seconds
TestRunner_sco-tester         PASS      14.68 seconds
TestRunner_ioctl-tester       PASS      9.90 seconds
TestRunner_mesh-tester        PASS      7.25 seconds
TestRunner_smp-tester         PASS      8.51 seconds
TestRunner_userchan-tester    PASS      6.13 seconds
IncrementalBuild              PENDING   0.92 seconds

Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:

##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:

##############################
Test: SubjectPrefix - FAIL
Desc: Check subject contains "Bluetooth" prefix
Output:
"Bluetooth: " prefix is not specified in the subject
##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:



---
Regards,
Linux Bluetooth

Re: [PATCH v1] driver: bluetooth: hci_qca:fix unable to load the BT driver

[patchwork-bot+bluetooth]

Hello:

This patch was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:

On Mon,  9 Jun 2025 18:55:00 +0800 you wrote:
> Some modules have BT_EN enabled via a hardware pull-up,
> meaning it is not defined in the DTS and is not controlled
> through the power sequence. In such cases, fall through
> to follow the legacy flow.
> 
> Signed-off-by: Shuai Zhang <quic_shuaz@quicinc.com>
> 
> [...]

Here is the summary with links:
  - [v1] driver: bluetooth: hci_qca:fix unable to load the BT driver
    https://git.kernel.org/bluetooth/bluetooth-next/c/6c31dab4ff1e

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH v1] driver: bluetooth: hci_qca:fix unable to load the BT driver

[brgl]

On Mon, 9 Jun 2025 18:55:00 +0800, Shuai Zhang <quic_shuaz@quicinc.com> wrote:
>
> Some modules have BT_EN enabled via a hardware pull-up,
> meaning it is not defined in the DTS and is not controlled
> through the power sequence. In such cases, fall through
> to follow the legacy flow.
>

Thanks Stephan for bringing this to my attention.

Shuai: you have not Cc'ed arm-msm or even linux-kernel and so didn't give us
any chance to object. I will separately send a MAINTAINERS change to add
arm-msm as the mailing list for this driver.

What is the problem you're seeing? The bt-enable GPIO is optional in the power
sequencing driver and if it's not there, then there should be no difference in
how this driver works. What are the offending platforms?

> Signed-off-by: Shuai Zhang <quic_shuaz@quicinc.com>
> ---
>  drivers/bluetooth/hci_qca.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
> index a2dc39c00..976ec88a0 100644
> --- a/drivers/bluetooth/hci_qca.c
> +++ b/drivers/bluetooth/hci_qca.c
> @@ -2392,10 +2392,17 @@ static int qca_serdev_probe(struct serdev_device *serdev)
>  		 */
>  			qcadev->bt_power->pwrseq = devm_pwrseq_get(&serdev->dev,
>  								   "bluetooth");
> -			if (IS_ERR(qcadev->bt_power->pwrseq))
> -				return PTR_ERR(qcadev->bt_power->pwrseq);
>
> -			break;
> +			/*
> +			 * Some modules have BT_EN enabled via a hardware pull-up,
> +			 * meaning it is not defined in the DTS and is not controlled
> +			 * through the power sequence. In such cases, fall through
> +			 * to follow the legacy flow.
> +			 */
> +			if (IS_ERR(qcadev->bt_power->pwrseq))
> +				qcadev->bt_power->pwrseq = NULL;
> +			else
> +				break;

This is wrong. It just effectively ignores all errors - even -EPROBE_DEFER.
This patch should be reverted as - depending on the run-time ordering of driver
probing - it will surely break existing platforms.

>  		}
>  		fallthrough;
>  	case QCA_WCN3950:
> --
> 2.34.1

Bart

Re: [PATCH v1] driver: bluetooth: hci_qca:fix unable to load the BT driver

[Shuai Zhang]

On 2025/7/2 18:23, brgl@bgdev.pl wrote:
> On Mon, 9 Jun 2025 18:55:00 +0800, Shuai Zhang <quic_shuaz@quicinc.com> wrote:
>>
>> Some modules have BT_EN enabled via a hardware pull-up,
>> meaning it is not defined in the DTS and is not controlled
>> through the power sequence. In such cases, fall through
>> to follow the legacy flow.
>>
> 
> Thanks Stephan for bringing this to my attention.
> 
> Shuai: you have not Cc'ed arm-msm or even linux-kernel and so didn't give us
> any chance to object. I will separately send a MAINTAINERS change to add
> arm-msm as the mailing list for this driver.
> 
> What is the problem you're seeing? The bt-enable GPIO is optional in the power
> sequencing driver and if it's not there, then there should be no difference in
> how this driver works. What are the offending platforms?
> 

The observed issue is that the Bluetooth driver fails to load, 
and bluetoothctl shows the message: "Waiting to connect to bluetoothd..."

The bt-enable GPIO is optional, but when the power sequence does 
not define "bluetooth", it results in an EPROBE_DEFER error, 
which causes the Bluetooth driver to be repeatedly reloaded.

On the Lemans platform, which uses the BT M.2 interface, the bt_en signal 
is pulled up via hardware, so it does not need to be defined in the DTS.

>> Signed-off-by: Shuai Zhang <quic_shuaz@quicinc.com>
>> ---
>>  drivers/bluetooth/hci_qca.c | 13 ++++++++++---
>>  1 file changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
>> index a2dc39c00..976ec88a0 100644
>> --- a/drivers/bluetooth/hci_qca.c
>> +++ b/drivers/bluetooth/hci_qca.c
>> @@ -2392,10 +2392,17 @@ static int qca_serdev_probe(struct serdev_device *serdev)
>>  		 */
>>  			qcadev->bt_power->pwrseq = devm_pwrseq_get(&serdev->dev,
>>  								   "bluetooth");
>> -			if (IS_ERR(qcadev->bt_power->pwrseq))
>> -				return PTR_ERR(qcadev->bt_power->pwrseq);
>>
>> -			break;
>> +			/*
>> +			 * Some modules have BT_EN enabled via a hardware pull-up,
>> +			 * meaning it is not defined in the DTS and is not controlled
>> +			 * through the power sequence. In such cases, fall through
>> +			 * to follow the legacy flow.
>> +			 */
>> +			if (IS_ERR(qcadev->bt_power->pwrseq))
>> +				qcadev->bt_power->pwrseq = NULL;
>> +			else
>> +				break;
> 
> This is wrong. It just effectively ignores all errors - even -EPROBE_DEFER.
> This patch should be reverted as - depending on the run-time ordering of driver
> probing - it will surely break existing platforms.
>

Thank you for your reminder. I will update the patch to add handling for 
errors and retain the handling for undefined cases.

>>  		}
>>  		fallthrough;
>>  	case QCA_WCN3950:
>> --
>> 2.34.1
> 
> Bart

Re: [PATCH v1] driver: bluetooth: hci_qca:fix unable to load the BT driver

[Bartosz Golaszewski]

On Fri, Jul 4, 2025 at 3:30 PM Shuai Zhang <quic_shuaz@quicinc.com> wrote:
>
>
>
> On 2025/7/2 18:23, brgl@bgdev.pl wrote:
> > On Mon, 9 Jun 2025 18:55:00 +0800, Shuai Zhang <quic_shuaz@quicinc.com> wrote:
> >>
> >> Some modules have BT_EN enabled via a hardware pull-up,
> >> meaning it is not defined in the DTS and is not controlled
> >> through the power sequence. In such cases, fall through
> >> to follow the legacy flow.
> >>
> >
> > Thanks Stephan for bringing this to my attention.
> >
> > Shuai: you have not Cc'ed arm-msm or even linux-kernel and so didn't give us
> > any chance to object. I will separately send a MAINTAINERS change to add
> > arm-msm as the mailing list for this driver.
> >
> > What is the problem you're seeing? The bt-enable GPIO is optional in the power
> > sequencing driver and if it's not there, then there should be no difference in
> > how this driver works. What are the offending platforms?
> >
>
> The observed issue is that the Bluetooth driver fails to load,
> and bluetoothctl shows the message: "Waiting to connect to bluetoothd..."
>
> The bt-enable GPIO is optional, but when the power sequence does
> not define "bluetooth", it results in an EPROBE_DEFER error,
> which causes the Bluetooth driver to be repeatedly reloaded.
>

We are talking about the bt-enable-gpios property of the pmu node,
right? What does it mean that the 'power sequence does not define
"bluetooth"'? Is the power sequencing provider not probing correctly?

> On the Lemans platform, which uses the BT M.2 interface, the bt_en signal
> is pulled up via hardware, so it does not need to be defined in the DTS.
>

Which lemans are we talking about?
arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi has the bt-enable-gpios
property. Unfortunately I cannot test it, because the board I have
remote access to has been reflashed and became sa8255p-ride instead.

Bartosz

> >> Signed-off-by: Shuai Zhang <quic_shuaz@quicinc.com>
> >> ---
> >>  drivers/bluetooth/hci_qca.c | 13 ++++++++++---
> >>  1 file changed, 10 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
> >> index a2dc39c00..976ec88a0 100644
> >> --- a/drivers/bluetooth/hci_qca.c
> >> +++ b/drivers/bluetooth/hci_qca.c
> >> @@ -2392,10 +2392,17 @@ static int qca_serdev_probe(struct serdev_device *serdev)
> >>               */
> >>                      qcadev->bt_power->pwrseq = devm_pwrseq_get(&serdev->dev,
> >>                                                                 "bluetooth");
> >> -                    if (IS_ERR(qcadev->bt_power->pwrseq))
> >> -                            return PTR_ERR(qcadev->bt_power->pwrseq);
> >>
> >> -                    break;
> >> +                    /*
> >> +                     * Some modules have BT_EN enabled via a hardware pull-up,
> >> +                     * meaning it is not defined in the DTS and is not controlled
> >> +                     * through the power sequence. In such cases, fall through
> >> +                     * to follow the legacy flow.
> >> +                     */
> >> +                    if (IS_ERR(qcadev->bt_power->pwrseq))
> >> +                            qcadev->bt_power->pwrseq = NULL;
> >> +                    else
> >> +                            break;
> >
> > This is wrong. It just effectively ignores all errors - even -EPROBE_DEFER.
> > This patch should be reverted as - depending on the run-time ordering of driver
> > probing - it will surely break existing platforms.
> >
>
> Thank you for your reminder. I will update the patch to add handling for
> errors and retain the handling for undefined cases.
>
> >>              }
> >>              fallthrough;
> >>      case QCA_WCN3950:
> >> --
> >> 2.34.1
> >
> > Bart
>

Re: [PATCH v1] driver: bluetooth: hci_qca:fix unable to load the BT driver

[Shuai Zhang]

Hi,iBart

On 7/2/2025 6:23 PM, brgl@bgdev.pl wrote:
> On Mon, 9 Jun 2025 18:55:00 +0800, Shuai Zhang <quic_shuaz@quicinc.com> wrote:
>>
>> Some modules have BT_EN enabled via a hardware pull-up,
>> meaning it is not defined in the DTS and is not controlled
>> through the power sequence. In such cases, fall through
>> to follow the legacy flow.
>>
> 
> Thanks Stephan for bringing this to my attention.
> 
> Shuai: you have not Cc'ed arm-msm or even linux-kernel and so didn't give us
> any chance to object. I will separately send a MAINTAINERS change to add
> arm-msm as the mailing list for this driver.
> 
> What is the problem you're seeing? The bt-enable GPIO is optional in the power
> sequencing driver and if it's not there, then there should be no difference in
> how this driver works. What are the offending platforms?
> 
>> Signed-off-by: Shuai Zhang <quic_shuaz@quicinc.com>
>> ---
>>  drivers/bluetooth/hci_qca.c | 13 ++++++++++---
>>  1 file changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
>> index a2dc39c00..976ec88a0 100644
>> --- a/drivers/bluetooth/hci_qca.c
>> +++ b/drivers/bluetooth/hci_qca.c
>> @@ -2392,10 +2392,17 @@ static int qca_serdev_probe(struct serdev_device *serdev)
>>  		 */
>>  			qcadev->bt_power->pwrseq = devm_pwrseq_get(&serdev->dev,
>>  								   "bluetooth");
>> -			if (IS_ERR(qcadev->bt_power->pwrseq))
>> -				return PTR_ERR(qcadev->bt_power->pwrseq);
>>
>> -			break;
>> +			/*
>> +			 * Some modules have BT_EN enabled via a hardware pull-up,
>> +			 * meaning it is not defined in the DTS and is not controlled
>> +			 * through the power sequence. In such cases, fall through
>> +			 * to follow the legacy flow.
>> +			 */
>> +			if (IS_ERR(qcadev->bt_power->pwrseq))
>> +				qcadev->bt_power->pwrseq = NULL;
>> +			else
>> +				break;
> 
> This is wrong. It just effectively ignores all errors - even -EPROBE_DEFER.
> This patch should be reverted as - depending on the run-time ordering of driver
> probing - it will surely break existing platforms.
> 

I'm very sorry—due to my oversight, I forgot to update it.

The updated strategy is as follows:
Handle nodev separately, while other errors will still be returned.

case:xxx	
....
....						   
if (IS_ERR(qcadev->bt_power->pwrseq)) {
    switch (PTR_ERR(qcadev->bt_power->pwrseq)) {
    case -ENODEV:
        qcadev->bt_power->pwrseq = NULL;
        break;
    default:
        return PTR_ERR(qcadev->bt_power->pwrseq);
    }
}
fallthtough;
case:xxx

>>  		}
>>  		fallthrough;
>>  	case QCA_WCN3950:
>> --
>> 2.34.1
> 
> Bart

BR,
Shuai

Re: [PATCH v1] driver: bluetooth: hci_qca:fix unable to load the BT driver

[Bartosz Golaszewski]

On Fri, Aug 22, 2025 at 11:49 AM Shuai Zhang <quic_shuaz@quicinc.com> wrote:
>
> Hi,iBart
>
> On 7/2/2025 6:23 PM, brgl@bgdev.pl wrote:
> > On Mon, 9 Jun 2025 18:55:00 +0800, Shuai Zhang <quic_shuaz@quicinc.com> wrote:
> >>
> >> Some modules have BT_EN enabled via a hardware pull-up,
> >> meaning it is not defined in the DTS and is not controlled
> >> through the power sequence. In such cases, fall through
> >> to follow the legacy flow.
> >>
> >
> > Thanks Stephan for bringing this to my attention.
> >
> > Shuai: you have not Cc'ed arm-msm or even linux-kernel and so didn't give us
> > any chance to object. I will separately send a MAINTAINERS change to add
> > arm-msm as the mailing list for this driver.
> >
> > What is the problem you're seeing? The bt-enable GPIO is optional in the power
> > sequencing driver and if it's not there, then there should be no difference in
> > how this driver works. What are the offending platforms?
> >
> >> Signed-off-by: Shuai Zhang <quic_shuaz@quicinc.com>
> >> ---
> >>  drivers/bluetooth/hci_qca.c | 13 ++++++++++---
> >>  1 file changed, 10 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
> >> index a2dc39c00..976ec88a0 100644
> >> --- a/drivers/bluetooth/hci_qca.c
> >> +++ b/drivers/bluetooth/hci_qca.c
> >> @@ -2392,10 +2392,17 @@ static int qca_serdev_probe(struct serdev_device *serdev)
> >>               */
> >>                      qcadev->bt_power->pwrseq = devm_pwrseq_get(&serdev->dev,
> >>                                                                 "bluetooth");
> >> -                    if (IS_ERR(qcadev->bt_power->pwrseq))
> >> -                            return PTR_ERR(qcadev->bt_power->pwrseq);
> >>
> >> -                    break;
> >> +                    /*
> >> +                     * Some modules have BT_EN enabled via a hardware pull-up,
> >> +                     * meaning it is not defined in the DTS and is not controlled
> >> +                     * through the power sequence. In such cases, fall through
> >> +                     * to follow the legacy flow.
> >> +                     */
> >> +                    if (IS_ERR(qcadev->bt_power->pwrseq))
> >> +                            qcadev->bt_power->pwrseq = NULL;
> >> +                    else
> >> +                            break;
> >
> > This is wrong. It just effectively ignores all errors - even -EPROBE_DEFER.
> > This patch should be reverted as - depending on the run-time ordering of driver
> > probing - it will surely break existing platforms.
> >
>
> I'm very sorry—due to my oversight, I forgot to update it.
>

Yeah and I was on vacation. Meanwhile this code is still wrong in mainline. :(

> The updated strategy is as follows:
> Handle nodev separately, while other errors will still be returned.
>
> case:xxx
> ....
> ....
> if (IS_ERR(qcadev->bt_power->pwrseq)) {
>     switch (PTR_ERR(qcadev->bt_power->pwrseq)) {
>     case -ENODEV:
>         qcadev->bt_power->pwrseq = NULL;
>         break;
>     default:
>         return PTR_ERR(qcadev->bt_power->pwrseq);

In what circumstances would we get ENODEV here?

And you surely don't need an if AND a switch.

>     }
> }
> fallthtough;
> case:xxx
>
> >>              }
> >>              fallthrough;
> >>      case QCA_WCN3950:
> >> --
> >> 2.34.1
> >
> > Bart
>
> BR,
> Shuai
>

Bart