Re: [PATCH 3/8] mac80211: enable driver to notify mac of status, change iwlwifi

[Johannes Berg <johannes@sipsolutions.net>]

[-- Attachment #1: Type: text/plain, Size: 1655 bytes --]


>  /**
> + * enum ieee80211_notification_type - Low level driver notification
> + * @TYPE_RE_ASSOC: start the re-association sequence

That I can understand, for whatever reason you want it to re-associate.
But please use a different prefix, not TYPE_, maybe something like
IEEE80211_NOTIFY_*

> + * @TYPE_DISCONNECT: disconnect from current association

That doesn't seem to make sense.

> + * @TYPE_RESET: low level driver HW problem reset

Nor that? Half-implemented?

Do you think suspend/resume should go through this interface as well? If
so, a hardware reset could just do a suspend/resume cycle and ignore
everything coming in during the suspend?

During suspend, I'd think the hardware should actually be deconfigured
completely and then re-configured completely for resume.

> +/*

Should be /**

> + * ieee80211_notify_mac - low level driver notification
> + * @hw: pointer as obtained from ieee80211_alloc_hw().
> + * @notification_types: enum ieee80211_notification_types
> + *
> + * This function must be called by low level driver to sync mac80211
> + * with low level state.
> + */
> +void ieee80211_notify_mac(struct ieee80211_hw *hw,
> +			  enum ieee80211_notification_types  notif_type);

I also think that the description could be more detailed. This seems to
indicate that somehow it's easy to get out of sync, it shouldn't be the
common case for that to happen!

> +	default:
> +		break;
> +	}
> +}
> +EXPORT_SYMBOL(ieee80211_notify_mac);

Please don't add default statements like that, just handle all cases,
that allows the compiler warnings to actually help coding.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]