Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

Bluetooth: hci_qca: wcn3990: Drop baudrate change vendor event

Firmware download to the WCN3990 often fails with a 'TLV response size
mismatch' error:

[ 133.064659] Bluetooth: hci0: setting up wcn3990
[ 133.489150] Bluetooth: hci0: QCA controller version 0x02140201
[ 133.495245] Bluetooth: hci0: QCA Downloading qca/crbtfw21.tlv
[ 133.507214] Bluetooth: hci0: QCA TLV response size mismatch
[ 133.513265] Bluetooth: hci0: QCA Failed to download patch (-84)

This is caused by a vendor event that corresponds to an earlier command
to change the baudrate. The event is not processed in the context of the
baudrate change and is later interpreted as response to the firmware
download command (which is also a vendor command), but the driver detects
that the event doesn't have the expected amount of associated data.

More details:

For the WCN3990 the vendor command for a baudrate change isn't sent as
synchronous HCI command, because the controller sends the corresponding
vendor event with the new baudrate. The event is received and decoded
after the baudrate change of the host port.

Identify the 'unused' event when it is received and don't add it to
the queue of RX frames.

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
Signed-off-by: Marcel Holtmann <marcel@holtmann.org>

authored by

Matthias Kaehlcke and committed by
Marcel Holtmann
2faa3f15 32646db8

+54 -1
+54 -1
drivers/bluetooth/hci_qca.c
··· 17 17 18 18 #include <linux/kernel.h> 19 19 #include <linux/clk.h> 20 + #include <linux/completion.h> 20 21 #include <linux/debugfs.h> 21 22 #include <linux/delay.h> 22 23 #include <linux/device.h> ··· 54 53 55 54 enum qca_flags { 56 55 QCA_IBS_ENABLED, 56 + QCA_DROP_VENDOR_EVENT, 57 57 }; 58 58 59 59 /* HCI_IBS transmit side sleep protocol states */ ··· 99 97 struct work_struct ws_rx_vote_off; 100 98 struct work_struct ws_tx_vote_off; 101 99 unsigned long flags; 100 + struct completion drop_ev_comp; 102 101 103 102 /* For debugging purpose */ 104 103 u64 ibs_sent_wacks; ··· 481 478 INIT_WORK(&qca->ws_tx_vote_off, qca_wq_serial_tx_clock_vote_off); 482 479 483 480 qca->hu = hu; 481 + init_completion(&qca->drop_ev_comp); 484 482 485 483 /* Assume we start with both sides asleep -- extra wakes OK */ 486 484 qca->tx_ibs_state = HCI_IBS_TX_ASLEEP; ··· 876 872 return hci_recv_frame(hdev, skb); 877 873 } 878 874 875 + static int qca_recv_event(struct hci_dev *hdev, struct sk_buff *skb) 876 + { 877 + struct hci_uart *hu = hci_get_drvdata(hdev); 878 + struct qca_data *qca = hu->priv; 879 + 880 + if (test_bit(QCA_DROP_VENDOR_EVENT, &qca->flags)) { 881 + struct hci_event_hdr *hdr = (void *)skb->data; 882 + 883 + /* For the WCN3990 the vendor command for a baudrate change 884 + * isn't sent as synchronous HCI command, because the 885 + * controller sends the corresponding vendor event with the 886 + * new baudrate. The event is received and properly decoded 887 + * after changing the baudrate of the host port. It needs to 888 + * be dropped, otherwise it can be misinterpreted as 889 + * response to a later firmware download command (also a 890 + * vendor command). 891 + */ 892 + 893 + if (hdr->evt == HCI_EV_VENDOR) 894 + complete(&qca->drop_ev_comp); 895 + 896 + kfree(skb); 897 + 898 + return 0; 899 + } 900 + 901 + return hci_recv_frame(hdev, skb); 902 + } 903 + 879 904 #define QCA_IBS_SLEEP_IND_EVENT \ 880 905 .type = HCI_IBS_SLEEP_IND, \ 881 906 .hlen = 0, \ ··· 929 896 static const struct h4_recv_pkt qca_recv_pkts[] = { 930 897 { H4_RECV_ACL, .recv = qca_recv_acl_data }, 931 898 { H4_RECV_SCO, .recv = hci_recv_frame }, 932 - { H4_RECV_EVENT, .recv = hci_recv_frame }, 899 + { H4_RECV_EVENT, .recv = qca_recv_event }, 933 900 { QCA_IBS_WAKE_IND_EVENT, .recv = qca_ibs_wake_ind }, 934 901 { QCA_IBS_WAKE_ACK_EVENT, .recv = qca_ibs_wake_ack }, 935 902 { QCA_IBS_SLEEP_IND_EVENT, .recv = qca_ibs_sleep_ind }, ··· 1124 1091 static int qca_set_speed(struct hci_uart *hu, enum qca_speed_type speed_type) 1125 1092 { 1126 1093 unsigned int speed, qca_baudrate; 1094 + struct qca_data *qca = hu->priv; 1127 1095 int ret = 0; 1128 1096 1129 1097 if (speed_type == QCA_INIT_SPEED) { ··· 1144 1110 if (qca_is_wcn399x(soc_type)) 1145 1111 hci_uart_set_flow_control(hu, true); 1146 1112 1113 + if (soc_type == QCA_WCN3990) { 1114 + reinit_completion(&qca->drop_ev_comp); 1115 + set_bit(QCA_DROP_VENDOR_EVENT, &qca->flags); 1116 + } 1117 + 1147 1118 qca_baudrate = qca_get_baudrate_value(speed); 1148 1119 bt_dev_dbg(hu->hdev, "Set UART speed to %d", speed); 1149 1120 ret = qca_set_baudrate(hu->hdev, qca_baudrate); ··· 1160 1121 error: 1161 1122 if (qca_is_wcn399x(soc_type)) 1162 1123 hci_uart_set_flow_control(hu, false); 1124 + 1125 + if (soc_type == QCA_WCN3990) { 1126 + /* Wait for the controller to send the vendor event 1127 + * for the baudrate change command. 1128 + */ 1129 + if (!wait_for_completion_timeout(&qca->drop_ev_comp, 1130 + msecs_to_jiffies(100))) { 1131 + bt_dev_err(hu->hdev, 1132 + "Failed to change controller baudrate\n"); 1133 + ret = -ETIMEDOUT; 1134 + } 1135 + 1136 + clear_bit(QCA_DROP_VENDOR_EVENT, &qca->flags); 1137 + } 1163 1138 } 1164 1139 1165 1140 return ret;