[RFC 1/3] Bluetooth: Implement Read RSSI command

[RFC 1/3] Bluetooth: Implement Read RSSI command

[anderson.briglia]

From: Anderson Briglia <anderson.briglia@openbossa.org>

This patch implements helper functions to make Read RSSI command
interceptable by MGMT Interface. It adds a new wrapper in HCI layer and
add a hook to call mgmt_read_rssi_complete when MGMT Interface has been
loaded.

Read RSSI command is defined on Part E, section 7.5.4 of Bluetooth 4.0
Spec.

Signed-off-by: Anderson Briglia <anderson.briglia@openbossa.org>
---
 include/net/bluetooth/hci.h      |   10 ++++
 include/net/bluetooth/hci_core.h |    2 +
 include/net/bluetooth/mgmt.h     |   10 ++++
 net/bluetooth/hci_event.c        |   24 +++++++++
 net/bluetooth/mgmt.c             |   96 ++++++++++++++++++++++++++++++++++++++
 5 files changed, 142 insertions(+), 0 deletions(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index c4fdeeb..276be6e 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -550,6 +550,16 @@ struct hci_cp_set_event_flt {
 	__u8     condition[0];
 } __packed;
 
+#define HCI_OP_READ_RSSI		0x1405
+struct hci_cp_read_rssi {
+	__le16	handle;
+} __packed;
+struct hci_rp_read_rssi {
+	__u8	status;
+	__le16	handle;
+	__s8	rssi;
+} __packed;
+
 /* Filter types */
 #define HCI_FLT_CLEAR_ALL	0x00
 #define HCI_FLT_INQ_RESULT	0x01
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 9e5736b..2a5b804 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -856,6 +856,8 @@ int mgmt_auth_failed(u16 index, bdaddr_t *bdaddr, u8 status);
 int mgmt_set_local_name_complete(u16 index, u8 *name, u8 status);
 int mgmt_read_local_oob_data_reply_complete(u16 index, u8 *hash, u8 *randomizer,
 								u8 status);
+int mgmt_read_rssi_complete(u16 index, bdaddr_t *bdaddr, s8 rssi, u8 status);
+int mgmt_read_rssi_failed(u16 index);
 int mgmt_device_found(u16 index, bdaddr_t *bdaddr, u8 *dev_class, s8 rssi,
 							u8 *eir, u8 eir_len);
 int mgmt_remote_name(u16 index, bdaddr_t *bdaddr, u8 *name);
diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index 5428fd3..ae0358b 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -211,6 +211,16 @@ struct mgmt_cp_unblock_device {
 	bdaddr_t bdaddr;
 } __packed;
 
+#define MGMT_OP_READ_RSSI		0x001F
+struct mgmt_cp_read_rssi {
+	bdaddr_t bdaddr;
+} __packed;
+struct mgmt_rp_read_rssi {
+	__u8 status;
+	bdaddr_t bdaddr;
+	__s8 rssi;
+} __packed;
+
 #define MGMT_EV_CMD_COMPLETE		0x0001
 struct mgmt_ev_cmd_complete {
 	__le16 opcode;
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 55872ff..e13a98b 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -982,6 +982,26 @@ static inline void hci_cc_write_le_host_supported(struct hci_dev *hdev,
 	hci_send_cmd(hdev, HCI_OP_READ_LOCAL_EXT_FEATURES, sizeof(cp), &cp);
 }
 
+static void hci_cc_read_rssi(struct hci_dev *hdev, struct sk_buff *skb)
+{
+	struct hci_rp_read_rssi *rp = (void *) skb->data;
+	struct hci_conn *conn;
+
+	BT_DBG("%s status 0x%x", hdev->name, rp->status);
+
+	if (!test_bit(HCI_MGMT, &hdev->flags))
+		return;
+
+	conn = hci_conn_hash_lookup_handle(hdev, __le16_to_cpu(rp->handle));
+	if (!conn) {
+		mgmt_read_rssi_failed(hdev->id);
+		return;
+	}
+
+	mgmt_read_rssi_complete(hdev->id, &conn->dst, rp->rssi,
+							rp->status);
+}
+
 static inline void hci_cs_inquiry(struct hci_dev *hdev, __u8 status)
 {
 	BT_DBG("%s status 0x%x", hdev->name, status);
@@ -2012,6 +2032,10 @@ static inline void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *sk
 		hci_cc_write_le_host_supported(hdev, skb);
 		break;
 
+	case HCI_OP_READ_RSSI:
+		hci_cc_read_rssi(hdev, skb);
+		break;
+
 	default:
 		BT_DBG("%s opcode 0x%x", hdev->name, opcode);
 		break;
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index f2d9078..18049d0 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1628,6 +1628,102 @@ static int remove_remote_oob_data(struct sock *sk, u16 index,
 	return err;
 }
 
+static int read_rssi(struct sock *sk, u16 index, unsigned char *data, u16 len)
+{
+	struct hci_dev *hdev;
+	struct mgmt_cp_read_rssi *cp = (void *) data;
+	struct hci_cp_read_rssi rs;
+	struct hci_conn *conn;
+	struct pending_cmd *cmd;
+	int err;
+
+	BT_DBG("hci%u", index);
+
+	if (len != sizeof(*cp))
+		return cmd_status(sk, index, MGMT_OP_READ_RSSI, EINVAL);
+
+	hdev = hci_dev_get(index);
+	if (!hdev)
+		return cmd_status(sk, index, MGMT_OP_READ_RSSI, ENODEV);
+
+	hci_dev_lock_bh(hdev);
+
+	if (!test_bit(HCI_UP, &hdev->flags)) {
+		err = cmd_status(sk, index, MGMT_OP_READ_RSSI, ENETDOWN);
+		goto unlock;
+	}
+
+	if (mgmt_pending_find(MGMT_OP_READ_RSSI, index)) {
+		err = cmd_status(sk, index, MGMT_OP_READ_RSSI, EBUSY);
+		goto unlock;
+	}
+
+	cmd = mgmt_pending_add(sk, MGMT_OP_READ_RSSI, index, NULL, 0);
+	if (!cmd) {
+		err = -ENOMEM;
+		goto unlock;
+	}
+
+	conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &cp->bdaddr);
+	if (!conn)
+		conn = hci_conn_hash_lookup_ba(hdev, LE_LINK, &cp->bdaddr);
+
+	if (!conn) {
+		err = cmd_status(sk, index, MGMT_OP_DISCONNECT, ENOTCONN);
+		goto unlock;
+	}
+
+	put_unaligned_le16(conn->handle, &rs.handle);
+
+	err = hci_send_cmd(hdev, HCI_OP_READ_RSSI, sizeof(rs), &rs);
+	if (err < 0)
+		mgmt_pending_remove(cmd);
+
+unlock:
+	hci_dev_unlock_bh(hdev);
+	hci_dev_put(hdev);
+
+	return err;
+}
+
+int mgmt_read_rssi_failed(u16 index)
+{
+	struct pending_cmd *cmd;
+	int err = 0;
+
+	cmd = mgmt_pending_find(MGMT_OP_READ_RSSI, index);
+	if (!cmd)
+		return -ENOENT;
+
+	err = cmd_status(cmd->sk, index, MGMT_OP_READ_RSSI, EIO);
+
+	mgmt_pending_remove(cmd);
+
+	return err;
+}
+
+int mgmt_read_rssi_complete(u16 index, bdaddr_t *bdaddr, s8 rssi, u8 status)
+{
+	struct pending_cmd *cmd;
+	struct mgmt_rp_read_rssi rp;
+	int err;
+
+	cmd = mgmt_pending_find(MGMT_OP_READ_RSSI, index);
+	if (!cmd)
+		return -ENOENT;
+
+	bacpy(&rp.bdaddr, bdaddr);
+	rp.status = status;
+	rp.rssi = rssi;
+
+	err = cmd_complete(cmd->sk, index, MGMT_OP_READ_RSSI, &rp,
+								sizeof(rp));
+
+	mgmt_pending_remove(cmd);
+
+	return err;
+}
+
 static int do_inquiry(struct hci_dev *hdev, __u8 inq_length)
 {
 	u8 lap[3] = { 0x33, 0x8b, 0x9e };
-- 
1.7.4.1

Re: [RFC 1/3] Bluetooth: Implement Read RSSI command

[Marcel Holtmann]

Hi Andreson,

> This patch implements helper functions to make Read RSSI command
> interceptable by MGMT Interface. It adds a new wrapper in HCI layer and
> add a hook to call mgmt_read_rssi_complete when MGMT Interface has been
> loaded.
> 
> Read RSSI command is defined on Part E, section 7.5.4 of Bluetooth 4.0
> Spec.

I think that I mentioned this before. This is not something I like to
see at all. 1:1 copies of HCI commands is pointless.

If you need RSSI results, then something like proper interval and
thresholds should be supported. Also with future Bluetooth SIG work on
having controller driven notifications in this area.

Regards

Marcel

Re: [RFC 1/3] Bluetooth: Implement Read RSSI command

[Anderson Briglia]

Hi Marcel,

On Wed, Jul 13, 2011 at 2:12 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Andreson,
>
>> This patch implements helper functions to make Read RSSI command
>> interceptable by MGMT Interface. It adds a new wrapper in HCI layer and
>> add a hook to call mgmt_read_rssi_complete when MGMT Interface has been
>> loaded.
>>
>> Read RSSI command is defined on Part E, section 7.5.4 of Bluetooth 4.0
>> Spec.
>
> I think that I mentioned this before. This is not something I like to
> see at all. 1:1 copies of HCI commands is pointless.
>
> If you need RSSI results, then something like proper interval and
> thresholds should be supported. Also with future Bluetooth SIG work on
> having controller driven notifications in this area.

Note that even the RSSI and TX Power are implemented, it is not
possible to userspace request a RSSI or TX Power actively. If
userspace wants to receive RSSI and TX Power from the kernel, it must
add to the "monitored commands" and wait for the responses. Read RSSI
and TX Power Level are not being parsed on mgmt_control().
Maybe I should change the commit message of this patch and the next one.

New Management commands were implemented in the last RFC patch to
monitor selected commands.

Regards,

Anderson Briglia

>
> Regards
>
> Marcel
>
>
>



-- 
INdT - Instituto Nokia de tecnologia
+55 2126 1122
+55 92 8423 3183

Re: [RFC 1/3] Bluetooth: Implement Read RSSI command

[Marcel Holtmann]

Hi Anderson,

> >> This patch implements helper functions to make Read RSSI command
> >> interceptable by MGMT Interface. It adds a new wrapper in HCI layer and
> >> add a hook to call mgmt_read_rssi_complete when MGMT Interface has been
> >> loaded.
> >>
> >> Read RSSI command is defined on Part E, section 7.5.4 of Bluetooth 4.0
> >> Spec.
> >
> > I think that I mentioned this before. This is not something I like to
> > see at all. 1:1 copies of HCI commands is pointless.
> >
> > If you need RSSI results, then something like proper interval and
> > thresholds should be supported. Also with future Bluetooth SIG work on
> > having controller driven notifications in this area.
> 
> Note that even the RSSI and TX Power are implemented, it is not
> possible to userspace request a RSSI or TX Power actively. If
> userspace wants to receive RSSI and TX Power from the kernel, it must
> add to the "monitored commands" and wait for the responses. Read RSSI
> and TX Power Level are not being parsed on mgmt_control().
> Maybe I should change the commit message of this patch and the next one.
> 
> New Management commands were implemented in the last RFC patch to
> monitor selected commands.

no matter what, this needs to be done future proof in conjunction with
where the Bluetooth SIG is going for the controller driven notifications
of RSSI changes.

With older controllers we just have to emulate the behavior via a simple
poll mechanism.

However such monitored commands things seems like a hack as well. Please
use proper interfaces for threshold and interval values. Otherwise this
is all pointless. And you just wake up userspace for no real reason. The
plan is to get away from pointless userspace wakeups.

Regards

Marcel

Re: [RFC 1/3] Bluetooth: Implement Read RSSI command

[Claudio Takahasi]

Hi Marcel,

On Wed, Jul 13, 2011 at 4:47 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Anderson,
>
>> >> This patch implements helper functions to make Read RSSI command
>> >> interceptable by MGMT Interface. It adds a new wrapper in HCI layer and
>> >> add a hook to call mgmt_read_rssi_complete when MGMT Interface has been
>> >> loaded.
>> >>
>> >> Read RSSI command is defined on Part E, section 7.5.4 of Bluetooth 4.0
>> >> Spec.
>> >
>> > I think that I mentioned this before. This is not something I like to
>> > see at all. 1:1 copies of HCI commands is pointless.
>> >
>> > If you need RSSI results, then something like proper interval and
>> > thresholds should be supported. Also with future Bluetooth SIG work on
>> > having controller driven notifications in this area.
>>
>> Note that even the RSSI and TX Power are implemented, it is not
>> possible to userspace request a RSSI or TX Power actively. If
>> userspace wants to receive RSSI and TX Power from the kernel, it must
>> add to the "monitored commands" and wait for the responses. Read RSSI
>> and TX Power Level are not being parsed on mgmt_control().
>> Maybe I should change the commit message of this patch and the next one.
>>
>> New Management commands were implemented in the last RFC patch to
>> monitor selected commands.
>
> no matter what, this needs to be done future proof in conjunction with
> where the Bluetooth SIG is going for the controller driven notifications
> of RSSI changes.
>
> With older controllers we just have to emulate the behavior via a simple
> poll mechanism.

The command to enable RSSI changes notification is not public, I hope
the changes in the Management API proposed by Briglia yesterday is
aligned with this new command.

>
> However such monitored commands things seems like a hack as well. Please
> use proper interfaces for threshold and interval values. Otherwise this
> is all pointless. And you just wake up userspace for no real reason. The
> plan is to get away from pointless userspace wakeups.
>
> Regards
>
> Marcel
>

About Tx Power... I understand that you don't want 1:1 mapping for HCI
commands in the management interface. Tx Power doesn't need to be
monitored, for LE it doesn't change. We could add a new management
command for Link Information command, but which relevant information
could be exposed by this Link Information command? Some commands are
applied to BREDR only, others to LE or AMP only.

At the moment we need Tx Power only, putting together Remote and Link
Information can be one alternative. Do you have other suggestion?
For example, we could create a management command called "Remote
Information" which returns: Tx Power, Features, version, ... Does it
make sense?

Regards,
Claudio