* Re: usb device removed from sysfs before input children devices
From: Karl Relton @ 2013-01-15 20:27 UTC (permalink / raw)
To: linux-usb, linux-bluetooth
In-Reply-To: <1357759635.2872.9.camel@dellpc>
On Wed, 2013-01-09 at 19:27 +0000, Karl Relton wrote:
> On coming out of suspend my usb bluetooth adaptor is being reset by the
> system.
>
> In linux 3.7 the usb devices are being removed from the sysfs tree
> first, and then the various 'child' devices (like my bluetooth mouse &
> keyboard related devices) afterwards. This is causing the udev events
> for the input devices to have 'orphaned' sysfs paths in the udev events.
>
> This in turn means the Xorg evdev driver does not recognise the events,
> and so doesn't see the removal of the input devices.
>
> This has been picked by some downstream distributions, e.g. see this
> thread by Google Chrome developers:
>
> http://code.google.com/p/chromium-os/issues/detail?id=33813
>
> Back on linux 3.2 this was not the case. The usb adaptor was reset, but
> device removal was orderly: first the input devices (will full paths in
> the udev events), then the usb devices walking up the tree.
>
>
> To illustrate the issue, here is the output of 'udevadm monitor' in 3.7:
>
> udevadm monitor
> monitor will print the received events for:
> KERNEL - the kernel uevent
> KERNEL[2203.173080] remove /devices/pci0000:00/0000:00:1a.0/usb3/3-2/3-2:1.0/bluetooth/hci0/rfkill2 (rfkill)
> KERNEL[2203.173148] remove /devices/pci0000:00/0000:00:1a.0/usb3/3-2/3-2:1.0/bluetooth/hci0 (bluetooth)
> KERNEL[2203.173420] remove /devices/pci0000:00/0000:00:1a.0/usb3/3-2/3-2:1.0 (usb)
> KERNEL[2203.173451] remove /devices/pci0000:00/0000:00:1a.0/usb3/3-2/3-2:1.1 (usb)
> KERNEL[2203.173475] remove /devices/pci0000:00/0000:00:1a.0/usb3/3-2/3-2:1.2 (usb)
> KERNEL[2203.173693] remove /devices/pci0000:00/0000:00:1a.0/usb3/3-2 (usb)
> KERNEL[2213.152339] remove /hci0/hci0:46/input14/mouse2 (input)
> KERNEL[2213.160374] remove /hci0/hci0:46/input14/event10 (input)
> KERNEL[2213.168366] remove /hci0/hci0:46/input14 (input)
> KERNEL[2213.169058] remove /hci0/hci0:46/0005:050D:0031.0005/hidraw/hidraw0 (hidraw)
> KERNEL[2213.169198] remove /hci0/hci0:46/0005:050D:0031.0005 (hid)
> KERNEL[2213.169242] remove /hci0/hci0:46 (bluetooth)
> KERNEL[2218.176527] remove /hci0/hci0:49/input13/event11 (input)
> KERNEL[2218.180403] remove /hci0/hci0:49/input13 (input)
> KERNEL[2218.180481] remove /hci0/hci0:49/0005:05AC:0256.0004/hidraw/hidraw1 (hidraw)
> KERNEL[2218.180538] remove /hci0/hci0:49/0005:05AC:0256.0004 (hid)
> KERNEL[2218.182005] remove /hci0/hci0:49 (bluetooth)
>
> See how the usb devices are moved first, and then the input/bluetooth related stuff
> with path-heads removed (paths are now /hci0/... instead of /devices/...)
>
>
> Here is the equiv sequence back in 3.2:
>
> KERNEL[158.378301] remove /devices/pci0000:00/0000:00:1a.0/usb3/3-2/3-2:1.0/bluetooth/hci0/hci0:49/input11/mouse2 (input)
> KERNEL[158.388283] remove /devices/pci0000:00/0000:00:1a.0/usb3/3-2/3-2:1.0/bluetooth/hci0/hci0:49/input11/event11 (input)
> KERNEL[158.409885] remove /devices/pci0000:00/0000:00:1a.0/usb3/3-2/3-2:1.0/bluetooth/hci0/hci0:49/input11 (input)
> KERNEL[158.411565] remove /devices/pci0000:00/0000:00:1a.0/usb3/3-2/3-2:1.0/bluetooth/hci0/hci0:49/0005:050D:0031.0002/hidraw/hidraw1 (hidraw)
> KERNEL[158.411598] remove /devices/pci0000:00/0000:00:1a.0/usb3/3-2/3-2:1.0/bluetooth/hci0/hci0:49/0005:050D:0031.0002 (hid)
> KERNEL[158.411621] remove /devices/pci0000:00/0000:00:1a.0/usb3/3-2/3-2:1.0/bluetooth/hci0/hci0:49 (bluetooth)
> KERNEL[158.436894] remove /devices/pci0000:00/0000:00:1a.0/usb3/3-2/3-2:1.0/bluetooth/hci0/hci0:46/input10/event10 (input)
> KERNEL[158.452211] remove /devices/pci0000:00/0000:00:1a.0/usb3/3-2/3-2:1.0/bluetooth/hci0/hci0:46/input10 (input)
> KERNEL[158.452628] remove /devices/pci0000:00/0000:00:1a.0/usb3/3-2/3-2:1.0/bluetooth/hci0/hci0:46/0005:05AC:0256.0001/hidraw/hidraw0 (hidraw)
> KERNEL[158.452662] remove /devices/pci0000:00/0000:00:1a.0/usb3/3-2/3-2:1.0/bluetooth/hci0/hci0:46/0005:05AC:0256.0001 (hid)
> KERNEL[158.452752] remove /devices/pci0000:00/0000:00:1a.0/usb3/3-2/3-2:1.0/bluetooth/hci0/hci0:46 (bluetooth)
> KERNEL[158.629847] remove /devices/pci0000:00/0000:00:1a.0/usb3/3-2/3-2:1.0/bluetooth/hci0/rfkill2 (rfkill)
> KERNEL[158.629920] remove /devices/pci0000:00/0000:00:1a.0/usb3/3-2/3-2:1.0/bluetooth/hci0 (bluetooth)
> KERNEL[158.635562] remove /devices/pci0000:00/0000:00:1a.0/usb3/3-2/3-2:1.0 (usb)
> KERNEL[158.635701] remove /devices/pci0000:00/0000:00:1a.0/usb3/3-2/3-2:1.1 (usb)
> KERNEL[158.635807] remove /devices/pci0000:00/0000:00:1a.0/usb3/3-2/3-2:1.2 (usb)
> KERNEL[158.637238] remove /devices/pci0000:00/0000:00:1a.0/usb3/3-2 (usb)
>
>
> The end result (for the user) is that even when the bluetooth
> mouse/keyboard is re-added, Xorg ignores it - thinking it is some hoax
> duplicate device. The keyboard/mouse is then non-operational.
>
Instrumenting the code suggests that the issue arises in a race between:
hidp_session() in bluetooth/hidp/core.c
and
hci_unregister_dev() in bluetooth/hci_core.c
Basically hci_unregister_dev() does a hci_dev_do_close() which in turn
shuts down the hidp connection. This causes hidp_session() running in
another thread to go into its cleanup phase, which is where the input
children devices are unregistered.
HOWEVER:
1) For some reason input_unregister_device() seems to stall some 3-5
seconds on my system before removing the input device
2) In parallel hci_unregister_dev() ploughs on, and progresses to remove
its hdev (hci<n> device) regardless, without waiting for hidp_session()
to complete.
I can't figure out why there is such a delay for
input_unregister_device().
But even if it didn't have the delay, there would still be a race
between the two threads (hci_unregister_dev vs hidp_session). No attempt
seems to be made to wait for the latter, so conceivably it could still
progress on even if hidp_session wasn't unnecessarily held up?
Question: What are the symantics of the wake_...() functions? Does the
function(thread) calling wake_... not move on until all the threads
woken by the call have completed or gone back to sleep. Or can the
calling thread() start progressing in parallel on a multi-processor
system?
I ask because if the calling function has to wait for the woken threads,
that would give the appropriate 'wait' on hci_unregister_dev ... but
ONLY IF hidp_session doesn't sleep again for some reason (e.g. while
waiting on a mutex inside input_unregister_device() perhaps ...)
Karl
^ permalink raw reply
* Re: [PATCH BlueZ v2 01/11] transport: Initialize the "Volume" property
From: Luiz Augusto von Dentz @ 2013-01-15 18:12 UTC (permalink / raw)
To: João Paulo Rechi Vita
Cc: linux-bluetooth@vger.kernel.org, Vinicius Gomes, Claudio Takahasi,
Luiz Augusto Von Dentz
In-Reply-To: <1358179829-29684-1-git-send-email-jprvita@openbossa.org>
Hi Joao,
On Mon, Jan 14, 2013 at 6:10 PM, João Paulo Rechi Vita
<jprvita@openbossa.org> wrote:
> ---
> profiles/audio/transport.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c
> index a4370a5..8defc6f 100644
> --- a/profiles/audio/transport.c
> +++ b/profiles/audio/transport.c
> @@ -787,7 +787,6 @@ struct media_transport *media_transport_create(struct media_endpoint *endpoint,
> struct a2dp_transport *a2dp;
>
> a2dp = g_new0(struct a2dp_transport, 1);
> - a2dp->volume = -1;
>
> transport->resume = resume_a2dp;
> transport->suspend = suspend_a2dp;
> @@ -795,14 +794,17 @@ struct media_transport *media_transport_create(struct media_endpoint *endpoint,
> transport->data = a2dp;
> transport->destroy = destroy_a2dp;
>
> - if (strcasecmp(uuid, A2DP_SOURCE_UUID) == 0)
> + if (strcasecmp(uuid, A2DP_SOURCE_UUID) == 0) {
> + a2dp->volume = -1;
> transport->sink_watch = sink_add_state_cb(
> sink_state_changed,
> transport);
> - else
> + } else {
> + a2dp->volume = 127;
> transport->source_watch = source_add_state_cb(
> source_state_changed,
> transport);
> + }
> } else
> goto fail;
>
> --
> 1.7.11.7
>
This set is now upstream, thanks.
--
Luiz Augusto von Dentz
^ permalink raw reply
* Re: [PATCH v3 3/3] Bluetooth: Fix stop discovery while in STARTING state
From: Marcel Holtmann @ 2013-01-15 16:17 UTC (permalink / raw)
To: Jaganath Kanakkassery; +Cc: linux-bluetooth
In-Reply-To: <1358254892-11303-3-git-send-email-jaganath.k@samsung.com>
Hi Jaganath,
> If stop_discovery() is called when discovery state is STARTING, it
> will be failed currently. This patch fixes this.
>
> Signed-off-by: Jaganath Kanakkassery <jaganath.k@samsung.com>
> ---
> include/net/bluetooth/hci_core.h | 1 +
> net/bluetooth/hci_event.c | 14 ++++++++++++--
> net/bluetooth/mgmt.c | 12 +++++++++++-
> 3 files changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index d8f68c7..01c723a 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -64,6 +64,7 @@ struct discovery_state {
> DISCOVERY_RESOLVING,
> DISCOVERY_STOPPING,
> } state;
> + u8 discovering;
what is this double spaces for? And why not a bool. Or a better name.
> struct list_head all; /* All devices found during inquiry */
> struct list_head unknown; /* Name state not known */
> struct list_head resolve; /* Name needs to be resolved */
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 97b4828..c616cbf 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -1259,7 +1259,12 @@ static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
> set_bit(HCI_LE_SCAN, &hdev->dev_flags);
>
> hci_dev_lock(hdev);
> - hci_discovery_set_state(hdev, DISCOVERY_FINDING);
> + if (hdev->discovery.state == DISCOVERY_STOPPING) {
> + hci_cancel_le_scan(hdev);
> + mgmt_start_discovery_cancelled(hdev);
> + } else {
> + hci_discovery_set_state(hdev, DISCOVERY_FINDING);
> + }
> hci_dev_unlock(hdev);
> break;
>
> @@ -1375,7 +1380,12 @@ static void hci_cs_inquiry(struct hci_dev *hdev, __u8 status)
> set_bit(HCI_INQUIRY, &hdev->flags);
>
> hci_dev_lock(hdev);
> - hci_discovery_set_state(hdev, DISCOVERY_FINDING);
> + if (hdev->discovery.state == DISCOVERY_STOPPING) {
> + hci_cancel_inquiry(hdev);
> + mgmt_start_discovery_cancelled(hdev);
> + } else {
> + hci_discovery_set_state(hdev, DISCOVERY_FINDING);
> + }
> hci_dev_unlock(hdev);
> }
>
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 0db9d66..6dc275f 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -2394,7 +2394,8 @@ static int stop_discovery(struct sock *sk, struct hci_dev *hdev, void *data,
>
> hci_dev_lock(hdev);
>
> - if (!hci_discovery_active(hdev)) {
> + if (hdev->discovery.state != DISCOVERY_STARTING &&
> + !hci_discovery_active(hdev)) {
> err = cmd_complete(sk, hdev->id, MGMT_OP_STOP_DISCOVERY,
> MGMT_STATUS_REJECTED, &mgmt_cp->type,
> sizeof(mgmt_cp->type));
> @@ -2442,6 +2443,10 @@ static int stop_discovery(struct sock *sk, struct hci_dev *hdev, void *data,
>
> break;
>
> + case DISCOVERY_STARTING:
> + err = 0;
> + break;
> +
> default:
> BT_DBG("unknown discovery state %u", hdev->discovery.state);
> err = -EFAULT;
> @@ -3720,6 +3725,11 @@ int mgmt_discovering(struct hci_dev *hdev, u8 discovering)
> mgmt_pending_remove(cmd);
> }
>
> + if (hdev->discovery.discovering == discovering)
> + return 0;
> +
> + hdev->discovery.discovering = discovering;
> +
> memset(&ev, 0, sizeof(ev));
> ev.type = hdev->discovery.type;
> ev.discovering = discovering;
Regards
Marcel
^ permalink raw reply
* [PATCH 5/5] obexd: Add parameter Status to GetMessageListing response
From: Christian Fetzer @ 2013-01-15 13:56 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1358258204-4090-1-git-send-email-christian.fetzer@oss.bmw-carit.de>
From: Christian Fetzer <christian.fetzer@bmw-carit.de>
The parameter Status indicates the status of reception of the message.
It can be used to differentiate between messages with a reception status
of "completed", "fractioned" and "notification".
This parameter got lost when obexd was refactored to use of D-Bus properties.
The documentation and the parsing code for it was still there.
---
obexd/client/map.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/obexd/client/map.c b/obexd/client/map.c
index f9f7c4f..ea5681a 100644
--- a/obexd/client/map.c
+++ b/obexd/client/map.c
@@ -595,6 +595,24 @@ static gboolean get_size(const GDBusPropertyTable *property,
return TRUE;
}
+static gboolean reception_status_exists(const GDBusPropertyTable *property,
+ void *data)
+{
+ struct map_msg *msg = data;
+
+ return msg->status != NULL;
+}
+
+static gboolean get_reception_status(const GDBusPropertyTable *property,
+ DBusMessageIter *iter, void *data)
+{
+ struct map_msg *msg = data;
+
+ dbus_message_iter_append_basic(iter, DBUS_TYPE_STRING, &msg->status);
+
+ return TRUE;
+}
+
static gboolean get_attachment_size(const GDBusPropertyTable *property,
DBusMessageIter *iter, void *data)
{
@@ -732,6 +750,7 @@ static const GDBusPropertyTable map_msg_properties[] = {
{ "Type", "s", get_type, NULL, type_exists },
{ "Size", "t", get_size },
{ "Text", "b", get_text },
+ { "Status", "s", get_reception_status, NULL, reception_status_exists },
{ "AttachmentSize", "t", get_attachment_size },
{ "Priority", "b", get_priority },
{ "Read", "b", get_read, set_read },
--
1.8.1
^ permalink raw reply related
* [PATCH 4/5] obexd: Add parameter AttachmentSize to GetMessageListing response
From: Christian Fetzer @ 2013-01-15 13:56 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1358258204-4090-1-git-send-email-christian.fetzer@oss.bmw-carit.de>
From: Christian Fetzer <christian.fetzer@bmw-carit.de>
The parameter AttachmentSize indicates if the message contains
any attachment and their overall size (in bytes).
---
doc/obex-api.txt | 4 ++++
obexd/client/map.c | 22 ++++++++++++++++++++++
2 files changed, 26 insertions(+)
diff --git a/doc/obex-api.txt b/doc/obex-api.txt
index 9e17f1a..e2eb55f 100644
--- a/doc/obex-api.txt
+++ b/doc/obex-api.txt
@@ -601,6 +601,10 @@ Methods void SetFolder(string name)
Possible values: "complete",
"fractioned" and "notification"
+ uint64 AttachmentSize:
+
+ Message overall attachment size in bytes
+
boolean Priority:
Message priority flag
diff --git a/obexd/client/map.c b/obexd/client/map.c
index d32d6ac..f9f7c4f 100644
--- a/obexd/client/map.c
+++ b/obexd/client/map.c
@@ -115,6 +115,7 @@ struct map_msg {
char *type;
uint64_t size;
char *status;
+ uint64_t attachment_size;
uint8_t flags;
GDBusPendingPropertySet pending;
};
@@ -594,6 +595,17 @@ static gboolean get_size(const GDBusPropertyTable *property,
return TRUE;
}
+static gboolean get_attachment_size(const GDBusPropertyTable *property,
+ DBusMessageIter *iter, void *data)
+{
+ struct map_msg *msg = data;
+
+ dbus_message_iter_append_basic(iter, DBUS_TYPE_UINT64,
+ &msg->attachment_size);
+
+ return TRUE;
+}
+
static gboolean get_flag(const GDBusPropertyTable *property,
DBusMessageIter *iter, uint8_t flag,
void *data)
@@ -720,6 +732,7 @@ static const GDBusPropertyTable map_msg_properties[] = {
{ "Type", "s", get_type, NULL, type_exists },
{ "Size", "t", get_size },
{ "Text", "b", get_text },
+ { "AttachmentSize", "t", get_attachment_size },
{ "Priority", "b", get_priority },
{ "Read", "b", get_read, set_read },
{ "Sent", "b", get_sent },
@@ -846,6 +859,14 @@ static void parse_status(struct map_msg *msg, const char *value,
obex_dbus_dict_append(iter, "Status", DBUS_TYPE_STRING, &value);
}
+static void parse_attachment_size(struct map_msg *msg, const char *value,
+ DBusMessageIter *iter)
+{
+ msg->attachment_size = g_ascii_strtoll(value, NULL, 10);
+ obex_dbus_dict_append(iter, "AttachmentSize", DBUS_TYPE_UINT64,
+ &msg->attachment_size);
+}
+
static void parse_priority(struct map_msg *msg, const char *value,
DBusMessageIter *iter)
{
@@ -914,6 +935,7 @@ static struct map_msg_parser {
{ "size", parse_size },
{ "text", parse_text },
{ "reception_status", parse_status },
+ { "attachment_size", parse_attachment_size },
{ "priority", parse_priority },
{ "read", parse_read },
{ "sent", parse_sent },
--
1.8.1
^ permalink raw reply related
* [PATCH 3/5] obexd: Add parameter Text to GetMessageListing response
From: Christian Fetzer @ 2013-01-15 13:56 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1358258204-4090-1-git-send-email-christian.fetzer@oss.bmw-carit.de>
From: Christian Fetzer <christian.fetzer@bmw-carit.de>
The Text flag indicates if the message contains any textual content
or has binary content only.
---
doc/obex-api.txt | 7 +++++++
obexd/client/map.c | 22 ++++++++++++++++++++++
2 files changed, 29 insertions(+)
diff --git a/doc/obex-api.txt b/doc/obex-api.txt
index bb14fba..9e17f1a 100644
--- a/doc/obex-api.txt
+++ b/doc/obex-api.txt
@@ -587,6 +587,13 @@ Methods void SetFolder(string name)
Message size in bytes
+ boolean Text:
+
+ Message text flag
+
+ Specifies whether message has textual
+ content or is binary only
+
string Status:
Message reception status
diff --git a/obexd/client/map.c b/obexd/client/map.c
index a51aa5e..d32d6ac 100644
--- a/obexd/client/map.c
+++ b/obexd/client/map.c
@@ -99,6 +99,7 @@ struct map_data {
#define MAP_MSG_FLAG_READ 0x02
#define MAP_MSG_FLAG_SENT 0x04
#define MAP_MSG_FLAG_PROTECTED 0x08
+#define MAP_MSG_FLAG_TEXT 0x10
struct map_msg {
struct map_data *data;
@@ -629,6 +630,12 @@ static gboolean get_protected(const GDBusPropertyTable *property,
return get_flag(property, iter, MAP_MSG_FLAG_PROTECTED, data);
}
+static gboolean get_text(const GDBusPropertyTable *property,
+ DBusMessageIter *iter, void *data)
+{
+ return get_flag(property, iter, MAP_MSG_FLAG_TEXT, data);
+}
+
static void set_status(const GDBusPropertyTable *property,
DBusMessageIter *iter, GDBusPendingPropertySet id,
uint8_t status, void *data)
@@ -712,6 +719,7 @@ static const GDBusPropertyTable map_msg_properties[] = {
recipient_address_exists },
{ "Type", "s", get_type, NULL, type_exists },
{ "Size", "t", get_size },
+ { "Text", "b", get_text },
{ "Priority", "b", get_priority },
{ "Read", "b", get_read, set_read },
{ "Sent", "b", get_sent },
@@ -817,6 +825,19 @@ static void parse_size(struct map_msg *msg, const char *value,
obex_dbus_dict_append(iter, "Size", DBUS_TYPE_UINT64, &msg->size);
}
+static void parse_text(struct map_msg *msg, const char *value,
+ DBusMessageIter *iter)
+{
+ gboolean flag = strcasecmp(value, "no") != 0;
+
+ if (flag)
+ msg->flags |= MAP_MSG_FLAG_TEXT;
+ else
+ msg->flags &= ~MAP_MSG_FLAG_TEXT;
+
+ obex_dbus_dict_append(iter, "Text", DBUS_TYPE_BOOLEAN, &flag);
+}
+
static void parse_status(struct map_msg *msg, const char *value,
DBusMessageIter *iter)
{
@@ -891,6 +912,7 @@ static struct map_msg_parser {
{ "recipient_addressing", parse_recipient_address },
{ "type", parse_type },
{ "size", parse_size },
+ { "text", parse_text },
{ "reception_status", parse_status },
{ "priority", parse_priority },
{ "read", parse_read },
--
1.8.1
^ permalink raw reply related
* [PATCH 2/5] obexd: Move parse_size function in map.c
From: Christian Fetzer @ 2013-01-15 13:56 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1358258204-4090-1-git-send-email-christian.fetzer@oss.bmw-carit.de>
From: Christian Fetzer <christian.fetzer@bmw-carit.de>
This reorders msg_parsers and moves the function parse_size
to match the order in the MAP specification.
---
obexd/client/map.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/obexd/client/map.c b/obexd/client/map.c
index 54a6a7f..a51aa5e 100644
--- a/obexd/client/map.c
+++ b/obexd/client/map.c
@@ -810,19 +810,19 @@ static void parse_type(struct map_msg *msg, const char *value,
obex_dbus_dict_append(iter, "Type", DBUS_TYPE_STRING, &value);
}
-static void parse_status(struct map_msg *msg, const char *value,
+static void parse_size(struct map_msg *msg, const char *value,
DBusMessageIter *iter)
{
- g_free(msg->status);
- msg->status = g_strdup(value);
- obex_dbus_dict_append(iter, "Status", DBUS_TYPE_STRING, &value);
+ msg->size = g_ascii_strtoll(value, NULL, 10);
+ obex_dbus_dict_append(iter, "Size", DBUS_TYPE_UINT64, &msg->size);
}
-static void parse_size(struct map_msg *msg, const char *value,
+static void parse_status(struct map_msg *msg, const char *value,
DBusMessageIter *iter)
{
- msg->size = g_ascii_strtoll(value, NULL, 10);
- obex_dbus_dict_append(iter, "Size", DBUS_TYPE_UINT64, &msg->size);
+ g_free(msg->status);
+ msg->status = g_strdup(value);
+ obex_dbus_dict_append(iter, "Status", DBUS_TYPE_STRING, &value);
}
static void parse_priority(struct map_msg *msg, const char *value,
@@ -890,8 +890,8 @@ static struct map_msg_parser {
{ "recipient_name", parse_recipient },
{ "recipient_addressing", parse_recipient_address },
{ "type", parse_type },
- { "reception_status", parse_status },
{ "size", parse_size },
+ { "reception_status", parse_status },
{ "priority", parse_priority },
{ "read", parse_read },
{ "sent", parse_sent },
--
1.8.1
^ permalink raw reply related
* [PATCH 1/5] obexd: Add parameter SubjectLength to map_list_messages
From: Christian Fetzer @ 2013-01-15 13:56 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1358258204-4090-1-git-send-email-christian.fetzer@oss.bmw-carit.de>
From: Christian Fetzer <christian.fetzer@bmw-carit.de>
This parameter allows the client to request a maximum length of the
parameter "subject" in the messages listing.
---
doc/obex-api.txt | 9 +++++++--
obexd/client/map.c | 16 ++++++++++++++++
2 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/doc/obex-api.txt b/doc/obex-api.txt
index b2998de..bb14fba 100644
--- a/doc/obex-api.txt
+++ b/doc/obex-api.txt
@@ -539,8 +539,8 @@ Methods void SetFolder(string name)
Returns an array containing the messages found in the
given folder.
- Possible Filters: Offset, MaxCount, Fields, Type,
- PeriodStart, PeriodEnd, Status, Recipient, Sender,
+ Possible Filters: Offset, MaxCount, SubjectLength, Fields,
+ Type, PeriodStart, PeriodEnd, Status, Recipient, Sender,
Priority
Each message is represented by an object path followed
@@ -627,6 +627,11 @@ Filter: uint16 Offset:
Maximum number of items, default is 1024
+ uint8 SubjectLength:
+
+ Maximum length of the Subject property in the
+ message, default is 256
+
array{string} Fields:
Message fields, default is all values.
diff --git a/obexd/client/map.c b/obexd/client/map.c
index 29efd51..54a6a7f 100644
--- a/obexd/client/map.c
+++ b/obexd/client/map.c
@@ -1052,6 +1052,19 @@ fail:
return reply;
}
+static GObexApparam *parse_subject_length(GObexApparam *apparam,
+ DBusMessageIter *iter)
+{
+ guint8 num;
+
+ if (dbus_message_iter_get_arg_type(iter) != DBUS_TYPE_BYTE)
+ return NULL;
+
+ dbus_message_iter_get_basic(iter, &num);
+
+ return g_obex_apparam_set_uint8(apparam, MAP_AP_SUBJECTLENGTH, num);
+}
+
static uint64_t get_filter_mask(const char *filterstr)
{
int i;
@@ -1263,6 +1276,9 @@ static GObexApparam *parse_message_filters(GObexApparam *apparam,
} else if (strcasecmp(key, "MaxCount") == 0) {
if (parse_max_count(apparam, &value) == NULL)
return NULL;
+ } else if (strcasecmp(key, "SubjectLength") == 0) {
+ if (parse_subject_length(apparam, &value) == NULL)
+ return NULL;
} else if (strcasecmp(key, "Fields") == 0) {
if (parse_fields(apparam, &value) == NULL)
return NULL;
--
1.8.1
^ permalink raw reply related
* [PATCH 0/5] MAP client: Add missing parameters for GetMessageListing
From: Christian Fetzer @ 2013-01-15 13:56 UTC (permalink / raw)
To: linux-bluetooth
From: Christian Fetzer <christian.fetzer@bmw-carit.de>
This patchset adds the missing parameters Text, AttachmentSize and Status
as well as the filter SubjectLength.
Christian Fetzer (5):
obexd: Add parameter SubjectLength to map_list_messages
obexd: Move parse_size function in map.c
obexd: Add parameter Text to GetMessageListing response
obexd: Add parameter AttachmentSize to GetMessageListing response
obexd: Add parameter Status to GetMessageListing response
doc/obex-api.txt | 20 +++++++++++--
obexd/client/map.c | 87 +++++++++++++++++++++++++++++++++++++++++++++++++++---
2 files changed, 101 insertions(+), 6 deletions(-)
--
1.8.1
^ permalink raw reply
* [PATCH v3 3/3] Bluetooth: Fix stop discovery while in STARTING state
From: Jaganath Kanakkassery @ 2013-01-15 13:01 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Jaganath Kanakkassery
In-Reply-To: <1358254892-11303-1-git-send-email-jaganath.k@samsung.com>
If stop_discovery() is called when discovery state is STARTING, it
will be failed currently. This patch fixes this.
Signed-off-by: Jaganath Kanakkassery <jaganath.k@samsung.com>
---
include/net/bluetooth/hci_core.h | 1 +
net/bluetooth/hci_event.c | 14 ++++++++++++--
net/bluetooth/mgmt.c | 12 +++++++++++-
3 files changed, 24 insertions(+), 3 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index d8f68c7..01c723a 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -64,6 +64,7 @@ struct discovery_state {
DISCOVERY_RESOLVING,
DISCOVERY_STOPPING,
} state;
+ u8 discovering;
struct list_head all; /* All devices found during inquiry */
struct list_head unknown; /* Name state not known */
struct list_head resolve; /* Name needs to be resolved */
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 97b4828..c616cbf 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1259,7 +1259,12 @@ static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
set_bit(HCI_LE_SCAN, &hdev->dev_flags);
hci_dev_lock(hdev);
- hci_discovery_set_state(hdev, DISCOVERY_FINDING);
+ if (hdev->discovery.state == DISCOVERY_STOPPING) {
+ hci_cancel_le_scan(hdev);
+ mgmt_start_discovery_cancelled(hdev);
+ } else {
+ hci_discovery_set_state(hdev, DISCOVERY_FINDING);
+ }
hci_dev_unlock(hdev);
break;
@@ -1375,7 +1380,12 @@ static void hci_cs_inquiry(struct hci_dev *hdev, __u8 status)
set_bit(HCI_INQUIRY, &hdev->flags);
hci_dev_lock(hdev);
- hci_discovery_set_state(hdev, DISCOVERY_FINDING);
+ if (hdev->discovery.state == DISCOVERY_STOPPING) {
+ hci_cancel_inquiry(hdev);
+ mgmt_start_discovery_cancelled(hdev);
+ } else {
+ hci_discovery_set_state(hdev, DISCOVERY_FINDING);
+ }
hci_dev_unlock(hdev);
}
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 0db9d66..6dc275f 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2394,7 +2394,8 @@ static int stop_discovery(struct sock *sk, struct hci_dev *hdev, void *data,
hci_dev_lock(hdev);
- if (!hci_discovery_active(hdev)) {
+ if (hdev->discovery.state != DISCOVERY_STARTING &&
+ !hci_discovery_active(hdev)) {
err = cmd_complete(sk, hdev->id, MGMT_OP_STOP_DISCOVERY,
MGMT_STATUS_REJECTED, &mgmt_cp->type,
sizeof(mgmt_cp->type));
@@ -2442,6 +2443,10 @@ static int stop_discovery(struct sock *sk, struct hci_dev *hdev, void *data,
break;
+ case DISCOVERY_STARTING:
+ err = 0;
+ break;
+
default:
BT_DBG("unknown discovery state %u", hdev->discovery.state);
err = -EFAULT;
@@ -3720,6 +3725,11 @@ int mgmt_discovering(struct hci_dev *hdev, u8 discovering)
mgmt_pending_remove(cmd);
}
+ if (hdev->discovery.discovering == discovering)
+ return 0;
+
+ hdev->discovery.discovering = discovering;
+
memset(&ev, 0, sizeof(ev));
ev.type = hdev->discovery.type;
ev.discovering = discovering;
--
1.7.9.5
^ permalink raw reply related
* [PATCH v3 2/3] Bluetooth: Add mgmt_start_discovery_cancelled()
From: Jaganath Kanakkassery @ 2013-01-15 13:01 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Jaganath Kanakkassery
In-Reply-To: <1358254892-11303-1-git-send-email-jaganath.k@samsung.com>
This function can be used to inform userspace that start discovery
is cancelled
Signed-off-by: Jaganath Kanakkassery <jaganath.k@samsung.com>
---
include/net/bluetooth/hci_core.h | 1 +
net/bluetooth/mgmt.c | 19 +++++++++++++++++++
2 files changed, 20 insertions(+)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 014a2ea..d8f68c7 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1112,6 +1112,7 @@ int mgmt_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
int mgmt_remote_name(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
u8 addr_type, s8 rssi, u8 *name, u8 name_len);
int mgmt_start_discovery_failed(struct hci_dev *hdev, u8 status);
+int mgmt_start_discovery_cancelled(struct hci_dev *hdev);
int mgmt_stop_discovery_failed(struct hci_dev *hdev, u8 status);
int mgmt_discovering(struct hci_dev *hdev, u8 discovering);
int mgmt_interleaved_discovery(struct hci_dev *hdev);
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index bcc7080..0db9d66 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -3665,6 +3665,25 @@ int mgmt_start_discovery_failed(struct hci_dev *hdev, u8 status)
return err;
}
+int mgmt_start_discovery_cancelled(struct hci_dev *hdev)
+{
+ struct pending_cmd *cmd;
+ u8 type;
+ int err;
+
+ cmd = mgmt_pending_find(MGMT_OP_START_DISCOVERY, hdev);
+ if (!cmd)
+ return -ENOENT;
+
+ type = hdev->discovery.type;
+
+ err = cmd_complete(cmd->sk, hdev->id, cmd->opcode, MGMT_STATUS_CANCELLED,
+ &type, sizeof(type));
+ mgmt_pending_remove(cmd);
+
+ return err;
+}
+
int mgmt_stop_discovery_failed(struct hci_dev *hdev, u8 status)
{
struct pending_cmd *cmd;
--
1.7.9.5
^ permalink raw reply related
* [PATCH v3 1/3] Bluetooth: Move discovery state check inside hci_dev_lock()
From: Jaganath Kanakkassery @ 2013-01-15 13:01 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Jaganath Kanakkassery
After checking the discovery state, if other thread modifies it
then it will be overwritten by the assignment in the first thread.
Signed-off-by: Jaganath Kanakkassery <jaganath.k@samsung.com>
---
net/bluetooth/hci_event.c | 9 ++++-----
net/bluetooth/mgmt.c | 4 ----
2 files changed, 4 insertions(+), 9 deletions(-)
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 705078a..97b4828 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1273,14 +1273,13 @@ static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
clear_bit(HCI_LE_SCAN, &hdev->dev_flags);
+ hci_dev_lock(hdev);
if (hdev->discovery.type == DISCOV_TYPE_INTERLEAVED &&
- hdev->discovery.state == DISCOVERY_FINDING) {
+ hdev->discovery.state == DISCOVERY_FINDING)
mgmt_interleaved_discovery(hdev);
- } else {
- hci_dev_lock(hdev);
+ else
hci_discovery_set_state(hdev, DISCOVERY_STOPPED);
- hci_dev_unlock(hdev);
- }
+ hci_dev_unlock(hdev);
break;
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index e5502a5..bcc7080 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2298,14 +2298,10 @@ int mgmt_interleaved_discovery(struct hci_dev *hdev)
BT_DBG("%s", hdev->name);
- hci_dev_lock(hdev);
-
err = hci_do_inquiry(hdev, INQUIRY_LEN_BREDR_LE);
if (err < 0)
hci_discovery_set_state(hdev, DISCOVERY_STOPPED);
- hci_dev_unlock(hdev);
-
return err;
}
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH BlueZ 01/11] transport: Initialize the "Volume" property with 50%
From: Luiz Augusto von Dentz @ 2013-01-15 10:43 UTC (permalink / raw)
To: Oleksandr.Domin
Cc: Joao Paulo Rechi Vita, Mikel Astiz, Luiz Augusto Von Dentz,
linux-bluetooth@vger.kernel.org, Vinicius Gomes, Claudio Takahasi
In-Reply-To: <9F89C4B87365E4408446E7FF5C4AE97115A2008B04@SMUCM17V.europe.bmw.corp>
Hi Oleksandr,
On Tue, Jan 15, 2013 at 10:02 AM, <Oleksandr.Domin@bmw.de> wrote:
> Hi Joao,
>>
>>Hello Mikel and Luiz,
>>
>>On Mon, Jan 14, 2013 at 12:34 PM, Luiz Augusto von Dentz
>><luiz.dentz@gmail.com> wrote:
>>> Hi Mikel,
>>>
>>> On Mon, Jan 14, 2013 at 4:55 PM, Mikel Astiz <Mikel.Astiz@bmw-carit.de>
>>wrote:
>>>> Hi all,
>>>>
>>>>> Hi Joao,
>>>>>
>>>>> On Mon, Jan 14, 2013 at 4:05 PM, Joao Paulo Rechi Vita
>>>>> <jprvita@openbossa.org> wrote:
>>>>> > On Sun, Jan 13, 2013 at 12:22 PM, Luiz Augusto von Dentz
>>>>> > <luiz.dentz@gmail.com> wrote:
>>>>> >> Hi Joao,
>>>>> >>
>>>>> >> On Fri, Jan 11, 2013 at 10:25 PM, João Paulo Rechi Vita
>>>>> >> <jprvita@openbossa.org> wrote:
>>>>> >>> ---
>>>>> >>> profiles/audio/transport.c | 2 +-
>>>>> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>> >>>
>>>>> >>> diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c
>>>>> >>> index a4370a5..6ffa98a 100644
>>>>> >>> --- a/profiles/audio/transport.c
>>>>> >>> +++ b/profiles/audio/transport.c
>>>>> >>> @@ -787,7 +787,7 @@ struct media_transport
>>>>> *media_transport_create(struct media_endpoint *endpoint,
>>>>> >>> struct a2dp_transport *a2dp;
>>>>> >>>
>>>>> >>> a2dp = g_new0(struct a2dp_transport, 1);
>>>>> >>> - a2dp->volume = -1;
>>>>> >>> + a2dp->volume = 63;
>>>>> >>>
>>>>> >>> transport->resume = resume_a2dp;
>>>>> >>> transport->suspend = suspend_a2dp;
>>>>> >>> --
>>>>> >>> 1.7.11.7
>>>>> >>
>>>>> >> Does the spec say anything regarding this? Actually it seems this
>>>>> >> value must be set by PA if it does support volume notification, which
>>>>> >> means a new version of PA, then it should set the value when the card
>>>>> >> is initialized, otherwise if the endpoint doesn't set a value it
>>>>> >> should remain -1/not available. If volume is not set by the endpoint
>>>>> >> we should either return and error upon register notification or
>>>>> >> return maximum volume always and refuse to SetAbsoluteVolume, my
>>>>> >> guess is that the latter is better for IOP reasons since the remote
>>>>> >> device may register to volume while the endpoint is setting up the
>>>>> >> transport so the volume may be set latter.
>>>>> >>
>>>>> >>
>>>>> >
>>>>> > I agree the right value will come from PA. The problem is that the
>>>>> > org.bluez.MediaTransport.Volume property doesn't exist when volume is
>>>>> > < 0 or > 127 and PA will not be able to inform the correct volume
>>>>> > value. Should we simply remove volume_exists(), or do you have any
>>>>> > other suggestion?
>>>>>
>>>>> I guess we could use the role of transport, if it is a sink/controller
>>transport it
>>>>> should be initialized to max volume otherwise it should still be
>>initialized with
>>>>> -1 so we can continue to omit in case of source/target role.
>>>>
>>>> A side question: AFAIK this feature has been added in AVRCP 1.4. How
>>would the endpoint (PulseAudio) know if it's supported or not by a certain
>>device?
>>>
>>> For the controller/sink role there is no point of PulseAudio knowing
>>> about this, it basically need to set the volume and that about it, if
>>> the remote device is interested it register to be notified otherwise
>>> we don't do anything with the value, plain and simple.
>>>
>>
>>I think PA will also want to register for notifications of changes on
>>that property, so it can reflects the current volume on the remote
>>through the master volume of the bluetooth card. The volume can be
>>changed on the remote side and it will inform bluez through the
>>SetAbsoluteVolume command.
>
> In our current AVRCP implementations, we only set the volume on the
> target to 100%. In this case the target will encode the audio
> with full quality. So the volume on the target is not coupled to the
> volume on the system.
The volume of the stream has nothing to do with the volume we are
talking about here, this is about output volume of the
sink/controller. In fact it is recommended that the stream volume be
constant at 100% to minimize precision errors of the codec.
>>
>>>> My proposal would be that the property is not present if the feature is
>>not supported (or also if AVRCP is not connected). Therefore, I'd propose
>>exposing a special value in D-Bus (i.e. -1) as soon as the feature becomes
>>available (AVRCP 1.4 connected) so the endpoint can set the initial value.
>>>
>>> That is a bad idea, the notification mechanism of AVRCP is not
>>> something I would like to see exposed over D-Bus, it is way too broken
>>> since you have to registration again every time the value changes.
>>>
>>
>>I personally thinks it makes sense to not support the property if
>>AVRCP < 1.4 (this was on my initial plans also), or if the remote
>>doesn't implement AVRCP (A2DP only). We just need to find an elegant
>>way to make this information available for the transport. Otherwise
>>AVRCP will be connected every time A2DP is also connected, right? Or
>>am I missing some corner case here?
>
> Question is do you really want to control the volume of your CT
> from the Target? I can speak from the OEM perspective and this is
> not what we want.
Not sure what you really mean here by not what we want, this is
specification and by not conforming with it you may get into IOP
problems or even not be able to qualify the stack. Note that similar
functionality exist also in HFP with SpeakerGain and MicrophoneGain,
so whatever you have against the phone controlling the output volume
is now past the point that you can say no to it.
--
Luiz Augusto von Dentz
^ permalink raw reply
* RE: [PATCH BlueZ 01/11] transport: Initialize the "Volume" property with 50%
From: Oleksandr.Domin @ 2013-01-15 8:02 UTC (permalink / raw)
To: jprvita, luiz.dentz
Cc: Mikel.Astiz, luiz.von.dentz, linux-bluetooth, vinicius.gomes,
claudio.takahasi
In-Reply-To: <CAAngNMb-W6j8q6H5-4uPTQm=AAAUDcD3icJYrmQK1_uzZ2zyag@mail.gmail.com>
SGkgSm9hbywNCj4NCj5IZWxsbyBNaWtlbCBhbmQgTHVpeiwNCj4NCj5PbiBNb24sIEphbiAxNCwg
MjAxMyBhdCAxMjozNCBQTSwgTHVpeiBBdWd1c3RvIHZvbiBEZW50eg0KPjxsdWl6LmRlbnR6QGdt
YWlsLmNvbT4gd3JvdGU6DQo+PiBIaSBNaWtlbCwNCj4+DQo+PiBPbiBNb24sIEphbiAxNCwgMjAx
MyBhdCA0OjU1IFBNLCBNaWtlbCBBc3RpeiA8TWlrZWwuQXN0aXpAYm13LWNhcml0LmRlPg0KPndy
b3RlOg0KPj4+IEhpIGFsbCwNCj4+Pg0KPj4+PiBIaSBKb2FvLA0KPj4+Pg0KPj4+PiBPbiBNb24s
IEphbiAxNCwgMjAxMyBhdCA0OjA1IFBNLCBKb2FvIFBhdWxvIFJlY2hpIFZpdGENCj4+Pj4gPGpw
cnZpdGFAb3BlbmJvc3NhLm9yZz4gd3JvdGU6DQo+Pj4+ID4gT24gU3VuLCBKYW4gMTMsIDIwMTMg
YXQgMTI6MjIgUE0sIEx1aXogQXVndXN0byB2b24gRGVudHoNCj4+Pj4gPiA8bHVpei5kZW50ekBn
bWFpbC5jb20+IHdyb3RlOg0KPj4+PiA+PiBIaSBKb2FvLA0KPj4+PiA+Pg0KPj4+PiA+PiBPbiBG
cmksIEphbiAxMSwgMjAxMyBhdCAxMDoyNSBQTSwgSm/Do28gUGF1bG8gUmVjaGkgVml0YQ0KPj4+
PiA+PiA8anBydml0YUBvcGVuYm9zc2Eub3JnPiB3cm90ZToNCj4+Pj4gPj4+IC0tLQ0KPj4+PiA+
Pj4gIHByb2ZpbGVzL2F1ZGlvL3RyYW5zcG9ydC5jIHwgMiArLQ0KPj4+PiA+Pj4gIDEgZmlsZSBj
aGFuZ2VkLCAxIGluc2VydGlvbigrKSwgMSBkZWxldGlvbigtKQ0KPj4+PiA+Pj4NCj4+Pj4gPj4+
IGRpZmYgLS1naXQgYS9wcm9maWxlcy9hdWRpby90cmFuc3BvcnQuYyBiL3Byb2ZpbGVzL2F1ZGlv
L3RyYW5zcG9ydC5jDQo+Pj4+ID4+PiBpbmRleCBhNDM3MGE1Li42ZmZhOThhIDEwMDY0NA0KPj4+
PiA+Pj4gLS0tIGEvcHJvZmlsZXMvYXVkaW8vdHJhbnNwb3J0LmMNCj4+Pj4gPj4+ICsrKyBiL3By
b2ZpbGVzL2F1ZGlvL3RyYW5zcG9ydC5jDQo+Pj4+ID4+PiBAQCAtNzg3LDcgKzc4Nyw3IEBAIHN0
cnVjdCBtZWRpYV90cmFuc3BvcnQNCj4+Pj4gKm1lZGlhX3RyYW5zcG9ydF9jcmVhdGUoc3RydWN0
IG1lZGlhX2VuZHBvaW50ICplbmRwb2ludCwNCj4+Pj4gPj4+ICAgICAgICAgICAgICAgICBzdHJ1
Y3QgYTJkcF90cmFuc3BvcnQgKmEyZHA7DQo+Pj4+ID4+Pg0KPj4+PiA+Pj4gICAgICAgICAgICAg
ICAgIGEyZHAgPSBnX25ldzAoc3RydWN0IGEyZHBfdHJhbnNwb3J0LCAxKTsNCj4+Pj4gPj4+IC0g
ICAgICAgICAgICAgICBhMmRwLT52b2x1bWUgPSAtMTsNCj4+Pj4gPj4+ICsgICAgICAgICAgICAg
ICBhMmRwLT52b2x1bWUgPSA2MzsNCj4+Pj4gPj4+DQo+Pj4+ID4+PiAgICAgICAgICAgICAgICAg
dHJhbnNwb3J0LT5yZXN1bWUgPSByZXN1bWVfYTJkcDsNCj4+Pj4gPj4+ICAgICAgICAgICAgICAg
ICB0cmFuc3BvcnQtPnN1c3BlbmQgPSBzdXNwZW5kX2EyZHA7DQo+Pj4+ID4+PiAtLQ0KPj4+PiA+
Pj4gMS43LjExLjcNCj4+Pj4gPj4NCj4+Pj4gPj4gRG9lcyB0aGUgc3BlYyBzYXkgYW55dGhpbmcg
cmVnYXJkaW5nIHRoaXM/IEFjdHVhbGx5IGl0IHNlZW1zIHRoaXMNCj4+Pj4gPj4gdmFsdWUgbXVz
dCBiZSBzZXQgYnkgUEEgaWYgaXQgZG9lcyBzdXBwb3J0IHZvbHVtZSBub3RpZmljYXRpb24sIHdo
aWNoDQo+Pj4+ID4+IG1lYW5zIGEgbmV3IHZlcnNpb24gb2YgUEEsIHRoZW4gaXQgc2hvdWxkIHNl
dCB0aGUgdmFsdWUgd2hlbiB0aGUgY2FyZA0KPj4+PiA+PiBpcyBpbml0aWFsaXplZCwgb3RoZXJ3
aXNlIGlmIHRoZSBlbmRwb2ludCBkb2Vzbid0IHNldCBhIHZhbHVlIGl0DQo+Pj4+ID4+IHNob3Vs
ZCByZW1haW4gLTEvbm90IGF2YWlsYWJsZS4gSWYgdm9sdW1lIGlzIG5vdCBzZXQgYnkgdGhlIGVu
ZHBvaW50DQo+Pj4+ID4+IHdlIHNob3VsZCBlaXRoZXIgcmV0dXJuIGFuZCBlcnJvciB1cG9uIHJl
Z2lzdGVyIG5vdGlmaWNhdGlvbiBvcg0KPj4+PiA+PiByZXR1cm4gbWF4aW11bSB2b2x1bWUgYWx3
YXlzIGFuZCByZWZ1c2UgdG8gU2V0QWJzb2x1dGVWb2x1bWUsIG15DQo+Pj4+ID4+IGd1ZXNzIGlz
IHRoYXQgdGhlIGxhdHRlciBpcyBiZXR0ZXIgZm9yIElPUCByZWFzb25zIHNpbmNlIHRoZSByZW1v
dGUNCj4+Pj4gPj4gZGV2aWNlIG1heSByZWdpc3RlciB0byB2b2x1bWUgd2hpbGUgdGhlIGVuZHBv
aW50IGlzIHNldHRpbmcgdXAgdGhlDQo+Pj4+ID4+IHRyYW5zcG9ydCBzbyB0aGUgdm9sdW1lIG1h
eSBiZSBzZXQgbGF0dGVyLg0KPj4+PiA+Pg0KPj4+PiA+Pg0KPj4+PiA+DQo+Pj4+ID4gSSBhZ3Jl
ZSB0aGUgcmlnaHQgdmFsdWUgd2lsbCBjb21lIGZyb20gUEEuIFRoZSBwcm9ibGVtIGlzIHRoYXQg
dGhlDQo+Pj4+ID4gb3JnLmJsdWV6Lk1lZGlhVHJhbnNwb3J0LlZvbHVtZSBwcm9wZXJ0eSBkb2Vz
bid0IGV4aXN0IHdoZW4gdm9sdW1lIGlzDQo+Pj4+ID4gPCAwIG9yID4gMTI3IGFuZCBQQSB3aWxs
IG5vdCBiZSBhYmxlIHRvIGluZm9ybSB0aGUgY29ycmVjdCB2b2x1bWUNCj4+Pj4gPiB2YWx1ZS4g
U2hvdWxkIHdlIHNpbXBseSByZW1vdmUgdm9sdW1lX2V4aXN0cygpLCBvciBkbyB5b3UgaGF2ZSBh
bnkNCj4+Pj4gPiBvdGhlciBzdWdnZXN0aW9uPw0KPj4+Pg0KPj4+PiBJIGd1ZXNzIHdlIGNvdWxk
IHVzZSB0aGUgcm9sZSBvZiB0cmFuc3BvcnQsIGlmIGl0IGlzIGEgc2luay9jb250cm9sbGVyDQo+
dHJhbnNwb3J0IGl0DQo+Pj4+IHNob3VsZCBiZSBpbml0aWFsaXplZCB0byBtYXggdm9sdW1lIG90
aGVyd2lzZSBpdCBzaG91bGQgc3RpbGwgYmUNCj5pbml0aWFsaXplZCB3aXRoDQo+Pj4+IC0xIHNv
IHdlIGNhbiBjb250aW51ZSB0byBvbWl0IGluIGNhc2Ugb2Ygc291cmNlL3RhcmdldCByb2xlLg0K
Pj4+DQo+Pj4gQSBzaWRlIHF1ZXN0aW9uOiBBRkFJSyB0aGlzIGZlYXR1cmUgaGFzIGJlZW4gYWRk
ZWQgaW4gQVZSQ1AgMS40LiBIb3cNCj53b3VsZCB0aGUgZW5kcG9pbnQgKFB1bHNlQXVkaW8pIGtu
b3cgaWYgaXQncyBzdXBwb3J0ZWQgb3Igbm90IGJ5IGEgY2VydGFpbg0KPmRldmljZT8NCj4+DQo+
PiBGb3IgdGhlIGNvbnRyb2xsZXIvc2luayByb2xlIHRoZXJlIGlzIG5vIHBvaW50IG9mIFB1bHNl
QXVkaW8ga25vd2luZw0KPj4gYWJvdXQgdGhpcywgaXQgYmFzaWNhbGx5IG5lZWQgdG8gc2V0IHRo
ZSB2b2x1bWUgYW5kIHRoYXQgYWJvdXQgaXQsIGlmDQo+PiB0aGUgcmVtb3RlIGRldmljZSBpcyBp
bnRlcmVzdGVkIGl0IHJlZ2lzdGVyIHRvIGJlIG5vdGlmaWVkIG90aGVyd2lzZQ0KPj4gd2UgZG9u
J3QgZG8gYW55dGhpbmcgd2l0aCB0aGUgdmFsdWUsIHBsYWluIGFuZCBzaW1wbGUuDQo+Pg0KPg0K
PkkgdGhpbmsgUEEgd2lsbCBhbHNvIHdhbnQgdG8gcmVnaXN0ZXIgZm9yIG5vdGlmaWNhdGlvbnMg
b2YgY2hhbmdlcyBvbg0KPnRoYXQgcHJvcGVydHksIHNvIGl0IGNhbiByZWZsZWN0cyB0aGUgY3Vy
cmVudCB2b2x1bWUgb24gdGhlIHJlbW90ZQ0KPnRocm91Z2ggdGhlIG1hc3RlciB2b2x1bWUgb2Yg
dGhlIGJsdWV0b290aCBjYXJkLiBUaGUgdm9sdW1lIGNhbiBiZQ0KPmNoYW5nZWQgb24gdGhlIHJl
bW90ZSBzaWRlIGFuZCBpdCB3aWxsIGluZm9ybSBibHVleiB0aHJvdWdoIHRoZQ0KPlNldEFic29s
dXRlVm9sdW1lIGNvbW1hbmQuDQoNCkluIG91ciBjdXJyZW50IEFWUkNQIGltcGxlbWVudGF0aW9u
cywgd2Ugb25seSBzZXQgdGhlIHZvbHVtZSBvbiB0aGUgDQp0YXJnZXQgdG8gMTAwJS4gSW4gdGhp
cyBjYXNlIHRoZSB0YXJnZXQgd2lsbCBlbmNvZGUgdGhlIGF1ZGlvDQp3aXRoIGZ1bGwgcXVhbGl0
eS4gU28gdGhlIHZvbHVtZSBvbiB0aGUgdGFyZ2V0IGlzIG5vdCBjb3VwbGVkIHRvIHRoZQ0Kdm9s
dW1lIG9uIHRoZSBzeXN0ZW0uDQoNCj4NCj4+PiBNeSBwcm9wb3NhbCB3b3VsZCBiZSB0aGF0IHRo
ZSBwcm9wZXJ0eSBpcyBub3QgcHJlc2VudCBpZiB0aGUgZmVhdHVyZSBpcw0KPm5vdCBzdXBwb3J0
ZWQgKG9yIGFsc28gaWYgQVZSQ1AgaXMgbm90IGNvbm5lY3RlZCkuIFRoZXJlZm9yZSwgSSdkIHBy
b3Bvc2UNCj5leHBvc2luZyBhIHNwZWNpYWwgdmFsdWUgaW4gRC1CdXMgKGkuZS4gLTEpIGFzIHNv
b24gYXMgdGhlIGZlYXR1cmUgYmVjb21lcw0KPmF2YWlsYWJsZSAoQVZSQ1AgMS40IGNvbm5lY3Rl
ZCkgc28gdGhlIGVuZHBvaW50IGNhbiBzZXQgdGhlIGluaXRpYWwgdmFsdWUuDQo+Pg0KPj4gVGhh
dCBpcyBhIGJhZCBpZGVhLCB0aGUgbm90aWZpY2F0aW9uIG1lY2hhbmlzbSBvZiBBVlJDUCBpcyBu
b3QNCj4+IHNvbWV0aGluZyBJIHdvdWxkIGxpa2UgdG8gc2VlIGV4cG9zZWQgb3ZlciBELUJ1cywg
aXQgaXMgd2F5IHRvbyBicm9rZW4NCj4+IHNpbmNlIHlvdSBoYXZlIHRvIHJlZ2lzdHJhdGlvbiBh
Z2FpbiBldmVyeSB0aW1lIHRoZSB2YWx1ZSBjaGFuZ2VzLg0KPj4NCj4NCj5JIHBlcnNvbmFsbHkg
dGhpbmtzIGl0IG1ha2VzIHNlbnNlIHRvIG5vdCBzdXBwb3J0IHRoZSBwcm9wZXJ0eSBpZg0KPkFW
UkNQIDwgMS40ICh0aGlzIHdhcyBvbiBteSBpbml0aWFsIHBsYW5zIGFsc28pLCBvciBpZiB0aGUg
cmVtb3RlDQo+ZG9lc24ndCBpbXBsZW1lbnQgQVZSQ1AgKEEyRFAgb25seSkuIFdlIGp1c3QgbmVl
ZCB0byBmaW5kIGFuIGVsZWdhbnQNCj53YXkgdG8gbWFrZSB0aGlzIGluZm9ybWF0aW9uIGF2YWls
YWJsZSBmb3IgdGhlIHRyYW5zcG9ydC4gT3RoZXJ3aXNlDQo+QVZSQ1Agd2lsbCBiZSBjb25uZWN0
ZWQgZXZlcnkgdGltZSBBMkRQIGlzIGFsc28gY29ubmVjdGVkLCByaWdodD8gT3INCj5hbSBJIG1p
c3Npbmcgc29tZSBjb3JuZXIgY2FzZSBoZXJlPw0KDQpRdWVzdGlvbiBpcyBkbyB5b3UgcmVhbGx5
IHdhbnQgdG8gY29udHJvbCB0aGUgdm9sdW1lIG9mIHlvdXIgQ1QNCmZyb20gdGhlIFRhcmdldD8g
SSBjYW4gc3BlYWsgZnJvbSB0aGUgT0VNIHBlcnNwZWN0aXZlIGFuZCB0aGlzIGlzDQpub3Qgd2hh
dCB3ZSB3YW50Lg0KDQpSZWdhcmRzLA0KT2xla3NhbmRyDQoNCi0tLQ0KQk1XIEdyb3VwDQpPbGVr
c2FuZHLCoERvbWluDQpBcHBDZW50ZXIsIEVudGVydGFpbm1lbnQgYW5kIE1vYmlsZSBFbmRnZXLD
pHRlDQpTb2Z0d2FyZSBBcmNoaXRlY3QgR0VOSVZJDQpNYXgtRGlhbWFuZC1TdHIuIDI1DQo4MDkz
N8KgTcO8bmNoZW4NCg0KVGVsOsKgKzQ5IDg5IDM4MiAtIDYwNjMyDQpNb2JpbGU6wqArNDkgMTc2
IDYwMTYwNjMyDQpNYWlsOsKgb2xla3NhbmRyLmRvbWluQGJtdy5kZQ0KV2ViOsKgaHR0cDovL3d3
dy5ibXdncm91cC5jb20NCi0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t
LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tDQpCYXllcmlzY2hlIE1vdG9y
ZW4gV2Vya2UgQWt0aWVuZ2VzZWxsc2NoYWZ0DQpCb2FyZCBvZiBNYW5hZ2VtZW50OiBOb3JiZXJ0
IFJlaXRob2ZlciwgQ2hhaXJtYW4sDQpGcmFuay1QZXRlciBBcm5kdCwgTWlsYWdyb3MgQ2Fpw7Fh
IENhcnJlaXJvLUFuZHJlZSwNCkhlcmJlcnQgRGllc3MsIEtsYXVzIERyYWVnZXIsIEZyaWVkcmlj
aCBFaWNoaW5lciwNCkhhcmFsZCBLcsO8Z2VyLCBJYW4gUm9iZXJ0c29uLg0KQ2hhaXJtYW4gb2Yg
U3VwZXJ2aXNvcnkgQm9hcmQ6IEpvYWNoaW0gTWlsYmVyZw0KUmVnaXN0ZXJlZCBpbiBHZXJtYW55
OiBNw7xuY2hlbiBIUkIgNDIyNDMNCi0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t
LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tDQoNCg0KDQo=
^ permalink raw reply
* Re: [PATCH 4/4] Bluetooth: Fix write_scan_enable when HCI_AUTO_OFF is set
From: Anderson Lizardo @ 2013-01-15 1:06 UTC (permalink / raw)
To: Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <1358195633-29303-5-git-send-email-johan.hedberg@gmail.com>
Hi Johan,
On Mon, Jan 14, 2013 at 4:33 PM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> [...]
> Since mgmt_powered() no longer takes care of the write_scan_enable we
> also need to ensure that it's part of the hci_setup procedure and hence
> the set_bredr_scan() function needs to be moved into hci_core.c from
> mgmt.c.
Small typo: hci_core.c -> hci_event.c
Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil
^ permalink raw reply
* Re: [PATCH 3/4] Bluetooth: Fix using system-global workqueue when not necessary
From: Marcel Holtmann @ 2013-01-14 20:44 UTC (permalink / raw)
To: Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <1358195633-29303-4-git-send-email-johan.hedberg@gmail.com>
Hi Johan,
> There's a per-HCI device workqueue (hdev->workqueue) that should be used
> for general per-HCI device work (except hdev->req_workqueue that's for
> hci_request() related work). This patch fixes places using the
> system-global work queue and makes them use the hdev->workqueue instead.
>
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> net/bluetooth/hci_core.c | 4 ++--
> net/bluetooth/mgmt.c | 3 ++-
> 2 files changed, 4 insertions(+), 3 deletions(-)
Acked-by: Marcel Holtmann <marcel@holtmann.org>
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 2/4] Bluetooth: Use req_workqueue for hci_request operations
From: Marcel Holtmann @ 2013-01-14 20:43 UTC (permalink / raw)
To: Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <1358195633-29303-3-git-send-email-johan.hedberg@gmail.com>
Hi Johan,
> This patch converts work assignment relying on hci_request() from the
> system-global work queue to the per-HCI device specific work queue
> (hdev->req_workqueue) intended for hci_request() related tasks.
>
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> net/bluetooth/hci_core.c | 5 +++--
> net/bluetooth/mgmt.c | 4 ++--
> 2 files changed, 5 insertions(+), 4 deletions(-)
Acked-by: Marcel Holtmann <marcel@holtmann.org>
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 1/4] Bluetooth: Add a new workqueue for hci_request operations
From: Marcel Holtmann @ 2013-01-14 20:42 UTC (permalink / raw)
To: Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <1358195633-29303-2-git-send-email-johan.hedberg@gmail.com>
Hi Johan,
> The hci_request function is blocking and cannot be called through the
> usual per-HCI device workqueue (hdev->workqueue). While hci_request is
> in progress any other work from que queue, including sending HCI
no idea what a que queue is ;)
> commands to the controller would be blocked and eventually cause the
> hci_request call to time out.
>
> This patch adds a second workqueue to be used by operations needing
> hci_request and thereby avoiding issues with blocking other workqueue
> users.
>
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> include/net/bluetooth/hci_core.h | 1 +
> net/bluetooth/hci_core.c | 11 +++++++++++
> 2 files changed, 12 insertions(+)
Acked-by: Marcel Holtmann <marcel@holtmann.org>
Regards
Marcel
^ permalink raw reply
* [PATCH 4/4] Bluetooth: Fix write_scan_enable when HCI_AUTO_OFF is set
From: Johan Hedberg @ 2013-01-14 20:33 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1358195633-29303-1-git-send-email-johan.hedberg@gmail.com>
From: Johan Hedberg <johan.hedberg@intel.com>
When the HCI_AUTO_OFF flag is set the HCI device has been powered on
automatically by the kernel but through the mgmt interface it looks like
its powered off (and requires an explicit mgmt_set_powered to be
considered powered on). However, since the mgmt interface accepts
commands such as set_discoverable and set_connectable when powered off
there's no reason to avoid sending HCI commands in this state.
The hdev_is_powered() macro checks for both HCI_UP and HCI_AUTO_OFF and
is usable for mgmt commands that should fail (return NOT_POWERED) when
powered off, however for commands that should succeed testing only for
HCI_UP makes more sense.
Since the power_off delayed work is active when HCI_AUTO_OFF is set, and
can be triggered at any moment, it's also important to modify the expiry
time with mod_delayed_work() to ensure that the HCI commands are safely
completed.
The added benefit of doing these commands before an explicit
mgmt_set_powered call is that HCI commands sent in the mgmt_powered
function do not have the same kind of tracking as those triggered by a
directly related mgmt command and can therefore cause race conditions
(when e.g. set_discoverable is called quickly after calling
set_powered).
Since mgmt_powered() no longer takes care of the write_scan_enable we
also need to ensure that it's part of the hci_setup procedure and hence
the set_bredr_scan() function needs to be moved into hci_core.c from
mgmt.c.
Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
net/bluetooth/hci_event.c | 17 +++++++++++++++++
net/bluetooth/mgmt.c | 28 ++++++++++------------------
2 files changed, 27 insertions(+), 18 deletions(-)
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 705078a..aaa18bd 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -573,6 +573,21 @@ static void hci_setup_event_mask(struct hci_dev *hdev)
}
}
+static int set_bredr_scan(struct hci_dev *hdev)
+{
+ u8 scan = 0;
+
+ if (test_bit(HCI_CONNECTABLE, &hdev->dev_flags))
+ scan |= SCAN_PAGE;
+ if (test_bit(HCI_DISCOVERABLE, &hdev->dev_flags))
+ scan |= SCAN_INQUIRY;
+
+ if (!scan)
+ return 0;
+
+ return hci_send_cmd(hdev, HCI_OP_WRITE_SCAN_ENABLE, 1, &scan);
+}
+
static void bredr_setup(struct hci_dev *hdev)
{
struct hci_cp_delete_stored_link_key cp;
@@ -602,6 +617,8 @@ static void bredr_setup(struct hci_dev *hdev)
bacpy(&cp.bdaddr, BDADDR_ANY);
cp.delete_all = 1;
hci_send_cmd(hdev, HCI_OP_DELETE_STORED_LINK_KEY, sizeof(cp), &cp);
+
+ set_bredr_scan(hdev);
}
static void le_setup(struct hci_dev *hdev)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index fc171f2..7cc2379 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -907,7 +907,7 @@ static int set_discoverable(struct sock *sk, struct hci_dev *hdev, void *data,
goto failed;
}
- if (!hdev_is_powered(hdev)) {
+ if (!test_bit(HCI_UP, &hdev->flags)) {
bool changed = false;
if (!!cp->val != test_bit(HCI_DISCOVERABLE, &hdev->dev_flags)) {
@@ -954,6 +954,10 @@ static int set_discoverable(struct sock *sk, struct hci_dev *hdev, void *data,
else
cancel_delayed_work(&hdev->discov_off);
+ if (test_bit(HCI_AUTO_OFF, &hdev->dev_flags))
+ mod_delayed_work(hdev->req_workqueue, &hdev->power_off,
+ HCI_AUTO_OFF_TIMEOUT);
+
err = hci_send_cmd(hdev, HCI_OP_WRITE_SCAN_ENABLE, 1, &scan);
if (err < 0)
mgmt_pending_remove(cmd);
@@ -986,7 +990,7 @@ static int set_connectable(struct sock *sk, struct hci_dev *hdev, void *data,
hci_dev_lock(hdev);
- if (!hdev_is_powered(hdev)) {
+ if (!test_bit(HCI_UP, &hdev->flags)) {
bool changed = false;
if (!!cp->val != test_bit(HCI_CONNECTABLE, &hdev->dev_flags))
@@ -1027,6 +1031,10 @@ static int set_connectable(struct sock *sk, struct hci_dev *hdev, void *data,
goto failed;
}
+ if (test_bit(HCI_AUTO_OFF, &hdev->dev_flags))
+ mod_delayed_work(hdev->req_workqueue, &hdev->power_off,
+ HCI_AUTO_OFF_TIMEOUT);
+
if (cp->val) {
scan = SCAN_PAGE;
} else {
@@ -2930,21 +2938,6 @@ static void settings_rsp(struct pending_cmd *cmd, void *data)
mgmt_pending_free(cmd);
}
-static int set_bredr_scan(struct hci_dev *hdev)
-{
- u8 scan = 0;
-
- if (test_bit(HCI_CONNECTABLE, &hdev->dev_flags))
- scan |= SCAN_PAGE;
- if (test_bit(HCI_DISCOVERABLE, &hdev->dev_flags))
- scan |= SCAN_INQUIRY;
-
- if (!scan)
- return 0;
-
- return hci_send_cmd(hdev, HCI_OP_WRITE_SCAN_ENABLE, 1, &scan);
-}
-
int mgmt_powered(struct hci_dev *hdev, u8 powered)
{
struct cmd_lookup match = { NULL, hdev };
@@ -2980,7 +2973,6 @@ int mgmt_powered(struct hci_dev *hdev, u8 powered)
}
if (lmp_bredr_capable(hdev)) {
- set_bredr_scan(hdev);
update_class(hdev);
update_name(hdev, hdev->dev_name);
update_eir(hdev);
--
1.7.10.4
^ permalink raw reply related
* [PATCH 3/4] Bluetooth: Fix using system-global workqueue when not necessary
From: Johan Hedberg @ 2013-01-14 20:33 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1358195633-29303-1-git-send-email-johan.hedberg@gmail.com>
From: Johan Hedberg <johan.hedberg@intel.com>
There's a per-HCI device workqueue (hdev->workqueue) that should be used
for general per-HCI device work (except hdev->req_workqueue that's for
hci_request() related work). This patch fixes places using the
system-global work queue and makes them use the hdev->workqueue instead.
Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
net/bluetooth/hci_core.c | 4 ++--
net/bluetooth/mgmt.c | 3 ++-
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 545553b..e061b35 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1622,8 +1622,8 @@ static int hci_do_le_scan(struct hci_dev *hdev, u8 type, u16 interval,
if (err < 0)
return err;
- schedule_delayed_work(&hdev->le_scan_disable,
- msecs_to_jiffies(timeout));
+ queue_delayed_work(hdev->workqueue, &hdev->le_scan_disable,
+ msecs_to_jiffies(timeout));
return 0;
}
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 54114ff..fc171f2 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1361,7 +1361,8 @@ static bool enable_service_cache(struct hci_dev *hdev)
return false;
if (!test_and_set_bit(HCI_SERVICE_CACHE, &hdev->dev_flags)) {
- schedule_delayed_work(&hdev->service_cache, CACHE_TIMEOUT);
+ queue_delayed_work(hdev->workqueue, &hdev->service_cache,
+ CACHE_TIMEOUT);
return true;
}
--
1.7.10.4
^ permalink raw reply related
* [PATCH 2/4] Bluetooth: Use req_workqueue for hci_request operations
From: Johan Hedberg @ 2013-01-14 20:33 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1358195633-29303-1-git-send-email-johan.hedberg@gmail.com>
From: Johan Hedberg <johan.hedberg@intel.com>
This patch converts work assignment relying on hci_request() from the
system-global work queue to the per-HCI device specific work queue
(hdev->req_workqueue) intended for hci_request() related tasks.
Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
net/bluetooth/hci_core.c | 5 +++--
net/bluetooth/mgmt.c | 4 ++--
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index f73907a..545553b 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1146,7 +1146,8 @@ static void hci_power_on(struct work_struct *work)
return;
if (test_bit(HCI_AUTO_OFF, &hdev->dev_flags))
- schedule_delayed_work(&hdev->power_off, HCI_AUTO_OFF_TIMEOUT);
+ queue_delayed_work(hdev->req_workqueue, &hdev->power_off,
+ HCI_AUTO_OFF_TIMEOUT);
if (test_and_clear_bit(HCI_SETUP, &hdev->dev_flags))
mgmt_index_added(hdev);
@@ -1830,7 +1831,7 @@ int hci_register_dev(struct hci_dev *hdev)
hci_notify(hdev, HCI_DEV_REG);
hci_dev_hold(hdev);
- schedule_work(&hdev->power_on);
+ queue_work(hdev->req_workqueue, &hdev->power_on);
return id;
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 37add53..54114ff 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -812,9 +812,9 @@ static int set_powered(struct sock *sk, struct hci_dev *hdev, void *data,
}
if (cp->val)
- schedule_work(&hdev->power_on);
+ queue_work(hdev->req_workqueue, &hdev->power_on);
else
- schedule_work(&hdev->power_off.work);
+ queue_work(hdev->req_workqueue, &hdev->power_off.work);
err = 0;
--
1.7.10.4
^ permalink raw reply related
* [PATCH 1/4] Bluetooth: Add a new workqueue for hci_request operations
From: Johan Hedberg @ 2013-01-14 20:33 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1358195633-29303-1-git-send-email-johan.hedberg@gmail.com>
From: Johan Hedberg <johan.hedberg@intel.com>
The hci_request function is blocking and cannot be called through the
usual per-HCI device workqueue (hdev->workqueue). While hci_request is
in progress any other work from que queue, including sending HCI
commands to the controller would be blocked and eventually cause the
hci_request call to time out.
This patch adds a second workqueue to be used by operations needing
hci_request and thereby avoiding issues with blocking other workqueue
users.
Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
include/net/bluetooth/hci_core.h | 1 +
net/bluetooth/hci_core.c | 11 +++++++++++
2 files changed, 12 insertions(+)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 014a2ea..769a740 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -216,6 +216,7 @@ struct hci_dev {
unsigned long le_last_tx;
struct workqueue_struct *workqueue;
+ struct workqueue_struct *req_workqueue;
struct work_struct power_on;
struct delayed_work power_off;
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 596660d..f73907a 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1799,6 +1799,15 @@ int hci_register_dev(struct hci_dev *hdev)
goto err;
}
+ hdev->req_workqueue = alloc_workqueue(hdev->name,
+ WQ_HIGHPRI | WQ_UNBOUND |
+ WQ_MEM_RECLAIM, 1);
+ if (!hdev->req_workqueue) {
+ destroy_workqueue(hdev->workqueue);
+ error = -ENOMEM;
+ goto err;
+ }
+
error = hci_add_sysfs(hdev);
if (error < 0)
goto err_wqueue;
@@ -1827,6 +1836,7 @@ int hci_register_dev(struct hci_dev *hdev)
err_wqueue:
destroy_workqueue(hdev->workqueue);
+ destroy_workqueue(hdev->req_workqueue);
err:
ida_simple_remove(&hci_index_ida, hdev->id);
write_lock(&hci_dev_list_lock);
@@ -1880,6 +1890,7 @@ void hci_unregister_dev(struct hci_dev *hdev)
hci_del_sysfs(hdev);
destroy_workqueue(hdev->workqueue);
+ destroy_workqueue(hdev->req_workqueue);
hci_dev_lock(hdev);
hci_blacklist_clear(hdev);
--
1.7.10.4
^ permalink raw reply related
* [PATCH 0/4] Bluetooth: Fix workqueue related issues
From: Johan Hedberg @ 2013-01-14 20:33 UTC (permalink / raw)
To: linux-bluetooth
Hi,
I was initially only going to fix the thing in the last patch of this
set but after starting to look at the code I found several other issues
and hence the rest of the patches.
The first three patches fix work queue handling. We should, whenever
possible, avoid the system-global workqueue (the schedule_*work
functions) and use hdev-specific ones instead. However, we can't use
hci_request() with the usual workqueue since that would block e.g.
hci_send_cmd from completing so a second work queue is needed.
The last patch fixes a race with setting the scan mode that I was
seeing every once in a while through a connectable or discoverable test
case of mgmt-tester failing. There are several other HCI commands with
the same potential issue (like the class of device and the local name)
but I haven't gotten around to fixing those yet.
Johan
^ permalink raw reply
* Re: [PATCH v2] Bluetooth: Power the device up after a rfkill unblock
From: Marcel Holtmann @ 2013-01-14 18:00 UTC (permalink / raw)
To: Vinicius Costa Gomes; +Cc: linux-bluetooth
In-Reply-To: <1358173227-5384-1-git-send-email-vinicius.gomes@openbossa.org>
Hi Vinicius,
> With the HCI_SETUP patches, this is all that is needed to make the
> case when a adapter is added with Bluetooth blocked in rfkill to work.
>
> When rfkill is unblocked, the device will be powered on if the device
> is in HCI_SETUP state, meaning that it was never properly initialized.
> If the device is not used by userspace, the HCI_AUTO_OFF flag will
> take care of powering it off.
>
> Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
> ---
>
> Samuel on IRC caused me to remember this. Thanks :-)
>
>
> net/bluetooth/hci_core.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 596660d..9796c0a 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -1124,8 +1124,11 @@ static int hci_rfkill_set_block(void *data, bool blocked)
>
> BT_DBG("%p name %s blocked %d", hdev, hdev->name, blocked);
>
> - if (!blocked)
> + if (!blocked) {
> + if (test_bit(HCI_SETUP, &hdev->dev_flags))
> + schedule_work(&hdev->power_on);
> return 0;
> + }
>
> hci_dev_do_close(hdev);
>
is this really the behavior that we want. Even if rfkill block all
Bluetooth is set, we should run the adapter initialization. That is not
triggering any RF. Sending the initial HCI_Reset is actually even more
important to ensure the adapter is really reset and has not some left
over state.
So I think what should be done is run the HCI_SETUP phase, but then
refuse to allow bringing up the controller.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 1/8] obexd: Handle empty path name in SetPath
From: Luiz Augusto von Dentz @ 2013-01-14 17:33 UTC (permalink / raw)
To: Christian Fetzer; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <50F3D948.9010605@oss.bmw-carit.de>
Hi Christian,
On Mon, Jan 14, 2013 at 12:09 PM, Christian Fetzer
<christian.fetzer@oss.bmw-carit.de> wrote:
> Hi Luiz,
>
>
> On 01/13/2013 04:55 PM, Luiz Augusto von Dentz wrote:
>>
>> But what empty means in this case, I would like to stick with cd behavior,
>> so empty should set back to root. Otherwise the patchset seems fine. -- Luiz
>> Augusto von Dentz
>
> But this is exactly what the patch is doing.
> g_obex_setpath called with an empty path won't set the parent flag,
> and thus it requests going back to the root folder.
>
> < ACL data: handle 21 flags 0x00 dlen 21
> L2CAP(d): cid 0x0041 len 17 [psm 3]
> RFCOMM(d): UIH: cr 1 dlci 32 pf 0 ilen 13 fcs 0xd8
> OBEX: SetPath cmd(f): len 13 flags 2 constants 0
> Connection ID (0xcb) = 98
> Name (0x01) = Unicode length 0
>> HCI Event: Number of Completed Packets (0x13) plen 5
> handle 21 packets 1
>> ACL data: handle 21 flags 0x02 dlen 11
> L2CAP(d): cid 0x0041 len 7 [psm 3]
> RFCOMM(d): UIH: cr 0 dlci 32 pf 0 ilen 3 fcs 0x2
> OBEX: SetPath rsp(f): status 200 len 3
>
> Or did I misunderstand you?
I was just confirming this would not cd back to parent, anyway this
set is now upstream, thanks!
Btw, Ive modified a little bit patch 06/08 to use hexadecimal values
to indicate better the size.
--
Luiz Augusto von Dentz
^ permalink raw reply
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