[PATCH v1] Bluetooth: btnxpuart: Resolve TX timeout error in power save stress test

[PATCH v1] Bluetooth: btnxpuart: Resolve TX timeout error in power save stress test

[Neeraj Sanjay Kale]

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:
[ 2206.497227] Bluetooth: btnxpuart_queue_skb(): 01 0A 20 01 00
[ 2206.498239] Bluetooth: hci0: Set UART break: on, status=0
[ 2206.503283] Bluetooth: hci0: btnxpuart_tx_wakeup() tx_work scheduled
[ 2206.503299] Bluetooth: hci0: btnxpuart_tx_work() dequeue: 01 0A 20 01 00
Can't set advertise mode on hci0: Connection timed out (110)
[ 2208.514238] 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 <neeraj.sanjaykale@nxp.com>
---
 drivers/bluetooth/btnxpuart.c | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/drivers/bluetooth/btnxpuart.c b/drivers/bluetooth/btnxpuart.c
index b7e66b7ac570..a68d10771c99 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 {
@@ -337,6 +338,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)
@@ -356,6 +358,8 @@ static void ps_control(struct hci_dev *hdev, u8 ps_state)
 	}
 	if (!status)
 		psdata->ps_state = ps_state;
+	mutex_unlock(&psdata->ps_lock);
+
 	if (ps_state == PS_STATE_AWAKE)
 		btnxpuart_tx_wakeup(nxpdev);
 }
@@ -391,17 +395,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;
+
+	mutex_lock(&psdata->ps_lock);
+	ps_state = psdata->ps_state;
+	mutex_unlock(&psdata->ps_lock);
 
-	if (psdata->ps_state != PS_STATE_AWAKE) {
+	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 +1183,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 +1197,11 @@ static void btnxpuart_tx_work(struct work_struct *work)
 	struct sk_buff *skb;
 	int len;
 
+	if (ps_wakeup(nxpdev)) {
+		schedule_work(&nxpdev->tx_work);
+		return;
+	}
+
 	while ((skb = nxp_dequeue(nxpdev))) {
 		len = serdev_device_write_buf(serdev, skb->data, skb->len);
 		hdev->stat.byte_tx += len;
-- 
2.34.1

RE: [v1] Bluetooth: btnxpuart: Resolve TX timeout error in power save stress test

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1427 bytes --]

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=797485

---Test result---

Test Summary:
CheckPatch                    PASS      0.78 seconds
GitLint                       PASS      0.30 seconds
SubjectPrefix                 PASS      0.09 seconds
BuildKernel                   PASS      42.35 seconds
CheckAllWarning               PASS      46.75 seconds
CheckSparse                   PASS      53.30 seconds
CheckSmatch                   PASS      141.45 seconds
BuildKernel32                 PASS      41.06 seconds
TestRunnerSetup               PASS      635.14 seconds
TestRunner_l2cap-tester       PASS      36.96 seconds
TestRunner_iso-tester         PASS      68.61 seconds
TestRunner_bnep-tester        PASS      12.92 seconds
TestRunner_mgmt-tester        PASS      260.39 seconds
TestRunner_rfcomm-tester      PASS      19.59 seconds
TestRunner_sco-tester         PASS      22.77 seconds
TestRunner_ioctl-tester       PASS      22.06 seconds
TestRunner_mesh-tester        PASS      16.30 seconds
TestRunner_smp-tester         PASS      20.28 seconds
TestRunner_userchan-tester    PASS      13.56 seconds
IncrementalBuild              PASS      38.69 seconds



---
Regards,
Linux Bluetooth

RE: [v1] Bluetooth: btnxpuart: Resolve TX timeout error in power save stress test

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1422 bytes --]

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=810985

---Test result---

Test Summary:
CheckPatch                    PASS      0.66 seconds
GitLint                       PASS      0.34 seconds
SubjectPrefix                 PASS      0.13 seconds
BuildKernel                   PASS      27.91 seconds
CheckAllWarning               PASS      30.83 seconds
CheckSparse                   PASS      36.35 seconds
CheckSmatch                   PASS      99.39 seconds
BuildKernel32                 PASS      27.18 seconds
TestRunnerSetup               PASS      430.21 seconds
TestRunner_l2cap-tester       PASS      23.55 seconds
TestRunner_iso-tester         PASS      44.31 seconds
TestRunner_bnep-tester        PASS      6.84 seconds
TestRunner_mgmt-tester        PASS      161.68 seconds
TestRunner_rfcomm-tester      PASS      10.83 seconds
TestRunner_sco-tester         PASS      14.42 seconds
TestRunner_ioctl-tester       PASS      12.24 seconds
TestRunner_mesh-tester        PASS      8.92 seconds
TestRunner_smp-tester         PASS      9.73 seconds
TestRunner_userchan-tester    PASS      7.15 seconds
IncrementalBuild              PASS      26.19 seconds



---
Regards,
Linux Bluetooth