Re: [PATCH 4/7] wiphy: add [DriverFlags].PowerSaveDisable flag

[James Prestwood <prestwoj@gmail.com>]

Hi Denis,

On 6/18/23 12:07 PM, Denis Kenzior wrote:
> Hi James,
> 
> On 6/15/23 14:24, James Prestwood wrote:
>> Certain drivers do not handle power save very well resulting in
>> missed frames, firmware crashes, or other bad behavior. Its easy
>> enough to disable power save via iw, iwconfig, etc but since IWD
>> removes and creates the interface on startup it blows away any
>> previous power save setting. The setting must be done *after* IWD
>> creates the interface which can be done, but needs to be via some
>> external daemon monitoring IWD's state. For minimal systems,
>> e.g. without NetworkManager, it becomes difficult and annoying to
>> persistently disable power save.
>>
>> For this reason a new driver flag POWER_SAVE_DISABLE is being
>> added. This can then be referenced when creating the interfaces
>> and if set, disable power save.
>> ---
>>   src/wiphy.c | 13 +++++++++++++
>>   src/wiphy.h |  1 +
>>   2 files changed, 14 insertions(+)
>>
> 
> <snip>
> 
>> @@ -723,6 +725,17 @@ bool wiphy_control_port_enabled(struct wiphy *wiphy)
>>       return enabled;
>>   }
>> +bool wiphy_disable_power_save(struct wiphy *wiphy)
> 
> I named this wiphy_power_save_disabled to be consistent with 
> wiphy_control_port_enabled()...
> 
> I went ahead and applied this, but I think we have to fix the behavior 
> of this and wiphy_control_port_enabled().  Namely:
> 
>> +{
>> +    if (wiphy->driver_flags & POWER_SAVE_DISABLE) {
>> +        l_info("Disabling power save due to driver quirks: %s",
>> +                wiphy_get_driver(wiphy));
> 
> We shouldn't really use l_info here.  This method is meant more as a 
> getter and might have other users in the future.  Same case with 
> wiphy_control_port_enabled(), but that one is a little bit more complex 
> to fix.

Ok I can send a follow up patch. The only use for all these APIs is when 
creating the netdev so we *should* only see it then but now that we 
print these flags with wiphy_print_basic_info I'll just remove them 
entirely.

> 
> Patches 4, 6 and 7 applied.
> 
> Regards,
> -Denis