2010-03-30 21:50:35

by Gertjan van Wingerde

[permalink] [raw]
Subject: [PATCH 0/4] rt2x00: Powersave fixes.

The following 4 patches fix powersave behavior of rt2x00.
The first patch fixes the rt2500usb powersave issues found lately.
The second on enables powersave for rt2500usb again, now it is fixed.
The third and fourth are ongoing work to bring the other PCI drivers
in a better shape.

All patches are against wireless-2.6.

The patch series is also available via:

git://git.gwingerde.nl/rt2x00-2.6.git

Gertjan van Wingerde (4):
rt2x00: Disable auto wakeup before waking up device.
rt2x00: Enable powersaving by default again on rt2500usb.
rt2x00: Add wakeup interrupt handler to rt61pci.
rt2x00: Add wakeup interrupt handler to rt2800pci.

drivers/net/wireless/rt2x00/rt2400pci.c | 4 ++++
drivers/net/wireless/rt2x00/rt2500pci.c | 4 ++++
drivers/net/wireless/rt2x00/rt2500usb.c | 9 ++++-----
drivers/net/wireless/rt2x00/rt2800lib.c | 4 ++--
drivers/net/wireless/rt2x00/rt2800pci.c | 11 +++++++++++
drivers/net/wireless/rt2x00/rt61pci.c | 14 ++++++++++++++
drivers/net/wireless/rt2x00/rt73usb.c | 6 +++---
7 files changed, 42 insertions(+), 10 deletions(-)



2010-03-30 21:50:35

by Gertjan van Wingerde

[permalink] [raw]
Subject: [PATCH 2/4] rt2x00: Enable powersaving by default again on rt2500usb.

Now that the powersave issues on rt2500usb have been tackled, powersave
can be enabled by default again.

Signed-off-by: Gertjan van Wingerde <[email protected]>
---
drivers/net/wireless/rt2x00/rt2500usb.c | 5 -----
1 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt2500usb.c b/drivers/net/wireless/rt2x00/rt2500usb.c
index 54d2716..c1eec17 100644
--- a/drivers/net/wireless/rt2x00/rt2500usb.c
+++ b/drivers/net/wireless/rt2x00/rt2500usb.c
@@ -1647,11 +1647,6 @@ static int rt2500usb_probe_hw_mode(struct rt2x00_dev *rt2x00dev)
unsigned int i;

/*
- * Disable powersaving as default.
- */
- rt2x00dev->hw->wiphy->flags &= ~WIPHY_FLAG_PS_ON_BY_DEFAULT;
-
- /*
* Initialize all hw fields.
*/
rt2x00dev->hw->flags =
--
1.7.0.3


2010-03-30 21:50:36

by Gertjan van Wingerde

[permalink] [raw]
Subject: [PATCH 1/4] rt2x00: Disable auto wakeup before waking up device.

In all drivers ensure that auto wakeup is disabled before waking up the device.
This is needed to prevent connection stability issues and problems in waking up
the device.

Based upon a patch from Ondrej Zary <[email protected]>

Signed-off-by: Gertjan van Wingerde <[email protected]>
Cc: Ondrej Zary <[email protected]>
---
drivers/net/wireless/rt2x00/rt2400pci.c | 4 ++++
drivers/net/wireless/rt2x00/rt2500pci.c | 4 ++++
drivers/net/wireless/rt2x00/rt2500usb.c | 4 ++++
drivers/net/wireless/rt2x00/rt2800lib.c | 4 ++--
drivers/net/wireless/rt2x00/rt73usb.c | 6 +++---
5 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt2400pci.c b/drivers/net/wireless/rt2x00/rt2400pci.c
index c22b040..08a4789 100644
--- a/drivers/net/wireless/rt2x00/rt2400pci.c
+++ b/drivers/net/wireless/rt2x00/rt2400pci.c
@@ -525,6 +525,10 @@ static void rt2400pci_config_ps(struct rt2x00_dev *rt2x00dev,

rt2x00_set_field32(&reg, CSR20_AUTOWAKE, 1);
rt2x00pci_register_write(rt2x00dev, CSR20, reg);
+ } else {
+ rt2x00pci_register_read(rt2x00dev, CSR20, &reg);
+ rt2x00_set_field32(&reg, CSR20_AUTOWAKE, 0);
+ rt2x00pci_register_write(rt2x00dev, CSR20, reg);
}

rt2x00dev->ops->lib->set_device_state(rt2x00dev, state);
diff --git a/drivers/net/wireless/rt2x00/rt2500pci.c b/drivers/net/wireless/rt2x00/rt2500pci.c
index 52bbcf1..d084d70 100644
--- a/drivers/net/wireless/rt2x00/rt2500pci.c
+++ b/drivers/net/wireless/rt2x00/rt2500pci.c
@@ -573,6 +573,10 @@ static void rt2500pci_config_ps(struct rt2x00_dev *rt2x00dev,

rt2x00_set_field32(&reg, CSR20_AUTOWAKE, 1);
rt2x00pci_register_write(rt2x00dev, CSR20, reg);
+ } else {
+ rt2x00pci_register_read(rt2x00dev, CSR20, &reg);
+ rt2x00_set_field32(&reg, CSR20_AUTOWAKE, 0);
+ rt2x00pci_register_write(rt2x00dev, CSR20, reg);
}

rt2x00dev->ops->lib->set_device_state(rt2x00dev, state);
diff --git a/drivers/net/wireless/rt2x00/rt2500usb.c b/drivers/net/wireless/rt2x00/rt2500usb.c
index dbaa781..54d2716 100644
--- a/drivers/net/wireless/rt2x00/rt2500usb.c
+++ b/drivers/net/wireless/rt2x00/rt2500usb.c
@@ -648,6 +648,10 @@ static void rt2500usb_config_ps(struct rt2x00_dev *rt2x00dev,

rt2x00_set_field16(&reg, MAC_CSR18_AUTO_WAKE, 1);
rt2500usb_register_write(rt2x00dev, MAC_CSR18, reg);
+ } else {
+ rt2500usb_register_read(rt2x00dev, MAC_CSR18, &reg);
+ rt2x00_set_field16(&reg, MAC_CSR18_AUTO_WAKE, 0);
+ rt2500usb_register_write(rt2x00dev, MAC_CSR18, reg);
}

rt2x00dev->ops->lib->set_device_state(rt2x00dev, state);
diff --git a/drivers/net/wireless/rt2x00/rt2800lib.c b/drivers/net/wireless/rt2x00/rt2800lib.c
index 326fce7..68d0cfe 100644
--- a/drivers/net/wireless/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/rt2x00/rt2800lib.c
@@ -1014,13 +1014,13 @@ static void rt2800_config_ps(struct rt2x00_dev *rt2x00dev,

rt2x00dev->ops->lib->set_device_state(rt2x00dev, state);
} else {
- rt2x00dev->ops->lib->set_device_state(rt2x00dev, state);
-
rt2800_register_read(rt2x00dev, AUTOWAKEUP_CFG, &reg);
rt2x00_set_field32(&reg, AUTOWAKEUP_CFG_AUTO_LEAD_TIME, 0);
rt2x00_set_field32(&reg, AUTOWAKEUP_CFG_TBCN_BEFORE_WAKE, 0);
rt2x00_set_field32(&reg, AUTOWAKEUP_CFG_AUTOWAKE, 0);
rt2800_register_write(rt2x00dev, AUTOWAKEUP_CFG, reg);
+
+ rt2x00dev->ops->lib->set_device_state(rt2x00dev, state);
}
}

diff --git a/drivers/net/wireless/rt2x00/rt73usb.c b/drivers/net/wireless/rt2x00/rt73usb.c
index 47f3e4a..7ebe14b 100644
--- a/drivers/net/wireless/rt2x00/rt73usb.c
+++ b/drivers/net/wireless/rt2x00/rt73usb.c
@@ -860,15 +860,15 @@ static void rt73usb_config_ps(struct rt2x00_dev *rt2x00dev,
rt2x00usb_vendor_request_sw(rt2x00dev, USB_DEVICE_MODE, 0,
USB_MODE_SLEEP, REGISTER_TIMEOUT);
} else {
- rt2x00usb_vendor_request_sw(rt2x00dev, USB_DEVICE_MODE, 0,
- USB_MODE_WAKEUP, REGISTER_TIMEOUT);
-
rt2x00usb_register_read(rt2x00dev, MAC_CSR11, &reg);
rt2x00_set_field32(&reg, MAC_CSR11_DELAY_AFTER_TBCN, 0);
rt2x00_set_field32(&reg, MAC_CSR11_TBCN_BEFORE_WAKEUP, 0);
rt2x00_set_field32(&reg, MAC_CSR11_AUTOWAKE, 0);
rt2x00_set_field32(&reg, MAC_CSR11_WAKEUP_LATENCY, 0);
rt2x00usb_register_write(rt2x00dev, MAC_CSR11, reg);
+
+ rt2x00usb_vendor_request_sw(rt2x00dev, USB_DEVICE_MODE, 0,
+ USB_MODE_WAKEUP, REGISTER_TIMEOUT);
}
}

--
1.7.0.3


2010-03-30 21:50:36

by Gertjan van Wingerde

[permalink] [raw]
Subject: [PATCH 3/4] rt2x00: Add wakeup interrupt handler to rt61pci.

This is needed to wake up the device automatically for receiving beacons,
and is required for proper powersave handling.

Signed-off-by: Gertjan van Wingerde <[email protected]>
---
drivers/net/wireless/rt2x00/rt61pci.c | 14 ++++++++++++++
1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt61pci.c b/drivers/net/wireless/rt2x00/rt61pci.c
index e2da928..ac69dbe 100644
--- a/drivers/net/wireless/rt2x00/rt61pci.c
+++ b/drivers/net/wireless/rt2x00/rt61pci.c
@@ -2117,6 +2117,14 @@ static void rt61pci_txdone(struct rt2x00_dev *rt2x00dev)
}
}

+static void rt61pci_wakeup(struct rt2x00_dev *rt2x00dev)
+{
+ struct ieee80211_conf conf = { .flags = 0 };
+ struct rt2x00lib_conf libconf = { .conf = &conf };
+
+ rt61pci_config(rt2x00dev, &libconf, IEEE80211_CONF_CHANGE_PS);
+}
+
static irqreturn_t rt61pci_interrupt(int irq, void *dev_instance)
{
struct rt2x00_dev *rt2x00dev = dev_instance;
@@ -2164,6 +2172,12 @@ static irqreturn_t rt61pci_interrupt(int irq, void *dev_instance)
rt2x00pci_register_write(rt2x00dev,
M2H_CMD_DONE_CSR, 0xffffffff);

+ /*
+ * 4 - MCU Autowakeup interrupt.
+ */
+ if (rt2x00_get_field32(reg_mcu, MCU_INT_SOURCE_CSR_TWAKEUP))
+ rt61pci_wakeup(rt2x00dev);
+
return IRQ_HANDLED;
}

--
1.7.0.3


2010-03-30 21:50:35

by Gertjan van Wingerde

[permalink] [raw]
Subject: [PATCH 4/4] rt2x00: Add wakeup interrupt handler to rt2800pci.

This is needed to wake up the device automatically for receiving beacons,
and is required for proper powersave handling.

Signed-off-by: Gertjan van Wingerde <[email protected]>
---
drivers/net/wireless/rt2x00/rt2800pci.c | 11 +++++++++++
1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt2800pci.c b/drivers/net/wireless/rt2x00/rt2800pci.c
index 91cce2d..357873c 100644
--- a/drivers/net/wireless/rt2x00/rt2800pci.c
+++ b/drivers/net/wireless/rt2x00/rt2800pci.c
@@ -1006,6 +1006,14 @@ static void rt2800pci_txdone(struct rt2x00_dev *rt2x00dev)
}
}

+static void rt2800pci_wakeup(struct rt2x00_dev *rt2x00dev)
+{
+ struct ieee80211_conf conf = { .flags = 0 };
+ struct rt2x00lib_conf libconf = { .conf = &conf };
+
+ rt2800_config(rt2x00dev, &libconf, IEEE80211_CONF_CHANGE_PS);
+}
+
static irqreturn_t rt2800pci_interrupt(int irq, void *dev_instance)
{
struct rt2x00_dev *rt2x00dev = dev_instance;
@@ -1030,6 +1038,9 @@ static irqreturn_t rt2800pci_interrupt(int irq, void *dev_instance)
if (rt2x00_get_field32(reg, INT_SOURCE_CSR_TX_FIFO_STATUS))
rt2800pci_txdone(rt2x00dev);

+ if (rt2x00_get_field32(reg, INT_SOURCE_CSR_AUTO_WAKEUP))
+ rt2800pci_wakeup(rt2x00dev);
+
return IRQ_HANDLED;
}

--
1.7.0.3


2010-04-06 19:45:08

by John W. Linville

[permalink] [raw]
Subject: Re: [PATCH 0/4] rt2x00: Powersave fixes.

On Tue, Mar 30, 2010 at 11:50:22PM +0200, Gertjan van Wingerde wrote:
> The following 4 patches fix powersave behavior of rt2x00.
> The first patch fixes the rt2500usb powersave issues found lately.
> The second on enables powersave for rt2500usb again, now it is fixed.
> The third and fourth are ongoing work to bring the other PCI drivers
> in a better shape.
>
> All patches are against wireless-2.6.
>
> The patch series is also available via:
>
> git://git.gwingerde.nl/rt2x00-2.6.git
>
> Gertjan van Wingerde (4):
> rt2x00: Disable auto wakeup before waking up device.
> rt2x00: Enable powersaving by default again on rt2500usb.
> rt2x00: Add wakeup interrupt handler to rt61pci.
> rt2x00: Add wakeup interrupt handler to rt2800pci.

I don't have any objection to these patches, but they don't
seem appropriate for 2.6.34 to me. As such, basing them on
wireless-next-2.6 seems more appropriate.

Any objection to me applying them for 2.6.35?

John
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.

2010-04-06 19:52:29

by Gertjan van Wingerde

[permalink] [raw]
Subject: Re: [PATCH 0/4] rt2x00: Powersave fixes.

On 04/06/10 21:34, John W. Linville wrote:
> On Tue, Mar 30, 2010 at 11:50:22PM +0200, Gertjan van Wingerde wrote:
>> The following 4 patches fix powersave behavior of rt2x00.
>> The first patch fixes the rt2500usb powersave issues found lately.
>> The second on enables powersave for rt2500usb again, now it is fixed.
>> The third and fourth are ongoing work to bring the other PCI drivers
>> in a better shape.
>>
>> All patches are against wireless-2.6.
>>
>> The patch series is also available via:
>>
>> git://git.gwingerde.nl/rt2x00-2.6.git
>>
>> Gertjan van Wingerde (4):
>> rt2x00: Disable auto wakeup before waking up device.
>> rt2x00: Enable powersaving by default again on rt2500usb.
>> rt2x00: Add wakeup interrupt handler to rt61pci.
>> rt2x00: Add wakeup interrupt handler to rt2800pci.
>
> I don't have any objection to these patches, but they don't
> seem appropriate for 2.6.34 to me. As such, basing them on
> wireless-next-2.6 seems more appropriate.
>
> Any objection to me applying them for 2.6.35?
>

You're the boss ;-)

No, no objections from my side. We already disabled powersaving in 2.6.34,
so we should be covered for that release.

Note that patch 2 then won't apply to wireless-next-2.6, as the patch
that disables powersaving is only in wireless-2.6.

How do you want to handle applying patch 2 from the series; how do we
synchronize?

---
Gertjan.

2010-03-31 22:30:12

by Adam Baker

[permalink] [raw]
Subject: Re: [PATCH 2/4] rt2x00: Enable powersaving by default again on rt2500usb.

Gertjan van Wingerde <gwingerde@...> writes:

>
> Now that the powersave issues on rt2500usb have been tackled, powersave
> can be enabled by default again.
>
> Signed-off-by: Gertjan van Wingerde <gwingerde@...>
> ---
> drivers/net/wireless/rt2x00/rt2500usb.c | 5 -----
> 1 files changed, 0 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/wireless/rt2x00/rt2500usb.c
b/drivers/net/wireless/rt2x00/rt2500usb.c
> index 54d2716..c1eec17 100644
> --- a/drivers/net/wireless/rt2x00/rt2500usb.c
> +++ b/drivers/net/wireless/rt2x00/rt2500usb.c
> @@ -1647,11 +1647,6 @@ static int rt2500usb_probe_hw_mode(struct rt2x00_dev
*rt2x00dev)
> unsigned int i;
>
> /*
> - * Disable powersaving as default.
> - */
> - rt2x00dev->hw->wiphy->flags &= ~WIPHY_FLAG_PS_ON_BY_DEFAULT;
>
This looks like it deletes one line too few leaving a nested open comment
block in the code.

Adam




2010-04-06 21:30:11

by John W. Linville

[permalink] [raw]
Subject: Re: [PATCH 0/4] rt2x00: Powersave fixes.

On Tue, Apr 06, 2010 at 09:52:22PM +0200, Gertjan van Wingerde wrote:
> On 04/06/10 21:34, John W. Linville wrote:

> > Any objection to me applying them for 2.6.35?
> >
>
> You're the boss ;-)

More like "coordinator" I think... :-)

> No, no objections from my side. We already disabled powersaving in 2.6.34,
> so we should be covered for that release.
>
> Note that patch 2 then won't apply to wireless-next-2.6, as the patch
> that disables powersaving is only in wireless-2.6.
>
> How do you want to handle applying patch 2 from the series; how do we
> synchronize?

I'm hitting this type of problem with a number of pending patches now.
I'll need to pull wireless-2.6 into wireless-next-2.6 soon.

John
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.

2010-03-31 22:34:55

by Gertjan van Wingerde

[permalink] [raw]
Subject: Re: [PATCH 2/4] rt2x00: Enable powersaving by default again on rt2500usb.

On 04/01/10 00:29, Adam Baker wrote:
> Gertjan van Wingerde <gwingerde@...> writes:
>
>>
>> Now that the powersave issues on rt2500usb have been tackled, powersave
>> can be enabled by default again.
>>
>> Signed-off-by: Gertjan van Wingerde <gwingerde@...>
>> ---
>> drivers/net/wireless/rt2x00/rt2500usb.c | 5 -----
>> 1 files changed, 0 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/net/wireless/rt2x00/rt2500usb.c
> b/drivers/net/wireless/rt2x00/rt2500usb.c
>> index 54d2716..c1eec17 100644
>> --- a/drivers/net/wireless/rt2x00/rt2500usb.c
>> +++ b/drivers/net/wireless/rt2x00/rt2500usb.c
>> @@ -1647,11 +1647,6 @@ static int rt2500usb_probe_hw_mode(struct rt2x00_dev
> *rt2x00dev)
>> unsigned int i;
>>
>> /*
>> - * Disable powersaving as default.
>> - */
>> - rt2x00dev->hw->wiphy->flags &= ~WIPHY_FLAG_PS_ON_BY_DEFAULT;
>>
> This looks like it deletes one line too few leaving a nested open comment
> block in the code.
>

Nope, it doesn't. This is just the peculiar way diff creates patches within git.
If you take a look at the full patch, then you'll see that the patch removes all lines
including the '/*' of the next comment.
So the end-result is still good.

---
Gertjan.

2010-04-06 21:26:53

by Ivo Van Doorn

[permalink] [raw]
Subject: Re: [PATCH 0/4] rt2x00: Powersave fixes.

On Tuesday 06 April 2010, Gertjan van Wingerde wrote:
> On 04/06/10 21:34, John W. Linville wrote:
> > On Tue, Mar 30, 2010 at 11:50:22PM +0200, Gertjan van Wingerde wrote:
> >> The following 4 patches fix powersave behavior of rt2x00.
> >> The first patch fixes the rt2500usb powersave issues found lately.
> >> The second on enables powersave for rt2500usb again, now it is fixed.
> >> The third and fourth are ongoing work to bring the other PCI drivers
> >> in a better shape.
> >>
> >> All patches are against wireless-2.6.
> >>
> >> The patch series is also available via:
> >>
> >> git://git.gwingerde.nl/rt2x00-2.6.git
> >>
> >> Gertjan van Wingerde (4):
> >> rt2x00: Disable auto wakeup before waking up device.
> >> rt2x00: Enable powersaving by default again on rt2500usb.
> >> rt2x00: Add wakeup interrupt handler to rt61pci.
> >> rt2x00: Add wakeup interrupt handler to rt2800pci.
> >
> > I don't have any objection to these patches, but they don't
> > seem appropriate for 2.6.34 to me. As such, basing them on
> > wireless-next-2.6 seems more appropriate.
> >
> > Any objection to me applying them for 2.6.35?
> >
>
> You're the boss ;-)
>
> No, no objections from my side. We already disabled powersaving in 2.6.34,
> so we should be covered for that release.
>
> Note that patch 2 then won't apply to wireless-next-2.6, as the patch
> that disables powersaving is only in wireless-2.6.
>
> How do you want to handle applying patch 2 from the series; how do we
> synchronize?

John, not sure how important you consider it, but please note that the first 2 patches would fix:
https://bugzilla.kernel.org/show_bug.cgi?id=15699

Ivo