On Tue, 2011-08-09 at 12:13 +0300, Eliad Peller wrote:
> Add wl1271_cmd_role_start_ibss() implementation and defintion.
> This function is used in order to start the IBSS role.
>
> Stopping the IBSS is done by using the same api as stop STA,
> so there is no need for a separate function.
Not really important now, but maybe we should find a more generic name
for the wl1271_cmd_role_stop_sta() function then. Or add a nice comment
there saying that it is also used for bss.
> diff --git a/drivers/net/wireless/wl12xx/cmd.c b/drivers/net/wireless/wl12xx/cmd.c
> index 41dee4f..b8d6848 100644
> --- a/drivers/net/wireless/wl12xx/cmd.c
> +++ b/drivers/net/wireless/wl12xx/cmd.c
> @@ -767,12 +767,73 @@ out_free:
> kfree(cmd);
>
> out:
> return ret;
> }
>
> +int wl1271_cmd_role_start_ibss(struct wl1271 *wl)
> +{
> + struct wl1271_cmd_role_start *cmd;
> + struct ieee80211_bss_conf *bss_conf = &wl->vif->bss_conf;
> + int ret;
> +
> + cmd = kzalloc(sizeof(*cmd), GFP_KERNEL);
> + if (!cmd) {
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> + wl1271_debug(DEBUG_CMD, "cmd role start ibss %d", wl->role_id);
> +
> + cmd->role_id = wl->role_id;
> + if (wl->band == IEEE80211_BAND_5GHZ)
> + cmd->band |= WL1271_BAND_5GHZ;
This looks wrong. I didn't see it before, but it was already introduced
in patch 08. The WL1271_BAND_* definitions are an enumeration defined
like this:
enum wl1271_band {
WL1271_BAND_2_4GHZ = 0, /* 2.4 Ghz band */
WL1271_BAND_5GHZ = 1, /* 5 Ghz band */
WL1271_BAND_JAPAN_4_9_GHZ = 2,
WL1271_BAND_DEFAULT = WL1271_BAND_2_4GHZ,
WL1271_BAND_INVALID = 0x7E,
WL1271_BAND_MAX_RADIO = 0x7F,
};
Why are we always ORing the value into cmd->band then? If these commands
can only work in single bands (which is probably the case), then we
should use "cmd->band = WL1271_BAND_5GHZ" instead. If the commands can
manage more than a band at once, then cmd->band should be a bitmask and
the enumeration should be defined using BIT(0), BIT(1) and so on.
--
Cheers,
Luca.
--
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
