Linux bluetooth development
 help / color / mirror / Atom feed
* [PATCH 1/2] Add master/slave parsing for le_conn_complete event
From: Ville Tervo @ 2010-09-29 13:56 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Ville Tervo

---
 parser/hci.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/parser/hci.c b/parser/hci.c
index 250ba58..7f23605 100644
--- a/parser/hci.c
+++ b/parser/hci.c
@@ -3239,7 +3239,8 @@ static inline void evt_le_conn_complete_dump(int level, struct frame *frm)
 	evt_le_connection_complete *evt = frm->ptr;
 
 	p_indent(level, frm);
-	printf("status 0x%2.2x handle %d\n", evt->status, btohs(evt->handle));
+	printf("status 0x%2.2x handle %d, role %s\n",
+		evt->status, btohs(evt->handle), evt->role?"slave":"master");
 }
 
 static inline void le_meta_ev_dump(int level, struct frame *frm)
-- 
1.7.1


^ permalink raw reply related

* Re: [PATCH 1/4] Add the Attribute interface to the API
From: Johan Hedberg @ 2010-09-29 13:49 UTC (permalink / raw)
  To: Claudio Takahasi; +Cc: linux-bluetooth, Vinicius Costa Gomes
In-Reply-To: <1285696155-32766-1-git-send-email-claudio.takahasi@openbossa.org>

Hi,

On Tue, Sep 28, 2010, Claudio Takahasi wrote:
> +Attribute Protocol hierarchy
> +============================
> +
> +Service		org.bluez
> +Interface	org.bluez.Attribute
> +Object path	[prefix]/{hci0}/{device0}
> +
> +
> +Methods		dict GetProperties()
> +
> +			Returns all properties for the interface. See the
> +			properties section for available properties.
> +
> +Properties	array{object} Services
> +
> +			List of all the Primary Services that this device
> +			implements.
> +
> +
>  Device Service hierarchy
>  ========================
>  
> diff --git a/doc/device-api.txt b/doc/device-api.txt
> index 95b5b22..b818299 100644
> --- a/doc/device-api.txt
> +++ b/doc/device-api.txt
> @@ -139,10 +139,6 @@ Properties	string Address [readonly]
>  			List of 128-bit UUIDs that represents the available
>  			remote services.
>  
> -		array{object} Services [readonly]
> -
> -			List of characteristics based services.
> -
>  		boolean Paired [readonly]
>  
>  			Indicates if the remote device is paired.

What's the motivation of moving this into its own D-Bus interface? I
thought the plan was to abstract both traditional SDP and ATT behind the
same API in which case having this in the Device interface makes more
sense imho.

Johan

^ permalink raw reply

* Re: Pull request: git://git.infradead.org/users/cktakahasi/bluez.git for-upstream
From: Johan Hedberg @ 2010-09-29 13:45 UTC (permalink / raw)
  To: Claudio Takahasi; +Cc: BlueZ development
In-Reply-To: <AANLkTindKFz1fx9cJMu9aSpehG5LaUHHO7TkgWqMzqj=@mail.gmail.com>

Hi Claudio,

On Thu, Sep 23, 2010, Claudio Takahasi wrote:
>       Rename hciops {start, stop}_discovery to {start, stop}_inquiry

I'm not sure about this patch. Wasn't the idea to hide the device
discovery details from the higher layers? I.e. the method could be
called discovery and it'd take care of both BR/EDR and LE discovery
methods.

Johan

^ permalink raw reply

* Re: [PATCH] Bluetooth: Replace hard code of configuration continuation flag.
From: haijun liu @ 2010-09-29 13:33 UTC (permalink / raw)
  To: Ville Tervo
  Cc: Gustavo F. Padovan, linux-bluetooth@vger.kernel.org,
	Mat Martineau, dantian.ip
In-Reply-To: <20100929131229.GK1931@null>

Replace hard code of configuration continuation flag with self-comment macro.


Signed-off-by: Haijun.Liu <haijun.liu@atheros.com>
---
 net/bluetooth/l2cap_core.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index efcf510..279f98a 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -2985,11 +2985,11 @@ static inline int l2cap_config_req(struct
l2cap_conn *conn, struct l2cap_cmd_hdr
 	memcpy(l2cap_pi(sk)->conf_req + l2cap_pi(sk)->conf_len, req->data, len);
 	l2cap_pi(sk)->conf_len += len;

-	if (flags & 0x0001) {
+	if (flags & L2CAP_CONF_FLAG_CONT) {
 		/* Incomplete config. Send empty response. */
 		l2cap_send_cmd(conn, cmd->ident, L2CAP_CONF_RSP,
 				l2cap_build_conf_rsp(sk, rsp,
-					L2CAP_CONF_SUCCESS, 0x0001), rsp);
+					L2CAP_CONF_SUCCESS, L2CAP_CONF_FLAG_CONT), rsp);
 		goto unlock;
 	}

@@ -3092,7 +3092,7 @@ static inline int l2cap_config_rsp(struct
l2cap_conn *conn, struct l2cap_cmd_hdr
 		goto done;
 	}

-	if (flags & 0x01)
+	if (flags & L2CAP_CONF_FLAG_CONT)
 		goto done;

 	l2cap_pi(sk)->conf_state |= L2CAP_CONF_INPUT_DONE;
-- 
1.6.3.3

^ permalink raw reply related

* Re: [PATCH] Bluetooth: Replace hard code of configuration continuation flag.
From: Ville Tervo @ 2010-09-29 13:12 UTC (permalink / raw)
  To: ext haijun liu
  Cc: Gustavo F. Padovan, linux-bluetooth@vger.kernel.org,
	Mat Martineau, dantian.ip
In-Reply-To: <AANLkTi=tyZ4rsn8QtqV8b04=f=1QA9qwJW03PQzroJ7O@mail.gmail.com>

Hi,

On Wed, Sep 29, 2010 at 02:42:35PM +0200, ext haijun liu wrote:
> Hi Ville,
> Do you mean l2cap_config_rsp()?

No. I meant change like this in addition to your changes.

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index efcf510..7a9c194 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -2989,7 +2989,7 @@ static inline int l2cap_config_req(struct l2cap_conn *conn, struct l2cap_cmd_hdr
                /* Incomplete config. Send empty response. */
                l2cap_send_cmd(conn, cmd->ident, L2CAP_CONF_RSP,
                                l2cap_build_conf_rsp(sk, rsp,
-                                       L2CAP_CONF_SUCCESS, 0x0001), rsp);
+                                       L2CAP_CONF_SUCCESS, L2CAP_CONF_FLAG_CONT), rsp);
                goto unlock;
        }


> I made it, but I don't why gmail hide that part.

I got the whole message.

-- 
Ville

^ permalink raw reply related

* Re: [PATCH] Bluetooth: Replace hard code of configuration continuation flag.
From: haijun liu @ 2010-09-29 12:42 UTC (permalink / raw)
  To: Ville Tervo
  Cc: Gustavo F. Padovan, linux-bluetooth@vger.kernel.org,
	Mat Martineau, dantian.ip
In-Reply-To: <20100929113615.GH1931@null>

Hi Ville,
Do you mean l2cap_config_rsp()?
I made it, but I don't why gmail hide that part.

-- 
Haijun Liu

^ permalink raw reply

* HDP development status and collaboration
From: Elvis Pfützenreuter @ 2010-09-29 12:14 UTC (permalink / raw)
  To: linux-bluetooth
  Cc: Jarvenpaa Jarmo (Nokia/Oulu), Aldenor Martins, Johan Hedberg,
	Jose Antonio Santos Cadenas, Santiago Carot-Nemesio

[-- Attachment #1: Type: text/plain, Size: 963 bytes --]

Attached is a spreadsheet with a simple breakdown of HDP development tasks. Managers like spreadsheets, most of us don't, so the most relevant part -- the pending tasks -- are listed below for convenience:

device.Echo			PENDING
device.CreateChannel		IN PROGRESS (OpenHealth)
device.DestroyChannel		PENDING
device.ChannelConnected		PENDING
device.ChannelDeleted		PENDING
device properties		PENDING
Channel object			PENDING
channel.Acquire			PENDING
channel.Release			PENDING
channel properties		PENDING
HDP verification against PTS	PENDING	(depends on the rest)

I'd say that we are getting near completion. Would be nearer if we could distribute those tasks; currently, only OpenHealth friends are working on HDP code, in their repository.

I know it's sometimes difficult to collaborate while working on the very same source files, but we should try; the breakdown by API calls seem to be the way to divide work among available developers.



[-- Attachment #2: hdp_development.xls --]
[-- Type: application/vnd.ms-excel, Size: 10752 bytes --]

^ permalink raw reply

* Re: a2dp, myth, pulse
From: Sander van Grieken @ 2010-09-29 11:37 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <AANLkTim27JkfgsumEwd3jK+n1DzYvTYWJGCLGreuw7y3@mail.gmail.com>

On Tuesday 28 September 2010 09:05:18 Brad Midgley wrote:
> Hey
> 
> > can avrcp be used to drive mythtv? I've got a setup where the myth
> > frontend is rarely the foreground app
> 
> So if avrcp were to present events in a /dev/input/eventX device, I
> could use inputlircd to connect those events to lirc clients. It might
> make it easier if a udev rule created something like /dev/input/avrcp0
> so we could find it more easily for the inputlircd config.
> 
> And I see now that it is by virtue of the use of liblirc that myth can
> get remote events even if it's not the foreground app.
> 
> Control is enabled by default in 4.60, is there anything I can check
> to see why I don't see any log messages about avrcp, no input device
> appear, nothing logged, etc? The main connection is initiated by the
> headset and the audio connection is initiated by the computer. If I
> remember tinkering with this stuff, bluez would need to initiate the
> control connection in this case.

Currently there is support in bluez to act as a AVRCP target, and it will deliver these 
events through the input layer. No new input device will appear, it will go through 
/dev/uinput (which I think you can open for reading then)


The Control connection is ONLY (implicitly) established when an audio connection 
(HS,HF,A2DP) is succesfully started, so you should be able to control myth (through lirc 
perhaps) with a headset that has some controls. If this is done, you should be able to see 
some debug logging w.r.t. RCP. You probably need to run bluetoothd using -d -n.


--
Sander

^ permalink raw reply

* Re: [PATCH] Bluetooth: Replace hard code of configuration continuation flag.
From: Ville Tervo @ 2010-09-29 11:36 UTC (permalink / raw)
  To: ext haijun liu
  Cc: Gustavo F. Padovan, linux-bluetooth@vger.kernel.org,
	Mat Martineau, dantian.ip
In-Reply-To: <AANLkTi=EVEwuKy3QwNJ1sVpe2_EJCkQQ3iGutKQQRneU@mail.gmail.com>

Hi,

Patch is ok but just one comment. See below.

On Wed, Sep 29, 2010 at 01:12:39PM +0200, haijun liu wrote:
> 
>  Replace hard code of configuration continuation flag with self-comment macro.
> 
> -	if (flags & 0x0001) {
> +	if (flags & L2CAP_CONF_FLAG_CONT) {
>  		/* Incomplete config. Send empty response. */
>  		l2cap_send_cmd(conn, cmd->ident, L2CAP_CONF_RSP,
>  				l2cap_build_conf_rsp(sk, rsp,

Maybe you could use L2CAP_CONF_FLAG_CONT also in l2cap_build_conf_rsp()?

-- 
Ville

^ permalink raw reply

* Re: [PATCH] Bluetooth: Replace hard code of configuration continuation flag.
From: haijun liu @ 2010-09-29 11:12 UTC (permalink / raw)
  To: Gustavo F. Padovan; +Cc: linux-bluetooth, Mat Martineau, dantian.ip
In-Reply-To: <20100928234820.GB8518@vigoh>

 Replace hard code of configuration continuation flag with self-comment macro.


Signed-off-by: Haijun.Liu <haijun.liu@atheros.com>
---
 include/net/bluetooth/l2cap.h |    2 ++
 net/bluetooth/l2cap_core.c    |    4 ++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index df599dc..2b114ca 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -193,6 +193,8 @@ struct l2cap_conf_rsp {
 #define L2CAP_CONF_REJECT	0x0002
 #define L2CAP_CONF_UNKNOWN	0x0003

+#define L2CAP_CONF_FLAG_CONT	0x0001
+
 struct l2cap_conf_opt {
 	__u8       type;
 	__u8       len;
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index efcf510..bef5c9f 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -2985,7 +2985,7 @@ static inline int l2cap_config_req(struct
l2cap_conn *conn, struct l2cap_cmd_hdr
 	memcpy(l2cap_pi(sk)->conf_req + l2cap_pi(sk)->conf_len, req->data, len);
 	l2cap_pi(sk)->conf_len += len;

-	if (flags & 0x0001) {
+	if (flags & L2CAP_CONF_FLAG_CONT) {
 		/* Incomplete config. Send empty response. */
 		l2cap_send_cmd(conn, cmd->ident, L2CAP_CONF_RSP,
 				l2cap_build_conf_rsp(sk, rsp,
@@ -3092,7 +3092,7 @@ static inline int l2cap_config_rsp(struct
l2cap_conn *conn, struct l2cap_cmd_hdr
 		goto done;
 	}

-	if (flags & 0x01)
+	if (flags & L2CAP_CONF_FLAG_CONT)
 		goto done;

 	l2cap_pi(sk)->conf_state |= L2CAP_CONF_INPUT_DONE;
-- 
1.6.3.3

^ permalink raw reply related

* Re: [PATCH 2/2] Bluetooth: Update conf_state before send config_req out.
From: haijun liu @ 2010-09-29 11:11 UTC (permalink / raw)
  To: Gustavo F. Padovan; +Cc: linux-bluetooth@vger.kernel.org, dantian.ip
In-Reply-To: <20100928234934.GC8518@vigoh>

 Update conf_state with L2CAP_CONF_REQ_SENT before send
 config_req out in l2cap_config_req().


Signed-off-by: Haijun.Liu <haijun.liu@atheros.com>
---
 net/bluetooth/l2cap_core.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index bef5c9f..f461fee 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -3026,6 +3026,7 @@ static inline int l2cap_config_req(struct
l2cap_conn *conn, struct l2cap_cmd_hdr

 	if (!(l2cap_pi(sk)->conf_state & L2CAP_CONF_REQ_SENT)) {
 		u8 buf[64];
+		l2cap_pi(sk)->conf_state |= L2CAP_CONF_REQ_SENT;
 		l2cap_send_cmd(conn, l2cap_get_ident(conn), L2CAP_CONF_REQ,
 					l2cap_build_conf_req(sk, buf), buf);
 		l2cap_pi(sk)->num_conf_req++;
-- 
1.6.3.3

^ permalink raw reply related

* Re: [PATCH 4/6] Bluetooth: Fix inconsistent lock state with RFCOMM
From: Yuri Kululin @ 2010-09-29  7:41 UTC (permalink / raw)
  To: ext Gustavo F. Padovan; +Cc: linux-bluetooth, Gustavo F. Padovan
In-Reply-To: <1285376504-3541-5-git-send-email-gustavo@padovan.org>

ext Gustavo F. Padovan wrote:
> From: Gustavo F. Padovan <padovan@profusion.mobi>
> 
> When receiving a rfcomm connection with the old dund deamon a
> inconsistent lock state happens. That's because interrupts were already
> disabled by l2cap_conn_start() when rfcomm_sk_state_change() try to lock
> the spin_lock.
> 
> As result we may have a inconsistent lock state for l2cap_conn_start()
> after rfcomm_sk_state_change() calls bh_lock_sock() and disable interrupts
> as well.
> 
> [ 2833.151999]
> [ 2833.151999] =================================
> [ 2833.151999] [ INFO: inconsistent lock state ]
> [ 2833.151999] 2.6.36-rc3 #2
> [ 2833.151999] ---------------------------------
> [ 2833.151999] inconsistent {IN-SOFTIRQ-W} -> {SOFTIRQ-ON-W} usage.
> [ 2833.151999] krfcommd/2306 [HC0[0]:SC0[0]:HE1:SE1] takes:
> [ 2833.151999]  (slock-AF_BLUETOOTH){+.?...}, at: [<ffffffffa00bcb56>] rfcomm_sk_state_change+0x46/0x170 [rfcomm]
> [ 2833.151999] {IN-SOFTIRQ-W} state was registered at:
> [ 2833.151999]   [<ffffffff81094346>] __lock_acquire+0x5b6/0x1560
> [ 2833.151999]   [<ffffffff8109534a>] lock_acquire+0x5a/0x70
> [ 2833.151999]   [<ffffffff81392b6c>] _raw_spin_lock+0x2c/0x40
> [ 2833.151999]   [<ffffffffa00a5092>] l2cap_conn_start+0x92/0x640 [l2cap]
> [ 2833.151999]   [<ffffffffa00a6a3f>] l2cap_sig_channel+0x6bf/0x1320 [l2cap]
> [ 2833.151999]   [<ffffffffa00a9173>] l2cap_recv_frame+0x133/0x770 [l2cap]
> [ 2833.151999]   [<ffffffffa00a997b>] l2cap_recv_acldata+0x1cb/0x390 [l2cap]
> [ 2833.151999]   [<ffffffffa000db4b>] hci_rx_task+0x2ab/0x450 [bluetooth]
> [ 2833.151999]   [<ffffffff8106b22b>] tasklet_action+0xcb/0xe0
> [ 2833.151999]   [<ffffffff8106b91e>] __do_softirq+0xae/0x150
> [ 2833.151999]   [<ffffffff8102bc0c>] call_softirq+0x1c/0x30
> [ 2833.151999]   [<ffffffff8102ddb5>] do_softirq+0x75/0xb0
> [ 2833.151999]   [<ffffffff8106b56d>] irq_exit+0x8d/0xa0
> [ 2833.151999]   [<ffffffff8104484b>] smp_apic_timer_interrupt+0x6b/0xa0
> [ 2833.151999]   [<ffffffff8102b6d3>] apic_timer_interrupt+0x13/0x20
> [ 2833.151999]   [<ffffffff81029dfa>] cpu_idle+0x5a/0xb0
> [ 2833.151999]   [<ffffffff81381ded>] rest_init+0xad/0xc0
> [ 2833.151999]   [<ffffffff817ebc4d>] start_kernel+0x2dd/0x2e8
> [ 2833.151999]   [<ffffffff817eb2e6>] x86_64_start_reservations+0xf6/0xfa
> [ 2833.151999]   [<ffffffff817eb3ce>] x86_64_start_kernel+0xe4/0xeb
> [ 2833.151999] irq event stamp: 731
> [ 2833.151999] hardirqs last  enabled at (731): [<ffffffff8106b762>] local_bh_enable_ip+0x82/0xe0
> [ 2833.151999] hardirqs last disabled at (729): [<ffffffff8106b93e>] __do_softirq+0xce/0x150
> [ 2833.151999] softirqs last  enabled at (730): [<ffffffff8106b96e>] __do_softirq+0xfe/0x150
> [ 2833.151999] softirqs last disabled at (711): [<ffffffff8102bc0c>] call_softirq+0x1c/0x30
> [ 2833.151999]
> [ 2833.151999] other info that might help us debug this:
> [ 2833.151999] 2 locks held by krfcommd/2306:
> [ 2833.151999]  #0:  (rfcomm_mutex){+.+.+.}, at: [<ffffffffa00bb744>] rfcomm_run+0x174/0xb20 [rfcomm]
> [ 2833.151999]  #1:  (&(&d->lock)->rlock){+.+...}, at: [<ffffffffa00b9223>] rfcomm_dlc_accept+0x53/0x100 [rfcomm]
> [ 2833.151999]
> [ 2833.151999] stack backtrace:
> [ 2833.151999] Pid: 2306, comm: krfcommd Tainted: G        W   2.6.36-rc3 #2
> [ 2833.151999] Call Trace:
> [ 2833.151999]  [<ffffffff810928e1>] print_usage_bug+0x171/0x180
> [ 2833.151999]  [<ffffffff810936c3>] mark_lock+0x333/0x400
> [ 2833.151999]  [<ffffffff810943ca>] __lock_acquire+0x63a/0x1560
> [ 2833.151999]  [<ffffffff810948b5>] ? __lock_acquire+0xb25/0x1560
> [ 2833.151999]  [<ffffffff8109534a>] lock_acquire+0x5a/0x70
> [ 2833.151999]  [<ffffffffa00bcb56>] ? rfcomm_sk_state_change+0x46/0x170 [rfcomm]
> [ 2833.151999]  [<ffffffff81392b6c>] _raw_spin_lock+0x2c/0x40
> [ 2833.151999]  [<ffffffffa00bcb56>] ? rfcomm_sk_state_change+0x46/0x170 [rfcomm]
> [ 2833.151999]  [<ffffffffa00bcb56>] rfcomm_sk_state_change+0x46/0x170 [rfcomm]
> [ 2833.151999]  [<ffffffffa00b9239>] rfcomm_dlc_accept+0x69/0x100 [rfcomm]
> [ 2833.151999]  [<ffffffffa00b9a49>] rfcomm_check_accept+0x59/0xd0 [rfcomm]
> [ 2833.151999]  [<ffffffffa00bacab>] rfcomm_recv_frame+0x9fb/0x1320 [rfcomm]
> [ 2833.151999]  [<ffffffff813932bb>] ? _raw_spin_unlock_irqrestore+0x3b/0x60
> [ 2833.151999]  [<ffffffff81093acd>] ? trace_hardirqs_on_caller+0x13d/0x180
> [ 2833.151999]  [<ffffffff81093b1d>] ? trace_hardirqs_on+0xd/0x10
> [ 2833.151999]  [<ffffffffa00bb7f1>] rfcomm_run+0x221/0xb20 [rfcomm]
> [ 2833.151999]  [<ffffffff813905e7>] ? schedule+0x287/0x780
> [ 2833.151999]  [<ffffffffa00bb5d0>] ? rfcomm_run+0x0/0xb20 [rfcomm]
> [ 2833.151999]  [<ffffffff81081026>] kthread+0x96/0xa0
> [ 2833.151999]  [<ffffffff8102bb14>] kernel_thread_helper+0x4/0x10
> [ 2833.151999]  [<ffffffff813936bc>] ? restore_args+0x0/0x30
> [ 2833.151999]  [<ffffffff81080f90>] ? kthread+0x0/0xa0
> [ 2833.151999]  [<ffffffff8102bb10>] ? kernel_thread_helper+0x0/0x10
> 
> Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
> ---
>  net/bluetooth/rfcomm/sock.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
> index 44a6232..194b3a0 100644
> --- a/net/bluetooth/rfcomm/sock.c
> +++ b/net/bluetooth/rfcomm/sock.c
> @@ -82,11 +82,14 @@ static void rfcomm_sk_data_ready(struct rfcomm_dlc *d, struct sk_buff *skb)
>  static void rfcomm_sk_state_change(struct rfcomm_dlc *d, int err)
>  {
>  	struct sock *sk = d->owner, *parent;
> +	unsigned long flags;
> +
>  	if (!sk)
>  		return;
>  
>  	BT_DBG("dlc %p state %ld err %d", d, d->state, err);
>  
> +	local_irq_save(flags);
>  	bh_lock_sock(sk);
>  
>  	if (err)
> @@ -108,6 +111,7 @@ static void rfcomm_sk_state_change(struct rfcomm_dlc *d, int err)
>  	}
>  
>  	bh_unlock_sock(sk);
> +	local_irq_restore(flags);
>  
>  	if (parent && sock_flag(sk, SOCK_ZAPPED)) {
>  		/* We have to drop DLC lock here, otherwise

I've faced with the similar warning and looks like this patch fixes the issue.

[ 2917.827178] =================================
[ 2917.833068] [ INFO: inconsistent lock state ]
[ 2917.837432] 2.6.32-09421-g65e7ba7 #54
[ 2917.841125] ---------------------------------
[ 2917.845520] inconsistent {IN-SOFTIRQ-W} -> {SOFTIRQ-ON-W} usage.
[ 2917.851562] krfcommd/1516 [HC0[0]:SC0[0]:HE1:SE1] takes:
[ 2917.856903]  (slock-AF_BLUETOOTH){+.?...}, at: [<bf057b50>]
rfcomm_sk_state_change+0x78/0x160 [rfcomm]
[ 2917.866363] {IN-SOFTIRQ-W} state was registered at:
[ 2917.871276]   [<c008d33c>] mark_lock+0x298/0x630
[ 2917.875946]   [<c008ed3c>] __lock_acquire+0x5f4/0x175c
[ 2917.881134]   [<c008ff0c>] lock_acquire+0x68/0x7c
[ 2917.885864]   [<c036938c>] _spin_lock+0x48/0x58
[ 2917.890441]   [<bf025960>] l2cap_conn_start+0x80/0x388 [l2cap]
[ 2917.896362]   [<bf028f44>] l2cap_recv_frame+0x1c58/0x2fe0 [l2cap]
[ 2917.902526]   [<bf02a3cc>] l2cap_recv_acldata+0x100/0x350 [l2cap]
[ 2917.908691]   [<bf0035a8>] hci_rx_task+0x244/0x478 [bluetooth]
[ 2917.914642]   [<c006c4d0>] tasklet_action+0x78/0xd8
[ 2917.919555]   [<c006cc34>] __do_softirq+0xa8/0x154
[ 2917.924407]   [<c006cd40>] irq_exit+0x60/0xb4
[ 2917.928802]   [<c0030078>] asm_do_IRQ+0x78/0x90
[ 2917.933380]   [<c0030af0>] __irq_svc+0x50/0xbc
[ 2917.937866]   [<c0043c74>] omap3_enter_idle_bm+0x1d0/0x238
[ 2917.943389]   [<c029ee94>] cpuidle_idle_call+0xb4/0x114
[ 2917.948669]   [<c00320b0>] cpu_idle+0x58/0xac
[ 2917.953063]   [<c0360b18>] rest_init+0x70/0x84
[ 2917.957550]   [<c00089fc>] start_kernel+0x2b4/0x318
[ 2917.962493]   [<80008034>] 0x80008034
[ 2917.966186] irq event stamp: 312
[ 2917.969421] hardirqs last  enabled at (312): [<c03691ac>]
_spin_unlock_irqrestore+0x44/0x70
[ 2917.977844] hardirqs last disabled at (311): [<c036947c>]
_spin_lock_irqsave+0x24/0x68
[ 2917.985809] softirqs last  enabled at (261): [<c006ccc8>]
__do_softirq+0x13c/0x154
[ 2917.993438] softirqs last disabled at (244): [<c006cde8>]
do_softirq+0x54/0x78
[ 2918.000732]
[ 2918.000732] other info that might help us debug this:
[ 2918.007293] 2 locks held by krfcommd/1516:
[ 2918.011413]  #0:  (rfcomm_mutex){+.+.+.}, at: [<bf054df4>]
rfcomm_run+0x1f0/0xb00 [rfcomm]
[ 2918.019805]  #1:  (&d->lock){+.+...}, at: [<bf055220>]
rfcomm_run+0x61c/0xb00 [rfcomm]
[ 2918.027832]
[ 2918.027832] stack backtrace:
[ 2918.032226] Backtrace:
[ 2918.034729] [<c00348d0>] (dump_backtrace+0x0/0x110) from [<c036616c>]
(dump_stack+0x18/0x1c)
[ 2918.043212]  r7:dc8f6c00 r6:c0425252 r5:00000001 r4:00000001
[ 2918.048950] [<c0366154>] (dump_stack+0x0/0x1c) from [<c008d060>]
(print_usage_bug+0x178/0x1bc)
[ 2918.057617] [<c008cee8>] (print_usage_bug+0x0/0x1bc) from [<c008d408>]
(mark_lock+0x364/0x630)
[ 2918.066284] [<c008d0a4>] (mark_lock+0x0/0x630) from [<c008edcc>]
(__lock_acquire+0x684/0x175c)
[ 2918.074951] [<c008e748>] (__lock_acquire+0x0/0x175c) from [<c008ff0c>]
(lock_acquire+0x68/0x7c)
[ 2918.083709] [<c008fea4>] (lock_acquire+0x0/0x7c) from [<c036938c>]
(_spin_lock+0x48/0x58)
[ 2918.091949]  r7:dba9402c r6:dba5c3c0 r5:dba9402c r4:bf057b50
[ 2918.097717] [<c0369344>] (_spin_lock+0x0/0x58) from [<bf057b50>]
(rfcomm_sk_state_change+0x78/0x160 [rfcomm])
[ 2918.107696]  r5:dba94000 r4:00000000
[ 2918.111358] [<bf057ad8>] (rfcomm_sk_state_change+0x0/0x160 [rfcomm]) from
[<bf055238>] (rfcomm_run+0x634/0xb00 [rfcomm])
[ 2918.122283]  r7:dba5c450 r6:dba5d6c0 r5:dba5c3c0 r4:dba5c430
[ 2918.128051] [<bf054c04>] (rfcomm_run+0x0/0xb00 [rfcomm]) from [<c007cc10>]
(kthread+0x88/0x90)
[ 2918.136749] [<c007cb88>] (kthread+0x0/0x90) from [<c006a86c>]
(do_exit+0x0/0x678)
[ 2918.144256]  r7:00000000 r6:00000000 r5:00000000 r4:00000000

Thanks,
Yuri K.

^ permalink raw reply

* Re: [PATCH 1/2] Bluetooth: set sk_sndbuf and sk_rcvbuf properly in ERTM
From: Gustavo F. Padovan @ 2010-09-29  0:32 UTC (permalink / raw)
  To: Mat Martineau; +Cc: linux-bluetooth
In-Reply-To: <alpine.DEB.2.00.1009271355090.27215@linux-sea-02>

Hi Mat,

* Mat Martineau <mathewm@codeaurora.org> [2010-09-27 14:22:07 -0700]:

> 
> Gustavo -
> 
> On Mon, 27 Sep 2010, Gustavo F. Padovan wrote:
> 
> > Setting both this value to MPS * TxWin * 1.2 guarantees that we are
> > reserving space to fit the whole txwindow in the memory, and that
> > sendmsg() will block when the transmission window is full avoid
> > overloading the system memory.
> > I don't have a strong reason about the 1.2 constant in the account, we
> > can do another tests in the future and change that value.
> >
> > Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
> > ---
> > net/bluetooth/l2cap_core.c |   10 +++++++++-
> > 1 files changed, 9 insertions(+), 1 deletions(-)
> >
> > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> > index 44aa034..1e2ab05 100644
> > --- a/net/bluetooth/l2cap_core.c
> > +++ b/net/bluetooth/l2cap_core.c
> > @@ -3129,9 +3129,17 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hdr
> > 		l2cap_pi(sk)->next_tx_seq = 0;
> > 		l2cap_pi(sk)->expected_tx_seq = 0;
> > 		__skb_queue_head_init(TX_QUEUE(sk));
> > -		if (l2cap_pi(sk)->mode ==  L2CAP_MODE_ERTM)
> > +		if (l2cap_pi(sk)->mode ==  L2CAP_MODE_ERTM) {
> > 			l2cap_ertm_init(sk);
> >
> > +			sk->sk_sndbuf = (l2cap_pi(sk)->remote_tx_win * 1.2 *
> > +						(sizeof(struct l2cap_pinfo) +
> > +						l2cap_pi(sk)->mps));
> > +			sk->sk_rcvbuf = (l2cap_pi(sk)->tx_win * 1.2 *
> > +						(sizeof(struct l2cap_pinfo) +
> > +						l2cap_pi(sk)->remote_mps));
> > +		}
> > +
> > 		l2cap_chan_ready(sk);
> > 	}
> >
> > -- 
> > 1.7.3

Thanks a lot for your comments, I'll rework the patches and then resend.

> 
> I think sizeof(struct sk_buff) would be better than
> sizeof(struct l2cap_pinfo), since these limits apply to data buffers, 
> not per-socket overhead.

Sure.

> 
> The 1.2 constant would need to be increased if we allow ERTM MPS
> bigger than the HCI MTU, since there would be multiple sk_buffs per 
> PDU.  However, the calculation could be updated when those MPS changes 
> are made.

Yes, we can change that when we start to allow MPS greater than HCI MTU.

> 
> It would also help to enforce some limits:
> SOCK_MIN_SNDBUF < sk->sk_sndbuf < sysctl_wmem_max
> SOCK_MIN_RCVBUF < sk->sk_rcvbuf < sysctl_rmem_max

Sure.

-- 
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi

^ permalink raw reply

* Re: [PATCH 2/2] Bluetooth: Update conf_state before send config_req out.
From: Gustavo F. Padovan @ 2010-09-28 23:49 UTC (permalink / raw)
  To: haijun liu; +Cc: linux-bluetooth@vger.kernel.org, dantian.ip
In-Reply-To: <AANLkTimQtkORjmsiO3wPULRErNXORBtHATbBvdCEsY27@mail.gmail.com>

Hi Haijun,

* haijun liu <liuhaijun.er@gmail.com> [2010-09-20 09:33:31 +0800]:

> Update conf_state with L2CAP_CONF_REQ_SENT before send config_req out.
> 
> 
> Signed-off-by: Haijun.Liu <Haijun.Liu@Atheros.com>

Both patches are corrupted, please fix that and rebase it against
http://git.kernel.org/?p=linux/kernel/git/padovan/bluetooth-next-2.6.git
Thanks.

-- 
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi

^ permalink raw reply

* Re: [PATCH] Bluetooth: Replace hard code of configuration continuation flag.
From: Gustavo F. Padovan @ 2010-09-28 23:48 UTC (permalink / raw)
  To: haijun liu; +Cc: linux-bluetooth, Mat Martineau, dantian.ip
In-Reply-To: <AANLkTi=mS2212wTd3nu_ENJ8ySyNSVMWgTmc-MT0421c@mail.gmail.com>

Hi Haijun,

* haijun liu <liuhaijun.er@gmail.com> [2010-09-16 17:21:40 +0800]:

> Replace hard code of configuration continuation flag with self-comment macro.
> 
> Signed-off-by: Haijun.Liu <Haijun.Liu@Atheros.com>

Your patch is corrupted, please fix that and rebase it against
http://git.kernel.org/?p=linux/kernel/git/padovan/bluetooth-next-2.6.git
Thanks.

-- 
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi

^ permalink raw reply

* Re: [PATCHv4 3/3] Bluetooth: check L2CAP length in first ACL fragment
From: Gustavo F. Padovan @ 2010-09-28 23:45 UTC (permalink / raw)
  To: Emeltchenko Andrei; +Cc: linux-bluetooth
In-Reply-To: <1284550124-31201-4-git-send-email-Andrei.Emeltchenko.news@gmail.com>

Hi Andrei,

* Emeltchenko Andrei <Andrei.Emeltchenko.news@gmail.com> [2010-09-15 14:28:44 +0300]:

> From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
> 
> Current Bluetooth code assembles fragments of big L2CAP packets
> in l2cap_recv_acldata and then checks allowed L2CAP size in
> assemled L2CAP packet (pi->imtu < skb->len).
> 
> The patch moves allowed L2CAP size check to the early stage when
> we receive the first fragment of L2CAP packet. We do not need to
> reserve and keep L2CAP fragments for bad packets.
> 
> Updated version after comments from Mat Martineau <mathewm@codeaurora.org>
> and Gustavo Padovan <padovan@profusion.mobi>.
> 
> Trace below is received when using stress tools sending big
> fragmented L2CAP packets.
> ...
> [ 1712.798492] swapper: page allocation failure. order:4, mode:0x4020
> [ 1712.804809] [<c0031870>] (unwind_backtrace+0x0/0xdc) from [<c00a1f70>]
> (__alloc_pages_nodemask+0x4)
> [ 1712.814666] [<c00a1f70>] (__alloc_pages_nodemask+0x47c/0x4d4) from
> [<c00a1fd8>] (__get_free_pages+)
> [ 1712.824645] [<c00a1fd8>] (__get_free_pages+0x10/0x3c) from [<c026eb5c>]
> (__alloc_skb+0x4c/0xfc)
> [ 1712.833465] [<c026eb5c>] (__alloc_skb+0x4c/0xfc) from [<bf28c738>]
> (l2cap_recv_acldata+0xf0/0x1f8 )
> [ 1712.843322] [<bf28c738>] (l2cap_recv_acldata+0xf0/0x1f8 [l2cap]) from
> [<bf0094ac>] (hci_rx_task+0x)
> ...
> 
> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
> ---
>  net/bluetooth/l2cap.c |   16 ++++++++++++++++
>  1 files changed, 16 insertions(+), 0 deletions(-)

Patch 1/3 was applied to my bluetooth-2.6 tree and 2/3 and 3/3 to
bluetooth-next-2.6. Thanks.

-- 
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi

^ permalink raw reply

* Re: bluetooth: add support for btusb devices in recent MacBook Pros
From: Gustavo F. Padovan @ 2010-09-28 22:58 UTC (permalink / raw)
  To: Will Woods; +Cc: Marcel Holtmann, linux-bluetooth, kernel
In-Reply-To: <1284757761-11994-1-git-send-email-wwoods@redhat.com>

Hi Will,

* Will Woods <wwoods@redhat.com> [2010-09-17 17:09:19 -0400]:

> These two patches add support for the USB bluetooth controllers found
> in recent MacBook Pro systems. In both cases the device class is
> ff (vend.) rather than the usual e0. The iMac11,1 has the same 
> problem and is handled with an explicit USB_DEVICE() entry, so 
> these two devices are handled the same way.

We already have these patches, yours are a duplicate. Thanks anyway. ;)

-- 
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi

^ permalink raw reply

* Re: pull-request: bluetooth-2.6 2010-09-27
From: Gustavo F. Padovan @ 2010-09-28 22:49 UTC (permalink / raw)
  To: David Miller; +Cc: linville, marcel, linux-bluetooth, netdev
In-Reply-To: <20100927.200016.226762808.davem@davemloft.net>

* David Miller <davem@davemloft.net> [2010-09-27 20:00:16 -0700]:

> From: "Gustavo F. Padovan" <padovan@profusion.mobi>
> Date: Mon, 27 Sep 2010 23:30:35 -0300
> 
> > And a fix for a deadlock issue between the sk_sndbuf and the backlog
> > queue in ERTM. The rest are also needed bug fixes.
> 
> This fix is still under discussion.
> 
> That change effects quite a few code paths.  And when I looked
> at them, I was not at all convinced that dropping the socket
> lock like that is safe.
> 
> Are you sure there are no pieces of socket or socket related state
> that might change under us while we drop that lock, which would thus
> make the operation suddenly invalid or cause a state corruption or
> crash?

We can group all the code paths in only two different code paths. One
wirh SCO, L2CAP Basic Mode and L2CAP Streaming Mode once they are very
similar and other for ERTM, a more complicated protocol.
For the first group the only bottom half action we have are incoming data,
which doesn't affect the sk states, and disconnection request, that can
change the sk states. We guarantee that this won't affect by checking the
sk_err after get the lock again. Looking to the code again we might
also want to check the sk->sk_shutdown value like TCP does inside
sk_stream_wait_memory().

Actually sk_stream_wait_memory is another point why it's safe to release
the lock and block waiting for memory. We've been doing that safely in
protocols like TCP, SCTP and DCCP for a long time.

Back to patch, the other code path it affects is the ERTM one, besides
the incoming data we have other bottom halves actions, but in the end the
only action that can affect ERTM flow is closing the channeli, but we are
prepared for that by checking the sk->sk_err and sk->sk_shutdown when we
get the lock back.


---

Bluetooth: Fix deadlock in the ERTM logic

The Enhanced Retransmission Mode(ERTM) is a realiable mode of operation
of the Bluetooth L2CAP layer. Think on it like a simplified version of
TCP.
The problem we were facing here was a deadlock. ERTM uses a backlog
queue to queue incomimg packets while the user is helding the lock. At
some moment the sk_sndbuf can be exceeded and we can't alloc new skbs
then the code sleep with the lock to wait for memory, that stalls the
ERTM connection once we can't read the acknowledgements packets in the
backlog queue to free memory and make the allocation of outcoming skb
successful.

This patch actually affect all users of bt_skb_send_alloc(), i.e., all
L2CAP modes and SCO.

We are safe against socket states changes or channels deletion while the
we are sleeping wait memory. Checking for the sk->sk_err and
sk->sk_shutdown make the code safe, since any action that can leave the
socket or the channel in a not usable state set one of the struct
members at least. Then we can check both of them when getting the lock
again and return with the proper error if something unexpected happens.

Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
Signed-off-by: Ulisses Furquim <ulisses@profusion.mobi>
---
 include/net/bluetooth/bluetooth.h |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index 27a902d..e8d64ba 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -161,12 +161,30 @@ static inline struct sk_buff *bt_skb_send_alloc(struct sock *sk, unsigned long l
 {
        struct sk_buff *skb;
 
+       release_sock(sk);
        if ((skb = sock_alloc_send_skb(sk, len + BT_SKB_RESERVE, nb, err))) {
                skb_reserve(skb, BT_SKB_RESERVE);
                bt_cb(skb)->incoming  = 0;
        }
+       lock_sock(sk);
+
+       if (!skb && *err)
+               return NULL;
+
+       *err = sock_error(sk);
+       if (*err)
+               goto out;
+
+       if (sk->sk_shutdown) {
+               *err = ECONNRESET;
+               goto out;
+       }
 
        return skb;
+
+out:
+       kfree_skb(skb);
+       return NULL;
 }
 
 int bt_err(__u16 code);
-- 
1.7.3


-- 
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi

^ permalink raw reply related

* [PATCH 4/4] Fix object path in the register watcher method
From: Claudio Takahasi @ 2010-09-28 17:49 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Claudio Takahasi
In-Reply-To: <1285696155-32766-1-git-send-email-claudio.takahasi@openbossa.org>

The path argument in RegisterCharacteristicsWatcher method shall be
the agent's path. An agent will monitor changes in all characteristics.
---
 doc/attribute-api.txt |   11 ++++-------
 1 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/doc/attribute-api.txt b/doc/attribute-api.txt
index d0ce6f8..f66e9bc 100644
--- a/doc/attribute-api.txt
+++ b/doc/attribute-api.txt
@@ -69,18 +69,15 @@ Methods 	dict GetProperties()
 			Returns all properties for the interface. See the
 			Properties section for the available properties.
 
-		RegisterCharacteristicsWatcher(object path)
+		RegisterCharacteristicsWatcher(object agent)
 
-			Register a watcher for changes in specific characteristics
-			to monitor changes.
+			Register a watcher to monitor characteristic changes.
 
 			A watcher will be registered for this service and will
-			notifier about any changed characteristics in the service.
+			notify about any changed characteristics in the service.
 			This also notifies about any included characteristics.
 
-			Method for the watch objects still need to be defined.
-
-		UnregisterCharacteristicsWatcher(object path)
+		UnregisterCharacteristicsWatcher(object agent)
 
 			Unregister a watcher.
 
-- 
1.7.3


^ permalink raw reply related

* [PATCH 3/4] Add SetProperty in the Device characteristic hierarchy
From: Claudio Takahasi @ 2010-09-28 17:49 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Claudio Takahasi
In-Reply-To: <1285696155-32766-1-git-send-email-claudio.takahasi@openbossa.org>

Add SetProperty method to allow characteristic watchers or any other
D-Bus client application to change the value of a given characteristic.
Use cases: Changing sensors thresholds, client characteristic
configuration for notification/indication.
---
 doc/attribute-api.txt |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/doc/attribute-api.txt b/doc/attribute-api.txt
index a5e475b..d0ce6f8 100644
--- a/doc/attribute-api.txt
+++ b/doc/attribute-api.txt
@@ -118,6 +118,13 @@ Methods		dict GetProperties()
 			Returns all properties for the characteristic. See the
 			properties section for available properties.
 
+		void SetProperty(string name, variant value)
+
+			Changes the value of the specified property. Only
+			read-write properties can be changed. On success
+			this will emit a PropertyChanged signal.
+
+			Possible Errors: org.bluez.Error.InvalidArguments
 
 Properties 	string UUID [readonly]
 
-- 
1.7.3


^ permalink raw reply related

* [PATCH 2/4] Add GetProperties method in the Device service hierarchy
From: Claudio Takahasi @ 2010-09-28 17:49 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Claudio Takahasi
In-Reply-To: <1285696155-32766-1-git-send-email-claudio.takahasi@openbossa.org>

Changes the attribute API to become compliant with other BlueZ
APIs. Characteristics properties can be retrieved using GetProperties
method of the Device Characteristic hierarchy. This patch also removes
unneeded characteristic properties description which is already
explained in the next section.
---
 doc/attribute-api.txt |   28 +++-------------------------
 1 files changed, 3 insertions(+), 25 deletions(-)

diff --git a/doc/attribute-api.txt b/doc/attribute-api.txt
index 0989099..a5e475b 100644
--- a/doc/attribute-api.txt
+++ b/doc/attribute-api.txt
@@ -64,15 +64,10 @@ Interface	org.bluez.Characteristic
 Object path	[prefix]/{hci0}/{device0}/{service0, service1, ...}
 		[prefix]/{hci0}/{device1}/{service0, service1, ...}
 
-Methods		array[(object, dict)] GetCharacteristics()
+Methods 	dict GetProperties()
 
-			Array of tuples with object path as identifier. An
-			alternative is doing dict of dict since the object
-			path is unique and the order of characteristics is
-			irrelevant. However it might be good to actually
-			present an order here.
-
-			See Characteristics properties for dictionary details.
+			Returns all properties for the interface. See the
+			Properties section for the available properties.
 
 		RegisterCharacteristicsWatcher(object path)
 
@@ -109,23 +104,6 @@ Properties	string Name (mandatory) [readonly]
 			includes. That way no complicated service includes array
 			is needed.
 
-			string UUID
-			string Name
-			string Description
-			struct Format (type, name, exponet etc.)
-
-			array{byte} Value
-			string Representation (of the binary Value)
-
-			object Service (the original service in case of includes)
-
-			At this point only GetProperties() method call should be
-			supported for simplicity. Changing characteristics is up
-			to future support.
-
-			The object path of the characteristics might be split
-			over multiple service objects, because of includes.
-
 
 Device Characteristic hierarchy
 ===============================
-- 
1.7.3


^ permalink raw reply related

* [PATCH 1/4] Add the Attribute interface to the API
From: Claudio Takahasi @ 2010-09-28 17:49 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Vinicius Costa Gomes

From: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>

For now the Attribute API will allow users to list all the GATT
services that a device has.
---
 doc/attribute-api.txt |   19 +++++++++++++++++++
 doc/device-api.txt    |    4 ----
 2 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/doc/attribute-api.txt b/doc/attribute-api.txt
index 011691e..0989099 100644
--- a/doc/attribute-api.txt
+++ b/doc/attribute-api.txt
@@ -37,6 +37,25 @@ Methods
 Properties
 
 
+Attribute Protocol hierarchy
+============================
+
+Service		org.bluez
+Interface	org.bluez.Attribute
+Object path	[prefix]/{hci0}/{device0}
+
+
+Methods		dict GetProperties()
+
+			Returns all properties for the interface. See the
+			properties section for available properties.
+
+Properties	array{object} Services
+
+			List of all the Primary Services that this device
+			implements.
+
+
 Device Service hierarchy
 ========================
 
diff --git a/doc/device-api.txt b/doc/device-api.txt
index 95b5b22..b818299 100644
--- a/doc/device-api.txt
+++ b/doc/device-api.txt
@@ -139,10 +139,6 @@ Properties	string Address [readonly]
 			List of 128-bit UUIDs that represents the available
 			remote services.
 
-		array{object} Services [readonly]
-
-			List of characteristics based services.
-
 		boolean Paired [readonly]
 
 			Indicates if the remote device is paired.
-- 
1.7.3


^ permalink raw reply related

* Re: [PATCH] Fix emitting TransferCompleted twice
From: Johan Hedberg @ 2010-09-28 13:41 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <1285679962-10367-1-git-send-email-luiz.dentz@gmail.com>

Hi Luiz,

On Tue, Sep 28, 2010, Luiz Augusto von Dentz wrote:
> From: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>
> 
> This is caused by REQDONE and latter disconnect generating duplicate
> signals in case of opp.
> 
> To fix this now we check if the object is valid before proceeding.
> ---
>  src/manager.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)

Thanks. The patch has been pushed upstream.

Johan

^ permalink raw reply

* Re: [PATCH v4 1/2] HID: Add Support for Setting and Getting Feature Reports from hidraw
From: Antonio Ospite @ 2010-09-28 13:30 UTC (permalink / raw)
  To: Alan Ott
  Cc: Jiri Kosina, Stefan Achatz, Alexey Dobriyan, Tejun Heo,
	Alan Stern, Greg Kroah-Hartman, Marcel Holtmann, Stephane Chatty,
	Michael Poole, David S. Miller, Bastien Nocera, Eric Dumazet,
	linux-input, linux-kernel, linux-usb, linux-bluetooth, netdev
In-Reply-To: <1281990059-3562-2-git-send-email-alan@signal11.us>


[-- Attachment #1.1: Type: text/plain, Size: 2624 bytes --]

On Mon, 16 Aug 2010 16:20:58 -0400
Alan Ott <alan@signal11.us> wrote:

> Per the HID Specification, Feature reports must be sent and received on
> the Configuration endpoint (EP 0) through the Set_Report/Get_Report
> interfaces.  This patch adds two ioctls to hidraw to set and get feature
> reports to and from the device.  Modifications were made to hidraw and
> usbhid.
> 
> New hidraw ioctls:
>   HIDIOCSFEATURE - Perform a Set_Report transfer of a Feature report.
>   HIDIOCGFEATURE - Perform a Get_Report transfer of a Feature report.
> 
> Signed-off-by: Alan Ott <alan@signal11.us>

Hi Alan, I am doing some stress testing on hidraw, if I have a loop with
HIDIOCGFEATURE on a given report and I disconnect the device while the
loop is running I get this:

BUG: unable to handle kernel NULL pointer dereference at 0000000000000028
IP: [<ffffffffa02c66b4>] hidraw_ioctl+0xfc/0x32c [hid]

Full log attached along with the test program, the device is a Sony PS3
Controller (sixaxis).

If my objdump analysis is right, hidraw_ioctl+0xfc should be around line
361 in hidraw.c (with your patch applied):

struct hid_device *hid = dev->hid;

It looks like 'dev' (which is hidraw_table[minor]) can be NULL
sometimes, can't it?
This is not introduced by your changes tho.

Just as a side note, the bug does not show up if the userspace program
handles return values properly and exits as soon as it gets an error
from the HID layer, see the XXX comment in test_hidraw_feature.c.

This fixes it, if it looks ok I will resend the patch rebased on
mainline code:

diff --git a/drivers/hid/hidraw.c b/drivers/hid/hidraw.c
index 7df1310..3c040c6 100644
--- a/drivers/hid/hidraw.c
+++ b/drivers/hid/hidraw.c
@@ -322,6 +322,10 @@ static long hidraw_ioctl(struct file *file, unsigned int cmd,

        mutex_lock(&minors_lock);
        dev = hidraw_table[minor];
+       if (!dev) {
+               ret = -ENODEV;
+               goto out;
+       }

        switch (cmd) {
                case HIDIOCGRDESCSIZE:
@@ -412,6 +416,7 @@ static long hidraw_ioctl(struct file *file, unsigned int cmd,

                ret = -ENOTTY;
        }
+out:
        mutex_unlock(&minors_lock);
        return ret;
 }


this change covers also the other uses of hidraw_table[minor] in
hidraw_send_report/hidraw_get_report.

Regards,
   Antonio

-- 
Antonio Ospite
http://ao2.it

PGP public key ID: 0x4553B001

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

[-- Attachment #1.2: dmesg_hidraw_feature_bug.log --]
[-- Type: application/octet-stream, Size: 3914 bytes --]

[  111.645836] usb 4-1: USB disconnect, address 2
[  111.669466] BUG: unable to handle kernel NULL pointer dereference at 0000000000000028
[  111.669484] IP: [<ffffffffa02c66b4>] hidraw_ioctl+0xfc/0x32c [hid]
[  111.669529] PGD 5a953067 PUD 6d741067 PMD 0 
[  111.669539] Oops: 0000 [#1] SMP 
[  111.669545] last sysfs file: /sys/devices/pci0000:00/0000:00:04.0/usb4/idVendor
[  111.669556] CPU 0 
[  111.669559] Modules linked in: hidp powernow_k8 mperf cpufreq_powersave cpufreq_conservative cpufreq_stats cpufreq_userspace lirc_serial(C) lirc_dev ipt_MASQUERADE bridge stp ppdev lp sco bnep rfcomm l2cap crc16 tun sit tunnel4 kvm_amd kvm binfmt_misc uinput fuse nfsd ip6table_raw ip6table_mangle ip6t_REJECT exportfs nfs ip6t_LOG lockd fscache nf_conntrack_ipv6 nfs_acl auth_rpcgss ip6table_filter sunrpc ip6_tables xt_tcpudp ipt_REJECT ipt_ULOG xt_limit xt_state xt_multiport iptable_filter iptable_nat nf_nat nf_conntrack_ipv4 nf_conntrack nf_defrag_ipv4 iptable_mangle iptable_raw ip_tables x_tables hwmon_vid loop snd_hda_codec_nvhdmi snd_hda_codec_via snd_hda_intel snd_hda_codec snd_hwdep snd_pcm_oss snd_mixer_oss snd_pcm snd_seq_midi snd_rawmidi nvidia(P) snd_seq_midi_event snd_seq parport_pc btusb snd_timer snd_seq_device asus_atk0110 joydev video snd bluetooth rfkill hid_sony output wmi parport edac_core i2c_nforce2 tpm_tis shpchp pci_hotplug k8temp edac_mce_amd pcspkr tpm tpm_bios evdev soundcore snd_page_alloc i2c_core button processor ext3 jbd mbcache dm_mod sg sr_mod sd_mod crc_t10dif cdrom ata_generic usbhid hid ahci libahci pata_amd ohci_hcd libata ehci_hcd scsi_mod usbcore thermal forcedeth nls_base thermal_sys floppy [last unloaded: scsi_wait_scan]
[  111.669736] 
[  111.669746] Pid: 2969, comm: test_hidraw_fea Tainted: P         C  2.6.36-rc5-ao2+ #1 M3N78-VM/System Product Name
[  111.669753] RIP: 0010:[<ffffffffa02c66b4>]  [<ffffffffa02c66b4>] hidraw_ioctl+0xfc/0x32c [hid]
[  111.669771] RSP: 0018:ffff880056831e78  EFLAGS: 00010206
[  111.669776] RAX: 0000000000000048 RBX: 00000000c02d4807 RCX: 00000000022c7e50
[  111.669783] RDX: 0000000100000000 RSI: ffffffff810377e0 RDI: ffffffffa02ceaa0
[  111.669789] RBP: 00000000022c7e50 R08: 00007f983f1cbec8 R09: 0000000000400aca
[  111.669795] R10: ffffffff8100769f R11: ffff880037a78dd8 R12: ffff88005a85c300
[  111.669801] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
[  111.669808] FS:  00007f983f3cc700(0000) GS:ffff880001a00000(0000) knlGS:0000000000000000
[  111.669815] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  111.669821] CR2: 0000000000000028 CR3: 000000005aba9000 CR4: 00000000000006f0
[  111.669827] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[  111.669833] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[  111.669840] Process test_hidraw_fea (pid: 2969, threadinfo ffff880056830000, task ffff88005ab93680)
[  111.669844] Stack:
[  111.669848]  00000000010aad14 ffff88005ab936b8 ffff88005a85c300 ffff880037fc6700
[  111.669857] <0> 00000000022c7e50 00000000022c7e50 0000000000000000 ffffffff810f64a5
[  111.669865] <0> ffff880001a14880 ffff88005ab93680 ffffffff81302776 0000000000000000
[  111.669875] Call Trace:
[  111.669898]  [<ffffffff810f64a5>] ? do_vfs_ioctl+0x4a2/0x4ef
[  111.669915]  [<ffffffff81302776>] ? schedule+0x573/0x5b7
[  111.669923]  [<ffffffff810f653d>] ? sys_ioctl+0x4b/0x72
[  111.669933]  [<ffffffff810ea002>] ? sys_write+0x5f/0x6b
[  111.669947]  [<ffffffff81008a02>] ? system_call_fastpath+0x16/0x1b
[  111.669952] Code: 2c 66 89 44 24 06 e8 1a c2 03 e1 48 89 e6 ba 08 00 00 00 48 89 ef e8 cc 79 ec e0 85 c0 0f 84 18 02 00 00 e9 01 02 00 00 0f b6 c7 <49> 8b 7d 28 83 f8 48 0f 85 fa 01 00 00 0f b6 c3 83 f8 06 75 24 
[  111.670011] RIP  [<ffffffffa02c66b4>] hidraw_ioctl+0xfc/0x32c [hid]
[  111.670025]  RSP <ffff880056831e78>
[  111.670029] CR2: 0000000000000028
[  111.670036] ---[ end trace 9c7530cfc7009202 ]---

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.3: test_hidraw_feature.c --]
[-- Type: text/x-csrc; name="test_hidraw_feature.c", Size: 1265 bytes --]

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <stdint.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <unistd.h>
#include <sys/ioctl.h>

/*
#include <linux/hidraw.h>
*/
#include "/home/ao2/Proj/linux/linux-2.6/include/linux/hidraw.h"

void dump_hex_string(unsigned char *buf, unsigned int len)
{
	unsigned int i;

	for (i = 0; i < len; i++)
		printf("%02x%c", buf[i], i < len - 1 ? ' ' : 0);
}

void dump_feature_report(int fd, uint8_t report_number, unsigned int len)
{
	unsigned char *buf;
	int ret;

	buf = calloc(len, sizeof(*buf));

	buf[0] = report_number;

	ret = ioctl(fd, HIDIOCGFEATURE(len), buf);
	if (ret < 0) {
		fprintf(stderr, "report: 0x%02x ret: %d\n", report_number, ret);
		/* XXX: if I put exit(1) here the bug is masked */
		return;
	}

	dump_hex_string(buf, len);
	printf("\r");
	fflush(stdout);

	free(buf);
}

int main(int argc, char *argv[])
{
	int fd = -1;

	if (argc != 2) {
		fprintf(stderr, "usage: %s </dev/hidrawX>\n", argv[0]);
		exit(1);
	}

	fd = open(argv[1], O_RDWR);
	if (fd < 0) {
		perror("hidraw open");
		exit(1);
	}

	while (1)
		dump_feature_report(fd, 0x01, 45);
	printf("\n");

	close(fd);
	exit(0);
}

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply related

* [PATCH] Fix emitting TransferCompleted twice
From: Luiz Augusto von Dentz @ 2010-09-28 13:19 UTC (permalink / raw)
  To: linux-bluetooth

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

This is caused by REQDONE and latter disconnect generating duplicate
signals in case of opp.

To fix this now we check if the object is valid before proceeding.
---
 src/manager.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/src/manager.c b/src/manager.c
index 78a329c..80140b6 100644
--- a/src/manager.c
+++ b/src/manager.c
@@ -648,7 +648,8 @@ void manager_emit_transfer_progress(struct obex_session *os)
 
 void manager_emit_transfer_completed(struct obex_session *os)
 {
-	emit_transfer_completed(os->cid, !os->aborted);
+	if (os->object)
+		emit_transfer_completed(os->cid, !os->aborted);
 }
 
 DBusConnection *obex_dbus_get_connection(void)
-- 
1.7.1


^ 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