[PATCH v1] Bluetooth: btintel_pcie: Fix event packet loss issue

[PATCH v1] Bluetooth: btintel_pcie: Fix event packet loss issue

[Kiran K]

In the current btintel_pcie driver implementation, when an interrupt is
received, the driver checks for the alive cause before the TX/RX cause.
Handling the alive cause involves resetting the TX/RX queue indices.
This flow works correctly when the causes are mutually exclusive.
However, if both cause bits are set simultaneously, the alive cause
resets the queue indices, resulting in an event packet drop and a
command timeout. To fix this issue, the driver is modified to handle all
other causes before checking for the alive cause.

Signed-off-by: Kiran K <kiran.k@intel.com>
Signed-off-by: Sai Teja Aluvala <aluvala.sai.teja@intel.com>
Fixes: c2b636b3f788 ("Bluetooth: btintel_pcie: Add support for PCIe transport")
---
 drivers/bluetooth/btintel_pcie.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
index 6d3963bd56a9..a075d8ec4677 100644
--- a/drivers/bluetooth/btintel_pcie.c
+++ b/drivers/bluetooth/btintel_pcie.c
@@ -1467,11 +1467,6 @@ static irqreturn_t btintel_pcie_irq_msix_handler(int irq, void *dev_id)
 	if (intr_hw & BTINTEL_PCIE_MSIX_HW_INT_CAUSES_GP1)
 		btintel_pcie_msix_gp1_handler(data);
 
-	/* This interrupt is triggered by the firmware after updating
-	 * boot_stage register and image_response register
-	 */
-	if (intr_hw & BTINTEL_PCIE_MSIX_HW_INT_CAUSES_GP0)
-		btintel_pcie_msix_gp0_handler(data);
 
 	/* For TX */
 	if (intr_fh & BTINTEL_PCIE_MSIX_FH_INT_CAUSES_0) {
@@ -1487,6 +1482,12 @@ static irqreturn_t btintel_pcie_irq_msix_handler(int irq, void *dev_id)
 			btintel_pcie_msix_tx_handle(data);
 	}
 
+	/* This interrupt is triggered by the firmware after updating
+	 * boot_stage register and image_response register
+	 */
+	if (intr_hw & BTINTEL_PCIE_MSIX_HW_INT_CAUSES_GP0)
+		btintel_pcie_msix_gp0_handler(data);
+
 	/*
 	 * Before sending the interrupt the HW disables it to prevent a nested
 	 * interrupt. This is done by writing 1 to the corresponding bit in
-- 
2.43.0

RE: [v1] Bluetooth: btintel_pcie: Fix event packet loss issue

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 2885 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=1011786

---Test result---

Test Summary:
CheckPatch                    PENDING   0.36 seconds
GitLint                       PENDING   0.25 seconds
SubjectPrefix                 PASS      0.08 seconds
BuildKernel                   PASS      24.89 seconds
CheckAllWarning               PASS      27.43 seconds
CheckSparse                   PASS      31.17 seconds
BuildKernel32                 PASS      25.11 seconds
TestRunnerSetup               PASS      497.61 seconds
TestRunner_l2cap-tester       PASS      24.07 seconds
TestRunner_iso-tester         PASS      84.73 seconds
TestRunner_bnep-tester        PASS      6.21 seconds
TestRunner_mgmt-tester        FAIL      123.63 seconds
TestRunner_rfcomm-tester      PASS      9.38 seconds
TestRunner_sco-tester         PASS      12.77 seconds
TestRunner_ioctl-tester       FAIL      10.36 seconds
TestRunner_mesh-tester        FAIL      11.38 seconds
TestRunner_smp-tester         PASS      8.57 seconds
TestRunner_userchan-tester    PASS      6.47 seconds
IncrementalBuild              PENDING   0.82 seconds

Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:

##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:

##############################
Test: TestRunner_mgmt-tester - FAIL
Desc: Run mgmt-tester with test-runner
Output:
Total: 490, Passed: 482 (98.4%), Failed: 4, Not Run: 4

Failed Test Cases
Pair Device - Legacy Success 1                       Failed       0.195 seconds
Pair Device - Sec Mode 3 Success 1                   Failed       0.174 seconds
Pair Device - Legacy Reject 2                        Failed       0.163 seconds
Read Exp Feature - Success                           Failed       0.110 seconds
##############################
Test: TestRunner_ioctl-tester - FAIL
Desc: Run ioctl-tester with test-runner
Output:
Total: 28, Passed: 26 (92.9%), Failed: 2, Not Run: 0

Failed Test Cases
Connection List                                      Failed       1.002 seconds
Connection Info                                      Failed       0.140 seconds
##############################
Test: TestRunner_mesh-tester - FAIL
Desc: Run mesh-tester with test-runner
Output:
Total: 10, Passed: 8 (80.0%), Failed: 2, Not Run: 0

Failed Test Cases
Mesh - Send cancel - 1                               Timed out    1.850 seconds
Mesh - Send cancel - 2                               Timed out    1.997 seconds
##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:



---
Regards,
Linux Bluetooth

Re: [PATCH v1] Bluetooth: btintel_pcie: Fix event packet loss issue

[Paul Menzel]

Dear Kiran,


Thank you for the patch.

Am 15.10.25 um 12:14 schrieb Kiran K:
> In the current btintel_pcie driver implementation, when an interrupt is
> received, the driver checks for the alive cause before the TX/RX cause.
> Handling the alive cause involves resetting the TX/RX queue indices.
> This flow works correctly when the causes are mutually exclusive.
> However, if both cause bits are set simultaneously, the alive cause
> resets the queue indices, resulting in an event packet drop and a
> command timeout. To fix this issue, the driver is modified to handle all
> other causes before checking for the alive cause.

Please share a test case.

> Signed-off-by: Kiran K <kiran.k@intel.com>
> Signed-off-by: Sai Teja Aluvala <aluvala.sai.teja@intel.com>
> Fixes: c2b636b3f788 ("Bluetooth: btintel_pcie: Add support for PCIe transport")
> ---
>   drivers/bluetooth/btintel_pcie.c | 11 ++++++-----
>   1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
> index 6d3963bd56a9..a075d8ec4677 100644
> --- a/drivers/bluetooth/btintel_pcie.c
> +++ b/drivers/bluetooth/btintel_pcie.c
> @@ -1467,11 +1467,6 @@ static irqreturn_t btintel_pcie_irq_msix_handler(int irq, void *dev_id)
>   	if (intr_hw & BTINTEL_PCIE_MSIX_HW_INT_CAUSES_GP1)
>   		btintel_pcie_msix_gp1_handler(data);
>   
> -	/* This interrupt is triggered by the firmware after updating
> -	 * boot_stage register and image_response register
> -	 */
> -	if (intr_hw & BTINTEL_PCIE_MSIX_HW_INT_CAUSES_GP0)
> -		btintel_pcie_msix_gp0_handler(data);
>   
>   	/* For TX */
>   	if (intr_fh & BTINTEL_PCIE_MSIX_FH_INT_CAUSES_0) {
> @@ -1487,6 +1482,12 @@ static irqreturn_t btintel_pcie_irq_msix_handler(int irq, void *dev_id)
>   			btintel_pcie_msix_tx_handle(data);
>   	}
>   
> +	/* This interrupt is triggered by the firmware after updating
> +	 * boot_stage register and image_response register
> +	 */
> +	if (intr_hw & BTINTEL_PCIE_MSIX_HW_INT_CAUSES_GP0)
> +		btintel_pcie_msix_gp0_handler(data);
> +
>   	/*
>   	 * Before sending the interrupt the HW disables it to prevent a nested
>   	 * interrupt. This is done by writing 1 to the corresponding bit in

The diff looks good.

Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>


Kind regards,

Paul

RE: [PATCH v1] Bluetooth: btintel_pcie: Fix event packet loss issue

[K, Kiran]

Hi Paul,

Thanks for the comments.

>Subject: Re: [PATCH v1] Bluetooth: btintel_pcie: Fix event packet loss issue
>
>Dear Kiran,
>
>
>Thank you for the patch.
>
>Am 15.10.25 um 12:14 schrieb Kiran K:
>> In the current btintel_pcie driver implementation, when an interrupt
>> is received, the driver checks for the alive cause before the TX/RX cause.
>> Handling the alive cause involves resetting the TX/RX queue indices.
>> This flow works correctly when the causes are mutually exclusive.
>> However, if both cause bits are set simultaneously, the alive cause
>> resets the queue indices, resulting in an event packet drop and a
>> command timeout. To fix this issue, the driver is modified to handle
>> all other causes before checking for the alive cause.
>
>Please share a test case.
I have updated the commit message with test case details and published v2 version.
>
>> Signed-off-by: Kiran K <kiran.k@intel.com>
>> Signed-off-by: Sai Teja Aluvala <aluvala.sai.teja@intel.com>
>> Fixes: c2b636b3f788 ("Bluetooth: btintel_pcie: Add support for PCIe
>> transport")
>> ---
>>   drivers/bluetooth/btintel_pcie.c | 11 ++++++-----
>>   1 file changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/bluetooth/btintel_pcie.c
>> b/drivers/bluetooth/btintel_pcie.c
>> index 6d3963bd56a9..a075d8ec4677 100644
>> --- a/drivers/bluetooth/btintel_pcie.c
>> +++ b/drivers/bluetooth/btintel_pcie.c
>> @@ -1467,11 +1467,6 @@ static irqreturn_t
>btintel_pcie_irq_msix_handler(int irq, void *dev_id)
>>   	if (intr_hw & BTINTEL_PCIE_MSIX_HW_INT_CAUSES_GP1)
>>   		btintel_pcie_msix_gp1_handler(data);
>>
>> -	/* This interrupt is triggered by the firmware after updating
>> -	 * boot_stage register and image_response register
>> -	 */
>> -	if (intr_hw & BTINTEL_PCIE_MSIX_HW_INT_CAUSES_GP0)
>> -		btintel_pcie_msix_gp0_handler(data);
>>
>>   	/* For TX */
>>   	if (intr_fh & BTINTEL_PCIE_MSIX_FH_INT_CAUSES_0) { @@ -1487,6
>> +1482,12 @@ static irqreturn_t btintel_pcie_irq_msix_handler(int irq, void
>*dev_id)
>>   			btintel_pcie_msix_tx_handle(data);
>>   	}
>>
>> +	/* This interrupt is triggered by the firmware after updating
>> +	 * boot_stage register and image_response register
>> +	 */
>> +	if (intr_hw & BTINTEL_PCIE_MSIX_HW_INT_CAUSES_GP0)
>> +		btintel_pcie_msix_gp0_handler(data);
>> +
>>   	/*
>>   	 * Before sending the interrupt the HW disables it to prevent a nested
>>   	 * interrupt. This is done by writing 1 to the corresponding bit in
>
>The diff looks good.
>
>Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
Ack

>
>
>Kind regards,
>
>Paul

Regards,
Kiran