* [RFC 0/1] Implement Compound (Multi-step) GATT Procedure Support
From: Brian Gix @ 2010-12-17 22:48 UTC (permalink / raw)
Cc: rshaffer, padovan, linux-bluetooth
The following two proposed patches implement a method for correctly
staging GATT procedures that require multiple ATT req/resp exchanges.
The first RFC / PATCH is gattrib.[ch], which allows the caller to specify
a gboolean indicating that the command being queued is Compound
(even if the procedure ends up being single/atomic) and a queue ID
number, which allows the caller to cancel the GATT procedure at any
stage of the transaction.
IF -
The ATT opcode being queued is specified as compound, the resulting response
will not cause the next item in the queue to be sent until *after* the
response has been forwarded to the caller via the response callback.
IF -
The ATT opcode being queued is *not* compound, the resulting response
will service the pkt queue immediately (legacy functionality).
IF -
The ID passed to g_attrib_send_seq is ZERO, the command pkt will
be added to the TAIL of the queue, and a new ID assigned to it.
(Legacy functionality)
IF -
The ID passed to g_attrib_send_seq is NON-ZERO, the command pkt
will be added to the HEAD of the queue, causing it to be the next
pkt sent to the remote device. The NON-ZERO ID is also then able
to cancel the command.
The second RFC / PATCH is to gatt.c. It modifies the existing gatt_read_char()
function to recognize the need for a compound Read procedure, and implements
it as a READ_REQ + READ_BLOB_REQ + READ_BLOB_REQ ...<etc> as needed, using
the g_attrib_send_seq() logic from the first RFC / PATCH.
--
Brian Gix
bgix@codeaurora.org
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply
* Re: [PATCH 0/1] Add enc_read_blob_req() as defined in BT Core v4.0
From: Brian Gix @ 2010-12-17 21:01 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1292612933-31095-1-git-send-email-bgix@codeaurora.org>
Hi All
On Fri, 2010-12-17 at 11:08 -0800, Brian Gix wrote:
> Trivial addition of ATT API to encode READ_BLOB_REQ pkt.
>
I apologize for messing up what must seem to be trivial rules for what
information to put where in the patches I am trying to get incorporated.
This particular patch is in fact a trivial addition to the ATT code,
which I believe probably is identical in all respects (including WS I
hope) to how anyone else here would have implemented it.
My follow on to this will be multi-step GATT procedures, I am going to
try as an RFC first, as the concept of GATT procedures that contain more
than one ATT Request/Response will be something that undoubtedly will
cause people to want to offer input on.
I also apologize for the sporadic nature of communication this week, as
my workstation had a catastrophic failure which is taking some time to
recover from.
> --
> Brian Gix
> bgix@codeaurora.org
> Employee of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH] Add enc_read_blob_req() as defined in BT Core Spec v4.0
From: Brian Gix @ 2010-12-17 19:08 UTC (permalink / raw)
Cc: rshaffer, padovan, linux-bluetooth, Brian Gix
In-Reply-To: <1292612933-31095-1-git-send-email-bgix@codeaurora.org>
---
attrib/att.c | 19 +++++++++++++++++++
attrib/att.h | 2 ++
2 files changed, 21 insertions(+), 0 deletions(-)
diff --git a/attrib/att.c b/attrib/att.c
index 445b192..f8dbc02 100644
--- a/attrib/att.c
+++ b/attrib/att.c
@@ -542,6 +542,25 @@ uint16_t enc_read_req(uint16_t handle, uint8_t *pdu, int len)
return min_len;
}
+uint16_t enc_read_blob_req(uint16_t handle, uint16_t offset, uint8_t *pdu,
+ int len)
+{
+ const uint16_t min_len = sizeof(pdu[0]) + sizeof(handle) +
+ sizeof(offset);
+
+ if (pdu == NULL)
+ return 0;
+
+ if (len < min_len)
+ return 0;
+
+ pdu[0] = ATT_OP_READ_BLOB_REQ;
+ att_put_u16(handle, &pdu[1]);
+ att_put_u16(offset, &pdu[3]);
+
+ return min_len;
+}
+
uint16_t dec_read_req(const uint8_t *pdu, int len, uint16_t *handle)
{
const uint16_t min_len = sizeof(pdu[0]) + sizeof(*handle);
diff --git a/attrib/att.h b/attrib/att.h
index 2c8c724..0b8612e 100644
--- a/attrib/att.h
+++ b/attrib/att.h
@@ -202,6 +202,8 @@ uint16_t enc_write_req(uint16_t handle, const uint8_t *value, int vlen,
uint16_t dec_write_req(const uint8_t *pdu, int len, uint16_t *handle,
uint8_t *value, int *vlen);
uint16_t enc_read_req(uint16_t handle, uint8_t *pdu, int len);
+uint16_t enc_read_blob_req(uint16_t handle, uint16_t offset, uint8_t *pdu,
+ int len);
uint16_t dec_read_req(const uint8_t *pdu, int len, uint16_t *handle);
uint16_t enc_read_resp(uint8_t *value, int vlen, uint8_t *pdu, int len);
uint16_t dec_read_resp(const uint8_t *pdu, int len, uint8_t *value, int *vlen);
--
1.7.2.2
^ permalink raw reply related
* [PATCH 0/1] Add enc_read_blob_req() as defined in BT Core v4.0
From: Brian Gix @ 2010-12-17 19:08 UTC (permalink / raw)
Cc: rshaffer, padovan, linux-bluetooth
Trivial addition of ATT API to encode READ_BLOB_REQ pkt.
--
Brian Gix
bgix@codeaurora.org
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply
* [RFC][PATCH] Re: EDR support
From: Manuel Naranjo @ 2010-12-17 16:41 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: BlueZ
In-Reply-To: <1292585241.2658.0.camel@aeonflux>
[-- Attachment #1: Type: text/plain, Size: 2544 bytes --]
Hi Marcel,
> Hi Manuel,
>
>> I was checking the latest BlueZ source code and the kernel source code,
>> and I noticed that by default EDR is disabled, when a dongle is detected
>> net/bluetooth/hci_core.c initializes the packet types with DM1, DH1 and
>> HV1, and then net/bluetooth/hci_event.c does it for DM3-5 DH3-5 and
>> HV3-5, but it never initializes the 2DHx or 3DHx. Was it made on purpose
>> or is it a bug in the code?
> you have read the specification and realized that EDR uses inverse
> values?
I can't find that, but still I could find that according to the 2.1
specs + EDR when you want to enable EDR is up to the master or slave to
ask for the package type switch, that means as far as I understand ask
the other side to enable 2-DH1,2-DH3,2-DH5,3-DH1,3-DH3,3-DH5.
I get from this that from the BlueZ point of view is either the kernel
that should do that, or the BlueZ daemon or just rely that to the user
application.
But I think there are a few of non covered issues here, first when you
attach an EDR dongle which has the EDR (this means is capable of doing
2mbps and 3mbps acl connections the kernel side doesn't show this to the
userland (hciconfig), I made a patch that does exactly that, during the
device recognition of the kernel just read the features provided and or
that to the pkt_type flag. This helps a lot, now my dongle shows it can
do EDR:
EDR compatible:
Features: 0xff 0xff 0x8f 0xfe 0x9b 0xff 0x59 0x83
Packet type: DM1 DM3 DM5 DH1 DH3 DH5 HV1 HV2 HV3 2-DH1 2-DH3
2-DH5 3-DH1 3-DH3 3-DH5
EDR not compatible:
Features: 0xff 0xff 0x8f 0xf8 0x1b 0xf8 0x00 0x80
Packet type: DM1 DM3 DM5 DH1 DH3 DH5 HV1 HV2 HV3
If you think the patch is sane to be included then I format it properly
so far I made it against compat-wireless-2.6.36-4 but it wouldn't be an
issue to make it against the current kernel source.
Next thing was playing with hcitool cpt and that did work, I get speed
improvements, from ~ 60KBs I get up to 100KBs to the same device, just
by forcing it to only do 3-DH5 (which I know is not a good idea, because
it's best for both radios to adapt, based on the environment).
I think it's a good idea to rely EDR "enabling" to the user space,
otherwise connection complexity on the kernel well be way harder to
keep. I think the best would be to add some stuff to the DBUS so first
let the user know which packet types are available, second to allow it
to change on a new connection.
Please let me know what you think about it,
Manuel
[-- Attachment #2: enableEDR.patch --]
[-- Type: text/plain, Size: 2926 bytes --]
diff -uprN compat-wireless-2.6.36-4/include/net/bluetooth/hci.h compat-wireless-2.6.36-4.mine//include/net/bluetooth/hci.h
--- compat-wireless-2.6.36-4/include/net/bluetooth/hci.h 2010-11-08 22:00:51.000000000 -0300
+++ compat-wireless-2.6.36-4.mine//include/net/bluetooth/hci.h 2010-12-11 19:13:14.000000000 -0300
@@ -1,6 +1,7 @@
/*
BlueZ - Bluetooth protocol stack for Linux
Copyright (C) 2000-2001 Qualcomm Incorporated
+ Copyright (C) 2010 Naranjo Manuel Francisco <manuel@aircable.net>
Written 2000,2001 by Maxim Krasnyansky <maxk@qualcomm.com>
@@ -120,17 +121,31 @@ enum {
#define HCI_VENDOR_PKT 0xff
/* HCI packet types */
+#define HCI_2DH1 0x0002
+#define HCI_3DH1 0x0004
#define HCI_DM1 0x0008
-#define HCI_DM3 0x0400
-#define HCI_DM5 0x4000
#define HCI_DH1 0x0010
+#define HCI_2DH3 0x0100
+#define HCI_3DH3 0x0200
+#define HCI_DM3 0x0400
#define HCI_DH3 0x0800
+#define HCI_2DH5 0x1000
+#define HCI_3DH5 0x2000
+#define HCI_DM5 0x4000
#define HCI_DH5 0x8000
#define HCI_HV1 0x0020
#define HCI_HV2 0x0040
#define HCI_HV3 0x0080
+#define HCI_EV3 0x0008
+#define HCI_EV4 0x0010
+#define HCI_EV5 0x0020
+#define HCI_2EV3 0x0040
+#define HCI_3EV3 0x0080
+#define HCI_2EV5 0x0100
+#define HCI_3EV5 0x0200
+
#define SCO_PTYPE_MASK (HCI_HV1 | HCI_HV2 | HCI_HV3)
#define ACL_PTYPE_MASK (~SCO_PTYPE_MASK)
@@ -183,6 +198,12 @@ enum {
#define LMP_PSCHEME 0x02
#define LMP_PCONTROL 0x04
+#define LMP_EDR_ACL_2M 0x02
+#define LMP_EDR_ACL_3M 0x04
+#define LMP_ENH_ISCAN 0x08
+#define LMP_ILACE_ISCAN 0x10
+#define LMP_ILACE_PSCAN 0x20
+#define LMP_RSSI_INQ 0x40
#define LMP_ESCO 0x80
#define LMP_EV4 0x01
diff -uprN compat-wireless-2.6.36-4/net/bluetooth/hci_event.c compat-wireless-2.6.36-4.mine//net/bluetooth/hci_event.c
--- compat-wireless-2.6.36-4/net/bluetooth/hci_event.c 2010-11-08 22:00:51.000000000 -0300
+++ compat-wireless-2.6.36-4.mine//net/bluetooth/hci_event.c 2010-12-11 19:13:46.000000000 -0300
@@ -1,6 +1,7 @@
/*
BlueZ - Bluetooth protocol stack for Linux
Copyright (c) 2000-2001, 2010, Code Aurora Forum. All rights reserved.
+ Copyright (c) 2010 Naranjo Manuel Francisco <manuel@aircable.net>
Written 2000,2001 by Maxim Krasnyansky <maxk@qualcomm.com>
@@ -478,6 +479,26 @@ static void hci_cc_read_local_features(s
if (hdev->features[3] & LMP_ESCO)
hdev->esco_type |= (ESCO_EV3);
+ if (hdev->features[3] & LMP_EDR_ACL_2M){
+ hdev->pkt_type |= (HCI_2DH1);
+
+ if (hdev->features[0] & LMP_3SLOT)
+ hdev->pkt_type |= (HCI_2DH3);
+
+ if (hdev->features[0] & LMP_5SLOT)
+ hdev->pkt_type |= (HCI_2DH5);
+ }
+
+ if (hdev->features[3] & LMP_EDR_ACL_3M){
+ hdev->pkt_type |= (HCI_3DH1);
+
+ if (hdev->features[0] & LMP_3SLOT)
+ hdev->pkt_type |= (HCI_3DH3);
+
+ if (hdev->features[0] & LMP_5SLOT)
+ hdev->pkt_type |= (HCI_3DH5);
+ }
+
if (hdev->features[4] & LMP_EV4)
hdev->esco_type |= (ESCO_EV4);
^ permalink raw reply
* [PATCH 4/4] Move local name parsing to parse_eir_data()
From: Bruna Moreira @ 2010-12-17 14:57 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
In-Reply-To: <1292597844-17135-1-git-send-email-bruna.moreira@openbossa.org>
From: Anderson Lizardo <anderson.lizardo@openbossa.org>
---
src/adapter.c | 12 +++++-------
src/adapter.h | 3 ++-
src/event.c | 34 ++++++++++++++++++++++------------
src/glib-helper.c | 22 ----------------------
src/glib-helper.h | 1 -
5 files changed, 29 insertions(+), 43 deletions(-)
diff --git a/src/adapter.c b/src/adapter.c
index c74019d..b826d4a 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2805,7 +2805,8 @@ static void remove_same_uuid(gpointer data, gpointer user_data)
void adapter_update_device_from_info(struct btd_adapter *adapter,
le_advertising_info *info,
- GSList *services, uint8_t flags)
+ char *name, GSList *services,
+ uint8_t flags)
{
struct remote_dev_info *dev;
bdaddr_t bdaddr;
@@ -2833,12 +2834,9 @@ void adapter_update_device_from_info(struct btd_adapter *adapter,
dev->flags = flags;
- if (info->length) {
- char *tmp_name = bt_extract_eir_name(info->data, NULL);
- if (tmp_name) {
- g_free(dev->name);
- dev->name = tmp_name;
- }
+ if (name) {
+ g_free(dev->name);
+ dev->name = name;
}
/* FIXME: check if other information was changed before emitting the
diff --git a/src/adapter.h b/src/adapter.h
index d5dceb9..52417b6 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -125,7 +125,8 @@ struct remote_dev_info *adapter_search_found_devices(struct btd_adapter *adapter
struct remote_dev_info *match);
void adapter_update_device_from_info(struct btd_adapter *adapter,
le_advertising_info *info,
- GSList *services, uint8_t flags);
+ char *name, GSList *services,
+ uint8_t flags);
void adapter_update_found_devices(struct btd_adapter *adapter, bdaddr_t *bdaddr,
int8_t rssi, uint32_t class, const char *name,
const char *alias, gboolean legacy,
diff --git a/src/event.c b/src/event.c
index 4672106..e138312 100644
--- a/src/event.c
+++ b/src/event.c
@@ -61,6 +61,8 @@
struct eir_data {
GSList *services;
uint8_t flags;
+ char *name;
+ gboolean name_complete;
};
static gboolean get_adapter_and_device(bdaddr_t *src, bdaddr_t *dst,
@@ -350,6 +352,16 @@ static int parse_eir_data(struct eir_data *eir, uint8_t *eir_data,
case EIR_FLAGS:
eir->flags = eir_data[2];
break;
+ case EIR_NAME_SHORT:
+ case EIR_NAME_COMPLETE:
+ if (g_utf8_validate((char *) &eir_data[2],
+ field_len - 1, NULL))
+ eir->name = g_strndup((char *) &eir_data[2],
+ field_len - 1);
+ else
+ eir->name = g_strdup("");
+ eir->name_complete = eir_data[1] == EIR_NAME_COMPLETE;
+ break;
}
len += field_len + 1;
@@ -426,8 +438,8 @@ void btd_event_advertising_report(bdaddr_t *local, le_advertising_info *info)
error("Error parsing advertising data: %s (%d)",
strerror(-err), -err);
- adapter_update_device_from_info(adapter, info, eir_data.services,
- eir_data.flags);
+ adapter_update_device_from_info(adapter, info, eir_data.name,
+ eir_data.services, eir_data.flags);
}
void btd_event_inquiry_result(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
@@ -436,9 +448,8 @@ void btd_event_inquiry_result(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
char filename[PATH_MAX + 1];
struct btd_adapter *adapter;
struct btd_device *device;
- char local_addr[18], peer_addr[18], *alias, *name, *tmp_name;
+ char local_addr[18], peer_addr[18], *alias, *name;
struct remote_dev_info *dev, match;
- uint8_t name_type = 0x00;
name_status_t name_status;
struct eir_data eir_data;
int state, err;
@@ -513,25 +524,24 @@ void btd_event_inquiry_result(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
} else
legacy = TRUE;
- tmp_name = bt_extract_eir_name(data, &name_type);
- if (tmp_name) {
- if (name_type == 0x09) {
- write_device_name(local, peer, tmp_name);
+ if (eir_data.name) {
+ if (eir_data.name_complete) {
+ write_device_name(local, peer, eir_data.name);
name_status = NAME_NOT_REQUIRED;
if (name)
g_free(name);
- name = tmp_name;
+ name = eir_data.name;
} else {
if (name)
- free(tmp_name);
+ free(eir_data.name);
else
- name = tmp_name;
+ name = eir_data.name;
}
}
- if (name && name_type != 0x08)
+ if (name && eir_data.name_complete)
name_status = NAME_SENT;
/* add in the list to track name sent/pending */
diff --git a/src/glib-helper.c b/src/glib-helper.c
index 6505249..c440e60 100644
--- a/src/glib-helper.c
+++ b/src/glib-helper.c
@@ -780,25 +780,3 @@ GSList *bt_string2list(const gchar *str)
return l;
}
-
-char *bt_extract_eir_name(uint8_t *data, uint8_t *type)
-{
- if (!data || !type)
- return NULL;
-
- if (data[0] == 0)
- return NULL;
-
- if (type)
- *type = data[1];
-
- switch (data[1]) {
- case EIR_NAME_SHORT:
- case EIR_NAME_COMPLETE:
- if (!g_utf8_validate((char *) (data + 2), data[0] - 1, NULL))
- return g_strdup("");
- return g_strndup((char *) (data + 2), data[0] - 1);
- }
-
- return NULL;
-}
diff --git a/src/glib-helper.h b/src/glib-helper.h
index 5bb20a6..10718bb 100644
--- a/src/glib-helper.h
+++ b/src/glib-helper.h
@@ -42,4 +42,3 @@ char *bt_name2string(const char *string);
int bt_string2uuid(uuid_t *uuid, const char *string);
gchar *bt_list2string(GSList *list);
GSList *bt_string2list(const gchar *str);
-char *bt_extract_eir_name(uint8_t *data, uint8_t *type);
--
1.7.0.4
^ permalink raw reply related
* [PATCH 3/4] Move AD flags parsing to parse_eir_data()
From: Bruna Moreira @ 2010-12-17 14:57 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
In-Reply-To: <1292597844-17135-1-git-send-email-bruna.moreira@openbossa.org>
From: Anderson Lizardo <anderson.lizardo@openbossa.org>
---
src/adapter.c | 22 ++++------------------
src/adapter.h | 3 ++-
src/event.c | 7 ++++++-
3 files changed, 12 insertions(+), 20 deletions(-)
diff --git a/src/adapter.c b/src/adapter.c
index 34bf24a..c74019d 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2786,21 +2786,6 @@ static struct remote_dev_info *get_found_dev(struct btd_adapter *adapter,
return dev;
}
-static gboolean extract_eir_flags(uint8_t *flags, uint8_t *eir_data)
-{
- if (eir_data[0] == 0)
- return FALSE;
-
- if (eir_data[1] != EIR_FLAGS)
- return FALSE;
-
- /* For now, only one octet is used for flags */
- if (flags)
- *flags = eir_data[2];
-
- return TRUE;
-}
-
static void remove_same_uuid(gpointer data, gpointer user_data)
{
struct remote_dev_info *dev = user_data;
@@ -2819,7 +2804,8 @@ static void remove_same_uuid(gpointer data, gpointer user_data)
}
void adapter_update_device_from_info(struct btd_adapter *adapter,
- le_advertising_info *info, GSList *services)
+ le_advertising_info *info,
+ GSList *services, uint8_t flags)
{
struct remote_dev_info *dev;
bdaddr_t bdaddr;
@@ -2845,14 +2831,14 @@ void adapter_update_device_from_info(struct btd_adapter *adapter,
g_slist_foreach(services, remove_same_uuid, dev);
dev->services = g_slist_concat(dev->services, services);
+ dev->flags = flags;
+
if (info->length) {
char *tmp_name = bt_extract_eir_name(info->data, NULL);
if (tmp_name) {
g_free(dev->name);
dev->name = tmp_name;
}
-
- extract_eir_flags(info->data, &dev->flags);
}
/* FIXME: check if other information was changed before emitting the
diff --git a/src/adapter.h b/src/adapter.h
index efcf5b8..d5dceb9 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -124,7 +124,8 @@ gboolean adapter_is_ready(struct btd_adapter *adapter);
struct remote_dev_info *adapter_search_found_devices(struct btd_adapter *adapter,
struct remote_dev_info *match);
void adapter_update_device_from_info(struct btd_adapter *adapter,
- le_advertising_info *info, GSList *services);
+ le_advertising_info *info,
+ GSList *services, uint8_t flags);
void adapter_update_found_devices(struct btd_adapter *adapter, bdaddr_t *bdaddr,
int8_t rssi, uint32_t class, const char *name,
const char *alias, gboolean legacy,
diff --git a/src/event.c b/src/event.c
index 3352717..4672106 100644
--- a/src/event.c
+++ b/src/event.c
@@ -60,6 +60,7 @@
struct eir_data {
GSList *services;
+ uint8_t flags;
};
static gboolean get_adapter_and_device(bdaddr_t *src, bdaddr_t *dst,
@@ -346,6 +347,9 @@ static int parse_eir_data(struct eir_data *eir, uint8_t *eir_data,
uuid128_count = field_len / 16;
uuid128 = &eir_data[2];
break;
+ case EIR_FLAGS:
+ eir->flags = eir_data[2];
+ break;
}
len += field_len + 1;
@@ -422,7 +426,8 @@ void btd_event_advertising_report(bdaddr_t *local, le_advertising_info *info)
error("Error parsing advertising data: %s (%d)",
strerror(-err), -err);
- adapter_update_device_from_info(adapter, info, eir_data.services);
+ adapter_update_device_from_info(adapter, info, eir_data.services,
+ eir_data.flags);
}
void btd_event_inquiry_result(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
--
1.7.0.4
^ permalink raw reply related
* [PATCH 2/4] Modify get_eir_uuids() to parse other EIR data
From: Bruna Moreira @ 2010-12-17 14:57 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
In-Reply-To: <1292597844-17135-1-git-send-email-bruna.moreira@openbossa.org>
From: Anderson Lizardo <anderson.lizardo@openbossa.org>
Currently, EIR parsing (name, UUIDs, flags etc.) is done in various
places. This leads to duplicated checks and does not support parsing
multiple AD/EIR structures on a single transfer.
These changes modify get_eir_uuids() to parse other EIR data types and
avoid passing raw EIR data to higher layers.
To accomplish this, get_eir_uuids() was renamed to parse_eir_data(), and
a new (internal) "struct eir_data" was introduced to store the parsed
data.
This first commit only handles the services UUIDs, following patches
will move other EIR parsing to it until everything is done inside
parse_eir_data().
---
src/event.c | 49 ++++++++++++++++++++++++++++++++-----------------
1 files changed, 32 insertions(+), 17 deletions(-)
diff --git a/src/event.c b/src/event.c
index 0e6c86f..3352717 100644
--- a/src/event.c
+++ b/src/event.c
@@ -58,6 +58,10 @@
#include "event.h"
#include "sdpd.h"
+struct eir_data {
+ GSList *services;
+};
+
static gboolean get_adapter_and_device(bdaddr_t *src, bdaddr_t *dst,
struct btd_adapter **adapter,
struct btd_device **device,
@@ -300,14 +304,14 @@ void btd_event_simple_pairing_complete(bdaddr_t *local, bdaddr_t *peer,
device_simple_pairing_complete(device, status);
}
-static GSList *get_eir_uuids(uint8_t *eir_data, size_t eir_length)
+static int parse_eir_data(struct eir_data *eir, uint8_t *eir_data,
+ size_t eir_length)
{
uint16_t len = 0;
size_t total;
size_t uuid16_count = 0;
size_t uuid32_count = 0;
size_t uuid128_count = 0;
- GSList *list = NULL;
uint8_t *uuid16 = NULL;
uint8_t *uuid32 = NULL;
uint8_t *uuid128 = NULL;
@@ -315,8 +319,9 @@ static GSList *get_eir_uuids(uint8_t *eir_data, size_t eir_length)
char *uuid_str;
unsigned int i;
+ /* No EIR data to parse */
if (eir_data == NULL || eir_length == 0)
- return list;
+ return 0;
while (len < eir_length - 1) {
uint8_t field_len = eir_data[0];
@@ -349,12 +354,13 @@ static GSList *get_eir_uuids(uint8_t *eir_data, size_t eir_length)
/* Bail out if got incorrect length */
if (len > eir_length)
- return list;
+ return -EINVAL;
total = uuid16_count + uuid32_count + uuid128_count;
+ /* No UUIDs were parsed, so skip code below */
if (!total)
- return list;
+ return 0;
/* Generate uuids in SDP format (EIR data is Little Endian) */
service.type = SDP_UUID16;
@@ -364,7 +370,7 @@ static GSList *get_eir_uuids(uint8_t *eir_data, size_t eir_length)
val16 = (val16 << 8) + uuid16[0];
service.value.uuid16 = val16;
uuid_str = bt_uuid2string(&service);
- list = g_slist_append(list, uuid_str);
+ eir->services = g_slist_append(eir->services, uuid_str);
uuid16 += 2;
}
@@ -378,7 +384,7 @@ static GSList *get_eir_uuids(uint8_t *eir_data, size_t eir_length)
service.value.uuid32 = val32;
uuid_str = bt_uuid2string(&service);
- list = g_slist_append(list, uuid_str);
+ eir->services = g_slist_append(eir->services, uuid_str);
uuid32 += 4;
}
@@ -390,17 +396,18 @@ static GSList *get_eir_uuids(uint8_t *eir_data, size_t eir_length)
service.value.uuid128.data[k] = uuid128[16 - k - 1];
uuid_str = bt_uuid2string(&service);
- list = g_slist_append(list, uuid_str);
+ eir->services = g_slist_append(eir->services, uuid_str);
uuid128 += 16;
}
- return list;
+ return 0;
}
void btd_event_advertising_report(bdaddr_t *local, le_advertising_info *info)
{
struct btd_adapter *adapter;
- GSList *services = NULL;
+ struct eir_data eir_data;
+ int err;
adapter = manager_find_adapter(local);
if (adapter == NULL) {
@@ -409,9 +416,13 @@ void btd_event_advertising_report(bdaddr_t *local, le_advertising_info *info)
}
/* Extract UUIDs from advertising data if any */
- services = get_eir_uuids(info->data, info->length);
+ memset(&eir_data, 0, sizeof(eir_data));
+ err = parse_eir_data(&eir_data, info->data, info->length);
+ if (err < 0)
+ error("Error parsing advertising data: %s (%d)",
+ strerror(-err), -err);
- adapter_update_device_from_info(adapter, info, services);
+ adapter_update_device_from_info(adapter, info, eir_data.services);
}
void btd_event_inquiry_result(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
@@ -424,8 +435,8 @@ void btd_event_inquiry_result(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
struct remote_dev_info *dev, match;
uint8_t name_type = 0x00;
name_status_t name_status;
- GSList *services = NULL;
- int state;
+ struct eir_data eir_data;
+ int state, err;
dbus_bool_t legacy;
unsigned char features[8];
@@ -454,7 +465,10 @@ void btd_event_inquiry_result(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
}
/* Extract UUIDs from extended inquiry response if any */
- services = get_eir_uuids(data, EIR_DATA_LENGTH);
+ memset(&eir_data, 0, sizeof(eir_data));
+ err = parse_eir_data(&eir_data, data, EIR_DATA_LENGTH);
+ if (err < 0)
+ error("Error parsing EIR data: %s (%d)", strerror(-err), -err);
memset(&match, 0, sizeof(struct remote_dev_info));
bacpy(&match.bdaddr, peer);
@@ -464,7 +478,8 @@ void btd_event_inquiry_result(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
if (dev) {
adapter_update_found_devices(adapter, peer, rssi, class,
NULL, NULL, dev->legacy,
- services, NAME_NOT_REQUIRED);
+ eir_data.services,
+ NAME_NOT_REQUIRED);
return;
}
@@ -516,7 +531,7 @@ void btd_event_inquiry_result(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
/* add in the list to track name sent/pending */
adapter_update_found_devices(adapter, peer, rssi, class, name, alias,
- legacy, services, name_status);
+ legacy, eir_data.services, name_status);
g_free(name);
g_free(alias);
--
1.7.0.4
^ permalink raw reply related
* [PATCH 1/4] Move get_eir_uuids() from src/adapter.c to src/event.c
From: Bruna Moreira @ 2010-12-17 14:57 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
From: Anderson Lizardo <anderson.lizardo@openbossa.org>
Moving get_eir_uuids() to src/event.c removes the need to pass the raw
EIR data to higher layers. Now it is not necessary to pass the original
GSList of service UUIDs, because the list is concatenated (with
verification of duplicate entries) on adapter_update_device_from_info()
(for LE) and adapter_update_found_devices() (for BR/EDR).
---
src/adapter.c | 145 +++++++++++---------------------------------------------
src/adapter.h | 7 +--
src/event.c | 114 +++++++++++++++++++++++++++++++++++++++++++--
3 files changed, 142 insertions(+), 124 deletions(-)
diff --git a/src/adapter.c b/src/adapter.c
index 5118306..34bf24a 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2696,117 +2696,8 @@ static char **strlist2array(GSList *list)
return array;
}
-static GSList *get_eir_uuids(uint8_t *eir_data, size_t eir_length, GSList *list)
-{
- uint16_t len = 0;
- size_t total;
- size_t uuid16_count = 0;
- size_t uuid32_count = 0;
- size_t uuid128_count = 0;
- uint8_t *uuid16;
- uint8_t *uuid32;
- uint8_t *uuid128;
- uuid_t service;
- char *uuid_str;
- unsigned int i;
-
- if (eir_data == NULL || eir_length == 0)
- return list;
-
- while (len < eir_length - 1) {
- uint8_t field_len = eir_data[0];
-
- /* Check for the end of EIR */
- if (field_len == 0)
- break;
-
- switch (eir_data[1]) {
- case EIR_UUID16_SOME:
- case EIR_UUID16_ALL:
- uuid16_count = field_len / 2;
- uuid16 = &eir_data[2];
- break;
- case EIR_UUID32_SOME:
- case EIR_UUID32_ALL:
- uuid32_count = field_len / 4;
- uuid32 = &eir_data[2];
- break;
- case EIR_UUID128_SOME:
- case EIR_UUID128_ALL:
- uuid128_count = field_len / 16;
- uuid128 = &eir_data[2];
- break;
- }
-
- len += field_len + 1;
- eir_data += field_len + 1;
- }
-
- /* Bail out if got incorrect length */
- if (len > eir_length)
- return list;
-
- total = uuid16_count + uuid32_count + uuid128_count;
-
- if (!total)
- return list;
-
- /* Generate uuids in SDP format (EIR data is Little Endian) */
- service.type = SDP_UUID16;
- for (i = 0; i < uuid16_count; i++) {
- uint16_t val16 = uuid16[1];
-
- val16 = (val16 << 8) + uuid16[0];
- service.value.uuid16 = val16;
- uuid_str = bt_uuid2string(&service);
- if (g_slist_find_custom(list, uuid_str,
- (GCompareFunc) strcmp) == NULL)
- list = g_slist_append(list, uuid_str);
- else
- g_free(uuid_str);
- uuid16 += 2;
- }
-
- service.type = SDP_UUID32;
- for (i = uuid16_count; i < uuid32_count + uuid16_count; i++) {
- uint32_t val32 = uuid32[3];
- int k;
-
- for (k = 2; k >= 0; k--)
- val32 = (val32 << 8) + uuid32[k];
-
- service.value.uuid32 = val32;
- uuid_str = bt_uuid2string(&service);
- if (g_slist_find_custom(list, uuid_str,
- (GCompareFunc) strcmp) == NULL)
- list = g_slist_append(list, uuid_str);
- else
- g_free(uuid_str);
- uuid32 += 4;
- }
-
- service.type = SDP_UUID128;
- for (i = uuid32_count + uuid16_count; i < total; i++) {
- int k;
-
- for (k = 0; k < 16; k++)
- service.value.uuid128.data[k] = uuid128[16 - k - 1];
-
- uuid_str = bt_uuid2string(&service);
- if (g_slist_find_custom(list, uuid_str,
- (GCompareFunc) strcmp) == NULL)
- list = g_slist_append(list, uuid_str);
- else
- g_free(uuid_str);
- uuid128 += 16;
- }
-
- return list;
-}
-
void adapter_emit_device_found(struct btd_adapter *adapter,
- struct remote_dev_info *dev,
- uint8_t *eir_data, size_t eir_length)
+ struct remote_dev_info *dev)
{
struct btd_device *device;
char peer_addr[18], local_addr[18];
@@ -2823,8 +2714,7 @@ void adapter_emit_device_found(struct btd_adapter *adapter,
if (device)
paired = device_is_paired(device);
- /* Extract UUIDs from extended inquiry response if any */
- dev->services = get_eir_uuids(eir_data, eir_length, dev->services);
+ /* The uuids string array is updated only if necessary */
uuid_count = g_slist_length(dev->services);
if (dev->services && dev->uuid_count != uuid_count) {
g_strfreev(dev->uuids);
@@ -2911,8 +2801,25 @@ static gboolean extract_eir_flags(uint8_t *flags, uint8_t *eir_data)
return TRUE;
}
+static void remove_same_uuid(gpointer data, gpointer user_data)
+{
+ struct remote_dev_info *dev = user_data;
+ GSList *l;
+
+ for (l = dev->services; l; l = l->next) {
+ char *current_uuid = l->data;
+ char *new_uuid = data;
+
+ if (strcmp(current_uuid, new_uuid) == 0) {
+ g_free(current_uuid);
+ dev->services = g_slist_delete_link(dev->services, l);
+ break;
+ }
+ }
+}
+
void adapter_update_device_from_info(struct btd_adapter *adapter,
- le_advertising_info *info)
+ le_advertising_info *info, GSList *services)
{
struct remote_dev_info *dev;
bdaddr_t bdaddr;
@@ -2935,6 +2842,9 @@ void adapter_update_device_from_info(struct btd_adapter *adapter,
adapter->found_devices = g_slist_sort(adapter->found_devices,
(GCompareFunc) dev_rssi_cmp);
+ g_slist_foreach(services, remove_same_uuid, dev);
+ dev->services = g_slist_concat(dev->services, services);
+
if (info->length) {
char *tmp_name = bt_extract_eir_name(info->data, NULL);
if (tmp_name) {
@@ -2947,13 +2857,13 @@ void adapter_update_device_from_info(struct btd_adapter *adapter,
/* FIXME: check if other information was changed before emitting the
* signal */
- adapter_emit_device_found(adapter, dev, info->data, info->length);
+ adapter_emit_device_found(adapter, dev);
}
void adapter_update_found_devices(struct btd_adapter *adapter, bdaddr_t *bdaddr,
int8_t rssi, uint32_t class, const char *name,
const char *alias, gboolean legacy,
- name_status_t name_status, uint8_t *eir_data)
+ GSList *services, name_status_t name_status)
{
struct remote_dev_info *dev;
gboolean new_dev;
@@ -2979,7 +2889,10 @@ void adapter_update_found_devices(struct btd_adapter *adapter, bdaddr_t *bdaddr,
adapter->found_devices = g_slist_sort(adapter->found_devices,
(GCompareFunc) dev_rssi_cmp);
- adapter_emit_device_found(adapter, dev, eir_data, EIR_DATA_LENGTH);
+ g_slist_foreach(services, remove_same_uuid, dev);
+ dev->services = g_slist_concat(dev->services, services);
+
+ adapter_emit_device_found(adapter, dev);
}
int adapter_remove_found_device(struct btd_adapter *adapter, bdaddr_t *bdaddr)
diff --git a/src/adapter.h b/src/adapter.h
index a02f61c..efcf5b8 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -124,15 +124,14 @@ gboolean adapter_is_ready(struct btd_adapter *adapter);
struct remote_dev_info *adapter_search_found_devices(struct btd_adapter *adapter,
struct remote_dev_info *match);
void adapter_update_device_from_info(struct btd_adapter *adapter,
- le_advertising_info *info);
+ le_advertising_info *info, GSList *services);
void adapter_update_found_devices(struct btd_adapter *adapter, bdaddr_t *bdaddr,
int8_t rssi, uint32_t class, const char *name,
const char *alias, gboolean legacy,
- name_status_t name_status, uint8_t *eir_data);
+ GSList *services, name_status_t name_status);
int adapter_remove_found_device(struct btd_adapter *adapter, bdaddr_t *bdaddr);
void adapter_emit_device_found(struct btd_adapter *adapter,
- struct remote_dev_info *dev,
- uint8_t *eir_data, size_t eir_length);
+ struct remote_dev_info *dev);
void adapter_mode_changed(struct btd_adapter *adapter, uint8_t scan_mode);
void adapter_update_local_name(struct btd_adapter *adapter, const char *name);
void adapter_service_insert(const bdaddr_t *bdaddr, void *rec);
diff --git a/src/event.c b/src/event.c
index 47b50c4..0e6c86f 100644
--- a/src/event.c
+++ b/src/event.c
@@ -56,6 +56,7 @@
#include "agent.h"
#include "storage.h"
#include "event.h"
+#include "sdpd.h"
static gboolean get_adapter_and_device(bdaddr_t *src, bdaddr_t *dst,
struct btd_adapter **adapter,
@@ -299,9 +300,107 @@ void btd_event_simple_pairing_complete(bdaddr_t *local, bdaddr_t *peer,
device_simple_pairing_complete(device, status);
}
+static GSList *get_eir_uuids(uint8_t *eir_data, size_t eir_length)
+{
+ uint16_t len = 0;
+ size_t total;
+ size_t uuid16_count = 0;
+ size_t uuid32_count = 0;
+ size_t uuid128_count = 0;
+ GSList *list = NULL;
+ uint8_t *uuid16 = NULL;
+ uint8_t *uuid32 = NULL;
+ uint8_t *uuid128 = NULL;
+ uuid_t service;
+ char *uuid_str;
+ unsigned int i;
+
+ if (eir_data == NULL || eir_length == 0)
+ return list;
+
+ while (len < eir_length - 1) {
+ uint8_t field_len = eir_data[0];
+
+ /* Check for the end of EIR */
+ if (field_len == 0)
+ break;
+
+ switch (eir_data[1]) {
+ case EIR_UUID16_SOME:
+ case EIR_UUID16_ALL:
+ uuid16_count = field_len / 2;
+ uuid16 = &eir_data[2];
+ break;
+ case EIR_UUID32_SOME:
+ case EIR_UUID32_ALL:
+ uuid32_count = field_len / 4;
+ uuid32 = &eir_data[2];
+ break;
+ case EIR_UUID128_SOME:
+ case EIR_UUID128_ALL:
+ uuid128_count = field_len / 16;
+ uuid128 = &eir_data[2];
+ break;
+ }
+
+ len += field_len + 1;
+ eir_data += field_len + 1;
+ }
+
+ /* Bail out if got incorrect length */
+ if (len > eir_length)
+ return list;
+
+ total = uuid16_count + uuid32_count + uuid128_count;
+
+ if (!total)
+ return list;
+
+ /* Generate uuids in SDP format (EIR data is Little Endian) */
+ service.type = SDP_UUID16;
+ for (i = 0; i < uuid16_count; i++) {
+ uint16_t val16 = uuid16[1];
+
+ val16 = (val16 << 8) + uuid16[0];
+ service.value.uuid16 = val16;
+ uuid_str = bt_uuid2string(&service);
+ list = g_slist_append(list, uuid_str);
+ uuid16 += 2;
+ }
+
+ service.type = SDP_UUID32;
+ for (i = uuid16_count; i < uuid32_count + uuid16_count; i++) {
+ uint32_t val32 = uuid32[3];
+ int k;
+
+ for (k = 2; k >= 0; k--)
+ val32 = (val32 << 8) + uuid32[k];
+
+ service.value.uuid32 = val32;
+ uuid_str = bt_uuid2string(&service);
+ list = g_slist_append(list, uuid_str);
+ uuid32 += 4;
+ }
+
+ service.type = SDP_UUID128;
+ for (i = uuid32_count + uuid16_count; i < total; i++) {
+ int k;
+
+ for (k = 0; k < 16; k++)
+ service.value.uuid128.data[k] = uuid128[16 - k - 1];
+
+ uuid_str = bt_uuid2string(&service);
+ list = g_slist_append(list, uuid_str);
+ uuid128 += 16;
+ }
+
+ return list;
+}
+
void btd_event_advertising_report(bdaddr_t *local, le_advertising_info *info)
{
struct btd_adapter *adapter;
+ GSList *services = NULL;
adapter = manager_find_adapter(local);
if (adapter == NULL) {
@@ -309,7 +408,10 @@ void btd_event_advertising_report(bdaddr_t *local, le_advertising_info *info)
return;
}
- adapter_update_device_from_info(adapter, info);
+ /* Extract UUIDs from advertising data if any */
+ services = get_eir_uuids(info->data, info->length);
+
+ adapter_update_device_from_info(adapter, info, services);
}
void btd_event_inquiry_result(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
@@ -322,6 +424,7 @@ void btd_event_inquiry_result(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
struct remote_dev_info *dev, match;
uint8_t name_type = 0x00;
name_status_t name_status;
+ GSList *services = NULL;
int state;
dbus_bool_t legacy;
unsigned char features[8];
@@ -350,6 +453,9 @@ void btd_event_inquiry_result(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
adapter_set_state(adapter, state);
}
+ /* Extract UUIDs from extended inquiry response if any */
+ services = get_eir_uuids(data, EIR_DATA_LENGTH);
+
memset(&match, 0, sizeof(struct remote_dev_info));
bacpy(&match.bdaddr, peer);
match.name_status = NAME_SENT;
@@ -358,7 +464,7 @@ void btd_event_inquiry_result(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
if (dev) {
adapter_update_found_devices(adapter, peer, rssi, class,
NULL, NULL, dev->legacy,
- NAME_NOT_REQUIRED, data);
+ services, NAME_NOT_REQUIRED);
return;
}
@@ -410,7 +516,7 @@ void btd_event_inquiry_result(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
/* add in the list to track name sent/pending */
adapter_update_found_devices(adapter, peer, rssi, class, name, alias,
- legacy, name_status, data);
+ legacy, services, name_status);
g_free(name);
g_free(alias);
@@ -500,7 +606,7 @@ void btd_event_remote_name(bdaddr_t *local, bdaddr_t *peer, uint8_t status,
if (dev_info) {
g_free(dev_info->name);
dev_info->name = g_strdup(name);
- adapter_emit_device_found(adapter, dev_info, NULL, 0);
+ adapter_emit_device_found(adapter, dev_info);
}
if (device)
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH v2] Fix crash when mmaping files which size is multiple of page size
From: Johan Hedberg @ 2010-12-17 13:53 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <1292592686-14908-1-git-send-email-luiz.dentz@gmail.com>
Hi Luiz,
On Fri, Dec 17, 2010, Luiz Augusto von Dentz wrote:
> From: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>
>
> In this case the buffer returned by mmap is not NULL terminated so
> functions like strpbrk that expect a string goes out of bounds.
>
> To fix this strpbrk_len was introduced which takes the size of the buffer
> making sure it never goes out of bounds.
> ---
> src/textfile.c | 38 +++++++++++++++++++++++++++++++++-----
> 1 files changed, 33 insertions(+), 5 deletions(-)
Thanks. The patch looks good and seems to compile and run fine too. It
has been pushed upstream.
Johan
^ permalink raw reply
* [PATCH v2] Fix crash when mmaping files which size is multiple of page size
From: Luiz Augusto von Dentz @ 2010-12-17 13:31 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>
In this case the buffer returned by mmap is not NULL terminated so
functions like strpbrk that expect a string goes out of bounds.
To fix this strpbrk_len was introduced which takes the size of the buffer
making sure it never goes out of bounds.
---
src/textfile.c | 38 +++++++++++++++++++++++++++++++++-----
1 files changed, 33 insertions(+), 5 deletions(-)
diff --git a/src/textfile.c b/src/textfile.c
index 2429cc7..d115ff6 100644
--- a/src/textfile.c
+++ b/src/textfile.c
@@ -156,6 +156,28 @@ static inline int write_key_value(int fd, const char *key, const char *value)
return err;
}
+static char *strnpbrk(const char *s, ssize_t len, const char *accept)
+{
+ const char *p = s;
+ const char *end;
+
+ end = s + len - 1;
+
+ while (p <= end && *p) {
+ const char *a = accept;
+
+ while (*a) {
+ if (*p == *a)
+ return (char *) p;
+ a++;
+ }
+
+ p++;
+ }
+
+ return NULL;
+}
+
static int write_key(const char *pathname, const char *key, const char *value, int icase)
{
struct stat st;
@@ -207,7 +229,7 @@ static int write_key(const char *pathname, const char *key, const char *value, i
base = off - map;
- end = strpbrk(off, "\r\n");
+ end = strnpbrk(off, size, "\r\n");
if (!end) {
err = EILSEQ;
goto unmap;
@@ -315,7 +337,7 @@ static char *read_key(const char *pathname, const char *key, int icase)
goto unmap;
}
- end = strpbrk(off, "\r\n");
+ end = strnpbrk(off, size - (map - off), "\r\n");
if (!end) {
err = EILSEQ;
goto unmap;
@@ -404,8 +426,8 @@ int textfile_foreach(const char *pathname, textfile_cb func, void *data)
off = map;
- while (1) {
- end = strpbrk(off, " ");
+ while (size - (off - map) > 0) {
+ end = strnpbrk(off, size - (off - map), " ");
if (!end) {
err = EILSEQ;
break;
@@ -424,7 +446,13 @@ int textfile_foreach(const char *pathname, textfile_cb func, void *data)
off = end + 1;
- end = strpbrk(off, "\r\n");
+ if (size - (off - map) < 0) {
+ err = EILSEQ;
+ free(key);
+ break;
+ }
+
+ end = strnpbrk(off, size - (off - map), "\r\n");
if (!end) {
err = EILSEQ;
free(key);
--
1.7.1
^ permalink raw reply related
* Re: [PATCH] Fix crash while reading from mapped file
From: Luiz Augusto von Dentz @ 2010-12-17 13:25 UTC (permalink / raw)
To: Anderson Lizardo
Cc: Lukasz Pawlik, Bastien Nocera, Johan Hedberg, linux-bluetooth
In-Reply-To: <AANLkTinHyH6i_pq-77_SYEkyf3jfwbfqL_WXav6nB2tc@mail.gmail.com>
Hi,
On Fri, Dec 17, 2010 at 11:29 AM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> Hi,
>
> On Thu, Dec 16, 2010 at 11:28 AM, Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
>> Hi,
>>
>> On Wed, Dec 15, 2010 at 8:19 PM, Anderson Lizardo
>> <anderson.lizardo@openbossa.org> wrote:
>>> On Tue, Dec 14, 2010 at 2:20 PM, Lukasz Pawlik <lucas.pawlik@gmail.com> wrote:
>>>> Hi,
>>>>
>>>>> If somebody can explain what that code is supposed to do, then writing a
>>>>> glib-ish version should be trivial, without having to duplicate the
>>>>> contents again. Then again, using something like:
>>>>> g_mapped_file_new ();
>>>>> g_strsplit ();
>>>>> g_mapped_file_unref ();
>>>>
>>>> That won't fix anything since g_mapped_file_new uses mmap function so
>>>> contents may not be zero-terminated and we want use string function
>>>> next.
>>>
>>> What about using g_strstr_len() instead of strpbrk() on the original
>>> code? See http://library.gnome.org/devel/glib/unstable/glib-String-Utility-Functions.html#g-strstr-len
>>
>> That looks to be a good replacement for strpbrk, probably 1 line patch.
>
> I guess we cannot use any of glib functions here since textfile.c is
> also used in some tools which doesn't link with glib, so if we don't
> want to add this dependency to the than we need some other way to fix
> it.
>
> What about this:
>
> diff --git a/src/textfile.c b/src/textfile.c
> index 2429cc7..2e4c642 100644
> --- a/src/textfile.c
> +++ b/src/textfile.c
> @@ -394,7 +394,7 @@ int textfile_foreach(const char *pathname,
> textfile_cb func, void *data)
> goto unlock;
> }
>
> - size = st.st_size;
> + size = st.st_size + 1;
>
> map = mmap(NULL, size, PROT_READ, MAP_SHARED, fd, 0);
> if (!map || map == MAP_FAILED) {
>
> It will probably use 1 more page if the file size is multiple of the
> page size but it seems correct if you compare to e.g. malloc, well if
> the possibility of the extra page is too much than we need our own
> version of g_strstr_len/strpbrk_len like the following:
>
> http://www.google.com/codesearch/p?hl=en#cZwlSNS7aEw/external/bluetooth/glib/glib/gstrfuncs.c&q=g_strstr_len&d=4
The patch doesn't really work, it cause Non-existent physical address
error, anyway I have sent a patch that introduces strpbrk_len, but I
will probably be changing to strnpbrk to conform with some other strn*
variants.
--
Luiz Augusto von Dentz
Computer Engineer
^ permalink raw reply
* [PATCH] Fix crash when mmaping files which size is multiple of page size
From: Luiz Augusto von Dentz @ 2010-12-17 13:23 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>
In this case the buffer returned by mmap is not NULL terminated so
functions like strpbrk that expect a string goes out of bounds.
To fix this strpbrk_len was introduced which takes the size of the buffer
making sure it never goes out of bounds.
---
src/textfile.c | 38 +++++++++++++++++++++++++++++++++-----
1 files changed, 33 insertions(+), 5 deletions(-)
diff --git a/src/textfile.c b/src/textfile.c
index 2429cc7..748f1bc 100644
--- a/src/textfile.c
+++ b/src/textfile.c
@@ -156,6 +156,28 @@ static inline int write_key_value(int fd, const char *key, const char *value)
return err;
}
+static char *strpbrk_len(const char *s, ssize_t len, const char *accept)
+{
+ const char *p = s;
+ const char *end;
+
+ end = s + len - 1;
+
+ while (p <= end && *p) {
+ const char *a = accept;
+
+ while (*a) {
+ if (*p == *a)
+ return (char *) p;
+ a++;
+ }
+
+ p++;
+ }
+
+ return NULL;
+}
+
static int write_key(const char *pathname, const char *key, const char *value, int icase)
{
struct stat st;
@@ -207,7 +229,7 @@ static int write_key(const char *pathname, const char *key, const char *value, i
base = off - map;
- end = strpbrk(off, "\r\n");
+ end = strpbrk_len(off, size, "\r\n");
if (!end) {
err = EILSEQ;
goto unmap;
@@ -315,7 +337,7 @@ static char *read_key(const char *pathname, const char *key, int icase)
goto unmap;
}
- end = strpbrk(off, "\r\n");
+ end = strpbrk_len(off, size - (map - off), "\r\n");
if (!end) {
err = EILSEQ;
goto unmap;
@@ -404,8 +426,8 @@ int textfile_foreach(const char *pathname, textfile_cb func, void *data)
off = map;
- while (1) {
- end = strpbrk(off, " ");
+ while (size - (off - map) > 0) {
+ end = strpbrk_len(off, size - (off - map), " ");
if (!end) {
err = EILSEQ;
break;
@@ -424,7 +446,13 @@ int textfile_foreach(const char *pathname, textfile_cb func, void *data)
off = end + 1;
- end = strpbrk(off, "\r\n");
+ if (size - (off - map) < 0) {
+ err = EILSEQ;
+ free(key);
+ break;
+ }
+
+ end = strpbrk_len(off, size - (off - map), "\r\n");
if (!end) {
err = EILSEQ;
free(key);
--
1.7.1
^ permalink raw reply related
* [PATCH v4] Bluetooth: Fix __hci_request synchronization for hci_open_dev
From: johan.hedberg @ 2010-12-17 12:48 UTC (permalink / raw)
To: linux-bluetooth
From: Johan Hedberg <johan.hedberg@nokia.com>
The initialization function used by hci_open_dev (hci_init_req) sends
many different HCI commands. The __hci_request function should only
return when all of these commands have completed (or a timeout occurs).
Several of these commands cause hci_req_complete to be called which
causes __hci_request to return prematurely.
This patch fixes the issue by adding a new hdev->req_last_cmd variable
which is set during the initialization procedure. The hci_req_complete
function will no longer mark the request as complete until the command
matching hdev->req_last_cmd completes.
Signed-off-by: Johan Hedberg <johan.hedberg@nokia.com>
---
v4: Use __u16 instead of int for req_last_cmd
include/net/bluetooth/hci_core.h | 3 ++-
net/bluetooth/hci_core.c | 10 +++++++---
net/bluetooth/hci_event.c | 33 +++++++++++++++++++++++----------
3 files changed, 32 insertions(+), 14 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 3786ee8..a29feb0 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -129,6 +129,7 @@ struct hci_dev {
wait_queue_head_t req_wait_q;
__u32 req_status;
__u32 req_result;
+ __u16 req_last_cmd;
struct inquiry_cache inq_cache;
struct hci_conn_hash conn_hash;
@@ -693,6 +694,6 @@ struct hci_sec_filter {
#define hci_req_lock(d) mutex_lock(&d->req_lock)
#define hci_req_unlock(d) mutex_unlock(&d->req_lock)
-void hci_req_complete(struct hci_dev *hdev, int result);
+void hci_req_complete(struct hci_dev *hdev, __u16 cmd, int result);
#endif /* __HCI_CORE_H */
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 1a4ec97..787c8df 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -91,9 +91,12 @@ static void hci_notify(struct hci_dev *hdev, int event)
/* ---- HCI requests ---- */
-void hci_req_complete(struct hci_dev *hdev, int result)
+void hci_req_complete(struct hci_dev *hdev, __u16 cmd, int result)
{
- BT_DBG("%s result 0x%2.2x", hdev->name, result);
+ BT_DBG("%s command 0x%04x result 0x%2.2x", hdev->name, cmd, result);
+
+ if (hdev->req_last_cmd && cmd != hdev->req_last_cmd)
+ return;
if (hdev->req_status == HCI_REQ_PEND) {
hdev->req_result = result;
@@ -149,7 +152,7 @@ static int __hci_request(struct hci_dev *hdev, void (*req)(struct hci_dev *hdev,
break;
}
- hdev->req_status = hdev->req_result = 0;
+ hdev->req_last_cmd = hdev->req_status = hdev->req_result = 0;
BT_DBG("%s end: err %d", hdev->name, err);
@@ -252,6 +255,7 @@ static void hci_init_req(struct hci_dev *hdev, unsigned long opt)
/* Connection accept timeout ~20 secs */
param = cpu_to_le16(0x7d00);
hci_send_cmd(hdev, HCI_OP_WRITE_CA_TIMEOUT, 2, ¶m);
+ hdev->req_last_cmd = HCI_OP_WRITE_CA_TIMEOUT;
}
static void hci_scan_req(struct hci_dev *hdev, unsigned long opt)
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 8923b36..3810017 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -58,7 +58,7 @@ static void hci_cc_inquiry_cancel(struct hci_dev *hdev, struct sk_buff *skb)
clear_bit(HCI_INQUIRY, &hdev->flags);
- hci_req_complete(hdev, status);
+ hci_req_complete(hdev, HCI_OP_INQUIRY_CANCEL, status);
hci_conn_check_pending(hdev);
}
@@ -174,7 +174,7 @@ static void hci_cc_write_def_link_policy(struct hci_dev *hdev, struct sk_buff *s
if (!status)
hdev->link_policy = get_unaligned_le16(sent);
- hci_req_complete(hdev, status);
+ hci_req_complete(hdev, HCI_OP_WRITE_DEF_LINK_POLICY, status);
}
static void hci_cc_reset(struct hci_dev *hdev, struct sk_buff *skb)
@@ -183,7 +183,7 @@ static void hci_cc_reset(struct hci_dev *hdev, struct sk_buff *skb)
BT_DBG("%s status 0x%x", hdev->name, status);
- hci_req_complete(hdev, status);
+ hci_req_complete(hdev, HCI_OP_RESET, status);
}
static void hci_cc_write_local_name(struct hci_dev *hdev, struct sk_buff *skb)
@@ -235,7 +235,7 @@ static void hci_cc_write_auth_enable(struct hci_dev *hdev, struct sk_buff *skb)
clear_bit(HCI_AUTH, &hdev->flags);
}
- hci_req_complete(hdev, status);
+ hci_req_complete(hdev, HCI_OP_WRITE_AUTH_ENABLE, status);
}
static void hci_cc_write_encrypt_mode(struct hci_dev *hdev, struct sk_buff *skb)
@@ -258,7 +258,7 @@ static void hci_cc_write_encrypt_mode(struct hci_dev *hdev, struct sk_buff *skb)
clear_bit(HCI_ENCRYPT, &hdev->flags);
}
- hci_req_complete(hdev, status);
+ hci_req_complete(hdev, HCI_OP_WRITE_ENCRYPT_MODE, status);
}
static void hci_cc_write_scan_enable(struct hci_dev *hdev, struct sk_buff *skb)
@@ -285,7 +285,7 @@ static void hci_cc_write_scan_enable(struct hci_dev *hdev, struct sk_buff *skb)
set_bit(HCI_PSCAN, &hdev->flags);
}
- hci_req_complete(hdev, status);
+ hci_req_complete(hdev, HCI_OP_WRITE_SCAN_ENABLE, status);
}
static void hci_cc_read_class_of_dev(struct hci_dev *hdev, struct sk_buff *skb)
@@ -383,7 +383,7 @@ static void hci_cc_host_buffer_size(struct hci_dev *hdev, struct sk_buff *skb)
BT_DBG("%s status 0x%x", hdev->name, status);
- hci_req_complete(hdev, status);
+ hci_req_complete(hdev, HCI_OP_HOST_BUFFER_SIZE, status);
}
static void hci_cc_read_ssp_mode(struct hci_dev *hdev, struct sk_buff *skb)
@@ -536,7 +536,16 @@ static void hci_cc_read_bd_addr(struct hci_dev *hdev, struct sk_buff *skb)
if (!rp->status)
bacpy(&hdev->bdaddr, &rp->bdaddr);
- hci_req_complete(hdev, rp->status);
+ hci_req_complete(hdev, HCI_OP_READ_BD_ADDR, rp->status);
+}
+
+static void hci_cc_write_ca_timeout(struct hci_dev *hdev, struct sk_buff *skb)
+{
+ __u8 status = *((__u8 *) skb->data);
+
+ BT_DBG("%s status 0x%x", hdev->name, status);
+
+ hci_req_complete(hdev, HCI_OP_WRITE_CA_TIMEOUT, status);
}
static inline void hci_cs_inquiry(struct hci_dev *hdev, __u8 status)
@@ -544,7 +553,7 @@ static inline void hci_cs_inquiry(struct hci_dev *hdev, __u8 status)
BT_DBG("%s status 0x%x", hdev->name, status);
if (status) {
- hci_req_complete(hdev, status);
+ hci_req_complete(hdev, HCI_OP_INQUIRY, status);
hci_conn_check_pending(hdev);
} else
@@ -871,7 +880,7 @@ static inline void hci_inquiry_complete_evt(struct hci_dev *hdev, struct sk_buff
clear_bit(HCI_INQUIRY, &hdev->flags);
- hci_req_complete(hdev, status);
+ hci_req_complete(hdev, HCI_OP_INQUIRY, status);
hci_conn_check_pending(hdev);
}
@@ -1379,6 +1388,10 @@ static inline void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *sk
hci_cc_read_bd_addr(hdev, skb);
break;
+ case HCI_OP_WRITE_CA_TIMEOUT:
+ hci_cc_write_ca_timeout(hdev, skb);
+ break;
+
default:
BT_DBG("%s opcode 0x%x", hdev->name, opcode);
break;
--
1.7.2.3
^ permalink raw reply related
* RE: [PATCH 00/11] mfd and bluetooth: Add CG2900 support
From: Par-Gunnar HJALMDAHL @ 2010-12-17 12:43 UTC (permalink / raw)
To: Vitaly Wool
Cc: Pavan Savoy, Alan Cox, Arnd Bergmann, Samuel Ortiz,
Marcel Holtmann, linux-kernel@vger.kernel.org,
linux-bluetooth@vger.kernel.org, Lukasz Rymanowski, Linus WALLEIJ,
Par-Gunnar Hjalmdahl
In-Reply-To: <AANLkTikboaQGMGTVchXyo6tidtq7c33KDRU2puL1youv@mail.gmail.com>
Hi Vitaly,
> -----Original Message-----
> From: Vitaly Wool [mailto:vitalywool@gmail.com]
> Sent: den 17 december 2010 13:11
> To: Par-Gunnar HJALMDAHL
> Cc: Pavan Savoy; Alan Cox; Arnd Bergmann; Samuel Ortiz; Marcel
> Holtmann; linux-kernel@vger.kernel.org; linux-
> bluetooth@vger.kernel.org; Lukasz Rymanowski; Linus WALLEIJ; Par-Gunnar
> Hjalmdahl
> Subject: Re: [PATCH 00/11] mfd and bluetooth: Add CG2900 support
>=20
> Hi Par,
>=20
> On Fri, Dec 17, 2010 at 12:20 PM, Par-Gunnar Hjalmdahl
> <par-gunnar.p.hjalmdahl@stericsson.com> wrote:
> > This is the 3rd patch set for the ST-Ericsson CG2900 connectivity
> > controller. The CG2900 is a combo controller supporting GPS,
> Bluetooth,
> > and FM radio. It uses HCI H:4 protocol to combine different
> functionalities
> > on a common transport, where first byte in the data indicates the
> current
> > channel. Channels 1-4 are standardized in the Bluetooth Core
> specification
> > while the other channels are vendor specific.
> >
> > Compared to 2nd patch set this patch set has the following changes:
> > =A0* UART handling is moved from mfd to bluetooth folder. It now reuses
> the
> > =A0 existing N_HCI line discipline.
> > =A0* mfd creation has been moved from cg2900_core into chip specific
> files.
> > =A0* All information for each channel, including API functions, exist
> in each
> > =A0 MFD devices, making them independent of each other.
> > =A0* All chip specific information has been moved from cg2900_core into
> the
> > =A0 chip specific files. cg2900_core now only handles registration and
> > =A0 connection between transport and chip driver.
> > =A0* Fixes for several review comments including use of existing debug
> system.
>=20
> this patchset has only multiplied my confusion.
>=20
> Not really diving into the details, I'm just trying to understand how
> I'd rewrite the TI shared transport using your approach if I had to.
> It looks like I'd have to, at least:
> - implement a heavyweight line discipline driver (like cg2900_uart.c)
> half duplicating the functionality in yours;
> - implement specific MFD driver.
>=20
> Actually the reuse of this implementation will be at a level of
> statistics error. The only thing to reuse is actually the approach
> which I'm not really happy about.
>=20
> So...
>=20
> If you are aiming for a generic solution, then why you get more and
> more things handled in cg2900-specific code (e. g. packet routing
> which is fairly generic)?
>=20
You are absolutely correct that there is not much that is generic and the r=
eason for this is that it is not much that is generic. If you for example l=
ook at packet routing the method can be considered pretty generic. Check fi=
rst byte and choose a structure type... But first of all this must be done =
with minimum overhead since it is done on every packet received. And second=
ly if you look at number of lines of code, it is not really much you save.
If you look closer at the code for the CG2900 you will see that the majorit=
y of the code is things that you could probably never apply to the chip of =
another vendor. A lot of the code in e.g. cg2900_chip.c is regarding bt_aud=
io and fm_audio, which is really chip specific.
Also, if you want to keep the structure modular and supporting e.g. multipl=
e transports with the same chip driver, you must focus on what is chip spec=
ific, what is transport specific, and what is chip and transport specific. =
When you start to think about it like this it becomes very hard to make too=
much code generic. Usually what you get with that kind of generic code is =
a solution way more complex than you started out with.
> If you are solving your particular problems, isn't the change too
> dramatic e. g. when it comes to line discipline drivers and baudrate
> manipulation?
>=20
The changes in hci_ldisc.c is not what I would call dramatic. It is quite s=
imple functions. As I've stated earlier, if people object to this, there is=
no big issue to solve this within cg2900_uart.c instead. But we think that=
these API functions are a proper extension to the current hci_ldisc API.
> As of now, I really see no reason why anyone would promote this
> solution instead of making something more generic out of ti-st and
> reusing it then.
>=20
> Thanks,
> Vitaly
Just to be clear, I have no problem with having both solutions (our and ti-=
st) side by side. They are structured quite differently and has different f=
ocus. I'm in no way trying to replace ti-st. I'm not trying to force anyone=
into using our structure.
/P-G
^ permalink raw reply
* RE: [PATCH 08/11] Bluetooth: Add support for CG2900 UART
From: Par-Gunnar HJALMDAHL @ 2010-12-17 12:29 UTC (permalink / raw)
To: Vitaly Wool
Cc: Pavan Savoy, Alan Cox, Arnd Bergmann, Samuel Ortiz,
Marcel Holtmann, linux-kernel@vger.kernel.org,
linux-bluetooth@vger.kernel.org, Lukasz Rymanowski, Linus WALLEIJ,
Par-Gunnar Hjalmdahl
In-Reply-To: <AANLkTimjDCF4V9wGszVcuDYqZ8Qq0CYhCzxnxEKcibxN@mail.gmail.com>
Hi Vitaly,
> -----Original Message-----
> From: Vitaly Wool [mailto:vitalywool@gmail.com]
> Sent: den 17 december 2010 13:03
> To: Par-Gunnar HJALMDAHL
> Cc: Pavan Savoy; Alan Cox; Arnd Bergmann; Samuel Ortiz; Marcel
> Holtmann; linux-kernel@vger.kernel.org; linux-
> bluetooth@vger.kernel.org; Lukasz Rymanowski; Linus WALLEIJ; Par-Gunnar
> Hjalmdahl
> Subject: Re: [PATCH 08/11] Bluetooth: Add support for CG2900 UART
>=20
> Hi Par,
>=20
> so on the top level: this is yet another H4 implementation plus
> channel-based packet routing, right?
>=20
> Could you please also elaborate
>=20
Yes, the low-level basis is similar to e.g. hci_h4.c, where we register to =
N_HCI line discipline and then use the first byte to separate between diffe=
rent channels.
While hci_h4.c supports only the standardized Bluetooth H4 channels, the cg=
2900_uart targets also the CG2900 specific H4 channels.
> More comments on the code are inlined.
>=20
> > +#define MAIN_DEV =A0 =A0 =A0 =A0 =A0 =A0 =A0 (uart_info->dev)
>=20
> What is that for?
>=20
This is just a simplification when using for example dev_err(). It's just t=
o shorten the text (not much in this case but there are other files where i=
t looks better).
No big problem to fix if you want that.
> > + * cg2900_uart_suspend() - Called by Linux PM to put the device in a
> low power mode.
> > + * @pdev: =A0 =A0 =A0Pointer to platform device.
> > + * @state: =A0 =A0 New state.
> > + *
> > + * In UART case, CG2900 driver does nothing on suspend.
> > + *
> > + * Returns:
> > + * =A0 0 - Success.
> > + */
> > +static int cg2900_uart_suspend(struct platform_device *pdev,
> pm_message_t state)
> > +{
> > + =A0 =A0 =A0 struct uart_info *uart_info =3D dev_get_drvdata(&pdev->de=
v);
> > +
> > + =A0 =A0 =A0 if (uart_info->sleep_state =3D=3D CHIP_POWERED_DOWN)
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return 0;
> > +
> > + =A0 =A0 =A0 if (uart_info->sleep_state !=3D CHIP_ASLEEP)
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return -EBUSY;
> > +
> > + =A0 =A0 =A0 dev_dbg(MAIN_DEV, "New sleep_state: CHIP_SUSPENDED\n");
> > + =A0 =A0 =A0 uart_info->sleep_state =3D CHIP_SUSPENDED;
> > + =A0 =A0 =A0 return 0;
> > +}
>=20
> I don't think this is safe wrt work queue. What if it gets scheduled
> when drivers are suspended?
>=20
> Thanks,
> Vitaly
I'm not 100% sure what you mean since I don't think sleep_state should ever=
be in CHIP_ASLEEP if we have a work ongoing or in the queue, but I will as=
k our low power expert to look into this.
/P-G
^ permalink raw reply
* RE: [PATCH 07/11] Bluetooth: Add UART API functions to ldisc
From: Par-Gunnar HJALMDAHL @ 2010-12-17 12:17 UTC (permalink / raw)
To: Vitaly Wool
Cc: Pavan Savoy, Alan Cox, Arnd Bergmann, Samuel Ortiz,
Marcel Holtmann, linux-kernel@vger.kernel.org,
linux-bluetooth@vger.kernel.org, Lukasz Rymanowski, Linus WALLEIJ,
Par-Gunnar Hjalmdahl
In-Reply-To: <AANLkTi=f6XPbsUiTSiFru2tj_XsQSu=-h898b0R2AePo@mail.gmail.com>
Hi Vitaly,
> -----Original Message-----
> From: Vitaly Wool [mailto:vitalywool@gmail.com]
> Sent: den 17 december 2010 12:41
> To: Par-Gunnar HJALMDAHL
> Cc: Pavan Savoy; Alan Cox; Arnd Bergmann; Samuel Ortiz; Marcel
> Holtmann; linux-kernel@vger.kernel.org; linux-
> bluetooth@vger.kernel.org; Lukasz Rymanowski; Linus WALLEIJ; Par-Gunnar
> Hjalmdahl
> Subject: Re: [PATCH 07/11] Bluetooth: Add UART API functions to ldisc
>=20
> Hi Par,
>=20
> On Fri, Dec 17, 2010 at 12:24 PM, Par-Gunnar Hjalmdahl
> <par-gunnar.p.hjalmdahl@stericsson.com> wrote:
> > This patch adds new functions to control break and flow
> > as well as changing UART baud rate directly through the ldisc API
> > rather then using the TTY API directly.
>=20
> What do you need that for?
>=20
For the break and flow control it is a means for the protocol driver to set=
break and flow using only the hci_uart structure, i.e. without interfacing=
directly towards tty. This is of course not 100% necessary to make it work=
, but it means a clean API where the protocol drivers can perform their tas=
ks without performing different operations on different stack levels. As it=
was before write was performed using hci_uart structure while flow control=
was done using tty structure and API, even though you could consider these=
functions to be similar (not in functionality of course but from a stack l=
evel perspective).
For baud rate changes it also simplifies handling a bit, since the new func=
tion also implements using the proper mutexes and calling right termios fun=
ctions. It should work simply (as long as you don't want different baud rat=
es for TX and RX which should be pretty rare).
> > It also adds a boolean parameter so it is optional to register
> > to the Bluetooth stack (for drivers where a separate module
> > registers to the Bluetooth stack instead).
>=20
> I never liked this idea and I don't like it now as it scatters the
> changes needed to support devices you are aiming to support with your
> changes.
>=20
OK. Sadly I can't do much about how you feel towards the change. In my opin=
ion this is probably the best reuse we can get of existing protocols and pa=
rts. There is so much that is vendor specific if you look at for example cg=
2900_uart.c that it is not much you could do generic.
> Thanks,
> Vitaly
^ permalink raw reply
* Re: [PATCH 00/11] mfd and bluetooth: Add CG2900 support
From: Vitaly Wool @ 2010-12-17 12:11 UTC (permalink / raw)
To: Par-Gunnar Hjalmdahl
Cc: Pavan Savoy, Alan Cox, Arnd Bergmann, Samuel Ortiz,
Marcel Holtmann, linux-kernel, linux-bluetooth, Lukasz Rymanowski,
Linus Walleij, Par-Gunnar Hjalmdahl
In-Reply-To: <1292584829-28279-1-git-send-email-par-gunnar.p.hjalmdahl@stericsson.com>
Hi Par,
On Fri, Dec 17, 2010 at 12:20 PM, Par-Gunnar Hjalmdahl
<par-gunnar.p.hjalmdahl@stericsson.com> wrote:
> This is the 3rd patch set for the ST-Ericsson CG2900 connectivity
> controller. The CG2900 is a combo controller supporting GPS, Bluetooth,
> and FM radio. It uses HCI H:4 protocol to combine different functionaliti=
es
> on a common transport, where first byte in the data indicates the current
> channel. Channels 1-4 are standardized in the Bluetooth Core specificatio=
n
> while the other channels are vendor specific.
>
> Compared to 2nd patch set this patch set has the following changes:
> =A0* UART handling is moved from mfd to bluetooth folder. It now reuses t=
he
> =A0 existing N_HCI line discipline.
> =A0* mfd creation has been moved from cg2900_core into chip specific file=
s.
> =A0* All information for each channel, including API functions, exist in =
each
> =A0 MFD devices, making them independent of each other.
> =A0* All chip specific information has been moved from cg2900_core into t=
he
> =A0 chip specific files. cg2900_core now only handles registration and
> =A0 connection between transport and chip driver.
> =A0* Fixes for several review comments including use of existing debug sy=
stem.
this patchset has only multiplied my confusion.
Not really diving into the details, I'm just trying to understand how
I'd rewrite the TI shared transport using your approach if I had to.
It looks like I'd have to, at least:
- implement a heavyweight line discipline driver (like cg2900_uart.c)
half duplicating the functionality in yours;
- implement specific MFD driver.
Actually the reuse of this implementation will be at a level of
statistics error. The only thing to reuse is actually the approach
which I'm not really happy about.
So...
If you are aiming for a generic solution, then why you get more and
more things handled in cg2900-specific code (e. g. packet routing
which is fairly generic)?
If you are solving your particular problems, isn't the change too
dramatic e. g. when it comes to line discipline drivers and baudrate
manipulation?
As of now, I really see no reason why anyone would promote this
solution instead of making something more generic out of ti-st and
reusing it then.
Thanks,
Vitaly
^ permalink raw reply
* Re: [PATCH 08/11] Bluetooth: Add support for CG2900 UART
From: Vitaly Wool @ 2010-12-17 12:02 UTC (permalink / raw)
To: Par-Gunnar Hjalmdahl
Cc: Pavan Savoy, Alan Cox, Arnd Bergmann, Samuel Ortiz,
Marcel Holtmann, linux-kernel, linux-bluetooth, Lukasz Rymanowski,
Linus Walleij, Par-Gunnar Hjalmdahl
In-Reply-To: <1292585117-28584-1-git-send-email-par-gunnar.p.hjalmdahl@stericsson.com>
Hi Par,
so on the top level: this is yet another H4 implementation plus
channel-based packet routing, right?
Could you please also elaborate
More comments on the code are inlined.
> +#define MAIN_DEV =A0 =A0 =A0 =A0 =A0 =A0 =A0 (uart_info->dev)
What is that for?
> + * cg2900_uart_suspend() - Called by Linux PM to put the device in a low=
power mode.
> + * @pdev: =A0 =A0 =A0Pointer to platform device.
> + * @state: =A0 =A0 New state.
> + *
> + * In UART case, CG2900 driver does nothing on suspend.
> + *
> + * Returns:
> + * =A0 0 - Success.
> + */
> +static int cg2900_uart_suspend(struct platform_device *pdev, pm_message_=
t state)
> +{
> + =A0 =A0 =A0 struct uart_info *uart_info =3D dev_get_drvdata(&pdev->dev)=
;
> +
> + =A0 =A0 =A0 if (uart_info->sleep_state =3D=3D CHIP_POWERED_DOWN)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return 0;
> +
> + =A0 =A0 =A0 if (uart_info->sleep_state !=3D CHIP_ASLEEP)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return -EBUSY;
> +
> + =A0 =A0 =A0 dev_dbg(MAIN_DEV, "New sleep_state: CHIP_SUSPENDED\n");
> + =A0 =A0 =A0 uart_info->sleep_state =3D CHIP_SUSPENDED;
> + =A0 =A0 =A0 return 0;
> +}
I don't think this is safe wrt work queue. What if it gets scheduled
when drivers are suspended?
Thanks,
Vitaly
^ permalink raw reply
* Re: [PATCH 07/11] Bluetooth: Add UART API functions to ldisc
From: Vitaly Wool @ 2010-12-17 11:40 UTC (permalink / raw)
To: Par-Gunnar Hjalmdahl
Cc: Pavan Savoy, Alan Cox, Arnd Bergmann, Samuel Ortiz,
Marcel Holtmann, linux-kernel, linux-bluetooth, Lukasz Rymanowski,
Linus Walleij, Par-Gunnar Hjalmdahl
In-Reply-To: <1292585094-28544-1-git-send-email-par-gunnar.p.hjalmdahl@stericsson.com>
Hi Par,
On Fri, Dec 17, 2010 at 12:24 PM, Par-Gunnar Hjalmdahl
<par-gunnar.p.hjalmdahl@stericsson.com> wrote:
> This patch adds new functions to control break and flow
> as well as changing UART baud rate directly through the ldisc API
> rather then using the TTY API directly.
What do you need that for?
> It also adds a boolean parameter so it is optional to register
> to the Bluetooth stack (for drivers where a separate module
> registers to the Bluetooth stack instead).
I never liked this idea and I don't like it now as it scatters the
changes needed to support devices you are aiming to support with your
changes.
Thanks,
Vitaly
^ permalink raw reply
* Re: [PATCH v2] Bluetooth: Fix __hci_request synchronization for hci_open_dev
From: Marcel Holtmann @ 2010-12-17 11:36 UTC (permalink / raw)
To: johan.hedberg; +Cc: linux-bluetooth
In-Reply-To: <1292434408-20793-1-git-send-email-johan.hedberg@gmail.com>
Hi Johan,
> The initialization function used by hci_open_dev (hci_init_req) sends
> many different HCI commands. The __hci_request function should only
> return when all of these commands have completed (or a timeout occurs).
> Several of these commands cause hci_req_complete to be called which
> causes __hci_request to return prematurely.
>
> This patch fixes the issue by adding a new hdev->last_init_cmd variable
> and making use of the HCI_INIT flag which is set during the
> initialization procedure. The hci_req_complete function will no longer
> do anything when this flag is set and the completed command wasn't the
> last_init_cmd one.
>
> Signed-off-by: Johan Hedberg <johan.hedberg@nokia.com>
> ---
> v2: (based on feedback from Gustavo Padovan) hci_event.c and hci_core.c
> are now less dependent on each other regarding this issue. Now the
> hci_init_complete call doesn't need to be moved to a new location in
> hci_event.c if the sequence of initialization HCI commands changes in
> hci_core.c.
>
> include/net/bluetooth/hci_core.h | 4 +++-
> net/bluetooth/hci_core.c | 8 ++++++--
> net/bluetooth/hci_event.c | 33 +++++++++++++++++++++++----------
> 3 files changed, 32 insertions(+), 13 deletions(-)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 3786ee8..23ca4b1 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -134,6 +134,8 @@ struct hci_dev {
> struct hci_conn_hash conn_hash;
> struct list_head blacklist;
>
> + int last_init_cmd;
> +
please use __u16 type here since you are storing an opcode. Also make
sure you always store the proper opcode in host order.
Regards
Marcel
^ permalink raw reply
* Re: AVDTP_DELAY_REPORT
From: Marcel Holtmann @ 2010-12-17 11:28 UTC (permalink / raw)
To: pl bossart; +Cc: linux-bluetooth
In-Reply-To: <AANLkTimHrJwGZmpErW=60pP08YhoFi=2LaD7cLG_FDnm@mail.gmail.com>
Hi Pierre,
> Does anyone know of audio headsets supporting the new DELAY_REPORT
> commands? This could be very useful to improve a/v sync and may
> require some additions to PulseAudio.
I have not seen a single headset that supports this yet.
Regards
Marcel
^ permalink raw reply
* Re: EDR support
From: Marcel Holtmann @ 2010-12-17 11:27 UTC (permalink / raw)
To: Manuel Naranjo; +Cc: BlueZ
In-Reply-To: <4D03DC83.8010200@aircable.net>
Hi Manuel,
> I was checking the latest BlueZ source code and the kernel source code,
> and I noticed that by default EDR is disabled, when a dongle is detected
> net/bluetooth/hci_core.c initializes the packet types with DM1, DH1 and
> HV1, and then net/bluetooth/hci_event.c does it for DM3-5 DH3-5 and
> HV3-5, but it never initializes the 2DHx or 3DHx. Was it made on purpose
> or is it a bug in the code?
you have read the specification and realized that EDR uses inverse
values?
Regards
Marcel
^ permalink raw reply
* [PATCH 11/11] Bluetooth and mach-ux500: Fix of minor issues
From: Par-Gunnar Hjalmdahl @ 2010-12-17 11:26 UTC (permalink / raw)
To: Pavan Savoy, Vitaly Wool, Alan Cox, Arnd Bergmann, Samuel Ortiz,
Marcel Holtmann
Cc: linux-kernel, linux-bluetooth, Lukasz Rymanowski, Linus Walleij,
Par-Gunnar Hjalmdahl, Par-Gunnar Hjalmdahl
This patch fixes some minor issues discovered late before
patch creation.
Signed-off-by: Par-Gunnar Hjalmdahl <par-gunnar.p.hjalmdahl@stericsson.com>
---
arch/arm/mach-ux500/devices-cg2900.c | 6 ++++--
drivers/bluetooth/hci_uart.h | 2 +-
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/arch/arm/mach-ux500/devices-cg2900.c b/arch/arm/mach-ux500/devices-cg2900.c
index 4dcef97..f0248e3 100644
--- a/arch/arm/mach-ux500/devices-cg2900.c
+++ b/arch/arm/mach-ux500/devices-cg2900.c
@@ -70,14 +70,16 @@ static void dcg2900_enable_chip(struct cg2900_chip_dev *dev)
{
struct dcg2900_info *info = dev->b_data;
- gpio_set_value(info->gbf_gpio, 1);
+ if (info->gbf_gpio != -1)
+ gpio_set_value(info->gbf_gpio, 1);
}
static void dcg2900_disable_chip(struct cg2900_chip_dev *dev)
{
struct dcg2900_info *info = dev->b_data;
- gpio_set_value(info->gbf_gpio, 0);
+ if (info->gbf_gpio != -1)
+ gpio_set_value(info->gbf_gpio, 0);
}
static struct sk_buff *dcg2900_get_power_switch_off_cmd
diff --git a/drivers/bluetooth/hci_uart.h b/drivers/bluetooth/hci_uart.h
index ab195fb..4c99222 100644
--- a/drivers/bluetooth/hci_uart.h
+++ b/drivers/bluetooth/hci_uart.h
@@ -35,7 +35,7 @@
#define HCIUARTGETFLAGS _IOR('U', 204, int)
/* UART protocols */
-#define HCI_UART_MAX_PROTO 6
+#define HCI_UART_MAX_PROTO 7
#define HCI_UART_H4 0
#define HCI_UART_BCSP 1
--
1.7.3.2
^ permalink raw reply related
* [PATCH 10/11] arch_mach-ux500: Add U8500 board support for CG2900
From: Par-Gunnar Hjalmdahl @ 2010-12-17 11:25 UTC (permalink / raw)
To: Pavan Savoy, Vitaly Wool, Alan Cox, Arnd Bergmann, Samuel Ortiz,
Marcel Holtmann
Cc: linux-kernel, linux-bluetooth, Lukasz Rymanowski, Linus Walleij,
Par-Gunnar Hjalmdahl, Par-Gunnar Hjalmdahl
This patch adds device settings and necessary callbacks for
the ST-Ericsson CG2900 MFD driver into the UX500 board files.
CG2900 is Connectivity combo controller supporting GPS, Bluetooth,
and FM radio.
Signed-off-by: Par-Gunnar Hjalmdahl <par-gunnar.p.hjalmdahl@stericsson.com>
---
arch/arm/mach-ux500/Makefile | 1 +
arch/arm/mach-ux500/board-mop500.c | 152 ++++++++++++++++
arch/arm/mach-ux500/devices-cg2900.c | 313 ++++++++++++++++++++++++++++++++++
arch/arm/mach-ux500/devices-cg2900.h | 19 ++
4 files changed, 485 insertions(+), 0 deletions(-)
create mode 100644 arch/arm/mach-ux500/devices-cg2900.c
create mode 100644 arch/arm/mach-ux500/devices-cg2900.h
diff --git a/arch/arm/mach-ux500/Makefile b/arch/arm/mach-ux500/Makefile
index 8411183..3497ec4 100644
--- a/arch/arm/mach-ux500/Makefile
+++ b/arch/arm/mach-ux500/Makefile
@@ -14,3 +14,4 @@ obj-$(CONFIG_REGULATOR_AB8500) += board-mop500-regulators.o
obj-$(CONFIG_U5500_MODEM_IRQ) += modem_irq.o
obj-$(CONFIG_U5500_MBOX) += mbox.o
obj-$(CONFIG_CPU_FREQ) += cpufreq.o
+obj-$(CONFIG_MFD_CG2900) += devices-cg2900.o
diff --git a/arch/arm/mach-ux500/board-mop500.c b/arch/arm/mach-ux500/board-mop500.c
index 3c3daf6..727d4a5 100644
--- a/arch/arm/mach-ux500/board-mop500.c
+++ b/arch/arm/mach-ux500/board-mop500.c
@@ -20,9 +20,13 @@
#include <linux/amba/serial.h>
#include <linux/spi/spi.h>
#include <linux/mfd/ab8500.h>
+#include <linux/mfd/cg2900.h>
#include <linux/mfd/tc35892.h>
#include <linux/input/matrix_keypad.h>
+#include <net/bluetooth/bluetooth.h>
+#include <net/bluetooth/hci.h>
+
#include <asm/mach-types.h>
#include <asm/mach/arch.h>
@@ -37,6 +41,7 @@
#include <mach/irqs.h>
#include "ste-dma40-db8500.h"
+#include "devices-cg2900.h"
#include "devices-db8500.h"
#include "pins-db8500.h"
#include "board-mop500.h"
@@ -133,6 +138,130 @@ static struct spi_board_info ab8500_spi_devices[] = {
},
};
+#ifdef CONFIG_MFD_CG2900
+#define CG2900_BT_ENABLE_GPIO 170
+#define CG2900_GBF_ENA_RESET_GPIO 171
+#define CG2900_BT_CTS_GPIO 0
+
+enum cg2900_gpio_pull_sleep cg2900_sleep_gpio[21] = {
+ CG2900_NO_PULL, /* GPIO 0: PTA_CONFX */
+ CG2900_PULL_DN, /* GPIO 1: PTA_STATUS */
+ CG2900_NO_PULL, /* GPIO 2: UART_CTSN */
+ CG2900_PULL_UP, /* GPIO 3: UART_RTSN */
+ CG2900_PULL_UP, /* GPIO 4: UART_TXD */
+ CG2900_NO_PULL, /* GPIO 5: UART_RXD */
+ CG2900_PULL_DN, /* GPIO 6: IOM_DOUT */
+ CG2900_NO_PULL, /* GPIO 7: IOM_FSC */
+ CG2900_NO_PULL, /* GPIO 8: IOM_CLK */
+ CG2900_NO_PULL, /* GPIO 9: IOM_DIN */
+ CG2900_PULL_DN, /* GPIO 10: PWR_REQ */
+ CG2900_PULL_DN, /* GPIO 11: HOST_WAKEUP */
+ CG2900_PULL_DN, /* GPIO 12: IIS_DOUT */
+ CG2900_NO_PULL, /* GPIO 13: IIS_WS */
+ CG2900_NO_PULL, /* GPIO 14: IIS_CLK */
+ CG2900_NO_PULL, /* GPIO 15: IIS_DIN */
+ CG2900_PULL_DN, /* GPIO 16: PTA_FREQ */
+ CG2900_PULL_DN, /* GPIO 17: PTA_RF_ACTIVE */
+ CG2900_NO_PULL, /* GPIO 18: NotConnected (J6428) */
+ CG2900_NO_PULL, /* GPIO 19: EXT_DUTY_CYCLE */
+ CG2900_NO_PULL, /* GPIO 20: EXT_FRM_SYNCH */
+};
+
+static struct platform_device ux500_cg2900_device = {
+ .name = "cg2900",
+};
+
+#ifdef CONFIG_MFD_CG2900_CHIP
+static struct platform_device ux500_cg2900_chip_device = {
+ .name = "cg2900-chip",
+ .dev = {
+ .parent = &ux500_cg2900_device.dev,
+ },
+};
+#endif /* CONFIG_MFD_CG2900_CHIP */
+
+#ifdef CONFIG_MFD_STLC2690_CHIP
+static struct platform_device ux500_stlc2690_chip_device = {
+ .name = "stlc2690-chip",
+ .dev = {
+ .parent = &ux500_cg2900_device.dev,
+ },
+};
+#endif /* CONFIG_MFD_STLC2690_CHIP */
+
+#ifdef CONFIG_MFD_CG2900_TEST
+static struct cg2900_platform_data cg2900_test_platform_data = {
+ .bus = HCI_VIRTUAL,
+ .gpio_sleep = cg2900_sleep_gpio,
+};
+
+static struct platform_device ux500_cg2900_test_device = {
+ .name = "cg2900-test",
+ .dev = {
+ .parent = &ux500_cg2900_device.dev,
+ .platform_data = &cg2900_test_platform_data,
+ },
+};
+#endif /* CONFIG_MFD_CG2900_TEST */
+
+#ifdef CONFIG_MFD_CG2900_UART
+static struct resource cg2900_uart_resources[] = {
+ {
+ .start = CG2900_GBF_ENA_RESET_GPIO,
+ .end = CG2900_GBF_ENA_RESET_GPIO,
+ .flags = IORESOURCE_IO,
+ .name = "gbf_ena_reset",
+ },
+ {
+ .start = CG2900_BT_ENABLE_GPIO,
+ .end = CG2900_BT_ENABLE_GPIO,
+ .flags = IORESOURCE_IO,
+ .name = "bt_enable",
+ },
+ {
+ .start = NOMADIK_GPIO_TO_IRQ(CG2900_BT_CTS_GPIO),
+ .end = NOMADIK_GPIO_TO_IRQ(CG2900_BT_CTS_GPIO),
+ .flags = IORESOURCE_IRQ,
+ .name = "cts_irq",
+ },
+};
+
+static pin_cfg_t cg2900_uart_enabled[] = {
+ GPIO0_U0_CTSn | PIN_INPUT_PULLUP,
+ GPIO1_U0_RTSn | PIN_OUTPUT_HIGH,
+ GPIO2_U0_RXD | PIN_INPUT_PULLUP,
+ GPIO3_U0_TXD | PIN_OUTPUT_HIGH
+};
+
+static pin_cfg_t cg2900_uart_disabled[] = {
+ GPIO0_GPIO | PIN_INPUT_PULLUP, /* CTS pull up. */
+ GPIO1_GPIO | PIN_OUTPUT_HIGH, /* RTS high-flow off. */
+ GPIO2_GPIO | PIN_INPUT_PULLUP, /* RX pull down. */
+ GPIO3_GPIO | PIN_OUTPUT_LOW /* TX low - break on. */
+};
+
+static struct cg2900_platform_data cg2900_uart_platform_data = {
+ .bus = HCI_UART,
+ .gpio_sleep = cg2900_sleep_gpio,
+ .uart = {
+ .n_uart_gpios = 4,
+ .uart_enabled = cg2900_uart_enabled,
+ .uart_disabled = cg2900_uart_disabled,
+ },
+};
+
+static struct platform_device ux500_cg2900_uart_device = {
+ .name = "cg2900-uart",
+ .dev = {
+ .platform_data = &cg2900_uart_platform_data,
+ .parent = &ux500_cg2900_device.dev,
+ },
+ .num_resources = ARRAY_SIZE(cg2900_uart_resources),
+ .resource = cg2900_uart_resources,
+};
+#endif /* CONFIG_MFD_CG2900_UART */
+#endif /* CONFIG_MFD_CG2900 */
+
/*
* TC35892
*/
@@ -446,6 +575,29 @@ static void __init u8500_init_machine(void)
ux500_ske_keypad_device.dev.platform_data = &ske_keypad_board;
platform_add_devices(platform_devs, ARRAY_SIZE(platform_devs));
+#ifdef CONFIG_MFD_CG2900
+#ifdef CONFIG_MFD_CG2900_TEST
+ dcg2900_init_platdata(&cg2900_test_platform_data);
+#endif /* CONFIG_MFD_CG2900_TEST */
+#ifdef CONFIG_MFD_CG2900_UART
+ dcg2900_init_platdata(&cg2900_uart_platform_data);
+#endif /* CONFIG_MFD_CG2900_UART */
+
+ platform_device_register(&ux500_cg2900_device);
+#ifdef CONFIG_MFD_CG2900_UART
+ platform_device_register(&ux500_cg2900_uart_device);
+#endif /* CONFIG_MFD_CG2900_UART */
+#ifdef CONFIG_MFD_CG2900_TEST
+ platform_device_register(&ux500_cg2900_test_device);
+#endif /* CONFIG_MFD_CG2900_TEST */
+#ifdef CONFIG_MFD_CG2900_CHIP
+ platform_device_register(&ux500_cg2900_chip_device);
+#endif /* CONFIG_MFD_CG2900_CHIP */
+#ifdef CONFIG_MFD_STLC2690_CHIP
+ platform_device_register(&ux500_stlc2690_chip_device);
+#endif /* CONFIG_MFD_STLC2690_CHIP */
+#endif /* CONFIG_MFD_CG2900 */
+
mop500_i2c_init();
mop500_sdi_init();
mop500_spi_init();
diff --git a/arch/arm/mach-ux500/devices-cg2900.c b/arch/arm/mach-ux500/devices-cg2900.c
new file mode 100644
index 0000000..4dcef97
--- /dev/null
+++ b/arch/arm/mach-ux500/devices-cg2900.c
@@ -0,0 +1,313 @@
+/*
+ * arch/arm/mach-ux500/devices-cg2900.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
+ *
+ * Board specific device support for the Linux Bluetooth HCI H:4 Driver
+ * for ST-Ericsson connectivity controller.
+ */
+#define NAME "devices-cg2900"
+#define pr_fmt(fmt) NAME ": " fmt "\n"
+
+#include <asm/byteorder.h>
+#include <asm-generic/errno-base.h>
+#include <linux/delay.h>
+#include <linux/gpio.h>
+#include <linux/interrupt.h>
+#include <linux/ioport.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/skbuff.h>
+#include <linux/string.h>
+#include <linux/types.h>
+#include <linux/mfd/cg2900.h>
+#include <plat/pincfg.h>
+
+#ifdef CONFIG_MFD_CG2900
+
+#define BT_VS_POWER_SWITCH_OFF 0xFD40
+
+#define H4_HEADER_LENGTH 0x01
+#define BT_HEADER_LENGTH 0x03
+
+#define STLC2690_HCI_REV 0x0600
+#define CG2900_PG1_HCI_REV 0x0101
+#define CG2900_PG2_HCI_REV 0x0200
+#define CG2900_PG1_SPECIAL_HCI_REV 0x0700
+
+struct vs_power_sw_off_cmd {
+ __le16 op_code;
+ u8 len;
+ u8 gpio_0_7_pull_up;
+ u8 gpio_8_15_pull_up;
+ u8 gpio_16_20_pull_up;
+ u8 gpio_0_7_pull_down;
+ u8 gpio_8_15_pull_down;
+ u8 gpio_16_20_pull_down;
+} __attribute__((packed));
+
+struct dcg2900_info {
+ int gbf_gpio;
+ int bt_gpio;
+ bool sleep_gpio_set;
+ u8 gpio_0_7_pull_up;
+ u8 gpio_8_15_pull_up;
+ u8 gpio_16_20_pull_up;
+ u8 gpio_0_7_pull_down;
+ u8 gpio_8_15_pull_down;
+ u8 gpio_16_20_pull_down;
+};
+
+static void dcg2900_enable_chip(struct cg2900_chip_dev *dev)
+{
+ struct dcg2900_info *info = dev->b_data;
+
+ gpio_set_value(info->gbf_gpio, 1);
+}
+
+static void dcg2900_disable_chip(struct cg2900_chip_dev *dev)
+{
+ struct dcg2900_info *info = dev->b_data;
+
+ gpio_set_value(info->gbf_gpio, 0);
+}
+
+static struct sk_buff *dcg2900_get_power_switch_off_cmd
+ (struct cg2900_chip_dev *dev, u16 *op_code)
+{
+ struct sk_buff *skb;
+ struct vs_power_sw_off_cmd *cmd;
+ struct dcg2900_info *info;
+ int i;
+
+ /* If connected chip does not support the command return NULL */
+ if (CG2900_PG1_SPECIAL_HCI_REV != dev->chip.hci_revision &&
+ CG2900_PG1_HCI_REV != dev->chip.hci_revision &&
+ CG2900_PG2_HCI_REV != dev->chip.hci_revision)
+ return NULL;
+
+ dev_dbg(dev->dev, "Generating PowerSwitchOff command\n");
+
+ info = dev->b_data;
+
+ skb = alloc_skb(sizeof(*cmd) + H4_HEADER_LENGTH, GFP_KERNEL);
+ if (!skb) {
+ dev_err(dev->dev, "Could not allocate skb\n");
+ return NULL;
+ }
+
+ skb_reserve(skb, H4_HEADER_LENGTH);
+ cmd = (struct vs_power_sw_off_cmd *)skb_put(skb, sizeof(*cmd));
+ cmd->op_code = cpu_to_le16(BT_VS_POWER_SWITCH_OFF);
+ cmd->len = sizeof(*cmd) - BT_HEADER_LENGTH;
+ /*
+ * Enter system specific GPIO settings here:
+ * Section data[3-5] is GPIO pull-up selection
+ * Section data[6-8] is GPIO pull-down selection
+ * Each section is a bitfield where
+ * - byte 0 bit 0 is GPIO 0
+ * - byte 0 bit 1 is GPIO 1
+ * - up to
+ * - byte 2 bit 4 which is GPIO 20
+ * where each bit means:
+ * - 0: No pull-up / no pull-down
+ * - 1: Pull-up / pull-down
+ * All GPIOs are set as input.
+ */
+ if (!info->sleep_gpio_set) {
+ struct cg2900_platform_data *pf_data;
+
+ pf_data = dev_get_platdata(dev->dev);
+ for (i = 0; i < 8; i++) {
+ if (pf_data->gpio_sleep[i] == CG2900_PULL_UP)
+ info->gpio_0_7_pull_up |= (1 << i);
+ else if (pf_data->gpio_sleep[i] == CG2900_PULL_DN)
+ info->gpio_0_7_pull_down |= (1 << i);
+ }
+ for (i = 8; i < 16; i++) {
+ if (pf_data->gpio_sleep[i] == CG2900_PULL_UP)
+ info->gpio_8_15_pull_up |= (1 << (i - 8));
+ else if (pf_data->gpio_sleep[i] == CG2900_PULL_DN)
+ info->gpio_8_15_pull_down |= (1 << (i - 8));
+ }
+ for (i = 16; i < 21; i++) {
+ if (pf_data->gpio_sleep[i] == CG2900_PULL_UP)
+ info->gpio_16_20_pull_up |= (1 << (i - 16));
+ else if (pf_data->gpio_sleep[i] == CG2900_PULL_DN)
+ info->gpio_16_20_pull_down |= (1 << (i - 16));
+ }
+ info->sleep_gpio_set = true;
+ }
+ cmd->gpio_0_7_pull_up = info->gpio_0_7_pull_up;
+ cmd->gpio_8_15_pull_up = info->gpio_8_15_pull_up;
+ cmd->gpio_16_20_pull_up = info->gpio_16_20_pull_up;
+ cmd->gpio_0_7_pull_down = info->gpio_0_7_pull_down;
+ cmd->gpio_8_15_pull_down = info->gpio_8_15_pull_down;
+ cmd->gpio_16_20_pull_down = info->gpio_16_20_pull_down;
+
+
+ if (op_code)
+ *op_code = BT_VS_POWER_SWITCH_OFF;
+
+ return skb;
+}
+
+static int dcg2900_init(struct cg2900_chip_dev *dev)
+{
+ int err = 0;
+ struct dcg2900_info *info;
+ struct resource *resource;
+ const char *gbf_name;
+ const char *bt_name = NULL;
+
+ /* First retrieve and save the resources */
+ info = kzalloc(sizeof(*info), GFP_KERNEL);
+ if (!info) {
+ dev_err(dev->dev, "Could not allocate dcg2900_info\n");
+ return -ENOMEM;
+ }
+
+ if (!dev->pdev->num_resources) {
+ dev_dbg(dev->dev, "No resources available\n");
+ info->gbf_gpio = -1;
+ info->bt_gpio = -1;
+ goto finished;
+ }
+
+ resource = platform_get_resource(dev->pdev, IORESOURCE_IO, 0);
+ if (!resource) {
+ dev_err(dev->dev, "GBF GPIO does not exist\n");
+ err = -EINVAL;
+ goto err_handling;
+ }
+ info->gbf_gpio = resource->start;
+ gbf_name = resource->name;
+
+ resource = platform_get_resource(dev->pdev, IORESOURCE_IO, 1);
+ /* BT Enable GPIO may not exist */
+ if (resource) {
+ info->bt_gpio = resource->start;
+ bt_name = resource->name;
+ }
+
+ /* Now setup the GPIOs */
+ err = gpio_request(info->gbf_gpio, gbf_name);
+ if (err < 0) {
+ dev_err(dev->dev, "gpio_request failed with err: %d\n", err);
+ goto err_handling;
+ }
+
+ err = gpio_direction_output(info->gbf_gpio, 0);
+ if (err < 0) {
+ dev_err(dev->dev, "gpio_direction_output failed with err: %d\n",
+ err);
+ goto err_handling_free_gpio_gbf;
+ }
+
+ if (!bt_name) {
+ info->bt_gpio = -1;
+ goto finished;
+ }
+
+ err = gpio_request(info->bt_gpio, bt_name);
+ if (err < 0) {
+ dev_err(dev->dev, "gpio_request failed with err: %d\n", err);
+ goto err_handling_free_gpio_gbf;
+ }
+
+ err = gpio_direction_output(info->bt_gpio, 1);
+ if (err < 0) {
+ dev_err(dev->dev, "gpio_direction_output failed with err: %d\n",
+ err);
+ goto err_handling_free_gpio_bt;
+ }
+
+finished:
+ dev->b_data = info;
+ return 0;
+
+err_handling_free_gpio_bt:
+ gpio_free(info->bt_gpio);
+err_handling_free_gpio_gbf:
+ gpio_free(info->gbf_gpio);
+err_handling:
+ kfree(info);
+ return err;
+}
+
+static void dcg2900_exit(struct cg2900_chip_dev *dev)
+{
+ struct dcg2900_info *info = dev->b_data;
+
+ dcg2900_disable_chip(dev);
+ if (info->bt_gpio != -1)
+ gpio_free(info->bt_gpio);
+ if (info->gbf_gpio != -1)
+ gpio_free(info->gbf_gpio);
+ kfree(info);
+ dev->b_data = NULL;
+}
+
+#ifdef CONFIG_MFD_CG2900_UART
+static int dcg2900_disable_uart(struct cg2900_chip_dev *dev)
+{
+ int err;
+ struct cg2900_platform_data *pdata = dev_get_platdata(dev->dev);
+
+ /*
+ * Without this delay we get interrupt on CTS immediately
+ * due to some turbulences on this line.
+ */
+ mdelay(4);
+
+ /* Disable UART functions. */
+ err = nmk_config_pins(pdata->uart.uart_disabled,
+ pdata->uart.n_uart_gpios);
+ if (err)
+ goto error;
+
+ return 0;
+
+error:
+ (void)nmk_config_pins(pdata->uart.uart_enabled,
+ pdata->uart.n_uart_gpios);
+ dev_err(dev->dev, "Cannot set interrupt (%d)\n", err);
+ return err;
+}
+
+static int dcg2900_enable_uart(struct cg2900_chip_dev *dev)
+{
+ int err;
+ struct cg2900_platform_data *pdata = dev_get_platdata(dev->dev);
+
+ /* Restore UART settings. */
+ err = nmk_config_pins(pdata->uart.uart_enabled,
+ pdata->uart.n_uart_gpios);
+ if (err)
+ dev_err(dev->dev, "Unable to enable UART (%d)\n", err);
+
+ return err;
+}
+#endif /* CONFIG_MFD_CG2900_UART */
+
+void dcg2900_init_platdata(struct cg2900_platform_data *data)
+{
+ data->init = dcg2900_init;
+ data->exit = dcg2900_exit;
+ data->enable_chip = dcg2900_enable_chip;
+ data->disable_chip = dcg2900_disable_chip;
+ data->get_power_switch_off_cmd = dcg2900_get_power_switch_off_cmd;
+#ifdef CONFIG_MFD_CG2900_UART
+ data->uart.enable_uart = dcg2900_enable_uart;
+ data->uart.disable_uart = dcg2900_disable_uart;
+#endif /* CONFIG_MFD_CG2900_UART */
+}
+#endif /* CONFIG_MFD_CG2900 */
diff --git a/arch/arm/mach-ux500/devices-cg2900.h b/arch/arm/mach-ux500/devices-cg2900.h
new file mode 100644
index 0000000..958b750
--- /dev/null
+++ b/arch/arm/mach-ux500/devices-cg2900.h
@@ -0,0 +1,19 @@
+/*
+ * Copyright (C) ST-Ericsson SA 2010
+ *
+ * Author: Par-Gunnar Hjalmdahl <par-gunnar.p.hjalmdahl@stericsson.com>
+ * License terms: GNU General Public License (GPL), version 2.
+ */
+
+#ifndef __DEVICES_CG2900_H
+#define __DEVICES_CG2900_H
+
+#include <linux/mfd/cg2900.h>
+
+/**
+ * dcg2900_init_platdata() - Initializes platform data with callback functions.
+ * @data: Platform data.
+ */
+extern void dcg2900_init_platdata(struct cg2900_platform_data *data);
+
+#endif /* __DEVICES_CG2900_H */
--
1.7.3.2
^ permalink raw reply related
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