[PATCH] Add the power save control framework and debugfs entry

YanBo dreamfly281 at gmail.com
Mon Jul 8 01:19:25 EDT 2013


On Sun, Jul 7, 2013 at 10:58 PM, YanBo <dreamfly281 at gmail.com> wrote:
> On Thu, Jul 4, 2013 at 3:25 PM, Eugene Krasnikov <k.eugene.e at gmail.com> wrote:
>>> +       /* TODO:Disable the TX queue */
>>
>> Do we really need to stop TX here? AFAIK it is possible to send frames
>> even in bmps mode right?
>
> Yes, we can sent packet in BMPS mode, what I know is the TX chain
> should be make sure "clean". before call enter BMPS mode,
> Maybe some status can be check to make sure the TX chain is clean but
> no more detail about this, so mark is as TODO, maybe change it to
> "TODO; make sure TX chain clean" is better
>
>>
>>>         struct wcn36xx_vif      *current_vif;
>>> +       struct ieee80211_vif    *vif; /* For debug get the tsf info */
>>
>> There is no need to store 2 pointers for both wcn36xx_vif  and
>> ieee80211_vif. Let's just replace wcn36xx_vif  with ieee80211_vif.
>> What do you think about that?
>
> Good point,
>>
>>> +       /* Power management */
>>> +       enum wcn36xx_power_state     pw_state;
>>> +
>>
>> I suggest to replace is_suspended parameters with pw_state what do you think?
>
> that is should be good, thanks for carefully review and sorry for my
> later response, (Too busy recently)

After reconsider this issue and the review the whole code , I think
the is_suspend and pw_state monitor for different status of the device
in currently implementation, which the BMPS has not necessarily linked
relationship with the is_suspend  true or false, combine them together
will make things a bit more confusion. I'd suggest keep them
separation and combine them together if we find the closed enough
relationship in the future. what your think?

BR /Yanbo
>> --
>> Best regards,
>> Eugene



More information about the wcn36xx mailing list