* Re: [PATCH v3] Bluetooth: Add blacklist support for incoming connections
From: Luiz Augusto von Dentz @ 2010-05-19 13:55 UTC (permalink / raw)
To: Gustavo F. Padovan; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <20100519084715.GA11110@vigoh>
Hi Gustavo,
On Wed, May 19, 2010 at 10:48 AM, Gustavo F. Padovan
<gustavo@padovan.org> wrote:
> Hi Johan,
>
> * Johan Hedberg <johan.hedberg@nokia.com> [2010-05-19 10:22:05 +0200]:
>
>> Hi Gustavo,
>>
>> On Wed, May 19, 2010, Gustavo F. Padovan wrote:
>> > > In some circumstances it could be desirable to reject incoming
>> > > connections on the baseband level. This patch adds this feature through
>> > > two new ioctl's: HCIBLOCKADDR and HCIUNBLOCKADDR. Both take a simple
>> > > Bluetooth address as a parameter. BDADDR_ANY can be used with
>> > > HCIUNBLOCKADDR to remove all devices from the blacklist.
>> >
>> > Which circumstances?
>>
>> Whenever you have a badly behaving remote device that keeps on trying to
>> connect to you (e.g. an OPP spambot or someone trying to actively DoS
>> you). I gave some more details in my reply to Jaikumar a few days ago on
>> this mailing list.
>
> Nice. Patch looks ok to me by the way. ;)
Its also very, almost a must, convenient when doing interoperability
testing in some events like upf where we have a lot (annoying) devices
trying to reconnect to us while we are testing with other devices.
@ Johan, Blocked does supersedes Trusted right? Maybe we should rework
those properties (deprecate them?) to something like Authorization
which takes a string where lets say can assume these values: "deny"
("block"), "ask" or "allow".
--
Luiz Augusto von Dentz
Computer Engineer
^ permalink raw reply
* Re: [PATCH v3] Bluetooth: Add blacklist support for incoming connections
From: Johan Hedberg @ 2010-05-19 14:05 UTC (permalink / raw)
To: Luiz Augusto von Dentz
Cc: Gustavo F. Padovan, linux-bluetooth@vger.kernel.org
In-Reply-To: <AANLkTimdw00tjbCHK52SNF9FGJjLVHR35f44dX_bzj__@mail.gmail.com>
Hi Luiz,
On Wed, May 19, 2010, Luiz Augusto von Dentz wrote:
> @ Johan, Blocked does supersedes Trusted right?
Right. What also happens when you set Blocked to False is that all the
device drivers are unloaded for that device. When Blocked goes back to
true the drivers get probed again.
> Maybe we should rework those properties (deprecate them?) to something
> like Authorization which takes a string where lets say can assume
> these values: "deny" ("block"), "ask" or "allow".
Sounds like an interesting idea, but I'm not completely convinced about
it since the properties act on different layers. Currently authorization
is on the profile layer and is completely optional for a profile
implementation (i.e. it needs to call btd_request_authorization
(bluetoothd plugin) or RequestAuthorization (D-Bus) whereas there's no
way to skip the blocked check in the kernel. I.e. if we'd have a unified
property, the value "blocked" would be unconditional but "ask" wouldn't
always mean "ask": it would only happen once the connection reaches the
profile level and only for the profiles that actually enforce
authorization.
Johan
^ permalink raw reply
* Re: [PATCH v3] Bluetooth: Add blacklist support for incoming connections
From: Johan Hedberg @ 2010-05-19 14:10 UTC (permalink / raw)
To: Luiz Augusto von Dentz, Gustavo F. Padovan,
linux-bluetooth@vger.kernel.org
In-Reply-To: <20100519140503.GA6822@jh-x301>
On Wed, May 19, 2010, Johan Hedberg wrote:
> What also happens when you set Blocked to False is that all the device
> drivers are unloaded for that device. When Blocked goes back to true
> the drivers get probed again.
I mixed up True/False there, but you probably realized that :)
Johan
^ permalink raw reply
* Re: [PATCH 0/2] Groundwork for Sixaxis paring into bluez.
From: Antonio Ospite @ 2010-05-20 9:05 UTC (permalink / raw)
To: linux-input; +Cc: Antonio Ospite, Bastien Nocera, linux-bluetooth, cbe-oss-dev
In-Reply-To: <1272917756-31098-1-git-send-email-ospite@studenti.unina.it>
[-- Attachment #1: Type: text/plain, Size: 1524 bytes --]
On Mon, 3 May 2010 22:15:54 +0200
Antonio Ospite <ospite@studenti.unina.it> wrote:
> Hi,
>
Ping.
> These changes are in preparation of seamless integration of the Sixaxis
> pairing procedure into bluez. It was about time :)
>
> The first one cleans up the quirks handling.
>
Even if the second patch in this series is not OK and extending hidraw
is the way, I still think that the first one can be accepted. Anyone?
> The second one adds a sysfs interface to set and get the device and master
> bdaddr, this is needed in order to setup the host for pairing. This kernel
> interface is a mere workaround for the lack of features of hidraw[1], if
> someone wants to add the missing features to hidraw I will be happy to drop
> this one, otherwise consider queuing these for 2.6.35 if it's too late for
> 2.6.34.
>
> Thanks,
> Antonio
>
> [1] http://www.spinics.net/lists/linux-input/msg08311.html
>
>
> Antonio Ospite (2):
> hid/hid-sony: Apply sixaxis quirks only to sixaxis
> hid/hid-sony: get and set Sixaxis bdaddr via sysfs
>
> drivers/hid/hid-sony.c | 170 +++++++++++++++++++++++++++++++++++++++++++----
> 1 files changed, 155 insertions(+), 15 deletions(-)
>
--
Antonio Ospite
http://ao2.it
PGP public key ID: 0x4553B001
A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?
[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* Re: + btusb-patch-add_apple_macbookpro62.patch added to -mm tree
From: Bernhard Froemel @ 2010-05-20 9:29 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: akpm, mm-commits, linux-bluetooth
In-Reply-To: <1274344173.27220.7.camel@localhost.localdomain>
[-- Attachment #1: Type: text/plain, Size: 2857 bytes --]
Hello Marcel,
On 05/20/2010 10:29 AM, Marcel Holtmann wrote:
> Please send your patch to linux-bluetooth@vger.kernel.org and include
>>> the part for this device from /proc/bus/usb/devices in your commit
>>> message.
>>>
>>> Regards
>>>
>>> Marcel
>>>
>>>
>>>
>> usbfs disappears on recent kernels [1], at least for desktop machines
>> (and most certainly on my stock distribution). Don't want to be contra
>> productive here, but if you insist on this "proof" (i.e. don't accept
>> the output of the above lsusb command or my forum discussion reference
>> [2] as proof enough) and want me to spend another hour compiling a
>> kernel with enabled usbfs for such a small fix, then I'll pass on this
>> one for now.
>>
> just run the attached script. It does the same thing. Or you could just
> mount usbfs.
>
There is *no* usbfs available in my current kernel - I'd need to enable
and recompile: that's why I can't mount. Distribution vendors (like
Ubuntu) follow the recommendation concerning usbfs support in their
stock kernels (it's marked deprecated).
Your script does not work on devices attached to (internal) hubs - they
are silently omitted. I modified it accordingly and that's the output:
> T: Bus=1 Dev#=8 Spd=12 MxCh=0
> D: Ver=2.00 Cls=ff(vendr) Sub=01 Prot=01 MxPS=64 #Cfgs=1
> P: Vendor=05ac ProdID=8218 Rev=0.22
> S: Manufacturer=Apple Inc.
> S: Product=Bluetooth USB Host Controller
> C: #Ifs=4 Cfg#=1 Atr=e0 MxPwr=0mA
> E: Ad=00(both) Atr=00(Control) MxPS=0040 Ivl=0ms
> I: If#=00 Alt=0 #EPs=03 Cls=ff(vendr) Sub=01 Prot=01 Driver=btusb
> I: If#=01 Alt=0 #EPs=02 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
> I: If#=02 Alt=0 #EPs=02 Cls=ff(vendr) Sub=ff Prot=ff Driver=(none)
> I: If#=03 Alt=0 #EPs=00 Cls=fe(apspe) Sub=01 Prot=01 Driver=(none)
As visible here and in my previously quoted lsusb output the class isn't
set correctly (0xff instead of 0xe0), so the driver won't detect the
device. It does here, because - of course - I run the patched btusb module.
When I remove my patched DKMS module (and use the stock module as
shipped in 2.6.32), the output is:
> T: Bus=1 Dev#=8 Spd=12 MxCh=0
> D: Ver=2.00 Cls=ff(vendr) Sub=01 Prot=01 MxPS=64 #Cfgs=1
> P: Vendor=05ac ProdID=8218 Rev=0.22
> S: Manufacturer=Apple Inc.
> S: Product=Bluetooth USB Host Controller
> C: #Ifs=4 Cfg#=1 Atr=e0 MxPwr=0mA
> E: Ad=00(both) Atr=00(Control) MxPS=0040 Ivl=0ms
> I: If#=00 Alt=0 #EPs=03 Cls=ff(vendr) Sub=01 Prot=01 Driver=(none)
> I: If#=01 Alt=0 #EPs=02 Cls=e0(wlcon) Sub=01 Prot=01 Driver=(none)
> I: If#=02 Alt=0 #EPs=02 Cls=ff(vendr) Sub=ff Prot=ff Driver=(none)
> I: If#=03 Alt=0 #EPs=00 Cls=fe(apspe) Sub=01 Prot=01 Driver=(none)
I hope you're finally convinced, so attached once more the patch (and CC
to linux-bluetooth).
Signed-off-by: Bernhard Froemel <froemel@vmars.tuwien.ac.at>
-bf
[-- Attachment #2: btusb_add_apple_MacBookPro62.patch --]
[-- Type: text/x-patch, Size: 351 bytes --]
--- linux/drivers/bluetooth/btusb.c 2010-05-06 15:56:18.228630685 +0200
+++ linux_p/drivers/bluetooth/btusb.c 2010-05-02 11:03:55.955362854 +0200
@@ -75,6 +75,9 @@
/* Canyon CN-BTU1 with HID interfaces */
{ USB_DEVICE(0x0c10, 0x0000) },
+ /* Apple MacBookPro6 BT device */
+ { USB_DEVICE(0x05ac, 0x8218) },
+
{ } /* Terminating entry */
};
^ permalink raw reply
* Re: bluez: lowercase hex digits in BDADDRs
From: Antonio Ospite @ 2010-05-20 9:57 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <20100505151220.137a0b37.ospite@studenti.unina.it>
[-- Attachment #1: Type: text/plain, Size: 1196 bytes --]
On Wed, 5 May 2010 15:12:20 +0200
Antonio Ospite <ospite@studenti.unina.it> wrote:
> Hi,
>
> when working on Sixaxis pairing plugin I found out a weird behavior, if
> I use a BDADDR with lowercase hex digits the association can fail.
>
Ping. Anyone ever experienced that?
> The interesting bits in bluetoothd logs are reported in the attached
> diff, maybe adapter_create_device() is not robust enough?
>
> You can reproduce this behavior using a lowercase _device_ bdaddr
> into these files:
> /var/lib/bluetooth/00:03:C9:XX:XX:XX/did
> /var/lib/bluetooth/00:03:C9:XX:XX:XX/lastused
> /var/lib/bluetooth/00:03:C9:XX:XX:XX/sdp
> /var/lib/bluetooth/00:03:C9:XX:XX:XX/trusts
> where 00:03:C9:XX:XX:XX is the _adapter_ bdaddr.
>
> Can anyone else confirm that? bluez-4.64
>
> Not a big deal, but worth reporting IMHO.
>
> Thanks,
> Antonio
>
--
Antonio Ospite
http://ao2.it
PGP public key ID: 0x4553B001
A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?
[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* [PATCH] Fix device_match_pattern function
From: Santiago Carot-Nemesio @ 2010-05-20 10:09 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Santiago Carot-Nemesio
This patch fixes a problem adding uuids to list provided when a
driver is probed. Without this patch the same uuids were added
more than once to list and if two or more uuids were in the
same class id list only the first one was included repeatedly
---
src/device.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/device.c b/src/device.c
index 6ba1612..d0768ce 100644
--- a/src/device.c
+++ b/src/device.c
@@ -1053,7 +1053,7 @@ static GSList *device_match_pattern(struct btd_device *device,
continue;
if (record_has_uuid(rec, match_uuid))
- uuids = g_slist_append(uuids, profile_uuid);
+ uuids = g_slist_append(uuids, match_uuid);
}
return uuids;
--
1.6.3.3
^ permalink raw reply related
* Re: [PATCH] Fix device_match_pattern function
From: Luiz Augusto von Dentz @ 2010-05-20 12:52 UTC (permalink / raw)
To: Santiago Carot-Nemesio; +Cc: linux-bluetooth
In-Reply-To: <1274350142-19083-1-git-send-email-sancane@gmail.com>
Hi Santiago,
On Thu, May 20, 2010 at 12:09 PM, Santiago Carot-Nemesio
<sancane@gmail.com> wrote:
> This patch fixes a problem adding uuids to list provided when a
> driver is probed. Without this patch the same uuids were added
> more than once to list and if two or more uuids were in the
> same class id list only the first one was included repeatedly
> ---
> src/device.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/src/device.c b/src/device.c
> index 6ba1612..d0768ce 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -1053,7 +1053,7 @@ static GSList *device_match_pattern(struct btd_device *device,
> continue;
>
> if (record_has_uuid(rec, match_uuid))
> - uuids = g_slist_append(uuids, profile_uuid);
> + uuids = g_slist_append(uuids, match_uuid);
> }
>
> return uuids;
It doesn't look right, if we do that the device will be probed by the
matched uuid which would not happen to have a record in the storage.
So in other words the list of uuids you get in the probe may not match
with the one present in the drivers .uuids. Also this would probably
break serial driver, did you tried this before submitting this to the
list?
--
Luiz Augusto von Dentz
Computer Engineer
^ permalink raw reply
* Re: [PATCH] Fix device_match_pattern function
From: Luiz Augusto von Dentz @ 2010-05-20 13:00 UTC (permalink / raw)
To: Santiago Carot-Nemesio; +Cc: linux-bluetooth
In-Reply-To: <AANLkTimv6CRgS-lPeadePxKXEmNmhEM7oLx-uTHuZfNF@mail.gmail.com>
Hi,
On Thu, May 20, 2010 at 2:52 PM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> Hi Santiago,
>
> On Thu, May 20, 2010 at 12:09 PM, Santiago Carot-Nemesio
> <sancane@gmail.com> wrote:
>> This patch fixes a problem adding uuids to list provided when a
>> driver is probed. Without this patch the same uuids were added
>> more than once to list and if two or more uuids were in the
>> same class id list only the first one was included repeatedly
>> ---
>> src/device.c | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/src/device.c b/src/device.c
>> index 6ba1612..d0768ce 100644
>> --- a/src/device.c
>> +++ b/src/device.c
>> @@ -1053,7 +1053,7 @@ static GSList *device_match_pattern(struct btd_device *device,
>> continue;
>>
>> if (record_has_uuid(rec, match_uuid))
>> - uuids = g_slist_append(uuids, profile_uuid);
>> + uuids = g_slist_append(uuids, match_uuid);
>> }
>>
>> return uuids;
>
>
> It doesn't look right, if we do that the device will be probed by the
> matched uuid which would not happen to have a record in the storage.
> So in other words the list of uuids you get in the probe may not match
> with the one present in the drivers .uuids. Also this would probably
> break serial driver, did you tried this before submitting this to the
> list?
Ok, I did apply you change to see what it happens, first it didn't
compile, but anyway I add cast just to make sure what would gonna
happen:
bluetoothd[23078]: Probe drivers for /org/bluez/23078/hci0/dev_00_26_CC_77_E3_36
bluetoothd[23078]: serial_probe: path
/org/bluez/23078/hci0/dev_00_26_CC_77_E3_36:
00000003-0000-1000-8000-00805F9B34FB
bluetoothd[23078]: serial_probe: path
/org/bluez/23078/hci0/dev_00_26_CC_77_E3_36:
00000003-0000-1000-8000-00805F9B34FB
bluetoothd[23078]: serial_probe: path
/org/bluez/23078/hci0/dev_00_26_CC_77_E3_36:
00000003-0000-1000-8000-00805F9B34FB
bluetoothd[23078]: serial_probe: path
/org/bluez/23078/hci0/dev_00_26_CC_77_E3_36:
00000003-0000-1000-8000-00805F9B34FB
bluetoothd[23078]: serial_probe: path
/org/bluez/23078/hci0/dev_00_26_CC_77_E3_36:
00000003-0000-1000-8000-00805F9B34FB
bluetoothd[23078]: serial_probe: path
/org/bluez/23078/hci0/dev_00_26_CC_77_E3_36:
00000003-0000-1000-8000-00805F9B34FB
bluetoothd[23078]: serial_probe: path
/org/bluez/23078/hci0/dev_00_26_CC_77_E3_36:
00000003-0000-1000-8000-00805F9B34FB
bluetoothd[23078]: headset_probe: path
/org/bluez/23078/hci0/dev_00_26_CC_77_E3_36
See, instead of the real profile uuid all I got was a multiple rfcomm
uuid which doesn't have any meaning alone.
--
Luiz Augusto von Dentz
Computer Engineer
^ permalink raw reply
* HDP API (version 0.6)
From: José Antonio Santos Cadenas @ 2010-05-20 13:01 UTC (permalink / raw)
To: linux-bluetooth
Hi all,
We tried to fit a little bit more with the BlueZ API reducing the number of
objects and centring the connection and disconnection issues in the
HealthDevice object. As was suggested previously in the list.
Regards
------------
BlueZ D-Bus Health API description
**********************************
Santiago Carot-Nemesio <sancane@gmail.com>
José Antonio Santos-Cadenas <santoscadenas@gmail.com>
Elvis Pfützenreuter <epx@signove.com>
Health Device Profile hierarchy
===============================
Service org.bluez
Interface org.bluez.HealthAdapter
Object path [variable prefix]/{hci0,hci1,...}
Methods:
uint32 CreateInstance(object path, dict config)
Returns the id of the new created instance. The path parameter
is the path of the remote object with the callbacks to notify
events (see org.bluez.HealthAgent at the end of this document)
This petition starts an mcap instance and also register in the
SDP if is needed.
Dict is defined as bellow:
{ "data_spec" : The data_spec is the data exchange specification
(see section 5.2.10 of the specification
document) possible values:
0x00 = reserved,
0x01 [IEEE 11073-20601],
0x02..0xff reserved,
(optional)
"end_points" : [{ (optional)
"mdepid" : uint8, (optional)
"role" : ("source" or "sink"), (mandatory)
"specs" :[{ (mandatory)
"data_type" : uint16, (mandatory)
"description" : string, (optional)
}]
}]
}
if "data_spec" is not set, no SDP record will be registered, so
all the other data in the dictionary will be ignored.
Instance will be closed by the call or implicitly when the
programs leaves the bus.
Possible errors: org.bluez.Error.InvalidArguments
void CloseInstance(uint32 )
Closes the HDP instance identified by the object path. Also
instance will be closed if the process that started leaves the
bus. If there is a SDP record associated to this instance it
will be removed.
Possible errors: org.bluez.Error.InvalidArguments
org.bluez.Error.NotFound
--------------------------------------------------------------------------------
Service org.bluez
Interface org.bluez.HealthDevice
Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
Methods:
array GetHealthInstances()
Gets the information of the remote instances present in this
device and published on its SDP record. The returned data
follows this format.
[{"id": uint32,
"data_spec" : data spec,
"end_points":
["mdepid": uint8,
"role" : "source" or "sink" ,
"specs" : [{
"dtype" : uint16,
"description" : string, (optional)
}]
]
}];
object Connect(uint32 local_instance_id, uint32 remote_instance_id)
Connects the local instance with the remote instance and returns
the path of the HealthLink object. You should get the remote
instance id running GetHealthInstances.
Only the bus client that created the local session will be able
to create connections using it.
Possible errors: org.bluez.Error.InvalidArguments
org.bluez.Error.HealthError
void Disconnect(object link, boolean cache)
Disconnect from the link. If cache is false, state will also be
deleted. Otherwise, the state will be kept for allowing future
reconnections until the adapter holding the local session is
removed.
Possible errors: org.bluez.Error.InvalidArguments
org.bluez.Error.NotFound
org.bluez.Error.HealthError
--------------------------------------------------------------------------------
Service org.bluez
Interface org.bluez.HealthLink
Object path [variable prefix]/{hci0,hci1,...}/{hdp0,hdp1,...}/rem_inst_id
Methods:
boolean Echo(array{byte})
Sends an echo petition to the remote intance. Returns True if
response matches with the buffer sent. If some error is detected
False value is returned and the associated MCL is closed.
uint16 OpenDataChannel(byte mdepid, byte config)
Creates a new data channel with the indicated config to the
remote MCAP Data End Point (MDEP).
The configuration should indicate the channel quality of
service. In the current version of HDP, valid values are 0x01
for reliable channels and 0x02 for streaming data channel.
Returns the data channel id.
Possible errors: org.bluez.Error.InvalidArguments
org.bluez.Error.HealthError
array GetDataChannelFileDescriptor(uint16 mdlid)
Gets a file descriptor where data can be read or
written for receive or sent by the data channel.
Returns an array of file descriptors one for write
and other for read.
Possible errors: org.bluez.Error.InvalidArguments
org.bluez.Error.NotFound
org.bluez.Error.HealthError
void DeleteDataChannel(uint16 mdlid)
Deletes a data channel so it will not be available for
use.
Possible errors: org.bluez.Error.InvalidArguments
org.bluez.Error.NotFound
org.bluez.Error.HealthError
void DeleteAllDataChannels()
Deletes all data channels so it will not be available
for use. Typically this function is called when the
connection with the remote device will be closed
permanently
Possible errors: org.bluez.Error.HealthError
dict GetDataChannelStatus()
Return a dictionary with all the data channels that
can be used to send data right now. The dictionary
is formed like follows:
{
"reliable": [mdlid_r1, mdlid_r2, ...],
"streaming" : [mdlid_s1, mdlid_s2, ...]
}
The fist reliable data channel will always be the first
data channel in reliable array.
HealthAgent hierarchy
=====================
(this object is implemented by the HDP user in order to receive notifications)
Service unique name
Interface org.bluez.HealthAgent
Object path freely definable
Methods:
void LinkConnected(object path)
This method is called whenever a new connection has been
established over the control channel of the current HDP
instance. The object path paremeter contains the object path of
the created HealthLink.
void LinkDisconnected(object path)
This method is called when a remote device is disconnected
definitively. Any future reconnections will fail. Also all data
channels associated to this device will be closed.
void CreatedDataChannel(object path, uint16 mdlid, byte conf)
This method is called when a new data channel is created
The path contains the object path of the HealthLink where the
new connection is created, the mdlid is the data channel
identificator and conf is the que quality of service of the data
channel (0x01 reliable, 0x02 streaming).
void DeletedDataChannel(object path, uint16 mdlid)
This method is called when a data channel is closed.
After this call the data channel id will not be valid and can be
reused for future created data channels.
^ permalink raw reply
* [GSoC'10] : New Command Line Tool(s) for bluez
From: Alexander Orlenko @ 2010-05-20 13:04 UTC (permalink / raw)
To: linux-bluetooth
Hi everyone,
My name is Alexander Orlenko, I am a GSoC student of this year. I am
doing a project "bluez-tools",
http://hostisdown.org/gsoc/projects/bluez-tools (a little more info
about the project). I need some feedback about what approach should I
take.
I'm from Russia, Khabarovsk. This is my first GSoC project.
^ permalink raw reply
* Re: [GSoC'10] : New Command Line Tool(s) for bluez
From: Bastien Nocera @ 2010-05-20 13:08 UTC (permalink / raw)
To: Alexander Orlenko; +Cc: linux-bluetooth
In-Reply-To: <AANLkTimnLyeZqNvNecognr_U9alZiC9bKNUOWjX9H_Hq@mail.gmail.com>
On Fri, 2010-05-21 at 00:04 +1100, Alexander Orlenko wrote:
> Hi everyone,
>
> My name is Alexander Orlenko, I am a GSoC student of this year. I am
> doing a project "bluez-tools",
> http://hostisdown.org/gsoc/projects/bluez-tools (a little more info
> about the project). I need some feedback about what approach should I
> take.
I'm not sure whether you'd be interested, but you could base your code
on the non-GTK parts of libgnome-bluetooth, which already implements a
number of those things, and would just need to be wrapped up.
Cheers
^ permalink raw reply
* Re: [PATCH v3] hciattach application support for Atheros AR300x Bluetooth Chip
From: suraj @ 2010-05-20 13:37 UTC (permalink / raw)
To: linux-bluetooth; +Cc: marcel, Luis.Rodriguez, Jothikumar.Mothilal
In-Reply-To: <1273672059.32536.14.camel@atheros013-desktop>
Hi,
On Wed, 2010-05-12 at 19:17 +0530, suraj wrote:
> Implements support for Atheros AR300x Bluetooth chip in hciattach.
> Adds feature to bring up AR300x Bluetooth chip
> with and without enhanced power management enabled.
>
> Signed-off-by: Suraj <suraj@Atheros.com>
>
> ---
> Makefile.tools | 1 +
> tools/hciattach.8 | 6 +
> tools/hciattach.c | 111 +++++
> tools/hciattach.h | 3 +
> tools/hciattach_ar3k.c | 1223 ++++++++++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 1344 insertions(+), 0 deletions(-)
> create mode 100755 tools/hciattach_ar3k.c
>
> diff --git a/Makefile.tools b/Makefile.tools
> index 2735d68..48cf097 100644
> --- a/Makefile.tools
> +++ b/Makefile.tools
> @@ -23,6 +23,7 @@ tools_l2ping_LDADD = lib/libbluetooth.la
> tools_hciattach_SOURCES = tools/hciattach.c tools/hciattach.h \
> tools/hciattach_st.c \
> tools/hciattach_ti.c \
> + tools/hciattach_ar3k.c \
> tools/hciattach_tialt.c
> tools_hciattach_LDADD = lib/libbluetooth.la
>
> diff --git a/tools/hciattach.8 b/tools/hciattach.8
> index f750222..ef943ea 100644
> --- a/tools/hciattach.8
> +++ b/tools/hciattach.8
> @@ -49,6 +49,12 @@ specific identifier. Currently supported types are
> .B any
> Unspecified HCI_UART interface, no vendor specific options
> .TP
> +.B ar3kalt
> +Atheros AR300x based serial bluetooth device with power management disabled
> +.TP
> +.B ar3k
> +Atheros AR300x based serial bluetooth device
> +.TP
> .B ericsson
> Ericsson based modules
> .TP
> diff --git a/tools/hciattach.c b/tools/hciattach.c
> index b13db1b..768a3f1 100644
> --- a/tools/hciattach.c
> +++ b/tools/hciattach.c
> @@ -653,6 +653,110 @@ static int csr(int fd, struct uart_t *u, struct termios *ti)
> }
>
> /*
> + * Atheros AR300x specific initialization post callback
> + * with power management dsiabled
> + * Suraj Sumangala <Suraj@Atheros.com>
> + */
> +static int ar3kpost(int fd, struct uart_t *u, struct termios *ti)
> +{
> + return ath_configure_sleep(fd, 0);
> +}
> +
> +/*
> + * Atheros AR300x specific initialization post callback
> + * with power management enabled
> + * Suraj Sumangala <Suraj@Atheros.com>
> + */
> +static int ar3kpmpost(int fd, struct uart_t *u, struct termios *ti)
> +{
> + return ath_configure_sleep(fd, 1);
> +}
> +/*
> + * Atheros AR300x specific initialization
> + * Suraj Sumangala <Suraj@Atheros.com>
> + */
> +static int ar3kinit(int fd, struct uart_t *u, struct termios *ti)
> +{
> + struct timespec tm = { 0, 500000 };
> + unsigned char cmd[14], rsp[100];
> + int r;
> + int baud;
> +
> + /* Download PS and patch */
> + r = ath_ps_download(fd);
> +
> + if (r < 0) {
> + perror("Failed to Download configuration");
> + return -1;
> + }
> +
> + /* Write BDADDR if user has provided any */
> + if (u->bdaddr != NULL) {
> + /* Set BD_ADDR */
> + memset(cmd, 0, sizeof(cmd));
> + memset(rsp, 0, sizeof(rsp));
> + cmd[0] = HCI_COMMAND_PKT;
> + cmd[1] = 0x0B;
> + cmd[2] = 0xfc;
> + cmd[3] = 0x0A;
> + cmd[4] = 0x01;
> + cmd[5] = 0x01;
> + cmd[6] = 0x00;
> + cmd[7] = 0x06;
> + str2ba(u->bdaddr, (bdaddr_t *) (cmd + 8));
> +
> + /* Send command */
> + if (write(fd, cmd, 14) != 14) {
> + fprintf(stderr, "Failed to write BD_ADDR command\n");
> + return -1;
> + }
> +
> + /* Read reply */
> + if (read_hci_event(fd, rsp, 10) < 0) {
> + fprintf(stderr, "Failed to set BD_ADDR\n");
> + return -1;
> + }
> + }
> +
> + /* Send HCI Reset to write the configuration */
> + cmd[0] = HCI_COMMAND_PKT;
> + cmd[1] = 0x03;
> + cmd[2] = 0x0C;
> + cmd[3] = 0x00;
> +
> + r = write(fd, cmd, 4);
> +
> + if (r != 4)
> + return -1;
> +
> + nanosleep(&tm, NULL);
> + if (read_hci_event(fd, rsp, sizeof(rsp)) < 0)
> + return -1;
> +
> + /* Set baud rate command,
> + * set controller baud rate to user specified value */
> + cmd[0] = HCI_COMMAND_PKT;
> + cmd[1] = 0x0C;
> + cmd[2] = 0xfc;
> + cmd[3] = 0x02;
> + baud = u->speed/100;
> + cmd[4] = (char)baud;
> + cmd[5] = (char)(baud >> 8);
> +
> + if (write(fd, cmd, 6) != 6) {
> + perror("Failed to write init command");
> + return -1;
> + }
> +
> + /* Wait for the command complete event for Baud rate change Command */
> + nanosleep(&tm, NULL);
> +
> + if (read_hci_event(fd, rsp, sizeof(rsp)) < 0)
> + return -1;
> +
> + return 0;
> +}
> +/*
> * Silicon Wave specific initialization
> * Thomas Moser <thomas.moser@tmoser.ch>
> */
> @@ -1071,6 +1175,13 @@ struct uart_t uart[] = {
> /* Broadcom BCM2035 */
> { "bcm2035", 0x0A5C, 0x2035, HCI_UART_H4, 115200, 460800, FLOW_CTL, NULL, bcm2035 },
>
> + /* ATHEROS AR300x */
> + { "ar3kalt", 0x0000, 0x0000, HCI_UART_ATH, 115200, 115200,
> + FLOW_CTL, NULL, ar3kinit, ar3kpost },
> +
> + { "ar3k", 0x0000, 0x0000, HCI_UART_ATH, 115200, 115200,
> + FLOW_CTL, NULL, ar3kinit, ar3kpmpost },
> +
> { NULL, 0 }
> };
>
> diff --git a/tools/hciattach.h b/tools/hciattach.h
> index 867563b..7719e04 100644
> --- a/tools/hciattach.h
> +++ b/tools/hciattach.h
> @@ -36,6 +36,7 @@
> #define HCI_UART_3WIRE 2
> #define HCI_UART_H4DS 3
> #define HCI_UART_LL 4
> +#define HCI_UART_ATH 5
>
> int read_hci_event(int fd, unsigned char* buf, int size);
> int set_speed(int fd, struct termios *ti, int speed);
> @@ -45,3 +46,5 @@ int texas_post(int fd, struct termios *ti);
> int texasalt_init(int fd, int speed, struct termios *ti);
> int stlc2500_init(int fd, bdaddr_t *bdaddr);
> int bgb2xx_init(int dd, bdaddr_t *bdaddr);
> +int ath_configure_sleep(int fd, int sleep_stat);
> +int ath_ps_download(int fd);
> diff --git a/tools/hciattach_ar3k.c b/tools/hciattach_ar3k.c
> new file mode 100755
> index 0000000..2542161
> --- /dev/null
> +++ b/tools/hciattach_ar3k.c
> @@ -0,0 +1,1223 @@
> +/*
> + * Copyright (c) 2009-2010 Atheros Communications Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> + *
> + */
> +
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +
> +#include <stdio.h>
> +#include <errno.h>
> +#include <unistd.h>
> +#include <stdlib.h>
> +#include <ctype.h>
> +#include <time.h>
> +#include <sys/time.h>
> +#include <sys/types.h>
> +#include <sys/param.h>
> +#include <sys/ioctl.h>
> +
> +#include <bluetooth/bluetooth.h>
> +#include <bluetooth/hci.h>
> +#include <bluetooth/hci_lib.h>
> +
> +#include "hciattach.h"
> +
> +/* The maximum number of bytes possible in a patch entry */
> +#define MAX_PATCH_SIZE 20000
> +
> +#define MAX_BYTE_LENGTH 244
> +
> +/* Maximum HCI packets that will be formed from the Patch file */
> +#define MAX_NUM_PATCH_ENTRY ((MAX_PATCH_SIZE/MAX_BYTE_LENGTH) + 1)
> +
> +#define DEV_REGISTER 0x4FFC
> +
> +#define FW_PATH "/lib/firmware/ar3k/"
> +
> +#define PS_ASIC_FILE "PS_ASIC.pst"
> +#define PS_FPGA_FILE "PS_FPGA.pst"
> +#define PATCH_FILE "RamPatch.txt"
> +#define BDADDR_FILE "ar3kbdaddr.pst"
> +
> +#define HCI_CMD_HEADER_LEN 7
> +
> +/* PS command types */
> +#define PS_RESET 2
> +#define PS_WRITE 1
> +#define WRITE_PATCH 8
> +#define PS_VERIFY_CRC 9
> +#define ENABLE_PATCH 11
> +
> +#define EXTRA_PATCH_SIZE 500
> +
> +/* PS configuration entry time */
> +#define PS_TYPE_HEX 0
> +#define PS_TYPE_DEC 1
> +
> +#define PS_RESET_PARAM_LEN 6
> +#define PS_RESET_CMD_LEN (PS_RESET_PARAM_LEN +\
> + HCI_CMD_HEADER_LEN)
> +
> +#define RAM_PS_REGION (1<<0)
> +#define RAM_PATCH_REGION (1<<1)
> +
> +#define RAMPS_MAX_PS_TAGS_PER_FILE 50
> +#define PS_MAX_LEN 500
> +#define LINE_SIZE_MAX (PS_MAX_LEN * 2)
> +
> +#define BYTES_OF_PS_DATA_PER_LINE 16
> +
> +#define skip_space(str) while (*(str) == (' ')) ((str)++)
> +
> +#define is_between(x, lower, upper) (((lower) <= (x)) && ((x) <= (upper)))
> +
> +#define tohexval(c) (isdigit((c)) ? ((c) - '0') : \
> + (is_between((c), 'A', 'Z') ? \
> + ((c) - 'A' + 10) : ((c) - 'a' + 10)))
> +
> +#define stringtohex(str) (((uint8_t)(tohexval((str)[0]) << 4)) |\
> + ((uint8_t)tohexval((str)[1])))
> +
> +#define set_pst_format(pst, type, array_val) ((pst)->data_type = (type),\
> + (pst)->is_array = (array_val))
> +
> +struct ps_tag_entry {
> + uint32_t tag_id;
> + uint32_t tag_len;
> + uint8_t *tag_data;
> +};
> +
> +struct ps_ram_patch {
> + int16_t Len;
> + uint8_t *Data;
> +};
> +
> +struct ps_data_format {
> + unsigned char data_type;
> + unsigned char is_array;
> +};
> +
> +struct ps_cmd_packet {
> + uint8_t *Hcipacket;
> + int packetLen;
> +};
> +
> +struct st_read_status {
> + unsigned section;
> + unsigned line_count;
> + unsigned char_cnt;
> + unsigned byte_count;
> +};
> +
> +struct ps_tag_entry ps_tag_entry[RAMPS_MAX_PS_TAGS_PER_FILE];
> +struct ps_ram_patch ram_patch[MAX_NUM_PATCH_ENTRY];
> +
> +static void load_hci_header(uint8_t *hci_ps_cmd,
> + uint8_t opcode,
> + int length,
> + int index)
> +{
> + hci_ps_cmd[0] = 0x0B;
> + hci_ps_cmd[1] = 0xFC;
> + hci_ps_cmd[2] = length + 4;
> + hci_ps_cmd[3] = opcode;
> + hci_ps_cmd[4] = (index & 0xFF);
> + hci_ps_cmd[5] = ((index >> 8) & 0xFF);
> + hci_ps_cmd[6] = length;
> +
> +}
> +
> +/*
> + *Create PS download commands from parsed data
> + */
> +static int ath_create_ps_command(uint8_t opcode,
> + uint32_t param_1,
> + struct ps_cmd_packet *ps_patch_packet,
> + uint32_t *index)
> +{
> + uint8_t *hci_ps_cmd;
> + int i;
> +
> + switch (opcode) {
> +
> + case WRITE_PATCH:
> +
> + for (i = 0; i < param_1; i++) {
> +
> + /* Allocate command buffer */
> + hci_ps_cmd = (uint8_t *) malloc(ram_patch[i].Len +
> + HCI_CMD_HEADER_LEN);
> +
> + if (!hci_ps_cmd)
> + return -ENOMEM;
> +
> + /* Update commands to buffer */
> + load_hci_header(hci_ps_cmd,
> + opcode,
> + ram_patch[i].Len,
> + i);
> + memcpy(&hci_ps_cmd[HCI_CMD_HEADER_LEN],
> + ram_patch[i].Data,
> + ram_patch[i].Len);
> +
> + ps_patch_packet[*index].Hcipacket = hci_ps_cmd;
> + ps_patch_packet[*index].packetLen = ram_patch[i].Len +
> + HCI_CMD_HEADER_LEN;
> +
> + (*index)++;
> + }
> +
> + break;
> + case ENABLE_PATCH:
> +
> + hci_ps_cmd = (uint8_t *) malloc(HCI_CMD_HEADER_LEN);
> +
> + if (!hci_ps_cmd)
> + return -ENOMEM;
> +
> + load_hci_header(hci_ps_cmd, opcode, 0, 0x00);
> + ps_patch_packet[*index].Hcipacket = hci_ps_cmd;
> + ps_patch_packet[*index].packetLen = HCI_CMD_HEADER_LEN;
> +
> + (*index)++;
> +
> + break;
> + case PS_RESET:
> +
> + hci_ps_cmd = (uint8_t *) malloc(PS_RESET_CMD_LEN);
> +
> + if (!hci_ps_cmd)
> + return -ENOMEM;
> +
> + load_hci_header(hci_ps_cmd, opcode, PS_RESET_PARAM_LEN, 0x00);
> +
> + hci_ps_cmd[7] = 0x00;
> + hci_ps_cmd[PS_RESET_CMD_LEN - 2] = (param_1 & 0xFF);
> + hci_ps_cmd[PS_RESET_CMD_LEN - 1] = ((param_1 >> 8) & 0xFF);
> +
> + ps_patch_packet[*index].Hcipacket = hci_ps_cmd;
> + ps_patch_packet[*index].packetLen = PS_RESET_CMD_LEN;
> +
> + (*index)++;
> +
> + break;
> + case PS_WRITE:
> +
> + for (i = 0; i < param_1; i++) {
> +
> + hci_ps_cmd =
> + (uint8_t *) malloc(ps_tag_entry[i].tag_len +
> + HCI_CMD_HEADER_LEN);
> + if (!hci_ps_cmd)
> + return -ENOMEM;
> +
> + load_hci_header(hci_ps_cmd,
> + opcode,
> + ps_tag_entry[i].tag_len,
> + ps_tag_entry[i].tag_id);
> +
> + memcpy(&hci_ps_cmd[HCI_CMD_HEADER_LEN],
> + ps_tag_entry[i].tag_data,
> + ps_tag_entry[i].tag_len);
> +
> + ps_patch_packet[*index].Hcipacket = hci_ps_cmd;
> +
> + ps_patch_packet[*index].packetLen =
> + ps_tag_entry[i].tag_len + HCI_CMD_HEADER_LEN;
> +
> + (*index)++;
> + }
> +
> + break;
> + default:
> + break;
> + }
> +
> + return 0;
> +}
> +
> +static int get_ps_type(char *line,
> + int eol_index,
> + unsigned char *type,
> + unsigned char *sub_type)
> +{
> +
> + switch (eol_index) {
> + case 1:
> + return 0;
> + case 2:
> + (*type) = toupper(line[1]);
> + break;
> + case 3:
> + if (line[2] == ':')
> + (*type) = toupper(line[1]);
> + else if (line[1] == ':')
> + (*sub_type) = toupper(line[2]);
> + else
> + return -1;
> +
> + break;
> + case 4:
> + if (line[2] != ':')
> + return -1;
> +
> + (*type) = toupper(line[1]);
> + (*sub_type) = toupper(line[3]);
> +
> + break;
> + case -1:
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +static int get_input_data_format(char *line, struct ps_data_format *pst_format)
> +{
> + unsigned char type = '\0';
> + unsigned char sub_type = '\0';
> + int eol_index = -1;
> + int i;
> +
> + /* The default values */
> + set_pst_format(pst_format, PS_TYPE_HEX, 1);
> +
> + if (line[0] != '[') {
> + set_pst_format(pst_format, PS_TYPE_HEX, 1);
> + return 0;
> + }
> +
> + for (i = 1; i < 5; i++) {
> + if (line[i] == ']') {
> + eol_index = i;
> + break;
> + }
> + }
> +
> + if (get_ps_type(line, eol_index, &type, &sub_type) < 0)
> + return -1;
> +
> + /* By default Hex array type is assumed */
> + if (type == '\0' && sub_type == '\0')
> + set_pst_format(pst_format, PS_TYPE_HEX, 1);
> +
> + /* Check is data type is of array */
> + if (type == 'A' || sub_type == 'A')
> + pst_format->is_array = 1;
> +
> + if (type == 'S' || sub_type == 'S')
> + pst_format->is_array = 0;
> +
> + switch (type) {
> +
> + case 'D':
> + case 'B':
> +
> + pst_format->data_type = PS_TYPE_DEC;
> + break;
> + default:
> +
> + pst_format->data_type = PS_TYPE_HEX;
> + break;
> +
> + }
> +
> + line += (eol_index + 1);
> +
> + return 0;
> +
> +}
> +
> +static unsigned int read_data_in_section(char *line,
> + struct ps_data_format format_info)
> +{
> + char *token_ptr = line;
> +
> + if (!token_ptr)
> + return 0x0FFF;
> +
> + if (token_ptr[0] == '[') {
> +
> + while (token_ptr[0] != ']' && token_ptr[0] != '\0')
> + token_ptr++;
> +
> + if (token_ptr[0] == '\0')
> + return 0x0FFF;
> +
> + token_ptr++;
> + }
> +
> + if (format_info.data_type == PS_TYPE_HEX) {
> +
> + if (format_info.is_array == 1)
> + return 0x0FFF;
> + else
> + return strtol(token_ptr, NULL, 16);
> +
> + } else
> + return 0x0FFF;
> +
> + return 0x0FFF;
> +
> +}
> +static int ath_parse_file(FILE *stream)
> +{
> + char *buffer;
> + char *line;
> + uint8_t tag_cnt = 0;
> + int16_t byte_count = 0;
> + int read_count;
> + int num_ps_entry;
> + struct ps_data_format stps_data_format;
> + struct st_read_status read_status = { 0, 0, 0, 0 };
> +
> + if (!stream) {
> + perror("Could not open config file .\n");
> + return -1;
> + }
> +
> + buffer = malloc(LINE_SIZE_MAX + 1);
> +
> + if (!buffer)
> + return -ENOMEM;
> +
> + do {
> + line = fgets(buffer, LINE_SIZE_MAX, stream);
> +
> + if (!line)
> + break;
> +
> + skip_space(line);
> +
> + if ((line[0] == '/') && (line[1] == '/'))
> + continue;
> +
> + if ((line[0] == '#')) {
> +
> + if (read_status.section != 0) {
> +
> + if (buffer)
> + free(buffer);
> + return -1;
> +
> + } else {
> + read_status.section = 1;
> + continue;
> + }
> + }
> +
> + if ((line[0] == '/') && (line[1] == '*')) {
> +
> + read_status.section = 0;
> + continue;
> + }
> +
> + if (read_status.section == 1) {
> + skip_space(line);
> +
> + if (get_input_data_format(
> + line, &stps_data_format) < 0) {
> +
> + if (buffer)
> + free(buffer);
> + return -1;
> +
> + }
> +
> + ps_tag_entry[tag_cnt].tag_id = read_data_in_section(
> + line,
> + stps_data_format);
> + read_status.section = 2;
> +
> + } else if (read_status.section == 2) {
> +
> + if (get_input_data_format(
> + line, &stps_data_format) < 0) {
> +
> + if (buffer)
> + free(buffer);
> + return -1;
> + }
> +
> + byte_count =
> + read_data_in_section(line, stps_data_format);
> +
> + read_status.section = 2;
> + if (byte_count > LINE_SIZE_MAX / 2) {
> + if (buffer)
> + free(buffer);
> +
> + return -1;
> + }
> +
> + ps_tag_entry[tag_cnt].tag_len = byte_count;
> + ps_tag_entry[tag_cnt].tag_data = (uint8_t *)
> + malloc(byte_count);
> +
> + read_status.section = 3;
> + read_status.line_count = 0;
> +
> + } else if (read_status.section == 3) {
> +
> + if (read_status.line_count == 0) {
> + if (get_input_data_format(
> + line, &stps_data_format) < 0) {
> + if (buffer)
> + free(buffer);
> + return -1;
> + }
> + }
> +
> + skip_space(line);
> + read_status.char_cnt = 0;
> +
> + if (line[read_status.char_cnt] == '[') {
> +
> + while (line[read_status.char_cnt] != ']' &&
> + line[read_status.char_cnt] != '\0')
> + read_status.char_cnt++;
> +
> + if (line[read_status.char_cnt] == ']')
> + read_status.char_cnt++;
> + else
> + read_status.char_cnt = 0;
> +
> + }
> +
> + read_count = (byte_count > BYTES_OF_PS_DATA_PER_LINE)
> + ? BYTES_OF_PS_DATA_PER_LINE : byte_count;
> +
> + if (stps_data_format.data_type == PS_TYPE_HEX &&
> + stps_data_format.is_array == 1) {
> +
> + while (read_count > 0) {
> +
> + ps_tag_entry[tag_cnt].tag_data
> + [read_status.byte_count] =
> + stringtohex(
> + &line[read_status.char_cnt]);
> +
> + ps_tag_entry[tag_cnt].tag_data
> + [read_status.byte_count + 1] =
> + stringtohex(
> + &line[read_status.char_cnt + 3]);
> +
> + read_status.char_cnt += 6;
> + read_status.byte_count += 2;
> + read_count -= 2;
> +
> + }
> +
> + if (byte_count > BYTES_OF_PS_DATA_PER_LINE)
> + byte_count -=
> + BYTES_OF_PS_DATA_PER_LINE;
> + else
> + byte_count = 0;
> + }
> +
> + read_status.line_count++;
> +
> + if (byte_count == 0) {
> + read_status.section = 0;
> + read_status.char_cnt = 0;
> + read_status.line_count = 0;
> + read_status.byte_count = 0;
> + } else
> + read_status.char_cnt = 0;
> +
> + if (read_status.section == 0) {
> + tag_cnt++;
> +
> + if (tag_cnt == RAMPS_MAX_PS_TAGS_PER_FILE) {
> + if (buffer)
> + free(buffer);
> + return -1;
> + }
> + }
> +
> + }
> +
> + } while (line);
> +
> + num_ps_entry = tag_cnt;
> +
> + if (tag_cnt > RAMPS_MAX_PS_TAGS_PER_FILE) {
> + if (buffer)
> + free(buffer);
> + return -1;
> + }
> +
> + if (buffer)
> + free(buffer);
> +
> + return num_ps_entry;
> +}
> +
> +static int parse_patch_file(FILE *stream)
> +{
> + char byte[3];
> + char line[MAX_BYTE_LENGTH + 1];
> + int byte_cnt, byte_cnt_org;
> + int patch_index;
> + int i, k;
> + int data;
> + int patch_count = 0;
> +
> + byte[2] = '\0';
> +
> + while (fgets(line, MAX_BYTE_LENGTH, stream)) {
> + if (strlen(line) <= 1 || !isxdigit(line[0]))
> + continue;
> + else
> + break;
> + }
> +
> + byte_cnt = strtol(line, NULL, 16);
> + byte_cnt_org = byte_cnt;
> +
> + while (byte_cnt > MAX_BYTE_LENGTH) {
> +
> + /* Handle case when the number of patch buffer is
> + * more than the 20K */
> + if (MAX_NUM_PATCH_ENTRY == patch_count) {
> + for (i = 0; i < patch_count; i++)
> + free(ram_patch[i].Data);
> + return -ENOMEM;
> + }
> +
> + ram_patch[patch_count].Len = MAX_BYTE_LENGTH;
> + ram_patch[patch_count].Data =
> + (uint8_t *) malloc(MAX_BYTE_LENGTH);
> +
> + if (!ram_patch[patch_count].Data)
> + return -ENOMEM;
> +
> + patch_count++;
> + byte_cnt = byte_cnt - MAX_BYTE_LENGTH;
> + }
> +
> + ram_patch[patch_count].Len = (byte_cnt & 0xFF);
> +
> + if (byte_cnt != 0) {
> + ram_patch[patch_count].Data = (uint8_t *) malloc(byte_cnt);
> +
> + if (!ram_patch[patch_count].Data)
> + return -ENOMEM;
> + patch_count++;
> + }
> +
> + while (byte_cnt_org > MAX_BYTE_LENGTH) {
> +
> + k = 0;
> + for (i = 0; i < MAX_BYTE_LENGTH * 2; i += 2) {
> + if (!fgets(byte, 3, stream))
> + return -1;
> + data = strtoul(&byte[0], NULL, 16);
> + ram_patch[patch_index].Data[k] = (data & 0xFF);
> +
> + k++;
> + }
> +
> + patch_index++;
> +
> + byte_cnt_org = byte_cnt_org - MAX_BYTE_LENGTH;
> + }
> +
> + if (patch_index == 0)
> + patch_index++;
> +
> + for (k = 0; k < byte_cnt_org; k++) {
> +
> + if (!fgets(byte, 3, stream))
> + return -1;
> +
> + data = strtoul(byte, NULL, 16);
> + ram_patch[patch_index].Data[k] = (data & 0xFF);
> + }
> +
> + return patch_count;
> +}
> +
> +static int ath_parse_ps(FILE *stream, int *total_tag_len)
> +{
> + int num_ps_tags;
> + int i;
> + unsigned char bdaddr_present = 0;
> +
> + if (stream)
> + num_ps_tags = ath_parse_file(stream);
> +
> + if (num_ps_tags < 0)
> + return -1;
> +
> + if (num_ps_tags == 0)
> + *total_tag_len = 10;
> + else {
> +
> + for (i = 0; i < num_ps_tags; i++) {
> +
> + if (ps_tag_entry[i].tag_id == 1)
> + bdaddr_present = 1;
> + if (ps_tag_entry[i].tag_len % 2 == 1)
> + *total_tag_len = *total_tag_len
> + + ps_tag_entry[i].tag_len + 1;
> + else
> + *total_tag_len =
> + *total_tag_len + ps_tag_entry[i].tag_len;
> +
> + }
> + }
> +
> + if (num_ps_tags > 0 && !bdaddr_present)
> + *total_tag_len = *total_tag_len + 10;
> +
> + *total_tag_len = *total_tag_len + 10 + (num_ps_tags * 4);
> +
> + return num_ps_tags;
> +}
> +
> +static int ath_create_cmd_list(struct ps_cmd_packet **hci_packet_list,
> + uint32_t *num_packets,
> + int tag_count,
> + int patch_count,
> + int total_tag_len)
> +{
> + uint8_t count;
> + uint32_t num_cmd_entry = 0;
> +
> + *num_packets = 0;
> +
> + if (patch_count || tag_count) {
> +
> + /* PS Reset Packet + Patch List + PS List */
> + num_cmd_entry += (1 + patch_count + tag_count);
> + if (patch_count > 0)
> + num_cmd_entry++; /* Patch Enable Command */
> +
> + (*hci_packet_list) =
> + malloc(sizeof(struct ps_cmd_packet) * num_cmd_entry);
> +
> + if (!(*hci_packet_list))
> + return -ENOMEM;
> +
> + if (patch_count > 0) {
> +
> + if (ath_create_ps_command(WRITE_PATCH, patch_count,
> + *hci_packet_list, num_packets) < 0)
> + return -1;
> + if (ath_create_ps_command(ENABLE_PATCH, 0,
> + *hci_packet_list, num_packets) < 0)
> + return -1;
> + if (ath_create_ps_command(PS_RESET,
> + total_tag_len + EXTRA_PATCH_SIZE,
> + *hci_packet_list, num_packets) < 0)
> + return -1;
> +
> + } else {
> +
> + if (ath_create_ps_command(PS_RESET, total_tag_len,
> + *hci_packet_list, num_packets) < 0)
> + return -1;
> + }
> +
> + if (tag_count > 0)
> + ath_create_ps_command(PS_WRITE, tag_count,
> + *hci_packet_list, num_packets);
> + }
> +
> + for (count = 0; count < patch_count; count++)
> + free(ram_patch[patch_count].Data);
> +
> + for (count = 0; count < tag_count; count++)
> + free(ps_tag_entry[count].tag_data);
> +
> + return *num_packets;
> +}
> +
> +static int ath_free_command_list(struct ps_cmd_packet **hci_packet_list,
> + uint32_t num_packets)
> +{
> + int i;
> +
> + if (!(*hci_packet_list))
> + return -1;
> +
> + for (i = 0; i < num_packets; i++)
> + free((*hci_packet_list)[i].Hcipacket);
> +
> + free(*hci_packet_list);
> +
> + return 0;
> +}
> +
> +/*
> + * This API is used to send the HCI command to controller and return
> + * with a HCI Command Complete event.
> + */
> +static int send_hci_cmd_wait_event(int dev,
> + uint8_t *hci_command,
> + int cmd_length,
> + uint8_t **event, uint8_t **buffer_to_free)
> +{
> + int r;
> + uint8_t *hci_event;
> + uint8_t pkt_type = 0x01;
> +
> + if (cmd_length == 0)
> + return -1;
> +
> + if (write(dev, &pkt_type, 1) != 1)
> + return -1;
> +
> + if (write(dev, (unsigned char *)hci_command, cmd_length) != cmd_length)
> + return -1;
> +
> + hci_event = (uint8_t *) malloc(100);
> +
> + if (!hci_event)
> + return -ENOMEM;
> +
> + r = read_hci_event(dev, (unsigned char *)hci_event, 100);
> +
> + if (r > 0) {
> + *event = hci_event;
> + *buffer_to_free = hci_event;
> + } else {
> +
> + /* Did not get an event from controller. return error */
> + free(hci_event);
> + *buffer_to_free = NULL;
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +static int read_ps_event(uint8_t *data)
> +{
> +
> + if (data[5] == 0xFC && data[6] == 0x00) {
> +
> + switch (data[4]) {
> +
> + case 0x0B:/* CRC Check */
> + case 0x0C:/* Change Baudrate */
> + case 0x04:/* Enable sleep */
> + return 0;
> + break;
> + default:
> + return -1;
> + break;
> +
> + }
> + }
> +
> + return -1;
> +}
> +
> +static int get_ps_file_name(int devtype, int rom_version, char *path)
> +{
> + char *filename;
> + int status = 0;
> +
> + if (devtype == 0xdeadc0de) {
> + filename = PS_ASIC_FILE;
> + status = 1;
> + } else {
> + filename = PS_FPGA_FILE;
> + status = 0;
> + }
> +
> + sprintf(path, "%s%x/%s", FW_PATH, rom_version, filename);
> +
> + return status;
> +}
> +
> +static int get_patch_file_name(int dev_type, int rom_version,
> + int build_version, char *path)
> +{
> +
> + if ((dev_type != 0) &&
> + (dev_type != 0xdeadc0de) &&
> + (rom_version == 0x99999999) &&
> + (build_version == 1))
> + path[0] = '\0';
> + else
> + sprintf(path, "%s%x/%s", FW_PATH, rom_version, PATCH_FILE);
> +
> + return 0;
> +}
> +static int get_ar3k_crc(int dev, int tag_count, int patch_count)
> +{
> + uint8_t hci_cmd[7];
> + uint8_t *event;
> + uint8_t *buffer_to_free = NULL;
> + int retval = 1;
> + int crc;
> +
> + if (patch_count > 0)
> + crc |= RAM_PATCH_REGION;
> + if (tag_count > 0)
> + crc |= RAM_PS_REGION;
> +
> + load_hci_header(hci_cmd, PS_VERIFY_CRC, 0, crc);
> +
> + if (send_hci_cmd_wait_event(dev,
> + hci_cmd,
> + sizeof(hci_cmd),
> + &event,
> + &buffer_to_free) == 0) {
> +
> + if (read_ps_event(event) == 0)
> + retval = -1;
> +
> + if (buffer_to_free != NULL)
> + free(buffer_to_free);
> + }
> +
> + return retval;
> +}
> +static int get_device_type(int dev, uint32_t *code)
> +{
> + uint8_t hci_cmd[] = { 0x05, 0xfc, 0x05, 0x00, 0x00, 0x00, 0x00, 0x04 };
> + uint8_t *event;
> + uint8_t *buffer_to_free = NULL;
> + uint32_t reg;
> + int result = -1;
> +
> + *code = 0;
> +
> + hci_cmd[3] = (uint8_t) (DEV_REGISTER & 0xFF);
> + hci_cmd[4] = (uint8_t) ((DEV_REGISTER >> 8) & 0xFF);
> + hci_cmd[5] = (uint8_t) ((DEV_REGISTER >> 16) & 0xFF);
> + hci_cmd[6] = (uint8_t) ((DEV_REGISTER >> 24) & 0xFF);
> +
> + if (send_hci_cmd_wait_event(dev,
> + hci_cmd,
> + sizeof(hci_cmd),
> + &event,
> + &buffer_to_free) == 0) {
> +
> + if (event[5] == 0xFC && event[6] == 0x00) {
> +
> + switch (event[4]) {
> + case 0x05:
> + reg = event[10];
> + reg = ((reg << 8) | event[9]);
> + reg = ((reg << 8) | event[8]);
> + reg = ((reg << 8) | event[7]);
> + *code = reg;
> + result = 0;
> +
> + break;
> +
> + case 0x06:
> + break;
> + }
> + }
> + }
> +
> + if (buffer_to_free)
> + free(buffer_to_free);
> +
> + return result;
> +}
> +
> +static int read_ar3k_version(int pConfig, int *rom_version, int *build_version)
> +{
> + uint8_t hci_cmd[] = { 0x1E, 0xfc, 0x00 };
> + uint8_t *event;
> + uint8_t *buffer_to_free = NULL;
> + int result = -1;
> +
> + if (send_hci_cmd_wait_event(pConfig,
> + hci_cmd,
> + sizeof(hci_cmd),
> + &event,
> + &buffer_to_free) == 0) {
> +
> + if (event[5] == 0xFC &&
> + event[6] == 0x00 &&
> + event[4] == 0x1E) {
> +
> + (*rom_version) = event[10];
> + (*rom_version) = (((*rom_version) << 8) | event[9]);
> + (*rom_version) = (((*rom_version) << 8) | event[8]);
> + (*rom_version) = (((*rom_version) << 8) | event[7]);
> +
> + (*build_version) = event[14];
> + (*build_version) = (((*build_version) << 8) |
> + event[13]);
> + (*build_version) = (((*build_version) << 8) |
> + event[12]);
> + (*build_version) = (((*build_version) << 8) |
> + event[11]);
> +
> + result = 1;
> +
> + }
> +
> + if (buffer_to_free)
> + free(buffer_to_free);
> + }
> +
> + return result;
> +}
> +
> +static int str2bdaddr(char *str_bdaddr, char *bdaddr)
> +{
> + char bdbyte[3];
> + char *str_byte = str_bdaddr;
> + int i, j;
> + unsigned char colon_present = 0;
> +
> + if (strstr(str_bdaddr, ":"))
> + colon_present = 1;
> +
> + bdbyte[2] = '\0';
> +
> + for (i = 0, j = 5; i < 6; i++, j--) {
> +
> + bdbyte[0] = str_byte[0];
> + bdbyte[1] = str_byte[1];
> + bdaddr[j] = strtol(bdbyte, NULL, 16);
> +
> + if (colon_present == 1)
> + str_byte += 3;
> + else
> + str_byte += 2;
> + }
> + return 0;
> +}
> +
> +static int write_bdaddr(int pConfig, char *bdaddr)
> +{
> + uint8_t bdaddr_cmd[] = { 0x0B, 0xFC, 0x0A,
> + 0x01, 0x01, 0x00,
> + 0x06, 0x00, 0x00,
> + 0x00, 0x00, 0x00,
> + 0x00 };
> + uint8_t *event;
> + uint8_t *buffer_to_free = NULL;
> + int result = -1;
> +
> + str2bdaddr(bdaddr, (char *)&bdaddr_cmd[7]);
> +
> + if (send_hci_cmd_wait_event(pConfig,
> + bdaddr_cmd,
> + sizeof(bdaddr_cmd),
> + &event,
> + &buffer_to_free) == 0) {
> +
> + if (event[5] == 0xFC && event[6] == 0x00) {
> + if (event[4] == 0x0B)
> + result = 0;
> + }
> +
> + }
> +
> + if (buffer_to_free)
> + free(buffer_to_free);
> +
> + return result;
> +}
> +
> +int ath_configure_sleep(int fd, int sleep_stat)
> +{
> + int dev_id, dd;
> + struct timespec tm = {0, 50000};
> +
> + dev_id = ioctl(fd, HCIUARTGETDEVICE, 0);
> +
> + if (dev_id < 0) {
> + perror("cannot get device id");
> + return -1;
> + }
> +
> + dd = hci_open_dev(dev_id);
> +
> + if (dd < 0) {
> + perror("HCI device open failed");
> + return -1;
> + }
> +
> + sleep(2);
> +
> + /* send vendor specific command with Sleep feature Enabled */
> + if (hci_send_cmd(dd, OGF_VENDOR_CMD, 0x04, 1, &sleep_stat) < 0)
> + perror("Power management Disabled");
> +
> + nanosleep(&tm, NULL);
> + hci_close_dev(dd);
> +
> + return 0;
> +}
> +
> +int ath_ps_download(int hdev)
> +{
> + int i;
> + int status = 0;
> + int tag_count;
> + int patch_count;
> + int total_tag_len = 0;
> + int rom_version = 0, build_version = 0;
> +
> + struct ps_cmd_packet *hci_cmd_list; /* List storing the commands */
> + uint32_t num_cmds;
> + uint8_t *event;
> + uint8_t *buffer_to_free;
> + uint32_t dev_type;
> +
> + char patch_file[PATH_MAX];
> + char ps_file[PATH_MAX];
> + char bdaddr_file[PATH_MAX];
> +
> + FILE *stream;
> + char bdaddr[21];
> +
> + hci_cmd_list = NULL;
> +
> + /*
> + * Verfiy firmware version. depending on it select the PS
> + * config file to download.
> + */
> + if (get_device_type(hdev, &dev_type) == -1) {
> + status = -1;
> + goto download_cmplete;
> + }
> + if (read_ar3k_version(hdev, &rom_version, &build_version) == -1) {
> + status = -1;
> + goto download_cmplete;
> + }
> +
> + get_ps_file_name(dev_type, rom_version, ps_file);
> +
> + get_patch_file_name(dev_type, rom_version, build_version, patch_file);
> +
> + /* Read the PS file to a dynamically allocated buffer */
> + stream = fopen(ps_file, "r");
> +
> + if (!stream) {
> + perror("firmware file open error\n");
> + status = -1;
> + goto download_cmplete;
> + }
> +
> + tag_count = ath_parse_ps(stream, &total_tag_len);
> +
> + fclose(stream);
> +
> + if (tag_count == -1) {
> + status = -1;
> + goto download_cmplete;
> + }
> +
> + /*
> + * It is not necessary that Patch file be available,
> + * continue with PS Operations if.
> + * failed.
> + */
> + if (patch_file[0] == '\0')
> + status = 0;
> +
> + stream = fopen(patch_file, "r");
> +
> + if (!stream) {
> + patch_count = 0;
> + status = 0;
> + } else {
> + /* parse and store the Patch file contents to
> + * a global variables
> + */
> + patch_count = parse_patch_file(stream);
> +
> + fclose(stream);
> +
> + if (patch_count < 0) {
> + status = -1;
> + goto download_cmplete;
> + }
> + }
> +
> + /*
> + * Send the CRC packet,
> + * Continue with the PS operations
> + * only if the CRC check failed
> + */
> + if (get_ar3k_crc(hdev, tag_count, patch_count) < 0) {
> + status = 0;
> + goto download_cmplete;
> + }
> +
> + /* Create an HCI command list
> + * from the parsed PS and patch information
> + */
> + ath_create_cmd_list(&hci_cmd_list,
> + &num_cmds,
> + tag_count,
> + patch_count,
> + total_tag_len);
> +
> + for (i = 0; i < num_cmds; i++) {
> +
> + if (send_hci_cmd_wait_event
> + (hdev,
> + hci_cmd_list[i].Hcipacket,
> + hci_cmd_list[i].packetLen,
> + &event,
> + &buffer_to_free) == 0) {
> +
> + if (read_ps_event(event) < 0) {
> +
> + /* Exit if the status is not success */
> + if (buffer_to_free)
> + free(buffer_to_free);
> +
> + status = -1;
> + goto download_cmplete;
> + }
> + if (buffer_to_free)
> + free(buffer_to_free);
> + } else {
> + status = 0;
> + goto download_cmplete;
> + }
> + }
> + /* Read the PS file to a dynamically allocated buffer */
> + sprintf(bdaddr_file, "%s%x/%s", FW_PATH, rom_version, BDADDR_FILE);
> +
> + stream = fopen(bdaddr_file, "r");
> +
> + if (!stream) {
> + status = 0;
> + goto download_cmplete;
> + }
> +
> + if (fgets(bdaddr, 20, stream))
> + status = write_bdaddr(hdev, bdaddr);
> +
> + fclose(stream);
> +
> +download_cmplete:
> +
> + if (hci_cmd_list)
> + ath_free_command_list(&hci_cmd_list, num_cmds);
> +
> + return status;
> +}
Please spend some time to verify the patch so that I can take it to the
next level.
Regards
Suraj
^ permalink raw reply
* Re: [PATCH] Fix device_match_pattern function
From: Santiago Carot-Nemesio @ 2010-05-20 14:04 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: Santiago Carot-Nemesio, linux-bluetooth
In-Reply-To: <AANLkTimoISCAwCfv8zktLF7OxT0aT5glDAXYeFM0mUvF@mail.gmail.com>
Hello Luiz,
El jue, 20-05-2010 a las 15:00 +0200, Luiz Augusto von Dentz escribió:
> Hi,
>
> On Thu, May 20, 2010 at 2:52 PM, Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> > Hi Santiago,
> >
> > On Thu, May 20, 2010 at 12:09 PM, Santiago Carot-Nemesio
> > <sancane@gmail.com> wrote:
> >> This patch fixes a problem adding uuids to list provided when a
> >> driver is probed. Without this patch the same uuids were added
> >> more than once to list and if two or more uuids were in the
> >> same class id list only the first one was included repeatedly
> >> ---
> >> src/device.c | 2 +-
> >> 1 files changed, 1 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/src/device.c b/src/device.c
> >> index 6ba1612..d0768ce 100644
> >> --- a/src/device.c
> >> +++ b/src/device.c
> >> @@ -1053,7 +1053,7 @@ static GSList *device_match_pattern(struct btd_device *device,
> >> continue;
> >>
> >> if (record_has_uuid(rec, match_uuid))
> >> - uuids = g_slist_append(uuids, profile_uuid);
> >> + uuids = g_slist_append(uuids, match_uuid);
> >> }
> >>
> >> return uuids;
> >
> >
> > It doesn't look right, if we do that the device will be probed by the
> > matched uuid which would not happen to have a record in the storage.
> > So in other words the list of uuids you get in the probe may not match
> > with the one present in the drivers .uuids. Also this would probably
> > break serial driver, did you tried this before submitting this to the
> > list?
>
> Ok, I did apply you change to see what it happens, first it didn't
> compile, but anyway I add cast just to make sure what would gonna
> happen:
>
Sorry, you are right I sent an erroneous patch, please excuse me that
was quite a slip-up. I'll revise it more in deep for next time. In any
case, it seems that there is a problem retrieving the service class ID
List when there are more than one entry. At current moment only the
first one is being provided when driver is probed. Please, note that I'm
speaking about profiles like HDP that can registry one or two entries
(Sink, Source or both if it is playing a dual role) although other else
profiles could want to do that too.
I'll revise that issue more in deep if you don't have any objection.
Sorry for the inconvenience and thank for your time and your attention.
Best regards.
^ permalink raw reply
* Re: [PATCH] Kernel crash when krfcomm is preempted by l2cap tasklet
From: Andrei Emeltchenko @ 2010-05-20 14:13 UTC (permalink / raw)
To: Gustavo F. Padovan; +Cc: Bluettooth Linux, marcel
In-Reply-To: <AANLkTilyAbXWIHHB1Ae7GP6xlH7zdzd1QZQzYP-NiMjV@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 691 bytes --]
Hi All,
I have done second experimental version of the patch.
Main difference is that when sk is owned by user I do not run
"l2cap_chan_del" example below:
+ /* don't delete l2cap channel if sk is owned by user */
+ if (sock_owned_by_user(sk)) {
+ sk->sk_state = BT_DISCONN;
+ l2cap_sock_clear_timer(sk);
+ l2cap_sock_set_timer(sk, HZ);
+ bh_unlock_sock(sk);
+ return 0;
+ }
I start timer which will run this function. I do also the same check
in timer context.
+ if (sock_owned_by_user(sk)) {
+ /* sk is owned by user. Try again later */
+ l2cap_sock_set_timer(sk, HZ * 2);
+ bh_unlock_sock(sk);
+ sock_put(sk);
+ return;
+ }
Please give me your comments.
Regards,
Andrei
[-- Attachment #2: 0001-Bluetooth-Check-sk-is-not-owned-before-freeing-l2cap.patch --]
[-- Type: text/x-patch, Size: 2911 bytes --]
From e0ab5b30d3c507224043f065c1c227f55ae29814 Mon Sep 17 00:00:00 2001
From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
Date: Mon, 17 May 2010 12:10:46 +0300
Subject: [PATCHv2 1/2] Bluetooth: Check sk is not owned before freeing l2cap_conn
Check that socket sk is not locked in user process before removing
l2cap connection handler.
krfcommd kernel thread may be preempted with l2cap tasklet which remove
l2cap_conn structure. If krfcommd is in process of sending of RFCOMM reply
(like "RFCOMM UA" reply to "RFCOMM DISC") then kernel crash happens.
...
[ 694.175933] Unable to handle kernel NULL pointer dereference at virtual address 00000000
[ 694.184936] pgd = c0004000
[ 694.187683] [00000000] *pgd=00000000
[ 694.191711] Internal error: Oops: 5 [#1] PREEMPT
[ 694.196350] last sysfs file: /sys/devices/platform/hci_h4p/firmware/hci_h4p/loading
[ 694.260375] CPU: 0 Not tainted (2.6.32.10 #1)
[ 694.265106] PC is at l2cap_sock_sendmsg+0x43c/0x73c [l2cap]
[ 694.270721] LR is at 0xd7017303
...
[ 694.525085] Backtrace:
[ 694.527587] [<bf266be0>] (l2cap_sock_sendmsg+0x0/0x73c [l2cap]) from [<c02f2cc8>] (sock_sendmsg+0xb8/0xd8)
[ 694.537292] [<c02f2c10>] (sock_sendmsg+0x0/0xd8) from [<c02f3044>] (kernel_sendmsg+0x48/0x80)
...
Modified version after comments of Gustavo F. Padovan <gustavo@padovan.org>
Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
---
net/bluetooth/l2cap.c | 25 +++++++++++++++++++++++++
1 files changed, 25 insertions(+), 0 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index bb00015..11060d6 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -2927,6 +2927,13 @@ static inline int l2cap_connect_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hd
break;
default:
+ /* don't delete l2cap channel if sk is owned by user */
+ if (sock_owned_by_user(sk)) {
+ sk->sk_state = BT_DISCONN;
+ l2cap_sock_clear_timer(sk);
+ l2cap_sock_set_timer(sk, HZ);
+ break;
+ }
l2cap_chan_del(sk, ECONNREFUSED);
break;
}
@@ -3135,6 +3142,15 @@ static inline int l2cap_disconnect_req(struct l2cap_conn *conn, struct l2cap_cmd
del_timer(&l2cap_pi(sk)->ack_timer);
}
+ /* don't delete l2cap channel if sk is owned by user */
+ if (sock_owned_by_user(sk)) {
+ sk->sk_state = BT_DISCONN;
+ l2cap_sock_clear_timer(sk);
+ l2cap_sock_set_timer(sk, HZ);
+ bh_unlock_sock(sk);
+ return 0;
+ }
+
l2cap_chan_del(sk, ECONNRESET);
bh_unlock_sock(sk);
@@ -3167,6 +3183,15 @@ static inline int l2cap_disconnect_rsp(struct l2cap_conn *conn, struct l2cap_cmd
del_timer(&l2cap_pi(sk)->ack_timer);
}
+ /* don't delete l2cap channel if sk is owned by user */
+ if (sock_owned_by_user(sk)) {
+ sk->sk_state = BT_DISCONN;
+ l2cap_sock_clear_timer(sk);
+ l2cap_sock_set_timer(sk, HZ);
+ bh_unlock_sock(sk);
+ return 0;
+ }
+
l2cap_chan_del(sk, 0);
bh_unlock_sock(sk);
--
1.7.0.4
[-- Attachment #3: 0002-Bluetooth-timer-check-sk-is-not-owned-before-freeing.patch --]
[-- Type: text/x-patch, Size: 1066 bytes --]
From 5adfee7a73883180e6b606cc46b792fbefbb6758 Mon Sep 17 00:00:00 2001
From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
Date: Thu, 20 May 2010 16:52:19 +0300
Subject: [PATCHv2 2/2] Bluetooth: timer check sk is not owned before freeing
In timer context we might delete l2cap channel used by krfcommd.
The check makes sure that sk is not owned.
Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
---
net/bluetooth/l2cap.c | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 11060d6..19d2fc1 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -93,6 +93,14 @@ static void l2cap_sock_timeout(unsigned long arg)
bh_lock_sock(sk);
+ if (sock_owned_by_user(sk)) {
+ /* sk is owned by user. Try again later */
+ l2cap_sock_set_timer(sk, HZ * 2);
+ bh_unlock_sock(sk);
+ sock_put(sk);
+ return;
+ }
+
if (sk->sk_state == BT_CONNECTED || sk->sk_state == BT_CONFIG)
reason = ECONNREFUSED;
else if (sk->sk_state == BT_CONNECT &&
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH] Kernel crash when krfcomm is preempted by l2cap tasklet
From: Andrei Emeltchenko @ 2010-05-20 14:40 UTC (permalink / raw)
To: Gustavo F. Padovan; +Cc: Bluettooth Linux, marcel
In-Reply-To: <AANLkTinhPeE2hkL_3j_vwOYmRE_kFauO5EYv0Q9_II0r@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1048 bytes --]
On Thu, May 20, 2010 at 5:13 PM, Andrei Emeltchenko
<andrei.emeltchenko.news@gmail.com> wrote:
> Hi All,
>
> I have done second experimental version of the patch.
>
> Main difference is that when sk is owned by user I do not run
> "l2cap_chan_del" example below:
>
> + /* don't delete l2cap channel if sk is owned by user */
> + if (sock_owned_by_user(sk)) {
> + sk->sk_state = BT_DISCONN;
> + l2cap_sock_clear_timer(sk);
> + l2cap_sock_set_timer(sk, HZ);
> + bh_unlock_sock(sk);
> + return 0;
> + }
>
> I start timer which will run this function. I do also the same check
> in timer context.
>
> + if (sock_owned_by_user(sk)) {
> + /* sk is owned by user. Try again later */
> + l2cap_sock_set_timer(sk, HZ * 2);
> + bh_unlock_sock(sk);
> + sock_put(sk);
> + return;
> + }
Little modification to the second patch attached.
-- Regards, Andrei
[-- Attachment #2: 0002-Bluetooth-timer-check-sk-is-not-owned-before-freeing.patch --]
[-- Type: text/x-patch, Size: 2241 bytes --]
From 99f3c96b432c2732dce8d84155a8be346e633a56 Mon Sep 17 00:00:00 2001
From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
Date: Thu, 20 May 2010 16:52:19 +0300
Subject: [PATCHv2 2/2] Bluetooth: timer check sk is not owned before freeing
In timer context we might delete l2cap channel used by krfcommd.
The check makes sure that sk is not owned.
Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
---
net/bluetooth/l2cap.c | 32 ++++++++++++++++++++------------
1 files changed, 20 insertions(+), 12 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 11060d6..1a6c5bb 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -84,6 +84,18 @@ static struct sk_buff *l2cap_build_cmd(struct l2cap_conn *conn,
u8 code, u8 ident, u16 dlen, void *data);
/* ---- L2CAP timers ---- */
+static void l2cap_sock_set_timer(struct sock *sk, long timeout)
+{
+ BT_DBG("sk %p state %d timeout %ld", sk, sk->sk_state, timeout);
+ sk_reset_timer(sk, &sk->sk_timer, jiffies + timeout);
+}
+
+static void l2cap_sock_clear_timer(struct sock *sk)
+{
+ BT_DBG("sock %p state %d", sk, sk->sk_state);
+ sk_stop_timer(sk, &sk->sk_timer);
+}
+
static void l2cap_sock_timeout(unsigned long arg)
{
struct sock *sk = (struct sock *) arg;
@@ -93,6 +105,14 @@ static void l2cap_sock_timeout(unsigned long arg)
bh_lock_sock(sk);
+ if (sock_owned_by_user(sk)) {
+ /* sk is owned by user. Try again later */
+ l2cap_sock_set_timer(sk, HZ * 2);
+ bh_unlock_sock(sk);
+ sock_put(sk);
+ return;
+ }
+
if (sk->sk_state == BT_CONNECTED || sk->sk_state == BT_CONFIG)
reason = ECONNREFUSED;
else if (sk->sk_state == BT_CONNECT &&
@@ -109,18 +129,6 @@ static void l2cap_sock_timeout(unsigned long arg)
sock_put(sk);
}
-static void l2cap_sock_set_timer(struct sock *sk, long timeout)
-{
- BT_DBG("sk %p state %d timeout %ld", sk, sk->sk_state, timeout);
- sk_reset_timer(sk, &sk->sk_timer, jiffies + timeout);
-}
-
-static void l2cap_sock_clear_timer(struct sock *sk)
-{
- BT_DBG("sock %p state %d", sk, sk->sk_state);
- sk_stop_timer(sk, &sk->sk_timer);
-}
-
/* ---- L2CAP channels ---- */
static struct sock *__l2cap_get_chan_by_dcid(struct l2cap_chan_list *l, u16 cid)
{
--
1.7.0.4
^ permalink raw reply related
* Re: + btusb-patch-add_apple_macbookpro62.patch added to -mm tree
From: Marcel Holtmann @ 2010-05-20 15:50 UTC (permalink / raw)
To: Bernhard Froemel; +Cc: akpm, mm-commits, linux-bluetooth
In-Reply-To: <4BF50114.2000108@vmars.tuwien.ac.at>
Hi Bernhard,
> >> usbfs disappears on recent kernels [1], at least for desktop machines
> >> (and most certainly on my stock distribution). Don't want to be contra
> >> productive here, but if you insist on this "proof" (i.e. don't accept
> >> the output of the above lsusb command or my forum discussion reference
> >> [2] as proof enough) and want me to spend another hour compiling a
> >> kernel with enabled usbfs for such a small fix, then I'll pass on this
> >> one for now.
> >>
> > just run the attached script. It does the same thing. Or you could just
> > mount usbfs.
> >
> There is *no* usbfs available in my current kernel - I'd need to enable
> and recompile: that's why I can't mount. Distribution vendors (like
> Ubuntu) follow the recommendation concerning usbfs support in their
> stock kernels (it's marked deprecated).
>
> Your script does not work on devices attached to (internal) hubs - they
> are silently omitted. I modified it accordingly and that's the output:
> > T: Bus=1 Dev#=8 Spd=12 MxCh=0
> > D: Ver=2.00 Cls=ff(vendr) Sub=01 Prot=01 MxPS=64 #Cfgs=1
> > P: Vendor=05ac ProdID=8218 Rev=0.22
> > S: Manufacturer=Apple Inc.
> > S: Product=Bluetooth USB Host Controller
> > C: #Ifs=4 Cfg#=1 Atr=e0 MxPwr=0mA
> > E: Ad=00(both) Atr=00(Control) MxPS=0040 Ivl=0ms
> > I: If#=00 Alt=0 #EPs=03 Cls=ff(vendr) Sub=01 Prot=01 Driver=btusb
> > I: If#=01 Alt=0 #EPs=02 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
> > I: If#=02 Alt=0 #EPs=02 Cls=ff(vendr) Sub=ff Prot=ff Driver=(none)
> > I: If#=03 Alt=0 #EPs=00 Cls=fe(apspe) Sub=01 Prot=01 Driver=(none)
> As visible here and in my previously quoted lsusb output the class isn't
> set correctly (0xff instead of 0xe0), so the driver won't detect the
> device. It does here, because - of course - I run the patched btusb module.
> When I remove my patched DKMS module (and use the stock module as
> shipped in 2.6.32), the output is:
> > T: Bus=1 Dev#=8 Spd=12 MxCh=0
> > D: Ver=2.00 Cls=ff(vendr) Sub=01 Prot=01 MxPS=64 #Cfgs=1
> > P: Vendor=05ac ProdID=8218 Rev=0.22
> > S: Manufacturer=Apple Inc.
> > S: Product=Bluetooth USB Host Controller
> > C: #Ifs=4 Cfg#=1 Atr=e0 MxPwr=0mA
> > E: Ad=00(both) Atr=00(Control) MxPS=0040 Ivl=0ms
> > I: If#=00 Alt=0 #EPs=03 Cls=ff(vendr) Sub=01 Prot=01 Driver=(none)
> > I: If#=01 Alt=0 #EPs=02 Cls=e0(wlcon) Sub=01 Prot=01 Driver=(none)
> > I: If#=02 Alt=0 #EPs=02 Cls=ff(vendr) Sub=ff Prot=ff Driver=(none)
> > I: If#=03 Alt=0 #EPs=00 Cls=fe(apspe) Sub=01 Prot=01 Driver=(none)
> I hope you're finally convinced, so attached once more the patch (and CC
> to linux-bluetooth).
this is really stupid from Apple. They must have been smoking something
nice when doing this change.
So what I need is a patch done with git format-patch that contains the
explanation why including the output from above.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH v3] hciattach application support for Atheros AR300x Bluetooth Chip
From: Marcel Holtmann @ 2010-05-20 16:00 UTC (permalink / raw)
To: suraj; +Cc: linux-bluetooth, Luis.Rodriguez, Jothikumar.Mothilal
In-Reply-To: <1273672059.32536.14.camel@atheros013-desktop>
Hi Suraj,
> Implements support for Atheros AR300x Bluetooth chip in hciattach.
> Adds feature to bring up AR300x Bluetooth chip
> with and without enhanced power management enabled.
>
> Signed-off-by: Suraj <suraj@Atheros.com>
not SOBs for BlueZ userspace. That is kernel only.
> ---
> Makefile.tools | 1 +
> tools/hciattach.8 | 6 +
> tools/hciattach.c | 111 +++++
> tools/hciattach.h | 3 +
> tools/hciattach_ar3k.c | 1223 ++++++++++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 1344 insertions(+), 0 deletions(-)
> create mode 100755 tools/hciattach_ar3k.c
While hciattach is kinda the bad sheep inside BlueZ, I will enforce the
general coding style for it for all new contributions. So could you
please fix that one first. Otherwise it makes no sense for me to go
through such a big patch if I have to complain about the coding style
breakage for every second line. A general rule is if it fails
check-patch.pl from the kernel, then you are doing something wrong.
Regards
Marcel
^ permalink raw reply
* Re: buffer starvation with multiple ACL link
From: Marcel Holtmann @ 2010-05-20 16:02 UTC (permalink / raw)
To: suraj; +Cc: linux-bluetooth, Luis.Rodriguez, Jothikumar.Mothilal
In-Reply-To: <1273131932.2229.9.camel@atheros013-desktop>
Hi Suraj,
please never start a new thread that hangs of an existing thread. I do
will miss these and ignore. You need to fix your mailer setup.
> I am seeing a strange issue with multiple ACL connection with CSR chip.
>
> I am having 2 ACL link, one with FTP going on and another streaming
> A2DP.
> The moment the A2DP link goes out of range ,I see that the FTP also
> stops. It resumes as soon as A2DP comes back in range.
>
> On further analysis I could verify that since the A2DP link is
> blocked( due to remote being OoR) the controller's ACL buffers with A2DP
> data are stuck.
> But at the host level this is not considered and Bluez keeps sending
> A2DP data until it blocks all the ACL buffers which in turns blocks FTP
> also.
>
> I was expecting controller to flush these packets sometime and free the
> A2DP buffers. Not sure, what is the default Flush timeout value.
We don't have anything as of now that could fix this. I would need more
details on this since it might be some outdated firmware or known bug in
the controller. Please start a proper new thread with details about the
hardware you are using and which software versions.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH v5] Add support for the Atheros AR300x Bluetooth Chip
From: Marcel Holtmann @ 2010-05-20 16:09 UTC (permalink / raw)
To: suraj; +Cc: linux-bluetooth, Luis.Rodriguez, Jothikumar.Mothilal, gfpadovan
In-Reply-To: <1271927414.1409.3.camel@atheros013-desktop>
Hi Suraj,
for every new patch version, please start a new thread. If you re-use an
old thread, I will most likely ignore it.
> +static struct sk_buff *ath_dequeue(struct hci_uart *hu)
> +{
> + struct ath_struct *ath = hu->priv;
> + struct sk_buff *skbuf;
> +
> + skbuf = skb_dequeue(&ath->txq);
> +
> + if (!skbuf)
> + return NULL;
> +
> + /*
> + * Check if the HCI command is HCI sleep enable and
> + * update the sleep enable flag with command parameter.
> + *
> + * Value of sleep enable flag will be used later
> + * to verify if controller has to be woken up before
> + * sending any packet.
> + */
> + if (skbuf->data[0] == 0x01 &&
> + skbuf->data[1] == 0x04 &&
> + skbuf->data[2] == 0xFC)
> + ath->cur_sleep = skbuf->data[4];
> +
> + return skbuf;
> +}
I don't really like this. I know we don't have any infrastructure to
forward vendor commands back to the driver. And we might need to change
that actually.
However at least for now use the headers and constants and not magic
numbers here.
> +static void ath_check_data_len(struct ath_struct *ath, int len)
> +{
> + int room = skb_tailroom(ath->rx_skb);
> +
> + BT_DBG("len %d room %d", len, room);
> +
> + if (len > room) {
> + BT_ERR("Data length is too large");
> + kfree_skb(ath->rx_skb);
> + ath->rx_state = HCIATH_W4_PACKET_TYPE;
> + ath->rx_skb = NULL;
> + ath->rx_count = 0;
> + } else {
> + ath->rx_state = HCIATH_W4_DATA;
> + ath->rx_count = len;
> + }
> +}
> +
> +/* Recv data */
> +static int ath_recv(struct hci_uart *hu, void *data, int count)
> +{
> + struct ath_struct *ath = hu->priv;
> + char *ptr = data;
> + struct hci_event_hdr *eh;
> + struct hci_acl_hdr *ah;
> + struct hci_sco_hdr *sh;
> + int len, type, dlen;
> +
> + BT_DBG("hu %p count %d rx_state %d rx_count %d", hu, count,
> + ath->rx_state, ath->rx_count);
> +
> + while (count) {
> + if (ath->rx_count) {
> +
> + len = min_t(unsigned int, ath->rx_count, count);
> + memcpy(skb_put(ath->rx_skb, len), ptr, len);
> + ath->rx_count -= len;
> + count -= len;
> + ptr += len;
> +
> + if (ath->rx_count)
> + continue;
> + switch (ath->rx_state) {
> + case HCIATH_W4_DATA:
> + hci_recv_frame(ath->rx_skb);
> + ath->rx_state = HCIATH_W4_PACKET_TYPE;
> + ath->rx_skb = NULL;
> + ath->rx_count = 0;
> + continue;
> +
> + case HCIATH_W4_EVENT_HDR:
> + eh = hci_event_hdr(ath->rx_skb);
> +
> + BT_DBG("Event header: evt 0x%2.2x plen %d",
> + eh->evt, eh->plen);
> +
> + ath_check_data_len(ath, eh->plen);
> + continue;
> +
> + case HCIATH_W4_ACL_HDR:
> + ah = hci_acl_hdr(ath->rx_skb);
> + dlen = __le16_to_cpu(ah->dlen);
> +
> + BT_DBG("ACL header: dlen %d", dlen);
> +
> + ath_check_data_len(ath, dlen);
> + continue;
> +
> + case HCIATH_W4_SCO_HDR:
> + sh = hci_sco_hdr(ath->rx_skb);
> +
> + BT_DBG("SCO header: dlen %d", sh->dlen);
> +
> + ath_check_data_len(ath, sh->dlen);
> + continue;
> +
> + }
> + }
> +
> + /* HCIATH_W4_PACKET_TYPE */
> + switch (*ptr) {
> + case HCI_EVENT_PKT:
> + BT_DBG("Event packet");
> + ath->rx_state = HCIATH_W4_EVENT_HDR;
> + ath->rx_count = HCI_EVENT_HDR_SIZE;
> + type = HCI_EVENT_PKT;
> + break;
> +
> + case HCI_ACLDATA_PKT:
> + BT_DBG("ACL packet");
> + ath->rx_state = HCIATH_W4_ACL_HDR;
> + ath->rx_count = HCI_ACL_HDR_SIZE;
> + type = HCI_ACLDATA_PKT;
> + break;
> +
> + case HCI_SCODATA_PKT:
> + BT_DBG("SCO packet");
> + ath->rx_state = HCIATH_W4_SCO_HDR;
> + ath->rx_count = HCI_SCO_HDR_SIZE;
> + type = HCI_SCODATA_PKT;
> + break;
> +
> + default:
> + BT_ERR("Unknown HCI packet type %2.2x", (__u8) *ptr);
> + hu->hdev->stat.err_rx++;
> + ptr++;
> + count--;
> + continue;
> +
> + };
> + ptr++;
> + count--;
> +
> + /* Allocate packet */
> + ath->rx_skb = bt_skb_alloc(HCI_MAX_FRAME_SIZE, GFP_ATOMIC);
> + if (!ath->rx_skb) {
> + BT_ERR("Can't allocate mem for new packet");
> + ath->rx_state = HCIATH_W4_PACKET_TYPE;
> + ath->rx_count = 0;
> +
> + return -ENOMEM;
> + }
> + ath->rx_skb->dev = (void *)hu->hdev;
> + bt_cb(ath->rx_skb)->pkt_type = type;
> + }
> +
> + return count;
> +}
What was the reason that hci_recv_fragment is not good enough here and
can not made work for this case. I really wanna avoid have multiple
implementation of this reassembly support all over the places.
Regards
Marcel
^ permalink raw reply
* [PATCH] Remove old defines from serial code
From: Gustavo F. Padovan @ 2010-05-21 1:28 UTC (permalink / raw)
To: linux-bluetooth; +Cc: gustavo
---
serial/manager.c | 4 ----
serial/proxy.c | 6 ------
2 files changed, 0 insertions(+), 10 deletions(-)
diff --git a/serial/manager.c b/serial/manager.c
index a7deab1..1b3f1d6 100644
--- a/serial/manager.c
+++ b/serial/manager.c
@@ -66,10 +66,6 @@
#include "sdpd.h"
#include "glib-helper.h"
-#define SERIAL_PORT_UUID "00001101-0000-1000-8000-00805F9B34FB"
-#define DIALUP_NET_UUID "00001103-0000-1000-8000-00805F9B34FB"
-#define OBJECT_PUSH_UUID "00001105-0000-1000-8000-00805F9B34FB"
-#define FILE_TRANSFER_UUID "00001106-0000-1000-8000-00805F9B34FB"
#define RFCOMM_UUID_STR "00000003-0000-1000-8000-00805F9B34FB"
static DBusConnection *connection = NULL;
diff --git a/serial/proxy.c b/serial/proxy.c
index 80c1189..6577fe2 100644
--- a/serial/proxy.c
+++ b/serial/proxy.c
@@ -63,12 +63,6 @@
#include "btio.h"
#include "proxy.h"
-#define SERIAL_PORT_NAME "spp"
-#define SERIAL_PORT_UUID "00001101-0000-1000-8000-00805F9B34FB"
-
-#define DIALUP_NET_NAME "dun"
-#define DIALUP_NET_UUID "00001103-0000-1000-8000-00805F9B34FB"
-
#define SERIAL_PROXY_INTERFACE "org.bluez.SerialProxy"
#define SERIAL_MANAGER_INTERFACE "org.bluez.SerialProxyManager"
#define BUF_SIZE 1024
--
1.7.1
^ permalink raw reply related
* use of hci_recv_fragment in HCI UART transport driver
From: suraj @ 2010-05-21 4:37 UTC (permalink / raw)
To: linux-bluetooth; +Cc: marcel, Luis.Rodriguez, Jothikumar.Mothilal
Hi,
The function "hci_recv_fragment()" was designed to avoid messy Bluetooth Rx packet reassembly on the HCI transport driver.
It does work well for HCI USB transport driver but it becomes a bit redundant when used on HCI UART transport driver.
This is basically due to the fact that the function require the caller to provide the HCI Packet type as input parameter.
This is pretty straight forward for a BT USB transport driver as both ACL data and HCI events are received through different callbacks.
Which means you will have 2 calls of hci_recv_fragment(). One with HCI_EVENT_PKT as packet type and other with HCI_ACLDATA_PKT, with packet type hard coded.
As far as HCI UART transport driver is concerned, it does not have this luxury. Both event and data are received through the same callback.
So, if the driver has to provide the packet type as input to hci_recv_fragment(), it will have to parse the HCI Rx data to get it in the first place.
This means driver will have to do everything hci_recv_fragment() does minus the memcpy, implementing the same messy code.
I know that we should be able to work around it by checking whether which reassembly buffer is not null and so on. But this is just a hack not a solution.
The second reason is, hci_recv_fragment() implements a certain policy on the driver i.e
" Whenever HCI Rx data is reassembled it directly has to be sent to host, without the driver interfering".
This robs the driver a chance have a look at the HCI event and do some housekeeping (it is entirely up to the driver what he wants to do then).
This is the one reason why someone would write a custom HCI transport driver protocol.
Other ways he could have used the standard H4 implementation if he just wanted to transfer packets to and from Host.
Please provide your comments.
Regards
Suraj
^ permalink raw reply
* Re: [PATCH v3] hciattach application support for Atheros AR300x Bluetooth Chip
From: Suraj Sumangala @ 2010-05-21 5:01 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Suraj Sumangala, linux-bluetooth@vger.kernel.org, Luis Rodriguez,
Jothikumar Mothilal
In-Reply-To: <1274371256.27220.15.camel@localhost.localdomain>
Hi Marcel,
Marcel Holtmann wrote:
> Hi Suraj,
>
>> Implements support for Atheros AR300x Bluetooth chip in hciattach.
>> Adds feature to bring up AR300x Bluetooth chip
>> with and without enhanced power management enabled.
>>
>> Signed-off-by: Suraj <suraj@Atheros.com>
>
> not SOBs for BlueZ userspace. That is kernel only.
>
>> ---
>> Makefile.tools | 1 +
>> tools/hciattach.8 | 6 +
>> tools/hciattach.c | 111 +++++
>> tools/hciattach.h | 3 +
>> tools/hciattach_ar3k.c | 1223 ++++++++++++++++++++++++++++++++++++++++++++++++
>> 5 files changed, 1344 insertions(+), 0 deletions(-)
>> create mode 100755 tools/hciattach_ar3k.c
>
> While hciattach is kinda the bad sheep inside BlueZ, I will enforce the
> general coding style for it for all new contributions. So could you
> please fix that one first. Otherwise it makes no sense for me to go
> through such a big patch if I have to complain about the coding style
> breakage for every second line. A general rule is if it fails
> check-patch.pl from the kernel, then you are doing something wrong.
>
> Regards
>
> Marcel
>
>
I had verified this patch using checkpatch and it did not complain. I
had also verified it manually according to the comments others have
given previously for different patches.
If you still see issues, then we can add that check too to checkpatch if
possible.
Being a big patch, there are bound to be problems.But I would appreciate
if you could review atleast part of it and give your comments. I can do
the changes for the full patch.
Also I would request others in the community to pitch in.
Regards
Suraj
^ permalink raw reply
* Re: [GSoC'10] : New Command Line Tool(s) for bluez
From: Andrei Emeltchenko @ 2010-05-21 7:29 UTC (permalink / raw)
To: Bastien Nocera; +Cc: Alexander Orlenko, linux-bluetooth
In-Reply-To: <1274360918.27274.3302.camel@localhost.localdomain>
On Thu, May 20, 2010 at 4:08 PM, Bastien Nocera <hadess@hadess.net> wrote:
> On Fri, 2010-05-21 at 00:04 +1100, Alexander Orlenko wrote:
>> Hi everyone,
>>
>> My name is Alexander Orlenko, I am a GSoC student of this year. I am
>> doing a project "bluez-tools",
>> http://hostisdown.org/gsoc/projects/bluez-tools (a little more info
>> about the project). I need some feedback about what approach should I
>> take.
>
> I'm not sure whether you'd be interested, but you could base your code
> on the non-GTK parts of libgnome-bluetooth, which already implements a
> number of those things, and would just need to be wrapped up.
There is also python code in bluez/test repository.
-- Andrei
>
> Cheers
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: use of hci_recv_fragment in HCI UART transport driver
From: Marcel Holtmann @ 2010-05-21 7:33 UTC (permalink / raw)
To: suraj; +Cc: linux-bluetooth, Luis.Rodriguez, Jothikumar.Mothilal
In-Reply-To: <1274416666.28388.29.camel@atheros013-desktop>
Hi Suraj,
> The function "hci_recv_fragment()" was designed to avoid messy Bluetooth Rx packet reassembly on the HCI transport driver.
> It does work well for HCI USB transport driver but it becomes a bit redundant when used on HCI UART transport driver.
>
> This is basically due to the fact that the function require the caller to provide the HCI Packet type as input parameter.
>
> This is pretty straight forward for a BT USB transport driver as both ACL data and HCI events are received through different callbacks.
> Which means you will have 2 calls of hci_recv_fragment(). One with HCI_EVENT_PKT as packet type and other with HCI_ACLDATA_PKT, with packet type hard coded.
>
> As far as HCI UART transport driver is concerned, it does not have this luxury. Both event and data are received through the same callback.
> So, if the driver has to provide the packet type as input to hci_recv_fragment(), it will have to parse the HCI Rx data to get it in the first place.
>
> This means driver will have to do everything hci_recv_fragment() does minus the memcpy, implementing the same messy code.
>
> I know that we should be able to work around it by checking whether which reassembly buffer is not null and so on. But this is just a hack not a solution.
I remember why I added the packet type to the function. The reason is
that events and ACL packets arrive on different USB endpoints and in
theory they can arrive at exactly the same time or intermix with each
other. So that needs to be protected.
So I think we need something like hci_recv_packet_fragment and
hci_recv_stream_fragment.
This would result at least in that we can consolidate all this code
duplication in the Bluetooth core.
> The second reason is, hci_recv_fragment() implements a certain policy on the driver i.e
>
> " Whenever HCI Rx data is reassembled it directly has to be sent to host, without the driver interfering".
>
> This robs the driver a chance have a look at the HCI event and do some housekeeping (it is entirely up to the driver what he wants to do then).
>
> This is the one reason why someone would write a custom HCI transport driver protocol.
> Other ways he could have used the standard H4 implementation if he just wanted to transfer packets to and from Host.
This is actually fully on purpose. A driver should not interfere with
HCI event or commands for that matter at all. It should be just a pure
transport.
If we need to hook something into vendor command/event handling then we
should find a different way in doing this. The major job of a HCI driver
is to be a pure, simple and stupid transport driver.
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