| Message ID | 20230615192415.1718516-4-prestwoj@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/7] wiphy: store driver flags directly in wiphy object | expand |
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
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
diff --git a/src/wiphy.c b/src/wiphy.c index 6f8f6826..2c09d47a 100644 --- a/src/wiphy.c +++ b/src/wiphy.c @@ -71,6 +71,7 @@ static unsigned int wiphy_dump_id; enum driver_flag { DEFAULT_IF = 0x1, FORCE_PAE = 0x2, + POWER_SAVE_DISABLE = 0x4, }; struct driver_flag_name { @@ -101,6 +102,7 @@ static const struct driver_info driver_infos[] = { static const struct driver_flag_name driver_flag_names[] = { { "DefaultInterface", DEFAULT_IF }, { "ForcePae", FORCE_PAE }, + { "PowerSaveDisable", POWER_SAVE_DISABLE }, }; struct wiphy { @@ -723,6 +725,17 @@ bool wiphy_control_port_enabled(struct wiphy *wiphy) return enabled; } +bool wiphy_disable_power_save(struct wiphy *wiphy) +{ + if (wiphy->driver_flags & POWER_SAVE_DISABLE) { + l_info("Disabling power save due to driver quirks: %s", + wiphy_get_driver(wiphy)); + return true; + } + + return false; +} + const uint8_t *wiphy_get_permanent_address(struct wiphy *wiphy) { return wiphy->permanent_addr; diff --git a/src/wiphy.h b/src/wiphy.h index f4f205ad..39837366 100644 --- a/src/wiphy.h +++ b/src/wiphy.h @@ -135,6 +135,7 @@ const char *wiphy_get_driver(struct wiphy *wiphy); const char *wiphy_get_name(struct wiphy *wiphy); bool wiphy_uses_default_if(struct wiphy *wiphy); bool wiphy_control_port_enabled(struct wiphy *wiphy); +bool wiphy_disable_power_save(struct wiphy *wiphy); const uint8_t *wiphy_get_permanent_address(struct wiphy *wiphy); const uint8_t *wiphy_get_extended_capabilities(struct wiphy *wiphy, uint32_t iftype);