Linux bluetooth development
 help / color / mirror / Atom feed
* [PATCH 1/6] Bluetooth: Add SMP command structures
From: Anderson Briglia @ 2010-10-22 23:56 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Ville Tervo

From: Ville Tervo <ville.tervo@nokia.com>

Add command structures for security manager protocol.

Signed-off-by: Ville Tervo <ville.tervo@nokia.com>
---
 include/net/bluetooth/smp.h |   76 +++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 76 insertions(+), 0 deletions(-)
 create mode 100644 include/net/bluetooth/smp.h

diff --git a/include/net/bluetooth/smp.h b/include/net/bluetooth/smp.h
new file mode 100644
index 0000000..8f2edbf
--- /dev/null
+++ b/include/net/bluetooth/smp.h
@@ -0,0 +1,76 @@
+#ifndef __SMP_H
+#define __SMP_H
+
+struct smp_command_hdr {
+	__u8	code;
+} __packed;
+
+#define SMP_CMD_PAIRING_REQ	0x01
+#define SMP_CMD_PAIRING_RSP	0x02
+struct smp_cmd_pairing {
+	__u8	io_capability;
+	__u8	oob_flag;
+	__u8	auth_req;
+	__u8	max_key_size;
+	__u8	init_key_dist;
+	__u8	resp_key_dist;
+} __packed;
+
+#define SMP_CMD_PAIRING_CONFIRM	0x03
+struct smp_cmd_pairing_confirm {
+	__u8	confirm_val[16];
+} __packed;
+
+#define SMP_CMD_PAIRING_RANDOM	0x04
+struct smp_cmd_pairing_random {
+	__u8	rand_val[16];
+} __packed;
+
+#define SMP_CMD_PAIRING_FAIL	0x05
+struct smp_cmd_pairing_fail {
+	__u8	reason;
+} __packed;
+
+#define SMP_CMD_ENCRYPT_INFO	0x06
+struct smp_cmd_encrypt_info {
+	__u8	ltk[16];
+} __packed;
+
+#define SMP_CMD_MASTER_IDENT	0x07
+struct smp_cmd_master_ident {
+	__u16	ediv;
+	__u8	rand[8];
+} __packed;
+
+#define SMP_CMD_IDENT_INFO	0x08
+struct smp_cmd_ident_info {
+	__u8	irk[16];
+} __packed;
+
+#define SMP_CMD_IDENT_ADDR_INFO	0x09
+struct smp_cmd_ident_addr_info {
+	__u8	addr_type;
+	bdaddr_t bdaddr;
+} __packed;
+
+#define SMP_CMD_SIGN_INFO	0x0a
+struct smp_cmd_sign_info {
+	__u8	csrk[16];
+} __packed;
+
+#define SMP_CMD_SECURITY_REQ	0x0b
+struct smp_cmd_security_req {
+	__u8	auth_req;
+} __packed;
+
+#define SMP_PASSKEY_ENTRY_FAILED	0x01
+#define SMP_OOB_NOT_AVAIL		0x02
+#define SMP_AUTH_REQUIREMENTS		0x03
+#define SMP_CONFIRM_FAILED		0x04
+#define SMP_PAIRING_NOTSUPP		0x05
+#define SMP_ENC_KEY_SIZE		0x06
+#define SMP_CMD_NOTSUPP		0x07
+#define SMP_UNSPECIFIED		0x08
+#define SMP_REPEATED_ATTEMPTS		0x09
+
+#endif /* __SMP_H */
-- 
1.7.0.4


^ permalink raw reply related

* [RFC] SMP initial draft
From: Anderson Briglia @ 2010-10-22 23:56 UTC (permalink / raw)
  To: linux-bluetooth

This RFC is about Security Manager Protocol (SMP).
Basically this patchset implements the initial negotiation over L2CAP and LE
connections between a Master and a Slave. TK and STK keys are not being generated
and/or exchanged yet, do not expect real encryption/decryption by now.
Actually, our next tasks are related to integrate current implementation and
Criptographic Toolbox ah, c1 and s1 functions for TK and STK key generation for
Just Works SMP pairing method.

To test this RFC you need to have the Ville Tervo[1] kernel tree with LE connection
patches. Of course you need LE devices and bluez git tree (master branch). Just run
the bluetoothd and try to list the primary services using gatttool over an LE channel.
eg.: gatttool --primary --le -i hci0 -b 00:17:E7:DD:08:FF

Comments are welcome.

[1] git://git.kernel.org/pub/scm/linux/kernel/git/vtervo/bluetooth-le-2.6.git

[PATCH 1/6] Bluetooth: Add SMP command structures
[PATCH 2/6] Bluetooth: fix receiving L2CAP packets over LE
[PATCH 3/6] Bluetooth: Implement the first SMP commands
[PATCH 4/6] Bluetooth: Start SMP procedure
[PATCH 5/6] Bluetooth: Fix initiated LE connections
[PATCH 6/6] Bluetooth: simple SMP pairing negotiation

^ permalink raw reply

* Re: [PATCH] Fix crash when GetProperties req is received before any adapters are set up
From: Johan Hedberg @ 2010-10-22 20:40 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: ext-tommi.keisala, linux-bluetooth
In-Reply-To: <AANLkTimwz0wYbiQpkPmJuNpmJxNxaAQ2UtudPqinh9kP@mail.gmail.com>

Hi Luiz,

On Fri, Oct 22, 2010, Luiz Augusto von Dentz wrote:
> Maybe we should also add a check here to omit adapters if we don't
> have at least one to append, it should be fine to have empty
> containers but I don't think this is very useful for the application
> since they have to iterate in the container to find out there is in
> fact nothing there.

Wouldn't it complicate e.g. python code in that you'd need

if manager_props.has_key('Adapters'):
	for adapter in manager_props['Adapters']:
		...

to avoid triggering a KeyError exception which could happen if you try
to access manager_props['Adapters'] directly.

Johan

^ permalink raw reply

* Re: [PATCH] Fix crash when GetProperties req is received before any adapters are set up
From: Luiz Augusto von Dentz @ 2010-10-22 20:25 UTC (permalink / raw)
  To: ext-tommi.keisala, linux-bluetooth
In-Reply-To: <20101022131207.GA29655@jh-x301>

Hi,

On Fri, Oct 22, 2010 at 4:12 PM, Johan Hedberg <johan.hedberg@nokia.com> wrote:
> Hi Tommi,
>
> On Fri, Oct 22, 2010, ext-tommi.keisala@nokia.com wrote:
>> This patch avoids a crash when org.bluez.Manager GetProperties request is
>> received and there is not yet any adapters ready. Happens often for example
>> when bluetoothd and ofonod is started next ot each other.
>>
>> Signed-off-by: Tommi Keisala <ext-tommi.keisala@nokia.com>
>> ---
>>  src/manager.c |    6 +++++-
>>  1 files changed, 5 insertions(+), 1 deletions(-)
>
> Looks like a bugfix is definitely in order here, but why introduce a new
> variable to the function? Wouldn't something like the folling be good
> enough:
>
> --- a/src/manager.c
> +++ b/src/manager.c
> @@ -197,13 +197,14 @@ static DBusMessage *get_properties(DBusConnection *conn,
>                        DBUS_DICT_ENTRY_END_CHAR_AS_STRING, &dict);
>
>        array = g_new0(char *, g_slist_length(adapters) + 1);
> -       for (i = 0, list = adapters; list; list = list->next, i++) {
> +       for (i = 0, list = adapters; list; list = list->next) {
>                struct btd_adapter *adapter = list->data;
>
>                if (!adapter_is_ready(adapter))
>                        continue;
>
>                array[i] = (char *) adapter_get_path(adapter);
> +               i++;
>        }
>        dict_append_array(&dict, "Adapters", DBUS_TYPE_OBJECT_PATH, &array, i);
>        g_free(array);
>
> Could you send a new patch which doesn't introduce a new variable? Also,
> please leave out the signed-off-by from the commit message since it's
> not used for userspace bluez patches. Thanks.

Maybe we should also add a check here to omit adapters if we don't
have at least one to append, it should be fine to have empty
containers but I don't think this is very useful for the application
since they have to iterate in the container to find out there is in
fact nothing there.


-- 
Luiz Augusto von Dentz
Computer Engineer

^ permalink raw reply

* (Health) ChannelConnected signal when MDL aborted?
From: Elvis Pfützenreuter @ 2010-10-22 20:10 UTC (permalink / raw)
  To: Santiago Carot-Nemesio, Jose Antonio Santos Cadenas; +Cc: linux-bluetooth

Taking a look in the code, I saw that hdp_mcap_mdl_aborted_cb() emits a ChannelConnected signal. 

I understand that after an abort, the MDL still exists, and can be reconnected, so I understand the intention of informing the application that a channel has been "created". But I'm not sure that it is opportune.

The first thing the application will do is trying to Acquire() the channel's file descriptor (that does not exist) and that triggers a reconnection attempt, from acceptor to initiator. While the most sensible thing (in my view) is waiting for the initiator to reconnect -- if it aborted, it must have a good reason, and it will probably reject the immediate call back.

What do you think?

^ permalink raw reply

* [PATCH 1/1] Fix small coding style issues
From: Elvis Pfützenreuter @ 2010-10-22 19:18 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: epx

Based on http://lxr.linux.no/linux+v2.6.36/Documentation/CodingStyle#L171
---
 health/hdp_util.c |   24 +++++++++++++-----------
 health/mcap_lib.h |    8 ++++----
 2 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/health/hdp_util.c b/health/hdp_util.c
index b0399f8..fefb6d3 100644
--- a/health/hdp_util.c
+++ b/health/hdp_util.c
@@ -181,8 +181,9 @@ static gboolean parse_role(DBusMessageIter *iter, gpointer data, GError **err)
 		dbus_message_iter_recurse(iter, &value);
 		ctype = dbus_message_iter_get_arg_type(&value);
 		string = &value;
-	} else
+	} else {
 		string = iter;
+	}
 
 	if (ctype != DBUS_TYPE_STRING) {
 		g_set_error(err, HDP_ERROR, HDP_UNSPECIFIED_ERROR,
@@ -191,11 +192,11 @@ static gboolean parse_role(DBusMessageIter *iter, gpointer data, GError **err)
 	}
 
 	dbus_message_iter_get_basic(string, &role);
-	if (g_ascii_strcasecmp(role, HDP_SINK_ROLE_AS_STRING) == 0)
+	if (g_ascii_strcasecmp(role, HDP_SINK_ROLE_AS_STRING) == 0) {
 		app->role = HDP_SINK;
-	else if (g_ascii_strcasecmp(role, HDP_SOURCE_ROLE_AS_STRING) == 0)
+	} else if (g_ascii_strcasecmp(role, HDP_SOURCE_ROLE_AS_STRING) == 0) {
 		app->role = HDP_SOURCE;
-	else {
+	} else {
 		g_set_error(err, HDP_ERROR, HDP_UNSPECIFIED_ERROR,
 			"Role value should be \"source\" or \"sink\"");
 		return FALSE;
@@ -221,8 +222,9 @@ static gboolean parse_desc(DBusMessageIter *iter, gpointer data, GError **err)
 		dbus_message_iter_recurse(iter, &variant);
 		ctype = dbus_message_iter_get_arg_type(&variant);
 		string = &variant;
-	} else
+	} else {
 		string = iter;
+	}
 
 	if (ctype != DBUS_TYPE_STRING) {
 		g_set_error(err, HDP_ERROR, HDP_DIC_ENTRY_PARSE_ERROR,
@@ -395,12 +397,12 @@ static gboolean register_service_protocols(struct hdp_adapter *adapter,
 		goto end;
 	}
 
-	if (!sdp_list_append( mcap_list, mcap_ver)) {
+	if (!sdp_list_append(mcap_list, mcap_ver)) {
 		ret = FALSE;
 		goto end;
 	}
 
-	if (!sdp_list_append( proto_list, mcap_list)) {
+	if (!sdp_list_append(proto_list, mcap_list)) {
 		ret = FALSE;
 		goto end;
 	}
@@ -442,7 +444,7 @@ static gboolean register_service_profiles(sdp_record_t *sdp_record)
 	sdp_profile_desc_t hdp_profile;
 
 	/* set hdp information */
-	sdp_uuid16_create( &hdp_profile.uuid, HDP_SVCLASS_ID);
+	sdp_uuid16_create(&hdp_profile.uuid, HDP_SVCLASS_ID);
 	hdp_profile.version = HDP_VERSION;
 	profile_list = sdp_list_append(NULL, &hdp_profile);
 	if (!profile_list)
@@ -459,7 +461,7 @@ static gboolean register_service_profiles(sdp_record_t *sdp_record)
 	return ret;
 }
 
-static gboolean register_service_aditional_protocols(
+static gboolean register_service_additional_protocols(
 						struct hdp_adapter *adapter,
 						sdp_record_t *sdp_record)
 {
@@ -502,7 +504,7 @@ static gboolean register_service_aditional_protocols(
 		goto end;
 	}
 
-	if (!sdp_list_append( proto_list, mcap_list)) {
+	if (!sdp_list_append(proto_list, mcap_list)) {
 		ret = FALSE;
 		goto end;
 	}
@@ -709,7 +711,7 @@ gboolean hdp_update_sdp_record(struct hdp_adapter *adapter, GSList *app_list)
 		goto fail;
 	if (!register_service_profiles(sdp_record))
 		goto fail;
-	if (!register_service_aditional_protocols(adapter, sdp_record))
+	if (!register_service_additional_protocols(adapter, sdp_record))
 		goto fail;
 
 	sdp_set_info_attr(sdp_record, HDP_SERVICE_NAME, HDP_SERVICE_PROVIDER,
diff --git a/health/mcap_lib.h b/health/mcap_lib.h
index daee1f8..7740623 100644
--- a/health/mcap_lib.h
+++ b/health/mcap_lib.h
@@ -69,7 +69,7 @@ struct sync_info_ind_data;
 
 /************ Callbacks ************/
 
-/* mdl callbacks */
+/* MDL callbacks */
 
 typedef void (* mcap_mdl_event_cb) (struct mcap_mdl *mdl, gpointer data);
 typedef void (* mcap_mdl_operation_conf_cb) (struct mcap_mdl *mdl, uint8_t conf,
@@ -85,7 +85,7 @@ typedef uint8_t (* mcap_remote_mdl_conn_req_cb) (struct mcap_mcl *mcl,
 typedef uint8_t (* mcap_remote_mdl_reconn_req_cb) (struct mcap_mdl *mdl,
 						gpointer data);
 
-/* mcl callbacks */
+/* MCL callbacks */
 
 typedef void (* mcap_mcl_event_cb) (struct mcap_mcl *mcl, gpointer data);
 typedef void (* mcap_mcl_connect_cb) (struct mcap_mcl *mcl, GError *err,
@@ -115,7 +115,7 @@ typedef void (* mcap_sync_set_cb) (struct mcap_mcl *mcl,
 
 /************ Operations ************/
 
-/* Mdl operations*/
+/* MDL operations */
 
 gboolean mcap_create_mdl(struct mcap_mcl *mcl,
 				uint8_t mdepid,
@@ -155,7 +155,7 @@ gboolean mcap_mdl_abort(struct mcap_mdl *mdl,
 int mcap_mdl_get_fd(struct mcap_mdl *mdl);
 uint16_t mcap_mdl_get_mdlid(struct mcap_mdl *mdl);
 
-/* Mcl operations*/
+/* MCL operations */
 
 gboolean mcap_create_mcl(struct mcap_instance *ms,
 				const bdaddr_t *addr,
-- 
1.7.0.4


^ permalink raw reply related

* Re: [PATCH 4/6] Bluetooth: Add LE connection support to L2CAP
From: Gustavo F. Padovan @ 2010-10-22 19:14 UTC (permalink / raw)
  To: Ville Tervo; +Cc: linux-bluetooth
In-Reply-To: <1287406976-13463-5-git-send-email-ville.tervo@nokia.com>

Hi Ville,

* Ville Tervo <ville.tervo@nokia.com> [2010-10-18 16:02:54 +0300]:

> Add basic LE connection support to L2CAP. LE
> connection can be created by specifying cid
> in struct sockaddr_l2
> 
> Signed-off-by: Ville Tervo <ville.tervo@nokia.com>
> ---
>  include/net/bluetooth/l2cap.h |    3 +++
>  net/bluetooth/l2cap.c         |   32 ++++++++++++++++++++++++--------
>  2 files changed, 27 insertions(+), 8 deletions(-)
> 
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index c819c8b..cc3a140 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -160,6 +160,9 @@ struct l2cap_conn_rsp {
>  /* channel indentifier */
>  #define L2CAP_CID_SIGNALING	0x0001
>  #define L2CAP_CID_CONN_LESS	0x0002
> +#define L2CAP_CID_LE_DATA	0x0004
> +#define L2CAP_CID_LE_SIGNALING	0x0005
> +#define L2CAP_CID_SMP		0x0006
>  #define L2CAP_CID_DYN_START	0x0040
>  #define L2CAP_CID_DYN_END	0xffff
>  
> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> index 16049de..bf5daf3 100644
> --- a/net/bluetooth/l2cap.c
> +++ b/net/bluetooth/l2cap.c
> @@ -617,6 +617,12 @@ static void l2cap_conn_ready(struct l2cap_conn *conn)
>  	for (sk = l->head; sk; sk = l2cap_pi(sk)->next_c) {
>  		bh_lock_sock(sk);
>  
> +		if (conn->hcon->type == LE_LINK) {
> +			l2cap_sock_clear_timer(sk);
> +			sk->sk_state = BT_CONNECTED;
> +			sk->sk_state_change(sk);
> +		}
> +
>  		if (sk->sk_type != SOCK_SEQPACKET &&
>  				sk->sk_type != SOCK_STREAM) {
>  			l2cap_sock_clear_timer(sk);
> @@ -675,7 +681,11 @@ static struct l2cap_conn *l2cap_conn_add(struct hci_conn *hcon, u8 status)
>  
>  	BT_DBG("hcon %p conn %p", hcon, conn);
>  
> -	conn->mtu = hcon->hdev->acl_mtu;
> +	if (hcon->hdev->le_mtu && hcon->type == LE_LINK)
> +		conn->mtu = hcon->hdev->le_mtu;
> +	else
> +		conn->mtu = hcon->hdev->acl_mtu;
> +
>  	conn->src = &hcon->hdev->bdaddr;
>  	conn->dst = &hcon->dst;
>  
> @@ -1102,8 +1112,13 @@ static int l2cap_do_connect(struct sock *sk)
>  		}
>  	}
>  
> -	hcon = hci_connect(hdev, ACL_LINK, dst,
> +	if (l2cap_pi(sk)->dcid == L2CAP_CID_LE_DATA)
> +		hcon = hci_connect(hdev, LE_LINK, dst,
> +					l2cap_pi(sk)->sec_level, auth_type);
> +	else


I think that "else if (l2cap_pi(sk)->psm)" is better here, we do not
want to permit go ahead with psm 0.


-- 
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi

^ permalink raw reply

* Re: [PATCH 3/6] Bluetooth: Use LE buffers for LE traffic
From: Gustavo F. Padovan @ 2010-10-22 18:53 UTC (permalink / raw)
  To: Ville Tervo; +Cc: linux-bluetooth
In-Reply-To: <1287406976-13463-4-git-send-email-ville.tervo@nokia.com>

Hi Ville,

* Ville Tervo <ville.tervo@nokia.com> [2010-10-18 16:02:53 +0300]:

> BLuetooth chips may have separate buffers for
> LE traffic. This patch add support to use LE
> buffers provided by the chip.
> 
> Signed-off-by: Ville Tervo <ville.tervo@nokia.com>
> ---
>  include/net/bluetooth/hci.h      |    2 +
>  include/net/bluetooth/hci_core.h |    5 +++
>  net/bluetooth/hci_conn.c         |    6 +++
>  net/bluetooth/hci_core.c         |   74 +++++++++++++++++++++++++++++++++++--
>  net/bluetooth/hci_event.c        |   40 +++++++++++++++++++-
>  5 files changed, 121 insertions(+), 6 deletions(-)
> 
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index 02055b9..b42edf0 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -189,6 +189,8 @@ enum {
>  
>  #define LMP_EV4		0x01
>  #define LMP_EV5		0x02
> +#define LMP_NO_BREDR	0x20

You are not using this one.

> +#define LMP_LE		0x40
>  
>  #define LMP_SNIFF_SUBR	0x02
>  #define LMP_EDR_ESCO_2M	0x20
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index fc2aaee..326d290 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -103,15 +103,19 @@ struct hci_dev {
>  	atomic_t	cmd_cnt;
>  	unsigned int	acl_cnt;
>  	unsigned int	sco_cnt;
> +	unsigned int	le_cnt;
>  
>  	unsigned int	acl_mtu;
>  	unsigned int	sco_mtu;
> +	unsigned int	le_mtu;
>  	unsigned int	acl_pkts;
>  	unsigned int	sco_pkts;
> +	unsigned int	le_pkts;
>  
>  	unsigned long	cmd_last_tx;
>  	unsigned long	acl_last_tx;
>  	unsigned long	sco_last_tx;
> +	unsigned long	le_last_tx;
>  
>  	struct workqueue_struct	*workqueue;
>  
> @@ -469,6 +473,7 @@ void hci_conn_del_sysfs(struct hci_conn *conn);
>  #define lmp_sniffsubr_capable(dev) ((dev)->features[5] & LMP_SNIFF_SUBR)
>  #define lmp_esco_capable(dev)      ((dev)->features[3] & LMP_ESCO)
>  #define lmp_ssp_capable(dev)       ((dev)->features[6] & LMP_SIMPLE_PAIR)
> +#define lmp_le_capable(dev)        ((dev)->features[4] & LMP_LE)
>  
>  /* ----- HCI protocols ----- */
>  struct hci_proto {
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index c1eb8e0..c7309e4 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -324,6 +324,11 @@ int hci_conn_del(struct hci_conn *conn)
>  
>  		/* Unacked frames */
>  		hdev->acl_cnt += conn->sent;
> +	} else if (conn->type == LE_LINK) {
> +		if (hdev->le_pkts)
> +			hdev->le_cnt += conn->sent;
> +		else
> +			hdev->acl_cnt += conn->sent;
>  	} else {
>  		struct hci_conn *acl = conn->link;
>  		if (acl) {
> @@ -409,6 +414,7 @@ struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst, __u8
>  			return NULL;
>  
>  		hci_le_connect(le);
> +		hci_conn_hold(le);
>  

This should be in 2/6, right?


-- 
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi

^ permalink raw reply

* Re: [PATCH 2/6] Bluetooth: Add LE connect support
From: Gustavo F. Padovan @ 2010-10-22 18:46 UTC (permalink / raw)
  To: Ville Tervo; +Cc: linux-bluetooth
In-Reply-To: <1287406976-13463-3-git-send-email-ville.tervo@nokia.com>

* Ville Tervo <ville.tervo@nokia.com> [2010-10-18 16:02:52 +0300]:

> Bluetooth V4.0 adds support for Low Energy (LE)
> connections. Specification introduses new set
> of hci commands to control LE connection.
> This patch adds logic to create, cancel and
> disconnect LE connections
> 
> Signed-off-by: Ville Tervo <ville.tervo@nokia.com>
> ---
>  include/net/bluetooth/hci.h      |    2 +
>  include/net/bluetooth/hci_core.h |   21 +++++++--
>  net/bluetooth/hci_conn.c         |   51 +++++++++++++++++++-
>  net/bluetooth/hci_event.c        |   94 +++++++++++++++++++++++++++++++++++++-
>  4 files changed, 160 insertions(+), 8 deletions(-)
> 
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index ee5beec..02055b9 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -159,6 +159,8 @@ enum {
>  #define SCO_LINK	0x00
>  #define ACL_LINK	0x01
>  #define ESCO_LINK	0x02
> +/* Low Energy links do not have defined link type. Use invented one */
> +#define LE_LINK		0x80
>  
>  /* LMP features */
>  #define LMP_3SLOT	0x01
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index ebec8c9..fc2aaee 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -60,6 +60,7 @@ struct hci_conn_hash {
>  	spinlock_t       lock;
>  	unsigned int     acl_num;
>  	unsigned int     sco_num;
> +	unsigned int     le_num;
>  };
>  
>  struct bdaddr_list {
> @@ -272,20 +273,32 @@ static inline void hci_conn_hash_add(struct hci_dev *hdev, struct hci_conn *c)
>  {
>  	struct hci_conn_hash *h = &hdev->conn_hash;
>  	list_add(&c->list, &h->list);
> -	if (c->type == ACL_LINK)
> +	switch (c->type) {
> +	case ACL_LINK:
>  		h->acl_num++;
> -	else
> +		break;
> +	case LE_LINK:
> +		h->le_num++;
> +		break;
> +	default:

I would add a 'case SCO_LINK' here. It changes nothing actually, but
make switch easy to understand.

>  		h->sco_num++;
> +	}
>  }
>  
>  static inline void hci_conn_hash_del(struct hci_dev *hdev, struct hci_conn *c)
>  {
>  	struct hci_conn_hash *h = &hdev->conn_hash;
>  	list_del(&c->list);
> -	if (c->type == ACL_LINK)
> +	switch (c->type) {
> +	case ACL_LINK:
>  		h->acl_num--;
> -	else
> +		break;
> +	case LE_LINK:
> +		h->le_num--;
> +		break;
> +	default:

Same here.

>  		h->sco_num--;
> +	}
>  }
>  
>  static inline struct hci_conn *hci_conn_hash_lookup_handle(struct hci_dev *hdev,
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index 0b1e460..c1eb8e0 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -45,6 +45,32 @@
>  #include <net/bluetooth/bluetooth.h>
>  #include <net/bluetooth/hci_core.h>
>  
> +void hci_le_connect(struct hci_conn *conn)
> +{
> +	struct hci_dev *hdev = conn->hdev;
> +	struct hci_cp_le_create_conn cp;
> +
> +	conn->state = BT_CONNECT;
> +	conn->out = 1;
> +
> +	memset(&cp, 0, sizeof(cp));
> +	cp.scan_interval = cpu_to_le16(0x0004);
> +	cp.scan_window = cpu_to_le16(0x0004);
> +	bacpy(&cp.peer_addr, &conn->dst);
> +	cp.conn_interval_min = cpu_to_le16(0x0008);
> +	cp.conn_interval_max = cpu_to_le16(0x0100);
> +	cp.supervision_timeout = cpu_to_le16(0x0064);
> +	cp.min_ce_len = cpu_to_le16(0x0001);
> +	cp.max_ce_len = cpu_to_le16(0x0001);
> +
> +	hci_send_cmd(hdev, HCI_OP_LE_CREATE_CONN, sizeof(cp), &cp);
> +}
> +
> +static void hci_le_connect_cancel(struct hci_conn *conn)
> +{
> +	hci_send_cmd(conn->hdev, HCI_OP_LE_CREATE_CONN_CANCEL, 0, NULL);
> +}
> +
>  void hci_acl_connect(struct hci_conn *conn)
>  {
>  	struct hci_dev *hdev = conn->hdev;
> @@ -192,8 +218,12 @@ static void hci_conn_timeout(unsigned long arg)
>  	switch (conn->state) {
>  	case BT_CONNECT:
>  	case BT_CONNECT2:
> -		if (conn->type == ACL_LINK && conn->out)
> -			hci_acl_connect_cancel(conn);
> +		if (conn->out) {
> +			if (conn->type == ACL_LINK)
> +				hci_acl_connect_cancel(conn);
> +			else if (conn->type == LE_LINK)
> +				hci_le_connect_cancel(conn);
> +		}
>  		break;
>  	case BT_CONFIG:
>  	case BT_CONNECTED:
> @@ -359,15 +389,30 @@ struct hci_dev *hci_get_route(bdaddr_t *dst, bdaddr_t *src)
>  }
>  EXPORT_SYMBOL(hci_get_route);
>  
> -/* Create SCO or ACL connection.
> +/* Create SCO, ACL or LE connection.
>   * Device _must_ be locked */
>  struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst, __u8 sec_level, __u8 auth_type)
>  {
>  	struct hci_conn *acl;
>  	struct hci_conn *sco;
> +	struct hci_conn *le;
>  
>  	BT_DBG("%s dst %s", hdev->name, batostr(dst));
>  
> +	if (type == LE_LINK) {
> +		le = hci_conn_hash_lookup_ba(hdev, LE_LINK, dst);
> +
> +		if (!le)
> +			le = hci_conn_add(hdev, LE_LINK, dst);
> +
> +		if (!le)
> +			return NULL;
> +
> +		hci_le_connect(le);
> +
> +		return le;
> +	}
> +
>  	if (!(acl = hci_conn_hash_lookup_ba(hdev, ACL_LINK, dst))) {
>  		if (!(acl = hci_conn_add(hdev, ACL_LINK, dst)))
>  			return NULL;
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 84093b0..4061613 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -822,6 +822,43 @@ static void hci_cs_exit_sniff_mode(struct hci_dev *hdev, __u8 status)
>  	hci_dev_unlock(hdev);
>  }
>  
> +static void hci_cs_le_create_conn(struct hci_dev *hdev, __u8 status)
> +{
> +	struct hci_cp_le_create_conn *cp;
> +	struct hci_conn *conn;
> +
> +	BT_DBG("%s status 0x%x", hdev->name, status);
> +
> +	cp = hci_sent_cmd_data(hdev, HCI_OP_LE_CREATE_CONN);
> +	if (!cp)
> +		return;
> +
> +	hci_dev_lock(hdev);
> +
> +	conn = hci_conn_hash_lookup_ba(hdev, LE_LINK, &cp->peer_addr);
> +
> +	BT_DBG("%s bdaddr %s conn %p", hdev->name, batostr(&cp->peer_addr),
> +		conn);
> +
> +	if (status) {
> +		if (conn && conn->state == BT_CONNECT) {
> +			conn->state = BT_CLOSED;
> +			hci_proto_connect_cfm(conn, status);
> +			hci_conn_del(conn);
> +		}
> +	} else {
> +		if (!conn) {
> +			conn = hci_conn_add(hdev, LE_LINK, &cp->peer_addr);
> +			if (conAvoid things like that in your patch.n)
> +				conn->out = 1;
> +			else
> +				BT_ERR("No memory for new connection");
> +		}
> +	}
> +
> +	hci_dev_unlock(hdev);
> +}
> +
>  static inline void hci_inquiry_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
>  {
>  	__u8 status = *((__u8 *) skb->data);
> @@ -1024,7 +1061,6 @@ static inline void hci_disconn_complete_evt(struct hci_dev *hdev, struct sk_buff
>  	conn = hci_conn_hash_lookup_handle(hdev, __le16_to_cpu(ev->handle));
>  	if (conn) {
>  		conn->state = BT_CLOSED;
> -

Avoid things like that in your patch.


-- 
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi

^ permalink raw reply

* Re: [PATCH 2/2 v2] Bluetooth: Fix system crash bug of no send queue protect
From: Gustavo F. Padovan @ 2010-10-22 17:34 UTC (permalink / raw)
  To: Haijun Liu; +Cc: linux-bluetooth
In-Reply-To: <1287714419-13545-2-git-send-email-haijun.liu@atheros.com>

* Haijun Liu <haijun.liu@atheros.com> [2010-10-22 10:26:59 +0800]:

> During test session with another vendor's bt stack, found that
> without lock protect for TX_QUEUE(sk) will cause system crash while
> data transfer over AMP controller. So I just add lock protect for
> TX_QUEUE(sk).

We already use the default socket lock protection. Is it not working for
you? Why? Could you show a crash case that requires your patch to fix
it?

-- 
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi

^ permalink raw reply

* Re: [PATCH 1/2 v2] Bluetooth: Fix system crash caused by del_timer()
From: Gustavo F. Padovan @ 2010-10-22 17:18 UTC (permalink / raw)
  To: Haijun Liu; +Cc: linux-bluetooth
In-Reply-To: <1287714419-13545-1-git-send-email-haijun.liu@atheros.com>

* Haijun Liu <haijun.liu@atheros.com> [2010-10-22 10:26:58 +0800]:

> During test session with another vendor's bt stack, found that in
> l2cap_chan_del() using del_timer() caused l2cap_monitor_timeout()
> be called after the sock was freed, so it raised a system crash.
> So I just replaced del_timer() with del_timer_sync() to solve it.

NAK on this. If you read the del_timer_sync() documentation you can
see that you can't call del_timer_sync() on interrupt context. The
possible solution here is to check in the beginning of
l2cap_monitor_timeout() if your sock is still valid.

-- 
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi

^ permalink raw reply

* Re: [PATCH 2/9] mfd: Add char devices for the ST-Ericsson CG2900.
From: Arnd Bergmann @ 2010-10-22 15:49 UTC (permalink / raw)
  To: pghatwork; +Cc: linus.walleij, linux-bluetooth, linux-kernel
In-Reply-To: <AANLkTimzWPqHBSDyeoObAcxewg_adubgkt1KQYUmpBRA@mail.gmail.com>

On Friday 22 October 2010 12:36:09 Par-Gunnar Hjalmdahl wrote:
> This patch adds char devices to the ST-Ericsson CG2900 driver.
> The reason for this is to allow users of CG2900, such as GPS, to
> be placed in user space.

Can you be more specific how you expect this to be used?

I guess you have hardware units that then each correspond to
one character device and can be talked to over a pseudo socket
interface, right?

For most devices (radio, audio, bluetooth, gps, ...), we already
have existing user interfaces, so how do they interact with this
one?

> +#define NAME					"CharDev "

?

> +/* Ioctls */
> +#define CG2900_CHAR_DEV_IOCTL_RESET		_IOW('U', 210, int)
> +#define CG2900_CHAR_DEV_IOCTL_CHECK4RESET	_IOR('U', 212, int)
> +#define CG2900_CHAR_DEV_IOCTL_GET_REVISION	_IOR('U', 213, int)
> +#define CG2900_CHAR_DEV_IOCTL_GET_SUB_VER	_IOR('U', 214, int)

These definitions look wrong -- you never use the ioctl argument...

> + *
> + * Returns:
> + *   Bytes successfully read (could be 0).
> + *   -EBADF if NULL pointer was supplied in private data.
> + *   -EFAULT if copy_to_user fails.
> + *   Error codes from wait_event_interruptible.
> + */
> +static ssize_t char_dev_read(struct file *filp, char __user *buf, size_t count,
> +			     loff_t *f_pos)

The same comment applies here that I made for the test interface:
Why is this not an AF_BLUETOOTH socket instead of a chardev?

Unless I'm mistaken, you actually send bluetooth frames after all.

> +	case CG2900_CHAR_DEV_IOCTL_RESET:
> +		if (!dev) {
> +			err = -EBADF;
> +			goto error_handling;
> +		}
> +		CG2900_INFO("ioctl reset command for device %s", dev->name);
> +		err = cg2900_reset(dev->dev);
> +		break;
> +
> +	case CG2900_CHAR_DEV_IOCTL_CHECK4RESET:
> +		if (!dev) {
> +			CG2900_INFO("ioctl check for reset command for device");
> +			/* Return positive value if closed */
> +			err = CG2900_CHAR_DEV_IOCTL_EVENT_CLOSED;
> +		} else if (dev->reset_state == CG2900_CHAR_RESET) {
> +			CG2900_INFO("ioctl check for reset command for device "
> +				    "%s", dev->name);
> +			/* Return positive value if reset */
> +			err = CG2900_CHAR_DEV_IOCTL_EVENT_RESET;
> +		}
> +		break;

Strange interface. Why do you need to check for the reset?

> +	case CG2900_CHAR_DEV_IOCTL_GET_REVISION:
> +		CG2900_INFO("ioctl check for local revision info");
> +		if (cg2900_get_local_revision(&rev_data)) {
> +			CG2900_DBG("Read revision data revision %d "
> +				   "sub_version %d",
> +				   rev_data.revision, rev_data.sub_version);
> +			err = rev_data.revision;
> +		} else {
> +			CG2900_DBG("No revision data available");
> +			err = -EIO;
> +		}
> +		break;
> +
> +	case CG2900_CHAR_DEV_IOCTL_GET_SUB_VER:
> +		CG2900_INFO("ioctl check for local sub-version info");
> +		if (cg2900_get_local_revision(&rev_data)) {
> +			CG2900_DBG("Read revision data revision %d "
> +				   "sub_version %d",
> +				   rev_data.revision, rev_data.sub_version);
> +			err = rev_data.sub_version;
> +		} else {
> +			CG2900_DBG("No revision data available");
> +			err = -EIO;
> +		}
> +		break;

These look like could better live in a sysfs attribute of the platform device.

	Arnd

^ permalink raw reply

* Re: [PATCH 1/9] mfd: Add support for the ST-Ericsson CG2900.
From: Arnd Bergmann @ 2010-10-22 15:36 UTC (permalink / raw)
  To: Par-Gunnar Hjalmdahl; +Cc: linus.walleij, linux-bluetooth, linux-kernel
In-Reply-To: <AANLkTineXA2ZY0J7+dZhqywz+4aAVgzihu+LDDbsL8mu@mail.gmail.com>

On Friday 22 October 2010 12:35:16 Par-Gunnar Hjalmdahl wrote:
> This patch adds support for the ST-Ericsson CG2900 Connectivity
> Combo controller.
> This patch adds the central framework to be able to register CG2900 users,
> transports, and chip handlers.
> 
> Signed-off-by: Par-Gunnar Hjalmdahl <par-gunnar.p.hjalmdahl@stericsson.com>

Looks ok from a coding style perspective, but some important information is
missing from the description:

* What is a CG2900?
* Why is it a MFD device rather than e.g. a bus or a subsystem?

> +config MFD_CG2900
> +	tristate "Support ST-Ericsson CG2900 main structure"
> +	depends on NET
> +	help
> +	  Support for ST-Ericsson CG2900 Connectivity Combo controller main structure.
> +	  Supports multiple functionalities muxed over a Bluetooth HCI H:4 interface.
> +	  CG2900 support Bluetooth, FM radio, and GPS.
> +

Can you explain what it means to mux over a H:4 interface? Does this mean
you use bluetooth infrastructure that is designed for wireless communication
in order to connect on-board or on-chip devices?

> @@ -0,0 +1,2401 @@
> +/*
> + * drivers/mfd/cg2900/cg2900_core.c
> + *
> + * Copyright (C) ST-Ericsson SA 2010
> + * Authors:
> + * Par-Gunnar Hjalmdahl (par-gunnar.p.hjalmdahl@stericsson.com) for
> ST-Ericsson.

Your email client rewraps lines. You need to fix so that other people
can apply your patches.

> +/*
> + * chip_handlers - List of the register handlers for different chips.
> + */
> +LIST_HEAD(chip_handlers);

Should this be static? Don't you need a lock to access the list?

> +/**
> + * find_h4_user() - Get H4 user based on supplied H4 channel.
> + * @h4_channel:	H4 channel.
> + * @dev:	Stored CG2900 device.
> + * @skb:	(optional) skb with received packet. Set to NULL if NA.
> + *
> + * Returns:
> + *   0 if there is no error.
> + *   -EINVAL if bad channel is supplied or no user was found.
> + *   -ENXIO if channel is audio channel but not registered with CG2900.
> + */
> +static int find_h4_user(int h4_channel, struct cg2900_device **dev,
> +			const struct sk_buff * const skb)
> +{
> +	int err = 0;
> +	struct cg2900_users *users = &(core_info->users);
> +	struct cg2900_h4_channels *chan = &(core_info->h4_channels);
> +
> +	if (h4_channel == chan->bt_cmd_channel) {
> +		*dev = users->bt_cmd;
> +	} else if (h4_channel == chan->bt_acl_channel) {
> +		*dev = users->bt_acl;
> +	} else if (h4_channel == chan->bt_evt_channel) {
> +		*dev = users->bt_evt;
> +		/* Check if it's event generated by previously sent audio user
> +		 * command. If so then that event should be dispatched to audio
> +		 * user*/
> +		err = find_bt_audio_user(h4_channel, dev, skb);
> +	} else if (h4_channel == chan->gnss_channel) {
> +		*dev = users->gnss;
> +	} else if (h4_channel == chan->fm_radio_channel) {
> +		*dev = users->fm_radio;
> +		/* Check if it's an event generated by previously sent audio
> +		 * user command. If so then that event should be dispatched to
> +		 * audio user */
> +		err = find_fm_audio_user(h4_channel, dev, skb);
> +	} else if (h4_channel == chan->debug_channel) {
> +		*dev = users->debug;
> +	} else if (h4_channel == chan->ste_tools_channel) {
> +		*dev = users->ste_tools;
> +	} else if (h4_channel == chan->hci_logger_channel) {
> +		*dev = users->hci_logger;
> +	} else if (h4_channel == chan->us_ctrl_channel) {
> +		*dev = users->us_ctrl;
> +	} else if (h4_channel == chan->core_channel) {
> +		*dev = users->core;
> +	} else {
> +		*dev = NULL;
> +		CG2900_ERR("Bad H:4 channel supplied: 0x%X", h4_channel);
> +		return -EINVAL;
> +	}
> +
> +	return err;
> +}

You seem to have a number of functions that need to go through each
possible user/channel/submodule. This looks like somethin is not
abstracted well enough.

> +
> +/**
> + * add_h4_user() - Add H4 user to user storage based on supplied H4 channel.
> + * @dev:	Stored CG2900 device.
> + * @name:	Device name to identify different devices that are using
> + *		the same H4 channel.
> + *
> + * Returns:
> + *   0 if there is no error.
> + *   -EINVAL if NULL pointer or bad channel is supplied.
> + *   -EBUSY if there already is a user for supplied channel.
> + */
> +static int add_h4_user(struct cg2900_device *dev, const char * const name)
> +{

Where does that name come from? Why not just use an enum if the name is
only use for strncmp?

> +static ssize_t test_char_dev_read(struct file *filp, char __user *buf,
> +				  size_t count, loff_t *f_pos)
> +{
> +	struct sk_buff *skb;
> +	int bytes_to_copy;
> +	int err;
> +	struct sk_buff_head *rx_queue = &core_info->test_char_dev->rx_queue;

What is this read/write interface used for?

The name suggests that this is only for testing and not an essential
part of your driver. Could this be made a separate driver?

It also looks like you do socket communication here, can't you use
a proper AF_BLUETOOTH socket to do the same?

> +	CG2900_INFO("test_char_dev_read");
> +
> +	if (skb_queue_empty(rx_queue))
> +		wait_event_interruptible(char_wait_queue,
> +					 !(skb_queue_empty(rx_queue)));
> +
> +	skb = skb_dequeue(rx_queue);
> +	if (!skb) {
> +		CG2900_INFO("skb queue is empty - return with zero bytes");
> +		bytes_to_copy = 0;
> +		goto finished;
> +	}
> +
> +	bytes_to_copy = min(count, skb->len);
> +	err = copy_to_user(buf, skb->data, bytes_to_copy);
> +	if (err) {
> +		skb_queue_head(rx_queue, skb);
> +		return -EFAULT;
> +	}
> +
> +	skb_pull(skb, bytes_to_copy);
> +
> +	if (skb->len > 0)
> +		skb_queue_head(rx_queue, skb);
> +	else
> +		kfree_skb(skb);
> +
> +finished:
> +	return bytes_to_copy;
> +}

This does not handle short/continued reads.

> +EXPORT_SYMBOL(cg2900_reset);

How about making your symbols EXPORT_SYMBOL_GPL instead of EXPORT_SYMBOL?

> +module_param(cg2900_debug_level, int, S_IRUGO | S_IWUSR | S_IWGRP);
> +MODULE_PARM_DESC(cg2900_debug_level,
> +		 "Debug level. Default 1. Possible values:\n"
> +		 "\t0  = No debug\n"
> +		 "\t1  = Error prints\n"
> +		 "\t10 = General info, e.g. function entries\n"
> +		 "\t20 = Debug info, e.g. steps in a functionality\n"
> +		 "\t25 = Data info, i.e. prints when data is transferred\n"
> +		 "\t30 = Data content, i.e. contents of the transferred data");

Please don't introduce new debugging frameworks, we have enough of them
already. Syslog handles different levels of output just fine, so just
remove all your debugging stuff and use dev_dbg, dev_info, etc to print
messages about the device if you must.

Many messages can probably be removed entirely.

> +#define CG2900_DEFAULT_DEBUG_LEVEL 1
> +
> +/* module_param declared in cg2900_core.c */
> +extern int cg2900_debug_level;
> +
> +#if defined(NDEBUG) || CG2900_DEFAULT_DEBUG_LEVEL == 0
> +	#define CG2900_DBG_DATA_CONTENT(__prefix, __buf, __len)
> +	#define CG2900_DBG_DATA(fmt, arg...)
> +	#define CG2900_DBG(fmt, arg...)
> +	#define CG2900_INFO(fmt, arg...)
> +	#define CG2900_ERR(fmt, arg...)
> +#else
> +
> +	#define CG2900_DBG_DATA_CONTENT(__prefix, __buf, __len)		\
> +	do {								\
> +		if (cg2900_debug_level >= 30)				\
> +			print_hex_dump_bytes("CG2900 " __prefix ": " ,	\
> +					     DUMP_PREFIX_NONE, __buf, __len); \
> +	} while (0)
> +
> +	#define CG2900_DBG_DATA(fmt, arg...)				\
> +	do {								\
> +		if (cg2900_debug_level >= 25)				\
> +			pr_debug("CG2900 %s: " fmt "\n" , __func__ , ## arg); \
> +	} while (0)
> +
> +	#define CG2900_DBG(fmt, arg...)					\
> +	do {								\
> +		if (cg2900_debug_level >= 20)				\
> +			pr_debug("CG2900 %s: " fmt "\n" , __func__ , ## arg); \
> +	} while (0)
> +
> +	#define CG2900_INFO(fmt, arg...)				\
> +	do {								\
> +		if (cg2900_debug_level >= 10)				\
> +			pr_info("CG2900: " fmt "\n" , ## arg);		\
> +	} while (0)
> +
> +	#define CG2900_ERR(fmt, arg...)					\
> +	do {								\
> +		if (cg2900_debug_level >= 1)				\
> +			pr_err("CG2900 %s: " fmt "\n" , __func__ , ## arg); \
> +	} while (0)
> +
> +#endif /* NDEBUG */

You'll feel relieved when all this is gone...

> +
> +#ifndef _BLUETOOTH_DEFINES_H_
> +#define _BLUETOOTH_DEFINES_H_
> +
> +#include <linux/types.h>
> +
> +/* H:4 offset in an HCI packet */
> +#define HCI_H4_POS				0
> +#define HCI_H4_SIZE				1
> +
> +/* Standardized Bluetooth H:4 channels */
> +#define HCI_BT_CMD_H4_CHANNEL			0x01
> +#define HCI_BT_ACL_H4_CHANNEL			0x02
> +#define HCI_BT_SCO_H4_CHANNEL			0x03
> +#define HCI_BT_EVT_H4_CHANNEL			0x04
> +
> +/* Bluetooth Opcode Group Field (OGF) */
> +#define HCI_BT_OGF_LINK_CTRL			0x01
> +#define HCI_BT_OGF_LINK_POLICY			0x02
> +#define HCI_BT_OGF_CTRL_BB			0x03
> +#define HCI_BT_OGF_LINK_INFO			0x04
> +#define HCI_BT_OGF_LINK_STATUS			0x05
> +#define HCI_BT_OGF_LINK_TESTING			0x06
> +#define HCI_BT_OGF_VS				0x3F

These all look like generic bluetooth definitions, not cg2900
specific. Should they be part of global headers? Are they perhaps
already?

	Arnd

^ permalink raw reply

* Re: [PATCH 5/9] mfd: Add UART support for the ST-Ericsson CG2900.
From: Alan Cox @ 2010-10-22 15:33 UTC (permalink / raw)
  To: Par-Gunnar Hjalmdahl
  Cc: linus.walleij, linux-bluetooth, linux-kernel, Lukasz.Rymanowski
In-Reply-To: <AANLkTik-S50ZX6mFG+vWhBv0x_WmipP=t+4VqmAgssRC@mail.gmail.com>

> The existence of the callback is checked in the function
> uart_tty_open. If either break_ctl or tiocmget is NULL we do not allow
> sleep and this code will never run.

OK yes see this now.

> >> +             CG2900_ERR("Failed to alloc memory for uart_work_struct!");
> >
> > Please use the standard dev_dbg etc functionality - or pr_err etc when
> > you have no device pointer. The newest kernel tty object has a device
> > pointer added so you could use that.
> >
> 
> OK. I like the debug system we have now (using module parameter to set
> debug level in runtime), but I've received comments regarding this
> before so I assume we will have to switch to standard printouts.
> Is it OK to use defines or inline functions to add "CG2900" before and
> '\n' after (as BT_INFO in include/net/bluetooth/bluetooth.h)?

The pr_fmt bit will do what you want for a non dev_dbg type thing. See
include/linux/kernel.h

> >> + * unset_cts_irq() - Disable interrupt on CTS.
> >> + */
> >> +static void unset_cts_irq(void)
> >
> > And no ability to support multiple devices
> >
> 
> OK. We will try to fix this.

That may go away when you clean up the device side. The line discipline
can be attached to any device so must be multi-device aware, the hardware
driver can certainly be single device only if you can only ever have one

[Although its a good idea to design it so it can be fixed because you
 never know when you'll find your sales team just sold someone a two
 device solution ;) ]

> >> +             set_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
> >> +             len = tty->ops->write(tty, skb->data, skb->len);
> >
> > A tty isn't required to have a write function
> 
> I don't quite understand your comment here. This particular code is
> inspired of the Bluetooth ldisc code and there it is used like. It
> seems to work fine so we do it the same way.

You copied a security hole from the bluetooth driver which we found a
couple of weeks ago

> How would you prefer it to be?

Check it is valid, in your case probably on open just refuse to attach to
a read only port.

> OK. We will try to figure out a new design.
> I'm not too happy about putting the ldisc part in Bluetooth though
> since it is only partly Bluetooth, it is also GPS and FM. Better could
> maybe be under char/?

Works for me - there is an ongoing "we must move tty ldiscs and core tty
code somewhere more sensible of their own" discussion, so if it is
dropped into char, it'll move with them just fine.

Alan

^ permalink raw reply

* Re: [PATCH 5/9] mfd: Add UART support for the ST-Ericsson CG2900.
From: Par-Gunnar Hjalmdahl @ 2010-10-22 14:54 UTC (permalink / raw)
  To: Alan Cox; +Cc: linus.walleij, linux-bluetooth, linux-kernel, Lukasz.Rymanowski
In-Reply-To: <20101022135130.617f0ce8@lxorguk.ukuu.org.uk>

Hi and thanks for your comments Alan.
See below for answers.

/P-G

2010/10/22 Alan Cox <alan@lxorguk.ukuu.org.uk>:
>
>> + * is_chip_flow_off() - Check if chip has set flow off.
>> + * @tty:     Pointer to tty.
>> + *
>> + * Returns:
>> + *   true - chip flows off.
>> + *   false - chip flows on.
>> + */
>> +static bool is_chip_flow_off(struct tty_struct *tty)
>> +{
>> +     int lines;
>> +
>> +     lines = tty->ops->tiocmget(tty, uart_info->fd);
>> +
>> +     if (lines & TIOCM_CTS)
>> +             return false;
>> +     else
>> +             return true;
>> +}
>
> What if the device doesn't have a tiocmget ? You must check this. If you
> want to call into the ttys own tiocmget froma sleeping context fine, but
> see tty_tiocmget and you'll notice you need to check the op is non NULL
> somewhere. You could do this when the ldisc is opened and refuse to open
> - some ldiscs do that
>

The existence of the callback is checked in the function
uart_tty_open. If either break_ctl or tiocmget is NULL we do not allow
sleep and this code will never run.

>
>> +
>> +/**
>> + * create_work_item() - Create work item and add it to the work queue.
>> + * @wq:              work queue struct where the work will be added.
>> + * @work_func:       Work function.
>> + * @data:    Private data for the work.
>> + *
>> + * Returns:
>> + *   0 if there is no error.
>> + *   -EBUSY if not possible to queue work.
>> + *   -ENOMEM if allocation fails.
>> + */
>> +static int create_work_item(struct workqueue_struct *wq, work_func_t work_func,
>> +                         void *data)
>> +{
>> +     struct uart_work_struct *new_work;
>> +     int err;
>> +
>> +     new_work = kmalloc(sizeof(*new_work), GFP_ATOMIC);
>
> So instead of a tiny object embedded in your device you've got a whole
> error path to worry about, a printk disguised in a macro and a text
> string longer than the struct size ? Surely this should be part of the
> device
>

OK. We can embed the work struct in the device structure.

>> +     if (!new_work) {
>> +             CG2900_ERR("Failed to alloc memory for uart_work_struct!");
>
> Please use the standard dev_dbg etc functionality - or pr_err etc when
> you have no device pointer. The newest kernel tty object has a device
> pointer added so you could use that.
>

OK. I like the debug system we have now (using module parameter to set
debug level in runtime), but I've received comments regarding this
before so I assume we will have to switch to standard printouts.
Is it OK to use defines or inline functions to add "CG2900" before and
'\n' after (as BT_INFO in include/net/bluetooth/bluetooth.h)?
>
>> + * set_tty_baud() - Called to set specific baud in TTY.
>> + * @tty:     Tty device.
>> + * @baud:    Baud to set.
>> + *
>> + * Returns:
>> + *   true - baudrate set with success.
>> + *   false - baundrate set failure.
>> + */
>> +static bool set_tty_baud(struct tty_struct *tty, int baud)
>> +{
>> +     struct ktermios *old_termios;
>> +     bool retval = true;
>> +
>> +     old_termios = kmalloc(sizeof(*old_termios), GFP_ATOMIC);
>
> termios struct is easily small enough to be fine on the stack
>

OK.

>> +     /* Let's mark that CG2900 driver uses c_ispeed and c_ospeed fields. */
>> +     tty->termios->c_cflag |= BOTHER;
>
> This shouldn't be needed - the tty_encode_baud_rate logic works out of
> BOTHER must be set
>

I will have to check with the guy who wrote this code. I think he put
it there for a reason.

>> +     tty->termios->c_cflag &= ~BOTHER;
>
> And your termios is now potentially invalid
>

As above, I will check.

>
>> +     /* Set IRQ on CTS. */
>> +     err = request_irq(pf_data->uart.cts_irq,
>> +                       cts_interrupt,
>> +                       IRQF_TRIGGER_FALLING,
>> +                       UART_NAME,
>> +                       NULL);
>
> So we now ave terminal tty/ldisc confusion
>

The guy responsible for this is thinking through a new design, where
we can move the hardware specific handling to another place.

>> +     if (err) {
>> +             CG2900_ERR("Could not request CTS IRQ (%d)", err);
>> +             goto error;
>> +     }
>> +
>> +#ifdef CONFIG_PM
>> +     enable_irq_wake(pf_data->uart.cts_irq);
>> +#endif
>> +     return 0;
>> +
>> +error:
>> +     if (pf_data->uart.enable_uart)
>> +             (void)pf_data->uart.enable_uart();
>> +     return err;
>
>> +}
>> +
>> +/**
>> + * unset_cts_irq() - Disable interrupt on CTS.
>> + */
>> +static void unset_cts_irq(void)
>
> And no ability to support multiple devices
>

OK. We will try to fix this.

>> +     CG2900_DBG("Clear break");
>> +     tty->ops->break_ctl(tty, TTY_BREAK_OFF);
>
> What if the tty doesn't have one ?
>

See answer above. It is checked in another place.

>
>> +     if (CHIP_AWAKE == uart_info->sleep_state) {
>> +             uart_info->tty->ops->break_ctl(uart_info->tty, TTY_BREAK_ON);
>
> Ditto
>

And ditto as well :-)

>
>> +             set_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
>> +             len = tty->ops->write(tty, skb->data, skb->len);
>
> A tty isn't required to have a write function

I don't quite understand your comment here. This particular code is
inspired of the Bluetooth ldisc code and there it is used like. It
seems to work fine so we do it the same way.
How would you prefer it to be?

>
>> +             if ((BAUD_START == uart_info->baud_rate_state) &&
>> +                 (is_set_baud_rate_cmd(skb->data))) {
>> +                     CG2900_INFO("UART set baud rate cmd found.");
>> +                     SET_BAUD_STATE(BAUD_SENDING);
>
> Do we really need this all in capitals ?
>

We use a define to get a debug printout when setting state, but I
understand it's not so popular so I guess we will have to skip it.
I guess it won't help if we would use an inline function instead of a
define (which would mean we could skip the capitals)?

>
>
>> + * uart_tty_open() - Called when UART line discipline changed to N_HCI.
>> + * @tty:     Pointer to associated TTY instance data.
>> + *
>> + * Returns:
>> + *   0 if there is no error.
>> + *   Errors from cg2900_register_trans_driver.
>> + */
>> +static int uart_tty_open(struct tty_struct *tty)
>> +{
>> +     int err;
>> +
>> +     CG2900_INFO("uart_tty_open");
>> +
>> +     /* Set the structure pointers and set the UART receive room */
>> +     uart_info->tty = tty;
>
> You must respect the kref handling in the kernel tty code. Take
> references by all means but do it properly.
>

OK. We will add tty_kref_get/put handling.

>
>> +static void uart_tty_wakeup(struct tty_struct *tty)
>> +{
>> +     int err;
>> +
>> +     CG2900_INFO("uart_tty_wakeup");
>> +
>> +     /*
>> +      * Clear the TTY_DO_WRITE_WAKEUP bit that is set in
>> +      * work_do_transmit().
>> +      */
>> +     clear_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
>> +
>> +     if (tty != uart_info->tty)
>> +             return;
>
> How can this occur - and why check it after you've changed tty->flags ???
>

OK. Will be removed.

>
>> +/**
>> + * uart_tty_receive() - Called by TTY low level driver when receive
>> data is available.
>> + * @tty:     Pointer to TTY instance data
>> + * @data:    Pointer to received data
>> + * @flags:   Pointer to flags for data
>> + * @count:   Count of received data in bytes
>> + */
>> +static void uart_tty_receive(struct tty_struct *tty, const u8 *data,
>> +                          char *flags, int count)
>> +{
>> +     CG2900_INFO("uart_tty_receive");
>> +
>> +     if (tty != uart_info->tty)
>> +             return;
>
> Again this is nonsense
>

OK.Will be removed.

>
>> +/*
>> + * We don't provide read/write/poll interface for user space.
>> + */
>> +static ssize_t uart_tty_read(struct tty_struct *tty, struct file *file,
>> +                          unsigned char __user *buf, size_t nr)
>> +{
>> +     CG2900_INFO("uart_tty_read");
>> +     return 0;
>> +}
>
> -EINVAL then
>

OK. We can probably skip registering the callback function completely otherwise.

>> +
>> +static ssize_t uart_tty_write(struct tty_struct *tty, struct file *file,
>> +                           const unsigned char *data, size_t count)
>> +{
>> +     CG2900_INFO("uart_tty_write");
>> +     return count;
>
> This is wrong - you can't jusr vanish the data

OK. To be changed.

>> +}
>
>
>
> This needs some restructuring I think
>
> A line discipline should contain no hardware awareness, that goes in the
> actual uart hardware driver. So we shouldn't have magic irq code in this
> part of things.
>
> You also need to sort out the tty kref handling in open/close and the
> fact you've got magic hardcoded single instance stuff buried i nit.
>
> Finally tty ldiscs don't go buried in the mfd directory, or they'll get
> missed during chanegs/updates. The ldisc probably belongs in bluetooth a
> and the hardware support in the mfd directory.
>
>
> So - NAK this for the moment, it needs to be split cleanly into ldisc
> (thing which speaks the protocol and eg sees "speed change required" and
> acts on it) and device (thing which knows about the hardware).
>
>

OK. We will try to figure out a new design.
I'm not too happy about putting the ldisc part in Bluetooth though
since it is only partly Bluetooth, it is also GPS and FM. Better could
maybe be under char/?

^ permalink raw reply

* Re: [PATCH] bluetooth: Fix NULL pointer dereference issue
From: Gustavo F. Padovan @ 2010-10-22 13:58 UTC (permalink / raw)
  To: Yuri Ershov
  Cc: marcel, davem, jprvita, linux-bluetooth, ville.tervo,
	andrei.emeltchenko
In-Reply-To: <1987fd374e92ea2e4ebb06b24c6321e65ab933c6.1287676475.git.ext-yuri.ershov@nokia.com>

Hi Yuri,

* Yuri Ershov <ext-yuri.ershov@nokia.com> [2010-10-21 20:08:58 +0400]:

> This patch fixes NULL pointer dereference at running test with
> connect-transfer-disconnect in loop. Sometimes sk_state is 
> BT_CLOSED and sk_refcnt equal to 0, so there is oops in 
> bt_accept_unlink. In normal case removed block is not used.

Question here is: Why sk_refcnt is 0 at that point of the code?  The
socket should be destroyed if it ref is 0, but it wasn't, so something
in another point of the code went is wrong. "Sometimes" is not a good
description of the problem, you have to show why that happened.

-- 
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi

^ permalink raw reply

* Re: Pull request git://gitorious.org/bluez-mcap-hdp/mcap-hdp.git for_upstream
From: Johan Hedberg @ 2010-10-22 13:43 UTC (permalink / raw)
  To: Jose Antonio Santos Cadenas; +Cc: linux-bluetooth
In-Reply-To: <AANLkTikei6=MbLUQBYNmzbkEQ-12ufzVAf0CA2f3uLu2@mail.gmail.com>

Hi Jose,

On Thu, Oct 21, 2010, Jose Antonio Santos Cadenas wrote:
> The following changes since commit a4b1673b878a358d2492f24b46e92ea36d8f8bbf:
> 
>   Fix hidd to use ServiceName attribute for device name (2010-10-20
> 13:37:36 +0300)
> 
> are available in the git repository at:
>   git://gitorious.org/bluez-mcap-hdp/mcap-hdp.git for_upstream
> 
> José Antonio Santos Cadenas (6):
>       Receive an string instead of an integer for ChannelType
>       Modify test-health script to make it more interactive
>       Notify main channel when it is locally opened
>       Emit a valid path when main channel is deleted
>       Delete data channels when their device is removed.
>       Check if the channel mode is correct when opening data channels
> 
> Santiago Carot-Nemesio (6):
>       Remove obsolete comment from MCAP
>       Fix segmentation fault freeing uninitialized pointers
>       Check l2cap configuration when data channels are connected
>       Close the data channel if remote side changes the configuration
>       Enable support to change mode for incoming data channels connections
>       Change data channel mode for incoming connections
> 
>  health/hdp.c      |  106 +++++++++++++++++++++++++--
>  health/hdp_util.c |   35 +++++++---
>  health/mcap.c     |   15 ++++-
>  health/mcap_lib.h |    3 +
>  test/test-health  |  207 +++++++++++++++++++++++++++++++++++++++++++++++++++--
>  5 files changed, 340 insertions(+), 26 deletions(-)

Pushed upstream. Thanks.

Johan

^ permalink raw reply

* Re: [PATCH 6/9] mfd: Add support for the ST-Ericsson CG2900 Audio.
From: Par-Gunnar Hjalmdahl @ 2010-10-22 13:40 UTC (permalink / raw)
  To: Alan Cox; +Cc: linus.walleij, linux-bluetooth, linux-kernel
In-Reply-To: <20101022135804.6f55be40@lxorguk.ukuu.org.uk>

Thanks for you comments Alan.

I guess I didn't put enough comments in the code or the commit message
regarding this.
In fact we (ST-Ericsson) are writing drivers that e.g. maps to the V4L
framework. They then
call the API in this driver. This driver has never been intended to
map directly towards ALSA
or V4L. We have other drivers under development that does that.

/P-G

2010/10/22 Alan Cox <alan@lxorguk.ukuu.org.uk>:
> On Fri, 22 Oct 2010 12:39:06 +0200
> Par-Gunnar Hjalmdahl <par-gunnar.p.hjalmdahl@stericsson.com> wrote:
>
>> This patch adds support for controlling audio paths within
>> the ST-Ericsson CG2900 Connectivity Combo controller.
>> This patch adds API for the Kernel as well as a char device for
>> user space stacks to control the CG2900 audio paths.
>
> I don't see any ALSA hooks for audio or any V4L hooks for the FM radio.
> Surely the audio routing wants exposing as a mixer and the audio via
> ALSA, likewise FM radio is video4linux so that it'll have the standard
> interfaces that are expected by the rest of the Linux OS.
>
> Alan
>

^ permalink raw reply

* RFCOMM connection from android device to desktop pc
From: Leandro de Oliveira @ 2010-10-22 13:21 UTC (permalink / raw)
  To: linux-bluetooth

Hi,

I'm trying to open an RFCOMM connection from my android device to my
desktop pc running Ubuntu 10.04, send some data, close the connection
and then do it again. My desktop program is written in Java and relies
on BlueCove[1] to handle bluetooth details which calls Bluez 4.x in
this case.

The problem is that I get a successful connection and data transfer in
the first communication attempt but nothing in subsequent attempts.

There is this thread[2] in the android-platform mailing list with much
more detail, but I think you could just take a look at this hcidump
that has been requested there to let me know if this is a problem in
bluez on the android or desktop side:
http://dl.dropbox.com/u/1401233/pc-hcidump.txt

Please, let me know if you need more info.

The same program works perfectly when the android application connects
to it running on Windows 7 using the Microsoft Bluetooth Stack.

[1] http://www.bluecove.org/
[2] http://groups.google.com/group/android-platform/browse_thread/thread/485db57409304d3e?hl=en

Thank you very much

^ permalink raw reply

* Re: [PATCH] Fix crash when GetProperties req is received before any adapters are set up
From: Johan Hedberg @ 2010-10-22 13:12 UTC (permalink / raw)
  To: ext-tommi.keisala; +Cc: linux-bluetooth
In-Reply-To: <1287743558-17002-2-git-send-email-ext-tommi.keisala@nokia.com>

Hi Tommi,

On Fri, Oct 22, 2010, ext-tommi.keisala@nokia.com wrote:
> This patch avoids a crash when org.bluez.Manager GetProperties request is
> received and there is not yet any adapters ready. Happens often for example
> when bluetoothd and ofonod is started next ot each other.
> 
> Signed-off-by: Tommi Keisala <ext-tommi.keisala@nokia.com>
> ---
>  src/manager.c |    6 +++++-
>  1 files changed, 5 insertions(+), 1 deletions(-)

Looks like a bugfix is definitely in order here, but why introduce a new
variable to the function? Wouldn't something like the folling be good
enough:

--- a/src/manager.c
+++ b/src/manager.c
@@ -197,13 +197,14 @@ static DBusMessage *get_properties(DBusConnection *conn,
                        DBUS_DICT_ENTRY_END_CHAR_AS_STRING, &dict);
 
        array = g_new0(char *, g_slist_length(adapters) + 1);
-       for (i = 0, list = adapters; list; list = list->next, i++) {
+       for (i = 0, list = adapters; list; list = list->next) {
                struct btd_adapter *adapter = list->data;
 
                if (!adapter_is_ready(adapter))
                        continue;
 
                array[i] = (char *) adapter_get_path(adapter);
+               i++;
        }
        dict_append_array(&dict, "Adapters", DBUS_TYPE_OBJECT_PATH, &array, i);
        g_free(array);

Could you send a new patch which doesn't introduce a new variable? Also,
please leave out the signed-off-by from the commit message since it's
not used for userspace bluez patches. Thanks.

Johan

^ permalink raw reply

* Re: [PATCH] Fix history listing queries in phonebook-tracker
From: Johan Hedberg @ 2010-10-22 13:06 UTC (permalink / raw)
  To: Radoslaw Jablonski; +Cc: linux-bluetooth
In-Reply-To: <1287663999-22701-1-git-send-email-ext-jablonski.radoslaw@nokia.com>

Hi Radek,

On Thu, Oct 21, 2010, Radoslaw Jablonski wrote:
> Now we are able to display contact's name in vcard-listing when
> contact has entry in phonebook available.
> ---
>  plugins/phonebook-tracker.c |  140 ++++++++++++++++++++++++++++++++++++-------
>  1 files changed, 118 insertions(+), 22 deletions(-)

Pushed upstream. Thanks.

Johan

^ permalink raw reply

* Re: [PATCH 6/9] mfd: Add support for the ST-Ericsson CG2900 Audio.
From: Alan Cox @ 2010-10-22 12:58 UTC (permalink / raw)
  To: pghatwork
  Cc: par-gunnar.p.hjalmdahl, linus.walleij, linux-bluetooth,
	linux-kernel
In-Reply-To: <AANLkTinHV3qyMRsjAn92uZZp22whESpJDZUfCHcy=Eh9@mail.gmail.com>

On Fri, 22 Oct 2010 12:39:06 +0200
Par-Gunnar Hjalmdahl <par-gunnar.p.hjalmdahl@stericsson.com> wrote:

> This patch adds support for controlling audio paths within
> the ST-Ericsson CG2900 Connectivity Combo controller.
> This patch adds API for the Kernel as well as a char device for
> user space stacks to control the CG2900 audio paths.

I don't see any ALSA hooks for audio or any V4L hooks for the FM radio.
Surely the audio routing wants exposing as a mixer and the audio via
ALSA, likewise FM radio is video4linux so that it'll have the standard
interfaces that are expected by the rest of the Linux OS.

Alan

^ permalink raw reply

* Re: [PATCH 5/9] mfd: Add UART support for the ST-Ericsson CG2900.
From: Alan Cox @ 2010-10-22 12:51 UTC (permalink / raw)
  To: pghatwork
  Cc: par-gunnar.p.hjalmdahl, linus.walleij, linux-bluetooth,
	linux-kernel
In-Reply-To: <AANLkTi=XTUau_GbASJEUBgsZ_FYYmRAX6W43YtRdv2wS@mail.gmail.com>


> + * is_chip_flow_off() - Check if chip has set flow off.
> + * @tty:	Pointer to tty.
> + *
> + * Returns:
> + *   true - chip flows off.
> + *   false - chip flows on.
> + */
> +static bool is_chip_flow_off(struct tty_struct *tty)
> +{
> +	int lines;
> +
> +	lines = tty->ops->tiocmget(tty, uart_info->fd);
> +
> +	if (lines & TIOCM_CTS)
> +		return false;
> +	else
> +		return true;
> +}

What if the device doesn't have a tiocmget ? You must check this. If you
want to call into the ttys own tiocmget froma sleeping context fine, but
see tty_tiocmget and you'll notice you need to check the op is non NULL
somewhere. You could do this when the ldisc is opened and refuse to open
- some ldiscs do that


> +
> +/**
> + * create_work_item() - Create work item and add it to the work queue.
> + * @wq:		work queue struct where the work will be added.
> + * @work_func:	Work function.
> + * @data:	Private data for the work.
> + *
> + * Returns:
> + *   0 if there is no error.
> + *   -EBUSY if not possible to queue work.
> + *   -ENOMEM if allocation fails.
> + */
> +static int create_work_item(struct workqueue_struct *wq, work_func_t work_func,
> +			    void *data)
> +{
> +	struct uart_work_struct *new_work;
> +	int err;
> +
> +	new_work = kmalloc(sizeof(*new_work), GFP_ATOMIC);

So instead of a tiny object embedded in your device you've got a whole
error path to worry about, a printk disguised in a macro and a text
string longer than the struct size ? Surely this should be part of the
device

> +	if (!new_work) {
> +		CG2900_ERR("Failed to alloc memory for uart_work_struct!");

Please use the standard dev_dbg etc functionality - or pr_err etc when
you have no device pointer. The newest kernel tty object has a device
pointer added so you could use that.


> + * set_tty_baud() - Called to set specific baud in TTY.
> + * @tty:	Tty device.
> + * @baud:	Baud to set.
> + *
> + * Returns:
> + *   true - baudrate set with success.
> + *   false - baundrate set failure.
> + */
> +static bool set_tty_baud(struct tty_struct *tty, int baud)
> +{
> +	struct ktermios *old_termios;
> +	bool retval = true;
> +
> +	old_termios = kmalloc(sizeof(*old_termios), GFP_ATOMIC);

termios struct is easily small enough to be fine on the stack

> +	/* Let's mark that CG2900 driver uses c_ispeed and c_ospeed fields. */
> +	tty->termios->c_cflag |= BOTHER;

This shouldn't be needed - the tty_encode_baud_rate logic works out of
BOTHER must be set

> +	tty->termios->c_cflag &= ~BOTHER;

And your termios is now potentially invalid


> +	/* Set IRQ on CTS. */
> +	err = request_irq(pf_data->uart.cts_irq,
> +			  cts_interrupt,
> +			  IRQF_TRIGGER_FALLING,
> +			  UART_NAME,
> +			  NULL);

So we now ave terminal tty/ldisc confusion

> +	if (err) {
> +		CG2900_ERR("Could not request CTS IRQ (%d)", err);
> +		goto error;
> +	}
> +
> +#ifdef CONFIG_PM
> +	enable_irq_wake(pf_data->uart.cts_irq);
> +#endif
> +	return 0;
> +
> +error:
> +	if (pf_data->uart.enable_uart)
> +		(void)pf_data->uart.enable_uart();
> +	return err;

> +}
> +
> +/**
> + * unset_cts_irq() - Disable interrupt on CTS.
> + */
> +static void unset_cts_irq(void)

And no ability to support multiple devices

> +	CG2900_DBG("Clear break");
> +	tty->ops->break_ctl(tty, TTY_BREAK_OFF);

What if the tty doesn't have one ?


> +	if (CHIP_AWAKE == uart_info->sleep_state) {
> +		uart_info->tty->ops->break_ctl(uart_info->tty, TTY_BREAK_ON);

Ditto


> +		set_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
> +		len = tty->ops->write(tty, skb->data, skb->len);

A tty isn't required to have a write function

> +		if ((BAUD_START == uart_info->baud_rate_state) &&
> +		    (is_set_baud_rate_cmd(skb->data))) {
> +			CG2900_INFO("UART set baud rate cmd found.");
> +			SET_BAUD_STATE(BAUD_SENDING);

Do we really need this all in capitals ?



> + * uart_tty_open() - Called when UART line discipline changed to N_HCI.
> + * @tty:	Pointer to associated TTY instance data.
> + *
> + * Returns:
> + *   0 if there is no error.
> + *   Errors from cg2900_register_trans_driver.
> + */
> +static int uart_tty_open(struct tty_struct *tty)
> +{
> +	int err;
> +
> +	CG2900_INFO("uart_tty_open");
> +
> +	/* Set the structure pointers and set the UART receive room */
> +	uart_info->tty = tty;

You must respect the kref handling in the kernel tty code. Take
references by all means but do it properly.


> +static void uart_tty_wakeup(struct tty_struct *tty)
> +{
> +	int err;
> +
> +	CG2900_INFO("uart_tty_wakeup");
> +
> +	/*
> +	 * Clear the TTY_DO_WRITE_WAKEUP bit that is set in
> +	 * work_do_transmit().
> +	 */
> +	clear_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
> +
> +	if (tty != uart_info->tty)
> +		return;

How can this occur - and why check it after you've changed tty->flags ???


> +/**
> + * uart_tty_receive() - Called by TTY low level driver when receive
> data is available.
> + * @tty:	Pointer to TTY instance data
> + * @data:	Pointer to received data
> + * @flags:	Pointer to flags for data
> + * @count:	Count of received data in bytes
> + */
> +static void uart_tty_receive(struct tty_struct *tty, const u8 *data,
> +			     char *flags, int count)
> +{
> +	CG2900_INFO("uart_tty_receive");
> +
> +	if (tty != uart_info->tty)
> +		return;

Again this is nonsense


> +/*
> + * We don't provide read/write/poll interface for user space.
> + */
> +static ssize_t uart_tty_read(struct tty_struct *tty, struct file *file,
> +			     unsigned char __user *buf, size_t nr)
> +{
> +	CG2900_INFO("uart_tty_read");
> +	return 0;
> +}

-EINVAL then

> +
> +static ssize_t uart_tty_write(struct tty_struct *tty, struct file *file,
> +			      const unsigned char *data, size_t count)
> +{
> +	CG2900_INFO("uart_tty_write");
> +	return count;

This is wrong - you can't jusr vanish the data
> +}



This needs some restructuring I think

A line discipline should contain no hardware awareness, that goes in the
actual uart hardware driver. So we shouldn't have magic irq code in this
part of things.

You also need to sort out the tty kref handling in open/close and the
fact you've got magic hardcoded single instance stuff buried i nit.

Finally tty ldiscs don't go buried in the mfd directory, or they'll get
missed during chanegs/updates. The ldisc probably belongs in bluetooth a
and the hardware support in the mfd directory.


So - NAK this for the moment, it needs to be split cleanly into ldisc
(thing which speaks the protocol and eg sees "speed change required" and
acts on it) and device (thing which knows about the hardware).


^ permalink raw reply

* [PATCH 9/9] DocBook: Add ST-Ericsson CG2900 docs
From: Par-Gunnar Hjalmdahl @ 2010-10-22 10:41 UTC (permalink / raw)
  To: linus.walleij, linux-bluetooth, linux-kernel

This patch adds documentation for the ST-Ericsson CG2900
Connectivity controller.

Signed-off-by: Par-Gunnar Hjalmdahl <par-gunnar.p.hjalmdahl@stericsson.com>
---
 Documentation/DocBook/Makefile    |    2 +-
 Documentation/DocBook/cg2900.tmpl | 1332 +++++++++++++++++++++++++++++++++++++
 2 files changed, 1333 insertions(+), 1 deletions(-)
 create mode 100644 Documentation/DocBook/cg2900.tmpl

diff --git a/Documentation/DocBook/Makefile b/Documentation/DocBook/Makefile
index 8b6e00a..e8319ec 100644
--- a/Documentation/DocBook/Makefile
+++ b/Documentation/DocBook/Makefile
@@ -14,7 +14,7 @@ DOCBOOKS := z8530book.xml mcabook.xml device-drivers.xml \
 	    genericirq.xml s390-drivers.xml uio-howto.xml scsi.xml \
 	    80211.xml debugobjects.xml sh.xml regulator.xml \
 	    alsa-driver-api.xml writing-an-alsa-driver.xml \
-	    tracepoint.xml media.xml drm.xml
+	    tracepoint.xml media.xml drm.xml cg2900.xml

 ###
 # The build process is as follows (targets):
diff --git a/Documentation/DocBook/cg2900.tmpl
b/Documentation/DocBook/cg2900.tmpl
new file mode 100644
index 0000000..e7eef1b
--- /dev/null
+++ b/Documentation/DocBook/cg2900.tmpl
@@ -0,0 +1,1332 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE book PUBLIC "-//OASIS//DTD DocBook XML V4.1.2//EN"
+	"http://www.oasis-open.org/docbook/xml/4.1.2/docbookx.dtd" []>
+
+<book id="STE-Connectivity-template">
+ <bookinfo>
+  <title>CG2900 Driver</title>
+
+  <authorgroup>
+   <author>
+    <firstname>Henrik</firstname>
+    <surname>Possung</surname>
+    <affiliation>
+     <address>
+      <email>henrik.possung@stericsson.com</email>
+     </address>
+    </affiliation>
+   </author>
+   <author>
+    <firstname>Par-Gunnar</firstname>
+    <surname>Hjalmdahl</surname>
+    <affiliation>
+     <address>
+      <email>par-gunnar.p.hjalmdahl@stericsson.com</email>
+     </address>
+    </affiliation>
+   </author>
+  </authorgroup>
+
+  <copyright>
+   <year>2010</year>
+   <holder>ST-Ericsson SA</holder>
+  </copyright>
+
+  <subjectset>
+    <subject>
+      <subjectterm>Connectivity</subjectterm>
+    </subject>
+  </subjectset>
+
+  <legalnotice>
+   <!-- Do NOT remove the legal notice below -->
+
+  <para>
+     This documentation is free software; you can redistribute
+     it and/or modify it under the terms of the GNU General Public
+     License as published by the Free Software Foundation; either
+     version 2 of the License, or (at your option) any later
+     version.
+   </para>
+
+   <para>
+     This program is distributed in the hope that it will be
+     useful, but WITHOUT ANY WARRANTY; without even the implied
+     warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+     See the GNU General Public License for more details.
+   </para>
+
+   <para>
+     You should have received a copy of the GNU General Public
+     License along with this program; if not, write to the Free
+     Software Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+     MA 02111-1307 USA
+   </para>
+
+   <para>
+     For more details see the file COPYING in the source
+     distribution of Linux.
+   </para>
+  </legalnotice>
+ </bookinfo>
+
+ <toc></toc>
+
+ <chapter id="intro">
+  <title>Introduction</title>
+  <!-- Do NOT change the chapter id or title! -->
+  <para>
+    This documentation describes the functions provided by the
ST-Ericsson Connectivity Driver for enabling
+    ST-Ericsson Connectivity Combo Controller Hardware.
+
+  </para>
+ </chapter>
+
+ <chapter id="gettingstarted">
+  <title>Getting Started</title>
+  <!-- Do NOT change the chapter id or title! -->
+  <para>
+     There are no special compilation flags needed to build the STE
connectivity driver.
+  </para>
+  <para>
+    There must be firmware and settings files that match the used
chip version inside the firmware folder.
+    The files:
+    <itemizedlist>
+      <listitem><para>cg2900_patch_info.fw.org</para></listitem>
+      <listitem><para>cg2900_settings_info.fw.org</para></listitem>
+    </itemizedlist>
+    handle the mapping between chip version and correct firmware file
(patch resp static settings file).
+    The necessary patch and settings files should be placed with the
extension <constant>.fw.org.</constant>.
+    Note that there is a limitation in the Kernel firmware system
regarding name length of a file.
+  </para>
+
+  <!-- TODO: If the driver needs preparations to be used
+        (special compilation flags, files in the file system,
+        knowledge about a specific domain etc), specify it here.
+        Remove this chapter completely if there is nothing
+        to mention and there is no tutorial needed.
+        Do NOT change the chapter id or title! -->
+  <!-- TODO: This guideline for this chapter may be extended
+        during the user-guide guidelines drop. -->
+
+  <section id="basic-tutorial">
+    <title>Basic Tutorial</title>
+    <para>
+      To enable the ST-Ericsson connectivity driver using KConfig go
to <constant>Device Drivers -> Multifunction Device Drivers</constant>
+      and enable the STE Connectivity Driver. If BlueZ shall be used
as Bluetooth stack also enable the STE HCI Connectivity driver.
+      Depending on choice the driver will either be included as LKM
or built into the Kernel.
+      If building as LKM, 2 files will be generated:
+      <itemizedlist>
+        <listitem><para>cg2900.ko which contains the main
driver</para></listitem>
+        <listitem><para>btcg2900.ko which contains the registration
and mapping towards the BlueZ Bluetooth stack</para></listitem>
+      </itemizedlist>
+
+      <!-- TODO: Provide a basic tutorial, outlining how
+            to test the presence of the driver,
+            for example how to configure, compile and run the
+            example(s).
+            Several sections with different tutorials,
+            all located within the Getting Started
+            chapter may be provided. -->
+    </para>
+
+    <para>
+     <!-- TODO: This guideline for this section may be extended
+           during the user-guide guidelines drop. -->
+    </para>
+  </section>
+
+ </chapter>
+
+ <chapter id="concepts">
+  <title>Concepts</title>
+  <!-- Do NOT change the chapter id or title! -->
+  <para>
+     The ST-Ericsson Connectivity driver works as a multiplexer
between different users, such as a Bluetooth stack and a FM driver,
+     and the connectivity chip. The driver supports multiple physical
transports, currently SPI and UART.
+     Apart from just transporting data between stacks and the chip,
the ST-Ericsson Connectivity driver also deals with power handling,
+     powering up and down the chip and also downloading necessary
patches and settings for the chip to start up properly.
+     <!-- TODO: A brief introduction about the concepts
+           which are introduced by the driver.
+           Remove this chapter completely if there are no
+           special concepts introduced by this driver.
+           Do NOT change the chapter id or title! -->
+     <!-- TODO: This guideline for this chapter may be extended
+           during the user-guide guidelines drop. -->
+  </para>
+ </chapter>
+
+ <chapter id="tasks">
+   <title>Tasks</title>
+   <!-- Do NOT change the chapter id or title! -->
+
+   <para>
+     <variablelist>
+       <varlistentry>
+         <term>Opening a channel</term>
+         <listitem>
+         <para>
+           In order to be able to send and receive data on an H:4
channel, the user (i.e. respective stack) must open the channel.
+           Opening a channel will make it possible to send data to
and receive data from the connectivity controller.
+           If the controller were earlier powered down, opening a
channel will also cause the controller to be powered up.
+           When chip is powered up, patches and settings for the ARM
subsystem will be downloaded as well.
+           Other IPs within the controller must however download each
respective patches and settings.
+           If chip was already powered up when opening the channel no
patch will be automatically downloaded.
+
+           <variablelist>
+             <varlistentry>
+               <term>Opening a channel from Kernel space</term>
+               <listitem>
+               <para>
+                 When a stack is placed in Kernel space, it shall
open a channel by calling the API function
<function>cg2900_register_user</function>.
+                 This function will search for the supplied channel
by using name look-up and open the channel.
+                 The function will return with a device reference
that shall be used when calling the other CG2900 API functions.
+               </para>
+               </listitem>
+             </varlistentry>
+           </variablelist>
+
+           <variablelist>
+             <varlistentry>
+               <term>Opening a channel from User space</term>
+               <listitem>
+               <para>
+                 When a stack is placed in User space, it shall open
a channel by calling the syscall function <function>open</function> on
the corresponding file.
+                 The files are located in folder
<filename>/dev/</filename> and are named
<filename>cg2900_gnss</filename> and similar. Each file
+                 corresponds to one H:4 channel.
+                 This function will open the channel.
+               </para>
+               </listitem>
+             </varlistentry>
+           </variablelist>
+
+         </para>
+         </listitem>
+       </varlistentry>
+
+       <varlistentry>
+         <term>Closing a channel</term>
+         <listitem>
+         <para>
+           When a user, i.e. a stack has no need for a functionality,
it should close the corresponding H:4 channel.
+           This is usually done when a user disables a certain
feature, for example Bluetooth. The reason why the channels
+           need to be closed is that the ST-E connectivity driver
will free the resources and also shutdown the controller if there are
+           no more active users of the chip. This will lower the
power consumption thereby increasing battery life.
+
+           <variablelist>
+             <varlistentry>
+               <term>Closing a channel from Kernel space</term>
+               <listitem>
+               <para>
+                 When a stack is placed in Kernel space, it shall
close a channel by calling the API function
+                 <function>cg2900_deregister_user</function>.
+                 This function will close the channel and also free
the allocated device that was allocated when registering.
+               </para>
+               </listitem>
+             </varlistentry>
+           </variablelist>
+
+           <variablelist>
+             <varlistentry>
+               <term>Closing a channel from User space</term>
+               <listitem>
+               <para>
+                 When a stack is placed in User space, it shall close
a channel by calling the syscall function
+                 <function>close</function> on the corresponding file.
+               </para>
+               </listitem>
+             </varlistentry>
+           </variablelist>
+
+         </para>
+         </listitem>
+       </varlistentry>
+
+       <varlistentry>
+         <term>Writing to a channel</term>
+         <listitem>
+         <para>
+           When a stack (Bluetooth, FM, or GNSS) wants to send a
packet it shall perform a write operation.
+           The packet shall not contain the H:4 header since this is
added by the ST-E connectivity driver.
+           All other data in the packet shall however exist in the
packet in the format correct for that HCI channel.
+           The ST-E connectivity users need to perform flow control
over channels so any ticket handling
+           or similar must be handled by respective stack.
+
+           <variablelist>
+             <varlistentry>
+               <term>Writing to a channel from Kernel space</term>
+               <listitem>
+               <para>
+                 When a stack is placed in Kernel space, it shall
start with allocating a packet of the correct size using
+                 <function>cg2900_alloc_skb</function>. This function
will return an sk_buff (Socket buffer) structure that
+                 has necessary space reserved for ST-E driver operation.
+                 The stack shall then copy the data, preferrably
using <function>skb_put</function>, and then call
+                 <function>cg2900_write</function> to perform the
write operation. When the function returns, the buffer has
+                 been transferred and there is no need for the
calling function to free the buffer. If the operation fails, i.e.
+                 an error code is returned, the caller must however
free the buffer.
+               </para>
+               </listitem>
+             </varlistentry>
+           </variablelist>
+
+           <variablelist>
+             <varlistentry>
+               <term>Writing to a channel from User space</term>
+               <listitem>
+               <para>
+                 When a stack is placed in User space, it shall call
the <function>write</function> function on
+                 the corresponding file to perform a transmit
operation. After function returns the data has been
+                 copied and is considered sent.
+                 The caller does not need to preserve the data.
+               </para>
+               </listitem>
+             </varlistentry>
+           </variablelist>
+
+           <variablelist>
+             <varlistentry>
+               <term>Writing to FM_Radio and FM_Audio channel</term>
+               <listitem>
+               <para>
+		CG2900 driver only supports FM legacy commands. The reason is that
the FM_Radio and FM_Audio uses the same H4 channel aginst the chip,
+		in order to multiplex the FM user commands the data pakets are
parsed by the CG2900 driver.
+               </para>
+               </listitem>
+             </varlistentry>
+           </variablelist>
+
+         </para>
+         </listitem>
+       </varlistentry>
+
+       <varlistentry>
+         <term>Reading from a channel</term>
+         <listitem>
+         <para>
+           When a stack (Bluetooth, FM, or GNSS) wants to receive a
packet it shall perform a receive operation.
+           The packet returned does not contain the H:4 header since
this is removed by the ST-E connectivity driver.
+           All other data in the packet in the packet is in the
format correct for that HCI channel.
+           The ST-E connectivity driver does not perform any flow
control over the H:4 channel so any ticket handling
+           or similar must be handled by respective stack.
+
+           <variablelist>
+             <varlistentry>
+               <term>Reading from a channel from Kernel space</term>
+               <listitem>
+               <para>
+                 When a stack is placed in Kernel space, it has to
supply a callback function for the receive functionality when calling
+                 <function>cg2900_register_user</function>. This
callback function will be called when the ST-E connectivity driver has
+                 received a packet. The packet received will always
be a complete HCI packet, i.e. no fragmention on HCI layer.
+                 When the packet has been received it is the
responsability of the receiver to see to that the packet is freed
using
+                 <function>kfree_skb</function> when it is no more needed.
+               </para>
+               </listitem>
+             </varlistentry>
+           </variablelist>
+
+           <variablelist>
+             <varlistentry>
+               <term>Reading from a channel from User space</term>
+               <listitem>
+               <para>
+                 When a stack is placed in User space, it shall call
the <function>read</function> function on
+                 the corresponding file to perform a receive
operation. This function will read as many bytes as there are present
+                 up to the size of the supplied buffer. If no data is
available the function will hang until data becomes available, reset
+                 occurs, or the channel is closed.
+                 For smooth operation it is recommended to use the
<function>poll</function> functionality on the file, preferrably
+                 from a dedicated thread. This way one thread can
monitor both read and reset operations in one common thread while
transmit
+                 operations may continue unblocked in a separate thread.
+               </para>
+               </listitem>
+             </varlistentry>
+           </variablelist>
+
+         </para>
+         </listitem>
+       </varlistentry>
+
+       <varlistentry>
+         <term>Reset handling</term>
+         <listitem>
+         <para>
+           The stacks shall always try to avoid performing Reset
operations. The Reset will result in a hardware reset of the
controller
+           and will therefore cause all existing links and settings
to be lost. All stacks using the controller must also be informed
+           about the reset and handle it in a proper way.
+           The reset operation should only be used when there is no
other option to get the controller into a working state, for example
+           if the controller has stopped answering to commands.
+           After the hardware reset, the ST-E connectivity driver
will automatically perform deregister the channel so it has to be
reopened again.
+
+           <variablelist>
+             <varlistentry>
+               <term>Reset handling from Kernel space</term>
+               <listitem>
+               <para>
+                 When a stack is placed in Kernel space, it initiates
a Reset operation by calling <function>cg2900_reset</function>.
+                 This will trigger a hardware reset of the
controller. When the hardware reset is finished all registered users
will be called
+                 through respective reset callback. When the callback
function is finished the registered device will be removed and when
all
+                 registered users have been informed and removed, the
chip is shutdown. This is similar to a deregistration of all
registered
+                 channels. The stack will then have to reregister to
the ST-E connectivity driver in order to use the channel once again.
+               </para>
+               </listitem>
+             </varlistentry>
+           </variablelist>
+
+           <variablelist>
+             <varlistentry>
+               <term>Reset handling from User space</term>
+               <listitem>
+               <para>
+                 When a stack is placed in User space, it shall call
the <function>ioctl</function> function on
+                 the corresponding file to perform a reset operation.
The command parameter <constant>CG2900_CHAR_DEV_IOCTL_RESET</constant>
+                 shall be used when calling <function>ioctl</function>.
+                 When the <function>ioctl</function> returns, the
stack shall close the channel and then re-open it again. This must be
done so
+                 the channel is registered correctly in Kernel space.
+                 For smooth operation it is recommended to use the
<function>poll</function> functionality on the file, preferrably
+                 from a dedicated thread. This way one thread can
monitor both read and reset operations in one common thread while
transmit
+                 operations may continue unblocked in a separate thread.
+               </para>
+               </listitem>
+             </varlistentry>
+           </variablelist>
+
+         </para>
+         </listitem>
+       </varlistentry>
+
+       <varlistentry>
+         <term>Example code Kernel space</term>
+         <listitem>
+         <para>
+           This example will register to the FM channel, write a
packet, read a packet and then deregister.
+
+           <programlisting>
+             struct cg2900_device *my_dev;
+             bool event_received;
+
+             void read_cb(struct cg2900_device *dev, struct sk_buff *skb)
+             {
+               event_received = true;
+               kfree_skb(skb);
+             }
+
+             void reset_cb(struct cg2900_device *dev)
+             {
+               /* Handle reset. Device will be automatically freed by
the ST-E driver */
+               my_dev = NULL;
+             }
+
+             static struct cg2900_callbacks my_cb = {
+               .read_cb = read_cb,
+               .reset_cb = reset_cb
+             };
+
+             void example_open(void)
+             {
+               my_dev = cg2900_register_user(CG2900_FM_RADIO, &amp;my_cb);
+               if (!my_dev) {
+                 printk("Error! Couldn't register!\n");
+               }
+             }
+
+             void example_close(void)
+             {
+               cg2900_deregister_user(my_dev);
+               my_dev = NULL;
+             }
+
+             void example_write_and_read(uint8_t *data, int len)
+             {
+               int err;
+               struct sk_buff *skb = cg2900_alloc_skb(len, GFP_KERNEL);
+
+               if (skb) {
+                 memcpy(skb_put(skb, len), data, len);
+                 err = cg2900_write(my_dev, skb);
+                 if (!err) {
+                   event_received = false;
+
+                   while (!event_received) {
+                     /* Wait for ack event. Received in read_cb() above */
+                     schedule_timeout_interruptible(jiffies + 50);
+                   }
+                 } else {
+                   printk("Couldn't write to controller (%d)\n", err);
+                   kfree_skb(skb);
+                 }
+               }
+             }
+           </programlisting>
+         </para>
+         </listitem>
+       </varlistentry>
+
+       <varlistentry>
+         <term>Example code User space</term>
+         <listitem>
+         <para>
+           This example will open the GNSS channel, write a packet,
read a packet and then close the channel.
+           In this example all functions are performed in the same thread.
+           It is however adviced to perform <function>read</function>
and <function>ioctl</function> read through a separate thread,
+           preferrably using <function>poll</function>.
+
+           <programlisting>
+             struct my_info_t {
+               int fd;
+             };
+
+             static struct my_info_t my_info;
+
+             /* This is a fake command and has nothing to do with
real GNSS commands.
+              * Note that the command does NOT contain the H:4 header.
+              * The header is added by the ST-E Connectivity driver.
+              */
+             static const uint8_t tx_cmd[] = {0x12, 0x34, 0x56};
+
+             int main(int argc, char **argv)
+             {
+               uint8_t rx_buffer[100];
+               int rx_bytes = 0;
+               int err;
+
+               my_info.fd = open("/dev/cg2900_gnss", O_RDWR);
+               if (my_info.fd &lt; 0) {
+                 printf("Error on open file: %d (%s)\n", errno,
strerror(errno));
+                 return errno;
+               }
+               if (0 &gt; write(my_info.fd, tx_cmd, sizeof(tx_cmd))) {
+                 printf("Error on write file: %d (%s)\n", errno,
strerror(errno));
+                 return errno;
+               }
+               /* Read will sleep until there is data available */
+               rx_bytes = read(my_info.fd, rx_buffer, 100);
+               if (rx_bytes &gt;= 0) {
+                 printf("Received %d bytes\n", rx_bytes);
+               } else {
+                 printf("Error on read file: %d (%s)\n", errno,
strerror(errno));
+                 return errno;
+               }
+               err = close(my_info.fd);
+               if (err) {
+                 printf("Error on close file: %d (%s)\n", errno,
strerror(errno));
+                 return errno;
+               }
+               return 0;
+             }
+           </programlisting>
+         </para>
+         </listitem>
+       </varlistentry>
+     </variablelist>
+
+     <!-- TODO: Task descriptions are step by step instructions
+           for performing specific actions and tasks.
+           Each task is typically one scenario.
+           Each task is described in a separate (section).
+           (section) tags can be nested, which is
+           especially recommended if
+           the task consists of several scenarios.
+           Remove this chapter completely if there are no
+           tasks to mention and there is no tutorial needed.
+           Do NOT change the chapter id or title! -->
+     <!-- TODO: This guideline for this chapter may be extended
+           during the user-guide guidelines drop. -->
+   </para>
+ </chapter>
+
+ <chapter id="driver-configuration">
+   <title>Driver Configuration and Interaction</title>
+   <!-- Do NOT change the chapter id or title! -->
+   <para>
+     For debug purposes the define CG2900_DEBUG_LEVEL in the file
cg2900_debug.h can be changed to set how much debug printouts
+     that shall be generated.
+     <itemizedlist>
+       <listitem><para>0 - No debug</para></listitem>
+       <listitem><para>1 - Error printouts</para></listitem>
+       <listitem><para>10 - Info printouts such as start of each
function</para></listitem>
+       <listitem><para>20 - Debug printouts such as descriptions of
operations</para></listitem>
+       <listitem><para>25 - Data printouts without content</para></listitem>
+       <listitem><para>30 - Data printouts with content</para></listitem>
+     </itemizedlist>
+     <!-- TODO: Use this paragraph as an introduction to driver
+           configuration and interaction. Describe the big picture. -->
+     <!-- TODO: This chapter contains driver specific way to perform
+           configuration and interaction. The chapter includes a
+           number of sections. They should not be removed and if
+           the driver does not have the specific support for
+           configuration or interaction should the text "not
+           applicable" be inserted. Do NOT change the chapter id
+           or title! -->
+     <!-- TODO: This guideline for this chapter may be extended
+           during the user-guide guidelines drop. -->
+   </para>
+
+   <section id="driver-implemented-operations">
+     <title>Implemented operations in driver</title>
+     <para>
+       <!-- TODO: Describe the actual usage of the driver. Specify the actual
+            implemented operations in struct
<structname>file_operations</structname>
+            and any other set of operations. Create a table with two columns
+            (see example in intro chapter how to create a table).
+            Column one list all operations supported (read,
+            write, open, close, ioctl etc) and column two a description of the
+            semantics of the operations in the specific context of the device
+            driver from the users perspective. Document the operations in a way
+            that a user of the driver can be helped. -->
+     </para>
+     <para>
+       <table>
+         <title> Supported device driver operations when using
character device </title>
+         <tgroup cols="2"><tbody>
+           <row><entry> open </entry> <entry> Opening a character
device will register the caller to that HCI channel.</entry> </row>
+           <row><entry> release </entry> <entry> Releasing a
character device will deregister the caller from that HCI
channel</entry> </row>
+           <row><entry> poll </entry> <entry> Polling a character
device will check if there is data to read on that HCI channel</entry>
</row>
+           <row><entry> read </entry> <entry> Reading from a
character device reads from that HCI channel</entry> </row>
+           <row><entry> write </entry> <entry> Writing to a character
device writes to that HCI channel</entry> </row>
+           <row><entry> unlocked_ioctl </entry> <entry> Performing IO
control on a character device will perform special operations such as
reset on that HCI channel</entry> </row>
+         </tbody></tgroup>
+       </table>
+     </para>
+   </section>
+
+   <section id="driver-loading">
+     <title>Driver loading parameters</title>
+     <para>
+       <!-- TODO: Describe parameters that can be specified at kernel
+            driver loading with insmod or modprobe. If the driver
+            has no parameters to be specified at load time, replace this
+            text with "Not Applicable". -->
+     </para>
+     <variablelist>
+       <varlistentry>
+         <term>uart_default_baud</term>
+         <listitem>
+         <para>
+           <variablelist>
+             <varlistentry>
+               <term>Parameter type</term>
+               <listitem><synopsis><type>int</type></synopsis></listitem>
+             </varlistentry>
+             <varlistentry>
+               <term>Default value</term>
+               <listitem><para>115200</para></listitem>
+             </varlistentry>
+             <varlistentry>
+               <term>Runtime readable/modifiable</term>
+               <listitem><para>Readable</para></listitem>
+             </varlistentry>
+             <varlistentry>
+               <term>Description</term>
+               <listitem>
+               <para>
+                 The parameter uart_default_baud in cg2900_uart.c
defines the baud rate used after a chip has just been powered up.
+                 It shall be set to the default baud rate of the controller.
+                 For ST-Ericsson controllers STLC2690 and CG2900 this
value shall be 115200.
+               </para>
+               </listitem>
+             </varlistentry>
+           </variablelist>
+         </para>
+         </listitem>
+       </varlistentry>
+
+       <varlistentry>
+         <term>uart_high_baud</term>
+         <listitem>
+         <para>
+           <variablelist>
+             <varlistentry>
+               <term>Parameter type</term>
+               <listitem><synopsis><type>int</type></synopsis></listitem>
+             </varlistentry>
+             <varlistentry>
+               <term>Default value</term>
+               <listitem><para>3000000</para></listitem>
+             </varlistentry>
+             <varlistentry>
+               <term>Runtime readable/modifiable</term>
+               <listitem><para>Modifiable</para></listitem>
+             </varlistentry>
+             <varlistentry>
+               <term>Description</term>
+               <listitem>
+               <para>
+                 The parameter uart_high_baud in cg2900_uart.c
defines the baud rate to use for normal data transfer.
+                 This should normally be the highest allowed by the
system with regards to flow control, clocks, etc.
+                 For ST-Ericsson controllers STLC2690 and CG2900 the
following values are supported:
+                 <itemizedlist>
+                   <listitem><para>57600</para></listitem>
+                   <listitem><para>115200</para></listitem>
+                   <listitem><para>230400</para></listitem>
+                   <listitem><para>460800</para></listitem>
+                   <listitem><para>921600</para></listitem>
+                   <listitem><para>2000000</para></listitem>
+                   <listitem><para>3000000</para></listitem>
+                   <listitem><para>4000000</para></listitem>
+                 </itemizedlist>
+               </para>
+               </listitem>
+             </varlistentry>
+           </variablelist>
+         </para>
+         </listitem>
+       </varlistentry>
+
+       <varlistentry>
+         <term>bd_address</term>
+         <listitem>
+         <para>
+           <variablelist>
+             <varlistentry>
+               <term>Parameter type</term>
+               <listitem><synopsis><type>array (Entered as comma
separated value)</type></synopsis></listitem>
+             </varlistentry>
+             <varlistentry>
+               <term>Default value</term>
+               <listitem><para>0x00 0x80 0xDE 0xAD 0xBE 0xEF</para></listitem>
+             </varlistentry>
+             <varlistentry>
+               <term>Runtime readable/modifiable</term>
+               <listitem><para>Modifiable</para></listitem>
+             </varlistentry>
+             <varlistentry>
+               <term>Description</term>
+               <listitem>
+               <para>
+                 The parameter bd_address in cg2900_core.c defines
the Bluetooth device address to use for the current device.
+                 The value is an array of 6 bytes and shall be
entered as a comma separated value.
+               </para>
+               </listitem>
+             </varlistentry>
+           </variablelist>
+         </para>
+         </listitem>
+       </varlistentry>
+
+       <varlistentry>
+         <term>debug_level</term>
+         <listitem>
+         <para>
+           <variablelist>
+             <varlistentry>
+               <term>Parameter type</term>
+               <listitem><synopsis><type>int</type></synopsis></listitem>
+             </varlistentry>
+             <varlistentry>
+               <term>Default value</term>
+               <listitem><para>1</para></listitem>
+             </varlistentry>
+             <varlistentry>
+               <term>Runtime readable/modifiable</term>
+               <listitem><para>Modifiable</para></listitem>
+             </varlistentry>
+             <varlistentry>
+               <term>Description</term>
+               <listitem>
+               <para>
+                 The parameter debug_level in cg2900_core.c defines
the debug level that is currently used.
+                 The higher the debug level the more print-outs are
received in the terminal window.
+                 The following values are supported:
+                 <itemizedlist>
+                   <listitem><para>0  = No debug</para></listitem>
+                   <listitem><para>1  = Error prints</para></listitem>
+                   <listitem><para>10 = General info, e.g. function
entries</para></listitem>
+                   <listitem><para>20 = Debug info, e.g. steps in a
functionality</para></listitem>
+                   <listitem><para>25 = Data info, i.e. prints when
data is transferred</para></listitem>
+                   <listitem><para>30 = Data content, i.e. contents
of the transferred data</para></listitem>
+                 </itemizedlist>
+               </para>
+               </listitem>
+             </varlistentry>
+           </variablelist>
+         </para>
+         </listitem>
+       </varlistentry>
+       <varlistentry>
+         <term>sleep_timeout_ms</term>
+         <listitem>
+         <para>
+           <variablelist>
+             <varlistentry>
+               <term>Parameter type</term>
+               <listitem><synopsis><type>int</type></synopsis></listitem>
+             </varlistentry>
+             <varlistentry>
+               <term>Default value</term>
+               <listitem><para>0</para></listitem>
+             </varlistentry>
+             <varlistentry>
+               <term>Runtime readable/modifiable</term>
+               <listitem><para>Modifiable</para></listitem>
+             </varlistentry>
+             <varlistentry>
+               <term>Description</term>
+               <listitem>
+               <para>
+                 The parameter sleep_timeout_ms in cg2900_core.c
defines the sleep timeout for data transmission in milliseconds.
+               </para>
+               </listitem>
+             </varlistentry>
+           </variablelist>
+         </para>
+         </listitem>
+       </varlistentry>
+      </variablelist>
+      <para>
+       <!-- TODO: This guideline for this section may be extended
+            during the user-guide guidelines drop. -->
+     </para>
+   </section>
+
+   <section id="driver-ioctl">
+     <title>Driver IO Control</title>
+     <para>
+       <!-- TODO: Describe driver parameters that can be modified
+            in runtime. Make a list of all device-dependent request code with
+            description of arguments, meaning etc.  If the driver has
no IO control
+            interface, replace this text with "Not Applicable". -->
+     </para>
+     <variablelist>
+       <varlistentry>
+         <term><constant>CG2900_CHAR_DEV_IOCTL_RESET</constant></term>
+         <listitem>
+         <variablelist>
+           <varlistentry>
+             <term>Direction</term>
+             <listitem><para>Set</para></listitem>
+           </varlistentry>
+           <varlistentry>
+             <term>Parameter</term>
+             <listitem><synopsis><type>void</type></synopsis></listitem>
+           </varlistentry>
+           <varlistentry>
+             <term>Description</term>
+             <listitem>
+             <para>
+               The <constant>CG2900_CHAR_DEV_IOCTL_RESET</constant>
IOCTL starts a reset
+               of the connectivity chip. This will affect the current
open channel and
+               all other open channels as well.
+             </para><para>
+               IOCTL value created using <constant>_IOW('U', 210,
int)</constant>.
+             </para><para>
+               Returned values are:
+               <itemizedlist>
+                 <listitem><para>If reset is performed without errors
the IOCTL function will return 0.</para></listitem>
+                 <listitem><para>A negative value will indicate
error.</para></listitem>
+               </itemizedlist>
+             </para>
+             </listitem>
+           </varlistentry>
+         </variablelist>
+         </listitem>
+       </varlistentry>
+
+       <varlistentry>
+         <term><constant>CG2900_CHAR_DEV_IOCTL_CHECK4RESET</constant></term>
+         <listitem>
+         <variablelist>
+           <varlistentry>
+             <term>Direction</term>
+             <listitem><para>Query</para></listitem>
+           </varlistentry>
+           <varlistentry>
+             <term>Parameter</term>
+             <listitem><synopsis><type>void</type></synopsis></listitem>
+           </varlistentry>
+           <varlistentry>
+             <term>Description</term>
+             <listitem>
+               <para>
+                 The
<constant>CG2900_CHAR_DEV_IOCTL_CHECK4RESET</constant> IOCTL checks if
a reset
+                 has been performed on a device.
+               </para><para>
+                 IOCTL value created using <constant>_IOR('U', 212,
int)</constant>.
+               </para><para>
+                 Returned values are:
+                 <itemizedlist>
+                   <listitem><para>If device is still open the IOCTL
function will return 0.</para></listitem>
+                   <listitem><para>If reset has occurred the IOCTL
function will return 1.</para></listitem>
+                   <listitem><para>If device has been closed the
IOCTL function will return 2.</para></listitem>
+                   <listitem><para>A negative value will indicate
error.</para></listitem>
+                 </itemizedlist>
+               </para>
+             </listitem>
+           </varlistentry>
+         </variablelist>
+         </listitem>
+       </varlistentry>
+
+       <varlistentry>
+         <term><constant>CG2900_CHAR_DEV_IOCTL_GET_REVISION</constant></term>
+         <listitem>
+         <variablelist>
+           <varlistentry>
+             <term>Direction</term>
+             <listitem><para>Query</para></listitem>
+           </varlistentry>
+           <varlistentry>
+             <term>Parameter</term>
+             <listitem><synopsis><type>void</type></synopsis></listitem>
+           </varlistentry>
+           <varlistentry>
+             <term>Description</term>
+             <listitem>
+               <para>
+                 The
<constant>CG2900_CHAR_DEV_IOCTL_GET_REVISION</constant> IOCTL returns
the revision value
+                 of the local connectivity controller if such
information is available.
+               </para><para>
+                 IOCTL value created using <constant>_IOR('U', 213,
int)</constant>.
+               </para><para>
+                 Returned values are according to information that
may be retrieved from chip manufacturer.
+                 It is however possible to get indications of the
value by looking in the file
+                 <constant>firmware/cg2900_patch_info.fw.org</constant>.
+               </para>
+             </listitem>
+           </varlistentry>
+         </variablelist>
+         </listitem>
+       </varlistentry>
+
+       <varlistentry>
+         <term><constant>CG2900_CHAR_DEV_IOCTL_GET_SUB_VER</constant></term>
+         <listitem>
+         <variablelist>
+           <varlistentry>
+             <term>Direction</term>
+             <listitem><para>Query</para></listitem>
+           </varlistentry>
+           <varlistentry>
+             <term>Parameter</term>
+             <listitem><synopsis><type>void</type></synopsis></listitem>
+           </varlistentry>
+           <varlistentry>
+             <term>Description</term>
+             <listitem>
+               <para>
+                 The
<constant>CG2900_CHAR_DEV_IOCTL_GET_SUB_VER</constant> IOCTL returns
the sub-version value
+                 of the local connectivity controller if such
information is available.
+               </para><para>
+                 IOCTL value created using <constant>_IOR('U', 214,
int)</constant>.
+               </para><para>
+                 Returned values are according to information that
may be retrieved from chip manufacturer.
+                 It is however possible to get indications of the
value by looking in the file
+                 <constant>firmware/cg2900_patch_info.fw.org</constant>.
+               </para>
+             </listitem>
+           </varlistentry>
+         </variablelist>
+         </listitem>
+       </varlistentry>
+
+     </variablelist>
+   </section>
+
+   <section id="driver-sysfs">
+     <title>Driver Interaction with Sysfs</title>
+     <para>
+       <!-- TODO: Describe data available for read and write on the drivers
+            Sysfs entry.  Specify where the entry for the device is located in
+            Sysfs such as <filename>/sys/devices/*</filename>,
<filename>/sys/devices/*</filename>
+            , etc.
+            Specify the data types for the attributes. Specify if the
+            attributes are read-only or write-only. If the driver has no Sysfs
+            interface, replace this text with "Not Applicable". -->
+       Not Applicable
+     </para>
+   </section>
+
+   <section id="driver-proc">
+     <title>Driver Interaction using /proc filesystem</title>
+     <para>
+       Not Applicable
+       <!-- TODO: Describe data available for read and write on the drivers
+            /proc entry. Specify where the entry for the device is located.
+            Specify the data types for the attributes. Specify if the
+            attributes are read-only or writeonly. If the driver has no /proc
+            interface, replace this text with "Not Applicable". -->
+     </para>
+   </section>
+
+   <section id="driver-other">
+     <title>Other means for Driver Interaction</title>
+     <para>
+       <!-- TODO: Does the driver have any configurations files?
Describe other means
+            for driver status access or configuration. If the driver
has no other
+            means (besides the one in already described in this
chapter), replace
+            this text with "Not Applicable". -->
+       Not Applicable
+     </para>
+   </section>
+
+ <section id="driver-node">
+   <title>Driver Node File</title>
+     <variablelist>
+     <varlistentry>
+       <term>CG2900 main device</term>
+       <listitem>
+         <variablelist>
+           <varlistentry>
+             <term>File</term>
+             <listitem><para><filename>/dev/cg2900_driver0</filename></para></listitem>
+           </varlistentry>
+           <varlistentry>
+             <term>Description</term>
+             <listitem>
+             <para>The cg2900_driver represents the main parent node
for all other character devices supplied in the ST-Ericsson
connectivity driver except for the CCD Test device. It does not
support any operations such as read or write.</para>
+             </listitem>
+           </varlistentry>
+         </variablelist>
+       </listitem>
+     </varlistentry>
+
+     <varlistentry>
+       <term>BT Command</term>
+       <listitem>
+         <variablelist>
+           <varlistentry>
+             <term>File</term>
+             <listitem><para><filename>/dev/cg2900_bt_cmd</filename></para></listitem>
+           </varlistentry>
+           <varlistentry>
+             <term>Description</term>
+             <listitem>
+             <para>The cg2900_bt_cmd is the device for the HCI
Bluetooth command channel.</para>
+             </listitem>
+           </varlistentry>
+         </variablelist>
+       </listitem>
+     </varlistentry>
+
+     <varlistentry>
+       <term>BT ACL</term>
+       <listitem>
+         <variablelist>
+           <varlistentry>
+             <term>File</term>
+             <listitem><para><filename>/dev/cg2900_bt_acl</filename></para></listitem>
+           </varlistentry>
+           <varlistentry>
+             <term>Description</term>
+             <listitem>
+             <para>The cg2900_bt_acl is the device for the HCI
Bluetooth ACL channel.</para>
+             </listitem>
+           </varlistentry>
+         </variablelist>
+       </listitem>
+     </varlistentry>
+
+     <varlistentry>
+       <term>BT Event</term>
+       <listitem>
+         <variablelist>
+           <varlistentry>
+             <term>File</term>
+             <listitem><para><filename>/dev/cg2900_bt_evt</filename></para></listitem>
+           </varlistentry>
+           <varlistentry>
+             <term>Description</term>
+             <listitem>
+             <para>The cg2900_bt_evt is the device for the HCI
Bluetooth event channel.</para>
+             </listitem>
+           </varlistentry>
+         </variablelist>
+       </listitem>
+     </varlistentry>
+
+     <varlistentry>
+       <term>FM Radio</term>
+       <listitem>
+         <variablelist>
+           <varlistentry>
+             <term>File</term>
+             <listitem><para><filename>/dev/cg2900_fm_radio</filename></para></listitem>
+           </varlistentry>
+           <varlistentry>
+             <term>Description</term>
+             <listitem>
+             <para>The cg2900_fm_radio is the device for the HCI FM
Radio channel.</para>
+             </listitem>
+           </varlistentry>
+         </variablelist>
+       </listitem>
+     </varlistentry>
+
+     <varlistentry>
+       <term>GNSS</term>
+       <listitem>
+         <variablelist>
+           <varlistentry>
+             <term>File</term>
+             <listitem><para><filename>/dev/cg2900_gnss</filename></para></listitem>
+           </varlistentry>
+           <varlistentry>
+             <term>Description</term>
+             <listitem>
+             <para>The cg2900_gnss is the device for the HCI GNSS
channel.</para>
+             </listitem>
+           </varlistentry>
+         </variablelist>
+       </listitem>
+     </varlistentry>
+
+     <varlistentry>
+       <term>Debug</term>
+       <listitem>
+         <variablelist>
+           <varlistentry>
+             <term>File</term>
+             <listitem><para><filename>/dev/cg2900_debug</filename></para></listitem>
+           </varlistentry>
+           <varlistentry>
+             <term>Description</term>
+             <listitem>
+             <para>The cg2900_debug is the device for the HCI Debug
channel.</para>
+             </listitem>
+           </varlistentry>
+         </variablelist>
+       </listitem>
+     </varlistentry>
+
+     <varlistentry>
+       <term>ST-Ericsson Tools</term>
+       <listitem>
+         <variablelist>
+           <varlistentry>
+             <term>File</term>
+             <listitem><para><filename>/dev/cg2900_ste_tools</filename></para></listitem>
+           </varlistentry>
+           <varlistentry>
+             <term>Description</term>
+             <listitem>
+             <para>The cg2900_ste_tools is the device for the HCI
ST-Ericsson tools channel.</para>
+             </listitem>
+           </varlistentry>
+         </variablelist>
+       </listitem>
+     </varlistentry>
+
+     <varlistentry>
+       <term>HCI Logger</term>
+       <listitem>
+         <variablelist>
+           <varlistentry>
+             <term>File</term>
+             <listitem><para><filename>/dev/cg2900_hci_logger</filename></para></listitem>
+           </varlistentry>
+           <varlistentry>
+             <term>Description</term>
+             <listitem>
+             <para>The cg2900_hci_logger is the device for the HCI
logger channel.</para>
+             </listitem>
+           </varlistentry>
+         </variablelist>
+       </listitem>
+     </varlistentry>
+
+     <varlistentry>
+       <term>User Space Control</term>
+       <listitem>
+         <variablelist>
+           <varlistentry>
+             <term>File</term>
+             <listitem><para><filename>/dev/cg2900_us_ctrl</filename></para></listitem>
+           </varlistentry>
+           <varlistentry>
+             <term>Description</term>
+             <listitem>
+             <para>The cg2900_us_ctrl is the device for
initialization and control of the STE CONN driver.</para>
+             </listitem>
+           </varlistentry>
+         </variablelist>
+       </listitem>
+     </varlistentry>
+
+     <varlistentry>
+       <term>CCD Test stub</term>
+       <listitem>
+         <variablelist>
+           <varlistentry>
+             <term>File</term>
+             <listitem><para><filename>/dev/cg2900_ccd_test</filename></para></listitem>
+           </varlistentry>
+           <varlistentry>
+             <term>Description</term>
+             <listitem>
+             <para>The cg2900_ccd_test is the device for performing
module tests of the ST-Ericsson connectivity driver. It acts as a stub
replacing the transport towards the chip. It is of the type Misc
devices.</para>
+             </listitem>
+           </varlistentry>
+         </variablelist>
+       </listitem>
+     </varlistentry>
+
+     <varlistentry>
+       <term>BT Audio</term>
+       <listitem>
+         <variablelist>
+           <varlistentry>
+             <term>File</term>
+             <listitem><para><filename>/dev/cg2900_bt_audio</filename></para></listitem>
+           </varlistentry>
+           <varlistentry>
+             <term>Description</term>
+             <listitem>
+             <para>The cg2900_bt_audio is the device for sending HCI
BT Audio controll commands to the chip. </para>
+             </listitem>
+           </varlistentry>
+         </variablelist>
+       </listitem>
+     </varlistentry>
+
+     <varlistentry>
+       <term>FM Audio</term>
+       <listitem>
+         <variablelist>
+           <varlistentry>
+             <term>File</term>
+             <listitem><para><filename>/dev/cg2900_fm_audio</filename></para></listitem>
+           </varlistentry>
+           <varlistentry>
+             <term>Description</term>
+             <listitem>
+             <para>The cg2900_fm_audio is the device for sending HCI
BT Audio controll commands to the chip.</para>
+             </listitem>
+           </varlistentry>
+         </variablelist>
+       </listitem>
+     </varlistentry>
+
+     <varlistentry>
+       <term>Core</term>
+       <listitem>
+         <variablelist>
+           <varlistentry>
+             <term>File</term>
+             <listitem><para><filename>/dev/cg2900_core</filename></para></listitem>
+           </varlistentry>
+           <varlistentry>
+             <term>Description</term>
+             <listitem>
+             <para>The cg2900_core is a device for turn on/off the
chip. NOTE other devices will also turn on/off the chip if
needed.</para>
+             </listitem>
+           </varlistentry>
+         </variablelist>
+       </listitem>
+     </varlistentry>
+
+     <varlistentry>
+       <term>CG29XX Audio</term>
+       <listitem>
+         <variablelist>
+           <varlistentry>
+             <term>File</term>
+             <listitem><para><filename>/dev/cg2900_audio</filename></para></listitem>
+           </varlistentry>
+           <varlistentry>
+             <term>Description</term>
+             <listitem>
+             <para>
+               The cg2900_audio is a device for testing the CG2900
Audio driver from User space.
+               It replicates the normal CG2900 Audio interface
through <constant>write/read</constant> operations.
+               The <constant>write</constant> command is used as following:
+               <itemizedlist>
+                 <listitem><para>4 byte op code (see below)</para></listitem>
+                 <listitem><para>Data field according to respective
CG29XX Audio function (no session ID needed)</para></listitem>
+               </itemizedlist>
+               If the operation fails the <constant>write</constant>
command operation will return the error.
+               Op codes are (4 bytes size):
+               <itemizedlist>
+                 <listitem><para>0x00000001 =
CHAR_DEV_OP_CODE_SET_DAI_CONF</para></listitem>
+                 <listitem><para>0x00000002 =
CHAR_DEV_OP_CODE_GET_DAI_CONF</para></listitem>
+                 <listitem><para>0x00000003 =
CHAR_DEV_OP_CODE_CONFIGURE_ENDPOINT</para></listitem>
+                 <listitem><para>0x00000004 =
CHAR_DEV_OP_CODE_CONNECT_AND_START_STREAM</para></listitem>
+                 <listitem><para>0x00000005 =
CHAR_DEV_OP_CODE_STOP_STREAM</para></listitem>
+               </itemizedlist>
+
+               The <constant>read</constant> command is used for the
commands <constant>CHAR_DEV_OP_CODE_GET_DAI_CONF</constant>
+               and
<constant>CHAR_DEV_OP_CODE_CONNECT_AND_START_STREAM</constant> if the
corresponding commands are successful.
+               The returned data will be formatted accordingly:
+               <itemizedlist>
+                 <listitem><para>4 byte op code (see below)</para></listitem>
+                 <listitem><para>Data field according to normal
CG29XX Audio functions, e.g. stream handle or
configuration</para></listitem>
+               </itemizedlist>
+               The <constant>CHAR_DEV_OP_CODE_GET_DAI_CONF</constant>
is a bit special since it require an endpoint in-parameter
+               (when calling <constant>write</constant>) to return
the corresponding DAI configuration when calling
<constant>read</constant>.
+             </para>
+             </listitem>
+           </varlistentry>
+         </variablelist>
+       </listitem>
+     </varlistentry>
+
+    </variablelist>
+  </section>
+
+
+ </chapter>
+
+
+ <chapter id="bugs">
+   <title>Known Bugs And Assumptions</title>
+   <!--  Do NOT change the chapter id or title! -->
+   <para>
+     <variablelist>
+     <varlistentry>
+       <term>Driver supports only one user per HCI channel.</term>
+       <listitem>
+         <para>
+           To simplify design and limitation as well as keeping the
API simple and reliable, the driver only supports one user per HCI
channel.
+           <!-- TODO: Briefly describe the limitation, unless all
+              information is already present in the title.
+              Use full english sentences.
+              Repeat the varlistentry for each limitation.
+              If none are known, replace this varlistentry
+              with the one below. -->
+           <!-- TODO: This guideline for this chapter may be extended
+              during the user-guide guidelines drop. -->
+         </para>
+       </listitem>
+     </varlistentry>
+     </variablelist>
+   </para>
+ </chapter>
+
+<chapter id="pubfunctions">
+   <title>Public Functions Provided</title>
+   <para>
+	List of public functions.
+   </para>
+   <!-- Do NOT change the chapter id or title! -->
+   <!-- TODO: Replace with link to appropriate headerfile(s).
+        One per row, ensure the
+        exclamation mark is on the first column! If no
+        appropriate header file exist describing a public interface,
+        replace the inclusion with a paragraph containing the text
+        "Not Applicable" -->
+  <section id="cg2900.h">
+    <title>cg2900.h</title>
+!Edrivers/mfd/cg2900/cg2900_core.c
+  </section>
+  <section id="cg2900_audio.h">
+    <title>cg2900_audio.h</title>
+!Edrivers/mfd/cg2900/cg2900_audio.c
+  </section>
+
+</chapter>
+
+<chapter id="internal-functions">
+   <title>Internal Functions Provided</title>
+   <para>
+	List of internal functions.
+   </para>
+   <!-- Do NOT change the chapter id or title! -->
+   <!-- TODO: Replace with link to appropriate headerfile(s),
+        source file(s), or both. One per row, ensure the
+        exclamation mark is on the first column! If no
+        appropriate header or source file exist describing a public interface,
+        replace the inclusion with a paragraph containing the text
+        "Not Applicable"-->
+  <section id="cg2900_core.h">
+    <title>cg2900_core.h</title>
+!Edrivers/mfd/cg2900/cg2900_core.h
+  </section>
+</chapter>
+</book>
-- 
1.6.3.3

^ permalink raw reply related

* [PATCH 8/9] Bluetooth: Add support for the ST-Ericsson CG2900.
From: Par-Gunnar Hjalmdahl @ 2010-10-22 10:40 UTC (permalink / raw)
  To: linus.walleij, linux-bluetooth, linux-kernel

This patch adds support for the ST-Ericsson CG2900 Connectivity
Combo controller.
This patch registers to the Bluetooth stack and, when opened, it
registers to the Bluetooth channels in the CG2900 MFD driver.

Signed-off-by: Par-Gunnar Hjalmdahl <par-gunnar.p.hjalmdahl@stericsson.com>
---
 drivers/bluetooth/Kconfig    |    7 +
 drivers/bluetooth/Makefile   |    1 +
 drivers/bluetooth/btcg2900.c |  925 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 933 insertions(+), 0 deletions(-)
 create mode 100644 drivers/bluetooth/btcg2900.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..1f8ce2d 100644
--- a/drivers/bluetooth/Makefile
+++ b/drivers/bluetooth/Makefile
@@ -16,6 +16,7 @@ obj-$(CONFIG_BT_HCIBTUSB)	+= btusb.o
 obj-$(CONFIG_BT_HCIBTSDIO)	+= btsdio.o

 obj-$(CONFIG_BT_ATH3K)		+= ath3k.o
+obj-$(CONFIG_BT_CG2900)		+= btcg2900.o
 obj-$(CONFIG_BT_MRVL)		+= btmrvl.o
 obj-$(CONFIG_BT_MRVL_SDIO)	+= btmrvl_sdio.o

diff --git a/drivers/bluetooth/btcg2900.c b/drivers/bluetooth/btcg2900.c
new file mode 100644
index 0000000..11d6518
--- /dev/null
+++ b/drivers/bluetooth/btcg2900.c
@@ -0,0 +1,925 @@
+/*
+ * Bluetooth driver for ST-Ericsson CG2900 connectivity controller.
+ *
+ * Copyright (C) ST-Ericsson SA 2010
+ *
+ * Authors:
+ * Par-Gunnar Hjalmdahl (par-gunnar.p.hjalmdahl@stericsson.com)
+ * Henrik Possung (henrik.possung@stericsson.com)
+ * Josef Kindberg (josef.kindberg@stericsson.com)
+ * Dariusz Szymszak (dariusz.xd.szymczak@stericsson.com)
+ * Kjell Andersson (kjell.k.andersson@stericsson.com)
+ *
+ * License terms:  GNU General Public License (GPL), version 2
+ */
+
+#include <asm/byteorder.h>
+#include <linux/jiffies.h>
+#include <linux/module.h>
+#include <linux/sched.h>
+#include <linux/platform_device.h>
+#include <linux/skbuff.h>
+#include <linux/time.h>
+#include <linux/timer.h>
+#include <linux/types.h>
+#include <linux/wait.h>
+#include <linux/workqueue.h>
+#include <linux/mfd/cg2900.h>
+#include <net/bluetooth/bluetooth.h>
+#include <net/bluetooth/hci.h>
+#include <net/bluetooth/hci_core.h>
+
+#define BT_VS_BT_ENABLE			0xFF10
+
+#define VS_BT_DISABLE			0x00
+#define VS_BT_ENABLE			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
+
+#define NAME				"BTCG2900 "
+
+/* Wait for 5 seconds for a response to our requests */
+#define RESP_TIMEOUT			5000
+
+/* 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 btcg2900_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.
+ * @pdev:		Device structure for platform device.
+ * @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 btcg2900_info {
+	struct cg2900_device	*bt_cmd;
+	struct cg2900_device	*bt_evt;
+	struct cg2900_device	*bt_acl;
+	struct platform_device	*pdev;
+	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 CG2900 driver.
+ *
+ * @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;
+};
+
+/**
+ * struct vs_bt_enable_cmd - Specifies HCI VS Bluetooth_Enable command.
+ *
+ * @op_code:	HCI command op code.
+ * @len:	Parameter length of command.
+ * @enable:	0 for disable BT, 1 for enable BT.
+ */
+struct vs_bt_enable_cmd {
+	__le16	op_code;
+	u8	len;
+	u8	enable;
+} __attribute__((packed));
+
+static struct btcg2900_info *btcg2900_info;
+
+/*
+ * hci_wait_queue - Main Wait Queue in HCI driver.
+ */
+static DECLARE_WAIT_QUEUE_HEAD(hci_wait_queue);
+
+/* Internal function declarations */
+static int register_bluetooth(void);
+
+/* Internal functions */
+
+/**
+ * get_bt_enable_cmd() - Get HCI BT enable command.
+ * @bt_enable:	true if Bluetooth IP shall be enabled, false otherwise.
+ *
+ * Returns:
+ *   NULL if no command shall be sent,
+ *   sk_buffer with command otherwise.
+ */
+struct sk_buff *get_bt_enable_cmd(bool bt_enable)
+{
+	struct sk_buff *skb;
+	struct vs_bt_enable_cmd *cmd;
+	struct cg2900_rev_data rev_data;
+
+	if (!cg2900_get_local_revision(&rev_data)) {
+		BT_ERR(NAME "Couldn't get revision");
+		return NULL;
+	}
+
+	/* If connected chip does not support the command return NULL */
+	if (CG2900_PG1_SPECIAL_HCI_REV != rev_data.revision &&
+	    CG2900_PG1_HCI_REV != rev_data.revision &&
+	    CG2900_PG2_HCI_REV != rev_data.revision)
+		return NULL;
+
+	/* CG2900 used */
+	skb = cg2900_alloc_skb(sizeof(*cmd), GFP_KERNEL);
+	if (!skb) {
+		BT_ERR(NAME "Could not allocate skb");
+		return NULL;
+	}
+
+	cmd = (struct vs_bt_enable_cmd *)skb_put(skb, sizeof(*cmd));
+	cmd->op_code = cpu_to_le16(BT_VS_BT_ENABLE);
+	cmd->len = sizeof(*cmd) - BT_HEADER_LENGTH;
+	if (bt_enable)
+		cmd->enable = VS_BT_ENABLE;
+	else
+		cmd->enable = VS_BT_DISABLE;
+
+	return skb;
+}
+
+/**
+ * remove_bt_users() - Unregister and remove any existing BT users.
+ * @info:	HCI driver info structure.
+ */
+static void remove_bt_users(struct btcg2900_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) {
+		BT_ERR(NAME "NULL supplied for skb");
+		return;
+	}
+
+	if (!dev) {
+		BT_ERR(NAME "dev == NULL");
+		goto fin_free_skb;
+	}
+
+	dev_info = (struct dev_info *)dev->user_data;
+
+	if (!dev_info) {
+		BT_ERR(NAME "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 ((btcg2900_info->enable_state == ENABLE_WAITING_BT_ENABLED_CC ||
+	     btcg2900_info->enable_state == ENABLE_WAITING_BT_DISABLED_CC) &&
+	    btcg2900_info->bt_evt->h4_channel == dev->h4_channel &&
+	    evt->evt == HCI_EV_CMD_COMPLETE &&
+	    le16_to_cpu(cmd_complete->opcode) == BT_VS_BT_ENABLE) {
+		/*
+		 * 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) {
+			BT_ERR(NAME "Could not enable/disable BT core (0x%X)",
+				   status);
+			BT_DBG("New enable_state: ENABLE_BT_ERROR");
+			btcg2900_info->enable_state = ENABLE_BT_ERROR;
+			goto fin_free_skb;
+		}
+
+		if (btcg2900_info->enable_state ==
+				ENABLE_WAITING_BT_ENABLED_CC) {
+			BT_DBG("New enable_state: ENABLE_BT_ENABLED");
+			btcg2900_info->enable_state = ENABLE_BT_ENABLED;
+			BT_INFO("CG2900 BT core is enabled");
+		} else {
+			BT_DBG("New enable_state: ENABLE_BT_DISABLED");
+			btcg2900_info->enable_state = ENABLE_BT_DISABLED;
+			BT_INFO("CG2900 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 ((btcg2900_info->enable_state ==
+			ENABLE_WAITING_BT_DISABLED_CC ||
+		    btcg2900_info->enable_state ==
+			ENABLE_WAITING_BT_ENABLED_CC) &&
+		   btcg2900_info->bt_evt->h4_channel == dev->h4_channel &&
+		   evt->evt == HCI_EV_CMD_STATUS &&
+		   le16_to_cpu(cmd_status->opcode) == BT_VS_BT_ENABLE) {
+		/*
+		 * Clear the status events since the Bluez is not expecting
+		 * them.
+		 */
+		BT_DBG("HCI Driver received Command Status (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++;
+
+		BT_DBG("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)
+			BT_ERR(NAME "Failed in supplying packet to Bluetooth"
+			       " stack (%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 btcg2900_info *info;
+
+	BT_INFO(NAME "hci_reset_cb");
+
+	if (!dev) {
+		BT_ERR(NAME "NULL supplied for dev");
+		return;
+	}
+
+	dev_info = (struct dev_info *)dev->user_data;
+	if (!dev_info) {
+		BT_ERR(NAME "NULL supplied for dev_info");
+		return;
+	}
+
+	hdev = dev_info->hdev;
+	if (!hdev) {
+		BT_ERR(NAME "NULL supplied for hdev");
+		return;
+	}
+
+	info = (struct btcg2900_info *)hdev->driver_data;
+	if (!info) {
+		BT_ERR(NAME "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:
+		BT_ERR(NAME "Unknown HCI data type:%d",
+		       dev_info->hci_data_type);
+		return;
+	}
+
+	BT_DBG("New reset_state: RESET_ACTIVATED");
+	btcg2900_info->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.
+	 */
+	BT_DBG("Deregister HCI device");
+	err = hci_unregister_dev(hdev);
+	if (err)
+		BT_ERR(NAME "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 == btcg2900_info->reset_state),
+			msecs_to_jiffies(RESP_TIMEOUT));
+	if (RESET_UNREGISTERED != btcg2900_info->reset_state)
+		/*
+		 * Now we are in trouble. Try to register a new hdev
+		 * anyway even though this will cost some memory.
+		 */
+		BT_ERR(NAME "Timeout expired. Could not deregister HCI device");
+
+	/* Init and register hdev */
+	BT_DBG("Register HCI device");
+	err = register_bluetooth();
+	if (err)
+		BT_ERR(NAME "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
+};
+
+/**
+ * btcg2900_open() - 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 btcg2900_open(struct hci_dev *hdev)
+{
+	struct btcg2900_info *info;
+	struct dev_info *dev_info;
+	struct sk_buff *enable_cmd;
+	int err;
+
+	BT_INFO("Open ST-Ericsson CG2900 driver");
+
+	if (!hdev) {
+		BT_ERR(NAME "NULL supplied for hdev");
+		return -EINVAL;
+	}
+
+	info = (struct btcg2900_info *)hdev->driver_data;
+	if (!info) {
+		BT_ERR(NAME "NULL supplied for driver_data");
+		return -EINVAL;
+	}
+
+	if (test_and_set_bit(HCI_RUNNING, &(hdev->flags))) {
+		BT_ERR(NAME "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 {
+			BT_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 {
+			BT_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 {
+			BT_ERR("Couldn't register CG2900_BT_ACL to CG2900");
+			err = -EACCES;
+			goto handle_error;
+		}
+	}
+
+	if (info->reset_state == RESET_ACTIVATED) {
+		BT_DBG("New reset_state: RESET_IDLE");
+		btcg2900_info->reset_state = RESET_IDLE;
+	}
+
+	/*
+	 * Call function that returns the chip dependent vs_bt_enable(true)
+	 * HCI command.
+	 * If NULL is returned, then no bt_enable command should be sent to the
+	 * chip.
+	 */
+	enable_cmd = get_bt_enable_cmd(true);
+	if (!enable_cmd) {
+		/* The chip is enabled by default */
+		BT_DBG("New enable_state: ENABLE_BT_ENABLED");
+		btcg2900_info->enable_state = ENABLE_BT_ENABLED;
+		return 0;
+	}
+
+	/* Set the HCI state before sending command to chip. */
+	BT_DBG("New enable_state: ENABLE_WAITING_BT_ENABLED_CC");
+	btcg2900_info->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) {
+		BT_ERR("Could not enable CG2900 BT core (%d)",
+		       info->enable_state);
+		err = -EACCES;
+		BT_DBG("New enable_state: ENABLE_BT_DISABLED");
+		btcg2900_info->enable_state = ENABLE_BT_DISABLED;
+		goto handle_error;
+	}
+
+	return 0;
+
+handle_error:
+	remove_bt_users(info);
+	clear_bit(HCI_RUNNING, &(hdev->flags));
+	return err;
+
+}
+
+/**
+ * btcg2900_close() - 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 btcg2900_close(struct hci_dev *hdev)
+{
+	struct btcg2900_info *info = NULL;
+	struct sk_buff *enable_cmd;
+
+	BT_DBG("btcg2900_close");
+
+	if (!hdev) {
+		BT_ERR(NAME "NULL supplied for hdev");
+		return -EINVAL;
+	}
+
+	info = (struct btcg2900_info *)hdev->driver_data;
+	if (!info) {
+		BT_ERR(NAME "NULL supplied for driver_data");
+		return -EINVAL;
+	}
+
+	if (!test_and_clear_bit(HCI_RUNNING, &(hdev->flags))) {
+		BT_ERR(NAME "Device already closed!");
+		return -EBUSY;
+	}
+
+	/* Do not do this if there is an reset ongoing */
+	if (btcg2900_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 = get_bt_enable_cmd(false);
+	if (!enable_cmd) {
+		/*
+		 * The chip is enabled by default and we should not disable it.
+		 */
+		BT_DBG("New enable_state: ENABLE_BT_ENABLED");
+		btcg2900_info->enable_state = ENABLE_BT_ENABLED;
+		goto remove_users;
+	}
+
+	/* Set the HCI state before sending command to chip */
+	BT_DBG("New enable_state: ENABLE_WAITING_BT_DISABLED_CC");
+	btcg2900_info->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) {
+		BT_ERR("Could not disable CG2900 BT core.");
+		BT_DBG("New enable_state: ENABLE_BT_ENABLED");
+		btcg2900_info->enable_state = ENABLE_BT_ENABLED;
+	}
+
+remove_users:
+	/* Finally deregister all users and free allocated data */
+	remove_bt_users(info);
+	return 0;
+}
+
+/**
+ * btcg2900_send() - 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 btcg2900_send(struct sk_buff *skb)
+{
+	struct hci_dev *hdev;
+	struct btcg2900_info *info;
+	int err = 0;
+
+	if (!skb) {
+		BT_ERR(NAME "NULL supplied for skb");
+		return -EINVAL;
+	}
+
+	hdev = (struct hci_dev *)(skb->dev);
+	if (!hdev) {
+		BT_ERR(NAME "NULL supplied for hdev");
+		return -EINVAL;
+	}
+
+	info = (struct btcg2900_info *)hdev->driver_data;
+	if (!info) {
+		BT_ERR(NAME "NULL supplied for info");
+		return -EINVAL;
+	}
+
+	/* Update BlueZ stats */
+	hdev->stat.byte_tx += skb->len;
+
+	BT_DBG("Data transmit %d bytes", skb->len);
+
+	switch (bt_cb(skb)->pkt_type) {
+	case HCI_COMMAND_PKT:
+		BT_DBG("Sending HCI_COMMAND_PKT");
+		err = cg2900_write(info->bt_cmd, skb);
+		hdev->stat.cmd_tx++;
+		break;
+	case HCI_ACLDATA_PKT:
+		BT_DBG("Sending HCI_ACLDATA_PKT");
+		err = cg2900_write(info->bt_acl, skb);
+		hdev->stat.acl_tx++;
+		break;
+	default:
+		BT_ERR(NAME "Trying to transmit unsupported packet type"
+		       " (0x%.2X)", bt_cb(skb)->pkt_type);
+		err = -EOPNOTSUPP;
+		break;
+	};
+
+	return err;
+}
+
+/**
+ * btcg2900_destruct() - Destruct HCI interface.
+ * @hdev:	HCI device being destructed.
+ */
+static void btcg2900_destruct(struct hci_dev *hdev)
+{
+	BT_DBG("btcg2900_destruct");
+
+	if (!btcg2900_info)
+		return;
+
+	/*
+	 * When destruct is called it means that the Bluetooth stack is done
+	 * with the HCI device and we can now free it.
+	 * Normally we do this only when removing the whole module through
+	 * btcg2900_remove(), but when being reset we free the device here and
+	 * we then set the reset state so that the reset handler can allocate a
+	 * new HCI device and then register it to the Bluetooth stack.
+	 */
+	if (btcg2900_info->reset_state == RESET_ACTIVATED) {
+		if (btcg2900_info->hdev)
+			hci_free_dev(btcg2900_info->hdev);
+		BT_DBG("New reset_state: RESET_UNREGISTERED");
+		btcg2900_info->reset_state = RESET_UNREGISTERED;
+		wake_up_interruptible(&hci_wait_queue);
+	}
+}
+
+/**
+ * register_bluetooth() - 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_bluetooth(void)
+{
+	int err;
+	static struct cg2900_bt_platform_data *pf_data;
+
+	pf_data = dev_get_platdata(&btcg2900_info->pdev->dev);
+
+	btcg2900_info->hdev = hci_alloc_dev();
+	if (!btcg2900_info->hdev) {
+		BT_ERR("Could not allocate mem for CG2900 BT driver");
+		return -ENOMEM;
+	}
+
+	SET_HCIDEV_DEV(btcg2900_info->hdev, &btcg2900_info->pdev->dev);
+	if (pf_data) {
+		btcg2900_info->hdev->bus = pf_data->bus;
+	} else {
+		BT_DBG(NAME "Missing platform data. Defaulting to UART");
+		btcg2900_info->hdev->bus = HCI_UART;
+	}
+	btcg2900_info->hdev->driver_data = btcg2900_info;
+	btcg2900_info->hdev->owner = THIS_MODULE;
+	btcg2900_info->hdev->open = btcg2900_open;
+	btcg2900_info->hdev->close = btcg2900_close;
+	btcg2900_info->hdev->send = btcg2900_send;
+	btcg2900_info->hdev->destruct = btcg2900_destruct;
+
+	err = hci_register_dev(btcg2900_info->hdev);
+	if (err) {
+		BT_ERR(NAME "Can not register HCI device (%d)", err);
+		hci_free_dev(btcg2900_info->hdev);
+	}
+
+	BT_DBG("New enable_state: ENABLE_IDLE");
+	btcg2900_info->enable_state = ENABLE_IDLE;
+	BT_DBG("New reset_state: RESET_IDLE");
+	btcg2900_info->reset_state = RESET_IDLE;
+
+	return err;
+}
+
+/**
+ * btcg2900_probe() - Initialize module.
+ *
+ * Allocate and initialize private data. Register to Bluetooth stack.
+ *
+ * Returns:
+ *   0 if there is no error.
+ *   -ENOMEM if allocation fails.
+ *   Error codes from register_bluetooth.
+ */
+static int __devinit btcg2900_probe(struct platform_device *pdev)
+{
+	int err;
+
+	BT_INFO("btcg2900_probe");
+
+	/* Initialize private data. */
+	btcg2900_info = kzalloc(sizeof(*btcg2900_info), GFP_KERNEL);
+	if (!btcg2900_info) {
+		BT_ERR("Could not alloc btcg2900_info struct.");
+		return -ENOMEM;
+	}
+
+	btcg2900_info->pdev = pdev;
+
+	/* Init and register hdev */
+	err = register_bluetooth();
+	if (err) {
+		BT_ERR("HCI Device registration error (%d)", err);
+		kfree(btcg2900_info);
+		btcg2900_info = NULL;
+		return err;
+	}
+
+	return 0;
+}
+
+/**
+ * btcg2900_remove() - Remove module.
+ */
+static int __devexit btcg2900_remove(struct platform_device *pdev)
+{
+	int err = 0;
+
+	BT_INFO("btcg2900_remove");
+
+	if (!btcg2900_info)
+		return 0;
+
+	if (!btcg2900_info->hdev)
+		goto finished;
+
+	err = hci_unregister_dev(btcg2900_info->hdev);
+	if (err)
+		BT_ERR("Can not unregister HCI device (%d)", err);
+	hci_free_dev(btcg2900_info->hdev);
+
+finished:
+	kfree(btcg2900_info);
+	btcg2900_info = NULL;
+	return err;
+}
+
+static struct platform_driver btcg2900_driver = {
+	.driver = {
+		.name	= "cg2900-bt",
+		.owner	= THIS_MODULE,
+	},
+	.probe	= btcg2900_probe,
+	.remove	= __devexit_p(btcg2900_remove),
+};
+
+/**
+ * btcg2900_init() - Initialize module.
+ *
+ * Registers platform driver.
+ */
+static int __init btcg2900_init(void)
+{
+	BT_INFO("btcg2900_init");
+	return platform_driver_register(&btcg2900_driver);
+}
+
+/**
+ * btcg2900_exit() - Remove module.
+ *
+ * Unregisters platform driver.
+ */
+static void __exit btcg2900_exit(void)
+{
+	BT_INFO("btcg2900_exit");
+	platform_driver_unregister(&btcg2900_driver);
+}
+
+module_init(btcg2900_init);
+module_exit(btcg2900_exit);
+
+MODULE_AUTHOR("Par-Gunnar Hjalmdahl ST-Ericsson");
+MODULE_AUTHOR("Henrik Possung ST-Ericsson");
+MODULE_AUTHOR("Josef Kindberg ST-Ericsson");
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Linux Bluetooth HCI H:4 Driver for ST-Ericsson
controller");
-- 
1.6.3.3

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox