Re: [PATCH RFC 3/5] Bluetooth: hci_qca: Support BD address from an nvmem cell

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] bluetooth: device wrongly left HCI_UNCONFIGURED after nvmem MAC init
--

commit 97b88af275cdce36b0a1523d7d18ad5eb2710216
Author: Abel Vesa <abel.vesa@oss.qualcomm.com>

Bluetooth: hci_qca: Support BD address from an nvmem cell

This commit reads an optional Bluetooth device address from an nvmem cell
and programs it into the controller during setup. This allows sourcing
BD_ADDR from firmware-provisioned storage.

> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
> --- a/drivers/bluetooth/hci_qca.c
> +++ b/drivers/bluetooth/hci_qca.c

[ ... ]

> @@ -2079,6 +2082,16 @@ static int qca_setup(struct hci_uart *hu)
>  	else
>  		hu->hdev->set_bdaddr = qca_set_bdaddr;
>  
> +	/* Program a BD address sourced from an nvmem cell, if provided. */
> +	if (qcadev && qcadev->bdaddr_valid) {
> +		int berr = hu->hdev->set_bdaddr(hdev, &qcadev->bdaddr);
> +
> +		if (berr)
> +			bt_dev_warn(hdev, "Failed to set nvmem bd-address: %d", berr);
> +		else
> +			bt_dev_info(hdev, "Set bd-address %pMR from nvmem", &qcadev->bdaddr);
> +	}

[Severity: High]
Does this leave the device wrongly flagged as HCI_UNCONFIGURED?

When the controller boots with a default MAC, qca_check_bdaddr() detects it
and sets HCI_QUIRK_USE_BDADDR_PROPERTY. Here in qca_setup(), the nvmem MAC
is programmed into the controller using hu->hdev->set_bdaddr(), but it
appears it is not copied to hdev->public_addr, and
HCI_QUIRK_USE_BDADDR_PROPERTY is not cleared.

When initialization returns to the core stack, hci_dev_setup_sync() sees
HCI_QUIRK_USE_BDADDR_PROPERTY is still set and hdev->public_addr is
BDADDR_ANY:

net/bluetooth/hci_sync.c:hci_dev_setup_sync() {
    ...
    if (invalid_bdaddr && bacmp(&hdev->public_addr, BDADDR_ANY) &&
        hdev->set_bdaddr) {
        ret = hdev->set_bdaddr(hdev, &hdev->public_addr);
        if (!ret)
            invalid_bdaddr = false;
    }
    ...
}

Because hdev->public_addr is still BDADDR_ANY, invalid_bdaddr remains
true.

The core stack then marks the device as HCI_UNCONFIGURED:

net/bluetooth/hci_sync.c:hci_dev_setup_sync() {
    ...
    if (hci_test_quirk(hdev, HCI_QUIRK_EXTERNAL_CONFIG) ||
        invalid_bdaddr)
        hci_dev_set_flag(hdev, HCI_UNCONFIGURED);
    ...
}

Will this cause the core Bluetooth stack to refuse to bring it up
automatically, requiring manual userspace intervention and breaking the
automatic nvmem MAC provisioning?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014-b4-nvmem-efivar-v1-0-187c85f65018@oss.qualcomm.com?part=3