* Re: [PATCH hcidump] Add TI Logger dump support
From: Arik Nemtsov @ 2012-12-23 14:30 UTC (permalink / raw)
To: chen.ganir; +Cc: marcel, linux-bluetooth
In-Reply-To: <CA+XVXfcm5k7qN_tgbX6zVtWjxo_5w+FNmmbhthw3+UtrH3tnYg@mail.gmail.com>
On Sun, Dec 23, 2012 at 4:11 PM, Arik Nemtsov <arik@wizery.com> wrote:
> On Sun, Dec 23, 2012 at 2:07 PM, <chen.ganir@ti.com> wrote:
>> From: Chen Ganir <chen.ganir@ti.com>
>>
>> Texas Instruments controllers can be configured to send the
>> internal firmware log through a vendor specific HCI event on
>> the hci transport.
>> This patch allows capturing those log events, and writing them
>> to a file, which can then be used with the latest TI Logger
>> application to read and show the logs.
>>
>> This is usefull in case there is no other way to get the TI log
>> (for example, the lack of a connection to the controller Log TX
>> hardware line).
> [...]
>> @@ -300,7 +308,24 @@ static int process_frames(int dev, int sock, int fd, unsigned long flags)
> [...]
>> + char out[2];
>> + int i;
>> +
>> + for(i = 0;i < tp->size-2;i++) {
>> + sprintf(out,"%02X",tp->data[i]);
>> + if (write_n(fd, out, 2) < 0) {
>
> Seems to me this can overflow when tp->size - 2 > sizeof(out) ?
I'm sorry. I misread the code - but there's still an off-by-one here,
since 3 bytes are written to out (the null terminator).
Arik
^ permalink raw reply
* [PATCH BlueZ 1/6] gdbus: Introduce G_DBUS_METHOD_FLAG_EXPERIMENTAL
From: Luiz Augusto von Dentz @ 2012-12-23 15:24 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
This flag can be used to mark methods as experimental, the marked
methods with this flag can be enabled by setting the environment variable
GDBUS_EXPERIMENTAL=1
---
gdbus/gdbus.h | 21 ++++++++++++++++++---
gdbus/object.c | 17 +++++++++++++++++
2 files changed, 35 insertions(+), 3 deletions(-)
diff --git a/gdbus/gdbus.h b/gdbus/gdbus.h
index 0e5c012..64f5d62 100644
--- a/gdbus/gdbus.h
+++ b/gdbus/gdbus.h
@@ -89,9 +89,10 @@ typedef void (* GDBusSecurityFunction) (DBusConnection *connection,
GDBusPendingReply pending);
enum GDBusMethodFlags {
- G_DBUS_METHOD_FLAG_DEPRECATED = (1 << 0),
- G_DBUS_METHOD_FLAG_NOREPLY = (1 << 1),
- G_DBUS_METHOD_FLAG_ASYNC = (1 << 2),
+ G_DBUS_METHOD_FLAG_DEPRECATED = (1 << 0),
+ G_DBUS_METHOD_FLAG_NOREPLY = (1 << 1),
+ G_DBUS_METHOD_FLAG_ASYNC = (1 << 2),
+ G_DBUS_METHOD_FLAG_EXPERIMENTAL = (1 << 3),
};
enum GDBusSignalFlags {
@@ -173,6 +174,20 @@ struct GDBusSecurityTable {
.function = _function, \
.flags = G_DBUS_METHOD_FLAG_ASYNC | G_DBUS_METHOD_FLAG_DEPRECATED
+#define GDBUS_EXPERIMENTAL_METHOD(_name, _in_args, _out_args, _function) \
+ .name = _name, \
+ .in_args = _in_args, \
+ .out_args = _out_args, \
+ .function = _function, \
+ .flags = G_DBUS_METHOD_FLAG_EXPERIMENTAL
+
+#define GDBUS_EXPERIMENTAL_ASYNC_METHOD(_name, _in_args, _out_args, _function) \
+ .name = _name, \
+ .in_args = _in_args, \
+ .out_args = _out_args, \
+ .function = _function, \
+ .flags = G_DBUS_METHOD_FLAG_ASYNC | G_DBUS_METHOD_FLAG_EXPERIMENTAL
+
#define GDBUS_NOREPLY_METHOD(_name, _in_args, _out_args, _function) \
.name = _name, \
.in_args = _in_args, \
diff --git a/gdbus/object.c b/gdbus/object.c
index 776d35e..180d48c 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -129,6 +129,14 @@ static void generate_interface_xml(GString *gstr, struct interface_data *iface)
G_DBUS_METHOD_FLAG_DEPRECATED;
gboolean noreply = method->flags &
G_DBUS_METHOD_FLAG_NOREPLY;
+ gboolean experimental = method->flags &
+ G_DBUS_METHOD_FLAG_EXPERIMENTAL;
+
+ if (experimental) {
+ const char *env = g_getenv("GDBUS_EXPERIMENTAL");
+ if (g_strcmp0(env, "1") != 0)
+ return;
+ }
if (!deprecated && !noreply &&
!(method->in_args && method->in_args->name) &&
@@ -1022,10 +1030,19 @@ static DBusHandlerResult generic_message(DBusConnection *connection,
for (method = iface->methods; method &&
method->name && method->function; method++) {
+ gboolean experimental = method->flags &
+ G_DBUS_METHOD_FLAG_EXPERIMENTAL;
+
if (dbus_message_is_method_call(message, iface->name,
method->name) == FALSE)
continue;
+ if (experimental) {
+ const char *env = g_getenv("GDBUS_EXPERIMENTAL");
+ if (g_strcmp0(env, "1") != 0)
+ return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
+ }
+
if (g_dbus_args_have_signature(method->in_args,
message) == FALSE)
continue;
--
1.7.11.7
^ permalink raw reply related
* [PATCH BlueZ 2/6] media: Enable RegisterPlayer and UnregisterPlayer methods as experimental
From: Luiz Augusto von Dentz @ 2012-12-23 15:24 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1356276298-13795-1-git-send-email-luiz.dentz@gmail.com>
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
profiles/audio/media.c | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/profiles/audio/media.c b/profiles/audio/media.c
index f728460..e4206e3 100644
--- a/profiles/audio/media.c
+++ b/profiles/audio/media.c
@@ -868,7 +868,6 @@ static DBusMessage *unregister_endpoint(DBusConnection *conn, DBusMessage *msg,
return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
}
-#if 0
static struct media_player *media_adapter_find_player(
struct media_adapter *adapter,
const char *sender,
@@ -1533,7 +1532,6 @@ static DBusMessage *unregister_player(DBusConnection *conn, DBusMessage *msg,
return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
}
-#endif
static const GDBusMethodTable media_methods[] = {
{ GDBUS_METHOD("RegisterEndpoint",
@@ -1541,14 +1539,12 @@ static const GDBusMethodTable media_methods[] = {
NULL, register_endpoint) },
{ GDBUS_METHOD("UnregisterEndpoint",
GDBUS_ARGS({ "endpoint", "o" }), NULL, unregister_endpoint) },
-#if 0
- { GDBUS_METHOD("RegisterPlayer",
+ { GDBUS_EXPERIMENTAL_METHOD("RegisterPlayer",
GDBUS_ARGS({ "player", "o" }, { "properties", "a{sv}" },
{ "metadata", "a{sv}" }),
NULL, register_player) },
- { GDBUS_METHOD("UnregisterPlayer",
+ { GDBUS_EXPERIMENTAL_METHOD("UnregisterPlayer",
GDBUS_ARGS({ "player", "o" }), NULL, unregister_player) },
-#endif
{ },
};
@@ -1559,10 +1555,8 @@ static void path_free(void *data)
while (adapter->endpoints)
release_endpoint(adapter->endpoints->data);
-#if 0
while (adapter->players)
media_player_destroy(adapter->players->data);
-#endif
adapters = g_slist_remove(adapters, adapter);
--
1.7.11.7
^ permalink raw reply related
* [PATCH BlueZ 3/6] gdbus: Introduce GDBusInterfaceFlags
From: Luiz Augusto von Dentz @ 2012-12-23 15:24 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1356276298-13795-1-git-send-email-luiz.dentz@gmail.com>
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
The flags should be passed in g_dbus_register_interface_with_flags,
currently there only one flag which is G_DBUS_INTERFACE_FLAG_EXPERIMENTAL
which works similarly to G_DBUS_METHOD_FLAG_EXPERIMENTAL but for the
whole interface.
---
gdbus/gdbus.h | 13 +++++++++++++
gdbus/object.c | 20 ++++++++++++++++++++
2 files changed, 33 insertions(+)
diff --git a/gdbus/gdbus.h b/gdbus/gdbus.h
index 64f5d62..f49d9f0 100644
--- a/gdbus/gdbus.h
+++ b/gdbus/gdbus.h
@@ -31,6 +31,7 @@ extern "C" {
#include <dbus/dbus.h>
#include <glib.h>
+typedef enum GDBusInterfaceFlags GDBusInterfaceFlags;
typedef enum GDBusMethodFlags GDBusMethodFlags;
typedef enum GDBusSignalFlags GDBusSignalFlags;
typedef enum GDBusPropertyFlags GDBusPropertyFlags;
@@ -88,6 +89,10 @@ typedef void (* GDBusSecurityFunction) (DBusConnection *connection,
gboolean interaction,
GDBusPendingReply pending);
+enum GDBusInterfaceFlags {
+ G_DBUS_INTERFACE_FLAG_EXPERIMENTAL = (1 << 0),
+};
+
enum GDBusMethodFlags {
G_DBUS_METHOD_FLAG_DEPRECATED = (1 << 0),
G_DBUS_METHOD_FLAG_NOREPLY = (1 << 1),
@@ -211,6 +216,14 @@ gboolean g_dbus_register_interface(DBusConnection *connection,
const GDBusPropertyTable *properties,
void *user_data,
GDBusDestroyFunction destroy);
+gboolean g_dbus_register_interface_with_flags(DBusConnection *connection,
+ const char *path, const char *name,
+ GDBusInterfaceFlags flags,
+ const GDBusMethodTable *methods,
+ const GDBusSignalTable *signals,
+ const GDBusPropertyTable *properties,
+ void *user_data,
+ GDBusDestroyFunction destroy);
gboolean g_dbus_unregister_interface(DBusConnection *connection,
const char *path, const char *name);
diff --git a/gdbus/object.c b/gdbus/object.c
index 180d48c..510763a 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -1350,6 +1350,26 @@ gboolean g_dbus_register_interface(DBusConnection *connection,
return TRUE;
}
+gboolean g_dbus_register_interface_with_flags(DBusConnection *connection,
+ const char *path, const char *name,
+ GDBusInterfaceFlags flags,
+ const GDBusMethodTable *methods,
+ const GDBusSignalTable *signals,
+ const GDBusPropertyTable *properties,
+ void *user_data,
+ GDBusDestroyFunction destroy)
+{
+ if (flags & G_DBUS_INTERFACE_FLAG_EXPERIMENTAL) {
+ const char *env = g_getenv("GDBUS_EXPERIMENTAL");
+ if (g_strcmp0(env, "1") != 0)
+ return FALSE;
+ }
+
+ return g_dbus_register_interface(connection, path, name, methods,
+ signals, properties, user_data,
+ destroy);
+}
+
gboolean g_dbus_unregister_interface(DBusConnection *connection,
const char *path, const char *name)
{
--
1.7.11.7
^ permalink raw reply related
* [PATCH BlueZ 4/6] gdbus: Call check_signals when sending signals with g_dbus_send_message
From: Luiz Augusto von Dentz @ 2012-12-23 15:24 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1356276298-13795-1-git-send-email-luiz.dentz@gmail.com>
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
If message passed to g_dbus_send_message is a signal verify if it is a
valid and there really exists an interface with respective signal name.
---
gdbus/object.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/gdbus/object.c b/gdbus/object.c
index 510763a..6951171 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -1475,6 +1475,15 @@ gboolean g_dbus_send_message(DBusConnection *connection, DBusMessage *message)
if (dbus_message_get_type(message) == DBUS_MESSAGE_TYPE_METHOD_CALL)
dbus_message_set_no_reply(message, TRUE);
+ else {
+ const char *path = dbus_message_get_path(message);
+ const char *interface = dbus_message_get_interface(message);
+ const char *name = dbus_message_get_member(message);
+ const GDBusArgInfo *args;
+
+ if (!check_signal(connection, path, interface, name, &args))
+ return FALSE;
+ }
result = dbus_connection_send(connection, message, NULL);
--
1.7.11.7
^ permalink raw reply related
* [PATCH BlueZ 5/6] player: Enable MediaPlayer1 interface as experimental
From: Luiz Augusto von Dentz @ 2012-12-23 15:24 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1356276298-13795-1-git-send-email-luiz.dentz@gmail.com>
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
This enable MediaPlayer1 when GDBUS_EXPERIMENTAL=1
---
profiles/audio/player.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/profiles/audio/player.c b/profiles/audio/player.c
index 005d0d1..60c7a8a 100644
--- a/profiles/audio/player.c
+++ b/profiles/audio/player.c
@@ -298,9 +298,9 @@ struct media_player *media_player_controller_create(const char *path)
g_free, g_free);
mp->progress = g_timer_new();
-#if 0
- if (!g_dbus_register_interface(btd_get_dbus_connection(),
+ if (!g_dbus_register_interface_with_flags(btd_get_dbus_connection(),
mp->path, MEDIA_PLAYER_INTERFACE,
+ G_DBUS_INTERFACE_FLAG_EXPERIMENTAL,
media_player_methods,
media_player_signals,
media_player_properties, mp, NULL)) {
@@ -308,7 +308,7 @@ struct media_player *media_player_controller_create(const char *path)
media_player_destroy(mp);
return NULL;
}
-#endif
+
DBG("%s", mp->path);
return mp;
@@ -410,7 +410,6 @@ void media_player_set_status(struct media_player *mp, const char *status)
static gboolean process_metadata_changed(void *user_data)
{
-#if 0
struct media_player *mp = user_data;
DBusMessage *signal;
DBusMessageIter iter, dict;
@@ -439,7 +438,7 @@ static gboolean process_metadata_changed(void *user_data)
dbus_message_iter_close_container(&iter, &dict);
g_dbus_send_message(btd_get_dbus_connection(), signal);
-#endif
+
return FALSE;
}
--
1.7.11.7
^ permalink raw reply related
* [PATCH BlueZ 6/6] AVRCP: Fix not checking for media_player_controller_create
From: Luiz Augusto von Dentz @ 2012-12-23 15:24 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1356276298-13795-1-git-send-email-luiz.dentz@gmail.com>
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Now that the MediaPlayer1 interface is experimental the interface
registration may fail which return NULL, in that case there is no
point on register to any notifications.
---
profiles/audio/avrcp.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
index 4e3d31d..ce070cd 100644
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -1987,6 +1987,11 @@ static gboolean avrcp_get_capabilities_resp(struct avctp *conn,
count = pdu->params[1];
+ path = device_get_path(session->dev->btd_dev);
+ mp = media_player_controller_create(path);
+ if (mp == NULL)
+ return FALSE;
+
for (; count > 0; count--) {
uint8_t event = pdu->params[1 + count];
@@ -2001,8 +2006,6 @@ static gboolean avrcp_get_capabilities_resp(struct avctp *conn,
}
}
- path = device_get_path(session->dev->btd_dev);
- mp = media_player_controller_create(path);
media_player_set_callbacks(mp, &ct_cbs, player);
player->user_data = mp;
player->destroy = (GDestroyNotify) media_player_destroy;
--
1.7.11.7
^ permalink raw reply related
* Re: [PATCH hcidump] Add TI Logger dump support
From: Marcel Holtmann @ 2012-12-23 15:57 UTC (permalink / raw)
To: chen.ganir; +Cc: linux-bluetooth
In-Reply-To: <1356264452-21200-1-git-send-email-chen.ganir@ti.com>
Hi Chen,
> Texas Instruments controllers can be configured to send the
> internal firmware log through a vendor specific HCI event on
> the hci transport.
> This patch allows capturing those log events, and writing them
> to a file, which can then be used with the latest TI Logger
> application to read and show the logs.
>
> This is usefull in case there is no other way to get the TI log
> (for example, the lack of a connection to the controller Log TX
> hardware line).
> ---
> parser/parser.h | 1 +
> src/hcidump.c | 44 ++++++++++++++++++++++++++++++++++++++++----
> 2 files changed, 41 insertions(+), 4 deletions(-)
we consumed hcidump into bluez.git. Please make patches against
bluez.git since bluez-hcidump.git is closed now.
> diff --git a/parser/parser.h b/parser/parser.h
> index f8f7009..d4fecf0 100644
> --- a/parser/parser.h
> +++ b/parser/parser.h
> @@ -63,6 +63,7 @@ struct frame {
> #define DUMP_BTSNOOP 0x1000
> #define DUMP_PKTLOG 0x2000
> #define DUMP_NOVENDOR 0x4000
> +#define DUMP_TILOGGER 0x8000
> #define DUMP_TYPE_MASK (DUMP_ASCII | DUMP_HEX | DUMP_EXT)
>
> /* Parser filter */
> diff --git a/src/hcidump.c b/src/hcidump.c
> index 18ae64e..3daf2b6 100644
> --- a/src/hcidump.c
> +++ b/src/hcidump.c
> @@ -110,6 +110,14 @@ struct pktlog_hdr {
> } __attribute__ ((packed));
> #define PKTLOG_HDR_SIZE (sizeof(struct pktlog_hdr))
>
> +struct tilogger_pkt {
> + uint8_t type;
> + uint8_t vendor;
> + uint8_t size;
> + uint16_t opcode;
> + uint8_t data[0]; /* Packet Data */
> +} __attribute__ ((packed));
> +
> static inline int read_n(int fd, char *buf, int len)
> {
> int t = 0, w;
> @@ -300,7 +308,24 @@ static int process_frames(int dev, int sock, int fd, unsigned long flags)
> case WRITE:
> case SERVER:
> /* Save or send dump */
> - if (flags & DUMP_BTSNOOP) {
> + if (flags & DUMP_TILOGGER && mode == WRITE) {
Don't bother with mode == WRITE check. I am taking the server code out
of the tool anyway.
> + struct tilogger_pkt *tp = frm.ptr;
> + if (tp->type == HCI_EVENT_PKT &&
> + tp->vendor == 0xFF &&
> + tp->opcode == 0x0400) {
This is not our coding style.
> + char out[2];
> + int i;
> +
> + for(i = 0;i < tp->size-2;i++) {
Neither is this.
> + sprintf(out,"%02X",tp->data[i]);
> + if (write_n(fd, out, 2) < 0) {
> + perror("Write error");
> + return -1;
> + }
> + }
> + }
> + continue;
> + } else if (flags & DUMP_BTSNOOP) {
> uint64_t ts;
> uint8_t pkt_type = ((uint8_t *) frm.data)[0];
> dp->size = htonl(frm.data_len);
> @@ -542,7 +567,9 @@ static int open_file(char *file, int mode, unsigned long flags)
> return fd;
> }
> } else {
> - if (flags & DUMP_BTSNOOP) {
> + if (flags & DUMP_TILOGGER) {
> + printf ("TI Logger dump\n");
And this is still not our coding style.
> + } else if (flags & DUMP_BTSNOOP) {
And here you broke a style that was fine before.
> btsnoop_version = 1;
> btsnoop_type = 1002;
>
> @@ -895,6 +922,7 @@ static void usage(void)
> " -a, --ascii Dump data in ascii\n"
> " -x, --hex Dump data in hex\n"
> " -X, --ext Dump data in hex and ascii\n"
> + " -T, --tilogger=file Dump TI hci log data to file\n"
> " -R, --raw Dump raw data\n"
> " -C, --cmtp=psm PSM for CMTP\n"
> " -H, --hcrp=psm PSM for HCRP\n"
> @@ -936,6 +964,7 @@ static struct option main_options[] = {
> { "nopermcheck", 0, 0, 'Z' },
> { "ipv4", 0, 0, '4' },
> { "ipv6", 0, 0, '6' },
> + { "tilogger", 1, 0, 'T' },
> { "help", 0, 0, 'h' },
> { "version", 0, 0, 'v' },
> { 0 }
> @@ -952,7 +981,7 @@ int main(int argc, char *argv[])
> uint16_t obex_port;
>
> while ((opt = getopt_long(argc, argv,
> - "i:l:p:m:w:r:d:taxXRC:H:O:P:S:D:A:YZ46hv",
> + "i:l:p:m:w:r:d:taxT:XRC:H:O:P:S:D:A:YZ46hv",
> main_options, NULL)) != -1) {
> switch(opt) {
> case 'i':
> @@ -1009,6 +1038,12 @@ int main(int argc, char *argv[])
> flags |= DUMP_RAW;
> break;
>
> + case 'T':
> + mode = WRITE;
> + dump_file = strdup(optarg);
> + flags |= DUMP_TILOGGER;
> + break;
> +
> case 'C':
> set_proto(0, atoi(optarg), 0, SDP_UUID_CMTP);
> break;
> @@ -1101,7 +1136,8 @@ int main(int argc, char *argv[])
> break;
>
> case WRITE:
> - flags |= DUMP_BTSNOOP;
> + if ((flags & DUMP_TILOGGER) == 0)
> + flags |= DUMP_BTSNOOP;
There are all else if check. So why is needed? You might better use a
tilogger_file instead using the global file.
> process_frames(device, open_socket(device, flags),
> open_file(dump_file, mode, flags), flags);
> break;
Regards
Marcel
^ permalink raw reply
* Re: [PATCH BlueZ 3/6] gdbus: Introduce GDBusInterfaceFlags
From: Marcel Holtmann @ 2012-12-23 16:09 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <1356276298-13795-3-git-send-email-luiz.dentz@gmail.com>
Hi Luiz,
> The flags should be passed in g_dbus_register_interface_with_flags,
> currently there only one flag which is G_DBUS_INTERFACE_FLAG_EXPERIMENTAL
> which works similarly to G_DBUS_METHOD_FLAG_EXPERIMENTAL but for the
> whole interface.
> ---
> gdbus/gdbus.h | 13 +++++++++++++
> gdbus/object.c | 20 ++++++++++++++++++++
> 2 files changed, 33 insertions(+)
can we not just check if the interface contains any non experimental
method, signals and properties and base that decision around that?
I am not really in favor of introducing a version with flags.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH BlueZ 4/6] gdbus: Call check_signals when sending signals with g_dbus_send_message
From: Marcel Holtmann @ 2012-12-23 16:10 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <1356276298-13795-4-git-send-email-luiz.dentz@gmail.com>
Hi Luiz,
> If message passed to g_dbus_send_message is a signal verify if it is a
> valid and there really exists an interface with respective signal name.
> ---
> gdbus/object.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/gdbus/object.c b/gdbus/object.c
> index 510763a..6951171 100644
> --- a/gdbus/object.c
> +++ b/gdbus/object.c
> @@ -1475,6 +1475,15 @@ gboolean g_dbus_send_message(DBusConnection *connection, DBusMessage *message)
>
> if (dbus_message_get_type(message) == DBUS_MESSAGE_TYPE_METHOD_CALL)
> dbus_message_set_no_reply(message, TRUE);
> + else {
> + const char *path = dbus_message_get_path(message);
> + const char *interface = dbus_message_get_interface(message);
> + const char *name = dbus_message_get_member(message);
> + const GDBusArgInfo *args;
> +
> + if (!check_signal(connection, path, interface, name, &args))
> + return FALSE;
> + }
we have more than just method calls and signals. This could be also used
used for sending async method returns.
>
> result = dbus_connection_send(connection, message, NULL);
>
Regards
Marcel
^ permalink raw reply
* Re: [PATCH BlueZ 3/6] gdbus: Introduce GDBusInterfaceFlags
From: Luiz Augusto von Dentz @ 2012-12-23 16:27 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <1356278946.29264.24.camel@aeonflux>
Hi Marcel,
On Sun, Dec 23, 2012 at 6:09 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Luiz,
>
>> The flags should be passed in g_dbus_register_interface_with_flags,
>> currently there only one flag which is G_DBUS_INTERFACE_FLAG_EXPERIMENTAL
>> which works similarly to G_DBUS_METHOD_FLAG_EXPERIMENTAL but for the
>> whole interface.
>> ---
>> gdbus/gdbus.h | 13 +++++++++++++
>> gdbus/object.c | 20 ++++++++++++++++++++
>> 2 files changed, 33 insertions(+)
>
> can we not just check if the interface contains any non experimental
> method, signals and properties and base that decision around that?
It can work, but we would have to iterate on every single item, so in
terms of code is quite a bit more complex so I favored adding a new
function even though we might replace it in the long run.
> I am not really in favor of introducing a version with flags.
The final solution I was thinking is passing another interface
containing the table as this tend to be quite static e.g.:
struct GDBusInteface {
const char *name;
GDBusInterfaceFlags flags;
const GDBusMethodTable *methods;
const GDBusSignalTable *signals;
const GDBusPropertiesTable *properties;
};
Obviously this would break g_dbus_register_interface and generate
quite a bit more changes to the daemon code.
--
Luiz Augusto von Dentz
^ permalink raw reply
* Re: [PATCH BlueZ 3/6] gdbus: Introduce GDBusInterfaceFlags
From: Marcel Holtmann @ 2012-12-23 16:30 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <CABBYNZ+set-vvzf+6whfPt5zE8c7X_t2DB4vJS6p8v2X0i_U-A@mail.gmail.com>
Hi Luiz,
> >> The flags should be passed in g_dbus_register_interface_with_flags,
> >> currently there only one flag which is G_DBUS_INTERFACE_FLAG_EXPERIMENTAL
> >> which works similarly to G_DBUS_METHOD_FLAG_EXPERIMENTAL but for the
> >> whole interface.
> >> ---
> >> gdbus/gdbus.h | 13 +++++++++++++
> >> gdbus/object.c | 20 ++++++++++++++++++++
> >> 2 files changed, 33 insertions(+)
> >
> > can we not just check if the interface contains any non experimental
> > method, signals and properties and base that decision around that?
>
> It can work, but we would have to iterate on every single item, so in
> terms of code is quite a bit more complex so I favored adding a new
> function even though we might replace it in the long run.
we have to iterate over every single item in the success case anyway, so
why bother trying to optimize this.
> > I am not really in favor of introducing a version with flags.
>
> The final solution I was thinking is passing another interface
> containing the table as this tend to be quite static e.g.:
If this is not the final solution you are heading for, then lets not
even bother introducing it. Lets go with the the simple approach that
does not introduce any new functions.
It has the advantage that you only need to change one place. Otherwise
you need to check the method structs and the calling register function.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH BlueZ 4/6] gdbus: Call check_signals when sending signals with g_dbus_send_message
From: Luiz Augusto von Dentz @ 2012-12-23 16:31 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <1356279010.29264.25.camel@aeonflux>
Hi Marcel,
On Sun, Dec 23, 2012 at 6:10 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Luiz,
>
>> If message passed to g_dbus_send_message is a signal verify if it is a
>> valid and there really exists an interface with respective signal name.
>> ---
>> gdbus/object.c | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/gdbus/object.c b/gdbus/object.c
>> index 510763a..6951171 100644
>> --- a/gdbus/object.c
>> +++ b/gdbus/object.c
>> @@ -1475,6 +1475,15 @@ gboolean g_dbus_send_message(DBusConnection *connection, DBusMessage *message)
>>
>> if (dbus_message_get_type(message) == DBUS_MESSAGE_TYPE_METHOD_CALL)
>> dbus_message_set_no_reply(message, TRUE);
>> + else {
>> + const char *path = dbus_message_get_path(message);
>> + const char *interface = dbus_message_get_interface(message);
>> + const char *name = dbus_message_get_member(message);
>> + const GDBusArgInfo *args;
>> +
>> + if (!check_signal(connection, path, interface, name, &args))
>> + return FALSE;
>> + }
>
> we have more than just method calls and signals. This could be also used
> used for sending async method returns.
I guess I should do else if and check for signal type then, or you
have a better idea?
--
Luiz Augusto von Dentz
^ permalink raw reply
* Re: [PATCH BlueZ 4/6] gdbus: Call check_signals when sending signals with g_dbus_send_message
From: Marcel Holtmann @ 2012-12-23 16:40 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <CABBYNZJ=cpoaSOk4fvk3Y7pp=vtYyN3Qqwacq6iG6UHBicM5-g@mail.gmail.com>
Hi Luiz,
> >> If message passed to g_dbus_send_message is a signal verify if it is a
> >> valid and there really exists an interface with respective signal name.
> >> ---
> >> gdbus/object.c | 9 +++++++++
> >> 1 file changed, 9 insertions(+)
> >>
> >> diff --git a/gdbus/object.c b/gdbus/object.c
> >> index 510763a..6951171 100644
> >> --- a/gdbus/object.c
> >> +++ b/gdbus/object.c
> >> @@ -1475,6 +1475,15 @@ gboolean g_dbus_send_message(DBusConnection *connection, DBusMessage *message)
> >>
> >> if (dbus_message_get_type(message) == DBUS_MESSAGE_TYPE_METHOD_CALL)
> >> dbus_message_set_no_reply(message, TRUE);
> >> + else {
> >> + const char *path = dbus_message_get_path(message);
> >> + const char *interface = dbus_message_get_interface(message);
> >> + const char *name = dbus_message_get_member(message);
> >> + const GDBusArgInfo *args;
> >> +
> >> + if (!check_signal(connection, path, interface, name, &args))
> >> + return FALSE;
> >> + }
> >
> > we have more than just method calls and signals. This could be also used
> > used for sending async method returns.
>
> I guess I should do else if and check for signal type then, or you
> have a better idea?
just do the else if part here. You could use a switch statement, but
then you can not have local variables. So it does not really help.
I am still wondering why we are setting no reply flag when this is a
method call. That makes no sense to me. Must be missing something.
Regards
Marcel
^ permalink raw reply
* [PATCH BlueZ 1/8] gdbus: Introduce G_DBUS_METHOD_FLAG_EXPERIMENTAL
From: Luiz Augusto von Dentz @ 2012-12-23 20:15 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
This flag can be used to mark methods as experimental, the marked
methods with this flag can be enabled by setting the environment variable
GDBUS_EXPERIMENTAL=1
---
gdbus/gdbus.h | 21 ++++++++++++++++++---
gdbus/object.c | 17 +++++++++++++++++
2 files changed, 35 insertions(+), 3 deletions(-)
diff --git a/gdbus/gdbus.h b/gdbus/gdbus.h
index 0e5c012..00fbb1c 100644
--- a/gdbus/gdbus.h
+++ b/gdbus/gdbus.h
@@ -89,9 +89,10 @@ typedef void (* GDBusSecurityFunction) (DBusConnection *connection,
GDBusPendingReply pending);
enum GDBusMethodFlags {
- G_DBUS_METHOD_FLAG_DEPRECATED = (1 << 0),
- G_DBUS_METHOD_FLAG_NOREPLY = (1 << 1),
- G_DBUS_METHOD_FLAG_ASYNC = (1 << 2),
+ G_DBUS_METHOD_FLAG_DEPRECATED = (1 << 0),
+ G_DBUS_METHOD_FLAG_NOREPLY = (1 << 1),
+ G_DBUS_METHOD_FLAG_ASYNC = (1 << 2),
+ G_DBUS_METHOD_FLAG_EXPERIMENTAL = (1 << 3),
};
enum GDBusSignalFlags {
@@ -173,6 +174,20 @@ struct GDBusSecurityTable {
.function = _function, \
.flags = G_DBUS_METHOD_FLAG_ASYNC | G_DBUS_METHOD_FLAG_DEPRECATED
+#define GDBUS_EXPERIMENTAL_METHOD(_name, _in_args, _out_args, _function) \
+ .name = _name, \
+ .in_args = _in_args, \
+ .out_args = _out_args, \
+ .function = _function, \
+ .flags = G_DBUS_METHOD_FLAG_EXPERIMENTAL
+
+#define GDBUS_EXPERIMENTAL_ASYNC_METHOD(_name, _in_args, _out_args, _function) \
+ .name = _name, \
+ .in_args = _in_args, \
+ .out_args = _out_args, \
+ .function = _function, \
+ .flags = G_DBUS_METHOD_FLAG_ASYNC | G_DBUS_METHOD_FLAG_EXPERIMENTAL
+
#define GDBUS_NOREPLY_METHOD(_name, _in_args, _out_args, _function) \
.name = _name, \
.in_args = _in_args, \
diff --git a/gdbus/object.c b/gdbus/object.c
index 776d35e..30dbbc2 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -129,6 +129,14 @@ static void generate_interface_xml(GString *gstr, struct interface_data *iface)
G_DBUS_METHOD_FLAG_DEPRECATED;
gboolean noreply = method->flags &
G_DBUS_METHOD_FLAG_NOREPLY;
+ gboolean experimental = method->flags &
+ G_DBUS_METHOD_FLAG_EXPERIMENTAL;
+
+ if (experimental) {
+ const char *env = g_getenv("GDBUS_EXPERIMENTAL");
+ if (g_strcmp0(env, "1") != 0)
+ continue;
+ }
if (!deprecated && !noreply &&
!(method->in_args && method->in_args->name) &&
@@ -1022,10 +1030,19 @@ static DBusHandlerResult generic_message(DBusConnection *connection,
for (method = iface->methods; method &&
method->name && method->function; method++) {
+ gboolean experimental = method->flags &
+ G_DBUS_METHOD_FLAG_EXPERIMENTAL;
+
if (dbus_message_is_method_call(message, iface->name,
method->name) == FALSE)
continue;
+ if (experimental) {
+ const char *env = g_getenv("GDBUS_EXPERIMENTAL");
+ if (g_strcmp0(env, "1") != 0)
+ return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
+ }
+
if (g_dbus_args_have_signature(method->in_args,
message) == FALSE)
continue;
--
1.7.11.7
^ permalink raw reply related
* [PATCH BlueZ 2/8] gdbus: Introduce G_DBUS_SIGNAL_FLAG_EXPERIMENTAL
From: Luiz Augusto von Dentz @ 2012-12-23 20:15 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1356293718-9348-1-git-send-email-luiz.dentz@gmail.com>
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
This flag can be used to mark signals as experimental, the marked
signals with this flag can be enabled by setting the environment variable
GDBUS_EXPERIMENTAL=1
---
gdbus/gdbus.h | 8 +++++++-
gdbus/object.c | 21 ++++++++++++++++++---
2 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/gdbus/gdbus.h b/gdbus/gdbus.h
index 00fbb1c..e6b51cd 100644
--- a/gdbus/gdbus.h
+++ b/gdbus/gdbus.h
@@ -96,7 +96,8 @@ enum GDBusMethodFlags {
};
enum GDBusSignalFlags {
- G_DBUS_SIGNAL_FLAG_DEPRECATED = (1 << 0),
+ G_DBUS_SIGNAL_FLAG_DEPRECATED = (1 << 0),
+ G_DBUS_SIGNAL_FLAG_EXPERIMENTAL = (1 << 1),
};
enum GDBusPropertyFlags {
@@ -204,6 +205,11 @@ struct GDBusSecurityTable {
.args = _args, \
.flags = G_DBUS_SIGNAL_FLAG_DEPRECATED
+#define GDBUS_EXPERIMENTAL_SIGNAL(_name, _args) \
+ .name = _name, \
+ .args = _args, \
+ .flags = G_DBUS_SIGNAL_FLAG_EXPERIMENTAL
+
gboolean g_dbus_register_interface(DBusConnection *connection,
const char *path, const char *name,
const GDBusMethodTable *methods,
diff --git a/gdbus/object.c b/gdbus/object.c
index 30dbbc2..20f7db9 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -165,6 +165,14 @@ static void generate_interface_xml(GString *gstr, struct interface_data *iface)
for (signal = iface->signals; signal && signal->name; signal++) {
gboolean deprecated = signal->flags &
G_DBUS_SIGNAL_FLAG_DEPRECATED;
+ gboolean experimental = signal->flags &
+ G_DBUS_SIGNAL_FLAG_EXPERIMENTAL;
+
+ if (experimental) {
+ const char *env = g_getenv("GDBUS_EXPERIMENTAL");
+ if (g_strcmp0(env, "1") != 0)
+ continue;
+ }
if (!deprecated && !(signal->args && signal->args->name))
g_string_append_printf(gstr,
@@ -1267,10 +1275,17 @@ static gboolean check_signal(DBusConnection *conn, const char *path,
}
for (signal = iface->signals; signal && signal->name; signal++) {
- if (!strcmp(signal->name, name)) {
- *args = signal->args;
- return TRUE;
+ if (strcmp(signal->name, name) != 0)
+ continue;
+
+ if (signal->flags & G_DBUS_SIGNAL_FLAG_EXPERIMENTAL) {
+ const char *env = g_getenv("GDBUS_EXPERIMENTAL");
+ if (g_strcmp0(env, "1") != 0)
+ break;
}
+
+ *args = signal->args;
+ return TRUE;
}
error("No signal named %s on interface %s", name, interface);
--
1.7.11.7
^ permalink raw reply related
* [PATCH BlueZ 3/8] gdbus: Introduce G_DBUS_PROPERTY_FLAG_EXPERIMENTAL
From: Luiz Augusto von Dentz @ 2012-12-23 20:15 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1356293718-9348-1-git-send-email-luiz.dentz@gmail.com>
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
This flag can be used to mark properties as experimental, the marked
properties with this flag can be enabled by setting the environment variable
GDBUS_EXPERIMENTAL=1
---
gdbus/gdbus.h | 3 ++-
gdbus/object.c | 60 ++++++++++++++++++++++++++++++++++++----------------------
2 files changed, 39 insertions(+), 24 deletions(-)
diff --git a/gdbus/gdbus.h b/gdbus/gdbus.h
index e6b51cd..da3cd53 100644
--- a/gdbus/gdbus.h
+++ b/gdbus/gdbus.h
@@ -101,7 +101,8 @@ enum GDBusSignalFlags {
};
enum GDBusPropertyFlags {
- G_DBUS_PROPERTY_FLAG_DEPRECATED = (1 << 0),
+ G_DBUS_PROPERTY_FLAG_DEPRECATED = (1 << 0),
+ G_DBUS_PROPERTY_FLAG_EXPERIMENTAL = (1 << 1),
};
enum GDBusSecurityFlags {
diff --git a/gdbus/object.c b/gdbus/object.c
index 20f7db9..7b58eea 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -118,6 +118,18 @@ static void print_arguments(GString *gstr, const GDBusArgInfo *args,
#define G_DBUS_ANNOTATE_NOREPLY(prefix_) \
G_DBUS_ANNOTATE(prefix_, "Method.NoReply", "true")
+static gboolean check_experimental(int flags, int flag)
+{
+ const char *env;
+
+ if (!(flags & flag))
+ return FALSE;
+
+ env = g_getenv("GDBUS_EXPERIMENTAL");
+
+ return g_strcmp0(env, "1") != 0;
+}
+
static void generate_interface_xml(GString *gstr, struct interface_data *iface)
{
const GDBusMethodTable *method;
@@ -129,14 +141,10 @@ static void generate_interface_xml(GString *gstr, struct interface_data *iface)
G_DBUS_METHOD_FLAG_DEPRECATED;
gboolean noreply = method->flags &
G_DBUS_METHOD_FLAG_NOREPLY;
- gboolean experimental = method->flags &
- G_DBUS_METHOD_FLAG_EXPERIMENTAL;
- if (experimental) {
- const char *env = g_getenv("GDBUS_EXPERIMENTAL");
- if (g_strcmp0(env, "1") != 0)
- continue;
- }
+ if (check_experimental(method->flags,
+ G_DBUS_METHOD_FLAG_EXPERIMENTAL))
+ continue;
if (!deprecated && !noreply &&
!(method->in_args && method->in_args->name) &&
@@ -165,14 +173,10 @@ static void generate_interface_xml(GString *gstr, struct interface_data *iface)
for (signal = iface->signals; signal && signal->name; signal++) {
gboolean deprecated = signal->flags &
G_DBUS_SIGNAL_FLAG_DEPRECATED;
- gboolean experimental = signal->flags &
- G_DBUS_SIGNAL_FLAG_EXPERIMENTAL;
- if (experimental) {
- const char *env = g_getenv("GDBUS_EXPERIMENTAL");
- if (g_strcmp0(env, "1") != 0)
- continue;
- }
+ if (check_experimental(signal->flags,
+ G_DBUS_SIGNAL_FLAG_EXPERIMENTAL))
+ continue;
if (!deprecated && !(signal->args && signal->args->name))
g_string_append_printf(gstr,
@@ -197,6 +201,10 @@ static void generate_interface_xml(GString *gstr, struct interface_data *iface)
gboolean deprecated = property->flags &
G_DBUS_PROPERTY_FLAG_DEPRECATED;
+ if (check_experimental(property->flags,
+ G_DBUS_PROPERTY_FLAG_EXPERIMENTAL))
+ continue;
+
g_string_append_printf(gstr, "\t\t<property name=\"%s\""
" type=\"%s\" access=\"%s%s\"",
property->name, property->type,
@@ -558,6 +566,10 @@ static void append_properties(struct interface_data *data,
DBUS_DICT_ENTRY_END_CHAR_AS_STRING, &dict);
for (p = data->properties; p && p->name; p++) {
+ if (check_experimental(p->flags,
+ G_DBUS_PROPERTY_FLAG_EXPERIMENTAL))
+ continue;
+
if (p->get == NULL)
continue;
@@ -759,8 +771,14 @@ static inline const GDBusPropertyTable *find_property(const GDBusPropertyTable *
const GDBusPropertyTable *p;
for (p = properties; p && p->name; p++) {
- if (strcmp(name, p->name) == 0)
- return p;
+ if (strcmp(name, p->name) != 0)
+ continue;
+
+ if (check_experimental(p->flags,
+ G_DBUS_PROPERTY_FLAG_EXPERIMENTAL))
+ break;
+
+ return p;
}
return NULL;
@@ -1038,18 +1056,14 @@ static DBusHandlerResult generic_message(DBusConnection *connection,
for (method = iface->methods; method &&
method->name && method->function; method++) {
- gboolean experimental = method->flags &
- G_DBUS_METHOD_FLAG_EXPERIMENTAL;
if (dbus_message_is_method_call(message, iface->name,
method->name) == FALSE)
continue;
- if (experimental) {
- const char *env = g_getenv("GDBUS_EXPERIMENTAL");
- if (g_strcmp0(env, "1") != 0)
- return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
- }
+ if (check_experimental(method->flags,
+ G_DBUS_METHOD_FLAG_EXPERIMENTAL))
+ return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
if (g_dbus_args_have_signature(method->in_args,
message) == FALSE)
--
1.7.11.7
^ permalink raw reply related
* [PATCH BlueZ 4/8] gdbus: Check if the interface being registered is valid
From: Luiz Augusto von Dentz @ 2012-12-23 20:15 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1356293718-9348-1-git-send-email-luiz.dentz@gmail.com>
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
This prevent registering interfaces that are empty or have all members
marked as experiemental.
---
gdbus/object.c | 42 ++++++++++++++++++++++++++++++++++++------
1 file changed, 36 insertions(+), 6 deletions(-)
diff --git a/gdbus/object.c b/gdbus/object.c
index 7b58eea..d864bf5 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -1177,7 +1177,7 @@ static const GDBusSignalTable manager_signals[] = {
{ }
};
-static void add_interface(struct generic_data *data,
+static gboolean add_interface(struct generic_data *data,
const char *name,
const GDBusMethodTable *methods,
const GDBusSignalTable *signals,
@@ -1186,7 +1186,32 @@ static void add_interface(struct generic_data *data,
GDBusDestroyFunction destroy)
{
struct interface_data *iface;
+ const GDBusMethodTable *method;
+ const GDBusSignalTable *signal;
+ const GDBusPropertyTable *property;
+
+ for (method = methods; method && method->name; method++) {
+ if (!check_experimental(method->flags,
+ G_DBUS_METHOD_FLAG_EXPERIMENTAL))
+ goto done;
+ }
+
+ for (signal = signals; signal && signal->name; signal++) {
+ if (!check_experimental(signal->flags,
+ G_DBUS_SIGNAL_FLAG_EXPERIMENTAL))
+ goto done;
+ }
+
+ for (property = properties; property && property->name; property++) {
+ if (!check_experimental(property->flags,
+ G_DBUS_PROPERTY_FLAG_EXPERIMENTAL))
+ goto done;
+ }
+ /* Nothing to register */
+ return FALSE;
+
+done:
iface = g_new0(struct interface_data, 1);
iface->name = g_strdup(name);
iface->methods = methods;
@@ -1197,13 +1222,15 @@ static void add_interface(struct generic_data *data,
data->interfaces = g_slist_append(data->interfaces, iface);
if (data->parent == NULL)
- return;
+ return TRUE;
data->added = g_slist_append(data->added, iface);
if (data->process_id > 0)
- return;
+ return TRUE;
data->process_id = g_idle_add(process_changes, data);
+
+ return TRUE;
}
static struct generic_data *object_path_ref(DBusConnection *connection,
@@ -1364,15 +1391,18 @@ gboolean g_dbus_register_interface(DBusConnection *connection,
return FALSE;
}
+ if (!add_interface(data, name, methods, signals, properties, user_data,
+ destroy)) {
+ object_path_unref(connection, path);
+ return FALSE;
+ }
+
if (properties != NULL && !find_interface(data->interfaces,
DBUS_INTERFACE_PROPERTIES))
add_interface(data, DBUS_INTERFACE_PROPERTIES,
properties_methods, properties_signals, NULL,
data, NULL);
- add_interface(data, name, methods, signals, properties, user_data,
- destroy);
-
g_free(data->introspect);
data->introspect = NULL;
--
1.7.11.7
^ permalink raw reply related
* [PATCH BlueZ 5/8] gdbus: Call check_signals when sending signals with g_dbus_send_message
From: Luiz Augusto von Dentz @ 2012-12-23 20:15 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1356293718-9348-1-git-send-email-luiz.dentz@gmail.com>
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
If message passed to g_dbus_send_message is a signal verify if it is a
valid and there really exists an interface with respective signal name.
---
gdbus/object.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/gdbus/object.c b/gdbus/object.c
index d864bf5..565dd6f 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -1514,6 +1514,15 @@ gboolean g_dbus_send_message(DBusConnection *connection, DBusMessage *message)
if (dbus_message_get_type(message) == DBUS_MESSAGE_TYPE_METHOD_CALL)
dbus_message_set_no_reply(message, TRUE);
+ else if (dbus_message_get_type(message) == DBUS_MESSAGE_TYPE_SIGNAL) {
+ const char *path = dbus_message_get_path(message);
+ const char *interface = dbus_message_get_interface(message);
+ const char *name = dbus_message_get_member(message);
+ const GDBusArgInfo *args;
+
+ if (!check_signal(connection, path, interface, name, &args))
+ return FALSE;
+ }
result = dbus_connection_send(connection, message, NULL);
--
1.7.11.7
^ permalink raw reply related
* [PATCH BlueZ 6/8] media: Enable RegisterPlayer and UnregisterPlayer methods as experimental
From: Luiz Augusto von Dentz @ 2012-12-23 20:15 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1356293718-9348-1-git-send-email-luiz.dentz@gmail.com>
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
profiles/audio/media.c | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/profiles/audio/media.c b/profiles/audio/media.c
index f728460..e4206e3 100644
--- a/profiles/audio/media.c
+++ b/profiles/audio/media.c
@@ -868,7 +868,6 @@ static DBusMessage *unregister_endpoint(DBusConnection *conn, DBusMessage *msg,
return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
}
-#if 0
static struct media_player *media_adapter_find_player(
struct media_adapter *adapter,
const char *sender,
@@ -1533,7 +1532,6 @@ static DBusMessage *unregister_player(DBusConnection *conn, DBusMessage *msg,
return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
}
-#endif
static const GDBusMethodTable media_methods[] = {
{ GDBUS_METHOD("RegisterEndpoint",
@@ -1541,14 +1539,12 @@ static const GDBusMethodTable media_methods[] = {
NULL, register_endpoint) },
{ GDBUS_METHOD("UnregisterEndpoint",
GDBUS_ARGS({ "endpoint", "o" }), NULL, unregister_endpoint) },
-#if 0
- { GDBUS_METHOD("RegisterPlayer",
+ { GDBUS_EXPERIMENTAL_METHOD("RegisterPlayer",
GDBUS_ARGS({ "player", "o" }, { "properties", "a{sv}" },
{ "metadata", "a{sv}" }),
NULL, register_player) },
- { GDBUS_METHOD("UnregisterPlayer",
+ { GDBUS_EXPERIMENTAL_METHOD("UnregisterPlayer",
GDBUS_ARGS({ "player", "o" }), NULL, unregister_player) },
-#endif
{ },
};
@@ -1559,10 +1555,8 @@ static void path_free(void *data)
while (adapter->endpoints)
release_endpoint(adapter->endpoints->data);
-#if 0
while (adapter->players)
media_player_destroy(adapter->players->data);
-#endif
adapters = g_slist_remove(adapters, adapter);
--
1.7.11.7
^ permalink raw reply related
* [PATCH BlueZ 7/8] player: Enable MediaPlayer1 interface as experimental
From: Luiz Augusto von Dentz @ 2012-12-23 20:15 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1356293718-9348-1-git-send-email-luiz.dentz@gmail.com>
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
This enable MediaPlayer1 when GDBUS_EXPERIMENTAL=1
---
profiles/audio/player.c | 28 ++++++++++++++++------------
1 file changed, 16 insertions(+), 12 deletions(-)
diff --git a/profiles/audio/player.c b/profiles/audio/player.c
index 005d0d1..693a590 100644
--- a/profiles/audio/player.c
+++ b/profiles/audio/player.c
@@ -239,25 +239,31 @@ static void set_setting(const GDBusPropertyTable *property,
}
static const GDBusMethodTable media_player_methods[] = {
- { GDBUS_METHOD("GetTrack",
+ { GDBUS_EXPERIMENTAL_METHOD("GetTrack",
NULL, GDBUS_ARGS({ "metadata", "a{sv}" }),
media_player_get_track) },
{ }
};
static const GDBusSignalTable media_player_signals[] = {
- { GDBUS_SIGNAL("TrackChanged",
+ { GDBUS_EXPERIMENTAL_SIGNAL("TrackChanged",
GDBUS_ARGS({ "metadata", "a{sv}" })) },
{ }
};
static const GDBusPropertyTable media_player_properties[] = {
- { "Position", "u", get_position },
- { "Status", "s", get_status, NULL, status_exists },
- { "Equalizer", "s", get_setting, set_setting, setting_exists },
- { "Repeat", "s", get_setting, set_setting, setting_exists },
- { "Shuffle", "s", get_setting, set_setting, setting_exists },
- { "Scan", "s", get_setting, set_setting, setting_exists },
+ { "Position", "u", get_position, NULL, NULL,
+ G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
+ { "Status", "s", get_status, NULL, status_exists,
+ G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
+ { "Equalizer", "s", get_setting, set_setting, setting_exists,
+ G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
+ { "Repeat", "s", get_setting, set_setting, setting_exists,
+ G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
+ { "Shuffle", "s", get_setting, set_setting, setting_exists,
+ G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
+ { "Scan", "s", get_setting, set_setting, setting_exists,
+ G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
{ }
};
@@ -298,7 +304,6 @@ struct media_player *media_player_controller_create(const char *path)
g_free, g_free);
mp->progress = g_timer_new();
-#if 0
if (!g_dbus_register_interface(btd_get_dbus_connection(),
mp->path, MEDIA_PLAYER_INTERFACE,
media_player_methods,
@@ -308,7 +313,7 @@ struct media_player *media_player_controller_create(const char *path)
media_player_destroy(mp);
return NULL;
}
-#endif
+
DBG("%s", mp->path);
return mp;
@@ -410,7 +415,6 @@ void media_player_set_status(struct media_player *mp, const char *status)
static gboolean process_metadata_changed(void *user_data)
{
-#if 0
struct media_player *mp = user_data;
DBusMessage *signal;
DBusMessageIter iter, dict;
@@ -439,7 +443,7 @@ static gboolean process_metadata_changed(void *user_data)
dbus_message_iter_close_container(&iter, &dict);
g_dbus_send_message(btd_get_dbus_connection(), signal);
-#endif
+
return FALSE;
}
--
1.7.11.7
^ permalink raw reply related
* [PATCH BlueZ 8/8] AVRCP: Fix not checking for media_player_controller_create
From: Luiz Augusto von Dentz @ 2012-12-23 20:15 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1356293718-9348-1-git-send-email-luiz.dentz@gmail.com>
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Now that the MediaPlayer1 interface is experimental the interface
registration may fail which return NULL, in that case there is no
point on register to any notifications.
---
profiles/audio/avrcp.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
index 4e3d31d..ce070cd 100644
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -1987,6 +1987,11 @@ static gboolean avrcp_get_capabilities_resp(struct avctp *conn,
count = pdu->params[1];
+ path = device_get_path(session->dev->btd_dev);
+ mp = media_player_controller_create(path);
+ if (mp == NULL)
+ return FALSE;
+
for (; count > 0; count--) {
uint8_t event = pdu->params[1 + count];
@@ -2001,8 +2006,6 @@ static gboolean avrcp_get_capabilities_resp(struct avctp *conn,
}
}
- path = device_get_path(session->dev->btd_dev);
- mp = media_player_controller_create(path);
media_player_set_callbacks(mp, &ct_cbs, player);
player->user_data = mp;
player->destroy = (GDestroyNotify) media_player_destroy;
--
1.7.11.7
^ permalink raw reply related
* Re: [PATCH BlueZ 1/8] gdbus: Introduce G_DBUS_METHOD_FLAG_EXPERIMENTAL
From: Marcel Holtmann @ 2012-12-23 20:58 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <1356293718-9348-1-git-send-email-luiz.dentz@gmail.com>
Hi Luiz,
> This flag can be used to mark methods as experimental, the marked
> methods with this flag can be enabled by setting the environment variable
> GDBUS_EXPERIMENTAL=1
> ---
> gdbus/gdbus.h | 21 ++++++++++++++++++---
> gdbus/object.c | 17 +++++++++++++++++
> 2 files changed, 35 insertions(+), 3 deletions(-)
>
> diff --git a/gdbus/gdbus.h b/gdbus/gdbus.h
> index 0e5c012..00fbb1c 100644
> --- a/gdbus/gdbus.h
> +++ b/gdbus/gdbus.h
> @@ -89,9 +89,10 @@ typedef void (* GDBusSecurityFunction) (DBusConnection *connection,
> GDBusPendingReply pending);
>
> enum GDBusMethodFlags {
> - G_DBUS_METHOD_FLAG_DEPRECATED = (1 << 0),
> - G_DBUS_METHOD_FLAG_NOREPLY = (1 << 1),
> - G_DBUS_METHOD_FLAG_ASYNC = (1 << 2),
> + G_DBUS_METHOD_FLAG_DEPRECATED = (1 << 0),
> + G_DBUS_METHOD_FLAG_NOREPLY = (1 << 1),
> + G_DBUS_METHOD_FLAG_ASYNC = (1 << 2),
> + G_DBUS_METHOD_FLAG_EXPERIMENTAL = (1 << 3),
> };
>
> enum GDBusSignalFlags {
> @@ -173,6 +174,20 @@ struct GDBusSecurityTable {
> .function = _function, \
> .flags = G_DBUS_METHOD_FLAG_ASYNC | G_DBUS_METHOD_FLAG_DEPRECATED
>
> +#define GDBUS_EXPERIMENTAL_METHOD(_name, _in_args, _out_args, _function) \
> + .name = _name, \
> + .in_args = _in_args, \
> + .out_args = _out_args, \
> + .function = _function, \
> + .flags = G_DBUS_METHOD_FLAG_EXPERIMENTAL
> +
> +#define GDBUS_EXPERIMENTAL_ASYNC_METHOD(_name, _in_args, _out_args, _function) \
> + .name = _name, \
> + .in_args = _in_args, \
> + .out_args = _out_args, \
> + .function = _function, \
> + .flags = G_DBUS_METHOD_FLAG_ASYNC | G_DBUS_METHOD_FLAG_EXPERIMENTAL
> +
> #define GDBUS_NOREPLY_METHOD(_name, _in_args, _out_args, _function) \
> .name = _name, \
> .in_args = _in_args, \
> diff --git a/gdbus/object.c b/gdbus/object.c
> index 776d35e..30dbbc2 100644
> --- a/gdbus/object.c
> +++ b/gdbus/object.c
> @@ -129,6 +129,14 @@ static void generate_interface_xml(GString *gstr, struct interface_data *iface)
> G_DBUS_METHOD_FLAG_DEPRECATED;
> gboolean noreply = method->flags &
> G_DBUS_METHOD_FLAG_NOREPLY;
> + gboolean experimental = method->flags &
> + G_DBUS_METHOD_FLAG_EXPERIMENTAL;
> +
> + if (experimental) {
> + const char *env = g_getenv("GDBUS_EXPERIMENTAL");
> + if (g_strcmp0(env, "1") != 0)
> + continue;
> + }
actually since this is a library, doing it this way is a bad idea.
Lets do something like g_dbus_enable_experimental(DBusConnection)
Regards
Marcel
^ permalink raw reply
* Re: [RFC/RFT] rtk_btusb: Bluetooth driver for Realtek RTL8723AE combo device
From: Oliver Neukum @ 2012-12-23 21:00 UTC (permalink / raw)
To: Larry Finger
Cc: linville, Marcel Holtmann, Gustavo Padovan, Johan Hedberg,
linux-wireless, linux-bluetooth, Champion Chen
In-Reply-To: <1356058371-17152-1-git-send-email-Larry.Finger@lwfinger.net>
On Thursday 20 December 2012 20:52:51 Larry Finger wrote:
> This new driver works with the RTL8723AE wireless/BT combo device. The
> corresponding firmware has been submitted to linux-firmware.
>
> Signed-off-by: Champion Chen <champion_chen@realsil.com.cn>
> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
> ---
> drivers/bluetooth/Kconfig | 10 +
> drivers/bluetooth/Makefile | 1 +
> drivers/bluetooth/rtk_btusb.c | 1649 +++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 1660 insertions(+)
> create mode 100644 drivers/bluetooth/rtk_btusb.c
>
> diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
> index e9f203e..efd3766 100644
> --- a/drivers/bluetooth/Kconfig
> +++ b/drivers/bluetooth/Kconfig
> @@ -241,4 +241,14 @@ config BT_WILINK
>
> Say Y here to compile support for Texas Instrument's WiLink7 driver
> into the kernel or say M to compile it as module.
> +
> +config BT_RTKUSB
> + tristate "Realtek BT driver for RTL8723AE"
> + select FW_LOADER
> + help
> + This enables the Bluetooth driver for the Realtek RTL8723AE Wifi/BT
> + combo device.
> +
> + Say Y here to compile support for these devices into the kernel
> + or say M to build it as a module.
> endmenu
> diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
> index 4afae20..167ccc0 100644
> --- a/drivers/bluetooth/Makefile
> +++ b/drivers/bluetooth/Makefile
> @@ -19,6 +19,7 @@ obj-$(CONFIG_BT_ATH3K) += ath3k.o
> obj-$(CONFIG_BT_MRVL) += btmrvl.o
> obj-$(CONFIG_BT_MRVL_SDIO) += btmrvl_sdio.o
> obj-$(CONFIG_BT_WILINK) += btwilink.o
> +obj-$(CONFIG_BT_RTKUSB) += rtk_btusb.o
>
> btmrvl-y := btmrvl_main.o
> btmrvl-$(CONFIG_DEBUG_FS) += btmrvl_debugfs.o
> diff --git a/drivers/bluetooth/rtk_btusb.c b/drivers/bluetooth/rtk_btusb.c
> new file mode 100644
> index 0000000..31c128a
> --- /dev/null
> +++ b/drivers/bluetooth/rtk_btusb.c
> @@ -0,0 +1,1650 @@
> +/*
> + *
> + * Realtek Bluetooth USB driver
> + *
> + * Copyright (C) 2012-2015 Edward Bian <edward_bian@realsil.com.cn>
> + *
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> + *
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/slab.h>
> +#include <linux/types.h>
> +#include <linux/sched.h>
> +#include <linux/errno.h>
> +#include <linux/skbuff.h>
> +#include <linux/usb.h>
> +#include <linux/completion.h>
> +#include <net/bluetooth/bluetooth.h>
> +#include <net/bluetooth/hci_core.h>
> +
> +#define VERSION "0.8"
> +
> +static struct usb_driv> diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
> index e9f203e..efd3766 100644
> +static struct usb_driver btusb_driver;
> +#if 1
> +#define RTKBT_DBG(fmt, arg...) pr_info("rtk_btusb: " fmt "\n" , ## arg)
No new debug macros please.
> +
> +static int btusb_open(struct hci_dev *hdev)
> +{
> + struct btusb_data *data = GET_DRV_DATA(hdev);
> + int err;
> +
> + err = usb_autopm_get_interface(data->intf);
> + if (err < 0)
> + return err;
> +
> + data->intf->needs_remote_wakeup = 1;
> + RTKBT_DBG("%s start pm_usage_cnt(0x%x)", __func__,
> + atomic_read(&(data->intf->pm_usage_cnt)));
> +
> + /*******************************/
> + if (0 == atomic_read(&hdev->promisc)) {
> + RTKBT_DBG("btusb_open hdev->promisc == 0");
> + err = -1;
This makes no sense
> + }
> + err = download_patch(data->intf);
> + if (err < 0)
> + goto failed;
On every open?
> + /*******************************/
> +
> + if (test_and_set_bit(HCI_RUNNING, &hdev->flags))
> + goto done;
> +
> + if (test_and_set_bit(BTUSB_INTR_RUNNING, &data->flags))
> + goto done;
> +
> + err = btusb_submit_intr_urb(hdev, GFP_KERNEL);
> + if (err < 0)
> + goto failed;
> +
> + err = btusb_submit_bulk_urb(hdev, GFP_KERNEL);
> + if (err < 0) {
> + mdelay(URB_CANCELING_DELAY_MS); /* Added by Realtek */
> + usb_kill_anchored_urbs(&data->intr_anchor);
> + goto failed;
> + }
> +
> + set_bit(BTUSB_BULK_RUNNING, &data->flags);
> + btusb_submit_bulk_urb(hdev, GFP_KERNEL);
> +
> +done:
> + usb_autopm_put_interface(data->intf);
> + RTKBT_DBG("%s end pm_usage_cnt(0x%x)", __func__,
> + atomic_read(&(data->intf->pm_usage_cnt)));
> +
> + return 0;
> +
> +failed:
> + clear_bit(BTUSB_INTR_RUNNING, &data->flags);
> + clear_bit(HCI_RUNNING, &hdev->flags);
> + usb_autopm_put_interface(data->intf);
> + RTKBT_DBG("%s failed pm_usage_cnt(0x%x)", __func__,
> + atomic_read(&(data->intf->pm_usage_cnt)));
> + return err;
> +}
> +
> +static void btusb_stop_traffic(struct btusb_data *data)
> +{
> + mdelay(URB_CANCELING_DELAY_MS); /* Added by Realtek */
> + usb_kill_anchored_urbs(&data->intr_anchor);
> + usb_kill_anchored_urbs(&data->bulk_anchor);
> + usb_kill_anchored_urbs(&data->isoc_anchor);
> +}
> +
> +static int btusb_close(struct hci_dev *hdev)
> +{
> + struct btusb_data *data = GET_DRV_DATA(hdev);
> + int i, err;
> +
> + if (!test_and_clear_bit(HCI_RUNNING, &hdev->flags))
> + return 0;
> +
> + RTKBT_DBG("btusb_close");
> + /*******************************/
> + for (i = 0; i < NUM_REASSEMBLY; i++) {
> + if (hdev->reassembly[i]) {
> + kfree_skb(hdev->reassembly[i]);
> + hdev->reassembly[i] = NULL;
> + RTKBT_DBG("%s free ressembly i =%d", __func__, i);
> + }
> + }
> + /*******************************/
> + cancel_work_sync(&data->work);
> + cancel_work_sync(&data->waker);
> +
> + clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
> + clear_bit(BTUSB_BULK_RUNNING, &data->flags);
> + clear_bit(BTUSB_INTR_RUNNING, &data->flags);
> +
> + btusb_stop_traffic(data);
> + err = usb_autopm_get_interface(data->intf);
> + if (err < 0)
> + goto failed;
> +
> + data->intf->needs_remote_wakeup = 0;
> + usb_autopm_put_interface(data->intf);
> +
> +failed:
> + mdelay(URB_CANCELING_DELAY_MS); /* Added by Realtek */
This makes no sense. Those URBs never went over the wire.
> + usb_scuttle_anchored_urbs(&data->deferred);
> + return 0;
> +}
> +
> +static int btusb_flush(struct hci_dev *hdev)
> +{
> + struct btusb_data *data = GET_DRV_DATA(hdev);
> +
> + RTKBT_DBG("%s add delay ", __func__);
> + mdelay(URB_CANCELING_DELAY_MS); /* Added by Realtek */
> + usb_kill_anchored_urbs(&data->tx_anchor);
> +
> + return 0;
> +}
> +
> +
> +static void btusb_work(struct work_struct *work)
> +{
> + struct btusb_data *data = container_of(work, struct btusb_data, work);
> + struct hci_dev *hdev = data->hdev;
> + int err;
> +
> + if (hdev->conn_hash.sco_num > 0) {
> + if (!test_bit(BTUSB_DID_ISO_RESUME, &data->flags)) {
> + err = usb_autopm_get_interface(data->isoc ? data->isoc :
> + data->intf);
> + if (err < 0) {
> + clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
> + /* Delay added by Realtek */
> + mdelay(URB_CANCELING_DELAY_MS);
> + usb_kill_anchored_urbs(&data->isoc_anchor);
> + return;
> + }
> +
> + set_bit(BTUSB_DID_ISO_RESUME, &data->flags);
> + }
> + if (data->isoc_altsetting != 2) {
> + clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
> + mdelay(URB_CANCELING_DELAY_MS); /* Added by Realtek */
> + usb_kill_anchored_urbs(&data->isoc_anchor);
> +
> + if (__set_isoc_interface(hdev, 2) < 0)
> + return;
> + }
> +
> + if (!test_and_set_bit(BTUSB_ISOC_RUNNING, &data->flags)) {
> + if (btusb_submit_isoc_urb(hdev, GFP_KERNEL) < 0)
> + clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
> + else
> + btusb_submit_isoc_urb(hdev, GFP_KERNEL);
> + }
> + } else {
> + clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
> + mdelay(URB_CANCELING_DELAY_MS); /* Added by Realtek */
> + usb_kill_anchored_urbs(&data->isoc_anchor);
> +
> + __set_isoc_interface(hdev, 0);
> + if (test_and_clear_bit(BTUSB_DID_ISO_RESUME, &data->flags))
> + usb_autopm_put_interface(data->isoc ? data->isoc :
> + data->intf);
> + }
> +}
> +static int btusb_probe(struct usb_interface *intf,
> + const struct usb_device_id *id)
> +{
> + struct usb_endpoint_descriptor *ep_desc;
> + struct btusb_data *data;
> + struct hci_dev *hdev;
> + int i, err, flag1, flag2;
> + struct usb_device *udev;
> + udev = interface_to_usbdev(intf);
> +
> + /* interface numbers are hardcoded in the spec */
> + if (intf->cur_altsetting->desc.bInterfaceNumber != 0)
> + return -ENODEV;
> +
> + /*******************************/
> + flag1 = device_can_wakeup(&udev->dev);
> + flag2 = device_may_wakeup(&udev->dev);
> + RTKBT_DBG("btusb_probe 1 ========== can_wakeup =%x flag2 =%x",
> + flag1, flag2);
> + device_wakeup_disable(&udev->dev);
Why?
> + flag1 = device_can_wakeup(&udev->dev);
> + flag2 = device_may_wakeup(&udev->dev);
> + RTKBT_DBG("wakeup_disable ========== can_wakeup =%x flag2 =%x",
> + flag1, flag2);
> + err = patch_add(intf);
> + if (err < 0)
> + return -1;
> + /*******************************/
> +
> + data = kzalloc(sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> +
> + for (i = 0; i < intf->cur_altsetting->desc.bNumEndpoints; i++) {
> + ep_desc = &intf->cur_altsetting->endpoint[i].desc;
> +
> + if (!data->intr_ep && usb_endpoint_is_int_in(ep_desc)) {
> + data->intr_ep = ep_desc;
> + continue;
> + }
> +
> + if (!data->bulk_tx_ep && usb_endpoint_is_bulk_out(ep_desc)) {
> + data->bulk_tx_ep = ep_desc;
> + continue;
> + }
> +
> + if (!data->bulk_rx_ep && usb_endpoint_is_bulk_in(ep_desc)) {
> + data->bulk_rx_ep = ep_desc;
> + continue;
> + }
> + }
> +
> + if (!data->intr_ep || !data->bulk_tx_ep || !data->bulk_rx_ep) {
> + kfree(data);
> + return -ENODEV;
> + }
> +
> + data->cmdreq_type = USB_TYPE_CLASS;
> +
> + data->udev = interface_to_usbdev(intf);
> + data->intf = intf;
> +
> + spin_lock_init(&data->lock);
> +
> + INIT_WORK(&data->work, btusb_work);
> + INIT_WORK(&data->waker, btusb_waker);
> + spin_lock_init(&data->txlock);
> +
> + init_usb_anchor(&data->tx_anchor);
> + init_usb_anchor(&data->intr_anchor);
> + init_usb_anchor(&data->bulk_anchor);
> + init_usb_anchor(&data->isoc_anchor);
> + init_usb_anchor(&data->deferred);
> +
> + hdev = hci_alloc_dev();
> + if (!hdev) {
> + kfree(data);
> + return -ENOMEM;
> + }
> +
> + HDEV_BUS = HCI_USB;
> +
> + data->hdev = hdev;
> +
> + SET_HCIDEV_DEV(hdev, &intf->dev);
> +
> + hdev->open = btusb_open;
> + hdev->close = btusb_close;
> + hdev->flush = btusb_flush;
> + hdev->send = btusb_send_frame;
> + hdev->notify = btusb_notify;
> +
> +
> + hci_set_drvdata(hdev, data);
> +
> + /* Interface numbers are hardcoded in the specification */
> + data->isoc = usb_ifnum_to_if(data->udev, 1);
> +
> + if (data->isoc) {
> + err = usb_driver_claim_interface(&btusb_driver,
> + data->isoc, data);
> + if (err < 0) {
> + hci_free_dev(hdev);
> + kfree(data);
> + return err;
> + }
> + }
> +
> + err = hci_register_dev(hdev);
> + if (err < 0) {
> + hci_free_dev(hdev);
> + kfree(data);
> + return err;
> + }
> +
> + usb_set_intfdata(intf, data);
> +
> + return 0;
> +}
> +
> +static void btusb_disconnect(struct usb_interface *intf)
> +{
> + struct btusb_data *data = usb_get_intfdata(intf);
> + struct hci_dev *hdev;
> + struct usb_device *udev;
> + udev = interface_to_usbdev(intf);
> +
> + if (intf->cur_altsetting->desc.bInterfaceNumber != 0)
> + return;
> +
> + if (!data)
> + return;
> +
> + RTKBT_DBG("btusb_disconnect");
> + /*******************************/
> + patch_remove(intf);
This is a race. The device is not dead at this time.
> + /*******************************/
> +
> + hdev = data->hdev;
> +
> + usb_set_intfdata(data->intf, NULL);
> +
> + if (data->isoc)
> + usb_set_intfdata(data->isoc, NULL);
> +
> + hci_unregister_dev(hdev);
> +
> + if (intf == data->isoc)
> + usb_driver_release_interface(&btusb_driver, data->intf);
> + else if (data->isoc)
> + usb_driver_release_interface(&btusb_driver, data->isoc);
> +
> + hci_free_dev(hdev);
> + kfree(data);
> +}
> +
> +#ifdef CONFIG_PM
> +static int btusb_suspend(struct usb_interface *intf, pm_message_t message)
> +{
> + struct btusb_data *data = usb_get_intfdata(intf);
> +
> + if (intf->cur_altsetting->desc.bInterfaceNumber != 0)
> + return 0;
> +
> + /*******************************/
> + RTKBT_DBG("btusb_suspend message.event = 0x%x, data->suspend_count =%d",
> + message.event, data->suspend_count);
> + if (!test_bit(HCI_RUNNING, &data->hdev->flags)) {
> + RTKBT_DBG("btusb_suspend-----bt is off");
> + set_btoff(data->intf);
Why repeat this if the device is already suspended?
> + }
> + /*******************************/
> +
> + if (data->suspend_count++)
> + return 0;
> +
> + spin_lock_irq(&data->txlock);
> + if (!((message.event & PM_EVENT_AUTO) && data->tx_in_flight)) {
> + set_bit(BTUSB_SUSPENDING, &data->flags);
> + spin_unlock_irq(&data->txlock);
> + } else {
> + spin_unlock_irq(&data->txlock);
> + data->suspend_count--;
> + return -EBUSY;
> + }
> +
> + cancel_work_sync(&data->work);
> +
> + btusb_stop_traffic(data);
> + mdelay(URB_CANCELING_DELAY_MS); /* Added by Realtek */
> + usb_kill_anchored_urbs(&data->tx_anchor);
> +
> + return 0;
> +}
> +
> +static void play_deferred(struct btusb_data *data)
> +{
> + struct urb *urb;
> + int err;
> +
> + while ((urb = usb_get_from_anchor(&data->deferred))) {
> +
> + /************************************/
> + usb_anchor_urb(urb, &data->tx_anchor);
> + err = usb_submit_urb(urb, GFP_ATOMIC);
> + if (err < 0) {
> + BT_ERR("play_deferred urb %p submission failed", urb);
> + kfree(urb->setup_packet);
> + usb_unanchor_urb(urb);
> + } else {
> + usb_mark_last_busy(data->udev);
> + }
> + usb_free_urb(urb);
> + /************************************/
> + data->tx_in_flight++;
> + }
> + mdelay(URB_CANCELING_DELAY_MS); /* Added by Realtek */
These URBs never went over the wire. Why a delay?
> + usb_scuttle_anchored_urbs(&data->deferred);
> +}
> +
> +static int btusb_resume(struct usb_interface *intf)
> +{
> + struct btusb_data *data = usb_get_intfdata(intf);
> + struct hci_dev *hdev = data->hdev;
> + int err = 0;
> +
> + if (intf->cur_altsetting->desc.bInterfaceNumber != 0)
> + return 0;
> +
> +
> + /*******************************/
> + RTKBT_DBG("btusb_resume data->suspend_count =%d", data->suspend_count);
> +
> + if (!test_bit(HCI_RUNNING, &hdev->flags)) {
> + RTKBT_DBG("btusb_resume-----bt is off, download patch");
> + download_patch(intf);
> + } else {
> + RTKBT_DBG("btusb_resume,----bt is on");
> + }
> + /*******************************/
> + if (--data->suspend_count)
> + return 0;
> +
> + if (test_bit(BTUSB_INTR_RUNNING, &data->flags)) {
> + err = btusb_submit_intr_urb(hdev, GFP_NOIO);
> + if (err < 0) {
> + clear_bit(BTUSB_INTR_RUNNING, &data->flags);
> + goto failed;
> + }
> + }
> +
> + if (test_bit(BTUSB_BULK_RUNNING, &data->flags)) {
> + err = btusb_submit_bulk_urb(hdev, GFP_NOIO);
> + if (err < 0) {
> + clear_bit(BTUSB_BULK_RUNNING, &data->flags);
> + goto failed;
> + }
> +
> + btusb_submit_bulk_urb(hdev, GFP_NOIO);
> + }
> +
> + if (test_bit(BTUSB_ISOC_RUNNING, &data->flags)) {
> + if (btusb_submit_isoc_urb(hdev, GFP_NOIO) < 0)
> + clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
> + else
> + btusb_submit_isoc_urb(hdev, GFP_NOIO);
> + }
> +
> + spin_lock_irq(&data->txlock);
> + play_deferred(data);
> + clear_bit(BTUSB_SUSPENDING, &data->flags);
> + spin_unlock_irq(&data->txlock);
> + schedule_work(&data->work);
> +
> + return 0;
> +
> +failed:
> + mdelay(URB_CANCELING_DELAY_MS); /* Added by Realtek */
Again, why?
> + usb_scuttle_anchored_urbs(&data->deferred);
> +/* done: */
> + spin_lock_irq(&data->txlock);
> + clear_bit(BTUSB_SUSPENDING, &data->flags);
> + spin_unlock_irq(&data->txlock);
> +
> + return err;
> +}
> +#endif
> +
> +static struct usb_driver btusb_driver = {
> + .name = "rtk_btusb",
> + .probe = btusb_probe,
> + .disconnect = btusb_disconnect,
> +#ifdef CONFIG_PM
> + .suspend = btusb_suspend,
> + .resume = btusb_resume,
> +#endif
> + .id_table = btusb_table,
> + .supports_autosuspend = 1,
> +};
> +
> +static int __init btusb_init(void)
> +{
> + RTKBT_DBG("Realtek Bluetooth USB driver ver %s", VERSION);
> +
> + return usb_register(&btusb_driver);
> +}
> +
> +static void __exit btusb_exit(void)
> +{
> + usb_deregister(&btusb_driver);
> +}
> +
> +module_init(btusb_init);
> +module_exit(btusb_exit);
> +
> +MODULE_AUTHOR("Edward Bian <edward_bian@realsil.com.cn>");
> +MODULE_DESCRIPTION("Realtek Bluetooth USB driver ver " VERSION);
> +MODULE_VERSION(VERSION);
> +MODULE_LICENSE("GPL");
> +MODULE_FIRMWARE("rtl_bt/rtl8723a.bin");
> +
> +/*******************************
> +** Reasil patch code
> +********************************/
> +
> +
> +#include <linux/firmware.h>
> +#include <linux/suspend.h>
> +#include <net/bluetooth/hci.h>
> +
> +
> +#define CMD_CMP_EVT 0x0e
> +#define PKT_LEN 300
> +#define MSG_TO 1000
> +#define PATCH_SEG_MAX 252
> +#define DATA_END 0x80
> +#define DOWNLOAD_OPCODE 0xfc20
> +#define BTOFF_OPCODE 0xfc28
> +#define TRUE 1
> +#define FALSE 0
> +#define CMD_HDR_LEN sizeof(struct hci_command_hdr)
> +#define EVT_HDR_LEN sizeof(struct hci_event_hdr)
> +#define CMD_CMP_LEN sizeof(struct hci_ev_cmd_complete)
> +
> +
> +enum rtk_endpoit {
> + CTRL_EP = 0,
> + INTR_EP = 1,
> + BULK_EP = 2,
> + ISOC_EP = 3
> +};
> +
> +struct patch_info {
> + uint16_t prod_id;
> + uint16_t lmp_sub;
> + char *patch_name;
> + char *config_name;
> + uint8_t *fw_cache;
> + int fw_len;
> +};
> +
> +struct xchange_data {
> + struct dev_data *dev_entry;
> + int pipe_in, pipe_out;
> + uint8_t send_pkt[PKT_LEN];
> + uint8_t rcv_pkt[PKT_LEN];
Violations of the DMA coherency rules, fails on non-x86
> + struct hci_command_hdr *cmd_hdr;
> + struct hci_event_hdr *evt_hdr;
> + struct hci_ev_cmd_complete *cmd_cmp;
> + uint8_t *req_para, *rsp_para;
> + uint8_t *fw_data;
> + int pkt_len, fw_len;
> +};
> +
> +struct dev_data {
> + struct list_head list_node;
> + struct usb_interface *intf;
> + struct usb_device *udev;
> + struct notifier_block pm_notifier;
> + struct patch_info *patch_entry;
> + struct xchange_data xdata;
> + struct completion firmware_loading_complete;
> + const struct firmware *fw;
> +};
> +
> +struct download_cp {
> + uint8_t index;
> + uint8_t data[PATCH_SEG_MAX];
DMA coherency
> +} __packed;
> +
> +struct download_rp {
> + uint8_t status;
> + uint8_t index;
> +} __packed;
> +
> +
> +static struct dev_data *dev_data_find(struct usb_interface *intf);
> +static struct patch_info *get_patch_entry(struct usb_device *udev);
> +static int rtkbt_pm_notify(struct notifier_block *notifier, ulong pm_event,
> + void *unused);
> +static int load_firmware(struct dev_data *dev_entry, uint8_t **buff);
> +static void init_xdata(struct xchange_data *xdata, struct dev_data *dev_entry);
> +static int check_fw_version(struct xchange_data *xdata);
> +static int get_firmware(struct xchange_data *xdata);
> +static int download_data(struct xchange_data *xdata);
> +static int send_hci_cmd(struct xchange_data *xdata);
> +static int rcv_hci_evt(struct xchange_data *xdata);
> +
> +
> +static struct patch_info patch_table[] = {
> + {0, 0x1200, "rtl_bt/rtl8723a.bin", "rtk8723_bt_config", NULL, 0}
> +};
> +
> +static LIST_HEAD(dev_data_list);
> +
> +
> +static int patch_add(struct usb_interface *intf)
> +{
> + struct dev_data *dev_entry;
> + struct usb_device *udev;
> +
> + RTKBT_DBG("patch_add");
> + dev_entry = dev_data_find(intf);
> + if (NULL != dev_entry)
> + return -1;
> +
> + udev = interface_to_usbdev(intf);
> +#if BTUSB_RPM
> + RTKBT_DBG("auto suspend is enabled");
> + usb_enable_autosuspend(udev);
> + pm_runtime_set_autosuspend_delay(&(udev->dev), 2000);
There is no good reason to overwrite the system values.
> +#endif
> +
> + dev_entry = kzalloc(sizeof(struct dev_data), GFP_KERNEL);
> + dev_entry->intf = intf;
> + dev_entry->udev = udev;
> + dev_entry->pm_notifier.notifier_call = rtkbt_pm_notify;
> + dev_entry->patch_entry = get_patch_entry(udev);
> + list_add(&dev_entry->list_node, &dev_data_list);
> + register_pm_notifier(&dev_entry->pm_notifier);
> +
> + return 0;
> +}
> +
> +static void patch_remove(struct usb_interface *intf)
> +{
> + struct dev_data *dev_entry;
> + struct usb_device *udev;
> +
> + udev = interface_to_usbdev(intf);
> +#if BTUSB_RPM
> + usb_disable_autosuspend(udev);
> +#endif
> +
> + dev_entry = dev_data_find(intf);
> + if (NULL == dev_entry)
> + return;
> +
> + RTKBT_DBG("patch_remove");
> + list_del(&dev_entry->list_node);
> + unregister_pm_notifier(&dev_entry->pm_notifier);
> + kfree(dev_entry);
> +}
> +
> +static int download_patch(struct usb_interface *intf)
> +{
> + struct dev_data *dev_entry;
> + uint8_t *fw_buf;
> + int ret_val;
> +
> + RTKBT_DBG("download_patch start");
> + dev_entry = dev_data_find(intf);
> + if (NULL == dev_entry) {
> + ret_val = -1;
> + RTKBT_DBG("NULL == dev_entry");
> + goto patch_end;
> + }
> +
> + init_xdata(&dev_entry->xdata, dev_entry);
> + ret_val = check_fw_version(&dev_entry->xdata);
> + if (ret_val != 0)
> + goto patch_end;
> +
> + ret_val = get_firmware(&dev_entry->xdata);
> + if (ret_val < 0) {
> + RTKBT_DBG("get_firmware failed!");
> + goto patch_end;
> + }
> + fw_buf = dev_entry->xdata.fw_data;
> +
> + ret_val = download_data(&dev_entry->xdata);
> + if (ret_val < 0) {
> + RTKBT_DBG("download_data failed!");
> + goto patch_fail;
> + }
> +
> + ret_val = check_fw_version(&dev_entry->xdata);
> + if (ret_val <= 0) {
> + ret_val = -1;
> + goto patch_fail;
> + }
> +
> + ret_val = 0;
> +patch_fail:
> + kfree(fw_buf);
> +patch_end:
> + RTKBT_DBG("Rtk patch end %d", ret_val);
> + return ret_val;
> +}
> +
> +static int set_btoff(struct usb_interface *intf)
> +{
> + struct dev_data *dev_entry;
> + int ret_val;
> +
> + RTKBT_DBG("set_btoff");
> + dev_entry = dev_data_find(intf);
> + if (NULL == dev_entry)
> + return -1;
> +
> + init_xdata(&dev_entry->xdata, dev_entry);
> + dev_entry->xdata.cmd_hdr->opcode = cpu_to_le16(BTOFF_OPCODE);
> + dev_entry->xdata.cmd_hdr->plen = 1;
> + dev_entry->xdata.pkt_len = CMD_HDR_LEN + 1;
> + dev_entry->xdata.send_pkt[CMD_HDR_LEN] = 1;
> +
> + ret_val = send_hci_cmd(&dev_entry->xdata);
> + if (ret_val < 0)
> + return ret_val;
> +
> + ret_val = rcv_hci_evt(&dev_entry->xdata);
> + if (ret_val < 0)
> + return ret_val;
> +
> + RTKBT_DBG("set_btoff done");
> + return 0;
> +}
> +
> +static struct dev_data *dev_data_find(struct usb_interface *intf)
> +{
> + struct dev_data *dev_entry;
> +
> + list_for_each_entry(dev_entry, &dev_data_list, list_node) {
> + if (dev_entry->intf == intf)
> + return dev_entry;
> + }
> +
> + return NULL;
> +}
> +
> +static struct patch_info *get_patch_entry(struct usb_device *udev)
> +{
> + struct patch_info *patch_entry;
> + uint16_t pid;
> +
> + patch_entry = patch_table;
> + pid = le16_to_cpu(udev->descriptor.idProduct);
> + while (pid != patch_entry->prod_id) {
> + if (0 == patch_entry->prod_id)
> + break;
> + patch_entry++;
> + }
> +
> + return patch_entry;
> +}
> +
> +static int rtkbt_pm_notify(struct notifier_block *notifier, ulong pm_event,
> + void *unused)
> +{
> + struct dev_data *dev_entry;
> + struct patch_info *patch_entry;
> + struct usb_device *udev;
> +
> + dev_entry = container_of(notifier, struct dev_data, pm_notifier);
> + patch_entry = dev_entry->patch_entry;
> + udev = dev_entry->udev;
> + RTKBT_DBG("rtkbt_pm_notify pm_event =%ld", pm_event);
> + switch (pm_event) {
> + case PM_SUSPEND_PREPARE:
> + case PM_HIBERNATION_PREPARE:
> + patch_entry->fw_len = load_firmware(dev_entry,
> + &patch_entry->fw_cache);
> + if (patch_entry->fw_len <= 0) {
> + RTKBT_DBG("rtkbt_pm_notify return NOTIFY_BAD");
> + return NOTIFY_BAD;
> + }
> +
> + if (!device_may_wakeup(&udev->dev)) {
> + dev_entry->intf->needs_binding = 1;
> + RTKBT_DBG("remote wakeup not support, set intf->needs_binding = 1");
> + }
> + break;
> +
> + case PM_POST_SUSPEND:
> + case PM_POST_HIBERNATION:
> + case PM_POST_RESTORE:
> + if (patch_entry->fw_len > 0) {
> + kfree(patch_entry->fw_cache);
> + patch_entry->fw_cache = NULL;
> + patch_entry->fw_len = 0;
> + }
> +#if BTUSB_RPM
> + usb_disable_autosuspend(udev);
> + usb_enable_autosuspend(udev);
> + pm_runtime_set_autosuspend_delay(&(udev->dev), 2000);
> +#endif
> + break;
> +
> + default:
> + break;
> + }
> +
> + return NOTIFY_DONE;
> +}
> +
> +static void bt_fw_cb(const struct firmware *firmware, void *context)
> +{
> + struct dev_data *dev_entry = context;
> +
> + dev_entry->fw = firmware;
> + if (!firmware)
> + pr_err("In callback routine, firmware file not available\n");
> + complete(&dev_entry->firmware_loading_complete);
> +}
> +
> +static int load_firmware(struct dev_data *dev_entry, uint8_t **buff)
> +{
> +#if LOAD_CONFIG
> + const struct firmware *fw;
> +#endif
> + struct usb_device *udev;
> + struct patch_info *patch_entry;
> + char *fw_name;
> + int fw_len = 0, ret_val;
> +
> + udev = dev_entry->udev;
> + init_completion(&dev_entry->firmware_loading_complete);
> + patch_entry = dev_entry->patch_entry;
> + fw_name = patch_entry->patch_name;
> + RTKBT_DBG("Reading firmware file %s", fw_name);
> + ret_val = request_firmware_nowait(THIS_MODULE, 1, fw_name, &udev->dev,
> + GFP_KERNEL, dev_entry, bt_fw_cb);
> + if (ret_val < 0)
> + goto fw_fail;
> +
> + wait_for_completion(&dev_entry->firmware_loading_complete);
> + if (!dev_entry->fw)
> + goto fw_fail;
> + *buff = kzalloc(dev_entry->fw->size, GFP_KERNEL);
> + if (NULL == *buff)
> + goto alloc_fail;
> + memcpy(*buff, dev_entry->fw->data, dev_entry->fw->size);
> + fw_len = dev_entry->fw->size;
> +
> +#if LOAD_CONFIG
> + release_firmware(dev_entry->fw);
> + fw_name = patch_entry->config_name;
> + ret_val = request_firmware(&fw, fw_name, &udev->dev);
> + if (ret_val < 0) {
> + fw_len = 0;
> + kfree(*buff);
> + *buff = NULL;
> + goto fw_fail;
> + }
> +
> + *buff = krealloc(*buff, fw_len + fw->size, GFP_KERNEL);
> + if (NULL == *buff) {
> + fw_len = 0;
> + release_firmware(fw);
> + goto fw_fail;
> + }
> + memcpy(*buff + fw_len, fw->data, fw->size);
> + fw_len += fw->size;
> +#endif
> +
> +alloc_fail:
> + release_firmware(dev_entry->fw);
> +fw_fail:
> + return fw_len;
> +}
> +
> +static void init_xdata(struct xchange_data *xdata, struct dev_data *dev_entry)
> +{
> + memset(xdata, 0, sizeof(struct xchange_data));
> + xdata->dev_entry = dev_entry;
> + xdata->pipe_in = usb_rcvintpipe(dev_entry->udev, INTR_EP);
> + xdata->pipe_out = usb_sndctrlpipe(dev_entry->udev, CTRL_EP);
> + xdata->cmd_hdr = (struct hci_command_hdr *)(xdata->send_pkt);
> + xdata->evt_hdr = (struct hci_event_hdr *)(xdata->rcv_pkt);
> + xdata->cmd_cmp = (struct hci_ev_cmd_complete *)(xdata->rcv_pkt +
> + EVT_HDR_LEN);
> + xdata->req_para = xdata->send_pkt + CMD_HDR_LEN;
> + xdata->rsp_para = xdata->rcv_pkt + EVT_HDR_LEN + CMD_CMP_LEN;
> +}
> +
> +static int check_fw_version(struct xchange_data *xdata)
> +{
> + struct hci_rp_read_local_version *read_ver_rsp;
> + struct patch_info *patch_entry;
> + int ret_val;
> +
> + xdata->cmd_hdr->opcode = cpu_to_le16(HCI_OP_READ_LOCAL_VERSION);
> + xdata->cmd_hdr->plen = 0;
> + xdata->pkt_len = CMD_HDR_LEN;
> +
> + ret_val = send_hci_cmd(xdata);
> + if (ret_val < 0)
> + goto version_end;
> +
> + ret_val = rcv_hci_evt(xdata);
> + if (ret_val < 0)
> + goto version_end;
> +
> + patch_entry = xdata->dev_entry->patch_entry;
> + read_ver_rsp = (struct hci_rp_read_local_version *)(xdata->rsp_para);
> + RTKBT_DBG("check_fw_version : read_ver_rsp->lmp_subver = 0x%x",
> + le16_to_cpu(read_ver_rsp->lmp_subver));
> + if (patch_entry->lmp_sub != le16_to_cpu(read_ver_rsp->lmp_subver))
> + return 1;
> +
> + ret_val = 0;
> +version_end:
> + return ret_val;
> +}
> +
> +static int get_firmware(struct xchange_data *xdata)
> +{
> + struct dev_data *dev_entry;
> + struct patch_info *patch_entry;
> +
> + dev_entry = xdata->dev_entry;
> + patch_entry = dev_entry->patch_entry;
> + if (patch_entry->fw_len > 0) {
> + xdata->fw_data = kzalloc(patch_entry->fw_len, GFP_KERNEL);
> + if (NULL == xdata->fw_data)
> + return -ENOMEM;
> + memcpy(xdata->fw_data, patch_entry->fw_cache,
> + patch_entry->fw_len);
> + xdata->fw_len = patch_entry->fw_len;
> + } else {
> + xdata->fw_len = load_firmware(dev_entry, &xdata->fw_data);
> + if (xdata->fw_len <= 0)
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +static int download_data(struct xchange_data *xdata)
> +{
> + struct download_cp *cmd_para;
> + struct download_rp *evt_para;
> + uint8_t *pcur;
> + int pkt_len, frag_num, frag_len;
> + int i, ret_val;
> +
> + cmd_para = (struct download_cp *)xdata->req_para;
> + evt_para = (struct download_rp *)xdata->rsp_para;
> + pcur = xdata->fw_data;
> + pkt_len = CMD_HDR_LEN + sizeof(struct download_cp);
> + frag_num = xdata->fw_len / PATCH_SEG_MAX + 1;
> + frag_len = PATCH_SEG_MAX;
> +
> + for (i = 0; i < frag_num; i++) {
> + cmd_para->index = i;
> + if (i == (frag_num - 1)) {
> + cmd_para->index |= DATA_END;
> + frag_len = xdata->fw_len % PATCH_SEG_MAX;
> + pkt_len -= (PATCH_SEG_MAX - frag_len);
> + }
> + xdata->cmd_hdr->opcode = cpu_to_le16(DOWNLOAD_OPCODE);
> + xdata->cmd_hdr->plen = sizeof(uint8_t) + frag_len;
> + xdata->pkt_len = pkt_len;
> + memcpy(cmd_para->data, pcur, frag_len);
> +
> + ret_val = send_hci_cmd(xdata);
> + if (ret_val < 0)
> + return ret_val;
> +
> + ret_val = rcv_hci_evt(xdata);
> + if (ret_val < 0)
> + return ret_val;
> + if (0 != evt_para->status)
> + return -1;
> +
> + pcur += PATCH_SEG_MAX;
> + }
> +
> + return xdata->fw_len;
> +}
> +
> +static int send_hci_cmd(struct xchange_data *xdata)
> +{
> + int ret_val;
> +
> + ret_val = usb_control_msg(
> + xdata->dev_entry->udev, xdata->pipe_out,
> + 0, USB_TYPE_CLASS, 0, 0,
> + (void *)(xdata->send_pkt),
> + xdata->pkt_len, MSG_TO);
> +
> + return ret_val;
> +}
> +
> +static int rcv_hci_evt(struct xchange_data *xdata)
> +{
> + int ret_len, ret_val;
> + int i; /* Added by Realtek */
> +
> + while (1) {
> +
> + /* **************************** Modifed by Realtek (begin) */
> + for (i = 0; i < 5; i++) {
> + /* Try to send USB interrupt message 5 times. */
> + ret_val = usb_interrupt_msg(
> + xdata->dev_entry->udev, xdata->pipe_in,
> + (void *)(xdata->rcv_pkt), PKT_LEN,
> + &ret_len, MSG_TO);
> + if (ret_val >= 0)
> + break;
> + }
> + /* **************************** Modifed by Realtek (end) */
> +
> + if (ret_val < 0)
> + return ret_val;
> +
> + if (CMD_CMP_EVT == xdata->evt_hdr->evt) {
> + if (xdata->cmd_hdr->opcode == xdata->cmd_cmp->opcode)
> + return ret_len;
> + }
> + }
> +}
>
^ permalink raw reply
* Re: [PATCH BlueZ 1/8] gdbus: Introduce G_DBUS_METHOD_FLAG_EXPERIMENTAL
From: Luiz Augusto von Dentz @ 2012-12-23 22:06 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <1356296297.29264.34.camel@aeonflux>
Hi Marcel,
On Sun, Dec 23, 2012 at 10:58 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Luiz,
>
>> This flag can be used to mark methods as experimental, the marked
>> methods with this flag can be enabled by setting the environment variable
>> GDBUS_EXPERIMENTAL=1
>> ---
>> gdbus/gdbus.h | 21 ++++++++++++++++++---
>> gdbus/object.c | 17 +++++++++++++++++
>> 2 files changed, 35 insertions(+), 3 deletions(-)
>>
>> diff --git a/gdbus/gdbus.h b/gdbus/gdbus.h
>> index 0e5c012..00fbb1c 100644
>> --- a/gdbus/gdbus.h
>> +++ b/gdbus/gdbus.h
>> @@ -89,9 +89,10 @@ typedef void (* GDBusSecurityFunction) (DBusConnection *connection,
>> GDBusPendingReply pending);
>>
>> enum GDBusMethodFlags {
>> - G_DBUS_METHOD_FLAG_DEPRECATED = (1 << 0),
>> - G_DBUS_METHOD_FLAG_NOREPLY = (1 << 1),
>> - G_DBUS_METHOD_FLAG_ASYNC = (1 << 2),
>> + G_DBUS_METHOD_FLAG_DEPRECATED = (1 << 0),
>> + G_DBUS_METHOD_FLAG_NOREPLY = (1 << 1),
>> + G_DBUS_METHOD_FLAG_ASYNC = (1 << 2),
>> + G_DBUS_METHOD_FLAG_EXPERIMENTAL = (1 << 3),
>> };
>>
>> enum GDBusSignalFlags {
>> @@ -173,6 +174,20 @@ struct GDBusSecurityTable {
>> .function = _function, \
>> .flags = G_DBUS_METHOD_FLAG_ASYNC | G_DBUS_METHOD_FLAG_DEPRECATED
>>
>> +#define GDBUS_EXPERIMENTAL_METHOD(_name, _in_args, _out_args, _function) \
>> + .name = _name, \
>> + .in_args = _in_args, \
>> + .out_args = _out_args, \
>> + .function = _function, \
>> + .flags = G_DBUS_METHOD_FLAG_EXPERIMENTAL
>> +
>> +#define GDBUS_EXPERIMENTAL_ASYNC_METHOD(_name, _in_args, _out_args, _function) \
>> + .name = _name, \
>> + .in_args = _in_args, \
>> + .out_args = _out_args, \
>> + .function = _function, \
>> + .flags = G_DBUS_METHOD_FLAG_ASYNC | G_DBUS_METHOD_FLAG_EXPERIMENTAL
>> +
>> #define GDBUS_NOREPLY_METHOD(_name, _in_args, _out_args, _function) \
>> .name = _name, \
>> .in_args = _in_args, \
>> diff --git a/gdbus/object.c b/gdbus/object.c
>> index 776d35e..30dbbc2 100644
>> --- a/gdbus/object.c
>> +++ b/gdbus/object.c
>> @@ -129,6 +129,14 @@ static void generate_interface_xml(GString *gstr, struct interface_data *iface)
>> G_DBUS_METHOD_FLAG_DEPRECATED;
>> gboolean noreply = method->flags &
>> G_DBUS_METHOD_FLAG_NOREPLY;
>> + gboolean experimental = method->flags &
>> + G_DBUS_METHOD_FLAG_EXPERIMENTAL;
>> +
>> + if (experimental) {
>> + const char *env = g_getenv("GDBUS_EXPERIMENTAL");
>> + if (g_strcmp0(env, "1") != 0)
>> + continue;
>> + }
>
> actually since this is a library, doing it this way is a bad idea.
I thought it was a common practice to use environment variables with
libraries to change certain defaults, glib does that with things like
G_SLICE=always-malloc, and it is quite convenient since you can change
easily without recompiling.
> Lets do something like g_dbus_enable_experimental(DBusConnection)
But this is not really per connection, anyway doing so you have to
handle this directly on the application code which IMO is not as
convenient.
--
Luiz Augusto von Dentz
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox