linux-kernel - Re: [PATCH v2] Bluetooth: btusb: Improve stability for QCA devices

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-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <59F65F10-0988-4E50-8956-69C601F05434@holtmann.org>
Date:   Fri, 11 Feb 2022 17:22:46 +0100
From:   Marcel Holtmann <marcel@...tmann.org>
To:     Zijun Hu <zijuhu@...eaurora.org>
Cc:     Johan Hedberg <johan.hedberg@...il.com>,
        Luiz Augusto von Dentz <luiz.dentz@...il.com>,
        linux-kernel@...r.kernel.org, linux-bluetooth@...r.kernel.org,
        linux-arm-msm@...r.kernel.org, c-hbandi@...eaurora.org,
        hemantg@...eaurora.org, rjliao@...eaurora.org,
        tjiang@...eaurora.org, Zijun Hu <quic_zijuhu@...cinc.com>
Subject: Re: [PATCH v2] Bluetooth: btusb: Improve stability for QCA devices

Hi Zijun,

> Controller will reset after NVM is downloaded for QCA
> device, so wait a moment for reset Done then go ahead
> to improve stability.
> 
> Signed-off-by: Zijun Hu <quic_zijuhu@...cinc.com>
> ---
> drivers/bluetooth/btusb.c | 5 +++++
> 1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index e03dfbd92fcc..20e36f53d2e7 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -2994,6 +2994,7 @@ static int btusb_set_bdaddr_wcn6855(struct hci_dev *hdev,
> #define QCA_PATCH_UPDATED	0x80
> #define QCA_DFU_TIMEOUT		3000
> #define QCA_FLAG_MULTI_NVM      0x80
> +#define QCA_BT_RESET_WAIT_MS    100
> 
> #define WCN6855_2_0_RAM_VERSION_GF 0x400c1200
> #define WCN6855_2_1_RAM_VERSION_GF 0x400c1211
> @@ -3320,6 +3321,10 @@ static int btusb_setup_qca(struct hci_dev *hdev)
> 		err = btusb_setup_qca_load_nvm(hdev, &ver, info);
> 		if (err < 0)
> 			return err;
> +		/* Controller will reset after NVM is downloaded, so wait a moment
> +		 * for reset Done, it will improve stability.
> +		 */
> +		msleep(QCA_BT_RESET_WAIT_MS);

how hard is to just grab the data sheet and figure out the appropriate time to wait? I will be all documented and then reference the documentation.

I really dislike this "add a sleep here and sleep there". It might just work for now. The next hardware generation comes around or if placed on a different board just behaves a little bit different. And at some point we are at 10 seconds sleep and you start complaining why the controller initialization takes so long. Stop guessing and reference the data sheet.

Regards

Marcel