Hi,iBart
On 7/2/2025 6:23 PM, brgl@xxxxxxxx wrote:
> On Mon, 9 Jun 2025 18:55:00 +0800, Shuai Zhang <quic_shuaz@xxxxxxxxxxx> 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@xxxxxxxxxxx>
>> ---
>> 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
