Hi Bing,
On Thu, Dec 22, 2011 at 11:06, Bing Zhao <bzhao@xxxxxxxxxxx> wrote:
> Hi Julian,
>
> Thanks for your comment.
>
>> Subject: Re: [PATCH 2/3] mwifiex: use IEEE80211_HT_PARAM_CHA_SEC_* macros
>>
>> Hi Bing and Amitkumar,
>>
>> Quick question:
>>
>> On Wed, Dec 21, 2011 at 18:47, Bing Zhao <bzhao@xxxxxxxxxxx> wrote:
>> > From: Amitkumar Karwar <akarwar@xxxxxxxxxxx>
>> >
>> > Replace driver specific macros with the corresponding
>> > IEEE80211_HT_PARAM_CHA_SEC_* macros defined in ieee80211.h.
>> > Also, rename 'adapter->chan_offset' to 'adapter->sec_chan_offset'
>> > for consistency.
>> >
>> > Signed-off-by: Amitkumar Karwar <akarwar@xxxxxxxxxxx>
>> > Signed-off-by: Bing Zhao <bzhao@xxxxxxxxxxx>
>> > ---
>> > diff --git a/drivers/net/wireless/mwifiex/cfg80211.c b/drivers/net/wireless/mwifiex/cfg80211.c
>> > index 64bf640..0723f61 100644
>> > --- a/drivers/net/wireless/mwifiex/cfg80211.c
>> > +++ b/drivers/net/wireless/mwifiex/cfg80211.c
>> > +static u8
>> > +mwifiex_cfg80211_channel_type_to_sec_chan_offset(enum nl80211_channel_type
>> > + channel_type)
>> > {
>> > switch (channel_type) {
>> > case NL80211_CHAN_NO_HT:
>> > case NL80211_CHAN_HT20:
>> > - return NO_SEC_CHANNEL;
>> > + return IEEE80211_HT_PARAM_CHA_SEC_NONE;
>> > case NL80211_CHAN_HT40PLUS:
>> > - return SEC_CHANNEL_ABOVE;
>> > + return IEEE80211_HT_PARAM_CHA_SEC_ABOVE;
>> > case NL80211_CHAN_HT40MINUS:
>> > - return SEC_CHANNEL_BELOW;
>> > + return IEEE80211_HT_PARAM_CHA_SEC_BELOW;
>> > default:
>> > - return NO_SEC_CHANNEL;
>> > + return IEEE80211_HT_PARAM_CHA_SEC_NONE;
>> > }
>> > }
>>
>> Is there a function in mac80211 to do this for you?
>
> The mac80211.h does define inline functions like this:
>
> static inline bool
> conf_is_ht40_minus(struct ieee80211_conf *conf)
> {
> return conf->channel_type == NL80211_CHAN_HT40MINUS;
> }
Ah, that makes things a little more tricky =)
> But they take 'struct ieee80211_conf *conf', instead of 'enum nl80211_channel_type', as the parameters. If I make use of them I will have to construct a redundant 'struct ieee80211_conf' variable in mwifiex driver and add "if (ht40_minus), else if (ht40_plus), else()" logic to get the return values of IEEE80211_HT_PARAM_CHA_SEC_ type.
That would not be a good solution.
My point about this was that there may be other drivers who would want
to use the IEEE80211_HT_PARAM_CHA_SEC_ values so maybe someone (not
necessarily you) should create a common function in mac80211 to switch
between these values, rather than having a function in mwiflex to do
something that isn't driver specific.
>> > @@ -51,20 +51,20 @@ mwifiex_cfg80211_channel_type_to_mwifiex_channels(enum nl80211_channel_type
>> > static enum nl80211_channel_type
>> > mwifiex_channels_to_cfg80211_channel_type(int channel_type)
>> > {
>> > switch (channel_type) {
>> > - case NO_SEC_CHANNEL:
>> > + case IEEE80211_HT_PARAM_CHA_SEC_NONE:
>> > return NL80211_CHAN_HT20;
>> > - case SEC_CHANNEL_ABOVE:
>> > + case IEEE80211_HT_PARAM_CHA_SEC_ABOVE:
>> > return NL80211_CHAN_HT40PLUS;
>> > - case SEC_CHANNEL_BELOW:
>> > + case IEEE80211_HT_PARAM_CHA_SEC_BELOW:
>> > return NL80211_CHAN_HT40MINUS;
>> > default:
>> > return NL80211_CHAN_HT20;
>>
>> Same again.
>
> The macro replacement in this function is temporary. This entire function has been removed in 3/3 in the same patch series.
I didn't read patch 3 that closely so I must have missed that change.
I asked about this for the same reason as above.
Thanks,
--
Julian Calaby
Email: julian.calaby@xxxxxxxxx
Profile: http://www.google.com/profiles/julian.calaby/
.Plan: http://sites.google.com/site/juliancalaby/
--
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
