[RFC] Add the BMPS timer to enter/exit BMPS mode when running

Eugene Krasnikov k.eugene.e at gmail.com
Tue Jul 23 12:46:43 EDT 2013


@Olof: I am not sure  what do you mean under DYNAMIC_PS but there is
one configuration parameters that is responsible for how time between
last data packet received from AP and NULL packet sent from STA to AP
with PS bit set.

@Yanbo: Have one concern that wcn36xx will try to enter BMPS state in
AP mode. Fix for this can be additional patch on top of this patch.

2013/7/23 Olof Johansson <dev at skyshaper.net>:
> Hello Yanbo.
>
> I have not checked the code yet but I'll try when I have some time.
> However, could you maybe explain why this feature is necessary?
>
> What would be the advantage of this over the DYNAMIC_PS functionality
> that the firmware has?
>
> Cheers
> --
> Olof
>
> On Tue, Jul 23, 2013 at 4:58 PM,  <dreamfly281 at gmail.com> wrote:
>> From: Yanbo Li <yanbol at qti.qualcomm.com>
>>
>> Add the BMPS timer to enter/exit BMPS mode when system running
>> The wcn36xx_vote_enter_full_power() and wcn36xx_vote_enter_bmps()
>> can be used to manual avoid enter BMPS mode in some scenario
>> like DHCP process or roaming.
>>
>> Signed-off-by: Yanbo Li <yanbol at qti.qualcomm.com>
>>
>> diff --git a/debug.c b/debug.c
>> index 529dd54..b43f074 100644
>> --- a/debug.c
>> +++ b/debug.c
>> @@ -36,7 +36,7 @@ static ssize_t read_file_bool_bmps(struct file *file, char __user *user_buf,
>>         struct wcn36xx *wcn = file->private_data;
>>         char buf[3];
>>
>> -       if (wcn->pw_state == WCN36XX_BMPS)
>> +       if (wcn->pw_state & WCN36XX_BMPS_MODE)
>>                 buf[0] = '1';
>>         else
>>                 buf[0] = '0';
>> @@ -50,9 +50,7 @@ static ssize_t write_file_bool_bmps(struct file *file,
>>                                     size_t count, loff_t *ppos)
>>  {
>>         struct wcn36xx *wcn = file->private_data;
>> -       struct ieee80211_vif *vif = container_of((void *)wcn->current_vif,
>> -                                                struct ieee80211_vif,
>> -                                                drv_priv);
>> +
>>         char buf[32];
>>         int buf_size;
>>
>> @@ -64,8 +62,7 @@ static ssize_t write_file_bool_bmps(struct file *file,
>>         case 'y':
>>         case 'Y':
>>         case '1':
>> -               wcn36xx_enable_keep_alive_null_packet(wcn);
>> -               wcn36xx_pmc_enter_bmps_state(wcn, vif->bss_conf.sync_tsf);
>> +               wcn36xx_pmc_enter_bmps_state(wcn);
>>                 break;
>>         case 'n':
>>         case 'N':
>> diff --git a/dxe.c b/dxe.c
>> index 5f47a70..5358515 100644
>> --- a/dxe.c
>> +++ b/dxe.c
>> @@ -595,7 +595,7 @@ int wcn36xx_dxe_tx_frame(struct wcn36xx *wcn,
>>          * mode and writing to the register will not wake up the chip. Instead
>>          * notify chip about new frame through SMSM bus.
>>          */
>> -        if ((wcn->pw_state == WCN36XX_BMPS) && is_low) {
>> +        if ((wcn->pw_state & WCN36XX_BMPS_MODE) && is_low) {
>>                 smsm_change_state(SMSM_APPS_STATE,
>>                                   0,
>>                                   WCN36XX_SMSM_WLAN_TX_ENABLE);
>> diff --git a/main.c b/main.c
>> index f0bd88c..ac2c838 100644
>> --- a/main.c
>> +++ b/main.c
>> @@ -270,6 +270,7 @@ static void wcn36xx_stop(struct ieee80211_hw *hw)
>>         wcn36xx_dbg(WCN36XX_DBG_MAC, "mac stop");
>>
>>         wcn36xx_debugfs_exit(wcn);
>> +       wcn36xx_pmc_deinit(wcn);
>>         wcn36xx_smd_stop(wcn);
>>         wcn36xx_dxe_deinit(wcn);
>>         wcn36xx_smd_close(wcn);
>> @@ -714,15 +715,11 @@ static int wcn36xx_sta_remove(struct ieee80211_hw *hw,
>>  static int wcn36xx_suspend(struct ieee80211_hw *hw, struct cfg80211_wowlan *wow)
>>  {
>>         struct wcn36xx *wcn = hw->priv;
>> -       struct ieee80211_vif *vif = container_of((void *)wcn->current_vif,
>> -                                                struct ieee80211_vif,
>> -                                                drv_priv);
>> +
>>         wcn36xx_dbg(WCN36XX_DBG_MAC, "mac suspend");
>>
>>         mutex_lock(&wcn->pm_mutex);
>> -       /* Enter BMPS only in connected state */
>> -       if ((wcn->aid > 0) && (wcn->pw_state != WCN36XX_BMPS))
>> -               wcn36xx_pmc_enter_bmps_state(wcn, vif->bss_conf.sync_tsf);
>> +       wcn36xx_pmc_enter_bmps_state(wcn);
>>         wcn->is_suspended = true;
>>         wcn->is_con_lost_pending = false;
>>
>> @@ -741,9 +738,6 @@ static int wcn36xx_resume(struct ieee80211_hw *hw)
>>         wcn36xx_dbg(WCN36XX_DBG_MAC, "mac resume");
>>
>>         wcn->is_suspended = false;
>> -       if (wcn->pw_state == WCN36XX_BMPS)
>> -               wcn36xx_pmc_exit_bmps_state(wcn);
>> -
>>         if (wcn->is_con_lost_pending) {
>>                 wcn36xx_dbg(WCN36XX_DBG_MAC, "report connection lost");
>>                 ieee80211_connection_loss(vif);
>> diff --git a/pmc.c b/pmc.c
>> index 31f4086..28b3f7a 100644
>> --- a/pmc.c
>> +++ b/pmc.c
>> @@ -20,29 +20,98 @@
>>   */
>>  #include "wcn36xx.h"
>>
>> -int wcn36xx_pmc_init(struct wcn36xx *wcn)
>> +static int wcn36xx_enable_keep_alive_null_packet(struct wcn36xx *wcn)
>>  {
>> -       wcn->pw_state = WCN36XX_FULL_POWER;
>> -       return 0;
>> +       wcn36xx_dbg(WCN36XX_DBG_PMC, "%s", __func__);
>> +       return wcn36xx_smd_keep_alive_req(wcn, WCN36XX_HAL_KEEP_ALIVE_NULL_PKT,
>> +                                         WCN36XX_KEEP_ALIVE_TIME_PERIOD);
>>  }
>>
>> -int wcn36xx_pmc_enter_bmps_state(struct wcn36xx *wcn, u64 tsf)
>> +
>> +static int wcn36xx_disable_keep_alive_null_packet(struct wcn36xx *wcn)
>>  {
>> -       /* TODO: Make sure the TX chain clean */
>> -       wcn36xx_smd_enter_bmps(wcn, tsf);
>> -       wcn->pw_state = WCN36XX_BMPS;
>> -       return 0;
>> +       return wcn36xx_smd_keep_alive_req(wcn, WCN36XX_HAL_KEEP_ALIVE_NULL_PKT,
>> +                                         0);
>> +}
>> +
>> +/*
>> + *
>> + * @return: 0 means enter BMPS success, other means not enter BMPS mode
>> + */
>> +int wcn36xx_pmc_enter_bmps_state(struct wcn36xx *wcn)
>> +{
>> +       struct ieee80211_vif *vif = container_of((void *)wcn->current_vif,
>> +                                                struct ieee80211_vif,
>> +                                                drv_priv);
>> +       int ret = -1;
>> +
>> +       if ((atomic_read(&wcn->full_power_cnt) == 0) && (wcn->aid > 0)) {
>> +               if (!(wcn->pw_state & WCN36XX_BMPS_MODE)) {
>> +                       wcn36xx_enable_keep_alive_null_packet(wcn);
>> +                       wcn36xx_smd_enter_bmps(wcn, vif->bss_conf.sync_tsf);
>> +                       wcn->pw_state |= WCN36XX_BMPS_MODE;
>> +                       ret = 0;
>> +               }
>> +       }
>> +
>> +       return ret;
>>  }
>>
>>  int wcn36xx_pmc_exit_bmps_state(struct wcn36xx *wcn)
>>  {
>> -       wcn36xx_smd_exit_bmps(wcn);
>> +       if ((atomic_read(&wcn->full_power_cnt) > 0) || (wcn->aid == 0)) {
>> +               if (wcn->pw_state & WCN36XX_BMPS_MODE) {
>> +                       wcn36xx_disable_keep_alive_null_packet(wcn);
>> +                       wcn36xx_smd_exit_bmps(wcn);
>> +                       wcn->pw_state &= ~WCN36XX_BMPS_MODE;
>> +               }
>> +       }
>> +       return 0;
>> +}
>> +
>> +/* TODO: For DHCP/roaming protect? */
>> +void wcn36xx_vote_enter_full_power(struct wcn36xx *wcn)
>> +{
>> +       atomic_inc(&wcn->full_power_cnt);
>> +}
>> +
>> +/*
>> + * This fucntion should be called pair with wcn36xx_vote_enter_full_power
>> + */
>> +void wcn36xx_vote_enter_bmps(struct wcn36xx *wcn)
>> +{
>> +       if (atomic_read(&wcn->full_power_cnt) <= 0)
>> +               wcn36xx_warn("The vote bmps called without paired");
>> +       atomic_dec(&wcn->full_power_cnt);
>> +}
>> +
>> +static void wcn36xx_pmc_bmps_measure_work(struct work_struct *work)
>> +{
>> +       struct wcn36xx *wcn = container_of(work, struct wcn36xx,
>> +                                          bmps_work.work);
>> +
>> +       /* Enter or Exit BMPS depend on the currently condition */
>> +       wcn36xx_pmc_enter_bmps_state(wcn);
>> +       wcn36xx_pmc_exit_bmps_state(wcn);
>> +
>> +       ieee80211_queue_delayed_work(wcn->hw, &wcn->bmps_work,
>> +                                    msecs_to_jiffies(BMPS_MEASURE_TIMER_DEFAULT));
>> +}
>> +
>> +int wcn36xx_pmc_init(struct wcn36xx *wcn)
>> +{
>>         wcn->pw_state = WCN36XX_FULL_POWER;
>> +       atomic_set(&wcn->full_power_cnt, 0);
>> +
>> +       INIT_DELAYED_WORK(&wcn->bmps_work, wcn36xx_pmc_bmps_measure_work);
>> +
>> +       ieee80211_queue_delayed_work(wcn->hw, &wcn->bmps_work,
>> +                                    msecs_to_jiffies(BMPS_MEASURE_TIMER_DEFAULT));
>>         return 0;
>>  }
>>
>> -int wcn36xx_enable_keep_alive_null_packet(struct wcn36xx *wcn)
>> +int wcn36xx_pmc_deinit(struct wcn36xx *wcn)
>>  {
>> -       wcn36xx_dbg(WCN36XX_DBG_PMC, "%s", __func__);
>> -       return wcn36xx_smd_keep_alive_req(wcn, WCN36XX_HAL_KEEP_ALIVE_NULL_PKT);
>> +       cancel_delayed_work(&wcn->bmps_work);
>> +       return 0;
>>  }
>> diff --git a/pmc.h b/pmc.h
>> index 2fd8b1e..068795a 100644
>> --- a/pmc.h
>> +++ b/pmc.h
>> @@ -23,14 +23,19 @@
>>
>>  struct wcn36xx;
>>
>> -enum wcn36xx_power_state {
>> -       WCN36XX_FULL_POWER,
>> -       WCN36XX_BMPS
>> -};
>> +#define WCN36XX_FULL_POWER     1
>> +#define WCN36XX_BMPS_MODE      2
>> +
>> +
>> +#define BMPS_MEASURE_TIMER_DEFAULT  5000 /* unit = ms */
>>
>>  int wcn36xx_pmc_init(struct wcn36xx *wcn);
>> -int wcn36xx_pmc_enter_bmps_state(struct wcn36xx *wcn, u64 tbtt);
>> +int wcn36xx_pmc_deinit(struct wcn36xx *wcn);
>> +
>> +int wcn36xx_pmc_enter_bmps_state(struct wcn36xx *wcn);
>>  int wcn36xx_pmc_exit_bmps_state(struct wcn36xx *wcn);
>> -int wcn36xx_enable_keep_alive_null_packet(struct wcn36xx *wcn);
>> -int wcn36xx_enable_keep_alive_null_packet(struct wcn36xx *wcn);
>> +
>> +void wcn36xx_vote_enter_full_power(struct wcn36xx *wcn);
>> +void wcn36xx_vote_enter_bmps(struct wcn36xx *wcn);
>> +
>>  #endif /* _WCN36XX_PMC_H_ */
>> diff --git a/smd.c b/smd.c
>> index 60e4c02..3c70705 100644
>> --- a/smd.c
>> +++ b/smd.c
>> @@ -1070,7 +1070,8 @@ int wcn36xx_smd_exit_bmps(struct wcn36xx *wcn)
>>  /* Notice: This function should be called after associated, or else it
>>   * will be invalid
>>   */
>> -int wcn36xx_smd_keep_alive_req(struct wcn36xx *wcn, int packet_type)
>> +int wcn36xx_smd_keep_alive_req(struct wcn36xx *wcn, int packet_type,
>> +                              u32 time_period)
>>  {
>>         struct wcn36xx_hal_keep_alive_req_msg msg_body;
>>
>> @@ -1079,9 +1080,9 @@ int wcn36xx_smd_keep_alive_req(struct wcn36xx *wcn, int packet_type)
>>         if (packet_type == WCN36XX_HAL_KEEP_ALIVE_NULL_PKT) {
>>                 msg_body.bss_index = 0;
>>                 msg_body.packet_type = WCN36XX_HAL_KEEP_ALIVE_NULL_PKT;
>> -               msg_body.time_period = WCN36XX_KEEP_ALIVE_TIME_PERIOD;
>> +               msg_body.time_period = time_period;
>>         } else if (packet_type == WCN36XX_HAL_KEEP_ALIVE_UNSOLICIT_ARP_RSP) {
>> -               /* TODO: it also support ARP response type */
>> +               /* TODO: it also support send ARP response type */
>>         } else {
>>                 wcn36xx_warn("unknow keep alive packet type %d", packet_type);
>>                 return -1;
>> diff --git a/smd.h b/smd.h
>> index c8161fb..a856773 100644
>> --- a/smd.h
>> +++ b/smd.h
>> @@ -89,7 +89,8 @@ int wcn36xx_smd_remove_bsskey(struct wcn36xx *wcn,
>>                               u8 keyidx);
>>  int wcn36xx_smd_enter_bmps(struct wcn36xx *wcn, u64 tbtt);
>>  int wcn36xx_smd_exit_bmps(struct wcn36xx *wcn);
>> -int wcn36xx_smd_keep_alive_req(struct wcn36xx *wcn, int packet_type);
>> +int wcn36xx_smd_keep_alive_req(struct wcn36xx *wcn, int packet_type,
>> +                              u32 time_period);
>>  int wcn36xx_smd_dump_cmd_req(struct wcn36xx *wcn, u32 arg1, u32 arg2,
>>                              u32 arg3, u32 arg4, u32 arg5);
>>  int wcn36xx_smd_feature_caps_exchange(struct wcn36xx *wcn);
>> diff --git a/wcn36xx.h b/wcn36xx.h
>> index 830ae2a..ee2edee 100644
>> --- a/wcn36xx.h
>> +++ b/wcn36xx.h
>> @@ -175,7 +175,9 @@ struct wcn36xx {
>>         struct sk_buff          *tx_ack_skb;
>>
>>         /* Power management */
>> -       enum wcn36xx_power_state     pw_state;
>> +       int                          pw_state;
>> +       atomic_t                     full_power_cnt;
>> +       struct delayed_work          bmps_work;
>>
>>         /* Debug file system entry */
>>         struct wcn36xx_dfs_entry    dfs;
>> --
>> 1.7.9.5
>>
>>
>> _______________________________________________
>> wcn36xx mailing list
>> wcn36xx at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/wcn36xx
>
> _______________________________________________
> wcn36xx mailing list
> wcn36xx at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/wcn36xx



-- 
Best regards,
Eugene



More information about the wcn36xx mailing list