[RFCv0 00/21] L2CAP signaling for AMP channel create/move

[RFCv0 00/21] L2CAP signaling for AMP channel create/move

[Mat Martineau]

Here are the changes that process commands on the L2CAP signaling
channel for setting up AMP channels.  There's still a lot of
integration to do as other AMP functionality is implemented.  I've
marked places that require this integration with "Placeholder"
comments (look for that string).

Many of the commit messages are still too short, I will expand on
those before changing this series from RFC to PATCH.  I didn't want to
delay Andrei's AMP work any further while I worked on commit messages.

Mat Martineau (21):
  Bluetooth: Add new l2cap_chan struct members for high speed channels
  Bluetooth: Factor out common L2CAP connection code
  Bluetooth: Add L2CAP create channel request handling.
  Bluetooth: Process create response and connect response identically
  Bluetooth: Lookup channel id for channel move
  Bluetooth: Channel move request handling
  Bluetooth: Add new ERTM receive states for channel move
  Bluetooth: Add move channel confirm handling
  Bluetooth: Move channel response
  Bluetooth: Add logical link confirm
  Bluetooth: Add move confirm response handling
  Bluetooth: Add state to hci_chan
  Bluetooth: Indentation fixes
  Bluetooth: Handle physical link completion
  Bluetooth: Enable data sends on AMP controller
  Bluetooth: Do not send data during channel move
  Bluetooth: Configure appropriate timeouts for AMP controllers
  Bluetooth: Ignore BR/EDR packet size constraints when fragmenting for
    AMP
  Bluetooth: Tie AMP link setup to connect/disconnect
  Bluetooth: Do not process ERTM reject during move
  Bluetooth: Start channel move when socket option is changed

 include/net/bluetooth/hci.h      |   1 +
 include/net/bluetooth/hci_core.h |   7 +-
 include/net/bluetooth/l2cap.h    |  34 ++
 net/bluetooth/hci_conn.c         |   1 +
 net/bluetooth/l2cap_core.c       | 999 +++++++++++++++++++++++++++++++++++++--
 net/bluetooth/l2cap_sock.c       |   5 +
 6 files changed, 995 insertions(+), 52 deletions(-)

-- 
1.7.11.2

--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

[RFCv0 01/21] Bluetooth: Add new l2cap_chan struct members for high speed channels

[Mat Martineau]

An L2CAP channel using high speed continues to be associated with a
BR/EDR l2cap_conn, while also tracking an additional hci_conn
(representing a physical link on a high speed controller) and hci_chan
(representing a logical link).  There may only be one physical link
between two high speed controllers.  Each physical link may contain
several logical links, with each logical link representing a channel
with specific quality of service.

During a channel move, the destination channel id, current move state,
and role (initiator vs. responder) are tracked and used by the channel
move state machine.  The ident value associated with a move request
must also be stored in order to use it in later move responses.

The active channel is stored in chan_id.

Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
 include/net/bluetooth/l2cap.h | 30 ++++++++++++++++++++++++++++++
 net/bluetooth/l2cap_core.c    |  6 ++++++
 2 files changed, 36 insertions(+)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index d206296..c585def 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -433,6 +433,8 @@ struct l2cap_chan {
 	struct sock *sk;
 
 	struct l2cap_conn	*conn;
+	struct hci_conn		*hs_hcon;
+	struct hci_chan		*hs_hchan;
 
 	__u8		state;
 
@@ -477,6 +479,12 @@ struct l2cap_chan {
 	unsigned long	conn_state;
 	unsigned long	flags;
 
+	__u8		chan_id;
+	__u8		move_id;
+	__u8		move_state;
+	__u8		move_role;
+	__u8		move_cmd_ident;
+
 	__u16		next_tx_seq;
 	__u16		expected_ack_seq;
 	__u16		expected_tx_seq;
@@ -641,6 +649,9 @@ enum {
 enum {
 	L2CAP_RX_STATE_RECV,
 	L2CAP_RX_STATE_SREJ_SENT,
+	L2CAP_RX_STATE_MOVE,
+	L2CAP_RX_STATE_WAIT_P,
+	L2CAP_RX_STATE_WAIT_F,
 };
 
 enum {
@@ -671,6 +682,25 @@ enum {
 	L2CAP_EV_RECV_FRAME,
 };
 
+enum {
+	L2CAP_MOVE_ROLE_NONE,
+	L2CAP_MOVE_ROLE_INITIATOR,
+	L2CAP_MOVE_ROLE_RESPONDER,
+};
+
+enum {
+	L2CAP_MOVE_STABLE,
+	L2CAP_MOVE_WAIT_REQ,
+	L2CAP_MOVE_WAIT_RSP,
+	L2CAP_MOVE_WAIT_RSP_SUCCESS,
+	L2CAP_MOVE_WAIT_CONFIRM,
+	L2CAP_MOVE_WAIT_CONFIRM_RSP,
+	L2CAP_MOVE_WAIT_LOGICAL_COMP,
+	L2CAP_MOVE_WAIT_LOGICAL_CFM,
+	L2CAP_MOVE_WAIT_LOCAL_BUSY,
+	L2CAP_MOVE_WAIT_PREPARE,
+};
+
 void l2cap_chan_hold(struct l2cap_chan *c);
 void l2cap_chan_put(struct l2cap_chan *c);
 
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 9f8b29e..fd9a349 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -2746,6 +2746,12 @@ int l2cap_ertm_init(struct l2cap_chan *chan)
 
 	skb_queue_head_init(&chan->tx_q);
 
+	chan->chan_id = 0;
+	chan->move_id = 0;
+	chan->move_state = L2CAP_MOVE_STABLE;
+	chan->move_role = L2CAP_MOVE_ROLE_NONE;
+	chan->move_cmd_ident = 0;
+
 	if (chan->mode != L2CAP_MODE_ERTM)
 		return 0;
 
-- 
1.7.11.2

--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

[RFCv0 02/21] Bluetooth: Factor out common L2CAP connection code

[Mat Martineau]

L2CAP connect requests and create channel requests share a significant
amount of code.  This change moves common code to a new function.

Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
 net/bluetooth/l2cap_core.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index fd9a349..24000ad 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -3362,7 +3362,8 @@ static inline int l2cap_command_rej(struct l2cap_conn *conn, struct l2cap_cmd_hd
 	return 0;
 }
 
-static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hdr *cmd, u8 *data)
+static void __l2cap_connect(struct l2cap_conn *conn, struct l2cap_cmd_hdr *cmd,
+			    u8 *data, u8 rsp_code, u8 amp_id)
 {
 	struct l2cap_conn_req *req = (struct l2cap_conn_req *) data;
 	struct l2cap_conn_rsp rsp;
@@ -3456,7 +3457,7 @@ sendresp:
 	rsp.dcid   = cpu_to_le16(dcid);
 	rsp.result = cpu_to_le16(result);
 	rsp.status = cpu_to_le16(status);
-	l2cap_send_cmd(conn, cmd->ident, L2CAP_CONN_RSP, sizeof(rsp), &rsp);
+	l2cap_send_cmd(conn, cmd->ident, rsp_code, sizeof(rsp), &rsp);
 
 	if (result == L2CAP_CR_PEND && status == L2CAP_CS_NO_INFO) {
 		struct l2cap_info_req info;
@@ -3479,7 +3480,12 @@ sendresp:
 					l2cap_build_conf_req(chan, buf), buf);
 		chan->num_conf_req++;
 	}
+}
 
+static int l2cap_connect_req(struct l2cap_conn *conn,
+			     struct l2cap_cmd_hdr *cmd, u8 *data)
+{
+	__l2cap_connect(conn, cmd, data, L2CAP_CONN_RSP, 0);
 	return 0;
 }
 
-- 
1.7.11.2

--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

[RFCv0 03/21] Bluetooth: Add L2CAP create channel request handling.

[Mat Martineau]

The L2CAP create channel request is very similar to an L2CAP connect
request, but it has an additional parameter for the controller ID.  If
the controller id is 0, the channel is set up on the BR/EDR controller
(just like a connect request).  Using a valid high speed controller ID
will cause the channel to be initially created on that high speed
controller.  While the L2CAP data will be initially routed over the
AMP controller, the L2CAP fixed signaling channel only uses BR/EDR.

When a create channel request is received for a high speed controller,
a pending response is always sent first.  After the high speed
physical and logical links are complete a success response will be
sent.

Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
 net/bluetooth/l2cap_core.c | 69 +++++++++++++++++++++++++++++++++++-----------
 1 file changed, 53 insertions(+), 16 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 24000ad..131d0da 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -3362,8 +3362,9 @@ static inline int l2cap_command_rej(struct l2cap_conn *conn, struct l2cap_cmd_hd
 	return 0;
 }
 
-static void __l2cap_connect(struct l2cap_conn *conn, struct l2cap_cmd_hdr *cmd,
-			    u8 *data, u8 rsp_code, u8 amp_id)
+static struct l2cap_chan *__l2cap_connect(struct l2cap_conn *conn,
+					  struct l2cap_cmd_hdr *cmd,
+					  u8 *data, u8 rsp_code, u8 chan_id)
 {
 	struct l2cap_conn_req *req = (struct l2cap_conn_req *) data;
 	struct l2cap_conn_rsp rsp;
@@ -3390,7 +3391,7 @@ static void __l2cap_connect(struct l2cap_conn *conn, struct l2cap_cmd_hdr *cmd,
 
 	/* Check if the ACL is secure enough (if not SDP) */
 	if (psm != __constant_cpu_to_le16(L2CAP_PSM_SDP) &&
-				!hci_conn_check_link_mode(conn->hcon)) {
+	    !hci_conn_check_link_mode(conn->hcon)) {
 		conn->disc_reason = HCI_ERROR_AUTH_FAILURE;
 		result = L2CAP_CR_SEC_BLOCK;
 		goto response;
@@ -3414,6 +3415,7 @@ static void __l2cap_connect(struct l2cap_conn *conn, struct l2cap_cmd_hdr *cmd,
 	bacpy(&bt_sk(sk)->dst, conn->dst);
 	chan->psm  = psm;
 	chan->dcid = scid;
+	chan->chan_id = chan_id;
 
 	bt_accept_enqueue(parent, sk);
 
@@ -3433,8 +3435,16 @@ static void __l2cap_connect(struct l2cap_conn *conn, struct l2cap_cmd_hdr *cmd,
 				status = L2CAP_CS_AUTHOR_PEND;
 				parent->sk_data_ready(parent, 0);
 			} else {
-				__l2cap_state_change(chan, BT_CONFIG);
-				result = L2CAP_CR_SUCCESS;
+				/* Force pending result for AMP controllers.
+				 * The connection will succeed after the
+				 * physical link is up. */
+				if (chan_id) {
+					__l2cap_state_change(chan, BT_CONNECT2);
+					result = L2CAP_CR_PEND;
+				} else {
+					__l2cap_state_change(chan, BT_CONFIG);
+					result = L2CAP_CR_SUCCESS;
+				}
 				status = L2CAP_CS_NO_INFO;
 			}
 		} else {
@@ -3480,6 +3490,8 @@ sendresp:
 					l2cap_build_conf_req(chan, buf), buf);
 		chan->num_conf_req++;
 	}
+
+	return chan;
 }
 
 static int l2cap_connect_req(struct l2cap_conn *conn,
@@ -3970,12 +3982,12 @@ static inline int l2cap_information_rsp(struct l2cap_conn *conn, struct l2cap_cm
 	return 0;
 }
 
-static inline int l2cap_create_channel_req(struct l2cap_conn *conn,
-					struct l2cap_cmd_hdr *cmd, u16 cmd_len,
-					void *data)
+static int l2cap_create_channel_req(struct l2cap_conn *conn,
+				    struct l2cap_cmd_hdr *cmd, u16 cmd_len,
+				    void *data)
 {
 	struct l2cap_create_chan_req *req = data;
-	struct l2cap_create_chan_rsp rsp;
+	struct l2cap_chan *chan;
 	u16 psm, scid;
 
 	if (cmd_len != sizeof(*req))
@@ -3989,14 +4001,39 @@ static inline 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);
 
-	/* Placeholder: Always reject */
-	rsp.dcid = 0;
-	rsp.scid = cpu_to_le16(scid);
-	rsp.result = __constant_cpu_to_le16(L2CAP_CR_NO_MEM);
-	rsp.status = __constant_cpu_to_le16(L2CAP_CS_NO_INFO);
+	if (req->amp_id) {
+		struct hci_dev *hdev;
 
-	l2cap_send_cmd(conn, cmd->ident, L2CAP_CREATE_CHAN_RSP,
-		       sizeof(rsp), &rsp);
+		/* Validate AMP controller id */
+		hdev = hci_dev_get(req->amp_id);
+		if (!hdev || !test_bit(HCI_UP, &hdev->flags)) {
+			struct l2cap_create_chan_rsp rsp;
+
+			rsp.dcid = 0;
+			rsp.scid = cpu_to_le16(scid);
+			rsp.result = L2CAP_CR_BAD_AMP;
+			rsp.status = L2CAP_CS_NO_INFO;
+
+			l2cap_send_cmd(conn, cmd->ident, L2CAP_CREATE_CHAN_RSP,
+				       sizeof(rsp), &rsp);
+
+			if (hdev)
+				hci_dev_put(hdev);
+
+			return 0;
+		}
+
+		hci_dev_put(hdev);
+	}
+
+	chan = __l2cap_connect(conn, cmd, data, L2CAP_CREATE_CHAN_RSP,
+			       req->amp_id);
+
+	/* Placeholder - uncomment when amp functions are available
+	if (chan && req->amp_id &&
+	    (conn->info_state & L2CAP_INFO_FEAT_MASK_REQ_DONE))
+		amp_accept_physical(conn, req->amp_id, sk);
+	*/
 
 	return 0;
 }
-- 
1.7.11.2

--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

Re: [RFCv0 03/21] Bluetooth: Add L2CAP create channel request handling.

[Andrei Emeltchenko]

Hi Mat,

On Wed, Jul 25, 2012 at 04:50:55PM -0700, Mat Martineau wrote:
> The L2CAP create channel request is very similar to an L2CAP connect
> request, but it has an additional parameter for the controller ID.  If
> the controller id is 0, the channel is set up on the BR/EDR controller
> (just like a connect request).  Using a valid high speed controller ID
> will cause the channel to be initially created on that high speed
> controller.  While the L2CAP data will be initially routed over the
> AMP controller, the L2CAP fixed signaling channel only uses BR/EDR.
> 
> When a create channel request is received for a high speed controller,
> a pending response is always sent first.  After the high speed
> physical and logical links are complete a success response will be
> sent.
> 
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
>  net/bluetooth/l2cap_core.c | 69 +++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 53 insertions(+), 16 deletions(-)
> 
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 24000ad..131d0da 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -3362,8 +3362,9 @@ static inline int l2cap_command_rej(struct l2cap_conn *conn, struct l2cap_cmd_hd
>  	return 0;
>  }
>  
> -static void __l2cap_connect(struct l2cap_conn *conn, struct l2cap_cmd_hdr *cmd,
> -			    u8 *data, u8 rsp_code, u8 amp_id)
> +static struct l2cap_chan *__l2cap_connect(struct l2cap_conn *conn,
> +					  struct l2cap_cmd_hdr *cmd,
> +					  u8 *data, u8 rsp_code, u8 chan_id)

Do you think chan_id is better then amp_id? At least for channel connect amp_id
looks better.

>  {
>  	struct l2cap_conn_req *req = (struct l2cap_conn_req *) data;
>  	struct l2cap_conn_rsp rsp;
> @@ -3390,7 +3391,7 @@ static void __l2cap_connect(struct l2cap_conn *conn, struct l2cap_cmd_hdr *cmd,
>  
>  	/* Check if the ACL is secure enough (if not SDP) */
>  	if (psm != __constant_cpu_to_le16(L2CAP_PSM_SDP) &&
> -				!hci_conn_check_link_mode(conn->hcon)) {
> +	    !hci_conn_check_link_mode(conn->hcon)) {

This change (and maybe some others may be merged with previous patch) ?

>  		conn->disc_reason = HCI_ERROR_AUTH_FAILURE;
>  		result = L2CAP_CR_SEC_BLOCK;
>  		goto response;
> @@ -3414,6 +3415,7 @@ static void __l2cap_connect(struct l2cap_conn *conn, struct l2cap_cmd_hdr *cmd,
>  	bacpy(&bt_sk(sk)->dst, conn->dst);
>  	chan->psm  = psm;
>  	chan->dcid = scid;
> +	chan->chan_id = chan_id;
>  
>  	bt_accept_enqueue(parent, sk);
>  
> @@ -3433,8 +3435,16 @@ static void __l2cap_connect(struct l2cap_conn *conn, struct l2cap_cmd_hdr *cmd,
>  				status = L2CAP_CS_AUTHOR_PEND;
>  				parent->sk_data_ready(parent, 0);
>  			} else {
> -				__l2cap_state_change(chan, BT_CONFIG);
> -				result = L2CAP_CR_SUCCESS;
> +				/* Force pending result for AMP controllers.
> +				 * The connection will succeed after the
> +				 * physical link is up. */
> +				if (chan_id) {
> +					__l2cap_state_change(chan, BT_CONNECT2);
> +					result = L2CAP_CR_PEND;
> +				} else {
> +					__l2cap_state_change(chan, BT_CONFIG);
> +					result = L2CAP_CR_SUCCESS;
> +				}
>  				status = L2CAP_CS_NO_INFO;
>  			}
>  		} else {
> @@ -3480,6 +3490,8 @@ sendresp:
>  					l2cap_build_conf_req(chan, buf), buf);
>  		chan->num_conf_req++;
>  	}
> +
> +	return chan;
>  }
>  
>  static int l2cap_connect_req(struct l2cap_conn *conn,
> @@ -3970,12 +3982,12 @@ static inline int l2cap_information_rsp(struct l2cap_conn *conn, struct l2cap_cm
>  	return 0;
>  }
>  
> -static inline int l2cap_create_channel_req(struct l2cap_conn *conn,
> -					struct l2cap_cmd_hdr *cmd, u16 cmd_len,
> -					void *data)
> +static int l2cap_create_channel_req(struct l2cap_conn *conn,
> +				    struct l2cap_cmd_hdr *cmd, u16 cmd_len,
> +				    void *data)
>  {
>  	struct l2cap_create_chan_req *req = data;
> -	struct l2cap_create_chan_rsp rsp;
> +	struct l2cap_chan *chan;
>  	u16 psm, scid;
>  
>  	if (cmd_len != sizeof(*req))
> @@ -3989,14 +4001,39 @@ static inline 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);
>  
> -	/* Placeholder: Always reject */
> -	rsp.dcid = 0;
> -	rsp.scid = cpu_to_le16(scid);
> -	rsp.result = __constant_cpu_to_le16(L2CAP_CR_NO_MEM);
> -	rsp.status = __constant_cpu_to_le16(L2CAP_CS_NO_INFO);
> +	if (req->amp_id) {
> +		struct hci_dev *hdev;
>  
> -	l2cap_send_cmd(conn, cmd->ident, L2CAP_CREATE_CHAN_RSP,
> -		       sizeof(rsp), &rsp);
> +		/* Validate AMP controller id */
> +		hdev = hci_dev_get(req->amp_id);
> +		if (!hdev || !test_bit(HCI_UP, &hdev->flags)) {

Should we also check dev_type here? This might be second BREDR controller.

> +			struct l2cap_create_chan_rsp rsp;
> +
> +			rsp.dcid = 0;
> +			rsp.scid = cpu_to_le16(scid);
> +			rsp.result = L2CAP_CR_BAD_AMP;
> +			rsp.status = L2CAP_CS_NO_INFO;
> +
> +			l2cap_send_cmd(conn, cmd->ident, L2CAP_CREATE_CHAN_RSP,
> +				       sizeof(rsp), &rsp);
> +
> +			if (hdev)
> +				hci_dev_put(hdev);
> +
> +			return 0;
> +		}
> +
> +		hci_dev_put(hdev);
> +	}
> +
> +	chan = __l2cap_connect(conn, cmd, data, L2CAP_CREATE_CHAN_RSP,
> +			       req->amp_id);
> +
> +	/* Placeholder - uncomment when amp functions are available
> +	if (chan && req->amp_id &&
> +	    (conn->info_state & L2CAP_INFO_FEAT_MASK_REQ_DONE))
> +		amp_accept_physical(conn, req->amp_id, sk);

I hope "sk" is a typo, we shall use "chan" here.

Best regards 
Andrei Emeltchenko 

Re: [RFCv0 03/21] Bluetooth: Add L2CAP create channel request handling.

[Mat Martineau]

On Thu, 26 Jul 2012, Andrei Emeltchenko wrote:

> Hi Mat,
>
> On Wed, Jul 25, 2012 at 04:50:55PM -0700, Mat Martineau wrote:
>> The L2CAP create channel request is very similar to an L2CAP connect
>> request, but it has an additional parameter for the controller ID.  If
>> the controller id is 0, the channel is set up on the BR/EDR controller
>> (just like a connect request).  Using a valid high speed controller ID
>> will cause the channel to be initially created on that high speed
>> controller.  While the L2CAP data will be initially routed over the
>> AMP controller, the L2CAP fixed signaling channel only uses BR/EDR.
>>
>> When a create channel request is received for a high speed controller,
>> a pending response is always sent first.  After the high speed
>> physical and logical links are complete a success response will be
>> sent.
>>
>> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
>> ---
>>  net/bluetooth/l2cap_core.c | 69 +++++++++++++++++++++++++++++++++++-----------
>>  1 file changed, 53 insertions(+), 16 deletions(-)
>>
>> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
>> index 24000ad..131d0da 100644
>> --- a/net/bluetooth/l2cap_core.c
>> +++ b/net/bluetooth/l2cap_core.c
>> @@ -3362,8 +3362,9 @@ static inline int l2cap_command_rej(struct l2cap_conn *conn, struct l2cap_cmd_hd
>>  	return 0;
>>  }
>>
>> -static void __l2cap_connect(struct l2cap_conn *conn, struct l2cap_cmd_hdr *cmd,
>> -			    u8 *data, u8 rsp_code, u8 amp_id)
>> +static struct l2cap_chan *__l2cap_connect(struct l2cap_conn *conn,
>> +					  struct l2cap_cmd_hdr *cmd,
>> +					  u8 *data, u8 rsp_code, u8 chan_id)
>
> Do you think chan_id is better then amp_id? At least for channel connect amp_id
> looks better.
>

When we were defining the socket option API, Marcel preferred "channel 
policy" over "AMP policy", so I tried to keep terminology more generic 
in this code as well.  That might not have been the correct decision. 
I'm fine with anything: amp_id, chan_id, controller_id, ...

>>  {
>>  	struct l2cap_conn_req *req = (struct l2cap_conn_req *) data;
>>  	struct l2cap_conn_rsp rsp;
>> @@ -3390,7 +3391,7 @@ static void __l2cap_connect(struct l2cap_conn *conn, struct l2cap_cmd_hdr *cmd,
>>
>>  	/* Check if the ACL is secure enough (if not SDP) */
>>  	if (psm != __constant_cpu_to_le16(L2CAP_PSM_SDP) &&
>> -				!hci_conn_check_link_mode(conn->hcon)) {
>> +	    !hci_conn_check_link_mode(conn->hcon)) {
>
> This change (and maybe some others may be merged with previous patch) ?

I think it fits better with this patch. This is a separate hunk in the 
patch, but the modifies the same function as the hunks before and 
after.

>>  		conn->disc_reason = HCI_ERROR_AUTH_FAILURE;
>>  		result = L2CAP_CR_SEC_BLOCK;
>>  		goto response;
>> @@ -3414,6 +3415,7 @@ static void __l2cap_connect(struct l2cap_conn *conn, struct l2cap_cmd_hdr *cmd,
>>  	bacpy(&bt_sk(sk)->dst, conn->dst);
>>  	chan->psm  = psm;
>>  	chan->dcid = scid;
>> +	chan->chan_id = chan_id;
>>
>>  	bt_accept_enqueue(parent, sk);
>>
>> @@ -3433,8 +3435,16 @@ static void __l2cap_connect(struct l2cap_conn *conn, struct l2cap_cmd_hdr *cmd,
>>  				status = L2CAP_CS_AUTHOR_PEND;
>>  				parent->sk_data_ready(parent, 0);
>>  			} else {
>> -				__l2cap_state_change(chan, BT_CONFIG);
>> -				result = L2CAP_CR_SUCCESS;
>> +				/* Force pending result for AMP controllers.
>> +				 * The connection will succeed after the
>> +				 * physical link is up. */
>> +				if (chan_id) {
>> +					__l2cap_state_change(chan, BT_CONNECT2);
>> +					result = L2CAP_CR_PEND;
>> +				} else {
>> +					__l2cap_state_change(chan, BT_CONFIG);
>> +					result = L2CAP_CR_SUCCESS;
>> +				}
>>  				status = L2CAP_CS_NO_INFO;
>>  			}
>>  		} else {
>> @@ -3480,6 +3490,8 @@ sendresp:
>>  					l2cap_build_conf_req(chan, buf), buf);
>>  		chan->num_conf_req++;
>>  	}
>> +
>> +	return chan;
>>  }
>>
>>  static int l2cap_connect_req(struct l2cap_conn *conn,
>> @@ -3970,12 +3982,12 @@ static inline int l2cap_information_rsp(struct l2cap_conn *conn, struct l2cap_cm
>>  	return 0;
>>  }
>>
>> -static inline int l2cap_create_channel_req(struct l2cap_conn *conn,
>> -					struct l2cap_cmd_hdr *cmd, u16 cmd_len,
>> -					void *data)
>> +static int l2cap_create_channel_req(struct l2cap_conn *conn,
>> +				    struct l2cap_cmd_hdr *cmd, u16 cmd_len,
>> +				    void *data)
>>  {
>>  	struct l2cap_create_chan_req *req = data;
>> -	struct l2cap_create_chan_rsp rsp;
>> +	struct l2cap_chan *chan;
>>  	u16 psm, scid;
>>
>>  	if (cmd_len != sizeof(*req))
>> @@ -3989,14 +4001,39 @@ static inline 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);
>>
>> -	/* Placeholder: Always reject */
>> -	rsp.dcid = 0;
>> -	rsp.scid = cpu_to_le16(scid);
>> -	rsp.result = __constant_cpu_to_le16(L2CAP_CR_NO_MEM);
>> -	rsp.status = __constant_cpu_to_le16(L2CAP_CS_NO_INFO);
>> +	if (req->amp_id) {
>> +		struct hci_dev *hdev;
>>
>> -	l2cap_send_cmd(conn, cmd->ident, L2CAP_CREATE_CHAN_RSP,
>> -		       sizeof(rsp), &rsp);
>> +		/* Validate AMP controller id */
>> +		hdev = hci_dev_get(req->amp_id);
>> +		if (!hdev || !test_bit(HCI_UP, &hdev->flags)) {
>
> Should we also check dev_type here? This might be second BREDR controller.
>

Yes.

>> +			struct l2cap_create_chan_rsp rsp;
>> +
>> +			rsp.dcid = 0;
>> +			rsp.scid = cpu_to_le16(scid);
>> +			rsp.result = L2CAP_CR_BAD_AMP;
>> +			rsp.status = L2CAP_CS_NO_INFO;
>> +
>> +			l2cap_send_cmd(conn, cmd->ident, L2CAP_CREATE_CHAN_RSP,
>> +				       sizeof(rsp), &rsp);
>> +
>> +			if (hdev)
>> +				hci_dev_put(hdev);
>> +
>> +			return 0;
>> +		}
>> +
>> +		hci_dev_put(hdev);
>> +	}
>> +
>> +	chan = __l2cap_connect(conn, cmd, data, L2CAP_CREATE_CHAN_RSP,
>> +			       req->amp_id);
>> +
>> +	/* Placeholder - uncomment when amp functions are available
>> +	if (chan && req->amp_id &&
>> +	    (conn->info_state & L2CAP_INFO_FEAT_MASK_REQ_DONE))
>> +		amp_accept_physical(conn, req->amp_id, sk);
>
> I hope "sk" is a typo, we shall use "chan" here.

Yes, just a typo in commented-out code.


--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

Re: [RFCv0 03/21] Bluetooth: Add L2CAP create channel request handling.

[Andrei Emeltchenko]

Hi Mat,

On Thu, Jul 26, 2012 at 08:04:11AM -0700, Mat Martineau wrote:
> >>-static void __l2cap_connect(struct l2cap_conn *conn, struct l2cap_cmd_hdr *cmd,
> >>-			    u8 *data, u8 rsp_code, u8 amp_id)
> >>+static struct l2cap_chan *__l2cap_connect(struct l2cap_conn *conn,
> >>+					  struct l2cap_cmd_hdr *cmd,
> >>+					  u8 *data, u8 rsp_code, u8 chan_id)
> >
> >Do you think chan_id is better then amp_id? At least for channel connect amp_id
> >looks better.
> >
> 
> When we were defining the socket option API, Marcel preferred
> "channel policy" over "AMP policy", so I tried to keep terminology
> more generic in this code as well.  That might not have been the
> correct decision. I'm fine with anything: amp_id, chan_id,
> controller_id, ...

I think amp_id and controller_id (or even remote_amp_id) are better names
since this is "the identifier of the Controller on the remote device
obtained via an AMP Manager Discover Available AMPs request."

chan_id is kind of identifier of a channel, a name would be messed with CID
which is already used for other purposes. I think the name channel policy
is also OK since it is a policy of a channel (for each channel we might
have different policy).

Best regards 
Andrei Emeltchenko 

[RFCv0 04/21] Bluetooth: Process create response and connect response identically

[Mat Martineau]

Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
 net/bluetooth/l2cap_core.c | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 131d0da..d7a501e 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -4038,14 +4038,6 @@ static int l2cap_create_channel_req(struct l2cap_conn *conn,
 	return 0;
 }
 
-static inline int l2cap_create_channel_rsp(struct l2cap_conn *conn,
-					struct l2cap_cmd_hdr *cmd, void *data)
-{
-	BT_DBG("conn %p", conn);
-
-	return l2cap_connect_rsp(conn, cmd, data);
-}
-
 static void l2cap_send_move_chan_rsp(struct l2cap_conn *conn, u8 ident,
 				     u16 icid, u16 result)
 {
@@ -4249,6 +4241,7 @@ static inline int l2cap_bredr_sig_cmd(struct l2cap_conn *conn,
 		break;
 
 	case L2CAP_CONN_RSP:
+	case L2CAP_CREATE_CHAN_RSP:
 		err = l2cap_connect_rsp(conn, cmd, data);
 		break;
 
@@ -4287,10 +4280,6 @@ static inline int l2cap_bredr_sig_cmd(struct l2cap_conn *conn,
 		err = l2cap_create_channel_req(conn, cmd, cmd_len, data);
 		break;
 
-	case L2CAP_CREATE_CHAN_RSP:
-		err = l2cap_create_channel_rsp(conn, cmd, data);
-		break;
-
 	case L2CAP_MOVE_CHAN_REQ:
 		err = l2cap_move_channel_req(conn, cmd, cmd_len, data);
 		break;
-- 
1.7.11.2

--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

Re: [RFCv0 04/21] Bluetooth: Process create response and connect response identically

[Andrei Emeltchenko]

Hi Mat,

On Wed, Jul 25, 2012 at 04:50:56PM -0700, Mat Martineau wrote:
...
> -static inline int l2cap_create_channel_rsp(struct l2cap_conn *conn,
> -					struct l2cap_cmd_hdr *cmd, void *data)
> -{
> -	BT_DBG("conn %p", conn);
> -
> -	return l2cap_connect_rsp(conn, cmd, data);
> -}
> -
>  static void l2cap_send_move_chan_rsp(struct l2cap_conn *conn, u8 ident,
>  				     u16 icid, u16 result)
>  {
> @@ -4249,6 +4241,7 @@ static inline int l2cap_bredr_sig_cmd(struct l2cap_conn *conn,
>  		break;
>  
>  	case L2CAP_CONN_RSP:
> +	case L2CAP_CREATE_CHAN_RSP:
>  		err = l2cap_connect_rsp(conn, cmd, data);
...

We might rename l2cap_connect_rsp to something like
l2cap_connect_create_rsp

Best regards 
Andrei Emeltchenko 

Re: [RFCv0 04/21] Bluetooth: Process create response and connect response identically

[Mat Martineau]

Hi Andrei -

On Tue, 14 Aug 2012, Andrei Emeltchenko wrote:

> Hi Mat,
>
> On Wed, Jul 25, 2012 at 04:50:56PM -0700, Mat Martineau wrote:
> ...
>> -static inline int l2cap_create_channel_rsp(struct l2cap_conn *conn,
>> -					struct l2cap_cmd_hdr *cmd, void *data)
>> -{
>> -	BT_DBG("conn %p", conn);
>> -
>> -	return l2cap_connect_rsp(conn, cmd, data);
>> -}
>> -
>>  static void l2cap_send_move_chan_rsp(struct l2cap_conn *conn, u8 ident,
>>  				     u16 icid, u16 result)
>>  {
>> @@ -4249,6 +4241,7 @@ static inline int l2cap_bredr_sig_cmd(struct l2cap_conn *conn,
>>  		break;
>>
>>  	case L2CAP_CONN_RSP:
>> +	case L2CAP_CREATE_CHAN_RSP:
>>  		err = l2cap_connect_rsp(conn, cmd, data);
> ...
>
> We might rename l2cap_connect_rsp to something like
> l2cap_connect_create_rsp

Ok, I'll update that function name.


--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

[RFCv0 05/21] Bluetooth: Lookup channel id for channel move

[Mat Martineau]

Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
 net/bluetooth/l2cap_core.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index d7a501e..176a81d 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -97,6 +97,21 @@ static struct l2cap_chan *l2cap_get_chan_by_scid(struct l2cap_conn *conn, u16 ci
 	return c;
 }
 
+/* Find channel with given DCID.
+ * Returns locked channel. */
+static struct l2cap_chan *l2cap_get_chan_by_dcid(struct l2cap_conn *conn, u16 cid)
+{
+	struct l2cap_chan *c;
+
+	mutex_lock(&conn->chan_lock);
+	c = __l2cap_get_chan_by_dcid(conn, cid);
+	if (c)
+		l2cap_chan_lock(c);
+	mutex_unlock(&conn->chan_lock);
+
+	return c;
+}
+
 static struct l2cap_chan *__l2cap_get_chan_by_ident(struct l2cap_conn *conn, u8 ident)
 {
 	struct l2cap_chan *c;
@@ -4086,6 +4101,7 @@ static inline int l2cap_move_channel_req(struct l2cap_conn *conn,
 					 u16 cmd_len, void *data)
 {
 	struct l2cap_move_chan_req *req = data;
+	struct l2cap_chan *chan;
 	u16 icid = 0;
 	u16 result = L2CAP_MR_NOT_ALLOWED;
 
@@ -4099,9 +4115,14 @@ static inline int l2cap_move_channel_req(struct l2cap_conn *conn,
 	if (!enable_hs)
 		return -EINVAL;
 
+	chan = l2cap_get_chan_by_dcid(conn, icid);
+
 	/* Placeholder: Always refuse */
 	l2cap_send_move_chan_rsp(conn, cmd->ident, icid, result);
 
+	if (chan)
+		l2cap_chan_unlock(chan);
+
 	return 0;
 }
 
-- 
1.7.11.2

--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

Re: [RFCv0 05/21] Bluetooth: Lookup channel id for channel move

[Andrei Emeltchenko]

Hi Mat,

On Wed, Jul 25, 2012 at 04:50:57PM -0700, Mat Martineau wrote:
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
>  net/bluetooth/l2cap_core.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index d7a501e..176a81d 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -97,6 +97,21 @@ static struct l2cap_chan *l2cap_get_chan_by_scid(struct l2cap_conn *conn, u16 ci
>  	return c;
>  }
>  
> +/* Find channel with given DCID.
> + * Returns locked channel. */

I believe the comment is not needed here.

> +static struct l2cap_chan *l2cap_get_chan_by_dcid(struct l2cap_conn *conn, u16 cid)
> +{
> +	struct l2cap_chan *c;
> +
> +	mutex_lock(&conn->chan_lock);
> +	c = __l2cap_get_chan_by_dcid(conn, cid);
> +	if (c)
> +		l2cap_chan_lock(c);
> +	mutex_unlock(&conn->chan_lock);
> +
> +	return c;
> +}
> +
>  static struct l2cap_chan *__l2cap_get_chan_by_ident(struct l2cap_conn *conn, u8 ident)
>  {
>  	struct l2cap_chan *c;
> @@ -4086,6 +4101,7 @@ static inline int l2cap_move_channel_req(struct l2cap_conn *conn,
>  					 u16 cmd_len, void *data)
>  {
>  	struct l2cap_move_chan_req *req = data;
> +	struct l2cap_chan *chan;
>  	u16 icid = 0;
>  	u16 result = L2CAP_MR_NOT_ALLOWED;
>  
> @@ -4099,9 +4115,14 @@ static inline int l2cap_move_channel_req(struct l2cap_conn *conn,
>  	if (!enable_hs)
>  		return -EINVAL;
>  
> +	chan = l2cap_get_chan_by_dcid(conn, icid);
> +
>  	/* Placeholder: Always refuse */
>  	l2cap_send_move_chan_rsp(conn, cmd->ident, icid, result);
>  
> +	if (chan)
> +		l2cap_chan_unlock(chan);
> +

this part does not change anything, might be merged with later patch which
would add functionality.

Best regards 
Andrei Emeltchenko 

Re: [RFCv0 05/21] Bluetooth: Lookup channel id for channel move

[Mat Martineau]

On Thu, 26 Jul 2012, Andrei Emeltchenko wrote:

> Hi Mat,
>
> On Wed, Jul 25, 2012 at 04:50:57PM -0700, Mat Martineau wrote:
>> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
>> ---
>>  net/bluetooth/l2cap_core.c | 21 +++++++++++++++++++++
>>  1 file changed, 21 insertions(+)
>>
>> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
>> index d7a501e..176a81d 100644
>> --- a/net/bluetooth/l2cap_core.c
>> +++ b/net/bluetooth/l2cap_core.c
>> @@ -97,6 +97,21 @@ static struct l2cap_chan *l2cap_get_chan_by_scid(struct l2cap_conn *conn, u16 ci
>>  	return c;
>>  }
>>
>> +/* Find channel with given DCID.
>> + * Returns locked channel. */
>
> I believe the comment is not needed here.

l2cap_get_chan_by_scid() has exactly the same comment, I'm trying to 
stay consistent with that.

>> +static struct l2cap_chan *l2cap_get_chan_by_dcid(struct l2cap_conn *conn, u16 cid)
>> +{
>> +	struct l2cap_chan *c;
>> +
>> +	mutex_lock(&conn->chan_lock);
>> +	c = __l2cap_get_chan_by_dcid(conn, cid);
>> +	if (c)
>> +		l2cap_chan_lock(c);
>> +	mutex_unlock(&conn->chan_lock);
>> +
>> +	return c;
>> +}
>> +
>>  static struct l2cap_chan *__l2cap_get_chan_by_ident(struct l2cap_conn *conn, u8 ident)
>>  {
>>  	struct l2cap_chan *c;
>> @@ -4086,6 +4101,7 @@ static inline int l2cap_move_channel_req(struct l2cap_conn *conn,
>>  					 u16 cmd_len, void *data)
>>  {
>>  	struct l2cap_move_chan_req *req = data;
>> +	struct l2cap_chan *chan;
>>  	u16 icid = 0;
>>  	u16 result = L2CAP_MR_NOT_ALLOWED;
>>
>> @@ -4099,9 +4115,14 @@ static inline int l2cap_move_channel_req(struct l2cap_conn *conn,
>>  	if (!enable_hs)
>>  		return -EINVAL;
>>
>> +	chan = l2cap_get_chan_by_dcid(conn, icid);
>> +
>>  	/* Placeholder: Always refuse */
>>  	l2cap_send_move_chan_rsp(conn, cmd->ident, icid, result);
>>
>> +	if (chan)
>> +		l2cap_chan_unlock(chan);
>> +
>
> this part does not change anything, might be merged with later patch which
> would add functionality.

It eliminates an "unused function" compiler warning for 
l2cap_get_chan_by_dcid().


--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

[RFCv0 06/21] Bluetooth: Channel move request handling

[Mat Martineau]

Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
 net/bluetooth/l2cap_core.c | 93 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 92 insertions(+), 1 deletion(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 176a81d..b71d62f 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -974,6 +974,37 @@ 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_move_setup(struct l2cap_chan *chan)
+{
+	struct sk_buff *skb;
+
+	BT_DBG("chan %p", chan);
+
+	__clear_retrans_timer(chan);
+	__clear_monitor_timer(chan);
+	__clear_ack_timer(chan);
+
+	chan->retry_count = 0;
+	skb_queue_walk(&chan->tx_q, skb) {
+		if (bt_cb(skb)->control.retries)
+			bt_cb(skb)->control.retries = 1;
+		else
+			break;
+	}
+
+	chan->expected_tx_seq = chan->buffer_seq;
+
+	clear_bit(CONN_REJ_ACT, &chan->conn_state);
+	clear_bit(CONN_SREJ_ACT, &chan->conn_state);
+	l2cap_seq_list_clear(&chan->retrans_list);
+	l2cap_seq_list_clear(&chan->srej_list);
+	skb_queue_purge(&chan->srej_q);
+
+	chan->tx_state = L2CAP_TX_STATE_XMIT;
+	chan->rx_state = L2CAP_RX_STATE_MOVE;
+
+	set_bit(CONN_REMOTE_BUSY, &chan->conn_state);
+}
 
 static void l2cap_chan_ready(struct l2cap_chan *chan)
 {
@@ -4117,7 +4148,67 @@ static inline int l2cap_move_channel_req(struct l2cap_conn *conn,
 
 	chan = l2cap_get_chan_by_dcid(conn, icid);
 
-	/* Placeholder: Always refuse */
+	if (!chan)
+		goto send_move_response;
+
+	if (chan->scid < L2CAP_CID_DYN_START || (chan->mode != L2CAP_MODE_ERTM
+	    && chan->mode != L2CAP_MODE_STREAMING))
+		goto send_move_response;
+
+	if (chan->chan_id == req->dest_amp_id) {
+		result = L2CAP_MR_SAME_ID;
+		goto send_move_response;
+	}
+
+	if (req->dest_amp_id) {
+		struct hci_dev *hdev;
+		hdev = hci_dev_get(req->dest_amp_id);
+		if (!hdev || !test_bit(HCI_UP, &hdev->flags)) {
+			if (hdev)
+				hci_dev_put(hdev);
+
+			result = L2CAP_MR_BAD_ID;
+			goto send_move_response;
+		}
+		hci_dev_put(hdev);
+	}
+
+	if (((chan->move_state != L2CAP_MOVE_STABLE &&
+	    chan->move_state != L2CAP_MOVE_WAIT_PREPARE) ||
+	    chan->move_role != L2CAP_MOVE_ROLE_NONE) &&
+	    bacmp(conn->src, conn->dst) > 0) {
+		result = L2CAP_MR_COLLISION;
+		goto send_move_response;
+	}
+
+	if (chan->chan_policy == BT_CHANNEL_POLICY_BREDR_ONLY) {
+		result = L2CAP_MR_NOT_ALLOWED;
+		goto send_move_response;
+	}
+
+	chan->move_cmd_ident = cmd->ident;
+	chan->move_role = L2CAP_MOVE_ROLE_RESPONDER;
+	l2cap_move_setup(chan);
+	chan->move_id = req->dest_amp_id;
+	icid = chan->dcid;
+
+	if (req->dest_amp_id == 0) {
+		/* Moving to BR/EDR */
+		if (test_bit(CONN_LOCAL_BUSY, &chan->conn_state)) {
+			chan->move_state = L2CAP_MOVE_WAIT_LOCAL_BUSY;
+			result = L2CAP_MR_PEND;
+		} else {
+			chan->move_state = L2CAP_MOVE_WAIT_CONFIRM;
+			result = L2CAP_MR_SUCCESS;
+		}
+	} else {
+		chan->move_state = L2CAP_MOVE_WAIT_PREPARE;
+		/* Placeholder - uncomment when amp functions are available */
+		/*amp_accept_physical(chan, req->dest_amp_id);*/
+		result = L2CAP_MR_PEND;
+	}
+
+send_move_response:
 	l2cap_send_move_chan_rsp(conn, cmd->ident, icid, result);
 
 	if (chan)
-- 
1.7.11.2

--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

Re: [RFCv0 06/21] Bluetooth: Channel move request handling

[Andrei Emeltchenko]

Hi Mat,

On Wed, Jul 25, 2012 at 04:50:58PM -0700, Mat Martineau wrote:

...

>  static void l2cap_chan_ready(struct l2cap_chan *chan)
>  {
> @@ -4117,7 +4148,67 @@ static inline int l2cap_move_channel_req(struct l2cap_conn *conn,
>  
>  	chan = l2cap_get_chan_by_dcid(conn, icid);
>  
> -	/* Placeholder: Always refuse */
> +	if (!chan)
> +		goto send_move_response;
> +
> +	if (chan->scid < L2CAP_CID_DYN_START || (chan->mode != L2CAP_MODE_ERTM
> +	    && chan->mode != L2CAP_MODE_STREAMING))

I think if we add here line below the code would be more understandable
and following the same style.
		"result = L2CAP_MR_NOT_ALLOWED;"

> +		goto send_move_response;
> +
> +	if (chan->chan_id == req->dest_amp_id) {
> +		result = L2CAP_MR_SAME_ID;
> +		goto send_move_response;
> +	}
> +
> +	if (req->dest_amp_id) {
> +		struct hci_dev *hdev;
> +		hdev = hci_dev_get(req->dest_amp_id);
> +		if (!hdev || !test_bit(HCI_UP, &hdev->flags)) {

Again here we shall check dev_type.

> +			if (hdev)
> +				hci_dev_put(hdev);
> +
> +			result = L2CAP_MR_BAD_ID;
> +			goto send_move_response;
> +		}
> +		hci_dev_put(hdev);
> +	}
> +
> +	if (((chan->move_state != L2CAP_MOVE_STABLE &&
> +	    chan->move_state != L2CAP_MOVE_WAIT_PREPARE) ||
> +	    chan->move_role != L2CAP_MOVE_ROLE_NONE) &&
> +	    bacmp(conn->src, conn->dst) > 0) {
> +		result = L2CAP_MR_COLLISION;
> +		goto send_move_response;
> +	}
> +
> +	if (chan->chan_policy == BT_CHANNEL_POLICY_BREDR_ONLY) {
> +		result = L2CAP_MR_NOT_ALLOWED;
> +		goto send_move_response;
> +	}
> +
> +	chan->move_cmd_ident = cmd->ident;

BTW: Why do we handle ident in a special way for channel move?

Best regards 
Andrei Emeltchenko 

Re: [RFCv0 06/21] Bluetooth: Channel move request handling

[Mat Martineau]

Hi Andrei -

On Thu, 26 Jul 2012, Andrei Emeltchenko wrote:

> Hi Mat,
>
> On Wed, Jul 25, 2012 at 04:50:58PM -0700, Mat Martineau wrote:
>
> ...
>
>>  static void l2cap_chan_ready(struct l2cap_chan *chan)
>>  {
>> @@ -4117,7 +4148,67 @@ static inline int l2cap_move_channel_req(struct l2cap_conn *conn,
>>
>>  	chan = l2cap_get_chan_by_dcid(conn, icid);
>>
>> -	/* Placeholder: Always refuse */
>> +	if (!chan)
>> +		goto send_move_response;
>> +
>> +	if (chan->scid < L2CAP_CID_DYN_START || (chan->mode != L2CAP_MODE_ERTM
>> +	    && chan->mode != L2CAP_MODE_STREAMING))
>
> I think if we add here line below the code would be more understandable
> and following the same style.
> 		"result = L2CAP_MR_NOT_ALLOWED;"
>

Ok.

>> +		goto send_move_response;
>> +
>> +	if (chan->chan_id == req->dest_amp_id) {
>> +		result = L2CAP_MR_SAME_ID;
>> +		goto send_move_response;
>> +	}
>> +
>> +	if (req->dest_amp_id) {
>> +		struct hci_dev *hdev;
>> +		hdev = hci_dev_get(req->dest_amp_id);
>> +		if (!hdev || !test_bit(HCI_UP, &hdev->flags)) {
>
> Again here we shall check dev_type.
>

Right.

>> +			if (hdev)
>> +				hci_dev_put(hdev);
>> +
>> +			result = L2CAP_MR_BAD_ID;
>> +			goto send_move_response;
>> +		}
>> +		hci_dev_put(hdev);
>> +	}
>> +
>> +	if (((chan->move_state != L2CAP_MOVE_STABLE &&
>> +	    chan->move_state != L2CAP_MOVE_WAIT_PREPARE) ||
>> +	    chan->move_role != L2CAP_MOVE_ROLE_NONE) &&
>> +	    bacmp(conn->src, conn->dst) > 0) {
>> +		result = L2CAP_MR_COLLISION;
>> +		goto send_move_response;
>> +	}
>> +
>> +	if (chan->chan_policy == BT_CHANNEL_POLICY_BREDR_ONLY) {
>> +		result = L2CAP_MR_NOT_ALLOWED;
>> +		goto send_move_response;
>> +	}
>> +
>> +	chan->move_cmd_ident = cmd->ident;
>
> BTW: Why do we handle ident in a special way for channel move?

At the time I wrote the code, I thought that chan->ident was only used 
to store idents of sent requests, not received requests.  But it does 
look like it is also used for storing received idents for use in 
sending responses.  chan->ident could be used as long as move 
collisions are handled correctly (where a move request is received 
when a move response is expected).

--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

Re: [RFCv0 06/21] Bluetooth: Channel move request handling

[Andrei Emeltchenko]

Hi Mat,

On Fri, Jul 27, 2012 at 09:54:03AM -0700, Mat Martineau wrote:

...

> >>+	if (chan->chan_policy == BT_CHANNEL_POLICY_BREDR_ONLY) {
> >>+		result = L2CAP_MR_NOT_ALLOWED;
> >>+		goto send_move_response;
> >>+	}
> >>+
> >>+	chan->move_cmd_ident = cmd->ident;
> >
> >BTW: Why do we handle ident in a special way for channel move?
> 
> At the time I wrote the code, I thought that chan->ident was only
> used to store idents of sent requests, not received requests.  But
> it does look like it is also used for storing received idents for
> use in sending responses.  chan->ident could be used as long as move
> collisions are handled correctly (where a move request is received
> when a move response is expected).

So can we use chan->ident instead of chan->move_cmd_ident? Is the channel
move somehow different from other L2CAP request / response sequence?

Best regards 
Andrei Emeltchenko 

Re: [RFCv0 06/21] Bluetooth: Channel move request handling

[Mat Martineau]

On Mon, 6 Aug 2012, Andrei Emeltchenko wrote:

> Hi Mat,
>
> On Fri, Jul 27, 2012 at 09:54:03AM -0700, Mat Martineau wrote:
>
> ...
>
>>>> +	if (chan->chan_policy == BT_CHANNEL_POLICY_BREDR_ONLY) {
>>>> +		result = L2CAP_MR_NOT_ALLOWED;
>>>> +		goto send_move_response;
>>>> +	}
>>>> +
>>>> +	chan->move_cmd_ident = cmd->ident;
>>>
>>> BTW: Why do we handle ident in a special way for channel move?
>>
>> At the time I wrote the code, I thought that chan->ident was only
>> used to store idents of sent requests, not received requests.  But
>> it does look like it is also used for storing received idents for
>> use in sending responses.  chan->ident could be used as long as move
>> collisions are handled correctly (where a move request is received
>> when a move response is expected).
>
> So can we use chan->ident instead of chan->move_cmd_ident? Is the channel
> move somehow different from other L2CAP request / response sequence?

Yes, chan->ident can be used.  It doesn't seem necessary to assign 
chan->ident when sending a move request, just when receiving one.

Connect and create requests also assign chan->ident when they are 
sent, but I'm not sure why.

--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

Re: [RFCv0 06/21] Bluetooth: Channel move request handling

[Andrei Emeltchenko]

Hi Mat,

On Mon, Aug 06, 2012 at 09:33:50AM -0700, Mat Martineau wrote:
> >>>>+	chan->move_cmd_ident = cmd->ident;
> >>>
> >>>BTW: Why do we handle ident in a special way for channel move?
> >>
> >>At the time I wrote the code, I thought that chan->ident was only
> >>used to store idents of sent requests, not received requests.  But
> >>it does look like it is also used for storing received idents for
> >>use in sending responses.  chan->ident could be used as long as move
> >>collisions are handled correctly (where a move request is received
> >>when a move response is expected).
> >
> >So can we use chan->ident instead of chan->move_cmd_ident? Is the channel
> >move somehow different from other L2CAP request / response sequence?
> 
> Yes, chan->ident can be used.  It doesn't seem necessary to assign
> chan->ident when sending a move request, just when receiving one.
> 
> Connect and create requests also assign chan->ident when they are
> sent, but I'm not sure why.

My understanding is that ident only used to match responses with requests.
So channel move is not a special case.

Reference: BLUETOOTH SPECIFICATION Version 4.0 [Vol 3] page 56 of 656

Best regards 
Andrei Emeltchenko 
 

[RFCv0 07/21] Bluetooth: Add new ERTM receive states for channel move

[Mat Martineau]

Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
 net/bluetooth/l2cap_core.c | 107 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 107 insertions(+)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index b71d62f..a327f02 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -974,6 +974,7 @@ 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_move_setup(struct l2cap_chan *chan)
 {
 	struct sk_buff *skb;
@@ -5145,6 +5146,106 @@ static int l2cap_rx_state_srej_sent(struct l2cap_chan *chan,
 	return err;
 }
 
+static int l2cap_finish_move(struct l2cap_chan *chan)
+{
+	int err = 0;
+
+	BT_DBG("chan %p", chan);
+
+	chan->move_role = L2CAP_MOVE_ROLE_NONE;
+	chan->rx_state = L2CAP_RX_STATE_RECV;
+
+	if (chan->hs_hcon)
+		chan->conn->mtu = chan->hs_hcon->hdev->acl_mtu;
+	else
+		chan->conn->mtu = chan->conn->hcon->hdev->acl_mtu;
+
+	/* Placeholder - resegment based on new conn->mtu */
+	/*err = l2cap_resegment(chan);*/
+
+	return err;
+}
+
+static int l2cap_rx_state_wait_p(struct l2cap_chan *chan, 
+				 struct l2cap_ctrl *control,
+				 struct sk_buff *skb, u8 event)
+{
+	int err = -EPROTO;
+
+	BT_DBG("chan %p, control %p, skb %p, event %d", chan, control, skb,
+	       event);
+
+	if (!control->poll)
+		return err;
+
+	l2cap_process_reqseq(chan, control->reqseq);
+
+	if (!skb_queue_empty(&chan->tx_q))
+		chan->tx_send_head = skb_peek(&chan->tx_q);
+	else
+		chan->tx_send_head = NULL;
+
+	/* Rewind next_tx_seq to the point expected
+	 * by the receiver.
+	 */
+	chan->next_tx_seq = control->reqseq;
+	chan->unacked_frames = 0;
+
+	err = l2cap_finish_move(chan);
+
+	if (err)
+		return err;
+
+	set_bit(CONN_SEND_FBIT, &chan->conn_state);
+	l2cap_send_i_or_rr_or_rnr(chan);
+
+	if (event == L2CAP_EV_RECV_IFRAME)
+		err = -EPROTO;
+	else
+		err = l2cap_rx_state_recv(chan, control, NULL, event);
+
+	return err;
+}
+
+static int l2cap_rx_state_wait_f(struct l2cap_chan *chan,
+				 struct l2cap_ctrl *control,
+				 struct sk_buff *skb, u8 event)
+{
+	int err = -EPROTO;
+
+	if (control->final) {
+		clear_bit(CONN_REMOTE_BUSY, &chan->conn_state);
+		chan->move_role = L2CAP_MOVE_ROLE_NONE;
+
+		chan->rx_state = L2CAP_RX_STATE_RECV;
+		l2cap_process_reqseq(chan, control->reqseq);
+
+		if (!skb_queue_empty(&chan->tx_q))
+			chan->tx_send_head = skb_peek(&chan->tx_q);
+		else
+			chan->tx_send_head = NULL;
+
+		/* Rewind next_tx_seq to the point expected
+		 * by the receiver.
+		 */
+		chan->next_tx_seq = control->reqseq;
+		chan->unacked_frames = 0;
+
+		if (chan->hs_hcon)
+			chan->conn->mtu = chan->hs_hcon->hdev->acl_mtu;
+		else
+			chan->conn->mtu = chan->conn->hcon->hdev->acl_mtu;
+
+		/* Placeholder - resegment based on new conn->mtu */
+		/*err = l2cap_resegment(chan);*/
+
+		if (!err)
+			err = l2cap_rx_state_recv(chan, control, skb, event);
+	}
+
+	return err;
+}
+
 static bool __valid_reqseq(struct l2cap_chan *chan, u16 reqseq)
 {
 	/* Make sure reqseq is for a packet that has been sent but not acked */
@@ -5171,6 +5272,12 @@ static int l2cap_rx(struct l2cap_chan *chan, struct l2cap_ctrl *control,
 			err = l2cap_rx_state_srej_sent(chan, control, skb,
 						       event);
 			break;
+		case L2CAP_RX_STATE_WAIT_P:
+			err = l2cap_rx_state_wait_p(chan, control, skb, event);
+			break;
+		case L2CAP_RX_STATE_WAIT_F:
+			err = l2cap_rx_state_wait_f(chan, control, skb, event);
+			break;
 		default:
 			/* shut it down */
 			break;
-- 
1.7.11.2

--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

[RFCv0 08/21] Bluetooth: Add move channel confirm handling

[Mat Martineau]

Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
 net/bluetooth/l2cap_core.c | 71 ++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 68 insertions(+), 3 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index a327f02..b91ad10 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -981,6 +981,9 @@ static void l2cap_move_setup(struct l2cap_chan *chan)
 
 	BT_DBG("chan %p", chan);
 
+	if (chan->mode != L2CAP_MODE_ERTM)
+		return;
+
 	__clear_retrans_timer(chan);
 	__clear_monitor_timer(chan);
 	__clear_ack_timer(chan);
@@ -1007,6 +1010,36 @@ static void l2cap_move_setup(struct l2cap_chan *chan)
 	set_bit(CONN_REMOTE_BUSY, &chan->conn_state);
 }
 
+static void l2cap_move_success(struct l2cap_chan *chan)
+{
+	BT_DBG("chan %p", chan);
+
+	if (chan->mode != L2CAP_MODE_ERTM)
+		return;
+
+	if (chan->move_role == L2CAP_MOVE_ROLE_INITIATOR) {
+		l2cap_tx(chan, NULL, NULL, L2CAP_EV_EXPLICIT_POLL);
+		chan->rx_state = L2CAP_RX_STATE_WAIT_F;
+	} else if (chan->move_role == L2CAP_MOVE_ROLE_RESPONDER) {
+		chan->rx_state = L2CAP_RX_STATE_WAIT_P;
+	}
+}
+
+static void l2cap_move_revert(struct l2cap_chan *chan)
+{
+	BT_DBG("chan %p", chan);
+
+	if (chan->mode != L2CAP_MODE_ERTM)
+		return;
+
+	if (chan->move_role == L2CAP_MOVE_ROLE_INITIATOR) {
+		l2cap_tx(chan, NULL, NULL, L2CAP_EV_EXPLICIT_POLL);
+		chan->rx_state = L2CAP_RX_STATE_WAIT_F;
+	} else if (chan->move_role == L2CAP_MOVE_ROLE_RESPONDER) {
+		chan->rx_state = L2CAP_RX_STATE_WAIT_P;
+	}
+}
+
 static void l2cap_chan_ready(struct l2cap_chan *chan)
 {
 	/* This clears all conf flags, including CONF_NOT_COMPLETE */
@@ -4239,11 +4272,12 @@ static inline int l2cap_move_channel_rsp(struct l2cap_conn *conn,
 	return 0;
 }
 
-static inline int l2cap_move_channel_confirm(struct l2cap_conn *conn,
-					     struct l2cap_cmd_hdr *cmd,
-					     u16 cmd_len, void *data)
+static int l2cap_move_channel_confirm(struct l2cap_conn *conn,
+				      struct l2cap_cmd_hdr *cmd,
+				      u16 cmd_len, void *data)
 {
 	struct l2cap_move_chan_cfm *cfm = data;
+	struct l2cap_chan *chan;
 	u16 icid, result;
 
 	if (cmd_len != sizeof(*cfm))
@@ -4254,8 +4288,39 @@ static inline int l2cap_move_channel_confirm(struct l2cap_conn *conn,
 
 	BT_DBG("icid 0x%4.4x, result 0x%4.4x", icid, result);
 
+	chan = l2cap_get_chan_by_dcid(conn, icid);
+
+	if (!chan)
+		goto send_move_confirm_response;
+
+	if (chan->move_state == L2CAP_MOVE_WAIT_CONFIRM) {
+		chan->move_state = L2CAP_MOVE_STABLE;
+		if (result == L2CAP_MC_CONFIRMED) {
+			chan->chan_id = chan->move_id;
+			if (!chan->chan_id) {
+				/* Have moved off of AMP, free the channel */
+				chan->hs_hchan = NULL;
+				chan->hs_hcon = NULL;
+
+				/* Placeholder - free the logical link */
+			}
+			l2cap_move_success(chan);
+		} else {
+			chan->move_id = chan->chan_id;
+			l2cap_move_revert(chan);
+		}
+		chan->move_role = L2CAP_MOVE_ROLE_NONE;
+	} else if (chan->move_state == L2CAP_MOVE_WAIT_LOGICAL_CFM) {
+		BT_DBG("Bad MOVE_STATE (%d)", chan->move_state);
+	}
+
+
+send_move_confirm_response:
 	l2cap_send_move_chan_cfm_rsp(conn, cmd->ident, icid);
 
+	if (chan)
+		l2cap_chan_unlock(chan);
+
 	return 0;
 }
 
-- 
1.7.11.2

--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

Re: [RFCv0 08/21] Bluetooth: Add move channel confirm handling

[Andrei Emeltchenko]

Hi Mat,

On Wed, Jul 25, 2012 at 04:51:00PM -0700, Mat Martineau wrote:
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
>  net/bluetooth/l2cap_core.c | 71 ++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 68 insertions(+), 3 deletions(-)
> 
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index a327f02..b91ad10 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -981,6 +981,9 @@ static void l2cap_move_setup(struct l2cap_chan *chan)
>  
>  	BT_DBG("chan %p", chan);
>  
> +	if (chan->mode != L2CAP_MODE_ERTM)
> +		return;
> +

The chunk above might be added to move_setup patch

>  	__clear_retrans_timer(chan);
>  	__clear_monitor_timer(chan);
>  	__clear_ack_timer(chan);
> @@ -1007,6 +1010,36 @@ static void l2cap_move_setup(struct l2cap_chan *chan)
>  	set_bit(CONN_REMOTE_BUSY, &chan->conn_state);
>  }
>  
> +static void l2cap_move_success(struct l2cap_chan *chan)
> +{
> +	BT_DBG("chan %p", chan);
> +
> +	if (chan->mode != L2CAP_MODE_ERTM)
> +		return;
> +
> +	if (chan->move_role == L2CAP_MOVE_ROLE_INITIATOR) {
> +		l2cap_tx(chan, NULL, NULL, L2CAP_EV_EXPLICIT_POLL);
> +		chan->rx_state = L2CAP_RX_STATE_WAIT_F;
> +	} else if (chan->move_role == L2CAP_MOVE_ROLE_RESPONDER) {

I think switch might work better here.

> +		chan->rx_state = L2CAP_RX_STATE_WAIT_P;
> +	}
> +}
> +
> +static void l2cap_move_revert(struct l2cap_chan *chan)
> +{
> +	BT_DBG("chan %p", chan);
> +
> +	if (chan->mode != L2CAP_MODE_ERTM)
> +		return;
> +
> +	if (chan->move_role == L2CAP_MOVE_ROLE_INITIATOR) {
> +		l2cap_tx(chan, NULL, NULL, L2CAP_EV_EXPLICIT_POLL);
> +		chan->rx_state = L2CAP_RX_STATE_WAIT_F;
> +	} else if (chan->move_role == L2CAP_MOVE_ROLE_RESPONDER) {

and here.

> +		chan->rx_state = L2CAP_RX_STATE_WAIT_P;
> +	}
> +}
> +
>  static void l2cap_chan_ready(struct l2cap_chan *chan)
>  {
>  	/* This clears all conf flags, including CONF_NOT_COMPLETE */
> @@ -4239,11 +4272,12 @@ static inline int l2cap_move_channel_rsp(struct l2cap_conn *conn,
>  	return 0;
>  }
>  
> -static inline int l2cap_move_channel_confirm(struct l2cap_conn *conn,
> -					     struct l2cap_cmd_hdr *cmd,
> -					     u16 cmd_len, void *data)
> +static int l2cap_move_channel_confirm(struct l2cap_conn *conn,
> +				      struct l2cap_cmd_hdr *cmd,
> +				      u16 cmd_len, void *data)
>  {
>  	struct l2cap_move_chan_cfm *cfm = data;
> +	struct l2cap_chan *chan;
>  	u16 icid, result;
>  
>  	if (cmd_len != sizeof(*cfm))
> @@ -4254,8 +4288,39 @@ static inline int l2cap_move_channel_confirm(struct l2cap_conn *conn,
>  
>  	BT_DBG("icid 0x%4.4x, result 0x%4.4x", icid, result);
>  
> +	chan = l2cap_get_chan_by_dcid(conn, icid);
> +
> +	if (!chan)
> +		goto send_move_confirm_response;
> +
> +	if (chan->move_state == L2CAP_MOVE_WAIT_CONFIRM) {
> +		chan->move_state = L2CAP_MOVE_STABLE;
> +		if (result == L2CAP_MC_CONFIRMED) {
> +			chan->chan_id = chan->move_id;
> +			if (!chan->chan_id) {
> +				/* Have moved off of AMP, free the channel */
> +				chan->hs_hchan = NULL;
> +				chan->hs_hcon = NULL;
> +
> +				/* Placeholder - free the logical link */
> +			}
> +			l2cap_move_success(chan);
> +		} else {
> +			chan->move_id = chan->chan_id;
> +			l2cap_move_revert(chan);
> +		}
> +		chan->move_role = L2CAP_MOVE_ROLE_NONE;
> +	} else if (chan->move_state == L2CAP_MOVE_WAIT_LOGICAL_CFM) {

and maybe here.

> +		BT_DBG("Bad MOVE_STATE (%d)", chan->move_state);
> +	}
> +
> +
> +send_move_confirm_response:
>  	l2cap_send_move_chan_cfm_rsp(conn, cmd->ident, icid);
>  
> +	if (chan)
> +		l2cap_chan_unlock(chan);
> +
>  	return 0;

BTW: Is exit code used somewhere?

Best regards 
Andrei Emeltchenko 

Re: [RFCv0 08/21] Bluetooth: Add move channel confirm handling

[Mat Martineau]

Hi Andrei -

On Thu, 26 Jul 2012, Andrei Emeltchenko wrote:

> Hi Mat,
>
> On Wed, Jul 25, 2012 at 04:51:00PM -0700, Mat Martineau wrote:
>> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
>> ---
>>  net/bluetooth/l2cap_core.c | 71 ++++++++++++++++++++++++++++++++++++++++++++--
>>  1 file changed, 68 insertions(+), 3 deletions(-)
>>
>> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
>> index a327f02..b91ad10 100644
>> --- a/net/bluetooth/l2cap_core.c
>> +++ b/net/bluetooth/l2cap_core.c
>> @@ -981,6 +981,9 @@ static void l2cap_move_setup(struct l2cap_chan *chan)
>>
>>  	BT_DBG("chan %p", chan);
>>
>> +	if (chan->mode != L2CAP_MODE_ERTM)
>> +		return;
>> +
>
> The chunk above might be added to move_setup patch
>

Good idea.

>>  	__clear_retrans_timer(chan);
>>  	__clear_monitor_timer(chan);
>>  	__clear_ack_timer(chan);
>> @@ -1007,6 +1010,36 @@ static void l2cap_move_setup(struct l2cap_chan *chan)
>>  	set_bit(CONN_REMOTE_BUSY, &chan->conn_state);
>>  }
>>
>> +static void l2cap_move_success(struct l2cap_chan *chan)
>> +{
>> +	BT_DBG("chan %p", chan);
>> +
>> +	if (chan->mode != L2CAP_MODE_ERTM)
>> +		return;
>> +
>> +	if (chan->move_role == L2CAP_MOVE_ROLE_INITIATOR) {
>> +		l2cap_tx(chan, NULL, NULL, L2CAP_EV_EXPLICIT_POLL);
>> +		chan->rx_state = L2CAP_RX_STATE_WAIT_F;
>> +	} else if (chan->move_role == L2CAP_MOVE_ROLE_RESPONDER) {
>
> I think switch might work better here.
>

Ok.

>> +		chan->rx_state = L2CAP_RX_STATE_WAIT_P;
>> +	}
>> +}
>> +
>> +static void l2cap_move_revert(struct l2cap_chan *chan)
>> +{
>> +	BT_DBG("chan %p", chan);
>> +
>> +	if (chan->mode != L2CAP_MODE_ERTM)
>> +		return;
>> +
>> +	if (chan->move_role == L2CAP_MOVE_ROLE_INITIATOR) {
>> +		l2cap_tx(chan, NULL, NULL, L2CAP_EV_EXPLICIT_POLL);
>> +		chan->rx_state = L2CAP_RX_STATE_WAIT_F;
>> +	} else if (chan->move_role == L2CAP_MOVE_ROLE_RESPONDER) {
>
> and here.
>

Ok.

>> +		chan->rx_state = L2CAP_RX_STATE_WAIT_P;
>> +	}
>> +}
>> +
>>  static void l2cap_chan_ready(struct l2cap_chan *chan)
>>  {
>>  	/* This clears all conf flags, including CONF_NOT_COMPLETE */
>> @@ -4239,11 +4272,12 @@ static inline int l2cap_move_channel_rsp(struct l2cap_conn *conn,
>>  	return 0;
>>  }
>>
>> -static inline int l2cap_move_channel_confirm(struct l2cap_conn *conn,
>> -					     struct l2cap_cmd_hdr *cmd,
>> -					     u16 cmd_len, void *data)
>> +static int l2cap_move_channel_confirm(struct l2cap_conn *conn,
>> +				      struct l2cap_cmd_hdr *cmd,
>> +				      u16 cmd_len, void *data)
>>  {
>>  	struct l2cap_move_chan_cfm *cfm = data;
>> +	struct l2cap_chan *chan;
>>  	u16 icid, result;
>>
>>  	if (cmd_len != sizeof(*cfm))
>> @@ -4254,8 +4288,39 @@ static inline int l2cap_move_channel_confirm(struct l2cap_conn *conn,
>>
>>  	BT_DBG("icid 0x%4.4x, result 0x%4.4x", icid, result);
>>
>> +	chan = l2cap_get_chan_by_dcid(conn, icid);
>> +
>> +	if (!chan)
>> +		goto send_move_confirm_response;
>> +
>> +	if (chan->move_state == L2CAP_MOVE_WAIT_CONFIRM) {
>> +		chan->move_state = L2CAP_MOVE_STABLE;
>> +		if (result == L2CAP_MC_CONFIRMED) {
>> +			chan->chan_id = chan->move_id;
>> +			if (!chan->chan_id) {
>> +				/* Have moved off of AMP, free the channel */
>> +				chan->hs_hchan = NULL;
>> +				chan->hs_hcon = NULL;
>> +
>> +				/* Placeholder - free the logical link */
>> +			}
>> +			l2cap_move_success(chan);
>> +		} else {
>> +			chan->move_id = chan->chan_id;
>> +			l2cap_move_revert(chan);
>> +		}
>> +		chan->move_role = L2CAP_MOVE_ROLE_NONE;
>> +	} else if (chan->move_state == L2CAP_MOVE_WAIT_LOGICAL_CFM) {
>
> and maybe here.
>

I'll just remove the second clause.

>> +		BT_DBG("Bad MOVE_STATE (%d)", chan->move_state);
>> +	}
>> +
>> +
>> +send_move_confirm_response:
>>  	l2cap_send_move_chan_cfm_rsp(conn, cmd->ident, icid);
>>
>> +	if (chan)
>> +		l2cap_chan_unlock(chan);
>> +
>>  	return 0;
>
> BTW: Is exit code used somewhere?

All of the signal handling functions called by 
l2cap_bredr_sig_cmd (except l2cap_command_rej) return int, and many 
return 0, so I was sticking with the convention there.

--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

[RFCv0 09/21] Bluetooth: Move channel response

[Mat Martineau]

Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
 include/net/bluetooth/l2cap.h |   2 +
 net/bluetooth/l2cap_core.c    | 138 +++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 138 insertions(+), 2 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index c585def..fcc971a 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -51,6 +51,8 @@
 #define L2CAP_ENC_TIMEOUT		msecs_to_jiffies(5000)
 #define L2CAP_CONN_TIMEOUT		msecs_to_jiffies(40000)
 #define L2CAP_INFO_TIMEOUT		msecs_to_jiffies(4000)
+#define L2CAP_MOVE_TIMEOUT		msecs_to_jiffies(4000)
+#define L2CAP_MOVE_ERTX_TIMEOUT		msecs_to_jiffies(60000)
 
 #define L2CAP_A2MP_DEFAULT_MTU		670
 
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index b91ad10..8d72f2e 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -4161,6 +4161,13 @@ static void l2cap_send_move_chan_cfm_rsp(struct l2cap_conn *conn, u8 ident,
 	l2cap_send_cmd(conn, ident, L2CAP_MOVE_CHAN_CFM_RSP, sizeof(rsp), &rsp);
 }
 
+static void l2cap_logical_cfm(struct l2cap_chan *chan, struct hci_chan *hchan,
+			      u8 status)
+{
+	/* Placeholder */
+	return;
+}
+
 static inline int l2cap_move_channel_req(struct l2cap_conn *conn,
 					 struct l2cap_cmd_hdr *cmd,
 					 u16 cmd_len, void *data)
@@ -4256,6 +4263,7 @@ static inline int l2cap_move_channel_rsp(struct l2cap_conn *conn,
 					 u16 cmd_len, void *data)
 {
 	struct l2cap_move_chan_rsp *rsp = data;
+	struct l2cap_chan *chan = NULL;
 	u16 icid, result;
 
 	if (cmd_len != sizeof(*rsp))
@@ -4266,8 +4274,134 @@ static inline int l2cap_move_channel_rsp(struct l2cap_conn *conn,
 
 	BT_DBG("icid 0x%4.4x, result 0x%4.4x", icid, result);
 
-	/* Placeholder: Always unconfirmed */
-	l2cap_send_move_chan_cfm(conn, NULL, icid, L2CAP_MC_UNCONFIRMED);
+	switch (result) {
+	case L2CAP_MR_SUCCESS:
+	case L2CAP_MR_PEND:
+		chan = l2cap_get_chan_by_scid(conn, icid);
+
+		if (!chan) {
+			l2cap_send_move_chan_cfm(conn, NULL, icid,
+						 L2CAP_MC_UNCONFIRMED);
+			break;
+		}
+
+		__clear_chan_timer(chan);
+		if (result == L2CAP_MR_PEND)
+			__set_chan_timer(chan, L2CAP_MOVE_ERTX_TIMEOUT);
+
+		if (chan->move_state == L2CAP_MOVE_WAIT_LOGICAL_COMP) {
+			/* Move confirm will be sent when logical link
+			 * is complete.
+			 */
+			chan->move_state = L2CAP_MOVE_WAIT_LOGICAL_CFM;
+		} else if (chan->move_state == L2CAP_MOVE_WAIT_RSP_SUCCESS) {
+			if (result == L2CAP_MR_PEND) {
+				break;
+			} else if (test_bit(CONN_LOCAL_BUSY,
+					    &chan->conn_state)) {
+				chan->move_state = L2CAP_MOVE_WAIT_LOCAL_BUSY;
+			} else {
+				/* Logical link is up or moving to BR/EDR,
+				 * proceed with move */
+				chan->move_state = L2CAP_MOVE_WAIT_CONFIRM_RSP;
+				l2cap_send_move_chan_cfm(conn, chan, chan->scid,
+							 L2CAP_MC_CONFIRMED);
+				__set_chan_timer(chan, L2CAP_MOVE_TIMEOUT);
+			}
+		} else if (chan->move_state == L2CAP_MOVE_WAIT_RSP) {
+			struct hci_chan *hchan = NULL;
+			/* Moving to AMP */
+			if (result == L2CAP_MR_SUCCESS) {
+				/* Remote is ready, send confirm immediately
+				 * after logical link is ready
+				 */
+				chan->move_state = L2CAP_MOVE_WAIT_LOGICAL_CFM;
+			} else {
+				/* Both logical link and move success
+				 * are required to confirm
+				 */
+				chan->move_state = L2CAP_MOVE_WAIT_LOGICAL_COMP;
+			}
+
+			/* Placeholder - get hci_chan for logical link */
+			if (!hchan) {
+				/* Logical link not available */
+				l2cap_send_move_chan_cfm(conn, chan, chan->scid,
+							 L2CAP_MC_UNCONFIRMED);
+				break;
+			}
+
+			/* If the logical link is not yet connected, do not
+			 * send confirmation.
+			 */
+			if (hchan->state != BT_CONNECTED)
+				break;
+
+			/* Logical link is already ready to go */
+
+			chan->hs_hcon = hchan->conn;
+			chan->hs_hcon->l2cap_data = chan->conn;
+
+			if (result == L2CAP_MR_SUCCESS) {
+				/* Can confirm now */
+				l2cap_send_move_chan_cfm(conn, chan, chan->scid,
+							 L2CAP_MC_CONFIRMED);
+			} else {
+				/* Now only need move success
+				 * to confirm
+				 */
+				chan->move_state = L2CAP_MOVE_WAIT_RSP_SUCCESS;
+			}
+
+			l2cap_logical_cfm(chan, hchan, L2CAP_MR_SUCCESS);
+		} else {
+			/* Any other amp move state means the move failed. */
+			chan->move_id = chan->chan_id;
+			chan->move_state = L2CAP_MOVE_STABLE;
+			l2cap_move_revert(chan);
+			chan->move_role = L2CAP_MOVE_ROLE_NONE;
+			l2cap_send_move_chan_cfm(conn, chan, chan->scid,
+						L2CAP_MC_UNCONFIRMED);
+			__set_chan_timer(chan, L2CAP_MOVE_TIMEOUT);
+		}
+		break;
+	default:
+		/* Failed (including collision case) */
+		mutex_lock(&conn->chan_lock);
+		chan = __l2cap_get_chan_by_ident(conn, cmd->ident);
+		if (chan)
+			l2cap_chan_lock(chan);
+		mutex_unlock(&conn->chan_lock);
+
+		if (!chan) {
+			/* Could not locate channel, icid is best guess */
+			l2cap_send_move_chan_cfm(conn, NULL, icid,
+						 L2CAP_MC_UNCONFIRMED);
+			break;
+		}
+
+		__clear_chan_timer(chan);
+
+		if (chan->move_role == L2CAP_MOVE_ROLE_INITIATOR) {
+			if (result == L2CAP_MR_COLLISION) {
+				chan->move_role = L2CAP_MOVE_ROLE_RESPONDER;
+			} else {
+				/* Cleanup - cancel move */
+				chan->move_id = chan->chan_id;
+				chan->move_state = L2CAP_MOVE_STABLE;
+				l2cap_move_revert(chan);
+				chan->move_role = L2CAP_MOVE_ROLE_NONE;
+			}
+		}
+
+		l2cap_send_move_chan_cfm(conn, chan, chan->scid,
+					 L2CAP_MC_UNCONFIRMED);
+		__set_chan_timer(chan, L2CAP_MOVE_TIMEOUT);
+		break;
+	}
+
+	if (chan)
+		l2cap_chan_unlock(chan);
 
 	return 0;
 }
-- 
1.7.11.2

--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

[RFCv0 10/21] Bluetooth: Add logical link confirm

[Mat Martineau]

Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
 include/net/bluetooth/l2cap.h |   1 +
 net/bluetooth/l2cap_core.c    | 120 +++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 119 insertions(+), 2 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index fcc971a..7c50606 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -461,6 +461,7 @@ struct l2cap_chan {
 
 	__u8		conf_req[64];
 	__u8		conf_len;
+	__u8		conf_ident;
 	__u8		num_conf_req;
 	__u8		num_conf_rsp;
 
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 8d72f2e..79f9d8e 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -3717,6 +3717,7 @@ static inline int l2cap_config_req(struct l2cap_conn *conn, struct l2cap_cmd_hdr
 		goto unlock;
 	}
 
+	chan->conf_ident = cmd->ident;
 	l2cap_send_cmd(conn, cmd->ident, L2CAP_CONF_RSP, len, rsp);
 	chan->num_conf_rsp++;
 
@@ -4161,11 +4162,126 @@ static void l2cap_send_move_chan_cfm_rsp(struct l2cap_conn *conn, u8 ident,
 	l2cap_send_cmd(conn, ident, L2CAP_MOVE_CHAN_CFM_RSP, sizeof(rsp), &rsp);
 }
 
+/* Call with chan locked */
 static void l2cap_logical_cfm(struct l2cap_chan *chan, struct hci_chan *hchan,
 			      u8 status)
 {
-	/* Placeholder */
-	return;
+	BT_DBG("chan %p, hchan %p, status %d", chan, hchan, status);
+
+	if (chan->state != BT_CONNECTED && !chan->chan_id)
+		return;
+
+	if (chan && !status && chan->state != BT_CONNECTED) {
+		struct l2cap_conf_rsp rsp;
+		u8 code;
+
+		/* Create channel complete */
+		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->conf_ident,
+			       L2CAP_CONF_RSP, code, &rsp);
+		set_bit(CONF_OUTPUT_DONE, &chan->conf_state);
+
+		if (test_bit(CONF_INPUT_DONE, &chan->conf_state)) {
+			int err = 0;
+
+			set_default_fcs(chan);
+
+			if (chan->mode == L2CAP_MODE_ERTM ||
+			    chan->mode == L2CAP_MODE_STREAMING)
+				err = l2cap_ertm_init(chan);
+
+			if (err < 0)
+				l2cap_send_disconn_req(chan->conn, chan, -err);
+			else
+				l2cap_chan_ready(chan);
+		}
+	} else if (chan && !status) {
+		/* Channel move */
+		chan->hs_hcon = hchan->conn;
+		chan->hs_hcon->l2cap_data = chan->conn;
+
+		BT_DBG("move_state %d", chan->move_state);
+
+		if (chan->state != BT_CONNECTED) {
+			return;
+		}
+
+		switch (chan->move_state) {
+		case L2CAP_MOVE_WAIT_LOGICAL_COMP:
+			/* Move confirm will be sent after a success
+			 * response is received
+			 */
+			chan->move_state = L2CAP_MOVE_WAIT_RSP_SUCCESS;
+			break;
+		case L2CAP_MOVE_WAIT_LOGICAL_CFM:
+			if (test_bit(CONN_LOCAL_BUSY, &chan->conn_state)) {
+				chan->move_state = L2CAP_MOVE_WAIT_LOCAL_BUSY;
+			} else if (chan->move_role ==
+				   L2CAP_MOVE_ROLE_INITIATOR) {
+				chan->move_state = L2CAP_MOVE_WAIT_CONFIRM_RSP;
+				l2cap_send_move_chan_cfm(chan->conn, chan,
+							 chan->scid,
+							 L2CAP_MR_SUCCESS);
+				__set_chan_timer(chan, L2CAP_MOVE_TIMEOUT);
+			} else if (chan->move_role ==
+				   L2CAP_MOVE_ROLE_RESPONDER) {
+				chan->move_state = L2CAP_MOVE_WAIT_CONFIRM;
+				l2cap_send_move_chan_rsp(chan->conn,
+							 chan->move_cmd_ident,
+							 chan->dcid,
+							 L2CAP_MR_SUCCESS);
+			}
+			break;
+		default:
+			/* Move was not in expected state, free the channel */
+			chan->hs_hchan = NULL;
+			chan->hs_hcon = NULL;
+
+			/* Placeholder - free the logical link */
+
+			chan->move_state = L2CAP_MOVE_STABLE;
+		}
+	} else {
+		/* Logical link setup failed. */
+
+		if (chan->state != BT_CONNECTED)
+			l2cap_send_disconn_req(chan->conn, chan, ECONNRESET);
+		else if (chan->move_role == L2CAP_MOVE_ROLE_RESPONDER) {
+			l2cap_move_revert(chan);
+			chan->move_role = L2CAP_MOVE_ROLE_NONE;
+			chan->move_state = L2CAP_MOVE_STABLE;
+			l2cap_send_move_chan_rsp(chan->conn,
+						 chan->move_cmd_ident,
+						 chan->dcid,
+						 L2CAP_MR_NOT_SUPP);
+		} else if (chan->move_role == L2CAP_MOVE_ROLE_INITIATOR) {
+			if (chan->move_state == L2CAP_MOVE_WAIT_LOGICAL_COMP ||
+			    chan->move_state == L2CAP_MOVE_WAIT_LOGICAL_CFM) {
+				/* Remote has only sent pending or
+				 * success responses, clean up
+				 */
+				l2cap_move_revert(chan);
+				chan->move_role = L2CAP_MOVE_ROLE_NONE;
+				chan->move_state = L2CAP_MOVE_STABLE;
+			}
+
+			/* Other amp move states imply that the move
+			 * has already aborted
+			 */
+			l2cap_send_move_chan_cfm(chan->conn, chan, chan->scid,
+						 L2CAP_MC_UNCONFIRMED);
+			__set_chan_timer(chan, L2CAP_MOVE_TIMEOUT);
+		}
+
+		chan->hs_hchan = NULL;
+		chan->hs_hcon = NULL;
+
+		/* Placeholder - free logical link */
+	}
 }
 
 static inline int l2cap_move_channel_req(struct l2cap_conn *conn,
-- 
1.7.11.2

--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

Re: [RFCv0 10/21] Bluetooth: Add logical link confirm

[Andrei Emeltchenko]

Hi Mat,

On Wed, Jul 25, 2012 at 04:51:02PM -0700, Mat Martineau wrote:
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
>  include/net/bluetooth/l2cap.h |   1 +
>  net/bluetooth/l2cap_core.c    | 120 +++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 119 insertions(+), 2 deletions(-)
> 
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index fcc971a..7c50606 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -461,6 +461,7 @@ struct l2cap_chan {
>  
>  	__u8		conf_req[64];
>  	__u8		conf_len;
> +	__u8		conf_ident;
>  	__u8		num_conf_req;
>  	__u8		num_conf_rsp;
>  
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 8d72f2e..79f9d8e 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -3717,6 +3717,7 @@ static inline int l2cap_config_req(struct l2cap_conn *conn, struct l2cap_cmd_hdr
>  		goto unlock;
>  	}
>  
> +	chan->conf_ident = cmd->ident;
>  	l2cap_send_cmd(conn, cmd->ident, L2CAP_CONF_RSP, len, rsp);
>  	chan->num_conf_rsp++;
>  
> @@ -4161,11 +4162,126 @@ static void l2cap_send_move_chan_cfm_rsp(struct l2cap_conn *conn, u8 ident,
>  	l2cap_send_cmd(conn, ident, L2CAP_MOVE_CHAN_CFM_RSP, sizeof(rsp), &rsp);
>  }
>  
> +/* Call with chan locked */
>  static void l2cap_logical_cfm(struct l2cap_chan *chan, struct hci_chan *hchan,
>  			      u8 status)
>  {
> -	/* Placeholder */
> -	return;
> +	BT_DBG("chan %p, hchan %p, status %d", chan, hchan, status);
> +
> +	if (chan->state != BT_CONNECTED && !chan->chan_id)
> +		return;
> +
> +	if (chan && !status && chan->state != BT_CONNECTED) {

If we consider chan might be NULL we would crash in the previous reference
of chan->state.

> +		struct l2cap_conf_rsp rsp;
> +		u8 code;
> +
> +		/* Create channel complete */
> +		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->conf_ident,
> +			       L2CAP_CONF_RSP, code, &rsp);
> +		set_bit(CONF_OUTPUT_DONE, &chan->conf_state);
> +
> +		if (test_bit(CONF_INPUT_DONE, &chan->conf_state)) {
> +			int err = 0;
> +
> +			set_default_fcs(chan);
> +
> +			if (chan->mode == L2CAP_MODE_ERTM ||
> +			    chan->mode == L2CAP_MODE_STREAMING)
> +				err = l2cap_ertm_init(chan);
> +
> +			if (err < 0)
> +				l2cap_send_disconn_req(chan->conn, chan, -err);
> +			else
> +				l2cap_chan_ready(chan);
> +		}
> +	} else if (chan && !status) {

Otherwise we always reference chan so we might check for chan in the
beginning.

> +		/* Channel move */
> +		chan->hs_hcon = hchan->conn;
> +		chan->hs_hcon->l2cap_data = chan->conn;
> +
> +		BT_DBG("move_state %d", chan->move_state);

in the future patches I would prefer something similar to state_to_string.

...

Best regards 
Andrei Emeltchenko 

Re: [RFCv0 10/21] Bluetooth: Add logical link confirm

[Mat Martineau]

Hi Andrei -

On Thu, 26 Jul 2012, Andrei Emeltchenko wrote:

> Hi Mat,
>
> On Wed, Jul 25, 2012 at 04:51:02PM -0700, Mat Martineau wrote:
>> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
>> ---
>>  include/net/bluetooth/l2cap.h |   1 +
>>  net/bluetooth/l2cap_core.c    | 120 +++++++++++++++++++++++++++++++++++++++++-
>>  2 files changed, 119 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
>> index fcc971a..7c50606 100644
>> --- a/include/net/bluetooth/l2cap.h
>> +++ b/include/net/bluetooth/l2cap.h
>> @@ -461,6 +461,7 @@ struct l2cap_chan {
>>
>>  	__u8		conf_req[64];
>>  	__u8		conf_len;
>> +	__u8		conf_ident;
>>  	__u8		num_conf_req;
>>  	__u8		num_conf_rsp;
>>
>> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
>> index 8d72f2e..79f9d8e 100644
>> --- a/net/bluetooth/l2cap_core.c
>> +++ b/net/bluetooth/l2cap_core.c
>> @@ -3717,6 +3717,7 @@ static inline int l2cap_config_req(struct l2cap_conn *conn, struct l2cap_cmd_hdr
>>  		goto unlock;
>>  	}
>>
>> +	chan->conf_ident = cmd->ident;
>>  	l2cap_send_cmd(conn, cmd->ident, L2CAP_CONF_RSP, len, rsp);
>>  	chan->num_conf_rsp++;
>>
>> @@ -4161,11 +4162,126 @@ static void l2cap_send_move_chan_cfm_rsp(struct l2cap_conn *conn, u8 ident,
>>  	l2cap_send_cmd(conn, ident, L2CAP_MOVE_CHAN_CFM_RSP, sizeof(rsp), &rsp);
>>  }
>>
>> +/* Call with chan locked */
>>  static void l2cap_logical_cfm(struct l2cap_chan *chan, struct hci_chan *hchan,
>>  			      u8 status)
>>  {
>> -	/* Placeholder */
>> -	return;
>> +	BT_DBG("chan %p, hchan %p, status %d", chan, hchan, status);
>> +
>> +	if (chan->state != BT_CONNECTED && !chan->chan_id)
>> +		return;
>> +
>> +	if (chan && !status && chan->state != BT_CONNECTED) {
>
> If we consider chan might be NULL we would crash in the previous reference
> of chan->state.
>

I'll fix that.

>> +		struct l2cap_conf_rsp rsp;
>> +		u8 code;
>> +
>> +		/* Create channel complete */
>> +		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->conf_ident,
>> +			       L2CAP_CONF_RSP, code, &rsp);
>> +		set_bit(CONF_OUTPUT_DONE, &chan->conf_state);
>> +
>> +		if (test_bit(CONF_INPUT_DONE, &chan->conf_state)) {
>> +			int err = 0;
>> +
>> +			set_default_fcs(chan);
>> +
>> +			if (chan->mode == L2CAP_MODE_ERTM ||
>> +			    chan->mode == L2CAP_MODE_STREAMING)
>> +				err = l2cap_ertm_init(chan);
>> +
>> +			if (err < 0)
>> +				l2cap_send_disconn_req(chan->conn, chan, -err);
>> +			else
>> +				l2cap_chan_ready(chan);
>> +		}
>> +	} else if (chan && !status) {
>
> Otherwise we always reference chan so we might check for chan in the
> beginning.
>
>> +		/* Channel move */
>> +		chan->hs_hcon = hchan->conn;
>> +		chan->hs_hcon->l2cap_data = chan->conn;
>> +
>> +		BT_DBG("move_state %d", chan->move_state);
>
> in the future patches I would prefer something similar to state_to_string.
>
> ...

Future patches that print move_state?  (This is now the only place 
that prints it)

--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

[RFCv0 11/21] Bluetooth: Add move confirm response handling

[Mat Martineau]

Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
 net/bluetooth/l2cap_core.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 79f9d8e..bc56f09 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -4579,6 +4579,7 @@ static inline int l2cap_move_channel_confirm_rsp(struct l2cap_conn *conn,
 						 u16 cmd_len, void *data)
 {
 	struct l2cap_move_chan_cfm_rsp *rsp = data;
+	struct l2cap_chan *chan;
 	u16 icid;
 
 	if (cmd_len != sizeof(*rsp))
@@ -4588,6 +4589,32 @@ static inline int l2cap_move_channel_confirm_rsp(struct l2cap_conn *conn,
 
 	BT_DBG("icid 0x%4.4x", icid);
 
+	chan = l2cap_get_chan_by_scid(conn, icid);
+
+	if (!chan)
+		return 0;
+
+	__clear_chan_timer(chan);
+
+	if (chan->move_state == L2CAP_MOVE_WAIT_CONFIRM_RSP) {
+		chan->move_state = L2CAP_MOVE_STABLE;
+		chan->chan_id = chan->move_id;
+
+		if (!chan->chan_id && chan->hs_hchan) {
+			/* Have moved off of AMP, free the channel */
+			chan->hs_hchan = NULL;
+			chan->hs_hcon = NULL;
+
+			/* Placeholder - free the logical link */
+		}
+
+		l2cap_move_success(chan);
+
+		chan->move_role = L2CAP_MOVE_ROLE_NONE;
+	}
+
+	l2cap_chan_unlock(chan);
+
 	return 0;
 }
 
-- 
1.7.11.2

--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

Re: [RFCv0 11/21] Bluetooth: Add move confirm response handling

[Andrei Emeltchenko]

Hi Mat,

On Wed, Jul 25, 2012 at 04:51:03PM -0700, Mat Martineau wrote:
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
>  net/bluetooth/l2cap_core.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 79f9d8e..bc56f09 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -4579,6 +4579,7 @@ static inline int l2cap_move_channel_confirm_rsp(struct l2cap_conn *conn,
>  						 u16 cmd_len, void *data)
>  {
>  	struct l2cap_move_chan_cfm_rsp *rsp = data;
> +	struct l2cap_chan *chan;
>  	u16 icid;
>  
>  	if (cmd_len != sizeof(*rsp))
> @@ -4588,6 +4589,32 @@ static inline int l2cap_move_channel_confirm_rsp(struct l2cap_conn *conn,
>  
>  	BT_DBG("icid 0x%4.4x", icid);
>  
> +	chan = l2cap_get_chan_by_scid(conn, icid);
> +

nitpick: extra new line here.

> +	if (!chan)
> +		return 0;

are we going to use return code?

...

Best regards 
Andrei Emeltchenko 

Re: [RFCv0 11/21] Bluetooth: Add move confirm response handling

[Mat Martineau]

On Thu, 26 Jul 2012, Andrei Emeltchenko wrote:

> Hi Mat,
>
> On Wed, Jul 25, 2012 at 04:51:03PM -0700, Mat Martineau wrote:
>> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
>> ---
>>  net/bluetooth/l2cap_core.c | 27 +++++++++++++++++++++++++++
>>  1 file changed, 27 insertions(+)
>>
>> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
>> index 79f9d8e..bc56f09 100644
>> --- a/net/bluetooth/l2cap_core.c
>> +++ b/net/bluetooth/l2cap_core.c
>> @@ -4579,6 +4579,7 @@ static inline int l2cap_move_channel_confirm_rsp(struct l2cap_conn *conn,
>>  						 u16 cmd_len, void *data)
>>  {
>>  	struct l2cap_move_chan_cfm_rsp *rsp = data;
>> +	struct l2cap_chan *chan;
>>  	u16 icid;
>>
>>  	if (cmd_len != sizeof(*rsp))
>> @@ -4588,6 +4589,32 @@ static inline int l2cap_move_channel_confirm_rsp(struct l2cap_conn *conn,
>>
>>  	BT_DBG("icid 0x%4.4x", icid);
>>
>> +	chan = l2cap_get_chan_by_scid(conn, icid);
>> +
>
> nitpick: extra new line here.
>

Ok.  There are a couple of other similar cases that I will fix too.

>> +	if (!chan)
>> +		return 0;
>
> are we going to use return code?
>

Same deal as l2cap_move_channel_confirm: sticking with pattern 
established by other L2CAP signal functions.

--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

[RFCv0 12/21] Bluetooth: Add state to hci_chan

[Mat Martineau]

Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
 include/net/bluetooth/hci_core.h | 7 ++++---
 net/bluetooth/hci_conn.c         | 1 +
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 41d9439..4de510a 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -336,11 +336,12 @@ struct hci_conn {
 };
 
 struct hci_chan {
-	struct list_head list;
+	struct list_head	list;
 
-	struct hci_conn *conn;
-	struct sk_buff_head data_q;
+	struct hci_conn	*conn;
+	struct sk_buff_head	data_q;
 	unsigned int	sent;
+	__u8	state;
 };
 
 extern struct list_head hci_dev_list;
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 5ad7da2..1b796e5 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -903,6 +903,7 @@ struct hci_chan *hci_chan_create(struct hci_conn *conn)
 
 	chan->conn = conn;
 	skb_queue_head_init(&chan->data_q);
+	chan->state = BT_CONNECTED;
 
 	list_add_rcu(&chan->list, &conn->chan_list);
 
-- 
1.7.11.2

--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

Re: [RFCv0 12/21] Bluetooth: Add state to hci_chan

[Andrei Emeltchenko]

Hi Mat,

On Wed, Jul 25, 2012 at 04:51:04PM -0700, Mat Martineau wrote:
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
>  include/net/bluetooth/hci_core.h | 7 ++++---
>  net/bluetooth/hci_conn.c         | 1 +
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 41d9439..4de510a 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -336,11 +336,12 @@ struct hci_conn {
>  };
>  
>  struct hci_chan {
> -	struct list_head list;
> +	struct list_head	list;
>  
> -	struct hci_conn *conn;
> -	struct sk_buff_head data_q;
> +	struct hci_conn	*conn;
> +	struct sk_buff_head	data_q;

the real patch start here?

>  	unsigned int	sent;
> +	__u8	state;
>  };
>  
>  extern struct list_head hci_dev_list;
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index 5ad7da2..1b796e5 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -903,6 +903,7 @@ struct hci_chan *hci_chan_create(struct hci_conn *conn)
>  
>  	chan->conn = conn;
>  	skb_queue_head_init(&chan->data_q);
> +	chan->state = BT_CONNECTED;
>  
>  	list_add_rcu(&chan->list, &conn->chan_list);
>  

[RFCv0 13/21] Bluetooth: Indentation fixes

[Mat Martineau]

Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
 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 bc56f09..d981d87 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -4133,8 +4133,8 @@ static void l2cap_send_move_chan_rsp(struct l2cap_conn *conn, u8 ident,
 }
 
 static void l2cap_send_move_chan_cfm(struct l2cap_conn *conn,
-				     struct l2cap_chan *chan,
-				     u16 icid, u16 result)
+				     struct l2cap_chan *chan, u16 icid,
+				     u16 result)
 {
 	struct l2cap_move_chan_cfm cfm;
 	u8 ident;
-- 
1.7.11.2

--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

[RFCv0 14/21] Bluetooth: Handle physical link completion

[Mat Martineau]

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)
+		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);
+}
+
 static inline int l2cap_move_channel_req(struct l2cap_conn *conn,
 					 struct l2cap_cmd_hdr *cmd,
 					 u16 cmd_len, void *data)
-- 
1.7.11.2

--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

Re: [RFCv0 14/21] Bluetooth: Handle physical link completion

[Andrei Emeltchenko]

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.

> +		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?

Best regards 
Andrei Emeltchenko 

Re: [RFCv0 14/21] Bluetooth: Handle physical link completion

[Mat Martineau]

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

Re: [RFCv0 14/21] Bluetooth: Handle physical link completion

[Andrei Emeltchenko]

Hi Mat,

On Wed, Aug 15, 2012 at 09:56:04AM -0700, Mat Martineau wrote:
...
> >
> >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.

Maybe we could define new state BT_MOVING ?

Best regards 
Andrei Emeltchenko 

[RFCv0 15/21] Bluetooth: Enable data sends on AMP controller

[Mat Martineau]

Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
 include/net/bluetooth/hci.h |  1 +
 net/bluetooth/l2cap_core.c  | 14 ++++++++++++--
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 7f19556..125082e 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -196,6 +196,7 @@ enum {
 #define ACL_START_NO_FLUSH	0x00
 #define ACL_CONT		0x01
 #define ACL_START		0x02
+#define ACL_COMPLETE		0x03
 #define ACL_ACTIVE_BCAST	0x04
 #define ACL_PICO_BCAST		0x08
 
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 7e253ce..113ea8e 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -732,10 +732,20 @@ static void l2cap_do_send(struct l2cap_chan *chan, struct sk_buff *skb)
 	u16 flags;
 
 	BT_DBG("chan %p, skb %p len %d priority %u", chan, skb, skb->len,
-							skb->priority);
+	       skb->priority);
+
+	if (chan->hs_hcon && (chan->move_state == L2CAP_MOVE_STABLE ||
+			      chan->move_state == L2CAP_MOVE_WAIT_PREPARE)) {
+		if (chan->hs_hchan)
+			hci_send_acl(chan->hs_hchan, skb, ACL_COMPLETE);
+		else
+			kfree_skb(skb);
+
+		return;
+	}
 
 	if (!test_bit(FLAG_FLUSHABLE, &chan->flags) &&
-					lmp_no_flush_capable(hcon->hdev))
+	    lmp_no_flush_capable(hcon->hdev))
 		flags = ACL_START_NO_FLUSH;
 	else
 		flags = ACL_START;
-- 
1.7.11.2

--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

[RFCv0 16/21] Bluetooth: Do not send data during channel move

[Mat Martineau]

Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
 net/bluetooth/l2cap_core.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 113ea8e..17b5bee 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -919,6 +919,10 @@ static void l2cap_send_sframe(struct l2cap_chan *chan,
 	if (!control->sframe)
 		return;
 
+	if (chan->move_state != L2CAP_MOVE_STABLE &&
+	    chan->move_state != L2CAP_MOVE_WAIT_PREPARE)
+		return;
+
 	if (test_and_clear_bit(CONN_SEND_FBIT, &chan->conn_state) &&
 	    !control->poll)
 		control->final = 1;
@@ -1757,6 +1761,10 @@ static void l2cap_streaming_send(struct l2cap_chan *chan,
 
 	BT_DBG("chan %p, skbs %p", chan, skbs);
 
+	if (chan->move_state != L2CAP_MOVE_STABLE &&
+	    chan->move_state != L2CAP_MOVE_WAIT_PREPARE)
+		return;
+
 	skb_queue_splice_tail_init(skbs, &chan->tx_q);
 
 	while (!skb_queue_empty(&chan->tx_q)) {
@@ -1799,6 +1807,10 @@ static int l2cap_ertm_send(struct l2cap_chan *chan)
 	if (test_bit(CONN_REMOTE_BUSY, &chan->conn_state))
 		return 0;
 
+	if (chan->move_state != L2CAP_MOVE_STABLE &&
+	    chan->move_state != L2CAP_MOVE_WAIT_PREPARE)
+		return 0;
+
 	while (chan->tx_send_head &&
 	       chan->unacked_frames < chan->remote_tx_win &&
 	       chan->tx_state == L2CAP_TX_STATE_XMIT) {
@@ -1864,6 +1876,10 @@ static void l2cap_ertm_resend(struct l2cap_chan *chan)
 	if (test_bit(CONN_REMOTE_BUSY, &chan->conn_state))
 		return;
 
+	if (chan->move_state != L2CAP_MOVE_STABLE &&
+	    chan->move_state != L2CAP_MOVE_WAIT_PREPARE)
+		return;
+
 	while (chan->retrans_list.head != L2CAP_SEQ_LIST_CLEAR) {
 		seq = l2cap_seq_list_pop(&chan->retrans_list);
 
-- 
1.7.11.2

--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

[RFCv0 17/21] Bluetooth: Configure appropriate timeouts for AMP controllers

[Mat Martineau]

Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
 net/bluetooth/l2cap_core.c | 47 ++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 41 insertions(+), 6 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 17b5bee..f430744 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -2917,6 +2917,44 @@ static inline bool __l2cap_efs_supported(struct l2cap_chan *chan)
 	return enable_hs && chan->conn->feat_mask & L2CAP_FEAT_EXT_FLOW;
 }
 
+static void __l2cap_set_ertm_timeouts(struct l2cap_chan *chan,
+				      struct l2cap_conf_rfc *rfc)
+{
+	if (chan->chan_id && chan->hs_hcon) {
+		u64 ertm_to = chan->hs_hcon->hdev->amp_be_flush_to;
+
+		/* Class 1 devices have must have ERTM timeouts
+		 * exceeding the Link Supervision Timeout.  The
+		 * default Link Supervision Timeout for AMP
+		 * controllers is 10 seconds.
+		 *
+		 * Class 1 devices use 0xffffffff for their
+		 * best-effort flush timeout, so the clamping logic
+		 * will result in a timeout that meets the above
+		 * requirement.  ERTM timeouts are 16-bit values, so
+		 * the maximum timeout is 65.535 seconds.
+		 */
+
+		/* Convert timeout to milliseconds and round */
+		ertm_to = div_u64(ertm_to + 999, 1000);
+
+		/* This is the recommended formula for class 2 devices
+		 * that start ERTM timers when packets are sent to the
+		 * controller.
+		 */
+		ertm_to = 3 * ertm_to + 500;
+
+		if (ertm_to > 0xffff)
+			ertm_to = 0xffff;
+
+		rfc->retrans_timeout = cpu_to_le16((u16) ertm_to);
+		rfc->monitor_timeout = rfc->retrans_timeout;
+	} else {
+		rfc->retrans_timeout = __constant_cpu_to_le16(L2CAP_DEFAULT_RETRANS_TO);
+		rfc->monitor_timeout = __constant_cpu_to_le16(L2CAP_DEFAULT_MONITOR_TO);
+	}
+}
+
 static inline void l2cap_txwin_setup(struct l2cap_chan *chan)
 {
 	if (chan->tx_win > L2CAP_DEFAULT_TX_WINDOW &&
@@ -2983,8 +3021,8 @@ done:
 	case L2CAP_MODE_ERTM:
 		rfc.mode            = L2CAP_MODE_ERTM;
 		rfc.max_transmit    = chan->max_tx;
-		rfc.retrans_timeout = 0;
-		rfc.monitor_timeout = 0;
+
+		__l2cap_set_ertm_timeouts(chan, &rfc);
 
 		size = min_t(u16, L2CAP_DEFAULT_MAX_PDU_SIZE, chan->conn->mtu -
 						L2CAP_EXT_HDR_SIZE -
@@ -3216,10 +3254,7 @@ done:
 			rfc.max_pdu_size = cpu_to_le16(size);
 			chan->remote_mps = size;
 
-			rfc.retrans_timeout =
-				__constant_cpu_to_le16(L2CAP_DEFAULT_RETRANS_TO);
-			rfc.monitor_timeout =
-				__constant_cpu_to_le16(L2CAP_DEFAULT_MONITOR_TO);
+			__l2cap_set_ertm_timeouts(chan, &rfc);
 
 			set_bit(CONF_MODE_DONE, &chan->conf_state);
 
-- 
1.7.11.2

--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

[RFCv0 18/21] Bluetooth: Ignore BR/EDR packet size constraints when fragmenting for AMP

[Mat Martineau]

Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
 net/bluetooth/l2cap_core.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index f430744..e4df53d 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -2222,7 +2222,9 @@ static int l2cap_segment_sdu(struct l2cap_chan *chan,
 	/* PDU size is derived from the HCI MTU */
 	pdu_len = chan->conn->mtu;
 
-	pdu_len = min_t(size_t, pdu_len, L2CAP_BREDR_MAX_PAYLOAD);
+	/* Constrain PDU size for BR/EDR connections */
+	if (!chan->hs_hcon)
+		pdu_len = min_t(size_t, pdu_len, L2CAP_BREDR_MAX_PAYLOAD);
 
 	/* Adjust for largest possible L2CAP overhead. */
 	if (chan->fcs)
-- 
1.7.11.2

--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

[RFCv0 19/21] Bluetooth: Tie AMP link setup to connect/disconnect

[Mat Martineau]

Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
 net/bluetooth/l2cap_core.c | 83 ++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 77 insertions(+), 6 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index e4df53d..d69b4b3 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -56,7 +56,10 @@ static void l2cap_send_disconn_req(struct l2cap_conn *conn,
 				   struct l2cap_chan *chan, int err);
 
 static void l2cap_tx(struct l2cap_chan *chan, struct l2cap_ctrl *control,
-		    struct sk_buff_head *skbs, u8 event);
+		     struct sk_buff_head *skbs, u8 event);
+
+static void l2cap_logical_cfm(struct l2cap_chan *chan, struct hci_chan *hchan,
+			      u8 status);
 
 /* ---- L2CAP channels ---- */
 
@@ -552,6 +555,13 @@ void l2cap_chan_del(struct l2cap_chan *chan, int err)
 			hci_conn_put(conn->hcon);
 	}
 
+	if (chan->hs_hchan) {
+		chan->hs_hchan = NULL;
+		chan->hs_hcon = NULL;
+
+		/* Placeholder - free logical link */
+	}
+
 	if (chan->ops->teardown)
 		chan->ops->teardown(chan, err);
 
@@ -1092,8 +1102,19 @@ 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)) {
+			if (enable_hs &&
+			    (conn->fixed_chan_mask & L2CAP_FC_A2MP) &&
+			    chan->chan_policy == BT_CHANNEL_POLICY_AMP_PREFERRED) {
+				set_bit(CONF_CONNECT_PEND, &chan->conf_state);
+
+				/* Placeholder
+				amp_create_phylink();
+				*/
+			} else {
+				l2cap_send_conn_req(chan);
+			}
+		}
 	} else {
 		struct l2cap_info_req req;
 		req.type = __constant_cpu_to_le16(L2CAP_IT_FEAT_MASK);
@@ -1188,8 +1209,17 @@ static void l2cap_conn_start(struct l2cap_conn *conn)
 				continue;
 			}
 
-			l2cap_send_conn_req(chan);
+			if (enable_hs &&
+			    (conn->fixed_chan_mask & L2CAP_FC_A2MP) &&
+			    chan->chan_policy == BT_CHANNEL_POLICY_AMP_PREFERRED) {
+				set_bit(CONF_CONNECT_PEND, &chan->conf_state);
 
+				/* Placeholder
+				amp_create_phylink();
+				*/
+			} else {
+				l2cap_send_conn_req(chan);
+			}
 		} else if (chan->state == BT_CONNECT2) {
 			struct l2cap_conn_rsp rsp;
 			char buf[128];
@@ -1217,8 +1247,18 @@ static void l2cap_conn_start(struct l2cap_conn *conn)
 				rsp.status = __constant_cpu_to_le16(L2CAP_CS_AUTHEN_PEND);
 			}
 
+			if (rsp.result == __constant_cpu_to_le16(L2CAP_CR_SUCCESS) &&
+			    chan->chan_id) {
+				/* Placeholder - uncomment when amp functions
+				 * are available
+				amp_accept_physical(chan, chan->chan_id);
+				 */
+				l2cap_chan_unlock(chan);
+				continue;
+			}
+
 			l2cap_send_cmd(conn, chan->ident, L2CAP_CONN_RSP,
-							sizeof(rsp), &rsp);
+				       sizeof(rsp), &rsp);
 
 			if (test_bit(CONF_REQ_SENT, &chan->conf_state) ||
 					rsp.result != L2CAP_CR_SUCCESS) {
@@ -3301,6 +3341,18 @@ done:
 			rfc.mode = chan->mode;
 		}
 
+		if (test_bit(CONF_LOC_CONF_PEND, &chan->conf_state) &&
+		    chan->chan_id) {
+			struct hci_chan *hchan = NULL;
+
+			/* Placeholder - get hci_chan for logical link */
+
+			if (hchan && hchan->state == BT_CONNECTED) {
+				l2cap_logical_cfm(chan, hchan,
+						  L2CAP_MR_SUCCESS);
+			}
+		}
+
 		if (result == L2CAP_CONF_SUCCESS)
 			set_bit(CONF_OUTPUT_DONE, &chan->conf_state);
 	}
@@ -6262,7 +6314,18 @@ int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
 
 		if (chan->state == BT_CONNECT) {
 			if (!status) {
-				l2cap_send_conn_req(chan);
+				if (enable_hs &&
+				    (conn->fixed_chan_mask & L2CAP_FC_A2MP) &&
+				    chan->chan_policy == BT_CHANNEL_POLICY_AMP_PREFERRED) {
+					set_bit(CONF_CONNECT_PEND,
+						&chan->conf_state);
+
+					/* Placeholder
+					amp_create_phylink();
+					*/
+				} else {
+					l2cap_send_conn_req(chan);
+				}
 			} else {
 				__set_chan_timer(chan, L2CAP_DISC_TIMEOUT);
 			}
@@ -6282,6 +6345,14 @@ int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
 					if (parent)
 						parent->sk_data_ready(parent, 0);
 				} else {
+					if (chan->chan_id) {
+						/* Placeholder - accept physical
+						 * link
+						amp_accept_physical(chan, chan->chan_id);
+						*/
+						continue;
+					}
+
 					__l2cap_state_change(chan, BT_CONFIG);
 					res = L2CAP_CR_SUCCESS;
 					stat = L2CAP_CS_NO_INFO;
-- 
1.7.11.2

--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

[RFCv0 20/21] Bluetooth: Do not process ERTM reject during move

[Mat Martineau]

Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
 net/bluetooth/l2cap_core.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index d69b4b3..752ff74 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -5560,8 +5560,9 @@ static int l2cap_rx_state_recv(struct l2cap_chan *chan,
 		if (control->final) {
 			clear_bit(CONN_REMOTE_BUSY, &chan->conn_state);
 
-			if (!test_and_clear_bit(CONN_REJ_ACT,
-						&chan->conn_state)) {
+			if (!test_and_clear_bit(CONN_REJ_ACT, &chan->conn_state)
+			    && (chan->move_state == L2CAP_MOVE_STABLE ||
+				chan->move_state == L2CAP_MOVE_WAIT_PREPARE)) {
 				control->final = 0;
 				l2cap_retransmit_all(chan, control);
 			}
-- 
1.7.11.2

--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

[RFCv0 21/21] Bluetooth: Start channel move when socket option is changed

[Mat Martineau]

Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
 include/net/bluetooth/l2cap.h |  1 +
 net/bluetooth/l2cap_core.c    | 20 ++++++++++++++++++++
 net/bluetooth/l2cap_sock.c    |  5 +++++
 3 files changed, 26 insertions(+)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 7c50606..1f4e9c0 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -802,5 +802,6 @@ 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_del(struct l2cap_chan *chan, int err);
+void l2cap_move_start(struct l2cap_chan *chan);
 
 #endif /* __L2CAP_H */
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 752ff74..bb61bd0 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -4430,6 +4430,26 @@ static void l2cap_logical_cfm(struct l2cap_chan *chan, struct hci_chan *hchan,
 	}
 }
 
+void l2cap_move_start(struct l2cap_chan *chan)
+{
+	BT_DBG("chan %p", chan);
+
+	if (chan->chan_id == 0) {
+		if (chan->chan_policy != BT_CHANNEL_POLICY_AMP_PREFERRED)
+			return;
+		chan->move_role = L2CAP_MOVE_ROLE_INITIATOR;
+		chan->move_state = L2CAP_MOVE_WAIT_PREPARE;
+		/* Placeholder - start physical link setup */
+	} else {
+		chan->move_role = L2CAP_MOVE_ROLE_INITIATOR;
+		chan->move_state = L2CAP_MOVE_WAIT_RSP_SUCCESS;
+		chan->move_id = 0;
+		l2cap_move_start(chan);
+		l2cap_send_move_chan_req(chan, 0);
+		__set_chan_timer(chan, L2CAP_MOVE_TIMEOUT);
+	}
+}
+
 void l2cap_physical_cfm(struct l2cap_chan *chan, int result, u8 local_id,
 			u8 remote_id)
 {
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 79350d1..8bee29d 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -721,6 +721,11 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname, ch
 		}
 
 		chan->chan_policy = (u8) opt;
+
+		if (sk->sk_state == BT_CONNECTED &&
+		    chan->move_role == L2CAP_MOVE_ROLE_NONE)
+			l2cap_move_start(chan);
+
 		break;
 
 	default:
-- 
1.7.11.2

--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum