Linux bluetooth development
 help / color / mirror / Atom feed
* Re: [PATCH] return value checking of e_book_async_get_contacts() was wrong.
From: Johan Hedberg @ 2010-08-09 20:55 UTC (permalink / raw)
  To: Marcel Mol; +Cc: linux-bluetooth
In-Reply-To: <201008092047.o79KlQDL015265@joshua.mesa.nl>

Hi Marcel,

On Mon, Aug 09, 2010, Marcel Mol wrote:
> phonebook_create_cache() failed because checking the return value
> e_book_async_get_contacts() was wrong. This would lead to a core dump
> as the data was freed but was still used in the callbacks.
> ---
>  plugins/phonebook-ebook.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/plugins/phonebook-ebook.c b/plugins/phonebook-ebook.c
> index 3c24107..1598d5f 100644
> --- a/plugins/phonebook-ebook.c
> +++ b/plugins/phonebook-ebook.c
> @@ -459,7 +459,7 @@ int phonebook_create_cache(const char *name, phonebook_entry_cb entry_cb,
>  
>  	ret = e_book_async_get_contacts(ebook, query, cache_cb, data);
>  	e_book_query_unref(query);
> -	if (ret == FALSE) {
> +	if (ret != FALSE) {
>  		g_free(data);
>  		return -EFAULT;
>  	}

Thanks for catching this. The patch is now upstream.

Johan

^ permalink raw reply

* [PATCH 0/2] Use HCI device type to determine suitability of HCI commands
From: David Scherba @ 2010-08-09 21:07 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: johan.hedberg, marcel, rshaffer

BlueZ assumes that HCI devices are either BR/EDR radios, or "RAW" (via
HCI_RAW).  At present, alternate radios are handled as RAW (reference
Marcel's 943da25d9 kernel commit).  More subtlety in BlueZ is useful as
AMP support is enhanced.

These patches remove the need to treat non-BR/EDR HCI devices as "RAW."

Cheers,
David

-- 
David Scherba
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum 

^ permalink raw reply

* [PATCH 1/2] Remove non-functional hci_devinfo calls in init_device()
From: David Scherba @ 2010-08-09 21:07 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: johan.hedberg, marcel, rshaffer, David Scherba
In-Reply-To: <1281388059-15945-1-git-send-email-dscherba@codeaurora.org>

---
 plugins/hciops.c |    6 ------
 1 files changed, 0 insertions(+), 6 deletions(-)

diff --git a/plugins/hciops.c b/plugins/hciops.c
index 5775cf1..b38c056 100644
--- a/plugins/hciops.c
+++ b/plugins/hciops.c
@@ -175,12 +175,6 @@ static void init_device(int index)
 		goto fail;
 	}
 
-	if (hci_devinfo(index, &di) < 0)
-		goto fail;
-
-	if (hci_test_bit(HCI_RAW, &di.flags))
-		goto done;
-
 done:
 	hci_close_dev(dd);
 	exit(0);
-- 
1.7.0
-- 
David Scherba
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.


^ permalink raw reply related

* [PATCH 2/2] Use HCI device type to gate BR/EDR-specific HCI commands
From: David Scherba @ 2010-08-09 21:07 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: johan.hedberg, marcel, rshaffer, David Scherba
In-Reply-To: <1281388059-15945-1-git-send-email-dscherba@codeaurora.org>

Use the hci_dev_info structure member 'type' to classify whether a HCI device
is BR/EDR, or not.  If not, gate BR/EDR-specific HCI commands.
---
 lib/hci_lib.h    |    5 +++++
 plugins/hciops.c |   23 +++++++++++++++--------
 src/adapter.c    |    3 ++-
 src/security.c   |    6 ++++--
 4 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/lib/hci_lib.h b/lib/hci_lib.h
index b63a2a4..c0786ab 100644
--- a/lib/hci_lib.h
+++ b/lib/hci_lib.h
@@ -169,6 +169,11 @@ static inline int hci_test_bit(int nr, void *addr)
 	return *((uint32_t *) addr + (nr >> 5)) & (1 << (nr & 31));
 }
 
+static inline int is_bredr_hci_device_type(uint8_t type)
+{
+  return (type >> 4) == HCI_BREDR;
+}
+
 /* HCI filter tools */
 static inline void hci_filter_clear(struct hci_filter *f)
 {
diff --git a/plugins/hciops.c b/plugins/hciops.c
index b38c056..705e9dd 100644
--- a/plugins/hciops.c
+++ b/plugins/hciops.c
@@ -85,7 +85,8 @@ static void device_devup_setup(int index)
 	if (hci_devinfo(index, &di) < 0)
 		return;
 
-	if (hci_test_bit(HCI_RAW, &di.flags))
+	if (!is_bredr_hci_device_type(di.type) ||
+	    hci_test_bit(HCI_RAW, &di.flags))
 		return;
 
 	dd = hci_open_dev(index);
@@ -160,12 +161,17 @@ static void init_device(int index)
 					index, strerror(err), err);
 	}
 
-	/* Set link policy */
-	dr.dev_opt = main_opts.link_policy;
-	if (ioctl(dd, HCISETLINKPOL, (unsigned long) &dr) < 0 &&
-							errno != ENETDOWN) {
-		error("Can't set link policy on hci%d: %s (%d)",
-					index, strerror(errno), errno);
+	/* Set link policy for BR/EDR HCI devices */
+	if (hci_devinfo(index, &di) < 0)
+		goto fail;
+
+	if (is_bredr_hci_device_type(di.type)) {
+		dr.dev_opt = main_opts.link_policy;
+		if (ioctl(dd, HCISETLINKPOL, (unsigned long) &dr) < 0 &&
+								errno != ENETDOWN) {
+			error("Can't set link policy on hci%d: %s (%d)",
+						index, strerror(errno), errno);
+		}
 	}
 
 	/* Start HCI device */
@@ -198,7 +204,8 @@ static void device_devreg_setup(int index)
 
 	devup = hci_test_bit(HCI_UP, &di.flags);
 
-	if (!hci_test_bit(HCI_RAW, &di.flags))
+	if (is_bredr_hci_device_type(di.type) &&
+	    !hci_test_bit(HCI_RAW, &di.flags))
 		manager_register_adapter(index, devup);
 }
 
diff --git a/src/adapter.c b/src/adapter.c
index 7d0e34d..acc1068 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2314,7 +2314,8 @@ int adapter_start(struct btd_adapter *adapter)
 	if (hci_devinfo(adapter->dev_id, &di) < 0)
 		return -errno;
 
-	if (hci_test_bit(HCI_RAW, &di.flags)) {
+	if (!is_bredr_hci_device_type(di.type) ||
+	    hci_test_bit(HCI_RAW, &di.flags)) {
 		dev->ignore = 1;
 		return -1;
 	}
diff --git a/src/security.c b/src/security.c
index ca394e1..6d07d86 100644
--- a/src/security.c
+++ b/src/security.c
@@ -999,7 +999,8 @@ static gboolean io_security_event(GIOChannel *chan, GIOCondition cond,
 
 	ioctl(dev, HCIGETDEVINFO, (void *) di);
 
-	if (hci_test_bit(HCI_RAW, &di->flags))
+	if (!is_bredr_hci_device_type(di->type) ||
+	    hci_test_bit(HCI_RAW, &di->flags))
 		return TRUE;
 
 	switch (eh->evt) {
@@ -1185,7 +1186,8 @@ void start_security_manager(int hdev)
 	io_data[hdev].channel = chan;
 	io_data[hdev].pin_length = -1;
 
-	if (hci_test_bit(HCI_RAW, &di->flags))
+	if (!is_bredr_hci_device_type(di->type) ||
+	    hci_test_bit(HCI_RAW, &di->flags))
 		return;
 
 	bacpy(&cp.bdaddr, BDADDR_ANY);
-- 
1.7.0
-- 
David Scherba
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

^ permalink raw reply related

* Re: RFC: QuIC's AMP + eL2CAP Technical Plans
From: Tim Monahan-Mitchell @ 2010-08-09 21:30 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: David Vrabel, tmonahan, linux-bluetooth
In-Reply-To: <1268072572.3712.24.camel@localhost.localdomain>

Hi David and Marcel,

I added on to this thread in regards to today's patch of "Bluetooth: HCI
devices are either BR/EDR or AMP radios" (09-Aug-2010).

>> > The block-based flow control is a missing piece, but the AMP type
>> > extension has been merged upstream. We can create HCI_BREDR and
>> > HCI_80211 controllers now. The AMP controllers are for now forced to
>> be
>> > raw devices, but that can be changed easily once we have the
>> controller
>> > init for AMP up and ready.
>>
>> Why HCI_80211 and not HCI_AMP?  The stack shouldn't care what the AMP's
>> radio actually is and with some devices (e.g., standard SDIO ones) it's
>> not even possible to tell what the radio is.
>
> we will be adding a HCI_UWB once that specification gets ratified. And
> the controller type is an official piece of information inside the AMP
> manager. That part needs to know which type of AMP it is. Also the
> driver actually should know what type of AMP it is.
>
> I see your point that for fully generic HCI transports they might not
> know the type upfront and we need to handle that. We will cross that
> bridge when we come to it. Right now we just got started.
>
> Regards
>
> Marcel

If the HCI_80211 symbol in hci.h is changed instead to HCI_AMP, where
should the 0x01 value for '802.11 AMP Controller' (listed in
Bluetooth.org's Assigned Number listing for Controller_Type) go?

Thanks,
Tim Monahan-Mitchell
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

^ permalink raw reply

* Re: [PATCH] Bluetooth: HCI devices are either BR/EDR or AMP radios
From: Marcel Holtmann @ 2010-08-09 21:39 UTC (permalink / raw)
  To: David Vrabel; +Cc: linux-bluetooth
In-Reply-To: <4C604F37.4020706@csr.com>

Hi David,

> HCI transport drivers may not know what type of radio an AMP device has
> so only say whether they're BR/EDR or AMP devices.
> 
> Signed-off-by: David Vrabel <david.vrabel@csr.com>
> ---
>   include/net/bluetooth/hci.h |    2 +-
>   net/bluetooth/hci_sysfs.c   |    4 ++--
>   2 files changed, 3 insertions(+), 3 deletions(-)

a little bit different version that also fixes the Marvell driver has
been applied to my tree. Thanks.

Regards

Marcel



^ permalink raw reply

* [PATCH] Bluetooth: btmrvl: devices are either BR/EDR or AMPs
From: David Vrabel @ 2010-08-09 21:43 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Marcel Holtmann

[-- Attachment #1: Type: text/plain, Size: 413 bytes --]


-- 
David Vrabel, Senior Software Engineer, Drivers
CSR, Churchill House, Cambridge Business Park,  Tel: +44 (0)1223 692562
Cowley Road, Cambridge, CB4 0WZ                 http://www.csr.com/


Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom

[-- Attachment #2: 0001-Bluetooth-btmrvl-devices-are-either-BR-EDR-or-AMPs.patch --]
[-- Type: text/x-diff, Size: 981 bytes --]

>From 5d16a92e61d8457077d0235d6b7006b824ff6891 Mon Sep 17 00:00:00 2001
From: David Vrabel <dv02@dv02-laptop.(none)>
Date: Mon, 9 Aug 2010 22:39:43 +0100
Subject: [PATCH] Bluetooth: btmrvl: devices are either BR/EDR or AMPs

Signed-off-by: David Vrabel <david.vrabel@csr.com>
---
 drivers/bluetooth/btmrvl_main.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/bluetooth/btmrvl_main.c b/drivers/bluetooth/btmrvl_main.c
index 0d32ec8..548d1d9 100644
--- a/drivers/bluetooth/btmrvl_main.c
+++ b/drivers/bluetooth/btmrvl_main.c
@@ -117,8 +117,8 @@ int btmrvl_process_event(struct btmrvl_private *priv, struct sk_buff *skb)
 				(event->data[2] == MODULE_ALREADY_UP)) ?
 				"Bring-up succeed" : "Bring-up failed");
 
-			if (event->length > 3)
-				priv->btmrvl_dev.dev_type = event->data[3];
+			if (event->length > 3 && event->data[3])
+				priv->btmrvl_dev.dev_type = HCI_AMP;
 			else
 				priv->btmrvl_dev.dev_type = HCI_BREDR;
 
-- 
1.5.4.3


^ permalink raw reply related

* Re: [PATCH] Bluetooth: Support SDIO devices that are AMP controllers
From: Marcel Holtmann @ 2010-08-09 21:44 UTC (permalink / raw)
  To: David Vrabel; +Cc: linux-bluetooth
In-Reply-To: <4C604FAD.40801@csr.com>

Hi David,

> Signed-off-by: David Vrabel <david.vrabel@csr.com>
> ---
>   drivers/bluetooth/btsdio.c   |    4 ++++
>   include/linux/mmc/sdio_ids.h |    1 +
>   2 files changed, 5 insertions(+), 0 deletions(-)

since your patch didn't apply cleanly, I re-wrote a little ;)

Regards

Marcel



^ permalink raw reply

* Re: [PATCH 1/2] Remove non-functional hci_devinfo calls in init_device()
From: Johan Hedberg @ 2010-08-09 21:51 UTC (permalink / raw)
  To: David Scherba; +Cc: linux-bluetooth, marcel, rshaffer
In-Reply-To: <1281388059-15945-2-git-send-email-dscherba@codeaurora.org>

Hi David,

On Mon, Aug 09, 2010, David Scherba wrote:
> ---
>  plugins/hciops.c |    6 ------
>  1 files changed, 0 insertions(+), 6 deletions(-)
> 
> diff --git a/plugins/hciops.c b/plugins/hciops.c
> index 5775cf1..b38c056 100644
> --- a/plugins/hciops.c
> +++ b/plugins/hciops.c
> @@ -175,12 +175,6 @@ static void init_device(int index)
>  		goto fail;
>  	}
>  
> -	if (hci_devinfo(index, &di) < 0)
> -		goto fail;
> -
> -	if (hci_test_bit(HCI_RAW, &di.flags))
> -		goto done;
> -
>  done:
>  	hci_close_dev(dd);
>  	exit(0);

Nice catch, but that's not quite enough:

plugins/hciops.c: In function ‘init_device’:
plugins/hciops.c:178: error: label ‘done’ defined but not used
plugins/hciops.c:125: error: unused variable ‘di’

Always check that your patch compiles cleanly with ./bootstrap-configure
before submitting upstream.

Johan

^ permalink raw reply

* Re: [PATCH 2/2] Use HCI device type to gate BR/EDR-specific HCI commands
From: Johan Hedberg @ 2010-08-09 21:59 UTC (permalink / raw)
  To: David Scherba; +Cc: linux-bluetooth, marcel, rshaffer
In-Reply-To: <1281388059-15945-3-git-send-email-dscherba@codeaurora.org>

Hi David,

On Mon, Aug 09, 2010, David Scherba wrote:
> +static inline int is_bredr_hci_device_type(uint8_t type)

Could you just call this ignore_device() which combines the AMP and RAW
checks.

> +{
> +  return (type >> 4) == HCI_BREDR;
> +}

Looks like the indentation is wrong inside the function (two spaces
instead of a tab).

> -	if (hci_test_bit(HCI_RAW, &di.flags))
> +	if (!is_bredr_hci_device_type(di.type) ||
> +	    hci_test_bit(HCI_RAW, &di.flags))

No spaces+tabs mixed indentation please. The second line should be
indented by at least two tabs more than the line above it (as many tabs
as you can as long as the line doesn't go beyond 79 characters). OTOH,
the second line goes away completely if you do the ignore_device
simplification as proposed above.

> -	if (!hci_test_bit(HCI_RAW, &di.flags))
> +	if (is_bredr_hci_device_type(di.type) &&
> +	    !hci_test_bit(HCI_RAW, &di.flags))

Same here.

> -	if (hci_test_bit(HCI_RAW, &di.flags)) {
> +	if (!is_bredr_hci_device_type(di.type) ||
> +	    hci_test_bit(HCI_RAW, &di.flags)) {

And here.

> -	if (hci_test_bit(HCI_RAW, &di->flags))
> +	if (!is_bredr_hci_device_type(di->type) ||
> +	    hci_test_bit(HCI_RAW, &di->flags))

And here.

> -	if (hci_test_bit(HCI_RAW, &di->flags))
> +	if (!is_bredr_hci_device_type(di->type) ||
> +	    hci_test_bit(HCI_RAW, &di->flags))

And here.

Johan

^ permalink raw reply

* Re: RFC: QuIC's AMP + eL2CAP Technical Plans
From: David Vrabel @ 2010-08-09 22:03 UTC (permalink / raw)
  To: tmonahan; +Cc: Marcel Holtmann, linux-bluetooth
In-Reply-To: <4fed618ae82fc803133cc742bbd2eeb6.squirrel@www.codeaurora.org>

Tim Monahan-Mitchell wrote:
> 
> If the HCI_80211 symbol in hci.h is changed instead to HCI_AMP, where
> should the 0x01 value for '802.11 AMP Controller' (listed in
> Bluetooth.org's Assigned Number listing for Controller_Type) go?e

I don't think it needs to go anywhere, the actual values aren't 
important from the point of view of the AMP manager.

David
-- 
David Vrabel, Senior Software Engineer, Drivers
CSR, Churchill House, Cambridge Business Park,  Tel: +44 (0)1223 692562
Cowley Road, Cambridge, CB4 0WZ                 http://www.csr.com/


Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom

^ permalink raw reply

* Re: RFC: QuIC's AMP + eL2CAP Technical Plans
From: David Vrabel @ 2010-08-09 22:12 UTC (permalink / raw)
  To: tmonahan; +Cc: Marcel Holtmann, linux-bluetooth
In-Reply-To: <4C607B43.2050207@csr.com>

David Vrabel wrote:
> Tim Monahan-Mitchell wrote:
>>
>> If the HCI_80211 symbol in hci.h is changed instead to HCI_AMP, where
>> should the 0x01 value for '802.11 AMP Controller' (listed in
>> Bluetooth.org's Assigned Number listing for Controller_Type) go?e
> 
> I don't think it needs to go anywhere, the actual values aren't 
> important from the point of view of the AMP manager.

I may have misunderstood your question.  There will need to be an 
additional "controller_type" field which will get filled in when the 
local amp info is read from the controller.

There will also need to be a table somewhere mapping controller types to 
dedicated amp link key length and KeyID.  The will be filled in from the 
assigned numbers and will be a very short table one 1 entry for now.

I do not think we need #define for the various AMP controller types.

David
-- 
David Vrabel, Senior Software Engineer, Drivers
CSR, Churchill House, Cambridge Business Park,  Tel: +44 (0)1223 692562
Cowley Road, Cambridge, CB4 0WZ                 http://www.csr.com/


Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom

^ permalink raw reply

* Re: RFC: QuIC's AMP + eL2CAP Technical Plans
From: Tim Monahan-Mitchell @ 2010-08-09 22:21 UTC (permalink / raw)
  To: David Vrabel; +Cc: tmonahan, Marcel Holtmann, linux-bluetooth
In-Reply-To: <4C607D4C.8090407@csr.com>

Hi, David.

> David Vrabel wrote:
>> Tim Monahan-Mitchell wrote:
>>>
>>> If the HCI_80211 symbol in hci.h is changed instead to HCI_AMP, where
>>> should the 0x01 value for '802.11 AMP Controller' (listed in
>>> Bluetooth.org's Assigned Number listing for Controller_Type) go?e
>>
>> I don't think it needs to go anywhere, the actual values aren't
>> important from the point of view of the AMP manager.
>
> I may have misunderstood your question.  There will need to be an
> additional "controller_type" field which will get filled in when the
> local amp info is read from the controller.
>
> There will also need to be a table somewhere mapping controller types to
> dedicated amp link key length and KeyID.  The will be filled in from the
> assigned numbers and will be a very short table one 1 entry for now.
>
> I do not think we need #define for the various AMP controller types.

I would think such a set of #defines are needed, so that everyone is on
the same page?

Put another way, if one device has an AMP of type 802.11, and a remote has
an AMP of type HCI_UWB, it would seem the AMP Manager needs to know not to
try to converse via AMP with that peer, no?

Also have a look at the Core Spec, under 'READ LOCAL AMP INFO COMMAND'.
The 'Controller_Type' list (of which there is only 802.11 now) will
grow...

Controller Type appears elsewhere in the A2MP protocol...

Sorry if I'm not following you completely.

-- 
Regards,
Tim Monahan-Mitchell
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.



^ permalink raw reply

* [PATCH] Fix multiple phone number problem in pull vcard
From: Radoslaw Jablonski @ 2010-08-10  7:23 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Radoslaw Jablonski

This fixes problem with pull vcard when contact has more than one home or
work number defined in tracker - more than one VCARD was generated in
response for pull vcard request. This was caused by nature of the data
retrieved from tracker - contact with multiple numbers set was returned as
many entries with identical id. Previously VCARDs was generated on the fly
- now added contact-data caching and checking for contact id. VCARD is now
generated when all responses of tracker were processed - and only one vcard
is returned for one contact entry.
---
 plugins/phonebook-tracker.c |  130 ++++++++++++++++++++++++++++++++++++-------
 1 files changed, 109 insertions(+), 21 deletions(-)

diff --git a/plugins/phonebook-tracker.c b/plugins/phonebook-tracker.c
index f2b9649..3a90d30 100644
--- a/plugins/phonebook-tracker.c
+++ b/plugins/phonebook-tracker.c
@@ -43,6 +43,9 @@
 #define TRACKER_RESOURCES_INTERFACE "org.freedesktop.Tracker1.Resources"
 
 #define TRACKER_DEFAULT_CONTACT_ME "http://www.semanticdesktop.org/ontologies/2007/03/22/nco#default-contact-me"
+#define CONTACTS_ID_COL 19
+#define PHONE_ID_HOME 0
+#define PHONE_ID_WORK 3
 
 #define CONTACTS_QUERY_ALL						\
 	"SELECT nco:phoneNumber(?h) nco:fullname(?c) "			\
@@ -275,13 +278,18 @@ struct pending_reply {
 	int num_fields;
 };
 
+struct contact_data {
+	char *id;
+	struct phonebook_contact *contact;
+};
+
 struct phonebook_data {
-	GString *vcards;
 	phonebook_cb cb;
 	void *user_data;
 	int index;
 	gboolean vcardentry;
 	const struct apparam_field *params;
+	GSList *contacts;
 };
 
 struct cache_data {
@@ -545,20 +553,103 @@ static void set_call_type(struct phonebook_contact *contact,
 	contact->datetime = iso8601_utc_to_localtime(datetime);
 }
 
+static struct phonebook_contact *find_contact(GSList *contacts, const char *id)
+{
+	GSList *l;
+	struct contact_data *c_data;
+
+	for (l = contacts; l; l = l->next) {
+		c_data = l->data;
+		if (g_strcmp0(c_data->id, id) == 0)
+			return c_data->contact;
+	}
+
+	return NULL;
+}
+
+static struct phonebook_number *find_phone(GSList *numbers, const char *phone,
+		int type)
+{
+	GSList *l;
+	struct phonebook_number *pb_num;
+
+	for (l = numbers; l; l = l->next) {
+		pb_num = l->data;
+		/* Returning phonebook number if phone values and type values
+		 * are equal */
+		if (g_strcmp0(pb_num->tel, phone) == 0 && pb_num->type == type)
+			return pb_num;
+	}
+
+	return NULL;
+}
+
+static void add_phone_number(struct phonebook_contact *contact,
+		const char *phone, int type)
+{
+	struct phonebook_number *number;
+
+	if (phone == NULL || strlen(phone) == 0)
+		return;
+
+	/* Not adding number if there is already added with the same value */
+	if (find_phone(contact->numbers, phone, type))
+		return;
+
+	number = g_new0(struct phonebook_number, 1);
+	number->tel = g_strdup(phone);
+	number->type = type;
+
+	contact->numbers = g_slist_append(contact->numbers, number);
+}
+
+static GString *gen_vcards(GSList *contacts,
+		const struct apparam_field *params)
+{
+	GSList *l;
+	GString *vcards;
+	struct contact_data *c_data;
+
+	vcards = g_string_new(NULL);
+
+	/* Generating VCARD string from contacts and freeing used contacts */
+	for (l = contacts; l; l = l->next) {
+		c_data = l->data;
+		phonebook_add_contact(vcards, c_data->contact,
+					params->filter, params->format);
+
+		g_free(c_data->id);
+		phonebook_contact_free(c_data->contact);
+	}
+
+	return vcards;
+}
+
 static void pull_contacts(char **reply, int num_fields, void *user_data)
 {
 	struct phonebook_data *data = user_data;
 	const struct apparam_field *params = data->params;
 	struct phonebook_contact *contact;
-	struct phonebook_number *number;
-	GString *vcards = data->vcards;
+	struct contact_data *contact_data;
+	GString *vcards;
 	int last_index, i;
+	gboolean cdata_present = FALSE;
 
 	DBG("reply %p", reply);
 
 	if (reply == NULL)
 		goto done;
 
+	/* Trying to find contact in recently added contacts. It is needed for
+	 * contacts that have more than one telephone number filled */
+	contact = find_contact(data->contacts, reply[CONTACTS_ID_COL]);
+
+	/* If contact is already created then adding only new phone numbers */
+	if (contact) {
+		cdata_present = TRUE;
+		goto add_numbers;
+	}
+
 	/* We are doing a PullvCardEntry, no need for those checks */
 	if (data->vcardentry)
 		goto add_entry;
@@ -603,33 +694,32 @@ add_entry:
 
 	set_call_type(contact, reply[16], reply[17], reply[18]);
 
-	number = g_new0(struct phonebook_number, 1);
-	number->tel = g_strdup(reply[0]);
-	number->type = 0; /* HOME */
-
-	contact->numbers = g_slist_append(contact->numbers, number);
-
-	/* Has WORK Phonenumber */
-	if (strlen(reply[8])) {
-		number = g_new0(struct phonebook_number, 1);
-		number->tel = g_strdup(reply[8]);
-		number->type = 3; /* WORK */
-
-		contact->numbers = g_slist_append(contact->numbers, number);
-	}
+add_numbers:
+	/* Adding phone numbers to contact struct */
+	add_phone_number(contact, reply[0], PHONE_ID_HOME);
+	add_phone_number(contact, reply[8], PHONE_ID_WORK);
 
 	DBG("contact %p", contact);
 
-	phonebook_add_contact(vcards, contact, params->filter, params->format);
-	phonebook_contact_free(contact);
+	/* Adding contacts data to wrapper struct - this data will be used to
+	 * generate vcard list */
+	if (!cdata_present) {
+		contact_data = g_new0(struct contact_data, 1);
+		contact_data->contact = contact;
+		contact_data->id = g_strdup(reply[CONTACTS_ID_COL]);
+		data->contacts = g_slist_append(data->contacts, contact_data);
+	}
 
 	return;
 
 done:
+	vcards = gen_vcards(data->contacts, params);
+
 	if (num_fields == 0)
 		data->cb(vcards->str, vcards->len, data->index, 0,
 							data->user_data);
 
+	g_slist_free(data->contacts);
 	g_string_free(vcards, TRUE);
 	g_free(data);
 }
@@ -775,7 +865,6 @@ int phonebook_pull(const char *name, const struct apparam_field *params,
 		return -ENOENT;
 
 	data = g_new0(struct phonebook_data, 1);
-	data->vcards = g_string_new(NULL);
 	data->params = params;
 	data->user_data = user_data;
 	data->cb = cb;
@@ -794,7 +883,6 @@ int phonebook_get_entry(const char *folder, const char *id,
 	DBG("folder %s id %s", folder, id);
 
 	data = g_new0(struct phonebook_data, 1);
-	data->vcards = g_string_new(NULL);
 	data->user_data = user_data;
 	data->params = params;
 	data->cb = cb;
-- 
1.7.0.4


^ permalink raw reply related

* Re: Question about BlueZ licenses (LGPL and Apache)
From: Luiz Augusto von Dentz @ 2010-08-10  7:49 UTC (permalink / raw)
  To: Pavan Savoy; +Cc: Madhavi Manchala, linux-bluetooth
In-Reply-To: <AANLkTikFQiEEVKA=9UnxzeunAPEviWQnjYcH=q6rfhjA@mail.gmail.com>

Hi,

On Mon, Aug 9, 2010 at 10:37 PM, Pavan Savoy <pavan_savoy@sify.com> wrote:
> As long as we are updating, can the liba2dp be abstracted out of not
> only the sbc encoding part but also the sending over UART part (as in
> transport ...)
>
> chips are getting smarter and the sbc codecs are moving to either a
> co-processor/dsp or even inside the BT chip itself, so we might have
> to send the PCM raw data somewhere else for the dsp to encode to sbc
> and send it over the air ...
>
> currently the data (sbc/pcm) and the control (connection
> establishment) is too tightly sort of coded in together.. isn't it ?
>
> for something like the above to happen our liba2dp had to be almost
> re-written ...

Im afraid liba2dp is some android specific thing, not upstream, and as
I said before upstream is probably going to use dbus to pass the fd
around, so this is probably not relevant here.

-- 
Luiz Augusto von Dentz
Computer Engineer

^ permalink raw reply

* Re: [PATCH] Changes in HDP API.
From: José Antonio Santos Cadenas @ 2010-08-10  8:01 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1281016183-4547-1-git-send-email-santoscadenas@gmail.com>

Hi all,

El Thursday 05 August 2010 15:49:43 Jose Antonio Santos Cadenas escribió:
> Now the connection is made directly to a device and bluetoothd daemon
> guess the PSM connection and the MDEPID based on the data get from the
> SDP record.
> ---
>  doc/health-api.txt |  105
> ++++++++++++++++++++-------------------------------- 1 files changed, 40
> insertions(+), 65 deletions(-)
> 
> diff --git a/doc/health-api.txt b/doc/health-api.txt
> index f469df3..89df8bd 100644
> --- a/doc/health-api.txt
> +++ b/doc/health-api.txt
> @@ -14,12 +14,9 @@ Object path	/org/bluez/
> 
>  Methods:
> 
> -	object	CreateApplication(dict config, object agent)
> +	object	CreateApplication(dict config)
> 
> -		Returns the path of the new registered application. The agent
> -		parameter is the path of the object with the callbacks to
> -		notify events (see org.bluez.HealthAgent at the end
> -		of this document).
> +		Returns the path of the new registered application.
> 
>  		Dict is defined as bellow:
>  		{
> @@ -39,32 +36,33 @@ Methods:
> 
>  		Closes the HDP application identified by the object path. Also
>  		application will be closed if the process that started it leaves
> -		the bus.
> +		the bus. Only the creator of the application will be able to
> +		destroy it.
> 
>  		Possible errors: org.bluez.Error.InvalidArguments
>  				org.bluez.Error.NotFound
> +				org.bluez.Error.NotAllowed
> 
>  --------------------------------------------------------------------------
> ------
> 
>  Service		org.bluez
> -Interface	org.bluez.HealthApplication
> -Object path	[variable prefix]/health_app_ZZZZ
> +Interface	org.bluez.HealthDevice
> +Object path	[variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
> 
>  Methods:
> 
> -	void Echo(object service)
> +	Boolean Echo()
> 
>  		Sends an echo petition to the remote service. Returns True if
>  		response matches with the buffer sent. If some error is detected
> -		False value is returned and the associated MCL is closed.
> +		False value is returned.
> 
>  		Possible errors: org.bluez.Error.InvalidArguments
>  				org.bluez.Error.OutOfRange
> 
> -	object CreateChannel(object service, string type)
> +	object CreateChannel(object application, string configuration)
> 
> -		Creates a new data channel with the indicated config to the
> -		remote Service.
> +		Creates a new data channel.
>  		The configuration should indicate the channel quality of
>  		service using one of this values "Reliable", "Streaming", "Any".
> 
> @@ -76,16 +74,27 @@ Methods:
> 
>  	void DestroyChannel(object channel)
> 
> -		Destroys the data channel object.
> +		Destroys the data channel object. Only the creator of the
> +		channel or the creator of the HealtApplication that received the
> +		data channel will be able to destroy it
> 
>  		Possible errors: org.bluez.Error.InvalidArguments
> -				orb.bluez.Error.NotFound
> +				org.bluez.Error.NotFound
> +				org.bluez.Error.NotAllowed
> 
> ---------------------------------------------------------------------------
> ------ +Signals:
> 
> -Service		org.bluez
> -Interface	org.bluez.HealthService
> -Object path	[variable
> prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/hdp_YYYY +	void
> ChannelConnected(object channel)
> +
> +		This signal is launched when a new data channel is created or
> +		when a known data channel is reconnected.
> +
> +	void ChannelDeleted(object channel)
> +
> +		This signal is launched when a data channel is deleted.
> +
> +		After this signal the data channel path will not be valid and
> +		its path can be reused for future data channels.
> 
>  --------------------------------------------------------------------------
> ------
> 
> @@ -94,6 +103,9 @@ Interface	org.bluez.HealthChannel
>  Object path	[variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/
>  							hdp_YYYY/channel_ZZ
> 
> +Only the process that created the data channel or the creator of the
> +HealthApplication that received it will be able to call this methods.
> +
>  Methods:
> 
>  	dict GetProperties()
> @@ -101,17 +113,23 @@ Methods:
>  		Returns all properties for the interface. See the properties
>  		section for available properties.
> 
> +		Posible errors: org.bluez.Error.NotAllowed
> +
>  	fd Acquire()
> 
>  		Returns the file descriptor for this data channel. If the data
>  		channel is not connected it will also reconnect.
> 
>  		Possible errors: org.bluez.Error.NotConnected
> +				org.bluez.Error.NotAllowed
> 
>  	void Release()
> 
>  		Releases the fd. Application should also need to close() it.
> 
> +		Possible errors: org.bluez.Error.NotAcquired
> +				org.bluez.Error.NotAllowed
> +
>  Properties:
> 
>  	string Type (read only)
> @@ -119,50 +137,7 @@ Properties:
>  		The quality of service of the data channel. ("Reliable" or
>  		"Streaming")
> 
> -	object Service (read only)
> -
> -		Identifies the Remote Service that is connected with. Maps with
> -		a HealthService object.
> -
> -HealthAgent hierarchy
> -=====================
> -
> -(this object is implemented by the HDP user in order to receive
> notifications) -
> -Service		unique name
> -Interface	org.bluez.HealthAgent
> -Object path	freely definable
> -
> -Methods:
> -
> -	void Release()
> -
> -		This method gets called when the service daemon unregisters the
> -		agent. An agent can use it to do cleanup tasks. There is no need
> -		to unregister the agent, because when this method gets called it
> -		has already been unregistered.
> -
> -	void ServiceDiscovered(object service)
> -
> -		This method is called when a device containing an HDP
> -		application is paired or when the method Update of the
> -		HealthManager is called and new HealthServices are discovered.
> -		The method will be called once for each HealthService.
> -
> -	void ServiceRemoved(object service)
> -
> -		This is called if during an Update some HealthServices
> -		have disappeared. The method is called once for each removed
> -		HealthService.
> -
> -	void ChannelConnected(object channel)
> -
> -		This method is called when a new data channel is created or when
> -		a known data channel is reconnected.
> -
> -	void ChannelDeleted(object channel)
> -
> -		This method is called when a data channel is deleted.
> +	object Device (read only)
> 
> -		After this call the data channel path will not be valid and can
> -		be reused for future creation of data channels.
> +		Identifies the Remote Device that is connected with. Maps with
> +		a HealthDevice object.


Any more comments about this? Are there any extra things to talk about? It 
will be great to fix this issues so we can continue developing the plugin.

Thanks.

Jose

^ permalink raw reply

* Interlaced page scan
From: Krishnan @ 2010-08-10  8:45 UTC (permalink / raw)
  To: linux-bluetooth

Hi,

Is there a way to enable interlaced page scan in BlueZ? I know 
hciconfig has an option to enable only interlaced inquiry scan.

Regards,
Krishnan

^ permalink raw reply

* Re: [PATCH 1/1] Bluetooth: hidp: Add support for hidraw     HIDIOCGFEATURE  and HIDIOCSFEATURE
From: Jiri Kosina @ 2010-08-10 11:46 UTC (permalink / raw)
  To: Alan Ott
  Cc: Marcel Holtmann, David S Miller, Michael Poole, Bastien Nocera,
	Eric Dumazet, linux-bluetooth, linux-kernel, netdev
In-Reply-To: <4C4878C2.9000903@signal11.us>

On Thu, 22 Jul 2010, Alan Ott wrote:

> > that the ioctl() API is synchronous is fine to me, however pushing that
> > down to the transport drivers seems wrong to me. I think the HID core
> > should be able to handle a fully asynchronous transport driver. I know
> > that with the USB subsystem you are little bit spoiled here, but for
> > Bluetooth it is not the case. And in the end even using the asynchronous
> > USB URB calls would be nice as well.
> >    
> 
> How are the URB calls better than using the synchronous calls? (see below)

Hi,

I think this was the last mail in the thread so far, right?

> > So why not make the core actually wait for responses from the transport
> > driver.
> 
> Because this makes the USB side a lot more complicated, and it would 
> introduce transport specific code into the core. Further, it would 
> involve the transport driver calling hidraw with _every_ single packet 
> it receives. Further, it would have to call hidraw with HANDSHAKE 
> protocol error packets as well.
>
> >   I would make the transport drivers a lot simpler in the long
> > run.
> 
> It would make the USB transport driver and drivers/hid/hidraw much more
> complicated right now, at the expense of making the BT transport driver
> marginally simpler (and I'm not even convinced whether it would really be
> simpler). (see below for more)

Seconded.

> >   And I know that most likely besides Bluetooth and USB you won't see
> > another, but you never know.
> > 
> I just don't understand the objection. In each transport type, the waiting
> will have to be done in a different way. USB and BT are different enough that
> this is the case already, without having to imagine future buses which use
> HID. In BT, you have to check each packet which comes back from the BT network
> to see whether it is the response packet that hidraw is waiting for. Further,
> you have to check for HANDSHAKE packets indicating protocol error. Where
> better for this to be done than in hidp? Further, how can this possibly happen
> in drivers/hid/hidraw, as it doesn't know about the details of bluetooth to
> make this determination, and why should it? In my last email (
> http://lkml.org/lkml/2010/7/9/231 ) to which I got no response, I laid out how
> doing the blocking in drivers/hid/hidraw would only make all the parts except
> bluetooth more complicated (including the core, and the USB side), and would
> also introduce bluetooth-specific things into the core.
> 
> Further, you're saying that using the asynchronous USB URB calls would be a
> benefit. How is it a benefit to replace a single function call which does
> exactly what I want, with a set of asynchronous calls and then adding my own
> blocking to make it do the same thing? This sounds to me like it would be 1:
> more text, 2: duplication of code, 3: error prone. I can't understand how this
> is of benefit. Please explain to me what I'm missing.
> 
> In theory, what you're saying makes sense. Making common code and logic
> actually common is always good. In practice though, in this case, I submit
> that there really isn't any commonality, and the only way for there to be
> commonality is to do the USB side the hard way. Further, drivers/hid/hidraw
> can't wait for a bluetooth packet without having code that's
> bluetooth-specific. It seems just that simple to me.
> 
> I'll give it some more thought, and take another look at the code to see if
> there's something obvious that I'm missing. If you know what I'm missing in my
> understanding of the problem, please tell me :)

Marcel, did you have time to review Alan's explanation a little bit?

I must say I would really like to have this feature merged, but of course 
not if you completely disagree .. but then we'll have to find some 
consensus. Currently Alan's summary above quite aligns with my opinion.

Thanks,

-- 
Jiri Kosina
SUSE Labs, Novell Inc.

^ permalink raw reply

* Re: [PATCH 1/1] Bluetooth: hidp: Add support for hidraw     HIDIOCGFEATURE  and HIDIOCSFEATURE
From: Marcel Holtmann @ 2010-08-10 12:12 UTC (permalink / raw)
  To: Jiri Kosina
  Cc: Alan Ott, David S Miller, Michael Poole, Bastien Nocera,
	Eric Dumazet, linux-bluetooth, linux-kernel, netdev
In-Reply-To: <alpine.LNX.2.00.1008101344480.29919@pobox.suse.cz>

Hi Jiri,

> > > that the ioctl() API is synchronous is fine to me, however pushing that
> > > down to the transport drivers seems wrong to me. I think the HID core
> > > should be able to handle a fully asynchronous transport driver. I know
> > > that with the USB subsystem you are little bit spoiled here, but for
> > > Bluetooth it is not the case. And in the end even using the asynchronous
> > > USB URB calls would be nice as well.
> > >    
> > 
> > How are the URB calls better than using the synchronous calls? (see below)
> 
> Hi,
> 
> I think this was the last mail in the thread so far, right?
> 
> > > So why not make the core actually wait for responses from the transport
> > > driver.
> > 
> > Because this makes the USB side a lot more complicated, and it would 
> > introduce transport specific code into the core. Further, it would 
> > involve the transport driver calling hidraw with _every_ single packet 
> > it receives. Further, it would have to call hidraw with HANDSHAKE 
> > protocol error packets as well.
> >
> > >   I would make the transport drivers a lot simpler in the long
> > > run.
> > 
> > It would make the USB transport driver and drivers/hid/hidraw much more
> > complicated right now, at the expense of making the BT transport driver
> > marginally simpler (and I'm not even convinced whether it would really be
> > simpler). (see below for more)
> 
> Seconded.
> 
> > >   And I know that most likely besides Bluetooth and USB you won't see
> > > another, but you never know.
> > > 
> > I just don't understand the objection. In each transport type, the waiting
> > will have to be done in a different way. USB and BT are different enough that
> > this is the case already, without having to imagine future buses which use
> > HID. In BT, you have to check each packet which comes back from the BT network
> > to see whether it is the response packet that hidraw is waiting for. Further,
> > you have to check for HANDSHAKE packets indicating protocol error. Where
> > better for this to be done than in hidp? Further, how can this possibly happen
> > in drivers/hid/hidraw, as it doesn't know about the details of bluetooth to
> > make this determination, and why should it? In my last email (
> > http://lkml.org/lkml/2010/7/9/231 ) to which I got no response, I laid out how
> > doing the blocking in drivers/hid/hidraw would only make all the parts except
> > bluetooth more complicated (including the core, and the USB side), and would
> > also introduce bluetooth-specific things into the core.
> > 
> > Further, you're saying that using the asynchronous USB URB calls would be a
> > benefit. How is it a benefit to replace a single function call which does
> > exactly what I want, with a set of asynchronous calls and then adding my own
> > blocking to make it do the same thing? This sounds to me like it would be 1:
> > more text, 2: duplication of code, 3: error prone. I can't understand how this
> > is of benefit. Please explain to me what I'm missing.
> > 
> > In theory, what you're saying makes sense. Making common code and logic
> > actually common is always good. In practice though, in this case, I submit
> > that there really isn't any commonality, and the only way for there to be
> > commonality is to do the USB side the hard way. Further, drivers/hid/hidraw
> > can't wait for a bluetooth packet without having code that's
> > bluetooth-specific. It seems just that simple to me.
> > 
> > I'll give it some more thought, and take another look at the code to see if
> > there's something obvious that I'm missing. If you know what I'm missing in my
> > understanding of the problem, please tell me :)
> 
> Marcel, did you have time to review Alan's explanation a little bit?
> 
> I must say I would really like to have this feature merged, but of course 
> not if you completely disagree .. but then we'll have to find some 
> consensus. Currently Alan's summary above quite aligns with my opinion.

my opinion is still that we should make the core do the async handling.
I think that we let USB dictate how APIs for HID should look like.

However that is maybe fine anyway since the Bluetooth HID guys where not
really inventive since they copying USB HID for the better and mostly
for the worst. Especially since Bluetooth doesn't have the endpoint
direction limits like USB does. Anyhow, just get the patches re-based
and re-submitted and I can have a second look.

Regards

Marcel



^ permalink raw reply

* Re: [PATCH] Bluetooth: btmrvl: devices are either BR/EDR or AMPs
From: Marcel Holtmann @ 2010-08-10 12:13 UTC (permalink / raw)
  To: David Vrabel; +Cc: linux-bluetooth
In-Reply-To: <4C607688.9050608@csr.com>

Hi David,

> From 5d16a92e61d8457077d0235d6b7006b824ff6891 Mon Sep 17 00:00:00 2001
> From: David Vrabel <dv02@dv02-laptop.(none)>
> Date: Mon, 9 Aug 2010 22:39:43 +0100
> Subject: [PATCH] Bluetooth: btmrvl: devices are either BR/EDR or AMPs
> 
> Signed-off-by: David Vrabel <david.vrabel@csr.com>
> ---
>  drivers/bluetooth/btmrvl_main.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)

I fixed this up in your original patch.

Regards

Marcel



^ permalink raw reply

* Re: [PATCH] Bluetooth: Process HCI events in a workqueue instead of a tasklet
From: Marcel Holtmann @ 2010-08-10 12:15 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1281323213-30907-1-git-send-email-marcel@holtmann.org>

Hi everybody,

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

so I stuffed this now into bluetooth-testing tree and would like to see
some extra testing exposure. So far this has only been tested by myself.

If there are no regression then this should make a lot of HCI and L2CAP
handling a lot simpler.

Regards

Marcel



^ permalink raw reply

* [PATCH] Bluetooth: new sockopt to enter active state when sending data
From: Emeltchenko Andrei @ 2010-08-10 12:29 UTC (permalink / raw)
  To: linux-bluetooth

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

Patch adds new socket option to enter active state when sending
data. Modified version of Fabien Chevalier patch (Sep/2008).
Discussions:
http://www.spinics.net/lists/bluez-devel/msg00567.html
http://www.spinics.net/lists/linux-bluetooth/msg03765.html

Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
---
 include/net/bluetooth/hci_core.h |    1 +
 include/net/bluetooth/l2cap.h    |    1 +
 net/bluetooth/hci_conn.c         |    7 ++++++-
 net/bluetooth/l2cap.c            |    3 +++
 4 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index e42f6ed..477a492 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -178,6 +178,7 @@ struct hci_conn {
 	__u8             auth_type;
 	__u8             sec_level;
 	__u8             power_save;
+	__u8             force_active;
 	__u16            disc_timeout;
 	unsigned long	 pend;
 
diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 7c695bf..0a8ac4c 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -57,6 +57,7 @@ struct l2cap_options {
 	__u8  mode;
 	__u8  fcs;
 	__u8  max_tx;
+	__u8  force_active;
 	__u16 txwin_size;
 };
 
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 800b6b9..b8ea611 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -214,6 +214,8 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst)
 	conn->auth_type = HCI_AT_GENERAL_BONDING;
 
 	conn->power_save = 1;
+	/* Enter active state by default */ 
+	conn->force_active = 1;
 	conn->disc_timeout = HCI_DISCONN_TIMEOUT;
 
 	switch (type) {
@@ -505,7 +507,10 @@ void hci_conn_enter_active_mode(struct hci_conn *conn)
 	if (test_bit(HCI_RAW, &hdev->flags))
 		return;
 
-	if (conn->mode != HCI_CM_SNIFF || !conn->power_save)
+	if (conn->mode != HCI_CM_SNIFF)
+		goto timer;
+
+	if (!conn->power_save && !conn->force_active)
 		goto timer;
 
 	if (!test_and_set_bit(HCI_CONN_MODE_CHANGE_PEND, &conn->pend)) {
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index cf3c407..a631cf1 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -1863,6 +1863,7 @@ static int l2cap_sock_setsockopt_old(struct socket *sock, int optname, char __us
 		opts.mode     = l2cap_pi(sk)->mode;
 		opts.fcs      = l2cap_pi(sk)->fcs;
 		opts.max_tx   = l2cap_pi(sk)->max_tx;
+		opts.force_active = l2cap_pi(sk)->conn->hcon->force_active;
 		opts.txwin_size = (__u16)l2cap_pi(sk)->tx_win;
 
 		len = min_t(unsigned int, sizeof(opts), optlen);
@@ -1889,6 +1890,7 @@ static int l2cap_sock_setsockopt_old(struct socket *sock, int optname, char __us
 		l2cap_pi(sk)->omtu = opts.omtu;
 		l2cap_pi(sk)->fcs  = opts.fcs;
 		l2cap_pi(sk)->max_tx = opts.max_tx;
+		l2cap_pi(sk)->conn->hcon->force_active = opts.force_active;
 		l2cap_pi(sk)->tx_win = (__u8)opts.txwin_size;
 		break;
 
@@ -2006,6 +2008,7 @@ static int l2cap_sock_getsockopt_old(struct socket *sock, int optname, char __us
 		opts.mode     = l2cap_pi(sk)->mode;
 		opts.fcs      = l2cap_pi(sk)->fcs;
 		opts.max_tx   = l2cap_pi(sk)->max_tx;
+		opts.force_active = l2cap_pi(sk)->conn->hcon->force_active;
 		opts.txwin_size = (__u16)l2cap_pi(sk)->tx_win;
 
 		len = min_t(unsigned int, len, sizeof(opts));
-- 
1.7.0.4


^ permalink raw reply related

* Re: [PATCH] Bluetooth: Process HCI events in a workqueue instead of a tasklet
From: David Vrabel @ 2010-08-10 12:50 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <1281442519.12579.209.camel@localhost.localdomain>

Marcel Holtmann wrote:
> Hi everybody,
> 
>> Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
>> ---
>>  include/net/bluetooth/hci_core.h |    2 +
>>  net/bluetooth/hci_core.c         |   58 ++++++++++++++++++++++++++++++++-----
>>  2 files changed, 52 insertions(+), 8 deletions(-)
> 
> so I stuffed this now into bluetooth-testing tree and would like to see
> some extra testing exposure. So far this has only been tested by myself.
> 
> If there are no regression then this should make a lot of HCI and L2CAP
> handling a lot simple.

This may result in packets being processed in a different order to that 
which they were received in.

e.g., what happens to an ACL packet processed before the connection 
complete event for that connection?

David
-- 
David Vrabel, Senior Software Engineer, Drivers
CSR, Churchill House, Cambridge Business Park,  Tel: +44 (0)1223 692562
Cowley Road, Cambridge, CB4 0WZ                 http://www.csr.com/


Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom

^ permalink raw reply

* Re: [PATCH] Bluetooth: new sockopt to enter active state when sending data
From: Marcel Holtmann @ 2010-08-10 13:07 UTC (permalink / raw)
  To: Emeltchenko Andrei; +Cc: linux-bluetooth
In-Reply-To: <1281443393-31160-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

Hi Andrei,

> From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
> 
> Patch adds new socket option to enter active state when sending
> data. Modified version of Fabien Chevalier patch (Sep/2008).
> Discussions:
> http://www.spinics.net/lists/bluez-devel/msg00567.html
> http://www.spinics.net/lists/linux-bluetooth/msg03765.html
> 
> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
> ---
>  include/net/bluetooth/hci_core.h |    1 +
>  include/net/bluetooth/l2cap.h    |    1 +
>  net/bluetooth/hci_conn.c         |    7 ++++++-
>  net/bluetooth/l2cap.c            |    3 +++
>  4 files changed, 11 insertions(+), 1 deletions(-)
> 
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index e42f6ed..477a492 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -178,6 +178,7 @@ struct hci_conn {
>  	__u8             auth_type;
>  	__u8             sec_level;
>  	__u8             power_save;
> +	__u8             force_active;
>  	__u16            disc_timeout;
>  	unsigned long	 pend;
>  
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index 7c695bf..0a8ac4c 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -57,6 +57,7 @@ struct l2cap_options {
>  	__u8  mode;
>  	__u8  fcs;
>  	__u8  max_tx;
> +	__u8  force_active;
>  	__u16 txwin_size;
>  };

this is a clear NAK. You are breaking API and ABI compatibility with
this change.

Also don't overload the l2cap_options, create a new one.

Regards

Marcel



^ permalink raw reply

* Re: [PATCH] Bluetooth: Process HCI events in a workqueue instead of a tasklet
From: Marcel Holtmann @ 2010-08-10 13:14 UTC (permalink / raw)
  To: David Vrabel; +Cc: linux-bluetooth
In-Reply-To: <4C614B01.6030000@csr.com>

Hi David,

> >> Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
> >> ---
> >>  include/net/bluetooth/hci_core.h |    2 +
> >>  net/bluetooth/hci_core.c         |   58 ++++++++++++++++++++++++++++++++-----
> >>  2 files changed, 52 insertions(+), 8 deletions(-)
> > 
> > so I stuffed this now into bluetooth-testing tree and would like to see
> > some extra testing exposure. So far this has only been tested by myself.
> > 
> > If there are no regression then this should make a lot of HCI and L2CAP
> > handling a lot simple.
> 
> This may result in packets being processed in a different order to that 
> which they were received in.
> 
> e.g., what happens to an ACL packet processed before the connection 
> complete event for that connection?

good point. So we would either a) need to disable the RX tasklet when we
receive an event and schedule it for processing or b) process the ACL
data also in a workqueue.

Regards

Marcel



^ 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