[PATCH v4 4/4] Bluetooth: btintel_pcie: use device_schedule_reprobe() after reset

[Daniel Golle]

The reset work re-probes its own device with device_reprobe() after an
FLR or a PLDR, so .remove() runs inside it. That needs the
current_work() check in .remove(), and 'data' must not be touched
after the call.

Schedule the re-probe with device_schedule_reprobe(). .remove() then
runs from the driver core's work, the current_work() check goes, and
disable_work_sync(&data->reset_work) waits for the reset work before
'data' is freed. Until the deferred detach 'data' stays alive but idle:
RECOVERY_IN_PROGRESS fails send_frame and coalesces further resets,
interrupts are masked, and the dump and rfkill workers stay disabled.
If scheduling fails in the FLR path, the reset work re-enables them as
on any FLR failure. The PLDR re-probe of the Wi-Fi sibling, a
different device, is unchanged.

Signed-off-by: Daniel Golle <daniel@xxxxxxxxxxxxxx>
---
v4: rebased onto bluetooth-next for its hardware rfkill work; commit
    message updated for the reworked helper
v3: rebased onto bluetooth-next, whose btintel_pcie has four dump
    workers (coredump/hwexp/fwtrigger/mbox) rather than the single one
    the mainline snapshot v2 targeted
v2: reconciled to the current tree and adapted the commit message
v1: initial RFC

 drivers/bluetooth/btintel_pcie.c | 47 ++++++++++++++++----------------
 1 file changed, 23 insertions(+), 24 deletions(-)

diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
index df95d16a163c8..9f65eb0ceb0b2 100644
--- a/drivers/bluetooth/btintel_pcie.c
+++ b/drivers/bluetooth/btintel_pcie.c
@@ -4093,7 +4093,7 @@ static void btintel_pcie_perform_pldr(struct btintel_pcie_data *data)
 	 * BT needs pci_save_state()/pci_restore_state() because the BT driver
 	 * is still partially attached when the _PRR runs (it hasn't been unbound yet).
 	 * The PCI device needs to remain minimally functional so that
-	 * device_reprobe(&pdev->dev) can work afterward
+	 * the deferred re-probe of the BT device can work afterward
 	 */
 	ret = btintel_pcie_acpi_reset_method(data);
 
@@ -4104,14 +4104,16 @@ static void btintel_pcie_perform_pldr(struct btintel_pcie_data *data)
 	}
 
 	if (!ret) {
-		if (device_reprobe(&pdev->dev))
-			BT_ERR("BT reprobe failed for BDF:%s", pci_name(pdev));
+		if (device_schedule_reprobe(&pdev->dev, 0))
+			BT_ERR("BT reprobe scheduling failed for BDF:%s",
+			       pci_name(pdev));
 	}
 }
 
 /*
- * Issue a Function Level Reset and hand teardown/re-init off to the PCI
- * core via device_reprobe(), mirroring the PLDR path's contract.
+ * Issue a Function Level Reset and hand teardown/re-init off to the
+ * driver core via device_schedule_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.
@@ -4133,14 +4135,12 @@ static int btintel_pcie_perform_flr(struct btintel_pcie_data *data)
 		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));
+	err = device_schedule_reprobe(&pdev->dev, 0);
+	if (err)
+		BT_ERR("BT reprobe scheduling failed for BDF:%s",
+		       pci_name(pdev));
 
-	return 0;
+	return err;
 }
 
 static void btintel_pcie_reset_work(struct work_struct *wk)
@@ -4173,11 +4173,15 @@ static void btintel_pcie_reset_work(struct work_struct *wk)
 	disable_work_sync(&data->mbox_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 dump workers 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
+	/* Both reset paths follow the same contract: on success the
+	 * deferred re-probe scheduled with device_schedule_reprobe()
+	 * destroys 'data' by re-running .probe() (which re-INIT_WORKs the
+	 * dump workers with disable count 0), so enable_work() must NOT be
+	 * called on the success path. 'data' stays alive until the deferred
+	 * detach runs; in this window new activity is fenced by
+	 * BTINTEL_PCIE_RECOVERY_IN_PROGRESS, the masked interrupts and the
+	 * disabled dump workers. Only the FLR path can fail with no
+	 * re-probe scheduled, in which case we balance the
 	 * disable_work_sync() calls above so a later successful reset is
 	 * not permanently blocked.
 	 *
@@ -4598,13 +4602,8 @@ static void btintel_pcie_remove(struct pci_dev *pdev)
 	disable_work_sync(&data->fwtrigger_work);
 	disable_work_sync(&data->mbox_work);
 
-	/* Cancel pending reset work. Skip only when remove() is called from
-	 * within the reset work itself (PLDR device_reprobe path) to avoid
-	 * deadlock. current_work() returns the work_struct of the caller if
-	 * we are in a workqueue context.
-	 */
-	if (current_work() != &data->reset_work)
-		disable_work_sync(&data->reset_work);
+	/* reset_work may still be running after scheduling the re-probe. */
+	disable_work_sync(&data->reset_work);
 
 	btintel_pcie_mask_hw_rfkill(data);
 	btintel_pcie_synchronize_irqs(data);
-- 
2.56.0