On Monday 06 February 2012 07:44 PM, Kalle Valo wrote:
On 02/06/2012 03:56 PM, rmani@xxxxxxxxxxxxxxxx wrote:
From: Raja Mani<rmani@xxxxxxxxxxxxxxxx>
* TX operation (ctrl tx and data tx) has to be controlled based on
WOW suspend state. i.e, control packets are allowed to send from
the host until the suspend state goes ATH6KL_STATE_WOW and
the data packets are allowed until WOW suspend operation starts.
* Similary, wow resume is NOT allowed if WOW suspend is in progress.
Both of the above scenarios are taken care in this patch.
Signed-off-by: Raja Mani<rmani@xxxxxxxxxxxxxxxx>
[...]
+enum ath6kl_wow_state {
+ ATH6KL_WOW_STATE_NONE,
+ ATH6KL_WOW_STATE_SUSPENDING,
+ ATH6KL_WOW_STATE_SUSPENDED,
+};
+
struct ath6kl {
struct device *dev;
struct wiphy *wiphy;
enum ath6kl_state state;
+ enum ath6kl_wow_state wow_state;
unsigned int testmode;
To be honest, adding a new state variable scares me. I don't see how we
are able to maintain two different state variables, the end result would
be a total mess.
ath6kl_wow_state is a just sub state of WOW. It roles over only in WOW
mode. However i understand your point.
I recommend to look at this problem by adding a new state to enum
ath6kl_state. That would make it a lot easier to handle all the
different states.
The condition to stop ctrl and data pkt transfer are different.
Ctrl pkt should be stopped when wow_suspended (after sending
set_host_sleep_cmd_mode). Data pkt should be dropped before
the moment we configure set_ip_cmd().
This where we need a state WOW_SUSPENDING and WOW_SUSPENDED.
enum ath6kl_state has over all ath6kl suspend state (cut pwr, deep
sleep,wow). IMHO, mixing WOW sub states there is not good approach.
If you feel maintaining separate state is not good idea, i could think
of introducing two new flag WOW_SUSPENDING, WOW_SUSPENDED in ar->flag.
I may be wrong, What do you say?
(I haven't looked rest of your patch yet.)
Kalle
--
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
