* RE: [RFC] Interface to set LE connection parameters
From: Mike Tsai @ 2010-11-15 18:15 UTC (permalink / raw)
To: Ville Tervo, linux-bluetooth@vger.kernel.org
In-Reply-To: <20101115120632.GB16464@null>
Hi Ville,
-----Original Message-----
From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-owner@vger.kernel.org] On Behalf Of Ville Tervo
Sent: Monday, November 15, 2010 4:07 AM
To: linux-bluetooth@vger.kernel.org
Subject: [RFC] Interface to set LE connection parameters
Hi,
LE profiles have different requirements for connection parameters. Mainly
trying to balance between power consumption and latencies. Probably more will
factors will be in future.
Currently I have plan to introduce new l2cap socket option which can be used
before connection creation to set inital settings and also change settings
while having a connection.
Since there is no equivalents in EDR/BR connection I'm planning to make then
apply only LE connection.
Other question which parameters should be exposed to user space? Connection
creation and connection update have these common parameters. Connection
creation has in addition some other parameters also but they should be handled
in some other way.
__le16 conn_interval_min;
__le16 conn_interval_max;
__le16 conn_latency;
__le16 supervision_timeout;
__le16 min_ce_len;
__le16 max_ce_len;
So far I have had two ideas. The first is to simply expose all these fields
with sock_opt. In that way profiles would be able to define their requirements
also in future without any sock opt changes.
Second is to define BT_LE_LOW_LAT for low latency connection requirements and
BT_LE_LOW_POWER connection where the latency is not an issue. It would make
usage of this sock opt interface simplier. OTOH the only user should be
bluetoothd so it doesn't need to be as simple as possible.
Comments please.
[MTsai] - how about following parameters,
Scan internal,
Scan window,
Peer address type,
Thanks,
Mike
--
Ville
--
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/4] Sim Access Profile API
From: Waldemar.Rymarkiewicz @ 2010-11-15 17:29 UTC (permalink / raw)
To: luiz.dentz, padovan
Cc: marcel, linux-bluetooth, suraj, johan.hedberg, joakim.xj.ceder
In-Reply-To: <AANLkTino6tJbpEMWxz-PLBmOyb8=3KCr1uDm9dxSbp98@mail.gmail.com>
Hi,=20
>Hi,
>>> >> + =A0 =A0 =A0 =A0 =A0void Disconnect(boolean type)
>>> >> +
>>> >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0Disconnect SAP client from the =
server.
>>> >The 'type'
>>> >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0parameter indicates disconnecti=
on type.
>>> >> +
>>> >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0True =A0- gracefull disconnecti=
on
>>> >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0False - immediate disconnection
>>> >> +
>>> >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0Possible errors: org.bluez.Erro=
r.Failed
>>> >
>>> >I don't like this style of method names at all. Using method names=20
>>> >like GracefulDisconnect and ImmediateDisconnect would be better.
>>>
>>> That's fine.
>>>
>>> >However I am not sure we should differentiate here at all.=20
>We should=20
>>> >always to the graceful disconnect. What will the immediate=20
>>> >disconnect bring us?
>>>
>>> That's actually intended for testing only. One of PTS test=20
>cases expects the tester to trigger immediate disconnect.
>>> In practce, it is only used when connection to sim card is=20
>lost, but this is obviously done internally.
>>
>>
>> So this shouldn't be in the API, no one is going to use it. You can=20
>> create something internally for the immediate disconnection that you=20
>> go and set manually inside the code. Make sure to comment in=20
>the code=20
>> why you are adding this there. That can also be a option in some of=20
>> the bluetooth config files. Let's see what others think here.
>
>Actually this looks a lot like virtual cable unplug that PTS=20
>wants to hid, it is just crap that nobody use, so maybe e.g.
>--enable-pts-bullshit would actually make sense to activate=20
>the nonsense that pts wants, well it is still annoying to=20
>recompile just to run PTS tests. For hid the virtual cable=20
>unplug can be emulate via RemoveDevice, so maybe it make sense=20
>to do a immediate disconnect in such case for sap too, but=20
>this is still inconvenient since you have to repair after doing it.
>
So, simply let's get rid of immediate disconnect from sap api and leave if =
for sap-driver (sap-*.c file ) provider. I could extend sap-dummy api just =
for an example.
Service org.bluez=20
Interface org.bluez.SimAccess=20
Object path [variable prefix]/{hci0,hci1,...}
Methods void Disconnect()
Disconnects SAP client from the server.
Possible errors: org.bluez.Error.Failed
void SetProperty(string name, variant value)
Changes the value of the specified property. Only
properties that are listed a read-write are changeable.
Possible Errors: org.bluez.Error.DoesNotExist=20
org.bluez.Error.InvalidArguments
dict GetProperties()
Return all properties for the interface. See the
properties section for available properties.
Possible Errors: org.bluez.Error.Failed
Signals PropertyChanged(string name, variant value)
This signal indicates a changed value of the given
property.
Properties boolean Enabled [readwrite]
Set to true to start-up SAP server and register SDP record for=20
it. Set to false to shutdown SAP server and remove the SDP record.
boolean Connected [readonly]
Indicates if SAP client is connected to the server.=20
More comments ?
Thanks,
/Waldek
^ permalink raw reply
* Re: [RFC] Interface to set LE connection parameters
From: Ville Tervo @ 2010-11-15 15:17 UTC (permalink / raw)
To: ext tim.howes@accenture.com; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <1AFE20D16950C745A2A83986B72E8748011F571E6CE6@EMEXM3131.dir.svc.accenture.com>
Hi Tim,
Please stop top posting on this ml.
On Mon, Nov 15, 2010 at 03:24:29PM +0100, ext tim.howes@accenture.com wrote:
> Hi Ville,
>
> As you note the different profiles would likely have different connection parameters. The different profiles may be running on the same LE link (indeed the same L2CAP [fixed] channel).
>
I guess the latency should override power requirements. Low power profile can
operate on low latency link but low latency profile fails on high latency. Of
course this gets much more complicated if there are more requirements.
Are these (latency and power) the only characteristics we need to deal with.
There might be some also. I'm not too familiar with profile drafts.
> Do you have a view on how the different profiles - on the same link - would have different requests arbitrated, and where that arbitration would be done? I'd imagine that the API towards the profiles should be of the abstract form - such as you mention (eg BT_LE_LOW_LAT). This would make it easier to arbitrate the different requests, as compared to if the profiles see an API of the "numerical" form (eg interval = N ms). I guess the arbitration could happen in user or kernel space; as long as there is something with singleton-like semantics to do it.
>
I think I need to get more details from profile specs and try to find out the
requirements from them.
Right now I'm trying to find out what would be the right interface from kernel
to user space.
> Also - a quick observation: yes, the connection parameters are only for LE links; however they have some semantic similarity with sniff mode in BR. Especially in the abstract form ("Low Latency" verus "Low Power"). Does BlueZ present an API like this already for BR (I'm new to BlueZ...I was more of a Symbian guy ;)? If not it might be worth having one API for both BR/Sniff and LE/Connection Parameters - and do the appropriate mapping "under the hood".
Actually this has been discussed earlier and we need also some api to change
sniff behaviour. Reason for this is broken devices which does not turn on
normal mode when low latency is needed. I guess Jaikumar had some patches
regarding this?
--
Ville
^ permalink raw reply
* RE: [RFC] Interface to set LE connection parameters
From: tim.howes @ 2010-11-15 14:24 UTC (permalink / raw)
To: ville.tervo, linux-bluetooth
In-Reply-To: <20101115120632.GB16464@null>
Hi Ville,
As you note the different profiles would likely have different connection parameters. The different profiles may be running on the same LE link (indeed the same L2CAP [fixed] channel).
Do you have a view on how the different profiles - on the same link - would have different requests arbitrated, and where that arbitration would be done? I'd imagine that the API towards the profiles should be of the abstract form - such as you mention (eg BT_LE_LOW_LAT). This would make it easier to arbitrate the different requests, as compared to if the profiles see an API of the "numerical" form (eg interval = N ms). I guess the arbitration could happen in user or kernel space; as long as there is something with singleton-like semantics to do it.
Also - a quick observation: yes, the connection parameters are only for LE links; however they have some semantic similarity with sniff mode in BR. Especially in the abstract form ("Low Latency" verus "Low Power"). Does BlueZ present an API like this already for BR (I'm new to BlueZ...I was more of a Symbian guy ;)? If not it might be worth having one API for both BR/Sniff and LE/Connection Parameters - and do the appropriate mapping "under the hood".
Cheers,
Tim
-----Original Message-----
From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-owner@vger.kernel.org] On Behalf Of Ville Tervo
Sent: 15 November 2010 12:07
To: linux-bluetooth@vger.kernel.org
Subject: [RFC] Interface to set LE connection parameters
Hi,
LE profiles have different requirements for connection parameters. Mainly
trying to balance between power consumption and latencies. Probably more will
factors will be in future.
Currently I have plan to introduce new l2cap socket option which can be used
before connection creation to set inital settings and also change settings
while having a connection.
Since there is no equivalents in EDR/BR connection I'm planning to make then
apply only LE connection.
Other question which parameters should be exposed to user space? Connection
creation and connection update have these common parameters. Connection
creation has in addition some other parameters also but they should be handled
in some other way.
__le16 conn_interval_min;
__le16 conn_interval_max;
__le16 conn_latency;
__le16 supervision_timeout;
__le16 min_ce_len;
__le16 max_ce_len;
So far I have had two ideas. The first is to simply expose all these fields
with sock_opt. In that way profiles would be able to define their requirements
also in future without any sock opt changes.
Second is to define BT_LE_LOW_LAT for low latency connection requirements and
BT_LE_LOW_POWER connection where the latency is not an issue. It would make
usage of this sock opt interface simplier. OTOH the only user should be
bluetoothd so it doesn't need to be as simple as possible.
Comments please.
--
Ville
--
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
This message is for the designated recipient only and may contain privileged, proprietary, or otherwise private information. If you have received it in error, please notify the sender immediately and delete the original. Any other use of the email by you is prohibited.
^ permalink raw reply
* Re: [PATCH v3 2/4] Advertising data: extract local name
From: Johan Hedberg @ 2010-11-15 14:07 UTC (permalink / raw)
To: Vinicius Costa Gomes; +Cc: linux-bluetooth, Bruna Moreira
In-Reply-To: <1289518769-3009-2-git-send-email-vinicius.gomes@openbossa.org>
Hi,
On Thu, Nov 11, 2010, Vinicius Costa Gomes wrote:
> @@ -3159,6 +3160,12 @@ void adapter_update_device_from_info(struct btd_adapter *adapter,
>
> adapter->found_devices = g_slist_sort(adapter->found_devices,
> (GCompareFunc) dev_rssi_cmp);
> +
> + if (info->length) {
> + char *tmp_name = bt_extract_eir_name(info->data, &type);
> + if (tmp_name)
> + dev->name = tmp_name;
> + }
> }
This looks like a potential memory leak to me. What if dev->name was
already set (e.g. by a previous event for the same device)?
Johan
^ permalink raw reply
* Re: [PATCH v3 1/4] Initial advertising data parsing implementation
From: Johan Hedberg @ 2010-11-15 14:05 UTC (permalink / raw)
To: Vinicius Costa Gomes; +Cc: linux-bluetooth, Bruna Moreira
In-Reply-To: <1289518769-3009-1-git-send-email-vinicius.gomes@openbossa.org>
Hi,
On Thu, Nov 11, 2010, Vinicius Costa Gomes wrote:
> Implement adapter_update_adv() function to parse advertising data
> received by btd_event_adv() function. Add some fields for advertising
> data in remote_device_info struct.
> ---
> plugins/hciops.c | 9 +++------
> src/adapter.c | 26 ++++++++++++++++++++++++++
> src/adapter.h | 6 ++++++
> src/event.c | 13 +++++++++++++
> src/event.h | 1 +
> 5 files changed, 49 insertions(+), 6 deletions(-)
Pushed upstream. Thanks.
Johan
^ permalink raw reply
* [PATCH] Fix fetching contact photo file name
From: Bartosz Szatkowski @ 2010-11-15 13:33 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Bartosz Szatkowski
Previously photo id was fetched. File name may be further converted to
binary data (actual image) if such behavior would be desired in the future.
---
plugins/phonebook-tracker.c | 60 ++++++++++++++++++++++++++++++++++++++----
1 files changed, 54 insertions(+), 6 deletions(-)
diff --git a/plugins/phonebook-tracker.c b/plugins/phonebook-tracker.c
index 672d59f..962cc4d 100644
--- a/plugins/phonebook-tracker.c
+++ b/plugins/phonebook-tracker.c
@@ -69,7 +69,7 @@
"nco:streetAddress(?p) nco:locality(?p) nco:region(?p) " \
"nco:postalcode(?p) nco:country(?p) ?f nco:emailAddress(?ew) " \
"nco:birthDate(?c) nco:nickname(?c) nco:url(?c) " \
- "nco:photo(?c) nco:fullname(?o) nco:department(?a) " \
+ "?file nco:fullname(?o) nco:department(?a) " \
"nco:role(?a) nco:pobox(?pw) nco:extendedAddress(?pw) " \
"nco:streetAddress(?pw) nco:locality(?pw) nco:region(?pw) " \
"nco:postalcode(?pw) nco:country(?pw) nco:contactUID(?c) " \
@@ -80,6 +80,10 @@
"\"NOTACALL\" \"false\" \"false\" ?c " \
"WHERE { " \
"?c a nco:PersonContact . " \
+ "OPTIONAL { " \
+ "?c a nco:PersonContact ; nco:photo ?pht . " \
+ "?pht a nfo:FileDataObject ; nie:url ?file . " \
+ "} " \
"OPTIONAL { ?c nco:hasPhoneNumber ?h . " \
"OPTIONAL {" \
"?h a nco:FaxNumber ; " \
@@ -135,7 +139,7 @@
"nco:streetAddress(?p) nco:locality(?p) nco:region(?p) " \
"nco:postalcode(?p) nco:country(?p) \"\" nco:emailAddress(?ew) "\
"nco:birthDate(?c) nco:nickname(?c) nco:url(?c) " \
- "nco:photo(?c) nco:fullname(?o) nco:department(?a) " \
+ "?file nco:fullname(?o) nco:department(?a) " \
"nco:role(?a) nco:pobox(?pw) nco:extendedAddress(?pw) " \
"nco:streetAddress(?pw) nco:locality(?pw) nco:region(?pw) " \
"nco:postalcode(?pw) nco:country(?pw) nco:contactUID(?c) " \
@@ -156,6 +160,10 @@
"?c a nco:PersonContact . " \
"?c nco:hasPhoneNumber ?t . " \
"OPTIONAL { " \
+ "?c a nco:PersonContact ; nco:photo ?pht . " \
+ "?pht a nfo:FileDataObject ; nie:url ?file . " \
+ "} " \
+ "OPTIONAL { " \
"?t a nco:CellPhoneNumber ; " \
"nco:phoneNumber ?vc . " \
"} " \
@@ -186,6 +194,10 @@
"?c nco:hasAffiliation ?a . " \
"?a nco:hasPhoneNumber ?tmp . " \
"OPTIONAL { " \
+ "?c a nco:PersonContact ; nco:photo ?pht . " \
+ "?pht a nfo:FileDataObject ; nie:url ?file . " \
+ "} " \
+ "OPTIONAL { " \
"?a rdfs:label \"Work\" . " \
"?tmp nco:phoneNumber ?w . " \
"OPTIONAL { ?a nco:hasEmailAddress ?ew . } " \
@@ -274,7 +286,7 @@
"nco:streetAddress(?p) nco:locality(?p) nco:region(?p) " \
"nco:postalcode(?p) nco:country(?p) \"\" nco:emailAddress(?ew) "\
"nco:birthDate(?c) nco:nickname(?c) nco:url(?c) " \
- "nco:photo(?c) nco:fullname(?o) nco:department(?a) " \
+ "?file nco:fullname(?o) nco:department(?a) " \
"nco:role(?a) nco:pobox(?pw) nco:extendedAddress(?pw) " \
"nco:streetAddress(?pw) nco:locality(?pw) nco:region(?pw) " \
"nco:postalcode(?pw) nco:country(?pw) nco:contactUID(?c) " \
@@ -295,6 +307,10 @@
"?c a nco:PersonContact . " \
"?c nco:hasPhoneNumber ?t . " \
"OPTIONAL { " \
+ "?c a nco:PersonContact ; nco:photo ?pht . " \
+ "?pht a nfo:FileDataObject ; nie:url ?file . " \
+ "} " \
+ "OPTIONAL { " \
"?t a nco:CellPhoneNumber ; " \
"nco:phoneNumber ?vc . " \
"} " \
@@ -325,6 +341,10 @@
"?c nco:hasAffiliation ?a . " \
"?a nco:hasPhoneNumber ?tmp . " \
"OPTIONAL { " \
+ "?c a nco:PersonContact ; nco:photo ?pht . " \
+ "?pht a nfo:FileDataObject ; nie:url ?file . " \
+ "} " \
+ "OPTIONAL { " \
"?a rdfs:label \"Work\" . " \
"?tmp nco:phoneNumber ?w . " \
"OPTIONAL { ?a nco:hasEmailAddress ?ew . } " \
@@ -412,7 +432,7 @@
"nco:streetAddress(?p) nco:locality(?p) nco:region(?p) " \
"nco:postalcode(?p) nco:country(?p) \"\" nco:emailAddress(?ew) "\
"nco:birthDate(?c) nco:nickname(?c) nco:url(?c) " \
- "nco:photo(?c) nco:fullname(?o) nco:department(?a) " \
+ "?file nco:fullname(?o) nco:department(?a) " \
"nco:role(?a) nco:pobox(?pw) nco:extendedAddress(?pw) " \
"nco:streetAddress(?pw) nco:locality(?pw) nco:region(?pw) " \
"nco:postalcode(?pw) nco:country(?pw) nco:contactUID(?c) " \
@@ -432,6 +452,10 @@
"?c a nco:PersonContact . " \
"?c nco:hasPhoneNumber ?t . " \
"OPTIONAL { " \
+ "?c a nco:PersonContact ; nco:photo ?pht . " \
+ "?pht a nfo:FileDataObject ; nie:url ?file . " \
+ "} " \
+ "OPTIONAL { " \
"?t a nco:CellPhoneNumber ; " \
"nco:phoneNumber ?vc . " \
"} " \
@@ -461,6 +485,10 @@
"?c nco:hasAffiliation ?a . " \
"?a nco:hasPhoneNumber ?tmp . " \
"OPTIONAL { " \
+ "?c a nco:PersonContact ; nco:photo ?pht . " \
+ "?pht a nfo:FileDataObject ; nie:url ?file . " \
+ "} " \
+ "OPTIONAL { " \
"?a rdfs:label \"Work\" . " \
"?tmp nco:phoneNumber ?w . " \
"OPTIONAL { ?a nco:hasEmailAddress ?ew . } " \
@@ -544,7 +572,7 @@
"nco:streetAddress(?p) nco:locality(?p) nco:region(?p) " \
"nco:postalcode(?p) nco:country(?p) \"\" nco:emailAddress(?ew) "\
"nco:birthDate(?c) nco:nickname(?c) nco:url(?c) " \
- "nco:photo(?c) nco:fullname(?o) nco:department(?a) " \
+ "?file nco:fullname(?o) nco:department(?a) " \
"nco:role(?a) nco:pobox(?pw) nco:extendedAddress(?pw) " \
"nco:streetAddress(?pw) nco:locality(?pw) nco:region(?pw) " \
"nco:postalcode(?pw) nco:country(?pw) nco:contactUID(?c) " \
@@ -564,6 +592,10 @@
"?c a nco:PersonContact . " \
"?c nco:hasPhoneNumber ?t . " \
"OPTIONAL { " \
+ "?c a nco:PersonContact ; nco:photo ?pht . " \
+ "?pht a nfo:FileDataObject ; nie:url ?file . " \
+ "} " \
+ "OPTIONAL { " \
"?t a nco:CellPhoneNumber ; " \
"nco:phoneNumber ?vc . " \
"} " \
@@ -593,6 +625,10 @@
"?c nco:hasAffiliation ?a . " \
"?a nco:hasPhoneNumber ?tmp . " \
"OPTIONAL { " \
+ "?c a nco:PersonContact ; nco:photo ?pht . " \
+ "?pht a nfo:FileDataObject ; nie:url ?file . " \
+ "} " \
+ "OPTIONAL { " \
"?a rdfs:label \"Work\" . " \
"?tmp nco:phoneNumber ?w . " \
"OPTIONAL { ?a nco:hasEmailAddress ?ew . } " \
@@ -641,6 +677,10 @@
"?c a nco:PersonContact . " \
"?c nco:hasPhoneNumber ?t . " \
"OPTIONAL { " \
+ "?c a nco:PersonContact ; nco:photo ?pht . " \
+ "?pht a nfo:FileDataObject ; nie:url ?file . " \
+ "} " \
+ "OPTIONAL { " \
"?t a nco:CellPhoneNumber ; " \
"nco:phoneNumber ?vc . " \
"} " \
@@ -670,6 +710,10 @@
"?c nco:hasAffiliation ?a . " \
"?a nco:hasPhoneNumber ?tmp . " \
"OPTIONAL { " \
+ "?c a nco:PersonContact ; nco:photo ?pht . " \
+ "?pht a nfo:FileDataObject ; nie:url ?file . " \
+ "} " \
+ "OPTIONAL { " \
"?a rdfs:label \"Work\" . " \
"?tmp nco:phoneNumber ?w . " \
"OPTIONAL { ?a nco:hasEmailAddress ?ew . } " \
@@ -775,7 +819,7 @@
"nco:streetAddress(?p) nco:locality(?p) nco:region(?p) " \
"nco:postalcode(?p) nco:country(?p) ?f nco:emailAddress(?ew) " \
"nco:birthDate(<%s>) nco:nickname(<%s>) nco:url(<%s>) " \
- "nco:photo(<%s>) nco:fullname(?o) nco:department(?a) " \
+ "?file nco:fullname(?o) nco:department(?a) " \
"nco:role(?a) nco:pobox(?pw) nco:extendedAddress(?pw) " \
"nco:streetAddress(?pw) nco:locality(?pw) nco:region(?pw) " \
"nco:postalcode(?pw) nco:country(?pw) nco:contactUID(<%s>) " \
@@ -786,6 +830,10 @@
"\"NOTACALL\" \"false\" \"false\" <%s> " \
"WHERE { " \
"<%s> a nco:PersonContact . " \
+ "OPTIONAL { " \
+ "<%s> a nco:PersonContact ; nco:photo ?pht . " \
+ "?pht a nfo:FileDataObject ; nie:url ?file . " \
+ "} " \
"OPTIONAL { <%s> nco:hasPhoneNumber ?h . " \
"OPTIONAL {" \
"?h a nco:FaxNumber ; " \
--
1.7.0.4
^ permalink raw reply related
* [RFC] Interface to set LE connection parameters
From: Ville Tervo @ 2010-11-15 12:06 UTC (permalink / raw)
To: linux-bluetooth
Hi,
LE profiles have different requirements for connection parameters. Mainly
trying to balance between power consumption and latencies. Probably more will
factors will be in future.
Currently I have plan to introduce new l2cap socket option which can be used
before connection creation to set inital settings and also change settings
while having a connection.
Since there is no equivalents in EDR/BR connection I'm planning to make then
apply only LE connection.
Other question which parameters should be exposed to user space? Connection
creation and connection update have these common parameters. Connection
creation has in addition some other parameters also but they should be handled
in some other way.
__le16 conn_interval_min;
__le16 conn_interval_max;
__le16 conn_latency;
__le16 supervision_timeout;
__le16 min_ce_len;
__le16 max_ce_len;
So far I have had two ideas. The first is to simply expose all these fields
with sock_opt. In that way profiles would be able to define their requirements
also in future without any sock opt changes.
Second is to define BT_LE_LOW_LAT for low latency connection requirements and
BT_LE_LOW_POWER connection where the latency is not an issue. It would make
usage of this sock opt interface simplier. OTOH the only user should be
bluetoothd so it doesn't need to be as simple as possible.
Comments please.
--
Ville
^ permalink raw reply
* [PATCH 2/2] Split pull_contacts to smaller functions
From: Bartosz Szatkowski @ 2010-11-15 12:01 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Bartosz Szatkowski
In-Reply-To: <1289822497-5338-1-git-send-email-bulislaw@linux.com>
Parts of pull_contact responsible for filling contact fields moved
to new functions.
---
plugins/phonebook-tracker.c | 159 ++++++++++++++++++++++++------------------
1 files changed, 91 insertions(+), 68 deletions(-)
diff --git a/plugins/phonebook-tracker.c b/plugins/phonebook-tracker.c
index 8f8a6e2..e9fc3ab 100644
--- a/plugins/phonebook-tracker.c
+++ b/plugins/phonebook-tracker.c
@@ -1395,6 +1395,92 @@ static void add_affiliation(char **field, const char *value)
*field = g_strdup(value);
}
+static void contact_init(struct phonebook_contact **contact, char **reply)
+{
+ if (*contact == NULL)
+ *contact = g_new0(struct phonebook_contact, 1);
+
+ (*contact)->fullname = g_strdup(reply[COL_FULL_NAME]);
+ (*contact)->family = g_strdup(reply[COL_FAMILY_NAME]);
+ (*contact)->given = g_strdup(reply[COL_GIVEN_NAME]);
+ (*contact)->additional = g_strdup(reply[COL_ADDITIONAL_NAME]);
+ (*contact)->prefix = g_strdup(reply[COL_NAME_PREFIX]);
+ (*contact)->suffix = g_strdup(reply[COL_NAME_SUFFIX]);
+ (*contact)->birthday = g_strdup(reply[COL_BIRTH_DATE]);
+ (*contact)->nickname = g_strdup(reply[COL_NICKNAME]);
+ (*contact)->website = g_strdup(reply[COL_URL]);
+ (*contact)->photo = g_strdup(reply[COL_PHOTO]);
+ (*contact)->company = g_strdup(reply[COL_ORG_NAME]);
+ (*contact)->department = g_strdup(reply[COL_ORG_DEPARTMENT]);
+ (*contact)->role = g_strdup(reply[COL_ORG_ROLE]);
+ (*contact)->uid = g_strdup(reply[COL_UID]);
+ (*contact)->title = g_strdup(reply[COL_TITLE]);
+
+ set_call_type(*contact, reply[COL_DATE], reply[COL_SENT],
+ reply[COL_ANSWERED]);
+}
+
+static void contact_add_numbers(struct phonebook_contact **contact, char **reply)
+{
+ add_phone_number(*contact, reply[COL_HOME_NUMBER], TEL_TYPE_HOME);
+ add_phone_number(*contact, reply[COL_WORK_NUMBER], TEL_TYPE_WORK);
+ add_phone_number(*contact, reply[COL_FAX_NUMBER], TEL_TYPE_FAX);
+ add_phone_number(*contact, reply[COL_CELL_NUMBER], TEL_TYPE_MOBILE);
+ if ((g_strcmp0(reply[COL_OTHER_NUMBER], reply[COL_CELL_NUMBER]) != 0) &&
+ (g_strcmp0(reply[COL_OTHER_NUMBER], reply[COL_WORK_NUMBER]) != 0) &&
+ (g_strcmp0(reply[COL_OTHER_NUMBER], reply[COL_HOME_NUMBER]) != 0))
+ add_phone_number(*contact, reply[COL_OTHER_NUMBER], TEL_TYPE_OTHER);
+}
+
+static void contact_add_emails(struct phonebook_contact **contact, char **reply)
+{
+ add_email(*contact, reply[COL_HOME_EMAIL], EMAIL_TYPE_HOME);
+ add_email(*contact, reply[COL_WORK_EMAIL], EMAIL_TYPE_WORK);
+ add_email(*contact, reply[COL_OTHER_EMAIL], EMAIL_TYPE_OTHER);
+}
+
+static void contact_add_addresses(struct phonebook_contact **contact, char **reply)
+{
+
+ char *home_addr, *work_addr, *other_addr;
+
+ home_addr = g_strdup_printf("%s;%s;%s;%s;%s;%s;%s",
+ reply[COL_HOME_ADDR_POBOX], reply[COL_HOME_ADDR_EXT],
+ reply[COL_HOME_ADDR_STREET], reply[COL_HOME_ADDR_LOCALITY],
+ reply[COL_HOME_ADDR_REGION], reply[COL_HOME_ADDR_CODE],
+ reply[COL_HOME_ADDR_COUNTRY]);
+
+ work_addr = g_strdup_printf("%s;%s;%s;%s;%s;%s;%s",
+ reply[COL_WORK_ADDR_POBOX], reply[COL_WORK_ADDR_EXT],
+ reply[COL_WORK_ADDR_STREET], reply[COL_WORK_ADDR_LOCALITY],
+ reply[COL_WORK_ADDR_REGION], reply[COL_WORK_ADDR_CODE],
+ reply[COL_WORK_ADDR_COUNTRY]);
+
+ other_addr = g_strdup_printf("%s;%s;%s;%s;%s;%s;%s",
+ reply[COL_OTHER_ADDR_POBOX], reply[COL_OTHER_ADDR_EXT],
+ reply[COL_OTHER_ADDR_STREET], reply[COL_OTHER_ADDR_LOCALITY],
+ reply[COL_OTHER_ADDR_REGION], reply[COL_OTHER_ADDR_CODE],
+ reply[COL_OTHER_ADDR_COUNTRY]);
+
+ add_address(*contact, home_addr, ADDR_TYPE_HOME);
+ add_address(*contact, work_addr, ADDR_TYPE_WORK);
+ add_address(*contact, other_addr, ADDR_TYPE_OTHER);
+
+ g_free(home_addr);
+ g_free(work_addr);
+ g_free(other_addr);
+}
+
+static void contact_add_organization(struct phonebook_contact **contact, char **reply)
+{
+ /* Adding fields connected by nco:hasAffiliation - they may be in
+ * separate replies */
+ add_affiliation(&(*contact)->title, reply[COL_TITLE]);
+ add_affiliation(&(*contact)->company, reply[COL_ORG_NAME]);
+ add_affiliation(&(*contact)->department, reply[COL_ORG_DEPARTMENT]);
+ add_affiliation(&(*contact)->role, reply[COL_ORG_ROLE]);
+}
+
static void pull_contacts(char **reply, int num_fields, void *user_data)
{
struct phonebook_data *data = user_data;
@@ -1404,7 +1490,6 @@ static void pull_contacts(char **reply, int num_fields, void *user_data)
GString *vcards;
int last_index, i;
gboolean cdata_present = FALSE;
- char *home_addr, *work_addr, *other_addr;
if (num_fields < 0) {
data->cb(NULL, 0, num_fields, 0, data->user_data);
@@ -1450,75 +1535,13 @@ static void pull_contacts(char **reply, int num_fields, void *user_data)
return;
add_entry:
- contact = g_new0(struct phonebook_contact, 1);
- contact->fullname = g_strdup(reply[COL_FULL_NAME]);
- contact->family = g_strdup(reply[COL_FAMILY_NAME]);
- contact->given = g_strdup(reply[COL_GIVEN_NAME]);
- contact->additional = g_strdup(reply[COL_ADDITIONAL_NAME]);
- contact->prefix = g_strdup(reply[COL_NAME_PREFIX]);
- contact->suffix = g_strdup(reply[COL_NAME_SUFFIX]);
- contact->birthday = g_strdup(reply[COL_BIRTH_DATE]);
- contact->nickname = g_strdup(reply[COL_NICKNAME]);
- contact->website = g_strdup(reply[COL_URL]);
- contact->photo = g_strdup(reply[COL_PHOTO]);
- contact->company = g_strdup(reply[COL_ORG_NAME]);
- contact->department = g_strdup(reply[COL_ORG_DEPARTMENT]);
- contact->role = g_strdup(reply[COL_ORG_ROLE]);
- contact->uid = g_strdup(reply[COL_UID]);
- contact->title = g_strdup(reply[COL_TITLE]);
-
- set_call_type(contact, reply[COL_DATE], reply[COL_SENT],
- reply[COL_ANSWERED]);
+ contact_init(&contact, reply);
add_numbers:
- /* Adding phone numbers to contact struct */
- add_phone_number(contact, reply[COL_HOME_NUMBER], TEL_TYPE_HOME);
- add_phone_number(contact, reply[COL_WORK_NUMBER], TEL_TYPE_WORK);
- add_phone_number(contact, reply[COL_FAX_NUMBER], TEL_TYPE_FAX);
- add_phone_number(contact, reply[COL_CELL_NUMBER], TEL_TYPE_MOBILE);
- if ((g_strcmp0(reply[COL_OTHER_NUMBER], reply[COL_CELL_NUMBER]) != 0) &&
- (g_strcmp0(reply[COL_OTHER_NUMBER], reply[COL_WORK_NUMBER]) != 0) &&
- (g_strcmp0(reply[COL_OTHER_NUMBER], reply[COL_HOME_NUMBER]) != 0))
- add_phone_number(contact, reply[COL_OTHER_NUMBER], TEL_TYPE_OTHER);
-
- /* Adding emails */
- add_email(contact, reply[COL_HOME_EMAIL], EMAIL_TYPE_HOME);
- add_email(contact, reply[COL_WORK_EMAIL], EMAIL_TYPE_WORK);
- add_email(contact, reply[COL_OTHER_EMAIL], EMAIL_TYPE_OTHER);
-
- /* Adding addresses */
- home_addr = g_strdup_printf("%s;%s;%s;%s;%s;%s;%s",
- reply[COL_HOME_ADDR_POBOX], reply[COL_HOME_ADDR_EXT],
- reply[COL_HOME_ADDR_STREET], reply[COL_HOME_ADDR_LOCALITY],
- reply[COL_HOME_ADDR_REGION], reply[COL_HOME_ADDR_CODE],
- reply[COL_HOME_ADDR_COUNTRY]);
-
- work_addr = g_strdup_printf("%s;%s;%s;%s;%s;%s;%s",
- reply[COL_WORK_ADDR_POBOX], reply[COL_WORK_ADDR_EXT],
- reply[COL_WORK_ADDR_STREET], reply[COL_WORK_ADDR_LOCALITY],
- reply[COL_WORK_ADDR_REGION], reply[COL_WORK_ADDR_CODE],
- reply[COL_WORK_ADDR_COUNTRY]);
-
- other_addr = g_strdup_printf("%s;%s;%s;%s;%s;%s;%s",
- reply[COL_OTHER_ADDR_POBOX], reply[COL_OTHER_ADDR_EXT],
- reply[COL_OTHER_ADDR_STREET], reply[COL_OTHER_ADDR_LOCALITY],
- reply[COL_OTHER_ADDR_REGION], reply[COL_OTHER_ADDR_CODE],
- reply[COL_OTHER_ADDR_COUNTRY]);
-
- add_address(contact, home_addr, ADDR_TYPE_HOME);
- add_address(contact, work_addr, ADDR_TYPE_WORK);
- add_address(contact, other_addr, ADDR_TYPE_OTHER);
-
- g_free(home_addr);
- g_free(work_addr);
- g_free(other_addr);
-
- /* Adding fields connected by nco:hasAffiliation - they may be in
- * separate replies */
- add_affiliation(&contact->title, reply[COL_TITLE]);
- add_affiliation(&contact->company, reply[COL_ORG_NAME]);
- add_affiliation(&contact->department, reply[COL_ORG_DEPARTMENT]);
- add_affiliation(&contact->role, reply[COL_ORG_ROLE]);
+ contact_add_numbers(&contact, reply);
+ contact_add_emails(&contact, reply);
+ contact_add_addresses(&contact, reply);
+ contact_add_organization(&contact, reply);
DBG("contact %p", contact);
--
1.7.0.4
^ permalink raw reply related
* [PATCH 1/2] Using field names instead of numbers in PBAP
From: Bartosz Szatkowski @ 2010-11-15 12:01 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Bartosz Szatkowski
Previously addressing via field numbers and field names were mixed in PBAP
query replies. Now there is defined name for each data base reply field.
---
plugins/phonebook-tracker.c | 104 +++++++++++++++++++++++++++++++-----------
1 files changed, 77 insertions(+), 27 deletions(-)
diff --git a/plugins/phonebook-tracker.c b/plugins/phonebook-tracker.c
index 672d59f..8f8a6e2 100644
--- a/plugins/phonebook-tracker.c
+++ b/plugins/phonebook-tracker.c
@@ -43,21 +43,65 @@
#define TRACKER_RESOURCES_INTERFACE "org.freedesktop.Tracker1.Resources"
#define TRACKER_DEFAULT_CONTACT_ME "http://www.semanticdesktop.org/ontologies/2007/03/22/nco#default-contact-me"
-#define CONTACTS_ID_COL 47
+#define ADDR_FIELD_AMOUNT 7
#define PULL_QUERY_COL_AMOUNT 48
#define COUNT_QUERY_COL_AMOUNT 1
+
#define COL_HOME_NUMBER 0
+#define COL_FULL_NAME 1
+#define COL_FAMILY_NAME 2
+#define COL_GIVEN_NAME 3
+#define COL_ADDITIONAL_NAME 4
+#define COL_NAME_PREFIX 5
+#define COL_NAME_SUFFIX 6
#define COL_HOME_EMAIL 7
#define COL_WORK_NUMBER 8
+
+#define COL_HOME_ADDR_POBOX 9
+#define COL_HOME_ADDR_EXT 10
+#define COL_HOME_ADDR_STREET 11
+#define COL_HOME_ADDR_LOCALITY 12
+#define COL_HOME_ADDR_REGION 13
+#define COL_HOME_ADDR_CODE 14
+#define COL_HOME_ADDR_COUNTRY 15
+
#define COL_FAX_NUMBER 16
#define COL_WORK_EMAIL 17
+#define COL_BIRTH_DATE 18
+#define COL_NICKNAME 19
+#define COL_URL 20
+#define COL_PHOTO 21
+
+#define COL_ORG_NAME 22
+#define COL_ORG_DEPARTMENT 23
+#define COL_ORG_ROLE 24
+
+#define COL_WORK_ADDR_POBOX 25
+#define COL_WORK_ADDR_EXT 26
+#define COL_WORK_ADDR_STREET 27
+#define COL_WORK_ADDR_LOCALITY 28
+#define COL_WORK_ADDR_REGION 29
+#define COL_WORK_ADDR_CODE 30
+#define COL_WORK_ADDR_COUNTRY 31
+
+#define COL_UID 32
+#define COL_TITLE 33
#define COL_OTHER_NUMBER 34
+
+#define COL_OTHER_ADDR_POBOX 35
+#define COL_OTHER_ADDR_EXT 36
+#define COL_OTHER_ADDR_STREET 37
+#define COL_OTHER_ADDR_LOCALITY 38
+#define COL_OTHER_ADDR_REGION 39
+#define COL_OTHER_ADDR_CODE 40
+#define COL_OTHER_ADDR_COUNTRY 41
+
#define COL_OTHER_EMAIL 42
#define COL_CELL_NUMBER 43
#define COL_DATE 44
#define COL_SENT 45
#define COL_ANSWERED 46
-#define ADDR_FIELD_AMOUNT 7
+#define CONTACTS_ID_COL 47
#define CONTACT_ID_PREFIX "contact:"
#define CONTACTS_QUERY_ALL \
@@ -1407,21 +1451,21 @@ static void pull_contacts(char **reply, int num_fields, void *user_data)
add_entry:
contact = g_new0(struct phonebook_contact, 1);
- contact->fullname = g_strdup(reply[1]);
- contact->family = g_strdup(reply[2]);
- contact->given = g_strdup(reply[3]);
- contact->additional = g_strdup(reply[4]);
- contact->prefix = g_strdup(reply[5]);
- contact->suffix = g_strdup(reply[6]);
- contact->birthday = g_strdup(reply[18]);
- contact->nickname = g_strdup(reply[19]);
- contact->website = g_strdup(reply[20]);
- contact->photo = g_strdup(reply[21]);
- contact->company = g_strdup(reply[22]);
- contact->department = g_strdup(reply[23]);
- contact->role = g_strdup(reply[24]);
- contact->uid = g_strdup(reply[32]);
- contact->title = g_strdup(reply[33]);
+ contact->fullname = g_strdup(reply[COL_FULL_NAME]);
+ contact->family = g_strdup(reply[COL_FAMILY_NAME]);
+ contact->given = g_strdup(reply[COL_GIVEN_NAME]);
+ contact->additional = g_strdup(reply[COL_ADDITIONAL_NAME]);
+ contact->prefix = g_strdup(reply[COL_NAME_PREFIX]);
+ contact->suffix = g_strdup(reply[COL_NAME_SUFFIX]);
+ contact->birthday = g_strdup(reply[COL_BIRTH_DATE]);
+ contact->nickname = g_strdup(reply[COL_NICKNAME]);
+ contact->website = g_strdup(reply[COL_URL]);
+ contact->photo = g_strdup(reply[COL_PHOTO]);
+ contact->company = g_strdup(reply[COL_ORG_NAME]);
+ contact->department = g_strdup(reply[COL_ORG_DEPARTMENT]);
+ contact->role = g_strdup(reply[COL_ORG_ROLE]);
+ contact->uid = g_strdup(reply[COL_UID]);
+ contact->title = g_strdup(reply[COL_TITLE]);
set_call_type(contact, reply[COL_DATE], reply[COL_SENT],
reply[COL_ANSWERED]);
@@ -1444,16 +1488,22 @@ add_numbers:
/* Adding addresses */
home_addr = g_strdup_printf("%s;%s;%s;%s;%s;%s;%s",
- reply[9], reply[10], reply[11], reply[12],
- reply[13], reply[14], reply[15]);
+ reply[COL_HOME_ADDR_POBOX], reply[COL_HOME_ADDR_EXT],
+ reply[COL_HOME_ADDR_STREET], reply[COL_HOME_ADDR_LOCALITY],
+ reply[COL_HOME_ADDR_REGION], reply[COL_HOME_ADDR_CODE],
+ reply[COL_HOME_ADDR_COUNTRY]);
work_addr = g_strdup_printf("%s;%s;%s;%s;%s;%s;%s",
- reply[25], reply[26], reply[27], reply[28],
- reply[29], reply[30], reply[31]);
+ reply[COL_WORK_ADDR_POBOX], reply[COL_WORK_ADDR_EXT],
+ reply[COL_WORK_ADDR_STREET], reply[COL_WORK_ADDR_LOCALITY],
+ reply[COL_WORK_ADDR_REGION], reply[COL_WORK_ADDR_CODE],
+ reply[COL_WORK_ADDR_COUNTRY]);
other_addr = g_strdup_printf("%s;%s;%s;%s;%s;%s;%s",
- reply[35], reply[36], reply[37], reply[38],
- reply[39], reply[40], reply[41]);
+ reply[COL_OTHER_ADDR_POBOX], reply[COL_OTHER_ADDR_EXT],
+ reply[COL_OTHER_ADDR_STREET], reply[COL_OTHER_ADDR_LOCALITY],
+ reply[COL_OTHER_ADDR_REGION], reply[COL_OTHER_ADDR_CODE],
+ reply[COL_OTHER_ADDR_COUNTRY]);
add_address(contact, home_addr, ADDR_TYPE_HOME);
add_address(contact, work_addr, ADDR_TYPE_WORK);
@@ -1465,10 +1515,10 @@ add_numbers:
/* Adding fields connected by nco:hasAffiliation - they may be in
* separate replies */
- add_affiliation(&contact->title, reply[33]);
- add_affiliation(&contact->company, reply[22]);
- add_affiliation(&contact->department, reply[23]);
- add_affiliation(&contact->role, reply[24]);
+ add_affiliation(&contact->title, reply[COL_TITLE]);
+ add_affiliation(&contact->company, reply[COL_ORG_NAME]);
+ add_affiliation(&contact->department, reply[COL_ORG_DEPARTMENT]);
+ add_affiliation(&contact->role, reply[COL_ORG_ROLE]);
DBG("contact %p", contact);
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH v3] Add iwmmxt optimization for sbc for pxa series cpu
From: Siarhei Siamashka @ 2010-11-15 11:08 UTC (permalink / raw)
To: Keith Mok; +Cc: linux-bluetooth
In-Reply-To: <AANLkTimQR5wMTTEO6KGuv3YvPBaftO-i+96WKYQEOzFU@mail.gmail.com>
[-- Attachment #1: Type: Text/Plain, Size: 2142 bytes --]
On Monday 15 November 2010 04:46:25 Keith Mok wrote:
> > I sometimes use different indentation levels in such cases in order to
> > improve readability after instructions reordering, so that each
> > logically independent block of code has its own indentation level and it
> > is still easily visible
>
> > after instructions reordering. For example, with the original code:
> Thanks for the hints. I rearranged the code.
Thanks, now the assembly code looks ok to me. I also discovered that qemu
supports iwmmxt1 emulation just fine and also tried to test your optimizations
for correctness myself (with a script which tries different encoding paramaters
for different audio samples and checks md5 checksums), no problems detected.
So if somebody else could check whether the other things are right (copyright
notices for example), then we are done with it.
> I removed the scale_factor optimization since from the result I
> tested, it shows little help in performance.
I guess after easily doubling performance by adding simd optimizations to the
sbc analysis filter, just roughly ~10% improvement (as measured for x86 and
arm neon) does not look particularly impressive anymore:
http://git.kernel.org/?p=bluetooth/bluez.git;a=commit;h=95465b816f0ce7f0ec10a183ce7ff0c6f83d86eb
http://git.kernel.org/?p=bluetooth/bluez.git;a=commit;h=d049a9a2aec2b518e04f11ef0ecc355db8237291
But I still think that every little bit helps. Did you also get something like
10% speedup, or was it even worse than that?
A bit more important in practice is the optimization for joint stereo scale
factors calculation (because it is typically used for A2DP). And it provided
almost 20% of performance improvement for arm neon:
http://git.kernel.org/?p=bluetooth/bluez.git;a=commit;h=e1ea3e76c72d56041c30b317818e8d7b5a0c7350
So 'sbc_calc_scalefactors_j_iwmmxt' may be a nice addition too, optimized
either as a whole for best performance (like in arm neon code), or just with
some small chunks of assembly like in 'sbc_calc_scalefactors_mmx' because it
is easier this way.
--
Best regards,
Siarhei Siamashka
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* [PATCH v2] Fix pull phonebook with non-zero offset parameter
From: Rafal Michalski @ 2010-11-15 10:41 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Rafal Michalski
For non-zero liststartoffset parameter, contacts which index was before
offset were indexed more than once (e.g. when contact had more than one
phone number or address etc.), so pulling was started earlier - before
offset index was reached. This patch fix this problem and each contact
is indexed only once.
---
plugins/phonebook-tracker.c | 9 ++++++++-
1 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/plugins/phonebook-tracker.c b/plugins/phonebook-tracker.c
index 672d59f..4309e28 100644
--- a/plugins/phonebook-tracker.c
+++ b/plugins/phonebook-tracker.c
@@ -1361,6 +1361,7 @@ static void pull_contacts(char **reply, int num_fields, void *user_data)
int last_index, i;
gboolean cdata_present = FALSE;
char *home_addr, *work_addr, *other_addr;
+ static char *temp_id = NULL;
if (num_fields < 0) {
data->cb(NULL, 0, num_fields, 0, data->user_data);
@@ -1396,7 +1397,11 @@ static void pull_contacts(char **reply, int num_fields, void *user_data)
TRACKER_DEFAULT_CONTACT_ME))
return;
- data->index++;
+ if (g_strcmp0(temp_id, reply[CONTACTS_ID_COL])) {
+ data->index++;
+ g_free(temp_id);
+ temp_id = g_strdup(reply[CONTACTS_ID_COL]);
+ }
last_index = params->liststartoffset + params->maxlistcount;
@@ -1495,6 +1500,8 @@ done:
fail:
g_slist_free(data->contacts);
g_free(data);
+ g_free(temp_id);
+ temp_id = NULL;
}
static void add_to_cache(char **reply, int num_fields, void *user_data)
--
1.6.3.3
^ permalink raw reply related
* Re: [PATCH 3/4] Add DBus OOB API documentation.
From: Szymon Janc @ 2010-11-15 10:11 UTC (permalink / raw)
To: jaikumar Ganesh; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <AANLkTikFAc43uU-976tRSFrntFHYApe9h9zzVHto22_6@mail.gmail.com>
Hi,
> So the only APIs added are these provider ones and one API in Agent
> code for approval.
>
> So how does bluetoothd decide whether OOB is present for a particular
> remote device for an outgoing pairing case ? Just on the basis of
> whether a provider is registered or am I missing something here.
If there is an active OOB plugin then it is asked for remote OOB data when
io capabilities are established. Based on the answer oob_data field in io
capability reply is set. Dbus OOB plugin becomes active when provider is
registered. If no oob plugin is active oob_data field is always set to 0x00.
> RequestRemoteOobData is called before initiating pairing or when the
> remote end asks for the OOB data ?
> Its unclear from the API description.
I can see that OOB mechanism is somewhat unclear, let me try to clarify:
- local oob data are data generated by local adapter
- remote oob data are data received from remote device
- remote and/or local data are exchange through some OOB mechanism - this takes
place before SSP is started
(note that data exchanged on OOB also contains BT address)
- during SSP active oob plugin is asked for oob data for remote device
So, RequestRemoteOobData is called when io capabilities are being established,
but this asks plugin if it already has OOB data for this device and not
a remote device to provide this data.
Hope this clarifies things a bit :)
--
Szymon Janc
^ permalink raw reply
* Re: [PATCH 3/4] Add DBus OOB API documentation.
From: Szymon Janc @ 2010-11-15 8:54 UTC (permalink / raw)
To: jaikumar Ganesh; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <AANLkTikZrbVTCDB2HRtqRJjUjYP3Z5PBieOovZZgP9id@mail.gmail.com>
Hi Jaikumar,
> > + void Deactivate()
>
> Would it better to make this a signal ? Deactivate by itself as the
> only method doesn't seem to be right.
I'll change it to Release() to be consistent with other (Agent) API as
suggested by Johan.
> > + void RegisterProvider(object provider)
> > +
> > + This method registers provider for DBus OOB plug-in.
> > + When provider is successfully registered plug-in becomes
> > + active. Only one provider can be registered at time.
>
> Why are we enforcing this limitation ?
Spec requires that each hash&randomizer values should be used only for one OOB
transfer. Implementation enforces that by allowing only one active OOB plugin
at time and DBUS OOB plugin only 'expose' plugin API over DBUS. So this
limitation is a consequence of that.
This limitation also allows for (IMHO) simpler plugins implementation as they
don't have to care about other plugins hanging around to fulfill that
requirement.
> > + array{bye}, array{byte} UpdateLocalOobData(string address)
>
> You are not updating anything here. You are just reading the local
> adapter OOB data
It is updating hash and randomizer. Underlying functions are called Read* as
this is how corresponding HCI command is called (see Vol2. Part E. 7.3.60).
I'll change it to Read to keep all calls name consistent with HCI command name
and add appropriate note in API documentation as this name may be somewhat
misleading (yet OOB plugin implementer should be aware of that already).
--
Szymon Janc
^ permalink raw reply
* Re: [PATCH v3] Add iwmmxt optimization for sbc for pxa series cpu
From: Keith Mok @ 2010-11-15 2:46 UTC (permalink / raw)
To: Siarhei Siamashka; +Cc: linux-bluetooth
In-Reply-To: <201011121522.51428.siarhei.siamashka@gmail.com>
> I sometimes use different indentation levels in such cases in order to improve
> readability after instructions reordering, so that each logically independent
> block of code has its own indentation level and it is still easily visible
> after instructions reordering. For example, with the original code:
Thanks for the hints. I rearranged the code.
> Not so much, but every little bit
> helps. And if for iwmmxt it causes such backwards compatibility issues, then it
> might be not worth it. It's up to you to decide.
I removed the scale_factor optimization since from the result I
tested, it shows little help in performance.
> Regarding the benchmarks and functions usage:
> 1. sbc_analyze_four_iwmmxt is important for 4 subbands case ('-s4' option for
> sbcenc)
> 2. sbc_analyze_eight_iwmmxt is important for 8 subbands case ('-s8' option for
> sbcenc)
=== Before (4 bands) ====
$ time ./sbcenc_orig -s 4 long.au > /dev/null
real 0m 2.44s
user 0m 2.39s
sys 0m 0.05s
=== After (4 bands) ====
$ time ./sbcenc -s 4 long.au > /dev/null
real 0m 1.59s
user 0m 1.49s
sys 0m 0.10s
=== Before (8 bands) ====
$ time ./sbcenc_orig -s 8 long.au > /dev/null
real 0m 4.05s
user 0m 3.98s
sys 0m 0.07s
=== After (8 bands) ====
$ time ./sbcenc -s 8 long.au > /dev/null
real 0m 1.48s
user 0m 1.41s
sys 0m 0.06s
=== Before (a2dp usage) ====
$ time ./sbcenc_orig -b53 -s8 -j long.au > /dev/null
real 0m 4.51s
user 0m 4.41s
sys 0m 0.10s
=== After (a2dp usage) ====
$ time ./sbcenc -b53 -s8 -j long.au > /dev/null
real 0m 2.05s
user 0m 1.99s
sys 0m 0.06s
Keith
Signed-off-by: Keith Mok <ek9852@gmail.com>
---
Makefile.am | 1 +
sbc/sbc_primitives.c | 4 +
sbc/sbc_primitives_iwmmxt.c | 301 +++++++++++++++++++++++++++++++++++++++++++
sbc/sbc_primitives_iwmmxt.h | 38 ++++++
4 files changed, 344 insertions(+), 0 deletions(-)
create mode 100644 sbc/sbc_primitives_iwmmxt.c
create mode 100644 sbc/sbc_primitives_iwmmxt.h
diff --git a/Makefile.am b/Makefile.am
index da308a7..03a9bf2 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -65,6 +65,7 @@ noinst_LTLIBRARIES += sbc/libsbc.la
sbc_libsbc_la_SOURCES = sbc/sbc.h sbc/sbc.c sbc/sbc_math.h sbc/sbc_tables.h \
sbc/sbc_primitives.h sbc/sbc_primitives.c \
sbc/sbc_primitives_mmx.h sbc/sbc_primitives_mmx.c \
+ sbc/sbc_primitives_iwmmxt.h sbc/sbc_primitives_iwmmxt.c \
sbc/sbc_primitives_neon.h sbc/sbc_primitives_neon.c \
sbc/sbc_primitives_armv6.h sbc/sbc_primitives_armv6.c
diff --git a/sbc/sbc_primitives.c b/sbc/sbc_primitives.c
index f87fb5a..ad780d0 100644
--- a/sbc/sbc_primitives.c
+++ b/sbc/sbc_primitives.c
@@ -33,6 +33,7 @@
#include "sbc_primitives.h"
#include "sbc_primitives_mmx.h"
+#include "sbc_primitives_iwmmxt.h"
#include "sbc_primitives_neon.h"
#include "sbc_primitives_armv6.h"
@@ -544,6 +545,9 @@ void sbc_init_primitives(struct sbc_encoder_state *state)
#ifdef SBC_BUILD_WITH_ARMV6_SUPPORT
sbc_init_primitives_armv6(state);
#endif
+#ifdef SBC_BUILD_WITH_IWMMXT_SUPPORT
+ sbc_init_primitives_iwmmxt(state);
+#endif
#ifdef SBC_BUILD_WITH_NEON_SUPPORT
sbc_init_primitives_neon(state);
#endif
diff --git a/sbc/sbc_primitives_iwmmxt.c b/sbc/sbc_primitives_iwmmxt.c
new file mode 100644
index 0000000..fc462d2
--- /dev/null
+++ b/sbc/sbc_primitives_iwmmxt.c
@@ -0,0 +1,301 @@
+/*
+ *
+ * Bluetooth low-complexity, subband codec (SBC) library
+ *
+ * Copyright (C) 2010 Keith Mok <ek9852@gmail.com>
+ * Based on sbc_primitives_mmx.c
+ *
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ */
+
+#include <stdint.h>
+#include <limits.h>
+#include "sbc.h"
+#include "sbc_math.h"
+#include "sbc_tables.h"
+
+#include "sbc_primitives_iwmmxt.h"
+
+/*
+ * IWMMXT optimizations
+ */
+
+#ifdef SBC_BUILD_WITH_IWMMXT_SUPPORT
+
+static inline void sbc_analyze_four_iwmmxt(const int16_t *in, int32_t *out,
+ const FIXED_T *consts)
+{
+ asm volatile (
+ "wldrd wr0, [%0]\n"
+ "tbcstw wr4, %2\n"
+ "wldrd wr2, [%1]\n"
+ "wldrd wr1, [%0, #8]\n"
+ "wldrd wr3, [%1, #8]\n"
+ "wmadds wr0, wr2, wr0\n"
+ " wldrd wr6, [%0, #16]\n"
+ "wmadds wr1, wr3, wr1\n"
+ " wldrd wr7, [%0, #24]\n"
+ "waddwss wr0, wr0, wr4\n"
+ " wldrd wr8, [%1, #16]\n"
+ "waddwss wr1, wr1, wr4\n"
+ " wldrd wr9, [%1, #24]\n"
+ " wmadds wr6, wr8, wr6\n"
+ " wldrd wr2, [%0, #32]\n"
+ " wmadds wr7, wr9, wr7\n"
+ " wldrd wr3, [%0, #40]\n"
+ " waddwss wr0, wr6, wr0\n"
+ " wldrd wr4, [%1, #32]\n"
+ " waddwss wr1, wr7, wr1\n"
+ " wldrd wr5, [%1, #40]\n"
+ " wmadds wr2, wr4, wr2\n"
+ "wldrd wr6, [%0, #48]\n"
+ " wmadds wr3, wr5, wr3\n"
+ "wldrd wr7, [%0, #56]\n"
+ " waddwss wr0, wr2, wr0\n"
+ "wldrd wr8, [%1, #48]\n"
+ " waddwss wr1, wr3, wr1\n"
+ "wldrd wr9, [%1, #56]\n"
+ "wmadds wr6, wr8, wr6\n"
+ " wldrd wr2, [%0, #64]\n"
+ "wmadds wr7, wr9, wr7\n"
+ " wldrd wr3, [%0, #72]\n"
+ "waddwss wr0, wr6, wr0\n"
+ " wldrd wr4, [%1, #64]\n"
+ "waddwss wr1, wr7, wr1\n"
+ " wldrd wr5, [%1, #72]\n"
+ " wmadds wr2, wr4, wr2\n"
+ "tmcr wcgr0, %4\n"
+ " wmadds wr3, wr5, wr3\n"
+ " waddwss wr0, wr2, wr0\n"
+ " waddwss wr1, wr3, wr1\n"
+ "\n"
+ "wsrawg wr0, wr0, wcgr0\n"
+ " wldrd wr4, [%1, #80]\n"
+ "wsrawg wr1, wr1, wcgr0\n"
+ " wldrd wr5, [%1, #88]\n"
+ "wpackwss wr0, wr0, wr0\n"
+ " wldrd wr6, [%1, #96]\n"
+ "wpackwss wr1, wr1, wr1\n"
+ "wmadds wr2, wr5, wr0\n"
+ " wldrd wr7, [%1, #104]\n"
+ "wmadds wr0, wr4, wr0\n"
+ "\n"
+ " wmadds wr3, wr7, wr1\n"
+ " wmadds wr1, wr6, wr1\n"
+ " waddwss wr2, wr3, wr2\n"
+ " waddwss wr0, wr1, wr0\n"
+ "\n"
+ "wstrd wr0, [%3]\n"
+ "wstrd wr2, [%3, #8]\n"
+ :
+ : "r" (in), "r" (consts),
+ "r" (1 << (SBC_PROTO_FIXED4_SCALE - 1)), "r" (out),
+ "r" (SBC_PROTO_FIXED4_SCALE)
+ : "wr0", "wr1", "wr2", "wr3", "wr4", "wr5", "wr6", "wr7",
+ "wr8", "wr9", "wcgr0", "memory");
+}
+
+static inline void sbc_analyze_eight_iwmmxt(const int16_t *in, int32_t *out,
+ const FIXED_T *consts)
+{
+ asm volatile (
+ "wldrd wr0, [%0]\n"
+ "tbcstw wr15, %2\n"
+ "wldrd wr1, [%0, #8]\n"
+ "wldrd wr2, [%0, #16]\n"
+ "wldrd wr3, [%0, #24]\n"
+ "wldrd wr4, [%1]\n"
+ "wldrd wr5, [%1, #8]\n"
+ "wldrd wr6, [%1, #16]\n"
+ "wldrd wr7, [%1, #24]\n"
+ "wmadds wr0, wr0, wr4\n"
+ " wldrd wr8, [%1, #32]\n"
+ "wmadds wr1, wr1, wr5\n"
+ " wldrd wr9, [%1, #40]\n"
+ "wmadds wr2, wr2, wr6\n"
+ " wldrd wr10, [%1, #48]\n"
+ "wmadds wr3, wr3, wr7\n"
+ " wldrd wr11, [%1, #56]\n"
+ "waddwss wr0, wr0, wr15\n"
+ " wldrd wr4, [%0, #32]\n"
+ "waddwss wr1, wr1, wr15\n"
+ " wldrd wr5, [%0, #40]\n"
+ "waddwss wr2, wr2, wr15\n"
+ " wldrd wr6, [%0, #48]\n"
+ "waddwss wr3, wr3, wr15\n"
+ " wldrd wr7, [%0, #56]\n"
+ " wmadds wr4, wr4, wr8\n"
+ " wldrd wr12, [%0, #64]\n"
+ " wmadds wr5, wr5, wr9\n"
+ " wldrd wr13, [%0, #72]\n"
+ " wmadds wr6, wr6, wr10\n"
+ " wldrd wr14, [%0, #80]\n"
+ " wmadds wr7, wr7, wr11\n"
+ " wldrd wr15, [%0, #88]\n"
+ " waddwss wr0, wr4, wr0\n"
+ " wldrd wr8, [%1, #64]\n"
+ " waddwss wr1, wr5, wr1\n"
+ " wldrd wr9, [%1, #72]\n"
+ " waddwss wr2, wr6, wr2\n"
+ " wldrd wr10, [%1, #80]\n"
+ " waddwss wr3, wr7, wr3\n"
+ " wldrd wr11, [%1, #88]\n"
+ " wmadds wr12, wr12, wr8\n"
+ "wldrd wr4, [%0, #96]\n"
+ " wmadds wr13, wr13, wr9\n"
+ "wldrd wr5, [%0, #104]\n"
+ " wmadds wr14, wr14, wr10\n"
+ "wldrd wr6, [%0, #112]\n"
+ " wmadds wr15, wr15, wr11\n"
+ "wldrd wr7, [%0, #120]\n"
+ " waddwss wr0, wr12, wr0\n"
+ "wldrd wr8, [%1, #96]\n"
+ " waddwss wr1, wr13, wr1\n"
+ "wldrd wr9, [%1, #104]\n"
+ " waddwss wr2, wr14, wr2\n"
+ "wldrd wr10, [%1, #112]\n"
+ " waddwss wr3, wr15, wr3\n"
+ "wldrd wr11, [%1, #120]\n"
+ "wmadds wr4, wr4, wr8\n"
+ " wldrd wr12, [%0, #128]\n"
+ "wmadds wr5, wr5, wr9\n"
+ " wldrd wr13, [%0, #136]\n"
+ "wmadds wr6, wr6, wr10\n"
+ " wldrd wr14, [%0, #144]\n"
+ "wmadds wr7, wr7, wr11\n"
+ " wldrd wr15, [%0, #152]\n"
+ "waddwss wr0, wr4, wr0\n"
+ " wldrd wr8, [%1, #128]\n"
+ "waddwss wr1, wr5, wr1\n"
+ " wldrd wr9, [%1, #136]\n"
+ "waddwss wr2, wr6, wr2\n"
+ " wldrd wr10, [%1, #144]\n"
+ " waddwss wr3, wr7, wr3\n"
+ " wldrd wr11, [%1, #152]\n"
+ " wmadds wr12, wr12, wr8\n"
+ "tmcr wcgr0, %4\n"
+ " wmadds wr13, wr13, wr9\n"
+ " wmadds wr14, wr14, wr10\n"
+ " wmadds wr15, wr15, wr11\n"
+ " waddwss wr0, wr12, wr0\n"
+ " waddwss wr1, wr13, wr1\n"
+ " waddwss wr2, wr14, wr2\n"
+ " waddwss wr3, wr15, wr3\n"
+ "\n"
+ "wsrawg wr0, wr0, wcgr0\n"
+ "wsrawg wr1, wr1, wcgr0\n"
+ "wsrawg wr2, wr2, wcgr0\n"
+ "wsrawg wr3, wr3, wcgr0\n"
+ "\n"
+ "wpackwss wr0, wr0, wr0\n"
+ "wpackwss wr1, wr1, wr1\n"
+ " wldrd wr4, [%1, #160]\n"
+ "wpackwss wr2, wr2, wr2\n"
+ " wldrd wr5, [%1, #168]\n"
+ "wpackwss wr3, wr3, wr3\n"
+ " wldrd wr6, [%1, #192]\n"
+ " wmadds wr4, wr4, wr0\n"
+ " wldrd wr7, [%1, #200]\n"
+ " wmadds wr5, wr5, wr0\n"
+ " wldrd wr8, [%1, #224]\n"
+ " wmadds wr6, wr6, wr1\n"
+ " wldrd wr9, [%1, #232]\n"
+ " wmadds wr7, wr7, wr1\n"
+ " waddwss wr4, wr6, wr4\n"
+ " waddwss wr5, wr7, wr5\n"
+ " wmadds wr8, wr8, wr2\n"
+ "wldrd wr6, [%1, #256]\n"
+ " wmadds wr9, wr9, wr2\n"
+ "wldrd wr7, [%1, #264]\n"
+ "waddwss wr4, wr8, wr4\n"
+ " waddwss wr5, wr9, wr5\n"
+ "wmadds wr6, wr6, wr3\n"
+ "wmadds wr7, wr7, wr3\n"
+ "waddwss wr4, wr6, wr4\n"
+ "waddwss wr5, wr7, wr5\n"
+ "\n"
+ "wstrd wr4, [%3]\n"
+ "wstrd wr5, [%3, #8]\n"
+ "\n"
+ "wldrd wr6, [%1, #176]\n"
+ "wldrd wr5, [%1, #184]\n"
+ "wmadds wr5, wr5, wr0\n"
+ "wldrd wr8, [%1, #208]\n"
+ "wmadds wr0, wr6, wr0\n"
+ "wldrd wr9, [%1, #216]\n"
+ "wmadds wr9, wr9, wr1\n"
+ "wldrd wr6, [%1, #240]\n"
+ "wmadds wr1, wr8, wr1\n"
+ "wldrd wr7, [%1, #248]\n"
+ "waddwss wr0, wr1, wr0\n"
+ "waddwss wr5, wr9, wr5\n"
+ "wmadds wr7, wr7, wr2\n"
+ "wldrd wr8, [%1, #272]\n"
+ "wmadds wr2, wr6, wr2\n"
+ "wldrd wr9, [%1, #280]\n"
+ "waddwss wr0, wr2, wr0\n"
+ "waddwss wr5, wr7, wr5\n"
+ "wmadds wr9, wr9, wr3\n"
+ "wmadds wr3, wr8, wr3\n"
+ "waddwss wr0, wr3, wr0\n"
+ "waddwss wr5, wr9, wr5\n"
+ "\n"
+ "wstrd wr0, [%3, #16]\n"
+ "wstrd wr5, [%3, #24]\n"
+ :
+ : "r" (in), "r" (consts),
+ "r" (1 << (SBC_PROTO_FIXED8_SCALE - 1)), "r" (out),
+ "r" (SBC_PROTO_FIXED8_SCALE)
+ : "wr0", "wr1", "wr2", "wr3", "wr4", "wr5", "wr6", "wr7",
+ "wr8", "wr9", "wr10", "wr11", "wr12", "wr13", "wr14", "wr15",
+ "wcgr0", "memory");
+}
+
+static inline void sbc_analyze_4b_4s_iwmmxt(int16_t *x, int32_t *out,
+ int out_stride)
+{
+ /* Analyze blocks */
+ sbc_analyze_four_iwmmxt(x + 12, out, analysis_consts_fixed4_simd_odd);
+ out += out_stride;
+ sbc_analyze_four_iwmmxt(x + 8, out, analysis_consts_fixed4_simd_even);
+ out += out_stride;
+ sbc_analyze_four_iwmmxt(x + 4, out, analysis_consts_fixed4_simd_odd);
+ out += out_stride;
+ sbc_analyze_four_iwmmxt(x + 0, out, analysis_consts_fixed4_simd_even);
+}
+
+static inline void sbc_analyze_4b_8s_iwmmxt(int16_t *x, int32_t *out,
+ int out_stride)
+{
+ /* Analyze blocks */
+ sbc_analyze_eight_iwmmxt(x + 24, out, analysis_consts_fixed8_simd_odd);
+ out += out_stride;
+ sbc_analyze_eight_iwmmxt(x + 16, out, analysis_consts_fixed8_simd_even);
+ out += out_stride;
+ sbc_analyze_eight_iwmmxt(x + 8, out, analysis_consts_fixed8_simd_odd);
+ out += out_stride;
+ sbc_analyze_eight_iwmmxt(x + 0, out, analysis_consts_fixed8_simd_even);
+}
+
+void sbc_init_primitives_iwmmxt(struct sbc_encoder_state *state)
+{
+ state->sbc_analyze_4b_4s = sbc_analyze_4b_4s_iwmmxt;
+ state->sbc_analyze_4b_8s = sbc_analyze_4b_8s_iwmmxt;
+ state->implementation_info = "IWMMXT";
+}
+
+#endif
diff --git a/sbc/sbc_primitives_iwmmxt.h b/sbc/sbc_primitives_iwmmxt.h
new file mode 100644
index 0000000..827d811
--- /dev/null
+++ b/sbc/sbc_primitives_iwmmxt.h
@@ -0,0 +1,38 @@
+/*
+ *
+ * Bluetooth low-complexity, subband codec (SBC) library
+ *
+ * Based on sbc_primitives_mmx.c
+ *
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ */
+
+#ifndef __SBC_PRIMITIVES_IWMMXT_H
+#define __SBC_PRIMITIVES_IWMMXT_H
+
+#include "sbc_primitives.h"
+
+#if defined(__GNUC__) && defined(__IWMMXT__) && \
+ !defined(SBC_HIGH_PRECISION) && (SCALE_OUT_BITS == 15)
+
+#define SBC_BUILD_WITH_IWMMXT_SUPPORT
+
+void sbc_init_primitives_iwmmxt(struct sbc_encoder_state *encoder_state);
+
+#endif
+
+#endif
--
1.6.3.3
^ permalink raw reply related
* Re: [PATCH v2 1/7] Fix invalid memory access when EIR field length is zero
From: Anderson Lizardo @ 2010-11-13 1:00 UTC (permalink / raw)
To: Inga Stotland, Vinicius Costa Gomes, linux-bluetooth,
Bruna Moreira
In-Reply-To: <20101112165434.GA13238@jh-x301>
On 11/12/10, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi Inga,
>
> On Thu, Nov 11, 2010, Inga Stotland wrote:
>> Was there a bug to begin with? :)
>> The access to eir_data[1] was always valid due to the check (len <
>> EIR_DATA_LENGTH - 1)
>> and the fact that eir_data is a buffer of fixed length of EIR_DATA_LENGTH
>> (240 bytes).
>
> On closer inspection it seems you might be right, however it'd be nice
> to get some comments from the original patch author about this (were
> there e.g. crashes or some valgrind warnings observed or was this just
> speculation based on looking at the code).
There were valgrind "unintialized value" warnings related to this
after we started using it for parsing adv data (which is max 31 bytes
long but may be smaller because spec allows radio to strip non
significant bytes before sending, which explains why we added a length
parameter in a later patch). Actually I dont think the current code is
"safe" as it assumes the EIR data is aways 240 bytes long, which seems
true as per spec , but less data could be sent, causing reading beyond
buffer.
> Btw, it seems I may need to slow down on my response time to patches so
> there's better time for other people to review them too. E.g. both you
> and Luiz were a bit late to the game on a couple of recent patches.
> Maybe a 24 hour period before I push anything might be good enough?
I appreciate if you could send your comments ASAP, so we can fix them
quickly. Pushing upstream can be delayed for non trivial patches.
PS: I'm out of office until next Tuesday so I may not be able answer
quickly until then. thanks to Vinicius for pushing the patches fixed
versions :)
Regards
--
Anderson Lizardo
OpenBossa Labs - INdT
Manaus - Brazil
^ permalink raw reply
* Re: [PATCH 3/4] Add DBus OOB API documentation.
From: jaikumar Ganesh @ 2010-11-12 19:07 UTC (permalink / raw)
To: jaikumar Ganesh, Szymon Janc, linux-bluetooth
In-Reply-To: <20101112182921.GA15584@jh-x301>
Hi Szymon:
On Fri, Nov 12, 2010 at 10:29 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi Jaikumar,
>
> On Fri, Nov 12, 2010, jaikumar Ganesh wrote:
>> > + void Deactivate()
>>
>> Would it better to make this a signal ? Deactivate by itself as the
>> only method doesn't seem to be right.
>
> I guess this is analogous to the Release() method which we have for
> agents, so in that sense a method call would be consistent with the rest
> of the BlueZ D-Bus API (but you'd really need to rename this to Release
> then).
>
> Johan
>
So the only APIs added are these provider ones and one API in Agent
code for approval.
So how does bluetoothd decide whether OOB is present for a particular
remote device for an outgoing pairing case ? Just on the basis of
whether a provider is registered or am I missing something here.
RequestRemoteOobData is called before initiating pairing or when the
remote end asks for the OOB data ?
Its unclear from the API description.
Thanks
Jaikumar
^ permalink raw reply
* Re: [PATCH] compat-wireless: support backporting bluetooth to RHEL 6
From: Johannes Berg @ 2010-11-12 18:39 UTC (permalink / raw)
To: John W. Linville; +Cc: Luis R. Rodriguez, linux-wireless, linux-bluetooth
In-Reply-To: <20101112181348.GG2338@tuxdriver.com>
On Fri, 2010-11-12 at 13:13 -0500, John W. Linville wrote:
>
> -+#if (LINUX_VERSION_CODE > KERNEL_VERSION(2,6,32))
> ++#if (LINUX_VERSION_CODE > KERNEL_VERSION(2,6,32)) || (defined(RHEL_MAJOR) && (RHEL_MAJOR == 6))
Hmmm. I kinda support this, but I'd rather see something like a header
file having
#if (LINUX_VERSION ...) || ...
#define OLD_BT_API 1
#endif
and then using ifdef OLD_BT_API? That way, other distros don't get into
a huge mess trying to add that too, and something like we had two days
ago where we had to make it work on some other custom kernel would be
much easier too since we can just amend the one check or define/undef
the OLD_BT_API symbol.
johannes
^ permalink raw reply
* Re: [PATCH 3/4] Add DBus OOB API documentation.
From: Johan Hedberg @ 2010-11-12 18:29 UTC (permalink / raw)
To: jaikumar Ganesh; +Cc: Szymon Janc, linux-bluetooth
In-Reply-To: <AANLkTikZrbVTCDB2HRtqRJjUjYP3Z5PBieOovZZgP9id@mail.gmail.com>
Hi Jaikumar,
On Fri, Nov 12, 2010, jaikumar Ganesh wrote:
> > + void Deactivate()
>
> Would it better to make this a signal ? Deactivate by itself as the
> only method doesn't seem to be right.
I guess this is analogous to the Release() method which we have for
agents, so in that sense a method call would be consistent with the rest
of the BlueZ D-Bus API (but you'd really need to rename this to Release
then).
Johan
^ permalink raw reply
* Re: [PATCH 3/4] Add DBus OOB API documentation.
From: jaikumar Ganesh @ 2010-11-12 18:20 UTC (permalink / raw)
To: Szymon Janc; +Cc: linux-bluetooth
In-Reply-To: <1288865461-3760-4-git-send-email-szymon.janc@tieto.com>
Hi Szymon:
On Thu, Nov 4, 2010 at 3:11 AM, Szymon Janc <szymon.janc@tieto.com> wrote:
> ---
> Makefile.am | 3 +-
> doc/oob-api.txt | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 64 insertions(+), 1 deletions(-)
> create mode 100644 doc/oob-api.txt
>
> diff --git a/Makefile.am b/Makefile.am
> index d6cbf92..b5157cd 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -358,7 +358,8 @@ EXTRA_DIST += doc/manager-api.txt \
> doc/service-api.txt doc/agent-api.txt doc/attribute-api.txt \
> doc/serial-api.txt doc/network-api.txt \
> doc/input-api.txt doc/audio-api.txt doc/control-api.txt \
> - doc/hfp-api.txt doc/assigned-numbers.txt
> + doc/hfp-api.txt doc/assigned-numbers.txt doc/oob-api.txt
> +
>
> AM_YFLAGS = -d
>
> diff --git a/doc/oob-api.txt b/doc/oob-api.txt
> new file mode 100644
> index 0000000..fce18a7
> --- /dev/null
> +++ b/doc/oob-api.txt
> @@ -0,0 +1,62 @@
> +BlueZ D-Bus OOB API description
> +*******************************
> +
> +Copyright (C) 2010 ST-Ericsson SA
> +
> +Author: Szymon Janc <szymon.janc@tieto.com> for ST-Ericsson
> +
> +OOB hierarchy
> +=================
> +
> +Service unique name
> +Interface org.bluez.OOB
> +Object path freely definable
> +
> +Methods array{bye}, array{byte} RequestRemoteOobData(string address)
> +
> + This method gets called when the service daemon needs to
> + get hash and randomizer for an OOB authentication.
> +
> + The return value should be pair of arrays of 16 bytes
> + each. First hash, second randomizer.
> +
> + If no OOB data is present for specified address empty
> + reply should be returned.
> +
> + void Deactivate()
Would it better to make this a signal ? Deactivate by itself as the
only method doesn't seem to be right.
> +
> + This method gets called when DBus plug-in for OOB was
> + deactivated. There is no need to unregister provider,
> + because when this method gets called it has already been
> + unregistered.
> +
> +--------------------------------------------------------------------------------
> +
> +Service org.bluez
> +Interface org.bluez.OOB
> +Object path /org/bluez
> +
> + void RegisterProvider(object provider)
> +
> + This method registers provider for DBus OOB plug-in.
> + When provider is successfully registered plug-in becomes
> + active. Only one provider can be registered at time.
Why are we enforcing this limitation ?
> +
> + Possible errors: org.bluez.Error.AlreadyExists
> +
> + void UnregisterProvider(object provider)
> +
> + This method unregisters provider for DBus OOB plug-in.
> + When provider is successfully unregistered plug-in
> + becomes inactive and will emit Deactivated() signal.
> +
> + Possible errors: org.bluez.Error.DoesNotExist
> +
> + array{bye}, array{byte} UpdateLocalOobData(string address)
You are not updating anything here. You are just reading the local
adapter OOB data
> +
> + This method generates new local OOB data for specified
> + address (adapter). Return value is pair of arrays 16
> + bytes each. First hash, second randomizer. Only
> + registered provider should call this method.
> +
> + Possible errors: org.bluez.Error.UpdateFailed
> --
> 1.7.1
>
> --
> 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
* [PATCH] compat-wireless: support backporting bluetooth to RHEL 6
From: John W. Linville @ 2010-11-12 18:13 UTC (permalink / raw)
To: Luis R. Rodriguez; +Cc: linux-wireless, linux-bluetooth
RHEL kernels in general (and RHEL 6 kernels in particular) often
contain features backported from later upstream kernels. This means
that the normal KERNEL_VERSION checks are not always correct for RHEL
kernels. This patch augments a number of such tests to be compatible
with building compat-wireless on RHEL 6 kernels.
Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
This patch contains the bluetooth backporting portions...
diff -up compat-wireless-2.6.36-4/patches/16-bluetooth.patch.orig compat-wireless-2.6.36-4/patches/16-bluetooth.patch
--- compat-wireless-2.6.36-4/patches/16-bluetooth.patch.orig 2010-11-12 12:42:52.602182539 -0500
+++ compat-wireless-2.6.36-4/patches/16-bluetooth.patch 2010-11-12 12:46:36.049287339 -0500
@@ -35,7 +35,7 @@ here still, but for now we keep this her
}
EXPORT_SYMBOL(bt_sock_unregister);
-+#if (LINUX_VERSION_CODE > KERNEL_VERSION(2,6,32))
++#if (LINUX_VERSION_CODE > KERNEL_VERSION(2,6,32)) || (defined(RHEL_MAJOR) && (RHEL_MAJOR == 6))
static int bt_sock_create(struct net *net, struct socket *sock, int proto,
int kern)
+#else
@@ -48,7 +48,7 @@ here still, but for now we keep this her
read_lock(&bt_proto_lock);
if (bt_proto[proto] && try_module_get(bt_proto[proto]->owner)) {
-+#if (LINUX_VERSION_CODE > KERNEL_VERSION(2,6,32))
++#if (LINUX_VERSION_CODE > KERNEL_VERSION(2,6,32)) || (defined(RHEL_MAJOR) && (RHEL_MAJOR == 6))
err = bt_proto[proto]->create(net, sock, proto, kern);
+#else
+ err = bt_proto[proto]->create(net, sock, proto);
@@ -127,7 +127,7 @@ here still, but for now we keep this her
.obj_size = sizeof(struct hci_pinfo)
};
-+#if (LINUX_VERSION_CODE > KERNEL_VERSION(2,6,32))
++#if (LINUX_VERSION_CODE > KERNEL_VERSION(2,6,32)) || (defined(RHEL_MAJOR) && (RHEL_MAJOR == 6))
static int hci_sock_create(struct net *net, struct socket *sock, int protocol,
int kern)
+#else
@@ -180,7 +180,7 @@ here still, but for now we keep this her
return hidp_queue_report(session, buf, rsize);
}
-+#if (LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,34))
++#if (LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,34)) || (defined(RHEL_MAJOR) && (RHEL_MAJOR == 6))
static int hidp_output_raw_report(struct hid_device *hid, unsigned char *data, size_t count,
unsigned char report_type)
{
@@ -438,7 +438,7 @@ here still, but for now we keep this her
return sk;
}
-+#if (LINUX_VERSION_CODE > KERNEL_VERSION(2,6,32))
++#if (LINUX_VERSION_CODE > KERNEL_VERSION(2,6,32)) || (defined(RHEL_MAJOR) && (RHEL_MAJOR == 6))
static int rfcomm_sock_create(struct net *net, struct socket *sock,
int protocol, int kern)
+#else
@@ -505,7 +505,7 @@ here still, but for now we keep this her
return sk;
}
-+#if (LINUX_VERSION_CODE > KERNEL_VERSION(2,6,32))
++#if (LINUX_VERSION_CODE > KERNEL_VERSION(2,6,32)) || (defined(RHEL_MAJOR) && (RHEL_MAJOR == 6))
static int sco_sock_create(struct net *net, struct socket *sock, int protocol,
int kern)
+#else
@@ -532,7 +532,7 @@ here still, but for now we keep this her
.obj_size = sizeof(struct bt_sock)
};
-+#if (LINUX_VERSION_CODE > KERNEL_VERSION(2,6,32))
++#if (LINUX_VERSION_CODE > KERNEL_VERSION(2,6,32)) || (defined(RHEL_MAJOR) && (RHEL_MAJOR == 6))
static int bnep_sock_create(struct net *net, struct socket *sock, int protocol,
int kern)
+#else
@@ -547,7 +547,7 @@ here still, but for now we keep this her
.obj_size = sizeof(struct bt_sock)
};
-+#if (LINUX_VERSION_CODE > KERNEL_VERSION(2,6,32))
++#if (LINUX_VERSION_CODE > KERNEL_VERSION(2,6,32)) || (defined(RHEL_MAJOR) && (RHEL_MAJOR == 6))
static int cmtp_sock_create(struct net *net, struct socket *sock, int protocol,
int kern)
+#else
@@ -562,7 +562,7 @@ here still, but for now we keep this her
.obj_size = sizeof(struct bt_sock)
};
-+#if (LINUX_VERSION_CODE > KERNEL_VERSION(2,6,32))
++#if (LINUX_VERSION_CODE > KERNEL_VERSION(2,6,32)) || (defined(RHEL_MAJOR) && (RHEL_MAJOR == 6))
static int hidp_sock_create(struct net *net, struct socket *sock, int protocol,
int kern)
+#else
@@ -577,7 +577,7 @@ here still, but for now we keep this her
return sk;
}
-+#if (LINUX_VERSION_CODE > KERNEL_VERSION(2,6,32))
++#if (LINUX_VERSION_CODE > KERNEL_VERSION(2,6,32)) || (defined(RHEL_MAJOR) && (RHEL_MAJOR == 6))
static int l2cap_sock_create(struct net *net, struct socket *sock, int protocol,
int kern)
+#else
--
John W. Linville Someday the world will need a hero, and you
linville@tuxdriver.com might be all we have. Be ready.
^ permalink raw reply
* [PATCH] compat: support building on RHEL 6 kernels
From: John W. Linville @ 2010-11-12 18:11 UTC (permalink / raw)
To: linux-wireless, linux-bluetooth
RHEL kernels in general (and RHEL 6 kernels in particular) often
contain features backported from later upstream kernels. This means
that the normal KERNEL_VERSION checks are not always correct for RHEL
kernels. This patch augments a number of such tests to be compatible
with building compat-wireless on RHEL 6 kernels.
Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
This patch contains the general compat portions...
diff -up compat-wireless-2.6.36-4/compat/compat_firmware_class.c.orig compat-wireless-2.6.36-4/compat/compat_firmware_class.c
--- compat-wireless-2.6.36-4/compat/compat_firmware_class.c.orig 2010-11-12 12:00:39.050193706 -0500
+++ compat-wireless-2.6.36-4/compat/compat_firmware_class.c 2010-11-12 12:10:47.288513373 -0500
@@ -314,9 +314,15 @@ static ssize_t firmware_loading_store(st
static DEVICE_ATTR(loading, 0644, firmware_loading_show, firmware_loading_store);
+#if !defined(RHEL_MAJOR) || (RHEL_MAJOR != 6)
static ssize_t firmware_data_read(struct kobject *kobj,
struct bin_attribute *bin_attr,
char *buffer, loff_t offset, size_t count)
+#else
+static ssize_t firmware_data_read(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *bin_attr,
+ char *buffer, loff_t offset, size_t count)
+#endif
{
struct device *dev = to_dev(kobj);
struct firmware_priv *fw_priv = to_firmware_priv(dev);
@@ -407,9 +413,15 @@ static int fw_realloc_buffer(struct firm
* Data written to the 'data' attribute will be later handed to
* the driver as a firmware image.
**/
+#if !defined(RHEL_MAJOR) || (RHEL_MAJOR != 6)
static ssize_t firmware_data_write(struct kobject *kobj,
struct bin_attribute *bin_attr,
char *buffer, loff_t offset, size_t count)
+#else
+static ssize_t firmware_data_write(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *bin_attr,
+ char *buffer, loff_t offset, size_t count)
+#endif
{
struct device *dev = to_dev(kobj);
struct firmware_priv *fw_priv = to_firmware_priv(dev);
diff -up compat-wireless-2.6.36-4/include/linux/compat-2.6.33.h.orig compat-wireless-2.6.36-4/include/linux/compat-2.6.33.h
--- compat-wireless-2.6.36-4/include/linux/compat-2.6.33.h.orig 2010-11-12 10:25:13.561172060 -0500
+++ compat-wireless-2.6.36-4/include/linux/compat-2.6.33.h 2010-11-12 11:19:10.369172382 -0500
@@ -47,7 +47,9 @@ static inline void compat_release_firmwa
}
#endif
+#if (!defined(RHEL_MAJOR) || (RHEL_MAJOR != 6))
#define KEY_RFKILL 247 /* Key that controls all radios */
+#endif
#define IFF_DONT_BRIDGE 0x800 /* disallow bridging this ether dev */
/* source: include/linux/if.h */
@@ -55,6 +57,7 @@ static inline void compat_release_firmwa
/* this will never happen on older kernels */
#define NETDEV_POST_INIT 0xffff
+#if (!defined(RHEL_MAJOR) || (RHEL_MAJOR != 6))
static inline struct sk_buff *netdev_alloc_skb_ip_align(struct net_device *dev,
unsigned int length)
{
@@ -64,6 +67,7 @@ static inline struct sk_buff *netdev_all
skb_reserve(skb, NET_IP_ALIGN);
return skb;
}
+#endif
#if defined(CONFIG_PCCARD) || defined(CONFIG_PCCARD_MODULE)
diff -up compat-wireless-2.6.36-4/include/linux/compat-2.6.34.h.orig compat-wireless-2.6.36-4/include/linux/compat-2.6.34.h
--- compat-wireless-2.6.36-4/include/linux/compat-2.6.34.h.orig 2010-11-12 10:25:44.165172202 -0500
+++ compat-wireless-2.6.36-4/include/linux/compat-2.6.34.h 2010-11-12 11:19:29.315182613 -0500
@@ -19,12 +19,14 @@
/* netdev_printk helpers, similar to dev_printk */
+#if (!defined(RHEL_MAJOR) || (RHEL_MAJOR != 6))
static inline const char *netdev_name(const struct net_device *dev)
{
if (dev->reg_state != NETREG_REGISTERED)
return "(unregistered net_device)";
return dev->name;
}
+#endif
#define netdev_printk(level, netdev, format, args...) \
dev_printk(level, (netdev)->dev.parent, \
--
John W. Linville Someday the world will need a hero, and you
linville@tuxdriver.com might be all we have. Be ready.
^ permalink raw reply
* Re: [PATCH v2 1/7] Fix invalid memory access when EIR field length is zero
From: Gustavo F. Padovan @ 2010-11-12 17:38 UTC (permalink / raw)
To: Inga Stotland, 'Vinicius Costa Gomes', linux-bluetooth,
'Bruna Moreira'
In-Reply-To: <20101112165434.GA13238@jh-x301>
Hi Johan,
* Johan Hedberg <johan.hedberg@gmail.com> [2010-11-12 18:54:34 +0200]:
> Hi Inga,
>
> On Thu, Nov 11, 2010, Inga Stotland wrote:
> > Was there a bug to begin with? :)
> > The access to eir_data[1] was always valid due to the check (len <
> > EIR_DATA_LENGTH - 1)
> > and the fact that eir_data is a buffer of fixed length of EIR_DATA_LENGTH
> > (240 bytes).
>
> On closer inspection it seems you might be right, however it'd be nice
> to get some comments from the original patch author about this (were
> there e.g. crashes or some valgrind warnings observed or was this just
> speculation based on looking at the code).
>
> Btw, it seems I may need to slow down on my response time to patches so
> there's better time for other people to review them too. E.g. both you
> and Luiz were a bit late to the game on a couple of recent patches.
> Maybe a 24 hour period before I push anything might be good enough?
I would say 48h, give more time to people review, in case you spent a
whole day off the linux-bluetooth.
--
Gustavo F. Padovan
http://profusion.mobi
^ permalink raw reply
* Re: [PATCH] Fix pull phonebook with non-zero offset parameter
From: Johan Hedberg @ 2010-11-12 17:10 UTC (permalink / raw)
To: Rafal Michalski; +Cc: linux-bluetooth
In-Reply-To: <1289558357-25724-1-git-send-email-michalski.raf@gmail.com>
Hi Rafal,
On Fri, Nov 12, 2010, Rafal Michalski wrote:
> For non-zero liststartoffset parameter, contacts which index was before
> offset were indexed more than once (e.g. when contact had more than one
> phone number or address etc.), so pulling was started earlier - before
> offset index was reached. This patch fix this problem and each contact
> is indexed only once.
> ---
> plugins/phonebook-tracker.c | 11 ++++++++++-
> 1 files changed, 10 insertions(+), 1 deletions(-)
Several issues with this patch:
> @@ -1361,6 +1361,7 @@ static void pull_contacts(char **reply, int num_fields, void *user_data)
> int last_index, i;
> gboolean cdata_present = FALSE;
> char *home_addr, *work_addr, *other_addr;
> + static gchar *temp_id;
Use char instead of gchar and initialize this to NULL here (static
variabled get initialized to 0 implicitly, but it's good to have this
explicit for clarity imo).
> + if (data->index == 0)
> + temp_id = NULL;
Is this supposed to compensate for the lack of initialization upon
declaration or do you really want to set to NULL here in case some
previous calls to the function left it as non-NULL. In the later case
you're missing a g_free call.
> @@ -1495,6 +1503,7 @@ done:
> fail:
> g_slist_free(data->contacts);
> g_free(data);
> + g_free(temp_id);
> }
Since this is a static variable you don't want it keeping it's value
after freeing it. So set to NULL after the g_free.
Btw, doesn't it bother you at all that this pull_contacts function is
huge? Feel free to send a refactoring patch for it. E.g. separate
functions called add_entry and add_numbers might not be a bad idea. The
function is also inconsistent in the usage of column numbers: sometimes
a define is used and sometimes a hard coded number.
Johan
^ permalink raw reply
* Re: [PATCH v2 1/7] Fix invalid memory access when EIR field length is zero
From: Johan Hedberg @ 2010-11-12 16:54 UTC (permalink / raw)
To: Inga Stotland
Cc: 'Vinicius Costa Gomes', linux-bluetooth,
'Bruna Moreira'
In-Reply-To: <000b01cb8200$02c24c90$0846e5b0$@org>
Hi Inga,
On Thu, Nov 11, 2010, Inga Stotland wrote:
> Was there a bug to begin with? :)
> The access to eir_data[1] was always valid due to the check (len <
> EIR_DATA_LENGTH - 1)
> and the fact that eir_data is a buffer of fixed length of EIR_DATA_LENGTH
> (240 bytes).
On closer inspection it seems you might be right, however it'd be nice
to get some comments from the original patch author about this (were
there e.g. crashes or some valgrind warnings observed or was this just
speculation based on looking at the code).
Btw, it seems I may need to slow down on my response time to patches so
there's better time for other people to review them too. E.g. both you
and Luiz were a bit late to the game on a couple of recent patches.
Maybe a 24 hour period before I push anything might be good enough?
Johan
^ 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