On Sat, 2008-11-22 at 23:41 -0500, Bob Copeland wrote:
> This patch introduces suspend and resume callbacks to mac80211, allowing
> mac80211 to quiesce its state (bringing down interfaces, removing keys, etc)
> in preparation for suspend. The driver is responsible for calling
> ieee80211_notify_mac with the appropriate option from its suspend/resume
> hook.
Thanks! Looks pretty good already, a few comments:
> +void __ieee80211_suspend(struct ieee80211_hw *hw)
> +{
> + struct ieee80211_local *local = hw_to_local(hw);
> + struct ieee80211_sub_if_data *sdata;
> + struct ieee80211_if_init_conf conf;
> + struct sta_info *sta;
> +
> + printk(KERN_DEBUG "mac80211: suspending\n");
> +
> + rtnl_lock();
> +
> + flush_workqueue(local->hw.workqueue);
> +
> + /* disable aggregation and notify that STAs are going away */
I think this should run through interfaces twice, in the first run to
remove keys, then remove stas and then in the second run remove the
interfaces, because otherwise you may be having keys left when the sta
for which the key was has been removed.
> + /* remove all interfaces */
> + list_for_each_entry(sdata, &local->interfaces, list) {
> + ieee80211_disable_keys(sdata);
This needs to take care that the interface is actually up and not a
monitor/ap-vlan interface (those we don't tell the driver about)
> + /* add interfaces */
> + list_for_each_entry(sdata, &local->interfaces, list) {
> +
> + conf.vif = &sdata->vif;
> + conf.type = sdata->vif.type;
> + conf.mac_addr = sdata->dev->dev_addr;
> + res = local->ops->add_interface(hw, &conf);
> +
> + ieee80211_enable_keys(sdata);
Similarly here, make sure the interface is actually up and re-enable the
keys only after adding the stas back.
> + /* setup RTS and frag thresholds */
> + if (local->ops->set_rts_threshold)
> + local->ops->set_rts_threshold(hw, local->rts_threshold);
> +
> + if (local->ops->set_frag_threshold)
> + local->ops->set_frag_threshold(hw,
> + local->fragmentation_threshold);
Heh, I didn't even think of those.
What were you thinking of when you said something is missing? I can't
think of anything right now. And what warnings do you get?
Resume definitely needs to defer to schedule_work(), I think, but that's
a trivial change.
johannes
Attachment:
signature.asc
Description: This is a digitally signed message part
