Linux bluetooth development
 help / color / mirror / Atom feed
* [PATCH 3/3] configure.ac: build system improvements
From: Cristian Rodríguez @ 2012-12-24 18:10 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Cristian Rodríguez

Use AC_SYS_LARGEFILE so proper flags are set on systems
that require special settings for large file support

Use AC_PROG_CC_STDC, this macro picks the latest recommended
C standard flags for the compiler.
---
 configure.ac | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index 522e349..614bf1f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -5,6 +5,7 @@ AM_INIT_AUTOMAKE([foreign subdir-objects color-tests silent-rules
 					tar-pax no-dist-gzip dist-xz])
 AM_CONFIG_HEADER(config.h)
 AC_USE_SYSTEM_EXTENSIONS
+AC_SYS_LARGEFILE
 
 m4_ifdef([AM_SILENT_RULES], [AM_SILENT_RULES([yes])])
 
@@ -20,7 +21,7 @@ AC_LANG_C
 
 AC_C_RESTRICT
 
-AC_PROG_CC
+AC_PROG_CC_STDC
 AM_PROG_CC_C_O
 AC_PROG_CC_PIE
 AC_PROG_INSTALL
-- 
1.8.0.2


^ permalink raw reply related

* Re: [PATCH] lib: use %m instead of strerror
From: Marcel Holtmann @ 2012-12-24 17:13 UTC (permalink / raw)
  To: Cristian Rodríguez; +Cc: linux-bluetooth
In-Reply-To: <1356362730-16557-1-git-send-email-crrodriguez@opensuse.org>

Hi Cristian,

> Unlike strerror, %m is thread safe and we do not know
> to what kind of program libbluetooh is being linked too.
> ---
>  lib/sdp.c | 17 ++++++++---------
>  1 file changed, 8 insertions(+), 9 deletions(-)

patch has been applied.

Regards

Marcel



^ permalink raw reply

* Re: [PATCH 2/2] lib: Use SOCK_CLOEXEC where needed
From: Marcel Holtmann @ 2012-12-24 17:13 UTC (permalink / raw)
  To: Cristian Rodríguez; +Cc: linux-bluetooth
In-Reply-To: <1356367487-1610-1-git-send-email-crrodriguez@opensuse.org>

Hi Cristian,

> Calling programs might fork().. execve() and we will end
> up leaking fds.
> ---
>  lib/hci.c |  8 ++++----
>  lib/sdp.c | 13 ++++++-------
>  2 files changed, 10 insertions(+), 11 deletions(-)

you need to pay more attention to your patches.

Applying: lib: Use SOCK_CLOEXEC where needed
/data/devel/bluez/.git/rebase-apply/patch:64: trailing whitespace.
	
fatal: 1 line adds whitespace errors.
Patch failed at 0001 lib: Use SOCK_CLOEXEC where needed

I fixed this up now as well.

> diff --git a/lib/hci.c b/lib/hci.c
> index 66b2d5f..1f9058f 100644
> --- a/lib/hci.c
> +++ b/lib/hci.c
> @@ -817,7 +817,7 @@ int hci_for_each_dev(int flag, int (*func)(int dd, int dev_id, long arg),
>  	int dev_id = -1;
>  	int i, sk, err = 0;
>  
> -	sk = socket(AF_BLUETOOTH, SOCK_RAW, BTPROTO_HCI);
> +	sk = socket(AF_BLUETOOTH, SOCK_RAW|SOCK_CLOEXEC, BTPROTO_HCI);

This needs an extra space between the socket flags. And so on...

Regards

Marcel



^ permalink raw reply

* Re: [PATCH] lib: use %m instead of strerror
From: Cristian Rodríguez @ 2012-12-24 16:53 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <1356367748.29264.43.camel@aeonflux>

On Mon 24 Dec 2012 01:49:08 PM CLST, Marcel Holtmann wrote:

> how does this work on uClibc and other C libraries. Is %m these days
> generally supported?

according to google uclibc support %m .. which is currently part of 
POSIX.

^ permalink raw reply

* Re: [PATCH] lib: use %m instead of strerror
From: Marcel Holtmann @ 2012-12-24 16:49 UTC (permalink / raw)
  To: Cristian Rodríguez; +Cc: linux-bluetooth
In-Reply-To: <1356362730-16557-1-git-send-email-crrodriguez@opensuse.org>

Hi Cristian,

> Unlike strerror, %m is thread safe and we do not know
> to what kind of program libbluetooh is being linked too.
> ---
>  lib/sdp.c | 17 ++++++++---------
>  1 file changed, 8 insertions(+), 9 deletions(-)

how does this work on uClibc and other C libraries. Is %m these days
generally supported?

Regards

Marcel



^ permalink raw reply

* Re: [PATCH 1/2] Bluetooth: Fix to update EIR for uuid16 properly
From: Marcel Holtmann @ 2012-12-24 16:45 UTC (permalink / raw)
  To: Syam Sidhardhan; +Cc: Johan Hedberg, Syam Sidhardhan, linux-bluetooth
In-Reply-To: <CAFBvHieL1n6ZHe2+475KLFRAVO7-nJXPz1c8tJu2MHg_+KvX6Q@mail.gmail.com>

Hi Syam,

> >> > If we register a uuid other than uuid16, especially custom 128 bit uuid
> >> > then nothing is updated in the EIR and it was broken.
> >> >
> >> > After registering a 16 bit uuid. ex: "sdptool add SP", we can see the
> >> > uuid in the EIR as below.
> >> > < 0000: 01 52 0c f1 00 08 09 52  65 64 77 6f 6f 64 15 03  .R.....Redwood..
> >> >   0010: 01 11 32 11 2f 11 06 11  05 11 0a 11 0e 11 0c 11  ..2./...........
> >> >   0020: 1f 11 12 11 00 00 00 00  00 00 00 00 00 00 00 00  ................
> >> >   0030: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ................
> >> >   0040: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ................
> >> >   0050: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ................
> >> >   0060: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ................
> >> >   0070: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ................
> >> >   0080: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ................
> >> >   0090: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ................
> >> >   00a0: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ................
> >> >   00b0: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ................
> >> >   00c0: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ................
> >> >   00d0: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ................
> >> >   00e0: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ................
> >> >   00f0: 00 00 00 00 00                                    .....
> >> > > 0000: 04 0e 04 01 52 0c 00                              ....R..
> >> >
> >> > But after register a user defined 128 bit uuid, nothing is
> >> > updated in the EIR.
> >> >
> >> > < 0000: 01 52 0c f1 00 08 09 52  65 64 77 6f 6f 64 00 00  .R.....Redwood..
> >> >   0010: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ................
> >> >   0020: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ................
> >> >   0030: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ................
> >> >   0040: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ................
> >> >   0050: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ................
> >> >   0060: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ................
> >> >   0070: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ................
> >> >   0080: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ................
> >> >   0090: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ................
> >> >   00a0: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ................
> >> >   00b0: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ................
> >> >   00c0: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ................
> >> >   00d0: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ................
> >> >   00e0: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ................
> >> >   00f0: 00 00 00 00 00                                    .....
> >> > > 0000: 04 0e 04 01 52 0c 00                              ....R..
> >> >
> >> > With this fix, we can see the EIR is updated properly.
> >> >
> >> > Signed-off-by: Syam Sidhardhan <s.syam@samsung.com>
> >> > ---
> >> >  net/bluetooth/mgmt.c |    2 --
> >> >  1 file changed, 2 deletions(-)
> >> >
> >> > diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> >> > index f559b96..512a3f5 100644
> >> > --- a/net/bluetooth/mgmt.c
> >> > +++ b/net/bluetooth/mgmt.c
> >> > @@ -514,8 +514,6 @@ static void create_eir(struct hci_dev *hdev, u8 *data)
> >> >             u16 uuid16;
> >> >
> >> >             uuid16 = get_uuid16(uuid->uuid);
> >> > -           if (uuid16 == 0)
> >> > -                   return;
> >> >
> >> >             if (uuid16 < 0x1100)
> >> >                     continue;
> >>
> >> Nak. The bug is real and should be fixed but your fix is wrong. The
> >> right fix it to convert this return statement into a continue statement
> >> since we do still want to check for a 0 return value from get_uuid16.
> >>
> 
> Since the next statements (uuid16 < 0x1100) indirectly do this logic,
> I intentionally removed it in order to avoid duplication.
> Probably for more clarity and readability, I can do it as per your
> suggestion.
> 
> >> Along with this patch please prepare another one to increment the mgmt
> >> revision. These two should go together to upstream trees so that we can
> >> introduce a check in user space to know whether it's safe to pass
> >> non-16bit UUIDs to the kernel or not.
> >
> Ok.
> > I want a fix that introduces also support for 32-bit and 128-bit UUIDs
> > now. No paper over the hole fixing here.
> >
> 
> As per the specification, "To reduce interference, the host should try
> to minimize the amount of EIR data such that the baseband can use
> a 1-slot or 3-slot EIR packet. This is advantageous because it reduces
> interference and maximizes the probability that the EIR packet will be
> received."
> 
> Does the addition of 128-bit and 32-bit uuid decreases the probability of the
> reception of EIR packet, if any application register more of these types?

that is not the point here. If you want to put a 128-bit UUID into the
EIR data, then you should be able to. Let bluetoothd make the decision
on what to give the kernel and what not.

Regards

Marcel



^ permalink raw reply

* [PATCH 2/2] lib: Use SOCK_CLOEXEC where needed
From: Cristian Rodríguez @ 2012-12-24 16:44 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Cristian Rodríguez

Calling programs might fork().. execve() and we will end
up leaking fds.
---
 lib/hci.c |  8 ++++----
 lib/sdp.c | 13 ++++++-------
 2 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/lib/hci.c b/lib/hci.c
index 66b2d5f..1f9058f 100644
--- a/lib/hci.c
+++ b/lib/hci.c
@@ -817,7 +817,7 @@ int hci_for_each_dev(int flag, int (*func)(int dd, int dev_id, long arg),
 	int dev_id = -1;
 	int i, sk, err = 0;
 
-	sk = socket(AF_BLUETOOTH, SOCK_RAW, BTPROTO_HCI);
+	sk = socket(AF_BLUETOOTH, SOCK_RAW|SOCK_CLOEXEC, BTPROTO_HCI);
 	if (sk < 0)
 		return -1;
 
@@ -909,7 +909,7 @@ int hci_devinfo(int dev_id, struct hci_dev_info *di)
 {
 	int dd, err, ret;
 
-	dd = socket(AF_BLUETOOTH, SOCK_RAW, BTPROTO_HCI);
+	dd = socket(AF_BLUETOOTH, SOCK_RAW|SOCK_CLOEXEC, BTPROTO_HCI);
 	if (dd < 0)
 		return dd;
 
@@ -965,7 +965,7 @@ int hci_inquiry(int dev_id, int len, int nrsp, const uint8_t *lap,
 		}
 	}
 
-	dd = socket(AF_BLUETOOTH, SOCK_RAW, BTPROTO_HCI);
+	dd = socket(AF_BLUETOOTH, SOCK_RAW|SOCK_CLOEXEC, BTPROTO_HCI);
 	if (dd < 0)
 		return dd;
 
@@ -1021,7 +1021,7 @@ int hci_open_dev(int dev_id)
 	int dd, err;
 
 	/* Create HCI socket */
-	dd = socket(AF_BLUETOOTH, SOCK_RAW, BTPROTO_HCI);
+	dd = socket(AF_BLUETOOTH, SOCK_RAW|SOCK_CLOEXEC, BTPROTO_HCI);
 	if (dd < 0)
 		return dd;
 
diff --git a/lib/sdp.c b/lib/sdp.c
index e1e37ed..1ed4304 100644
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -4536,7 +4536,7 @@ static int sdp_connect_local(sdp_session_t *session)
 {
 	struct sockaddr_un sa;
 
-	session->sock = socket(PF_UNIX, SOCK_STREAM, 0);
+	session->sock = socket(PF_UNIX, SOCK_STREAM|SOCK_CLOEXEC, 0);
 	if (session->sock < 0)
 		return -1;
 	session->local = 1;
@@ -4553,19 +4553,18 @@ static int sdp_connect_l2cap(const bdaddr_t *src,
 	uint32_t flags = session->flags;
 	struct sockaddr_l2 sa;
 	int sk;
+	int sockflags = SOCK_SEQPACKET | SOCK_CLOEXEC;
+	
+	if (flags & SDP_NON_BLOCKING)
+		sockflags |= SOCK_NONBLOCK;
 
-	session->sock = socket(PF_BLUETOOTH, SOCK_SEQPACKET, BTPROTO_L2CAP);
+	session->sock = socket(PF_BLUETOOTH, sockflags, BTPROTO_L2CAP);
 	if (session->sock < 0)
 		return -1;
 	session->local = 0;
 
 	sk = session->sock;
 
-	if (flags & SDP_NON_BLOCKING) {
-		long arg = fcntl(sk, F_GETFL, 0);
-		fcntl(sk, F_SETFL, arg | O_NONBLOCK);
-	}
-
 	memset(&sa, 0, sizeof(sa));
 
 	sa.l2_family = AF_BLUETOOTH;
-- 
1.8.0.2


^ permalink raw reply related

* [PATCH] lib: use %m instead of strerror
From: Cristian Rodríguez @ 2012-12-24 15:25 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Cristian Rodríguez

Unlike strerror, %m is thread safe and we do not know
to what kind of program libbluetooh is being linked too.
---
 lib/sdp.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/lib/sdp.c b/lib/sdp.c
index a760b73..e1e37ed 100644
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -1719,7 +1719,7 @@ int sdp_send_req_w4_rsp(sdp_session_t *session, uint8_t *reqbuf,
 
 	SDPDBG("");
 	if (0 > sdp_send_req(session, reqbuf, reqsize)) {
-		SDPERR("Error sending data:%s", strerror(errno));
+		SDPERR("Error sending data:%m");
 		return -1;
 	}
 	n = sdp_read_rsp(session, rspbuf, SDP_RSP_BUFFER_SIZE);
@@ -3787,7 +3787,7 @@ int sdp_service_search_async(sdp_session_t *session, const sdp_list_t *search, u
 	reqhdr->plen = htons((t->reqsize + cstate_len) - sizeof(sdp_pdu_hdr_t));
 
 	if (sdp_send_req(session, t->reqbuf, t->reqsize + cstate_len) < 0) {
-		SDPERR("Error sendind data:%s", strerror(errno));
+		SDPERR("Error sendind data:%m");
 		t->err = errno;
 		goto end;
 	}
@@ -3898,7 +3898,7 @@ int sdp_service_attr_async(sdp_session_t *session, uint32_t handle, sdp_attrreq_
 	reqhdr->plen = htons((t->reqsize + cstate_len) - sizeof(sdp_pdu_hdr_t));
 
 	if (sdp_send_req(session, t->reqbuf, t->reqsize + cstate_len) < 0) {
-		SDPERR("Error sendind data:%s", strerror(errno));
+		SDPERR("Error sendind data:%m");
 		t->err = errno;
 		goto end;
 	}
@@ -4014,7 +4014,7 @@ int sdp_service_search_attr_async(sdp_session_t *session, const sdp_list_t *sear
 	reqhdr->plen = htons((t->reqsize + cstate_len) - sizeof(sdp_pdu_hdr_t));
 
 	if (sdp_send_req(session, t->reqbuf, t->reqsize + cstate_len) < 0) {
-		SDPERR("Error sendind data:%s", strerror(errno));
+		SDPERR("Error sendind data:%m");
 		t->err = errno;
 		goto end;
 	}
@@ -4090,8 +4090,7 @@ int sdp_process(sdp_session_t *session)
 
 	rspbuf = malloc(SDP_RSP_BUFFER_SIZE);
 	if (!rspbuf) {
-		SDPERR("Response buffer alloc failure:%s (%d)",
-				strerror(errno), errno);
+		SDPERR("Response buffer alloc failure:%m (%d)", errno);
 		return -1;
 	}
 
@@ -4105,7 +4104,7 @@ int sdp_process(sdp_session_t *session)
 
 	n = sdp_read_rsp(session, rspbuf, SDP_RSP_BUFFER_SIZE);
 	if (n < 0) {
-		SDPERR("Read response:%s (%d)", strerror(errno), errno);
+		SDPERR("Read response:%m (%d)", errno);
 		t->err = errno;
 		goto end;
 	}
@@ -4232,7 +4231,7 @@ int sdp_process(sdp_session_t *session)
 		reqhdr->plen = htons(reqsize - sizeof(sdp_pdu_hdr_t));
 
 		if (sdp_send_req(session, t->reqbuf, reqsize) < 0) {
-			SDPERR("Error sendind data:%s(%d)", strerror(errno), errno);
+			SDPERR("Error sendind data:%m(%d)", errno);
 			status = 0xffff;
 			t->err = errno;
 			goto end;
@@ -4498,7 +4497,7 @@ int sdp_general_inquiry(inquiry_info *ii, int num_dev, int duration, uint8_t *fo
 {
 	int n = hci_inquiry(-1, 10, num_dev, NULL, &ii, 0);
 	if (n < 0) {
-		SDPERR("Inquiry failed:%s", strerror(errno));
+		SDPERR("Inquiry failed:%m");
 		return -1;
 	}
 	*found = n;
-- 
1.8.0.2


^ permalink raw reply related

* [PATCH] hcidump: Add TI Logger dump support
From: chen.ganir @ 2012-12-24 13:04 UTC (permalink / raw)
  To: marcel, linux-bluetooth; +Cc: Chen Ganir

From: Chen Ganir <chen.ganir@ti.com>

Texas Instruments controllers can be configured to send the
internal firmware log through a vendor specific HCI event on
the hci transport.
This patch allows capturing those log events, and writing them
to a file, which can then be used with the latest TI Logger
application to read and show the logs.

This is usefull in case there is no other way to get the TI log
(for example, the lack of a connection to the controller Log TX
hardware line).
---
 tools/hcidump.c |   49 ++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 46 insertions(+), 3 deletions(-)

diff --git a/tools/hcidump.c b/tools/hcidump.c
index 055c8fa..64ae3a9 100644
--- a/tools/hcidump.c
+++ b/tools/hcidump.c
@@ -46,6 +46,7 @@
 #include "lib/hci_lib.h"
 
 #define SNAP_LEN	HCI_MAX_FRAME_SIZE
+#define TILOG_OUT_SIZE  3
 
 /* Modes */
 enum {
@@ -53,7 +54,8 @@ enum {
 	READ,
 	WRITE,
 	PPPDUMP,
-	AUDIO
+	AUDIO,
+	TILOGR
 };
 
 /* Default options */
@@ -102,6 +104,14 @@ struct pktlog_hdr {
 } __attribute__ ((packed));
 #define PKTLOG_HDR_SIZE (sizeof(struct pktlog_hdr))
 
+struct tilogger_pkt {
+	uint8_t type;
+	uint8_t vendor;
+	uint8_t size;
+	uint16_t opcode;
+	uint8_t	 data[0];	/* Packet Data */
+} __attribute__ ((packed));
+
 static inline int read_n(int fd, char *buf, int len)
 {
 	int t = 0, w;
@@ -148,6 +158,7 @@ static int process_frames(int dev, int sock, int fd, unsigned long flags)
 	int nfds = 0;
 	char *buf, *ctrl;
 	int len, hdr_size = HCIDUMP_HDR_SIZE;
+	struct tilogger_pkt *tp;
 
 	if (sock < 0)
 		return -1;
@@ -245,6 +256,7 @@ static int process_frames(int dev, int sock, int fd, unsigned long flags)
 
 		frm.ptr = frm.data;
 		frm.len = frm.data_len;
+		tp = frm.ptr;
 
 		switch (mode) {
 		case WRITE:
@@ -274,6 +286,23 @@ static int process_frames(int dev, int sock, int fd, unsigned long flags)
 			}
 			break;
 
+		case TILOGR:
+			if (tp->type == HCI_EVENT_PKT && tp->vendor == 0xFF &&
+							tp->opcode == 0x0400) {
+				char out[TILOG_OUT_SIZE];
+				int i, data_len = tp->size - 2;
+
+				/* Format as required by TI Log application */
+				for (i = 0; i < data_len; i++) {
+					sprintf(out,"%02X",tp->data[i]);
+					if (write_n(fd, out, 2) < 0) {
+						perror("Write error");
+						return -1;
+					}
+				}
+			}
+			break;
+
 		default:
 			/* Parse and print */
 			parse(&frm);
@@ -443,7 +472,7 @@ static int open_file(char *file, int mode, unsigned long flags)
 	struct btsnoop_hdr *hdr = (struct btsnoop_hdr *) buf;
 	int fd, len, open_flags;
 
-	if (mode == WRITE || mode == PPPDUMP || mode == AUDIO)
+	if (mode == WRITE || mode == PPPDUMP || mode == AUDIO || mode == TILOGR)
 		open_flags = O_WRONLY | O_CREAT | O_TRUNC;
 	else
 		open_flags = O_RDONLY;
@@ -490,6 +519,8 @@ static int open_file(char *file, int mode, unsigned long flags)
 			lseek(fd, 0, SEEK_SET);
 			return fd;
 		}
+	} else if (mode == TILOGR) {
+		printf ("TI Logger dump\n");
 	} else {
 		if (flags & DUMP_BTSNOOP) {
 			btsnoop_version = 1;
@@ -648,6 +679,7 @@ static void usage(void)
 	"  -a, --ascii                Dump data in ascii\n"
 	"  -x, --hex                  Dump data in hex\n"
 	"  -X, --ext                  Dump data in hex and ascii\n"
+	"  -T, --tilogger=file        Dump TI hci log data to file\n"
 	"  -R, --raw                  Dump raw data\n"
 	"  -C, --cmtp=psm             PSM for CMTP\n"
 	"  -H, --hcrp=psm             PSM for HCRP\n"
@@ -684,6 +716,7 @@ static struct option main_options[] = {
 	{ "audio",		1, 0, 'A' },
 	{ "novendor",		0, 0, 'Y' },
 	{ "nopermcheck",	0, 0, 'Z' },
+	{ "tilogger",   1, 0, 'T' },
 	{ "help",		0, 0, 'h' },
 	{ "version",		0, 0, 'v' },
 	{ 0 }
@@ -700,7 +733,7 @@ int main(int argc, char *argv[])
 	uint16_t obex_port;
 
 	while ((opt = getopt_long(argc, argv,
-				"i:l:p:m:w:r:taxXRC:H:O:P:S:D:A:YZhv",
+				"i:l:p:m:w:r:taxT:XRC:H:O:P:S:D:A:YZhv",
 				main_options, NULL)) != -1) {
 		switch(opt) {
 		case 'i':
@@ -752,6 +785,11 @@ int main(int argc, char *argv[])
 			flags |= DUMP_RAW;
 			break;
 
+		case 'T':
+			mode = TILOGR;
+			dump_file = strdup(optarg);
+			break;
+
 		case 'C':
 			set_proto(0, atoi(optarg), 0, SDP_UUID_CMTP);
 			break;
@@ -837,6 +875,11 @@ int main(int argc, char *argv[])
 		read_dump(open_file(dump_file, mode, flags));
 		break;
 
+	case TILOGR:
+		process_frames(device, open_socket(device, flags),
+				open_file(dump_file, mode, flags), flags);
+		break;
+
 	case WRITE:
 		flags |= DUMP_BTSNOOP;
 		process_frames(device, open_socket(device, flags),
-- 
1.7.10.4

^ permalink raw reply related

* Re: [PATCH 1/2] Bluetooth: Fix to update EIR for uuid16 properly
From: Syam Sidhardhan @ 2012-12-24 11:46 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: Johan Hedberg, Syam Sidhardhan, linux-bluetooth
In-Reply-To: <1356194344.29264.16.camel@aeonflux>

Hi Marcel, Johan,

On Sat, Dec 22, 2012 at 10:09 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Johan,
>
>> > If we register a uuid other than uuid16, especially custom 128 bit uuid
>> > then nothing is updated in the EIR and it was broken.
>> >
>> > After registering a 16 bit uuid. ex: "sdptool add SP", we can see the
>> > uuid in the EIR as below.
>> > < 0000: 01 52 0c f1 00 08 09 52  65 64 77 6f 6f 64 15 03  .R.....Redwood..
>> >   0010: 01 11 32 11 2f 11 06 11  05 11 0a 11 0e 11 0c 11  ..2./...........
>> >   0020: 1f 11 12 11 00 00 00 00  00 00 00 00 00 00 00 00  ................
>> >   0030: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ................
>> >   0040: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ................
>> >   0050: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ................
>> >   0060: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ................
>> >   0070: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ................
>> >   0080: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ................
>> >   0090: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ................
>> >   00a0: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ................
>> >   00b0: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ................
>> >   00c0: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ................
>> >   00d0: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ................
>> >   00e0: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ................
>> >   00f0: 00 00 00 00 00                                    .....
>> > > 0000: 04 0e 04 01 52 0c 00                              ....R..
>> >
>> > But after register a user defined 128 bit uuid, nothing is
>> > updated in the EIR.
>> >
>> > < 0000: 01 52 0c f1 00 08 09 52  65 64 77 6f 6f 64 00 00  .R.....Redwood..
>> >   0010: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ................
>> >   0020: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ................
>> >   0030: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ................
>> >   0040: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ................
>> >   0050: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ................
>> >   0060: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ................
>> >   0070: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ................
>> >   0080: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ................
>> >   0090: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ................
>> >   00a0: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ................
>> >   00b0: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ................
>> >   00c0: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ................
>> >   00d0: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ................
>> >   00e0: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ................
>> >   00f0: 00 00 00 00 00                                    .....
>> > > 0000: 04 0e 04 01 52 0c 00                              ....R..
>> >
>> > With this fix, we can see the EIR is updated properly.
>> >
>> > Signed-off-by: Syam Sidhardhan <s.syam@samsung.com>
>> > ---
>> >  net/bluetooth/mgmt.c |    2 --
>> >  1 file changed, 2 deletions(-)
>> >
>> > diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
>> > index f559b96..512a3f5 100644
>> > --- a/net/bluetooth/mgmt.c
>> > +++ b/net/bluetooth/mgmt.c
>> > @@ -514,8 +514,6 @@ static void create_eir(struct hci_dev *hdev, u8 *data)
>> >             u16 uuid16;
>> >
>> >             uuid16 = get_uuid16(uuid->uuid);
>> > -           if (uuid16 == 0)
>> > -                   return;
>> >
>> >             if (uuid16 < 0x1100)
>> >                     continue;
>>
>> Nak. The bug is real and should be fixed but your fix is wrong. The
>> right fix it to convert this return statement into a continue statement
>> since we do still want to check for a 0 return value from get_uuid16.
>>

Since the next statements (uuid16 < 0x1100) indirectly do this logic,
I intentionally removed it in order to avoid duplication.
Probably for more clarity and readability, I can do it as per your
suggestion.

>> Along with this patch please prepare another one to increment the mgmt
>> revision. These two should go together to upstream trees so that we can
>> introduce a check in user space to know whether it's safe to pass
>> non-16bit UUIDs to the kernel or not.
>
Ok.
> I want a fix that introduces also support for 32-bit and 128-bit UUIDs
> now. No paper over the hole fixing here.
>

As per the specification, "To reduce interference, the host should try
to minimize the amount of EIR data such that the baseband can use
a 1-slot or 3-slot EIR packet. This is advantageous because it reduces
interference and maximizes the probability that the EIR packet will be
received."

Does the addition of 128-bit and 32-bit uuid decreases the probability of the
reception of EIR packet, if any application register more of these types?

Regards,
Syam

^ permalink raw reply

* Re: [PATCH] Fix missing config.h includes
From: Marcel Holtmann @ 2012-12-24  3:06 UTC (permalink / raw)
  To: Cristian Rodríguez; +Cc: linux-bluetooth
In-Reply-To: <1356313961.29264.38.camel@aeonflux>

Hi Cristian,

> > ---
> >  attrib/att.c                      | 4 ++++
> >  attrib/gattrib.c                  | 4 ++++
> >  attrib/interactive.c              | 5 +++++
> >  attrib/utils.c                    | 4 ++++
> >  btio/btio.c                       | 5 +++++
> >  obexd/plugins/phonebook-tracker.c | 4 ++++
> >  profiles/gatt/manager.c           | 4 ++++
> >  profiles/health/hdp.c             | 4 ++++
> >  profiles/health/mcap.c            | 4 ++++
> >  profiles/sap/sap-u8500.c          | 4 ++++
> >  tools/btiotest.c                  | 5 +++++
> >  tools/obex-client-tool.c          | 4 ++++
> >  unit/test-gobex-apparam.c         | 4 ++++
> >  unit/test-gobex-header.c          | 4 ++++
> >  unit/test-gobex-packet.c          | 4 ++++
> >  unit/test-gobex-transfer.c        | 4 ++++
> >  unit/test-gobex.c                 | 4 ++++
> >  unit/util.c                       | 4 ++++
> >  18 files changed, 75 insertions(+)
> 
> please break this up into one patch per directory and prefix the subject
> line with that directory.

since we are just before the BlueZ 5.0 release and this seems to be a
useful fix, I split it by myself. Next time this is your job.

Regards

Marcel



^ permalink raw reply

* Re: [PATCH] Fix missing config.h includes
From: Marcel Holtmann @ 2012-12-24  1:52 UTC (permalink / raw)
  To: Cristian Rodríguez; +Cc: linux-bluetooth
In-Reply-To: <1356309150-22990-1-git-send-email-crrodriguez@opensuse.org>

Hi Christian,

> ---
>  attrib/att.c                      | 4 ++++
>  attrib/gattrib.c                  | 4 ++++
>  attrib/interactive.c              | 5 +++++
>  attrib/utils.c                    | 4 ++++
>  btio/btio.c                       | 5 +++++
>  obexd/plugins/phonebook-tracker.c | 4 ++++
>  profiles/gatt/manager.c           | 4 ++++
>  profiles/health/hdp.c             | 4 ++++
>  profiles/health/mcap.c            | 4 ++++
>  profiles/sap/sap-u8500.c          | 4 ++++
>  tools/btiotest.c                  | 5 +++++
>  tools/obex-client-tool.c          | 4 ++++
>  unit/test-gobex-apparam.c         | 4 ++++
>  unit/test-gobex-header.c          | 4 ++++
>  unit/test-gobex-packet.c          | 4 ++++
>  unit/test-gobex-transfer.c        | 4 ++++
>  unit/test-gobex.c                 | 4 ++++
>  unit/util.c                       | 4 ++++
>  18 files changed, 75 insertions(+)

please break this up into one patch per directory and prefix the subject
line with that directory.

Regards

Marcel



^ permalink raw reply

* [PATCH] Fix missing config.h includes
From: Cristian Rodríguez @ 2012-12-24  0:32 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Cristian Rodríguez

---
 attrib/att.c                      | 4 ++++
 attrib/gattrib.c                  | 4 ++++
 attrib/interactive.c              | 5 +++++
 attrib/utils.c                    | 4 ++++
 btio/btio.c                       | 5 +++++
 obexd/plugins/phonebook-tracker.c | 4 ++++
 profiles/gatt/manager.c           | 4 ++++
 profiles/health/hdp.c             | 4 ++++
 profiles/health/mcap.c            | 4 ++++
 profiles/sap/sap-u8500.c          | 4 ++++
 tools/btiotest.c                  | 5 +++++
 tools/obex-client-tool.c          | 4 ++++
 unit/test-gobex-apparam.c         | 4 ++++
 unit/test-gobex-header.c          | 4 ++++
 unit/test-gobex-packet.c          | 4 ++++
 unit/test-gobex-transfer.c        | 4 ++++
 unit/test-gobex.c                 | 4 ++++
 unit/util.c                       | 4 ++++
 18 files changed, 75 insertions(+)

diff --git a/attrib/att.c b/attrib/att.c
index 0ed4178..de11811 100644
--- a/attrib/att.c
+++ b/attrib/att.c
@@ -22,6 +22,10 @@
  *
  */
 
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
 #include <errno.h>
 #include <stdint.h>
 #include <stdlib.h>
diff --git a/attrib/gattrib.c b/attrib/gattrib.c
index 0acefc8..bf40532 100644
--- a/attrib/gattrib.c
+++ b/attrib/gattrib.c
@@ -22,6 +22,10 @@
  *
  */
 
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
 #include <stdint.h>
 #include <string.h>
 #include <glib.h>
diff --git a/attrib/interactive.c b/attrib/interactive.c
index 1ea35cd..51f620a 100644
--- a/attrib/interactive.c
+++ b/attrib/interactive.c
@@ -20,6 +20,11 @@
  *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
  *
  */
+
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
 #include <string.h>
 #include <stdlib.h>
 #include <errno.h>
diff --git a/attrib/utils.c b/attrib/utils.c
index 08365bd..43244c2 100644
--- a/attrib/utils.c
+++ b/attrib/utils.c
@@ -21,6 +21,10 @@
  *
  */
 
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
 #include <stdlib.h>
 #include <glib.h>
 
diff --git a/btio/btio.c b/btio/btio.c
index 44c2f9b..bbf1208 100644
--- a/btio/btio.c
+++ b/btio/btio.c
@@ -21,6 +21,11 @@
  *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
  *
  */
+
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
 #include <stdarg.h>
 #include <stdlib.h>
 #include <unistd.h>
diff --git a/obexd/plugins/phonebook-tracker.c b/obexd/plugins/phonebook-tracker.c
index 2fd7ba1..433f95a 100644
--- a/obexd/plugins/phonebook-tracker.c
+++ b/obexd/plugins/phonebook-tracker.c
@@ -20,6 +20,10 @@
  *
  */
 
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
 #include <string.h>
 #include <stdlib.h>
 #include <time.h>
diff --git a/profiles/gatt/manager.c b/profiles/gatt/manager.c
index ce0ed91..151d60f 100644
--- a/profiles/gatt/manager.c
+++ b/profiles/gatt/manager.c
@@ -20,6 +20,10 @@
  *
  */
 
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
 #include <glib.h>
 #include <errno.h>
 #include <stdbool.h>
diff --git a/profiles/health/hdp.c b/profiles/health/hdp.c
index bf8f82f..e637420 100644
--- a/profiles/health/hdp.c
+++ b/profiles/health/hdp.c
@@ -20,6 +20,10 @@
  *
  */
 
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
 #include <stdlib.h>
 #include <stdint.h>
 #include <stdbool.h>
diff --git a/profiles/health/mcap.c b/profiles/health/mcap.c
index 466d266..f08a7fa 100644
--- a/profiles/health/mcap.c
+++ b/profiles/health/mcap.c
@@ -20,6 +20,10 @@
  *
  */
 
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
 #include <netinet/in.h>
 #include <stdlib.h>
 #include <errno.h>
diff --git a/profiles/sap/sap-u8500.c b/profiles/sap/sap-u8500.c
index b1aee57..39169a0 100644
--- a/profiles/sap/sap-u8500.c
+++ b/profiles/sap/sap-u8500.c
@@ -22,6 +22,10 @@
  *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
  */
 
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
 #include <stdio.h>
 #include <stdlib.h>
 #include <fcntl.h>
diff --git a/tools/btiotest.c b/tools/btiotest.c
index 4b170ac..7a77bb7 100644
--- a/tools/btiotest.c
+++ b/tools/btiotest.c
@@ -21,6 +21,11 @@
  *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
  *
  */
+
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
 #include <stdio.h>
 #include <stdlib.h>
 #include <stdint.h>
diff --git a/tools/obex-client-tool.c b/tools/obex-client-tool.c
index 8488a20..b7220fa 100644
--- a/tools/obex-client-tool.c
+++ b/tools/obex-client-tool.c
@@ -19,6 +19,10 @@
  *
  */
 
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
 #include <sys/types.h>
 #include <sys/socket.h>
 #include <fcntl.h>
diff --git a/unit/test-gobex-apparam.c b/unit/test-gobex-apparam.c
index f232380..1c86274 100644
--- a/unit/test-gobex-apparam.c
+++ b/unit/test-gobex-apparam.c
@@ -19,6 +19,10 @@
  *
  */
 
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
 #include <stdint.h>
 #include <string.h>
 
diff --git a/unit/test-gobex-header.c b/unit/test-gobex-header.c
index c933a01..2a198c6 100644
--- a/unit/test-gobex-header.c
+++ b/unit/test-gobex-header.c
@@ -19,6 +19,10 @@
  *
  */
 
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
 #include <stdint.h>
 #include <string.h>
 
diff --git a/unit/test-gobex-packet.c b/unit/test-gobex-packet.c
index 39650f9..b40dc66 100644
--- a/unit/test-gobex-packet.c
+++ b/unit/test-gobex-packet.c
@@ -19,6 +19,10 @@
  *
  */
 
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
 #include <stdint.h>
 #include <string.h>
 #include <errno.h>
diff --git a/unit/test-gobex-transfer.c b/unit/test-gobex-transfer.c
index f013617..a8578fd 100644
--- a/unit/test-gobex-transfer.c
+++ b/unit/test-gobex-transfer.c
@@ -19,6 +19,10 @@
  *
  */
 
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
 #include <stdlib.h>
 #include <sys/types.h>
 #include <sys/socket.h>
diff --git a/unit/test-gobex.c b/unit/test-gobex.c
index 2075f4a..8beaa44 100644
--- a/unit/test-gobex.c
+++ b/unit/test-gobex.c
@@ -19,6 +19,10 @@
  *
  */
 
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
 #include <stdlib.h>
 #include <sys/types.h>
 #include <sys/socket.h>
diff --git a/unit/util.c b/unit/util.c
index a591ab1..c76acdf 100644
--- a/unit/util.c
+++ b/unit/util.c
@@ -19,6 +19,10 @@
  *
  */
 
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
 #include <sys/types.h>
 #include <sys/socket.h>
 #include <unistd.h>
-- 
1.8.0.2


^ permalink raw reply related

* Re: [PATCH BlueZ 1/8] gdbus: Introduce G_DBUS_METHOD_FLAG_EXPERIMENTAL
From: Luiz Augusto von Dentz @ 2012-12-24  0:12 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <1356305714.29264.37.camel@aeonflux>

HI Marcel,

On Mon, Dec 24, 2012 at 1:35 AM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Luiz,
>
>> >> This flag can be used to mark methods as experimental, the marked
>> >> methods with this flag can be enabled by setting the environment variable
>> >> GDBUS_EXPERIMENTAL=1
>> >> ---
>> >>  gdbus/gdbus.h  | 21 ++++++++++++++++++---
>> >>  gdbus/object.c | 17 +++++++++++++++++
>> >>  2 files changed, 35 insertions(+), 3 deletions(-)
>> >>
>> >> diff --git a/gdbus/gdbus.h b/gdbus/gdbus.h
>> >> index 0e5c012..00fbb1c 100644
>> >> --- a/gdbus/gdbus.h
>> >> +++ b/gdbus/gdbus.h
>> >> @@ -89,9 +89,10 @@ typedef void (* GDBusSecurityFunction) (DBusConnection *connection,
>> >>                                               GDBusPendingReply pending);
>> >>
>> >>  enum GDBusMethodFlags {
>> >> -     G_DBUS_METHOD_FLAG_DEPRECATED = (1 << 0),
>> >> -     G_DBUS_METHOD_FLAG_NOREPLY    = (1 << 1),
>> >> -     G_DBUS_METHOD_FLAG_ASYNC      = (1 << 2),
>> >> +     G_DBUS_METHOD_FLAG_DEPRECATED   = (1 << 0),
>> >> +     G_DBUS_METHOD_FLAG_NOREPLY      = (1 << 1),
>> >> +     G_DBUS_METHOD_FLAG_ASYNC        = (1 << 2),
>> >> +     G_DBUS_METHOD_FLAG_EXPERIMENTAL = (1 << 3),
>> >>  };
>> >>
>> >>  enum GDBusSignalFlags {
>> >> @@ -173,6 +174,20 @@ struct GDBusSecurityTable {
>> >>       .function = _function, \
>> >>       .flags = G_DBUS_METHOD_FLAG_ASYNC | G_DBUS_METHOD_FLAG_DEPRECATED
>> >>
>> >> +#define GDBUS_EXPERIMENTAL_METHOD(_name, _in_args, _out_args, _function) \
>> >> +     .name = _name, \
>> >> +     .in_args = _in_args, \
>> >> +     .out_args = _out_args, \
>> >> +     .function = _function, \
>> >> +     .flags = G_DBUS_METHOD_FLAG_EXPERIMENTAL
>> >> +
>> >> +#define GDBUS_EXPERIMENTAL_ASYNC_METHOD(_name, _in_args, _out_args, _function) \
>> >> +     .name = _name, \
>> >> +     .in_args = _in_args, \
>> >> +     .out_args = _out_args, \
>> >> +     .function = _function, \
>> >> +     .flags = G_DBUS_METHOD_FLAG_ASYNC | G_DBUS_METHOD_FLAG_EXPERIMENTAL
>> >> +
>> >>  #define GDBUS_NOREPLY_METHOD(_name, _in_args, _out_args, _function) \
>> >>       .name = _name, \
>> >>       .in_args = _in_args, \
>> >> diff --git a/gdbus/object.c b/gdbus/object.c
>> >> index 776d35e..30dbbc2 100644
>> >> --- a/gdbus/object.c
>> >> +++ b/gdbus/object.c
>> >> @@ -129,6 +129,14 @@ static void generate_interface_xml(GString *gstr, struct interface_data *iface)
>> >>                                               G_DBUS_METHOD_FLAG_DEPRECATED;
>> >>               gboolean noreply = method->flags &
>> >>                                               G_DBUS_METHOD_FLAG_NOREPLY;
>> >> +             gboolean experimental = method->flags &
>> >> +                                     G_DBUS_METHOD_FLAG_EXPERIMENTAL;
>> >> +
>> >> +             if (experimental) {
>> >> +                     const char *env = g_getenv("GDBUS_EXPERIMENTAL");
>> >> +                     if (g_strcmp0(env, "1") != 0)
>> >> +                             continue;
>> >> +             }
>> >
>> > actually since this is a library, doing it this way is a bad idea.
>>
>> I thought it was a common practice to use environment variables with
>> libraries to change certain defaults, glib does that with things like
>> G_SLICE=always-malloc, and it is quite convenient since you can change
>> easily without recompiling.
>
> GLib does this, but we never did this. GAtChat, GDHCP, GWeb etc.
> provided a function to enable it. The hooking up to environment variable
> is then the responsibility of the main program.

GObex does have it on environment variables and I can even enable them
while running make check so if a test fail I can debug like the daemon
itself.

>> > Lets do something like g_dbus_enable_experimental(DBusConnection)
>>
>> But this is not really per connection, anyway doing so you have to
>> handle this directly on the application code which IMO is not as
>> convenient.
>
> Making this per connection would be pretty convenient if you are
> connected to more than one bus.

Except that we don't actually change during runtime to be able to use
the connection and it would probably confuse applications that already
read the introspection data if we do so.



--
Luiz Augusto von Dentz

^ permalink raw reply

* Re: [PATCH BlueZ 1/8] gdbus: Introduce G_DBUS_METHOD_FLAG_EXPERIMENTAL
From: Marcel Holtmann @ 2012-12-23 23:35 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <CABBYNZJCA6ompsqzjPcBT-=z20QgrL_ynGR2aDsN=q3rknt=vw@mail.gmail.com>

Hi Luiz,

> >> This flag can be used to mark methods as experimental, the marked
> >> methods with this flag can be enabled by setting the environment variable
> >> GDBUS_EXPERIMENTAL=1
> >> ---
> >>  gdbus/gdbus.h  | 21 ++++++++++++++++++---
> >>  gdbus/object.c | 17 +++++++++++++++++
> >>  2 files changed, 35 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/gdbus/gdbus.h b/gdbus/gdbus.h
> >> index 0e5c012..00fbb1c 100644
> >> --- a/gdbus/gdbus.h
> >> +++ b/gdbus/gdbus.h
> >> @@ -89,9 +89,10 @@ typedef void (* GDBusSecurityFunction) (DBusConnection *connection,
> >>                                               GDBusPendingReply pending);
> >>
> >>  enum GDBusMethodFlags {
> >> -     G_DBUS_METHOD_FLAG_DEPRECATED = (1 << 0),
> >> -     G_DBUS_METHOD_FLAG_NOREPLY    = (1 << 1),
> >> -     G_DBUS_METHOD_FLAG_ASYNC      = (1 << 2),
> >> +     G_DBUS_METHOD_FLAG_DEPRECATED   = (1 << 0),
> >> +     G_DBUS_METHOD_FLAG_NOREPLY      = (1 << 1),
> >> +     G_DBUS_METHOD_FLAG_ASYNC        = (1 << 2),
> >> +     G_DBUS_METHOD_FLAG_EXPERIMENTAL = (1 << 3),
> >>  };
> >>
> >>  enum GDBusSignalFlags {
> >> @@ -173,6 +174,20 @@ struct GDBusSecurityTable {
> >>       .function = _function, \
> >>       .flags = G_DBUS_METHOD_FLAG_ASYNC | G_DBUS_METHOD_FLAG_DEPRECATED
> >>
> >> +#define GDBUS_EXPERIMENTAL_METHOD(_name, _in_args, _out_args, _function) \
> >> +     .name = _name, \
> >> +     .in_args = _in_args, \
> >> +     .out_args = _out_args, \
> >> +     .function = _function, \
> >> +     .flags = G_DBUS_METHOD_FLAG_EXPERIMENTAL
> >> +
> >> +#define GDBUS_EXPERIMENTAL_ASYNC_METHOD(_name, _in_args, _out_args, _function) \
> >> +     .name = _name, \
> >> +     .in_args = _in_args, \
> >> +     .out_args = _out_args, \
> >> +     .function = _function, \
> >> +     .flags = G_DBUS_METHOD_FLAG_ASYNC | G_DBUS_METHOD_FLAG_EXPERIMENTAL
> >> +
> >>  #define GDBUS_NOREPLY_METHOD(_name, _in_args, _out_args, _function) \
> >>       .name = _name, \
> >>       .in_args = _in_args, \
> >> diff --git a/gdbus/object.c b/gdbus/object.c
> >> index 776d35e..30dbbc2 100644
> >> --- a/gdbus/object.c
> >> +++ b/gdbus/object.c
> >> @@ -129,6 +129,14 @@ static void generate_interface_xml(GString *gstr, struct interface_data *iface)
> >>                                               G_DBUS_METHOD_FLAG_DEPRECATED;
> >>               gboolean noreply = method->flags &
> >>                                               G_DBUS_METHOD_FLAG_NOREPLY;
> >> +             gboolean experimental = method->flags &
> >> +                                     G_DBUS_METHOD_FLAG_EXPERIMENTAL;
> >> +
> >> +             if (experimental) {
> >> +                     const char *env = g_getenv("GDBUS_EXPERIMENTAL");
> >> +                     if (g_strcmp0(env, "1") != 0)
> >> +                             continue;
> >> +             }
> >
> > actually since this is a library, doing it this way is a bad idea.
> 
> I thought it was a common practice to use environment variables with
> libraries to change certain defaults, glib does that with things like
> G_SLICE=always-malloc, and it is quite convenient since you can change
> easily without recompiling.

GLib does this, but we never did this. GAtChat, GDHCP, GWeb etc.
provided a function to enable it. The hooking up to environment variable
is then the responsibility of the main program.

> > Lets do something like g_dbus_enable_experimental(DBusConnection)
> 
> But this is not really per connection, anyway doing so you have to
> handle this directly on the application code which IMO is not as
> convenient.

Making this per connection would be pretty convenient if you are
connected to more than one bus.

Regards

Marcel



^ permalink raw reply

* Re: [PATCH BlueZ 1/8] gdbus: Introduce G_DBUS_METHOD_FLAG_EXPERIMENTAL
From: Luiz Augusto von Dentz @ 2012-12-23 22:06 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <1356296297.29264.34.camel@aeonflux>

Hi Marcel,

On Sun, Dec 23, 2012 at 10:58 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Luiz,
>
>> This flag can be used to mark methods as experimental, the marked
>> methods with this flag can be enabled by setting the environment variable
>> GDBUS_EXPERIMENTAL=1
>> ---
>>  gdbus/gdbus.h  | 21 ++++++++++++++++++---
>>  gdbus/object.c | 17 +++++++++++++++++
>>  2 files changed, 35 insertions(+), 3 deletions(-)
>>
>> diff --git a/gdbus/gdbus.h b/gdbus/gdbus.h
>> index 0e5c012..00fbb1c 100644
>> --- a/gdbus/gdbus.h
>> +++ b/gdbus/gdbus.h
>> @@ -89,9 +89,10 @@ typedef void (* GDBusSecurityFunction) (DBusConnection *connection,
>>                                               GDBusPendingReply pending);
>>
>>  enum GDBusMethodFlags {
>> -     G_DBUS_METHOD_FLAG_DEPRECATED = (1 << 0),
>> -     G_DBUS_METHOD_FLAG_NOREPLY    = (1 << 1),
>> -     G_DBUS_METHOD_FLAG_ASYNC      = (1 << 2),
>> +     G_DBUS_METHOD_FLAG_DEPRECATED   = (1 << 0),
>> +     G_DBUS_METHOD_FLAG_NOREPLY      = (1 << 1),
>> +     G_DBUS_METHOD_FLAG_ASYNC        = (1 << 2),
>> +     G_DBUS_METHOD_FLAG_EXPERIMENTAL = (1 << 3),
>>  };
>>
>>  enum GDBusSignalFlags {
>> @@ -173,6 +174,20 @@ struct GDBusSecurityTable {
>>       .function = _function, \
>>       .flags = G_DBUS_METHOD_FLAG_ASYNC | G_DBUS_METHOD_FLAG_DEPRECATED
>>
>> +#define GDBUS_EXPERIMENTAL_METHOD(_name, _in_args, _out_args, _function) \
>> +     .name = _name, \
>> +     .in_args = _in_args, \
>> +     .out_args = _out_args, \
>> +     .function = _function, \
>> +     .flags = G_DBUS_METHOD_FLAG_EXPERIMENTAL
>> +
>> +#define GDBUS_EXPERIMENTAL_ASYNC_METHOD(_name, _in_args, _out_args, _function) \
>> +     .name = _name, \
>> +     .in_args = _in_args, \
>> +     .out_args = _out_args, \
>> +     .function = _function, \
>> +     .flags = G_DBUS_METHOD_FLAG_ASYNC | G_DBUS_METHOD_FLAG_EXPERIMENTAL
>> +
>>  #define GDBUS_NOREPLY_METHOD(_name, _in_args, _out_args, _function) \
>>       .name = _name, \
>>       .in_args = _in_args, \
>> diff --git a/gdbus/object.c b/gdbus/object.c
>> index 776d35e..30dbbc2 100644
>> --- a/gdbus/object.c
>> +++ b/gdbus/object.c
>> @@ -129,6 +129,14 @@ static void generate_interface_xml(GString *gstr, struct interface_data *iface)
>>                                               G_DBUS_METHOD_FLAG_DEPRECATED;
>>               gboolean noreply = method->flags &
>>                                               G_DBUS_METHOD_FLAG_NOREPLY;
>> +             gboolean experimental = method->flags &
>> +                                     G_DBUS_METHOD_FLAG_EXPERIMENTAL;
>> +
>> +             if (experimental) {
>> +                     const char *env = g_getenv("GDBUS_EXPERIMENTAL");
>> +                     if (g_strcmp0(env, "1") != 0)
>> +                             continue;
>> +             }
>
> actually since this is a library, doing it this way is a bad idea.

I thought it was a common practice to use environment variables with
libraries to change certain defaults, glib does that with things like
G_SLICE=always-malloc, and it is quite convenient since you can change
easily without recompiling.

> Lets do something like g_dbus_enable_experimental(DBusConnection)

But this is not really per connection, anyway doing so you have to
handle this directly on the application code which IMO is not as
convenient.

--
Luiz Augusto von Dentz

^ permalink raw reply

* Re: [RFC/RFT] rtk_btusb: Bluetooth driver for Realtek RTL8723AE combo device
From: Oliver Neukum @ 2012-12-23 21:00 UTC (permalink / raw)
  To: Larry Finger
  Cc: linville, Marcel Holtmann, Gustavo Padovan, Johan Hedberg,
	linux-wireless, linux-bluetooth, Champion Chen
In-Reply-To: <1356058371-17152-1-git-send-email-Larry.Finger@lwfinger.net>

On Thursday 20 December 2012 20:52:51 Larry Finger wrote:
> This new driver works with the RTL8723AE wireless/BT combo device. The
> corresponding firmware has been submitted to linux-firmware.
> 
> Signed-off-by: Champion Chen <champion_chen@realsil.com.cn>
> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
> ---
>  drivers/bluetooth/Kconfig     |   10 +
>  drivers/bluetooth/Makefile    |    1 +
>  drivers/bluetooth/rtk_btusb.c | 1649 +++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 1660 insertions(+)
>  create mode 100644 drivers/bluetooth/rtk_btusb.c
> 
> diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
> index e9f203e..efd3766 100644
> --- a/drivers/bluetooth/Kconfig
> +++ b/drivers/bluetooth/Kconfig
> @@ -241,4 +241,14 @@ config BT_WILINK
>  
>  	  Say Y here to compile support for Texas Instrument's WiLink7 driver
>  	  into the kernel or say M to compile it as module.
> +
> +config BT_RTKUSB
> +	tristate "Realtek BT driver for RTL8723AE"
> +	select FW_LOADER
> +	help
> +	  This enables the Bluetooth driver for the Realtek RTL8723AE Wifi/BT
> +	  combo device.
> +
> +	  Say Y here to compile support for these devices into the kernel
> +	  or say M to build it as a module.
>  endmenu
> diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
> index 4afae20..167ccc0 100644
> --- a/drivers/bluetooth/Makefile
> +++ b/drivers/bluetooth/Makefile
> @@ -19,6 +19,7 @@ obj-$(CONFIG_BT_ATH3K)		+= ath3k.o
>  obj-$(CONFIG_BT_MRVL)		+= btmrvl.o
>  obj-$(CONFIG_BT_MRVL_SDIO)	+= btmrvl_sdio.o
>  obj-$(CONFIG_BT_WILINK)		+= btwilink.o
> +obj-$(CONFIG_BT_RTKUSB)		+= rtk_btusb.o
>  
>  btmrvl-y			:= btmrvl_main.o
>  btmrvl-$(CONFIG_DEBUG_FS)	+= btmrvl_debugfs.o
> diff --git a/drivers/bluetooth/rtk_btusb.c b/drivers/bluetooth/rtk_btusb.c
> new file mode 100644
> index 0000000..31c128a
> --- /dev/null
> +++ b/drivers/bluetooth/rtk_btusb.c
> @@ -0,0 +1,1650 @@
> +/*
> + *
> + *  Realtek Bluetooth USB driver
> + *
> + *  Copyright (C) 2012-2015  Edward Bian <edward_bian@realsil.com.cn>
> + *
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation; either version 2 of the License, or
> + *  (at your option) any later version.
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with this program; if not, write to the Free Software
> + *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
> + *
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/slab.h>
> +#include <linux/types.h>
> +#include <linux/sched.h>
> +#include <linux/errno.h>
> +#include <linux/skbuff.h>
> +#include <linux/usb.h>
> +#include <linux/completion.h>
> +#include <net/bluetooth/bluetooth.h>
> +#include <net/bluetooth/hci_core.h>
> +
> +#define VERSION "0.8"
> +
> +static struct usb_driv> diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
> index e9f203e..efd3766 100644

> +static struct usb_driver btusb_driver;
> +#if 1
> +#define RTKBT_DBG(fmt, arg...) pr_info("rtk_btusb: " fmt "\n" , ## arg)

No new debug macros please.










> +
> +static int btusb_open(struct hci_dev *hdev)
> +{
> +	struct btusb_data *data = GET_DRV_DATA(hdev);
> +	int err;
> +
> +	err = usb_autopm_get_interface(data->intf);
> +	if (err < 0)
> +		return err;
> +
> +	data->intf->needs_remote_wakeup = 1;
> +	RTKBT_DBG("%s start pm_usage_cnt(0x%x)", __func__,
> +		  atomic_read(&(data->intf->pm_usage_cnt)));
> +
> +	/*******************************/
> +	if (0 == atomic_read(&hdev->promisc)) {
> +		RTKBT_DBG("btusb_open hdev->promisc == 0");
> +		err = -1;

This makes no sense

> +	}
> +	err = download_patch(data->intf);
> +	if (err < 0)
> +		goto failed;

On every open?

> +	/*******************************/
> +
> +	if (test_and_set_bit(HCI_RUNNING, &hdev->flags))
> +		goto done;
> +
> +	if (test_and_set_bit(BTUSB_INTR_RUNNING, &data->flags))
> +		goto done;
> +
> +	err = btusb_submit_intr_urb(hdev, GFP_KERNEL);
> +	if (err < 0)
> +		goto failed;
> +
> +	err = btusb_submit_bulk_urb(hdev, GFP_KERNEL);
> +	if (err < 0) {
> +		mdelay(URB_CANCELING_DELAY_MS);      /*  Added by Realtek */
> +		usb_kill_anchored_urbs(&data->intr_anchor);
> +		goto failed;
> +	}
> +
> +	set_bit(BTUSB_BULK_RUNNING, &data->flags);
> +	btusb_submit_bulk_urb(hdev, GFP_KERNEL);
> +
> +done:
> +	usb_autopm_put_interface(data->intf);
> +	RTKBT_DBG("%s end  pm_usage_cnt(0x%x)", __func__,
> +		  atomic_read(&(data->intf->pm_usage_cnt)));
> +
> +	return 0;
> +
> +failed:
> +	clear_bit(BTUSB_INTR_RUNNING, &data->flags);
> +	clear_bit(HCI_RUNNING, &hdev->flags);
> +	usb_autopm_put_interface(data->intf);
> +	RTKBT_DBG("%s failed  pm_usage_cnt(0x%x)", __func__,
> +		  atomic_read(&(data->intf->pm_usage_cnt)));
> +	return err;
> +}
> +
> +static void btusb_stop_traffic(struct btusb_data *data)
> +{
> +	mdelay(URB_CANCELING_DELAY_MS);    /*  Added by Realtek */
> +	usb_kill_anchored_urbs(&data->intr_anchor);
> +	usb_kill_anchored_urbs(&data->bulk_anchor);
> +	usb_kill_anchored_urbs(&data->isoc_anchor);
> +}
> +
> +static int btusb_close(struct hci_dev *hdev)
> +{
> +	struct btusb_data *data = GET_DRV_DATA(hdev);
> +	int i, err;
> +
> +	if (!test_and_clear_bit(HCI_RUNNING, &hdev->flags))
> +		return 0;
> +
> +	RTKBT_DBG("btusb_close");
> +	/*******************************/
> +	for (i = 0; i < NUM_REASSEMBLY; i++) {
> +		if (hdev->reassembly[i])	{
> +			kfree_skb(hdev->reassembly[i]);
> +			hdev->reassembly[i] = NULL;
> +			RTKBT_DBG("%s free ressembly i =%d", __func__, i);
> +		}
> +	}
> +	/*******************************/
> +	cancel_work_sync(&data->work);
> +	cancel_work_sync(&data->waker);
> +
> +	clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
> +	clear_bit(BTUSB_BULK_RUNNING, &data->flags);
> +	clear_bit(BTUSB_INTR_RUNNING, &data->flags);
> +
> +	btusb_stop_traffic(data);
> +	err = usb_autopm_get_interface(data->intf);
> +	if (err < 0)
> +		goto failed;
> +
> +	data->intf->needs_remote_wakeup = 0;
> +	usb_autopm_put_interface(data->intf);
> +
> +failed:
> +	mdelay(URB_CANCELING_DELAY_MS);     /*  Added by Realtek */

This makes no sense. Those URBs never went over the wire.

> +	usb_scuttle_anchored_urbs(&data->deferred);
> +	return 0;
> +}
> +
> +static int btusb_flush(struct hci_dev *hdev)
> +{
> +	struct btusb_data *data = GET_DRV_DATA(hdev);
> +
> +	RTKBT_DBG("%s add delay ", __func__);
> +	mdelay(URB_CANCELING_DELAY_MS);     /*  Added by Realtek */
> +	usb_kill_anchored_urbs(&data->tx_anchor);
> +
> +	return 0;
> +}
> +


> +
> +static void btusb_work(struct work_struct *work)
> +{
> +	struct btusb_data *data = container_of(work, struct btusb_data, work);
> +	struct hci_dev *hdev = data->hdev;
> +	int err;
> +
> +	if (hdev->conn_hash.sco_num > 0) {
> +		if (!test_bit(BTUSB_DID_ISO_RESUME, &data->flags)) {
> +			err = usb_autopm_get_interface(data->isoc ? data->isoc :
> +						       data->intf);
> +			if (err < 0) {
> +				clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
> +				/*  Delay added by Realtek */
> +				mdelay(URB_CANCELING_DELAY_MS);
> +				usb_kill_anchored_urbs(&data->isoc_anchor);
> +				return;
> +			}
> +
> +			set_bit(BTUSB_DID_ISO_RESUME, &data->flags);
> +		}
> +		if (data->isoc_altsetting != 2) {
> +			clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
> +			mdelay(URB_CANCELING_DELAY_MS);  /*  Added by Realtek */
> +			usb_kill_anchored_urbs(&data->isoc_anchor);
> +
> +			if (__set_isoc_interface(hdev, 2) < 0)
> +				return;
> +		}
> +
> +		if (!test_and_set_bit(BTUSB_ISOC_RUNNING, &data->flags)) {
> +			if (btusb_submit_isoc_urb(hdev, GFP_KERNEL) < 0)
> +				clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
> +			else
> +				btusb_submit_isoc_urb(hdev, GFP_KERNEL);
> +		}
> +	} else {
> +		clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
> +		mdelay(URB_CANCELING_DELAY_MS);      /*  Added by Realtek */
> +		usb_kill_anchored_urbs(&data->isoc_anchor);
> +
> +		__set_isoc_interface(hdev, 0);
> +		if (test_and_clear_bit(BTUSB_DID_ISO_RESUME, &data->flags))
> +			usb_autopm_put_interface(data->isoc ? data->isoc :
> +						 data->intf);
> +	}
> +}

> +static int btusb_probe(struct usb_interface *intf,
> +				const struct usb_device_id *id)
> +{
> +	struct usb_endpoint_descriptor *ep_desc;
> +	struct btusb_data *data;
> +	struct hci_dev *hdev;
> +	int i, err, flag1, flag2;
> +	struct usb_device *udev;
> +	udev = interface_to_usbdev(intf);
> +
> +	/* interface numbers are hardcoded in the spec */
> +	if (intf->cur_altsetting->desc.bInterfaceNumber != 0)
> +		return -ENODEV;
> +
> +	/*******************************/
> +	flag1 = device_can_wakeup(&udev->dev);
> +	flag2 = device_may_wakeup(&udev->dev);
> +	RTKBT_DBG("btusb_probe 1 ========== can_wakeup =%x	 flag2 =%x",
> +		  flag1, flag2);
> +	device_wakeup_disable(&udev->dev);

Why?

> +	flag1 = device_can_wakeup(&udev->dev);
> +	flag2 = device_may_wakeup(&udev->dev);
> +	RTKBT_DBG("wakeup_disable ========== can_wakeup =%x	 flag2 =%x",
> +		  flag1, flag2);
> +	err = patch_add(intf);
> +	if (err < 0)
> +		return -1;
> +	/*******************************/
> +
> +	data = kzalloc(sizeof(*data), GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +
> +	for (i = 0; i < intf->cur_altsetting->desc.bNumEndpoints; i++) {
> +		ep_desc = &intf->cur_altsetting->endpoint[i].desc;
> +
> +		if (!data->intr_ep && usb_endpoint_is_int_in(ep_desc)) {
> +			data->intr_ep = ep_desc;
> +			continue;
> +		}
> +
> +		if (!data->bulk_tx_ep && usb_endpoint_is_bulk_out(ep_desc)) {
> +			data->bulk_tx_ep = ep_desc;
> +			continue;
> +		}
> +
> +		if (!data->bulk_rx_ep && usb_endpoint_is_bulk_in(ep_desc)) {
> +			data->bulk_rx_ep = ep_desc;
> +			continue;
> +		}
> +	}
> +
> +	if (!data->intr_ep || !data->bulk_tx_ep || !data->bulk_rx_ep) {
> +		kfree(data);
> +		return -ENODEV;
> +	}
> +
> +	data->cmdreq_type = USB_TYPE_CLASS;
> +
> +	data->udev = interface_to_usbdev(intf);
> +	data->intf = intf;
> +
> +	spin_lock_init(&data->lock);
> +
> +	INIT_WORK(&data->work, btusb_work);
> +	INIT_WORK(&data->waker, btusb_waker);
> +	spin_lock_init(&data->txlock);
> +
> +	init_usb_anchor(&data->tx_anchor);
> +	init_usb_anchor(&data->intr_anchor);
> +	init_usb_anchor(&data->bulk_anchor);
> +	init_usb_anchor(&data->isoc_anchor);
> +	init_usb_anchor(&data->deferred);
> +
> +	hdev = hci_alloc_dev();
> +	if (!hdev) {
> +		kfree(data);
> +		return -ENOMEM;
> +	}
> +
> +	HDEV_BUS = HCI_USB;
> +
> +	data->hdev = hdev;
> +
> +	SET_HCIDEV_DEV(hdev, &intf->dev);
> +
> +	hdev->open     = btusb_open;
> +	hdev->close    = btusb_close;
> +	hdev->flush    = btusb_flush;
> +	hdev->send     = btusb_send_frame;
> +	hdev->notify   = btusb_notify;
> +
> +
> +	hci_set_drvdata(hdev, data);
> +
> +	/* Interface numbers are hardcoded in the specification */
> +	data->isoc = usb_ifnum_to_if(data->udev, 1);
> +
> +	if (data->isoc) {
> +		err = usb_driver_claim_interface(&btusb_driver,
> +						 data->isoc, data);
> +		if (err < 0) {
> +			hci_free_dev(hdev);
> +			kfree(data);
> +			return err;
> +		}
> +	}
> +
> +	err = hci_register_dev(hdev);
> +	if (err < 0) {
> +		hci_free_dev(hdev);
> +		kfree(data);
> +		return err;
> +	}
> +
> +	usb_set_intfdata(intf, data);
> +
> +	return 0;
> +}
> +
> +static void btusb_disconnect(struct usb_interface *intf)
> +{
> +	struct btusb_data *data = usb_get_intfdata(intf);
> +	struct hci_dev *hdev;
> +	struct usb_device *udev;
> +	udev = interface_to_usbdev(intf);
> +
> +	if (intf->cur_altsetting->desc.bInterfaceNumber != 0)
> +		return;
> +
> +	if (!data)
> +		return;
> +
> +	RTKBT_DBG("btusb_disconnect");
> +	/*******************************/
> +	patch_remove(intf);

This is a race. The device is not dead at this time.

> +	/*******************************/
> +
> +	hdev = data->hdev;
> +
> +	usb_set_intfdata(data->intf, NULL);
> +
> +	if (data->isoc)
> +		usb_set_intfdata(data->isoc, NULL);
> +
> +	hci_unregister_dev(hdev);
> +
> +	if (intf == data->isoc)
> +		usb_driver_release_interface(&btusb_driver, data->intf);
> +	else if (data->isoc)
> +		usb_driver_release_interface(&btusb_driver, data->isoc);
> +
> +	hci_free_dev(hdev);
> +	kfree(data);
> +}
> +
> +#ifdef CONFIG_PM
> +static int btusb_suspend(struct usb_interface *intf, pm_message_t message)
> +{
> +	struct btusb_data *data = usb_get_intfdata(intf);
> +
> +	if (intf->cur_altsetting->desc.bInterfaceNumber != 0)
> +		return 0;
> +
> +	/*******************************/
> +	RTKBT_DBG("btusb_suspend message.event = 0x%x, data->suspend_count =%d",
> +		  message.event, data->suspend_count);
> +	if (!test_bit(HCI_RUNNING, &data->hdev->flags)) {
> +		RTKBT_DBG("btusb_suspend-----bt is off");
> +		set_btoff(data->intf);

Why repeat this if the device is already suspended?

> +	}
> +	/*******************************/
> +
> +	if (data->suspend_count++)
> +		return 0;
> +
> +	spin_lock_irq(&data->txlock);
> +	if (!((message.event & PM_EVENT_AUTO) && data->tx_in_flight)) {
> +		set_bit(BTUSB_SUSPENDING, &data->flags);
> +		spin_unlock_irq(&data->txlock);
> +	} else {
> +		spin_unlock_irq(&data->txlock);
> +		data->suspend_count--;
> +		return -EBUSY;
> +	}
> +
> +	cancel_work_sync(&data->work);
> +
> +	btusb_stop_traffic(data);
> +	mdelay(URB_CANCELING_DELAY_MS);      /*  Added by Realtek */
> +	usb_kill_anchored_urbs(&data->tx_anchor);
> +
> +	return 0;
> +}
> +
> +static void play_deferred(struct btusb_data *data)
> +{
> +	struct urb *urb;
> +	int err;
> +
> +	while ((urb = usb_get_from_anchor(&data->deferred))) {
> +
> +		/************************************/
> +		usb_anchor_urb(urb, &data->tx_anchor);
> +		err = usb_submit_urb(urb, GFP_ATOMIC);
> +		if (err < 0) {
> +			BT_ERR("play_deferred urb %p submission failed",  urb);
> +			kfree(urb->setup_packet);
> +			usb_unanchor_urb(urb);
> +		} else {
> +			usb_mark_last_busy(data->udev);
> +		}
> +		usb_free_urb(urb);
> +		/************************************/
> +		data->tx_in_flight++;
> +	}
> +	mdelay(URB_CANCELING_DELAY_MS);     /*  Added by Realtek */

These URBs never went over the wire. Why a delay?

> +	usb_scuttle_anchored_urbs(&data->deferred);
> +}
> +
> +static int btusb_resume(struct usb_interface *intf)
> +{
> +	struct btusb_data *data = usb_get_intfdata(intf);
> +	struct hci_dev *hdev = data->hdev;
> +	int err = 0;
> +
> +	if (intf->cur_altsetting->desc.bInterfaceNumber != 0)
> +		return 0;
> +
> +
> +	/*******************************/
> +	RTKBT_DBG("btusb_resume data->suspend_count =%d", data->suspend_count);
> +
> +	if (!test_bit(HCI_RUNNING, &hdev->flags)) {
> +		RTKBT_DBG("btusb_resume-----bt is off, download patch");
> +		download_patch(intf);
> +	} else {
> +		RTKBT_DBG("btusb_resume,----bt is on");
> +	}
> +	/*******************************/
> +	if (--data->suspend_count)
> +		return 0;
> +
> +	if (test_bit(BTUSB_INTR_RUNNING, &data->flags)) {
> +		err = btusb_submit_intr_urb(hdev, GFP_NOIO);
> +		if (err < 0) {
> +			clear_bit(BTUSB_INTR_RUNNING, &data->flags);
> +			goto failed;
> +		}
> +	}
> +
> +	if (test_bit(BTUSB_BULK_RUNNING, &data->flags)) {
> +		err = btusb_submit_bulk_urb(hdev, GFP_NOIO);
> +		if (err < 0) {
> +			clear_bit(BTUSB_BULK_RUNNING, &data->flags);
> +			goto failed;
> +		}
> +
> +		btusb_submit_bulk_urb(hdev, GFP_NOIO);
> +	}
> +
> +	if (test_bit(BTUSB_ISOC_RUNNING, &data->flags)) {
> +		if (btusb_submit_isoc_urb(hdev, GFP_NOIO) < 0)
> +			clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
> +		else
> +			btusb_submit_isoc_urb(hdev, GFP_NOIO);
> +	}
> +
> +	spin_lock_irq(&data->txlock);
> +	play_deferred(data);
> +	clear_bit(BTUSB_SUSPENDING, &data->flags);
> +	spin_unlock_irq(&data->txlock);
> +	schedule_work(&data->work);
> +
> +	return 0;
> +
> +failed:
> +	mdelay(URB_CANCELING_DELAY_MS);      /*  Added by Realtek */

Again, why?

> +	usb_scuttle_anchored_urbs(&data->deferred);
> +/* done: */
> +	spin_lock_irq(&data->txlock);
> +	clear_bit(BTUSB_SUSPENDING, &data->flags);
> +	spin_unlock_irq(&data->txlock);
> +
> +	return err;
> +}
> +#endif
> +
> +static struct usb_driver btusb_driver = {
> +	.name		= "rtk_btusb",
> +	.probe		= btusb_probe,
> +	.disconnect	= btusb_disconnect,
> +#ifdef CONFIG_PM
> +	.suspend	= btusb_suspend,
> +	.resume		= btusb_resume,
> +#endif
> +	.id_table	= btusb_table,
> +	.supports_autosuspend = 1,
> +};
> +
> +static int __init btusb_init(void)
> +{
> +	RTKBT_DBG("Realtek Bluetooth USB driver ver %s", VERSION);
> +
> +	return usb_register(&btusb_driver);
> +}
> +
> +static void __exit btusb_exit(void)
> +{
> +	usb_deregister(&btusb_driver);
> +}
> +
> +module_init(btusb_init);
> +module_exit(btusb_exit);
> +
> +MODULE_AUTHOR("Edward Bian <edward_bian@realsil.com.cn>");
> +MODULE_DESCRIPTION("Realtek Bluetooth USB driver ver " VERSION);
> +MODULE_VERSION(VERSION);
> +MODULE_LICENSE("GPL");
> +MODULE_FIRMWARE("rtl_bt/rtl8723a.bin");
> +
> +/*******************************
> +**    Reasil patch code
> +********************************/
> +
> +
> +#include <linux/firmware.h>
> +#include <linux/suspend.h>
> +#include <net/bluetooth/hci.h>
> +
> +
> +#define CMD_CMP_EVT		0x0e
> +#define PKT_LEN			300
> +#define MSG_TO			1000
> +#define PATCH_SEG_MAX	252
> +#define DATA_END		0x80
> +#define DOWNLOAD_OPCODE	0xfc20
> +#define BTOFF_OPCODE	0xfc28
> +#define TRUE			1
> +#define FALSE			0
> +#define CMD_HDR_LEN		sizeof(struct hci_command_hdr)
> +#define EVT_HDR_LEN		sizeof(struct hci_event_hdr)
> +#define CMD_CMP_LEN		sizeof(struct hci_ev_cmd_complete)
> +
> +
> +enum rtk_endpoit {
> +	CTRL_EP = 0,
> +	INTR_EP = 1,
> +	BULK_EP = 2,
> +	ISOC_EP = 3
> +};
> +
> +struct patch_info {
> +	uint16_t	prod_id;
> +	uint16_t	lmp_sub;
> +	char		*patch_name;
> +	char		*config_name;
> +	uint8_t		*fw_cache;
> +	int		fw_len;
> +};
> +
> +struct xchange_data {
> +	struct dev_data	*dev_entry;
> +	int pipe_in, pipe_out;
> +	uint8_t send_pkt[PKT_LEN];
> +	uint8_t rcv_pkt[PKT_LEN];

Violations of the DMA coherency rules, fails on non-x86

> +	struct hci_command_hdr *cmd_hdr;
> +	struct hci_event_hdr *evt_hdr;
> +	struct hci_ev_cmd_complete *cmd_cmp;
> +	uint8_t *req_para, *rsp_para;
> +	uint8_t *fw_data;
> +	int pkt_len, fw_len;
> +};
> +
> +struct dev_data {
> +	struct list_head	list_node;
> +	struct usb_interface	*intf;
> +	struct usb_device	*udev;
> +	struct notifier_block	pm_notifier;
> +	struct patch_info	*patch_entry;
> +	struct xchange_data	xdata;
> +	struct completion firmware_loading_complete;
> +	const struct firmware *fw;
> +};
> +
> +struct download_cp {
> +	uint8_t index;
> +	uint8_t data[PATCH_SEG_MAX];

DMA coherency

> +} __packed;
> +
> +struct download_rp {
> +	uint8_t status;
> +	uint8_t index;
> +} __packed;
> +
> +
> +static struct dev_data *dev_data_find(struct usb_interface *intf);
> +static struct patch_info *get_patch_entry(struct usb_device *udev);
> +static int rtkbt_pm_notify(struct notifier_block *notifier, ulong pm_event,
> +			   void *unused);
> +static int load_firmware(struct dev_data *dev_entry, uint8_t **buff);
> +static void init_xdata(struct xchange_data *xdata, struct dev_data *dev_entry);
> +static int check_fw_version(struct xchange_data *xdata);
> +static int get_firmware(struct xchange_data *xdata);
> +static int download_data(struct xchange_data *xdata);
> +static int send_hci_cmd(struct xchange_data *xdata);
> +static int rcv_hci_evt(struct xchange_data *xdata);
> +
> +
> +static struct patch_info patch_table[] = {
> +	{0, 0x1200, "rtl_bt/rtl8723a.bin", "rtk8723_bt_config", NULL, 0}
> +};
> +
> +static LIST_HEAD(dev_data_list);
> +
> +
> +static int patch_add(struct usb_interface *intf)
> +{
> +	struct dev_data	*dev_entry;
> +	struct usb_device *udev;
> +
> +	RTKBT_DBG("patch_add");
> +	dev_entry = dev_data_find(intf);
> +	if (NULL != dev_entry)
> +		return -1;
> +
> +	udev = interface_to_usbdev(intf);
> +#if BTUSB_RPM
> +	RTKBT_DBG("auto suspend is enabled");
> +	usb_enable_autosuspend(udev);
> +	pm_runtime_set_autosuspend_delay(&(udev->dev), 2000);

There is no good reason to overwrite the system values.

> +#endif
> +
> +	dev_entry = kzalloc(sizeof(struct dev_data), GFP_KERNEL);
> +	dev_entry->intf = intf;
> +	dev_entry->udev = udev;
> +	dev_entry->pm_notifier.notifier_call = rtkbt_pm_notify;
> +	dev_entry->patch_entry = get_patch_entry(udev);
> +	list_add(&dev_entry->list_node, &dev_data_list);
> +	register_pm_notifier(&dev_entry->pm_notifier);
> +
> +	return 0;
> +}
> +
> +static void patch_remove(struct usb_interface *intf)
> +{
> +	struct dev_data *dev_entry;
> +	struct usb_device *udev;
> +
> +	udev = interface_to_usbdev(intf);
> +#if BTUSB_RPM
> +	usb_disable_autosuspend(udev);
> +#endif
> +
> +	dev_entry = dev_data_find(intf);
> +	if (NULL == dev_entry)
> +		return;
> +
> +	RTKBT_DBG("patch_remove");
> +	list_del(&dev_entry->list_node);
> +	unregister_pm_notifier(&dev_entry->pm_notifier);
> +	kfree(dev_entry);
> +}
> +
> +static int download_patch(struct usb_interface *intf)
> +{
> +	struct dev_data		*dev_entry;
> +	uint8_t			*fw_buf;
> +	int			ret_val;
> +
> +	RTKBT_DBG("download_patch start");
> +	dev_entry = dev_data_find(intf);
> +	if (NULL == dev_entry) {
> +		ret_val = -1;
> +		RTKBT_DBG("NULL == dev_entry");
> +		goto patch_end;
> +	}
> +
> +	init_xdata(&dev_entry->xdata, dev_entry);
> +	ret_val = check_fw_version(&dev_entry->xdata);
> +	if (ret_val != 0)
> +		goto patch_end;
> +
> +	ret_val = get_firmware(&dev_entry->xdata);
> +	if (ret_val < 0) {
> +		RTKBT_DBG("get_firmware failed!");
> +		goto patch_end;
> +	}
> +	fw_buf = dev_entry->xdata.fw_data;
> +
> +	ret_val = download_data(&dev_entry->xdata);
> +	if (ret_val < 0) {
> +		RTKBT_DBG("download_data failed!");
> +		goto patch_fail;
> +	}
> +
> +	ret_val = check_fw_version(&dev_entry->xdata);
> +	if (ret_val <= 0) {
> +		ret_val = -1;
> +		goto patch_fail;
> +	}
> +
> +	ret_val = 0;
> +patch_fail:
> +	kfree(fw_buf);
> +patch_end:
> +	RTKBT_DBG("Rtk patch end %d", ret_val);
> +	return ret_val;
> +}
> +
> +static int set_btoff(struct usb_interface *intf)
> +{
> +	struct dev_data *dev_entry;
> +	int ret_val;
> +
> +	RTKBT_DBG("set_btoff");
> +	dev_entry = dev_data_find(intf);
> +	if (NULL == dev_entry)
> +		return -1;
> +
> +	init_xdata(&dev_entry->xdata, dev_entry);
> +	dev_entry->xdata.cmd_hdr->opcode = cpu_to_le16(BTOFF_OPCODE);
> +	dev_entry->xdata.cmd_hdr->plen = 1;
> +	dev_entry->xdata.pkt_len = CMD_HDR_LEN + 1;
> +	dev_entry->xdata.send_pkt[CMD_HDR_LEN] = 1;
> +
> +	ret_val = send_hci_cmd(&dev_entry->xdata);
> +	if (ret_val < 0)
> +		return ret_val;
> +
> +	ret_val = rcv_hci_evt(&dev_entry->xdata);
> +	if (ret_val < 0)
> +		return ret_val;
> +
> +	RTKBT_DBG("set_btoff done");
> +	return 0;
> +}
> +
> +static struct dev_data *dev_data_find(struct usb_interface *intf)
> +{
> +	struct dev_data *dev_entry;
> +
> +	list_for_each_entry(dev_entry, &dev_data_list, list_node) {
> +		if (dev_entry->intf == intf)
> +			return dev_entry;
> +	}
> +
> +	return NULL;
> +}
> +
> +static struct patch_info *get_patch_entry(struct usb_device *udev)
> +{
> +	struct patch_info	*patch_entry;
> +	uint16_t	pid;
> +
> +	patch_entry = patch_table;
> +	pid = le16_to_cpu(udev->descriptor.idProduct);
> +	while (pid != patch_entry->prod_id) {
> +		if (0 == patch_entry->prod_id)
> +			break;
> +		patch_entry++;
> +	}
> +
> +	return patch_entry;
> +}
> +
> +static int rtkbt_pm_notify(struct notifier_block *notifier, ulong pm_event,
> +		    void *unused)
> +{
> +	struct dev_data	*dev_entry;
> +	struct patch_info	*patch_entry;
> +	struct usb_device *udev;
> +
> +	dev_entry = container_of(notifier, struct dev_data, pm_notifier);
> +	patch_entry = dev_entry->patch_entry;
> +	udev = dev_entry->udev;
> +	RTKBT_DBG("rtkbt_pm_notify pm_event =%ld", pm_event);
> +	switch (pm_event) {
> +	case PM_SUSPEND_PREPARE:
> +	case PM_HIBERNATION_PREPARE:
> +		patch_entry->fw_len = load_firmware(dev_entry,
> +						    &patch_entry->fw_cache);
> +		if (patch_entry->fw_len <= 0) {
> +			RTKBT_DBG("rtkbt_pm_notify return NOTIFY_BAD");
> +			return NOTIFY_BAD;
> +		}
> +
> +		if (!device_may_wakeup(&udev->dev)) {
> +			dev_entry->intf->needs_binding = 1;
> +			RTKBT_DBG("remote wakeup not support, set intf->needs_binding = 1");
> +		}
> +		break;
> +
> +	case PM_POST_SUSPEND:
> +	case PM_POST_HIBERNATION:
> +	case PM_POST_RESTORE:
> +		if (patch_entry->fw_len > 0) {
> +			kfree(patch_entry->fw_cache);
> +			patch_entry->fw_cache = NULL;
> +			patch_entry->fw_len = 0;
> +		}
> +#if BTUSB_RPM
> +		usb_disable_autosuspend(udev);
> +		usb_enable_autosuspend(udev);
> +		pm_runtime_set_autosuspend_delay(&(udev->dev), 2000);
> +#endif
> +		break;
> +
> +	default:
> +		break;
> +	}
> +
> +	return NOTIFY_DONE;
> +}
> +
> +static void bt_fw_cb(const struct firmware *firmware, void *context)
> +{
> +	struct dev_data *dev_entry = context;
> +
> +	dev_entry->fw = firmware;
> +	if (!firmware)
> +		pr_err("In callback routine, firmware file not available\n");
> +	complete(&dev_entry->firmware_loading_complete);
> +}
> +
> +static int load_firmware(struct dev_data *dev_entry, uint8_t **buff)
> +{
> +#if LOAD_CONFIG
> +	const struct firmware *fw;
> +#endif
> +	struct usb_device *udev;
> +	struct patch_info *patch_entry;
> +	char *fw_name;
> +	int fw_len = 0, ret_val;
> +
> +	udev = dev_entry->udev;
> +	init_completion(&dev_entry->firmware_loading_complete);
> +	patch_entry = dev_entry->patch_entry;
> +	fw_name = patch_entry->patch_name;
> +	RTKBT_DBG("Reading firmware file %s", fw_name);
> +	ret_val = request_firmware_nowait(THIS_MODULE, 1, fw_name, &udev->dev,
> +					  GFP_KERNEL, dev_entry, bt_fw_cb);
> +	if (ret_val < 0)
> +		goto fw_fail;
> +
> +	wait_for_completion(&dev_entry->firmware_loading_complete);
> +	if (!dev_entry->fw)
> +		goto fw_fail;
> +	*buff = kzalloc(dev_entry->fw->size, GFP_KERNEL);
> +	if (NULL == *buff)
> +		goto alloc_fail;
> +	memcpy(*buff, dev_entry->fw->data, dev_entry->fw->size);
> +	fw_len = dev_entry->fw->size;
> +
> +#if LOAD_CONFIG
> +	release_firmware(dev_entry->fw);
> +	fw_name = patch_entry->config_name;
> +	ret_val = request_firmware(&fw, fw_name, &udev->dev);
> +	if (ret_val < 0) {
> +		fw_len = 0;
> +		kfree(*buff);
> +		*buff = NULL;
> +		goto fw_fail;
> +	}
> +
> +	*buff = krealloc(*buff, fw_len + fw->size, GFP_KERNEL);
> +	if (NULL == *buff) {
> +		fw_len = 0;
> +		release_firmware(fw);
> +		goto fw_fail;
> +	}
> +	memcpy(*buff + fw_len, fw->data, fw->size);
> +	fw_len += fw->size;
> +#endif
> +
> +alloc_fail:
> +	release_firmware(dev_entry->fw);
> +fw_fail:
> +	return fw_len;
> +}
> +
> +static void init_xdata(struct xchange_data *xdata, struct dev_data *dev_entry)
> +{
> +	memset(xdata, 0, sizeof(struct xchange_data));
> +	xdata->dev_entry = dev_entry;
> +	xdata->pipe_in = usb_rcvintpipe(dev_entry->udev, INTR_EP);
> +	xdata->pipe_out = usb_sndctrlpipe(dev_entry->udev, CTRL_EP);
> +	xdata->cmd_hdr = (struct hci_command_hdr *)(xdata->send_pkt);
> +	xdata->evt_hdr = (struct hci_event_hdr *)(xdata->rcv_pkt);
> +	xdata->cmd_cmp = (struct hci_ev_cmd_complete *)(xdata->rcv_pkt +
> +							EVT_HDR_LEN);
> +	xdata->req_para = xdata->send_pkt + CMD_HDR_LEN;
> +	xdata->rsp_para = xdata->rcv_pkt + EVT_HDR_LEN + CMD_CMP_LEN;
> +}
> +
> +static int check_fw_version(struct xchange_data *xdata)
> +{
> +	struct hci_rp_read_local_version *read_ver_rsp;
> +	struct patch_info *patch_entry;
> +	int ret_val;
> +
> +	xdata->cmd_hdr->opcode = cpu_to_le16(HCI_OP_READ_LOCAL_VERSION);
> +	xdata->cmd_hdr->plen = 0;
> +	xdata->pkt_len = CMD_HDR_LEN;
> +
> +	ret_val = send_hci_cmd(xdata);
> +	if (ret_val < 0)
> +		goto version_end;
> +
> +	ret_val = rcv_hci_evt(xdata);
> +	if (ret_val < 0)
> +		goto version_end;
> +
> +	patch_entry = xdata->dev_entry->patch_entry;
> +	read_ver_rsp = (struct hci_rp_read_local_version *)(xdata->rsp_para);
> +	RTKBT_DBG("check_fw_version : read_ver_rsp->lmp_subver = 0x%x",
> +		  le16_to_cpu(read_ver_rsp->lmp_subver));
> +	if (patch_entry->lmp_sub != le16_to_cpu(read_ver_rsp->lmp_subver))
> +		return 1;
> +
> +	ret_val = 0;
> +version_end:
> +	return ret_val;
> +}
> +
> +static int get_firmware(struct xchange_data *xdata)
> +{
> +	struct dev_data	*dev_entry;
> +	struct patch_info *patch_entry;
> +
> +	dev_entry = xdata->dev_entry;
> +	patch_entry = dev_entry->patch_entry;
> +	if (patch_entry->fw_len > 0) {
> +		xdata->fw_data = kzalloc(patch_entry->fw_len, GFP_KERNEL);
> +		if (NULL == xdata->fw_data)
> +			return -ENOMEM;
> +		memcpy(xdata->fw_data, patch_entry->fw_cache,
> +		       patch_entry->fw_len);
> +		xdata->fw_len = patch_entry->fw_len;
> +	} else {
> +		xdata->fw_len = load_firmware(dev_entry, &xdata->fw_data);
> +		if (xdata->fw_len <= 0)
> +			return -1;
> +	}
> +
> +	return 0;
> +}
> +
> +static int download_data(struct xchange_data *xdata)
> +{
> +	struct download_cp *cmd_para;
> +	struct download_rp *evt_para;
> +	uint8_t *pcur;
> +	int pkt_len, frag_num, frag_len;
> +	int i, ret_val;
> +
> +	cmd_para = (struct download_cp *)xdata->req_para;
> +	evt_para = (struct download_rp *)xdata->rsp_para;
> +	pcur = xdata->fw_data;
> +	pkt_len = CMD_HDR_LEN + sizeof(struct download_cp);
> +	frag_num = xdata->fw_len / PATCH_SEG_MAX + 1;
> +	frag_len = PATCH_SEG_MAX;
> +
> +	for (i = 0; i < frag_num; i++) {
> +		cmd_para->index = i;
> +		if (i == (frag_num - 1)) {
> +			cmd_para->index |= DATA_END;
> +			frag_len = xdata->fw_len % PATCH_SEG_MAX;
> +			pkt_len -= (PATCH_SEG_MAX - frag_len);
> +		}
> +		xdata->cmd_hdr->opcode = cpu_to_le16(DOWNLOAD_OPCODE);
> +		xdata->cmd_hdr->plen = sizeof(uint8_t) + frag_len;
> +		xdata->pkt_len = pkt_len;
> +		memcpy(cmd_para->data, pcur, frag_len);
> +
> +		ret_val = send_hci_cmd(xdata);
> +		if (ret_val < 0)
> +			return ret_val;
> +
> +		ret_val = rcv_hci_evt(xdata);
> +		if (ret_val < 0)
> +			return ret_val;
> +		if (0 != evt_para->status)
> +			return -1;
> +
> +		pcur += PATCH_SEG_MAX;
> +	}
> +
> +	return xdata->fw_len;
> +}
> +
> +static int send_hci_cmd(struct xchange_data *xdata)
> +{
> +	int ret_val;
> +
> +	ret_val = usb_control_msg(
> +		xdata->dev_entry->udev, xdata->pipe_out,
> +		0, USB_TYPE_CLASS, 0, 0,
> +		(void *)(xdata->send_pkt),
> +		xdata->pkt_len, MSG_TO);
> +
> +	return ret_val;
> +}
> +
> +static int rcv_hci_evt(struct xchange_data *xdata)
> +{
> +	int ret_len, ret_val;
> +	int i;   /*  Added by Realtek */
> +
> +	while (1) {
> +
> +		/*  **************************** Modifed by Realtek (begin) */
> +		for (i = 0; i < 5; i++) {
> +			/*  Try to send USB interrupt message 5 times. */
> +			ret_val = usb_interrupt_msg(
> +				xdata->dev_entry->udev, xdata->pipe_in,
> +				(void *)(xdata->rcv_pkt), PKT_LEN,
> +				&ret_len, MSG_TO);
> +			if (ret_val >= 0)
> +				break;
> +		}
> +		/*  **************************** Modifed by Realtek (end) */
> +
> +		if (ret_val < 0)
> +			return ret_val;
> +
> +		if (CMD_CMP_EVT == xdata->evt_hdr->evt) {
> +			if (xdata->cmd_hdr->opcode == xdata->cmd_cmp->opcode)
> +				return ret_len;
> +		}
> +	}
> +}
> 

^ permalink raw reply

* Re: [PATCH BlueZ 1/8] gdbus: Introduce G_DBUS_METHOD_FLAG_EXPERIMENTAL
From: Marcel Holtmann @ 2012-12-23 20:58 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <1356293718-9348-1-git-send-email-luiz.dentz@gmail.com>

Hi Luiz,

> This flag can be used to mark methods as experimental, the marked
> methods with this flag can be enabled by setting the environment variable
> GDBUS_EXPERIMENTAL=1
> ---
>  gdbus/gdbus.h  | 21 ++++++++++++++++++---
>  gdbus/object.c | 17 +++++++++++++++++
>  2 files changed, 35 insertions(+), 3 deletions(-)
> 
> diff --git a/gdbus/gdbus.h b/gdbus/gdbus.h
> index 0e5c012..00fbb1c 100644
> --- a/gdbus/gdbus.h
> +++ b/gdbus/gdbus.h
> @@ -89,9 +89,10 @@ typedef void (* GDBusSecurityFunction) (DBusConnection *connection,
>  						GDBusPendingReply pending);
>  
>  enum GDBusMethodFlags {
> -	G_DBUS_METHOD_FLAG_DEPRECATED = (1 << 0),
> -	G_DBUS_METHOD_FLAG_NOREPLY    = (1 << 1),
> -	G_DBUS_METHOD_FLAG_ASYNC      = (1 << 2),
> +	G_DBUS_METHOD_FLAG_DEPRECATED   = (1 << 0),
> +	G_DBUS_METHOD_FLAG_NOREPLY      = (1 << 1),
> +	G_DBUS_METHOD_FLAG_ASYNC        = (1 << 2),
> +	G_DBUS_METHOD_FLAG_EXPERIMENTAL = (1 << 3),
>  };
>  
>  enum GDBusSignalFlags {
> @@ -173,6 +174,20 @@ struct GDBusSecurityTable {
>  	.function = _function, \
>  	.flags = G_DBUS_METHOD_FLAG_ASYNC | G_DBUS_METHOD_FLAG_DEPRECATED
>  
> +#define GDBUS_EXPERIMENTAL_METHOD(_name, _in_args, _out_args, _function) \
> +	.name = _name, \
> +	.in_args = _in_args, \
> +	.out_args = _out_args, \
> +	.function = _function, \
> +	.flags = G_DBUS_METHOD_FLAG_EXPERIMENTAL
> +
> +#define GDBUS_EXPERIMENTAL_ASYNC_METHOD(_name, _in_args, _out_args, _function) \
> +	.name = _name, \
> +	.in_args = _in_args, \
> +	.out_args = _out_args, \
> +	.function = _function, \
> +	.flags = G_DBUS_METHOD_FLAG_ASYNC | G_DBUS_METHOD_FLAG_EXPERIMENTAL
> +
>  #define GDBUS_NOREPLY_METHOD(_name, _in_args, _out_args, _function) \
>  	.name = _name, \
>  	.in_args = _in_args, \
> diff --git a/gdbus/object.c b/gdbus/object.c
> index 776d35e..30dbbc2 100644
> --- a/gdbus/object.c
> +++ b/gdbus/object.c
> @@ -129,6 +129,14 @@ static void generate_interface_xml(GString *gstr, struct interface_data *iface)
>  						G_DBUS_METHOD_FLAG_DEPRECATED;
>  		gboolean noreply = method->flags &
>  						G_DBUS_METHOD_FLAG_NOREPLY;
> +		gboolean experimental = method->flags &
> +					G_DBUS_METHOD_FLAG_EXPERIMENTAL;
> +
> +		if (experimental) {
> +			const char *env = g_getenv("GDBUS_EXPERIMENTAL");
> +			if (g_strcmp0(env, "1") != 0)
> +				continue;
> +		}

actually since this is a library, doing it this way is a bad idea.

Lets do something like g_dbus_enable_experimental(DBusConnection)

Regards

Marcel



^ permalink raw reply

* [PATCH BlueZ 8/8] AVRCP: Fix not checking for media_player_controller_create
From: Luiz Augusto von Dentz @ 2012-12-23 20:15 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1356293718-9348-1-git-send-email-luiz.dentz@gmail.com>

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

Now that the MediaPlayer1 interface is experimental the interface
registration may fail which return NULL, in that case there is no
point on register to any notifications.
---
 profiles/audio/avrcp.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
index 4e3d31d..ce070cd 100644
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -1987,6 +1987,11 @@ static gboolean avrcp_get_capabilities_resp(struct avctp *conn,
 
 	count = pdu->params[1];
 
+	path = device_get_path(session->dev->btd_dev);
+	mp = media_player_controller_create(path);
+	if (mp == NULL)
+		return FALSE;
+
 	for (; count > 0; count--) {
 		uint8_t event = pdu->params[1 + count];
 
@@ -2001,8 +2006,6 @@ static gboolean avrcp_get_capabilities_resp(struct avctp *conn,
 		}
 	}
 
-	path = device_get_path(session->dev->btd_dev);
-	mp = media_player_controller_create(path);
 	media_player_set_callbacks(mp, &ct_cbs, player);
 	player->user_data = mp;
 	player->destroy = (GDestroyNotify) media_player_destroy;
-- 
1.7.11.7


^ permalink raw reply related

* [PATCH BlueZ 7/8] player: Enable MediaPlayer1 interface as experimental
From: Luiz Augusto von Dentz @ 2012-12-23 20:15 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1356293718-9348-1-git-send-email-luiz.dentz@gmail.com>

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This enable MediaPlayer1 when GDBUS_EXPERIMENTAL=1
---
 profiles/audio/player.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/profiles/audio/player.c b/profiles/audio/player.c
index 005d0d1..693a590 100644
--- a/profiles/audio/player.c
+++ b/profiles/audio/player.c
@@ -239,25 +239,31 @@ static void set_setting(const GDBusPropertyTable *property,
 }
 
 static const GDBusMethodTable media_player_methods[] = {
-	{ GDBUS_METHOD("GetTrack",
+	{ GDBUS_EXPERIMENTAL_METHOD("GetTrack",
 			NULL, GDBUS_ARGS({ "metadata", "a{sv}" }),
 			media_player_get_track) },
 	{ }
 };
 
 static const GDBusSignalTable media_player_signals[] = {
-	{ GDBUS_SIGNAL("TrackChanged",
+	{ GDBUS_EXPERIMENTAL_SIGNAL("TrackChanged",
 			GDBUS_ARGS({ "metadata", "a{sv}" })) },
 	{ }
 };
 
 static const GDBusPropertyTable media_player_properties[] = {
-	{ "Position", "u", get_position },
-	{ "Status", "s", get_status, NULL, status_exists },
-	{ "Equalizer", "s", get_setting, set_setting, setting_exists },
-	{ "Repeat", "s", get_setting, set_setting, setting_exists },
-	{ "Shuffle", "s", get_setting, set_setting, setting_exists },
-	{ "Scan", "s", get_setting, set_setting, setting_exists },
+	{ "Position", "u", get_position, NULL, NULL,
+					G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
+	{ "Status", "s", get_status, NULL, status_exists,
+					G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
+	{ "Equalizer", "s", get_setting, set_setting, setting_exists,
+					G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
+	{ "Repeat", "s", get_setting, set_setting, setting_exists,
+					G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
+	{ "Shuffle", "s", get_setting, set_setting, setting_exists,
+					G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
+	{ "Scan", "s", get_setting, set_setting, setting_exists,
+					G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
 	{ }
 };
 
@@ -298,7 +304,6 @@ struct media_player *media_player_controller_create(const char *path)
 							g_free, g_free);
 	mp->progress = g_timer_new();
 
-#if 0
 	if (!g_dbus_register_interface(btd_get_dbus_connection(),
 					mp->path, MEDIA_PLAYER_INTERFACE,
 					media_player_methods,
@@ -308,7 +313,7 @@ struct media_player *media_player_controller_create(const char *path)
 		media_player_destroy(mp);
 		return NULL;
 	}
-#endif
+
 	DBG("%s", mp->path);
 
 	return mp;
@@ -410,7 +415,6 @@ void media_player_set_status(struct media_player *mp, const char *status)
 
 static gboolean process_metadata_changed(void *user_data)
 {
-#if 0
 	struct media_player *mp = user_data;
 	DBusMessage *signal;
 	DBusMessageIter iter, dict;
@@ -439,7 +443,7 @@ static gboolean process_metadata_changed(void *user_data)
 	dbus_message_iter_close_container(&iter, &dict);
 
 	g_dbus_send_message(btd_get_dbus_connection(), signal);
-#endif
+
 	return FALSE;
 }
 
-- 
1.7.11.7


^ permalink raw reply related

* [PATCH BlueZ 6/8] media: Enable RegisterPlayer and UnregisterPlayer methods as experimental
From: Luiz Augusto von Dentz @ 2012-12-23 20:15 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1356293718-9348-1-git-send-email-luiz.dentz@gmail.com>

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

---
 profiles/audio/media.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/profiles/audio/media.c b/profiles/audio/media.c
index f728460..e4206e3 100644
--- a/profiles/audio/media.c
+++ b/profiles/audio/media.c
@@ -868,7 +868,6 @@ static DBusMessage *unregister_endpoint(DBusConnection *conn, DBusMessage *msg,
 	return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
 }
 
-#if 0
 static struct media_player *media_adapter_find_player(
 						struct media_adapter *adapter,
 						const char *sender,
@@ -1533,7 +1532,6 @@ static DBusMessage *unregister_player(DBusConnection *conn, DBusMessage *msg,
 
 	return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
 }
-#endif
 
 static const GDBusMethodTable media_methods[] = {
 	{ GDBUS_METHOD("RegisterEndpoint",
@@ -1541,14 +1539,12 @@ static const GDBusMethodTable media_methods[] = {
 		NULL, register_endpoint) },
 	{ GDBUS_METHOD("UnregisterEndpoint",
 		GDBUS_ARGS({ "endpoint", "o" }), NULL, unregister_endpoint) },
-#if 0
-	{ GDBUS_METHOD("RegisterPlayer",
+	{ GDBUS_EXPERIMENTAL_METHOD("RegisterPlayer",
 		GDBUS_ARGS({ "player", "o" }, { "properties", "a{sv}" },
 						{ "metadata", "a{sv}" }),
 		NULL, register_player) },
-	{ GDBUS_METHOD("UnregisterPlayer",
+	{ GDBUS_EXPERIMENTAL_METHOD("UnregisterPlayer",
 		GDBUS_ARGS({ "player", "o" }), NULL, unregister_player) },
-#endif
 	{ },
 };
 
@@ -1559,10 +1555,8 @@ static void path_free(void *data)
 	while (adapter->endpoints)
 		release_endpoint(adapter->endpoints->data);
 
-#if 0
 	while (adapter->players)
 		media_player_destroy(adapter->players->data);
-#endif
 
 	adapters = g_slist_remove(adapters, adapter);
 
-- 
1.7.11.7


^ permalink raw reply related

* [PATCH BlueZ 5/8] gdbus: Call check_signals when sending signals with g_dbus_send_message
From: Luiz Augusto von Dentz @ 2012-12-23 20:15 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1356293718-9348-1-git-send-email-luiz.dentz@gmail.com>

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

If message passed to g_dbus_send_message is a signal verify if it is a
valid and there really exists an interface with respective signal name.
---
 gdbus/object.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/gdbus/object.c b/gdbus/object.c
index d864bf5..565dd6f 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -1514,6 +1514,15 @@ gboolean g_dbus_send_message(DBusConnection *connection, DBusMessage *message)
 
 	if (dbus_message_get_type(message) == DBUS_MESSAGE_TYPE_METHOD_CALL)
 		dbus_message_set_no_reply(message, TRUE);
+	else if (dbus_message_get_type(message) == DBUS_MESSAGE_TYPE_SIGNAL) {
+		const char *path = dbus_message_get_path(message);
+		const char *interface = dbus_message_get_interface(message);
+		const char *name = dbus_message_get_member(message);
+		const GDBusArgInfo *args;
+
+		if (!check_signal(connection, path, interface, name, &args))
+			return FALSE;
+	}
 
 	result = dbus_connection_send(connection, message, NULL);
 
-- 
1.7.11.7


^ permalink raw reply related

* [PATCH BlueZ 4/8] gdbus: Check if the interface being registered is valid
From: Luiz Augusto von Dentz @ 2012-12-23 20:15 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1356293718-9348-1-git-send-email-luiz.dentz@gmail.com>

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This prevent registering interfaces that are empty or have all members
marked as experiemental.
---
 gdbus/object.c | 42 ++++++++++++++++++++++++++++++++++++------
 1 file changed, 36 insertions(+), 6 deletions(-)

diff --git a/gdbus/object.c b/gdbus/object.c
index 7b58eea..d864bf5 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -1177,7 +1177,7 @@ static const GDBusSignalTable manager_signals[] = {
 	{ }
 };
 
-static void add_interface(struct generic_data *data,
+static gboolean add_interface(struct generic_data *data,
 				const char *name,
 				const GDBusMethodTable *methods,
 				const GDBusSignalTable *signals,
@@ -1186,7 +1186,32 @@ static void add_interface(struct generic_data *data,
 				GDBusDestroyFunction destroy)
 {
 	struct interface_data *iface;
+	const GDBusMethodTable *method;
+	const GDBusSignalTable *signal;
+	const GDBusPropertyTable *property;
+
+	for (method = methods; method && method->name; method++) {
+		if (!check_experimental(method->flags,
+					G_DBUS_METHOD_FLAG_EXPERIMENTAL))
+			goto done;
+	}
+
+	for (signal = signals; signal && signal->name; signal++) {
+		if (!check_experimental(signal->flags,
+					G_DBUS_SIGNAL_FLAG_EXPERIMENTAL))
+			goto done;
+	}
+
+	for (property = properties; property && property->name; property++) {
+		if (!check_experimental(property->flags,
+					G_DBUS_PROPERTY_FLAG_EXPERIMENTAL))
+			goto done;
+	}
 
+	/* Nothing to register */
+	return FALSE;
+
+done:
 	iface = g_new0(struct interface_data, 1);
 	iface->name = g_strdup(name);
 	iface->methods = methods;
@@ -1197,13 +1222,15 @@ static void add_interface(struct generic_data *data,
 
 	data->interfaces = g_slist_append(data->interfaces, iface);
 	if (data->parent == NULL)
-		return;
+		return TRUE;
 
 	data->added = g_slist_append(data->added, iface);
 	if (data->process_id > 0)
-		return;
+		return TRUE;
 
 	data->process_id = g_idle_add(process_changes, data);
+
+	return TRUE;
 }
 
 static struct generic_data *object_path_ref(DBusConnection *connection,
@@ -1364,15 +1391,18 @@ gboolean g_dbus_register_interface(DBusConnection *connection,
 		return FALSE;
 	}
 
+	if (!add_interface(data, name, methods, signals, properties, user_data,
+								destroy)) {
+		object_path_unref(connection, path);
+		return FALSE;
+	}
+
 	if (properties != NULL && !find_interface(data->interfaces,
 						DBUS_INTERFACE_PROPERTIES))
 		add_interface(data, DBUS_INTERFACE_PROPERTIES,
 				properties_methods, properties_signals, NULL,
 				data, NULL);
 
-	add_interface(data, name, methods, signals, properties, user_data,
-								destroy);
-
 	g_free(data->introspect);
 	data->introspect = NULL;
 
-- 
1.7.11.7


^ permalink raw reply related

* [PATCH BlueZ 3/8] gdbus: Introduce G_DBUS_PROPERTY_FLAG_EXPERIMENTAL
From: Luiz Augusto von Dentz @ 2012-12-23 20:15 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1356293718-9348-1-git-send-email-luiz.dentz@gmail.com>

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This flag can be used to mark properties as experimental, the marked
properties with this flag can be enabled by setting the environment variable
GDBUS_EXPERIMENTAL=1
---
 gdbus/gdbus.h  |  3 ++-
 gdbus/object.c | 60 ++++++++++++++++++++++++++++++++++++----------------------
 2 files changed, 39 insertions(+), 24 deletions(-)

diff --git a/gdbus/gdbus.h b/gdbus/gdbus.h
index e6b51cd..da3cd53 100644
--- a/gdbus/gdbus.h
+++ b/gdbus/gdbus.h
@@ -101,7 +101,8 @@ enum GDBusSignalFlags {
 };
 
 enum GDBusPropertyFlags {
-	G_DBUS_PROPERTY_FLAG_DEPRECATED = (1 << 0),
+	G_DBUS_PROPERTY_FLAG_DEPRECATED   = (1 << 0),
+	G_DBUS_PROPERTY_FLAG_EXPERIMENTAL = (1 << 1),
 };
 
 enum GDBusSecurityFlags {
diff --git a/gdbus/object.c b/gdbus/object.c
index 20f7db9..7b58eea 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -118,6 +118,18 @@ static void print_arguments(GString *gstr, const GDBusArgInfo *args,
 #define G_DBUS_ANNOTATE_NOREPLY(prefix_) \
 	G_DBUS_ANNOTATE(prefix_, "Method.NoReply", "true")
 
+static gboolean check_experimental(int flags, int flag)
+{
+	const char *env;
+
+	if (!(flags & flag))
+		return FALSE;
+
+	env = g_getenv("GDBUS_EXPERIMENTAL");
+
+	return g_strcmp0(env, "1") != 0;
+}
+
 static void generate_interface_xml(GString *gstr, struct interface_data *iface)
 {
 	const GDBusMethodTable *method;
@@ -129,14 +141,10 @@ static void generate_interface_xml(GString *gstr, struct interface_data *iface)
 						G_DBUS_METHOD_FLAG_DEPRECATED;
 		gboolean noreply = method->flags &
 						G_DBUS_METHOD_FLAG_NOREPLY;
-		gboolean experimental = method->flags &
-					G_DBUS_METHOD_FLAG_EXPERIMENTAL;
 
-		if (experimental) {
-			const char *env = g_getenv("GDBUS_EXPERIMENTAL");
-			if (g_strcmp0(env, "1") != 0)
-				continue;
-		}
+		if (check_experimental(method->flags,
+					G_DBUS_METHOD_FLAG_EXPERIMENTAL))
+			continue;
 
 		if (!deprecated && !noreply &&
 				!(method->in_args && method->in_args->name) &&
@@ -165,14 +173,10 @@ static void generate_interface_xml(GString *gstr, struct interface_data *iface)
 	for (signal = iface->signals; signal && signal->name; signal++) {
 		gboolean deprecated = signal->flags &
 						G_DBUS_SIGNAL_FLAG_DEPRECATED;
-		gboolean experimental = signal->flags &
-					G_DBUS_SIGNAL_FLAG_EXPERIMENTAL;
 
-		if (experimental) {
-			const char *env = g_getenv("GDBUS_EXPERIMENTAL");
-			if (g_strcmp0(env, "1") != 0)
-				continue;
-		}
+		if (check_experimental(signal->flags,
+					G_DBUS_SIGNAL_FLAG_EXPERIMENTAL))
+			continue;
 
 		if (!deprecated && !(signal->args && signal->args->name))
 			g_string_append_printf(gstr,
@@ -197,6 +201,10 @@ static void generate_interface_xml(GString *gstr, struct interface_data *iface)
 		gboolean deprecated = property->flags &
 					G_DBUS_PROPERTY_FLAG_DEPRECATED;
 
+		if (check_experimental(property->flags,
+					G_DBUS_PROPERTY_FLAG_EXPERIMENTAL))
+			continue;
+
 		g_string_append_printf(gstr, "\t\t<property name=\"%s\""
 					" type=\"%s\" access=\"%s%s\"",
 					property->name,	property->type,
@@ -558,6 +566,10 @@ static void append_properties(struct interface_data *data,
 				DBUS_DICT_ENTRY_END_CHAR_AS_STRING, &dict);
 
 	for (p = data->properties; p && p->name; p++) {
+		if (check_experimental(p->flags,
+					G_DBUS_PROPERTY_FLAG_EXPERIMENTAL))
+			continue;
+
 		if (p->get == NULL)
 			continue;
 
@@ -759,8 +771,14 @@ static inline const GDBusPropertyTable *find_property(const GDBusPropertyTable *
 	const GDBusPropertyTable *p;
 
 	for (p = properties; p && p->name; p++) {
-		if (strcmp(name, p->name) == 0)
-			return p;
+		if (strcmp(name, p->name) != 0)
+			continue;
+
+		if (check_experimental(p->flags,
+					G_DBUS_PROPERTY_FLAG_EXPERIMENTAL))
+			break;
+
+		return p;
 	}
 
 	return NULL;
@@ -1038,18 +1056,14 @@ static DBusHandlerResult generic_message(DBusConnection *connection,
 
 	for (method = iface->methods; method &&
 			method->name && method->function; method++) {
-		gboolean experimental = method->flags &
-					G_DBUS_METHOD_FLAG_EXPERIMENTAL;
 
 		if (dbus_message_is_method_call(message, iface->name,
 							method->name) == FALSE)
 			continue;
 
-		if (experimental) {
-			const char *env = g_getenv("GDBUS_EXPERIMENTAL");
-			if (g_strcmp0(env, "1") != 0)
-				return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
-		}
+		if (check_experimental(method->flags,
+					G_DBUS_METHOD_FLAG_EXPERIMENTAL))
+			return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
 
 		if (g_dbus_args_have_signature(method->in_args,
 							message) == FALSE)
-- 
1.7.11.7


^ permalink raw reply related

* [PATCH BlueZ 2/8] gdbus: Introduce G_DBUS_SIGNAL_FLAG_EXPERIMENTAL
From: Luiz Augusto von Dentz @ 2012-12-23 20:15 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1356293718-9348-1-git-send-email-luiz.dentz@gmail.com>

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This flag can be used to mark signals as experimental, the marked
signals with this flag can be enabled by setting the environment variable
GDBUS_EXPERIMENTAL=1
---
 gdbus/gdbus.h  |  8 +++++++-
 gdbus/object.c | 21 ++++++++++++++++++---
 2 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/gdbus/gdbus.h b/gdbus/gdbus.h
index 00fbb1c..e6b51cd 100644
--- a/gdbus/gdbus.h
+++ b/gdbus/gdbus.h
@@ -96,7 +96,8 @@ enum GDBusMethodFlags {
 };
 
 enum GDBusSignalFlags {
-	G_DBUS_SIGNAL_FLAG_DEPRECATED = (1 << 0),
+	G_DBUS_SIGNAL_FLAG_DEPRECATED   = (1 << 0),
+	G_DBUS_SIGNAL_FLAG_EXPERIMENTAL = (1 << 1),
 };
 
 enum GDBusPropertyFlags {
@@ -204,6 +205,11 @@ struct GDBusSecurityTable {
 	.args = _args, \
 	.flags = G_DBUS_SIGNAL_FLAG_DEPRECATED
 
+#define GDBUS_EXPERIMENTAL_SIGNAL(_name, _args) \
+	.name = _name, \
+	.args = _args, \
+	.flags = G_DBUS_SIGNAL_FLAG_EXPERIMENTAL
+
 gboolean g_dbus_register_interface(DBusConnection *connection,
 					const char *path, const char *name,
 					const GDBusMethodTable *methods,
diff --git a/gdbus/object.c b/gdbus/object.c
index 30dbbc2..20f7db9 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -165,6 +165,14 @@ static void generate_interface_xml(GString *gstr, struct interface_data *iface)
 	for (signal = iface->signals; signal && signal->name; signal++) {
 		gboolean deprecated = signal->flags &
 						G_DBUS_SIGNAL_FLAG_DEPRECATED;
+		gboolean experimental = signal->flags &
+					G_DBUS_SIGNAL_FLAG_EXPERIMENTAL;
+
+		if (experimental) {
+			const char *env = g_getenv("GDBUS_EXPERIMENTAL");
+			if (g_strcmp0(env, "1") != 0)
+				continue;
+		}
 
 		if (!deprecated && !(signal->args && signal->args->name))
 			g_string_append_printf(gstr,
@@ -1267,10 +1275,17 @@ static gboolean check_signal(DBusConnection *conn, const char *path,
 	}
 
 	for (signal = iface->signals; signal && signal->name; signal++) {
-		if (!strcmp(signal->name, name)) {
-			*args = signal->args;
-			return TRUE;
+		if (strcmp(signal->name, name) != 0)
+			continue;
+
+		if (signal->flags & G_DBUS_SIGNAL_FLAG_EXPERIMENTAL) {
+			const char *env = g_getenv("GDBUS_EXPERIMENTAL");
+			if (g_strcmp0(env, "1") != 0)
+				break;
 		}
+
+		*args = signal->args;
+		return TRUE;
 	}
 
 	error("No signal named %s on interface %s", name, interface);
-- 
1.7.11.7


^ permalink raw reply related


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