[v1] Bluetooth: btnxpuart: Enable flow control before checking boot signature

Commit Message

Neeraj Sanjay Kale April 19, 2023, 9:03 a.m. UTC

This enables flow control before checking for bootloader signature and
deciding whether FW download is needed or not. In case of V1 bootloader
chips w8987 and w8997, it is observed that if WLAN FW is downloaded first
and power save is enabled in wlan core, bootloader signatures are not
emitted by the BT core when the chip is put to sleep. This results in BT
FW download timeout error. By enabling the flow control, the host enables
its RTS pin, and an interrupt in chip's UART peripheral causes the
bootloader to wakeup, enabling the bootloader signatures, which then helps
in downloading the bluetooth FW file.

Signed-off-by: Neeraj Sanjay Kale <[e-mail]>
---
 drivers/bluetooth/btnxpuart.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

[e-mail] April 19, 2023, 9:33 a.m. UTC | #1

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=741256

---Test result---

Test Summary:
CheckPatch                    PASS      0.68 seconds
GitLint                       PASS      0.34 seconds
SubjectPrefix                 PASS      0.12 seconds
BuildKernel                   PASS      32.23 seconds
CheckAllWarning               PASS      34.90 seconds
CheckSparse                   PASS      39.82 seconds
CheckSmatch                   PASS      109.22 seconds
BuildKernel32                 PASS      30.95 seconds
TestRunnerSetup               PASS      440.73 seconds
TestRunner_l2cap-tester       PASS      16.86 seconds
TestRunner_iso-tester         PASS      20.67 seconds
TestRunner_bnep-tester        PASS      5.59 seconds
TestRunner_mgmt-tester        PASS      115.18 seconds
TestRunner_rfcomm-tester      PASS      8.98 seconds
TestRunner_sco-tester         PASS      8.33 seconds
TestRunner_ioctl-tester       PASS      9.72 seconds
TestRunner_mesh-tester        PASS      7.09 seconds
TestRunner_smp-tester         PASS      8.08 seconds
TestRunner_userchan-tester    PASS      5.84 seconds
IncrementalBuild              PASS      29.67 seconds



---
Regards,
Linux Bluetooth

Paul Menzel April 19, 2023, 10:33 a.m. UTC | #2

Dear Neeraj,


Thank you for your patch.

Am 19.04.23 um 11:03 schrieb Neeraj Sanjay Kale:
> This enables flow control before checking for bootloader signature and
> deciding whether FW download is needed or not. In case of V1 bootloader
> chips w8987 and w8997, it is observed that if WLAN FW is downloaded first
> and power save is enabled in wlan core, bootloader signatures are not
> emitted by the BT core when the chip is put to sleep. This results in BT
> FW download timeout error.

If there is a log message, maybe paste it.

> By enabling the flow control, the host enables
> its RTS pin, and an interrupt in chip's UART peripheral causes the
> bootloader to wakeup, enabling the bootloader signatures, which then helps

Nit: The verb wake up is spelled with a space.

> in downloading the bluetooth FW file.
> 
> Signed-off-by: Neeraj Sanjay Kale <[e-mail]>
> ---
>   drivers/bluetooth/btnxpuart.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/bluetooth/btnxpuart.c b/drivers/bluetooth/btnxpuart.c
> index 93f3afc0c0c8..3ee41a0d45e7 100644
> --- a/drivers/bluetooth/btnxpuart.c
> +++ b/drivers/bluetooth/btnxpuart.c
> @@ -967,7 +967,7 @@ static int nxp_set_ind_reset(struct hci_dev *hdev, void *data)
>   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, 0);
> +	serdev_device_set_flow_control(nxpdev->serdev, 1);

It was explicitly disabled before. Is it on by default?

Also, is there now a redundant line later in the flow enabling flow 
control, or was it never enabled?

Other place in the source code seem to use `true` or `false` instead of 1/0.

>   	set_bit(BTNXPUART_CHECK_BOOT_SIGNATURE, &nxpdev->tx_state);
>   
>   	return wait_event_interruptible_timeout(nxpdev->check_boot_sign_wait_q,


Kind regards,

Paul

Neeraj Sanjay Kale April 19, 2023, 2:27 p.m. UTC | #3

Hi Paul,

Thank you for reviewing this patch.

> >   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, 0);
> > +     serdev_device_set_flow_control(nxpdev->serdev, 1);
> 
> It was explicitly disabled before. Is it on by default?
Flow control is disabled by default, and this is the first instance here, where it is enabled.

> 
> Also, is there now a redundant line later in the flow enabling flow control, or
> was it never enabled?
Yes. Removed the redundant line as we are now enabling flow control in nxp_check_boot_sign().

> 
> Other place in the source code seem to use `true` or `false` instead of 1/0.
Changed all instances of 0/1 to false/true in v2 patch.

Thanks,
Neeraj

Patch

diff --git a/drivers/bluetooth/btnxpuart.c b/drivers/bluetooth/btnxpuart.c
index 93f3afc0c0c8..3ee41a0d45e7 100644
--- a/drivers/bluetooth/btnxpuart.c
+++ b/drivers/bluetooth/btnxpuart.c
@@ -967,7 +967,7 @@  static int nxp_set_ind_reset(struct hci_dev *hdev, void *data)
 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, 0);
+	serdev_device_set_flow_control(nxpdev->serdev, 1);
 	set_bit(BTNXPUART_CHECK_BOOT_SIGNATURE, &nxpdev->tx_state);
 
 	return wait_event_interruptible_timeout(nxpdev->check_boot_sign_wait_q,