* [PATCH BlueZ 03/12] health: Fix struct mcap_csp "csp_req" field type
From: Anderson Lizardo @ 2014-01-04 1:55 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
In-Reply-To: <1388800528-10699-1-git-send-email-anderson.lizardo@openbossa.org>
The values to which this field is set have nothing to do with MCAPCtrl.
Fixes clang error:
profiles/health/mcap_sync.c:767:24: error: comparison of constant 17
with expression of type 'MCAPCtrl' is always true
[-Werror,-Wtautological-constant-out-of-range-compare]
---
profiles/health/mcap_sync.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/profiles/health/mcap_sync.c b/profiles/health/mcap_sync.c
index 0d9f17d..7c35e4a 100644
--- a/profiles/health/mcap_sync.c
+++ b/profiles/health/mcap_sync.c
@@ -55,7 +55,7 @@ struct mcap_csp {
guint remote_caps; /* CSP-Slave: remote master got caps */
guint rem_req_acc; /* CSP-Slave: accuracy required by master */
guint ind_expected; /* CSP-Master: indication expected */
- MCAPCtrl csp_req; /* CSP-Master: Request control flag */
+ uint8_t csp_req; /* CSP-Master: Request control flag */
guint ind_timer; /* CSP-Slave: indication timer */
guint set_timer; /* CSP-Slave: delayed set timer */
void *set_data; /* CSP-Slave: delayed set data */
--
1.8.3.2
^ permalink raw reply related
* [PATCH BlueZ 04/12] attrib: Fix sprintf() format specification
From: Anderson Lizardo @ 2014-01-04 1:55 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
In-Reply-To: <1388800528-10699-1-git-send-email-anderson.lizardo@openbossa.org>
Fixes clang error:
src/attrib-server.c:916:26: error: format specifies type 'unsigned char'
but the argument has type 'uint16_t' (aka 'unsigned short')
[-Werror,-Wformat]
---
src/attrib-server.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/attrib-server.c b/src/attrib-server.c
index a6f1066..a7ee55d 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -913,7 +913,7 @@ static uint16_t write_value(struct gatt_channel *channel, uint16_t handle,
g_key_file_load_from_file(key_file, filename, 0, NULL);
sprintf(group, "%hu", handle);
- sprintf(value, "%hhX", cccval);
+ sprintf(value, "%hX", cccval);
g_key_file_set_string(key_file, group, "Value", value);
data = g_key_file_to_data(key_file, &length, NULL);
--
1.8.3.2
^ permalink raw reply related
* [PATCH BlueZ 05/12] core: Fix sscanf() format specification
From: Anderson Lizardo @ 2014-01-04 1:55 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
In-Reply-To: <1388800528-10699-1-git-send-email-anderson.lizardo@openbossa.org>
Also change the type of the "handle" variable so it is compatible with
the "%hu" specification used for sprintf() on the same function.
Fixes clang error:
src/adapter.c:3550:24: error: format specifies type 'unsigned short' but
the argument has type 'int' [-Werror,-Wformat]
---
src/adapter.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/adapter.c b/src/adapter.c
index 9480103..230f3ce 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -3515,16 +3515,16 @@ static void convert_ccc_entry(char *key, char *value, void *user_data)
char *src_addr = user_data;
char dst_addr[18];
char type = BDADDR_BREDR;
- int handle, ret;
+ uint16_t handle;
+ int ret, err;
char filename[PATH_MAX + 1];
GKeyFile *key_file;
struct stat st;
- int err;
char group[6];
char *data;
gsize length = 0;
- ret = sscanf(key, "%17s#%hhu#%04X", dst_addr, &type, &handle);
+ ret = sscanf(key, "%17s#%hhu#%04hX", dst_addr, &type, &handle);
if (ret < 3)
return;
@@ -3565,16 +3565,16 @@ static void convert_gatt_entry(char *key, char *value, void *user_data)
char *src_addr = user_data;
char dst_addr[18];
char type = BDADDR_BREDR;
- int handle, ret;
+ uint16_t handle;
+ int ret, err;
char filename[PATH_MAX + 1];
GKeyFile *key_file;
struct stat st;
- int err;
char group[6];
char *data;
gsize length = 0;
- ret = sscanf(key, "%17s#%hhu#%04X", dst_addr, &type, &handle);
+ ret = sscanf(key, "%17s#%hhu#%04hX", dst_addr, &type, &handle);
if (ret < 3)
return;
--
1.8.3.2
^ permalink raw reply related
* [PATCH BlueZ 06/12] tools: Fix unaligned memory access on smp-tester
From: Anderson Lizardo @ 2014-01-04 1:55 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
In-Reply-To: <1388800528-10699-1-git-send-email-anderson.lizardo@openbossa.org>
Fixes clang errors like:
tools/smp-tester.c:263:11: error: cast from 'uint8_t *' (aka 'unsigned
char *') to 'u128 *' increases required alignment from 1 to 8
[-Werror,-Wcast-align]
---
tools/smp-tester.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/tools/smp-tester.c b/tools/smp-tester.c
index 685d379..20429a9 100644
--- a/tools/smp-tester.c
+++ b/tools/smp-tester.c
@@ -233,10 +233,16 @@ typedef struct {
uint64_t a, b;
} u128;
-static inline void u128_xor(u128 *r, const u128 *p, const u128 *q)
+static inline void u128_xor(void *r, const void *p, const void *q)
{
- r->a = p->a ^ q->a;
- r->b = p->b ^ q->b;
+ const u128 pp = bt_get_unaligned((const u128 *) p);
+ const u128 qq = bt_get_unaligned((const u128 *) q);
+ u128 rr;
+
+ rr.a = pp.a ^ qq.a;
+ rr.b = pp.b ^ qq.b;
+
+ bt_put_unaligned(rr, (u128 *) r);
}
static int smp_c1(uint8_t r[16], uint8_t res[16])
@@ -260,7 +266,7 @@ static int smp_c1(uint8_t r[16], uint8_t res[16])
baswap((bdaddr_t *) (p2 + 10), (bdaddr_t *) data->ra);
/* res = r XOR p1 */
- u128_xor((u128 *) res, (u128 *) r, (u128 *) p1);
+ u128_xor(res, r, p1);
/* res = e(k, res) */
err = smp_e(data->smp_tk, res, res);
@@ -268,7 +274,7 @@ static int smp_c1(uint8_t r[16], uint8_t res[16])
return err;
/* res = res XOR p2 */
- u128_xor((u128 *) res, (u128 *) res, (u128 *) p2);
+ u128_xor(res, res, p2);
/* res = e(k, res) */
return smp_e(data->smp_tk, res, res);
--
1.8.3.2
^ permalink raw reply related
* [PATCH BlueZ 07/12] tools: Fix possible uninitialized variable in obexctl
From: Anderson Lizardo @ 2014-01-04 1:55 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
In-Reply-To: <1388800528-10699-1-git-send-email-anderson.lizardo@openbossa.org>
On set_default_session(), if g_dbus_proxy_get_property() returns FALSE,
desc will be uninitialized. Given that this function already checks for
NULL proxy internally, it is enough to check whether it fails (and if
so, set a default prompt without destination).
Fixes this clang error:
tools/obexctl.c:439:6: error: variable 'desc' is used uninitialized
whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized]
---
tools/obexctl.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/tools/obexctl.c b/tools/obexctl.c
index 2e38298..f0d5438 100644
--- a/tools/obexctl.c
+++ b/tools/obexctl.c
@@ -431,14 +431,12 @@ static void set_default_session(GDBusProxy *proxy)
default_session = proxy;
- if (proxy == NULL) {
+ if (!g_dbus_proxy_get_property(proxy, "Destination", &iter)) {
desc = g_strdup(PROMPT_ON);
goto done;
}
- if (g_dbus_proxy_get_property(proxy, "Destination", &iter))
- dbus_message_iter_get_basic(&iter, &desc);
-
+ dbus_message_iter_get_basic(&iter, &desc);
desc = g_strdup_printf(COLOR_BLUE "[%s]" COLOR_OFF "# ", desc);
done:
--
1.8.3.2
^ permalink raw reply related
* [PATCH BlueZ 08/12] android/client: Fix incorrect usage of bt_state_t2str()
From: Anderson Lizardo @ 2014-01-04 1:55 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
In-Reply-To: <1388800528-10699-1-git-send-email-anderson.lizardo@openbossa.org>
Fixes clang error:
android/client/if-bt.c:313:54: error: implicit conversion from
enumeration type 'bt_status_t' to different enumeration type
'bt_state_t' [-Werror,-Wenum-conversion]
---
android/client/if-bt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/android/client/if-bt.c b/android/client/if-bt.c
index 10b79f1..6771df4 100644
--- a/android/client/if-bt.c
+++ b/android/client/if-bt.c
@@ -310,7 +310,7 @@ static void dut_mode_recv_cb(uint16_t opcode, uint8_t *buf, uint8_t len)
#if PLATFORM_SDK_VERSION > 17
static void le_test_mode_cb(bt_status_t status, uint16_t num_packets)
{
- haltest_info("%s %s %d\n", __func__, bt_state_t2str(status),
+ haltest_info("%s %s %d\n", __func__, bt_status_t2str(status),
num_packets);
}
#endif
--
1.8.3.2
^ permalink raw reply related
* [PATCH BlueZ 09/12] android/client: Remove duplicate "const" specifier
From: Anderson Lizardo @ 2014-01-04 1:55 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
In-Reply-To: <1388800528-10699-1-git-send-email-anderson.lizardo@openbossa.org>
Fixes clang errors like:
android/client/if-hh.c:181:39: error: duplicate 'const' declaration
specifier [-Werror,-Wduplicate-decl-specifier]
---
android/client/if-hh.c | 16 ++++++++--------
android/client/if-pan.c | 8 ++++----
2 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/android/client/if-hh.c b/android/client/if-hh.c
index b8ebc8e..56eb698 100644
--- a/android/client/if-hh.c
+++ b/android/client/if-hh.c
@@ -178,7 +178,7 @@ static void init_p(int argc, const char **argv)
/* connect */
-static void connect_c(int argc, const const char **argv, enum_func *enum_func,
+static void connect_c(int argc, const char **argv, enum_func *enum_func,
void **user)
{
if (argc == 3) {
@@ -243,8 +243,8 @@ static void set_info_p(int argc, const char **argv)
/* get_protocol */
-static void get_protocol_c(int argc, const const char **argv,
- enum_func *enum_func, void **user)
+static void get_protocol_c(int argc, const char **argv, enum_func *enum_func,
+ void **user)
{
if (argc == 3) {
*user = connected_device_addr;
@@ -296,8 +296,8 @@ static void set_protocol_p(int argc, const char **argv)
/* get_report */
-static void get_report_c(int argc, const const char **argv,
- enum_func *enum_func, void **user)
+static void get_report_c(int argc, const char **argv, enum_func *enum_func,
+ void **user)
{
if (argc == 3) {
*user = connected_device_addr;
@@ -341,8 +341,8 @@ static void get_report_p(int argc, const char **argv)
/* set_report */
-static void set_report_c(int argc, const const char **argv,
- enum_func *enum_func, void **user)
+static void set_report_c(int argc, const char **argv, enum_func *enum_func,
+ void **user)
{
if (argc == 3) {
*user = connected_device_addr;
@@ -377,7 +377,7 @@ static void set_report_p(int argc, const char **argv)
/* send_data */
-static void send_data_c(int argc, const const char **argv, enum_func *enum_func,
+static void send_data_c(int argc, const char **argv, enum_func *enum_func,
void **user)
{
if (argc == 3) {
diff --git a/android/client/if-pan.c b/android/client/if-pan.c
index a11f2a3..bdb36cc 100644
--- a/android/client/if-pan.c
+++ b/android/client/if-pan.c
@@ -80,7 +80,7 @@ static void init_p(int argc, const char **argv)
/* enable */
-static void enable_c(int argc, const const char **argv, enum_func *enum_func,
+static void enable_c(int argc, const char **argv, enum_func *enum_func,
void **user)
{
if (argc == 3) {
@@ -121,7 +121,7 @@ static void get_local_role_p(int argc, const char **argv)
/* connect */
-static void connect_c(int argc, const const char **argv, enum_func *enum_func,
+static void connect_c(int argc, const char **argv, enum_func *enum_func,
void **user)
{
if (argc == 3) {
@@ -165,8 +165,8 @@ static void connect_p(int argc, const char **argv)
/* disconnect */
-static void disconnect_c(int argc, const const char **argv,
- enum_func *enum_func, void **user)
+static void disconnect_c(int argc, const char **argv, enum_func *enum_func,
+ void **user)
{
if (argc == 3) {
*user = last_used_addr;
--
1.8.3.2
^ permalink raw reply related
* [PATCH BlueZ 10/12] android/client: Use memcpy() for getting CMSG_DATA()
From: Anderson Lizardo @ 2014-01-04 1:55 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
In-Reply-To: <1388800528-10699-1-git-send-email-anderson.lizardo@openbossa.org>
This is how it is done in all other places in BlueZ. Also drop
unnecessary "descs" local variable.
Fixes clang error:
android/client/if-sock.c:164:11: error: cast from 'unsigned char *' to
'int *' increases required alignment from 1 to 4 [-Werror,-Wcast-align]
---
android/client/if-sock.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/android/client/if-sock.c b/android/client/if-sock.c
index 5394a5d..050bc96 100644
--- a/android/client/if-sock.c
+++ b/android/client/if-sock.c
@@ -154,21 +154,19 @@ static void read_accepted(int fd)
for (cmsgptr = CMSG_FIRSTHDR(&msg);
cmsgptr != NULL; cmsgptr = CMSG_NXTHDR(&msg, cmsgptr)) {
- int *descs;
int count;
if (cmsgptr->cmsg_level != SOL_SOCKET ||
cmsgptr->cmsg_type != SCM_RIGHTS)
continue;
- descs = (int *) CMSG_DATA(cmsgptr);
+ memcpy(&accepted_fd, CMSG_DATA(cmsgptr), sizeof(accepted_fd));
count = ((cmsgptr->cmsg_len - CMSG_LEN(0)) / sizeof(int));
if (count != 1)
haltest_error("Failed to accept descriptors count=%d\n",
count);
- accepted_fd = descs[0];
break;
}
--
1.8.3.2
^ permalink raw reply related
* [PATCH BlueZ 11/12] android/client: Fix uninitialized "sock_fd" variable
From: Anderson Lizardo @ 2014-01-04 1:55 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
In-Reply-To: <1388800528-10699-1-git-send-email-anderson.lizardo@openbossa.org>
If EXEC() macro is called with the first pointer argument NULL, sock_fd
will not be initialized. Given that the NULL check is not fatal, it is a
good idea to initialize the variable to -1 so the code has defined
behavior on this situation.
Detected by clang:
android/client/if-sock.c:251:7: error: variable 'sock_fd' is used
uninitialized whenever 'if' condition is false
[-Werror,-Wsometimes-uninitialized]
---
android/client/if-sock.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/android/client/if-sock.c b/android/client/if-sock.c
index 050bc96..4c1af82 100644
--- a/android/client/if-sock.c
+++ b/android/client/if-sock.c
@@ -207,7 +207,7 @@ static void listen_p(int argc, const char **argv)
const char *service_name;
bt_uuid_t service_uuid;
int channel;
- int sock_fd;
+ int sock_fd = -1;
int flags;
RETURN_IF_NULL(if_sock);
@@ -281,7 +281,7 @@ static void connect_p(int argc, const char **argv)
btsock_type_t type;
bt_uuid_t uuid;
int channel;
- int sock_fd;
+ int sock_fd = -1;
int flags;
/* Address */
--
1.8.3.2
^ permalink raw reply related
* [PATCH BlueZ 12/12] android/system-emulator: Remove useless "static" qualifier
From: Anderson Lizardo @ 2014-01-04 1:55 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
In-Reply-To: <1388800528-10699-1-git-send-email-anderson.lizardo@openbossa.org>
The value of SYSTEM_SOCKET_PATH is just copied to another memory
location using memcpy() (on the same function), therefore the static
qualifier is unnecessary.
---
android/system-emulator.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/android/system-emulator.c b/android/system-emulator.c
index 2dc9ad8..f1c6622 100644
--- a/android/system-emulator.c
+++ b/android/system-emulator.c
@@ -185,8 +185,7 @@ static void signal_callback(int signum, void *user_data)
int main(int argc, char *argv[])
{
- static const char SYSTEM_SOCKET_PATH[] = "\0android_system";
-
+ const char SYSTEM_SOCKET_PATH[] = "\0android_system";
sigset_t mask;
struct sockaddr_un addr;
int fd;
--
1.8.3.2
^ permalink raw reply related
* Re: [REGRESSION] rfcomm (userland) broken by commit 29cd718b
From: Benson Chow @ 2014-01-04 4:32 UTC (permalink / raw)
To: Gianluca Anzolin; +Cc: linux-bluetooth
In-Reply-To: <20131228084421.GA27196@sottospazio.it>
Gianluca,
This patch looks like it did it, from a quick check, everything looks like
it's working as well as what it was before the 3.8 patch.
Thanks a bunch!
-Benson
On Sat, 28 Dec 2013, Gianluca Anzolin wrote:
> Date: Sat, 28 Dec 2013 09:44:21 +0100
> From: Gianluca Anzolin <gianluca@sottospazio.it>
> To: Benson Chow <blc+bluez@mail.vanade.com>
> Cc: linux-bluetooth@vger.kernel.org
> Subject: Re: [REGRESSION] rfcomm (userland) broken by commit 29cd718b
>
> On Fri, Dec 27, 2013 at 04:01:27PM -0700, Benson Chow wrote:
>> First off, thanks for the fix to stop rfcomm from taking down the
>> machine. However, I have noted that blueman and
>> networkmanager/modemmanager no longer recognize the /dev/rfcomm0
>> device as a valid dialup device. This seems to be a
>> kernel-userspace interface regression as I can boot into 3.6.11 and
>> it would work just fine.
>>
>> When I saw this thread, I agree there appears to be some
>> kernel-userspace changes that broke something, but the recent patch
>> still did not seem to let modemmanger detect the bluetooth device as
>> it did pre linux-3.12.
>>
>> Blueman reports "connection failed: modem manager did not support
>> the connection" implying there's still some userspace differences
>> from the old behavior.
>>
>> I do notice I can run a terminal emulator on /dev/rfcomm0 and able
>> to run modem AT-commands which means that I can communicate with the
>> phone through bluetooth, so that part is working. Plus, tearing up
>> that connection no longer results in a crash like before linux-3.8.
>>
>> Any other information I could get from my system to help debug
>> what's going on here? Or perhaps modem-manager will need to be
>> updated to work with the new behavior?
>>
>> Thanks,
>> -Benson
>
> Thank you for the report.
>
> Could you try the attached patch on top of the last rfc3.patch and see if it
> works?
>
> Regards,
> Gianluca
>
WARNING: All HTML emails get auto deleted. DO NOT SEND HTML MAIL.
WARNING: Emails with large typo counts/spelling errors will also be deleted.
^ permalink raw reply
* [PATCH 0/2] Regression fixes for rfcomm/tty.c
From: Gianluca Anzolin @ 2014-01-04 16:30 UTC (permalink / raw)
To: gustavo
Cc: peter, marcel, linux-bluetooth, gregkh, jslaby, stable,
Gianluca Anzolin
The following two patches fix a couple of regressions introduced with
the rfcomm tty_port conversion.
The first patch restores the expected behaviour of the rfcomm port when
it's created with the flag RFCOMM_RELEASE_ONHUP.
The second patch fixes a bug that affect userspace (ModemManager) when
the port is created with the flag RFCOMM_REUSE_DLC.
Gianluca Anzolin (2):
rfcomm: release the port when the last user closes the tty
rfcomm: move the device under its parent if already connected
net/bluetooth/rfcomm/tty.c | 68 ++++++++++++++++++++++++++--------------------
1 file changed, 39 insertions(+), 29 deletions(-)
--
1.8.5.2
^ permalink raw reply
* [PATCH 1/2] rfcomm: release the port when the last user closes the tty
From: Gianluca Anzolin @ 2014-01-04 16:30 UTC (permalink / raw)
To: gustavo
Cc: peter, marcel, linux-bluetooth, gregkh, jslaby, stable,
Gianluca Anzolin
In-Reply-To: <1388853038-6917-1-git-send-email-gianluca@sottospazio.it>
This patch fixes a userspace regression introduced by the commit
29cd718b.
If the rfcomm device was created with the flag RFCOMM_RELEASE_ONHUP the
user space expects that the tty_port is released as soon as the last
process closes the tty.
The current code attempts to release the port in the function
rfcomm_dev_state_change(). However it won't get a reference to the
relevant tty to send a HUP: at that point the tty is already destroyed
and therefore NULL.
This patch fixes the regression by taking over the tty refcount in the
tty install method(). This way the tty_port is automatically released as
soon as the tty is destroyed.
As a consequence the check for RFCOMM_RELEASE_ONHUP flag in the hangup()
method is now redundant. Instead we have to be careful with the reference
counting in the rfcomm_release_dev() function.
Signed-off-by: Gianluca Anzolin <gianluca@sottospazio.it>
Reported-by: Alexander Holler <holler@ahsoftware.de>
---
net/bluetooth/rfcomm/tty.c | 21 ++++++++++++++-------
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index 84fcf9f..a535ef1 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -437,7 +437,8 @@ static int rfcomm_release_dev(void __user *arg)
tty_kref_put(tty);
}
- if (!test_and_set_bit(RFCOMM_TTY_RELEASED, &dev->flags))
+ if (!test_bit(RFCOMM_RELEASE_ONHUP, &dev->flags) &&
+ !test_and_set_bit(RFCOMM_TTY_RELEASED, &dev->flags))
tty_port_put(&dev->port);
tty_port_put(&dev->port);
@@ -670,10 +671,20 @@ static int rfcomm_tty_install(struct tty_driver *driver, struct tty_struct *tty)
/* install the tty_port */
err = tty_port_install(&dev->port, driver, tty);
- if (err)
+ if (err) {
rfcomm_tty_cleanup(tty);
+ return err;
+ }
- return err;
+ /* take over the tty_port reference if the port was created with the
+ * flag RFCOMM_RELEASE_ONHUP. This will force the release of the port
+ * when the last process closes the tty. The behaviour is expected by
+ * userspace.
+ */
+ if (test_bit(RFCOMM_RELEASE_ONHUP, &dev->flags))
+ tty_port_put(&dev->port);
+
+ return 0;
}
static int rfcomm_tty_open(struct tty_struct *tty, struct file *filp)
@@ -1010,10 +1021,6 @@ static void rfcomm_tty_hangup(struct tty_struct *tty)
BT_DBG("tty %p dev %p", tty, dev);
tty_port_hangup(&dev->port);
-
- if (test_bit(RFCOMM_RELEASE_ONHUP, &dev->flags) &&
- !test_and_set_bit(RFCOMM_TTY_RELEASED, &dev->flags))
- tty_port_put(&dev->port);
}
static int rfcomm_tty_tiocmget(struct tty_struct *tty)
--
1.8.5.2
^ permalink raw reply related
* [PATCH 2/2] rfcomm: move the device under its parent if already connected
From: Gianluca Anzolin @ 2014-01-04 16:30 UTC (permalink / raw)
To: gustavo
Cc: peter, marcel, linux-bluetooth, gregkh, jslaby, stable,
Gianluca Anzolin
In-Reply-To: <1388853038-6917-1-git-send-email-gianluca@sottospazio.it>
This patch fixes a userspace regression introduced with the conversion
of the rfcomm tty code to a proper tty_port driver.
Currently the tty device is moved under the parent only when a
successful BT connection is established.
Unfortunately this doesn't take into account the situation in which the
BT connection is already there, i.e. when the rfcomm device is created
with the flag RFCOMM_REUSE_DLC: in this case the tty device is never
moved under its parent.
This causes a regression with ModemManager which checks the parent
device to determine if the tty is likely to be connected to a modem:
it finds a NULL parent and skip the AT probe sequence.
The patch fixes the regression by calling device_move() in the function
rfcomm_dev_carrier_raised().
Signed-off-by: Gianluca Anzolin <gianluca@sottospazio.it>
Reported-by: Beson Chow <blc+bluez@mail.vanade.com>
---
net/bluetooth/rfcomm/tty.c | 47 ++++++++++++++++++++++++----------------------
1 file changed, 25 insertions(+), 22 deletions(-)
diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index a535ef1..9e217d7 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -111,12 +111,34 @@ static int rfcomm_dev_activate(struct tty_port *port, struct tty_struct *tty)
return rfcomm_dlc_open(dev->dlc, &dev->src, &dev->dst, dev->channel);
}
+static struct device *rfcomm_get_device(struct rfcomm_dev *dev)
+{
+ struct hci_dev *hdev;
+ struct hci_conn *conn;
+
+ hdev = hci_get_route(&dev->dst, &dev->src);
+ if (!hdev)
+ return NULL;
+
+ conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &dev->dst);
+
+ hci_dev_put(hdev);
+
+ return conn ? &conn->dev : NULL;
+}
+
/* we block the open until the dlc->state becomes BT_CONNECTED */
static int rfcomm_dev_carrier_raised(struct tty_port *port)
{
struct rfcomm_dev *dev = container_of(port, struct rfcomm_dev, port);
- return (dev->dlc->state == BT_CONNECTED);
+ if (dev->dlc->state == BT_CONNECTED) {
+ device_move(dev->tty_dev, rfcomm_get_device(dev),
+ DPM_ORDER_DEV_AFTER_PARENT);
+ return 1;
+ }
+
+ return 0;
}
/* device-specific cleanup: close the dlc */
@@ -169,22 +191,6 @@ static struct rfcomm_dev *rfcomm_dev_get(int id)
return dev;
}
-static struct device *rfcomm_get_device(struct rfcomm_dev *dev)
-{
- struct hci_dev *hdev;
- struct hci_conn *conn;
-
- hdev = hci_get_route(&dev->dst, &dev->src);
- if (!hdev)
- return NULL;
-
- conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &dev->dst);
-
- hci_dev_put(hdev);
-
- return conn ? &conn->dev : NULL;
-}
-
static ssize_t show_address(struct device *tty_dev, struct device_attribute *attr, char *buf)
{
struct rfcomm_dev *dev = dev_get_drvdata(tty_dev);
@@ -576,12 +582,9 @@ static void rfcomm_dev_state_change(struct rfcomm_dlc *dlc, int err)
BT_DBG("dlc %p dev %p err %d", dlc, dev, err);
dev->err = err;
- if (dlc->state == BT_CONNECTED) {
- device_move(dev->tty_dev, rfcomm_get_device(dev),
- DPM_ORDER_DEV_AFTER_PARENT);
-
+ if (dlc->state == BT_CONNECTED)
wake_up_interruptible(&dev->port.open_wait);
- } else if (dlc->state == BT_CLOSED)
+ else if (dlc->state == BT_CLOSED)
tty_port_tty_hangup(&dev->port, false);
}
--
1.8.5.2
^ permalink raw reply related
* Re: [PATCH 1/2] Bluetooth: Add quirk for disabling Delete Stored Link Key command
From: Johan Hedberg @ 2014-01-04 18:12 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <1388746956-9944-1-git-send-email-marcel@holtmann.org>
Hi Marcel,
On Fri, Jan 03, 2014, Marcel Holtmann wrote:
> Some controller pretend they support the Delete Stored Link Key command,
> but in reality they really don't support it.
>
> < HCI Command: Delete Stored Link Key (0x03|0x0012) plen 7
> bdaddr 00:00:00:00:00:00 all 1
> > HCI Event: Command Complete (0x0e) plen 4
> Delete Stored Link Key (0x03|0x0012) ncmd 1
> status 0x11 deleted 0
> Error: Unsupported Feature or Parameter Value
>
> Not correctly supporting this command causes the controller setup to
> fail and will make a device not work. However sending the command for
> controller that handle stored link keys is important. This quirk
> allows a driver to disable the command if it knows that this command
> handling is broken.
>
> Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
> ---
> include/net/bluetooth/hci.h | 3 ++-
> net/bluetooth/hci_core.c | 7 ++++++-
> 2 files changed, 8 insertions(+), 2 deletions(-)
Both patches have been applied to the bluetooth-next tree. Thanks.
Johan
^ permalink raw reply
* [PATCH 1/3] android/socket: Make channel int32_t in IPC specification
From: Szymon Janc @ 2014-01-04 20:16 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
This match IPC type with type in socket HAL API. This allows to pass
data directly from HAL library and will allow to reduce logic in it.
---
android/hal-msg.h | 20 ++++++++++----------
android/socket.c | 2 +-
2 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/android/hal-msg.h b/android/hal-msg.h
index c351501..bbbb99c 100644
--- a/android/hal-msg.h
+++ b/android/hal-msg.h
@@ -233,20 +233,20 @@ struct hal_cmd_le_test_mode {
#define HAL_OP_SOCK_LISTEN 0x01
struct hal_cmd_sock_listen {
- uint8_t type;
- uint8_t name[256];
- uint8_t uuid[16];
- uint16_t channel;
- uint8_t flags;
+ uint8_t type;
+ uint8_t name[256];
+ uint8_t uuid[16];
+ int32_t channel;
+ uint8_t flags;
} __attribute__((packed));
#define HAL_OP_SOCK_CONNECT 0x02
struct hal_cmd_sock_connect {
- uint8_t bdaddr[6];
- uint8_t type;
- uint8_t uuid[16];
- uint16_t channel;
- uint8_t flags;
+ uint8_t bdaddr[6];
+ uint8_t type;
+ uint8_t uuid[16];
+ int32_t channel;
+ uint8_t flags;
} __attribute__((packed));
/* Bluetooth HID Host HAL API */
diff --git a/android/socket.c b/android/socket.c
index 11d64f8..f68fbf0 100644
--- a/android/socket.c
+++ b/android/socket.c
@@ -804,7 +804,7 @@ static void handle_listen(const void *buf, uint16_t len)
profile = get_profile_by_uuid(cmd->uuid);
if (!profile) {
- if (!cmd->channel)
+ if (cmd->channel <= 0)
goto failed;
chan = cmd->channel;
--
1.8.5.2
^ permalink raw reply related
* [PATCH 2/3] android/socket: Move logic from HAL to daemon in listen
From: Szymon Janc @ 2014-01-04 20:16 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1388866574-20158-1-git-send-email-szymon.janc@gmail.com>
This reduce logic in HAL to bare minimum e.g. no modifications in
library will be needed to add different socket type support.
Both bdaddr2str and btuuid2str handle NULL pointers so it is safe to
print debug unconditionally.
---
android/hal-msg.h | 4 +++
android/hal-sock.c | 36 ++++++--------------------
android/socket.c | 74 +++++++++++++++++++++++++++++++++++++-----------------
3 files changed, 63 insertions(+), 51 deletions(-)
diff --git a/android/hal-msg.h b/android/hal-msg.h
index bbbb99c..ceaa3b2 100644
--- a/android/hal-msg.h
+++ b/android/hal-msg.h
@@ -231,6 +231,10 @@ struct hal_cmd_le_test_mode {
/* Bluetooth Socket HAL api */
+#define HAL_SOCK_RFCOMM 0x01
+#define HAL_SOCK_SCO 0x02
+#define HAL_SOCK_L2CAP 0x03
+
#define HAL_OP_SOCK_LISTEN 0x01
struct hal_cmd_sock_listen {
uint8_t type;
diff --git a/android/hal-sock.c b/android/hal-sock.c
index c39ca6a..47de63d 100644
--- a/android/hal-sock.c
+++ b/android/hal-sock.c
@@ -26,18 +26,23 @@
#include "hal-utils.h"
#include "hal.h"
-static bt_status_t sock_listen_rfcomm(const char *service_name,
+static bt_status_t sock_listen(btsock_type_t type, const char *service_name,
const uint8_t *uuid, int chan,
int *sock, int flags)
{
struct hal_cmd_sock_listen cmd;
- DBG("");
+ if (!sock)
+ return BT_STATUS_PARM_INVALID;
+
+ DBG("uuid %s chan %d sock %p type %d service_name %s flags 0x%02x",
+ btuuid2str(uuid), chan, sock, type, service_name, flags);
memset(&cmd, 0, sizeof(cmd));
+ /* type match IPC type */
+ cmd.type = type;
cmd.flags = flags;
- cmd.type = BTSOCK_RFCOMM;
cmd.channel = chan;
if (uuid)
@@ -50,31 +55,6 @@ static bt_status_t sock_listen_rfcomm(const char *service_name,
sizeof(cmd), &cmd, NULL, NULL, sock);
}
-static bt_status_t sock_listen(btsock_type_t type, const char *service_name,
- const uint8_t *uuid, int chan,
- int *sock, int flags)
-{
- if ((!uuid && chan <= 0) || !sock || !type) {
- error("Invalid params: uuid %s, chan %d, sock %p",
- btuuid2str(uuid), chan, sock);
- return BT_STATUS_PARM_INVALID;
- }
-
- DBG("uuid %s chan %d sock %p type %d service_name %s flags 0x%02x",
- btuuid2str(uuid), chan, sock, type, service_name, flags);
-
- switch (type) {
- case BTSOCK_RFCOMM:
- return sock_listen_rfcomm(service_name, uuid, chan, sock,
- flags);
- default:
- error("%s: Socket type %d not supported", __func__, type);
- break;
- }
-
- return BT_STATUS_UNSUPPORTED;
-}
-
static bt_status_t sock_connect(const bt_bdaddr_t *bdaddr, btsock_type_t type,
const uint8_t *uuid, int chan,
int *sock, int flags)
diff --git a/android/socket.c b/android/socket.c
index f68fbf0..98862cb 100644
--- a/android/socket.c
+++ b/android/socket.c
@@ -62,6 +62,8 @@
static bdaddr_t adapter_addr;
+static const uint8_t zero_uuid[16] = { 0 };
+
/* Simple list of RFCOMM server sockets */
GList *servers = NULL;
@@ -787,38 +789,40 @@ static void accept_cb(GIOChannel *io, GError *err, gpointer user_data)
rfsock_acc->rfcomm_watch);
}
-static void handle_listen(const void *buf, uint16_t len)
+static uint8_t rfcomm_listen(int chan, const uint8_t *name, const uint8_t *uuid,
+ uint8_t flags, int *hal_fd)
{
- const struct hal_cmd_sock_listen *cmd = buf;
const struct profile_info *profile;
struct rfcomm_sock *rfsock = NULL;
BtIOSecLevel sec_level;
GIOChannel *io, *io_stack;
GIOCondition cond;
GError *err = NULL;
- int hal_fd = -1;
- int chan;
guint id;
DBG("");
- profile = get_profile_by_uuid(cmd->uuid);
+ if (!memcmp(uuid, zero_uuid, sizeof(zero_uuid)) && chan <= 0) {
+ error("Invalid rfcomm listen params");
+ return HAL_STATUS_INVALID;
+ }
+
+ profile = get_profile_by_uuid(uuid);
if (!profile) {
- if (cmd->channel <= 0)
- goto failed;
+ if (chan <= 0)
+ return HAL_STATUS_INVALID;
- chan = cmd->channel;
sec_level = BT_IO_SEC_MEDIUM;
} else {
chan = profile->channel;
sec_level = profile->sec_level;
}
- DBG("rfcomm channel %d svc_name %s", chan, cmd->name);
+ DBG("rfcomm channel %d svc_name %s", chan, name);
- rfsock = create_rfsock(-1, &hal_fd);
+ rfsock = create_rfsock(-1, hal_fd);
if (!rfsock)
- goto failed;
+ return HAL_STATUS_FAILED;
io = bt_io_listen(accept_cb, NULL, rfsock, NULL, &err,
BT_IO_OPT_SOURCE_BDADDR, &adapter_addr,
@@ -847,31 +851,56 @@ static void handle_listen(const void *buf, uint16_t len)
rfsock->stack_watch = id;
DBG("real_sock %d fd %d hal_fd %d", rfsock->real_sock, rfsock->fd,
- hal_fd);
+ *hal_fd);
if (write(rfsock->fd, &chan, sizeof(chan)) != sizeof(chan)) {
error("Error sending RFCOMM channel");
goto failed;
}
- rfsock->service_handle = sdp_service_register(profile, cmd->name);
+ rfsock->service_handle = sdp_service_register(profile, name);
servers = g_list_append(servers, rfsock);
+ return HAL_STATUS_SUCCESS;
+
+failed:
+
+ cleanup_rfsock(rfsock);
+ close(*hal_fd);
+ return HAL_STATUS_FAILED;
+}
+
+static void handle_listen(const void *buf, uint16_t len)
+{
+ const struct hal_cmd_sock_listen *cmd = buf;
+ uint8_t status;
+ int hal_fd;
+
+ switch (cmd->type) {
+ case HAL_SOCK_RFCOMM:
+ status = rfcomm_listen(cmd->channel, cmd->name, cmd->uuid,
+ cmd->flags, &hal_fd);
+ break;
+ case HAL_SOCK_SCO:
+ case HAL_SOCK_L2CAP:
+ status = HAL_STATUS_UNSUPPORTED;
+ break;
+ default:
+ status = HAL_STATUS_INVALID;
+ break;
+ }
+
+ if (status != HAL_STATUS_SUCCESS)
+ goto failed;
+
ipc_send_rsp_full(HAL_SERVICE_ID_SOCK, HAL_OP_SOCK_LISTEN, 0, NULL,
hal_fd);
close(hal_fd);
- return;
+ return ;
failed:
- ipc_send_rsp(HAL_SERVICE_ID_SOCK, HAL_OP_SOCK_LISTEN,
- HAL_STATUS_FAILED);
-
- if (rfsock)
- cleanup_rfsock(rfsock);
-
- if (hal_fd >= 0)
- close(hal_fd);
+ ipc_send_rsp(HAL_SERVICE_ID_SOCK, HAL_OP_SOCK_LISTEN, status);
}
static bool sock_send_connect(struct rfcomm_sock *rfsock, bdaddr_t *bdaddr)
@@ -1036,7 +1065,6 @@ fail:
static void handle_connect(const void *buf, uint16_t len)
{
const struct hal_cmd_sock_connect *cmd = buf;
- static const uint8_t zero_uuid[16] = { 0 };
struct rfcomm_sock *rfsock;
uuid_t uuid;
int hal_fd = -1;
--
1.8.5.2
^ permalink raw reply related
* [PATCH 3/3] android/socket: Move logic from HAL to daemon in connect
From: Szymon Janc @ 2014-01-04 20:16 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1388866574-20158-1-git-send-email-szymon.janc@gmail.com>
This reduce logic in HAL to bare minimum e.g. no modifications in
library will be needed to add different socket type support.
Both bdaddr2str and btuuid2str handle NULL pointers so it is safe to
print debug unconditionally.
---
android/hal-sock.c | 16 ++++-------
android/socket.c | 81 ++++++++++++++++++++++++++++++++++++++----------------
2 files changed, 62 insertions(+), 35 deletions(-)
diff --git a/android/hal-sock.c b/android/hal-sock.c
index 47de63d..f62b7ef 100644
--- a/android/hal-sock.c
+++ b/android/hal-sock.c
@@ -61,30 +61,24 @@ static bt_status_t sock_connect(const bt_bdaddr_t *bdaddr, btsock_type_t type,
{
struct hal_cmd_sock_connect cmd;
- if ((!uuid && chan <= 0) || !bdaddr || !sock || !type) {
- error("Invalid params: bd_addr %s, uuid %s, chan %d, sock %p",
- bdaddr2str(bdaddr), btuuid2str(uuid), chan, sock);
+ if (!sock)
return BT_STATUS_PARM_INVALID;
- }
DBG("bdaddr %s uuid %s chan %d sock %p type %d flags 0x%02x",
bdaddr2str(bdaddr), btuuid2str(uuid), chan, sock, type, flags);
- if (type != BTSOCK_RFCOMM) {
- error("Socket type %u not supported", type);
- return BT_STATUS_UNSUPPORTED;
- }
-
memset(&cmd, 0, sizeof(cmd));
- cmd.flags = flags;
+ /* type match IPC type */
cmd.type = type;
+ cmd.flags = flags;
cmd.channel = chan;
if (uuid)
memcpy(cmd.uuid, uuid, sizeof(cmd.uuid));
- memcpy(cmd.bdaddr, bdaddr, sizeof(cmd.bdaddr));
+ if (bdaddr)
+ memcpy(cmd.bdaddr, bdaddr, sizeof(cmd.bdaddr));
return hal_ipc_cmd(HAL_SERVICE_ID_SOCK, HAL_OP_SOCK_CONNECT,
sizeof(cmd), &cmd, NULL, NULL, sock);
diff --git a/android/socket.c b/android/socket.c
index 98862cb..69b39ee 100644
--- a/android/socket.c
+++ b/android/socket.c
@@ -975,7 +975,7 @@ fail:
cleanup_rfsock(rfsock);
}
-static bool do_connect(struct rfcomm_sock *rfsock, int chan)
+static bool do_rfcomm_connect(struct rfcomm_sock *rfsock, int chan)
{
BtIOSecLevel sec_level = BT_IO_SEC_MEDIUM;
GIOChannel *io;
@@ -1056,58 +1056,91 @@ static void sdp_search_cb(sdp_list_t *recs, int err, gpointer data)
DBG("Got RFCOMM channel %d", chan);
- if (do_connect(rfsock, chan))
+ if (do_rfcomm_connect(rfsock, chan))
return;
fail:
cleanup_rfsock(rfsock);
}
-static void handle_connect(const void *buf, uint16_t len)
+static uint8_t connect_rfcomm(const bdaddr_t *addr, int chan,
+ const uint8_t *uuid, uint8_t flags, int *hal_fd)
{
- const struct hal_cmd_sock_connect *cmd = buf;
struct rfcomm_sock *rfsock;
- uuid_t uuid;
- int hal_fd = -1;
+ uuid_t uu;
- DBG("");
+ if ((!memcmp(uuid, zero_uuid, sizeof(zero_uuid)) && chan <= 0) ||
+ !bacmp(addr, BDADDR_ANY)) {
+ error("Invalid rfcomm connect params");
+ return HAL_STATUS_INVALID;
+ }
- rfsock = create_rfsock(-1, &hal_fd);
+ rfsock = create_rfsock(-1, hal_fd);
if (!rfsock)
- goto failed;
+ return HAL_STATUS_FAILED;
- android2bdaddr(cmd->bdaddr, &rfsock->dst);
+ bacpy(&rfsock->dst, addr);
- if (!memcmp(cmd->uuid, zero_uuid, sizeof(zero_uuid))) {
- if (!do_connect(rfsock, cmd->channel))
+ if (!memcmp(uuid, zero_uuid, sizeof(zero_uuid))) {
+ if (!do_rfcomm_connect(rfsock, chan))
goto failed;
} else {
- memset(&uuid, 0, sizeof(uuid));
- uuid.type = SDP_UUID128;
- memcpy(&uuid.value.uuid128, cmd->uuid, sizeof(uint128_t));
+ memset(&uu, 0, sizeof(uu));
+ uu.type = SDP_UUID128;
+ memcpy(&uu.value.uuid128, uuid, sizeof(uint128_t));
- rfsock->profile = get_profile_by_uuid(cmd->uuid);
+ rfsock->profile = get_profile_by_uuid(uuid);
- if (bt_search_service(&adapter_addr, &rfsock->dst, &uuid,
+ if (bt_search_service(&adapter_addr, &rfsock->dst, &uu,
sdp_search_cb, rfsock, NULL) < 0) {
error("Failed to search SDP records");
goto failed;
}
}
+ return HAL_STATUS_SUCCESS;
+
+failed:
+ cleanup_rfsock(rfsock);
+ close(*hal_fd);
+ return HAL_STATUS_FAILED;
+}
+
+static void handle_connect(const void *buf, uint16_t len)
+{
+ const struct hal_cmd_sock_connect *cmd = buf;
+ bdaddr_t bdaddr;
+ uint8_t status;
+ int hal_fd;
+
+ DBG("");
+
+ android2bdaddr(cmd->bdaddr, &bdaddr);
+
+ switch (cmd->type) {
+ case HAL_SOCK_RFCOMM:
+ status = connect_rfcomm(&bdaddr, cmd->channel, cmd->uuid,
+ cmd->flags, &hal_fd);
+ break;
+ case HAL_SOCK_SCO:
+ case HAL_SOCK_L2CAP:
+ status = HAL_STATUS_UNSUPPORTED;
+ break;
+ default:
+ status = HAL_STATUS_INVALID;
+ break;
+ }
+
+ if (status != HAL_STATUS_SUCCESS)
+ goto failed;
+
ipc_send_rsp_full(HAL_SERVICE_ID_SOCK, HAL_OP_SOCK_CONNECT, 0, NULL,
hal_fd);
close(hal_fd);
return;
failed:
- ipc_send_rsp(HAL_SERVICE_ID_SOCK, HAL_OP_SOCK_CONNECT,
- HAL_STATUS_FAILED);
-
- if (rfsock)
- cleanup_rfsock(rfsock);
+ ipc_send_rsp(HAL_SERVICE_ID_SOCK, HAL_OP_SOCK_CONNECT, status);
- if (hal_fd >= 0)
- close(hal_fd);
}
static const struct ipc_handler cmd_handlers[] = {
--
1.8.5.2
^ permalink raw reply related
* Re: A problem with "rfcomm bind" and wvdial
From: Gianluca Anzolin @ 2014-01-05 15:49 UTC (permalink / raw)
To: andrey.vihrov; +Cc: peter, marcel, gregkh, jslaby, linux-bluetooth
Hello,
I looked at your problem this afternoon and I think I know what's happening:
wvdial is opening the port with the flag O_NONBLOCK. As expected the rfcomm
code returns immediately instead of waiting for the BT connection to come
up. Then wvdial sends the AT commands and the writes fail.
In the past it worked because the carrier_raised() logic was open coded inside
the rfcomm code: it always waited for a successful or failed connection even
with a non-blocking tty.
But now the code uses the tty_port methods and won't wait for a successful
bluetooth connection on open().
But there is more: when wvdial terminates, it will set the CLOCAL flag for the
tty and this will result in the same behaviour described above, even without
setting the O_NONBLOCK flag. This will affect other users of the tty.
These two cases make clear that the use of carrier_raised() method was not that
fair after all: while that allowed us to remove some code we now have to work
around it to get the previous behaviour.
For example we could mask off the O_NONBLOCK flag before opening the port and
then restore it when the tty_port_open() is completed. We could do something
similar for the CLOCAL flag: always remove it in rfcomm_tty_set_termios().
The alternative is to remove the carrier_raised() method and wait for the BT
connection in rfcomm_dev_activate() like the old code did.
I'm CC-ing some people who know the code better than me to get a suggestion
on how to proceed.
Thank you,
Gianluca
^ permalink raw reply
* Re: [PATCH BlueZ v2 01/10] android/ipc: Add initial code for audio IPC
From: Szymon Janc @ 2014-01-05 17:43 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <1388663914-25003-1-git-send-email-luiz.dentz@gmail.com>
Hi Luiz,
On Thursday 02 January 2014 13:58:25 Luiz Augusto von Dentz wrote:
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>
> This add initial code for listen and accept connections on the abstract
> socket defined for the audio IPC.
> ---
> v2: Split audio IPC services for HAL services and fix invalid messages or
> disconnections causing the daemon to exit. The audio HAL is independent of
> bluetooth and should only affect A2DP service.
What seems to be missing is some way to recover from Audio IPC error.
>
> android/hal-msg.h | 1 +
> android/ipc.c | 78
> ++++++++++++++++++++++++++++++++++++++++++++++++++----- android/ipc.h |
> 3 +++
> 3 files changed, 76 insertions(+), 6 deletions(-)
>
> diff --git a/android/hal-msg.h b/android/hal-msg.h
> index c351501..b14eced 100644
> --- a/android/hal-msg.h
> +++ b/android/hal-msg.h
> @@ -24,6 +24,7 @@
> #define BLUEZ_HAL_MTU 1024
>
> static const char BLUEZ_HAL_SK_PATH[] = "\0bluez_hal_socket";
> +static const char BLUEZ_AUDIO_SK_PATH[] = "\0bluez_audio_socket";
>
> struct hal_hdr {
> uint8_t service_id;
> diff --git a/android/ipc.c b/android/ipc.c
> index 9e8ccc3..4c5a77e 100644
> --- a/android/ipc.c
> +++ b/android/ipc.c
> @@ -49,6 +49,7 @@ static struct service_handler services[HAL_SERVICE_ID_MAX
> + 1];
>
> static GIOChannel *cmd_io = NULL;
> static GIOChannel *notif_io = NULL;
> +static GIOChannel *audio_io = NULL;
>
> static void ipc_handle_msg(const void *buf, ssize_t len)
> {
> @@ -145,7 +146,8 @@ static gboolean notif_watch_cb(GIOChannel *io,
> GIOCondition cond, return FALSE;
> }
>
> -static GIOChannel *connect_hal(GIOFunc connect_cb)
> +static GIOChannel *ipc_connect(const char *path, size_t size,
> + GIOFunc connect_cb)
> {
> struct sockaddr_un addr;
> GIOCondition cond;
> @@ -167,11 +169,11 @@ static GIOChannel *connect_hal(GIOFunc connect_cb)
> memset(&addr, 0, sizeof(addr));
> addr.sun_family = AF_UNIX;
>
> - memcpy(addr.sun_path, BLUEZ_HAL_SK_PATH, sizeof(BLUEZ_HAL_SK_PATH));
> + memcpy(addr.sun_path, path, size);
>
> if (connect(sk, (struct sockaddr *) &addr, sizeof(addr)) < 0) {
> - error("IPC: failed to connect HAL socket: %d (%s)", errno,
> - strerror(errno));
> + error("IPC: failed to connect HAL socket %s: %d (%s)", &path[1],
> + errno, strerror(errno));
> g_io_channel_unref(io);
> return NULL;
> }
> @@ -218,7 +220,8 @@ static gboolean cmd_connect_cb(GIOChannel *io,
> GIOCondition cond, return FALSE;
> }
>
> - notif_io = connect_hal(notif_connect_cb);
> + notif_io = ipc_connect(BLUEZ_HAL_SK_PATH, sizeof(BLUEZ_HAL_SK_PATH),
> + notif_connect_cb);
> if (!notif_io)
> raise(SIGTERM);
>
> @@ -227,7 +230,8 @@ static gboolean cmd_connect_cb(GIOChannel *io,
> GIOCondition cond,
>
> void ipc_init(void)
> {
> - cmd_io = connect_hal(cmd_connect_cb);
> + cmd_io = ipc_connect(BLUEZ_HAL_SK_PATH, sizeof(BLUEZ_HAL_SK_PATH),
> + cmd_connect_cb);
> if (!cmd_io)
> raise(SIGTERM);
> }
> @@ -338,3 +342,65 @@ void ipc_unregister(uint8_t service)
> services[service].handler = NULL;
> services[service].size = 0;
> }
> +
> +static gboolean audio_watch_cb(GIOChannel *io, GIOCondition cond,
> + gpointer user_data)
> +{
> + char buf[BLUEZ_HAL_MTU];
> + ssize_t ret;
> + int fd;
> +
> + if (cond & (G_IO_NVAL | G_IO_ERR | G_IO_HUP)) {
> + info("Audio IPC: command socket closed");
> + goto fail;
> + }
> +
> + fd = g_io_channel_unix_get_fd(io);
> +
> + ret = read(fd, buf, sizeof(buf));
> + if (ret < 0) {
> + error("Audio IPC: command read failed (%s)", strerror(errno));
> + goto fail;
> + }
> +
> + return TRUE;
> +
> +fail:
> + audio_ipc_cleanup();
> + return FALSE;
> +}
> +
> +static gboolean audio_connect_cb(GIOChannel *io, GIOCondition cond,
> + gpointer user_data)
> +{
> + DBG("");
> +
> + if (cond & (G_IO_NVAL | G_IO_ERR | G_IO_HUP)) {
> + error("Audio IPC: socket connect failed");
> + audio_ipc_cleanup();
> + return FALSE;
> + }
> +
> + cond = G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL;
> +
> + g_io_add_watch(audio_io, cond, audio_watch_cb, NULL);
> +
> + info("Audio IPC: successfully connected");
> +
> + return FALSE;
> +}
> +
> +void audio_ipc_init(void)
> +{
> + audio_io = ipc_connect(BLUEZ_AUDIO_SK_PATH, sizeof(BLUEZ_AUDIO_SK_PATH),
> + audio_connect_cb);
> +}
> +
> +void audio_ipc_cleanup(void)
> +{
> + if (audio_io) {
> + g_io_channel_shutdown(audio_io, TRUE, NULL);
> + g_io_channel_unref(audio_io);
> + audio_io = NULL;
> + }
> +}
> diff --git a/android/ipc.h b/android/ipc.h
> index 6cd102b..8e92811 100644
> --- a/android/ipc.h
> +++ b/android/ipc.h
> @@ -37,3 +37,6 @@ void ipc_send_notif(uint8_t service_id, uint8_t opcode,
> uint16_t len, void ipc_register(uint8_t service, const struct ipc_handler
> *handlers, uint8_t size);
> void ipc_unregister(uint8_t service);
> +
> +void audio_ipc_init(void);
> +void audio_ipc_cleanup(void);
--
Szymon K. Janc
szymon.janc@gmail.com
^ permalink raw reply
* Re: [PATCH v4] Bluetooth: Add hci_h4p driver
From: Pavel Machek @ 2014-01-05 22:32 UTC (permalink / raw)
To: Marcel Holtmann, Pali Rohár,
Ивайло Димитров,
Gustavo F. Padovan, Johan Hedberg, linux-kernel,
linux-bluetooth@vger.kernel.org development, Ville Tervo
In-Reply-To: <20140103010519.GA27678@earth.universe>
Hi!
> > Changes from v3: Moved platform data into
> > include/linux/platform_data/, something I missed before.
>
> As I wrote before Tony plans to remove the boardcode for all
> OMAP boards including the Nokia N900 for 3.14, so you cannot
> boot without DT from 3.14 onwards.
Well, I believe it was agreed that DT should work for at least one
version before platform support is removed.
> The drivers can still be initialized the old way using pdata quirks
> until all drivers are converted, but I think this driver can simply
> be prepared for DT directly:
Well, normally DT support means a bit of argumentation and bikeshed
painting... I believe it would be better to get the driver in as-is
and then work on adding DT support.
And yes, obviously it needs to be done.
Regards,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
^ permalink raw reply
* Re: [PATCH v4] Bluetooth: Add hci_h4p driver
From: Sebastian Reichel @ 2014-01-05 23:01 UTC (permalink / raw)
To: Pavel Machek
Cc: Marcel Holtmann, Pali Rohár,
Ивайло Димитров,
Gustavo F. Padovan, Johan Hedberg, linux-kernel,
linux-bluetooth@vger.kernel.org development, Ville Tervo
In-Reply-To: <20140105223250.GB27517@amd.pavel.ucw.cz>
[-- Attachment #1: Type: text/plain, Size: 1328 bytes --]
On Sun, Jan 05, 2014 at 11:32:50PM +0100, Pavel Machek wrote:
> > > Changes from v3: Moved platform data into
> > > include/linux/platform_data/, something I missed before.
> >
> > As I wrote before Tony plans to remove the boardcode for all
> > OMAP boards including the Nokia N900 for 3.14, so you cannot
> > boot without DT from 3.14 onwards.
>
> Well, I believe it was agreed that DT should work for at least one
> version before platform support is removed.
https://git.kernel.org/cgit/linux/kernel/git/tmlind/linux-omap.git/tag/?id=omap-for-v3.14/omap3-board-removal-signed
There have been some discussion with RMK and iirc at the end
the plan still was to remove the boardcode for 3.14.
> > The drivers can still be initialized the old way using pdata quirks
> > until all drivers are converted, but I think this driver can simply
> > be prepared for DT directly:
>
> Well, normally DT support means a bit of argumentation and bikeshed
> painting... I believe it would be better to get the driver in as-is
> and then work on adding DT support.
Maybe. Especially the UART reference may result in some discussion.
But having the discussion early means its finished earlier :)
Assuming this is not merged for 3.14 there is quite some time for
the DT binding discussion ;)
-- Sebastian
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v4] Bluetooth: Add hci_h4p driver
From: Pavel Machek @ 2014-01-06 0:27 UTC (permalink / raw)
To: Marcel Holtmann, Pali Rohár,
Ивайло Димитров,
Gustavo F. Padovan, Johan Hedberg, linux-kernel,
linux-bluetooth@vger.kernel.org development, Ville Tervo
In-Reply-To: <20140105230155.GA2681@earth.universe>
> > > The drivers can still be initialized the old way using pdata quirks
> > > until all drivers are converted, but I think this driver can simply
> > > be prepared for DT directly:
> >
> > Well, normally DT support means a bit of argumentation and bikeshed
> > painting... I believe it would be better to get the driver in as-is
> > and then work on adding DT support.
>
> Maybe. Especially the UART reference may result in some discussion.
> But having the discussion early means its finished earlier :)
Well, merging the basic driver does not mean discussion can't start
early. If it is going to be complex, that's a very good reason to do
it in separate step. That is, have a working driver, then add device
tree support.
Thanks,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
^ permalink raw reply
* Re: [PATCH BlueZ v2 01/10] android/ipc: Add initial code for audio IPC
From: Luiz Augusto von Dentz @ 2014-01-06 9:06 UTC (permalink / raw)
To: Szymon Janc; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <2688157.rF2s9RrCYM@athlon>
Hi Szymon,
On Sun, Jan 5, 2014 at 7:43 PM, Szymon Janc <szymon.janc@gmail.com> wrote:
> Hi Luiz,
>
> On Thursday 02 January 2014 13:58:25 Luiz Augusto von Dentz wrote:
>> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>>
>> This add initial code for listen and accept connections on the abstract
>> socket defined for the audio IPC.
>> ---
>> v2: Split audio IPC services for HAL services and fix invalid messages or
>> disconnections causing the daemon to exit. The audio HAL is independent of
>> bluetooth and should only affect A2DP service.
>
> What seems to be missing is some way to recover from Audio IPC error.
This has been discussed already with Lukasz, Audio HAL is a little bit
different than Bluetooth HAL because it does not seems to be able to
restart the service from the UI so the Audio IPC disconnecting are
probably due to crashes, we will indeed need a way to recover from
Audio IPC errors but it is not really a feature but more a error
recover which we can be done in a separate set.
Luiz Augusto von Dentz
^ permalink raw reply
* Re: A problem with "rfcomm bind" and wvdial
From: Gianluca Anzolin @ 2014-01-06 11:33 UTC (permalink / raw)
To: andrey.vihrov; +Cc: peter, marcel, gregkh, jslaby, linux-bluetooth
In-Reply-To: <20140105154942.GA12621@sottospazio.it>
[-- Attachment #1: Type: text/plain, Size: 550 bytes --]
On Sun, Jan 05, 2014 at 04:49:42PM +0100, Gianluca Anzolin wrote:
> Hello,
>
> I looked at your problem this afternoon and I think I know what's happening:
> wvdial is opening the port with the flag O_NONBLOCK. As expected the rfcomm
> code returns immediately instead of waiting for the BT connection to come
> up. Then wvdial sends the AT commands and the writes fail.
Hi,
could you please test the attached patch?
If it works and people are ok with it I'll submit it along with the other fixes
I already sent to the list.
Thank you,
Gianluca
[-- Attachment #2: 0001-rfcomm-always-wait-for-a-bt-connection-on-open.patch --]
[-- Type: text/x-diff, Size: 5303 bytes --]
>From 2da1095aceb22bc323ceb9e1a4f43d92ebe5c2b6 Mon Sep 17 00:00:00 2001
From: Gianluca Anzolin <gianluca@sottospazio.it>
Date: Mon, 6 Jan 2014 12:19:51 +0100
Subject: [PATCH] rfcomm: always wait for a bt connection on open()
This patch fixes a regression introduced with the recent rfcomm tty
rework.
The current code uses the carrier_raised() method to wait for the
bluetooth connection when a process opens the tty. However processes may
open the port with the O_NONBLOCK flag or set the CLOCAL termios flag.
In those cases the open() syscall returns immediately without waiting
for the bluetooth connection to complete. This behaviour confuses
userspace which expects a working bluetooth connection.
The patch removes the carries_raised() method and restores the previous
working behaviour by waiting for the connection in rfcomm_dev_activate().
Signed-off-by: Gianluca Anzolin <gianluca@sottospazio.it>
Reported-by: Andrey Vihrov <andrey.vihrov@gmail.com>
---
net/bluetooth/rfcomm/tty.c | 79 ++++++++++++++++++++++++++++------------------
1 file changed, 49 insertions(+), 30 deletions(-)
diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index 84fcf9f..e9683bb 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -58,6 +58,7 @@ struct rfcomm_dev {
uint modem_status;
struct rfcomm_dlc *dlc;
+ wait_queue_head_t conn_wait;
struct device *tty_dev;
@@ -103,20 +104,57 @@ static void rfcomm_dev_destruct(struct tty_port *port)
module_put(THIS_MODULE);
}
-/* device-specific initialization: open the dlc */
-static int rfcomm_dev_activate(struct tty_port *port, struct tty_struct *tty)
+static struct device *rfcomm_get_device(struct rfcomm_dev *dev)
{
- struct rfcomm_dev *dev = container_of(port, struct rfcomm_dev, port);
+ struct hci_dev *hdev;
+ struct hci_conn *conn;
+
+ hdev = hci_get_route(&dev->dst, &dev->src);
+ if (!hdev)
+ return NULL;
+
+ conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &dev->dst);
+
+ hci_dev_put(hdev);
- return rfcomm_dlc_open(dev->dlc, &dev->src, &dev->dst, dev->channel);
+ return conn ? &conn->dev : NULL;
}
-/* we block the open until the dlc->state becomes BT_CONNECTED */
-static int rfcomm_dev_carrier_raised(struct tty_port *port)
+/* device-specific initialization: open the dlc */
+static int rfcomm_dev_activate(struct tty_port *port, struct tty_struct *tty)
{
struct rfcomm_dev *dev = container_of(port, struct rfcomm_dev, port);
+ DEFINE_WAIT(wait);
+ int err;
- return (dev->dlc->state == BT_CONNECTED);
+ err = rfcomm_dlc_open(dev->dlc, &dev->src, &dev->dst, dev->channel);
+ if (err)
+ return err;
+
+ while (1) {
+ prepare_to_wait(&dev->conn_wait, &wait, TASK_INTERRUPTIBLE);
+
+ if (dev->dlc->state == BT_CLOSED) {
+ err = -dev->err;
+ break;
+ } else if (dev->dlc->state == BT_CONNECTED)
+ break;
+ else if (signal_pending(current)) {
+ err = -ERESTARTSYS;
+ break;
+ }
+
+ tty_unlock(tty);
+ schedule();
+ tty_lock(tty);
+ }
+ finish_wait(&dev->conn_wait, &wait);
+
+ if (!err)
+ device_move(dev->tty_dev, rfcomm_get_device(dev),
+ DPM_ORDER_DEV_AFTER_PARENT);
+
+ return err;
}
/* device-specific cleanup: close the dlc */
@@ -135,7 +173,6 @@ static const struct tty_port_operations rfcomm_port_ops = {
.destruct = rfcomm_dev_destruct,
.activate = rfcomm_dev_activate,
.shutdown = rfcomm_dev_shutdown,
- .carrier_raised = rfcomm_dev_carrier_raised,
};
static struct rfcomm_dev *__rfcomm_dev_get(int id)
@@ -169,22 +206,6 @@ static struct rfcomm_dev *rfcomm_dev_get(int id)
return dev;
}
-static struct device *rfcomm_get_device(struct rfcomm_dev *dev)
-{
- struct hci_dev *hdev;
- struct hci_conn *conn;
-
- hdev = hci_get_route(&dev->dst, &dev->src);
- if (!hdev)
- return NULL;
-
- conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &dev->dst);
-
- hci_dev_put(hdev);
-
- return conn ? &conn->dev : NULL;
-}
-
static ssize_t show_address(struct device *tty_dev, struct device_attribute *attr, char *buf)
{
struct rfcomm_dev *dev = dev_get_drvdata(tty_dev);
@@ -258,6 +279,7 @@ static int rfcomm_dev_add(struct rfcomm_dev_req *req, struct rfcomm_dlc *dlc)
tty_port_init(&dev->port);
dev->port.ops = &rfcomm_port_ops;
+ init_waitqueue_head(&dev->conn_wait);
skb_queue_head_init(&dev->pending);
@@ -575,12 +597,9 @@ static void rfcomm_dev_state_change(struct rfcomm_dlc *dlc, int err)
BT_DBG("dlc %p dev %p err %d", dlc, dev, err);
dev->err = err;
- if (dlc->state == BT_CONNECTED) {
- device_move(dev->tty_dev, rfcomm_get_device(dev),
- DPM_ORDER_DEV_AFTER_PARENT);
+ wake_up_interruptible(&dev->conn_wait);
- wake_up_interruptible(&dev->port.open_wait);
- } else if (dlc->state == BT_CLOSED)
+ if (dlc->state == BT_CLOSED)
tty_port_tty_hangup(&dev->port, false);
}
@@ -1096,7 +1115,7 @@ int __init rfcomm_init_ttys(void)
rfcomm_tty_driver->subtype = SERIAL_TYPE_NORMAL;
rfcomm_tty_driver->flags = TTY_DRIVER_REAL_RAW | TTY_DRIVER_DYNAMIC_DEV;
rfcomm_tty_driver->init_termios = tty_std_termios;
- rfcomm_tty_driver->init_termios.c_cflag = B9600 | CS8 | CREAD | HUPCL;
+ rfcomm_tty_driver->init_termios.c_cflag = B9600 | CS8 | CREAD | HUPCL | CLOCAL;
rfcomm_tty_driver->init_termios.c_lflag &= ~ICANON;
tty_set_operations(rfcomm_tty_driver, &rfcomm_ops);
--
1.8.5.2
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox