On Fri, May 13, 2011 at 2:59 AM, Arend van Spriel <arend@xxxxxxxxxxxx> wrote:
> From: Sukesh Srikakula <sukeshs@xxxxxxxxxxxx>
>
> Currently, there are 2 callbacks registered with OS for getting
> notifications when system goes to suspend/resume. Racing between
> these 2 callbacks results in random suspend failures. With this fix,
> we avoid registering dhd callback for suspend/resume notification
> when cfg80211 is used. Relevent functionality in dhd suspend/resume
> callback function is moved to cfg80211 suspend/resume functions.
>
> Cc: devel@xxxxxxxxxxxxxxxxxxxxxx
> Cc: linux-wireless@xxxxxxxxxxxxxxx
> Cc: Grant Grundler <grundler@xxxxxxxxxxxx>
Thanks guys again! :)
Except for use of atomic_t instead of volatile, this appears to be the same as:
http://codereview.chromium.org/6802002/
which I tested and fixes bug:
http://crosbug.com/12337
thanks,
grant
> Reviewed-by: Franky (Zhenhui) Lin <frankyl@xxxxxxxxxxxx>
> Reviewed-by: Brett Rudley <brudley@xxxxxxxxxxxx>
> Signed-off-by: Arend van Spriel <arend@xxxxxxxxxxxx>
> ---
> Âdrivers/staging/brcm80211/brcmfmac/bcmsdh_sdmmc.c | Â Â5 +-
> Âdrivers/staging/brcm80211/brcmfmac/dhd.h     Â|  22 ++++---
> Âdrivers/staging/brcm80211/brcmfmac/dhd_linux.c  Â|  20 ++++--
> Âdrivers/staging/brcm80211/brcmfmac/wl_cfg80211.c Â| Â 77 ++++++++++++++++++---
> Âdrivers/staging/brcm80211/brcmfmac/wl_cfg80211.h Â| Â Â1 +
> Â5 files changed, 97 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/staging/brcm80211/brcmfmac/bcmsdh_sdmmc.c b/drivers/staging/brcm80211/brcmfmac/bcmsdh_sdmmc.c
> index 43aebfd..c0ffbd3 100644
> --- a/drivers/staging/brcm80211/brcmfmac/bcmsdh_sdmmc.c
> +++ b/drivers/staging/brcm80211/brcmfmac/bcmsdh_sdmmc.c
> @@ -26,14 +26,11 @@
> Â#include <linux/mmc/core.h>
> Â#include <linux/mmc/sdio_func.h>
> Â#include <linux/mmc/sdio_ids.h>
> +#include <linux/suspend.h>
>
> Â#include <dngl_stats.h>
> Â#include <dhd.h>
>
> -#if defined(CONFIG_PM_SLEEP)
> -#include <linux/suspend.h>
> -extern volatile bool dhd_mmc_suspend;
> -#endif
> Â#include "bcmsdh_sdmmc.h"
>
> Âextern int sdio_function_init(void);
> diff --git a/drivers/staging/brcm80211/brcmfmac/dhd.h b/drivers/staging/brcm80211/brcmfmac/dhd.h
> index 99c38dd..a726b49 100644
> --- a/drivers/staging/brcm80211/brcmfmac/dhd.h
> +++ b/drivers/staging/brcm80211/brcmfmac/dhd.h
> @@ -31,6 +31,7 @@
> Â#include <linux/random.h>
> Â#include <linux/spinlock.h>
> Â#include <linux/ethtool.h>
> +#include <linux/suspend.h>
> Â#include <asm/uaccess.h>
> Â#include <asm/unaligned.h>
> Â/* The kernel threading is sdio-specific */
> @@ -122,19 +123,22 @@ typedef struct dhd_pub {
> Â} dhd_pub_t;
>
> Â#if defined(CONFIG_PM_SLEEP)
> -
> +extern atomic_t dhd_mmc_suspend;
> Â#define DHD_PM_RESUME_WAIT_INIT(a) DECLARE_WAIT_QUEUE_HEAD(a);
> -#define _DHD_PM_RESUME_WAIT(a, b) do {\
> - Â Â Â Â Â Â Â Â Â Â Â int retry = 0; \
> - Â Â Â Â Â Â Â Â Â Â Â while (dhd_mmc_suspend && retry++ != b) { \
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â wait_event_timeout(a, false, HZ/100); \
> - Â Â Â Â Â Â Â Â Â Â Â } \
> - Â Â Â Â Â Â Â } Â Â Â while (0)
> +#define _DHD_PM_RESUME_WAIT(a, b) do { \
> + Â Â Â Â Â Â Â int retry = 0; \
> + Â Â Â Â Â Â Â while (atomic_read(&dhd_mmc_suspend) && retry++ != b) { \
> + Â Â Â Â Â Â Â Â Â Â Â wait_event_timeout(a, false, HZ/100); \
> + Â Â Â Â Â Â Â } \
> + Â Â Â } Â Â Â while (0)
> Â#define DHD_PM_RESUME_WAIT(a) Â_DHD_PM_RESUME_WAIT(a, 30)
> Â#define DHD_PM_RESUME_WAIT_FOREVER(a) Â_DHD_PM_RESUME_WAIT(a, ~0)
> Â#define DHD_PM_RESUME_RETURN_ERROR(a) Â\
> - Â Â Â do { if (dhd_mmc_suspend) return a; } while (0)
> -#define DHD_PM_RESUME_RETURN Â do { if (dhd_mmc_suspend) return; } while (0)
> + Â Â Â do { if (atomic_read(&dhd_mmc_suspend)) return a; } while (0)
> +#define DHD_PM_RESUME_RETURN Â do { \
> + Â Â Â if (atomic_read(&dhd_mmc_suspend)) \
> + Â Â Â Â Â Â Â return; \
> + Â Â Â } while (0)
>
> Â#define DHD_SPINWAIT_SLEEP_INIT(a) DECLARE_WAIT_QUEUE_HEAD(a);
> Â#define SPINWAIT_SLEEP(a, exp, us) do { \
> diff --git a/drivers/staging/brcm80211/brcmfmac/dhd_linux.c b/drivers/staging/brcm80211/brcmfmac/dhd_linux.c
> index ba73ce0..31a5ca0 100644
> --- a/drivers/staging/brcm80211/brcmfmac/dhd_linux.c
> +++ b/drivers/staging/brcm80211/brcmfmac/dhd_linux.c
> @@ -166,7 +166,7 @@ void wifi_del_dev(void)
>
> Â#if defined(CONFIG_PM_SLEEP)
> Â#include <linux/suspend.h>
> -volatile bool dhd_mmc_suspend = false;
> +atomic_t dhd_mmc_suspend;
> ÂDECLARE_WAIT_QUEUE_HEAD(dhd_dpc_wait);
> Â#endif /* Âdefined(CONFIG_PM_SLEEP) */
>
> @@ -407,11 +407,11 @@ static int dhd_sleep_pm_callback(struct notifier_block *nfb,
> Â Â Â Âswitch (action) {
> Â Â Â Âcase PM_HIBERNATION_PREPARE:
> Â Â Â Âcase PM_SUSPEND_PREPARE:
> - Â Â Â Â Â Â Â dhd_mmc_suspend = true;
> + Â Â Â Â Â Â Â atomic_set(&dhd_mmc_suspend, true);
> Â Â Â Â Â Â Â Âreturn NOTIFY_OK;
> Â Â Â Âcase PM_POST_HIBERNATION:
> Â Â Â Âcase PM_POST_SUSPEND:
> - Â Â Â Â Â Â Â dhd_mmc_suspend = false;
> + Â Â Â Â Â Â Â atomic_set(&dhd_mmc_suspend, false);
> Â Â Â Â Â Â Â Âreturn NOTIFY_OK;
> Â Â Â Â}
> Â Â Â Âreturn 0;
> @@ -2011,7 +2011,9 @@ dhd_pub_t *dhd_attach(struct dhd_bus *bus, uint bus_hdrlen)
> Â Â Â Âg_bus = bus;
> Â#endif
> Â#if defined(CONFIG_PM_SLEEP)
> - Â Â Â register_pm_notifier(&dhd_sleep_pm_notifier);
> + Â Â Â atomic_set(&dhd_mmc_suspend, false);
> + Â Â Â if (!IS_CFG80211_FAVORITE())
> + Â Â Â Â Â Â Â register_pm_notifier(&dhd_sleep_pm_notifier);
> Â#endif /* defined(CONFIG_PM_SLEEP) */
> Â Â Â Â/* && defined(DHD_GPL) */
> Â Â Â Â/* Init lock suspend to prevent kernel going to suspend */
> @@ -2305,7 +2307,8 @@ void dhd_detach(dhd_pub_t *dhdp)
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âwl_cfg80211_detach();
>
> Â#if defined(CONFIG_PM_SLEEP)
> - Â Â Â Â Â Â Â Â Â Â Â unregister_pm_notifier(&dhd_sleep_pm_notifier);
> + Â Â Â Â Â Â Â Â Â Â Â if (!IS_CFG80211_FAVORITE())
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â unregister_pm_notifier(&dhd_sleep_pm_notifier);
> Â#endif /* defined(CONFIG_PM_SLEEP) */
> Â Â Â Â Â Â Â Â Â Â Â Â/* && defined(DHD_GPL) */
> Â Â Â Â Â Â Â Â Â Â Â Âfree_netdev(ifp->net);
> @@ -2816,6 +2819,13 @@ int dhd_wait_pend8021x(struct net_device *dev)
> Â Â Â Âreturn pend;
> Â}
>
> +void wl_os_wd_timer(struct net_device *ndev, uint wdtick)
> +{
> + Â Â Â dhd_info_t *dhd = *(dhd_info_t **)netdev_priv(ndev);
> +
> + Â Â Â dhd_os_wd_timer(&dhd->pub, wdtick);
> +}
> +
> Â#ifdef DHD_DEBUG
> Âint write_to_file(dhd_pub_t *dhd, u8 *buf, int size)
> Â{
> diff --git a/drivers/staging/brcm80211/brcmfmac/wl_cfg80211.c b/drivers/staging/brcm80211/brcmfmac/wl_cfg80211.c
> index c60fc7c..2d67048 100644
> --- a/drivers/staging/brcm80211/brcmfmac/wl_cfg80211.c
> +++ b/drivers/staging/brcm80211/brcmfmac/wl_cfg80211.c
> @@ -1969,34 +1969,91 @@ wl_cfg80211_set_bitrate_mask(struct wiphy *wiphy, struct net_device *dev,
>
> Âstatic s32 wl_cfg80211_resume(struct wiphy *wiphy)
> Â{
> - Â Â Â s32 err = 0;
> + Â Â Â struct wl_priv *wl = wiphy_to_wl(wiphy);
> + Â Â Â struct net_device *ndev = wl_to_ndev(wl);
>
> - Â Â Â CHECK_SYS_UP();
> - Â Â Â wl_invoke_iscan(wiphy_to_wl(wiphy));
> + Â Â Â /*
> + Â Â Â Â* Check for WL_STATUS_READY before any function call which
> + Â Â Â Â* could result is bus access. Don't block the resume for
> + Â Â Â Â* any driver error conditions
> + Â Â Â Â*/
>
> - Â Â Â return err;
> +#if defined(CONFIG_PM_SLEEP)
> + Â Â Â atomic_set(&dhd_mmc_suspend, false);
> +#endif /* Âdefined(CONFIG_PM_SLEEP) */
> +
> + Â Â Â if (test_bit(WL_STATUS_READY, &wl->status)) {
> + Â Â Â Â Â Â Â /* Turn on Watchdog timer */
> + Â Â Â Â Â Â Â wl_os_wd_timer(ndev, dhd_watchdog_ms);
> + Â Â Â Â Â Â Â wl_invoke_iscan(wiphy_to_wl(wiphy));
> + Â Â Â }
> +
> + Â Â Â return 0;
> Â}
>
> Âstatic s32 wl_cfg80211_suspend(struct wiphy *wiphy)
> Â{
> Â Â Â Âstruct wl_priv *wl = wiphy_to_wl(wiphy);
> Â Â Â Âstruct net_device *ndev = wl_to_ndev(wl);
> - Â Â Â s32 err = 0;
> +
> +
> + Â Â Â /*
> + Â Â Â Â* Check for WL_STATUS_READY before any function call which
> + Â Â Â Â* could result is bus access. Don't block the suspend for
> + Â Â Â Â* any driver error conditions
> + Â Â Â Â*/
> +
> + Â Â Â /*
> + Â Â Â Â* While going to suspend if associated with AP disassociate
> + Â Â Â Â* from AP to save power while system is in suspended state
> + Â Â Â Â*/
> + Â Â Â if (test_bit(WL_STATUS_CONNECTED, &wl->status) &&
> + Â Â Â Â Â Â Â test_bit(WL_STATUS_READY, &wl->status)) {
> + Â Â Â Â Â Â Â WL_INFO("Disassociating from AP"
> + Â Â Â Â Â Â Â Â Â Â Â " while entering suspend state\n");
> + Â Â Â Â Â Â Â wl_link_down(wl);
> +
> + Â Â Â Â Â Â Â /*
> + Â Â Â Â Â Â Â Â* Make sure WPA_Supplicant receives all the event
> + Â Â Â Â Â Â Â Â* generated due to DISASSOC call to the fw to keep
> + Â Â Â Â Â Â Â Â* the state fw and WPA_Supplicant state consistent
> + Â Â Â Â Â Â Â Â*/
> + Â Â Â Â Â Â Â rtnl_unlock();
> + Â Â Â Â Â Â Â wl_delay(500);
> + Â Â Â Â Â Â Â rtnl_lock();
> + Â Â Â }
>
> Â Â Â Âset_bit(WL_STATUS_SCAN_ABORTING, &wl->status);
> - Â Â Â wl_term_iscan(wl);
> + Â Â Â if (test_bit(WL_STATUS_READY, &wl->status))
> + Â Â Â Â Â Â Â wl_term_iscan(wl);
> +
> Â Â Â Âif (wl->scan_request) {
> - Â Â Â Â Â Â Â cfg80211_scan_done(wl->scan_request, true); Â Â /* true means
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âabort */
> - Â Â Â Â Â Â Â wl_set_mpc(ndev, 1);
> + Â Â Â Â Â Â Â /* Indidate scan abort to cfg80211 layer */
> + Â Â Â Â Â Â Â WL_INFO("Terminating scan in progress\n");
> + Â Â Â Â Â Â Â cfg80211_scan_done(wl->scan_request, true);
> Â Â Â Â Â Â Â Âwl->scan_request = NULL;
> Â Â Â Â}
> Â Â Â Âclear_bit(WL_STATUS_SCANNING, &wl->status);
> Â Â Â Âclear_bit(WL_STATUS_SCAN_ABORTING, &wl->status);
> + Â Â Â clear_bit(WL_STATUS_CONNECTING, &wl->status);
> + Â Â Â clear_bit(WL_STATUS_CONNECTED, &wl->status);
>
> + Â Â Â /* Inform SDIO stack not to switch off power to the chip */
> Â Â Â Âsdioh_sdio_set_host_pm_flags(MMC_PM_KEEP_POWER);
>
> - Â Â Â return err;
> + Â Â Â /* Turn off watchdog timer */
> + Â Â Â if (test_bit(WL_STATUS_READY, &wl->status)) {
> + Â Â Â Â Â Â Â WL_INFO("Terminate watchdog timer and enable MPC\n");
> + Â Â Â Â Â Â Â wl_set_mpc(ndev, 1);
> + Â Â Â Â Â Â Â wl_os_wd_timer(ndev, 0);
> + Â Â Â }
> +
> +#if defined(CONFIG_PM_SLEEP)
> + Â Â Â atomic_set(&dhd_mmc_suspend, true);
> +#endif /* Âdefined(CONFIG_PM_SLEEP) */
> +
> +
> + Â Â Â return 0;
> Â}
>
> Âstatic __used s32
> diff --git a/drivers/staging/brcm80211/brcmfmac/wl_cfg80211.h b/drivers/staging/brcm80211/brcmfmac/wl_cfg80211.h
> index 5112160..3c8b902 100644
> --- a/drivers/staging/brcm80211/brcmfmac/wl_cfg80211.h
> +++ b/drivers/staging/brcm80211/brcmfmac/wl_cfg80211.h
> @@ -375,5 +375,6 @@ extern s8 *wl_cfg80211_get_fwname(void); Â Â/* get firmware name for
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â the dongle */
> Âextern s8 *wl_cfg80211_get_nvramname(void); Â Â/* get nvram name for
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â the dongle */
> +extern void wl_os_wd_timer(struct net_device *ndev, uint wdtick);
>
> Â#endif             /* _wl_cfg80211_h_ */
> --
> 1.7.4.1
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
