On Wed, Jan 17, 2024 at 08:35:01AM +0530, Neeraj Sanjay Kale wrote: > 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@xxxxxxxxxxxx/ > > Signed-off-by: Neeraj Sanjay Kale <neeraj.sanjaykale@xxxxxxx> > Reported-by: Marcel Ziswiler <marcel.ziswiler@xxxxxxxxxxx> > Closes: https://patchwork.kernel.org/project/bluetooth/patch/20231018145540.34014-3-marcel@xxxxxxxxxxxx/ > --- > 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 > @@ -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; why not just return -EPROBE_DEFER; and remove everything else, no need for any kind of retry or delayed work if the driver core takes care of it. Francesco
