[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <wxb6oo3f7cri27eq3fnqttbl74lkn3rjo5o5rfttbwz7k5v2ug@w4hlrtig2nza>
Date: Tue, 10 Dec 2024 17:28:16 +0200
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To: Cheng Jiang <quic_chejiang@...cinc.com>
Cc: 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>,
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 v4 2/4] Bluetooth: qca: Add support in firmware-name to
load board specific nvm
On Tue, Dec 10, 2024 at 11:16:34PM +0800, 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";
>
> Signed-off-by: Cheng Jiang <quic_chejiang@...cinc.com>
> ---
> drivers/bluetooth/btqca.c | 67 +++++++++++++++++++++++++++++++++++++--
> 1 file changed, 65 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
> index dfbbac922..deb2b1753 100644
> --- a/drivers/bluetooth/btqca.c
> +++ b/drivers/bluetooth/btqca.c
> @@ -272,6 +272,27 @@ int qca_send_pre_shutdown_cmd(struct hci_dev *hdev)
> }
> EXPORT_SYMBOL_GPL(qca_send_pre_shutdown_cmd);
>
> +static bool qca_get_alt_nvm_path(char *path, size_t max_size)
> +{
> + char fwname[64];
> + const char *suffix;
> +
> + suffix = strrchr(path, '.');
> +
> + /* nvm file name has a suffix, replace with .bin */
> + if (suffix && suffix != path && *(suffix + 1) != '\0' && strchr(suffix, '/') == NULL) {
> + strscpy(fwname, path, suffix - path + 1);
> + snprintf(fwname + (suffix - path), sizeof(fwname) - (suffix - path), ".bin");
> + /* If nvm file is already the default one, return false to skip the retry. */
> + if (strcmp(fwname, path) == 0)
> + return false;
> +
> + snprintf(path, max_size, "%s", fwname);
> + return true;
> + }
> + return false;
> +}
> +
> static int qca_tlv_check_data(struct hci_dev *hdev,
> struct qca_fw_config *config,
> u8 *fw_data, size_t fw_size,
> @@ -564,6 +585,19 @@ static int qca_download_firmware(struct hci_dev *hdev,
> config->fwname, ret);
> return ret;
> }
> + }
> + /* 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.
> + */
> + else if (config->type == TLV_TYPE_NVM &&
> + qca_get_alt_nvm_path(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);
> @@ -730,6 +764,26 @@ static inline void qca_get_nvm_name_generic(struct qca_fw_config *cfg,
> "qca/%snv%02x.b%02x", stem, rom_ver, bid);
> }
>
> +static void qca_get_nvm_name_by_board(char *fwname, size_t max_size,
> + const char *firmware_name, struct qca_btsoc_version ver,
> + enum qca_btsoc_type soc_type, u16 bid)
> +{
> + const char *variant;
> +
> + /* Set the variant to empty by default */
> + variant = "";
> + /* hsp gf chip */
> + if (soc_type == QCA_WCN6855) {
> + if ((le32_to_cpu(ver.soc_id) & QCA_HSP_GF_SOC_MASK) == QCA_HSP_GF_SOC_ID)
> + variant = "g";
> + }
> +
> + if (bid == 0x0 || bid == 0xffff)
> + snprintf(fwname, max_size, "qca/%s%s.bin", firmware_name, variant);
> + else
> + snprintf(fwname, max_size, "qca/%s%s.b%02x", firmware_name, variant, bid);
So, we have qca_generate_hsp_nvm_name(), qca_get_nvm_name_generic(), now
you are adding a third one. Can we please have a single function that
handles that?
> +}
> +
> int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
> enum qca_btsoc_type soc_type, struct qca_btsoc_version ver,
> const char *firmware_name)
> @@ -739,6 +793,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
> u8 rom_ver = 0;
> u32 soc_ver;
> u16 boardid = 0;
> + const char *suffix;
>
> bt_dev_dbg(hdev, "QCA setup on UART");
>
> @@ -816,8 +871,16 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
> /* Download NVM configuration */
> config.type = TLV_TYPE_NVM;
> if (firmware_name) {
> - snprintf(config.fwname, sizeof(config.fwname),
> - "qca/%s", firmware_name);
> + /* The firmware name has suffix, use it directly */
> + suffix = strrchr(firmware_name, '.');
> + if (suffix && suffix != firmware_name &&
> + *(suffix + 1) != '\0' && strchr(suffix, '/') == NULL) {
The have-suffix code should be extracted to a helper function. You have
two copies of it.
> + snprintf(config.fwname, sizeof(config.fwname), "qca/%s", firmware_name);
> + } else {
> + qca_read_fw_board_id(hdev, &boardid);
> + qca_get_nvm_name_by_board(config.fwname, sizeof(config.fwname),
> + firmware_name, ver, soc_type, boardid);
> + }
> } else {
> switch (soc_type) {
> case QCA_WCN3990:
> --
> 2.25.1
>
--
With best wishes
Dmitry
Powered by blists - more mailing lists