Return-Path: Date: Wed, 23 May 2012 16:16:37 -0700 (PDT) From: Mat Martineau To: Andrei Emeltchenko cc: linux-bluetooth@vger.kernel.org Subject: Re: [PATCHv2 02/19] Bluetooth: Add ready to L2CAP channel ops In-Reply-To: <1337787445-23235-3-git-send-email-Andrei.Emeltchenko.news@gmail.com> Message-ID: References: <1337351150-20526-1-git-send-email-Andrei.Emeltchenko.news@gmail.com> <1337787445-23235-1-git-send-email-Andrei.Emeltchenko.news@gmail.com> <1337787445-23235-3-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, 23 May 2012, Andrei Emeltchenko wrote: > From: Andrei Emeltchenko > > Addings ops->ready() helps to separate socket and socketless > L2CAP channels. > > Signed-off-by: Andrei Emeltchenko > --- > include/net/bluetooth/l2cap.h | 1 + > net/bluetooth/l2cap_core.c | 42 +++++++++++------------------------------ > net/bluetooth/l2cap_sock.c | 28 +++++++++++++++++++++++++++ > 3 files changed, 40 insertions(+), 31 deletions(-) > > diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h > index a017249..83a67e8 100644 > --- a/include/net/bluetooth/l2cap.h > +++ b/include/net/bluetooth/l2cap.h > @@ -530,6 +530,7 @@ struct l2cap_ops { > struct sk_buff *(*alloc_skb) (struct l2cap_chan *chan, > unsigned long len, int nb); > void (*unlink) (struct l2cap_chan *chan, int err); > + void (*ready) (struct l2cap_chan *chan); > }; > > struct l2cap_conn { > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c > index c81b828..d5ea8af 100644 > --- a/net/bluetooth/l2cap_core.c > +++ b/net/bluetooth/l2cap_core.c > @@ -407,7 +407,7 @@ struct l2cap_chan *l2cap_chan_create(void) > > atomic_set(&chan->refcnt, 1); > > - /* This flag is cleared in l2cap_chan_ready() */ > + /* This flag is cleared in l2cap chan ops->ready() */ > set_bit(CONF_NOT_COMPLETE, &chan->conf_state); > > BT_DBG("chan %p", chan); > @@ -953,36 +953,13 @@ static void l2cap_send_conn_req(struct l2cap_chan *chan) > l2cap_send_cmd(conn, chan->ident, L2CAP_CONN_REQ, sizeof(req), &req); > } > > -static void l2cap_chan_ready(struct l2cap_chan *chan) > -{ > - struct sock *sk = chan->sk; > - struct sock *parent; > - > - lock_sock(sk); > - > - parent = bt_sk(sk)->parent; > - > - BT_DBG("sk %p, parent %p", sk, parent); > - > - /* This clears all conf flags, including CONF_NOT_COMPLETE */ > - chan->conf_state = 0; > - __clear_chan_timer(chan); > - > - __l2cap_state_change(chan, BT_CONNECTED); > - sk->sk_state_change(sk); > - > - if (parent) > - parent->sk_data_ready(parent, 0); > - > - release_sock(sk); > -} > - > static void l2cap_do_start(struct l2cap_chan *chan) > { > struct l2cap_conn *conn = chan->conn; > > if (conn->hcon->type == LE_LINK) { > - l2cap_chan_ready(chan); > + if (chan->ops->ready) > + chan->ops->ready(chan); > return; > } > > @@ -1242,8 +1219,8 @@ static void l2cap_conn_ready(struct l2cap_conn *conn) > > if (conn->hcon->type == LE_LINK) { > if (smp_conn_security(conn, chan->sec_level)) > - l2cap_chan_ready(chan); > - > + if (chan->ops->ready) > + chan->ops->ready(chan); > } else if (chan->chan_type != L2CAP_CHAN_CONN_ORIENTED) { > struct sock *sk = chan->sk; > __clear_chan_timer(chan); > @@ -3639,7 +3616,8 @@ static inline int l2cap_config_req(struct l2cap_conn *conn, struct l2cap_cmd_hdr > if (err < 0) > l2cap_send_disconn_req(chan->conn, chan, -err); > else > - l2cap_chan_ready(chan); > + if (chan->ops->ready) > + chan->ops->ready(chan); > > goto unlock; > } > @@ -3770,7 +3748,8 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hdr > if (err < 0) > l2cap_send_disconn_req(chan->conn, chan, -err); > else > - l2cap_chan_ready(chan); > + if (chan->ops->ready) > + chan->ops->ready(chan); > } > > done: > @@ -5420,7 +5399,8 @@ int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt) > if (chan->scid == L2CAP_CID_LE_DATA) { > if (!status && encrypt) { > chan->sec_level = hcon->sec_level; > - l2cap_chan_ready(chan); > + if (chan->ops->ready) > + chan->ops->ready(chan); > } > > l2cap_chan_unlock(chan); > diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c > index 6749d11..2e078c4 100644 > --- a/net/bluetooth/l2cap_sock.c > +++ b/net/bluetooth/l2cap_sock.c > @@ -978,6 +978,33 @@ static void l2cap_sock_unlink_cb(struct l2cap_chan *chan, int err) > release_sock(sk); > } > > +static void l2cap_sock_chan_ready_cb(struct l2cap_chan *chan) > +{ > + struct sock *sk = chan->sk; > + struct sock *parent; > + > + lock_sock(sk); > + > + parent = bt_sk(sk)->parent; > + > + BT_DBG("sk %p, parent %p", sk, parent); > + > + __clear_chan_timer(chan); > + > + /* This clears all conf flags, including CONF_NOT_COMPLETE */ > + chan->conf_state = 0; > + > + chan->state = BT_CONNECTED; Similar to patch 1, would there be some benefit to leaving the chan code (timer clearing, state and conf_state modification) in l2cap_core? These things need to happen even if chan->ops->ready is NULL. > + sk->sk_state = BT_CONNECTED; > + > + sk->sk_state_change(sk); > + > + if (parent) > + parent->sk_data_ready(parent, 0); > + > + release_sock(sk); > +} > + > static struct l2cap_ops l2cap_chan_ops = { > .name = "L2CAP Socket Interface", > .new_connection = l2cap_sock_new_connection_cb, > @@ -986,6 +1013,7 @@ static struct l2cap_ops l2cap_chan_ops = { > .state_change = l2cap_sock_state_change_cb, > .alloc_skb = l2cap_sock_alloc_skb_cb, > .unlink = l2cap_sock_unlink_cb, > + .ready = l2cap_sock_chan_ready_cb, > }; > > static void l2cap_sock_destruct(struct sock *sk) > -- > 1.7.9.5 -- Mat Martineau Employee of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum