On 02/15/2012 02:22 PM, rmani@xxxxxxxxxxxxxxxx wrote:
> From: Raja Mani <rmani@xxxxxxxxxxxxxxxx>
>
> It gives flexibility to the user to define suspend policy
> when the suspend mode is set to WOW and the device is in
> disconnected state at the time of suspend.
>
> Some bits (bit 4 to 7) of existing mod parameter suspend_mode
> is used for that purpose. There is no change in the existing
> way of defining suspend mode.
>
> To force cut power:
> suspend_mode=0x1
>
> To force deep sleep:
> suspend_mode=0x2
>
> To force wow (deep sleep is the default mode
> in disconnected state):
> suspend_mode=0x3
>
> To force wow in connected state and cut power
> in disconnected state:
> suspend_mode=0x13
What's the difference between values 0x3 and 0x13? It wasn't clear for
me from the description and neither from the code.
> To force wow in connected state and deep sleep
> in disconnected state:
> suspend_mode=0x23
I'm not sure if adding bits to suspend_mode is the best way. To me
adding a separate parameter wow_mode looks easier from user's point of
view. What do you think?
And if you add a new module paratemer please use charp type so that can
use strings like 'deepsleep' and 'cutpower'. I wish that we had used
that already with suspend_mode but it's too late now :(
> --- a/drivers/net/wireless/ath/ath6kl/cfg80211.c
> +++ b/drivers/net/wireless/ath/ath6kl/cfg80211.c
> @@ -2063,7 +2063,6 @@ int ath6kl_cfg80211_suspend(struct ath6kl *ar,
>
> ret = ath6kl_wow_suspend(ar, wow);
> if (ret) {
> - ath6kl_err("wow suspend failed: %d\n", ret);
> ar->state = prev_state;
> return ret;
> }
Please convert this to a debug message, maybe using the suspend level.
It might be useful when debugging something.
> diff --git a/drivers/net/wireless/ath/ath6kl/core.c b/drivers/net/wireless/ath/ath6kl/core.c
> index c4926cf..37c92a1 100644
> --- a/drivers/net/wireless/ath/ath6kl/core.c
> +++ b/drivers/net/wireless/ath/ath6kl/core.c
> @@ -41,6 +41,7 @@ int ath6kl_core_init(struct ath6kl *ar)
> {
> struct ath6kl_bmi_target_info targ_info;
> struct net_device *ndev;
> + u16 pri_suspend_mode, wow2_suspend_mode;
Why the number two? Why not just wow_suspend_mode?
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
