On Fri, Aug 22, 2025 at 11:49 AM Shuai Zhang <quic_shuaz@xxxxxxxxxxx> wrote:
>
> 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.
>
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
