* [PATCH v2] wifi: iwlwifi: mld: fix building with CONFIG_PM_SLEEP disabled
@ 2025-04-06 13:43 Lukas Wunner
2025-04-07 6:42 ` Korenblit, Miriam Rachel
0 siblings, 1 reply; 4+ messages in thread
From: Lukas Wunner @ 2025-04-06 13:43 UTC (permalink / raw)
To: Miri Korenblit, Johannes Berg, Emmanuel Grumbach, Benjamin Berg,
Yedidya Benshimol, Arnd Bergmann, Avraham Stern, Daniel Gabay,
Anjaneyulu, Shaul Triebitz
Cc: linux-wireless
From: Arnd Bergmann <arnd@arndb.de>
The newly added driver causes multiple build problems when CONFIG_PM_SLEEP
is disabled:
drivers/net/wireless/intel/iwlwifi/mld/mac80211.c:1982:12: error: 'iwl_mld_resume' defined but not used [-Werror=unused-function]
1982 | static int iwl_mld_resume(struct ieee80211_hw *hw)
| ^~~~~~~~~~~~~~
drivers/net/wireless/intel/iwlwifi/mld/mac80211.c:1960:1: error: 'iwl_mld_suspend' defined but not used [-Werror=unused-function]
1960 | iwl_mld_suspend(struct ieee80211_hw *hw, struct cfg80211_wowlan *wowlan)
| ^~~~~~~~~~~~~~~
drivers/net/wireless/intel/iwlwifi/mld/mac80211.c:1946:13: error: 'iwl_mld_set_wakeup' defined but not used [-Werror=unused-function]
1946 | static void iwl_mld_set_wakeup(struct ieee80211_hw *hw, bool enabled)
| ^~~~~~~~~~~~~~~~~~
drivers/net/wireless/intel/iwlwifi/mld/mac80211.c: In function 'iwl_mld_mac80211_start':
drivers/net/wireless/intel/iwlwifi/mld/mac80211.c:504:20: error: 'ret' is used uninitialized [-Werror=uninitialized]
504 | if (!in_d3 || ret) {
| ^~
drivers/net/wireless/intel/iwlwifi/mld/mac80211.c:478:13: note: 'ret' was declared here
478 | int ret;
| ^~~
Hide the unused functions and make sure the 'ret' variable is initialized
before being used.
Fixes: d1e879ec600f ("wifi: iwlwifi: add iwlmld sub-driver")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/r/202504032255.N6ptuCNG-lkp@intel.com/
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
[lukas: initialize ret to 0, use static inline]
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
Changes v1 -> v2:
Initialize ret to 0 instead of removing it from the if-clause (Miriam).
Use static inline for iwl_mld_no_wowlan_suspend() instead of an #ifdef
in accordance with section 21 of Documentation/process/coding-style.rst.
Link to v1:
https://lore.kernel.org/r/20250325084340.378724-1-arnd@kernel.org/
drivers/net/wireless/intel/iwlwifi/mld/d3.h | 4 ++++
drivers/net/wireless/intel/iwlwifi/mld/mac80211.c | 4 +++-
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/intel/iwlwifi/mld/d3.h b/drivers/net/wireless/intel/iwlwifi/mld/d3.h
index 618d6fb..4157f82 100644
--- 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; }
+#endif
#endif /* __iwl_mld_d3_h__ */
diff --git a/drivers/net/wireless/intel/iwlwifi/mld/mac80211.c b/drivers/net/wireless/intel/iwlwifi/mld/mac80211.c
index 6851064..18cd607 100644
--- a/drivers/net/wireless/intel/iwlwifi/mld/mac80211.c
+++ b/drivers/net/wireless/intel/iwlwifi/mld/mac80211.c
@@ -475,8 +475,8 @@ int iwl_mld_register_hw(struct iwl_mld *mld)
int iwl_mld_mac80211_start(struct ieee80211_hw *hw)
{
struct iwl_mld *mld = IWL_MAC80211_GET_MLD(hw);
- int ret;
bool in_d3 = false;
+ int ret = 0;
lockdep_assert_wiphy(mld->wiphy);
@@ -1943,6 +1943,7 @@ static void iwl_mld_sta_rc_update(struct ieee80211_hw *hw,
}
}
+#ifdef CONFIG_PM_SLEEP
static void iwl_mld_set_wakeup(struct ieee80211_hw *hw, bool enabled)
{
struct iwl_mld *mld = IWL_MAC80211_GET_MLD(hw);
@@ -1994,6 +1995,7 @@ static int iwl_mld_resume(struct ieee80211_hw *hw)
return 0;
}
+#endif
static int iwl_mld_alloc_ptk_pn(struct iwl_mld *mld,
struct iwl_mld_sta *mld_sta,
--
2.43.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* RE: [PATCH v2] wifi: iwlwifi: mld: fix building with CONFIG_PM_SLEEP disabled
2025-04-06 13:43 [PATCH v2] wifi: iwlwifi: mld: fix building with CONFIG_PM_SLEEP disabled Lukas Wunner
@ 2025-04-07 6:42 ` Korenblit, Miriam Rachel
2025-04-07 7:09 ` Lukas Wunner
0 siblings, 1 reply; 4+ messages in thread
From: Korenblit, Miriam Rachel @ 2025-04-07 6:42 UTC (permalink / raw)
To: Lukas Wunner, Berg, Johannes, Grumbach, Emmanuel, Berg, Benjamin,
Ben Shimol, Yedidya, Arnd Bergmann, Stern, Avraham, Gabay, Daniel,
Anjaneyulu, Pagadala Yesu, Triebitz, Shaul
Cc: linux-wireless@vger.kernel.org
> -----Original Message-----
> From: Lukas Wunner <lukas@wunner.de>
> Sent: Sunday, 6 April 2025 16:44
> To: Korenblit, Miriam Rachel <miriam.rachel.korenblit@intel.com>; Berg,
> Johannes <johannes.berg@intel.com>; Grumbach, Emmanuel
> <emmanuel.grumbach@intel.com>; Berg, Benjamin <benjamin.berg@intel.com>;
> Ben Shimol, Yedidya <yedidya.ben.shimol@intel.com>; Arnd Bergmann
> <arnd@arndb.de>; Stern, Avraham <avraham.stern@intel.com>; Gabay, Daniel
> <daniel.gabay@intel.com>; Anjaneyulu, Pagadala Yesu
> <pagadala.yesu.anjaneyulu@intel.com>; Triebitz, Shaul
> <shaul.triebitz@intel.com>
> Cc: linux-wireless@vger.kernel.org
> Subject: [PATCH v2] wifi: iwlwifi: mld: fix building with CONFIG_PM_SLEEP
> disabled
>
> From: Arnd Bergmann <arnd@arndb.de>
>
> The newly added driver causes multiple build problems when CONFIG_PM_SLEEP
> is disabled:
>
> drivers/net/wireless/intel/iwlwifi/mld/mac80211.c:1982:12: error:
> 'iwl_mld_resume' defined but not used [-Werror=unused-function]
> 1982 | static int iwl_mld_resume(struct ieee80211_hw *hw)
> | ^~~~~~~~~~~~~~
> drivers/net/wireless/intel/iwlwifi/mld/mac80211.c:1960:1: error:
> 'iwl_mld_suspend' defined but not used [-Werror=unused-function]
> 1960 | iwl_mld_suspend(struct ieee80211_hw *hw, struct cfg80211_wowlan
> *wowlan)
> | ^~~~~~~~~~~~~~~
> drivers/net/wireless/intel/iwlwifi/mld/mac80211.c:1946:13: error:
> 'iwl_mld_set_wakeup' defined but not used [-Werror=unused-function]
> 1946 | static void iwl_mld_set_wakeup(struct ieee80211_hw *hw, bool enabled)
> | ^~~~~~~~~~~~~~~~~~
> drivers/net/wireless/intel/iwlwifi/mld/mac80211.c: In function
> 'iwl_mld_mac80211_start':
> drivers/net/wireless/intel/iwlwifi/mld/mac80211.c:504:20: error: 'ret' is used
> uninitialized [-Werror=uninitialized]
> 504 | if (!in_d3 || ret) {
> | ^~
> drivers/net/wireless/intel/iwlwifi/mld/mac80211.c:478:13: note: 'ret' was
> declared here
> 478 | int ret;
> | ^~~
>
> Hide the unused functions and make sure the 'ret' variable is initialized before
> being used.
>
> Fixes: d1e879ec600f ("wifi: iwlwifi: add iwlmld sub-driver")
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/r/202504032255.N6ptuCNG-lkp@intel.com/
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> [lukas: initialize ret to 0, use static inline]
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> ---
> Changes v1 -> v2:
> Initialize ret to 0 instead of removing it from the if-clause (Miriam).
> Use static inline for iwl_mld_no_wowlan_suspend() instead of an #ifdef in
> accordance with section 21 of Documentation/process/coding-style.rst.
>
> Link to v1:
> https://lore.kernel.org/r/20250325084340.378724-1-arnd@kernel.org/
>
> drivers/net/wireless/intel/iwlwifi/mld/d3.h | 4 ++++
> drivers/net/wireless/intel/iwlwifi/mld/mac80211.c | 4 +++-
> 2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/intel/iwlwifi/mld/d3.h
> b/drivers/net/wireless/intel/iwlwifi/mld/d3.h
> index 618d6fb..4157f82 100644
> --- 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?
} #endif
>
> #endif /* __iwl_mld_d3_h__ */
> diff --git a/drivers/net/wireless/intel/iwlwifi/mld/mac80211.c
> b/drivers/net/wireless/intel/iwlwifi/mld/mac80211.c
> index 6851064..18cd607 100644
> --- a/drivers/net/wireless/intel/iwlwifi/mld/mac80211.c
> +++ b/drivers/net/wireless/intel/iwlwifi/mld/mac80211.c
> @@ -475,8 +475,8 @@ int iwl_mld_register_hw(struct iwl_mld *mld) int
> iwl_mld_mac80211_start(struct ieee80211_hw *hw) {
> struct iwl_mld *mld = IWL_MAC80211_GET_MLD(hw);
> - int ret;
> bool in_d3 = false;
> + int ret = 0;
>
> lockdep_assert_wiphy(mld->wiphy);
>
> @@ -1943,6 +1943,7 @@ static void iwl_mld_sta_rc_update(struct
> ieee80211_hw *hw,
> }
> }
>
> +#ifdef CONFIG_PM_SLEEP
> static void iwl_mld_set_wakeup(struct ieee80211_hw *hw, bool enabled) {
> struct iwl_mld *mld = IWL_MAC80211_GET_MLD(hw); @@ -1994,6
> +1995,7 @@ static int iwl_mld_resume(struct ieee80211_hw *hw)
>
> return 0;
> }
> +#endif
>
> static int iwl_mld_alloc_ptk_pn(struct iwl_mld *mld,
> struct iwl_mld_sta *mld_sta,
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] wifi: iwlwifi: mld: fix building with CONFIG_PM_SLEEP disabled
2025-04-07 6:42 ` Korenblit, Miriam Rachel
@ 2025-04-07 7:09 ` Lukas Wunner
2025-04-08 8:56 ` Korenblit, Miriam Rachel
0 siblings, 1 reply; 4+ messages in thread
From: Lukas Wunner @ 2025-04-07 7:09 UTC (permalink / raw)
To: Korenblit, Miriam Rachel
Cc: Berg, Johannes, Grumbach, Emmanuel, Berg, Benjamin,
Ben Shimol, Yedidya, Arnd Bergmann, Stern, Avraham, Gabay, Daniel,
Anjaneyulu, Pagadala Yesu, Triebitz, Shaul,
linux-wireless@vger.kernel.org
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH v2] wifi: iwlwifi: mld: fix building with CONFIG_PM_SLEEP disabled
2025-04-07 7:09 ` Lukas Wunner
@ 2025-04-08 8:56 ` Korenblit, Miriam Rachel
0 siblings, 0 replies; 4+ messages in thread
From: Korenblit, Miriam Rachel @ 2025-04-08 8:56 UTC (permalink / raw)
To: Lukas Wunner
Cc: Berg, Johannes, Grumbach, Emmanuel, Berg, Benjamin,
Ben Shimol, Yedidya, Arnd Bergmann, Stern, Avraham, Gabay, Daniel,
Anjaneyulu, Pagadala Yesu, Triebitz, Shaul,
linux-wireless@vger.kernel.org
> -----Original Message-----
> From: Lukas Wunner <lukas@wunner.de>
> Sent: Monday, 7 April 2025 10:09
> To: Korenblit, Miriam Rachel <miriam.rachel.korenblit@intel.com>
> Cc: Berg, Johannes <johannes.berg@intel.com>; Grumbach, Emmanuel
> <emmanuel.grumbach@intel.com>; Berg, Benjamin <benjamin.berg@intel.com>;
> Ben Shimol, Yedidya <yedidya.ben.shimol@intel.com>; Arnd Bergmann
> <arnd@arndb.de>; Stern, Avraham <avraham.stern@intel.com>; Gabay, Daniel
> <daniel.gabay@intel.com>; Anjaneyulu, Pagadala Yesu
> <pagadala.yesu.anjaneyulu@intel.com>; Triebitz, Shaul
> <shaul.triebitz@intel.com>; linux-wireless@vger.kernel.org
> Subject: Re: [PATCH v2] wifi: iwlwifi: mld: fix building with CONFIG_PM_SLEEP
> disabled
>
> 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
Acked-by: Miri Korenblit <miriam.rachel.korenblit@intel.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-04-08 8:57 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-06 13:43 [PATCH v2] wifi: iwlwifi: mld: fix building with CONFIG_PM_SLEEP disabled Lukas Wunner
2025-04-07 6:42 ` Korenblit, Miriam Rachel
2025-04-07 7:09 ` Lukas Wunner
2025-04-08 8:56 ` Korenblit, Miriam Rachel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).