Linux bluetooth development
 help / color / mirror / Atom feed
* [RFC v2 1/9] Bluetooth: Add SMP command structures
From: Vinicius Costa Gomes @ 2010-12-06 21:43 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Ville Tervo
In-Reply-To: <1291671832-13435-1-git-send-email-vinicius.gomes@openbossa.org>

From: Ville Tervo <ville.tervo@nokia.com>

Add command structures for security manager protocol.

Signed-off-by: Ville Tervo <ville.tervo@nokia.com>
---
 include/net/bluetooth/smp.h |   76 +++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 76 insertions(+), 0 deletions(-)
 create mode 100644 include/net/bluetooth/smp.h

diff --git a/include/net/bluetooth/smp.h b/include/net/bluetooth/smp.h
new file mode 100644
index 0000000..8f2edbf
--- /dev/null
+++ b/include/net/bluetooth/smp.h
@@ -0,0 +1,76 @@
+#ifndef __SMP_H
+#define __SMP_H
+
+struct smp_command_hdr {
+	__u8	code;
+} __packed;
+
+#define SMP_CMD_PAIRING_REQ	0x01
+#define SMP_CMD_PAIRING_RSP	0x02
+struct smp_cmd_pairing {
+	__u8	io_capability;
+	__u8	oob_flag;
+	__u8	auth_req;
+	__u8	max_key_size;
+	__u8	init_key_dist;
+	__u8	resp_key_dist;
+} __packed;
+
+#define SMP_CMD_PAIRING_CONFIRM	0x03
+struct smp_cmd_pairing_confirm {
+	__u8	confirm_val[16];
+} __packed;
+
+#define SMP_CMD_PAIRING_RANDOM	0x04
+struct smp_cmd_pairing_random {
+	__u8	rand_val[16];
+} __packed;
+
+#define SMP_CMD_PAIRING_FAIL	0x05
+struct smp_cmd_pairing_fail {
+	__u8	reason;
+} __packed;
+
+#define SMP_CMD_ENCRYPT_INFO	0x06
+struct smp_cmd_encrypt_info {
+	__u8	ltk[16];
+} __packed;
+
+#define SMP_CMD_MASTER_IDENT	0x07
+struct smp_cmd_master_ident {
+	__u16	ediv;
+	__u8	rand[8];
+} __packed;
+
+#define SMP_CMD_IDENT_INFO	0x08
+struct smp_cmd_ident_info {
+	__u8	irk[16];
+} __packed;
+
+#define SMP_CMD_IDENT_ADDR_INFO	0x09
+struct smp_cmd_ident_addr_info {
+	__u8	addr_type;
+	bdaddr_t bdaddr;
+} __packed;
+
+#define SMP_CMD_SIGN_INFO	0x0a
+struct smp_cmd_sign_info {
+	__u8	csrk[16];
+} __packed;
+
+#define SMP_CMD_SECURITY_REQ	0x0b
+struct smp_cmd_security_req {
+	__u8	auth_req;
+} __packed;
+
+#define SMP_PASSKEY_ENTRY_FAILED	0x01
+#define SMP_OOB_NOT_AVAIL		0x02
+#define SMP_AUTH_REQUIREMENTS		0x03
+#define SMP_CONFIRM_FAILED		0x04
+#define SMP_PAIRING_NOTSUPP		0x05
+#define SMP_ENC_KEY_SIZE		0x06
+#define SMP_CMD_NOTSUPP		0x07
+#define SMP_UNSPECIFIED		0x08
+#define SMP_REPEATED_ATTEMPTS		0x09
+
+#endif /* __SMP_H */
-- 
1.7.3.2


^ permalink raw reply related

* [RFC v2 0/9] SMP Implementation
From: Vinicius Costa Gomes @ 2010-12-06 21:43 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Vinicius Costa Gomes

Hi,


Here goes just the SMP implementation (excluding Ville's LE patches).

Ville, I took the liberty of making the changes that Gustavo suggested to your
patches, the final result is here:

git://git.infradead.org/users/vcgomes/linux-2.6.git for-next

This tree is rebased on top of bluetooth-next.


Cheers
--

Anderson Briglia (3):
  Bluetooth: Start SMP procedure
  Bluetooth: simple SMP pairing negotiation
  Bluetooth: LE SMP Cryptoolbox functions

Ville Tervo (1):
  Bluetooth: Add SMP command structures

Vinicius Costa Gomes (5):
  Bluetooth: Implement the first SMP commands
  Bluetooth: Add support for using the crypto subsystem
  Bluetooth: Add support for SMP confirmation checks
  Bluetooth: Add support for LE Start Encryption
  Bluetooth: Add support for resuming socket when SMP is finished

 include/net/bluetooth/hci.h             |   34 +++
 include/net/bluetooth/hci_core.h        |    7 +
 include/net/bluetooth/l2cap.h           |    5 +
 include/net/bluetooth/smp.h             |   80 ++++++
 net/bluetooth/Makefile                  |    1 +
 net/bluetooth/hci_conn.c                |   47 +++
 net/bluetooth/hci_core.c                |   10 +
 net/bluetooth/hci_event.c               |   67 +++++
 net/bluetooth/{l2cap.c => l2cap_core.c} |   78 ++++--
 net/bluetooth/smp.c                     |  469 +++++++++++++++++++++++++++++++
 10 files changed, 769 insertions(+), 29 deletions(-)
 create mode 100644 include/net/bluetooth/smp.h
 rename net/bluetooth/{l2cap.c => l2cap_core.c} (99%)
 create mode 100644 net/bluetooth/smp.c

-- 
1.7.3.2


^ permalink raw reply

* Re: [PATCH v7] Bluetooth: btwilink driver
From: Vitaly Wool @ 2010-12-06 21:35 UTC (permalink / raw)
  To: Gustavo F. Padovan; +Cc: Pavan Savoy, marcel, linux-bluetooth, linux-kernel
In-Reply-To: <20101206212326.GI883@vigoh>

Hi Gustavo,

On Mon, Dec 6, 2010 at 10:23 PM, Gustavo F. Padovan
<padovan@profusion.mobi> wrote:

> Can't you differentiate Bluetooth data in a generic way, withou looking if it
> is ACL, SCO or HCI EVENT? That done, you can just accumulate in a buffer all
> the Bluetooth data you received in that stream then send it to Bluetooth
> driver after finish that stream processing.

I'm afraid he can't do this because he needs to route events to the
appropriate entity (BT/FM/GPS). I'm not sure how it can be done
without analyzing the incoming packet.

Thanks,
   Vitaly

^ permalink raw reply

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

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

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

Sure, the TI "shared transport" code is mostly at drivers/misc/ti-st.

Some Broadcom BCM43xx chips work in a similar way AFAIK but I'm not
sure about the mainline support for those.

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

I guess so but it's probably more of a detail than what I'm up to now :)

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

I actually don't think it's such a big indirection, I prefer to think
of it more as of the abstraction layer. If not use this, are we going
to have direct SPI device drivers? I'm afraid we might end up with a
huge duplication of code in that case.

Thanks,
   Vitaly

^ permalink raw reply

* Re: [PATCH v7] Bluetooth: btwilink driver
From: Gustavo F. Padovan @ 2010-12-06 21:23 UTC (permalink / raw)
  To: Pavan Savoy; +Cc: marcel, linux-bluetooth, linux-kernel
In-Reply-To: <AANLkTiktDkjFH=aGbu3f+mkVUgmFDwB6xLW3e1VNXKtz@mail.gmail.com>

Hi Pavan,

* Pavan Savoy <pavan_savoy@ti.com> [2010-11-30 21:30:47 +0530]:

> Gustavo,
>=20
> On Tue, Nov 30, 2010 at 9:16 PM, Gustavo F. Padovan
> <padovan@profusion.mobi> wrote:
> > Hi Pavan,
> >
> > * pavan_savoy@ti.com <pavan_savoy@ti.com> [2010-11-26 04:20:57 -0500]:
> >
> >> From: Pavan Savoy <pavan_savoy@ti.com>
> >>
> >> Marcel, Gustavo,
> >>
> >> comments attended to from v5 and v6,
> >>
> >> 1. Inside ti_st_open, I previously only checked for EINPROGRESS & EPER=
M,
> >> Now I handle for EINPROGRESS - which is not really an error and
> >> return during all other error cases.
> >>
> >> 2. _write is still a function pointer and not an exported function, I
> >> need to change the underlying driver's code for this.
> >> However, previous lkml comments on the underlying driver's code
> >> suggested it to be kept as a function pointer and not EXPORT.
> >> Gustavo, Marcel - Please comment on this.
> >> Is this absolutely required? If so why?
> >>
> >> 3. test_and_set_bit of HCI_RUNNING is done at beginning of
> >> ti_st_open, and did not see issues during firmware download.
> >> However ideally I would still like to set HCI_RUNNING once the firmware
> >> download is done, because I don't want to allow a _send_frame during
> >> firmware download - Marcel, Gustavo - Please comment.
> >>
> >> 4. test_and_clear of HCI_RUNNING now done @ beginning of close.
> >>
> >> 5. EAGAIN on failure of st_write is to suggest to try and write again.
> >> I have never this happen - However only if UART goes bad this case may
> >> occur.
> >>
> >> 6. ti_st_tx_complete is very similar to hci_ldisc's tx_complete - in
> >> fact the code is pretty much borrowed from there.
> >> Marcel, Gustavo - Please suggest where should it be done? If not here.
> >>
> >> 7. comments cleaned-up + hst memory leak fixed when hci_alloc_dev fail=
s.
> >>
> >> 8. platform_driver registration inside module_init now is similar to
> >> other drivers.
> >>
> >> 9. Dan Carpenter's comments on leaking hst memory on failed
> >> hci_register_dev and empty space after quotes in debug statements
> >> fixed.
> >>
> >> Thanks for the comments...
> >> Sorry, for previously not being very clear on which comments were
> >> handled and which were not.
> >>
> >> -- patch description --
> >>
> >> This is the bluetooth protocol driver for the TI WiLink7 chipsets.
> >> Texas Instrument's WiLink chipsets combine wireless technologies
> >> like BT, FM, GPS and WLAN onto a single chip.
> >>
> >> This Bluetooth driver works on top of the TI_ST shared transport
> >> line discipline driver which also allows other drivers like
> >> FM V4L2 and GPS character driver to make use of the same UART interfac=
e.
> >>
> >> Kconfig and Makefile modifications to enable the Bluetooth
> >> driver for Texas Instrument's WiLink 7 chipset.
> >>
> >> Signed-off-by: Pavan Savoy <pavan_savoy@ti.com>
> >> ---
> >> =A0drivers/bluetooth/Kconfig =A0 =A0| =A0 10 ++
> >> =A0drivers/bluetooth/Makefile =A0 | =A0 =A01 +
> >> =A0drivers/bluetooth/btwilink.c | =A0363 +++++++++++++++++++++++++++++=
+++++++++++++
> >> =A03 files changed, 374 insertions(+), 0 deletions(-)
> >> =A0create mode 100644 drivers/bluetooth/btwilink.c
> >
> > So as part of reviewing this I took a look at your underlying driver and
> > I didn't like what I saw there, you are handling Bluetooth stuff inside
> > the core driver and that is just wrong. You have a Bluetooth driver here
> > then you have to leave the Bluetooth data handling to the Bluetooth
> > driver and do not do that in the core.
>=20
> Thanks for reviewing this and the underlying driver.
> yes, we do have Bluetooth/FM/GPS handling inside the TI ST driver, on
> addition of further technologies
> we do plan to have them inside the ST driver too.
>=20
> The understanding of BT or FM or GPS is required for the ST driver
> because, the data coming from the chip
> can either be of these technologies, further-more the data might not
> come in a set.
> As in, an a2dp/ftp ACL frame might come in 2 frames instead of 1, and
> in other cases,
> there might be a HCI-EVENT + FM CH8 data in a single frame received by th=
e UART.

Can't you differentiate Bluetooth data in a generic way, withou looking if =
it
is ACL, SCO or HCI EVENT? That done, you can just accumulate in a buffer all
the Bluetooth data you received in that stream then send it to Bluetooth
driver after finish that stream processing.

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

^ permalink raw reply

* Re: [PATCH 1/1] bluetooth: Fix NULL pointer dereference issue
From: Gustavo F. Padovan @ 2010-12-06 21:15 UTC (permalink / raw)
  To: Yuri Ershov
  Cc: marcel, davem, jprvita, linux-bluetooth, ville.tervo,
	andrei.emeltchenko
In-Reply-To: <e98b2c6fd02b11300d82577cb54cfdb63e8400d2.1290678703.git.ext-yuri.ershov@nokia.com>

Hi Yuri,

* Yuri Ershov <ext-yuri.ershov@nokia.com> [2010-11-25 12:55:33 +0300]:

> This patch is an addition to my previous patch for this issue.
> The problem is in resynchronization between two loops:
> 1. Main controlling loop (l2cap_connect_req, l2cap_config_req,
> l2cap_config_rsp, l2cap_disconnect_req, etc.)
> 2. Loop waiting of BT_CONNECTED state of socket (l2cap_sock_accept,
> bt_accept_dequeue, etc.).
> In case of fast sequence of connect/disconnect operations the loop #1
> makes several cycles, while the loop #2 only has time to make one
> cycle and it results crash.
> The aim of the patch is to skeep handling of sockets queued for
> deletion.
> 
> Signed-off-by: Yuri Ershov <ext-yuri.ershov@nokia.com>
> ---
>  net/bluetooth/af_bluetooth.c |    2 ++
>  net/bluetooth/l2cap.c        |    6 ++++--
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c
> index c4cf3f5..f9389da 100644
> --- a/net/bluetooth/af_bluetooth.c
> +++ b/net/bluetooth/af_bluetooth.c
> @@ -200,6 +200,8 @@ struct sock *bt_accept_dequeue(struct sock *parent, struct socket *newsock)
>  	BT_DBG("parent %p", parent);
>  
>  	list_for_each_safe(p, n, &bt_sk(parent)->accept_q) {
> +		if (n == p)
> +			break;

So in which situations (n == p), or (p == p->next)? That should happen only
when p is the only element in the list, then p == head, right?

>  		sk = (struct sock *) list_entry(p, struct bt_sock, accept_q);
>  
>  		lock_sock(sk);
> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> index 12b4aa2..29f30b0 100644
> --- a/net/bluetooth/l2cap.c
> +++ b/net/bluetooth/l2cap.c
> @@ -133,7 +133,8 @@ static struct sock *__l2cap_get_chan_by_dcid(struct l2cap_chan_list *l, u16 cid)
>  {
>  	struct sock *s;
>  	for (s = l->head; s; s = l2cap_pi(s)->next_c) {
> -		if (l2cap_pi(s)->dcid == cid)
> +		if ((l2cap_pi(s)->dcid == cid) &&
> +		    (sk->sk_state != BT_DISCONN) && (sk->sk_state != BT_CLOSED))

I think its better check for the cid first, and if it matches check for the
socket states, if they are BT_DISCONN or BT_CLOSED return NULL. Then you avoid
unnecessary loops here.

>  			break;
>  	}
>  	return s;
> @@ -143,7 +144,8 @@ static struct sock *__l2cap_get_chan_by_scid(struct l2cap_chan_list *l, u16 cid)
>  {
>  	struct sock *s;
>  	for (s = l->head; s; s = l2cap_pi(s)->next_c) {
> -		if (l2cap_pi(s)->scid == cid)
> +		if ((l2cap_pi(s)->scid == cid) &&
> +		    (sk->sk_state != BT_DISCONN) && (sk->sk_state != BT_CLOSED))
>  			break;

Same for this one.

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

^ permalink raw reply

* Re: [PATCH 2/3] Bluetooth: Add initial Bluetooth Management interface callbacks
From: Gustavo F. Padovan @ 2010-12-06 20:37 UTC (permalink / raw)
  To: Anderson Lizardo, linux-bluetooth
In-Reply-To: <20101206142116.GA19084@jh-x301>

Hi Johan,

* Johan Hedberg <johan.hedberg@gmail.com> [2010-12-06 16:21:16 +0200]:

> 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 :)

>From what I understand the Acked-by is more about the whole idea of the patch,
so to me it is fine to do such styles fixes and keep the ack in the patch. You
won't change essentials parts of the patch.

-- 
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 20:07 UTC (permalink / raw)
  To: Joe Perches
  Cc: Michał Mirosław, Marcel Holtmann, netdev,
	David S. Miller, linux-bluetooth, linux-kernel
In-Reply-To: <1291661413.17494.219.camel@Joe-Laptop>

Hi Joe,

* Joe Perches <joe@perches.com> [2010-12-06 10:50:13 -0800]:

> 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?
>=20
> No worries, it was done against next-20101202.
>=20
> Do you care about using %pMR vs %pMbt as Micha=C5=82 suggested in
> https://lkml.org/lkml/2010/12/4/21 ?

I'm fine either way. It depends more if another subsystem will want to use
%pMR or not as you said.

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

^ permalink raw reply

* [PATCH 9/9] Add btd_error_not_authorized()
From: Gustavo F. Padovan @ 2010-12-06 19:10 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1291662648-10651-8-git-send-email-padovan@profusion.mobi>

---
 attrib/client.c   |    8 +-------
 plugins/service.c |   22 ++++++++--------------
 src/adapter.c     |    8 +-------
 src/device.c      |    8 ++------
 src/error.c       |    7 +++++++
 src/error.h       |    1 +
 6 files changed, 20 insertions(+), 34 deletions(-)

diff --git a/attrib/client.c b/attrib/client.c
index ac78fbd..0805492 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 *not_authorized(DBusMessage *msg)
-{
-	return g_dbus_create_error(msg, ERROR_INTERFACE ".NotAuthorized",
-			"Not authorized");
-}
-
 static void append_char_dict(DBusMessageIter *iter, struct characteristic *chr)
 {
 	DBusMessageIter dict;
@@ -502,7 +496,7 @@ static DBusMessage *unregister_watcher(DBusConnection *conn,
 	l = g_slist_find_custom(prim->watchers, match, watcher_cmp);
 	watcher_free(match);
 	if (!l)
-		return not_authorized(msg);
+		return btd_error_not_authorized(msg);
 
 	watcher = l->data;
 	g_dbus_remove_watch(conn, watcher->id);
diff --git a/plugins/service.c b/plugins/service.c
index 406ee7d..5267671 100644
--- a/plugins/service.c
+++ b/plugins/service.c
@@ -348,12 +348,6 @@ static inline DBusMessage *failed_strerror(DBusMessage *msg, int err)
 							"%s", strerror(err));
 }
 
-static inline DBusMessage *not_authorized(DBusMessage *msg)
-{
-	return g_dbus_create_error(msg, ERROR_INTERFACE ".NotAuthorized",
-					"Not Authorized");
-}
-
 static int add_xml_record(DBusConnection *conn, const char *sender,
 			struct service_adapter *serv_adapter,
 			const char *record, dbus_uint32_t *handle)
@@ -555,7 +549,7 @@ static void auth_cb(DBusError *derr, void *user_data)
 	if (derr) {
 		error("Access denied: %s", derr->message);
 
-		reply = not_authorized(auth->msg);
+		reply = btd_error_not_authorized(auth->msg);
 		dbus_message_unref(auth->msg);
 		g_dbus_send_message(auth->conn, reply);
 		goto done;
@@ -612,20 +606,20 @@ static DBusMessage *request_authorization(DBusConnection *conn,
 	if (!user_record) {
 		user_record = find_record(serv_adapter_any, handle, sender);
 		if (!user_record)
-			return not_authorized(msg);
+			return btd_error_not_authorized(msg);
 	}
 
 	record = sdp_record_find(user_record->handle);
 	if (record == NULL)
-		return not_authorized(msg);
+		return btd_error_not_authorized(msg);
 
 	if (sdp_get_service_classes(record, &services) < 0) {
 		sdp_record_free(record);
-		return not_authorized(msg);
+		return btd_error_not_authorized(msg);
 	}
 
 	if (services == NULL)
-		return not_authorized(msg);
+		return btd_error_not_authorized(msg);
 
 	uuid = services->data;
 	uuid128 = sdp_uuid_to_uuid128(uuid);
@@ -634,7 +628,7 @@ static DBusMessage *request_authorization(DBusConnection *conn,
 
 	if (sdp_uuid2strn(uuid128, uuid_str, MAX_LEN_UUID_STR) < 0) {
 		bt_free(uuid128);
-		return not_authorized(msg);
+		return btd_error_not_authorized(msg);
 	}
 	bt_free(uuid128);
 
@@ -662,7 +656,7 @@ static DBusMessage *request_authorization(DBusConnection *conn,
 		serv_adapter->pending_list = g_slist_remove(serv_adapter->pending_list,
 									auth);
 		g_free(auth);
-		return not_authorized(msg);
+		return btd_error_not_authorized(msg);
 	}
 
 	return NULL;
@@ -690,7 +684,7 @@ static DBusMessage *cancel_authorization(DBusConnection *conn,
 
 	btd_cancel_authorization(&src, &auth->dst);
 
-	reply = not_authorized(auth->msg);
+	reply = btd_error_not_authorized(auth->msg);
 	dbus_message_unref(auth->msg);
 	g_dbus_send_message(auth->conn, reply);
 
diff --git a/src/adapter.c b/src/adapter.c
index 2d47856..9199f24 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -164,12 +164,6 @@ static inline DBusMessage *not_in_progress(DBusMessage *msg, const char *str)
 								"%s", str);
 }
 
-static inline DBusMessage *not_authorized(DBusMessage *msg)
-{
-	return g_dbus_create_error(msg, ERROR_INTERFACE ".NotAuthorized",
-			"Not authorized");
-}
-
 static int found_device_cmp(const struct remote_dev_info *d1,
 			const struct remote_dev_info *d2)
 {
@@ -1666,7 +1660,7 @@ static DBusMessage *cancel_device_creation(DBusConnection *conn,
 		return not_in_progress(msg, "Device creation not in progress");
 
 	if (!device_is_creating(device, sender))
-		return not_authorized(msg);
+		return btd_error_not_authorized(msg);
 
 	device_set_temporary(device, TRUE);
 
diff --git a/src/device.c b/src/device.c
index 3f94efc..cfe00c5 100644
--- a/src/device.c
+++ b/src/device.c
@@ -727,17 +727,13 @@ static DBusMessage *cancel_discover(DBusConnection *conn,
 
 	if (!dbus_message_is_method_call(device->browse->msg, DEVICE_INTERFACE,
 					"DiscoverServices"))
-		return g_dbus_create_error(msg,
-				ERROR_INTERFACE ".NotAuthorized",
-				"Not Authorized");
+		return btd_error_not_authorized(msg);
 
 	requestor = browse_request_get_requestor(device->browse);
 
 	/* only the discover requestor can cancel the inquiry process */
 	if (!requestor || !g_str_equal(requestor, sender))
-		return g_dbus_create_error(msg,
-				ERROR_INTERFACE ".NotAuthorized",
-				"Not Authorized");
+		return btd_error_not_authorized(msg);
 
 	discover_services_reply(device->browse, -ECANCELED, NULL);
 
diff --git a/src/error.c b/src/error.c
index 7c7d14e..3c09567 100644
--- a/src/error.c
+++ b/src/error.c
@@ -103,3 +103,10 @@ DBusMessage *btd_error_does_not_exist(DBusMessage *msg)
 					".DoesNotExist",
 					"Does Not Exist");
 }
+
+DBusMessage *btd_error_not_authorized(DBusMessage *msg)
+{
+	return g_dbus_create_error(msg, ERROR_INTERFACE
+					".NotAuthorized",
+					"Operation Not Authorized");
+}
diff --git a/src/error.h b/src/error.h
index 0ecbf6e..a7028bd 100644
--- a/src/error.h
+++ b/src/error.h
@@ -38,3 +38,4 @@ DBusMessage *btd_error_not_connected(DBusMessage *msg);
 DBusMessage *btd_error_not_available(DBusMessage *msg);
 DBusMessage *btd_error_in_progress(DBusMessage *msg);
 DBusMessage *btd_error_does_not_exist(DBusMessage *msg);
+DBusMessage *btd_error_not_authorized(DBusMessage *msg);
-- 
1.7.3.2


^ permalink raw reply related

* [PATCH 8/9] Add btd_error_does_not_exist()
From: Gustavo F. Padovan @ 2010-12-06 19:10 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1291662648-10651-7-git-send-email-padovan@profusion.mobi>

---
 plugins/service.c |   10 ++--------
 serial/port.c     |   12 ++----------
 serial/proxy.c    |    9 +--------
 src/adapter.c     |   13 ++++---------
 src/error.c       |    7 +++++++
 src/error.h       |    1 +
 6 files changed, 17 insertions(+), 35 deletions(-)

diff --git a/plugins/service.c b/plugins/service.c
index a71540c..406ee7d 100644
--- a/plugins/service.c
+++ b/plugins/service.c
@@ -354,12 +354,6 @@ static inline DBusMessage *not_authorized(DBusMessage *msg)
 					"Not Authorized");
 }
 
-static inline DBusMessage *does_not_exist(DBusMessage *msg)
-{
-	return g_dbus_create_error(msg, ERROR_INTERFACE ".DoesNotExist",
-					"Does Not Exist");
-}
-
 static int add_xml_record(DBusConnection *conn, const char *sender,
 			struct service_adapter *serv_adapter,
 			const char *record, dbus_uint32_t *handle)
@@ -656,7 +650,7 @@ static DBusMessage *request_authorization(DBusConnection *conn,
 
 	auth = next_pending(serv_adapter);
 	if (auth == NULL)
-		return does_not_exist(msg);
+		return btd_error_does_not_exist(msg);
 
 	if (serv_adapter->adapter)
 		adapter_get_address(serv_adapter->adapter, &src);
@@ -687,7 +681,7 @@ static DBusMessage *cancel_authorization(DBusConnection *conn,
 
 	auth = find_pending_by_sender(serv_adapter, sender);
 	if (auth == NULL)
-		return does_not_exist(msg);
+		return btd_error_does_not_exist(msg);
 
 	if (serv_adapter->adapter)
 		adapter_get_address(serv_adapter->adapter, &src);
diff --git a/serial/port.c b/serial/port.c
index b593311..add8ae0 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_DOES_NOT_EXIST	"org.bluez.Error.DoesNotExist"
 
 #define MAX_OPEN_TRIES		5
 #define OPEN_WAIT		300	/* ms. udev node creation retry wait */
@@ -235,13 +234,6 @@ void port_release_all(void)
 	g_slist_free(devices);
 }
 
-static inline DBusMessage *does_not_exist(DBusMessage *msg,
-					const char *description)
-{
-	return g_dbus_create_error(msg, ERROR_INTERFACE ".DoesNotExist",
-							"%s", description);
-}
-
 static inline DBusMessage *failed(DBusMessage *msg, const char *description)
 {
 	return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed",
@@ -497,7 +489,7 @@ static DBusMessage *port_connect(DBusConnection *conn,
 
 		channel = strtol(pattern, &endptr, 10);
 		if ((endptr && *endptr != '\0') || channel < 1 || channel > 30)
-			return does_not_exist(msg, "Does not match");
+			return btd_error_does_not_exist(msg);
 
 		port = create_port(device, NULL, channel);
 	}
@@ -538,7 +530,7 @@ static DBusMessage *port_disconnect(DBusConnection *conn,
 
 	port = find_port(device->ports, dev);
 	if (!port)
-		return does_not_exist(msg, "Port does not exist");
+		return btd_error_does_not_exist(msg);
 
 	if (!port->listener_id)
 		return failed(msg, "Not connected");
diff --git a/serial/proxy.c b/serial/proxy.c
index b2ced98..778deb0 100644
--- a/serial/proxy.c
+++ b/serial/proxy.c
@@ -131,13 +131,6 @@ static void proxy_free(struct serial_proxy *prx)
 	g_free(prx);
 }
 
-static inline DBusMessage *does_not_exist(DBusMessage *msg,
-					const char *description)
-{
-	return g_dbus_create_error(msg, ERROR_INTERFACE ".DoesNotExist",
-							"%s", description);
-}
-
 static inline DBusMessage *failed(DBusMessage *msg, const char *description)
 {
 	return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed",
@@ -1113,7 +1106,7 @@ static DBusMessage *remove_proxy(DBusConnection *conn,
 
 	l = g_slist_find_custom(adapter->proxies, path, proxy_pathcmp);
 	if (!l)
-		return does_not_exist(msg, "Invalid proxy path");
+		return btd_error_does_not_exist(msg);
 
 	prx = l->data;
 
diff --git a/src/adapter.c b/src/adapter.c
index 2aa9977..2d47856 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -1794,9 +1794,8 @@ static DBusMessage *remove_device(DBusConnection *conn, DBusMessage *msg,
 	l = g_slist_find_custom(adapter->devices,
 			path, (GCompareFunc) device_path_cmp);
 	if (!l)
-		return g_dbus_create_error(msg,
-				ERROR_INTERFACE ".DoesNotExist",
-				"Device does not exist");
+		return btd_error_does_not_exist(msg);
+
 	device = l->data;
 
 	if (device_is_temporary(device) || device_is_busy(device))
@@ -1832,9 +1831,7 @@ static DBusMessage *find_device(DBusConnection *conn, DBusMessage *msg,
 	l = g_slist_find_custom(adapter->devices,
 			address, (GCompareFunc) device_address_cmp);
 	if (!l)
-		return g_dbus_create_error(msg,
-				ERROR_INTERFACE ".DoesNotExist",
-				"Device does not exist");
+		return btd_error_does_not_exist(msg);
 
 	device = l->data;
 
@@ -1905,9 +1902,7 @@ static DBusMessage *unregister_agent(DBusConnection *conn, DBusMessage *msg,
 	name = dbus_message_get_sender(msg);
 
 	if (!adapter->agent || !agent_matches(adapter->agent, name, path))
-		return g_dbus_create_error(msg,
-					ERROR_INTERFACE ".DoesNotExist",
-					"No such agent");
+		return btd_error_does_not_exist(msg);
 
 	agent_free(adapter->agent);
 	adapter->agent = NULL;
diff --git a/src/error.c b/src/error.c
index cf3c54d..7c7d14e 100644
--- a/src/error.c
+++ b/src/error.c
@@ -96,3 +96,10 @@ DBusMessage *btd_error_not_available(DBusMessage *msg)
 					".NotAvailable",
 					"Operation currently not available");
 }
+
+DBusMessage *btd_error_does_not_exist(DBusMessage *msg)
+{
+	return g_dbus_create_error(msg, ERROR_INTERFACE
+					".DoesNotExist",
+					"Does Not Exist");
+}
diff --git a/src/error.h b/src/error.h
index 7ffd8b7..0ecbf6e 100644
--- a/src/error.h
+++ b/src/error.h
@@ -37,3 +37,4 @@ DBusMessage *btd_error_not_supported(DBusMessage *msg);
 DBusMessage *btd_error_not_connected(DBusMessage *msg);
 DBusMessage *btd_error_not_available(DBusMessage *msg);
 DBusMessage *btd_error_in_progress(DBusMessage *msg);
+DBusMessage *btd_error_does_not_exist(DBusMessage *msg);
-- 
1.7.3.2


^ permalink raw reply related

* [PATCH 7/9] Add btd_error_busy()
From: Gustavo F. Padovan @ 2010-12-06 19:10 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1291662648-10651-6-git-send-email-padovan@profusion.mobi>

---
 audio/headset.c |    4 +---
 audio/sink.c    |    6 ++----
 audio/source.c  |    3 +--
 input/device.c  |    6 ------
 src/error.c     |    6 ++++++
 src/error.h     |    1 +
 6 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/audio/headset.c b/audio/headset.c
index 0413588..8402789 100644
--- a/audio/headset.c
+++ b/audio/headset.c
@@ -1804,9 +1804,7 @@ static DBusMessage *hs_play(DBusConnection *conn, DBusMessage *msg,
 			hs->pending->msg = dbus_message_ref(msg);
 			return NULL;
 		}
-		return g_dbus_create_error(msg, ERROR_INTERFACE
-						".InProgress",
-						"Play in Progress");
+		return btd_error_busy(msg);
 	case HEADSET_STATE_PLAYING:
 		return g_dbus_create_error(msg, ERROR_INTERFACE
 						".AlreadyConnected",
diff --git a/audio/sink.c b/audio/sink.c
index e9a529b..37ba8c0 100644
--- a/audio/sink.c
+++ b/audio/sink.c
@@ -439,8 +439,7 @@ static DBusMessage *sink_connect(DBusConnection *conn,
 						"Unable to get a session");
 
 	if (sink->connect || sink->disconnect)
-		return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed",
-						"%s", strerror(EBUSY));
+		return btd_error_busy(msg);
 
 	if (sink->stream_state >= AVDTP_STATE_OPEN)
 		return g_dbus_create_error(msg, ERROR_INTERFACE
@@ -475,8 +474,7 @@ static DBusMessage *sink_disconnect(DBusConnection *conn,
 		return btd_error_not_connected(msg);
 
 	if (sink->connect || sink->disconnect)
-		return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed",
-						"%s", strerror(EBUSY));
+		return btd_error_busy(msg);
 
 	if (sink->stream_state < AVDTP_STATE_OPEN) {
 		DBusMessage *reply = dbus_message_new_method_return(msg);
diff --git a/audio/source.c b/audio/source.c
index 75f50ff..a6fd8e7 100644
--- a/audio/source.c
+++ b/audio/source.c
@@ -390,8 +390,7 @@ static DBusMessage *source_connect(DBusConnection *conn,
 						"Unable to get a session");
 
 	if (source->connect || source->disconnect)
-		return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed",
-						"%s", strerror(EBUSY));
+		return btd_error_busy(msg);
 
 	if (source->stream_state >= AVDTP_STATE_OPEN)
 		return g_dbus_create_error(msg, ERROR_INTERFACE
diff --git a/input/device.c b/input/device.c
index ac425a0..f7f96be 100644
--- a/input/device.c
+++ b/input/device.c
@@ -321,12 +321,6 @@ static inline DBusMessage *not_supported(DBusMessage *msg)
 							"Not supported");
 }
 
-static inline DBusMessage *in_progress(DBusMessage *msg)
-{
-	return g_dbus_create_error(msg, ERROR_INTERFACE ".InProgress",
-				"Device connection already in progress");
-}
-
 static inline DBusMessage *already_connected(DBusMessage *msg)
 {
 	return g_dbus_create_error(msg, ERROR_INTERFACE ".AlreadyConnected",
diff --git a/src/error.c b/src/error.c
index 3f9acd9..cf3c54d 100644
--- a/src/error.c
+++ b/src/error.c
@@ -56,6 +56,12 @@ DBusMessage *btd_error_invalid_args(DBusMessage *msg)
 					"Invalid arguments in method call");
 }
 
+DBusMessage *btd_error_busy(DBusMessage *msg)
+{
+	return g_dbus_create_error(msg, ERROR_INTERFACE ".InProgress",
+					"Operation already in progress");
+}
+
 DBusMessage *btd_error_already_exists(DBusMessage *msg)
 {
 	return g_dbus_create_error(msg,
diff --git a/src/error.h b/src/error.h
index 623860e..7ffd8b7 100644
--- a/src/error.h
+++ b/src/error.h
@@ -31,6 +31,7 @@ DBusHandlerResult error_common_reply(DBusConnection *conn, DBusMessage *msg,
 					const char *name, const char *descr);
 
 DBusMessage *btd_error_invalid_args(DBusMessage *msg);
+DBusMessage *btd_error_busy(DBusMessage *msg);
 DBusMessage *btd_error_already_exists(DBusMessage *msg);
 DBusMessage *btd_error_not_supported(DBusMessage *msg);
 DBusMessage *btd_error_not_connected(DBusMessage *msg);
-- 
1.7.3.2


^ permalink raw reply related

* [PATCH 6/9] Add btd_error_not_available()
From: Gustavo F. Padovan @ 2010-12-06 19:10 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1291662648-10651-5-git-send-email-padovan@profusion.mobi>

---
 audio/headset.c   |    9 +++------
 plugins/service.c |   16 +++-------------
 src/error.c       |    7 +++++++
 src/error.h       |    1 +
 4 files changed, 14 insertions(+), 19 deletions(-)

diff --git a/audio/headset.c b/audio/headset.c
index c3d7f82..0413588 100644
--- a/audio/headset.c
+++ b/audio/headset.c
@@ -1792,8 +1792,7 @@ static DBusMessage *hs_play(DBusConnection *conn, DBusMessage *msg,
 	if (sco_hci) {
 		error("Refusing Headset.Play() because SCO HCI routing "
 				"is enabled");
-		return g_dbus_create_error(msg, ERROR_INTERFACE ".NotAvailable",
-						"Operation not Available");
+		return btd_error_not_available(msg);
 	}
 
 	switch (hs->state) {
@@ -1838,8 +1837,7 @@ static DBusMessage *hs_get_speaker_gain(DBusConnection *conn,
 	dbus_uint16_t gain;
 
 	if (hs->state < HEADSET_STATE_CONNECTED)
-		return g_dbus_create_error(msg, ERROR_INTERFACE ".NotAvailable",
-						"Operation not Available");
+		return btd_error_not_available(msg);
 
 	reply = dbus_message_new_method_return(msg);
 	if (!reply)
@@ -1864,8 +1862,7 @@ static DBusMessage *hs_get_mic_gain(DBusConnection *conn,
 	dbus_uint16_t gain;
 
 	if (hs->state < HEADSET_STATE_CONNECTED || slc == NULL)
-		return g_dbus_create_error(msg, ERROR_INTERFACE ".NotAvailable",
-						"Operation not Available");
+		return btd_error_not_available(msg);
 
 	reply = dbus_message_new_method_return(msg);
 	if (!reply)
diff --git a/plugins/service.c b/plugins/service.c
index 12e05c1..a71540c 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 *not_available(DBusMessage *msg)
-{
-	return g_dbus_create_error(msg, ERROR_INTERFACE ".NotAvailable",
-							"Not Available");
-}
-
 static inline DBusMessage *failed(DBusMessage *msg)
 {
 	return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed", "Failed");
@@ -417,9 +411,7 @@ static DBusMessage *update_record(DBusConnection *conn, DBusMessage *msg,
 
 	if (remove_record_from_server(handle) < 0) {
 		sdp_record_free(sdp_record);
-		return g_dbus_create_error(msg,
-				ERROR_INTERFACE ".NotAvailable",
-				"Not Available");
+		return btd_error_not_available(msg);
 	}
 
 	if (serv_adapter->adapter)
@@ -463,9 +455,7 @@ static DBusMessage *update_xml_record(DBusConnection *conn,
 	user_record = find_record(serv_adapter, handle,
 				dbus_message_get_sender(msg));
 	if (!user_record)
-		return g_dbus_create_error(msg,
-				ERROR_INTERFACE ".NotAvailable",
-				"Not Available");
+		return btd_error_not_available(msg);
 
 	sdp_record = sdp_xml_parse_record(record, len);
 	if (!sdp_record) {
@@ -550,7 +540,7 @@ static DBusMessage *remove_service_record(DBusConnection *conn,
 	sender = dbus_message_get_sender(msg);
 
 	if (remove_record(conn, sender, serv_adapter, handle) < 0)
-		return not_available(msg);
+		return btd_error_not_available(msg);
 
 	return dbus_message_new_method_return(msg);
 }
diff --git a/src/error.c b/src/error.c
index 9b18842..3f9acd9 100644
--- a/src/error.c
+++ b/src/error.c
@@ -83,3 +83,10 @@ DBusMessage *btd_error_in_progress(DBusMessage *msg)
 					".InProgress",
 					"In Progress");
 }
+
+DBusMessage *btd_error_not_available(DBusMessage *msg)
+{
+	return g_dbus_create_error(msg, ERROR_INTERFACE
+					".NotAvailable",
+					"Operation currently not available");
+}
diff --git a/src/error.h b/src/error.h
index b8066ce..623860e 100644
--- a/src/error.h
+++ b/src/error.h
@@ -34,4 +34,5 @@ DBusMessage *btd_error_invalid_args(DBusMessage *msg);
 DBusMessage *btd_error_already_exists(DBusMessage *msg);
 DBusMessage *btd_error_not_supported(DBusMessage *msg);
 DBusMessage *btd_error_not_connected(DBusMessage *msg);
+DBusMessage *btd_error_not_available(DBusMessage *msg);
 DBusMessage *btd_error_in_progress(DBusMessage *msg);
-- 
1.7.3.2


^ permalink raw reply related

* [PATCH 5/9] Add btd_error_in_progress()
From: Gustavo F. Padovan @ 2010-12-06 19:10 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1291662648-10651-4-git-send-email-padovan@profusion.mobi>

---
 audio/device.c  |    3 +--
 audio/headset.c |    3 +--
 input/device.c  |    2 +-
 src/device.c    |   11 ++---------
 src/error.c     |    7 +++++++
 src/error.h     |    1 +
 6 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/audio/device.c b/audio/device.c
index 72c103b..6e005a5 100644
--- a/audio/device.c
+++ b/audio/device.c
@@ -519,8 +519,7 @@ static DBusMessage *dev_connect(DBusConnection *conn, DBusMessage *msg,
 	struct dev_priv *priv = dev->priv;
 
 	if (priv->state == AUDIO_STATE_CONNECTING)
-		return g_dbus_create_error(msg, ERROR_INTERFACE ".InProgress",
-						"Connect in Progress");
+		return btd_error_in_progress(msg);
 	else if (priv->state == AUDIO_STATE_CONNECTED)
 		return g_dbus_create_error(msg, ERROR_INTERFACE
 						".AlreadyConnected",
diff --git a/audio/headset.c b/audio/headset.c
index 7fc5afb..c3d7f82 100644
--- a/audio/headset.c
+++ b/audio/headset.c
@@ -1695,8 +1695,7 @@ static DBusMessage *hs_connect(DBusConnection *conn, DBusMessage *msg,
 	int err;
 
 	if (hs->state == HEADSET_STATE_CONNECTING)
-		return g_dbus_create_error(msg, ERROR_INTERFACE ".InProgress",
-						"Connect in Progress");
+		return btd_error_in_progress(msg);
 	else if (hs->state > HEADSET_STATE_CONNECTING)
 		return g_dbus_create_error(msg, ERROR_INTERFACE
 						".AlreadyConnected",
diff --git a/input/device.c b/input/device.c
index e09a6cf..ac425a0 100644
--- a/input/device.c
+++ b/input/device.c
@@ -946,7 +946,7 @@ static DBusMessage *input_device_connect(DBusConnection *conn,
 		return not_supported(msg);
 
 	if (iconn->pending_connect)
-		return in_progress(msg);
+		return btd_error_in_progress(msg);
 
 	if (is_connected(iconn))
 		return already_connected(msg);
diff --git a/src/device.c b/src/device.c
index 6363ed0..3f94efc 100644
--- a/src/device.c
+++ b/src/device.c
@@ -177,12 +177,6 @@ static inline DBusMessage *no_such_adapter(DBusMessage *msg)
 							"No such adapter");
 }
 
-static inline DBusMessage *in_progress(DBusMessage *msg, const char *str)
-{
-	return g_dbus_create_error(msg, ERROR_INTERFACE ".InProgress",
-								"%s", str);
-}
-
 static void browse_request_free(struct browse_req *req)
 {
 	if (req->listener_id)
@@ -611,8 +605,7 @@ static DBusMessage *discover_services(DBusConnection *conn,
 	int err;
 
 	if (device->browse)
-		return g_dbus_create_error(msg, ERROR_INTERFACE ".InProgress",
-						"Discover in progress");
+		return btd_error_in_progress(msg);
 
 	if (dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &pattern,
 						DBUS_TYPE_INVALID) == FALSE)
@@ -1991,7 +1984,7 @@ DBusMessage *device_create_bonding(struct btd_device *device,
 	ba2str(&device->bdaddr, dstaddr);
 
 	if (device->bonding)
-		return in_progress(msg, "Bonding in progress");
+		return btd_error_in_progress(msg);
 
 	/* check if a link key already exists */
 	create_name(filename, PATH_MAX, STORAGEDIR, srcaddr,
diff --git a/src/error.c b/src/error.c
index c8cccf9..9b18842 100644
--- a/src/error.c
+++ b/src/error.c
@@ -76,3 +76,10 @@ DBusMessage *btd_error_not_connected(DBusMessage *msg)
 					".NotConnected",
 					"Not Connected");
 }
+
+DBusMessage *btd_error_in_progress(DBusMessage *msg)
+{
+	return g_dbus_create_error(msg, ERROR_INTERFACE
+					".InProgress",
+					"In Progress");
+}
diff --git a/src/error.h b/src/error.h
index 7450029..b8066ce 100644
--- a/src/error.h
+++ b/src/error.h
@@ -34,3 +34,4 @@ DBusMessage *btd_error_invalid_args(DBusMessage *msg);
 DBusMessage *btd_error_already_exists(DBusMessage *msg);
 DBusMessage *btd_error_not_supported(DBusMessage *msg);
 DBusMessage *btd_error_not_connected(DBusMessage *msg);
+DBusMessage *btd_error_in_progress(DBusMessage *msg);
-- 
1.7.3.2


^ permalink raw reply related

* [PATCH 4/9] Add btd_error_not_connected()
From: Gustavo F. Padovan @ 2010-12-06 19:10 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1291662648-10651-3-git-send-email-padovan@profusion.mobi>

---
 audio/control.c |    8 ++------
 audio/device.c  |    3 +--
 audio/gateway.c |    4 +---
 audio/headset.c |   24 ++++++------------------
 audio/sink.c    |    4 +---
 audio/source.c  |    4 +---
 src/error.c     |    7 +++++++
 src/error.h     |    1 +
 8 files changed, 20 insertions(+), 35 deletions(-)

diff --git a/audio/control.c b/audio/control.c
index e710f8f..98b6682 100644
--- a/audio/control.c
+++ b/audio/control.c
@@ -1014,9 +1014,7 @@ static DBusMessage *volume_up(DBusConnection *conn, DBusMessage *msg,
 		return NULL;
 
 	if (control->state != AVCTP_STATE_CONNECTED)
-		return g_dbus_create_error(msg,
-					ERROR_INTERFACE ".NotConnected",
-					"Device not Connected");
+		return btd_error_not_connected(msg);
 
 	if (!control->target)
 		return btd_error_not_supported(msg);
@@ -1042,9 +1040,7 @@ static DBusMessage *volume_down(DBusConnection *conn, DBusMessage *msg,
 		return NULL;
 
 	if (control->state != AVCTP_STATE_CONNECTED)
-		return g_dbus_create_error(msg,
-					ERROR_INTERFACE ".NotConnected",
-					"Device not Connected");
+		return btd_error_not_connected(msg);
 
 	if (!control->target)
 		return btd_error_not_supported(msg);
diff --git a/audio/device.c b/audio/device.c
index 30ae30d..72c103b 100644
--- a/audio/device.c
+++ b/audio/device.c
@@ -562,8 +562,7 @@ static DBusMessage *dev_disconnect(DBusConnection *conn, DBusMessage *msg,
 	struct dev_priv *priv = dev->priv;
 
 	if (priv->state == AUDIO_STATE_DISCONNECTED)
-		return g_dbus_create_error(msg, ERROR_INTERFACE ".NotConnected",
-						"Not connected");
+		return btd_error_not_connected(msg);
 
 	if (priv->dc_req)
 		return dbus_message_new_method_return(msg);
diff --git a/audio/gateway.c b/audio/gateway.c
index 79847b7..93c4301 100644
--- a/audio/gateway.c
+++ b/audio/gateway.c
@@ -423,9 +423,7 @@ static DBusMessage *ag_disconnect(DBusConnection *conn, DBusMessage *msg,
 		return NULL;
 
 	if (!gw->rfcomm)
-		return g_dbus_create_error(msg, ERROR_INTERFACE
-						".NotConnected",
-						"Device not Connected");
+		return  btd_error_not_connected(msg);
 
 	gateway_close(device);
 	ba2str(&device->dst, gw_addr);
diff --git a/audio/headset.c b/audio/headset.c
index 0932477..7fc5afb 100644
--- a/audio/headset.c
+++ b/audio/headset.c
@@ -1618,9 +1618,7 @@ static DBusMessage *hs_stop(DBusConnection *conn, DBusMessage *msg,
 	DBusMessage *reply = NULL;
 
 	if (hs->state < HEADSET_STATE_PLAY_IN_PROGRESS)
-		return g_dbus_create_error(msg, ERROR_INTERFACE
-						".NotConnected",
-						"Device not Connected");
+		return btd_error_not_connected(msg);
 
 	reply = dbus_message_new_method_return(msg);
 	if (!reply)
@@ -1659,9 +1657,7 @@ static DBusMessage *hs_disconnect(DBusConnection *conn, DBusMessage *msg,
 	char hs_address[18];
 
 	if (hs->state == HEADSET_STATE_DISCONNECTED)
-		return g_dbus_create_error(msg, ERROR_INTERFACE
-						".NotConnected",
-						"Device not Connected");
+		return btd_error_not_connected(msg);
 
 	headset_shutdown(device);
 	ba2str(&device->dst, hs_address);
@@ -1737,9 +1733,7 @@ static DBusMessage *hs_ring(DBusConnection *conn, DBusMessage *msg,
 	int err;
 
 	if (hs->state < HEADSET_STATE_CONNECTED)
-		return g_dbus_create_error(msg, ERROR_INTERFACE
-						".NotConnected",
-						"Device not Connected");
+		return btd_error_not_connected(msg);
 
 	reply = dbus_message_new_method_return(msg);
 	if (!reply)
@@ -1774,9 +1768,7 @@ static DBusMessage *hs_cancel_call(DBusConnection *conn,
 	DBusMessage *reply = NULL;
 
 	if (hs->state < HEADSET_STATE_CONNECTED)
-		return g_dbus_create_error(msg, ERROR_INTERFACE
-						".NotConnected",
-						"Device not Connected");
+		return btd_error_not_connected(msg);
 
 	reply = dbus_message_new_method_return(msg);
 	if (!reply)
@@ -1808,9 +1800,7 @@ static DBusMessage *hs_play(DBusConnection *conn, DBusMessage *msg,
 	switch (hs->state) {
 	case HEADSET_STATE_DISCONNECTED:
 	case HEADSET_STATE_CONNECTING:
-		return g_dbus_create_error(msg, ERROR_INTERFACE
-						".NotConnected",
-						"Device not Connected");
+		return btd_error_not_connected(msg);
 	case HEADSET_STATE_PLAY_IN_PROGRESS:
 		if (hs->pending && hs->pending->msg == NULL) {
 			hs->pending->msg = dbus_message_ref(msg);
@@ -1901,9 +1891,7 @@ static DBusMessage *hs_set_gain(DBusConnection *conn,
 	int err;
 
 	if (hs->state < HEADSET_STATE_CONNECTED)
-		return g_dbus_create_error(msg, ERROR_INTERFACE
-						".NotConnected",
-						"Device not Connected");
+		return btd_error_not_connected(msg);
 
 	err = headset_set_gain(device, gain, type);
 	if (err < 0)
diff --git a/audio/sink.c b/audio/sink.c
index 1d350c0..e9a529b 100644
--- a/audio/sink.c
+++ b/audio/sink.c
@@ -472,9 +472,7 @@ static DBusMessage *sink_disconnect(DBusConnection *conn,
 	int err;
 
 	if (!sink->session)
-		return g_dbus_create_error(msg, ERROR_INTERFACE
-						".NotConnected",
-						"Device not Connected");
+		return btd_error_not_connected(msg);
 
 	if (sink->connect || sink->disconnect)
 		return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed",
diff --git a/audio/source.c b/audio/source.c
index 8258fcd..75f50ff 100644
--- a/audio/source.c
+++ b/audio/source.c
@@ -423,9 +423,7 @@ static DBusMessage *source_disconnect(DBusConnection *conn,
 	int err;
 
 	if (!source->session)
-		return g_dbus_create_error(msg, ERROR_INTERFACE
-						".NotConnected",
-						"Device not Connected");
+		return btd_error_not_connected(msg);
 
 	if (source->connect || source->disconnect)
 		return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed",
diff --git a/src/error.c b/src/error.c
index 0ea007a..c8cccf9 100644
--- a/src/error.c
+++ b/src/error.c
@@ -69,3 +69,10 @@ DBusMessage *btd_error_not_supported(DBusMessage *msg)
 					".NotSupported",
 					"Operation is not supported");
 }
+
+DBusMessage *btd_error_not_connected(DBusMessage *msg)
+{
+	return g_dbus_create_error(msg, ERROR_INTERFACE
+					".NotConnected",
+					"Not Connected");
+}
diff --git a/src/error.h b/src/error.h
index 71b40d9..7450029 100644
--- a/src/error.h
+++ b/src/error.h
@@ -33,3 +33,4 @@ DBusHandlerResult error_common_reply(DBusConnection *conn, DBusMessage *msg,
 DBusMessage *btd_error_invalid_args(DBusMessage *msg);
 DBusMessage *btd_error_already_exists(DBusMessage *msg);
 DBusMessage *btd_error_not_supported(DBusMessage *msg);
+DBusMessage *btd_error_not_connected(DBusMessage *msg);
-- 
1.7.3.2


^ permalink raw reply related

* [PATCH 3/9] Add btd_error_not_supported()
From: Gustavo F. Padovan @ 2010-12-06 19:10 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1291662648-10651-2-git-send-email-padovan@profusion.mobi>

---
 audio/control.c |    8 ++------
 src/device.c    |    4 +---
 src/error.c     |    7 +++++++
 src/error.h     |    1 +
 4 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/audio/control.c b/audio/control.c
index ce34eb4..e710f8f 100644
--- a/audio/control.c
+++ b/audio/control.c
@@ -1019,9 +1019,7 @@ static DBusMessage *volume_up(DBusConnection *conn, DBusMessage *msg,
 					"Device not Connected");
 
 	if (!control->target)
-		return g_dbus_create_error(msg,
-					ERROR_INTERFACE ".NotSupported",
-					"AVRCP Target role not supported");
+		return btd_error_not_supported(msg);
 
 	err = avctp_send_passthrough(control, VOL_UP_OP);
 	if (err < 0)
@@ -1049,9 +1047,7 @@ static DBusMessage *volume_down(DBusConnection *conn, DBusMessage *msg,
 					"Device not Connected");
 
 	if (!control->target)
-		return g_dbus_create_error(msg,
-					ERROR_INTERFACE ".NotSupported",
-					"AVRCP Target role not supported");
+		return btd_error_not_supported(msg);
 
 	err = avctp_send_passthrough(control, VOL_DOWN_OP);
 	if (err < 0)
diff --git a/src/device.c b/src/device.c
index 3a9cc76..6363ed0 100644
--- a/src/device.c
+++ b/src/device.c
@@ -538,9 +538,7 @@ static DBusMessage *set_blocked(DBusConnection *conn, DBusMessage *msg,
 	case 0:
 		return dbus_message_new_method_return(msg);
 	case EINVAL:
-		return g_dbus_create_error(msg,
-					ERROR_INTERFACE ".NotSupported",
-					"Kernel lacks blacklist support");
+		return btd_error_not_supported(msg);
 	default:
 		return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed",
 						"%s", strerror(-err));
diff --git a/src/error.c b/src/error.c
index 415511d..0ea007a 100644
--- a/src/error.c
+++ b/src/error.c
@@ -62,3 +62,10 @@ DBusMessage *btd_error_already_exists(DBusMessage *msg)
 				ERROR_INTERFACE ".AlreadyExists",
 				"Already Exists");
 }
+
+DBusMessage *btd_error_not_supported(DBusMessage *msg)
+{
+	return g_dbus_create_error(msg, ERROR_INTERFACE
+					".NotSupported",
+					"Operation is not supported");
+}
diff --git a/src/error.h b/src/error.h
index de40f57..71b40d9 100644
--- a/src/error.h
+++ b/src/error.h
@@ -32,3 +32,4 @@ DBusHandlerResult error_common_reply(DBusConnection *conn, DBusMessage *msg,
 
 DBusMessage *btd_error_invalid_args(DBusMessage *msg);
 DBusMessage *btd_error_already_exists(DBusMessage *msg);
+DBusMessage *btd_error_not_supported(DBusMessage *msg);
-- 
1.7.3.2


^ permalink raw reply related

* [PATCH 2/9] Add btd_error_already_exists()
From: Gustavo F. Padovan @ 2010-12-06 19:10 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1291662648-10651-1-git-send-email-padovan@profusion.mobi>

---
 audio/gateway.c  |    4 +---
 audio/media.c    |    3 +--
 network/server.c |    2 +-
 serial/proxy.c   |    3 +--
 src/adapter.c    |    8 ++------
 src/device.c     |    4 +---
 src/error.c      |    7 +++++++
 src/error.h      |    1 +
 8 files changed, 15 insertions(+), 17 deletions(-)

diff --git a/audio/gateway.c b/audio/gateway.c
index 450772b..79847b7 100644
--- a/audio/gateway.c
+++ b/audio/gateway.c
@@ -485,9 +485,7 @@ static DBusMessage *register_agent(DBusConnection *conn,
 	const char *path, *name;
 
 	if (gw->agent)
-		return g_dbus_create_error(msg,
-					ERROR_INTERFACE ".AlreadyExists",
-					"Agent already exists");
+		return btd_error_already_exists(msg);
 
 	if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &path,
 						DBUS_TYPE_INVALID))
diff --git a/audio/media.c b/audio/media.c
index bf42bdf..b893231 100644
--- a/audio/media.c
+++ b/audio/media.c
@@ -318,8 +318,7 @@ static DBusMessage *register_endpoint(DBusConnection *conn, DBusMessage *msg,
 	dbus_message_iter_next(&args);
 
 	if (media_adapter_find_endpoint(adapter, sender, path, NULL) != NULL)
-		return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed",
-				"Endpoint already registered");
+		return btd_error_already_exists(msg);
 
 	dbus_message_iter_recurse(&args, &props);
 	if (dbus_message_iter_get_arg_type(&props) != DBUS_TYPE_DICT_ENTRY)
diff --git a/network/server.c b/network/server.c
index 33da06d..60e1a81 100644
--- a/network/server.c
+++ b/network/server.c
@@ -603,7 +603,7 @@ static DBusMessage *register_server(DBusConnection *conn,
 		return failed(msg, "Invalid UUID");
 
 	if (ns->record_id)
-		return failed(msg, "Already registered");
+		return btd_error_already_exists(msg);
 
 	reply = dbus_message_new_method_return(msg);
 	if (!reply)
diff --git a/serial/proxy.c b/serial/proxy.c
index b5f5578..b2ced98 100644
--- a/serial/proxy.c
+++ b/serial/proxy.c
@@ -1056,8 +1056,7 @@ static DBusMessage *create_proxy(DBusConnection *conn,
 	if (err == -EINVAL)
 		return btd_error_invalid_args(msg);
 	else if (err == -EALREADY)
-		return g_dbus_create_error(msg, ERROR_INTERFACE ".AlreadyExist",
-						"Proxy already exists");
+		return btd_error_already_exists(msg);
 	else if (err < 0)
 		return g_dbus_create_error(msg, ERROR_INTERFACE "Failed",
 				"Proxy creation failed (%s)", strerror(-err));
diff --git a/src/adapter.c b/src/adapter.c
index 428de66..2aa9977 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -1698,9 +1698,7 @@ static DBusMessage *create_device(DBusConnection *conn,
 		return btd_error_invalid_args(msg);
 
 	if (adapter_find_device(adapter, address))
-		return g_dbus_create_error(msg,
-				ERROR_INTERFACE ".AlreadyExists",
-				"Device already exists");
+		return btd_error_already_exists(msg);
 
 	DBG("%s", address);
 
@@ -1871,9 +1869,7 @@ static DBusMessage *register_agent(DBusConnection *conn, DBusMessage *msg,
 		return NULL;
 
 	if (adapter->agent)
-		return g_dbus_create_error(msg,
-				ERROR_INTERFACE ".AlreadyExists",
-				"Agent already exists");
+		return btd_error_already_exists(msg);
 
 	cap = parse_io_capability(capability);
 	if (cap == IO_CAPABILITY_INVALID)
diff --git a/src/device.c b/src/device.c
index ab7ef93..3a9cc76 100644
--- a/src/device.c
+++ b/src/device.c
@@ -2002,9 +2002,7 @@ DBusMessage *device_create_bonding(struct btd_device *device,
 	str = textfile_caseget(filename, dstaddr);
 	if (str) {
 		free(str);
-		return g_dbus_create_error(msg,
-				ERROR_INTERFACE ".AlreadyExists",
-				"Bonding already exists");
+		return btd_error_already_exists(msg);
 	}
 
 	/* If our IO capability is NoInputNoOutput use medium security
diff --git a/src/error.c b/src/error.c
index 0807965..415511d 100644
--- a/src/error.c
+++ b/src/error.c
@@ -55,3 +55,10 @@ DBusMessage *btd_error_invalid_args(DBusMessage *msg)
 					".InvalidArguments",
 					"Invalid arguments in method call");
 }
+
+DBusMessage *btd_error_already_exists(DBusMessage *msg)
+{
+	return g_dbus_create_error(msg,
+				ERROR_INTERFACE ".AlreadyExists",
+				"Already Exists");
+}
diff --git a/src/error.h b/src/error.h
index 3a01114..de40f57 100644
--- a/src/error.h
+++ b/src/error.h
@@ -31,3 +31,4 @@ DBusHandlerResult error_common_reply(DBusConnection *conn, DBusMessage *msg,
 					const char *name, const char *descr);
 
 DBusMessage *btd_error_invalid_args(DBusMessage *msg);
+DBusMessage *btd_error_already_exists(DBusMessage *msg);
-- 
1.7.3.2


^ permalink raw reply related

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

* 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

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


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