Re: [PATCH v2] bluetooth: bcm: Use HCI_QUIRK_USE_BDADDR_PROPERTY

[Andre Heider <a.heider@gmail.com>]

Hi,

On 19/11/2019 07:16, Marcel Holtmann wrote:
> Hi Andre,
> 
>> Some devices ship with the controller default address, like the
>> Orange Pi 3 (BCM4345C5).
>>
>> Allow the bootloader to set a valid address through the device tree.
>>
>> Signed-off-by: Andre Heider <a.heider@gmail.com>
>> ---
>> drivers/bluetooth/btbcm.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
>> index 2d2e6d862068..9d16162d01ea 100644
>> --- a/drivers/bluetooth/btbcm.c
>> +++ b/drivers/bluetooth/btbcm.c
>> @@ -439,6 +439,7 @@ int btbcm_finalize(struct hci_dev *hdev)
>> 	btbcm_check_bdaddr(hdev);
>>
>> 	set_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks);
>> +	set_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &hdev->quirks);
>>
>> 	return 0;
>> }
> 
> have you actually tested this? I might be mistaken, but the code that I read in hci_dev_do_open() would drop this into unconfigured state since HCI_QURIK_INVALID_BDADDR is still set.

I thought so, but double-checking something obviously failed...

What would be an acceptable solution to this 
HCI_QUIRK_USE_BDADDR_PROPERTY|HCI_QUIRK_INVALID_BDADDR situation?

Getting rid of the quirk in the driver in e.g. set_bdaddr() doesn't 
sound right.

How about:

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 04bc79359a17..7bc384be89f8 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1470,7 +1470,8 @@ static int hci_dev_do_open(struct hci_dev *hdev)
          * start up as unconfigured.
          */
         if (test_bit(HCI_QUIRK_EXTERNAL_CONFIG, &hdev->quirks) ||
-           test_bit(HCI_QUIRK_INVALID_BDADDR, &hdev->quirks))
+           (test_bit(HCI_QUIRK_INVALID_BDADDR, &hdev->quirks) &&
+            !test_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &hdev->quirks)))
             hci_dev_set_flag(hdev, HCI_UNCONFIGURED);

         /* For an unconfigured controller it is required to

That works for me (double-checked this time ;)

Thanks,
Andre