Hi Andre,
>>> This patch introduces a new helper, which uses the HCI request
>>> framework, for creating LE connectons. All the handling is now
>>> done by this function so we can remove the hci_cs_le_create_conn()
>>> event handler.
>>>
>>> This patch also removes the old hci_le_create_connection() since
>>> it is not used anymore.
>>>
>>> Signed-off-by: Andre Guedes <andre.guedes@xxxxxxxxxxxxx>
>>> ---
>>> include/net/bluetooth/hci_core.h | 2 ++
>>> net/bluetooth/hci_conn.c | 30 +++++-----------------
>>> net/bluetooth/hci_core.c | 55 ++++++++++++++++++++++++++++++++++++++++
>>> net/bluetooth/hci_event.c | 31 ----------------------
>>> 4 files changed, 63 insertions(+), 55 deletions(-)
>>>
>>> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
>>> index c065527..6ac542c 100644
>>> --- a/include/net/bluetooth/hci_core.h
>>> +++ b/include/net/bluetooth/hci_core.h
>>> @@ -1188,6 +1188,8 @@ void hci_le_start_enc(struct hci_conn *conn, __le16 ediv, __u8 rand[8],
>>>
>>> u8 bdaddr_to_le(u8 bdaddr_type);
>>>
>>> +int hci_create_le_conn(struct hci_dev *hdev, bdaddr_t *addr, u8 type);
>>> +
>>> #define SCO_AIRMODE_MASK 0x0003
>>> #define SCO_AIRMODE_CVSD 0x0000
>>> #define SCO_AIRMODE_TRANSP 0x0003
>>> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
>>> index 2a220a8..31f6712 100644
>>> --- a/net/bluetooth/hci_conn.c
>>> +++ b/net/bluetooth/hci_conn.c
>>> @@ -49,29 +49,6 @@ static const struct sco_param sco_param_wideband[] = {
>>> { EDR_ESCO_MASK | ESCO_EV3, 0x0008 }, /* T1 */
>>> };
>>>
>>> -static void hci_le_create_connection(struct hci_conn *conn)
>>> -{
>>> - struct hci_dev *hdev = conn->hdev;
>>> - struct hci_cp_le_create_conn cp;
>>> -
>>> - memset(&cp, 0, sizeof(cp));
>>> - cp.scan_interval = __constant_cpu_to_le16(0x0060);
>>> - cp.scan_window = __constant_cpu_to_le16(0x0030);
>>> - bacpy(&cp.peer_addr, &conn->dst);
>>> - cp.peer_addr_type = conn->dst_type;
>>> - if (bacmp(&hdev->bdaddr, BDADDR_ANY))
>>> - cp.own_address_type = ADDR_LE_DEV_PUBLIC;
>>> - else
>>> - cp.own_address_type = ADDR_LE_DEV_RANDOM;
>>> - cp.conn_interval_min = __constant_cpu_to_le16(0x0028);
>>> - cp.conn_interval_max = __constant_cpu_to_le16(0x0038);
>>> - cp.supervision_timeout = __constant_cpu_to_le16(0x002a);
>>> - cp.min_ce_len = __constant_cpu_to_le16(0x0000);
>>> - cp.max_ce_len = __constant_cpu_to_le16(0x0000);
>>> -
>>> - hci_send_cmd(hdev, HCI_OP_LE_CREATE_CONN, sizeof(cp), &cp);
>>> -}
>>> -
>>> static void hci_le_create_connection_cancel(struct hci_conn *conn)
>>> {
>>> hci_send_cmd(conn->hdev, HCI_OP_LE_CREATE_CONN_CANCEL, 0, NULL);
>>> @@ -549,6 +526,7 @@ static struct hci_conn *hci_connect_le(struct hci_dev *hdev, bdaddr_t *dst,
>>> u8 dst_type, u8 sec_level, u8 auth_type)
>>> {
>>> struct hci_conn *conn;
>>> + int err;
>>>
>>> if (test_bit(HCI_ADVERTISING, &hdev->flags))
>>> return ERR_PTR(-ENOTSUPP);
>>> @@ -569,7 +547,11 @@ static struct hci_conn *hci_connect_le(struct hci_dev *hdev, bdaddr_t *dst,
>>> conn->link_mode |= HCI_LM_MASTER;
>>> conn->sec_level = BT_SECURITY_LOW;
>>>
>>> - hci_le_create_connection(conn);
>>> + err = hci_create_le_conn(hdev, &conn->dst, conn->dst_type);
>>> + if (err) {
>>> + hci_conn_del(conn);
>>> + return ERR_PTR(err);
>>> + }
>>
>> I am wondering why we do not keep the void function here and handle the error directly in hci_create_le_conn.
>>
>> Any reason why you are doing it this way?
>
> Yes, hci_create_le_conn() may return error if hci_req_run() returns error (e.g. ENOMEM). For that case, we have to destroy the hci_conn object created by hci_conn_add() otherwise this object will leak. Besides, if the HCI command could not be sent to controller we should notify the caller about this (e.g. connect() returns error to the user or MGMT_OP_PAIR_DEVICE completes with error)
you are not answering my question. Why is the error handling not done directly in hci_create_le_conn? Do it where hci_req_run fails and not some other place.
Regards
Marcel
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
