Re: [PATCH v1 1/3] Bluetooth: btintel_pcie: Fix driver not posting maximum rx buffers

Re: [PATCH v1 1/3] Bluetooth: btintel_pcie: Fix driver not posting maximum rx buffers

[Paul Menzel]

Dear Kiran,


Thank you for the patch.

Am 25.05.25 um 07:39 schrieb Kiran K:
> The driver was posting only 6 rx buffers, despite the maximum rx buffers
> being defined as 16.

The second part is worded strangely, as the macro, you remove, is 
defined to be 6. Was it just a typo?

> Update the driver to post the full 16 Rx buffers to
> utilize the maximum capacity and improve performance.

Please also note, that you remove the macro, because the value seems to 
be detectable.

Please also mention, how your change can be verified, for example read 
out in the logs or from /sys, and how the performance can be measured, 
and what you measured before and after. (Claimns regarding performance 
should always come with measurements.)

> Signed-off-by: Chandrashekar Devegowda <chandrashekar.devegowda@intel.com>
> Signed-off-by: Kiran K <kiran.k@intel.com>
> Fixes: c2b636b3f788 ("Bluetooth: btintel_pcie: Add support for PCIe transport")
> ---
>   drivers/bluetooth/btintel_pcie.c | 3 ++-
>   drivers/bluetooth/btintel_pcie.h | 3 ---
>   2 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
> index 50fe17f1e1d1..2c7731803c9f 100644
> --- a/drivers/bluetooth/btintel_pcie.c
> +++ b/drivers/bluetooth/btintel_pcie.c
> @@ -396,8 +396,9 @@ static int btintel_pcie_submit_rx(struct btintel_pcie_data *data)
>   static int btintel_pcie_start_rx(struct btintel_pcie_data *data)
>   {
>   	int i, ret;
> +	struct rxq *rxq = &data->rxq;
>   
> -	for (i = 0; i < BTINTEL_PCIE_RX_MAX_QUEUE; i++) {
> +	for (i = 0; i < rxq->count; i++) {
>   		ret = btintel_pcie_submit_rx(data);
>   		if (ret)
>   			return ret;
> diff --git a/drivers/bluetooth/btintel_pcie.h b/drivers/bluetooth/btintel_pcie.h
> index 21b964b15c1c..5ddd6d7d8d45 100644
> --- a/drivers/bluetooth/btintel_pcie.h
> +++ b/drivers/bluetooth/btintel_pcie.h
> @@ -177,9 +177,6 @@ enum {
>   /* Doorbell vector for TFD */
>   #define BTINTEL_PCIE_TX_DB_VEC	0
>   
> -/* Number of pending RX requests for downlink */
> -#define BTINTEL_PCIE_RX_MAX_QUEUE	6
> -
>   /* Doorbell vector for FRBD */
>   #define BTINTEL_PCIE_RX_DB_VEC	513


Kind regards,

Paul

[PATCH v1 1/3] Bluetooth: btintel_pcie: Fix driver not posting maximum rx buffers

[Kiran K]

The driver was posting only 6 rx buffers, despite the maximum rx buffers
being defined as 16. Update the driver to post the full 16 Rx buffers to
utilize the maximum capacity and improve performance.

Signed-off-by: Chandrashekar Devegowda <chandrashekar.devegowda@intel.com>
Signed-off-by: Kiran K <kiran.k@intel.com>
Fixes: c2b636b3f788 ("Bluetooth: btintel_pcie: Add support for PCIe transport")
---
 drivers/bluetooth/btintel_pcie.c | 3 ++-
 drivers/bluetooth/btintel_pcie.h | 3 ---
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
index 50fe17f1e1d1..2c7731803c9f 100644
--- a/drivers/bluetooth/btintel_pcie.c
+++ b/drivers/bluetooth/btintel_pcie.c
@@ -396,8 +396,9 @@ static int btintel_pcie_submit_rx(struct btintel_pcie_data *data)
 static int btintel_pcie_start_rx(struct btintel_pcie_data *data)
 {
 	int i, ret;
+	struct rxq *rxq = &data->rxq;
 
-	for (i = 0; i < BTINTEL_PCIE_RX_MAX_QUEUE; i++) {
+	for (i = 0; i < rxq->count; i++) {
 		ret = btintel_pcie_submit_rx(data);
 		if (ret)
 			return ret;
diff --git a/drivers/bluetooth/btintel_pcie.h b/drivers/bluetooth/btintel_pcie.h
index 21b964b15c1c..5ddd6d7d8d45 100644
--- a/drivers/bluetooth/btintel_pcie.h
+++ b/drivers/bluetooth/btintel_pcie.h
@@ -177,9 +177,6 @@ enum {
 /* Doorbell vector for TFD */
 #define BTINTEL_PCIE_TX_DB_VEC	0
 
-/* Number of pending RX requests for downlink */
-#define BTINTEL_PCIE_RX_MAX_QUEUE	6
-
 /* Doorbell vector for FRBD */
 #define BTINTEL_PCIE_RX_DB_VEC	513
 
-- 
2.43.0

[PATCH v1 2/3] Bluetooth: btintel_pcie: Increase the tx and rx descriptor count

[Kiran K]

From: Chandrashekar Devegowda <chandrashekar.devegowda@intel.com>

This change addresses latency issues observed in HID use cases where
events arrive in bursts. By increasing the Rx descriptor count to 64,
the firmware can handle bursty data more effectively, reducing latency
and preventing buffer overflows.

Signed-off-by: Chandrashekar Devegowda <chandrashekar.devegowda@intel.com>
Signed-off-by: Kiran K <kiran.k@intel.com>
---
 drivers/bluetooth/btintel_pcie.c | 24 ++++++++++++------------
 drivers/bluetooth/btintel_pcie.h |  7 +++++--
 2 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
index 2c7731803c9f..03f13de4a723 100644
--- a/drivers/bluetooth/btintel_pcie.c
+++ b/drivers/bluetooth/btintel_pcie.c
@@ -1783,8 +1783,8 @@ static int btintel_pcie_alloc(struct btintel_pcie_data *data)
 	 *  + size of index * Number of queues(2) * type of index array(4)
 	 *  + size of context information
 	 */
-	total = (sizeof(struct tfd) + sizeof(struct urbd0) + sizeof(struct frbd)
-		+ sizeof(struct urbd1)) * BTINTEL_DESCS_COUNT;
+	total = (sizeof(struct tfd) + sizeof(struct urbd0)) * BTINTEL_PCIE_TX_DESCS_COUNT;
+	total += (sizeof(struct frbd) + sizeof(struct urbd1)) * BTINTEL_PCIE_RX_DESCS_COUNT;
 
 	/* Add the sum of size of index array and size of ci struct */
 	total += (sizeof(u16) * BTINTEL_PCIE_NUM_QUEUES * 4) + sizeof(struct ctx_info);
@@ -1809,36 +1809,36 @@ static int btintel_pcie_alloc(struct btintel_pcie_data *data)
 	data->dma_v_addr = v_addr;
 
 	/* Setup descriptor count */
-	data->txq.count = BTINTEL_DESCS_COUNT;
-	data->rxq.count = BTINTEL_DESCS_COUNT;
+	data->txq.count = BTINTEL_PCIE_TX_DESCS_COUNT;
+	data->rxq.count = BTINTEL_PCIE_RX_DESCS_COUNT;
 
 	/* Setup tfds */
 	data->txq.tfds_p_addr = p_addr;
 	data->txq.tfds = v_addr;
 
-	p_addr += (sizeof(struct tfd) * BTINTEL_DESCS_COUNT);
-	v_addr += (sizeof(struct tfd) * BTINTEL_DESCS_COUNT);
+	p_addr += (sizeof(struct tfd) * BTINTEL_PCIE_TX_DESCS_COUNT);
+	v_addr += (sizeof(struct tfd) * BTINTEL_PCIE_TX_DESCS_COUNT);
 
 	/* Setup urbd0 */
 	data->txq.urbd0s_p_addr = p_addr;
 	data->txq.urbd0s = v_addr;
 
-	p_addr += (sizeof(struct urbd0) * BTINTEL_DESCS_COUNT);
-	v_addr += (sizeof(struct urbd0) * BTINTEL_DESCS_COUNT);
+	p_addr += (sizeof(struct urbd0) * BTINTEL_PCIE_TX_DESCS_COUNT);
+	v_addr += (sizeof(struct urbd0) * BTINTEL_PCIE_TX_DESCS_COUNT);
 
 	/* Setup FRBD*/
 	data->rxq.frbds_p_addr = p_addr;
 	data->rxq.frbds = v_addr;
 
-	p_addr += (sizeof(struct frbd) * BTINTEL_DESCS_COUNT);
-	v_addr += (sizeof(struct frbd) * BTINTEL_DESCS_COUNT);
+	p_addr += (sizeof(struct frbd) * BTINTEL_PCIE_RX_DESCS_COUNT);
+	v_addr += (sizeof(struct frbd) * BTINTEL_PCIE_RX_DESCS_COUNT);
 
 	/* Setup urbd1 */
 	data->rxq.urbd1s_p_addr = p_addr;
 	data->rxq.urbd1s = v_addr;
 
-	p_addr += (sizeof(struct urbd1) * BTINTEL_DESCS_COUNT);
-	v_addr += (sizeof(struct urbd1) * BTINTEL_DESCS_COUNT);
+	p_addr += (sizeof(struct urbd1) * BTINTEL_PCIE_RX_DESCS_COUNT);
+	v_addr += (sizeof(struct urbd1) * BTINTEL_PCIE_RX_DESCS_COUNT);
 
 	/* Setup data buffers for txq */
 	err = btintel_pcie_setup_txq_bufs(data, &data->txq);
diff --git a/drivers/bluetooth/btintel_pcie.h b/drivers/bluetooth/btintel_pcie.h
index 5ddd6d7d8d45..7dad4523236c 100644
--- a/drivers/bluetooth/btintel_pcie.h
+++ b/drivers/bluetooth/btintel_pcie.h
@@ -154,8 +154,11 @@ enum msix_mbox_int_causes {
 /* Default interrupt timeout in msec */
 #define BTINTEL_DEFAULT_INTR_TIMEOUT_MS	3000
 
-/* The number of descriptors in TX/RX queues */
-#define BTINTEL_DESCS_COUNT	16
+/* The number of descriptors in TX queues */
+#define BTINTEL_PCIE_TX_DESCS_COUNT	32
+
+/* The number of descriptors in RX queues */
+#define BTINTEL_PCIE_RX_DESCS_COUNT	64
 
 /* Number of Queue for TX and RX
  * It indicates the index of the IA(Index Array)
-- 
2.43.0

[PATCH v1 3/3] Bluetooth: btintel_pcie: Reduce driver buffer posting to prevent race condition

[Kiran K]

From: Chandrashekar Devegowda <chandrashekar.devegowda@intel.com>

Modify the driver to post 3 fewer buffers than the maximum rx buffers
(64) allowed for the firmware. This change mitigates a hardware issue
causing a race condition in the firmware, improving stability and data
handling.

Signed-off-by: Chandrashekar Devegowda <chandrashekar.devegowda@intel.com>
Signed-off-by: Kiran K <kiran.k@intel.com>
---
 drivers/bluetooth/btintel_pcie.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
index 03f13de4a723..14f000e08e1e 100644
--- a/drivers/bluetooth/btintel_pcie.c
+++ b/drivers/bluetooth/btintel_pcie.c
@@ -398,7 +398,10 @@ static int btintel_pcie_start_rx(struct btintel_pcie_data *data)
 	int i, ret;
 	struct rxq *rxq = &data->rxq;
 
-	for (i = 0; i < rxq->count; i++) {
+	/* Post (BTINTEL_PCIE_RX_DESCS_COUNT - 3) buffers to overcome the
+	 * hardware issues leading to race condition at the firmware.
+	 */
+	for (i = 0; i < rxq->count - 3; i++) {
 		ret = btintel_pcie_submit_rx(data);
 		if (ret)
 			return ret;
-- 
2.43.0

Re: [PATCH v1 3/3] Bluetooth: btintel_pcie: Reduce driver buffer posting to prevent race condition

[Paul Menzel]

Dear Chandrashekar, dear Kiran,


Thank you for the patch.

Am 25.05.25 um 07:39 schrieb Kiran K:
> From: Chandrashekar Devegowda <chandrashekar.devegowda@intel.com>
> 
> Modify the driver to post 3 fewer buffers than the maximum rx buffers
> (64) allowed for the firmware. This change mitigates a hardware issue
> causing a race condition in the firmware, improving stability and data
> handling.

Interesting. Please elaborate, which firmware versions are affected, and 
if a fix is going to be expected, and how to reproduce the issue, so it 
can be tested without and with your patch.

Is the errata published? Why three buffers less and not two or four?

Out of curiosity: Does the Microsoft Windows driver do the same?

> Signed-off-by: Chandrashekar Devegowda <chandrashekar.devegowda@intel.com>
> Signed-off-by: Kiran K <kiran.k@intel.com>
> ---
>   drivers/bluetooth/btintel_pcie.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
> index 03f13de4a723..14f000e08e1e 100644
> --- a/drivers/bluetooth/btintel_pcie.c
> +++ b/drivers/bluetooth/btintel_pcie.c
> @@ -398,7 +398,10 @@ static int btintel_pcie_start_rx(struct btintel_pcie_data *data)
>   	int i, ret;
>   	struct rxq *rxq = &data->rxq;
>   
> -	for (i = 0; i < rxq->count; i++) {
> +	/* Post (BTINTEL_PCIE_RX_DESCS_COUNT - 3) buffers to overcome the
> +	 * hardware issues leading to race condition at the firmware.

If you had an errata, it’d be great to document it here to.

I’d remove *the*.

> +	 */
> +	for (i = 0; i < rxq->count - 3; i++) {
>   		ret = btintel_pcie_submit_rx(data);
>   		if (ret)
>   			return ret;


Kind regards,

Paul

RE: [v1,1/3] Bluetooth: btintel_pcie: Fix driver not posting maximum rx buffers

[bluez.test.bot]

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

---Test result---

Test Summary:
CheckPatch                    PENDING   0.33 seconds
GitLint                       PENDING   0.21 seconds
SubjectPrefix                 PASS      0.34 seconds
BuildKernel                   PASS      24.56 seconds
CheckAllWarning               PASS      26.73 seconds
CheckSparse                   PASS      30.13 seconds
BuildKernel32                 PASS      23.98 seconds
TestRunnerSetup               PASS      452.70 seconds
TestRunner_l2cap-tester       PASS      25.09 seconds
TestRunner_iso-tester         PASS      37.26 seconds
TestRunner_bnep-tester        PASS      5.86 seconds
TestRunner_mgmt-tester        PASS      131.88 seconds
TestRunner_rfcomm-tester      PASS      9.27 seconds
TestRunner_sco-tester         PASS      14.55 seconds
TestRunner_ioctl-tester       PASS      9.89 seconds
TestRunner_mesh-tester        PASS      7.30 seconds
TestRunner_smp-tester         PASS      8.71 seconds
TestRunner_userchan-tester    PASS      6.09 seconds
IncrementalBuild              PENDING   0.94 seconds

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

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

##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:



---
Regards,
Linux Bluetooth

RE: [PATCH v1 1/3] Bluetooth: btintel_pcie: Fix driver not posting maximum rx buffers

[K, Kiran]

Hi Paul,

Thanks for the comments.

>Subject: Re: [PATCH v1 1/3] Bluetooth: btintel_pcie: Fix driver not posting
>maximum rx buffers
>
>Dear Kiran,
>
>
>Thank you for the patch.
>
>Am 25.05.25 um 07:39 schrieb Kiran K:
>> The driver was posting only 6 rx buffers, despite the maximum rx
>> buffers being defined as 16.
>
>The second part is worded strangely, as the macro, you remove, is defined to
>be 6. Was it just a typo?
>
The macro value was defined incorrectly to 6, instead of 16.

>> Update the driver to post the full 16 Rx buffers to utilize the
>> maximum capacity and improve performance.
>
>Please also note, that you remove the macro, because the value seems to be
>detectable.
>
Instead of using macro, rxq->count is used which is initialized to 16  in the buffer allocation function btintel_pcie_alloc().

>Please also mention, how your change can be verified, for example read out in

The issue was reported internally over Android. On connecting bluetooth mouse, there was an exception reported from firmware in less than 5 seconds as there are not enough buffer for the firmware to send the hid reports.  Along with this change, we would also need 2/3 and 3/3 patches to fully fix the exception issue.

>the logs or from /sys, and how the performance can be measured, and what

When the firmware exception occurs, driver prints the exception in the kernel message.

>you measured before and after. (Claimns regarding performance should always
>come with measurements.)

This issue is more to do with stability rather than performance. I will update the commit message in the v2 version of the patch.

>
>> Signed-off-by: Chandrashekar Devegowda
>> <chandrashekar.devegowda@intel.com>
>> Signed-off-by: Kiran K <kiran.k@intel.com>
>> Fixes: c2b636b3f788 ("Bluetooth: btintel_pcie: Add support for PCIe
>> transport")
>> ---
>>   drivers/bluetooth/btintel_pcie.c | 3 ++-
>>   drivers/bluetooth/btintel_pcie.h | 3 ---
>>   2 files changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/bluetooth/btintel_pcie.c
>> b/drivers/bluetooth/btintel_pcie.c
>> index 50fe17f1e1d1..2c7731803c9f 100644
>> --- a/drivers/bluetooth/btintel_pcie.c
>> +++ b/drivers/bluetooth/btintel_pcie.c
>> @@ -396,8 +396,9 @@ static int btintel_pcie_submit_rx(struct
>btintel_pcie_data *data)
>>   static int btintel_pcie_start_rx(struct btintel_pcie_data *data)
>>   {
>>   	int i, ret;
>> +	struct rxq *rxq = &data->rxq;
>>
>> -	for (i = 0; i < BTINTEL_PCIE_RX_MAX_QUEUE; i++) {
>> +	for (i = 0; i < rxq->count; i++) {
>>   		ret = btintel_pcie_submit_rx(data);
>>   		if (ret)
>>   			return ret;
>> diff --git a/drivers/bluetooth/btintel_pcie.h
>> b/drivers/bluetooth/btintel_pcie.h
>> index 21b964b15c1c..5ddd6d7d8d45 100644
>> --- a/drivers/bluetooth/btintel_pcie.h
>> +++ b/drivers/bluetooth/btintel_pcie.h
>> @@ -177,9 +177,6 @@ enum {
>>   /* Doorbell vector for TFD */
>>   #define BTINTEL_PCIE_TX_DB_VEC	0
>>
>> -/* Number of pending RX requests for downlink */
>> -#define BTINTEL_PCIE_RX_MAX_QUEUE	6
>> -
>>   /* Doorbell vector for FRBD */
>>   #define BTINTEL_PCIE_RX_DB_VEC	513
>
>
>Kind regards,
>
>Paul

Thanks,
Kiran

RE: [PATCH v1 3/3] Bluetooth: btintel_pcie: Reduce driver buffer posting to prevent race condition

[K, Kiran]

Hi Paul,

Thanks for your comments.

>Subject: Re: [PATCH v1 3/3] Bluetooth: btintel_pcie: Reduce driver buffer
>posting to prevent race condition
>
>Dear Chandrashekar, dear Kiran,
>
>
>Thank you for the patch.
>
>Am 25.05.25 um 07:39 schrieb Kiran K:
>> From: Chandrashekar Devegowda <chandrashekar.devegowda@intel.com>
>>
>> Modify the driver to post 3 fewer buffers than the maximum rx buffers
>> (64) allowed for the firmware. This change mitigates a hardware issue
>> causing a race condition in the firmware, improving stability and data
>> handling.
>

>Interesting. Please elaborate, which firmware versions are affected, and if a fix
>is going to be expected, and how to reproduce the issue, so it can be tested
>without and with your patch.
>
The firmware is for the upcoming product and is not available in public yet. As I said in 1/3,  the issue is seen on android kernel and  it's very hard to reproduce the issue on vanilla kernel. 

>Is the errata published? 
>
Our internal documents are updated.  I have also entered a comment in code.

> Why three buffers less and not two or four?
The recommendation from firmware / HW is to use at least 3 buffers as guard buffers. Anything less than three causes RFH (receive flow handler - RX path) blockage. 

>Out of curiosity: Does the Microsoft Windows driver do the same?
Yes.

>
>> Signed-off-by: Chandrashekar Devegowda
>> <chandrashekar.devegowda@intel.com>
>> Signed-off-by: Kiran K <kiran.k@intel.com>
>> ---
>>   drivers/bluetooth/btintel_pcie.c | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/bluetooth/btintel_pcie.c
>> b/drivers/bluetooth/btintel_pcie.c
>> index 03f13de4a723..14f000e08e1e 100644
>> --- a/drivers/bluetooth/btintel_pcie.c
>> +++ b/drivers/bluetooth/btintel_pcie.c
>> @@ -398,7 +398,10 @@ static int btintel_pcie_start_rx(struct
>btintel_pcie_data *data)
>>   	int i, ret;
>>   	struct rxq *rxq = &data->rxq;
>>
>> -	for (i = 0; i < rxq->count; i++) {
>> +	/* Post (BTINTEL_PCIE_RX_DESCS_COUNT - 3) buffers to overcome the
>> +	 * hardware issues leading to race condition at the firmware.
>
>If you had an errata, it’d be great to document it here to.
>
>I’d remove *the*.
Ack
>
>> +	 */
>> +	for (i = 0; i < rxq->count - 3; i++) {
>>   		ret = btintel_pcie_submit_rx(data);
>>   		if (ret)
>>   			return ret;
>
>
>Kind regards,
>
>Paul

Regards,
Kiran

Re: [PATCH v1 3/3] Bluetooth: btintel_pcie: Reduce driver buffer posting to prevent race condition

[Luiz Augusto von Dentz]

Hi Kiran,

On Mon, May 26, 2025 at 11:58 AM K, Kiran <kiran.k@intel.com> wrote:
>
>
> Hi Paul,
>
> Thanks for your comments.
>
> >Subject: Re: [PATCH v1 3/3] Bluetooth: btintel_pcie: Reduce driver buffer
> >posting to prevent race condition
> >
> >Dear Chandrashekar, dear Kiran,
> >
> >
> >Thank you for the patch.
> >
> >Am 25.05.25 um 07:39 schrieb Kiran K:
> >> From: Chandrashekar Devegowda <chandrashekar.devegowda@intel.com>
> >>
> >> Modify the driver to post 3 fewer buffers than the maximum rx buffers
> >> (64) allowed for the firmware. This change mitigates a hardware issue
> >> causing a race condition in the firmware, improving stability and data
> >> handling.
> >
>
> >Interesting. Please elaborate, which firmware versions are affected, and if a fix
> >is going to be expected, and how to reproduce the issue, so it can be tested
> >without and with your patch.
> >
> The firmware is for the upcoming product and is not available in public yet. As I said in 1/3,  the issue is seen on android kernel and  it's very hard to reproduce the issue on vanilla kernel.

If it affects Android specific versions only then it should be posted
for upstream, anyway this sounds like it is more of a workaround then
a proper fix for the problem.

> >Is the errata published?
> >
> Our internal documents are updated.  I have also entered a comment in code.
>
> > Why three buffers less and not two or four?
> The recommendation from firmware / HW is to use at least 3 buffers as guard buffers. Anything less than three causes RFH (receive flow handler - RX path) blockage.

Are these buffers discovered at runtime or they are hardcoded, for the
former then the firmware shall be adjusted to give a proper number and
in case of the later then the driver shall be updated, either way
having to do -3 sounds like a bad idea in the long term.


> >Out of curiosity: Does the Microsoft Windows driver do the same?
> Yes.
>
> >
> >> Signed-off-by: Chandrashekar Devegowda
> >> <chandrashekar.devegowda@intel.com>
> >> Signed-off-by: Kiran K <kiran.k@intel.com>
> >> ---
> >>   drivers/bluetooth/btintel_pcie.c | 5 ++++-
> >>   1 file changed, 4 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/bluetooth/btintel_pcie.c
> >> b/drivers/bluetooth/btintel_pcie.c
> >> index 03f13de4a723..14f000e08e1e 100644
> >> --- a/drivers/bluetooth/btintel_pcie.c
> >> +++ b/drivers/bluetooth/btintel_pcie.c
> >> @@ -398,7 +398,10 @@ static int btintel_pcie_start_rx(struct
> >btintel_pcie_data *data)
> >>      int i, ret;
> >>      struct rxq *rxq = &data->rxq;
> >>
> >> -    for (i = 0; i < rxq->count; i++) {
> >> +    /* Post (BTINTEL_PCIE_RX_DESCS_COUNT - 3) buffers to overcome the
> >> +     * hardware issues leading to race condition at the firmware.
> >
> >If you had an errata, it’d be great to document it here to.
> >
> >I’d remove *the*.
> Ack
> >
> >> +     */
> >> +    for (i = 0; i < rxq->count - 3; i++) {
> >>              ret = btintel_pcie_submit_rx(data);
> >>              if (ret)
> >>                      return ret;
> >
> >
> >Kind regards,
> >
> >Paul
>
> Regards,
> Kiran
>


--
Luiz Augusto von Dentz

RE: [PATCH v1 3/3] Bluetooth: btintel_pcie: Reduce driver buffer posting to prevent race condition

[K, Kiran]

Hi Luiz,

>Subject: Re: [PATCH v1 3/3] Bluetooth: btintel_pcie: Reduce driver buffer
>posting to prevent race condition
>
>Hi Kiran,
>
>On Mon, May 26, 2025 at 11:58 AM K, Kiran <kiran.k@intel.com> wrote:
>>
>>
>> Hi Paul,
>>
>> Thanks for your comments.
>>
>> >Subject: Re: [PATCH v1 3/3] Bluetooth: btintel_pcie: Reduce driver
>> >buffer posting to prevent race condition
>> >
>> >Dear Chandrashekar, dear Kiran,
>> >
>> >
>> >Thank you for the patch.
>> >
>> >Am 25.05.25 um 07:39 schrieb Kiran K:
>> >> From: Chandrashekar Devegowda
><chandrashekar.devegowda@intel.com>
>> >>
>> >> Modify the driver to post 3 fewer buffers than the maximum rx
>> >> buffers
>> >> (64) allowed for the firmware. This change mitigates a hardware
>> >> issue causing a race condition in the firmware, improving stability
>> >> and data handling.
>> >
>>
>> >Interesting. Please elaborate, which firmware versions are affected,
>> >and if a fix is going to be expected, and how to reproduce the issue,
>> >so it can be tested without and with your patch.
>> >
>> The firmware is for the upcoming product and is not available in public yet.
>As I said in 1/3,  the issue is seen on android kernel and  it's very hard to
>reproduce the issue on vanilla kernel.
>
>If it affects Android specific versions only then it should be posted for
>upstream, anyway this sounds like it is more of a workaround then a proper fix
>for the problem.

The issue is seen on Linux also but the reproduction rate - (1/25) is less compared to android (1/5). As the firmware content is same for both, I feel this work around  is applicable for Linux as well.
>
>> >Is the errata published?
>> >
>> Our internal documents are updated.  I have also entered a comment in
>code.
>>
>> > Why three buffers less and not two or four?
>> The recommendation from firmware / HW is to use at least 3 buffers as guard
>buffers. Anything less than three causes RFH (receive flow handler - RX path)
>blockage.
>
>Are these buffers discovered at runtime or they are hardcoded, for the former
>then the firmware shall be adjusted to give a proper number and in case of the
>later then the driver shall be updated, either way having to do -3 sounds like a
>bad idea in the long term.
>
The maximum number of buffers for RX (N) and the buffers granted for firmware(N-3)  are hardcoded. As the issue is related to the hardware, it would take sometime to get it fixed. 
>
>> >Out of curiosity: Does the Microsoft Windows driver do the same?
>> Yes.
>>
>> >
>> >> Signed-off-by: Chandrashekar Devegowda
>> >> <chandrashekar.devegowda@intel.com>
>> >> Signed-off-by: Kiran K <kiran.k@intel.com>
>> >> ---
>> >>   drivers/bluetooth/btintel_pcie.c | 5 ++++-
>> >>   1 file changed, 4 insertions(+), 1 deletion(-)
>> >>
>> >> diff --git a/drivers/bluetooth/btintel_pcie.c
>> >> b/drivers/bluetooth/btintel_pcie.c
>> >> index 03f13de4a723..14f000e08e1e 100644
>> >> --- a/drivers/bluetooth/btintel_pcie.c
>> >> +++ b/drivers/bluetooth/btintel_pcie.c
>> >> @@ -398,7 +398,10 @@ static int btintel_pcie_start_rx(struct
>> >btintel_pcie_data *data)
>> >>      int i, ret;
>> >>      struct rxq *rxq = &data->rxq;
>> >>
>> >> -    for (i = 0; i < rxq->count; i++) {
>> >> +    /* Post (BTINTEL_PCIE_RX_DESCS_COUNT - 3) buffers to overcome the
>> >> +     * hardware issues leading to race condition at the firmware.
>> >
>> >If you had an errata, it’d be great to document it here to.
>> >
>> >I’d remove *the*.
>> Ack
>> >
>> >> +     */
>> >> +    for (i = 0; i < rxq->count - 3; i++) {
>> >>              ret = btintel_pcie_submit_rx(data);
>> >>              if (ret)
>> >>                      return ret;
>> >
>> >
>> >Kind regards,
>> >
>> >Paul
>>
>> Regards,
>> Kiran
>>
>
>
>--
>Luiz Augusto von Dentz

Thanks,
Kiran