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 PM 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 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]>: > > 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 PM Sebastian Müller <[email protected]> > 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 the >> SoC/PCH configuration: >> >> * 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). >> >> 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 = 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 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. >> >> 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 that >> 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 helper >> 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" = "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 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. >> >> Alternatives considered >> ----------------------- >> >> * 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. >> >> Open questions >> -------------- >> >> 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? >> >> 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] >
_______________________________________________ coreboot mailing list -- [email protected] To unsubscribe send an email to [email protected]
