Mohammed Shafi Shajakhan wrote: > +#ifdef CONFIG_PM_SLEEP > + > +void ath_wow_cleanup(struct ath_softc *sc) > +{ > + struct ath9k_wow_info *wow_info = &sc->wow_info; > + struct ath9k_wow_pattern *wow_pattern = NULL, *tmp; > + > + if (!(sc->wow_enabled & AH_WOW_USER_PATTERN_EN)) > + return; > + > + list_for_each_entry_safe(wow_pattern, tmp, > + &wow_info->wow_patterns, list) { > + > + list_del(&wow_pattern->list); > + kfree(wow_pattern); > + } > + > +} I am slightly unclear about this... > + u8 dis_deauth_pattern[MAX_PATTERN_SIZE]; > + u8 dis_deauth_mask[MAX_PATTERN_SIZE]; > + > + memset(dis_deauth_pattern, 0, MAX_PATTERN_SIZE); > + memset(dis_deauth_mask, 0, MAX_PATTERN_SIZE); MAX_PATTERN_MASK_SIZE ? > + memset(wow_pattern->pattern_bytes, 0, MAX_PATTERN_SIZE); > + memset(wow_pattern->mask_bytes, 0, MAX_PATTERN_SIZE); The memset() calls are not needed, but maybe I am missing something - why exactly are we maintaining a list of patterns in the driver ? Can't the patterns be retrieved as part of the suspend() callback and just programmed in the HW ? > + > + if (!device_can_wakeup(sc->dev)) { > + ath_dbg(common, WOW, "device_can_wakeup failed, WoW is not enabled\n"); > + ret = 1; > + goto fail_wow; > + } Can this happen ? > + /* > + * we can now sync irq and kill any running tasklets, since we already > + * disabled interrupts and not holding a spin lock > + */ > + synchronize_irq(sc->irq); > + tasklet_kill(&sc->intr_tq); > + > + ath9k_hw_wow_enable(ah, wow_triggers_enabled); > + > + ath9k_ps_restore(sc); > + ath_dbg(common, ANY, "WoW enabled in ath9k\n"); > + sc->wow_sleep_proc_intr = true; This is racy with the ISR, atomic ops can be used instead, since this (along with wow_got_bmiss_intr) are just boolean variables. > +static void ath9k_set_wakeup(struct ieee80211_hw *hw, bool enabled) > +{ > + struct ath_softc *sc = hw->priv; > + > + mutex_lock(&sc->mutex); > + device_init_wakeup(sc->dev, 1); > + device_set_wakeup_enable(sc->dev, enabled); > + mutex_unlock(&sc->mutex); > +} device_init_wakeup() should be part of the probe sequence, I think. Sujith -- 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