Linux bluetooth development
 help / color / mirror / Atom feed
* Re: [PATCH v2 2/3] android: Add skeleton of BlueZ Android daemon
From: Marcel Holtmann @ 2013-09-27  2:05 UTC (permalink / raw)
  To: Frederic Danis; +Cc: linux-bluetooth
In-Reply-To: <1380024798-32017-2-git-send-email-frederic.danis@linux.intel.com>

Hi Fred,

> 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 +++

I rather keep the changes to a global Makefile separate from the android/ stuff.

> android/Android.mk |   24 ++++++++++++
> android/main.c     |  110 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 141 insertions(+)
> create mode 100644 android/main.c

It is perfectly fine to split this into two patches. Add the *.c file(s) first and in a second patch add the build changes.

> 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

You can not put the EXTRA_DIST under an if ANDROID. Make that part global.

I expect that make distcheck works no matter what is configured. Check the final tarballs if all files are actually included.

Regards

Marcel


^ permalink raw reply

* Re: [PATCH v2 3/3] android: Android version of log.c
From: Marcel Holtmann @ 2013-09-27  2:01 UTC (permalink / raw)
  To: Frederic Danis; +Cc: linux-bluetooth
In-Reply-To: <1380024798-32017-3-git-send-email-frederic.danis@linux.intel.com>

Hi Fred,

> Add logging system to BlueZ Android daemon.
> Android build will use android/log.c file while autotools build will use
> src/log.c instead.

lets just use stdout for now. Changing this later is pretty trivial and introducing large build alternates is a bit too complicated at this point.

Regards

Marcel


^ permalink raw reply

* Re: [PATCH 2/2] android: Android version of log.c
From: Marcel Holtmann @ 2013-09-27  1:58 UTC (permalink / raw)
  To: Frederic Danis; +Cc: linux-bluetooth
In-Reply-To: <524155D9.4050807@linux.intel.com>

Hi Fred,

> <snip>
>>> @@ -110,10 +137,22 @@ int main(int argc, char *argv[])
>>> 	sa.sa_handler = signal_handler;
>>> 	sigaction(SIGINT, &sa, NULL);
>>> 	sigaction(SIGTERM, &sa, NULL);
>>> +	sigaction(SIGUSR2, &sa, NULL);
>>> +
>>> +	__btd_log_init(option_debug, option_detach);
>>> +
>>> +	/* no need to keep parsed option in memory */
>>> +	free_options();
>>> +
>>> +	DBG("Entering main loop");
>> 
>> Don't do this debug. It is not helpful. The main() is so short, no point in logging that.
> 
> I need at least one DBG call to be able to build, otherwise I got "undefined reference to `__start___debug'" error messages.
> So, this one seems ok to me.

fair enough for the beginning. Just remove it once you have more than one DBG in the source code and this is no longer needed.

Regards

Marcel


^ permalink raw reply

* Re: [PATCH 1/2] android: Add skeleton of BlueZ Android daemon
From: Marcel Holtmann @ 2013-09-27  1:56 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: Lukasz Rymanowski, Frederic Danis, linux-bluetooth
In-Reply-To: <20130924073124.GA26962@aemeltch-MOBL1>

Hi Andrei,

>>>>> 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 +
>>>>> Android.mk         |    9 ++++
>>>>> Makefile.am        |    1 +
>>>>> Makefile.android   |    7 ++++
>>>>> android/Android.mk |   24 +++++++++++
>>>>> android/main.c     |  119
>>>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>> configure.ac       |    5 +++
>>>>> 7 files changed, 167 insertions(+)
>>>>> create mode 100644 Android.mk
>>>>> create mode 100644 Makefile.android
>>>>> create mode 100644 android/Android.mk
>>>>> create mode 100644 android/main.c
>>>> 
>>>> 
>>>> lets split this out a little bit. Code additions should not be intermixed
>>>> with additions to the build system and especially configure options.
>>>> 
>>>> I rather not have a top-level Android.mk. It should be enough to provide
>>>> an android/Android.mk.
>> 
>> BTW, do you expect to have only one Android.mk for the whole project or
>> this is OK for you to have smaller Android.mk in different subfolders like
>> /src/shared etc. Both ways are valid.
> 
> While having a single huge Android makefile is somehow make sense it would have
> several disadvantages:
> - we cannot use local build with "mm" for building for example only tools
>  or libs
> - Android.mk will become very large and difficult to read

I have no idea what this means. Lets start with a single android/Android.mk and then go from there. I am strictly against cluttering the whole project with Android specific make files.

Regards

Marcel


^ permalink raw reply

* Re: Problems with too many connections
From: Marcel Holtmann @ 2013-09-27  1:53 UTC (permalink / raw)
  To: Markus Roppelt; +Cc: linux-bluetooth
In-Reply-To: <CAOJw-HaHLZsbgCGi_e1ZspAf_16gKQuC6N7JM+5boWxQHCXWFQ@mail.gmail.com>

Hi Markus,

> I want to write an application which repeats the following procedure
> for several (100+) bluetooth (low energy) devices:
> 
> 1. connect
> 2. read register values
> 3. disconnect
> 
> Therefore I modified the source code from /attrib/interactive.c.  For
> testing purposes I am only looping  over one device. See attached
> source code.
> 
> The code works fine. It connects, reads the values and disconnects.
> However, after 1020 repetitions, the following error occurs:
> (process:10205): GLib-WARNING **: poll(2) failed due to: Invalid argument.
> 
> I think the problem has to do with some sockets / file descriptors not
> being closed properly.
> 
> Can someone help me to get this fixed?

have you considered trying to write this from scratch and not basing this off existing code.

And yes, this will be most likely an issues with GSource handling of the attribute IO channel, but since you hacked the code is extremely hard to debug. It was never designed for what you are doing.

Regards

Marcel


^ permalink raw reply

* Re: pull request: bluetooth 2013-09-23
From: John W. Linville @ 2013-09-26 17:47 UTC (permalink / raw)
  To: Gustavo Padovan, linux-wireless, linux-bluetooth, linux-kernel
In-Reply-To: <20130923210049.GG18215@joana>

On Mon, Sep 23, 2013 at 06:00:49PM -0300, Gustavo Padovan wrote:
> Hi John,
> 
> First Bluetooth fixes to 3.12, it includes:
> 
> * 3 patches to add device id for 3 new hardwares.
> 
> * 2 patches from Johan to fix the rfkill behaviour during setup stage
> 
> * a small clean up in the rfcomm TTY code that fixes a potential racy
> condition (by Gianluca Anzolin)
> 
> * 2 fixes to proper set encryption key size and security level in the
> peripheral role of Bluetooth LE devices. (by Andre Guedes)
> 
> * a fix for dealing devices where pairing is not necessary, we were keeping
> the Bluetooth ACL connection alive for too much time. (by Syam Sidhardhan)
> 
> Please pull or let me know of any problems! Thanks!
> 
> 
> 	Gustavo
> 
> ---
> The following changes since commit f4e1a4d3ecbb9e42bdf8e7869ee8a4ebfa27fb20:
> 
>   rt2800: change initialization sequence to fix system freeze (2013-09-09 14:44:34 -0400)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth master
> 
> for you to fetch changes up to 5bcecf325378218a8e248bb6bcae96ec7362f8ef:
> 
>   Bluetooth: btusb: Add support for Belkin F8065bf (2013-09-23 17:44:25 -0300)

Pulling now...

-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

^ permalink raw reply

* [RFC] audio/AVCTP: Retry rejected outgoing connect if incoming was rejected
From: Szymon Janc @ 2013-09-26 13:54 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

Incoming AVCTP connection is rejected when there is outgoing connection
in progress. It is possible that remote device will do the same and
AVCTP will not be connected at all. In such case retry outgoing
connection if it was rejected by remote and in the meantime incoming
connection was rejected by bluetoothd.

profiles/audio/avrcp.c:avrcp_connect() path /org/bluez/hci0/
    dev_00_1E_DE_21_85_6A
profiles/audio/avctp.c:avctp_set_state() AVCTP Connecting
profiles/audio/sink.c:sink_set_state() State changed
    /org/bluez/hci0/dev_00_1E_DE_21_85_6A: SINK_STATE_CONNECTING ->
    SINK_STATE_CONNECTED
profiles/audio/transport.c:transport_update_playing()
    /org/bluez/hci0/dev_00_1E_DE_21_85_6A/fd1
    State=TRANSPORT_STATE_IDLE Playing=0
profiles/audio/avctp.c:avctp_confirm_cb() AVCTP: incoming connect
    from 00:1E:DE:21:85:6A
Control: Refusing unexpected connect
profiles/audio/avctp.c:avctp_set_state() AVCTP Disconnected
connect error: Connection refused (111)

This fix connecting to Nokia BH-505 headset.
---
 profiles/audio/avctp.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/profiles/audio/avctp.c b/profiles/audio/avctp.c
index 845027f..d9b798c 100644
--- a/profiles/audio/avctp.c
+++ b/profiles/audio/avctp.c
@@ -202,6 +202,8 @@ struct avctp {
 	uint8_t key_quirks[256];
 	struct key_pressed key;
 	bool initiator;
+
+	bool incoming_refused;
 };
 
 struct avctp_passthrough_handler {
@@ -1162,6 +1164,36 @@ fail:
 	}
 }
 
+static void avctp_connect_cb(GIOChannel *chan, GError *err, gpointer data);
+
+static bool retry_control_connect(struct avctp *session)
+{
+	GIOChannel *io;
+
+	info ("Control: retrying to connect");
+
+	if (session->control) {
+		avctp_channel_destroy(session->control);
+		session->control = NULL;
+	}
+
+	io = bt_io_connect(avctp_connect_cb, session, NULL, NULL,
+				BT_IO_OPT_SOURCE_BDADDR,
+				adapter_get_address(session->server->adapter),
+				BT_IO_OPT_DEST_BDADDR,
+				device_get_address(session->device),
+				BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_MEDIUM,
+				BT_IO_OPT_PSM, AVCTP_CONTROL_PSM,
+				BT_IO_OPT_INVALID);
+	if (io == NULL)
+		return false;
+
+	session->control = avctp_channel_create(session, io, NULL);
+	g_io_channel_unref(io);
+
+	return true;
+}
+
 static void avctp_connect_cb(GIOChannel *chan, GError *err, gpointer data)
 {
 	struct avctp *session = data;
@@ -1170,6 +1202,11 @@ static void avctp_connect_cb(GIOChannel *chan, GError *err, gpointer data)
 	GError *gerr = NULL;
 
 	if (err) {
+		if (session->incoming_refused && err->code == ECONNREFUSED) {
+			if (retry_control_connect(session))
+				return;
+		}
+
 		avctp_set_state(session, AVCTP_STATE_DISCONNECTED);
 		error("%s", err->message);
 		return;
@@ -1301,6 +1338,8 @@ static void avctp_control_confirm(struct avctp *session, GIOChannel *chan,
 	if (session->control != NULL) {
 		error("Control: Refusing unexpected connect");
 		g_io_channel_shutdown(chan, TRUE, NULL);
+
+		session->incoming_refused = session->initiator;
 		return;
 	}
 
-- 
1.8.4


^ permalink raw reply related

* Re: [PATCH v2 0/4] obexd: Fix setting message folder
From: Luiz Augusto von Dentz @ 2013-09-26 13:01 UTC (permalink / raw)
  To: Christian Fetzer; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <1379928269-14651-1-git-send-email-christian.fetzer@oss.bmw-carit.de>

Hi Christian,

On Mon, Sep 23, 2013 at 12:24 PM, Christian Fetzer
<christian.fetzer@oss.bmw-carit.de> wrote:
> From: Christian Fetzer <christian.fetzer@bmw-carit.de>
>
> This fixes an issue that the folder property for messages was set incorrectly
> when ListMessages got called with a subfolder parameter.
>
> Calling ListMessages within /telecom/msg for the subfolder inbox would set the
> folder property to /telecom/msg instead of /telecom/msg/inbox.
>
> The patchset therefore changes map_msg_create to not set the folder property
> to the current folder, but to a folder given as parameter. This change will be
> needed as well for new message notifications (that can indicate new messages
> for any folder).
>
> In addition I've updated the documentation to clarify that the folder property
> for ListMessages and PushMessage can be used only for a subfolder of the
> current folder. It's not possible to specify an absolute or relative path.
>
> --
> v2: Store folder in MAP pending request
>
> Christian Fetzer (4):
>   obexd: Add folder property to map_msg_create
>   obexd: Fix setting message folder for relative folder in ListMessages
>   obexd: Clarify the folder property of ListMessages
>   obexd: Clarify the folder property of PushMessage
>
>  doc/obex-api.txt   |  8 +++++---
>  obexd/client/map.c | 22 +++++++++++++++++++---
>  2 files changed, 24 insertions(+), 6 deletions(-)
>
> --
> 1.8.3.4

All 4 patches are now applied, but Ive done some changes to
get_absolute_folder so to have less lines breaks so it looks more
readable now.


-- 
Luiz Augusto von Dentz

^ permalink raw reply

* Re: [PATCH] Bluetooth: ath3k: enlarge the usb urb timeout
From: AceLan Kao @ 2013-09-26  9:27 UTC (permalink / raw)
  To: linux-bluetooth, Marcel Holtmann, Gustavo Padovan, Johan Hedberg
In-Reply-To: <1380171766-21845-1-git-send-email-acelan.kao@canonical.com>

Hi all,

Sorry, please ignore this patch.
This is not the root cause, and it didn't fix the issue.

Best regards,
AceLan Kao.


2013/9/26 AceLan Kao <acelan.kao@canonical.com>:
> We encountered a issue that the time is not sufficient to load the firmware.
>
> [   14.646698] Bluetooth: Error in firmware loading err = -110,len = 448, size = 4096
> [   14.646710] Bluetooth: Loading patch file failed
> [   14.646719] ath3k: probe of 3-4:1.0 failed with error -110
>
> And according to the log, it needs 10 times time to finish the transfer,
> so, enlarge the timeout 10 times to overcome it.
>
> Signed-off-by: AceLan Kao <acelan.kao@canonical.com>
> ---
>  drivers/bluetooth/ath3k.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/bluetooth/ath3k.c b/drivers/bluetooth/ath3k.c
> index a12b923..2047deb 100644
> --- a/drivers/bluetooth/ath3k.c
> +++ b/drivers/bluetooth/ath3k.c
> @@ -49,6 +49,8 @@
>  #define ATH3K_XTAL_FREQ_19P2                   0x02
>  #define ATH3K_NAME_LEN                         0xFF
>
> +#define USB_URB_TIMEOUT                                30000
> +
>  struct ath3k_version {
>         unsigned int    rom_version;
>         unsigned int    build_version;
> @@ -182,7 +184,7 @@ static int ath3k_load_firmware(struct usb_device *udev,
>                 memcpy(send_buf, firmware->data + sent, size);
>
>                 err = usb_bulk_msg(udev, pipe, send_buf, size,
> -                                       &len, 3000);
> +                                       &len, USB_URB_TIMEOUT);
>
>                 if (err || (len != size)) {
>                         BT_ERR("Error in firmware loading err = %d,"
> @@ -279,7 +281,7 @@ static int ath3k_load_fwfile(struct usb_device *udev,
>                 memcpy(send_buf, firmware->data + sent, size);
>
>                 err = usb_bulk_msg(udev, pipe, send_buf, size,
> -                                       &len, 3000);
> +                                       &len, USB_URB_TIMEOUT);
>                 if (err || (len != size)) {
>                         BT_ERR("Error in firmware loading err = %d,"
>                                 "len = %d, size = %d", err, len, size);
> --
> 1.8.3.2
>



-- 
Chia-Lin Kao(AceLan)
http://blog.acelan.idv.tw/
E-Mail: acelan.kaoATcanonical.com (s/AT/@/)

^ permalink raw reply

* [PATCH 2/2] gdbus: Remove not needed check for NULL DBusPendingCall
From: Szymon Janc @ 2013-09-26  8:02 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1380182555-11609-1-git-send-email-szymon.janc@tieto.com>

It is now checked by g_dbus_send_message_with_reply() so there is no
need to double check that in caller.
---
 gdbus/client.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/gdbus/client.c b/gdbus/client.c
index 7bffdad..be8cc29 100644
--- a/gdbus/client.c
+++ b/gdbus/client.c
@@ -112,11 +112,6 @@ static gboolean modify_match(DBusConnection *conn, const char *member,
 		return FALSE;
 	}
 
-	if (call == NULL) {
-		dbus_message_unref(msg);
-		return FALSE;
-	}
-
 	dbus_pending_call_set_notify(call, modify_match_reply, NULL, NULL);
 	dbus_pending_call_unref(call);
 
-- 
1.8.4


^ permalink raw reply related

* [PATCH 1/2] gdbus: Check for NULL DBusPendingCall in g_dbus_send_message_with_reply
From: Szymon Janc @ 2013-09-26  8:02 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

>From D-Bus documentation for dbus_connection_send_with_reply():
"Warning: if the connection is disconnected or you try to send Unix file
descriptors on a connection that does not support them, the
DBusPendingCall will be set to NULL, so be careful with this."

Check this in g_dbus_send_message_with_reply so that callers don't need
to double check for NULL if g_dbus_send_message_with_reply returned
TRUE.

This also fix crash if passing FD over D-Bus is blocked e.g. by SELinux
policy.

bluetoothd[1894]: profiles/audio/avdtp.c:session_cb()
bluetoothd[1894]: profiles/audio/avdtp.c:avdtp_parse_cmd() Received
    SET_CONFIGURATION_CMD
bluetoothd[1894]: profiles/audio/a2dp.c:endpoint_setconf_ind() Source
    0x6c5000: Set_Configuration_Ind
bluetoothd[1894]: profiles/audio/avdtp.c:avdtp_ref() 0x6df360: ref=1
bluetoothd[1894]: profiles/audio/a2dp.c:setup_ref() 0x6d32b0: ref=1
process 1894: arguments to dbus_pending_call_set_notify() were incorrect,
     assertion "pending != NULL" failed in file dbus-pending-call.c line
     636.
This is normally a bug in some application using the D-Bus library.
---
 gdbus/object.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/gdbus/object.c b/gdbus/object.c
index 0822fe8..268fed5 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -1510,11 +1510,20 @@ gboolean g_dbus_send_message_with_reply(DBusConnection *connection,
 					DBusMessage *message,
 					DBusPendingCall **call, int timeout)
 {
+	dbus_bool_t ret;
+
 	/* Flush pending signal to guarantee message order */
 	g_dbus_flush(connection);
 
-	return dbus_connection_send_with_reply(connection, message, call,
+	ret = dbus_connection_send_with_reply(connection, message, call,
 								timeout);
+
+	if (ret == TRUE && call != NULL && *call == NULL) {
+		error("Unable to send message (passing fd blocked?)");
+		return FALSE;
+	}
+
+	return ret;
 }
 
 gboolean g_dbus_send_error_valist(DBusConnection *connection,
-- 
1.8.4


^ permalink raw reply related

* [PATCH] Bluetooth: ath3k: enlarge the usb urb timeout
From: AceLan Kao @ 2013-09-26  5:02 UTC (permalink / raw)
  To: linux-bluetooth, Marcel Holtmann, Gustavo Padovan, Johan Hedberg

We encountered a issue that the time is not sufficient to load the firmware.

[   14.646698] Bluetooth: Error in firmware loading err = -110,len = 448, size = 4096
[   14.646710] Bluetooth: Loading patch file failed
[   14.646719] ath3k: probe of 3-4:1.0 failed with error -110

And according to the log, it needs 10 times time to finish the transfer,
so, enlarge the timeout 10 times to overcome it.

Signed-off-by: AceLan Kao <acelan.kao@canonical.com>
---
 drivers/bluetooth/ath3k.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/bluetooth/ath3k.c b/drivers/bluetooth/ath3k.c
index a12b923..2047deb 100644
--- a/drivers/bluetooth/ath3k.c
+++ b/drivers/bluetooth/ath3k.c
@@ -49,6 +49,8 @@
 #define ATH3K_XTAL_FREQ_19P2			0x02
 #define ATH3K_NAME_LEN				0xFF
 
+#define USB_URB_TIMEOUT				30000
+
 struct ath3k_version {
 	unsigned int	rom_version;
 	unsigned int	build_version;
@@ -182,7 +184,7 @@ static int ath3k_load_firmware(struct usb_device *udev,
 		memcpy(send_buf, firmware->data + sent, size);
 
 		err = usb_bulk_msg(udev, pipe, send_buf, size,
-					&len, 3000);
+					&len, USB_URB_TIMEOUT);
 
 		if (err || (len != size)) {
 			BT_ERR("Error in firmware loading err = %d,"
@@ -279,7 +281,7 @@ static int ath3k_load_fwfile(struct usb_device *udev,
 		memcpy(send_buf, firmware->data + sent, size);
 
 		err = usb_bulk_msg(udev, pipe, send_buf, size,
-					&len, 3000);
+					&len, USB_URB_TIMEOUT);
 		if (err || (len != size)) {
 			BT_ERR("Error in firmware loading err = %d,"
 				"len = %d, size = %d", err, len, size);
-- 
1.8.3.2

^ permalink raw reply related

* Re: [PATCH v5 1/2] Bluetooth: btmrvl: add setup handler
From: Marcel Holtmann @ 2013-09-26  1:45 UTC (permalink / raw)
  To: Bing Zhao
  Cc: Johan Hedberg, linux-bluetooth@vger.kernel.org development,
	Gustavo F. Padovan, linux-wireless@vger.kernel.org Wireless,
	Mike Frysinger, Hyuckjoo Lee, Amitkumar Karwar
In-Reply-To: <477F20668A386D41ADCC57781B1F70430F45077E5B@SC-VEXCH1.marvell.com>

Hi Bing,

>> 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
> 
> We have tested your patch. Yes, it fixes the problem. Thanks!

then lets get a proper version with full commit message explaining the issue merged upstream. As I said, this is a real bug we need to fix.

>> 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.
> 
> Do you think the following change helps?
> 
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 3d9f02b..24814b0 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -1665,7 +1665,7 @@ static void hci_power_on(struct work_struct *work)
> 	BT_DBG("%s", hdev->name);
> 
> 	err = hci_dev_open(hdev->id);
> -	if (err < 0) {
> +	if (err < 0 && err != -EALREADY) {
> 		mgmt_set_powered_failed(hdev, err);
> 		return;
> 	}

I am more and more convinced that we might just need to disable certain ioctl in case there is a mgmt socket/client present and HCI_MGMT is set. Trying to be graceful with legacy ioctl that BlueZ 5 will never use anymore seems to come at a too high cost. I rather fail on the legacy commands with a proper error than having mgmt interface behave inconsistent.

We might actually be able to keep HCIDEVUP and HCIDEVDOWN around, but then they need to behave like mgmt set powered command. So both commands need to go through the same mgmt engine for this.

Regards

Marcel


^ permalink raw reply

* RE: [PATCH v5 1/2] Bluetooth: btmrvl: add setup handler
From: Bing Zhao @ 2013-09-25 23:23 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,

> 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

We have tested your patch. Yes, it fixes the problem. Thanks!

> 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.

Do you think the following change helps?

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 3d9f02b..24814b0 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1665,7 +1665,7 @@ static void hci_power_on(struct work_struct *work)
 	BT_DBG("%s", hdev->name);
=20
 	err =3D hci_dev_open(hdev->id);
-	if (err < 0) {
+	if (err < 0 && err !=3D -EALREADY) {
 		mgmt_set_powered_failed(hdev, err);
 		return;
 	}

Thanks,
Bing

^ permalink raw reply related

* Re: [PATCH v2 7/7] Bluetooth: Add new mgmt_set_advertising command
From: Gustavo Padovan @ 2013-09-25 17:30 UTC (permalink / raw)
  To: johan.hedberg; +Cc: linux-bluetooth
In-Reply-To: <1380104770-8022-8-git-send-email-johan.hedberg@gmail.com>

Hi Johan,

2013-09-25 johan.hedberg@gmail.com <johan.hedberg@gmail.com>:

> From: Johan Hedberg <johan.hedberg@intel.com>
> 
> This patch adds a new mgmt command for enabling and disabling
> LE advertising. The command depends on the LE setting being enabled
> first and will return a "rejected" response otherwise. The patch also
> adds safeguards so that there will ever only be one set_le or
> set_advertising command pending per adapter.
> 
> The response handling and new_settings event sending is done in an
> asynchronous request callback, meaning raw HCI access from user space to
> enable advertising (e.g. hciconfig leadv) will not trigger the
> new_settings event. This is intentional since trying to support mixed
> raw HCI and mgmt access would mean adding extra state tracking or new
> helper functions, essentially negating the benefit of using the
> asynchronous request framework. The HCI_LE_ENABLED and HCI_LE_PERIPHERAL
> flags however are updated correctly even with raw HCI access so this
> will not completely break subsequent access over mgmt.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
>  include/net/bluetooth/mgmt.h |  2 +
>  net/bluetooth/mgmt.c         | 97 +++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 98 insertions(+), 1 deletion(-)

All patches have been applied to bluetooth-next. Thanks.

	Gustavo

^ permalink raw reply

* Re: [PATCH v2 7/7] Bluetooth: Add new mgmt_set_advertising command
From: Marcel Holtmann @ 2013-09-25 10:31 UTC (permalink / raw)
  To: johan.hedberg; +Cc: linux-bluetooth
In-Reply-To: <1380104770-8022-8-git-send-email-johan.hedberg@gmail.com>

Hi Johan,

> This patch adds a new mgmt command for enabling and disabling
> LE advertising. The command depends on the LE setting being enabled
> first and will return a "rejected" response otherwise. The patch also
> adds safeguards so that there will ever only be one set_le or
> set_advertising command pending per adapter.
> 
> The response handling and new_settings event sending is done in an
> asynchronous request callback, meaning raw HCI access from user space to
> enable advertising (e.g. hciconfig leadv) will not trigger the
> new_settings event. This is intentional since trying to support mixed
> raw HCI and mgmt access would mean adding extra state tracking or new
> helper functions, essentially negating the benefit of using the
> asynchronous request framework. The HCI_LE_ENABLED and HCI_LE_PERIPHERAL
> flags however are updated correctly even with raw HCI access so this
> will not completely break subsequent access over mgmt.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> include/net/bluetooth/mgmt.h |  2 +
> net/bluetooth/mgmt.c         | 97 +++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 98 insertions(+), 1 deletion(-)

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

Regards

Marcel


^ permalink raw reply

* Re: [PATCH v2 2/7] Bluetooth: Clean up socket locking in l2cap_sock_recvmsg
From: Marcel Holtmann @ 2013-09-25 10:30 UTC (permalink / raw)
  To: johan.hedberg; +Cc: linux-bluetooth
In-Reply-To: <1380104770-8022-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 | 4 ++--
> 1 file changed, 2 insertions(+), 2 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: Johan Hedberg @ 2013-09-25 10:28 UTC (permalink / raw)
  To: Anderson Lizardo, BlueZ development
In-Reply-To: <20130924172120.GA30191@x220.p-661hnu-f1>

Hi,

On Tue, Sep 24, 2013, Johan Hedberg wrote:
> 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.

So I did take another look and this gets tricky since we don't just need
to emit a new_settings, but also ensure that it does not get emitted on
the mgmt socket that may have caused the command to be sent. Therefore,
I decided against trying to have "full" support of mixing mgmt with raw
HCI. What I did do is ensure that our dev_flags remain correct by using
the hci_event.c handler to set them. This ensures that subsequent mgmt
commands still work even though bluetoothd may not always be completely
up to date with these settings.

Johan

^ permalink raw reply

* [PATCH v2 7/7] Bluetooth: Add new mgmt_set_advertising command
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>

This patch adds a new mgmt command for enabling and disabling
LE advertising. The command depends on the LE setting being enabled
first and will return a "rejected" response otherwise. The patch also
adds safeguards so that there will ever only be one set_le or
set_advertising command pending per adapter.

The response handling and new_settings event sending is done in an
asynchronous request callback, meaning raw HCI access from user space to
enable advertising (e.g. hciconfig leadv) will not trigger the
new_settings event. This is intentional since trying to support mixed
raw HCI and mgmt access would mean adding extra state tracking or new
helper functions, essentially negating the benefit of using the
asynchronous request framework. The HCI_LE_ENABLED and HCI_LE_PERIPHERAL
flags however are updated correctly even with raw HCI access so this
will not completely break subsequent access over mgmt.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 include/net/bluetooth/mgmt.h |  2 +
 net/bluetooth/mgmt.c         | 97 +++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 98 insertions(+), 1 deletion(-)

diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index 6cc72b6..421d763 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -352,6 +352,8 @@ struct mgmt_cp_set_device_id {
 } __packed;
 #define MGMT_SET_DEVICE_ID_SIZE		8
 
+#define MGMT_OP_SET_ADVERTISING		0x0029
+
 #define MGMT_EV_CMD_COMPLETE		0x0001
 struct mgmt_ev_cmd_complete {
 	__le16	opcode;
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 9a2faa3..1b5b10f 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -76,6 +76,7 @@ static const u16 mgmt_commands[] = {
 	MGMT_OP_BLOCK_DEVICE,
 	MGMT_OP_UNBLOCK_DEVICE,
 	MGMT_OP_SET_DEVICE_ID,
+	MGMT_OP_SET_ADVERTISING,
 };
 
 static const u16 mgmt_events[] = {
@@ -1431,7 +1432,8 @@ static int set_le(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
 		goto unlock;
 	}
 
-	if (mgmt_pending_find(MGMT_OP_SET_LE, hdev)) {
+	if (mgmt_pending_find(MGMT_OP_SET_LE, hdev) ||
+	    mgmt_pending_find(MGMT_OP_SET_ADVERTISING, hdev)) {
 		err = cmd_status(sk, hdev->id, MGMT_OP_SET_LE,
 				 MGMT_STATUS_BUSY);
 		goto unlock;
@@ -3136,6 +3138,98 @@ static int set_device_id(struct sock *sk, struct hci_dev *hdev, void *data,
 	return err;
 }
 
+static void set_advertising_complete(struct hci_dev *hdev, u8 status)
+{
+	struct cmd_lookup match = { NULL, hdev };
+
+	if (status) {
+		u8 mgmt_err = mgmt_status(status);
+
+		mgmt_pending_foreach(MGMT_OP_SET_ADVERTISING, hdev,
+				     cmd_status_rsp, &mgmt_err);
+		return;
+	}
+
+	mgmt_pending_foreach(MGMT_OP_SET_ADVERTISING, hdev, settings_rsp,
+			     &match);
+
+	new_settings(hdev, match.sk);
+
+	if (match.sk)
+		sock_put(match.sk);
+}
+
+static int set_advertising(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
+{
+	struct mgmt_mode *cp = data;
+	struct pending_cmd *cmd;
+	struct hci_request req;
+	u8 val, enabled;
+	int err;
+
+	BT_DBG("request for %s", hdev->name);
+
+	if (!lmp_le_capable(hdev))
+		return cmd_status(sk, hdev->id, MGMT_OP_SET_ADVERTISING,
+				  MGMT_STATUS_NOT_SUPPORTED);
+
+	if (!test_bit(HCI_LE_ENABLED, &hdev->dev_flags))
+		return cmd_status(sk, hdev->id, MGMT_OP_SET_ADVERTISING,
+				  MGMT_STATUS_REJECTED);
+
+	if (cp->val != 0x00 && cp->val != 0x01)
+		return cmd_status(sk, hdev->id, MGMT_OP_SET_ADVERTISING,
+				  MGMT_STATUS_INVALID_PARAMS);
+
+	hci_dev_lock(hdev);
+
+	val = !!cp->val;
+	enabled = test_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags);
+
+	if (!hdev_is_powered(hdev) || val == enabled) {
+		bool changed = false;
+
+		if (val != test_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags)) {
+			change_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags);
+			changed = true;
+		}
+
+		err = send_settings_rsp(sk, MGMT_OP_SET_ADVERTISING, hdev);
+		if (err < 0)
+			goto unlock;
+
+		if (changed)
+			err = new_settings(hdev, sk);
+
+		goto unlock;
+	}
+
+	if (mgmt_pending_find(MGMT_OP_SET_ADVERTISING, hdev) ||
+	    mgmt_pending_find(MGMT_OP_SET_LE, hdev)) {
+		err = cmd_status(sk, hdev->id, MGMT_OP_SET_ADVERTISING,
+				 MGMT_STATUS_BUSY);
+		goto unlock;
+	}
+
+	cmd = mgmt_pending_add(sk, MGMT_OP_SET_ADVERTISING, hdev, data, len);
+	if (!cmd) {
+		err = -ENOMEM;
+		goto unlock;
+	}
+
+	hci_req_init(&req, hdev);
+
+	hci_req_add(&req, HCI_OP_LE_SET_ADV_ENABLE, sizeof(val), &val);
+
+	err = hci_req_run(&req, set_advertising_complete);
+	if (err < 0)
+		mgmt_pending_remove(cmd);
+
+unlock:
+	hci_dev_unlock(hdev);
+	return err;
+}
+
 static void fast_connectable_complete(struct hci_dev *hdev, u8 status)
 {
 	struct pending_cmd *cmd;
@@ -3347,6 +3441,7 @@ static const struct mgmt_handler {
 	{ block_device,           false, MGMT_BLOCK_DEVICE_SIZE },
 	{ unblock_device,         false, MGMT_UNBLOCK_DEVICE_SIZE },
 	{ set_device_id,          false, MGMT_SET_DEVICE_ID_SIZE },
+	{ set_advertising,        false, MGMT_SETTING_SIZE },
 };
 
 
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH v2 6/7] Bluetooth: Add new mgmt setting for LE advertising
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>

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>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
---
 include/net/bluetooth/mgmt.h |  1 +
 net/bluetooth/hci_event.c    |  1 +
 net/bluetooth/mgmt.c         | 21 ++++++++++++++++++++-
 3 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index 9944c3e..6cc72b6 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -93,6 +93,7 @@ struct mgmt_rp_read_index_list {
 #define MGMT_SETTING_BREDR		0x00000080
 #define MGMT_SETTING_HS			0x00000100
 #define MGMT_SETTING_LE			0x00000200
+#define MGMT_SETTING_ADVERTISING	0x00000400
 
 #define MGMT_OP_READ_INFO		0x0004
 #define MGMT_READ_INFO_SIZE		0
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 48db81f..917c7c8 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1000,6 +1000,7 @@ static void hci_cc_write_le_host_supported(struct hci_dev *hdev,
 		} else {
 			hdev->features[1][0] &= ~LMP_HOST_LE;
 			clear_bit(HCI_LE_ENABLED, &hdev->dev_flags);
+			clear_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags);
 		}
 
 		if (sent->simul)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 4c3984e..9a2faa3 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -384,8 +384,10 @@ static u32 get_supported_settings(struct hci_dev *hdev)
 	if (enable_hs)
 		settings |= MGMT_SETTING_HS;
 
-	if (lmp_le_capable(hdev))
+	if (lmp_le_capable(hdev)) {
 		settings |= MGMT_SETTING_LE;
+		settings |= MGMT_SETTING_ADVERTISING;
+	}
 
 	return settings;
 }
@@ -424,6 +426,9 @@ static u32 get_current_settings(struct hci_dev *hdev)
 	if (test_bit(HCI_HS_ENABLED, &hdev->dev_flags))
 		settings |= MGMT_SETTING_HS;
 
+	if (test_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags))
+		settings |= MGMT_SETTING_ADVERTISING;
+
 	return settings;
 }
 
@@ -1411,6 +1416,11 @@ static int set_le(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
 			changed = true;
 		}
 
+		if (!val && test_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags)) {
+			clear_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags);
+			changed = true;
+		}
+
 		err = send_settings_rsp(sk, MGMT_OP_SET_LE, hdev);
 		if (err < 0)
 			goto unlock;
@@ -1442,6 +1452,9 @@ static int set_le(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
 
 	hci_req_init(&req, hdev);
 
+	if (test_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags) && !val)
+		hci_req_add(&req, HCI_OP_LE_SET_ADV_ENABLE, sizeof(val), &val);
+
 	hci_req_add(&req, HCI_OP_WRITE_LE_HOST_SUPPORTED, sizeof(hci_cp),
 		    &hci_cp);
 
@@ -3517,6 +3530,12 @@ static int powered_update_hci(struct hci_dev *hdev)
 				    sizeof(cp), &cp);
 	}
 
+	if (test_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags)) {
+		u8 adv = 0x01;
+
+		hci_req_add(&req, HCI_OP_LE_SET_ADV_ENABLE, sizeof(adv), &adv);
+	}
+
 	link_sec = test_bit(HCI_LINK_SECURITY, &hdev->dev_flags);
 	if (link_sec != test_bit(HCI_AUTH, &hdev->flags))
 		hci_req_add(&req, HCI_OP_WRITE_AUTH_ENABLE,
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH v2 5/7] Bluetooth: Use async request for LE enable/disable
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>

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>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
---
 include/net/bluetooth/hci_core.h |  1 -
 net/bluetooth/hci_event.c        | 11 +++----
 net/bluetooth/mgmt.c             | 67 ++++++++++++++++------------------------
 3 files changed, 32 insertions(+), 47 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 3ede820..26cc9f7 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1168,7 +1168,6 @@ int mgmt_set_class_of_dev_complete(struct hci_dev *hdev, u8 *dev_class,
 int mgmt_set_local_name_complete(struct hci_dev *hdev, u8 *name, u8 status);
 int mgmt_read_local_oob_data_reply_complete(struct hci_dev *hdev, u8 *hash,
 					    u8 *randomizer, u8 status);
-int mgmt_le_enable_complete(struct hci_dev *hdev, u8 enable, u8 status);
 int mgmt_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
 		      u8 addr_type, u8 *dev_class, s8 rssi, u8 cfm_name,
 		      u8 ssp, u8 *eir, u16 eir_len);
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 94aab73..48db81f 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -994,20 +994,19 @@ static void hci_cc_write_le_host_supported(struct hci_dev *hdev,
 		return;
 
 	if (!status) {
-		if (sent->le)
+		if (sent->le) {
 			hdev->features[1][0] |= LMP_HOST_LE;
-		else
+			set_bit(HCI_LE_ENABLED, &hdev->dev_flags);
+		} else {
 			hdev->features[1][0] &= ~LMP_HOST_LE;
+			clear_bit(HCI_LE_ENABLED, &hdev->dev_flags);
+		}
 
 		if (sent->simul)
 			hdev->features[1][0] |= LMP_HOST_LE_BREDR;
 		else
 			hdev->features[1][0] &= ~LMP_HOST_LE_BREDR;
 	}
-
-	if (test_bit(HCI_MGMT, &hdev->dev_flags) &&
-	    !test_bit(HCI_INIT, &hdev->flags))
-		mgmt_le_enable_complete(hdev, sent->le, status);
 }
 
 static void hci_cc_write_remote_amp_assoc(struct hci_dev *hdev,
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 61d4b19..4c3984e 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1354,11 +1354,32 @@ static int set_hs(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
 	return send_settings_rsp(sk, MGMT_OP_SET_HS, hdev);
 }
 
+static void le_enable_complete(struct hci_dev *hdev, u8 status)
+{
+	struct cmd_lookup match = { NULL, hdev };
+
+	if (status) {
+		u8 mgmt_err = mgmt_status(status);
+
+		mgmt_pending_foreach(MGMT_OP_SET_LE, hdev, cmd_status_rsp,
+				     &mgmt_err);
+		return;
+	}
+
+	mgmt_pending_foreach(MGMT_OP_SET_LE, hdev, settings_rsp, &match);
+
+	new_settings(hdev, match.sk);
+
+	if (match.sk)
+		sock_put(match.sk);
+}
+
 static int set_le(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
 {
 	struct mgmt_mode *cp = data;
 	struct hci_cp_write_le_host_supported hci_cp;
 	struct pending_cmd *cmd;
+	struct hci_request req;
 	int err;
 	u8 val, enabled;
 
@@ -1419,8 +1440,12 @@ static int set_le(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
 		hci_cp.simul = lmp_le_br_capable(hdev);
 	}
 
-	err = hci_send_cmd(hdev, HCI_OP_WRITE_LE_HOST_SUPPORTED, sizeof(hci_cp),
-			   &hci_cp);
+	hci_req_init(&req, hdev);
+
+	hci_req_add(&req, HCI_OP_WRITE_LE_HOST_SUPPORTED, sizeof(hci_cp),
+		    &hci_cp);
+
+	err = hci_req_run(&req, le_enable_complete);
 	if (err < 0)
 		mgmt_pending_remove(cmd);
 
@@ -4141,44 +4166,6 @@ int mgmt_read_local_oob_data_reply_complete(struct hci_dev *hdev, u8 *hash,
 	return err;
 }
 
-int mgmt_le_enable_complete(struct hci_dev *hdev, u8 enable, u8 status)
-{
-	struct cmd_lookup match = { NULL, hdev };
-	bool changed = false;
-	int err = 0;
-
-	if (status) {
-		u8 mgmt_err = mgmt_status(status);
-
-		if (enable && test_and_clear_bit(HCI_LE_ENABLED,
-						 &hdev->dev_flags))
-			err = new_settings(hdev, NULL);
-
-		mgmt_pending_foreach(MGMT_OP_SET_LE, hdev, cmd_status_rsp,
-				     &mgmt_err);
-
-		return err;
-	}
-
-	if (enable) {
-		if (!test_and_set_bit(HCI_LE_ENABLED, &hdev->dev_flags))
-			changed = true;
-	} else {
-		if (test_and_clear_bit(HCI_LE_ENABLED, &hdev->dev_flags))
-			changed = true;
-	}
-
-	mgmt_pending_foreach(MGMT_OP_SET_LE, hdev, settings_rsp, &match);
-
-	if (changed)
-		err = new_settings(hdev, match.sk);
-
-	if (match.sk)
-		sock_put(match.sk);
-
-	return err;
-}
-
 int mgmt_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
 		      u8 addr_type, u8 *dev_class, s8 rssi, u8 cfm_name, u8
 		      ssp, u8 *eir, u16 eir_len)
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH v2 4/7] Bluetooth: Move mgmt response convenience functions to a better location
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 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>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
---
 net/bluetooth/mgmt.c | 60 ++++++++++++++++++++++++++--------------------------
 1 file changed, 30 insertions(+), 30 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 85bfa21..61d4b19 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -886,6 +886,36 @@ static int new_settings(struct hci_dev *hdev, struct sock *skip)
 	return mgmt_event(MGMT_EV_NEW_SETTINGS, hdev, &ev, sizeof(ev), skip);
 }
 
+struct cmd_lookup {
+	struct sock *sk;
+	struct hci_dev *hdev;
+	u8 mgmt_status;
+};
+
+static void settings_rsp(struct pending_cmd *cmd, void *data)
+{
+	struct cmd_lookup *match = data;
+
+	send_settings_rsp(cmd->sk, cmd->opcode, match->hdev);
+
+	list_del(&cmd->list);
+
+	if (match->sk == NULL) {
+		match->sk = cmd->sk;
+		sock_hold(match->sk);
+	}
+
+	mgmt_pending_free(cmd);
+}
+
+static void cmd_status_rsp(struct pending_cmd *cmd, void *data)
+{
+	u8 *status = data;
+
+	cmd_status(cmd->sk, cmd->index, cmd->opcode, *status);
+	mgmt_pending_remove(cmd);
+}
+
 static int set_discoverable(struct sock *sk, struct hci_dev *hdev, void *data,
 			    u16 len)
 {
@@ -3374,14 +3404,6 @@ done:
 	return err;
 }
 
-static void cmd_status_rsp(struct pending_cmd *cmd, void *data)
-{
-	u8 *status = data;
-
-	cmd_status(cmd->sk, cmd->index, cmd->opcode, *status);
-	mgmt_pending_remove(cmd);
-}
-
 int mgmt_index_added(struct hci_dev *hdev)
 {
 	if (!mgmt_valid_hdev(hdev))
@@ -3402,28 +3424,6 @@ int mgmt_index_removed(struct hci_dev *hdev)
 	return mgmt_event(MGMT_EV_INDEX_REMOVED, hdev, NULL, 0, NULL);
 }
 
-struct cmd_lookup {
-	struct sock *sk;
-	struct hci_dev *hdev;
-	u8 mgmt_status;
-};
-
-static void settings_rsp(struct pending_cmd *cmd, void *data)
-{
-	struct cmd_lookup *match = data;
-
-	send_settings_rsp(cmd->sk, cmd->opcode, match->hdev);
-
-	list_del(&cmd->list);
-
-	if (match->sk == NULL) {
-		match->sk = cmd->sk;
-		sock_hold(match->sk);
-	}
-
-	mgmt_pending_free(cmd);
-}
-
 static void set_bredr_scan(struct hci_request *req)
 {
 	struct hci_dev *hdev = req->hdev;
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH v2 3/7] Bluetooth: Fix busy return for mgmt_set_powered in some cases
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>

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>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
---
 net/bluetooth/mgmt.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 3070e77..85bfa21 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -807,6 +807,12 @@ static int set_powered(struct sock *sk, struct hci_dev *hdev, void *data,
 
 	hci_dev_lock(hdev);
 
+	if (mgmt_pending_find(MGMT_OP_SET_POWERED, hdev)) {
+		err = cmd_status(sk, hdev->id, MGMT_OP_SET_POWERED,
+				 MGMT_STATUS_BUSY);
+		goto failed;
+	}
+
 	if (test_and_clear_bit(HCI_AUTO_OFF, &hdev->dev_flags)) {
 		cancel_delayed_work(&hdev->power_off);
 
@@ -823,12 +829,6 @@ static int set_powered(struct sock *sk, struct hci_dev *hdev, void *data,
 		goto failed;
 	}
 
-	if (mgmt_pending_find(MGMT_OP_SET_POWERED, hdev)) {
-		err = cmd_status(sk, hdev->id, MGMT_OP_SET_POWERED,
-				 MGMT_STATUS_BUSY);
-		goto failed;
-	}
-
 	cmd = mgmt_pending_add(sk, MGMT_OP_SET_POWERED, hdev, data, len);
 	if (!cmd) {
 		err = -ENOMEM;
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH v2 2/7] Bluetooth: Clean up socket locking in l2cap_sock_recvmsg
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>

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 | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index ad95b42..c85537c 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -805,8 +805,8 @@ 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;
+		err = 0;
+		goto done;
 	}
 
 	release_sock(sk);
-- 
1.8.3.1


^ permalink raw reply related

* [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


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