[PATCH v2] ath6kl: Implement support for listen interval from userspace

[PATCH v2] ath6kl: Implement support for listen interval from userspace

[Rishi Panjwani]

The patch allows modification of listen interval thereby allowing change
in sleep/awake cycle causing change in power consumption numbers.

Rishi Panjwani (1):
  ath6kl: Implement support for listen interval from userspace

 drivers/net/wireless/ath/ath6kl/debug.c |   60 +++++++++++++++++++++++++++++++
 1 files changed, 60 insertions(+), 0 deletions(-)

[PATCH v2] ath6kl: Implement support for listen interval from userspace

[Rishi Panjwani]

In order to allow user space based control of listen interval, we use
available debugfs infrastructure. Listen interval implies how frequently
we want the WLAN chip to wake up and synchronize the beacons in case it
is in sleep mode. The feature has been added for testing purposes. The
command requires two parameters in the following order:
1) listen_interval_time
2) listen_interval_beacons

The user has to write the listen interval_time (in msecs) and
listen_interval_beacons (in no. of beacons) to the listen_interval file in
ath6kl debug directory.

Example:

echo "30 1" > listen_interval

Signed-off-by: Rishi Panjwani <rpanjwan@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath6kl/debug.c |   60 +++++++++++++++++++++++++++++++
 1 files changed, 60 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/ath/ath6kl/debug.c b/drivers/net/wireless/ath/ath6kl/debug.c
index 3eaa291..b589ab6 100644
--- a/drivers/net/wireless/ath/ath6kl/debug.c
+++ b/drivers/net/wireless/ath/ath6kl/debug.c
@@ -1489,6 +1489,66 @@ static const struct file_operations fops_bgscan_int = {
 	.llseek = default_llseek,
 };
 
+static ssize_t ath6kl_listen_int_write(struct file *file,
+						const char __user *user_buf,
+						size_t count, loff_t *ppos)
+{
+	struct ath6kl *ar = file->private_data;
+	u16 listen_int_t, listen_int_b;
+	char buf[32];
+	char *sptr, *token;
+	ssize_t len;
+
+	len = min(count, sizeof(buf) - 1);
+	if (copy_from_user(buf, user_buf, len))
+		return -EFAULT;
+
+	buf[len] = '\0';
+	sptr = buf;
+
+	token = strsep(&sptr, " ");
+	if (!token)
+		return -EINVAL;
+	if (kstrtou16(token, 0, &listen_int_t) ||
+		kstrtou16(sptr, 0, &listen_int_b))
+		return -EINVAL;
+
+
+	if ((listen_int_t >= 15) && (listen_int_t <= 5000) &&
+		(listen_int_b >= 1) && (listen_int_b <= 50)) {
+		ar->listen_intvl_t = listen_int_t;
+		ar->listen_intvl_b = listen_int_b;
+		ath6kl_wmi_listeninterval_cmd(ar->wmi, ar->listen_intvl_t,
+							ar->listen_intvl_b);
+	} else {
+		return -EINVAL;
+	}
+
+	return count;
+}
+
+static ssize_t ath6kl_listen_int_read(struct file *file,
+						char __user *user_buf,
+						size_t count, loff_t *ppos)
+{
+	struct ath6kl *ar = file->private_data;
+	char buf[16];
+	int len;
+
+	len = snprintf(buf, sizeof(buf), "%u %u\n", ar->listen_intvl_t,
+					ar->listen_intvl_b);
+
+	return simple_read_from_buffer(user_buf, count, ppos, buf, len);
+}
+
+static const struct file_operations fops_listen_int = {
+	.read = ath6kl_listen_int_read,
+	.write = ath6kl_listen_int_write,
+	.open = ath6kl_debugfs_open,
+	.owner = THIS_MODULE,
+	.llseek = default_llseek,
+};
+
 int ath6kl_debug_init(struct ath6kl *ar)
 {
 	ar->debug.fwlog_buf.buf = vmalloc(ATH6KL_FWLOG_SIZE);
-- 
1.7.0.4

Re: [PATCH v2] ath6kl: Implement support for listen interval from userspace

[Kalle Valo]

On 10/21/2011 01:35 AM, Rishi Panjwani wrote:
> In order to allow user space based control of listen interval, we use
> available debugfs infrastructure. Listen interval implies how frequently
> we want the WLAN chip to wake up and synchronize the beacons in case it
> is in sleep mode. The feature has been added for testing purposes. The
> command requires two parameters in the following order:

An empty line here.

> 1) listen_interval_time
> 2) listen_interval_beacons
> 
> The user has to write the listen interval_time (in msecs) and
> listen_interval_beacons (in no. of beacons) to the listen_interval file in
> ath6kl debug directory.
> 
> Example:
> 
> echo "30 1" > listen_interval

[...]

> +	token = strsep(&sptr, " ");
> +	if (!token)
> +		return -EINVAL;
> +	if (kstrtou16(token, 0, &listen_int_t) ||
> +		kstrtou16(sptr, 0, &listen_int_b))
> +		return -EINVAL;

Just to improve readability separate these to two if clauses:

if (kstrtou16(token, 0, &listen_int_t))
	return -EINVAL;

if (kstrtou16(sptr, 0, &listen_int_b))
	return -EINVAL;

> +	if ((listen_int_t >= 15) && (listen_int_t <= 5000) &&
> +		(listen_int_b >= 1) && (listen_int_b <= 50)) {
> +		ar->listen_intvl_t = listen_int_t;
> +		ar->listen_intvl_b = listen_int_b;
> +		ath6kl_wmi_listeninterval_cmd(ar->wmi, ar->listen_intvl_t,
> +							ar->listen_intvl_b);
> +	} else {
> +		return -EINVAL;
> +	}

The style in kernel usually is that we handle the errors in the if
blocks and the normal code flow is without any indentation. Easier to
follow code path that way. So you could change this to:

if ((listen_int_t < 15) || (listen_int_t > 5000))
	return -EINVAL;

if ((listen_int_b < 1) > (listen_int_b > 50))
	return -EINVAL;

ar->listen_intvl_t = listen_int_t;
ar->listen_intvl_b = listen_int_b;

ath6kl_wmi_listeninterval_cmd(ar->wmi, ar->listen_intvl_t,
				ar->listen_intvl_b);

A lot easier to read that way.

Kalle