On Mon, 2010-10-11 at 17:47 +0200, ext Shahar Levi wrote:
> Two acx commands: ht_capabilities & ht_information, 11n sta capabilities macro.
>
> Signed-off-by: Shahar Levi <shahar_levi@xxxxxx>
> ---
A few more minor comments.
> diff --git a/drivers/net/wireless/wl12xx/wl1271_acx.h b/drivers/net/wireless/wl12xx/wl1271_acx.h
> index ebb341d..90082c5 100644
> --- a/drivers/net/wireless/wl12xx/wl1271_acx.h
> +++ b/drivers/net/wireless/wl12xx/wl1271_acx.h
[...]
> + /*
> + * Indicates to which peer these capabilities are relevant.
> + * Note, currently this value will be set to FFFFFFFFFFFF to indicate it
> + * is relevant for all peers since we only support HT in infrastructure
> + * mode. Later on this field will be relevant to IBSS/DLS operation
> + */
> + u8 mac_address[ETH_ALEN];
Okay, I was not very clear, but in my comment to patch 02/03, I said:
"[...]you could actually remove the Note from the wl1271_acx.h file and
leave it here[...]". The reasoning to leave it in the C file is that it
is there that you actually set the mac_address to FFFFFFFFFFFF. So,
remove from the wl1271_acx.h file and put it back on the wl1271_acx.c
file.
> +
> + /*
> + * This the maximum A-MPDU length supported by the AP. The FW may not
> + * exceed this length when sending A-MPDUs
> + */
> + u8 ampdu_max_length;
> +
> + /*
> + * This is the minimal spacing required when sending A-MPDUs to the AP
> + */
No need to have this comment in multiple lines. You could write just
"Minimal spacing..." and then it will fit nicely in one line ;)
> +/* HT Capabilites Fw Bit Mask Mapping */
> +#define WL1271_ACX_FW_CAP_BIT_MASK_HT_OPERATION BIT(0)
> +#define WL1271_ACX_FW_CAP_BIT_MASK_GREENFIELD_FRAME_FORMAT BIT(1)
> +#define WL1271_ACX_FW_CAP_BIT_MASK_SHORT_GI_FOR_20MHZ_PACKETS BIT(2)
> +#define WL1271_ACX_FW_CAP_BIT_MASK_LSIG_TXOP_PROTECTION BIT(3)
> +#define WL1271_ACX_FW_CAP_BIT_MASK_HT_CONTROL_FIELDS BIT(4)
> +#define WL1271_ACX_FW_CAP_BIT_MASK_RD_INITIATION BIT(5)
Repeating: No need for the TABs after the #defines.
> +/*
> + * ACX_HT_BSS_OPERATION
> + * Configure HT capabilities - AP rules for behavior in the BSS.
> + */
> +struct wl1271_acx_ht_information {
> + struct acx_header header;
> +
> + /* Values: 0 - RIFS not allowed, 1 - RIFS allowed */
> + u8 rifs_mode;
> +
> + /* Values: 0 - 3 like in spec */
> + u8 ht_protection;
> +
> + /* Values: 0 - GF protection not required, 1 - GF protection required */
> + u8 gf_protection;
> +
> + /*Values: 0 - TX Burst limit not required, 1 - TX Burst Limit required*/
> + u8 ht_tx_burst_limit;
> +
> + /*
> + * Values: 0 - Dual CTS protection not required,
> + * 1 Dual CTS Protection required
Alignment.
--
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
