* Bluetooth Dongle version mismatch Confusion
From: nirav rabara @ 2010-03-10 9:44 UTC (permalink / raw)
To: linux-bluetooth
Hi,
I have been using bluez 4.58 with 2.6.30 on ARM platform , I am newbie
and found problem with Headset & Dongle version mismatch.
I can use v2.0 Dongle with v2.0 Headset & v2.1 Dongle with v2.1
Headset.( result is good I can hear voice on HS)
But if I use v2.0 Dongle with v2.1 HS & v2.1 Dongle with v2.0 HS
result is unpredictable some time noise or No voice).
I am doing anything wrong with SCO implementation? OR Bluetooth have
limitation with version mismatch. OR I must use same version Dongle &
HS(why ?).
Thanks in Advance
--
With Regards,
Nirav Rabara
^ permalink raw reply
* AMP Key Management Simplification?
From: Tim Monahan-Mitchell @ 2010-03-10 5:06 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: BlueZ mailing list
Hi Marcel,
A discussion was missing from our original posting ("QuIC's AMP + eL2CAP
Technical Plans"), regarding AMP Keys.
We would like to defer the ability to work with more than one AMP device
type initially, since doing so would necessitate having to persist the
Dedicated AMP Keys for each AMP device type. Is this approach acceptable?
Limiting the initial design to a single AMP device, the Dedicated AMP key
can be generated any time from the associated link key.
I can provide more details as to why if needed (refer to 8.1 & 8.2 in the
Core spec).
Thanks,
Tim Monahan-Mitchell
Qualcomm Innovation Center, Inc.,
A member of the Code Aurora Forum
^ permalink raw reply
* RE: [bluetooth-next V2] Bluetooth: handle device reset event
From: Winkler, Tomas @ 2010-03-09 21:22 UTC (permalink / raw)
To: Marcel Holtmann
Cc: linux-bluetooth@vger.kernel.org, Cohen, Guy, Rindjunsky, Ron,
Paskar, Gregory
In-Reply-To: <1268096587.3712.36.camel@localhost.localdomain>
DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogTWFyY2VsIEhvbHRtYW5u
IFttYWlsdG86bWFyY2VsQGhvbHRtYW5uLm9yZ10NCj4gU2VudDogVHVlc2RheSwgTWFyY2ggMDks
IDIwMTAgMzowMyBBTQ0KPiBUbzogV2lua2xlciwgVG9tYXMNCj4gQ2M6IGxpbnV4LWJsdWV0b290
aEB2Z2VyLmtlcm5lbC5vcmc7IENvaGVuLCBHdXk7IFJpbmRqdW5za3ksIFJvbjsgUGFza2FyLA0K
PiBHcmVnb3J5DQo+IFN1YmplY3Q6IFJlOiBbYmx1ZXRvb3RoLW5leHQgVjJdIEJsdWV0b290aDog
aGFuZGxlIGRldmljZSByZXNldCBldmVudA0KPiANCj4gSGkgVG9tYXMsDQo+IA0KPiA+IEEgQmx1
ZXRvb3RoIGRldmljZSBleHBlcmllbmNpbmcgaGFyZHdhcmUgZmFpbHVyZSBtYXkgaXNzdWUNCj4g
PiBhIEhBUkRXQVJFX0VSUk9SIGhjaSBldmVudC4gVGhlIHJlYWN0aW9uIHRvIHRoaXMgZXZlbnQg
aXMgZGV2aWNlDQo+ID4gcmVzZXQgZmxvdyBpbXBsZW1lbnRlZCBpbiBmb2xsb3dpbmcgc2VxdWVu
Y2UuDQo+ID4NCj4gPiAxLiBOb3RpZnk6IEhDSV9ERVZfRE9XTg0KPiA+IDIuIFJlaW5pdGlhbGl6
ZSBpbnRlcm5hbCBzdHJ1Y3R1cmVzLg0KPiA+IDMuIENhbGwgZHJpdmVyIGZsdXNoIGZ1bmN0aW9u
DQo+ID4gNC4gU2VuZCBIQ0kgcmVzZXQgcmVxdWVzdCB0byB0aGUgZGV2aWNlLg0KPiA+IDUuIFNl
bmQgSENJIGluaXQgc2VxdWVuY2UgcmVzZXQgdG8gdGhlIGRldmljZS4NCj4gPiA2LiBOb3RpZnkg
SENJX0RFVl9VUC4NCj4gDQo+IEkgcHJlZmVyIGlmIHdlIGNyZWF0ZSBhIGdlbmVyaWMgcGVyIGNv
bnRyb2xsZXIgd29ya3F1ZXVlIGZpcnN0IGJlZm9yZQ0KPiBoYXZpbmcgYSB3b3JrcXVldWUgZm9y
IGV2ZXJ5IHRhc2suIFNvbWV0aGluZyBzaW1pbGFyIHRvIHdoYXQgdGhlDQo+IG1hYzgwMjExIGxh
eWVyIG9mZmVycyByaWdodCBub3cuDQoNClRoYXQgd291bGQgYmUgZ29vZCBhcHByb2FjaCBidXQg
d2UgYXJlIHVzaW5nIGRlZmF1bHQga2VybmVsIHdvcmtxdWV1ZSBpbiB0aGlzIHNvbHV0aW9uIHNv
IHRoZXJlIGlzIG5vIHdvcmtxdWV1ZSBmb3IgZXZlcnkgdGFzay4gDQpJJ20gbm90IHN1cmUgaWYg
dGhpcyBlZmZvcnQgc2hvdWxkIGJsb2NrIHRoaXMgcGF0Y2guIA0KDQo+IEFsc28gaW4gYSBzZWNv
bmQgc3RlcCB3ZSBtaWdodCB3YW5uYSBtb3ZlIHRoZSBIQ0kgZXZlbnQgcHJvY2Vzc2luZw0KPiBj
b21wbGV0ZWx5IGludG8gYSB3b3JrcXVldWUuIElmIHdlIGdldCBubyBwZXJmb3JtYW5jZSBoaXQg
d2l0aCB0aGF0LA0KPiB0aGVuIHN5c2ZzIGhhbmRsaW5nIGFuZCBkZXZpY2UgcmVzZXQgYmVjb21l
cyBhIGxvdCBzaW1wbGVyIGFuZCBsZXNzDQo+IHByb25lIHRvIHJhY2UgY29uZGl0aW9ucyB3aXRo
IGRldmljZSByZW1vdmFsLg0KDQpMb29rcyBnb29kIHRvIG1lLiBTbyB3aGVuIHRoaXMgaXMgcmVh
ZHkgd2UgY2FuIG1vdmUgYWxzbyB0aGUgcmVzZXQgYWxzbyB0byBwZXIgY29udHJvbGxlciBxdWV1
ZS4NCg0KVGhhbmtzDQpUb21hcyANCg0KLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t
LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tCkludGVsIElzcmFlbCAoNzQpIExp
bWl0ZWQKClRoaXMgZS1tYWlsIGFuZCBhbnkgYXR0YWNobWVudHMgbWF5IGNvbnRhaW4gY29uZmlk
ZW50aWFsIG1hdGVyaWFsIGZvcgp0aGUgc29sZSB1c2Ugb2YgdGhlIGludGVuZGVkIHJlY2lwaWVu
dChzKS4gQW55IHJldmlldyBvciBkaXN0cmlidXRpb24KYnkgb3RoZXJzIGlzIHN0cmljdGx5IHBy
b2hpYml0ZWQuIElmIHlvdSBhcmUgbm90IHRoZSBpbnRlbmRlZApyZWNpcGllbnQsIHBsZWFzZSBj
b250YWN0IHRoZSBzZW5kZXIgYW5kIGRlbGV0ZSBhbGwgY29waWVzLgo=
^ permalink raw reply
* Re: [PATCH] Expose wacom pen tablet battery and ac thru power_supply class
From: Jiri Kosina @ 2010-03-09 21:22 UTC (permalink / raw)
To: Przemo Firszt
Cc: Bastien Nocera, linux-bluetooth, marcel, Peter Hutterer, Ping,
Peter Huewe
In-Reply-To: <1268162757.3632.33.camel@pldmachine>
On Tue, 9 Mar 2010, Przemo Firszt wrote:
> Please ignore previous patch
Hi Prezemo,
thanks for basing the patch on power_supply infrastructure.
Anyway, you'll have to sort out the new dependency on CONFIG_POWER_SUPPLY
somehow (compiling the battery code out from the driver if
CONFIG_POWER_SUPPLY is unset, or selecting it directly from Kconfig).
Thanks,
--
Jiri Kosina
SUSE Labs, Novell Inc.
^ permalink raw reply
* RE: [bluetooth-next V2] bluetooth: hci_sysfs: use strict_strtoul instead of simple_strtoul
From: Winkler, Tomas @ 2010-03-09 21:13 UTC (permalink / raw)
To: Marcel Holtmann
Cc: linux-bluetooth@vger.kernel.org, Cohen, Guy, Rindjunsky, Ron
In-Reply-To: <1268167314.3712.59.camel@localhost.localdomain>
DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogTWFyY2VsIEhvbHRtYW5u
IFttYWlsdG86bWFyY2VsQGhvbHRtYW5uLm9yZ10NCj4gU2VudDogVHVlc2RheSwgTWFyY2ggMDks
IDIwMTAgMTA6NDIgUE0NCj4gVG86IFdpbmtsZXIsIFRvbWFzDQo+IENjOiBsaW51eC1ibHVldG9v
dGhAdmdlci5rZXJuZWwub3JnOyBDb2hlbiwgR3V5OyBSaW5kanVuc2t5LCBSb24NCj4gU3ViamVj
dDogUmU6IFtibHVldG9vdGgtbmV4dCBWMl0gYmx1ZXRvb3RoOiBoY2lfc3lzZnM6IHVzZSBzdHJp
Y3Rfc3RydG91bA0KPiBpbnN0ZWFkIG9mIHNpbXBsZV9zdHJ0b3VsDQo+IA0KPiBIaSBUb21hcywN
Cj4gDQo+ID4gdXNlIHN0cmljdF9zdHJ0b3VsIGFzIHN1Z2dlc3RlZCBieSBjaGVja3BhdGNoLnBs
DQo+ID4NCj4gPiBTaWduZWQtb2ZmLWJ5OiBUb21hcyBXaW5rbGVyIDx0b21hcy53aW5rbGVyQGlu
dGVsLmNvbT4NCj4gPiAtLS0NCj4gPiBWMjoNCj4gPiAxLiBtb3JlIHZlcmJvc2UgY29tbWl0IG1l
c3NhZ2UNCj4gPiAyLiByZXR1cm4gdGhlIGVycm9yIGNvZGUgdGhhdCB3YXMgcHJvZHVjZWQgYnkg
c3RyaWN0X3N0cnRvdWwNCj4gDQo+IHdoeSBkbyB5b3UgYm90aGVyIGFjdHVhbGx5LiBSZWFkaW5n
IHRoZSBjb21tZW50IGFib3V0IHN0cnVjdF9zdHJ0b3VsIGl0DQo+IHdpbGwgb25seSByZXR1cm4g
LUVJTlZBTCBvciAwLiBTbyB1c2luZyBteSBwcm9wb3NhbCB3b3VsZCBiZSBqdXN0IGZpbmUuDQo+
IEkgYWxzbyBkb24ndCBwcmVmZXIgdG8gZGlmZmVyIHRoZSByZXR1cm4gdmFsdWUgdG8gdXNlciBz
cGFjZSB1bnRpbCBpdA0KPiBhY3R1YWxseSBtYWtlcyBzZW5zZS4gSW52YWxpZCBhcmd1bWVudCBp
cyBqdXN0IGZpbmUgZm9yIGFsbCBlcnJvciBjYXNlcy4NCj4NClllcyBJIGtub3cgYnV0IEkndmUg
dXNlIG9mIHRoaXMgb3ZlciB0aGUga2VybmVsIGNvZGUgYW5kIHdoYXQgSSd2ZSB1c2VkIGlzIHRo
ZSBtb3N0bHkgdXNlZCBpZGlvbQ0KDQpUaGFua3MNClRvbWFzDQoNCg0KDQotLS0tLS0tLS0tLS0t
LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0K
SW50ZWwgSXNyYWVsICg3NCkgTGltaXRlZAoKVGhpcyBlLW1haWwgYW5kIGFueSBhdHRhY2htZW50
cyBtYXkgY29udGFpbiBjb25maWRlbnRpYWwgbWF0ZXJpYWwgZm9yCnRoZSBzb2xlIHVzZSBvZiB0
aGUgaW50ZW5kZWQgcmVjaXBpZW50KHMpLiBBbnkgcmV2aWV3IG9yIGRpc3RyaWJ1dGlvbgpieSBv
dGhlcnMgaXMgc3RyaWN0bHkgcHJvaGliaXRlZC4gSWYgeW91IGFyZSBub3QgdGhlIGludGVuZGVk
CnJlY2lwaWVudCwgcGxlYXNlIGNvbnRhY3QgdGhlIHNlbmRlciBhbmQgZGVsZXRlIGFsbCBjb3Bp
ZXMuCg==
^ permalink raw reply
* Re: [PATCH] Fix compilation when --enable-test is passed
From: Pacho Ramos @ 2010-03-09 20:53 UTC (permalink / raw)
To: Vinicius Costa Gomes; +Cc: linux-bluetooth
In-Reply-To: <1268089292-18303-1-git-send-email-vinicius.gomes@openbossa.org>
[-- Attachment #1: Type: text/plain, Size: 1251 bytes --]
El lun, 08-03-2010 a las 20:01 -0300, Vinicius Costa Gomes escribió:
> When --enable-test is passed along with --disable-alsa and
> --disable-gstreamer, the SBC lib is not built, which breaks
> compilation of the ipctest test program.
> ---
> acinclude.m4 | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/acinclude.m4 b/acinclude.m4
> index 2e4444d..0eaf236 100644
> --- a/acinclude.m4
> +++ b/acinclude.m4
> @@ -316,7 +316,8 @@ AC_DEFUN([AC_ARG_BLUEZ], [
>
> AM_CONDITIONAL(SNDFILE, test "${sndfile_enable}" = "yes" && test "${sndfile_found}" = "yes")
> AM_CONDITIONAL(USB, test "${usb_enable}" = "yes" && test "${usb_found}" = "yes")
> - AM_CONDITIONAL(SBC, test "${alsa_enable}" = "yes" || test "${gstreamer_enable}" = "yes")
> + AM_CONDITIONAL(SBC, test "${alsa_enable}" = "yes" || test "${gstreamer_enable}" = "yes" ||
> + test "${test_enable}" = "yes")
> AM_CONDITIONAL(ALSA, test "${alsa_enable}" = "yes" && test "${alsa_found}" = "yes")
> AM_CONDITIONAL(GSTREAMER, test "${gstreamer_enable}" = "yes" && test "${gstreamer_found}" = "yes")
> AM_CONDITIONAL(AUDIOPLUGIN, test "${audio_enable}" = "yes")
Thanks a lot for the patch, I will try it as soon as I am able to :-)
[-- Attachment #2: Esta parte del mensaje está firmada digitalmente --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* Re: RFC: Allow Bluez to select flushable or non-flushable ACL packets with L2CAP_LM_RELIABLE
From: Marcel Holtmann @ 2010-03-09 20:45 UTC (permalink / raw)
To: Nick Pelly; +Cc: linux-bluetooth
In-Reply-To: <35c90d961003091207u66571bt789461dcc7972693@mail.gmail.com>
Hi Nick,
> >>> >> Right now Bluez always requests flushable ACL packets (but does not
> >>> >> set a flush timeout, so effectively they are non-flushable):
> >>> >>
> >>> >> However it is desirable to use an ACL flush timeout on A2DP packets so
> >>> >> that if the ACL packets block for some reason then the LM can flush
> >>> >> them to make room for newer packets.
> >>> >>
> >>> >> Is it reasonable for Bluez to use the 0x00 ACL packet boundary flag by
> >>> >> default (non-flushable packet), and let userspace request flushable
> >>> >> packets on A2DP L2CAP sockets with the socket option
> >>> >> L2CAP_LM_RELIABLE.
> >>> >
> >>> > the reliable option has a different meaning. It comes back from the old
> >>> > Bluetooth 1.1 qualification days where we had to tests on L2CAP that had
> >>> > to confirm that we can detect malformed packets and report them. These
> >>> > days it is just fine to drop them.
> >>>
> >>> Got it, how about introducing
> >>>
> >>> #define L2CAP_LM_FLUSHABLE 0x0040
> >>
> >> that l2cap_sock_setsockopt_old() sets this didn't give you a hint that
> >> we might wanna deprecate this socket options ;)
> >>
> >> I need to read up on the flushable stuff, but in the end it deserves its
> >> own socket option. Also an ioctl() to actually trigger Enhanced flush
> >> might be needed.
> >>
> >>> struct l2cap_pinfo {
> >>> ...
> >>> __u8 flushable;
> >>> }
> >>
> >> Sure. In the long run we need to turn this into a bitmask. We are just
> >> wasting memory here.
> >
> > Attached is an updated patch, that checks the LMP features bitmask
> > before using the new non-flushable packet type.
> >
> > I am still using L2CAP_LM_FLUSHABLE socket option in
> > l2cap_sock_setsockopt_old(), which I don't think you are happy with.
> > So how about a new option:
> >
> > SOL_L2CAP, L2CAP_ACL_FLUSH
> > which has a default value of 0, and can be set to 1 to make the ACL
> > data sent by this L2CAP socket flushable.
> >
> > In a later commit we would then add
> > SOL_ACL, ACL_FLUSH_TIMEOUT
> > That is used to set an automatic flush timeout for the ACL link on a
> > L2CAP socket. Note that SOL_ACL is new.
> >
> > But maybe this is not what you had in mind, so I'm looking for your
> > advice before I implement this.
>
> Attached an updated patch for 2.6.32 kernel. We've been using this
> patch successfully on production devices.
can see anything wrong with that patch. However we need to use
SOL_BLUETOOTH for it of course. So we need to come up with something to
make this simple.
An additional change I like to see is to use flags for booleans like
flushable in the structures. Can you work on changing that.
Also do we have decoding support for this in hcidump. It might be nice
to include some really simple examples in the commit message.
Regards
Marcel
^ permalink raw reply
* Re: [bluetooth-next V2] bluetooth: hci_sysfs: use strict_strtoul instead of simple_strtoul
From: Marcel Holtmann @ 2010-03-09 20:41 UTC (permalink / raw)
To: Tomas Winkler; +Cc: linux-bluetooth, guy.cohen, ron.rindjunsky
In-Reply-To: <1268163483-26181-1-git-send-email-tomas.winkler@intel.com>
Hi Tomas,
> use strict_strtoul as suggested by checkpatch.pl
>
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> ---
> V2:
> 1. more verbose commit message
> 2. return the error code that was produced by strict_strtoul
why do you bother actually. Reading the comment about struct_strtoul it
will only return -EINVAL or 0. So using my proposal would be just fine.
I also don't prefer to differ the return value to user space until it
actually makes sense. Invalid argument is just fine for all error cases.
Regards
Marcel
^ permalink raw reply
* Re: RFC: Allow Bluez to select flushable or non-flushable ACL packets with L2CAP_LM_RELIABLE
From: Nick Pelly @ 2010-03-09 20:07 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <35c90d960912161359u2b3f9b2fi875288896a7a8478@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2496 bytes --]
On Wed, Dec 16, 2009 at 1:59 PM, Nick Pelly <npelly@google.com> wrote:
> Hi Marcel,
>
> On Thu, Dec 10, 2009 at 2:03 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
>> Hi Nick,
>>
>>> >> Right now Bluez always requests flushable ACL packets (but does not
>>> >> set a flush timeout, so effectively they are non-flushable):
>>> >>
>>> >> However it is desirable to use an ACL flush timeout on A2DP packets so
>>> >> that if the ACL packets block for some reason then the LM can flush
>>> >> them to make room for newer packets.
>>> >>
>>> >> Is it reasonable for Bluez to use the 0x00 ACL packet boundary flag by
>>> >> default (non-flushable packet), and let userspace request flushable
>>> >> packets on A2DP L2CAP sockets with the socket option
>>> >> L2CAP_LM_RELIABLE.
>>> >
>>> > the reliable option has a different meaning. It comes back from the old
>>> > Bluetooth 1.1 qualification days where we had to tests on L2CAP that had
>>> > to confirm that we can detect malformed packets and report them. These
>>> > days it is just fine to drop them.
>>>
>>> Got it, how about introducing
>>>
>>> #define L2CAP_LM_FLUSHABLE 0x0040
>>
>> that l2cap_sock_setsockopt_old() sets this didn't give you a hint that
>> we might wanna deprecate this socket options ;)
>>
>> I need to read up on the flushable stuff, but in the end it deserves its
>> own socket option. Also an ioctl() to actually trigger Enhanced flush
>> might be needed.
>>
>>> struct l2cap_pinfo {
>>> ...
>>> __u8 flushable;
>>> }
>>
>> Sure. In the long run we need to turn this into a bitmask. We are just
>> wasting memory here.
>
> Attached is an updated patch, that checks the LMP features bitmask
> before using the new non-flushable packet type.
>
> I am still using L2CAP_LM_FLUSHABLE socket option in
> l2cap_sock_setsockopt_old(), which I don't think you are happy with.
> So how about a new option:
>
> SOL_L2CAP, L2CAP_ACL_FLUSH
> which has a default value of 0, and can be set to 1 to make the ACL
> data sent by this L2CAP socket flushable.
>
> In a later commit we would then add
> SOL_ACL, ACL_FLUSH_TIMEOUT
> That is used to set an automatic flush timeout for the ACL link on a
> L2CAP socket. Note that SOL_ACL is new.
>
> But maybe this is not what you had in mind, so I'm looking for your
> advice before I implement this.
Attached an updated patch for 2.6.32 kernel. We've been using this
patch successfully on production devices.
Nick
[-- Attachment #2: 0001-Bluetooth-Use-non-flushable-pb-flag-by-default-for-A.patch --]
[-- Type: application/octet-stream, Size: 6834 bytes --]
From 80ea1ef6d82e5bcbde85c23c8585ccac820f5515 Mon Sep 17 00:00:00 2001
From: Nick Pelly <npelly@google.com>
Date: Tue, 8 Dec 2009 19:42:21 -0800
Subject: [PATCH] Bluetooth: Use non-flushable pb flag by default for ACL data on capable chipsets.
With Bluetooth 2.1 ACL packets can be flushable or non-flushable. This commit
makes ACL data packets non-flushable by default on compatible chipsets, and
adds the L2CAP_LM_FLUSHABLE socket option to explicitly request flushable ACL
data packets for a given L2CAP socket. This is useful for A2DP data which can
be safely discarded if it can not be delivered within a short time (while
other ACL data should not be discarded).
Note that making ACL data flushable has no effect unless the automatic flush
timeout for that ACL link is changed from its default of 0 (infinite).
Signed-off-by: Nick Pelly <npelly@google.com>
---
include/net/bluetooth/hci.h | 4 ++++
include/net/bluetooth/hci_core.h | 1 +
include/net/bluetooth/l2cap.h | 2 ++
net/bluetooth/hci_core.c | 6 ++++--
net/bluetooth/l2cap.c | 25 ++++++++++++++++++++++---
5 files changed, 33 insertions(+), 5 deletions(-)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 9716e5a..bfd23ae 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -145,11 +145,14 @@ enum {
EDR_ESCO_MASK)
/* ACL flags */
+#define ACL_START_NO_FLUSH 0x00
#define ACL_CONT 0x01
#define ACL_START 0x02
#define ACL_ACTIVE_BCAST 0x04
#define ACL_PICO_BCAST 0x08
+#define ACL_PB_MASK (ACL_CONT | ACL_START)
+
/* Baseband links */
#define SCO_LINK 0x00
#define ACL_LINK 0x01
@@ -188,6 +191,7 @@ enum {
#define LMP_EDR_ESCO_3M 0x40
#define LMP_EDR_3S_ESCO 0x80
+#define LMP_NO_FLUSH 0x01
#define LMP_SIMPLE_PAIR 0x08
/* Connection modes */
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index cbcc5b1..f5da4e6 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -479,6 +479,7 @@ void hci_conn_del_sysfs(struct hci_conn *conn);
#define lmp_sniffsubr_capable(dev) ((dev)->features[5] & LMP_SNIFF_SUBR)
#define lmp_esco_capable(dev) ((dev)->features[3] & LMP_ESCO)
#define lmp_ssp_capable(dev) ((dev)->features[6] & LMP_SIMPLE_PAIR)
+#define lmp_no_flush_capable(dev) ((dev)->features[6] & LMP_NO_FLUSH)
/* ----- HCI protocols ----- */
struct hci_proto {
diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 9516f4b..23f37b4 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -70,6 +70,7 @@ struct l2cap_conninfo {
#define L2CAP_LM_TRUSTED 0x0008
#define L2CAP_LM_RELIABLE 0x0010
#define L2CAP_LM_SECURE 0x0020
+#define L2CAP_LM_FLUSHABLE 0x0040
/* L2CAP command codes */
#define L2CAP_COMMAND_REJ 0x01
@@ -316,6 +317,7 @@ struct l2cap_pinfo {
__u8 sec_level;
__u8 role_switch;
__u8 force_reliable;
+ __u8 flushable;
__u8 conf_req[64];
__u8 conf_len;
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index e1da8f6..84a9d75 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1239,7 +1239,7 @@ int hci_send_acl(struct hci_conn *conn, struct sk_buff *skb, __u16 flags)
skb->dev = (void *) hdev;
bt_cb(skb)->pkt_type = HCI_ACLDATA_PKT;
- hci_add_acl_hdr(skb, conn->handle, flags | ACL_START);
+ hci_add_acl_hdr(skb, conn->handle, flags);
if (!(list = skb_shinfo(skb)->frag_list)) {
/* Non fragmented */
@@ -1256,12 +1256,14 @@ int hci_send_acl(struct hci_conn *conn, struct sk_buff *skb, __u16 flags)
spin_lock_bh(&conn->data_q.lock);
__skb_queue_tail(&conn->data_q, skb);
+ flags &= ~ACL_PB_MASK;
+ flags |= ACL_CONT;
do {
skb = list; list = list->next;
skb->dev = (void *) hdev;
bt_cb(skb)->pkt_type = HCI_ACLDATA_PKT;
- hci_add_acl_hdr(skb, conn->handle, flags | ACL_CONT);
+ hci_add_acl_hdr(skb, conn->handle, flags);
BT_DBG("%s frag %p len %d", hdev->name, skb, skb->len);
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 6e3b596..4529e99 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -325,13 +325,19 @@ static inline u8 l2cap_get_ident(struct l2cap_conn *conn)
static inline int l2cap_send_cmd(struct l2cap_conn *conn, u8 ident, u8 code, u16 len, void *data)
{
struct sk_buff *skb = l2cap_build_cmd(conn, code, ident, len, data);
+ u8 flags;
BT_DBG("code 0x%2.2x", code);
if (!skb)
return -ENOMEM;
- return hci_send_acl(conn->hcon, skb, 0);
+ if (lmp_no_flush_capable(conn->hcon->hdev))
+ flags = ACL_START_NO_FLUSH;
+ else
+ flags = ACL_START;
+
+ return hci_send_acl(conn->hcon, skb, flags);
}
static inline int l2cap_send_sframe(struct l2cap_pinfo *pi, u16 control)
@@ -770,6 +776,7 @@ static void l2cap_sock_init(struct sock *sk, struct sock *parent)
pi->sec_level = l2cap_pi(parent)->sec_level;
pi->role_switch = l2cap_pi(parent)->role_switch;
pi->force_reliable = l2cap_pi(parent)->force_reliable;
+ pi->flushable = l2cap_pi(parent)->flushable;
} else {
pi->imtu = L2CAP_DEFAULT_MTU;
pi->omtu = 0;
@@ -778,6 +785,7 @@ static void l2cap_sock_init(struct sock *sk, struct sock *parent)
pi->sec_level = BT_SECURITY_LOW;
pi->role_switch = 0;
pi->force_reliable = 0;
+ pi->flushable = 0;
}
/* Default config options */
@@ -1258,11 +1266,18 @@ static void l2cap_drop_acked_frames(struct sock *sk)
static inline int l2cap_do_send(struct sock *sk, struct sk_buff *skb)
{
struct l2cap_pinfo *pi = l2cap_pi(sk);
+ struct hci_conn *hcon = pi->conn->hcon;
int err;
+ u16 flags;
BT_DBG("sk %p, skb %p len %d", sk, skb, skb->len);
- err = hci_send_acl(pi->conn->hcon, skb, 0);
+ if (lmp_no_flush_capable(hcon->hdev) && !l2cap_pi(sk)->flushable)
+ flags = ACL_START_NO_FLUSH;
+ else
+ flags = ACL_START;
+
+ err = hci_send_acl(hcon, skb, flags);
if (err < 0)
kfree_skb(skb);
@@ -1747,6 +1762,7 @@ static int l2cap_sock_setsockopt_old(struct socket *sock, int optname, char __us
l2cap_pi(sk)->role_switch = (opt & L2CAP_LM_MASTER);
l2cap_pi(sk)->force_reliable = (opt & L2CAP_LM_RELIABLE);
+ l2cap_pi(sk)->flushable = (opt & L2CAP_LM_FLUSHABLE);
break;
default:
@@ -1874,6 +1890,9 @@ static int l2cap_sock_getsockopt_old(struct socket *sock, int optname, char __us
if (l2cap_pi(sk)->force_reliable)
opt |= L2CAP_LM_RELIABLE;
+ if (l2cap_pi(sk)->flushable)
+ opt |= L2CAP_LM_FLUSHABLE;
+
if (put_user(opt, (u32 __user *) optval))
err = -EFAULT;
break;
@@ -3801,7 +3820,7 @@ static int l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 fl
BT_DBG("conn %p len %d flags 0x%x", conn, skb->len, flags);
- if (flags & ACL_START) {
+ if (!(flags & ACL_CONT)) {
struct l2cap_hdr *hdr;
int len;
--
1.6.5.3
^ permalink raw reply related
* [bluetooth-next V2] bluetooth: hci_sysfs: use strict_strtoul instead of simple_strtoul
From: Tomas Winkler @ 2010-03-09 19:38 UTC (permalink / raw)
To: marcel, linux-bluetooth; +Cc: guy.cohen, ron.rindjunsky, Tomas Winkler
use strict_strtoul as suggested by checkpatch.pl
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
V2:
1. more verbose commit message
2. return the error code that was produced by strict_strtoul
net/bluetooth/hci_sysfs.c | 30 +++++++++++++++---------------
1 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
index 1a79a6c..dd11d61 100644
--- a/net/bluetooth/hci_sysfs.c
+++ b/net/bluetooth/hci_sysfs.c
@@ -282,12 +282,12 @@ static ssize_t show_idle_timeout(struct device *dev, struct device_attribute *at
static ssize_t store_idle_timeout(struct device *dev, struct device_attribute *attr, const char *buf, size_t count)
{
struct hci_dev *hdev = dev_get_drvdata(dev);
- char *ptr;
- __u32 val;
+ unsigned long val;
+ int ret;
- val = simple_strtoul(buf, &ptr, 10);
- if (ptr == buf)
- return -EINVAL;
+ ret = strict_strtoul(buf, 0, &val);
+ if (ret)
+ return ret;
if (val != 0 && (val < 500 || val > 3600000))
return -EINVAL;
@@ -306,12 +306,12 @@ static ssize_t show_sniff_max_interval(struct device *dev, struct device_attribu
static ssize_t store_sniff_max_interval(struct device *dev, struct device_attribute *attr, const char *buf, size_t count)
{
struct hci_dev *hdev = dev_get_drvdata(dev);
- char *ptr;
- __u16 val;
+ unsigned long val;
+ int ret;
- val = simple_strtoul(buf, &ptr, 10);
- if (ptr == buf)
- return -EINVAL;
+ ret = strict_strtoul(buf, 0, &val);
+ if (ret)
+ return ret;
if (val < 0x0002 || val > 0xFFFE || val % 2)
return -EINVAL;
@@ -333,12 +333,12 @@ static ssize_t show_sniff_min_interval(struct device *dev, struct device_attribu
static ssize_t store_sniff_min_interval(struct device *dev, struct device_attribute *attr, const char *buf, size_t count)
{
struct hci_dev *hdev = dev_get_drvdata(dev);
- char *ptr;
- __u16 val;
+ unsigned long val;
+ int ret;
- val = simple_strtoul(buf, &ptr, 10);
- if (ptr == buf)
- return -EINVAL;
+ ret = strict_strtoul(buf, 0, &val);
+ if (ret)
+ return ret;
if (val < 0x0002 || val > 0xFFFE || val % 2)
return -EINVAL;
--
1.6.6.1
---------------------------------------------------------------------
Intel Israel (74) Limited
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
^ permalink raw reply related
* Re: [PATCH] Expose wacom pen tablet battery and ac thru power_supply class
From: Przemo Firszt @ 2010-03-09 19:25 UTC (permalink / raw)
To: Bastien Nocera
Cc: linux-bluetooth, marcel, Jiri Kosina, Peter Hutterer, Ping,
Peter Huewe
In-Reply-To: <1268161944.3632.21.camel@pldmachine>
[-- Attachment #1: Type: text/plain, Size: 39 bytes --]
Please ignore previous patch
--
Przemo
[-- Attachment #2: 0001-Expose-wacom-pen-tablet-battery-and-ac-thru-power_su.patch --]
[-- Type: text/x-patch, Size: 4655 bytes --]
>From 74b4e83f11b74d800850b659c81d64fe32fd8ca8 Mon Sep 17 00:00:00 2001
From: Przemo Firszt <przemo@firszt.eu>
Date: Fri, 5 Mar 2010 17:19:44 +0000
Subject: [PATCH] Expose wacom pen tablet battery and ac thru power_supply class
This patch exposes wacom pen tablet battery capacity and ac state thru
power_supply class is sysfs.
Signed-off-by: Przemo Firszt <przemo@firszt.eu>
---
drivers/hid/hid-wacom.c | 114 +++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 114 insertions(+), 0 deletions(-)
diff --git a/drivers/hid/hid-wacom.c b/drivers/hid/hid-wacom.c
index 8d3b46f..9fc2898 100644
--- a/drivers/hid/hid-wacom.c
+++ b/drivers/hid/hid-wacom.c
@@ -21,14 +21,82 @@
#include <linux/device.h>
#include <linux/hid.h>
#include <linux/module.h>
+#include <linux/power_supply.h>
#include "hid-ids.h"
struct wacom_data {
__u16 tool;
unsigned char butstate;
+ int battery_capacity;
+ struct power_supply battery;
+ struct power_supply ac;
};
+/*percent of battery capacity, 0 means AC online*/
+static unsigned short batcap[8] = { 1, 15, 25, 35, 50, 70, 100, 0 };
+
+static enum power_supply_property wacom_battery_props[] = {
+ POWER_SUPPLY_PROP_PRESENT,
+ POWER_SUPPLY_PROP_CAPACITY
+};
+
+static enum power_supply_property wacom_ac_props[] = {
+ POWER_SUPPLY_PROP_PRESENT,
+ POWER_SUPPLY_PROP_ONLINE
+};
+
+static int wacom_battery_get_property(struct power_supply *psy,
+ enum power_supply_property psp,
+ union power_supply_propval *val)
+{
+ struct wacom_data *wdata = container_of(psy,
+ struct wacom_data, battery);
+ int power_state = batcap[wdata->battery_capacity];
+ int ret = 0;
+
+ switch (psp) {
+ case POWER_SUPPLY_PROP_PRESENT:
+ val->intval = 1;
+ break;
+ case POWER_SUPPLY_PROP_CAPACITY:
+ /* show 100% battery capacity when charging */
+ if (power_state == 0)
+ val->intval = 100;
+ else
+ val->intval = power_state;
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
+ return ret;
+}
+
+static int wacom_ac_get_property(struct power_supply *psy,
+ enum power_supply_property psp,
+ union power_supply_propval *val)
+{
+ struct wacom_data *wdata = container_of(psy, struct wacom_data, ac);
+ int power_state = batcap[wdata->battery_capacity];
+ int ret = 0;
+
+ switch (psp) {
+ case POWER_SUPPLY_PROP_PRESENT:
+ /* fall through */
+ case POWER_SUPPLY_PROP_ONLINE:
+ if (power_state == 0)
+ val->intval = 1;
+ else
+ val->intval = 0;
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
+ return ret;
+}
+
static int wacom_raw_event(struct hid_device *hdev, struct hid_report *report,
u8 *raw_data, int size)
{
@@ -147,6 +215,11 @@ static int wacom_raw_event(struct hid_device *hdev, struct hid_report *report,
input_sync(input);
}
+ /* Store current battery capacity */
+ rw = (data[7] >> 2 & 0x07);
+ if (rw != wdata->battery_capacity)
+ wdata->battery_capacity = rw;
+
return 1;
}
@@ -206,6 +279,43 @@ static int wacom_probe(struct hid_device *hdev,
if (ret < 0)
dev_warn(&hdev->dev, "failed to poke device #2, %d\n", ret);
+ wdata->battery.properties = wacom_battery_props;
+ wdata->battery.num_properties = ARRAY_SIZE(wacom_battery_props);
+ wdata->battery.get_property = wacom_battery_get_property;
+ wdata->battery.name = "wacom_battery";
+ wdata->battery.type = POWER_SUPPLY_TYPE_BATTERY;
+ wdata->battery.use_for_apm = 0;
+
+ ret = power_supply_register(&hdev->dev, &wdata->battery);
+ if (ret) {
+ dev_warn(&hdev->dev,
+ "can't create sysfs battery attribute, err: %d\n", ret);
+ /*
+ * battery attribute is not critical for the tablet, but if it
+ * failed then there is no need to create ac attribute
+ */
+ goto move_on;
+ }
+
+ wdata->ac.properties = wacom_ac_props;
+ wdata->ac.num_properties = ARRAY_SIZE(wacom_ac_props);
+ wdata->ac.get_property = wacom_ac_get_property;
+ wdata->ac.name = "wacom_ac";
+ wdata->ac.type = POWER_SUPPLY_TYPE_MAINS;
+ wdata->ac.use_for_apm = 0;
+
+ ret = power_supply_register(&hdev->dev, &wdata->ac);
+ if (ret) {
+ dev_warn(&hdev->dev,
+ "can't create ac battery attribute, err: %d\n", ret);
+ /*
+ * ac attribute is not critical for the tablet, but if it
+ * failed then we don't want to battery attribute to exist
+ */
+ power_supply_unregister(&wdata->battery);
+ }
+
+move_on:
hidinput = list_entry(hdev->inputs.next, struct hid_input, list);
input = hidinput->input;
@@ -250,7 +360,11 @@ err_free:
static void wacom_remove(struct hid_device *hdev)
{
+ struct wacom_data *wdata = hid_get_drvdata(hdev);
+
hid_hw_stop(hdev);
+ power_supply_unregister(&wdata->battery);
+ power_supply_unregister(&wdata->ac);
kfree(hid_get_drvdata(hdev));
}
--
1.7.0.1
^ permalink raw reply related
* [PATCH] Expose wacom pen tablet battery and ac thru power_supply class
From: Przemo Firszt @ 2010-03-09 19:12 UTC (permalink / raw)
To: Bastien Nocera
Cc: linux-bluetooth, marcel, Jiri Kosina, Peter Hutterer, Ping,
Peter Huewe
In-Reply-To: <1267531889.23521.14301.camel@localhost.localdomain>
[-- Attachment #1: Type: text/plain, Size: 941 bytes --]
Dnia 2010-03-02, wto o godzinie 12:11 +0000, Bastien Nocera pisze:
[..]
> > A couple of comments:
> <snip>
> > - isn't there a more kernel-y way to export that data, so that it's
> > automatically picked up by things like upower (né DeviceKit-power)?
>
> I've been told it should use the power_supply class, so it would work
> pretty much out-of-the-box with things like upower and
> gnome-power-manager.
Thanks for the comments - it was exactly that what I was looking
for! :-)
Please find attached revamped patch - this time battery & ac patch only.
Speed switching isn't yet ready for release.
For some reason gnome-power-manager isn't updating the values properly,
despite of that they are OK in:
/sys/class/power_supply/wacom_{ac,battery}/uevent
I'm not sure if it's a result of lack of .external_power_changed (it's
missing in some drivers using power_supply) or something else. Any
hints?
Cheers,
Przemo
[-- Attachment #2: 0001-Expose-wacom-pen-tablet-battery-and-ac-thru-power_su.patch --]
[-- Type: text/x-patch, Size: 5490 bytes --]
>From c56101ee1fe781a46e43de930b94ee3397b08b69 Mon Sep 17 00:00:00 2001
From: Przemo Firszt <przemo@firszt.eu>
Date: Fri, 5 Mar 2010 17:19:44 +0000
Subject: [PATCH] Expose wacom pen tablet battery and ac thru power_supply class
This patch exposes wacom pen tablet battery capacity and ac state thru
power_supply class is sysfs.
Signed-off-by: Przemo Firszt <przemo@firszt.eu>
---
drivers/hid/hid-wacom.c | 150 +++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 150 insertions(+), 0 deletions(-)
diff --git a/drivers/hid/hid-wacom.c b/drivers/hid/hid-wacom.c
index 8d3b46f..e5fa473 100644
--- a/drivers/hid/hid-wacom.c
+++ b/drivers/hid/hid-wacom.c
@@ -21,14 +21,118 @@
#include <linux/device.h>
#include <linux/hid.h>
#include <linux/module.h>
+#include <linux/power_supply.h>
#include "hid-ids.h"
struct wacom_data {
__u16 tool;
unsigned char butstate;
+ int battery_capacity;
+ struct power_supply battery;
+ struct power_supply ac;
};
+/*percent of battery capacity, 0 means AC online*/
+static unsigned short batcap[8] = { 1, 15, 25, 35, 50, 70, 100, 0 };
+
+static enum power_supply_property wacom_battery_props[] = {
+ POWER_SUPPLY_PROP_PRESENT,
+ POWER_SUPPLY_PROP_CAPACITY
+};
+
+static enum power_supply_property wacom_ac_props[] = {
+ POWER_SUPPLY_PROP_PRESENT,
+ POWER_SUPPLY_PROP_ONLINE
+};
+
+static int wacom_battery_get_property(struct power_supply *psy,
+ enum power_supply_property psp,
+ union power_supply_propval *val)
+{
+ struct wacom_data *wdata = container_of(psy,
+ struct wacom_data, battery);
+ int power_state = batcap[wdata->battery_capacity];
+ int ret = 0;
+
+ switch (psp) {
+ case POWER_SUPPLY_PROP_PRESENT:
+ val->intval = 1;
+ break;
+ case POWER_SUPPLY_PROP_CAPACITY:
+ /* show 100% battery capacity when charging */
+ if (power_state == 0)
+ val->intval = 100;
+ else
+ val->intval = power_state;
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
+ return ret;
+}
+
+static int wacom_ac_get_property(struct power_supply *psy,
+ enum power_supply_property psp,
+ union power_supply_propval *val)
+{
+ struct wacom_data *wdata = container_of(psy, struct wacom_data, ac);
+ int power_state = batcap[wdata->battery_capacity];
+ int ret = 0;
+
+ switch (psp) {
+ case POWER_SUPPLY_PROP_PRESENT:
+ /* fall through */
+ case POWER_SUPPLY_PROP_ONLINE:
+ if (power_state == 0)
+ val->intval = 1;
+ else
+ val->intval = 0;
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
+ return ret;
+}
+
+static void wacom_poke(struct hid_device *hdev, u8 data)
+{
+ int limit;
+ int ret;
+ char rep_data[2];
+ /*
+ * data has to be one of those:
+ * 0x05 - low reporting speed
+ * 0x06 - high reporting speed
+ */
+
+ /*
+ * Note that if the raw queries fail, it's not a hard failure and it
+ * is safe to continue
+ */
+ rep_data[0] = 0x03 ; rep_data[1] = 0x00;
+ limit = 3;
+ do {
+ ret = hdev->hid_output_raw_report(hdev, rep_data, 2,
+ HID_FEATURE_REPORT);
+ } while (ret < 0 && limit-- > 0);
+ if (ret >= 0) {
+ rep_data[0] = data ; rep_data[1] = 0x00;
+ limit = 3;
+ do {
+ ret = hdev->hid_output_raw_report(hdev, rep_data, 2,
+ HID_FEATURE_REPORT);
+ } while (ret < 0 && limit-- > 0);
+ if (ret < 0)
+ dev_warn(&hdev->dev,
+ "failed to poke device, command %d, err %d\n"
+ , data, ret);
+ }
+
+}
+
static int wacom_raw_event(struct hid_device *hdev, struct hid_report *report,
u8 *raw_data, int size)
{
@@ -147,6 +251,11 @@ static int wacom_raw_event(struct hid_device *hdev, struct hid_report *report,
input_sync(input);
}
+ /* Store current battery capacity */
+ rw = (data[7] >> 2 & 0x07);
+ if (rw != wdata->battery_capacity)
+ wdata->battery_capacity = rw;
+
return 1;
}
@@ -206,6 +315,43 @@ static int wacom_probe(struct hid_device *hdev,
if (ret < 0)
dev_warn(&hdev->dev, "failed to poke device #2, %d\n", ret);
+ wdata->battery.properties = wacom_battery_props;
+ wdata->battery.num_properties = ARRAY_SIZE(wacom_battery_props);
+ wdata->battery.get_property = wacom_battery_get_property;
+ wdata->battery.name = "wacom_battery";
+ wdata->battery.type = POWER_SUPPLY_TYPE_BATTERY;
+ wdata->battery.use_for_apm = 0;
+
+ ret = power_supply_register(&hdev->dev, &wdata->battery);
+ if (ret) {
+ dev_warn(&hdev->dev,
+ "can't create sysfs battery attribute, err: %d\n", ret);
+ /*
+ * battery attribute is not critical for the tablet, but if it
+ * failed then there is no need to create ac attribute
+ */
+ goto move_on;
+ }
+
+ wdata->ac.properties = wacom_ac_props;
+ wdata->ac.num_properties = ARRAY_SIZE(wacom_ac_props);
+ wdata->ac.get_property = wacom_ac_get_property;
+ wdata->ac.name = "wacom_ac";
+ wdata->ac.type = POWER_SUPPLY_TYPE_MAINS;
+ wdata->ac.use_for_apm = 0;
+
+ ret = power_supply_register(&hdev->dev, &wdata->ac);
+ if (ret) {
+ dev_warn(&hdev->dev,
+ "can't create ac battery attribute, err: %d\n", ret);
+ /*
+ * ac attribute is not critical for the tablet, but if it
+ * failed then we don't want to battery attribute to exist
+ */
+ power_supply_unregister(&wdata->battery);
+ }
+
+move_on:
hidinput = list_entry(hdev->inputs.next, struct hid_input, list);
input = hidinput->input;
@@ -250,7 +396,11 @@ err_free:
static void wacom_remove(struct hid_device *hdev)
{
+ struct wacom_data *wdata = hid_get_drvdata(hdev);
+
hid_hw_stop(hdev);
+ power_supply_unregister(&wdata->battery);
+ power_supply_unregister(&wdata->ac);
kfree(hid_get_drvdata(hdev));
}
--
1.7.0.1
^ permalink raw reply related
* Re: Phonebook functions for BlueZ
From: Marcel Holtmann @ 2010-03-09 16:58 UTC (permalink / raw)
To: Stefan Seyfried; +Cc: linux-bluetooth
In-Reply-To: <20100309150213.32d47c48@strolchi>
Hi Stefan,
> > Use your own personal judgment here. However if the reviewing the code
> > make my brain hurt, then you did it wrong ;)
>
> Now all one needs to invent is a "Marcel's-brain-hurt-O-meter" :-P
if you get one of these, send me one ;)
Regards
Marcel
^ permalink raw reply
* Re: lockdep warns: inconsistent lock state ({IN-SOFTIRQ-W} -> {SOFTIRQ-ON-W})
From: Marcel Holtmann @ 2010-03-09 16:57 UTC (permalink / raw)
To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <508e92ca1003090034y53871120n928f62573f56417d@mail.gmail.com>
Hi Andrei,
> > is this still present with 2.6.34-rc1 kernel?
>
> We are using 2.6.32-XX kernel so far.
please check with 2.6.34-rc1 to ensure it is still present. And do you
have instructions on how to re-produce it?
Regards
Marcel
^ permalink raw reply
* Re: Phonebook functions for BlueZ
From: Stefan Seyfried @ 2010-03-09 14:02 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1268112441.3712.46.camel@localhost.localdomain>
On Mon, 08 Mar 2010 21:27:21 -0800
Marcel Holtmann <marcel@holtmann.org> wrote:
> Use your own personal judgment here. However if the reviewing the code
> make my brain hurt, then you did it wrong ;)
Now all one needs to invent is a "Marcel's-brain-hurt-O-meter" :-P
Have fun,
seife
--
Stefan Seyfried
"Any ideas, John?"
"Well, surrounding them's out."
^ permalink raw reply
* Re: lockdep warns: inconsistent lock state ({IN-SOFTIRQ-W} -> {SOFTIRQ-ON-W})
From: Andrei Emeltchenko @ 2010-03-09 8:34 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <1268096416.3712.34.camel@localhost.localdomain>
Hi Marcel
On Tue, Mar 9, 2010 at 3:00 AM, Marcel Holtmann <marcel@holtmann.org> wrote=
:
> Hi Andrei,
>
>> Enabling locking debug we have triggered warning below:
>>
>> [ 2917.827178] =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>> [ 2917.833068] [ INFO: inconsistent lock state ]
>> [ 2917.837432] 2.6.32 #54
>> [ 2917.841125] ---------------------------------
>> [ 2917.845520] inconsistent {IN-SOFTIRQ-W} -> {SOFTIRQ-ON-W} usage.
>> [ 2917.851562] krfcommd/1516 [HC0[0]:SC0[0]:HE1:SE1] takes:
>> [ 2917.856903] =A0(slock-AF_BLUETOOTH){+.?...}, at: [<bf057b50>]
>> rfcomm_sk_state_change+0x78/0x160 [rfcomm]
>> [ 2917.866363] {IN-SOFTIRQ-W} state was registered at:
>> [ 2917.871276] =A0 [<c008d33c>] mark_lock+0x298/0x630
>> [ 2917.875946] =A0 [<c008ed3c>] __lock_acquire+0x5f4/0x175c
>> [ 2917.881134] =A0 [<c008ff0c>] lock_acquire+0x68/0x7c
>> [ 2917.885864] =A0 [<c036938c>] _spin_lock+0x48/0x58
>> [ 2917.890441] =A0 [<bf025960>] l2cap_conn_start+0x80/0x388 [l2cap]
>> [ 2917.896362] =A0 [<bf028f44>] l2cap_recv_frame+0x1c58/0x2fe0 [l2cap]
>> [ 2917.902526] =A0 [<bf02a3cc>] l2cap_recv_acldata+0x100/0x350 [l2cap]
>> [ 2917.908691] =A0 [<bf0035a8>] hci_rx_task+0x244/0x478 [bluetooth]
>> [ 2917.914642] =A0 [<c006c4d0>] tasklet_action+0x78/0xd8
>> [ 2917.919555] =A0 [<c006cc34>] __do_softirq+0xa8/0x154
>> [ 2917.924407] =A0 [<c006cd40>] irq_exit+0x60/0xb4
>> [ 2917.928802] =A0 [<c0030078>] asm_do_IRQ+0x78/0x90
>> [ 2917.933380] =A0 [<c0030af0>] __irq_svc+0x50/0xbc
>> [ 2917.937866] =A0 [<c0043c74>] omap3_enter_idle_bm+0x1d0/0x238
>> [ 2917.943389] =A0 [<c029ee94>] cpuidle_idle_call+0xb4/0x114
>> [ 2917.948669] =A0 [<c00320b0>] cpu_idle+0x58/0xac
>> [ 2917.953063] =A0 [<c0360b18>] rest_init+0x70/0x84
>> [ 2917.957550] =A0 [<c00089fc>] start_kernel+0x2b4/0x318
>> [ 2917.962493] =A0 [<80008034>] 0x80008034
>> [ 2917.966186] irq event stamp: 312
>> [ 2917.969421] hardirqs last =A0enabled at (312): [<c03691ac>]
>> _spin_unlock_irqrestore+0x44/0x70
>> [ 2917.977844] hardirqs last disabled at (311): [<c036947c>]
>> _spin_lock_irqsave+0x24/0x68
>> [ 2917.985809] softirqs last =A0enabled at (261): [<c006ccc8>]
>> __do_softirq+0x13c/0x154
>> [ 2917.993438] softirqs last disabled at (244): [<c006cde8>]
>> do_softirq+0x54/0x78
>> [ 2918.000732]
>> [ 2918.000732] other info that might help us debug this:
>> [ 2918.007293] 2 locks held by krfcommd/1516:
>> [ 2918.011413] =A0#0: =A0(rfcomm_mutex){+.+.+.}, at: [<bf054df4>]
>> rfcomm_run+0x1f0/0xb00 [rfcomm]
>> [ 2918.019805] =A0#1: =A0(&d->lock){+.+...}, at: [<bf055220>]
>> rfcomm_run+0x61c/0xb00 [rfcomm]
>> [ 2918.027832]
>> [ 2918.027832] stack backtrace:
>> [ 2918.032226] Backtrace:
>> [ 2918.034729] [<c00348d0>] (dump_backtrace+0x0/0x110) from [<c036616c>]
>> (dump_stack+0x18/0x1c)
>> [ 2918.043212] =A0r7:dc8f6c00 r6:c0425252 r5:00000001 r4:00000001
>> [ 2918.048950] [<c0366154>] (dump_stack+0x0/0x1c) from [<c008d060>]
>> (print_usage_bug+0x178/0x1bc)
>> [ 2918.057617] [<c008cee8>] (print_usage_bug+0x0/0x1bc) from [<c008d408>=
]
>> (mark_lock+0x364/0x630)
>> [ 2918.066284] [<c008d0a4>] (mark_lock+0x0/0x630) from [<c008edcc>]
>> (__lock_acquire+0x684/0x175c)
>> [ 2918.074951] [<c008e748>] (__lock_acquire+0x0/0x175c) from [<c008ff0c>=
]
>> (lock_acquire+0x68/0x7c)
>> [ 2918.083709] [<c008fea4>] (lock_acquire+0x0/0x7c) from [<c036938c>]
>> (_spin_lock+0x48/0x58)
>> [ 2918.091949] =A0r7:dba9402c r6:dba5c3c0 r5:dba9402c r4:bf057b50
>> [ 2918.097717] [<c0369344>] (_spin_lock+0x0/0x58) from [<bf057b50>]
>> (rfcomm_sk_state_change+0x78/0x160 [rfcomm])
>> [ 2918.107696] =A0r5:dba94000 r4:00000000
>> [ 2918.111358] [<bf057ad8>] (rfcomm_sk_state_change+0x0/0x160 [rfcomm]) =
from
>> [<bf055238>] (rfcomm_run+0x634/0xb00 [rfcomm])
>> [ 2918.122283] =A0r7:dba5c450 r6:dba5d6c0 r5:dba5c3c0 r4:dba5c430
>> [ 2918.128051] [<bf054c04>] (rfcomm_run+0x0/0xb00 [rfcomm]) from [<c007c=
c10>]
>> (kthread+0x88/0x90)
>> [ 2918.136749] [<c007cb88>] (kthread+0x0/0x90) from [<c006a86c>]
>> (do_exit+0x0/0x678)
>> [ 2918.144256] =A0r7:00000000 r6:00000000 r5:00000000 r4:00000000
>
> is this still present with 2.6.34-rc1 kernel?
We are using 2.6.32-XX kernel so far.
Regards,
Andrei
^ permalink raw reply
* Re: Kernel panic in rfcomm_run - unbalanced refcount on rfcomm_session
From: Nick Pelly @ 2010-03-09 7:31 UTC (permalink / raw)
To: Ville Tervo; +Cc: Dave Young, Bluettooth Linux, Marcel Holtmann
In-Reply-To: <4B95F67A.9090305@nokia.com>
On Mon, Mar 8, 2010 at 11:19 PM, Ville Tervo <ville.tervo@nokia.com> wrote:
> Tervo Ville (Nokia-D/Helsinki) wrote:
>>
>> l2cap socket status might change while rfcomm is processing frames. And
>> that makes rfcomm_process_rx to do double rfcomm_session_put() for incoming
>> session reference. We cannot use sk_state.
>>
>> Could you try with this patch if it helps to your problems also? My OPP
>> problems went away with this patch.
>>
>> I moved rfcomm_session_put() for incoming session to rfcomm_session_close
>> in order to get more clear _hold()/_put() pairs.
>>
>>
>
> Any comments about the patch in previous mail?
Your patch looks sane to me, although I know enough of the Bluez
rfcomm state machine to know that I don't know it that well :)
Nick
^ permalink raw reply
* Re: Kernel panic in rfcomm_run - unbalanced refcount on rfcomm_session
From: Ville Tervo @ 2010-03-09 7:19 UTC (permalink / raw)
To: Tervo Ville (Nokia-D/Helsinki)
Cc: Nick Pelly, Dave Young, Bluettooth Linux, Marcel Holtmann
In-Reply-To: <4B87A10C.4070100@nokia.com>
Tervo Ville (Nokia-D/Helsinki) wrote:
>
> l2cap socket status might change while rfcomm is processing frames. And
> that makes rfcomm_process_rx to do double rfcomm_session_put() for
> incoming session reference. We cannot use sk_state.
>
> Could you try with this patch if it helps to your problems also? My OPP
> problems went away with this patch.
>
> I moved rfcomm_session_put() for incoming session to
> rfcomm_session_close in order to get more clear _hold()/_put() pairs.
>
>
Any comments about the patch in previous mail?
--
Ville
^ permalink raw reply
* Re: Phonebook functions for BlueZ
From: Marcel Holtmann @ 2010-03-09 5:27 UTC (permalink / raw)
To: Johan Hedberg; +Cc: Felix Huber, linux-bluetooth
In-Reply-To: <20100309033841.GA21210@jh-x301>
Hi Johan,
> > > > + if (g_str_equal(property, "direction")) {
> > > > + dbus_message_iter_get_basic(&sub, &direction);
> > > > + } else if (g_str_equal(property, "peer")) {
> > > > + dbus_message_iter_get_basic(&sub, &peer);
> > > > + vc->number = g_strdup(peer);
> > > > + } else if (g_str_equal(property, "reason")) {
> > > > + dbus_message_iter_get_basic(&sub, &reason);
> > > > + } else if (g_str_equal(property, "auxstatus")) {
> > > > + dbus_message_iter_get_basic(&sub, &auxstatus);
> > > > + } else if (g_str_equal(property, "line")) {
> > > > + dbus_message_iter_get_basic(&sub, &line);
> > > > + }
> > >
> > > No braces for one-line scopes.
> > Well, here we have a conflict: The kernel style guide says that if one
> > of the blocks has braces the other one should also have, even if it is a
> > single line.
>
> Yesh, I noticed the same thing when checking the kernel coding style
> guidelines. Most of BlueZ code is in conflict with the kernel coding
> style in this respect, so I think we'd need some comment from Marcel on
> what exactly he wants the BlueZ style to be (might be that I've already
> discussed this a long time ago with him but I can't remember the outcome
> right now).
there is not real rule here that can be followed and will be true in all
cases. As long as the code is easy to read and understand it is fine. It
goes more like this: If the else statement is by itself then don't
bother with braces around it. Even if the if needs braces. If you have
multiple else if then the braces are actually a good idea. I would
almost go that far for complex ones like the one above using braces
would make it less error prone if you change something later. Even if
the compiler actually does warn you these days.
Use your own personal judgment here. However if the reviewing the code
make my brain hurt, then you did it wrong ;)
Regards
Marcel
^ permalink raw reply
* Re: Phonebook functions for BlueZ
From: Johan Hedberg @ 2010-03-09 3:38 UTC (permalink / raw)
To: Felix Huber; +Cc: linux-bluetooth
In-Reply-To: <1267982102.7714.87.camel@rb-l1.nos.office>
Hi Felix,
On Sun, Mar 07, 2010, Felix Huber wrote:
> > > +int get_ATtype(const char *buf, int *offset)
> > > +{
> > > + const char *ATquery = "=?", *ATcheck ="?", *ATset = "=";
> >
> > We usually don't use capital letters in any variable or function names.
> > Pre-processor defines are an exception. So use something like
> > get_at_type, at_query, etc.
> Well usually, but since AT in this case is a proper name, I found it
> very confusing to read it like the prepostion "at" or even a spelled-out
> @-sign. I added an underscore for better indication of what it is.
I understand your point, but this is simply the way the coding style is.
Even if I would accept it (which I wont) it'd ultimately get rejected by
Marcel.
> > > +char *fso_categories[NUM_CATEGORIES] = {"contacts", "emergency", "fixed", "dialed", "received", "missed", "own"};
> > > +char *gsm_categories[NUM_CATEGORIES] = {"\"SM\"", "\"EN\"", "\"FD\"","\"DC\"", "\"RC\"", "\"MC\"", "\"ON\""};
>
> > Probably you could split these to fit within 80 columns.
> Done, but now we loose the one-to-one correspondance.
Ah, I missed that relationship between the two lists. If this one-to-one
correspondance is so important, why not create a simple struct with two
const char* members and have just one list? Then you'd have the
definition as something like:
struct category categories[] = {
{ "contacts", "\"SM\"" },
{ "emergency", "\"EN\"" },
...
{ NULL, NULL }
};
> > > + if (!strcmp(phonebooks[i], category))
> >
> > The convention has usually been to use == 0 in the case of strcmp for
> > readability.
> >
> Not quite, as it seems: I looked at the other files and they also used
> the ! (including one commited by you :) ). So I chose the logical not,
> which also frees one from having to handle 0 vs. NULL.
BlueZ is unfortunately full of many such inconsistencies in the code:
jh@jh-x301~/src/bluez{master}$ git grep '!.*cmp('|wc -l
183
jh@jh-x301~/src/bluez{master}$ git grep 'cmp(.*== 0'|wc -l
111
But you're right in that the '!' form seems to be more frequent (those
regexps might have some false positives though). IMHO since the test in
question is a positive one ("if A is equal to B, then...") the negation
sign in the statement is at least initially counterintuitive. You might
also want to consider using g_str_equal which is quite popular througout
the code base (I got 102 hits) or g_strcmp0 in the case that there is
risk that one of the inputs could be NULL.
> > > + if ((vc = find_vc_with_status(CALL_STATUS_ACTIVE))) {
> > > + } else if ((vc = find_vc_with_status(CALL_STATUS_DIALING))) {
> > > + } else if ((vc = find_vc_with_status(CALL_STATUS_INCOMING))) {
> > > + }
> >
> > The purpose of this construction isn't imediately clear imo. Wouldn't
> > something like doing specific NULL checks after each find() call be more
> > readable? I.e.
> >
> Well, to me it was clear that these are nested calls until a valid vc is
> found. But anyway, I copied this from telephony-ofono and tried to stay
> close to the original code for better re-recognition. So either both
> should be changed or none but not be strict only on new code, since this
> makes copy-and-paste ineffective.
Ok, so let's just leave it as it is for now.
> > > +#if 0
> > > + if (!strncmp(number, "*31#", 4)) {
> > > + number += 4;
> > > + clir = "enabled";
> > > + } else if (!strncmp(number, "#31#", 4)) {
> > > + number += 4;
> > > + clir = "disabled";
> > > + } else
> > > + clir = "default";
> > > +#endif
> >
> > Is this really code that you think can be enabled later? If not I'd just
> > remove it instead of having it commented out.
> >
> Yes, it is needed. My car kit (and maybe others) have a menu to activate
> this, but the current FSO API cannot handle it yet. Since this driver
> needs to be update anyhow once the opimd is final, I left it in so it
> cannot get forgotten to be reenabled.
Alright, so it's fine then.
> > > + if (cmd) {
> > > + err = send_method_call(FSO_BUS_NAME, FSO_MODEM_OBJ_PATH,
> > > + FSO_GSMC_INTERFACE,
> > > + cmd, NULL, NULL,
> > > + DBUS_TYPE_INT32, &vc->call_index,
> > > + DBUS_TYPE_INVALID);
> > > + }
> > > + if (err < 0)
> > > + telephony_key_press_rsp(telephony_device, CME_ERROR_AG_FAILURE);
> > > + else
> > > + telephony_key_press_rsp(telephony_device, CME_ERROR_NONE);
> >
> > Shouldn't it be an error if cmd is NULL? In general doing
> > initializations upon declaration, especially for error variables, should
> > be avoided. Would e.g. having a final "else err = -EINVAL" at the end of
> > the else/else if statement make sense (which would allow removing the
> > initialzation of err to 0?
> No no, beware! It can happen that the other side of the phone call just hung up
> before the key press or that a nasty user presses a key when nothing is going on.
> In this case, the press is silenty ignored instead of confusing some headsets with an
> unexpected failure.
Right. Do fix the braces usage for the single-line though. You might
want to consider getting rid of the act and rel variables completely and
just assign directly the string to cmd (or if it bothers you to have the
same string twice for ACTIVE and DIALING create #defines for them.
> > > +static void retrieve_phonebook_reply(DBusPendingCall *call, void *user_data)
> > > +{
> > > + DBusError err;
> > > + DBusMessage *reply;
> > > + DBusMessageIter iter, array;
> > > + int ret = 0;
> >
> > Instead of initializing ret upon declaration (and btw, we use "err"
> > instead of "ret" usually) you could set it to 0 right before the done
> > label.
> Again, I checked with the other telephony files: they use ret for their
> codes and err for DBus error. So I stayed consistent with the names.
I guess this is yet another example of BlueZ internal inconsistency
then. However, I know that Marcel prefers err and we should strive to
use it in all new code.
> > > + gstr = g_string_new("(");
> > > + for (i=0; i< n_s; i++) {
> >
> > Missing spaces before and after '=', before '<'. Can you come up with a
> > more descriptive name for the n_s variable please?
> What about num_strings? This is my feeling only, since I copied this
> from the DBus tutorial.
num_strings is certianly more descriptive than the current name and I
can't come up with anything better right now, so let's just go with it.
> > > + sscanf(readindex, "%d,%d", &phonebook_info.first, &phonebook_info.last);
> > > + if (phonebook_info.first == -1)
> > > + break;
> >
> > Probably you'd also want to check for sscanf return value (i.e. == 2).
> > Maybe that's the only thing you should check for and not try to
> > initialize these variables here since you already do that in the
> > beginning of telephony-fso.c where you define the phonebook_info struct?
> Nope, the arguments are optional and both 0,1 or two conversions can
> happen, so the return of sscanf serves nothing in this case.
Ok.
> > > + if (g_str_equal(property, "direction")) {
> > > + dbus_message_iter_get_basic(&sub, &direction);
> > > + } else if (g_str_equal(property, "peer")) {
> > > + dbus_message_iter_get_basic(&sub, &peer);
> > > + vc->number = g_strdup(peer);
> > > + } else if (g_str_equal(property, "reason")) {
> > > + dbus_message_iter_get_basic(&sub, &reason);
> > > + } else if (g_str_equal(property, "auxstatus")) {
> > > + dbus_message_iter_get_basic(&sub, &auxstatus);
> > > + } else if (g_str_equal(property, "line")) {
> > > + dbus_message_iter_get_basic(&sub, &line);
> > > + }
> >
> > No braces for one-line scopes.
> Well, here we have a conflict: The kernel style guide says that if one
> of the blocks has braces the other one should also have, even if it is a
> single line.
Yesh, I noticed the same thing when checking the kernel coding style
guidelines. Most of BlueZ code is in conflict with the kernel coding
style in this respect, so I think we'd need some comment from Marcel on
what exactly he wants the BlueZ style to be (might be that I've already
discussed this a long time ago with him but I can't remember the outcome
right now).
> > > + /* ARRAY -> ENTRY -> VARIANT*/
> >
> > Space before */
> Done, copy and paste telephony-ofono.c -> should be fixed there too.
Alright. I just pushed a fix for telephony-ofono.c.
> > > --- a/audio/telephony.h
> > > +++ b/audio/telephony.h
> > > @@ -127,6 +127,12 @@ typedef enum {
> > > CME_ERROR_NETWORK_NOT_ALLOWED = 32,
> > > } cme_error_t;
> > >
> > > +/* AT command types */
> > > +#define ATNONE 0
> > > +#define ATQUERY 1
> > > +#define ATCHECK 2
> > > +#define ATSET 3
>
> > These should probably be an enum and have some namespacing (e.g.
> > AT_TYPE_).
> Again, in order to be consistent with the existing codes, I looked at
> what the other files do. I adopted the use of the Call parameters, like
> CALL_STATUS_ACTIVE. These are all #defines
Right. So AT_* should be enough then (as you've done).
Johan
^ permalink raw reply
* RE: [bluetooth-next] bluetooth: hci_sysfs: use strict_strtoul instead of simple_strtoul
From: Marcel Holtmann @ 2010-03-09 2:16 UTC (permalink / raw)
To: Winkler, Tomas
Cc: linux-bluetooth@vger.kernel.org, Cohen, Guy, Rindjunsky, Ron
In-Reply-To: <6F5C1D715B2DA5498A628E6B9C124F04016C0E8D6B@hasmsx504.ger.corp.intel.com>
Hi Tomas,
> > > Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> > > ---
> > > net/bluetooth/hci_sysfs.c | 24 ++++++++++++------------
> > > 1 files changed, 12 insertions(+), 12 deletions(-)
> >
> > can you please explain the rational behind this change. What is the
> > benefit? I just fail to see it right away.
>
> The real reason using strict instead of simple strtoul is explained here thttp://www.kernel.org/doc/htmldocs/kernel-api/re42.html
> In the bottom line it just something a chackpatch is complain about. I've touched the file to insert some test hook for the HCI reset so I fixed that on the way.
I am fine with that. However please use the following constructs:
if (strict_strtoul(...) < 0)
return -EINVAL;
There is no point in having ret variable if you don't use it.
Also I like to have a commit body and not only a subject line. It
doesn't have to be a novel, but only the subject is not good enough for
me.
Regards
Marcel
^ permalink raw reply
* RE: [bluetooth-next] bluetooth: hci_sysfs: use strict_strtoul instead of simple_strtoul
From: Winkler, Tomas @ 2010-03-09 1:14 UTC (permalink / raw)
To: Marcel Holtmann
Cc: linux-bluetooth@vger.kernel.org, Cohen, Guy, Rindjunsky, Ron
In-Reply-To: <1268096639.3712.37.camel@localhost.localdomain>
DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogTWFyY2VsIEhvbHRtYW5u
IFttYWlsdG86bWFyY2VsQGhvbHRtYW5uLm9yZ10NCj4gU2VudDogVHVlc2RheSwgTWFyY2ggMDks
IDIwMTAgMzowNCBBTQ0KPiBUbzogV2lua2xlciwgVG9tYXMNCj4gQ2M6IGxpbnV4LWJsdWV0b290
aEB2Z2VyLmtlcm5lbC5vcmc7IENvaGVuLCBHdXk7IFJpbmRqdW5za3ksIFJvbg0KPiBTdWJqZWN0
OiBSZTogW2JsdWV0b290aC1uZXh0XSBibHVldG9vdGg6IGhjaV9zeXNmczogdXNlIHN0cmljdF9z
dHJ0b3VsDQo+IGluc3RlYWQgb2Ygc2ltcGxlX3N0cnRvdWwNCj4gDQo+IEhpIFRvbWFzLA0KPiAN
Cj4gPiBTaWduZWQtb2ZmLWJ5OiBUb21hcyBXaW5rbGVyIDx0b21hcy53aW5rbGVyQGludGVsLmNv
bT4NCj4gPiAtLS0NCj4gPiAgbmV0L2JsdWV0b290aC9oY2lfc3lzZnMuYyB8ICAgMjQgKysrKysr
KysrKysrLS0tLS0tLS0tLS0tDQo+ID4gIDEgZmlsZXMgY2hhbmdlZCwgMTIgaW5zZXJ0aW9ucygr
KSwgMTIgZGVsZXRpb25zKC0pDQo+IA0KPiBjYW4geW91IHBsZWFzZSBleHBsYWluIHRoZSByYXRp
b25hbCBiZWhpbmQgdGhpcyBjaGFuZ2UuIFdoYXQgaXMgdGhlDQo+IGJlbmVmaXQ/IEkganVzdCBm
YWlsIHRvIHNlZSBpdCByaWdodCBhd2F5Lg0KDQpUaGUgcmVhbCByZWFzb24gdXNpbmcgc3RyaWN0
IGluc3RlYWQgb2Ygc2ltcGxlIHN0cnRvdWwgaXMgZXhwbGFpbmVkIGhlcmUgdGh0dHA6Ly93d3cu
a2VybmVsLm9yZy9kb2MvaHRtbGRvY3Mva2VybmVsLWFwaS9yZTQyLmh0bWwNCkluIHRoZSBib3R0
b20gbGluZSBpdCBqdXN0IHNvbWV0aGluZyBhIGNoYWNrcGF0Y2ggaXMgY29tcGxhaW4gYWJvdXQu
IEkndmUgdG91Y2hlZCB0aGUgZmlsZSB0byBpbnNlcnQgc29tZSB0ZXN0IGhvb2sgZm9yIHRoZSBI
Q0kgcmVzZXQgc28gSSBmaXhlZCB0aGF0IG9uIHRoZSB3YXkuIA0KDQpUaGFua3MNClRvbWFzDQoN
Ci0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t
LS0tLS0tLS0tLS0tLQpJbnRlbCBJc3JhZWwgKDc0KSBMaW1pdGVkCgpUaGlzIGUtbWFpbCBhbmQg
YW55IGF0dGFjaG1lbnRzIG1heSBjb250YWluIGNvbmZpZGVudGlhbCBtYXRlcmlhbCBmb3IKdGhl
IHNvbGUgdXNlIG9mIHRoZSBpbnRlbmRlZCByZWNpcGllbnQocykuIEFueSByZXZpZXcgb3IgZGlz
dHJpYnV0aW9uCmJ5IG90aGVycyBpcyBzdHJpY3RseSBwcm9oaWJpdGVkLiBJZiB5b3UgYXJlIG5v
dCB0aGUgaW50ZW5kZWQKcmVjaXBpZW50LCBwbGVhc2UgY29udGFjdCB0aGUgc2VuZGVyIGFuZCBk
ZWxldGUgYWxsIGNvcGllcy4K
^ permalink raw reply
* Re: [bluetooth-next] bluetooth: hci_sysfs: use strict_strtoul instead of simple_strtoul
From: Marcel Holtmann @ 2010-03-09 1:03 UTC (permalink / raw)
To: Tomas Winkler; +Cc: linux-bluetooth, guy.cohen, ron.rindjunsky
In-Reply-To: <1266843933-28802-2-git-send-email-tomas.winkler@intel.com>
Hi Tomas,
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> ---
> net/bluetooth/hci_sysfs.c | 24 ++++++++++++------------
> 1 files changed, 12 insertions(+), 12 deletions(-)
can you please explain the rational behind this change. What is the
benefit? I just fail to see it right away.
Regards
Marcel
^ permalink raw reply
* Re: [bluetooth-next V2] Bluetooth: handle device reset event
From: Marcel Holtmann @ 2010-03-09 1:03 UTC (permalink / raw)
To: Tomas Winkler; +Cc: linux-bluetooth, guy.cohen, ron.rindjunsky, Gregory Paskar
In-Reply-To: <1266843933-28802-1-git-send-email-tomas.winkler@intel.com>
Hi Tomas,
> A Bluetooth device experiencing hardware failure may issue
> a HARDWARE_ERROR hci event. The reaction to this event is device
> reset flow implemented in following sequence.
>
> 1. Notify: HCI_DEV_DOWN
> 2. Reinitialize internal structures.
> 3. Call driver flush function
> 4. Send HCI reset request to the device.
> 5. Send HCI init sequence reset to the device.
> 6. Notify HCI_DEV_UP.
I prefer if we create a generic per controller workqueue first before
having a workqueue for every task. Something similar to what the
mac80211 layer offers right now.
Also in a second step we might wanna move the HCI event processing
completely into a workqueue. If we get no performance hit with that,
then sysfs handling and device reset becomes a lot simpler and less
prone to race conditions with device removal.
Regards
Marcel
^ permalink raw reply
* Re: lockdep warns: inconsistent lock state ({IN-SOFTIRQ-W} -> {SOFTIRQ-ON-W})
From: Marcel Holtmann @ 2010-03-09 1:00 UTC (permalink / raw)
To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <508e92ca1002260126r600ff956hb18c333504fe6525@mail.gmail.com>
Hi Andrei,
> Enabling locking debug we have triggered warning below:
>
> [ 2917.827178] =================================
> [ 2917.833068] [ INFO: inconsistent lock state ]
> [ 2917.837432] 2.6.32 #54
> [ 2917.841125] ---------------------------------
> [ 2917.845520] inconsistent {IN-SOFTIRQ-W} -> {SOFTIRQ-ON-W} usage.
> [ 2917.851562] krfcommd/1516 [HC0[0]:SC0[0]:HE1:SE1] takes:
> [ 2917.856903] (slock-AF_BLUETOOTH){+.?...}, at: [<bf057b50>]
> rfcomm_sk_state_change+0x78/0x160 [rfcomm]
> [ 2917.866363] {IN-SOFTIRQ-W} state was registered at:
> [ 2917.871276] [<c008d33c>] mark_lock+0x298/0x630
> [ 2917.875946] [<c008ed3c>] __lock_acquire+0x5f4/0x175c
> [ 2917.881134] [<c008ff0c>] lock_acquire+0x68/0x7c
> [ 2917.885864] [<c036938c>] _spin_lock+0x48/0x58
> [ 2917.890441] [<bf025960>] l2cap_conn_start+0x80/0x388 [l2cap]
> [ 2917.896362] [<bf028f44>] l2cap_recv_frame+0x1c58/0x2fe0 [l2cap]
> [ 2917.902526] [<bf02a3cc>] l2cap_recv_acldata+0x100/0x350 [l2cap]
> [ 2917.908691] [<bf0035a8>] hci_rx_task+0x244/0x478 [bluetooth]
> [ 2917.914642] [<c006c4d0>] tasklet_action+0x78/0xd8
> [ 2917.919555] [<c006cc34>] __do_softirq+0xa8/0x154
> [ 2917.924407] [<c006cd40>] irq_exit+0x60/0xb4
> [ 2917.928802] [<c0030078>] asm_do_IRQ+0x78/0x90
> [ 2917.933380] [<c0030af0>] __irq_svc+0x50/0xbc
> [ 2917.937866] [<c0043c74>] omap3_enter_idle_bm+0x1d0/0x238
> [ 2917.943389] [<c029ee94>] cpuidle_idle_call+0xb4/0x114
> [ 2917.948669] [<c00320b0>] cpu_idle+0x58/0xac
> [ 2917.953063] [<c0360b18>] rest_init+0x70/0x84
> [ 2917.957550] [<c00089fc>] start_kernel+0x2b4/0x318
> [ 2917.962493] [<80008034>] 0x80008034
> [ 2917.966186] irq event stamp: 312
> [ 2917.969421] hardirqs last enabled at (312): [<c03691ac>]
> _spin_unlock_irqrestore+0x44/0x70
> [ 2917.977844] hardirqs last disabled at (311): [<c036947c>]
> _spin_lock_irqsave+0x24/0x68
> [ 2917.985809] softirqs last enabled at (261): [<c006ccc8>]
> __do_softirq+0x13c/0x154
> [ 2917.993438] softirqs last disabled at (244): [<c006cde8>]
> do_softirq+0x54/0x78
> [ 2918.000732]
> [ 2918.000732] other info that might help us debug this:
> [ 2918.007293] 2 locks held by krfcommd/1516:
> [ 2918.011413] #0: (rfcomm_mutex){+.+.+.}, at: [<bf054df4>]
> rfcomm_run+0x1f0/0xb00 [rfcomm]
> [ 2918.019805] #1: (&d->lock){+.+...}, at: [<bf055220>]
> rfcomm_run+0x61c/0xb00 [rfcomm]
> [ 2918.027832]
> [ 2918.027832] stack backtrace:
> [ 2918.032226] Backtrace:
> [ 2918.034729] [<c00348d0>] (dump_backtrace+0x0/0x110) from [<c036616c>]
> (dump_stack+0x18/0x1c)
> [ 2918.043212] r7:dc8f6c00 r6:c0425252 r5:00000001 r4:00000001
> [ 2918.048950] [<c0366154>] (dump_stack+0x0/0x1c) from [<c008d060>]
> (print_usage_bug+0x178/0x1bc)
> [ 2918.057617] [<c008cee8>] (print_usage_bug+0x0/0x1bc) from [<c008d408>]
> (mark_lock+0x364/0x630)
> [ 2918.066284] [<c008d0a4>] (mark_lock+0x0/0x630) from [<c008edcc>]
> (__lock_acquire+0x684/0x175c)
> [ 2918.074951] [<c008e748>] (__lock_acquire+0x0/0x175c) from [<c008ff0c>]
> (lock_acquire+0x68/0x7c)
> [ 2918.083709] [<c008fea4>] (lock_acquire+0x0/0x7c) from [<c036938c>]
> (_spin_lock+0x48/0x58)
> [ 2918.091949] r7:dba9402c r6:dba5c3c0 r5:dba9402c r4:bf057b50
> [ 2918.097717] [<c0369344>] (_spin_lock+0x0/0x58) from [<bf057b50>]
> (rfcomm_sk_state_change+0x78/0x160 [rfcomm])
> [ 2918.107696] r5:dba94000 r4:00000000
> [ 2918.111358] [<bf057ad8>] (rfcomm_sk_state_change+0x0/0x160 [rfcomm]) from
> [<bf055238>] (rfcomm_run+0x634/0xb00 [rfcomm])
> [ 2918.122283] r7:dba5c450 r6:dba5d6c0 r5:dba5c3c0 r4:dba5c430
> [ 2918.128051] [<bf054c04>] (rfcomm_run+0x0/0xb00 [rfcomm]) from [<c007cc10>]
> (kthread+0x88/0x90)
> [ 2918.136749] [<c007cb88>] (kthread+0x0/0x90) from [<c006a86c>]
> (do_exit+0x0/0x678)
> [ 2918.144256] r7:00000000 r6:00000000 r5:00000000 r4:00000000
is this still present with 2.6.34-rc1 kernel?
Regards
Marcel
^ 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