From: Mat Martineau <mathewm@codeaurora.org>
To: Andrei Emeltchenko <Andrei.Emeltchenko.news@gmail.com>
Cc: linux-bluetooth@vger.kernel.org, gustavo@padovan.org,
pkrystad@codeaurora.org
Subject: Re: [RFCv0 14/21] Bluetooth: Handle physical link completion
Date: Wed, 15 Aug 2012 09:56:04 -0700 (PDT) [thread overview]
Message-ID: <alpine.DEB.2.02.1208150951470.30981@mathewm-linux> (raw)
In-Reply-To: <20120815135105.GA3555@aemeltch-MOBL1>
Andrei -
On Wed, 15 Aug 2012, Andrei Emeltchenko wrote:
> Hi Mat,
>
> On Wed, Jul 25, 2012 at 04:51:06PM -0700, Mat Martineau wrote:
>> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
>> ---
>> net/bluetooth/l2cap_core.c | 147 +++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 147 insertions(+)
>>
>> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
>> index d981d87..7e253ce 100644
>> --- a/net/bluetooth/l2cap_core.c
>> +++ b/net/bluetooth/l2cap_core.c
>> @@ -975,6 +975,19 @@ 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_send_create_chan_req(struct l2cap_chan *chan, u8 chan_id)
>> +{
>> + struct l2cap_create_chan_req req;
>> + req.scid = cpu_to_le16(chan->scid);
>> + req.psm = chan->psm;
>> + req.amp_id = chan_id;
>> +
>> + chan->ident = l2cap_get_ident(chan->conn);
>> +
>> + l2cap_send_cmd(chan->conn, chan->ident, L2CAP_CREATE_CHAN_REQ,
>> + sizeof(req), &req);
>> +}
>> +
>> static void l2cap_move_setup(struct l2cap_chan *chan)
>> {
>> struct sk_buff *skb;
>> @@ -4119,6 +4132,24 @@ static int l2cap_create_channel_req(struct l2cap_conn *conn,
>> return 0;
>> }
>>
>> +static void l2cap_send_move_chan_req(struct l2cap_chan *chan, u8 dest_amp_id)
>> +{
>> + struct l2cap_move_chan_req req;
>> + u8 ident;
>> +
>> + BT_DBG("chan %p, dest_amp_id %d", chan, dest_amp_id);
>> +
>> + ident = l2cap_get_ident(chan->conn);
>> + if (chan)
>
> I think that the check is not needed otherwise you crash at referencing
> chan->scid below.
Or at the line above. This check was added before everything had been
moved from l2cap_pi to l2cap_chan, and is no longer relevant.
>
>> + chan->ident = ident;
>> +
>> + req.icid = cpu_to_le16(chan->scid);
>> + req.dest_amp_id = dest_amp_id;
>> +
>> + l2cap_send_cmd(chan->conn, ident, L2CAP_MOVE_CHAN_REQ, sizeof(req),
>> + &req);
>> +}
>> +
>> static void l2cap_send_move_chan_rsp(struct l2cap_conn *conn, u8 ident,
>> u16 icid, u16 result)
>> {
>> @@ -4284,6 +4315,122 @@ static void l2cap_logical_cfm(struct l2cap_chan *chan, struct hci_chan *hchan,
>> }
>> }
>>
>> +void l2cap_physical_cfm(struct l2cap_chan *chan, int result, u8 local_id,
>> + u8 remote_id)
>> +{
>> + BT_DBG("chan %p, result %d, local_id %d, remote_id %d", chan, result,
>> + local_id, remote_id);
>> +
>> + l2cap_chan_lock(chan);
>> +
>> + if (chan->state == BT_DISCONN || chan->state == BT_CLOSED) {
>> + l2cap_chan_unlock(chan);
>> + return;
>> + }
>> +
>> + if (chan->state != BT_CONNECTED) {
>> + if (!test_bit(CONF_CONNECT_PEND, &chan->conf_state)) {
>> + struct l2cap_conn_rsp rsp;
>> + char buf[128];
>> + rsp.scid = cpu_to_le16(chan->dcid);
>> + rsp.dcid = cpu_to_le16(chan->scid);
>> +
>> + /* Incoming channel on AMP */
>> + if (result == L2CAP_CR_SUCCESS) {
>> + /* Send successful response */
>> + rsp.result = cpu_to_le16(L2CAP_CR_SUCCESS);
>> + rsp.status = cpu_to_le16(L2CAP_CS_NO_INFO);
>> + } else {
>> + /* Send negative response */
>> + rsp.result = cpu_to_le16(L2CAP_CR_NO_MEM);
>> + rsp.status = cpu_to_le16(L2CAP_CS_NO_INFO);
>> + }
>> +
>> + l2cap_send_cmd(chan->conn, chan->ident,
>> + L2CAP_CREATE_CHAN_RSP,
>> + sizeof(rsp), &rsp);
>> +
>> + if (result == L2CAP_CR_SUCCESS) {
>> + __l2cap_state_change(chan, BT_CONFIG);
>> + set_bit(CONF_REQ_SENT, &chan->conf_state);
>> + l2cap_send_cmd(chan->conn,
>> + l2cap_get_ident(chan->conn),
>> + L2CAP_CONF_REQ,
>> + l2cap_build_conf_req(chan, buf),
>> + buf);
>> + chan->num_conf_req++;
>> + }
>> + } else {
>> + /* Outgoing channel on AMP */
>> + if (result == L2CAP_CR_SUCCESS) {
>> + chan->chan_id = local_id;
>> + l2cap_send_create_chan_req(chan, remote_id);
>> + } else {
>> + /* Revert to BR/EDR connect */
>> + l2cap_send_conn_req(chan);
>> + }
>> + }
>> + } else if (result == L2CAP_MR_SUCCESS &&
>> + chan->move_role == L2CAP_MOVE_ROLE_INITIATOR) {
>> + l2cap_move_setup(chan);
>> + chan->move_id = local_id;
>> + chan->move_state = L2CAP_MOVE_WAIT_RSP;
>> +
>> + l2cap_send_move_chan_req(chan, remote_id);
>> + __set_chan_timer(chan, L2CAP_MOVE_TIMEOUT);
>> + } else if (result == L2CAP_MR_SUCCESS &&
>> + chan->move_role == L2CAP_MOVE_ROLE_RESPONDER) {
>> + struct hci_chan *hchan = NULL;
>> +
>> + /* Placeholder - get hci_chan for logical link */
>> +
>> + if (hchan) {
>> + if (hchan->state == BT_CONNECTED) {
>> + /* Logical link is ready to go */
>> + chan->hs_hcon = hchan->conn;
>> + chan->hs_hcon->l2cap_data = chan->conn;
>> + chan->move_state = L2CAP_MOVE_WAIT_CONFIRM;
>> + l2cap_send_move_chan_rsp(chan->conn,
>> + chan->move_cmd_ident,
>> + chan->dcid,
>> + L2CAP_MR_SUCCESS);
>> +
>> + l2cap_logical_cfm(chan, hchan,
>> + L2CAP_MR_SUCCESS);
>> + } else {
>> + /* Wait for logical link to be ready */
>> + chan->move_state = L2CAP_MOVE_WAIT_LOGICAL_CFM;
>> + }
>> + } else {
>> + /* Logical link not available */
>> + l2cap_send_move_chan_rsp(chan->conn,
>> + chan->move_cmd_ident,
>> + chan->dcid,
>> + L2CAP_MR_NOT_ALLOWED);
>> + }
>> + } else {
>> + if (chan->move_role == L2CAP_MOVE_ROLE_RESPONDER) {
>> + u8 rsp_result;
>> + if (result == -EINVAL)
>> + rsp_result = L2CAP_MR_BAD_ID;
>> + else
>> + rsp_result = L2CAP_MR_NOT_ALLOWED;
>> +
>> + l2cap_send_move_chan_rsp(chan->conn,
>> + chan->move_cmd_ident,
>> + chan->dcid, rsp_result);
>> + }
>> +
>> + chan->move_role = L2CAP_MOVE_ROLE_NONE;
>> + chan->move_state = L2CAP_MOVE_STABLE;
>> +
>> + /* Restart data transmission */
>> + l2cap_ertm_send(chan);
>> + }
>> +
>> + l2cap_chan_unlock(chan);
>> +}
>
> I feel that this function is too big. Maybe we could split channel move
> and channel create operations?
Sure, the "if (chan->state != BT_CONNECTED)" clause would be clearer
in a channel create function, and the other clauses can go in a
channel move function.
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
next prev parent reply other threads:[~2012-08-15 16:56 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-07-25 23:50 [RFCv0 00/21] L2CAP signaling for AMP channel create/move Mat Martineau
2012-07-25 23:50 ` [RFCv0 01/21] Bluetooth: Add new l2cap_chan struct members for high speed channels Mat Martineau
2012-07-25 23:50 ` [RFCv0 02/21] Bluetooth: Factor out common L2CAP connection code Mat Martineau
2012-07-25 23:50 ` [RFCv0 03/21] Bluetooth: Add L2CAP create channel request handling Mat Martineau
2012-07-26 13:13 ` Andrei Emeltchenko
2012-07-26 15:04 ` Mat Martineau
2012-07-26 19:43 ` Andrei Emeltchenko
2012-07-25 23:50 ` [RFCv0 04/21] Bluetooth: Process create response and connect response identically Mat Martineau
2012-08-14 10:12 ` Andrei Emeltchenko
2012-08-14 17:25 ` Mat Martineau
2012-07-25 23:50 ` [RFCv0 05/21] Bluetooth: Lookup channel id for channel move Mat Martineau
2012-07-26 13:30 ` Andrei Emeltchenko
2012-07-26 15:07 ` Mat Martineau
2012-07-25 23:50 ` [RFCv0 06/21] Bluetooth: Channel move request handling Mat Martineau
2012-07-26 13:59 ` Andrei Emeltchenko
2012-07-27 16:54 ` Mat Martineau
2012-08-06 8:44 ` Andrei Emeltchenko
2012-08-06 16:33 ` Mat Martineau
2012-08-07 8:31 ` Andrei Emeltchenko
2012-07-25 23:50 ` [RFCv0 07/21] Bluetooth: Add new ERTM receive states for channel move Mat Martineau
2012-07-25 23:51 ` [RFCv0 08/21] Bluetooth: Add move channel confirm handling Mat Martineau
2012-07-26 14:28 ` Andrei Emeltchenko
2012-07-26 15:16 ` Mat Martineau
2012-07-25 23:51 ` [RFCv0 09/21] Bluetooth: Move channel response Mat Martineau
2012-07-25 23:51 ` [RFCv0 10/21] Bluetooth: Add logical link confirm Mat Martineau
2012-07-26 14:41 ` Andrei Emeltchenko
2012-07-26 15:24 ` Mat Martineau
2012-07-25 23:51 ` [RFCv0 11/21] Bluetooth: Add move confirm response handling Mat Martineau
2012-07-26 14:45 ` Andrei Emeltchenko
2012-07-26 15:33 ` Mat Martineau
2012-07-25 23:51 ` [RFCv0 12/21] Bluetooth: Add state to hci_chan Mat Martineau
2012-07-26 19:54 ` Andrei Emeltchenko
2012-07-25 23:51 ` [RFCv0 13/21] Bluetooth: Indentation fixes Mat Martineau
2012-07-25 23:51 ` [RFCv0 14/21] Bluetooth: Handle physical link completion Mat Martineau
2012-08-15 13:51 ` Andrei Emeltchenko
2012-08-15 16:56 ` Mat Martineau [this message]
2012-08-16 7:23 ` Andrei Emeltchenko
2012-07-25 23:51 ` [RFCv0 15/21] Bluetooth: Enable data sends on AMP controller Mat Martineau
2012-07-25 23:51 ` [RFCv0 16/21] Bluetooth: Do not send data during channel move Mat Martineau
2012-07-25 23:51 ` [RFCv0 17/21] Bluetooth: Configure appropriate timeouts for AMP controllers Mat Martineau
2012-07-25 23:51 ` [RFCv0 18/21] Bluetooth: Ignore BR/EDR packet size constraints when fragmenting for AMP Mat Martineau
2012-07-25 23:51 ` [RFCv0 19/21] Bluetooth: Tie AMP link setup to connect/disconnect Mat Martineau
2012-07-25 23:51 ` [RFCv0 20/21] Bluetooth: Do not process ERTM reject during move Mat Martineau
2012-07-25 23:51 ` [RFCv0 21/21] Bluetooth: Start channel move when socket option is changed Mat Martineau
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=alpine.DEB.2.02.1208150951470.30981@mathewm-linux \
--to=mathewm@codeaurora.org \
--cc=Andrei.Emeltchenko.news@gmail.com \
--cc=gustavo@padovan.org \
--cc=linux-bluetooth@vger.kernel.org \
--cc=pkrystad@codeaurora.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox