govinds@xxxxxxxxxxxxxx writes: > On 2018-04-18 18:46, Kalle Valo wrote: >> (fixing top posting) >> >> govinds@xxxxxxxxxxxxxx writes: >> >>> On 2018-04-18 12:36, Sebastian Gottschall wrote: >>>> from my point of view powersave should be optional and not forced. >>>> >>>> consider : >>>> iw dev <devname> set power_save on/off >>>> >>>> so there is already a config option made for that purpose, >>> >>> Thanks Sebastian for your review. Agree this should not be forced with >>> in driver. >>> >>> I will check if i can set the idle ps at the end of >>> ath10k_mac_vif_setup_ps by checking arvif->ps. >>> I will update the patch accordingly. >> >> So what power save is this exactly? I assumed it's some bus level power >> save (turning of clocks etc) but is it actually 802.11 power save? > > Yes this is WIFI chip set level power-save(based on idleness) and not > related to protocol power save. FW will turn off/scale down the > resources(clock/rails) based on opportunity(when ever idle mode is > detected). This power save is mostly done in disconnected state. I am > not really sure when framework/user-space triggers power-save > config(iw dev <devname> set power_save on/off). Then doing this from > user-space will be unnecessarily toggling this config or may not be > setting at disconnected state. I think that 'set power_save' commands affects struct ieee80211_bss_conf::ps parameter and I don't think it should be used in this case. We already have ath10k_config_ps() for 802.11 level of power saving. Arend again proposed runtime-pm with which I'm not very familiar. But why would we want to disable this? Doesn't it make sense to have this feature always enabled to save power? wcn3990 is a chip for a mobile device anyway. -- Kalle Valo
