linux-kernel - [RFC PATCH] Bluetooth: btnxpuart: Fix nxp_setup in case chip is powered on late

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20240117030501.149114-1-neeraj.sanjaykale@nxp.com>
Date: Wed, 17 Jan 2024 08:35:01 +0530
From: Neeraj Sanjay Kale <neeraj.sanjaykale@....com>
To: marcel@...tmann.org,
	johan.hedberg@...il.com,
	luiz.dentz@...il.com,
	marcel.ziswiler@...adex.com
Cc: amitkumar.karwar@....com,
	neeraj.sanjaykale@....com,
	linux-bluetooth@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	sherry.sun@....com,
	rohit.fule@....com
Subject: [RFC PATCH] Bluetooth: btnxpuart: Fix nxp_setup in case chip is powered on late

This adds a setup retry mechanism in case the chip is powered on after the
btnxpuart driver is loaded.

The NXP chipsets have a common PDn pin shared between Wi-Fi and Bluetooth.

When customers use mwifiex_sdio drivers for Wi-Fi, the pwrseq tied to the
driver toggles the GPIO connected to the chip's PDn pin, powering it on.

The btnxpuart driver is loaded before mwifiex, and the setup function does
not receive any bootloader signature, as PDn is held low at this moment.
The driver inadvertently assumes that FW is already running on the chip.

The nxp_setup exits with a success, and BT subsystem proceeds with sending
initialization commands, which result in a timeout.
[  284.588177] Bluetooth: hci0: Opcode 0x0c03 failed: -110
[  286.636167] Bluetooth: hci0: Setting wake-up method failed (-110)

Later when mwifiex is loaded, the pwrseq makes PDn pin high, and downloads
either WiFi or combo FW.

However, the btnxpuart is in a bad state, and re-loading btnxpuart module
does not help.

This fix adds a check for CTS pin, in case no bootloader signatures are
received. If CTS is high, it means that the chip is currently powered off,
and nxp_setup will return an error, preventing any HCI initialization
commands to be sent by the BT subsystem.

The driver attempts to check for bootloader signatures and CTS again, by
scheduling the hci power_on work after every 5 seconds, as long as the
btnxpuart module is inserted in the kernel.

This fix attempts to improvise the fix provided my Marcel Ziswiler and
handle this scenario gracefully.
https://patchwork.kernel.org/project/bluetooth/patch/20231018145540.34014-3-marcel@ziswiler.com/

Signed-off-by: Neeraj Sanjay Kale <neeraj.sanjaykale@....com>
Reported-by: Marcel Ziswiler <marcel.ziswiler@...adex.com>
Closes: https://patchwork.kernel.org/project/bluetooth/patch/20231018145540.34014-3-marcel@ziswiler.com/
---
 drivers/bluetooth/btnxpuart.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/bluetooth/btnxpuart.c b/drivers/bluetooth/btnxpuart.c
index 7f88b6f52f26..20a3a5bd5529 100644
--- a/drivers/bluetooth/btnxpuart.c
+++ b/drivers/bluetooth/btnxpuart.c
@@ -171,6 +171,7 @@ struct btnxpuart_dev {
 	bool timeout_changed;
 	bool baudrate_changed;
 	bool helper_downloaded;
+	struct delayed_work setup_retry_work;

 	struct ps_data psdata;
 	struct btnxpuart_data *nxp_data;
@@ -240,6 +241,8 @@ struct v3_start_ind {
 	u8 crc;
 } __packed;

+#define NXP_SETUP_RETRY_TIME_MS	5000
+
 /* UART register addresses of BT chip */
 #define CLKDIVADDR	0x7f00008f
 #define UARTDIVADDR	0x7f000090
@@ -1008,6 +1011,15 @@ static int nxp_check_boot_sign(struct btnxpuart_dev *nxpdev)
 					       msecs_to_jiffies(1000));
 }

+static void nxp_setup_retry_work(struct work_struct *work)
+{
+	struct btnxpuart_dev *nxpdev = container_of(work, struct btnxpuart_dev,
+						    setup_retry_work.work);
+	struct hci_dev *hdev = nxpdev->hdev;
+
+	queue_work(hdev->req_workqueue, &hdev->power_on);
+}
+
 static int nxp_set_ind_reset(struct hci_dev *hdev, void *data)
 {
 	static const u8 ir_hw_err[] = { HCI_EV_HARDWARE_ERROR,
@@ -1036,6 +1048,13 @@ static int nxp_setup(struct hci_dev *hdev)
 		err = nxp_download_firmware(hdev);
 		if (err < 0)
 			return err;
+	} else if (!serdev_device_get_cts(nxpdev->serdev)) {
+		/* CTS is high and no bootloader signatures detected */
+		bt_dev_dbg(hdev, "Controller not detected. Will check again in %d msec",
+			   NXP_SETUP_RETRY_TIME_MS);
+		schedule_delayed_work(&nxpdev->setup_retry_work,
+				      msecs_to_jiffies(NXP_SETUP_RETRY_TIME_MS));
+		return -ENODEV;
 	} else {
 		bt_dev_dbg(hdev, "FW already running.");
 		clear_bit(BTNXPUART_FW_DOWNLOADING, &nxpdev->tx_state);
@@ -1373,6 +1392,7 @@ static int nxp_serdev_probe(struct serdev_device *serdev)
 	}

 	ps_setup(hdev);
+	INIT_DELAYED_WORK(&nxpdev->setup_retry_work, nxp_setup_retry_work);

 	return 0;
 }
@@ -1391,6 +1411,7 @@ static void nxp_serdev_remove(struct serdev_device *serdev)
 		nxp_set_baudrate_cmd(hdev, NULL);
 	}

+	cancel_delayed_work_sync(&nxpdev->setup_retry_work);
 	ps_cancel_timer(nxpdev);
 	hci_unregister_dev(hdev);
 	hci_free_dev(hdev);
-- 
2.34.1