Linux bluetooth development
 help / color / mirror / Atom feed
* [PATCH 2/3] Bluetooth: Add initial Bluetooth Management interface callbacks
From: johan.hedberg @ 2010-12-05 18:19 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Johan Hedberg
In-Reply-To: <1291573170-3708-1-git-send-email-johan.hedberg@gmail.com>

From: Johan Hedberg <johan.hedberg@nokia.com>

Add initial code for handling Bluetooth Management interface messages.

Signed-off-by: Johan Hedberg <johan.hedberg@nokia.com>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
Acked-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
---
 include/net/bluetooth/hci_core.h |    3 +
 net/bluetooth/Makefile           |    2 +-
 net/bluetooth/hci_sock.c         |   39 +++++++++++++--
 net/bluetooth/mgmt.c             |   99 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 136 insertions(+), 7 deletions(-)
 create mode 100644 net/bluetooth/mgmt.c

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 3e34359..1992fac 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -660,6 +660,9 @@ void hci_si_event(struct hci_dev *hdev, int type, int dlen, void *data);
 /* ----- HCI Sockets ----- */
 void hci_send_to_sock(struct hci_dev *hdev, struct sk_buff *skb);
 
+/* Management interface */
+int mgmt_control(struct sock *sk, struct msghdr *msg, size_t len);
+
 /* HCI info for socket */
 #define hci_pi(sk) ((struct hci_pinfo *) sk)
 
diff --git a/net/bluetooth/Makefile b/net/bluetooth/Makefile
index d1e433f..8b411d9 100644
--- a/net/bluetooth/Makefile
+++ b/net/bluetooth/Makefile
@@ -10,4 +10,4 @@ obj-$(CONFIG_BT_BNEP)	+= bnep/
 obj-$(CONFIG_BT_CMTP)	+= cmtp/
 obj-$(CONFIG_BT_HIDP)	+= hidp/
 
-bluetooth-objs := af_bluetooth.o hci_core.o hci_conn.o hci_event.o hci_sock.o hci_sysfs.o lib.o
+bluetooth-objs := af_bluetooth.o hci_core.o hci_conn.o hci_event.o mgmt.o hci_sock.o hci_sysfs.o lib.o
diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
index b3753ba..207be7a 100644
--- a/net/bluetooth/hci_sock.c
+++ b/net/bluetooth/hci_sock.c
@@ -49,6 +49,8 @@
 #include <net/bluetooth/bluetooth.h>
 #include <net/bluetooth/hci_core.h>
 
+static int enable_mgmt;
+
 /* ----- HCI socket interface ----- */
 
 static inline int hci_test_bit(int nr, void *addr)
@@ -353,25 +355,35 @@ static int hci_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long a
 
 static int hci_sock_bind(struct socket *sock, struct sockaddr *addr, int addr_len)
 {
-	struct sockaddr_hci *haddr = (struct sockaddr_hci *) addr;
+	struct sockaddr_hci haddr;
 	struct sock *sk = sock->sk;
 	struct hci_dev *hdev = NULL;
-	int err = 0;
+	int len, err = 0;
 
 	BT_DBG("sock %p sk %p", sock, sk);
 
-	if (!haddr || haddr->hci_family != AF_BLUETOOTH)
+	if (!addr)
+		return -EINVAL;
+
+	memset(&haddr, 0, sizeof(haddr));
+	len = min_t(unsigned int, sizeof(haddr), addr_len);
+	memcpy(&haddr, addr, len);
+
+	if (haddr.hci_family != AF_BLUETOOTH)
+		return -EINVAL;
+
+	if (haddr.hci_channel != HCI_CHANNEL_RAW && !enable_mgmt)
 		return -EINVAL;
 
 	lock_sock(sk);
 
-	if (hci_pi(sk)->hdev) {
+	if (sk->sk_state == BT_BOUND || hci_pi(sk)->hdev) {
 		err = -EALREADY;
 		goto done;
 	}
 
-	if (haddr->hci_dev != HCI_DEV_NONE) {
-		hdev = hci_dev_get(haddr->hci_dev);
+	if (haddr.hci_dev != HCI_DEV_NONE) {
+		hdev = hci_dev_get(haddr.hci_dev);
 		if (!hdev) {
 			err = -ENODEV;
 			goto done;
@@ -380,6 +392,7 @@ static int hci_sock_bind(struct socket *sock, struct sockaddr *addr, int addr_le
 		atomic_inc(&hdev->promisc);
 	}
 
+	hci_pi(sk)->channel = haddr.hci_channel;
 	hci_pi(sk)->hdev = hdev;
 	sk->sk_state = BT_BOUND;
 
@@ -502,6 +515,17 @@ static int hci_sock_sendmsg(struct kiocb *iocb, struct socket *sock,
 
 	lock_sock(sk);
 
+	switch (hci_pi(sk)->channel) {
+	case HCI_CHANNEL_RAW:
+		break;
+	case HCI_CHANNEL_CONTROL:
+		err = mgmt_control(sk, msg, len);
+		goto done;
+	default:
+		err = -EINVAL;
+		goto done;
+	}
+
 	hdev = hci_pi(sk)->hdev;
 	if (!hdev) {
 		err = -EBADFD;
@@ -831,3 +855,6 @@ void __exit hci_sock_cleanup(void)
 
 	proto_unregister(&hci_sk_proto);
 }
+
+module_param(enable_mgmt, bool, 0644);
+MODULE_PARM_DESC(enable_mgmt, "Enable Management interface");
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
new file mode 100644
index 0000000..78255f1
--- /dev/null
+++ b/net/bluetooth/mgmt.c
@@ -0,0 +1,99 @@
+/*
+   BlueZ - Bluetooth protocol stack for Linux
+   Copyright (C) 2010  Nokia Corporation
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License version 2 as
+   published by the Free Software Foundation;
+
+   THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+   OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+   FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT OF THIRD PARTY RIGHTS.
+   IN NO EVENT SHALL THE COPYRIGHT HOLDER(S) AND AUTHOR(S) BE LIABLE FOR ANY
+   CLAIM, OR ANY SPECIAL INDIRECT OR CONSEQUENTIAL DAMAGES, OR ANY DAMAGES
+   WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+   ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+   OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+
+   ALL LIABILITY, INCLUDING LIABILITY FOR INFRINGEMENT OF ANY PATENTS,
+   COPYRIGHTS, TRADEMARKS OR OTHER RIGHTS, RELATING TO USE OF THIS
+   SOFTWARE IS DISCLAIMED.
+*/
+
+/* Bluetooth HCI Management interface */
+
+#include <asm/uaccess.h>
+#include <asm/unaligned.h>
+
+#include <net/bluetooth/bluetooth.h>
+#include <net/bluetooth/hci_core.h>
+#include <net/bluetooth/mgmt.h>
+
+static void cmd_status(struct sock *sk, u16 cmd, u8 status)
+{
+	struct sk_buff *skb;
+	struct mgmt_hdr *hdr;
+	struct mgmt_ev_cmd_status *ev;
+
+	BT_DBG("sock %p", sk);
+
+	skb = alloc_skb(sizeof(*hdr) + sizeof(*ev), GFP_ATOMIC);
+	if (!skb)
+		return;
+
+	hdr = (void *) skb_put(skb, sizeof(struct mgmt_hdr));
+
+	hdr->opcode = cpu_to_le16(MGMT_EV_CMD_STATUS);
+	hdr->len = cpu_to_le16(3);
+
+	ev = (void *) skb_put(skb, sizeof(*ev));
+	ev->status = status;
+	put_unaligned_le16(cmd, &ev->opcode);
+
+	if (sock_queue_rcv_skb(sk, skb) < 0)
+		kfree_skb(skb);
+}
+
+int mgmt_control(struct sock *sk, struct msghdr *msg, size_t msglen)
+{
+	unsigned char *buf;
+	struct mgmt_hdr *hdr;
+	u16 opcode, len;
+	int err;
+
+	BT_DBG("got %zu bytes", msglen);
+
+	if (msglen < sizeof(*hdr))
+		return -EINVAL;
+
+	buf = kmalloc(msglen, GFP_ATOMIC);
+	if (!buf)
+		return -ENOMEM;
+
+	if (memcpy_fromiovec(buf, msg->msg_iov, msglen)) {
+		err = -EFAULT;
+		goto done;
+	}
+
+	hdr = (struct mgmt_hdr *) buf;
+	opcode = get_unaligned_le16(&hdr->opcode);
+	len = get_unaligned_le16(&hdr->len);
+
+	if (len != msglen - sizeof(struct mgmt_hdr)) {
+		err = -EINVAL;
+		goto done;
+	}
+
+	switch (opcode) {
+	default:
+		BT_DBG("Unknown op %u", opcode);
+		cmd_status(sk, opcode, 0x01);
+		break;
+	}
+
+	err = msglen;
+
+done:
+	kfree(buf);
+	return err;
+}
-- 
1.7.2.3


^ permalink raw reply related

* [PATCH 3/3] Bluetooth: Make hci_send_to_sock usable for management control sockets
From: johan.hedberg @ 2010-12-05 18:19 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Johan Hedberg
In-Reply-To: <1291573170-3708-1-git-send-email-johan.hedberg@gmail.com>

From: Johan Hedberg <johan.hedberg@nokia.com>

In order to send data to management control sockets the function should:

  - skip checks intended for raw HCI data and stack internal events
  - make sure RAW HCI data or stack internal events don't go to
    management control sockets

In order to accomplish this the patch adds a new member to the bluetooth
skb private data to flag skb's that are destined for management control
sockets.

Signed-off-by: Johan Hedberg <johan.hedberg@nokia.com>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
---
 include/net/bluetooth/bluetooth.h |    1 +
 net/bluetooth/hci_sock.c          |   10 +++++++++-
 2 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index d81ea79..0c5e725 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -144,6 +144,7 @@ struct bt_skb_cb {
 	__u8 tx_seq;
 	__u8 retries;
 	__u8 sar;
+	unsigned short channel;
 };
 #define bt_cb(skb) ((struct bt_skb_cb *)((skb)->cb))
 
diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
index 207be7a..f6c18ab 100644
--- a/net/bluetooth/hci_sock.c
+++ b/net/bluetooth/hci_sock.c
@@ -104,6 +104,12 @@ void hci_send_to_sock(struct hci_dev *hdev, struct sk_buff *skb)
 		if (skb->sk == sk)
 			continue;
 
+		if (bt_cb(skb)->channel != hci_pi(sk)->channel)
+			continue;
+
+		if (bt_cb(skb)->channel == HCI_CHANNEL_CONTROL)
+			goto clone;
+
 		/* Apply filter */
 		flt = &hci_pi(sk)->filter;
 
@@ -127,12 +133,14 @@ void hci_send_to_sock(struct hci_dev *hdev, struct sk_buff *skb)
 				continue;
 		}
 
+clone:
 		nskb = skb_clone(skb, GFP_ATOMIC);
 		if (!nskb)
 			continue;
 
 		/* Put type byte before the data */
-		memcpy(skb_push(nskb, 1), &bt_cb(nskb)->pkt_type, 1);
+		if (bt_cb(skb)->channel == HCI_CHANNEL_RAW)
+			memcpy(skb_push(nskb, 1), &bt_cb(nskb)->pkt_type, 1);
 
 		if (sock_queue_rcv_skb(sk, nskb))
 			kfree_skb(nskb);
-- 
1.7.2.3


^ permalink raw reply related

* Re: [PATCH 5/9] mfd: Add UART support for the ST-Ericsson CG2900.
From: Par-Gunnar Hjalmdahl @ 2010-12-06  9:06 UTC (permalink / raw)
  To: Vitaly Wool
  Cc: Alan Cox, linus.walleij, linux-bluetooth, linux-kernel,
	Arnd Bergmann, Marcel Holtmann
In-Reply-To: <AANLkTim7brB0JecKXDNddt0am=Um457Kfbm6bRG_yzFM@mail.gmail.com>

Hi Vitaly,

2010/12/3 Vitaly Wool <vitalywool@gmail.com>:
> Hi Par,
>
> On Fri, Dec 3, 2010 at 10:16 AM, Par-Gunnar Hjalmdahl
> <pghatwork@gmail.com> wrote:
>> 2010/10/29 Alan Cox <alan@lxorguk.ukuu.org.uk>:
>
>> By the way, I will soon release a new patch set for CG2900 (hopefully
>> next week), which contains major rework. It's hard to explain
>> everything here and now but changes include:
>> =A0- Reuse of existing HCI line discipline under /drivers/bluetooth/.
>> Line discipline has been modified so it is selectable from protocol if
>> line discipline should register to Bluetooth stack or not.
>
> So is it true that if there's the other chip with similar
> functionality I have to implement support for, I'll have to pretty
> much duplicate your line discipline driver?
>

What will be in the protocol file, i.e. the same level as hci_h4.c,
hci_bcsp.c, etc, will be to ~80% vendor specific. The only H4 channels
that are in a standardized specification are the Bluetooth channels
1-4. Other channels such as FM and GPS are vendor specific. I've seen
that for example TI has the same channels for FM and GPS, but these
channels, 8 and 9, are in no way standardized. The CG2900 also carries
a number of other channels, such as debug and device channels, which
I'm pretty sure is individual for this chip.
This identification of the channels is also only one function, even
though it is an important function. There are also a lot of other code
regarding baud rate settings, low power handling, etc that I'm
positive will not apply to chips of other vendors. It is of course
possible that other chips from ST-Ericsson, current and future, will
be able to reuse this file, but as I said I doubt that other vendors
might be able to use it.

> Isn't it better to have a new line discipline that the standard
> drivers (H4, LL, ...) will be applicable to?
>
> ~Vitaly
>

I'm not certain what you mean. We are modifying the line discipline a
bit, but the only thing we have needed to do with existing protocol
drivers has been to add a boolean parameter for the protocol settings
(so they will register to the Bluetooth stack). I don't see why we
should create a new line discipline driver and then move the existing
protocol drivers to this.

/P-G

^ permalink raw reply

* Re: [PATCHv4 0/7] Support for out of band association model
From: Szymon Janc @ 2010-12-06  9:46 UTC (permalink / raw)
  To: Mike Tsai; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <35B17FE5076C7040809188FBE7913F98406D68D125@SC1EXMB-MBCL.global.atheros.com>

Hi,

> [Mtsai correction]
> The local OOB data present is set when host send HCI command "Read Local OOB
> Data". And this "OOB data present" flag is sent over the air to peer device
> during pairing.

I think you confused local and remote OOB data.
This flag is about "OOB authentication data from remote device present".
Please see Vol.2 Part E. 7.1.29 "IO Capability Request Reply Command".

-- 
Szymon Janc
on behalf of ST-Ericsson


^ permalink raw reply

* Re: [PATCH 5/9] mfd: Add UART support for the ST-Ericsson CG2900.
From: Vitaly Wool @ 2010-12-06  9:46 UTC (permalink / raw)
  To: Par-Gunnar Hjalmdahl
  Cc: Alan Cox, linus.walleij, linux-bluetooth, linux-kernel,
	Arnd Bergmann, Marcel Holtmann
In-Reply-To: <AANLkTik4T=PsMzB+umSQPLDPmeD-6sebgCtrQRePH_2h@mail.gmail.com>

Hi Par,

On Mon, Dec 6, 2010 at 10:06 AM, Par-Gunnar Hjalmdahl
<pghatwork@gmail.com> wrote:

>> So is it true that if there's the other chip with similar
>> functionality I have to implement support for, I'll have to pretty
>> much duplicate your line discipline driver?
>>
>
> What will be in the protocol file, i.e. the same level as hci_h4.c,
> hci_bcsp.c, etc, will be to ~80% vendor specific. The only H4 channels
> that are in a standardized specification are the Bluetooth channels
> 1-4. Other channels such as FM and GPS are vendor specific. I've seen
> that for example TI has the same channels for FM and GPS, but these
> channels, 8 and 9, are in no way standardized. The CG2900 also carries
> a number of other channels, such as debug and device channels, which
> I'm pretty sure is individual for this chip.
> This identification of the channels is also only one function, even
> though it is an important function. There are also a lot of other code
> regarding baud rate settings, low power handling, etc that I'm
> positive will not apply to chips of other vendors. It is of course
> possible that other chips from ST-Ericsson, current and future, will
> be able to reuse this file, but as I said I doubt that other vendors
> might be able to use it.

Wait. What level of abstraction is this file at if even some future
STE chips are not guaranteed to work with it? Is it at a level of
hardcoding the GPIO numbers?

>> Isn't it better to have a new line discipline that the standard
>> drivers (H4, LL, ...) will be applicable to?

> I'm not certain what you mean. We are modifying the line discipline a
> bit, but the only thing we have needed to do with existing protocol
> drivers has been to add a boolean parameter for the protocol settings
> (so they will register to the Bluetooth stack). I don't see why we
> should create a new line discipline driver and then move the existing
> protocol drivers to this.

Okay, let me try to be more specific. There's for instance
drivers/bluetooth/hci_ll.c which is the line discipline driver that is
meant to work with any BT chip supporting LL protocol. Your solution
seems to imply that one will have to create a variation of this
implementation for each BT chip supporting LL that will use your
shared transport implementation. Is it really the case?

Thanks,
   Vitaly

^ permalink raw reply

* Re: [PATCH 5/9] mfd: Add UART support for the ST-Ericsson CG2900.
From: Par-Gunnar Hjalmdahl @ 2010-12-06 12:01 UTC (permalink / raw)
  To: Vitaly Wool
  Cc: Alan Cox, linus.walleij, linux-bluetooth, linux-kernel,
	Arnd Bergmann, Marcel Holtmann
In-Reply-To: <AANLkTimjACtU_Mx5mDHkm9OpVJ4ueNWgZPsX3YjL0cTi@mail.gmail.com>

Hi Vitaly,

2010/12/6 Vitaly Wool <vitalywool@gmail.com>:
> Hi Par,
>
> On Mon, Dec 6, 2010 at 10:06 AM, Par-Gunnar Hjalmdahl
> <pghatwork@gmail.com> wrote:
>
>>> So is it true that if there's the other chip with similar
>>> functionality I have to implement support for, I'll have to pretty
>>> much duplicate your line discipline driver?
>>>
>>
>> What will be in the protocol file, i.e. the same level as hci_h4.c,
>> hci_bcsp.c, etc, will be to ~80% vendor specific. The only H4 channels
>> that are in a standardized specification are the Bluetooth channels
>> 1-4. Other channels such as FM and GPS are vendor specific. I've seen
>> that for example TI has the same channels for FM and GPS, but these
>> channels, 8 and 9, are in no way standardized. The CG2900 also carries
>> a number of other channels, such as debug and device channels, which
>> I'm pretty sure is individual for this chip.
>> This identification of the channels is also only one function, even
>> though it is an important function. There are also a lot of other code
>> regarding baud rate settings, low power handling, etc that I'm
>> positive will not apply to chips of other vendors. It is of course
>> possible that other chips from ST-Ericsson, current and future, will
>> be able to reuse this file, but as I said I doubt that other vendors
>> might be able to use it.
>
> Wait. What level of abstraction is this file at if even some future
> STE chips are not guaranteed to work with it? Is it at a level of
> hardcoding the GPIO numbers?
>

There are no GPIOs or similar in the driver code; those are defined
and used in the board specific files under the /arch/ files. What
could be a problem with a future chip would be if for example the HCI
command to change baud rate would change. We have no indication that
this will change, but I cannot give a lifetime guarantee that nothing
will ever change with how the chip startup is handled.

>>> Isn't it better to have a new line discipline that the standard
>>> drivers (H4, LL, ...) will be applicable to?
>
>> I'm not certain what you mean. We are modifying the line discipline a
>> bit, but the only thing we have needed to do with existing protocol
>> drivers has been to add a boolean parameter for the protocol settings
>> (so they will register to the Bluetooth stack). I don't see why we
>> should create a new line discipline driver and then move the existing
>> protocol drivers to this.
>
> Okay, let me try to be more specific. There's for instance
> drivers/bluetooth/hci_ll.c which is the line discipline driver that is
> meant to work with any BT chip supporting LL protocol. Your solution
> seems to imply that one will have to create a variation of this
> implementation for each BT chip supporting LL that will use your
> shared transport implementation. Is it really the case?
>
> Thanks,
> =A0 Vitaly
>

Well, from what I can see LL is an extension of the H4, basically it
adds sleep mode handling in a vendor specific way to the normal H4
protocol. So it is also based on the hci_h4 just as our file is. But
our file has basically nothing in common with what has been for the LL
file. We don't support any of the LL sleep commands for example.
So if I would make a driver for a combo chip supporting LL, I would
either modify the existing hci_ll.c or I would make a new file based
on hci_ll.c. There is not much you could really reuse from our new
file. Basically it would be the handling of any common channels, so if
you would for example have the same specification of FM and GPS you
could maybe save around 20 lines of common code, but you would
probably have to add a lot of more code just to keep the solution
generic.
One major difference is also that hci_ll never changes baud rate or
other settings. I assume that is done from hciattach during startup
instead. But we cannot run with that since we have to shut down the
chip when no user is connected in order to save power. This means that
we have to add vendor specific commands in order to for example set
baud rate. And then you run into these vendor specific problems. If
there would be a standardized specification on how to set baud rate
and how to put chip in sleep I assume things could be solved
differently, but that is not the case.

As a quick answer to your question: that would depend on the
difference between the different controllers, I guess. But CG2900
doesn't support the LL protocol so it is not an issue for that.

/P-G

^ permalink raw reply

* Re: [PATCH 5/9] mfd: Add UART support for the ST-Ericsson CG2900.
From: Vitaly Wool @ 2010-12-06 12:25 UTC (permalink / raw)
  To: Par-Gunnar Hjalmdahl
  Cc: Alan Cox, linus.walleij, linux-bluetooth, linux-kernel,
	Arnd Bergmann, Marcel Holtmann
In-Reply-To: <AANLkTikMAWikj9HQ76YVeHr7pMoFK6ME2=HHpuQ1kUxq@mail.gmail.com>

Par,

> Well, from what I can see LL is an extension of the H4, basically it
> adds sleep mode handling in a vendor specific way to the normal H4
> protocol. So it is also based on the hci_h4 just as our file is. But
> our file has basically nothing in common with what has been for the LL
> file. We don't support any of the LL sleep commands for example.
> So if I would make a driver for a combo chip supporting LL, I would
> either modify the existing hci_ll.c or I would make a new file based
> on hci_ll.c. There is not much you could really reuse from our new
> file. Basically it would be the handling of any common channels, so if
> you would for example have the same specification of FM and GPS you
> could maybe save around 20 lines of common code, but you would
> probably have to add a lot of more code just to keep the solution
> generic.

Right, but this gives me the hard time seeing how your implementation
is applicable to other multi-functional chips with similar
functionality.

> One major difference is also that hci_ll never changes baud rate or
> other settings. I assume that is done from hciattach during startup
> instead. But we cannot run with that since we have to shut down the
> chip when no user is connected in order to save power. This means that
> we have to add vendor specific commands in order to for example set
> baud rate. And then you run into these vendor specific problems. If
> there would be a standardized specification on how to set baud rate
> and how to put chip in sleep I assume things could be solved
> differently, but that is not the case.

Again, there are at least TI and Broadcom chips that support HCI_LL
and if they were to use your implementation of the core, they would
have had to add 2 more implementations of the corresponding line
discipline driver.

> As a quick answer to your question: that would depend on the
> difference between the different controllers, I guess. But CG2900
> doesn't support the LL protocol so it is not an issue for that.

Right, but if you are only aiming cg2000, why would you create a
framework for that?  I initially thought your solution was generic
enough to handle other "many-in-one" Bluetooth chips but I'm
completely unsure about that now.

Thanks,
   Vitaly

^ permalink raw reply

* Re: [PATCH 2/3] Bluetooth: Add initial Bluetooth Management interface callbacks
From: Anderson Lizardo @ 2010-12-06 13:11 UTC (permalink / raw)
  To: johan.hedberg; +Cc: linux-bluetooth, Johan Hedberg
In-Reply-To: <1291573170-3708-3-git-send-email-johan.hedberg@gmail.com>

Hi Johan,

On Sun, Dec 5, 2010 at 2:19 PM,  <johan.hedberg@gmail.com> wrote:
> +static void cmd_status(struct sock *sk, u16 cmd, u8 status)
> +{

I see some inconsistence on how you calculate struct sizes on this
function. See below...

> +       struct sk_buff *skb;
> +       struct mgmt_hdr *hdr;
> +       struct mgmt_ev_cmd_status *ev;
> +
> +       BT_DBG("sock %p", sk);
> +
> +       skb = alloc_skb(sizeof(*hdr) + sizeof(*ev), GFP_ATOMIC);

Here you use sizeof(<var>)

> +       if (!skb)
> +               return;
> +
> +       hdr = (void *) skb_put(skb, sizeof(struct mgmt_hdr));

But here you use sizeof(<struct>). Could be sizeof(*hdr) ? Note there
is also the (unused) MGMT_HDR_SIZE.

> +
> +       hdr->opcode = cpu_to_le16(MGMT_EV_CMD_STATUS);
> +       hdr->len = cpu_to_le16(3);

and here a hard-coded size. Could be sizeof(struct mgmt_ev_cmd_status)
? or maybe create a #define for it?

> +
> +       ev = (void *) skb_put(skb, sizeof(*ev));
> +       ev->status = status;
> +       put_unaligned_le16(cmd, &ev->opcode);
> +
> +       if (sock_queue_rcv_skb(sk, skb) < 0)
> +               kfree_skb(skb);
> +}
> +
> +int mgmt_control(struct sock *sk, struct msghdr *msg, size_t msglen)
> +{
> +       unsigned char *buf;
> +       struct mgmt_hdr *hdr;
> +       u16 opcode, len;
> +       int err;
> +
> +       BT_DBG("got %zu bytes", msglen);
> +
> +       if (msglen < sizeof(*hdr))
> +               return -EINVAL;
> +
> +       buf = kmalloc(msglen, GFP_ATOMIC);
> +       if (!buf)
> +               return -ENOMEM;
> +
> +       if (memcpy_fromiovec(buf, msg->msg_iov, msglen)) {
> +               err = -EFAULT;
> +               goto done;
> +       }
> +
> +       hdr = (struct mgmt_hdr *) buf;
> +       opcode = get_unaligned_le16(&hdr->opcode);
> +       len = get_unaligned_le16(&hdr->len);
> +
> +       if (len != msglen - sizeof(struct mgmt_hdr)) {

You could use sizeof(*hdr) here.

> +               err = -EINVAL;
> +               goto done;
> +       }
> +
> +       switch (opcode) {
> +       default:
> +               BT_DBG("Unknown op %u", opcode);
> +               cmd_status(sk, opcode, 0x01);
> +               break;
> +       }
> +
> +       err = msglen;

Would there be a chance of integer overflow here? The function returns
(signed) int, but msglen is (unsigned) size_t.

> +
> +done:
> +       kfree(buf);
> +       return err;
> +}

Regards,
-- 
Anderson Lizardo
OpenBossa Labs - INdT
Manaus - Brazil

^ permalink raw reply

* [PATCH] Bluetooth: Fix initial RFCOMM DLC security level
From: johan.hedberg @ 2010-12-06 13:56 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Johan Hedberg

From: Johan Hedberg <johan.hedberg@nokia.com>

Due to commit 63ce0900 connections initiated through TTYs created with
"rfcomm bind ..." would have security level BT_SECURITY_SDP instead of
BT_SECURITY_LOW. This would cause instant connection failure between any
two SSP capable devices due to the L2CAP connect request to RFCOMM being
sent before authentication has been performed. This patch fixes the
regression by always initializing the DLC security level to
BT_SECURITY_LOW.

Signed-off-by: Johan Hedberg <johan.hedberg@nokia.com>
---
 net/bluetooth/rfcomm/core.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index fa642aa..432a9a6 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -311,6 +311,7 @@ static void rfcomm_dlc_clear_state(struct rfcomm_dlc *d)
 	d->state      = BT_OPEN;
 	d->flags      = 0;
 	d->mscex      = 0;
+	d->sec_level  = BT_SECURITY_LOW;
 	d->mtu        = RFCOMM_DEFAULT_MTU;
 	d->v24_sig    = RFCOMM_V24_RTC | RFCOMM_V24_RTR | RFCOMM_V24_DV;
 
-- 
1.7.1


^ permalink raw reply related

* Re: [PATCH 5/9] mfd: Add UART support for the ST-Ericsson CG2900.
From: Arnd Bergmann @ 2010-12-06 14:06 UTC (permalink / raw)
  To: Vitaly Wool
  Cc: Par-Gunnar Hjalmdahl, Alan Cox, linus.walleij, linux-bluetooth,
	linux-kernel, Marcel Holtmann
In-Reply-To: <AANLkTimjACtU_Mx5mDHkm9OpVJ4ueNWgZPsX3YjL0cTi@mail.gmail.com>

On Monday 06 December 2010, Vitaly Wool wrote:

> On Mon, Dec 6, 2010 at 10:06 AM, Par-Gunnar Hjalmdahl
> <pghatwork@gmail.com> wrote:

> >> Isn't it better to have a new line discipline that the standard
> >> drivers (H4, LL, ...) will be applicable to?
> 
> > I'm not certain what you mean. We are modifying the line discipline a
> > bit, but the only thing we have needed to do with existing protocol
> > drivers has been to add a boolean parameter for the protocol settings
> > (so they will register to the Bluetooth stack). I don't see why we
> > should create a new line discipline driver and then move the existing
> > protocol drivers to this.
> 
> Okay, let me try to be more specific. There's for instance
> drivers/bluetooth/hci_ll.c which is the line discipline driver that is
> meant to work with any BT chip supporting LL protocol. Your solution
> seems to imply that one will have to create a variation of this
> implementation for each BT chip supporting LL that will use your
> shared transport implementation. Is it really the case?

hci_ll is not the line discipline but the a TI specific uart protocol.
The line discipline driver (hci_ldisc.c) is shared across all protocols
(h4, ll, ...) and gets extended slightly so it can deal with cg2900
in addition to the existing HCIs. The rest of the cg2900 support is
about adding more hci_uart_protos.

	Arnd

^ permalink raw reply

* Re: [PATCH 2/3] Bluetooth: Add initial Bluetooth Management interface callbacks
From: Johan Hedberg @ 2010-12-06 14:21 UTC (permalink / raw)
  To: Anderson Lizardo; +Cc: linux-bluetooth
In-Reply-To: <AANLkTimYKSvo8WpNa-ft61MoBaAgKZxt5j0qVuRBfCb3@mail.gmail.com>

Hi Anderson,

On Mon, Dec 06, 2010, Anderson Lizardo wrote:
> On Sun, Dec 5, 2010 at 2:19 PM,  <johan.hedberg@gmail.com> wrote:
> > +static void cmd_status(struct sock *sk, u16 cmd, u8 status)
> > +{
> 
> I see some inconsistence on how you calculate struct sizes on this
> function. See below...
> 
> > +       struct sk_buff *skb;
> > +       struct mgmt_hdr *hdr;
> > +       struct mgmt_ev_cmd_status *ev;
> > +
> > +       BT_DBG("sock %p", sk);
> > +
> > +       skb = alloc_skb(sizeof(*hdr) + sizeof(*ev), GFP_ATOMIC);
> 
> Here you use sizeof(<var>)

Yep, in Chapter 14 of Documentation/CodingStyle this seems to be the
preferred form.

> > +       if (!skb)
> > +               return;
> > +
> > +       hdr = (void *) skb_put(skb, sizeof(struct mgmt_hdr));
> 
> But here you use sizeof(<struct>). Could be sizeof(*hdr)?

Yes, could be.

> > +
> > +       hdr->opcode = cpu_to_le16(MGMT_EV_CMD_STATUS);
> > +       hdr->len = cpu_to_le16(3);
> 
> and here a hard-coded size. Could be sizeof(struct mgmt_ev_cmd_status)?

Yes, could be.

> > +       if (len != msglen - sizeof(struct mgmt_hdr)) {
> 
> You could use sizeof(*hdr) here.

Indeed.

I suppose these style fixes should be as a separate patch since the
original one already got acks from the relevant people? (if not, someone
please enlighten me how the kernel patch process deals with comments
received after acks :)

> > +               err = -EINVAL;
> > +               goto done;
> > +       }
> > +
> > +       switch (opcode) {
> > +       default:
> > +               BT_DBG("Unknown op %u", opcode);
> > +               cmd_status(sk, opcode, 0x01);
> > +               break;
> > +       }
> > +
> > +       err = msglen;
> 
> Would there be a chance of integer overflow here? The function returns
> (signed) int, but msglen is (unsigned) size_t.

The core of the issue is in the way that the sendmsg callback for
sockets is defined. At least L2CAP and RFCOMM sockets do similar
assignments in their sendmsg callbacks so I've assumed it to be ok.

Johan

^ permalink raw reply

* Re: [PATCH] Bluetooth: Fix initial RFCOMM DLC security level
From: Luiz Augusto von Dentz @ 2010-12-06 14:34 UTC (permalink / raw)
  To: johan.hedberg; +Cc: linux-bluetooth, Johan Hedberg
In-Reply-To: <1291643777-18851-1-git-send-email-johan.hedberg@gmail.com>

Hi,

On Mon, Dec 6, 2010 at 3:56 PM,  <johan.hedberg@gmail.com> wrote:
> From: Johan Hedberg <johan.hedberg@nokia.com>
>
> Due to commit 63ce0900 connections initiated through TTYs created with
> "rfcomm bind ..." would have security level BT_SECURITY_SDP instead of
> BT_SECURITY_LOW. This would cause instant connection failure between any
> two SSP capable devices due to the L2CAP connect request to RFCOMM being
> sent before authentication has been performed. This patch fixes the
> regression by always initializing the DLC security level to
> BT_SECURITY_LOW.
>
> Signed-off-by: Johan Hedberg <johan.hedberg@nokia.com>
> ---
>  net/bluetooth/rfcomm/core.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
> index fa642aa..432a9a6 100644
> --- a/net/bluetooth/rfcomm/core.c
> +++ b/net/bluetooth/rfcomm/core.c
> @@ -311,6 +311,7 @@ static void rfcomm_dlc_clear_state(struct rfcomm_dlc *d)
>        d->state      = BT_OPEN;
>        d->flags      = 0;
>        d->mscex      = 0;
> +       d->sec_level  = BT_SECURITY_LOW;
>        d->mtu        = RFCOMM_DEFAULT_MTU;
>        d->v24_sig    = RFCOMM_V24_RTC | RFCOMM_V24_RTR | RFCOMM_V24_DV;
>

Nice catch, I completely forgot to check if tty initialize sec_level
with anything.

Acked-by: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>

-- 
Luiz Augusto von Dentz
Computer Engineer

^ permalink raw reply

* Re: [PATCH 5/9] mfd: Add UART support for the ST-Ericsson CG2900.
From: Arnd Bergmann @ 2010-12-06 14:49 UTC (permalink / raw)
  To: Vitaly Wool
  Cc: Par-Gunnar Hjalmdahl, Alan Cox, linus.walleij, linux-bluetooth,
	linux-kernel, Marcel Holtmann
In-Reply-To: <AANLkTi=V+89hmBP8iqz13xMhYVfnBkbiVnk=FY=OBE8T@mail.gmail.com>

On Monday 06 December 2010, Vitaly Wool wrote:
> > As a quick answer to your question: that would depend on the
> > difference between the different controllers, I guess. But CG2900
> > doesn't support the LL protocol so it is not an issue for that.
> 
> Right, but if you are only aiming cg2000, why would you create a
> framework for that?  I initially thought your solution was generic
> enough to handle other "many-in-one" Bluetooth chips but I'm
> completely unsure about that now.

As far as I understand, the point is that it's no longer a 'solution'
at the core, i.e. there is no replacement for hci_ldisc or any
of these, just modules for the additional h4 protocols that don't
have a linux implementation yet.

The patch set that was originally posted here had a new framework,
but after the comments from Alan and me, Par-Gunnar agreed to use
the existing framework instead and extend it in a useful way.
Please read all of the discussion we already had. You made a good
point here, but I fear you had both Par-Gunnar and me confused
because it was a point that already got resolved.

	Arnd

^ permalink raw reply

* Re: LE Kernel (bluetooth-le-2.6) and LE Security Manager
From: Vinicius Costa Gomes @ 2010-12-06 14:50 UTC (permalink / raw)
  To: Brian Gix
  Cc: 'Gustavo F. Padovan', 'Ville Tervo',
	linux-bluetooth
In-Reply-To: <001f01cb934b$d065e4c0$7131ae40$@org>

Hi Brian,

On 16:40 Fri 03 Dec, Brian Gix wrote:
> 
> 
> Hi Vinicius & Gustavo,
> 
> 
> -----Original Message-----
> From: Vinicius Costa Gomes [mailto:vinicius.gomes@openbossa.org] 
> Sent: 03 December, 2010 2:06 PM
> To: Brian Gix
> Cc: linux-bluetooth@vger.kernel.org
> Subject: Re: LE Kernel (bluetooth-le-2.6) and LE Security Manager
> 
> Hi Brian,
> 
> On 11:11 Fri 03 Dec, Brian Gix wrote:
> > 
> > Hi Claudio, Johan & All,
> > 
> > Is this LE capable kernel that Ville is working on, the development stream
> > for the LE Security Manager?  And if so, is it in a partial fleshed out
> > state?
> 
> There is a simple implementation of SMP here[1] on my "devel" branch. I am 
> cleaning it up for sending it for review.
> 
> If you want to help, have any comments or just want to tell us what you are
> working on, please drop by #bluez on freenode, or send an email.
> 
> [Gix] Unfortunately, I can't access git.infradead.org.  Gustavo, would it be
> possible for you to pull Vinicius' devel branch from
> git://git.infradead.org/users/vcgomes/linux-2.6.git to an le-devel branch on
> your bluetooth-testing repository?
> 

Sorry for the delay.

I pushed a copy to gitorious:

http://gitorious.org/bluetooth-next branch devel.


> [Gix] I am trying to integrate Qualcomm's latest LE capable (BR/EDR/LE)
> chipset with bluez.  Additionally, I do have a working non-open-source
> [Gix]  host which has had it's LE Security Manager UPF tested, which I hope
> to be able to use to help test/solidify the bluez version.
> 
> 
> > 
> > >
> git://git.kernel.org/pub/scm/linux/kernel/git/vtervo/bluetooth-le-2.6.git
> > 
> > 
> > Brian Gix
> > bgix@codeaurora.org
> > Employee of Qualcomm Innovation Center, Inc.
> > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
> > 
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-bluetooth"
> in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> Cheers,
> -- 
> Vinicius
> 
> [1] git://git.infradead.org/users/vcgomes/linux-2.6.git 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


Cheers,
-- 
Vinicius

^ permalink raw reply

* Re: [PATCH 5/9] mfd: Add UART support for the ST-Ericsson CG2900.
From: Vitaly Wool @ 2010-12-06 14:54 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Par-Gunnar Hjalmdahl, Alan Cox, linus.walleij, linux-bluetooth,
	linux-kernel, Marcel Holtmann
In-Reply-To: <201012061506.04364.arnd@arndb.de>

Hi Arnd,

On Mon, Dec 6, 2010 at 3:06 PM, Arnd Bergmann <arnd@arndb.de> wrote:

> hci_ll is not the line discipline but the a TI specific uart protocol.
> The line discipline driver (hci_ldisc.c) is shared across all protocols
> (h4, ll, ...) and gets extended slightly so it can deal with cg2900
> in addition to the existing HCIs. The rest of the cg2900 support is
> about adding more hci_uart_protos.

I was referring to hci_ll as to the line discipline driver, not the
line discipline itself.

The thing is, there are different Bluetooth combo chips which use HCI
to encapsulate commands to the sub-chips behind, and there's also the
implementation for TI chip doing that which is in the mainline. So
instead of keeping reinventing the wheel, I think it's fairly
beneficial for everything if we finally agree on something that works
for all the combo chips of the type.

Thanks,
   Vitaly

^ permalink raw reply

* Re: [PATCH 5/9] mfd: Add UART support for the ST-Ericsson CG2900.
From: Vitaly Wool @ 2010-12-06 14:57 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Par-Gunnar Hjalmdahl, Alan Cox, linus.walleij, linux-bluetooth,
	linux-kernel, Marcel Holtmann
In-Reply-To: <201012061549.01534.arnd@arndb.de>

On Mon, Dec 6, 2010 at 3:49 PM, Arnd Bergmann <arnd@arndb.de> wrote:

> As far as I understand, the point is that it's no longer a 'solution'
> at the core, i.e. there is no replacement for hci_ldisc or any
> of these, just modules for the additional h4 protocols that don't
> have a linux implementation yet.
>
> The patch set that was originally posted here had a new framework,
> but after the comments from Alan and me, Par-Gunnar agreed to use
> the existing framework instead and extend it in a useful way.
> Please read all of the discussion we already had. You made a good
> point here, but I fear you had both Par-Gunnar and me confused
> because it was a point that already got resolved.

Yeah I've read through that thread but the only conclusion I'd been
able to make was that you agreed on not introducing yet another line
discipline. I'm not sure if I could make a conclusion that Par gave up
on his attempt to make his patchset generic/extensible/reusable for
other chips of similar type. But if that's really the case, I see no
point in this patchset at all, as opposed to generalizing the TI
shared transport thingie and using that one.

Thanks,
   Vitaly

^ permalink raw reply

* Re: [PATCH 5/9] mfd: Add UART support for the ST-Ericsson CG2900.
From: Arnd Bergmann @ 2010-12-06 15:15 UTC (permalink / raw)
  To: Vitaly Wool
  Cc: Par-Gunnar Hjalmdahl, Alan Cox, linus.walleij, linux-bluetooth,
	linux-kernel, Marcel Holtmann
In-Reply-To: <AANLkTikUeH2BL3svwydt1kGGuzn7C8QxiqRqMZPXHhZ_@mail.gmail.com>

On Monday 06 December 2010, Vitaly Wool wrote:
> On Mon, Dec 6, 2010 at 3:06 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> 
> > hci_ll is not the line discipline but the a TI specific uart protocol.
> > The line discipline driver (hci_ldisc.c) is shared across all protocols
> > (h4, ll, ...) and gets extended slightly so it can deal with cg2900
> > in addition to the existing HCIs. The rest of the cg2900 support is
> > about adding more hci_uart_protos.
> 
> I was referring to hci_ll as to the line discipline driver, not the
> line discipline itself.
> 
> The thing is, there are different Bluetooth combo chips which use HCI
> to encapsulate commands to the sub-chips behind, and there's also the
> implementation for TI chip doing that which is in the mainline. So
> instead of keeping reinventing the wheel, I think it's fairly
> beneficial for everything if we finally agree on something that works
> for all the combo chips of the type.

Yes, that makes sense. I'm probably missing something here, but
it seems to me that hci_ll is only about the power management stuff
on TI (and broadcom, as you say) chips, and the multi-protocol
support is currently handled in hci_ldisc by allowing multiple
protocols like h4 and ll to be registered. It that correct?

One aspect that Par-Gunnar mentioned was that the multi-protocol
stuff for cg2900 and I suspect other similar devices would also
need to work with non-UART-based HCIs, which don't use hci_uart_proto
but would need something similar. Also, hci_uart is currently not
modular, e.g. you cannot build hci_ll as a loadable module
as you'd need for dynamic registration.

	Arnd

^ permalink raw reply

* Re: [PATCH 5/9] mfd: Add UART support for the ST-Ericsson CG2900.
From: Vitaly Wool @ 2010-12-06 15:28 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Par-Gunnar Hjalmdahl, Alan Cox, linus.walleij, linux-bluetooth,
	linux-kernel, Marcel Holtmann
In-Reply-To: <201012061615.28093.arnd@arndb.de>

On Mon, Dec 6, 2010 at 4:15 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> Yes, that makes sense. I'm probably missing something here, but
> it seems to me that hci_ll is only about the power management stuff
> on TI (and broadcom, as you say) chips, and the multi-protocol
> support is currently handled in hci_ldisc by allowing multiple
> protocols like h4 and ll to be registered. It that correct?

Yeah, it's basic for TI and it's supported by Broadcom although they
have their own protocol for power saving.

But I was trying to make a different point here. On a basic level,
there's this cg2000 chip from STE that does BT, FM and GPS. There's
the chip from TI that does BT, FM and GPS, and there's the Broadcom
chip that does BT+FM. They all use HCI to access the other functions
of the combo chip and they do it in a really simiar way, with the
differences mostly in power management techniques. So I think it's
quite sensible to have some kind of framework that is suitable for
such devices.

> One aspect that Par-Gunnar mentioned was that the multi-protocol
> stuff for cg2900 and I suspect other similar devices would also
> need to work with non-UART-based HCIs, which don't use hci_uart_proto
> but would need something similar. Also, hci_uart is currently not
> modular, e.g. you cannot build hci_ll as a loadable module
> as you'd need for dynamic registration.

But generally speaking, isn't a line discipline/driver attached to a
tty? We can use dumb tty for e. g. SPI and still be able to use
hci_ll, right?

Thanks,
   Vitaly

^ permalink raw reply

* Re: [PATCH 5/9] mfd: Add UART support for the ST-Ericsson CG2900.
From: Arnd Bergmann @ 2010-12-06 16:54 UTC (permalink / raw)
  To: Vitaly Wool
  Cc: Par-Gunnar Hjalmdahl, Alan Cox, linus.walleij, linux-bluetooth,
	linux-kernel, Marcel Holtmann
In-Reply-To: <AANLkTi=upn_ieeW_OXzKaHsrEumH2F_fMn07P6-97qb1@mail.gmail.com>

On Monday 06 December 2010, Vitaly Wool wrote:
> On Mon, Dec 6, 2010 at 4:15 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > Yes, that makes sense. I'm probably missing something here, but
> > it seems to me that hci_ll is only about the power management stuff
> > on TI (and broadcom, as you say) chips, and the multi-protocol
> > support is currently handled in hci_ldisc by allowing multiple
> > protocols like h4 and ll to be registered. It that correct?
> 
> Yeah, it's basic for TI and it's supported by Broadcom although they
> have their own protocol for power saving.
> 
> But I was trying to make a different point here. On a basic level,
> there's this cg2000 chip from STE that does BT, FM and GPS. There's
> the chip from TI that does BT, FM and GPS, and there's the Broadcom
> chip that does BT+FM. They all use HCI to access the other functions
> of the combo chip and they do it in a really simiar way, with the
> differences mostly in power management techniques. So I think it's
> quite sensible to have some kind of framework that is suitable for
> such devices.

Yes, I agree 100% in principle. I could not find the code that 
Broadcom/TI FM and GPS stuff so far, can you point us to that?

The cg2900 solution for this was to use MFD (plus another layer
in the posted version, but that will go away I assume). Using
MFD is not the only possibility here, but I could not see anything
wrong with it either. Do you think we can move them all over to
use MFD infrastructure?

> > One aspect that Par-Gunnar mentioned was that the multi-protocol
> > stuff for cg2900 and I suspect other similar devices would also
> > need to work with non-UART-based HCIs, which don't use hci_uart_proto
> > but would need something similar. Also, hci_uart is currently not
> > modular, e.g. you cannot build hci_ll as a loadable module
> > as you'd need for dynamic registration.
> 
> But generally speaking, isn't a line discipline/driver attached to a
> tty? We can use dumb tty for e. g. SPI and still be able to use
> hci_ll, right?

I suggested that as well, but the point was made that this would
add an unnecessary indirection for the SPI case, which is not
really much like a serial port. It's certainly possible to do it
like you say, but if we add a way to register the high-level
protocols with an HCI-like multi-function device, we could
also do it in a way that does not rely on tty-ldisc but keeps it
as one of the options.

	Arnd

^ permalink raw reply

* Re: [PATCH] Bluetooth: Fix initial RFCOMM DLC security level
From: Gustavo F. Padovan @ 2010-12-06 17:53 UTC (permalink / raw)
  To: johan.hedberg; +Cc: linux-bluetooth, Johan Hedberg
In-Reply-To: <1291643777-18851-1-git-send-email-johan.hedberg@gmail.com>

Hi Johan,

* johan.hedberg@gmail.com <johan.hedberg@gmail.com> [2010-12-06 15:56:17 +0200]:

> From: Johan Hedberg <johan.hedberg@nokia.com>
> 
> Due to commit 63ce0900 connections initiated through TTYs created with
> "rfcomm bind ..." would have security level BT_SECURITY_SDP instead of
> BT_SECURITY_LOW. This would cause instant connection failure between any
> two SSP capable devices due to the L2CAP connect request to RFCOMM being
> sent before authentication has been performed. This patch fixes the
> regression by always initializing the DLC security level to
> BT_SECURITY_LOW.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@nokia.com>

Patch has been applied to bluetooth-2.6, thanks.

-- 
Gustavo F. Padovan
http://profusion.mobi

^ permalink raw reply

* Re: [PATCH 2/2] bluetooth: Use printf extension %pMbt
From: Gustavo F. Padovan @ 2010-12-06 18:15 UTC (permalink / raw)
  To: Joe Perches
  Cc: Marcel Holtmann, netdev, David S. Miller, linux-bluetooth,
	linux-kernel
In-Reply-To: <b966e9510dec12556a3d1313b026cb2c8281e4d6.1291419007.git.joe@perches.com>

Hi Joe,

* Joe Perches <joe@perches.com> [2010-12-03 18:33:04 -0800]:

> Save some text and bss.
> Remove function batostr so there's no possibility of bad output.
> 
> from the net/bluetooth directory:
> 
> $ size built-in.o.*
>    text	   data	    bss	    dec	    hex	filename
>  293562	  16265	  70088	 379915	  5cc0b	built-in.o.allyesconfig.new
>  294619	  16269	  70480	 381368	  5d1b8	built-in.o.allyesconfig.old
>   30359	    772	     56	  31187	   79d3	built-in.o.btonly.new
>   30555	    776	     92	  31423	   7abf	built-in.o.btonly.old
> 
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
>  net/bluetooth/bnep/core.c   |    3 +--
>  net/bluetooth/cmtp/core.c   |    2 +-
>  net/bluetooth/hci_conn.c    |    6 +++---
>  net/bluetooth/hci_core.c    |    8 ++++----
>  net/bluetooth/hci_event.c   |    6 +++---
>  net/bluetooth/hci_sysfs.c   |   10 +++++-----
>  net/bluetooth/hidp/core.c   |    4 ++--
>  net/bluetooth/l2cap.c       |   19 +++++++++----------
>  net/bluetooth/lib.c         |   14 --------------
>  net/bluetooth/rfcomm/core.c |   16 ++++++++--------
>  net/bluetooth/rfcomm/sock.c |    8 ++++----
>  net/bluetooth/rfcomm/tty.c  |    6 +++---
>  net/bluetooth/sco.c         |   12 ++++++------
>  13 files changed, 49 insertions(+), 65 deletions(-)

This patch doesn't apply to the bluetooth-next-2.6 tree. Can you please rebase
it against the bluetooth-next-2.6 tree? The tree is at:

git://git.kernel.org/pub/scm/linux/kernel/git/padovan/bluetooth-next-2.6.git


-- 
Gustavo F. Padovan
http://profusion.mobi

^ permalink raw reply

* Re: [PATCH 1/5] Add device type to identify LE, BR/EDR or dual mode devices
From: Claudio Takahasi @ 2010-12-06 18:20 UTC (permalink / raw)
  To: Anderson Lizardo; +Cc: linux-bluetooth
In-Reply-To: <AANLkTik=WyfpbULNywPPXGi-KB=6tK3on2p6TYL94E0O@mail.gmail.com>

Hi Lizardo,

>> +{
>> +       /* Inferring the remote type based on the EIR Flags field */
>> +
>> +       if (flags & EIR_SIM_CONTROLLER && flags & EIR_SIM_HOST)
>> +               return DUALMODE_TYPE;
>
> Can the above be simplified to the following?
>
>  if (flags & (EIR_SIM_CONTROLLER | EIR_SIM_HOST))

We are both wrong, read the last sentence for dual mode devices.

>From Bluetooth SPEC(page 1744):

BR/EDR/LE OPERATION MODES AND PROCEDURE
13.1.1.2 Discoverable Mode
...
A device supporting both BR/EDR and LE physical links shall expose the
capabilities of both physical links by performing the following steps:
a) The ‘LE Supported (Controller)’ and ‘LE Supported (Host)’ bits in
the LMP features shall be set as defined in [Vol. 2], Part C Section
3.2.
b) The ‘BR/EDR Not Supported’ bit in the Flags AD type shall be set to
‘0’ as defined in Section 18.1.
c) The ‘Simultaneous LE and BR/EDR to Same Device Capable
(Controller)’ and ‘Simultaneous LE and BR/EDR to Same Device Capable
(Host)’ bits in the Flags AD type shall be set to ‘0’ as defined in
Section 18.1

>
> I know the semantics will change slightly, but I suppose that either
> host or controller supports simultaneous LE and BR/EDR, it can be
> considered dual mode. It would no be necessary to have *both* bits set
> (my impression).
>
>> @@ -3082,6 +3098,7 @@ void adapter_update_device_from_info(struct btd_adapter *adapter,
>>        if (new_dev) {
>>                dev->le = TRUE;
>>                dev->evt_type = info->evt_type;
>> +               dev->type = LE_TYPE;
>>        } else if (dev->rssi == rssi)
>>                return;
>>
>
> Can you remind me why both dev->le and dev->type are necessary in
> struct remote_dev_info ?

"le" will be removed. Thanks!

Regards,
Claudio

^ permalink raw reply

* Re: [PATCH 1/5] Add device type to identify LE, BR/EDR or dual mode devices
From: Claudio Takahasi @ 2010-12-06 18:21 UTC (permalink / raw)
  To: Anderson Lizardo; +Cc: linux-bluetooth
In-Reply-To: <AANLkTik=WyfpbULNywPPXGi-KB=6tK3on2p6TYL94E0O@mail.gmail.com>

Hi Lizardo,

>> +{
>> +       /* Inferring the remote type based on the EIR Flags field */
>> +
>> +       if (flags & EIR_SIM_CONTROLLER && flags & EIR_SIM_HOST)
>> +               return DUALMODE_TYPE;
>
> Can the above be simplified to the following?
>
>  if (flags & (EIR_SIM_CONTROLLER | EIR_SIM_HOST))

We are both wrong, read the last sentence for dual mode devices.

>From Bluetooth SPEC(page 1744):

BR/EDR/LE OPERATION MODES AND PROCEDURE
13.1.1.2 Discoverable Mode
...
A device supporting both BR/EDR and LE physical links shall expose the
capabilities of both physical links by performing the following steps:
a) The ‘LE Supported (Controller)’ and ‘LE Supported (Host)’ bits in
the LMP features shall be set as defined in [Vol. 2], Part C Section
3.2.
b) The ‘BR/EDR Not Supported’ bit in the Flags AD type shall be set to
‘0’ as defined in Section 18.1.
c) The ‘Simultaneous LE and BR/EDR to Same Device Capable
(Controller)’ and ‘Simultaneous LE and BR/EDR to Same Device Capable
(Host)’ bits in the Flags AD type shall be set to ‘0’ as defined in
Section 18.1

>
> I know the semantics will change slightly, but I suppose that either
> host or controller supports simultaneous LE and BR/EDR, it can be
> considered dual mode. It would no be necessary to have *both* bits set
> (my impression).
>
>> @@ -3082,6 +3098,7 @@ void adapter_update_device_from_info(struct btd_adapter *adapter,
>>        if (new_dev) {
>>                dev->le = TRUE;
>>                dev->evt_type = info->evt_type;
>> +               dev->type = LE_TYPE;
>>        } else if (dev->rssi == rssi)
>>                return;
>>
>
> Can you remind me why both dev->le and dev->type are necessary in
> struct remote_dev_info ?

"le" will be removed. Thanks!

Regards,
Claudio

^ permalink raw reply

* Re: [PATCH 2/2] bluetooth: Use printf extension %pMbt
From: Joe Perches @ 2010-12-06 18:50 UTC (permalink / raw)
  To: Gustavo F. Padovan, Michał Mirosław
  Cc: Marcel Holtmann, netdev, David S. Miller, linux-bluetooth,
	linux-kernel
In-Reply-To: <20101206181507.GC883@vigoh>

On Mon, 2010-12-06 at 16:15 -0200, Gustavo F. Padovan wrote:
> This patch doesn't apply to the bluetooth-next-2.6 tree.
> Can you please rebase it against the bluetooth-next-2.6 tree?

No worries, it was done against next-20101202.

Do you care about using %pMR vs %pMbt as Michał suggested in
https://lkml.org/lkml/2010/12/4/21 ?

I think %pMbt more specific, Michał %pMR more generic.
Doesn't matter much to me.  Do tell, I'll resubmit either way.

^ permalink raw reply

* [PATCH 1/9] Create btd_error_invalid_args()
From: Gustavo F. Padovan @ 2010-12-06 19:10 UTC (permalink / raw)
  To: linux-bluetooth

DBus error handling in BlueZ is a mess. This is the first patch to unify
all DBus error handling like in ConnMan and oFono. This unifies all
.InvalidArguments errors.
---
 attrib/client.c          |   20 ++++++-----------
 audio/gateway.c          |    8 +-----
 audio/headset.c          |   18 +++++----------
 audio/media.c            |    9 ++-----
 audio/telephony-dummy.c  |   25 ++++++++------------
 audio/telephony-maemo5.c |   11 ++------
 audio/telephony-maemo6.c |   11 ++------
 audio/transport.c        |   14 +++--------
 health/hdp.c             |   54 ++++++++++++----------------------------------
 network/server.c         |    7 ------
 plugins/service.c        |    8 +------
 serial/port.c            |    8 ------
 serial/proxy.c           |   19 +++++-----------
 src/adapter.c            |   52 +++++++++++++++++++------------------------
 src/device.c             |   22 ++++++------------
 src/error.c              |    7 ++++++
 src/error.h              |    2 +
 src/manager.c            |    7 ------
 18 files changed, 99 insertions(+), 203 deletions(-)

diff --git a/attrib/client.c b/attrib/client.c
index a8a4051..ac78fbd 100644
--- a/attrib/client.c
+++ b/attrib/client.c
@@ -191,12 +191,6 @@ static int watcher_cmp(gconstpointer a, gconstpointer b)
 	return g_strcmp0(watcher->path, match->path);
 }
 
-static inline DBusMessage *invalid_args(DBusMessage *msg)
-{
-	return g_dbus_create_error(msg, ERROR_INTERFACE ".InvalidArguments",
-			"Invalid arguments in method call");
-}
-
 static inline DBusMessage *not_authorized(DBusMessage *msg)
 {
 	return g_dbus_create_error(msg, ERROR_INTERFACE ".NotAuthorized",
@@ -466,7 +460,7 @@ static DBusMessage *register_watcher(DBusConnection *conn,
 
 	if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &path,
 							DBUS_TYPE_INVALID))
-		return invalid_args(msg);
+		return btd_error_invalid_args(msg);
 
 	if (l2cap_connect(prim->gatt, &gerr, TRUE) < 0) {
 		DBusMessage *reply;
@@ -500,7 +494,7 @@ static DBusMessage *unregister_watcher(DBusConnection *conn,
 
 	if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &path,
 							DBUS_TYPE_INVALID))
-		return invalid_args(msg);
+		return btd_error_invalid_args(msg);
 
 	match = g_new0(struct watcher, 1);
 	match->name = g_strdup(sender);
@@ -538,7 +532,7 @@ static DBusMessage *set_value(DBusConnection *conn, DBusMessage *msg,
 
 	if (dbus_message_iter_get_arg_type(iter) != DBUS_TYPE_ARRAY ||
 			dbus_message_iter_get_element_type(iter) != DBUS_TYPE_BYTE)
-		return invalid_args(msg);
+		return btd_error_invalid_args(msg);
 
 	dbus_message_iter_recurse(iter, &sub);
 
@@ -587,23 +581,23 @@ static DBusMessage *set_property(DBusConnection *conn,
 	const char *property;
 
 	if (!dbus_message_iter_init(msg, &iter))
-		return invalid_args(msg);
+		return btd_error_invalid_args(msg);
 
 	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
-		return invalid_args(msg);
+		return btd_error_invalid_args(msg);
 
 	dbus_message_iter_get_basic(&iter, &property);
 	dbus_message_iter_next(&iter);
 
 	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_VARIANT)
-		return invalid_args(msg);
+		return btd_error_invalid_args(msg);
 
 	dbus_message_iter_recurse(&iter, &sub);
 
 	if (g_str_equal("Value", property))
 		return set_value(conn, msg, &sub, chr);
 
-	return invalid_args(msg);
+	return btd_error_invalid_args(msg);
 }
 
 static GDBusMethodTable char_methods[] = {
diff --git a/audio/gateway.c b/audio/gateway.c
index e5f92a7..450772b 100644
--- a/audio/gateway.c
+++ b/audio/gateway.c
@@ -491,9 +491,7 @@ static DBusMessage *register_agent(DBusConnection *conn,
 
 	if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &path,
 						DBUS_TYPE_INVALID))
-		return g_dbus_create_error(msg,
-					ERROR_INTERFACE ".InvalidArguments",
-					"Invalid argument");
+		return btd_error_invalid_args(msg);
 
 	name = dbus_message_get_sender(msg);
 	agent = g_new0(struct hf_agent, 1);
@@ -526,9 +524,7 @@ static DBusMessage *unregister_agent(DBusConnection *conn,
 	if (!dbus_message_get_args(msg, NULL,
 				DBUS_TYPE_OBJECT_PATH, &path,
 				DBUS_TYPE_INVALID))
-		return g_dbus_create_error(msg,
-				ERROR_INTERFACE ".InvalidArguments",
-				"Invalid argument");
+		return btd_error_invalid_args(msg);
 
 	if (strcmp(gw->agent->path, path) != 0)
 		return g_dbus_create_error(msg,
diff --git a/audio/headset.c b/audio/headset.c
index 2fa0a55..0932477 100644
--- a/audio/headset.c
+++ b/audio/headset.c
@@ -176,12 +176,6 @@ struct event {
 
 static GSList *headset_callbacks = NULL;
 
-static inline DBusMessage *invalid_args(DBusMessage *msg)
-{
-	return g_dbus_create_error(msg, ERROR_INTERFACE ".InvalidArguments",
-			"Invalid arguments in method call");
-}
-
 static DBusHandlerResult error_not_supported(DBusConnection *conn,
 							DBusMessage *msg)
 {
@@ -2022,35 +2016,35 @@ static DBusMessage *hs_set_property(DBusConnection *conn,
 	uint16_t gain;
 
 	if (!dbus_message_iter_init(msg, &iter))
-		return invalid_args(msg);
+		return btd_error_invalid_args(msg);
 
 	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
-		return invalid_args(msg);
+		return btd_error_invalid_args(msg);
 
 	dbus_message_iter_get_basic(&iter, &property);
 	dbus_message_iter_next(&iter);
 
 	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_VARIANT)
-		return invalid_args(msg);
+		return btd_error_invalid_args(msg);
 	dbus_message_iter_recurse(&iter, &sub);
 
 	if (g_str_equal("SpeakerGain", property)) {
 		if (dbus_message_iter_get_arg_type(&sub) != DBUS_TYPE_UINT16)
-			return invalid_args(msg);
+			return btd_error_invalid_args(msg);
 
 		dbus_message_iter_get_basic(&sub, &gain);
 		return hs_set_gain(conn, msg, data, gain,
 					HEADSET_GAIN_SPEAKER);
 	} else if (g_str_equal("MicrophoneGain", property)) {
 		if (dbus_message_iter_get_arg_type(&sub) != DBUS_TYPE_UINT16)
-			return invalid_args(msg);
+			return btd_error_invalid_args(msg);
 
 		dbus_message_iter_get_basic(&sub, &gain);
 		return hs_set_gain(conn, msg, data, gain,
 					HEADSET_GAIN_MICROPHONE);
 	}
 
-	return invalid_args(msg);
+	return btd_error_invalid_args(msg);
 }
 static GDBusMethodTable headset_methods[] = {
 	{ "Connect",		"",	"",	hs_connect,
diff --git a/audio/media.c b/audio/media.c
index b6c90f9..bf42bdf 100644
--- a/audio/media.c
+++ b/audio/media.c
@@ -323,18 +323,15 @@ static DBusMessage *register_endpoint(DBusConnection *conn, DBusMessage *msg,
 
 	dbus_message_iter_recurse(&args, &props);
 	if (dbus_message_iter_get_arg_type(&props) != DBUS_TYPE_DICT_ENTRY)
-		return g_dbus_create_error(msg, ERROR_INTERFACE
-					".Failed", "Invalid argument");
+		return btd_error_invalid_args(msg);
 
 	if (parse_properties(&props, &uuid, &delay_reporting, &codec,
 				&capabilities, &size) || uuid == NULL)
-		return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed",
-						"Invalid argument");
+		return btd_error_invalid_args(msg);
 
 	if (media_endpoint_create(adapter, sender, path, uuid, delay_reporting,
 				codec, capabilities, size) == FALSE)
-		return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed",
-						"Invalid argument");
+		return btd_error_invalid_args(msg);
 
 	return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
 }
diff --git a/audio/telephony-dummy.c b/audio/telephony-dummy.c
index 54b1857..c02cea2 100644
--- a/audio/telephony-dummy.c
+++ b/audio/telephony-dummy.c
@@ -35,6 +35,7 @@
 
 #include "log.h"
 #include "telephony.h"
+#include "error.h"
 
 #define TELEPHONY_DUMMY_IFACE "org.bluez.TelephonyTest"
 #define TELEPHONY_DUMMY_PATH "/org/bluez/test"
@@ -69,12 +70,6 @@ static struct indicator dummy_indicators[] =
 	{ NULL }
 };
 
-static inline DBusMessage *invalid_args(DBusMessage *msg)
-{
-	return g_dbus_create_error(msg, "org.bluez.Error.InvalidArguments",
-					"Invalid arguments in method call");
-}
-
 void telephony_device_connected(void *telephony_device)
 {
 	DBG("telephony-dummy: device %p connected", telephony_device);
@@ -236,7 +231,7 @@ static DBusMessage *outgoing_call(DBusConnection *conn, DBusMessage *msg,
 
 	if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &number,
 						DBUS_TYPE_INVALID))
-		return invalid_args(msg);
+		return btd_error_invalid_args(msg);
 
 	DBG("telephony-dummy: outgoing call to %s", number);
 
@@ -261,7 +256,7 @@ static DBusMessage *incoming_call(DBusConnection *conn, DBusMessage *msg,
 
 	if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &number,
 						DBUS_TYPE_INVALID))
-		return invalid_args(msg);
+		return btd_error_invalid_args(msg);
 
 	DBG("telephony-dummy: incoming call to %s", number);
 
@@ -307,10 +302,10 @@ static DBusMessage *signal_strength(DBusConnection *conn, DBusMessage *msg,
 
 	if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_UINT32, &strength,
 						DBUS_TYPE_INVALID))
-		return invalid_args(msg);
+		return btd_error_invalid_args(msg);
 
 	if (strength > 5)
-		return invalid_args(msg);
+		return btd_error_invalid_args(msg);
 
 	telephony_update_indicator(dummy_indicators, "signal", strength);
 
@@ -326,10 +321,10 @@ static DBusMessage *battery_level(DBusConnection *conn, DBusMessage *msg,
 
 	if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_UINT32, &level,
 						DBUS_TYPE_INVALID))
-		return invalid_args(msg);
+		return btd_error_invalid_args(msg);
 
 	if (level > 5)
-		return invalid_args(msg);
+		return btd_error_invalid_args(msg);
 
 	telephony_update_indicator(dummy_indicators, "battchg", level);
 
@@ -346,7 +341,7 @@ static DBusMessage *roaming_status(DBusConnection *conn, DBusMessage *msg,
 
 	if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_BOOLEAN, &roaming,
 						DBUS_TYPE_INVALID))
-		return invalid_args(msg);
+		return btd_error_invalid_args(msg);
 
 	val = roaming ? EV_ROAM_ACTIVE : EV_ROAM_INACTIVE;
 
@@ -365,7 +360,7 @@ static DBusMessage *registration_status(DBusConnection *conn, DBusMessage *msg,
 
 	if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_BOOLEAN, &registration,
 						DBUS_TYPE_INVALID))
-		return invalid_args(msg);
+		return btd_error_invalid_args(msg);
 
 	val = registration ? EV_SERVICE_PRESENT : EV_SERVICE_NONE;
 
@@ -384,7 +379,7 @@ static DBusMessage *set_subscriber_number(DBusConnection *conn,
 
 	if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &number,
 						DBUS_TYPE_INVALID))
-		return invalid_args(msg);
+		return btd_error_invalid_args(msg);
 
 	g_free(subscriber_number);
 	subscriber_number = g_strdup(number);
diff --git a/audio/telephony-maemo5.c b/audio/telephony-maemo5.c
index 4d0134c..6ee57c7 100644
--- a/audio/telephony-maemo5.c
+++ b/audio/telephony-maemo5.c
@@ -38,6 +38,7 @@
 
 #include "log.h"
 #include "telephony.h"
+#include "error.h"
 
 /* SSC D-Bus definitions */
 #define SSC_DBUS_NAME  "com.nokia.phone.SSC"
@@ -1880,12 +1881,6 @@ static void csd_init(void)
 	}
 }
 
-static inline DBusMessage *invalid_args(DBusMessage *msg)
-{
-	return g_dbus_create_error(msg,"org.bluez.Error.InvalidArguments",
-					"Invalid arguments in method call");
-}
-
 static uint32_t get_callflag(const char *callerid_setting)
 {
 	if (callerid_setting != NULL) {
@@ -1950,7 +1945,7 @@ static DBusMessage *set_callerid(DBusConnection *conn, DBusMessage *msg,
 	if (dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING,
 						&callerid_setting,
 						DBUS_TYPE_INVALID) == FALSE)
-		return invalid_args(msg);
+		return btd_error_invalid_args(msg);
 
 	if (g_str_equal(callerid_setting, "allowed") ||
 			g_str_equal(callerid_setting, "restricted") ||
@@ -1964,7 +1959,7 @@ static DBusMessage *set_callerid(DBusConnection *conn, DBusMessage *msg,
 
 	error("telephony-maemo: invalid argument %s for method call"
 					" SetCallerId", callerid_setting);
-		return invalid_args(msg);
+		return btd_error_invalid_args(msg);
 }
 
 static GDBusMethodTable telephony_maemo_methods[] = {
diff --git a/audio/telephony-maemo6.c b/audio/telephony-maemo6.c
index f671a42..57f022d 100644
--- a/audio/telephony-maemo6.c
+++ b/audio/telephony-maemo6.c
@@ -38,6 +38,7 @@
 
 #include "log.h"
 #include "telephony.h"
+#include "error.h"
 
 /* SSC D-Bus definitions */
 #define SSC_DBUS_NAME  "com.nokia.phone.SSC"
@@ -1720,12 +1721,6 @@ static void csd_init(void)
 	}
 }
 
-static inline DBusMessage *invalid_args(DBusMessage *msg)
-{
-	return g_dbus_create_error(msg,"org.bluez.Error.InvalidArguments",
-					"Invalid arguments in method call");
-}
-
 static uint32_t get_callflag(const char *callerid_setting)
 {
 	if (callerid_setting != NULL) {
@@ -1790,7 +1785,7 @@ static DBusMessage *set_callerid(DBusConnection *conn, DBusMessage *msg,
 	if (dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING,
 						&callerid_setting,
 						DBUS_TYPE_INVALID) == FALSE)
-		return invalid_args(msg);
+		return btd_error_invalid_args(msg);
 
 	if (g_str_equal(callerid_setting, "allowed") ||
 			g_str_equal(callerid_setting, "restricted") ||
@@ -1804,7 +1799,7 @@ static DBusMessage *set_callerid(DBusConnection *conn, DBusMessage *msg,
 
 	error("telephony-maemo6: invalid argument %s for method call"
 					" SetCallerId", callerid_setting);
-		return invalid_args(msg);
+		return btd_error_invalid_args(msg);
 }
 
 static DBusMessage *clear_lastnumber(DBusConnection *conn, DBusMessage *msg,
diff --git a/audio/transport.c b/audio/transport.c
index eda46e1..48af0ea 100644
--- a/audio/transport.c
+++ b/audio/transport.c
@@ -93,12 +93,6 @@ struct media_transport {
 					DBusMessageIter *value);
 };
 
-static inline DBusMessage *invalid_args(DBusMessage *msg)
-{
-	return g_dbus_create_error(msg, ERROR_INTERFACE ".InvalidArguments",
-					"Invalid arguments in method call");
-}
-
 static inline DBusMessage *error_failed(DBusMessage *msg, const char *desc)
 {
 	return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed", "%s", desc);
@@ -549,16 +543,16 @@ static DBusMessage *set_property(DBusConnection *conn, DBusMessage *msg,
 	int err;
 
 	if (!dbus_message_iter_init(msg, &iter))
-		return invalid_args(msg);
+		return btd_error_invalid_args(msg);
 
 	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
-		return invalid_args(msg);
+		return btd_error_invalid_args(msg);
 
 	dbus_message_iter_get_basic(&iter, &property);
 	dbus_message_iter_next(&iter);
 
 	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_VARIANT)
-		return invalid_args(msg);
+		return btd_error_invalid_args(msg);
 	dbus_message_iter_recurse(&iter, &value);
 
 	sender = dbus_message_get_sender(msg);
@@ -577,7 +571,7 @@ static DBusMessage *set_property(DBusConnection *conn, DBusMessage *msg,
 
 	if (err < 0) {
 		if (err == -EINVAL)
-			return invalid_args(msg);
+			return btd_error_invalid_args(msg);
 		return error_failed(msg, strerror(-err));
 	}
 
diff --git a/health/hdp.c b/health/hdp.c
index 769e300..dc1f803 100644
--- a/health/hdp.c
+++ b/health/hdp.c
@@ -353,13 +353,8 @@ static DBusMessage *manager_create_application(DBusConnection *conn,
 	dbus_message_iter_init(msg, &iter);
 	app = hdp_get_app_config(&iter, &err);
 	if (err) {
-		DBusMessage *reply;
-
-		reply = g_dbus_create_error(msg,
-					ERROR_INTERFACE ".InvalidArguments",
-					"Invalid arguments: %s", err->message);
 		g_error_free(err);
-		return reply;
+		return btd_error_invalid_args(msg);
 	}
 
 	name = dbus_message_get_sender(msg);
@@ -400,11 +395,8 @@ static DBusMessage *manager_destroy_application(DBusConnection *conn,
 	GSList *l;
 
 	if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &path,
-						DBUS_TYPE_INVALID)){
-		return g_dbus_create_error(msg,
-					ERROR_INTERFACE ".InvalidArguments",
-					"Invalid arguments in method call");
-	}
+						DBUS_TYPE_INVALID))
+		return btd_error_invalid_args(msg);
 
 	l = g_slist_find_custom(applications, path, cmp_app);
 
@@ -1871,18 +1863,13 @@ static DBusMessage *device_create_channel(DBusConnection *conn,
 
 	if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &app_path,
 							DBUS_TYPE_STRING, &conf,
-							DBUS_TYPE_INVALID)) {
-		return g_dbus_create_error(msg,
-					ERROR_INTERFACE ".InvalidArguments",
-					"Invalid arguments in method call");
-	}
+							DBUS_TYPE_INVALID))
+		return btd_error_invalid_args(msg);
 
 	l = g_slist_find_custom(applications, app_path, cmp_app);
 	if (!l)
-		return g_dbus_create_error(msg,
-					ERROR_INTERFACE ".InvalidArguments",
-					"Invalid arguments in method call, "
-					"no such application");
+		return btd_error_invalid_args(msg);
+
 	app = l->data;
 
 	if (g_ascii_strcasecmp("Reliable", conf) == 0)
@@ -1892,25 +1879,16 @@ static DBusMessage *device_create_channel(DBusConnection *conn,
 	else if (g_ascii_strcasecmp("Any", conf) == 0)
 		config = HDP_NO_PREFERENCE_DC;
 	else
-		return g_dbus_create_error(msg,
-					ERROR_INTERFACE ".InvalidArguments",
-					"Invalid arguments in method call");
+		return btd_error_invalid_args(msg);
 
 	if (app->role == HDP_SINK && config != HDP_NO_PREFERENCE_DC)
-		return g_dbus_create_error(msg,
-					ERROR_INTERFACE ".InvalidArguments",
-					"Configuration not valid for sinks");
+		return btd_error_invalid_args(msg);
 
 	if (app->role == HDP_SOURCE && config == HDP_NO_PREFERENCE_DC)
-		return g_dbus_create_error(msg,
-					ERROR_INTERFACE ".InvalidArguments",
-					"Configuration not valid for sources");
+		return btd_error_invalid_args(msg);
 
 	if (!device->fr && config == HDP_STREAMING_DC)
-		return g_dbus_create_error(msg,
-					ERROR_INTERFACE ".InvalidArguments",
-					"Configuration not valid, first "
-					"channel should be reliable");
+		return btd_error_invalid_args(msg);
 
 	data = g_new0(struct hdp_create_dc, 1);
 	data->dev = health_device_ref(device);
@@ -1994,17 +1972,13 @@ static DBusMessage *device_destroy_channel(DBusConnection *conn,
 
 	if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &path,
 							DBUS_TYPE_INVALID)){
-		return g_dbus_create_error(msg,
-					ERROR_INTERFACE ".InvalidArguments",
-					"Invalid arguments in method call");
+		return btd_error_invalid_args(msg);
 	}
 
 	l = g_slist_find_custom(device->channels, path, cmp_chan_path);
 	if (!l)
-		return g_dbus_create_error(msg,
-					ERROR_INTERFACE ".InvalidArguments",
-					"Invalid arguments in method call, "
-					"no such channel");
+		return btd_error_invalid_args(msg);
+
 	hdp_chan = l->data;
 	del_data = g_new0(struct hdp_tmp_dc_data, 1);
 	del_data->msg = dbus_message_ref(msg);
diff --git a/network/server.c b/network/server.c
index bb119ce..33da06d 100644
--- a/network/server.c
+++ b/network/server.c
@@ -573,13 +573,6 @@ static inline DBusMessage *failed(DBusMessage *msg, const char *description)
 							"%s", description);
 }
 
-static inline DBusMessage *invalid_arguments(DBusMessage *msg,
-					const char *description)
-{
-	return g_dbus_create_error(msg, ERROR_INTERFACE ".InvalidArguments",
-							"%s", description);
-}
-
 static void server_disconnect(DBusConnection *conn, void *user_data)
 {
 	struct network_server *ns = user_data;
diff --git a/plugins/service.c b/plugins/service.c
index f37db7f..12e05c1 100644
--- a/plugins/service.c
+++ b/plugins/service.c
@@ -337,12 +337,6 @@ static void exit_callback(DBusConnection *conn, void *user_data)
 	g_free(user_record);
 }
 
-static inline DBusMessage *invalid_arguments(DBusMessage *msg)
-{
-	return g_dbus_create_error(msg, ERROR_INTERFACE ".InvalidArguments",
-					"Invalid arguments in method call");
-}
-
 static inline DBusMessage *not_available(DBusMessage *msg)
 {
 	return g_dbus_create_error(msg, ERROR_INTERFACE ".NotAvailable",
@@ -464,7 +458,7 @@ static DBusMessage *update_xml_record(DBusConnection *conn,
 
 	len = (record ? strlen(record) : 0);
 	if (len == 0)
-		return invalid_arguments(msg);
+		return btd_error_invalid_args(msg);
 
 	user_record = find_record(serv_adapter, handle,
 				dbus_message_get_sender(msg));
diff --git a/serial/port.c b/serial/port.c
index 0398f2e..b593311 100644
--- a/serial/port.c
+++ b/serial/port.c
@@ -57,7 +57,6 @@
 #include "port.h"
 
 #define SERIAL_PORT_INTERFACE	"org.bluez.Serial"
-#define ERROR_INVALID_ARGS	"org.bluez.Error.InvalidArguments"
 #define ERROR_DOES_NOT_EXIST	"org.bluez.Error.DoesNotExist"
 
 #define MAX_OPEN_TRIES		5
@@ -243,13 +242,6 @@ static inline DBusMessage *does_not_exist(DBusMessage *msg,
 							"%s", description);
 }
 
-static inline DBusMessage *invalid_arguments(DBusMessage *msg,
-					const char *description)
-{
-	return g_dbus_create_error(msg, ERROR_INTERFACE ".InvalidArguments",
-							"%s", description);
-}
-
 static inline DBusMessage *failed(DBusMessage *msg, const char *description)
 {
 	return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed",
diff --git a/serial/proxy.c b/serial/proxy.c
index 2211583..b5f5578 100644
--- a/serial/proxy.c
+++ b/serial/proxy.c
@@ -138,13 +138,6 @@ static inline DBusMessage *does_not_exist(DBusMessage *msg,
 							"%s", description);
 }
 
-static inline DBusMessage *invalid_arguments(DBusMessage *msg,
-					const char *description)
-{
-	return g_dbus_create_error(msg, ERROR_INTERFACE ".InvalidArguments",
-							"%s", description);
-}
-
 static inline DBusMessage *failed(DBusMessage *msg, const char *description)
 {
 	return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed",
@@ -753,17 +746,17 @@ static DBusMessage *proxy_set_serial_params(DBusConnection *conn,
 		return NULL;
 
 	if (str2speed(ratestr, &speed)  == B0)
-		return invalid_arguments(msg, "Invalid baud rate");
+		return btd_error_invalid_args(msg);
 
 	ctrl = prx->proxy_ti.c_cflag;
 	if (set_databits(databits, &ctrl) < 0)
-		return invalid_arguments(msg, "Invalid data bits");
+		return btd_error_invalid_args(msg);
 
 	if (set_stopbits(stopbits, &ctrl) < 0)
-		return invalid_arguments(msg, "Invalid stop bits");
+		return btd_error_invalid_args(msg);
 
 	if (set_parity(paritystr, &ctrl) < 0)
-		return invalid_arguments(msg, "Invalid parity");
+		return btd_error_invalid_args(msg);
 
 	prx->proxy_ti.c_cflag = ctrl;
 	prx->proxy_ti.c_cflag |= (CLOCAL | CREAD);
@@ -1055,13 +1048,13 @@ static DBusMessage *create_proxy(DBusConnection *conn,
 
 	uuid_str = bt_name2string(pattern);
 	if (!uuid_str)
-		return invalid_arguments(msg, "Invalid UUID");
+		return btd_error_invalid_args(msg);
 
 	err = register_proxy(adapter, uuid_str, address, &proxy);
 	g_free(uuid_str);
 
 	if (err == -EINVAL)
-		return invalid_arguments(msg, "Invalid address");
+		return btd_error_invalid_args(msg);
 	else if (err == -EALREADY)
 		return g_dbus_create_error(msg, ERROR_INTERFACE ".AlreadyExist",
 						"Proxy already exists");
diff --git a/src/adapter.c b/src/adapter.c
index 62afc0c..428de66 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -146,12 +146,6 @@ struct btd_adapter {
 static void adapter_set_pairable_timeout(struct btd_adapter *adapter,
 					guint interval);
 
-static inline DBusMessage *invalid_args(DBusMessage *msg)
-{
-	return g_dbus_create_error(msg, ERROR_INTERFACE ".InvalidArguments",
-			"Invalid arguments in method call");
-}
-
 static inline DBusMessage *adapter_not_ready(DBusMessage *msg)
 {
 	return g_dbus_create_error(msg, ERROR_INTERFACE ".NotReady",
@@ -1041,7 +1035,7 @@ static DBusMessage *set_name(DBusConnection *conn, DBusMessage *msg,
 
 	if (!g_utf8_validate(name, -1, NULL)) {
 		error("Name change failed: supplied name isn't valid UTF-8");
-		return invalid_args(msg);
+		return btd_error_invalid_args(msg);
 	}
 
 	if (strncmp(name, dev->name, MAX_NAME_LENGTH) == 0)
@@ -1488,23 +1482,23 @@ static DBusMessage *set_property(DBusConnection *conn,
 	ba2str(&adapter->bdaddr, srcaddr);
 
 	if (!dbus_message_iter_init(msg, &iter))
-		return invalid_args(msg);
+		return btd_error_invalid_args(msg);
 
 	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
-		return invalid_args(msg);
+		return btd_error_invalid_args(msg);
 
 	dbus_message_iter_get_basic(&iter, &property);
 	dbus_message_iter_next(&iter);
 
 	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_VARIANT)
-		return invalid_args(msg);
+		return btd_error_invalid_args(msg);
 	dbus_message_iter_recurse(&iter, &sub);
 
 	if (g_str_equal("Name", property)) {
 		const char *name;
 
 		if (dbus_message_iter_get_arg_type(&sub) != DBUS_TYPE_STRING)
-			return invalid_args(msg);
+			return btd_error_invalid_args(msg);
 		dbus_message_iter_get_basic(&sub, &name);
 
 		return set_name(conn, msg, name, data);
@@ -1512,7 +1506,7 @@ static DBusMessage *set_property(DBusConnection *conn,
 		gboolean powered;
 
 		if (dbus_message_iter_get_arg_type(&sub) != DBUS_TYPE_BOOLEAN)
-			return invalid_args(msg);
+			return btd_error_invalid_args(msg);
 
 		dbus_message_iter_get_basic(&sub, &powered);
 
@@ -1521,7 +1515,7 @@ static DBusMessage *set_property(DBusConnection *conn,
 		gboolean discoverable;
 
 		if (dbus_message_iter_get_arg_type(&sub) != DBUS_TYPE_BOOLEAN)
-			return invalid_args(msg);
+			return btd_error_invalid_args(msg);
 
 		dbus_message_iter_get_basic(&sub, &discoverable);
 
@@ -1530,7 +1524,7 @@ static DBusMessage *set_property(DBusConnection *conn,
 		uint32_t timeout;
 
 		if (dbus_message_iter_get_arg_type(&sub) != DBUS_TYPE_UINT32)
-			return invalid_args(msg);
+			return btd_error_invalid_args(msg);
 
 		dbus_message_iter_get_basic(&sub, &timeout);
 
@@ -1539,7 +1533,7 @@ static DBusMessage *set_property(DBusConnection *conn,
 		gboolean pairable;
 
 		if (dbus_message_iter_get_arg_type(&sub) != DBUS_TYPE_BOOLEAN)
-			return invalid_args(msg);
+			return btd_error_invalid_args(msg);
 
 		dbus_message_iter_get_basic(&sub, &pairable);
 
@@ -1548,14 +1542,14 @@ static DBusMessage *set_property(DBusConnection *conn,
 		uint32_t timeout;
 
 		if (dbus_message_iter_get_arg_type(&sub) != DBUS_TYPE_UINT32)
-			return invalid_args(msg);
+			return btd_error_invalid_args(msg);
 
 		dbus_message_iter_get_basic(&sub, &timeout);
 
 		return set_pairable_timeout(conn, msg, timeout, data);
 	}
 
-	return invalid_args(msg);
+	return btd_error_invalid_args(msg);
 }
 
 static DBusMessage *request_session(DBusConnection *conn,
@@ -1629,7 +1623,7 @@ static DBusMessage *list_devices(DBusConnection *conn,
 	const gchar *dev_path;
 
 	if (!dbus_message_has_signature(msg, DBUS_TYPE_INVALID_AS_STRING))
-		return invalid_args(msg);
+		return btd_error_invalid_args(msg);
 
 	reply = dbus_message_new_method_return(msg);
 	if (!reply)
@@ -1662,10 +1656,10 @@ static DBusMessage *cancel_device_creation(DBusConnection *conn,
 
 	if (dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &address,
 						DBUS_TYPE_INVALID) == FALSE)
-		return invalid_args(msg);
+		return btd_error_invalid_args(msg);
 
 	if (check_address(address) < 0)
-		return invalid_args(msg);
+		return btd_error_invalid_args(msg);
 
 	device = adapter_find_device(adapter, address);
 	if (!device || !device_is_creating(device, NULL))
@@ -1698,10 +1692,10 @@ static DBusMessage *create_device(DBusConnection *conn,
 
 	if (dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &address,
 						DBUS_TYPE_INVALID) == FALSE)
-		return invalid_args(msg);
+		return btd_error_invalid_args(msg);
 
 	if (check_address(address) < 0)
-		return invalid_args(msg);
+		return btd_error_invalid_args(msg);
 
 	if (adapter_find_device(adapter, address))
 		return g_dbus_create_error(msg,
@@ -1755,21 +1749,21 @@ static DBusMessage *create_paired_device(DBusConnection *conn,
 					DBUS_TYPE_OBJECT_PATH, &agent_path,
 					DBUS_TYPE_STRING, &capability,
 					DBUS_TYPE_INVALID) == FALSE)
-		return invalid_args(msg);
+		return btd_error_invalid_args(msg);
 
 	if (check_address(address) < 0)
-		return invalid_args(msg);
+		return btd_error_invalid_args(msg);
 
 	sender = dbus_message_get_sender(msg);
 	if (adapter->agent &&
 			agent_matches(adapter->agent, sender, agent_path)) {
 		error("Refusing adapter agent usage as device specific one");
-		return invalid_args(msg);
+		return btd_error_invalid_args(msg);
 	}
 
 	cap = parse_io_capability(capability);
 	if (cap == IO_CAPABILITY_INVALID)
-		return invalid_args(msg);
+		return btd_error_invalid_args(msg);
 
 	device = adapter_get_device(conn, adapter, address);
 	if (!device)
@@ -1797,7 +1791,7 @@ static DBusMessage *remove_device(DBusConnection *conn, DBusMessage *msg,
 
 	if (dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &path,
 						DBUS_TYPE_INVALID) == FALSE)
-		return invalid_args(msg);
+		return btd_error_invalid_args(msg);
 
 	l = g_slist_find_custom(adapter->devices,
 			path, (GCompareFunc) device_path_cmp);
@@ -1835,7 +1829,7 @@ static DBusMessage *find_device(DBusConnection *conn, DBusMessage *msg,
 
 	if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &address,
 						DBUS_TYPE_INVALID))
-		return invalid_args(msg);
+		return btd_error_invalid_args(msg);
 
 	l = g_slist_find_custom(adapter->devices,
 			address, (GCompareFunc) device_address_cmp);
@@ -1883,7 +1877,7 @@ static DBusMessage *register_agent(DBusConnection *conn, DBusMessage *msg,
 
 	cap = parse_io_capability(capability);
 	if (cap == IO_CAPABILITY_INVALID)
-		return invalid_args(msg);
+		return btd_error_invalid_args(msg);
 
 	name = dbus_message_get_sender(msg);
 
diff --git a/src/device.c b/src/device.c
index 5326e3f..ab7ef93 100644
--- a/src/device.c
+++ b/src/device.c
@@ -547,12 +547,6 @@ static DBusMessage *set_blocked(DBusConnection *conn, DBusMessage *msg,
 	}
 }
 
-static inline DBusMessage *invalid_args(DBusMessage *msg)
-{
-	return g_dbus_create_error(msg, ERROR_INTERFACE ".InvalidArguments",
-					"Invalid arguments in method call");
-}
-
 static DBusMessage *set_property(DBusConnection *conn,
 				DBusMessage *msg, void *data)
 {
@@ -561,22 +555,22 @@ static DBusMessage *set_property(DBusConnection *conn,
 	const char *property;
 
 	if (!dbus_message_iter_init(msg, &iter))
-		return invalid_args(msg);
+		return btd_error_invalid_args(msg);
 
 	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
-		return invalid_args(msg);
+		return btd_error_invalid_args(msg);
 
 	dbus_message_iter_get_basic(&iter, &property);
 	dbus_message_iter_next(&iter);
 
 	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_VARIANT)
-		return invalid_args(msg);
+		return btd_error_invalid_args(msg);
 	dbus_message_iter_recurse(&iter, &sub);
 
 	if (g_str_equal("Trusted", property)) {
 		dbus_bool_t value;
 		if (dbus_message_iter_get_arg_type(&sub) != DBUS_TYPE_BOOLEAN)
-			return invalid_args(msg);
+			return btd_error_invalid_args(msg);
 		dbus_message_iter_get_basic(&sub, &value);
 
 		return set_trust(conn, msg, value, data);
@@ -584,7 +578,7 @@ static DBusMessage *set_property(DBusConnection *conn,
 		const char *alias;
 
 		if (dbus_message_iter_get_arg_type(&sub) != DBUS_TYPE_STRING)
-			return invalid_args(msg);
+			return btd_error_invalid_args(msg);
 		dbus_message_iter_get_basic(&sub, &alias);
 
 		return set_alias(conn, msg, alias, data);
@@ -592,14 +586,14 @@ static DBusMessage *set_property(DBusConnection *conn,
 		dbus_bool_t value;
 
 		if (dbus_message_iter_get_arg_type(&sub) != DBUS_TYPE_BOOLEAN)
-			return invalid_args(msg);
+			return btd_error_invalid_args(msg);
 
 		dbus_message_iter_get_basic(&sub, &value);
 
 		return set_blocked(conn, msg, value, data);
 	}
 
-	return invalid_args(msg);
+	return btd_error_invalid_args(msg);
 }
 
 static void discover_services_req_exit(DBusConnection *conn, void *user_data)
@@ -634,7 +628,7 @@ static DBusMessage *discover_services(DBusConnection *conn,
 		uuid_t uuid;
 
 		if (bt_string2uuid(&uuid, pattern) < 0)
-			return invalid_args(msg);
+			return btd_error_invalid_args(msg);
 
 		sdp_uuid128_to_uuid(&uuid);
 
diff --git a/src/error.c b/src/error.c
index c1a2fbf..0807965 100644
--- a/src/error.c
+++ b/src/error.c
@@ -48,3 +48,10 @@ DBusHandlerResult error_common_reply(DBusConnection *conn, DBusMessage *msg,
 
 	return DBUS_HANDLER_RESULT_HANDLED;
 }
+
+DBusMessage *btd_error_invalid_args(DBusMessage *msg)
+{
+	return g_dbus_create_error(msg, ERROR_INTERFACE
+					".InvalidArguments",
+					"Invalid arguments in method call");
+}
diff --git a/src/error.h b/src/error.h
index 49ec05e..3a01114 100644
--- a/src/error.h
+++ b/src/error.h
@@ -29,3 +29,5 @@
 
 DBusHandlerResult error_common_reply(DBusConnection *conn, DBusMessage *msg,
 					const char *name, const char *descr);
+
+DBusMessage *btd_error_invalid_args(DBusMessage *msg);
diff --git a/src/manager.c b/src/manager.c
index 27eeea7..939a563 100644
--- a/src/manager.c
+++ b/src/manager.c
@@ -64,13 +64,6 @@ void manager_update_svc(struct btd_adapter* adapter, uint8_t svc)
 	adapter_set_service_classes(adapter, svc);
 }
 
-static inline DBusMessage *invalid_args(DBusMessage *msg)
-{
-	return g_dbus_create_error(msg,
-			ERROR_INTERFACE ".InvalidArguments",
-			"Invalid arguments in method call");
-}
-
 static inline DBusMessage *no_such_adapter(DBusMessage *msg)
 {
 	return g_dbus_create_error(msg,
-- 
1.7.3.2


^ permalink raw reply related


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