2009-12-13 01:19:31

by Gertjan van Wingerde

[permalink] [raw]
Subject: [PATCH 2.6.33] rt2x00: Fix rt2800usb detection in rt2800lib.

rt2800lib incorrectly detected whether RT2800USB was enabled because
it didn't account for a modularized RT2800USB driver.

Signed-off-by: Gertjan van Wingerde <[email protected]>
---

John, this one is a fix for 2.6.33.

---
drivers/net/wireless/rt2x00/rt2800lib.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt2800lib.c b/drivers/net/wireless/rt2x00/rt2800lib.c
index eb1e1d0..bdc3286 100644
--- a/drivers/net/wireless/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/rt2x00/rt2800lib.c
@@ -33,6 +33,10 @@
Abstract: rt2800 generic device routines.
*/

+#ifdef CONFIG_RT2800USB_MODULE
+#define CONFIG_RT2800USB
+#endif
+
#include <linux/kernel.h>
#include <linux/module.h>

--
1.6.5.5



2009-12-14 19:15:26

by John W. Linville

[permalink] [raw]
Subject: [PATCH] rt2x00: Fix rt2800usb detection in rt2800lib.

From: Gertjan van Wingerde <[email protected]>

rt2800lib incorrectly detected whether RT2800USB was enabled because
it didn't account for a modularized RT2800USB driver.

Signed-off-by: Gertjan van Wingerde <[email protected]>
Acked-by: Ivo van Doorn <[email protected]>
---
I'm merging this version based on Randy's feedback...

drivers/net/wireless/rt2x00/rt2800lib.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt2800lib.c b/drivers/net/wireless/rt2x00/rt2800lib.c
index eb1e1d0..6e13650 100644
--- a/drivers/net/wireless/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/rt2x00/rt2800lib.c
@@ -37,7 +37,7 @@
#include <linux/module.h>

#include "rt2x00.h"
-#ifdef CONFIG_RT2800USB
+#if defined(CONFIG_RT2800USB) || defined(CONFIG_RT2800USB_MODULE)
#include "rt2x00usb.h"
#endif
#include "rt2800lib.h"
@@ -1121,7 +1121,7 @@ int rt2800_init_registers(struct rt2x00_dev *rt2x00dev)

if (rt2x00_intf_is_usb(rt2x00dev)) {
rt2800_register_write(rt2x00dev, USB_DMA_CFG, 0x00000000);
-#ifdef CONFIG_RT2800USB
+#if defined(CONFIG_RT2800USB) || defined(CONFIG_RT2800USB_MODULE)
rt2x00usb_vendor_request_sw(rt2x00dev, USB_DEVICE_MODE, 0,
USB_MODE_RESET, REGISTER_TIMEOUT);
#endif
--
1.6.2.5


2009-12-14 19:27:52

by Gertjan van Wingerde

[permalink] [raw]
Subject: Re: [PATCH 2.6.33] rt2x00: Disable powersaving for rt61pci.

On 12/13/09 16:21, Gertjan van Wingerde wrote:
> On 12/13/09 16:18, Ivo van Doorn wrote:
>> On Sunday 13 December 2009, Gertjan van Wingerde wrote:
>>> On 12/13/09 11:15, Ivo van Doorn wrote:
>>>> On Sunday 13 December 2009, Gertjan van Wingerde wrote:
>>>>> We've had many reports of rt61pci failures with powersaving enabled.
>>>>> Therefore, as a stop-gap measure, disable powersaving of the rt61pci
>>>>> until we have found a proper solution.
>>>>
>>>> This disables powersaving completely, can't we set the default powersaving
>>>> to disabled? That way for people for who it does work, it can still be enabled.
>>>>
>>>
>>> Well, when I was looking into this I came up with a number of issues in the code
>>> which makes me believe that it simply isn't working at all.
>>> One example is that the rt61pci devices rely on handling the wakeup interrupt
>>> by the driver to wake up the devices. As far as I can tell there is no handling
>>> of the wakeup interrupt in the rt61pci driver.
>>> That's why I decided to disable it all together, as I don't think it works
>>> properly for anyone (although some people may not really notice it).
>>
>> Ok, but then please disable it for _all_ rt2x00 drivers, or at least all PCI rt2x00 drivers
>> since they would have the same wakeup interrupt requirement.
>>
>
> I'll see if this will be the case for all PCI rt2x00 devices and adapt for the ones that
> need it. I didn't look into the other devices yet, and ISTR that at least rt2500pci doesn't
> have this requirement.
>

OK. I can see that this does not apply to rt2400pci and rt2500pci. I'll leave those alone.
I'll add the disablement to rt2800pci as well, as it follows the same scheme as rt61pci.

Also, I will use the suggestion of you to just disable the default powersaving instead of
making it completely unusable.

Expect a new version any minute.

---
Gertjan.


2009-12-13 10:14:53

by Ivo Van Doorn

[permalink] [raw]
Subject: Re: [PATCH 2.6.33] rt2x00: Fix rt2800usb detection in rt2800lib.

On Sunday 13 December 2009, Gertjan van Wingerde wrote:
> rt2800lib incorrectly detected whether RT2800USB was enabled because
> it didn't account for a modularized RT2800USB driver.

This should be a temporary solution, since rt2800lib shouldn't use the
rt2800usb defines at all (it should be independent of rt2800pci/usb.


> Signed-off-by: Gertjan van Wingerde <[email protected]>

But for now:

Acked-by: Ivo van Doorn <[email protected]>

> ---
>
> John, this one is a fix for 2.6.33.
>
> ---
> drivers/net/wireless/rt2x00/rt2800lib.c | 4 ++++
> 1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/wireless/rt2x00/rt2800lib.c b/drivers/net/wireless/rt2x00/rt2800lib.c
> index eb1e1d0..bdc3286 100644
> --- a/drivers/net/wireless/rt2x00/rt2800lib.c
> +++ b/drivers/net/wireless/rt2x00/rt2800lib.c
> @@ -33,6 +33,10 @@
> Abstract: rt2800 generic device routines.
> */
>
> +#ifdef CONFIG_RT2800USB_MODULE
> +#define CONFIG_RT2800USB
> +#endif
> +
> #include <linux/kernel.h>
> #include <linux/module.h>
>



2009-12-13 15:16:58

by Gertjan van Wingerde

[permalink] [raw]
Subject: Re: [PATCH 2.6.33] rt2x00: Disable powersaving for rt61pci.

On 12/13/09 11:15, Ivo van Doorn wrote:
> On Sunday 13 December 2009, Gertjan van Wingerde wrote:
>> We've had many reports of rt61pci failures with powersaving enabled.
>> Therefore, as a stop-gap measure, disable powersaving of the rt61pci
>> until we have found a proper solution.
>
> This disables powersaving completely, can't we set the default powersaving
> to disabled? That way for people for who it does work, it can still be enabled.
>

Well, when I was looking into this I came up with a number of issues in the code
which makes me believe that it simply isn't working at all.
One example is that the rt61pci devices rely on handling the wakeup interrupt
by the driver to wake up the devices. As far as I can tell there is no handling
of the wakeup interrupt in the rt61pci driver.
That's why I decided to disable it all together, as I don't think it works
properly for anyone (although some people may not really notice it).

---
Gertjan.

2009-12-13 15:18:52

by Ivo Van Doorn

[permalink] [raw]
Subject: Re: [PATCH 2.6.33] rt2x00: Disable powersaving for rt61pci.

On Sunday 13 December 2009, Gertjan van Wingerde wrote:
> On 12/13/09 11:15, Ivo van Doorn wrote:
> > On Sunday 13 December 2009, Gertjan van Wingerde wrote:
> >> We've had many reports of rt61pci failures with powersaving enabled.
> >> Therefore, as a stop-gap measure, disable powersaving of the rt61pci
> >> until we have found a proper solution.
> >
> > This disables powersaving completely, can't we set the default powersaving
> > to disabled? That way for people for who it does work, it can still be enabled.
> >
>
> Well, when I was looking into this I came up with a number of issues in the code
> which makes me believe that it simply isn't working at all.
> One example is that the rt61pci devices rely on handling the wakeup interrupt
> by the driver to wake up the devices. As far as I can tell there is no handling
> of the wakeup interrupt in the rt61pci driver.
> That's why I decided to disable it all together, as I don't think it works
> properly for anyone (although some people may not really notice it).

Ok, but then please disable it for _all_ rt2x00 drivers, or at least all PCI rt2x00 drivers
since they would have the same wakeup interrupt requirement.

Ivo

2009-12-13 15:21:53

by Gertjan van Wingerde

[permalink] [raw]
Subject: Re: [PATCH 2.6.33] rt2x00: Disable powersaving for rt61pci.

On 12/13/09 16:18, Ivo van Doorn wrote:
> On Sunday 13 December 2009, Gertjan van Wingerde wrote:
>> On 12/13/09 11:15, Ivo van Doorn wrote:
>>> On Sunday 13 December 2009, Gertjan van Wingerde wrote:
>>>> We've had many reports of rt61pci failures with powersaving enabled.
>>>> Therefore, as a stop-gap measure, disable powersaving of the rt61pci
>>>> until we have found a proper solution.
>>>
>>> This disables powersaving completely, can't we set the default powersaving
>>> to disabled? That way for people for who it does work, it can still be enabled.
>>>
>>
>> Well, when I was looking into this I came up with a number of issues in the code
>> which makes me believe that it simply isn't working at all.
>> One example is that the rt61pci devices rely on handling the wakeup interrupt
>> by the driver to wake up the devices. As far as I can tell there is no handling
>> of the wakeup interrupt in the rt61pci driver.
>> That's why I decided to disable it all together, as I don't think it works
>> properly for anyone (although some people may not really notice it).
>
> Ok, but then please disable it for _all_ rt2x00 drivers, or at least all PCI rt2x00 drivers
> since they would have the same wakeup interrupt requirement.
>

I'll see if this will be the case for all PCI rt2x00 devices and adapt for the ones that
need it. I didn't look into the other devices yet, and ISTR that at least rt2500pci doesn't
have this requirement.

---
Gertjan.


2009-12-13 01:19:29

by Gertjan van Wingerde

[permalink] [raw]
Subject: [PATCH 2.6.33] rt2x00: Disable powersaving for rt61pci.

We've had many reports of rt61pci failures with powersaving enabled.
Therefore, as a stop-gap measure, disable powersaving of the rt61pci
until we have found a proper solution.

Cc: [email protected]
Signed-off-by: Gertjan van Wingerde <[email protected]>
---

John, this is an urgent fix for 2.6.33 (and stable).

---
drivers/net/wireless/rt2x00/rt61pci.c | 4 +---
1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt61pci.c b/drivers/net/wireless/rt2x00/rt61pci.c
index 687e17d..2b43a2e 100644
--- a/drivers/net/wireless/rt2x00/rt61pci.c
+++ b/drivers/net/wireless/rt2x00/rt61pci.c
@@ -2543,9 +2543,7 @@ static int rt61pci_probe_hw_mode(struct rt2x00_dev *rt2x00dev)
*/
rt2x00dev->hw->flags =
IEEE80211_HW_HOST_BROADCAST_PS_BUFFERING |
- IEEE80211_HW_SIGNAL_DBM |
- IEEE80211_HW_SUPPORTS_PS |
- IEEE80211_HW_PS_NULLFUNC_STACK;
+ IEEE80211_HW_SIGNAL_DBM;

SET_IEEE80211_DEV(rt2x00dev->hw, rt2x00dev->dev);
SET_IEEE80211_PERM_ADDR(rt2x00dev->hw,
--
1.6.5.5


2009-12-13 15:45:11

by Benoit Papillault

[permalink] [raw]
Subject: Re: [rt2x00-users] [PATCH 2.6.33] rt2x00: Disable powersaving for rt61pci.

Gertjan van Wingerde a écrit :
> On 12/13/09 16:18, Ivo van Doorn wrote:
>> On Sunday 13 December 2009, Gertjan van Wingerde wrote:
>>> On 12/13/09 11:15, Ivo van Doorn wrote:
>>>> On Sunday 13 December 2009, Gertjan van Wingerde wrote:
>>>>> We've had many reports of rt61pci failures with powersaving enabled.
>>>>> Therefore, as a stop-gap measure, disable powersaving of the rt61pci
>>>>> until we have found a proper solution.
>>>> This disables powersaving completely, can't we set the default powersaving
>>>> to disabled? That way for people for who it does work, it can still be enabled.
>>>>
>>> Well, when I was looking into this I came up with a number of issues in the code
>>> which makes me believe that it simply isn't working at all.
>>> One example is that the rt61pci devices rely on handling the wakeup interrupt
>>> by the driver to wake up the devices. As far as I can tell there is no handling
>>> of the wakeup interrupt in the rt61pci driver.
>>> That's why I decided to disable it all together, as I don't think it works
>>> properly for anyone (although some people may not really notice it).
>> Ok, but then please disable it for _all_ rt2x00 drivers, or at least all PCI rt2x00 drivers
>> since they would have the same wakeup interrupt requirement.
>>
>
> I'll see if this will be the case for all PCI rt2x00 devices and adapt for the ones that
> need it. I didn't look into the other devices yet, and ISTR that at least rt2500pci doesn't
> have this requirement.
>
> ---
> Gertjan.

If the same mecanism is used by rt2800usb, that would explain why power save is
not working as well on my rt2870 USB device. Is there a way to start the device
with power save disabled at startup (so it can be reenabled later to debug it).

Regards,
Benoit

2009-12-13 03:54:59

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH 2.6.33] rt2x00: Fix rt2800usb detection in rt2800lib.

On Sun, 13 Dec 2009 00:39:39 +0100 Gertjan van Wingerde wrote:

> rt2800lib incorrectly detected whether RT2800USB was enabled because
> it didn't account for a modularized RT2800USB driver.
>
> Signed-off-by: Gertjan van Wingerde <[email protected]>
> ---
>
> John, this one is a fix for 2.6.33.
>
> ---
> drivers/net/wireless/rt2x00/rt2800lib.c | 4 ++++
> 1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/wireless/rt2x00/rt2800lib.c b/drivers/net/wireless/rt2x00/rt2800lib.c
> index eb1e1d0..bdc3286 100644
> --- a/drivers/net/wireless/rt2x00/rt2800lib.c
> +++ b/drivers/net/wireless/rt2x00/rt2800lib.c
> @@ -33,6 +33,10 @@
> Abstract: rt2800 generic device routines.
> */
>
> +#ifdef CONFIG_RT2800USB_MODULE
> +#define CONFIG_RT2800USB
> +#endif
> +

The idiom that I am familiar with (and is the common one afaik)
is to use:

#if defined(CONFIG_RT2800USB) || defined(CONFIG_RT2800USB_MODULE)

Besides, it's not nice to muck with kconfig symbols outside of Kconfig files.


> #include <linux/kernel.h>
> #include <linux/module.h>


---
~Randy

2009-12-14 21:11:26

by Gertjan van Wingerde

[permalink] [raw]
Subject: Re: [PATCH] rt2x00: Fix rt2800usb detection in rt2800lib.

On 12/14/09 20:13, John W. Linville wrote:
> From: Gertjan van Wingerde <[email protected]>
>
> rt2800lib incorrectly detected whether RT2800USB was enabled because
> it didn't account for a modularized RT2800USB driver.
>
> Signed-off-by: Gertjan van Wingerde <[email protected]>
> Acked-by: Ivo van Doorn <[email protected]>
> ---
> I'm merging this version based on Randy's feedback...
>
> drivers/net/wireless/rt2x00/rt2800lib.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/rt2x00/rt2800lib.c b/drivers/net/wireless/rt2x00/rt2800lib.c
> index eb1e1d0..6e13650 100644
> --- a/drivers/net/wireless/rt2x00/rt2800lib.c
> +++ b/drivers/net/wireless/rt2x00/rt2800lib.c
> @@ -37,7 +37,7 @@
> #include <linux/module.h>
>
> #include "rt2x00.h"
> -#ifdef CONFIG_RT2800USB
> +#if defined(CONFIG_RT2800USB) || defined(CONFIG_RT2800USB_MODULE)
> #include "rt2x00usb.h"
> #endif
> #include "rt2800lib.h"
> @@ -1121,7 +1121,7 @@ int rt2800_init_registers(struct rt2x00_dev *rt2x00dev)
>
> if (rt2x00_intf_is_usb(rt2x00dev)) {
> rt2800_register_write(rt2x00dev, USB_DMA_CFG, 0x00000000);
> -#ifdef CONFIG_RT2800USB
> +#if defined(CONFIG_RT2800USB) || defined(CONFIG_RT2800USB_MODULE)
> rt2x00usb_vendor_request_sw(rt2x00dev, USB_DEVICE_MODE, 0,
> USB_MODE_RESET, REGISTER_TIMEOUT);
> #endif

Thanks, looks good to me.

---
Gertjan.

2009-12-13 10:15:42

by Ivo Van Doorn

[permalink] [raw]
Subject: Re: [PATCH 2.6.33] rt2x00: Disable powersaving for rt61pci.

On Sunday 13 December 2009, Gertjan van Wingerde wrote:
> We've had many reports of rt61pci failures with powersaving enabled.
> Therefore, as a stop-gap measure, disable powersaving of the rt61pci
> until we have found a proper solution.

This disables powersaving completely, can't we set the default powersaving
to disabled? That way for people for who it does work, it can still be enabled.

Ivo

> Cc: [email protected]
> Signed-off-by: Gertjan van Wingerde <[email protected]>
> ---
>
> John, this is an urgent fix for 2.6.33 (and stable).
>
> ---
> drivers/net/wireless/rt2x00/rt61pci.c | 4 +---
> 1 files changed, 1 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/wireless/rt2x00/rt61pci.c b/drivers/net/wireless/rt2x00/rt61pci.c
> index 687e17d..2b43a2e 100644
> --- a/drivers/net/wireless/rt2x00/rt61pci.c
> +++ b/drivers/net/wireless/rt2x00/rt61pci.c
> @@ -2543,9 +2543,7 @@ static int rt61pci_probe_hw_mode(struct rt2x00_dev *rt2x00dev)
> */
> rt2x00dev->hw->flags =
> IEEE80211_HW_HOST_BROADCAST_PS_BUFFERING |
> - IEEE80211_HW_SIGNAL_DBM |
> - IEEE80211_HW_SUPPORTS_PS |
> - IEEE80211_HW_PS_NULLFUNC_STACK;
> + IEEE80211_HW_SIGNAL_DBM;
>
> SET_IEEE80211_DEV(rt2x00dev->hw, rt2x00dev->dev);
> SET_IEEE80211_PERM_ADDR(rt2x00dev->hw,