Re: [PATCH v2 3/4] Bluetooth: hci_qca: Support QCA2066 on M.2 connector via pwrseq

[Manivannan Sadhasivam]

On Tue, Jul 07, 2026 at 11:14:10AM +0200, Loic Poulain wrote:
> On Mon, Jul 6, 2026 at 8:44 AM Manivannan Sadhasivam <mani@xxxxxxxxxx> wrote:
> >
> > On Sat, Jul 04, 2026 at 03:11:13AM +0300, Dmitry Baryshkov wrote:
> > > On Thu, Jul 02, 2026 at 05:08:56PM +0200, Manivannan Sadhasivam wrote:
> > > > On Thu, Jul 02, 2026 at 05:34:31PM +0300, Dmitry Baryshkov wrote:
> > > > > On Thu, Jul 02, 2026 at 04:17:43PM +0200, Manivannan Sadhasivam wrote:
> > > > > > On Thu, Jul 02, 2026 at 03:14:49PM +0300, Dmitry Baryshkov wrote:
> > > > > > > On Thu, Jul 02, 2026 at 12:46:15PM +0200, Loic Poulain wrote:
> > > > > > > > For QCA2066 (and other QCA chips) on M.2 connectors, the UART enable
> > > > > > > > is controlled by the W_DISABLE2# signal managed by the pcie-m2 power
> > > > > > > > sequencer rather than a dedicated BT enable GPIO.
> > > > > > > >
> > > > > > > > When the serdev controller has an OF graph (indicating it is connected
> > > > > > > > to an M.2 connector), acquire the 'uart' pwrseq target from the
> > > > > > > > connector's power sequencer and use it to control BT power instead of
> > > > > > > > the bt-enable GPIO.
> > > > > > > >
> > > > > > > > Also allocate bt_power unconditionally for all SOC types since the
> > > > > > >
> > > > > > > Can we just fold it into the main struct?
> > > > > > >
> > > > > > > > pwrseq path is independent of the SOC type switch.
> > > > > > > >
> > > > > > > > Signed-off-by: Loic Poulain <loic.poulain@xxxxxxxxxxxxxxxx>
> > > > > > > > ---
> > > > > > > >  drivers/bluetooth/hci_qca.c | 81 ++++++++++++++++++++++++---------------------
> > > > > > > >  1 file changed, 43 insertions(+), 38 deletions(-)
> > > > > > > > @@ -2387,6 +2390,35 @@ static int qca_init_regulators(struct qca_power *qca,
> > > > > > > >     return 0;
> > > > > > > >  }
> > > > > > > >
> > > > > > > > +/*
> > > > > > > > + * Acquire the M.2 connector power sequencer.
> > > > > > > > + *
> > > > > > > > + * An OF graph link on the serdev controller is only present when the BT
> > > > > > > > + * device is attached through an M.2 Key E connector. In that case the UART
> > > > > > > > + * enable (W_DISABLE2#) is driven by the pcie-m2 power sequencer instead of a
> > > > > > > > + * dedicated BT enable GPIO, so grab the "uart" pwrseq target from it.
> > > > > > > > + *
> > > > > > > > + * Returns 0 if no M.2 connector is present (nothing to do), a negative errno
> > > > > > > > + * on error, otherwise 0 with qcadev->bt_power->pwrseq populated.
> > > > > > > > + */
> > > > > > > > +static int qca_serdev_get_m2_pwrseq(struct qca_serdev *qcadev, bool *bt_en_available)
> > > > > > > > +{
> > > > > > > > +   struct serdev_device *serdev = qcadev->serdev_hu.serdev;
> > > > > > > > +   struct device *dev;
> > > > > > > > +
> > > > > > > > +   if (!of_graph_is_present(dev_of_node(&serdev->ctrl->dev)))
> > > > > > > > +           return 0;
> > > > > > > > +
> > > > > > > > +   qcadev->bt_power->pwrseq = devm_pwrseq_get(&serdev->ctrl->dev, "uart");
> > > > > > > > +   if (IS_ERR(qcadev->bt_power->pwrseq))
> > > > > > > > +           return PTR_ERR(qcadev->bt_power->pwrseq);
> > > > > > > > +
> > > > > > > > +   dev = pwrseq_to_device(qcadev->bt_power->pwrseq);
> > > > > > > > +   *bt_en_available = device_property_present(dev, "w-disable2-gpios");
> > > > > > >
> > > > > > > I think here you are looking into the exact details of the other of the
> > > > > > > graph. There might be other devices on that side, while the code now
> > > > > > > assumes M.2. Or, consider having an M.2 controller which handles
> > > > > > > W_DISABLE2# internally rather than through the GPIO.
> > > > > > >
> > > > > >
> > > > > > This code only deals with M.2 connector in specific, so I'm not sure why we need
> > > > > > to worry about *other* kind of devices. Let's worry about them when they show up
> > > > > > (with graph interface ofc).
> > > > >
> > > > > I don't think we want to go through the drivers using M.2 connectors in
> > > > > such a case. In the end, the contract should be that there is a power
> > > > > sequencer on the other side of the graph, but the specifics of the
> > > > > connector should be abstracted out. Do you know, if in the x86 world the
> > > > > W_DISABLE2# is a GPIO or is controleed by the hub.
> > > > >
> > > >
> > > > I tried to abstract out, but Bartosz didn't want pwrctrl APIs to do that level
> > > > of abstraction as pwrctrl APIs should be generic and should not be bind to a
> > > > specific connector and exposing its internals.
> > > >
> > > > That's why we ended up having pwrctrl core exposing the 'struct dev' using
> > > > pwrseq_to_device() and letting the consumer extracting whatever information it
> > > > needs.
> > >
> > > Do we have other ways to control M.2? For example on the x86 systems,
> > > are those signals controlled via GPIOs (or GPIO-like registers) or are
> > > they controlled separately by something like M.2 controller? Or do you
> > > have an idea about other non-x86 systems?
> > >
> >
> > Thre is no OS-visible M.2 power control in ACPI systems. ACPI defines PRx
> > objects to control power to the PCI devices based on the D-state and the OS just
> > evaluates the _ON/_OFF methods of the respective objects.
> >
> > So this API is not going to be useful on non-DT systems where the BT_EN GPIO
> > handling is abstracted away. And also on platforms where BT_EN is not controlled
> > by GPIOs. But I haven't seen DT platforms handling BT_EN (or W_DISABLE2#)
> > signal in a non-GPIO way.
> >
> > I too prefer an API to query whether the connector supports BT_EN or not, but
> > I'm not sure how to come up with a generic pwrseq API which also satisfies the
> > requirement. If you have any suggestions, please let me know!
> >
> > FWIW, I tried adding pwrseq_is_fixed() API [1] earlier, which was turned down by
> > Bartosz.
> 
> So, I'll submit a new version incorporating Dmitry's request to fold
> bt_power into the main structure.
> However, what should we do about the powerseq point? Should we keep it as it is?
> 

@Bartosz: Thoughts?

- Mani

-- 
மணிவண்ணன் சதாசிவம்