Hi Matthias,
On 2018-11-27 05:57, Matthias Kaehlcke wrote:
On Mon, Nov 26, 2018 at 04:09:50PM -0800, Matthias Kaehlcke wrote:
On Thu, Nov 22, 2018 at 05:45:12PM +0530, Balakrishna Godavarthi
wrote:
> During initalization of wcn3990, we observed UART is reading some
> stray bytes on the Rx line. This is logging Frame reassembly errors
> on the serial console. This could be because of tristate of Tx line
> of wcn3990 during boot up.
>
> [ 176.929612] Bluetooth: hci_qca.c:qca_recv() hci0: Frame reassembly failed (-84)
> [ 176.945734] Bluetooth: hci_qca.c:qca_recv() hci0: Frame reassembly failed (-84)
> [ 176.953298] Bluetooth: hci_qca.c:qca_recv() hci0: Frame reassembly failed (-84)
> [ 177.010660] Bluetooth: hci_qca.c:qca_recv() hci0: Frame reassembly failed (-84)
> [ 177.067633] Bluetooth: hci_qca.c:qca_recv() hci0: Frame reassembly failed (-84)
>
> Now we enable a flag during bootup to stop executing proto receive
> function and clear it back once the initialization is done.
>
> Signed-off-by: Balakrishna Godavarthi <bgodavar@xxxxxxxxxxxxxx>
> Tested-by: Matthias Kaehlcke <mka@xxxxxxxxxxxx>
> ---
> v2:
> * Updated commit text & comments.
> v1:
> * initial patch
> ---
> drivers/bluetooth/hci_qca.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
> index ed905ea1c58a..2f3d9e16ba5a 100644
> --- a/drivers/bluetooth/hci_qca.c
> +++ b/drivers/bluetooth/hci_qca.c
> @@ -56,6 +56,7 @@
>
> /* Controller states */
> #define STATE_IN_BAND_SLEEP_ENABLED 1
> +#define STATE_DISCARD_RX 2
>
> #define IBS_WAKE_RETRANS_TIMEOUT_MS 100
> #define IBS_TX_IDLE_TIMEOUT_MS 2000
> @@ -511,6 +512,7 @@ static int qca_open(struct hci_uart *hu)
> } else {
> hu->init_speed = qcadev->init_speed;
> hu->oper_speed = qcadev->oper_speed;
> + set_bit(STATE_DISCARD_RX, &qca->flags);
> ret = qca_power_setup(hu, true);
> if (ret) {
> destroy_workqueue(qca->workqueue);
> @@ -903,6 +905,13 @@ static int qca_recv(struct hci_uart *hu, const void *data, int count)
> if (!test_bit(HCI_UART_REGISTERED, &hu->flags))
> return -EUNATCH;
>
> + /* We discard Rx data received while device is in booting
> + * stage, This is because of BT chip Tx line is in tristate.
> + * Due to this we read some garbage data on UART Rx.
> + */
> + if (test_bit(STATE_DISCARD_RX, &qca->flags))
> + return 0;
> +
> qca->rx_skb = h4_recv_buf(hu->hdev, qca->rx_skb, data, count,
> qca_recv_pkts, ARRAY_SIZE(qca_recv_pkts));
> if (IS_ERR(qca->rx_skb)) {
> @@ -1193,6 +1202,7 @@ static int qca_setup(struct hci_uart *hu)
> if (ret)
> return ret;
>
> + clear_bit(STATE_DISCARD_RX, &qca->flags);
> ret = qca_read_soc_version(hdev, &soc_ver);
> if (ret)
> return ret;
> @@ -1269,6 +1279,14 @@ static const struct qca_vreg_data qca_soc_data = {
>
> static void qca_power_shutdown(struct hci_uart *hu)
> {
> + struct qca_data *qca = hu->priv;
> +
> + /* From this point we go into power off state,
> + * disable IBS and discard all the queued data.
> + */
> + clear_bit(STATE_IN_BAND_SLEEP_ENABLED, &qca->flags);
Is IBS actually related to the frame reassembly errors or is this
addressing a different issue? In 100 iteratios of 'hciconfig up/down'
without clearing the flag I didn't observe any frame reassembly
errors.
Looks like this addresses the "Bluetooth: Received HCI_IBS_SLEEP_IND
in rx state 0" messages that are seen when the interface is brought up
on a system with firmware binaries (with IBS support?). Please spin
this out into a separate patch.
[Bala]: "Bluetooth: Received HCI_IBS_SLEEP_IND in rx state 0 is an
different issue.
i suspect that is due to mismatch of ibs timers over flow value
between wcn3990 & HOST.
clearing ibs is required, to stop ibs state machine while we do
hci close
this is intern help in resolving the frame reassembly errors
during close (in rare case observed)
anyways will send this a separate patch.
Thanks
Matthias
--
Regards
Balakrishna.
