| 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
| ||
|
Message-ID: <4pfooyem3h2ljapiye5faljkwi7aekaep6z6o4jviqghu3nsdg@7tslak7jyuky>
Date: Wed, 24 Dec 2025 11:39:20 +0200
From: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
To: Shuai Zhang <shuai.zhang@....qualcomm.com>
Cc: Bartosz Golaszewski <brgl@...ev.pl>, Marcel Holtmann <marcel@...tmann.org>,
Luiz Augusto von Dentz <luiz.dentz@...il.com>,
linux-arm-msm@...r.kernel.org, linux-bluetooth@...r.kernel.org,
linux-kernel@...r.kernel.org, cheng.jiang@....qualcomm.com,
quic_chezhou@...cinc.com, wei.deng@....qualcomm.com
Subject: Re: [PATCH v3 1/1] Bluetooth: btqca: Add WCN6855 firmware priority
selection feature
On Wed, Dec 24, 2025 at 02:54:24PM +0800, Shuai Zhang wrote:
>
> On 12/24/2025 12:23 PM, Dmitry Baryshkov wrote:
> > On Tue, Dec 23, 2025 at 10:03:44AM +0800, Shuai Zhang wrote:
> > > Hi Dmitry
> > >
> > > On 12/21/2025 11:21 PM, Dmitry Baryshkov wrote:
> > > > On Fri, Dec 19, 2025 at 05:19:30PM +0800, Shuai Zhang wrote:
> > > > > Hi Dmitry
> > > > >
> > > > > On 11/19/2025 3:59 PM, Dmitry Baryshkov wrote:
> > > > > > On Mon, Nov 17, 2025 at 10:16:45AM +0800, Shuai Zhang wrote:
> > > > > > > Historically, WCN685x and QCA2066 shared the same firmware files.
> > > > > > > Now, changes are planned for the firmware that will make it incompatible
> > > > > > > with QCA2066, so a new firmware name is required for WCN685x.
> > > > > > >
> > > > > > > Test Steps:
> > > > > > > - Boot device
> > > > > > > - Check the BTFW loading status via dmesg
> > > > > > >
> > > > > > > Sanity pass and Test Log:
> > > > > > > QCA Downloading qca/wcnhpbftfw21.tlv
> > > > > > > Direct firmware load for qca/wcnhpbftfw21.tlv failed with error -2
> > > > > > > QCA Downloading qca/hpbftfw21.tlv
> > > > > > >
> > > > > > > Signed-off-by: Shuai Zhang<shuai.zhang@....qualcomm.com>
> > > > > > > ---
> > > > > > > drivers/bluetooth/btqca.c | 22 ++++++++++++++++++++--
> > > > > > > 1 file changed, 20 insertions(+), 2 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
> > > > > > > index 7c958d606..8e0004ef7 100644
> > > > > > > --- a/drivers/bluetooth/btqca.c
> > > > > > > +++ b/drivers/bluetooth/btqca.c
> > > > > > > @@ -847,8 +847,12 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
> > > > > > > "qca/msbtfw%02x.mbn", rom_ver);
> > > > > > > break;
> > > > > > > case QCA_WCN6855:
> > > > > > > + /* Due to historical reasons, WCN685x chip has been using firmware
> > > > > > > + * without the "wcn" prefix. The mapping between the chip and its
> > > > > > > + * corresponding firmware has now been corrected.
> > > > > > > + */
> > > > > > > snprintf(config.fwname, sizeof(config.fwname),
> > > > > > > - "qca/hpbtfw%02x.tlv", rom_ver);
> > > > > > > + "qca/wcnhpbtfw%02x.tlv", rom_ver);
> > > > > > > break;
> > > > > > > case QCA_WCN7850:
> > > > > > > snprintf(config.fwname, sizeof(config.fwname),
> > > > > > > @@ -861,6 +865,13 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
> > > > > > > }
> > > > > > > err = qca_download_firmware(hdev, &config, soc_type, rom_ver);
> > > > > > > +
> > > > > > > + if (!rampatch_name && err < 0 && soc_type == QCA_WCN6855) {
> > > > > > > + snprintf(config.fwname, sizeof(config.fwname),
> > > > > > > + "qca/hpbtfw%02x.tlv", rom_ver);
> > > > > > > + err = qca_download_firmware(hdev, &config, soc_type, rom_ver);
> > > > > > > + }
> > > > > > Is there a reason for ignoring how it was done already for other cases when
> > > > > > we need a similar fallback? Please extend the existing code (or rewrite
> > > > > > it) instead of adding a similar hook at a completely different place.
> > > > > Current Strategy (when DTS does not specify rampatch and firmware):
> > > > > Rampatch: Load the rampatch based on soc_type.
> > > > > NVM: Load the NVM with board_id based on soc_type.
> > > > > If the file corresponding to board_id does not exist, then
> > > > > load the NVM file ending with .bin.
> > > > > For HSP (new requirement):
> > > > > First, load the rampatch/NVM files wcnhpbtfw and wcnhpnv.
> > > > > If not found:
> > > > > Rampatch: Fall back to loading the hpbtfw rampatch file.
> > > > > NVM: Starting from wcnhpnv.bxxx, load the NVM file ending with
> > > > > .bin.
> > > > > If still not found, look for hpnv.bxxx and then apply
> > > > > the above NVM strategy again (soc_type(board_id) to .bin).
> > > > >
> > > > > The current changes are based on the original implementation, which should
> > > > > make them the clearest modifications.
> > > > > Please review according to the existing strategy, and feel free to let me
> > > > > know if you have any questions.
> > > > qca_download_firmware() has workaround code for WCN6750, loading TLV
> > > > file if MBN is not present. It doesn't make sense to have similar
> > > > workardounds in two different places. Could you please unify code
> > > > (either by moving existing code or by moving your workaround).
> > > I tried to move the changes into |qca_download_firmware|, but it conflicts
> > > with the logic for
> > > loading the default NVM. Specifically, when there is no NVM corresponding to
> > > the board_id,
> > > it will not load the |.bin| NVM file. I’m not sure whether this limitation
> > > is within a controllable range.
> > >
> > > https://github.com/shuaz-shuai/Add-WCN6855-firmware-priority-selection-feature
> > So, the solution is to move the quirk for WCN6750 out of
> > qca_download_firmware().
>
> I’m not entirely clear on the rationale for removing WCN6750,
>
> as our current discussion focuses on handling WCN6855.
>
As I wrote earlier, code uniformity. We already have a quirk code
handling almost the same pattern as yours: load the fallback file if the
expected one is missing.
Also please fix your editor not to insert extra empty lines in your response.
>
> If the logic for loading hpbtfw.tlv and hpnv.bxxx when wcnhpbtfw.tlv and
> wcnhpnv.bxxx
>
> are missing is moved into qca_download_firmware(),
>
> it won’t affect the firmware (hpbtfw.tlv). However, for NVM, if loading
> hpnv.bxxx fails,
>
> the hpnv.bin file will also not be loaded, which is a defect.
As I wrote, you are providing more and more arguments for moving WCN6750
code from qca_download_firmware() to the caller site.
>
> This is why I prefer retaining the V3 changes.
>
> Please let me know if you have any concerns or questions.
>
> > > > > > > +
> > > > > > > if (err < 0) {
> > > > > > > bt_dev_err(hdev, "QCA Failed to download patch (%d)", err);
> > > > > > > return err;
> > > > > > > @@ -923,7 +934,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
> > > > > > > case QCA_WCN6855:
> > > > > > > qca_read_fw_board_id(hdev, &boardid);
> > > > > > > qca_get_nvm_name_by_board(config.fwname, sizeof(config.fwname),
> > > > > > > - "hpnv", soc_type, ver, rom_ver, boardid);
> > > > > > > + "wcnhpnv", soc_type, ver, rom_ver, boardid);
> > > > > > > break;
> > > > > > > case QCA_WCN7850:
> > > > > > > qca_get_nvm_name_by_board(config.fwname, sizeof(config.fwname),
> > > > > > > @@ -936,6 +947,13 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
> > > > > > > }
> > > > > > > err = qca_download_firmware(hdev, &config, soc_type, rom_ver);
> > > > > > > +
> > > > > > > + if (!firmware_name && err < 0 && soc_type == QCA_WCN6855) {
> > > > > > > + qca_get_nvm_name_by_board(config.fwname, sizeof(config.fwname),
> > > > > > > + "hpnv", soc_type, ver, rom_ver, boardid);
> > > > > > > + err = qca_download_firmware(hdev, &config, soc_type, rom_ver);
> > > > > > > + }
> > > > > > > +
> > > > > > > if (err < 0) {
> > > > > > > bt_dev_err(hdev, "QCA Failed to download NVM (%d)", err);
> > > > > > > return err;
> > > > > > > --
> > > > > > > 2.34.1
> > > > > > >
> > > > > Thanks,
> > > > > Shuai
--
With best wishes
Dmitry
Powered by blists - more mailing lists