Initialization of the QCA WCN3990 often fails at boot time:
[ 15.205224] Bluetooth: hci0: setting up wcn3990
[ 17.341062] Bluetooth: hci0: command 0xfc00 tx timeout
[ 22.101453] ERROR: Bluetooth initialization failed
[ 25.337740] Bluetooth: hci0: Reading QCA version information failed (-110)
A short delay after sending a power-off pulse (which is done immediately
before sending a power-on pulse) fixes this.
Also move the delays to qca_send_power_pulse(), since they are directly
related with sending the pulses. Change the interface of
qca_send_power_pulse() to receive a boolean instead of a power pulse
command, this improves readability of the function with the delays
added.
Matthias Kaehlcke (3):
Bluetooth: hci_qca: Pass boolean 'on/off' to qca_send_power_pulse()
Bluetooth: hci_qca: Move boot delay to qca_send_power_pulse()
Bluetooth: hci_qca: Add delay after power-off pulse
drivers/bluetooth/hci_qca.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
--
2.21.0.rc0.258.g878e2cd30e-goog
There are only two types of power pulses 'on' or 'off', pass a boolean
instead of the power pulse 'command'.
Signed-off-by: Matthias Kaehlcke <[email protected]>
---
drivers/bluetooth/hci_qca.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index 5e03504c4e0ca..e4128774e9686 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -1004,10 +1004,11 @@ static inline void host_set_baudrate(struct hci_uart *hu, unsigned int speed)
hci_uart_set_baudrate(hu, speed);
}
-static int qca_send_power_pulse(struct hci_uart *hu, u8 cmd)
+static int qca_send_power_pulse(struct hci_uart *hu, bool on)
{
int ret;
int timeout = msecs_to_jiffies(POWER_PULSE_TRANS_TIMEOUT_MS);
+ u8 cmd = on ? QCA_WCN3990_POWERON_PULSE : QCA_WCN3990_POWEROFF_PULSE;
/* These power pulses are single byte command which are sent
* at required baudrate to wcn3990. On wcn3990, we have an external
@@ -1138,12 +1139,12 @@ static int qca_wcn3990_init(struct hci_uart *hu)
/* Forcefully enable wcn3990 to enter in to boot mode. */
host_set_baudrate(hu, 2400);
- ret = qca_send_power_pulse(hu, QCA_WCN3990_POWEROFF_PULSE);
+ ret = qca_send_power_pulse(hu, false);
if (ret)
return ret;
qca_set_speed(hu, QCA_INIT_SPEED);
- ret = qca_send_power_pulse(hu, QCA_WCN3990_POWERON_PULSE);
+ ret = qca_send_power_pulse(hu, true);
if (ret)
return ret;
@@ -1289,7 +1290,7 @@ static void qca_power_shutdown(struct hci_uart *hu)
spin_unlock_irqrestore(&qca->hci_ibs_lock, flags);
host_set_baudrate(hu, 2400);
- qca_send_power_pulse(hu, QCA_WCN3990_POWEROFF_PULSE);
+ qca_send_power_pulse(hu, false);
qca_power_setup(hu, false);
}
--
2.21.0.rc0.258.g878e2cd30e-goog
During initialization the power-on pulse is currently sent inmediately
after the prior power-off pulse. With this initialization often fails
at boot time:
[ 15.205224] Bluetooth: hci0: setting up wcn3990
[ 17.341062] Bluetooth: hci0: command 0xfc00 tx timeout
[ 22.101453] ERROR: Bluetooth initialization failed
[ 25.337740] Bluetooth: hci0: Reading QCA version information failed (-110)
After a power-off pulse wait 10ms to give the controller time to power
off.
Signed-off-by: Matthias Kaehlcke <[email protected]>
---
drivers/bluetooth/hci_qca.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index eacc108c422d0..c89c1ed87ffe9 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -1036,8 +1036,11 @@ static int qca_send_power_pulse(struct hci_uart *hu, bool on)
usleep_range(100, 200);
hci_uart_set_flow_control(hu, false);
+ /* Give to controller time to boot/shutdown */
if (on)
msleep(100);
+ else
+ msleep(10);
return 0;
}
--
2.21.0.rc0.258.g878e2cd30e-goog
After sending a power on pulse the driver has a delay of 100ms
to allow the host controller to boot. Move the delay into
qca_send_power_pulse(), since it is directly related with the
power-on pulse.
Signed-off-by: Matthias Kaehlcke <[email protected]>
---
drivers/bluetooth/hci_qca.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index e4128774e9686..eacc108c422d0 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -1036,6 +1036,9 @@ static int qca_send_power_pulse(struct hci_uart *hu, bool on)
usleep_range(100, 200);
hci_uart_set_flow_control(hu, false);
+ if (on)
+ msleep(100);
+
return 0;
}
@@ -1148,9 +1151,6 @@ static int qca_wcn3990_init(struct hci_uart *hu)
if (ret)
return ret;
- /* Wait for 100 ms for SoC to boot */
- msleep(100);
-
/* Now the device is in ready state to communicate with host.
* To sync host with device we need to reopen port.
* Without this, we will have RTS and CTS synchronization
--
2.21.0.rc0.258.g878e2cd30e-goog
On 2019-02-26 05:19, Matthias Kaehlcke wrote:
> There are only two types of power pulses 'on' or 'off', pass a boolean
> instead of the power pulse 'command'.
>
> Signed-off-by: Matthias Kaehlcke <[email protected]>
> ---
> drivers/bluetooth/hci_qca.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
> index 5e03504c4e0ca..e4128774e9686 100644
> --- a/drivers/bluetooth/hci_qca.c
> +++ b/drivers/bluetooth/hci_qca.c
> @@ -1004,10 +1004,11 @@ static inline void host_set_baudrate(struct
> hci_uart *hu, unsigned int speed)
> hci_uart_set_baudrate(hu, speed);
> }
>
> -static int qca_send_power_pulse(struct hci_uart *hu, u8 cmd)
> +static int qca_send_power_pulse(struct hci_uart *hu, bool on)
> {
> int ret;
> int timeout = msecs_to_jiffies(POWER_PULSE_TRANS_TIMEOUT_MS);
> + u8 cmd = on ? QCA_WCN3990_POWERON_PULSE : QCA_WCN3990_POWEROFF_PULSE;
>
> /* These power pulses are single byte command which are sent
> * at required baudrate to wcn3990. On wcn3990, we have an external
> @@ -1138,12 +1139,12 @@ static int qca_wcn3990_init(struct hci_uart
> *hu)
>
> /* Forcefully enable wcn3990 to enter in to boot mode. */
> host_set_baudrate(hu, 2400);
> - ret = qca_send_power_pulse(hu, QCA_WCN3990_POWEROFF_PULSE);
> + ret = qca_send_power_pulse(hu, false);
> if (ret)
> return ret;
>
> qca_set_speed(hu, QCA_INIT_SPEED);
> - ret = qca_send_power_pulse(hu, QCA_WCN3990_POWERON_PULSE);
> + ret = qca_send_power_pulse(hu, true);
> if (ret)
> return ret;
>
> @@ -1289,7 +1290,7 @@ static void qca_power_shutdown(struct hci_uart
> *hu)
> spin_unlock_irqrestore(&qca->hci_ibs_lock, flags);
>
> host_set_baudrate(hu, 2400);
> - qca_send_power_pulse(hu, QCA_WCN3990_POWEROFF_PULSE);
> + qca_send_power_pulse(hu, false);
> qca_power_setup(hu, false);
> }
Reviewed-by: Balakrishna Godavarthi <[email protected]>
--
Regards
Balakrishna.
On 2019-02-26 05:19, Matthias Kaehlcke wrote:
> After sending a power on pulse the driver has a delay of 100ms
> to allow the host controller to boot. Move the delay into
> qca_send_power_pulse(), since it is directly related with the
> power-on pulse.
>
> Signed-off-by: Matthias Kaehlcke <[email protected]>
> ---
> drivers/bluetooth/hci_qca.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
> index e4128774e9686..eacc108c422d0 100644
> --- a/drivers/bluetooth/hci_qca.c
> +++ b/drivers/bluetooth/hci_qca.c
> @@ -1036,6 +1036,9 @@ static int qca_send_power_pulse(struct hci_uart
> *hu, bool on)
> usleep_range(100, 200);
> hci_uart_set_flow_control(hu, false);
>
> + if (on)
> + msleep(100);
> +
> return 0;
> }
>
> @@ -1148,9 +1151,6 @@ static int qca_wcn3990_init(struct hci_uart *hu)
> if (ret)
> return ret;
>
> - /* Wait for 100 ms for SoC to boot */
> - msleep(100);
> -
> /* Now the device is in ready state to communicate with host.
> * To sync host with device we need to reopen port.
> * Without this, we will have RTS and CTS synchronization
Reviewed-by: Balakrishna Godavarthi <[email protected]>
--
Regards
Balakrishna.
On 2019-02-26 05:19, Matthias Kaehlcke wrote:
> During initialization the power-on pulse is currently sent inmediately
> after the prior power-off pulse. With this initialization often fails
> at boot time:
>
> [ 15.205224] Bluetooth: hci0: setting up wcn3990
> [ 17.341062] Bluetooth: hci0: command 0xfc00 tx timeout
> [ 22.101453] ERROR: Bluetooth initialization failed
> [ 25.337740] Bluetooth: hci0: Reading QCA version information failed
> (-110)
>
> After a power-off pulse wait 10ms to give the controller time to power
> off.
>
> Signed-off-by: Matthias Kaehlcke <[email protected]>
> ---
> drivers/bluetooth/hci_qca.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
> index eacc108c422d0..c89c1ed87ffe9 100644
> --- a/drivers/bluetooth/hci_qca.c
> +++ b/drivers/bluetooth/hci_qca.c
> @@ -1036,8 +1036,11 @@ static int qca_send_power_pulse(struct hci_uart
> *hu, bool on)
> usleep_range(100, 200);
> hci_uart_set_flow_control(hu, false);
>
> + /* Give to controller time to boot/shutdown */
> if (on)
> msleep(100);
> + else
> + msleep(10);
>
> return 0;
> }
Reviewed-by: Balakrishna Godavarthi <[email protected]>
--
Regards
Balakrishna.
hi Matthias,
On 2019-02-26 17:48, Balakrishna Godavarthi wrote:
> On 2019-02-26 05:19, Matthias Kaehlcke wrote:
>> After sending a power on pulse the driver has a delay of 100ms
>> to allow the host controller to boot. Move the delay into
>> qca_send_power_pulse(), since it is directly related with the
>> power-on pulse.
>>
>> Signed-off-by: Matthias Kaehlcke <[email protected]>
>> ---
>> drivers/bluetooth/hci_qca.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
>> index e4128774e9686..eacc108c422d0 100644
>> --- a/drivers/bluetooth/hci_qca.c
>> +++ b/drivers/bluetooth/hci_qca.c
>> @@ -1036,6 +1036,9 @@ static int qca_send_power_pulse(struct hci_uart
>> *hu, bool on)
>> usleep_range(100, 200);
[Bala] : i still doubt do we require this delay.
>> hci_uart_set_flow_control(hu, false);
>>
>> + if (on)
>> + msleep(100);
>> +
>> return 0;
>> }
>>
>> @@ -1148,9 +1151,6 @@ static int qca_wcn3990_init(struct hci_uart *hu)
>> if (ret)
>> return ret;
>>
>> - /* Wait for 100 ms for SoC to boot */
>> - msleep(100);
>> -
>> /* Now the device is in ready state to communicate with host.
>> * To sync host with device we need to reopen port.
>> * Without this, we will have RTS and CTS synchronization
>
> Reviewed-by: Balakrishna Godavarthi <[email protected]>
--
Regards
Balakrishna.
Hi Balakrishna,
Thanks for the reviews!
On Tue, Feb 26, 2019 at 05:54:24PM +0530, Balakrishna Godavarthi wrote:
> hi Matthias,
>
> On 2019-02-26 17:48, Balakrishna Godavarthi wrote:
> > On 2019-02-26 05:19, Matthias Kaehlcke wrote:
> > > After sending a power on pulse the driver has a delay of 100ms
> > > to allow the host controller to boot. Move the delay into
> > > qca_send_power_pulse(), since it is directly related with the
> > > power-on pulse.
> > >
> > > Signed-off-by: Matthias Kaehlcke <[email protected]>
> > > ---
> > > drivers/bluetooth/hci_qca.c | 6 +++---
> > > 1 file changed, 3 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
> > > index e4128774e9686..eacc108c422d0 100644
> > > --- a/drivers/bluetooth/hci_qca.c
> > > +++ b/drivers/bluetooth/hci_qca.c
> > > @@ -1036,6 +1036,9 @@ static int qca_send_power_pulse(struct hci_uart
> > > *hu, bool on)
> > > usleep_range(100, 200);
>
> [Bala] : i still doubt do we require this delay.
I don't know if it was necessary (or at least helped in the power-off
case) without this series, but my testing suggests it is not needed
with the delay after the power off pulse ("[3/3] Bluetooth: hci_qca:
Add delay after power-off pulse").
I'll send a new version without the delay.
Thanks
Matthias