* Re: [PATCH 2/6] Add Find By Type Value Response encoding/decoding functions
From: Johan Hedberg @ 2010-11-18 14:47 UTC (permalink / raw)
To: Claudio Takahasi; +Cc: linux-bluetooth
In-Reply-To: <1290017386-23969-2-git-send-email-claudio.takahasi@openbossa.org>
Hi Claudio,
On Wed, Nov 17, 2010, Claudio Takahasi wrote:
> --- a/attrib/att.h
> +++ b/attrib/att.h
> @@ -122,6 +122,11 @@ struct att_data_list {
> uint8_t **data;
> };
>
> +struct range {
> + uint16_t start;
> + uint16_t end;
> +};
> +
Since this is in the public header file it should be prefixed with att_,
right? (which should also apply for all public symbols here). However,
it seems like you're only using the struct internally in att.c so
probably it doesn't even need to be in the public header file?
Johan
^ permalink raw reply
* Re: [PATCH 1/6] Implement Find by Type request encode/decoding
From: Johan Hedberg @ 2010-11-18 14:45 UTC (permalink / raw)
To: Claudio Takahasi; +Cc: linux-bluetooth, Bruna Moreira
In-Reply-To: <1290017386-23969-1-git-send-email-claudio.takahasi@openbossa.org>
Hi,
On Wed, Nov 17, 2010, Claudio Takahasi wrote:
> From: Bruna Moreira <bruna.moreira@openbossa.org>
>
> ---
> attrib/att.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> attrib/att.h | 4 ++-
> 2 files changed, 73 insertions(+), 3 deletions(-)
Pushed upstream. Thanks.
Johan
^ permalink raw reply
* Re: [PATCH 3/3] Emit "DeviceFound" signal for LE devices
From: Johan Hedberg @ 2010-11-18 14:44 UTC (permalink / raw)
To: Bruna Moreira; +Cc: linux-bluetooth
In-Reply-To: <1290009593-13658-3-git-send-email-bruna.moreira@openbossa.org>
Hi Bruna,
On Wed, Nov 17, 2010, Bruna Moreira wrote:
> The adapter_emit_device_found() function was modified to emit
> DeviceFound signal for LE devices as well.
> ---
> src/adapter.c | 30 +++++++++++++++++++++++++-----
> 1 files changed, 25 insertions(+), 5 deletions(-)
This patch has also been pushed upstream. Thanks.
Johan
^ permalink raw reply
* Re: [PATCH 2/3] Extract service UUIDs from advertising data
From: Johan Hedberg @ 2010-11-18 14:42 UTC (permalink / raw)
To: Anderson Lizardo; +Cc: Bruna Moreira, linux-bluetooth
In-Reply-To: <AANLkTi=xBthrmGDV0pUK_Tbw-Lob4gT9RmTgPrPAYj7M@mail.gmail.com>
Hi Anderson,
On Thu, Nov 18, 2010, Anderson Lizardo wrote:
> > What's the reason that the get_eir_uuids API is designed so that it can
> > handle a list which already contains elements before calling the
> > function? Since you free the list right after creating the uuids array
> > it seems like this shouldn't happen. Or am I missing something?
>
> The list is destroyed here just to keep the old semantics for BR/EDR,
> which ignores service UUIDs from previous EIR data.
>
> For LE devices (as you can see on the following 3/3 patch) we do not
> destroy the previous list when new advertising data is parsed.
> Instead, we "merge" the UUIDs list.
Fair enough. I've pushed the patch upstream now.
> Do you think we can unify semantics of BR/EDR and LE and always merge
> the service UUIDs for both EIR and advertising data?
Yeah, I think that could make sense. Feel free to send a patch for it.
> On the current code, the UUIDs array is always created on each EIR
> data (inside get_eir_uuids()) and destroyed right after the
> DeviceFound signal is emitted. For simplicity, we kept this same
> behavior.
>
> I agree we can make some optimization here and avoid heap
> allocations/deallocations when UUIDs do not change. One idea might be
> to keep the uuids char* array as well as the GSList on the
> remote_dev_info struct, and only recreate this array if any new UUIDs
> were added to the GSList (this can be identified if the uuidcount has
> changed, because we never delete UUIDs). What do you think?
Yes, that sounds like a good optimization.
Johan
^ permalink raw reply
* Re: [PATCH 2/3] Extract service UUIDs from advertising data
From: Anderson Lizardo @ 2010-11-18 14:03 UTC (permalink / raw)
To: Bruna Moreira, linux-bluetooth
In-Reply-To: <20101118124320.GA4918@jh-x301>
On Thu, Nov 18, 2010 at 8:43 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi Bruna,
>
> On Wed, Nov 17, 2010, Bruna Moreira wrote:
>> + /* Extract UUIDs from extended inquiry response if any */
>> + dev->services = get_eir_uuids(eir_data, eir_length, dev->services);
>> + uuid_count = g_slist_length(dev->services);
>> +
>> + if (dev->services) {
>> + uuids = strlist2array(dev->services);
>> + g_slist_foreach(dev->services, (GFunc) g_free, NULL);
>> + g_slist_free(dev->services);
>> + dev->services = NULL;
>> + }
>
> What's the reason that the get_eir_uuids API is designed so that it can
> handle a list which already contains elements before calling the
> function? Since you free the list right after creating the uuids array
> it seems like this shouldn't happen. Or am I missing something?
The list is destroyed here just to keep the old semantics for BR/EDR,
which ignores service UUIDs from previous EIR data.
For LE devices (as you can see on the following 3/3 patch) we do not
destroy the previous list when new advertising data is parsed.
Instead, we "merge" the UUIDs list.
Do you think we can unify semantics of BR/EDR and LE and always merge
the service UUIDs for both EIR and advertising data?
> Btw, is there something that could be optimized here since it seems like
> you're regenerating the uuids array before every signal even though the
> set of service might not have changed since the previous one. Maybe you
> should track this and only regenerate the array if there has been a
> changed from the previous signal. What do you think?
On the current code, the UUIDs array is always created on each EIR
data (inside get_eir_uuids()) and destroyed right after the
DeviceFound signal is emitted. For simplicity, we kept this same
behavior.
I agree we can make some optimization here and avoid heap
allocations/deallocations when UUIDs do not change. One idea might be
to keep the uuids char* array as well as the GSList on the
remote_dev_info struct, and only recreate this array if any new UUIDs
were added to the GSList (this can be identified if the uuidcount has
changed, because we never delete UUIDs). What do you think?
Regards,
--
Anderson Lizardo
OpenBossa Labs - INdT
Manaus - Brazil
^ permalink raw reply
* Re: [PATCHv3 3/7] Add DBus OOB API documentation.
From: Johan Hedberg @ 2010-11-18 13:45 UTC (permalink / raw)
To: Szymon Janc; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <201011181151.48796.szymon.janc@tieto.com>
Hi Szymon,
On Thu, Nov 18, 2010, Szymon Janc wrote:
> OOB hierarchy
> =================
>
> Service unique name
> Interface org.bluez.OOB
I suppose this should be called org.bluez.OobProvider, OobAgent or
something similar?
> Methods array{byte} hash, array{byte} randomizer
> RequestRemoteOobData(object device)
>
> This method gets called when the service daemon needs to
> get device's hash and randomizer for an OOB
> authentication. Each array should be 16 bytes long.
>
> Possible errors: org.bluez.Error.NoData
>
> void Release()
>
> This method gets called when D-Bus 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
Interface definitions are supposed to be unique. You can't reuse the
same interface name for a different set of methods and signals.
> Object path /
Regarding this, as you said the healt stuff is using /org/bluez. I'd
like to have someone who knows comment on the reasons why /org/bluez was
chosen instead of /. It seems inconsistent to me since the manager
interface uses /.
> --------------------------------------------------------------------------------
>
> Service org.bluez
> Interface org.bluez.Adapter
> Object path [variable prefix]/{hci0,hci1,...}
>
> array{byte} hash, array{byte} randomizer ReadLocalOobData()
I agree that this is good to have in the Adapter path, but we can still
discuss about whether it should be on it's own interface or not. Maybe
reusing the adapter path is fine since we're only dealing with one
method in it.
> Service unique name
> Interface org.bluez.Agent
> Object path freely definable
>
> Methods void RequestPairingApproval(object device)
I still like RequestPairing better (it's shorter). Marcel, do you have
any opinion or new suggestions for this? It would be reused for incoming
just-works pairing too (though only for 5.x).
Johan
^ permalink raw reply
* [PATCH] Add iwmmxt optimization for sbc for pxa series cpu
From: Keith Mok @ 2010-11-18 13:33 UTC (permalink / raw)
To: Siarhei Siamashka; +Cc: linux-bluetooth
In-Reply-To: <201011181505.29428.siarhei.siamashka@gmail.com>
Add iwmmxt optimization for sbc for pxa series cpu.
Benchmarked on ARM PXA platform:
=== 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
---
Makefile.am | 1 +
sbc/sbc_primitives.c | 4 +
sbc/sbc_primitives_iwmmxt.c | 304 +++++++++++++++++++++++++++++++++++++++++++
sbc/sbc_primitives_iwmmxt.h | 42 ++++++
4 files changed, 351 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..213967e
--- /dev/null
+++ b/sbc/sbc_primitives_iwmmxt.c
@@ -0,0 +1,304 @@
+/*
+ *
+ * Bluetooth low-complexity, subband codec (SBC) library
+ *
+ * Copyright (C) 2010 Keith Mok <ek9852@gmail.com>
+ * Copyright (C) 2008-2010 Nokia Corporation
+ * Copyright (C) 2004-2010 Marcel Holtmann <marcel@holtmann.org>
+ * Copyright (C) 2004-2005 Henryk Ploetz <henryk@ploetzli.ch>
+ * Copyright (C) 2005-2006 Brad Midgley <bmidgley@xmission.com>
+ *
+ *
+ * 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..b535e68
--- /dev/null
+++ b/sbc/sbc_primitives_iwmmxt.h
@@ -0,0 +1,42 @@
+/*
+ *
+ * Bluetooth low-complexity, subband codec (SBC) library
+ *
+ * Copyright (C) 2010 Keith Mok <ek9852@gmail.com>
+ * Copyright (C) 2008-2010 Nokia Corporation
+ * Copyright (C) 2004-2010 Marcel Holtmann <marcel@holtmann.org>
+ * Copyright (C) 2004-2005 Henryk Ploetz <henryk@ploetzli.ch>
+ * Copyright (C) 2005-2006 Brad Midgley <bmidgley@xmission.com>
+ *
+ *
+ * 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 v3] Add iwmmxt optimization for sbc for pxa series cpu
From: Johan Hedberg @ 2010-11-18 13:31 UTC (permalink / raw)
To: Siarhei Siamashka; +Cc: Keith Mok, linux-bluetooth
In-Reply-To: <201011181505.29428.siarhei.siamashka@gmail.com>
Hi Siarhei,
On Thu, Nov 18, 2010, Siarhei Siamashka wrote:
> 1. Make a final patch in such a form that can be pushed to git repository
> without any modifications, it means that you need a clean commit message and
> not just some text intermixed with the parts and quotations of discussion from
> this mailing list.
> 2. "Signed-off-by" header is not needed for the userspace parts of bluez.
> 3. All files must have copyright notices, even a small one like
> 'sbc_primitives_iwmmxt.h'. And probably you should just replicate all the
> copyright notices from the source files with sbc mmx optimizations and add your
> own copyright on top.
>
> Hopefully that should be enough to get your optimizations applied. Thanks.
Yep, those things would be needed before pushing upstream. Thanks for
reminding me about this patch. I had actually forgotten about it.
Johan
^ permalink raw reply
* Re: [PATCH 1/3] Advertising data: extract local name
From: Anderson Lizardo @ 2010-11-18 13:29 UTC (permalink / raw)
To: Bruna Moreira, linux-bluetooth
In-Reply-To: <20101118123310.GA4101@jh-x301>
Hi Johan,
On Thu, Nov 18, 2010 at 8:33 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Variables should be always declared in the smallest possible scope, so
> your new type variable is in the wrong place (it should be declared
> inside the if-statement. Since this was the only issue I found with this
> patch I fixed it myself and pushed it upstream. Btw, is it really safe
> to ignore the type here? What if it's EIR_NAME_SHORT? Wouldn't you then
> want to perform full name discovery using e.g. the GAP GATT service
> later?
The idea is to populate dev->name with some temporary information
available on the advertising data, and later when GATT service
discovery happens (as part of device creation), we get the definitive
name. At the moment, the GATT service discovery is not happening
automatically (but Claudio's patches are a beginning of that, IIRC),
so the advertising information is the only name source for LE devices,
currently.
Of course, we need to make sure that once name resolution through GATT
service discovery is done, any future names in advertising data would
be ignored, but we have not implemented this yet.
Regards,
--
Anderson Lizardo
OpenBossa Labs - INdT
Manaus - Brazil
^ permalink raw reply
* Re: [PATCH v3] Add iwmmxt optimization for sbc for pxa series cpu
From: Siarhei Siamashka @ 2010-11-18 13:05 UTC (permalink / raw)
To: Keith Mok; +Cc: linux-bluetooth
In-Reply-To: <201011151308.31972.siarhei.siamashka@gmail.com>
On Monday 15 November 2010 13:08:19 Siarhei Siamashka wrote:
> 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.
As nobody else has stepped in, I guess it's still my responsibility to provide
some further guidance even though I'm a very infrequent contributor myself.
Hopefully somebody will correct me if I'm wrong.
So please
1. Make a final patch in such a form that can be pushed to git repository
without any modifications, it means that you need a clean commit message and
not just some text intermixed with the parts and quotations of discussion from
this mailing list.
2. "Signed-off-by" header is not needed for the userspace parts of bluez.
3. All files must have copyright notices, even a small one like
'sbc_primitives_iwmmxt.h'. And probably you should just replicate all the
copyright notices from the source files with sbc mmx optimizations and add your
own copyright on top.
Hopefully that should be enough to get your optimizations applied. Thanks.
--
Best regards,
Siarhei Siamashka
^ permalink raw reply
* Re: [PATCH 2/3] Extract service UUIDs from advertising data
From: Johan Hedberg @ 2010-11-18 12:43 UTC (permalink / raw)
To: Bruna Moreira; +Cc: linux-bluetooth
In-Reply-To: <1290009593-13658-2-git-send-email-bruna.moreira@openbossa.org>
Hi Bruna,
On Wed, Nov 17, 2010, Bruna Moreira wrote:
> + /* Extract UUIDs from extended inquiry response if any */
> + dev->services = get_eir_uuids(eir_data, eir_length, dev->services);
> + uuid_count = g_slist_length(dev->services);
> +
> + if (dev->services) {
> + uuids = strlist2array(dev->services);
> + g_slist_foreach(dev->services, (GFunc) g_free, NULL);
> + g_slist_free(dev->services);
> + dev->services = NULL;
> + }
What's the reason that the get_eir_uuids API is designed so that it can
handle a list which already contains elements before calling the
function? Since you free the list right after creating the uuids array
it seems like this shouldn't happen. Or am I missing something?
Btw, is there something that could be optimized here since it seems like
you're regenerating the uuids array before every signal even though the
set of service might not have changed since the previous one. Maybe you
should track this and only regenerate the array if there has been a
changed from the previous signal. What do you think?
Johan
^ permalink raw reply
* Re: [PATCH 1/3] Advertising data: extract local name
From: Johan Hedberg @ 2010-11-18 12:33 UTC (permalink / raw)
To: Bruna Moreira; +Cc: linux-bluetooth
In-Reply-To: <1290009593-13658-1-git-send-email-bruna.moreira@openbossa.org>
Hi Bruna,
On Wed, Nov 17, 2010, Bruna Moreira wrote:
> @@ -3007,6 +3007,7 @@ void adapter_update_device_from_info(struct btd_adapter *adapter,
> bdaddr_t bdaddr;
> gboolean new_dev;
> int8_t rssi;
> + uint8_t type;
>
> rssi = *(info->data + info->length);
> bdaddr = info->bdaddr;
> @@ -3023,6 +3024,14 @@ 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) {
> + g_free(dev->name);
> + dev->name = tmp_name;
> + }
> + }
Variables should be always declared in the smallest possible scope, so
your new type variable is in the wrong place (it should be declared
inside the if-statement. Since this was the only issue I found with this
patch I fixed it myself and pushed it upstream. Btw, is it really safe
to ignore the type here? What if it's EIR_NAME_SHORT? Wouldn't you then
want to perform full name discovery using e.g. the GAP GATT service
later?
Johan
^ permalink raw reply
* Re: [PATCH v3 2/2] Split pull_contacts to smaller functions
From: Johan Hedberg @ 2010-11-18 12:22 UTC (permalink / raw)
To: Bartosz Szatkowski; +Cc: linux-bluetooth
In-Reply-To: <1290078266-16279-1-git-send-email-bulislaw@linux.com>
Hi Bartosz,
On Thu, Nov 18, 2010, Bartosz Szatkowski wrote:
> Parts of pull_contact responsible for filling contact fields moved
> to new functions.
> ---
> plugins/phonebook-tracker.c | 176 ++++++++++++++++++++++++++----------------
> 1 files changed, 109 insertions(+), 67 deletions(-)
Thanks. The patch looks now fine and has been pushed upstream.
Johan
^ permalink raw reply
* Re: [PATCH v3] Adding a new option to specify security level for gatttool
From: Johan Hedberg @ 2010-11-18 12:21 UTC (permalink / raw)
To: Sheldon Demario; +Cc: linux-bluetooth
In-Reply-To: <1290000400-20001-1-git-send-email-sheldon.demario@openbossa.org>
Hi Sheldon,
On Wed, Nov 17, 2010, Sheldon Demario wrote:
> ---
> TODO | 6 ------
> attrib/gatttool.c | 15 +++++++++++++--
> 2 files changed, 13 insertions(+), 8 deletions(-)
The patch has been pushed upstream. Thanks.
Johan
^ permalink raw reply
* Re: [PATCH] Adding a new option to specify security level for gatttool
From: Johan Hedberg @ 2010-11-18 12:19 UTC (permalink / raw)
To: tim.howes; +Cc: Mike.Tsai, linux-bluetooth
In-Reply-To: <1AFE20D16950C745A2A83986B72E8748011F571E7497@EMEXM3131.dir.svc.accenture.com>
Hi Tim,
Nice to see you on this list! :)
On Wed, Nov 17, 2010, tim.howes@accenture.com wrote:
> > [Mtsai] I am not sure what are the definition of "low", "medium" or
> > "high". By the spec of Core 4.0, LE has 2 security modes and different
> > security levels based on the method of pairing (or bonding). It may be
> > appeal to end user with "low", "medium" and "high" definition, but it
> > can't be reference with LE spec. I would suggest, instead, following
> > terms,
> >
> > "No security",
> > "unauthenticated encryption",
> > "authenticated encryption",
> > "unauthenticated data signing",
> > "authenticated data signing,
>
> To some extent I agree; however, the semantics of such an API would
> have to be careful. A particular profile should not "force" data
> signing because if the link is already encrypted there is little point
> using data signing. So from that point of view exposing a more
> abstract API (a bit like "high") is better. However, it is hard to
> map "high" onto any of the ones you listed (which I agree is a good
> list). So perhaps it is better to have the API semantics as
> "advisory" or "requests" which can be fulfilled by the underlying
> stack in other ways (eg encryption for data-signing).
Something like that will probably be needed, yes. However the idea of
the current command line switch to gatttool is to simply map to the
existing kernel API, and that API only has low, medium and high. So at
least in the short term the patch is fine.
Johan
^ permalink raw reply
* [PATCH v3 2/2] Split pull_contacts to smaller functions
From: Bartosz Szatkowski @ 2010-11-18 11:04 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Bartosz Szatkowski
Parts of pull_contact responsible for filling contact fields moved
to new functions.
---
plugins/phonebook-tracker.c | 176 ++++++++++++++++++++++++++----------------
1 files changed, 109 insertions(+), 67 deletions(-)
diff --git a/plugins/phonebook-tracker.c b/plugins/phonebook-tracker.c
index 71ed2f0..6d8915e 100644
--- a/plugins/phonebook-tracker.c
+++ b/plugins/phonebook-tracker.c
@@ -1395,6 +1395,110 @@ static void add_affiliation(char **field, const char *value)
*field = g_strdup(value);
}
+static void contact_init(struct phonebook_contact *contact, char **reply)
+{
+
+ 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)
+ return;
+
+ if (g_strcmp0(reply[COL_OTHER_NUMBER], reply[COL_WORK_NUMBER]) == 0)
+ return;
+
+ if (g_strcmp0(reply[COL_OTHER_NUMBER], reply[COL_HOME_NUMBER]) == 0)
+ return;
+
+ 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 +1508,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;
static char *temp_id = NULL;
if (num_fields < 0) {
@@ -1456,74 +1559,13 @@ 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[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
* Re: [PATCHv3 3/7] Add DBus OOB API documentation.
From: Szymon Janc @ 2010-11-18 10:51 UTC (permalink / raw)
To: Johan Hedberg; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <201011171349.51512.szymon.janc@tieto.com>
> > > +Service org.bluez
> > > +Interface org.bluez.OOB
> > > +Object path /org/bluez
> >
> > I think this should use the same path as the Manager API, which at the
> > moment is /
>
> I was following HealthManager way, but I'm fine if you prefer / path.
>
> > > + array{byte} hash, array{byte} randomizer
> > > + ReadLocalOobData(object adapter)
> >
> > Having to pass an adapter path as a parameter seems weird. Wouldn't it
> > make more sense to have this method in the Adapter path/interface
> > instead? Also, we could toy around with the thought of putting the two
> > other methods into the current Manager interface.
>
> It is OK to me to move ReadLocalOobData to adapter path as this is an adapter
> related data.
>
> My initial idea was to keep all oob related methods in org.bluez.OOB interface
> but I'm not going to insist on it.
Please see the following proposal.
Register/UnregisterProvider methods can be easily moved to manager interface if
decided (with name changes to i.e. RegisterOobProvider for clarity).
Moving ReadLocalOobData to adapter path requires some code changes in dbusoob
plugin so I prefer to clarify that before coding.
Any comments?
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{byte} hash, array{byte} randomizer
RequestRemoteOobData(object device)
This method gets called when the service daemon needs to
get device's hash and randomizer for an OOB
authentication. Each array should be 16 bytes long.
Possible errors: org.bluez.Error.NoData
void Release()
This method gets called when D-Bus 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 /
Methods void RegisterProvider(object provider)
This method registers provider for D-Bus OOB plug-in.
When provider is successfully registered plug-in becomes
active. Only one provider can be registered at time.
Possible errors: org.bluez.Error.AlreadyExists
void UnregisterProvider(object provider)
This method unregisters provider for D-Bus OOB plug-in.
Possible errors: org.bluez.Error.DoesNotExist
--------------------------------------------------------------------------------
Service org.bluez
Interface org.bluez.Adapter
Object path [variable prefix]/{hci0,hci1,...}
array{byte} hash, array{byte} randomizer ReadLocalOobData()
This method reads local OOB data for adapter. Return
value is pair of arrays 16 bytes each. Only registered
provider should call this method. This method is
available only for 2.1 (or higher) adapters.
Note: This method will generate and return new local
OOB data.
Possible errors: org.bluez.Error.ReadFailed
org.bluez.Error.NoProvider
org.bluez.Error.InProgress
--------------------------------------------------------------------------------
Service unique name
Interface org.bluez.Agent
Object path freely definable
Methods void RequestPairingApproval(object device)
This method gets called when the service daemon
needs to confirm incoming OOB pairing request.
Possible errors: org.bluez.Error.Rejected
org.bluez.Error.Canceled
--
Szymon Janc
on behalf of ST-Ericsson
^ permalink raw reply
* Re: Apple Wireless Keyboard connection issue
From: Johan Hedberg @ 2010-11-18 8:44 UTC (permalink / raw)
To: Andre Kuehne; +Cc: Gustavo F. Padovan, linux-bluetooth
In-Reply-To: <4CE48602.1000009@gmx.net>
Hi Andre,
On Thu, Nov 18, 2010, Andre Kuehne wrote:
> > git bisect good
> a352058752e541539b09e55124d411a534cc14af is the first bad commit
> commit a352058752e541539b09e55124d411a534cc14af
> Author: Johan Hedberg <johan.hedberg@nokia.com>
> Date: Thu Nov 4 04:38:35 2010 +0200
>
> Cache adapter address for quick lookup
That's actually not related to the HID issue. There's a later commit
that fixed the general initialization issue that this commit caused.
I was now able to get hold of a Bluetooth keyboard and fixed the issue.
The problem was indeed in the commit that Gustavo originally pointed
out. It's just that my original patch for it was incomplete and so
didn't properly fix the issue. Anyway, a proper fix has now been pushed
to git. Please test with that.
Johan
^ permalink raw reply
* Re: [PATCH v5] Bluetooth: btwilink driver
From: Pavan Savoy @ 2010-11-18 5:18 UTC (permalink / raw)
To: Gustavo F. Padovan; +Cc: Vitaly Wool, marcel, linux-bluetooth, linux-kernel
In-Reply-To: <20101117165248.GB21729@vigoh>
On Wed, Nov 17, 2010 at 10:22 PM, Gustavo F. Padovan
<padovan@profusion.mobi> wrote:
> Hi Pavan,
>
> * Pavan Savoy <pavan_savoy@sify.com> [2010-11-17 11:13:26 +0530]:
>
>> On Wed, Nov 17, 2010 at 4:50 AM, Vitaly Wool <vitalywool@gmail.com> wrot=
e:
>> >>> + =C2=A0 =C2=A0 /* Registration with ST layer is successful,
>> >>> + =C2=A0 =C2=A0 =C2=A0* hardware is ready to accept commands from HC=
I core.
>> >>> + =C2=A0 =C2=A0 =C2=A0*/
>> >>> + =C2=A0 =C2=A0 if (test_and_set_bit(HCI_RUNNING, &hdev->flags)) {
>> >>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 clear_bit(HCI_RUNNING, &=
hdev->flags);
>> >>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 err =3D st_unregister(ST=
_BT);
>> >>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (err)
>> >>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 BT_ERR("st_unregister() failed with error %d", err);
>> >>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 hst->st_write =3D NULL;
>> >>> + =C2=A0 =C2=A0 }
>> >>
>> >>
>> >> What are you trying to do here? test_and_set_bit() result doesn't say
>> >> nothing about error and you shall put test_and_set_bit should be in t=
he
>> >> beginning, to know if your device is already opened or not and then
>> >> clear_bit if some error ocurrs during the function.
>> >>
>> >
>> > Yeap, this piece of code beats me is well. Why is it an error if this
>> > bit wasn't already set?
>>
>> Vitaly, Gustavo,
>>
>> I suppose I never understood HCI_RUNNING flag that way, as in an error
>> check mechanism to avoid multiple hci0 ups.
>>
>> What I understood was that HCI_RUNNING suggested as to when hci0 was
>> ready to be used. With this understanding, I wanted to make sure I
>> downloaded the firmware for the chip before I proclaim to the world
>> that the hci0 is ready to be used, as in HCI_RUNNING.
>>
>> For example, I didn't want my _send_frame to be called before I did
>> the firmware download - since firmware download takes time - 45kb
>> send/wait commands :(
>>
>> But I suppose I now understand - What I would rather do is test_bit in
>> the beginning of function and do a set_bit at the end of function -
>> does this make sense ?
>
> It does, but does it as test_and_set and then clear if error as we do in
> other drivers.
Ok, I understand, will do it this way.
However, still I am not too convinced - I honestly don't want to set
HCI_RUNNING before firmware download required for WiLink happens - and
this happens inside the st_register function here.
So the question again, How can I ensure _send_frame is not called when
firmware download is in progress - one of the major reasons why I
delayed the setting of HCI_RUNNING.
As mentioned before I will go ahead and create the patch - But would
still like to have an answer to this.
> Gustavo F. Padovan
> http://profusion.mobi
>
^ permalink raw reply
* Re: Apple Wireless Keyboard connection issue
From: Andre Kuehne @ 2010-11-18 1:48 UTC (permalink / raw)
To: Gustavo F. Padovan, linux-bluetooth
In-Reply-To: <20101117222751.GA20735@jh-x301>
[-- Attachment #1: Type: text/plain, Size: 1673 bytes --]
On 11/17/2010 11:27 PM, Johan Hedberg wrote:
> Hi,
>
> On Thu, Nov 18, 2010, Andre Kuehne wrote:
>> On 11/17/2010 10:05 AM, Johan Hedberg wrote:
>>> On Tue, Nov 16, 2010, Gustavo F. Padovan wrote:
>>>> * "André Kühne"<andre.kuehne@gmx.net> [2010-11-16 22:27:01 +0100]:
>>>>> I noticed the following on my system: After upgrading to bluez-4.79
>>>>> connecting my Apple Wireless Keyboard does not work anymore. With
>>>>> bluez-4.77 the connection works just fine.
>>>>
>>>> My Microsoft keyboard is not working too. That is probably due to commit
>>>> abe7cd44124a from Johan. It should be fixed soon.
>>>
>>> The only real difference that patch makes is the reuse of the HCI socket
>>> inside hciops.c. Maybe that screws up the event filters somehow or
>>> something similar. I don't have a keyboard to verify a fix, but could
>>> you try the attached patch and see if it helps?
>>>
>> I installed
>>
>> http://www.kernel.org/pub/linux/bluetooth/bluez-4.79.tar.gz + hciops_encrypt.patch
>>
>> but the patch does not work for me. For verification I downloaded
>>
>> http://www.kernel.org/pub/linux/bluetooth/bluez-4.77.tar.gz
>>
>> and compiled/installed it the same way (without the patch) and the connection works.
>>
>> I did the same using bluez-4.78 with the result that both my keyboard and mouse failed to connect.
>
> Ok, so then it seems like abe7cd44124a might not be to blame. Could you
> do a proper git bisect so we can figure out exactly at which point this
> broke. abe7cd44124a is really the only commit touching HID code since
> 4.77 so this is starting to get a bit strange.
>
> Johan
>
Please find attached my bisect results.
Best regards
Andre
[-- Attachment #2: bluez-bisect.txt --]
[-- Type: text/plain, Size: 1540 bytes --]
> git bisect good
a352058752e541539b09e55124d411a534cc14af is the first bad commit
commit a352058752e541539b09e55124d411a534cc14af
Author: Johan Hedberg <johan.hedberg@nokia.com>
Date: Thu Nov 4 04:38:35 2010 +0200
Cache adapter address for quick lookup
:040000 040000 b7b14eb9f69c0b74ebda87b2fa40a4574f08be44 b96739e2d58fcc5a5c6654431ba49a36ee623932 M plugins
> git bisect log
git bisect start
# good: [b079cca768419d11e011cc5a7d59bc802226e633] Release 4.77
git bisect good b079cca768419d11e011cc5a7d59bc802226e633
# bad: [018c5b71748c178dcd2ffb0164c1d975788a2acc] Release 4.78
git bisect bad 018c5b71748c178dcd2ffb0164c1d975788a2acc
# good: [d5d0fa3be8f7a20d128dcbaf8fc529c38bc52395] Optimize device disconnect callback processing
git bisect good d5d0fa3be8f7a20d128dcbaf8fc529c38bc52395
# good: [b67a8a42e6ea9c2d2b0877ac3832de75ec4e00e5] Fix bdaddr naming consistency
git bisect good b67a8a42e6ea9c2d2b0877ac3832de75ec4e00e5
# bad: [5295b6a0ab0ad67a27926273a3fbf61f02663219] Fix not ignoring case of uuid given to RegisterEndpoint
git bisect bad 5295b6a0ab0ad67a27926273a3fbf61f02663219
# good: [db5266f0afedc33e2fd6fc3601c16498865f746d] Remove redundant tracking of ignored adapters
git bisect good db5266f0afedc33e2fd6fc3601c16498865f746d
# bad: [a352058752e541539b09e55124d411a534cc14af] Cache adapter address for quick lookup
git bisect bad a352058752e541539b09e55124d411a534cc14af
# good: [b289e54d53c332a51be483c1660a42c267991dd3] Remove redundant hci_devinfo call
git bisect good b289e54d53c332a51be483c1660a42c267991dd3
^ permalink raw reply
* [PATCH 2/2] Bluetooth: Get ride of __rfcomm_get_sock_by_channel()
From: Gustavo F. Padovan @ 2010-11-17 23:22 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1290036176-4022-1-git-send-email-padovan@profusion.mobi>
rfcomm_get_sock_by_channel() was the only user of this function, so I merged
both into rfcomm_get_sock_by_channel(). The socket lock now should be hold
outside of rfcomm_get_sock_by_channel() once we hold and release it inside the
same function now.
Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
---
net/bluetooth/rfcomm/sock.c | 19 +++++++------------
1 files changed, 7 insertions(+), 12 deletions(-)
diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
index aec505f..0207bd6 100644
--- a/net/bluetooth/rfcomm/sock.c
+++ b/net/bluetooth/rfcomm/sock.c
@@ -140,11 +140,13 @@ static struct sock *__rfcomm_get_sock_by_addr(u8 channel, bdaddr_t *src)
/* Find socket with channel and source bdaddr.
* Returns closest match.
*/
-static struct sock *__rfcomm_get_sock_by_channel(int state, u8 channel, bdaddr_t *src)
+static struct sock *rfcomm_get_sock_by_channel(int state, u8 channel, bdaddr_t *src)
{
struct sock *sk = NULL, *sk1 = NULL;
struct hlist_node *node;
+ read_lock(&rfcomm_sk_list.lock);
+
sk_for_each(sk, node, &rfcomm_sk_list.head) {
if (state && sk->sk_state != state)
continue;
@@ -159,19 +161,10 @@ static struct sock *__rfcomm_get_sock_by_channel(int state, u8 channel, bdaddr_t
sk1 = sk;
}
}
- return node ? sk : sk1;
-}
-/* Find socket with given address (channel, src).
- * Returns locked socket */
-static inline struct sock *rfcomm_get_sock_by_channel(int state, u8 channel, bdaddr_t *src)
-{
- struct sock *s;
- read_lock(&rfcomm_sk_list.lock);
- s = __rfcomm_get_sock_by_channel(state, channel, src);
- if (s) bh_lock_sock(s);
read_unlock(&rfcomm_sk_list.lock);
- return s;
+
+ return node ? sk : sk1;
}
static void rfcomm_sock_destruct(struct sock *sk)
@@ -945,6 +938,8 @@ int rfcomm_connect_ind(struct rfcomm_session *s, u8 channel, struct rfcomm_dlc *
if (!parent)
return 0;
+ bh_lock_sock(parent);
+
/* Check for backlog size */
if (sk_acceptq_is_full(parent)) {
BT_DBG("backlog full %d", parent->sk_ack_backlog);
--
1.7.3.1
^ permalink raw reply related
* [PATCH 1/2] Bluetooth: Get ride of __l2cap_get_sock_by_psm()
From: Gustavo F. Padovan @ 2010-11-17 23:22 UTC (permalink / raw)
To: linux-bluetooth
l2cap_get_sock_by_psm() was the only user of this function, so I merged
both into l2cap_get_sock_by_psm(). The socket lock now should be hold
outside of l2cap_get_sock_by_psm() once we hold and release it inside the
same function now.
Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
---
net/bluetooth/l2cap.c | 22 +++++++++-------------
1 files changed, 9 insertions(+), 13 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 18a802c..12b4aa2 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -751,11 +751,13 @@ found:
/* Find socket with psm and source bdaddr.
* Returns closest match.
*/
-static struct sock *__l2cap_get_sock_by_psm(int state, __le16 psm, bdaddr_t *src)
+static struct sock *l2cap_get_sock_by_psm(int state, __le16 psm, bdaddr_t *src)
{
struct sock *sk = NULL, *sk1 = NULL;
struct hlist_node *node;
+ read_lock(&l2cap_sk_list.lock);
+
sk_for_each(sk, node, &l2cap_sk_list.head) {
if (state && sk->sk_state != state)
continue;
@@ -770,20 +772,10 @@ static struct sock *__l2cap_get_sock_by_psm(int state, __le16 psm, bdaddr_t *src
sk1 = sk;
}
}
- return node ? sk : sk1;
-}
-/* Find socket with given address (psm, src).
- * Returns locked socket */
-static inline struct sock *l2cap_get_sock_by_psm(int state, __le16 psm, bdaddr_t *src)
-{
- struct sock *s;
- read_lock(&l2cap_sk_list.lock);
- s = __l2cap_get_sock_by_psm(state, psm, src);
- if (s)
- bh_lock_sock(s);
read_unlock(&l2cap_sk_list.lock);
- return s;
+
+ return node ? sk : sk1;
}
static void l2cap_sock_destruct(struct sock *sk)
@@ -2934,6 +2926,8 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd
goto sendresp;
}
+ bh_lock_sock(parent);
+
/* Check if the ACL is secure enough (if not SDP) */
if (psm != cpu_to_le16(0x0001) &&
!hci_conn_check_link_mode(conn->hcon)) {
@@ -4464,6 +4458,8 @@ static inline int l2cap_conless_channel(struct l2cap_conn *conn, __le16 psm, str
if (!sk)
goto drop;
+ bh_lock_sock(sk);
+
BT_DBG("sk %p, len %d", sk, skb->len);
if (sk->sk_state != BT_BOUND && sk->sk_state != BT_CONNECTED)
--
1.7.3.1
^ permalink raw reply related
* Re: [PATCH 1/7] Bluetooth: Hold the lock inside l2cap_get_sock_by_addr()
From: Gustavo F. Padovan @ 2010-11-17 23:17 UTC (permalink / raw)
To: Ville Tervo; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <20101117231141.GB32261@vigoh>
* Gustavo F. Padovan <padovan@profusion.mobi> [2010-11-17 21:11:41 -0200]:
> * Gustavo F. Padovan <padovan@profusion.mobi> [2010-11-05 10:37:11 -0400]:
>
> > Hi Ville,
> >
> > * Ville Tervo <ville.tervo@nokia.com> [2010-11-05 15:49:35 +0200]:
> >
> > > Hi Gustavo,
> > >
> > > On Tue, Nov 02, 2010 at 04:03:12PM +0100, ext Gustavo F. Padovan wrote:
> > > > It also have to change the name of the function to
> > > > l2cap_get_sock_by_addr() because we do hold the lock inside it now.
> > > >
> > > > Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
> > > > ---
> > > > net/bluetooth/l2cap.c | 17 ++++++-----------
> > > > 1 files changed, 6 insertions(+), 11 deletions(-)
> > > >
> > > > diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> > > > index 6f931cc..3d48867 100644
> > > > --- a/net/bluetooth/l2cap.c
> > > > +++ b/net/bluetooth/l2cap.c
> > > > @@ -728,15 +728,18 @@ static inline void l2cap_chan_add(struct l2cap_conn *conn, struct sock *sk, stru
> > > > }
> > > >
> > > > /* ---- Socket interface ---- */
> > > > -static struct sock *__l2cap_get_sock_by_addr(__le16 psm, bdaddr_t *src)
> > > > +static struct sock *l2cap_get_sock_by_addr(__le16 psm, bdaddr_t *src)
> > > > {
> > > > struct sock *sk;
> > > > struct hlist_node *node;
> > > > +
> > > > + write_lock_bh(&l2cap_sk_list.lock);
> > >
> > > Code is only reading so read_lock_bh would be enough?
> >
> > Sure, I didn't looked to that, I just keept the same code that we were
> > using before. I'll fix it.
>
> I figured out that we need write_lock_bh() here, because set the psm and
> sport is like a new element to the list. l2cap_get_sock_by_addr()
> searches for either psm or sport.
>
> I'm also dropping the option to use RCU on the bt_sk_list(), It does not
> fit on our case. We can't have anyone writing the list while we are
> reading it.
That said, only patch 4 and 5 are still valid (I'll resend them), and 6 is
so trivial that I put it upstream already.
--
Gustavo F. Padovan
http://profusion.mobi
^ permalink raw reply
* Re: [PATCH 1/7] Bluetooth: Hold the lock inside l2cap_get_sock_by_addr()
From: Gustavo F. Padovan @ 2010-11-17 23:11 UTC (permalink / raw)
To: Ville Tervo; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <20101105143711.GB9116@vigoh>
* Gustavo F. Padovan <padovan@profusion.mobi> [2010-11-05 10:37:11 -0400]:
> Hi Ville,
>
> * Ville Tervo <ville.tervo@nokia.com> [2010-11-05 15:49:35 +0200]:
>
> > Hi Gustavo,
> >
> > On Tue, Nov 02, 2010 at 04:03:12PM +0100, ext Gustavo F. Padovan wrote:
> > > It also have to change the name of the function to
> > > l2cap_get_sock_by_addr() because we do hold the lock inside it now.
> > >
> > > Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
> > > ---
> > > net/bluetooth/l2cap.c | 17 ++++++-----------
> > > 1 files changed, 6 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> > > index 6f931cc..3d48867 100644
> > > --- a/net/bluetooth/l2cap.c
> > > +++ b/net/bluetooth/l2cap.c
> > > @@ -728,15 +728,18 @@ static inline void l2cap_chan_add(struct l2cap_conn *conn, struct sock *sk, stru
> > > }
> > >
> > > /* ---- Socket interface ---- */
> > > -static struct sock *__l2cap_get_sock_by_addr(__le16 psm, bdaddr_t *src)
> > > +static struct sock *l2cap_get_sock_by_addr(__le16 psm, bdaddr_t *src)
> > > {
> > > struct sock *sk;
> > > struct hlist_node *node;
> > > +
> > > + write_lock_bh(&l2cap_sk_list.lock);
> >
> > Code is only reading so read_lock_bh would be enough?
>
> Sure, I didn't looked to that, I just keept the same code that we were
> using before. I'll fix it.
I figured out that we need write_lock_bh() here, because set the psm and
sport is like a new element to the list. l2cap_get_sock_by_addr()
searches for either psm or sport.
I'm also dropping the option to use RCU on the bt_sk_list(), It does not
fit on our case. We can't have anyone writing the list while we are
reading it.
--
Gustavo F. Padovan
http://profusion.mobi
^ permalink raw reply
* Re: Apple Wireless Keyboard connection issue
From: Andre Kuehne @ 2010-11-17 23:04 UTC (permalink / raw)
To: Gustavo F. Padovan, linux-bluetooth
In-Reply-To: <20101117090534.GA11494@jh-x301>
Hi Johan
On 11/17/2010 10:05 AM, Johan Hedberg wrote:
> Hi Gustavo,
>
> On Tue, Nov 16, 2010, Gustavo F. Padovan wrote:
>> * "André Kühne"<andre.kuehne@gmx.net> [2010-11-16 22:27:01 +0100]:
>>> I noticed the following on my system: After upgrading to bluez-4.79
>>> connecting my Apple Wireless Keyboard does not work anymore. With
>>> bluez-4.77 the connection works just fine.
>>
>> My Microsoft keyboard is not working too. That is probably due to commit
>> abe7cd44124a from Johan. It should be fixed soon.
>
> The only real difference that patch makes is the reuse of the HCI socket
> inside hciops.c. Maybe that screws up the event filters somehow or
> something similar. I don't have a keyboard to verify a fix, but could
> you try the attached patch and see if it helps?
>
I installed
http://www.kernel.org/pub/linux/bluetooth/bluez-4.79.tar.gz + hciops_encrypt.patch
but the patch does not work for me. For verification I downloaded
http://www.kernel.org/pub/linux/bluetooth/bluez-4.77.tar.gz
and compiled/installed it the same way (without the patch) and the connection works.
I did the same using bluez-4.78 with the result that both my keyboard and mouse failed to connect.
Best regards
Andre
^ 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