[PATCH 0/6] Bluetooth: Some fixes and full peripheral role support

[PATCH 0/6] Bluetooth: Some fixes and full peripheral role support

[Johan Hedberg]

Hi,

This set of patches contains some basic fixes to SSP and LE enablement
HCI commands as well as the two missing patches from my previous set,
i.e. implementation of mgmt_set_le for peripheral mode as well as the
writing of advertising data.

Johan

[PATCH 1/6] Bluetooth: Fix setting host feature bits for SSP

[Johan Hedberg]

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

When we get a successful command complete for HCI_Write_SSP_Mode we need
to update the host feature bits for the hdev struct accordingly.

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

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index aae8053..dc60d31 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -440,7 +440,7 @@ static void hci_cc_host_buffer_size(struct hci_dev *hdev, struct sk_buff *skb)
 static void hci_cc_write_ssp_mode(struct hci_dev *hdev, struct sk_buff *skb)
 {
 	__u8 status = *((__u8 *) skb->data);
-	void *sent;
+	struct hci_cp_write_ssp_mode *sent;
 
 	BT_DBG("%s status 0x%2.2x", hdev->name, status);
 
@@ -448,10 +448,17 @@ static void hci_cc_write_ssp_mode(struct hci_dev *hdev, struct sk_buff *skb)
 	if (!sent)
 		return;
 
+	if (!status) {
+		if (sent->mode)
+			hdev->host_features[0] |= LMP_HOST_SSP;
+		else
+			hdev->host_features[0] &= ~LMP_HOST_SSP;
+	}
+
 	if (test_bit(HCI_MGMT, &hdev->dev_flags))
-		mgmt_ssp_enable_complete(hdev, *((u8 *) sent), status);
+		mgmt_ssp_enable_complete(hdev, sent->mode, status);
 	else if (!status) {
-		if (*((u8 *) sent))
+		if (sent->mode)
 			set_bit(HCI_SSP_ENABLED, &hdev->dev_flags);
 		else
 			clear_bit(HCI_SSP_ENABLED, &hdev->dev_flags);
-- 
1.7.10.4

[PATCH 2/6] Bluetooth: Fix sending unnecessary HCI_Write_SSP_Mode command

[Johan Hedberg]

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

This patch fixes sending an unnecessary HCI_Write_SSP_Mode command if
the command has already been sent as part of the default HCI init
sequence.

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

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 6642b3c..b3490c6 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -770,6 +770,7 @@ void hci_conn_del_sysfs(struct hci_conn *conn);
 #define lmp_ext_feat_capable(dev)  ((dev)->features[7] & LMP_EXTFEATURES)
 
 /* ----- Extended LMP capabilities ----- */
+#define lmp_host_ssp_capable(dev)  ((dev)->host_features[0] & LMP_HOST_SSP)
 #define lmp_host_le_capable(dev)   ((dev)->host_features[0] & LMP_HOST_LE)
 #define lmp_host_le_br_capable(dev) ((dev)->host_features[0] & LMP_HOST_LE_BREDR)
 
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 0ee150e..02dc5f8 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2914,7 +2914,8 @@ int mgmt_powered(struct hci_dev *hdev, u8 powered)
 	mgmt_pending_foreach(MGMT_OP_SET_POWERED, hdev, settings_rsp, &match);
 
 	if (powered) {
-		if (test_bit(HCI_SSP_ENABLED, &hdev->dev_flags)) {
+		if (test_bit(HCI_SSP_ENABLED, &hdev->dev_flags) &&
+		    !lmp_host_ssp_capable(hdev)) {
 			u8 ssp = 1;
 
 			hci_send_cmd(hdev, HCI_OP_WRITE_SSP_MODE, 1, &ssp);
-- 
1.7.10.4

[PATCH 3/6] Bluetooth: Fix sending unnecessary HCI_LE_Host_Enable

[Johan Hedberg]

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

This patch fixes sending an unnecessary HCI_LE_Host_Enable command if
the command has already been sent as part of the default HCI init
sequence.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/mgmt.c |   10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 02dc5f8..03dc176 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2927,8 +2927,14 @@ int mgmt_powered(struct hci_dev *hdev, u8 powered)
 			cp.le = 1;
 			cp.simul = !!lmp_le_br_capable(hdev);
 
-			hci_send_cmd(hdev, HCI_OP_WRITE_LE_HOST_SUPPORTED,
-				     sizeof(cp), &cp);
+			/* Check first if we already have the right
+			 * host state (host features set)
+			 */
+			if (cp.le != !!lmp_host_le_capable(hdev) ||
+			    cp.simul != !!lmp_host_le_br_capable(hdev))
+				hci_send_cmd(hdev,
+					     HCI_OP_WRITE_LE_HOST_SUPPORTED,
+					     sizeof(cp), &cp);
 		}
 
 		if (lmp_bredr_capable(hdev)) {
-- 
1.7.10.4

[PATCH 4/6] Bluetooth: Fix unnecessary EIR update during powering on

[Johan Hedberg]

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

When powered on the EIR data gets updated as the last step by mgmt.
Therefore avoid an update when getting a local name update as that's
part of the normal HCI init sequence.

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

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 03dc176..9fe386f 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -3523,7 +3523,12 @@ send_event:
 		err = mgmt_event(MGMT_EV_LOCAL_NAME_CHANGED, hdev, &ev,
 				 sizeof(ev), cmd ? cmd->sk : NULL);
 
-	update_eir(hdev);
+	/* EIR is taken care of separately when powering on the
+	 * adapter so only update them here if this is a name change
+	 * unrelated to power on.
+	 */
+	if (!test_bit(HCI_INIT, &hdev->flags))
+		update_eir(hdev);
 
 failed:
 	if (cmd)
-- 
1.7.10.4

[PATCH 5/6] Bluetooth: mgmt: Add support for switching to LE peripheral mode

[Johan Hedberg]

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

This patch extends the mgmt_set_le command to allow for a new value
(0x02) to mean that LE should be enabled together with advertising
enabled. This paves the way for acting in a fully functional LE
peripheral role. The change to the mgmt_set_le command is fully
backwards compatible as the value 0x01 means LE central role (which was
the old behavior).

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

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 6c414f4..a633da0 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -939,6 +939,8 @@ struct hci_rp_le_read_adv_tx_power {
 	__s8	tx_power;
 } __packed;
 
+#define HCI_OP_LE_SET_ADV_ENABLE	0x200a
+
 #define HCI_OP_LE_SET_SCAN_PARAM	0x200b
 struct hci_cp_le_set_scan_param {
 	__u8    type;
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index b3490c6..9d7e94e 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1083,6 +1083,7 @@ int mgmt_set_local_name_complete(struct hci_dev *hdev, u8 *name, u8 status);
 int mgmt_read_local_oob_data_reply_complete(struct hci_dev *hdev, u8 *hash,
 					    u8 *randomizer, u8 status);
 int mgmt_le_enable_complete(struct hci_dev *hdev, u8 enable, u8 status);
+int mgmt_le_adv_enable_complete(struct hci_dev *hdev, u8 enable, u8 status);
 int mgmt_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
 		      u8 addr_type, u8 *dev_class, s8 rssi, u8 cfm_name,
 		      u8 ssp, u8 *eir, u16 eir_len);
diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index 22980a7..26c9a4d 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -92,6 +92,7 @@ struct mgmt_rp_read_index_list {
 #define MGMT_SETTING_BREDR		0x00000080
 #define MGMT_SETTING_HS			0x00000100
 #define MGMT_SETTING_LE			0x00000200
+#define MGMT_SETTING_PERIPHERAL		0x00000400
 
 #define MGMT_OP_READ_INFO		0x0004
 #define MGMT_READ_INFO_SIZE		0
@@ -134,6 +135,11 @@ struct mgmt_cp_set_discoverable {
 #define MGMT_OP_SET_HS			0x000C
 
 #define MGMT_OP_SET_LE			0x000D
+
+#define MGMT_LE_OFF			0x00
+#define MGMT_LE_CENTRAL			0x01
+#define MGMT_LE_PERIPHERAL		0x02
+
 #define MGMT_OP_SET_DEV_CLASS		0x000E
 struct mgmt_cp_set_dev_class {
 	__u8	major;
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index dc60d31..ed6b1e2 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -790,9 +790,24 @@ static void hci_set_le_support(struct hci_dev *hdev)
 		cp.simul = !!lmp_le_br_capable(hdev);
 	}
 
+	/* If the host features don't reflect the desired state for LE
+	 * then send the write_le_host_supported command. The command
+	 * complete handler for it will take care of any necessary
+	 * subsequent commands like set_adv_enable.
+	 *
+	 * If the host features for LE are already correct and
+	 * peripheral mode is enabled directly send the le_set_adv
+	 * command. The value of &cp.le is used so that advertising will
+	 * not be enabled in the exceptional case that LE for some
+	 * reason isn't enabled - something that should only be possible
+	 * if someone is doing direct raw access to HCI.
+	 */
 	if (cp.le != !!lmp_host_le_capable(hdev))
 		hci_send_cmd(hdev, HCI_OP_WRITE_LE_HOST_SUPPORTED, sizeof(cp),
 			     &cp);
+	else if (test_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags))
+		hci_send_cmd(hdev, HCI_OP_LE_SET_ADV_ENABLE, sizeof(cp.le),
+			     &cp.le);
 }
 
 static void hci_cc_read_local_ext_features(struct hci_dev *hdev,
@@ -1196,6 +1211,39 @@ static void hci_cc_le_set_scan_param(struct hci_dev *hdev, struct sk_buff *skb)
 	}
 }
 
+static void hci_cc_le_set_adv_enable(struct hci_dev *hdev, struct sk_buff *skb)
+{
+	__u8 *sent, status = *((__u8 *) skb->data);
+
+	BT_DBG("%s status 0x%2.2x", hdev->name, status);
+
+	sent = hci_sent_cmd_data(hdev, HCI_OP_LE_SET_ADV_ENABLE);
+	if (!sent)
+		return;
+
+	hci_dev_lock(hdev);
+
+	if (!status) {
+		/* These set_bit and clear_bit calls will not actually
+		 * change anything if mgmt was used since the flag is
+		 * already set in the mgmt command handler. The calls
+		 * are here so that the reported settings remain correct
+		 * if mgmt is bypassed e.g. with hciconfig.
+		 */
+		if (*sent)
+			set_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags);
+		else
+			clear_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags);
+	}
+
+	if (!test_bit(HCI_INIT, &hdev->flags))
+		mgmt_le_adv_enable_complete(hdev, *sent, status);
+
+	hci_dev_unlock(hdev);
+
+	hci_req_complete(hdev, HCI_OP_LE_SET_ADV_ENABLE, status);
+}
+
 static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
 				      struct sk_buff *skb)
 {
@@ -1299,6 +1347,11 @@ static void hci_cc_write_le_host_supported(struct hci_dev *hdev,
 			hdev->host_features[0] |= LMP_HOST_LE_BREDR;
 		else
 			hdev->host_features[0] &= ~LMP_HOST_LE_BREDR;
+
+		if (test_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags) &&
+		    test_bit(HCI_INIT, &hdev->flags))
+			hci_send_cmd(hdev, HCI_OP_LE_SET_ADV_ENABLE,
+				     sizeof(sent->le), &sent->le);
 	}
 
 	if (test_bit(HCI_MGMT, &hdev->dev_flags) &&
@@ -2561,6 +2614,10 @@ static void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
 		hci_cc_le_set_scan_param(hdev, skb);
 		break;
 
+	case HCI_OP_LE_SET_ADV_ENABLE:
+		hci_cc_le_set_adv_enable(hdev, skb);
+		break;
+
 	case HCI_OP_LE_SET_SCAN_ENABLE:
 		hci_cc_le_set_scan_enable(hdev, skb);
 		break;
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 9fe386f..6770835 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -393,8 +393,10 @@ static u32 get_supported_settings(struct hci_dev *hdev)
 	if (enable_hs)
 		settings |= MGMT_SETTING_HS;
 
-	if (lmp_le_capable(hdev))
+	if (lmp_le_capable(hdev)) {
 		settings |= MGMT_SETTING_LE;
+		settings |= MGMT_SETTING_PERIPHERAL;
+	}
 
 	return settings;
 }
@@ -418,9 +420,13 @@ static u32 get_current_settings(struct hci_dev *hdev)
 	if (lmp_bredr_capable(hdev))
 		settings |= MGMT_SETTING_BREDR;
 
-	if (test_bit(HCI_LE_ENABLED, &hdev->dev_flags))
+	if (test_bit(HCI_LE_ENABLED, &hdev->dev_flags)) {
 		settings |= MGMT_SETTING_LE;
 
+		if (test_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags))
+			settings |= MGMT_SETTING_PERIPHERAL;
+	}
+
 	if (test_bit(HCI_LINK_SECURITY, &hdev->dev_flags))
 		settings |= MGMT_SETTING_LINK_SECURITY;
 
@@ -1207,13 +1213,36 @@ static int set_hs(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
 	return send_settings_rsp(sk, MGMT_OP_SET_HS, hdev);
 }
 
+static u8 get_le_mode(struct hci_dev *hdev)
+{
+	if (lmp_host_le_capable(hdev)) {
+		if (test_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags))
+			return MGMT_LE_PERIPHERAL;
+		return MGMT_LE_CENTRAL;
+	}
+
+	return MGMT_LE_OFF;
+}
+
+static bool valid_le_mode(u8 mode)
+{
+	switch (mode) {
+	case MGMT_LE_OFF:
+	case MGMT_LE_CENTRAL:
+	case MGMT_LE_PERIPHERAL:
+		return true;
+	}
+
+	return false;
+}
+
 static int set_le(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
 {
-	struct mgmt_mode *cp = data;
 	struct hci_cp_write_le_host_supported hci_cp;
+	struct mgmt_mode *cp = data;
 	struct pending_cmd *cmd;
+	u8 old_mode, new_mode;
 	int err;
-	u8 val, enabled;
 
 	BT_DBG("request for %s", hdev->name);
 
@@ -1225,48 +1254,78 @@ static int set_le(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
 		goto unlock;
 	}
 
-	val = !!cp->val;
-	enabled = !!lmp_host_le_capable(hdev);
+	if (!valid_le_mode(cp->val)) {
+		err = cmd_status(sk, hdev->id, MGMT_OP_SET_LE,
+				 MGMT_STATUS_INVALID_PARAMS);
+		goto unlock;
+	}
 
-	if (!hdev_is_powered(hdev) || val == enabled) {
-		bool changed = false;
+	if (mgmt_pending_find(MGMT_OP_SET_LE, hdev)) {
+		err = cmd_status(sk, hdev->id, MGMT_OP_SET_LE,
+				 MGMT_STATUS_BUSY);
+		goto unlock;
+	}
+
+	new_mode = cp->val;
+	old_mode = get_le_mode(hdev);
+
+	BT_DBG("%s old_mode %u new_mode %u", hdev->name, old_mode, new_mode);
+
+	if (new_mode == old_mode) {
+		err = send_settings_rsp(sk, MGMT_OP_SET_LE, hdev);
+		goto unlock;
+	}
+
+	/* If we're switching away from or into peripheral role toggle
+	 * the corresponding flag
+	 */
+	if (old_mode == MGMT_LE_PERIPHERAL || new_mode == MGMT_LE_PERIPHERAL)
+		change_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags);
 
-		if (val != test_bit(HCI_LE_ENABLED, &hdev->dev_flags)) {
+	if (!hdev_is_powered(hdev)) {
+		if (old_mode == MGMT_LE_OFF || new_mode == MGMT_LE_OFF)
 			change_bit(HCI_LE_ENABLED, &hdev->dev_flags);
-			changed = true;
-		}
 
 		err = send_settings_rsp(sk, MGMT_OP_SET_LE, hdev);
 		if (err < 0)
 			goto unlock;
 
-		if (changed)
-			err = new_settings(hdev, sk);
+		err = new_settings(hdev, sk);
 
 		goto unlock;
 	}
 
-	if (mgmt_pending_find(MGMT_OP_SET_LE, hdev)) {
-		err = cmd_status(sk, hdev->id, MGMT_OP_SET_LE,
-				 MGMT_STATUS_BUSY);
-		goto unlock;
-	}
-
 	cmd = mgmt_pending_add(sk, MGMT_OP_SET_LE, hdev, data, len);
 	if (!cmd) {
 		err = -ENOMEM;
 		goto unlock;
 	}
 
+	/* If we're doing a mode change from central to peripheral or to
+	 * any mode from peripheral start by sending the set_adv_enable
+	 * command.
+	 */
+	if ((old_mode != MGMT_LE_OFF && new_mode == MGMT_LE_PERIPHERAL) ||
+	    old_mode == MGMT_LE_PERIPHERAL) {
+		u8 adv_enable = (new_mode == MGMT_LE_PERIPHERAL);
+
+		err = hci_send_cmd(hdev, HCI_OP_LE_SET_ADV_ENABLE,
+				   sizeof(adv_enable), &adv_enable);
+		if (err < 0)
+			mgmt_pending_remove(cmd);
+
+		goto unlock;
+	}
+
 	memset(&hci_cp, 0, sizeof(hci_cp));
 
-	if (val) {
-		hci_cp.le = val;
+	if (new_mode != MGMT_LE_OFF) {
+		hci_cp.le = 1;
 		hci_cp.simul = !!lmp_le_br_capable(hdev);
 	}
 
-	err = hci_send_cmd(hdev, HCI_OP_WRITE_LE_HOST_SUPPORTED, sizeof(hci_cp),
-			   &hci_cp);
+	err = hci_send_cmd(hdev, HCI_OP_WRITE_LE_HOST_SUPPORTED,
+			   sizeof(hci_cp), &hci_cp);
 	if (err < 0)
 		mgmt_pending_remove(cmd);
 
@@ -3569,7 +3628,8 @@ int mgmt_read_local_oob_data_reply_complete(struct hci_dev *hdev, u8 *hash,
 
 int mgmt_le_enable_complete(struct hci_dev *hdev, u8 enable, u8 status)
 {
-	struct cmd_lookup match = { NULL, hdev };
+	struct pending_cmd *cmd;
+	struct mgmt_mode *cp;
 	bool changed = false;
 	int err = 0;
 
@@ -3594,17 +3654,67 @@ int mgmt_le_enable_complete(struct hci_dev *hdev, u8 enable, u8 status)
 			changed = true;
 	}
 
-	mgmt_pending_foreach(MGMT_OP_SET_LE, hdev, settings_rsp, &match);
+	cmd = mgmt_pending_find(MGMT_OP_SET_LE, hdev);
+	if (!cmd)
+		goto done;
+
+	cp = cmd->param;
+	if (enable && cp->val == MGMT_LE_PERIPHERAL)
+		return hci_send_cmd(hdev, HCI_OP_LE_SET_ADV_ENABLE,
+				    sizeof(enable), &enable);
+
+	send_settings_rsp(cmd->sk, cmd->opcode, hdev);
+	list_del(&cmd->list);
 
+done:
 	if (changed)
-		err = new_settings(hdev, match.sk);
+		err = new_settings(hdev, cmd ? cmd->sk : NULL);
 
-	if (match.sk)
-		sock_put(match.sk);
+	if (cmd)
+		mgmt_pending_free(cmd);
 
 	return err;
 }
 
+int mgmt_le_adv_enable_complete(struct hci_dev *hdev, u8 enable, u8 status)
+{
+	struct pending_cmd *cmd;
+	struct mgmt_mode *cp;
+
+	if (status) {
+		u8 mgmt_err = mgmt_status(status);
+
+		mgmt_pending_foreach(MGMT_OP_SET_LE, hdev, cmd_status_rsp,
+				     &mgmt_err);
+
+		return 0;
+	}
+
+	cmd = mgmt_pending_find(MGMT_OP_SET_LE, hdev);
+	if (!cmd) {
+		new_settings(hdev, NULL);
+		return 0;
+	}
+
+	cp = cmd->param;
+	if (!enable && cp->val == MGMT_LE_OFF) {
+		struct hci_cp_write_le_host_supported hci_cp;
+
+		memset(&hci_cp, 0, sizeof(hci_cp));
+
+		return hci_send_cmd(hdev, HCI_OP_WRITE_LE_HOST_SUPPORTED,
+				    sizeof(hci_cp), &hci_cp);
+	}
+
+	new_settings(hdev, cmd->sk);
+	send_settings_rsp(cmd->sk, cmd->opcode, hdev);
+
+	list_del(&cmd->list);
+	mgmt_pending_free(cmd);
+
+	return 0;
+}
+
 int mgmt_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
 		      u8 addr_type, u8 *dev_class, s8 rssi, u8 cfm_name, u8
 		      ssp, u8 *eir, u16 eir_len)
-- 
1.7.10.4

[PATCH 6/6] Bluetooth: Add support for setting LE advertising data

[Johan Hedberg]

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

This patch adds support for setting basing LE advertising data. The
three elements supported for now are the advertising flags, the TX power
and the friendly name.

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

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index a633da0..f850e94 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -335,6 +335,13 @@ enum {
 #define EIR_SSP_RAND_R		0x0F /* Simple Pairing Randomizer R */
 #define EIR_DEVICE_ID		0x10 /* device ID */
 
+/* Low Energy Advertising Flags */
+#define LE_AD_LIMITED		0x01 /* Limited Discoverable */
+#define LE_AD_GENERAL		0x02 /* General Discoverable */
+#define LE_AD_NO_BREDR		0x04 /* BR/EDR not supported */
+#define LE_AD_SIM_LE_BREDR_CTRL	0x08 /* Simultaneous LE & BR/EDR Controller */
+#define LE_AD_SIM_LE_BREDR_HOST	0x10 /* Simultaneous LE & BR/EDR Host */
+
 /* -----  HCI Commands ---- */
 #define HCI_OP_NOP			0x0000
 
@@ -939,6 +946,14 @@ struct hci_rp_le_read_adv_tx_power {
 	__s8	tx_power;
 } __packed;
 
+#define HCI_MAX_AD_LENGTH		31
+
+#define HCI_OP_LE_SET_ADV_DATA		0x2008
+struct hci_cp_le_set_adv_data {
+	__u8		length;
+	__u8		data[HCI_MAX_AD_LENGTH];
+} __packed;
+
 #define HCI_OP_LE_SET_ADV_ENABLE	0x200a
 
 #define HCI_OP_LE_SET_SCAN_PARAM	0x200b
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 9d7e94e..0981d3c 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -279,6 +279,8 @@ struct hci_dev {
 	struct le_scan_params	le_scan_params;
 
 	__s8			adv_tx_power;
+	__u8			adv_data[HCI_MAX_AD_LENGTH];
+	__u8			adv_data_len;
 
 	int (*open)(struct hci_dev *hdev);
 	int (*close)(struct hci_dev *hdev);
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index ed6b1e2..3691251 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1108,6 +1108,15 @@ static void hci_cc_le_read_adv_tx_power(struct hci_dev *hdev,
 	if (!rp->status)
 		hdev->adv_tx_power = rp->tx_power;
 
+	if (hdev->adv_data_len > 0) {
+		struct hci_cp_le_set_adv_data cp;
+
+		memcpy(cp.data, hdev->adv_data, sizeof(cp.data));
+		cp.length = cpu_to_le16(hdev->adv_data_len);
+
+		hci_send_cmd(hdev, HCI_OP_LE_SET_ADV_DATA, sizeof(cp), &cp);
+	}
+
 	hci_req_complete(hdev, HCI_OP_LE_READ_ADV_TX_POWER, rp->status);
 }
 
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 6770835..98c776a 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -593,6 +593,93 @@ static int update_eir(struct hci_dev *hdev)
 	return hci_send_cmd(hdev, HCI_OP_WRITE_EIR, sizeof(cp), &cp);
 }
 
+static u16 create_ad(struct hci_dev *hdev, u8 *data)
+{
+	u8 *ptr = data;
+	u16 ad_len = 0;
+	size_t name_len;
+	u8 flags = 0;
+
+	if (test_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags))
+		flags |= LE_AD_GENERAL;
+
+	if (!lmp_bredr_capable(hdev))
+		flags |= LE_AD_NO_BREDR;
+
+	if (lmp_le_br_capable(hdev))
+		flags |= LE_AD_SIM_LE_BREDR_CTRL;
+
+	if (lmp_host_le_br_capable(hdev))
+		flags |= LE_AD_SIM_LE_BREDR_HOST;
+
+	if (flags) {
+		BT_DBG("adv flags 0x%02x", flags);
+
+		ptr[0] = 2;
+		ptr[1] = EIR_FLAGS;
+		ptr[2] = flags;
+
+		ad_len += 3;
+		ptr += 3;
+	}
+
+	if (hdev->adv_tx_power) {
+		ptr[0] = 2;
+		ptr[1] = EIR_TX_POWER;
+		ptr[2] = (u8) hdev->adv_tx_power;
+
+		ad_len += 3;
+		ptr += 3;
+	}
+
+	name_len = strlen(hdev->dev_name);
+	if (name_len > 0) {
+		size_t max_len = HCI_MAX_AD_LENGTH - ad_len - 2;
+
+		if (name_len > max_len) {
+			name_len = max_len;
+			ptr[1] = EIR_NAME_SHORT;
+		} else
+			ptr[1] = EIR_NAME_COMPLETE;
+
+		ptr[0] = name_len + 1;
+
+		memcpy(ptr + 2, hdev->dev_name, name_len);
+
+		ad_len += (name_len + 2);
+		ptr += (name_len + 2);
+	}
+
+	return ad_len;
+}
+
+static int update_ad(struct hci_dev *hdev)
+{
+	struct hci_cp_le_set_adv_data cp;
+	u16 len;
+
+	if (!hdev_is_powered(hdev))
+		return 0;
+
+	if (!lmp_le_capable(hdev))
+		return 0;
+
+	memset(&cp, 0, sizeof(cp));
+
+	len = create_ad(hdev, cp.data);
+
+	if (hdev->adv_data_len == len &&
+	    memcmp(cp.data, hdev->adv_data, len) == 0)
+		return 0;
+
+	memcpy(hdev->adv_data, cp.data, sizeof(cp.data));
+	hdev->adv_data_len = len;
+
+	cp.length = cpu_to_le16(len);
+
+	return hci_send_cmd(hdev, HCI_OP_LE_SET_ADV_DATA, sizeof(cp), &cp);
+}
+
 static u8 get_service_classes(struct hci_dev *hdev)
 {
 	struct bt_uuid *uuid;
@@ -1279,8 +1366,10 @@ static int set_le(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
 	/* If we're switching away from or into peripheral role toggle
 	 * the corresponding flag
 	 */
-	if (old_mode == MGMT_LE_PERIPHERAL || new_mode == MGMT_LE_PERIPHERAL)
+	if (old_mode == MGMT_LE_PERIPHERAL || new_mode == MGMT_LE_PERIPHERAL) {
 		change_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags);
+		update_ad(hdev);
+	}
 
 	if (!hdev_is_powered(hdev)) {
 		if (old_mode == MGMT_LE_OFF || new_mode == MGMT_LE_OFF)
@@ -3002,6 +3091,7 @@ int mgmt_powered(struct hci_dev *hdev, u8 powered)
 			update_name(hdev, hdev->dev_name);
 			update_eir(hdev);
 		}
+		update_ad(hdev);
 	} else {
 		u8 status = MGMT_STATUS_NOT_POWERED;
 		mgmt_pending_foreach(0, hdev, cmd_status_rsp, &status);
@@ -3582,12 +3672,14 @@ send_event:
 		err = mgmt_event(MGMT_EV_LOCAL_NAME_CHANGED, hdev, &ev,
 				 sizeof(ev), cmd ? cmd->sk : NULL);
 
-	/* EIR is taken care of separately when powering on the
+	/* EIR and AD are taken care of separately when powering on the
 	 * adapter so only update them here if this is a name change
 	 * unrelated to power on.
 	 */
-	if (!test_bit(HCI_INIT, &hdev->flags))
+	if (!test_bit(HCI_INIT, &hdev->flags)) {
 		update_eir(hdev);
+		update_ad(hdev);
+	}
 
 failed:
 	if (cmd)
@@ -3662,6 +3754,8 @@ int mgmt_le_enable_complete(struct hci_dev *hdev, u8 enable, u8 status)
 	if (enable && cp->val == MGMT_LE_PERIPHERAL)
 		return hci_send_cmd(hdev, HCI_OP_LE_SET_ADV_ENABLE,
 				    sizeof(enable), &enable);
+	else
+		update_ad(hdev);
 
 	send_settings_rsp(cmd->sk, cmd->opcode, hdev);
 	list_del(&cmd->list);
@@ -3687,6 +3781,8 @@ int mgmt_le_adv_enable_complete(struct hci_dev *hdev, u8 enable, u8 status)
 		mgmt_pending_foreach(MGMT_OP_SET_LE, hdev, cmd_status_rsp,
 				     &mgmt_err);
 
+		update_ad(hdev);
+
 		return 0;
 	}
 
-- 
1.7.10.4

Re: [PATCH 1/6] Bluetooth: Fix setting host feature bits for SSP

[Marcel Holtmann]

Hi Johan,

> When we get a successful command complete for HCI_Write_SSP_Mode we need
> to update the host feature bits for the hdev struct accordingly.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
>  net/bluetooth/hci_event.c |   13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)

Acked-by: Marcel Holtmann <marcel@holtmann.org>

Regards

Marcel

Re: [PATCH 2/6] Bluetooth: Fix sending unnecessary HCI_Write_SSP_Mode command

[Marcel Holtmann]

Hi Johan,

> This patch fixes sending an unnecessary HCI_Write_SSP_Mode command if
> the command has already been sent as part of the default HCI init
> sequence.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
>  include/net/bluetooth/hci_core.h |    1 +
>  net/bluetooth/mgmt.c             |    3 ++-
>  2 files changed, 3 insertions(+), 1 deletion(-)

Acked-by: Marcel Holtmann <marcel@holtmann.org>

Regards

Marcel

Re: [PATCH 3/6] Bluetooth: Fix sending unnecessary HCI_LE_Host_Enable

[Marcel Holtmann]

Hi Johan,

> This patch fixes sending an unnecessary HCI_LE_Host_Enable command if
> the command has already been sent as part of the default HCI init
> sequence.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
>  net/bluetooth/mgmt.c |   10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)

Acked-by: Marcel Holtmann <marcel@holtmann.org>

Regards

Marcel

Re: [PATCH 4/6] Bluetooth: Fix unnecessary EIR update during powering on

[Marcel Holtmann]

Hi Johan,

> When powered on the EIR data gets updated as the last step by mgmt.
> Therefore avoid an update when getting a local name update as that's
> part of the normal HCI init sequence.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
>  net/bluetooth/mgmt.c |    7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)

Acked-by: Marcel Holtmann <marcel@holtmann.org>

Regards

Marcel

Re: [PATCH 0/6] Bluetooth: Some fixes and full peripheral role support

[Marcel Holtmann]

Hi Johan,

> This set of patches contains some basic fixes to SSP and LE enablement
> HCI commands as well as the two missing patches from my previous set,
> i.e. implementation of mgmt_set_le for peripheral mode as well as the
> writing of advertising data.

patches 1-4 look good to me. With 5-6, I would hold off a little bit
until we get this all figured out.

However we might should merge some of the basic HCI constants and
structs you need for advertisement handling. So that Aloisio's and your
patches do not conflict that much.

Regards

Marcel

Re: [PATCH 5/6] Bluetooth: mgmt: Add support for switching to LE peripheral mode

[Vinicius Costa Gomes]

Hi Johan,

On 00:09 Thu 25 Oct, Johan Hedberg wrote:
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index dc60d31..ed6b1e2 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -790,9 +790,24 @@ static void hci_set_le_support(struct hci_dev *hdev)
>  		cp.simul = !!lmp_le_br_capable(hdev);
>  	}
>  
> +	/* If the host features don't reflect the desired state for LE
> +	 * then send the write_le_host_supported command. The command
> +	 * complete handler for it will take care of any necessary
> +	 * subsequent commands like set_adv_enable.
> +	 *
> +	 * If the host features for LE are already correct and
> +	 * peripheral mode is enabled directly send the le_set_adv
> +	 * command. The value of &cp.le is used so that advertising will
> +	 * not be enabled in the exceptional case that LE for some
> +	 * reason isn't enabled - something that should only be possible
> +	 * if someone is doing direct raw access to HCI.
> +	 */
>  	if (cp.le != !!lmp_host_le_capable(hdev))
>  		hci_send_cmd(hdev, HCI_OP_WRITE_LE_HOST_SUPPORTED, sizeof(cp),
>  			     &cp);
> +	else if (test_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags))
> +		hci_send_cmd(hdev, HCI_OP_LE_SET_ADV_ENABLE, sizeof(cp.le),
> +			     &cp.le);
>  }

I agree with Marcel, and one point that worried me was this
unconditional advertising, I feel that we should be smarter about when
to start advertising, for example, here we are not taking into account
the user's visiblity settings.


Cheers,
-- 
Vinicius

Re: [PATCH 5/6] Bluetooth: mgmt: Add support for switching to LE peripheral mode

[Johan Hedberg]

Hi Vinicius,

On Wed, Oct 24, 2012, Vinicius Costa Gomes wrote:
> On 00:09 Thu 25 Oct, Johan Hedberg wrote:
> > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> > index dc60d31..ed6b1e2 100644
> > --- a/net/bluetooth/hci_event.c
> > +++ b/net/bluetooth/hci_event.c
> > @@ -790,9 +790,24 @@ static void hci_set_le_support(struct hci_dev *hdev)
> >  		cp.simul = !!lmp_le_br_capable(hdev);
> >  	}
> >  
> > +	/* If the host features don't reflect the desired state for LE
> > +	 * then send the write_le_host_supported command. The command
> > +	 * complete handler for it will take care of any necessary
> > +	 * subsequent commands like set_adv_enable.
> > +	 *
> > +	 * If the host features for LE are already correct and
> > +	 * peripheral mode is enabled directly send the le_set_adv
> > +	 * command. The value of &cp.le is used so that advertising will
> > +	 * not be enabled in the exceptional case that LE for some
> > +	 * reason isn't enabled - something that should only be possible
> > +	 * if someone is doing direct raw access to HCI.
> > +	 */
> >  	if (cp.le != !!lmp_host_le_capable(hdev))
> >  		hci_send_cmd(hdev, HCI_OP_WRITE_LE_HOST_SUPPORTED, sizeof(cp),
> >  			     &cp);
> > +	else if (test_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags))
> > +		hci_send_cmd(hdev, HCI_OP_LE_SET_ADV_ENABLE, sizeof(cp.le),
> > +			     &cp.le);
> >  }
> 
> I agree with Marcel, and one point that worried me was this
> unconditional advertising, I feel that we should be smarter about when
> to start advertising, for example, here we are not taking into account
> the user's visiblity settings.

If you're a peripheral and you're not connectable/discoverable then what
good does it do to have Bluetooth enabled at all? Since a peripheral
can't scan or initiate connections you can't really do anything. So if a
higher layer doesn't want us advertising it could either set LE to off
or central role or even power off the adapter completely.

Johan

Re: [PATCH 5/6] Bluetooth: mgmt: Add support for switching to LE peripheral mode

[Vinicius Costa Gomes]

Hi Johan,

On 01:36 Thu 25 Oct, Johan Hedberg wrote:
> Hi Vinicius,
> 
> On Wed, Oct 24, 2012, Vinicius Costa Gomes wrote:
> > On 00:09 Thu 25 Oct, Johan Hedberg wrote:
> > > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> > > index dc60d31..ed6b1e2 100644
> > > --- a/net/bluetooth/hci_event.c
> > > +++ b/net/bluetooth/hci_event.c
> > > @@ -790,9 +790,24 @@ static void hci_set_le_support(struct hci_dev *hdev)
> > >  		cp.simul = !!lmp_le_br_capable(hdev);
> > >  	}
> > >  
> > > +	/* If the host features don't reflect the desired state for LE
> > > +	 * then send the write_le_host_supported command. The command
> > > +	 * complete handler for it will take care of any necessary
> > > +	 * subsequent commands like set_adv_enable.
> > > +	 *
> > > +	 * If the host features for LE are already correct and
> > > +	 * peripheral mode is enabled directly send the le_set_adv
> > > +	 * command. The value of &cp.le is used so that advertising will
> > > +	 * not be enabled in the exceptional case that LE for some
> > > +	 * reason isn't enabled - something that should only be possible
> > > +	 * if someone is doing direct raw access to HCI.
> > > +	 */
> > >  	if (cp.le != !!lmp_host_le_capable(hdev))
> > >  		hci_send_cmd(hdev, HCI_OP_WRITE_LE_HOST_SUPPORTED, sizeof(cp),
> > >  			     &cp);
> > > +	else if (test_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags))
> > > +		hci_send_cmd(hdev, HCI_OP_LE_SET_ADV_ENABLE, sizeof(cp.le),
> > > +			     &cp.le);
> > >  }
> > 
> > I agree with Marcel, and one point that worried me was this
> > unconditional advertising, I feel that we should be smarter about when
> > to start advertising, for example, here we are not taking into account
> > the user's visiblity settings.
> 
> If you're a peripheral and you're not connectable/discoverable then what
> good does it do to have Bluetooth enabled at all? Since a peripheral
> can't scan or initiate connections you can't really do anything. So if a
> higher layer doesn't want us advertising it could either set LE to off
> or central role or even power off the adapter completely.

And what about the device that receive these advertising events? It
won't be able to do anything with them. And we would be increasing the
risk of interfering with the communication of others.

For me, the LE peripheral mode setting has a longer lifetime than (and is
orthogonal to) the Discoverable/Connectable settings. So, I would enable
Peripheral mode once during the lifetime of my session, and control the
type and whether I am advertising using the Connectable/Discoverable
settings (perhaps even from the Apps that are listening for connections).

Having to change the role of the device to the Central role or turning
the controller off (which would mean to re-do a lot of the
initialization when I turn it on again) to stop advertising seems, at
least, not intuitive.

Then, the only way to send not Discoverable nor Connectable advertising
events would be with the Broadcaster role.

> 
> Johan


Cheers,
-- 
Vinicius

Re: [PATCH 5/6] Bluetooth: mgmt: Add support for switching to LE peripheral mode

[Marcel Holtmann]

Hi Vinicius,

> > > On 00:09 Thu 25 Oct, Johan Hedberg wrote:
> > > > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> > > > index dc60d31..ed6b1e2 100644
> > > > --- a/net/bluetooth/hci_event.c
> > > > +++ b/net/bluetooth/hci_event.c
> > > > @@ -790,9 +790,24 @@ static void hci_set_le_support(struct hci_dev *hdev)
> > > >  		cp.simul = !!lmp_le_br_capable(hdev);
> > > >  	}
> > > >  
> > > > +	/* If the host features don't reflect the desired state for LE
> > > > +	 * then send the write_le_host_supported command. The command
> > > > +	 * complete handler for it will take care of any necessary
> > > > +	 * subsequent commands like set_adv_enable.
> > > > +	 *
> > > > +	 * If the host features for LE are already correct and
> > > > +	 * peripheral mode is enabled directly send the le_set_adv
> > > > +	 * command. The value of &cp.le is used so that advertising will
> > > > +	 * not be enabled in the exceptional case that LE for some
> > > > +	 * reason isn't enabled - something that should only be possible
> > > > +	 * if someone is doing direct raw access to HCI.
> > > > +	 */
> > > >  	if (cp.le != !!lmp_host_le_capable(hdev))
> > > >  		hci_send_cmd(hdev, HCI_OP_WRITE_LE_HOST_SUPPORTED, sizeof(cp),
> > > >  			     &cp);
> > > > +	else if (test_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags))
> > > > +		hci_send_cmd(hdev, HCI_OP_LE_SET_ADV_ENABLE, sizeof(cp.le),
> > > > +			     &cp.le);
> > > >  }
> > > 
> > > I agree with Marcel, and one point that worried me was this
> > > unconditional advertising, I feel that we should be smarter about when
> > > to start advertising, for example, here we are not taking into account
> > > the user's visiblity settings.
> > 
> > If you're a peripheral and you're not connectable/discoverable then what
> > good does it do to have Bluetooth enabled at all? Since a peripheral
> > can't scan or initiate connections you can't really do anything. So if a
> > higher layer doesn't want us advertising it could either set LE to off
> > or central role or even power off the adapter completely.
> 
> And what about the device that receive these advertising events? It
> won't be able to do anything with them. And we would be increasing the
> risk of interfering with the communication of others.
> 
> For me, the LE peripheral mode setting has a longer lifetime than (and is
> orthogonal to) the Discoverable/Connectable settings. So, I would enable
> Peripheral mode once during the lifetime of my session, and control the
> type and whether I am advertising using the Connectable/Discoverable
> settings (perhaps even from the Apps that are listening for connections).
> 
> Having to change the role of the device to the Central role or turning
> the controller off (which would mean to re-do a lot of the
> initialization when I turn it on again) to stop advertising seems, at
> least, not intuitive.
> 
> Then, the only way to send not Discoverable nor Connectable advertising
> events would be with the Broadcaster role.

that is actually not my worries. That is something that bluetoothd will
handle for the applications. And I do not want to intermix connectable
and discoverable from BR/EDR. These are two different things.

The way I see it currently is that peripheral mode is tight to an
application that is current providing a service. Or even a plugin inside
bluetoothd.

I am currently almost leading with Johan here that once peripheral mode
is enabled, we should just advertise. If you do not want to advertise,
then just disable peripheral mode.

The one thing I want to have figured out before we go ahead with
peripheral support is of course broadcaster and observer support.

Regards

Marcel

Re: [PATCH 5/6] Bluetooth: mgmt: Add support for switching to LE peripheral mode

[Vinicius Costa Gomes]

Hi Marcel,

On 18:54 Wed 24 Oct, Marcel Holtmann wrote:
> Hi Vinicius,
> 
> > > > On 00:09 Thu 25 Oct, Johan Hedberg wrote:
> > > > > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> > > > > index dc60d31..ed6b1e2 100644
> > > > > --- a/net/bluetooth/hci_event.c
> > > > > +++ b/net/bluetooth/hci_event.c
> > > > > @@ -790,9 +790,24 @@ static void hci_set_le_support(struct hci_dev *hdev)
> > > > >  		cp.simul = !!lmp_le_br_capable(hdev);
> > > > >  	}
> > > > >  
> > > > > +	/* If the host features don't reflect the desired state for LE
> > > > > +	 * then send the write_le_host_supported command. The command
> > > > > +	 * complete handler for it will take care of any necessary
> > > > > +	 * subsequent commands like set_adv_enable.
> > > > > +	 *
> > > > > +	 * If the host features for LE are already correct and
> > > > > +	 * peripheral mode is enabled directly send the le_set_adv
> > > > > +	 * command. The value of &cp.le is used so that advertising will
> > > > > +	 * not be enabled in the exceptional case that LE for some
> > > > > +	 * reason isn't enabled - something that should only be possible
> > > > > +	 * if someone is doing direct raw access to HCI.
> > > > > +	 */
> > > > >  	if (cp.le != !!lmp_host_le_capable(hdev))
> > > > >  		hci_send_cmd(hdev, HCI_OP_WRITE_LE_HOST_SUPPORTED, sizeof(cp),
> > > > >  			     &cp);
> > > > > +	else if (test_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags))
> > > > > +		hci_send_cmd(hdev, HCI_OP_LE_SET_ADV_ENABLE, sizeof(cp.le),
> > > > > +			     &cp.le);
> > > > >  }
> > > > 
> > > > I agree with Marcel, and one point that worried me was this
> > > > unconditional advertising, I feel that we should be smarter about when
> > > > to start advertising, for example, here we are not taking into account
> > > > the user's visiblity settings.
> > > 
> > > If you're a peripheral and you're not connectable/discoverable then what
> > > good does it do to have Bluetooth enabled at all? Since a peripheral
> > > can't scan or initiate connections you can't really do anything. So if a
> > > higher layer doesn't want us advertising it could either set LE to off
> > > or central role or even power off the adapter completely.
> > 
> > And what about the device that receive these advertising events? It
> > won't be able to do anything with them. And we would be increasing the
> > risk of interfering with the communication of others.
> > 
> > For me, the LE peripheral mode setting has a longer lifetime than (and is
> > orthogonal to) the Discoverable/Connectable settings. So, I would enable
> > Peripheral mode once during the lifetime of my session, and control the
> > type and whether I am advertising using the Connectable/Discoverable
> > settings (perhaps even from the Apps that are listening for connections).
> > 
> > Having to change the role of the device to the Central role or turning
> > the controller off (which would mean to re-do a lot of the
> > initialization when I turn it on again) to stop advertising seems, at
> > least, not intuitive.
> > 
> > Then, the only way to send not Discoverable nor Connectable advertising
> > events would be with the Broadcaster role.
> 
> that is actually not my worries. That is something that bluetoothd will
> handle for the applications. And I do not want to intermix connectable
> and discoverable from BR/EDR. These are two different things.

Ok. This is new to me. So, what information will we use to set the
advertising type and the Flags field in the advertising event (think
general/limited discoverable modes)?

> 
> The way I see it currently is that peripheral mode is tight to an
> application that is current providing a service. Or even a plugin inside
> bluetoothd.

Makes total sense.

> 
> I am currently almost leading with Johan here that once peripheral mode
> is enabled, we should just advertise. If you do not want to advertise,
> then just disable peripheral mode.

Ok. I think I got it. I was with the impression that the Peripheral bit was
something like a capability, something that I would set and forget. In
reality, it is something meant to be more transient. I would argue that
this confusion alone is a point in favor of having a separate command for it.
But I guess it is too late now.

So, my worry of not sending useless advertising events would be addressed by
bluetoothd. Sounds good enough.

> 
> The one thing I want to have figured out before we go ahead with
> peripheral support is of course broadcaster and observer support.
>
> 
> Regards
> 
> Marcel
> 
> 


Cheers,
-- 
Vinicius

Re: [PATCH 5/6] Bluetooth: mgmt: Add support for switching to LE peripheral mode

[Johan Hedberg]

Hi Vinicius,

On Thu, Oct 25, 2012, Vinicius Costa Gomes wrote:
> So, what information will we use to set the advertising type and the
> Flags field in the advertising event (think general/limited
> discoverable modes)?

Take a look at the create_ad() function in the last patch of this set.
Setting the right flags value is really quite simple. The advertising
type could be selected in a similar way, i.e. through hdev->dev_flags.

Johan

Re: [PATCH 5/6] Bluetooth: mgmt: Add support for switching to LE peripheral mode

[Anderson Lizardo]

Hi Johan,

On Thu, Oct 25, 2012 at 3:50 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi Vinicius,
>
> On Thu, Oct 25, 2012, Vinicius Costa Gomes wrote:
>> So, what information will we use to set the advertising type and the
>> Flags field in the advertising event (think general/limited
>> discoverable modes)?
>
> Take a look at the create_ad() function in the last patch of this set.
> Setting the right flags value is really quite simple. The advertising
> type could be selected in a similar way, i.e. through hdev->dev_flags.

I don't see any use now, but "actual" LE peripherals allow to use
directed advertising when for instance, they need to send an ATT
notification just to a specific device, and not let anyone else
connect to them. Do you see an easy way of selecting an specific
device to advertise to?

The issue here is that once LE Peripheral is enabled, any device can
connect to you (e.g. other previously bonded devices will for sure
attempt to reconnect). If you want to be connected by a specific
device, you need to use directed advertising from the beginning.

Also is LE advertising re-enabled when a connection is established
(remember it is disabled automatically by the controller after
connection is established)? And after an HCI Reset ?

Regards,
-- 
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

Re: [PATCH 5/6] Bluetooth: mgmt: Add support for switching to LE peripheral mode

[Johan Hedberg]

Hi Lizardo,

On Thu, Oct 25, 2012, Anderson Lizardo wrote:
> On Thu, Oct 25, 2012 at 3:50 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> > Hi Vinicius,
> >
> > On Thu, Oct 25, 2012, Vinicius Costa Gomes wrote:
> >> So, what information will we use to set the advertising type and the
> >> Flags field in the advertising event (think general/limited
> >> discoverable modes)?
> >
> > Take a look at the create_ad() function in the last patch of this set.
> > Setting the right flags value is really quite simple. The advertising
> > type could be selected in a similar way, i.e. through hdev->dev_flags.
> 
> I don't see any use now, but "actual" LE peripherals allow to use
> directed advertising when for instance, they need to send an ATT
> notification just to a specific device, and not let anyone else
> connect to them. Do you see an easy way of selecting an specific
> device to advertise to?
> 
> The issue here is that once LE Peripheral is enabled, any device can
> connect to you (e.g. other previously bonded devices will for sure
> attempt to reconnect). If you want to be connected by a specific
> device, you need to use directed advertising from the beginning.

This will need some more thinking. We could add a new mgmt command for
that, or if you wanna go crazy why not map this to the L2CAP socket
interface's connect() system call. I.e. if LE GAP is in peripheral mode
instead of doing HCI_LE_Create_Connection or just failing with "not
allowed" a socket connect() would simply trigger directed advertising to
the device in question and deliver the successful connection in the same
way as a central role triggered connect would do (unless we time out
waiting for the remote device to connect). Now that I think of this
second option it actually sounds quite natural and not so crazy after
all :)

> Also is LE advertising re-enabled when a connection is established
> (remember it is disabled automatically by the controller after
> connection is established)? And after an HCI Reset ?

The "after a connection" part is still missing and I'll have that as a
separate patch not to grow the already quite big patch. As for reset,
right now that's only sent when power-cycling the adapter and the HCI
init sequence then takes care of enabling advertising.

Johan

Re: [PATCH 5/6] Bluetooth: mgmt: Add support for switching to LE peripheral mode

[Johan Hedberg]

Hi,

On Thu, Oct 25, 2012, Johan Hedberg wrote:
> This will need some more thinking. We could add a new mgmt command for
> that, or if you wanna go crazy why not map this to the L2CAP socket
> interface's connect() system call. I.e. if LE GAP is in peripheral mode
> instead of doing HCI_LE_Create_Connection or just failing with "not
> allowed" a socket connect() would simply trigger directed advertising to
> the device in question and deliver the successful connection in the same
> way as a central role triggered connect would do (unless we time out
> waiting for the remote device to connect). Now that I think of this
> second option it actually sounds quite natural and not so crazy after
> all :)

There's on problem with this though: we'd still be undirected
connectable between doing mgmt_set_le(peripheral) and issuing the socket
connect(). So maybe we might need to introduce a mgmt_set_le_connectable
command and a "le-connectable" setting after all (that could only be set
in peripheral mode).

Johan

Re: [PATCH 5/6] Bluetooth: mgmt: Add support for switching to LE peripheral mode

[Vinicius Costa Gomes]

Hi,

On 15:04 Thu 25 Oct, Johan Hedberg wrote:
> Hi,
> 
> On Thu, Oct 25, 2012, Johan Hedberg wrote:
> > This will need some more thinking. We could add a new mgmt command for
> > that, or if you wanna go crazy why not map this to the L2CAP socket
> > interface's connect() system call. I.e. if LE GAP is in peripheral mode
> > instead of doing HCI_LE_Create_Connection or just failing with "not
> > allowed" a socket connect() would simply trigger directed advertising to
> > the device in question and deliver the successful connection in the same
> > way as a central role triggered connect would do (unless we time out
> > waiting for the remote device to connect). Now that I think of this
> > second option it actually sounds quite natural and not so crazy after
> > all :)
> 
> There's on problem with this though: we'd still be undirected
> connectable between doing mgmt_set_le(peripheral) and issuing the socket
> connect(). So maybe we might need to introduce a mgmt_set_le_connectable
> command and a "le-connectable" setting after all (that could only be set
> in peripheral mode).


Why not sending undirected connectable events when there's an active
listen()?

> 
> Johan


Cheers,
-- 
Vinicius

Re: [PATCH 3/6] Bluetooth: Fix sending unnecessary HCI_LE_Host_Enable

[Gustavo Padovan]

Hi Johan,

* Johan Hedberg <johan.hedberg@gmail.com> [2012-10-25 00:09:53 +0300]:

> From: Johan Hedberg <johan.hedberg@intel.com>
> 
> This patch fixes sending an unnecessary HCI_LE_Host_Enable command if
> the command has already been sent as part of the default HCI init
> sequence.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
>  net/bluetooth/mgmt.c |   10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 02dc5f8..03dc176 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -2927,8 +2927,14 @@ int mgmt_powered(struct hci_dev *hdev, u8 powered)
>  			cp.le = 1;
>  			cp.simul = !!lmp_le_br_capable(hdev);
>  
> -			hci_send_cmd(hdev, HCI_OP_WRITE_LE_HOST_SUPPORTED,
> -				     sizeof(cp), &cp);
> +			/* Check first if we already have the right
> +			 * host state (host features set)
> +			 */
> +			if (cp.le != !!lmp_host_le_capable(hdev) ||
> +			    cp.simul != !!lmp_host_le_br_capable(hdev))

Shouldn't the !! be part of the macro itself? Looks we will be using !! always
with these macros.

	Gustavo

Re: [PATCH 5/6] Bluetooth: mgmt: Add support for switching to LE peripheral mode

[Johan Hedberg]

Hi Vinicius,

On Thu, Oct 25, 2012, Vinicius Costa Gomes wrote:
> > On Thu, Oct 25, 2012, Johan Hedberg wrote:
> > > This will need some more thinking. We could add a new mgmt command for
> > > that, or if you wanna go crazy why not map this to the L2CAP socket
> > > interface's connect() system call. I.e. if LE GAP is in peripheral mode
> > > instead of doing HCI_LE_Create_Connection or just failing with "not
> > > allowed" a socket connect() would simply trigger directed advertising to
> > > the device in question and deliver the successful connection in the same
> > > way as a central role triggered connect would do (unless we time out
> > > waiting for the remote device to connect). Now that I think of this
> > > second option it actually sounds quite natural and not so crazy after
> > > all :)
> > 
> > There's on problem with this though: we'd still be undirected
> > connectable between doing mgmt_set_le(peripheral) and issuing the socket
> > connect(). So maybe we might need to introduce a mgmt_set_le_connectable
> > command and a "le-connectable" setting after all (that could only be set
> > in peripheral mode).
> 
> Why not sending undirected connectable events when there's an active
> listen()?

But we always have that, don't we? (the GATT server socket). Or are you
saying that bluetoothd could close its GATT server socket before
switching to peripheral mode and that would ensure that the kernel
doesn't enable connectable advertising?

Johan

Re: [PATCH 3/6] Bluetooth: Fix sending unnecessary HCI_LE_Host_Enable

[Marcel Holtmann]

Hi Gustavo,

> > From: Johan Hedberg <johan.hedberg@intel.com>
> > 
> > This patch fixes sending an unnecessary HCI_LE_Host_Enable command if
> > the command has already been sent as part of the default HCI init
> > sequence.
> > 
> > Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> > ---
> >  net/bluetooth/mgmt.c |   10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> > 
> > diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> > index 02dc5f8..03dc176 100644
> > --- a/net/bluetooth/mgmt.c
> > +++ b/net/bluetooth/mgmt.c
> > @@ -2927,8 +2927,14 @@ int mgmt_powered(struct hci_dev *hdev, u8 powered)
> >  			cp.le = 1;
> >  			cp.simul = !!lmp_le_br_capable(hdev);
> >  
> > -			hci_send_cmd(hdev, HCI_OP_WRITE_LE_HOST_SUPPORTED,
> > -				     sizeof(cp), &cp);
> > +			/* Check first if we already have the right
> > +			 * host state (host features set)
> > +			 */
> > +			if (cp.le != !!lmp_host_le_capable(hdev) ||
> > +			    cp.simul != !!lmp_host_le_br_capable(hdev))
> 
> Shouldn't the !! be part of the macro itself? Looks we will be using !! always
> with these macros.

we could do that actually. However I prefer to do that with a separate
patch.

Regards

Marcel

Re: [PATCH 4/6] Bluetooth: Fix unnecessary EIR update during powering on

[Gustavo Padovan]

Hi Johan,

* Johan Hedberg <johan.hedberg@gmail.com> [2012-10-25 00:09:54 +0300]:

> From: Johan Hedberg <johan.hedberg@intel.com>
> 
> When powered on the EIR data gets updated as the last step by mgmt.
> Therefore avoid an update when getting a local name update as that's
> part of the normal HCI init sequence.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
>  net/bluetooth/mgmt.c |    7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)

Patch 1, 2 and 4 have been applied to bluetooth-next. Thanks.

	Gustavo

Re: [PATCH 3/6] Bluetooth: Fix sending unnecessary HCI_LE_Host_Enable

[Johan Hedberg]

Hi,

On Thu, Oct 25, 2012, Marcel Holtmann wrote:
> > > From: Johan Hedberg <johan.hedberg@intel.com>
> > > 
> > > This patch fixes sending an unnecessary HCI_LE_Host_Enable command if
> > > the command has already been sent as part of the default HCI init
> > > sequence.
> > > 
> > > Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> > > ---
> > >  net/bluetooth/mgmt.c |   10 ++++++++--
> > >  1 file changed, 8 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> > > index 02dc5f8..03dc176 100644
> > > --- a/net/bluetooth/mgmt.c
> > > +++ b/net/bluetooth/mgmt.c
> > > @@ -2927,8 +2927,14 @@ int mgmt_powered(struct hci_dev *hdev, u8 powered)
> > >  			cp.le = 1;
> > >  			cp.simul = !!lmp_le_br_capable(hdev);
> > >  
> > > -			hci_send_cmd(hdev, HCI_OP_WRITE_LE_HOST_SUPPORTED,
> > > -				     sizeof(cp), &cp);
> > > +			/* Check first if we already have the right
> > > +			 * host state (host features set)
> > > +			 */
> > > +			if (cp.le != !!lmp_host_le_capable(hdev) ||
> > > +			    cp.simul != !!lmp_host_le_br_capable(hdev))
> > 
> > Shouldn't the !! be part of the macro itself? Looks we will be using !! always
> > with these macros.
> 
> we could do that actually. However I prefer to do that with a separate
> patch.

I was thinking of the same earlier, and yes a separate patch to convert
all of the feature test macros would be the cleanest.

Johan

Re: [PATCH 3/6] Bluetooth: Fix sending unnecessary HCI_LE_Host_Enable

[Gustavo Padovan]

Hi,

* Johan Hedberg <johan.hedberg@gmail.com> [2012-10-25 18:14:30 +0300]:

> Hi,
> 
> On Thu, Oct 25, 2012, Marcel Holtmann wrote:
> > > > From: Johan Hedberg <johan.hedberg@intel.com>
> > > > 
> > > > This patch fixes sending an unnecessary HCI_LE_Host_Enable command if
> > > > the command has already been sent as part of the default HCI init
> > > > sequence.
> > > > 
> > > > Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> > > > ---
> > > >  net/bluetooth/mgmt.c |   10 ++++++++--
> > > >  1 file changed, 8 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> > > > index 02dc5f8..03dc176 100644
> > > > --- a/net/bluetooth/mgmt.c
> > > > +++ b/net/bluetooth/mgmt.c
> > > > @@ -2927,8 +2927,14 @@ int mgmt_powered(struct hci_dev *hdev, u8 powered)
> > > >  			cp.le = 1;
> > > >  			cp.simul = !!lmp_le_br_capable(hdev);
> > > >  
> > > > -			hci_send_cmd(hdev, HCI_OP_WRITE_LE_HOST_SUPPORTED,
> > > > -				     sizeof(cp), &cp);
> > > > +			/* Check first if we already have the right
> > > > +			 * host state (host features set)
> > > > +			 */
> > > > +			if (cp.le != !!lmp_host_le_capable(hdev) ||
> > > > +			    cp.simul != !!lmp_host_le_br_capable(hdev))
> > > 
> > > Shouldn't the !! be part of the macro itself? Looks we will be using !! always
> > > with these macros.
> > 
> > we could do that actually. However I prefer to do that with a separate
> > patch.
> 
> I was thinking of the same earlier, and yes a separate patch to convert
> all of the feature test macros would be the cleanest.

I didn't say it needed to be in the same patch. Anyway, I applied this patch
now, I'll do the macro's conversion later today.

	Gustavo

Re: [PATCH 5/6] Bluetooth: mgmt: Add support for switching to LE peripheral mode

[Vinicius Costa Gomes]

Hi Johan,

On 18:07 Thu 25 Oct, Johan Hedberg wrote:
> Hi Vinicius,
> 
> On Thu, Oct 25, 2012, Vinicius Costa Gomes wrote:
> > > On Thu, Oct 25, 2012, Johan Hedberg wrote:
> > > > This will need some more thinking. We could add a new mgmt command for
> > > > that, or if you wanna go crazy why not map this to the L2CAP socket
> > > > interface's connect() system call. I.e. if LE GAP is in peripheral mode
> > > > instead of doing HCI_LE_Create_Connection or just failing with "not
> > > > allowed" a socket connect() would simply trigger directed advertising to
> > > > the device in question and deliver the successful connection in the same
> > > > way as a central role triggered connect would do (unless we time out
> > > > waiting for the remote device to connect). Now that I think of this
> > > > second option it actually sounds quite natural and not so crazy after
> > > > all :)
> > > 
> > > There's on problem with this though: we'd still be undirected
> > > connectable between doing mgmt_set_le(peripheral) and issuing the socket
> > > connect(). So maybe we might need to introduce a mgmt_set_le_connectable
> > > command and a "le-connectable" setting after all (that could only be set
> > > in peripheral mode).
> > 
> > Why not sending undirected connectable events when there's an active
> > listen()?
> 
> But we always have that, don't we? (the GATT server socket). Or are you
> saying that bluetoothd could close its GATT server socket before
> switching to peripheral mode and that would ensure that the kernel
> doesn't enable connectable advertising?

Right now bluetoothd always has a listen() active, yes. But remember
that that listen() was added only to help testing, in the case that
BlueZ always initiates connections that listen() is not needed.

What I am thinking is having more control about when to advertise, for
example, each profile that needs the peripheral role would increase the
reference of a listener, when all the references are dropped I stop
advertising.

> 
> Johan


Cheers,
-- 
Vinicius