[PATCH v1] Bluetooth: btnxpuart: Improve inband Independent Reset handling

[PATCH v1] Bluetooth: btnxpuart: Improve inband Independent Reset handling

[Neeraj Sanjay Kale]

This improves the inband IR command handling for NXP BT chipsets.
When the IR vendor command is received, the driver injects a HW
error event, which causes a reset sequence in hci_error_reset().
The vendor IR command is sent to the controller while hci dev
is been closed, and FW is re-downloaded when nxp_setup() is
called during hci_dev_do_open().
The HCI_SETUP flag is set in nxp_hw_err() to make sure that
nxp_setup() is been called during hci_dev_do_open().

This also makes the nxp_setup() and power save functions more
generic.

Signed-off-by: Neeraj Sanjay Kale <neeraj.sanjaykale@nxp.com>
---
 drivers/bluetooth/btnxpuart.c | 172 +++++++++++++++++++---------------
 1 file changed, 96 insertions(+), 76 deletions(-)

diff --git a/drivers/bluetooth/btnxpuart.c b/drivers/bluetooth/btnxpuart.c
index ee6f6c872a34..de92fc0f08ad 100644
--- a/drivers/bluetooth/btnxpuart.c
+++ b/drivers/bluetooth/btnxpuart.c
@@ -28,6 +28,10 @@
 #define BTNXPUART_FW_DOWNLOADING	2
 #define BTNXPUART_CHECK_BOOT_SIGNATURE	3
 #define BTNXPUART_SERDEV_OPEN		4
+#define BTNXPUART_IR_IN_PROGRESS	5
+
+/* NXP HW err codes */
+#define BTNXPUART_IR_HW_ERR		0xb0
 
 #define FIRMWARE_W8987		"nxp/uartuart8987_bt.bin"
 #define FIRMWARE_W8997		"nxp/uartuart8997_bt_v4.bin"
@@ -375,39 +379,13 @@ static void ps_timeout_func(struct timer_list *t)
 	}
 }
 
-static int ps_init_work(struct hci_dev *hdev)
+static void ps_setup(struct hci_dev *hdev)
 {
 	struct btnxpuart_dev *nxpdev = hci_get_drvdata(hdev);
 	struct ps_data *psdata = &nxpdev->psdata;
 
-	psdata->h2c_ps_interval = PS_DEFAULT_TIMEOUT_PERIOD_MS;
-	psdata->ps_state = PS_STATE_AWAKE;
-	psdata->target_ps_mode = DEFAULT_PS_MODE;
 	psdata->hdev = hdev;
-	psdata->c2h_wakeupmode = BT_HOST_WAKEUP_METHOD_NONE;
-	psdata->c2h_wakeup_gpio = 0xff;
-
-	switch (DEFAULT_H2C_WAKEUP_MODE) {
-	case WAKEUP_METHOD_DTR:
-		psdata->h2c_wakeupmode = WAKEUP_METHOD_DTR;
-		break;
-	case WAKEUP_METHOD_BREAK:
-	default:
-		psdata->h2c_wakeupmode = WAKEUP_METHOD_BREAK;
-		break;
-	}
-	psdata->cur_psmode = PS_MODE_DISABLE;
-	psdata->cur_h2c_wakeupmode = WAKEUP_METHOD_INVALID;
 	INIT_WORK(&psdata->work, ps_work_func);
-
-	return 0;
-}
-
-static void ps_init_timer(struct hci_dev *hdev)
-{
-	struct btnxpuart_dev *nxpdev = hci_get_drvdata(hdev);
-	struct ps_data *psdata = &nxpdev->psdata;
-
 	timer_setup(&psdata->ps_timer, ps_timeout_func, 0);
 }
 
@@ -510,19 +488,31 @@ static void ps_init(struct hci_dev *hdev)
 	serdev_device_set_tiocm(nxpdev->serdev, TIOCM_RTS, 0);
 	usleep_range(5000, 10000);
 
-	switch (psdata->h2c_wakeupmode) {
+	psdata->ps_state = PS_STATE_AWAKE;
+	psdata->c2h_wakeupmode = BT_HOST_WAKEUP_METHOD_NONE;
+	psdata->c2h_wakeup_gpio = 0xff;
+
+	psdata->cur_h2c_wakeupmode = WAKEUP_METHOD_INVALID;
+	psdata->h2c_ps_interval = PS_DEFAULT_TIMEOUT_PERIOD_MS;
+	switch (DEFAULT_H2C_WAKEUP_MODE) {
 	case WAKEUP_METHOD_DTR:
+		psdata->h2c_wakeupmode = WAKEUP_METHOD_DTR;
 		serdev_device_set_tiocm(nxpdev->serdev, 0, TIOCM_DTR);
 		serdev_device_set_tiocm(nxpdev->serdev, TIOCM_DTR, 0);
 		break;
 	case WAKEUP_METHOD_BREAK:
 	default:
+		psdata->h2c_wakeupmode = WAKEUP_METHOD_BREAK;
 		serdev_device_break_ctl(nxpdev->serdev, -1);
 		usleep_range(5000, 10000);
 		serdev_device_break_ctl(nxpdev->serdev, 0);
 		usleep_range(5000, 10000);
 		break;
 	}
+
+	psdata->cur_psmode = PS_MODE_DISABLE;
+	psdata->target_ps_mode = DEFAULT_PS_MODE;
+
 	if (psdata->cur_h2c_wakeupmode != psdata->h2c_wakeupmode)
 		hci_cmd_sync_queue(hdev, send_wakeup_method_cmd, NULL, NULL);
 	if (psdata->cur_psmode != psdata->target_ps_mode)
@@ -702,7 +692,7 @@ static int nxp_recv_chip_ver_v1(struct hci_dev *hdev, struct sk_buff *skb)
 		goto free_skb;
 
 	chip_id = le16_to_cpu(req->chip_id ^ req->chip_id_comp);
-	if (chip_id == 0xffff) {
+	if (chip_id == 0xffff && nxpdev->fw_dnld_v1_offset) {
 		nxpdev->fw_dnld_v1_offset = 0;
 		nxpdev->fw_v1_sent_bytes = 0;
 		nxpdev->fw_v1_expected_len = HDR_LEN;
@@ -969,45 +959,13 @@ static int nxp_set_baudrate_cmd(struct hci_dev *hdev, void *data)
 	return 0;
 }
 
-static int nxp_set_ind_reset(struct hci_dev *hdev, void *data)
-{
-	struct btnxpuart_dev *nxpdev = hci_get_drvdata(hdev);
-	struct sk_buff *skb;
-	u8 *status;
-	u8 pcmd = 0;
-	int err = 0;
-
-	skb = nxp_drv_send_cmd(hdev, HCI_NXP_IND_RESET, 1, &pcmd);
-	if (IS_ERR(skb))
-		return PTR_ERR(skb);
-
-	status = skb_pull_data(skb, 1);
-	if (!status || *status)
-		goto free_skb;
-
-	set_bit(BTNXPUART_FW_DOWNLOADING, &nxpdev->tx_state);
-	err = nxp_download_firmware(hdev);
-	if (err < 0)
-		goto free_skb;
-	serdev_device_set_baudrate(nxpdev->serdev, nxpdev->fw_init_baudrate);
-	nxpdev->current_baudrate = nxpdev->fw_init_baudrate;
-	if (nxpdev->current_baudrate != HCI_NXP_SEC_BAUDRATE) {
-		nxpdev->new_baudrate = HCI_NXP_SEC_BAUDRATE;
-		nxp_set_baudrate_cmd(hdev, NULL);
-	}
-	hci_cmd_sync_queue(hdev, send_wakeup_method_cmd, NULL, NULL);
-	hci_cmd_sync_queue(hdev, send_ps_cmd, NULL, NULL);
-
-free_skb:
-	kfree_skb(skb);
-	return err;
-}
-
-/* NXP protocol */
 static int nxp_check_boot_sign(struct btnxpuart_dev *nxpdev)
 {
 	serdev_device_set_baudrate(nxpdev->serdev, HCI_NXP_PRI_BAUDRATE);
-	serdev_device_set_flow_control(nxpdev->serdev, true);
+	if (test_bit(BTNXPUART_IR_IN_PROGRESS, &nxpdev->tx_state))
+		serdev_device_set_flow_control(nxpdev->serdev, false);
+	else
+		serdev_device_set_flow_control(nxpdev->serdev, true);
 	set_bit(BTNXPUART_CHECK_BOOT_SIGNATURE, &nxpdev->tx_state);
 
 	return wait_event_interruptible_timeout(nxpdev->check_boot_sign_wait_q,
@@ -1016,15 +974,29 @@ static int nxp_check_boot_sign(struct btnxpuart_dev *nxpdev)
 					       msecs_to_jiffies(1000));
 }
 
+static int nxp_set_ind_reset(struct hci_dev *hdev, void *data)
+{
+	static const u8 ir_hw_err[] = { HCI_EV_HARDWARE_ERROR,
+					0x01, BTNXPUART_IR_HW_ERR };
+	struct sk_buff *skb;
+
+	skb = bt_skb_alloc(3, GFP_ATOMIC);
+	if (!skb)
+		return -ENOMEM;
+
+	hci_skb_pkt_type(skb) = HCI_EVENT_PKT;
+	skb_put_data(skb, ir_hw_err, 3);
+
+	/* Inject Hardware Error to upper stack */
+	return hci_recv_frame(hdev, skb);
+}
+
+/* NXP protocol */
 static int nxp_setup(struct hci_dev *hdev)
 {
 	struct btnxpuart_dev *nxpdev = hci_get_drvdata(hdev);
 	int err = 0;
 
-	set_bit(BTNXPUART_FW_DOWNLOADING, &nxpdev->tx_state);
-	init_waitqueue_head(&nxpdev->fw_dnld_done_wait_q);
-	init_waitqueue_head(&nxpdev->check_boot_sign_wait_q);
-
 	if (nxp_check_boot_sign(nxpdev)) {
 		bt_dev_dbg(hdev, "Need FW Download.");
 		err = nxp_download_firmware(hdev);
@@ -1035,10 +1007,6 @@ static int nxp_setup(struct hci_dev *hdev)
 		clear_bit(BTNXPUART_FW_DOWNLOADING, &nxpdev->tx_state);
 	}
 
-	device_property_read_u32(&nxpdev->serdev->dev, "fw-init-baudrate",
-				 &nxpdev->fw_init_baudrate);
-	if (!nxpdev->fw_init_baudrate)
-		nxpdev->fw_init_baudrate = FW_INIT_BAUDRATE;
 	serdev_device_set_baudrate(nxpdev->serdev, nxpdev->fw_init_baudrate);
 	nxpdev->current_baudrate = nxpdev->fw_init_baudrate;
 
@@ -1049,6 +1017,46 @@ static int nxp_setup(struct hci_dev *hdev)
 
 	ps_init(hdev);
 
+	if (test_and_clear_bit(BTNXPUART_IR_IN_PROGRESS, &nxpdev->tx_state))
+		hci_dev_clear_flag(hdev, HCI_SETUP);
+
+	return 0;
+}
+
+static void nxp_hw_err(struct hci_dev *hdev, u8 code)
+{
+	struct btnxpuart_dev *nxpdev = hci_get_drvdata(hdev);
+
+	switch (code) {
+	case BTNXPUART_IR_HW_ERR:
+		set_bit(BTNXPUART_IR_IN_PROGRESS, &nxpdev->tx_state);
+		hci_dev_set_flag(hdev, HCI_SETUP);
+		break;
+	default:
+		break;
+	}
+}
+
+static int nxp_shutdown(struct hci_dev *hdev)
+{
+	struct btnxpuart_dev *nxpdev = hci_get_drvdata(hdev);
+	struct sk_buff *skb;
+	u8 *status;
+	u8 pcmd = 0;
+
+	if (test_bit(BTNXPUART_IR_IN_PROGRESS, &nxpdev->tx_state)) {
+		skb = nxp_drv_send_cmd(hdev, HCI_NXP_IND_RESET, 1, &pcmd);
+		if (IS_ERR(skb))
+			return PTR_ERR(skb);
+
+		status = skb_pull_data(skb, 1);
+		if (status) {
+			serdev_device_set_flow_control(nxpdev->serdev, false);
+			set_bit(BTNXPUART_FW_DOWNLOADING, &nxpdev->tx_state);
+		}
+		kfree_skb(skb);
+	}
+
 	return 0;
 }
 
@@ -1256,7 +1264,8 @@ static int btnxpuart_receive_buf(struct serdev_device *serdev, const u8 *data,
 		nxpdev->rx_skb = NULL;
 		return err;
 	}
-	nxpdev->hdev->stat.byte_rx += count;
+	if (!is_fw_downloading(nxpdev))
+		nxpdev->hdev->stat.byte_rx += count;
 	return count;
 }
 
@@ -1289,6 +1298,16 @@ static int nxp_serdev_probe(struct serdev_device *serdev)
 	INIT_WORK(&nxpdev->tx_work, btnxpuart_tx_work);
 	skb_queue_head_init(&nxpdev->txq);
 
+	init_waitqueue_head(&nxpdev->fw_dnld_done_wait_q);
+	init_waitqueue_head(&nxpdev->check_boot_sign_wait_q);
+
+	device_property_read_u32(&nxpdev->serdev->dev, "fw-init-baudrate",
+				 &nxpdev->fw_init_baudrate);
+	if (!nxpdev->fw_init_baudrate)
+		nxpdev->fw_init_baudrate = FW_INIT_BAUDRATE;
+
+	set_bit(BTNXPUART_FW_DOWNLOADING, &nxpdev->tx_state);
+
 	crc8_populate_msb(crc8_table, POLYNOMIAL8);
 
 	/* Initialize and register HCI device */
@@ -1309,6 +1328,8 @@ static int nxp_serdev_probe(struct serdev_device *serdev)
 	hdev->flush = btnxpuart_flush;
 	hdev->setup = nxp_setup;
 	hdev->send  = nxp_enqueue;
+	hdev->hw_error = nxp_hw_err;
+	hdev->shutdown = nxp_shutdown;
 	SET_HCIDEV_DEV(hdev, &serdev->dev);
 
 	if (hci_register_dev(hdev) < 0) {
@@ -1317,8 +1338,7 @@ static int nxp_serdev_probe(struct serdev_device *serdev)
 		return -ENODEV;
 	}
 
-	ps_init_work(hdev);
-	ps_init_timer(hdev);
+	ps_setup(hdev);
 
 	return 0;
 }
-- 
2.34.1

RE: [v1] Bluetooth: btnxpuart: Improve inband Independent Reset handling

[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=774991

---Test result---

Test Summary:
CheckPatch                    PASS      0.76 seconds
GitLint                       PASS      0.29 seconds
SubjectPrefix                 PASS      0.10 seconds
BuildKernel                   PASS      32.88 seconds
CheckAllWarning               PASS      35.82 seconds
CheckSparse                   PASS      40.56 seconds
CheckSmatch                   PASS      113.79 seconds
BuildKernel32                 PASS      32.16 seconds
TestRunnerSetup               PASS      480.11 seconds
TestRunner_l2cap-tester       PASS      22.35 seconds
TestRunner_iso-tester         PASS      44.96 seconds
TestRunner_bnep-tester        PASS      10.17 seconds
TestRunner_mgmt-tester        PASS      213.02 seconds
TestRunner_rfcomm-tester      PASS      15.43 seconds
TestRunner_sco-tester         PASS      18.38 seconds
TestRunner_ioctl-tester       PASS      17.15 seconds
TestRunner_mesh-tester        PASS      12.63 seconds
TestRunner_smp-tester         PASS      13.77 seconds
TestRunner_userchan-tester    PASS      10.68 seconds
IncrementalBuild              PASS      30.31 seconds



---
Regards,
Linux Bluetooth

Re: [PATCH v1] Bluetooth: btnxpuart: Improve inband Independent Reset handling

[patchwork-bot+bluetooth]

Hello:

This patch was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:

On Thu, 10 Aug 2023 19:25:09 +0530 you wrote:
> This improves the inband IR command handling for NXP BT chipsets.
> When the IR vendor command is received, the driver injects a HW
> error event, which causes a reset sequence in hci_error_reset().
> The vendor IR command is sent to the controller while hci dev
> is been closed, and FW is re-downloaded when nxp_setup() is
> called during hci_dev_do_open().
> The HCI_SETUP flag is set in nxp_hw_err() to make sure that
> nxp_setup() is been called during hci_dev_do_open().
> 
> [...]

Here is the summary with links:
  - [v1] Bluetooth: btnxpuart: Improve inband Independent Reset handling
    https://git.kernel.org/bluetooth/bluetooth-next/c/db4d87d4a68e

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html