diff mbox

[RFC] rt61pci: fix module reloading with power saving enabled

Message ID 1248846761.18329.20.camel@ct (mailing list archive)
State RFC, archived
Headers show

Commit Message

Pavel Roskin July 29, 2009, 5:52 a.m. UTC
Unloading rt61pci can leave the device in such state that reloading
rt61pci would fail to reinitialize it.  Bogus data would be read from
the EEPROM and the RF version won't be recognized.  Only reboot would
return the device to a sane state.

It appears that it happens if rt61pci is unloaded while power saving is
active.  To initialize the device properly, SOFT_RESET_CSR should be set
to the same value as rt61pci_config_ps() uses to wake up the device.

Signed-off-by: Pavel Roskin <proski@gnu.org>
---

I don't know what 0x00000007 is and I don't want to make up a name for
it.

rt61pci_config_ps() does other things, but this is the minimal required
change.  If anyone has a specification for rt61, it should mention
SOFT_RESET_CSR and possibly other registers to be set before EEPROM may
be accessed.

Please note that unloading rt61pci (as well as rt73usb) can trigger an
oops or panic in sysfs code that I didn't have a chance to fix yet.
It's very elusive and manifests differently every time.  Fortunately
(for everyone not debugging it), it's rare.


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

Comments

Ivo van Doorn July 30, 2009, 6:47 p.m. UTC | #1
Hi,

On Wednesday 29 July 2009, Pavel Roskin wrote:
> Unloading rt61pci can leave the device in such state that reloading
> rt61pci would fail to reinitialize it.  Bogus data would be read from
> the EEPROM and the RF version won't be recognized.  Only reboot would
> return the device to a sane state.
> 
> It appears that it happens if rt61pci is unloaded while power saving is
> active.  To initialize the device properly, SOFT_RESET_CSR should be set
> to the same value as rt61pci_config_ps() uses to wake up the device.
> 
> Signed-off-by: Pavel Roskin <proski@gnu.org>
> ---
> 
> I don't know what 0x00000007 is and I don't want to make up a name for
> it.

I'll look it up in the specification document.

> rt61pci_config_ps() does other things, but this is the minimal required
> change.  If anyone has a specification for rt61, it should mention
> SOFT_RESET_CSR and possibly other registers to be set before EEPROM may
> be accessed.
> 
> Please note that unloading rt61pci (as well as rt73usb) can trigger an
> oops or panic in sysfs code that I didn't have a chance to fix yet.
> It's very elusive and manifests differently every time.  Fortunately
> (for everyone not debugging it), it's rare.
> 
> 
>  drivers/net/wireless/rt2x00/rt61pci.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/net/wireless/rt2x00/rt61pci.c b/drivers/net/wireless/rt2x00/rt61pci.c
> index fb95b8c..f615fe6 100644
> --- a/drivers/net/wireless/rt2x00/rt61pci.c
> +++ b/drivers/net/wireless/rt2x00/rt61pci.c
> @@ -2600,6 +2600,8 @@ static int rt61pci_probe_hw(struct rt2x00_dev *rt2x00dev)
>  {
>  	int retval;
>  
> +	rt2x00pci_register_write(rt2x00dev, SOFT_RESET_CSR, 0x00000007);
> +
>  	/*
>  	 * Allocate eeprom data.
>  	 */

The only comment about the patch at this time is that it might require a nice comment. ;)
I'll also think that perhaps a more generic approach is required where rt2x00lib forces
PS to be disabled before probe_hw(), just in case other hardware also have these problems.

Thanks,

Ivo

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/net/wireless/rt2x00/rt61pci.c b/drivers/net/wireless/rt2x00/rt61pci.c
index fb95b8c..f615fe6 100644
--- a/drivers/net/wireless/rt2x00/rt61pci.c
+++ b/drivers/net/wireless/rt2x00/rt61pci.c
@@ -2600,6 +2600,8 @@  static int rt61pci_probe_hw(struct rt2x00_dev *rt2x00dev)
 {
 	int retval;
 
+	rt2x00pci_register_write(rt2x00dev, SOFT_RESET_CSR, 0x00000007);
+
 	/*
 	 * Allocate eeprom data.
 	 */