On Fri, 2007-09-21 at 17:00 -0400, Luis R. Rodriguez wrote:
Ok one thing up front: I'm looking at this basically as "new code", iow
something we should do right from the start. mac80211 stuff is a mess,
so imho we should clean it up at this point when we break things anyway.
> + * @IEEE80211_CHAN_W_SCAN: if this flag is set it informs the lower layer that
> + * this channel can be passively scanned for.
Are there really channels that don't have _SCAN?
> +/**
> + * ieee80211_channel - internal structure definiton for an IEEE-802.11 channel
> + *
> + * This defines an ieee80211_channel. The IEEE-802.11 regulatory domain agent
> + * is in charge of filling most of these fields out. The low-level driver
> + * is expected to fill in, if needed, the val field. Note that val is already
> + * set by the regulatory agent to the same channel as in chan.
Shouldn't you also set chan and freq in the driver?
> + * @modulation_cap: could be IEEE80211_RATE_*
Could be?
> + * @list: lets you make this structure part of a linked list
What do we need this for?
> +struct ieee80211_channel {
> + /* XXX change to u8 */
> + short chan;
Not sure, is a u8 always enough? I thought 802.11a had huge channel
numbers?
> +/* XXX consider removing the holes bellow */
I think these flags are shared with hostapd right now... :(
> +/**
> + * enum rate_flags - internal &ieee80211_rate flags
> + * Use these flags to indicate to the lower layers details about an
> + * &ieee80211_rate.
^ struct ieee80211_rate, for kernel-doc
> + * @IEEE80211_RATE_ERP: indicates if the rate is an Extended Rate PHY (ERP)
> + * @IEEE80211_RATE_BASIC: indicates the rate is a basic rate for the
> + * currently used mode
> + * @IEEE80211_RATE_PREAMBLE2: used to indicates that the rate for short
> + * preamble is to be used. This is set in &ieee80211_rate's @val2.
Couldn't we just call it SHORT_PREAMBLE? :)
> + * @IEEE80211_RATE_SUPPORTED: indicates if rate is supported by the given mode
What do we use this for?
> + * @IEEE80211_RATE_OFDM: indicates support for ODFM modulation
> + * @IEEE80211_RATE_CCK: indicates support for CCK modulation
I think this should be called "this rate is an OFDM/CCK rate"
> + * @IEEE80211_RATE_MANDATORY: indicates if this rate is mandatory for the
> + * currently used mode
I think this is wrong. Isn't the "mandatory" flag something for an AP to
ask it's stations for?
> +/* XXX move to inline and add documenation, kernel-doc can't doc
> defines */
Why don't you just do that then? :)
> + * Low-level driver should set PREAMBLE2, OFDM and CCK flags.
> + * BASIC, SUPPORTED, ERP, and MANDATORY flags are set in 80211.o based on the
> + * configuration.
Oh come on. There hasn't been 80211.o for a loong time :)
> + * @flags: IEEE80211_RATE_* flags
> + * @val2: hw specific value for the rate when using short preamble
> + * (only when IEEE80211_RATE_PREAMBLE2 flag is set, i.e., for
> + * 2, 5.5, and 11 Mbps)
Can't we just get rid of this field? Does anybody use it?
> + * @min_rssi_ack: Minimum required RSSI of packet to generate an ACK ??
> + * @min_rssi_ack_delta: ??
I thought we didn't use these fields.
> + /* the following fields are set by 80211.o and need not be filled by the
80211.o again :)
johannes
Attachment:
signature.asc
Description: This is a digitally signed message part
