Re: [PATCH] Bluetooth: hci_qca: Add delay for wcn3990 stability
Hi Jeffrey,
On Sat, Oct 19, 2019 at 02:31:18PM -0600, Jeffrey Hugo wrote:
> On Fri, Oct 18, 2019 at 5:15 PM Matthias Kaehlcke wrote:
> >
> > On Fri, Oct 18, 2019 at 04:36:23PM -0600, Jeffrey Hugo wrote:
> > > On Fri, Oct 18, 2019 at 3:33 PM Matthias Kaehlcke
> > > wrote:
> > > >
> > > > On Fri, Oct 18, 2019 at 01:51:39PM -0600, Jeffrey Hugo wrote:
> > > > > On Fri, Oct 18, 2019 at 1:40 PM Matthias Kaehlcke
> > > > > wrote:
> > > > > >
> > > > > > On Fri, Oct 18, 2019 at 12:30:09PM -0600, Jeffrey Hugo wrote:
> > > > > > > On Fri, Oct 18, 2019 at 12:03 PM Matthias Kaehlcke
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > On Thu, Oct 17, 2019 at 02:29:55PM -0700, Jeffrey Hugo wrote:
> > > > > > > > > On the msm8998 mtp, the response to the baudrate change
> > > > > > > > > command is never
> > > > > > > > > received. On the Lenovo Miix 630, the response to the
> > > > > > > > > baudrate change
> > > > > > > > > command is corrupted - "Frame reassembly failed (-84)".
> > > > > > > > >
> > > > > > > > > Adding a 50ms delay before re-enabling flow to receive the
> > > > > > > > > baudrate change
> > > > > > > > > command response from the wcn3990 addesses both issues, and
> > > > > > > > > allows
> > > > > > > > > bluetooth to become functional.
> > > > > > > >
> > > > > > > > From my earlier debugging on sdm845 I don't think this is what
> > > > > > > > happens.
> > > > > > > > The problem is that the wcn3990 sends the response to the
> > > > > > > > baudrate change
> > > > > > > > command using the new baudrate, while the UART on the SoC still
> > > > > > > > operates
> > > > > > > > with the prior speed (for details see 2faa3f15fa2f ("Bluetooth:
> > > > > > > > hci_qca:
> > > > > > > > wcn3990: Drop baudrate change vendor event"))
> > > > > > > >
> > > > > > > > IIRC the 50ms delay causes the HCI core to discard the received
> > > > > > > > data,
> > > > > > > > which is why the "Frame reassembly failed" message disappears,
> > > > > > > > not
> > > > > > > > because the response was received. In theory commit 78e8fa2972e5
> > > > > > > > ("Bluetooth: hci_qca: Deassert RTS while baudrate change
> > > > > > > > command")
> > > > > > > > should have fixed those messages, do you know if CTS/RTS are
> > > > > > > > connected
> > > > > > > > on the Bluetooth UART of the Lenovo Miix 630?
> > > > > > >
> > > > > > > I was testing with 5.4-rc1 which contains the indicated RTS fix.
> > > > > > >
> > > > > > > Yes, CTS/RTS are connected on the Lenovo Miix 630.
> > > > > > >
> > > > > > > I added debug statements which indicated that data was received,
> > > > > > > however it was corrupt, and the packet type did not match what was
> > > > > > > expected, hence the frame reassembly errors.
> > > > > >
> > > > > > Do you know if any data is received during the delay? In theory that
> > > > > > shouldn't be the case since RTS is deasserted, just double-checking.
> > > > >
> > > > > I don't think so, but I've run so many tests, I'm not 100% positive.
> > > > > Let me go double check and get back to you.
> > >
> > > Apparently I'd be wrong. I instrumented the uart driver so that it
> > > would indicate when it got data from the bam. Apparently its getting
> > > the data during the 50ms sleep, approximately right after the host
> > > baud rate is set.
> >
> > Good finding!
> >
> > > > >
> > > > > >
> > > > > > What happens if you add a longer delay (e.g. 1s) before/after
> > > > > > setting
> > > > > > the host baudrate?
> > > > >
> > > > > Hmm, not exactly sure. I will test.
> > >
> > > Adding a 1 second delay before setting the host baud rate did not
> > > change the observed results - still received the data during the 50ms
> > > sleep after the host baud rate set operation.
> > > Adding a 1 second delay after setting the host baud rate did not
> > > change when the data was received.
> >
> > Thanks for testing!
> >
> > > > >
> > > > > >
> > > > > > > In response to this patch, Balakrishna pointed me to a bug report
> > > > > > > which indicated that some of the UART GPIO lines need to have a
> > > > > > > bias
> > > > > > > applied to prevent errant data from floating lines -
> > > > > > >
> > > > > > > https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/1391888
> > > > > >
> > > > > > Yeah, that was another source of frame reassembly errors that we
> > > > > > were
> > > > > > seeing on SDM845.
> > > > > >
> > > > > > Balakrishna, please post these kind of replies on-list, so that
> > > > > > everybody can benefit from possible solutions or contribute to the
> > > > > > discussion.
> > > > > >
> > > > > > > It turns out this fix was never applied to msm8998. Applying the
> > > > > > > fix
> > > > > > > does cause the the frame reassembly errors to go away, however
> > > > > > > then
> > > > > > > the host SoC never receives the baud rate change response (I
> > > > > > > increased
> > > > > > > the timeout from 2faa3f15fa2f ("Bluetooth: hci_qca: wcn3990: Drop
> >
Re: [PATCH] Bluetooth: hci_qca: Add delay for wcn3990 stability
Hi Jeffrey,
> On the msm8998 mtp, the response to the baudrate change command is
> never
> received. On the Lenovo Miix 630, the response to the baudrate change
> command is corrupted - "Frame reassembly failed (-84)".
>
> Adding a 50ms delay before re-enabling flow to receive the baudrate
> change
> command response from the wcn3990 addesses both issues, and allows
> bluetooth to become functional.
From my earlier debugging on sdm845 I don't think this is what happens.
The problem is that the wcn3990 sends the response to the baudrate
change
command using the new baudrate, while the UART on the SoC still
operates
with the prior speed (for details see 2faa3f15fa2f ("Bluetooth:
hci_qca:
wcn3990: Drop baudrate change vendor event"))
IIRC the 50ms delay causes the HCI core to discard the received data,
which is why the "Frame reassembly failed" message disappears, not
because the response was received. In theory commit 78e8fa2972e5
("Bluetooth: hci_qca: Deassert RTS while baudrate change command")
should have fixed those messages, do you know if CTS/RTS are connected
on the Bluetooth UART of the Lenovo Miix 630?
>>>
>>> I was testing with 5.4-rc1 which contains the indicated RTS fix.
>>>
>>> Yes, CTS/RTS are connected on the Lenovo Miix 630.
>>>
>>> I added debug statements which indicated that data was received,
>>> however it was corrupt, and the packet type did not match what was
>>> expected, hence the frame reassembly errors.
>>
>> Do you know if any data is received during the delay? In theory that
>> shouldn't be the case since RTS is deasserted, just double-checking.
>
> I don't think so, but I've run so many tests, I'm not 100% positive.
> Let me go double check and get back to you.
>>>
>>> Apparently I'd be wrong. I instrumented the uart driver so that it
>>> would indicate when it got data from the bam. Apparently its getting
>>> the data during the 50ms sleep, approximately right after the host
>>> baud rate is set.
>>
>> Good finding!
>>
>
>>
>> What happens if you add a longer delay (e.g. 1s) before/after setting
>> the host baudrate?
>
> Hmm, not exactly sure. I will test.
>>>
>>> Adding a 1 second delay before setting the host baud rate did not
>>> change the observed results - still received the data during the 50ms
>>> sleep after the host baud rate set operation.
>>> Adding a 1 second delay after setting the host baud rate did not
>>> change when the data was received.
>>
>> Thanks for testing!
>>
>
>>
>>> In response to this patch, Balakrishna pointed me to a bug report
>>> which indicated that some of the UART GPIO lines need to have a bias
>>> applied to prevent errant data from floating lines -
>>>
>>> https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/1391888
>>
>> Yeah, that was another source of frame reassembly errors that we were
>> seeing on SDM845.
>>
>> Balakrishna, please post these kind of replies on-list, so that
>> everybody can benefit from possible solutions or contribute to the
>> discussion.
>>
>>> It turns out this fix was never applied to msm8998. Applying the fix
>>> does cause the the frame reassembly errors to go away, however then
>>> the host SoC never receives the baud rate change response (I increased
>>> the timeout from 2faa3f15fa2f ("Bluetooth: hci_qca: wcn3990: Drop
>>> baudrate change vendor event") to 5 seconds). As of now, this patch
>>> is still required.
>>
>> Interesting.
>>
>> FTR, this is the full UART pin configuration for cheza (SDM845):
>>
>> &qup_uart6_default {
>>/* Change pinmux to all 4 pins since CTS and RTS are connected */
>>pinmux {
>>pins = "gpio45", "gpio46",
>> "gpio47", "gpio48";
>>};
>>
>>pinconf-cts {
>>/*
>> * Configure a pull-down on 45 (CTS) to match the pull of
>> * the Bluetooth module.
>> */
>>pins = "gpio45";
>>bias-pull-down;
>>};
>>
>>pinconf-rts-tx {
>>/* We'll drive 46 (RTS) and 47 (TX), so no pull */
>>pins = "gpio46", "gpio47";
>>drive-strength = <2>;
>>bias-disable;
>>};
>>
>>pinconf-rx {
>>/*
>> * Configure a pull-up on 48 (RX). This is needed to avoid
>> * garbage data when the TX pin of the Bluetooth module is
>> * in tri-state (mo
Re: [PATCH] Bluetooth: hci_qca: Add delay for wcn3990 stability
On Fri, Oct 18, 2019 at 5:15 PM Matthias Kaehlcke wrote:
>
> On Fri, Oct 18, 2019 at 04:36:23PM -0600, Jeffrey Hugo wrote:
> > On Fri, Oct 18, 2019 at 3:33 PM Matthias Kaehlcke wrote:
> > >
> > > On Fri, Oct 18, 2019 at 01:51:39PM -0600, Jeffrey Hugo wrote:
> > > > On Fri, Oct 18, 2019 at 1:40 PM Matthias Kaehlcke
> > > > wrote:
> > > > >
> > > > > On Fri, Oct 18, 2019 at 12:30:09PM -0600, Jeffrey Hugo wrote:
> > > > > > On Fri, Oct 18, 2019 at 12:03 PM Matthias Kaehlcke
> > > > > > wrote:
> > > > > > >
> > > > > > > On Thu, Oct 17, 2019 at 02:29:55PM -0700, Jeffrey Hugo wrote:
> > > > > > > > On the msm8998 mtp, the response to the baudrate change command
> > > > > > > > is never
> > > > > > > > received. On the Lenovo Miix 630, the response to the baudrate
> > > > > > > > change
> > > > > > > > command is corrupted - "Frame reassembly failed (-84)".
> > > > > > > >
> > > > > > > > Adding a 50ms delay before re-enabling flow to receive the
> > > > > > > > baudrate change
> > > > > > > > command response from the wcn3990 addesses both issues, and
> > > > > > > > allows
> > > > > > > > bluetooth to become functional.
> > > > > > >
> > > > > > > From my earlier debugging on sdm845 I don't think this is what
> > > > > > > happens.
> > > > > > > The problem is that the wcn3990 sends the response to the
> > > > > > > baudrate change
> > > > > > > command using the new baudrate, while the UART on the SoC still
> > > > > > > operates
> > > > > > > with the prior speed (for details see 2faa3f15fa2f ("Bluetooth:
> > > > > > > hci_qca:
> > > > > > > wcn3990: Drop baudrate change vendor event"))
> > > > > > >
> > > > > > > IIRC the 50ms delay causes the HCI core to discard the received
> > > > > > > data,
> > > > > > > which is why the "Frame reassembly failed" message disappears, not
> > > > > > > because the response was received. In theory commit 78e8fa2972e5
> > > > > > > ("Bluetooth: hci_qca: Deassert RTS while baudrate change command")
> > > > > > > should have fixed those messages, do you know if CTS/RTS are
> > > > > > > connected
> > > > > > > on the Bluetooth UART of the Lenovo Miix 630?
> > > > > >
> > > > > > I was testing with 5.4-rc1 which contains the indicated RTS fix.
> > > > > >
> > > > > > Yes, CTS/RTS are connected on the Lenovo Miix 630.
> > > > > >
> > > > > > I added debug statements which indicated that data was received,
> > > > > > however it was corrupt, and the packet type did not match what was
> > > > > > expected, hence the frame reassembly errors.
> > > > >
> > > > > Do you know if any data is received during the delay? In theory that
> > > > > shouldn't be the case since RTS is deasserted, just double-checking.
> > > >
> > > > I don't think so, but I've run so many tests, I'm not 100% positive.
> > > > Let me go double check and get back to you.
> >
> > Apparently I'd be wrong. I instrumented the uart driver so that it
> > would indicate when it got data from the bam. Apparently its getting
> > the data during the 50ms sleep, approximately right after the host
> > baud rate is set.
>
> Good finding!
>
> > > >
> > > > >
> > > > > What happens if you add a longer delay (e.g. 1s) before/after setting
> > > > > the host baudrate?
> > > >
> > > > Hmm, not exactly sure. I will test.
> >
> > Adding a 1 second delay before setting the host baud rate did not
> > change the observed results - still received the data during the 50ms
> > sleep after the host baud rate set operation.
> > Adding a 1 second delay after setting the host baud rate did not
> > change when the data was received.
>
> Thanks for testing!
>
> > > >
> > > > >
> > > > > > In response to this patch, Balakrishna pointed me to a bug report
> > > > > > which indicated that some of the UART GPIO lines need to have a bias
> > > > > > applied to prevent errant data from floating lines -
> > > > > >
> > > > > > https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/1391888
> > > > >
> > > > > Yeah, that was another source of frame reassembly errors that we were
> > > > > seeing on SDM845.
> > > > >
> > > > > Balakrishna, please post these kind of replies on-list, so that
> > > > > everybody can benefit from possible solutions or contribute to the
> > > > > discussion.
> > > > >
> > > > > > It turns out this fix was never applied to msm8998. Applying the
> > > > > > fix
> > > > > > does cause the the frame reassembly errors to go away, however then
> > > > > > the host SoC never receives the baud rate change response (I
> > > > > > increased
> > > > > > the timeout from 2faa3f15fa2f ("Bluetooth: hci_qca: wcn3990: Drop
> > > > > > baudrate change vendor event") to 5 seconds). As of now, this patch
> > > > > > is still required.
> > > > >
> > > > > Interesting.
> > > > >
> > > > > FTR, this is the full UART pin configuration for cheza (SDM845):
> > > > >
> > > > > &qup_uart6_default {
> > > > > /* Change pinmux to all 4 pins since CTS and RTS are
> > > > > connected */
> > > > >
Re: [PATCH] Bluetooth: hci_qca: Add delay for wcn3990 stability
Hi Matthias,
>> On the msm8998 mtp, the response to the baudrate change command is never
>> received. On the Lenovo Miix 630, the response to the baudrate change
>> command is corrupted - "Frame reassembly failed (-84)".
>>
>> Adding a 50ms delay before re-enabling flow to receive the baudrate
>> change
>> command response from the wcn3990 addesses both issues, and allows
>> bluetooth to become functional.
>
> From my earlier debugging on sdm845 I don't think this is what happens.
> The problem is that the wcn3990 sends the response to the baudrate change
> command using the new baudrate, while the UART on the SoC still operates
> with the prior speed (for details see 2faa3f15fa2f ("Bluetooth: hci_qca:
> wcn3990: Drop baudrate change vendor event"))
>
> IIRC the 50ms delay causes the HCI core to discard the received data,
> which is why the "Frame reassembly failed" message disappears, not
> because the response was received. In theory commit 78e8fa2972e5
> ("Bluetooth: hci_qca: Deassert RTS while baudrate change command")
> should have fixed those messages, do you know if CTS/RTS are connected
> on the Bluetooth UART of the Lenovo Miix 630?
I was testing with 5.4-rc1 which contains the indicated RTS fix.
Yes, CTS/RTS are connected on the Lenovo Miix 630.
I added debug statements which indicated that data was received,
however it was corrupt, and the packet type did not match what was
expected, hence the frame reassembly errors.
>>>
>>> Do you know if any data is received during the delay? In theory that
>>> shouldn't be the case since RTS is deasserted, just double-checking.
>>
>> I don't think so, but I've run so many tests, I'm not 100% positive.
>> Let me go double check and get back to you.
>>
>>>
>>> What happens if you add a longer delay (e.g. 1s) before/after setting
>>> the host baudrate?
>>
>> Hmm, not exactly sure. I will test.
>>
>>>
In response to this patch, Balakrishna pointed me to a bug report
which indicated that some of the UART GPIO lines need to have a bias
applied to prevent errant data from floating lines -
https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/1391888
>>>
>>> Yeah, that was another source of frame reassembly errors that we were
>>> seeing on SDM845.
>>>
>>> Balakrishna, please post these kind of replies on-list, so that
>>> everybody can benefit from possible solutions or contribute to the
>>> discussion.
>>>
It turns out this fix was never applied to msm8998. Applying the fix
does cause the the frame reassembly errors to go away, however then
the host SoC never receives the baud rate change response (I increased
the timeout from 2faa3f15fa2f ("Bluetooth: hci_qca: wcn3990: Drop
baudrate change vendor event") to 5 seconds). As of now, this patch
is still required.
>>>
>>> Interesting.
>>>
>>> FTR, this is the full UART pin configuration for cheza (SDM845):
>>>
>>> &qup_uart6_default {
>>>/* Change pinmux to all 4 pins since CTS and RTS are connected */
>>>pinmux {
>>>pins = "gpio45", "gpio46",
>>> "gpio47", "gpio48";
>>>};
>>>
>>>pinconf-cts {
>>>/*
>>> * Configure a pull-down on 45 (CTS) to match the pull of
>>> * the Bluetooth module.
>>> */
>>>pins = "gpio45";
>>>bias-pull-down;
>>>};
>>>
>>>pinconf-rts-tx {
>>>/* We'll drive 46 (RTS) and 47 (TX), so no pull */
>>>pins = "gpio46", "gpio47";
>>>drive-strength = <2>;
>>>bias-disable;
>>>};
>>>
>>>pinconf-rx {
>>>/*
>>> * Configure a pull-up on 48 (RX). This is needed to avoid
>>> * garbage data when the TX pin of the Bluetooth module is
>>> * in tri-state (module powered off or not driving the
>>> * signal yet).
>>> */
>>>pins = "gpio48";
>>>bias-pull-up;
>>>};
>>> };
>>>
>>> Does this correspond to what you tried on the Lenovo Miix 630?
>>
>> Which GPIO maps to which pin is different -
>> 45 - TX
>> 46 - RX
>> 47 - CTS
>> 48 - RFR (RTS)
>>
>> However, accounting for that, yes that corresponds to what I used.
>
> Thanks for re-confirming.
>
I have no idea why the delay is required, and was hoping that posting
this patch would result in someone else providing some missing pieces
to determine the real root cause. I suspect that asserting RTS at the
wrong time may cause an issue for the wcn3990, but I have no data nor
documentation to support this guess. I welcome any further insights
you may have.
>>>
>>> Unfortunately I don't have a clear suggestion at this point, d
Re: [PATCH] Bluetooth: hci_qca: Add delay for wcn3990 stability
On Fri, Oct 18, 2019 at 04:36:23PM -0600, Jeffrey Hugo wrote:
> On Fri, Oct 18, 2019 at 3:33 PM Matthias Kaehlcke wrote:
> >
> > On Fri, Oct 18, 2019 at 01:51:39PM -0600, Jeffrey Hugo wrote:
> > > On Fri, Oct 18, 2019 at 1:40 PM Matthias Kaehlcke
> > > wrote:
> > > >
> > > > On Fri, Oct 18, 2019 at 12:30:09PM -0600, Jeffrey Hugo wrote:
> > > > > On Fri, Oct 18, 2019 at 12:03 PM Matthias Kaehlcke
> > > > > wrote:
> > > > > >
> > > > > > On Thu, Oct 17, 2019 at 02:29:55PM -0700, Jeffrey Hugo wrote:
> > > > > > > On the msm8998 mtp, the response to the baudrate change command
> > > > > > > is never
> > > > > > > received. On the Lenovo Miix 630, the response to the baudrate
> > > > > > > change
> > > > > > > command is corrupted - "Frame reassembly failed (-84)".
> > > > > > >
> > > > > > > Adding a 50ms delay before re-enabling flow to receive the
> > > > > > > baudrate change
> > > > > > > command response from the wcn3990 addesses both issues, and allows
> > > > > > > bluetooth to become functional.
> > > > > >
> > > > > > From my earlier debugging on sdm845 I don't think this is what
> > > > > > happens.
> > > > > > The problem is that the wcn3990 sends the response to the baudrate
> > > > > > change
> > > > > > command using the new baudrate, while the UART on the SoC still
> > > > > > operates
> > > > > > with the prior speed (for details see 2faa3f15fa2f ("Bluetooth:
> > > > > > hci_qca:
> > > > > > wcn3990: Drop baudrate change vendor event"))
> > > > > >
> > > > > > IIRC the 50ms delay causes the HCI core to discard the received
> > > > > > data,
> > > > > > which is why the "Frame reassembly failed" message disappears, not
> > > > > > because the response was received. In theory commit 78e8fa2972e5
> > > > > > ("Bluetooth: hci_qca: Deassert RTS while baudrate change command")
> > > > > > should have fixed those messages, do you know if CTS/RTS are
> > > > > > connected
> > > > > > on the Bluetooth UART of the Lenovo Miix 630?
> > > > >
> > > > > I was testing with 5.4-rc1 which contains the indicated RTS fix.
> > > > >
> > > > > Yes, CTS/RTS are connected on the Lenovo Miix 630.
> > > > >
> > > > > I added debug statements which indicated that data was received,
> > > > > however it was corrupt, and the packet type did not match what was
> > > > > expected, hence the frame reassembly errors.
> > > >
> > > > Do you know if any data is received during the delay? In theory that
> > > > shouldn't be the case since RTS is deasserted, just double-checking.
> > >
> > > I don't think so, but I've run so many tests, I'm not 100% positive.
> > > Let me go double check and get back to you.
>
> Apparently I'd be wrong. I instrumented the uart driver so that it
> would indicate when it got data from the bam. Apparently its getting
> the data during the 50ms sleep, approximately right after the host
> baud rate is set.
Good finding!
> > >
> > > >
> > > > What happens if you add a longer delay (e.g. 1s) before/after setting
> > > > the host baudrate?
> > >
> > > Hmm, not exactly sure. I will test.
>
> Adding a 1 second delay before setting the host baud rate did not
> change the observed results - still received the data during the 50ms
> sleep after the host baud rate set operation.
> Adding a 1 second delay after setting the host baud rate did not
> change when the data was received.
Thanks for testing!
> > >
> > > >
> > > > > In response to this patch, Balakrishna pointed me to a bug report
> > > > > which indicated that some of the UART GPIO lines need to have a bias
> > > > > applied to prevent errant data from floating lines -
> > > > >
> > > > > https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/1391888
> > > >
> > > > Yeah, that was another source of frame reassembly errors that we were
> > > > seeing on SDM845.
> > > >
> > > > Balakrishna, please post these kind of replies on-list, so that
> > > > everybody can benefit from possible solutions or contribute to the
> > > > discussion.
> > > >
> > > > > It turns out this fix was never applied to msm8998. Applying the fix
> > > > > does cause the the frame reassembly errors to go away, however then
> > > > > the host SoC never receives the baud rate change response (I increased
> > > > > the timeout from 2faa3f15fa2f ("Bluetooth: hci_qca: wcn3990: Drop
> > > > > baudrate change vendor event") to 5 seconds). As of now, this patch
> > > > > is still required.
> > > >
> > > > Interesting.
> > > >
> > > > FTR, this is the full UART pin configuration for cheza (SDM845):
> > > >
> > > > &qup_uart6_default {
> > > > /* Change pinmux to all 4 pins since CTS and RTS are connected
> > > > */
> > > > pinmux {
> > > > pins = "gpio45", "gpio46",
> > > >"gpio47", "gpio48";
> > > > };
> > > >
> > > > pinconf-cts {
> > > > /*
> > > > * Configure a pull-down on 45 (CTS) to match the pull
> > > > of
> > > >
Re: [PATCH] Bluetooth: hci_qca: Add delay for wcn3990 stability
On Fri, Oct 18, 2019 at 3:33 PM Matthias Kaehlcke wrote:
>
> On Fri, Oct 18, 2019 at 01:51:39PM -0600, Jeffrey Hugo wrote:
> > On Fri, Oct 18, 2019 at 1:40 PM Matthias Kaehlcke wrote:
> > >
> > > On Fri, Oct 18, 2019 at 12:30:09PM -0600, Jeffrey Hugo wrote:
> > > > On Fri, Oct 18, 2019 at 12:03 PM Matthias Kaehlcke
> > > > wrote:
> > > > >
> > > > > On Thu, Oct 17, 2019 at 02:29:55PM -0700, Jeffrey Hugo wrote:
> > > > > > On the msm8998 mtp, the response to the baudrate change command is
> > > > > > never
> > > > > > received. On the Lenovo Miix 630, the response to the baudrate
> > > > > > change
> > > > > > command is corrupted - "Frame reassembly failed (-84)".
> > > > > >
> > > > > > Adding a 50ms delay before re-enabling flow to receive the baudrate
> > > > > > change
> > > > > > command response from the wcn3990 addesses both issues, and allows
> > > > > > bluetooth to become functional.
> > > > >
> > > > > From my earlier debugging on sdm845 I don't think this is what
> > > > > happens.
> > > > > The problem is that the wcn3990 sends the response to the baudrate
> > > > > change
> > > > > command using the new baudrate, while the UART on the SoC still
> > > > > operates
> > > > > with the prior speed (for details see 2faa3f15fa2f ("Bluetooth:
> > > > > hci_qca:
> > > > > wcn3990: Drop baudrate change vendor event"))
> > > > >
> > > > > IIRC the 50ms delay causes the HCI core to discard the received data,
> > > > > which is why the "Frame reassembly failed" message disappears, not
> > > > > because the response was received. In theory commit 78e8fa2972e5
> > > > > ("Bluetooth: hci_qca: Deassert RTS while baudrate change command")
> > > > > should have fixed those messages, do you know if CTS/RTS are connected
> > > > > on the Bluetooth UART of the Lenovo Miix 630?
> > > >
> > > > I was testing with 5.4-rc1 which contains the indicated RTS fix.
> > > >
> > > > Yes, CTS/RTS are connected on the Lenovo Miix 630.
> > > >
> > > > I added debug statements which indicated that data was received,
> > > > however it was corrupt, and the packet type did not match what was
> > > > expected, hence the frame reassembly errors.
> > >
> > > Do you know if any data is received during the delay? In theory that
> > > shouldn't be the case since RTS is deasserted, just double-checking.
> >
> > I don't think so, but I've run so many tests, I'm not 100% positive.
> > Let me go double check and get back to you.
Apparently I'd be wrong. I instrumented the uart driver so that it
would indicate when it got data from the bam. Apparently its getting
the data during the 50ms sleep, approximately right after the host
baud rate is set.
> >
> > >
> > > What happens if you add a longer delay (e.g. 1s) before/after setting
> > > the host baudrate?
> >
> > Hmm, not exactly sure. I will test.
Adding a 1 second delay before setting the host baud rate did not
change the observed results - still received the data during the 50ms
sleep after the host baud rate set operation.
Adding a 1 second delay after setting the host baud rate did not
change when the data was received.
> >
> > >
> > > > In response to this patch, Balakrishna pointed me to a bug report
> > > > which indicated that some of the UART GPIO lines need to have a bias
> > > > applied to prevent errant data from floating lines -
> > > >
> > > > https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/1391888
> > >
> > > Yeah, that was another source of frame reassembly errors that we were
> > > seeing on SDM845.
> > >
> > > Balakrishna, please post these kind of replies on-list, so that
> > > everybody can benefit from possible solutions or contribute to the
> > > discussion.
> > >
> > > > It turns out this fix was never applied to msm8998. Applying the fix
> > > > does cause the the frame reassembly errors to go away, however then
> > > > the host SoC never receives the baud rate change response (I increased
> > > > the timeout from 2faa3f15fa2f ("Bluetooth: hci_qca: wcn3990: Drop
> > > > baudrate change vendor event") to 5 seconds). As of now, this patch
> > > > is still required.
> > >
> > > Interesting.
> > >
> > > FTR, this is the full UART pin configuration for cheza (SDM845):
> > >
> > > &qup_uart6_default {
> > > /* Change pinmux to all 4 pins since CTS and RTS are connected */
> > > pinmux {
> > > pins = "gpio45", "gpio46",
> > >"gpio47", "gpio48";
> > > };
> > >
> > > pinconf-cts {
> > > /*
> > > * Configure a pull-down on 45 (CTS) to match the pull of
> > > * the Bluetooth module.
> > > */
> > > pins = "gpio45";
> > > bias-pull-down;
> > > };
> > >
> > > pinconf-rts-tx {
> > > /* We'll drive 46 (RTS) and 47 (TX), so no pull */
> > > pins = "gpio46", "gpio47";
> > > drive-strength = <2>;
> > >
Re: [PATCH] Bluetooth: hci_qca: Add delay for wcn3990 stability
On Fri, Oct 18, 2019 at 01:51:39PM -0600, Jeffrey Hugo wrote:
> On Fri, Oct 18, 2019 at 1:40 PM Matthias Kaehlcke wrote:
> >
> > On Fri, Oct 18, 2019 at 12:30:09PM -0600, Jeffrey Hugo wrote:
> > > On Fri, Oct 18, 2019 at 12:03 PM Matthias Kaehlcke
> > > wrote:
> > > >
> > > > On Thu, Oct 17, 2019 at 02:29:55PM -0700, Jeffrey Hugo wrote:
> > > > > On the msm8998 mtp, the response to the baudrate change command is
> > > > > never
> > > > > received. On the Lenovo Miix 630, the response to the baudrate change
> > > > > command is corrupted - "Frame reassembly failed (-84)".
> > > > >
> > > > > Adding a 50ms delay before re-enabling flow to receive the baudrate
> > > > > change
> > > > > command response from the wcn3990 addesses both issues, and allows
> > > > > bluetooth to become functional.
> > > >
> > > > From my earlier debugging on sdm845 I don't think this is what happens.
> > > > The problem is that the wcn3990 sends the response to the baudrate
> > > > change
> > > > command using the new baudrate, while the UART on the SoC still operates
> > > > with the prior speed (for details see 2faa3f15fa2f ("Bluetooth: hci_qca:
> > > > wcn3990: Drop baudrate change vendor event"))
> > > >
> > > > IIRC the 50ms delay causes the HCI core to discard the received data,
> > > > which is why the "Frame reassembly failed" message disappears, not
> > > > because the response was received. In theory commit 78e8fa2972e5
> > > > ("Bluetooth: hci_qca: Deassert RTS while baudrate change command")
> > > > should have fixed those messages, do you know if CTS/RTS are connected
> > > > on the Bluetooth UART of the Lenovo Miix 630?
> > >
> > > I was testing with 5.4-rc1 which contains the indicated RTS fix.
> > >
> > > Yes, CTS/RTS are connected on the Lenovo Miix 630.
> > >
> > > I added debug statements which indicated that data was received,
> > > however it was corrupt, and the packet type did not match what was
> > > expected, hence the frame reassembly errors.
> >
> > Do you know if any data is received during the delay? In theory that
> > shouldn't be the case since RTS is deasserted, just double-checking.
>
> I don't think so, but I've run so many tests, I'm not 100% positive.
> Let me go double check and get back to you.
>
> >
> > What happens if you add a longer delay (e.g. 1s) before/after setting
> > the host baudrate?
>
> Hmm, not exactly sure. I will test.
>
> >
> > > In response to this patch, Balakrishna pointed me to a bug report
> > > which indicated that some of the UART GPIO lines need to have a bias
> > > applied to prevent errant data from floating lines -
> > >
> > > https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/1391888
> >
> > Yeah, that was another source of frame reassembly errors that we were
> > seeing on SDM845.
> >
> > Balakrishna, please post these kind of replies on-list, so that
> > everybody can benefit from possible solutions or contribute to the
> > discussion.
> >
> > > It turns out this fix was never applied to msm8998. Applying the fix
> > > does cause the the frame reassembly errors to go away, however then
> > > the host SoC never receives the baud rate change response (I increased
> > > the timeout from 2faa3f15fa2f ("Bluetooth: hci_qca: wcn3990: Drop
> > > baudrate change vendor event") to 5 seconds). As of now, this patch
> > > is still required.
> >
> > Interesting.
> >
> > FTR, this is the full UART pin configuration for cheza (SDM845):
> >
> > &qup_uart6_default {
> > /* Change pinmux to all 4 pins since CTS and RTS are connected */
> > pinmux {
> > pins = "gpio45", "gpio46",
> >"gpio47", "gpio48";
> > };
> >
> > pinconf-cts {
> > /*
> > * Configure a pull-down on 45 (CTS) to match the pull of
> > * the Bluetooth module.
> > */
> > pins = "gpio45";
> > bias-pull-down;
> > };
> >
> > pinconf-rts-tx {
> > /* We'll drive 46 (RTS) and 47 (TX), so no pull */
> > pins = "gpio46", "gpio47";
> > drive-strength = <2>;
> > bias-disable;
> > };
> >
> > pinconf-rx {
> > /*
> > * Configure a pull-up on 48 (RX). This is needed to avoid
> > * garbage data when the TX pin of the Bluetooth module is
> > * in tri-state (module powered off or not driving the
> > * signal yet).
> > */
> > pins = "gpio48";
> > bias-pull-up;
> > };
> > };
> >
> > Does this correspond to what you tried on the Lenovo Miix 630?
>
> Which GPIO maps to which pin is different -
> 45 - TX
> 46 - RX
> 47 - CTS
> 48 - RFR (RTS)
>
> However, accounting for that, yes that corresponds to what I used.
Thanks for re-confirming.
> > > I have no idea why the delay is required, and was hopi
Re: [PATCH] Bluetooth: hci_qca: Add delay for wcn3990 stability
On Fri, Oct 18, 2019 at 1:40 PM Matthias Kaehlcke wrote:
>
> On Fri, Oct 18, 2019 at 12:30:09PM -0600, Jeffrey Hugo wrote:
> > On Fri, Oct 18, 2019 at 12:03 PM Matthias Kaehlcke
> > wrote:
> > >
> > > On Thu, Oct 17, 2019 at 02:29:55PM -0700, Jeffrey Hugo wrote:
> > > > On the msm8998 mtp, the response to the baudrate change command is never
> > > > received. On the Lenovo Miix 630, the response to the baudrate change
> > > > command is corrupted - "Frame reassembly failed (-84)".
> > > >
> > > > Adding a 50ms delay before re-enabling flow to receive the baudrate
> > > > change
> > > > command response from the wcn3990 addesses both issues, and allows
> > > > bluetooth to become functional.
> > >
> > > From my earlier debugging on sdm845 I don't think this is what happens.
> > > The problem is that the wcn3990 sends the response to the baudrate change
> > > command using the new baudrate, while the UART on the SoC still operates
> > > with the prior speed (for details see 2faa3f15fa2f ("Bluetooth: hci_qca:
> > > wcn3990: Drop baudrate change vendor event"))
> > >
> > > IIRC the 50ms delay causes the HCI core to discard the received data,
> > > which is why the "Frame reassembly failed" message disappears, not
> > > because the response was received. In theory commit 78e8fa2972e5
> > > ("Bluetooth: hci_qca: Deassert RTS while baudrate change command")
> > > should have fixed those messages, do you know if CTS/RTS are connected
> > > on the Bluetooth UART of the Lenovo Miix 630?
> >
> > I was testing with 5.4-rc1 which contains the indicated RTS fix.
> >
> > Yes, CTS/RTS are connected on the Lenovo Miix 630.
> >
> > I added debug statements which indicated that data was received,
> > however it was corrupt, and the packet type did not match what was
> > expected, hence the frame reassembly errors.
>
> Do you know if any data is received during the delay? In theory that
> shouldn't be the case since RTS is deasserted, just double-checking.
I don't think so, but I've run so many tests, I'm not 100% positive.
Let me go double check and get back to you.
>
> What happens if you add a longer delay (e.g. 1s) before/after setting
> the host baudrate?
Hmm, not exactly sure. I will test.
>
> > In response to this patch, Balakrishna pointed me to a bug report
> > which indicated that some of the UART GPIO lines need to have a bias
> > applied to prevent errant data from floating lines -
> >
> > https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/1391888
>
> Yeah, that was another source of frame reassembly errors that we were
> seeing on SDM845.
>
> Balakrishna, please post these kind of replies on-list, so that
> everybody can benefit from possible solutions or contribute to the
> discussion.
>
> > It turns out this fix was never applied to msm8998. Applying the fix
> > does cause the the frame reassembly errors to go away, however then
> > the host SoC never receives the baud rate change response (I increased
> > the timeout from 2faa3f15fa2f ("Bluetooth: hci_qca: wcn3990: Drop
> > baudrate change vendor event") to 5 seconds). As of now, this patch
> > is still required.
>
> Interesting.
>
> FTR, this is the full UART pin configuration for cheza (SDM845):
>
> &qup_uart6_default {
> /* Change pinmux to all 4 pins since CTS and RTS are connected */
> pinmux {
> pins = "gpio45", "gpio46",
>"gpio47", "gpio48";
> };
>
> pinconf-cts {
> /*
> * Configure a pull-down on 45 (CTS) to match the pull of
> * the Bluetooth module.
> */
> pins = "gpio45";
> bias-pull-down;
> };
>
> pinconf-rts-tx {
> /* We'll drive 46 (RTS) and 47 (TX), so no pull */
> pins = "gpio46", "gpio47";
> drive-strength = <2>;
> bias-disable;
> };
>
> pinconf-rx {
> /*
> * Configure a pull-up on 48 (RX). This is needed to avoid
> * garbage data when the TX pin of the Bluetooth module is
> * in tri-state (module powered off or not driving the
> * signal yet).
> */
> pins = "gpio48";
> bias-pull-up;
> };
> };
>
> Does this correspond to what you tried on the Lenovo Miix 630?
Which GPIO maps to which pin is different -
45 - TX
46 - RX
47 - CTS
48 - RFR (RTS)
However, accounting for that, yes that corresponds to what I used.
>
> > I have no idea why the delay is required, and was hoping that posting
> > this patch would result in someone else providing some missing pieces
> > to determine the real root cause. I suspect that asserting RTS at the
> > wrong time may cause an issue for the wcn3990, but I have no data nor
> > documentation to support this guess. I welcome any further insights
> > you may have.
>
> Unfortunate
Re: [PATCH] Bluetooth: hci_qca: Add delay for wcn3990 stability
On Fri, Oct 18, 2019 at 12:30:09PM -0600, Jeffrey Hugo wrote:
> On Fri, Oct 18, 2019 at 12:03 PM Matthias Kaehlcke wrote:
> >
> > On Thu, Oct 17, 2019 at 02:29:55PM -0700, Jeffrey Hugo wrote:
> > > On the msm8998 mtp, the response to the baudrate change command is never
> > > received. On the Lenovo Miix 630, the response to the baudrate change
> > > command is corrupted - "Frame reassembly failed (-84)".
> > >
> > > Adding a 50ms delay before re-enabling flow to receive the baudrate change
> > > command response from the wcn3990 addesses both issues, and allows
> > > bluetooth to become functional.
> >
> > From my earlier debugging on sdm845 I don't think this is what happens.
> > The problem is that the wcn3990 sends the response to the baudrate change
> > command using the new baudrate, while the UART on the SoC still operates
> > with the prior speed (for details see 2faa3f15fa2f ("Bluetooth: hci_qca:
> > wcn3990: Drop baudrate change vendor event"))
> >
> > IIRC the 50ms delay causes the HCI core to discard the received data,
> > which is why the "Frame reassembly failed" message disappears, not
> > because the response was received. In theory commit 78e8fa2972e5
> > ("Bluetooth: hci_qca: Deassert RTS while baudrate change command")
> > should have fixed those messages, do you know if CTS/RTS are connected
> > on the Bluetooth UART of the Lenovo Miix 630?
>
> I was testing with 5.4-rc1 which contains the indicated RTS fix.
>
> Yes, CTS/RTS are connected on the Lenovo Miix 630.
>
> I added debug statements which indicated that data was received,
> however it was corrupt, and the packet type did not match what was
> expected, hence the frame reassembly errors.
Do you know if any data is received during the delay? In theory that
shouldn't be the case since RTS is deasserted, just double-checking.
What happens if you add a longer delay (e.g. 1s) before/after setting
the host baudrate?
> In response to this patch, Balakrishna pointed me to a bug report
> which indicated that some of the UART GPIO lines need to have a bias
> applied to prevent errant data from floating lines -
>
> https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/1391888
Yeah, that was another source of frame reassembly errors that we were
seeing on SDM845.
Balakrishna, please post these kind of replies on-list, so that
everybody can benefit from possible solutions or contribute to the
discussion.
> It turns out this fix was never applied to msm8998. Applying the fix
> does cause the the frame reassembly errors to go away, however then
> the host SoC never receives the baud rate change response (I increased
> the timeout from 2faa3f15fa2f ("Bluetooth: hci_qca: wcn3990: Drop
> baudrate change vendor event") to 5 seconds). As of now, this patch
> is still required.
Interesting.
FTR, this is the full UART pin configuration for cheza (SDM845):
&qup_uart6_default {
/* Change pinmux to all 4 pins since CTS and RTS are connected */
pinmux {
pins = "gpio45", "gpio46",
"gpio47", "gpio48";
};
pinconf-cts {
/*
* Configure a pull-down on 45 (CTS) to match the pull of
* the Bluetooth module.
*/
pins = "gpio45";
bias-pull-down;
};
pinconf-rts-tx {
/* We'll drive 46 (RTS) and 47 (TX), so no pull */
pins = "gpio46", "gpio47";
drive-strength = <2>;
bias-disable;
};
pinconf-rx {
/*
* Configure a pull-up on 48 (RX). This is needed to avoid
* garbage data when the TX pin of the Bluetooth module is
* in tri-state (module powered off or not driving the
* signal yet).
*/
pins = "gpio48";
bias-pull-up;
};
};
Does this correspond to what you tried on the Lenovo Miix 630?
> I have no idea why the delay is required, and was hoping that posting
> this patch would result in someone else providing some missing pieces
> to determine the real root cause. I suspect that asserting RTS at the
> wrong time may cause an issue for the wcn3990, but I have no data nor
> documentation to support this guess. I welcome any further insights
> you may have.
Unfortunately I don't have a clear suggestion at this point, debugging
the original problem which lead to 2faa3f15fa2f ("Bluetooth: hci_qca:
wcn3990: Drop baudrate change vendor event") involved quite some time
and hooking up a scope/logic analyzer ...
I also suspect RTS is involved, and potentially the configuration of
the pulls. It might be interesting to analyze the data that leads to
the frame assembly error and determine if it is just noise (wrong
pulls/drive strength?) or received with a non-matching baud-rate.
The 50ms delay doesn't really cause any harm, but ideally we'd
und
Re: [PATCH] Bluetooth: hci_qca: Add delay for wcn3990 stability
On Fri, Oct 18, 2019 at 12:03 PM Matthias Kaehlcke wrote:
>
> On Thu, Oct 17, 2019 at 02:29:55PM -0700, Jeffrey Hugo wrote:
> > On the msm8998 mtp, the response to the baudrate change command is never
> > received. On the Lenovo Miix 630, the response to the baudrate change
> > command is corrupted - "Frame reassembly failed (-84)".
> >
> > Adding a 50ms delay before re-enabling flow to receive the baudrate change
> > command response from the wcn3990 addesses both issues, and allows
> > bluetooth to become functional.
>
> From my earlier debugging on sdm845 I don't think this is what happens.
> The problem is that the wcn3990 sends the response to the baudrate change
> command using the new baudrate, while the UART on the SoC still operates
> with the prior speed (for details see 2faa3f15fa2f ("Bluetooth: hci_qca:
> wcn3990: Drop baudrate change vendor event"))
>
> IIRC the 50ms delay causes the HCI core to discard the received data,
> which is why the "Frame reassembly failed" message disappears, not
> because the response was received. In theory commit 78e8fa2972e5
> ("Bluetooth: hci_qca: Deassert RTS while baudrate change command")
> should have fixed those messages, do you know if CTS/RTS are connected
> on the Bluetooth UART of the Lenovo Miix 630?
I was testing with 5.4-rc1 which contains the indicated RTS fix.
Yes, CTS/RTS are connected on the Lenovo Miix 630.
I added debug statements which indicated that data was received,
however it was corrupt, and the packet type did not match what was
expected, hence the frame reassembly errors.
In response to this patch, Balakrishna pointed me to a bug report
which indicated that some of the UART GPIO lines need to have a bias
applied to prevent errant data from floating lines -
https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/1391888
It turns out this fix was never applied to msm8998. Applying the fix
does cause the the frame reassembly errors to go away, however then
the host SoC never receives the baud rate change response (I increased
the timeout from 2faa3f15fa2f ("Bluetooth: hci_qca: wcn3990: Drop
baudrate change vendor event") to 5 seconds). As of now, this patch
is still required.
I have no idea why the delay is required, and was hoping that posting
this patch would result in someone else providing some missing pieces
to determine the real root cause. I suspect that asserting RTS at the
wrong time may cause an issue for the wcn3990, but I have no data nor
documentation to support this guess. I welcome any further insights
you may have.
Re: [PATCH] Bluetooth: hci_qca: Add delay for wcn3990 stability
On Thu, Oct 17, 2019 at 02:29:55PM -0700, Jeffrey Hugo wrote:
> On the msm8998 mtp, the response to the baudrate change command is never
> received. On the Lenovo Miix 630, the response to the baudrate change
> command is corrupted - "Frame reassembly failed (-84)".
>
> Adding a 50ms delay before re-enabling flow to receive the baudrate change
> command response from the wcn3990 addesses both issues, and allows
> bluetooth to become functional.
>From my earlier debugging on sdm845 I don't think this is what happens.
The problem is that the wcn3990 sends the response to the baudrate change
command using the new baudrate, while the UART on the SoC still operates
with the prior speed (for details see 2faa3f15fa2f ("Bluetooth: hci_qca:
wcn3990: Drop baudrate change vendor event"))
IIRC the 50ms delay causes the HCI core to discard the received data,
which is why the "Frame reassembly failed" message disappears, not
because the response was received. In theory commit 78e8fa2972e5
("Bluetooth: hci_qca: Deassert RTS while baudrate change command")
should have fixed those messages, do you know if CTS/RTS are connected
on the Bluetooth UART of the Lenovo Miix 630?
Re: [PATCH] Bluetooth: hci_qca: Add delay for wcn3990 stability
Hi Jeffrey, > On the msm8998 mtp, the response to the baudrate change command is never > received. On the Lenovo Miix 630, the response to the baudrate change > command is corrupted - "Frame reassembly failed (-84)". > > Adding a 50ms delay before re-enabling flow to receive the baudrate change > command response from the wcn3990 addesses both issues, and allows > bluetooth to become functional. > > Signed-off-by: Jeffrey Hugo > --- > drivers/bluetooth/hci_qca.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) patch has been applied to bluetooth-next tree. Regards Marcel
[PATCH] Bluetooth: hci_qca: Add delay for wcn3990 stability
On the msm8998 mtp, the response to the baudrate change command is never
received. On the Lenovo Miix 630, the response to the baudrate change
command is corrupted - "Frame reassembly failed (-84)".
Adding a 50ms delay before re-enabling flow to receive the baudrate change
command response from the wcn3990 addesses both issues, and allows
bluetooth to become functional.
Signed-off-by: Jeffrey Hugo
---
drivers/bluetooth/hci_qca.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index e3164c200eac..265fc60c3850 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -1156,8 +1156,10 @@ static int qca_set_speed(struct hci_uart *hu, enum
qca_speed_type speed_type)
host_set_baudrate(hu, speed);
error:
- if (qca_is_wcn399x(soc_type))
+ if (qca_is_wcn399x(soc_type)) {
+ msleep(50);
hci_uart_set_flow_control(hu, false);
+ }
if (soc_type == QCA_WCN3990) {
/* Wait for the controller to send the vendor event
--
2.17.1
