Re: [PATCH v3 2/3] Bluetooth: qca: Expand firmware-name to load specific nvm and rampatch

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Dec 05, 2024 at 06:22:12PM +0800, Cheng Jiang wrote:
> The firmware-name property has been expanded to specify the names of NVM
> and rampatch firmware for certain chips, such as the QCA6698 Bluetooth
> chip. Although it shares the same IP core as the WCN6855, the QCA6698
> has different RF components and RAM sizes, necessitating new firmware
> files. This change allows for the configuration of NVM and rampatch in
> DT.
> 
> 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.

Two separate commits, one for NVM, another one for RAM patch.

> 
> 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.bin", "QCA6698/hpbtfw21.tlv";
> firmware-name = "QCA6698/hpnv21", "QCA6698/hpbtfw21.tlv";
> firmware-name = "QCA6698/hpnv21.bin";
> 
> Signed-off-by: Cheng Jiang <quic_chejiang@xxxxxxxxxxx>
> ---
>  drivers/bluetooth/btqca.c   | 154 ++++++++++++++++++++++++++----------
>  drivers/bluetooth/btqca.h   |   5 +-
>  drivers/bluetooth/hci_qca.c |  21 ++++-
>  3 files changed, 134 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
> index dfbbac922..e8b89b8cc 100644
> --- a/drivers/bluetooth/btqca.c
> +++ b/drivers/bluetooth/btqca.c
> @@ -272,6 +272,31 @@ int qca_send_pre_shutdown_cmd(struct hci_dev *hdev)
>  }
>  EXPORT_SYMBOL_GPL(qca_send_pre_shutdown_cmd);
>  
> +static int qca_get_alt_nvm_path(char *path, size_t max_size)

int is usually for errors, the code suggests bool return type.

> +{
> +	char fwname[64];
> +	const char *suffix;
> +
> +	suffix = strrchr(path, '.');
> +
> +	if (!suffix)
> +		return 0;
> +
> +	strscpy(fwname, path, strlen(path));

64 bytes ought to be enough for anybody, correct?

> +	fwname[suffix - path] = 0;

with path = "qcom/sc7180/Oh.My.Device/name" this is broken.

> +
> +	snprintf(fwname, sizeof(fwname), "%s.bin", fwname);
> +
> +	/* If nvm file is already the default one, return false to
> +	 * skip the retry.
> +	 */
> +	if (strcmp(fwname, path) == 0)
> +		return 0;
> +
> +	snprintf(path, max_size, "%s", fwname);
> +	return 1;
> +}
> +
>  static int qca_tlv_check_data(struct hci_dev *hdev,
>  			       struct qca_fw_config *config,
>  			       u8 *fw_data, size_t fw_size,
> @@ -564,6 +589,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))) {

Please, don't rewrite the config. The file may be not present now, but
it will reappear later (e.g. when rootfs gets mounted).

> +			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,15 +768,38 @@ 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";

Didn't you get the 'set but unused' here?

> +	}
> +
> +	if (bid == 0x0)

0x0 or 0xff?

> +		snprintf(fwname, max_size, "qca/%s.bin", firmware_name);
> +	else if (bid & 0xff00)
> +		snprintf(fwname, max_size, "qca/%s.b%x", firmware_name, bid);

Doesn't ".b%02x" work in this case too?

> +	else
> +		snprintf(fwname, max_size, "qca/%s.b%02x", firmware_name, bid);
> +}
> +
>  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)
> +		   const char *firmware_name, const char *rampatch_name)
>  {
>  	struct qca_fw_config config = {};
>  	int err;
>  	u8 rom_ver = 0;
>  	u32 soc_ver;
>  	u16 boardid = 0;
> +	const char *suffix;
>  
>  	bt_dev_dbg(hdev, "QCA setup on UART");
>  
> @@ -761,44 +822,48 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
>  
>  	/* Download rampatch file */
>  	config.type = TLV_TYPE_PATCH;
> -	switch (soc_type) {
> -	case QCA_WCN3990:
> -	case QCA_WCN3991:
> -	case QCA_WCN3998:
> -		snprintf(config.fwname, sizeof(config.fwname),
> -			 "qca/crbtfw%02x.tlv", rom_ver);
> -		break;
> -	case QCA_WCN3988:
> -		snprintf(config.fwname, sizeof(config.fwname),
> -			 "qca/apbtfw%02x.tlv", rom_ver);
> -		break;
> -	case QCA_QCA2066:
> -		snprintf(config.fwname, sizeof(config.fwname),
> -			 "qca/hpbtfw%02x.tlv", rom_ver);
> -		break;
> -	case QCA_QCA6390:
> -		snprintf(config.fwname, sizeof(config.fwname),
> -			 "qca/htbtfw%02x.tlv", rom_ver);
> -		break;
> -	case QCA_WCN6750:
> -		/* Choose mbn file by default.If mbn file is not found
> -		 * then choose tlv file
> -		 */
> -		config.type = ELF_TYPE_PATCH;
> -		snprintf(config.fwname, sizeof(config.fwname),
> -			 "qca/msbtfw%02x.mbn", rom_ver);
> -		break;
> -	case QCA_WCN6855:
> -		snprintf(config.fwname, sizeof(config.fwname),
> -			 "qca/hpbtfw%02x.tlv", rom_ver);
> -		break;
> -	case QCA_WCN7850:
> -		snprintf(config.fwname, sizeof(config.fwname),
> -			 "qca/hmtbtfw%02x.tlv", rom_ver);
> -		break;
> -	default:
> -		snprintf(config.fwname, sizeof(config.fwname),
> -			 "qca/rampatch_%08x.bin", soc_ver);
> +	if (rampatch_name) {
> +		snprintf(config.fwname, sizeof(config.fwname), "qca/%s", rampatch_name);
> +	} else {
> +		switch (soc_type) {
> +		case QCA_WCN3990:
> +		case QCA_WCN3991:
> +		case QCA_WCN3998:
> +			snprintf(config.fwname, sizeof(config.fwname),
> +				 "qca/crbtfw%02x.tlv", rom_ver);
> +			break;
> +		case QCA_WCN3988:
> +			snprintf(config.fwname, sizeof(config.fwname),
> +				 "qca/apbtfw%02x.tlv", rom_ver);
> +			break;
> +		case QCA_QCA2066:
> +			snprintf(config.fwname, sizeof(config.fwname),
> +				 "qca/hpbtfw%02x.tlv", rom_ver);
> +			break;
> +		case QCA_QCA6390:
> +			snprintf(config.fwname, sizeof(config.fwname),
> +				 "qca/htbtfw%02x.tlv", rom_ver);
> +			break;
> +		case QCA_WCN6750:
> +			/* Choose mbn file by default.If mbn file is not found
> +			 * then choose tlv file
> +			 */
> +			config.type = ELF_TYPE_PATCH;
> +			snprintf(config.fwname, sizeof(config.fwname),
> +				 "qca/msbtfw%02x.mbn", rom_ver);
> +			break;
> +		case QCA_WCN6855:
> +			snprintf(config.fwname, sizeof(config.fwname),
> +				 "qca/hpbtfw%02x.tlv", rom_ver);
> +			break;
> +		case QCA_WCN7850:
> +			snprintf(config.fwname, sizeof(config.fwname),
> +				 "qca/hmtbtfw%02x.tlv", rom_ver);
> +			break;
> +		default:
> +			snprintf(config.fwname, sizeof(config.fwname),
> +				 "qca/rampatch_%08x.bin", soc_ver);
> +		}
>  	}
>  
>  	err = qca_download_firmware(hdev, &config, soc_type, rom_ver);
> @@ -816,8 +881,15 @@ 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 + 1) != '\0' && *(suffix + 1) != '.') {
> +			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:
> diff --git a/drivers/bluetooth/btqca.h b/drivers/bluetooth/btqca.h
> index bb5207d7a..9d28c8800 100644
> --- a/drivers/bluetooth/btqca.h
> +++ b/drivers/bluetooth/btqca.h
> @@ -161,7 +161,7 @@ enum qca_btsoc_type {
>  int qca_set_bdaddr_rome(struct hci_dev *hdev, const bdaddr_t *bdaddr);
>  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);
> +		   const char *firmware_name, const char *rampatch_name);
>  int qca_read_soc_version(struct hci_dev *hdev, struct qca_btsoc_version *ver,
>  			 enum qca_btsoc_type);
>  int qca_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr);
> @@ -176,7 +176,8 @@ static inline int qca_set_bdaddr_rome(struct hci_dev *hdev, const bdaddr_t *bdad
>  static inline 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)
> +				 const char *firmware_name,
> +				 const char *rampatch_name)
>  {
>  	return -EOPNOTSUPP;
>  }
> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
> index 37129e6cb..c566d0bf2 100644
> --- a/drivers/bluetooth/hci_qca.c
> +++ b/drivers/bluetooth/hci_qca.c
> @@ -229,6 +229,7 @@ struct qca_serdev {
>  	u32 oper_speed;
>  	bool bdaddr_property_broken;
>  	const char *firmware_name;
> +	const char *rampatch_name;
>  };
>  
>  static int qca_regulator_enable(struct qca_serdev *qcadev);
> @@ -264,6 +265,17 @@ static const char *qca_get_firmware_name(struct hci_uart *hu)
>  	}
>  }
>  
> +static const char *qca_get_rampatch_name(struct hci_uart *hu)
> +{
> +	if (hu->serdev) {
> +		struct qca_serdev *qsd = serdev_device_get_drvdata(hu->serdev);
> +
> +		return qsd->rampatch_name;
> +	} else {
> +		return NULL;
> +	}
> +}
> +
>  static void __serial_clock_on(struct tty_struct *tty)
>  {
>  	/* TODO: Some chipset requires to enable UART clock on client
> @@ -1855,6 +1867,7 @@ static int qca_setup(struct hci_uart *hu)
>  	unsigned int retries = 0;
>  	enum qca_btsoc_type soc_type = qca_soc_type(hu);
>  	const char *firmware_name = qca_get_firmware_name(hu);
> +	const char *rampatch_name = qca_get_rampatch_name(hu);
>  	int ret;
>  	struct qca_btsoc_version ver;
>  	struct qca_serdev *qcadev;
> @@ -1963,7 +1976,7 @@ static int qca_setup(struct hci_uart *hu)
>  
>  	/* Setup patch / NVM configurations */
>  	ret = qca_uart_setup(hdev, qca_baudrate, soc_type, ver,
> -			firmware_name);
> +			firmware_name, rampatch_name);
>  	if (!ret) {
>  		clear_bit(QCA_IBS_DISABLED, &qca->flags);
>  		qca_debugfs_init(hdev);
> @@ -2309,8 +2322,10 @@ static int qca_serdev_probe(struct serdev_device *serdev)
>  	qcadev->serdev_hu.serdev = serdev;
>  	data = device_get_match_data(&serdev->dev);
>  	serdev_device_set_drvdata(serdev, qcadev);
> -	device_property_read_string(&serdev->dev, "firmware-name",
> -					 &qcadev->firmware_name);
> +	of_property_read_string_index(serdev->dev.of_node, "firmware-name",
> +					 0, &qcadev->firmware_name);
> +	of_property_read_string_index(serdev->dev.of_node, "firmware-name",
> +					 1, &qcadev->rampatch_name);
>  	device_property_read_u32(&serdev->dev, "max-speed",
>  				 &qcadev->oper_speed);
>  	if (!qcadev->oper_speed)
> -- 
> 2.25.1
> 

-- 
With best wishes
Dmitry




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux