Re: [PATCH] Bluetooth: hci_sync: Fix not using conn_timeout

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

> 	/* If the controller supports the LE Connection Update
> 	 * command, enable the corresponding event.
> 	 */
> @@ -5188,7 +5188,7 @@ static int hci_le_ext_create_conn_sync(struct hci_dev *hdev,
> 	return __hci_cmd_sync_status_sk(hdev, HCI_OP_LE_EXT_CREATE_CONN,
> 					plen, data,
> 					HCI_EV_LE_ENHANCED_CONN_COMPLETE,
> -					HCI_CMD_TIMEOUT, NULL);
> +					conn->conn_timeout, NULL);
> }
> 
> int hci_le_create_conn_sync(struct hci_dev *hdev, struct hci_conn *conn)
> @@ -5274,8 +5274,11 @@ int hci_le_create_conn_sync(struct hci_dev *hdev, struct hci_conn *conn)
> 	cp.max_ce_len = cpu_to_le16(0x0000);
> 
> 	err = __hci_cmd_sync_status_sk(hdev, HCI_OP_LE_CREATE_CONN,
> -				       sizeof(cp), &cp, HCI_EV_LE_CONN_COMPLETE,
> -				       HCI_CMD_TIMEOUT, NULL);
> +				       sizeof(cp), &cp,
> +				       ll_privacy_capable(hdev) ?
> +				       HCI_EV_LE_ENHANCED_CONN_COMPLETE :
> +				       HCI_EV_LE_CONN_COMPLETE,
> +				       conn->conn_timeout, NULL);

This is stupid. We should not be using LE Create Connection in the first place here. If the LE Extended Create Connection is available, we unmask its event and also use the command.

Regards

Marcel




[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux