Re: [PATCH v1 2/2] Bluetooth: btnxpuart: Fix nxp_setup

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Dear Marcel,


Thank you for your patch.

Am 18.10.23 um 16:55 schrieb Marcel Ziswiler:
From: Marcel Ziswiler <marcel.ziswiler@xxxxxxxxxxx>

“Fix nxp_setup” is really generic. A more specific message would be great. Maybe: Wait up to 10 s for firmware.

Unfortunately, nxp_setup() may inadvertently assume that the
firmware is already running while the module is not even powered yet.
Fix this by waiting up to 10 seconds for the CTS to go up as the combo
firmware might be loaded by the Wi-Fi driver over SDIO (mwifiex_sdio).

Fixes: 689ca16e5232 ("Bluetooth: NXP: Add protocol support for NXP Bluetooth chipsets")

Signed-off-by: Marcel Ziswiler <marcel.ziswiler@xxxxxxxxxxx>

---
This is what may happen without this fix:
[  284.588177] Bluetooth: hci0: Opcode 0x0c03 failed: -110
[  286.636167] Bluetooth: hci0: Setting wake-up method failed (-110)
Unfortunately, even re-loading the btnxpuart kernel module would not
recover from this condition.

I’d add that information to the commit message.

  drivers/bluetooth/btnxpuart.c | 10 ++++++++++
  1 file changed, 10 insertions(+)

diff --git a/drivers/bluetooth/btnxpuart.c b/drivers/bluetooth/btnxpuart.c
index 9cb7529eef09..4b83a0aa3459 100644
--- a/drivers/bluetooth/btnxpuart.c
+++ b/drivers/bluetooth/btnxpuart.c
@@ -1021,6 +1021,16 @@ static int nxp_setup(struct hci_dev *hdev)
  		if (err < 0)
  			return err;
  	} else {
+		/* The combo firmware might be loaded by the Wi-Fi driver over SDIO (mwifiex_sdio).
+		 * We wait up to 10s for the CTS to go up. Afterwards, we know that the firmware is
+		 * really ready.
+		 */
+		err = serdev_device_wait_for_cts(nxpdev->serdev, true, 10000);
+		if (err) {
+			bt_dev_err(nxpdev->hdev, "Wait for CTS failed with %d", err);
+			return err;
+		}
+
  		bt_dev_dbg(hdev, "FW already running.");
  		clear_bit(BTNXPUART_FW_DOWNLOADING, &nxpdev->tx_state);
  	}

Isn’t there another way to find out, if the firmware is there? Adding an arbitrary timeout of ten seconds sounds like a fundamental flaw in the driver?


Kind regards,

Paul



[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux