* [PATCH 7/7] obexd: Prefix folders in event reports with leading slash
From: Christian Fetzer @ 2013-09-24 14:16 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1380032167-6440-1-git-send-email-christian.fetzer@oss.bmw-carit.de>
From: Christian Fetzer <christian.fetzer@bmw-carit.de>
The internal representation of folder paths starts with a leading slash.
Therefore we have to prepend it to the folders received in event reports.
---
obexd/client/mns.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/obexd/client/mns.c b/obexd/client/mns.c
index 76a5e24..ca44e5f 100644
--- a/obexd/client/mns.c
+++ b/obexd/client/mns.c
@@ -188,13 +188,25 @@ static void parse_event_report_handle(struct map_event *event,
static void parse_event_report_folder(struct map_event *event,
const char *value)
{
- event->folder = g_strdup(value);
+ if (!value)
+ return;
+
+ if (g_str_has_prefix(value, "/"))
+ event->folder = g_strdup(value);
+ else
+ event->folder = g_strconcat("/", value, NULL);
}
static void parse_event_report_old_folder(struct map_event *event,
const char *value)
{
- event->old_folder = g_strdup(value);
+ if (!value)
+ return;
+
+ if (g_str_has_prefix(value, "/"))
+ event->old_folder = g_strdup(value);
+ else
+ event->old_folder = g_strconcat("/", value, NULL);
}
static void parse_event_report_msg_type(struct map_event *event,
--
1.8.3.4
^ permalink raw reply related
* [PATCH BlueZ 1/2] gobex: Fix crash on g_obex_pending_req_abort
From: Luiz Augusto von Dentz @ 2013-09-24 14:47 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Jaganath Kanakkassery
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
It is not safe to call g_obex_pending_req_abort directly as pending_req
can be NULL:
Invalid read of size 4
at 0x41231E: g_obex_pending_req_abort (gobex.c:693)
by 0x416A8A: g_obex_cancel_transfer (gobex-transfer.c:647)
by 0x42DEF2: obc_transfer_cancel (transfer.c:180)
by 0x43D833: process_message.isra.5 (object.c:259)
by 0x3B0701CE85: ??? (in /usr/lib64/libdbus-1.so.3.7.4)
by 0x3B0700FA30: dbus_connection_dispatch (in /usr/lib64/libdbus-1.so.3.7.4)
by 0x43A5B7: message_dispatch (mainloop.c:76)
by 0x3B03C48962: ??? (in /usr/lib64/libglib-2.0.so.0.3600.3)
by 0x3B03C47E05: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.3600.3)
by 0x3B03C48157: ??? (in /usr/lib64/libglib-2.0.so.0.3600.3)
by 0x3B03C48559: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.3600.3)
by 0x40D53C: main (main.c:319)
Address 0x30 is not stack'd, malloc'd or (recently) free'd
---
gobex/gobex-transfer.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/gobex/gobex-transfer.c b/gobex/gobex-transfer.c
index 4203fec..b815d60 100644
--- a/gobex/gobex-transfer.c
+++ b/gobex/gobex-transfer.c
@@ -644,7 +644,10 @@ gboolean g_obex_cancel_transfer(guint id, GObexFunc complete_func,
transfer->complete_func = complete_func;
transfer->user_data = user_data;
- ret = g_obex_pending_req_abort(transfer->obex, NULL);
+ if (transfer->req_id == 0)
+ goto done;
+
+ ret = g_obex_cancel_req(transfer->obex, transfer->req_id, FALSE);
if (ret)
return TRUE;
--
1.8.3.1
^ permalink raw reply related
* [PATCH BlueZ 2/2] gobex: Revert g_obex_pending_req_abort to static pending_req_abort
From: Luiz Augusto von Dentz @ 2013-09-24 14:47 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Jaganath Kanakkassery
In-Reply-To: <1380034036-25143-1-git-send-email-luiz.dentz@gmail.com>
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
This reverts the changes introduced in
9095deb82572112fc0870095bf2222964610eafe that made pending_req_abort
public which is not necessary considering g_obex_cancel_req can do the
same and is safe to call even if the request is not pending.
---
gobex/gobex.c | 4 ++--
gobex/gobex.h | 2 --
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/gobex/gobex.c b/gobex/gobex.c
index deeab40..8c08b1e 100644
--- a/gobex/gobex.c
+++ b/gobex/gobex.c
@@ -685,7 +685,7 @@ static int pending_pkt_cmp(gconstpointer a, gconstpointer b)
return (p->id - id);
}
-gboolean g_obex_pending_req_abort(GObex *obex, GError **err)
+static gboolean pending_req_abort(GObex *obex, GError **err)
{
struct pending_pkt *p = obex->pending_req;
GObexPacket *req;
@@ -729,7 +729,7 @@ gboolean g_obex_cancel_req(GObex *obex, guint req_id, gboolean remove_callback)
struct pending_pkt *p;
if (obex->pending_req && obex->pending_req->id == req_id) {
- if (!g_obex_pending_req_abort(obex, NULL)) {
+ if (!pending_req_abort(obex, NULL)) {
p = obex->pending_req;
obex->pending_req = NULL;
goto immediate_completion;
diff --git a/gobex/gobex.h b/gobex/gobex.h
index 3ac7b13..76a224e 100644
--- a/gobex/gobex.h
+++ b/gobex/gobex.h
@@ -50,8 +50,6 @@ guint g_obex_send_req(GObex *obex, GObexPacket *req, int timeout,
gboolean g_obex_cancel_req(GObex *obex, guint req_id,
gboolean remove_callback);
-gboolean g_obex_pending_req_abort(GObex *obex, GError **err);
-
gboolean g_obex_send_rsp(GObex *obex, guint8 rspcode, GError **err,
guint8 first_hdr_type, ...);
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH 8/8] Bluetooth: Add new mgmt_set_advertising command
From: Johan Hedberg @ 2013-09-24 15:42 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1380031363-1266-9-git-send-email-johan.hedberg@gmail.com>
Hi,
On Tue, Sep 24, 2013, johan.hedberg@gmail.com wrote:
> This patch adds a new mgmt command for enabling and disabling
> LE advertising.
>
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> include/net/bluetooth/mgmt.h | 2 +
> net/bluetooth/mgmt.c | 99 +++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 100 insertions(+), 1 deletion(-)
Apologies for this really short commit message. I was going to fill this
in before sending but seems I forgot. I'll fix this for the next
revision but I'll first wait at least for some feedback before that.
Johan
^ permalink raw reply
* Re: [PATCH 7/8] Bluetooth: Add new mgmt setting for LE advertising
From: Anderson Lizardo @ 2013-09-24 15:45 UTC (permalink / raw)
To: Johan Hedberg; +Cc: BlueZ development
In-Reply-To: <1380031363-1266-8-git-send-email-johan.hedberg@gmail.com>
Hi Johan,
On Tue, Sep 24, 2013 at 10:02 AM, <johan.hedberg@gmail.com> wrote:
> From: Johan Hedberg <johan.hedberg@intel.com>
>
> This patch adds a new mgmt setting for LE advertising and hooks up the
> necessary places in the mgmt code to operate on the HCI_LE_PERIPHERAL
> flag (which corresponds to this setting). This patch does not yet add
> any new command for enabling the setting - that is left for a subsequent
> patch.
How this code behaves if we enable/disable LE advertising using
hciconfig hci0 leadv/noleadv? IIRC the LE_SET_ADV_ENABLE command will
fail if advertising is already set on the controller.
Best Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil
^ permalink raw reply
* [PATCH v3 1/3] build: Add skeleton for BlueZ Android
From: Frederic Danis @ 2013-09-24 16:11 UTC (permalink / raw)
To: linux-bluetooth
---
Makefile.am | 4 +++-
Makefile.android | 4 ++++
android/Android.mk | 5 +++++
bootstrap-configure | 3 ++-
configure.ac | 4 ++++
5 files changed, 18 insertions(+), 2 deletions(-)
create mode 100644 Makefile.android
create mode 100644 android/Android.mk
diff --git a/Makefile.am b/Makefile.am
index 4e4b1c5..51204f4 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -179,6 +179,7 @@ test_scripts =
include Makefile.tools
include Makefile.obexd
+include Makefile.android
if HID2HCI
rulesdir = @UDEV_DIR@/rules.d
@@ -293,7 +294,8 @@ pkgconfig_DATA = lib/bluez.pc
endif
DISTCHECK_CONFIGURE_FLAGS = --disable-datafiles --enable-library \
- --disable-systemd --disable-udev
+ --disable-systemd --disable-udev \
+ --enable-android
DISTCLEANFILES = $(pkgconfig_DATA)
diff --git a/Makefile.android b/Makefile.android
new file mode 100644
index 0000000..5e43730
--- /dev/null
+++ b/Makefile.android
@@ -0,0 +1,4 @@
+
+if ANDROID
+EXTRA_DIST += android/Android.mk
+endif
diff --git a/android/Android.mk b/android/Android.mk
new file mode 100644
index 0000000..31e2c1e
--- /dev/null
+++ b/android/Android.mk
@@ -0,0 +1,5 @@
+LOCAL_PATH := $(call my-dir)
+
+# Retrieve BlueZ version from configure.ac file
+BLUEZ_VERSION := $(shell grep ^AC_INIT $(LOCAL_PATH)/../configure.ac | cpp -P -D'AC_INIT(_,v)=v')
+
diff --git a/bootstrap-configure b/bootstrap-configure
index 7a6e7d1..8bde920 100755
--- a/bootstrap-configure
+++ b/bootstrap-configure
@@ -12,4 +12,5 @@ fi
--sysconfdir=/etc \
--localstatedir=/var \
--enable-experimental \
- --disable-datafiles $*
+ --disable-datafiles \
+ --enable-android $*
diff --git a/configure.ac b/configure.ac
index 41c2935..22ab240 100644
--- a/configure.ac
+++ b/configure.ac
@@ -242,4 +242,8 @@ AC_DEFINE_UNQUOTED(CONFIGDIR, "${configdir}",
[Directory for the configuration files])
AC_SUBST(CONFIGDIR, "${configdir}")
+AC_ARG_ENABLE(android, AC_HELP_STRING([--enable-android],
+ [enable BlueZ Android]), [android=${enableval}])
+AM_CONDITIONAL(ANDROID, test "${android}" = "yes")
+
AC_OUTPUT(Makefile src/bluetoothd.8 lib/bluez.pc)
--
1.7.9.5
^ permalink raw reply related
* [PATCH v3 2/3] android: Add skeleton of BlueZ Android daemon
From: Frederic Danis @ 2013-09-24 16:11 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1380039070-825-1-git-send-email-frederic.danis@linux.intel.com>
Define local mapping to glib path, otherwise this has to be inside central
place in the build repository.
Retrieve Bluetooth version from configure.ac.
---
.gitignore | 2 +
Makefile.android | 5 +++
android/Android.mk | 24 ++++++++++++
android/main.c | 110 ++++++++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 141 insertions(+)
create mode 100644 android/main.c
diff --git a/.gitignore b/.gitignore
index 8a25a3e..3707209 100644
--- a/.gitignore
+++ b/.gitignore
@@ -98,3 +98,5 @@ unit/test-gobex-packet
unit/test-gobex-transfer
unit/test-*.log
unit/test-*.trs
+
+android/bluetoothd
diff --git a/Makefile.android b/Makefile.android
index 5e43730..e056dce 100644
--- a/Makefile.android
+++ b/Makefile.android
@@ -1,4 +1,9 @@
if ANDROID
+noinst_PROGRAMS += android/bluetoothd
+
+android_bluetoothd_SOURCES = android/main.c
+android_bluetoothd_LDADD = @GLIB_LIBS@
+
EXTRA_DIST += android/Android.mk
endif
diff --git a/android/Android.mk b/android/Android.mk
index 31e2c1e..fc3d6c2 100644
--- a/android/Android.mk
+++ b/android/Android.mk
@@ -3,3 +3,27 @@ LOCAL_PATH := $(call my-dir)
# Retrieve BlueZ version from configure.ac file
BLUEZ_VERSION := $(shell grep ^AC_INIT $(LOCAL_PATH)/../configure.ac | cpp -P -D'AC_INIT(_,v)=v')
+# Specify pathmap for glib
+pathmap_INCL += glib:external/bluetooth/glib
+
+#
+# Android BlueZ daemon (bluetoothd)
+#
+
+include $(CLEAR_VARS)
+
+LOCAL_SRC_FILES := \
+ main.c \
+
+LOCAL_C_INCLUDES := \
+ $(call include-path-for, glib) \
+ $(call include-path-for, glib)/glib \
+
+LOCAL_CFLAGS := -DVERSION=\"$(BLUEZ_VERSION)\"
+
+LOCAL_SHARED_LIBRARIES := \
+ libglib \
+
+LOCAL_MODULE := bluetoothd
+
+include $(BUILD_EXECUTABLE)
diff --git a/android/main.c b/android/main.c
new file mode 100644
index 0000000..1dba2d4
--- /dev/null
+++ b/android/main.c
@@ -0,0 +1,110 @@
+/*
+ *
+ * BlueZ - Bluetooth protocol stack for Linux
+ *
+ * Copyright (C) 2013 Intel Corporation. All rights reserved.
+ *
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <signal.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdbool.h>
+#include <string.h>
+
+#include <glib.h>
+
+#define SHUTDOWN_GRACE_SECONDS 10
+
+static GMainLoop *event_loop;
+
+static gboolean quit_eventloop(gpointer user_data)
+{
+ g_main_loop_quit(event_loop);
+
+ return FALSE;
+}
+
+static void sig_term(int sig)
+{
+ static bool __terminated = false;
+
+ if (!__terminated) {
+ g_timeout_add_seconds(SHUTDOWN_GRACE_SECONDS,
+ quit_eventloop, NULL);
+ }
+
+ __terminated = true;
+}
+
+static gboolean option_detach = TRUE;
+static gboolean option_version = FALSE;
+
+static GOptionEntry options[] = {
+ { "nodetach", 'n', G_OPTION_FLAG_REVERSE,
+ G_OPTION_ARG_NONE, &option_detach,
+ "Run with logging in foreground", NULL },
+ { "version", 'v', 0, G_OPTION_ARG_NONE, &option_version,
+ "Show version information and exit", NULL },
+ { NULL }
+};
+
+int main(int argc, char *argv[])
+{
+ GOptionContext *context;
+ GError *err = NULL;
+ struct sigaction sa;
+
+ context = g_option_context_new(NULL);
+ g_option_context_add_main_entries(context, options, NULL);
+
+ if (g_option_context_parse(context, &argc, &argv, &err) == FALSE) {
+ if (err != NULL) {
+ g_printerr("%s\n", err->message);
+ g_error_free(err);
+ } else
+ g_printerr("An unknown error occurred\n");
+
+ exit(EXIT_FAILURE);
+ }
+
+ g_option_context_free(context);
+
+ if (option_version == TRUE) {
+ printf("%s\n", VERSION);
+ exit(EXIT_SUCCESS);
+ }
+
+ event_loop = g_main_loop_new(NULL, FALSE);
+
+ memset(&sa, 0, sizeof(sa));
+ sa.sa_handler = sig_term;
+ sigaction(SIGINT, &sa, NULL);
+ sigaction(SIGTERM, &sa, NULL);
+
+ g_main_loop_run(event_loop);
+
+ g_main_loop_unref(event_loop);
+
+ return EXIT_SUCCESS;
+}
--
1.7.9.5
^ permalink raw reply related
* [PATCH v3 3/3] android: Android version of log.c
From: Frederic Danis @ 2013-09-24 16:11 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1380039070-825-1-git-send-email-frederic.danis@linux.intel.com>
Add logging system to BlueZ Android daemon.
Android build will use android/log.c file while autotools build will use
src/log.c instead.
---
Makefile.android | 4 +-
android/Android.mk | 1 +
android/log.c | 172 ++++++++++++++++++++++++++++++++++++++++++++++++++++
android/main.c | 34 +++++++++++
4 files changed, 209 insertions(+), 2 deletions(-)
create mode 100644 android/log.c
diff --git a/Makefile.android b/Makefile.android
index e056dce..1184e5f 100644
--- a/Makefile.android
+++ b/Makefile.android
@@ -2,8 +2,8 @@
if ANDROID
noinst_PROGRAMS += android/bluetoothd
-android_bluetoothd_SOURCES = android/main.c
+android_bluetoothd_SOURCES = android/main.c src/log.c
android_bluetoothd_LDADD = @GLIB_LIBS@
-EXTRA_DIST += android/Android.mk
+EXTRA_DIST += android/Android.mk android/log.c
endif
diff --git a/android/Android.mk b/android/Android.mk
index fc3d6c2..e3f0fd7 100644
--- a/android/Android.mk
+++ b/android/Android.mk
@@ -14,6 +14,7 @@ include $(CLEAR_VARS)
LOCAL_SRC_FILES := \
main.c \
+ log.c \
LOCAL_C_INCLUDES := \
$(call include-path-for, glib) \
diff --git a/android/log.c b/android/log.c
new file mode 100644
index 0000000..908f883
--- /dev/null
+++ b/android/log.c
@@ -0,0 +1,172 @@
+/*
+ *
+ * BlueZ - Bluetooth protocol stack for Linux
+ *
+ * Copyright (C) 2013 Intel Corporation. All rights reserved.
+ *
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdarg.h>
+#include <unistd.h>
+#include <sys/uio.h>
+
+#include <glib.h>
+
+#include "log.h"
+
+#define LOG_DEBUG 3
+#define LOG_INFO 4
+#define LOG_WARN 5
+#define LOG_ERR 6
+
+static const char tag[] = "BlueZ";
+static int system_fd;
+
+static void android_log(int pri, const char *fmt, va_list ap)
+{
+ char *msg;
+ struct iovec vec[3];
+
+ if (system_fd == -1)
+ return;
+
+ msg = g_strdup_vprintf(fmt, ap);
+
+ vec[0].iov_base = (unsigned char *) &pri;
+ vec[0].iov_len = 1;
+ vec[1].iov_base = (void *) tag;
+ vec[1].iov_len = strlen(tag) + 1;
+ vec[2].iov_base = (void *) msg;
+ vec[2].iov_len = strlen(msg) + 1;
+
+ writev(system_fd, vec, 3);
+
+ g_free(msg);
+}
+
+void info(const char *format, ...)
+{
+ va_list ap;
+
+ va_start(ap, format);
+
+ android_log(LOG_INFO, format, ap);
+
+ va_end(ap);
+}
+
+void warn(const char *format, ...)
+{
+ va_list ap;
+
+ va_start(ap, format);
+
+ android_log(LOG_WARN, format, ap);
+
+ va_end(ap);
+}
+
+void error(const char *format, ...)
+{
+ va_list ap;
+
+ va_start(ap, format);
+
+ android_log(LOG_ERR, format, ap);
+
+ va_end(ap);
+}
+
+void btd_debug(const char *format, ...)
+{
+ va_list ap;
+
+ va_start(ap, format);
+
+ android_log(LOG_DEBUG, format, ap);
+
+ va_end(ap);
+}
+
+extern struct btd_debug_desc __start___debug[];
+extern struct btd_debug_desc __stop___debug[];
+
+static char **enabled = NULL;
+
+static gboolean is_enabled(struct btd_debug_desc *desc)
+{
+ int i;
+
+ if (enabled == NULL)
+ return 0;
+
+ for (i = 0; enabled[i] != NULL; i++)
+ if (desc->file != NULL && g_pattern_match_simple(enabled[i],
+ desc->file) == TRUE)
+ return 1;
+
+ return 0;
+}
+
+void __btd_enable_debug(struct btd_debug_desc *start,
+ struct btd_debug_desc *stop)
+{
+ struct btd_debug_desc *desc;
+
+ if (start == NULL || stop == NULL)
+ return;
+
+ for (desc = start; desc < stop; desc++) {
+ if (is_enabled(desc))
+ desc->flags |= BTD_DEBUG_FLAG_PRINT;
+ }
+}
+
+void __btd_toggle_debug(void)
+{
+ struct btd_debug_desc *desc;
+
+ for (desc = __start___debug; desc < __stop___debug; desc++)
+ desc->flags |= BTD_DEBUG_FLAG_PRINT;
+}
+
+void __btd_log_init(const char *debug, int detach)
+{
+ if (debug != NULL)
+ enabled = g_strsplit_set(debug, ":, ", 0);
+
+ __btd_enable_debug(__start___debug, __stop___debug);
+
+ system_fd = open("/dev/log/system", O_WRONLY);
+
+ info("Bluetooth daemon %s", VERSION);
+}
+
+void __btd_log_cleanup(void)
+{
+ close(system_fd);
+ system_fd = -1;
+
+ g_strfreev(enabled);
+}
diff --git a/android/main.c b/android/main.c
index 1dba2d4..c0a56b1 100644
--- a/android/main.c
+++ b/android/main.c
@@ -34,6 +34,8 @@
#include <glib.h>
+#include "log.h"
+
#define SHUTDOWN_GRACE_SECONDS 10
static GMainLoop *event_loop;
@@ -57,10 +59,31 @@ static void sig_term(int sig)
__terminated = true;
}
+static char *option_debug = NULL;
static gboolean option_detach = TRUE;
static gboolean option_version = FALSE;
+static void free_options(void)
+{
+ g_free(option_debug);
+ option_debug = NULL;
+}
+
+static gboolean parse_debug(const char *key, const char *value,
+ gpointer user_data, GError **error)
+{
+ if (value)
+ option_debug = g_strdup(value);
+ else
+ option_debug = g_strdup("*");
+
+ return TRUE;
+}
+
static GOptionEntry options[] = {
+ { "debug", 'd', G_OPTION_FLAG_OPTIONAL_ARG,
+ G_OPTION_ARG_CALLBACK, parse_debug,
+ "Specify debug options to enable", "DEBUG" },
{ "nodetach", 'n', G_OPTION_FLAG_REVERSE,
G_OPTION_ARG_NONE, &option_detach,
"Run with logging in foreground", NULL },
@@ -102,9 +125,20 @@ int main(int argc, char *argv[])
sigaction(SIGINT, &sa, NULL);
sigaction(SIGTERM, &sa, NULL);
+ __btd_log_init(option_debug, option_detach);
+
+ /* no need to keep parsed option in memory */
+ free_options();
+
+ DBG("Entering main loop");
+
g_main_loop_run(event_loop);
g_main_loop_unref(event_loop);
+ info("Exit");
+
+ __btd_log_cleanup();
+
return EXIT_SUCCESS;
}
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH 7/8] Bluetooth: Add new mgmt setting for LE advertising
From: Johan Hedberg @ 2013-09-24 17:21 UTC (permalink / raw)
To: Anderson Lizardo; +Cc: BlueZ development
In-Reply-To: <CAJdJm_MDoR2ed6++x5JAxaG2=0+zUdAGztL_L3i-ObtaM+WbRA@mail.gmail.com>
Hi Lizardo,
On Tue, Sep 24, 2013, Anderson Lizardo wrote:
> On Tue, Sep 24, 2013 at 10:02 AM, <johan.hedberg@gmail.com> wrote:
> > This patch adds a new mgmt setting for LE advertising and hooks up the
> > necessary places in the mgmt code to operate on the HCI_LE_PERIPHERAL
> > flag (which corresponds to this setting). This patch does not yet add
> > any new command for enabling the setting - that is left for a subsequent
> > patch.
>
> How this code behaves if we enable/disable LE advertising using
> hciconfig hci0 leadv/noleadv? IIRC the LE_SET_ADV_ENABLE command will
> fail if advertising is already set on the controller.
You're right that a mix of mgmt and hciconfig will mix things up on the
kernel side. This is something I was aware of but didn't investigate
much further since I was assuming it would add too much complexity to
the code. The principle has always been that we keep compatibility/good
behavior with mixed mgmg/raw HCI access only as long as it doesn't
needlessly complicate the code.
That said, I'll take a another look if the flag setting could be moved
to a hci_event.c handler from the request callback without requiring the
addition of a second flag or state variable. This issue is not unique to
this new setting but actually exists for many of them. What we probably
need is a generic mgmt_send_new_settings function that hci_event.c
handlers can call when they know that new_settings should be emitted. I
suspect that might solve the issue.
Johan
^ permalink raw reply
* Re: [PATCH 5/8] Bluetooth: Move mgmt response convenience functions to a better location
From: Marcel Holtmann @ 2013-09-24 18:11 UTC (permalink / raw)
To: johan.hedberg; +Cc: linux-bluetooth
In-Reply-To: <1380031363-1266-6-git-send-email-johan.hedberg@gmail.com>
Hi Johan,
> The settings_rsp and cmd_status_rsp functions can be useful for all mgmt
> command handlers when asynchronous request callbacks are used. They will
> e.g. be used by subsequent patches to change set_le to use an async
> request as well as a new set_advertising command. Therefore, move them
> higher up in the mgmt.c file to avoid unnecessary forward declarations
> or mixing this trivial change with other patches.
>
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> net/bluetooth/mgmt.c | 60 ++++++++++++++++++++++++++--------------------------
> 1 file changed, 30 insertions(+), 30 deletions(-)
Acked-by: Marcel Holtmann <marcel@holtmann.org>
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 1/8] Bluetooth: Add clarifying comment to bt_sock_wait_state()
From: Marcel Holtmann @ 2013-09-24 18:11 UTC (permalink / raw)
To: johan.hedberg; +Cc: linux-bluetooth
In-Reply-To: <1380031363-1266-2-git-send-email-johan.hedberg@gmail.com>
Hi Johan,
> The bt_sock_wait_state requires the sk lock to be held (through
> lock_sock) so document it clearly in the code.
>
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> net/bluetooth/af_bluetooth.c | 1 +
> 1 file changed, 1 insertion(+)
Acked-by: Marcel Holtmann <marcel@holtmann.org>
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 2/8] Bluetooth: Clean up socket locking in l2cap_sock_recvmsg
From: Marcel Holtmann @ 2013-09-24 18:16 UTC (permalink / raw)
To: johan.hedberg; +Cc: linux-bluetooth
In-Reply-To: <1380031363-1266-3-git-send-email-johan.hedberg@gmail.com>
Hi Johan,
> This patch cleans up the locking login in l2cap_sock_recvmsg by pairing
> up each lock_sock call with a release_sock call. The function already
> has a "done" label that handles releasing the socket and returning from
> the function so the fix is rather simple.
>
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> net/bluetooth/l2cap_sock.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> index ad95b42..5853c1e 100644
> --- a/net/bluetooth/l2cap_sock.c
> +++ b/net/bluetooth/l2cap_sock.c
> @@ -795,7 +795,7 @@ static int l2cap_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
> {
> struct sock *sk = sock->sk;
> struct l2cap_pinfo *pi = l2cap_pi(sk);
> - int err;
> + int err = 0;
>
> lock_sock(sk);
>
> @@ -805,8 +805,7 @@ static int l2cap_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
> pi->chan->state = BT_CONFIG;
>
> __l2cap_connect_rsp_defer(pi->chan);
> - release_sock(sk);
> - return 0;
can we just do
err = 0;
here instead of declaring it global.
> + goto done;
> }
>
> release_sock(sk);
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 4/8] Bluetooth: Fix busy return for mgmt_set_powered in some cases
From: Marcel Holtmann @ 2013-09-24 18:17 UTC (permalink / raw)
To: johan.hedberg; +Cc: linux-bluetooth
In-Reply-To: <1380031363-1266-5-git-send-email-johan.hedberg@gmail.com>
Hi Johan,
> We should return a "busy" error always when there is another
> mgmt_set_powered operation in progress. Previously when powering on
> while the auto off timer was still set the code could have let two or
> more pending power on commands to be queued. This patch fixes the issue
> by moving the check for duplicate commands to an earlier point in the
> set_powered handler.
>
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> net/bluetooth/mgmt.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
Acked-by: Marcel Holtmann <marcel@holtmann.org>
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 3/8] Bluetooth: Test for HCI_SETUP and HCI_USER_CHANNEL in mgmt_valid_hdev()
From: Marcel Holtmann @ 2013-09-24 18:19 UTC (permalink / raw)
To: johan.hedberg; +Cc: linux-bluetooth
In-Reply-To: <1380031363-1266-4-git-send-email-johan.hedberg@gmail.com>
Hi Johan,
> If either one of the HCI_SETUP or HCI_USER_CHANNEL flags is set the
> device is not considered valid for mgmt. By having these checks inside
> the mgmt_valid_hdev function the a couple of places using it can be
> simplified.
I looked at doing this and decided not to. Reason was that the device gets removed from mgmt anyway.
So I need a bit more detail why we better do it this way.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 6/8] Bluetooth: Use async request for LE enable/disable
From: Marcel Holtmann @ 2013-09-24 18:20 UTC (permalink / raw)
To: johan.hedberg; +Cc: linux-bluetooth
In-Reply-To: <1380031363-1266-7-git-send-email-johan.hedberg@gmail.com>
Hi Johan,
> This patch updates the code to use an asynchronous request for handling
> the enabling and disabling of LE support. This refactoring is necessary
> as a preparation for adding advertising support, since when LE is
> disabled we should also disable advertising, and the cleanest way to do
> this is to perform the two respective HCI commands in the same
> asynchronous request.
>
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> include/net/bluetooth/hci_core.h | 1 -
> net/bluetooth/hci_event.c | 11 -------
> net/bluetooth/mgmt.c | 69 +++++++++++++++++-----------------------
> 3 files changed, 29 insertions(+), 52 deletions(-)
Acked-by: Marcel Holtmann <marcel@holtmann.org>
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 7/8] Bluetooth: Add new mgmt setting for LE advertising
From: Marcel Holtmann @ 2013-09-24 18:29 UTC (permalink / raw)
To: johan.hedberg; +Cc: linux-bluetooth
In-Reply-To: <1380031363-1266-8-git-send-email-johan.hedberg@gmail.com>
Hi Johan,
> This patch adds a new mgmt setting for LE advertising and hooks up the
> necessary places in the mgmt code to operate on the HCI_LE_PERIPHERAL
> flag (which corresponds to this setting). This patch does not yet add
> any new command for enabling the setting - that is left for a subsequent
> patch.
>
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> include/net/bluetooth/mgmt.h | 1 +
> net/bluetooth/mgmt.c | 24 +++++++++++++++++++++++-
> 2 files changed, 24 insertions(+), 1 deletion(-)
Acked-by: Marcel Holtmann <marcel@holtmann.org>
Regards
Marcel
^ permalink raw reply
* RE: [PATCH v5 1/2] Bluetooth: btmrvl: add setup handler
From: Bing Zhao @ 2013-09-24 19:04 UTC (permalink / raw)
To: Marcel Holtmann
Cc: linux-bluetooth@vger.kernel.org, Gustavo Padovan, Johan Hedberg,
linux-wireless@vger.kernel.org, Mike Frysinger, Hyuckjoo Lee,
Amitkumar Karwar
In-Reply-To: <DF6283E3-8476-400F-A230-CB7CA0F98564@holtmann.org>
[-- Attachment #1: Type: text/plain, Size: 1434 bytes --]
Hi Marcel,
> > It's observed that sometimes the setup handler is called twice when Bluetooth daemon is running in
> background. We will rebase to latest commit on bluetooth-next tree and test again. If the issue is
> gone with the latest code in -next tree we will remove the setup_done flag.
>
> that is a bug. It should only be ever called once. Could this be due to RFKILL issue we had? Please
> re-test with Johan's patches applied and check if it makes a difference. Otherwise please send some
> logs since we want to get this fixed.
Amitkumar Karwar has tested it with latest code on bluetooth-next tree but the result is the same.
Apparently two threads race to call hci_dev_open(). If the thread from hci_sock calls hci_dev_open earlier, it ends up not updating HCI_SETUP hdev flag in hci_power_on(). This results that the setup handler gets called again when user brings up the interface later.
Attached are the debug logs and the patch used to generate them.
I checked the bluetooth-next tree, the following two patches (by Johan) are not present in this tree.
bf54303 Bluetooth: Fix rfkill functionality during the HCI setup stage
5e13036 Bluetooth: Introduce a new HCI_RFKILLED flag
They are in bluetooth.git tree. So, I'm not certain if Amitkumar has applied them manually or not. Anyway we will re-test with Johan's patches applied and confirm if they fix the race or not.
Thanks,
Bing
[-- Attachment #2: success.log --]
[-- Type: application/octet-stream, Size: 1097 bytes --]
[ 1717.100628] mmc1: new SDIO card at address 0001
[ 1717.100862] Bluetooth: vendor=0x2df, device=0x912a, class=255, fn=2
[ 1718.381306] BT_DBG: enter: hci_register_dev
[ 1718.381668] BT_DBG: in hci_register_dev queueing work power_on
[ 1718.381676] BT_DBG: exit: hci_register_dev
[ 1718.382658] BT_DBG: enter: hci_power_on line=1673 pid=3686
[ 1718.382664] BT_DBG: hci_dev_open line=1186 pid=3686
[ 1718.382669] BT_DBG: hci_dev_open line=1200 pid=3686
[ 1718.382673] BT_DBG: hci_dev_open line=1206 pid=3686
[ 1718.382677] BT_DBG: hci_dev_open line=1212 pid=3686
[ 1718.382681] BT_DBG: hci_dev_open line=1217 pid=3686
[ 1718.382684] btmrvl: enter btmrvl_setup()
[ 1718.383231] BT_DBG: calling hci_dev_open from hci_sock.c pid=3689
[ 1718.383236] BT_DBG: hci_dev_open line=1186 pid=3689
[ 1718.489930] BT_DBG: hci_dev_open line=1225 pid=3686
[ 1718.635973] BT_DBG: hci_power_on line=1684 updating HCI_SETUP hdev flag pid=3686
[ 1718.635984] BT_DBG: exit: hci_power_on line=1687 pid=3686
[ 1718.636282] BT_DBG: hci_dev_open line=1200 pid=3689
[ 1718.636288] BT_DBG: hci_dev_open line=1206 pid=3689
[-- Attachment #3: failure.log --]
[-- Type: application/octet-stream, Size: 951 bytes --]
[ 132.180560] mmc1: new SDIO card at address 0001
[ 132.415282] Bluetooth: vendor=0x2df, device=0x912a, class=255, fn=2
[ 133.784309] BT_DBG: enter: hci_register_dev
[ 133.784694] BT_DBG: in hci_register_dev queueing work power_on
[ 133.784703] BT_DBG: exit: hci_register_dev
[ 133.786616] BT_DBG: calling hci_dev_open from hci_sock.c pid=3287
[ 133.786624] BT_DBG: hci_dev_open line=1186 pid=3287
[ 133.786628] BT_DBG: hci_dev_open line=1200 pid=3287
[ 133.786633] BT_DBG: hci_dev_open line=1206 pid=3287
[ 133.786636] BT_DBG: hci_dev_open line=1212 pid=3287
[ 133.786640] BT_DBG: hci_dev_open line=1217 pid=3287
[ 133.786644] btmrvl: enter btmrvl_setup()
[ 133.789683] BT_DBG: enter: hci_power_on line=1673 pid=3283
[ 133.789691] BT_DBG: hci_dev_open line=1186 pid=3283
[ 133.954749] BT_DBG: hci_dev_open line=1225 pid=3287
[ 134.101863] BT_DBG: hci_dev_open line=1200 pid=3283
[ 134.101871] BT_DBG: hci_dev_open line=1206 pid=3283
[-- Attachment #4: bt_debug.diff --]
[-- Type: application/octet-stream, Size: 3517 bytes --]
diff --git a/drivers/bluetooth/btmrvl_main.c b/drivers/bluetooth/btmrvl_main.c
index 6eea188..b474dde 100644
--- a/drivers/bluetooth/btmrvl_main.c
+++ b/drivers/bluetooth/btmrvl_main.c
@@ -621,6 +621,7 @@ static int btmrvl_setup(struct hci_dev *hdev)
struct btmrvl_private *priv = hci_get_drvdata(hdev);
struct btmrvl_adapter *adapter = priv->adapter;
+ printk("btmrvl: enter btmrvl_setup()\n");
if (adapter->setup_done)
return 0;
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 3d9f02b..d0795e9 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1183,6 +1183,7 @@ int hci_dev_open(__u16 dev)
struct hci_dev *hdev;
int ret = 0;
+ printk("BT_DBG: hci_dev_open line=%d pid=%d\n", __LINE__, current->pid);
hdev = hci_dev_get(dev);
if (!hdev)
return -ENODEV;
@@ -1196,20 +1197,24 @@ int hci_dev_open(__u16 dev)
goto done;
}
+ printk("BT_DBG: hci_dev_open line=%d pid=%d\n", __LINE__, current->pid);
if (hdev->rfkill && rfkill_blocked(hdev->rfkill)) {
ret = -ERFKILL;
goto done;
}
+ printk("BT_DBG: hci_dev_open line=%d pid=%d\n", __LINE__, current->pid);
if (test_bit(HCI_UP, &hdev->flags)) {
ret = -EALREADY;
goto done;
}
+ printk("BT_DBG: hci_dev_open line=%d pid=%d\n", __LINE__, current->pid);
if (hdev->open(hdev)) {
ret = -EIO;
goto done;
}
+ printk("BT_DBG: hci_dev_open line=%d pid=%d\n", __LINE__, current->pid);
atomic_set(&hdev->cmd_cnt, 1);
set_bit(HCI_INIT, &hdev->flags);
@@ -1217,6 +1222,7 @@ int hci_dev_open(__u16 dev)
if (hdev->setup && test_bit(HCI_SETUP, &hdev->dev_flags))
ret = hdev->setup(hdev);
+ printk("BT_DBG: hci_dev_open line=%d pid=%d\n", __LINE__, current->pid);
if (!ret) {
/* Treat all non BR/EDR controllers as raw devices if
* enable_hs is not set.
@@ -1664,6 +1670,7 @@ static void hci_power_on(struct work_struct *work)
BT_DBG("%s", hdev->name);
+ printk("BT_DBG: enter: hci_power_on line=%d pid=%d\n", __LINE__, current->pid);
err = hci_dev_open(hdev->id);
if (err < 0) {
mgmt_set_powered_failed(hdev, err);
@@ -1674,8 +1681,10 @@ static void hci_power_on(struct work_struct *work)
queue_delayed_work(hdev->req_workqueue, &hdev->power_off,
HCI_AUTO_OFF_TIMEOUT);
+ printk("BT_DBG: hci_power_on line=%d updating HCI_SETUP hdev flag pid=%d\n", __LINE__, current->pid);
if (test_and_clear_bit(HCI_SETUP, &hdev->dev_flags))
mgmt_index_added(hdev);
+ printk("BT_DBG: exit: hci_power_on line=%d pid=%d\n", __LINE__, current->pid);
}
static void hci_power_off(struct work_struct *work)
@@ -2234,6 +2243,7 @@ int hci_register_dev(struct hci_dev *hdev)
{
int id, error;
+ printk("BT_DBG: enter: hci_register_dev\n");
if (!hdev->open || !hdev->close)
return -EINVAL;
@@ -2300,7 +2310,9 @@ int hci_register_dev(struct hci_dev *hdev)
hci_notify(hdev, HCI_DEV_REG);
hci_dev_hold(hdev);
+ printk("BT_DBG: in hci_register_dev queueing work power_on\n");
queue_work(hdev->req_workqueue, &hdev->power_on);
+ printk("BT_DBG: exit: hci_register_dev\n");
return id;
diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
index c09e976..ddefcbe 100644
--- a/net/bluetooth/hci_sock.c
+++ b/net/bluetooth/hci_sock.c
@@ -587,6 +587,7 @@ static int hci_sock_ioctl(struct socket *sock, unsigned int cmd,
case HCIDEVUP:
if (!capable(CAP_NET_ADMIN))
return -EPERM;
+ printk("BT_DBG: calling hci_dev_open from hci_sock.c pid=%d\n", current->pid);
return hci_dev_open(arg);
case HCIDEVDOWN:
^ permalink raw reply related
* RE: [PATCH v5 2/2] Bluetooth: btmrvl: add calibration data download support
From: Bing Zhao @ 2013-09-24 19:22 UTC (permalink / raw)
To: Marcel Holtmann
Cc: linux-bluetooth@vger.kernel.org, Gustavo Padovan, Johan Hedberg,
linux-wireless@vger.kernel.org, Mike Frysinger, Hyuckjoo Lee,
Amitkumar Karwar
In-Reply-To: <34088279-04B9-49D0-9387-88A602F518EC@holtmann.org>
Hi Marcel,
> > The reason of not using __hci_cmd_sync() is that we are sending vendor =
specific command here
> (MRVL_VENDOR_PKT). The __hci_cmd_sync seems handle HCI_COMMAND_PKT only.
> > Please let us know if you have any suggestion to solve this problem.
>=20
> what is a MRVL_VENDOR_PKT actually?
It's defined as 0xfe in our driver. The firmware doesn't understand 0xff (H=
CI_VENDOR_PKT).
>=20
> If you guys are not using standard HCI command/event for vendor operation=
, then this obviously does
> not fit. However a similar model might make sense instead of manually bui=
lding packets all the time.
We can extend __hci_cmd_sync() function with a new parameter 'type'.
This way we can pass HCI_VENDOR_PKT into __hci_cmd_sync(), while other driv=
ers will pass in HCI_COMMAND_PKT.
Our driver will make HCI_VENDOR_PKT -> MRVL_VENDOR_PKT conversion before do=
wnloading the frame to firmware. And the MRVL_VENDOR_PKT frame from firmwar=
e will be replaced with HCI_VENDOR_PKT while uploading the frame to stack.
Please let us know if this approach works for you or not.
Thanks,
Bing
^ permalink raw reply
* Re: [PATCH v5 1/2] Bluetooth: btmrvl: add setup handler
From: Johan Hedberg @ 2013-09-24 19:30 UTC (permalink / raw)
To: Bing Zhao
Cc: Marcel Holtmann, linux-bluetooth@vger.kernel.org, Gustavo Padovan,
linux-wireless@vger.kernel.org, Mike Frysinger, Hyuckjoo Lee,
Amitkumar Karwar
In-Reply-To: <477F20668A386D41ADCC57781B1F70430F450779A4@SC-VEXCH1.marvell.com>
[-- Attachment #1: Type: text/plain, Size: 1869 bytes --]
Hi Bing,
On Tue, Sep 24, 2013, Bing Zhao wrote:
> > that is a bug. It should only be ever called once. Could this be due
> > to RFKILL issue we had? Please re-test with Johan's patches applied
> > and check if it makes a difference. Otherwise please send some logs
> > since we want to get this fixed.
>
> Amitkumar Karwar has tested it with latest code on bluetooth-next tree
> but the result is the same.
> Apparently two threads race to call hci_dev_open(). If the thread from
> hci_sock calls hci_dev_open earlier, it ends up not updating HCI_SETUP
> hdev flag in hci_power_on(). This results that the setup handler gets
> called again when user brings up the interface later.
Let's see if I understood this right: the only hci_dev_open call in
hci_sock.c is the one for the HCIDEVUP ioctl. So what you're doing is
having user space call the HCIDEVUP ioctl before our own hci_power_on
callback gets called to initialize the adapter?
You're right that we're missing the clearing of the HCI_SETUP flag for
such a scenario. Could you try the attached patch. It should fix the
issue. One problem that it does have is that if the HCIDEVUP ioctl path
goes through before hci_power_on gets called we will never notify mgmt
of the adapter. However, that might be acceptable here since if you're
using HCIDEVUP like this it seems it's not a mgmt based system anyway.
> I checked the bluetooth-next tree, the following two patches (by
> Johan) are not present in this tree.
>
> bf54303 Bluetooth: Fix rfkill functionality during the HCI setup stage
> 5e13036 Bluetooth: Introduce a new HCI_RFKILLED flag
>
> They are in bluetooth.git tree. So, I'm not certain if Amitkumar has
> applied them manually or not. Anyway we will re-test with Johan's
> patches applied and confirm if they fix the race or not.
I don't think these patches will help you in this case.
Johan
[-- Attachment #2: hci-setup.patch --]
[-- Type: text/plain, Size: 1114 bytes --]
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 634deba..c48bf1a 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1164,7 +1164,7 @@ int hci_dev_open(__u16 dev)
atomic_set(&hdev->cmd_cnt, 1);
set_bit(HCI_INIT, &hdev->flags);
- if (hdev->setup && test_bit(HCI_SETUP, &hdev->dev_flags))
+ if (test_and_clear_bit(HCI_SETUP, &hdev->dev_flags) && hdev->setup)
ret = hdev->setup(hdev);
if (!ret) {
@@ -1581,10 +1581,13 @@ static const struct rfkill_ops hci_rfkill_ops = {
static void hci_power_on(struct work_struct *work)
{
struct hci_dev *hdev = container_of(work, struct hci_dev, power_on);
+ bool setup;
int err;
BT_DBG("%s", hdev->name);
+ setup = test_bit(HCI_SETUP, &hdev->dev_flags);
+
err = hci_dev_open(hdev->id);
if (err < 0) {
mgmt_set_powered_failed(hdev, err);
@@ -1595,7 +1598,7 @@ static void hci_power_on(struct work_struct *work)
queue_delayed_work(hdev->req_workqueue, &hdev->power_off,
HCI_AUTO_OFF_TIMEOUT);
- if (test_and_clear_bit(HCI_SETUP, &hdev->dev_flags))
+ if (setup)
mgmt_index_added(hdev);
}
^ permalink raw reply related
* RE: [PATCH v5 1/2] Bluetooth: btmrvl: add setup handler
From: Bing Zhao @ 2013-09-24 19:42 UTC (permalink / raw)
To: Johan Hedberg
Cc: Marcel Holtmann, linux-bluetooth@vger.kernel.org, Gustavo Padovan,
linux-wireless@vger.kernel.org, Mike Frysinger, Hyuckjoo Lee,
Amitkumar Karwar
In-Reply-To: <20130924193010.GA2584@x220.p-661hnu-f1>
Hi Johan,
> Hi Bing,
>=20
> On Tue, Sep 24, 2013, Bing Zhao wrote:
> > > that is a bug. It should only be ever called once. Could this be due
> > > to RFKILL issue we had? Please re-test with Johan's patches applied
> > > and check if it makes a difference. Otherwise please send some logs
> > > since we want to get this fixed.
> >
> > Amitkumar Karwar has tested it with latest code on bluetooth-next tree
> > but the result is the same.
> > Apparently two threads race to call hci_dev_open(). If the thread from
> > hci_sock calls hci_dev_open earlier, it ends up not updating HCI_SETUP
> > hdev flag in hci_power_on(). This results that the setup handler gets
> > called again when user brings up the interface later.
>=20
> Let's see if I understood this right: the only hci_dev_open call in
> hci_sock.c is the one for the HCIDEVUP ioctl. So what you're doing is
> having user space call the HCIDEVUP ioctl before our own hci_power_on
> callback gets called to initialize the adapter?
That's right. The ioctl is initiated by the Bluetooth daemon.
Amitkumar has a setup that can reproduce this corner case easily.
I tested it on my Ubuntu but I couldn't replicate it.
>=20
> You're right that we're missing the clearing of the HCI_SETUP flag for
> such a scenario. Could you try the attached patch. It should fix the
> issue. One problem that it does have is that if the HCIDEVUP ioctl path
> goes through before hci_power_on gets called we will never notify mgmt
> of the adapter. However, that might be acceptable here since if you're
> using HCIDEVUP like this it seems it's not a mgmt based system anyway.
>=20
> > I checked the bluetooth-next tree, the following two patches (by
> > Johan) are not present in this tree.
> >
> > bf54303 Bluetooth: Fix rfkill functionality during the HCI setup stage
> > 5e13036 Bluetooth: Introduce a new HCI_RFKILLED flag
> >
> > They are in bluetooth.git tree. So, I'm not certain if Amitkumar has
> > applied them manually or not. Anyway we will re-test with Johan's
> > patches applied and confirm if they fix the race or not.
>=20
> I don't think these patches will help you in this case.
OK, we will test your patch instead.
Thanks,
Bing
^ permalink raw reply
* Re: [PATCH v5 2/2] Bluetooth: btmrvl: add calibration data download support
From: Marcel Holtmann @ 2013-09-24 19:43 UTC (permalink / raw)
To: Bing Zhao
Cc: linux-bluetooth@vger.kernel.org, Gustavo Padovan, Johan Hedberg,
linux-wireless@vger.kernel.org, Mike Frysinger, Hyuckjoo Lee,
Amitkumar Karwar
In-Reply-To: <477F20668A386D41ADCC57781B1F70430F450779BE@SC-VEXCH1.marvell.com>
Hi Bing,
>>> The reason of not using __hci_cmd_sync() is that we are sending vendor specific command here
>> (MRVL_VENDOR_PKT). The __hci_cmd_sync seems handle HCI_COMMAND_PKT only.
>>> Please let us know if you have any suggestion to solve this problem.
>>
>> what is a MRVL_VENDOR_PKT actually?
>
> It's defined as 0xfe in our driver. The firmware doesn't understand 0xff (HCI_VENDOR_PKT).
so it is actually out-of-channel vendor packet.
>>
>> If you guys are not using standard HCI command/event for vendor operation, then this obviously does
>> not fit. However a similar model might make sense instead of manually building packets all the time.
>
> We can extend __hci_cmd_sync() function with a new parameter 'type'.
> This way we can pass HCI_VENDOR_PKT into __hci_cmd_sync(), while other drivers will pass in HCI_COMMAND_PKT.
That will actually not work. And I also do not want to do that. The __hci_cmd_sync() is for real HCI packets. That means types 0x01 and 0x04 only. They need to adhere to the HCI flow control mechanism for commands.
> Our driver will make HCI_VENDOR_PKT -> MRVL_VENDOR_PKT conversion before downloading the frame to firmware. And the MRVL_VENDOR_PKT frame from firmware will be replaced with HCI_VENDOR_PKT while uploading the frame to stack.
>
> Please let us know if this approach works for you or not.
I think this is best kept inside the driver. However you might consider building something like __hci_cmd_sync() that is specific to your driver, but allows for a similar flow within ->setup().
Regards
Marcel
^ permalink raw reply
* RE: [PATCH v5 2/2] Bluetooth: btmrvl: add calibration data download support
From: Bing Zhao @ 2013-09-24 19:54 UTC (permalink / raw)
To: Marcel Holtmann
Cc: linux-bluetooth@vger.kernel.org, Gustavo Padovan, Johan Hedberg,
linux-wireless@vger.kernel.org, Mike Frysinger, Hyuckjoo Lee,
Amitkumar Karwar
In-Reply-To: <C42E9D11-1E8B-499E-96E0-74469F7649EC@holtmann.org>
Hi Marcel,
> > We can extend __hci_cmd_sync() function with a new parameter 'type'.
> > This way we can pass HCI_VENDOR_PKT into __hci_cmd_sync(), while other =
drivers will pass in
> HCI_COMMAND_PKT.
>=20
> That will actually not work. And I also do not want to do that. The __hci=
_cmd_sync() is for real HCI
> packets. That means types 0x01 and 0x04 only. They need to adhere to the =
HCI flow control mechanism
> for commands.
I see.
>=20
> > Our driver will make HCI_VENDOR_PKT -> MRVL_VENDOR_PKT conversion befor=
e downloading the frame to
> firmware. And the MRVL_VENDOR_PKT frame from firmware will be replaced wi=
th HCI_VENDOR_PKT while
> uploading the frame to stack.
> >
> > Please let us know if this approach works for you or not.
>=20
> I think this is best kept inside the driver. However you might consider b=
uilding something like
> __hci_cmd_sync() that is specific to your driver, but allows for a simila=
r flow within ->setup().
Sure, we will consider building a function in the driver to handle this.
Thanks,
Bing
^ permalink raw reply
* Re: [PATCH 3/8] Bluetooth: Test for HCI_SETUP and HCI_USER_CHANNEL in mgmt_valid_hdev()
From: Johan Hedberg @ 2013-09-25 10:03 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <DFCF34C2-5AA9-4644-BB4B-3D6073DA9D42@holtmann.org>
Hi Marcel,
On Tue, Sep 24, 2013, Marcel Holtmann wrote:
> > If either one of the HCI_SETUP or HCI_USER_CHANNEL flags is set the
> > device is not considered valid for mgmt. By having these checks inside
> > the mgmt_valid_hdev function the a couple of places using it can be
> > simplified.
>
> I looked at doing this and decided not to. Reason was that the device
> gets removed from mgmt anyway.
I'm not sure what significance you think "device gets removed from mgmt"
has. All that is is a mgmt event saying that index has been removed. In
addition to that we need to ensure that we don't send any more mgmt
events for such devices and that we don't include such devices in the
response to mgmt_read_index list.
These are the two places of the code that my patch simplifies, one is
the check for whether to send a "power on" mgmt event and the other the
response handling of read_index_list.
Johan
^ permalink raw reply
* [PATCH v2 0/7] Bluetooth: Cleanups and LE advertising support
From: johan.hedberg @ 2013-09-25 10:26 UTC (permalink / raw)
To: linux-bluetooth
Hi,
Here's an updated set based on feedback on the first one. The
mgmt_valid_hdev patch has been dropped since it's a trivial one and I
didn't want the debate over it to slow down the overall progress of this
set.
The main difference to the initial set_le and set_advertising code is
that I now let the hci_event.c handlers take care of setting the right
values of the dev_flags bits. This works fine since these handlers are
always executed before the async request callbacks which then get called
with dev_flags already having the correct value. The benefit of using
the event handlers like this is that we retain at least partial support
for raw HCI access (hciconfig) by ensuring that the kernel state flags
are always correct.
Johan
----------------------------------------------------------------
Johan Hedberg (7):
Bluetooth: Add clarifying comment to bt_sock_wait_state()
Bluetooth: Clean up socket locking in l2cap_sock_recvmsg
Bluetooth: Fix busy return for mgmt_set_powered in some cases
Bluetooth: Move mgmt response convenience functions to a better location
Bluetooth: Use async request for LE enable/disable
Bluetooth: Add new mgmt setting for LE advertising
Bluetooth: Add new mgmt_set_advertising command
include/net/bluetooth/hci_core.h | 1 -
include/net/bluetooth/mgmt.h | 3 +
net/bluetooth/af_bluetooth.c | 1 +
net/bluetooth/hci_event.c | 12 +-
net/bluetooth/l2cap_sock.c | 4 +-
net/bluetooth/mgmt.c | 257 ++++++++++++++++++++++++++------------
6 files changed, 191 insertions(+), 87 deletions(-)
^ permalink raw reply
* [PATCH v2 1/7] Bluetooth: Add clarifying comment to bt_sock_wait_state()
From: johan.hedberg @ 2013-09-25 10:26 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1380104770-8022-1-git-send-email-johan.hedberg@gmail.com>
From: Johan Hedberg <johan.hedberg@intel.com>
The bt_sock_wait_state requires the sk lock to be held (through
lock_sock) so document it clearly in the code.
Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
---
net/bluetooth/af_bluetooth.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c
index c600631..e6e1278 100644
--- a/net/bluetooth/af_bluetooth.c
+++ b/net/bluetooth/af_bluetooth.c
@@ -490,6 +490,7 @@ int bt_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
}
EXPORT_SYMBOL(bt_sock_ioctl);
+/* This function expects the sk lock to be held when called */
int bt_sock_wait_state(struct sock *sk, int state, unsigned long timeo)
{
DECLARE_WAITQUEUE(wait, current);
--
1.8.3.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox