Re: [PATCH v1] Bluetooth: btintel_pcie: Reset controller before configuring MSI-X

[Paul Menzel <pmenzel@molgen.mpg.de>]

Dear Kiran,


Thank you for your patch.

Am 11.05.26 um 15:27 schrieb Kiran K:
> From: Sai Teja Aluvala <aluvala.sai.teja@intel.com>
> 
> Perform the shared hardware reset in btintel_pcie_probe() before
> configuring MSI-X so the controller starts from a known clean state.

Did you run into problems with the current ordering?

> While here, move btintel_pcie_config_msix() out of
> btintel_pcie_config_pcie() and into the probe sequence, and propagate
> errors from btintel_pcie_reset_bt() so probe fails cleanly if the
> shared HW reset does not complete.

“While here” also sounds to me to split the commit into two. But your call.

Please add a paragraph how to test it. (Maybe what log messages to look 
out for.) Is there a way to force `btintel_pcie_reset_bt()` to fail?

> Signed-off-by: Sai Teja Aluvala <aluvala.sai.teja@intel.com>
> Signed-off-by: Kiran K <kiran.k@intel.com>
> ---
>   drivers/bluetooth/btintel_pcie.c | 11 ++++++++---
>   1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
> index fda474406003..6ff08de9ec78 100644
> --- a/drivers/bluetooth/btintel_pcie.c
> +++ b/drivers/bluetooth/btintel_pcie.c
> @@ -1646,9 +1646,6 @@ static int btintel_pcie_config_pcie(struct pci_dev *pdev,
>   	if (err)
>   		return err;
>   
> -	/* Configure MSI-X with causes list */
> -	btintel_pcie_config_msix(data);
> -
>   	return 0;
>   }
>   
> @@ -2659,6 +2656,14 @@ static int btintel_pcie_probe(struct pci_dev *pdev,
>   	if (err)
>   		goto exit_error;
>   
> +	err = btintel_pcie_reset_bt(data);
> +	if (err) {
> +		dev_err(&pdev->dev, "Bluetooth shared HW reset failed (%d)\n", err);

What is “shared HW reset”?

> +		goto exit_error;
> +	}
> +
> +	/* Configure MSI-X with causes list */
> +	btintel_pcie_config_msix(data);
>   	pci_set_drvdata(pdev, data);
>   
>   	err = btintel_pcie_alloc(data);


Kind regards,

Paul