Re: [PATCH v6] Bluetooth: btintel_pcie: Add vendor_reset PCI sysfs for PLDR

[Alex Williamson <alex@shazbot.org>]

On Mon, 27 Jul 2026 10:51:02 +0530
Chandrashekar Devegowda <chandrashekar.devegowda@intel.com> wrote:

> Add a read-write sysfs entry at /sys/bus/pci/devices/<BDF>/vendor_reset
> to allow userspace to trigger PLDR (Product Level Device Reset).
> Reading the attribute displays supported reset types. Writing
> integer 0 triggers PLDR. Any other input is rejected with
> -EINVAL and a warning log.
> 
> Signed-off-by: Chandrashekar Devegowda <chandrashekar.devegowda@intel.com>
> ---
> Changes in v6:
>   - Fixed .driver.dev_groups -> .dev_groups on struct pci_driver
>     (__pci_register_driver overwrites .driver.dev_groups with .dev_groups)
>   - Removed unrelated schedule_work() failure handling from
>     btintel_pcie_request_reset() that caused CI context mismatch
> 
> Changes in v5:
>   - Renamed sysfs from vendor_rst to vendor_reset (reviewer feedback)
>   - Switched from device_create_file() to driver dev_groups for
>     automatic race-free sysfs lifecycle management
>   - Added ABI doc at Documentation/ABI/testing/sysfs-bus-pci-drivers-btintel_pcie
>   - Added WiFi impact description to ABI documentation
>   - Added MAINTAINERS F: entry for ABI doc
>   - v5: https://lore.kernel.org/all/20260723063520.1149474-1-chandrashekar.devegowda@intel.com/
> 
> Changes in v4:
>   - Rebased on latest bluetooth-next to fix CI apply failure
>   - v4: https://lore.kernel.org/all/20260722012158.813793-1-chandrashekar.devegowda@intel.com/
> 
> Changes in v3:
>   - Dropped reset_type parameter approach from hdev->reset()
>   - Directly call btintel_pcie_request_reset() instead of manual
>     flag manipulation and schedule_work()
>   - Accept only integer 0 for PLDR trigger
>   - v3: https://lore.kernel.org/all/20260722001434.804931-1-chandrashekar.devegowda@intel.com/
> 
> Changes in v2:
>   - Added reset_type parameter to hdev->reset() callback (1/2)
>   - vendor_rst sysfs used reset_type to select PLDR (2/2)
>   - v2: https://lore.kernel.org/all/20260618085016.9173-1-chandrashekar.devegowda@intel.com/
> 
> Changes in v1:
>   - Initial implementation
>   - v1: https://lore.kernel.org/all/20260612012832.2395034-1-chandrashekar.devegowda@intel.com/
>  .../sysfs-bus-pci-drivers-btintel_pcie        | 15 ++++++++
>  MAINTAINERS                                   |  1 +
>  drivers/bluetooth/btintel_pcie.c              | 38 +++++++++++++++++++
>  3 files changed, 54 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-pci-drivers-btintel_pcie
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-pci-drivers-btintel_pcie b/Documentation/ABI/testing/sysfs-bus-pci-drivers-btintel_pcie
> new file mode 100644
> index 000000000000..cceec6ac96bc
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-pci-drivers-btintel_pcie
> @@ -0,0 +1,15 @@
> +What:		/sys/bus/pci/devices/<BDF>/vendor_reset

Nit, domain is present too, not just BDF.

> +Date:		22-Jul-2026
> +KernelVersion:	6.17

Kernel 6.17 was 10 months ago.

> +Contact:	linux-bluetooth@vger.kernel.org
> +Description:	This read-write attribute allows userspace to trigger a
> +		Product Level Device Reset (PLDR) on Intel PCIe Bluetooth
> +		controllers. Reading the attribute displays the supported
> +		reset type. Writing integer 0 triggers PLDR. Any other
> +		input is rejected with -EINVAL.

This is opposite of /sys/bus/pci/devices/<BDF>/reset for no apparent
reason and seems like it infringes on the namespace of sysfs-bus-pci.

> +
> +		PLDR resets the entire on-chip platform shared between
> +		Bluetooth and WiFi. This means any driver attached to
> +		the WiFi device that shares hardware with this Bluetooth
> +		device will be released, the platform will be reset, and
> +		both the Bluetooth and WiFi devices will be re-probed.
> diff --git a/MAINTAINERS b/MAINTAINERS
> index eb8cdcc76324..39d390f1ceb4 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4625,6 +4625,7 @@ S:	Supported
>  W:	http://www.bluez.org/
>  T:	git git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth.git
>  T:	git git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git
> +F:	Documentation/ABI/testing/sysfs-bus-pci-drivers-btintel_pcie
>  F:	Documentation/devicetree/bindings/net/bluetooth/
>  F:	drivers/bluetooth/
>  
> diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
> index ef42b8d11d4d..005c77a4f5eb 100644
> --- a/drivers/bluetooth/btintel_pcie.c
> +++ b/drivers/bluetooth/btintel_pcie.c
> @@ -2790,6 +2790,43 @@ static void btintel_pcie_hci_reset(struct hci_dev *hdev)
>  	btintel_pcie_request_reset(data, BTINTEL_PCIE_IOSF_PRR_FLR);
>  }
>  
> +static ssize_t vendor_reset_store(struct device *dev,
> +				  struct device_attribute *attr,
> +				  const char *buf, size_t count)
> +{
> +	unsigned int val;
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +	struct btintel_pcie_data *data = pci_get_drvdata(pdev);
> +
> +	if (!data || !data->hdev)
> +		return -ENODEV;
> +
> +	if (kstrtouint(buf, 10, &val) || val != 0) {
> +		bt_dev_warn(data->hdev, "PLDR rejected: invalid input");
> +		return -EINVAL;
> +	}
> +
> +	bt_dev_info(data->hdev, "PLDR triggered via sysfs");
> +	btintel_pcie_request_reset(data, BTINTEL_PCIE_IOSF_PRR_PLDR);

This is pretty scary on its own, scanning for specific wifi device IDs,
finding the first match and releasing the driver for it while holding
pci_lock_rescan_remove().  So an attribute on one device that unbinds
the driver for another device (or worse, maybe reset it anyway if the
SKUs are out of sync), and the reset happens asynchronously at some
point in the future.  If the wifi device happens to be in use by
vfio-pci, that delay is unbounded as it relies on a userspace driver or
VM to release the device.

> +
> +	return count;
> +}
> +
> +static ssize_t vendor_reset_show(struct device *dev,
> +				 struct device_attribute *attr, char *buf)
> +{
> +	return sysfs_emit(buf, "0 - PLDR\n");

It seems like this is implying an enumerable list of resets, but the
API is committing to zero/-EINVAL.  The string also isn't very sysfs
compliant.  Thanks,

Alex

> +}
> +
> +static DEVICE_ATTR_RW(vendor_reset);
> +
> +static struct attribute *btintel_pcie_attrs[] = {
> +	&dev_attr_vendor_reset.attr,
> +	NULL,
> +};
> +
> +ATTRIBUTE_GROUPS(btintel_pcie);
> +
>  static void btintel_pcie_hw_error(struct hci_dev *hdev, u8 code)
>  {
>  	struct btintel_pcie_dev_recovery *rec;
> @@ -3250,6 +3287,7 @@ static struct pci_driver btintel_pcie_driver = {
>  	.probe = btintel_pcie_probe,
>  	.remove = btintel_pcie_remove,
>  	.driver.pm = pm_sleep_ptr(&btintel_pcie_pm_ops),
> +	.dev_groups = btintel_pcie_groups,
>  #ifdef CONFIG_DEV_COREDUMP
>  	.driver.coredump = btintel_pcie_coredump
>  #endif