Power saving features for iwl4965

Power saving features for iwl4965

[Tino Keitel]

Hi,

up to 2.6.38, the iwl4965 driver was able to use power saving features
by invoking "iwconfig <interface> power on", at least when unsetting
the broken_powersave variable.

2.6.39 introduced a new iwlegacy driver for iwl4965 hardware, which was
missing power saving.  I was able to port the 2.6.38 driver up to
kernel 3.1, but it got too complicated for me with 3.2. 

Are there any plans to fix this power usage regression at some point in
the iwlegacy driver? I think there are still a lot of iwl4965 users out
there.

Please include my address in any replies, as I'm not subscribed.

Regards,
Tino

Re: Power saving features for iwl4965

[Johannes Berg]

On 4/18/2012 1:35 PM, Tino Keitel wrote:
> up to 2.6.38, the iwl4965 driver was able to use power saving features
> by invoking "iwconfig<interface>  power on", at least when unsetting
> the broken_powersave variable.

This isn't quite correct. Yes, power saving was enabled there, but 
relatively frequently it caused the device to crash. Therefore, it was 
disabled as a stability fix, not to remove powersaving from you :)

johannes

Re: Power saving features for iwl4965

[tino]

------- Original message -------
> From: Johannes Berg <johannes@sipsolutions.net>
> To: tino.keitel@tikei.de
> Cc: linux-wireless@vger.kernel.org
> Sent: 19.4.'12,  16:25
>
> On 4/18/2012 1:35 PM, Tino Keitel wrote:
>> up to 2.6.38, the iwl4965 driver was able to use power saving features
>> by invoking "iwconfig<interface>  power on", at least when unsetting
>> the broken_powersave variable.
>
> This isn't quite correct. Yes, power saving was enabled there, but 
> relatively frequently it caused the device to crash. Therefore, it was 
> disabled as a stability fix, not to remove powersaving from you :)

Hi,

I was aware that it was disabled because of stability issues in certain 
setups. I never experienced any issue when running with power saving 
enabled, though. Other people did the same and even suggested patches to 
clear broken_powersave with a module parameter.

Regards,
Tino

Re: Power saving features for iwl4965

[Stanislaw Gruszka]

On Wed, Apr 18, 2012 at 10:35:43PM +0200, Tino Keitel wrote:
> up to 2.6.38, the iwl4965 driver was able to use power saving features
> by invoking "iwconfig <interface> power on", at least when unsetting
> the broken_powersave variable.
> 
> 2.6.39 introduced a new iwlegacy driver for iwl4965 hardware, which was
> missing power saving.  I was able to port the 2.6.38 driver up to
> kernel 3.1, but it got too complicated for me with 3.2. 
> 
> Are there any plans to fix this power usage regression at some point in
> the iwlegacy driver? I think there are still a lot of iwl4965 users out
> there.
Yes, adding PS support is on my iwlegacy TODO list.

Stanislaw

Re: Power saving features for iwl4965

[Tino Keitel]

On Wed, Apr 25, 2012 at 14:25:55 +0200, Stanislaw Gruszka wrote:
> On Wed, Apr 18, 2012 at 10:35:43PM +0200, Tino Keitel wrote:
> > up to 2.6.38, the iwl4965 driver was able to use power saving features
> > by invoking "iwconfig <interface> power on", at least when unsetting
> > the broken_powersave variable.
> > 
> > 2.6.39 introduced a new iwlegacy driver for iwl4965 hardware, which was
> > missing power saving.  I was able to port the 2.6.38 driver up to
> > kernel 3.1, but it got too complicated for me with 3.2. 
> > 
> > Are there any plans to fix this power usage regression at some point in
> > the iwlegacy driver? I think there are still a lot of iwl4965 users out
> > there.
> Yes, adding PS support is on my iwlegacy TODO list.

Thanks, I'd be glad if I could help, testing etc.

Regards,
Tino

Re: Power saving features for iwl4965

[Tino Keitel]

On Thu, May 03, 2012 at 20:28:53 +0200, Tino Keitel wrote:
> On Wed, Apr 25, 2012 at 14:25:55 +0200, Stanislaw Gruszka wrote:
> > On Wed, Apr 18, 2012 at 10:35:43PM +0200, Tino Keitel wrote:
> > > up to 2.6.38, the iwl4965 driver was able to use power saving features
> > > by invoking "iwconfig <interface> power on", at least when unsetting
> > > the broken_powersave variable.
> > > 
> > > 2.6.39 introduced a new iwlegacy driver for iwl4965 hardware, which was
> > > missing power saving.  I was able to port the 2.6.38 driver up to
> > > kernel 3.1, but it got too complicated for me with 3.2. 
> > > 
> > > Are there any plans to fix this power usage regression at some point in
> > > the iwlegacy driver? I think there are still a lot of iwl4965 users out
> > > there.
> > Yes, adding PS support is on my iwlegacy TODO list.
> 
> Thanks, I'd be glad if I could help, testing etc.

Hi Stanislaw,

is there any progress regarding this? If not, I could try to implement
it in the current kernel.  However, I don't have any knowlegde about
hardware related features.  If you have, maybe I could use your
knowledge to do it myself.

Regards,
Tino

Re: Power saving features for iwl4965

[Stanislaw Gruszka]

Hi

On Wed, Dec 26, 2012 at 07:54:25PM +0100, Tino Keitel wrote:
> On Thu, May 03, 2012 at 20:28:53 +0200, Tino Keitel wrote:
> > On Wed, Apr 25, 2012 at 14:25:55 +0200, Stanislaw Gruszka wrote:
> > > On Wed, Apr 18, 2012 at 10:35:43PM +0200, Tino Keitel wrote:
> > > > up to 2.6.38, the iwl4965 driver was able to use power saving features
> > > > by invoking "iwconfig <interface> power on", at least when unsetting
> > > > the broken_powersave variable.
> > > > 
> > > > 2.6.39 introduced a new iwlegacy driver for iwl4965 hardware, which was
> > > > missing power saving.  I was able to port the 2.6.38 driver up to
> > > > kernel 3.1, but it got too complicated for me with 3.2. 
> > > > 
> > > > Are there any plans to fix this power usage regression at some point in
> > > > the iwlegacy driver? I think there are still a lot of iwl4965 users out
> > > > there.
> > > Yes, adding PS support is on my iwlegacy TODO list.
> > 
> > Thanks, I'd be glad if I could help, testing etc.
> 
> Hi Stanislaw,
> 
> is there any progress regarding this? If not, I could try to implement
> it in the current kernel.  However, I don't have any knowlegde about
> hardware related features.  If you have, maybe I could use your
> knowledge to do it myself.

I posted patch here
http://marc.info/?l=linux-wireless&m=135601033021616&w=2

But I did not review code regarding power save to catch possible
problems. Testing are bug reporting are welcome ...

Thanks
Stanislaw

Re: Power saving features for iwl4965

[Pedro Francisco]

On Mon, Jan 7, 2013 at 11:08 AM, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
> Hi
>
> On Wed, Dec 26, 2012 at 07:54:25PM +0100, Tino Keitel wrote:
>>
>> Hi Stanislaw,
>>
>> is there any progress regarding this? If not, I could try to implement
>> it in the current kernel.  However, I don't have any knowlegde about
>> hardware related features.  If you have, maybe I could use your
>> knowledge to do it myself.
>
> I posted patch here
> http://marc.info/?l=linux-wireless&m=135601033021616&w=2
>
> But I did not review code regarding power save to catch possible
> problems. Testing are bug reporting are welcome ...

Is the firmware crashing only after suspend? Or 'always'? (iwl3945)

Re: Power saving features for iwl4965

[Stanislaw Gruszka]

On Tue, Jan 08, 2013 at 01:48:01AM +0000, Pedro Francisco wrote:
> On Mon, Jan 7, 2013 at 11:08 AM, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
> > Hi
> >
> > On Wed, Dec 26, 2012 at 07:54:25PM +0100, Tino Keitel wrote:
> >>
> >> Hi Stanislaw,
> >>
> >> is there any progress regarding this? If not, I could try to implement
> >> it in the current kernel.  However, I don't have any knowlegde about
> >> hardware related features.  If you have, maybe I could use your
> >> knowledge to do it myself.
> >
> > I posted patch here
> > http://marc.info/?l=linux-wireless&m=135601033021616&w=2
> >
> > But I did not review code regarding power save to catch possible
> > problems. Testing are bug reporting are welcome ...
> 
> Is the firmware crashing only after suspend? Or 'always'? (iwl3945)

I don't have 3945 crashes on my local machine when I enable PS. The only
documented issue on 3945 I found is this bug:
http://bugzilla.intellinuxwireless.org/show_bug.cgi?id=2125
It stands that problem happen just after associate.

BTW: PS is still disabled by default, to test it need to be enabled by
iw or iwconfig tool.

Stanislaw

Re: Power saving features for iwl4965

[Tino Keitel]

On Mon, Jan 07, 2013 at 12:08:00 +0100, Stanislaw Gruszka wrote:

[...]

> I posted patch here
> http://marc.info/?l=linux-wireless&m=135601033021616&w=2
> 
> But I did not review code regarding power save to catch possible
> problems. Testing are bug reporting are welcome ...

Hi,

the patch is surprisingly small. To me it looks like it only contains
the code to make "iwconfig wlan0 power on" work, the actual power
management is missing.

I just did some tests using powertop to see if I'm right. With the old
pre-iwlegacy driver, the difference between "iwconfig wlan0 power on"
and "...  power off" is more then 0,8W, which is ~10% of the total idle
power usage of my X61s with dimmed screen.  With a current kernel and
your patch, I can't measure a difference between "iwconfig wlan0 power
on" and "...  power off".  To me it seems that the patch is pretty
useless, at least on 4965AGN hardware.

It would be really nice to have proper power management in a current
kernel, as the laptop gets noticeably hotter with the current iwlegacy
driver.  That's why I still use a 3.1.10 kernel with an old
forward-ported iwlagn driver.

Regards,
Tino

Re: Power saving features for iwl4965

[Stanislaw Gruszka]

On Mon, Jun 03, 2013 at 10:52:39AM +0200, Tino Keitel wrote:
> On Mon, Jan 07, 2013 at 12:08:00 +0100, Stanislaw Gruszka wrote:
> 
> [...]
> 
> > I posted patch here
> > http://marc.info/?l=linux-wireless&m=135601033021616&w=2
> > 
> > But I did not review code regarding power save to catch possible
> > problems. Testing are bug reporting are welcome ...
> 
> Hi,
> 
> the patch is surprisingly small. To me it looks like it only contains
> the code to make "iwconfig wlan0 power on" work, the actual power
> management is missing.
> 
> I just did some tests using powertop to see if I'm right. With the old
> pre-iwlegacy driver, the difference between "iwconfig wlan0 power on"
> and "...  power off" is more then 0,8W, which is ~10% of the total idle
> power usage of my X61s with dimmed screen.  With a current kernel and
> your patch, I can't measure a difference between "iwconfig wlan0 power
> on" and "...  power off".  To me it seems that the patch is pretty
> useless, at least on 4965AGN hardware.

Yes, we do not send any command to put hardware in sleep mode when 
mac80211 enable PS.

> It would be really nice to have proper power management in a current
> kernel, as the laptop gets noticeably hotter with the current iwlegacy
> driver.  That's why I still use a 3.1.10 kernel with an old
> forward-ported iwlagn driver.

Could you try this experimental patch?

diff --git a/drivers/net/wireless/iwlegacy/commands.h b/drivers/net/wireless/iwlegacy/commands.h
index 3b6c994..baf3ae7 100644
--- a/drivers/net/wireless/iwlegacy/commands.h
+++ b/drivers/net/wireless/iwlegacy/commands.h
@@ -2278,7 +2278,8 @@ struct il_spectrum_notification {
  */
 #define IL_POWER_VEC_SIZE 5
 
-#define IL_POWER_DRIVER_ALLOW_SLEEP_MSK	cpu_to_le16(BIT(0))
+#define IL_POWER_DRIVER_ALLOW_SLEEP_MSK		cpu_to_le16(BIT(0))
+#define IL_POWER_SLEEP_OVER_DTIM_MSK		cpu_to_le16(BIT(2))
 #define IL_POWER_PCI_PM_MSK			cpu_to_le16(BIT(3))
 
 struct il3945_powertable_cmd {
diff --git a/drivers/net/wireless/iwlegacy/common.c b/drivers/net/wireless/iwlegacy/common.c
index 592d0aa..07769ae 100644
--- a/drivers/net/wireless/iwlegacy/common.c
+++ b/drivers/net/wireless/iwlegacy/common.c
@@ -1079,29 +1079,80 @@ EXPORT_SYMBOL(il_get_channel_info);
  * Setting power level allows the card to go to sleep when not busy.
  *
  * We calculate a sleep command based on the required latency, which
- * we get from mac80211. In order to handle thermal throttling, we can
- * also use pre-defined power levels.
+ * we get from mac80211.
  */
 
-/*
- * This defines the old power levels. They are still used by default
- * (level 1) and for thermal throttle (levels 3 through 5)
- */
-
-struct il_power_vec_entry {
-	struct il_powertable_cmd cmd;
-	u8 no_dtim;		/* number of skip dtim */
-};
+#define SLP_VEC(X0, X1, X2, X3, X4) {cpu_to_le32(X0), \
+                                     cpu_to_le32(X1), \
+                                     cpu_to_le32(X2), \
+                                     cpu_to_le32(X3), \
+                                     cpu_to_le32(X4)}
 
 static void
-il_power_sleep_cam_cmd(struct il_priv *il, struct il_powertable_cmd *cmd)
+il_build_powertable_cmd(struct il_priv *il, struct il_powertable_cmd *cmd)
 {
+	const __le32 interval[3][IL_POWER_VEC_SIZE] = {
+		SLP_VEC(2, 2, 4, 6, 0xFF),
+		SLP_VEC(2, 4, 7, 10, 10),
+ 		SLP_VEC(4, 7, 10, 10, 0xFF)
+	};
+	int i, dtim_period, no_dtim;
+	u32 max_sleep;
+	bool skip;
+
 	memset(cmd, 0, sizeof(*cmd));
 
 	if (il->power_data.pci_pm)
 		cmd->flags |= IL_POWER_PCI_PM_MSK;
 
-	D_POWER("Sleep command for CAM\n");
+	/* if no Power Save, we are done */
+	if (il->power_data.ps_disabled)
+		return;
+
+	cmd->flags = IL_POWER_DRIVER_ALLOW_SLEEP_MSK;
+	cmd->keep_alive_seconds = 0;
+	cmd->debug_flags = 0;
+	cmd->rx_data_timeout = cpu_to_le32(25 * 1024);
+	cmd->tx_data_timeout = cpu_to_le32(25 * 1024);
+	cmd->keep_alive_beacons = 0;
+
+	dtim_period = il->vif ? il->vif->bss_conf.dtim_period : 0;
+
+	if (dtim_period <= 2) {
+		memcpy(cmd->sleep_interval, interval[0], sizeof(interval[0]));
+		no_dtim = 2;
+	} else if (dtim_period <= 10) {
+		memcpy(cmd->sleep_interval, interval[1], sizeof(interval[1]));
+		no_dtim = 2;
+	} else {
+		memcpy(cmd->sleep_interval, interval[2], sizeof(interval[2]));
+		no_dtim = 0;
+	}
+
+	if (dtim_period == 0) {
+		dtim_period = 1;
+		skip = false;
+	} else {
+		skip = !!no_dtim;
+	}
+
+	if (skip) {
+		__le32 tmp = cmd->sleep_interval[IL_POWER_VEC_SIZE - 1];
+
+		max_sleep = le32_to_cpu(tmp);
+		if (max_sleep == 0xFF)
+			max_sleep = dtim_period * (skip + 1);
+		else if (max_sleep >  dtim_period)
+			max_sleep = (max_sleep / dtim_period) * dtim_period;
+		cmd->flags |= IL_POWER_SLEEP_OVER_DTIM_MSK;
+	} else {
+		max_sleep = dtim_period;
+		cmd->flags &= ~IL_POWER_SLEEP_OVER_DTIM_MSK;
+	}
+
+	for (i = 0; i < IL_POWER_VEC_SIZE; i++)
+		if (le32_to_cpu(cmd->sleep_interval[i]) > max_sleep)
+			cmd->sleep_interval[i] = cpu_to_le32(max_sleep);
 }
 
 static int
@@ -1174,7 +1225,8 @@ il_power_update_mode(struct il_priv *il, bool force)
 {
 	struct il_powertable_cmd cmd;
 
-	il_power_sleep_cam_cmd(il, &cmd);
+	il_build_powertable_cmd(il, &cmd);
+
 	return il_power_set_mode(il, &cmd, force);
 }
 EXPORT_SYMBOL(il_power_update_mode);
@@ -5081,6 +5133,7 @@ set_ch_out:
 	}
 
 	if (changed & (IEEE80211_CONF_CHANGE_PS | IEEE80211_CONF_CHANGE_IDLE)) {
+		il->power_data.ps_disabled = !(conf->flags & IEEE80211_CONF_PS);
 		ret = il_power_update_mode(il, false);
 		if (ret)
 			D_MAC80211("Error setting sleep level\n");
diff --git a/drivers/net/wireless/iwlegacy/common.h b/drivers/net/wireless/iwlegacy/common.h
index f8246f2..8f81983 100644
--- a/drivers/net/wireless/iwlegacy/common.h
+++ b/drivers/net/wireless/iwlegacy/common.h
@@ -1123,6 +1123,7 @@ struct il_power_mgr {
 	struct il_powertable_cmd sleep_cmd_next;
 	int debug_sleep_level_override;
 	bool pci_pm;
+	bool ps_disabled;
 };
 
 struct il_priv {

Re: Power saving features for iwl4965

[Pedro Francisco]

On Mon, Jun 3, 2013 at 3:18 PM, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
> On Mon, Jun 03, 2013 at 10:52:39AM +0200, Tino Keitel wrote:
>> On Mon, Jan 07, 2013 at 12:08:00 +0100, Stanislaw Gruszka wrote:
>>
>> [...]
>>
>> > I posted patch here
>> > http://marc.info/?l=linux-wireless&m=135601033021616&w=2
>> >
>> > But I did not review code regarding power save to catch possible
>> > problems. Testing are bug reporting are welcome ...
>>
>> Hi,
>>
>> the patch is surprisingly small. To me it looks like it only contains
>> the code to make "iwconfig wlan0 power on" work, the actual power
>> management is missing.
>>
>> I just did some tests using powertop to see if I'm right. With the old
>> pre-iwlegacy driver, the difference between "iwconfig wlan0 power on"
>> and "...  power off" is more then 0,8W, which is ~10% of the total idle
>> power usage of my X61s with dimmed screen.  With a current kernel and
>> your patch, I can't measure a difference between "iwconfig wlan0 power
>> on" and "...  power off".  To me it seems that the patch is pretty
>> useless, at least on 4965AGN hardware.
>
> Yes, we do not send any command to put hardware in sleep mode when
> mac80211 enable PS.
>
>> It would be really nice to have proper power management in a current
>> kernel, as the laptop gets noticeably hotter with the current iwlegacy
>> driver.  That's why I still use a 3.1.10 kernel with an old
>> forward-ported iwlagn driver.
>
> Could you try this experimental patch? (...)

I am trying the iwlegacy powersave patch along with CPU scheduler BFS
and IO scheduler BFQ.

I've got this today:

Jun 09 00:51:34 s2 kernel: iwl3945 0000:02:00.0: Read idx for DMA
queue txq_id (2) idx 51 is out of range [0-256] 51 51
Jun 09 00:51:34 s2 kernel: iwl3945 0000:02:00.0: Read idx for DMA
queue txq_id (2) idx 52 is out of range [0-256] 51 51
(...)
Jun 09 00:51:38 s2 kernel: iwl3945 0000:02:00.0: Read idx for DMA
queue txq_id (2) idx 219 is out of range [0-256] 57 51
Jun 09 00:51:38 s2 kernel: iwl3945 0000:02:00.0: Read idx for DMA
queue txq_id (2) idx 220 is out of range [0-256] 57 51

Jun 09 00:51:34 s2 kernel: iwl3945 0000:02:00.0: Read idx for DMA
queue txq_id (2) idx 51 is out of range [0-256] 51 51
Jun 09 00:51:34 s2 kernel: iwl3945 0000:02:00.0: Read idx for DMA
queue txq_id (2) idx 52 is out of range [0-256] 51 51
(...)
Jun 09 00:51:38 s2 kernel: iwl3945 0000:02:00.0: Read idx for DMA
queue txq_id (2) idx 254 is out of range [0-256] 58 51
Jun 09 00:51:38 s2 kernel: iwl3945 0000:02:00.0: Read idx for DMA
queue txq_id (2) idx 255 is out of range [0-256] 58 51
Jun 09 00:51:38 s2 kernel: iwl3945 0000:02:00.0: Read idx for DMA
queue txq_id (2) idx 0 is out of range [0-256] 58 51
Jun 09 00:51:38 s2 kernel: iwl3945 0000:02:00.0: Read idx for DMA
queue txq_id (2) idx 1 is out of range [0-256] 58 51
(...)
Jun 09 00:51:39 s2 kernel: iwl3945 0000:02:00.0: Read idx for DMA
queue txq_id (2) idx 49 is out of range [0-256] 58 51
Jun 09 00:51:39 s2 kernel: iwl3945 0000:02:00.0: Read idx for DMA
queue txq_id (2) idx 50 is out of range [0-256] 58 51

$ uname -a
Linux s2 3.9.4-301.local.fc19.i686

I have just made a clean boot (as in, this has not happened after a
resume for suspend or hibernation).

May it be related to the patch? Or may heavy IO with out-of-tree CPU
scheduler BFS and out-of-tree IO scheduler BFQ be responsible for the
warnings?

Thanks in Advance,
-- 
Pedro Francisco

Re: Power saving features for iwl4965

[Pedro Francisco]

On Sun, Jun 9, 2013 at 1:28 AM, Pedro Francisco
<pedrogfrancisco@gmail.com> wrote:
> On Mon, Jun 3, 2013 at 3:18 PM, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
>> On Mon, Jun 03, 2013 at 10:52:39AM +0200, Tino Keitel wrote:
>>> (...)
>>> It would be really nice to have proper power management in a current
>>> kernel, as the laptop gets noticeably hotter with the current iwlegacy
>>> driver.  That's why I still use a 3.1.10 kernel with an old
>>> forward-ported iwlagn driver.
>>
>> Could you try this experimental patch? (...)
>
> I am trying the iwlegacy powersave patch along with CPU scheduler BFS
> and IO scheduler BFQ.
>
> I've got this today: (...)
>
> $ uname -a
> Linux s2 3.9.4-301.local.fc19.i686
>
> I have just made a clean boot (as in, this has not happened after a
> resume for suspend or hibernation).
>
> May it be related to the patch? Or may heavy IO with out-of-tree CPU
> scheduler BFS and out-of-tree IO scheduler BFQ be responsible for the
> warnings?

I also got a SYSASSERT:
[Jun 9 10:35] iwl3945 0000:02:00.0: Read idx for DMA queue txq_id (2)
idx 186 is out of range [0-256] 186 186
(...)
[  +0,000164] iwl3945 0000:02:00.0: Read idx for DMA queue txq_id (2)
idx 185 is out of range [0-256] 186 186
[Jun 9 10:36] iwl3945 0000:02:00.0: Microcode SW error detected.
Restarting 0x82000008.
[  +0,000008] iwl3945 0000:02:00.0: Loaded firmware version: 15.32.2.9
[  +0,000034] iwl3945 0000:02:00.0: Start IWL Error Log Dump:
[  +0,000004] iwl3945 0000:02:00.0: Status: 0x000302E4, count: 1
[  +0,000004] iwl3945 0000:02:00.0: Desc       Time       asrtPC
blink2 ilink1  nmiPC   Line
[  +0,000224] iwl3945 0000:02:00.0: SYSASSERT     (0x5) 0228106348
0x008B6 0x0035E 0x0031C 0x00000 267

[  +0,000010] iwl3945 0000:02:00.0: Error Reply type 0x00000001 cmd
UNKNOWN (0xFC) seq 0x2099 ser 0x00FC0000
[  +0,000466] iwl3945 0000:02:00.0: Can't stop Rx DMA.
[  +0,001647] ieee80211 phy0: Hardware restart was requested
[Jun 9 10:38] iwl3945 0000:02:00.0: Read idx for DMA queue txq_id (2)
idx 186 is out of range [0-256] 186 186
(...)

I disabled powersave (but kept running the same kernel) and none of
the errors appeared again.

Re: Power saving features for iwl4965

[Stanislaw Gruszka]

On Mon, Jun 10, 2013 at 08:31:33PM +0100, Pedro Francisco wrote:
> On Sun, Jun 9, 2013 at 1:28 AM, Pedro Francisco
> <pedrogfrancisco@gmail.com> wrote:
> > On Mon, Jun 3, 2013 at 3:18 PM, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
> >> On Mon, Jun 03, 2013 at 10:52:39AM +0200, Tino Keitel wrote:
> >>> (...)
> >>> It would be really nice to have proper power management in a current
> >>> kernel, as the laptop gets noticeably hotter with the current iwlegacy
> >>> driver.  That's why I still use a 3.1.10 kernel with an old
> >>> forward-ported iwlagn driver.
> >>
> >> Could you try this experimental patch? (...)
> >
> > I am trying the iwlegacy powersave patch along with CPU scheduler BFS
> > and IO scheduler BFQ.
> >
> > I've got this today: (...)
> >
> > $ uname -a
> > Linux s2 3.9.4-301.local.fc19.i686
> >
> > I have just made a clean boot (as in, this has not happened after a
> > resume for suspend or hibernation).
> >
> > May it be related to the patch? Or may heavy IO with out-of-tree CPU
> > scheduler BFS and out-of-tree IO scheduler BFQ be responsible for the
> > warnings?
> 
> I also got a SYSASSERT:
> [Jun 9 10:35] iwl3945 0000:02:00.0: Read idx for DMA queue txq_id (2)
> idx 186 is out of range [0-256] 186 186
> (...)
> [  +0,000164] iwl3945 0000:02:00.0: Read idx for DMA queue txq_id (2)
> idx 185 is out of range [0-256] 186 186
> [Jun 9 10:36] iwl3945 0000:02:00.0: Microcode SW error detected.
> Restarting 0x82000008.
> [  +0,000008] iwl3945 0000:02:00.0: Loaded firmware version: 15.32.2.9
> [  +0,000034] iwl3945 0000:02:00.0: Start IWL Error Log Dump:
> [  +0,000004] iwl3945 0000:02:00.0: Status: 0x000302E4, count: 1
> [  +0,000004] iwl3945 0000:02:00.0: Desc       Time       asrtPC
> blink2 ilink1  nmiPC   Line
> [  +0,000224] iwl3945 0000:02:00.0: SYSASSERT     (0x5) 0228106348
> 0x008B6 0x0035E 0x0031C 0x00000 267
> 
> [  +0,000010] iwl3945 0000:02:00.0: Error Reply type 0x00000001 cmd
> UNKNOWN (0xFC) seq 0x2099 ser 0x00FC0000
> [  +0,000466] iwl3945 0000:02:00.0: Can't stop Rx DMA.
> [  +0,001647] ieee80211 phy0: Hardware restart was requested
> [Jun 9 10:38] iwl3945 0000:02:00.0: Read idx for DMA queue txq_id (2)
> idx 186 is out of range [0-256] 186 186
> (...)
> 
> I disabled powersave (but kept running the same kernel) and none of
> the errors appeared again.

Yes, this seems to be iwlegacy PS issue and has to be fixed before
this patch could be applied.

Thanks for testing.
Stanislaw 

Re: Power saving features for iwl4965

[Tino Keitel]

On Mon, Jun 03, 2013 at 16:18:05 +0200, Stanislaw Gruszka wrote:

> Could you try this experimental patch?

Hi,

on top of kernel 3.9.4 (with TuxOnIce patch), iwconfig wlan0 power on
now saves more than 1 W on my ThinkPad X61s with iwl4965. I can finally
switch to a recent kernel. Thanks a lot.

Regards,
Tino

Re: Power saving features for iwl4965

[Tino Keitel]

Hi,

in my kernel log, I also see a SYSASSERT, but only once after resume
from suspend to RAM.

2013-06-11_18:40:58.80553 kern.notice: sd 0:0:0:0: [sda] Starting disk
2013-06-11_18:40:58.80554 kern.err: iwl4965 0000:03:00.0: Microcode SW
error detected.  Restarting 0x82000000.
2013-06-11_18:40:58.80555 kern.err: iwl4965 0000:03:00.0: Loaded
firmware version: 228.61.2.24
2013-06-11_18:40:58.80556 kern.err: iwl4965 0000:03:00.0: Start IWL
Error Log Dump:
2013-06-11_18:40:58.80557 kern.err: iwl4965 0000:03:00.0: Status:
0x000313E4, count: 5
2013-06-11_18:40:58.80558 kern.err: iwl4965 0000:03:00.0: Desc
Time       data1      data2      line
2013-06-11_18:40:58.80559 kern.err: iwl4965 0000:03:00.0: SYSASSERT
(0x0005) 0000000095 0x00000000 0x00000000 262
2013-06-11_18:40:58.80560 kern.err: iwl4965 0000:03:00.0: pc
blink1  blink2  ilink1  ilink2  hcmd
2013-06-11_18:40:58.80561 kern.err: iwl4965 0000:03:00.0: 0x0C924
0x0C90E 0x0C93A 0x006DA 0x00000 0x4090010
2013-06-11_18:40:58.80562 kern.err: iwl4965 0000:03:00.0: FH register
values:
2013-06-11_18:40:58.80563 kern.err: iwl4965 0000:03:00.0:
FH49_RSCSR_CHNL0_STTS_WPTR_REG: 0X0b101300
2013-06-11_18:40:58.80564 kern.err: iwl4965 0000:03:00.0:
FH49_RSCSR_CHNL0_RBDCB_BASE_REG: 0X00b10120
2013-06-11_18:40:58.80565 kern.err: iwl4965 0000:03:00.0:
FH49_RSCSR_CHNL0_WPTR: 0X00000008
2013-06-11_18:40:58.80565 kern.err: iwl4965 0000:03:00.0:
FH49_MEM_RCSR_CHNL0_CONFIG_REG: 0X80819000
2013-06-11_18:40:58.80566 kern.err: iwl4965 0000:03:00.0:
FH49_MEM_RSSR_SHARED_CTRL_REG: 0X0000003c
2013-06-11_18:40:58.80567 kern.err: iwl4965 0000:03:00.0:
FH49_MEM_RSSR_RX_STATUS_REG: 0X07030000
2013-06-11_18:40:58.80569 kern.err: iwl4965 0000:03:00.0:
FH49_MEM_RSSR_RX_ENABLE_ERR_IRQ2DRV: 0X00000000
2013-06-11_18:40:58.80570 kern.err: iwl4965 0000:03:00.0:
FH49_TSSR_TX_STATUS_REG: 0X07ff0002
2013-06-11_18:40:58.80571 kern.err: iwl4965 0000:03:00.0:
FH49_TSSR_TX_ERROR_REG: 0X00000000
2013-06-11_18:40:58.80572 kern.err: iwl4965 0000:03:00.0: Command
C_RXON failed: FW Error
2013-06-11_18:40:58.80575 kern.err: iwl4965 0000:03:00.0: Error setting
new RXON (-5)
2013-06-11_18:40:58.80576 kern.warn: iwl4965 0000:03:00.0: Try to add
interface when device not ready

Regards,
Tino

Re: Power saving features for iwl4965

[Pedro Francisco]

On Tue, Jun 11, 2013 at 5:19 PM, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
> On Mon, Jun 10, 2013 at 08:31:33PM +0100, Pedro Francisco wrote:
>> On Sun, Jun 9, 2013 at 1:28 AM, Pedro Francisco
>> <pedrogfrancisco@gmail.com> wrote:
>> > On Mon, Jun 3, 2013 at 3:18 PM, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
>> >> On Mon, Jun 03, 2013 at 10:52:39AM +0200, Tino Keitel wrote:
>> >>> (...)
>> >>> It would be really nice to have proper power management in a current
>> >>> kernel, as the laptop gets noticeably hotter with the current iwlegacy
>> >>> driver.  That's why I still use a 3.1.10 kernel with an old
>> >>> forward-ported iwlagn driver.
>> >>
>> >> Could you try this experimental patch? (...)
>> >
>> > I am trying the iwlegacy powersave patch along with CPU scheduler BFS
>> > and IO scheduler BFQ.
>> >
>> > I've got this today: (...)
>> >
>> > (...)
>> I also got a SYSASSERT:
>> (...)
>>
>> I disabled powersave (but kept running the same kernel) and none of
>> the errors appeared again.
>
> Yes, this seems to be iwlegacy PS issue and has to be fixed before
> this patch could be applied.

But is it a driver-only issue? I had assumed it was some sort of
fireware+driver issue...

Will running with debug on help? Or is it easily reproducible on any
iwl3945 / iwl4465 ?

Because in my case (iwl3945) it happens after boot, no prior suspend
to RAM is required to trigger the issues.

P.S.: the demise of bugzilla.intellinuxwireless.org it's very
difficult to find useful info about this. It would appear 4465 is just
affected after suspend, believing the patch which disabled PS in 2009.

Re: Power saving features for iwl4965

[Stanislaw Gruszka]

On Fri, Jun 14, 2013 at 01:50:58PM +0100, Pedro Francisco wrote:
> >> >> Could you try this experimental patch? (...)
> >> >
> >> > I am trying the iwlegacy powersave patch along with CPU scheduler BFS
> >> > and IO scheduler BFQ.
> >> >
> >> > I've got this today: (...)
> >> >
> >> > (...)
> >> I also got a SYSASSERT:
> >> (...)
> >>
> >> I disabled powersave (but kept running the same kernel) and none of
> >> the errors appeared again.
> >
> > Yes, this seems to be iwlegacy PS issue and has to be fixed before
> > this patch could be applied.
> 
> But is it a driver-only issue? I had assumed it was some sort of
> fireware+driver issue...
Looks like driver issue or firmware issue that can be workaround in 
the driver.

> Will running with debug on help? Or is it easily reproducible on any
> iwl3945 / iwl4465 ?
> 
> Because in my case (iwl3945) it happens after boot, no prior suspend
> to RAM is required to trigger the issues.
I can reproduce microcode error on iwl4965 by reloading modules. Looks
like we put device in sleep mode and it can not be properly booted when
driver initalize. I did not yet test patch on iwl3945. When I'll do
this, I think I'll be able to reproduce problems you discovered. If not
I'll ask for more debug info.

Thanks
Stanislaw

Re: Power saving features for iwl4965

[Stanislaw Gruszka]

[-- Attachment #1: Type: text/plain, Size: 2056 bytes --]

On Fri, Jun 14, 2013 at 03:18:29PM +0200, Stanislaw Gruszka wrote:
> > >> I also got a SYSASSERT:
> > >> (...)
> > >>
> > >> I disabled powersave (but kept running the same kernel) and none of
> > >> the errors appeared again.
> > >
> > > Yes, this seems to be iwlegacy PS issue and has to be fixed before
> > > this patch could be applied.
> > 
> > But is it a driver-only issue? I had assumed it was some sort of
> > fireware+driver issue...
> Looks like driver issue or firmware issue that can be workaround in 
> the driver.
> 
> > Will running with debug on help? Or is it easily reproducible on any
> > iwl3945 / iwl4465 ?
> > 
> > Because in my case (iwl3945) it happens after boot, no prior suspend
> > to RAM is required to trigger the issues.
> I can reproduce microcode error on iwl4965 by reloading modules. Looks
> like we put device in sleep mode and it can not be properly booted when
> driver initalize. I did not yet test patch on iwl3945. When I'll do
> this, I think I'll be able to reproduce problems you discovered. If not
> I'll ask for more debug info.

I found problem on 4965, we have to send power configuration command to
device earlier, before some other commands. Attached patch solved
microcode errors I have on 4965.

Regarding iwl3945. Display on my 3945 laptop no longer works. I took
3945 device from that laptop and installed it on two different machines.
Unfortunately on none of them device was detected by pci system :-/
I have figure out how to get working 3945 testing environment, but for
now, perhaps you could provide me some more debug information.

Pedro, please do the fallowing:

Add this line in /etc/rsyslog.conf: 
kern.*                                                  /var/log/kernel

Restart rsyslog services:
# systemctl restart rsyslog.service

Restart iwl3945 module with verbose debug enabled:
modprobe -r iwl3945
modprobe iwl3945 debug=0x47ffffff

Reproduce the problem.

Unload module:
modprobe -r iwl3945

Then provide me generated /var/log/kernel file (compressed if needed).

Thanks
Stanislaw


[-- Attachment #2: iwl4965-update-power-mode-early.patch --]
[-- Type: text/plain, Size: 730 bytes --]

diff --git a/drivers/net/wireless/iwlegacy/4965-mac.c b/drivers/net/wireless/iwlegacy/4965-mac.c
index d287fd2..e7821f6 100644
--- a/drivers/net/wireless/iwlegacy/4965-mac.c
+++ b/drivers/net/wireless/iwlegacy/4965-mac.c
@@ -5334,6 +5334,9 @@ il4965_alive_start(struct il_priv *il)
 
 	il->active_rate = RATES_MASK;
 
+	il_power_update_mode(il, true);
+	D_INFO("Updated power mode\n");
+
 	if (il_is_associated(il)) {
 		struct il_rxon_cmd *active_rxon =
 		    (struct il_rxon_cmd *)&il->active;
@@ -5364,9 +5367,6 @@ il4965_alive_start(struct il_priv *il)
 	D_INFO("ALIVE processing complete.\n");
 	wake_up(&il->wait_command_queue);
 
-	il_power_update_mode(il, true);
-	D_INFO("Updated power mode\n");
-
 	return;
 
 restart:

Re: Power saving features for iwl4965

[Pedro Francisco]

1On Tue, Jun 25, 2013 at 3:25 PM, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
> On Fri, Jun 14, 2013 at 03:18:29PM +0200, Stanislaw Gruszka wrote:
>> > >> I also got a SYSASSERT:
>> > >> (...)
>> > >>
>> > >> I disabled powersave (but kept running the same kernel) and none of
>> > >> the errors appeared again.
>> > >
>> > > Yes, this seems to be iwlegacy PS issue and has to be fixed before
>> > > this patch could be applied.
>> >
>> > But is it a driver-only issue? I had assumed it was some sort of
>> > fireware+driver issue...
>> Looks like driver issue or firmware issue that can be workaround in
>> the driver.
>>
>> > Will running with debug on help? Or is it easily reproducible on any
>> > iwl3945 / iwl4465 ?
>> >
>> > Because in my case (iwl3945) it happens after boot, no prior suspend
>> > to RAM is required to trigger the issues.
>> I can reproduce microcode error on iwl4965 by reloading modules. Looks
>> like we put device in sleep mode and it can not be properly booted when
>> driver initalize. I did not yet test patch on iwl3945. When I'll do
>> this, I think I'll be able to reproduce problems you discovered. If not
>> I'll ask for more debug info.
>
> I found problem on 4965, we have to send power configuration command to
> device earlier, before some other commands. Attached patch solved
> microcode errors I have on 4965.
>
> Regarding iwl3945. Display on my 3945 laptop no longer works. I took
> 3945 device from that laptop and installed it on two different machines.
> Unfortunately on none of them device was detected by pci system :-/
> I have figure out how to get working 3945 testing environment, but for
> now, perhaps you could provide me some more debug information.
>
> Pedro, please do the fallowing:
>
> Add this line in /etc/rsyslog.conf:
> kern.*                                                  /var/log/kernel
>
> Restart rsyslog services:
> # systemctl restart rsyslog.service
>
> Restart iwl3945 module with verbose debug enabled:
> modprobe -r iwl3945
> modprobe iwl3945 debug=0x47ffffff
>
> Reproduce the problem.
>
> Unload module:
> modprobe -r iwl3945
>
> Then provide me generated /var/log/kernel file (compressed if needed).

I seem only to be able to trigger it with disable_hw_scan=0, I need
further testing with disable_hw_scan=1 (I use disable_hw_scan=0
because it prevents me from getting disconnected from eduroam Cisco
APs -- haven't tested disable_hw_scan=0 since the VOIP-friendly SW
scanning patch, however).

Do you want the log anyway? (modprobe iwl3945 debug=0x47ffffff
disable_hw_scan=0 and runtime PCI powersave also enabled -- I don't
know if it matters).
--
Pedro

Re: Power saving features for iwl4965

[Stanislaw Gruszka]

On Thu, Jul 11, 2013 at 10:02:22PM +0100, Pedro Francisco wrote:
> > Pedro, please do the fallowing:
> >
> > Add this line in /etc/rsyslog.conf:
> > kern.*                                                  /var/log/kernel
> >
> > Restart rsyslog services:
> > # systemctl restart rsyslog.service
> >
> > Restart iwl3945 module with verbose debug enabled:
> > modprobe -r iwl3945
> > modprobe iwl3945 debug=0x47ffffff
> >
> > Reproduce the problem.
> >
> > Unload module:
> > modprobe -r iwl3945
> >
> > Then provide me generated /var/log/kernel file (compressed if needed).
> 
> I seem only to be able to trigger it with disable_hw_scan=0, I need
> further testing with disable_hw_scan=1 (I use disable_hw_scan=0
> because it prevents me from getting disconnected from eduroam Cisco
> APs -- haven't tested disable_hw_scan=0 since the VOIP-friendly SW
> scanning patch, however).
> 
> Do you want the log anyway? (modprobe iwl3945 debug=0x47ffffff
> disable_hw_scan=0 and runtime PCI powersave also enabled -- I don't
> know if it matters).

To test that patch powersave has to be explicitly enabled, since
it is disabled by default.

As this is not causing troubles with default disable_hw_scan option,
I'll post that patch.

You can provide me logs with disable_hw_scan=0, if you manage to
trigger a Microcode error.

Thanks
Stanislaw

  

> --
> Pedro

Re: Power saving features for iwl4965

[Pedro Francisco]

On Tue, Jul 16, 2013 at 11:27 AM, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
> You can provide me logs with disable_hw_scan=0, if you manage to
> trigger a Microcode error.

I can't trigger the microcode error with the debug. The firmware
however gets corrupted and reloading the module does not fix it
(firmware validation fails until I restart) -- curiously after a
'power info' RXON (? or similar, can't access the logs now) is sent to
the card.

I'll test again tomorrow.

Re: Power saving features for iwl4965

[Pedro Francisco]

On Tue, Jul 16, 2013 at 11:27 AM, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
>> I seem only to be able to trigger it with disable_hw_scan=0, I need
>> further testing with disable_hw_scan=1 (I use disable_hw_scan=0
>> because it prevents me from getting disconnected from eduroam Cisco
>> APs -- haven't tested disable_hw_scan=0 since the VOIP-friendly SW
>> scanning patch, however).
>>
>> Do you want the log anyway? (modprobe iwl3945 debug=0x47ffffff
>> disable_hw_scan=0 and runtime PCI powersave also enabled -- I don't
>> know if it matters).
>
> As this is not causing troubles with default disable_hw_scan option,
> I'll post that patch.

My mistake, I can trigger error conditions _independently_ of
disable_hw_scan option being enabled or disabled, as long as powersave
is enabled.

I'll send you a private email with the logs.

Re: Power saving features for iwl4965

[Stanislaw Gruszka]

[-- Attachment #1: Type: text/plain, Size: 1238 bytes --]

On Wed, Jul 17, 2013 at 12:48:30PM +0100, Pedro Francisco wrote:
> On Tue, Jul 16, 2013 at 11:27 AM, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
> >> I seem only to be able to trigger it with disable_hw_scan=0, I need
> >> further testing with disable_hw_scan=1 (I use disable_hw_scan=0
> >> because it prevents me from getting disconnected from eduroam Cisco
> >> APs -- haven't tested disable_hw_scan=0 since the VOIP-friendly SW
> >> scanning patch, however).
> >>
> >> Do you want the log anyway? (modprobe iwl3945 debug=0x47ffffff
> >> disable_hw_scan=0 and runtime PCI powersave also enabled -- I don't
> >> know if it matters).
> >
> > As this is not causing troubles with default disable_hw_scan option,
> > I'll post that patch.
> 
> My mistake, I can trigger error conditions _independently_ of
> disable_hw_scan option being enabled or disabled, as long as powersave
> is enabled.
> 
> I'll send you a private email with the logs.

I think I found bug which couse this firmware crash. We have only 5
queues so updating write_ptr for txq[5] can cause random value write
to HBUS_TARG_WRPTR register. I also added spin_lock to do not abuse
writes we do in tx skb.

Please test attached patch (with powersave on :-)

Stanislaw


[-- Attachment #2: iwlegacy_fix_wakeup_interrupt.patch --]
[-- Type: text/plain, Size: 818 bytes --]

diff --git a/drivers/net/wireless/iwlegacy/3945-mac.c b/drivers/net/wireless/iwlegacy/3945-mac.c
index b37a582..afa5c6e 100644
--- a/drivers/net/wireless/iwlegacy/3945-mac.c
+++ b/drivers/net/wireless/iwlegacy/3945-mac.c
@@ -1495,12 +1495,14 @@ il3945_irq_tasklet(struct il_priv *il)
 	if (inta & CSR_INT_BIT_WAKEUP) {
 		D_ISR("Wakeup interrupt\n");
 		il_rx_queue_update_write_ptr(il, &il->rxq);
+
+		spin_lock_irqsave(&il->lock, flags);
 		il_txq_update_write_ptr(il, &il->txq[0]);
 		il_txq_update_write_ptr(il, &il->txq[1]);
 		il_txq_update_write_ptr(il, &il->txq[2]);
 		il_txq_update_write_ptr(il, &il->txq[3]);
 		il_txq_update_write_ptr(il, &il->txq[4]);
-		il_txq_update_write_ptr(il, &il->txq[5]);
+		spin_unlock_irqrestore(&il->lock, flags);
 
 		il->isr_stats.wakeup++;
 		handled |= CSR_INT_BIT_WAKEUP;

Re: Power saving features for iwl4965

[Pedro Francisco]

On Wed, Jul 31, 2013 at 1:08 PM, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
> On Wed, Jul 17, 2013 at 12:48:30PM +0100, Pedro Francisco wrote:
>> On Tue, Jul 16, 2013 at 11:27 AM, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
>> >> I seem only to be able to trigger it with disable_hw_scan=0, I need
>> >> further testing with disable_hw_scan=1 (I use disable_hw_scan=0
>> >> because it prevents me from getting disconnected from eduroam Cisco
>> >> APs -- haven't tested disable_hw_scan=0 since the VOIP-friendly SW
>> >> scanning patch, however).
>> >>
>> >> Do you want the log anyway? (modprobe iwl3945 debug=0x47ffffff
>> >> disable_hw_scan=0 and runtime PCI powersave also enabled -- I don't
>> >> know if it matters).
>> >
>> > As this is not causing troubles with default disable_hw_scan option,
>> > I'll post that patch.
>>
>> My mistake, I can trigger error conditions _independently_ of
>> disable_hw_scan option being enabled or disabled, as long as powersave
>> is enabled.
>>
>> I'll send you a private email with the logs.
>
> I think I found bug which couse this firmware crash. We have only 5
> queues so updating write_ptr for txq[5] can cause random value write
> to HBUS_TARG_WRPTR register. I also added spin_lock to do not abuse
> writes we do in tx skb.
>
> Please test attached patch (with powersave on :-)

Still not working :( I tested for two nights, first one was fine,
second was not (the only difference was I had kernel parameter
`slub_debug` on the second night, but my guess it shouldn't affect
anything?).

Snippet of log follows, full logs I'll send privately (beware,
unzipped it's 423MB).

Anyway, any idea what is the value 0xa5a5a5a2 ? It's always the same
value which makes the reloaded firmware to fail... And in other
firmware failures around the Web, both iwlegacy and iwlwifi.

Aug  4 09:23:58 s2 kernel: [32309.937084] iwl3945 0000:02:00.0: Queue
4 stuck for 2004 ms.
Aug  4 09:23:58 s2 kernel: [32309.937106] iwl3945 0000:02:00.0: On
demand firmware reload
Aug  4 09:23:58 s2 kernel: [32309.937128] ieee80211 phy1: U
__il3945_down iwl3945 is going down
Aug  4 09:23:58 s2 kernel: [32309.937137] ieee80211 phy1: U
il_scan_cancel_timeout Scan cancel timeout
Aug  4 09:23:58 s2 kernel: [32309.937143] ieee80211 phy1: U
il_do_scan_abort Not performing scan to abort
Aug  4 09:23:58 s2 kernel: [32309.937149] ieee80211 phy1: U
il_clear_ucode_stations Clearing ucode stations in driver
Aug  4 09:23:58 s2 kernel: [32309.937155] ieee80211 phy1: U
il_clear_ucode_stations Clearing ucode active for station 0
Aug  4 09:23:58 s2 kernel: [32309.937162] ieee80211 phy1: U
il_clear_ucode_stations Clearing ucode active for station 24
Aug  4 09:23:58 s2 kernel: [32309.938116] ieee80211 phy1: U
_il_apm_stop Stop card, put in low power state
Aug  4 09:23:58 s2 kernel: [32309.938116] iwl3945 0000:02:00.0: Master
Disable Timed Out, 100 usec
Aug  4 09:23:58 s2 kernel: [32309.938116] ieee80211 phy1: U
_il_apm_stop_master stop master
Aug  4 09:23:58 s2 kernel: [32309.954705] ieee80211 phy1: U
il3945_clear_free_frames 0 frames on pre-allocated heap on clear.
Aug  4 09:23:58 s2 kernel: [32309.954740] ieee80211 phy1: Hardware
restart was requested
Aug  4 09:23:58 s2 kernel: [32309.954757] ieee80211 phy1: U
il3945_mac_start enter
Aug  4 09:23:58 s2 kernel: [32309.954763] ieee80211 phy1: U
il_prep_station Add STA to driver ID 24: ff:ff:ff:ff:ff:ff
Aug  4 09:23:58 s2 kernel: [32309.954774] ieee80211 phy1: U
il_apm_init Init card's basic functions
Aug  4 09:23:58 s2 kernel: [32309.955111] ieee80211 phy1: U
il3945_nic_config HW Revision ID = 0x2
Aug  4 09:23:58 s2 kernel: [32309.955117] ieee80211 phy1: U
il3945_nic_config 3945 RADIO-MM type
Aug  4 09:23:58 s2 kernel: [32309.955127] ieee80211 phy1: U
il3945_nic_config SKU OP mode is basic
Aug  4 09:23:58 s2 kernel: [32309.955131] ieee80211 phy1: U
il3945_nic_config 3945ABG revision is 0xF1
Aug  4 09:23:58 s2 kernel: [32309.955140] ieee80211 phy1: U
il3945_nic_config Card M type B version is 0x3
Aug  4 09:23:58 s2 kernel: [32309.955150] ieee80211 phy1: U
il3945_nic_config SW RF KILL supported in EEPROM.
Aug  4 09:23:58 s2 kernel: [32309.955153] ieee80211 phy1: U
il3945_nic_config HW RF KILL supported in EEPROM.
Aug  4 09:23:58 s2 kernel: [32309.974318] ieee80211 phy1: U
il3945_load_bsm Begin load bsm
Aug  4 09:23:58 s2 kernel: [32310.018857] ieee80211 phy1: U
il3945_verify_bsm Begin verify bsm
Aug  4 09:23:58 s2 kernel: [32310.020628] iwl3945 0000:02:00.0: BSM
uCode verification failed at addr 0x00003800+0 (of 900), is
0xa5a5a5a2, s/b 0xf802020
Aug  4 09:23:58 s2 kernel: [32310.020632] iwl3945 0000:02:00.0: Unable
to set up bootstrap uCode: -5
Aug  4 09:23:58 s2 kernel: [32310.020636] ieee80211 phy1: U
il3945_load_bsm Begin load bsm
Aug  4 09:23:58 s2 kernel: [32310.041691] hrtimer: interrupt took 3021833 ns
Aug  4 09:23:58 s2 kernel: [32310.065681] ieee80211 phy1: U
il3945_verify_bsm Begin verify bsm
Aug  4 09:23:58 s2 kernel: [32310.067345] iwl3945 0000:02:00.0: BSM
uCode verification failed at addr 0x00003800+0 (of 900), is
0xa5a5a5a2, s/b 0xf802020

-- 
Pedro

Re: Power saving features for iwl4965

[Pedro Francisco]

On Sun, Aug 4, 2013 at 3:24 PM, Pedro Francisco
<pedrogfrancisco@gmail.com> wrote:
> On Wed, Jul 31, 2013 at 1:08 PM, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
>> On Wed, Jul 17, 2013 at 12:48:30PM +0100, Pedro Francisco wrote:
>>> On Tue, Jul 16, 2013 at 11:27 AM, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
>>> >> I seem only to be able to trigger it with disable_hw_scan=0, I need
>>> >> further testing with disable_hw_scan=1 (I use disable_hw_scan=0
>>> >> because it prevents me from getting disconnected from eduroam Cisco
>>> >> APs -- haven't tested disable_hw_scan=0 since the VOIP-friendly SW
>>> >> scanning patch, however).
>>> >>
>>> >> Do you want the log anyway? (modprobe iwl3945 debug=0x47ffffff
>>> >> disable_hw_scan=0 and runtime PCI powersave also enabled -- I don't
>>> >> know if it matters).
>>> >
>>> > As this is not causing troubles with default disable_hw_scan option,
>>> > I'll post that patch.
>>>
>>> My mistake, I can trigger error conditions _independently_ of
>>> disable_hw_scan option being enabled or disabled, as long as powersave
>>> is enabled.
>>>
>>> I'll send you a private email with the logs.
>>
>> I think I found bug which couse this firmware crash. We have only 5
>> queues so updating write_ptr for txq[5] can cause random value write
>> to HBUS_TARG_WRPTR register. I also added spin_lock to do not abuse
>> writes we do in tx skb.
>>
>> Please test attached patch (with powersave on :-)
>
> Still not working :( I tested for two nights, first one was fine,
> second was not (the only difference was I had kernel parameter
> `slub_debug` on the second night, but my guess it shouldn't affect
> anything?).
>
> Snippet of log follows, full logs I'll send privately (beware,
> unzipped it's 423MB).
>
> Anyway, any idea what is the value 0xa5a5a5a2 ? It's always the same
> value which makes the reloaded firmware to fail... And in other
> firmware failures around the Web, both iwlegacy and iwlwifi.
>
> Aug  4 09:23:58 s2 kernel: [32309.937084] iwl3945 0000:02:00.0: Queue
> 4 stuck for 2004 ms.
> Aug  4 09:23:58 s2 kernel: [32309.937106] iwl3945 0000:02:00.0: On
> demand firmware reload
> Aug  4 09:23:58 s2 kernel: [32309.937128] ieee80211 phy1: U
> __il3945_down iwl3945 is going down
> Aug  4 09:23:58 s2 kernel: [32309.937137] ieee80211 phy1: U
> il_scan_cancel_timeout Scan cancel timeout
> Aug  4 09:23:58 s2 kernel: [32309.937143] ieee80211 phy1: U
> il_do_scan_abort Not performing scan to abort
> Aug  4 09:23:58 s2 kernel: [32309.937149] ieee80211 phy1: U
> il_clear_ucode_stations Clearing ucode stations in driver
> Aug  4 09:23:58 s2 kernel: [32309.937155] ieee80211 phy1: U
> il_clear_ucode_stations Clearing ucode active for station 0
> Aug  4 09:23:58 s2 kernel: [32309.937162] ieee80211 phy1: U
> il_clear_ucode_stations Clearing ucode active for station 24
> Aug  4 09:23:58 s2 kernel: [32309.938116] ieee80211 phy1: U
> _il_apm_stop Stop card, put in low power state
> Aug  4 09:23:58 s2 kernel: [32309.938116] iwl3945 0000:02:00.0: Master
> Disable Timed Out, 100 usec
> Aug  4 09:23:58 s2 kernel: [32309.938116] ieee80211 phy1: U
> _il_apm_stop_master stop master
> Aug  4 09:23:58 s2 kernel: [32309.954705] ieee80211 phy1: U
> il3945_clear_free_frames 0 frames on pre-allocated heap on clear.
> Aug  4 09:23:58 s2 kernel: [32309.954740] ieee80211 phy1: Hardware
> restart was requested
> Aug  4 09:23:58 s2 kernel: [32309.954757] ieee80211 phy1: U
> il3945_mac_start enter
> Aug  4 09:23:58 s2 kernel: [32309.954763] ieee80211 phy1: U
> il_prep_station Add STA to driver ID 24: ff:ff:ff:ff:ff:ff
> Aug  4 09:23:58 s2 kernel: [32309.954774] ieee80211 phy1: U
> il_apm_init Init card's basic functions
> Aug  4 09:23:58 s2 kernel: [32309.955111] ieee80211 phy1: U
> il3945_nic_config HW Revision ID = 0x2
> Aug  4 09:23:58 s2 kernel: [32309.955117] ieee80211 phy1: U
> il3945_nic_config 3945 RADIO-MM type
> Aug  4 09:23:58 s2 kernel: [32309.955127] ieee80211 phy1: U
> il3945_nic_config SKU OP mode is basic
> Aug  4 09:23:58 s2 kernel: [32309.955131] ieee80211 phy1: U
> il3945_nic_config 3945ABG revision is 0xF1
> Aug  4 09:23:58 s2 kernel: [32309.955140] ieee80211 phy1: U
> il3945_nic_config Card M type B version is 0x3
> Aug  4 09:23:58 s2 kernel: [32309.955150] ieee80211 phy1: U
> il3945_nic_config SW RF KILL supported in EEPROM.
> Aug  4 09:23:58 s2 kernel: [32309.955153] ieee80211 phy1: U
> il3945_nic_config HW RF KILL supported in EEPROM.
> Aug  4 09:23:58 s2 kernel: [32309.974318] ieee80211 phy1: U
> il3945_load_bsm Begin load bsm
> Aug  4 09:23:58 s2 kernel: [32310.018857] ieee80211 phy1: U
> il3945_verify_bsm Begin verify bsm
> Aug  4 09:23:58 s2 kernel: [32310.020628] iwl3945 0000:02:00.0: BSM
> uCode verification failed at addr 0x00003800+0 (of 900), is
> 0xa5a5a5a2, s/b 0xf802020
> Aug  4 09:23:58 s2 kernel: [32310.020632] iwl3945 0000:02:00.0: Unable
> to set up bootstrap uCode: -5
> Aug  4 09:23:58 s2 kernel: [32310.020636] ieee80211 phy1: U
> il3945_load_bsm Begin load bsm
> Aug  4 09:23:58 s2 kernel: [32310.041691] hrtimer: interrupt took 3021833 ns
> Aug  4 09:23:58 s2 kernel: [32310.065681] ieee80211 phy1: U
> il3945_verify_bsm Begin verify bsm
> Aug  4 09:23:58 s2 kernel: [32310.067345] iwl3945 0000:02:00.0: BSM
> uCode verification failed at addr 0x00003800+0 (of 900), is
> 0xa5a5a5a2, s/b 0xf802020

Other thing, which may or may not be relevant. The 'script':
rmmod iwl3945; modprobe iwl3945 debug=0x47ffffff ; sleep 10; rmmod
iwl3945; modprobe iwl3945

At the end of this 'script', shouldn't debug mode be disabled? I
thought it would be disabled, but until `modprobe iwl3945 debug=0` is
done, debug is still active. Does this mean firmware reload/module
reload isn't completely resetting the card, as I suppose it was
supposed to do?

-- 
Pedro

Re: Power saving features for iwl4965

[Stanislaw Gruszka]

[-- Attachment #1: Type: text/plain, Size: 2086 bytes --]

On Sun, Aug 04, 2013 at 03:53:14PM +0100, Pedro Francisco wrote:
> On Sun, Aug 4, 2013 at 3:24 PM, Pedro Francisco
> <pedrogfrancisco@gmail.com> wrote:
> > On Wed, Jul 31, 2013 at 1:08 PM, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
> >> On Wed, Jul 17, 2013 at 12:48:30PM +0100, Pedro Francisco wrote:
> >>> On Tue, Jul 16, 2013 at 11:27 AM, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
> >>> >> I seem only to be able to trigger it with disable_hw_scan=0, I need
> >>> >> further testing with disable_hw_scan=1 (I use disable_hw_scan=0
> >>> >> because it prevents me from getting disconnected from eduroam Cisco
> >>> >> APs -- haven't tested disable_hw_scan=0 since the VOIP-friendly SW
> >>> >> scanning patch, however).
> >>> >>
> >>> >> Do you want the log anyway? (modprobe iwl3945 debug=0x47ffffff
> >>> >> disable_hw_scan=0 and runtime PCI powersave also enabled -- I don't
> >>> >> know if it matters).
> >>> >
> >>> > As this is not causing troubles with default disable_hw_scan option,
> >>> > I'll post that patch.
> >>>
> >>> My mistake, I can trigger error conditions _independently_ of
> >>> disable_hw_scan option being enabled or disabled, as long as powersave
> >>> is enabled.
> >>>
> >>> I'll send you a private email with the logs.
> >>
> >> I think I found bug which couse this firmware crash. We have only 5
> >> queues so updating write_ptr for txq[5] can cause random value write
> >> to HBUS_TARG_WRPTR register. I also added spin_lock to do not abuse
> >> writes we do in tx skb.
> >>
> >> Please test attached patch (with powersave on :-)
> >
> > Still not working :( I tested for two nights, first one was fine,
> > second was not (the only difference was I had kernel parameter
> > `slub_debug` on the second night, but my guess it shouldn't affect
> > anything?).
> >
> > Snippet of log follows, full logs I'll send privately (beware,
> > unzipped it's 423MB).

Sorry for late answer. I have two more patches, which perhaps make
powersave stop crashing on 3945. Please test them (note that I only
compile tested, be carefull :-)

Thanks
Stanislaw


[-- Attachment #2: 0001-iwlegacy-poke-device-when-waiting-for-hcmd.patch --]
[-- Type: text/plain, Size: 2005 bytes --]

>From c9da2966ce58ed1446367a0b14b0953ebc3fbfbe Mon Sep 17 00:00:00 2001
From: Stanislaw Gruszka <sgruszka@redhat.com>
Date: Thu, 17 Oct 2013 10:48:44 +0200
Subject: [PATCH 1/2] iwlegacy poke device when waiting for hcmd

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/iwlegacy/common.c |   27 ++++++++++++++++++++-------
 1 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/iwlegacy/common.c b/drivers/net/wireless/iwlegacy/common.c
index b03e22e..7ada864 100644
--- a/drivers/net/wireless/iwlegacy/common.c
+++ b/drivers/net/wireless/iwlegacy/common.c
@@ -254,8 +254,6 @@ il_get_cmd_string(u8 cmd)
 }
 EXPORT_SYMBOL(il_get_cmd_string);
 
-#define HOST_COMPLETE_TIMEOUT (HZ / 2)
-
 static void
 il_generic_cmd_callback(struct il_priv *il, struct il_device_cmd *cmd,
 			struct il_rx_pkt *pkt)
@@ -305,11 +303,15 @@ il_send_cmd_async(struct il_priv *il, struct il_host_cmd *cmd)
 	return 0;
 }
 
+#define HOST_COMPLETE_TIMEOUT (HZ / 2)
+#define COMMAND_POKE_TIMEOUT  (HZ / 10)
+
 int
 il_send_cmd_sync(struct il_priv *il, struct il_host_cmd *cmd)
 {
-	int cmd_idx;
-	int ret;
+	unsigned long flags;
+	int cmd_idx, ret;
+	int timeout = HOST_COMPLETE_TIMEOUT;
 
 	lockdep_assert_held(&il->mutex);
 
@@ -333,9 +335,20 @@ il_send_cmd_sync(struct il_priv *il, struct il_host_cmd *cmd)
 		goto out;
 	}
 
-	ret = wait_event_timeout(il->wait_command_queue,
-				 !test_bit(S_HCMD_ACTIVE, &il->status),
-				 HOST_COMPLETE_TIMEOUT);
+	while (timeout) {
+		ret = wait_event_timeout(il->wait_command_queue,
+					 !test_bit(S_HCMD_ACTIVE, &il->status),
+					 COMMAND_POKE_TIMEOUT);
+		if (ret)
+			break;
+
+		/* Poke the device, it may have lost the command. */
+		spin_lock_irqsave(&il->reg_lock, flags);
+		_il_grab_nic_access(il);
+		_il_release_nic_access(il);
+		spin_unlock_irqrestore(&il->reg_lock, flags);
+	}
+
 	if (!ret) {
 		if (test_bit(S_HCMD_ACTIVE, &il->status)) {
 			IL_ERR("Error sending %s: time out after %dms.\n",
-- 
1.7.1


[-- Attachment #3: 0002-iwlegacy-merge-and-fix-reclaim-for-3945.patch --]
[-- Type: text/plain, Size: 3829 bytes --]

>From 05b86620fcec5f18293b0df2a4dddbe34ae24125 Mon Sep 17 00:00:00 2001
From: Stanislaw Gruszka <sgruszka@redhat.com>
Date: Thu, 17 Oct 2013 11:03:21 +0200
Subject: [PATCH 2/2] iwlegacy: merge and fix reclaim for 3945

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/iwlegacy/3945-mac.c |    9 +--------
 drivers/net/wireless/iwlegacy/4965-mac.c |   12 +-----------
 drivers/net/wireless/iwlegacy/common.h   |   14 ++++++++++++++
 3 files changed, 16 insertions(+), 19 deletions(-)

diff --git a/drivers/net/wireless/iwlegacy/3945-mac.c b/drivers/net/wireless/iwlegacy/3945-mac.c
index dea3b50..d7c8dd3 100644
--- a/drivers/net/wireless/iwlegacy/3945-mac.c
+++ b/drivers/net/wireless/iwlegacy/3945-mac.c
@@ -1248,14 +1248,7 @@ il3945_rx_handle(struct il_priv *il)
 		len = le32_to_cpu(pkt->len_n_flags) & IL_RX_FRAME_SIZE_MSK;
 		len += sizeof(u32);	/* account for status word */
 
-		/* Reclaim a command buffer only if this packet is a response
-		 *   to a (driver-originated) command.
-		 * If the packet (e.g. Rx frame) originated from uCode,
-		 *   there is no command buffer to reclaim.
-		 * Ucode should set SEQ_RX_FRAME bit if ucode-originated,
-		 *   but apparently a few don't get set; catch them here. */
-		reclaim = !(pkt->hdr.sequence & SEQ_RX_FRAME) &&
-		    pkt->hdr.cmd != N_STATS && pkt->hdr.cmd != C_TX;
+		reclaim = il_need_reclaim(il, pkt);
 
 		/* Based on type of command response or notification,
 		 *   handle those that need handling via function in
diff --git a/drivers/net/wireless/iwlegacy/4965-mac.c b/drivers/net/wireless/iwlegacy/4965-mac.c
index 86812b9..97cfaa9 100644
--- a/drivers/net/wireless/iwlegacy/4965-mac.c
+++ b/drivers/net/wireless/iwlegacy/4965-mac.c
@@ -4274,17 +4274,7 @@ il4965_rx_handle(struct il_priv *il)
 		len = le32_to_cpu(pkt->len_n_flags) & IL_RX_FRAME_SIZE_MSK;
 		len += sizeof(u32);	/* account for status word */
 
-		/* Reclaim a command buffer only if this packet is a response
-		 *   to a (driver-originated) command.
-		 * If the packet (e.g. Rx frame) originated from uCode,
-		 *   there is no command buffer to reclaim.
-		 * Ucode should set SEQ_RX_FRAME bit if ucode-originated,
-		 *   but apparently a few don't get set; catch them here. */
-		reclaim = !(pkt->hdr.sequence & SEQ_RX_FRAME) &&
-		    (pkt->hdr.cmd != N_RX_PHY) && (pkt->hdr.cmd != N_RX) &&
-		    (pkt->hdr.cmd != N_RX_MPDU) &&
-		    (pkt->hdr.cmd != N_COMPRESSED_BA) &&
-		    (pkt->hdr.cmd != N_STATS) && (pkt->hdr.cmd != C_TX);
+		reclaim = il_need_reclaim(il, pkt);
 
 		/* Based on type of command response or notification,
 		 *   handle those that need handling via function in
diff --git a/drivers/net/wireless/iwlegacy/common.h b/drivers/net/wireless/iwlegacy/common.h
index 83f8ed8..eccd6a4 100644
--- a/drivers/net/wireless/iwlegacy/common.h
+++ b/drivers/net/wireless/iwlegacy/common.h
@@ -1978,6 +1978,20 @@ extern void il_wr_prph(struct il_priv *il, u32 addr, u32 val);
 extern u32 il_read_targ_mem(struct il_priv *il, u32 addr);
 extern void il_write_targ_mem(struct il_priv *il, u32 addr, u32 val);
 
+static inline bool il_need_reclaim(struct il_priv *il, struct il_rx_pkt *pkt)
+{
+	/* Reclaim a command buffer only if this packet is a response
+	 * to a (driver-originated) command. If the packet (e.g. Rx frame)
+	 * originated from uCode, there is no command buffer to reclaim.
+	 * Ucode should set SEQ_RX_FRAME bit if ucode-originated, but
+	 * apparently a few don't get set; catch them here.
+	 */
+	return !(pkt->hdr.sequence & SEQ_RX_FRAME) &&
+	       pkt->hdr.cmd != N_STATS && pkt->hdr.cmd != C_TX &&
+	       pkt->hdr.cmd != N_RX_PHY && pkt->hdr.cmd != N_RX &&
+	       pkt->hdr.cmd != N_RX_MPDU && pkt->hdr.cmd != N_COMPRESSED_BA;
+}
+
 static inline void
 _il_write8(struct il_priv *il, u32 ofs, u8 val)
 {
-- 
1.7.1

Re: Power saving features for iwl4965

[Stanislaw Gruszka]

[-- Attachment #1: Type: text/plain, Size: 307 bytes --]

On Thu, Oct 17, 2013 at 11:06:55AM +0200, Stanislaw Gruszka wrote:
> Sorry for late answer. I have two more patches, which perhaps make
> powersave stop crashing on 3945. Please test them (note that I only
> compile tested, be carefull :-)

First patch has a bug. I'm attaching improved version.

Stanislaw

[-- Attachment #2: 0001-iwlegacy-poke-device-when-waiting-for-hcmd.patch --]
[-- Type: text/plain, Size: 2045 bytes --]

>From dae9f167853f266d5efe688f965968725375e2ae Mon Sep 17 00:00:00 2001
From: Stanislaw Gruszka <sgruszka@redhat.com>
Date: Thu, 17 Oct 2013 15:30:07 +0200
Subject: [PATCH] iwlegacy poke device when waiting for hcmd

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/iwlegacy/common.c | 29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/iwlegacy/common.c b/drivers/net/wireless/iwlegacy/common.c
index b03e22e..3d8dae3c 100644
--- a/drivers/net/wireless/iwlegacy/common.c
+++ b/drivers/net/wireless/iwlegacy/common.c
@@ -254,8 +254,6 @@ il_get_cmd_string(u8 cmd)
 }
 EXPORT_SYMBOL(il_get_cmd_string);
 
-#define HOST_COMPLETE_TIMEOUT (HZ / 2)
-
 static void
 il_generic_cmd_callback(struct il_priv *il, struct il_device_cmd *cmd,
 			struct il_rx_pkt *pkt)
@@ -305,11 +303,15 @@ il_send_cmd_async(struct il_priv *il, struct il_host_cmd *cmd)
 	return 0;
 }
 
+#define HOST_COMPLETE_TIMEOUT (HZ / 2)
+#define COMMAND_POKE_TIMEOUT  (HZ / 10)
+
 int
 il_send_cmd_sync(struct il_priv *il, struct il_host_cmd *cmd)
 {
-	int cmd_idx;
-	int ret;
+	unsigned long flags;
+	int cmd_idx, ret;
+	int timeout = HOST_COMPLETE_TIMEOUT;
 
 	lockdep_assert_held(&il->mutex);
 
@@ -333,9 +335,22 @@ il_send_cmd_sync(struct il_priv *il, struct il_host_cmd *cmd)
 		goto out;
 	}
 
-	ret = wait_event_timeout(il->wait_command_queue,
-				 !test_bit(S_HCMD_ACTIVE, &il->status),
-				 HOST_COMPLETE_TIMEOUT);
+	while (timeout > 0) {
+		ret = wait_event_timeout(il->wait_command_queue,
+					 !test_bit(S_HCMD_ACTIVE, &il->status),
+					 COMMAND_POKE_TIMEOUT);
+		if (ret)
+			break;
+
+		/* Poke the device, it may have lost the command. */
+		spin_lock_irqsave(&il->reg_lock, flags);
+		_il_grab_nic_access(il);
+		_il_release_nic_access(il);
+		spin_unlock_irqrestore(&il->reg_lock, flags);
+
+		timeout -= COMMAND_POKE_TIMEOUT;
+	}
+
 	if (!ret) {
 		if (test_bit(S_HCMD_ACTIVE, &il->status)) {
 			IL_ERR("Error sending %s: time out after %dms.\n",
-- 
1.8.3.1

Re: Power saving features for iwl4965

[Stanislaw Gruszka]

On Tue, Jun 11, 2013 at 08:51:20PM +0200, Tino Keitel wrote:
> On Mon, Jun 03, 2013 at 16:18:05 +0200, Stanislaw Gruszka wrote:
> 
> > Could you try this experimental patch?
> 
> Hi,
> 
> on top of kernel 3.9.4 (with TuxOnIce patch), iwconfig wlan0 power on
> now saves more than 1 W on my ThinkPad X61s with iwl4965. I can finally
> switch to a recent kernel. Thanks a lot.

This patch stuck on mailing list far too long, I'll post it. Even if it
crash firmware on 3945, power save is disabled by default.

Stanislaw

Re: Power saving features for iwl4965

[Emmanuel Grumbach]

On Tue, Feb 18, 2014 at 12:57 PM, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
> On Tue, Jun 11, 2013 at 08:51:20PM +0200, Tino Keitel wrote:
>> On Mon, Jun 03, 2013 at 16:18:05 +0200, Stanislaw Gruszka wrote:
>>
>> > Could you try this experimental patch?
>>
>> Hi,
>>
>> on top of kernel 3.9.4 (with TuxOnIce patch), iwconfig wlan0 power on
>> now saves more than 1 W on my ThinkPad X61s with iwl4965. I can finally
>> switch to a recent kernel. Thanks a lot.
>
> This patch stuck on mailing list far too long, I'll post it. Even if it
> crash firmware on 3945, power save is disabled by default.
>

I'd be very surprised if this patch helped. I guess you took the code
from iwlwifi in which we have enabled shadow registers for 7000
series. This HW is problematic but it doesn't exit on 4965.

Re: Power saving features for iwl4965

[Stanislaw Gruszka]

On Tue, Feb 18, 2014 at 01:32:44PM +0200, Emmanuel Grumbach wrote:
> On Tue, Feb 18, 2014 at 12:57 PM, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
> > On Tue, Jun 11, 2013 at 08:51:20PM +0200, Tino Keitel wrote:
> >> On Mon, Jun 03, 2013 at 16:18:05 +0200, Stanislaw Gruszka wrote:
> >>
> >> > Could you try this experimental patch?
> >>
> >> Hi,
> >>
> >> on top of kernel 3.9.4 (with TuxOnIce patch), iwconfig wlan0 power on
> >> now saves more than 1 W on my ThinkPad X61s with iwl4965. I can finally
> >> switch to a recent kernel. Thanks a lot.
> >
> > This patch stuck on mailing list far too long, I'll post it. Even if it
> > crash firmware on 3945, power save is disabled by default.
> >
> 
> I'd be very surprised if this patch helped. I guess you took the code
> from iwlwifi in which we have enabled shadow registers for 7000
> series. This HW is problematic but it doesn't exit on 4965.

It is besed on old iwlwifi code (when iwlwifi & iwlegacy were one driver).

Stanislaw

Re: Power saving features for iwl4965

[Pedro Francisco]

On Tue, Feb 18, 2014 at 10:57 AM, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
> On Tue, Jun 11, 2013 at 08:51:20PM +0200, Tino Keitel wrote:
>> On Mon, Jun 03, 2013 at 16:18:05 +0200, Stanislaw Gruszka wrote:
>>
>> > Could you try this experimental patch?
>>
>> Hi,
>>
>> on top of kernel 3.9.4 (with TuxOnIce patch), iwconfig wlan0 power on
>> now saves more than 1 W on my ThinkPad X61s with iwl4965. I can finally
>> switch to a recent kernel. Thanks a lot.
>
> This patch stuck on mailing list far too long, I'll post it. Even if it
> crash firmware on 3945, power save is disabled by default.

Ok!
The patch was not forgotten, just indefinitely delayed :/
Now that I can compile a kernel in 10 minutes instead of >1h, I'll try
to test it :)
-- 
Pedro

Re: Power saving features for iwl4965

[Pedro Francisco]

2014-02-20 12:08 GMT+00:00 Pedro Francisco <pedrogfrancisco@gmail.com>:
> On Tue, Feb 18, 2014 at 10:57 AM, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
>> On Tue, Jun 11, 2013 at 08:51:20PM +0200, Tino Keitel wrote:
>>> On Mon, Jun 03, 2013 at 16:18:05 +0200, Stanislaw Gruszka wrote:
>>>
>>> > Could you try this experimental patch?
>>>
>>> Hi,
>>>
>>> on top of kernel 3.9.4 (with TuxOnIce patch), iwconfig wlan0 power on
>>> now saves more than 1 W on my ThinkPad X61s with iwl4965. I can finally
>>> switch to a recent kernel. Thanks a lot.
>>
>> This patch stuck on mailing list far too long, I'll post it. Even if it
>> crash firmware on 3945, power save is disabled by default.
>
> Ok!
> The patch was not forgotten, just indefinitely delayed :/
> Now that I can compile a kernel in 10 minutes instead of >1h, I'll try
> to test it :)

Leaving the computer running for 24h gave no signs of any kind of
issues, besides a few AP timeout after 500ms.

If you wish me to further test more patches, please do include me in CC.
However, my access to the 3945 hardware is now very reduced, so I'll
probably take several weeks to test the patches, if at all.

Thank you for your patience, time and code since my first half-made
debugging of the HW scan crash in the iwl3945 firmware :)
-- 
Pedro

Re: Power saving features for iwl4965

[Tino Keitel]

On Wed, Apr 25, 2012 at 14:25:55 +0200, Stanislaw Gruszka wrote:
> On Wed, Apr 18, 2012 at 10:35:43PM +0200, Tino Keitel wrote:
> > up to 2.6.38, the iwl4965 driver was able to use power saving features
> > by invoking "iwconfig <interface> power on", at least when unsetting
> > the broken_powersave variable.
> > 
> > 2.6.39 introduced a new iwlegacy driver for iwl4965 hardware, which was
> > missing power saving.  I was able to port the 2.6.38 driver up to
> > kernel 3.1, but it got too complicated for me with 3.2. 
> > 
> > Are there any plans to fix this power usage regression at some point in
> > the iwlegacy driver? I think there are still a lot of iwl4965 users out
> > there.
> Yes, adding PS support is on my iwlegacy TODO list.

Hi,

is there any progress regarding this? I didn't see something obvious in
the git log.

Regards,
Tino