Linux bluetooth development
 help / color / mirror / Atom feed
* [PATCH] Fix possible memory leak of the GIOChannel in the attribute server
From: Claudio Takahasi @ 2010-11-10 21:56 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Claudio Takahasi
In-Reply-To: <1289426161-10045-1-git-send-email-claudio.takahasi@openbossa.org>

---
 src/attrib-server.c |   32 +++++++++++++++++++-------------
 1 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/src/attrib-server.c b/src/attrib-server.c
index 1fc1c18..f644091 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -609,6 +609,20 @@ int attrib_server_init(void)
 		return -1;
 	}
 
+	record = server_record_new();
+	if (record == NULL) {
+		error("Unable to create GATT service record");
+		goto failed;
+	}
+
+	if (add_record_to_server(BDADDR_ANY, record) < 0) {
+		error("Failed to register GATT service record");
+		sdp_record_free(record);
+		goto failed;
+	}
+
+	sdp_handle = record->handle;
+
 	/* LE socket */
 	le_io = bt_io_listen(BT_IO_L2CAP, NULL, confirm_event,
 					NULL, NULL, &gerr,
@@ -623,21 +637,13 @@ int attrib_server_init(void)
 		/* Doesn't have LE support, continue */
 	}
 
-	record = server_record_new();
-	if (record == NULL) {
-		error("Unable to create GATT service record");
-		return -1;
-	}
-
-	if (add_record_to_server(BDADDR_ANY, record) < 0) {
-		error("Failed to register GATT service record");
-		sdp_record_free(record);
-		return -1;
-	}
+	return 0;
 
-	sdp_handle = record->handle;
+failed:
+	g_io_channel_unref(l2cap_io);
+	l2cap_io = NULL;
 
-	return 0;
+	return -1;
 }
 
 void attrib_server_exit(void)
-- 
1.7.3.2


^ permalink raw reply related

* [PATCH] Add a new configuration option to disable Low Energy support
From: Claudio Takahasi @ 2010-11-10 21:56 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Claudio Takahasi
In-Reply-To: <1289426161-10045-1-git-send-email-claudio.takahasi@openbossa.org>

Disable LE interleave discovery and attribute server over LE link.
Option required to force disabling Low energy support for LE capable
adapters.
---
 src/adapter.c       |    2 +-
 src/attrib-server.c |    4 ++++
 src/hcid.h          |    1 +
 src/main.c          |    7 +++++++
 src/main.conf       |    7 ++++++-
 5 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index 31014e5..0741550 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2082,7 +2082,7 @@ static int adapter_setup(struct btd_adapter *adapter, const char *mode)
 	if (dev->features[7] & LMP_INQ_TX_PWR)
 		adapter_ops->read_inq_tx_pwr(adapter->dev_id);
 
-	if (dev->features[4] & LMP_LE) {
+	if (dev->features[4] & LMP_LE && main_opts.le) {
 		uint8_t simul = (dev->features[6] & LMP_LE_BREDR) ? 0x01 : 0x00;
 		err = adapter_ops->write_le_host(adapter->dev_id, 0x01, simul);
 		if (err < 0) {
diff --git a/src/attrib-server.c b/src/attrib-server.c
index f644091..375b731 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -40,6 +40,7 @@
 #include "glib-helper.h"
 #include "btio.h"
 #include "sdpd.h"
+#include "hcid.h"
 #include "att.h"
 #include "gattrib.h"
 
@@ -623,6 +624,9 @@ int attrib_server_init(void)
 
 	sdp_handle = record->handle;
 
+	if (!main_opts.le)
+		return 0;
+
 	/* LE socket */
 	le_io = bt_io_listen(BT_IO_L2CAP, NULL, confirm_event,
 					NULL, NULL, &gerr,
diff --git a/src/hcid.h b/src/hcid.h
index 48d489a..a9484a6 100644
--- a/src/hcid.h
+++ b/src/hcid.h
@@ -56,6 +56,7 @@ struct main_opts {
 	gboolean	name_resolv;
 	gboolean	debug_keys;
 	gboolean	attrib_server;
+	gboolean	le;
 
 	uint8_t		scan;
 	uint8_t		mode;
diff --git a/src/main.c b/src/main.c
index adbb374..9abdd60 100644
--- a/src/main.c
+++ b/src/main.c
@@ -222,6 +222,13 @@ static void parse_config(GKeyFile *config)
 	else
 		main_opts.attrib_server = boolean;
 
+	boolean = g_key_file_get_boolean(config, "General",
+						"EnableLE", &err);
+	if (err)
+		g_clear_error(&err);
+	else
+		main_opts.le = boolean;
+
 	main_opts.link_mode = HCI_LM_ACCEPT;
 
 	main_opts.link_policy = HCI_LP_RSWITCH | HCI_LP_SNIFF |
diff --git a/src/main.conf b/src/main.conf
index f92bf42..c03f135 100644
--- a/src/main.conf
+++ b/src/main.conf
@@ -56,6 +56,11 @@ NameResolving = true
 # that they were created for.
 DebugKeys = false
 
+# Enable Low Energy support if the dongle supports. Default is false.
+# Enable/Disable interleave discovery and attribute server over LE.
+EnableLE = false
+
 # Enable the GATT Attribute Server. Default is false, because it is only
-# useful for testing.
+# useful for testing. Attribute server is not enabled over LE if EnableLE
+# is false.
 AttributeServer = false
-- 
1.7.3.2


^ permalink raw reply related

* Re: [PATCH 3/6] Bluetooth: Use LE buffers for LE traffic
From: Ville Tervo @ 2010-11-11  5:53 UTC (permalink / raw)
  To: ext Claudio Takahasi; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <AANLkTiktxkQQXH2m2+s-zyfyWh-jUT9rwjU+GPyPWyhS@mail.gmail.com>

Hi Claudio,

On Wed, Nov 10, 2010 at 05:53:02PM +0100, ext Claudio Takahasi wrote:
> Hi Ville,
> 
> On Mon, Oct 25, 2010 at 10:21 AM, Ville Tervo <ville.tervo@nokia.com> wrote:
> > BLuetooth chips may have separate buffers for
> > LE traffic. This patch add support to use LE
> > buffers provided by the chip.
> >
> > Signed-off-by: Ville Tervo <ville.tervo@nokia.com>
> > ---
> >  include/net/bluetooth/hci.h      |    1 +
> >  include/net/bluetooth/hci_core.h |    5 +++
> >  net/bluetooth/hci_conn.c         |    5 +++
> >  net/bluetooth/hci_core.c         |   74 +++++++++++++++++++++++++++++++++++--
> >  net/bluetooth/hci_event.c        |   40 +++++++++++++++++++-
> >  5 files changed, 119 insertions(+), 6 deletions(-)
> >
> > diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> > index 02055b9..2103731 100644
> > --- a/include/net/bluetooth/hci.h
> > +++ b/include/net/bluetooth/hci.h
> > @@ -189,6 +189,7 @@ enum {
> >
> >  #define LMP_EV4                0x01
> >  #define LMP_EV5                0x02
> > +#define LMP_LE         0x40
> >
> >  #define LMP_SNIFF_SUBR 0x02
> >  #define LMP_EDR_ESCO_2M        0x20
> > diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> > index 2b7f94a..e2d857a 100644
> > --- a/include/net/bluetooth/hci_core.h
> > +++ b/include/net/bluetooth/hci_core.h
> > @@ -103,15 +103,19 @@ struct hci_dev {
> >        atomic_t        cmd_cnt;
> >        unsigned int    acl_cnt;
> >        unsigned int    sco_cnt;
> > +       unsigned int    le_cnt;
> >
> >        unsigned int    acl_mtu;
> >        unsigned int    sco_mtu;
> > +       unsigned int    le_mtu;
> >        unsigned int    acl_pkts;
> >        unsigned int    sco_pkts;
> > +       unsigned int    le_pkts;
> >
> >        unsigned long   cmd_last_tx;
> >        unsigned long   acl_last_tx;
> >        unsigned long   sco_last_tx;
> > +       unsigned long   le_last_tx;
> >
> >        struct workqueue_struct *workqueue;
> >
> > @@ -473,6 +477,7 @@ void hci_conn_del_sysfs(struct hci_conn *conn);
> >  #define lmp_sniffsubr_capable(dev) ((dev)->features[5] & LMP_SNIFF_SUBR)
> >  #define lmp_esco_capable(dev)      ((dev)->features[3] & LMP_ESCO)
> >  #define lmp_ssp_capable(dev)       ((dev)->features[6] & LMP_SIMPLE_PAIR)
> > +#define lmp_le_capable(dev)        ((dev)->features[4] & LMP_LE)
> >
> >  /* ----- HCI protocols ----- */
> >  struct hci_proto {
> > diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> > index 0944c0c..ddc2e5e 100644
> > --- a/net/bluetooth/hci_conn.c
> > +++ b/net/bluetooth/hci_conn.c
> > @@ -324,6 +324,11 @@ int hci_conn_del(struct hci_conn *conn)
> >
> >                /* Unacked frames */
> >                hdev->acl_cnt += conn->sent;
> > +       } else if (conn->type == LE_LINK) {
> > +               if (hdev->le_pkts)
> > +                       hdev->le_cnt += conn->sent;
> > +               else
> > +                       hdev->acl_cnt += conn->sent;
> >        } else {
> >                struct hci_conn *acl = conn->link;
> >                if (acl) {
> > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> > index bc2a052..45c78c2 100644
> > --- a/net/bluetooth/hci_core.c
> > +++ b/net/bluetooth/hci_core.c
> > @@ -254,6 +254,14 @@ static void hci_init_req(struct hci_dev *hdev, unsigned long opt)
> >        hci_send_cmd(hdev, HCI_OP_WRITE_CA_TIMEOUT, 2, &param);
> >  }
> >
> > +static void hci_le_init_req(struct hci_dev *hdev, unsigned long opt)
> > +{
> > +       BT_DBG("%s", hdev->name);
> > +
> > +       /* Read LE buffer size */
> > +       hci_send_cmd(hdev, HCI_OP_LE_READ_BUFFER_SIZE, 0, NULL);
> > +}
> > +
> >  static void hci_scan_req(struct hci_dev *hdev, unsigned long opt)
> >  {
> >        __u8 scan = opt;
> > @@ -509,6 +517,10 @@ int hci_dev_open(__u16 dev)
> >                ret = __hci_request(hdev, hci_init_req, 0,
> >                                        msecs_to_jiffies(HCI_INIT_TIMEOUT));
> >
> > +               if (lmp_le_capable(hdev))
> > +                       ret = __hci_request(hdev, hci_le_init_req, 0,
> > +                                       msecs_to_jiffies(HCI_INIT_TIMEOUT));
> > +
> >                clear_bit(HCI_INIT, &hdev->flags);
> >        }
> >
> > @@ -645,7 +657,7 @@ int hci_dev_reset(__u16 dev)
> >                hdev->flush(hdev);
> >
> >        atomic_set(&hdev->cmd_cnt, 1);
> > -       hdev->acl_cnt = 0; hdev->sco_cnt = 0;
> > +       hdev->acl_cnt = 0; hdev->sco_cnt = 0; hdev->le_cnt = 0;
> >
> >        if (!test_bit(HCI_RAW, &hdev->flags))
> >                ret = __hci_request(hdev, hci_reset_req, 0,
> > @@ -1456,8 +1468,25 @@ static inline struct hci_conn *hci_low_sent(struct hci_dev *hdev, __u8 type, int
> >        }
> >
> >        if (conn) {
> > -               int cnt = (type == ACL_LINK ? hdev->acl_cnt : hdev->sco_cnt);
> > -               int q = cnt / num;
> > +               int cnt, q;
> > +
> > +               switch (conn->type) {
> > +               case ACL_LINK:
> > +                       cnt = hdev->acl_cnt;
> > +                       break;
> > +               case SCO_LINK:
> > +               case ESCO_LINK:
> > +                       cnt = hdev->sco_cnt;
> > +                       break;
> > +               case LE_LINK:
> > +                       cnt = hdev->le_mtu ? hdev->le_cnt : hdev->acl_cnt;
> > +                       break;
> > +               default:
> > +                       cnt = 0;
> > +                       BT_ERR("Unknown link type");
> > +               }
> > +
> > +               q = cnt / num;
> >                *quote = q ? q : 1;
> >        } else
> >                *quote = 0;
> > @@ -1556,6 +1585,40 @@ static inline void hci_sched_esco(struct hci_dev *hdev)
> >        }
> >  }
> >
> > +static inline void hci_sched_le(struct hci_dev *hdev)
> > +{
> > +       struct hci_conn *conn;
> > +       struct sk_buff *skb;
> > +       int quote, cnt;
> > +
> > +       BT_DBG("%s", hdev->name);
> > +
> > +       if (!test_bit(HCI_RAW, &hdev->flags)) {
> > +               /* ACL tx timeout must be longer than maximum
> > +                * link supervision timeout (40.9 seconds) */
> > +               if (!hdev->le_cnt &&
> > +                   time_after(jiffies, hdev->le_last_tx + HZ * 45))
> > +                       hci_acl_tx_to(hdev);
> > +       }
> 
> It seems that the ACL tx timeout is causing some problems: BR/EDR and
> LE connections are not working properly on macbooks! Don't ask me why
> on macbooks only! But I double checked. I tested your branch and also
> bluetooth-next + LE patches and both are not working as expected. I
> didn't have time to investigate it, do you have any clue?
> 
> For BR/EDR I am receiving "killing stalled ACL connection" messages
> for all connections.
> 

Yes you are right. I found this also yesterday and I have a patch to fix it
already. I'll clean it up and submit later today.

-- 
Ville

^ permalink raw reply

* [PATCH 1/3] Split up pbap object and pbap session
From: Dmitriy Paliy @ 2010-11-11  7:36 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Dmitriy Paliy

Pbap object and session are splitted in this patch. Reason is that
obex firstly makes disconnect of service_data, which corresponds to
session in pbap, and than it closes object, which also corresponds
to session in pbap.

When the session is disconnected, it also deallocates memory. When
obex closes the object, it is trying to dereference the deallocated
memory in order to free pbap->buffer data.

Here object and session are separated, while pointers are created to
make one-to-one mapping. Pbap object is created in vobject_..._open
functions. Session and object are handled separately when freed.
---
 plugins/pbap.c |   89 +++++++++++++++++++++++++++++++++++++------------------
 1 files changed, 60 insertions(+), 29 deletions(-)

diff --git a/plugins/pbap.c b/plugins/pbap.c
index a40563c..7b9f1ff 100644
--- a/plugins/pbap.c
+++ b/plugins/pbap.c
@@ -141,8 +141,13 @@ struct pbap_session {
 	struct apparam_field *params;
 	char *folder;
 	uint32_t find_handle;
-	GString *buffer;
 	struct cache cache;
+	struct pbap_object *obj;
+};
+
+struct pbap_object {
+	GString *buffer;
+	struct pbap_session *session;
 };
 
 static const uint8_t PBAP_TARGET[TARGET_SIZE] = {
@@ -239,9 +244,9 @@ static void phonebook_size_result(const char *buffer, size_t bufsize,
 	hdr->len = PHONEBOOKSIZE_LEN;
 	memcpy(hdr->val, &phonebooksize, sizeof(phonebooksize));
 
-	pbap->buffer = g_string_new_len(aparam, sizeof(aparam));
+	pbap->obj->buffer = g_string_new_len(aparam, sizeof(aparam));
 
-	obex_object_set_io_flags(pbap, G_IO_IN, 0);
+	obex_object_set_io_flags(pbap->obj, G_IO_IN, 0);
 }
 
 static void query_result(const char *buffer, size_t bufsize, int vcards,
@@ -252,17 +257,17 @@ static void query_result(const char *buffer, size_t bufsize, int vcards,
 	DBG("");
 
 	if (vcards <= 0) {
-		obex_object_set_io_flags(pbap, G_IO_ERR, -ENOENT);
+		obex_object_set_io_flags(pbap->obj, G_IO_ERR, -ENOENT);
 		return;
 	}
 
-	if (!pbap->buffer)
-		pbap->buffer = g_string_new_len(buffer, bufsize);
+	if (!pbap->obj->buffer)
+		pbap->obj->buffer = g_string_new_len(buffer, bufsize);
 	else
-		pbap->buffer = g_string_append_len(pbap->buffer, buffer,
+		pbap->obj->buffer = g_string_append_len(pbap->obj->buffer, buffer,
 								bufsize);
 
-	obex_object_set_io_flags(pbap, G_IO_IN, 0);
+	obex_object_set_io_flags(pbap->obj, G_IO_IN, 0);
 }
 
 static void cache_entry_notify(const char *id, uint32_t handle,
@@ -394,7 +399,7 @@ static void cache_ready_notify(void *user_data)
 		hdr->len = PHONEBOOKSIZE_LEN;
 		memcpy(hdr->val, &size, sizeof(size));
 
-		pbap->buffer = g_string_new_len(aparam, sizeof(aparam));
+		pbap->obj->buffer = g_string_new_len(aparam, sizeof(aparam));
 		goto done;
 	}
 
@@ -408,29 +413,29 @@ static void cache_ready_notify(void *user_data)
 
 	if (sorted == NULL) {
 		pbap->cache.valid = TRUE;
-		obex_object_set_io_flags(pbap, G_IO_ERR, -ENOENT);
+		obex_object_set_io_flags(pbap->obj, G_IO_ERR, -ENOENT);
 		return;
 	}
 
 	/* Computing offset considering first entry of the phonebook */
 	l = g_slist_nth(sorted, pbap->params->liststartoffset);
 
-	pbap->buffer = g_string_new(VCARD_LISTING_BEGIN);
+	pbap->obj->buffer = g_string_new(VCARD_LISTING_BEGIN);
 	for (; l && max; l = l->next, max--) {
 		const struct cache_entry *entry = l->data;
 
-		g_string_append_printf(pbap->buffer, VCARD_LISTING_ELEMENT,
+		g_string_append_printf(pbap->obj->buffer, VCARD_LISTING_ELEMENT,
 						entry->handle, entry->name);
 	}
 
-	pbap->buffer = g_string_append(pbap->buffer, VCARD_LISTING_END);
+	pbap->obj->buffer = g_string_append(pbap->obj->buffer, VCARD_LISTING_END);
 
 	g_slist_free(sorted);
 
 done:
 	if (!pbap->cache.valid) {
 		pbap->cache.valid = TRUE;
-		obex_object_set_io_flags(pbap, G_IO_IN, 0);
+		obex_object_set_io_flags(pbap->obj, G_IO_IN, 0);
 	}
 }
 
@@ -446,15 +451,14 @@ static void cache_entry_done(void *user_data)
 
 	id = cache_find(&pbap->cache, pbap->find_handle);
 	if (id == NULL) {
-		DBG("Entry %d not found on cache", pbap->find_handle);
-		obex_object_set_io_flags(pbap, G_IO_ERR, -ENOENT);
+		obex_object_set_io_flags(pbap->obj, G_IO_ERR, -ENOENT);
 		return;
 	}
 
 	ret = phonebook_get_entry(pbap->folder, id, pbap->params,
 						query_result, pbap);
 	if (ret < 0)
-		obex_object_set_io_flags(pbap, G_IO_ERR, ret);
+		obex_object_set_io_flags(pbap->obj, G_IO_ERR, ret);
 }
 
 static struct apparam_field *parse_aparam(const uint8_t *buffer, uint32_t hlen)
@@ -549,6 +553,7 @@ static void *pbap_connect(struct obex_session *os, int *err)
 	pbap = g_new0(struct pbap_session, 1);
 	pbap->folder = g_strdup("/");
 	pbap->find_handle = PHONEBOOK_INVALID_HANDLE;
+	pbap->obj = NULL;
 
 	if (err)
 		*err = 0;
@@ -692,10 +697,23 @@ static struct obex_service_driver pbap = {
 	.chkput = pbap_chkput
 };
 
+static void *vobject_create(void *user_data)
+{
+	struct pbap_session *pbap = user_data;
+	struct pbap_object *obj;
+
+	obj = g_new0(struct pbap_object, 1);
+	obj->session = pbap;
+	pbap->obj = obj;
+
+	return obj;
+}
+
 static void *vobject_pull_open(const char *name, int oflag, mode_t mode,
 				void *context, size_t *size, int *err)
 {
 	struct pbap_session *pbap = context;
+	struct pbap_object *obj;
 	phonebook_cb cb;
 	int ret;
 
@@ -718,10 +736,13 @@ static void *vobject_pull_open(const char *name, int oflag, mode_t mode,
 		cb = query_result;
 
 	ret = phonebook_pull(name, pbap->params, cb, pbap);
+
 	if (ret < 0)
 		goto fail;
 
-	return pbap;
+	obj = vobject_create(pbap);
+
+	return obj;
 
 fail:
 	if (err)
@@ -734,6 +755,7 @@ static void *vobject_list_open(const char *name, int oflag, mode_t mode,
 				void *context, size_t *size, int *err)
 {
 	struct pbap_session *pbap = context;
+	struct pbap_object *obj;
 	int ret;
 
 	DBG("name %s context %p valid %d", name, context, pbap->cache.valid);
@@ -755,7 +777,7 @@ static void *vobject_list_open(const char *name, int oflag, mode_t mode,
 		 * Valid cache and empty buffer mean that cache was already
 		 * created within a single session, but no data is available.
 		 */
-		if (!pbap->buffer) {
+		if (!pbap->obj->buffer) {
 			ret = -ENOENT;
 			goto fail;
 		}
@@ -771,7 +793,9 @@ static void *vobject_list_open(const char *name, int oflag, mode_t mode,
 		goto fail;
 
 done:
-	return pbap;
+	obj = vobject_create(pbap);
+
+	return obj;
 
 fail:
 	if (err)
@@ -784,6 +808,7 @@ static void *vobject_vcard_open(const char *name, int oflag, mode_t mode,
 					void *context, size_t *size, int *err)
 {
 	struct pbap_session *pbap = context;
+	struct pbap_object *obj;
 	const char *id;
 	uint32_t handle;
 	int ret;
@@ -820,7 +845,9 @@ done:
 	if (ret < 0)
 		goto fail;
 
-	return pbap;
+	obj = vobject_create(pbap);
+
+	return obj;
 
 fail:
 	if (err)
@@ -832,12 +859,13 @@ fail:
 static ssize_t vobject_pull_read(void *object, void *buf, size_t count,
 								uint8_t *hi)
 {
-	struct pbap_session *pbap = object;
+	struct pbap_object *obj = object;
+	struct pbap_session *pbap = obj->session;
 
-	DBG("buffer %p maxlistcount %d", pbap->buffer,
+	DBG("buffer %p maxlistcount %d", obj->buffer,
 						pbap->params->maxlistcount);
 
-	if (!pbap->buffer)
+	if (!obj->buffer)
 		return -EAGAIN;
 
 	/* PhoneBookSize */
@@ -847,13 +875,14 @@ static ssize_t vobject_pull_read(void *object, void *buf, size_t count,
 		/* Stream data */
 		*hi = OBEX_HDR_BODY;
 
-	return string_read(pbap->buffer, buf, count);
+	return string_read(obj->buffer, buf, count);
 }
 
 static ssize_t vobject_list_read(void *object, void *buf, size_t count,
 								uint8_t *hi)
 {
-	struct pbap_session *pbap = object;
+	struct pbap_object *obj = object;
+	struct pbap_session *pbap = obj->session;
 
 	DBG("valid %d maxlistcount %d", pbap->cache.valid,
 						pbap->params->maxlistcount);
@@ -867,13 +896,13 @@ static ssize_t vobject_list_read(void *object, void *buf, size_t count,
 	else
 		*hi = OBEX_HDR_BODY;
 
-	return string_read(pbap->buffer, buf, count);
+	return string_read(obj->buffer, buf, count);
 }
 
 static ssize_t vobject_vcard_read(void *object, void *buf, size_t count,
 								uint8_t *hi)
 {
-	struct pbap_session *pbap = object;
+	struct pbap_object *pbap = object;
 
 	DBG("buffer %p", pbap->buffer);
 
@@ -886,13 +915,15 @@ static ssize_t vobject_vcard_read(void *object, void *buf, size_t count,
 
 static int vobject_close(void *object)
 {
-	struct pbap_session *pbap = object;
+	struct pbap_object *pbap = object;
 
 	if (pbap->buffer) {
 		string_free(pbap->buffer);
 		pbap->buffer = NULL;
 	}
 
+	g_free(pbap);
+
 	return 0;
 }
 
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH 2/3] Code clean up: pbap->params = params removed
From: Dmitriy Paliy @ 2010-11-11  7:36 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Dmitriy Paliy
In-Reply-To: <1289460972-2971-1-git-send-email-dmitriy.paliy@nokia.com>

pbap->params = params; removed due to the fact that this assignment is
already used in the same function.
---
 plugins/pbap.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/plugins/pbap.c b/plugins/pbap.c
index 7b9f1ff..af60741 100644
--- a/plugins/pbap.c
+++ b/plugins/pbap.c
@@ -617,7 +617,6 @@ static int pbap_get(struct obex_session *os, obex_object_t *obj,
 	if (path == NULL)
 		return -EBADR;
 
-	pbap->params = params;
 	ret = obex_get_stream_start(os, path);
 
 	g_free(path);
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH 3/3] Code clean up: cache->folder removed
From: Dmitriy Paliy @ 2010-11-11  7:36 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Dmitriy Paliy
In-Reply-To: <1289460972-2971-1-git-send-email-dmitriy.paliy@nokia.com>

cache->folder is not used anywhere and therefore removed.
---
 plugins/pbap.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/plugins/pbap.c b/plugins/pbap.c
index af60741..660b17d 100644
--- a/plugins/pbap.c
+++ b/plugins/pbap.c
@@ -125,7 +125,6 @@ struct aparam_header {
 struct cache {
 	gboolean valid;
 	uint32_t index;
-	char *folder;
 	GSList *entries;
 };
 
@@ -219,7 +218,6 @@ static const char *cache_find(struct cache *cache, uint32_t handle)
 
 static void cache_clear(struct cache *cache)
 {
-	g_free(cache->folder);
 	g_slist_foreach(cache->entries, (GFunc) cache_entry_free, NULL);
 	g_slist_free(cache->entries);
 	cache->entries = NULL;
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH] Add iwmmxt optimization for sbc for pxa series cpu
From: Keith Mok @ 2010-11-11  8:05 UTC (permalink / raw)
  To: linux-bluetooth

Hi all,

This patch add iwmmxt (Intel wireless mmx, pxa platform) optimzation
for sbc, based on the mmx code.
Have verified the encoded result against the mmx generated one.

Keith

Signed-off-by: Keith Mok <ek9852@gmail.com>
---
 Makefile.am                 |    1 +
 sbc/sbc_primitives.c        |    4 +
 sbc/sbc_primitives_iwmmxt.c |  361 +++++++++++++++++++++++++++++++++++++++++++
 sbc/sbc_primitives_iwmmxt.h |   38 +++++
 4 files changed, 404 insertions(+), 0 deletions(-)
 create mode 100644 sbc/sbc_primitives_iwmmxt.c
 create mode 100644 sbc/sbc_primitives_iwmmxt.h

diff --git a/Makefile.am b/Makefile.am
index da308a7..03a9bf2 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -65,6 +65,7 @@ noinst_LTLIBRARIES += sbc/libsbc.la
 sbc_libsbc_la_SOURCES = sbc/sbc.h sbc/sbc.c sbc/sbc_math.h sbc/sbc_tables.h \
 			sbc/sbc_primitives.h sbc/sbc_primitives.c \
 			sbc/sbc_primitives_mmx.h sbc/sbc_primitives_mmx.c \
+			sbc/sbc_primitives_iwmmxt.h sbc/sbc_primitives_iwmmxt.c \
 			sbc/sbc_primitives_neon.h sbc/sbc_primitives_neon.c \
 			sbc/sbc_primitives_armv6.h sbc/sbc_primitives_armv6.c

diff --git a/sbc/sbc_primitives.c b/sbc/sbc_primitives.c
index f87fb5a..ad780d0 100644
--- a/sbc/sbc_primitives.c
+++ b/sbc/sbc_primitives.c
@@ -33,6 +33,7 @@

 #include "sbc_primitives.h"
 #include "sbc_primitives_mmx.h"
+#include "sbc_primitives_iwmmxt.h"
 #include "sbc_primitives_neon.h"
 #include "sbc_primitives_armv6.h"

@@ -544,6 +545,9 @@ void sbc_init_primitives(struct sbc_encoder_state *state)
 #ifdef SBC_BUILD_WITH_ARMV6_SUPPORT
 	sbc_init_primitives_armv6(state);
 #endif
+#ifdef SBC_BUILD_WITH_IWMMXT_SUPPORT
+	sbc_init_primitives_iwmmxt(state);
+#endif
 #ifdef SBC_BUILD_WITH_NEON_SUPPORT
 	sbc_init_primitives_neon(state);
 #endif
diff --git a/sbc/sbc_primitives_iwmmxt.c b/sbc/sbc_primitives_iwmmxt.c
new file mode 100644
index 0000000..4825998
--- /dev/null
+++ b/sbc/sbc_primitives_iwmmxt.c
@@ -0,0 +1,361 @@
+/*
+ *
+ *  Bluetooth low-complexity, subband codec (SBC) library
+ *
+ *  Copyright (C) 2010 Keith Mok <ek9852@gmail.com>
+ *  Based on sbc_primitives_mmx.c
+ *
+ *
+ *  This library is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU Lesser General Public
+ *  License as published by the Free Software Foundation; either
+ *  version 2.1 of the License, or (at your option) any later version.
+ *
+ *  This library is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ *  Lesser General Public License for more details.
+ *
+ *  You should have received a copy of the GNU Lesser General Public
+ *  License along with this library; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#include <stdint.h>
+#include <limits.h>
+#include "sbc.h"
+#include "sbc_math.h"
+#include "sbc_tables.h"
+
+#include "sbc_primitives_iwmmxt.h"
+
+/*
+ * IWMMXT optimizations
+ */
+
+#ifdef SBC_BUILD_WITH_IWMMXT_SUPPORT
+
+static inline void sbc_analyze_four_iwmmxt(const int16_t *in, int32_t *out,
+					const FIXED_T *consts)
+{
+	asm volatile (
+		"tbcstw       wr4, %2\n"
+		"wldrd        wr0, [%0]\n"
+		"wldrd        wr1, [%0, #8]\n"
+		"wldrd        wr2, [%1]\n"
+		"wldrd        wr3, [%1, #8]\n"
+		"wmadds       wr0, wr2, wr0\n"
+		"wmadds       wr1, wr3, wr1\n"
+		"waddwss      wr0, wr0, wr4\n"
+		"waddwss      wr1, wr1, wr4\n"
+		"\n"
+		"wldrd        wr2, [%0, #16]\n"
+		"wldrd        wr3, [%0, #24]\n"
+		"wldrd        wr4, [%1, #16]\n"
+		"wldrd        wr5, [%1, #24]\n"
+		"wmadds       wr2, wr4, wr2\n"
+		"wmadds       wr3, wr5, wr3\n"
+		"waddwss      wr0, wr2, wr0\n"
+		"waddwss      wr1, wr3, wr1\n"
+		"\n"
+		"wldrd        wr2, [%0, #32]\n"
+		"wldrd        wr3, [%0, #40]\n"
+		"wldrd        wr4, [%1, #32]\n"
+		"wldrd        wr5, [%1, #40]\n"
+		"wmadds       wr2, wr4, wr2\n"
+		"wmadds       wr3, wr5, wr3\n"
+		"waddwss      wr0, wr2, wr0\n"
+		"waddwss      wr1, wr3, wr1\n"
+		"\n"
+		"wldrd        wr2, [%0, #48]\n"
+		"wldrd        wr3, [%0, #56]\n"
+		"wldrd        wr4, [%1, #48]\n"
+		"wldrd        wr5, [%1, #56]\n"
+		"wmadds       wr2, wr4, wr2\n"
+		"wmadds       wr3, wr5, wr3\n"
+		"waddwss      wr0, wr2, wr0\n"
+		"waddwss      wr1, wr3, wr1\n"
+		"\n"
+		"wldrd        wr2, [%0, #64]\n"
+		"wldrd        wr3, [%0, #72]\n"
+		"wldrd        wr4, [%1, #64]\n"
+		"wldrd        wr5, [%1, #72]\n"
+		"wmadds       wr2, wr4, wr2\n"
+		"wmadds       wr3, wr5, wr3\n"
+		"waddwss      wr0, wr2, wr0\n"
+		"waddwss      wr1, wr3, wr1\n"
+		"\n"
+		"tmcr       wcgr0, %4\n"
+		"wsrawg       wr0, wr0, wcgr0\n"
+		"wsrawg       wr1, wr1, wcgr0\n"
+		"wpackwss     wr0, wr0, wr0\n"
+		"wpackwss     wr1, wr1, wr1\n"
+		"\n"
+		"wldrd        wr4, [%1, #80]\n"
+		"wldrd        wr5, [%1, #88]\n"
+		"wldrd        wr6, [%1, #96]\n"
+		"wldrd        wr7, [%1, #104]\n"
+		"wmadds       wr2, wr5, wr0\n"
+		"wmadds       wr0, wr4, wr0\n"
+		"\n"
+		"wmadds       wr3, wr7, wr1\n"
+		"wmadds       wr1, wr6, wr1\n"
+		"waddwss      wr0, wr1, wr0\n"
+		"waddwss      wr2, wr3, wr2\n"
+		"\n"
+		"wstrd        wr0, [%3]\n"
+		"wstrd        wr2, [%3, #8]\n"
+		:
+		: "r" (in), "r" (consts),
+			"r" (1 << (SBC_PROTO_FIXED4_SCALE - 1)), "r" (out),
+			"r" (SBC_PROTO_FIXED4_SCALE)
+		: "memory");
+}
+
+static inline void sbc_analyze_eight_iwmmxt(const int16_t *in, int32_t *out,
+							const FIXED_T *consts)
+{
+	asm volatile (
+		"tbcstw       wr8, %2\n"
+		"wldrd        wr0, [%0]\n"
+		"wldrd        wr1, [%0, #8]\n"
+		"wldrd        wr2, [%0, #16]\n"
+		"wldrd        wr3, [%0, #24]\n"
+		"wldrd        wr4, [%1]\n"
+		"wldrd        wr5, [%1, #8]\n"
+		"wldrd        wr6, [%1, #16]\n"
+		"wldrd        wr7, [%1, #24]\n"
+		"wmadds       wr0, wr0, wr4\n"
+		"wmadds       wr1, wr1, wr5\n"
+		"wmadds       wr2, wr2, wr6\n"
+		"wmadds       wr3, wr3, wr7\n"
+		"waddwss      wr0, wr0, wr8\n"
+		"waddwss      wr1, wr1, wr8\n"
+		"waddwss      wr2, wr2, wr8\n"
+		"waddwss      wr3, wr3, wr8\n"
+		"\n"
+		"wldrd        wr4, [%0, #32]\n"
+		"wldrd        wr5, [%0, #40]\n"
+		"wldrd        wr6, [%0, #48]\n"
+		"wldrd        wr7, [%0, #56]\n"
+		"wldrd        wr8, [%1, #32]\n"
+		"wldrd        wr9, [%1, #40]\n"
+		"wldrd       wr10, [%1, #48]\n"
+		"wldrd       wr11, [%1, #56]\n"
+		"wmadds       wr4, wr4, wr8\n"
+		"wmadds       wr5, wr5, wr9\n"
+		"wmadds       wr6, wr6, wr10\n"
+		"wmadds       wr7, wr7, wr11\n"
+		"waddwss      wr0, wr4, wr0\n"
+		"waddwss      wr1, wr5, wr1\n"
+		"waddwss      wr2, wr6, wr2\n"
+		"waddwss      wr3, wr7, wr3\n"
+		"\n"
+		"wldrd        wr4, [%0, #64]\n"
+		"wldrd        wr5, [%0, #72]\n"
+		"wldrd        wr6, [%0, #80]\n"
+		"wldrd        wr7, [%0, #88]\n"
+		"wldrd        wr8, [%1, #64]\n"
+		"wldrd        wr9, [%1, #72]\n"
+		"wldrd       wr10, [%1, #80]\n"
+		"wldrd       wr11, [%1, #88]\n"
+		"wmadds       wr4, wr4, wr8\n"
+		"wmadds       wr5, wr5, wr9\n"
+		"wmadds       wr6, wr6, wr10\n"
+		"wmadds       wr7, wr7, wr11\n"
+		"waddwss      wr0, wr4, wr0\n"
+		"waddwss      wr1, wr5, wr1\n"
+		"waddwss      wr2, wr6, wr2\n"
+		"waddwss      wr3, wr7, wr3\n"
+		"\n"
+		"wldrd        wr4, [%0, #96]\n"
+		"wldrd        wr5, [%0, #104]\n"
+		"wldrd        wr6, [%0, #112]\n"
+		"wldrd        wr7, [%0, #120]\n"
+		"wldrd        wr8, [%1, #96]\n"
+		"wldrd        wr9, [%1, #104]\n"
+		"wldrd       wr10, [%1, #112]\n"
+		"wldrd       wr11, [%1, #120]\n"
+		"wmadds       wr4, wr4, wr8\n"
+		"wmadds       wr5, wr5, wr9\n"
+		"wmadds       wr6, wr6, wr10\n"
+		"wmadds       wr7, wr7, wr11\n"
+		"waddwss      wr0, wr4, wr0\n"
+		"waddwss      wr1, wr5, wr1\n"
+		"waddwss      wr2, wr6, wr2\n"
+		"waddwss      wr3, wr7, wr3\n"
+		"\n"
+		"wldrd        wr4, [%0, #128]\n"
+		"wldrd        wr5, [%0, #136]\n"
+		"wldrd        wr6, [%0, #144]\n"
+		"wldrd        wr7, [%0, #152]\n"
+		"wldrd        wr8, [%1, #128]\n"
+		"wldrd        wr9, [%1, #136]\n"
+		"wldrd       wr10, [%1, #144]\n"
+		"wldrd       wr11, [%1, #152]\n"
+		"wmadds       wr4, wr4, wr8\n"
+		"wmadds       wr5, wr5, wr9\n"
+		"wmadds       wr6, wr6, wr10\n"
+		"wmadds       wr7, wr7, wr11\n"
+		"waddwss      wr0, wr4, wr0\n"
+		"waddwss      wr1, wr5, wr1\n"
+		"waddwss      wr2, wr6, wr2\n"
+		"waddwss      wr3, wr7, wr3\n"
+		"\n"
+		"tmcr       wcgr0, %4\n"
+		"wsrawg       wr0, wr0, wcgr0\n"
+		"wsrawg       wr1, wr1, wcgr0\n"
+		"wsrawg       wr2, wr2, wcgr0\n"
+		"wsrawg       wr3, wr3, wcgr0\n"
+		"\n"
+		"wpackwss     wr0, wr0, wr0\n"
+		"wpackwss     wr1, wr1, wr1\n"
+		"wpackwss     wr2, wr2, wr2\n"
+		"wpackwss     wr3, wr3, wr3\n"
+		"\n"
+		"wldrd        wr4, [%1, #160]\n"
+		"wldrd        wr5, [%1, #168]\n"
+		"wmadds       wr4, wr4, wr0\n"
+		"wmadds       wr5, wr5, wr0\n"
+		"\n"
+		"wldrd        wr6, [%1, #192]\n"
+		"wldrd        wr7, [%1, #200]\n"
+		"wmadds       wr6, wr6, wr1\n"
+		"wmadds       wr7, wr7, wr1\n"
+		"waddwss      wr4, wr6, wr4\n"
+		"waddwss      wr5, wr7, wr5\n"
+		"\n"
+		"wldrd        wr6, [%1, #224]\n"
+		"wldrd        wr7, [%1, #232]\n"
+		"wmadds       wr6, wr6, wr2\n"
+		"wmadds       wr7, wr7, wr2\n"
+		"waddwss      wr4, wr6, wr4\n"
+		"waddwss      wr5, wr7, wr5\n"
+		"\n"
+		"wldrd        wr6, [%1, #256]\n"
+		"wldrd        wr7, [%1, #264]\n"
+		"wmadds       wr6, wr6, wr3\n"
+		"wmadds       wr7, wr7, wr3\n"
+		"waddwss      wr4, wr6, wr4\n"
+		"waddwss      wr5, wr7, wr5\n"
+		"\n"
+		"wstrd        wr4, [%3]\n"
+		"wstrd        wr5, [%3, #8]\n"
+		"\n"
+		"wldrd        wr4, [%1, #176]\n"
+		"wldrd        wr5, [%1, #184]\n"
+		"wmadds       wr5, wr5, wr0\n"
+		"wmadds       wr0, wr4, wr0\n"
+		"\n"
+		"wldrd        wr4, [%1, #208]\n"
+		"wldrd        wr7, [%1, #216]\n"
+		"wmadds       wr7, wr7, wr1\n"
+		"wmadds       wr1, wr4, wr1\n"
+		"waddwss      wr0, wr1, wr0\n"
+		"waddwss      wr5, wr7, wr5\n"
+		"\n"
+		"wldrd        wr4, [%1, #240]\n"
+		"wldrd        wr7, [%1, #248]\n"
+		"wmadds       wr7, wr7, wr2\n"
+		"wmadds       wr2, wr4, wr2\n"
+		"waddwss      wr0, wr2, wr0\n"
+		"waddwss      wr5, wr7, wr5\n"
+		"\n"
+		"wldrd        wr4, [%1, #272]\n"
+		"wldrd        wr7, [%1, #280]\n"
+		"wmadds       wr7, wr7, wr3\n"
+		"wmadds       wr3, wr4, wr3\n"
+		"waddwss      wr0, wr3, wr0\n"
+		"waddwss      wr5, wr7, wr5\n"
+		"\n"
+		"wstrd        wr0, [%3, #16]\n"
+		"wstrd        wr5, [%3, #24]\n"
+		:
+		: "r" (in), "r" (consts),
+			"r" (1 << (SBC_PROTO_FIXED8_SCALE - 1)), "r" (out),
+			"r" (SBC_PROTO_FIXED8_SCALE)
+		: "memory");
+}
+
+static inline void sbc_analyze_4b_4s_iwmmxt(int16_t *x, int32_t *out,
+						int out_stride)
+{
+	/* Analyze blocks */
+	sbc_analyze_four_iwmmxt(x + 12, out, analysis_consts_fixed4_simd_odd);
+	out += out_stride;
+	sbc_analyze_four_iwmmxt(x + 8, out, analysis_consts_fixed4_simd_even);
+	out += out_stride;
+	sbc_analyze_four_iwmmxt(x + 4, out, analysis_consts_fixed4_simd_odd);
+	out += out_stride;
+	sbc_analyze_four_iwmmxt(x + 0, out, analysis_consts_fixed4_simd_even);
+}
+
+static inline void sbc_analyze_4b_8s_iwmmxt(int16_t *x, int32_t *out,
+						int out_stride)
+{
+	/* Analyze blocks */
+	sbc_analyze_eight_iwmmxt(x + 24, out, analysis_consts_fixed8_simd_odd);
+	out += out_stride;
+	sbc_analyze_eight_iwmmxt(x + 16, out, analysis_consts_fixed8_simd_even);
+	out += out_stride;
+	sbc_analyze_eight_iwmmxt(x + 8, out, analysis_consts_fixed8_simd_odd);
+	out += out_stride;
+	sbc_analyze_eight_iwmmxt(x + 0, out, analysis_consts_fixed8_simd_even);
+}
+
+static void sbc_calc_scalefactors_iwmmxt(
+	int32_t sb_sample_f[16][2][8],
+	uint32_t scale_factor[2][8],
+	int blocks, int channels, int subbands)
+{
+	int ch, sb;
+	intptr_t blk;
+	for (ch = 0; ch < channels; ch++) {
+		for (sb = 0; sb < subbands; sb += 2) {
+			int b;
+			blk = &sb_sample_f[0][ch][sb];
+			b = blocks;
+			asm volatile (
+				"tbcstw       wr0, %4\n"
+			"1:\n"
+				"wldrd        wr1, [%0], %2\n"
+				"wxor         wr2, wr2, wr2\n"
+				"wcmpgtsw     wr3, wr1, wr2\n"
+				"waddwss      wr1, wr1, wr3\n"
+				"wcmpgtsw     wr2, wr2, wr1\n"
+				"wxor         wr1, wr1, wr2\n"
+
+				"wor          wr0, wr0, wr1\n"
+
+				"subs         %1, %1, #1\n"
+				"bne          1b\n"
+
+				"tmrrc        %0, %1, wr0\n"
+				"clz          %0, %0\n"
+				"rsb          %0, %0, %5\n"
+				"str          %0, [%3]\n"
+
+				"clz          %1, %1\n"
+				"rsb          %1, %1, %5\n"
+				"str          %1, [%3, #4]\n"
+			: "+&r" (blk), "+&r" (b)
+			: "i" ((char *) &sb_sample_f[1][0][0] -
+				(char *) &sb_sample_f[0][0][0]),
+				"r" (&scale_factor[ch][sb]),
+				"r" (1 << SCALE_OUT_BITS),
+				"i" (SCALE_OUT_BITS+1)
+			: "memory");
+		}
+	}
+}
+
+void sbc_init_primitives_iwmmxt(struct sbc_encoder_state *state)
+{
+	state->sbc_analyze_4b_4s = sbc_analyze_4b_4s_iwmmxt;
+	state->sbc_analyze_4b_8s = sbc_analyze_4b_8s_iwmmxt;
+	state->sbc_calc_scalefactors = sbc_calc_scalefactors_iwmmxt;
+	state->implementation_info = "IWMMXT";
+}
+
+#endif
diff --git a/sbc/sbc_primitives_iwmmxt.h b/sbc/sbc_primitives_iwmmxt.h
new file mode 100644
index 0000000..827d811
--- /dev/null
+++ b/sbc/sbc_primitives_iwmmxt.h
@@ -0,0 +1,38 @@
+/*
+ *
+ *  Bluetooth low-complexity, subband codec (SBC) library
+ *
+ *  Based on sbc_primitives_mmx.c
+ *
+ *
+ *  This library is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU Lesser General Public
+ *  License as published by the Free Software Foundation; either
+ *  version 2.1 of the License, or (at your option) any later version.
+ *
+ *  This library is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ *  Lesser General Public License for more details.
+ *
+ *  You should have received a copy of the GNU Lesser General Public
+ *  License along with this library; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifndef __SBC_PRIMITIVES_IWMMXT_H
+#define __SBC_PRIMITIVES_IWMMXT_H
+
+#include "sbc_primitives.h"
+
+#if defined(__GNUC__) && defined(__IWMMXT__) && \
+		!defined(SBC_HIGH_PRECISION) && (SCALE_OUT_BITS == 15)
+
+#define SBC_BUILD_WITH_IWMMXT_SUPPORT
+
+void sbc_init_primitives_iwmmxt(struct sbc_encoder_state *encoder_state);
+
+#endif
+
+#endif
-- 
1.6.3.3

^ permalink raw reply related

* Re: [PATCH] Add a new configuration option to disable Low Energy support
From: Johan Hedberg @ 2010-11-11  9:18 UTC (permalink / raw)
  To: Claudio Takahasi; +Cc: linux-bluetooth
In-Reply-To: <1289426161-10045-3-git-send-email-claudio.takahasi@openbossa.org>

Hi Claudio,

On Wed, Nov 10, 2010, Claudio Takahasi wrote:
> @@ -623,6 +624,9 @@ int attrib_server_init(void)
>  
>  	sdp_handle = record->handle;
>  
> +	if (!main_opts.le)
> +		return 0;
> +

If the option is called "le" it shouldn't affect GATT over BR/EDR, so I
think the check in attrib_server_init should only affect the LE socket
(or then rename the option to e.g. main.opts.attrib).

Johan

^ permalink raw reply

* Re: [PATCH] hcitool: Bring up device before sending commands over socket
From: Johan Hedberg @ 2010-11-11  9:26 UTC (permalink / raw)
  To: Anderson Lizardo; +Cc: linux-bluetooth
In-Reply-To: <1289327397-21935-1-git-send-email-anderson.lizardo@openbossa.org>

Hi,

On Tue, Nov 09, 2010, Anderson Lizardo wrote:
> The Texas specific initialization code sends HCI commands over the
> bluetooth socket, but does not bring up the device. This gives these
> errors when running "hciattach /dev/ttyUSB0 texas":
> 
> Found a Texas Instruments' chip!
> Firmware file : /lib/firmware/TIInit_XX.Y.ZZ.bts
> Loaded BTS script version 1
> Cannot send hci command to socket: Network is down
> Can't initialize device: Network is down
> ---
>  tools/hciattach_ti.c |    7 +++++++
>  1 files changed, 7 insertions(+), 0 deletions(-)

Since no one has objected to this I went ahead and pushed it upstream
(after doing s/hcitool/hciattach/ to the summary).

Johan

^ permalink raw reply

* Re: [PATCH] Check HealthApplication path before trying to destroy it
From: Johan Hedberg @ 2010-11-11  9:27 UTC (permalink / raw)
  To: Elvis Pfützenreuter; +Cc: linux-bluetooth
In-Reply-To: <1289355108-4041-1-git-send-email-epx@signove.com>

Hi Elvis,

On Wed, Nov 10, 2010, Elvis Pfützenreuter wrote:
> ---
>  health/hdp.c |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)

Pushed upstream. Thanks.

Johan

^ permalink raw reply

* Re: [PATCH] Fix segfault in HDP during device re-creation
From: Johan Hedberg @ 2010-11-11  9:28 UTC (permalink / raw)
  To: Jose Antonio Santos Cadenas; +Cc: linux-bluetooth
In-Reply-To: <1289382461-10510-1-git-send-email-santoscadenas@gmail.com>

Hi,

On Wed, Nov 10, 2010, Jose Antonio Santos Cadenas wrote:
> ---
>  health/hdp.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/health/hdp.c b/health/hdp.c
> index 1eba8e1..d361b27 100644
> --- a/health/hdp.c
> +++ b/health/hdp.c
> @@ -259,6 +259,7 @@ static void device_unref_mcl(struct hdp_device *hdp_device)
>  	if (!hdp_device->mcl)
>  		return;
>  
> +	mcap_close_mcl(hdp_device->mcl, FALSE);
>  	mcap_mcl_unref(hdp_device->mcl);
>  	hdp_device->mcl = NULL;
>  	hdp_device->mcl_conn = FALSE;

Pushed upstream. Thanks.

Johan

^ permalink raw reply

* Re: [PATCH] Fix pull phonebook reply if filter not set
From: Johan Hedberg @ 2010-11-11  9:29 UTC (permalink / raw)
  To: Lukasz Pawlik; +Cc: linux-bluetooth
In-Reply-To: <1289389387-14308-1-git-send-email-lucas.pawlik@gmail.com>

Hi Lukasz,

On Wed, Nov 10, 2010, Lukasz Pawlik wrote:
> According to the PBAP specification if filter is not set or is set to
> 0x00000000 in the application parameters header all attributes of the vCard
> should be returned. Previously only mandatory attributes were returned in
> phonebook pull reply. This patch fix this and now all currently supported
> vCards attributes will be returned.
> ---
>  plugins/vcard.c |   10 ++++++++--
>  1 files changed, 8 insertions(+), 2 deletions(-)

Pushed upstream. Thanks.

Johan

^ permalink raw reply

* [PATCH] sbc: added "cc" to the clobber list of mmx inline assembly
From: Siarhei Siamashka @ 2010-11-11  9:29 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Siarhei Siamashka

From: Siarhei Siamashka <siarhei.siamashka@nokia.com>

In the case of scale factors calculation optimizations, the inline
assembly code has instructions which update flags register, but
"cc" was not mentioned in the clobber list. When optimizing code,
gcc theoretically is allowed to do a comparison before the inline
assembly block, and a conditional branch after it which would lead
to a problem if the flags register gets clobbered. While this is
apparently not happening in practice with the current versions of
gcc, the clobber list needs to be corrected.

Regarding the other inline assembly blocks. While most likely it
is actually unnecessary based on quick review, "cc" is also added
there to the clobber list because it should have no impact on
performance in practice. It's kind of cargo cult, but relieves
us from the need to track the potential updates of flags register
in all these places.
---
 sbc/sbc_primitives_mmx.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/sbc/sbc_primitives_mmx.c b/sbc/sbc_primitives_mmx.c
index 45c62ac..7f2fbc3 100644
--- a/sbc/sbc_primitives_mmx.c
+++ b/sbc/sbc_primitives_mmx.c
@@ -101,7 +101,7 @@ static inline void sbc_analyze_four_mmx(const int16_t *in, int32_t *out,
 		:
 		: "r" (in), "r" (consts), "r" (&round_c), "r" (out),
 			"i" (SBC_PROTO_FIXED4_SCALE)
-		: "memory");
+		: "cc", "memory");
 }
 
 static inline void sbc_analyze_eight_mmx(const int16_t *in, int32_t *out,
@@ -243,7 +243,7 @@ static inline void sbc_analyze_eight_mmx(const int16_t *in, int32_t *out,
 		:
 		: "r" (in), "r" (consts), "r" (&round_c), "r" (out),
 			"i" (SBC_PROTO_FIXED8_SCALE)
-		: "memory");
+		: "cc", "memory");
 }
 
 static inline void sbc_analyze_4b_4s_mmx(int16_t *x, int32_t *out,
@@ -323,7 +323,7 @@ static void sbc_calc_scalefactors_mmx(
 				"r" (&scale_factor[ch][sb]),
 				"r" (&consts),
 				"i" (SCALE_OUT_BITS)
-			: "memory");
+			: "cc", "memory");
 		}
 	}
 	asm volatile ("emms\n");
-- 
1.7.2.2


^ permalink raw reply related

* Re: [PATCH 1/7] Fix proper type handling in contacts_query_all
From: Johan Hedberg @ 2010-11-11  9:32 UTC (permalink / raw)
  To: Bartosz Szatkowski; +Cc: linux-bluetooth
In-Reply-To: <1289394930-6694-1-git-send-email-bulislaw@linux.com>

Hi Bartosz,

On Wed, Nov 10, 2010, Bartosz Szatkowski wrote:
> Previously all phone numbers, addresses and emails was considered to be "work".
> Now there are three working types for emails and addresses: "work", "home",
> "other" and four for phone numbers - these three as well as "cell".
> ---
>  plugins/phonebook-tracker.c |   61 +++++++++++++++++++++++++++++++------------
>  1 files changed, 44 insertions(+), 17 deletions(-)

All seven patches have been pushed upstream. Thanks.

Johan

^ permalink raw reply

* Re: [PATCH 1/7] Fix invalid memory access when EIR field length is zero
From: Johan Hedberg @ 2010-11-11  9:35 UTC (permalink / raw)
  To: Anderson Lizardo; +Cc: linux-bluetooth, Bruna Moreira
In-Reply-To: <1289411487-6113-1-git-send-email-anderson.lizardo@openbossa.org>

Hi,

On Wed, Nov 10, 2010, Anderson Lizardo wrote:
> -	uint8_t *uuid16;
> -	uint8_t *uuid32;
> -	uint8_t *uuid128;
> +	uint8_t *uuid16 = 0;
> +	uint8_t *uuid32 = 0;
> +	uint8_t *uuid128 = 0;

0 is for integers, NULL for pointers. However, in this case I don't see
why the inialization is needed at all since the rest of the patch seems
to take care of the bug by itself.

Johan

^ permalink raw reply

* [PATCH 1/2] Remove a2dp setup callbacks after they return
From: Luiz Augusto von Dentz @ 2010-11-11  9:40 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>

Since the callback won't be ever called again it make no sense to keep
them, also this cause a2dp_cancel to assume there are still some pending
callbacks to be processed and do not abort when it should.
---
 audio/a2dp.c |   55 +++++++++++++++++++++++++++++++++++--------------------
 1 files changed, 35 insertions(+), 20 deletions(-)

diff --git a/audio/a2dp.c b/audio/a2dp.c
index 3aaf022..a55020d 100644
--- a/audio/a2dp.c
+++ b/audio/a2dp.c
@@ -160,15 +160,18 @@ static gboolean finalize_config(struct a2dp_setup *s)
 	struct avdtp_stream *stream = s->err ? NULL : s->stream;
 
 	setup_ref(s);
-	for (l = s->cb; l != NULL; l = l->next) {
+	for (l = s->cb; l != NULL; ){
 		struct a2dp_setup_cb *cb = l->data;
 
+		l = l->next;
+
 		if (!cb->config_cb)
 			continue;
 
 		cb->config_cb(s->session, s->sep, stream, s->err,
 							cb->user_data);
-		cb->config_cb = NULL;
+		s->cb = g_slist_remove(s->cb, cb);
+		g_free(cb);
 		setup_unref(s);
 	}
 
@@ -193,14 +196,18 @@ static gboolean finalize_resume(struct a2dp_setup *s)
 
 	setup_ref(s);
 
-	for (l = s->cb; l != NULL; l = l->next) {
+	for (l = s->cb; l != NULL; ) {
 		struct a2dp_setup_cb *cb = l->data;
 
-		if (cb && cb->resume_cb) {
-			cb->resume_cb(s->session, s->err, cb->user_data);
-			cb->resume_cb = NULL;
-			setup_unref(s);
-		}
+		l = l->next;
+
+		if (!cb->resume_cb)
+			continue;
+
+		cb->resume_cb(s->session, s->err, cb->user_data);
+		s->cb = g_slist_remove(s->cb, cb);
+		g_free(cb);
+		setup_unref(s);
 	}
 
 	setup_unref(s);
@@ -213,14 +220,18 @@ static gboolean finalize_suspend(struct a2dp_setup *s)
 	GSList *l;
 
 	setup_ref(s);
-	for (l = s->cb; l != NULL; l = l->next) {
+	for (l = s->cb; l != NULL; ) {
 		struct a2dp_setup_cb *cb = l->data;
 
-		if (cb->suspend_cb) {
-			cb->suspend_cb(s->session, s->err, cb->user_data);
-			cb->suspend_cb = NULL;
-			setup_unref(s);
-		}
+		l = l->next;
+
+		if (!cb->suspend_cb)
+			continue;
+
+		cb->suspend_cb(s->session, s->err, cb->user_data);
+		s->cb = g_slist_remove(s->cb, cb);
+		g_free(cb);
+		setup_unref(s);
 	}
 
 	setup_unref(s);
@@ -242,14 +253,18 @@ static gboolean finalize_select(struct a2dp_setup *s, GSList *caps)
 	GSList *l;
 
 	setup_ref(s);
-	for (l = s->cb; l != NULL; l = l->next) {
+	for (l = s->cb; l != NULL; ) {
 		struct a2dp_setup_cb *cb = l->data;
 
-		if (cb->select_cb) {
-			cb->select_cb(s->session, s->sep, caps, cb->user_data);
-			cb->select_cb = NULL;
-			setup_unref(s);
-		}
+		l = l->next;
+
+		if (!cb->select_cb)
+			continue;
+
+		cb->select_cb(s->session, s->sep, caps, cb->user_data);
+		s->cb = g_slist_remove(s->cb, cb);
+		g_free(cb);
+		setup_unref(s);
 	}
 
 	setup_unref(s);
-- 
1.7.1


^ permalink raw reply related

* [PATCH 2/2] Fix not aborting sink stream configuration on disconnect
From: Luiz Augusto von Dentz @ 2010-11-11  9:40 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1289468425-11997-1-git-send-email-luiz.dentz@gmail.com>

From: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>

If stream configuration is not complete it should be aborted so we can
proceed with disconnection process.
---
 audio/sink.c |   28 +++++++++++++++++++++++++++-
 1 files changed, 27 insertions(+), 1 deletions(-)

diff --git a/audio/sink.c b/audio/sink.c
index a9f6307..cb3ca74 100644
--- a/audio/sink.c
+++ b/audio/sink.c
@@ -351,6 +351,12 @@ static void discovery_complete(struct avdtp *session, GSList *seps, struct avdtp
 	struct pending_request *pending;
 	int id;
 
+	if (!sink->connect) {
+		avdtp_unref(sink->session);
+		sink->session = NULL;
+		return;
+	}
+
 	pending = sink->connect;
 
 	if (err) {
@@ -669,11 +675,31 @@ gboolean sink_new_stream(struct audio_device *dev, struct avdtp *session,
 
 gboolean sink_shutdown(struct sink *sink)
 {
-	if (!sink->stream)
+	if (!sink->session)
 		return FALSE;
 
 	avdtp_set_device_disconnect(sink->session, TRUE);
 
+	/* cancel pending connect */
+	if (sink->connect) {
+		struct pending_request *pending = sink->connect;
+
+		if (pending->msg)
+			error_failed(pending->conn, pending->msg,
+							"Stream setup failed");
+		pending_request_free(sink->dev, pending);
+		sink->connect = NULL;
+
+		return TRUE;
+	}
+
+	/* disconnect already ongoing */
+	if (sink->disconnect)
+		return TRUE;
+
+	if (!sink->stream)
+		return FALSE;
+
 	if (avdtp_close(sink->session, sink->stream, FALSE) < 0)
 		return FALSE;
 
-- 
1.7.1


^ permalink raw reply related

* Re: [PATCH 2/7] Refactor get_eir_uuids() to get EIR data length parameter
From: Johan Hedberg @ 2010-11-11  9:42 UTC (permalink / raw)
  To: Anderson Lizardo; +Cc: linux-bluetooth
In-Reply-To: <1289411487-6113-2-git-send-email-anderson.lizardo@openbossa.org>

Hi,

On Wed, Nov 10, 2010, Anderson Lizardo wrote:
> get_eir_uuids() will be reused to parse LE advertising data as well, as
> they share the same format. But for Advertising, maximum data length is
> different (31 bytes vs. 240 bytes for EIR), and the radio is not
> required to send the non-significant (zero-filled) bytes.
> 
> adapter_emit_device_found() now also accepts a EIR data length
> parameter, so it can be reused for LE and can propagate the exact data
> length.
> ---
>  src/adapter.c |   17 ++++++++++-------
>  src/adapter.h |    2 +-
>  src/event.c   |    2 +-
>  3 files changed, 12 insertions(+), 9 deletions(-)

This patch seems fine, but it doesn't apply anymore since the first
patch of the set had issues and I didn't apply it.

Johan

^ permalink raw reply

* Re: [PATCH 3/7] Refactoring adapter_update_found_devices() function
From: Johan Hedberg @ 2010-11-11 10:01 UTC (permalink / raw)
  To: Anderson Lizardo; +Cc: linux-bluetooth, Bruna Moreira
In-Reply-To: <1289411487-6113-3-git-send-email-anderson.lizardo@openbossa.org>

Hi,

On Wed, Nov 10, 2010, Anderson Lizardo wrote:
> +void adapter_update_found_devices(struct btd_adapter *adapter, bdaddr_t *bdaddr,
> +				int8_t rssi, uint32_t class, const char *name,
> +				const char *alias, gboolean legacy,
> +				name_status_t name_status, uint8_t *eir_data)
> +{
> +	struct remote_dev_info *dev;
> +
> +	if (!update_found_devices(adapter, bdaddr, rssi, &dev)) {
> +		dev->class = class;
> +		if (name)
> +			dev->name = g_strdup(name);
> +		if (alias)
> +			dev->alias = g_strdup(alias);
> +		dev->legacy = legacy;
> +		dev->name_status = name_status;
> +	} else if (dev->rssi == rssi)
> +		return;

I find the name and signature of update_found_devices() a little bit
unintuitive which makes following the code that uses it a bit hard.
Additionally it seems you're not using the rssi anymore within the
update_found_devices function so there's no point in passing the rssi to
it. To make the code more readable, could you change the function so
that its usage would look something like:

	struct remote_dev_info *dev;
	gboolean new_dev;

	dev = get_found_dev(adapter, bdaddr, &new_dev);

	if (new_dev) {
		<set the fields for the new device>
	} else if (dev->rssi == rssi)
		return;

	dev->rssi = rssi;
	...

Since the rest of the patches seem to depend on this one I'll stop the
review here and wait until you fix the issues I've mentioned so far.

Johan

^ permalink raw reply

* Re: [PATCH] Print LE link type on hcitool
From: Johan Hedberg @ 2010-11-11 10:08 UTC (permalink / raw)
  To: Sheldon Demario; +Cc: linux-bluetooth
In-Reply-To: <1289415288-18470-1-git-send-email-sheldon.demario@openbossa.org>

Hi Sheldon,

On Wed, Nov 10, 2010, Sheldon Demario wrote:
> ---
>  lib/hci.h       |    1 +
>  tools/hcitool.c |    2 ++
>  2 files changed, 3 insertions(+), 0 deletions(-)

Pushed upstream. Thanks.

Johan

^ permalink raw reply

* Re: [PATCH] Use reference counting of the device object while discovering services
From: Johan Hedberg @ 2010-11-11 10:08 UTC (permalink / raw)
  To: Claudio Takahasi; +Cc: linux-bluetooth
In-Reply-To: <1289426161-10045-1-git-send-email-claudio.takahasi@openbossa.org>

Hi Claudio,

On Wed, Nov 10, 2010, Claudio Takahasi wrote:
> ---
>  attrib/client.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)

Pushed upstream. Thanks.

Johan

^ permalink raw reply

* Re: [PATCH] Fix possible memory leak of the GIOChannel in the attribute server
From: Johan Hedberg @ 2010-11-11 10:09 UTC (permalink / raw)
  To: Claudio Takahasi; +Cc: linux-bluetooth
In-Reply-To: <1289426161-10045-2-git-send-email-claudio.takahasi@openbossa.org>

Hi Claudio,

On Wed, Nov 10, 2010, Claudio Takahasi wrote:
> ---
>  src/attrib-server.c |   32 +++++++++++++++++++-------------
>  1 files changed, 19 insertions(+), 13 deletions(-)

Pushed upstream. Thanks.

Johan

^ permalink raw reply

* Re: [PATCH 1/3] Split up pbap object and pbap session
From: Johan Hedberg @ 2010-11-11 10:25 UTC (permalink / raw)
  To: Dmitriy Paliy; +Cc: linux-bluetooth
In-Reply-To: <1289460972-2971-1-git-send-email-dmitriy.paliy@nokia.com>

Hi Dmitriy,

On Thu, Nov 11, 2010, Dmitriy Paliy wrote:
> +static void *vobject_create(void *user_data)

There's no reason for the void * types here. Use specific types instead.

> @@ -718,10 +736,13 @@ static void *vobject_pull_open(const char *name, int oflag, mode_t mode,
>  		cb = query_result;
>  
>  	ret = phonebook_pull(name, pbap->params, cb, pbap);
> +
>  	if (ret < 0)
>  		goto fail;

No need to add the empty line.

>  
> -	return pbap;
> +	obj = vobject_create(pbap);
> +
> +	return obj;

Just do "return vobject_create(pbap);"

> +	obj = vobject_create(pbap);
> +
> +	return obj;

Same here.

> +	obj = vobject_create(pbap);
> +
> +	return obj;

And here.

Johan

^ permalink raw reply

* Re: [PATCH 2/3] Code clean up: pbap->params = params removed
From: Johan Hedberg @ 2010-11-11 10:26 UTC (permalink / raw)
  To: Dmitriy Paliy; +Cc: linux-bluetooth
In-Reply-To: <1289460972-2971-2-git-send-email-dmitriy.paliy@nokia.com>

Hi Dmitriy,

On Thu, Nov 11, 2010, Dmitriy Paliy wrote:
> pbap->params = params; removed due to the fact that this assignment is
> already used in the same function.
> ---
>  plugins/pbap.c |    1 -
>  1 files changed, 0 insertions(+), 1 deletions(-)
> 
> diff --git a/plugins/pbap.c b/plugins/pbap.c
> index 7b9f1ff..af60741 100644
> --- a/plugins/pbap.c
> +++ b/plugins/pbap.c
> @@ -617,7 +617,6 @@ static int pbap_get(struct obex_session *os, obex_object_t *obj,
>  	if (path == NULL)
>  		return -EBADR;
>  
> -	pbap->params = params;
>  	ret = obex_get_stream_start(os, path);
>  
>  	g_free(path);

Pushed upstream. Thanks.

Johan

^ permalink raw reply

* Re: [PATCH 3/3] Code clean up: cache->folder removed
From: Johan Hedberg @ 2010-11-11 10:26 UTC (permalink / raw)
  To: Dmitriy Paliy; +Cc: linux-bluetooth
In-Reply-To: <1289460972-2971-3-git-send-email-dmitriy.paliy@nokia.com>

Hi Dmitriy,

On Thu, Nov 11, 2010, Dmitriy Paliy wrote:
> cache->folder is not used anywhere and therefore removed.
> ---
>  plugins/pbap.c |    2 --
>  1 files changed, 0 insertions(+), 2 deletions(-)
> 
> diff --git a/plugins/pbap.c b/plugins/pbap.c
> index af60741..660b17d 100644
> --- a/plugins/pbap.c
> +++ b/plugins/pbap.c
> @@ -125,7 +125,6 @@ struct aparam_header {
>  struct cache {
>  	gboolean valid;
>  	uint32_t index;
> -	char *folder;
>  	GSList *entries;
>  };
>  
> @@ -219,7 +218,6 @@ static const char *cache_find(struct cache *cache, uint32_t handle)
>  
>  static void cache_clear(struct cache *cache)
>  {
> -	g_free(cache->folder);
>  	g_slist_foreach(cache->entries, (GFunc) cache_entry_free, NULL);
>  	g_slist_free(cache->entries);
>  	cache->entries = NULL;

This one is also now upstream. Thanks.

Johan

^ 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