Re: [RFC] Platform-Level Device Reset (_PRR) for CNVi Bluetooth over USB
Basti <[email protected]> Wed, 1 Jul 2026 20:53:02 +0200
| Newsgroups | gmane.linux.bios |
|---|---|
| Message-ID | <[email protected]> |
--===============1205947584243177445== Content-Type: multipart/alternative; boundary="Apple-Mail=_58765EE8-F2D4-4B94-AC3C-D120801417EA" --Apple-Mail=_58765EE8-F2D4-4B94-AC3C-D120801417EA Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 Hi Matt, Thanks, that's a good point and thanks for your thoughts about this - I'd really missed is_intel_bluetooth, and you're right that it already = generates the _PRR and the BTRT power resource under the USB device scope. That already solves the "no PCI function, so nothing gets emitted" half = of my problem. So let me narrow the RFC down to the one piece it doesn't cover. The thing that matters here is *how the reset is actually performed*. There are two different mechanisms: 1. GPIO reset - toggle a physical pin (the WDISABLE2 / BT_RF_KILL_N line). This is what is_intel_bluetooth implements today: BTRT._RST drives reset_gpio/enable_gpio via BTRK/SBTE, and every branch is guarded by reset_gpio->pin_count. Give it no GPIOs and _RST is an empty method. 2. CNVi sideband PLDR - no pin at all; the reset is a sideband register write, PCRO(PID_CNVI, CNVI_ABORT_PLDR, 0x03) (PCR 0x73, offset 0x80). This is exactly the CNVP._RST body that src/soc/intel/common/block/cnvi/cnvi.c cnvb_fill_ssdt() already emits - but only for the PCI CNVi BT function. On this Alder Lake-N board the CNVi Bluetooth reaches the host as a USB device, and its platform-level reset is mechanism (2), the sideband PLDR. There is no BT reset GPIO on the board - the reset simply isn't a pin. So "supply the enable/reset GPIOs" doesn't apply here: I have nothing to hand to is_intel_bluetooth, and if I pass empty GPIOs its _RST does nothing. So the gap is specifically: is_intel_bluetooth can emit the _PRR scaffolding for a USB BT device, but its only _RST implementation is the GPIO one. The sideband-PLDR _RST that a CNVi part needs lives in cnvi.c and is reachable only from the PCI path. Given that, I think the cleanest fix is smaller than my original proposal (a separate is_intel_cnvi_bluetooth flag): better extend is_intel_bluetooth with a PLDR reset variant. Concretely - factor the PLDR _RST/CNVP acpigen out of cnvi.c into a shared helper, and have acpi_device_intel_bt() emit that variant instead of the GPIO-toggle _RST when the board marks the port as CNVi-over-USB (no reset GPIO). Same _PRR/power-resource plumbing you already have, one source of truth for the PLDR ASL, and it replaces the hand-written DSDT I'm carrying per board today. Does that structure make sense for you? If so I'm happy to maybe send a = patch with the helper refactor + the reset-variant selection, so cnvi usb bluetooth = can be reset via the sideband PLDR if GPIO is not available. Thanks, Sebastian > Am 01.07.2026 um 16:02 schrieb Matt DeVillier = <[email protected]>: >=20 > hi Sebastian, >=20 > I'd need to take a closer look to compare to your proposal, but = there's already an 'is_intel_bluetooth' flag for drivers/usb/acpi which = generates a _PRR for any Intel BT (CNVi or discrete), as long as you = supply the enable/reset GPIOs >=20 > Is this not sufficient? If not, can it be adapted/extended to do what = you need? >=20 > -Matt >=20 > On Sat, Jun 27, 2026 at 12:16=E2=80=AFPM Sebastian M=C3=BCller = <[email protected] <mailto:[email protected]>> wrote: >> Hi all, >>=20 >> While bringing up an Alder Lake-N board I hit a small gap in the CNVi >> Bluetooth support and would like to sanity-check an approach before >> writing the patch, since it touches a generic driver and has a couple = of >> design choices a maintainer should steer. >>=20 >> Problem >> ------- >>=20 >> Intel CNVi Bluetooth reaches the host in one of two ways depending on = the >> SoC/PCH configuration: >>=20 >> * as a PCI function (Tiger Lake, Panther Lake, Nova Lake, ... - the = IDs >> in bt_pci_device_ids[]), or >> * as a USB device on an internal XHCI port (Alder Lake-N and other >> IoT / "-N" configurations, where the BT companion is = CNVi-over-USB). >>=20 >> coreboot implements the CNVi Bluetooth Platform-Level Device Reset = (PLDR) >> - the CNVP power resource with the _RST method, plus the _PRR object = the >> kernel looks for - in src/soc/intel/common/block/cnvi/cnvi.c >> (cnvb_fill_ssdt()), and it is emitted only by the PCI Bluetooth = driver >> (cnvi_bt_ops, bound to bt_pci_device_ids[]). >>=20 >> When CNVi Bluetooth is on USB there is no PCI BT function, so >> cnvb_fill_ssdt() never runs and no _PRR is generated. The kernel = logs: >>=20 >> Bluetooth: hci0: No support for _PRR ACPI method >>=20 >> and cannot perform the platform-level reset. (The stock vendor = firmware on >> these boards also omits it, so this is not a regression - but = coreboot >> already has the PLDR logic and could provide the _PRR here too.) >>=20 >> What the reset actually is (hardware-verified, Alder Lake-N) >> ----------------------------------------------------------- >>=20 >> On the board the CNVi Bluetooth companion enumerates as a USB2 device >> under \_SB.PCI0.XHCI.RHUB.HS10. The PLDR is a CNVi-sideband PCR = write: >>=20 >> * reset register =3D PCR port 0x73, offset 0x80 (0xfd730080 MMIO), >> CNVI_ABORT_PLDR value 0x80 (non-PCH-S) - matching the constants >> already in cnvi.c (CNVI_ABORT_PLDR, PID_CNVI). >> * readback reports CNVI_READY (0x04) at idle and PRRS reports >> completion - identical semantics to the PCI path. >>=20 >> So the reset operation is the same as the existing PCI PLDR; only the = ACPI >> device scope that carries the _PRR differs (a USB device node instead = of >> the PCI BT function). I currently work around this with a = hand-written >> power resource in the board dsdt.asl (a Scope(HS10) with a _RST doing >> PCRO(0x73, 0x80, 0x03) plus Name(_PRR)). It works, but every = USB-CNVi-BT >> board would re-implement the same ASL by hand. >>=20 >> Proposed mechanism >> ------------------ >>=20 >> Let a board mark the USB port that carries the CNVi Bluetooth = companion, >> and have coreboot emit the existing PLDR power resource + _PRR under = that >> USB device's ACPI scope, reusing the PCI path's logic: >>=20 >> 1. Refactor the PLDR acpigen out of cnvb_fill_ssdt() into a = reusable >> helper in soc/intel/common/block/cnvi, e.g. >> cnvi_acpi_write_bt_pldr(), called by both the PCI BT driver and = the >> new USB path (single source of truth for the PLDR ASL). >>=20 >> 2. Add an opt-in to drivers/usb/acpi config >> (struct drivers_usb_acpi_config), e.g. >> bool is_intel_cnvi_bluetooth; >>=20 >> 3. In drivers/usb/acpi/usb_acpi.c, when the flag is set, call the = helper >> inside the USB device's scope (alongside the _DSD/_PRW it = already >> emits). >>=20 >> 4. The board enables it on the BT USB port in devicetree: >>=20 >> chip drivers/usb/acpi >> register "is_intel_cnvi_bluetooth" =3D "1" >> device usb x.y on end # the CNVi BT companion port >> end >>=20 >> The PLDR is already pure acpigen, and the SoC exposes the PCRO/PCRR >> sideband ASL helpers (src/soc/intel/common/acpi/pch_pcr.asl), so the >> helper is pure ACPI generation with no SoC C callback - it writes a = method >> that calls the existing PCRR/PCRO (exactly what the board workaround = does >> today). drivers/usb/acpi only needs to know whether to emit the = resource, >> not how the reset works. >>=20 >> Alternatives considered >> ----------------------- >>=20 >> * Board-local DSDT (status quo): works, but duplicates the PLDR ASL = per >> board and drifts from cnvi.c. >> * A dedicated drivers/intel/cnvi_usb_bt chip: heavier; = drivers/usb/acpi >> already owns the USB device's ACPI node, so a flag there is = lighter. >> * Auto-detection of the BT USB port: not feasible generically - = which >> internal port carries the companion is board-specific. >>=20 >> Open questions >> -------------- >>=20 >> 1. Is drivers/usb/acpi the right home for the flag, or would you = prefer a >> thin shim driver? >> 2. Should the flag instead be derived from the existing CNVi = devicetree >> config so a board describes "CNVi BT is on USB port X" in one = place? >> 3. Naming: is_intel_cnvi_bluetooth vs cnvi_bt_pldr vs reusing a = CNVi enum. >> 4. Should the shared PLDR acpigen helper live in the common CNVi = block or >> in a small ACPI helper next to pch_pcr.asl? >>=20 >> If the direction is agreeable I'll send a patch (with the refactor + = a >> board using it) and verify on the hardware. >>=20 >> Thanks, >> Sebastian >> _______________________________________________ >> coreboot mailing list -- [email protected] = <mailto:[email protected]> >> To unsubscribe send an email to [email protected] = <mailto:[email protected]> --Apple-Mail=_58765EE8-F2D4-4B94-AC3C-D120801417EA Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=utf-8 <html aria-label=3D"message body"><head><meta http-equiv=3D"content-type" = content=3D"text/html; charset=3Dutf-8"></head><body = style=3D"overflow-wrap: break-word; -webkit-nbsp-mode: space; = line-break: after-white-space;">Hi Matt,<br><br>Thanks, that's a good = point and thanks for your thoughts about this -<br>I'd really missed = is_intel_bluetooth, and you're right that it already generates<br>the = _PRR and the BTRT power resource under the USB device scope.<br>That = already solves the "no PCI function, so nothing gets emitted" half of my = problem.<br>So let me narrow the RFC down to the one piece it doesn't = cover.<br><br>The thing that matters here is *how the reset is actually = performed*.<br>There are two different mechanisms:<br><br>1. GPIO reset = - toggle a physical pin (the WDISABLE2 / BT_RF_KILL_N<br>line). This is = what is_intel_bluetooth implements today: BTRT._RST<br>drives = reset_gpio/enable_gpio via BTRK/SBTE, and every branch is<br>guarded by = reset_gpio->pin_count. Give it no GPIOs and _RST is an<br>empty = method.<br><br>2. CNVi sideband PLDR - no pin at all; the reset is a = sideband<br>register write, PCRO(PID_CNVI, CNVI_ABORT_PLDR, 0x03) (PCR = 0x73,<br>offset 0x80). This is exactly the CNVP._RST body = that<br>src/soc/intel/common/block/cnvi/cnvi.c cnvb_fill_ssdt() = already<br>emits - but only for the PCI CNVi BT function.<br><br>On this = Alder Lake-N board the CNVi Bluetooth reaches the host as a = USB<br>device, and its platform-level reset is mechanism (2), the = sideband<br>PLDR. There is no BT reset GPIO on the board - the reset = simply isn't a<br>pin. So "supply the enable/reset GPIOs" doesn't apply = here: I have<br>nothing to hand to is_intel_bluetooth, and if I pass = empty GPIOs its<br>_RST does nothing.<br><br>So the gap is specifically: = is_intel_bluetooth can emit the _PRR<br>scaffolding for a USB BT device, = but its only _RST implementation is the<br>GPIO one. The sideband-PLDR = _RST that a CNVi part needs lives in cnvi.c<br>and is reachable only = from the PCI path.<br><br>Given that, I think the cleanest fix is = smaller than my original<br>proposal (a separate is_intel_cnvi_bluetooth = flag): better extend<br>is_intel_bluetooth with a PLDR reset variant. = Concretely - factor the<br>PLDR _RST/CNVP acpigen out of cnvi.c into a = shared helper, and have<br>acpi_device_intel_bt() emit that variant = instead of the GPIO-toggle _RST<br>when the board marks the port as = CNVi-over-USB (no reset GPIO). Same<br>_PRR/power-resource plumbing you = already have, one source of truth for<br>the PLDR ASL, and it replaces = the hand-written DSDT I'm carrying per<br>board today.<br><br>Does that = structure make sense for you? If so I'm happy to maybe send a patch = with<br>the helper refactor + the reset-variant selection, so cnvi usb = bluetooth can be reset via the<br>sideband PLDR if GPIO is not = available.<br><br>Thanks,<br>Sebastian<br = id=3D"lineBreakAtBeginningOfMessage"><div><br><blockquote = type=3D"cite"><div>Am 01.07.2026 um 16:02 schrieb Matt DeVillier = <[email protected]>:</div><br = class=3D"Apple-interchange-newline"><div><div dir=3D"ltr">hi = Sebastian,<div><br></div><div>I'd need to take a closer look to compare = to your proposal, but there's already an 'is_intel_bluetooth' flag for = drivers/usb/acpi which generates a _PRR for any Intel BT (CNVi or = discrete), as long as you supply the enable/reset = GPIOs</div><div><br></div><div>Is this not sufficient? If not, can it be = adapted/extended to do what you = need?</div><div><br></div><div>-Matt</div></div><br><div = class=3D"gmail_quote gmail_quote_container"><div dir=3D"ltr" = class=3D"gmail_attr">On Sat, Jun 27, 2026 at 12:16=E2=80=AFPM Sebastian = M=C3=BCller <<a = href=3D"mailto:[email protected]">[email protected]</a>> = wrote:<br></div><blockquote class=3D"gmail_quote" style=3D"margin:0px = 0px 0px 0.8ex;border-left:1px solid = rgb(204,204,204);padding-left:1ex">Hi all,<br> <br> While bringing up an Alder Lake-N board I hit a small gap in the = CNVi<br> Bluetooth support and would like to sanity-check an approach before<br> writing the patch, since it touches a generic driver and has a couple = of<br> design choices a maintainer should steer.<br> <br> Problem<br> -------<br> <br> Intel CNVi Bluetooth reaches the host in one of two ways depending on = the<br> SoC/PCH configuration:<br> <br> * as a PCI function (Tiger Lake, Panther Lake, Nova Lake, ... - = the IDs<br> in bt_pci_device_ids[]), or<br> * as a USB device on an internal XHCI port (Alder Lake-N and = other<br> IoT / "-N" configurations, where the BT companion is = CNVi-over-USB).<br> <br> coreboot implements the CNVi Bluetooth Platform-Level Device Reset = (PLDR)<br> - the CNVP power resource with the _RST method, plus the _PRR object = the<br> kernel looks for - in src/soc/intel/common/block/cnvi/cnvi.c<br> (cnvb_fill_ssdt()), and it is emitted only by the PCI Bluetooth = driver<br> (cnvi_bt_ops, bound to bt_pci_device_ids[]).<br> <br> When CNVi Bluetooth is on USB there is no PCI BT function, so<br> cnvb_fill_ssdt() never runs and no _PRR is generated. The kernel = logs:<br> <br> Bluetooth: hci0: No support for _PRR ACPI method<br> <br> and cannot perform the platform-level reset. (The stock vendor firmware = on<br> these boards also omits it, so this is not a regression - but = coreboot<br> already has the PLDR logic and could provide the _PRR here too.)<br> <br> What the reset actually is (hardware-verified, Alder Lake-N)<br> -----------------------------------------------------------<br> <br> On the board the CNVi Bluetooth companion enumerates as a USB2 = device<br> under \_SB.PCI0.XHCI.RHUB.HS10. The PLDR is a CNVi-sideband PCR = write:<br> <br> * reset register =3D PCR port 0x73, offset 0x80 (0xfd730080 = MMIO),<br> CNVI_ABORT_PLDR value 0x80 (non-PCH-S) - matching the = constants<br> already in cnvi.c (CNVI_ABORT_PLDR, PID_CNVI).<br> * readback reports CNVI_READY (0x04) at idle and PRRS reports<br> completion - identical semantics to the PCI path.<br> <br> So the reset operation is the same as the existing PCI PLDR; only the = ACPI<br> device scope that carries the _PRR differs (a USB device node instead = of<br> the PCI BT function). I currently work around this with a = hand-written<br> power resource in the board dsdt.asl (a Scope(HS10) with a _RST = doing<br> PCRO(0x73, 0x80, 0x03) plus Name(_PRR)). It works, but every = USB-CNVi-BT<br> board would re-implement the same ASL by hand.<br> <br> Proposed mechanism<br> ------------------<br> <br> Let a board mark the USB port that carries the CNVi Bluetooth = companion,<br> and have coreboot emit the existing PLDR power resource + _PRR under = that<br> USB device's ACPI scope, reusing the PCI path's logic:<br> <br> 1. Refactor the PLDR acpigen out of cnvb_fill_ssdt() into a = reusable<br> helper in soc/intel/common/block/cnvi, e.g.<br> cnvi_acpi_write_bt_pldr(), called by both the PCI BT = driver and the<br> new USB path (single source of truth for the PLDR = ASL).<br> <br> 2. Add an opt-in to drivers/usb/acpi config<br> (struct drivers_usb_acpi_config), e.g.<br> bool is_intel_cnvi_bluetooth;<br> <br> 3. In drivers/usb/acpi/usb_acpi.c, when the flag is set, call the = helper<br> inside the USB device's scope (alongside the = _DSD/_PRW it already<br> emits).<br> <br> 4. The board enables it on the BT USB port in devicetree:<br> <br> chip drivers/usb/acpi<br> register = "is_intel_cnvi_bluetooth" =3D "1"<br> device usb x.y on = end # the CNVi BT companion port<br> end<br> <br> The PLDR is already pure acpigen, and the SoC exposes the PCRO/PCRR<br> sideband ASL helpers (src/soc/intel/common/acpi/pch_pcr.asl), so the<br> helper is pure ACPI generation with no SoC C callback - it writes a = method<br> that calls the existing PCRR/PCRO (exactly what the board workaround = does<br> today). drivers/usb/acpi only needs to know whether to emit the = resource,<br> not how the reset works.<br> <br> Alternatives considered<br> -----------------------<br> <br> * Board-local DSDT (status quo): works, but duplicates the PLDR = ASL per<br> board and drifts from cnvi.c.<br> * A dedicated drivers/intel/cnvi_usb_bt chip: heavier; = drivers/usb/acpi<br> already owns the USB device's ACPI node, so a flag there = is lighter.<br> * Auto-detection of the BT USB port: not feasible generically - = which<br> internal port carries the companion is board-specific.<br> <br> Open questions<br> --------------<br> <br> 1. Is drivers/usb/acpi the right home for the flag, or would you = prefer a<br> thin shim driver?<br> 2. Should the flag instead be derived from the existing CNVi = devicetree<br> config so a board describes "CNVi BT is on USB port = X" in one place?<br> 3. Naming: is_intel_cnvi_bluetooth vs cnvi_bt_pldr vs reusing a = CNVi enum.<br> 4. Should the shared PLDR acpigen helper live in the common CNVi = block or<br> in a small ACPI helper next to pch_pcr.asl?<br> <br> If the direction is agreeable I'll send a patch (with the refactor + = a<br> board using it) and verify on the hardware.<br> <br> Thanks,<br> Sebastian<br> _______________________________________________<br> coreboot mailing list -- <a href=3D"mailto:[email protected]" = target=3D"_blank">[email protected]</a><br> To unsubscribe send an email to <a = href=3D"mailto:[email protected]" = target=3D"_blank">[email protected]</a><br> </blockquote></div> </div></blockquote></div><br></body></html>= --Apple-Mail=_58765EE8-F2D4-4B94-AC3C-D120801417EA-- --===============1205947584243177445== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ coreboot mailing list -- [email protected] To unsubscribe send an email to [email protected] --===============1205947584243177445==--