[PATCH v2 1/2] Bluetooth: hci_core: Add reset_type parameter to hdev->reset() callback

[PATCH v2 1/2] Bluetooth: hci_core: Add reset_type parameter to hdev->reset() callback

[Chandrashekar Devegowda]

Add a u8 reset_type parameter to the hdev->reset() callback to allow
userspace to select the reset level via sysfs. Each driver interprets
the level according to its own capabilities.

The reset_type values are:
  0 - default reset
  1 - deeper reset

Writing any value other than 1 defaults to level 0.

Internal callers (command timeout, suspend/resume, coredump)
default to level 0.

All drivers implementing the reset callback are updated to accept
the new parameter:
  - btusb: btusb_intel_reset, btusb_qca_reset, btusb_rtl_reset
  - hci_qca: qca_reset
  - btmtksdio: btmtksdio_reset
  - btmtk: btmtk_reset_sync
  - btnxpuart: nxp_reset
  - btintel_pcie: btintel_pcie_reset

Signed-off-by: Chandrashekar Devegowda <chandrashekar.devegowda@intel.com>
---
v2:
  - Use generic reset level terminology instead of Intel-specific
    FLR/PLDR in sysfs ABI documentation and commit message (Luiz)
  - Fix docutils formatting warning by adding blank lines around
    the indented list in sysfs-class-bluetooth (kernel test robot)
v1:
  - Initial version
---
 Documentation/ABI/stable/sysfs-class-bluetooth |  9 ++++++++-
 drivers/bluetooth/btintel_pcie.c               | 17 +++++++++++------
 drivers/bluetooth/btmtk.c                      |  6 +++---
 drivers/bluetooth/btmtk.h                      |  4 ++--
 drivers/bluetooth/btmtksdio.c                  |  2 +-
 drivers/bluetooth/btnxpuart.c                  |  2 +-
 drivers/bluetooth/btusb.c                      |  6 +++---
 drivers/bluetooth/hci_qca.c                    |  2 +-
 include/net/bluetooth/hci_core.h               |  2 +-
 net/bluetooth/hci_core.c                       |  2 +-
 net/bluetooth/hci_sysfs.c                      | 10 ++++++++--
 11 files changed, 40 insertions(+), 22 deletions(-)

diff --git a/Documentation/ABI/stable/sysfs-class-bluetooth b/Documentation/ABI/stable/sysfs-class-bluetooth
index 36be02471174..fb445e20f972 100644
--- a/Documentation/ABI/stable/sysfs-class-bluetooth
+++ b/Documentation/ABI/stable/sysfs-class-bluetooth
@@ -3,7 +3,14 @@ Date:		14-Jan-2025
 KernelVersion:	6.13
 Contact:	linux-bluetooth@vger.kernel.org
 Description: 	This write-only attribute allows users to trigger the vendor reset
-		method on the Bluetooth device when arbitrary data is written.
+		method on the Bluetooth device. The value written selects the
+		reset level. Each driver interprets the level according to its
+		own capabilities:
+
+		  - 0: default reset
+		  - 1: deeper reset
+
+		Writing any value other than 1 defaults to level 0.
 		The reset may or may not be done through the device transport
 		(e.g., UART/USB), and can also be done through an out-of-band
 		approach such as GPIO.
diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
index 9e39327dc1fe..d3a03cf96421 100644
--- a/drivers/bluetooth/btintel_pcie.c
+++ b/drivers/bluetooth/btintel_pcie.c
@@ -2486,7 +2486,7 @@ static void btintel_pcie_inc_recovery_count(struct pci_dev *pdev,
 }
 
 static int btintel_pcie_setup_hdev(struct btintel_pcie_data *data);
-static void btintel_pcie_reset(struct hci_dev *hdev);
+static void btintel_pcie_reset(struct hci_dev *hdev, u8 reset_type);
 
 static int btintel_pcie_acpi_reset_method(struct btintel_pcie_data *data)
 {
@@ -2680,7 +2680,7 @@ static void btintel_pcie_reset_work(struct work_struct *wk)
 	pci_unlock_rescan_remove();
 }
 
-static void btintel_pcie_reset(struct hci_dev *hdev)
+static void btintel_pcie_reset(struct hci_dev *hdev, u8 reset_type)
 {
 	struct btintel_pcie_data *data;
 
@@ -2692,6 +2692,12 @@ static void btintel_pcie_reset(struct hci_dev *hdev)
 	if (test_and_set_bit(BTINTEL_PCIE_RECOVERY_IN_PROGRESS, &data->flags))
 		return;
 
+	data->reset_type = (reset_type == 1) ?
+			    BTINTEL_PCIE_IOSF_PRR_PLDR : BTINTEL_PCIE_IOSF_PRR_FLR;
+
+	bt_dev_info(hdev, "Reset triggered: %s",
+		    data->reset_type == BTINTEL_PCIE_IOSF_PRR_PLDR ? "PLDR" : "FLR");
+
 	pci_dev_get(data->pdev);
 	schedule_work(&data->reset_work);
 }
@@ -2729,7 +2735,7 @@ static void btintel_pcie_hw_error(struct hci_dev *hdev, u8 code)
 		return;
 	}
 	btintel_pcie_inc_recovery_count(pdev, &hdev->dev);
-	btintel_pcie_reset(hdev);
+	btintel_pcie_reset(hdev, (code == 0x13) ? 1 : 0);
 }
 
 static bool btintel_pcie_wakeup(struct hci_dev *hdev)
@@ -3111,8 +3117,7 @@ static int btintel_pcie_resume(struct device *dev)
 	if (data->pm_sx_event == PM_EVENT_FREEZE ||
 	    data->pm_sx_event == PM_EVENT_HIBERNATE) {
 		set_bit(BTINTEL_PCIE_CORE_HALTED, &data->flags);
-		data->reset_type = BTINTEL_PCIE_IOSF_PRR_FLR;
-		btintel_pcie_reset(data->hdev);
+		btintel_pcie_reset(data->hdev, 0);
 		return 0;
 	}
 
@@ -3143,7 +3148,7 @@ static int btintel_pcie_resume(struct device *dev)
 			queue_work(data->coredump_workqueue, &data->coredump_work);
 		}
 		set_bit(BTINTEL_PCIE_CORE_HALTED, &data->flags);
-		btintel_pcie_reset(data->hdev);
+		btintel_pcie_reset(data->hdev, 0);
 	}
 	return err;
 }
diff --git a/drivers/bluetooth/btmtk.c b/drivers/bluetooth/btmtk.c
index 02a96342e964..641f62912f63 100644
--- a/drivers/bluetooth/btmtk.c
+++ b/drivers/bluetooth/btmtk.c
@@ -104,7 +104,7 @@ static void btmtk_coredump_notify(struct hci_dev *hdev, int state)
 	case HCI_DEVCOREDUMP_ABORT:
 	case HCI_DEVCOREDUMP_DONE:
 		data->cd_info.state = HCI_DEVCOREDUMP_IDLE;
-		btmtk_reset_sync(hdev);
+		btmtk_reset_sync(hdev, 0);
 		break;
 	}
 }
@@ -384,7 +384,7 @@ int btmtk_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr)
 }
 EXPORT_SYMBOL_GPL(btmtk_set_bdaddr);
 
-void btmtk_reset_sync(struct hci_dev *hdev)
+void btmtk_reset_sync(struct hci_dev *hdev, u8 reset_type)
 {
 	struct btmtk_data *reset_work = hci_get_priv(hdev);
 	int err;
@@ -1403,7 +1403,7 @@ int btmtk_usb_setup(struct hci_dev *hdev)
 		if (err < 0) {
 			/* retry once if setup firmware error */
 			if (!test_and_set_bit(BTMTK_FIRMWARE_DL_RETRY, &btmtk_data->flags))
-				btmtk_reset_sync(hdev);
+				btmtk_reset_sync(hdev, 0);
 			bt_dev_err(hdev, "Failed to set up firmware (%d)", err);
 			return err;
 		}
diff --git a/drivers/bluetooth/btmtk.h b/drivers/bluetooth/btmtk.h
index c83c24897c95..5cda42444e94 100644
--- a/drivers/bluetooth/btmtk.h
+++ b/drivers/bluetooth/btmtk.h
@@ -196,7 +196,7 @@ int btmtk_setup_firmware_79xx(struct hci_dev *hdev, const char *fwname,
 int btmtk_setup_firmware(struct hci_dev *hdev, const char *fwname,
 			 wmt_cmd_sync_func_t wmt_cmd_sync);
 
-void btmtk_reset_sync(struct hci_dev *hdev);
+void btmtk_reset_sync(struct hci_dev *hdev, u8 reset_type);
 
 int btmtk_register_coredump(struct hci_dev *hdev, const char *name,
 			    u32 fw_version);
@@ -244,7 +244,7 @@ static inline int btmtk_setup_firmware(struct hci_dev *hdev, const char *fwname,
 	return -EOPNOTSUPP;
 }
 
-static inline void btmtk_reset_sync(struct hci_dev *hdev)
+static inline void btmtk_reset_sync(struct hci_dev *hdev, u8 reset_type)
 {
 }
 
diff --git a/drivers/bluetooth/btmtksdio.c b/drivers/bluetooth/btmtksdio.c
index c6f80c419e90..3c4401ce1e00 100644
--- a/drivers/bluetooth/btmtksdio.c
+++ b/drivers/bluetooth/btmtksdio.c
@@ -1269,7 +1269,7 @@ static int btmtksdio_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
 	return 0;
 }
 
-static void btmtksdio_reset(struct hci_dev *hdev)
+static void btmtksdio_reset(struct hci_dev *hdev, u8 reset_type)
 {
 	struct btmtksdio_dev *bdev = hci_get_drvdata(hdev);
 	u32 status;
diff --git a/drivers/bluetooth/btnxpuart.c b/drivers/bluetooth/btnxpuart.c
index e7036a48ce48..e2416b48116c 100644
--- a/drivers/bluetooth/btnxpuart.c
+++ b/drivers/bluetooth/btnxpuart.c
@@ -1539,7 +1539,7 @@ static bool nxp_wakeup(struct hci_dev *hdev)
 	return false;
 }
 
-static void nxp_reset(struct hci_dev *hdev)
+static void nxp_reset(struct hci_dev *hdev, u8 reset_type)
 {
 	struct btnxpuart_dev *nxpdev = hci_get_drvdata(hdev);
 
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 7f14ce96319b..ffe109b3b587 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -1045,7 +1045,7 @@ static void btusb_reset(struct hci_dev *hdev)
 	usb_queue_reset_device(data->intf);
 }
 
-static void btusb_intel_reset(struct hci_dev *hdev)
+static void btusb_intel_reset(struct hci_dev *hdev, u8 reset_type)
 {
 	struct btusb_data *data = hci_get_drvdata(hdev);
 	struct gpio_desc *reset_gpio = data->reset_gpio;
@@ -1123,7 +1123,7 @@ static inline void btusb_rtl_alloc_devcoredump(struct hci_dev *hdev,
 	}
 }
 
-static void btusb_rtl_reset(struct hci_dev *hdev)
+static void btusb_rtl_reset(struct hci_dev *hdev, u8 reset_type)
 {
 	struct btusb_data *data = hci_get_drvdata(hdev);
 	struct gpio_desc *reset_gpio = data->reset_gpio;
@@ -1167,7 +1167,7 @@ static void btusb_rtl_hw_error(struct hci_dev *hdev, u8 code)
 	btusb_rtl_alloc_devcoredump(hdev, &hdr, NULL, 0);
 }
 
-static void btusb_qca_reset(struct hci_dev *hdev)
+static void btusb_qca_reset(struct hci_dev *hdev, u8 reset_type)
 {
 	struct btusb_data *data = hci_get_drvdata(hdev);
 	struct gpio_desc *reset_gpio = data->reset_gpio;
diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index 244447195619..02b4afe77669 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -1693,7 +1693,7 @@ static void qca_hw_error(struct hci_dev *hdev, u8 code)
 	clear_bit(QCA_HW_ERROR_EVENT, &qca->flags);
 }
 
-static void qca_reset(struct hci_dev *hdev)
+static void qca_reset(struct hci_dev *hdev, u8 reset_type)
 {
 	struct hci_uart *hu = hci_get_drvdata(hdev);
 	struct qca_data *qca = hu->priv;
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 7e15da47fe3a..00421352fcb5 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -650,7 +650,7 @@ struct hci_dev {
 	int (*post_init)(struct hci_dev *hdev);
 	int (*set_diag)(struct hci_dev *hdev, bool enable);
 	int (*set_bdaddr)(struct hci_dev *hdev, const bdaddr_t *bdaddr);
-	void (*reset)(struct hci_dev *hdev);
+	void (*reset)(struct hci_dev *hdev, u8 reset_type);
 	bool (*wakeup)(struct hci_dev *hdev);
 	int (*set_quality_report)(struct hci_dev *hdev, bool enable);
 	int (*get_data_path_id)(struct hci_dev *hdev, __u8 *data_path);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 5ba9fe8261ec..360b329ae6da 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1435,7 +1435,7 @@ static void hci_cmd_timeout(struct work_struct *work)
 	}
 
 	if (hdev->reset)
-		hdev->reset(hdev);
+		hdev->reset(hdev, 0);
 
 	atomic_set(&hdev->cmd_cnt, 1);
 	queue_work(hdev->workqueue, &hdev->cmd_work);
diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
index 8957ce7c21b7..a4fe329158cf 100644
--- a/net/bluetooth/hci_sysfs.c
+++ b/net/bluetooth/hci_sysfs.c
@@ -97,8 +97,14 @@ static ssize_t reset_store(struct device *dev, struct device_attribute *attr,
 {
 	struct hci_dev *hdev = to_hci_dev(dev);
 
-	if (hdev->reset)
-		hdev->reset(hdev);
+	if (hdev->reset) {
+		int val;
+
+		if (kstrtoint(buf, 10, &val) || val != 1)
+			hdev->reset(hdev, 0);
+		else
+			hdev->reset(hdev, 1);
+	}
 
 	return count;
 }
-- 
2.43.0

[PATCH v2 2/2] Bluetooth: btintel_pcie: Fix TOCTOU race in reset path

[Chandrashekar Devegowda]

Move the test_and_set_bit(BTINTEL_PCIE_RECOVERY_IN_PROGRESS) check
before the SETUP_DONE check to fix a Time-Of-Check to Time-Of-Use
race. Previously, multiple callers could pass the SETUP_DONE check
concurrently and then race on the RECOVERY_IN_PROGRESS flag,
potentially scheduling conflicting removal work.

By reordering the existing atomic guard to execute first, concurrent
reset requests are atomically rejected before any state is inspected.
The RECOVERY_IN_PROGRESS flag is cleared on the early-exit path
(setup not done) to allow future reset attempts.

Signed-off-by: Chandrashekar Devegowda <chandrashekar.devegowda@intel.com>
---
v2:
  - No changes
v1:
  - Initial version
---
 drivers/bluetooth/btintel_pcie.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
index d3a03cf96421..f4f400421833 100644
--- a/drivers/bluetooth/btintel_pcie.c
+++ b/drivers/bluetooth/btintel_pcie.c
@@ -2686,11 +2686,15 @@ static void btintel_pcie_reset(struct hci_dev *hdev, u8 reset_type)
 
 	data = hci_get_drvdata(hdev);
 
-	if (!test_bit(BTINTEL_PCIE_SETUP_DONE, &data->flags))
+	if (test_and_set_bit(BTINTEL_PCIE_RECOVERY_IN_PROGRESS, &data->flags)) {
+		bt_dev_warn(hdev, "Reset rejected: recovery already in progress");
 		return;
+	}
 
-	if (test_and_set_bit(BTINTEL_PCIE_RECOVERY_IN_PROGRESS, &data->flags))
+	if (!test_bit(BTINTEL_PCIE_SETUP_DONE, &data->flags)) {
+		clear_bit(BTINTEL_PCIE_RECOVERY_IN_PROGRESS, &data->flags);
 		return;
+	}
 
 	data->reset_type = (reset_type == 1) ?
 			    BTINTEL_PCIE_IOSF_PRR_PLDR : BTINTEL_PCIE_IOSF_PRR_FLR;
-- 
2.43.0

Re: [PATCH v2 1/2] Bluetooth: hci_core: Add reset_type parameter to hdev->reset() callback

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] UAPI Breakage: Redefining '1' to mean 'deeper reset' breaks existing userspace scripts.

Pre-existing issues:
- [High] Data Race: Unguarded write to `dev_data->reset_type` in `btintel_pcie_hw_error()` clobbers concurrent sysfs resets.
- [High] TOCTOU Race Condition: Concurrent driver unbind causes NULL pointer dereference in `reset_store()`.
--

commit 4b14894786f1236a0de480b4c65121356669aba4
Author: Chandrashekar Devegowda <chandrashekar.devegowda@intel.com>

Bluetooth: hci_core: Add reset_type parameter to hdev->reset() callback

Add a u8 reset_type parameter to the hdev->reset() callback to allow
userspace to select the reset level via sysfs. Each driver interprets
the level according to its own capabilities.

> diff --git a/Documentation/ABI/stable/sysfs-class-bluetooth b/Documentation/ABI/stable/sysfs-class-bluetooth
> index 36be024711747..fb445e20f9726 100644
> --- a/Documentation/ABI/stable/sysfs-class-bluetooth
> +++ b/Documentation/ABI/stable/sysfs-class-bluetooth
> @@ -3,7 +3,14 @@ Date:		14-Jan-2025
>  KernelVersion:	6.13
>  Contact:	linux-bluetooth@vger.kernel.org
>  Description: 	This write-only attribute allows users to trigger the vendor reset
> -		method on the Bluetooth device when arbitrary data is written.
> +		method on the Bluetooth device. The value written selects the
> +		reset level. Each driver interprets the level according to its
> +		own capabilities:
> +
> +		  - 0: default reset
> +		  - 1: deeper reset
> +
> +		Writing any value other than 1 defaults to level 0.

[Severity: High]
Will redefining 1 to mean a deeper reset break existing userspace scripts? 

Historically, the reset sysfs attribute triggered the default reset when
arbitrary data was written, and in the Linux ecosystem, userspace typically
writes 1 to trigger boolean actions.

If existing scripts use echo 1 > reset, wouldn't they unknowingly trigger a
much more disruptive reset than before? Could we map 1 to the default reset
and use a distinct value (e.g., 2) for the deeper reset to maintain backward
compatibility?

[ ... ]

> diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
> index 9e39327dc1fe2..d3a03cf964216 100644
> --- a/drivers/bluetooth/btintel_pcie.c
> +++ b/drivers/bluetooth/btintel_pcie.c
[ ... ]
> @@ -2729,7 +2735,7 @@ static void btintel_pcie_hw_error(struct hci_dev *hdev, u8 code)
>  		return;
>  	}
>  	btintel_pcie_inc_recovery_count(pdev, &hdev->dev);
> -	btintel_pcie_reset(hdev);
> +	btintel_pcie_reset(hdev, (code == 0x13) ? 1 : 0);
>  }

[Severity: High]
This is a pre-existing issue, but does the unguarded write to
dev_data->reset_type earlier in btintel_pcie_hw_error() cause a data race?

While this patch updates btintel_pcie_reset() to safely assign the reset
type under the BTINTEL_PCIE_RECOVERY_IN_PROGRESS bitlock,
btintel_pcie_hw_error() still directly modifies dev_data->reset_type
without holding any structural locks or exclusion bits.

If a sysfs reset (FLR) is concurrently triggered and acquires the bitlock,
could this unguarded write in btintel_pcie_hw_error() silently overwrite the
pending reset type to PLDR, causing the asynchronously scheduled reset work
to read the corrupted type and perform the wrong level of reset?

[ ... ]

> diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
> index 8957ce7c21b76..a4fe329158cf7 100644
> --- a/net/bluetooth/hci_sysfs.c
> +++ b/net/bluetooth/hci_sysfs.c
> @@ -97,8 +97,14 @@ static ssize_t reset_store(struct device *dev, struct device_attribute *attr,
>  {
>  	struct hci_dev *hdev = to_hci_dev(dev);
>  
> -	if (hdev->reset)
> -		hdev->reset(hdev);
> +	if (hdev->reset) {
> +		int val;
> +
> +		if (kstrtoint(buf, 10, &val) || val != 1)
> +			hdev->reset(hdev, 0);
> +		else
> +			hdev->reset(hdev, 1);
> +	}
>  
>  	return count;
>  }

[Severity: High]
This is a pre-existing issue, but does checking hdev->reset and then calling
it after kstrtoint() introduce a TOCTOU race condition?

Because kstrtoint() is an external function call, it acts as a compiler
barrier, forcing the compiler to reload hdev->reset from memory before
invoking it as a function pointer.

If a concurrent teardown path sets hdev->reset = NULL between the check and
the execution, could reset_store() dereference a NULL pointer and crash the
kernel? While a smaller race window existed before, this patch significantly
widens it by inserting the kstrtoint() call. Should the function pointer be
cached in a local variable before the NULL check?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618085016.9173-1-chandrashekar.devegowda@intel.com?part=1

Re: [PATCH v2 2/2] Bluetooth: btintel_pcie: Fix TOCTOU race in reset path

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Kernel panic due to unconditionally dereferencing a NULL `hdev` pointer during PM resume if a prior reset failed.
- [High] Non-atomic modification of data->flags causes data race with atomic bit operations, risking lost coredumps and permanent lockouts.
- [High] Unprotected assignment to dev_data->reset_type in btintel_pcie_hw_error can corrupt an ongoing reset.
--

commit c9025f5524eb70c6235927f87e97777af4f0b9ab
Author: Chandrashekar Devegowda <chandrashekar.devegowda@intel.com>

Bluetooth: btintel_pcie: Fix TOCTOU race in reset path

This commit fixes a Time-Of-Check to Time-Of-Use race condition in the
reset path of the btintel_pcie driver. It reorders the RECOVERY_IN_PROGRESS
atomic guard to execute before checking the SETUP_DONE flag, preventing
concurrent reset requests from racing.

> diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
> index d3a03cf964216..f4f400421833b 100644
> --- a/drivers/bluetooth/btintel_pcie.c
> +++ b/drivers/bluetooth/btintel_pcie.c
> @@ -2686,11 +2686,15 @@ static void btintel_pcie_reset(struct hci_dev *hdev, u8 reset_type)
>  
>  	data = hci_get_drvdata(hdev);

[Severity: Critical]
This is a pre-existing issue, but can this dereference cause a kernel panic
during PM resume if a prior reset failed?

If a reset fails (e.g., in btintel_pcie_setup_hdev()), the device is
unregistered and data->hdev is left permanently set to NULL. If the system
goes into a suspend/hibernate cycle, the resume handler unconditionally
calls this reset function:

btintel_pcie_resume()
    set_bit(BTINTEL_PCIE_CORE_HALTED, &data->flags);
    btintel_pcie_reset(data->hdev, 0);

When btintel_pcie_reset() runs, it calls hci_get_drvdata(hdev), which blindly
dereferences hdev. Should there be a NULL pointer check for hdev here?

> -	if (!test_bit(BTINTEL_PCIE_SETUP_DONE, &data->flags))
> +	if (test_and_set_bit(BTINTEL_PCIE_RECOVERY_IN_PROGRESS, &data->flags)) {
> +		bt_dev_warn(hdev, "Reset rejected: recovery already in progress");
>  		return;
> +	}

[Severity: High]
This isn't a bug introduced by this patch, but since we are modifying atomic
flags operations, I noticed a potential data race with data->flags in the
worker thread:

btintel_pcie_reset_work()
    btintel_pcie_start_rx(data);
    data->flags = 0;
    err = btintel_pcie_setup_hdev(data);

Because data->flags is cleared non-atomically immediately after interrupts are
re-enabled, could this erase an atomic modification from an incoming MSI-X
interrupt, such as test_and_set_bit(BTINTEL_PCIE_HWEXP_INPROGRESS, &data->flags)?

Would it be safer to clear the specific bits using atomic operations instead?

> -	if (test_and_set_bit(BTINTEL_PCIE_RECOVERY_IN_PROGRESS, &data->flags))
> +	if (!test_bit(BTINTEL_PCIE_SETUP_DONE, &data->flags)) {
> +		clear_bit(BTINTEL_PCIE_RECOVERY_IN_PROGRESS, &data->flags);
>  		return;
> +	}
>  
>  	data->reset_type = (reset_type == 1) ?
>  			    BTINTEL_PCIE_IOSF_PRR_PLDR : BTINTEL_PCIE_IOSF_PRR_FLR;

[Severity: High]
This is a pre-existing issue, but I have a question about how the shared
reset_type state is handled concurrently.

In the hardware error handler, dev_data->reset_type is overwritten outside
of any lock before the atomic guard in btintel_pcie_reset() is reached:

btintel_pcie_hw_error()
    if (code == 0x13)
        dev_data->reset_type = BTINTEL_PCIE_IOSF_PRR_PLDR;
    else
        dev_data->reset_type = BTINTEL_PCIE_IOSF_PRR_FLR;
        
    btintel_pcie_reset(hdev, ...);

If btintel_pcie_reset_work() is actively executing a recovery and reads
data->reset_type locklessly, could this unprotected assignment cause the
worker thread to observe an inconsistent state and corrupt the recovery
sequence?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618085016.9173-1-chandrashekar.devegowda@intel.com?part=2