Re: [PATCH v2 6/8] Bluetooth: hci_sync: Add NVMEM-backed BD address retrieval

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Retrieval of NVMEM data during `hci_dev_setup_sync()` bypasses the probe deferral mechanism and ignores errors, causing silent initialization failures.
--

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

Bluetooth: hci_sync: Add NVMEM-backed BD address retrieval

This commit adds support for retrieving the Bluetooth BD address from an
NVMEM cell using the NVMEM framework. It introduces quirks to indicate when
the address should be read from NVMEM and whether the address is stored in
big-endian byte order.

> diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
> index fd3aacdea512a..589ccdfa26c12 100644
> --- a/net/bluetooth/hci_sync.c
> +++ b/net/bluetooth/hci_sync.c
[ ... ]
> @@ -3588,6 +3589,37 @@ int hci_powered_update_sync(struct hci_dev *hdev)
>  	return 0;
>  }
>  
> +/**
> + * hci_dev_get_bd_addr_from_nvmem - Get the Bluetooth Device Address
[ ... ]
> +static int hci_dev_get_bd_addr_from_nvmem(struct hci_dev *hdev)
> +{
> +	struct device_node *np = dev_of_node(hdev->dev.parent);
> +	u8 ba[sizeof(bdaddr_t)];
> +	int err;
> +
> +	if (!np)
> +		return -ENODEV;
> +
> +	err = of_get_nvmem_eui48(np, "local-bd-address", ba);
> +	if (err)
> +		return err;
> +
> +	if (hci_test_quirk(hdev, HCI_QUIRK_BDADDR_NVMEM_BE))
> +		baswap(&hdev->public_addr, (bdaddr_t *)ba);
> +	else
> +		bacpy(&hdev->public_addr, (bdaddr_t *)ba);
> +
> +	return 0;
> +}
[ ... ]
> @@ -5042,12 +5074,17 @@ static int hci_dev_setup_sync(struct hci_dev *hdev)
[ ... ]
>  		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);

Are errors from reading the NVMEM cell intentionally ignored here?

If of_get_nvmem_eui48() fails, the error is lost and the controller might
be initialized with an all-zero BD address.

Also, if the NVMEM provider is not yet probed, of_get_nvmem_eui48() will
return -EPROBE_DEFER. Because this is called from hci_dev_setup_sync()
rather than the transport driver's probe path, will this bypass the kernel's
probe deferral mechanism?

Could this leave the device stuck in the HCI_UNCONFIGURED state instead of
retrying later? To properly support probe deferral, should NVMEM resources
be queried in the transport driver's probe function before
hci_register_dev() is called?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260507-block-as-nvmem-v2-0-bf17edd5134e@oss.qualcomm.com?part=6