[PATCH 1/2] wl12xx: add dynamic_ps_timeout debugfs entry

[PATCH 1/2] wl12xx: add dynamic_ps_timeout debugfs entry

[Eliad Peller]

Add debugfs entry to control the dynamic ps default
timeout (value in ms).

Signed-off-by: Eliad Peller <eliad@wizery.com>
---
 drivers/net/wireless/wl12xx/debugfs.c |   28 ++++++++++++++++++++++++++++
 1 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/debugfs.c b/drivers/net/wireless/wl12xx/debugfs.c
index 3999fd5..271dac0 100644
--- a/drivers/net/wireless/wl12xx/debugfs.c
+++ b/drivers/net/wireless/wl12xx/debugfs.c
@@ -311,6 +311,33 @@ static const struct file_operations start_recovery_ops = {
 	.llseek = default_llseek,
 };
 
+static ssize_t dynamic_ps_timeout_write(struct file *file,
+				    const char __user *user_buf,
+				    size_t count, loff_t *ppos)
+{
+	struct wl1271 *wl = file->private_data;
+	long value;
+	int ret;
+
+	ret = kstrtol_from_user(user_buf, count, 10, &value);
+	if (ret < 0) {
+		wl1271_warning("illegal value in dynamic_ps");
+		return -EINVAL;
+	}
+
+	mutex_lock(&wl->mutex);
+	ieee80211_set_dyn_ps_timeout(wl->vif, value);
+	mutex_unlock(&wl->mutex);
+
+	return count;
+}
+
+static const struct file_operations dynamic_ps_timeout_ops = {
+	.write = dynamic_ps_timeout_write,
+	.open = wl1271_open_file_generic,
+	.llseek = default_llseek,
+};
+
 static ssize_t driver_state_read(struct file *file, char __user *user_buf,
 				 size_t count, loff_t *ppos)
 {
@@ -769,6 +796,7 @@ static int wl1271_debugfs_add_files(struct wl1271 *wl,
 
 	DEBUGFS_ADD(gpio_power, rootdir);
 	DEBUGFS_ADD(start_recovery, rootdir);
+	DEBUGFS_ADD(dynamic_ps_timeout, rootdir);
 	DEBUGFS_ADD(driver_state, rootdir);
 	DEBUGFS_ADD(dtim_interval, rootdir);
 	DEBUGFS_ADD(beacon_interval, rootdir);
-- 
1.7.6.401.g6a319

[PATCH 2/2] wl12xx: handle CHANGE_AUTO_MODE_TIMEOUT_EVENT_ID

[Eliad Peller]

The fw sends the CHANGE_AUTO_MODE_TIMEOUT_EVENT_ID event
when it wants the driver to increase the dynamic ps (auto-mode)
timeout (or restore the original timeout).

unmask CHANGE_AUTO_MODE_TIMEOUT_EVENT_ID, and increase/restore
the dynamic ps default timeout upon getting this event.

Signed-off-by: Eliad Peller <eliad@wizery.com>
---
 drivers/net/wireless/wl12xx/boot.c  |    1 +
 drivers/net/wireless/wl12xx/event.c |    8 ++++++++
 2 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/boot.c b/drivers/net/wireless/wl12xx/boot.c
index 6d5664b..13798a3 100644
--- a/drivers/net/wireless/wl12xx/boot.c
+++ b/drivers/net/wireless/wl12xx/boot.c
@@ -496,6 +496,7 @@ static int wl1271_boot_run_firmware(struct wl1271 *wl)
 		RSSI_SNR_TRIGGER_0_EVENT_ID |
 		PSPOLL_DELIVERY_FAILURE_EVENT_ID |
 		SOFT_GEMINI_SENSE_EVENT_ID |
+		CHANGE_AUTO_MODE_TIMEOUT_EVENT_ID |
 		PERIODIC_SCAN_REPORT_EVENT_ID |
 		PERIODIC_SCAN_COMPLETE_EVENT_ID |
 		DUMMY_PACKET_EVENT_ID |
diff --git a/drivers/net/wireless/wl12xx/event.c b/drivers/net/wireless/wl12xx/event.c
index c73fe4c..79ff3bb 100644
--- a/drivers/net/wireless/wl12xx/event.c
+++ b/drivers/net/wireless/wl12xx/event.c
@@ -258,6 +258,14 @@ static int wl1271_event_process(struct wl1271 *wl, struct event_mailbox *mbox)
 		wl12xx_event_soft_gemini_sense(wl,
 					       mbox->soft_gemini_sense_info);
 
+	if (vector & CHANGE_AUTO_MODE_TIMEOUT_EVENT_ID &&
+	    wl->bss_type == BSS_TYPE_STA_BSS) {
+		int timeout = -1;
+		if (mbox->change_auto_mode_timeout)
+			timeout = 500;
+		ieee80211_set_dyn_ps_timeout(wl->vif, timeout);
+	}
+
 	/*
 	 * The BSS_LOSE_EVENT_ID is only needed while psm (and hence beacon
 	 * filtering) is enabled. Without PSM, the stack will receive all
-- 
1.7.6.401.g6a319

Re: [PATCH 1/2] wl12xx: add dynamic_ps_timeout debugfs entry

[Luciano Coelho]

Hi Eliad,

On Sun, 2011-09-18 at 15:09 +0300, Eliad Peller wrote: 
> Add debugfs entry to control the dynamic ps default
> timeout (value in ms).
> 
> Signed-off-by: Eliad Peller <eliad@wizery.com>
> ---

Please mention the dependency to the mac80211 changes in this cases.
This was easy to figure out, but explicitly mentioning it makes my life
a bit easier. ;)

-- 
Cheers,
Luca.

Re: [PATCH 1/2] wl12xx: add dynamic_ps_timeout debugfs entry

[Luciano Coelho]

On Sun, 2011-09-18 at 15:09 +0300, Eliad Peller wrote: 
> Add debugfs entry to control the dynamic ps default
> timeout (value in ms).
> 
> Signed-off-by: Eliad Peller <eliad@wizery.com>
> ---

Another thing, how are you planning to use this debugfs entry? Just for
testing, right? And if yes, I think it shouldn't, in the end, be a
debugfs entry, but something that the driver figures out by itself.  We
really cannot have non-debugging values settable in debugfs.

-- 
Cheers,
Luca.

Re: [PATCH 2/2] wl12xx: handle CHANGE_AUTO_MODE_TIMEOUT_EVENT_ID

[Luciano Coelho]

On Sun, 2011-09-18 at 15:09 +0300, Eliad Peller wrote: 
> The fw sends the CHANGE_AUTO_MODE_TIMEOUT_EVENT_ID event
> when it wants the driver to increase the dynamic ps (auto-mode)
> timeout (or restore the original timeout).
> 
> unmask CHANGE_AUTO_MODE_TIMEOUT_EVENT_ID, and increase/restore
> the dynamic ps default timeout upon getting this event.
> 
> Signed-off-by: Eliad Peller <eliad@wizery.com>
> ---

Okay, now I see you're doing it automatically here (I should read all
the series before commenting :P).  Anyway, do we really need the debugfs
stuff? It's okay to make it readable, but I still don't see why we need
to write.


> diff --git a/drivers/net/wireless/wl12xx/event.c b/drivers/net/wireless/wl12xx/event.c
> index c73fe4c..79ff3bb 100644
> --- a/drivers/net/wireless/wl12xx/event.c
> +++ b/drivers/net/wireless/wl12xx/event.c
> @@ -258,6 +258,14 @@ static int wl1271_event_process(struct wl1271 *wl, struct event_mailbox *mbox)
>  		wl12xx_event_soft_gemini_sense(wl,
>  					       mbox->soft_gemini_sense_info);
>  
> +	if (vector & CHANGE_AUTO_MODE_TIMEOUT_EVENT_ID &&
> +	    wl->bss_type == BSS_TYPE_STA_BSS) {
> +		int timeout = -1;
> +		if (mbox->change_auto_mode_timeout)
> +			timeout = 500;
> +		ieee80211_set_dyn_ps_timeout(wl->vif, timeout);
> +	}
> +
>  	/*
>  	 * The BSS_LOSE_EVENT_ID is only needed while psm (and hence beacon
>  	 * filtering) is enabled. Without PSM, the stack will receive all

Any more details on why you chose 500? Should this be part of conf or a
macro to avoid magic numbers?

-- 
Cheers,
Luca.

Re: [PATCH 2/2] wl12xx: handle CHANGE_AUTO_MODE_TIMEOUT_EVENT_ID

[Eliad Peller]

On Mon, Sep 19, 2011 at 8:21 AM, Luciano Coelho <coelho@ti.com> wrote:
> On Sun, 2011-09-18 at 15:09 +0300, Eliad Peller wrote:
>> The fw sends the CHANGE_AUTO_MODE_TIMEOUT_EVENT_ID event
>> when it wants the driver to increase the dynamic ps (auto-mode)
>> timeout (or restore the original timeout).
>>
>> unmask CHANGE_AUTO_MODE_TIMEOUT_EVENT_ID, and increase/restore
>> the dynamic ps default timeout upon getting this event.
>>
>> Signed-off-by: Eliad Peller <eliad@wizery.com>
>> ---
>
> Okay, now I see you're doing it automatically here (I should read all
> the series before commenting :P).  Anyway, do we really need the debugfs
> stuff? It's okay to make it readable, but I still don't see why we need
> to write.
>
i used the debugfs entry for debugging.
i don't mind dropping it.

>
>> diff --git a/drivers/net/wireless/wl12xx/event.c b/drivers/net/wireless/wl12xx/event.c
>> index c73fe4c..79ff3bb 100644
>> --- a/drivers/net/wireless/wl12xx/event.c
>> +++ b/drivers/net/wireless/wl12xx/event.c
>> @@ -258,6 +258,14 @@ static int wl1271_event_process(struct wl1271 *wl, struct event_mailbox *mbox)
>>               wl12xx_event_soft_gemini_sense(wl,
>>                                              mbox->soft_gemini_sense_info);
>>
>> +     if (vector & CHANGE_AUTO_MODE_TIMEOUT_EVENT_ID &&
>> +         wl->bss_type == BSS_TYPE_STA_BSS) {
>> +             int timeout = -1;
>> +             if (mbox->change_auto_mode_timeout)
>> +                     timeout = 500;
>> +             ieee80211_set_dyn_ps_timeout(wl->vif, timeout);
>> +     }
>> +
>>       /*
>>        * The BSS_LOSE_EVENT_ID is only needed while psm (and hence beacon
>>        * filtering) is enabled. Without PSM, the stack will receive all
>
> Any more details on why you chose 500? Should this be part of conf or a
> macro to avoid magic numbers?
>
500 was the magic number i got from the coex guys :)
i'll add it to the conf.

thanks for the review,
Eliad.

Re: [PATCH 1/2] wl12xx: add dynamic_ps_timeout debugfs entry

[Luciano Coelho]

On Sun, 2011-09-18 at 15:09 +0300, Eliad Peller wrote: 
> Add debugfs entry to control the dynamic ps default
> timeout (value in ms).
> 
> Signed-off-by: Eliad Peller <eliad@wizery.com>
> ---

Dropped this series, since the mac80211 change that supports this was
not accepted.  We'll get all this dynamic PS stuff in the firmware soon
anyway.

-- 
Cheers,
Luca.