This fixes the tx timeout issue seen while running a stress test on
btnxpuart for couple of hours, such that the interval between two HCI
commands coincide with the power save timeout value of 2 seconds.
Test procedure using bash script:
<load btnxpuart.ko>
hciconfig hci0 up
//Enable Power Save feature
hcitool -i hci0 cmd 3f 23 02 00 00
while (true)
do
hciconfig hci0 leadv
sleep 2
hciconfig hci0 noleadv
sleep 2
done
Error log, after adding few more debug prints:
Bluetooth: btnxpuart_queue_skb(): 01 0A 20 01 00
Bluetooth: hci0: Set UART break: on, status=0
Bluetooth: hci0: btnxpuart_tx_wakeup() tx_work scheduled
Bluetooth: hci0: btnxpuart_tx_work() dequeue: 01 0A 20 01 00
Can't set advertise mode on hci0: Connection timed out (110)
Bluetooth: hci0: command 0x200a tx timeout
When the power save mechanism turns on UART break, and btnxpuart_tx_work()
is scheduled simultaneously, psdata->ps_state is read as PS_STATE_AWAKE,
which prevents the psdata->work from being scheduled, which is responsible
to turn OFF UART break.
This issue is fixed by adding a ps_lock mutex around UART break on/off as
well as around ps_state read/write.
btnxpuart_tx_wakeup() will now read updated ps_state value. If ps_state is
PS_STATE_SLEEP, it will first schedule psdata->work, and then it will
reschedule itself once UART break has been turned off and ps_state is
PS_STATE_AWAKE.
Tested above script for 50,000 iterations and TX timeout error was not
observed anymore.
Signed-off-by: Neeraj Sanjay Kale <[email protected]>
---
v2: Add 20msec delay after enabling UART break, cancel ps_work when ps_timer is restarted.
v3: Fix build error. Keep a common delay of 20msec for UART break enable/disable.
---
drivers/bluetooth/btnxpuart.c | 24 +++++++++++++++++++++---
1 file changed, 21 insertions(+), 3 deletions(-)
diff --git a/drivers/bluetooth/btnxpuart.c b/drivers/bluetooth/btnxpuart.c
index b7e66b7ac570..9131615b6f76 100644
--- a/drivers/bluetooth/btnxpuart.c
+++ b/drivers/bluetooth/btnxpuart.c
@@ -126,6 +126,7 @@ struct ps_data {
struct hci_dev *hdev;
struct work_struct work;
struct timer_list ps_timer;
+ struct mutex ps_lock;
};
struct wakeup_cmd_payload {
@@ -317,6 +318,9 @@ static void ps_start_timer(struct btnxpuart_dev *nxpdev)
if (psdata->cur_psmode == PS_MODE_ENABLE)
mod_timer(&psdata->ps_timer, jiffies + msecs_to_jiffies(psdata->h2c_ps_interval));
+
+ if (psdata->ps_state == PS_STATE_AWAKE && psdata->ps_cmd == PS_CMD_ENTER_PS)
+ cancel_work_sync(&psdata->work);
}
static void ps_cancel_timer(struct btnxpuart_dev *nxpdev)
@@ -337,6 +341,7 @@ static void ps_control(struct hci_dev *hdev, u8 ps_state)
!test_bit(BTNXPUART_SERDEV_OPEN, &nxpdev->tx_state))
return;
+ mutex_lock(&psdata->ps_lock);
switch (psdata->cur_h2c_wakeupmode) {
case WAKEUP_METHOD_DTR:
if (ps_state == PS_STATE_AWAKE)
@@ -350,12 +355,15 @@ static void ps_control(struct hci_dev *hdev, u8 ps_state)
status = serdev_device_break_ctl(nxpdev->serdev, 0);
else
status = serdev_device_break_ctl(nxpdev->serdev, -1);
+ msleep(20); /* Allow chip to detect UART-break and enter sleep */
bt_dev_dbg(hdev, "Set UART break: %s, status=%d",
str_on_off(ps_state == PS_STATE_SLEEP), status);
break;
}
if (!status)
psdata->ps_state = ps_state;
+ mutex_unlock(&psdata->ps_lock);
+
if (ps_state == PS_STATE_AWAKE)
btnxpuart_tx_wakeup(nxpdev);
}
@@ -391,17 +399,25 @@ static void ps_setup(struct hci_dev *hdev)
psdata->hdev = hdev;
INIT_WORK(&psdata->work, ps_work_func);
+ mutex_init(&psdata->ps_lock);
timer_setup(&psdata->ps_timer, ps_timeout_func, 0);
}
-static void ps_wakeup(struct btnxpuart_dev *nxpdev)
+static bool ps_wakeup(struct btnxpuart_dev *nxpdev)
{
struct ps_data *psdata = &nxpdev->psdata;
+ u8 ps_state;
- if (psdata->ps_state != PS_STATE_AWAKE) {
+ mutex_lock(&psdata->ps_lock);
+ ps_state = psdata->ps_state;
+ mutex_unlock(&psdata->ps_lock);
+
+ if (ps_state != PS_STATE_AWAKE) {
psdata->ps_cmd = PS_CMD_EXIT_PS;
schedule_work(&psdata->work);
+ return true;
}
+ return false;
}
static int send_ps_cmd(struct hci_dev *hdev, void *data)
@@ -1171,7 +1187,6 @@ static struct sk_buff *nxp_dequeue(void *data)
{
struct btnxpuart_dev *nxpdev = (struct btnxpuart_dev *)data;
- ps_wakeup(nxpdev);
ps_start_timer(nxpdev);
return skb_dequeue(&nxpdev->txq);
}
@@ -1186,6 +1201,9 @@ static void btnxpuart_tx_work(struct work_struct *work)
struct sk_buff *skb;
int len;
+ if (ps_wakeup(nxpdev))
+ return;
+
while ((skb = nxp_dequeue(nxpdev))) {
len = serdev_device_write_buf(serdev, skb->data, skb->len);
hdev->stat.byte_tx += len;
--
2.34.1
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=813051
---Test result---
Test Summary:
CheckPatch PASS 0.52 seconds
GitLint FAIL 0.51 seconds
SubjectPrefix PASS 0.44 seconds
BuildKernel PASS 28.18 seconds
CheckAllWarning PASS 31.07 seconds
CheckSparse PASS 36.48 seconds
CheckSmatch PASS 100.20 seconds
BuildKernel32 PASS 27.63 seconds
TestRunnerSetup PASS 438.20 seconds
TestRunner_l2cap-tester PASS 23.06 seconds
TestRunner_iso-tester PASS 45.98 seconds
TestRunner_bnep-tester PASS 8.20 seconds
TestRunner_mgmt-tester PASS 162.81 seconds
TestRunner_rfcomm-tester PASS 10.97 seconds
TestRunner_sco-tester PASS 14.49 seconds
TestRunner_ioctl-tester PASS 12.28 seconds
TestRunner_mesh-tester PASS 8.75 seconds
TestRunner_smp-tester PASS 9.68 seconds
TestRunner_userchan-tester PASS 7.26 seconds
IncrementalBuild PASS 26.22 seconds
Details
##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
[v3] Bluetooth: btnxpuart: Resolve TX timeout error in power save stress test
WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search
45: B1 Line exceeds max length (90>80): "v2: Add 20msec delay after enabling UART break, cancel ps_work when ps_timer is restarted."
46: B1 Line exceeds max length (81>80): "v3: Fix build error. Keep a common delay of 20msec for UART break enable/disable."
---
Regards,
Linux Bluetooth
Hello:
This patch was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <[email protected]>:
On Wed, 27 Dec 2023 18:59:27 +0530 you wrote:
> This fixes the tx timeout issue seen while running a stress test on
> btnxpuart for couple of hours, such that the interval between two HCI
> commands coincide with the power save timeout value of 2 seconds.
>
> Test procedure using bash script:
> <load btnxpuart.ko>
> hciconfig hci0 up
> //Enable Power Save feature
> hcitool -i hci0 cmd 3f 23 02 00 00
> while (true)
> do
> hciconfig hci0 leadv
> sleep 2
> hciconfig hci0 noleadv
> sleep 2
> done
>
> [...]
Here is the summary with links:
- [v3] Bluetooth: btnxpuart: Resolve TX timeout error in power save stress test
https://git.kernel.org/bluetooth/bluetooth-next/c/c7ee0bc8db32
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html