* 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 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/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/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 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 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 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
* [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
* (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
* 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
* 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
* [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
* [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
* [PATCH 2/6] Bluetooth: fix receiving L2CAP packets over LE
From: Anderson Briglia @ 2010-10-22 23:56 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Vinicius Costa Gomes
From: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
As L2CAP packets coming over LE don't have any more encapsulation,
other than L2CAP, we are able to process them as soon as they arrive.
Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
---
net/bluetooth/l2cap.c | 17 +++++++++++++++--
1 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 2bf083e..1ac44f4 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -4768,17 +4768,30 @@ static int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
static int l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
{
struct l2cap_conn *conn = hcon->l2cap_data;
+ struct l2cap_hdr *hdr;
+ int len;
if (!conn && !(conn = l2cap_conn_add(hcon, 0)))
goto drop;
BT_DBG("conn %p len %d flags 0x%x", conn, skb->len, flags);
+ if (hcon->type == LE_LINK) {
+ hdr = (struct l2cap_hdr *) skb->data;
+ len = __le16_to_cpu(hdr->len) + L2CAP_HDR_SIZE;
+
+ if (len == skb->len) {
+ /* Complete frame received */
+ l2cap_recv_frame(conn, skb);
+ return 0;
+ }
+
+ goto drop;
+ }
+
if (flags & ACL_START) {
- struct l2cap_hdr *hdr;
struct sock *sk;
u16 cid;
- int len;
if (conn->rx_len) {
BT_ERR("Unexpected start frame (len %d)", skb->len);
--
1.7.0.4
^ permalink raw reply related
* [PATCH 3/6] Bluetooth: Implement the first SMP commands
From: Anderson Briglia @ 2010-10-22 23:56 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Vinicius Costa Gomes
From: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
These simple commands will allow the SMP procedure to be started
and terminated with a not supported error. This is the first step
toward something useful.
Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
---
net/bluetooth/l2cap.c | 117 +++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 117 insertions(+), 0 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 1ac44f4..ba87c84 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -54,6 +54,7 @@
#include <net/bluetooth/bluetooth.h>
#include <net/bluetooth/hci_core.h>
#include <net/bluetooth/l2cap.h>
+#include <net/bluetooth/smp.h>
#define VERSION "2.15"
@@ -307,6 +308,85 @@ static void l2cap_chan_del(struct sock *sk, int err)
}
}
+static struct sk_buff *smp_build_cmd(struct l2cap_conn *conn, u8 code,
+ u16 dlen, void *data)
+{
+ struct sk_buff *skb;
+ struct l2cap_hdr *lh;
+ int len;
+
+ len = L2CAP_HDR_SIZE + 1 + dlen;
+
+ if (len > conn->mtu)
+ return NULL;
+
+ skb = bt_skb_alloc(len, GFP_ATOMIC);
+ if (!skb)
+ return NULL;
+
+ lh = (struct l2cap_hdr *) skb_put(skb, L2CAP_HDR_SIZE);
+ lh->len = cpu_to_le16(1 + dlen);
+ lh->cid = cpu_to_le16(L2CAP_CID_SMP);
+
+ memcpy(skb_put(skb, 1), &code, 1);
+
+ memcpy(skb_put(skb, dlen), data, dlen);
+
+ return skb;
+}
+
+static inline void smp_send_cmd(struct l2cap_conn *conn, u8 code, u16 len, void *data)
+{
+ struct sk_buff *skb = smp_build_cmd(conn, code, len, data);
+
+ BT_DBG("code 0x%2.2x", code);
+
+ if (!skb)
+ return;
+
+ hci_send_acl(conn->hcon, skb, 0);
+}
+
+static int smp_conn_security(struct l2cap_conn *conn, __u8 sec_level)
+{
+ __u8 authreq;
+
+ BT_DBG("conn %p hcon %p level 0x%2.2x", conn, conn->hcon, sec_level);
+
+ switch (sec_level) {
+ case BT_SECURITY_MEDIUM:
+ /* Encrypted, no MITM protection */
+ authreq = 0x01;
+ break;
+
+ case BT_SECURITY_HIGH:
+ /* Bonding, MITM protection */
+ authreq = 0x05;
+ break;
+
+ case BT_SECURITY_LOW:
+ default:
+ return 1;
+ }
+
+ if (conn->hcon->link_mode & HCI_LM_MASTER) {
+ struct smp_cmd_pairing cp;
+ cp.io_capability = 0x00;
+ cp.oob_flag = 0x00;
+ cp.max_key_size = 16;
+ cp.init_key_dist = 0x00;
+ cp.resp_key_dist = 0x00;
+ cp.auth_req = authreq;
+ smp_send_cmd(conn, SMP_CMD_PAIRING_REQ, sizeof(cp), &cp);
+ } else {
+ struct smp_cmd_security_req cp;
+ cp.auth_req = authreq;
+ smp_send_cmd(conn, SMP_CMD_SECURITY_REQ, sizeof(cp), &cp);
+ }
+
+ return 0;
+}
+
/* Service level security */
static inline int l2cap_check_security(struct sock *sk)
{
@@ -4562,6 +4642,43 @@ done:
return 0;
}
+static inline void smp_sig_channel(struct l2cap_conn *conn, struct sk_buff *skb)
+{
+ __u8 code = skb->data[0];
+ __u8 reason;
+
+ skb_pull(skb, 1);
+
+ switch (code) {
+ case SMP_CMD_PAIRING_REQ:
+ reason = SMP_PAIRING_NOTSUPP;
+ smp_send_cmd(conn, SMP_CMD_PAIRING_FAIL, 1, &reason);
+ l2cap_conn_del(conn->hcon, 0x05);
+ break;
+
+ case SMP_CMD_PAIRING_FAIL:
+ break;
+
+ case SMP_CMD_PAIRING_RSP:
+ case SMP_CMD_PAIRING_CONFIRM:
+ case SMP_CMD_PAIRING_RANDOM:
+ case SMP_CMD_ENCRYPT_INFO:
+ case SMP_CMD_MASTER_IDENT:
+ case SMP_CMD_IDENT_INFO:
+ case SMP_CMD_IDENT_ADDR_INFO:
+ case SMP_CMD_SIGN_INFO:
+ case SMP_CMD_SECURITY_REQ:
+ default:
+ BT_DBG("Unknown command code 0x%2.2x", code);
+
+ reason = SMP_CMD_NOTSUPP;
+ smp_send_cmd(conn, SMP_CMD_PAIRING_FAIL, 1, &reason);
+ l2cap_conn_del(conn->hcon, 0x05);
+ }
+
+ kfree_skb(skb);
+}
+
static void l2cap_recv_frame(struct l2cap_conn *conn, struct sk_buff *skb)
{
struct l2cap_hdr *lh = (void *) skb->data;
--
1.7.0.4
^ permalink raw reply related
* [PATCH 4/6] Bluetooth: Start SMP procedure
From: Anderson Briglia @ 2010-10-22 23:56 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Briglia, Vinicius Costa Gomes
Start SMP procedure for LE connections. This modification intercepts l2cap
received frames and call proper SMP functions to start the SMP procedure. By
now, no keys are being used.
Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
Signed-off-by: Anderson Briglia <anderson.briglia@openbossa.org>
---
net/bluetooth/l2cap.c | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index ba87c84..2a88f23 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -714,6 +714,8 @@ static void l2cap_conn_ready(struct l2cap_conn *conn)
l2cap_sock_clear_timer(sk);
sk->sk_state = BT_CONNECTED;
sk->sk_state_change(sk);
+ if (smp_conn_security(conn, l2cap_pi(sk)->sec_level))
+ BT_DBG("Insufficient security");
}
if (sk->sk_type != SOCK_SEQPACKET &&
@@ -4707,6 +4709,10 @@ static void l2cap_recv_frame(struct l2cap_conn *conn, struct sk_buff *skb)
l2cap_conless_channel(conn, psm, skb);
break;
+ case L2CAP_CID_SMP:
+ smp_sig_channel(conn, skb);
+ break;
+
default:
l2cap_data_channel(conn, cid, skb);
break;
--
1.7.0.4
^ permalink raw reply related
* [PATCH 5/6] Bluetooth: Fix initiated LE connections
From: Anderson Briglia @ 2010-10-22 23:56 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Vinicius Costa Gomes
From: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
Fix LE connections not being marked as master.
Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
---
net/bluetooth/hci_conn.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index c7309e4..1a48c6a 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -52,6 +52,7 @@ void hci_le_connect(struct hci_conn *conn)
conn->state = BT_CONNECT;
conn->out = 1;
+ conn->link_mode |= HCI_LM_MASTER;
memset(&cp, 0, sizeof(cp));
cp.scan_interval = cpu_to_le16(0x0004);
--
1.7.0.4
^ permalink raw reply related
* [PATCH 6/6] Bluetooth: simple SMP pairing negotiation
From: Anderson Briglia @ 2010-10-22 23:57 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Vinicius Costa Gomes
From: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
This implementation only exchanges SMP messages between the Host and the
Remote. No keys are being generated. TK and STK generation will be
provided in further patches.
Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
---
net/bluetooth/l2cap.c | 118 +++++++++++++++++++++++++++++++++++++++++++++++--
1 files changed, 114 insertions(+), 4 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 2a88f23..be91a33 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -4644,6 +4644,104 @@ done:
return 0;
}
+static void smp_cmd_pairing_req(struct l2cap_conn *conn, struct sk_buff *skb)
+{
+ struct smp_cmd_pairing *rp = (void *) skb->data;
+
+ BT_DBG("");
+
+ skb_pull(skb, sizeof(struct smp_cmd_pairing));
+
+ rp->io_capability = 0x00;
+ rp->oob_flag = 0x00;
+ rp->max_key_size = 16;
+ rp->init_key_dist = 0x00;
+ rp->resp_key_dist = 0x00;
+ rp->auth_req &= 0x05;
+
+ smp_send_cmd(conn, SMP_CMD_PAIRING_RSP, sizeof(*rp), rp);
+}
+
+static void smp_cmd_pairing_rsp(struct l2cap_conn *conn, struct sk_buff *skb)
+{
+ struct smp_cmd_pairing_confirm cp;
+
+ BT_DBG("");
+
+ memset(&cp, 0, sizeof(struct smp_cmd_pairing_confirm));
+
+ smp_send_cmd(conn, SMP_CMD_PAIRING_CONFIRM, sizeof(cp), &cp);
+}
+
+static void smp_cmd_pairing_confirm(struct l2cap_conn *conn,
+ struct sk_buff *skb)
+{
+ BT_DBG("");
+
+ if (conn->hcon->link_mode & HCI_LM_MASTER) {
+
+ struct smp_cmd_pairing_random random;
+
+ BT_DBG("master");
+
+ memset(&random, 0, sizeof(struct smp_cmd_pairing_random));
+
+ smp_send_cmd(conn, SMP_CMD_PAIRING_RANDOM, sizeof(random),
+ &random);
+ } else {
+ struct smp_cmd_pairing_confirm confirm;
+
+ BT_DBG("slave");
+
+ memset(&confirm, 0, sizeof(struct smp_cmd_pairing_confirm));
+
+ smp_send_cmd(conn, SMP_CMD_PAIRING_CONFIRM, sizeof(confirm),
+ &confirm);
+ }
+}
+
+static void smp_cmd_pairing_random(struct l2cap_conn *conn, struct sk_buff *skb)
+{
+ struct smp_cmd_pairing_random cp;
+
+ BT_DBG("");
+
+ skb_pull(skb, sizeof(struct smp_cmd_pairing_random));
+
+ /* FIXME: check if random matches */
+
+ if (conn->hcon->link_mode & HCI_LM_MASTER) {
+ BT_DBG("master");
+ /* FIXME: start encryption */
+ } else {
+ BT_DBG("slave");
+
+ memset(&cp, 0, sizeof(struct smp_cmd_pairing_random));
+
+ smp_send_cmd(conn, SMP_CMD_PAIRING_RANDOM, sizeof(cp), &cp);
+ }
+}
+
+static void smp_cmd_security_req(struct l2cap_conn *conn, struct sk_buff *skb)
+{
+ struct smp_cmd_security_req *rp = (void *) skb->data;
+ struct smp_cmd_pairing cp;
+
+ BT_DBG("");
+
+ skb_pull(skb, sizeof(struct smp_cmd_security_req));
+ memset(&cp, 0, sizeof(struct smp_cmd_pairing));
+
+ cp.io_capability = 0x00;
+ cp.oob_flag = 0x00;
+ cp.max_key_size = 16;
+ cp.init_key_dist = 0x00;
+ cp.resp_key_dist = 0x00;
+ cp.auth_req = rp->auth_req & 0x05;
+
+ smp_send_cmd(conn, SMP_CMD_PAIRING_REQ, sizeof(cp), &cp);
+}
+
static inline void smp_sig_channel(struct l2cap_conn *conn, struct sk_buff *skb)
{
__u8 code = skb->data[0];
@@ -4651,25 +4749,37 @@ static inline void smp_sig_channel(struct l2cap_conn *conn, struct sk_buff *skb)
skb_pull(skb, 1);
+ BT_DBG("conn %p code 0x%2.2x", conn, code);
+
switch (code) {
case SMP_CMD_PAIRING_REQ:
- reason = SMP_PAIRING_NOTSUPP;
- smp_send_cmd(conn, SMP_CMD_PAIRING_FAIL, 1, &reason);
- l2cap_conn_del(conn->hcon, 0x05);
+ smp_cmd_pairing_req(conn, skb);
break;
case SMP_CMD_PAIRING_FAIL:
break;
case SMP_CMD_PAIRING_RSP:
+ smp_cmd_pairing_rsp(conn, skb);
+ break;
+
+ case SMP_CMD_SECURITY_REQ:
+ smp_cmd_security_req(conn, skb);
+ break;
+
case SMP_CMD_PAIRING_CONFIRM:
+ smp_cmd_pairing_confirm(conn, skb);
+ break;
+
case SMP_CMD_PAIRING_RANDOM:
+ smp_cmd_pairing_random(conn, skb);
+ break;
+
case SMP_CMD_ENCRYPT_INFO:
case SMP_CMD_MASTER_IDENT:
case SMP_CMD_IDENT_INFO:
case SMP_CMD_IDENT_ADDR_INFO:
case SMP_CMD_SIGN_INFO:
- case SMP_CMD_SECURITY_REQ:
default:
BT_DBG("Unknown command code 0x%2.2x", code);
--
1.7.0.4
^ permalink raw reply related
* [PATCH] This patch avoids a crash when org.bluez.Manager GetProperties request is
From: Tommi Keisala @ 2010-10-23 5:33 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <20101022131207.GA29655 () jh-x301>
I made changes requested
^ permalink raw reply
* [PATCH] 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.
From: Tommi Keisala @ 2010-10-23 5:33 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Tommi Keisala
In-Reply-To: <20101022131207.GA29655 () jh-x301>
---
src/manager.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/src/manager.c b/src/manager.c
index aff069c..dd9560f 100644
--- 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);
--
1.7.0.4
^ permalink raw reply related
* Re: (Health) ChannelConnected signal when MDL aborted?
From: Santiago Carot @ 2010-10-23 7:50 UTC (permalink / raw)
To: Elvis Pfützenreuter; +Cc: Jose Antonio Santos Cadenas, linux-bluetooth
In-Reply-To: <295EA951-2357-4B6B-B558-0B3F1D6FFDD9@signove.com>
Hi,
2010/10/22 Elvis Pfützenreuter <epx@signove.com>:
> 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.
>
You are forgetting that there may be more than one health applications
running over HDP at the same time, if one of them creates a data
channel, that data data channel will be exist at MCAP level even if
the initiator abort the connection. If other application wants to
create a new data channel with the same configuration, it may be want
reconnect that data channel avoiding to create another data channel.
It is a best practise wich is recomended in MCAP and best explained in
the Health Device Profile white paper document to reduce the amount of
data interchanged between medical devices (in IEEE/11073-20601
terminology: "agents" and "managers"). Remember that channels may be
shared between applications.
That is the reason why HDP sends out a signal when a data channel has
been created.
Of course, the efficence of the protocol will depend on intelligence
of the programmer of the applications to take advantages of MCAP
reconnections. In any case, you will need to know when a data channel
has been created over the MCL (by you, or by other application).
Regards,
Santiago
^ permalink raw reply
* Re: (Health) ChannelConnected signal when MDL aborted?
From: Santiago Carot @ 2010-10-23 8:00 UTC (permalink / raw)
To: Elvis Pfützenreuter; +Cc: Jose Antonio Santos Cadenas, linux-bluetooth
In-Reply-To: <AANLkTi=65C7HQXQW1Bqbgf4sNY3wbV2CzqoENBc-b04g@mail.gmail.com>
Hi Elvis,
2010/10/23 Santiago Carot <sancane@gmail.com>:
> Hi,
>
> 2010/10/22 Elvis Pfützenreuter <epx@signove.com>:
>> 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.
>>
>
> You are forgetting that there may be more than one health applications
> running over HDP at the same time, if one of them creates a data
> channel, that data data channel will be exist at MCAP level even if
> the initiator abort the connection. If other application wants to
> create a new data channel with the same configuration, it may be want
> reconnect that data channel avoiding to create another data channel.
> It is a best practise wich is recomended in MCAP and best explained in
> the Health Device Profile white paper document to reduce the amount of
> data interchanged between medical devices (in IEEE/11073-20601
> terminology: "agents" and "managers"). Remember that channels may be
> shared between applications.
>
I found the example that I was trying to explain here in page 16 of
the HDP whitepaper, there you can see two applications over HDP
sharing the same data channel (one of them providing support to
diferents device specializations).
> That is the reason why HDP sends out a signal when a data channel has
> been created.
> Of course, the efficence of the protocol will depend on intelligence
> of the programmer of the applications to take advantages of MCAP
> reconnections. In any case, you will need to know when a data channel
> has been created over the MCL (by you, or by other application).
>
> Regards,
>
> Santiago
>
^ permalink raw reply
* Re: (Health) ChannelConnected signal when MDL aborted?
From: Santiago Carot @ 2010-10-23 8:25 UTC (permalink / raw)
To: Elvis Pfützenreuter; +Cc: Jose Antonio Santos Cadenas, linux-bluetooth
In-Reply-To: <AANLkTimvQH4jA=AudrGA7Sj1vAFB3OjocOur2J2=Yy2f@mail.gmail.com>
Hi again,
2010/10/23 Santiago Carot <sancane@gmail.com>:
> Hi Elvis,
>
> 2010/10/23 Santiago Carot <sancane@gmail.com>:
>> Hi,
>>
>> 2010/10/22 Elvis Pfützenreuter <epx@signove.com>:
>>> 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.
>>>
>>
>> You are forgetting that there may be more than one health applications
>> running over HDP at the same time, if one of them creates a data
>> channel, that data data channel will be exist at MCAP level even if
>> the initiator abort the connection. If other application wants to
>> create a new data channel with the same configuration, it may be want
>> reconnect that data channel avoiding to create another data channel.
>> It is a best practise wich is recomended in MCAP and best explained in
>> the Health Device Profile white paper document to reduce the amount of
>> data interchanged between medical devices (in IEEE/11073-20601
>> terminology: "agents" and "managers"). Remember that channels may be
>> shared between applications.
>>
>
> I found the example that I was trying to explain here in page 16 of
> the HDP whitepaper, there you can see two applications over HDP
> sharing the same data channel (one of them providing support to
> diferents device specializations).
>
Well, may be that above reference is not the best because in the
picture applications share the same MDEP too, but I think that it is
enough to see analogies.
>> That is the reason why HDP sends out a signal when a data channel has
>> been created.
>> Of course, the efficence of the protocol will depend on intelligence
>> of the programmer of the applications to take advantages of MCAP
>> reconnections. In any case, you will need to know when a data channel
>> has been created over the MCL (by you, or by other application).
>>
>> Regards,
>>
>> Santiago
>>
>
^ permalink raw reply
* Re: [PATCH 1/1] Fix small coding style issues
From: Johan Hedberg @ 2010-10-23 12:28 UTC (permalink / raw)
To: Elvis Pfützenreuter; +Cc: linux-bluetooth
In-Reply-To: <1287775122-12536-1-git-send-email-epx@signove.com>
Hi Elvis,
On Fri, Oct 22, 2010, Elvis Pfützenreuter wrote:
> 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(-)
Thanks, I've pushed the patch upstream. FWIW, The BlueZ code base
doesn't really enforce the kernel style rule of having braces for all
parts of if-else statements when at least one branch has them.
Johan
^ permalink raw reply
* Re: [PATCH] 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.
From: Johan Hedberg @ 2010-10-23 12:31 UTC (permalink / raw)
To: Tommi Keisala; +Cc: linux-bluetooth
In-Reply-To: <1287811998-20490-2-git-send-email-ext-tommi.keisala@nokia.com>
Hi Tommi,
On Sat, Oct 23, 2010, Tommi Keisala wrote:
> ---
> src/manager.c | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
You seem to have somehow managed to screw up the commit message this
time: everything in the summary line and nothing in the message body.
Anyway, to avoid further feedback rounds (since the patch itself is fine
now) I went ahead and fixed the commit message and pushed your patch
upstream. Thanks.
Johan
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox