Linux bluetooth development
 help / color / mirror / Atom feed
* [PATCH BlueZ 8/8 v2] player: Add Device property
From: Luiz Augusto von Dentz @ 2013-01-04 12:39 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1357303197-19673-1-git-send-email-luiz.dentz@gmail.com>

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

Device property indicates which device object the player belongs to.
---
 doc/media-api.txt       |  4 ++++
 profiles/audio/player.c | 14 ++++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/doc/media-api.txt b/doc/media-api.txt
index 12a2d7a..e2cafd1 100644
--- a/doc/media-api.txt
+++ b/doc/media-api.txt
@@ -288,6 +288,10 @@ Properties	string Equalizer [readwrite]
 
 					Track duration in milliseconds
 
+		object Device [readonly]
+
+			Device object path.
+
 MediaEndpoint1 hierarchy
 ========================
 
diff --git a/profiles/audio/player.c b/profiles/audio/player.c
index 81bb7fe..bac5649 100644
--- a/profiles/audio/player.c
+++ b/profiles/audio/player.c
@@ -56,6 +56,7 @@ struct pending_req {
 };
 
 struct media_player {
+	char			*device;	/* Device path */
 	char			*path;		/* Player object path */
 	GHashTable		*settings;	/* Player settings */
 	GHashTable		*track;		/* Player current track */
@@ -239,6 +240,16 @@ static gboolean get_track(const GDBusPropertyTable *property,
 	return TRUE;
 }
 
+static gboolean get_device(const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *data)
+{
+	struct media_player *mp = data;
+
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_STRING, &mp->device);
+
+	return TRUE;
+}
+
 static DBusMessage *media_player_play(DBusConnection *conn, DBusMessage *msg,
 								void *data)
 {
@@ -417,6 +428,7 @@ static const GDBusPropertyTable media_player_properties[] = {
 	{ "Shuffle", "s", get_setting, set_setting, setting_exists },
 	{ "Scan", "s", get_setting, set_setting, setting_exists },
 	{ "Track", "a{sv}", get_track, NULL, NULL },
+	{ "Device", "s", get_device, NULL, NULL },
 	{ }
 };
 
@@ -442,6 +454,7 @@ void media_player_destroy(struct media_player *mp)
 	g_free(mp->cb);
 	g_free(mp->status);
 	g_free(mp->path);
+	g_free(mp->device);
 	g_free(mp);
 }
 
@@ -450,6 +463,7 @@ struct media_player *media_player_controller_create(const char *path)
 	struct media_player *mp;
 
 	mp = g_new0(struct media_player, 1);
+	mp->device = g_strdup(path);
 	mp->path = g_strdup_printf("%s/player1", path);
 	mp->settings = g_hash_table_new_full(g_str_hash, g_str_equal,
 							g_free, g_free);
-- 
1.8.0.1


^ permalink raw reply related

* [PATCH BlueZ 7/8 v2] player: Remove experimental flag from MediaPlayer1
From: Luiz Augusto von Dentz @ 2013-01-04 12:39 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1357303197-19673-1-git-send-email-luiz.dentz@gmail.com>

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

---
 doc/media-api.txt       |  2 +-
 profiles/audio/player.c | 21 +++++++--------------
 2 files changed, 8 insertions(+), 15 deletions(-)

diff --git a/doc/media-api.txt b/doc/media-api.txt
index 5a6b68c..12a2d7a 100644
--- a/doc/media-api.txt
+++ b/doc/media-api.txt
@@ -179,7 +179,7 @@ MediaPlayer1 hierarchy
 ======================
 
 Service		org.bluez (Controller role)
-Interface	org.bluez.MediaPlayer1 [Experimental]
+Interface	org.bluez.MediaPlayer1
 Object path	[variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/playerX
 
 Methods		void Play()
diff --git a/profiles/audio/player.c b/profiles/audio/player.c
index 0b69912..81bb7fe 100644
--- a/profiles/audio/player.c
+++ b/profiles/audio/player.c
@@ -410,20 +410,13 @@ static const GDBusSignalTable media_player_signals[] = {
 };
 
 static const GDBusPropertyTable media_player_properties[] = {
-	{ "Position", "u", get_position, NULL, NULL,
-					G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
-	{ "Status", "s", get_status, NULL, status_exists,
-					G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
-	{ "Equalizer", "s", get_setting, set_setting, setting_exists,
-					G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
-	{ "Repeat", "s", get_setting, set_setting, setting_exists,
-					G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
-	{ "Shuffle", "s", get_setting, set_setting, setting_exists,
-					G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
-	{ "Scan", "s", get_setting, set_setting, setting_exists,
-					G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
-	{ "Track", "a{sv}", get_track, NULL, NULL,
-					G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
+	{ "Position", "u", get_position, NULL, NULL },
+	{ "Status", "s", get_status, NULL, status_exists },
+	{ "Equalizer", "s", get_setting, set_setting, setting_exists },
+	{ "Repeat", "s", get_setting, set_setting, setting_exists },
+	{ "Shuffle", "s", get_setting, set_setting, setting_exists },
+	{ "Scan", "s", get_setting, set_setting, setting_exists },
+	{ "Track", "a{sv}", get_track, NULL, NULL },
 	{ }
 };
 
-- 
1.8.0.1


^ permalink raw reply related

* [PATCH BlueZ 6/8 v2] control: Mark all members of MediaControl1 as deprecated
From: Luiz Augusto von Dentz @ 2013-01-04 12:39 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1357303197-19673-1-git-send-email-luiz.dentz@gmail.com>

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

---
 profiles/audio/control.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/profiles/audio/control.c b/profiles/audio/control.c
index 642fdd5..70faab8 100644
--- a/profiles/audio/control.c
+++ b/profiles/audio/control.c
@@ -222,20 +222,23 @@ static gboolean control_property_get_connected(
 }
 
 static const GDBusMethodTable control_methods[] = {
-	{ GDBUS_METHOD("Play", NULL, NULL, control_play) },
-	{ GDBUS_METHOD("Pause", NULL, NULL, control_pause) },
-	{ GDBUS_METHOD("Stop", NULL, NULL, control_stop) },
-	{ GDBUS_METHOD("Next", NULL, NULL, control_next) },
-	{ GDBUS_METHOD("Previous", NULL, NULL, control_previous) },
-	{ GDBUS_METHOD("VolumeUp", NULL, NULL, control_volume_up) },
-	{ GDBUS_METHOD("VolumeDown", NULL, NULL, control_volume_down) },
-	{ GDBUS_METHOD("FastForward", NULL, NULL, control_fast_forward) },
-	{ GDBUS_METHOD("Rewind", NULL, NULL, control_rewind) },
+	{ GDBUS_DEPRECATED_METHOD("Play", NULL, NULL, control_play) },
+	{ GDBUS_DEPRECATED_METHOD("Pause", NULL, NULL, control_pause) },
+	{ GDBUS_DEPRECATED_METHOD("Stop", NULL, NULL, control_stop) },
+	{ GDBUS_DEPRECATED_METHOD("Next", NULL, NULL, control_next) },
+	{ GDBUS_DEPRECATED_METHOD("Previous", NULL, NULL, control_previous) },
+	{ GDBUS_DEPRECATED_METHOD("VolumeUp", NULL, NULL, control_volume_up) },
+	{ GDBUS_DEPRECATED_METHOD("VolumeDown", NULL, NULL,
+							control_volume_down) },
+	{ GDBUS_DEPRECATED_METHOD("FastForward", NULL, NULL,
+							control_fast_forward) },
+	{ GDBUS_DEPRECATED_METHOD("Rewind", NULL, NULL, control_rewind) },
 	{ }
 };
 
 static const GDBusPropertyTable control_properties[] = {
-	{ "Connected", "b", control_property_get_connected },
+	{ "Connected", "b", control_property_get_connected, NULL, NULL,
+					G_DBUS_PROPERTY_FLAG_DEPRECATED },
 	{ }
 };
 
-- 
1.8.0.1


^ permalink raw reply related

* [PATCH BlueZ 5/8 v2] AVRCP: Always create a controller player even for version 1.0
From: Luiz Augusto von Dentz @ 2013-01-04 12:39 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1357303197-19673-1-git-send-email-luiz.dentz@gmail.com>

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

Since the buttons controls are now part of the MediaPlayer1 it can be used
even with AVRCP version 1.0.
---
 profiles/audio/avrcp.c | 33 +++++++++++++++++----------------
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
index 99741e8..74ef3ea 100644
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -1952,6 +1952,9 @@ static bool ct_set_setting(struct media_player *mp, const char *key,
 	if (session == NULL)
 		return false;
 
+	if (session->version < 0x0103)
+		return false;
+
 	attr = attr_to_val(key);
 	if (attr < 0)
 		return false;
@@ -2063,24 +2066,15 @@ static gboolean avrcp_get_capabilities_resp(struct avctp *conn,
 					void *user_data)
 {
 	struct avrcp *session = user_data;
-	struct avrcp_player *player = session->player;
-	struct media_player *mp;
 	struct avrcp_header *pdu = (void *) operands;
 	uint16_t events = 0;
 	uint8_t count;
-	const char *path;
 
 	if (pdu->params[0] != CAP_EVENTS_SUPPORTED)
 		return FALSE;
 
 	count = pdu->params[1];
 
-	path = device_get_path(session->dev->btd_dev);
-
-	mp = media_player_controller_create(path);
-	if (mp == NULL)
-		return FALSE;
-
 	for (; count > 0; count--) {
 		uint8_t event = pdu->params[1 + count];
 
@@ -2095,10 +2089,6 @@ static gboolean avrcp_get_capabilities_resp(struct avctp *conn,
 		}
 	}
 
-	media_player_set_callbacks(mp, &ct_cbs, player);
-	player->user_data = mp;
-	player->destroy = (GDestroyNotify) media_player_destroy;
-
 	if (!(events & (1 << AVRCP_EVENT_SETTINGS_CHANGED)))
 		avrcp_list_player_attributes(session);
 
@@ -2185,6 +2175,8 @@ static void session_tg_init(struct avrcp *session)
 static void session_ct_init(struct avrcp *session)
 {
 	struct avrcp_player *player;
+	struct media_player *mp;
+	const char *path;
 
 	session->control_handlers = ct_control_handlers;
 
@@ -2195,13 +2187,22 @@ static void session_ct_init(struct avrcp *session)
 							handle_vendordep_pdu,
 							session);
 
-	if (session->version < 0x0103)
-		return;
-
 	player = g_new0(struct avrcp_player, 1);
 	player->sessions = g_slist_prepend(player->sessions, session);
 	session->player = player;
 
+	path = device_get_path(session->dev->btd_dev);
+
+	mp = media_player_controller_create(path);
+	if (mp != NULL) {
+		media_player_set_callbacks(mp, &ct_cbs, player);
+		player->user_data = mp;
+		player->destroy = (GDestroyNotify) media_player_destroy;
+	}
+
+	if (session->version < 0x0103)
+		return;
+
 	avrcp_get_capabilities(session);
 }
 
-- 
1.8.0.1


^ permalink raw reply related

* [PATCH BlueZ 4/8 v2] player: Fix documentation to use TrackNumber in track metadata
From: Luiz Augusto von Dentz @ 2013-01-04 12:39 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1357303197-19673-1-git-send-email-luiz.dentz@gmail.com>

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

Using Track key inside a Track property would be pointless, despite the
documentation and code where also inconsistent.
---
 doc/media-api.txt       | 2 +-
 profiles/audio/avrcp.c  | 2 +-
 profiles/audio/player.c | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/doc/media-api.txt b/doc/media-api.txt
index eb1f74f..5a6b68c 100644
--- a/doc/media-api.txt
+++ b/doc/media-api.txt
@@ -280,7 +280,7 @@ Properties	string Equalizer [readwrite]
 
 					Number of tracks in total
 
-				uint32 Number:
+				uint32 TrackNumber:
 
 					Track number
 
diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
index 7b7c2ef..99741e8 100644
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -649,7 +649,7 @@ static const char *metadata_to_str(uint32_t id)
 	case AVRCP_MEDIA_ATTRIBUTE_GENRE:
 		return "Genre";
 	case AVRCP_MEDIA_ATTRIBUTE_TRACK:
-		return "Track";
+		return "TrackNumber";
 	case AVRCP_MEDIA_ATTRIBUTE_N_TRACKS:
 		return "NumberOfTracks";
 	case AVRCP_MEDIA_ATTRIBUTE_DURATION:
diff --git a/profiles/audio/player.c b/profiles/audio/player.c
index 82c5bfb..0b69912 100644
--- a/profiles/audio/player.c
+++ b/profiles/audio/player.c
@@ -72,7 +72,7 @@ static void append_metadata(void *key, void *value, void *user_data)
 	DBusMessageIter *dict = user_data;
 
 	if (strcasecmp((char *) key, "Duration") == 0 ||
-			strcasecmp((char *) key, "Track") == 0 ||
+			strcasecmp((char *) key, "TrackNumber") == 0 ||
 			strcasecmp((char *) key, "NumberOfTracks") == 0)  {
 		uint32_t num = atoi(value);
 		dict_append_entry(dict, key, DBUS_TYPE_UINT32, &num);
-- 
1.8.0.1


^ permalink raw reply related

* [PATCH BlueZ 3/8 v2] player: Add support for button controls
From: Luiz Augusto von Dentz @ 2013-01-04 12:39 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1357303197-19673-1-git-send-email-luiz.dentz@gmail.com>

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

This adds support for buttons controls in MediaPlayer1
---
 profiles/audio/avrcp.c  |  91 ++++++++++++++++++++++++++-
 profiles/audio/player.c | 162 ++++++++++++++++++++++++++++++++++++++++++++++++
 profiles/audio/player.h |   9 +++
 3 files changed, 261 insertions(+), 1 deletion(-)

diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
index ce070cd..7b7c2ef 100644
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -1965,8 +1965,96 @@ static bool ct_set_setting(struct media_player *mp, const char *key,
 	return true;
 }
 
+static int ct_press(struct avrcp_player *player, uint8_t op)
+{
+	int err;
+	struct avrcp *session;
+
+	session = player->sessions->data;
+	if (session == NULL)
+		return -ENOTCONN;
+
+	err = avctp_send_passthrough(session->conn, op);
+	if (err < 0)
+		return err;
+
+	return 0;
+}
+
+static int ct_play(struct media_player *mp, void *user_data)
+{
+	struct avrcp_player *player = user_data;
+
+	return ct_press(player, AVC_PLAY);
+}
+
+static int ct_pause(struct media_player *mp, void *user_data)
+{
+	struct avrcp_player *player = user_data;
+
+	return ct_press(player, AVC_PAUSE);
+}
+
+static int ct_stop(struct media_player *mp, void *user_data)
+{
+	struct avrcp_player *player = user_data;
+
+	return ct_press(player, AVC_STOP);
+}
+
+static int ct_next(struct media_player *mp, void *user_data)
+{
+	struct avrcp_player *player = user_data;
+
+	return ct_press(player, AVC_FORWARD);
+}
+
+static int ct_previous(struct media_player *mp, void *user_data)
+{
+	struct avrcp_player *player = user_data;
+
+	return ct_press(player, AVC_BACKWARD);
+}
+
+static int ct_fast_forward(struct media_player *mp, void *user_data)
+{
+	struct avrcp_player *player = user_data;
+
+	return ct_press(player, AVC_FAST_FORWARD);
+}
+
+static int ct_rewind(struct media_player *mp, void *user_data)
+{
+	struct avrcp_player *player = user_data;
+
+	return ct_press(player, AVC_REWIND);
+}
+
+static int ct_volume_up(struct media_player *mp, void *user_data)
+{
+	struct avrcp_player *player = user_data;
+
+	return ct_press(player, AVC_VOLUME_UP);
+}
+
+static int ct_volume_down(struct media_player *mp, void *user_data)
+{
+	struct avrcp_player *player = user_data;
+
+	return ct_press(player, AVC_VOLUME_DOWN);
+}
+
 static const struct media_player_callback ct_cbs = {
-	.set_setting = ct_set_setting,
+	.set_setting	= ct_set_setting,
+	.play		= ct_play,
+	.pause		= ct_pause,
+	.stop		= ct_stop,
+	.next		= ct_next,
+	.previous	= ct_previous,
+	.fast_forward	= ct_fast_forward,
+	.rewind		= ct_rewind,
+	.volume_up	= ct_volume_up,
+	.volume_down	= ct_volume_down,
 };
 
 static gboolean avrcp_get_capabilities_resp(struct avctp *conn,
@@ -1988,6 +2076,7 @@ static gboolean avrcp_get_capabilities_resp(struct avctp *conn,
 	count = pdu->params[1];
 
 	path = device_get_path(session->dev->btd_dev);
+
 	mp = media_player_controller_create(path);
 	if (mp == NULL)
 		return FALSE;
diff --git a/profiles/audio/player.c b/profiles/audio/player.c
index c3aaef0..82c5bfb 100644
--- a/profiles/audio/player.c
+++ b/profiles/audio/player.c
@@ -239,7 +239,169 @@ static gboolean get_track(const GDBusPropertyTable *property,
 	return TRUE;
 }
 
+static DBusMessage *media_player_play(DBusConnection *conn, DBusMessage *msg,
+								void *data)
+{
+	struct media_player *mp = data;
+	struct player_callback *cb = mp->cb;
+	int err;
+
+	if (cb->cbs->play == NULL)
+		return btd_error_not_supported(msg);
+
+	err = cb->cbs->play(mp, cb->user_data);
+	if (err < 0)
+		return btd_error_failed(msg, strerror(-err));
+
+	return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
+}
+
+static DBusMessage *media_player_pause(DBusConnection *conn, DBusMessage *msg,
+								void *data)
+{
+	struct media_player *mp = data;
+	struct player_callback *cb = mp->cb;
+	int err;
+
+	if (cb->cbs->pause == NULL)
+		return btd_error_not_supported(msg);
+
+	err = cb->cbs->pause(mp, cb->user_data);
+	if (err < 0)
+		return btd_error_failed(msg, strerror(-err));
+
+	return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
+}
+
+static DBusMessage *media_player_stop(DBusConnection *conn, DBusMessage *msg,
+								void *data)
+{
+	struct media_player *mp = data;
+	struct player_callback *cb = mp->cb;
+	int err;
+
+	if (cb->cbs->stop == NULL)
+		return btd_error_not_supported(msg);
+
+	err = cb->cbs->stop(mp, cb->user_data);
+	if (err < 0)
+		return btd_error_failed(msg, strerror(-err));
+
+	return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
+}
+
+static DBusMessage *media_player_next(DBusConnection *conn, DBusMessage *msg,
+								void *data)
+{
+	struct media_player *mp = data;
+	struct player_callback *cb = mp->cb;
+	int err;
+
+	if (cb->cbs->next == NULL)
+		return btd_error_not_supported(msg);
+
+	err = cb->cbs->next(mp, cb->user_data);
+	if (err < 0)
+		return btd_error_failed(msg, strerror(-err));
+
+	return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
+}
+
+static DBusMessage *media_player_previous(DBusConnection *conn,
+						DBusMessage *msg, void *data)
+{
+	struct media_player *mp = data;
+	struct player_callback *cb = mp->cb;
+	int err;
+
+	if (cb->cbs->previous == NULL)
+		return btd_error_not_supported(msg);
+
+	err = cb->cbs->previous(mp, cb->user_data);
+	if (err < 0)
+		return btd_error_failed(msg, strerror(-err));
+
+	return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
+}
+
+static DBusMessage *media_player_fast_forward(DBusConnection *conn,
+						DBusMessage *msg, void *data)
+{
+	struct media_player *mp = data;
+	struct player_callback *cb = mp->cb;
+	int err;
+
+	if (cb->cbs->fast_forward == NULL)
+		return btd_error_not_supported(msg);
+
+	err = cb->cbs->fast_forward(mp, cb->user_data);
+	if (err < 0)
+		return btd_error_failed(msg, strerror(-err));
+
+	return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
+}
+
+static DBusMessage *media_player_rewind(DBusConnection *conn, DBusMessage *msg,
+								void *data)
+{
+	struct media_player *mp = data;
+	struct player_callback *cb = mp->cb;
+	int err;
+
+	if (cb->cbs->rewind == NULL)
+		return btd_error_not_supported(msg);
+
+	err = cb->cbs->rewind(mp, cb->user_data);
+	if (err < 0)
+		return btd_error_failed(msg, strerror(-err));
+
+	return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
+}
+
+static DBusMessage *media_player_volume_up(DBusConnection *conn,
+						DBusMessage *msg, void *data)
+{
+	struct media_player *mp = data;
+	struct player_callback *cb = mp->cb;
+	int err;
+
+	if (cb->cbs->volume_up == NULL)
+		return btd_error_not_supported(msg);
+
+	err = cb->cbs->volume_up(mp, cb->user_data);
+	if (err < 0)
+		return btd_error_failed(msg, strerror(-err));
+
+	return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
+}
+
+static DBusMessage *media_player_volume_down(DBusConnection *conn,
+						DBusMessage *msg, void *data)
+{
+	struct media_player *mp = data;
+	struct player_callback *cb = mp->cb;
+	int err;
+
+	if (cb->cbs->volume_down == NULL)
+		return btd_error_not_supported(msg);
+
+	err = cb->cbs->volume_down(mp, cb->user_data);
+	if (err < 0)
+		return btd_error_failed(msg, strerror(-err));
+
+	return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
+}
+
 static const GDBusMethodTable media_player_methods[] = {
+	{ GDBUS_METHOD("Play", NULL, NULL, media_player_play) },
+	{ GDBUS_METHOD("Pause", NULL, NULL, media_player_pause) },
+	{ GDBUS_METHOD("Stop", NULL, NULL, media_player_stop) },
+	{ GDBUS_METHOD("Next", NULL, NULL, media_player_next) },
+	{ GDBUS_METHOD("Previous", NULL, NULL, media_player_previous) },
+	{ GDBUS_METHOD("FastForward", NULL, NULL, media_player_fast_forward) },
+	{ GDBUS_METHOD("Rewind", NULL, NULL, media_player_rewind) },
+	{ GDBUS_METHOD("VolumeUp", NULL, NULL, media_player_volume_up) },
+	{ GDBUS_METHOD("VolumeDown", NULL, NULL, media_player_volume_down) },
 	{ }
 };
 
diff --git a/profiles/audio/player.h b/profiles/audio/player.h
index 87dfb66..4b92b30 100644
--- a/profiles/audio/player.h
+++ b/profiles/audio/player.h
@@ -28,6 +28,15 @@ struct media_player;
 struct media_player_callback {
 	bool (*set_setting) (struct media_player *mp, const char *key,
 				const char *value, void *user_data);
+	int (*play) (struct media_player *mp, void *user_data);
+	int (*pause) (struct media_player *mp, void *user_data);
+	int (*stop) (struct media_player *mp, void *user_data);
+	int (*next) (struct media_player *mp, void *user_data);
+	int (*previous) (struct media_player *mp, void *user_data);
+	int (*fast_forward) (struct media_player *mp, void *user_data);
+	int (*rewind) (struct media_player *mp, void *user_data);
+	int (*volume_up) (struct media_player *mp, void *user_data);
+	int (*volume_down) (struct media_player *mp, void *user_data);
 };
 
 struct media_player *media_player_controller_create(const char *path);
-- 
1.8.0.1


^ permalink raw reply related

* [PATCH BlueZ 2/8 v2] player: Remove GetTrack and TrackChanged
From: Luiz Augusto von Dentz @ 2013-01-04 12:39 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1357303197-19673-1-git-send-email-luiz.dentz@gmail.com>

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

This turn track metadata into a property called "Track" of MediaPlayer1
---
 profiles/audio/player.c | 81 +++++++++++++++----------------------------------
 1 file changed, 25 insertions(+), 56 deletions(-)

diff --git a/profiles/audio/player.c b/profiles/audio/player.c
index 8748893..c3aaef0 100644
--- a/profiles/audio/player.c
+++ b/profiles/audio/player.c
@@ -82,33 +82,6 @@ static void append_metadata(void *key, void *value, void *user_data)
 	dict_append_entry(dict, key, DBUS_TYPE_STRING, &value);
 }
 
-static DBusMessage *media_player_get_track(DBusConnection *conn,
-						DBusMessage *msg, void *data)
-{
-	struct media_player *mp = data;
-	DBusMessage *reply;
-	DBusMessageIter iter, dict;
-
-	reply = dbus_message_new_method_return(msg);
-	if (!reply)
-		return NULL;
-
-	dbus_message_iter_init_append(reply, &iter);
-
-	dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
-					DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
-					DBUS_TYPE_STRING_AS_STRING
-					DBUS_TYPE_VARIANT_AS_STRING
-					DBUS_DICT_ENTRY_END_CHAR_AS_STRING,
-					&dict);
-
-	g_hash_table_foreach(mp->track, append_metadata, &dict);
-
-	dbus_message_iter_close_container(&iter, &dict);
-
-	return reply;
-}
-
 static struct pending_req *find_pending(struct media_player *mp,
 							const char *key)
 {
@@ -246,16 +219,31 @@ static void set_setting(const GDBusPropertyTable *property,
 	player_set_setting(mp, id, property->name, value);
 }
 
+static gboolean get_track(const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *data)
+{
+	struct media_player *mp = data;
+	DBusMessageIter dict;
+
+	dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY,
+					DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
+					DBUS_TYPE_STRING_AS_STRING
+					DBUS_TYPE_VARIANT_AS_STRING
+					DBUS_DICT_ENTRY_END_CHAR_AS_STRING,
+					&dict);
+
+	g_hash_table_foreach(mp->track, append_metadata, &dict);
+
+	dbus_message_iter_close_container(iter, &dict);
+
+	return TRUE;
+}
+
 static const GDBusMethodTable media_player_methods[] = {
-	{ GDBUS_EXPERIMENTAL_METHOD("GetTrack",
-			NULL, GDBUS_ARGS({ "metadata", "a{sv}" }),
-			media_player_get_track) },
 	{ }
 };
 
 static const GDBusSignalTable media_player_signals[] = {
-	{ GDBUS_EXPERIMENTAL_SIGNAL("TrackChanged",
-			GDBUS_ARGS({ "metadata", "a{sv}" })) },
 	{ }
 };
 
@@ -272,6 +260,8 @@ static const GDBusPropertyTable media_player_properties[] = {
 					G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
 	{ "Scan", "s", get_setting, set_setting, setting_exists,
 					G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
+	{ "Track", "a{sv}", get_track, NULL, NULL,
+					G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
 	{ }
 };
 
@@ -424,33 +414,12 @@ void media_player_set_status(struct media_player *mp, const char *status)
 static gboolean process_metadata_changed(void *user_data)
 {
 	struct media_player *mp = user_data;
-	DBusMessage *signal;
-	DBusMessageIter iter, dict;
 
 	mp->process_id = 0;
 
-	signal = dbus_message_new_signal(mp->path, MEDIA_PLAYER_INTERFACE,
-							"TrackChanged");
-	if (signal == NULL) {
-		error("Unable to allocate TrackChanged signal");
-		return FALSE;
-	}
-
-	dbus_message_iter_init_append(signal, &iter);
-
-	dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
-					DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
-					DBUS_TYPE_STRING_AS_STRING
-					DBUS_TYPE_VARIANT_AS_STRING
-					DBUS_DICT_ENTRY_END_CHAR_AS_STRING,
-					&dict);
-
-
-	g_hash_table_foreach(mp->track, append_metadata, &dict);
-
-	dbus_message_iter_close_container(&iter, &dict);
-
-	g_dbus_send_message(btd_get_dbus_connection(), signal);
+	g_dbus_emit_property_changed(btd_get_dbus_connection(),
+					mp->path, MEDIA_PLAYER_INTERFACE,
+					"Track");
 
 	return FALSE;
 }
-- 
1.8.0.1


^ permalink raw reply related

* [PATCH BlueZ 1/8 v2] media-api: Add playback control methods to MediaPlayer1
From: Luiz Augusto von Dentz @ 2013-01-04 12:39 UTC (permalink / raw)
  To: linux-bluetooth

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

This adds methods such as Play, Pause directly in MediaPlayer1, in
addition to that Track is now turn into a property to take advantage of
ObjectManager and document the interface as experimental.
---
v2: Fix only emitting Track changes once and add device object propery

 doc/media-api.txt | 99 ++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 57 insertions(+), 42 deletions(-)

diff --git a/doc/media-api.txt b/doc/media-api.txt
index e2a72dc..eb1f74f 100644
--- a/doc/media-api.txt
+++ b/doc/media-api.txt
@@ -129,7 +129,7 @@ Media Control hierarchy
 =======================
 
 Service		org.bluez
-Interface	org.bluez.MediaControl1
+Interface	org.bluez.MediaControl1 [Deprecated]
 Object path	[variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
 
 Methods		void Play()
@@ -178,65 +178,47 @@ Properties
 MediaPlayer1 hierarchy
 ======================
 
-Service		unique name (Target role)
-Interface	org.bluez.MediaPlayer1
-Object path	freely definable
-
 Service		org.bluez (Controller role)
-Interface	org.bluez.MediaPlayer1
+Interface	org.bluez.MediaPlayer1 [Experimental]
 Object path	[variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/playerX
 
-Methods		dict GetTrack()
-
-			Returns known metadata of the current track.
-
-			See TrackChanged for possible values.
-
-		void Release()
-
-			This method gets called when the service daemon
-			unregisters the player which can then perform
-			cleanup tasks. There is no need to unregister the
-			player, because when this method gets called it has
-			already been unregistered.
+Methods		void Play()
 
-Signals		TrackChanged(dict metadata)
+			Resume playback.
 
-			This signal indicates that current track has changed.
-			All available metadata for the new track shall be set
-			at once in the metadata argument. Metadata cannot be
-			updated in parts, otherwise it will be interpreted as
-			multiple track changes.
+		void Pause()
 
-			Possible values:
+			Pause playback.
 
-				string Title:
+		void Stop()
 
-					Track title name
+			Stop playback.
 
-				string Artist:
+		void Next()
 
-					Track artist name
+			Next item.
 
-				string Album:
+		void Previous()
 
-					Track album name
+			Previous item.
 
-				string Genre:
+		void VolumeUp()
 
-					Track genre name
+			Adjust remote volume one step up
 
-				uint32 NumberOfTracks:
+		void VolumeDown()
 
-					Number of tracks in total
+			Adjust remote volume one step down
 
-				uint32 Number:
+		void FastForward()
 
-					Track number
+			Fast forward playback, this action is only stopped
+			when another method in this interface is called.
 
-				uint32 Duration:
+		void Rewind()
 
-					Track duration in milliseconds
+			Rewind playback, this action is only stopped
+			when another method in this interface is called.
 
 Properties	string Equalizer [readwrite]
 
@@ -258,8 +240,8 @@ Properties	string Equalizer [readwrite]
 		string Status [readonly]
 
 			Possible status: "playing", "stopped", "paused",
-					"forward-seek", "reverse-seek" or
-					"error"
+					"forward-seek", "reverse-seek"
+					or "error"
 
 		uint32 Position [readonly]
 
@@ -272,6 +254,39 @@ Properties	string Equalizer [readwrite]
 			possible to signal its end by setting position to the
 			maximum uint32 value.
 
+		dict Track [readonly]
+
+			Track metadata.
+
+			Possible values:
+
+				string Title:
+
+					Track title name
+
+				string Artist:
+
+					Track artist name
+
+				string Album:
+
+					Track album name
+
+				string Genre:
+
+					Track genre name
+
+				uint32 NumberOfTracks:
+
+					Number of tracks in total
+
+				uint32 Number:
+
+					Track number
+
+				uint32 Duration:
+
+					Track duration in milliseconds
 
 MediaEndpoint1 hierarchy
 ========================
-- 
1.8.0.1


^ permalink raw reply related

* Re: [PATCH v2 2/2] Bluetooth: Fix stop discovery while in STARTING state
From: Jaganath Kanakkassery @ 2013-01-04  7:46 UTC (permalink / raw)
  To: Gustavo Padovan; +Cc: linux-bluetooth
In-Reply-To: <20130103193837.GB2114@joana>

Hi Gustavo,

--------------------------------------------------
From: "Gustavo Padovan" <gustavo@padovan.org>
Sent: Friday, January 04, 2013 1:08 AM
To: "Jaganath Kanakkassery" <jaganath.k@samsung.com>
Cc: <linux-bluetooth@vger.kernel.org>
Subject: Re: [PATCH v2 2/2] Bluetooth: Fix stop discovery while in STARTING 
state

> Hi Jaganath,
>
> * Jaganath Kanakkassery <jaganath.k@samsung.com> [2012-12-21 18:20:25 
> +0530]:
>
>> If stop_discovery() is called when discovery state is STARTING, it
>> will be failed currently. This patch fixes this.
>>
>> Signed-off-by: Jaganath Kanakkassery <jaganath.k@samsung.com>
>> ---
>>  include/net/bluetooth/hci_core.h |    2 ++
>>  net/bluetooth/hci_event.c        |   14 ++++++++++++--
>>  net/bluetooth/mgmt.c             |   31 ++++++++++++++++++++++++++++++-
>>  3 files changed, 44 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/net/bluetooth/hci_core.h 
>> b/include/net/bluetooth/hci_core.h
>> index 119fcb6..c2886b7 100644
>> --- a/include/net/bluetooth/hci_core.h
>> +++ b/include/net/bluetooth/hci_core.h
>> @@ -64,6 +64,7 @@ struct discovery_state {
>>  DISCOVERY_RESOLVING,
>>  DISCOVERY_STOPPING,
>>  } state;
>> + u8  discovering;
>>  struct list_head all; /* All devices found during inquiry */
>>  struct list_head unknown; /* Name state not known */
>>  struct list_head resolve; /* Name needs to be resolved */
>> @@ -1066,6 +1067,7 @@ int mgmt_device_found(struct hci_dev *hdev, 
>> bdaddr_t *bdaddr, u8 link_type,
>>  int mgmt_remote_name(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 
>> link_type,
>>       u8 addr_type, s8 rssi, u8 *name, u8 name_len);
>>  int mgmt_start_discovery_failed(struct hci_dev *hdev, u8 status);
>> +int mgmt_start_discovery_complete(struct hci_dev *hdev, u8 status);
>>  int mgmt_stop_discovery_failed(struct hci_dev *hdev, u8 status);
>>  int mgmt_discovering(struct hci_dev *hdev, u8 discovering);
>>  int mgmt_interleaved_discovery(struct hci_dev *hdev);
>> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
>> index e248e7c..b486458 100644
>> --- a/net/bluetooth/hci_event.c
>> +++ b/net/bluetooth/hci_event.c
>> @@ -1092,7 +1092,12 @@ static void hci_cc_le_set_scan_enable(struct 
>> hci_dev *hdev,
>>  set_bit(HCI_LE_SCAN, &hdev->dev_flags);
>>
>>  hci_dev_lock(hdev);
>> - hci_discovery_set_state(hdev, DISCOVERY_FINDING);
>> + if (hdev->discovery.state == DISCOVERY_STOPPING) {
>> + hci_cancel_le_scan(hdev);
>> + mgmt_start_discovery_complete(hdev, 0);
>
> Reply to mgmt with an error here might be better.

I think the best error which can be given here is
MGMT_STATUS_CANCELLED. But this error is not accessible in hci_event.c

>> + } else {
>> + hci_discovery_set_state(hdev, DISCOVERY_FINDING);
>> + }
>>  hci_dev_unlock(hdev);
>>  break;
>>
>> @@ -1189,7 +1194,12 @@ static void hci_cs_inquiry(struct hci_dev *hdev, 
>> __u8 status)
>>  set_bit(HCI_INQUIRY, &hdev->flags);
>>
>>  hci_dev_lock(hdev);
>> - hci_discovery_set_state(hdev, DISCOVERY_FINDING);
>> + if (hdev->discovery.state == DISCOVERY_STOPPING) {
>> + hci_cancel_inquiry(hdev);
>> + mgmt_start_discovery_complete(hdev, 0);
>> + } else {
>> + hci_discovery_set_state(hdev, DISCOVERY_FINDING);
>> + }
>>  hci_dev_unlock(hdev);
>>  }
>>
>> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
>> index d6c0d78..ba4171f 100644
>> --- a/net/bluetooth/mgmt.c
>> +++ b/net/bluetooth/mgmt.c
>> @@ -2385,7 +2385,8 @@ static int stop_discovery(struct sock *sk, struct 
>> hci_dev *hdev, void *data,
>>
>>  hci_dev_lock(hdev);
>>
>> - if (!hci_discovery_active(hdev)) {
>> + if (hdev->discovery.state != DISCOVERY_STARTING &&
>> +     !hci_discovery_active(hdev)) {
>>  err = cmd_complete(sk, hdev->id, MGMT_OP_STOP_DISCOVERY,
>>     MGMT_STATUS_REJECTED, &mgmt_cp->type,
>>     sizeof(mgmt_cp->type));
>> @@ -2433,6 +2434,10 @@ static int stop_discovery(struct sock *sk, struct 
>> hci_dev *hdev, void *data,
>>
>>  break;
>>
>> + case DISCOVERY_STARTING:
>> + err = 0;
>> + break;
>> +
>>  default:
>>  BT_DBG("unknown discovery state %u", hdev->discovery.state);
>>  err = -EFAULT;
>> @@ -3624,6 +3629,25 @@ int mgmt_start_discovery_failed(struct hci_dev 
>> *hdev, u8 status)
>>  return err;
>>  }
>>
>> +int mgmt_start_discovery_complete(struct hci_dev *hdev, u8 status)
>> +{
>> + struct pending_cmd *cmd;
>> + u8 type;
>> + int err;
>> +
>> + cmd = mgmt_pending_find(MGMT_OP_START_DISCOVERY, hdev);
>> + if (!cmd)
>> + return -ENOENT;
>> +
>> + type = hdev->discovery.type;
>> +
>> + err = cmd_complete(cmd->sk, hdev->id, cmd->opcode, mgmt_status(status),
>> +    &type, sizeof(type));
>> + mgmt_pending_remove(cmd);
>> +
>> + return err;
>> +}
>
> This is exactly the same thing as mgmt_start_discovery_failed(), just 
> rename it
> to _complete() as you did with mgmt_stop_discovery_failed(). Do it as a
> separate patch.

mgmt_start_discovery_failed() sets discovery state to STOPPED which also 
sends
stop_discovery_complete internally. I think both are inappropriate at the 
point
where mgmt_start_discovery_complete() is called.

How abt renaming the new function mgmt_start_discovery_complete() to
mgmt_start_discovery_cancelled and send MGMT_STATUS_CANCELLED in that?

This way your first comment also will be taken care.

Please let me know your opinion.

Thanks,
Jaganath
 


^ permalink raw reply

* Re: [PATCH 2/2] Bluetooth: Fix authentication if acl data comes before remote feature evt
From: Jaganath Kanakkassery @ 2013-01-04  5:57 UTC (permalink / raw)
  To: Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <20130103140026.GB18154@x220.ger.corp.intel.com>

Hi Johan,

--------------------------------------------------
From: "Johan Hedberg" <johan.hedberg@gmail.com>
Sent: Thursday, January 03, 2013 7:30 PM
To: "Jaganath Kanakkassery" <jaganath.k@samsung.com>
Cc: <linux-bluetooth@vger.kernel.org>
Subject: Re: [PATCH 2/2] Bluetooth: Fix authentication if acl data comes 
before remote feature evt

> Hi Jaganath,
>
> On Thu, Jan 03, 2013, Jaganath Kanakkassery wrote:
>> If remote device sends l2cap info request before read_remote_ext_feature
>> completes then mgmt_connected will be sent in hci_acldata_packet() and
>> remote name request wont be sent and eventually authentication wont 
>> happen
>>
>> Hcidump log of the issue
>>
>> < HCI Command: Create Connection (0x01|0x0005) plen 13
>>     bdaddr BC:85:1F:74:7F:29 ptype 0xcc18 rswitch 0x01 clkoffset 0x4bf7 
>> (valid)
>>     Packet type: DM1 DM3 DM5 DH1 DH3 DH5
>> > HCI Event: Command Status (0x0f) plen 4
>>     Create Connection (0x01|0x0005) status 0x00 ncmd 1
>> > HCI Event: Connect Complete (0x03) plen 11
>>     status 0x00 handle 12 bdaddr BC:85:1F:74:7F:29 type ACL encrypt 0x00
>> < HCI Command: Read Remote Supported Features (0x01|0x001b) plen 2
>>     handle 12
>> > HCI Event: Command Status (0x0f) plen 4
>>     Read Remote Supported Features (0x01|0x001b) status 0x00 ncmd 1
>> > HCI Event: Read Remote Supported Features (0x0b) plen 11
>>     status 0x00 handle 12
>>     Features: 0xbf 0xfe 0xcf 0xfe 0xdb 0xff 0x7b 0x87
>> > HCI Event: Max Slots Change (0x1b) plen 3
>>     handle 12 slots 5
>> < HCI Command: Read Remote Extended Features (0x01|0x001c) plen 3
>>     handle 12 page 1
>> > HCI Event: Command Status (0x0f) plen 4
>>     Read Remote Extended Features (0x01|0x001c) status 0x00 ncmd 1
>> > ACL data: handle 12 flags 0x02 dlen 10
>>     L2CAP(s): Info req: type 2
>> < ACL data: handle 12 flags 0x00 dlen 16
>>     L2CAP(s): Info rsp: type 2 result 0
>>       Extended feature mask 0x00b8
>>         Enhanced Retransmission mode
>>         Streaming mode
>>         FCS Option
>>         Fixed Channels
>> > HCI Event: Read Remote Extended Features (0x23) plen 13
>>     status 0x00 handle 12 page 1 max 1
>>     Features: 0x01 0x00 0x00 0x00 0x00 0x00 0x00 0x00
>> > ACL data: handle 12 flags 0x02 dlen 10
>>     L2CAP(s): Info req: type 3
>> < ACL data: handle 12 flags 0x00 dlen 20
>>     L2CAP(s): Info rsp: type 3 result 0
>>       Fixed channel list 0x00000002
>>         L2CAP Signalling Channel
>> > HCI Event: Number of Completed Packets (0x13) plen 5
>>     handle 12 packets 2
>>
>> Signed-off-by: Jaganath Kanakkassery <jaganath.k@samsung.com>
>> ---
>>  net/bluetooth/hci_core.c |    1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
>> index 596660d..c14def9 100644
>> --- a/net/bluetooth/hci_core.c
>> +++ b/net/bluetooth/hci_core.c
>> @@ -2812,6 +2812,7 @@ static void hci_acldata_packet(struct hci_dev 
>> *hdev, struct sk_buff *skb)
>>
>>  hci_dev_lock(hdev);
>>  if (test_bit(HCI_MGMT, &hdev->dev_flags) &&
>> +     !hci_outgoing_auth_needed(hdev, conn) &&
>>      !test_and_set_bit(HCI_CONN_MGMT_CONNECTED, &conn->flags))
>>  mgmt_device_connected(hdev, &conn->dst, conn->type,
>>        conn->dst_type, 0, NULL, 0,
>
> I'm not completely sure if this is the right way or even the right place
> to fix the issue. The reason why this if-clause is here is so that we
> don't get a too late mgmt_connected event in case the remote device is
> fast in sending an L2CAP Connect Request. Maybe if-clause needs to be
> made L2CAP Connect request specific (and moved to an L2CAP specific
> location) or then something added to the code path taken for the info
> request?

If the reason for mgmt_connected in acl_data() is to handle early l2cap
connect request from remote then I think it is better to move it to
l2cap connect request as you said.

So I will add mgmt_connected in l2cap_connect_req() before sending
l2cap connect response?

This will solve the authentication issue as well.

Thanks,
Jaganath 


^ permalink raw reply

* [PATCH 2/2] rfkill: Fix count parameter in read
From: Jaganath Kanakkassery @ 2013-01-04  5:42 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Jaganath Kanakkassery
In-Reply-To: <1357278148-27872-1-git-send-email-jaganath.k@samsung.com>

Since g_str_has_prefix() has been used with the output buffer of read
it should be NULL terminated
---
 src/rfkill.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/rfkill.c b/src/rfkill.c
index bac9efd..51591cb 100644
--- a/src/rfkill.c
+++ b/src/rfkill.c
@@ -114,7 +114,7 @@ static gboolean rfkill_event(GIOChannel *chan,
 
 	memset(sysname, 0, sizeof(sysname));
 
-	if (read(fd, sysname, sizeof(sysname)) < 4) {
+	if (read(fd, sysname, sizeof(sysname) - 1) < 4) {
 		close(fd);
 		return TRUE;
 	}
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 1/2] storage: Fix memory leak
From: Jaganath Kanakkassery @ 2013-01-04  5:42 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Jaganath Kanakkassery

If bt_uuid2string() returns NULL then svcclass has to be freed
---
 src/storage.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/storage.c b/src/storage.c
index 375974a..be3bbf2 100644
--- a/src/storage.c
+++ b/src/storage.c
@@ -180,8 +180,10 @@ sdp_record_t *find_record_in_list(sdp_list_t *recs, const char *uuid)
 
 		/* Extract the uuid */
 		uuid_str = bt_uuid2string(svcclass->data);
-		if (!uuid_str)
+		if (!uuid_str) {
+			sdp_list_free(svcclass, free);
 			continue;
+		}
 
 		if (!strcasecmp(uuid_str, uuid)) {
 			sdp_list_free(svcclass, free);
-- 
1.7.9.5


^ permalink raw reply related

* Re: [PATCH BlueZ] gdbus: Don't include just added interfaces in GetManagedObjects
From: Marcel Holtmann @ 2013-01-04  5:36 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: linux-bluetooth
In-Reply-To: <1357270424-24036-1-git-send-email-lucas.demarchi@profusion.mobi>

Hi Lucas,

> If we received a call to ObjectManager.GetManagedObject we should not
> include in the response the interfaces in data->added. This is because
> it's not guaranteed that those interfaces will trigger an
> InterfacesAdded signal, which is the case if the interface is removed in
> the same mainloop iteration.
> ---
>  gdbus/object.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)

patch has been applied.

Regards

Marcel



^ permalink raw reply

* Re: [PATCH BlueZ 3/3] gitignore: Ignore file generated by Automake 1.13
From: Marcel Holtmann @ 2013-01-04  5:32 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: linux-bluetooth, Lucas De Marchi
In-Reply-To: <1357262465-19231-3-git-send-email-lucas.demarchi@profusion.mobi>

Hi Lucas,

> ---
>  .gitignore | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/.gitignore b/.gitignore
> index 04c9862..619f492 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -25,6 +25,7 @@ ltmain.sh
>  missing
>  stamp-h1
>  autom4te.cache
> +test-driver

no idea what this is. I do not have that. What is it actually.

Regards

Marcel



^ permalink raw reply

* Re: [PATCH BlueZ 2/3] gdbus: Simplify generated introspection
From: Marcel Holtmann @ 2013-01-04  5:32 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: linux-bluetooth, Lucas De Marchi
In-Reply-To: <1357262465-19231-2-git-send-email-lucas.demarchi@profusion.mobi>

Hi Lucas,

> The generated introspection is not supposed to be read as is by human,
> so there's no point in printing the indentation or writing more code to
> use auto-close tags.
> 
> If it's desired to read the raw xml file, user can always use other
> tools to transform the output such as "xmllint --format".
> 
> This also fixes a missing </property> when property is deprecated.
> ---
>  gdbus/object.c | 103 +++++++++++++++++++++------------------------------------
>  1 file changed, 38 insertions(+), 65 deletions(-)

patch has been applied.

Regards

Marcel



^ permalink raw reply

* Re: [PATCH BlueZ 1/3] build: Do not use deprecated AM_CONFIG_HEADER
From: Marcel Holtmann @ 2013-01-04  5:29 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: linux-bluetooth, Lucas De Marchi
In-Reply-To: <1357262465-19231-1-git-send-email-lucas.demarchi@profusion.mobi>

Hi Lucas,

> The long-obsoleted AM_CONFIG_HEADER macro was removed in automake 1.13.
> Use AC_CONFIG_HEADERS instead.
> ---
>  configure.ac | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

patch has been applied.

For extra credits, you could update the other projects as well.

Regards

Marcel



^ permalink raw reply

* [PATCH BlueZ] gdbus: Don't include just added interfaces in GetManagedObjects
From: Lucas De Marchi @ 2013-01-04  3:33 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Lucas De Marchi

If we received a call to ObjectManager.GetManagedObject we should not
include in the response the interfaces in data->added. This is because
it's not guaranteed that those interfaces will trigger an
InterfacesAdded signal, which is the case if the interface is removed in
the same mainloop iteration.
---
 gdbus/object.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/gdbus/object.c b/gdbus/object.c
index 2b6ae31..e569acb 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -1064,6 +1064,7 @@ static const GDBusMethodTable introspect_methods[] = {
 static void append_interfaces(struct generic_data *data, DBusMessageIter *iter)
 {
 	DBusMessageIter array;
+	GSList *l;
 
 	dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY,
 				DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
@@ -1075,7 +1076,12 @@ static void append_interfaces(struct generic_data *data, DBusMessageIter *iter)
 				DBUS_DICT_ENTRY_END_CHAR_AS_STRING
 				DBUS_DICT_ENTRY_END_CHAR_AS_STRING, &array);
 
-	g_slist_foreach(data->interfaces, append_interface, &array);
+	for (l = data->interfaces; l != NULL; l = l->next) {
+		if (g_slist_find(data->added, l->data))
+			continue;
+
+		append_interface(l->data, &array);
+	}
 
 	dbus_message_iter_close_container(iter, &array);
 }
-- 
1.8.1


^ permalink raw reply related

* [RFC] gdbus: Process changes upon ObjectManager.GetManagedObjects
From: Lucas De Marchi @ 2013-01-04  1:50 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Lucas De Marchi

From: Lucas De Marchi <lucas.de.marchi@gmail.com>

If ObjectManager.GetManagedObjects is called, we need to flush all
pending signals for interfaces added/removed, otherwise we might answer
the method saying an interface exists but never send a signal when it's
removed.
---

It's an RFC because it's untested. If anyone can write a proper unit test for
this, it would be good. Otherwise I can probably do it in the weekend.

 gdbus/object.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/gdbus/object.c b/gdbus/object.c
index 2b6ae31..3d9ab5f 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -1108,6 +1108,11 @@ static DBusMessage *get_objects(DBusConnection *connection,
 	if (reply == NULL)
 		return NULL;
 
+	if (data->process_id > 0) {
+		g_source_remove(data->process_id);
+		process_changes(data);
+	}
+
 	dbus_message_iter_init_append(reply, &iter);
 
 	dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
-- 
1.8.1


^ permalink raw reply related

* [PATCH BlueZ 3/3] gitignore: Ignore file generated by Automake 1.13
From: Lucas De Marchi @ 2013-01-04  1:21 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Lucas De Marchi
In-Reply-To: <1357262465-19231-1-git-send-email-lucas.demarchi@profusion.mobi>

From: Lucas De Marchi <lucas.de.marchi@gmail.com>

---
 .gitignore | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.gitignore b/.gitignore
index 04c9862..619f492 100644
--- a/.gitignore
+++ b/.gitignore
@@ -25,6 +25,7 @@ ltmain.sh
 missing
 stamp-h1
 autom4te.cache
+test-driver
 
 lib/bluez.pc
 lib/bluetooth
-- 
1.8.1


^ permalink raw reply related

* [PATCH BlueZ 2/3] gdbus: Simplify generated introspection
From: Lucas De Marchi @ 2013-01-04  1:21 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Lucas De Marchi
In-Reply-To: <1357262465-19231-1-git-send-email-lucas.demarchi@profusion.mobi>

From: Lucas De Marchi <lucas.de.marchi@gmail.com>

The generated introspection is not supposed to be read as is by human,
so there's no point in printing the indentation or writing more code to
use auto-close tags.

If it's desired to read the raw xml file, user can always use other
tools to transform the output such as "xmllint --format".

This also fixes a missing </property> when property is deprecated.
---
 gdbus/object.c | 103 +++++++++++++++++++++------------------------------------
 1 file changed, 38 insertions(+), 65 deletions(-)

diff --git a/gdbus/object.c b/gdbus/object.c
index b9cb284..2b6ae31 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -97,7 +97,7 @@ static void print_arguments(GString *gstr, const GDBusArgInfo *args,
 {
 	for (; args && args->name; args++) {
 		g_string_append_printf(gstr,
-					"\t\t\t<arg name=\"%s\" type=\"%s\"",
+					"<arg name=\"%s\" type=\"%s\"",
 					args->name, args->signature);
 
 		if (direction)
@@ -109,15 +109,15 @@ static void print_arguments(GString *gstr, const GDBusArgInfo *args,
 	}
 }
 
-#define G_DBUS_ANNOTATE(prefix_, name_, value_)				\
-	prefix_ "<annotation name=\"org.freedesktop.DBus." name_ "\" "	\
-	"value=\"" value_ "\"/>\n"
+#define G_DBUS_ANNOTATE(name_, value_)				\
+	"<annotation name=\"org.freedesktop.DBus." name_ "\" "	\
+	"value=\"" value_ "\"/>"
 
-#define G_DBUS_ANNOTATE_DEPRECATED(prefix_) \
-	G_DBUS_ANNOTATE(prefix_, "Deprecated", "true")
+#define G_DBUS_ANNOTATE_DEPRECATED \
+	G_DBUS_ANNOTATE("Deprecated", "true")
 
-#define G_DBUS_ANNOTATE_NOREPLY(prefix_) \
-	G_DBUS_ANNOTATE(prefix_, "Method.NoReply", "true")
+#define G_DBUS_ANNOTATE_NOREPLY \
+	G_DBUS_ANNOTATE("Method.NoReply", "true")
 
 static gboolean check_experimental(int flags, int flag)
 {
@@ -134,85 +134,58 @@ static void generate_interface_xml(GString *gstr, struct interface_data *iface)
 	const GDBusPropertyTable *property;
 
 	for (method = iface->methods; method && method->name; method++) {
-		gboolean deprecated = method->flags &
-						G_DBUS_METHOD_FLAG_DEPRECATED;
-		gboolean noreply = method->flags &
-						G_DBUS_METHOD_FLAG_NOREPLY;
-
 		if (check_experimental(method->flags,
 					G_DBUS_METHOD_FLAG_EXPERIMENTAL))
 			continue;
 
-		if (!deprecated && !noreply &&
-				!(method->in_args && method->in_args->name) &&
-				!(method->out_args && method->out_args->name))
-			g_string_append_printf(gstr,
-						"\t\t<method name=\"%s\"/>\n",
-						method->name);
-		else {
+		g_string_append_printf(gstr, "<method name=\"%s\">",
+								method->name);
+		print_arguments(gstr, method->in_args, "in");
+		print_arguments(gstr, method->out_args, "out");
+
+		if (method->flags & G_DBUS_METHOD_FLAG_DEPRECATED)
 			g_string_append_printf(gstr,
-						"\t\t<method name=\"%s\">\n",
-						method->name);
-			print_arguments(gstr, method->in_args, "in");
-			print_arguments(gstr, method->out_args, "out");
-
-			if (deprecated)
-				g_string_append_printf(gstr,
-					G_DBUS_ANNOTATE_DEPRECATED("\t\t\t"));
-			if (noreply)
-				g_string_append_printf(gstr,
-					G_DBUS_ANNOTATE_NOREPLY("\t\t\t"));
-
-			g_string_append_printf(gstr, "\t\t</method>\n");
-		}
+						G_DBUS_ANNOTATE_DEPRECATED);
+
+		if (method->flags & G_DBUS_METHOD_FLAG_NOREPLY)
+			g_string_append_printf(gstr, G_DBUS_ANNOTATE_NOREPLY);
+
+		g_string_append_printf(gstr, "</method>");
 	}
 
 	for (signal = iface->signals; signal && signal->name; signal++) {
-		gboolean deprecated = signal->flags &
-						G_DBUS_SIGNAL_FLAG_DEPRECATED;
-
 		if (check_experimental(signal->flags,
 					G_DBUS_SIGNAL_FLAG_EXPERIMENTAL))
 			continue;
 
-		if (!deprecated && !(signal->args && signal->args->name))
-			g_string_append_printf(gstr,
-						"\t\t<signal name=\"%s\"/>\n",
-						signal->name);
-		else {
-			g_string_append_printf(gstr,
-						"\t\t<signal name=\"%s\">\n",
-						signal->name);
-			print_arguments(gstr, signal->args, NULL);
+		g_string_append_printf(gstr, "<signal name=\"%s\">",
+								signal->name);
+		print_arguments(gstr, signal->args, NULL);
 
-			if (deprecated)
-				g_string_append_printf(gstr,
-					G_DBUS_ANNOTATE_DEPRECATED("\t\t\t"));
+		if (signal->flags & G_DBUS_SIGNAL_FLAG_DEPRECATED)
+			g_string_append_printf(gstr,
+						G_DBUS_ANNOTATE_DEPRECATED);
 
-			g_string_append_printf(gstr, "\t\t</signal>\n");
-		}
+		g_string_append_printf(gstr, "</signal>\n");
 	}
 
 	for (property = iface->properties; property && property->name;
 								property++) {
-		gboolean deprecated = property->flags &
-					G_DBUS_PROPERTY_FLAG_DEPRECATED;
-
 		if (check_experimental(property->flags,
 					G_DBUS_PROPERTY_FLAG_EXPERIMENTAL))
 			continue;
 
-		g_string_append_printf(gstr, "\t\t<property name=\"%s\""
-					" type=\"%s\" access=\"%s%s\"",
+		g_string_append_printf(gstr, "<property name=\"%s\""
+					" type=\"%s\" access=\"%s%s\">",
 					property->name,	property->type,
 					property->get ? "read" : "",
 					property->set ? "write" : "");
 
-		if (!deprecated)
-			g_string_append_printf(gstr, "/>\n");
-		else
+		if (property->flags & G_DBUS_PROPERTY_FLAG_DEPRECATED)
 			g_string_append_printf(gstr,
-				G_DBUS_ANNOTATE_DEPRECATED(">\n\t\t\t"));
+						G_DBUS_ANNOTATE_DEPRECATED);
+
+		g_string_append_printf(gstr, "</property>");
 	}
 }
 
@@ -228,30 +201,30 @@ static void generate_introspection_xml(DBusConnection *conn,
 
 	gstr = g_string_new(DBUS_INTROSPECT_1_0_XML_DOCTYPE_DECL_NODE);
 
-	g_string_append_printf(gstr, "<node>\n");
+	g_string_append_printf(gstr, "<node>");
 
 	for (list = data->interfaces; list; list = list->next) {
 		struct interface_data *iface = list->data;
 
-		g_string_append_printf(gstr, "\t<interface name=\"%s\">\n",
+		g_string_append_printf(gstr, "<interface name=\"%s\">",
 								iface->name);
 
 		generate_interface_xml(gstr, iface);
 
-		g_string_append_printf(gstr, "\t</interface>\n");
+		g_string_append_printf(gstr, "</interface>");
 	}
 
 	if (!dbus_connection_list_registered(conn, path, &children))
 		goto done;
 
 	for (i = 0; children[i]; i++)
-		g_string_append_printf(gstr, "\t<node name=\"%s\"/>\n",
+		g_string_append_printf(gstr, "<node name=\"%s\"/>",
 								children[i]);
 
 	dbus_free_string_array(children);
 
 done:
-	g_string_append_printf(gstr, "</node>\n");
+	g_string_append_printf(gstr, "</node>");
 
 	data->introspect = g_string_free(gstr, FALSE);
 }
-- 
1.8.1


^ permalink raw reply related

* [PATCH BlueZ 1/3] build: Do not use deprecated AM_CONFIG_HEADER
From: Lucas De Marchi @ 2013-01-04  1:21 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Lucas De Marchi

From: Lucas De Marchi <lucas.de.marchi@gmail.com>

The long-obsoleted AM_CONFIG_HEADER macro was removed in automake 1.13.
Use AC_CONFIG_HEADERS instead.
---
 configure.ac | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index cdfc013..e8193ca 100644
--- a/configure.ac
+++ b/configure.ac
@@ -3,7 +3,7 @@ AC_INIT(bluez, 5.0)
 
 AM_INIT_AUTOMAKE([foreign subdir-objects color-tests silent-rules
 					tar-pax no-dist-gzip dist-xz])
-AM_CONFIG_HEADER(config.h)
+AC_CONFIG_HEADERS(config.h)
 AC_USE_SYSTEM_EXTENSIONS
 
 m4_ifdef([AM_SILENT_RULES], [AM_SILENT_RULES([yes])])
-- 
1.8.1


^ permalink raw reply related

* RE: [PATCH] Bluetooth: fix the oops due to conn->hcon == NULL in shutdown case
From: Liu, Chuansheng @ 2013-01-04  0:55 UTC (permalink / raw)
  To: Gustavo Padovan
  Cc: marcel@holtmann.org, johan.hedberg@gmail.com,
	linux-bluetooth@vger.kernel.org, linux-kernel@vger.kernel.org,
	Liu, Chuansheng
In-Reply-To: <20130103220258.GF2114@joana>



> -----Original Message-----
> From: Gustavo Padovan [mailto:gustavo@padovan.org]
> Sent: Friday, January 04, 2013 6:03 AM
> To: Liu, Chuansheng
> Cc: marcel@holtmann.org; johan.hedberg@gmail.com;
> linux-bluetooth@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] Bluetooth: fix the oops due to conn->hcon =3D=3D NUL=
L in
> shutdown case
>=20
> Hi Chuansheng,
>=20
> * Chuansheng Liu <chuansheng.liu@intel.com> [2012-12-25 18:04:17 +0800]:
>=20
> >
> > Meet one panic issue as below stack:


> > Disassemble the code:
> > base address of __sco_sock_close is 0xc184f410
> > 0xc184f4f8 <+232>:   lock decl 0x8(%ebx) < =3D=3D crash here, ebx is 0x=
0,
> >
> > the related source code is:
> > (gdb) l *0xc184f4f8
> > 0xc184f4f8 is in __sco_sock_close (arch/x86/include/asm/atomic.h:123)
> > 119     static inline int atomic_dec_and_test(atomic_t *v)
> > 123             asm volatile(LOCK_PREFIX "decl %0; sete %1"
> >
> > The whole call stack is:
> > sys_shutdown()
> >   sco_sock_shutdown()
> >     __sco_sock_close()
> >       hci_conn_put()
> >         atomic_dec_and_test()
> >
> > Due to the conn->hcon is NULL, and the member hcon->refcnt is at offset=
 0x8,
> > so "BUG: unable to handle kernel NULL pointer dereference at 00000008"
> > appears.
Could you add the above crash info to indicate where crashed? Thanks.

> >
> > Here fix it that adding the condition if conn->hcon is NULL, just like
> > in sco_chan_del().
> >
> > Signed-off-by: liu chuansheng <chuansheng.liu@intel.com>
> > ---
> >  net/bluetooth/sco.c |    6 ++++--
> >  1 files changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
> > index 531a93d..190f70c 100644
> > --- a/net/bluetooth/sco.c
> > +++ b/net/bluetooth/sco.c
> > @@ -355,8 +355,10 @@ static void __sco_sock_close(struct sock *sk)
> >  		if (sco_pi(sk)->conn) {
> >  			sk->sk_state =3D BT_DISCONN;
> >  			sco_sock_set_timer(sk, SCO_DISCONN_TIMEOUT);
> > -			hci_conn_put(sco_pi(sk)->conn->hcon);
> > -			sco_pi(sk)->conn->hcon =3D NULL;
> > +			if (sco_pi(sk)->conn->hcon) {
> > +				hci_conn_put(sco_pi(sk)->conn->hcon);
> > +				sco_pi(sk)->conn->hcon =3D NULL;
> > +			}
> >  		} else
> >  			sco_chan_del(sk, ECONNRESET);
> >  		break;
>=20
> Please check if the following patch fixes the issue for you:
>=20
> commit ae5668c1fc155d3034d0eedcdb52798390975a39 (HEAD, master)
> Author: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> Date:   Thu Jan 3 19:59:28 2013 -0200
>=20
>     Bluetooth: Check if the hci connection exists in SCO shutdown
>=20
>     Checking only for sco_conn seems to not be enough and lead to NULL
>     dereferences in the code, check for hcon instead.
>=20
>     <1>[11340.226404] BUG: unable to handle kernel NULL pointer
> dereference at
>     0000000
>     8
>     <4>[11340.226619] EIP is at __sco_sock_close+0xe8/0x1a0
>     <4>[11340.226629] EAX: f063a740 EBX: 00000000 ECX: f58f4544 EDX:
> 00000000
>     <4>[11340.226640] ESI: dec83e00 EDI: 5f9a081f EBP: e0fdff38 ESP:
> e0fdff1c
>     <0>[11340.226674] Stack:
>     <4>[11340.226682]  c184db87 c1251028 dec83e00 e0fdff38 c1754aef
> dec83e00
>     00000000
>     e0fdff5c
>     <4>[11340.226718]  c184f587 e0fdff64 e0fdff68 5f9a081f e0fdff5c
> c1751852
>     d7813800
>     62262f10
>     <4>[11340.226752]  e0fdff70 c1753c00 00000000 00000001 0000000d
> e0fdffac
>     c175425c
>     00000041
>     <0>[11340.226793] Call Trace:
>     <4>[11340.226813]  [<c184db87>] ? sco_sock_clear_timer+0x27/0x60
>     <4>[11340.226831]  [<c1251028>] ? local_bh_enable+0x68/0xd0
>     <4>[11340.226846]  [<c1754aef>] ? lock_sock_nested+0x4f/0x60
>     <4>[11340.226862]  [<c184f587>] sco_sock_shutdown+0x67/0xb0
>     <4>[11340.226879]  [<c1751852>] ? sockfd_lookup_light+0x22/0x80
>     <4>[11340.226897]  [<c1753c00>] sys_shutdown+0x30/0x60
>     <4>[11340.226912]  [<c175425c>] sys_socketcall+0x1dc/0x2a0
>     <4>[11340.226929]  [<c149ba78>] ? trace_hardirqs_on_thunk+0xc/0x10
>     <4>[11340.226944]  [<c18860f1>] syscall_call+0x7/0xb
>     <4>[11340.226960]  [<c1880000>] ? restore_cur+0x5e/0xd7
>     <0>[11340.226969] Code: <f0> ff 4b 08 0f 94 c0 84 c0 74 20 80 7b 19 0=
1 74
>     2f b8 0a 00 00
>=20
>     Reported-by: Chuansheng Liu <chuansheng.liu@intel.com>
>     Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>=20
> diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
> index 531a93d..57f250c 100644
> --- a/net/bluetooth/sco.c
> +++ b/net/bluetooth/sco.c
> @@ -352,7 +352,7 @@ static void __sco_sock_close(struct sock *sk)
>=20
>  	case BT_CONNECTED:
>  	case BT_CONFIG:
> -		if (sco_pi(sk)->conn) {
> +		if (sco_pi(sk)->conn->hcon) {
Your fix is incomplete, at least it should be:
		if ( (sco_pi(sk)->conn) && (sco_pi(sk)->conn->hcon)) {
Otherwise, it will bring another crash case. So could you add signed-off-by=
 me also?
Although it is not easy to reproduce, thanks.
Signed-off-by: liu chuansheng <chuansheng.liu@intel.com>

>  			sk->sk_state =3D BT_DISCONN;
>  			sco_sock_set_timer(sk, SCO_DISCONN_TIMEOUT);
>  			hci_conn_put(sco_pi(sk)->conn->hcon);

^ permalink raw reply

* Re: [PATCH] Bluetooth device 04ca:3008 should use ath3k
From: Sergio Cambra @ 2013-01-03 23:22 UTC (permalink / raw)
  To: Gustavo Padovan; +Cc: linux-bluetooth
In-Reply-To: <20130103203901.GD2114@joana>

Output of /sys/kernel/debug/usb/devices
T:  Bus=03 Lev=02 Prnt=02 Port=00 Cnt=01 Dev#=  6 Spd=12   MxCh= 0
D:  Ver= 1.10 Cls=e0(wlcon) Sub=01 Prot=01 MxPS=64 #Cfgs=  1
P:  Vendor=04ca ProdID=3008 Rev= 0.02
C:* #Ifs= 2 Cfg#= 1 Atr=e0 MxPwr=100mA
I:* If#= 0 Alt= 0 #EPs= 3 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=81(I) Atr=03(Int.) MxPS=  16 Ivl=1ms
E:  Ad=82(I) Atr=02(Bulk) MxPS=  64 Ivl=0ms
E:  Ad=02(O) Atr=02(Bulk) MxPS=  64 Ivl=0ms
I:* If#= 1 Alt= 0 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=83(I) Atr=01(Isoc) MxPS=   0 Ivl=1ms
E:  Ad=03(O) Atr=01(Isoc) MxPS=   0 Ivl=1ms
I:  If#= 1 Alt= 1 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=83(I) Atr=01(Isoc) MxPS=   9 Ivl=1ms
E:  Ad=03(O) Atr=01(Isoc) MxPS=   9 Ivl=1ms
I:  If#= 1 Alt= 2 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=83(I) Atr=01(Isoc) MxPS=  17 Ivl=1ms
E:  Ad=03(O) Atr=01(Isoc) MxPS=  17 Ivl=1ms
I:  If#= 1 Alt= 3 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=83(I) Atr=01(Isoc) MxPS=  25 Ivl=1ms
E:  Ad=03(O) Atr=01(Isoc) MxPS=  25 Ivl=1ms
I:  If#= 1 Alt= 4 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=83(I) Atr=01(Isoc) MxPS=  33 Ivl=1ms
E:  Ad=03(O) Atr=01(Isoc) MxPS=  33 Ivl=1ms
I:  If#= 1 Alt= 5 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E:  Ad=83(I) Atr=01(Isoc) MxPS=  49 Ivl=1ms
E:  Ad=03(O) Atr=01(Isoc) MxPS=  49 Ivl=1ms

Signed-off-by: Sergio Cambra <sergio@programatica.es>
---
 drivers/bluetooth/ath3k.c | 2 ++
 drivers/bluetooth/btusb.c | 1 +
 2 files changed, 3 insertions(+)

diff --git a/drivers/bluetooth/ath3k.c b/drivers/bluetooth/ath3k.c
index be17894..33c9a44 100644
--- a/drivers/bluetooth/ath3k.c
+++ b/drivers/bluetooth/ath3k.c
@@ -78,6 +78,7 @@ static struct usb_device_id ath3k_table[] = {
        { USB_DEVICE(0x13d3, 0x3375) },
        { USB_DEVICE(0x04CA, 0x3005) },
        { USB_DEVICE(0x04CA, 0x3006) },
+       { USB_DEVICE(0x04CA, 0x3008) },
        { USB_DEVICE(0x13d3, 0x3362) },
        { USB_DEVICE(0x0CF3, 0xE004) },
        { USB_DEVICE(0x0930, 0x0219) },
@@ -109,6 +110,7 @@ static struct usb_device_id ath3k_blist_tbl[] = {
        { USB_DEVICE(0x13d3, 0x3375), .driver_info = BTUSB_ATH3012 },
        { USB_DEVICE(0x04ca, 0x3005), .driver_info = BTUSB_ATH3012 },
        { USB_DEVICE(0x04ca, 0x3006), .driver_info = BTUSB_ATH3012 },
+       { USB_DEVICE(0x04ca, 0x3008), .driver_info = BTUSB_ATH3012 },
        { USB_DEVICE(0x13d3, 0x3362), .driver_info = BTUSB_ATH3012 },
        { USB_DEVICE(0x0cf3, 0xe004), .driver_info = BTUSB_ATH3012 },
        { USB_DEVICE(0x0930, 0x0219), .driver_info = BTUSB_ATH3012 },
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 3f6a993..7e351e3 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -136,6 +136,7 @@ static struct usb_device_id blacklist_table[] = {
        { USB_DEVICE(0x13d3, 0x3375), .driver_info = BTUSB_ATH3012 },
        { USB_DEVICE(0x04ca, 0x3005), .driver_info = BTUSB_ATH3012 },
        { USB_DEVICE(0x04ca, 0x3006), .driver_info = BTUSB_ATH3012 },
+       { USB_DEVICE(0x04ca, 0x3008), .driver_info = BTUSB_ATH3012 },
        { USB_DEVICE(0x13d3, 0x3362), .driver_info = BTUSB_ATH3012 },
        { USB_DEVICE(0x0cf3, 0xe004), .driver_info = BTUSB_ATH3012 },
        { USB_DEVICE(0x0930, 0x0219), .driver_info = BTUSB_ATH3012 },
-- 
1.8.0.2



^ permalink raw reply related

* Re: [PATCH 12/25] bluetooth/l2cap: don't use [delayed_]work_pending()
From: Gustavo Padovan @ 2013-01-03 22:27 UTC (permalink / raw)
  To: Tejun Heo; +Cc: linux-kernel, Marcel Holtmann, Johan Hedberg, linux-bluetooth
In-Reply-To: <1356141435-17340-13-git-send-email-tj@kernel.org>

Hi Tejun,

* Tejun Heo <tj@kernel.org> [2012-12-21 17:57:02 -0800]:

> There's no need to test whether a (delayed) work item in pending
> before queueing, flushing or cancelling it.  Most uses are unnecessary
> and quite a few of them are buggy.
> 
> Reimplement l2cap_set_timer() such that it uses mod_delayed_work() or
> schedule_delayed_work() depending on a new param @override and let the
> users specify whether to override or not instead of using
> delayed_work_pending().
> 
> Only compile tested.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Cc: Marcel Holtmann <marcel@holtmann.org>
> Cc: Gustavo Padovan <gustavo@padovan.org>
> Cc: Johan Hedberg <johan.hedberg@gmail.com>
> Cc: linux-bluetooth@vger.kernel.org
> ---
> Please let me know how this patch should be routed.  I can take it
> through the workqueue tree if necessary.
> 
> Thanks.
> 
>  include/net/bluetooth/l2cap.h | 24 ++++++++++++++++--------
>  net/bluetooth/l2cap_core.c    |  7 +++----
>  2 files changed, 19 insertions(+), 12 deletions(-)
> 
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index 7588ef4..f12cbeb 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -718,17 +718,25 @@ static inline void l2cap_chan_unlock(struct l2cap_chan *chan)
>  }
>  
>  static inline void l2cap_set_timer(struct l2cap_chan *chan,
> -				   struct delayed_work *work, long timeout)
> +				   struct delayed_work *work, long timeout,
> +				   bool override)
>  {
> +	bool was_pending;
> +
>  	BT_DBG("chan %p state %s timeout %ld", chan,
>  	       state_to_string(chan->state), timeout);
>  
> -	/* If delayed work cancelled do not hold(chan)
> -	   since it is already done with previous set_timer */
> -	if (!cancel_delayed_work(work))
> -		l2cap_chan_hold(chan);
> +	/* @work should hold a reference to @chan */
> +	l2cap_chan_hold(chan);
> +
> +	if (override)
> +		was_pending = mod_delayed_work(system_wq, work, timeout);
> +	else
> +		was_pending = !schedule_delayed_work(work, timeout);
>  
> -	schedule_delayed_work(work, timeout);
> +	/* if @work was already pending, lose the extra ref */
> +	if (was_pending)
> +		l2cap_chan_put(chan);
>  }
>  
>  static inline bool l2cap_clear_timer(struct l2cap_chan *chan,
> @@ -745,12 +753,12 @@ static inline bool l2cap_clear_timer(struct l2cap_chan *chan,
>  	return ret;
>  }
>  
> -#define __set_chan_timer(c, t) l2cap_set_timer(c, &c->chan_timer, (t))
> +#define __set_chan_timer(c, t) l2cap_set_timer(c, &c->chan_timer, (t), true)
>  #define __clear_chan_timer(c) l2cap_clear_timer(c, &c->chan_timer)
>  #define __clear_retrans_timer(c) l2cap_clear_timer(c, &c->retrans_timer)
>  #define __clear_monitor_timer(c) l2cap_clear_timer(c, &c->monitor_timer)
>  #define __set_ack_timer(c) l2cap_set_timer(c, &chan->ack_timer, \
> -		msecs_to_jiffies(L2CAP_DEFAULT_ACK_TO));
> +		msecs_to_jiffies(L2CAP_DEFAULT_ACK_TO), true);
>  #define __clear_ack_timer(c) l2cap_clear_timer(c, &c->ack_timer)
>  
>  static inline int __seq_offset(struct l2cap_chan *chan, __u16 seq1, __u16 seq2)
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 2c78208..91db91c 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -246,10 +246,9 @@ static inline void l2cap_chan_set_err(struct l2cap_chan *chan, int err)
>  
>  static void __set_retrans_timer(struct l2cap_chan *chan)
>  {
> -	if (!delayed_work_pending(&chan->monitor_timer) &&
> -	    chan->retrans_timeout) {
> +	if (chan->retrans_timeout) {
>  		l2cap_set_timer(chan, &chan->retrans_timer,
> -				msecs_to_jiffies(chan->retrans_timeout));
> +				msecs_to_jiffies(chan->retrans_timeout), false);

Did you notice we are talking about two different works here?
delayed_work_pending() is checking the monitor_timer while l2cap_set_timer is
setting the retrans_timer. We can't run one of them while the is running.
This mean you can just get rid of the override flag and simplify this patch a
lot.

	Gustavo

^ permalink raw reply

* Re: [RFC 3/3] Bluetooth:  Provide mgmt API for reading list of supported codecs
From: Vinicius Costa Gomes @ 2013-01-03 22:16 UTC (permalink / raw)
  To: Gustavo Padovan, Michael Knudsen, linux-bluetooth,
	Michael Knudsen
In-Reply-To: <20130103190916.GA2114@joana>

Hi Gustavo,

> 
> We think it is better to read the list of codecs from the SCO socket, we need
> to allow both oFono and PulseAudio to read them without the need of talking to
> BlueZ. Also, bluetoothd has nothing intersting to do with this information.

Take a look at the thread "CSA2: User space aspect"[1]. I have to agree
with Marcel, the mgmt command makes more sense. As how that information
will get to oFono/PulseAudio we have NewConnection() in the Profile API
and SetConfiguration() on the Media API, that may be extended (if
needed).

> 
> 	Gustavo
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


Cheers,
-- 
Vinicius

[1] http://thread.gmane.org/gmane.linux.bluez.kernel/31746

^ 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