Linux bluetooth development
 help / color / mirror / Atom feed
* [PATCH BlueZ 3/5] unit: Add gdbus/client_get_boolean_property
From: Luiz Augusto von Dentz @ 2013-01-16 13:54 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1358344480-23777-1-git-send-email-luiz.dentz@gmail.com>

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

---
 unit/test-gdbus-client.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 65 insertions(+)

diff --git a/unit/test-gdbus-client.c b/unit/test-gdbus-client.c
index cf80087..ca82bad 100644
--- a/unit/test-gdbus-client.c
+++ b/unit/test-gdbus-client.c
@@ -392,6 +392,68 @@ static void client_get_string_property(void)
 	destroy_context(context);
 }
 
+static void proxy_get_boolean(GDBusProxy *proxy, void *user_data)
+{
+	struct context *context = user_data;
+	DBusMessageIter iter;
+	dbus_bool_t value;
+
+	if (g_test_verbose())
+		g_print("proxy %s found\n",
+					g_dbus_proxy_get_interface(proxy));
+
+	g_assert(g_dbus_proxy_get_property(proxy, "Boolean", &iter));
+	g_assert(dbus_message_iter_get_arg_type(&iter) == DBUS_TYPE_BOOLEAN);
+
+	dbus_message_iter_get_basic(&iter, &value);
+	g_assert(value == TRUE);
+
+	g_dbus_client_unref(context->dbus_client);
+}
+
+static gboolean get_boolean(const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *data)
+{
+	dbus_bool_t value = TRUE;
+
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_BOOLEAN, &value);
+
+	return TRUE;
+}
+
+static void client_get_boolean_property(void)
+{
+	struct context *context = create_context();
+	static const GDBusPropertyTable boolean_properties[] = {
+		{ "Boolean", "b", get_boolean },
+		{ },
+	};
+
+	if (context == NULL)
+		return;
+
+	g_dbus_register_interface(context->dbus_conn,
+				SERVICE_PATH, SERVICE_NAME,
+				methods, signals, boolean_properties,
+				NULL, NULL);
+
+	context->dbus_client = g_dbus_client_new(context->dbus_conn,
+						SERVICE_NAME, SERVICE_PATH);
+
+	g_dbus_client_set_proxy_handlers(context->dbus_client,
+						proxy_get_boolean,
+						NULL, NULL, context);
+	g_dbus_client_set_disconnect_watch(context->dbus_client,
+						disconnect_handler, context);
+
+	g_main_loop_run(context->main_loop);
+
+	g_dbus_unregister_interface(context->dbus_conn,
+					SERVICE_PATH, SERVICE_NAME);
+
+	destroy_context(context);
+}
+
 int main(int argc, char *argv[])
 {
 	g_test_init(&argc, &argv, NULL);
@@ -404,6 +466,9 @@ int main(int argc, char *argv[])
 	g_test_add_func("/gdbus/client_get_string_property",
 						client_get_string_property);
 
+	g_test_add_func("/gdbus/client_get_boolean_property",
+						client_get_boolean_property);
+
 	g_test_add_func("/gdbus/client_get_dict_property",
 						client_get_dict_property);
 
-- 
1.8.0.2


^ permalink raw reply related

* [PATCH BlueZ 4/5] unit: Add gdbus/client_get_array_property
From: Luiz Augusto von Dentz @ 2013-01-16 13:54 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1358344480-23777-1-git-send-email-luiz.dentz@gmail.com>

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

---
 unit/test-gdbus-client.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 79 insertions(+)

diff --git a/unit/test-gdbus-client.c b/unit/test-gdbus-client.c
index ca82bad..534d8ce 100644
--- a/unit/test-gdbus-client.c
+++ b/unit/test-gdbus-client.c
@@ -454,6 +454,82 @@ static void client_get_boolean_property(void)
 	destroy_context(context);
 }
 
+static void proxy_get_array(GDBusProxy *proxy, void *user_data)
+{
+	struct context *context = user_data;
+	DBusMessageIter iter, entry;
+	const char *value1, *value2;
+
+	if (g_test_verbose())
+		g_print("proxy %s found\n",
+					g_dbus_proxy_get_interface(proxy));
+
+	g_assert(g_dbus_proxy_get_property(proxy, "Array", &iter));
+	g_assert(dbus_message_iter_get_arg_type(&iter) == DBUS_TYPE_ARRAY);
+
+	dbus_message_iter_recurse(&iter, &entry);
+	g_assert(dbus_message_iter_get_arg_type(&entry) == DBUS_TYPE_STRING);
+
+	dbus_message_iter_get_basic(&entry, &value1);
+	g_assert(g_strcmp0(value1, "value1") == 0);
+
+	dbus_message_iter_next(&entry);
+	g_assert(dbus_message_iter_get_arg_type(&entry) == DBUS_TYPE_STRING);
+
+	dbus_message_iter_get_basic(&entry, &value2);
+	g_assert(g_strcmp0(value2, "value2") == 0);
+
+	g_dbus_client_unref(context->dbus_client);
+}
+
+static gboolean get_array(const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *data)
+{
+	const char *value[2] = { "value1", "value2" };
+	DBusMessageIter array;
+
+	dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY, "s", &array);
+
+	dbus_message_iter_append_basic(&array, DBUS_TYPE_STRING, &value[0]);
+	dbus_message_iter_append_basic(&array, DBUS_TYPE_STRING, &value[1]);
+
+	dbus_message_iter_close_container(iter, &array);
+
+	return TRUE;
+}
+
+static void client_get_array_property(void)
+{
+	struct context *context = create_context();
+	static const GDBusPropertyTable array_properties[] = {
+		{ "Array", "as", get_array },
+		{ },
+	};
+
+	if (context == NULL)
+		return;
+
+	g_dbus_register_interface(context->dbus_conn,
+				SERVICE_PATH, SERVICE_NAME,
+				methods, signals, array_properties,
+				NULL, NULL);
+
+	context->dbus_client = g_dbus_client_new(context->dbus_conn,
+						SERVICE_NAME, SERVICE_PATH);
+
+	g_dbus_client_set_proxy_handlers(context->dbus_client, proxy_get_array,
+						NULL, NULL, context);
+	g_dbus_client_set_disconnect_watch(context->dbus_client,
+						disconnect_handler, context);
+
+	g_main_loop_run(context->main_loop);
+
+	g_dbus_unregister_interface(context->dbus_conn,
+					SERVICE_PATH, SERVICE_NAME);
+
+	destroy_context(context);
+}
+
 int main(int argc, char *argv[])
 {
 	g_test_init(&argc, &argv, NULL);
@@ -469,6 +545,9 @@ int main(int argc, char *argv[])
 	g_test_add_func("/gdbus/client_get_boolean_property",
 						client_get_boolean_property);
 
+	g_test_add_func("/gdbus/client_get_array_property",
+						client_get_array_property);
+
 	g_test_add_func("/gdbus/client_get_dict_property",
 						client_get_dict_property);
 
-- 
1.8.0.2


^ permalink raw reply related

* [PATCH BlueZ 5/5] unit: Add gdbus/client_get_uint64_property
From: Luiz Augusto von Dentz @ 2013-01-16 13:54 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1358344480-23777-1-git-send-email-luiz.dentz@gmail.com>

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

---
 unit/test-gdbus-client.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 65 insertions(+)

diff --git a/unit/test-gdbus-client.c b/unit/test-gdbus-client.c
index 534d8ce..7fc97ed 100644
--- a/unit/test-gdbus-client.c
+++ b/unit/test-gdbus-client.c
@@ -530,6 +530,68 @@ static void client_get_array_property(void)
 	destroy_context(context);
 }
 
+static void proxy_get_uint64(GDBusProxy *proxy, void *user_data)
+{
+	struct context *context = user_data;
+	DBusMessageIter iter;
+	guint64 value;
+
+	if (g_test_verbose())
+		g_print("proxy %s found\n",
+					g_dbus_proxy_get_interface(proxy));
+
+	g_assert(g_dbus_proxy_get_property(proxy, "Number", &iter));
+	g_assert(dbus_message_iter_get_arg_type(&iter) == DBUS_TYPE_UINT64);
+
+	dbus_message_iter_get_basic(&iter, &value);
+	g_assert(value == G_MAXUINT64);
+
+	g_dbus_client_unref(context->dbus_client);
+}
+
+static gboolean get_uint64(const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *data)
+{
+	guint64 value = G_MAXUINT64;
+
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_UINT64, &value);
+
+	return TRUE;
+}
+
+static void client_get_uint64_property(void)
+{
+	struct context *context = create_context();
+	static const GDBusPropertyTable uint64_properties[] = {
+		{ "Number", "t", get_uint64 },
+		{ },
+	};
+
+	if (context == NULL)
+		return;
+
+	g_dbus_register_interface(context->dbus_conn,
+				SERVICE_PATH, SERVICE_NAME,
+				methods, signals, uint64_properties,
+				NULL, NULL);
+
+	context->dbus_client = g_dbus_client_new(context->dbus_conn,
+						SERVICE_NAME, SERVICE_PATH);
+
+	g_dbus_client_set_proxy_handlers(context->dbus_client,
+						proxy_get_uint64,
+						NULL, NULL, context);
+	g_dbus_client_set_disconnect_watch(context->dbus_client,
+						disconnect_handler, context);
+
+	g_main_loop_run(context->main_loop);
+
+	g_dbus_unregister_interface(context->dbus_conn,
+					SERVICE_PATH, SERVICE_NAME);
+
+	destroy_context(context);
+}
+
 int main(int argc, char *argv[])
 {
 	g_test_init(&argc, &argv, NULL);
@@ -545,6 +607,9 @@ int main(int argc, char *argv[])
 	g_test_add_func("/gdbus/client_get_boolean_property",
 						client_get_boolean_property);
 
+	g_test_add_func("/gdbus/client_get_uint64_property",
+						client_get_uint64_property);
+
 	g_test_add_func("/gdbus/client_get_array_property",
 						client_get_array_property);
 
-- 
1.8.0.2


^ permalink raw reply related

* [PATCH] Bluetooth: Fix Class of Device indication when powering off
From: Johan Hedberg @ 2013-01-16 14:15 UTC (permalink / raw)
  To: linux-bluetooth

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

When a HCI device is powered off the Management interface specification
dictates that the class of device value is indicated as zero. This patch
fixes sending of the appropriate class of device changed event when a
HCI device is powered off.

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

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index fc171f2..54f3ddba 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2987,7 +2987,13 @@ int mgmt_powered(struct hci_dev *hdev, u8 powered)
 		}
 	} else {
 		u8 status = MGMT_STATUS_NOT_POWERED;
+		u8 zero_cod[] = { 0, 0, 0 };
+
 		mgmt_pending_foreach(0, hdev, cmd_status_rsp, &status);
+
+		if (memcmp(hdev->dev_class, zero_cod, sizeof(zero_cod)) != 0)
+			mgmt_event(MGMT_EV_CLASS_OF_DEV_CHANGED, hdev,
+				   zero_cod, sizeof(zero_cod), NULL);
 	}
 
 	err = new_settings(hdev, match.sk);
-- 
1.7.10.4


^ permalink raw reply related

* [PATCH] core: Fix allow registering same custom property
From: Frédéric Dalleau @ 2013-01-16 16:59 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Frédéric Dalleau

With several adapters, MediaEndpoint custom property is only created for the
first adapter. When a new connection happens on another adapter
btd_profile_custom_property_exists fails because the custom property doesn't
exist for this last adapter. In case of HFP ofono fails with error: "Media
Endpoint missing error".

This patch allows the creation of several custom property with same name that
can be distinguished based on a 'key'. When querying the property, additional
checking must happen. For example endpoint_properties_exists, compares
btd_adapter.
'NULL' key maintains existing behavior.
---
 profiles/audio/media.c |    5 +++--
 src/profile.c          |   22 ++++++++++++++++------
 src/profile.h          |    6 ++++--
 3 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/profiles/audio/media.c b/profiles/audio/media.c
index 4b91656..60a5da5 100644
--- a/profiles/audio/media.c
+++ b/profiles/audio/media.c
@@ -195,7 +195,7 @@ static void media_endpoint_remove(struct media_endpoint *endpoint)
 
 	if (media_adapter_find_endpoint(adapter, NULL, NULL,
 						endpoint->uuid) == NULL)
-		btd_profile_remove_custom_prop(endpoint->uuid,
+		btd_profile_remove_custom_prop(adapter, endpoint->uuid,
 							"MediaEndpoints");
 
 	media_endpoint_destroy(endpoint);
@@ -740,7 +740,8 @@ static struct media_endpoint *media_endpoint_create(struct media_adapter *adapte
 						endpoint, NULL);
 
 	if (media_adapter_find_endpoint(adapter, NULL, NULL, uuid) == NULL) {
-		btd_profile_add_custom_prop(uuid, "a{sv}", "MediaEndpoints",
+		btd_profile_add_custom_prop(adapter, uuid, "a{sv}",
+						"MediaEndpoints",
 						endpoint_properties_exists,
 						endpoint_properties_get,
 						adapter);
diff --git a/src/profile.c b/src/profile.c
index b0c8500..2d0d2ba 100644
--- a/src/profile.c
+++ b/src/profile.c
@@ -572,6 +572,7 @@ struct ext_record {
 };
 
 struct btd_profile_custom_property {
+	const void *key;
 	char *uuid;
 	char *type;
 	char *name;
@@ -2208,7 +2209,8 @@ static const GDBusMethodTable methods[] = {
 	{ }
 };
 
-static struct btd_profile_custom_property *find_custom_prop(const char *uuid,
+static struct btd_profile_custom_property *find_custom_prop(const char *key,
+				const char *uuid,
 							const char *name)
 {
 	GSList *l;
@@ -2216,6 +2218,9 @@ static struct btd_profile_custom_property *find_custom_prop(const char *uuid,
 	for (l = custom_props; l; l = l->next) {
 		struct btd_profile_custom_property *prop = l->data;
 
+		if (prop->key != key)
+			continue;
+
 		if (strcasecmp(prop->uuid, uuid) != 0)
 			continue;
 
@@ -2226,7 +2231,8 @@ static struct btd_profile_custom_property *find_custom_prop(const char *uuid,
 	return NULL;
 }
 
-bool btd_profile_add_custom_prop(const char *uuid, const char *type,
+bool btd_profile_add_custom_prop(const void *key, const char *uuid,
+					const char *type,
 					const char *name,
 					btd_profile_prop_exists exists,
 					btd_profile_prop_get get,
@@ -2234,12 +2240,15 @@ bool btd_profile_add_custom_prop(const char *uuid, const char *type,
 {
 	struct btd_profile_custom_property *prop;
 
-	prop = find_custom_prop(uuid, name);
-	if (prop != NULL)
+	prop = find_custom_prop(key, uuid, name);
+	if (prop != NULL) {
+		DBG("Custom property %s already exists", name);
 		return false;
+	}
 
 	prop = g_new0(struct btd_profile_custom_property, 1);
 
+	prop->key = key;
 	prop->uuid = g_strdup(uuid);
 	prop->type = g_strdup(type);
 	prop->name = g_strdup(name);
@@ -2263,11 +2272,12 @@ static void free_property(gpointer data)
 	g_free(p);
 }
 
-bool btd_profile_remove_custom_prop(const char *uuid, const char *name)
+bool btd_profile_remove_custom_prop(const void *key, const char *uuid,
+							const char *name)
 {
 	struct btd_profile_custom_property *prop;
 
-	prop = find_custom_prop(uuid, name);
+	prop = find_custom_prop(key, uuid, name);
 	if (prop == NULL)
 		return false;
 
diff --git a/src/profile.h b/src/profile.h
index d858925..0764f6f 100644
--- a/src/profile.h
+++ b/src/profile.h
@@ -67,12 +67,14 @@ typedef bool (*btd_profile_prop_get)(const char *uuid,
 						DBusMessageIter *iter,
 						void *user_data);
 
-bool btd_profile_add_custom_prop(const char *uuid, const char *type,
+bool btd_profile_add_custom_prop(const void *key, const char *uuid,
+					const char *type,
 					const char *name,
 					btd_profile_prop_exists exists,
 					btd_profile_prop_get get,
 					void *user_data);
-bool btd_profile_remove_custom_prop(const char *uuid, const char *name);
+bool btd_profile_remove_custom_prop(const void *key, const char *uuid,
+					const char *name);
 
 void btd_profile_init(void);
 void btd_profile_cleanup(void);
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH v2] core: Fix allow registering same custom property
From: Frédéric Dalleau @ 2013-01-16 17:07 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Frédéric Dalleau

With several adapters, MediaEndpoint custom property is only created for the
first adapter. When a new connection happens on another adapter
btd_profile_custom_property_exists fails because the custom property doesn't
exist for this last adapter. In case of HFP ofono fails with error: "Media
Endpoint missing error".

This patch allows the creation of several custom property with same name that
can be distinguished based on a 'key'. When querying the property, additional
checking must happen. For example endpoint_properties_exists, compares
btd_adapter.
'NULL' key maintains existing behavior.
---
 profiles/audio/media.c |    5 +++--
 src/profile.c          |   22 ++++++++++++++++------
 src/profile.h          |    6 ++++--
 3 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/profiles/audio/media.c b/profiles/audio/media.c
index 4b91656..60a5da5 100644
--- a/profiles/audio/media.c
+++ b/profiles/audio/media.c
@@ -195,7 +195,7 @@ static void media_endpoint_remove(struct media_endpoint *endpoint)
 
 	if (media_adapter_find_endpoint(adapter, NULL, NULL,
 						endpoint->uuid) == NULL)
-		btd_profile_remove_custom_prop(endpoint->uuid,
+		btd_profile_remove_custom_prop(adapter, endpoint->uuid,
 							"MediaEndpoints");
 
 	media_endpoint_destroy(endpoint);
@@ -740,7 +740,8 @@ static struct media_endpoint *media_endpoint_create(struct media_adapter *adapte
 						endpoint, NULL);
 
 	if (media_adapter_find_endpoint(adapter, NULL, NULL, uuid) == NULL) {
-		btd_profile_add_custom_prop(uuid, "a{sv}", "MediaEndpoints",
+		btd_profile_add_custom_prop(adapter, uuid, "a{sv}",
+						"MediaEndpoints",
 						endpoint_properties_exists,
 						endpoint_properties_get,
 						adapter);
diff --git a/src/profile.c b/src/profile.c
index b0c8500..7d48cfc 100644
--- a/src/profile.c
+++ b/src/profile.c
@@ -572,6 +572,7 @@ struct ext_record {
 };
 
 struct btd_profile_custom_property {
+	const void *key;
 	char *uuid;
 	char *type;
 	char *name;
@@ -2208,7 +2209,8 @@ static const GDBusMethodTable methods[] = {
 	{ }
 };
 
-static struct btd_profile_custom_property *find_custom_prop(const char *uuid,
+static struct btd_profile_custom_property *find_custom_prop(const char *key,
+							const char *uuid,
 							const char *name)
 {
 	GSList *l;
@@ -2216,6 +2218,9 @@ static struct btd_profile_custom_property *find_custom_prop(const char *uuid,
 	for (l = custom_props; l; l = l->next) {
 		struct btd_profile_custom_property *prop = l->data;
 
+		if (prop->key != key)
+			continue;
+
 		if (strcasecmp(prop->uuid, uuid) != 0)
 			continue;
 
@@ -2226,7 +2231,8 @@ static struct btd_profile_custom_property *find_custom_prop(const char *uuid,
 	return NULL;
 }
 
-bool btd_profile_add_custom_prop(const char *uuid, const char *type,
+bool btd_profile_add_custom_prop(const void *key, const char *uuid,
+					const char *type,
 					const char *name,
 					btd_profile_prop_exists exists,
 					btd_profile_prop_get get,
@@ -2234,12 +2240,15 @@ bool btd_profile_add_custom_prop(const char *uuid, const char *type,
 {
 	struct btd_profile_custom_property *prop;
 
-	prop = find_custom_prop(uuid, name);
-	if (prop != NULL)
+	prop = find_custom_prop(key, uuid, name);
+	if (prop != NULL) {
+		DBG("Custom property %s already exists", name);
 		return false;
+	}
 
 	prop = g_new0(struct btd_profile_custom_property, 1);
 
+	prop->key = key;
 	prop->uuid = g_strdup(uuid);
 	prop->type = g_strdup(type);
 	prop->name = g_strdup(name);
@@ -2263,11 +2272,12 @@ static void free_property(gpointer data)
 	g_free(p);
 }
 
-bool btd_profile_remove_custom_prop(const char *uuid, const char *name)
+bool btd_profile_remove_custom_prop(const void *key, const char *uuid,
+							const char *name)
 {
 	struct btd_profile_custom_property *prop;
 
-	prop = find_custom_prop(uuid, name);
+	prop = find_custom_prop(key, uuid, name);
 	if (prop == NULL)
 		return false;
 
diff --git a/src/profile.h b/src/profile.h
index d858925..0764f6f 100644
--- a/src/profile.h
+++ b/src/profile.h
@@ -67,12 +67,14 @@ typedef bool (*btd_profile_prop_get)(const char *uuid,
 						DBusMessageIter *iter,
 						void *user_data);
 
-bool btd_profile_add_custom_prop(const char *uuid, const char *type,
+bool btd_profile_add_custom_prop(const void *key, const char *uuid,
+					const char *type,
 					const char *name,
 					btd_profile_prop_exists exists,
 					btd_profile_prop_get get,
 					void *user_data);
-bool btd_profile_remove_custom_prop(const char *uuid, const char *name);
+bool btd_profile_remove_custom_prop(const void *key, const char *uuid,
+					const char *name);
 
 void btd_profile_init(void);
 void btd_profile_cleanup(void);
-- 
1.7.9.5


^ permalink raw reply related

* HCI Create Connection Issue
From: Deepthi @ 2013-01-17  4:11 UTC (permalink / raw)
  To: linux-bluetooth

Hi,


I 'm not able to create a ACL connection, when an active sco connection 
is running. I'm getting error at the HCI level saying "Connection 
Rejected due to Limited Resources", L2cap giving error saying Connection 
Refused. Why Connection Rejected due to Limited Resources happens ? How 
to overcome this issue?


Please Help,

Thanks & Regards,
Deepthi Elizabeth P V


^ permalink raw reply

* Re: [PATCH] Bluetooth: Fix Class of Device indication when powering off
From: Marcel Holtmann @ 2013-01-17  5:21 UTC (permalink / raw)
  To: Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <1358345734-17892-1-git-send-email-johan.hedberg@gmail.com>

Hi Johan,

> When a HCI device is powered off the Management interface specification
> dictates that the class of device value is indicated as zero. This patch
> fixes sending of the appropriate class of device changed event when a
> HCI device is powered off.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
>  net/bluetooth/mgmt.c |    6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index fc171f2..54f3ddba 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -2987,7 +2987,13 @@ int mgmt_powered(struct hci_dev *hdev, u8 powered)
>  		}
>  	} else {
>  		u8 status = MGMT_STATUS_NOT_POWERED;
> +		u8 zero_cod[] = { 0, 0, 0 };
> +
>  		mgmt_pending_foreach(0, hdev, cmd_status_rsp, &status);
> +
> +		if (memcmp(hdev->dev_class, zero_cod, sizeof(zero_cod)) != 0)

shouldn't we better use if (memcmp( ...)) here.

> +			mgmt_event(MGMT_EV_CLASS_OF_DEV_CHANGED, hdev,
> +				   zero_cod, sizeof(zero_cod), NULL);
>  	}

Otherwise this is fine with me.

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

Regards

Marcel



^ permalink raw reply

* Re: bluez: atomic operations
From: Marcel Holtmann @ 2013-01-17  5:25 UTC (permalink / raw)
  To: Kling, Andreas; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <sig.6728472516.B3F251FD87EC894A88C2221EEEBDF54807096F7F3B@Exchange>

Hi Andy,

> using gcc atomic buildins breaks support for older gcc/platforms.
> 
> src/bluetoothd-adapter.o: In function `btd_adapter_unref':
> adapter.c:(.text+0x63f8): undefined reference to `__sync_sub_and_fetch_4'
> 
> I suggest to use glib g_atomic_* group of functions.
> They fall back to traditional locking if buildins are not available.

the GLib atomic API functions are not really stable. The GLib people for
some reason decided to break their own API/ABI guarantee. So we moved
away from using them. And I have no intention to get back to them.

Long term, we are moving away from GLib and want to switch to ELL. So
that means gcc atomics are required. My advise would be to get a modern
platform and not rely on workarounds.

Regards

Marcel



^ permalink raw reply

* Re: 3.8.0-rc3: possible circular locking dependency: &tty->legacy_mutex / &tty->hangup_work with serial/RFCOMM connection via USB bluetooth dongle
From: Sander Eikelenboom @ 2013-01-17  9:08 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-serial, linux-bluetooth, Alan Cox, Greg Kroah-Hartman,
	marcel
In-Reply-To: <34560309.20130112194639@eikelenboom.it>


Saturday, January 12, 2013, 7:46:39 PM, you wrote:

> Hi,

> Running a 3.8.0-rc3 kernel (latest commit b719f43059903820c31edb30f4663a2818836e7f) kernel (debian squeeze os), i'm running into this lockdep warning when:

Ok, this seems to be fixed by commit: 852e4a8152b427c3f318bb0e1b5e938d64dcdc32 (tty: don't deadlock while flushing workqueue)


> - Running a perl script that uses rfcomm to communicatie via bluetooth with a bluetooth/TTL converter.
> - It can run ok for a few hours before this lockdep occurs and the perl script freezes.
> - The info related to bluetooth from syslog:

> Jan 12 10:24:08 serveerstertje kernel: [    7.919775] Bluetooth: Virtual HCI driver ver 1.3
> Jan 12 10:24:08 serveerstertje kernel: [    7.920314] Bluetooth: HCI UART driver ver 2.2
> Jan 12 10:24:08 serveerstertje kernel: [    7.920316] Bluetooth: HCI H4 protocol initialized
> Jan 12 10:24:08 serveerstertje kernel: [    7.920317] Bluetooth: HCI BCSP protocol initialized
> Jan 12 10:24:08 serveerstertje kernel: [    7.920318] Bluetooth: HCILL protocol initialized
> Jan 12 10:24:08 serveerstertje kernel: [    7.920318] Bluetooth: HCIATH3K protocol initialized
> Jan 12 10:24:08 serveerstertje kernel: [    7.920319] Bluetooth: HCI Three-wire UART (H5) protocol initialized

> Jan 12 10:24:08 serveerstertje kernel: [    8.191897] Bluetooth: RFCOMM TTY layer initialized
> Jan 12 10:24:08 serveerstertje kernel: [    8.191930] Bluetooth: RFCOMM socket layer initialized
> Jan 12 10:24:08 serveerstertje kernel: [    8.191931] Bluetooth: RFCOMM ver 1.11
> Jan 12 10:24:08 serveerstertje kernel: [    8.191932] Bluetooth: BNEP (Ethernet Emulation) ver 1.3
> Jan 12 10:24:08 serveerstertje kernel: [    8.191933] Bluetooth: BNEP filters: protocol multicast
> Jan 12 10:24:08 serveerstertje kernel: [    8.191944] Bluetooth: BNEP socket layer initialized
> Jan 12 10:24:08 serveerstertje kernel: [    8.191945] Bluetooth: HIDP (Human Interface Emulation) ver 1.2
> Jan 12 10:24:08 serveerstertje kernel: [    8.191954] Bluetooth: HIDP socket layer initialized

> Jan 12 10:24:09 serveerstertje bluetoothd[3912]: Bluetooth deamon 4.66
> Jan 12 10:24:09 serveerstertje bluetoothd[3912]: Starting SDP server
> Jan 12 10:24:09 serveerstertje bluetoothd[3912]: Starting experimental netlink support
> Jan 12 10:24:09 serveerstertje bluetoothd[3912]: Failed to find Bluetooth netlink family
> Jan 12 10:24:09 serveerstertje bluetoothd[3912]: Failed to init netlink plugin
> Jan 12 10:24:09 serveerstertje bluetoothd[3912]: bridge pan0 created
> Jan 12 10:24:09 serveerstertje bluetoothd[3912]: HCI dev 0 registered
> Jan 12 10:24:09 serveerstertje bluetoothd[3912]: Failed to open RFKILL control device
> Jan 12 10:24:09 serveerstertje bluetoothd[3912]: HCI dev 0 up
> Jan 12 10:24:09 serveerstertje bluetoothd[3912]: Starting security manager 0
> Jan 12 10:24:09 serveerstertje bluetoothd[3912]: Adapter /org/bluez/3912/hci0 has been enabled
> Jan 12 10:24:09 serveerstertje bluetoothd[3912]: Failed to access HAL


> - And the lockdep warning itself:

> [28678.458250]
> [28678.476588] ======================================================
> [28678.494887] [ INFO: possible circular locking dependency detected ]
> [28678.513013] 3.8.0-rc3-20130112-netpatched-rocketscience-radeon #1 Not tainted
> [28678.530909] -------------------------------------------------------
> [28678.548636] kworker/2:1/19513 is trying to acquire lock:
> [28678.566070]  (&tty->legacy_mutex){+.+.+.}, at: [<ffffffff819ba5ee>] tty_lock_nested+0x3e/0x80
> [28678.583577]
> [28678.583577] but task is already holding lock:
> [28678.617615]  ((&tty->hangup_work)){+.+...}, at: [<ffffffff81080bf8>] process_one_work+0x158/0x4b0
> [28678.634569]
> [28678.634569] which lock already depends on the new lock.
> [28678.634569]
> [28678.683868]
> [28678.683868] the existing dependency chain (in reverse order) is:
> [28678.715354]
[28678.715354] ->> #2 ((&tty->hangup_work)){+.+...}:
> [28678.745890]        [<ffffffff810b4d2e>] __lock_acquire+0x44e/0xdd0
> [28678.760975]        [<ffffffff810b576a>] lock_acquire+0xba/0x100
> [28678.775834]        [<ffffffff8108322a>] flush_work+0x3a/0x250
> [28678.790408]        [<ffffffff81451568>] tty_ldisc_flush_works+0x18/0x40
> [28678.804877]        [<ffffffff814517ae>] tty_ldisc_release+0x2e/0x90
> [28678.818952]        [<ffffffff8144b827>] tty_release+0x3c7/0x590
> [28678.832813]        [<ffffffff8114e009>] __fput+0xa9/0x2c0
> [28678.846411]        [<ffffffff8114e289>] ____fput+0x9/0x10
> [28678.859644]        [<ffffffff810854d5>] task_work_run+0x95/0xb0
> [28678.872661]        [<ffffffff8100dc4d>] do_notify_resume+0x6d/0x80
> [28678.885516]        [<ffffffff819bb5a2>] int_signal+0x12/0x17
> [28678.898047]
[28678.898047] ->> #1 (&tty->legacy_mutex/1){+.+...}:
> [28678.922334]        [<ffffffff810b4d2e>] __lock_acquire+0x44e/0xdd0
> [28678.934268]        [<ffffffff810b576a>] lock_acquire+0xba/0x100
> [28678.945916]        [<ffffffff819b754c>] mutex_lock_nested+0x4c/0x450
> [28678.957318]        [<ffffffff819ba5ee>] tty_lock_nested+0x3e/0x80
> [28678.968500]        [<ffffffff819ba6aa>] tty_lock_pair+0x6a/0x70
> [28678.979405]        [<ffffffff8144b5cb>] tty_release+0x16b/0x590
> [28678.990012]        [<ffffffff8114e009>] __fput+0xa9/0x2c0
> [28679.000367]        [<ffffffff8114e289>] ____fput+0x9/0x10
> [28679.009455] FW: BLOCKED low udp input: IN=eth0 OUT= MAC=40:61:86:f4:67:d9:00:08:ae:10:46:60:08:00 SRC=112.203.174.221 DST=88.159.69.252 LEN=131 TOS=0x00 PREC=0x00 TTL=38 ID=17898 PROTO=UDP SPT=27001 DPT=1024 LEN=111
> [28679.030869]        [<ffffffff810854d5>] task_work_run+0x95/0xb0
> [28679.040727]        [<ffffffff8100dc4d>] do_notify_resume+0x6d/0x80
> [28679.050419]        [<ffffffff819bb5a2>] int_signal+0x12/0x17
> [28679.059880]
[28679.059880] ->> #0 (&tty->legacy_mutex){+.+.+.}:
> [28679.077823]        [<ffffffff810b41d8>] validate_chain+0x1258/0x1300
> [28679.086583]        [<ffffffff810b4d2e>] __lock_acquire+0x44e/0xdd0
> [28679.095126]        [<ffffffff810b576a>] lock_acquire+0xba/0x100
> [28679.103399]        [<ffffffff819b754c>] mutex_lock_nested+0x4c/0x450
> [28679.111468]        [<ffffffff819ba5ee>] tty_lock_nested+0x3e/0x80
> [28679.119247]        [<ffffffff819ba63b>] tty_lock+0xb/0x10
> [28679.126712]        [<ffffffff814492b5>] __tty_hangup+0x65/0x3c0
> [28679.133940]        [<ffffffff81449620>] do_tty_hangup+0x10/0x20
> [28679.140970]        [<ffffffff81080c60>] process_one_work+0x1c0/0x4b0
> [28679.147755]        [<ffffffff8108134e>] worker_thread+0x11e/0x3d0
> [28679.154383]        [<ffffffff81088a36>] kthread+0xd6/0xe0
> [28679.160649]        [<ffffffff819bb1bc>] ret_from_fork+0x7c/0xb0
> [28679.166666]
> [28679.166666] other info that might help us debug this:
> [28679.166666]
> [28679.183748] Chain exists of:
> [28679.183748]   &tty->legacy_mutex --> &tty->legacy_mutex/1 --> (&tty->hangup_work)
> [28679.183748]
> [28679.200495]  Possible unsafe locking scenario:
> [28679.200495]
> [28679.211416]        CPU0                    CPU1
> [28679.216751]        ----                    ----
> [28679.222049]   lock((&tty->hangup_work));
> [28679.227206]                                lock(&tty->legacy_mutex/1);
> [28679.232380]                                lock((&tty->hangup_work));
> [28679.237532]   lock(&tty->legacy_mutex);
> [28679.242673]
> [28679.242673]  *** DEADLOCK ***
> [28679.242673]
> [28679.257840] 2 locks held by kworker/2:1/19513:
> [28679.262888]  #0:  (events){.+.+.+}, at: [<ffffffff81080bf8>] process_one_work+0x158/0x4b0
> [28679.268053]  #1:  ((&tty->hangup_work)){+.+...}, at: [<ffffffff81080bf8>] process_one_work+0x158/0x4b0
> [28679.273381]
> [28679.273381] stack backtrace:
> [28679.283820] Pid: 19513, comm: kworker/2:1 Not tainted 3.8.0-rc3-20130112-netpatched-rocketscience-radeon #1
> [28679.289347] Call Trace:
> [28679.294804]  [<ffffffff810b2c74>] print_circular_bug+0x204/0x300
> [28679.300384]  [<ffffffff810b41d8>] validate_chain+0x1258/0x1300
> [28679.305997]  [<ffffffff810b4d2e>] __lock_acquire+0x44e/0xdd0
> [28679.311599]  [<ffffffff810b4d4b>] ? __lock_acquire+0x46b/0xdd0
> [28679.317222]  [<ffffffff810b576a>] lock_acquire+0xba/0x100
> [28679.322889]  [<ffffffff819ba5ee>] ? tty_lock_nested+0x3e/0x80
> [28679.328481]  [<ffffffff819ba5ee>] ? tty_lock_nested+0x3e/0x80
> [28679.334023]  [<ffffffff819b754c>] mutex_lock_nested+0x4c/0x450
> [28679.339415]  [<ffffffff819ba5ee>] ? tty_lock_nested+0x3e/0x80
> [28679.344784]  [<ffffffff810b5788>] ? lock_acquire+0xd8/0x100
> [28679.350154]  [<ffffffff81449279>] ? __tty_hangup+0x29/0x3c0
> [28679.355506]  [<ffffffff819ba5ee>] tty_lock_nested+0x3e/0x80
> [28679.360939]  [<ffffffff819ba63b>] tty_lock+0xb/0x10
> [28679.366282]  [<ffffffff814492b5>] __tty_hangup+0x65/0x3c0
> [28679.371651]  [<ffffffff81080bf8>] ? process_one_work+0x158/0x4b0
> [28679.377032]  [<ffffffff810b1918>] ? trace_hardirqs_on_caller+0xf8/0x200
> [28679.382273]  [<ffffffff81449620>] do_tty_hangup+0x10/0x20
> [28679.387216]  [<ffffffff81080c60>] process_one_work+0x1c0/0x4b0
> [28679.392118]  [<ffffffff81080bf8>] ? process_one_work+0x158/0x4b0
> [28679.397039]  [<ffffffff81449610>] ? __tty_hangup+0x3c0/0x3c0
> [28679.401952]  [<ffffffff8108134e>] worker_thread+0x11e/0x3d0
> [28679.406859]  [<ffffffff810b1918>] ? trace_hardirqs_on_caller+0xf8/0x200
> [28679.411787]  [<ffffffff81081230>] ? manage_workers+0x2e0/0x2e0
> [28679.416683]  [<ffffffff81088a36>] kthread+0xd6/0xe0
> [28679.421538]  [<ffffffff81088960>] ? __init_kthread_worker+0x70/0x70
> [28679.426429]  [<ffffffff819bb1bc>] ret_from_fork+0x7c/0xb0
> [28679.431278]  [<ffffffff81088960>] ? __init_kthread_worker+0x70/0x70


> - This is followed by blocked task messages for the perl script:

> Jan 12 18:25:29 serveerstertje kernel: [28926.144229] INFO: task zabbix_slimmeme:26976 blocked for more than 120 seconds.
> Jan 12 18:25:29 serveerstertje kernel: [28926.162883] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> Jan 12 18:25:29 serveerstertje kernel: [28926.181312] zabbix_slimmeme D ffff88003851d230     0 26976  22112 0x00000000
> Jan 12 18:25:29 serveerstertje kernel: [28926.199596]  ffff88002d473818 0000000000000216 ffff880000000002 ffffffff8202be38
> Jan 12 18:25:29 serveerstertje kernel: [28926.217728]  ffff88003851d230 0000000000013040 ffff88002d473fd8 ffff88002d472010
> Jan 12 18:25:29 serveerstertje kernel: [28926.235627]  0000000000013040 0000000000013040 ffff88002d473fd8 0000000000013040
> Jan 12 18:25:29 serveerstertje kernel: [28926.253346] Call Trace:
> Jan 12 18:25:29 serveerstertje kernel: [28926.270648]  [<ffffffff810be1ed>] ? __module_text_address+0xd/0x60
> Jan 12 18:25:29 serveerstertje kernel: [28926.322462]  [<ffffffff810be1ed>] ? __module_text_address+0xd/0x60
> Jan 12 18:25:29 serveerstertje kernel: [28926.339617]  [<ffffffff810be44b>] ? is_module_text_address+0x2b/0x60
> Jan 12 18:25:29 serveerstertje kernel: [28926.356452]  [<ffffffff81085958>] ? __kernel_text_address+0x58/0x80
> Jan 12 18:25:29 serveerstertje kernel: [28926.373057]  [<ffffffff81140049>] ? sysfs_slab_add+0x149/0x200
> Jan 12 18:25:29 serveerstertje kernel: [28926.389435]  [<ffffffff81140067>] ? sysfs_slab_add+0x167/0x200
> Jan 12 18:25:29 serveerstertje kernel: [28926.405516]  [<ffffffff819b8d04>] schedule+0x24/0x70
> Jan 12 18:25:29 serveerstertje kernel: [28926.421242]  [<ffffffff819b5f6d>] schedule_timeout+0x1bd/0x220
> Jan 12 18:25:29 serveerstertje kernel: [28926.436793]  [<ffffffff810b5788>] ? lock_acquire+0xd8/0x100
> Jan 12 18:25:29 serveerstertje kernel: [28926.452138]  [<ffffffff819b8201>] ? wait_for_common+0x31/0x170
> Jan 12 18:25:29 serveerstertje kernel: [28926.467171]  [<ffffffff810b5c17>] ? lock_release+0x117/0x250
> Jan 12 18:25:29 serveerstertje kernel: [28926.481938]  [<ffffffff819b82d1>] wait_for_common+0x101/0x170
> Jan 12 18:25:29 serveerstertje kernel: [28926.496482]  [<ffffffff81098730>] ? try_to_wake_up+0x310/0x310
> Jan 12 18:25:29 serveerstertje kernel: [28926.510873]  [<ffffffff819b83e8>] wait_for_completion+0x18/0x20
> Jan 12 18:25:29 serveerstertje kernel: [28926.525014]  [<ffffffff81083385>] flush_work+0x195/0x250
> Jan 12 18:25:29 serveerstertje kernel: [28926.538855]  [<ffffffff810833a0>] ? flush_work+0x1b0/0x250
> Jan 12 18:25:29 serveerstertje kernel: [28926.552411]  [<ffffffff81080400>] ? cwq_dec_nr_in_flight+0xd0/0xd0
> Jan 12 18:25:29 serveerstertje kernel: [28926.565910]  [<ffffffff81451568>] tty_ldisc_flush_works+0x18/0x40
> Jan 12 18:25:29 serveerstertje kernel: [28926.579013]  [<ffffffff814517ae>] tty_ldisc_release+0x2e/0x90
> Jan 12 18:25:29 serveerstertje kernel: [28926.591876]  [<ffffffff8144b827>] tty_release+0x3c7/0x590
> Jan 12 18:25:29 serveerstertje kernel: [28926.604527]  [<ffffffff810b1a2d>] ? trace_hardirqs_on+0xd/0x10
> Jan 12 18:25:29 serveerstertje kernel: [28926.616853]  [<ffffffff819b63a9>] ? __mutex_unlock_slowpath+0x149/0x1d0
> Jan 12 18:25:29 serveerstertje kernel: [28926.628997]  [<ffffffff81098730>] ? try_to_wake_up+0x310/0x310
> Jan 12 18:25:29 serveerstertje kernel: [28926.640952]  [<ffffffff8144bdb4>] tty_open+0x3c4/0x5f0
> Jan 12 18:25:30 serveerstertje kernel: [28926.652584]  [<ffffffff81150a18>] chrdev_open+0x98/0x170
> Jan 12 18:25:30 serveerstertje kernel: [28926.663972]  [<ffffffff810912cd>] ? lg_local_unlock+0x3d/0x70
> Jan 12 18:25:30 serveerstertje kernel: [28926.675152]  [<ffffffff81150980>] ? cdev_put+0x30/0x30
> Jan 12 18:25:30 serveerstertje kernel: [28926.686173]  [<ffffffff8114b1fe>] do_dentry_open+0x25e/0x310
> Jan 12 18:25:30 serveerstertje kernel: [28926.696868]  [<ffffffff8114b3c0>] finish_open+0x30/0x50
> Jan 12 18:25:30 serveerstertje kernel: [28926.707267]  [<ffffffff8115a79e>] do_last+0x30e/0xe90
> Jan 12 18:25:30 serveerstertje kernel: [28926.717404]  [<ffffffff81157aba>] ? link_path_walk+0x9a/0x9f0
> Jan 12 18:25:30 serveerstertje kernel: [28926.727353]  [<ffffffff8115b3ce>] path_openat+0xae/0x4e0
> Jan 12 18:25:30 serveerstertje kernel: [28926.737008]  [<ffffffff810b5c17>] ? lock_release+0x117/0x250
> Jan 12 18:25:30 serveerstertje kernel: [28926.746482]  [<ffffffff81160264>] ? do_select+0x5f4/0x6d0
> Jan 12 18:25:30 serveerstertje kernel: [28926.755613]  [<ffffffff8115b934>] do_filp_open+0x44/0xa0
> Jan 12 18:25:30 serveerstertje kernel: [28926.764542]  [<ffffffff811691e3>] ? __alloc_fd+0xb3/0x150
> Jan 12 18:25:30 serveerstertje kernel: [28926.773224]  [<ffffffff8114ad13>] do_sys_open+0x103/0x1f0
> Jan 12 18:25:30 serveerstertje kernel: [28926.781585]  [<ffffffff8114ae3c>] sys_open+0x1c/0x20
> Jan 12 18:25:30 serveerstertje kernel: [28926.789685]  [<ffffffff819bb269>] system_call_fastpath+0x16/0x1b
> Jan 12 18:25:30 serveerstertje kernel: [28926.797570] INFO: lockdep is turned off.

^ permalink raw reply

* [PATCH 0/5] sco: SCO socket option for mode
From: Frédéric Dalleau @ 2013-01-17 12:39 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Frédéric Dalleau

Hi,

This is the patch version of the socket option for enabling transparent SCO
sockets.  RFC didn't generate feedback, so if we consider that people only
complains when something is wrong, this patch is fine ;)
In between, the patch series has been simplified, the most visible change is
that RFC was declaring HCI_CONN_SCO_T1_SETTINGS and removing it
later, plus some __cpu_to_le16 conversion were added.

The initial mode corresponding to current behavior is SCO_MODE_CVSD.
SCO_MODE_TRANSPARENT is the mode for setting up transparent sockets.
SCO_MODE_ENHANCED is targeted at CSA2. It is only declared and not
implemented.

Let me know what you think.
Best regards,

Frédéric


Frédéric Dalleau (5):
  Bluetooth: Add option for SCO socket mode
  Bluetooth: Add setsockopt for SCO socket mode
  Bluetooth: Use mode when accepting SCO connection
  Bluetooth: Parameters for outgoing SCO connections
  Bluetooth: Fallback transparent SCO from T2 to T1

 include/net/bluetooth/hci_core.h |    5 +++-
 include/net/bluetooth/sco.h      |   20 ++++++++++++++
 net/bluetooth/hci_conn.c         |   36 +++++++++++++++++++++-----
 net/bluetooth/hci_event.c        |   22 +++++++++++++---
 net/bluetooth/sco.c              |   53 +++++++++++++++++++++++++++++++++++---
 5 files changed, 121 insertions(+), 15 deletions(-)

-- 
1.7.9.5


^ permalink raw reply

* [PATCH 1/5] Bluetooth: Add option for SCO socket mode
From: Frédéric Dalleau @ 2013-01-17 12:39 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Frédéric Dalleau
In-Reply-To: <1358426389-25903-1-git-send-email-frederic.dalleau@linux.intel.com>

This patch extends the current SCO socket option to add a 'mode' field. This
field is intended to choose data type at runtime. Current modes are CVSD and
transparent SCO, but adding new modes could allow support for CSA2 and fine
tuning a sco connection, for example latency, bandwith, voice setting. Incoming
connections will be setup during defered setup. Outgoing connections have to
be setup before connect(). The selected type is stored in the sco socket info.
This patch declares needed members and implements getsockopt().
---
 include/net/bluetooth/sco.h |   20 ++++++++++++++++++++
 net/bluetooth/sco.c         |    3 +++
 2 files changed, 23 insertions(+)

diff --git a/include/net/bluetooth/sco.h b/include/net/bluetooth/sco.h
index 1e35c43..bc5d3d6 100644
--- a/include/net/bluetooth/sco.h
+++ b/include/net/bluetooth/sco.h
@@ -41,8 +41,27 @@ struct sockaddr_sco {
 
 /* SCO socket options */
 #define SCO_OPTIONS	0x01
+
+#define SCO_MODE_CVSD		0x00
+#define SCO_MODE_TRANSPARENT	0x01
+#define SCO_MODE_ENHANCED	0x02
+
 struct sco_options {
 	__u16 mtu;
+	__u8 mode;
+};
+
+struct sco_coding {
+	__u8 format;
+	__u16 vid;
+	__u16 cid;
+};
+
+struct sco_options_enhanced {
+	__u16 mtu;
+	__u8 mode;
+	struct sco_coding host;
+	struct sco_coding air;
 };
 
 #define SCO_CONNINFO	0x02
@@ -73,6 +92,7 @@ struct sco_conn {
 struct sco_pinfo {
 	struct bt_sock	bt;
 	__u32		flags;
+	__u8		mode;
 	struct sco_conn	*conn;
 };
 
diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index 531a93d..bdb21b2 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -418,6 +418,8 @@ static struct sock *sco_sock_alloc(struct net *net, struct socket *sock, int pro
 	sk->sk_protocol = proto;
 	sk->sk_state    = BT_OPEN;
 
+	sco_pi(sk)->mode = SCO_MODE_CVSD;
+
 	setup_timer(&sk->sk_timer, sco_sock_timeout, (unsigned long)sk);
 
 	bt_sock_link(&sco_sk_list, sk);
@@ -736,6 +738,7 @@ static int sco_sock_getsockopt_old(struct socket *sock, int optname, char __user
 		}
 
 		opts.mtu = sco_pi(sk)->conn->mtu;
+		opts.mode = sco_pi(sk)->mode;
 
 		BT_DBG("mtu %d", opts.mtu);
 
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 2/5] Bluetooth: Add setsockopt for SCO socket mode
From: Frédéric Dalleau @ 2013-01-17 12:39 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Frédéric Dalleau
In-Reply-To: <1358426389-25903-1-git-send-email-frederic.dalleau@linux.intel.com>

This patch implements setsockopt().
---
 net/bluetooth/sco.c |   44 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index bdb21b2..22ad5fa 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -678,6 +678,47 @@ static int sco_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
 	return bt_sock_recvmsg(iocb, sock, msg, len, flags);
 }
 
+static int sco_sock_setsockopt_old(struct socket *sock, int optname,
+				   char __user *optval, unsigned int optlen)
+{
+	struct sock *sk = sock->sk;
+	struct sco_options opts;
+	int len, err = 0;
+
+	BT_DBG("sk %p", sk);
+
+	lock_sock(sk);
+
+	switch (optname) {
+	case SCO_OPTIONS:
+		if (sk->sk_state != BT_OPEN &&
+		    sk->sk_state != BT_BOUND &&
+		    sk->sk_state != BT_CONNECT2) {
+			err = -EINVAL;
+			break;
+		}
+
+		opts.mode = SCO_MODE_CVSD;
+
+		len = min_t(unsigned int, sizeof(opts), optlen);
+		if (copy_from_user((char *) &opts, optval, len)) {
+			err = -EFAULT;
+			break;
+		}
+
+		sco_pi(sk)->mode = opts.mode;
+		BT_DBG("mode %d", opts.mode);
+		break;
+
+	default:
+		err = -ENOPROTOOPT;
+		break;
+	}
+
+	release_sock(sk);
+	return err;
+}
+
 static int sco_sock_setsockopt(struct socket *sock, int level, int optname, char __user *optval, unsigned int optlen)
 {
 	struct sock *sk = sock->sk;
@@ -686,6 +727,9 @@ static int sco_sock_setsockopt(struct socket *sock, int level, int optname, char
 
 	BT_DBG("sk %p", sk);
 
+	if (level == SOL_SCO)
+		return sco_sock_setsockopt_old(sock, optname, optval, optlen);
+
 	lock_sock(sk);
 
 	switch (optname) {
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 3/5] Bluetooth: Use mode when accepting SCO connection
From: Frédéric Dalleau @ 2013-01-17 12:39 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Frédéric Dalleau
In-Reply-To: <1358426389-25903-1-git-send-email-frederic.dalleau@linux.intel.com>

When an incoming SCO connection is requested, check the selected mode, and
reply appropriately. Mode should have been negotiated previously. For example,
in case of HFP, the codec is negotiated using AT commands on the RFCOMM
channel. This patch only changes replies for socket with defered setup enabled.
---
 include/net/bluetooth/hci_core.h |    2 +-
 net/bluetooth/hci_event.c        |   21 +++++++++++++++++----
 net/bluetooth/sco.c              |    2 +-
 3 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 014a2ea..cb5d131 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -577,7 +577,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);
+void hci_conn_accept(struct hci_conn *conn, int mask, int mode);
 
 struct hci_chan *hci_chan_create(struct hci_conn *conn);
 void hci_chan_del(struct hci_chan *chan);
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 705078a..afa0104 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -2047,7 +2047,7 @@ unlock:
 	hci_conn_check_pending(hdev);
 }
 
-void hci_conn_accept(struct hci_conn *conn, int mask)
+void hci_conn_accept(struct hci_conn *conn, int mask, int mode)
 {
 	struct hci_dev *hdev = conn->hdev;
 
@@ -2074,9 +2074,22 @@ void hci_conn_accept(struct hci_conn *conn, int mask)
 
 		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;
+
+		switch (mode) {
+		case 0:
+			cp.max_latency    = __constant_cpu_to_le16(0xffff);
+			cp.content_format = cpu_to_le16(hdev->voice_setting);
+			cp.retrans_effort = 0xff;
+			break;
+		case 1:
+			if (conn->pkt_type & ESCO_2EV3)
+				cp.max_latency = __constant_cpu_to_le16(0x0008);
+			else
+				cp.max_latency = __constant_cpu_to_le16(0x000D);
+			cp.content_format = __constant_cpu_to_le16(0x0003);
+			cp.retrans_effort = 0x02;
+			break;
+		}
 
 		hci_send_cmd(hdev, HCI_OP_ACCEPT_SYNC_CONN_REQ,
 			     sizeof(cp), &cp);
diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index 22ad5fa..6a957a3 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -666,7 +666,7 @@ static int sco_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
 
 	if (sk->sk_state == BT_CONNECT2 &&
 	    test_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags)) {
-		hci_conn_accept(pi->conn->hcon, 0);
+		hci_conn_accept(pi->conn->hcon, 0, pi->mode);
 		sk->sk_state = BT_CONFIG;
 
 		release_sock(sk);
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 4/5] Bluetooth: Parameters for outgoing SCO connections
From: Frédéric Dalleau @ 2013-01-17 12:39 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Frédéric Dalleau
In-Reply-To: <1358426389-25903-1-git-send-email-frederic.dalleau@linux.intel.com>

In order to establish a transparent SCO connection, the correct settings must
be specified in the SetupSynchronousConnection request. For that, a bit is set
in ACL connection flags to set up the desired parameters. If this bit is not
set, a legacy SCO connection will be requested.
This patch uses T2 settings.
---
 include/net/bluetooth/hci_core.h |    3 +++
 net/bluetooth/hci_conn.c         |   27 ++++++++++++++++++++-------
 net/bluetooth/sco.c              |    4 ++--
 3 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index cb5d131..9ef7fe0 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -438,6 +438,7 @@ enum {
 	HCI_CONN_SSP_ENABLED,
 	HCI_CONN_POWER_SAVE,
 	HCI_CONN_REMOTE_OOB,
+	HCI_CONN_SCO_TRANSPARENT,
 };
 
 static inline bool hci_conn_ssp_enabled(struct hci_conn *conn)
@@ -586,6 +587,8 @@ struct hci_chan *hci_chan_lookup_handle(struct hci_dev *hdev, __u16 handle);
 
 struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst,
 			     __u8 dst_type, __u8 sec_level, __u8 auth_type);
+struct hci_conn *hci_connect_sco(struct hci_dev *hdev, int type, bdaddr_t *dst,
+				 u8 sec_level, u8 auth_type, u8 codec);
 int hci_conn_check_link_mode(struct hci_conn *conn);
 int hci_conn_check_secure(struct hci_conn *conn, __u8 sec_level);
 int hci_conn_security(struct hci_conn *conn, __u8 sec_level, __u8 auth_type);
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 25bfce0..e3f1fad 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -175,13 +175,22 @@ void hci_setup_sync(struct hci_conn *conn, __u16 handle)
 	conn->attempt++;
 
 	cp.handle   = cpu_to_le16(handle);
-	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.voice_setting  = cpu_to_le16(hdev->voice_setting);
-	cp.retrans_effort = 0xff;
+
+	if (test_and_clear_bit(HCI_CONN_SCO_TRANSPARENT, &conn->flags)) {
+		cp.pkt_type       = __constant_cpu_to_le16(EDR_ESCO_MASK &
+							   ~ESCO_2EV3);
+		cp.max_latency    = __constant_cpu_to_le16(0x000d);
+		cp.voice_setting  = __constant_cpu_to_le16(0x0003);
+		cp.retrans_effort = 0x02;
+	} else {
+		cp.pkt_type       = cpu_to_le16(conn->pkt_type);
+		cp.max_latency    = __constant_cpu_to_le16(0xffff);
+		cp.voice_setting  = cpu_to_le16(hdev->voice_setting);
+		cp.retrans_effort = 0xff;
+	}
 
 	hci_send_cmd(hdev, HCI_OP_SETUP_SYNC_CONN, sizeof(cp), &cp);
 }
@@ -551,8 +560,9 @@ static struct hci_conn *hci_connect_acl(struct hci_dev *hdev, bdaddr_t *dst,
 	return acl;
 }
 
-static struct hci_conn *hci_connect_sco(struct hci_dev *hdev, int type,
-				bdaddr_t *dst, u8 sec_level, u8 auth_type)
+struct hci_conn *hci_connect_sco(struct hci_dev *hdev, int type,
+					bdaddr_t *dst, u8 sec_level,
+					u8 auth_type, u8 codec)
 {
 	struct hci_conn *acl;
 	struct hci_conn *sco;
@@ -575,6 +585,9 @@ static struct hci_conn *hci_connect_sco(struct hci_dev *hdev, int type,
 
 	hci_conn_hold(sco);
 
+	if (codec)
+		set_bit(HCI_CONN_SCO_TRANSPARENT, &sco->flags);
+
 	if (acl->state == BT_CONNECTED &&
 	    (sco->state == BT_OPEN || sco->state == BT_CLOSED)) {
 		set_bit(HCI_CONN_POWER_SAVE, &acl->flags);
@@ -605,7 +618,7 @@ struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst,
 		return hci_connect_acl(hdev, dst, sec_level, auth_type);
 	case SCO_LINK:
 	case ESCO_LINK:
-		return hci_connect_sco(hdev, type, dst, sec_level, auth_type);
+		return hci_connect_sco(hdev, type, dst, sec_level, auth_type, 0);
 	}
 
 	return ERR_PTR(-EINVAL);
diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index 6a957a3..bee0b5c 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -176,8 +176,8 @@ static int sco_connect(struct sock *sk)
 	else
 		type = SCO_LINK;
 
-	hcon = hci_connect(hdev, type, dst, BDADDR_BREDR, BT_SECURITY_LOW,
-			   HCI_AT_NO_BONDING);
+	hcon = hci_connect_sco(hdev, type, dst, BT_SECURITY_LOW,
+			       HCI_AT_NO_BONDING, sco_pi(sk)->mode);
 	if (IS_ERR(hcon)) {
 		err = PTR_ERR(hcon);
 		goto done;
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 5/5] Bluetooth: Fallback transparent SCO from T2 to T1
From: Frédéric Dalleau @ 2013-01-17 12:39 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Frédéric Dalleau
In-Reply-To: <1358426389-25903-1-git-send-email-frederic.dalleau@linux.intel.com>

When initiating a transparent eSCO connection, make use of T2 settings at
first try. T2 is the recommended settings from HFP 1.6 WideBand Speech. Upon
connection failure, try T1 settings.
To know which of T2 or T1 should be used, the connection attempt index is used.
T2 failure is detected if Synchronous Connection Complete Event fails with
error 0x0d. This error code has been found experimentally by sending a T2
request to a T1 only SCO listener. It means "Connection Rejected due to
Limited resource".
---
 net/bluetooth/hci_conn.c  |   11 ++++++++++-
 net/bluetooth/hci_event.c |    1 +
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index e3f1fad..d08f001 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -179,12 +179,21 @@ void hci_setup_sync(struct hci_conn *conn, __u16 handle)
 	cp.tx_bandwidth   = __constant_cpu_to_le32(0x00001f40);
 	cp.rx_bandwidth   = __constant_cpu_to_le32(0x00001f40);
 
-	if (test_and_clear_bit(HCI_CONN_SCO_TRANSPARENT, &conn->flags)) {
+	if (conn->attempt == 1 &&
+	    test_bit(HCI_CONN_SCO_TRANSPARENT, &conn->flags)) {
 		cp.pkt_type       = __constant_cpu_to_le16(EDR_ESCO_MASK &
 							   ~ESCO_2EV3);
 		cp.max_latency    = __constant_cpu_to_le16(0x000d);
 		cp.voice_setting  = __constant_cpu_to_le16(0x0003);
 		cp.retrans_effort = 0x02;
+	} else if (conn->attempt == 2 &&
+		   test_bit(HCI_CONN_SCO_TRANSPARENT, &conn->flags)) {
+		cp.pkt_type       = __constant_cpu_to_le16(EDR_ESCO_MASK |
+							   ESCO_EV3);
+		cp.max_latency    = __constant_cpu_to_le16(0x0007);
+		cp.voice_setting  = __constant_cpu_to_le16(0x0003);
+		cp.retrans_effort = 0x02;
+		clear_bit(HCI_CONN_SCO_TRANSPARENT, &conn->flags);
 	} else {
 		cp.pkt_type       = cpu_to_le16(conn->pkt_type);
 		cp.max_latency    = __constant_cpu_to_le16(0xffff);
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index afa0104..ba4af46 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -3319,6 +3319,7 @@ static void hci_sync_conn_complete_evt(struct hci_dev *hdev,
 		hci_conn_add_sysfs(conn);
 		break;
 
+	case 0x0d:	/* No resource available */
 	case 0x11:	/* Unsupported Feature or Parameter Value */
 	case 0x1c:	/* SCO interval rejected */
 	case 0x1a:	/* Unsupported Remote Feature */
-- 
1.7.9.5


^ permalink raw reply related

* Re: [PATCH 1/5] Bluetooth: Add option for SCO socket mode
From: Marcel Holtmann @ 2013-01-17 14:09 UTC (permalink / raw)
  To: Frédéric Dalleau; +Cc: linux-bluetooth
In-Reply-To: <1358426389-25903-2-git-send-email-frederic.dalleau@linux.intel.com>

Hi Fred,

> This patch extends the current SCO socket option to add a 'mode' field. This
> field is intended to choose data type at runtime. Current modes are CVSD and
> transparent SCO, but adding new modes could allow support for CSA2 and fine
> tuning a sco connection, for example latency, bandwith, voice setting. Incoming
> connections will be setup during defered setup. Outgoing connections have to
> be setup before connect(). The selected type is stored in the sco socket info.
> This patch declares needed members and implements getsockopt().
> ---
>  include/net/bluetooth/sco.h |   20 ++++++++++++++++++++
>  net/bluetooth/sco.c         |    3 +++
>  2 files changed, 23 insertions(+)
> 
> diff --git a/include/net/bluetooth/sco.h b/include/net/bluetooth/sco.h
> index 1e35c43..bc5d3d6 100644
> --- a/include/net/bluetooth/sco.h
> +++ b/include/net/bluetooth/sco.h
> @@ -41,8 +41,27 @@ struct sockaddr_sco {
>  
>  /* SCO socket options */
>  #define SCO_OPTIONS	0x01
> +
> +#define SCO_MODE_CVSD		0x00
> +#define SCO_MODE_TRANSPARENT	0x01
> +#define SCO_MODE_ENHANCED	0x02
> +

I do not like this enhanced magic. How is this suppose to work? And it
is also not explained in the commit message. Or used in this patch.

Regards

Marcel



^ permalink raw reply

* Re: [PATCH 2/5] Bluetooth: Add setsockopt for SCO socket mode
From: Marcel Holtmann @ 2013-01-17 14:12 UTC (permalink / raw)
  To: Frédéric Dalleau; +Cc: linux-bluetooth
In-Reply-To: <1358426389-25903-3-git-send-email-frederic.dalleau@linux.intel.com>

Hi Fred,

> This patch implements setsockopt().

not acceptable commit message.

> ---
>  net/bluetooth/sco.c |   44 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
> 
> diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
> index bdb21b2..22ad5fa 100644
> --- a/net/bluetooth/sco.c
> +++ b/net/bluetooth/sco.c
> @@ -678,6 +678,47 @@ static int sco_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
>  	return bt_sock_recvmsg(iocb, sock, msg, len, flags);
>  }
>  
> +static int sco_sock_setsockopt_old(struct socket *sock, int optname,
> +				   char __user *optval, unsigned int optlen)
> +{
> +	struct sock *sk = sock->sk;
> +	struct sco_options opts;
> +	int len, err = 0;
> +
> +	BT_DBG("sk %p", sk);
> +
> +	lock_sock(sk);
> +
> +	switch (optname) {
> +	case SCO_OPTIONS:
> +		if (sk->sk_state != BT_OPEN &&
> +		    sk->sk_state != BT_BOUND &&
> +		    sk->sk_state != BT_CONNECT2) {
> +			err = -EINVAL;
> +			break;
> +		}
> +
> +		opts.mode = SCO_MODE_CVSD;

Don't we need to set a opts.mtu here as well? This is user input. So we
need to be 100% sure to verify it.

Regards

Marcel



^ permalink raw reply

* Re: [PATCH 3/5] Bluetooth: Use mode when accepting SCO connection
From: Marcel Holtmann @ 2013-01-17 14:15 UTC (permalink / raw)
  To: Frédéric Dalleau; +Cc: linux-bluetooth
In-Reply-To: <1358426389-25903-4-git-send-email-frederic.dalleau@linux.intel.com>

Hi Fred,

> When an incoming SCO connection is requested, check the selected mode, and
> reply appropriately. Mode should have been negotiated previously. For example,
> in case of HFP, the codec is negotiated using AT commands on the RFCOMM
> channel. This patch only changes replies for socket with defered setup enabled.
> ---
>  include/net/bluetooth/hci_core.h |    2 +-
>  net/bluetooth/hci_event.c        |   21 +++++++++++++++++----
>  net/bluetooth/sco.c              |    2 +-
>  3 files changed, 19 insertions(+), 6 deletions(-)
> 
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 014a2ea..cb5d131 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -577,7 +577,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);
> +void hci_conn_accept(struct hci_conn *conn, int mask, int mode);
>  
>  struct hci_chan *hci_chan_create(struct hci_conn *conn);
>  void hci_chan_del(struct hci_chan *chan);
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 705078a..afa0104 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -2047,7 +2047,7 @@ unlock:
>  	hci_conn_check_pending(hdev);
>  }
>  
> -void hci_conn_accept(struct hci_conn *conn, int mask)
> +void hci_conn_accept(struct hci_conn *conn, int mask, int mode)
>  {
>  	struct hci_dev *hdev = conn->hdev;
>  
> @@ -2074,9 +2074,22 @@ void hci_conn_accept(struct hci_conn *conn, int mask)
>  
>  		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;
> +
> +		switch (mode) {
> +		case 0:
> +			cp.max_latency    = __constant_cpu_to_le16(0xffff);
> +			cp.content_format = cpu_to_le16(hdev->voice_setting);
> +			cp.retrans_effort = 0xff;
> +			break;
> +		case 1:
> +			if (conn->pkt_type & ESCO_2EV3)
> +				cp.max_latency = __constant_cpu_to_le16(0x0008);
> +			else
> +				cp.max_latency = __constant_cpu_to_le16(0x000D);
> +			cp.content_format = __constant_cpu_to_le16(0x0003);
> +			cp.retrans_effort = 0x02;
> +			break;
> +		}

so what happens if someone sets mode == 0x02, then we just send some
random data. This reminds me, we need to do range checks for the
setsockopt call. Only valid modes are suppose to be allowed.
>  
>  		hci_send_cmd(hdev, HCI_OP_ACCEPT_SYNC_CONN_REQ,
>  			     sizeof(cp), &cp);
> diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
> index 22ad5fa..6a957a3 100644
> --- a/net/bluetooth/sco.c
> +++ b/net/bluetooth/sco.c
> @@ -666,7 +666,7 @@ static int sco_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
>  
>  	if (sk->sk_state == BT_CONNECT2 &&
>  	    test_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags)) {
> -		hci_conn_accept(pi->conn->hcon, 0);
> +		hci_conn_accept(pi->conn->hcon, 0, pi->mode);
>  		sk->sk_state = BT_CONFIG;
>  
>  		release_sock(sk);

Regards

Marcel



^ permalink raw reply

* Re: [PATCH 4/5] Bluetooth: Parameters for outgoing SCO connections
From: Marcel Holtmann @ 2013-01-17 14:17 UTC (permalink / raw)
  To: Frédéric Dalleau; +Cc: linux-bluetooth
In-Reply-To: <1358426389-25903-5-git-send-email-frederic.dalleau@linux.intel.com>

Hi Fred,

> In order to establish a transparent SCO connection, the correct settings must
> be specified in the SetupSynchronousConnection request. For that, a bit is set
> in ACL connection flags to set up the desired parameters. If this bit is not
> set, a legacy SCO connection will be requested.
> This patch uses T2 settings.
> ---
>  include/net/bluetooth/hci_core.h |    3 +++
>  net/bluetooth/hci_conn.c         |   27 ++++++++++++++++++++-------
>  net/bluetooth/sco.c              |    4 ++--
>  3 files changed, 25 insertions(+), 9 deletions(-)
> 
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index cb5d131..9ef7fe0 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -438,6 +438,7 @@ enum {
>  	HCI_CONN_SSP_ENABLED,
>  	HCI_CONN_POWER_SAVE,
>  	HCI_CONN_REMOTE_OOB,
> +	HCI_CONN_SCO_TRANSPARENT,
>  };
>  
>  static inline bool hci_conn_ssp_enabled(struct hci_conn *conn)
> @@ -586,6 +587,8 @@ struct hci_chan *hci_chan_lookup_handle(struct hci_dev *hdev, __u16 handle);
>  
>  struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst,
>  			     __u8 dst_type, __u8 sec_level, __u8 auth_type);
> +struct hci_conn *hci_connect_sco(struct hci_dev *hdev, int type, bdaddr_t *dst,
> +				 u8 sec_level, u8 auth_type, u8 codec);
>  int hci_conn_check_link_mode(struct hci_conn *conn);
>  int hci_conn_check_secure(struct hci_conn *conn, __u8 sec_level);
>  int hci_conn_security(struct hci_conn *conn, __u8 sec_level, __u8 auth_type);
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index 25bfce0..e3f1fad 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -175,13 +175,22 @@ void hci_setup_sync(struct hci_conn *conn, __u16 handle)
>  	conn->attempt++;
>  
>  	cp.handle   = cpu_to_le16(handle);
> -	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.voice_setting  = cpu_to_le16(hdev->voice_setting);
> -	cp.retrans_effort = 0xff;
> +
> +	if (test_and_clear_bit(HCI_CONN_SCO_TRANSPARENT, &conn->flags)) {
> +		cp.pkt_type       = __constant_cpu_to_le16(EDR_ESCO_MASK &
> +							   ~ESCO_2EV3);
> +		cp.max_latency    = __constant_cpu_to_le16(0x000d);
> +		cp.voice_setting  = __constant_cpu_to_le16(0x0003);
> +		cp.retrans_effort = 0x02;
> +	} else {
> +		cp.pkt_type       = cpu_to_le16(conn->pkt_type);
> +		cp.max_latency    = __constant_cpu_to_le16(0xffff);
> +		cp.voice_setting  = cpu_to_le16(hdev->voice_setting);
> +		cp.retrans_effort = 0xff;
> +	}
>  
>  	hci_send_cmd(hdev, HCI_OP_SETUP_SYNC_CONN, sizeof(cp), &cp);
>  }
> @@ -551,8 +560,9 @@ static struct hci_conn *hci_connect_acl(struct hci_dev *hdev, bdaddr_t *dst,
>  	return acl;
>  }
>  
> -static struct hci_conn *hci_connect_sco(struct hci_dev *hdev, int type,
> -				bdaddr_t *dst, u8 sec_level, u8 auth_type)
> +struct hci_conn *hci_connect_sco(struct hci_dev *hdev, int type,
> +					bdaddr_t *dst, u8 sec_level,
> +					u8 auth_type, u8 codec)
>  {
>  	struct hci_conn *acl;
>  	struct hci_conn *sco;
> @@ -575,6 +585,9 @@ static struct hci_conn *hci_connect_sco(struct hci_dev *hdev, int type,
>  
>  	hci_conn_hold(sco);
>  
> +	if (codec)
> +		set_bit(HCI_CONN_SCO_TRANSPARENT, &sco->flags);
> +
>  	if (acl->state == BT_CONNECTED &&
>  	    (sco->state == BT_OPEN || sco->state == BT_CLOSED)) {
>  		set_bit(HCI_CONN_POWER_SAVE, &acl->flags);
> @@ -605,7 +618,7 @@ struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst,
>  		return hci_connect_acl(hdev, dst, sec_level, auth_type);
>  	case SCO_LINK:
>  	case ESCO_LINK:
> -		return hci_connect_sco(hdev, type, dst, sec_level, auth_type);
> +		return hci_connect_sco(hdev, type, dst, sec_level, auth_type, 0);
>  	}
>  
>  	return ERR_PTR(-EINVAL);
> diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
> index 6a957a3..bee0b5c 100644
> --- a/net/bluetooth/sco.c
> +++ b/net/bluetooth/sco.c
> @@ -176,8 +176,8 @@ static int sco_connect(struct sock *sk)
>  	else
>  		type = SCO_LINK;
>  
> -	hcon = hci_connect(hdev, type, dst, BDADDR_BREDR, BT_SECURITY_LOW,
> -			   HCI_AT_NO_BONDING);
> +	hcon = hci_connect_sco(hdev, type, dst, BT_SECURITY_LOW,
> +			       HCI_AT_NO_BONDING, sco_pi(sk)->mode);

if we start doing this, then the security requirements for SCO channels
are pointless. They do not exists.

Regards

Marcel



^ permalink raw reply

* Re: [PATCH 1/5] Bluetooth: Add option for SCO socket mode
From: Frédéric Dalleau @ 2013-01-17 15:36 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: Frédéric Dalleau, linux-bluetooth
In-Reply-To: <1358431772.3059.13.camel@aeonflux>

On Thu, Jan 17, 2013 at 06:09:32AM -0800, Marcel Holtmann wrote:
Hi Marcel,
> I do not like this enhanced magic. How is this suppose to work? And it
> is also not explained in the commit message. Or used in this patch.

sco_options_enhanced is not definitive, this patch is meant to describe
different parameters for the socket option that could be selected based on the
mode field. To ensure compatibility accross versions, the size of the socket
options structure can be checked.

The idea is to read supported codecs list using mgmt API. Then,it is possible
to select some codecs and the enhanced accept/setup sync conn are sent.

It does not make sense to get this option since the codec is negociated
externally.

If this mode is removed, we fallback on version 1 of RFC (except at this time,
sco_options.mode was sco_options.codec).  And it is it is easy to add new modes
to select latency, or packet types without breaking existing API.

Thanks,
Frédéric

^ permalink raw reply

* Thoughts about LE scanning
From: Marcel Holtmann @ 2013-01-17 22:02 UTC (permalink / raw)
  To: linux-bluetooth

Hello,

so I have been looking into how we handle LE scanning in central role a
little bit. And I am not really happy with what we do right now. Instead
of just adding new profiles, we need to take one step back and get the
basics right.

The one thing that I stumbled about is that we are trying to use the
Start Discovery management command to scan for connectable device that
we are paired with. And it is all triggered by bluetoothd. It tries to
get this done right, but fails badly at it. Trying to do this ends up in
a convoluted solution that will just break down eventually.

So Start Discovery and Stop Discovery management commands are for
finding new devices. They are for finding devices that we want to pair
with. They are not for checking if a paired device is in range or
signals connection requests to us. It is called discovery for a reason.

It might have looked like a good idea to just re-use these two commands,
but what I am seeing when working with multiple paired LE devices is
just a big mess. The amount of code that is needed to keep track of
states between running D-Bus commands for discovery, discovery triggered
management commands, scanning triggered management commands etc. is not
a good idea.

I am failing to understand why we tried to solve this inside bluetoothd
and not just let the kernel take full control here. We are loading the
list of paired LE devices at controller power on anyway. So the kernel
does know about our paired devices. It can control sensible scan
intervals and also sync a device discovery with scanning for connection
triggers from know remote devices. It also can sensible disconnect on
idle and scan for other devices that requests connections.

What I think we should have is a management command that allows us to
load device specific scan parameter configuration into the kernel. And
then the kernel will execute this on our behalf. We need to decouple the
commands for device discovery from the commands that scan for known
devices.

Seems also we should actually implement the scan parameters service as a
core feature of bluetoothd and not just a plugin. For me it looks like
it essential that we allow different behavior for different devices.

Regards

Marcel



^ permalink raw reply

* Re: [PATCH 3/4] Bluetooth: Fix using system-global workqueue when not necessary
From: Gustavo Padovan @ 2013-01-18  5:07 UTC (permalink / raw)
  To: Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <1358195633-29303-4-git-send-email-johan.hedberg@gmail.com>

Hi Johan,

* Johan Hedberg <johan.hedberg@gmail.com> [2013-01-14 22:33:52 +0200]:

> From: Johan Hedberg <johan.hedberg@intel.com>
> 
> There's a per-HCI device workqueue (hdev->workqueue) that should be used
> for general per-HCI device work (except hdev->req_workqueue that's for
> hci_request() related work). This patch fixes places using the
> system-global work queue and makes them use the hdev->workqueue instead.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
>  net/bluetooth/hci_core.c |    4 ++--
>  net/bluetooth/mgmt.c     |    3 ++-
>  2 files changed, 4 insertions(+), 3 deletions(-)

Patches 1 to 3 have been applied to bluetooth-next. Thanks.

	Gustavo

^ permalink raw reply

* Re: [PATCH] Bluetooth: btmrvl_sdio: look for sd8688 firmware in alternate place
From: Lubomir Rintel @ 2013-01-18  7:33 UTC (permalink / raw)
  To: Marcel Holtmann, David Woodhouse, Ben Hutchings
  Cc: Bing Zhao, libertas-dev@lists.infradead.org,
	linux-bluetooth@vger.kernel.org, Gustavo Padovan, Johan Hedberg,
	linux-kernel@vger.kernel.org
In-Reply-To: <1357713345.1806.44.camel@aeonflux>

On Tue, 2013-01-08 at 22:35 -0800, Marcel Holtmann wrote:
> Hi Lubomir,
> 
> > > > > linux-firmware ships the sd8688* firmware images that are shared with
> > > > > libertas_sdio WiFi driver under libertas/. libertas_sdio looks in both places
> > > > > and so should we.
> > > > >
> > > > > Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> > > > > ---
> > > > >  drivers/bluetooth/btmrvl_sdio.c |   24 ++++++++++++++++++++++--
> > > > >  drivers/bluetooth/btmrvl_sdio.h |    6 ++++--
> > > > >  2 files changed, 26 insertions(+), 4 deletions(-)
> > > > 
> > > > NAK from me on this one. I do not want the driver to check two
> > > > locations. That is what userspace can work around.
> > > > 
> > > > If we want to unify the location between the WiFi driver and the
> > > > Bluetooth driver, I am fine with that, but seriously, just pick one over
> > > > the other. I do not care which one.
> > > 
> > > The unified location is mrvl/ directory.
> > > 
> > > We can probably move SD8688 firmware & helper binaries to mrvl/ and have both drivers grab the images there?
> > 
> > That would break existing setups, wouldn't it?
> > 
> > I was under impression (commit 3d32a58b) that we care about
> > compatibility here. Do we?
> 
> that is what symlinks are for.

David, Ben: please pull the following branch then:
git pull git://github.com/lkundrak/linux-firmware.git sd8688-move

Thank you!

^ permalink raw reply

* [PATCH] bluetooth: btmrvl_sdio: look for sd8688 firmware in proper location
From: Lubomir Rintel @ 2013-01-18  7:37 UTC (permalink / raw)
  To: Marcel Holtmann, Bing Zhao
  Cc: Ben Hutchings, David Woodhouse, Gustavo Padovan, Johan Hedberg,
	libertas-dev, linux-bluetooth, linux-kernel, Lubomir Rintel
In-Reply-To: <477F20668A386D41ADCC57781B1F70430D13FDB783@SC-VEXCH1.marvell.com>

Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
 drivers/bluetooth/btmrvl_sdio.c |    8 ++++----
 drivers/bluetooth/btmrvl_sdio.h |    6 ++++--
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/bluetooth/btmrvl_sdio.c b/drivers/bluetooth/btmrvl_sdio.c
index 3f4bfc8..bc27d01 100644
--- a/drivers/bluetooth/btmrvl_sdio.c
+++ b/drivers/bluetooth/btmrvl_sdio.c
@@ -83,8 +83,8 @@ static const struct btmrvl_sdio_card_reg btmrvl_reg_87xx = {
 };
 
 static const struct btmrvl_sdio_device btmrvl_sdio_sd8688 = {
-	.helper		= "sd8688_helper.bin",
-	.firmware	= "sd8688.bin",
+	.helper		= "mrvl/sd8688_helper.bin",
+	.firmware	= "mrvl/sd8688.bin",
 	.reg		= &btmrvl_reg_8688,
 	.sd_blksz_fw_dl	= 64,
 };
@@ -1179,7 +1179,7 @@ MODULE_AUTHOR("Marvell International Ltd.");
 MODULE_DESCRIPTION("Marvell BT-over-SDIO driver ver " VERSION);
 MODULE_VERSION(VERSION);
 MODULE_LICENSE("GPL v2");
-MODULE_FIRMWARE("sd8688_helper.bin");
-MODULE_FIRMWARE("sd8688.bin");
+MODULE_FIRMWARE("mrvl/sd8688_helper.bin");
+MODULE_FIRMWARE("mrvl/sd8688.bin");
 MODULE_FIRMWARE("mrvl/sd8787_uapsta.bin");
 MODULE_FIRMWARE("mrvl/sd8797_uapsta.bin");
diff --git a/drivers/bluetooth/btmrvl_sdio.h b/drivers/bluetooth/btmrvl_sdio.h
index 43d35a6..4a5a019 100644
--- a/drivers/bluetooth/btmrvl_sdio.h
+++ b/drivers/bluetooth/btmrvl_sdio.h
@@ -84,7 +84,9 @@ struct btmrvl_sdio_card {
 	struct sdio_func *func;
 	u32 ioport;
 	const char *helper;
+	const char *helper2;
 	const char *firmware;
+	const char *firmware2;
 	const struct btmrvl_sdio_card_reg *reg;
 	u16 sd_blksz_fw_dl;
 	u8 rx_unit;
@@ -92,8 +94,8 @@ struct btmrvl_sdio_card {
 };
 
 struct btmrvl_sdio_device {
-	const char *helper;
-	const char *firmware;
+	const char *helper, *helper2;
+	const char *firmware, *firmware2;
 	const struct btmrvl_sdio_card_reg *reg;
 	u16 sd_blksz_fw_dl;
 };
-- 
1.7.1

^ permalink raw reply related


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