Hi Matthias,
On 2018-11-15 01:06, Matthias Kaehlcke wrote:
On Tue, Nov 06, 2018 at 05:35:28PM +0530, Balakrishna Godavarthi wrote:
[ 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)
The above errors log on console due to receiving of stray bytes
when wcn3990 boot up's i.e. when during initial setup procedure.
Please shortly introduce the topic instead of starting with the log
messages.
[Bala]: will update.
Signed-off-by: Balakrishna Godavarthi <bgodavar@xxxxxxxxxxxxxx>
---
drivers/bluetooth/hci_qca.c | 20 +++++++++++++++++++-
1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index 97b57e0f4725..341f80606574 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 due multiple baudrate switch is causing
+ * UART to read some garbage data.
+ */
This isn't entirely correct. I saw frame reassembly errors before
qca_wcn3990_init() is called, i.e. no baudrate changes. Some of the
garbage seems to be sent shortly after powering on the controller.
[Bala]: yes your correct, will update the comment.
+ 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)) {
@@ -1192,10 +1201,12 @@ static int qca_setup(struct hci_uart *hu)
set_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &hdev->quirks);
clear_bit(HCI_QUIRK_RESET_ON_CLOSE, &hdev->quirks);
hu->hdev->shutdown = qca_power_off;
+
ret = qca_wcn3990_init(hu);
if (ret)
return ret;
+ clear_bit(STATE_DISCARD_RX, &qca->flags);
ret = qca_read_soc_version(hdev, &soc_ver);
if (ret)
return ret;
@@ -1278,8 +1289,15 @@ static void qca_power_shutdown(struct hci_uart
*hu)
struct serdev_device *serdev = hu->serdev;
unsigned char cmd = QCA_WCN3990_POWEROFF_PULSE;
- host_set_baudrate(hu, 2400);
+ /* 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);
+ set_bit(STATE_DISCARD_RX, &qca->flags);
+ qca_flush(hu);
+
hci_uart_set_flow_control(hu, true);
+ host_set_baudrate(hu, 2400);
serdev_device_write(serdev, &cmd, sizeof(cmd), 0);
serdev_device_wait_until_sent(serdev, 0);
hci_uart_set_flow_control(hu, false);
This change won't win a beauty price, but it seems it is needed to
suppress the 'frame reassembly' spam, unless the controller can be
convinced to stop sending garbage in the first place.
[Bala]: This is due to serdev_open function call before we trun on the
regulators.
where we don't have control over serdev open and close calls.
Tested-by: Matthias Kaehlcke <mka@xxxxxxxxxxxx>
[Bala]: Thanks for testing.
--
Regards
Balakrishna.
