* Re: [PATCH 3/6] Bluetooth: Implement the first SMP commands
From: Gustavo F. Padovan @ 2010-10-25 13:03 UTC (permalink / raw)
To: Anderson Briglia; +Cc: linux-bluetooth, Vinicius Costa Gomes
In-Reply-To: <1287791820-22693-4-git-send-email-anderson.briglia@openbossa.org>
Hi Vinicius,
* Anderson Briglia <anderson.briglia@openbossa.org> [2010-10-22 19:56:57 -0400]:
> From: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
>
> These simple commands will allow the SMP procedure to be started
> and terminated with a not supported error. This is the first step
> toward something useful.
>
> Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
> ---
> net/bluetooth/l2cap.c | 117 +++++++++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 117 insertions(+), 0 deletions(-)
>
> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> index 1ac44f4..ba87c84 100644
> --- a/net/bluetooth/l2cap.c
> +++ b/net/bluetooth/l2cap.c
> @@ -54,6 +54,7 @@
> #include <net/bluetooth/bluetooth.h>
> #include <net/bluetooth/hci_core.h>
> #include <net/bluetooth/l2cap.h>
> +#include <net/bluetooth/smp.h>
>
> #define VERSION "2.15"
>
> @@ -307,6 +308,85 @@ static void l2cap_chan_del(struct sock *sk, int err)
> }
> }
>
> +static struct sk_buff *smp_build_cmd(struct l2cap_conn *conn, u8 code,
> + u16 dlen, void *data)
Call this l2cap_smp_build_cmd()
> +{
> + struct sk_buff *skb;
> + struct l2cap_hdr *lh;
> + int len;
> +
> + len = L2CAP_HDR_SIZE + 1 + dlen;
> +
> + if (len > conn->mtu)
> + return NULL;
> +
> + skb = bt_skb_alloc(len, GFP_ATOMIC);
> + if (!skb)
> + return NULL;
> +
> + lh = (struct l2cap_hdr *) skb_put(skb, L2CAP_HDR_SIZE);
> + lh->len = cpu_to_le16(1 + dlen);
cpu_to_le16(len - L2CAP_HDR_SIZE) here
> + lh->cid = cpu_to_le16(L2CAP_CID_SMP);
> +
> + memcpy(skb_put(skb, 1), &code, 1);
> +
> + memcpy(skb_put(skb, dlen), data, dlen);
> +
> + return skb;
> +}
> +
> +static inline void smp_send_cmd(struct l2cap_conn *conn, u8 code, u16 len, void *data)
and this l2cap_smp_send_cmd.
> +{
> + struct sk_buff *skb = smp_build_cmd(conn, code, len, data);
> +
> + BT_DBG("code 0x%2.2x", code);
> +
> + if (!skb)
> + return;
> +
> + hci_send_acl(conn->hcon, skb, 0);
> +}
> +
> +static int smp_conn_security(struct l2cap_conn *conn, __u8 sec_level)
> +{
l2cap_smp_conn_security() here.
> + __u8 authreq;
> +
> + BT_DBG("conn %p hcon %p level 0x%2.2x", conn, conn->hcon, sec_level);
> +
> + switch (sec_level) {
> + case BT_SECURITY_MEDIUM:
> + /* Encrypted, no MITM protection */
> + authreq = 0x01;
> + break;
> +
> + case BT_SECURITY_HIGH:
> + /* Bonding, MITM protection */
> + authreq = 0x05;
> + break;
> +
> + case BT_SECURITY_LOW:
> + default:
> + return 1;
> + }
> +
> + if (conn->hcon->link_mode & HCI_LM_MASTER) {
> + struct smp_cmd_pairing cp;
> + cp.io_capability = 0x00;
> + cp.oob_flag = 0x00;
> + cp.max_key_size = 16;
> + cp.init_key_dist = 0x00;
> + cp.resp_key_dist = 0x00;
> + cp.auth_req = authreq;
> + smp_send_cmd(conn, SMP_CMD_PAIRING_REQ, sizeof(cp), &cp);
> + } else {
> + struct smp_cmd_security_req cp;
> + cp.auth_req = authreq;
> + smp_send_cmd(conn, SMP_CMD_SECURITY_REQ, sizeof(cp), &cp);
> + }
> +
> + return 0;
> +}
> +
> /* Service level security */
> static inline int l2cap_check_security(struct sock *sk)
> {
> @@ -4562,6 +4642,43 @@ done:
> return 0;
> }
>
> +static inline void smp_sig_channel(struct l2cap_conn *conn, struct sk_buff *skb)
l2cap_smp_sig_channel()
--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi
^ permalink raw reply
* [PATCH 1/1] Fix fd comparison and comment
From: Elvis Pfützenreuter @ 2010-10-25 13:29 UTC (permalink / raw)
To: linux-bluetooth; +Cc: epx
---
health/hdp.c | 6 +++---
health/mcap_sync.c | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/health/hdp.c b/health/hdp.c
index fd3b1ca..7332cd5 100644
--- a/health/hdp.c
+++ b/health/hdp.c
@@ -339,7 +339,7 @@ static DBusMessage *manager_create_application(DBusConnection *conn,
"Can't get sender name");
}
- if (!set_app_path(app)){
+ if (!set_app_path(app)) {
free_application(app);
return g_dbus_create_error(msg,
ERROR_INTERFACE ".HealthError",
@@ -477,7 +477,7 @@ static void hdp_mdl_reconn_cb(struct mcap_mdl *mdl, GError *err, gpointer data)
}
fd = mcap_mdl_get_fd(dc_data->hdp_chann->mdl);
- if (fd <= 0)
+ if (fd < 0)
reply = g_dbus_create_error(dc_data->msg,
ERROR_INTERFACE ".HealthError",
"Cannot get file descriptor");
@@ -568,7 +568,7 @@ static DBusMessage *channel_acquire_continue(struct hdp_tmp_dc_data *data,
}
fd = mcap_mdl_get_fd(data->hdp_chann->mdl);
- if (fd > 0)
+ if (fd >= 0)
return g_dbus_create_reply(data->msg, DBUS_TYPE_UNIX_FD, &fd,
DBUS_TYPE_INVALID);
diff --git a/health/mcap_sync.c b/health/mcap_sync.c
index 49661a7..57ba0fd 100644
--- a/health/mcap_sync.c
+++ b/health/mcap_sync.c
@@ -350,7 +350,7 @@ static gboolean initialize_caps(struct mcap_mcl *mcl)
clock_gettime(CLK, &t1);
read_btclock_retry(mcl, &btclock, &btaccuracy);
- /* Do clock read a number of times and measure latency */
+ /* Read clock a number of times and measure latency */
avg = 0;
i = 0;
retries = MAX_RETRIES;
--
1.7.0.4
^ permalink raw reply related
* Re: [RFC] LE connections and advertising management
From: Anderson Lizardo @ 2010-10-25 13:34 UTC (permalink / raw)
To: Claudio Takahasi; +Cc: BlueZ development
In-Reply-To: <AANLkTin8pUACCu4YYuhLpFB7DxCfsYhN3cOpYYaVcTQ6@mail.gmail.com>
On Mon, Oct 25, 2010 at 8:53 AM, Claudio Takahasi
<claudio.takahasi@openbossa.org> wrote:
> 4. Kernel manages the connection establishment, currently there isn't
> a way to change the connection parameters. BMI or ioctls will be
> required to change the default parameters and also to trigger SMP
> negotiation.
Another idea would be to use setsockopt(), like it's currently being
used for HCI filters (see lib/hci.c). E.g.:
hci_filter_clear(&nf);
hci_filter_set_ptype(HCI_EVENT_PKT, &nf);
hci_filter_set_event(EVT_LE_META_EVENT, &nf);
setsockopt(dd, SOL_HCI, HCI_FILTER, &nf, sizeof(nf);
Not sure if the SMP negotiation happens too soon to use it though.
Regards,
--
Anderson Lizardo
OpenBossa Labs - INdT
Manaus - Brazil
^ permalink raw reply
* [PATCH] Send a proper configuration when a source doesn't have a first reliable
From: Santiago Carot-Nemesio @ 2010-10-25 13:43 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Santiago Carot-Nemesio
---
health/hdp.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/health/hdp.c b/health/hdp.c
index fd3b1ca..10f4c5f 100644
--- a/health/hdp.c
+++ b/health/hdp.c
@@ -1026,7 +1026,7 @@ static uint8_t hdp_mcap_mdl_conn_req_cb(struct mcap_mcl *mcl, uint8_t mdepid,
case HDP_NO_PREFERENCE_DC:
if (app->role == HDP_SINK)
return MCAP_CONFIGURATION_REJECTED;
- else if (app->chan_type_set)
+ else if (dev->fr && app->chan_type_set)
*conf = app->chan_type;
else
*conf = HDP_RELIABLE_DC;
--
1.7.3.1
^ permalink raw reply related
* Re: [PATCH 1/1] Fix fd comparison and comment
From: Johan Hedberg @ 2010-10-25 16:22 UTC (permalink / raw)
To: Elvis Pfützenreuter; +Cc: linux-bluetooth
In-Reply-To: <1288013352-23258-1-git-send-email-epx@signove.com>
Hi Elvis,
On Mon, Oct 25, 2010, Elvis Pfützenreuter wrote:
> ---
> health/hdp.c | 6 +++---
> health/mcap_sync.c | 2 +-
> 2 files changed, 4 insertions(+), 4 deletions(-)
Please don't mix coding style cleanup and real bug fixes into a single
patch. I.e. could you please split these up. Thanks.
Johan
^ permalink raw reply
* Re: [PATCH] Provide extra query for vCard single call
From: Johan Hedberg @ 2010-10-25 16:26 UTC (permalink / raw)
To: Rafal Michalski; +Cc: linux-bluetooth
In-Reply-To: <1287997498-2576-1-git-send-email-michalski.raf@gmail.com>
Hi Rafal,
On Mon, Oct 25, 2010, Rafal Michalski wrote:
> This patch makes that additional circumstance is recognized - after
> making a call with number that is out of phonebook it can be downloaded
> as a single vCard containing number with OTHER type.
> ---
> plugins/phonebook-tracker.c | 20 ++++++++++++++++++--
> 1 files changed, 18 insertions(+), 2 deletions(-)
Pushed upstream, but I had to fix your coding style first:
> - query = g_strdup_printf(CONTACTS_QUERY_FROM_URI, id, id, id, id, id,
> - id, id, id, id, id, id, id,
> + if (strncmp(id, CONTACT_ID_PREFIX, strlen(CONTACT_ID_PREFIX)) == 0) {
> + query = g_strdup_printf(CONTACTS_QUERY_FROM_URI, id, id, id, id,
> + id, id, id, id, id, id, id, id,
> id, id, id, id, id);
> + } else {
> + query = g_strdup_printf(CONTACTS_OTHER_QUERY_FROM_URI,
> + id, id, id);
> + }
>
> ret = query_tracker(query, PULL_QUERY_COL_AMOUNT, pull_contacts, data);
Single line branches shouldn't use curly braces.
Johan
^ permalink raw reply
* [PATCH 1/2] Fix fd comparison
From: Elvis Pfützenreuter @ 2010-10-25 16:28 UTC (permalink / raw)
To: linux-bluetooth; +Cc: epx
---
health/hdp.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/health/hdp.c b/health/hdp.c
index fd3b1ca..eb27613 100644
--- a/health/hdp.c
+++ b/health/hdp.c
@@ -477,7 +477,7 @@ static void hdp_mdl_reconn_cb(struct mcap_mdl *mdl, GError *err, gpointer data)
}
fd = mcap_mdl_get_fd(dc_data->hdp_chann->mdl);
- if (fd <= 0)
+ if (fd < 0)
reply = g_dbus_create_error(dc_data->msg,
ERROR_INTERFACE ".HealthError",
"Cannot get file descriptor");
@@ -568,7 +568,7 @@ static DBusMessage *channel_acquire_continue(struct hdp_tmp_dc_data *data,
}
fd = mcap_mdl_get_fd(data->hdp_chann->mdl);
- if (fd > 0)
+ if (fd >= 0)
return g_dbus_create_reply(data->msg, DBUS_TYPE_UNIX_FD, &fd,
DBUS_TYPE_INVALID);
--
1.7.0.4
^ permalink raw reply related
* [PATCH 2/2] Fix coding style and comment
From: Elvis Pfützenreuter @ 2010-10-25 16:28 UTC (permalink / raw)
To: linux-bluetooth; +Cc: epx
In-Reply-To: <1288024124-24492-1-git-send-email-epx@signove.com>
---
health/hdp.c | 2 +-
health/mcap_sync.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/health/hdp.c b/health/hdp.c
index eb27613..7332cd5 100644
--- a/health/hdp.c
+++ b/health/hdp.c
@@ -339,7 +339,7 @@ static DBusMessage *manager_create_application(DBusConnection *conn,
"Can't get sender name");
}
- if (!set_app_path(app)){
+ if (!set_app_path(app)) {
free_application(app);
return g_dbus_create_error(msg,
ERROR_INTERFACE ".HealthError",
diff --git a/health/mcap_sync.c b/health/mcap_sync.c
index 49661a7..57ba0fd 100644
--- a/health/mcap_sync.c
+++ b/health/mcap_sync.c
@@ -350,7 +350,7 @@ static gboolean initialize_caps(struct mcap_mcl *mcl)
clock_gettime(CLK, &t1);
read_btclock_retry(mcl, &btclock, &btaccuracy);
- /* Do clock read a number of times and measure latency */
+ /* Read clock a number of times and measure latency */
avg = 0;
i = 0;
retries = MAX_RETRIES;
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH] Send a proper configuration when a source doesn't have a first reliable
From: Johan Hedberg @ 2010-10-25 16:29 UTC (permalink / raw)
To: Santiago Carot-Nemesio; +Cc: linux-bluetooth
In-Reply-To: <1288014239-10832-1-git-send-email-sancane@gmail.com>
Hi,
On Mon, Oct 25, 2010, Santiago Carot-Nemesio wrote:
> ---
> health/hdp.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/health/hdp.c b/health/hdp.c
> index fd3b1ca..10f4c5f 100644
> --- a/health/hdp.c
> +++ b/health/hdp.c
> @@ -1026,7 +1026,7 @@ static uint8_t hdp_mcap_mdl_conn_req_cb(struct mcap_mcl *mcl, uint8_t mdepid,
> case HDP_NO_PREFERENCE_DC:
> if (app->role == HDP_SINK)
> return MCAP_CONFIGURATION_REJECTED;
> - else if (app->chan_type_set)
> + else if (dev->fr && app->chan_type_set)
> *conf = app->chan_type;
> else
> *conf = HDP_RELIABLE_DC;
Pushed upstream. Thanks.
Johan
^ permalink raw reply
* RE: [RFC] LE connections and advertising management
From: Mike Tsai @ 2010-10-25 17:11 UTC (permalink / raw)
To: Claudio Takahasi, BlueZ development
In-Reply-To: <AANLkTin8pUACCu4YYuhLpFB7DxCfsYhN3cOpYYaVcTQ6@mail.gmail.com>
-----Original Message-----
From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-owner@vger.kernel.org] On Behalf Of Claudio Takahasi
Sent: Monday, October 25, 2010 5:53 AM
To: BlueZ development
Subject: [RFC] LE connections and advertising management
Hi all,
Interleave BR/EDR/LE discovery is implemented, the next step in the
user space is how to manage advertising and LE connections.
Some aspects:
1. Only one LE connection is allowed(per peer), meaning only one
GAttrib instance will be allowed otherwise it will not be possible to
serialize commands/events
2. The remote/Peripheral can support more than one GATT primary service
3. We are planning to use "direct" connections only, meaning that we
will not use whitelist and the application interested must inform the
remote address/object to connect to.
4. Kernel manages the connection establishment, currently there isn't
a way to change the connection parameters. BMI or ioctls will be
required to change the default parameters and also to trigger SMP
negotiation.
Some ideas:
1. implement a characteristic driver: basically to provide an
abstraction to GATT clients. ex: Proximity, Health, ...
2. We don't need to implement Proximity and other GATT clients as a
plugin at the moment, it can be enabled automatically by
--enable-attrib
3. GATT clients could register a watcher/filter for advertising events
4. GATT clients doesn't need to know ATT, in theory it can handle
characteristics only
5. GATT clients needs to control/request LE connections based on the
advertisement received
An initial draft implementing part of this idea is here:
git://git.infradead.org/users/cktakahasi/bluez.git devel
Comments?
Regards,
Claudio
>>MT comments,
1. Only one LE connection is allowed(per peer), meaning only one
GAttrib instance will be allowed otherwise it will not be possible to
serialize commands/events
>> You mean the master (or client) can only connect to 1 slave (or server) or a slave can only connect to 1 master?
4. GATT clients doesn't need to know ATT, in theory it can handle
characteristics only
>> You mean both characteristic value and characteristic descriptors of characteristic?
Regards,
Mike
--
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
^ permalink raw reply
* Re: [RFC] LE connections and advertising management
From: Claudio Takahasi @ 2010-10-25 17:55 UTC (permalink / raw)
To: Mike Tsai; +Cc: BlueZ development
In-Reply-To: <35B17FE5076C7040809188FBE7913F983F844057B9@SC1EXMB-MBCL.global.atheros.com>
On Mon, Oct 25, 2010 at 2:11 PM, Mike Tsai <Mike.Tsai@atheros.com> wrote:
>
> -----Original Message-----
> From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-owner@vger.kernel.org] On Behalf Of Claudio Takahasi
> Sent: Monday, October 25, 2010 5:53 AM
> To: BlueZ development
> Subject: [RFC] LE connections and advertising management
>
> Hi all,
>
> Interleave BR/EDR/LE discovery is implemented, the next step in the
> user space is how to manage advertising and LE connections.
>
> Some aspects:
> 1. Only one LE connection is allowed(per peer), meaning only one
> GAttrib instance will be allowed otherwise it will not be possible to
> serialize commands/events
> 2. The remote/Peripheral can support more than one GATT primary service
> 3. We are planning to use "direct" connections only, meaning that we
> will not use whitelist and the application interested must inform the
> remote address/object to connect to.
> 4. Kernel manages the connection establishment, currently there isn't
> a way to change the connection parameters. BMI or ioctls will be
> required to change the default parameters and also to trigger SMP
> negotiation.
>
>
> Some ideas:
> 1. implement a characteristic driver: basically to provide an
> abstraction to GATT clients. ex: Proximity, Health, ...
> 2. We don't need to implement Proximity and other GATT clients as a
> plugin at the moment, it can be enabled automatically by
> --enable-attrib
> 3. GATT clients could register a watcher/filter for advertising events
> 4. GATT clients doesn't need to know ATT, in theory it can handle
> characteristics only
> 5. GATT clients needs to control/request LE connections based on the
> advertisement received
>
> An initial draft implementing part of this idea is here:
> git://git.infradead.org/users/cktakahasi/bluez.git devel
>
> Comments?
>
> Regards,
> Claudio
>>>MT comments,
>
> 1. Only one LE connection is allowed(per peer), meaning only one
> GAttrib instance will be allowed otherwise it will not be possible to
> serialize commands/events
>
>>> You mean the master (or client) can only connect to 1 slave (or server) or a slave can only connect to 1 master?
[Claudio Takahasi]
The master can be connected to more than one slave. But here, I wanna
emphasize that only one instance of GAtttrib shall be created to
represent the physical channel between two LE capable devices. GAttrib
is the abstraction that we created in BlueZ to represent the GATT/ATT
connection, it is necessary to address multiple applications/profiles
and queue commands/events.
>
> 4. GATT clients doesn't need to know ATT, in theory it can handle
> characteristics only
>
>>> You mean both characteristic value and characteristic descriptors of characteristic?
[Claudio Takahasi]
Both. For the GATT client role, an interface can be created exporting
profile specific details.
Sometimes GATT clients will also need to access some low level information.
Regards,
Claudio
>
> Regards,
>
> Mike
>
> --
> 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
>
^ permalink raw reply
* RE: [RFC] LE connections and advertising management
From: Mike Tsai @ 2010-10-25 18:16 UTC (permalink / raw)
To: Claudio Takahasi; +Cc: BlueZ development
In-Reply-To: <AANLkTinS-NhD3FhqcnpNRRdHKxn4mm+2zTmysrSeSjW8@mail.gmail.com>
-----Original Message-----
From: Claudio Takahasi [mailto:claudio.takahasi@openbossa.org]
Sent: Monday, October 25, 2010 10:56 AM
To: Mike Tsai
Cc: BlueZ development
Subject: Re: [RFC] LE connections and advertising management
On Mon, Oct 25, 2010 at 2:11 PM, Mike Tsai <Mike.Tsai@atheros.com> wrote:
>
> -----Original Message-----
> From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-owner@vger.kernel.org] On Behalf Of Claudio Takahasi
> Sent: Monday, October 25, 2010 5:53 AM
> To: BlueZ development
> Subject: [RFC] LE connections and advertising management
>
> Hi all,
>
> Interleave BR/EDR/LE discovery is implemented, the next step in the
> user space is how to manage advertising and LE connections.
>
> Some aspects:
> 1. Only one LE connection is allowed(per peer), meaning only one
> GAttrib instance will be allowed otherwise it will not be possible to
> serialize commands/events
> 2. The remote/Peripheral can support more than one GATT primary service
> 3. We are planning to use "direct" connections only, meaning that we
> will not use whitelist and the application interested must inform the
> remote address/object to connect to.
> 4. Kernel manages the connection establishment, currently there isn't
> a way to change the connection parameters. BMI or ioctls will be
> required to change the default parameters and also to trigger SMP
> negotiation.
>
>
> Some ideas:
> 1. implement a characteristic driver: basically to provide an
> abstraction to GATT clients. ex: Proximity, Health, ...
> 2. We don't need to implement Proximity and other GATT clients as a
> plugin at the moment, it can be enabled automatically by
> --enable-attrib
> 3. GATT clients could register a watcher/filter for advertising events
> 4. GATT clients doesn't need to know ATT, in theory it can handle
> characteristics only
> 5. GATT clients needs to control/request LE connections based on the
> advertisement received
>
> An initial draft implementing part of this idea is here:
> git://git.infradead.org/users/cktakahasi/bluez.git devel
>
> Comments?
>
> Regards,
> Claudio
>>>MT comments,
>
> 1. Only one LE connection is allowed(per peer), meaning only one
> GAttrib instance will be allowed otherwise it will not be possible to
> serialize commands/events
>
>>> You mean the master (or client) can only connect to 1 slave (or server) or a slave can only connect to 1 master?
[Claudio Takahasi]
The master can be connected to more than one slave. But here, I wanna
emphasize that only one instance of GAtttrib shall be created to
represent the physical channel between two LE capable devices. GAttrib
is the abstraction that we created in BlueZ to represent the GATT/ATT
connection, it is necessary to address multiple applications/profiles
and queue commands/events.
[Mike Tsai]
I see. In order to handle multiple profiles (as multiple applications running on top of the GAttrib) in a single physical link, you will need something like "application handle" to dispatch the response/identification to the appropriate application correctly.
May you refer me to the API document that you are opening to the GATT client(s) from this GAttrib?
>
> 4. GATT clients doesn't need to know ATT, in theory it can handle
> characteristics only
>
>>> You mean both characteristic value and characteristic descriptors of characteristic?
[Claudio Takahasi]
Both. For the GATT client role, an interface can be created exporting
profile specific details.
Sometimes GATT clients will also need to access some low level information.
[Mike Tsai]
Yes, I will really appreciate if you may show me the planned API for this GAttrib,
Regards,
Claudio
>
> Regards,
>
> Mike
>
> --
> 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
>
^ permalink raw reply
* Re: [PATCH 1/2] Fix fd comparison
From: Johan Hedberg @ 2010-10-25 18:49 UTC (permalink / raw)
To: Elvis Pfützenreuter; +Cc: linux-bluetooth
In-Reply-To: <1288024124-24492-1-git-send-email-epx@signove.com>
Hi Elvis,
On Mon, Oct 25, 2010, Elvis Pfützenreuter wrote:
> ---
> health/hdp.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
Pushed upstream. Thanks.
Johan
^ permalink raw reply
* Re: [PATCH 2/2] Fix coding style and comment
From: Johan Hedberg @ 2010-10-25 18:50 UTC (permalink / raw)
To: Elvis Pfützenreuter; +Cc: linux-bluetooth
In-Reply-To: <1288024124-24492-2-git-send-email-epx@signove.com>
Hi Elvis,
On Mon, Oct 25, 2010, Elvis Pfützenreuter wrote:
> ---
> health/hdp.c | 2 +-
> health/mcap_sync.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
This one has also been pushed upstream. Thanks.
Johan
^ permalink raw reply
* Re: [RFC] LE connections and advertising management
From: Claudio Takahasi @ 2010-10-25 18:54 UTC (permalink / raw)
To: Mike Tsai; +Cc: BlueZ development
In-Reply-To: <35B17FE5076C7040809188FBE7913F983F8440581D@SC1EXMB-MBCL.global.atheros.com>
On Mon, Oct 25, 2010 at 3:16 PM, Mike Tsai <Mike.Tsai@atheros.com> wrote:
>
>
> -----Original Message-----
> From: Claudio Takahasi [mailto:claudio.takahasi@openbossa.org]
> Sent: Monday, October 25, 2010 10:56 AM
> To: Mike Tsai
> Cc: BlueZ development
> Subject: Re: [RFC] LE connections and advertising management
>
> On Mon, Oct 25, 2010 at 2:11 PM, Mike Tsai <Mike.Tsai@atheros.com> wrote:
>>
>> -----Original Message-----
>> From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-owner@vger.kernel.org] On Behalf Of Claudio Takahasi
>> Sent: Monday, October 25, 2010 5:53 AM
>> To: BlueZ development
>> Subject: [RFC] LE connections and advertising management
>>
>> Hi all,
>>
>> Interleave BR/EDR/LE discovery is implemented, the next step in the
>> user space is how to manage advertising and LE connections.
>>
>> Some aspects:
>> 1. Only one LE connection is allowed(per peer), meaning only one
>> GAttrib instance will be allowed otherwise it will not be possible to
>> serialize commands/events
>> 2. The remote/Peripheral can support more than one GATT primary service
>> 3. We are planning to use "direct" connections only, meaning that we
>> will not use whitelist and the application interested must inform the
>> remote address/object to connect to.
>> 4. Kernel manages the connection establishment, currently there isn't
>> a way to change the connection parameters. BMI or ioctls will be
>> required to change the default parameters and also to trigger SMP
>> negotiation.
>>
>>
>> Some ideas:
>> 1. implement a characteristic driver: basically to provide an
>> abstraction to GATT clients. ex: Proximity, Health, ...
>> 2. We don't need to implement Proximity and other GATT clients as a
>> plugin at the moment, it can be enabled automatically by
>> --enable-attrib
>> 3. GATT clients could register a watcher/filter for advertising events
>> 4. GATT clients doesn't need to know ATT, in theory it can handle
>> characteristics only
>> 5. GATT clients needs to control/request LE connections based on the
>> advertisement received
>>
>> An initial draft implementing part of this idea is here:
>> git://git.infradead.org/users/cktakahasi/bluez.git devel
>>
>> Comments?
>>
>> Regards,
>> Claudio
>>>>MT comments,
>>
>> 1. Only one LE connection is allowed(per peer), meaning only one
>> GAttrib instance will be allowed otherwise it will not be possible to
>> serialize commands/events
>>
>>>> You mean the master (or client) can only connect to 1 slave (or server) or a slave can only connect to 1 master?
> [Claudio Takahasi]
> The master can be connected to more than one slave. But here, I wanna
> emphasize that only one instance of GAtttrib shall be created to
> represent the physical channel between two LE capable devices. GAttrib
> is the abstraction that we created in BlueZ to represent the GATT/ATT
> connection, it is necessary to address multiple applications/profiles
> and queue commands/events.
> [Mike Tsai]
> I see. In order to handle multiple profiles (as multiple applications running on top of the GAttrib) in a single physical link, you will need something like "application handle" to dispatch the response/identification to the appropriate application correctly.
> May you refer me to the API document that you are opening to the GATT client(s) from this GAttrib?
[Claudio Takahasi]
Hi Mike,
At the moment it is not clear to me if an "application handle" will be
needed, maybe we can also hide it from the higher layers. All requests
are serialized and for each request there is a response, the
exceptions are write without response and notification. Each primary
service representation will have an object path, meaning that maybe it
is possible forward the response to the right source based on the last
request and handle information.
GAttrib will not be exposed to the UI. UI needs to access BlueZ GATT
clients services using D-Bus.
GATT clients in general will have two pieces:
1- UI: Qt, GTK, python, ...
2- "module" in the BlueZ for profile specific tasks and D-Bus service
interface.
You can find the current attribute API in the file: doc/attribute-api.txt
Claudio
>
>>
>> 4. GATT clients doesn't need to know ATT, in theory it can handle
>> characteristics only
>>
>>>> You mean both characteristic value and characteristic descriptors of characteristic?
> [Claudio Takahasi]
> Both. For the GATT client role, an interface can be created exporting
> profile specific details.
> Sometimes GATT clients will also need to access some low level information.
> [Mike Tsai]
> Yes, I will really appreciate if you may show me the planned API for this GAttrib,
>
>
> Regards,
> Claudio
>>
>> Regards,
>>
>> Mike
>>
>> --
>> 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
>>
>
--
--
Claudio Takahasi
Instituto Nokia de Tecnologia
Recife - Pernambuco - Brasil
+55 81 30879999
^ permalink raw reply
* RE: [RFC] LE connections and advertising management
From: Brian Redding @ 2010-10-25 19:27 UTC (permalink / raw)
To: 'Claudio Takahasi'; +Cc: 'BlueZ development'
In-Reply-To: <AANLkTinnjXuAOBTY8VZz756VdrDFv0-OVKPNkwWbxi9-@mail.gmail.com>
> GAttrib will not be exposed to the UI. UI needs to access BlueZ GATT
> clients services using D-Bus.
> GATT clients in general will have two pieces:
> 1- UI: Qt, GTK, python, ...
> 2- "module" in the BlueZ for profile specific tasks and D-Bus service
> interface.
> You can find the current attribute API in the file: doc/attribute-api.txt
> Claudio
Hi Claudio,
Are there still interfaces that need to be added to attribute-api.txt to handle client and server characteristic configuration as well as presentation and aggregate formats? I see those as TODO items but wondered if the APIs for them have been defined yet.
One thing to note on the server API is that a GATT-based profile could specify behavior on a characteristic value that when the characteristic value is read to perform some action in a similar way as a hardware register. It appears that the notes I'm reading in the code thus far only consider changes (or writes) to characteristic values for the watch code.
Also does the current API handle included services?
The Bluetooth SIG is beginning to look at 3rd party application developer API's for both client and servers for various platforms so understanding what is currently defined in BlueZ and what still needs to be added would be useful.
Thanks,
Brian
---
Brian A. Redding
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply
* Re: [PATCH 2/6] Bluetooth: Add LE connect support
From: Anderson Lizardo @ 2010-10-25 20:33 UTC (permalink / raw)
To: Ville Tervo; +Cc: linux-bluetooth
In-Reply-To: <1288009280-5149-3-git-send-email-ville.tervo@nokia.com>
Hi,
On Mon, Oct 25, 2010 at 8:21 AM, Ville Tervo <ville.tervo@nokia.com> wrote:
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index 0b1e460..0944c0c 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -45,6 +45,32 @@
> #include <net/bluetooth/bluetooth.h>
> #include <net/bluetooth/hci_core.h>
>
> +void hci_le_connect(struct hci_conn *conn)
This function could be made static, right? (just noticed this now)
> +{
> + struct hci_dev *hdev = conn->hdev;
> + struct hci_cp_le_create_conn cp;
> +
> + conn->state = BT_CONNECT;
> + conn->out = 1;
> +
> + memset(&cp, 0, sizeof(cp));
> + cp.scan_interval = cpu_to_le16(0x0004);
> + cp.scan_window = cpu_to_le16(0x0004);
> + bacpy(&cp.peer_addr, &conn->dst);
> + cp.conn_interval_min = cpu_to_le16(0x0008);
> + cp.conn_interval_max = cpu_to_le16(0x0100);
> + cp.supervision_timeout = cpu_to_le16(0x0064);
> + cp.min_ce_len = cpu_to_le16(0x0001);
> + cp.max_ce_len = cpu_to_le16(0x0001);
> +
> + hci_send_cmd(hdev, HCI_OP_LE_CREATE_CONN, sizeof(cp), &cp);
> +}
Regards,
--
Anderson Lizardo
OpenBossa Labs - INdT
Manaus - Brazil
^ permalink raw reply
* Re: 4.76 possible regression: bluetoothd segfaults when launching bluetooth programs
From: Johan Hedberg @ 2010-10-25 20:40 UTC (permalink / raw)
To: Ilya Basin; +Cc: linux-bluetooth
In-Reply-To: <453819375.20101024163802@gmail.com>
Hi Ilya,
On Sun, Oct 24, 2010, Ilya Basin wrote:
> Program received signal SIGSEGV, Segmentation fault.
> 0xb7d3e653 in strlen () from /lib/libc.so.6
> (gdb) bt
> #0 0xb7d3e653 in strlen () from /lib/libc.so.6
> #1 0xb7e5eb10 in ?? () from /usr/lib/libdbus-1.so.3
> #2 0xb7e4a34b in ?? () from /usr/lib/libdbus-1.so.3
> #3 0xb7e4e7a9 in dbus_message_iter_append_basic () from /usr/lib/libdbus-1.so.3
> #4 0xb7fef03d in append_array_variant ()
> #5 0xb7fef799 in emit_array_property_changed ()
> #6 0xb7fe4de4 in adapter_service_ins_rem ()
> #7 0xb7fd7fb1 in sdp_record_add ()
> #8 0xb7fd79de in service_register_req ()
> #9 0xb7fd5dfc in handle_request ()
> #10 0xb7fd496e in io_session_event ()
> #11 0xb7ef7a2b in ?? () from /usr/lib/libglib-2.0.so.0
> #12 0xb7eb0b72 in g_main_context_dispatch () from /usr/lib/libglib-2.0.so.0
> #13 0xb7eb1350 in ?? () from /usr/lib/libglib-2.0.so.0
> #14 0xb7eb1a1b in g_main_loop_run () from /usr/lib/libglib-2.0.so.0
> #15 0xb7fd1bbd in main ()
> (gdb)
Unfortunately this doesn't give too much info since you don't seem to
have all debug symbols enabled. Could you try to reproduce this with
latest bluez git. You don't need to install anything but just compile
(./boostrap-configure && make) and run (src/bluetoothd -nd) from the
source tree directly. Then, it'd also be nice if you could use git
bisect to determine the exact commit between 4.69 and 4.76 that
introduced this regression.
Thanks.
Johan
^ permalink raw reply
* Re: [PATCH 1/2 v2] Bluetooth: Fix system crash caused by del_timer()
From: haijun liu @ 2010-10-26 1:32 UTC (permalink / raw)
To: Gustavo F. Padovan; +Cc: Haijun Liu, linux-bluetooth
In-Reply-To: <20101025110131.GA7721@vigoh>
Hi Gustavo,
>> >> During test session with another vendor's bt stack, found that in
>> >> l2cap_chan_del() using del_timer() caused l2cap_monitor_timeout()
>> >> be called after the sock was freed, so it raised a system crash.
>> >> So I just replaced del_timer() with del_timer_sync() to solve it.
>> >
>> > NAK on this. If you read the del_timer_sync() documentation you can
>> > see that you can't call del_timer_sync() on interrupt context. The
>> > possible solution here is to check in the beginning of
>> > l2cap_monitor_timeout() if your sock is still valid.
>> >
>>
>> You are right, I only considered close() interface, so missed the interrupt
>> context.
>>
>> It's very difficult to check sock valid or not in timeout procedure, since it's
>> an interrupt context, and only can get context from parameter pre-stored,
>> except global variables.
>
> I think you can check for sk == null there.
>
It's a pre-stored parameter, it will not change by itself.
--
Haijun Liu
^ permalink raw reply
* Re: [RFC] LE connections and advertising management
From: Claudio Takahasi @ 2010-10-26 2:51 UTC (permalink / raw)
To: Brian Redding; +Cc: BlueZ development
In-Reply-To: <000001cb747a$9fd61270$df823750$@org>
Hi Brian,
On Mon, Oct 25, 2010 at 4:27 PM, Brian Redding <bredding@codeaurora.org> wrote:
> Hi Claudio,
>
> Are there still interfaces that need to be added to attribute-api.txt to handle client and server characteristic configuration as well as presentation and aggregate formats? I see those as TODO items but wondered if the APIs for them have been defined yet.
>
> One thing to note on the server API is that a GATT-based profile could specify behavior on a characteristic value that when the characteristic value is read to perform some action in a similar way as a hardware register. It appears that the notes I'm reading in the code thus far only consider changes (or writes) to characteristic values for the watch code.
>
> Also does the current API handle included services?
>
> The Bluetooth SIG is beginning to look at 3rd party application developer API's for both client and servers for various platforms so understanding what is currently defined in BlueZ and what still needs to be added would be useful.
>
> Thanks,
> Brian
> ---
> Brian A. Redding
> Employee of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
The API to address characteristic descriptors is still being defined.
We are focusing in the advertising and connection management at the
moment. If you have suggestion of how to represent the descriptors in
the attribute API, suggestions are welcome!
There isn't a server API at the moment, we implemented a server for
testing purpose only. But we will need to define it soon.
Which pages/section of the spec describes this read characteristic behavior?
Included services are not supported by our client. How important is
it? It is mandatory for qualification?
Regards,
Claudio.
^ permalink raw reply
* Re: [PATCH 1/9] mfd: Add support for the ST-Ericsson CG2900.
From: Par-Gunnar Hjalmdahl @ 2010-10-26 6:54 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: linus.walleij, linux-bluetooth, linux-kernel
In-Reply-To: <201010221736.57671.arnd@arndb.de>
Hi Arnd,
Thanks for your comments. See below for answers.
/P-G
2010/10/22 Arnd Bergmann <arnd@arndb.de>:
> On Friday 22 October 2010 12:35:16 Par-Gunnar Hjalmdahl wrote:
>> This patch adds support for the ST-Ericsson CG2900 Connectivity
>> Combo controller.
>> This patch adds the central framework to be able to register CG2900 users,
>> transports, and chip handlers.
>>
>> Signed-off-by: Par-Gunnar Hjalmdahl <par-gunnar.p.hjalmdahl@stericsson.com>
>
> Looks ok from a coding style perspective, but some important information is
> missing from the description:
>
> * What is a CG2900?
> * Why is it a MFD device rather than e.g. a bus or a subsystem?
>
I will add some more info:
- CG2900 is a GPS, Bluetooth, FM controller
- I do not really know what makes a device qualify as a certain type,
but at least for us this is a multifunction device. But I can't really
say that it isn't really a bus (could be stated as multifunction HCI
bus). I guess it could be qualified as a subsystem as well, but I
cannot give you a reason to have it as either.
>> +config MFD_CG2900
>> + tristate "Support ST-Ericsson CG2900 main structure"
>> + depends on NET
>> + help
>> + Support for ST-Ericsson CG2900 Connectivity Combo controller main structure.
>> + Supports multiple functionalities muxed over a Bluetooth HCI H:4 interface.
>> + CG2900 support Bluetooth, FM radio, and GPS.
>> +
>
> Can you explain what it means to mux over a H:4 interface? Does this mean
> you use bluetooth infrastructure that is designed for wireless communication
> in order to connect on-board or on-chip devices?
>
The H:4 protocol is defined in the Bluetooth Core specification. It
uses a single byte first in the data packet to determine an HCI
channel. In the Bluetooth Core specification there are 4 channels
specified, 0x01-0x04 (0x01 is BT command, etc). The rest of the
channels are reserved, but these have been used on a proprietary basis
to transport data to different IPs within the controller. In our API
we have chosen to have a channel separation on an API basis and the
H:4 byte is then added within the driver. So we have multiple channels
coming in from the users that we mux onto a single data transport.
That's what I mean in the text. I guess I could rewrite it a bit to
make it clearer.
>> @@ -0,0 +1,2401 @@
>> +/*
>> + * drivers/mfd/cg2900/cg2900_core.c
>> + *
>> + * Copyright (C) ST-Ericsson SA 2010
>> + * Authors:
>> + * Par-Gunnar Hjalmdahl (par-gunnar.p.hjalmdahl@stericsson.com) for
>> ST-Ericsson.
>
> Your email client rewraps lines. You need to fix so that other people
> can apply your patches.
>
I will fix this.
>> +/*
>> + * chip_handlers - List of the register handlers for different chips.
>> + */
>> +LIST_HEAD(chip_handlers);
>
> Should this be static? Don't you need a lock to access the list?
>
You're right. I need to have mutex for this and it can just as well be
in the allocated cg2900_info structure.
>> +/**
>> + * find_h4_user() - Get H4 user based on supplied H4 channel.
>> + * @h4_channel: H4 channel.
>> + * @dev: Stored CG2900 device.
>> + * @skb: (optional) skb with received packet. Set to NULL if NA.
>> + *
>> + * Returns:
>> + * 0 if there is no error.
>> + * -EINVAL if bad channel is supplied or no user was found.
>> + * -ENXIO if channel is audio channel but not registered with CG2900.
>> + */
>> +static int find_h4_user(int h4_channel, struct cg2900_device **dev,
>> + const struct sk_buff * const skb)
>> +{
>> + int err = 0;
>> + struct cg2900_users *users = &(core_info->users);
>> + struct cg2900_h4_channels *chan = &(core_info->h4_channels);
>> +
>> + if (h4_channel == chan->bt_cmd_channel) {
>> + *dev = users->bt_cmd;
>> + } else if (h4_channel == chan->bt_acl_channel) {
>> + *dev = users->bt_acl;
>> + } else if (h4_channel == chan->bt_evt_channel) {
>> + *dev = users->bt_evt;
>> + /* Check if it's event generated by previously sent audio user
>> + * command. If so then that event should be dispatched to audio
>> + * user*/
>> + err = find_bt_audio_user(h4_channel, dev, skb);
>> + } else if (h4_channel == chan->gnss_channel) {
>> + *dev = users->gnss;
>> + } else if (h4_channel == chan->fm_radio_channel) {
>> + *dev = users->fm_radio;
>> + /* Check if it's an event generated by previously sent audio
>> + * user command. If so then that event should be dispatched to
>> + * audio user */
>> + err = find_fm_audio_user(h4_channel, dev, skb);
>> + } else if (h4_channel == chan->debug_channel) {
>> + *dev = users->debug;
>> + } else if (h4_channel == chan->ste_tools_channel) {
>> + *dev = users->ste_tools;
>> + } else if (h4_channel == chan->hci_logger_channel) {
>> + *dev = users->hci_logger;
>> + } else if (h4_channel == chan->us_ctrl_channel) {
>> + *dev = users->us_ctrl;
>> + } else if (h4_channel == chan->core_channel) {
>> + *dev = users->core;
>> + } else {
>> + *dev = NULL;
>> + CG2900_ERR("Bad H:4 channel supplied: 0x%X", h4_channel);
>> + return -EINVAL;
>> + }
>> +
>> + return err;
>> +}
>
> You seem to have a number of functions that need to go through each
> possible user/channel/submodule. This looks like somethin is not
> abstracted well enough.
>
This is part of what I stated in the cover letter "Future
development". We will move this functionality out to each chip handler
file. I agree that current code is not the perfect abstraction
level... ;-)
>> +
>> +/**
>> + * add_h4_user() - Add H4 user to user storage based on supplied H4 channel.
>> + * @dev: Stored CG2900 device.
>> + * @name: Device name to identify different devices that are using
>> + * the same H4 channel.
>> + *
>> + * Returns:
>> + * 0 if there is no error.
>> + * -EINVAL if NULL pointer or bad channel is supplied.
>> + * -EBUSY if there already is a user for supplied channel.
>> + */
>> +static int add_h4_user(struct cg2900_device *dev, const char * const name)
>> +{
>
> Where does that name come from? Why not just use an enum if the name is
> only use for strncmp?
>
At a point in time we used to have an enum, but from what I can see it
is easier to keep an API stable if you use name lookup instead. If we
want to have minimal changes to the API in the future this is quite a
flexible solution, but this code should be moved to each respective
chip handler.
>> +static ssize_t test_char_dev_read(struct file *filp, char __user *buf,
>> + size_t count, loff_t *f_pos)
>> +{
>> + struct sk_buff *skb;
>> + int bytes_to_copy;
>> + int err;
>> + struct sk_buff_head *rx_queue = &core_info->test_char_dev->rx_queue;
>
> What is this read/write interface used for?
>
> The name suggests that this is only for testing and not an essential
> part of your driver. Could this be made a separate driver?
>
> It also looks like you do socket communication here, can't you use
> a proper AF_BLUETOOTH socket to do the same?
>
This interface can be used for module testing where you can have a
test engine in user space that acts as a chip. Yes, it could be made
into a separate driver but it would be a very small driver. Don't know
if it will be worth having it in a separate file...
We use the sk_buffer, which I guess is the reason that you mention
sockets. We could of course use a socket instead of a char dev, both
here and in the cg2900_char_dev file (which of course then would be
renamed). It was just a choice we made during development. We think
that the transport as such is more like a streaming interface, but I
see no real problem to have sockets. We already have several users
using char dev though so I would prefer to keep char dev rather than
switching to sockets unless you have a strong reason for this.
I see no reason to use AF_BLUETOOTH though, the transport is not only
for Bluetooth.
>> + CG2900_INFO("test_char_dev_read");
>> +
>> + if (skb_queue_empty(rx_queue))
>> + wait_event_interruptible(char_wait_queue,
>> + !(skb_queue_empty(rx_queue)));
>> +
>> + skb = skb_dequeue(rx_queue);
>> + if (!skb) {
>> + CG2900_INFO("skb queue is empty - return with zero bytes");
>> + bytes_to_copy = 0;
>> + goto finished;
>> + }
>> +
>> + bytes_to_copy = min(count, skb->len);
>> + err = copy_to_user(buf, skb->data, bytes_to_copy);
>> + if (err) {
>> + skb_queue_head(rx_queue, skb);
>> + return -EFAULT;
>> + }
>> +
>> + skb_pull(skb, bytes_to_copy);
>> +
>> + if (skb->len > 0)
>> + skb_queue_head(rx_queue, skb);
>> + else
>> + kfree_skb(skb);
>> +
>> +finished:
>> + return bytes_to_copy;
>> +}
>
> This does not handle short/continued reads.
>
As I mentioned above this interface is used for testing and we
therefore have some restriction of usage. I don't think we need to
impose all things necessary for a full interface upon it.
>> +EXPORT_SYMBOL(cg2900_reset);
>
> How about making your symbols EXPORT_SYMBOL_GPL instead of EXPORT_SYMBOL?
>
I have no problems with this. I will check with our company policy
first though before I give a final answer.
But as far as I can see we can use EXPORT_SYMBOL_GPL.
>> +module_param(cg2900_debug_level, int, S_IRUGO | S_IWUSR | S_IWGRP);
>> +MODULE_PARM_DESC(cg2900_debug_level,
>> + "Debug level. Default 1. Possible values:\n"
>> + "\t0 = No debug\n"
>> + "\t1 = Error prints\n"
>> + "\t10 = General info, e.g. function entries\n"
>> + "\t20 = Debug info, e.g. steps in a functionality\n"
>> + "\t25 = Data info, i.e. prints when data is transferred\n"
>> + "\t30 = Data content, i.e. contents of the transferred data");
>
> Please don't introduce new debugging frameworks, we have enough of them
> already. Syslog handles different levels of output just fine, so just
> remove all your debugging stuff and use dev_dbg, dev_info, etc to print
> messages about the device if you must.
>
> Many messages can probably be removed entirely.
>
I have already received comments for this so this will be remade, or
rather removed...
>> +#define CG2900_DEFAULT_DEBUG_LEVEL 1
>> +
>> +/* module_param declared in cg2900_core.c */
>> +extern int cg2900_debug_level;
>> +
>> +#if defined(NDEBUG) || CG2900_DEFAULT_DEBUG_LEVEL == 0
>> + #define CG2900_DBG_DATA_CONTENT(__prefix, __buf, __len)
>> + #define CG2900_DBG_DATA(fmt, arg...)
>> + #define CG2900_DBG(fmt, arg...)
>> + #define CG2900_INFO(fmt, arg...)
>> + #define CG2900_ERR(fmt, arg...)
>> +#else
>> +
>> + #define CG2900_DBG_DATA_CONTENT(__prefix, __buf, __len) \
>> + do { \
>> + if (cg2900_debug_level >= 30) \
>> + print_hex_dump_bytes("CG2900 " __prefix ": " , \
>> + DUMP_PREFIX_NONE, __buf, __len); \
>> + } while (0)
>> +
>> + #define CG2900_DBG_DATA(fmt, arg...) \
>> + do { \
>> + if (cg2900_debug_level >= 25) \
>> + pr_debug("CG2900 %s: " fmt "\n" , __func__ , ## arg); \
>> + } while (0)
>> +
>> + #define CG2900_DBG(fmt, arg...) \
>> + do { \
>> + if (cg2900_debug_level >= 20) \
>> + pr_debug("CG2900 %s: " fmt "\n" , __func__ , ## arg); \
>> + } while (0)
>> +
>> + #define CG2900_INFO(fmt, arg...) \
>> + do { \
>> + if (cg2900_debug_level >= 10) \
>> + pr_info("CG2900: " fmt "\n" , ## arg); \
>> + } while (0)
>> +
>> + #define CG2900_ERR(fmt, arg...) \
>> + do { \
>> + if (cg2900_debug_level >= 1) \
>> + pr_err("CG2900 %s: " fmt "\n" , __func__ , ## arg); \
>> + } while (0)
>> +
>> +#endif /* NDEBUG */
>
> You'll feel relieved when all this is gone...
>
The only thing is it's been pretty nice to have it, but I will remove it.
Is it OK to keep defines so we can have "CG2900" in front and "\n"
after the text?
>> +
>> +#ifndef _BLUETOOTH_DEFINES_H_
>> +#define _BLUETOOTH_DEFINES_H_
>> +
>> +#include <linux/types.h>
>> +
>> +/* H:4 offset in an HCI packet */
>> +#define HCI_H4_POS 0
>> +#define HCI_H4_SIZE 1
>> +
>> +/* Standardized Bluetooth H:4 channels */
>> +#define HCI_BT_CMD_H4_CHANNEL 0x01
>> +#define HCI_BT_ACL_H4_CHANNEL 0x02
>> +#define HCI_BT_SCO_H4_CHANNEL 0x03
>> +#define HCI_BT_EVT_H4_CHANNEL 0x04
>> +
>> +/* Bluetooth Opcode Group Field (OGF) */
>> +#define HCI_BT_OGF_LINK_CTRL 0x01
>> +#define HCI_BT_OGF_LINK_POLICY 0x02
>> +#define HCI_BT_OGF_CTRL_BB 0x03
>> +#define HCI_BT_OGF_LINK_INFO 0x04
>> +#define HCI_BT_OGF_LINK_STATUS 0x05
>> +#define HCI_BT_OGF_LINK_TESTING 0x06
>> +#define HCI_BT_OGF_VS 0x3F
>
> These all look like generic bluetooth definitions, not cg2900
> specific. Should they be part of global headers? Are they perhaps
> already?
>
> Arnd
>
The H4 defines are standardized values and can be in a common h-file.
I don't know if they should be in e.g. include/net/bluetooth/hci.h,
but there are no other H4 defines there and I'm not sure if they would
fit there. Regarding the last defines (the OGFs) they should fit in
hci.h, but I don't know if anyone else would use them. I'm not
actually sure that we use them ourselves anymore so we might be able
to just remove them.
In short, I will check this up and change as needed.
^ permalink raw reply
* Re: [PATCH 1/2 v2] Bluetooth: Fix system crash caused by del_timer()
From: haijun liu @ 2010-10-26 7:35 UTC (permalink / raw)
To: shy; +Cc: Gustavo F. Padovan, Haijun Liu, linux-bluetooth
In-Reply-To: <AANLkTin+dNkjySQBvCSLK9f5aRF9445UqjhXaNvKWSz_@mail.gmail.com>
Hi Shy,
> You can check the sk in list l2cap_sk_list.
If we use global lock, in case of mp env., we have to keep it locked
before exiting l2cap_monitor_timeout().
I don't know whether it can be accepted.
--
Haijun Liu
^ permalink raw reply
* [PATCH] Add separate queries for listing size in PBAP
From: Radoslaw Jablonski @ 2010-10-26 7:42 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Radoslaw Jablonski
Previosly to get phonebook-size or call history size, full phonebook
query was used and a lot of unnecessary operations need to be done on
that returned data. Now separate query is used to get size of each
listing so performance for "getPhonebookSize" operations improved a lot.
---
plugins/phonebook-tracker.c | 107 ++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 105 insertions(+), 2 deletions(-)
diff --git a/plugins/phonebook-tracker.c b/plugins/phonebook-tracker.c
index a6e21f8..1919698 100644
--- a/plugins/phonebook-tracker.c
+++ b/plugins/phonebook-tracker.c
@@ -45,6 +45,7 @@
#define TRACKER_DEFAULT_CONTACT_ME "http://www.semanticdesktop.org/ontologies/2007/03/22/nco#default-contact-me"
#define CONTACTS_ID_COL 38
#define PULL_QUERY_COL_AMOUNT 39
+#define COUNT_QUERY_COL_AMOUNT 1
#define COL_HOME_NUMBER 0
#define COL_HOME_EMAIL 7
#define COL_WORK_NUMBER 8
@@ -580,6 +581,61 @@
"<%s> nco:hasPhoneNumber ?t . " \
"} "
+#define CONTACTS_COUNT_QUERY \
+ "SELECT COUNT(?c) " \
+ "WHERE {" \
+ "?c a nco:PersonContact ." \
+ "FILTER (regex(str(?c), \"contact:\") || " \
+ "regex(str(?c), \"nco#default-contact-me\"))" \
+ "}"
+
+#define MISSED_CALLS_COUNT_QUERY \
+ "SELECT COUNT(?call) WHERE {" \
+ "?c a nco:Contact ;" \
+ "nco:hasPhoneNumber ?h ." \
+ "?call a nmo:Call ;" \
+ "nmo:isSent false ;" \
+ "nmo:from ?c ;" \
+ "nmo:isAnswered false ." \
+ "}"
+
+
+#define INCOMING_CALLS_COUNT_QUERY \
+ "SELECT COUNT(?call) WHERE {" \
+ "?c a nco:Contact ;" \
+ "nco:hasPhoneNumber ?h ." \
+ "?call a nmo:Call ;" \
+ "nmo:isSent false ;" \
+ "nmo:from ?c ;" \
+ "nmo:isAnswered true ." \
+ "}"
+
+#define OUTGOING_CALLS_COUNT_QUERY \
+ "SELECT COUNT(?call) WHERE {" \
+ "?c a nco:Contact ;" \
+ "nco:hasPhoneNumber ?h ." \
+ "?call a nmo:Call ;" \
+ "nmo:isSent true ;" \
+ "nmo:to ?c ." \
+ "}"
+
+
+#define COMBINED_CALLS_COUNT_QUERY \
+ "SELECT COUNT(?call) WHERE {" \
+ "{" \
+ "?c a nco:Contact ;" \
+ "nco:hasPhoneNumber ?h ." \
+ "?call a nmo:Call ;" \
+ "nmo:isSent true ;" \
+ "nmo:to ?c ." \
+ "}UNION {" \
+ "?c a nco:Contact ;" \
+ "nco:hasPhoneNumber ?h ." \
+ "?call a nmo:Call ;" \
+ "nmo:from ?c ." \
+ "}" \
+ "}"
+
typedef void (*reply_list_foreach_t) (char **reply, int num_fields,
void *user_data);
@@ -634,6 +690,22 @@ static const char *name2query(const char *name)
return NULL;
}
+static const char *name2count_query(const char *name)
+{
+ if (g_str_equal(name, "telecom/pb.vcf"))
+ return CONTACTS_COUNT_QUERY;
+ else if (g_str_equal(name, "telecom/ich.vcf"))
+ return INCOMING_CALLS_COUNT_QUERY;
+ else if (g_str_equal(name, "telecom/och.vcf"))
+ return OUTGOING_CALLS_COUNT_QUERY;
+ else if (g_str_equal(name, "telecom/mch.vcf"))
+ return MISSED_CALLS_COUNT_QUERY;
+ else if (g_str_equal(name, "telecom/cch.vcf"))
+ return COMBINED_CALLS_COUNT_QUERY;
+
+ return NULL;
+}
+
static gboolean folder_is_valid(const char *folder)
{
if (folder == NULL)
@@ -1023,6 +1095,26 @@ static GString *gen_vcards(GSList *contacts,
return vcards;
}
+static void pull_contacts_size (char **reply, int num_fields, void *user_data)
+{
+ struct phonebook_data *data = user_data;
+
+ if (num_fields < 0) {
+ data->cb(NULL, 0, num_fields, 0, data->user_data);
+ goto fail;
+ }
+
+ if (reply != NULL) {
+ data->index = atoi(reply[0]);
+ return;
+ }
+
+ data->cb(NULL, 0, data->index, 0, data->user_data);
+
+fail:
+ g_free(data);
+}
+
static void pull_contacts(char **reply, int num_fields, void *user_data)
{
struct phonebook_data *data = user_data;
@@ -1285,10 +1377,21 @@ int phonebook_pull(const char *name, const struct apparam_field *params,
{
struct phonebook_data *data;
const char *query;
+ reply_list_foreach_t pull_cb;
+ int col_amount;
DBG("name %s", name);
- query = name2query(name);
+ if (params->maxlistcount == 0) {
+ query = name2count_query(name);
+ col_amount = COUNT_QUERY_COL_AMOUNT;
+ pull_cb = pull_contacts_size;
+ } else {
+ query = name2query(name);
+ col_amount = PULL_QUERY_COL_AMOUNT;
+ pull_cb = pull_contacts;
+ }
+
if (query == NULL)
return -ENOENT;
@@ -1297,7 +1400,7 @@ int phonebook_pull(const char *name, const struct apparam_field *params,
data->user_data = user_data;
data->cb = cb;
- return query_tracker(query, PULL_QUERY_COL_AMOUNT, pull_contacts, data);
+ return query_tracker(query, col_amount, pull_cb, data);
}
int phonebook_get_entry(const char *folder, const char *id,
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH 2/6] Bluetooth: Add LE connect support
From: Ville Tervo @ 2010-10-26 7:55 UTC (permalink / raw)
To: ext Anderson Lizardo; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <AANLkTi=3VB-q-H6vcTAMAC1hYdYVJiF9wVOBKchYEktt@mail.gmail.com>
Hi,
On Mon, Oct 25, 2010 at 10:33:05PM +0200, ext Anderson Lizardo wrote:
> Hi,
>
> On Mon, Oct 25, 2010 at 8:21 AM, Ville Tervo <ville.tervo@nokia.com> wrote:
> > diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> > index 0b1e460..0944c0c 100644
> > --- a/net/bluetooth/hci_conn.c
> > +++ b/net/bluetooth/hci_conn.c
> > @@ -45,6 +45,32 @@
> > #include <net/bluetooth/bluetooth.h>
> > #include <net/bluetooth/hci_core.h>
> >
> > +void hci_le_connect(struct hci_conn *conn)
>
> This function could be made static, right? (just noticed this now)
Yes. Thanks.
>
> > +{
> > + struct hci_dev *hdev = conn->hdev;
> > + struct hci_cp_le_create_conn cp;
> > +
> > + conn->state = BT_CONNECT;
> > + conn->out = 1;
> > +
> > + memset(&cp, 0, sizeof(cp));
> > + cp.scan_interval = cpu_to_le16(0x0004);
> > + cp.scan_window = cpu_to_le16(0x0004);
> > + bacpy(&cp.peer_addr, &conn->dst);
> > + cp.conn_interval_min = cpu_to_le16(0x0008);
> > + cp.conn_interval_max = cpu_to_le16(0x0100);
> > + cp.supervision_timeout = cpu_to_le16(0x0064);
> > + cp.min_ce_len = cpu_to_le16(0x0001);
> > + cp.max_ce_len = cpu_to_le16(0x0001);
> > +
> > + hci_send_cmd(hdev, HCI_OP_LE_CREATE_CONN, sizeof(cp), &cp);
> > +}
--
Ville
^ permalink raw reply
* [PATCH 1/2] Remove unnecessary empty line in phonebook-tracker
From: Radoslaw Jablonski @ 2010-10-26 8:29 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Radoslaw Jablonski
---
plugins/phonebook-tracker.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/plugins/phonebook-tracker.c b/plugins/phonebook-tracker.c
index a6e21f8..277290c 100644
--- a/plugins/phonebook-tracker.c
+++ b/plugins/phonebook-tracker.c
@@ -531,7 +531,6 @@
"}" \
"} GROUP BY ?call ORDER BY DESC(nmo:receivedDate(?call))"
-
#define CONTACTS_QUERY_FROM_URI \
"SELECT ?v nco:fullname(<%s>) " \
"nco:nameFamily(<%s>) nco:nameGiven(<%s>) " \
--
1.7.0.4
^ permalink raw reply related
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