Re: [PATCH v1] Bluetooth: btnxpuart: Add support for IW624 chipset

[Neeraj sanjay kale]

Hi Francesco

Thank you for reviewing this patch.

> > --- a/drivers/bluetooth/btnxpuart.c
> > +++ b/drivers/bluetooth/btnxpuart.c
> ...
> > @@ -547,7 +553,7 @@ static int nxp_download_firmware(struct hci_dev
> *hdev)
> >       serdev_device_set_flow_control(nxpdev->serdev, false);
> >       nxpdev->current_baudrate = HCI_NXP_PRI_BAUDRATE;
> >
> > -     /* Wait till FW is downloaded and CTS becomes low */
> > +     /* Wait till FW is downloaded */
> >       err = wait_event_interruptible_timeout(nxpdev->fw_dnld_done_wait_q,
> >                                              !test_bit(BTNXPUART_FW_DOWNLOADING,
> >
> > &nxpdev->tx_state), @@ -558,16 +564,11 @@ static int
> nxp_download_firmware(struct hci_dev *hdev)
> >       }
> >
> >       serdev_device_set_flow_control(nxpdev->serdev, true);
> > -     err = serdev_device_wait_for_cts(nxpdev->serdev, 1, 60000);
> > -     if (err < 0) {
> > -             bt_dev_err(hdev, "CTS is still high. FW Download failed.");
> > -             return err;
> > -     }
> this seems like an unrelated change, and it's moving from a 60secs timeout
> polling CTS to nothing.
> 
> What's the reason for this? Should be this a separate commit with a proper
> explanation?
> 
While working on integrating IW624 in btnxpuart driver, I observed that the first reset command was getting timed out, after FW download was complete 2 out of 10 times. On further timing analysis, I noticed that this wait for CTS code did not actually help much, since CTS is already low after FW download, and becomes high after few more milli-seconds, and then low again after FW is initialized.
 So it was either adding a "wait for CTS high" followed by "wait for CTS low", or simply increasing the sleep delay from 1000msec to 1200msec.
I chose the later as it seemed more cleaner, and did the job perfectly, and tested all previously supported chipsets to make sure nothing is broke.
But you are right, I should add an explanation for this change in the commit message in the v2 patch.

Thanks,
Neeraj