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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




Andrei -

On Fri, 2 Nov 2012, Andrei Emeltchenko wrote:

Hi Mat,

On Thu, Nov 01, 2012 at 10:51:19AM -0700, Mat Martineau wrote:

Andrei -

On Wed, 31 Oct 2012, Andrei Emeltchenko wrote:

From: Andrei Emeltchenko <andrei.emeltchenko@xxxxxxxxx>

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

Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@xxxxxxxxx>
Acked-by: Marcel Holtmann <marcel@xxxxxxxxxxxx>
---
include/net/bluetooth/amp.h   |    1 +
include/net/bluetooth/l2cap.h |    1 +
net/bluetooth/amp.c           |   24 ++++++++++++++++++++++++
net/bluetooth/hci_event.c     |   15 ++-------------
4 files changed, 28 insertions(+), 13 deletions(-)

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

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

+void amp_physical_cfm(struct hci_conn *bredr_hcon, struct hci_conn *hs_hcon)
+{
+	struct hci_dev *bredr_hdev = hci_dev_hold(bredr_hcon->hdev);
+	struct amp_mgr *mgr = hs_hcon->amp_mgr;
+	struct l2cap_chan *bredr_chan;
+
+	BT_DBG("bredr_hcon %p hs_hcon %p mgr %p", bredr_hcon, hs_hcon, mgr);
+
+	if (!bredr_hdev || !mgr || !mgr->bredr_chan)
+		return;
+
+	bredr_chan = mgr->bredr_chan;
+
+	set_bit(FLAG_EFS_ENABLE, &bredr_chan->flags);
+	bredr_chan->ctrl_id = hs_hcon->remote_id;
+	bredr_chan->hs_hcon = hs_hcon;
+	bredr_chan->conn->mtu = hs_hcon->hdev->block_mtu;
+	bredr_chan->fcs = L2CAP_FCS_NONE;

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


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

what about locking here and changing l2cap_physical_cfm to unlocked
__l2cap_physical_cfm ?

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




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

Regards,

--
Mat Martineau

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

--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux