Hi Luiz,
>>> When using hci_le_create_conn_sync it shall wait for the conn_timeout
>>> since the connection complete may take longer than just 2 seconds.
>>>
>>> Also fix the masking of HCI_EV_LE_ENHANCED_CONN_COMPLETE and
>>> HCI_EV_LE_CONN_COMPLETE so they are never both set so we can predict
>>> which one the controller will use in case of HCI_OP_LE_CREATE_CONN.
>>>
>>> Fixes: 6cd29ec6ae5e3 ("Bluetooth: hci_sync: Wait for proper events when connecting LE")
>>> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx>
>>> ---
>>> net/bluetooth/hci_sync.c | 27 +++++++++++++++------------
>>> 1 file changed, 15 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
>>> index 9dbf007e3dc7..002f9c5b5371 100644
>>> --- a/net/bluetooth/hci_sync.c
>>> +++ b/net/bluetooth/hci_sync.c
>>> @@ -3265,11 +3265,17 @@ static int hci_le_set_event_mask_sync(struct hci_dev *hdev)
>>> if (hdev->le_features[0] & HCI_LE_DATA_LEN_EXT)
>>> events[0] |= 0x40; /* LE Data Length Change */
>>>
>>> - /* If the controller supports LL Privacy feature, enable
>>> - * the corresponding event.
>>> + /* If the controller supports LL Privacy feature or LE Extended
>>> + * Create Connection, enable the corresponding event.
>>> */
>>> - if (hdev->le_features[0] & HCI_LE_LL_PRIVACY)
>>> + if (ll_privacy_capable(hdev) || hdev->commands[37] & 0x80) {
>>> events[1] |= 0x02; /* LE Enhanced Connection Complete */
>>> + } else if (hdev->commands[26] & 0x10) {
>>> + /* If the controller supports the LE Create Connection
>>> + * command, enable the corresponding event.
>>> + */
>>> + events[0] |= 0x01; /* LE Connection Complete */
>>> + }
>>>
>>> /* If the controller supports Extended Scanner Filter
>>> * Policies, enable the corresponding event.
>>> @@ -3289,12 +3295,6 @@ static int hci_le_set_event_mask_sync(struct hci_dev *hdev)
>>> if (hdev->commands[26] & 0x08)
>>> events[0] |= 0x02; /* LE Advertising Report */
>>>
>>> - /* If the controller supports the LE Create Connection
>>> - * command, enable the corresponding event.
>>> - */
>>> - if (hdev->commands[26] & 0x10)
>>> - events[0] |= 0x01; /* LE Connection Complete */
>>> -
>>
>> I do not understand why you are trying to intermix this with LL Privacy. If the controller supports the LE Extended Create Connection, then we should enable that event. No matter if we have LL Privacy supported or enabled.
>>
>> If we have other code that intermixes this, then it needs to be untangled.
>>
>> What we should be doing is to only support LL Privacy if we also have support for LE Extended Create Connection command, but the assumption the other way around makes no sense.
>
> The spec does allow the use of LE Create Connection and Enhanced
> Connection Complete since it does support own_address_type to be
> 0x02/0x03 which means LL Privacy, I believe LE Extented Create
> Connection was introduced much later than LL Privacy so we may find
> controllers supporting LL Privacy with LE Create Connection but
> without support for LE Extended Create Connection.
my memory is getting old and even while I actively worked on 4.2 and 5.x specs, I keep forgetting details.
So here is the thing from the latest specs for the LE Enhanced Connection Complete event:
C24: Mandatory if the Controller supports Connection State and either LE Feature (LL Privacy) or LE Feature (Extended Advertising) is supported, otherwise optional if the Controller supports Connection State, otherwise excluded.
That means that when either of these features are listed as supported, we unmask the event. Keep the LE Connection Complete also unmasked since the spec is clear that if both are unmasked the “enhanced” version shall be used.
Maybe introduce an use_enhanced_conn_complete() macro.
Regards
Marcel
