Linux bluetooth development
 help / color / mirror / Atom feed
* [PATCH 05/10] Bluetooth: Use explicit AMP controller id value for BR/EDR
From: Marcel Holtmann @ 2013-10-05 18:47 UTC (permalink / raw)
  To: linux-bluetooth

The special AMP controller id 0 is reserved for the BR/EDR controller
that has the main link. It is a fixed value and so use a constant for
this throughout the code to make it more visible when the handling is
for the BR/EDR channel or when it is for the AMP channel.

Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
---
 include/net/bluetooth/hci.h |  2 +-
 net/bluetooth/a2mp.c        |  4 ++--
 net/bluetooth/l2cap_core.c  | 28 ++++++++++++++--------------
 3 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 8e3076e..393eabc 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -64,7 +64,7 @@
 #define HCI_AMP		0x01
 
 /* First BR/EDR Controller shall have ID = 0 */
-#define HCI_BREDR_ID	0
+#define AMP_ID_BREDR	0x00
 
 /* AMP controller types */
 #define AMP_TYPE_BREDR	0x00
diff --git a/net/bluetooth/a2mp.c b/net/bluetooth/a2mp.c
index b2ef3d3..6d62d31 100644
--- a/net/bluetooth/a2mp.c
+++ b/net/bluetooth/a2mp.c
@@ -77,7 +77,7 @@ u8 __next_ident(struct amp_mgr *mgr)
 
 static inline void __a2mp_cl_bredr(struct a2mp_cl *cl)
 {
-	cl->id = 0;
+	cl->id = AMP_ID_BREDR;
 	cl->type = AMP_TYPE_BREDR;
 	cl->status = 1;
 }
@@ -216,7 +216,7 @@ static int a2mp_discover_rsp(struct amp_mgr *mgr, struct sk_buff *skb,
 		BT_DBG("Remote AMP id %d type %d status %d", cl->id, cl->type,
 		       cl->status);
 
-		if (cl->id != HCI_BREDR_ID && cl->type == HCI_AMP) {
+		if (cl->id != AMP_ID_BREDR && cl->type == HCI_AMP) {
 			struct a2mp_info_req req;
 
 			found = true;
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 02dba4e..b0208e2 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -3046,8 +3046,8 @@ int l2cap_ertm_init(struct l2cap_chan *chan)
 
 	skb_queue_head_init(&chan->tx_q);
 
-	chan->local_amp_id = 0;
-	chan->move_id = 0;
+	chan->local_amp_id = AMP_ID_BREDR;
+	chan->move_id = AMP_ID_BREDR;
 	chan->move_state = L2CAP_MOVE_STABLE;
 	chan->move_role = L2CAP_MOVE_ROLE_NONE;
 
@@ -3100,7 +3100,7 @@ static inline bool __l2cap_efs_supported(struct l2cap_conn *conn)
 static void __l2cap_set_ertm_timeouts(struct l2cap_chan *chan,
 				      struct l2cap_conf_rfc *rfc)
 {
-	if (chan->local_amp_id && chan->hs_hcon) {
+	if (chan->local_amp_id != AMP_ID_BREDR && chan->hs_hcon) {
 		u64 ertm_to = chan->hs_hcon->hdev->amp_be_flush_to;
 
 		/* Class 1 devices have must have ERTM timeouts
@@ -3791,12 +3791,12 @@ static struct l2cap_chan *l2cap_connect(struct l2cap_conn *conn,
 				 * The connection will succeed after the
 				 * physical link is up.
 				 */
-				if (amp_id) {
-					__l2cap_state_change(chan, BT_CONNECT2);
-					result = L2CAP_CR_PEND;
-				} else {
+				if (amp_id == AMP_ID_BREDR) {
 					__l2cap_state_change(chan, BT_CONFIG);
 					result = L2CAP_CR_SUCCESS;
+				} else {
+					__l2cap_state_change(chan, BT_CONNECT2);
+					result = L2CAP_CR_PEND;
 				}
 				status = L2CAP_CS_NO_INFO;
 			}
@@ -4423,7 +4423,7 @@ static 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);
 
 	/* For controller id 0 make BR/EDR connection */
-	if (req->amp_id == HCI_BREDR_ID) {
+	if (req->amp_id == AMP_ID_BREDR) {
 		l2cap_connect(conn, cmd, data, L2CAP_CREATE_CHAN_RSP,
 			      req->amp_id);
 		return 0;
@@ -4658,7 +4658,7 @@ void l2cap_logical_cfm(struct l2cap_chan *chan, struct hci_chan *hchan,
 
 	if (chan->state != BT_CONNECTED) {
 		/* Ignore logical link if channel is on BR/EDR */
-		if (chan->local_amp_id)
+		if (chan->local_amp_id != AMP_ID_BREDR)
 			l2cap_logical_finish_create(chan, hchan);
 	} else {
 		l2cap_logical_finish_move(chan, hchan);
@@ -4669,7 +4669,7 @@ void l2cap_move_start(struct l2cap_chan *chan)
 {
 	BT_DBG("chan %p", chan);
 
-	if (chan->local_amp_id == HCI_BREDR_ID) {
+	if (chan->local_amp_id == AMP_ID_BREDR) {
 		if (chan->chan_policy != BT_CHANNEL_POLICY_AMP_PREFERRED)
 			return;
 		chan->move_role = L2CAP_MOVE_ROLE_INITIATOR;
@@ -4868,7 +4868,7 @@ static inline int l2cap_move_channel_req(struct l2cap_conn *conn,
 		goto send_move_response;
 	}
 
-	if (req->dest_amp_id) {
+	if (req->dest_amp_id != AMP_ID_BREDR) {
 		struct hci_dev *hdev;
 		hdev = hci_dev_get(req->dest_amp_id);
 		if (!hdev || hdev->dev_type != HCI_AMP ||
@@ -4898,7 +4898,7 @@ static inline int l2cap_move_channel_req(struct l2cap_conn *conn,
 	chan->move_id = req->dest_amp_id;
 	icid = chan->dcid;
 
-	if (!req->dest_amp_id) {
+	if (req->dest_amp_id == AMP_ID_BREDR) {
 		/* Moving to BR/EDR */
 		if (test_bit(CONN_LOCAL_BUSY, &chan->conn_state)) {
 			chan->move_state = L2CAP_MOVE_WAIT_LOCAL_BUSY;
@@ -5090,7 +5090,7 @@ static int l2cap_move_channel_confirm(struct l2cap_conn *conn,
 	if (chan->move_state == L2CAP_MOVE_WAIT_CONFIRM) {
 		if (result == L2CAP_MC_CONFIRMED) {
 			chan->local_amp_id = chan->move_id;
-			if (!chan->local_amp_id)
+			if (chan->local_amp_id == AMP_ID_BREDR)
 				__release_logical_link(chan);
 		} else {
 			chan->move_id = chan->local_amp_id;
@@ -5130,7 +5130,7 @@ static inline int l2cap_move_channel_confirm_rsp(struct l2cap_conn *conn,
 	if (chan->move_state == L2CAP_MOVE_WAIT_CONFIRM_RSP) {
 		chan->local_amp_id = chan->move_id;
 
-		if (!chan->local_amp_id && chan->hs_hchan)
+		if (chan->local_amp_id == AMP_ID_BREDR && chan->hs_hchan)
 			__release_logical_link(chan);
 
 		l2cap_move_done(chan);
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH 04/10] Bluetooth: Separate AMP controller type from HCI device type
From: Marcel Holtmann @ 2013-10-05 18:47 UTC (permalink / raw)
  To: linux-bluetooth

There are two defined HCI device types. One is for BR/EDR controllers
and the other is for AMP controllers. The HCI device type is not the
same as the AMP controller type. It just happens that currently the
defined types match, but that is not guaranteed.

Split the usage of AMP controller type into its own domain so that
it is possible to separate between BR/EDR controllers, 802.11 AMP
controllers and any other AMP technology that might be defined in
the future.

Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
---
 include/net/bluetooth/hci_core.h | 2 +-
 net/bluetooth/a2mp.c             | 8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index ab52dc7..86e2dbe 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -805,7 +805,7 @@ static inline bool hci_amp_capable(void)
 
 	read_lock(&hci_dev_list_lock);
 	list_for_each_entry(hdev, &hci_dev_list, list)
-		if (hdev->amp_type == HCI_AMP &&
+		if (hdev->amp_type != AMP_TYPE_BREDR &&
 		    test_bit(HCI_UP, &hdev->flags))
 			ret = true;
 	read_unlock(&hci_dev_list_lock);
diff --git a/net/bluetooth/a2mp.c b/net/bluetooth/a2mp.c
index a6e45e1..b2ef3d3 100644
--- a/net/bluetooth/a2mp.c
+++ b/net/bluetooth/a2mp.c
@@ -78,7 +78,7 @@ u8 __next_ident(struct amp_mgr *mgr)
 static inline void __a2mp_cl_bredr(struct a2mp_cl *cl)
 {
 	cl->id = 0;
-	cl->type = 0;
+	cl->type = AMP_TYPE_BREDR;
 	cl->status = 1;
 }
 
@@ -352,7 +352,7 @@ static int a2mp_getampassoc_req(struct amp_mgr *mgr, struct sk_buff *skb,
 	tmp = amp_mgr_lookup_by_state(READ_LOC_AMP_ASSOC);
 
 	hdev = hci_dev_get(req->id);
-	if (!hdev || hdev->amp_type == HCI_BREDR || tmp) {
+	if (!hdev || hdev->amp_type == AMP_TYPE_BREDR || tmp) {
 		struct a2mp_amp_assoc_rsp rsp;
 		rsp.id = req->id;
 
@@ -459,7 +459,7 @@ static int a2mp_createphyslink_req(struct amp_mgr *mgr, struct sk_buff *skb,
 	rsp.remote_id = req->local_id;
 
 	hdev = hci_dev_get(req->remote_id);
-	if (!hdev || hdev->amp_type != HCI_AMP) {
+	if (!hdev || hdev->amp_type == AMP_TYPE_BREDR) {
 		rsp.status = A2MP_STATUS_INVALID_CTRL_ID;
 		goto send_rsp;
 	}
@@ -879,7 +879,7 @@ void a2mp_send_getinfo_rsp(struct hci_dev *hdev)
 	rsp.id = hdev->id;
 	rsp.status = A2MP_STATUS_INVALID_CTRL_ID;
 
-	if (hdev->amp_type != HCI_BREDR) {
+	if (hdev->amp_type != AMP_TYPE_BREDR) {
 		rsp.status = 0;
 		rsp.total_bw = cpu_to_le32(hdev->amp_total_bw);
 		rsp.max_bw = cpu_to_le32(hdev->amp_max_bw);
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH 03/10] Bluetooth: Add constants for AMP controller type
From: Marcel Holtmann @ 2013-10-05 18:47 UTC (permalink / raw)
  To: linux-bluetooth

Add the constants for BR/EDR and 802.11 AMP controller types.

Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
---
 include/net/bluetooth/hci.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index e8bba05..8e3076e 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -66,6 +66,10 @@
 /* First BR/EDR Controller shall have ID = 0 */
 #define HCI_BREDR_ID	0
 
+/* AMP controller types */
+#define AMP_TYPE_BREDR	0x00
+#define AMP_TYPE_80211	0x01
+
 /* AMP controller status */
 #define AMP_CTRL_POWERED_DOWN			0x00
 #define AMP_CTRL_BLUETOOTH_ONLY			0x01
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH 02/10] Bluetooth: Remove useless external function to count controllers
From: Marcel Holtmann @ 2013-10-05 18:47 UTC (permalink / raw)
  To: linux-bluetooth

The list of controllers can be counted ahead of time and inline
inside the AMP discover handling. There is no need to export such
a function at all.

In addition just count the AMP controller and only allocated space
for a single mandatory BR/EDR controller. No need to allocate more
space than needed.

Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
---
 include/net/bluetooth/hci_core.h | 13 -------------
 net/bluetooth/a2mp.c             | 10 +++++++++-
 2 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 4194851..ab52dc7 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -706,19 +706,6 @@ static inline void hci_set_drvdata(struct hci_dev *hdev, void *data)
 	dev_set_drvdata(&hdev->dev, data);
 }
 
-/* hci_dev_list shall be locked */
-static inline uint8_t __hci_num_ctrl(void)
-{
-	uint8_t count = 0;
-	struct list_head *p;
-
-	list_for_each(p, &hci_dev_list) {
-		count++;
-	}
-
-	return count;
-}
-
 struct hci_dev *hci_dev_get(int index);
 struct hci_dev *hci_get_route(bdaddr_t *dst, bdaddr_t *src);
 
diff --git a/net/bluetooth/a2mp.c b/net/bluetooth/a2mp.c
index eb0f05e..a6e45e1 100644
--- a/net/bluetooth/a2mp.c
+++ b/net/bluetooth/a2mp.c
@@ -129,6 +129,7 @@ static int a2mp_discover_req(struct amp_mgr *mgr, struct sk_buff *skb,
 	struct a2mp_discov_rsp *rsp;
 	u16 ext_feat;
 	u8 num_ctrl;
+	struct hci_dev *hdev;
 
 	if (len < sizeof(*req))
 		return -EINVAL;
@@ -152,7 +153,14 @@ static int a2mp_discover_req(struct amp_mgr *mgr, struct sk_buff *skb,
 
 	read_lock(&hci_dev_list_lock);
 
-	num_ctrl = __hci_num_ctrl();
+	/* at minimum the BR/EDR needs to be listed */
+	num_ctrl = 1;
+
+	list_for_each_entry(hdev, &hci_dev_list, list) {
+		if (hdev->dev_type == HCI_AMP)
+			num_ctrl++;
+	}
+
 	len = num_ctrl * sizeof(struct a2mp_cl) + sizeof(*rsp);
 	rsp = kmalloc(len, GFP_ATOMIC);
 	if (!rsp) {
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH 01/10] Bluetooth: Fix controller list for AMP discover response
From: Marcel Holtmann @ 2013-10-05 18:47 UTC (permalink / raw)
  To: linux-bluetooth

The AMP discover response should list exactly one BR/EDR controller
and ignore all other BR/EDR controller.

Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
---
 net/bluetooth/a2mp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/bluetooth/a2mp.c b/net/bluetooth/a2mp.c
index 17f33a6..eb0f05e 100644
--- a/net/bluetooth/a2mp.c
+++ b/net/bluetooth/a2mp.c
@@ -92,7 +92,7 @@ static void __a2mp_add_cl(struct amp_mgr *mgr, struct a2mp_cl *cl, u8 num_ctrl)
 
 	list_for_each_entry(hdev, &hci_dev_list, list) {
 		/* Iterate through AMP controllers */
-		if (hdev->id == HCI_BREDR_ID)
+		if (hdev->dev_type != HCI_AMP)
 			continue;
 
 		/* Starting from second entry */
-- 
1.8.3.1


^ permalink raw reply related

* Re: [PATCH BlueZ] monitor: Fix EIR Data Type / AD Type assigned numbers
From: Marcel Holtmann @ 2013-10-05 13:10 UTC (permalink / raw)
  To: João Paulo Rechi Vita; +Cc: linux-bluetooth, João Paulo Rechi Vita
In-Reply-To: <1380668677-5259-1-git-send-email-jprvita@gmail.com>

Hi Joao Paulo,

> From: João Paulo Rechi Vita <jprvita@openbossa.org>
> 
> The values for Public Target Address and Random Target Address were
> swapped. This information can be verified in the Bluetooth SIG Assigned
> numbers webpage:
> 
> https://www.bluetooth.org/en-us/specification/assigned-numbers/generic-access-profile
> ---
> monitor/packet.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)

patch has been applied.

Regards

Marcel


^ permalink raw reply

* Re: [PATCH 0/3] Bluetooth: LE Advertising fixes & improvements
From: Marcel Holtmann @ 2013-10-05 10:04 UTC (permalink / raw)
  To: johan.hedberg; +Cc: linux-bluetooth
In-Reply-To: <1380967266-28782-1-git-send-email-johan.hedberg@gmail.com>

Hi Johan,

> Here's a patch set to improve and fix LE advertising behavior. The first
> one renames the used flag to something more appropriate, and the two
> other ones fix advertising re-enablement due to the automatic disabling
> that controllers do when they get a new connection.
> 
> Johan
> 
> ----------------------------------------------------------------
> Johan Hedberg (3):
>      Bluetooth: Rename HCI_LE_PERIPHERAL to HCI_ADVERTISING
>      Bluetooth: Add public mgmt function to send New Settings event
>      Bluetooth: Fix re-enabling advertising after a connection
> 
> include/net/bluetooth/hci.h      |  2 +-
> include/net/bluetooth/hci_core.h |  1 +
> net/bluetooth/hci_conn.c         |  2 +-
> net/bluetooth/hci_core.c         |  2 +-
> net/bluetooth/hci_event.c        | 57 +++++++++++++++++++++++++++++++++++++---
> net/bluetooth/mgmt.c             | 23 +++++++++-------
> 6 files changed, 71 insertions(+), 16 deletions(-)

all 3 patches have been applied to bluetooth-next tree.

Regards

Marcel


^ permalink raw reply

* [PATCH 3/3] Bluetooth: Fix re-enabling advertising after a connection
From: johan.hedberg @ 2013-10-05 10:01 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1380967266-28782-1-git-send-email-johan.hedberg@gmail.com>

From: Johan Hedberg <johan.hedberg@intel.com>

LE controllers will automatically disable advertising whenever they
accept a new connection. In order not to fall out of sync with the
advertising setting we need to re-enable advertising whenever the last
LE connection drops. A failure to re-enable advertising should cause the
setting to be disabled, so this patch also calls mgmt_new_settings()
when this happens.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/hci_event.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 50 insertions(+), 1 deletion(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index e1ad858..6eaef6e 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1796,6 +1796,40 @@ static u8 hci_to_mgmt_reason(u8 err)
 	}
 }
 
+static void adv_enable_complete(struct hci_dev *hdev, u8 status)
+{
+	BT_DBG("%s status %u", hdev->name, status);
+
+	/* Clear the advertising mgmt setting if we failed to re-enable it */
+	if (status) {
+		clear_bit(HCI_ADVERTISING, &hdev->dev_flags);
+		mgmt_new_settings(hdev);
+	}
+}
+
+static void reenable_advertising(struct hci_dev *hdev)
+{
+	struct hci_request req;
+	u8 enable = 0x01;
+
+	if (hdev->conn_hash.le_num)
+		return;
+
+	if (!test_bit(HCI_ADVERTISING, &hdev->dev_flags))
+		return;
+
+	hci_req_init(&req, hdev);
+	hci_req_add(&req, HCI_OP_LE_SET_ADV_ENABLE, sizeof(enable), &enable);
+
+	/* If this fails we have no option but to let user space know
+	 * that we've disabled advertising.
+	 */
+	if (hci_req_run(&req, adv_enable_complete) < 0) {
+		clear_bit(HCI_ADVERTISING, &hdev->dev_flags);
+		mgmt_new_settings(hdev);
+	}
+}
+
 static void hci_disconn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
 {
 	struct hci_ev_disconn_complete *ev = (void *) skb->data;
@@ -1826,10 +1860,25 @@ static void hci_disconn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
 	}
 
 	if (ev->status == 0) {
-		if (conn->type == ACL_LINK && conn->flush_key)
+		u8 type = conn->type;
+
+		if (type == ACL_LINK && conn->flush_key)
 			hci_remove_link_key(hdev, &conn->dst);
 		hci_proto_disconn_cfm(conn, ev->reason);
 		hci_conn_del(conn);
+
+		/* Re-enable advertising if necessary, since it might
+		 * have been disabled by the connection. From the
+		 * HCI_LE_Set_Advertise_Enable command description in
+		 * the core specification (v4.0):
+		 * "The Controller shall continue advertising until the Host
+		 * issues an LE_Set_Advertise_Enable command with
+		 * Advertising_Enable set to 0x00 (Advertising is disabled)
+		 * or until a connection is created or until the Advertising
+		 * is timed out due to Directed Advertising."
+		 */
+		if (type == LE_LINK)
+			reenable_advertising(hdev);
 	}
 
 unlock:
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH 2/3] Bluetooth: Add public mgmt function to send New Settings event
From: johan.hedberg @ 2013-10-05 10:01 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1380967266-28782-1-git-send-email-johan.hedberg@gmail.com>

From: Johan Hedberg <johan.hedberg@intel.com>

A function is needed so that the HCI event processing can ask the mgmt
code to emit a new settings event. This is necessary e.g. when the event
processing does updates to mgmt related states without any dependency of
actual mgmt commands.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 include/net/bluetooth/hci_core.h | 1 +
 net/bluetooth/mgmt.c             | 5 +++++
 2 files changed, 6 insertions(+)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index e09c305..079c5c5 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1123,6 +1123,7 @@ void hci_sock_dev_event(struct hci_dev *hdev, int event);
 int mgmt_control(struct sock *sk, struct msghdr *msg, size_t len);
 int mgmt_index_added(struct hci_dev *hdev);
 int mgmt_index_removed(struct hci_dev *hdev);
+int mgmt_new_settings(struct hci_dev *hdev);
 int mgmt_set_powered_failed(struct hci_dev *hdev, int err);
 int mgmt_powered(struct hci_dev *hdev, u8 powered);
 int mgmt_discoverable(struct hci_dev *hdev, u8 discoverable);
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 7e43d37..6e808d1 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -890,6 +890,11 @@ static int new_settings(struct hci_dev *hdev, struct sock *skip)
 	return mgmt_event(MGMT_EV_NEW_SETTINGS, hdev, &ev, sizeof(ev), skip);
 }
 
+int mgmt_new_settings(struct hci_dev *hdev)
+{
+	return new_settings(hdev, NULL);
+}
+
 struct cmd_lookup {
 	struct sock *sk;
 	struct hci_dev *hdev;
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH 1/3] Bluetooth: Rename HCI_LE_PERIPHERAL to HCI_ADVERTISING
From: johan.hedberg @ 2013-10-05 10:01 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1380967266-28782-1-git-send-email-johan.hedberg@gmail.com>

From: Johan Hedberg <johan.hedberg@intel.com>

This flag is used to indicate whether we want to have advertising
enabled or not, so give it a more suitable name.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 include/net/bluetooth/hci.h |  2 +-
 net/bluetooth/hci_conn.c    |  2 +-
 net/bluetooth/hci_core.c    |  2 +-
 net/bluetooth/hci_event.c   |  6 +++---
 net/bluetooth/mgmt.c        | 18 +++++++++---------
 5 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index b90eec5..e8bba05 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -118,7 +118,7 @@ enum {
 	HCI_SSP_ENABLED,
 	HCI_HS_ENABLED,
 	HCI_LE_ENABLED,
-	HCI_LE_PERIPHERAL,
+	HCI_ADVERTISING,
 	HCI_CONNECTABLE,
 	HCI_DISCOVERABLE,
 	HCI_LINK_SECURITY,
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 08e601c..8b0d08f 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -546,7 +546,7 @@ static struct hci_conn *hci_connect_le(struct hci_dev *hdev, bdaddr_t *dst,
 {
 	struct hci_conn *conn;
 
-	if (test_bit(HCI_LE_PERIPHERAL, &hdev->flags))
+	if (test_bit(HCI_ADVERTISING, &hdev->flags))
 		return ERR_PTR(-ENOTSUPP);
 
 	conn = hci_conn_hash_lookup_ba(hdev, LE_LINK, dst);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 82dbdc6..aa63ebb 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1105,7 +1105,7 @@ static u8 create_ad(struct hci_dev *hdev, u8 *ptr)
 	u8 ad_len = 0, flags = 0;
 	size_t name_len;
 
-	if (test_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags))
+	if (test_bit(HCI_ADVERTISING, &hdev->dev_flags))
 		flags |= LE_AD_GENERAL;
 
 	if (test_bit(HCI_BREDR_ENABLED, &hdev->dev_flags)) {
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 4785ab0..e1ad858 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -918,9 +918,9 @@ static void hci_cc_le_set_adv_enable(struct hci_dev *hdev, struct sk_buff *skb)
 
 	if (!status) {
 		if (*sent)
-			set_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags);
+			set_bit(HCI_ADVERTISING, &hdev->dev_flags);
 		else
-			clear_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags);
+			clear_bit(HCI_ADVERTISING, &hdev->dev_flags);
 	}
 
 	if (!test_bit(HCI_INIT, &hdev->flags)) {
@@ -1005,7 +1005,7 @@ static void hci_cc_write_le_host_supported(struct hci_dev *hdev,
 		} else {
 			hdev->features[1][0] &= ~LMP_HOST_LE;
 			clear_bit(HCI_LE_ENABLED, &hdev->dev_flags);
-			clear_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags);
+			clear_bit(HCI_ADVERTISING, &hdev->dev_flags);
 		}
 
 		if (sent->simul)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 16125ff9..7e43d37 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -425,7 +425,7 @@ static u32 get_current_settings(struct hci_dev *hdev)
 	if (test_bit(HCI_HS_ENABLED, &hdev->dev_flags))
 		settings |= MGMT_SETTING_HS;
 
-	if (test_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags))
+	if (test_bit(HCI_ADVERTISING, &hdev->dev_flags))
 		settings |= MGMT_SETTING_ADVERTISING;
 
 	return settings;
@@ -1463,8 +1463,8 @@ static int set_le(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
 			changed = true;
 		}
 
-		if (!val && test_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags)) {
-			clear_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags);
+		if (!val && test_bit(HCI_ADVERTISING, &hdev->dev_flags)) {
+			clear_bit(HCI_ADVERTISING, &hdev->dev_flags);
 			changed = true;
 		}
 
@@ -1500,7 +1500,7 @@ static int set_le(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
 
 	hci_req_init(&req, hdev);
 
-	if (test_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags) && !val)
+	if (test_bit(HCI_ADVERTISING, &hdev->dev_flags) && !val)
 		hci_req_add(&req, HCI_OP_LE_SET_ADV_ENABLE, sizeof(val), &val);
 
 	hci_req_add(&req, HCI_OP_WRITE_LE_HOST_SUPPORTED, sizeof(hci_cp),
@@ -2888,7 +2888,7 @@ static int start_discovery(struct sock *sk, struct hci_dev *hdev,
 			goto failed;
 		}
 
-		if (test_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags)) {
+		if (test_bit(HCI_ADVERTISING, &hdev->dev_flags)) {
 			err = cmd_status(sk, hdev->id, MGMT_OP_START_DISCOVERY,
 					 MGMT_STATUS_REJECTED);
 			mgmt_pending_remove(cmd);
@@ -3236,13 +3236,13 @@ static int set_advertising(struct sock *sk, struct hci_dev *hdev, void *data, u1
 	hci_dev_lock(hdev);
 
 	val = !!cp->val;
-	enabled = test_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags);
+	enabled = test_bit(HCI_ADVERTISING, &hdev->dev_flags);
 
 	if (!hdev_is_powered(hdev) || val == enabled) {
 		bool changed = false;
 
-		if (val != test_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags)) {
-			change_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags);
+		if (val != test_bit(HCI_ADVERTISING, &hdev->dev_flags)) {
+			change_bit(HCI_ADVERTISING, &hdev->dev_flags);
 			changed = true;
 		}
 
@@ -3851,7 +3851,7 @@ static int powered_update_hci(struct hci_dev *hdev)
 				    &hdev->static_addr);
 	}
 
-	if (test_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags)) {
+	if (test_bit(HCI_ADVERTISING, &hdev->dev_flags)) {
 		u8 adv = 0x01;
 
 		hci_req_add(&req, HCI_OP_LE_SET_ADV_ENABLE, sizeof(adv), &adv);
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH 0/3] Bluetooth: LE Advertising fixes & improvements
From: johan.hedberg @ 2013-10-05 10:01 UTC (permalink / raw)
  To: linux-bluetooth

Hi,

Here's a patch set to improve and fix LE advertising behavior. The first
one renames the used flag to something more appropriate, and the two
other ones fix advertising re-enablement due to the automatic disabling
that controllers do when they get a new connection.

Johan

----------------------------------------------------------------
Johan Hedberg (3):
      Bluetooth: Rename HCI_LE_PERIPHERAL to HCI_ADVERTISING
      Bluetooth: Add public mgmt function to send New Settings event
      Bluetooth: Fix re-enabling advertising after a connection

 include/net/bluetooth/hci.h      |  2 +-
 include/net/bluetooth/hci_core.h |  1 +
 net/bluetooth/hci_conn.c         |  2 +-
 net/bluetooth/hci_core.c         |  2 +-
 net/bluetooth/hci_event.c        | 57 +++++++++++++++++++++++++++++++++++++---
 net/bluetooth/mgmt.c             | 23 +++++++++-------
 6 files changed, 71 insertions(+), 16 deletions(-)


^ permalink raw reply

* [RFC] Bluetooth: Set ISOC altsetting from within notify callback
From: Marcel Holtmann @ 2013-10-05  5:51 UTC (permalink / raw)
  To: linux-bluetooth

Since the event handling is done within a workqueue, the notify
callback can now sleep. So no need to trigger a separate workqueue
from within the Bluetooth USB driver.

This should give a little bit better latency with the SCO packet
processing since the ISOC altsetting is correct from the beginning.

However I am not sure if we can actually sleep in the USB reset
handler and what we need to do to restore the correct altsetting
in there. This could potentially fail, so please test ;)

Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
---
 drivers/bluetooth/btusb.c | 34 ++++++++++++++--------------------
 1 file changed, 14 insertions(+), 20 deletions(-)

diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index f3dfc0a..32cae73 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -245,7 +245,6 @@ struct btusb_data {
 
 	unsigned long flags;
 
-	struct work_struct work;
 	struct work_struct waker;
 
 	struct usb_anchor tx_anchor;
@@ -685,7 +684,6 @@ static int btusb_close(struct hci_dev *hdev)
 	if (!test_and_clear_bit(HCI_RUNNING, &hdev->flags))
 		return 0;
 
-	cancel_work_sync(&data->work);
 	cancel_work_sync(&data->waker);
 
 	clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
@@ -827,18 +825,6 @@ done:
 	return err;
 }
 
-static void btusb_notify(struct hci_dev *hdev, unsigned int evt)
-{
-	struct btusb_data *data = hci_get_drvdata(hdev);
-
-	BT_DBG("%s evt %d", hdev->name, evt);
-
-	if (hdev->conn_hash.sco_num != data->sco_num) {
-		data->sco_num = hdev->conn_hash.sco_num;
-		schedule_work(&data->work);
-	}
-}
-
 static inline int __set_isoc_interface(struct hci_dev *hdev, int altsetting)
 {
 	struct btusb_data *data = hci_get_drvdata(hdev);
@@ -882,9 +868,8 @@ static inline int __set_isoc_interface(struct hci_dev *hdev, int altsetting)
 	return 0;
 }
 
-static void btusb_work(struct work_struct *work)
+static void btusb_update_isoc_altsetting(struct btusb_data *data)
 {
-	struct btusb_data *data = container_of(work, struct btusb_data, work);
 	struct hci_dev *hdev = data->hdev;
 	int new_alts;
 	int err;
@@ -932,6 +917,18 @@ static void btusb_work(struct work_struct *work)
 	}
 }
 
+static void btusb_notify(struct hci_dev *hdev, unsigned int evt)
+{
+	struct btusb_data *data = hci_get_drvdata(hdev);
+
+	BT_DBG("%s evt %d", hdev->name, evt);
+
+	if (hdev->conn_hash.sco_num != data->sco_num) {
+		data->sco_num = hdev->conn_hash.sco_num;
+		btusb_update_isoc_altsetting(data);
+	}
+}
+
 static void btusb_waker(struct work_struct *work)
 {
 	struct btusb_data *data = container_of(work, struct btusb_data, waker);
@@ -1404,7 +1401,6 @@ static int btusb_probe(struct usb_interface *intf,
 
 	spin_lock_init(&data->lock);
 
-	INIT_WORK(&data->work, btusb_work);
 	INIT_WORK(&data->waker, btusb_waker);
 	spin_lock_init(&data->txlock);
 
@@ -1540,8 +1536,6 @@ static int btusb_suspend(struct usb_interface *intf, pm_message_t message)
 		return -EBUSY;
 	}
 
-	cancel_work_sync(&data->work);
-
 	btusb_stop_traffic(data);
 	usb_kill_anchored_urbs(&data->tx_anchor);
 
@@ -1606,8 +1600,8 @@ static int btusb_resume(struct usb_interface *intf)
 	play_deferred(data);
 	clear_bit(BTUSB_SUSPENDING, &data->flags);
 	spin_unlock_irq(&data->txlock);
-	schedule_work(&data->work);
 
+	btusb_update_isoc_altsetting(data);
 	return 0;
 
 failed:
-- 
1.8.3.1


^ permalink raw reply related

* Re: [PATCH 1/2] Bluetooth: Make LE scan interval and window a controller option
From: Marcel Holtmann @ 2013-10-05  2:39 UTC (permalink / raw)
  To: Andre Guedes; +Cc: linux-bluetooth
In-Reply-To: <524F3872.7070609@openbossa.org>

Hi Andre,

>> The scan interval and window for LE passive scanning and connection
>> establishment should be configurable on a per controller basis. So
>> introduce a setting that later on will allow modifying it.
> 
> Each remote device has their particular set of parameters based on the profiles it support. So, these parameters should be per device, not per controller.
> 
> IIRC, we discussed about this in New Orleans and the connection parameters will be informed to the kernel through the Mgmt command to load the connection list. This way, we'll have the parameters set per device.
> 
> So I don't understand exactly what is the purpose of this command you are introducing. Am I missing something?

I looked through the core specification and also the scan parameters profile. And actually the way I read it is that these two values are pretty much controller centric.

So yes, depending on the profile you are running you can change them. Or if you are running on an active device or if your device is in sleep mode. However at the end of day, the kernel does not really care much what these two values are. It can not make any smart decision about it.

The way I read especially the scan parameters profile is that not the peripheral is telling us what would be good scan parameters. Instead the central is telling what scan parameters it is using and then the peripheral is suppose to adjust its advertising to it.

All the information to pick good scan parameters are based on the remote profiles and whatever the system is currently doing. So my current take is that we let bluetoothd make that call and set the right parameters and the kernel just uses them. Of course buetoothd can adjust these at runtime any time it wants to.

Settings like the connection interval are different. They are learned from the advertising data and can be used by the connection process. The scan internal and window however make no difference. For background scanning and auto-connection handling these values should be used. In case of pairing and direct connections, we better set them to same value to get a continuous scan anyway. Same as we do for active scanning during discovery right now.

If anybody knows any better, I am happy to discuss this, but that is what I got from reading the actual specifications.

Regards

Marcel


^ permalink raw reply

* Re: [PATCH 1/2] Bluetooth: Make LE scan interval and window a controller option
From: Andre Guedes @ 2013-10-04 21:51 UTC (permalink / raw)
  To: Marcel Holtmann, linux-bluetooth
In-Reply-To: <1380881955-59853-1-git-send-email-marcel@holtmann.org>

Hi Marcel,

On 10/04/2013 07:19 AM, Marcel Holtmann wrote:
> The scan interval and window for LE passive scanning and connection
> establishment should be configurable on a per controller basis. So
> introduce a setting that later on will allow modifying it.

Each remote device has their particular set of parameters based on the 
profiles it support. So, these parameters should be per device, not per 
controller.

IIRC, we discussed about this in New Orleans and the connection 
parameters will be informed to the kernel through the Mgmt command to 
load the connection list. This way, we'll have the parameters set per 
device.

So I don't understand exactly what is the purpose of this command you 
are introducing. Am I missing something?

Regards,

Andre

^ permalink raw reply

* Re: [PATCH v2 4/4] Bluetooth: Refactor hci_connect_le
From: Andre Guedes @ 2013-10-04 20:58 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <AC94A523-A61F-4089-8BB8-5EFE52C34ACE@holtmann.org>

Hi Marcel,

On 10/04/2013 03:27 AM, Marcel Holtmann wrote:
> Hi Andre,
>
>> This patch does some code refactoring in hci_connect_le() by moving
>> the exception code into if statements and letting the main flow in
>> first level of function scope. It also adds extra comments to improve
>> the code readability.
>>
>> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
>> ---
>> net/bluetooth/hci_conn.c | 55 ++++++++++++++++++++++++++++++------------------
>> 1 file changed, 35 insertions(+), 20 deletions(-)
>>
>> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
>> index cb0e5d7..ff15841 100644
>> --- a/net/bluetooth/hci_conn.c
>> +++ b/net/bluetooth/hci_conn.c
>> @@ -531,34 +531,49 @@ static struct hci_conn *hci_connect_le(struct hci_dev *hdev, bdaddr_t *dst,
>> 	if (test_bit(HCI_LE_PERIPHERAL, &hdev->flags))
>> 		return ERR_PTR(-ENOTSUPP);
>>
>> +	/* XXX: Some devices send ATT messages as soon as the physical link
>
> no idea what the XXX wants to tell me here.

The XXX: tag is just a convention programmers use to warn other 
programmers about some problematic or special condition in the code. It 
is used widely in several projects, including the Linux kernel.

I just "grep'ed" and realized we are not used to use XXX: in Bluetooth 
subsystem so I'll remove it.

>
>> +	 * is established. To be able to handle these ATT messages, the user-
>> +	 * space first establishes the connection and then starts the pairing
>> +	 * process.
>> +	 *
>> +	 * So if a hci_conn object already exists for the following connection
>> +	 * attempt, we simply update pending_sec_level and auth_type fields
>> +	 * and return the object found.
>> +	 */
>> 	conn = hci_conn_hash_lookup_ba(hdev, LE_LINK, dst);
>> -	if (!conn) {
>> -		conn = hci_conn_hash_lookup_state(hdev, LE_LINK, BT_CONNECT);
>> -		if (conn)
>> -			return ERR_PTR(-EBUSY);
>> -
>> -		conn = hci_conn_add(hdev, LE_LINK, dst);
>> -		if (!conn)
>> -			return ERR_PTR(-ENOMEM);
>> +	if (conn) {
>> +		conn->pending_sec_level = sec_level;
>> +		conn->auth_type = auth_type;
>> +		goto out;
>> +	}
>
> Call such a label "done". I never really liked the out part.

Ok, I'll rename this label.

Regards,

Andre

^ permalink raw reply

* Re: [PATCH v2 3/4] Bluetooth: Use HCI request for LE connection
From: Andre Guedes @ 2013-10-04 20:57 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <BDCBCB2D-249D-4E3A-92C3-BCD21E28255D@holtmann.org>

Hi Marcel,

On 10/04/2013 03:25 AM, Marcel Holtmann wrote:
> Hi Andre,
>
>> This patch introduces a new helper, which uses the HCI request
>> framework, for creating LE connectons. All the handling is now
>> done by this function so we can remove the hci_cs_le_create_conn()
>> event handler.
>>
>> This patch also removes the old hci_le_create_connection() since
>> it is not used anymore.
>>
>> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
>> ---
>> include/net/bluetooth/hci_core.h |  2 ++
>> net/bluetooth/hci_conn.c         | 26 ++++++-----------------
>> net/bluetooth/hci_core.c         | 46 ++++++++++++++++++++++++++++++++++++++++
>> net/bluetooth/hci_event.c        | 31 ---------------------------
>> 4 files changed, 54 insertions(+), 51 deletions(-)
>>
>> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
>> index e09c305..0835cf9 100644
>> --- a/include/net/bluetooth/hci_core.h
>> +++ b/include/net/bluetooth/hci_core.h
>> @@ -1217,6 +1217,8 @@ void hci_le_start_enc(struct hci_conn *conn, __le16 ediv, __u8 rand[8],
>>
>> u8 bdaddr_to_le(u8 bdaddr_type);
>>
>> +int hci_create_le_conn(struct hci_dev *hdev, bdaddr_t *addr, u8 type);
>> +
>> #define SCO_AIRMODE_MASK       0x0003
>> #define SCO_AIRMODE_CVSD       0x0000
>> #define SCO_AIRMODE_TRANSP     0x0003
>> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
>> index 08e601c..cb0e5d7 100644
>> --- a/net/bluetooth/hci_conn.c
>> +++ b/net/bluetooth/hci_conn.c
>> @@ -49,25 +49,6 @@ static const struct sco_param sco_param_wideband[] = {
>> 	{ EDR_ESCO_MASK | ESCO_EV3,   0x0008 }, /* T1 */
>> };
>>
>> -static void hci_le_create_connection(struct hci_conn *conn)
>> -{
>> -	struct hci_dev *hdev = conn->hdev;
>> -	struct hci_cp_le_create_conn cp;
>> -
>> -	memset(&cp, 0, sizeof(cp));
>> -	cp.scan_interval = __constant_cpu_to_le16(0x0060);
>> -	cp.scan_window = __constant_cpu_to_le16(0x0030);
>> -	bacpy(&cp.peer_addr, &conn->dst);
>> -	cp.peer_addr_type = conn->dst_type;
>> -	cp.conn_interval_min = __constant_cpu_to_le16(0x0028);
>> -	cp.conn_interval_max = __constant_cpu_to_le16(0x0038);
>> -	cp.supervision_timeout = __constant_cpu_to_le16(0x002a);
>> -	cp.min_ce_len = __constant_cpu_to_le16(0x0000);
>> -	cp.max_ce_len = __constant_cpu_to_le16(0x0000);
>> -
>> -	hci_send_cmd(hdev, HCI_OP_LE_CREATE_CONN, sizeof(cp), &cp);
>> -}
>> -
>> static void hci_le_create_connection_cancel(struct hci_conn *conn)
>> {
>> 	hci_send_cmd(conn->hdev, HCI_OP_LE_CREATE_CONN_CANCEL, 0, NULL);
>> @@ -545,6 +526,7 @@ static struct hci_conn *hci_connect_le(struct hci_dev *hdev, bdaddr_t *dst,
>> 				    u8 dst_type, u8 sec_level, u8 auth_type)
>> {
>> 	struct hci_conn *conn;
>> +	int err;
>>
>> 	if (test_bit(HCI_LE_PERIPHERAL, &hdev->flags))
>> 		return ERR_PTR(-ENOTSUPP);
>> @@ -565,7 +547,11 @@ static struct hci_conn *hci_connect_le(struct hci_dev *hdev, bdaddr_t *dst,
>> 		conn->link_mode |= HCI_LM_MASTER;
>> 		conn->sec_level = BT_SECURITY_LOW;
>>
>> -		hci_le_create_connection(conn);
>> +		err = hci_create_le_conn(hdev, &conn->dst, conn->dst_type);
>> +		if (err) {
>> +			hci_conn_del(conn);
>> +			return ERR_PTR(err);
>> +		}
>
> it is safe to sleep here? Since that is what the new call is doing.

We are not running in an atomic section here so we are allowed to sleep.

Actually, there are other calls in this flow that may sleep. If you take 
a look at a few lines up in this function, you'll find a call to 
hci_conn_add() which allocates memory using GFP_KERNEL flag (so it may 
sleep).

Anyway, I always compile my kernels using all those kernel hacking 
options which detects errors like that and it doesn't complain about this.

>
>> 	}
>>
>> 	conn->pending_sec_level = sec_level;
>> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
>> index 82dbdc6..1002510 100644
>> --- a/net/bluetooth/hci_core.c
>> +++ b/net/bluetooth/hci_core.c
>> @@ -3662,3 +3662,49 @@ u8 bdaddr_to_le(u8 bdaddr_type)
>> 		return ADDR_LE_DEV_RANDOM;
>> 	}
>> }
>> +
>> +static void create_le_conn_complete(struct hci_dev *hdev, u8 status)
>> +{
>> +	struct hci_conn *conn;
>> +
>> +	if (status == 0)
>> +		return;
>> +
>> +	BT_ERR("HCI request failed to initiate LE connection: status 0x%2.2x",
>> +	       status);
>
> This should say "create" instead of "initiate".

I'll fix it.

>
>> +
>> +	conn = hci_conn_hash_lookup_state(hdev, LE_LINK, BT_CONNECT);
>> +	if (!conn)
>> +		return;
>> +
>
> We are no longer setting conn->state to BT_CLOSED on purpose here?

No, this is an error. I'll fix it in v3.

>
>> +	mgmt_connect_failed(hdev, &conn->dst, conn->type, conn->dst_type,
>> +			    status);
>> +
>> +	hci_proto_connect_cfm(conn, status);
>> +
>> +	hci_dev_lock(hdev);
>> +	hci_conn_del(conn);
>> +	hci_dev_unlock(hdev);
>> +}
>
> You are changing the locking behavior without any explanation. Previous code has it around the whole section. I am not a huge fan of changes that get sneaked in this way. That should be a separate patch to explain why it is correct. This patch should not change the current behavior.

Sure, I'll fix it.

Regards,

Andre

^ permalink raw reply

* Re: [PATCHv6 0/5] Initial skeleton
From: Johan Hedberg @ 2013-10-04 12:28 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <1380887327-6652-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

Hi Andrei,

On Fri, Oct 04, 2013, Andrei Emeltchenko wrote:
> Those are modified patches from Frederic Danis.
> 
> Changes:
> 	* v6: removed -n option, print to stdout & stderr depending on type of logs.
> 	* v5: log is now printed to stderr
> 
> Frederic Danis (5):
>   build: Add skeleton for BlueZ Android
>   android: Add skeleton of BlueZ Android daemon
>   android-build: Add BlueZ Android daemon
>   build: Add BlueZ Android daemon
>   android: Android version of log.c
> 
>  .gitignore          |    2 +
>  Makefile.am         |    4 +-
>  Makefile.android    |    8 ++++
>  android/Android.mk  |   31 ++++++++++++++
>  android/log.c       |   82 +++++++++++++++++++++++++++++++++++++
>  android/main.c      |  112 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  bootstrap-configure |    1 +
>  configure.ac        |    5 +++
>  8 files changed, 244 insertions(+), 1 deletion(-)
>  create mode 100644 Makefile.android
>  create mode 100644 android/Android.mk
>  create mode 100644 android/log.c
>  create mode 100644 android/main.c

This patch set has now been applied upstream. Thanks.

Johan

^ permalink raw reply

* [PATCHv6 5/5] android: Android version of log.c
From: Andrei Emeltchenko @ 2013-10-04 11:48 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1380887327-6652-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

From: Frederic Danis <frederic.danis@linux.intel.com>

Add logging for Android, currently print logs to stderr and stdout.
---
 Makefile.android   |    4 +--
 android/Android.mk |    2 ++
 android/log.c      |   82 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 android/main.c     |    6 ++++
 4 files changed, 92 insertions(+), 2 deletions(-)
 create mode 100644 android/log.c

diff --git a/Makefile.android b/Makefile.android
index e792c10..e161e6d 100644
--- a/Makefile.android
+++ b/Makefile.android
@@ -1,8 +1,8 @@
 if ANDROID
 noinst_PROGRAMS += android/bluetoothd
 
-android_bluetoothd_SOURCES = android/main.c
+android_bluetoothd_SOURCES = android/main.c src/log.c
 android_bluetoothd_LDADD = @GLIB_LIBS@
 endif
 
-EXTRA_DIST += android/Android.mk
+EXTRA_DIST += android/Android.mk android/log.c
diff --git a/android/Android.mk b/android/Android.mk
index fc3d6c2..ec820ac 100644
--- a/android/Android.mk
+++ b/android/Android.mk
@@ -14,10 +14,12 @@ include $(CLEAR_VARS)
 
 LOCAL_SRC_FILES := \
 	main.c \
+	log.c \
 
 LOCAL_C_INCLUDES := \
 	$(call include-path-for, glib) \
 	$(call include-path-for, glib)/glib \
+	$(LOCAL_PATH)/../src \
 
 LOCAL_CFLAGS := -DVERSION=\"$(BLUEZ_VERSION)\"
 
diff --git a/android/log.c b/android/log.c
new file mode 100644
index 0000000..ce07b82
--- /dev/null
+++ b/android/log.c
@@ -0,0 +1,82 @@
+/*
+ *
+ *  BlueZ - Bluetooth protocol stack for Linux
+ *
+ *  Copyright (C) 2013  Intel Corporation. All rights reserved.
+ *
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdarg.h>
+#include <unistd.h>
+#include <sys/uio.h>
+
+#include <glib.h>
+
+void info(const char *format, ...)
+{
+	va_list ap;
+
+	va_start(ap, format);
+
+	vfprintf(stdout, format, ap);
+	fprintf(stdout, "\n");
+
+	va_end(ap);
+}
+
+void warn(const char *format, ...)
+{
+	va_list ap;
+
+	va_start(ap, format);
+
+	vfprintf(stderr, format, ap);
+	fprintf(stderr, "\n");
+
+	va_end(ap);
+}
+
+void error(const char *format, ...)
+{
+	va_list ap;
+
+	va_start(ap, format);
+
+	vfprintf(stderr, format, ap);
+	fprintf(stderr, "\n");
+
+	va_end(ap);
+}
+
+void btd_debug(const char *format, ...)
+{
+	va_list ap;
+
+	va_start(ap, format);
+
+	vfprintf(stdout, format, ap);
+	fprintf(stdout, "\n");
+
+	va_end(ap);
+}
diff --git a/android/main.c b/android/main.c
index f4240c3..f75b0a8 100644
--- a/android/main.c
+++ b/android/main.c
@@ -34,6 +34,8 @@
 
 #include <glib.h>
 
+#include "log.h"
+
 #define SHUTDOWN_GRACE_SECONDS 10
 
 static GMainLoop *event_loop;
@@ -98,9 +100,13 @@ int main(int argc, char *argv[])
 	sigaction(SIGINT, &sa, NULL);
 	sigaction(SIGTERM, &sa, NULL);
 
+	DBG("Entering main loop");
+
 	g_main_loop_run(event_loop);
 
 	g_main_loop_unref(event_loop);
 
+	info("Exit");
+
 	return EXIT_SUCCESS;
 }
-- 
1.7.10.4


^ permalink raw reply related

* [PATCHv6 4/5] build: Add BlueZ Android daemon
From: Andrei Emeltchenko @ 2013-10-04 11:48 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1380887327-6652-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

From: Frederic Danis <frederic.danis@linux.intel.com>

---
 .gitignore       |    2 ++
 Makefile.android |    7 +++++++
 2 files changed, 9 insertions(+)

diff --git a/.gitignore b/.gitignore
index 8a25a3e..3707209 100644
--- a/.gitignore
+++ b/.gitignore
@@ -98,3 +98,5 @@ unit/test-gobex-packet
 unit/test-gobex-transfer
 unit/test-*.log
 unit/test-*.trs
+
+android/bluetoothd
diff --git a/Makefile.android b/Makefile.android
index 56fa9a7..e792c10 100644
--- a/Makefile.android
+++ b/Makefile.android
@@ -1 +1,8 @@
+if ANDROID
+noinst_PROGRAMS += android/bluetoothd
+
+android_bluetoothd_SOURCES = android/main.c
+android_bluetoothd_LDADD = @GLIB_LIBS@
+endif
+
 EXTRA_DIST += android/Android.mk
-- 
1.7.10.4


^ permalink raw reply related

* [PATCHv6 3/5] android-build: Add BlueZ Android daemon
From: Andrei Emeltchenko @ 2013-10-04 11:48 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1380887327-6652-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

From: Frederic Danis <frederic.danis@linux.intel.com>

Define local mapping to glib path, otherwise this has to be inside central
place in the build repository.
---
 android/Android.mk |   25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/android/Android.mk b/android/Android.mk
index 183f7e2..fc3d6c2 100644
--- a/android/Android.mk
+++ b/android/Android.mk
@@ -2,3 +2,28 @@ LOCAL_PATH := $(call my-dir)
 
 # Retrieve BlueZ version from configure.ac file
 BLUEZ_VERSION := $(shell grep ^AC_INIT $(LOCAL_PATH)/../configure.ac | cpp -P -D'AC_INIT(_,v)=v')
+
+# Specify pathmap for glib
+pathmap_INCL += glib:external/bluetooth/glib
+
+#
+# Android BlueZ daemon (bluetoothd)
+#
+
+include $(CLEAR_VARS)
+
+LOCAL_SRC_FILES := \
+	main.c \
+
+LOCAL_C_INCLUDES := \
+	$(call include-path-for, glib) \
+	$(call include-path-for, glib)/glib \
+
+LOCAL_CFLAGS := -DVERSION=\"$(BLUEZ_VERSION)\"
+
+LOCAL_SHARED_LIBRARIES := \
+	libglib \
+
+LOCAL_MODULE := bluetoothd
+
+include $(BUILD_EXECUTABLE)
-- 
1.7.10.4


^ permalink raw reply related

* [PATCHv6 2/5] android: Add skeleton of BlueZ Android daemon
From: Andrei Emeltchenko @ 2013-10-04 11:48 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1380887327-6652-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

From: Frederic Danis <frederic.danis@linux.intel.com>

---
 android/main.c |  106 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 106 insertions(+)
 create mode 100644 android/main.c

diff --git a/android/main.c b/android/main.c
new file mode 100644
index 0000000..f4240c3
--- /dev/null
+++ b/android/main.c
@@ -0,0 +1,106 @@
+/*
+ *
+ *  BlueZ - Bluetooth protocol stack for Linux
+ *
+ *  Copyright (C) 2013  Intel Corporation. All rights reserved.
+ *
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <signal.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdbool.h>
+#include <string.h>
+
+#include <glib.h>
+
+#define SHUTDOWN_GRACE_SECONDS 10
+
+static GMainLoop *event_loop;
+
+static gboolean quit_eventloop(gpointer user_data)
+{
+	g_main_loop_quit(event_loop);
+
+	return FALSE;
+}
+
+static void sig_term(int sig)
+{
+	static bool __terminated = false;
+
+	if (!__terminated) {
+		g_timeout_add_seconds(SHUTDOWN_GRACE_SECONDS,
+						quit_eventloop, NULL);
+	}
+
+	__terminated = true;
+}
+
+static gboolean option_version = FALSE;
+
+static GOptionEntry options[] = {
+	{ "version", 'v', 0, G_OPTION_ARG_NONE, &option_version,
+				"Show version information and exit", NULL },
+	{ NULL }
+};
+
+int main(int argc, char *argv[])
+{
+	GOptionContext *context;
+	GError *err = NULL;
+	struct sigaction sa;
+
+	context = g_option_context_new(NULL);
+	g_option_context_add_main_entries(context, options, NULL);
+
+	if (g_option_context_parse(context, &argc, &argv, &err) == FALSE) {
+		if (err != NULL) {
+			g_printerr("%s\n", err->message);
+			g_error_free(err);
+		} else
+			g_printerr("An unknown error occurred\n");
+
+		exit(EXIT_FAILURE);
+	}
+
+	g_option_context_free(context);
+
+	if (option_version == TRUE) {
+		printf("%s\n", VERSION);
+		exit(EXIT_SUCCESS);
+	}
+
+	event_loop = g_main_loop_new(NULL, FALSE);
+
+	memset(&sa, 0, sizeof(sa));
+	sa.sa_handler = sig_term;
+	sigaction(SIGINT, &sa, NULL);
+	sigaction(SIGTERM, &sa, NULL);
+
+	g_main_loop_run(event_loop);
+
+	g_main_loop_unref(event_loop);
+
+	return EXIT_SUCCESS;
+}
-- 
1.7.10.4


^ permalink raw reply related

* [PATCHv6 1/5] build: Add skeleton for BlueZ Android
From: Andrei Emeltchenko @ 2013-10-04 11:48 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1380887327-6652-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

From: Frederic Danis <frederic.danis@linux.intel.com>

Retrieve Bluetooth version from configure.ac.
---
 Makefile.am         |    4 +++-
 Makefile.android    |    1 +
 android/Android.mk  |    4 ++++
 bootstrap-configure |    1 +
 configure.ac        |    5 +++++
 5 files changed, 14 insertions(+), 1 deletion(-)
 create mode 100644 Makefile.android
 create mode 100644 android/Android.mk

diff --git a/Makefile.am b/Makefile.am
index 4e4b1c5..51204f4 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -179,6 +179,7 @@ test_scripts =
 
 include Makefile.tools
 include Makefile.obexd
+include Makefile.android
 
 if HID2HCI
 rulesdir = @UDEV_DIR@/rules.d
@@ -293,7 +294,8 @@ pkgconfig_DATA = lib/bluez.pc
 endif
 
 DISTCHECK_CONFIGURE_FLAGS = --disable-datafiles --enable-library \
-					--disable-systemd --disable-udev
+					--disable-systemd --disable-udev \
+					--enable-android
 
 DISTCLEANFILES = $(pkgconfig_DATA)
 
diff --git a/Makefile.android b/Makefile.android
new file mode 100644
index 0000000..56fa9a7
--- /dev/null
+++ b/Makefile.android
@@ -0,0 +1 @@
+EXTRA_DIST += android/Android.mk
diff --git a/android/Android.mk b/android/Android.mk
new file mode 100644
index 0000000..183f7e2
--- /dev/null
+++ b/android/Android.mk
@@ -0,0 +1,4 @@
+LOCAL_PATH := $(call my-dir)
+
+# Retrieve BlueZ version from configure.ac file
+BLUEZ_VERSION := $(shell grep ^AC_INIT $(LOCAL_PATH)/../configure.ac | cpp -P -D'AC_INIT(_,v)=v')
diff --git a/bootstrap-configure b/bootstrap-configure
index 7a6e7d1..dc36311 100755
--- a/bootstrap-configure
+++ b/bootstrap-configure
@@ -12,4 +12,5 @@ fi
 		--sysconfdir=/etc \
 		--localstatedir=/var \
 		--enable-experimental \
+		--enable-android \
 		--disable-datafiles $*
diff --git a/configure.ac b/configure.ac
index 41c2935..7b1f64a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -242,4 +242,9 @@ AC_DEFINE_UNQUOTED(CONFIGDIR, "${configdir}",
 			[Directory for the configuration files])
 AC_SUBST(CONFIGDIR, "${configdir}")
 
+AC_ARG_ENABLE(android, AC_HELP_STRING([--enable-android],
+			[enable BlueZ for Android]),
+					[enable_android=${enableval}])
+AM_CONDITIONAL(ANDROID, test "${enable_android}" = "yes")
+
 AC_OUTPUT(Makefile src/bluetoothd.8 lib/bluez.pc)
-- 
1.7.10.4


^ permalink raw reply related

* [PATCHv6 0/5] Initial skeleton
From: Andrei Emeltchenko @ 2013-10-04 11:48 UTC (permalink / raw)
  To: linux-bluetooth

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Those are modified patches from Frederic Danis.

Changes:
	* v6: removed -n option, print to stdout & stderr depending on type of logs.
	* v5: log is now printed to stderr

Frederic Danis (5):
  build: Add skeleton for BlueZ Android
  android: Add skeleton of BlueZ Android daemon
  android-build: Add BlueZ Android daemon
  build: Add BlueZ Android daemon
  android: Android version of log.c

 .gitignore          |    2 +
 Makefile.am         |    4 +-
 Makefile.android    |    8 ++++
 android/Android.mk  |   31 ++++++++++++++
 android/log.c       |   82 +++++++++++++++++++++++++++++++++++++
 android/main.c      |  112 +++++++++++++++++++++++++++++++++++++++++++++++++++
 bootstrap-configure |    1 +
 configure.ac        |    5 +++
 8 files changed, 244 insertions(+), 1 deletion(-)
 create mode 100644 Makefile.android
 create mode 100644 android/Android.mk
 create mode 100644 android/log.c
 create mode 100644 android/main.c

-- 
1.7.10.4


^ permalink raw reply

* Re: [PATCH 01/14] obexd/bluetooth: Remove unused local variables
From: Luiz Augusto von Dentz @ 2013-10-04 11:22 UTC (permalink / raw)
  To: Szymon Janc; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <1380877538-22614-1-git-send-email-szymon.janc@tieto.com>

Hi Szymon,

On Fri, Oct 4, 2013 at 12:05 PM, Szymon Janc <szymon.janc@tieto.com> wrote:
> This ix following build errors:
>
>   CC     obexd/plugins/obexd-bluetooth.o
> obexd/plugins/bluetooth.c: In function ‘register_profile_reply’:
> obexd/plugins/bluetooth.c:202:10: error: unused variable ‘err’
>     [-Werror=unused-variable]
>
> obexd/plugins/bluetooth.c: In function ‘name_acquired’:
> obexd/plugins/bluetooth.c:367:15: error: unused variable ‘uuid’
>     [-Werror=unused-variable]
>
> obexd/plugins/bluetooth.c: In function ‘name_released’:
> obexd/plugins/bluetooth.c:389:15: error: unused variable ‘uuid’
>     [-Werror=unused-variable]
>
> obexd/plugins/bluetooth.c: In function ‘bluetooth_start’:
> obexd/plugins/bluetooth.c:400:10: error: unused variable ‘ios’
>     [-Werror=unused-variable]
> ---
>  obexd/plugins/bluetooth.c | 4 ----
>  1 file changed, 4 deletions(-)
>
> diff --git a/obexd/plugins/bluetooth.c b/obexd/plugins/bluetooth.c
> index 017ff60..7f8a26d 100644
> --- a/obexd/plugins/bluetooth.c
> +++ b/obexd/plugins/bluetooth.c
> @@ -199,7 +199,6 @@ static void register_profile_reply(DBusPendingCall *call, void *user_data)
>         struct bluetooth_profile *profile = user_data;
>         DBusMessage *reply = dbus_pending_call_steal_reply(call);
>         DBusError derr;
> -       GError *err = NULL;
>
>         dbus_error_init(&derr);
>         if (!dbus_set_error_from_message(&derr, reply)) {
> @@ -364,7 +363,6 @@ static void name_acquired(DBusConnection *conn, void *user_data)
>
>         for (l = profiles; l; l = l->next) {
>                 struct bluetooth_profile *profile = l->data;
> -               const char *uuid;
>
>                 if (profile->path != NULL)
>                         continue;
> @@ -386,7 +384,6 @@ static void name_released(DBusConnection *conn, void *user_data)
>
>         for (l = profiles; l; l = l->next) {
>                 struct bluetooth_profile *profile = l->data;
> -               const char *uuid;
>
>                 if (profile->path == NULL)
>                         continue;
> @@ -397,7 +394,6 @@ static void name_released(DBusConnection *conn, void *user_data)
>
>  static void *bluetooth_start(struct obex_server *server, int *err)
>  {
> -       GSList *ios = NULL;
>         const GSList *l;
>
>         for (l = server->drivers; l; l = l->next) {
> --
> 1.8.4

All 14 patches applied, thanks.


-- 
Luiz Augusto von Dentz

^ permalink raw reply

* [PATCHv5 7/7] build: Add logging system
From: Andrei Emeltchenko @ 2013-10-04 10:54 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1380884056-27808-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

From: Frederic Danis <frederic.danis@linux.intel.com>

---
 Makefile.android |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Makefile.android b/Makefile.android
index e792c10..e161e6d 100644
--- a/Makefile.android
+++ b/Makefile.android
@@ -1,8 +1,8 @@
 if ANDROID
 noinst_PROGRAMS += android/bluetoothd
 
-android_bluetoothd_SOURCES = android/main.c
+android_bluetoothd_SOURCES = android/main.c src/log.c
 android_bluetoothd_LDADD = @GLIB_LIBS@
 endif
 
-EXTRA_DIST += android/Android.mk
+EXTRA_DIST += android/Android.mk android/log.c
-- 
1.7.10.4


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox