Thanks for the review.
All will be fix on v2.
Two comments inline.
regards,
Shahar
Luciano Coelho wrote:
On Thu, 2010-10-07 at 15:06 +0200, ext Shahar Levi wrote:
New acx cmd wl1271_acx_ba_session_policy to set BA policy to the FW.
Macros to use with BA setting.
Signed-off-by: Shahar Levi <shahar_levi@xxxxxx>
---
You can merge this patch with the next patch, no need to make the
changes first in the header files and then in the c files, since they go
very much hand in hand.
drivers/net/wireless/wl12xx/wl1271_acx.h | 16 ++++++++++++++++
drivers/net/wireless/wl12xx/wl1271_conf.h | 3 +++
2 files changed, 19 insertions(+), 0 deletions(-)
diff --git a/drivers/net/wireless/wl12xx/wl1271_acx.h b/drivers/net/wireless/wl12xx/wl1271_acx.h
index b8da1bc..6c3c7c0 100644
--- a/drivers/net/wireless/wl12xx/wl1271_acx.h
+++ b/drivers/net/wireless/wl12xx/wl1271_acx.h
@@ -1055,6 +1055,22 @@ struct wl1271_acx_ht_information {
u8 padding[3];
} __packed;
+/*
+ * BA sessen interface structure
+ */
BA session. Actually you can get rid of this comment, as it is obvious
from the name of the structure that this is about BA sessions.
+struct wl1271_acx_ba_session_policy {
+ struct acx_header header;
+ /* Mac address of: SA as receiver / RA as initiator */
+ u8 mac_address[ETH_ALEN];
Is this really SA for receiver and RA for initiator? Not SA and DA? Or
TA and RA?
yes, as initiator it is the receiver address, as receiver it is source
address. I change the comment to:
Mac address of the peer: SA as receiver / RA as initiator
+ u8 tid; /* TID */
Comment here is unnecessary.
+ u8 policy; /* Enable / Disable */
Could we change policy to enable here? Then the comment can go away too,
because the name of the element will be clear enough already.
+ u16 win_size; /* windows size in num of packet */
"number of packets"
+ u16 inactivity_timeout; /* as initiator inactivity timeout
+ * in time units(TU) of 1024us.
+ * as receiver reserved
+ */
The comment style is wrong.
diff --git a/drivers/net/wireless/wl12xx/wl1271_conf.h b/drivers/net/wireless/wl12xx/wl1271_conf.h
index b3e608e..63a0a9a 100644
--- a/drivers/net/wireless/wl12xx/wl1271_conf.h
+++ b/drivers/net/wireless/wl12xx/wl1271_conf.h
@@ -94,6 +94,9 @@ enum {
#define HW_BG_RATES_MASK 0xffff
#define HW_HT_RATES_OFFSET 16
+#define BA_RECEIVER_WIN_SIZE 8
move to wl1271_acx.h, not configurable.
+#define BA_INACTIVITY_TIMEOUT 10000
This should be added as a real configuration, like the other stuff in
the wl1271_conf.h file, which is then used in wl1271_main.c
default_conf. Please define a struct conf_ba with win_size and
inactivity_timeout elements and set the actual values in the main file.
--
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
