Re: [PATCH 1/3] Bluetooth: qca: add QCC2072 support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Uninitialized `config.calib_name` when using the `firmware-name` device tree property causes failure to load calibration data.

Pre-existing issues:
- [High] Unbounded polling loop on the TX queue can lead to a permanent task hang.
--

--- Patch [1]: [PATCH 1/3] Bluetooth: qca: add QCC2072 support ---
commit a8d57bcf86987ec0c8079a742e8a1f1f50322cdc
Author: Rahul Samana <rahul.samana@oss.qualcomm.com>

    Bluetooth: qca: add QCC2072 support
    
    Add QCC2072 as a supported Qualcomm Bluetooth controller.

> diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
> index 10c496eaea2c1..69427cc7eb5a9 100644
> --- a/drivers/bluetooth/btqca.c
> +++ b/drivers/bluetooth/btqca.c

[ ... ]

> @@ -614,6 +662,11 @@ static int qca_download_firmware(struct hci_dev *hdev,
>  	memcpy(data, fw->data, size);
>  	release_firmware(fw);
>  
> +	if (soc_type == QCA_QCC2072 && config->type == TLV_TYPE_NVM)
> +		qca_combine_nvm_calib(hdev, &data, &size,
> +				      config->calib_name,
> +				      sizeof(config->calib_name));
> +
>  	ret = qca_tlv_check_data(hdev, config, data, size, soc_type);

[Severity: High]
Does this code handle custom firmware names properly?

In qca_uart_setup(), if the firmware-name device tree property is set, the
driver enters the if (firmware_name) branch:

	if (firmware_name) {
		/* The firmware name has an extension, use it directly */
		if (qca_filename_has_extension(firmware_name)) {

When this path is taken, config.calib_name is never initialized. Then,
qca_download_firmware() unconditionally passes the empty string to
qca_combine_nvm_calib() for the QCC2072 controller.

This causes request_firmware() to fail with -EINVAL, logging a spurious error
and skipping the calibration data append.

Could we initialize config.calib_name when a custom firmware name is provided
via the device tree?

[ ... ]

> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
> index 1222f97800f4a..6fe3aab688e64 100644
> --- a/drivers/bluetooth/hci_qca.c
> +++ b/drivers/bluetooth/hci_qca.c

[ ... ]

> @@ -1375,6 +1375,7 @@ static int qca_set_baudrate(struct hci_dev *hdev, uint8_t baudrate)
>  
>  	/* Give the controller time to process the request */
>  	switch (qca_soc_type(hu)) {
> +	case QCA_QCC2072:
>  	case QCA_WCN3950:

[Severity: High]
This is a pre-existing issue, but can this lead to a permanent task hang?

Just above this switch statement in qca_set_baudrate(), there is a polling
loop that checks the transmit queue:

	while (!skb_queue_empty(&qca->txq))
		usleep_range(100, 200);

This loop polls the transmit queue indefinitely without a bounding timeout.

If the UART hardware stops accepting bytes, for example due to stuck flow
control or a hardware crash, the queue is never emptied and the loop will spin
indefinitely.

Could a timeout mechanism be added here to prevent a task hang if the hardware
malfunctions?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260723-rb3-industrial-bt-uart-v1-0-3c8e580c09d1@oss.qualcomm.com?part=1