Hi Johannes, > -----Original Message----- > From: Johannes Berg [mailto:johannes@xxxxxxxxxxxxxxxx] > Sent: Tuesday, December 07, 2010 1:42 AM > To: Bing Zhao > Cc: linux-wireless@xxxxxxxxxxxxxxx; John W. Linville; Amitkumar Karwar; Kiran Divekar; Frank Huang > Subject: Re: [PATCH] mwifiex: remove some element ID's definitions > > On Mon, 2010-12-06 at 22:16 -0800, Bing Zhao wrote: > > From: Amitkumar Karwar <akarwar@xxxxxxxxxxx> > > > > Those are already defined in include/linux/ieee80211.h > > under "enum ieee80211_eid". > > Good patch :) > > > - enum ieee_types_elementid_e element_id; > > + enum ieee80211_eid element_id; > > You should probably just use a u8, since that'll match better what the > value really is, it can't have any more values than that and an enum > really is an "int" not a "u8". Also lets you remove the typecasts (that > I don't think are necessary to start with) > > > enum ieee_types_elementid_e { > > > BSSCO_2040 = 72, > > OVERLAP_BSS_SCAN_PARAM = 74, > > EXT_CAPABILITY = 127, > > I think you should add these remaining ones into the WLAN_EID_ namespace > instead of keeping them. > > > + WPS_IE = WLAN_EID_VENDOR_SPECIFIC, > > + WMM_IE = WLAN_EID_VENDOR_SPECIFIC, > > And these seem just plain misleading since they don't identify > anything ... one could be tempted to use both in a switch() statement > for example. I'd replace their use with just vendor-specific I think > (although I'm aware that some older code we have in the stack does this > as well, IIRC) Thanks for your comments. I will resend the patch to incorporate all of above. Regards, Bing > > johannes ÿô.nlj·Ÿ®‰†+%ŠË±é¥Šwÿº{.nlj·¥Š{±ÿ«zW¬³ø¡Ü}©ž²ÆzÚj:+v‰¨þø®w¥þŠàÞ¨è&¢)ß«a¶Úÿûz¹ÞúŽŠÝjÿŠwèf
