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.
> >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@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
