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

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] BTINTEL_PCIE_RECOVERY_IN_PROGRESS flag is never cleared on reset failure, causing a permanent deadlock of the recovery mechanism.
--

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

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

This patch adds a reset_type parameter to the Bluetooth hci_dev reset
callback, allowing userspace to specify whether to perform a Function Level
Reset (FLR) or a Product Level Device Reset (PLDR) via sysfs. Internal
callers are updated to default to FLR.

> 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
[ ... ]
> @@ -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;

[Severity: High]
This is a pre-existing issue, but if btintel_pcie_reset_work() fails, does
the BTINTEL_PCIE_RECOVERY_IN_PROGRESS flag remain set permanently?

Looking at btintel_pcie_reset_work() in the same file, if
pci_try_reset_function() or btintel_pcie_setup_hdev() fail, the code jumps
to the out_enable label:

btintel_pcie_reset_work() {
        err = pci_try_reset_function(pdev);
        if (err) {
                BT_ERR("Failed resetting the pcie device (%d)", err);
                goto out_enable;
        }
        ...
        data->flags = 0;
        ...
out_enable:
        enable_work(&data->coredump_work);
out:
        ...
}

Because the error paths bypass the data->flags = 0 assignment, the
BTINTEL_PCIE_RECOVERY_IN_PROGRESS flag is never cleared. Would this cause
all future reset attempts (including the sysfs triggers introduced in this
patch) to be rejected indefinitely by the test_and_set_bit() check until
the module is reloaded?

> +	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);
>  }

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