[PATCH v1] Bluetooth: btintel_pcie: Refactor FLR to use device_reprobe()

[PATCH v1] Bluetooth: btintel_pcie: Refactor FLR to use device_reprobe()

[Kiran K]

The FLR branch in btintel_pcie_reset_work() open-coded the entire
re-init sequence: btintel_pcie_release_hdev() (hci_unregister_dev +
hci_free_dev), pci_try_reset_function(), enable_interrupts /
config_msix / enable_bt / reset_ia / start_rx, then
btintel_pcie_setup_hdev() (hci_alloc_dev_priv + hci_register_dev).
Every probe() init step had to be kept in sync with this second
copy in the reset path, and any failure mid-sequence left state to
unwind by hand.

The PLDR path already delegates teardown and re-init to the PCI
core via device_reprobe(): .remove() destroys data through devres
and unregisters hdev, then .probe() rebuilds everything from
scratch. Apply the same model to FLR.

Introduce btintel_pcie_perform_flr() mirroring perform_pldr(). It
runs pci_try_reset_function() (required to avoid the device_lock
ABBA against btintel_pcie_remove(), which calls
disable_work_sync(&reset_work) while holding device_lock) followed
by device_reprobe(). On success, data is destroyed and a fresh
probe re-INIT_WORKs coredump_work with disable count 0, so
enable_work() must not be called; on failure, data is still alive
and the caller balances the earlier disable_work_sync(). The
contract is documented on the helper and reiterated at the
reset_work() call site.

reset_work() shrinks to interrupt/worker drain, dispatch on
reset_type, and the single asymmetry between the two paths. The
out_enable label, the manual unregister/register pair, and the
forward declaration of btintel_pcie_setup_hdev() are dropped.

No intended functional change; FLR and PLDR now share one
teardown contract.

Fixes: 256ab9520d15 ("Bluetooth: btintel_pcie: Support Function level reset")
Assisted-by: GitHub-Copilot:claude-4.7-opus
Signed-off-by: Kiran K <kiran.k@intel.com>
---
 drivers/bluetooth/btintel_pcie.c | 99 ++++++++++++++++----------------
 1 file changed, 49 insertions(+), 50 deletions(-)

diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
index 9e39327dc1fe..41b28fbcc43b 100644
--- a/drivers/bluetooth/btintel_pcie.c
+++ b/drivers/bluetooth/btintel_pcie.c
@@ -2485,7 +2485,6 @@ 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 int btintel_pcie_acpi_reset_method(struct btintel_pcie_data *data)
@@ -2596,12 +2595,45 @@ static void btintel_pcie_perform_pldr(struct btintel_pcie_data *data)
 	}
 }
 
+/*
+ * Issue a Function Level Reset and hand teardown/re-init off to the PCI
+ * core via device_reprobe(), mirroring the PLDR path's contract.
+ *
+ * Caller must hold pci_lock_rescan_remove() and must have already
+ * disabled interrupts and drained both rx_work and coredump_work.
+ */
+static int btintel_pcie_perform_flr(struct btintel_pcie_data *data)
+{
+	struct pci_dev *pdev = data->pdev;
+	int err;
+
+	/* pci_try_reset_function() avoids the device_lock ABBA against
+	 * btintel_pcie_remove(): .remove() runs with device_lock held and
+	 * then waits for this work via disable_work_sync(); the blocking
+	 * pci_reset_function() would deadlock by trying to re-acquire
+	 * device_lock here.
+	 */
+	err = pci_try_reset_function(pdev);
+	if (err) {
+		BT_ERR("Failed resetting the pcie device (%d)", err);
+		return err;
+	}
+
+	/* device_reprobe() always detaches the driver first (running
+	 * .remove(), which frees 'data'); any re-probe failure leaves the
+	 * device unbound but 'data' is already gone, so just log it.
+	 */
+	if (device_reprobe(&pdev->dev))
+		BT_ERR("BT reprobe failed for BDF:%s", pci_name(pdev));
+
+	return 0;
+}
+
 static void btintel_pcie_reset_work(struct work_struct *wk)
 {
 	struct btintel_pcie_data *data =
 		container_of(wk, struct btintel_pcie_data, reset_work);
 	struct pci_dev *pdev = data->pdev;
-	int err;
 
 	pci_lock_rescan_remove();
 
@@ -2621,60 +2653,27 @@ static void btintel_pcie_reset_work(struct work_struct *wk)
 	disable_work_sync(&data->coredump_work);
 
 	bt_dev_dbg(data->hdev, "Release bluetooth interface");
+
+	/* Both reset paths follow the same contract: on success they
+	 * destroy 'data' via device_reprobe() (a fresh probe re-INIT_WORKs
+	 * the coredump_work with disable count 0), so enable_work() must
+	 * NOT be called on the success path. Only the FLR path can fail
+	 * with 'data' still alive, in which case we balance the
+	 * disable_work_sync() above so a later successful reset is not
+	 * permanently blocked.
+	 *
+	 * pci_lock_rescan_remove() (held above) serializes against PCI
+	 * device addition/removal (hotplug), so no device can be added to
+	 * or removed from the bus list while this code runs.
+	 */
 	if (data->reset_type == BTINTEL_PCIE_IOSF_PRR_PLDR) {
-		/* This function holds pci_lock_rescan_remove(), which acquires
-		 * pci_rescan_remove_lock. This mutex serializes against PCI device
-		 * addition/removal (hotplug), so no device can be added to or
-		 * removed from the bus list while this code runs.
-		 *
-		 * device_reprobe() inside btintel_pcie_perform_pldr() destroys
-		 * 'data' via .remove(); a fresh probe re-INIT_WORKs the
-		 * coredump_work with disable count 0, so we must not call
-		 * enable_work() on this path.
-		 */
 		btintel_pcie_perform_pldr(data);
 		goto out;
 	}
-	btintel_pcie_release_hdev(data);
 
-	/* Use pci_try_reset_function() rather than pci_reset_function() to
-	 * avoid an ABBA deadlock against btintel_pcie_remove(): the PCI core
-	 * calls .remove() with device_lock held, and remove() then waits for
-	 * this work via cancel_work_sync(); pci_reset_function() would in
-	 * turn try to acquire the same device_lock, deadlocking both paths.
-	 */
-	err = pci_try_reset_function(pdev);
-	if (err) {
-		BT_ERR("Failed resetting the pcie device (%d)", err);
-		goto out_enable;
-	}
-
-	btintel_pcie_enable_interrupts(data);
-	btintel_pcie_config_msix(data);
+	if (btintel_pcie_perform_flr(data))
+		enable_work(&data->coredump_work);
 
-	err = btintel_pcie_enable_bt(data);
-	if (err) {
-		BT_ERR("Failed to enable bluetooth hardware after reset (%d)",
-		       err);
-		goto out_enable;
-	}
-
-	btintel_pcie_reset_ia(data);
-	btintel_pcie_start_rx(data);
-	data->flags = 0;
-
-	err = btintel_pcie_setup_hdev(data);
-	if (err) {
-		BT_ERR("Failed registering hdev (%d)", err);
-		goto out_enable;
-	}
-
-out_enable:
-	/* Balance disable_work_sync() above on every exit. Leaving the
-	 * counter incremented on a failed reset would permanently disable
-	 * coredump_work even after a later successful reset.
-	 */
-	enable_work(&data->coredump_work);
 out:
 	pci_dev_put(pdev);
 	pci_unlock_rescan_remove();
-- 
2.54.0

RE: [v1] Bluetooth: btintel_pcie: Refactor FLR to use device_reprobe()

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1181 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=1118318

---Test result---

Test Summary:
CheckPatch                    PASS      1.27 seconds
VerifyFixes                   PASS      0.20 seconds
VerifySignedoff               PASS      0.20 seconds
GitLint                       PASS      0.72 seconds
SubjectPrefix                 PASS      0.26 seconds
BuildKernel                   PASS      25.29 seconds
CheckAllWarning               PASS      28.44 seconds
CheckSparse                   PASS      27.39 seconds
BuildKernel32                 PASS      32.92 seconds
CheckKernelLLVM               SKIP      0.00 seconds
TestRunnerSetup               PASS      458.70 seconds
IncrementalBuild              PASS      24.94 seconds

Details
##############################
Test: CheckKernelLLVM - SKIP
Desc: Build kernel with LLVM + context analysis
Output:
Clang not found


https://github.com/bluez/bluetooth-next/pull/369

---
Regards,
Linux Bluetooth