2012-10-16 15:01:02

by Andrei Emeltchenko

[permalink] [raw]
Subject: [RFCv0 0/8] Handling physical and logical link

From: Andrei Emeltchenko <[email protected]>

Set of patches adding handling for physical and logical link
complete events.

Andrei Emeltchenko (8):
Bluetooth: AMP: Process Physical Link Complete evt
Bluetooth: AMP: Process Logical Link complete evt
Bluetooth: Add logical link create function
Bluetooth: AMP: Process Disc Logical Link
Bluetooth: AMP: Process Disc Physical Link complete evt
Bluetooth: AMP: Remove hci_conn receiving error command status
Bluetooth: Disconnect logical link when deleteing chan
Bluetooth: Add put(hcon) when deleting hchan

include/net/bluetooth/amp.h | 3 +
include/net/bluetooth/l2cap.h | 1 +
net/bluetooth/amp.c | 67 ++++++++++++++++
net/bluetooth/hci_conn.c | 2 +
net/bluetooth/hci_event.c | 174 ++++++++++++++++++++++++++++++++++++++++-
net/bluetooth/l2cap_core.c | 33 ++++++++
6 files changed, 276 insertions(+), 4 deletions(-)

--
1.7.9.5



2012-10-31 18:51:36

by Gustavo Padovan

[permalink] [raw]
Subject: Re: [PATCHv1 11/12] Bluetooth: AMP: Use l2cap_physical_cfm in phylink complete evt

Hi Andrei,

* Andrei Emeltchenko <[email protected]> [2012-10-31 15:46:36 +0200]:

> From: Andrei Emeltchenko <[email protected]>
>
> When receiving HCI Phylink Complete event run amp_physical_cfm
> which initialize BR/EDR L2CAP channel associated with High Speed
> link and run l2cap_physical_cfm which shall send L2CAP Create
> Chan Request.
>
> Signed-off-by: Andrei Emeltchenko <[email protected]>
> Acked-by: Marcel Holtmann <[email protected]>
> ---
> include/net/bluetooth/amp.h | 1 +
> include/net/bluetooth/l2cap.h | 1 +
> net/bluetooth/amp.c | 24 ++++++++++++++++++++++++
> net/bluetooth/hci_event.c | 15 ++-------------
> 4 files changed, 28 insertions(+), 13 deletions(-)

Patches 1-11 have been applied to bluetooth-next. Thanks.

Gustavo

2012-10-31 16:24:02

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCHv1 12/12] Bluetooth: Process Create Chan Request

Hi Andrei,

> Add processing L2CAP Create Chan Request. When channel is created
> save associated high speed link hs_hcon.
>
> Signed-off-by: Andrei Emeltchenko <[email protected]>
> ---
> net/bluetooth/l2cap_core.c | 63 ++++++++++++++++++++++++++++++--------------
> 1 file changed, 43 insertions(+), 20 deletions(-)
>
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index bdffc4c..783ffae 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -4237,7 +4237,9 @@ static int l2cap_create_channel_req(struct l2cap_conn *conn,
> u16 cmd_len, void *data)
> {
> struct l2cap_create_chan_req *req = data;
> + struct l2cap_create_chan_rsp rsp;
> struct l2cap_chan *chan;
> + struct hci_dev *hdev = NULL;

why is this one set to NULL. Can we avoid that please.

Regards

Marcel



2012-10-31 13:46:36

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCHv1 11/12] Bluetooth: AMP: Use l2cap_physical_cfm in phylink complete evt

From: Andrei Emeltchenko <[email protected]>

When receiving HCI Phylink Complete event run amp_physical_cfm
which initialize BR/EDR L2CAP channel associated with High Speed
link and run l2cap_physical_cfm which shall send L2CAP Create
Chan Request.

Signed-off-by: Andrei Emeltchenko <[email protected]>
Acked-by: Marcel Holtmann <[email protected]>
---
include/net/bluetooth/amp.h | 1 +
include/net/bluetooth/l2cap.h | 1 +
net/bluetooth/amp.c | 24 ++++++++++++++++++++++++
net/bluetooth/hci_event.c | 15 ++-------------
4 files changed, 28 insertions(+), 13 deletions(-)

diff --git a/include/net/bluetooth/amp.h b/include/net/bluetooth/amp.h
index f1c0017..7ea3db7 100644
--- a/include/net/bluetooth/amp.h
+++ b/include/net/bluetooth/amp.h
@@ -46,6 +46,7 @@ void amp_accept_phylink(struct hci_dev *hdev, struct amp_mgr *mgr,
struct hci_conn *hcon);
void amp_write_remote_assoc(struct hci_dev *hdev, u8 handle);
void amp_write_rem_assoc_continue(struct hci_dev *hdev, u8 handle);
+void amp_physical_cfm(struct hci_conn *bredr_hcon, struct hci_conn *hs_hcon);
void amp_create_logical_link(struct l2cap_chan *chan);
void amp_disconnect_logical_link(struct hci_chan *hchan);
void amp_destroy_logical_link(struct hci_chan *hchan, u8 reason);
diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 24c61ef..18149c8 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -812,5 +812,6 @@ void l2cap_send_conn_req(struct l2cap_chan *chan);
void l2cap_move_start(struct l2cap_chan *chan);
void l2cap_logical_cfm(struct l2cap_chan *chan, struct hci_chan *hchan,
u8 status);
+void l2cap_physical_cfm(struct l2cap_chan *chan, int result);

#endif /* __L2CAP_H */
diff --git a/net/bluetooth/amp.c b/net/bluetooth/amp.c
index 917e034..650bb8d 100644
--- a/net/bluetooth/amp.c
+++ b/net/bluetooth/amp.c
@@ -373,6 +373,30 @@ void amp_accept_phylink(struct hci_dev *hdev, struct amp_mgr *mgr,
hci_send_cmd(hdev, HCI_OP_ACCEPT_PHY_LINK, sizeof(cp), &cp);
}

+void amp_physical_cfm(struct hci_conn *bredr_hcon, struct hci_conn *hs_hcon)
+{
+ struct hci_dev *bredr_hdev = hci_dev_hold(bredr_hcon->hdev);
+ struct amp_mgr *mgr = hs_hcon->amp_mgr;
+ struct l2cap_chan *bredr_chan;
+
+ BT_DBG("bredr_hcon %p hs_hcon %p mgr %p", bredr_hcon, hs_hcon, mgr);
+
+ if (!bredr_hdev || !mgr || !mgr->bredr_chan)
+ return;
+
+ bredr_chan = mgr->bredr_chan;
+
+ set_bit(FLAG_EFS_ENABLE, &bredr_chan->flags);
+ bredr_chan->ctrl_id = hs_hcon->remote_id;
+ bredr_chan->hs_hcon = hs_hcon;
+ bredr_chan->conn->mtu = hs_hcon->hdev->block_mtu;
+ bredr_chan->fcs = L2CAP_FCS_NONE;
+
+ l2cap_physical_cfm(bredr_chan, 0);
+
+ hci_dev_put(bredr_hdev);
+}
+
void amp_create_logical_link(struct l2cap_chan *chan)
{
struct hci_cp_create_accept_logical_link cp;
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 03d51a1..0b37f55 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -3697,20 +3697,9 @@ static void hci_phy_link_complete_evt(struct hci_dev *hdev,
hci_conn_hold_device(hcon);
hci_conn_add_sysfs(hcon);

- hci_dev_unlock(hdev);
-
- if (hcon->out) {
- struct hci_dev *bredr_hdev = hci_dev_hold(bredr_hcon->hdev);
-
- if (!bredr_hdev)
- return;
+ amp_physical_cfm(bredr_hcon, hcon);

- /* Placeholder - create chan req
- l2cap_chan_create_cfm(bredr_hcon, hcon->remote_id);
- */
-
- hci_dev_put(bredr_hdev);
- }
+ hci_dev_unlock(hdev);
}

static void hci_loglink_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
--
1.7.9.5


2012-10-31 13:46:28

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCHv1 03/12] Bluetooth: Return correct L2CAP response type

From: Andrei Emeltchenko <[email protected]>

Return L2CAP_CREATE_CHAN_RSP for Create Channel Request and
L2CAP_CONN_RSP for Create Connection Request.

Signed-off-by: Andrei Emeltchenko <[email protected]>
Acked-by: Marcel Holtmann <[email protected]>
---
net/bluetooth/l2cap_core.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 4ef85d2..d51741f 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -3478,12 +3478,21 @@ void __l2cap_connect_rsp_defer(struct l2cap_chan *chan)
struct l2cap_conn_rsp rsp;
struct l2cap_conn *conn = chan->conn;
u8 buf[128];
+ u8 rsp_code;

rsp.scid = cpu_to_le16(chan->dcid);
rsp.dcid = cpu_to_le16(chan->scid);
rsp.result = __constant_cpu_to_le16(L2CAP_CR_SUCCESS);
rsp.status = __constant_cpu_to_le16(L2CAP_CS_NO_INFO);
- l2cap_send_cmd(conn, chan->ident, L2CAP_CONN_RSP, sizeof(rsp), &rsp);
+
+ if (chan->hs_hcon)
+ rsp_code = L2CAP_CREATE_CHAN_RSP;
+ else
+ rsp_code = L2CAP_CONN_RSP;
+
+ BT_DBG("chan %p rsp_code %u", chan, rsp_code);
+
+ l2cap_send_cmd(conn, chan->ident, rsp_code, sizeof(rsp), &rsp);

if (test_and_set_bit(CONF_REQ_SENT, &chan->conf_state))
return;
--
1.7.9.5


2012-10-31 13:46:33

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCHv1 08/12] Bluetooth: AMP: Remove hci_conn receiving error command status

From: Andrei Emeltchenko <[email protected]>

When receiving HCI Event: Command Status for Create Physical Link
with Error code remove AMP hcon.

Signed-off-by: Andrei Emeltchenko <[email protected]>
Acked-by: Marcel Holtmann <[email protected]>
---
net/bluetooth/hci_event.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 7ca98b9..03d51a1 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1810,14 +1810,23 @@ static void hci_cs_create_phylink(struct hci_dev *hdev, u8 status)

BT_DBG("%s status 0x%2.2x", hdev->name, status);

- if (status)
- return;
-
cp = hci_sent_cmd_data(hdev, HCI_OP_CREATE_PHY_LINK);
if (!cp)
return;

- amp_write_remote_assoc(hdev, cp->phy_handle);
+ hci_dev_lock(hdev);
+
+ if (status) {
+ struct hci_conn *hcon;
+
+ hcon = hci_conn_hash_lookup_handle(hdev, cp->phy_handle);
+ if (hcon)
+ hci_conn_del(hcon);
+ } else {
+ amp_write_remote_assoc(hdev, cp->phy_handle);
+ }
+
+ hci_dev_unlock(hdev);
}

static void hci_cs_accept_phylink(struct hci_dev *hdev, u8 status)
--
1.7.9.5


2012-10-31 13:46:30

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCHv1 05/12] Bluetooth: AMP: Add Logical Link Create function

From: Andrei Emeltchenko <[email protected]>

After physical link is created logical link needs to be created.
The process starts after L2CAP channel is created and L2CAP
Configuration Response with result PENDING is received.

Signed-off-by: Andrei Emeltchenko <[email protected]>
Acked-by: Marcel Holtmann <[email protected]>
---
include/net/bluetooth/amp.h | 1 +
net/bluetooth/amp.c | 49 +++++++++++++++++++++++++++++++++++++++++++
net/bluetooth/hci_event.c | 9 ++++++++
net/bluetooth/l2cap_core.c | 19 ++++++++++++-----
4 files changed, 73 insertions(+), 5 deletions(-)

diff --git a/include/net/bluetooth/amp.h b/include/net/bluetooth/amp.h
index 2e7c79e..70d5c15 100644
--- a/include/net/bluetooth/amp.h
+++ b/include/net/bluetooth/amp.h
@@ -46,5 +46,6 @@ void amp_accept_phylink(struct hci_dev *hdev, struct amp_mgr *mgr,
struct hci_conn *hcon);
void amp_write_remote_assoc(struct hci_dev *hdev, u8 handle);
void amp_write_rem_assoc_continue(struct hci_dev *hdev, u8 handle);
+void amp_create_logical_link(struct l2cap_chan *chan);

#endif /* __AMP_H */
diff --git a/net/bluetooth/amp.c b/net/bluetooth/amp.c
index 231d7ef..fbb6360 100644
--- a/net/bluetooth/amp.c
+++ b/net/bluetooth/amp.c
@@ -372,3 +372,52 @@ void amp_accept_phylink(struct hci_dev *hdev, struct amp_mgr *mgr,

hci_send_cmd(hdev, HCI_OP_ACCEPT_PHY_LINK, sizeof(cp), &cp);
}
+
+void amp_create_logical_link(struct l2cap_chan *chan)
+{
+ struct hci_cp_create_accept_logical_link cp;
+ struct hci_conn *hcon;
+ struct hci_dev *hdev;
+
+ BT_DBG("chan %p", chan);
+
+ if (!chan->hs_hcon)
+ return;
+
+ hdev = hci_dev_hold(chan->hs_hcon->hdev);
+ if (!hdev)
+ return;
+
+ BT_DBG("chan %p ctrl_id %d dst %pMR", chan, chan->ctrl_id,
+ chan->conn->dst);
+
+ hcon = hci_conn_hash_lookup_ba(hdev, AMP_LINK, chan->conn->dst);
+ if (!hcon)
+ goto done;
+
+ cp.phy_handle = hcon->handle;
+
+ cp.tx_flow_spec.id = chan->local_id;
+ cp.tx_flow_spec.stype = chan->local_stype;
+ cp.tx_flow_spec.msdu = cpu_to_le16(chan->local_msdu);
+ cp.tx_flow_spec.sdu_itime = cpu_to_le32(chan->local_sdu_itime);
+ cp.tx_flow_spec.acc_lat = cpu_to_le32(chan->local_acc_lat);
+ cp.tx_flow_spec.flush_to = cpu_to_le32(chan->local_flush_to);
+
+ cp.rx_flow_spec.id = chan->remote_id;
+ cp.rx_flow_spec.stype = chan->remote_stype;
+ cp.rx_flow_spec.msdu = cpu_to_le16(chan->remote_msdu);
+ cp.rx_flow_spec.sdu_itime = cpu_to_le32(chan->remote_sdu_itime);
+ cp.rx_flow_spec.acc_lat = cpu_to_le32(chan->remote_acc_lat);
+ cp.rx_flow_spec.flush_to = cpu_to_le32(chan->remote_flush_to);
+
+ if (hcon->out)
+ hci_send_cmd(hdev, HCI_OP_CREATE_LOGICAL_LINK, sizeof(cp),
+ &cp);
+ else
+ hci_send_cmd(hdev, HCI_OP_ACCEPT_LOGICAL_LINK, sizeof(cp),
+ &cp);
+
+done:
+ hci_dev_put(hdev);
+}
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index aa79ed2..9963ec9 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1836,6 +1836,11 @@ static void hci_cs_accept_phylink(struct hci_dev *hdev, u8 status)
amp_write_remote_assoc(hdev, cp->phy_handle);
}

+static void hci_cs_create_logical_link(struct hci_dev *hdev, u8 status)
+{
+ BT_DBG("%s status 0x%2.2x", hdev->name, status);
+}
+
static void hci_inquiry_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
{
__u8 status = *((__u8 *) skb->data);
@@ -2670,6 +2675,10 @@ static void hci_cmd_status_evt(struct hci_dev *hdev, struct sk_buff *skb)
hci_cs_accept_phylink(hdev, ev->status);
break;

+ case HCI_OP_CREATE_LOGICAL_LINK:
+ hci_cs_create_logical_link(hdev, ev->status);
+ break;
+
default:
BT_DBG("%s opcode 0x%4.4x", hdev->name, opcode);
break;
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 782e49c..ecc5020 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -38,6 +38,7 @@
#include <net/bluetooth/l2cap.h>
#include <net/bluetooth/smp.h>
#include <net/bluetooth/a2mp.h>
+#include <net/bluetooth/amp.h>

bool disable_ertm;

@@ -1013,6 +1014,12 @@ static bool __amp_capable(struct l2cap_chan *chan)
return false;
}

+static bool l2cap_check_efs(struct l2cap_chan *chan)
+{
+ /* Check EFS parameters */
+ return true;
+}
+
void l2cap_send_conn_req(struct l2cap_chan *chan)
{
struct l2cap_conn *conn = chan->conn;
@@ -3957,13 +3964,15 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn,
goto done;
}

- /* check compatibility */
-
- if (!chan->ctrl_id)
+ if (!chan->ctrl_id) {
l2cap_send_efs_conf_rsp(chan, buf, cmd->ident,
0);
- else
- chan->ident = cmd->ident;
+ } else {
+ if (l2cap_check_efs(chan)) {
+ amp_create_logical_link(chan);
+ chan->ident = cmd->ident;
+ }
+ }
}
goto done;

--
1.7.9.5


2012-10-31 13:46:34

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCHv1 09/12] Bluetooth: Disconnect logical link when deleteing chan

From: Andrei Emeltchenko <[email protected]>

Disconnect logical link for high speed channel hs_hchan
associated with L2CAP channel chan.

Signed-off-by: Andrei Emeltchenko <[email protected]>
Acked-by: Marcel Holtmann <[email protected]>
---
include/net/bluetooth/amp.h | 1 +
net/bluetooth/amp.c | 14 ++++++++++++++
net/bluetooth/l2cap_core.c | 7 +++++++
3 files changed, 22 insertions(+)

diff --git a/include/net/bluetooth/amp.h b/include/net/bluetooth/amp.h
index 405fb9c..f1c0017 100644
--- a/include/net/bluetooth/amp.h
+++ b/include/net/bluetooth/amp.h
@@ -47,6 +47,7 @@ void amp_accept_phylink(struct hci_dev *hdev, struct amp_mgr *mgr,
void amp_write_remote_assoc(struct hci_dev *hdev, u8 handle);
void amp_write_rem_assoc_continue(struct hci_dev *hdev, u8 handle);
void amp_create_logical_link(struct l2cap_chan *chan);
+void amp_disconnect_logical_link(struct hci_chan *hchan);
void amp_destroy_logical_link(struct hci_chan *hchan, u8 reason);

#endif /* __AMP_H */
diff --git a/net/bluetooth/amp.c b/net/bluetooth/amp.c
index 0f3fef3..917e034 100644
--- a/net/bluetooth/amp.c
+++ b/net/bluetooth/amp.c
@@ -422,6 +422,20 @@ done:
hci_dev_put(hdev);
}

+void amp_disconnect_logical_link(struct hci_chan *hchan)
+{
+ struct hci_conn *hcon = hchan->conn;
+ struct hci_cp_disconn_logical_link cp;
+
+ if (hcon->state != BT_CONNECTED) {
+ BT_DBG("hchan %p not connected", hchan);
+ return;
+ }
+
+ cp.log_handle = cpu_to_le16(hchan->handle);
+ hci_send_cmd(hcon->hdev, HCI_OP_DISCONN_LOGICAL_LINK, sizeof(cp), &cp);
+}
+
void amp_destroy_logical_link(struct hci_chan *hchan, u8 reason)
{
BT_DBG("hchan %p", hchan);
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index ecc5020..bb2cd9e 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -578,6 +578,13 @@ void l2cap_chan_del(struct l2cap_chan *chan, int err)
mgr->bredr_chan = NULL;
}

+ if (chan->hs_hchan) {
+ struct hci_chan *hs_hchan = chan->hs_hchan;
+
+ BT_DBG("chan %p disconnect hs_hchan %p", chan, hs_hchan);
+ amp_disconnect_logical_link(hs_hchan);
+ }
+
chan->ops->teardown(chan, err);

if (test_bit(CONF_NOT_COMPLETE, &chan->conf_state))
--
1.7.9.5


2012-10-31 13:46:37

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCHv1 12/12] Bluetooth: Process Create Chan Request

From: Andrei Emeltchenko <[email protected]>

Add processing L2CAP Create Chan Request. When channel is created
save associated high speed link hs_hcon.

Signed-off-by: Andrei Emeltchenko <[email protected]>
---
net/bluetooth/l2cap_core.c | 63 ++++++++++++++++++++++++++++++--------------
1 file changed, 43 insertions(+), 20 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index bdffc4c..783ffae 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -4237,7 +4237,9 @@ static int l2cap_create_channel_req(struct l2cap_conn *conn,
u16 cmd_len, void *data)
{
struct l2cap_create_chan_req *req = data;
+ struct l2cap_create_chan_rsp rsp;
struct l2cap_chan *chan;
+ struct hci_dev *hdev = NULL;
u16 psm, scid;

if (cmd_len != sizeof(*req))
@@ -4251,36 +4253,57 @@ static int l2cap_create_channel_req(struct l2cap_conn *conn,

BT_DBG("psm 0x%2.2x, scid 0x%4.4x, amp_id %d", psm, scid, req->amp_id);

- if (req->amp_id) {
- struct hci_dev *hdev;
-
- /* Validate AMP controller id */
- hdev = hci_dev_get(req->amp_id);
- if (!hdev || hdev->dev_type != HCI_AMP ||
- !test_bit(HCI_UP, &hdev->flags)) {
- struct l2cap_create_chan_rsp rsp;
+ /* BR/EDR connection */
+ if (req->amp_id == HCI_BREDR_ID) {
+ l2cap_connect(conn, cmd, data, L2CAP_CREATE_CHAN_RSP,
+ req->amp_id);
+ return 0;
+ }

- rsp.dcid = 0;
- rsp.scid = cpu_to_le16(scid);
- rsp.result = __constant_cpu_to_le16(L2CAP_CR_BAD_AMP);
- rsp.status = __constant_cpu_to_le16(L2CAP_CS_NO_INFO);
+ /* Validate AMP controller id */
+ hdev = hci_dev_get(req->amp_id);
+ if (!hdev)
+ goto error;

- l2cap_send_cmd(conn, cmd->ident, L2CAP_CREATE_CHAN_RSP,
- sizeof(rsp), &rsp);
+ if (hdev->dev_type != HCI_AMP || !test_bit(HCI_UP, &hdev->flags)) {
+ hci_dev_put(hdev);
+ goto error;
+ }

- if (hdev)
- hci_dev_put(hdev);
+ chan = l2cap_connect(conn, cmd, data, L2CAP_CREATE_CHAN_RSP,
+ req->amp_id);
+ if (chan) {
+ struct amp_mgr *mgr = conn->hcon->amp_mgr;
+ struct hci_conn *hs_hcon;

- return 0;
+ hs_hcon = hci_conn_hash_lookup_ba(hdev, AMP_LINK, conn->dst);
+ if (!hs_hcon) {
+ hci_dev_put(hdev);
+ return -EFAULT;
}

- hci_dev_put(hdev);
+ BT_DBG("mgr %p bredr_chan %p hs_hcon %p", mgr, chan, hs_hcon);
+
+ chan->local_amp_id = req->amp_id;
+ mgr->bredr_chan = chan;
+ chan->hs_hcon = hs_hcon;
+ conn->mtu = hdev->block_mtu;
}

- chan = l2cap_connect(conn, cmd, data, L2CAP_CREATE_CHAN_RSP,
- req->amp_id);
+ hci_dev_put(hdev);

return 0;
+
+error:
+ rsp.dcid = 0;
+ rsp.scid = cpu_to_le16(scid);
+ rsp.result = __constant_cpu_to_le16(L2CAP_CR_BAD_AMP);
+ rsp.status = __constant_cpu_to_le16(L2CAP_CS_NO_INFO);
+
+ l2cap_send_cmd(conn, cmd->ident, L2CAP_CREATE_CHAN_RSP,
+ sizeof(rsp), &rsp);
+
+ return -EFAULT;
}

static void l2cap_send_move_chan_req(struct l2cap_chan *chan, u8 dest_amp_id)
--
1.7.9.5


2012-10-31 13:46:35

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCHv1 10/12] Bluetooth: AMP: Check for hs_hcon instead of ctrl_id

From: Andrei Emeltchenko <[email protected]>

When deciding whether to send EFS configuration response with success,
check rather for existence of High Speed physical link hs_hcon then
ctrl_id. There might be cases when there is ctrl_id but high speed link
is not established yet.

Signed-off-by: Andrei Emeltchenko <[email protected]>
Acked-by: Marcel Holtmann <[email protected]>
---
net/bluetooth/l2cap_core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index bb2cd9e..bdffc4c 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -3921,7 +3921,7 @@ static inline int l2cap_config_req(struct l2cap_conn *conn,
/* check compatibility */

/* Send rsp for BR/EDR channel */
- if (!chan->ctrl_id)
+ if (!chan->hs_hcon)
l2cap_send_efs_conf_rsp(chan, rsp, cmd->ident, flags);
else
chan->ident = cmd->ident;
@@ -3971,7 +3971,7 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn,
goto done;
}

- if (!chan->ctrl_id) {
+ if (!chan->hs_hcon) {
l2cap_send_efs_conf_rsp(chan, buf, cmd->ident,
0);
} else {
--
1.7.9.5


2012-10-31 13:46:29

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCHv1 04/12] Bluetooth: Derive remote and local amp id from chan struct

From: Andrei Emeltchenko <[email protected]>

l2cap_chan already keeps information about *_amp_id.

Signed-off-by: Andrei Emeltchenko <[email protected]>
Acked-by: Marcel Holtmann <[email protected]>
---
net/bluetooth/l2cap_core.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index d51741f..782e49c 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -4573,9 +4573,11 @@ static void l2cap_do_move_cancel(struct l2cap_chan *chan, int result)
l2cap_ertm_send(chan);
}

-void l2cap_physical_cfm(struct l2cap_chan *chan, int result, u8 local_amp_id,
- u8 remote_amp_id)
+void l2cap_physical_cfm(struct l2cap_chan *chan, int result)
{
+ u8 local_amp_id = chan->local_amp_id;
+ u8 remote_amp_id = chan->ctrl_id;
+
BT_DBG("chan %p, result %d, local_amp_id %d, remote_amp_id %d",
chan, result, local_amp_id, remote_amp_id);

--
1.7.9.5


2012-10-31 13:46:32

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCHv1 07/12] Bluetooth: AMP: Process Disc Physical Link Complete evt

From: Andrei Emeltchenko <[email protected]>

Add processing for HCI Disconnection Physical Link Complete Event.

Signed-off-by: Andrei Emeltchenko <[email protected]>
Acked-by: Marcel Holtmann <[email protected]>
---
net/bluetooth/hci_event.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 036cd8d..7ca98b9 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -3766,6 +3766,28 @@ unlock:
hci_dev_unlock(hdev);
}

+static void hci_disconn_phylink_complete_evt(struct hci_dev *hdev,
+ struct sk_buff *skb)
+{
+ struct hci_ev_disconn_phy_link_complete *ev = (void *) skb->data;
+ struct hci_conn *hcon;
+
+ BT_DBG("%s status 0x%2.2x", hdev->name, ev->status);
+
+ if (ev->status)
+ return;
+
+ hci_dev_lock(hdev);
+
+ hcon = hci_conn_hash_lookup_handle(hdev, ev->phy_handle);
+ if (hcon) {
+ hcon->state = BT_CLOSED;
+ hci_conn_del(hcon);
+ }
+
+ hci_dev_unlock(hdev);
+}
+
static void hci_le_conn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
{
struct hci_ev_le_conn_complete *ev = (void *) skb->data;
@@ -4105,6 +4127,10 @@ void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb)
hci_disconn_loglink_complete_evt(hdev, skb);
break;

+ case HCI_EV_DISCONN_PHY_LINK_COMPLETE:
+ hci_disconn_phylink_complete_evt(hdev, skb);
+ break;
+
case HCI_EV_NUM_COMP_BLOCKS:
hci_num_comp_blocks_evt(hdev, skb);
break;
--
1.7.9.5


2012-10-31 13:46:31

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCHv1 06/12] Bluetooth: AMP: Process Disc Logical Link

From: Andrei Emeltchenko <[email protected]>

Add processing for HCI Disconnection Logical Link Complete
Event.

Signed-off-by: Andrei Emeltchenko <[email protected]>
Acked-by: Marcel Holtmann <[email protected]>
---
include/net/bluetooth/amp.h | 1 +
net/bluetooth/amp.c | 7 +++++++
net/bluetooth/hci_event.c | 28 ++++++++++++++++++++++++++++
3 files changed, 36 insertions(+)

diff --git a/include/net/bluetooth/amp.h b/include/net/bluetooth/amp.h
index 70d5c15..405fb9c 100644
--- a/include/net/bluetooth/amp.h
+++ b/include/net/bluetooth/amp.h
@@ -47,5 +47,6 @@ void amp_accept_phylink(struct hci_dev *hdev, struct amp_mgr *mgr,
void amp_write_remote_assoc(struct hci_dev *hdev, u8 handle);
void amp_write_rem_assoc_continue(struct hci_dev *hdev, u8 handle);
void amp_create_logical_link(struct l2cap_chan *chan);
+void amp_destroy_logical_link(struct hci_chan *hchan, u8 reason);

#endif /* __AMP_H */
diff --git a/net/bluetooth/amp.c b/net/bluetooth/amp.c
index fbb6360..0f3fef3 100644
--- a/net/bluetooth/amp.c
+++ b/net/bluetooth/amp.c
@@ -421,3 +421,10 @@ void amp_create_logical_link(struct l2cap_chan *chan)
done:
hci_dev_put(hdev);
}
+
+void amp_destroy_logical_link(struct hci_chan *hchan, u8 reason)
+{
+ BT_DBG("hchan %p", hchan);
+
+ hci_chan_del(hchan);
+}
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 9963ec9..036cd8d 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -3742,6 +3742,30 @@ static void hci_loglink_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
}
}

+static void hci_disconn_loglink_complete_evt(struct hci_dev *hdev,
+ struct sk_buff *skb)
+{
+ struct hci_ev_disconn_logical_link_complete *ev = (void *) skb->data;
+ struct hci_chan *hchan;
+
+ BT_DBG("%s log handle 0x%4.4x status 0x%2.2x", hdev->name,
+ le16_to_cpu(ev->handle), ev->status);
+
+ if (ev->status)
+ return;
+
+ hci_dev_lock(hdev);
+
+ hchan = hci_chan_lookup_handle(hdev, le16_to_cpu(ev->handle));
+ if (!hchan)
+ goto unlock;
+
+ amp_destroy_logical_link(hchan, ev->reason);
+
+unlock:
+ hci_dev_unlock(hdev);
+}
+
static void hci_le_conn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
{
struct hci_ev_le_conn_complete *ev = (void *) skb->data;
@@ -4077,6 +4101,10 @@ void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb)
hci_loglink_complete_evt(hdev, skb);
break;

+ case HCI_EV_DISCONN_LOGICAL_LINK_COMPLETE:
+ hci_disconn_loglink_complete_evt(hdev, skb);
+ break;
+
case HCI_EV_NUM_COMP_BLOCKS:
hci_num_comp_blocks_evt(hdev, skb);
break;
--
1.7.9.5


2012-10-31 13:46:27

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCHv1 02/12] Bluetooth: Save hs_hchan instead of hs_hcon in loglink complete

From: Andrei Emeltchenko <[email protected]>

When logical link creation is completed we need to save hs_hchan
which represents logical link instead of hs_hcon representing
physical link. hs_hcon shall be saved when receiving physical link
complete event.

Signed-off-by: Andrei Emeltchenko <[email protected]>
Acked-by: Marcel Holtmann <[email protected]>
---
net/bluetooth/l2cap_core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index c2fbaf9..4ef85d2 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -4375,7 +4375,7 @@ static void l2cap_logical_finish_create(struct l2cap_chan *chan,
{
struct l2cap_conf_rsp rsp;

- chan->hs_hcon = hchan->conn;
+ chan->hs_hchan = hchan;
chan->hs_hcon->l2cap_data = chan->conn;

l2cap_send_efs_conf_rsp(chan, &rsp, chan->ident, 0);
--
1.7.9.5


2012-10-31 13:46:26

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCHv1 01/12] Bluetooth: trivial: Fix braces style and remove empty line

From: Andrei Emeltchenko <[email protected]>

Signed-off-by: Andrei Emeltchenko <[email protected]>
Acked-by: Marcel Holtmann <[email protected]>
---
net/bluetooth/l2cap_core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index d1728af..c2fbaf9 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -6252,9 +6252,9 @@ void l2cap_connect_cfm(struct hci_conn *hcon, u8 status)
conn = l2cap_conn_add(hcon, status);
if (conn)
l2cap_conn_ready(conn);
- } else
+ } else {
l2cap_conn_del(hcon, bt_to_errno(status));
-
+ }
}

int l2cap_disconn_ind(struct hci_conn *hcon)
--
1.7.9.5


2012-10-31 13:46:25

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCHv1 00/12] Handling physical and logical link fixes

From: Andrei Emeltchenko <[email protected]>

Set of patches adding fixes for physical and logical link event handlings.

Changes:
* PATCHv1: Added Marcel's acks, rewritten patch
"12/12 Bluetooth: Process Create Chan Request" following review comments.
* rfcv2: Added several new patches, fixing style issues.
* rfcv1: Rebased on top of Mat's patches, preserve Marcel's ack for
not-changed-much patches only.
* rfcv0: original

Andrei Emeltchenko (12):
Bluetooth: trivial: Fix braces style and remove empty line
Bluetooth: Save hs_hchan instead of hs_hcon in loglink complete
Bluetooth: Return correct L2CAP response type
Bluetooth: Derive remote and local amp id from chan struct
Bluetooth: AMP: Add Logical Link Create function
Bluetooth: AMP: Process Disc Logical Link
Bluetooth: AMP: Process Disc Physical Link Complete evt
Bluetooth: AMP: Remove hci_conn receiving error command status
Bluetooth: Disconnect logical link when deleteing chan
Bluetooth: AMP: Check for hs_hcon instead of ctrl_id
Bluetooth: AMP: Use l2cap_physical_cfm in phylink complete evt
Bluetooth: Process Create Chan Request

include/net/bluetooth/amp.h | 4 ++
include/net/bluetooth/l2cap.h | 1 +
net/bluetooth/amp.c | 94 +++++++++++++++++++++++++++++++++
net/bluetooth/hci_event.c | 95 ++++++++++++++++++++++++++++------
net/bluetooth/l2cap_core.c | 114 +++++++++++++++++++++++++++++------------
5 files changed, 259 insertions(+), 49 deletions(-)

--
1.7.9.5


2012-10-30 17:48:19

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [RFCv2 03/12] Bluetooth: Return correct L2CAP response type

Hi Andrei,

> > > Return L2CAP_CREATE_CHAN_RSP for Create Channel Request and
> > > L2CAP_CONN_RSP for Create Connection Request.
> > >
> > > Signed-off-by: Andrei Emeltchenko <[email protected]>
> > > ---
> > > net/bluetooth/l2cap_core.c | 11 ++++++++++-
> > > 1 file changed, 10 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> > > index 4ef85d2..d51741f 100644
> > > --- a/net/bluetooth/l2cap_core.c
> > > +++ b/net/bluetooth/l2cap_core.c
> > > @@ -3478,12 +3478,21 @@ void __l2cap_connect_rsp_defer(struct l2cap_chan *chan)
> > > struct l2cap_conn_rsp rsp;
> > > struct l2cap_conn *conn = chan->conn;
> > > u8 buf[128];
> > > + u8 rsp_code;
> > >
> > > rsp.scid = cpu_to_le16(chan->dcid);
> > > rsp.dcid = cpu_to_le16(chan->scid);
> > > rsp.result = __constant_cpu_to_le16(L2CAP_CR_SUCCESS);
> > > rsp.status = __constant_cpu_to_le16(L2CAP_CS_NO_INFO);
> > > - l2cap_send_cmd(conn, chan->ident, L2CAP_CONN_RSP, sizeof(rsp), &rsp);
> > > +
> > > + if (chan->hs_hcon)
> > > + rsp_code = L2CAP_CREATE_CHAN_RSP;
> > > + else
> > > + rsp_code = L2CAP_CONN_RSP;
> > > +
> > > + BT_DBG("chan %p rsp_code %u", chan, rsp_code);
> > > +
> > > + l2cap_send_cmd(conn, chan->ident, rsp_code, sizeof(rsp), &rsp);
> >
> > looks a bit hackish to me, but seems to work since both responses are
> > essentially identical. Separate functions might be better, but that can
> > be also fixed later on.
>
> I have continued to use method proposed by Mat. Do you think it shall be
> changed?

I think at some point we should change this. However for now I am
actually fine keeping this in favor of making progress. You do need to
get back at some point and do some cleanups. Just make a note of it.

Regards

Marcel



2012-10-30 17:16:18

by Andrei Emeltchenko

[permalink] [raw]
Subject: Re: [RFCv2 03/12] Bluetooth: Return correct L2CAP response type

Hi Marcel,

On Tue, Oct 30, 2012 at 09:49:03AM -0700, Marcel Holtmann wrote:
> Hi Andrei,
>
> > Return L2CAP_CREATE_CHAN_RSP for Create Channel Request and
> > L2CAP_CONN_RSP for Create Connection Request.
> >
> > Signed-off-by: Andrei Emeltchenko <[email protected]>
> > ---
> > net/bluetooth/l2cap_core.c | 11 ++++++++++-
> > 1 file changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> > index 4ef85d2..d51741f 100644
> > --- a/net/bluetooth/l2cap_core.c
> > +++ b/net/bluetooth/l2cap_core.c
> > @@ -3478,12 +3478,21 @@ void __l2cap_connect_rsp_defer(struct l2cap_chan *chan)
> > struct l2cap_conn_rsp rsp;
> > struct l2cap_conn *conn = chan->conn;
> > u8 buf[128];
> > + u8 rsp_code;
> >
> > rsp.scid = cpu_to_le16(chan->dcid);
> > rsp.dcid = cpu_to_le16(chan->scid);
> > rsp.result = __constant_cpu_to_le16(L2CAP_CR_SUCCESS);
> > rsp.status = __constant_cpu_to_le16(L2CAP_CS_NO_INFO);
> > - l2cap_send_cmd(conn, chan->ident, L2CAP_CONN_RSP, sizeof(rsp), &rsp);
> > +
> > + if (chan->hs_hcon)
> > + rsp_code = L2CAP_CREATE_CHAN_RSP;
> > + else
> > + rsp_code = L2CAP_CONN_RSP;
> > +
> > + BT_DBG("chan %p rsp_code %u", chan, rsp_code);
> > +
> > + l2cap_send_cmd(conn, chan->ident, rsp_code, sizeof(rsp), &rsp);
>
> looks a bit hackish to me, but seems to work since both responses are
> essentially identical. Separate functions might be better, but that can
> be also fixed later on.

I have continued to use method proposed by Mat. Do you think it shall be
changed?

Best regards
Andrei Emeltchenko


2012-10-30 16:53:22

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [RFCv2 12/12] Bluetooth: Process Create Chan Request

Hi Andrei,

> Add processing L2CAP Create Chan Request. When channel is created
> save associated high speed link hs_hcon.
>
> Signed-off-by: Andrei Emeltchenko <[email protected]>
> ---
> net/bluetooth/l2cap_core.c | 31 ++++++++++++++++++++++---------
> 1 file changed, 22 insertions(+), 9 deletions(-)
>
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index bdffc4c..0e14a1a 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -4238,6 +4238,7 @@ static int l2cap_create_channel_req(struct l2cap_conn *conn,
> {
> struct l2cap_create_chan_req *req = data;
> struct l2cap_chan *chan;
> + struct hci_dev *hdev = NULL;
> u16 psm, scid;
>
> if (cmd_len != sizeof(*req))
> @@ -4252,8 +4253,6 @@ static int l2cap_create_channel_req(struct l2cap_conn *conn,
> BT_DBG("psm 0x%2.2x, scid 0x%4.4x, amp_id %d", psm, scid, req->amp_id);
>
> if (req->amp_id) {
> - struct hci_dev *hdev;
> -
> /* Validate AMP controller id */
> hdev = hci_dev_get(req->amp_id);
> if (!hdev || hdev->dev_type != HCI_AMP ||
> @@ -4267,19 +4266,33 @@ static int l2cap_create_channel_req(struct l2cap_conn *conn,
>
> l2cap_send_cmd(conn, cmd->ident, L2CAP_CREATE_CHAN_RSP,
> sizeof(rsp), &rsp);
> -
> - if (hdev)
> - hci_dev_put(hdev);
> -
> - return 0;
> + goto done;
> }
> -
> - hci_dev_put(hdev);
> }
>
> chan = l2cap_connect(conn, cmd, data, L2CAP_CREATE_CHAN_RSP,
> req->amp_id);
>
> + if (chan && hdev) {
> + struct amp_mgr *mgr = conn->hcon->amp_mgr;
> + struct hci_conn *hs_hcon;
> +
> + hs_hcon = hci_conn_hash_lookup_ba(hdev, AMP_LINK, conn->dst);
> + if (!hs_hcon)
> + goto done;
> +
> + BT_DBG("mgr %p bredr_chan %p hs_hcon %p", mgr, chan, hs_hcon);
> +
> + chan->local_amp_id = req->amp_id;
> + mgr->bredr_chan = chan;
> + chan->hs_hcon = hs_hcon;
> + conn->mtu = hdev->block_mtu;
> + }
> +
> +done:
> + if (hdev)
> + hci_dev_put(hdev);
> +

same comment that I gave Mat, I do not like these unbalanced locking or
in your case reference counting. That is just hacking it together.

Regards

Marcel



2012-10-30 16:51:46

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [RFCv2 11/12] Bluetooth: AMP: Use l2cap_physical_cfm in phylink complete evt

Hi Andrei,

> When receiving HCI Phylink Complete event run amp_physical_cfm
> which initialize BR/EDR L2CAP channel associated with High Speed
> link and run l2cap_physical_cfm which shall send L2CAP Create
> Chan Request.
>
> Signed-off-by: Andrei Emeltchenko <[email protected]>
> ---
> include/net/bluetooth/amp.h | 1 +
> include/net/bluetooth/l2cap.h | 1 +
> net/bluetooth/amp.c | 24 ++++++++++++++++++++++++
> net/bluetooth/hci_event.c | 15 ++-------------
> 4 files changed, 28 insertions(+), 13 deletions(-)

Acked-by: Marcel Holtmann <[email protected]>

Regards

Marcel



2012-10-30 16:50:58

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [RFCv2 10/12] Bluetooth: AMP: Check for hs_hcon instead of ctrl_id

Hi Andrei,

> When deciding whether to send EFS configuration response with success,
> check rather for existence of High Speed physical link hs_hcon then
> ctrl_id. There might be cases when there is ctrl_id but high speed link
> is not established yet.
>
> Signed-off-by: Andrei Emeltchenko <[email protected]>
> ---
> net/bluetooth/l2cap_core.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)

Acked-by: Marcel Holtmann <[email protected]>

Regards

Marcel



2012-10-30 16:49:46

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [RFCv2 04/12] Bluetooth: Derive remote and local amp id from chan struct

Hi Andrei,

> l2cap_chan already keeps information about *_amp_id.
>
> Signed-off-by: Andrei Emeltchenko <[email protected]>
> ---
> net/bluetooth/l2cap_core.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)

Acked-by: Marcel Holtmann <[email protected]>

Regards

Marcel



2012-10-30 16:49:03

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [RFCv2 03/12] Bluetooth: Return correct L2CAP response type

Hi Andrei,

> Return L2CAP_CREATE_CHAN_RSP for Create Channel Request and
> L2CAP_CONN_RSP for Create Connection Request.
>
> Signed-off-by: Andrei Emeltchenko <[email protected]>
> ---
> net/bluetooth/l2cap_core.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 4ef85d2..d51741f 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -3478,12 +3478,21 @@ void __l2cap_connect_rsp_defer(struct l2cap_chan *chan)
> struct l2cap_conn_rsp rsp;
> struct l2cap_conn *conn = chan->conn;
> u8 buf[128];
> + u8 rsp_code;
>
> rsp.scid = cpu_to_le16(chan->dcid);
> rsp.dcid = cpu_to_le16(chan->scid);
> rsp.result = __constant_cpu_to_le16(L2CAP_CR_SUCCESS);
> rsp.status = __constant_cpu_to_le16(L2CAP_CS_NO_INFO);
> - l2cap_send_cmd(conn, chan->ident, L2CAP_CONN_RSP, sizeof(rsp), &rsp);
> +
> + if (chan->hs_hcon)
> + rsp_code = L2CAP_CREATE_CHAN_RSP;
> + else
> + rsp_code = L2CAP_CONN_RSP;
> +
> + BT_DBG("chan %p rsp_code %u", chan, rsp_code);
> +
> + l2cap_send_cmd(conn, chan->ident, rsp_code, sizeof(rsp), &rsp);

looks a bit hackish to me, but seems to work since both responses are
essentially identical. Separate functions might be better, but that can
be also fixed later on.

Acked-by: Marcel Holtmann <[email protected]>

Regards

Marcel



2012-10-30 16:46:36

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [RFCv2 02/12] Bluetooth: Save hs_hchan instead of hs_hcon in loglink complete

Hi Andrei,

> When logical link creation is completed we need to save hs_hchan
> which represents logical link instead of hs_hcon representing
> physical link. hs_hcon shall be saved when receiving physical link
> complete event.
>
> Signed-off-by: Andrei Emeltchenko <[email protected]>
> ---
> net/bluetooth/l2cap_core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)

Acked-by: Marcel Holtmann <[email protected]>

Regards

Marcel



2012-10-30 16:45:46

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [RFCv2 01/12] Bluetooth: trivial: Fix braces style and remove empty line

Hi Andrei,

> Signed-off-by: Andrei Emeltchenko <[email protected]>
> ---
> net/bluetooth/l2cap_core.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)

Acked-by: Marcel Holtmann <[email protected]>

Regards

Marcel



2012-10-30 15:52:53

by Andrei Emeltchenko

[permalink] [raw]
Subject: [RFCv2 01/12] Bluetooth: trivial: Fix braces style and remove empty line

From: Andrei Emeltchenko <[email protected]>


Signed-off-by: Andrei Emeltchenko <[email protected]>
---
net/bluetooth/l2cap_core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index d1728af..c2fbaf9 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -6252,9 +6252,9 @@ void l2cap_connect_cfm(struct hci_conn *hcon, u8 status)
conn = l2cap_conn_add(hcon, status);
if (conn)
l2cap_conn_ready(conn);
- } else
+ } else {
l2cap_conn_del(hcon, bt_to_errno(status));
-
+ }
}

int l2cap_disconn_ind(struct hci_conn *hcon)
--
1.7.9.5


2012-10-30 15:52:55

by Andrei Emeltchenko

[permalink] [raw]
Subject: [RFCv2 03/12] Bluetooth: Return correct L2CAP response type

From: Andrei Emeltchenko <[email protected]>

Return L2CAP_CREATE_CHAN_RSP for Create Channel Request and
L2CAP_CONN_RSP for Create Connection Request.

Signed-off-by: Andrei Emeltchenko <[email protected]>
---
net/bluetooth/l2cap_core.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 4ef85d2..d51741f 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -3478,12 +3478,21 @@ void __l2cap_connect_rsp_defer(struct l2cap_chan *chan)
struct l2cap_conn_rsp rsp;
struct l2cap_conn *conn = chan->conn;
u8 buf[128];
+ u8 rsp_code;

rsp.scid = cpu_to_le16(chan->dcid);
rsp.dcid = cpu_to_le16(chan->scid);
rsp.result = __constant_cpu_to_le16(L2CAP_CR_SUCCESS);
rsp.status = __constant_cpu_to_le16(L2CAP_CS_NO_INFO);
- l2cap_send_cmd(conn, chan->ident, L2CAP_CONN_RSP, sizeof(rsp), &rsp);
+
+ if (chan->hs_hcon)
+ rsp_code = L2CAP_CREATE_CHAN_RSP;
+ else
+ rsp_code = L2CAP_CONN_RSP;
+
+ BT_DBG("chan %p rsp_code %u", chan, rsp_code);
+
+ l2cap_send_cmd(conn, chan->ident, rsp_code, sizeof(rsp), &rsp);

if (test_and_set_bit(CONF_REQ_SENT, &chan->conf_state))
return;
--
1.7.9.5


2012-10-30 15:52:58

by Andrei Emeltchenko

[permalink] [raw]
Subject: [RFCv2 06/12] Bluetooth: AMP: Process Disc Logical Link

From: Andrei Emeltchenko <[email protected]>

Add processing for HCI Disconnection Logical Link Complete
Event.

Signed-off-by: Andrei Emeltchenko <[email protected]>
Acked-by: Marcel Holtmann <[email protected]>
---
include/net/bluetooth/amp.h | 1 +
net/bluetooth/amp.c | 7 +++++++
net/bluetooth/hci_event.c | 28 ++++++++++++++++++++++++++++
3 files changed, 36 insertions(+)

diff --git a/include/net/bluetooth/amp.h b/include/net/bluetooth/amp.h
index 70d5c15..405fb9c 100644
--- a/include/net/bluetooth/amp.h
+++ b/include/net/bluetooth/amp.h
@@ -47,5 +47,6 @@ void amp_accept_phylink(struct hci_dev *hdev, struct amp_mgr *mgr,
void amp_write_remote_assoc(struct hci_dev *hdev, u8 handle);
void amp_write_rem_assoc_continue(struct hci_dev *hdev, u8 handle);
void amp_create_logical_link(struct l2cap_chan *chan);
+void amp_destroy_logical_link(struct hci_chan *hchan, u8 reason);

#endif /* __AMP_H */
diff --git a/net/bluetooth/amp.c b/net/bluetooth/amp.c
index fbb6360..0f3fef3 100644
--- a/net/bluetooth/amp.c
+++ b/net/bluetooth/amp.c
@@ -421,3 +421,10 @@ void amp_create_logical_link(struct l2cap_chan *chan)
done:
hci_dev_put(hdev);
}
+
+void amp_destroy_logical_link(struct hci_chan *hchan, u8 reason)
+{
+ BT_DBG("hchan %p", hchan);
+
+ hci_chan_del(hchan);
+}
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 9963ec9..036cd8d 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -3742,6 +3742,30 @@ static void hci_loglink_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
}
}

+static void hci_disconn_loglink_complete_evt(struct hci_dev *hdev,
+ struct sk_buff *skb)
+{
+ struct hci_ev_disconn_logical_link_complete *ev = (void *) skb->data;
+ struct hci_chan *hchan;
+
+ BT_DBG("%s log handle 0x%4.4x status 0x%2.2x", hdev->name,
+ le16_to_cpu(ev->handle), ev->status);
+
+ if (ev->status)
+ return;
+
+ hci_dev_lock(hdev);
+
+ hchan = hci_chan_lookup_handle(hdev, le16_to_cpu(ev->handle));
+ if (!hchan)
+ goto unlock;
+
+ amp_destroy_logical_link(hchan, ev->reason);
+
+unlock:
+ hci_dev_unlock(hdev);
+}
+
static void hci_le_conn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
{
struct hci_ev_le_conn_complete *ev = (void *) skb->data;
@@ -4077,6 +4101,10 @@ void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb)
hci_loglink_complete_evt(hdev, skb);
break;

+ case HCI_EV_DISCONN_LOGICAL_LINK_COMPLETE:
+ hci_disconn_loglink_complete_evt(hdev, skb);
+ break;
+
case HCI_EV_NUM_COMP_BLOCKS:
hci_num_comp_blocks_evt(hdev, skb);
break;
--
1.7.9.5


2012-10-30 15:53:03

by Andrei Emeltchenko

[permalink] [raw]
Subject: [RFCv2 11/12] Bluetooth: AMP: Use l2cap_physical_cfm in phylink complete evt

From: Andrei Emeltchenko <[email protected]>

When receiving HCI Phylink Complete event run amp_physical_cfm
which initialize BR/EDR L2CAP channel associated with High Speed
link and run l2cap_physical_cfm which shall send L2CAP Create
Chan Request.

Signed-off-by: Andrei Emeltchenko <[email protected]>
---
include/net/bluetooth/amp.h | 1 +
include/net/bluetooth/l2cap.h | 1 +
net/bluetooth/amp.c | 24 ++++++++++++++++++++++++
net/bluetooth/hci_event.c | 15 ++-------------
4 files changed, 28 insertions(+), 13 deletions(-)

diff --git a/include/net/bluetooth/amp.h b/include/net/bluetooth/amp.h
index f1c0017..7ea3db7 100644
--- a/include/net/bluetooth/amp.h
+++ b/include/net/bluetooth/amp.h
@@ -46,6 +46,7 @@ void amp_accept_phylink(struct hci_dev *hdev, struct amp_mgr *mgr,
struct hci_conn *hcon);
void amp_write_remote_assoc(struct hci_dev *hdev, u8 handle);
void amp_write_rem_assoc_continue(struct hci_dev *hdev, u8 handle);
+void amp_physical_cfm(struct hci_conn *bredr_hcon, struct hci_conn *hs_hcon);
void amp_create_logical_link(struct l2cap_chan *chan);
void amp_disconnect_logical_link(struct hci_chan *hchan);
void amp_destroy_logical_link(struct hci_chan *hchan, u8 reason);
diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 24c61ef..18149c8 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -812,5 +812,6 @@ void l2cap_send_conn_req(struct l2cap_chan *chan);
void l2cap_move_start(struct l2cap_chan *chan);
void l2cap_logical_cfm(struct l2cap_chan *chan, struct hci_chan *hchan,
u8 status);
+void l2cap_physical_cfm(struct l2cap_chan *chan, int result);

#endif /* __L2CAP_H */
diff --git a/net/bluetooth/amp.c b/net/bluetooth/amp.c
index 917e034..650bb8d 100644
--- a/net/bluetooth/amp.c
+++ b/net/bluetooth/amp.c
@@ -373,6 +373,30 @@ void amp_accept_phylink(struct hci_dev *hdev, struct amp_mgr *mgr,
hci_send_cmd(hdev, HCI_OP_ACCEPT_PHY_LINK, sizeof(cp), &cp);
}

+void amp_physical_cfm(struct hci_conn *bredr_hcon, struct hci_conn *hs_hcon)
+{
+ struct hci_dev *bredr_hdev = hci_dev_hold(bredr_hcon->hdev);
+ struct amp_mgr *mgr = hs_hcon->amp_mgr;
+ struct l2cap_chan *bredr_chan;
+
+ BT_DBG("bredr_hcon %p hs_hcon %p mgr %p", bredr_hcon, hs_hcon, mgr);
+
+ if (!bredr_hdev || !mgr || !mgr->bredr_chan)
+ return;
+
+ bredr_chan = mgr->bredr_chan;
+
+ set_bit(FLAG_EFS_ENABLE, &bredr_chan->flags);
+ bredr_chan->ctrl_id = hs_hcon->remote_id;
+ bredr_chan->hs_hcon = hs_hcon;
+ bredr_chan->conn->mtu = hs_hcon->hdev->block_mtu;
+ bredr_chan->fcs = L2CAP_FCS_NONE;
+
+ l2cap_physical_cfm(bredr_chan, 0);
+
+ hci_dev_put(bredr_hdev);
+}
+
void amp_create_logical_link(struct l2cap_chan *chan)
{
struct hci_cp_create_accept_logical_link cp;
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 03d51a1..0b37f55 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -3697,20 +3697,9 @@ static void hci_phy_link_complete_evt(struct hci_dev *hdev,
hci_conn_hold_device(hcon);
hci_conn_add_sysfs(hcon);

- hci_dev_unlock(hdev);
-
- if (hcon->out) {
- struct hci_dev *bredr_hdev = hci_dev_hold(bredr_hcon->hdev);
-
- if (!bredr_hdev)
- return;
+ amp_physical_cfm(bredr_hcon, hcon);

- /* Placeholder - create chan req
- l2cap_chan_create_cfm(bredr_hcon, hcon->remote_id);
- */
-
- hci_dev_put(bredr_hdev);
- }
+ hci_dev_unlock(hdev);
}

static void hci_loglink_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
--
1.7.9.5


2012-10-30 15:52:59

by Andrei Emeltchenko

[permalink] [raw]
Subject: [RFCv2 07/12] Bluetooth: AMP: Process Disc Physical Link Complete evt

From: Andrei Emeltchenko <[email protected]>

Add processing for HCI Disconnection Physical Link Complete Event.

Signed-off-by: Andrei Emeltchenko <[email protected]>
Acked-by: Marcel Holtmann <[email protected]>
---
net/bluetooth/hci_event.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 036cd8d..7ca98b9 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -3766,6 +3766,28 @@ unlock:
hci_dev_unlock(hdev);
}

+static void hci_disconn_phylink_complete_evt(struct hci_dev *hdev,
+ struct sk_buff *skb)
+{
+ struct hci_ev_disconn_phy_link_complete *ev = (void *) skb->data;
+ struct hci_conn *hcon;
+
+ BT_DBG("%s status 0x%2.2x", hdev->name, ev->status);
+
+ if (ev->status)
+ return;
+
+ hci_dev_lock(hdev);
+
+ hcon = hci_conn_hash_lookup_handle(hdev, ev->phy_handle);
+ if (hcon) {
+ hcon->state = BT_CLOSED;
+ hci_conn_del(hcon);
+ }
+
+ hci_dev_unlock(hdev);
+}
+
static void hci_le_conn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
{
struct hci_ev_le_conn_complete *ev = (void *) skb->data;
@@ -4105,6 +4127,10 @@ void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb)
hci_disconn_loglink_complete_evt(hdev, skb);
break;

+ case HCI_EV_DISCONN_PHY_LINK_COMPLETE:
+ hci_disconn_phylink_complete_evt(hdev, skb);
+ break;
+
case HCI_EV_NUM_COMP_BLOCKS:
hci_num_comp_blocks_evt(hdev, skb);
break;
--
1.7.9.5


2012-10-30 15:53:02

by Andrei Emeltchenko

[permalink] [raw]
Subject: [RFCv2 10/12] Bluetooth: AMP: Check for hs_hcon instead of ctrl_id

From: Andrei Emeltchenko <[email protected]>

When deciding whether to send EFS configuration response with success,
check rather for existence of High Speed physical link hs_hcon then
ctrl_id. There might be cases when there is ctrl_id but high speed link
is not established yet.

Signed-off-by: Andrei Emeltchenko <[email protected]>
---
net/bluetooth/l2cap_core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index bb2cd9e..bdffc4c 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -3921,7 +3921,7 @@ static inline int l2cap_config_req(struct l2cap_conn *conn,
/* check compatibility */

/* Send rsp for BR/EDR channel */
- if (!chan->ctrl_id)
+ if (!chan->hs_hcon)
l2cap_send_efs_conf_rsp(chan, rsp, cmd->ident, flags);
else
chan->ident = cmd->ident;
@@ -3971,7 +3971,7 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn,
goto done;
}

- if (!chan->ctrl_id) {
+ if (!chan->hs_hcon) {
l2cap_send_efs_conf_rsp(chan, buf, cmd->ident,
0);
} else {
--
1.7.9.5


2012-10-30 15:52:56

by Andrei Emeltchenko

[permalink] [raw]
Subject: [RFCv2 04/12] Bluetooth: Derive remote and local amp id from chan struct

From: Andrei Emeltchenko <[email protected]>

l2cap_chan already keeps information about *_amp_id.

Signed-off-by: Andrei Emeltchenko <[email protected]>
---
net/bluetooth/l2cap_core.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index d51741f..782e49c 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -4573,9 +4573,11 @@ static void l2cap_do_move_cancel(struct l2cap_chan *chan, int result)
l2cap_ertm_send(chan);
}

-void l2cap_physical_cfm(struct l2cap_chan *chan, int result, u8 local_amp_id,
- u8 remote_amp_id)
+void l2cap_physical_cfm(struct l2cap_chan *chan, int result)
{
+ u8 local_amp_id = chan->local_amp_id;
+ u8 remote_amp_id = chan->ctrl_id;
+
BT_DBG("chan %p, result %d, local_amp_id %d, remote_amp_id %d",
chan, result, local_amp_id, remote_amp_id);

--
1.7.9.5


2012-10-30 15:53:00

by Andrei Emeltchenko

[permalink] [raw]
Subject: [RFCv2 08/12] Bluetooth: AMP: Remove hci_conn receiving error command status

From: Andrei Emeltchenko <[email protected]>

When receiving HCI Event: Command Status for Create Physical Link
with Error code remove AMP hcon.

Signed-off-by: Andrei Emeltchenko <[email protected]>
Acked-by: Marcel Holtmann <[email protected]>
---
net/bluetooth/hci_event.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 7ca98b9..03d51a1 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1810,14 +1810,23 @@ static void hci_cs_create_phylink(struct hci_dev *hdev, u8 status)

BT_DBG("%s status 0x%2.2x", hdev->name, status);

- if (status)
- return;
-
cp = hci_sent_cmd_data(hdev, HCI_OP_CREATE_PHY_LINK);
if (!cp)
return;

- amp_write_remote_assoc(hdev, cp->phy_handle);
+ hci_dev_lock(hdev);
+
+ if (status) {
+ struct hci_conn *hcon;
+
+ hcon = hci_conn_hash_lookup_handle(hdev, cp->phy_handle);
+ if (hcon)
+ hci_conn_del(hcon);
+ } else {
+ amp_write_remote_assoc(hdev, cp->phy_handle);
+ }
+
+ hci_dev_unlock(hdev);
}

static void hci_cs_accept_phylink(struct hci_dev *hdev, u8 status)
--
1.7.9.5


2012-10-30 15:52:52

by Andrei Emeltchenko

[permalink] [raw]
Subject: [RFCv2 00/12] Handling physical and logical link

From: Andrei Emeltchenko <[email protected]>

Set of patches adding handling for physical and logical link
complete events.

Changes:
* rfcv2: Added several new patches, fixing style issues.
* rfcv1: Rebased on top of Mat's patches, preserve Marcel's ack for
not-changed-much patches only.
* rfcv0: original

Andrei Emeltchenko (12):
Bluetooth: trivial: Fix braces style and remove empty line
Bluetooth: Save hs_hchan instead of hs_hcon in loglink complete
Bluetooth: Return correct L2CAP response type
Bluetooth: Derive remote and local amp id from chan struct
Bluetooth: AMP: Add Logical Link Create function
Bluetooth: AMP: Process Disc Logical Link
Bluetooth: AMP: Process Disc Physical Link Complete evt
Bluetooth: AMP: Remove hci_conn receiving error command status
Bluetooth: Disconnect logical link when deleteing chan
Bluetooth: AMP: Check for hs_hcon instead of ctrl_id
Bluetooth: AMP: Use l2cap_physical_cfm in phylink complete evt
Bluetooth: Process Create Chan Request

include/net/bluetooth/amp.h | 4 ++
include/net/bluetooth/l2cap.h | 1 +
net/bluetooth/amp.c | 94 ++++++++++++++++++++++++++++++++++++++++
net/bluetooth/hci_event.c | 95 +++++++++++++++++++++++++++++++++--------
net/bluetooth/l2cap_core.c | 82 ++++++++++++++++++++++++++---------
5 files changed, 238 insertions(+), 38 deletions(-)

--
1.7.9.5


2012-10-30 15:52:54

by Andrei Emeltchenko

[permalink] [raw]
Subject: [RFCv2 02/12] Bluetooth: Save hs_hchan instead of hs_hcon in loglink complete

From: Andrei Emeltchenko <[email protected]>

When logical link creation is completed we need to save hs_hchan
which represents logical link instead of hs_hcon representing
physical link. hs_hcon shall be saved when receiving physical link
complete event.

Signed-off-by: Andrei Emeltchenko <[email protected]>
---
net/bluetooth/l2cap_core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index c2fbaf9..4ef85d2 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -4375,7 +4375,7 @@ static void l2cap_logical_finish_create(struct l2cap_chan *chan,
{
struct l2cap_conf_rsp rsp;

- chan->hs_hcon = hchan->conn;
+ chan->hs_hchan = hchan;
chan->hs_hcon->l2cap_data = chan->conn;

l2cap_send_efs_conf_rsp(chan, &rsp, chan->ident, 0);
--
1.7.9.5


2012-10-30 15:52:57

by Andrei Emeltchenko

[permalink] [raw]
Subject: [RFCv2 05/12] Bluetooth: AMP: Add Logical Link Create function

From: Andrei Emeltchenko <[email protected]>

After physical link is created logical link needs to be created.
The process starts after L2CAP channel is created and L2CAP
Configuration Response with result PENDING is received.

Signed-off-by: Andrei Emeltchenko <[email protected]>
Acked-by: Marcel Holtmann <[email protected]>
---
include/net/bluetooth/amp.h | 1 +
net/bluetooth/amp.c | 49 +++++++++++++++++++++++++++++++++++++++++++
net/bluetooth/hci_event.c | 9 ++++++++
net/bluetooth/l2cap_core.c | 19 ++++++++++++-----
4 files changed, 73 insertions(+), 5 deletions(-)

diff --git a/include/net/bluetooth/amp.h b/include/net/bluetooth/amp.h
index 2e7c79e..70d5c15 100644
--- a/include/net/bluetooth/amp.h
+++ b/include/net/bluetooth/amp.h
@@ -46,5 +46,6 @@ void amp_accept_phylink(struct hci_dev *hdev, struct amp_mgr *mgr,
struct hci_conn *hcon);
void amp_write_remote_assoc(struct hci_dev *hdev, u8 handle);
void amp_write_rem_assoc_continue(struct hci_dev *hdev, u8 handle);
+void amp_create_logical_link(struct l2cap_chan *chan);

#endif /* __AMP_H */
diff --git a/net/bluetooth/amp.c b/net/bluetooth/amp.c
index 231d7ef..fbb6360 100644
--- a/net/bluetooth/amp.c
+++ b/net/bluetooth/amp.c
@@ -372,3 +372,52 @@ void amp_accept_phylink(struct hci_dev *hdev, struct amp_mgr *mgr,

hci_send_cmd(hdev, HCI_OP_ACCEPT_PHY_LINK, sizeof(cp), &cp);
}
+
+void amp_create_logical_link(struct l2cap_chan *chan)
+{
+ struct hci_cp_create_accept_logical_link cp;
+ struct hci_conn *hcon;
+ struct hci_dev *hdev;
+
+ BT_DBG("chan %p", chan);
+
+ if (!chan->hs_hcon)
+ return;
+
+ hdev = hci_dev_hold(chan->hs_hcon->hdev);
+ if (!hdev)
+ return;
+
+ BT_DBG("chan %p ctrl_id %d dst %pMR", chan, chan->ctrl_id,
+ chan->conn->dst);
+
+ hcon = hci_conn_hash_lookup_ba(hdev, AMP_LINK, chan->conn->dst);
+ if (!hcon)
+ goto done;
+
+ cp.phy_handle = hcon->handle;
+
+ cp.tx_flow_spec.id = chan->local_id;
+ cp.tx_flow_spec.stype = chan->local_stype;
+ cp.tx_flow_spec.msdu = cpu_to_le16(chan->local_msdu);
+ cp.tx_flow_spec.sdu_itime = cpu_to_le32(chan->local_sdu_itime);
+ cp.tx_flow_spec.acc_lat = cpu_to_le32(chan->local_acc_lat);
+ cp.tx_flow_spec.flush_to = cpu_to_le32(chan->local_flush_to);
+
+ cp.rx_flow_spec.id = chan->remote_id;
+ cp.rx_flow_spec.stype = chan->remote_stype;
+ cp.rx_flow_spec.msdu = cpu_to_le16(chan->remote_msdu);
+ cp.rx_flow_spec.sdu_itime = cpu_to_le32(chan->remote_sdu_itime);
+ cp.rx_flow_spec.acc_lat = cpu_to_le32(chan->remote_acc_lat);
+ cp.rx_flow_spec.flush_to = cpu_to_le32(chan->remote_flush_to);
+
+ if (hcon->out)
+ hci_send_cmd(hdev, HCI_OP_CREATE_LOGICAL_LINK, sizeof(cp),
+ &cp);
+ else
+ hci_send_cmd(hdev, HCI_OP_ACCEPT_LOGICAL_LINK, sizeof(cp),
+ &cp);
+
+done:
+ hci_dev_put(hdev);
+}
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index aa79ed2..9963ec9 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1836,6 +1836,11 @@ static void hci_cs_accept_phylink(struct hci_dev *hdev, u8 status)
amp_write_remote_assoc(hdev, cp->phy_handle);
}

+static void hci_cs_create_logical_link(struct hci_dev *hdev, u8 status)
+{
+ BT_DBG("%s status 0x%2.2x", hdev->name, status);
+}
+
static void hci_inquiry_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
{
__u8 status = *((__u8 *) skb->data);
@@ -2670,6 +2675,10 @@ static void hci_cmd_status_evt(struct hci_dev *hdev, struct sk_buff *skb)
hci_cs_accept_phylink(hdev, ev->status);
break;

+ case HCI_OP_CREATE_LOGICAL_LINK:
+ hci_cs_create_logical_link(hdev, ev->status);
+ break;
+
default:
BT_DBG("%s opcode 0x%4.4x", hdev->name, opcode);
break;
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 782e49c..ecc5020 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -38,6 +38,7 @@
#include <net/bluetooth/l2cap.h>
#include <net/bluetooth/smp.h>
#include <net/bluetooth/a2mp.h>
+#include <net/bluetooth/amp.h>

bool disable_ertm;

@@ -1013,6 +1014,12 @@ static bool __amp_capable(struct l2cap_chan *chan)
return false;
}

+static bool l2cap_check_efs(struct l2cap_chan *chan)
+{
+ /* Check EFS parameters */
+ return true;
+}
+
void l2cap_send_conn_req(struct l2cap_chan *chan)
{
struct l2cap_conn *conn = chan->conn;
@@ -3957,13 +3964,15 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn,
goto done;
}

- /* check compatibility */
-
- if (!chan->ctrl_id)
+ if (!chan->ctrl_id) {
l2cap_send_efs_conf_rsp(chan, buf, cmd->ident,
0);
- else
- chan->ident = cmd->ident;
+ } else {
+ if (l2cap_check_efs(chan)) {
+ amp_create_logical_link(chan);
+ chan->ident = cmd->ident;
+ }
+ }
}
goto done;

--
1.7.9.5


2012-10-30 15:53:01

by Andrei Emeltchenko

[permalink] [raw]
Subject: [RFCv2 09/12] Bluetooth: Disconnect logical link when deleteing chan

From: Andrei Emeltchenko <[email protected]>

Disconnect logical link for high speed channel hs_hchan
associated with L2CAP channel chan.

Signed-off-by: Andrei Emeltchenko <[email protected]>
Acked-by: Marcel Holtmann <[email protected]>
---
include/net/bluetooth/amp.h | 1 +
net/bluetooth/amp.c | 14 ++++++++++++++
net/bluetooth/l2cap_core.c | 7 +++++++
3 files changed, 22 insertions(+)

diff --git a/include/net/bluetooth/amp.h b/include/net/bluetooth/amp.h
index 405fb9c..f1c0017 100644
--- a/include/net/bluetooth/amp.h
+++ b/include/net/bluetooth/amp.h
@@ -47,6 +47,7 @@ void amp_accept_phylink(struct hci_dev *hdev, struct amp_mgr *mgr,
void amp_write_remote_assoc(struct hci_dev *hdev, u8 handle);
void amp_write_rem_assoc_continue(struct hci_dev *hdev, u8 handle);
void amp_create_logical_link(struct l2cap_chan *chan);
+void amp_disconnect_logical_link(struct hci_chan *hchan);
void amp_destroy_logical_link(struct hci_chan *hchan, u8 reason);

#endif /* __AMP_H */
diff --git a/net/bluetooth/amp.c b/net/bluetooth/amp.c
index 0f3fef3..917e034 100644
--- a/net/bluetooth/amp.c
+++ b/net/bluetooth/amp.c
@@ -422,6 +422,20 @@ done:
hci_dev_put(hdev);
}

+void amp_disconnect_logical_link(struct hci_chan *hchan)
+{
+ struct hci_conn *hcon = hchan->conn;
+ struct hci_cp_disconn_logical_link cp;
+
+ if (hcon->state != BT_CONNECTED) {
+ BT_DBG("hchan %p not connected", hchan);
+ return;
+ }
+
+ cp.log_handle = cpu_to_le16(hchan->handle);
+ hci_send_cmd(hcon->hdev, HCI_OP_DISCONN_LOGICAL_LINK, sizeof(cp), &cp);
+}
+
void amp_destroy_logical_link(struct hci_chan *hchan, u8 reason)
{
BT_DBG("hchan %p", hchan);
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index ecc5020..bb2cd9e 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -578,6 +578,13 @@ void l2cap_chan_del(struct l2cap_chan *chan, int err)
mgr->bredr_chan = NULL;
}

+ if (chan->hs_hchan) {
+ struct hci_chan *hs_hchan = chan->hs_hchan;
+
+ BT_DBG("chan %p disconnect hs_hchan %p", chan, hs_hchan);
+ amp_disconnect_logical_link(hs_hchan);
+ }
+
chan->ops->teardown(chan, err);

if (test_bit(CONF_NOT_COMPLETE, &chan->conf_state))
--
1.7.9.5


2012-10-30 15:53:04

by Andrei Emeltchenko

[permalink] [raw]
Subject: [RFCv2 12/12] Bluetooth: Process Create Chan Request

From: Andrei Emeltchenko <[email protected]>

Add processing L2CAP Create Chan Request. When channel is created
save associated high speed link hs_hcon.

Signed-off-by: Andrei Emeltchenko <[email protected]>
---
net/bluetooth/l2cap_core.c | 31 ++++++++++++++++++++++---------
1 file changed, 22 insertions(+), 9 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index bdffc4c..0e14a1a 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -4238,6 +4238,7 @@ static int l2cap_create_channel_req(struct l2cap_conn *conn,
{
struct l2cap_create_chan_req *req = data;
struct l2cap_chan *chan;
+ struct hci_dev *hdev = NULL;
u16 psm, scid;

if (cmd_len != sizeof(*req))
@@ -4252,8 +4253,6 @@ static int l2cap_create_channel_req(struct l2cap_conn *conn,
BT_DBG("psm 0x%2.2x, scid 0x%4.4x, amp_id %d", psm, scid, req->amp_id);

if (req->amp_id) {
- struct hci_dev *hdev;
-
/* Validate AMP controller id */
hdev = hci_dev_get(req->amp_id);
if (!hdev || hdev->dev_type != HCI_AMP ||
@@ -4267,19 +4266,33 @@ static int l2cap_create_channel_req(struct l2cap_conn *conn,

l2cap_send_cmd(conn, cmd->ident, L2CAP_CREATE_CHAN_RSP,
sizeof(rsp), &rsp);
-
- if (hdev)
- hci_dev_put(hdev);
-
- return 0;
+ goto done;
}
-
- hci_dev_put(hdev);
}

chan = l2cap_connect(conn, cmd, data, L2CAP_CREATE_CHAN_RSP,
req->amp_id);

+ if (chan && hdev) {
+ struct amp_mgr *mgr = conn->hcon->amp_mgr;
+ struct hci_conn *hs_hcon;
+
+ hs_hcon = hci_conn_hash_lookup_ba(hdev, AMP_LINK, conn->dst);
+ if (!hs_hcon)
+ goto done;
+
+ BT_DBG("mgr %p bredr_chan %p hs_hcon %p", mgr, chan, hs_hcon);
+
+ chan->local_amp_id = req->amp_id;
+ mgr->bredr_chan = chan;
+ chan->hs_hcon = hs_hcon;
+ conn->mtu = hdev->block_mtu;
+ }
+
+done:
+ if (hdev)
+ hci_dev_put(hdev);
+
return 0;
}

--
1.7.9.5


2012-10-29 09:24:57

by Andrei Emeltchenko

[permalink] [raw]
Subject: Re: [RFCv1 05/11] Bluetooth: AMP: Add Logical Link Create function

Hi Mat,

On Fri, Oct 26, 2012 at 10:01:01AM -0700, Mat Martineau wrote:

...

> >@@ -3948,13 +3955,15 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn,
> > goto done;
> > }
> >
> >- /* check compatibility */
> >-
> > if (!chan->ctrl_id)
> > l2cap_send_efs_conf_rsp(chan, buf, cmd->ident,
> > 0);
> >- else
> >- chan->ident = cmd->ident;
> >+ else {
> >+ if (l2cap_check_efs(chan)) {
> >+ amp_create_logical_link(chan);
> >+ chan->ident = cmd->ident;
> >+ }
> >+ }
>
> Minor style issue - if one block of an if/else needs braces, then
> they all get braces.

Sure. Will fix this.

Best regards
Andrei Emeltchenko


2012-10-29 09:22:37

by Andrei Emeltchenko

[permalink] [raw]
Subject: Re: [RFCv1 03/11] Bluetooth: AMP: Process Physical Link Complete evt

Hi Mat,

On Fri, Oct 26, 2012 at 10:16:53AM -0700, Mat Martineau wrote:\
...
> >+static void hci_phy_link_complete_evt(struct hci_dev *hdev,
> >+ struct sk_buff *skb)
> >+{
> >+ struct hci_ev_phy_link_complete *ev = (void *) skb->data;
> >+ struct hci_conn *hcon, *bredr_hcon;
> >+
> >+ BT_DBG("%s handle 0x%2.2x status 0x%2.2x", hdev->name, ev->phy_handle,
> >+ ev->status);
> >+
> >+ hci_dev_lock(hdev);
> >+
> >+ hcon = hci_conn_hash_lookup_handle(hdev, ev->phy_handle);
> >+ if (!hcon) {
> >+ hci_dev_unlock(hdev);
> >+ return;
> >+ }
> >+
> >+ if (ev->status) {
> >+ hci_conn_del(hcon);
> >+ hci_dev_unlock(hdev);
> >+ return;
> >+ }
> >+
> >+ bredr_hcon = hcon->amp_mgr->l2cap_conn->hcon;
> >+
> >+ hcon->state = BT_CONNECTED;
> >+ bacpy(&hcon->dst, &bredr_hcon->dst);
> >+
> >+ hci_conn_hold(hcon);
> >+ hcon->disc_timeout = HCI_DISCONN_TIMEOUT;
> >+ hci_conn_put(hcon);
> >+
> >+ hci_conn_hold_device(hcon);
> >+ hci_conn_add_sysfs(hcon);
> >+
> >+ hci_dev_unlock(hdev);
> >+
> >+ if (hcon->out) {
> >+ struct hci_dev *bredr_hdev = hci_dev_hold(bredr_hcon->hdev);
> >+
> >+ if (!bredr_hdev)
> >+ return;
> >+
> >+ /* Placeholder - create chan req
> >+ l2cap_chan_create_cfm(bredr_hcon, hcon->remote_id);
> >+ */
>
> I think this is where you would call l2cap_physical_cfm(), but that
> function requires more information. Is there enough context in hcon
> to get the local amp ID

This one is easy to manage: hcon->dev->id

> and l2cap_chan, or does the AMP manager need
> to be notified of the physical link so it can match up the physical
> link with other information?

I get chan from amp_mgr through hcon->amp_mgr

I will send the patch later this week.

Best regards
Andrei Emeltchenko

2012-10-26 17:27:28

by Mat Martineau

[permalink] [raw]
Subject: Re: [RFCv1 00/11] Handling physical and logical link


Hi Marcel and Andrei -

On Thu, 25 Oct 2012, Andrei Emeltchenko wrote:

> Hi Marcel,
>
> On Thu, Oct 25, 2012 at 06:11:08AM -0700, Marcel Holtmann wrote:
>> Hi Andrei,
>>
>>> Set of patches adding handling for physical and logical link
>>> complete events.
>>>
>>> Changes:
>>> * rfcv1: Rebased on top of Mat's patches, preserve Marcel's ack for
>>> not-changed-much patches only.
>>> * rfcv0: original
>>
>> all in all, this looks pretty good to me.
>>
>> Mat, can you take an extra look at it as well.
>>
>> What is actually missing after this set of patches?
>
> Still a lot. First it is finishing physical link establishment and then
> channel move.

I looked things over and don't have any major comments or issues.

The main AMP/A2MP/L2CAP task right now is to get the right information
propagated to L2CAP using l2cap_physical_cfm() and
l2cap_logical_cfm(). After that, there are the various "Placeholder"
comments in l2cap_core.c

There is another piece of work involving L2CAP ERTM resegmentation
that depends on MSG_MORE support:

http://www.spinics.net/lists/linux-bluetooth/msg25615.html


--
Mat Martineau

Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

2012-10-26 17:16:53

by Mat Martineau

[permalink] [raw]
Subject: Re: [RFCv1 03/11] Bluetooth: AMP: Process Physical Link Complete evt


Andrei -

On Thu, 25 Oct 2012, Andrei Emeltchenko wrote:

> From: Andrei Emeltchenko <[email protected]>
>
> Add processing for HCI Physical Link Complete event. Upon
> successful status received start L2CAP create channel process.
>
> Signed-off-by: Andrei Emeltchenko <[email protected]>
> Acked-by: Marcel Holtmann <[email protected]>
> ---
> net/bluetooth/hci_event.c | 55 +++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 55 insertions(+)
>
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index aae8053..183d8bd 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -3637,6 +3637,57 @@ unlock:
> hci_dev_unlock(hdev);
> }
>
> +static void hci_phy_link_complete_evt(struct hci_dev *hdev,
> + struct sk_buff *skb)
> +{
> + struct hci_ev_phy_link_complete *ev = (void *) skb->data;
> + struct hci_conn *hcon, *bredr_hcon;
> +
> + BT_DBG("%s handle 0x%2.2x status 0x%2.2x", hdev->name, ev->phy_handle,
> + ev->status);
> +
> + hci_dev_lock(hdev);
> +
> + hcon = hci_conn_hash_lookup_handle(hdev, ev->phy_handle);
> + if (!hcon) {
> + hci_dev_unlock(hdev);
> + return;
> + }
> +
> + if (ev->status) {
> + hci_conn_del(hcon);
> + hci_dev_unlock(hdev);
> + return;
> + }
> +
> + bredr_hcon = hcon->amp_mgr->l2cap_conn->hcon;
> +
> + hcon->state = BT_CONNECTED;
> + bacpy(&hcon->dst, &bredr_hcon->dst);
> +
> + hci_conn_hold(hcon);
> + hcon->disc_timeout = HCI_DISCONN_TIMEOUT;
> + hci_conn_put(hcon);
> +
> + hci_conn_hold_device(hcon);
> + hci_conn_add_sysfs(hcon);
> +
> + hci_dev_unlock(hdev);
> +
> + if (hcon->out) {
> + struct hci_dev *bredr_hdev = hci_dev_hold(bredr_hcon->hdev);
> +
> + if (!bredr_hdev)
> + return;
> +
> + /* Placeholder - create chan req
> + l2cap_chan_create_cfm(bredr_hcon, hcon->remote_id);
> + */

I think this is where you would call l2cap_physical_cfm(), but that
function requires more information. Is there enough context in hcon
to get the local amp ID and l2cap_chan, or does the AMP manager need
to be notified of the physical link so it can match up the physical
link with other information?

> +
> + hci_dev_put(bredr_hdev);
> + }
> +}
> +
> static void hci_le_conn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
> {
> struct hci_ev_le_conn_complete *ev = (void *) skb->data;
> @@ -3964,6 +4015,10 @@ void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb)
> hci_remote_oob_data_request_evt(hdev, skb);
> break;
>
> + case HCI_EV_PHY_LINK_COMPLETE:
> + hci_phy_link_complete_evt(hdev, skb);
> + break;
> +
> case HCI_EV_NUM_COMP_BLOCKS:
> hci_num_comp_blocks_evt(hdev, skb);
> break;
> --
> 1.7.9.5


--
Mat Martineau

Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation


2012-10-26 17:01:01

by Mat Martineau

[permalink] [raw]
Subject: Re: [RFCv1 05/11] Bluetooth: AMP: Add Logical Link Create function


Hi Andrei -

On Thu, 25 Oct 2012, Andrei Emeltchenko wrote:

> From: Andrei Emeltchenko <[email protected]>
>
> After physical link is created logical link needs to be created.
> The process starts after L2CAP channel is created and L2CAP
> Configuration Response with result PENDING is received.
>
> Signed-off-by: Andrei Emeltchenko <[email protected]>
> Acked-by: Marcel Holtmann <[email protected]>
> ---
> include/net/bluetooth/amp.h | 1 +
> net/bluetooth/amp.c | 49 +++++++++++++++++++++++++++++++++++++++++++
> net/bluetooth/hci_event.c | 9 ++++++++
> net/bluetooth/l2cap_core.c | 17 +++++++++++----
> 4 files changed, 72 insertions(+), 4 deletions(-)
>
> diff --git a/include/net/bluetooth/amp.h b/include/net/bluetooth/amp.h
> index 2e7c79e..70d5c15 100644
> --- a/include/net/bluetooth/amp.h
> +++ b/include/net/bluetooth/amp.h
> @@ -46,5 +46,6 @@ void amp_accept_phylink(struct hci_dev *hdev, struct amp_mgr *mgr,
> struct hci_conn *hcon);
> void amp_write_remote_assoc(struct hci_dev *hdev, u8 handle);
> void amp_write_rem_assoc_continue(struct hci_dev *hdev, u8 handle);
> +void amp_create_logical_link(struct l2cap_chan *chan);
>
> #endif /* __AMP_H */
> diff --git a/net/bluetooth/amp.c b/net/bluetooth/amp.c
> index 231d7ef..fbb6360 100644
> --- a/net/bluetooth/amp.c
> +++ b/net/bluetooth/amp.c
> @@ -372,3 +372,52 @@ void amp_accept_phylink(struct hci_dev *hdev, struct amp_mgr *mgr,
>
> hci_send_cmd(hdev, HCI_OP_ACCEPT_PHY_LINK, sizeof(cp), &cp);
> }
> +
> +void amp_create_logical_link(struct l2cap_chan *chan)
> +{
> + struct hci_cp_create_accept_logical_link cp;
> + struct hci_conn *hcon;
> + struct hci_dev *hdev;
> +
> + BT_DBG("chan %p", chan);
> +
> + if (!chan->hs_hcon)
> + return;
> +
> + hdev = hci_dev_hold(chan->hs_hcon->hdev);
> + if (!hdev)
> + return;
> +
> + BT_DBG("chan %p ctrl_id %d dst %pMR", chan, chan->ctrl_id,
> + chan->conn->dst);
> +
> + hcon = hci_conn_hash_lookup_ba(hdev, AMP_LINK, chan->conn->dst);
> + if (!hcon)
> + goto done;
> +
> + cp.phy_handle = hcon->handle;
> +
> + cp.tx_flow_spec.id = chan->local_id;
> + cp.tx_flow_spec.stype = chan->local_stype;
> + cp.tx_flow_spec.msdu = cpu_to_le16(chan->local_msdu);
> + cp.tx_flow_spec.sdu_itime = cpu_to_le32(chan->local_sdu_itime);
> + cp.tx_flow_spec.acc_lat = cpu_to_le32(chan->local_acc_lat);
> + cp.tx_flow_spec.flush_to = cpu_to_le32(chan->local_flush_to);
> +
> + cp.rx_flow_spec.id = chan->remote_id;
> + cp.rx_flow_spec.stype = chan->remote_stype;
> + cp.rx_flow_spec.msdu = cpu_to_le16(chan->remote_msdu);
> + cp.rx_flow_spec.sdu_itime = cpu_to_le32(chan->remote_sdu_itime);
> + cp.rx_flow_spec.acc_lat = cpu_to_le32(chan->remote_acc_lat);
> + cp.rx_flow_spec.flush_to = cpu_to_le32(chan->remote_flush_to);
> +
> + if (hcon->out)
> + hci_send_cmd(hdev, HCI_OP_CREATE_LOGICAL_LINK, sizeof(cp),
> + &cp);
> + else
> + hci_send_cmd(hdev, HCI_OP_ACCEPT_LOGICAL_LINK, sizeof(cp),
> + &cp);
> +
> +done:
> + hci_dev_put(hdev);
> +}
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 00021cb..2c562a7 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -1829,6 +1829,11 @@ static void hci_cs_accept_phylink(struct hci_dev *hdev, u8 status)
> amp_write_remote_assoc(hdev, cp->phy_handle);
> }
>
> +static void hci_cs_create_logical_link(struct hci_dev *hdev, u8 status)
> +{
> + BT_DBG("%s status 0x%2.2x", hdev->name, status);
> +}
> +
> static void hci_inquiry_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
> {
> __u8 status = *((__u8 *) skb->data);
> @@ -2663,6 +2668,10 @@ static void hci_cmd_status_evt(struct hci_dev *hdev, struct sk_buff *skb)
> hci_cs_accept_phylink(hdev, ev->status);
> break;
>
> + case HCI_OP_CREATE_LOGICAL_LINK:
> + hci_cs_create_logical_link(hdev, ev->status);
> + break;
> +
> default:
> BT_DBG("%s opcode 0x%4.4x", hdev->name, opcode);
> break;
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index d1728af..8e1525f 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -38,6 +38,7 @@
> #include <net/bluetooth/l2cap.h>
> #include <net/bluetooth/smp.h>
> #include <net/bluetooth/a2mp.h>
> +#include <net/bluetooth/amp.h>
>
> bool disable_ertm;
>
> @@ -1013,6 +1014,12 @@ static bool __amp_capable(struct l2cap_chan *chan)
> return false;
> }
>
> +static bool l2cap_check_efs(struct l2cap_chan *chan)
> +{
> + /* Check EFS parameters */
> + return true;
> +}
> +
> void l2cap_send_conn_req(struct l2cap_chan *chan)
> {
> struct l2cap_conn *conn = chan->conn;
> @@ -3948,13 +3955,15 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn,
> goto done;
> }
>
> - /* check compatibility */
> -
> if (!chan->ctrl_id)
> l2cap_send_efs_conf_rsp(chan, buf, cmd->ident,
> 0);
> - else
> - chan->ident = cmd->ident;
> + else {
> + if (l2cap_check_efs(chan)) {
> + amp_create_logical_link(chan);
> + chan->ident = cmd->ident;
> + }
> + }

Minor style issue - if one block of an if/else needs braces, then they
all get braces.

> }
> goto done;
>
> --
> 1.7.9.5


--
Mat Martineau

Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

2012-10-25 15:22:08

by Gustavo Padovan

[permalink] [raw]
Subject: Re: [RFCv1 10/11] Bluetooth: Add put(hcon) when deleting hchan

Hi Andrei,

* Andrei Emeltchenko <[email protected]> [2012-10-25 15:20:51 +0300]:

> From: Andrei Emeltchenko <[email protected]>
>
> When refcnt reaches zero disconnect timeout will run and hci_conn
> will be disconnected.
>
> Signed-off-by: Andrei Emeltchenko <[email protected]>
> ---
> net/bluetooth/hci_conn.c | 2 ++
> 1 file changed, 2 insertions(+)

Patches 1-4 and 10 have been applied to bluetooth-next. I'll wait for more
feedback on the others, as this is still an RFC. Thanks.

Gustavo

2012-10-25 13:35:13

by Andrei Emeltchenko

[permalink] [raw]
Subject: Re: [RFCv1 00/11] Handling physical and logical link

Hi Marcel,

On Thu, Oct 25, 2012 at 06:11:08AM -0700, Marcel Holtmann wrote:
> Hi Andrei,
>
> > Set of patches adding handling for physical and logical link
> > complete events.
> >
> > Changes:
> > * rfcv1: Rebased on top of Mat's patches, preserve Marcel's ack for
> > not-changed-much patches only.
> > * rfcv0: original
>
> all in all, this looks pretty good to me.
>
> Mat, can you take an extra look at it as well.
>
> What is actually missing after this set of patches?

Still a lot. First it is finishing physical link establishment and then
channel move.

Best regards
Andrei Emeltchenko


2012-10-25 13:09:41

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [RFCv1 11/11] Bluetooth: AMP: Check for hs_hcon instead of ctrl_id

Hi Andrei,

> When deciding whether to send EFS configuration response with success,
> check rather for existence of High Speed physical link hs_hcon then
> ctrl_id. There might be cases when there is ctrl_id but high speed link
> is not established yet.
>
> Signed-off-by: Andrei Emeltchenko <[email protected]>
> ---
> net/bluetooth/l2cap_core.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)

Acked-by: Marcel Holtmann <[email protected]>

Regards

Marcel



2012-10-25 13:11:08

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [RFCv1 00/11] Handling physical and logical link

Hi Andrei,

> Set of patches adding handling for physical and logical link
> complete events.
>
> Changes:
> * rfcv1: Rebased on top of Mat's patches, preserve Marcel's ack for
> not-changed-much patches only.
> * rfcv0: original

all in all, this looks pretty good to me.

Mat, can you take an extra look at it as well.

What is actually missing after this set of patches?

Regards

Marcel



2012-10-25 13:09:11

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [RFCv1 10/11] Bluetooth: Add put(hcon) when deleting hchan

Hi Andrei,

> When refcnt reaches zero disconnect timeout will run and hci_conn
> will be disconnected.
>
> Signed-off-by: Andrei Emeltchenko <[email protected]>
> ---
> net/bluetooth/hci_conn.c | 2 ++
> 1 file changed, 2 insertions(+)

Acked-by: Marcel Holtmann <[email protected]>

Regards

Marcel



2012-10-25 13:08:22

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [RFCv1 04/11] Bluetooth: AMP: Process Logical Link complete evt

Hi Andrei,

> After receiving HCI Logical Link Complete event finish EFS
> configuration by sending L2CAP Conf Response with success code.
>
> Signed-off-by: Andrei Emeltchenko <[email protected]>
> ---
> include/net/bluetooth/l2cap.h | 2 ++
> net/bluetooth/hci_event.c | 42 +++++++++++++++++++++++++++++++++++++++++
> net/bluetooth/l2cap_core.c | 4 ++--
> 3 files changed, 46 insertions(+), 2 deletions(-)

Acked-by: Marcel Holtmann <[email protected]>

Regards

Marcel



2012-10-25 13:07:19

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [RFCv1 02/11] Bluetooth: Use helper function sending EFS conf rsp

Hi Andrei,

> There is helper function used to send EFS Configuration Response.
>
> Signed-off-by: Andrei Emeltchenko <[email protected]>
> ---
> net/bluetooth/l2cap_core.c | 7 +------
> 1 file changed, 1 insertion(+), 6 deletions(-)

Acked-by: Marcel Holtmann <[email protected]>

Regards

Marcel



2012-10-25 13:06:45

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [RFCv1 01/11] Bluetooth: trivial: Remove unneeded assignment

Hi Andrei,

> Assignment is not needed here since err is always gets value.
>
> Signed-off-by: Andrei Emeltchenko <[email protected]>
> ---
> net/bluetooth/l2cap_core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)

Acked-by: Marcel Holtmann <[email protected]>

Regards

Marcel



2012-10-25 12:20:45

by Andrei Emeltchenko

[permalink] [raw]
Subject: [RFCv1 04/11] Bluetooth: AMP: Process Logical Link complete evt

From: Andrei Emeltchenko <[email protected]>

After receiving HCI Logical Link Complete event finish EFS
configuration by sending L2CAP Conf Response with success code.

Signed-off-by: Andrei Emeltchenko <[email protected]>
---
include/net/bluetooth/l2cap.h | 2 ++
net/bluetooth/hci_event.c | 42 +++++++++++++++++++++++++++++++++++++++++
net/bluetooth/l2cap_core.c | 4 ++--
3 files changed, 46 insertions(+), 2 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 49783e9..24c61ef 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -810,5 +810,7 @@ void __l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan);
void l2cap_chan_del(struct l2cap_chan *chan, int err);
void l2cap_send_conn_req(struct l2cap_chan *chan);
void l2cap_move_start(struct l2cap_chan *chan);
+void l2cap_logical_cfm(struct l2cap_chan *chan, struct hci_chan *hchan,
+ u8 status);

#endif /* __L2CAP_H */
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 183d8bd..00021cb 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -3688,6 +3688,44 @@ static void hci_phy_link_complete_evt(struct hci_dev *hdev,
}
}

+static void hci_loglink_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
+{
+ struct hci_ev_logical_link_complete *ev = (void *) skb->data;
+ struct hci_conn *hcon;
+ struct hci_chan *hchan;
+ struct amp_mgr *mgr;
+
+ BT_DBG("%s log_handle 0x%4.4x phy_handle 0x%2.2x status 0x%2.2x",
+ hdev->name, le16_to_cpu(ev->handle), ev->phy_handle,
+ ev->status);
+
+ hcon = hci_conn_hash_lookup_handle(hdev, ev->phy_handle);
+ if (!hcon)
+ return;
+
+ /* Create AMP hchan */
+ hchan = hci_chan_create(hcon);
+ if (!hchan)
+ return;
+
+ hchan->handle = le16_to_cpu(ev->handle);
+
+ BT_DBG("hcon %p mgr %p hchan %p", hcon, hcon->amp_mgr, hchan);
+
+ mgr = hcon->amp_mgr;
+ if (mgr && mgr->bredr_chan) {
+ struct l2cap_chan *bredr_chan = mgr->bredr_chan;
+
+ l2cap_chan_lock(bredr_chan);
+
+ bredr_chan->conn->mtu = hdev->block_mtu;
+ l2cap_logical_cfm(bredr_chan, hchan, 0);
+ hci_conn_hold(hcon);
+
+ l2cap_chan_unlock(bredr_chan);
+ }
+}
+
static void hci_le_conn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
{
struct hci_ev_le_conn_complete *ev = (void *) skb->data;
@@ -4019,6 +4057,10 @@ void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb)
hci_phy_link_complete_evt(hdev, skb);
break;

+ case HCI_EV_LOGICAL_LINK_COMPLETE:
+ hci_loglink_complete_evt(hdev, skb);
+ break;
+
case HCI_EV_NUM_COMP_BLOCKS:
hci_num_comp_blocks_evt(hdev, skb);
break;
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 600d808..d1728af 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -4428,8 +4428,8 @@ static void l2cap_logical_finish_move(struct l2cap_chan *chan,
}

/* Call with chan locked */
-static void l2cap_logical_cfm(struct l2cap_chan *chan, struct hci_chan *hchan,
- u8 status)
+void l2cap_logical_cfm(struct l2cap_chan *chan, struct hci_chan *hchan,
+ u8 status)
{
BT_DBG("chan %p, hchan %p, status %d", chan, hchan, status);

--
1.7.9.5


2012-10-25 12:20:44

by Andrei Emeltchenko

[permalink] [raw]
Subject: [RFCv1 03/11] Bluetooth: AMP: Process Physical Link Complete evt

From: Andrei Emeltchenko <[email protected]>

Add processing for HCI Physical Link Complete event. Upon
successful status received start L2CAP create channel process.

Signed-off-by: Andrei Emeltchenko <[email protected]>
Acked-by: Marcel Holtmann <[email protected]>
---
net/bluetooth/hci_event.c | 55 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 55 insertions(+)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index aae8053..183d8bd 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -3637,6 +3637,57 @@ unlock:
hci_dev_unlock(hdev);
}

+static void hci_phy_link_complete_evt(struct hci_dev *hdev,
+ struct sk_buff *skb)
+{
+ struct hci_ev_phy_link_complete *ev = (void *) skb->data;
+ struct hci_conn *hcon, *bredr_hcon;
+
+ BT_DBG("%s handle 0x%2.2x status 0x%2.2x", hdev->name, ev->phy_handle,
+ ev->status);
+
+ hci_dev_lock(hdev);
+
+ hcon = hci_conn_hash_lookup_handle(hdev, ev->phy_handle);
+ if (!hcon) {
+ hci_dev_unlock(hdev);
+ return;
+ }
+
+ if (ev->status) {
+ hci_conn_del(hcon);
+ hci_dev_unlock(hdev);
+ return;
+ }
+
+ bredr_hcon = hcon->amp_mgr->l2cap_conn->hcon;
+
+ hcon->state = BT_CONNECTED;
+ bacpy(&hcon->dst, &bredr_hcon->dst);
+
+ hci_conn_hold(hcon);
+ hcon->disc_timeout = HCI_DISCONN_TIMEOUT;
+ hci_conn_put(hcon);
+
+ hci_conn_hold_device(hcon);
+ hci_conn_add_sysfs(hcon);
+
+ hci_dev_unlock(hdev);
+
+ if (hcon->out) {
+ struct hci_dev *bredr_hdev = hci_dev_hold(bredr_hcon->hdev);
+
+ if (!bredr_hdev)
+ return;
+
+ /* Placeholder - create chan req
+ l2cap_chan_create_cfm(bredr_hcon, hcon->remote_id);
+ */
+
+ hci_dev_put(bredr_hdev);
+ }
+}
+
static void hci_le_conn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
{
struct hci_ev_le_conn_complete *ev = (void *) skb->data;
@@ -3964,6 +4015,10 @@ void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb)
hci_remote_oob_data_request_evt(hdev, skb);
break;

+ case HCI_EV_PHY_LINK_COMPLETE:
+ hci_phy_link_complete_evt(hdev, skb);
+ break;
+
case HCI_EV_NUM_COMP_BLOCKS:
hci_num_comp_blocks_evt(hdev, skb);
break;
--
1.7.9.5


2012-10-25 12:20:47

by Andrei Emeltchenko

[permalink] [raw]
Subject: [RFCv1 06/11] Bluetooth: AMP: Process Disc Logical Link

From: Andrei Emeltchenko <[email protected]>

Add processing for HCI Disconnection Logical Link Complete
Event.

Signed-off-by: Andrei Emeltchenko <[email protected]>
Acked-by: Marcel Holtmann <[email protected]>
---
include/net/bluetooth/amp.h | 1 +
net/bluetooth/amp.c | 7 +++++++
net/bluetooth/hci_event.c | 28 ++++++++++++++++++++++++++++
3 files changed, 36 insertions(+)

diff --git a/include/net/bluetooth/amp.h b/include/net/bluetooth/amp.h
index 70d5c15..405fb9c 100644
--- a/include/net/bluetooth/amp.h
+++ b/include/net/bluetooth/amp.h
@@ -47,5 +47,6 @@ void amp_accept_phylink(struct hci_dev *hdev, struct amp_mgr *mgr,
void amp_write_remote_assoc(struct hci_dev *hdev, u8 handle);
void amp_write_rem_assoc_continue(struct hci_dev *hdev, u8 handle);
void amp_create_logical_link(struct l2cap_chan *chan);
+void amp_destroy_logical_link(struct hci_chan *hchan, u8 reason);

#endif /* __AMP_H */
diff --git a/net/bluetooth/amp.c b/net/bluetooth/amp.c
index fbb6360..0f3fef3 100644
--- a/net/bluetooth/amp.c
+++ b/net/bluetooth/amp.c
@@ -421,3 +421,10 @@ void amp_create_logical_link(struct l2cap_chan *chan)
done:
hci_dev_put(hdev);
}
+
+void amp_destroy_logical_link(struct hci_chan *hchan, u8 reason)
+{
+ BT_DBG("hchan %p", hchan);
+
+ hci_chan_del(hchan);
+}
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 2c562a7..3dae3ac 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -3735,6 +3735,30 @@ static void hci_loglink_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
}
}

+static void hci_disconn_loglink_complete_evt(struct hci_dev *hdev,
+ struct sk_buff *skb)
+{
+ struct hci_ev_disconn_logical_link_complete *ev = (void *) skb->data;
+ struct hci_chan *hchan;
+
+ BT_DBG("%s log handle 0x%4.4x status 0x%2.2x", hdev->name,
+ le16_to_cpu(ev->handle), ev->status);
+
+ if (ev->status)
+ return;
+
+ hci_dev_lock(hdev);
+
+ hchan = hci_chan_lookup_handle(hdev, le16_to_cpu(ev->handle));
+ if (!hchan)
+ goto unlock;
+
+ amp_destroy_logical_link(hchan, ev->reason);
+
+unlock:
+ hci_dev_unlock(hdev);
+}
+
static void hci_le_conn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
{
struct hci_ev_le_conn_complete *ev = (void *) skb->data;
@@ -4070,6 +4094,10 @@ void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb)
hci_loglink_complete_evt(hdev, skb);
break;

+ case HCI_EV_DISCONN_LOGICAL_LINK_COMPLETE:
+ hci_disconn_loglink_complete_evt(hdev, skb);
+ break;
+
case HCI_EV_NUM_COMP_BLOCKS:
hci_num_comp_blocks_evt(hdev, skb);
break;
--
1.7.9.5


2012-10-25 12:20:46

by Andrei Emeltchenko

[permalink] [raw]
Subject: [RFCv1 05/11] Bluetooth: AMP: Add Logical Link Create function

From: Andrei Emeltchenko <[email protected]>

After physical link is created logical link needs to be created.
The process starts after L2CAP channel is created and L2CAP
Configuration Response with result PENDING is received.

Signed-off-by: Andrei Emeltchenko <[email protected]>
Acked-by: Marcel Holtmann <[email protected]>
---
include/net/bluetooth/amp.h | 1 +
net/bluetooth/amp.c | 49 +++++++++++++++++++++++++++++++++++++++++++
net/bluetooth/hci_event.c | 9 ++++++++
net/bluetooth/l2cap_core.c | 17 +++++++++++----
4 files changed, 72 insertions(+), 4 deletions(-)

diff --git a/include/net/bluetooth/amp.h b/include/net/bluetooth/amp.h
index 2e7c79e..70d5c15 100644
--- a/include/net/bluetooth/amp.h
+++ b/include/net/bluetooth/amp.h
@@ -46,5 +46,6 @@ void amp_accept_phylink(struct hci_dev *hdev, struct amp_mgr *mgr,
struct hci_conn *hcon);
void amp_write_remote_assoc(struct hci_dev *hdev, u8 handle);
void amp_write_rem_assoc_continue(struct hci_dev *hdev, u8 handle);
+void amp_create_logical_link(struct l2cap_chan *chan);

#endif /* __AMP_H */
diff --git a/net/bluetooth/amp.c b/net/bluetooth/amp.c
index 231d7ef..fbb6360 100644
--- a/net/bluetooth/amp.c
+++ b/net/bluetooth/amp.c
@@ -372,3 +372,52 @@ void amp_accept_phylink(struct hci_dev *hdev, struct amp_mgr *mgr,

hci_send_cmd(hdev, HCI_OP_ACCEPT_PHY_LINK, sizeof(cp), &cp);
}
+
+void amp_create_logical_link(struct l2cap_chan *chan)
+{
+ struct hci_cp_create_accept_logical_link cp;
+ struct hci_conn *hcon;
+ struct hci_dev *hdev;
+
+ BT_DBG("chan %p", chan);
+
+ if (!chan->hs_hcon)
+ return;
+
+ hdev = hci_dev_hold(chan->hs_hcon->hdev);
+ if (!hdev)
+ return;
+
+ BT_DBG("chan %p ctrl_id %d dst %pMR", chan, chan->ctrl_id,
+ chan->conn->dst);
+
+ hcon = hci_conn_hash_lookup_ba(hdev, AMP_LINK, chan->conn->dst);
+ if (!hcon)
+ goto done;
+
+ cp.phy_handle = hcon->handle;
+
+ cp.tx_flow_spec.id = chan->local_id;
+ cp.tx_flow_spec.stype = chan->local_stype;
+ cp.tx_flow_spec.msdu = cpu_to_le16(chan->local_msdu);
+ cp.tx_flow_spec.sdu_itime = cpu_to_le32(chan->local_sdu_itime);
+ cp.tx_flow_spec.acc_lat = cpu_to_le32(chan->local_acc_lat);
+ cp.tx_flow_spec.flush_to = cpu_to_le32(chan->local_flush_to);
+
+ cp.rx_flow_spec.id = chan->remote_id;
+ cp.rx_flow_spec.stype = chan->remote_stype;
+ cp.rx_flow_spec.msdu = cpu_to_le16(chan->remote_msdu);
+ cp.rx_flow_spec.sdu_itime = cpu_to_le32(chan->remote_sdu_itime);
+ cp.rx_flow_spec.acc_lat = cpu_to_le32(chan->remote_acc_lat);
+ cp.rx_flow_spec.flush_to = cpu_to_le32(chan->remote_flush_to);
+
+ if (hcon->out)
+ hci_send_cmd(hdev, HCI_OP_CREATE_LOGICAL_LINK, sizeof(cp),
+ &cp);
+ else
+ hci_send_cmd(hdev, HCI_OP_ACCEPT_LOGICAL_LINK, sizeof(cp),
+ &cp);
+
+done:
+ hci_dev_put(hdev);
+}
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 00021cb..2c562a7 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1829,6 +1829,11 @@ static void hci_cs_accept_phylink(struct hci_dev *hdev, u8 status)
amp_write_remote_assoc(hdev, cp->phy_handle);
}

+static void hci_cs_create_logical_link(struct hci_dev *hdev, u8 status)
+{
+ BT_DBG("%s status 0x%2.2x", hdev->name, status);
+}
+
static void hci_inquiry_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
{
__u8 status = *((__u8 *) skb->data);
@@ -2663,6 +2668,10 @@ static void hci_cmd_status_evt(struct hci_dev *hdev, struct sk_buff *skb)
hci_cs_accept_phylink(hdev, ev->status);
break;

+ case HCI_OP_CREATE_LOGICAL_LINK:
+ hci_cs_create_logical_link(hdev, ev->status);
+ break;
+
default:
BT_DBG("%s opcode 0x%4.4x", hdev->name, opcode);
break;
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index d1728af..8e1525f 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -38,6 +38,7 @@
#include <net/bluetooth/l2cap.h>
#include <net/bluetooth/smp.h>
#include <net/bluetooth/a2mp.h>
+#include <net/bluetooth/amp.h>

bool disable_ertm;

@@ -1013,6 +1014,12 @@ static bool __amp_capable(struct l2cap_chan *chan)
return false;
}

+static bool l2cap_check_efs(struct l2cap_chan *chan)
+{
+ /* Check EFS parameters */
+ return true;
+}
+
void l2cap_send_conn_req(struct l2cap_chan *chan)
{
struct l2cap_conn *conn = chan->conn;
@@ -3948,13 +3955,15 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn,
goto done;
}

- /* check compatibility */
-
if (!chan->ctrl_id)
l2cap_send_efs_conf_rsp(chan, buf, cmd->ident,
0);
- else
- chan->ident = cmd->ident;
+ else {
+ if (l2cap_check_efs(chan)) {
+ amp_create_logical_link(chan);
+ chan->ident = cmd->ident;
+ }
+ }
}
goto done;

--
1.7.9.5


2012-10-25 12:20:50

by Andrei Emeltchenko

[permalink] [raw]
Subject: [RFCv1 09/11] Bluetooth: Disconnect logical link when deleteing chan

From: Andrei Emeltchenko <[email protected]>

Disconnect logical link for high speed channel hs_hchan
associated with L2CAP channel chan.

Signed-off-by: Andrei Emeltchenko <[email protected]>
Acked-by: Marcel Holtmann <[email protected]>
---
include/net/bluetooth/amp.h | 1 +
net/bluetooth/amp.c | 14 ++++++++++++++
net/bluetooth/l2cap_core.c | 7 +++++++
3 files changed, 22 insertions(+)

diff --git a/include/net/bluetooth/amp.h b/include/net/bluetooth/amp.h
index 405fb9c..f1c0017 100644
--- a/include/net/bluetooth/amp.h
+++ b/include/net/bluetooth/amp.h
@@ -47,6 +47,7 @@ void amp_accept_phylink(struct hci_dev *hdev, struct amp_mgr *mgr,
void amp_write_remote_assoc(struct hci_dev *hdev, u8 handle);
void amp_write_rem_assoc_continue(struct hci_dev *hdev, u8 handle);
void amp_create_logical_link(struct l2cap_chan *chan);
+void amp_disconnect_logical_link(struct hci_chan *hchan);
void amp_destroy_logical_link(struct hci_chan *hchan, u8 reason);

#endif /* __AMP_H */
diff --git a/net/bluetooth/amp.c b/net/bluetooth/amp.c
index 0f3fef3..917e034 100644
--- a/net/bluetooth/amp.c
+++ b/net/bluetooth/amp.c
@@ -422,6 +422,20 @@ done:
hci_dev_put(hdev);
}

+void amp_disconnect_logical_link(struct hci_chan *hchan)
+{
+ struct hci_conn *hcon = hchan->conn;
+ struct hci_cp_disconn_logical_link cp;
+
+ if (hcon->state != BT_CONNECTED) {
+ BT_DBG("hchan %p not connected", hchan);
+ return;
+ }
+
+ cp.log_handle = cpu_to_le16(hchan->handle);
+ hci_send_cmd(hcon->hdev, HCI_OP_DISCONN_LOGICAL_LINK, sizeof(cp), &cp);
+}
+
void amp_destroy_logical_link(struct hci_chan *hchan, u8 reason)
{
BT_DBG("hchan %p", hchan);
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 8e1525f..e46fd56 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -578,6 +578,13 @@ void l2cap_chan_del(struct l2cap_chan *chan, int err)
mgr->bredr_chan = NULL;
}

+ if (chan->hs_hchan) {
+ struct hci_chan *hs_hchan = chan->hs_hchan;
+
+ BT_DBG("chan %p disconnect hs_hchan %p", chan, hs_hchan);
+ amp_disconnect_logical_link(hs_hchan);
+ }
+
chan->ops->teardown(chan, err);

if (test_bit(CONF_NOT_COMPLETE, &chan->conf_state))
--
1.7.9.5


2012-10-25 12:20:51

by Andrei Emeltchenko

[permalink] [raw]
Subject: [RFCv1 10/11] Bluetooth: Add put(hcon) when deleting hchan

From: Andrei Emeltchenko <[email protected]>

When refcnt reaches zero disconnect timeout will run and hci_conn
will be disconnected.

Signed-off-by: Andrei Emeltchenko <[email protected]>
---
net/bluetooth/hci_conn.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index dc331ce..25bfce0 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -980,6 +980,8 @@ void hci_chan_del(struct hci_chan *chan)

synchronize_rcu();

+ hci_conn_put(conn);
+
skb_queue_purge(&chan->data_q);
kfree(chan);
}
--
1.7.9.5


2012-10-25 12:20:48

by Andrei Emeltchenko

[permalink] [raw]
Subject: [RFCv1 07/11] Bluetooth: AMP: Process Disc Physical Link Complete evt

From: Andrei Emeltchenko <[email protected]>

Add processing for HCI Disconnection Physical Link Complete Event.

Signed-off-by: Andrei Emeltchenko <[email protected]>
Acked-by: Marcel Holtmann <[email protected]>
---
net/bluetooth/hci_event.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 3dae3ac..14c5f3f 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -3759,6 +3759,28 @@ unlock:
hci_dev_unlock(hdev);
}

+static void hci_disconn_phylink_complete_evt(struct hci_dev *hdev,
+ struct sk_buff *skb)
+{
+ struct hci_ev_disconn_phy_link_complete *ev = (void *) skb->data;
+ struct hci_conn *hcon;
+
+ BT_DBG("%s status 0x%2.2x", hdev->name, ev->status);
+
+ if (ev->status)
+ return;
+
+ hci_dev_lock(hdev);
+
+ hcon = hci_conn_hash_lookup_handle(hdev, ev->phy_handle);
+ if (hcon) {
+ hcon->state = BT_CLOSED;
+ hci_conn_del(hcon);
+ }
+
+ hci_dev_unlock(hdev);
+}
+
static void hci_le_conn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
{
struct hci_ev_le_conn_complete *ev = (void *) skb->data;
@@ -4098,6 +4120,10 @@ void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb)
hci_disconn_loglink_complete_evt(hdev, skb);
break;

+ case HCI_EV_DISCONN_PHY_LINK_COMPLETE:
+ hci_disconn_phylink_complete_evt(hdev, skb);
+ break;
+
case HCI_EV_NUM_COMP_BLOCKS:
hci_num_comp_blocks_evt(hdev, skb);
break;
--
1.7.9.5


2012-10-25 12:20:49

by Andrei Emeltchenko

[permalink] [raw]
Subject: [RFCv1 08/11] Bluetooth: AMP: Remove hci_conn receiving error command status

From: Andrei Emeltchenko <[email protected]>

When receiving HCI Event: Command Status for Create Physical Link
with Error code remove AMP hcon.

Signed-off-by: Andrei Emeltchenko <[email protected]>
Acked-by: Marcel Holtmann <[email protected]>
---
net/bluetooth/hci_event.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 14c5f3f..827243d 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1803,14 +1803,23 @@ static void hci_cs_create_phylink(struct hci_dev *hdev, u8 status)

BT_DBG("%s status 0x%2.2x", hdev->name, status);

- if (status)
- return;
-
cp = hci_sent_cmd_data(hdev, HCI_OP_CREATE_PHY_LINK);
if (!cp)
return;

- amp_write_remote_assoc(hdev, cp->phy_handle);
+ hci_dev_lock(hdev);
+
+ if (status) {
+ struct hci_conn *hcon;
+
+ hcon = hci_conn_hash_lookup_handle(hdev, cp->phy_handle);
+ if (hcon)
+ hci_conn_del(hcon);
+ } else {
+ amp_write_remote_assoc(hdev, cp->phy_handle);
+ }
+
+ hci_dev_unlock(hdev);
}

static void hci_cs_accept_phylink(struct hci_dev *hdev, u8 status)
--
1.7.9.5


2012-10-25 12:20:52

by Andrei Emeltchenko

[permalink] [raw]
Subject: [RFCv1 11/11] Bluetooth: AMP: Check for hs_hcon instead of ctrl_id

From: Andrei Emeltchenko <[email protected]>

When deciding whether to send EFS configuration response with success,
check rather for existence of High Speed physical link hs_hcon then
ctrl_id. There might be cases when there is ctrl_id but high speed link
is not established yet.

Signed-off-by: Andrei Emeltchenko <[email protected]>
---
net/bluetooth/l2cap_core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index e46fd56..ac44365 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -3912,7 +3912,7 @@ static inline int l2cap_config_req(struct l2cap_conn *conn,
/* check compatibility */

/* Send rsp for BR/EDR channel */
- if (!chan->ctrl_id)
+ if (!chan->hs_hcon)
l2cap_send_efs_conf_rsp(chan, rsp, cmd->ident, flags);
else
chan->ident = cmd->ident;
@@ -3962,7 +3962,7 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn,
goto done;
}

- if (!chan->ctrl_id)
+ if (!chan->hs_hcon)
l2cap_send_efs_conf_rsp(chan, buf, cmd->ident,
0);
else {
--
1.7.9.5


2012-10-25 12:20:42

by Andrei Emeltchenko

[permalink] [raw]
Subject: [RFCv1 01/11] Bluetooth: trivial: Remove unneeded assignment

From: Andrei Emeltchenko <[email protected]>

Assignment is not needed here since err is always gets value.

Signed-off-by: Andrei Emeltchenko <[email protected]>
---
net/bluetooth/l2cap_core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index fae0c70..962a322 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -4386,7 +4386,7 @@ static void l2cap_logical_finish_create(struct l2cap_chan *chan,
set_bit(CONF_OUTPUT_DONE, &chan->conf_state);

if (test_bit(CONF_INPUT_DONE, &chan->conf_state)) {
- int err = 0;
+ int err;

set_default_fcs(chan);

--
1.7.9.5


2012-10-25 12:20:43

by Andrei Emeltchenko

[permalink] [raw]
Subject: [RFCv1 02/11] Bluetooth: Use helper function sending EFS conf rsp

From: Andrei Emeltchenko <[email protected]>

There is helper function used to send EFS Configuration Response.

Signed-off-by: Andrei Emeltchenko <[email protected]>
---
net/bluetooth/l2cap_core.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 962a322..600d808 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -4374,16 +4374,11 @@ static void l2cap_logical_finish_create(struct l2cap_chan *chan,
struct hci_chan *hchan)
{
struct l2cap_conf_rsp rsp;
- u8 code;

chan->hs_hcon = hchan->conn;
chan->hs_hcon->l2cap_data = chan->conn;

- code = l2cap_build_conf_rsp(chan, &rsp,
- L2CAP_CONF_SUCCESS, 0);
- l2cap_send_cmd(chan->conn, chan->ident, L2CAP_CONF_RSP, code,
- &rsp);
- set_bit(CONF_OUTPUT_DONE, &chan->conf_state);
+ l2cap_send_efs_conf_rsp(chan, &rsp, chan->ident, 0);

if (test_bit(CONF_INPUT_DONE, &chan->conf_state)) {
int err;
--
1.7.9.5


2012-10-25 12:20:41

by Andrei Emeltchenko

[permalink] [raw]
Subject: [RFCv1 00/11] Handling physical and logical link

From: Andrei Emeltchenko <[email protected]>

Set of patches adding handling for physical and logical link
complete events.

Changes:
* rfcv1: Rebased on top of Mat's patches, preserve Marcel's ack for
not-changed-much patches only.
* rfcv0: original

Andrei Emeltchenko (11):
Bluetooth: trivial: Remove unneeded assignment
Bluetooth: Use helper function sending EFS conf rsp
Bluetooth: AMP: Process Physical Link Complete evt
Bluetooth: AMP: Process Logical Link complete evt
Bluetooth: AMP: Add Logical Link Create function
Bluetooth: AMP: Process Disc Logical Link
Bluetooth: AMP: Process Disc Physical Link Complete evt
Bluetooth: AMP: Remove hci_conn receiving error command status
Bluetooth: Disconnect logical link when deleteing chan
Bluetooth: Add put(hcon) when deleting hchan
Bluetooth: AMP: Check for hs_hcon instead of ctrl_id

include/net/bluetooth/amp.h | 3 +
include/net/bluetooth/l2cap.h | 2 +
net/bluetooth/amp.c | 70 ++++++++++++++++
net/bluetooth/hci_conn.c | 2 +
net/bluetooth/hci_event.c | 177 ++++++++++++++++++++++++++++++++++++++++-
net/bluetooth/l2cap_core.c | 41 ++++++----
6 files changed, 276 insertions(+), 19 deletions(-)

--
1.7.9.5


2012-10-18 10:44:46

by Andrei Emeltchenko

[permalink] [raw]
Subject: Re: [RFCv0 0/8] Handling physical and logical link

Hi

On Wed, Oct 17, 2012 at 10:07:02AM -0700, Marcel Holtmann wrote:
> Hi Andrei,
>
> > Set of patches adding handling for physical and logical link
> > complete events.

Those patches depends on 1st patch from Mat last series.

> > Andrei Emeltchenko (8):
> > Bluetooth: AMP: Process Physical Link Complete evt
> > Bluetooth: AMP: Process Logical Link complete evt
> > Bluetooth: Add logical link create function
> > Bluetooth: AMP: Process Disc Logical Link
> > Bluetooth: AMP: Process Disc Physical Link complete evt
> > Bluetooth: AMP: Remove hci_conn receiving error command status
> > Bluetooth: Disconnect logical link when deleteing chan
> > Bluetooth: Add put(hcon) when deleting hchan
>
> I looked over the patches and the look fine to me.
>
> Acked-by: Marcel Holtmann <[email protected]>

Marcel, I will keep you ack even with some rather cosmetic change.

Best regards
Andrei Emeltchenko

2012-10-17 17:07:02

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [RFCv0 0/8] Handling physical and logical link

Hi Andrei,

> Set of patches adding handling for physical and logical link
> complete events.
>
> Andrei Emeltchenko (8):
> Bluetooth: AMP: Process Physical Link Complete evt
> Bluetooth: AMP: Process Logical Link complete evt
> Bluetooth: Add logical link create function
> Bluetooth: AMP: Process Disc Logical Link
> Bluetooth: AMP: Process Disc Physical Link complete evt
> Bluetooth: AMP: Remove hci_conn receiving error command status
> Bluetooth: Disconnect logical link when deleteing chan
> Bluetooth: Add put(hcon) when deleting hchan

I looked over the patches and the look fine to me.

Acked-by: Marcel Holtmann <[email protected]>

Regards

Marcel



2012-10-16 15:01:08

by Andrei Emeltchenko

[permalink] [raw]
Subject: [RFCv0 6/8] Bluetooth: AMP: Remove hci_conn receiving error command status

From: Andrei Emeltchenko <[email protected]>

When receiving HCI Event: Command Status for Create Physical Link
with Error code remove AMP hcon.

Signed-off-by: Andrei Emeltchenko <[email protected]>
---
net/bluetooth/hci_event.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 3440e52..e534209 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1718,14 +1718,23 @@ static void hci_cs_create_phylink(struct hci_dev *hdev, u8 status)

BT_DBG("%s status 0x%2.2x", hdev->name, status);

- if (status)
- return;
-
cp = hci_sent_cmd_data(hdev, HCI_OP_CREATE_PHY_LINK);
if (!cp)
return;

- amp_write_remote_assoc(hdev, cp->phy_handle);
+ hci_dev_lock(hdev);
+
+ if (status) {
+ struct hci_conn *hcon;
+
+ hcon = hci_conn_hash_lookup_handle(hdev, cp->phy_handle);
+ if (hcon)
+ hci_conn_del(hcon);
+ } else {
+ amp_write_remote_assoc(hdev, cp->phy_handle);
+ }
+
+ hci_dev_unlock(hdev);
}

static void hci_cs_accept_phylink(struct hci_dev *hdev, u8 status)
--
1.7.9.5


2012-10-16 15:01:05

by Andrei Emeltchenko

[permalink] [raw]
Subject: [RFCv0 3/8] Bluetooth: Add logical link create function

From: Andrei Emeltchenko <[email protected]>

After physical link is created logical link needs to be created
above physical link.

Signed-off-by: Andrei Emeltchenko <[email protected]>
---
include/net/bluetooth/amp.h | 1 +
net/bluetooth/amp.c | 46 +++++++++++++++++++++++++++++++++++++++++++
net/bluetooth/hci_event.c | 9 +++++++++
net/bluetooth/l2cap_core.c | 9 +++++++++
4 files changed, 65 insertions(+)

diff --git a/include/net/bluetooth/amp.h b/include/net/bluetooth/amp.h
index 2e7c79e..70d5c15 100644
--- a/include/net/bluetooth/amp.h
+++ b/include/net/bluetooth/amp.h
@@ -46,5 +46,6 @@ void amp_accept_phylink(struct hci_dev *hdev, struct amp_mgr *mgr,
struct hci_conn *hcon);
void amp_write_remote_assoc(struct hci_dev *hdev, u8 handle);
void amp_write_rem_assoc_continue(struct hci_dev *hdev, u8 handle);
+void amp_create_logical_link(struct l2cap_chan *chan);

#endif /* __AMP_H */
diff --git a/net/bluetooth/amp.c b/net/bluetooth/amp.c
index e525e23..6f10e8b 100644
--- a/net/bluetooth/amp.c
+++ b/net/bluetooth/amp.c
@@ -374,3 +374,49 @@ void amp_accept_phylink(struct hci_dev *hdev, struct amp_mgr *mgr,

hci_send_cmd(hdev, HCI_OP_ACCEPT_PHY_LINK, sizeof(cp), &cp);
}
+
+void amp_create_logical_link(struct l2cap_chan *chan)
+{
+ struct hci_cp_create_accept_logical_link cp;
+ struct hci_conn *hcon;
+ struct hci_dev *hdev;
+
+ BT_DBG("chan %p", chan);
+
+ hdev = hci_dev_get(chan->ctrl_id);
+ if (!hdev)
+ return;
+
+ BT_DBG("chan %p ctrl_id %d dst %pMR", chan, chan->ctrl_id,
+ chan->conn->dst);
+
+ hcon = hci_conn_hash_lookup_ba(hdev, AMP_LINK, chan->conn->dst);
+ if (!hcon)
+ goto done;
+
+ cp.phy_handle = hcon->handle;
+
+ cp.tx_flow_spec.id = chan->local_id;
+ cp.tx_flow_spec.stype = chan->local_stype;
+ cp.tx_flow_spec.msdu = cpu_to_le16(chan->local_msdu);
+ cp.tx_flow_spec.sdu_itime = cpu_to_le32(chan->local_sdu_itime);
+ cp.tx_flow_spec.acc_lat = cpu_to_le32(chan->local_acc_lat);
+ cp.tx_flow_spec.flush_to = cpu_to_le32(chan->local_flush_to);
+
+ cp.rx_flow_spec.id = chan->remote_id;
+ cp.rx_flow_spec.stype = chan->remote_stype;
+ cp.rx_flow_spec.msdu = cpu_to_le16(chan->remote_msdu);
+ cp.rx_flow_spec.sdu_itime = cpu_to_le32(chan->remote_sdu_itime);
+ cp.rx_flow_spec.acc_lat = cpu_to_le32(chan->remote_acc_lat);
+ cp.rx_flow_spec.flush_to = cpu_to_le32(chan->remote_flush_to);
+
+ if (hcon->out)
+ hci_send_cmd(hdev, HCI_OP_CREATE_LOGICAL_LINK, sizeof(cp),
+ &cp);
+ else
+ hci_send_cmd(hdev, HCI_OP_ACCEPT_LOGICAL_LINK, sizeof(cp),
+ &cp);
+
+done:
+ hci_dev_put(hdev);
+}
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 2c37896..3f99214 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1744,6 +1744,11 @@ static void hci_cs_accept_phylink(struct hci_dev *hdev, u8 status)
amp_write_remote_assoc(hdev, cp->phy_handle);
}

+static void hci_cs_create_logical_link(struct hci_dev *hdev, u8 status)
+{
+ BT_DBG("%s status 0x%2.2x", hdev->name, status);
+}
+
static void hci_inquiry_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
{
__u8 status = *((__u8 *) skb->data);
@@ -2570,6 +2575,10 @@ static void hci_cmd_status_evt(struct hci_dev *hdev, struct sk_buff *skb)
hci_cs_accept_phylink(hdev, ev->status);
break;

+ case HCI_OP_CREATE_LOGICAL_LINK:
+ hci_cs_create_logical_link(hdev, ev->status);
+ break;
+
default:
BT_DBG("%s opcode 0x%4.4x", hdev->name, opcode);
break;
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index f3cfe89..382d92a 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -38,6 +38,7 @@
#include <net/bluetooth/l2cap.h>
#include <net/bluetooth/smp.h>
#include <net/bluetooth/a2mp.h>
+#include <net/bluetooth/amp.h>

bool disable_ertm;

@@ -964,6 +965,12 @@ static bool __amp_capable(struct l2cap_chan *chan)
return false;
}

+static bool l2cap_check_efs(struct l2cap_chan *chan)
+{
+ /* Check EFS parameters */
+ return true;
+}
+
void l2cap_send_conn_req(struct l2cap_chan *chan)
{
struct l2cap_conn *conn = chan->conn;
@@ -3801,6 +3808,8 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn,
}

/* check compatibility */
+ if (l2cap_check_efs(chan))
+ amp_create_logical_link(chan);

if (!chan->ctrl_id)
l2cap_send_efs_conf_rsp(chan, buf, cmd->ident,
--
1.7.9.5


2012-10-16 15:01:03

by Andrei Emeltchenko

[permalink] [raw]
Subject: [RFCv0 1/8] Bluetooth: AMP: Process Physical Link Complete evt

From: Andrei Emeltchenko <[email protected]>

Add processing for HCI Physical Link Complete event. Upon
successful status received start L2CAP create channel process.

Signed-off-by: Andrei Emeltchenko <[email protected]>
---
net/bluetooth/hci_event.c | 52 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 52 insertions(+)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 0383635..a1ce09f 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -3544,6 +3544,54 @@ unlock:
hci_dev_unlock(hdev);
}

+static void hci_phy_link_complete_evt(struct hci_dev *hdev,
+ struct sk_buff *skb)
+{
+ struct hci_ev_phy_link_complete *ev = (void *) skb->data;
+ struct hci_conn *hcon, *bredr_hcon;
+
+ BT_DBG("%s handle 0x%2.2x status 0x%2.2x", hdev->name, ev->phy_handle,
+ ev->status);
+
+ hci_dev_lock(hdev);
+
+ hcon = hci_conn_hash_lookup_handle(hdev, ev->phy_handle);
+ if (!hcon) {
+ hci_dev_unlock(hdev);
+ return;
+ }
+
+ if (ev->status) {
+ hci_conn_del(hcon);
+ hci_dev_unlock(hdev);
+ return;
+ }
+
+ bredr_hcon = hcon->amp_mgr->l2cap_conn->hcon;
+
+ hcon->state = BT_CONNECTED;
+ bacpy(&hcon->dst, &bredr_hcon->dst);
+
+ hci_conn_hold(hcon);
+ hcon->disc_timeout = HCI_DISCONN_TIMEOUT;
+ hci_conn_put(hcon);
+
+ hci_conn_hold_device(hcon);
+ hci_conn_add_sysfs(hcon);
+
+ hci_dev_unlock(hdev);
+
+ if (hcon->out) {
+ struct hci_dev *bredr_hdev = bredr_hcon->hdev;
+
+ hci_dev_hold(bredr_hdev);
+ /* Placeholder - create chan req
+ l2cap_chan_create_cfm(bredr_hcon, hcon->remote_id);
+ */
+ hci_dev_put(bredr_hdev);
+ }
+}
+
static void hci_le_conn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
{
struct hci_ev_le_conn_complete *ev = (void *) skb->data;
@@ -3871,6 +3919,10 @@ void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb)
hci_remote_oob_data_request_evt(hdev, skb);
break;

+ case HCI_EV_PHY_LINK_COMPLETE:
+ hci_phy_link_complete_evt(hdev, skb);
+ break;
+
case HCI_EV_NUM_COMP_BLOCKS:
hci_num_comp_blocks_evt(hdev, skb);
break;
--
1.7.9.5


2012-10-16 15:01:04

by Andrei Emeltchenko

[permalink] [raw]
Subject: [RFCv0 2/8] Bluetooth: AMP: Process Logical Link complete evt

From: Andrei Emeltchenko <[email protected]>

After receiving HCI Logical Link Complete event finish EFS
configuration by sending L2CAP Conf Response with success code.

Signed-off-by: Andrei Emeltchenko <[email protected]>
---
include/net/bluetooth/l2cap.h | 1 +
net/bluetooth/hci_event.c | 42 +++++++++++++++++++++++++++++++++++++++++
net/bluetooth/l2cap_core.c | 17 +++++++++++++++++
3 files changed, 60 insertions(+)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 6d3615e..150199e 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -807,5 +807,6 @@ void l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan);
void __l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan);
void l2cap_chan_del(struct l2cap_chan *chan, int err);
void l2cap_send_conn_req(struct l2cap_chan *chan);
+void l2cap_finish_efs_config(struct l2cap_chan *chan);

#endif /* __L2CAP_H */
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index a1ce09f..2c37896 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -3592,6 +3592,44 @@ static void hci_phy_link_complete_evt(struct hci_dev *hdev,
}
}

+static void hci_loglink_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
+{
+ struct hci_ev_logical_link_complete *ev = (void *) skb->data;
+ struct hci_conn *hcon;
+ struct hci_chan *hchan;
+ struct amp_mgr *mgr;
+
+ BT_DBG("%s log_handle 0x%4.4x phy_handle 0x%2.2x status 0x%2.2x",
+ hdev->name, le16_to_cpu(ev->handle), ev->phy_handle,
+ ev->status);
+
+ hcon = hci_conn_hash_lookup_handle(hdev, ev->phy_handle);
+ if (!hcon)
+ return;
+
+ /* Create AMP hchan */
+ hchan = hci_chan_create(hcon);
+ if (!hchan)
+ return;
+
+ hchan->handle = le16_to_cpu(ev->handle);
+
+ BT_DBG("hcon %p mgr %p hchan %p", hcon, hcon->amp_mgr, hchan);
+
+ mgr = hcon->amp_mgr;
+ if (mgr && mgr->bredr_chan) {
+ struct l2cap_chan *bredr_chan = mgr->bredr_chan;
+
+ bredr_chan->hs_hcon = hcon;
+ bredr_chan->hs_hchan = hchan;
+ hcon->l2cap_data = bredr_chan->conn;
+
+ l2cap_finish_efs_config(bredr_chan);
+ hci_conn_hold(hcon);
+ bredr_chan->conn->mtu = hdev->block_mtu;
+ }
+}
+
static void hci_le_conn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
{
struct hci_ev_le_conn_complete *ev = (void *) skb->data;
@@ -3923,6 +3961,10 @@ void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb)
hci_phy_link_complete_evt(hdev, skb);
break;

+ case HCI_EV_LOGICAL_LINK_COMPLETE:
+ hci_loglink_complete_evt(hdev, skb);
+ break;
+
case HCI_EV_NUM_COMP_BLOCKS:
hci_num_comp_blocks_evt(hdev, skb);
break;
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 4035fce..f3cfe89 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -3635,6 +3635,23 @@ static void l2cap_send_efs_conf_rsp(struct l2cap_chan *chan, void *data,
L2CAP_CONF_SUCCESS, flags), data);
}

+void l2cap_finish_efs_config(struct l2cap_chan *chan)
+{
+ char buf[64];
+
+ BT_DBG("chan %p", chan);
+
+ /* Init also BR/EDR chan */
+ l2cap_ertm_init(chan);
+
+ l2cap_send_efs_conf_rsp(chan, buf, chan->ident, 0);
+
+ if (test_bit(CONF_INPUT_DONE, &chan->conf_state)) {
+ set_default_fcs(chan);
+ l2cap_chan_ready(chan);
+ }
+}
+
static inline int l2cap_config_req(struct l2cap_conn *conn,
struct l2cap_cmd_hdr *cmd, u16 cmd_len,
u8 *data)
--
1.7.9.5


2012-10-16 15:01:10

by Andrei Emeltchenko

[permalink] [raw]
Subject: [RFCv0 8/8] Bluetooth: Add put(hcon) when deleting hchan

From: Andrei Emeltchenko <[email protected]>

When refcnt reaches zero disconnect timeout will run and hci_conn
will be disconnected.

Signed-off-by: Andrei Emeltchenko <[email protected]>
---
net/bluetooth/hci_conn.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index fe64621..6f9c2b3 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -976,6 +976,8 @@ void hci_chan_del(struct hci_chan *chan)

synchronize_rcu();

+ hci_conn_put(conn);
+
skb_queue_purge(&chan->data_q);
kfree(chan);
}
--
1.7.9.5


2012-10-16 15:01:06

by Andrei Emeltchenko

[permalink] [raw]
Subject: [RFCv0 4/8] Bluetooth: AMP: Process Disc Logical Link

From: Andrei Emeltchenko <[email protected]>

Add processing for HCI Disconnection Logical Link Complete
Event.

Signed-off-by: Andrei Emeltchenko <[email protected]>
---
include/net/bluetooth/amp.h | 1 +
net/bluetooth/amp.c | 7 +++++++
net/bluetooth/hci_event.c | 28 ++++++++++++++++++++++++++++
3 files changed, 36 insertions(+)

diff --git a/include/net/bluetooth/amp.h b/include/net/bluetooth/amp.h
index 70d5c15..405fb9c 100644
--- a/include/net/bluetooth/amp.h
+++ b/include/net/bluetooth/amp.h
@@ -47,5 +47,6 @@ void amp_accept_phylink(struct hci_dev *hdev, struct amp_mgr *mgr,
void amp_write_remote_assoc(struct hci_dev *hdev, u8 handle);
void amp_write_rem_assoc_continue(struct hci_dev *hdev, u8 handle);
void amp_create_logical_link(struct l2cap_chan *chan);
+void amp_destroy_logical_link(struct hci_chan *hchan, u8 reason);

#endif /* __AMP_H */
diff --git a/net/bluetooth/amp.c b/net/bluetooth/amp.c
index 6f10e8b..40c5d95 100644
--- a/net/bluetooth/amp.c
+++ b/net/bluetooth/amp.c
@@ -420,3 +420,10 @@ void amp_create_logical_link(struct l2cap_chan *chan)
done:
hci_dev_put(hdev);
}
+
+void amp_destroy_logical_link(struct hci_chan *hchan, u8 reason)
+{
+ BT_DBG("hchan %p", hchan);
+
+ hci_chan_del(hchan);
+}
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 3f99214..dc24489 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -3639,6 +3639,30 @@ static void hci_loglink_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
}
}

+static void hci_disconn_loglink_complete_evt(struct hci_dev *hdev,
+ struct sk_buff *skb)
+{
+ struct hci_ev_disconn_logical_link_complete *ev = (void *) skb->data;
+ struct hci_chan *hchan;
+
+ BT_DBG("%s log handle 0x%4.4x status 0x%2.2x", hdev->name,
+ le16_to_cpu(ev->handle), ev->status);
+
+ if (ev->status)
+ return;
+
+ hci_dev_lock(hdev);
+
+ hchan = hci_chan_lookup_handle(hdev, le16_to_cpu(ev->handle));
+ if (!hchan)
+ goto unlock;
+
+ amp_destroy_logical_link(hchan, ev->reason);
+
+unlock:
+ hci_dev_unlock(hdev);
+}
+
static void hci_le_conn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
{
struct hci_ev_le_conn_complete *ev = (void *) skb->data;
@@ -3974,6 +3998,10 @@ void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb)
hci_loglink_complete_evt(hdev, skb);
break;

+ case HCI_EV_DISCONN_LOGICAL_LINK_COMPLETE:
+ hci_disconn_loglink_complete_evt(hdev, skb);
+ break;
+
case HCI_EV_NUM_COMP_BLOCKS:
hci_num_comp_blocks_evt(hdev, skb);
break;
--
1.7.9.5


2012-10-16 15:01:09

by Andrei Emeltchenko

[permalink] [raw]
Subject: [RFCv0 7/8] Bluetooth: Disconnect logical link when deleteing chan

From: Andrei Emeltchenko <[email protected]>

Disconnect logical link for high speed channel hs_hchan
associated with L2CAP channel chan.

Signed-off-by: Andrei Emeltchenko <[email protected]>
---
include/net/bluetooth/amp.h | 1 +
net/bluetooth/amp.c | 14 ++++++++++++++
net/bluetooth/l2cap_core.c | 7 +++++++
3 files changed, 22 insertions(+)

diff --git a/include/net/bluetooth/amp.h b/include/net/bluetooth/amp.h
index 405fb9c..f1c0017 100644
--- a/include/net/bluetooth/amp.h
+++ b/include/net/bluetooth/amp.h
@@ -47,6 +47,7 @@ void amp_accept_phylink(struct hci_dev *hdev, struct amp_mgr *mgr,
void amp_write_remote_assoc(struct hci_dev *hdev, u8 handle);
void amp_write_rem_assoc_continue(struct hci_dev *hdev, u8 handle);
void amp_create_logical_link(struct l2cap_chan *chan);
+void amp_disconnect_logical_link(struct hci_chan *hchan);
void amp_destroy_logical_link(struct hci_chan *hchan, u8 reason);

#endif /* __AMP_H */
diff --git a/net/bluetooth/amp.c b/net/bluetooth/amp.c
index 40c5d95..0cbc325 100644
--- a/net/bluetooth/amp.c
+++ b/net/bluetooth/amp.c
@@ -421,6 +421,20 @@ done:
hci_dev_put(hdev);
}

+void amp_disconnect_logical_link(struct hci_chan *hchan)
+{
+ struct hci_conn *hcon = hchan->conn;
+ struct hci_cp_disconn_logical_link cp;
+
+ if (hcon->state != BT_CONNECTED) {
+ BT_DBG("hchan %p not connected", hchan);
+ return;
+ }
+
+ cp.log_handle = cpu_to_le16(hchan->handle);
+ hci_send_cmd(hcon->hdev, HCI_OP_DISCONN_LOGICAL_LINK, sizeof(cp), &cp);
+}
+
void amp_destroy_logical_link(struct hci_chan *hchan, u8 reason)
{
BT_DBG("hchan %p", hchan);
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 382d92a..748f022 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -547,6 +547,13 @@ void l2cap_chan_del(struct l2cap_chan *chan, int err)
mgr->bredr_chan = NULL;
}

+ if (chan->hs_hchan) {
+ struct hci_chan *hs_hchan = chan->hs_hchan;
+
+ BT_DBG("chan %p disconnect hs_hchan %p", chan, hs_hchan);
+ amp_disconnect_logical_link(hs_hchan);
+ }
+
chan->ops->teardown(chan, err);

if (test_bit(CONF_NOT_COMPLETE, &chan->conf_state))
--
1.7.9.5


2012-10-16 15:01:07

by Andrei Emeltchenko

[permalink] [raw]
Subject: [RFCv0 5/8] Bluetooth: AMP: Process Disc Physical Link complete evt

From: Andrei Emeltchenko <[email protected]>

Add processing for HCI Disconnection Physical Link Complete Event.

Signed-off-by: Andrei Emeltchenko <[email protected]>
---
net/bluetooth/hci_event.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index dc24489..3440e52 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -3663,6 +3663,28 @@ unlock:
hci_dev_unlock(hdev);
}

+static void hci_disconn_phylink_complete_evt(struct hci_dev *hdev,
+ struct sk_buff *skb)
+{
+ struct hci_ev_disconn_phy_link_complete *ev = (void *) skb->data;
+ struct hci_conn *hcon;
+
+ BT_DBG("%s status %d", hdev->name, ev->status);
+
+ if (ev->status)
+ return;
+
+ hci_dev_lock(hdev);
+
+ hcon = hci_conn_hash_lookup_handle(hdev, ev->phy_handle);
+ if (hcon) {
+ hcon->state = BT_CLOSED;
+ hci_conn_del(hcon);
+ }
+
+ hci_dev_unlock(hdev);
+}
+
static void hci_le_conn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
{
struct hci_ev_le_conn_complete *ev = (void *) skb->data;
@@ -4002,6 +4024,10 @@ void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb)
hci_disconn_loglink_complete_evt(hdev, skb);
break;

+ case HCI_EV_DISCONN_PHY_LINK_COMPLETE:
+ hci_disconn_phylink_complete_evt(hdev, skb);
+ break;
+
case HCI_EV_NUM_COMP_BLOCKS:
hci_num_comp_blocks_evt(hdev, skb);
break;
--
1.7.9.5


2012-11-14 12:19:20

by Andrei Emeltchenko

[permalink] [raw]
Subject: Re: [PATCHv1 11/12] Bluetooth: AMP: Use l2cap_physical_cfm in phylink complete evt

Hi Mat,

On Tue, Nov 13, 2012 at 09:29:38AM -0800, Mat Martineau wrote:
> >>>>>+void amp_physical_cfm(struct hci_conn *bredr_hcon, struct hci_conn *hs_hcon)
> >>>>>+{
> >>>>>+ struct hci_dev *bredr_hdev = hci_dev_hold(bredr_hcon->hdev);
> >>>>>+ struct amp_mgr *mgr = hs_hcon->amp_mgr;
> >>>>>+ struct l2cap_chan *bredr_chan;
> >>>>>+
> >>>>>+ BT_DBG("bredr_hcon %p hs_hcon %p mgr %p", bredr_hcon, hs_hcon, mgr);
> >>>>>+
> >>>>>+ if (!bredr_hdev || !mgr || !mgr->bredr_chan)
> >>>>>+ return;
> >>>>>+
> >>>>>+ bredr_chan = mgr->bredr_chan;
> >>>>>+
> >>>>>+ set_bit(FLAG_EFS_ENABLE, &bredr_chan->flags);
> >>>>>+ bredr_chan->ctrl_id = hs_hcon->remote_id;
> >>>>>+ bredr_chan->hs_hcon = hs_hcon;
> >>>>>+ bredr_chan->conn->mtu = hs_hcon->hdev->block_mtu;
> >>>>>+ bredr_chan->fcs = L2CAP_FCS_NONE;
> >>
> >>Changing FCS requires L2CAP reconfiguration for the channel, and
> >>chan->fcs shouldn't be modified until reconfiguration happens.
> >>While it doesn't make much sense to do so, the remote device may
> >>want to keep FCS enabled. The move may also fail and you don't want
> >>to forget the original FCS setting in that case.
> >
> >So we agree that FCS shall not be used for High Speed channels. I was
> >thinking more about the case where we start sending data right over HS
> >channel. The configuration should be just started.
>
> No matter what the BlueZ implementation prefers, it must be able to
> handle a remote device that requests FCS during L2CAP config. If
> one side requests FCS, then it must be used.

The idea here is to disable FCS on our side, we would still respect FCS
requests from remote side. This code needs to be moved to l2cap_do_create.

> Do we want BlueZ to always ignore the L2CAP FCS socket option on AMP
> controllers? (Marcel?) This checksum is always redundant with
> 802.11 AMP controllers.
>
> >What would be the better place to init FCS? l2cap_physical_cfm after
> >checking channel moving status?
>
> I think it should be as late as possible before L2CAP configuration
> begins in order to be sure the channel is really on AMP. For the
> "create channel" case, set_default_fcs could see if an AMP link is
> being used and turn the FCS off.

set_default_fcs sets FCS when configuration is finished, this way we
cannot respect other side FCS settings.

Best regards
Andrei Emeltchenko

2012-11-13 22:49:24

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCHv1 11/12] Bluetooth: AMP: Use l2cap_physical_cfm in phylink complete evt

Hi Mat,

> > ...
> >>>>> +void amp_physical_cfm(struct hci_conn *bredr_hcon, struct hci_conn *hs_hcon)
> >>>>> +{
> >>>>> + struct hci_dev *bredr_hdev = hci_dev_hold(bredr_hcon->hdev);
> >>>>> + struct amp_mgr *mgr = hs_hcon->amp_mgr;
> >>>>> + struct l2cap_chan *bredr_chan;
> >>>>> +
> >>>>> + BT_DBG("bredr_hcon %p hs_hcon %p mgr %p", bredr_hcon, hs_hcon, mgr);
> >>>>> +
> >>>>> + if (!bredr_hdev || !mgr || !mgr->bredr_chan)
> >>>>> + return;
> >>>>> +
> >>>>> + bredr_chan = mgr->bredr_chan;
> >>>>> +
> >>>>> + set_bit(FLAG_EFS_ENABLE, &bredr_chan->flags);
> >>>>> + bredr_chan->ctrl_id = hs_hcon->remote_id;
> >>>>> + bredr_chan->hs_hcon = hs_hcon;
> >>>>> + bredr_chan->conn->mtu = hs_hcon->hdev->block_mtu;
> >>>>> + bredr_chan->fcs = L2CAP_FCS_NONE;
> >>
> >> Changing FCS requires L2CAP reconfiguration for the channel, and
> >> chan->fcs shouldn't be modified until reconfiguration happens.
> >> While it doesn't make much sense to do so, the remote device may
> >> want to keep FCS enabled. The move may also fail and you don't want
> >> to forget the original FCS setting in that case.
> >
> > So we agree that FCS shall not be used for High Speed channels. I was
> > thinking more about the case where we start sending data right over HS
> > channel. The configuration should be just started.
>
> No matter what the BlueZ implementation prefers, it must be able to
> handle a remote device that requests FCS during L2CAP config. If one
> side requests FCS, then it must be used.
>
> Do we want BlueZ to always ignore the L2CAP FCS socket option on AMP
> controllers? (Marcel?) This checksum is always redundant with 802.11
> AMP controllers.

if it is there, we should check it. Even on 802.11. Mainly just in case
someone tries to sneak packets in. But if by any means possible we
should try to disable FCS on AMP controllers.

Regards

Marcel



2012-11-13 17:29:38

by Mat Martineau

[permalink] [raw]
Subject: Re: [PATCHv1 11/12] Bluetooth: AMP: Use l2cap_physical_cfm in phylink complete evt


Hi Andrei -

On Mon, 12 Nov 2012, Andrei Emeltchenko wrote:

> Hi Mat,
>
> On Fri, Nov 02, 2012 at 08:39:09AM -0700, Mat Martineau wrote:
> ...
>>>>> +void amp_physical_cfm(struct hci_conn *bredr_hcon, struct hci_conn *hs_hcon)
>>>>> +{
>>>>> + struct hci_dev *bredr_hdev = hci_dev_hold(bredr_hcon->hdev);
>>>>> + struct amp_mgr *mgr = hs_hcon->amp_mgr;
>>>>> + struct l2cap_chan *bredr_chan;
>>>>> +
>>>>> + BT_DBG("bredr_hcon %p hs_hcon %p mgr %p", bredr_hcon, hs_hcon, mgr);
>>>>> +
>>>>> + if (!bredr_hdev || !mgr || !mgr->bredr_chan)
>>>>> + return;
>>>>> +
>>>>> + bredr_chan = mgr->bredr_chan;
>>>>> +
>>>>> + set_bit(FLAG_EFS_ENABLE, &bredr_chan->flags);
>>>>> + bredr_chan->ctrl_id = hs_hcon->remote_id;
>>>>> + bredr_chan->hs_hcon = hs_hcon;
>>>>> + bredr_chan->conn->mtu = hs_hcon->hdev->block_mtu;
>>>>> + bredr_chan->fcs = L2CAP_FCS_NONE;
>>
>> Changing FCS requires L2CAP reconfiguration for the channel, and
>> chan->fcs shouldn't be modified until reconfiguration happens.
>> While it doesn't make much sense to do so, the remote device may
>> want to keep FCS enabled. The move may also fail and you don't want
>> to forget the original FCS setting in that case.
>
> So we agree that FCS shall not be used for High Speed channels. I was
> thinking more about the case where we start sending data right over HS
> channel. The configuration should be just started.

No matter what the BlueZ implementation prefers, it must be able to
handle a remote device that requests FCS during L2CAP config. If one
side requests FCS, then it must be used.

Do we want BlueZ to always ignore the L2CAP FCS socket option on AMP
controllers? (Marcel?) This checksum is always redundant with 802.11
AMP controllers.

> What would be the better place to init FCS? l2cap_physical_cfm after
> checking channel moving status?

I think it should be as late as possible before L2CAP configuration
begins in order to be sure the channel is really on AMP. For the
"create channel" case, set_default_fcs could see if an AMP link is
being used and turn the FCS off.

>>>> Sorry I missed this earlier: bredr_chan needs to be locked before
>>>> changing it. I suggest passing the information to
>>>> l2cap_physical_cfm and letting that function update the structure
>>>> members while it holds the lock.
>>>
>>> what about locking here and changing l2cap_physical_cfm to unlocked
>>> __l2cap_physical_cfm ?
>>
>> My preference is to not manipulate l2cap_chan too much outside of
>> l2cap_core, and to keep the channel move or channel create state
>> machines inside l2cap_core. l2cap_physical_cfm checks the channel
>> state before modifying it. The move or new connection may have been
>> canceled or be in the wrong state, in which case the structure
>> should not be modified even though the physical link was completed.
>
> so maybe we shall move some code to l2cap_physical_cfm ?

Yes, I think l2cap_physical_cfm is a better place for changes to
l2cap_chan.


Regards,
--
Mat Martineau

Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

2012-11-12 09:26:43

by Andrei Emeltchenko

[permalink] [raw]
Subject: Re: [PATCHv1 11/12] Bluetooth: AMP: Use l2cap_physical_cfm in phylink complete evt

Hi Mat,

On Fri, Nov 02, 2012 at 08:39:09AM -0700, Mat Martineau wrote:
...
> >>>+void amp_physical_cfm(struct hci_conn *bredr_hcon, struct hci_conn *hs_hcon)
> >>>+{
> >>>+ struct hci_dev *bredr_hdev = hci_dev_hold(bredr_hcon->hdev);
> >>>+ struct amp_mgr *mgr = hs_hcon->amp_mgr;
> >>>+ struct l2cap_chan *bredr_chan;
> >>>+
> >>>+ BT_DBG("bredr_hcon %p hs_hcon %p mgr %p", bredr_hcon, hs_hcon, mgr);
> >>>+
> >>>+ if (!bredr_hdev || !mgr || !mgr->bredr_chan)
> >>>+ return;
> >>>+
> >>>+ bredr_chan = mgr->bredr_chan;
> >>>+
> >>>+ set_bit(FLAG_EFS_ENABLE, &bredr_chan->flags);
> >>>+ bredr_chan->ctrl_id = hs_hcon->remote_id;
> >>>+ bredr_chan->hs_hcon = hs_hcon;
> >>>+ bredr_chan->conn->mtu = hs_hcon->hdev->block_mtu;
> >>>+ bredr_chan->fcs = L2CAP_FCS_NONE;
>
> Changing FCS requires L2CAP reconfiguration for the channel, and
> chan->fcs shouldn't be modified until reconfiguration happens.
> While it doesn't make much sense to do so, the remote device may
> want to keep FCS enabled. The move may also fail and you don't want
> to forget the original FCS setting in that case.

So we agree that FCS shall not be used for High Speed channels. I was
thinking more about the case where we start sending data right over HS
channel. The configuration should be just started.

What would be the better place to init FCS? l2cap_physical_cfm after
checking channel moving status?

> >>Sorry I missed this earlier: bredr_chan needs to be locked before
> >>changing it. I suggest passing the information to
> >>l2cap_physical_cfm and letting that function update the structure
> >>members while it holds the lock.
> >
> >what about locking here and changing l2cap_physical_cfm to unlocked
> >__l2cap_physical_cfm ?
>
> My preference is to not manipulate l2cap_chan too much outside of
> l2cap_core, and to keep the channel move or channel create state
> machines inside l2cap_core. l2cap_physical_cfm checks the channel
> state before modifying it. The move or new connection may have been
> canceled or be in the wrong state, in which case the structure
> should not be modified even though the physical link was completed.

so maybe we shall move some code to l2cap_physical_cfm ?

Best regards
Andrei Emeltchenko


2012-11-02 15:39:09

by Mat Martineau

[permalink] [raw]
Subject: Re: [PATCHv1 11/12] Bluetooth: AMP: Use l2cap_physical_cfm in phylink complete evt


Andrei -

On Fri, 2 Nov 2012, Andrei Emeltchenko wrote:

> Hi Mat,
>
> On Thu, Nov 01, 2012 at 10:51:19AM -0700, Mat Martineau wrote:
>>
>> Andrei -
>>
>> On Wed, 31 Oct 2012, Andrei Emeltchenko wrote:
>>
>>> From: Andrei Emeltchenko <[email protected]>
>>>
>>> When receiving HCI Phylink Complete event run amp_physical_cfm
>>> which initialize BR/EDR L2CAP channel associated with High Speed
>>> link and run l2cap_physical_cfm which shall send L2CAP Create
>>> Chan Request.
>>>
>>> Signed-off-by: Andrei Emeltchenko <[email protected]>
>>> Acked-by: Marcel Holtmann <[email protected]>
>>> ---
>>> include/net/bluetooth/amp.h | 1 +
>>> include/net/bluetooth/l2cap.h | 1 +
>>> net/bluetooth/amp.c | 24 ++++++++++++++++++++++++
>>> net/bluetooth/hci_event.c | 15 ++-------------
>>> 4 files changed, 28 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/include/net/bluetooth/amp.h b/include/net/bluetooth/amp.h
>>> index f1c0017..7ea3db7 100644
>>> --- a/include/net/bluetooth/amp.h
>>> +++ b/include/net/bluetooth/amp.h
>>> @@ -46,6 +46,7 @@ void amp_accept_phylink(struct hci_dev *hdev, struct amp_mgr *mgr,
>>> struct hci_conn *hcon);
>>> void amp_write_remote_assoc(struct hci_dev *hdev, u8 handle);
>>> void amp_write_rem_assoc_continue(struct hci_dev *hdev, u8 handle);
>>> +void amp_physical_cfm(struct hci_conn *bredr_hcon, struct hci_conn *hs_hcon);
>>> void amp_create_logical_link(struct l2cap_chan *chan);
>>> void amp_disconnect_logical_link(struct hci_chan *hchan);
>>> void amp_destroy_logical_link(struct hci_chan *hchan, u8 reason);
>>> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
>>> index 24c61ef..18149c8 100644
>>> --- a/include/net/bluetooth/l2cap.h
>>> +++ b/include/net/bluetooth/l2cap.h
>>> @@ -812,5 +812,6 @@ void l2cap_send_conn_req(struct l2cap_chan *chan);
>>> void l2cap_move_start(struct l2cap_chan *chan);
>>> void l2cap_logical_cfm(struct l2cap_chan *chan, struct hci_chan *hchan,
>>> u8 status);
>>> +void l2cap_physical_cfm(struct l2cap_chan *chan, int result);
>>>
>>> #endif /* __L2CAP_H */
>>> diff --git a/net/bluetooth/amp.c b/net/bluetooth/amp.c
>>> index 917e034..650bb8d 100644
>>> --- a/net/bluetooth/amp.c
>>> +++ b/net/bluetooth/amp.c
>>> @@ -373,6 +373,30 @@ void amp_accept_phylink(struct hci_dev *hdev, struct amp_mgr *mgr,
>>> hci_send_cmd(hdev, HCI_OP_ACCEPT_PHY_LINK, sizeof(cp), &cp);
>>> }
>>>
>>> +void amp_physical_cfm(struct hci_conn *bredr_hcon, struct hci_conn *hs_hcon)
>>> +{
>>> + struct hci_dev *bredr_hdev = hci_dev_hold(bredr_hcon->hdev);
>>> + struct amp_mgr *mgr = hs_hcon->amp_mgr;
>>> + struct l2cap_chan *bredr_chan;
>>> +
>>> + BT_DBG("bredr_hcon %p hs_hcon %p mgr %p", bredr_hcon, hs_hcon, mgr);
>>> +
>>> + if (!bredr_hdev || !mgr || !mgr->bredr_chan)
>>> + return;
>>> +
>>> + bredr_chan = mgr->bredr_chan;
>>> +
>>> + set_bit(FLAG_EFS_ENABLE, &bredr_chan->flags);
>>> + bredr_chan->ctrl_id = hs_hcon->remote_id;
>>> + bredr_chan->hs_hcon = hs_hcon;
>>> + bredr_chan->conn->mtu = hs_hcon->hdev->block_mtu;
>>> + bredr_chan->fcs = L2CAP_FCS_NONE;

Changing FCS requires L2CAP reconfiguration for the channel, and
chan->fcs shouldn't be modified until reconfiguration happens. While
it doesn't make much sense to do so, the remote device may want to
keep FCS enabled. The move may also fail and you don't want to forget
the original FCS setting in that case.

>>
>> Sorry I missed this earlier: bredr_chan needs to be locked before
>> changing it. I suggest passing the information to
>> l2cap_physical_cfm and letting that function update the structure
>> members while it holds the lock.
>
> what about locking here and changing l2cap_physical_cfm to unlocked
> __l2cap_physical_cfm ?

My preference is to not manipulate l2cap_chan too much outside of
l2cap_core, and to keep the channel move or channel create state
machines inside l2cap_core. l2cap_physical_cfm checks the channel
state before modifying it. The move or new connection may have been
canceled or be in the wrong state, in which case the structure should
not be modified even though the physical link was completed.

>
>>
>>
>>> +
>>> + l2cap_physical_cfm(bredr_chan, 0);
>>> +
>>> + hci_dev_put(bredr_hdev);
>>> +}

Regards,

--
Mat Martineau

Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation


2012-11-02 07:48:51

by Andrei Emeltchenko

[permalink] [raw]
Subject: Re: [PATCHv1 11/12] Bluetooth: AMP: Use l2cap_physical_cfm in phylink complete evt

Hi Mat,

On Thu, Nov 01, 2012 at 10:51:19AM -0700, Mat Martineau wrote:
>
> Andrei -
>
> On Wed, 31 Oct 2012, Andrei Emeltchenko wrote:
>
> >From: Andrei Emeltchenko <[email protected]>
> >
> >When receiving HCI Phylink Complete event run amp_physical_cfm
> >which initialize BR/EDR L2CAP channel associated with High Speed
> >link and run l2cap_physical_cfm which shall send L2CAP Create
> >Chan Request.
> >
> >Signed-off-by: Andrei Emeltchenko <[email protected]>
> >Acked-by: Marcel Holtmann <[email protected]>
> >---
> >include/net/bluetooth/amp.h | 1 +
> >include/net/bluetooth/l2cap.h | 1 +
> >net/bluetooth/amp.c | 24 ++++++++++++++++++++++++
> >net/bluetooth/hci_event.c | 15 ++-------------
> >4 files changed, 28 insertions(+), 13 deletions(-)
> >
> >diff --git a/include/net/bluetooth/amp.h b/include/net/bluetooth/amp.h
> >index f1c0017..7ea3db7 100644
> >--- a/include/net/bluetooth/amp.h
> >+++ b/include/net/bluetooth/amp.h
> >@@ -46,6 +46,7 @@ void amp_accept_phylink(struct hci_dev *hdev, struct amp_mgr *mgr,
> > struct hci_conn *hcon);
> >void amp_write_remote_assoc(struct hci_dev *hdev, u8 handle);
> >void amp_write_rem_assoc_continue(struct hci_dev *hdev, u8 handle);
> >+void amp_physical_cfm(struct hci_conn *bredr_hcon, struct hci_conn *hs_hcon);
> >void amp_create_logical_link(struct l2cap_chan *chan);
> >void amp_disconnect_logical_link(struct hci_chan *hchan);
> >void amp_destroy_logical_link(struct hci_chan *hchan, u8 reason);
> >diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> >index 24c61ef..18149c8 100644
> >--- a/include/net/bluetooth/l2cap.h
> >+++ b/include/net/bluetooth/l2cap.h
> >@@ -812,5 +812,6 @@ void l2cap_send_conn_req(struct l2cap_chan *chan);
> >void l2cap_move_start(struct l2cap_chan *chan);
> >void l2cap_logical_cfm(struct l2cap_chan *chan, struct hci_chan *hchan,
> > u8 status);
> >+void l2cap_physical_cfm(struct l2cap_chan *chan, int result);
> >
> >#endif /* __L2CAP_H */
> >diff --git a/net/bluetooth/amp.c b/net/bluetooth/amp.c
> >index 917e034..650bb8d 100644
> >--- a/net/bluetooth/amp.c
> >+++ b/net/bluetooth/amp.c
> >@@ -373,6 +373,30 @@ void amp_accept_phylink(struct hci_dev *hdev, struct amp_mgr *mgr,
> > hci_send_cmd(hdev, HCI_OP_ACCEPT_PHY_LINK, sizeof(cp), &cp);
> >}
> >
> >+void amp_physical_cfm(struct hci_conn *bredr_hcon, struct hci_conn *hs_hcon)
> >+{
> >+ struct hci_dev *bredr_hdev = hci_dev_hold(bredr_hcon->hdev);
> >+ struct amp_mgr *mgr = hs_hcon->amp_mgr;
> >+ struct l2cap_chan *bredr_chan;
> >+
> >+ BT_DBG("bredr_hcon %p hs_hcon %p mgr %p", bredr_hcon, hs_hcon, mgr);
> >+
> >+ if (!bredr_hdev || !mgr || !mgr->bredr_chan)
> >+ return;
> >+
> >+ bredr_chan = mgr->bredr_chan;
> >+
> >+ set_bit(FLAG_EFS_ENABLE, &bredr_chan->flags);
> >+ bredr_chan->ctrl_id = hs_hcon->remote_id;
> >+ bredr_chan->hs_hcon = hs_hcon;
> >+ bredr_chan->conn->mtu = hs_hcon->hdev->block_mtu;
> >+ bredr_chan->fcs = L2CAP_FCS_NONE;
>
> Sorry I missed this earlier: bredr_chan needs to be locked before
> changing it. I suggest passing the information to
> l2cap_physical_cfm and letting that function update the structure
> members while it holds the lock.

what about locking here and changing l2cap_physical_cfm to unlocked
__l2cap_physical_cfm ?

>
>
> >+
> >+ l2cap_physical_cfm(bredr_chan, 0);
> >+
> >+ hci_dev_put(bredr_hdev);
> >+}

Best regards
Andrei Emeltchenko

2012-11-02 00:31:34

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 2/2] Bluetooth: Rename ctrl_id to remote_amp_id

Hi Andrei,

> Since we have started to use local_amp_id for local AMP
> Controller Id it makes sense to rename ctrl_id to remote_amp_id
> since it represents remote AMP controller Id.
>
> Signed-off-by: Andrei Emeltchenko <[email protected]>
> ---
> include/net/bluetooth/l2cap.h | 3 +--
> net/bluetooth/a2mp.c | 4 ++--
> net/bluetooth/amp.c | 5 ++---
> net/bluetooth/l2cap_core.c | 2 +-
> 4 files changed, 6 insertions(+), 8 deletions(-)

Acked-by: Marcel Holtmann <[email protected]>

Regards

Marcel



2012-11-02 00:30:57

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 1/2] Bluetooth: Process Create Chan Request

Hi Andrei,

> Add processing L2CAP Create Chan Request. When channel is created
> save associated high speed link hs_hcon.
>
> Signed-off-by: Andrei Emeltchenko <[email protected]>
> ---
> net/bluetooth/l2cap_core.c | 63 ++++++++++++++++++++++++++++++--------------
> 1 file changed, 43 insertions(+), 20 deletions(-)

Acked-by: Marcel Holtmann <[email protected]>

Regards

Marcel



2012-11-01 17:51:19

by Mat Martineau

[permalink] [raw]
Subject: Re: [PATCHv1 11/12] Bluetooth: AMP: Use l2cap_physical_cfm in phylink complete evt


Andrei -

On Wed, 31 Oct 2012, Andrei Emeltchenko wrote:

> From: Andrei Emeltchenko <[email protected]>
>
> When receiving HCI Phylink Complete event run amp_physical_cfm
> which initialize BR/EDR L2CAP channel associated with High Speed
> link and run l2cap_physical_cfm which shall send L2CAP Create
> Chan Request.
>
> Signed-off-by: Andrei Emeltchenko <[email protected]>
> Acked-by: Marcel Holtmann <[email protected]>
> ---
> include/net/bluetooth/amp.h | 1 +
> include/net/bluetooth/l2cap.h | 1 +
> net/bluetooth/amp.c | 24 ++++++++++++++++++++++++
> net/bluetooth/hci_event.c | 15 ++-------------
> 4 files changed, 28 insertions(+), 13 deletions(-)
>
> diff --git a/include/net/bluetooth/amp.h b/include/net/bluetooth/amp.h
> index f1c0017..7ea3db7 100644
> --- a/include/net/bluetooth/amp.h
> +++ b/include/net/bluetooth/amp.h
> @@ -46,6 +46,7 @@ void amp_accept_phylink(struct hci_dev *hdev, struct amp_mgr *mgr,
> struct hci_conn *hcon);
> void amp_write_remote_assoc(struct hci_dev *hdev, u8 handle);
> void amp_write_rem_assoc_continue(struct hci_dev *hdev, u8 handle);
> +void amp_physical_cfm(struct hci_conn *bredr_hcon, struct hci_conn *hs_hcon);
> void amp_create_logical_link(struct l2cap_chan *chan);
> void amp_disconnect_logical_link(struct hci_chan *hchan);
> void amp_destroy_logical_link(struct hci_chan *hchan, u8 reason);
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index 24c61ef..18149c8 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -812,5 +812,6 @@ void l2cap_send_conn_req(struct l2cap_chan *chan);
> void l2cap_move_start(struct l2cap_chan *chan);
> void l2cap_logical_cfm(struct l2cap_chan *chan, struct hci_chan *hchan,
> u8 status);
> +void l2cap_physical_cfm(struct l2cap_chan *chan, int result);
>
> #endif /* __L2CAP_H */
> diff --git a/net/bluetooth/amp.c b/net/bluetooth/amp.c
> index 917e034..650bb8d 100644
> --- a/net/bluetooth/amp.c
> +++ b/net/bluetooth/amp.c
> @@ -373,6 +373,30 @@ void amp_accept_phylink(struct hci_dev *hdev, struct amp_mgr *mgr,
> hci_send_cmd(hdev, HCI_OP_ACCEPT_PHY_LINK, sizeof(cp), &cp);
> }
>
> +void amp_physical_cfm(struct hci_conn *bredr_hcon, struct hci_conn *hs_hcon)
> +{
> + struct hci_dev *bredr_hdev = hci_dev_hold(bredr_hcon->hdev);
> + struct amp_mgr *mgr = hs_hcon->amp_mgr;
> + struct l2cap_chan *bredr_chan;
> +
> + BT_DBG("bredr_hcon %p hs_hcon %p mgr %p", bredr_hcon, hs_hcon, mgr);
> +
> + if (!bredr_hdev || !mgr || !mgr->bredr_chan)
> + return;
> +
> + bredr_chan = mgr->bredr_chan;
> +
> + set_bit(FLAG_EFS_ENABLE, &bredr_chan->flags);
> + bredr_chan->ctrl_id = hs_hcon->remote_id;
> + bredr_chan->hs_hcon = hs_hcon;
> + bredr_chan->conn->mtu = hs_hcon->hdev->block_mtu;
> + bredr_chan->fcs = L2CAP_FCS_NONE;

Sorry I missed this earlier: bredr_chan needs to be locked before
changing it. I suggest passing the information to l2cap_physical_cfm
and letting that function update the structure members while it holds
the lock.


> +
> + l2cap_physical_cfm(bredr_chan, 0);
> +
> + hci_dev_put(bredr_hdev);
> +}
> +
> void amp_create_logical_link(struct l2cap_chan *chan)
> {
> struct hci_cp_create_accept_logical_link cp;
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 03d51a1..0b37f55 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -3697,20 +3697,9 @@ static void hci_phy_link_complete_evt(struct hci_dev *hdev,
> hci_conn_hold_device(hcon);
> hci_conn_add_sysfs(hcon);
>
> - hci_dev_unlock(hdev);
> -
> - if (hcon->out) {
> - struct hci_dev *bredr_hdev = hci_dev_hold(bredr_hcon->hdev);
> -
> - if (!bredr_hdev)
> - return;
> + amp_physical_cfm(bredr_hcon, hcon);
>
> - /* Placeholder - create chan req
> - l2cap_chan_create_cfm(bredr_hcon, hcon->remote_id);
> - */
> -
> - hci_dev_put(bredr_hdev);
> - }
> + hci_dev_unlock(hdev);
> }
>
> static void hci_loglink_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
> --
> 1.7.9.5
>
>

Regards,

--
Mat Martineau

Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation




2012-11-01 13:37:02

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCH 1/2] Bluetooth: Process Create Chan Request

From: Andrei Emeltchenko <[email protected]>

Add processing L2CAP Create Chan Request. When channel is created
save associated high speed link hs_hcon.

Signed-off-by: Andrei Emeltchenko <[email protected]>
---
net/bluetooth/l2cap_core.c | 63 ++++++++++++++++++++++++++++++--------------
1 file changed, 43 insertions(+), 20 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index bdffc4c..2f0e165 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -4237,7 +4237,9 @@ static int l2cap_create_channel_req(struct l2cap_conn *conn,
u16 cmd_len, void *data)
{
struct l2cap_create_chan_req *req = data;
+ struct l2cap_create_chan_rsp rsp;
struct l2cap_chan *chan;
+ struct hci_dev *hdev;
u16 psm, scid;

if (cmd_len != sizeof(*req))
@@ -4251,36 +4253,57 @@ static int l2cap_create_channel_req(struct l2cap_conn *conn,

BT_DBG("psm 0x%2.2x, scid 0x%4.4x, amp_id %d", psm, scid, req->amp_id);

- if (req->amp_id) {
- struct hci_dev *hdev;
-
- /* Validate AMP controller id */
- hdev = hci_dev_get(req->amp_id);
- if (!hdev || hdev->dev_type != HCI_AMP ||
- !test_bit(HCI_UP, &hdev->flags)) {
- struct l2cap_create_chan_rsp rsp;
+ /* For controller id 0 make BR/EDR connection */
+ if (req->amp_id == HCI_BREDR_ID) {
+ l2cap_connect(conn, cmd, data, L2CAP_CREATE_CHAN_RSP,
+ req->amp_id);
+ return 0;
+ }

- rsp.dcid = 0;
- rsp.scid = cpu_to_le16(scid);
- rsp.result = __constant_cpu_to_le16(L2CAP_CR_BAD_AMP);
- rsp.status = __constant_cpu_to_le16(L2CAP_CS_NO_INFO);
+ /* Validate AMP controller id */
+ hdev = hci_dev_get(req->amp_id);
+ if (!hdev)
+ goto error;

- l2cap_send_cmd(conn, cmd->ident, L2CAP_CREATE_CHAN_RSP,
- sizeof(rsp), &rsp);
+ if (hdev->dev_type != HCI_AMP || !test_bit(HCI_UP, &hdev->flags)) {
+ hci_dev_put(hdev);
+ goto error;
+ }

- if (hdev)
- hci_dev_put(hdev);
+ chan = l2cap_connect(conn, cmd, data, L2CAP_CREATE_CHAN_RSP,
+ req->amp_id);
+ if (chan) {
+ struct amp_mgr *mgr = conn->hcon->amp_mgr;
+ struct hci_conn *hs_hcon;

- return 0;
+ hs_hcon = hci_conn_hash_lookup_ba(hdev, AMP_LINK, conn->dst);
+ if (!hs_hcon) {
+ hci_dev_put(hdev);
+ return -EFAULT;
}

- hci_dev_put(hdev);
+ BT_DBG("mgr %p bredr_chan %p hs_hcon %p", mgr, chan, hs_hcon);
+
+ chan->local_amp_id = req->amp_id;
+ mgr->bredr_chan = chan;
+ chan->hs_hcon = hs_hcon;
+ conn->mtu = hdev->block_mtu;
}

- chan = l2cap_connect(conn, cmd, data, L2CAP_CREATE_CHAN_RSP,
- req->amp_id);
+ hci_dev_put(hdev);

return 0;
+
+error:
+ rsp.dcid = 0;
+ rsp.scid = cpu_to_le16(scid);
+ rsp.result = __constant_cpu_to_le16(L2CAP_CR_BAD_AMP);
+ rsp.status = __constant_cpu_to_le16(L2CAP_CS_NO_INFO);
+
+ l2cap_send_cmd(conn, cmd->ident, L2CAP_CREATE_CHAN_RSP,
+ sizeof(rsp), &rsp);
+
+ return -EFAULT;
}

static void l2cap_send_move_chan_req(struct l2cap_chan *chan, u8 dest_amp_id)
--
1.7.9.5


2012-11-01 13:37:03

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCH 2/2] Bluetooth: Rename ctrl_id to remote_amp_id

From: Andrei Emeltchenko <[email protected]>

Since we have started to use local_amp_id for local AMP
Controller Id it makes sense to rename ctrl_id to remote_amp_id
since it represents remote AMP controller Id.

Signed-off-by: Andrei Emeltchenko <[email protected]>
---
include/net/bluetooth/l2cap.h | 3 +--
net/bluetooth/a2mp.c | 4 ++--
net/bluetooth/amp.c | 5 ++---
net/bluetooth/l2cap_core.c | 2 +-
4 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 18149c8..d65db45 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -481,6 +481,7 @@ struct l2cap_chan {
unsigned long conn_state;
unsigned long flags;

+ __u8 remote_amp_id;
__u8 local_amp_id;
__u8 move_id;
__u8 move_state;
@@ -518,8 +519,6 @@ struct l2cap_chan {
__u32 remote_acc_lat;
__u32 remote_flush_to;

- __u8 ctrl_id;
-
struct delayed_work chan_timer;
struct delayed_work retrans_timer;
struct delayed_work monitor_timer;
diff --git a/net/bluetooth/a2mp.c b/net/bluetooth/a2mp.c
index d5136cf..2f67d5e 100644
--- a/net/bluetooth/a2mp.c
+++ b/net/bluetooth/a2mp.c
@@ -423,7 +423,7 @@ static int a2mp_getampassoc_rsp(struct amp_mgr *mgr, struct sk_buff *skb,

BT_DBG("Created hcon %p: loc:%d -> rem:%d", hcon, hdev->id, rsp->id);

- mgr->bredr_chan->ctrl_id = rsp->id;
+ mgr->bredr_chan->remote_amp_id = rsp->id;

amp_create_phylink(hdev, mgr, hcon);

@@ -939,7 +939,7 @@ void a2mp_send_create_phy_link_req(struct hci_dev *hdev, u8 status)
goto clean;

req->local_id = hdev->id;
- req->remote_id = bredr_chan->ctrl_id;
+ req->remote_id = bredr_chan->remote_amp_id;
memcpy(req->amp_assoc, loc_assoc->data, loc_assoc->len);

a2mp_send(mgr, A2MP_CREATEPHYSLINK_REQ, __next_ident(mgr), len, req);
diff --git a/net/bluetooth/amp.c b/net/bluetooth/amp.c
index 650bb8d..4b2fea6 100644
--- a/net/bluetooth/amp.c
+++ b/net/bluetooth/amp.c
@@ -387,7 +387,7 @@ void amp_physical_cfm(struct hci_conn *bredr_hcon, struct hci_conn *hs_hcon)
bredr_chan = mgr->bredr_chan;

set_bit(FLAG_EFS_ENABLE, &bredr_chan->flags);
- bredr_chan->ctrl_id = hs_hcon->remote_id;
+ bredr_chan->remote_amp_id = hs_hcon->remote_id;
bredr_chan->hs_hcon = hs_hcon;
bredr_chan->conn->mtu = hs_hcon->hdev->block_mtu;
bredr_chan->fcs = L2CAP_FCS_NONE;
@@ -412,8 +412,7 @@ void amp_create_logical_link(struct l2cap_chan *chan)
if (!hdev)
return;

- BT_DBG("chan %p ctrl_id %d dst %pMR", chan, chan->ctrl_id,
- chan->conn->dst);
+ BT_DBG("chan %p dst %pMR", chan, chan->conn->dst);

hcon = hci_conn_hash_lookup_ba(hdev, AMP_LINK, chan->conn->dst);
if (!hcon)
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 2f0e165..a1faaab 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -4615,7 +4615,7 @@ static void l2cap_do_move_cancel(struct l2cap_chan *chan, int result)
void l2cap_physical_cfm(struct l2cap_chan *chan, int result)
{
u8 local_amp_id = chan->local_amp_id;
- u8 remote_amp_id = chan->ctrl_id;
+ u8 remote_amp_id = chan->remote_amp_id;

BT_DBG("chan %p, result %d, local_amp_id %d, remote_amp_id %d",
chan, result, local_amp_id, remote_amp_id);
--
1.7.9.5


2012-11-01 08:55:06

by Andrei Emeltchenko

[permalink] [raw]
Subject: Re: [PATCHv1 12/12] Bluetooth: Process Create Chan Request

Hi Marcel,

On Wed, Oct 31, 2012 at 09:24:02AM -0700, Marcel Holtmann wrote:
> Hi Andrei,
>
> > Add processing L2CAP Create Chan Request. When channel is created
> > save associated high speed link hs_hcon.
> >
> > Signed-off-by: Andrei Emeltchenko <[email protected]>
> > ---
> > net/bluetooth/l2cap_core.c | 63 ++++++++++++++++++++++++++++++--------------
> > 1 file changed, 43 insertions(+), 20 deletions(-)
> >
> > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> > index bdffc4c..783ffae 100644
> > --- a/net/bluetooth/l2cap_core.c
> > +++ b/net/bluetooth/l2cap_core.c
> > @@ -4237,7 +4237,9 @@ static int l2cap_create_channel_req(struct l2cap_conn *conn,
> > u16 cmd_len, void *data)
> > {
> > struct l2cap_create_chan_req *req = data;
> > + struct l2cap_create_chan_rsp rsp;
> > struct l2cap_chan *chan;
> > + struct hci_dev *hdev = NULL;
>
> why is this one set to NULL. Can we avoid that please.

Sorry, forgot to clean this up from the previous patch. With the new patch
we do not need to set this NULL.

Best regards
Andrei Emeltchenko