* Re: [PATCH] Fix problem with operator name length
From: Johan Hedberg @ 2010-10-13 14:19 UTC (permalink / raw)
To: Lukasz Pawlik; +Cc: linux-bluetooth
In-Reply-To: <1286974170-12282-1-git-send-email-lucas.pawlik@gmail.com>
Hi Lukasz,
On Wed, Oct 13, 2010, Lukasz Pawlik wrote:
> - net.operator_name = g_strdup(name);
> -
> + net.operator_name = g_strndup(name, 16);
Is it possible that the the format of the name would be such that byte
boundaries are not always the same as character boundaries (e.g. UTF-8)?
If so, using g_strndup sounds dangerous since it assumes one byte per
character.
Johan
^ permalink raw reply
* Re: [PATCH] Adjust mce_bt_set flag to gboolean type values
From: Johan Hedberg @ 2010-10-13 14:24 UTC (permalink / raw)
To: Rafal Michalski; +Cc: linux-bluetooth
In-Reply-To: <1286975300-2832-1-git-send-email-michalski.raf@gmail.com>
Hi Rafal,
On Wed, Oct 13, 2010, Rafal Michalski wrote:
> - mce_bt_set = !!(sigvalue & MCE_RADIO_STATE_BLUETOOTH);
> + mce_bt_set = sigvalue & MCE_RADIO_STATE_BLUETOOTH ?
> + TRUE : FALSE;
Ok, that's basically a coding style fix.
> - mce_bt_set = radio_states & MCE_RADIO_STATE_BLUETOOTH;
> + mce_bt_set = radio_states & MCE_RADIO_STATE_BLUETOOTH ? TRUE : FALSE;
And that's a clear bug fix (since there's no guarantee that the binary
and results in the value 1 or TRUE).
In principle I'd prefer having coding style patches separate from real
bug fixes (even for trivial changes), but since these are related I've
let it go for now and pushed the patch upstream.
Johan
^ permalink raw reply
* Re: [PATCH] Fix X-IRMC-CALL-DATETIME format
From: Johan Hedberg @ 2010-10-13 14:25 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <1286975756-13134-1-git-send-email-luiz.dentz@gmail.com>
Hi Luiz,
On Wed, Oct 13, 2010, Luiz Augusto von Dentz wrote:
> From: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>
>
> Use the format suggested in the spec:
>
> "For instance, a call that was missed on March 20th, 2005 at 10 am would
> be stamped: X-IRMC-CALL-DATETIME;MISSED:20050320T100000"
> ---
> plugins/phonebook-tracker.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
Pushed upstream. Thanks.
Johan
^ permalink raw reply
* Re: [PATCH] Some code cleanup in hdp
From: Johan Hedberg @ 2010-10-13 14:43 UTC (permalink / raw)
To: Jose Antonio Santos Cadenas; +Cc: linux-bluetooth
In-Reply-To: <1286978044-16484-1-git-send-email-santoscadenas@gmail.com>
Hi Jose,
On Wed, Oct 13, 2010, Jose Antonio Santos Cadenas wrote:
> ---
> health/hdp.c | 38 ++++++++++++++++++++++----------------
> health/hdp_util.c | 29 ++++++++++++++++++-----------
> 2 files changed, 40 insertions(+), 27 deletions(-)
Thanks. Pushed upstream.
Johan
^ permalink raw reply
* Re: [PATCH] Fix problem with operator name length
From: Dmitriy Paliy @ 2010-10-13 14:52 UTC (permalink / raw)
To: ext Johan Hedberg, Lukasz Pawlik; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <20101013141921.GB4828@jh-x301>
Hi,
At the moment it is received in ascii from csd since not all car kits
can handle utf8.
Br,
Dmitriy
On Wed, 2010-10-13 at 16:19 +0200, ext Johan Hedberg wrote:
> Hi Lukasz,
>
> On Wed, Oct 13, 2010, Lukasz Pawlik wrote:
> > - net.operator_name = g_strdup(name);
> > -
> > + net.operator_name = g_strndup(name, 16);
>
> Is it possible that the the format of the name would be such that byte
> boundaries are not always the same as character boundaries (e.g. UTF-8)?
> If so, using g_strndup sounds dangerous since it assumes one byte per
> character.
>
> Johan
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] Fix problem with operator name length
From: Johan Hedberg @ 2010-10-13 15:09 UTC (permalink / raw)
To: Dmitriy Paliy; +Cc: Lukasz Pawlik, linux-bluetooth@vger.kernel.org
In-Reply-To: <1286981571.2142.5.camel@dp-x301>
Hi Dmitriy,
First of all, don't top-post on this list.
On Wed, Oct 13, 2010, Dmitriy Paliy wrote:
> At the moment it is received in ascii from csd since not all car kits
> can handle utf8.
Ok, in that case it should be safe. I went ahead and and pushed the
patch along with a clarification in the commit message about this
guarantee that the csd API gives.
Johan
^ permalink raw reply
* Enabling SCO mode
From: Greg Mercurio @ 2010-10-13 15:45 UTC (permalink / raw)
To: linux-bluetooth
I am new to Bluez and have a question about sco audio. I am using Bluez version 4.47 in an Android environment.
The Bluetooth chip I’m using is initialized to use the PCM interface for sco audio data. So, the chip should be ready to transmit the sco audio when enabled.
>From what I can tell by looking at the Android code, the Bluetooth Handsfree Java application in initiates a sco connection by opening a sco socket connection with the stack.
I assume that opening the sco connection will send vendor independent HCI commands to the Bluetooth chip to put it into sco mode. I don’t see in the code where this is done.
Can someone confirm my assumptions and point me to the place in the code where enabling the sco commands are sent to the chip?
Thanks,
Greg
^ permalink raw reply
* [PATCH 1/2] Disable automatic notification/indication in the attribute server
From: Claudio Takahasi @ 2010-10-13 16:32 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Claudio Takahasi
In-Reply-To: <1286459639-25194-1-git-send-email-claudio.takahasi@openbossa.org>
Attribute server shall not send automatic indication/notification
messages if the client didn't request them. Client shall use Client
/Server Characteristic Configuration descriptors to be notified about
attribute changes. If the server doesn't support these descriptors,
the client shall implement a polling mechanism to check for attribute
value changes.
---
TODO | 7 ------
src/attrib-server.c | 60 ++++----------------------------------------------
2 files changed, 5 insertions(+), 62 deletions(-)
diff --git a/TODO b/TODO
index 3885c78..3e62518 100644
--- a/TODO
+++ b/TODO
@@ -18,13 +18,6 @@ Background
ATT/GATT
========
-- Sample server shouldn't send any indications or notifications without
- the client requesting them
-
- Priority: Medium
- Complexity: C2
- Owner: Claudio Takahasi <claudio.takahasi@openbossa.org>
-
- Add ATT/GATT parsing to hcidump
Priority: Medium
diff --git a/src/attrib-server.c b/src/attrib-server.c
index 666b5fa..1fc1c18 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -590,54 +590,6 @@ static void confirm_event(GIOChannel *io, void *user_data)
return;
}
-static gboolean send_notification(gpointer user_data)
-{
- uint8_t pdu[ATT_MAX_MTU];
- guint handle = GPOINTER_TO_UINT(user_data);
- struct attribute *a;
- GSList *l;
- uint16_t length;
-
- l = g_slist_find_custom(database, GUINT_TO_POINTER(handle), handle_cmp);
- if (!l)
- return FALSE;
-
- a = l->data;
-
- for (l = clients; l; l = l->next) {
- struct gatt_channel *channel = l->data;
-
- length = enc_notification(a, pdu, channel->mtu);
- g_attrib_send(channel->attrib, pdu[0], pdu, length, NULL, NULL, NULL);
- }
-
- return FALSE;
-}
-
-static gboolean send_indication(gpointer user_data)
-{
- uint8_t pdu[ATT_MAX_MTU];
- guint handle = GPOINTER_TO_UINT(user_data);
- struct attribute *a;
- GSList *l;
- uint16_t length;
-
- l = g_slist_find_custom(database, GUINT_TO_POINTER(handle), handle_cmp);
- if (!l)
- return FALSE;
-
- a = l->data;
-
- for (l = clients; l; l = l->next) {
- struct gatt_channel *channel = l->data;
-
- length = enc_indication(a, pdu, channel->mtu);
- g_attrib_send(channel->attrib, pdu[0], pdu, length, NULL, NULL, NULL);
- }
-
- return FALSE;
-}
-
int attrib_server_init(void)
{
GError *gerr = NULL;
@@ -757,14 +709,12 @@ int attrib_db_update(uint16_t handle, uuid_t *uuid, const uint8_t *value,
memcpy(a->data, value, len);
/*
- * Characteristic configuration descriptor is not being used yet.
- * If the attribute changes, all connected clients will be notified.
- * For testing purposes, we send a Notification and a Indication for
- * each update.
+ * <<Client/Server Characteristic Configuration>> descriptors are
+ * not supported yet. If a given attribute changes, the attribute
+ * server shall report the new values using the mechanism selected
+ * by the client. Notification/Indication shall not be automatically
+ * sent if the client didn't request them.
*/
- g_idle_add(send_notification, GUINT_TO_POINTER(h));
-
- g_idle_add(send_indication, GUINT_TO_POINTER(h));
return 0;
}
--
1.7.3.1
^ permalink raw reply related
* [PATCH 2/2] TODO: Implement Client Characteristic Configuration
From: Claudio Takahasi @ 2010-10-13 16:34 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Claudio Takahasi
In-Reply-To: <1286459639-25194-2-git-send-email-claudio.takahasi@openbossa.org>
---
TODO | 9 +++++++++
1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/TODO b/TODO
index 3e62518..7939d9d 100644
--- a/TODO
+++ b/TODO
@@ -89,3 +89,12 @@ ATT/GATT
Priority: Low
Complexity: C2
+
+- Implement Client Characteristic Configuration support in the attribute
+ server to manage indications and notications. This is a per client attribute
+ to control how the client wants to receive reports of changes in a given
+ characteristic value.
+ See Volume 3, Part G, section 3.3.3.3 for more information
+
+ Priority: Low
+ Complexity: C2
--
1.7.3.1
^ permalink raw reply related
* [PATCH] TODO: Implement Server Characteristic Configuration
From: Claudio Takahasi @ 2010-10-13 16:37 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Claudio Takahasi
In-Reply-To: <1286460773-25436-1-git-send-email-claudio.takahasi@openbossa.org>
---
TODO | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/TODO b/TODO
index 7939d9d..0a7b482 100644
--- a/TODO
+++ b/TODO
@@ -78,6 +78,14 @@ ATT/GATT
Priority: Low
Complexity: C1
+- Implement Server characteristic Configuration support in the attribute
+ server to manage characteristic value broadcasting. There is a single
+ instance of the Server Characteristic Configuration for all clients.
+ See Volume 3, Part G, section 3.3.3.4 for more information.
+
+ Priority: Low
+ Complexity: C1
+
- Long reads/writes don't work (consisting of multiple request packets)
Priority: Low
--
1.7.3.1
^ permalink raw reply related
* Re: [PATCH 1/2] Disable automatic notification/indication in the attribute server
From: Johan Hedberg @ 2010-10-13 16:59 UTC (permalink / raw)
To: Claudio Takahasi; +Cc: linux-bluetooth
In-Reply-To: <1286987539-25833-1-git-send-email-claudio.takahasi@openbossa.org>
Hi Claudio,
On Wed, Oct 13, 2010, Claudio Takahasi wrote:
> Attribute server shall not send automatic indication/notification
> messages if the client didn't request them. Client shall use Client
> /Server Characteristic Configuration descriptors to be notified about
> attribute changes. If the server doesn't support these descriptors,
> the client shall implement a polling mechanism to check for attribute
> value changes.
> ---
> TODO | 7 ------
> src/attrib-server.c | 60 ++++----------------------------------------------
> 2 files changed, 5 insertions(+), 62 deletions(-)
Thanks. The patch is now upstream.
Johan
^ permalink raw reply
* Re: [PATCH 2/2] TODO: Implement Client Characteristic Configuration
From: Johan Hedberg @ 2010-10-13 16:59 UTC (permalink / raw)
To: Claudio Takahasi; +Cc: linux-bluetooth
In-Reply-To: <1286987650-25914-1-git-send-email-claudio.takahasi@openbossa.org>
Hi Claudio,
On Wed, Oct 13, 2010, Claudio Takahasi wrote:
> ---
> TODO | 9 +++++++++
> 1 files changed, 9 insertions(+), 0 deletions(-)
Thanks. This one has also been pushed upstream.
Johan
^ permalink raw reply
* Re: [PATCH] TODO: Implement Server Characteristic Configuration
From: Johan Hedberg @ 2010-10-13 17:00 UTC (permalink / raw)
To: Claudio Takahasi; +Cc: linux-bluetooth
In-Reply-To: <1286987833-25972-1-git-send-email-claudio.takahasi@openbossa.org>
Hi Claudio,
On Wed, Oct 13, 2010, Claudio Takahasi wrote:
> ---
> TODO | 8 ++++++++
> 1 files changed, 8 insertions(+), 0 deletions(-)
And this one has also been pushed. Thanks.
Johan
^ permalink raw reply
* Re: Firmware versioning best practices: ath3k-2.fw rename or replace ath3k-1.fw ?
From: Luis R. Rodriguez @ 2010-10-13 17:42 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Henry Ptasinski, Suraj Sumangala, Luis Rodriguez, David Woodhouse,
linux-bluetooth, linux-kernel@vger.kernel.org, linux-wireless
In-Reply-To: <1286964370.3316.6.camel@aeonflux>
On Wed, Oct 13, 2010 at 3:06 AM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Henry,
>
>> > > Marcel had answered me before. It makes sense to have same file name.
>> > > Other ways we end up changing the driver whenever there is a firmware
>> > > change.
>> >
>> > > > I last tried to document a thread we had over this here:
>> > > >
>> > > > http://wireless.kernel.org/en/developers/Documentation/firmware-versioning
>> > > >
>> >
>> > Thanks, I've updated that link above to document bug fixing does not require
>> > a filename change.
>>
>> I don't really understand why you would not want to change the code revision
>> part of the filename.
>>
>> I totally agree that you don't want to change the driver every time the
>> firmware gets a bug fix, but wasn't that the whole point of splitting the name
>> into API and code revisions portions, and symlinking the file to one that just
>> has the API version?
>>
>> What's the issue with using the process as originally documented?
>
> as I stated before, for Bluetooth this makes no sense. You don't need
> API version numbers since the API is a STANDARD. It is called HCI. So
> please don't use API version numbers in the firmware files.
>
> I will reject firmware file versions for upstream drivers.
Does the HCI standard ever get improved upon? If so, how do devices
never get firmware updates that would allow them to use some newer HCI
APIs?
I've updated the documentation above for 802.11 and Bluetooth with the
above, please feel free to further extend it as you see fit.
Luis
^ permalink raw reply
* [PATCH] TODO: Device Name Characteristic for Low Energy
From: Claudio Takahasi @ 2010-10-13 17:44 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Claudio Takahasi
---
TODO | 14 ++++++++++++++
1 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/TODO b/TODO
index 0a7b482..2c17c6b 100644
--- a/TODO
+++ b/TODO
@@ -14,6 +14,20 @@ Background
Higher complexity tasks should be refined into several lower complexity tasks
once the task is better understood.
+LE Generic Access Profile
+=========================
+
+- Device Name Characteristic is a GAP characteristic for Low Energy. This
+ characteristic shall be integrated/used in the discovery procedure. The
+ ideia is to report the value of this characteristic using DeviceFound signals.
+ Discussion with the community is needed before to start this task. Other GAP
+ characteristics for LE needs to follow a similar approach. It is not clear
+ if all GAP characteristics can be exposed using properties instead of a primary
+ service characteristics.
+ See Volume 3, Part C, section 12.1 for more information.
+
+ Priority: Low
+ Complexity: C2
ATT/GATT
========
--
1.7.3.1
^ permalink raw reply related
* RE: Firmware versioning best practices: ath3k-2.fw rename or replace ath3k-1.fw ?
From: Kevin Hayes @ 2010-10-13 17:54 UTC (permalink / raw)
To: Luis Rodriguez, Marcel Holtmann
Cc: Henry Ptasinski, Suraj Sumangala, Luis Rodriguez, David Woodhouse,
linux-bluetooth, linux-kernel@vger.kernel.org, linux-wireless
In-Reply-To: <AANLkTimAeoS2Vi7GhKSTrghqaOQ=XKSzjcou15dn+YDR@mail.gmail.com>
SGkgTHVpcywNCg0KDQo+IC0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQo+IEZyb206IGxpbnV4
LWJsdWV0b290aC1vd25lckB2Z2VyLmtlcm5lbC5vcmcgW21haWx0bzpsaW51eC1ibHVldG9vdGgt
DQo+IG93bmVyQHZnZXIua2VybmVsLm9yZ10gT24gQmVoYWxmIE9mIEx1aXMgUi4gUm9kcmlndWV6
DQo+IFNlbnQ6IFdlZG5lc2RheSwgT2N0b2JlciAxMywgMjAxMCAxMDo0MyBBTQ0KPiBUbzogTWFy
Y2VsIEhvbHRtYW5uDQo+IENjOiBIZW5yeSBQdGFzaW5za2k7IFN1cmFqIFN1bWFuZ2FsYTsgTHVp
cyBSb2RyaWd1ZXo7IERhdmlkIFdvb2Rob3VzZTsNCj4gbGludXgtYmx1ZXRvb3RoOyBsaW51eC1r
ZXJuZWxAdmdlci5rZXJuZWwub3JnOyBsaW51eC13aXJlbGVzcw0KPiBTdWJqZWN0OiBSZTogRmly
bXdhcmUgdmVyc2lvbmluZyBiZXN0IHByYWN0aWNlczogYXRoM2stMi5mdyByZW5hbWUgb3INCj4g
cmVwbGFjZSBhdGgzay0xLmZ3ID8NCj4gDQo+IE9uIFdlZCwgT2N0IDEzLCAyMDEwIGF0IDM6MDYg
QU0sIE1hcmNlbCBIb2x0bWFubiA8bWFyY2VsQGhvbHRtYW5uLm9yZz4NCj4gd3JvdGU6DQo+ID4g
SGkgSGVucnksDQo+ID4NCj4gPj4gPiA+IE1hcmNlbCBoYWQgYW5zd2VyZWQgbWUgYmVmb3JlLiBJ
dCBtYWtlcyBzZW5zZSB0byBoYXZlIHNhbWUgZmlsZQ0KPiBuYW1lLg0KPiA+PiA+ID4gT3RoZXIg
d2F5cyB3ZSBlbmQgdXAgY2hhbmdpbmcgdGhlIGRyaXZlciB3aGVuZXZlciB0aGVyZSBpcyBhDQo+
IGZpcm13YXJlDQo+ID4+ID4gPiBjaGFuZ2UuDQo+ID4+ID4NCj4gPj4gPiA+ID4gSSBsYXN0IHRy
aWVkIHRvIGRvY3VtZW50IGEgdGhyZWFkIHdlIGhhZCBvdmVyIHRoaXMgaGVyZToNCj4gPj4gPiA+
ID4NCj4gPj4gPiA+ID4NCj4gaHR0cDovL3dpcmVsZXNzLmtlcm5lbC5vcmcvZW4vZGV2ZWxvcGVy
cy9Eb2N1bWVudGF0aW9uL2Zpcm13YXJlLQ0KPiB2ZXJzaW9uaW5nDQo+ID4+ID4gPiA+DQo+ID4+
ID4NCj4gPj4gPiBUaGFua3MsIEkndmUgdXBkYXRlZCB0aGF0IGxpbmsgYWJvdmUgdG8gZG9jdW1l
bnQgYnVnIGZpeGluZyBkb2VzDQo+IG5vdCByZXF1aXJlDQo+ID4+ID4gYSBmaWxlbmFtZSBjaGFu
Z2UuDQo+ID4+DQo+ID4+IEkgZG9uJ3QgcmVhbGx5IHVuZGVyc3RhbmQgd2h5IHlvdSB3b3VsZCBu
b3Qgd2FudCB0byBjaGFuZ2UgdGhlIGNvZGUNCj4gcmV2aXNpb24NCj4gPj4gcGFydCBvZiB0aGUg
ZmlsZW5hbWUuDQo+ID4+DQo+ID4+IEkgdG90YWxseSBhZ3JlZSB0aGF0IHlvdSBkb24ndCB3YW50
IHRvIGNoYW5nZSB0aGUgZHJpdmVyIGV2ZXJ5IHRpbWUNCj4gdGhlDQo+ID4+IGZpcm13YXJlIGdl
dHMgYSBidWcgZml4LCBidXQgd2Fzbid0IHRoYXQgdGhlIHdob2xlIHBvaW50IG9mDQo+IHNwbGl0
dGluZyB0aGUgbmFtZQ0KPiA+PiBpbnRvIEFQSSBhbmQgY29kZSByZXZpc2lvbnMgcG9ydGlvbnMs
IGFuZCBzeW1saW5raW5nIHRoZSBmaWxlIHRvIG9uZQ0KPiB0aGF0IGp1c3QNCj4gPj4gaGFzIHRo
ZSBBUEkgdmVyc2lvbj8NCj4gPj4NCj4gPj4gV2hhdCdzIHRoZSBpc3N1ZSB3aXRoIHVzaW5nIHRo
ZSBwcm9jZXNzIGFzIG9yaWdpbmFsbHkgZG9jdW1lbnRlZD8NCj4gPg0KPiA+IGFzIEkgc3RhdGVk
IGJlZm9yZSwgZm9yIEJsdWV0b290aCB0aGlzIG1ha2VzIG5vIHNlbnNlLiBZb3UgZG9uJ3QgbmVl
ZA0KPiA+IEFQSSB2ZXJzaW9uIG51bWJlcnMgc2luY2UgdGhlIEFQSSBpcyBhIFNUQU5EQVJELiBJ
dCBpcyBjYWxsZWQgSENJLiBTbw0KPiA+IHBsZWFzZSBkb24ndCB1c2UgQVBJIHZlcnNpb24gbnVt
YmVycyBpbiB0aGUgZmlybXdhcmUgZmlsZXMuDQo+ID4NCj4gPiBJIHdpbGwgcmVqZWN0IGZpcm13
YXJlIGZpbGUgdmVyc2lvbnMgZm9yIHVwc3RyZWFtIGRyaXZlcnMuDQo+IA0KPiBEb2VzIHRoZSBI
Q0kgc3RhbmRhcmQgZXZlciBnZXQgaW1wcm92ZWQgdXBvbj8gSWYgc28sIGhvdyBkbyBkZXZpY2Vz
DQo+IG5ldmVyIGdldCBmaXJtd2FyZSB1cGRhdGVzIHRoYXQgd291bGQgYWxsb3cgdGhlbSB0byB1
c2Ugc29tZSBuZXdlciBIQ0kNCj4gQVBJcz8NCj4gDQo+IEkndmUgdXBkYXRlZCB0aGUgZG9jdW1l
bnRhdGlvbiBhYm92ZSBmb3IgODAyLjExIGFuZCBCbHVldG9vdGggd2l0aCB0aGUNCj4gYWJvdmUs
IHBsZWFzZSBmZWVsIGZyZWUgdG8gZnVydGhlciBleHRlbmQgaXQgYXMgeW91IHNlZSBmaXQuDQo+
IA0KPiAgIEx1aXMNCg0KSENJIGlzIGFsd2F5cyBiYWNrd2FyZCBjb21wYXRpYmxlLiAgTmV3ZXIg
Y29tbWFuZHMgYXJlIHByb3Blcmx5IGRpc2NvdmVyYWJsZSBieSBib3RoIHNpZGVzIG9mIHRoZSBI
Q0kgbGluay4NCkFzIGxvbmcgYXMgdGhlIHByb2NlZHVyZSB0byBkb3dubG9hZCBmaXJtd2FyZSBk
b2VzIG5vdCBkZXBlbmQgb24gbmV3IEhDSSBjb21tYW5kcyAoaXQgZG9lcyBub3QpLCB0aGVuIHRo
ZSBmaXJtd2FyZSBpdHNlbGYgY2FuIHRlYWNoIGFuIG9sZCBjb250cm9sbGVyIHRvIGxlYXJuIG5l
dyB0cmlja3MuDQoNCglLKysNCg==
^ permalink raw reply
* Re: Firmware versioning best practices: ath3k-2.fw rename or replace ath3k-1.fw ?
From: Luis R. Rodriguez @ 2010-10-13 18:09 UTC (permalink / raw)
To: Kevin Hayes
Cc: Luis Rodriguez, Marcel Holtmann, Henry Ptasinski, Suraj Sumangala,
David Woodhouse, linux-bluetooth, linux-kernel@vger.kernel.org,
linux-wireless
In-Reply-To: <B7132A25476D334D9130FE7532F2A56314B5929629@SC1EXMB-MBCL.global.atheros.com>
On Wed, Oct 13, 2010 at 10:54 AM, Kevin Hayes <kevin@atheros.com> wrote:
> Hi Luis,
>
>
>> -----Original Message-----
>> From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-
>> owner@vger.kernel.org] On Behalf Of Luis R. Rodriguez
>> Sent: Wednesday, October 13, 2010 10:43 AM
>> To: Marcel Holtmann
>> Cc: Henry Ptasinski; Suraj Sumangala; Luis Rodriguez; David Woodhouse;
>> linux-bluetooth; linux-kernel@vger.kernel.org; linux-wireless
>> Subject: Re: Firmware versioning best practices: ath3k-2.fw rename or
>> replace ath3k-1.fw ?
>>
>> On Wed, Oct 13, 2010 at 3:06 AM, Marcel Holtmann <marcel@holtmann.org>
>> wrote:
>> > Hi Henry,
>> >
>> >> > > Marcel had answered me before. It makes sense to have same file
>> name.
>> >> > > Other ways we end up changing the driver whenever there is a
>> firmware
>> >> > > change.
>> >> >
>> >> > > > I last tried to document a thread we had over this here:
>> >> > > >
>> >> > > >
>> http://wireless.kernel.org/en/developers/Documentation/firmware-
>> versioning
>> >> > > >
>> >> >
>> >> > Thanks, I've updated that link above to document bug fixing does
>> not require
>> >> > a filename change.
>> >>
>> >> I don't really understand why you would not want to change the code
>> revision
>> >> part of the filename.
>> >>
>> >> I totally agree that you don't want to change the driver every time
>> the
>> >> firmware gets a bug fix, but wasn't that the whole point of
>> splitting the name
>> >> into API and code revisions portions, and symlinking the file to one
>> that just
>> >> has the API version?
>> >>
>> >> What's the issue with using the process as originally documented?
>> >
>> > as I stated before, for Bluetooth this makes no sense. You don't need
>> > API version numbers since the API is a STANDARD. It is called HCI. So
>> > please don't use API version numbers in the firmware files.
>> >
>> > I will reject firmware file versions for upstream drivers.
>>
>> Does the HCI standard ever get improved upon? If so, how do devices
>> never get firmware updates that would allow them to use some newer HCI
>> APIs?
>>
>> I've updated the documentation above for 802.11 and Bluetooth with the
>> above, please feel free to further extend it as you see fit.
>>
>> =C2=A0 Luis
>
> HCI is always backward compatible. =C2=A0Newer commands are properly disc=
overable by both sides of the HCI link.
> As long as the procedure to download firmware does not depend on new HCI =
commands (it does not), then the firmware itself can teach an old controlle=
r to learn new tricks.
Does HCI support uploading firmware? Can't we resolve this blacklist
crap issue if devices just used HCI to upload firmware?
Luis
^ permalink raw reply
* RE: Firmware versioning best practices: ath3k-2.fw rename or replace ath3k-1.fw ?
From: Kevin Hayes @ 2010-10-13 18:41 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: Luis Rodriguez, Marcel Holtmann, Henry Ptasinski, Suraj Sumangala,
David Woodhouse, linux-bluetooth, linux-kernel@vger.kernel.org,
linux-wireless
In-Reply-To: <AANLkTik9oYwspVG4KFbMMq63SpBp++iZe=vxOSQhxRki@mail.gmail.com>
SGkgTHVpcywNCg0KPiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiBsaW51eC1i
bHVldG9vdGgtb3duZXJAdmdlci5rZXJuZWwub3JnIFttYWlsdG86bGludXgtYmx1ZXRvb3RoLQ0K
PiBvd25lckB2Z2VyLmtlcm5lbC5vcmddIE9uIEJlaGFsZiBPZiBMdWlzIFIuIFJvZHJpZ3Vleg0K
PiBTZW50OiBXZWRuZXNkYXksIE9jdG9iZXIgMTMsIDIwMTAgMTE6MTAgQU0NCj4gVG86IEtldmlu
IEhheWVzDQo+IENjOiBMdWlzIFJvZHJpZ3VlejsgTWFyY2VsIEhvbHRtYW5uOyBIZW5yeSBQdGFz
aW5za2k7IFN1cmFqIFN1bWFuZ2FsYTsNCj4gRGF2aWQgV29vZGhvdXNlOyBsaW51eC1ibHVldG9v
dGg7IGxpbnV4LWtlcm5lbEB2Z2VyLmtlcm5lbC5vcmc7IGxpbnV4LQ0KPiB3aXJlbGVzcw0KPiBT
dWJqZWN0OiBSZTogRmlybXdhcmUgdmVyc2lvbmluZyBiZXN0IHByYWN0aWNlczogYXRoM2stMi5m
dyByZW5hbWUgb3INCj4gcmVwbGFjZSBhdGgzay0xLmZ3ID8NCj4gDQo+IE9uIFdlZCwgT2N0IDEz
LCAyMDEwIGF0IDEwOjU0IEFNLCBLZXZpbiBIYXllcyA8a2V2aW5AYXRoZXJvcy5jb20+DQo+IHdy
b3RlOg0KPiA+IEhpIEx1aXMsDQo+ID4NCj4gPg0KPiA+PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2Ut
LS0tLQ0KPiA+PiBGcm9tOiBsaW51eC1ibHVldG9vdGgtb3duZXJAdmdlci5rZXJuZWwub3JnIFtt
YWlsdG86bGludXgtYmx1ZXRvb3RoLQ0KPiA+PiBvd25lckB2Z2VyLmtlcm5lbC5vcmddIE9uIEJl
aGFsZiBPZiBMdWlzIFIuIFJvZHJpZ3Vleg0KPiA+PiBTZW50OiBXZWRuZXNkYXksIE9jdG9iZXIg
MTMsIDIwMTAgMTA6NDMgQU0NCj4gPj4gVG86IE1hcmNlbCBIb2x0bWFubg0KPiA+PiBDYzogSGVu
cnkgUHRhc2luc2tpOyBTdXJhaiBTdW1hbmdhbGE7IEx1aXMgUm9kcmlndWV6OyBEYXZpZA0KPiBX
b29kaG91c2U7DQo+ID4+IGxpbnV4LWJsdWV0b290aDsgbGludXgta2VybmVsQHZnZXIua2VybmVs
Lm9yZzsgbGludXgtd2lyZWxlc3MNCj4gPj4gU3ViamVjdDogUmU6IEZpcm13YXJlIHZlcnNpb25p
bmcgYmVzdCBwcmFjdGljZXM6IGF0aDNrLTIuZncgcmVuYW1lDQo+IG9yDQo+ID4+IHJlcGxhY2Ug
YXRoM2stMS5mdyA/DQo+ID4+DQo+ID4+IE9uIFdlZCwgT2N0IDEzLCAyMDEwIGF0IDM6MDYgQU0s
IE1hcmNlbCBIb2x0bWFubg0KPiA8bWFyY2VsQGhvbHRtYW5uLm9yZz4NCj4gPj4gd3JvdGU6DQo+
ID4+ID4gSGkgSGVucnksDQo+ID4+ID4NCj4gPj4gPj4gPiA+IE1hcmNlbCBoYWQgYW5zd2VyZWQg
bWUgYmVmb3JlLiBJdCBtYWtlcyBzZW5zZSB0byBoYXZlIHNhbWUNCj4gZmlsZQ0KPiA+PiBuYW1l
Lg0KPiA+PiA+PiA+ID4gT3RoZXIgd2F5cyB3ZSBlbmQgdXAgY2hhbmdpbmcgdGhlIGRyaXZlciB3
aGVuZXZlciB0aGVyZSBpcyBhDQo+ID4+IGZpcm13YXJlDQo+ID4+ID4+ID4gPiBjaGFuZ2UuDQo+
ID4+ID4+ID4NCj4gPj4gPj4gPiA+ID4gSSBsYXN0IHRyaWVkIHRvIGRvY3VtZW50IGEgdGhyZWFk
IHdlIGhhZCBvdmVyIHRoaXMgaGVyZToNCj4gPj4gPj4gPiA+ID4NCj4gPj4gPj4gPiA+ID4NCj4g
Pj4gaHR0cDovL3dpcmVsZXNzLmtlcm5lbC5vcmcvZW4vZGV2ZWxvcGVycy9Eb2N1bWVudGF0aW9u
L2Zpcm13YXJlLQ0KPiA+PiB2ZXJzaW9uaW5nDQo+ID4+ID4+ID4gPiA+DQo+ID4+ID4+ID4NCj4g
Pj4gPj4gPiBUaGFua3MsIEkndmUgdXBkYXRlZCB0aGF0IGxpbmsgYWJvdmUgdG8gZG9jdW1lbnQg
YnVnIGZpeGluZw0KPiBkb2VzDQo+ID4+IG5vdCByZXF1aXJlDQo+ID4+ID4+ID4gYSBmaWxlbmFt
ZSBjaGFuZ2UuDQo+ID4+ID4+DQo+ID4+ID4+IEkgZG9uJ3QgcmVhbGx5IHVuZGVyc3RhbmQgd2h5
IHlvdSB3b3VsZCBub3Qgd2FudCB0byBjaGFuZ2UgdGhlDQo+IGNvZGUNCj4gPj4gcmV2aXNpb24N
Cj4gPj4gPj4gcGFydCBvZiB0aGUgZmlsZW5hbWUuDQo+ID4+ID4+DQo+ID4+ID4+IEkgdG90YWxs
eSBhZ3JlZSB0aGF0IHlvdSBkb24ndCB3YW50IHRvIGNoYW5nZSB0aGUgZHJpdmVyIGV2ZXJ5DQo+
IHRpbWUNCj4gPj4gdGhlDQo+ID4+ID4+IGZpcm13YXJlIGdldHMgYSBidWcgZml4LCBidXQgd2Fz
bid0IHRoYXQgdGhlIHdob2xlIHBvaW50IG9mDQo+ID4+IHNwbGl0dGluZyB0aGUgbmFtZQ0KPiA+
PiA+PiBpbnRvIEFQSSBhbmQgY29kZSByZXZpc2lvbnMgcG9ydGlvbnMsIGFuZCBzeW1saW5raW5n
IHRoZSBmaWxlIHRvDQo+IG9uZQ0KPiA+PiB0aGF0IGp1c3QNCj4gPj4gPj4gaGFzIHRoZSBBUEkg
dmVyc2lvbj8NCj4gPj4gPj4NCj4gPj4gPj4gV2hhdCdzIHRoZSBpc3N1ZSB3aXRoIHVzaW5nIHRo
ZSBwcm9jZXNzIGFzIG9yaWdpbmFsbHkgZG9jdW1lbnRlZD8NCj4gPj4gPg0KPiA+PiA+IGFzIEkg
c3RhdGVkIGJlZm9yZSwgZm9yIEJsdWV0b290aCB0aGlzIG1ha2VzIG5vIHNlbnNlLiBZb3UgZG9u
J3QNCj4gbmVlZA0KPiA+PiA+IEFQSSB2ZXJzaW9uIG51bWJlcnMgc2luY2UgdGhlIEFQSSBpcyBh
IFNUQU5EQVJELiBJdCBpcyBjYWxsZWQgSENJLg0KPiBTbw0KPiA+PiA+IHBsZWFzZSBkb24ndCB1
c2UgQVBJIHZlcnNpb24gbnVtYmVycyBpbiB0aGUgZmlybXdhcmUgZmlsZXMuDQo+ID4+ID4NCj4g
Pj4gPiBJIHdpbGwgcmVqZWN0IGZpcm13YXJlIGZpbGUgdmVyc2lvbnMgZm9yIHVwc3RyZWFtIGRy
aXZlcnMuDQo+ID4+DQo+ID4+IERvZXMgdGhlIEhDSSBzdGFuZGFyZCBldmVyIGdldCBpbXByb3Zl
ZCB1cG9uPyBJZiBzbywgaG93IGRvIGRldmljZXMNCj4gPj4gbmV2ZXIgZ2V0IGZpcm13YXJlIHVw
ZGF0ZXMgdGhhdCB3b3VsZCBhbGxvdyB0aGVtIHRvIHVzZSBzb21lIG5ld2VyDQo+IEhDSQ0KPiA+
PiBBUElzPw0KPiA+Pg0KPiA+PiBJJ3ZlIHVwZGF0ZWQgdGhlIGRvY3VtZW50YXRpb24gYWJvdmUg
Zm9yIDgwMi4xMSBhbmQgQmx1ZXRvb3RoIHdpdGgNCj4gdGhlDQo+ID4+IGFib3ZlLCBwbGVhc2Ug
ZmVlbCBmcmVlIHRvIGZ1cnRoZXIgZXh0ZW5kIGl0IGFzIHlvdSBzZWUgZml0Lg0KPiA+Pg0KPiA+
PiDCoCBMdWlzDQo+ID4NCj4gPiBIQ0kgaXMgYWx3YXlzIGJhY2t3YXJkIGNvbXBhdGlibGUuIMKg
TmV3ZXIgY29tbWFuZHMgYXJlIHByb3Blcmx5DQo+IGRpc2NvdmVyYWJsZSBieSBib3RoIHNpZGVz
IG9mIHRoZSBIQ0kgbGluay4NCj4gPiBBcyBsb25nIGFzIHRoZSBwcm9jZWR1cmUgdG8gZG93bmxv
YWQgZmlybXdhcmUgZG9lcyBub3QgZGVwZW5kIG9uIG5ldw0KPiBIQ0kgY29tbWFuZHMgKGl0IGRv
ZXMgbm90KSwgdGhlbiB0aGUgZmlybXdhcmUgaXRzZWxmIGNhbiB0ZWFjaCBhbiBvbGQNCj4gY29u
dHJvbGxlciB0byBsZWFybiBuZXcgdHJpY2tzLg0KPiANCj4gRG9lcyBIQ0kgc3VwcG9ydCB1cGxv
YWRpbmcgZmlybXdhcmU/IENhbid0IHdlIHJlc29sdmUgdGhpcyBibGFja2xpc3QNCj4gY3JhcCBp
c3N1ZSBpZiBkZXZpY2VzIGp1c3QgdXNlZCBIQ0kgdG8gdXBsb2FkIGZpcm13YXJlPw0KPiANCj4g
ICBMdWlzDQo+IC0tDQoNCk5vdCByZWFsbHkgYmVjYXVzZSB0aGVyZSBpcyBub3QgZW5vdWdoIHNw
YWNlIGluIHRoZSBzZmxhc2ggdG8gZG8gbXVjaCBvZiBhbnl0aGluZyBleGNlcHQgcmVwb3J0IHRo
ZSBQSUQuDQpJdCBtdXN0IGJlIHNvbWUgZXh0ZXJuYWwgY29kZSB0aGF0IHBpY2tzIHRoZSBmaXJt
d2FyZSB0byBsb2FkIGFuZCBpbmplY3QgaXQgaW50byB0aGUgY29udHJvbGxlci4NClRoYXQgZXh0
ZXJuYWwgY29kZSBpcyB0aGUgREZVIGJpdC4gIFdlIG11c3QgYmxhY2tsaXN0IHRoZSBkZXZpY2Ug
c28gdGhhdCBidHVzYiBkb2VzIG5vdCBjbGFpbSBpdCwgc28gd2UgY2FuIHVzZSB0aGUgREZVLg0K
DQogICAgSysrDQoNCltQYXJkb24gdGhlIGdhcmJhZ2UgdGhhdCBvdXIgbWFpbCBzZXJ2ZXIgYWRk
cy4uLl0NCg0KDQoNCg0KDQo=
^ permalink raw reply
* Re: [PATCH] TODO: Device Name Characteristic for Low Energy
From: Johan Hedberg @ 2010-10-13 19:27 UTC (permalink / raw)
To: Claudio Takahasi; +Cc: linux-bluetooth
In-Reply-To: <1286991867-26866-1-git-send-email-claudio.takahasi@openbossa.org>
Hi Claudio,
On Wed, Oct 13, 2010, Claudio Takahasi wrote:
> ---
> TODO | 14 ++++++++++++++
> 1 files changed, 14 insertions(+), 0 deletions(-)
Pushed upstream. Thanks.
Johan
^ permalink raw reply
* Re: pull request: bluetooth-next-2.6 2010-10-13
From: Luis R. Rodriguez @ 2010-10-13 21:41 UTC (permalink / raw)
To: Gustavo F. Padovan
Cc: linville, marcel, linux-wireless, linux-bluetooth, linux-kernel
In-Reply-To: <20101013034808.GA3205@vigoh>
On Tue, Oct 12, 2010 at 8:48 PM, Gustavo F. Padovan
<padovan@profusion.mobi> wrote:
> One notable change is in the MAINTAINERS file, From now and on I'm going
> to maintain the Bluetooth trees as well as send the pull requests to you.
I noted here you mentioned "as well", does this mean there may be
updates from two people now instead of just one?
> Despite this maintenance change, Marcel will remain helping with patch
> review, and ACK/NAK in the Bluetooth subsystem as he always did. Not a
> big change in the end. ;)
Luis
^ permalink raw reply
* Re: [PATCH 6/6] This patch adds support for using the ST-Ericsson CG2900
From: Marcel Holtmann @ 2010-10-13 21:48 UTC (permalink / raw)
To: Par-Gunnar Hjalmdahl
Cc: linux-bluetooth, linux-kernel, linus.walleij, Pavan Savoy
In-Reply-To: <AANLkTinCUscmm=1t7iEAz2LQ0O_TM5-G7zANhLcuA-d9@mail.gmail.com>
Hi Par-Gunnar,
> Sorry for not answering to your mail earlier. I've been on a business
> trip whole last week where I just did not have the possibility to
> answer.
>
> We are currently working on a new version of our driver, both the MFD
> and the Bluetooth part, where we address the comments that we have so
> far received. Hopefully I will able to send it later this week.
>
> As answer to last mail: yes, we will change our debug system to be
> reuse existing functionality in the Kernel.
sounds good to me. dynamic debug is actually pretty nice :)
>
> /P-G
>
> 2010/10/5 Marcel Holtmann <marcel@holtmann.org>:
> > Hi Par-Gunnar,
> >
> >> This patch adds support for using the ST-Ericsson CG2900
> >> connectivity controller as a driver for the BlueZ Bluetooth
> >> stack.
> >> This patch registers as a driver into the BlueZ framework and, when
> >> opened by BlueZ, it registers as user for bt_cmd, bt_acl, and bt_evt
> >> channels.
> >>
> >> Signed-off-by: Par-Gunnar Hjalmdahl <par-gunnar.p.hjalmdahl@stericsson.com>
> >> ---
> >> drivers/bluetooth/Kconfig | 7 +
> >> drivers/bluetooth/Makefile | 2 +
> >> drivers/bluetooth/cg2900_hci.c | 896 ++++++++++++++++++++++++++++++++++++++++
> >> 3 files changed, 905 insertions(+), 0 deletions(-)
> >> create mode 100644 drivers/bluetooth/cg2900_hci.c
> >>
> >> diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
> >> index 02deef4..9ca8d69 100644
> >> --- a/drivers/bluetooth/Kconfig
> >> +++ b/drivers/bluetooth/Kconfig
> >> @@ -219,4 +219,11 @@ config BT_ATH3K
> >> Say Y here to compile support for "Atheros firmware download driver"
> >> into the kernel or say M to compile it as module (ath3k).
> >>
> >> +config BT_CG2900
> >> + tristate "ST-Ericsson CG2900 driver"
> >> + depends on MFD_CG2900 && BT
> >> + help
> >> + Select if ST-Ericsson CG2900 Connectivity controller shall be used as
> >> + Bluetooth controller for BlueZ.
> >> +
> >> endmenu
> >> diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
> >> index 71bdf13..a479c16 100644
> >> --- a/drivers/bluetooth/Makefile
> >> +++ b/drivers/bluetooth/Makefile
> >> @@ -19,6 +19,8 @@ obj-$(CONFIG_BT_ATH3K) += ath3k.o
> >> obj-$(CONFIG_BT_MRVL) += btmrvl.o
> >> obj-$(CONFIG_BT_MRVL_SDIO) += btmrvl_sdio.o
> >>
> >> +obj-$(CONFIG_BT_CG2900) += cg2900_hci.o
> >> +
> >
> > Please sort this after ath3k and before btmvrl config.
> >
> > And for the name either just use cg2900 if it uniquely identifies the
> > Bluetooth chip used in the combo or use btcg2900.o as module name if it
> > is the name of the combo module.
> >
> > We clearly moved into the direction of either unique hardware names
> > without bt prefix or we have the bt prefix in front of it. The fully
> > generic term hci will be phased out of the driver names.
> >
>
> I can change the name to btcg2900.o since the whole chip is called
> cg2900 and this file is just for the BT part of it.
> I will also sort it correctly into the file.
Actually btcg2900.ko sounds good to me.
> >> btmrvl-y := btmrvl_main.o
> >> btmrvl-$(CONFIG_DEBUG_FS) += btmrvl_debugfs.o
> >>
> >> diff --git a/drivers/bluetooth/cg2900_hci.c b/drivers/bluetooth/cg2900_hci.c
> >> new file mode 100644
> >> index 0000000..de1ada8
> >> --- /dev/null
> >> +++ b/drivers/bluetooth/cg2900_hci.c
> >> @@ -0,0 +1,896 @@
> >> +/*
> >> + * drivers/bluetooth/cg2900_hci.c
> >> + *
> >> + * Copyright (C) ST-Ericsson SA 2010
> >> + * Authors:
> >> + * Par-Gunnar Hjalmdahl (par-gunnar.p.hjalmdahl@stericsson.com) for
> >> ST-Ericsson.
> >> + * Henrik Possung (henrik.possung@stericsson.com) for ST-Ericsson.
> >> + * Josef Kindberg (josef.kindberg@stericsson.com) for ST-Ericsson.
> >> + * Dariusz Szymszak (dariusz.xd.szymczak@stericsson.com) for ST-Ericsson.
> >> + * Kjell Andersson (kjell.k.andersson@stericsson.com) for ST-Ericsson.
> >> + * License terms: GNU General Public License (GPL), version 2
> >> + *
> >> + * Linux Bluetooth HCI H:4 Driver for ST-Ericsson CG2900 connectivity
> >> controller
> >> + * towards the BlueZ Bluetooth stack.
> >> + */
> >
> > Drop the filename in this header and don't bother with the description
> > towards BlueZ or Bluetooth subsystem. That is clear from the location of
> > the file.
> >
> > Also what is up with the "for ST-Ericsson"?
> >
>
> OK regarding the header comments. We have been instructed to use "for
> ST-Ericsson" when writing author name. Is that a problem?
I find this fully pointless since a) you have an stericsson.com email
address and b) the code is copyright by ST-Ericsson. So my take is, yes,
we get it ST-Ericsson has written this code.
> >> +
> >> +#include <linux/module.h>
> >> +#include <linux/types.h>
> >> +#include <linux/skbuff.h>
> >> +#include <asm/byteorder.h>
> >> +#include <net/bluetooth/bluetooth.h>
> >> +#include <net/bluetooth/hci.h>
> >> +#include <net/bluetooth/hci_core.h>
> >> +
> >> +#include <linux/workqueue.h>
> >> +#include <linux/wait.h>
> >> +#include <linux/time.h>
> >> +#include <linux/jiffies.h>
> >> +#include <linux/sched.h>
> >> +#include <linux/timer.h>
> >> +
> >> +#include <linux/mfd/cg2900.h>
> >> +#include <mach/cg2900_devices.h>
> >> +
> >> +/* module_param declared in cg2900_core.c */
> >> +extern int cg2900_debug_level;
> >> +
> >> +#define NAME "CG2900 HCI"
> >> +
> >> +/* Debug defines */
> >> +#define CG2900_DBG_DATA(fmt, arg...) \
> >> +do { \
> >> + if (cg2900_debug_level >= 25) \
> >> + printk(KERN_DEBUG NAME " %s: " fmt "\n" , __func__ , ## arg); \
> >> +} while (0)
> >> +
> >> +#define CG2900_DBG(fmt, arg...) \
> >> +do { \
> >> + if (cg2900_debug_level >= 20) \
> >> + printk(KERN_DEBUG NAME " %s: " fmt "\n" , __func__ , ## arg); \
> >> +} while (0)
> >> +
> >> +#define CG2900_INFO(fmt, arg...) \
> >> +do { \
> >> + if (cg2900_debug_level >= 10) \
> >> + printk(KERN_INFO NAME ": " fmt "\n" , ## arg); \
> >> +} while (0)
> >> +
> >> +#define CG2900_ERR(fmt, arg...) \
> >> +do { \
> >> + if (cg2900_debug_level >= 1) \
> >> + printk(KERN_ERR NAME " %s: " fmt "\n" , __func__ , ## arg); \
> >> +} while (0)
> >
> > Remove all this debug stuff. Use BT_DBG, BT_ERR etc.
>
> OK
>
> >
> >> +#define CG2900_SET_STATE(__name, __var, __new_state) \
> >> +do { \
> >> + CG2900_DBG("New %s: 0x%X", __name, (uint32_t)__new_state); \
> >> + __var = __new_state; \
> >> +} while (0)
> >
> > What is this for? Please don't do that. It just obfuscates the code.
> >
>
> Is it OK to use inline functions instead of defines? It's pretty
> useful to be able to printout when changing state.
> So something like:
>
> static inline void set_reset_state(enum reset_state new_state)
> {
> BT_DBG("New reset state: %x", new_state);
> hci_info->reset_state = new_state;
> }
Not really. It just clutters the code. Do the debugging with dynamic
debug and keep the code readable.
I do see where you are coming from and it might be useful during initial
development. For longterm maintainability this is not helping.
> >> +/* HCI device type */
> >> +#define HCI_CG2900 HCI_VIRTUAL
> >
> > This is the wrong type. Don't do that. And don't create a new define for
> > it. Use the standard types. I assume that HCI_UART is most likely what
> > you want here.
> >
>
> The problem here is that the physical transport used is not shown for
> this module. It is handled with the CG2900 MFD driver. But I guess I
> could expose the type through an API function.
Yes, please do that instead, but essentially HCI_UART is a way better
fit than HCI_VIRTUAL if you are in doubt.
> >> +/* Wait for 5 seconds for a response to our requests */
> >> +#define RESP_TIMEOUT 5000
> >> +
> >> +/* State-setting defines */
> >> +#define SET_RESET_STATE(__hci_reset_new_state) \
> >> + CG2900_SET_STATE("reset_state", hci_info->reset_state, \
> >> + __hci_reset_new_state)
> >> +#define SET_ENABLE_STATE(__hci_enable_new_state) \
> >> + CG2900_SET_STATE("enable_state", hci_info->enable_state, \
> >> + __hci_enable_new_state)
> >
> > Don't do this. It is just highly obfuscating the code flow.
> >
>
> As stated above, is it OK to use static inline functions instead?
No. Just fix the code to do it properly there. You will see that it is
just fine.
> >> +/* Bluetooth error codes */
> >> +#define HCI_ERR_NO_ERROR 0x00
> >> +#define HCI_ERR_CMD_DISALLOWED 0x0C
> >> +
> >> +/**
> >> + * enum reset_state - RESET-states of the HCI driver.
> >> + *
> >> + * @RESET_IDLE: No reset in progress.
> >> + * @RESET_ACTIVATED: Reset in progress.
> >> + * @RESET_UNREGISTERED: hdev is unregistered.
> >> + */
> >> +
> >> +enum reset_state {
> >> + RESET_IDLE,
> >> + RESET_ACTIVATED,
> >> + RESET_UNREGISTERED
> >> +};
> >> +
> >> +/**
> >> + * enum enable_state - ENABLE-states of the HCI driver.
> >> + *
> >> + * @ENABLE_IDLE: The HCI driver is loaded but not opened.
> >> + * @ENABLE_WAITING_BT_ENABLED_CC: The HCI driver is waiting for a command
> >> + * complete event from the BT chip as a
> >> + * response to a BT Enable (true) command.
> >> + * @ENABLE_BT_ENABLED: The BT chip is enabled.
> >> + * @ENABLE_WAITING_BT_DISABLED_CC: The HCI driver is waiting for a command
> >> + * complete event from the BT chip as a
> >> + * response to a BT Enable (false) command.
> >> + * @ENABLE_BT_DISABLED: The BT chip is disabled.
> >> + * @ENABLE_BT_ERROR: The HCI driver is in a bad state, some
> >> + * thing has failed and is not expected to
> >> + * work properly.
> >> + */
> >> +enum enable_state {
> >> + ENABLE_IDLE,
> >> + ENABLE_WAITING_BT_ENABLED_CC,
> >> + ENABLE_BT_ENABLED,
> >> + ENABLE_WAITING_BT_DISABLED_CC,
> >> + ENABLE_BT_DISABLED,
> >> + ENABLE_BT_ERROR
> >> +};
> >> +
> >> +/**
> >> + * struct hci_info - Specifies HCI driver private data.
> >> + *
> >> + * This type specifies CG2900 HCI driver private data.
> >> + *
> >> + * @bt_cmd: Device structure for BT command channel.
> >> + * @bt_evt: Device structure for BT event channel.
> >> + * @bt_acl: Device structure for BT ACL channel.
> >> + * @hdev: Device structure for HCI device.
> >> + * @reset_state: Device enum for HCI driver reset state.
> >> + * @enable_state: Device enum for HCI driver BT enable state.
> >> + */
> >> +struct hci_info {
> >> + struct cg2900_device *bt_cmd;
> >> + struct cg2900_device *bt_evt;
> >> + struct cg2900_device *bt_acl;
> >> + struct hci_dev *hdev;
> >> + enum reset_state reset_state;
> >> + enum enable_state enable_state;
> >> +};
> >> +
> >> +/**
> >> + * struct dev_info - Specifies private data used when receiving
> >> callbacks from CPD.
> >> + *
> >> + * @hdev: Device structure for HCI device.
> >> + * @hci_data_type: Type of data according to BlueZ.
> >> + */
> >> +struct dev_info {
> >> + struct hci_dev *hdev;
> >> + u8 hci_data_type;
> >> +};
> >> +
> >> +/* Variables where LSB and MSB for bt_enable command is stored */
> >> +static u16 bt_enable_cmd;
> >> +
> >> +static struct hci_info *hci_info;
> >> +
> >> +/*
> >> + * hci_wait_queue - Main Wait Queue in HCI driver.
> >> + */
> >> +static DECLARE_WAIT_QUEUE_HEAD(hci_wait_queue);
> >> +
> >> +/* Internal function declarations */
> >> +static int register_to_bluez(void);
> >
> > Don't use bluez in kernel code. Just use bluetooth or bt.
> >
>
> OK
>
> >> +/* Internal functions */
> >> +
> >> +/**
> >> + * remove_bt_users() - Unregister and remove any existing BT users.
> >> + * @info: HCI driver info structure.
> >> + */
> >> +static void remove_bt_users(struct hci_info *info)
> >> +{
> >> + if (info->bt_cmd) {
> >> + kfree(info->bt_cmd->user_data);
> >> + info->bt_cmd->user_data = NULL;
> >> + cg2900_deregister_user(info->bt_cmd);
> >> + info->bt_cmd = NULL;
> >> + }
> >> +
> >> + if (info->bt_evt) {
> >> + kfree(info->bt_evt->user_data);
> >> + info->bt_evt->user_data = NULL;
> >> + cg2900_deregister_user(info->bt_evt);
> >> + info->bt_evt = NULL;
> >> + }
> >> +
> >> + if (info->bt_acl) {
> >> + kfree(info->bt_acl->user_data);
> >> + info->bt_acl->user_data = NULL;
> >> + cg2900_deregister_user(info->bt_acl);
> >> + info->bt_acl = NULL;
> >> + }
> >> +}
> >> +
> >> +/**
> >> + * hci_read_cb() - Callback for handling data received from CG2900 driver.
> >> + * @dev: Device receiving data.
> >> + * @skb: Buffer with data coming from device.
> >> + */
> >> +static void hci_read_cb(struct cg2900_device *dev, struct sk_buff *skb)
> >> +{
> >> + int err = 0;
> >> + struct dev_info *dev_info;
> >> + struct hci_event_hdr *evt;
> >> + struct hci_ev_cmd_complete *cmd_complete;
> >> + struct hci_ev_cmd_status *cmd_status;
> >> + u8 status;
> >> +
> >> + if (!skb) {
> >> + CG2900_ERR("NULL supplied for skb");
> >> + return;
> >> + }
> >> +
> >> + if (!dev) {
> >> + CG2900_ERR("dev == NULL");
> >> + goto fin_free_skb;
> >> + }
> >> +
> >> + dev_info = (struct dev_info *)dev->user_data;
> >> +
> >> + if (!dev_info) {
> >> + CG2900_ERR("dev_info == NULL");
> >> + goto fin_free_skb;
> >> + }
> >> +
> >> + evt = (struct hci_event_hdr *)skb->data;
> >> + cmd_complete = (struct hci_ev_cmd_complete *)(skb->data + sizeof(*evt));
> >> + cmd_status = (struct hci_ev_cmd_status *)(skb->data + sizeof(*evt));
> >> +
> >> + /*
> >> + * Check if HCI Driver it self is expecting a Command Complete packet
> >> + * from the chip after a BT Enable command.
> >> + */
> >> + if ((hci_info->enable_state == ENABLE_WAITING_BT_ENABLED_CC ||
> >> + hci_info->enable_state == ENABLE_WAITING_BT_DISABLED_CC) &&
> >> + hci_info->bt_evt->h4_channel == dev->h4_channel &&
> >> + evt->evt == HCI_EV_CMD_COMPLETE &&
> >> + le16_to_cpu(cmd_complete->opcode) == bt_enable_cmd) {
> >> + /*
> >> + * This is the command complete event for
> >> + * the HCI_Cmd_VS_Bluetooth_Enable.
> >> + * Check result and update state.
> >> + *
> >> + * The BT chip is enabled/disabled. Either it was enabled/
> >> + * disabled now (status NO_ERROR) or it was already enabled/
> >> + * disabled (assuming status CMD_DISALLOWED is already enabled/
> >> + * disabled).
> >> + */
> >> + status = *(skb->data + sizeof(*evt) + sizeof(*cmd_complete));
> >> + if (status != HCI_ERR_NO_ERROR &&
> >> + status != HCI_ERR_CMD_DISALLOWED) {
> >> + CG2900_ERR("Could not enable/disable BT core (0x%X)",
> >> + status);
> >> + SET_ENABLE_STATE(ENABLE_BT_ERROR);
> >> + goto fin_free_skb;
> >> + }
> >> +
> >> + if (hci_info->enable_state == ENABLE_WAITING_BT_ENABLED_CC) {
> >> + SET_ENABLE_STATE(ENABLE_BT_ENABLED);
> >> + CG2900_INFO("BT core is enabled");
> >> + } else {
> >> + SET_ENABLE_STATE(ENABLE_BT_DISABLED);
> >> + CG2900_INFO("BT core is disabled");
> >> + }
> >> +
> >> + /* Wake up whom ever is waiting for this result. */
> >> + wake_up_interruptible(&hci_wait_queue);
> >> + goto fin_free_skb;
> >> + } else if ((hci_info->enable_state == ENABLE_WAITING_BT_DISABLED_CC ||
> >> + hci_info->enable_state == ENABLE_WAITING_BT_ENABLED_CC) &&
> >> + hci_info->bt_evt->h4_channel == dev->h4_channel &&
> >> + evt->evt == HCI_EV_CMD_STATUS &&
> >> + le16_to_cpu(cmd_status->opcode) == bt_enable_cmd) {
> >> + /*
> >> + * Clear the status events since the Bluez is not expecting
> >> + * them.
> >> + */
> >> + CG2900_DBG("HCI Driver received Command Status(for "
> >> + "BT enable): 0x%X", cmd_status->status);
> >> + /*
> >> + * This is the command status event for
> >> + * the HCI_Cmd_VS_Bluetooth_Enable.
> >> + * Just free the packet.
> >> + */
> >> + goto fin_free_skb;
> >> + } else {
> >> + bt_cb(skb)->pkt_type = dev_info->hci_data_type;
> >> + skb->dev = (struct net_device *)dev_info->hdev;
> >> + /* Update BlueZ stats */
> >> + dev_info->hdev->stat.byte_rx += skb->len;
> >> + if (bt_cb(skb)->pkt_type == HCI_ACLDATA_PKT)
> >> + dev_info->hdev->stat.acl_rx++;
> >> + else
> >> + dev_info->hdev->stat.evt_rx++;
> >> +
> >> + CG2900_DBG_DATA("Data receive %d bytes", skb->len);
> >> +
> >> + /* Provide BlueZ with received frame*/
> >> + err = hci_recv_frame(skb);
> >> + /* If err, skb have been freed in hci_recv_frame() */
> >> + if (err)
> >> + CG2900_ERR("Failed in supplying packet to BlueZ (%d)",
> >> + err);
> >> + }
> >> +
> >> + return;
> >> +
> >> +fin_free_skb:
> >> + kfree_skb(skb);
> >> +}
> >> +
> >> +/**
> >> + * hci_reset_cb() - Callback for handling reset from CG2900 driver.
> >> + * @dev: CPD device resetting.
> >> + */
> >> +static void hci_reset_cb(struct cg2900_device *dev)
> >> +{
> >> + int err;
> >> + struct hci_dev *hdev;
> >> + struct dev_info *dev_info;
> >> + struct hci_info *info;
> >> +
> >> + CG2900_INFO("BluezDriver: hci_reset_cb");
> >> +
> >> + if (!dev) {
> >> + CG2900_ERR("NULL supplied for dev");
> >> + return;
> >> + }
> >> +
> >> + dev_info = (struct dev_info *)dev->user_data;
> >> + if (!dev_info) {
> >> + CG2900_ERR("NULL supplied for dev_info");
> >> + return;
> >> + }
> >> +
> >> + hdev = dev_info->hdev;
> >> + if (!hdev) {
> >> + CG2900_ERR("NULL supplied for hdev");
> >> + return;
> >> + }
> >> +
> >> + info = (struct hci_info *)hdev->driver_data;
> >> + if (!info) {
> >> + CG2900_ERR("NULL supplied for driver_data");
> >> + return;
> >> + }
> >> +
> >> + switch (dev_info->hci_data_type) {
> >> +
> >> + case HCI_EVENT_PKT:
> >> + info->bt_evt = NULL;
> >> + break;
> >> +
> >> + case HCI_COMMAND_PKT:
> >> + info->bt_cmd = NULL;
> >> + break;
> >> +
> >> + case HCI_ACLDATA_PKT:
> >> + info->bt_acl = NULL;
> >> + break;
> >> +
> >> + default:
> >> + CG2900_ERR("Unknown HCI data type:%d",
> >> + dev_info->hci_data_type);
> >> + return;
> >> + }
> >> +
> >> + SET_RESET_STATE(RESET_ACTIVATED);
> >> +
> >> + /* Free userdata as device info structure will be freed by CG2900
> >> + * when this callback returns */
> >> + kfree(dev->user_data);
> >> + dev->user_data = NULL;
> >> +
> >> + /*
> >> + * Continue to deregister hdev if all channels has been reset else
> >> + * return.
> >> + */
> >> + if (info->bt_evt || info->bt_cmd || info->bt_acl)
> >> + return;
> >> +
> >> + /*
> >> + * Deregister HCI device. Close and Destruct functions should
> >> + * in turn be called by BlueZ.
> >> + */
> >> + CG2900_INFO("Deregister HCI device");
> >> + err = hci_unregister_dev(hdev);
> >> + if (err)
> >> + CG2900_ERR("Can not deregister HCI device! (%d)", err);
> >> + /*
> >> + * Now we are in trouble. Try to register a new hdev
> >> + * anyway even though this will cost some memory.
> >> + */
> >> +
> >> + wait_event_interruptible_timeout(hci_wait_queue,
> >> + (RESET_UNREGISTERED == hci_info->reset_state),
> >> + msecs_to_jiffies(RESP_TIMEOUT));
> >> + if (RESET_UNREGISTERED != hci_info->reset_state)
> >> + /*
> >> + * Now we are in trouble. Try to register a new hdev
> >> + * anyway even though this will cost some memory.
> >> + */
> >> + CG2900_ERR("Timeout expired. Could not deregister HCI device");
> >> +
> >> + /* Init and register hdev */
> >> + CG2900_INFO("Register HCI device");
> >> + err = register_to_bluez();
> >> + if (err)
> >> + CG2900_ERR("HCI Device registration error (%d).", err);
> >> +}
> >> +
> >> +/*
> >> + * struct cg2900_cb - Specifies callback structure for CG2900 user.
> >> + *
> >> + * @read_cb: Callback function called when data is received from
> >> + * the controller.
> >> + * @reset_cb: Callback function called when the controller has been reset.
> >> + */
> >> +static struct cg2900_callbacks cg2900_cb = {
> >> + .read_cb = hci_read_cb,
> >> + .reset_cb = hci_reset_cb
> >> +};
> >> +
> >> +/**
> >> + * open_from_hci() - Open HCI interface.
> >> + * @hdev: HCI device being opened.
> >> + *
> >> + * BlueZ callback function for opening HCI interface to device.
> >> + *
> >> + * Returns:
> >> + * 0 if there is no error.
> >> + * -EINVAL if NULL pointer is supplied.
> >> + * -EOPNOTSUPP if supplied packet type is not supported.
> >> + * -EBUSY if device is already opened.
> >> + * -EACCES if opening of channels failed.
> >> + */
> >> +static int open_from_hci(struct hci_dev *hdev)
> >> +{
> >> + struct hci_info *info;
> >> + struct dev_info *dev_info;
> >> + struct sk_buff *enable_cmd;
> >> + int err;
> >> +
> >> + CG2900_INFO("Open ST-Ericsson connectivity HCI driver");
> >> +
> >> + if (!hdev) {
> >> + CG2900_ERR("NULL supplied for hdev");
> >> + return -EINVAL;
> >> + }
> >> +
> >> + info = (struct hci_info *)hdev->driver_data;
> >> + if (!info) {
> >> + CG2900_ERR("NULL supplied for driver_data");
> >> + return -EINVAL;
> >> + }
> >> +
> >> + if (test_and_set_bit(HCI_RUNNING, &(hdev->flags))) {
> >> + CG2900_ERR("Device already opened!");
> >> + return -EBUSY;
> >> + }
> >> +
> >> + if (!(info->bt_cmd)) {
> >> + info->bt_cmd = cg2900_register_user(CG2900_BT_CMD,
> >> + &cg2900_cb);
> >> + if (info->bt_cmd) {
> >> + dev_info = kmalloc(sizeof(*dev_info), GFP_KERNEL);
> >> + if (dev_info) {
> >> + dev_info->hdev = hdev;
> >> + dev_info->hci_data_type = HCI_COMMAND_PKT;
> >> + }
> >> + info->bt_cmd->user_data = dev_info;
> >> + } else {
> >> + CG2900_ERR("Couldn't register CG2900_BT_CMD to CG2900");
> >> + err = -EACCES;
> >> + goto handle_error;
> >> + }
> >> + }
> >> +
> >> + if (!(info->bt_evt)) {
> >> + info->bt_evt = cg2900_register_user(CG2900_BT_EVT,
> >> + &cg2900_cb);
> >> + if (info->bt_evt) {
> >> + dev_info = kmalloc(sizeof(*dev_info), GFP_KERNEL);
> >> + if (dev_info) {
> >> + dev_info->hdev = hdev;
> >> + dev_info->hci_data_type = HCI_EVENT_PKT;
> >> + }
> >> + info->bt_evt->user_data = dev_info;
> >> + } else {
> >> + CG2900_ERR("Couldn't register CG2900_BT_EVT to CG2900");
> >> + err = -EACCES;
> >> + goto handle_error;
> >> + }
> >> + }
> >> +
> >> + if (!(info->bt_acl)) {
> >> + info->bt_acl = cg2900_register_user(CG2900_BT_ACL,
> >> + &cg2900_cb);
> >> + if (info->bt_acl) {
> >> + dev_info = kmalloc(sizeof(*dev_info), GFP_KERNEL);
> >> + if (dev_info) {
> >> + dev_info->hdev = hdev;
> >> + dev_info->hci_data_type = HCI_ACLDATA_PKT;
> >> + }
> >> + info->bt_acl->user_data = dev_info;
> >> + } else {
> >> + CG2900_ERR("Couldn't register CG2900_BT_ACL to CG2900");
> >> + err = -EACCES;
> >> + goto handle_error;
> >> + }
> >> + }
> >> +
> >> + if (info->reset_state == RESET_ACTIVATED)
> >> + SET_RESET_STATE(RESET_IDLE);
> >> +
> >> + /*
> >> + * Call platform specific function that returns the chip dependent
> >> + * bt enable(true) HCI command.
> >> + * If NULL is returned, then no bt_enable command should be sent to the
> >> + * chip.
> >> + */
> >> + enable_cmd = cg2900_devices_get_bt_enable_cmd(&bt_enable_cmd, true);
> >> + if (!enable_cmd) {
> >> + /* The chip is enabled by default */
> >> + SET_ENABLE_STATE(ENABLE_BT_ENABLED);
> >> + return 0;
> >> + }
> >> +
> >> + /* Set the HCI state before sending command to chip. */
> >> + SET_ENABLE_STATE(ENABLE_WAITING_BT_ENABLED_CC);
> >> +
> >> + /* Send command to chip */
> >> + cg2900_write(info->bt_cmd, enable_cmd);
> >> +
> >> + /*
> >> + * Wait for callback to receive command complete and then wake us up
> >> + * again.
> >> + */
> >> + wait_event_interruptible_timeout(hci_wait_queue,
> >> + (info->enable_state == ENABLE_BT_ENABLED),
> >> + msecs_to_jiffies(RESP_TIMEOUT));
> >> + /* Check the current state to se that it worked. */
> >> + if (info->enable_state != ENABLE_BT_ENABLED) {
> >> + CG2900_ERR("Could not enable BT core (%d)", info->enable_state);
> >> + err = -EACCES;
> >> + SET_ENABLE_STATE(ENABLE_BT_DISABLED);
> >> + goto handle_error;
> >> + }
> >> +
> >> + return 0;
> >> +
> >> +handle_error:
> >> + remove_bt_users(info);
> >> + clear_bit(HCI_RUNNING, &(hdev->flags));
> >> + return err;
> >> +
> >> +}
> >> +
> >> +/**
> >> + * flush_from_hci() - Flush HCI interface.
> >> + * @hdev: HCI device being flushed.
> >> + *
> >> + * Returns:
> >> + * 0 if there is no error.
> >> + * -EINVAL if NULL pointer is supplied.
> >> + */
> >> +static int flush_from_hci(struct hci_dev *hdev)
> >> +{
> >> + CG2900_INFO("flush_from_hci");
> >> +
> >> + if (!hdev) {
> >> + CG2900_ERR("NULL supplied for hdev");
> >> + return -EINVAL;
> >> + }
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +/**
> >> + * close_from_hci() - Close HCI interface.
> >> + * @hdev: HCI device being closed.
> >> + *
> >> + * BlueZ callback function for closing HCI interface.
> >> + * It flushes the interface first.
> >> + *
> >> + * Returns:
> >> + * 0 if there is no error.
> >> + * -EINVAL if NULL pointer is supplied.
> >> + * -EOPNOTSUPP if supplied packet type is not supported.
> >> + * -EBUSY if device is not opened.
> >> + */
> >> +static int close_from_hci(struct hci_dev *hdev)
> >> +{
> >> + struct hci_info *info = NULL;
> >> + struct sk_buff *enable_cmd;
> >> +
> >> + CG2900_INFO("close_from_hci");
> >> +
> >> + if (!hdev) {
> >> + CG2900_ERR("NULL supplied for hdev");
> >> + return -EINVAL;
> >> + }
> >> +
> >> + info = (struct hci_info *)hdev->driver_data;
> >> + if (!info) {
> >> + CG2900_ERR("NULL supplied for driver_data");
> >> + return -EINVAL;
> >> + }
> >> +
> >> + if (!test_and_clear_bit(HCI_RUNNING, &(hdev->flags))) {
> >> + CG2900_ERR("Device already closed!");
> >> + return -EBUSY;
> >> + }
> >> +
> >> + flush_from_hci(hdev);
> >> +
> >> + /* Do not do this if there is an reset ongoing */
> >> + if (hci_info->reset_state == RESET_ACTIVATED)
> >> + goto remove_users;
> >> +
> >> + /*
> >> + * Get the chip dependent BT Enable HCI command. The command parameter
> >> + * shall be set to false to disable the BT core.
> >> + * If NULL is returned, then no BT Enable command should be sent to the
> >> + * chip.
> >> + */
> >> + enable_cmd = cg2900_devices_get_bt_enable_cmd(&bt_enable_cmd, false);
> >> + if (!enable_cmd) {
> >> + /*
> >> + * The chip is enabled by default and we should not disable it.
> >> + */
> >> + SET_ENABLE_STATE(ENABLE_BT_ENABLED);
> >> + goto remove_users;
> >> + }
> >> +
> >> + /* Set the HCI state before sending command to chip */
> >> + SET_ENABLE_STATE(ENABLE_WAITING_BT_DISABLED_CC);
> >> +
> >> + /* Send command to chip */
> >> + cg2900_write(info->bt_cmd, enable_cmd);
> >> +
> >> + /*
> >> + * Wait for callback to receive command complete and then wake us up
> >> + * again.
> >> + */
> >> + wait_event_interruptible_timeout(hci_wait_queue,
> >> + (info->enable_state == ENABLE_BT_DISABLED),
> >> + msecs_to_jiffies(RESP_TIMEOUT));
> >> + /* Check the current state to se that it worked. */
> >> + if (info->enable_state != ENABLE_BT_DISABLED) {
> >> + CG2900_ERR("Could not disable BT core.");
> >> + SET_ENABLE_STATE(ENABLE_BT_ENABLED);
> >> + }
> >> +
> >> +remove_users:
> >> + /* Finally deregister all users and free allocated data */
> >> + remove_bt_users(info);
> >> + return 0;
> >> +}
> >> +
> >> +/**
> >> + * send_from_hci() - Send packet to device.
> >> + * @skb: sk buffer to be sent.
> >> + *
> >> + * BlueZ callback function for sending sk buffer.
> >> + *
> >> + * Returns:
> >> + * 0 if there is no error.
> >> + * -EINVAL if NULL pointer is supplied.
> >> + * -EOPNOTSUPP if supplied packet type is not supported.
> >> + * Error codes from cg2900_write.
> >> + */
> >> +static int send_from_hci(struct sk_buff *skb)
> >> +{
> >> + struct hci_dev *hdev;
> >> + struct hci_info *info;
> >> + int err = 0;
> >> +
> >> + if (!skb) {
> >> + CG2900_ERR("NULL supplied for skb");
> >> + return -EINVAL;
> >> + }
> >> +
> >> + hdev = (struct hci_dev *)(skb->dev);
> >> + if (!hdev) {
> >> + CG2900_ERR("NULL supplied for hdev");
> >> + return -EINVAL;
> >> + }
> >> +
> >> + info = (struct hci_info *)hdev->driver_data;
> >> + if (!info) {
> >> + CG2900_ERR("NULL supplied for info");
> >> + return -EINVAL;
> >> + }
> >> +
> >> + /* Update BlueZ stats */
> >> + hdev->stat.byte_tx += skb->len;
> >> +
> >> + CG2900_DBG_DATA("Data transmit %d bytes", skb->len);
> >> +
> >> + switch (bt_cb(skb)->pkt_type) {
> >> + case HCI_COMMAND_PKT:
> >> + CG2900_DBG("Sending HCI_COMMAND_PKT");
> >> + err = cg2900_write(info->bt_cmd, skb);
> >> + hdev->stat.cmd_tx++;
> >> + break;
> >> + case HCI_ACLDATA_PKT:
> >> + CG2900_DBG("Sending HCI_ACLDATA_PKT");
> >> + err = cg2900_write(info->bt_acl, skb);
> >> + hdev->stat.acl_tx++;
> >> + break;
> >> + default:
> >> + CG2900_ERR("Trying to transmit unsupported packet type "
> >> + "(0x%.2X)", bt_cb(skb)->pkt_type);
> >> + err = -EOPNOTSUPP;
> >> + break;
> >> + };
> >> +
> >> + return err;
> >> +}
> >> +
> >> +/**
> >> + * destruct_from_hci() - Destruct HCI interface.
> >> + * @hdev: HCI device being destructed.
> >> + */
> >> +static void destruct_from_hci(struct hci_dev *hdev)
> >> +{
> >> + CG2900_INFO("destruct_from_hci");
> >> +
> >> + if (!hci_info)
> >> + return;
> >> +
> >> + /* When being reset, register a new hdev when hdev is deregistered */
> >> + if (hci_info->reset_state == RESET_ACTIVATED) {
> >> + if (hci_info->hdev)
> >> + hci_free_dev(hci_info->hdev);
> >> + SET_RESET_STATE(RESET_UNREGISTERED);
> >> + }
> >> +}
> >
> > Please name thse cg2900_destruct or btcg2900_destruct. And not from_hci.
> > Follow how the other Bluetooth drivers do the naming.
> >
> > And the destruct callback is for freeing memory. I am also not sure how
> > reset and unregister/re-register HCI is a good idea.
> >
>
> Regarding naming: no problem, we'll change that. We will see how we
> solve the reset. It's a bit tricky to get it working correctly, but
> we'll see what we can do. We have to be able in some way to handle if
> another stack, such as GPS or FM, suddenly reset the chip.
It sounds wrong to me, but please enlighten me.
> >> +/**
> >> + * notify_from_hci() - Notification from the HCI interface.
> >> + * @hdev: HCI device notifying.
> >> + * @evt: Notification event.
> >> + */
> >> +static void notify_from_hci(struct hci_dev *hdev, unsigned int evt)
> >> +{
> >> + CG2900_INFO("notify_from_hci - evt = 0x%.2X", evt);
> >> +}
> >
> > If you don't use it, then just leave it out.
> >
>
> OK
>
> >> +/**
> >> + * ioctl_from_hci() - Handle IOCTL call to the HCI interface.
> >> + * @hdev: HCI device.
> >> + * @cmd: IOCTL command.
> >> + * @arg: IOCTL argument.
> >> + *
> >> + * Returns:
> >> + * -EINVAL if NULL is supplied for hdev.
> >> + * -EPERM otherwise since current driver supports no IOCTL.
> >> + */
> >> +static int ioctl_from_hci(struct hci_dev *hdev, unsigned int cmd,
> >> + unsigned long arg)
> >> +{
> >> + CG2900_INFO("ioctl_from_hci - cmd 0x%X", cmd);
> >> + CG2900_DBG("DIR: %d, TYPE: %d, NR: %d, SIZE: %d",
> >> + _IOC_DIR(cmd), _IOC_TYPE(cmd), _IOC_NR(cmd),
> >> + _IOC_SIZE(cmd));
> >> +
> >> + if (!hdev) {
> >> + CG2900_ERR("NULL supplied for hdev");
> >> + return -EINVAL;;
> >> + }
> >> +
> >> + return -EPERM;
> >> +}
> >
> > Since you are not using anything with the ioctl, just leave it out. The
> > Bluetooth subsystem will do the right thing.
> >
>
> OK
>
> >> +/**
> >> + * register_to_bluez() - Initialize module.
> >> + *
> >> + * Alloc, init, and register HCI device to BlueZ.
> >> + *
> >> + * Returns:
> >> + * 0 if there is no error.
> >> + * -ENOMEM if allocation fails.
> >> + * Error codes from hci_register_dev.
> >> + */
> >> +static int register_to_bluez(void)
> >> +{
> >> + int err;
> >> +
> >> + hci_info->hdev = hci_alloc_dev();
> >> + if (!hci_info->hdev) {
> >> + CG2900_ERR("Could not allocate mem for HCI driver");
> >> + return -ENOMEM;
> >> + }
> >> +
> >> + hci_info->hdev->bus = HCI_CG2900;
> >> + hci_info->hdev->driver_data = hci_info;
> >> + hci_info->hdev->owner = THIS_MODULE;
> >> + hci_info->hdev->open = open_from_hci;
> >> + hci_info->hdev->close = close_from_hci;
> >> + hci_info->hdev->flush = flush_from_hci;
> >> + hci_info->hdev->send = send_from_hci;
> >> + hci_info->hdev->destruct = destruct_from_hci;
> >> + hci_info->hdev->notify = notify_from_hci;
> >> + hci_info->hdev->ioctl = ioctl_from_hci;
> >> +
> >> + err = hci_register_dev(hci_info->hdev);
> >> + if (err) {
> >> + CG2900_ERR("Can not register HCI device (%d)", err);
> >> + hci_free_dev(hci_info->hdev);
> >> + }
> >> +
> >> + SET_ENABLE_STATE(ENABLE_IDLE);
> >> + SET_RESET_STATE(RESET_IDLE);
> >> +
> >> + return err;
> >> +}
> >
> > So whenever the module is loaded it registers the HCI device. That is
> > not really useful here. This driver needs to be able to be compiled into
> > the kernel (or as module) and run on hardware that doesn't have this
> > chip built in.
> >
>
> OK. We will think this through and solve it in a better way.
The TI guys seem to favoring a platform device. So either you have to do
that or create your own bus.
Regards
Marcel
^ permalink raw reply
* Re: pull request: bluetooth-next-2.6 2010-10-13
From: Gustavo F. Padovan @ 2010-10-13 22:01 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: linville, marcel, linux-wireless, linux-bluetooth, linux-kernel
In-Reply-To: <AANLkTinH38ut+Uj1d+MJXjUQygdpUOVw=7rMKTk-Y-8d@mail.gmail.com>
Hi Luis,
* Luis R. Rodriguez <mcgrof@gmail.com> [2010-10-13 14:41:24 -0700]:
> On Tue, Oct 12, 2010 at 8:48 PM, Gustavo F. Padovan
> <padovan@profusion.mobi> wrote:
> > One notable change is in the MAINTAINERS file, From now and on I'm going
> > to maintain the Bluetooth trees as well as send the pull requests to you.
>
> I noted here you mentioned "as well", does this mean there may be
> updates from two people now instead of just one?
No, updates are coming only from me. You misunderstood it due to my poor
English. I'm going to maitain the trees and send the pull requests.
That's what I said. Sorry, for the bad English. ;)
--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi
^ permalink raw reply
* [PATCH -next] bluetooth: fix hidp kconfig dependency warning
From: Randy Dunlap @ 2010-10-14 1:16 UTC (permalink / raw)
To: lkml; +Cc: akpm, Marcel Holtmann, linux-bluetooth
From: Randy Dunlap <randy.dunlap@oracle.com>
Fix kconfig dependency warning to satisfy dependencies:
warning: (BT_HIDP && NET && BT && BT_L2CAP && INPUT || USB_HID && HID_SUPPORT && USB && INPUT) selects HID which has unmet direct dependencies (HID_SUPPORT && INPUT)
Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
---
net/bluetooth/hidp/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- linux-next-20101013.orig/net/bluetooth/hidp/Kconfig
+++ linux-next-20101013/net/bluetooth/hidp/Kconfig
@@ -1,6 +1,6 @@
config BT_HIDP
tristate "HIDP protocol support"
- depends on BT && BT_L2CAP && INPUT
+ depends on BT && BT_L2CAP && INPUT && HID_SUPPORT
select HID
help
HIDP (Human Interface Device Protocol) is a transport layer
^ permalink raw reply
* Re: [PATCH -next] bluetooth: fix hidp kconfig dependency warning
From: Marcel Holtmann @ 2010-10-14 4:22 UTC (permalink / raw)
To: Randy Dunlap; +Cc: lkml, akpm, linux-bluetooth
In-Reply-To: <20101013181652.7ddf481e.randy.dunlap@oracle.com>
Hi Randy,
> Fix kconfig dependency warning to satisfy dependencies:
>
> warning: (BT_HIDP && NET && BT && BT_L2CAP && INPUT || USB_HID && HID_SUPPORT && USB && INPUT) selects HID which has unmet direct dependencies (HID_SUPPORT && INPUT)
>
> Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
looks fine to me.
Acked-by: Marcel Holtmann <marcel@holtmann.org>
Regards
Marcel
^ permalink raw reply
* Re: Firmware versioning best practices: ath3k-2.fw rename or replace ath3k-1.fw ?
From: Suraj Sumangala @ 2010-10-14 4:23 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: Kevin Hayes, Luis Rodriguez, Marcel Holtmann, Henry Ptasinski,
Suraj Sumangala, David Woodhouse, linux-bluetooth,
linux-kernel@vger.kernel.org, linux-wireless
In-Reply-To: <AANLkTik9oYwspVG4KFbMMq63SpBp++iZe=vxOSQhxRki@mail.gmail.com>
Hi Luis,
On 10/13/2010 11:39 PM, Luis R. Rodriguez wrote:
> Does HCI support uploading firmware? Can't we resolve this blacklist
> crap issue if devices just used HCI to upload firmware?
HCI does not support uploading firmware. But HCI does provide options
for vendor specific commands that can be used for uploading firmware as
long as your device has enough intelligence to understand the command
when it comes.
This is what we do for AR300x serial devices. We do not download
firmware, but we do download all configuration entries as VS HCI commands.
>
> Luis
Regards
Suraj
^ 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