Linux bluetooth development
 help / color / mirror / Atom feed
* [PATCH 09/14] obexd/service: Remove unused local variable
From: Szymon Janc @ 2013-10-04  9:05 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1380877538-22614-1-git-send-email-szymon.janc@tieto.com>

This fix following build error:

  CC     obexd/src/obexd-service.o
obexd/src/service.c: In function ‘obex_service_driver_register’:
obexd/src/service.c:100:10: error: unused variable ‘l’ [-Werror=unused-variable]
---
 obexd/src/service.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/obexd/src/service.c b/obexd/src/service.c
index 6b8533b..c088535 100644
--- a/obexd/src/service.c
+++ b/obexd/src/service.c
@@ -97,8 +97,6 @@ static struct obex_service_driver *find_driver(uint16_t service)
 
 int obex_service_driver_register(struct obex_service_driver *driver)
 {
-	GSList *l;
-
 	if (!driver) {
 		error("Invalid driver");
 		return -EINVAL;
-- 
1.8.4


^ permalink raw reply related

* [PATCH 08/14] obexd: Return "error" for unknown status in status2str
From: Szymon Janc @ 2013-10-04  9:05 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1380877538-22614-1-git-send-email-szymon.janc@tieto.com>

This can happend only if there is a bug in obexd code.
This fix following buld error:

  CC     obexd/src/obexd-manager.o
obexd/src/manager.c: In function ‘status2str’:
obexd/src/manager.c:292:1: error: control reaches end of non-void
    function [-Werror=return-type]
---
 obexd/src/manager.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/obexd/src/manager.c b/obexd/src/manager.c
index 00f3537..cec8a39 100644
--- a/obexd/src/manager.c
+++ b/obexd/src/manager.c
@@ -287,6 +287,7 @@ static const char *status2str(uint8_t status)
 	case TRANSFER_STATUS_COMPLETE:
 		return "complete";
 	case TRANSFER_STATUS_ERROR:
+	default:
 		return "error";
 	}
 }
-- 
1.8.4


^ permalink raw reply related

* [PATCH 07/14] obexd: Remove unused local variables
From: Szymon Janc @ 2013-10-04  9:05 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1380877538-22614-1-git-send-email-szymon.janc@tieto.com>

This fix following build errors:

obexd/src/manager.c: In function ‘get_root’:
obexd/src/manager.c:253:23: error: unused variable ‘os’
    [-Werror=unused-variable]

obexd/src/manager.c: In function ‘manager_emit_transfer_started’:
obexd/src/manager.c:534:22: error: unused variable ‘id’
    [-Werror=unused-variable]

obexd/src/manager.c: In function ‘manager_request_authorization’:
obexd/src/manager.c:684:14: error: unused variable ‘type’
    [-Werror=unused-variable]
obexd/src/manager.c:683:14: error: unused variable ‘filename’
    [-Werror=unused-variable]
---
 obexd/src/manager.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/obexd/src/manager.c b/obexd/src/manager.c
index a67838c..00f3537 100644
--- a/obexd/src/manager.c
+++ b/obexd/src/manager.c
@@ -250,7 +250,6 @@ static gboolean get_target(const GDBusPropertyTable *property,
 static gboolean get_root(const GDBusPropertyTable *property,
 					DBusMessageIter *iter, void *data)
 {
-	struct obex_session *os = data;
 	const char *root;
 
 	root = obex_option_root_folder();
@@ -531,8 +530,6 @@ void manager_cleanup(void)
 
 void manager_emit_transfer_started(struct obex_transfer *transfer)
 {
-	static unsigned int id = 0;
-
 	transfer->status = TRANSFER_STATUS_ACTIVE;
 
 	g_dbus_emit_property_changed(connection, transfer->path,
@@ -680,8 +677,6 @@ int manager_request_authorization(struct obex_transfer *transfer, int32_t time,
 	struct obex_session *os = transfer->session;
 	DBusMessage *msg;
 	DBusPendingCall *call;
-	const char *filename = os->name ? os->name : "";
-	const char *type = os->type ? os->type : "";
 	unsigned int watch;
 	gboolean got_reply;
 
-- 
1.8.4


^ permalink raw reply related

* [PATCH 06/14] obexd: Remove unused static functions
From: Szymon Janc @ 2013-10-04  9:05 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1380877538-22614-1-git-send-email-szymon.janc@tieto.com>

This fix following build error:

obexd/src/manager.c: At top level:
obexd/src/manager.c:190:13: error:
     ‘dbus_message_iter_append_dict_entry’ defined but not used
     [-Werror=unused-function]
---
 obexd/src/manager.c | 75 -----------------------------------------------------
 1 file changed, 75 deletions(-)

diff --git a/obexd/src/manager.c b/obexd/src/manager.c
index f64b7b9..a67838c 100644
--- a/obexd/src/manager.c
+++ b/obexd/src/manager.c
@@ -127,81 +127,6 @@ static inline DBusMessage *not_authorized(DBusMessage *msg)
 			"Not authorized");
 }
 
-static void dbus_message_iter_append_variant(DBusMessageIter *iter,
-						int type, void *val)
-{
-	DBusMessageIter value;
-	DBusMessageIter array;
-	const char *sig;
-
-	switch (type) {
-	case DBUS_TYPE_STRING:
-		sig = DBUS_TYPE_STRING_AS_STRING;
-		break;
-	case DBUS_TYPE_BYTE:
-		sig = DBUS_TYPE_BYTE_AS_STRING;
-		break;
-	case DBUS_TYPE_INT16:
-		sig = DBUS_TYPE_INT16_AS_STRING;
-		break;
-	case DBUS_TYPE_UINT16:
-		sig = DBUS_TYPE_UINT16_AS_STRING;
-		break;
-	case DBUS_TYPE_INT32:
-		sig = DBUS_TYPE_INT32_AS_STRING;
-		break;
-	case DBUS_TYPE_UINT32:
-		sig = DBUS_TYPE_UINT32_AS_STRING;
-		break;
-	case DBUS_TYPE_BOOLEAN:
-		sig = DBUS_TYPE_BOOLEAN_AS_STRING;
-		break;
-	case DBUS_TYPE_ARRAY:
-		sig = DBUS_TYPE_ARRAY_AS_STRING DBUS_TYPE_STRING_AS_STRING;
-		break;
-	case DBUS_TYPE_OBJECT_PATH:
-		sig = DBUS_TYPE_OBJECT_PATH_AS_STRING;
-		break;
-	default:
-		error("Could not append variant with type %d", type);
-		return;
-	}
-
-	dbus_message_iter_open_container(iter, DBUS_TYPE_VARIANT, sig, &value);
-
-	if (type == DBUS_TYPE_ARRAY) {
-		int i;
-		const char ***str_array = val;
-
-		dbus_message_iter_open_container(&value, DBUS_TYPE_ARRAY,
-			DBUS_TYPE_STRING_AS_STRING, &array);
-
-		for (i = 0; (*str_array)[i]; i++)
-			dbus_message_iter_append_basic(&array, DBUS_TYPE_STRING,
-							&((*str_array)[i]));
-
-		dbus_message_iter_close_container(&value, &array);
-	} else
-		dbus_message_iter_append_basic(&value, type, val);
-
-	dbus_message_iter_close_container(iter, &value);
-}
-
-static void dbus_message_iter_append_dict_entry(DBusMessageIter *dict,
-					const char *key, int type, void *val)
-{
-	DBusMessageIter entry;
-
-	dbus_message_iter_open_container(dict, DBUS_TYPE_DICT_ENTRY,
-					NULL, &entry);
-
-	dbus_message_iter_append_basic(&entry, DBUS_TYPE_STRING, &key);
-
-	dbus_message_iter_append_variant(&entry, type, val);
-
-	dbus_message_iter_close_container(dict, &entry);
-}
-
 static void agent_disconnected(DBusConnection *conn, void *user_data)
 {
 	DBG("Agent exited");
-- 
1.8.4


^ permalink raw reply related

* [PATCH 05/14] obexd/MAP: Remove unsued mas_drivers variable
From: Szymon Janc @ 2013-10-04  9:05 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1380877538-22614-1-git-send-email-szymon.janc@tieto.com>

This fix following build error:

  CC     obexd/client/obexd-mns.o
obexd/client/mns.c:344:38: error: ‘mas_drivers’ defined but not used
    [-Werror=unused-variable]
cc1: all warnings being treated as errors
---
 obexd/client/mns.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/obexd/client/mns.c b/obexd/client/mns.c
index 0663def..315764b 100644
--- a/obexd/client/mns.c
+++ b/obexd/client/mns.c
@@ -341,11 +341,6 @@ static struct obex_mime_type_driver mime_event_report = {
 	.write = event_report_write,
 };
 
-static struct obex_mime_type_driver *mas_drivers[] = {
-	&mime_event_report,
-	NULL
-};
-
 static int mns_init(void)
 {
 	int err;
-- 
1.8.4


^ permalink raw reply related

* [PATCH 04/14] obexd/MAP: Fix missing include
From: Szymon Janc @ 2013-10-04  9:05 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1380877538-22614-1-git-send-email-szymon.janc@tieto.com>

This fix following build error:

  CC     obexd/client/obexd-mns.o
obexd/client/mns.c: In function ‘parse_event_report_handle’:
obexd/client/mns.c:187:2: error: implicit declaration of function
    ‘strtoull’ [-Werror=implicit-function-declaration]
---
 obexd/client/mns.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/obexd/client/mns.c b/obexd/client/mns.c
index b709650..0663def 100644
--- a/obexd/client/mns.c
+++ b/obexd/client/mns.c
@@ -31,6 +31,7 @@
 #include <fcntl.h>
 #include <inttypes.h>
 #include <stdbool.h>
+#include <stdlib.h>
 
 #include <gobex/gobex.h>
 #include <gobex/gobex-apparam.h>
-- 
1.8.4


^ permalink raw reply related

* [PATCH 03/14] obexd/MAP: Fix missing include
From: Szymon Janc @ 2013-10-04  9:05 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1380877538-22614-1-git-send-email-szymon.janc@tieto.com>

This fix following build error:

  CC     obexd/client/obexd-mns.o
obexd/client/mns.c: In function ‘mns_connect’:
obexd/client/mns.c:105:2: error: implicit declaration of function
    ‘manager_register_session’ [-Werror=implicit-function-declaration]
obexd/client/mns.c: In function ‘mns_disconnect’:
obexd/client/mns.c:128:2: error: implicit declaration of function
    ‘manager_unregister_session’ [-Werror=implicit-function-declaration]
---
 obexd/client/mns.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/obexd/client/mns.c b/obexd/client/mns.c
index ee64ed6..b709650 100644
--- a/obexd/client/mns.c
+++ b/obexd/client/mns.c
@@ -44,6 +44,8 @@
 #include "map_ap.h"
 #include "map-event.h"
 
+#include "../src/manager.h"
+
 struct mns_session {
 	GString *buffer;
 	GObexApparam *inparams;
-- 
1.8.4


^ permalink raw reply related

* [PATCH 02/14] obexd/bluetooth: Make dict_append_entry function static
From: Szymon Janc @ 2013-10-04  9:05 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1380877538-22614-1-git-send-email-szymon.janc@tieto.com>

This fix following build error:

  CC     obexd/plugins/obexd-bluetooth.o
obexd/plugins/bluetooth.c:242:6: error: no previous declaration for
     ‘dict_append_entry’ [-Werror=missing-declarations]
---
 obexd/plugins/bluetooth.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/obexd/plugins/bluetooth.c b/obexd/plugins/bluetooth.c
index 7f8a26d..143d0c2 100644
--- a/obexd/plugins/bluetooth.c
+++ b/obexd/plugins/bluetooth.c
@@ -239,7 +239,7 @@ static void append_variant(DBusMessageIter *iter, int type, void *val)
 }
 
 
-void dict_append_entry(DBusMessageIter *dict,
+static void dict_append_entry(DBusMessageIter *dict,
 			const char *key, int type, void *val)
 {
 	DBusMessageIter entry;
-- 
1.8.4


^ permalink raw reply related

* [PATCH 01/14] obexd/bluetooth: Remove unused local variables
From: Szymon Janc @ 2013-10-04  9:05 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

This ix following build errors:

  CC     obexd/plugins/obexd-bluetooth.o
obexd/plugins/bluetooth.c: In function ‘register_profile_reply’:
obexd/plugins/bluetooth.c:202:10: error: unused variable ‘err’
    [-Werror=unused-variable]

obexd/plugins/bluetooth.c: In function ‘name_acquired’:
obexd/plugins/bluetooth.c:367:15: error: unused variable ‘uuid’
    [-Werror=unused-variable]

obexd/plugins/bluetooth.c: In function ‘name_released’:
obexd/plugins/bluetooth.c:389:15: error: unused variable ‘uuid’
    [-Werror=unused-variable]

obexd/plugins/bluetooth.c: In function ‘bluetooth_start’:
obexd/plugins/bluetooth.c:400:10: error: unused variable ‘ios’
    [-Werror=unused-variable]
---
 obexd/plugins/bluetooth.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/obexd/plugins/bluetooth.c b/obexd/plugins/bluetooth.c
index 017ff60..7f8a26d 100644
--- a/obexd/plugins/bluetooth.c
+++ b/obexd/plugins/bluetooth.c
@@ -199,7 +199,6 @@ static void register_profile_reply(DBusPendingCall *call, void *user_data)
 	struct bluetooth_profile *profile = user_data;
 	DBusMessage *reply = dbus_pending_call_steal_reply(call);
 	DBusError derr;
-	GError *err = NULL;
 
 	dbus_error_init(&derr);
 	if (!dbus_set_error_from_message(&derr, reply)) {
@@ -364,7 +363,6 @@ static void name_acquired(DBusConnection *conn, void *user_data)
 
 	for (l = profiles; l; l = l->next) {
 		struct bluetooth_profile *profile = l->data;
-		const char *uuid;
 
 		if (profile->path != NULL)
 			continue;
@@ -386,7 +384,6 @@ static void name_released(DBusConnection *conn, void *user_data)
 
 	for (l = profiles; l; l = l->next) {
 		struct bluetooth_profile *profile = l->data;
-		const char *uuid;
 
 		if (profile->path == NULL)
 			continue;
@@ -397,7 +394,6 @@ static void name_released(DBusConnection *conn, void *user_data)
 
 static void *bluetooth_start(struct obex_server *server, int *err)
 {
-	GSList *ios = NULL;
 	const GSList *l;
 
 	for (l = server->drivers; l; l = l->next) {
-- 
1.8.4


^ permalink raw reply related

* Re: [PATCH v4 5/7] android: Android version of log.c
From: Andrei Emeltchenko @ 2013-10-04  8:16 UTC (permalink / raw)
  To: Frederic Danis; +Cc: linux-bluetooth
In-Reply-To: <1380632319-7701-5-git-send-email-frederic.danis@linux.intel.com>

On Tue, Oct 01, 2013 at 02:58:37PM +0200, Frederic Danis wrote:
> ---
>  android/log.c  |  172 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  android/main.c |   34 +++++++++++
>  2 files changed, 206 insertions(+)
>  create mode 100644 android/log.c

What is the status with review. Do I understand correctly that it needs to
be changed something like:

 void info(const char *format, ...)
 {
        va_list ap;
 
        va_start(ap, format);
 
-       android_log(LOG_INFO, format, ap);
+       vfprintf(stderr, format, ap);
+       fprintf(stderr, "\n");
 
        va_end(ap);
 }

If so I can change this myself and resend patch series.

Best regards 
Andrei Emeltchenko 


^ permalink raw reply

* Re: [PATCH] obexd/MAP: Fix invalid snprintf format string in map_msg_get()
From: Luiz Augusto von Dentz @ 2013-10-04  7:35 UTC (permalink / raw)
  To: Szymon Janc; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <1380870086-3663-1-git-send-email-szymon.janc@tieto.com>

Hi Szymon,

On Fri, Oct 4, 2013 at 10:01 AM, Szymon Janc <szymon.janc@tieto.com> wrote:
> obexd/client/map.c: In function ‘map_msg_get’:
> obexd/client/map.c:446:2: warning: format ‘%u’ expects argument of type
>      ‘unsigned int’, but argument 4 has type ‘uint64_t’ [-Wformat]
> obexd/client/map.c:446:2: warning: format ‘%u’ expects argument of type
>      ‘unsigned int’, but argument 4 has type ‘uint64_t’ [-Wformat]
> ---
>  obexd/client/map.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/obexd/client/map.c b/obexd/client/map.c
> index 72c4de4..fe15bab 100644
> --- a/obexd/client/map.c
> +++ b/obexd/client/map.c
> @@ -443,7 +443,7 @@ static DBusMessage *map_msg_get(DBusConnection *connection,
>                 return g_dbus_create_error(message,
>                                 ERROR_INTERFACE ".InvalidArguments", NULL);
>
> -       if (snprintf(handle, sizeof(handle), "%u" PRIu64, msg->handle) < 0)
> +       if (snprintf(handle, sizeof(handle), "%" PRIu64, msg->handle) < 0)
>                 goto fail;
>
>         transfer = obc_transfer_get("x-bt/message", handle, target_file, &err);
> --
> 1.8.4

Applied, thanks

-- 
Luiz Augusto von Dentz

^ permalink raw reply

* Re: [RFCv2 06/14] android: Create HAL API header skeleton
From: Andrei Emeltchenko @ 2013-10-04  7:16 UTC (permalink / raw)
  To: Szymon Janc; +Cc: linux-bluetooth
In-Reply-To: <4501039.OZxtA2WV21@uw000953>

Hi All,

On Thu, Oct 03, 2013 at 04:07:08PM +0200, Szymon Janc wrote:
> > +
> > +/* Core Service */
> > +
> > +struct hal_msg_rsp {
> > +	struct hal_msg_hdr hdr;
> > +	uint8_t status;
> > +} __packed;
> 
> Non-error response to request has no status field, just header.

I have removed the structure in the following patches but I still have
doubts that this is a good approach.

So in a case of error we get:

struct error_rsp {
	struct hal_msg_hdr hdr;
	uint8_t status;
}

and in a case of success we get:

struct rsp {
        struct hal_msg_hdr hdr;
}

Does it make sense to omit status? We could read error_rsp and check
status for error while in the current design we have to check for size
received. Are we gonna save uint8_t?

Best regards 
Andrei Emeltchenko 


^ permalink raw reply

* [PATCH] obexd/MAP: Fix invalid snprintf format string in map_msg_get()
From: Szymon Janc @ 2013-10-04  7:01 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

obexd/client/map.c: In function ‘map_msg_get’:
obexd/client/map.c:446:2: warning: format ‘%u’ expects argument of type
     ‘unsigned int’, but argument 4 has type ‘uint64_t’ [-Wformat]
obexd/client/map.c:446:2: warning: format ‘%u’ expects argument of type
     ‘unsigned int’, but argument 4 has type ‘uint64_t’ [-Wformat]
---
 obexd/client/map.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/obexd/client/map.c b/obexd/client/map.c
index 72c4de4..fe15bab 100644
--- a/obexd/client/map.c
+++ b/obexd/client/map.c
@@ -443,7 +443,7 @@ static DBusMessage *map_msg_get(DBusConnection *connection,
 		return g_dbus_create_error(message,
 				ERROR_INTERFACE ".InvalidArguments", NULL);
 
-	if (snprintf(handle, sizeof(handle), "%u" PRIu64, msg->handle) < 0)
+	if (snprintf(handle, sizeof(handle), "%" PRIu64, msg->handle) < 0)
 		goto fail;
 
 	transfer = obc_transfer_get("x-bt/message", handle, target_file, &err);
-- 
1.8.4


^ permalink raw reply related

* Re: [PATCH v2 4/4] Bluetooth: Refactor hci_connect_le
From: Marcel Holtmann @ 2013-10-04  6:27 UTC (permalink / raw)
  To: Andre Guedes; +Cc: linux-bluetooth
In-Reply-To: <1380835547-3640-5-git-send-email-andre.guedes@openbossa.org>

Hi Andre,

> This patch does some code refactoring in hci_connect_le() by moving
> the exception code into if statements and letting the main flow in
> first level of function scope. It also adds extra comments to improve
> the code readability.
> 
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
> net/bluetooth/hci_conn.c | 55 ++++++++++++++++++++++++++++++------------------
> 1 file changed, 35 insertions(+), 20 deletions(-)
> 
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index cb0e5d7..ff15841 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -531,34 +531,49 @@ static struct hci_conn *hci_connect_le(struct hci_dev *hdev, bdaddr_t *dst,
> 	if (test_bit(HCI_LE_PERIPHERAL, &hdev->flags))
> 		return ERR_PTR(-ENOTSUPP);
> 
> +	/* XXX: Some devices send ATT messages as soon as the physical link

no idea what the XXX wants to tell me here.

> +	 * is established. To be able to handle these ATT messages, the user-
> +	 * space first establishes the connection and then starts the pairing
> +	 * process.
> +	 *
> +	 * So if a hci_conn object already exists for the following connection
> +	 * attempt, we simply update pending_sec_level and auth_type fields
> +	 * and return the object found.
> +	 */
> 	conn = hci_conn_hash_lookup_ba(hdev, LE_LINK, dst);
> -	if (!conn) {
> -		conn = hci_conn_hash_lookup_state(hdev, LE_LINK, BT_CONNECT);
> -		if (conn)
> -			return ERR_PTR(-EBUSY);
> -
> -		conn = hci_conn_add(hdev, LE_LINK, dst);
> -		if (!conn)
> -			return ERR_PTR(-ENOMEM);
> +	if (conn) {
> +		conn->pending_sec_level = sec_level;
> +		conn->auth_type = auth_type;
> +		goto out;
> +	}

Call such a label "done". I never really liked the out part.

If anybody wants to fix the few users of "out" as label, that would be welcome.

> 
> -		conn->dst_type = bdaddr_to_le(dst_type);
> -		conn->state = BT_CONNECT;
> -		conn->out = true;
> -		conn->link_mode |= HCI_LM_MASTER;
> -		conn->sec_level = BT_SECURITY_LOW;
> +	/* Since the controller supports only one LE connection attempt at a
> +	 * time, we return -EBUSY if there is any connection attempt running.
> +	 */
> +	conn = hci_conn_hash_lookup_state(hdev, LE_LINK, BT_CONNECT);
> +	if (conn)
> +		return ERR_PTR(-EBUSY);
> 
> -		err = hci_create_le_conn(hdev, &conn->dst, conn->dst_type);
> -		if (err) {
> -			hci_conn_del(conn);
> -			return ERR_PTR(err);
> -		}
> -	}
> +	conn = hci_conn_add(hdev, LE_LINK, dst);
> +	if (!conn)
> +		return ERR_PTR(-ENOMEM);
> 
> +	conn->dst_type = bdaddr_to_le(dst_type);
> +	conn->state = BT_CONNECT;
> +	conn->out = true;
> +	conn->link_mode |= HCI_LM_MASTER;
> +	conn->sec_level = BT_SECURITY_LOW;
> 	conn->pending_sec_level = sec_level;
> 	conn->auth_type = auth_type;
> 
> -	hci_conn_hold(conn);
> +	err = hci_create_le_conn(hdev, &conn->dst, conn->dst_type);
> +	if (err) {
> +		hci_conn_del(conn);
> +		return ERR_PTR(err);
> +	}
> 
> +out:
> +	hci_conn_hold(conn);
> 	return conn;
> }

Regards

Marcel


^ permalink raw reply

* Re: [PATCH v2 3/4] Bluetooth: Use HCI request for LE connection
From: Marcel Holtmann @ 2013-10-04  6:25 UTC (permalink / raw)
  To: Andre Guedes; +Cc: linux-bluetooth
In-Reply-To: <1380835547-3640-4-git-send-email-andre.guedes@openbossa.org>

Hi Andre,

> This patch introduces a new helper, which uses the HCI request
> framework, for creating LE connectons. All the handling is now
> done by this function so we can remove the hci_cs_le_create_conn()
> event handler.
> 
> This patch also removes the old hci_le_create_connection() since
> it is not used anymore.
> 
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
> include/net/bluetooth/hci_core.h |  2 ++
> net/bluetooth/hci_conn.c         | 26 ++++++-----------------
> net/bluetooth/hci_core.c         | 46 ++++++++++++++++++++++++++++++++++++++++
> net/bluetooth/hci_event.c        | 31 ---------------------------
> 4 files changed, 54 insertions(+), 51 deletions(-)
> 
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index e09c305..0835cf9 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -1217,6 +1217,8 @@ void hci_le_start_enc(struct hci_conn *conn, __le16 ediv, __u8 rand[8],
> 
> u8 bdaddr_to_le(u8 bdaddr_type);
> 
> +int hci_create_le_conn(struct hci_dev *hdev, bdaddr_t *addr, u8 type);
> +
> #define SCO_AIRMODE_MASK       0x0003
> #define SCO_AIRMODE_CVSD       0x0000
> #define SCO_AIRMODE_TRANSP     0x0003
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index 08e601c..cb0e5d7 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -49,25 +49,6 @@ static const struct sco_param sco_param_wideband[] = {
> 	{ EDR_ESCO_MASK | ESCO_EV3,   0x0008 }, /* T1 */
> };
> 
> -static void hci_le_create_connection(struct hci_conn *conn)
> -{
> -	struct hci_dev *hdev = conn->hdev;
> -	struct hci_cp_le_create_conn cp;
> -
> -	memset(&cp, 0, sizeof(cp));
> -	cp.scan_interval = __constant_cpu_to_le16(0x0060);
> -	cp.scan_window = __constant_cpu_to_le16(0x0030);
> -	bacpy(&cp.peer_addr, &conn->dst);
> -	cp.peer_addr_type = conn->dst_type;
> -	cp.conn_interval_min = __constant_cpu_to_le16(0x0028);
> -	cp.conn_interval_max = __constant_cpu_to_le16(0x0038);
> -	cp.supervision_timeout = __constant_cpu_to_le16(0x002a);
> -	cp.min_ce_len = __constant_cpu_to_le16(0x0000);
> -	cp.max_ce_len = __constant_cpu_to_le16(0x0000);
> -
> -	hci_send_cmd(hdev, HCI_OP_LE_CREATE_CONN, sizeof(cp), &cp);
> -}
> -
> static void hci_le_create_connection_cancel(struct hci_conn *conn)
> {
> 	hci_send_cmd(conn->hdev, HCI_OP_LE_CREATE_CONN_CANCEL, 0, NULL);
> @@ -545,6 +526,7 @@ static struct hci_conn *hci_connect_le(struct hci_dev *hdev, bdaddr_t *dst,
> 				    u8 dst_type, u8 sec_level, u8 auth_type)
> {
> 	struct hci_conn *conn;
> +	int err;
> 
> 	if (test_bit(HCI_LE_PERIPHERAL, &hdev->flags))
> 		return ERR_PTR(-ENOTSUPP);
> @@ -565,7 +547,11 @@ static struct hci_conn *hci_connect_le(struct hci_dev *hdev, bdaddr_t *dst,
> 		conn->link_mode |= HCI_LM_MASTER;
> 		conn->sec_level = BT_SECURITY_LOW;
> 
> -		hci_le_create_connection(conn);
> +		err = hci_create_le_conn(hdev, &conn->dst, conn->dst_type);
> +		if (err) {
> +			hci_conn_del(conn);
> +			return ERR_PTR(err);
> +		}

it is safe to sleep here? Since that is what the new call is doing.

> 	}
> 
> 	conn->pending_sec_level = sec_level;
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 82dbdc6..1002510 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -3662,3 +3662,49 @@ u8 bdaddr_to_le(u8 bdaddr_type)
> 		return ADDR_LE_DEV_RANDOM;
> 	}
> }
> +
> +static void create_le_conn_complete(struct hci_dev *hdev, u8 status)
> +{
> +	struct hci_conn *conn;
> +
> +	if (status == 0)
> +		return;
> +
> +	BT_ERR("HCI request failed to initiate LE connection: status 0x%2.2x",
> +	       status);

This should say "create" instead of "initiate".

> +
> +	conn = hci_conn_hash_lookup_state(hdev, LE_LINK, BT_CONNECT);
> +	if (!conn)
> +		return;
> +

We are no longer setting conn->state to BT_CLOSED on purpose here?

> +	mgmt_connect_failed(hdev, &conn->dst, conn->type, conn->dst_type,
> +			    status);
> +
> +	hci_proto_connect_cfm(conn, status);
> +
> +	hci_dev_lock(hdev);
> +	hci_conn_del(conn);
> +	hci_dev_unlock(hdev);
> +}

You are changing the locking behavior without any explanation. Previous code has it around the whole section. I am not a huge fan of changes that get sneaked in this way. That should be a separate patch to explain why it is correct. This patch should not change the current behavior.

> +
> +int hci_create_le_conn(struct hci_dev *hdev, bdaddr_t *addr, u8 type)
> +{
> +	struct hci_cp_le_create_conn cp;
> +	struct hci_request req;
> +
> +	hci_req_init(&req, hdev);
> +
> +	memset(&cp, 0, sizeof(cp));
> +	cp.scan_interval = __constant_cpu_to_le16(0x0060);
> +	cp.scan_window = __constant_cpu_to_le16(0x0030);
> +	bacpy(&cp.peer_addr, addr);
> +	cp.peer_addr_type = type;
> +	cp.conn_interval_min = __constant_cpu_to_le16(0x0028);
> +	cp.conn_interval_max = __constant_cpu_to_le16(0x0038);
> +	cp.supervision_timeout = __constant_cpu_to_le16(0x002a);
> +	cp.min_ce_len = __constant_cpu_to_le16(0x0000);
> +	cp.max_ce_len = __constant_cpu_to_le16(0x0000);
> +	hci_req_add(&req, HCI_OP_LE_CREATE_CONN, sizeof(cp), &cp);
> +
> +	return hci_req_run(&req, create_le_conn_complete);
> +}

Regards

Marcel


^ permalink raw reply

* Re: [PATCH v2 2/4] Bluetooth: Initialize hci_conn fields in hci_connect_le
From: Marcel Holtmann @ 2013-10-04  6:09 UTC (permalink / raw)
  To: Andre Guedes; +Cc: linux-bluetooth
In-Reply-To: <1380835547-3640-3-git-send-email-andre.guedes@openbossa.org>

Hi Andre,

> This patch moves some hci_conn fields initialization from hci_le_
> create_connection() to hci_connect_le(). It makes more sense to
> initialize these fields within the function that creates the hci_
> conn object.
> 
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
> net/bluetooth/hci_conn.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)

patch has been applied to bluetooth-next tree.

Regards

Marcel


^ permalink raw reply

* Re: [PATCH v2 1/4] Bluetooth: Rename hci_conn variable in hci_connect_le()
From: Marcel Holtmann @ 2013-10-04  6:08 UTC (permalink / raw)
  To: Andre Guedes; +Cc: linux-bluetooth
In-Reply-To: <1380835547-3640-2-git-send-email-andre.guedes@openbossa.org>

Hi Andre,

> This patch simply rename the hci_conn variable "le" to "conn"
> since it is a better name.
> 
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
> net/bluetooth/hci_conn.c | 26 +++++++++++++-------------
> 1 file changed, 13 insertions(+), 13 deletions(-)

patch has been applied to bluetooth-next tree.

Regards

Marcel


^ permalink raw reply

* Re: [PATCH] Bluetooth: btmrvl: operate on 16-bit opcodes instead of ogf/ocf
From: Anderson Lizardo @ 2013-10-03 23:26 UTC (permalink / raw)
  To: Bing Zhao
  Cc: BlueZ development, Marcel Holtmann, Gustavo Padovan,
	Johan Hedberg, linux-wireless@vger.kernel.org, Amitkumar Karwar
In-Reply-To: <477F20668A386D41ADCC57781B1F70430F451FB266@SC-VEXCH1.marvell.com>

Hi Bing,

On Thu, Oct 3, 2013 at 5:06 PM, Bing Zhao <bzhao@marvell.com> wrote:
>> >         hdr = (struct hci_command_hdr *)skb_put(skb, HCI_COMMAND_HDR_SIZE);
>> > -       hdr->opcode = cpu_to_le16(hci_opcode_pack(OGF, cmd_no));
>> > +       hdr->opcode = cpu_to_le16(opcode);
>>
>> Are you sure the callers of btmrvl_send_sync_cmd() do not need to be
>> changed to pass an opcode instead of just the OCF?
>
> Previously we pass the cmd_no which is the OCF bits to the function, and the function packs it to opcode.
>
> Now I changed the macros of the cmd_no from OCF to opcode as shown below, so no change to the callers.
>
> -/* Bluetooth commands  */
> -#define BT_CMD_AUTO_SLEEP_MODE         0x23
> -#define BT_CMD_HOST_SLEEP_CONFIG       0x59
> -#define BT_CMD_HOST_SLEEP_ENABLE       0x5A
> -#define BT_CMD_MODULE_CFG_REQ          0x5B
> -#define BT_CMD_LOAD_CONFIG_DATA                0x61
> +/* Vendor specific Bluetooth commands */
> +#define BT_CMD_AUTO_SLEEP_MODE         0xFC23
> +#define BT_CMD_HOST_SLEEP_CONFIG       0xFC59
> +#define BT_CMD_HOST_SLEEP_ENABLE       0xFC5A
> +#define BT_CMD_MODULE_CFG_REQ          0xFC5B
> +#define BT_CMD_LOAD_CONFIG_DATA                0xFC61

Now I got it :)

Best Regards,
-- 
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

^ permalink raw reply

* Re: RE: [PATCH v6 0/4] Bluetooth: btmrvl cal data downloading
From: 이혁주 @ 2013-10-03 22:52 UTC (permalink / raw)
  To: Bing Zhao, Marcel Holtmann, 이원준,
	정종필
  Cc: linux-bluetooth@vger.kernel.org, Gustavo Padovan, Johan Hedberg,
	linux-wireless@vger.kernel.org, Mike Frysinger, Amitkumar Karwar

[-- Attachment #1: Type: text/html, Size: 5289 bytes --]

[-- Attachment #2: 201310040752684_BEI0XT4N.gif --]
[-- Type: image/gif, Size: 14036 bytes --]

^ permalink raw reply

* [PATCH v2 4/4] Bluetooth: Refactor hci_connect_le
From: Andre Guedes @ 2013-10-03 21:25 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1380835547-3640-1-git-send-email-andre.guedes@openbossa.org>

This patch does some code refactoring in hci_connect_le() by moving
the exception code into if statements and letting the main flow in
first level of function scope. It also adds extra comments to improve
the code readability.

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

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index cb0e5d7..ff15841 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -531,34 +531,49 @@ static struct hci_conn *hci_connect_le(struct hci_dev *hdev, bdaddr_t *dst,
 	if (test_bit(HCI_LE_PERIPHERAL, &hdev->flags))
 		return ERR_PTR(-ENOTSUPP);
 
+	/* XXX: Some devices send ATT messages as soon as the physical link
+	 * is established. To be able to handle these ATT messages, the user-
+	 * space first establishes the connection and then starts the pairing
+	 * process.
+	 *
+	 * So if a hci_conn object already exists for the following connection
+	 * attempt, we simply update pending_sec_level and auth_type fields
+	 * and return the object found.
+	 */
 	conn = hci_conn_hash_lookup_ba(hdev, LE_LINK, dst);
-	if (!conn) {
-		conn = hci_conn_hash_lookup_state(hdev, LE_LINK, BT_CONNECT);
-		if (conn)
-			return ERR_PTR(-EBUSY);
-
-		conn = hci_conn_add(hdev, LE_LINK, dst);
-		if (!conn)
-			return ERR_PTR(-ENOMEM);
+	if (conn) {
+		conn->pending_sec_level = sec_level;
+		conn->auth_type = auth_type;
+		goto out;
+	}
 
-		conn->dst_type = bdaddr_to_le(dst_type);
-		conn->state = BT_CONNECT;
-		conn->out = true;
-		conn->link_mode |= HCI_LM_MASTER;
-		conn->sec_level = BT_SECURITY_LOW;
+	/* Since the controller supports only one LE connection attempt at a
+	 * time, we return -EBUSY if there is any connection attempt running.
+	 */
+	conn = hci_conn_hash_lookup_state(hdev, LE_LINK, BT_CONNECT);
+	if (conn)
+		return ERR_PTR(-EBUSY);
 
-		err = hci_create_le_conn(hdev, &conn->dst, conn->dst_type);
-		if (err) {
-			hci_conn_del(conn);
-			return ERR_PTR(err);
-		}
-	}
+	conn = hci_conn_add(hdev, LE_LINK, dst);
+	if (!conn)
+		return ERR_PTR(-ENOMEM);
 
+	conn->dst_type = bdaddr_to_le(dst_type);
+	conn->state = BT_CONNECT;
+	conn->out = true;
+	conn->link_mode |= HCI_LM_MASTER;
+	conn->sec_level = BT_SECURITY_LOW;
 	conn->pending_sec_level = sec_level;
 	conn->auth_type = auth_type;
 
-	hci_conn_hold(conn);
+	err = hci_create_le_conn(hdev, &conn->dst, conn->dst_type);
+	if (err) {
+		hci_conn_del(conn);
+		return ERR_PTR(err);
+	}
 
+out:
+	hci_conn_hold(conn);
 	return conn;
 }
 
-- 
1.8.4


^ permalink raw reply related

* [PATCH v2 3/4] Bluetooth: Use HCI request for LE connection
From: Andre Guedes @ 2013-10-03 21:25 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1380835547-3640-1-git-send-email-andre.guedes@openbossa.org>

This patch introduces a new helper, which uses the HCI request
framework, for creating LE connectons. All the handling is now
done by this function so we can remove the hci_cs_le_create_conn()
event handler.

This patch also removes the old hci_le_create_connection() since
it is not used anymore.

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

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index e09c305..0835cf9 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1217,6 +1217,8 @@ void hci_le_start_enc(struct hci_conn *conn, __le16 ediv, __u8 rand[8],
 
 u8 bdaddr_to_le(u8 bdaddr_type);
 
+int hci_create_le_conn(struct hci_dev *hdev, bdaddr_t *addr, u8 type);
+
 #define SCO_AIRMODE_MASK       0x0003
 #define SCO_AIRMODE_CVSD       0x0000
 #define SCO_AIRMODE_TRANSP     0x0003
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 08e601c..cb0e5d7 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -49,25 +49,6 @@ static const struct sco_param sco_param_wideband[] = {
 	{ EDR_ESCO_MASK | ESCO_EV3,   0x0008 }, /* T1 */
 };
 
-static void hci_le_create_connection(struct hci_conn *conn)
-{
-	struct hci_dev *hdev = conn->hdev;
-	struct hci_cp_le_create_conn cp;
-
-	memset(&cp, 0, sizeof(cp));
-	cp.scan_interval = __constant_cpu_to_le16(0x0060);
-	cp.scan_window = __constant_cpu_to_le16(0x0030);
-	bacpy(&cp.peer_addr, &conn->dst);
-	cp.peer_addr_type = conn->dst_type;
-	cp.conn_interval_min = __constant_cpu_to_le16(0x0028);
-	cp.conn_interval_max = __constant_cpu_to_le16(0x0038);
-	cp.supervision_timeout = __constant_cpu_to_le16(0x002a);
-	cp.min_ce_len = __constant_cpu_to_le16(0x0000);
-	cp.max_ce_len = __constant_cpu_to_le16(0x0000);
-
-	hci_send_cmd(hdev, HCI_OP_LE_CREATE_CONN, sizeof(cp), &cp);
-}
-
 static void hci_le_create_connection_cancel(struct hci_conn *conn)
 {
 	hci_send_cmd(conn->hdev, HCI_OP_LE_CREATE_CONN_CANCEL, 0, NULL);
@@ -545,6 +526,7 @@ static struct hci_conn *hci_connect_le(struct hci_dev *hdev, bdaddr_t *dst,
 				    u8 dst_type, u8 sec_level, u8 auth_type)
 {
 	struct hci_conn *conn;
+	int err;
 
 	if (test_bit(HCI_LE_PERIPHERAL, &hdev->flags))
 		return ERR_PTR(-ENOTSUPP);
@@ -565,7 +547,11 @@ static struct hci_conn *hci_connect_le(struct hci_dev *hdev, bdaddr_t *dst,
 		conn->link_mode |= HCI_LM_MASTER;
 		conn->sec_level = BT_SECURITY_LOW;
 
-		hci_le_create_connection(conn);
+		err = hci_create_le_conn(hdev, &conn->dst, conn->dst_type);
+		if (err) {
+			hci_conn_del(conn);
+			return ERR_PTR(err);
+		}
 	}
 
 	conn->pending_sec_level = sec_level;
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 82dbdc6..1002510 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3662,3 +3662,49 @@ u8 bdaddr_to_le(u8 bdaddr_type)
 		return ADDR_LE_DEV_RANDOM;
 	}
 }
+
+static void create_le_conn_complete(struct hci_dev *hdev, u8 status)
+{
+	struct hci_conn *conn;
+
+	if (status == 0)
+		return;
+
+	BT_ERR("HCI request failed to initiate LE connection: status 0x%2.2x",
+	       status);
+
+	conn = hci_conn_hash_lookup_state(hdev, LE_LINK, BT_CONNECT);
+	if (!conn)
+		return;
+
+	mgmt_connect_failed(hdev, &conn->dst, conn->type, conn->dst_type,
+			    status);
+
+	hci_proto_connect_cfm(conn, status);
+
+	hci_dev_lock(hdev);
+	hci_conn_del(conn);
+	hci_dev_unlock(hdev);
+}
+
+int hci_create_le_conn(struct hci_dev *hdev, bdaddr_t *addr, u8 type)
+{
+	struct hci_cp_le_create_conn cp;
+	struct hci_request req;
+
+	hci_req_init(&req, hdev);
+
+	memset(&cp, 0, sizeof(cp));
+	cp.scan_interval = __constant_cpu_to_le16(0x0060);
+	cp.scan_window = __constant_cpu_to_le16(0x0030);
+	bacpy(&cp.peer_addr, addr);
+	cp.peer_addr_type = type;
+	cp.conn_interval_min = __constant_cpu_to_le16(0x0028);
+	cp.conn_interval_max = __constant_cpu_to_le16(0x0038);
+	cp.supervision_timeout = __constant_cpu_to_le16(0x002a);
+	cp.min_ce_len = __constant_cpu_to_le16(0x0000);
+	cp.max_ce_len = __constant_cpu_to_le16(0x0000);
+	hci_req_add(&req, HCI_OP_LE_CREATE_CONN, sizeof(cp), &cp);
+
+	return hci_req_run(&req, create_le_conn_complete);
+}
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 4785ab0..a6c6c17 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1470,33 +1470,6 @@ static void hci_cs_disconnect(struct hci_dev *hdev, u8 status)
 	hci_dev_unlock(hdev);
 }
 
-static void hci_cs_le_create_conn(struct hci_dev *hdev, __u8 status)
-{
-	struct hci_conn *conn;
-
-	BT_DBG("%s status 0x%2.2x", hdev->name, status);
-
-	if (status) {
-		hci_dev_lock(hdev);
-
-		conn = hci_conn_hash_lookup_state(hdev, LE_LINK, BT_CONNECT);
-		if (!conn) {
-			hci_dev_unlock(hdev);
-			return;
-		}
-
-		BT_DBG("%s bdaddr %pMR conn %p", hdev->name, &conn->dst, conn);
-
-		conn->state = BT_CLOSED;
-		mgmt_connect_failed(hdev, &conn->dst, conn->type,
-				    conn->dst_type, status);
-		hci_proto_connect_cfm(conn, status);
-		hci_conn_del(conn);
-
-		hci_dev_unlock(hdev);
-	}
-}
-
 static void hci_cs_create_phylink(struct hci_dev *hdev, u8 status)
 {
 	struct hci_cp_create_phy_link *cp;
@@ -2347,10 +2320,6 @@ static void hci_cmd_status_evt(struct hci_dev *hdev, struct sk_buff *skb)
 		hci_cs_disconnect(hdev, ev->status);
 		break;
 
-	case HCI_OP_LE_CREATE_CONN:
-		hci_cs_le_create_conn(hdev, ev->status);
-		break;
-
 	case HCI_OP_CREATE_PHY_LINK:
 		hci_cs_create_phylink(hdev, ev->status);
 		break;
-- 
1.8.4


^ permalink raw reply related

* [PATCH v2 2/4] Bluetooth: Initialize hci_conn fields in hci_connect_le
From: Andre Guedes @ 2013-10-03 21:25 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1380835547-3640-1-git-send-email-andre.guedes@openbossa.org>

This patch moves some hci_conn fields initialization from hci_le_
create_connection() to hci_connect_le(). It makes more sense to
initialize these fields within the function that creates the hci_
conn object.

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

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 5aba520..08e601c 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -54,11 +54,6 @@ static void hci_le_create_connection(struct hci_conn *conn)
 	struct hci_dev *hdev = conn->hdev;
 	struct hci_cp_le_create_conn cp;
 
-	conn->state = BT_CONNECT;
-	conn->out = true;
-	conn->link_mode |= HCI_LM_MASTER;
-	conn->sec_level = BT_SECURITY_LOW;
-
 	memset(&cp, 0, sizeof(cp));
 	cp.scan_interval = __constant_cpu_to_le16(0x0060);
 	cp.scan_window = __constant_cpu_to_le16(0x0030);
@@ -565,6 +560,11 @@ static struct hci_conn *hci_connect_le(struct hci_dev *hdev, bdaddr_t *dst,
 			return ERR_PTR(-ENOMEM);
 
 		conn->dst_type = bdaddr_to_le(dst_type);
+		conn->state = BT_CONNECT;
+		conn->out = true;
+		conn->link_mode |= HCI_LM_MASTER;
+		conn->sec_level = BT_SECURITY_LOW;
+
 		hci_le_create_connection(conn);
 	}
 
-- 
1.8.4


^ permalink raw reply related

* [PATCH v2 1/4] Bluetooth: Rename hci_conn variable in hci_connect_le()
From: Andre Guedes @ 2013-10-03 21:25 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1380835547-3640-1-git-send-email-andre.guedes@openbossa.org>

This patch simply rename the hci_conn variable "le" to "conn"
since it is a better name.

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

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 514148b..5aba520 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -549,31 +549,31 @@ EXPORT_SYMBOL(hci_get_route);
 static struct hci_conn *hci_connect_le(struct hci_dev *hdev, bdaddr_t *dst,
 				    u8 dst_type, u8 sec_level, u8 auth_type)
 {
-	struct hci_conn *le;
+	struct hci_conn *conn;
 
 	if (test_bit(HCI_LE_PERIPHERAL, &hdev->flags))
 		return ERR_PTR(-ENOTSUPP);
 
-	le = hci_conn_hash_lookup_ba(hdev, LE_LINK, dst);
-	if (!le) {
-		le = hci_conn_hash_lookup_state(hdev, LE_LINK, BT_CONNECT);
-		if (le)
+	conn = hci_conn_hash_lookup_ba(hdev, LE_LINK, dst);
+	if (!conn) {
+		conn = hci_conn_hash_lookup_state(hdev, LE_LINK, BT_CONNECT);
+		if (conn)
 			return ERR_PTR(-EBUSY);
 
-		le = hci_conn_add(hdev, LE_LINK, dst);
-		if (!le)
+		conn = hci_conn_add(hdev, LE_LINK, dst);
+		if (!conn)
 			return ERR_PTR(-ENOMEM);
 
-		le->dst_type = bdaddr_to_le(dst_type);
-		hci_le_create_connection(le);
+		conn->dst_type = bdaddr_to_le(dst_type);
+		hci_le_create_connection(conn);
 	}
 
-	le->pending_sec_level = sec_level;
-	le->auth_type = auth_type;
+	conn->pending_sec_level = sec_level;
+	conn->auth_type = auth_type;
 
-	hci_conn_hold(le);
+	hci_conn_hold(conn);
 
-	return le;
+	return conn;
 }
 
 static struct hci_conn *hci_connect_acl(struct hci_dev *hdev, bdaddr_t *dst,
-- 
1.8.4


^ permalink raw reply related

* [PATCH v2 0/4] LE connection refactoring and fixes
From: Andre Guedes @ 2013-10-03 21:25 UTC (permalink / raw)
  To: linux-bluetooth

Hi all,

This v2 takes in consideration comments from Marcel and Lizardo. This patchset
is smaller since I squashed 3 patches and removed the last two patches from
the previous patchset.

Regards,

Andre


Andre Guedes (4):
  Bluetooth: Rename hci_conn variable in hci_connect_le()
  Bluetooth: Initialize hci_conn fields in hci_connect_le
  Bluetooth: Use HCI request for LE connection
  Bluetooth: Refactor hci_connect_le

 include/net/bluetooth/hci_core.h |  2 +
 net/bluetooth/hci_conn.c         | 81 ++++++++++++++++++++--------------------
 net/bluetooth/hci_core.c         | 46 +++++++++++++++++++++++
 net/bluetooth/hci_event.c        | 31 ---------------
 4 files changed, 89 insertions(+), 71 deletions(-)

-- 
1.8.4


^ permalink raw reply

* [PATCH v2] Bluetooth: btmrvl: operate on 16-bit opcodes instead of ogf/ocf
From: Bing Zhao @ 2013-10-03 21:19 UTC (permalink / raw)
  To: linux-bluetooth
  Cc: Marcel Holtmann, Gustavo Padovan, Johan Hedberg, Anderson Lizardo,
	linux-wireless, Amitkumar Karwar, Bing Zhao

Replace ogf/ocf and its packing with 16-bit opcodes.

Signed-off-by: Bing Zhao <bzhao@marvell.com>
Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
---
v2: Use hci_opcode_ogf() to make code more readable. (Anderson Lizardo)

 drivers/bluetooth/btmrvl_drv.h  | 19 +++++++++++--------
 drivers/bluetooth/btmrvl_main.c | 21 +++++++++------------
 2 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/drivers/bluetooth/btmrvl_drv.h b/drivers/bluetooth/btmrvl_drv.h
index f9d1833..e3b49c6 100644
--- a/drivers/bluetooth/btmrvl_drv.h
+++ b/drivers/bluetooth/btmrvl_drv.h
@@ -90,12 +90,12 @@ struct btmrvl_private {
 
 #define MRVL_VENDOR_PKT			0xFE
 
-/* Bluetooth commands  */
-#define BT_CMD_AUTO_SLEEP_MODE		0x23
-#define BT_CMD_HOST_SLEEP_CONFIG	0x59
-#define BT_CMD_HOST_SLEEP_ENABLE	0x5A
-#define BT_CMD_MODULE_CFG_REQ		0x5B
-#define BT_CMD_LOAD_CONFIG_DATA		0x61
+/* Vendor specific Bluetooth commands */
+#define BT_CMD_AUTO_SLEEP_MODE		0xFC23
+#define BT_CMD_HOST_SLEEP_CONFIG	0xFC59
+#define BT_CMD_HOST_SLEEP_ENABLE	0xFC5A
+#define BT_CMD_MODULE_CFG_REQ		0xFC5B
+#define BT_CMD_LOAD_CONFIG_DATA		0xFC61
 
 /* Sub-commands: Module Bringup/Shutdown Request/Response */
 #define MODULE_BRINGUP_REQ		0xF1
@@ -104,6 +104,11 @@ struct btmrvl_private {
 
 #define MODULE_SHUTDOWN_REQ		0xF2
 
+/* Vendor specific Bluetooth events */
+#define BT_EVENT_AUTO_SLEEP_MODE	0x23
+#define BT_EVENT_HOST_SLEEP_CONFIG	0x59
+#define BT_EVENT_HOST_SLEEP_ENABLE	0x5A
+#define BT_EVENT_MODULE_CFG_REQ		0x5B
 #define BT_EVENT_POWER_STATE		0x20
 
 /* Bluetooth Power States */
@@ -111,8 +116,6 @@ struct btmrvl_private {
 #define BT_PS_DISABLE			0x03
 #define BT_PS_SLEEP			0x01
 
-#define OGF				0x3F
-
 /* Host Sleep states */
 #define HS_ACTIVATED			0x01
 #define HS_DEACTIVATED			0x00
diff --git a/drivers/bluetooth/btmrvl_main.c b/drivers/bluetooth/btmrvl_main.c
index 6e7bd4e..ffec74e 100644
--- a/drivers/bluetooth/btmrvl_main.c
+++ b/drivers/bluetooth/btmrvl_main.c
@@ -50,12 +50,10 @@ bool btmrvl_check_evtpkt(struct btmrvl_private *priv, struct sk_buff *skb)
 
 	if (hdr->evt == HCI_EV_CMD_COMPLETE) {
 		struct hci_ev_cmd_complete *ec;
-		u16 opcode, ocf, ogf;
+		u16 opcode;
 
 		ec = (void *) (skb->data + HCI_EVENT_HDR_SIZE);
 		opcode = __le16_to_cpu(ec->opcode);
-		ocf = hci_opcode_ocf(opcode);
-		ogf = hci_opcode_ogf(opcode);
 
 		if (priv->btmrvl_dev.sendcmdflag) {
 			priv->btmrvl_dev.sendcmdflag = false;
@@ -63,9 +61,8 @@ bool btmrvl_check_evtpkt(struct btmrvl_private *priv, struct sk_buff *skb)
 			wake_up_interruptible(&priv->adapter->cmd_wait_q);
 		}
 
-		if (ogf == OGF) {
-			BT_DBG("vendor event skipped: ogf 0x%4.4x ocf 0x%4.4x",
-			       ogf, ocf);
+		if (hci_opcode_ogf(opcode) == 0x3F) {
+			BT_DBG("vendor event skipped: opcode=%#4.4x", opcode);
 			kfree_skb(skb);
 			return false;
 		}
@@ -89,7 +86,7 @@ int btmrvl_process_event(struct btmrvl_private *priv, struct sk_buff *skb)
 	}
 
 	switch (event->data[0]) {
-	case BT_CMD_AUTO_SLEEP_MODE:
+	case BT_EVENT_AUTO_SLEEP_MODE:
 		if (!event->data[2]) {
 			if (event->data[1] == BT_PS_ENABLE)
 				adapter->psmode = 1;
@@ -102,7 +99,7 @@ int btmrvl_process_event(struct btmrvl_private *priv, struct sk_buff *skb)
 		}
 		break;
 
-	case BT_CMD_HOST_SLEEP_CONFIG:
+	case BT_EVENT_HOST_SLEEP_CONFIG:
 		if (!event->data[3])
 			BT_DBG("gpio=%x, gap=%x", event->data[1],
 							event->data[2]);
@@ -110,7 +107,7 @@ int btmrvl_process_event(struct btmrvl_private *priv, struct sk_buff *skb)
 			BT_DBG("HSCFG command failed");
 		break;
 
-	case BT_CMD_HOST_SLEEP_ENABLE:
+	case BT_EVENT_HOST_SLEEP_ENABLE:
 		if (!event->data[1]) {
 			adapter->hs_state = HS_ACTIVATED;
 			if (adapter->psmode)
@@ -121,7 +118,7 @@ int btmrvl_process_event(struct btmrvl_private *priv, struct sk_buff *skb)
 		}
 		break;
 
-	case BT_CMD_MODULE_CFG_REQ:
+	case BT_EVENT_MODULE_CFG_REQ:
 		if (priv->btmrvl_dev.sendcmdflag &&
 				event->data[1] == MODULE_BRINGUP_REQ) {
 			BT_DBG("EVENT:%s",
@@ -166,7 +163,7 @@ exit:
 }
 EXPORT_SYMBOL_GPL(btmrvl_process_event);
 
-static int btmrvl_send_sync_cmd(struct btmrvl_private *priv, u16 cmd_no,
+static int btmrvl_send_sync_cmd(struct btmrvl_private *priv, u16 opcode,
 				const void *param, u8 len)
 {
 	struct sk_buff *skb;
@@ -179,7 +176,7 @@ static int btmrvl_send_sync_cmd(struct btmrvl_private *priv, u16 cmd_no,
 	}
 
 	hdr = (struct hci_command_hdr *)skb_put(skb, HCI_COMMAND_HDR_SIZE);
-	hdr->opcode = cpu_to_le16(hci_opcode_pack(OGF, cmd_no));
+	hdr->opcode = cpu_to_le16(opcode);
 	hdr->plen = len;
 
 	if (len)
-- 
1.8.0

^ 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