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@xxxxxxxxx> 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@xxxxxxxxx>
>> >>
>> >> 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@xxxxxxxxx>
>> >> Signed-off-by: Kiran K <kiran.k@xxxxxxxxx>
>> >> ---
>> >> 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
