On 12.12.2024 4:02 PM, Cheng Jiang wrote:
> Different connectivity boards may be attached to the same platform. For
> example, QCA6698-based boards can support either a two-antenna or
> three-antenna solution, both of which work on the sa8775p-ride platform.
> Due to differences in connectivity boards and variations in RF
> performance from different foundries, different NVM configurations are
> used based on the board ID.
>
> Therefore, in the firmware-name property, if the NVM file has an
> extension, the NVM file will be used. Otherwise, the system will first
> try the .bNN (board ID) file, and if that fails, it will fall back to
> the .bin file.
>
> Possible configurations:
> firmware-name = "QCA6698/hpnv21";
> firmware-name = "QCA6698/hpnv21.bin";
I think we should agree on one and then do some magic to look up
the other variants.
>
> Signed-off-by: Cheng Jiang <quic_chejiang@xxxxxxxxxxx>
> ---
> drivers/bluetooth/btqca.c | 112 ++++++++++++++++++++++++++++----------
> 1 file changed, 84 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
> index dfbbac922..4842f4335 100644
> --- a/drivers/bluetooth/btqca.c
> +++ b/drivers/bluetooth/btqca.c
> @@ -272,6 +272,38 @@ int qca_send_pre_shutdown_cmd(struct hci_dev *hdev)
> }
> EXPORT_SYMBOL_GPL(qca_send_pre_shutdown_cmd);
>
> +static bool qca_filename_has_extension(const char *filename)
> +{
> + const char *suffix;
> +
> + suffix = strrchr(filename, '.');
> + if (suffix && suffix != filename && *(suffix + 1) != '\0' && strchr(suffix, '/') == NULL)
> + return true;
> + else
> + return false;
How about:
const char *suffix = strrchr(filename, '.');
/* File extensions require a dot, but not as the last character */
if (!suffix || *(suffix + 1) == NULL)
return false;
/* Avoid matching directories with names that look like files with extensions */
return !(suffix, '/');
> + }
> + /* For nvm, if desired nvm file is not present and it's not the
> + * default nvm file(ends with .bin), try to load the default nvm.
nvm appears 4 times in two lines, how about:
/*
* 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?
b) why would we want to specify the .bin file if it's the default anyway?
> + */
> + else if (config->type == TLV_TYPE_NVM &&
> + qca_get_alt_nvm_file(config->fwname, sizeof(config->fwname))) {
> + bt_dev_info(hdev, "QCA Downloading %s", config->fwname);
> + ret = request_firmware(&fw, config->fwname, &hdev->dev);
> + if (ret) {
> + bt_dev_err(hdev, "QCA Failed to request file: %s (%d)",
> + config->fwname, ret);
> + return ret;
> + }
> } else {
> bt_dev_err(hdev, "QCA Failed to request file: %s (%d)",
> config->fwname, ret);
> @@ -700,34 +745,38 @@ static int qca_check_bdaddr(struct hci_dev *hdev, const struct qca_fw_config *co
> return 0;
> }
>
> -static void qca_generate_hsp_nvm_name(char *fwname, size_t max_size,
> +static void qca_get_nvm_name_by_board(char *fwname, size_t max_size,
> + const char *stem, enum qca_btsoc_type soc_type,
> struct qca_btsoc_version ver, u8 rom_ver, u16 bid)
> {
> const char *variant;
> + const char *prefix;
>
> - /* hsp gf chip */
> - if ((le32_to_cpu(ver.soc_id) & QCA_HSP_GF_SOC_MASK) == QCA_HSP_GF_SOC_ID)
> - variant = "g";
> - else
> - variant = "";
> + /* Set the defalut value to variant and prefixt */
typos: default, prefix
> + variant = "";
> + prefix = "b";
>
> - if (bid == 0x0)
> - snprintf(fwname, max_size, "qca/hpnv%02x%s.bin", rom_ver, variant);
> - else
> - snprintf(fwname, max_size, "qca/hpnv%02x%s.%x", rom_ver, variant, bid);
> -}
> + if (soc_type == QCA_QCA2066)
> + prefix = "";
>
> -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
Konrad
