Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(275)

Issue 6851008: Fix for suspend/resume bug

Created:
9 years, 8 months ago by sukeshs
Modified:
9 years, 8 months ago
Reviewers:
Olof Johansson
CC:
chromium-os-reviews_chromium.org
Visibility:
Public.

Description

Fix for suspend/resume bug Currently, there are 2 callbacks registered with OS for getting notifications when system goes to suspend/resume. Racing between these 2 callbacks is the reason for the failure. 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. Change-Id: Ic29f711eed74d975e6d9558cf93899b85db0678d BUG=chromium-os:13867, chromium-os:12337 TEST= Manually tested the suspend/resume functionality

Patch Set 1 #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+21 lines, -7 lines) Patch
M drivers/staging/brcm80211/brcmfmac/bcmsdh_sdmmc.c View 1 chunk +0 lines, -4 lines 0 comments Download
M drivers/staging/brcm80211/brcmfmac/dhd.h View 2 chunks +6 lines, -0 lines 2 comments Download
M drivers/staging/brcm80211/brcmfmac/dhd_linux.c View 2 chunks +8 lines, -2 lines 0 comments Download
M drivers/staging/brcm80211/brcmfmac/wl_cfg80211.c View 2 chunks +7 lines, -1 line 1 comment Download

Messages

Total messages: 1 (0 generated)
Olof Johansson
9 years, 8 months ago (2011-04-14 18:13:43 UTC) #1
Still need better patch subject and signed-off-by. But code comments below.

http://codereview.chromium.org/6851008/diff/1/drivers/staging/brcm80211/brcmf...
File drivers/staging/brcm80211/brcmfmac/dhd.h (right):

http://codereview.chromium.org/6851008/diff/1/drivers/staging/brcm80211/brcmf...
drivers/staging/brcm80211/brcmfmac/dhd.h:42: #endif
No need to ifdef the include.

http://codereview.chromium.org/6851008/diff/1/drivers/staging/brcm80211/brcmf...
drivers/staging/brcm80211/brcmfmac/dhd.h:131: extern volatile bool
dhd_mmc_suspend;
Extern volatile? Huge red flags. Please move to an appropriate header file, and
why is it volatile?

http://codereview.chromium.org/6851008/diff/1/drivers/staging/brcm80211/brcmf...
File drivers/staging/brcm80211/brcmfmac/wl_cfg80211.c (right):

http://codereview.chromium.org/6851008/diff/1/drivers/staging/brcm80211/brcmf...
drivers/staging/brcm80211/brcmfmac/wl_cfg80211.c:1973: #endif	/* 
defined(CONFIG_PM_SLEEP) */
If you move the variable out of ifdef, you can do away with the ifdefs here too
(same for the below).

Powered by Google App Engine
This is Rietveld 408576698