Re: [PATCH v2] wifi: iwlwifi: mld: fix building with CONFIG_PM_SLEEP disabled

[Lukas Wunner <lukas@wunner.de>]

On Mon, Apr 07, 2025 at 06:42:47AM +0000, Korenblit, Miriam Rachel wrote:
> > From: Lukas Wunner <lukas@wunner.de>
> > Sent: Sunday, 6 April 2025 16:44
[...]
> > --- a/drivers/net/wireless/intel/iwlwifi/mld/d3.h
> > +++ b/drivers/net/wireless/intel/iwlwifi/mld/d3.h
> > @@ -34,6 +34,7 @@ struct iwl_mld_wowlan_data {
> >  	struct iwl_mld_rekey_data rekey_data;
> >  };
> > 
> > +#ifdef CONFIG_PM_SLEEP
> >  int iwl_mld_no_wowlan_resume(struct iwl_mld *mld);  int
> > iwl_mld_no_wowlan_suspend(struct iwl_mld *mld);  int
> > iwl_mld_wowlan_suspend(struct iwl_mld *mld, @@ -47,5 +48,8 @@ void
> > iwl_mld_ipv6_addr_change(struct ieee80211_hw *hw,
> >  			      struct ieee80211_vif *vif,
> >  			      struct inet6_dev *idev);
> >  #endif
> > +#else
> > +static inline int iwl_mld_no_wowlan_suspend(struct iwl_mld *mld) {
> > +return 0;
> 
> You are not supposed to get to this function not under the ifdef.
> So if you do, I'd return an error value, not 0?

No.  This code section of iwl_mld_mac80211_stop()...

	/* if the suspend flow fails the fw is in error. Stop it here, and it
	 * will be started upon wakeup
	 */
	if (!suspend || iwl_mld_no_wowlan_suspend(mld))
		iwl_mld_stop_fw(mld);

...would unconditionally call iwl_mld_stop_fw() if the empty inline stub
of iwl_mld_no_wowlan_suspend() returned an error value.

That doesn't seem to be the desired behavior here, judging by the code
comment:  Apparently iwl_mld_stop_fw() is only supposed to be called
if the firmware "is in error".  So returning 0 is very much intentional.

Thanks,

Lukas