* [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).