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

[Denis Kenzior <denkenz@gmail.com>]

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.

Patches 4, 6 and 7 applied.

Regards,
-Denis