* [PATCH 0/2] Reduce delay after sending baudrate request for WCN3990
@ 2019-02-26 20:08 Matthias Kaehlcke
2019-02-26 20:08 ` [PATCH 1/2] hci_qca: Use msleep() instead of open coding it Matthias Kaehlcke
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Matthias Kaehlcke @ 2019-02-26 20:08 UTC (permalink / raw)
To: Marcel Holtmann, Johan Hedberg
Cc: linux-bluetooth, linux-kernel, Balakrishna Godavarthi, Hemantg,
Matthias Kaehlcke
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.
Also use msleep() instead of a set_current_state() / schedule_timeout()
combo.
Matthias Kaehlcke (2):
hci_qca: Use msleep() instead of open coding it
hci_qca: Reduce delay after sending baudrate request for WCN3990
drivers/bluetooth/hci_qca.c | 30 +++++++++++++++++++++---------
1 file changed, 21 insertions(+), 9 deletions(-)
--
2.21.0.rc2.261.ga7da99ff1b-goog
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 1/2] hci_qca: Use msleep() instead of open coding it 2019-02-26 20:08 [PATCH 0/2] Reduce delay after sending baudrate request for WCN3990 Matthias Kaehlcke @ 2019-02-26 20:08 ` Matthias Kaehlcke 2019-02-26 20:08 ` [PATCH 2/2] hci_qca: Reduce delay after sending baudrate request for WCN3990 Matthias Kaehlcke 2019-02-27 7:48 ` [PATCH 0/2] " Marcel Holtmann 2 siblings, 0 replies; 6+ messages in thread From: Matthias Kaehlcke @ 2019-02-26 20:08 UTC (permalink / raw) To: Marcel Holtmann, Johan Hedberg Cc: linux-bluetooth, linux-kernel, Balakrishna Godavarthi, Hemantg, Matthias Kaehlcke Call msleep() in qca_set_baudrate() instead of reimplementing it. Signed-off-by: Matthias Kaehlcke <mka@chromium.org> --- drivers/bluetooth/hci_qca.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c index 26efc2ef98d9a..703e099515f24 100644 --- a/drivers/bluetooth/hci_qca.c +++ b/drivers/bluetooth/hci_qca.c @@ -989,9 +989,7 @@ static int qca_set_baudrate(struct hci_dev *hdev, uint8_t baudrate) * controller will come back after they receive this HCI command * then host can communicate with new baudrate to controller */ - set_current_state(TASK_UNINTERRUPTIBLE); - schedule_timeout(msecs_to_jiffies(BAUDRATE_SETTLE_TIMEOUT_MS)); - set_current_state(TASK_RUNNING); + msleep(BAUDRATE_SETTLE_TIMEOUT_MS); return 0; } -- 2.21.0.rc2.261.ga7da99ff1b-goog ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] hci_qca: Reduce delay after sending baudrate request for WCN3990 2019-02-26 20:08 [PATCH 0/2] Reduce delay after sending baudrate request for WCN3990 Matthias Kaehlcke 2019-02-26 20:08 ` [PATCH 1/2] hci_qca: Use msleep() instead of open coding it Matthias Kaehlcke @ 2019-02-26 20:08 ` Matthias Kaehlcke 2019-02-26 20:20 ` Matthias Kaehlcke 2019-02-27 7:48 ` [PATCH 0/2] " Marcel Holtmann 2 siblings, 1 reply; 6+ messages in thread From: Matthias Kaehlcke @ 2019-02-26 20:08 UTC (permalink / raw) To: Marcel Holtmann, Johan Hedberg Cc: linux-bluetooth, linux-kernel, Balakrishna Godavarthi, Hemantg, Matthias Kaehlcke 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 <mka@chromium.org> --- 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)); + + /* Make sure there is enough time for the BT + * controller to process the baudrate change + */ + msleep(10); + } else { + msleep(300); + } return 0; } @@ -1005,7 +1019,7 @@ static inline void host_set_baudrate(struct hci_uart *hu, unsigned int speed) static int qca_send_power_pulse(struct hci_uart *hu, u8 cmd) { int ret; - int timeout = msecs_to_jiffies(POWER_PULSE_TRANS_TIMEOUT_MS); + int timeout = msecs_to_jiffies(CMD_TRANS_TIMEOUT_MS); /* These power pulses are single byte command which are sent * at required baudrate to wcn3990. On wcn3990, we have an external -- 2.21.0.rc2.261.ga7da99ff1b-goog ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] hci_qca: Reduce delay after sending baudrate request for WCN3990 2019-02-26 20:08 ` [PATCH 2/2] hci_qca: Reduce delay after sending baudrate request for WCN3990 Matthias Kaehlcke @ 2019-02-26 20:20 ` Matthias Kaehlcke 0 siblings, 0 replies; 6+ messages in thread From: Matthias Kaehlcke @ 2019-02-26 20:20 UTC (permalink / raw) To: Marcel Holtmann, Johan Hedberg Cc: linux-bluetooth, linux-kernel, Balakrishna Godavarthi, Hemantg 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 <mka@chromium.org> > --- > 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 0/2] Reduce delay after sending baudrate request for WCN3990 2019-02-26 20:08 [PATCH 0/2] Reduce delay after sending baudrate request for WCN3990 Matthias Kaehlcke 2019-02-26 20:08 ` [PATCH 1/2] hci_qca: Use msleep() instead of open coding it Matthias Kaehlcke 2019-02-26 20:08 ` [PATCH 2/2] hci_qca: Reduce delay after sending baudrate request for WCN3990 Matthias Kaehlcke @ 2019-02-27 7:48 ` Marcel Holtmann 2019-02-27 20:35 ` Matthias Kaehlcke 2 siblings, 1 reply; 6+ messages in thread From: Marcel Holtmann @ 2019-02-27 7:48 UTC (permalink / raw) To: Matthias Kaehlcke Cc: Johan Hedberg, linux-bluetooth, linux-kernel, Balakrishna Godavarthi, Hemantg Hi Matthias, > 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. > > Also use msleep() instead of a set_current_state() / schedule_timeout() > combo. > > Matthias Kaehlcke (2): > hci_qca: Use msleep() instead of open coding it > hci_qca: Reduce delay after sending baudrate request for WCN3990 > > drivers/bluetooth/hci_qca.c | 30 +++++++++++++++++++++--------- > 1 file changed, 21 insertions(+), 9 deletions(-) patch 1/2 has been applied to bluetooth-next tree. The patch 2/2 fails to apply. Regards Marcel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 0/2] Reduce delay after sending baudrate request for WCN3990 2019-02-27 7:48 ` [PATCH 0/2] " Marcel Holtmann @ 2019-02-27 20:35 ` Matthias Kaehlcke 0 siblings, 0 replies; 6+ messages in thread From: Matthias Kaehlcke @ 2019-02-27 20:35 UTC (permalink / raw) To: Marcel Holtmann Cc: Johan Hedberg, linux-bluetooth, linux-kernel, Balakrishna Godavarthi, Hemantg Hi Marcel, On Wed, Feb 27, 2019 at 08:48:31AM +0100, Marcel Holtmann wrote: > Hi Matthias, > > > 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. > > > > Also use msleep() instead of a set_current_state() / schedule_timeout() > > combo. > > > > Matthias Kaehlcke (2): > > hci_qca: Use msleep() instead of open coding it > > hci_qca: Reduce delay after sending baudrate request for WCN3990 > > > > drivers/bluetooth/hci_qca.c | 30 +++++++++++++++++++++--------- > > 1 file changed, 21 insertions(+), 9 deletions(-) > > patch 1/2 has been applied to bluetooth-next tree. Thanks! > The patch 2/2 fails to apply. Right, the series "Bluetooth: hci_qca: Add delay after power-off pulse" that was just applied touches the same code. I'll send a rebased version soon. Cheers Matthias ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-02-27 20:35 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-02-26 20:08 [PATCH 0/2] Reduce delay after sending baudrate request for WCN3990 Matthias Kaehlcke 2019-02-26 20:08 ` [PATCH 1/2] hci_qca: Use msleep() instead of open coding it Matthias Kaehlcke 2019-02-26 20:08 ` [PATCH 2/2] hci_qca: Reduce delay after sending baudrate request for WCN3990 Matthias Kaehlcke 2019-02-26 20:20 ` Matthias Kaehlcke 2019-02-27 7:48 ` [PATCH 0/2] " Marcel Holtmann 2019-02-27 20:35 ` Matthias Kaehlcke
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).