Linux bluetooth development
 help / color / mirror / Atom feed
* Re: [PATCH 7/9 v3] Bluetooth: Refactor UUID-16 list generation into its own function
From: Marcel Holtmann @ 2013-01-26 17:13 UTC (permalink / raw)
  To: Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <1359194924-3151-8-git-send-email-johan.hedberg@gmail.com>

Hi Johan,

> We will need to create three separate UUID lists in the EIR data (for
> 16, 32 and 128 bit UUIDs) so the code is easier to follow if each list
> is generated in their own function.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
>  net/bluetooth/mgmt.c |   78 ++++++++++++++++++++++++++++----------------------
>  1 file changed, 43 insertions(+), 35 deletions(-)

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

Regards

Marcel



^ permalink raw reply

* Re: [PATCH 6/9 v3] Bluetooth: Remove useless eir_len variable from EIR creation
From: Marcel Holtmann @ 2013-01-26 17:12 UTC (permalink / raw)
  To: Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <1359194924-3151-7-git-send-email-johan.hedberg@gmail.com>

Hi Johan,

> The amount of data encoded so far in the create_eir() function can be
> calculated simply through the difference between the data and ptr
> pointer variables. The eir_len variable then becomes essentially
> useless.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
>  net/bluetooth/mgmt.c |    8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)

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

Regards

Marcel



^ permalink raw reply

* Re: [PATCH 5/9 v3] Bluetooth: Simplify UUID16 list generation for EIR
From: Marcel Holtmann @ 2013-01-26 17:11 UTC (permalink / raw)
  To: Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <1359194924-3151-6-git-send-email-johan.hedberg@gmail.com>

Hi Johan,

> There's no need to use two separate loops to generate a UUID list for
> the EIR data. This patch merges the two loops previously used for the
> 16-bit UUID list generation into a single loop, thus simplifying the
> code a great deal.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
>  net/bluetooth/mgmt.c |   46 +++++++++++++++-------------------------------
>  1 file changed, 15 insertions(+), 31 deletions(-)

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

Regards

Marcel



^ permalink raw reply

* Re: [PATCH 4/9 v3] Bluetooth: Simplify UUID removal code
From: Marcel Holtmann @ 2013-01-26 17:09 UTC (permalink / raw)
  To: Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <1359194924-3151-5-git-send-email-johan.hedberg@gmail.com>

Hi Johan,

> The UUID removal code can be simplified by using
> list_for_each_entry_safe instead of list_for_each_safe.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
>  net/bluetooth/mgmt.c |    6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index d9f6b2c..ea8d74a 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -1372,7 +1372,7 @@ static int remove_uuid(struct sock *sk, struct hci_dev *hdev, void *data,
>  {
>  	struct mgmt_cp_remove_uuid *cp = data;
>  	struct pending_cmd *cmd;
> -	struct list_head *p, *n;
> +	struct bt_uuid *match, *tmp;
>  	u8 bt_uuid_any[] = { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 };

while seeing this patch, this could have been static const actually. Not
that is affects this patch, just saying.

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

Regards

Marcel



^ permalink raw reply

* Re: [PATCH 3/9 v3] Bluetooth: Keep track of UUID type upon addition
From: Marcel Holtmann @ 2013-01-26 17:07 UTC (permalink / raw)
  To: Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <1359194924-3151-4-git-send-email-johan.hedberg@gmail.com>

Hi Johan,

> The primary purpose of the UUIDs is to eable generation of EIR and AD

assume this is suppose to read "enable".

> data. In these data formats the UUIDs are split into separate fields
> based on whether they're 16, 32 or 128 bit UUIDs. To make the generation
> of these data fields simpler this patch adds a type member to the
> bt_uuid struct and assigns a value to it as soon as the UUID is added to
> the kernel. This way the type doesn't need to be calculated each time
> the UUID list is later iterated.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
>  include/net/bluetooth/hci_core.h |    5 ++++
>  net/bluetooth/mgmt.c             |   48 ++++++++++++++++++--------------------
>  2 files changed, 28 insertions(+), 25 deletions(-)
> 
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index bcf8ffe..6ed91ef 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -83,9 +83,14 @@ struct bdaddr_list {
>  	bdaddr_t bdaddr;
>  };
>  
> +#define BT_UUID16	1
> +#define BT_UUID32	2
> +#define BT_UUID128	3
> +

I would have defined them as 16, 32 and 128. And not used constants at
all.

>  struct bt_uuid {
>  	struct list_head list;
>  	u8 uuid[16];
> +	u8 type;

And you could have used size instead of type here.

>  	u8 svc_hint;
>  };

Not that I care much, but that is how I would have done it. I am open to
discuss this.

> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 4fd45a3..d9f6b2c 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -435,28 +435,6 @@ static u32 get_current_settings(struct hci_dev *hdev)
>  
>  #define PNP_INFO_SVCLASS_ID		0x1200
>  
> -static u8 bluetooth_base_uuid[] = {
> -			0xFB, 0x34, 0x9B, 0x5F, 0x80, 0x00, 0x00, 0x80,
> -			0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> -};
> -
> -static u16 get_uuid16(u8 *uuid128)
> -{
> -	u32 val;
> -	int i;
> -
> -	for (i = 0; i < 12; i++) {
> -		if (bluetooth_base_uuid[i] != uuid128[i])
> -			return 0;
> -	}
> -
> -	val = get_unaligned_le32(&uuid128[12]);
> -	if (val > 0xffff)
> -		return 0;
> -
> -	return (u16) val;
> -}
> -
>  static void create_eir(struct hci_dev *hdev, u8 *data)
>  {
>  	u8 *ptr = data;
> @@ -513,10 +491,10 @@ static void create_eir(struct hci_dev *hdev, u8 *data)
>  	list_for_each_entry(uuid, &hdev->uuids, list) {
>  		u16 uuid16;
>  
> -		uuid16 = get_uuid16(uuid->uuid);
> -		if (uuid16 == 0)
> -			return;
> +		if (uuid->type != BT_UUID16)
> +			continue;
>  
> +		uuid16 = get_unaligned_le16(&uuid->uuid[12]);
>  		if (uuid16 < 0x1100)
>  			continue;
>  
> @@ -1304,6 +1282,25 @@ unlock:
>  	return err;
>  }
>  
> +static u8 bluetooth_base_uuid[] = {
> +			0xfb, 0x34, 0x9b, 0x5f, 0x80, 0x00, 0x00, 0x80,
> +			0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +};
> +

Make this one const.

> +static u8 bt_uuid_type(u8 *uuid)
> +{

Any reason the parameter can not be const here.

> +	u32 val;
> +
> +	if (memcmp(uuid, bluetooth_base_uuid, 12))
> +		return BT_UUID128;
> +
> +	val = get_unaligned_le32(&uuid[12]);
> +	if (val > 0xffff)
> +		return BT_UUID32;
> +
> +	return BT_UUID16;
> +}
> +

And then called this get_uuid_size or something similar.

>  static int add_uuid(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
>  {
>  	struct mgmt_cp_add_uuid *cp = data;
> @@ -1329,6 +1326,7 @@ static int add_uuid(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
>  
>  	memcpy(uuid->uuid, cp->uuid, 16);
>  	uuid->svc_hint = cp->svc_hint;
> +	uuid->type = bt_uuid_type(cp->uuid);
>  
>  	list_add_tail(&uuid->list, &hdev->uuids);
>  

Regards

Marcel



^ permalink raw reply

* Re: [PATCH 2/9 v3] Bluetooth: Simplify UUIDs clearing code
From: Marcel Holtmann @ 2013-01-26 17:03 UTC (permalink / raw)
  To: Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <1359194924-3151-3-git-send-email-johan.hedberg@gmail.com>

Hi Johan,

> The code for clearing the UUIDs list can be simplified by using
> list_for_each_entry_safe instead of list_for_each_safe.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
>  net/bluetooth/hci_core.c |   10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)

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

Regards

Marcel



^ permalink raw reply

* Re: [PATCH 1/9 v3] Bluetooth: Store UUIDs in the same order that they were added
From: Marcel Holtmann @ 2013-01-26 17:03 UTC (permalink / raw)
  To: Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <1359194924-3151-2-git-send-email-johan.hedberg@gmail.com>

Hi Johan,

> We should be encoding UUIDs to the EIR data in the same order that they
> were added to the kernel, i.e. each UUID should be added to the end of
> the UUIDs list. This patch fixes the issue by using list_add_tail
> instead of list_add for storing the UUIDs.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
>  net/bluetooth/mgmt.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

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

Regards

Marcel



^ permalink raw reply

* [PATCH 9/9 v3] Bluetooth: Add support for 128-bit UUIDs in EIR data
From: Johan Hedberg @ 2013-01-26 10:08 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1359194924-3151-1-git-send-email-johan.hedberg@gmail.com>

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

This patch adds the necessary code for encoding a list of 128-bit UUIDs
into the EIR data.

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

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 4ebcfee..f5be685 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -510,6 +510,39 @@ static u8 *create_uuid32_list(struct hci_dev *hdev, u8 *data, ptrdiff_t len)
 	return ptr;
 }
 
+static u8 *create_uuid128_list(struct hci_dev *hdev, u8 *data, ptrdiff_t len)
+{
+	u8 *ptr = data, *uuids_start = NULL;
+	struct bt_uuid *uuid;
+
+	if (len < 18)
+		return ptr;
+
+	list_for_each_entry(uuid, &hdev->uuids, list) {
+		if (uuid->type != BT_UUID128)
+			continue;
+
+		if (!uuids_start) {
+			uuids_start = ptr;
+			uuids_start[0] = 1;
+			uuids_start[1] = EIR_UUID128_ALL;
+			ptr += 2;
+		}
+
+		/* Stop if not enough space to put next UUID */
+		if ((ptr - data) + 16 > len) {
+			uuids_start[1] = EIR_UUID128_SOME;
+			break;
+		}
+
+		memcpy(ptr, uuid->uuid, 16);
+		ptr += 16;
+		uuids_start[0] += 16;
+	}
+
+	return ptr;
+}
+
 static void create_eir(struct hci_dev *hdev, u8 *data)
 {
 	u8 *ptr = data;
@@ -555,6 +588,7 @@ static void create_eir(struct hci_dev *hdev, u8 *data)
 
 	ptr = create_uuid16_list(hdev, ptr, HCI_MAX_EIR_LENGTH - (ptr - data));
 	ptr = create_uuid32_list(hdev, ptr, HCI_MAX_EIR_LENGTH - (ptr - data));
+	ptr = create_uuid128_list(hdev, ptr, HCI_MAX_EIR_LENGTH - (ptr - data));
 }
 
 static int update_eir(struct hci_dev *hdev)
-- 
1.7.10.4


^ permalink raw reply related

* [PATCH 8/9 v3] Bluetooth: Add support for 32-bit UUIDs in EIR data
From: Johan Hedberg @ 2013-01-26 10:08 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1359194924-3151-1-git-send-email-johan.hedberg@gmail.com>

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

This patch adds the necessary code for inserting a list of 32-bit UUIDs
into the EIR data.

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

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index f142b9a..4ebcfee 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -477,6 +477,39 @@ static u8 *create_uuid16_list(struct hci_dev *hdev, u8 *data, ptrdiff_t len)
 	return ptr;
 }
 
+static u8 *create_uuid32_list(struct hci_dev *hdev, u8 *data, ptrdiff_t len)
+{
+	u8 *ptr = data, *uuids_start = NULL;
+	struct bt_uuid *uuid;
+
+	if (len < 6)
+		return ptr;
+
+	list_for_each_entry(uuid, &hdev->uuids, list) {
+		if (uuid->type != BT_UUID32)
+			continue;
+
+		if (!uuids_start) {
+			uuids_start = ptr;
+			uuids_start[0] = 1;
+			uuids_start[1] = EIR_UUID32_ALL;
+			ptr += 2;
+		}
+
+		/* Stop if not enough space to put next UUID */
+		if ((ptr - data) + sizeof(u32) > len) {
+			uuids_start[1] = EIR_UUID32_SOME;
+			break;
+		}
+
+		memcpy(ptr, &uuid->uuid[12], sizeof(u32));
+		ptr += sizeof(u32);
+		uuids_start[0] += sizeof(u32);
+	}
+
+	return ptr;
+}
+
 static void create_eir(struct hci_dev *hdev, u8 *data)
 {
 	u8 *ptr = data;
@@ -521,6 +554,7 @@ static void create_eir(struct hci_dev *hdev, u8 *data)
 	}
 
 	ptr = create_uuid16_list(hdev, ptr, HCI_MAX_EIR_LENGTH - (ptr - data));
+	ptr = create_uuid32_list(hdev, ptr, HCI_MAX_EIR_LENGTH - (ptr - data));
 }
 
 static int update_eir(struct hci_dev *hdev)
-- 
1.7.10.4


^ permalink raw reply related

* [PATCH 7/9 v3] Bluetooth: Refactor UUID-16 list generation into its own function
From: Johan Hedberg @ 2013-01-26 10:08 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1359194924-3151-1-git-send-email-johan.hedberg@gmail.com>

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

We will need to create three separate UUID lists in the EIR data (for
16, 32 and 128 bit UUIDs) so the code is easier to follow if each list
is generated in their own function.

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

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 3a74ded..f142b9a 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -435,11 +435,51 @@ static u32 get_current_settings(struct hci_dev *hdev)
 
 #define PNP_INFO_SVCLASS_ID		0x1200
 
+static u8 *create_uuid16_list(struct hci_dev *hdev, u8 *data, ptrdiff_t len)
+{
+	u8 *ptr = data, *uuids_start = NULL;
+	struct bt_uuid *uuid;
+
+	if (len < 4)
+		return ptr;
+
+	list_for_each_entry(uuid, &hdev->uuids, list) {
+		u16 uuid16;
+
+		if (uuid->type != BT_UUID16)
+			continue;
+
+		uuid16 = get_unaligned_le16(&uuid->uuid[12]);
+		if (uuid16 < 0x1100)
+			continue;
+
+		if (uuid16 == PNP_INFO_SVCLASS_ID)
+			continue;
+
+		if (!uuids_start) {
+			uuids_start = ptr;
+			uuids_start[0] = 1;
+			uuids_start[1] = EIR_UUID16_ALL;
+			ptr += 2;
+		}
+
+		/* Stop if not enough space to put next UUID */
+		if ((ptr - data) + sizeof(u16) > len) {
+			uuids_start[1] = EIR_UUID16_SOME;
+			break;
+		}
+
+		*ptr++ = (uuid16 & 0x00ff);
+		*ptr++ = (uuid16 & 0xff00) >> 8;
+		uuids_start[0] += sizeof(uuid16);
+	}
+
+	return ptr;
+}
+
 static void create_eir(struct hci_dev *hdev, u8 *data)
 {
 	u8 *ptr = data;
-	u8 *uuids_start;
-	struct bt_uuid *uuid;
 	size_t name_len;
 
 	name_len = strlen(hdev->dev_name);
@@ -480,39 +520,7 @@ static void create_eir(struct hci_dev *hdev, u8 *data)
 		ptr += 10;
 	}
 
-	uuids_start = NULL;
-
-	/* Group all UUID16 types */
-	list_for_each_entry(uuid, &hdev->uuids, list) {
-		u16 uuid16;
-
-		if (uuid->type != BT_UUID16)
-			continue;
-
-		uuid16 = get_unaligned_le16(&uuid->uuid[12]);
-		if (uuid16 < 0x1100)
-			continue;
-
-		if (uuid16 == PNP_INFO_SVCLASS_ID)
-			continue;
-
-		if (!uuids_start) {
-			uuids_start = ptr;
-			uuids_start[0] = 1;
-			uuids_start[1] = EIR_UUID16_ALL;
-			ptr += 2;
-		}
-
-		/* Stop if not enough space to put next UUID */
-		if ((ptr - data) + 2 + sizeof(u16) > HCI_MAX_EIR_LENGTH) {
-			uuids_start[1] = EIR_UUID16_SOME;
-			break;
-		}
-
-		*ptr++ = (uuid16 & 0x00ff);
-		*ptr++ = (uuid16 & 0xff00) >> 8;
-		uuids_start[0] += sizeof(uuid16);
-	}
+	ptr = create_uuid16_list(hdev, ptr, HCI_MAX_EIR_LENGTH - (ptr - data));
 }
 
 static int update_eir(struct hci_dev *hdev)
-- 
1.7.10.4


^ permalink raw reply related

* [PATCH 6/9 v3] Bluetooth: Remove useless eir_len variable from EIR creation
From: Johan Hedberg @ 2013-01-26 10:08 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1359194924-3151-1-git-send-email-johan.hedberg@gmail.com>

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

The amount of data encoded so far in the create_eir() function can be
calculated simply through the difference between the data and ptr
pointer variables. The eir_len variable then becomes essentially
useless.

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

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index bf0f32e..3a74ded 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -439,7 +439,6 @@ static void create_eir(struct hci_dev *hdev, u8 *data)
 {
 	u8 *ptr = data;
 	u8 *uuids_start;
-	u16 eir_len = 0;
 	struct bt_uuid *uuid;
 	size_t name_len;
 
@@ -458,7 +457,6 @@ static void create_eir(struct hci_dev *hdev, u8 *data)
 
 		memcpy(ptr + 2, hdev->dev_name, name_len);
 
-		eir_len += (name_len + 2);
 		ptr += (name_len + 2);
 	}
 
@@ -467,7 +465,6 @@ static void create_eir(struct hci_dev *hdev, u8 *data)
 		ptr[1] = EIR_TX_POWER;
 		ptr[2] = (u8) hdev->inq_tx_power;
 
-		eir_len += 3;
 		ptr += 3;
 	}
 
@@ -480,7 +477,6 @@ static void create_eir(struct hci_dev *hdev, u8 *data)
 		put_unaligned_le16(hdev->devid_product, ptr + 6);
 		put_unaligned_le16(hdev->devid_version, ptr + 8);
 
-		eir_len += 10;
 		ptr += 10;
 	}
 
@@ -505,18 +501,16 @@ static void create_eir(struct hci_dev *hdev, u8 *data)
 			uuids_start[0] = 1;
 			uuids_start[1] = EIR_UUID16_ALL;
 			ptr += 2;
-			eir_len += 2;
 		}
 
 		/* Stop if not enough space to put next UUID */
-		if (eir_len + 2 + sizeof(u16) > HCI_MAX_EIR_LENGTH) {
+		if ((ptr - data) + 2 + sizeof(u16) > HCI_MAX_EIR_LENGTH) {
 			uuids_start[1] = EIR_UUID16_SOME;
 			break;
 		}
 
 		*ptr++ = (uuid16 & 0x00ff);
 		*ptr++ = (uuid16 & 0xff00) >> 8;
-		eir_len += sizeof(uuid16);
 		uuids_start[0] += sizeof(uuid16);
 	}
 }
-- 
1.7.10.4


^ permalink raw reply related

* [PATCH 5/9 v3] Bluetooth: Simplify UUID16 list generation for EIR
From: Johan Hedberg @ 2013-01-26 10:08 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1359194924-3151-1-git-send-email-johan.hedberg@gmail.com>

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

There's no need to use two separate loops to generate a UUID list for
the EIR data. This patch merges the two loops previously used for the
16-bit UUID list generation into a single loop, thus simplifying the
code a great deal.

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

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index ea8d74a..bf0f32e 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -438,9 +438,8 @@ static u32 get_current_settings(struct hci_dev *hdev)
 static void create_eir(struct hci_dev *hdev, u8 *data)
 {
 	u8 *ptr = data;
+	u8 *uuids_start;
 	u16 eir_len = 0;
-	u16 uuid16_list[HCI_MAX_EIR_LENGTH / sizeof(u16)];
-	int i, truncated = 0;
 	struct bt_uuid *uuid;
 	size_t name_len;
 
@@ -485,7 +484,7 @@ static void create_eir(struct hci_dev *hdev, u8 *data)
 		ptr += 10;
 	}
 
-	memset(uuid16_list, 0, sizeof(uuid16_list));
+	uuids_start = NULL;
 
 	/* Group all UUID16 types */
 	list_for_each_entry(uuid, &hdev->uuids, list) {
@@ -501,39 +500,24 @@ static void create_eir(struct hci_dev *hdev, u8 *data)
 		if (uuid16 == PNP_INFO_SVCLASS_ID)
 			continue;
 
+		if (!uuids_start) {
+			uuids_start = ptr;
+			uuids_start[0] = 1;
+			uuids_start[1] = EIR_UUID16_ALL;
+			ptr += 2;
+			eir_len += 2;
+		}
+
 		/* Stop if not enough space to put next UUID */
 		if (eir_len + 2 + sizeof(u16) > HCI_MAX_EIR_LENGTH) {
-			truncated = 1;
+			uuids_start[1] = EIR_UUID16_SOME;
 			break;
 		}
 
-		/* Check for duplicates */
-		for (i = 0; uuid16_list[i] != 0; i++)
-			if (uuid16_list[i] == uuid16)
-				break;
-
-		if (uuid16_list[i] == 0) {
-			uuid16_list[i] = uuid16;
-			eir_len += sizeof(u16);
-		}
-	}
-
-	if (uuid16_list[0] != 0) {
-		u8 *length = ptr;
-
-		/* EIR Data type */
-		ptr[1] = truncated ? EIR_UUID16_SOME : EIR_UUID16_ALL;
-
-		ptr += 2;
-		eir_len += 2;
-
-		for (i = 0; uuid16_list[i] != 0; i++) {
-			*ptr++ = (uuid16_list[i] & 0x00ff);
-			*ptr++ = (uuid16_list[i] & 0xff00) >> 8;
-		}
-
-		/* EIR Data length */
-		*length = (i * sizeof(u16)) + 1;
+		*ptr++ = (uuid16 & 0x00ff);
+		*ptr++ = (uuid16 & 0xff00) >> 8;
+		eir_len += sizeof(uuid16);
+		uuids_start[0] += sizeof(uuid16);
 	}
 }
 
-- 
1.7.10.4


^ permalink raw reply related

* [PATCH 4/9 v3] Bluetooth: Simplify UUID removal code
From: Johan Hedberg @ 2013-01-26 10:08 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1359194924-3151-1-git-send-email-johan.hedberg@gmail.com>

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

The UUID removal code can be simplified by using
list_for_each_entry_safe instead of list_for_each_safe.

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

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index d9f6b2c..ea8d74a 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1372,7 +1372,7 @@ static int remove_uuid(struct sock *sk, struct hci_dev *hdev, void *data,
 {
 	struct mgmt_cp_remove_uuid *cp = data;
 	struct pending_cmd *cmd;
-	struct list_head *p, *n;
+	struct bt_uuid *match, *tmp;
 	u8 bt_uuid_any[] = { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 };
 	int err, found;
 
@@ -1400,9 +1400,7 @@ static int remove_uuid(struct sock *sk, struct hci_dev *hdev, void *data,
 
 	found = 0;
 
-	list_for_each_safe(p, n, &hdev->uuids) {
-		struct bt_uuid *match = list_entry(p, struct bt_uuid, list);
-
+	list_for_each_entry_safe(match, tmp, &hdev->uuids, list) {
 		if (memcmp(match->uuid, cp->uuid, 16) != 0)
 			continue;
 
-- 
1.7.10.4


^ permalink raw reply related

* [PATCH 3/9 v3] Bluetooth: Keep track of UUID type upon addition
From: Johan Hedberg @ 2013-01-26 10:08 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1359194924-3151-1-git-send-email-johan.hedberg@gmail.com>

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

The primary purpose of the UUIDs is to eable generation of EIR and AD
data. In these data formats the UUIDs are split into separate fields
based on whether they're 16, 32 or 128 bit UUIDs. To make the generation
of these data fields simpler this patch adds a type member to the
bt_uuid struct and assigns a value to it as soon as the UUID is added to
the kernel. This way the type doesn't need to be calculated each time
the UUID list is later iterated.

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

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index bcf8ffe..6ed91ef 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -83,9 +83,14 @@ struct bdaddr_list {
 	bdaddr_t bdaddr;
 };
 
+#define BT_UUID16	1
+#define BT_UUID32	2
+#define BT_UUID128	3
+
 struct bt_uuid {
 	struct list_head list;
 	u8 uuid[16];
+	u8 type;
 	u8 svc_hint;
 };
 
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 4fd45a3..d9f6b2c 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -435,28 +435,6 @@ static u32 get_current_settings(struct hci_dev *hdev)
 
 #define PNP_INFO_SVCLASS_ID		0x1200
 
-static u8 bluetooth_base_uuid[] = {
-			0xFB, 0x34, 0x9B, 0x5F, 0x80, 0x00, 0x00, 0x80,
-			0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-};
-
-static u16 get_uuid16(u8 *uuid128)
-{
-	u32 val;
-	int i;
-
-	for (i = 0; i < 12; i++) {
-		if (bluetooth_base_uuid[i] != uuid128[i])
-			return 0;
-	}
-
-	val = get_unaligned_le32(&uuid128[12]);
-	if (val > 0xffff)
-		return 0;
-
-	return (u16) val;
-}
-
 static void create_eir(struct hci_dev *hdev, u8 *data)
 {
 	u8 *ptr = data;
@@ -513,10 +491,10 @@ static void create_eir(struct hci_dev *hdev, u8 *data)
 	list_for_each_entry(uuid, &hdev->uuids, list) {
 		u16 uuid16;
 
-		uuid16 = get_uuid16(uuid->uuid);
-		if (uuid16 == 0)
-			return;
+		if (uuid->type != BT_UUID16)
+			continue;
 
+		uuid16 = get_unaligned_le16(&uuid->uuid[12]);
 		if (uuid16 < 0x1100)
 			continue;
 
@@ -1304,6 +1282,25 @@ unlock:
 	return err;
 }
 
+static u8 bluetooth_base_uuid[] = {
+			0xfb, 0x34, 0x9b, 0x5f, 0x80, 0x00, 0x00, 0x80,
+			0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+};
+
+static u8 bt_uuid_type(u8 *uuid)
+{
+	u32 val;
+
+	if (memcmp(uuid, bluetooth_base_uuid, 12))
+		return BT_UUID128;
+
+	val = get_unaligned_le32(&uuid[12]);
+	if (val > 0xffff)
+		return BT_UUID32;
+
+	return BT_UUID16;
+}
+
 static int add_uuid(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
 {
 	struct mgmt_cp_add_uuid *cp = data;
@@ -1329,6 +1326,7 @@ static int add_uuid(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
 
 	memcpy(uuid->uuid, cp->uuid, 16);
 	uuid->svc_hint = cp->svc_hint;
+	uuid->type = bt_uuid_type(cp->uuid);
 
 	list_add_tail(&uuid->list, &hdev->uuids);
 
-- 
1.7.10.4


^ permalink raw reply related

* [PATCH 2/9 v3] Bluetooth: Simplify UUIDs clearing code
From: Johan Hedberg @ 2013-01-26 10:08 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1359194924-3151-1-git-send-email-johan.hedberg@gmail.com>

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

The code for clearing the UUIDs list can be simplified by using
list_for_each_entry_safe instead of list_for_each_safe.

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

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index e061b35..618ca1a 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1183,14 +1183,10 @@ static void hci_discov_off(struct work_struct *work)
 
 int hci_uuids_clear(struct hci_dev *hdev)
 {
-	struct list_head *p, *n;
-
-	list_for_each_safe(p, n, &hdev->uuids) {
-		struct bt_uuid *uuid;
+	struct bt_uuid *uuid, *tmp;
 
-		uuid = list_entry(p, struct bt_uuid, list);
-
-		list_del(p);
+	list_for_each_entry_safe(uuid, tmp, &hdev->uuids, list) {
+		list_del(&uuid->list);
 		kfree(uuid);
 	}
 
-- 
1.7.10.4


^ permalink raw reply related

* [PATCH 1/9 v3] Bluetooth: Store UUIDs in the same order that they were added
From: Johan Hedberg @ 2013-01-26 10:08 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1359194924-3151-1-git-send-email-johan.hedberg@gmail.com>

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

We should be encoding UUIDs to the EIR data in the same order that they
were added to the kernel, i.e. each UUID should be added to the end of
the UUIDs list. This patch fixes the issue by using list_add_tail
instead of list_add for storing the UUIDs.

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

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index e7f944f..4fd45a3 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1330,7 +1330,7 @@ static int add_uuid(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
 	memcpy(uuid->uuid, cp->uuid, 16);
 	uuid->svc_hint = cp->svc_hint;
 
-	list_add(&uuid->list, &hdev->uuids);
+	list_add_tail(&uuid->list, &hdev->uuids);
 
 	err = update_class(hdev);
 	if (err < 0)
-- 
1.7.10.4


^ permalink raw reply related

* [PATCH 0/9 v3] Bluetooth: Add 32 and 128 bit EIR UUID support
From: Johan Hedberg @ 2013-01-26 10:08 UTC (permalink / raw)
  To: linux-bluetooth

Hi,

Here's a third revision of the patch set to add 32 and 128 bit UUID support.

The main difference is merging back the different UUID lists to one (as
per Marcel's wish) and instead adding a type member to the bt_uuid
struct so that the type doesn't need to be evaluated multiple times (due
to the three iterations of the hdev->uuids list that now needs to be
done).

The added/removed statistics ended up (funnily enough) giving the exact
same result as v2 of this patch set, i.e. the code base ends up growing
51 lines (insertions - deletions).

Johan Hedberg (9):
      Bluetooth: Store UUIDs in the same order that they were added
      Bluetooth: Simplify UUIDs clearing code
      Bluetooth: Keep track of UUID type upon addition
      Bluetooth: Simplify UUID removal code
      Bluetooth: Simplify UUID16 list generation for EIR
      Bluetooth: Remove useless eir_len variable from EIR creation
      Bluetooth: Refactor UUID-16 list generation into its own function
      Bluetooth: Add support for 32-bit UUIDs in EIR data
      Bluetooth: Add support for 128-bit UUIDs in EIR data

 include/net/bluetooth/hci_core.h |    5 ++++
 net/bluetooth/hci_core.c         |   10 +++-----
 net/bluetooth/mgmt.c             |  202 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------------------------------------------------
 3 files changed, 134 insertions(+), 83 deletions(-)



^ permalink raw reply

* Re: [PATCH 2/2] Bluetooth: Rename hci_acl_disconn
From: Marcel Holtmann @ 2013-01-26  3:30 UTC (permalink / raw)
  To: Andre Guedes; +Cc: linux-bluetooth
In-Reply-To: <1359148426-22271-2-git-send-email-andre.guedes@openbossa.org>

Hi Andre,

> As hci_acl_disconn function basically sends the HCI Disconnect Command
> and it is used to disconnect both ACL and LE links, renaming it to
> hci_disconnect is more suitable.
> 
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
>  include/net/bluetooth/hci_core.h | 2 +-
>  net/bluetooth/hci_conn.c         | 4 ++--
>  net/bluetooth/hci_core.c         | 2 +-
>  net/bluetooth/hci_event.c        | 4 ++--
>  4 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index bcf8ffe..def944d 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -573,7 +573,7 @@ static inline struct hci_conn *hci_conn_hash_lookup_state(struct hci_dev *hdev,
>  	return NULL;
>  }
>  
> -void hci_acl_disconn(struct hci_conn *conn, __u8 reason);
> +void hci_disconnect(struct hci_conn *conn, __u8 reason);
>  void hci_setup_sync(struct hci_conn *conn, __u16 handle);
>  void hci_sco_setup(struct hci_conn *conn, __u8 status);

a better name might be hci_conn_disconnect, but I do realize that
overlaps with the timeout callback. And we have not been really super
strict with the function name prefixes anyway.

Regards

Marcel



^ permalink raw reply

* Re: [PATCH 1/2] Bluetooth: Fix L2CAP socket shutdown for LE connections
From: Marcel Holtmann @ 2013-01-26  3:28 UTC (permalink / raw)
  To: Andre Guedes; +Cc: linux-bluetooth
In-Reply-To: <1359148426-22271-1-git-send-email-andre.guedes@openbossa.org>

Hi Andre,

> During the L2CAP socket shutdown, the LE connection is not terminated
> as expected. This bug can be reproduced using l2test tool. Once the
> LE connection is established, kill l2test and the LE connection will
> not terminate.
> 
> This patch fixes hci_conn_disconnect function so it is able to
> terminate LE connections.
> 
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
>  net/bluetooth/hci_conn.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index 25bfce0..0492949 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -250,6 +250,7 @@ static void hci_conn_disconnect(struct hci_conn *conn)
>  
>  	switch (conn->type) {
>  	case ACL_LINK:
> +	case LE_LINK:
>  		hci_acl_disconn(conn, reason);
>  		break;
>  	case AMP_LINK:

I am wondering if we are not missing SCO_LINK here either.

Regards

Marcel



^ permalink raw reply

* Re: What is involved in supporting SCO packets in HCI_RAW mode?
From: Marcel Holtmann @ 2013-01-26  3:23 UTC (permalink / raw)
  To: Eponymous -; +Cc: linux-bluetooth
In-Reply-To: <CAJ+AEyM4aZY80+Eo7g5=z+3t2fZB+n0rW6a02p8G2CJZ9JvAQQ@mail.gmail.com>

Hi,

> After the patch was added to btusb.c to allow ACL packets to be
> sent/received in HCI_RAW mode I was wondering what would be involved
> in allowing raw SCO packets to be sent/received?
> 
> This would be a useful feature for someone like me who works with
> Bluetooth firmware at this level on a daily basis.
> 
> However, I can understand there may be some limitations.
> 
> Is this something that can be realistically done?

I do not see a way to do this properly. The main problem here is the USB
transport. It requires special settings and active URBs.

Once you switch it to HCI_RAW mode, the Bluetooth host stack does no
command/event tracking and with that no connection tracking anymore.
This means it can not inform the driver about any changes. And for USB
it is required to change its alternate setting based on the number of
connections and start ISOC URBs. Having ISOC URBs submitted all the time
is not feasible either since that consumes way too much power.

For other transports like UART, the SCO packets in HCI_RAW mode should
still work.

Regards

Marcel



^ permalink raw reply

* What is involved in supporting SCO packets in HCI_RAW mode?
From: Eponymous - @ 2013-01-26  0:21 UTC (permalink / raw)
  To: linux-bluetooth

Hi all,

After the patch was added to btusb.c to allow ACL packets to be
sent/received in HCI_RAW mode I was wondering what would be involved
in allowing raw SCO packets to be sent/received?

This would be a useful feature for someone like me who works with
Bluetooth firmware at this level on a daily basis.

However, I can understand there may be some limitations.

Is this something that can be realistically done?

Thanks.

^ permalink raw reply

* [PATCH 2/2] Bluetooth: Rename hci_acl_disconn
From: Andre Guedes @ 2013-01-25 21:13 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1359148426-22271-1-git-send-email-andre.guedes@openbossa.org>

As hci_acl_disconn function basically sends the HCI Disconnect Command
and it is used to disconnect both ACL and LE links, renaming it to
hci_disconnect is more suitable.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 include/net/bluetooth/hci_core.h | 2 +-
 net/bluetooth/hci_conn.c         | 4 ++--
 net/bluetooth/hci_core.c         | 2 +-
 net/bluetooth/hci_event.c        | 4 ++--
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index bcf8ffe..def944d 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -573,7 +573,7 @@ static inline struct hci_conn *hci_conn_hash_lookup_state(struct hci_dev *hdev,
 	return NULL;
 }
 
-void hci_acl_disconn(struct hci_conn *conn, __u8 reason);
+void hci_disconnect(struct hci_conn *conn, __u8 reason);
 void hci_setup_sync(struct hci_conn *conn, __u16 handle);
 void hci_sco_setup(struct hci_conn *conn, __u8 status);
 
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 0492949..e1ea2c9 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -117,7 +117,7 @@ static void hci_acl_create_connection_cancel(struct hci_conn *conn)
 	hci_send_cmd(conn->hdev, HCI_OP_CREATE_CONN_CANCEL, sizeof(cp), &cp);
 }
 
-void hci_acl_disconn(struct hci_conn *conn, __u8 reason)
+void hci_disconnect(struct hci_conn *conn, __u8 reason)
 {
 	struct hci_cp_disconnect cp;
 
@@ -251,7 +251,7 @@ static void hci_conn_disconnect(struct hci_conn *conn)
 	switch (conn->type) {
 	case ACL_LINK:
 	case LE_LINK:
-		hci_acl_disconn(conn, reason);
+		hci_disconnect(conn, reason);
 		break;
 	case AMP_LINK:
 		hci_amp_disconn(conn, reason);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index e061b35..5c38ce9 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2402,7 +2402,7 @@ static void hci_link_tx_to(struct hci_dev *hdev, __u8 type)
 		if (c->type == type && c->sent) {
 			BT_ERR("%s killing stalled connection %pMR",
 			       hdev->name, &c->dst);
-			hci_acl_disconn(c, HCI_ERROR_REMOTE_USER_TERM);
+			hci_disconnect(c, HCI_ERROR_REMOTE_USER_TERM);
 		}
 	}
 
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index d4fcba6..5c78480 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -2399,7 +2399,7 @@ static void hci_encrypt_change_evt(struct hci_dev *hdev, struct sk_buff *skb)
 		clear_bit(HCI_CONN_ENCRYPT_PEND, &conn->flags);
 
 		if (ev->status && conn->state == BT_CONNECTED) {
-			hci_acl_disconn(conn, HCI_ERROR_AUTH_FAILURE);
+			hci_disconnect(conn, HCI_ERROR_AUTH_FAILURE);
 			hci_conn_put(conn);
 			goto unlock;
 		}
@@ -3472,7 +3472,7 @@ static void hci_key_refresh_complete_evt(struct hci_dev *hdev,
 	clear_bit(HCI_CONN_ENCRYPT_PEND, &conn->flags);
 
 	if (ev->status && conn->state == BT_CONNECTED) {
-		hci_acl_disconn(conn, HCI_ERROR_AUTH_FAILURE);
+		hci_disconnect(conn, HCI_ERROR_AUTH_FAILURE);
 		hci_conn_put(conn);
 		goto unlock;
 	}
-- 
1.8.1.1


^ permalink raw reply related

* [PATCH 1/2] Bluetooth: Fix L2CAP socket shutdown for LE connections
From: Andre Guedes @ 2013-01-25 21:13 UTC (permalink / raw)
  To: linux-bluetooth

During the L2CAP socket shutdown, the LE connection is not terminated
as expected. This bug can be reproduced using l2test tool. Once the
LE connection is established, kill l2test and the LE connection will
not terminate.

This patch fixes hci_conn_disconnect function so it is able to
terminate LE connections.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 net/bluetooth/hci_conn.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 25bfce0..0492949 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -250,6 +250,7 @@ static void hci_conn_disconnect(struct hci_conn *conn)
 
 	switch (conn->type) {
 	case ACL_LINK:
+	case LE_LINK:
 		hci_acl_disconn(conn, reason);
 		break;
 	case AMP_LINK:
-- 
1.8.1.1


^ permalink raw reply related

* [PATCH BlueZ] avctp: Handle Vol Up/Down operations
From: João Paulo Rechi Vita @ 2013-01-25 17:54 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: luiz.dentz, João Paulo Rechi Vita

The AVRCP spec mandates to support 'volume up' and 'volume down'
operations when claiming support for Category 2 TG.
---
 profiles/audio/avctp.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/profiles/audio/avctp.c b/profiles/audio/avctp.c
index d2dce3e..28c414d 100644
--- a/profiles/audio/avctp.c
+++ b/profiles/audio/avctp.c
@@ -213,6 +213,8 @@ static struct {
 	uint8_t avc;
 	uint16_t uinput;
 } key_map[] = {
+	{ "VOLUME UP",		AVC_VOLUME_UP,		KEY_VOLUMEUP},
+	{ "VOLUME DOWN",	AVC_VOLUME_DOWN,	KEY_VOLUMEDOWN},
 	{ "PLAY",		AVC_PLAY,		KEY_PLAYCD },
 	{ "STOP",		AVC_STOP,		KEY_STOPCD },
 	{ "PAUSE",		AVC_PAUSE,		KEY_PAUSECD },
-- 
1.7.11.7


^ permalink raw reply related

* Re: [PATCH BlueZ 1/2] avctp: Create ignore quirk
From: Joao Paulo Rechi Vita @ 2013-01-25 17:05 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <CABBYNZK7nK6LPq1rYv40fq_Ko_MB2f-ANG08ozHhSLyg_BUAMg@mail.gmail.com>

On Fri, Jan 25, 2013 at 10:35 AM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> Hi Joao,
>
> On Tue, Jan 22, 2013 at 8:18 PM, João Paulo Rechi Vita
> <jprvita@openbossa.org> wrote:
>> Create a quirk to be able to accept and trow away certain keys.
>> ---
>>  profiles/audio/avctp.c | 8 +++++++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/profiles/audio/avctp.c b/profiles/audio/avctp.c
>> index 61890cc..f7e607e 100644
>> --- a/profiles/audio/avctp.c
>> +++ b/profiles/audio/avctp.c
>> @@ -58,7 +58,8 @@
>>  #include "avctp.h"
>>  #include "avrcp.h"
>>
>> -#define QUIRK_NO_RELEASE 1 << 0
>> +#define QUIRK_NO_RELEASE       1 << 0
>> +#define QUIRK_IGNORE           1 << 1
>>
>>  /* Message types */
>>  #define AVCTP_COMMAND          0
>> @@ -287,6 +288,11 @@ static size_t handle_panel_passthrough(struct avctp *session,
>>
>>                 key_quirks = session->key_quirks[key_map[i].avc];
>>
>> +               if (key_quirks & QUIRK_IGNORE) {
>> +                       DBG("AV/C: ignoring %s %s", key_map[i].name, status);
>> +                       break;
>> +               }
>> +
>>                 if (key_quirks & QUIRK_NO_RELEASE) {
>>                         if (!pressed) {
>>                                 DBG("AV/C: Ignoring release");
>> --
>> 1.7.11.7
>
> In the end I think we should just accept the commands normally, let me
> quote the recommendations (RD=Rendering Device MP=Media Player):
>
>   "Recommendation 16:
>
>   If volume is changed on the RD, the RD should not send an AVRCP
> volume command to the MP device.
>
>   Motivation 16:
>
>   Sending an AVRCP volume command to the MP may cause the MP to send
> again an AVRCP volume
>   command to the RD device which could lead to an endless loop of
> AVRCP volume commands.
>
>   Recommendation 17:
>
>   If a device receives an AVRCP volume command, it shall not send back
> an AVRCP volume command.
>
>   Motivation 17:
>
>   This will also ensure that endless loop does not happen with
> existing devices which do not comply with the
>   recommendation."
>
> So there is nothing against the RD accepting the Volume Up/Down it
> should just no send it back.
>

All right, so I'll update the patches to forward the Volume Up and
Down to uinput.


--
João Paulo Rechi Vita
Openbossa Labs - INdT

^ permalink raw reply


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