Linux bluetooth development
 help / color / mirror / Atom feed
* [PATCH] avdtp: Fix crash on connection lost
From: Szymon Janc @ 2012-10-16 12:37 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

This fix avdtp session reference counting. avdtp_unref has a special
case when ref==1 and due to nested calls session could be free in
consecutive call resulting in accesing already freed memory.

Instead of having extra free lock keep extra local reference. This also
make avdtp_unref easier to undertand as there is no double ref
decrement inside one unref call.

avdtp_unref will set session state to disconnected when reference count
drops to 1 so there is no need to explicite set it in connection_lost.

bluetoothd[29474]: audio/avdtp.c:avdtp_ref() 0x555555856aa0: ref=2
bluetoothd[29474]: audio/source.c:source_connect() stream creation in progress
bluetoothd[29474]: src/mgmt.c:mgmt_event() cond 1
bluetoothd[29474]: src/mgmt.c:mgmt_event() Received 31 bytes from management socket
bluetoothd[29474]: src/mgmt.c:mgmt_device_connected() hci0 device 84:00:D2:DB:F7:8C connected eir_len 12
bluetoothd[29474]: src/adapter.c:adapter_get_device() 84:00:D2:DB:F7:8C
bluetoothd[29474]: src/mgmt.c:mgmt_event() cond 1
bluetoothd[29474]: src/mgmt.c:mgmt_event() Received 14 bytes from management socket
bluetoothd[29474]: src/mgmt.c:mgmt_auth_failed() hci0 auth failed status 6
bluetoothd[29474]: src/device.c:device_bonding_complete() bonding (nil) status 0x06
bluetoothd[29474]: connect error: Invalid exchange (52)
bluetoothd[29474]: audio/avdtp.c:connection_lost() Disconnected from 84:00:D2:DB:F7:8C
bluetoothd[29474]: audio/avdtp.c:avdtp_unref() 0x555555856aa0: ref=1
bluetoothd[29474]: audio/avdtp.c:avdtp_unref() 0x555555856aa0: ref=0
bluetoothd[29474]: audio/avdtp.c:avdtp_unref() 0x555555856aa0: freeing session and removing from list
bluetoothd[29474]: audio/avdtp.c:avdtp_unref() 0x555555856aa0: freeing session and removing from list

Program received signal SIGSEGV, Segmentation fault.
avdtp_unref (session=0x555555856aa0) at audio/avdtp.c:1239
1239            server->sessions = g_slist_remove(server->sessions, session);

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

diff --git a/audio/avdtp.c b/audio/avdtp.c
index bd91cb6..4109be9 100644
--- a/audio/avdtp.c
+++ b/audio/avdtp.c
@@ -388,7 +388,6 @@ struct avdtp_stream {
 
 struct avdtp {
 	int ref;
-	int free_lock;
 
 	uint16_t version;
 
@@ -1158,14 +1157,14 @@ static void connection_lost(struct avdtp *session, int err)
 	if (err != EACCES)
 		avdtp_cancel_authorization(session);
 
-	session->free_lock = 1;
+	avdtp_ref(session);
 
 	finalize_discovery(session, err);
 
 	g_slist_foreach(session->streams, (GFunc) release_stream, session);
 	session->streams = NULL;
 
-	session->free_lock = 0;
+	avdtp_unref(session);
 
 	if (session->io) {
 		g_io_channel_shutdown(session->io, FALSE, NULL);
@@ -1173,8 +1172,6 @@ static void connection_lost(struct avdtp *session, int err)
 		session->io = NULL;
 	}
 
-	avdtp_set_state(session, AVDTP_SESSION_STATE_DISCONNECTED);
-
 	if (session->io_id) {
 		g_source_remove(session->io_id);
 		session->io_id = 0;
@@ -1221,9 +1218,6 @@ void avdtp_unref(struct avdtp *session)
 
 		if (session->io)
 			set_disconnect_timer(session);
-		else if (!session->free_lock) /* Drop the local ref if we
-						 aren't connected */
-			session->ref--;
 	}
 
 	if (session->ref > 0)
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH BlueZ 2/2] AVRCP: Don't respond with errors when no player is registered
From: Luiz Augusto von Dentz @ 2012-10-16 11:58 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1350388692-15239-1-git-send-email-luiz.dentz@gmail.com>

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

Some devices w.g. Sony MW600 will stop using certain commands if an
error happen, so the code now just fake a player and once a real
player is registered it takes place of the fake one.
---
 audio/avrcp.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 74 insertions(+), 17 deletions(-)

diff --git a/audio/avrcp.c b/audio/avrcp.c
index 5a18cb4..cd57358 100644
--- a/audio/avrcp.c
+++ b/audio/avrcp.c
@@ -493,6 +493,28 @@ void avrcp_player_event(struct avrcp_player *player, uint8_t id, void *data)
 	return;
 }
 
+static const char *player_get_string(struct avrcp_player *player, uint32_t attr)
+{
+	if (player != NULL)
+		return player->cb->get_string(attr, player->user_data);
+
+	if (attr == AVRCP_MEDIA_ATTRIBUTE_TITLE)
+		return "";
+
+	return NULL;
+}
+
+static uint32_t player_get_uint32(struct avrcp_player *player, uint32_t attr)
+{
+	if (player != NULL)
+		return player->cb->get_uint32(attr, player->user_data);
+
+	if (attr == AVRCP_MEDIA_ATTRIBUTE_DURATION)
+		return UINT32_MAX;
+
+	return 0;
+}
+
 static uint16_t player_write_media_attribute(struct avrcp_player *player,
 						uint32_t id, uint8_t *buf,
 						uint16_t *pos,
@@ -510,7 +532,7 @@ static uint16_t player_write_media_attribute(struct avrcp_player *player,
 	case AVRCP_MEDIA_ATTRIBUTE_TRACK:
 	case AVRCP_MEDIA_ATTRIBUTE_N_TRACKS:
 	case AVRCP_MEDIA_ATTRIBUTE_DURATION:
-		num = player->cb->get_uint32(id, player->user_data);
+		num = player_get_uint32(player, id);
 		if (num == 0)
 			break;
 
@@ -518,7 +540,7 @@ static uint16_t player_write_media_attribute(struct avrcp_player *player,
 		value = valstr;
 		break;
 	default:
-		value = player->cb->get_string(id, player->user_data);
+		value = player_get_string(player, id);
 	}
 
 	if (value == NULL) {
@@ -610,6 +632,9 @@ static int player_set_attribute(struct avrcp_player *player,
 {
 	DBG("Change attribute: %u %u", attr, val);
 
+	if (player == NULL)
+		return -ENOENT;
+
 	return player->cb->set_setting(attr, val, player->user_data);
 }
 
@@ -619,6 +644,9 @@ static int player_get_attribute(struct avrcp_player *player, uint8_t attr)
 
 	DBG("attr %u", attr);
 
+	if (player == NULL)
+		return -ENOENT;
+
 	value = player->cb->get_setting(attr, player->user_data);
 	if (value < 0)
 		DBG("attr %u not supported by player", attr);
@@ -675,7 +703,7 @@ static uint8_t avrcp_handle_list_player_attributes(struct avrcp *session,
 	uint16_t len = ntohs(pdu->params_len);
 	unsigned int i;
 
-	if (len != 0 || player == NULL) {
+	if (len != 0) {
 		pdu->params_len = htons(1);
 		pdu->params[0] = AVRCP_STATUS_INVALID_PARAM;
 		return AVC_CTYPE_REJECTED;
@@ -707,7 +735,7 @@ static uint8_t avrcp_handle_list_player_values(struct avrcp *session,
 	uint16_t len = ntohs(pdu->params_len);
 	unsigned int i;
 
-	if (len != 1 || player == NULL)
+	if (len != 1)
 		goto err;
 
 	if (player_get_attribute(player, pdu->params[0]) < 0)
@@ -729,6 +757,15 @@ err:
 	return AVC_CTYPE_REJECTED;
 }
 
+static GList *player_list_metadata(struct avrcp_player *player)
+{
+	if (player != NULL)
+		return player->cb->list_metadata(player->user_data);
+
+	return g_list_prepend(NULL,
+				GUINT_TO_POINTER(AVRCP_MEDIA_ATTRIBUTE_TITLE));
+}
+
 static uint8_t avrcp_handle_get_element_attributes(struct avrcp *session,
 						struct avrcp_header *pdu,
 						uint8_t transaction)
@@ -741,7 +778,7 @@ static uint8_t avrcp_handle_get_element_attributes(struct avrcp *session,
 	GList *attr_ids;
 	uint16_t offset;
 
-	if (len < 9 || identifier != 0 || player == NULL)
+	if (len < 9 || identifier != 0)
 		goto err;
 
 	nattr = pdu->params[8];
@@ -754,7 +791,7 @@ static uint8_t avrcp_handle_get_element_attributes(struct avrcp *session,
 		 * Return all available information, at least
 		 * title must be returned if there's a track selected.
 		 */
-		attr_ids = player->cb->list_metadata(player->user_data);
+		attr_ids = player_list_metadata(player);
 		len = g_list_length(attr_ids);
 	} else {
 		unsigned int i;
@@ -810,8 +847,7 @@ static uint8_t avrcp_handle_get_current_player_value(struct avrcp *session,
 	uint8_t *settings;
 	unsigned int i;
 
-	if (player == NULL || len <= 1 || pdu->params[0] != len - 1 ||
-							player == NULL)
+	if (len <= 1 || pdu->params[0] != len - 1)
 		goto err;
 
 	/*
@@ -944,6 +980,14 @@ err:
 	return AVC_CTYPE_REJECTED;
 }
 
+static uint32_t player_get_position(struct avrcp_player *player)
+{
+	if (player == NULL)
+		return 0;
+
+	return player->cb->get_position(player->user_data);
+}
+
 static uint8_t avrcp_handle_get_play_status(struct avrcp *session,
 						struct avrcp_header *pdu,
 						uint8_t transaction)
@@ -953,17 +997,14 @@ static uint8_t avrcp_handle_get_play_status(struct avrcp *session,
 	uint32_t position;
 	uint32_t duration;
 
-	if (len != 0 || player == NULL) {
+	if (len != 0) {
 		pdu->params_len = htons(1);
 		pdu->params[0] = AVRCP_STATUS_INVALID_PARAM;
 		return AVC_CTYPE_REJECTED;
 	}
 
-	position = player->cb->get_position(player->user_data);
-	duration = player->cb->get_uint32(AVRCP_MEDIA_ATTRIBUTE_DURATION,
-							player->user_data);
-	if (duration == 0)
-		duration = UINT32_MAX;
+	position = player_get_position(player);
+	duration = player_get_uint32(player, AVRCP_MEDIA_ATTRIBUTE_DURATION);
 
 	position = htonl(position);
 	duration = htonl(duration);
@@ -977,6 +1018,22 @@ static uint8_t avrcp_handle_get_play_status(struct avrcp *session,
 	return AVC_CTYPE_STABLE;
 }
 
+static uint64_t player_get_uid(struct avrcp_player *player)
+{
+	if (player == NULL)
+		return UINT64_MAX;
+
+	return player->cb->get_status(player->user_data);
+}
+
+static uint8_t player_get_status(struct avrcp_player *player)
+{
+	if (player == NULL)
+		return AVRCP_PLAY_STATUS_STOPPED;
+
+	return player->cb->get_status(player->user_data);
+}
+
 static uint8_t avrcp_handle_register_notification(struct avrcp *session,
 						struct avrcp_header *pdu,
 						uint8_t transaction)
@@ -990,18 +1047,18 @@ static uint8_t avrcp_handle_register_notification(struct avrcp *session,
 	 * one is applicable only for EVENT_PLAYBACK_POS_CHANGED. See AVRCP
 	 * 1.3 spec, section 5.4.2.
 	 */
-	if (len != 5 || player == NULL)
+	if (len != 5)
 		goto err;
 
 	switch (pdu->params[0]) {
 	case AVRCP_EVENT_STATUS_CHANGED:
 		len = 2;
-		pdu->params[1] = player->cb->get_status(player->user_data);
+		pdu->params[1] = player_get_status(player);
 
 		break;
 	case AVRCP_EVENT_TRACK_CHANGED:
 		len = 9;
-		uid = player->cb->get_uid(player->user_data);
+		uid = player_get_uid(player);
 		memcpy(&pdu->params[1], &uid, sizeof(uint64_t));
 
 		break;
-- 
1.7.11.4


^ permalink raw reply related

* [PATCH BlueZ 1/2] AVRCP: Fix using void * for metadata values
From: Luiz Augusto von Dentz @ 2012-10-16 11:58 UTC (permalink / raw)
  To: linux-bluetooth

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

This replaces get_metadata callback with get_string and get_uint32
which uses proper types as return.
---
 audio/avrcp.c | 32 ++++++++++++++++++--------------
 audio/avrcp.h |  3 ++-
 audio/media.c | 31 ++++++++++++++++++++++++-------
 3 files changed, 44 insertions(+), 22 deletions(-)

diff --git a/audio/avrcp.c b/audio/avrcp.c
index 2f5df21..5a18cb4 100644
--- a/audio/avrcp.c
+++ b/audio/avrcp.c
@@ -501,23 +501,29 @@ static uint16_t player_write_media_attribute(struct avrcp_player *player,
 	uint16_t len;
 	uint16_t attr_len;
 	char valstr[20];
-	void *value;
+	const char *value = NULL;
+	uint32_t num;
 
 	DBG("%u", id);
 
-	value = player->cb->get_metadata(id, player->user_data);
-	if (value == NULL) {
-		*offset = 0;
-		return 0;
-	}
-
 	switch (id) {
 	case AVRCP_MEDIA_ATTRIBUTE_TRACK:
 	case AVRCP_MEDIA_ATTRIBUTE_N_TRACKS:
 	case AVRCP_MEDIA_ATTRIBUTE_DURATION:
-		snprintf(valstr, 20, "%u", GPOINTER_TO_UINT(value));
+		num = player->cb->get_uint32(id, player->user_data);
+		if (num == 0)
+			break;
+
+		snprintf(valstr, 20, "%u", num);
 		value = valstr;
 		break;
+	default:
+		value = player->cb->get_string(id, player->user_data);
+	}
+
+	if (value == NULL) {
+		*offset = 0;
+		return 0;
 	}
 
 	attr_len = strlen(value);
@@ -946,7 +952,6 @@ static uint8_t avrcp_handle_get_play_status(struct avrcp *session,
 	uint16_t len = ntohs(pdu->params_len);
 	uint32_t position;
 	uint32_t duration;
-	void *pduration;
 
 	if (len != 0 || player == NULL) {
 		pdu->params_len = htons(1);
@@ -955,14 +960,13 @@ static uint8_t avrcp_handle_get_play_status(struct avrcp *session,
 	}
 
 	position = player->cb->get_position(player->user_data);
-	pduration = player->cb->get_metadata(AVRCP_MEDIA_ATTRIBUTE_DURATION,
+	duration = player->cb->get_uint32(AVRCP_MEDIA_ATTRIBUTE_DURATION,
 							player->user_data);
-	if (pduration != NULL)
-		duration = htonl(GPOINTER_TO_UINT(pduration));
-	else
-		duration = htonl(UINT32_MAX);
+	if (duration == 0)
+		duration = UINT32_MAX;
 
 	position = htonl(position);
+	duration = htonl(duration);
 
 	memcpy(&pdu->params[0], &duration, 4);
 	memcpy(&pdu->params[4], &position, 4);
diff --git a/audio/avrcp.h b/audio/avrcp.h
index 6c651dd..1a1e111 100644
--- a/audio/avrcp.h
+++ b/audio/avrcp.h
@@ -80,7 +80,8 @@ struct avrcp_player_cb {
 	int (*get_setting) (uint8_t attr, void *user_data);
 	int (*set_setting) (uint8_t attr, uint8_t value, void *user_data);
 	uint64_t (*get_uid) (void *user_data);
-	void *(*get_metadata) (uint32_t id, void *user_data);
+	uint32_t (*get_uint32) (uint32_t id, void *user_data);
+	const char *(*get_string) (uint32_t id, void *user_data);
 	GList *(*list_metadata) (void *user_data);
 	uint8_t (*get_status) (void *user_data);
 	uint32_t (*get_position) (void *user_data);
diff --git a/audio/media.c b/audio/media.c
index f2b5b2f..116e2cd 100644
--- a/audio/media.c
+++ b/audio/media.c
@@ -1281,7 +1281,7 @@ static uint64_t get_uid(void *user_data)
 	return 0;
 }
 
-static void *get_metadata(uint32_t id, void *user_data)
+static const char *get_string(uint32_t id, void *user_data)
 {
 	struct media_player *mp = user_data;
 	struct metadata_value *value;
@@ -1295,16 +1295,32 @@ static void *get_metadata(uint32_t id, void *user_data)
 	if (!value)
 		return NULL;
 
-	switch (value->type) {
-	case DBUS_TYPE_STRING:
+	if (value->type == DBUS_TYPE_STRING)
 		return value->value.str;
-	case DBUS_TYPE_UINT32:
-		return GUINT_TO_POINTER(value->value.num);
-	}
 
 	return NULL;
 }
 
+static uint32_t get_uint32(uint32_t id, void *user_data)
+{
+	struct media_player *mp = user_data;
+	struct metadata_value *value;
+
+	DBG("%s", metadata_to_str(id));
+
+	if (mp->track == NULL)
+		return 0;
+
+	value = g_hash_table_lookup(mp->track, GUINT_TO_POINTER(id));
+	if (!value)
+		return 0;
+
+	if (value->type == DBUS_TYPE_UINT32)
+		return value->value.num;
+
+	return 0;
+}
+
 static uint8_t get_status(void *user_data)
 {
 	struct media_player *mp = user_data;
@@ -1360,7 +1376,8 @@ static struct avrcp_player_cb player_cb = {
 	.set_setting = set_setting,
 	.list_metadata = list_metadata,
 	.get_uid = get_uid,
-	.get_metadata = get_metadata,
+	.get_uint32 = get_uint32,
+	.get_string = get_string,
 	.get_position = get_position,
 	.get_status = get_status,
 	.set_volume = set_volume
-- 
1.7.11.4


^ permalink raw reply related

* Re: [PATCH 14/19] Bluetooth: Do not send data during channel move
From: Andrei Emeltchenko @ 2012-10-16 10:50 UTC (permalink / raw)
  To: Mat Martineau; +Cc: linux-bluetooth, gustavo, sunnyk, marcel
In-Reply-To: <1350315248-7690-15-git-send-email-mathewm@codeaurora.org>

Hi Mat,

On Mon, Oct 15, 2012 at 08:34:03AM -0700, Mat Martineau wrote:
> Outgoing ERTM data is queued during a channel move.  The ERTM state
> machine is partially reset at the start of a move, and must be
> resynchronized with the remote state machine at the end of the move.
> Data is not sent so that there are no state transitions between the
> partial reset and the resync.
> 
> Streaming mode frames are dropped during a move.
> 
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
>  net/bluetooth/l2cap_core.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index d7b1bf3..cf1dc4e 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -927,6 +927,10 @@ static void l2cap_send_sframe(struct l2cap_chan *chan,
>  	if (!control->sframe)
>  		return;
>  
> +	if (chan->move_state != L2CAP_MOVE_STABLE &&
> +	    chan->move_state != L2CAP_MOVE_WAIT_PREPARE)

this block is extensively used here and in the previous patch, can it be
put to function or macro?

> +		return;
> +
>  	if (test_and_clear_bit(CONN_SEND_FBIT, &chan->conn_state) &&
>  	    !control->poll)
>  		control->final = 1;
> @@ -1805,6 +1809,10 @@ static void l2cap_streaming_send(struct l2cap_chan *chan,
>  
>  	BT_DBG("chan %p, skbs %p", chan, skbs);
>  
> +	if (chan->move_state != L2CAP_MOVE_STABLE &&
> +	    chan->move_state != L2CAP_MOVE_WAIT_PREPARE)
> +		return;
> +
>  	skb_queue_splice_tail_init(skbs, &chan->tx_q);
>  
>  	while (!skb_queue_empty(&chan->tx_q)) {
> @@ -1847,6 +1855,10 @@ static int l2cap_ertm_send(struct l2cap_chan *chan)
>  	if (test_bit(CONN_REMOTE_BUSY, &chan->conn_state))
>  		return 0;
>  
> +	if (chan->move_state != L2CAP_MOVE_STABLE &&
> +	    chan->move_state != L2CAP_MOVE_WAIT_PREPARE)
> +		return 0;
> +
>  	while (chan->tx_send_head &&
>  	       chan->unacked_frames < chan->remote_tx_win &&
>  	       chan->tx_state == L2CAP_TX_STATE_XMIT) {
> @@ -1912,6 +1924,10 @@ static void l2cap_ertm_resend(struct l2cap_chan *chan)
>  	if (test_bit(CONN_REMOTE_BUSY, &chan->conn_state))
>  		return;
>  
> +	if (chan->move_state != L2CAP_MOVE_STABLE &&
> +	    chan->move_state != L2CAP_MOVE_WAIT_PREPARE)
> +		return;
> +
>  	while (chan->retrans_list.head != L2CAP_SEQ_LIST_CLEAR) {
>  		seq = l2cap_seq_list_pop(&chan->retrans_list);
>  

Best regards 
Andrei Emeltchenko 

^ permalink raw reply

* Re: [PATCH 10/19] Bluetooth: Add logical link confirm
From: Andrei Emeltchenko @ 2012-10-16 10:38 UTC (permalink / raw)
  To: Mat Martineau; +Cc: linux-bluetooth, gustavo, sunnyk, marcel
In-Reply-To: <1350315248-7690-11-git-send-email-mathewm@codeaurora.org>

Hi Mat,

On Mon, Oct 15, 2012 at 08:33:59AM -0700, Mat Martineau wrote:
> The logical link confirm callback is executed when the AMP controller
> completes its logical link setup.  During a channel move, a newly
> formed logical link allows a move responder to send a move channel
> response.  A move initiator will send a move channel confirm.  A
> failed logical link will end the channel move and send an appropriate
> response or confirm command indicating a failure.
> 
> If the channel is being created on an AMP controller, L2CAP
> configuration is started after the logical link is set up.
> 
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
>  net/bluetooth/l2cap_core.c | 117 ++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 115 insertions(+), 2 deletions(-)
> 
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index c687cc1..7e31e98 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -3779,6 +3779,7 @@ static inline int l2cap_config_req(struct l2cap_conn *conn,
>  		goto unlock;
>  	}
>  
> +	chan->ident = cmd->ident;
>  	l2cap_send_cmd(conn, cmd->ident, L2CAP_CONF_RSP, len, rsp);
>  	chan->num_conf_rsp++;
>  
> @@ -4221,11 +4222,123 @@ static void l2cap_send_move_chan_cfm_rsp(struct l2cap_conn *conn, u8 ident,
>  	l2cap_send_cmd(conn, ident, L2CAP_MOVE_CHAN_CFM_RSP, sizeof(rsp), &rsp);
>  }
>  
> +/* Call with chan locked */
>  static void l2cap_logical_cfm(struct l2cap_chan *chan, struct hci_chan *hchan,
>  			      u8 status)
>  {
> -	/* Placeholder */
> -	return;
> +	BT_DBG("chan %p, hchan %p, status %d", chan, hchan, status);
> +
> +	if (chan->state != BT_CONNECTED && !chan->local_amp_id)
> +		return;
> +
> +	if (!status && chan->state != BT_CONNECTED) {
> +		struct l2cap_conf_rsp rsp;
> +		u8 code;
> +
> +		/* Create channel complete */
> +		chan->hs_hcon = hchan->conn;
> +		chan->hs_hcon->l2cap_data = chan->conn;
> +
> +		code = l2cap_build_conf_rsp(chan, &rsp,
> +					    L2CAP_CONF_SUCCESS, 0);
> +		l2cap_send_cmd(chan->conn, chan->ident, L2CAP_CONF_RSP, code,
> +			       &rsp);
> +		set_bit(CONF_OUTPUT_DONE, &chan->conf_state);
> +
> +		if (test_bit(CONF_INPUT_DONE, &chan->conf_state)) {
> +			int err = 0;
> +
> +			set_default_fcs(chan);
> +
> +			if (chan->mode == L2CAP_MODE_ERTM ||
> +			    chan->mode == L2CAP_MODE_STREAMING)

Shall we check for this? Can we have here basic mode?

Best regards 
Andrei Emeltchenko 

^ permalink raw reply

* Re: [PATCH 1/8] audio: Move btd_device unref after last use in device_free
From: Johan Hedberg @ 2012-10-16 10:37 UTC (permalink / raw)
  To: Szymon Janc; +Cc: linux-bluetooth
In-Reply-To: <1350382379-30489-1-git-send-email-szymon.janc@tieto.com>

Hi Szymon,

On Tue, Oct 16, 2012, Szymon Janc wrote:
> btd_device should not be used after local reference was dropped.
> 
> ---
>  audio/device.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

All patches in this set have been applied. Thanks.

As a note for future commit messages please prefix lib/* changes with
"lib: ..". I fixed it manually this time.

Johan

^ permalink raw reply

* Re: [PATCH 07/19] Bluetooth: Add move channel confirm handling
From: Andrei Emeltchenko @ 2012-10-16 10:27 UTC (permalink / raw)
  To: Mat Martineau; +Cc: linux-bluetooth, gustavo, sunnyk, marcel
In-Reply-To: <1350315248-7690-8-git-send-email-mathewm@codeaurora.org>

Hi Mat,

On Mon, Oct 15, 2012 at 08:33:56AM -0700, Mat Martineau wrote:
> After sending a move channel response, a move responder waits for a
> move channel confirm command.  If the received command has a
> "confirmed" result the move is proceeding, and "unconfirmed" means the
> move has failed and the channel will not change controllers.
> 
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
>  net/bluetooth/l2cap_core.c | 70 ++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 67 insertions(+), 3 deletions(-)
> 
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index aab7f79..ef744a9 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -1030,6 +1030,42 @@ static void l2cap_move_setup(struct l2cap_chan *chan)
>  	set_bit(CONN_REMOTE_BUSY, &chan->conn_state);
>  }
>  
> +static void l2cap_move_success(struct l2cap_chan *chan)
> +{
> +	BT_DBG("chan %p", chan);
> +
> +	if (chan->mode != L2CAP_MODE_ERTM)
> +		return;
> +
> +	switch (chan->move_role) {
> +	case L2CAP_MOVE_ROLE_INITIATOR:
> +		l2cap_tx(chan, NULL, NULL, L2CAP_EV_EXPLICIT_POLL);
> +		chan->rx_state = L2CAP_RX_STATE_WAIT_F;
> +		break;
> +	case L2CAP_MOVE_ROLE_RESPONDER:
> +		chan->rx_state = L2CAP_RX_STATE_WAIT_P;
> +		break;
> +	}
> +}
> +
> +static void l2cap_move_revert(struct l2cap_chan *chan)
> +{
> +	BT_DBG("chan %p", chan);
> +
> +	if (chan->mode != L2CAP_MODE_ERTM)
> +		return;

Not sure that all those function need to check for ERTM. Can it be done
before calling them (for example in l2cap_move_channel_confirm)?

Best regards 
Andrei Emeltchenko 

^ permalink raw reply

* Re: [PATCH 06/19] Bluetooth: Add new ERTM receive states for channel move
From: Andrei Emeltchenko @ 2012-10-16 10:21 UTC (permalink / raw)
  To: Mat Martineau; +Cc: linux-bluetooth, gustavo, sunnyk, marcel
In-Reply-To: <1350315248-7690-7-git-send-email-mathewm@codeaurora.org>

Hi Mat,

On Mon, Oct 15, 2012 at 08:33:55AM -0700, Mat Martineau wrote:
> Two new states are required to implement channel moves with the ERTM
> receive state machine.
> 
> The "WAIT_P" state is used by a move responder to wait for a "poll"
> flag after a move is completed (success or failure).  "WAIT_F" is
> similarly used by a move initiator to wait for a "final" flag when the
> move is completing.  In either state, the reqseq value in the
> poll/final frame tells the state machine exactly which frame should be
> expected next.
> 
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
>  net/bluetooth/l2cap_core.c | 106 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 106 insertions(+)
> 
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 649c06b..aab7f79 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -5207,6 +5207,106 @@ static int l2cap_rx_state_srej_sent(struct l2cap_chan *chan,
>  	return err;
>  }
>  
> +static int l2cap_finish_move(struct l2cap_chan *chan)
> +{
> +	int err = 0;
> +
> +	BT_DBG("chan %p", chan);
> +
> +	chan->move_role = L2CAP_MOVE_ROLE_NONE;
> +	chan->rx_state = L2CAP_RX_STATE_RECV;
> +
> +	if (chan->hs_hcon)
> +		chan->conn->mtu = chan->hs_hcon->hdev->acl_mtu;

Should we use here block_mtu?

> +	else
> +		chan->conn->mtu = chan->conn->hcon->hdev->acl_mtu;
> +
> +	/* Placeholder - resegment based on new conn->mtu */
> +	/*err = l2cap_resegment(chan);*/
> +
> +	return err;
> +}
> +
> +static int l2cap_rx_state_wait_p(struct l2cap_chan *chan,
> +				 struct l2cap_ctrl *control,
> +				 struct sk_buff *skb, u8 event)
> +{
> +	int err = -EPROTO;
> +
> +	BT_DBG("chan %p, control %p, skb %p, event %d", chan, control, skb,
> +	       event);
> +
> +	if (!control->poll)
> +		return err;
> +
> +	l2cap_process_reqseq(chan, control->reqseq);
> +
> +	if (!skb_queue_empty(&chan->tx_q))
> +		chan->tx_send_head = skb_peek(&chan->tx_q);
> +	else
> +		chan->tx_send_head = NULL;
> +
> +	/* Rewind next_tx_seq to the point expected
> +	 * by the receiver.
> +	 */
> +	chan->next_tx_seq = control->reqseq;
> +	chan->unacked_frames = 0;
> +
> +	err = l2cap_finish_move(chan);
> +
> +	if (err)

extra new line

> +		return err;
> +
> +	set_bit(CONN_SEND_FBIT, &chan->conn_state);
> +	l2cap_send_i_or_rr_or_rnr(chan);
> +
> +	if (event == L2CAP_EV_RECV_IFRAME)
> +		err = -EPROTO;
> +	else
> +		err = l2cap_rx_state_recv(chan, control, NULL, event);
> +
> +	return err;
> +}
> +
> +static int l2cap_rx_state_wait_f(struct l2cap_chan *chan,
> +				 struct l2cap_ctrl *control,
> +				 struct sk_buff *skb, u8 event)
> +{
> +	int err = -EPROTO;
> +
> +	if (control->final) {
> +		clear_bit(CONN_REMOTE_BUSY, &chan->conn_state);
> +		chan->move_role = L2CAP_MOVE_ROLE_NONE;
> +
> +		chan->rx_state = L2CAP_RX_STATE_RECV;
> +		l2cap_process_reqseq(chan, control->reqseq);
> +
> +		if (!skb_queue_empty(&chan->tx_q))
> +			chan->tx_send_head = skb_peek(&chan->tx_q);
> +		else
> +			chan->tx_send_head = NULL;
> +
> +		/* Rewind next_tx_seq to the point expected
> +		 * by the receiver.
> +		 */
> +		chan->next_tx_seq = control->reqseq;
> +		chan->unacked_frames = 0;
> +
> +		if (chan->hs_hcon)
> +			chan->conn->mtu = chan->hs_hcon->hdev->acl_mtu;

block_mtu again

> +		else
> +			chan->conn->mtu = chan->conn->hcon->hdev->acl_mtu;
> +
> +		/* Placeholder - resegment based on new conn->mtu */
> +		/*err = l2cap_resegment(chan);*/
> +
> +		if (!err)
> +			err = l2cap_rx_state_recv(chan, control, skb, event);
> +	}
> +
> +	return err;
> +}
> +
>  static bool __valid_reqseq(struct l2cap_chan *chan, u16 reqseq)
>  {
>  	/* Make sure reqseq is for a packet that has been sent but not acked */
> @@ -5233,6 +5333,12 @@ static int l2cap_rx(struct l2cap_chan *chan, struct l2cap_ctrl *control,
>  			err = l2cap_rx_state_srej_sent(chan, control, skb,
>  						       event);
>  			break;
> +		case L2CAP_RX_STATE_WAIT_P:
> +			err = l2cap_rx_state_wait_p(chan, control, skb, event);
> +			break;
> +		case L2CAP_RX_STATE_WAIT_F:
> +			err = l2cap_rx_state_wait_f(chan, control, skb, event);
> +			break;
>  		default:
>  			/* shut it down */
>  			break;

Best regards 
Andrei Emeltchenko 

^ permalink raw reply

* Re: [PATCH 05/19] Bluetooth: Channel move request handling
From: Andrei Emeltchenko @ 2012-10-16 10:18 UTC (permalink / raw)
  To: Mat Martineau; +Cc: linux-bluetooth, gustavo, sunnyk, marcel
In-Reply-To: <1350315248-7690-6-git-send-email-mathewm@codeaurora.org>

Hi Mat,

On Mon, Oct 15, 2012 at 08:33:54AM -0700, Mat Martineau wrote:
> On receipt of a channel move request, the request must be validated
> based on the L2CAP mode, connection state, and controller
> capabilities.  ERTM channels must have their state machines cleared
> and transmission paused while the channel move takes place.
> 
> If the channel is being moved to an AMP controller then
> an AMP physical link must be prepared.  Moving the channel back to
> BR/EDR proceeds immediately.
> 
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
>  net/bluetooth/l2cap_core.c | 98 +++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 97 insertions(+), 1 deletion(-)
> 
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index a55644f..649c06b 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -995,6 +995,41 @@ void l2cap_send_conn_req(struct l2cap_chan *chan)
>  	l2cap_send_cmd(conn, chan->ident, L2CAP_CONN_REQ, sizeof(req), &req);
>  }
>  
> +static void l2cap_move_setup(struct l2cap_chan *chan)
> +{
> +	struct sk_buff *skb;
> +
> +	BT_DBG("chan %p", chan);
> +
> +	if (chan->mode != L2CAP_MODE_ERTM)
> +		return;

I think you use double check, first before calling this function and then
this one. Please verify that both checks for ERTM are really needed.

> +
> +	__clear_retrans_timer(chan);
> +	__clear_monitor_timer(chan);
> +	__clear_ack_timer(chan);
> +
> +	chan->retry_count = 0;
> +	skb_queue_walk(&chan->tx_q, skb) {
> +		if (bt_cb(skb)->control.retries)
> +			bt_cb(skb)->control.retries = 1;
> +		else
> +			break;
> +	}
> +
> +	chan->expected_tx_seq = chan->buffer_seq;
> +
> +	clear_bit(CONN_REJ_ACT, &chan->conn_state);
> +	clear_bit(CONN_SREJ_ACT, &chan->conn_state);
> +	l2cap_seq_list_clear(&chan->retrans_list);
> +	l2cap_seq_list_clear(&chan->srej_list);
> +	skb_queue_purge(&chan->srej_q);
> +
> +	chan->tx_state = L2CAP_TX_STATE_XMIT;
> +	chan->rx_state = L2CAP_RX_STATE_MOVE;
> +
> +	set_bit(CONN_REMOTE_BUSY, &chan->conn_state);
> +}
> +
>  static void l2cap_chan_ready(struct l2cap_chan *chan)
>  {
>  	/* This clears all conf flags, including CONF_NOT_COMPLETE */
> @@ -4171,7 +4206,68 @@ static inline int l2cap_move_channel_req(struct l2cap_conn *conn,
>  
>  	chan = l2cap_get_chan_by_dcid(conn, icid);
>  
> -	/* Placeholder: Always refuse */
> +	if (!chan || chan->scid < L2CAP_CID_DYN_START ||
> +	    (chan->mode != L2CAP_MODE_ERTM &&

first check is here

> +	     chan->mode != L2CAP_MODE_STREAMING)) {
> +		result = L2CAP_MR_NOT_ALLOWED;
> +		goto send_move_response;
> +	}
> +
> +	if (chan->local_amp_id == req->dest_amp_id) {
> +		result = L2CAP_MR_SAME_ID;
> +		goto send_move_response;
> +	}
> +
> +	if (req->dest_amp_id) {
> +		struct hci_dev *hdev;
> +		hdev = hci_dev_get(req->dest_amp_id);
> +		if (!hdev || hdev->dev_type != HCI_AMP ||
> +		    !test_bit(HCI_UP, &hdev->flags)) {
> +			if (hdev)
> +				hci_dev_put(hdev);
> +
> +			result = L2CAP_MR_BAD_ID;
> +			goto send_move_response;
> +		}
> +		hci_dev_put(hdev);
> +	}
> +
> +	if (((chan->move_state != L2CAP_MOVE_STABLE &&
> +	    chan->move_state != L2CAP_MOVE_WAIT_PREPARE) ||
> +	    chan->move_role != L2CAP_MOVE_ROLE_NONE) &&
> +	    bacmp(conn->src, conn->dst) > 0) {
> +		result = L2CAP_MR_COLLISION;
> +		goto send_move_response;
> +	}
> +
> +	if (chan->chan_policy == BT_CHANNEL_POLICY_BREDR_ONLY) {

I believe this check must be moved to the beginning of the function right
after getting chan.

> +		result = L2CAP_MR_NOT_ALLOWED;
> +		goto send_move_response;
> +	}
> +
> +	chan->ident = cmd->ident;
> +	chan->move_role = L2CAP_MOVE_ROLE_RESPONDER;
> +	l2cap_move_setup(chan);
> +	chan->move_id = req->dest_amp_id;
> +	icid = chan->dcid;
> +
> +	if (req->dest_amp_id == 0) {
> +		/* Moving to BR/EDR */
> +		if (test_bit(CONN_LOCAL_BUSY, &chan->conn_state)) {
> +			chan->move_state = L2CAP_MOVE_WAIT_LOCAL_BUSY;
> +			result = L2CAP_MR_PEND;
> +		} else {
> +			chan->move_state = L2CAP_MOVE_WAIT_CONFIRM;
> +			result = L2CAP_MR_SUCCESS;
> +		}
> +	} else {
> +		chan->move_state = L2CAP_MOVE_WAIT_PREPARE;
> +		/* Placeholder - uncomment when amp functions are available */
> +		/*amp_accept_physical(chan, req->dest_amp_id);*/
> +		result = L2CAP_MR_PEND;
> +	}
> +
> +send_move_response:
>  	l2cap_send_move_chan_rsp(conn, cmd->ident, icid, result);
>  
>  	if (chan)

Best regards 
Andrei Emeltchenko 

^ permalink raw reply

* [PATCH 8/8] adapter: Remove not used defines
From: Szymon Janc @ 2012-10-16 10:12 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1350382379-30489-1-git-send-email-szymon.janc@tieto.com>

check_address and OFF_TIMER are not used anymore.

---
 src/adapter.c |    4 ----
 1 file changed, 4 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index 86a2124..7c2aec0 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -80,10 +80,6 @@
 #define IO_CAPABILITY_KEYBOARDDISPLAY	0x04
 #define IO_CAPABILITY_INVALID		0xFF
 
-#define check_address(address) bachk(address)
-
-#define OFF_TIMER 3
-
 #define REMOVE_TEMP_TIMEOUT (3 * 60)
 #define PENDING_FOUND_MAX 5
 
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 7/8] Simplify str2ba function
From: Szymon Janc @ 2012-10-16 10:12 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1350382379-30489-1-git-send-email-szymon.janc@tieto.com>

Reverse loop iteration so that baswap call is not needed.

---
 lib/bluetooth.c |    7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/lib/bluetooth.c b/lib/bluetooth.c
index d092d01..d9360dc 100644
--- a/lib/bluetooth.c
+++ b/lib/bluetooth.c
@@ -82,7 +82,6 @@ int ba2str(const bdaddr_t *ba, char *str)
 
 int str2ba(const char *str, bdaddr_t *ba)
 {
-	bdaddr_t b;
 	int i;
 
 	if (bachk(str) < 0) {
@@ -90,10 +89,8 @@ int str2ba(const char *str, bdaddr_t *ba)
 		return -1;
 	}
 
-	for (i = 0; i < 6; i++, str += 3)
-		b.b[i] = strtol(str, NULL, 16);
-
-	baswap(ba, &b);
+	for (i = 5; i >= 0; i--, str += 3)
+		ba->b[i] = strtol(str, NULL, 16);
 
 	return 0;
 }
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 6/8] mgmt: Remove not used members from controller_info
From: Szymon Janc @ 2012-10-16 10:12 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1350382379-30489-1-git-send-email-szymon.janc@tieto.com>

notified, version, manufacturer and dev_class were not used for
anything usefull. For debug print reply can be used directly.

---
 src/mgmt.c |   12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/src/mgmt.c b/src/mgmt.c
index 8caabe8..627fb4b 100644
--- a/src/mgmt.c
+++ b/src/mgmt.c
@@ -61,13 +61,9 @@ struct pending_uuid {
 static int max_index = -1;
 static struct controller_info {
 	gboolean valid;
-	gboolean notified;
 	bdaddr_t bdaddr;
-	uint8_t version;
-	uint16_t manufacturer;
 	uint32_t supported_settings;
 	uint32_t current_settings;
-	uint8_t dev_class[3];
 	GSList *connections;
 	uint8_t discov_type;
 
@@ -1075,20 +1071,16 @@ static void read_info_complete(int sk, uint16_t index, void *buf, size_t len)
 	info = &controllers[index];
 
 	bacpy(&info->bdaddr, &rp->bdaddr);
-	info->version = rp->version;
-	info->manufacturer = bt_get_le16(&rp->manufacturer);
 
 	memcpy(&info->supported_settings, &rp->supported_settings,
 					sizeof(info->supported_settings));
 	memcpy(&info->current_settings, &rp->current_settings,
 					sizeof(info->current_settings));
 
-	memcpy(info->dev_class, rp->dev_class, sizeof(info->dev_class));
-
 	ba2str(&info->bdaddr, addr);
 	DBG("hci%u addr %s version %u manufacturer %u class 0x%02x%02x%02x\n",
-		index, addr, info->version, info->manufacturer,
-		info->dev_class[2], info->dev_class[1], info->dev_class[0]);
+		index, addr, rp->version, bt_get_le16(&rp->manufacturer),
+		rp->dev_class[2], rp->dev_class[1], rp->dev_class[0]);
 	DBG("hci%u settings", index);
 	DBG("hci%u name %s", index, (char *) rp->name);
 	DBG("hci%u short name %s", index, (char *) rp->short_name);
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 5/8] input: Remove not needed adapters list in manager
From: Szymon Janc @ 2012-10-16 10:12 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1350382379-30489-1-git-send-email-szymon.janc@tieto.com>

adapters list is not used for anything usefull.

---
 profiles/input/manager.c |   15 +--------------
 1 file changed, 1 insertion(+), 14 deletions(-)

diff --git a/profiles/input/manager.c b/profiles/input/manager.c
index 3f68620..a2690b5 100644
--- a/profiles/input/manager.c
+++ b/profiles/input/manager.c
@@ -44,8 +44,6 @@
 
 static int idle_timeout = 0;
 
-static GSList *adapters = NULL;
-
 static void input_remove(struct btd_device *device, const char *uuid)
 {
 	const gchar *path = device_get_path(device);
@@ -77,24 +75,13 @@ static void hid_device_remove(struct btd_profile *p, struct btd_device *device)
 
 static int hid_server_probe(struct btd_profile *p, struct btd_adapter *adapter)
 {
-	int ret;
-
-	ret = server_start(adapter_get_address(adapter));
-	if (ret < 0)
-		return ret;
-
-	adapters = g_slist_append(adapters, btd_adapter_ref(adapter));
-
-	return 0;
+	return server_start(adapter_get_address(adapter));
 }
 
 static void hid_server_remove(struct btd_profile *p,
 						struct btd_adapter *adapter)
 {
 	server_stop(adapter_get_address(adapter));
-
-	adapters = g_slist_remove(adapters, adapter);
-	btd_adapter_unref(adapter);
 }
 
 static struct btd_profile input_profile = {
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 4/8] network: Remove redundant struct network_peer members
From: Szymon Janc @ 2012-10-16 10:12 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1350382379-30489-1-git-send-email-szymon.janc@tieto.com>

struct network_peer holds reference to btd_device so path, src and dst
can be accessed from it.

---
 profiles/network/common.c     |    2 +-
 profiles/network/common.h     |    2 +-
 profiles/network/connection.c |   66 ++++++++++++++++++++---------------------
 profiles/network/connection.h |    5 ++--
 profiles/network/manager.c    |   22 ++++----------
 5 files changed, 43 insertions(+), 54 deletions(-)

diff --git a/profiles/network/common.c b/profiles/network/common.c
index da493c1..34ab788 100644
--- a/profiles/network/common.c
+++ b/profiles/network/common.c
@@ -124,7 +124,7 @@ int bnep_cleanup(void)
 	return 0;
 }
 
-int bnep_kill_connection(bdaddr_t *dst)
+int bnep_kill_connection(const bdaddr_t *dst)
 {
 	struct bnep_conndel_req req;
 
diff --git a/profiles/network/common.h b/profiles/network/common.h
index 62f2f59..0a8c4fe 100644
--- a/profiles/network/common.h
+++ b/profiles/network/common.h
@@ -28,7 +28,7 @@ uint16_t bnep_service_id(const char *svc);
 const char *bnep_uuid(uint16_t id);
 const char *bnep_name(uint16_t id);
 
-int bnep_kill_connection(bdaddr_t *dst);
+int bnep_kill_connection(const bdaddr_t *dst);
 int bnep_kill_all_connections(void);
 
 int bnep_connadd(int sk, uint16_t role, char *dev);
diff --git a/profiles/network/connection.c b/profiles/network/connection.c
index 9646e5a..8564bca 100644
--- a/profiles/network/connection.c
+++ b/profiles/network/connection.c
@@ -59,9 +59,6 @@ typedef enum {
 } conn_state;
 
 struct network_peer {
-	bdaddr_t	src;
-	bdaddr_t	dst;
-	char		*path;		/* D-Bus path */
 	struct btd_device *device;
 	GSList		*connections;
 };
@@ -86,12 +83,12 @@ struct __service_16 {
 
 static GSList *peers = NULL;
 
-static struct network_peer *find_peer(GSList *list, const char *path)
+static struct network_peer *find_peer(GSList *list, struct btd_device *device)
 {
 	for (; list; list = list->next) {
 		struct network_peer *peer = list->data;
 
-		if (!strcmp(peer->path, path))
+		if (peer->device == device)
 			return peer;
 	}
 
@@ -116,14 +113,15 @@ static gboolean bnep_watchdog_cb(GIOChannel *chan, GIOCondition cond,
 	struct network_conn *nc = data;
 	gboolean connected = FALSE;
 	const char *property = "";
+	const char *path = device_get_path(nc->peer->device);
 
-	emit_property_changed(nc->peer->path,
+	emit_property_changed(path,
 				NETWORK_PEER_INTERFACE, "Connected",
 				DBUS_TYPE_BOOLEAN, &connected);
-	emit_property_changed(nc->peer->path,
+	emit_property_changed(path,
 				NETWORK_PEER_INTERFACE, "Interface",
 				DBUS_TYPE_STRING, &property);
-	emit_property_changed(nc->peer->path,
+	emit_property_changed(path,
 				NETWORK_PEER_INTERFACE, "UUID",
 				DBUS_TYPE_STRING, &property);
 	device_remove_disconnect_watch(nc->peer->device, nc->dc_id);
@@ -180,7 +178,7 @@ static void connection_destroy(DBusConnection *conn, void *user_data)
 
 	if (nc->state == CONNECTED) {
 		bnep_if_down(nc->dev);
-		bnep_kill_connection(&nc->peer->dst);
+		bnep_kill_connection(device_get_address(nc->peer->device));
 	} else if (nc->io)
 		cancel_connection(nc, NULL);
 }
@@ -190,7 +188,7 @@ static void disconnect_cb(struct btd_device *device, gboolean removal,
 {
 	struct network_conn *nc = user_data;
 
-	info("Network: disconnect %s", nc->peer->path);
+	info("Network: disconnect %s", device_get_path(nc->peer->device));
 
 	connection_destroy(NULL, user_data);
 }
@@ -206,6 +204,7 @@ static gboolean bnep_setup_cb(GIOChannel *chan, GIOCondition cond,
 	int sk;
 	const char *pdev, *uuid;
 	gboolean connected;
+	const char *path;
 
 	if (cond & G_IO_NVAL)
 		return FALSE;
@@ -277,14 +276,16 @@ static gboolean bnep_setup_cb(GIOChannel *chan, GIOCondition cond,
 	dbus_message_unref(nc->msg);
 	nc->msg = NULL;
 
+	path = device_get_path(nc->peer->device);
+
 	connected = TRUE;
-	emit_property_changed(nc->peer->path,
+	emit_property_changed(path,
 				NETWORK_PEER_INTERFACE, "Connected",
 				DBUS_TYPE_BOOLEAN, &connected);
-	emit_property_changed(nc->peer->path,
+	emit_property_changed(path,
 				NETWORK_PEER_INTERFACE, "Interface",
 				DBUS_TYPE_STRING, &pdev);
-	emit_property_changed(nc->peer->path,
+	emit_property_changed(path,
 				NETWORK_PEER_INTERFACE, "UUID",
 				DBUS_TYPE_STRING, &uuid);
 
@@ -404,6 +405,8 @@ static DBusMessage *connection_connect(DBusConnection *conn,
 	const char *svc;
 	uint16_t id;
 	GError *err = NULL;
+	const bdaddr_t *src;
+	const bdaddr_t *dst;
 
 	if (dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &svc,
 						DBUS_TYPE_INVALID) == FALSE)
@@ -417,10 +420,13 @@ static DBusMessage *connection_connect(DBusConnection *conn,
 	if (nc->state != DISCONNECTED)
 		return btd_error_already_connected(msg);
 
+	src = adapter_get_address(device_get_adapter(peer->device));
+	dst = device_get_address(peer->device);
+
 	nc->io = bt_io_connect(connect_cb, nc,
 				NULL, &err,
-				BT_IO_OPT_SOURCE_BDADDR, &peer->src,
-				BT_IO_OPT_DEST_BDADDR, &peer->dst,
+				BT_IO_OPT_SOURCE_BDADDR, src,
+				BT_IO_OPT_DEST_BDADDR, dst,
 				BT_IO_OPT_PSM, BNEP_PSM,
 				BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_MEDIUM,
 				BT_IO_OPT_OMTU, BNEP_MTU,
@@ -544,7 +550,6 @@ static void peer_free(struct network_peer *peer)
 {
 	g_slist_free_full(peer->connections, connection_free);
 	btd_device_unref(peer->device);
-	g_free(peer->path);
 	g_free(peer);
 }
 
@@ -553,7 +558,7 @@ static void path_unregister(void *data)
 	struct network_peer *peer = data;
 
 	DBG("Unregistered interface %s on path %s",
-		NETWORK_PEER_INTERFACE, peer->path);
+		NETWORK_PEER_INTERFACE, device_get_path(peer->device));
 
 	peers = g_slist_remove(peers, peer);
 	peer_free(peer);
@@ -578,11 +583,11 @@ static const GDBusSignalTable connection_signals[] = {
 	{ }
 };
 
-void connection_unregister(const char *path)
+void connection_unregister(struct btd_device *device)
 {
 	struct network_peer *peer;
 
-	peer = find_peer(peers, path);
+	peer = find_peer(peers, device);
 	if (!peer)
 		return;
 
@@ -590,20 +595,19 @@ void connection_unregister(const char *path)
 	peer->connections = NULL;
 
 	g_dbus_unregister_interface(btd_get_dbus_connection(),
-						path, NETWORK_PEER_INTERFACE);
+						device_get_path(device),
+						NETWORK_PEER_INTERFACE);
 }
 
-static struct network_peer *create_peer(struct btd_device *device,
-					const char *path, const bdaddr_t *src,
-					const bdaddr_t *dst)
+static struct network_peer *create_peer(struct btd_device *device)
 {
 	struct network_peer *peer;
+	const char *path;
 
 	peer = g_new0(struct network_peer, 1);
 	peer->device = btd_device_ref(device);
-	peer->path = g_strdup(path);
-	bacpy(&peer->src, src);
-	bacpy(&peer->dst, dst);
+
+	path = device_get_path(device);
 
 	if (g_dbus_register_interface(btd_get_dbus_connection(), path,
 					NETWORK_PEER_INTERFACE,
@@ -622,18 +626,14 @@ static struct network_peer *create_peer(struct btd_device *device,
 	return peer;
 }
 
-int connection_register(struct btd_device *device, const char *path,
-			const bdaddr_t *src, const bdaddr_t *dst, uint16_t id)
+int connection_register(struct btd_device *device, uint16_t id)
 {
 	struct network_peer *peer;
 	struct network_conn *nc;
 
-	if (!path)
-		return -EINVAL;
-
-	peer = find_peer(peers, path);
+	peer = find_peer(peers, device);
 	if (!peer) {
-		peer = create_peer(device, path, src, dst);
+		peer = create_peer(device);
 		if (!peer)
 			return -1;
 		peers = g_slist_append(peers, peer);
diff --git a/profiles/network/connection.h b/profiles/network/connection.h
index efc4ccb..50c0774 100644
--- a/profiles/network/connection.h
+++ b/profiles/network/connection.h
@@ -21,6 +21,5 @@
  *
  */
 
-int connection_register(struct btd_device *device, const char *path,
-			const bdaddr_t *src, const bdaddr_t *dst, uint16_t id);
-void connection_unregister(const char *path);
+int connection_register(struct btd_device *device, uint16_t id);
+void connection_unregister(struct btd_device *device);
diff --git a/profiles/network/manager.c b/profiles/network/manager.c
index 049fbc7..aa564bd 100644
--- a/profiles/network/manager.c
+++ b/profiles/network/manager.c
@@ -76,33 +76,23 @@ done:
 static int network_probe(struct btd_profile *p, struct btd_device *device,
 								GSList *uuids)
 {
-	struct btd_adapter *adapter = device_get_adapter(device);
-	const gchar *path = device_get_path(device);
-	const bdaddr_t *src;
-	const bdaddr_t *dst;
-
-	DBG("path %s", path);
-
-	src = adapter_get_address(adapter);
-	dst = device_get_address(device);
+	DBG("path %s", device_get_path(device));
 
 	if (g_slist_find_custom(uuids, PANU_UUID, bt_uuid_strcmp))
-		connection_register(device, path, src, dst, BNEP_SVC_PANU);
+		connection_register(device, BNEP_SVC_PANU);
 	if (g_slist_find_custom(uuids, GN_UUID, bt_uuid_strcmp))
-		connection_register(device, path, src, dst, BNEP_SVC_GN);
+		connection_register(device, BNEP_SVC_GN);
 	if (g_slist_find_custom(uuids, NAP_UUID, bt_uuid_strcmp))
-		connection_register(device, path, src, dst, BNEP_SVC_NAP);
+		connection_register(device, BNEP_SVC_NAP);
 
 	return 0;
 }
 
 static void network_remove(struct btd_profile *p, struct btd_device *device)
 {
-	const gchar *path = device_get_path(device);
-
-	DBG("path %s", path);
+	DBG("path %s", device_get_path(device));
 
-	connection_unregister(path);
+	connection_unregister(device);
 }
 
 static int network_server_probe(struct btd_profile *p,
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 3/8] audio: Remove path from struct audio_device
From: Szymon Janc @ 2012-10-16 10:12 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1350382379-30489-1-git-send-email-szymon.janc@tieto.com>

struct audio_device is holding a reference to btd_device and path can
be easily taken from it when needed.

---
NOTE:
src and dst can also be removed but this require a bit more work and is not
included in this patch

 audio/control.c   |   23 +++++++++++--------
 audio/device.c    |   26 +++++++++------------
 audio/device.h    |    5 ++--
 audio/gateway.c   |   16 +++++++++----
 audio/headset.c   |   66 ++++++++++++++++++++++++++++++-----------------------
 audio/manager.c   |   10 ++++----
 audio/sink.c      |   17 +++++++-------
 audio/source.c    |   13 ++++++-----
 audio/transport.c |    9 +++++---
 9 files changed, 102 insertions(+), 83 deletions(-)

diff --git a/audio/control.c b/audio/control.c
index 8bb085a..37b027b 100644
--- a/audio/control.c
+++ b/audio/control.c
@@ -46,6 +46,9 @@
 #include <dbus/dbus.h>
 #include <gdbus.h>
 
+#include "../src/adapter.h"
+#include "../src/device.h"
+
 #include "log.h"
 #include "error.h"
 #include "device.h"
@@ -70,6 +73,7 @@ static void state_changed(struct audio_device *dev, avctp_state_t old_state,
 {
 	DBusConnection *conn = btd_get_dbus_connection();
 	struct control *control = dev->control;
+	const char *path = device_get_path(dev->btd_dev);
 
 	switch (new_state) {
 	case AVCTP_STATE_DISCONNECTED:
@@ -94,10 +98,9 @@ static void state_changed(struct audio_device *dev, avctp_state_t old_state,
 		if (old_state != AVCTP_STATE_CONNECTED)
 			break;
 
-		g_dbus_emit_signal(conn, dev->path,
-					AUDIO_CONTROL_INTERFACE,
+		g_dbus_emit_signal(conn, path, AUDIO_CONTROL_INTERFACE,
 					"Disconnected", DBUS_TYPE_INVALID);
-		g_dbus_emit_property_changed(conn, dev->path,
+		g_dbus_emit_property_changed(conn, path,
 					AUDIO_CONTROL_INTERFACE, "Connected");
 
 		break;
@@ -116,10 +119,10 @@ static void state_changed(struct audio_device *dev, avctp_state_t old_state,
 			control->connect = NULL;
 		}
 
-		g_dbus_emit_signal(conn, dev->path,
+		g_dbus_emit_signal(conn, path,
 				AUDIO_CONTROL_INTERFACE, "Connected",
 				DBUS_TYPE_INVALID);
-		g_dbus_emit_property_changed(conn, dev->path,
+		g_dbus_emit_property_changed(conn, path,
 					AUDIO_CONTROL_INTERFACE, "Connected");
 		break;
 	default:
@@ -298,7 +301,7 @@ static void path_unregister(void *data)
 	struct control *control = dev->control;
 
 	DBG("Unregistered interface %s on path %s",
-		AUDIO_CONTROL_INTERFACE, dev->path);
+		AUDIO_CONTROL_INTERFACE, device_get_path(dev->btd_dev));
 
 	if (control->session)
 		avctp_disconnect(control->session);
@@ -315,7 +318,8 @@ static void path_unregister(void *data)
 
 void control_unregister(struct audio_device *dev)
 {
-	g_dbus_unregister_interface(btd_get_dbus_connection(), dev->path,
+	g_dbus_unregister_interface(btd_get_dbus_connection(),
+						device_get_path(dev->btd_dev),
 						AUDIO_CONTROL_INTERFACE);
 }
 
@@ -329,7 +333,8 @@ struct control *control_init(struct audio_device *dev, GSList *uuids)
 {
 	struct control *control;
 
-	if (!g_dbus_register_interface(btd_get_dbus_connection(), dev->path,
+	if (!g_dbus_register_interface(btd_get_dbus_connection(),
+					device_get_path(dev->btd_dev),
 					AUDIO_CONTROL_INTERFACE,
 					control_methods, control_signals,
 					control_properties, dev,
@@ -337,7 +342,7 @@ struct control *control_init(struct audio_device *dev, GSList *uuids)
 		return NULL;
 
 	DBG("Registered interface %s on path %s",
-		AUDIO_CONTROL_INTERFACE, dev->path);
+		AUDIO_CONTROL_INTERFACE, device_get_path(dev->btd_dev));
 
 	control = g_new0(struct control, 1);
 
diff --git a/audio/device.c b/audio/device.c
index be63031..aee4161 100644
--- a/audio/device.c
+++ b/audio/device.c
@@ -117,7 +117,6 @@ static void device_free(struct audio_device *dev)
 
 	btd_device_unref(dev->btd_dev);
 
-	g_free(dev->path);
 	g_free(dev);
 }
 
@@ -266,7 +265,7 @@ static void device_set_state(struct audio_device *dev, audio_state_t new_state)
 		g_dbus_send_message(conn, reply);
 	}
 
-	emit_property_changed(dev->path,
+	emit_property_changed(device_get_path(dev->btd_dev),
 				AUDIO_INTERFACE, "State",
 				DBUS_TYPE_STRING, &state_str);
 }
@@ -619,35 +618,31 @@ static const GDBusSignalTable dev_signals[] = {
 };
 
 struct audio_device *audio_device_register(struct btd_device *device,
-					const char *path, const bdaddr_t *src,
-					const bdaddr_t *dst)
+							const bdaddr_t *src,
+							const bdaddr_t *dst)
 {
 	struct audio_device *dev;
 
-	if (!path)
-		return NULL;
-
 	dev = g_new0(struct audio_device, 1);
 
 	dev->btd_dev = btd_device_ref(device);
-	dev->path = g_strdup(path);
 	bacpy(&dev->dst, dst);
 	bacpy(&dev->src, src);
 	dev->priv = g_new0(struct dev_priv, 1);
 	dev->priv->state = AUDIO_STATE_DISCONNECTED;
 
 	if (!g_dbus_register_interface(btd_get_dbus_connection(),
-					dev->path, AUDIO_INTERFACE,
-					dev_methods, dev_signals, NULL,
-					dev, NULL)) {
+					device_get_path(dev->btd_dev),
+					AUDIO_INTERFACE, dev_methods,
+					dev_signals, NULL, dev, NULL)) {
 		error("Unable to register %s on %s", AUDIO_INTERFACE,
-								dev->path);
+						device_get_path(dev->btd_dev));
 		device_free(dev);
 		return NULL;
 	}
 
 	DBG("Registered interface %s on path %s", AUDIO_INTERFACE,
-								dev->path);
+						device_get_path(dev->btd_dev));
 
 	if (sink_callback_id == 0)
 		sink_callback_id = sink_add_state_cb(device_sink_cb, NULL);
@@ -714,8 +709,9 @@ void audio_device_unregister(struct audio_device *device)
 	if (device->control)
 		control_unregister(device);
 
-	g_dbus_unregister_interface(btd_get_dbus_connection(), device->path,
-						AUDIO_INTERFACE);
+	g_dbus_unregister_interface(btd_get_dbus_connection(),
+					device_get_path(device->btd_dev),
+					AUDIO_INTERFACE);
 
 	device_free(device);
 }
diff --git a/audio/device.h b/audio/device.h
index 2b92528..1e2cac1 100644
--- a/audio/device.h
+++ b/audio/device.h
@@ -33,7 +33,6 @@ struct dev_priv;
 struct audio_device {
 	struct btd_device *btd_dev;
 
-	char *path;
 	bdaddr_t src;
 	bdaddr_t dst;
 
@@ -53,8 +52,8 @@ struct audio_device {
 };
 
 struct audio_device *audio_device_register(struct btd_device *device,
-					const char *path, const bdaddr_t *src,
-					const bdaddr_t *dst);
+							const bdaddr_t *src,
+							const bdaddr_t *dst);
 
 void audio_device_unregister(struct audio_device *device);
 
diff --git a/audio/gateway.c b/audio/gateway.c
index b4d96f0..2d5b4aa 100644
--- a/audio/gateway.c
+++ b/audio/gateway.c
@@ -42,6 +42,9 @@
 #include <bluetooth/sdp.h>
 #include <bluetooth/sdp_lib.h>
 
+#include "../src/adapter.h"
+#include "../src/device.h"
+
 #include "sdp-client.h"
 #include "device.h"
 #include "gateway.h"
@@ -135,7 +138,7 @@ static void change_state(struct audio_device *dev, gateway_state_t new_state)
 	old_state = gw->state;
 	gw->state = new_state;
 
-	emit_property_changed(dev->path,
+	emit_property_changed(device_get_path(dev->btd_dev),
 				AUDIO_GATEWAY_INTERFACE, "State",
 				DBUS_TYPE_STRING, &val);
 
@@ -649,7 +652,8 @@ static DBusMessage *ag_disconnect(DBusConnection *conn, DBusMessage *msg,
 
 	gateway_close(device);
 	ba2str(&device->dst, gw_addr);
-	DBG("Disconnected from %s, %s", gw_addr, device->path);
+	DBG("Disconnected from %s, %s", gw_addr,
+					device_get_path(device->btd_dev));
 
 	return reply;
 }
@@ -779,7 +783,7 @@ static void path_unregister(void *data)
 	struct audio_device *dev = data;
 
 	DBG("Unregistered interface %s on path %s",
-		AUDIO_GATEWAY_INTERFACE, dev->path);
+		AUDIO_GATEWAY_INTERFACE, device_get_path(dev->btd_dev));
 
 	gateway_close(dev);
 
@@ -792,13 +796,15 @@ void gateway_unregister(struct audio_device *dev)
 	if (dev->gateway->agent)
 		agent_disconnect(dev, dev->gateway->agent);
 
-	g_dbus_unregister_interface(btd_get_dbus_connection(), dev->path,
+	g_dbus_unregister_interface(btd_get_dbus_connection(),
+						device_get_path(dev->btd_dev),
 						AUDIO_GATEWAY_INTERFACE);
 }
 
 struct gateway *gateway_init(struct audio_device *dev)
 {
-	if (!g_dbus_register_interface(btd_get_dbus_connection(), dev->path,
+	if (!g_dbus_register_interface(btd_get_dbus_connection(),
+					device_get_path(dev->btd_dev),
 					AUDIO_GATEWAY_INTERFACE,
 					gateway_methods, gateway_signals,
 					NULL, dev, path_unregister))
diff --git a/audio/headset.c b/audio/headset.c
index c822e35..5e42a61 100644
--- a/audio/headset.c
+++ b/audio/headset.c
@@ -582,7 +582,7 @@ static void sco_connect_cb(GIOChannel *chan, GError *err, gpointer user_data)
 		return;
 	}
 
-	DBG("SCO socket opened for headset %s", dev->path);
+	DBG("SCO socket opened for headset %s", device_get_path(dev->btd_dev));
 
 	sk = g_io_channel_unix_get_fd(chan);
 
@@ -802,7 +802,8 @@ static int key_press(struct audio_device *device, const char *buf)
 	if (strlen(buf) < 9)
 		return -EINVAL;
 
-	g_dbus_emit_signal(btd_get_dbus_connection(), device->path,
+	g_dbus_emit_signal(btd_get_dbus_connection(),
+			device_get_path(device->btd_dev),
 			AUDIO_HEADSET_INTERFACE, "AnswerRequested",
 			DBUS_TYPE_INVALID);
 
@@ -847,7 +848,8 @@ int telephony_terminate_call_rsp(void *telephony_device,
 	if (err != CME_ERROR_NONE)
 		return telephony_generic_rsp(telephony_device, err);
 
-	g_dbus_emit_signal(btd_get_dbus_connection(), device->path,
+	g_dbus_emit_signal(btd_get_dbus_connection(),
+			device_get_path(device->btd_dev),
 			AUDIO_HEADSET_INTERFACE, "CallTerminated",
 			DBUS_TYPE_INVALID);
 
@@ -982,12 +984,13 @@ static int headset_set_gain(struct audio_device *device, uint16_t gain, char typ
 		return -EINVAL;
 	}
 
-	g_dbus_emit_signal(btd_get_dbus_connection(), device->path,
+	g_dbus_emit_signal(btd_get_dbus_connection(),
+				device_get_path(device->btd_dev),
 				AUDIO_HEADSET_INTERFACE, name,
 				DBUS_TYPE_UINT16, &gain,
 				DBUS_TYPE_INVALID);
 
-	emit_property_changed(device->path,
+	emit_property_changed(device_get_path(device->btd_dev),
 				AUDIO_HEADSET_INTERFACE, property,
 				DBUS_TYPE_UINT16, &gain);
 
@@ -1401,7 +1404,7 @@ void headset_connect_cb(GIOChannel *chan, GError *err, gpointer user_data)
 	g_io_add_watch(chan, G_IO_IN | G_IO_ERR | G_IO_HUP| G_IO_NVAL,
 			(GIOFunc) rfcomm_io_cb, dev);
 
-	DBG("%s: Connected to %s", dev->path, hs_address);
+	DBG("%s: Connected to %s", device_get_path(dev->btd_dev), hs_address);
 
 	hs->slc = g_new0(struct headset_slc, 1);
 	hs->slc->sp_gain = 15;
@@ -1619,8 +1622,8 @@ static int rfcomm_connect(struct audio_device *dev, headset_stream_cb_t cb,
 
 	ba2str(&dev->dst, address);
 
-	DBG("%s: Connecting to %s channel %d", dev->path, address,
-		hs->rfcomm_ch);
+	DBG("%s: Connecting to %s channel %d", device_get_path(dev->btd_dev),
+						address, hs->rfcomm_ch);
 
 	hs->tmp_rfcomm = bt_io_connect(headset_connect_cb, dev,
 					NULL, &err,
@@ -1686,7 +1689,8 @@ static DBusMessage *hs_disconnect(DBusConnection *conn, DBusMessage *msg,
 
 	headset_shutdown(device);
 	ba2str(&device->dst, hs_address);
-	info("Disconnected from %s, %s", hs_address, device->path);
+	info("Disconnected from %s, %s", hs_address,
+					device_get_path(device->btd_dev));
 
 	return dbus_message_new_method_return(msg);
 
@@ -2024,15 +2028,16 @@ static void path_unregister(void *data)
 	}
 
 	DBG("Unregistered interface %s on path %s",
-		AUDIO_HEADSET_INTERFACE, dev->path);
+		AUDIO_HEADSET_INTERFACE, device_get_path(dev->btd_dev));
 
 	headset_free(dev);
 }
 
 void headset_unregister(struct audio_device *dev)
 {
-	g_dbus_unregister_interface(btd_get_dbus_connection(), dev->path,
-		AUDIO_HEADSET_INTERFACE);
+	g_dbus_unregister_interface(btd_get_dbus_connection(),
+						device_get_path(dev->btd_dev),
+						AUDIO_HEADSET_INTERFACE);
 }
 
 struct headset *headset_init(struct audio_device *dev, GSList *uuids,
@@ -2046,7 +2051,8 @@ struct headset *headset_init(struct audio_device *dev, GSList *uuids,
 
 	headset_update(dev, hs, uuids);
 
-	if (!g_dbus_register_interface(btd_get_dbus_connection(), dev->path,
+	if (!g_dbus_register_interface(btd_get_dbus_connection(),
+					device_get_path(dev->btd_dev),
 					AUDIO_HEADSET_INTERFACE,
 					headset_methods, headset_signals, NULL,
 					dev, path_unregister)) {
@@ -2055,7 +2061,7 @@ struct headset *headset_init(struct audio_device *dev, GSList *uuids,
 	}
 
 	DBG("Registered interface %s on path %s", AUDIO_HEADSET_INTERFACE,
-								dev->path);
+						device_get_path(dev->btd_dev));
 
 	return hs;
 }
@@ -2341,15 +2347,16 @@ void headset_set_state(struct audio_device *dev, headset_state_t state)
 		value = FALSE;
 		close_sco(dev);
 		headset_close_rfcomm(dev);
-		emit_property_changed(dev->path,
+		emit_property_changed(device_get_path(dev->btd_dev),
 					AUDIO_HEADSET_INTERFACE, "State",
 					DBUS_TYPE_STRING, &state_str);
-		g_dbus_emit_signal(btd_get_dbus_connection(), dev->path,
+		g_dbus_emit_signal(btd_get_dbus_connection(),
+					device_get_path(dev->btd_dev),
 					AUDIO_HEADSET_INTERFACE,
 					"Disconnected",
 					DBUS_TYPE_INVALID);
 		if (hs->state > HEADSET_STATE_CONNECTING) {
-			emit_property_changed(dev->path,
+			emit_property_changed(device_get_path(dev->btd_dev),
 					AUDIO_HEADSET_INTERFACE, "Connected",
 					DBUS_TYPE_BOOLEAN, &value);
 			telephony_device_disconnected(dev);
@@ -2357,14 +2364,14 @@ void headset_set_state(struct audio_device *dev, headset_state_t state)
 		active_devices = g_slist_remove(active_devices, dev);
 		break;
 	case HEADSET_STATE_CONNECTING:
-		emit_property_changed(dev->path,
+		emit_property_changed(device_get_path(dev->btd_dev),
 					AUDIO_HEADSET_INTERFACE, "State",
 					DBUS_TYPE_STRING, &state_str);
 		break;
 	case HEADSET_STATE_CONNECTED:
 		close_sco(dev);
 		if (hs->state != HEADSET_STATE_PLAY_IN_PROGRESS)
-			emit_property_changed(dev->path,
+			emit_property_changed(device_get_path(dev->btd_dev),
 					AUDIO_HEADSET_INTERFACE, "State",
 					DBUS_TYPE_STRING, &state_str);
 		if (hs->state < state) {
@@ -2372,12 +2379,13 @@ void headset_set_state(struct audio_device *dev, headset_state_t state)
 				slc->inband_ring = TRUE;
 			else
 				slc->inband_ring = FALSE;
-			g_dbus_emit_signal(btd_get_dbus_connection(), dev->path,
+			g_dbus_emit_signal(btd_get_dbus_connection(),
+						device_get_path(dev->btd_dev),
 						AUDIO_HEADSET_INTERFACE,
 						"Connected",
 						DBUS_TYPE_INVALID);
 			value = TRUE;
-			emit_property_changed(dev->path,
+			emit_property_changed(device_get_path(dev->btd_dev),
 						AUDIO_HEADSET_INTERFACE,
 						"Connected",
 						DBUS_TYPE_BOOLEAN, &value);
@@ -2385,11 +2393,12 @@ void headset_set_state(struct audio_device *dev, headset_state_t state)
 			telephony_device_connected(dev);
 		} else if (hs->state == HEADSET_STATE_PLAYING) {
 			value = FALSE;
-			g_dbus_emit_signal(btd_get_dbus_connection(), dev->path,
+			g_dbus_emit_signal(btd_get_dbus_connection(),
+						device_get_path(dev->btd_dev),
 						AUDIO_HEADSET_INTERFACE,
 						"Stopped",
 						DBUS_TYPE_INVALID);
-			emit_property_changed(dev->path,
+			emit_property_changed(device_get_path(dev->btd_dev),
 						AUDIO_HEADSET_INTERFACE,
 						"Playing",
 						DBUS_TYPE_BOOLEAN, &value);
@@ -2399,7 +2408,7 @@ void headset_set_state(struct audio_device *dev, headset_state_t state)
 		break;
 	case HEADSET_STATE_PLAYING:
 		value = TRUE;
-		emit_property_changed(dev->path,
+		emit_property_changed(device_get_path(dev->btd_dev),
 					AUDIO_HEADSET_INTERFACE, "State",
 					DBUS_TYPE_STRING, &state_str);
 
@@ -2409,10 +2418,11 @@ void headset_set_state(struct audio_device *dev, headset_state_t state)
 					G_IO_ERR | G_IO_NVAL,
 					(GIOFunc) sco_cb, dev);
 
-		g_dbus_emit_signal(btd_get_dbus_connection(), dev->path,
+		g_dbus_emit_signal(btd_get_dbus_connection(),
+					device_get_path(dev->btd_dev),
 					AUDIO_HEADSET_INTERFACE, "Playing",
 					DBUS_TYPE_INVALID);
-		emit_property_changed(dev->path,
+		emit_property_changed(device_get_path(dev->btd_dev),
 					AUDIO_HEADSET_INTERFACE, "Playing",
 					DBUS_TYPE_BOOLEAN, &value);
 
@@ -2425,8 +2435,8 @@ void headset_set_state(struct audio_device *dev, headset_state_t state)
 
 	hs->state = state;
 
-	DBG("State changed %s: %s -> %s", dev->path, str_state[old_state],
-		str_state[state]);
+	DBG("State changed %s: %s -> %s", device_get_path(dev->btd_dev),
+				str_state[old_state], str_state[state]);
 
 	for (l = headset_callbacks; l != NULL; l = l->next) {
 		struct headset_state_callback *cb = l->data;
diff --git a/audio/manager.c b/audio/manager.c
index b052009..4ea61bf 100644
--- a/audio/manager.c
+++ b/audio/manager.c
@@ -435,7 +435,7 @@ static void gateway_auth_cb(DBusError *derr, void *user_data)
 
 		ba2str(&device->dst, ag_address);
 		DBG("Accepted AG connection from %s for %s",
-			ag_address, device->path);
+			ag_address, device_get_path(device->btd_dev));
 
 		gateway_start_service(device);
 	}
@@ -1240,7 +1240,8 @@ GSList *manager_find_devices(const char *path,
 	for (l = devices; l != NULL; l = l->next) {
 		struct audio_device *dev = l->data;
 
-		if ((path && (strcmp(path, "")) && strcmp(dev->path, path)))
+		if ((path && (strcmp(path, "")) &&
+				strcmp(device_get_path(dev->btd_dev), path)))
 			continue;
 
 		if ((src && bacmp(src, BDADDR_ANY)) && bacmp(&dev->src, src))
@@ -1304,7 +1305,6 @@ struct audio_device *manager_get_device(const bdaddr_t *src,
 	struct btd_adapter *adapter;
 	struct btd_device *device;
 	char addr[18];
-	const char *path;
 
 	dev = manager_find_device(NULL, src, dst, NULL, FALSE);
 	if (dev)
@@ -1330,9 +1330,7 @@ struct audio_device *manager_get_device(const bdaddr_t *src,
 		return NULL;
 	}
 
-	path = device_get_path(device);
-
-	dev = audio_device_register(device, path, src, dst);
+	dev = audio_device_register(device, src, dst);
 	if (!dev)
 		return NULL;
 
diff --git a/audio/sink.c b/audio/sink.c
index d45185c..2e63579 100644
--- a/audio/sink.c
+++ b/audio/sink.c
@@ -114,12 +114,12 @@ static void sink_set_state(struct audio_device *dev, sink_state_t new_state)
 
 	state_str = state2str(new_state);
 	if (state_str)
-		emit_property_changed(dev->path,
+		emit_property_changed(device_get_path(dev->btd_dev),
 					AUDIO_SINK_INTERFACE, "State",
 					DBUS_TYPE_STRING, &state_str);
 
-	DBG("State changed %s: %s -> %s", dev->path, str_state[old_state],
-		str_state[new_state]);
+	DBG("State changed %s: %s -> %s", device_get_path(dev->btd_dev),
+				str_state[old_state], str_state[new_state]);
 
 	for (l = sink_callbacks; l != NULL; l = l->next) {
 		struct sink_state_callback *cb = l->data;
@@ -532,29 +532,30 @@ static void path_unregister(void *data)
 	struct audio_device *dev = data;
 
 	DBG("Unregistered interface %s on path %s",
-		AUDIO_SINK_INTERFACE, dev->path);
+			AUDIO_SINK_INTERFACE, device_get_path(dev->btd_dev));
 
 	sink_free(dev);
 }
 
 void sink_unregister(struct audio_device *dev)
 {
-	g_dbus_unregister_interface(btd_get_dbus_connection(), dev->path,
-		AUDIO_SINK_INTERFACE);
+	g_dbus_unregister_interface(btd_get_dbus_connection(),
+			device_get_path(dev->btd_dev), AUDIO_SINK_INTERFACE);
 }
 
 struct sink *sink_init(struct audio_device *dev)
 {
 	struct sink *sink;
 
-	if (!g_dbus_register_interface(btd_get_dbus_connection(), dev->path,
+	if (!g_dbus_register_interface(btd_get_dbus_connection(),
+					device_get_path(dev->btd_dev),
 					AUDIO_SINK_INTERFACE,
 					sink_methods, sink_signals, NULL,
 					dev, path_unregister))
 		return NULL;
 
 	DBG("Registered interface %s on path %s",
-		AUDIO_SINK_INTERFACE, dev->path);
+			AUDIO_SINK_INTERFACE, device_get_path(dev->btd_dev));
 
 	if (avdtp_callback_id == 0)
 		avdtp_callback_id = avdtp_add_state_cb(avdtp_state_callback,
diff --git a/audio/source.c b/audio/source.c
index e111954..1d0c74a 100644
--- a/audio/source.c
+++ b/audio/source.c
@@ -108,7 +108,7 @@ static void source_set_state(struct audio_device *dev, source_state_t new_state)
 
 	state_str = state2str(new_state);
 	if (state_str)
-		emit_property_changed(dev->path,
+		emit_property_changed(device_get_path(dev->btd_dev),
 					AUDIO_SOURCE_INTERFACE, "State",
 					DBUS_TYPE_STRING, &state_str);
 
@@ -521,29 +521,30 @@ static void path_unregister(void *data)
 	struct audio_device *dev = data;
 
 	DBG("Unregistered interface %s on path %s",
-		AUDIO_SOURCE_INTERFACE, dev->path);
+			AUDIO_SOURCE_INTERFACE, device_get_path(dev->btd_dev));
 
 	source_free(dev);
 }
 
 void source_unregister(struct audio_device *dev)
 {
-	g_dbus_unregister_interface(btd_get_dbus_connection(), dev->path,
-						AUDIO_SOURCE_INTERFACE);
+	g_dbus_unregister_interface(btd_get_dbus_connection(),
+			device_get_path(dev->btd_dev), AUDIO_SOURCE_INTERFACE);
 }
 
 struct source *source_init(struct audio_device *dev)
 {
 	struct source *source;
 
-	if (!g_dbus_register_interface(btd_get_dbus_connection(), dev->path,
+	if (!g_dbus_register_interface(btd_get_dbus_connection(),
+					device_get_path(dev->btd_dev),
 					AUDIO_SOURCE_INTERFACE,
 					source_methods, source_signals, NULL,
 					dev, path_unregister))
 		return NULL;
 
 	DBG("Registered interface %s on path %s",
-					AUDIO_SOURCE_INTERFACE, dev->path);
+			AUDIO_SOURCE_INTERFACE, device_get_path(dev->btd_dev));
 
 	if (avdtp_callback_id == 0)
 		avdtp_callback_id = avdtp_add_state_cb(avdtp_state_callback,
diff --git a/audio/transport.c b/audio/transport.c
index daafff8..46ab0a7 100644
--- a/audio/transport.c
+++ b/audio/transport.c
@@ -34,6 +34,7 @@
 #include <gdbus.h>
 
 #include "../src/adapter.h"
+#include "../src/device.h"
 #include "../src/dbus-common.h"
 
 #include "log.h"
@@ -1040,6 +1041,7 @@ void transport_get_properties(struct media_transport *transport,
 	const char *uuid;
 	uint8_t codec;
 	const char *state;
+	const char *path;
 
 	dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY,
 			DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
@@ -1047,8 +1049,8 @@ void transport_get_properties(struct media_transport *transport,
 			DBUS_DICT_ENTRY_END_CHAR_AS_STRING, &dict);
 
 	/* Device */
-	dict_append_entry(&dict, "Device", DBUS_TYPE_OBJECT_PATH,
-						&transport->device->path);
+	path = device_get_path(transport->device->btd_dev);
+	dict_append_entry(&dict, "Device", DBUS_TYPE_OBJECT_PATH, &path);
 
 	uuid = media_endpoint_get_uuid(transport->endpoint);
 	dict_append_entry(&dict, "UUID", DBUS_TYPE_STRING, &uuid);
@@ -1265,7 +1267,8 @@ struct media_transport *media_transport_create(struct media_endpoint *endpoint,
 	transport->configuration = g_new(uint8_t, size);
 	memcpy(transport->configuration, configuration, size);
 	transport->size = size;
-	transport->path = g_strdup_printf("%s/fd%d", device->path, fd++);
+	transport->path = g_strdup_printf("%s/fd%d",
+				device_get_path(device->btd_dev), fd++);
 	transport->fd = -1;
 
 	uuid = media_endpoint_get_uuid(endpoint);
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 2/8] mgmt: Make mgmt_update_powered return void
From: Szymon Janc @ 2012-10-16 10:12 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1350382379-30489-1-git-send-email-szymon.janc@tieto.com>

mgmt_update_powered always returned 0 and this was never checked by
caller.

---
 src/mgmt.c |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/src/mgmt.c b/src/mgmt.c
index 2a66be2..8caabe8 100644
--- a/src/mgmt.c
+++ b/src/mgmt.c
@@ -352,7 +352,7 @@ static void update_settings(struct btd_adapter *adapter, uint32_t settings)
 		mgmt_set_low_energy(index, TRUE);
 }
 
-static int mgmt_update_powered(struct btd_adapter *adapter,
+static void mgmt_update_powered(struct btd_adapter *adapter,
 						struct controller_info *info,
 						uint32_t settings)
 {
@@ -363,14 +363,12 @@ static int mgmt_update_powered(struct btd_adapter *adapter,
 		info->pending_uuid = FALSE;
 		info->pending_class = FALSE;
 		info->pending_cod_change = FALSE;
-		return 0;
+		return;
 	}
 
 	btd_adapter_start(adapter);
 
 	update_settings(adapter, settings);
-
-	return 0;
 }
 
 static int mode_changed(uint32_t s1, uint32_t s2)
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 1/8] audio: Move btd_device unref after last use in device_free
From: Szymon Janc @ 2012-10-16 10:12 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

btd_device should not be used after local reference was dropped.

---
 audio/device.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/audio/device.c b/audio/device.c
index 175e3c0..be63031 100644
--- a/audio/device.c
+++ b/audio/device.c
@@ -98,8 +98,6 @@ static void device_free(struct audio_device *dev)
 {
 	struct dev_priv *priv = dev->priv;
 
-	btd_device_unref(dev->btd_dev);
-
 	if (priv) {
 		if (priv->control_timer)
 			g_source_remove(priv->control_timer);
@@ -117,6 +115,8 @@ static void device_free(struct audio_device *dev)
 		g_free(priv);
 	}
 
+	btd_device_unref(dev->btd_dev);
+
 	g_free(dev->path);
 	g_free(dev);
 }
-- 
1.7.9.5


^ permalink raw reply related

* Re: Is it incorrect in checking of "NO_RESET" bit in hci_core.c:hci_dev_do_close()?
From: chen kris @ 2012-10-16  9:49 UTC (permalink / raw)
  To: Anderson Lizardo; +Cc: Marcel Holtmann, linux-bluetooth
In-Reply-To: <CAJdJm_MfoNrSt62tiGr9+hae8zEVucex9rCY8zk+e7mNGkzHQg@mail.gmail.com>

Hi, Marcel

Thanks!

Hi, Anderson

Sorry, the kernel source I used is compat-wireless-3.5-rc3-2-snpc.

I checked newer kernel code, it do have the new flag name.

now I know the meaning and logic of this flag, thanks!

Kris


On Tue, Oct 16, 2012 at 2:09 AM, Anderson Lizardo
<anderson.lizardo@openbossa.org> wrote:
> Hi,
>
> On Mon, Oct 15, 2012 at 3:02 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
>> Hi Chen,
>>
>>> static int hci_dev_do_close(struct hci_dev *hdev)
>>> {
>>>     ...
>>>
>>>     /* Reset device */
>>>     skb_queue_purge(&hdev->cmd_q);
>>>     atomic_set(&hdev->cmd_cnt, 1);
>>>     if (!test_bit(HCI_RAW, &hdev->flags) &&
>>>                 test_bit(HCI_QUIRK_NO_RESET, &hdev->quirks)) {
>>>         set_bit(HCI_INIT, &hdev->flags);
>>>         __hci_request(hdev, hci_reset_req, 0,
>>>                     msecs_to_jiffies(250));
>>>         clear_bit(HCI_INIT, &hdev->flags);
>>>     }
>>>     ...
>>> }
>>>
>>> I guess the condition should be as following:
>>>
>>>     if (!test_bit(HCI_RAW, &hdev->flags) &&
>>>                 *! *test_bit(HCI_QUIRK_NO_RESET, &hdev->quirks))
>>>
>>> it means the code should sent HCI_RESET, only if both of the following
>>> conditions are met:
>>> 1. HCI_RAW is NOT set;
>>> 2. HCI_QUIRK_NO_RESET is *NOT* set.
>>
>> the name is confusing, when closing the device and NO_RESET is set, then
>> it needs to send the HCI Reset.
>
> I was almost sure this flag has been renamed to something like
> "HCI_QUIRK_RESET_ON_CLOSE" on recent kernels?
>
> Regards,
> --
> Anderson Lizardo
> Instituto Nokia de Tecnologia - INdT
> Manaus - Brazil

^ permalink raw reply

* REMINDER: Linux wireless networking summit, Barcelona 8-9 November
From: John W. Linville @ 2012-10-15 19:36 UTC (permalink / raw)
  To: linux-wireless, linux-bluetooth

Just a reminder...there will be a Linux wireless networking mini-summit
event coming soon.  It will be on 8-9 November in Barcelona, just
after LinuxCon Europe.  If you haven't already made plans, you had
better get moving!

Attendees of the wireless mini-summit must also register for LinuxCon
Europe.  More information is available here:

	http://wireless.kernel.org/en/developers/Summits/Barcelona-2012

Please notice that there is a wiki there with schedules for discussion
topics.  If you have suggestions or want other changes, please make
them and/or let us know here.

I hope to see you in Barcelona!

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

^ permalink raw reply

* Re: [PATCH 19/19] Bluetooth: Start channel move when socket option is changed
From: Marcel Holtmann @ 2012-10-15 18:47 UTC (permalink / raw)
  To: Mat Martineau; +Cc: linux-bluetooth, gustavo, sunnyk
In-Reply-To: <1350315248-7690-20-git-send-email-mathewm@codeaurora.org>

Hi Mat,

> Channel moves are triggered by changes to the BT_CHANNEL_POLICY
> sockopt when an ERTM or streaming-mode channel is connected.
> 
> Moves are only started if enable_hs is true.
> 
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
>  include/net/bluetooth/l2cap.h |  1 +
>  net/bluetooth/l2cap_core.c    | 20 ++++++++++++++++++++
>  net/bluetooth/l2cap_sock.c    |  5 +++++
>  3 files changed, 26 insertions(+)

Acked-by: Marcel Holtmann <marcel@holtmann.org>

Regards

Marcel



^ permalink raw reply

* Re: [PATCH 18/19] Bluetooth: Do not retransmit data during a channel move
From: Marcel Holtmann @ 2012-10-15 18:46 UTC (permalink / raw)
  To: Mat Martineau; +Cc: linux-bluetooth, gustavo, sunnyk
In-Reply-To: <1350315248-7690-19-git-send-email-mathewm@codeaurora.org>

Hi Mat,

> Do not retransmit previously-sent data when a "receiver ready" s-frame
> with the "final" flag is received during a move.
> 
> The ERTM state machines will resynchronize at the end of a channel
> move, and the state machine needs to avoid state changes during a
> move.
> 
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
>  net/bluetooth/l2cap_core.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 57b1b02..d7819c1 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -5615,8 +5615,9 @@ static int l2cap_rx_state_recv(struct l2cap_chan *chan,
>  		if (control->final) {
>  			clear_bit(CONN_REMOTE_BUSY, &chan->conn_state);
>  
> -			if (!test_and_clear_bit(CONN_REJ_ACT,
> -						&chan->conn_state)) {
> +			if (!test_and_clear_bit(CONN_REJ_ACT, &chan->conn_state)
> +			    && (chan->move_state == L2CAP_MOVE_STABLE ||

can we just break the line width limit and put && at the end.

> +				chan->move_state == L2CAP_MOVE_WAIT_PREPARE)) {

Otherwise this becomes hard to read.

>  				control->final = 0;
>  				l2cap_retransmit_all(chan, control);
>  			}

Acked-by: Marcel Holtmann <marcel@holtmann.org>

Regards

Marcel



^ permalink raw reply

* Re: [PATCH 15/19] Bluetooth: Configure appropriate timeouts for AMP controllers
From: Marcel Holtmann @ 2012-10-15 18:44 UTC (permalink / raw)
  To: Mat Martineau; +Cc: linux-bluetooth, gustavo, sunnyk
In-Reply-To: <1350315248-7690-16-git-send-email-mathewm@codeaurora.org>

Hi Mat,

> The L2CAP spec recommends specific retransmit and monitor timeouts for
> ERTM channels that are on AMP controllers.  These timeouts are
> calculated from the AMP controller's best effort flush timeout.
> 
> BR/EDR controllers use the default retransmit and monitor timeouts.
> 
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
>  net/bluetooth/l2cap_core.c | 47 ++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 41 insertions(+), 6 deletions(-)
> 
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index cf1dc4e..dbcd1e3 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -2965,6 +2965,44 @@ static inline bool __l2cap_efs_supported(struct l2cap_chan *chan)
>  	return enable_hs && chan->conn->feat_mask & L2CAP_FEAT_EXT_FLOW;
>  }
>  
> +static void __l2cap_set_ertm_timeouts(struct l2cap_chan *chan,
> +				      struct l2cap_conf_rfc *rfc)
> +{
> +	if (chan->local_amp_id && chan->hs_hcon) {
> +		u64 ertm_to = chan->hs_hcon->hdev->amp_be_flush_to;
> +
> +		/* Class 1 devices have must have ERTM timeouts
> +		 * exceeding the Link Supervision Timeout.  The
> +		 * default Link Supervision Timeout for AMP
> +		 * controllers is 10 seconds.
> +		 *
> +		 * Class 1 devices use 0xffffffff for their
> +		 * best-effort flush timeout, so the clamping logic
> +		 * will result in a timeout that meets the above
> +		 * requirement.  ERTM timeouts are 16-bit values, so
> +		 * the maximum timeout is 65.535 seconds.
> +		 */
> +
> +		/* Convert timeout to milliseconds and round */
> +		ertm_to = div_u64(ertm_to + 999, 1000);

don't we have a proper ROUND function for this?

> +
> +		/* This is the recommended formula for class 2 devices
> +		 * that start ERTM timers when packets are sent to the
> +		 * controller.
> +		 */
> +		ertm_to = 3 * ertm_to + 500;
> +
> +		if (ertm_to > 0xffff)
> +			ertm_to = 0xffff;
> +
> +		rfc->retrans_timeout = cpu_to_le16((u16) ertm_to);
> +		rfc->monitor_timeout = rfc->retrans_timeout;
> +	} else {
> +		rfc->retrans_timeout = __constant_cpu_to_le16(L2CAP_DEFAULT_RETRANS_TO);
> +		rfc->monitor_timeout = __constant_cpu_to_le16(L2CAP_DEFAULT_MONITOR_TO);
> +	}
> +}
> +
>  static inline void l2cap_txwin_setup(struct l2cap_chan *chan)
>  {
>  	if (chan->tx_win > L2CAP_DEFAULT_TX_WINDOW &&
> @@ -3031,8 +3069,8 @@ done:
>  	case L2CAP_MODE_ERTM:
>  		rfc.mode            = L2CAP_MODE_ERTM;
>  		rfc.max_transmit    = chan->max_tx;
> -		rfc.retrans_timeout = 0;
> -		rfc.monitor_timeout = 0;
> +
> +		__l2cap_set_ertm_timeouts(chan, &rfc);
>  
>  		size = min_t(u16, L2CAP_DEFAULT_MAX_PDU_SIZE, chan->conn->mtu -
>  			     L2CAP_EXT_HDR_SIZE - L2CAP_SDULEN_SIZE -
> @@ -3260,10 +3298,7 @@ done:
>  			rfc.max_pdu_size = cpu_to_le16(size);
>  			chan->remote_mps = size;
>  
> -			rfc.retrans_timeout =
> -				__constant_cpu_to_le16(L2CAP_DEFAULT_RETRANS_TO);
> -			rfc.monitor_timeout =
> -				__constant_cpu_to_le16(L2CAP_DEFAULT_MONITOR_TO);
> +			__l2cap_set_ertm_timeouts(chan, &rfc);
>  
>  			set_bit(CONF_MODE_DONE, &chan->conf_state);
>  

Otherwise, looks good.

Acked-by: Marcel Holtmann <marcel@holtmann.org>

Regards

Marcel



^ permalink raw reply

* Re: [PATCH 14/19] Bluetooth: Do not send data during channel move
From: Marcel Holtmann @ 2012-10-15 18:42 UTC (permalink / raw)
  To: Mat Martineau; +Cc: linux-bluetooth, gustavo, sunnyk
In-Reply-To: <1350315248-7690-15-git-send-email-mathewm@codeaurora.org>

Hi Mat,

> Outgoing ERTM data is queued during a channel move.  The ERTM state
> machine is partially reset at the start of a move, and must be
> resynchronized with the remote state machine at the end of the move.
> Data is not sent so that there are no state transitions between the
> partial reset and the resync.
> 
> Streaming mode frames are dropped during a move.
> 
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
>  net/bluetooth/l2cap_core.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)

Acked-by: Marcel Holtmann <marcel@holtmann.org>

Regards

Marcel



^ permalink raw reply

* Re: [PATCH 13/19] Bluetooth: Flag ACL frames as complete for AMP controllers
From: Marcel Holtmann @ 2012-10-15 18:41 UTC (permalink / raw)
  To: Mat Martineau; +Cc: linux-bluetooth, gustavo, sunnyk
In-Reply-To: <1350315248-7690-14-git-send-email-mathewm@codeaurora.org>

Hi Mat,

> AMP controllers expect to transmit only "complete" ACL frames.  These
> frames have both the "start" and "cont" bits set.  AMP does not allow
> fragmented ACLs.
> 
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
>  net/bluetooth/l2cap_core.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)

Acked-by: Marcel Holtmann <marcel@holtmann.org>

Regards

Marcel



^ permalink raw reply

* Re: [PATCH 12/19] Bluetooth: Handle physical link completion
From: Marcel Holtmann @ 2012-10-15 18:40 UTC (permalink / raw)
  To: Mat Martineau; +Cc: linux-bluetooth, gustavo, sunnyk
In-Reply-To: <1350315248-7690-13-git-send-email-mathewm@codeaurora.org>

Hi Mat,

> Several different actions may be taken when an AMP physical link
> becomes available.  A channel being created on an AMP controller must
> continue the connection process.  A channel being moved needs to
> either send a move request or a move response.  A failed physical link
> will revert to using a BR/EDR controller if possible.
> 
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
>  net/bluetooth/l2cap_core.c | 166 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 166 insertions(+)

Acked-by: Marcel Holtmann <marcel@holtmann.org>

Regards

Marcel



^ permalink raw reply


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