[PATCH] staging: brcm80211: fix suspend/resume issue in brcmsmac

[PATCH] staging: brcm80211: fix suspend/resume issue in brcmsmac

[Arend van Spriel]

PCI PM suspend callback took down the interface and resume brought
it back up. In the mac80211 context this is done in subsequent calls.
Rework implementation so that suspend only stores config, and sets
PCI power state. The resume return to full power state (D0), restores
the config, and brings hardware back up. Full bringup is done by
subsequent mac80211 calls.

Reviewed-by: Brett Rudley <brudley@broadcom.com>
Signed-off-by: Arend van Spriel <arend@broadcom.com>
---
 drivers/staging/brcm80211/brcmsmac/wl_mac80211.c |   28 ++++++++++++---------
 drivers/staging/brcm80211/brcmsmac/wl_mac80211.h |    4 +--
 2 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/drivers/staging/brcm80211/brcmsmac/wl_mac80211.c b/drivers/staging/brcm80211/brcmsmac/wl_mac80211.c
index c2663bd..8f11409 100644
--- a/drivers/staging/brcm80211/brcmsmac/wl_mac80211.c
+++ b/drivers/staging/brcm80211/brcmsmac/wl_mac80211.c
@@ -179,7 +179,6 @@ static void wl_ops_stop(struct ieee80211_hw *hw)
 	struct wl_info *wl = hw->priv;
 	ASSERT(wl);
 	WL_LOCK(wl);
-	wl_down(wl);
 	ieee80211_stop_queues(hw);
 	WL_UNLOCK(wl);
 
@@ -216,6 +215,14 @@ wl_ops_add_interface(struct ieee80211_hw *hw, struct ieee80211_vif *vif)
 static void
 wl_ops_remove_interface(struct ieee80211_hw *hw, struct ieee80211_vif *vif)
 {
+	struct wl_info *wl;
+
+	wl = HW_TO_WL(hw);
+
+	/* put driver in down state */
+	WL_LOCK(wl);
+	wl_down(wl);
+	WL_UNLOCK(wl);
 	return;
 }
 
@@ -1059,7 +1066,6 @@ wl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	return 0;
 }
 
-#ifdef LINUXSTA_PS
 static int wl_suspend(struct pci_dev *pdev, pm_message_t state)
 {
 	struct wl_info *wl;
@@ -1074,11 +1080,12 @@ static int wl_suspend(struct pci_dev *pdev, pm_message_t state)
 		return -ENODEV;
 	}
 
+	/* only need to flag hw is down for proper resume */
 	WL_LOCK(wl);
-	wl_down(wl);
 	wl->pub->hw_up = false;
 	WL_UNLOCK(wl);
-	pci_save_state(pdev, wl->pci_psstate);
+
+	pci_save_state(pdev);
 	pci_disable_device(pdev);
 	return pci_set_power_state(pdev, PCI_D3hot);
 }
@@ -1102,7 +1109,7 @@ static int wl_resume(struct pci_dev *pdev)
 	if (err)
 		return err;
 
-	pci_restore_state(pdev, wl->pci_psstate);
+	pci_restore_state(pdev);
 
 	err = pci_enable_device(pdev);
 	if (err)
@@ -1114,13 +1121,12 @@ static int wl_resume(struct pci_dev *pdev)
 	if ((val & 0x0000ff00) != 0)
 		pci_write_config_dword(pdev, 0x40, val & 0xffff00ff);
 
-	WL_LOCK(wl);
-	err = wl_up(wl);
-	WL_UNLOCK(wl);
-
+	/*
+	*  done. driver will be put in up state
+	*  in wl_ops_add_interface() call.
+	*/
 	return err;
 }
-#endif				/* LINUXSTA_PS */
 
 static void wl_remove(struct pci_dev *pdev)
 {
@@ -1155,10 +1161,8 @@ static void wl_remove(struct pci_dev *pdev)
 static struct pci_driver wl_pci_driver = {
 	.name  = KBUILD_MODNAME,
 	.probe = wl_pci_probe,
-#ifdef LINUXSTA_PS
 	.suspend = wl_suspend,
 	.resume  = wl_resume,
-#endif				/* LINUXSTA_PS */
 	.remove   = __devexit_p(wl_remove),
 	.id_table = wl_id_table,
 };
diff --git a/drivers/staging/brcm80211/brcmsmac/wl_mac80211.h b/drivers/staging/brcm80211/brcmsmac/wl_mac80211.h
index 070fa94..f6b7354 100644
--- a/drivers/staging/brcm80211/brcmsmac/wl_mac80211.h
+++ b/drivers/staging/brcm80211/brcmsmac/wl_mac80211.h
@@ -67,9 +67,7 @@ struct wl_info {
 	struct wl_timer *timers;	/* timer cleanup queue */
 	struct tasklet_struct tasklet;	/* dpc tasklet */
 	bool resched;		/* dpc needs to be and is rescheduled */
-#ifdef LINUXSTA_PS
-	u32 pci_psstate[16];	/* pci ps-state save/restore */
-#endif
+
 	/* RPC, handle, lock, txq, workitem */
 	uint stats_id;		/* the current set of stats */
 	/* ping-pong stats counters updated by Linux watchdog */
-- 
1.7.1

Re: [PATCH] staging: brcm80211: fix suspend/resume issue in brcmsmac

[Greg KH]

On Wed, Jan 12, 2011 at 06:59:27PM +0100, Arend van Spriel wrote:
> PCI PM suspend callback took down the interface and resume brought
> it back up. In the mac80211 context this is done in subsequent calls.
> Rework implementation so that suspend only stores config, and sets
> PCI power state. The resume return to full power state (D0), restores
> the config, and brings hardware back up. Full bringup is done by
> subsequent mac80211 calls.

Ah, does this solve the suspend/resume bug that a number of people have
reported for the driver since the .36 release?  If so, should it go into
the kernel now and be backported to older releases?

thanks,

greg k-h

Re: [PATCH] staging: brcm80211: fix suspend/resume issue in brcmsmac

[Henry Ptasinski]

On Wed, Jan 12, 2011 at 10:08:29AM -0800, Greg KH wrote:
> On Wed, Jan 12, 2011 at 06:59:27PM +0100, Arend van Spriel wrote:
> > PCI PM suspend callback took down the interface and resume brought
> > it back up. In the mac80211 context this is done in subsequent calls.
> > Rework implementation so that suspend only stores config, and sets
> > PCI power state. The resume return to full power state (D0), restores
> > the config, and brings hardware back up. Full bringup is done by
> > subsequent mac80211 calls.
> 
> Ah, does this solve the suspend/resume bug that a number of people have
> reported for the driver since the .36 release?  If so, should it go into
> the kernel now and be backported to older releases?


Yes, this should solve the suspend/resume bug that people have reported. 
(It doesn't solve the rfkill issues; Arend is working on a separate fix for
that.)

It should get backported, but it would be nice to get some confirmation from
people that it fixes the issues they're seeing first.

- Henry

Re: [PATCH] staging: brcm80211: fix suspend/resume issue in brcmsmac

[Greg KH]

On Wed, Jan 12, 2011 at 11:44:39AM -0800, Henry Ptasinski wrote:
> On Wed, Jan 12, 2011 at 10:08:29AM -0800, Greg KH wrote:
> > On Wed, Jan 12, 2011 at 06:59:27PM +0100, Arend van Spriel wrote:
> > > PCI PM suspend callback took down the interface and resume brought
> > > it back up. In the mac80211 context this is done in subsequent calls.
> > > Rework implementation so that suspend only stores config, and sets
> > > PCI power state. The resume return to full power state (D0), restores
> > > the config, and brings hardware back up. Full bringup is done by
> > > subsequent mac80211 calls.
> > 
> > Ah, does this solve the suspend/resume bug that a number of people have
> > reported for the driver since the .36 release?  If so, should it go into
> > the kernel now and be backported to older releases?
> 
> 
> Yes, this should solve the suspend/resume bug that people have reported. 
> (It doesn't solve the rfkill issues; Arend is working on a separate fix for
> that.)
> 
> It should get backported, but it would be nice to get some confirmation from
> people that it fixes the issues they're seeing first.

Ok, that would be good to have :)

thanks,

greg k-h

Re: [PATCH] staging: brcm80211: fix suspend/resume issue in brcmsmac

[Greg KH]

On Wed, Jan 12, 2011 at 06:59:27PM +0100, Arend van Spriel wrote:
> PCI PM suspend callback took down the interface and resume brought
> it back up. In the mac80211 context this is done in subsequent calls.
> Rework implementation so that suspend only stores config, and sets
> PCI power state. The resume return to full power state (D0), restores
> the config, and brings hardware back up. Full bringup is done by
> subsequent mac80211 calls.
> 
> Reviewed-by: Brett Rudley <brudley@broadcom.com>
> Signed-off-by: Arend van Spriel <arend@broadcom.com>
> ---
>  drivers/staging/brcm80211/brcmsmac/wl_mac80211.c |   28 ++++++++++++---------
>  drivers/staging/brcm80211/brcmsmac/wl_mac80211.h |    4 +--
>  2 files changed, 17 insertions(+), 15 deletions(-)

This patch doesn't apply to Linus's tree, which is where it needs to go
right now.  Can you please redo it so that I can apply it?

thanks,

greg k-h

RE: [PATCH] staging: brcm80211: fix suspend/resume issue in brcmsmac

[Arend Van Spriel]

Hi Greg,

I did not retest the suspend/resume change functionally (only compiled it). I decided
to do that on v2.6.38-rc1 to be sure and on my system the suspend works. However,
upon resume my system reboots without any feedback. System seem to come to
live (DVD spinning) and then BIOS boot screen pops up. So I tried suspend/resume
without our driver loaded. Results were the same. Also 2.6.37 shows same behaviour.

On 2.6.37-rc5 I did not have any issues and suspend/resume worked for our driver
and the kernel in general. Anyone else observed such behaviour?

Gr. AvS
________________________________________
From: linux-wireless-owner@vger.kernel.org [linux-wireless-owner@vger.kernel.org] On Behalf Of Greg KH [greg@kroah.com]
Sent: Thursday, January 20, 2011 9:17 PM
To: Arend Van Spriel
Cc: gregkh@suse.de; devel@linuxdriverproject.org; linux-wireless@vger.kernel.org
Subject: Re: [PATCH] staging: brcm80211: fix suspend/resume issue in brcmsmac

On Wed, Jan 12, 2011 at 06:59:27PM +0100, Arend van Spriel wrote:
> PCI PM suspend callback took down the interface and resume brought
> it back up. In the mac80211 context this is done in subsequent calls.
> Rework implementation so that suspend only stores config, and sets
> PCI power state. The resume return to full power state (D0), restores
> the config, and brings hardware back up. Full bringup is done by
> subsequent mac80211 calls.
>
> Reviewed-by: Brett Rudley <brudley@broadcom.com>
> Signed-off-by: Arend van Spriel <arend@broadcom.com>
> ---
>  drivers/staging/brcm80211/brcmsmac/wl_mac80211.c |   28 ++++++++++++---------
>  drivers/staging/brcm80211/brcmsmac/wl_mac80211.h |    4 +--
>  2 files changed, 17 insertions(+), 15 deletions(-)

This patch doesn't apply to Linus's tree, which is where it needs to go
right now.  Can you please redo it so that I can apply it?

thanks,

greg k-h
--
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

Re: [PATCH] staging: brcm80211: fix suspend/resume issue in brcmsmac

[Greg KH]

On Fri, Jan 21, 2011 at 06:37:54AM -0800, Arend Van Spriel wrote:
> Hi Greg,
> 
> I did not retest the suspend/resume change functionally (only compiled it). I decided
> to do that on v2.6.38-rc1 to be sure and on my system the suspend works. However,
> upon resume my system reboots without any feedback. System seem to come to
> live (DVD spinning) and then BIOS boot screen pops up. So I tried suspend/resume
> without our driver loaded. Results were the same. Also 2.6.37 shows same behaviour.
> 
> On 2.6.37-rc5 I did not have any issues and suspend/resume worked for our driver
> and the kernel in general. Anyone else observed such behaviour?

Yes, lots of other people reported problems with this, I thought you got
the reports (like from Jon Masters?)

thanks,

greg k-h

RE: [PATCH] staging: brcm80211: fix suspend/resume issue in brcmsmac

[Arend Van Spriel]

Hi Greg,

I think you miss my point. I currently have a suspend/resume issue with 2.6.37 and 2.6.38-rc1
that is not related to our driver as I did suspend/resume without loading our driver module.

Gr. AvS
________________________________________
From: Greg KH [gregkh@suse.de]
Sent: Friday, January 21, 2011 4:12 PM
To: Arend Van Spriel
Cc: Greg KH; devel@linuxdriverproject.org; linux-wireless@vger.kernel.org
Subject: Re: [PATCH] staging: brcm80211: fix suspend/resume issue in brcmsmac

On Fri, Jan 21, 2011 at 06:37:54AM -0800, Arend Van Spriel wrote:
> Hi Greg,
>
> I did not retest the suspend/resume change functionally (only compiled it). I decided
> to do that on v2.6.38-rc1 to be sure and on my system the suspend works. However,
> upon resume my system reboots without any feedback. System seem to come to
> live (DVD spinning) and then BIOS boot screen pops up. So I tried suspend/resume
> without our driver loaded. Results were the same. Also 2.6.37 shows same behaviour.
>
> On 2.6.37-rc5 I did not have any issues and suspend/resume worked for our driver
> and the kernel in general. Anyone else observed such behaviour?

Yes, lots of other people reported problems with this, I thought you got
the reports (like from Jon Masters?)

thanks,

greg k-h