Re: [PATCH v1] Bluetooth: btintel_pcie: Retry FLR on device lock contention

[Luiz Augusto von Dentz]

Hi Ravindra,

On Mon, Oct 12, 2026 at 5:17 AM Ravindra <ravindra@xxxxxxxxx> wrote:
>
> Several paths request an FLR asynchronously, including S3/S4 resume,
> resume recovery, HCI reset, and hardware-error recovery. When a request
> comes from a PM callback, reset work may run before the PM core releases
> the PCI device lock. pci_try_reset_function() then returns -EAGAIN
> without attempting the FLR. Other FLR requests can encounter the same
> error if another operation holds the device lock.
>
> Retry the FLR on -EAGAIN. If all attempts fail, clear the recovery
> in-progress flag so a later reset request can try again.
>
> Fixes: 9c36951474d8 ("Bluetooth: btintel_pcie: Refactor FLR to use device_reprobe()")
> Signed-off-by: Ravindra <ravindra@xxxxxxxxx>
> ---
>  drivers/bluetooth/btintel_pcie.c | 21 +++++++++++++++------
>  1 file changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
> index 3a555aa50c87..251dd4e2ceb4 100644
> --- a/drivers/bluetooth/btintel_pcie.c
> +++ b/drivers/bluetooth/btintel_pcie.c
> @@ -3743,7 +3743,7 @@ static void btintel_pcie_perform_pldr(struct btintel_pcie_data *data)
>  static int btintel_pcie_perform_flr(struct btintel_pcie_data *data)
>  {
>         struct pci_dev *pdev = data->pdev;
> -       int err;
> +       int err, retry;
>
>         /* pci_try_reset_function() avoids the device_lock ABBA against
>          * btintel_pcie_remove(): .remove() runs with device_lock held and
> @@ -3751,7 +3751,16 @@ static int btintel_pcie_perform_flr(struct btintel_pcie_data *data)
>          * pci_reset_function() would deadlock by trying to re-acquire
>          * device_lock here.
>          */
> -       err = pci_try_reset_function(pdev);
> +       /* PM callbacks hold the device lock, so the queued resume reset may
> +        * run before the callback releases it. Retry after that lock is dropped.
> +        */
> +       for (retry = 0; retry < 3; retry++) {
> +               err = pci_try_reset_function(pdev);
> +               if (err != -EAGAIN)
> +                       break;
> +
> +               usleep_range(1000, 2000);
> +       }

We have to make our mind here, either we use pci_try_reset_function
and gracefully handle -EAGAIN, or we don't then we must use
pci_reset_function which awaits for the lock, retying a couple of
times with sleep won't solve for it for good since we don't know how
long it will take for pci_dev_trylock to be available.

>         if (err) {
>                 BT_ERR("Failed resetting the pcie device (%d)", err);
>                 return err;
> @@ -3817,6 +3826,7 @@ static void btintel_pcie_reset_work(struct work_struct *wk)
>                 enable_work(&data->hwexp_work);
>                 enable_work(&data->fwtrigger_work);
>                 enable_work(&data->mbox_work);
> +               clear_bit(BTINTEL_PCIE_RECOVERY_IN_PROGRESS, &data->flags);
>         }
>
>  out:
> @@ -3841,10 +3851,9 @@ static void btintel_pcie_reset_work(struct work_struct *wk)
>   *     in-flight one (whose recovery will reinitialize the device
>   *     regardless of the dropped trigger's variant).
>   *
> - * The bit is cleared only by .remove() / re-probe via fresh devm
> - * allocation, which is the intended one-shot semantics: a reset
> - * tears down and re-probes 'data', so there is no "in-flight"
> - * reset to follow up after device_reprobe() succeeds.
> + * A successful reset clears the bit through .remove() / re-probe via
> + * fresh devm allocation. A failed FLR clears it in reset_work so a
> + * later request can retry; there is no in-flight reset after failure.
>   */
>  static void btintel_pcie_request_reset(struct btintel_pcie_data *data,
>                                        enum btintel_pcie_reset_type type)
> --
> 2.43.0
>


-- 
Luiz Augusto von Dentz