linux-kernel - Re: [PATCH v5 2/4] Bluetooth: qca: Update firmware-name to support board specific nvm

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: <d5fe224b-7ef5-47ae-840c-7b6df21da816@oss.qualcomm.com>
Date: Fri, 20 Dec 2024 14:46:25 +0100
From: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
To: "Cheng Jiang (IOE)" <quic_chejiang@...cinc.com>,
        Konrad Dybcio <konrad.dybcio@....qualcomm.com>,
        Marcel Holtmann <marcel@...tmann.org>,
        Luiz Augusto von Dentz <luiz.dentz@...il.com>,
        Rob Herring
 <robh@...nel.org>,
        Krzysztof Kozlowski <krzk+dt@...nel.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio <konradybcio@...nel.org>,
        Balakrishna Godavarthi <quic_bgodavar@...cinc.com>,
        Rocky Liao <quic_rjliao@...cinc.com>
Cc: linux-bluetooth@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
        quic_jiaymao@...cinc.com, quic_shuaz@...cinc.com,
        quic_zijuhu@...cinc.com, quic_mohamull@...cinc.com
Subject: Re: [PATCH v5 2/4] Bluetooth: qca: Update firmware-name to support
 board specific nvm

On 13.12.2024 8:05 AM, Cheng Jiang (IOE) wrote:

[...]

>> /*
>>  * If the board-specific file is missing, try loading the default
>>  * one, unless that was attempted already
>>  */
>>
>> But, even more importantly:
>>
>> a) do we want to load the "incorrect" file?
> Normally, there is a default NVM file ending with .bin, which is suitable 
> for most boards. However, some boards have different configurations that 
> require a specific NVM. If a board-specific NVM is not found, a default 
> NVM is preferred over not loading any NVM.

So, if one is specified, but not found, this should either be a loud error,
or a very loud warning & fallback. Otherwise, the device may provide subpar
user experience without the user getting a chance to know the reason.

I think failing is better here, as that sends a clearer message, and would
only happen if the DT has a specific path (meaning the user put some
intentions behind that choice)

>> b) why would we want to specify the .bin file if it's the default anyway?
> The default NVM directory is the root of qca. The 'firmware-name' property 
> can specify an NVM file in another directory. This can be either a default 
> NVM like 'QCA6698/hpnv21.bin' or a board-specific NVM like 'QCA6698/hpnv21.b205'.

Do we expect QCA6698/hpnv21.bin and QCAabcd/hpnv21.bin to be compatible?

[...]

>>> -static inline void qca_get_nvm_name_generic(struct qca_fw_config *cfg,
>>> -					    const char *stem, u8 rom_ver, u16 bid)
>>> -{
>>> -	if (bid == 0x0)
>>> -		snprintf(cfg->fwname, sizeof(cfg->fwname), "qca/%snv%02x.bin", stem, rom_ver);
>>> -	else if (bid & 0xff00)
>>> -		snprintf(cfg->fwname, sizeof(cfg->fwname),
>>> -			 "qca/%snv%02x.b%x", stem, rom_ver, bid);
>>> -	else
>>> -		snprintf(cfg->fwname, sizeof(cfg->fwname),
>>> -			 "qca/%snv%02x.b%02x", stem, rom_ver, bid);
>>> +	if (soc_type == QCA_WCN6855 || soc_type == QCA_QCA2066) {
>>> +		/* hsp gf chip */
>>
>> This is a good opportunity to explain what that means
>>
> Ack. This means the chip is produced by GlobalFoundries.

Please update the comment there

Konrad