On Tue, Feb 26, 2019 at 12:08:48PM -0800, Matthias Kaehlcke wrote:
> The current 300ms delay after a baudrate change is extremely long.
> For WCM3990 it is sufficient to wait 10ms after the baudrate change
> request has been sent over the wire.
>
> Signed-off-by: Matthias Kaehlcke <[email protected]>
> ---
> drivers/bluetooth/hci_qca.c | 28 +++++++++++++++++++++-------
> 1 file changed, 21 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
> index 703e099515f24..fd018cc5605c6 100644
> --- a/drivers/bluetooth/hci_qca.c
> +++ b/drivers/bluetooth/hci_qca.c
> @@ -59,8 +59,7 @@
>
> #define IBS_WAKE_RETRANS_TIMEOUT_MS 100
> #define IBS_TX_IDLE_TIMEOUT_MS 2000
> -#define BAUDRATE_SETTLE_TIMEOUT_MS 300
> -#define POWER_PULSE_TRANS_TIMEOUT_MS 100
> +#define CMD_TRANS_TIMEOUT_MS 100
>
> /* susclk rate */
> #define SUSCLK_RATE_32KHZ 32768
> @@ -964,6 +963,7 @@ static int qca_set_baudrate(struct hci_dev *hdev, uint8_t
> baudrate)
> {
> struct hci_uart *hu = hci_get_drvdata(hdev);
> struct qca_data *qca = hu->priv;
> + struct qca_serdev *qcadev;
> struct sk_buff *skb;
> u8 cmd[] = { 0x01, 0x48, 0xFC, 0x01, 0x00 };
>
> @@ -985,11 +985,25 @@ static int qca_set_baudrate(struct hci_dev *hdev,
> uint8_t baudrate)
> skb_queue_tail(&qca->txq, skb);
> hci_uart_tx_wakeup(hu);
>
> - /* wait 300ms to change new baudrate on controller side
> - * controller will come back after they receive this HCI command
> - * then host can communicate with new baudrate to controller
> + qcadev = serdev_device_get_drvdata(hu->serdev);
> +
> + /* Wait for the baudrate change command to be sent and
> + * processed by the controller.
> */
> - msleep(BAUDRATE_SETTLE_TIMEOUT_MS);
> + if (qcadev->btsoc_type == QCA_WCN3990) {
> + while (!skb_queue_empty(&qca->txq))
> + usleep_range(100, 200);
> +
> + serdev_device_wait_until_sent(hu->serdev,
> + msecs_to_jiffies(CMD_TRANS_TIMEOUT_MS));
note: the above could also be in the common path, personally I don't
have a strong preference.
> +
> + /* Make sure there is enough time for the BT
> + * controller to process the baudrate change
> + */
> + msleep(10);
> + } else {
> + msleep(300);
I wonder if the non-wcn3990 delay could be reduced too, 300ms seems an
eternity, however I don't have hardware to validate such a change.
That raises a question: how many different chips are actually
supported by this driver? In earlier reviews we were frequently
talking about the ROME family and I assumed it must be a larger
number, however there are only compatible strings for the qca6174 and
the wcn3990. If there is only one other controller (or a low number
of very similar ones that use the same compatible string?) maybe
Qualcomm could help with testing?
Thanks
Matthias
