Return-Path: Date: Thu, 13 Sep 2012 09:26:27 -0700 (PDT) From: Mat Martineau To: Andrei Emeltchenko cc: linux-bluetooth@vger.kernel.org, gustavo@padovan.org Subject: Re: [PATCHv4 11/17] Bluetooth: Choose connection based on capabilities In-Reply-To: <1347437192-24694-12-git-send-email-Andrei.Emeltchenko.news@gmail.com> Message-ID: References: <1347437192-24694-1-git-send-email-Andrei.Emeltchenko.news@gmail.com> <1347437192-24694-12-git-send-email-Andrei.Emeltchenko.news@gmail.com> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Sender: linux-bluetooth-owner@vger.kernel.org List-ID: Andrei - On Wed, 12 Sep 2012, Andrei Emeltchenko wrote: > From: Andrei Emeltchenko > > Choose which L2CAP connection to establish by checking support > for HS and remote side supported features. > > Signed-off-by: Andrei Emeltchenko > --- > include/net/bluetooth/a2mp.h | 2 ++ > include/net/bluetooth/l2cap.h | 1 + > net/bluetooth/a2mp.c | 34 +++++++++++++++++++++++++++++----- > net/bluetooth/l2cap_core.c | 33 ++++++++++++++++++++++++++++----- > 4 files changed, 60 insertions(+), 10 deletions(-) > > diff --git a/include/net/bluetooth/a2mp.h b/include/net/bluetooth/a2mp.h > index 93967f1..6e88a80 100644 > --- a/include/net/bluetooth/a2mp.h > +++ b/include/net/bluetooth/a2mp.h > @@ -23,6 +23,7 @@ struct amp_mgr { > struct list_head list; > struct l2cap_conn *l2cap_conn; > struct l2cap_chan *a2mp_chan; > + struct l2cap_chan *bredr_chan; > struct kref kref; > __u8 ident; > __u8 handle; > @@ -135,6 +136,7 @@ struct l2cap_chan *a2mp_channel_create(struct l2cap_conn *conn, > struct sk_buff *skb); > struct amp_mgr *amp_mgr_lookup_by_state(u8 state); > void a2mp_send(struct amp_mgr *mgr, u8 code, u8 ident, u16 len, void *data); > +void a2mp_discover_amp(struct l2cap_chan *chan); > void a2mp_send_getinfo_rsp(struct hci_dev *hdev); > void a2mp_send_getampassoc_rsp(struct hci_dev *hdev, u8 status); > > diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h > index 7ed8e35..aba830f 100644 > --- a/include/net/bluetooth/l2cap.h > +++ b/include/net/bluetooth/l2cap.h > @@ -767,6 +767,7 @@ int l2cap_chan_check_security(struct l2cap_chan *chan); > void l2cap_chan_set_defaults(struct l2cap_chan *chan); > int l2cap_ertm_init(struct l2cap_chan *chan); > 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); > > #endif /* __L2CAP_H */ > diff --git a/net/bluetooth/a2mp.c b/net/bluetooth/a2mp.c > index 42bce4a..c550589 100644 > --- a/net/bluetooth/a2mp.c > +++ b/net/bluetooth/a2mp.c > @@ -629,7 +629,7 @@ static struct l2cap_ops a2mp_chan_ops = { > .ready = l2cap_chan_no_ready, > }; > > -static struct l2cap_chan *a2mp_chan_open(struct l2cap_conn *conn) > +static struct l2cap_chan *a2mp_chan_open(struct l2cap_conn *conn, bool locked) > { > struct l2cap_chan *chan; > int err; > @@ -664,7 +664,10 @@ static struct l2cap_chan *a2mp_chan_open(struct l2cap_conn *conn) > > chan->conf_state = 0; > > - l2cap_chan_add(conn, chan); > + if (locked) > + __l2cap_chan_add(conn, chan); > + else > + l2cap_chan_add(conn, chan); > > chan->remote_mps = chan->omtu; > chan->mps = chan->omtu; > @@ -703,7 +706,7 @@ int amp_mgr_put(struct amp_mgr *mgr) > return kref_put(&mgr->kref, &_mgr_destroy); > } > > -static struct amp_mgr *amp_mgr_create(struct l2cap_conn *conn) > +static struct amp_mgr *amp_mgr_create(struct l2cap_conn *conn, bool locked) > { > struct amp_mgr *mgr; > struct l2cap_chan *chan; > @@ -716,7 +719,7 @@ static struct amp_mgr *amp_mgr_create(struct l2cap_conn *conn) > > mgr->l2cap_conn = conn; > > - chan = a2mp_chan_open(conn); > + chan = a2mp_chan_open(conn, locked); > if (!chan) { > kfree(mgr); > return NULL; > @@ -745,7 +748,7 @@ struct l2cap_chan *a2mp_channel_create(struct l2cap_conn *conn, > { > struct amp_mgr *mgr; > > - mgr = amp_mgr_create(conn); > + mgr = amp_mgr_create(conn, false); > if (!mgr) { > BT_ERR("Could not create AMP manager"); > return NULL; > @@ -833,3 +836,24 @@ void a2mp_send_getampassoc_rsp(struct hci_dev *hdev, u8 status) > amp_mgr_put(mgr); > kfree(rsp); > } > + > +void a2mp_discover_amp(struct l2cap_chan *chan) > +{ > + struct l2cap_conn *conn = chan->conn; > + struct amp_mgr *mgr = conn->hcon->amp_mgr; > + struct a2mp_discov_req req; > + > + BT_DBG("chan %p conn %p mgr %p", chan, conn, mgr); > + > + if (!mgr) { > + mgr = amp_mgr_create(conn, true); > + if (!mgr) > + return; > + } > + > + mgr->bredr_chan = chan; > + > + req.mtu = cpu_to_le16(L2CAP_A2MP_DEFAULT_MTU); > + req.ext_feat = 0; > + a2mp_send(mgr, A2MP_DISCOVER_REQ, 1, sizeof(req), &req); > +} > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c > index 9732f03..cfe047f 100644 > --- a/net/bluetooth/l2cap_core.c > +++ b/net/bluetooth/l2cap_core.c > @@ -455,7 +455,7 @@ void l2cap_chan_set_defaults(struct l2cap_chan *chan) > set_bit(FLAG_FORCE_ACTIVE, &chan->flags); > } > > -static void __l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan) > +void __l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan) > { > BT_DBG("conn %p, psm 0x%2.2x, dcid 0x%4.4x", conn, > __le16_to_cpu(chan->psm), chan->dcid); > @@ -946,6 +946,18 @@ static inline int __l2cap_no_conn_pending(struct l2cap_chan *chan) > return !test_bit(CONF_CONNECT_PEND, &chan->conf_state); > } > > +static bool __amp_capable(struct l2cap_chan *chan) > +{ > + struct l2cap_conn *conn = chan->conn; > + > + if (enable_hs && > + chan->chan_policy == BT_CHANNEL_POLICY_AMP_PREFERRED && > + conn->fixed_chan_mask & L2CAP_FC_A2MP) > + return true; > + else > + return false; > +} > + > static void l2cap_send_conn_req(struct l2cap_chan *chan) > { > struct l2cap_conn *conn = chan->conn; > @@ -972,6 +984,16 @@ static void l2cap_chan_ready(struct l2cap_chan *chan) > chan->ops->ready(chan); > } > > +static void l2cap_choose_conn(struct l2cap_chan *chan) I find this function name a little confusing - "choose" sounds like it is just making a decision. This function also takes the action of starting physical link setup or sending a connection request, which is more than just "choosing". l2cap_start_connection, maybe? > +{ > + if (__amp_capable(chan)) { > + BT_DBG("chan %p AMP capable: discover AMPs", chan); > + a2mp_discover_amp(chan); > + } else { > + l2cap_send_conn_req(chan); > + } > +} > + > static void l2cap_do_start(struct l2cap_chan *chan) > { > struct l2cap_conn *conn = chan->conn; > @@ -986,8 +1008,9 @@ static void l2cap_do_start(struct l2cap_chan *chan) > return; > > if (l2cap_chan_check_security(chan) && > - __l2cap_no_conn_pending(chan)) > - l2cap_send_conn_req(chan); > + __l2cap_no_conn_pending(chan)) { > + l2cap_choose_conn(chan); > + } > } else { > struct l2cap_info_req req; > req.type = __constant_cpu_to_le16(L2CAP_IT_FEAT_MASK); > @@ -1082,7 +1105,7 @@ static void l2cap_conn_start(struct l2cap_conn *conn) > continue; > } > > - l2cap_send_conn_req(chan); > + l2cap_choose_conn(chan); > > } else if (chan->state == BT_CONNECT2) { > struct l2cap_conn_rsp rsp; > @@ -5454,7 +5477,7 @@ int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt) > > if (chan->state == BT_CONNECT) { > if (!status) { > - l2cap_send_conn_req(chan); > + l2cap_choose_conn(chan); > } else { > __set_chan_timer(chan, L2CAP_DISC_TIMEOUT); > } > -- > 1.7.9.5 -- Mat Martineau The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation