linux-kernel - Re: [PATCH v4 1/1] Bluetooth: btusb: add default nvm file

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] [day] [month] [year] [list]
Message-ID: <78fa2388-3815-4ba5-84c8-38d85e880221@quicinc.com>
Date: Mon, 10 Nov 2025 21:01:47 +0800
From: Shuai Zhang <quic_shuaz@...cinc.com>
To: Paul Menzel <pmenzel@...gen.mpg.de>
CC: Marcel Holtmann <marcel@...tmann.org>,
        Luiz Augusto von Dentz
	<luiz.dentz@...il.com>,
        <linux-bluetooth@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <linux-arm-msm@...r.kernel.org>, <quic_chejiang@...cinc.com>,
        <quic_jiaymao@...cinc.com>, <quic_chezhou@...cinc.com>
Subject: Re: [PATCH v4 1/1] Bluetooth: btusb: add default nvm file

Dear Paul

Thank you for taking the time to review it.

On 11/10/2025 4:01 PM, Paul Menzel wrote:
> Dear Shuai,
> 
> 
> Thank you for the patch.
> 
> Am 10.11.25 um 04:41 schrieb Shuai Zhang:
>> If no NVM file matches the board_id, load the default NVM file.
> 
> Maybe also add the comment, that the NVM file is always compatible(?) and just might not give the best performance.
> 

The default NVM file may differ in functionality and performance because specific NVM files enable certain 
vendor commands based on chip capabilities. To ensure compatibility, the logic to load a default NVM file is added
when a dedicated NVM file is not yet available, so that basic BT functionality remains operational.

> Also, please add a comment about passing board_id now. (Is that necessary or just an optimization?)
> 

The board_id is used to determine the appropriate NVM file, and it is mandatory.

I will also update the new patch to include these comments.

>> Signed-off-by: Shuai Zhang <quic_shuaz@...cinc.com>
>> ---
>>   drivers/bluetooth/btusb.c | 26 +++++++++++++++++---------
>>   1 file changed, 17 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
>> index dcbff7641..020dbb0ab 100644
>> --- a/drivers/bluetooth/btusb.c
>> +++ b/drivers/bluetooth/btusb.c
>> @@ -3482,15 +3482,14 @@ static int btusb_setup_qca_load_rampatch(struct hci_dev *hdev,
>>   }
>>     static void btusb_generate_qca_nvm_name(char *fwname, size_t max_size,
>> -                    const struct qca_version *ver)
>> +                    const struct qca_version *ver,
>> +                    u16 board_id)
>>   {
>>       u32 rom_version = le32_to_cpu(ver->rom_version);
>>       const char *variant, *fw_subdir;
>>       int len;
>> -    u16 board_id;
>>         fw_subdir = qca_get_fw_subdirectory(ver);
>> -    board_id = qca_extract_board_id(ver);
>>         switch (le32_to_cpu(ver->ram_version)) {
>>       case WCN6855_2_0_RAM_VERSION_GF:
>> @@ -3517,14 +3516,14 @@ static void btusb_generate_qca_nvm_name(char *fwname, size_t max_size,
>>     static int btusb_setup_qca_load_nvm(struct hci_dev *hdev,
>>                       struct qca_version *ver,
>> -                    const struct qca_device_info *info)
>> +                    const struct qca_device_info *info,
>> +                    u16 board_id)
>>   {
>>       const struct firmware *fw;
>>       char fwname[80];
>>       int err;
>>   -    btusb_generate_qca_nvm_name(fwname, sizeof(fwname), ver);
>> -
>> +    btusb_generate_qca_nvm_name(fwname, sizeof(fwname), ver, board_id);
>>       err = request_firmware(&fw, fwname, &hdev->dev);
>>       if (err) {
>>           bt_dev_err(hdev, "failed to request NVM file: %s (%d)",
>> @@ -3606,10 +3605,19 @@ static int btusb_setup_qca(struct hci_dev *hdev)
>>       btdata->qca_dump.controller_id = le32_to_cpu(ver.rom_version);
>>         if (!(status & QCA_SYSCFG_UPDATED)) {
>> -        err = btusb_setup_qca_load_nvm(hdev, &ver, info);
>> -        if (err < 0)
>> -            return err;
>> +        u16 board_id = qca_extract_board_id(&ver);
>>   +        err = btusb_setup_qca_load_nvm(hdev, &ver, info, board_id);
>> +        if (err < 0) {
>> +            //if the board id is not 0, try to load the defalut nvm file
> 
> Did `checkpatch.pl` not complain about the missing space, and misspelling of *default*?

Sorry for the typo. I will update it. > 
>> +            if (err == -ENOENT && board_id != 0) {
>> +                err = btusb_setup_qca_load_nvm(hdev, &ver, info, 0);
>> +                if (err < 0)
>> +                    return err;
>> +            } else {
>> +                return err;
>> +            }
>> +        }
>>           /* WCN6855 2.1 and later will reset to apply firmware downloaded here, so
>>            * wait ~100ms for reset Done then go ahead, otherwise, it maybe
>>            * cause potential enable failure.
> 
> Please try to catch the style errors before submitting patches.
> 

 I will update it. 

> 
> Kind regards,
> 
> Paul

Best,regard
Shuai