Re: [PATCH v7 7/9] Bluetooth: hci_sync: Add NVMEM-backed BD address retrieval

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Asynchronous NVMEM read silently swallows `-EPROBE_DEFER`, permanently leaving the device without an address.
- [High] Missing `HCI_QUIRK_USE_BDADDR_NVMEM` checks in MGMT interface cause unconfigured devices to report 0 missing options.
--

commit b5310ae697f8c9e43fa71d6c9350c84adcaff918
Author: Loic Poulain <loic.poulain@oss.qualcomm.com>

Bluetooth: hci_sync: Add NVMEM-backed BD address retrieval

This commit adds support for reading the Bluetooth BD address from a
'local-bd-address' NVMEM cell, similar to Ethernet or WiFi MAC addresses.
It introduces two new quirks, HCI_QUIRK_USE_BDADDR_NVMEM and
HCI_QUIRK_BDADDR_NVMEM_BE, to indicate whether the platform should attempt
to read the address from NVMEM and whether it is stored in big-endian byte
order.

> diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
> index 3be8c3581c6ca..c0ed7fb14d19b 100644
> --- a/net/bluetooth/hci_sync.c
> +++ b/net/bluetooth/hci_sync.c

[ ... ]

> @@ -5072,12 +5106,17 @@ static int hci_dev_setup_sync(struct hci_dev *hdev)
>  	 * its setup callback.
>  	 */
>  	invalid_bdaddr = hci_test_quirk(hdev, HCI_QUIRK_INVALID_BDADDR) ||
> -			 hci_test_quirk(hdev, HCI_QUIRK_USE_BDADDR_PROPERTY);
> +			 hci_test_quirk(hdev, HCI_QUIRK_USE_BDADDR_PROPERTY) ||
> +			 hci_test_quirk(hdev, HCI_QUIRK_USE_BDADDR_NVMEM);

[Severity: High]
Does net/bluetooth/mgmt.c also need to be updated to check for
HCI_QUIRK_USE_BDADDR_NVMEM?

In get_missing_options() and is_configured(), there are similar checks for
HCI_QUIRK_INVALID_BDADDR and HCI_QUIRK_USE_BDADDR_PROPERTY. If this new
quirk is missing from those checks, userspace might be informed that it
has 0 missing options despite the device being unconfigured, preventing
userspace from resolving the unconfigured state.

>  	if (!ret) {
>  		if (hci_test_quirk(hdev, HCI_QUIRK_USE_BDADDR_PROPERTY) &&
>  		    !bacmp(&hdev->public_addr, BDADDR_ANY))
>  			hci_dev_get_bd_addr_from_property(hdev);
>  
> +		if (hci_test_quirk(hdev, HCI_QUIRK_USE_BDADDR_NVMEM) &&
> +		    !bacmp(&hdev->public_addr, BDADDR_ANY))
> +			hci_dev_get_bd_addr_from_nvmem(hdev);

[Severity: High]
Does this silently swallow -EPROBE_DEFER?

If of_get_nvmem_eui48() returns -EPROBE_DEFER because the NVMEM provider
has not yet probed, the return value of hci_dev_get_bd_addr_from_nvmem()
is ignored here. Since hci_dev_setup_sync() runs asynchronously in a
workqueue, the error cannot be propagated, and the device might be
permanently left without an address.

> +
>  		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;
>  		}
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260701-block-as-nvmem-v7-0-3fe8205ef0a8@oss.qualcomm.com?part=7