Re: [RFC] Platform-Level Device Reset (_PRR) for CNVi Bluetooth over USB
Matt DeVillier <[email protected]> Wed, 1 Jul 2026 13:56:25 -0500
| Newsgroups | gmane.linux.bios |
|---|---|
| Message-ID | <CAFTm+6AXiJGZ0GW99orwvUZTs4JrJZv6q5X2xfQPc9ZuzvipOg@mail.gmail.com> |
--===============6995716830800801290== Content-Type: multipart/alternative; boundary="000000000000485e29065591401c" --000000000000485e29065591401c Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Perhaps refactor is_intel_bt() to do the sideband reset if the GPIOs are not specified? location wise, maybe put in acpigen.c and pass in the params needed to populate? pushing to gerrit and linking here is the best way to get the most eyes on it. On Wed, Jul 1, 2026 at 1:53=E2=80=AFPM Basti <[email protected]> wrote: > 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 o= f > 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]>: > > hi Sebastian, > > 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 > > Is this not sufficient? If not, can it be adapted/extended to do what you > need? > > -Matt > > On Sat, Jun 27, 2026 at 12:16=E2=80=AFPM Sebastian M=C3=BCller <thxgiving= @gmail.com> > wrote: > >> Hi all, >> >> 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. >> >> Problem >> ------- >> >> Intel CNVi Bluetooth reaches the host in one of two ways depending on th= e >> SoC/PCH configuration: >> >> * as a PCI function (Tiger Lake, Panther Lake, Nova Lake, ... - the ID= s >> 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). >> >> 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[]). >> >> 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: >> >> Bluetooth: hci0: No support for _PRR ACPI method >> >> 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.) >> >> What the reset actually is (hardware-verified, Alder Lake-N) >> ----------------------------------------------------------- >> >> 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: >> >> * 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. >> >> So the reset operation is the same as the existing PCI PLDR; only the AC= PI >> 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. >> >> Proposed mechanism >> ------------------ >> >> Let a board mark the USB port that carries the CNVi Bluetooth companion, >> and have coreboot emit the existing PLDR power resource + _PRR under tha= t >> USB device's ACPI scope, reusing the PCI path's logic: >> >> 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). >> >> 2. Add an opt-in to drivers/usb/acpi config >> (struct drivers_usb_acpi_config), e.g. >> bool is_intel_cnvi_bluetooth; >> >> 3. In drivers/usb/acpi/usb_acpi.c, when the flag is set, call the help= er >> inside the USB device's scope (alongside the _DSD/_PRW it already >> emits). >> >> 4. The board enables it on the BT USB port in devicetree: >> >> chip drivers/usb/acpi >> register "is_intel_cnvi_bluetooth" =3D "1" >> device usb x.y on end # the CNVi BT companion port >> end >> >> 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 meth= od >> that calls the existing PCRR/PCRO (exactly what the board workaround doe= s >> today). drivers/usb/acpi only needs to know whether to emit the resource= , >> not how the reset works. >> >> Alternatives considered >> ----------------------- >> >> * Board-local DSDT (status quo): works, but duplicates the PLDR ASL pe= r >> board and drifts from cnvi.c. >> * A dedicated drivers/intel/cnvi_usb_bt chip: heavier; drivers/usb/acp= i >> 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. >> >> Open questions >> -------------- >> >> 1. Is drivers/usb/acpi the right home for the flag, or would you prefe= r >> a >> thin shim driver? >> 2. Should the flag instead be derived from the existing CNVi devicetre= e >> 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? >> >> If the direction is agreeable I'll send a patch (with the refactor + a >> board using it) and verify on the hardware. >> >> Thanks, >> Sebastian >> _______________________________________________ >> coreboot mailing list -- [email protected] >> To unsubscribe send an email to [email protected] >> > > _______________________________________________ > coreboot mailing list -- [email protected] > To unsubscribe send an email to [email protected] > --000000000000485e29065591401c Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable <div dir=3D"ltr">Perhaps refactor is_intel_bt() to do the sideband reset if= the GPIOs are not specified?<br><br>location wise, maybe put in acpigen.c = and pass in the params needed to populate?<br><br>pushing to gerrit and lin= king here is the best way to get the most eyes on it.</div><br><div class= =3D"gmail_quote gmail_quote_container"><div dir=3D"ltr" class=3D"gmail_attr= ">On Wed, Jul 1, 2026 at 1:53=E2=80=AFPM Basti <<a href=3D"mailto:thxgiv= [email protected]">[email protected]</a>> wrote:<br></div><blockquote clas= s=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid r= gb(204,204,204);padding-left:1ex"><div>Hi Matt,<br><br>Thanks, that's a= good point and thanks for your thoughts about this -<br>I'd really mis= sed is_intel_bluetooth, and you're right that it already generates<br>t= he _PRR and the BTRT power resource under the USB device scope.<br>That alr= eady solves the "no PCI function, so nothing gets emitted" half o= f my problem.<br>So let me narrow the RFC down to the one piece it doesn= 9;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_co= unt. Give it no GPIOs and _RST is an<br>empty method.<br><br>2. CNVi sideba= nd PLDR - no pin at all; the reset is a sideband<br>register write, PCRO(PI= D_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>de= vice, 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>pi= n. So "supply the enable/reset GPIOs" doesn't apply here: I h= ave<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_bluetoot= h can emit the _PRR<br>scaffolding for a USB BT device, but its only _RST i= mplementation is the<br>GPIO one. The sideband-PLDR _RST that a CNVi part n= eeds lives in cnvi.c<br>and is reachable only from the PCI path.<br><br>Giv= en 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_bluetoo= th with a PLDR reset variant. Concretely - factor the<br>PLDR _RST/CNVP acp= igen 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 plum= bing you already have, one source of truth for<br>the PLDR ASL, and it repl= aces 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 pat= ch with<br>the helper refactor + the reset-variant selection, so cnvi usb b= luetooth can be reset via the<br>sideband PLDR if GPIO is not available.<br= ><br>Thanks,<br>Sebastian<br id=3D"m_-2851947046461812738lineBreakAtBeginni= ngOfMessage"><div><br><blockquote type=3D"cite"><div>Am 01.07.2026 um 16:02= schrieb Matt DeVillier <<a href=3D"mailto:[email protected]" tar= get=3D"_blank">[email protected]</a>>:</div><br><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_bluetoo= th' 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"><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:thxgiving@= gmail.com" target=3D"_blank">[email protected]</a>> wrote:<br></div><b= lockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-le= ft: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<b= r> SoC/PCH configuration:<br> <br> =C2=A0 * as a PCI function (Tiger Lake, Panther Lake, Nova Lake, ... - the = IDs<br> =C2=A0 =C2=A0 in bt_pci_device_ids[]), or<br> =C2=A0 * as a USB device on an internal XHCI port (Alder Lake-N and other<b= r> =C2=A0 =C2=A0 IoT / "-N" configurations, where the BT companion i= s CNVi-over-USB).<br> <br> coreboot implements the CNVi Bluetooth Platform-Level Device Reset (PLDR)<b= r> - 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> =C2=A0 =C2=A0 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> =C2=A0 * reset register =3D PCR port 0x73, offset 0x80 (0xfd730080 MMIO),<b= r> =C2=A0 =C2=A0 CNVI_ABORT_PLDR value 0x80 (non-PCH-S) - matching the constan= ts<br> =C2=A0 =C2=A0 already in cnvi.c (CNVI_ABORT_PLDR, PID_CNVI).<br> =C2=A0 * readback reports CNVI_READY (0x04) at idle and PRRS reports<br> =C2=A0 =C2=A0 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<b= r> USB device's ACPI scope, reusing the PCI path's logic:<br> <br> =C2=A0 1. Refactor the PLDR acpigen out of cnvb_fill_ssdt() into a reusable= <br> =C2=A0 =C2=A0 =C2=A0helper in soc/intel/common/block/cnvi, e.g.<br> =C2=A0 =C2=A0 =C2=A0cnvi_acpi_write_bt_pldr(), called by both the PCI BT dr= iver and the<br> =C2=A0 =C2=A0 =C2=A0new USB path (single source of truth for the PLDR ASL).= <br> <br> =C2=A0 2. Add an opt-in to drivers/usb/acpi config<br> =C2=A0 =C2=A0 =C2=A0(struct drivers_usb_acpi_config), e.g.<br> =C2=A0 =C2=A0 =C2=A0bool is_intel_cnvi_bluetooth;<br> <br> =C2=A0 3. In drivers/usb/acpi/usb_acpi.c, when the flag is set, call the he= lper<br> =C2=A0 =C2=A0 =C2=A0inside the USB device's scope (alongside the _DSD/_= PRW it already<br> =C2=A0 =C2=A0 =C2=A0emits).<br> <br> =C2=A0 4. The board enables it on the BT USB port in devicetree:<br> <br> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0chip drivers/usb/acpi<br> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0register "is_intel_cnv= i_bluetooth" =3D "1"<br> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0device usb x.y on end=C2=A0= =C2=A0# the CNVi BT companion port<br> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0end<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<b= r> today). drivers/usb/acpi only needs to know whether to emit the resource,<b= r> not how the reset works.<br> <br> Alternatives considered<br> -----------------------<br> <br> =C2=A0 * Board-local DSDT (status quo): works, but duplicates the PLDR ASL = per<br> =C2=A0 =C2=A0 board and drifts from cnvi.c.<br> =C2=A0 * A dedicated drivers/intel/cnvi_usb_bt chip: heavier; drivers/usb/a= cpi<br> =C2=A0 =C2=A0 already owns the USB device's ACPI node, so a flag there = is lighter.<br> =C2=A0 * Auto-detection of the BT USB port: not feasible generically - whic= h<br> =C2=A0 =C2=A0 internal port carries the companion is board-specific.<br> <br> Open questions<br> --------------<br> <br> =C2=A0 1. Is drivers/usb/acpi the right home for the flag, or would you pre= fer a<br> =C2=A0 =C2=A0 =C2=A0thin shim driver?<br> =C2=A0 2. Should the flag instead be derived from the existing CNVi devicet= ree<br> =C2=A0 =C2=A0 =C2=A0config so a board describes "CNVi BT is on USB por= t X" in one place?<br> =C2=A0 3. Naming: is_intel_cnvi_bluetooth vs cnvi_bt_pldr vs reusing a CNVi= enum.<br> =C2=A0 4. Should the shared PLDR acpigen helper live in the common CNVi blo= ck or<br> =C2=A0 =C2=A0 =C2=A0in 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]= rg" target=3D"_blank">[email protected]</a><br> </blockquote></div> </div></blockquote></div><br></div>________________________________________= _______<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]= rg" target=3D"_blank">[email protected]</a><br> </blockquote></div> --000000000000485e29065591401c-- --===============6995716830800801290== 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] --===============6995716830800801290==--