Linux bluetooth development
 help / color / mirror / Atom feed
* Re: [PATCH v4 00/16] mSBC tests
From: Frédéric Dalleau @ 2012-11-14 10:00 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1351589975-22640-1-git-send-email-frederic.dalleau@linux.intel.com>

Hi folks, Siarhei,

On 10/30/2012 10:39 AM, Frédéric Dalleau wrote:
> Hi,
>
> v4 integrate latest comments from Siarhei:
>   - remove state->pending field from sbc_encoder_state
>   - add armv6 and iwmmxt primitives
>   - use simd primitive instead of neon
>   - reorder patches so that the SBC_MSBC flag is exposed to users only when
> implementation is complete.

Any feedback ?

Thanks,
Frédéric

^ permalink raw reply

* Re: [RFC] Bluetooth: Add BT_DEFER_SETUP option to sco socket
From: Arnaud Mouiche @ 2012-11-14 10:09 UTC (permalink / raw)
  To: Frédéric Dalleau; +Cc: linux-bluetooth
In-Reply-To: <1352830825-13987-2-git-send-email-frederic.dalleau@linux.intel.com>

Hello,

First thank you for this RFC. Defer (or something similar) is a must 
have for mSBC vs CVSD connection accept from multiple device management, 
at least for HFP case.
some comments / ideas:
- can't we take the opportunity to configure the whole set of eSCO 
parameters (bandwith / latency / retransmission effort) instead of only 
getting the possibility to change the voice settings
- can we introduce a better way to change the voice settings, instead of 
modifying the adapter config (hdev->voice_setting)
- any possibility to reject the eSCO creating (for 'connection rejected 
due to limited resources' reason)

I will try to find some time to try your patch.

regards,
arnaud



On 11/13/2012 07:20 PM, Frédéric Dalleau wrote:
> In order to authenticate and later configure an incoming SCO connection, the
> BT_DEFER_SETUP option is added.
> When an connection is requested, the listening socket is unblocked but the
> effective connection setup happens only on first recv.
> Any send between accept and recv fails with -ENOTCONN.
> ---
>   include/net/bluetooth/hci_core.h |   15 +++++++
>   net/bluetooth/hci_event.c        |   47 +++++++++++++++++++-
>   net/bluetooth/sco.c              |   88 ++++++++++++++++++++++++++++++++++++--
>   3 files changed, 145 insertions(+), 5 deletions(-)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index ef5b85d..2ee0ecb 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -377,6 +377,7 @@ extern int l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb,
>   			      u16 flags);
>   
>   extern int sco_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr);
> +extern int sco_defer(struct hci_dev *hdev, bdaddr_t *bdaddr);
>   extern void sco_connect_cfm(struct hci_conn *hcon, __u8 status);
>   extern void sco_disconn_cfm(struct hci_conn *hcon, __u8 reason);
>   extern int sco_recv_scodata(struct hci_conn *hcon, struct sk_buff *skb);
> @@ -577,6 +578,7 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst);
>   int hci_conn_del(struct hci_conn *conn);
>   void hci_conn_hash_flush(struct hci_dev *hdev);
>   void hci_conn_check_pending(struct hci_dev *hdev);
> +void hci_conn_accept(struct hci_conn *conn, int mask);
>   
>   struct hci_chan *hci_chan_create(struct hci_conn *conn);
>   void hci_chan_del(struct hci_chan *chan);
> @@ -796,6 +798,19 @@ static inline int hci_proto_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr,
>   	}
>   }
>   
> +static inline int hci_proto_defered(struct hci_dev *hdev, bdaddr_t *bdaddr,
> +								__u8 type)
> +{
> +	switch (type) {
> +	case SCO_LINK:
> +	case ESCO_LINK:
> +		return sco_defer(hdev, bdaddr);
> +
> +	default:
> +		return 0;
> +	}
> +}
> +
>   static inline void hci_proto_connect_cfm(struct hci_conn *conn, __u8 status)
>   {
>   	switch (conn->type) {
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 9f5c5f2..167eca0 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -2047,15 +2047,54 @@ unlock:
>   	hci_conn_check_pending(hdev);
>   }
>   
> +void hci_conn_accept(struct hci_conn *conn, int mask)
> +{
> +	struct hci_dev *hdev = conn->hdev;
> +
> +	BT_DBG("conn %p", conn);
> +
> +	if (!lmp_esco_capable(hdev)) {
> +		struct hci_cp_accept_conn_req cp;
> +
> +		conn->state = BT_CONNECT;
> +		bacpy(&cp.bdaddr, &conn->dst);
> +
> +		if (lmp_rswitch_capable(hdev) && (mask & HCI_LM_MASTER))
> +			cp.role = 0x00; /* Become master */
> +		else
> +			cp.role = 0x01; /* Remain slave */
> +
> +		hci_send_cmd(hdev, HCI_OP_ACCEPT_CONN_REQ, sizeof(cp),
> +			     &cp);
> +	} else /* lmp_esco_capable(hdev)) */ {
> +		struct hci_cp_accept_sync_conn_req cp;
> +
> +		conn->state = BT_CONNECT;
> +		bacpy(&cp.bdaddr, &conn->dst);
> +		cp.pkt_type = cpu_to_le16(conn->pkt_type);
> +
> +		cp.tx_bandwidth   = __constant_cpu_to_le32(0x00001f40);
> +		cp.rx_bandwidth   = __constant_cpu_to_le32(0x00001f40);
> +		cp.max_latency    = __constant_cpu_to_le16(0xffff);
> +		cp.content_format = cpu_to_le16(hdev->voice_setting);
> +		cp.retrans_effort = 0xff;
> +
> +		hci_send_cmd(hdev, HCI_OP_ACCEPT_SYNC_CONN_REQ,
> +			     sizeof(cp), &cp);
> +	}
> +}
> +
>   static void hci_conn_request_evt(struct hci_dev *hdev, struct sk_buff *skb)
>   {
>   	struct hci_ev_conn_request *ev = (void *) skb->data;
>   	int mask = hdev->link_mode;
> +	int defered;
>   
>   	BT_DBG("%s bdaddr %pMR type 0x%x", hdev->name, &ev->bdaddr,
>   	       ev->link_type);
>   
>   	mask |= hci_proto_connect_ind(hdev, &ev->bdaddr, ev->link_type);
> +	defered = hci_proto_defered(hdev, &ev->bdaddr, ev->link_type);
>   
>   	if ((mask & HCI_LM_ACCEPT) &&
>   	    !hci_blacklist_lookup(hdev, &ev->bdaddr)) {
> @@ -2085,7 +2124,8 @@ static void hci_conn_request_evt(struct hci_dev *hdev, struct sk_buff *skb)
>   
>   		hci_dev_unlock(hdev);
>   
> -		if (ev->link_type == ACL_LINK || !lmp_esco_capable(hdev)) {
> +		if (ev->link_type == ACL_LINK ||
> +				(!defered && !lmp_esco_capable(hdev))) {
>   			struct hci_cp_accept_conn_req cp;
>   
>   			bacpy(&cp.bdaddr, &ev->bdaddr);
> @@ -2097,7 +2137,7 @@ static void hci_conn_request_evt(struct hci_dev *hdev, struct sk_buff *skb)
>   
>   			hci_send_cmd(hdev, HCI_OP_ACCEPT_CONN_REQ, sizeof(cp),
>   				     &cp);
> -		} else {
> +		} else if (!defered) {
>   			struct hci_cp_accept_sync_conn_req cp;
>   
>   			bacpy(&cp.bdaddr, &ev->bdaddr);
> @@ -2111,6 +2151,9 @@ static void hci_conn_request_evt(struct hci_dev *hdev, struct sk_buff *skb)
>   
>   			hci_send_cmd(hdev, HCI_OP_ACCEPT_SYNC_CONN_REQ,
>   				     sizeof(cp), &cp);
> +		} else {
> +			hci_proto_connect_cfm(conn, 0);
> +			hci_conn_put(conn);
>   		}
>   	} else {
>   		/* Connection rejected */
> diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
> index 450cdcd..416801a 100644
> --- a/net/bluetooth/sco.c
> +++ b/net/bluetooth/sco.c
> @@ -662,16 +662,57 @@ static int sco_sock_sendmsg(struct kiocb *iocb, struct socket *sock,
>   	return err;
>   }
>   
> +static int sco_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
> +			      struct msghdr *msg, size_t len, int flags)
> +{
> +	struct sock *sk = sock->sk;
> +	struct sco_pinfo *pi = sco_pi(sk);
> +
> +	lock_sock(sk);
> +
> +	if (sk->sk_state == BT_CONNECT &&
> +			test_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags)) {
> +		hci_conn_accept(pi->conn->hcon, 0);
> +		sk->sk_state = BT_CONNECT2;
> +
> +		release_sock(sk);
> +		return 0;
> +	}
> +
> +	release_sock(sk);
> +
> +	return bt_sock_recvmsg(iocb, sock, msg, len, flags);
> +}
> +
>   static int sco_sock_setsockopt(struct socket *sock, int level, int optname, char __user *optval, unsigned int optlen)
>   {
>   	struct sock *sk = sock->sk;
>   	int err = 0;
> +	u32 opt;
>   
>   	BT_DBG("sk %p", sk);
>   
>   	lock_sock(sk);
>   
>   	switch (optname) {
> +
> +	case BT_DEFER_SETUP:
> +		if (sk->sk_state != BT_BOUND && sk->sk_state != BT_LISTEN) {
> +			err = -EINVAL;
> +			break;
> +		}
> +
> +		if (get_user(opt, (u32 __user *) optval)) {
> +			err = -EFAULT;
> +			break;
> +		}
> +
> +		if (opt)
> +			set_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags);
> +		else
> +			clear_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags);
> +		break;
> +
>   	default:
>   		err = -ENOPROTOOPT;
>   		break;
> @@ -753,6 +794,19 @@ static int sco_sock_getsockopt(struct socket *sock, int level, int optname, char
>   	lock_sock(sk);
>   
>   	switch (optname) {
> +
> +	case BT_DEFER_SETUP:
> +		if (sk->sk_state != BT_BOUND && sk->sk_state != BT_LISTEN) {
> +			err = -EINVAL;
> +			break;
> +		}
> +
> +		if (put_user(test_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags),
> +			     (u32 __user *) optval))
> +			err = -EFAULT;
> +
> +		break;
> +
>   	default:
>   		err = -ENOPROTOOPT;
>   		break;
> @@ -874,7 +928,10 @@ static void sco_conn_ready(struct sco_conn *conn)
>   		hci_conn_hold(conn->hcon);
>   		__sco_chan_add(conn, sk, parent);
>   
> -		sk->sk_state = BT_CONNECTED;
> +		if (test_bit(BT_SK_DEFER_SETUP, &bt_sk(parent)->flags))
> +			sk->sk_state = BT_CONNECT;
> +		else
> +			sk->sk_state = BT_CONNECTED;
>   
>   		/* Wake up parent */
>   		parent->sk_data_ready(parent, 1);
> @@ -912,6 +969,32 @@ int sco_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr)
>   	return lm;
>   }
>   
> +int sco_defer(struct hci_dev *hdev, bdaddr_t *bdaddr)
> +{
> +	struct sock *sk;
> +	struct hlist_node *node;
> +	int defer = 0;
> +
> +	BT_DBG("hdev %s, bdaddr %pMR", hdev->name, bdaddr);
> +
> +	/* Find listening sockets */
> +	read_lock(&sco_sk_list.lock);
> +	sk_for_each(sk, node, &sco_sk_list.head) {
> +		if (sk->sk_state != BT_LISTEN)
> +			continue;
> +
> +		if (!bacmp(&bt_sk(sk)->src, &hdev->bdaddr) ||
> +		    !bacmp(&bt_sk(sk)->src, BDADDR_ANY)) {
> +			if (test_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags))
> +				defer = 1;
> +			break;
> +		}
> +	}
> +	read_unlock(&sco_sk_list.lock);
> +
> +	return defer;
> +}
> +
>   void sco_connect_cfm(struct hci_conn *hcon, __u8 status)
>   {
>   	BT_DBG("hcon %p bdaddr %pMR status %d", hcon, &hcon->dst, status);
> @@ -992,7 +1075,7 @@ static const struct proto_ops sco_sock_ops = {
>   	.accept		= sco_sock_accept,
>   	.getname	= sco_sock_getname,
>   	.sendmsg	= sco_sock_sendmsg,
> -	.recvmsg	= bt_sock_recvmsg,
> +	.recvmsg	= sco_sock_recvmsg,
>   	.poll		= bt_sock_poll,
>   	.ioctl		= bt_sock_ioctl,
>   	.mmap		= sock_no_mmap,
> @@ -1036,7 +1119,6 @@ int __init sco_init(void)
>   			BT_ERR("Failed to create SCO debug file");
>   	}
>   
> -	BT_INFO("SCO socket layer initialized");
>   
>   	return 0;
>   


^ permalink raw reply

* Re: [RFC] Bluetooth: Add BT_DEFER_SETUP option to sco socket
From: Frédéric Dalleau @ 2012-11-14 10:10 UTC (permalink / raw)
  To: Michael Knudsen; +Cc: linux-bluetooth
In-Reply-To: <50A355CF.4030809@samsung.com>

Hi Michael,

On 11/14/2012 09:26 AM, Michael Knudsen wrote:

> My idea is to define (I'm not sure if this is exactly the same as what you
> suggested yourself) a bit, LM_DEFER, that e.g. sco_connect_ind() may return
> and propagate this bit to hci_conn_request_evt().  This way all policing of
> accepting SCO connections is handled from a single entry point into sco.c.

You detailed it much further than I did, but yes it is the same idea. I 
fully support it! If everybody agrees, then I can update the patch.

hci.h would receive the folowing line:

+ #define HCI_LM_DEFER   0x4000

Best regards,
Frédéric

^ permalink raw reply

* [PATCH 1/4] adapter: Convert storage aliases
From: Frédéric Danis @ 2012-11-14 10:16 UTC (permalink / raw)
  To: linux-bluetooth

---
 src/adapter.c |   66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/src/adapter.c b/src/adapter.c
index f9eb3af..ea7b8ff 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -86,6 +86,7 @@
 
 #define SETTINGS_PATH STORAGEDIR "/%s/settings"
 #define CACHE_PATH STORAGEDIR "/%s/cache/%s"
+#define DEVICE_INFO_PATH STORAGEDIR "/%s/%s/info"
 
 static GSList *adapter_drivers = NULL;
 
@@ -2533,6 +2534,68 @@ static void convert_names_entry(char *key, char *value, void *user_data)
 	store_cached_name(&local, &peer, value);
 }
 
+struct device_converter {
+	char *address;
+	void (*cb)(GKeyFile *key_file, void *value);
+};
+
+static void convert_aliases_entry(GKeyFile *key_file, void *value)
+{
+	g_key_file_set_string(key_file, "General", "Alias", value);
+}
+
+static void convert_entry(char *key, char *value, void *user_data)
+{
+	struct device_converter *converter = user_data;
+	char filename[PATH_MAX + 1];
+	GKeyFile *key_file;
+	char *data;
+	gsize length = 0;
+
+	if (key[17] == '#')
+		key[17] = '\0';
+
+	if (bachk(key) != 0)
+		return;
+
+	snprintf(filename, PATH_MAX, DEVICE_INFO_PATH, converter->address, key);
+	filename[PATH_MAX] = '\0';
+	create_file(filename, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
+
+	key_file = g_key_file_new();
+	g_key_file_load_from_file(key_file, filename, 0, NULL);
+	converter->cb(key_file, value);
+
+	data = g_key_file_to_data(key_file, &length, NULL);
+	g_file_set_contents(filename, data, length, NULL);
+	g_free(data);
+
+	g_key_file_free(key_file);
+}
+
+static void convert_file(char *file, char *address,
+				void (*cb)(GKeyFile *key_file, void *value))
+{
+	char filename[PATH_MAX + 1];
+	struct device_converter converter;
+	char *str;
+
+	snprintf(filename, PATH_MAX, STORAGEDIR "/%s/%s", address, file);
+	filename[PATH_MAX] = '\0';
+
+	str = textfile_get(filename, "converted");
+	if (str && strcmp(str, "yes") == 0) {
+		DBG("Legacy file %s already converted", filename);
+	} else {
+		converter.address = address;
+		converter.cb = cb;
+
+		textfile_foreach(filename, convert_entry, &converter);
+		textfile_put(filename, "converted", "yes");
+	}
+	free(str);
+}
+
 static void convert_device_storage(struct btd_adapter *adapter)
 {
 	char filename[PATH_MAX + 1];
@@ -2553,6 +2616,9 @@ static void convert_device_storage(struct btd_adapter *adapter)
 		textfile_put(filename, "converted", "yes");
 	}
 	free(str);
+
+	/* Convert aliases */
+	convert_file("aliases", address, convert_aliases_entry);
 }
 
 static void convert_config(struct btd_adapter *adapter, const char *filename,
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 2/4] device: Use new storage for device alias
From: Frédéric Danis @ 2012-11-14 10:16 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1352888166-7391-1-git-send-email-frederic.danis@linux.intel.com>

As set_alias() changes value in device structure and write to
storage is deferred, property set could not return failure.
---
 src/device.c |   28 +++++++++++-----------------
 1 file changed, 11 insertions(+), 17 deletions(-)

diff --git a/src/device.c b/src/device.c
index 48970d2..c508d18 100644
--- a/src/device.c
+++ b/src/device.c
@@ -222,6 +222,10 @@ static gboolean store_device_info_cb(gpointer user_data)
 
 	g_key_file_set_string(key_file, "General", "Name", device->name);
 
+	if (device->alias != NULL)
+		g_key_file_set_string(key_file, "General", "Alias",
+					device->alias);
+
 	ba2str(adapter_get_address(device->adapter), adapter_addr);
 	ba2str(&device->bdaddr, device_addr);
 	snprintf(filename, PATH_MAX, INFO_PATH, adapter_addr, device_addr);
@@ -432,28 +436,17 @@ static void set_alias(GDBusPendingPropertySet id, const char *alias,
 								void *data)
 {
 	struct btd_device *device = data;
-	struct btd_adapter *adapter = device->adapter;
-	char srcaddr[18], dstaddr[18];
-	int err;
 
 	/* No change */
 	if ((device->alias == NULL && g_str_equal(alias, "")) ||
 					g_strcmp0(device->alias, alias) == 0)
 		return g_dbus_pending_property_success(id);
 
-	ba2str(adapter_get_address(adapter), srcaddr);
-	ba2str(&device->bdaddr, dstaddr);
-
-	/* Remove alias if empty string */
-	err = write_device_alias(srcaddr, dstaddr, device->bdaddr_type,
-					g_str_equal(alias, "") ? NULL : alias);
-	if (err < 0)
-		return g_dbus_pending_property_error(id,
-				ERROR_INTERFACE ".Failed", strerror(-err));
-
 	g_free(device->alias);
 	device->alias = g_str_equal(alias, "") ? NULL : g_strdup(alias);
 
+	store_device_info(device);
+
 	g_dbus_emit_property_changed(btd_get_dbus_connection(),
 				device->path, DEVICE_INTERFACE, "Alias");
 
@@ -1746,6 +1739,10 @@ static void load_info(struct btd_device *device, const gchar *local,
 		g_free(str);
 	}
 
+	/* Load alias */
+	device->alias = g_key_file_get_string(key_file, "General", "Alias",
+						NULL);
+
 	if (store_needed)
 		store_device_info(device);
 
@@ -1759,7 +1756,7 @@ struct btd_device *device_create(struct btd_adapter *adapter,
 	struct btd_device *device;
 	const gchar *adapter_path = adapter_get_path(adapter);
 	const bdaddr_t *src;
-	char srcaddr[18], alias[MAX_NAME_LENGTH + 1];
+	char srcaddr[18];
 	uint16_t vendor, product, version;
 
 	device = g_try_malloc0(sizeof(struct btd_device));
@@ -1790,9 +1787,6 @@ struct btd_device *device_create(struct btd_adapter *adapter,
 
 	load_info(device, srcaddr, address);
 
-	if (read_device_alias(srcaddr, address, bdaddr_type, alias,
-							sizeof(alias)) == 0)
-		device->alias = g_strdup(alias);
 	device->trusted = read_trust(src, address, device->bdaddr_type);
 
 	if (read_blocked(src, &device->bdaddr, device->bdaddr_type))
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 3/4] adapter: Convert storage trusts
From: Frédéric Danis @ 2012-11-14 10:16 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1352888166-7391-1-git-send-email-frederic.danis@linux.intel.com>

All entry in trusts file are set to [all] (if a device is not trusted
it does not have entry in this file).
So, we do not need to check entry value and set device (entry key) as
trusted.
---
 src/adapter.c |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/src/adapter.c b/src/adapter.c
index ea7b8ff..99d58b5 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2544,6 +2544,11 @@ static void convert_aliases_entry(GKeyFile *key_file, void *value)
 	g_key_file_set_string(key_file, "General", "Alias", value);
 }
 
+static void convert_trusts_entry(GKeyFile *key_file, void *value)
+{
+	g_key_file_set_boolean(key_file, "General", "Trusted", TRUE);
+}
+
 static void convert_entry(char *key, char *value, void *user_data)
 {
 	struct device_converter *converter = user_data;
@@ -2619,6 +2624,9 @@ static void convert_device_storage(struct btd_adapter *adapter)
 
 	/* Convert aliases */
 	convert_file("aliases", address, convert_aliases_entry);
+
+	/* Convert trusts */
+	convert_file("trusts", address, convert_trusts_entry);
 }
 
 static void convert_config(struct btd_adapter *adapter, const char *filename,
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 4/4] device: Use new storage for device trust
From: Frédéric Danis @ 2012-11-14 10:16 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1352888166-7391-1-git-send-email-frederic.danis@linux.intel.com>

As set_trust() changes value in device structure and write to
storage is deferred, property set could not return failure.
---
 src/device.c |   22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/src/device.c b/src/device.c
index c508d18..792fe3d 100644
--- a/src/device.c
+++ b/src/device.c
@@ -226,6 +226,9 @@ static gboolean store_device_info_cb(gpointer user_data)
 		g_key_file_set_string(key_file, "General", "Alias",
 					device->alias);
 
+	g_key_file_set_boolean(key_file, "General", "Trusted",
+				device->trusted);
+
 	ba2str(adapter_get_address(device->adapter), adapter_addr);
 	ba2str(&device->bdaddr, device_addr);
 	snprintf(filename, PATH_MAX, INFO_PATH, adapter_addr, device_addr);
@@ -713,23 +716,14 @@ static gboolean dev_property_get_trusted(const GDBusPropertyTable *property,
 static void set_trust(GDBusPendingPropertySet id, gboolean value, void *data)
 {
 	struct btd_device *device = data;
-	struct btd_adapter *adapter = device->adapter;
-	char srcaddr[18], dstaddr[18];
-	int err;
 
 	if (device->trusted == value)
 		return g_dbus_pending_property_success(id);
 
-	ba2str(adapter_get_address(adapter), srcaddr);
-	ba2str(&device->bdaddr, dstaddr);
-
-	err = write_trust(srcaddr, dstaddr, device->bdaddr_type, value);
-	if (err < 0)
-		return g_dbus_pending_property_error(id,
-				ERROR_INTERFACE ".Failed", strerror(-err));
-
 	device->trusted = value;
 
+	store_device_info(device);
+
 	g_dbus_emit_property_changed(btd_get_dbus_connection(),
 				device->path, DEVICE_INTERFACE, "Trusted");
 
@@ -1743,6 +1737,10 @@ static void load_info(struct btd_device *device, const gchar *local,
 	device->alias = g_key_file_get_string(key_file, "General", "Alias",
 						NULL);
 
+	/* Load trust */
+	device->trusted = g_key_file_get_boolean(key_file, "General",
+							"Trusted", NULL);
+
 	if (store_needed)
 		store_device_info(device);
 
@@ -1787,8 +1785,6 @@ struct btd_device *device_create(struct btd_adapter *adapter,
 
 	load_info(device, srcaddr, address);
 
-	device->trusted = read_trust(src, address, device->bdaddr_type);
-
 	if (read_blocked(src, &device->bdaddr, device->bdaddr_type))
 		device_block(device, FALSE);
 
-- 
1.7.9.5


^ permalink raw reply related

* Re: [RFC] Bluetooth: Add BT_DEFER_SETUP option to sco socket
From: Frédéric Dalleau @ 2012-11-14 10:18 UTC (permalink / raw)
  To: Arnaud Mouiche; +Cc: linux-bluetooth
In-Reply-To: <50A36DD1.1080507@invoxia.com>

Hi Arnaud,

On 11/14/2012 11:09 AM, Arnaud Mouiche wrote:
> Hello,
> I will try to find some time to try your patch.

It is a bit early since I haven't tested it myself, should be in much
better shape before end of the week.

Regards,
Frédéric

^ permalink raw reply

* RE: [RFC] Bluetooth: Add BT_DEFER_SETUP option to sco socket
From: Michael Knudsen @ 2012-11-14 10:21 UTC (permalink / raw)
  To: 'Arnaud Mouiche', 'Frédéric Dalleau'
  Cc: linux-bluetooth
In-Reply-To: <50A36DD1.1080507@invoxia.com>

> - can't we take the opportunity to configure the whole set
> of eSCO parameters (bandwith / latency / retransmission effort)
> instead of only getting the possibility to change the voice
> settings

I am currently looking into this as mentioned in my mail last
week:

	<509BB56A.80609@samsung.com>

If anyone has any input for this, I'm all ears.

-m.



^ permalink raw reply

* Re: [RFC] Bluetooth: Add BT_DEFER_SETUP option to sco socket
From: Arnaud Mouiche @ 2012-11-14 10:33 UTC (permalink / raw)
  To: frederic.dalleau; +Cc: linux-bluetooth
In-Reply-To: <50A3700C.6090801@linux.intel.com>

What is your plan once the kernel will gain the defer capability.
- is it up to bluetoothd/audio to accept and configure ?
- is it the external HFP apps (for the HFP case) to do the job ?

arnaud

On 11/14/2012 11:18 AM, Frédéric Dalleau wrote:
> Hi Arnaud,
>
> On 11/14/2012 11:09 AM, Arnaud Mouiche wrote:
>> Hello,
>> I will try to find some time to try your patch.
>
> It is a bit early since I haven't tested it myself, should be in much
> better shape before end of the week.
>
> Regards,
> Frédéric


^ permalink raw reply

* Re: [PATCH 1/4] adapter: Convert storage aliases
From: Johan Hedberg @ 2012-11-14 10:45 UTC (permalink / raw)
  To: Frédéric Danis; +Cc: linux-bluetooth
In-Reply-To: <1352888166-7391-1-git-send-email-frederic.danis@linux.intel.com>

Hi Frederic,

On Wed, Nov 14, 2012, Frédéric Danis wrote:
>  #define SETTINGS_PATH STORAGEDIR "/%s/settings"
>  #define CACHE_PATH STORAGEDIR "/%s/cache/%s"
> +#define DEVICE_INFO_PATH STORAGEDIR "/%s/%s/info"

Why do you have these separate defines? Do you need them in multiple
places? The define names are longer than the actual values and you're
just obfuscating the actual call that uses them (since you need to look
in two places to figure out of the format parameters are actually
matching the format string). So please don't use these.

Johan

^ permalink raw reply

* Re: [PATCH 1/4] adapter: Convert storage aliases
From: Frederic Danis @ 2012-11-14 11:04 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <20121114104553.GA6519@x220>

Hello Johan,

On 14/11/2012 11:45, Johan Hedberg wrote:
> Hi Frederic,
>
> On Wed, Nov 14, 2012, Frédéric Danis wrote:
>>   #define SETTINGS_PATH STORAGEDIR "/%s/settings"
>>   #define CACHE_PATH STORAGEDIR "/%s/cache/%s"
>> +#define DEVICE_INFO_PATH STORAGEDIR "/%s/%s/info"
>
> Why do you have these separate defines? Do you need them in multiple
> places? The define names are longer than the actual values and you're
> just obfuscating the actual call that uses them (since you need to look
> in two places to figure out of the format parameters are actually
> matching the format string). So please don't use these.
>

CACHE_PATH and DEVICE_INFO_PATH are only used in one place, so they can 
be removed.

SETTINGS_PATH is used in both store_adapter_info() and load_config(), so 
I think it is better to keep it.

Fred


-- 
Frederic Danis                            Open Source Technology Center
frederic.danis@intel.com                              Intel Corporation


^ permalink raw reply

* Re: [PATCH 1/4] adapter: Convert storage aliases
From: Johan Hedberg @ 2012-11-14 11:11 UTC (permalink / raw)
  To: Frederic Danis; +Cc: linux-bluetooth
In-Reply-To: <50A37AC7.1090009@linux.intel.com>

Hi Frederic,

On Wed, Nov 14, 2012, Frederic Danis wrote:
> On 14/11/2012 11:45, Johan Hedberg wrote:
> >Hi Frederic,
> >
> >On Wed, Nov 14, 2012, Frédéric Danis wrote:
> >>  #define SETTINGS_PATH STORAGEDIR "/%s/settings"
> >>  #define CACHE_PATH STORAGEDIR "/%s/cache/%s"
> >>+#define DEVICE_INFO_PATH STORAGEDIR "/%s/%s/info"
> >
> >Why do you have these separate defines? Do you need them in multiple
> >places? The define names are longer than the actual values and you're
> >just obfuscating the actual call that uses them (since you need to look
> >in two places to figure out of the format parameters are actually
> >matching the format string). So please don't use these.
> >
> 
> CACHE_PATH and DEVICE_INFO_PATH are only used in one place, so they
> can be removed.
> 
> SETTINGS_PATH is used in both store_adapter_info() and
> load_config(), so I think it is better to keep it.

I'd still just duplicate it in both places for readability. These
locations will be static at least for the entire 5.x series and maybe
longer.

Johan

^ permalink raw reply

* [PATCH v2 1/5] adapter: Convert storage aliases
From: Frédéric Danis @ 2012-11-14 11:28 UTC (permalink / raw)
  To: linux-bluetooth

---
 src/adapter.c |   66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/src/adapter.c b/src/adapter.c
index f9eb3af..7ba821e 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2533,6 +2533,69 @@ static void convert_names_entry(char *key, char *value, void *user_data)
 	store_cached_name(&local, &peer, value);
 }
 
+struct device_converter {
+	char *address;
+	void (*cb)(GKeyFile *key_file, void *value);
+};
+
+static void convert_aliases_entry(GKeyFile *key_file, void *value)
+{
+	g_key_file_set_string(key_file, "General", "Alias", value);
+}
+
+static void convert_entry(char *key, char *value, void *user_data)
+{
+	struct device_converter *converter = user_data;
+	char filename[PATH_MAX + 1];
+	GKeyFile *key_file;
+	char *data;
+	gsize length = 0;
+
+	if (key[17] == '#')
+		key[17] = '\0';
+
+	if (bachk(key) != 0)
+		return;
+
+	snprintf(filename, PATH_MAX, STORAGEDIR "/%s/%s/info",
+			converter->address, key);
+	filename[PATH_MAX] = '\0';
+	create_file(filename, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
+
+	key_file = g_key_file_new();
+	g_key_file_load_from_file(key_file, filename, 0, NULL);
+	converter->cb(key_file, value);
+
+	data = g_key_file_to_data(key_file, &length, NULL);
+	g_file_set_contents(filename, data, length, NULL);
+	g_free(data);
+
+	g_key_file_free(key_file);
+}
+
+static void convert_file(char *file, char *address,
+				void (*cb)(GKeyFile *key_file, void *value))
+{
+	char filename[PATH_MAX + 1];
+	struct device_converter converter;
+	char *str;
+
+	snprintf(filename, PATH_MAX, STORAGEDIR "/%s/%s", address, file);
+	filename[PATH_MAX] = '\0';
+
+	str = textfile_get(filename, "converted");
+	if (str && strcmp(str, "yes") == 0) {
+		DBG("Legacy file %s already converted", filename);
+	} else {
+		converter.address = address;
+		converter.cb = cb;
+
+		textfile_foreach(filename, convert_entry, &converter);
+		textfile_put(filename, "converted", "yes");
+	}
+	free(str);
+}
+
 static void convert_device_storage(struct btd_adapter *adapter)
 {
 	char filename[PATH_MAX + 1];
@@ -2553,6 +2616,9 @@ static void convert_device_storage(struct btd_adapter *adapter)
 		textfile_put(filename, "converted", "yes");
 	}
 	free(str);
+
+	/* Convert aliases */
+	convert_file("aliases", address, convert_aliases_entry);
 }
 
 static void convert_config(struct btd_adapter *adapter, const char *filename,
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH v2 2/5] device: Use new storage for device alias
From: Frédéric Danis @ 2012-11-14 11:28 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1352892533-15154-1-git-send-email-frederic.danis@linux.intel.com>

As set_alias() changes value in device structure and write to
storage is deferred, property set could not return failure.
---
 src/device.c |   28 +++++++++++-----------------
 1 file changed, 11 insertions(+), 17 deletions(-)

diff --git a/src/device.c b/src/device.c
index 48970d2..c508d18 100644
--- a/src/device.c
+++ b/src/device.c
@@ -222,6 +222,10 @@ static gboolean store_device_info_cb(gpointer user_data)
 
 	g_key_file_set_string(key_file, "General", "Name", device->name);
 
+	if (device->alias != NULL)
+		g_key_file_set_string(key_file, "General", "Alias",
+					device->alias);
+
 	ba2str(adapter_get_address(device->adapter), adapter_addr);
 	ba2str(&device->bdaddr, device_addr);
 	snprintf(filename, PATH_MAX, INFO_PATH, adapter_addr, device_addr);
@@ -432,28 +436,17 @@ static void set_alias(GDBusPendingPropertySet id, const char *alias,
 								void *data)
 {
 	struct btd_device *device = data;
-	struct btd_adapter *adapter = device->adapter;
-	char srcaddr[18], dstaddr[18];
-	int err;
 
 	/* No change */
 	if ((device->alias == NULL && g_str_equal(alias, "")) ||
 					g_strcmp0(device->alias, alias) == 0)
 		return g_dbus_pending_property_success(id);
 
-	ba2str(adapter_get_address(adapter), srcaddr);
-	ba2str(&device->bdaddr, dstaddr);
-
-	/* Remove alias if empty string */
-	err = write_device_alias(srcaddr, dstaddr, device->bdaddr_type,
-					g_str_equal(alias, "") ? NULL : alias);
-	if (err < 0)
-		return g_dbus_pending_property_error(id,
-				ERROR_INTERFACE ".Failed", strerror(-err));
-
 	g_free(device->alias);
 	device->alias = g_str_equal(alias, "") ? NULL : g_strdup(alias);
 
+	store_device_info(device);
+
 	g_dbus_emit_property_changed(btd_get_dbus_connection(),
 				device->path, DEVICE_INTERFACE, "Alias");
 
@@ -1746,6 +1739,10 @@ static void load_info(struct btd_device *device, const gchar *local,
 		g_free(str);
 	}
 
+	/* Load alias */
+	device->alias = g_key_file_get_string(key_file, "General", "Alias",
+						NULL);
+
 	if (store_needed)
 		store_device_info(device);
 
@@ -1759,7 +1756,7 @@ struct btd_device *device_create(struct btd_adapter *adapter,
 	struct btd_device *device;
 	const gchar *adapter_path = adapter_get_path(adapter);
 	const bdaddr_t *src;
-	char srcaddr[18], alias[MAX_NAME_LENGTH + 1];
+	char srcaddr[18];
 	uint16_t vendor, product, version;
 
 	device = g_try_malloc0(sizeof(struct btd_device));
@@ -1790,9 +1787,6 @@ struct btd_device *device_create(struct btd_adapter *adapter,
 
 	load_info(device, srcaddr, address);
 
-	if (read_device_alias(srcaddr, address, bdaddr_type, alias,
-							sizeof(alias)) == 0)
-		device->alias = g_strdup(alias);
 	device->trusted = read_trust(src, address, device->bdaddr_type);
 
 	if (read_blocked(src, &device->bdaddr, device->bdaddr_type))
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH v2 3/5] adapter: Convert storage trusts
From: Frédéric Danis @ 2012-11-14 11:28 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1352892533-15154-1-git-send-email-frederic.danis@linux.intel.com>

All entry in trusts file are set to [all] (if a device is not trusted
it does not have entry in this file).
So, we do not need to check entry value and set device (entry key) as
trusted.
---
 src/adapter.c |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/src/adapter.c b/src/adapter.c
index 7ba821e..bdea378 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2543,6 +2543,11 @@ static void convert_aliases_entry(GKeyFile *key_file, void *value)
 	g_key_file_set_string(key_file, "General", "Alias", value);
 }
 
+static void convert_trusts_entry(GKeyFile *key_file, void *value)
+{
+	g_key_file_set_boolean(key_file, "General", "Trusted", TRUE);
+}
+
 static void convert_entry(char *key, char *value, void *user_data)
 {
 	struct device_converter *converter = user_data;
@@ -2619,6 +2624,9 @@ static void convert_device_storage(struct btd_adapter *adapter)
 
 	/* Convert aliases */
 	convert_file("aliases", address, convert_aliases_entry);
+
+	/* Convert trusts */
+	convert_file("trusts", address, convert_trusts_entry);
 }
 
 static void convert_config(struct btd_adapter *adapter, const char *filename,
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH v2 4/5] device: Use new storage for device trust
From: Frédéric Danis @ 2012-11-14 11:28 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1352892533-15154-1-git-send-email-frederic.danis@linux.intel.com>

As set_trust() changes value in device structure and write to
storage is deferred, property set could not return failure.
---
 src/device.c |   22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/src/device.c b/src/device.c
index c508d18..792fe3d 100644
--- a/src/device.c
+++ b/src/device.c
@@ -226,6 +226,9 @@ static gboolean store_device_info_cb(gpointer user_data)
 		g_key_file_set_string(key_file, "General", "Alias",
 					device->alias);
 
+	g_key_file_set_boolean(key_file, "General", "Trusted",
+				device->trusted);
+
 	ba2str(adapter_get_address(device->adapter), adapter_addr);
 	ba2str(&device->bdaddr, device_addr);
 	snprintf(filename, PATH_MAX, INFO_PATH, adapter_addr, device_addr);
@@ -713,23 +716,14 @@ static gboolean dev_property_get_trusted(const GDBusPropertyTable *property,
 static void set_trust(GDBusPendingPropertySet id, gboolean value, void *data)
 {
 	struct btd_device *device = data;
-	struct btd_adapter *adapter = device->adapter;
-	char srcaddr[18], dstaddr[18];
-	int err;
 
 	if (device->trusted == value)
 		return g_dbus_pending_property_success(id);
 
-	ba2str(adapter_get_address(adapter), srcaddr);
-	ba2str(&device->bdaddr, dstaddr);
-
-	err = write_trust(srcaddr, dstaddr, device->bdaddr_type, value);
-	if (err < 0)
-		return g_dbus_pending_property_error(id,
-				ERROR_INTERFACE ".Failed", strerror(-err));
-
 	device->trusted = value;
 
+	store_device_info(device);
+
 	g_dbus_emit_property_changed(btd_get_dbus_connection(),
 				device->path, DEVICE_INTERFACE, "Trusted");
 
@@ -1743,6 +1737,10 @@ static void load_info(struct btd_device *device, const gchar *local,
 	device->alias = g_key_file_get_string(key_file, "General", "Alias",
 						NULL);
 
+	/* Load trust */
+	device->trusted = g_key_file_get_boolean(key_file, "General",
+							"Trusted", NULL);
+
 	if (store_needed)
 		store_device_info(device);
 
@@ -1787,8 +1785,6 @@ struct btd_device *device_create(struct btd_adapter *adapter,
 
 	load_info(device, srcaddr, address);
 
-	device->trusted = read_trust(src, address, device->bdaddr_type);
-
 	if (read_blocked(src, &device->bdaddr, device->bdaddr_type))
 		device_block(device, FALSE);
 
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH v2 5/5] adapter: Remove storage path #defines
From: Frédéric Danis @ 2012-11-14 11:28 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1352892533-15154-1-git-send-email-frederic.danis@linux.intel.com>

SETTINGS_PATH and CACHE_PATH will be static for the
entire 5.x series
---
 src/adapter.c |    9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index bdea378..3f09295 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -84,9 +84,6 @@
 #define REMOVE_TEMP_TIMEOUT (3 * 60)
 #define PENDING_FOUND_MAX 5
 
-#define SETTINGS_PATH STORAGEDIR "/%s/settings"
-#define CACHE_PATH STORAGEDIR "/%s/cache/%s"
-
 static GSList *adapter_drivers = NULL;
 
 enum session_req_type {
@@ -244,7 +241,7 @@ static void store_adapter_info(struct btd_adapter *adapter)
 					adapter->discov_timeout);
 
 	ba2str(&adapter->bdaddr, address);
-	snprintf(filename, PATH_MAX, SETTINGS_PATH, address);
+	snprintf(filename, PATH_MAX, STORAGEDIR "/%s/settings", address);
 	filename[PATH_MAX] = '\0';
 
 	create_file(filename, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
@@ -267,7 +264,7 @@ static void store_cached_name(const bdaddr_t *local, const bdaddr_t *peer,
 
 	ba2str(local, s_addr);
 	ba2str(peer, d_addr);
-	snprintf(filename, PATH_MAX, CACHE_PATH, s_addr, d_addr);
+	snprintf(filename, PATH_MAX, STORAGEDIR "/%s/cache/%s", s_addr, d_addr);
 	filename[PATH_MAX] = '\0';
 	create_file(filename, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 
@@ -2703,7 +2700,7 @@ static void load_config(struct btd_adapter *adapter)
 
 	key_file = g_key_file_new();
 
-	snprintf(filename, PATH_MAX, SETTINGS_PATH, address);
+	snprintf(filename, PATH_MAX, STORAGEDIR "/%s/settings", address);
 	filename[PATH_MAX] = '\0';
 
 	if (!g_key_file_load_from_file(key_file, filename, 0, NULL))
-- 
1.7.9.5


^ permalink raw reply related

* Re: [PATCH v2 1/5] adapter: Convert storage aliases
From: Johan Hedberg @ 2012-11-14 11:54 UTC (permalink / raw)
  To: Frédéric Danis; +Cc: linux-bluetooth
In-Reply-To: <1352892533-15154-1-git-send-email-frederic.danis@linux.intel.com>

Hi Frederic,

On Wed, Nov 14, 2012, Frédéric Danis wrote:
> ---
>  src/adapter.c |   66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 66 insertions(+)

All patches in this set have been applied. Thanks.

Johan

^ permalink raw reply

* Re: [PATCHv1 11/12] Bluetooth: AMP: Use l2cap_physical_cfm in phylink complete evt
From: Andrei Emeltchenko @ 2012-11-14 12:19 UTC (permalink / raw)
  To: Mat Martineau; +Cc: linux-bluetooth, marcel
In-Reply-To: <alpine.DEB.2.02.1211130911200.27104@mathewm-linux>

Hi Mat,

On Tue, Nov 13, 2012 at 09:29:38AM -0800, Mat Martineau wrote:
> >>>>>+void amp_physical_cfm(struct hci_conn *bredr_hcon, struct hci_conn *hs_hcon)
> >>>>>+{
> >>>>>+	struct hci_dev *bredr_hdev = hci_dev_hold(bredr_hcon->hdev);
> >>>>>+	struct amp_mgr *mgr = hs_hcon->amp_mgr;
> >>>>>+	struct l2cap_chan *bredr_chan;
> >>>>>+
> >>>>>+	BT_DBG("bredr_hcon %p hs_hcon %p mgr %p", bredr_hcon, hs_hcon, mgr);
> >>>>>+
> >>>>>+	if (!bredr_hdev || !mgr || !mgr->bredr_chan)
> >>>>>+		return;
> >>>>>+
> >>>>>+	bredr_chan = mgr->bredr_chan;
> >>>>>+
> >>>>>+	set_bit(FLAG_EFS_ENABLE, &bredr_chan->flags);
> >>>>>+	bredr_chan->ctrl_id = hs_hcon->remote_id;
> >>>>>+	bredr_chan->hs_hcon = hs_hcon;
> >>>>>+	bredr_chan->conn->mtu = hs_hcon->hdev->block_mtu;
> >>>>>+	bredr_chan->fcs = L2CAP_FCS_NONE;
> >>
> >>Changing FCS requires L2CAP reconfiguration for the channel, and
> >>chan->fcs shouldn't be modified until reconfiguration happens.
> >>While it doesn't make much sense to do so, the remote device may
> >>want to keep FCS enabled.  The move may also fail and you don't want
> >>to forget the original FCS setting in that case.
> >
> >So we agree that FCS shall not be used for High Speed channels. I was
> >thinking more about the case where we start sending data right over HS
> >channel. The configuration should be just started.
> 
> No matter what the BlueZ implementation prefers, it must be able to
> handle a remote device that requests FCS during L2CAP config.  If
> one side requests FCS, then it must be used.

The idea here is to disable FCS on our side, we would still respect FCS
requests from remote side. This code needs to be moved to l2cap_do_create.

> Do we want BlueZ to always ignore the L2CAP FCS socket option on AMP
> controllers?  (Marcel?)  This checksum is always redundant with
> 802.11 AMP controllers.
> 
> >What would be the better place to init FCS? l2cap_physical_cfm after
> >checking channel moving status?
> 
> I think it should be as late as possible before L2CAP configuration
> begins in order to be sure the channel is really on AMP.  For the
> "create channel" case, set_default_fcs could see if an AMP link is
> being used and turn the FCS off.

set_default_fcs sets FCS when configuration is finished, this way we
cannot respect other side FCS settings.

Best regards 
Andrei Emeltchenko 

^ permalink raw reply

* Re: [PATCH BlueZ 1/2] build: Require D-Bus 1.5 or later
From: Syam Sidhardhan @ 2012-11-14 13:03 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <CAFBvHif+xS12ywGEUdpycTtU--TVZSTgAb4mJuN_O5NKJSz36w@mail.gmail.com>

Hi,

On Tue, Nov 13, 2012 at 10:01 PM, Syam Sidhardhan
<syamsidhardh@gmail.com> wrote:
> Hi Marcel,
>
> On Fri, Nov 9, 2012 at 4:14 PM, Marcel Holtmann <marcel@holtmann.org> wro=
te:
>> Hi Syam,
>>
>>> The gdbus require D-Bus 1.5 version.
>>>
>>> Log:
>>>   CC     gdbus/object.o
>>> gdbus/object.c: In function =E2=80=98properties_set=E2=80=99:
>>> gdbus/object.c:876:7: error: =E2=80=98DBUS_ERROR_UNKNOWN_PROPERTY=E2=80=
=99 undeclared
>>>  (first use in this function)
>>> gdbus/object.c:876:7: note: each undeclared identifier is reported
>>>  only once for each function it appears in
>>> gdbus/object.c:881:6: error: =E2=80=98DBUS_ERROR_PROPERTY_READ_ONLY=E2=
=80=99
>>>  undeclared (first use in this function)
>>> make[1]: *** [gdbus/object.o] Error 1
>>> make: *** [all] Error 2
>>
>> if they are still simple defines, then lets just #ifndef them and define
>> them in gdbus/object.c
>>
>
> Yes, those are simple defines. I'll send a patch according to your sugges=
tion.
>

Missing the mailing list in my earlier reply. CC-ing here.

Regards,
Syam.

^ permalink raw reply

* [PATCH BlueZ 1/1] gdbus: Fix compilation error due to missing #defines
From: Syam Sidhardhan @ 2012-11-14 13:07 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Syam Sidhardhan

Since these are simple #define strings, we are defining it here
instead of upgrading to D-Bus 1.5 or later.

Log:
  CC     gdbus/object.o
gdbus/object.c: In function ‘properties_set’:
gdbus/object.c:876:7: error: ‘DBUS_ERROR_UNKNOWN_PROPERTY’ undeclared
 (first use in this function)
gdbus/object.c:876:7: note: each undeclared identifier is reported
 only once for each function it appears in
gdbus/object.c:881:6: error: ‘DBUS_ERROR_PROPERTY_READ_ONLY’
 undeclared (first use in this function)
make[1]: *** [gdbus/object.o] Error 1
make: *** [all] Error 2
---
 gdbus/object.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/gdbus/object.c b/gdbus/object.c
index e3ad067..3101ca6 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -39,6 +39,14 @@
 
 #define DBUS_INTERFACE_OBJECT_MANAGER "org.freedesktop.DBus.ObjectManager"
 
+#ifndef DBUS_ERROR_UNKNOWN_PROPERTY
+#define DBUS_ERROR_UNKNOWN_PROPERTY "org.freedesktop.DBus.Error.UnknownProperty"
+#endif
+
+#ifndef DBUS_ERROR_PROPERTY_READ_ONLY
+#define DBUS_ERROR_PROPERTY_READ_ONLY "org.freedesktop.DBus.Error.PropertyReadOnly"
+#endif
+
 struct generic_data {
 	unsigned int refcount;
 	DBusConnection *conn;
-- 
1.7.4.1


^ permalink raw reply related

* Re: [RFC] Bluetooth: Add BT_DEFER_SETUP option to sco socket
From: Marcel Holtmann @ 2012-11-14 14:39 UTC (permalink / raw)
  To: frederic.dalleau; +Cc: Michael Knudsen, linux-bluetooth
In-Reply-To: <50A36E06.2060206@linux.intel.com>

Hi Fred,

> > My idea is to define (I'm not sure if this is exactly the same as what you
> > suggested yourself) a bit, LM_DEFER, that e.g. sco_connect_ind() may return
> > and propagate this bit to hci_conn_request_evt().  This way all policing of
> > accepting SCO connections is handled from a single entry point into sco.c.
> 
> You detailed it much further than I did, but yes it is the same idea. I 
> fully support it! If everybody agrees, then I can update the patch.
> 
> hci.h would receive the folowing line:
> 
> + #define HCI_LM_DEFER   0x4000

can we actually not that that. This is an old public API and I rather
not extend it.

Regards

Marcel



^ permalink raw reply

* Re: [PATCH v4 11/16] sbc: Add SBC_MSBC flag to enable 15 block encoding
From: Marcel Holtmann @ 2012-11-14 14:49 UTC (permalink / raw)
  To: Frédéric Dalleau; +Cc: linux-bluetooth
In-Reply-To: <1351589975-22640-12-git-send-email-frederic.dalleau@linux.intel.com>

Hi Fred,

> ---
>  sbc/sbc.c |   23 +++++++++++++++--------
>  sbc/sbc.h |    3 +++
>  2 files changed, 18 insertions(+), 8 deletions(-)
> 
> diff --git a/sbc/sbc.c b/sbc/sbc.c
> index ffdf05d..22fa898 100644
> --- a/sbc/sbc.c
> +++ b/sbc/sbc.c
> @@ -52,6 +52,9 @@
>  
>  #define SBC_SYNCWORD	0x9C
>  
> +#define MSBC_SYNCWORD       0xAD
> +#define MSBC_BLOCKS         15
> +
>  /* This structure contains an unpacked SBC frame.
>     Yes, there is probably quite some unused space herein */
>  struct sbc_frame {
> @@ -903,12 +906,12 @@ static ssize_t sbc_pack_frame(uint8_t *data, struct sbc_frame *frame, size_t len
>  	}
>  }
>  
> -static void sbc_encoder_init(struct sbc_encoder_state *state,
> -					const struct sbc_frame *frame)
> +static void sbc_encoder_init(unsigned long flags,
> +		struct sbc_encoder_state *state, const struct sbc_frame *frame)
>  {
>  	memset(&state->X, 0, sizeof(state->X));
>  	state->position = (SBC_X_BUFFER_SIZE - frame->subbands * 9) & ~7;
> -	state->increment = 4;
> +	state->increment = flags & SBC_MSBC ? 1 : 4;

just for pure cosmetics, use an if statement.

>  
>  	sbc_init_primitives(state);
>  }
> @@ -922,6 +925,7 @@ struct sbc_priv {
>  
>  static void sbc_set_defaults(sbc_t *sbc, unsigned long flags)
>  {
> +	sbc->flags = flags;
>  	sbc->frequency = SBC_FREQ_44100;
>  	sbc->mode = SBC_MODE_STEREO;
>  	sbc->subbands = SBC_SB_8;
> @@ -1057,12 +1061,13 @@ SBC_EXPORT ssize_t sbc_encode(sbc_t *sbc, const void *input, size_t input_len,
>  		priv->frame.subband_mode = sbc->subbands;
>  		priv->frame.subbands = sbc->subbands ? 8 : 4;
>  		priv->frame.block_mode = sbc->blocks;
> -		priv->frame.blocks = 4 + (sbc->blocks * 4);
> +		priv->frame.blocks = sbc->flags & SBC_MSBC ?
> +					MSBC_BLOCKS : 4 + (sbc->blocks * 4);

Same here.

>  		priv->frame.bitpool = sbc->bitpool;
>  		priv->frame.codesize = sbc_get_codesize(sbc);
>  		priv->frame.length = sbc_get_frame_length(sbc);
>  
> -		sbc_encoder_init(&priv->enc_state, &priv->frame);
> +		sbc_encoder_init(sbc->flags, &priv->enc_state, &priv->frame);
>  		priv->init = 1;
>  	} else if (priv->frame.bitpool != sbc->bitpool) {
>  		priv->frame.length = sbc_get_frame_length(sbc);
> @@ -1141,7 +1146,7 @@ SBC_EXPORT size_t sbc_get_frame_length(sbc_t *sbc)
>  		return priv->frame.length;
>  
>  	subbands = sbc->subbands ? 8 : 4;
> -	blocks = 4 + (sbc->blocks * 4);
> +	blocks = sbc->flags & SBC_MSBC ? MSBC_BLOCKS : 4 + (sbc->blocks * 4);

And for this one as well.

>  	channels = sbc->mode == SBC_MODE_MONO ? 1 : 2;
>  	joint = sbc->mode == SBC_MODE_JOINT_STEREO ? 1 : 0;
>  	bitpool = sbc->bitpool;
> @@ -1165,7 +1170,8 @@ SBC_EXPORT unsigned sbc_get_frame_duration(sbc_t *sbc)
>  	priv = sbc->priv;
>  	if (!priv->init) {
>  		subbands = sbc->subbands ? 8 : 4;
> -		blocks = 4 + (sbc->blocks * 4);
> +		blocks = sbc->flags & SBC_MSBC ?
> +					MSBC_BLOCKS : 4 + (sbc->blocks * 4);

Even here it is better cosmetics.

>  	} else {
>  		subbands = priv->frame.subbands;
>  		blocks = priv->frame.blocks;
> @@ -1202,7 +1208,8 @@ SBC_EXPORT size_t sbc_get_codesize(sbc_t *sbc)
>  	priv = sbc->priv;
>  	if (!priv->init) {
>  		subbands = sbc->subbands ? 8 : 4;
> -		blocks = 4 + (sbc->blocks * 4);
> +		blocks = sbc->flags & SBC_MSBC ?
> +					MSBC_BLOCKS : 4 + (sbc->blocks * 4);

Here as well.

>  		channels = sbc->mode == SBC_MODE_MONO ? 1 : 2;
>  	} else {
>  		subbands = priv->frame.subbands;
> diff --git a/sbc/sbc.h b/sbc/sbc.h
> index bbd45da..3511119 100644
> --- a/sbc/sbc.h
> +++ b/sbc/sbc.h
> @@ -64,6 +64,9 @@ extern "C" {
>  #define SBC_LE			0x00
>  #define SBC_BE			0x01
>  
> +/* Additional features */
> +#define SBC_MSBC		0x01
> +

I am debating to actually call this SBC_FLAG_MSBC instead of just
SBC_MSBC. Anyone any comments?

Regards

Marcel



^ permalink raw reply

* Re: [PATCH v4 00/16] mSBC tests
From: Marcel Holtmann @ 2012-11-14 14:50 UTC (permalink / raw)
  To: frederic.dalleau; +Cc: linux-bluetooth
In-Reply-To: <50A36BAC.4090604@linux.intel.com>

Hi Fred,

> > v4 integrate latest comments from Siarhei:
> >   - remove state->pending field from sbc_encoder_state
> >   - add armv6 and iwmmxt primitives
> >   - use simd primitive instead of neon
> >   - reorder patches so that the SBC_MSBC flag is exposed to users only when
> > implementation is complete.
> 
> Any feedback ?

beside a tiny cosmetic comment, I have nothing. However I am not the
expert here. If Siarhei looks over this and is fine with it, I will be
as well.

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