Hi,
Right now, we are using only one set of (hardcoded) connection
parameters for LE. I guess the first step to have different
connection parameters classes for different Profiles requirements
(think "Low Latency", "Low Power", etc.) is to have a nice set of
parameters. For that we need to measure and see which are the sweet
spots, This series aims to help the measuring part.
This is an idea so we can easily change the connection parameters
without need for recompiling the kernel side, so tests can be more
practical.
It works, but just lightly tested. This is more to get some feedback
on how to proceed with the LE connection parameters thing.
Cheers,
Vinicius Costa Gomes (3):
Bluetooth: Keep the LE connection parameters in its own structure
Bluetooth: Add LE connection parameters to debugfs
Bluetooth: Add support for setting LE connection parameters via
debugfs
include/net/bluetooth/hci_core.h | 10 ++++++
net/bluetooth/hci_conn.c | 11 +++---
net/bluetooth/hci_event.c | 10 ++++++
net/bluetooth/hci_sysfs.c | 77 ++++++++++++++++++++++++++++++++++++++++
4 files changed, 103 insertions(+), 5 deletions(-)
--
1.7.12.4
Hi Vinicius,
On Mon, Nov 05, 2012 at 02:25:42PM +0100, Vinicius Costa Gomes wrote:
> Only reading the parameters is supported for now.
>
> Signed-off-by: Vinicius Costa Gomes <[email protected]>
> ---
> net/bluetooth/hci_sysfs.c | 34 ++++++++++++++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
>
> diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
> index 55cceee..a9554ec 100644
> --- a/net/bluetooth/hci_sysfs.c
> +++ b/net/bluetooth/hci_sysfs.c
> @@ -532,6 +532,36 @@ static int auto_accept_delay_get(void *data, u64 *val)
> DEFINE_SIMPLE_ATTRIBUTE(auto_accept_delay_fops, auto_accept_delay_get,
> auto_accept_delay_set, "%llu\n");
>
> +
> +static int le_conn_parameters_show(struct seq_file *f, void *p)
> +{
> + struct hci_dev *hdev = f->private;
> + struct le_conn_params *params = &hdev->le_conn_params;
> +
> + hci_dev_lock(hdev);
> +
> + seq_printf(f, "0x%.4x 0x%.4x 0x%.4x 0x%.4x 0x%.4x\n",
BTW: you might want to use format "0x%4.4x" we use in hci_core.c
Best regards
Andrei Emeltchenko
Hi,
On 14:42 Mon 05 Nov, Kim Schulz wrote:
> Den 2012-11-05 14:25, Vinicius Costa Gomes skrev:
> >Hi,
> >
> >Right now, we are using only one set of (hardcoded) connection
> >parameters for LE. I guess the first step to have different
> >connection parameters classes for different Profiles requirements
> >(think "Low Latency", "Low Power", etc.) is to have a nice set of
> >parameters. For that we need to measure and see which are the sweet
> >spots, This series aims to help the measuring part.
> >
> >This is an idea so we can easily change the connection parameters
> >without need for recompiling the kernel side, so tests can be more
> >practical.
> >
> [snip]
>
>
> Good idea to split them out as every single profile defines their
> own. Have you
> concidered what should happen if two LE connections to two different
> profils should
> happen at the same time and the connection parameters are different?
> i.e. should bluez
> maybe have a per-profile set of connection parameters?
The current plan is to have the most restrictive (in terms of latency)
set of parameters to prevail over the others.
> Also make room in the struct for Preferred Connection Paramters (the
> values read
> from the peer DB after connection) as these can differ from the ones
> the profile proposes.
I don't think that this information should be in the kernel side, but thank
you for reminding me of this. About the problem, that could be dealt with
having that as another set of parameters (perhaps with a higher priority).
>
> --
> Kim Schulz
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Cheers,
--
Vinicius
Den 2012-11-05 14:25, Vinicius Costa Gomes skrev:
> Hi,
>
> Right now, we are using only one set of (hardcoded) connection
> parameters for LE. I guess the first step to have different
> connection parameters classes for different Profiles requirements
> (think "Low Latency", "Low Power", etc.) is to have a nice set of
> parameters. For that we need to measure and see which are the sweet
> spots, This series aims to help the measuring part.
>
> This is an idea so we can easily change the connection parameters
> without need for recompiling the kernel side, so tests can be more
> practical.
>
[snip]
Good idea to split them out as every single profile defines their own.
Have you
concidered what should happen if two LE connections to two different
profils should
happen at the same time and the connection parameters are different?
i.e. should bluez
maybe have a per-profile set of connection parameters?
Also make room in the struct for Preferred Connection Paramters (the
values read
from the peer DB after connection) as these can differ from the ones
the profile proposes.
--
Kim Schulz
This will allow for much more pratical measures of the impact of
different connection parameters in different scenarios.
Signed-off-by: Vinicius Costa Gomes <[email protected]>
---
net/bluetooth/hci_sysfs.c | 43 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 43 insertions(+)
diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
index a9554ec..8474d02 100644
--- a/net/bluetooth/hci_sysfs.c
+++ b/net/bluetooth/hci_sysfs.c
@@ -550,6 +550,48 @@ static int le_conn_parameters_show(struct seq_file *f, void *p)
return 0;
}
+static ssize_t le_conn_parameters_write(struct file *filp,
+ const char __user *ubuf,
+ size_t cnt, loff_t *ppos)
+{
+ struct seq_file *seqf = filp->private_data;
+ struct hci_dev *hdev = seqf->private;
+ struct le_conn_params params;
+ char buffer[36];
+ int n;
+
+ /* No partial writes */
+ if (*ppos != 0)
+ return 0;
+
+ /* Format: '0x0000 0x0000 0x0000 0x0000 0x0000' length 35 */
+ if (cnt != 35)
+ return -ENOSPC;
+
+ memset(buffer, 0, sizeof(buffer));
+
+ n = copy_from_user(buffer, ubuf, cnt);
+ if (n < 0)
+ return n;
+
+ memset(¶ms, 0, sizeof(params));
+
+ n = sscanf(buffer, "%hx %hx %hx %hx %hx", ¶ms.scan_interval,
+ ¶ms.scan_window, ¶ms.conn_interval_min,
+ ¶ms.conn_interval_max, ¶ms.supervision_timeout);
+
+ if (n < 5)
+ return -EINVAL;
+
+ hci_dev_lock(hdev);
+
+ memcpy(&hdev->le_conn_params, ¶ms, sizeof(params));
+
+ hci_dev_unlock(hdev);
+
+ return cnt;
+}
+
static int le_conn_parameters_open(struct inode *inode, struct file *file)
{
return single_open(file, le_conn_parameters_show, inode->i_private);
@@ -557,6 +599,7 @@ static int le_conn_parameters_open(struct inode *inode, struct file *file)
static const struct file_operations le_conn_parameters_fops = {
.open = le_conn_parameters_open,
+ .write = le_conn_parameters_write,
.read = seq_read,
.llseek = seq_lseek,
.release = single_release,
--
1.7.12.4
Only reading the parameters is supported for now.
Signed-off-by: Vinicius Costa Gomes <[email protected]>
---
net/bluetooth/hci_sysfs.c | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
index 55cceee..a9554ec 100644
--- a/net/bluetooth/hci_sysfs.c
+++ b/net/bluetooth/hci_sysfs.c
@@ -532,6 +532,36 @@ static int auto_accept_delay_get(void *data, u64 *val)
DEFINE_SIMPLE_ATTRIBUTE(auto_accept_delay_fops, auto_accept_delay_get,
auto_accept_delay_set, "%llu\n");
+
+static int le_conn_parameters_show(struct seq_file *f, void *p)
+{
+ struct hci_dev *hdev = f->private;
+ struct le_conn_params *params = &hdev->le_conn_params;
+
+ hci_dev_lock(hdev);
+
+ seq_printf(f, "0x%.4x 0x%.4x 0x%.4x 0x%.4x 0x%.4x\n",
+ params->scan_interval, params->scan_window,
+ params->conn_interval_min, params->conn_interval_max,
+ params->supervision_timeout);
+
+ hci_dev_unlock(hdev);
+
+ return 0;
+}
+
+static int le_conn_parameters_open(struct inode *inode, struct file *file)
+{
+ return single_open(file, le_conn_parameters_show, inode->i_private);
+}
+
+static const struct file_operations le_conn_parameters_fops = {
+ .open = le_conn_parameters_open,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = single_release,
+};
+
void hci_init_sysfs(struct hci_dev *hdev)
{
struct device *dev = &hdev->dev;
@@ -573,6 +603,10 @@ int hci_add_sysfs(struct hci_dev *hdev)
debugfs_create_file("auto_accept_delay", 0444, hdev->debugfs, hdev,
&auto_accept_delay_fops);
+
+ debugfs_create_file("le_conn_parameters", 0644, hdev->debugfs, hdev,
+ &le_conn_parameters_fops);
+
return 0;
}
--
1.7.12.4
This will allow for easier parameterization of these fields. This is
the first step for allowing to change the parameters during runtime.
Signed-off-by: Vinicius Costa Gomes <[email protected]>
---
include/net/bluetooth/hci_core.h | 10 ++++++++++
net/bluetooth/hci_conn.c | 11 ++++++-----
net/bluetooth/hci_event.c | 10 ++++++++++
3 files changed, 26 insertions(+), 5 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index ce6dbeb..f477d43 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -123,6 +123,14 @@ struct le_scan_params {
int timeout;
};
+struct le_conn_params {
+ u16 scan_interval;
+ u16 scan_window;
+ u16 conn_interval_min;
+ u16 conn_interval_max;
+ u16 supervision_timeout;
+};
+
#define HCI_MAX_SHORT_NAME_LENGTH 10
struct amp_assoc {
@@ -278,6 +286,8 @@ struct hci_dev {
struct work_struct le_scan;
struct le_scan_params le_scan_params;
+ struct le_conn_params le_conn_params;
+
__s8 adv_tx_power;
int (*open)(struct hci_dev *hdev);
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 25bfce0..d8ff5c0 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -35,6 +35,7 @@ static void hci_le_create_connection(struct hci_conn *conn)
{
struct hci_dev *hdev = conn->hdev;
struct hci_cp_le_create_conn cp;
+ struct le_conn_params *params = &hdev->le_conn_params;
conn->state = BT_CONNECT;
conn->out = true;
@@ -42,13 +43,13 @@ static void hci_le_create_connection(struct hci_conn *conn)
conn->sec_level = BT_SECURITY_LOW;
memset(&cp, 0, sizeof(cp));
- cp.scan_interval = __constant_cpu_to_le16(0x0060);
- cp.scan_window = __constant_cpu_to_le16(0x0030);
+ cp.scan_interval = __cpu_to_le16(params->scan_interval);
+ cp.scan_window = __cpu_to_le16(params->scan_window);
bacpy(&cp.peer_addr, &conn->dst);
cp.peer_addr_type = conn->dst_type;
- cp.conn_interval_min = __constant_cpu_to_le16(0x0028);
- cp.conn_interval_max = __constant_cpu_to_le16(0x0038);
- cp.supervision_timeout = __constant_cpu_to_le16(0x002a);
+ cp.conn_interval_min = __cpu_to_le16(params->conn_interval_min);
+ cp.conn_interval_max = __cpu_to_le16(params->conn_interval_max);
+ cp.supervision_timeout = __cpu_to_le16(params->supervision_timeout);
cp.min_ce_len = __constant_cpu_to_le16(0x0000);
cp.max_ce_len = __constant_cpu_to_le16(0x0000);
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index c08ac7c..3675dbd 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1298,6 +1298,16 @@ static void hci_cc_write_le_host_supported(struct hci_dev *hdev,
hdev->host_features[0] |= LMP_HOST_LE_BREDR;
else
hdev->host_features[0] &= ~LMP_HOST_LE_BREDR;
+
+ if (sent->le || sent->simul) {
+ struct le_conn_params *params = &hdev->le_conn_params;
+
+ params->scan_interval = 0x0060;
+ params->scan_window = 0x0030;
+ params->conn_interval_min = 0x0028;
+ params->conn_interval_max = 0x0038;
+ params->supervision_timeout = 0x002a;
+ }
}
if (test_bit(HCI_MGMT, &hdev->dev_flags) &&
--
1.7.12.4