IWLWIFI should depend on SUSPEND

IWLWIFI should depend on SUSPEND

[Sebastian Kemper]

Hello everybody,

iwlwifi doesn't start without suspend support in the kernel. The last
thing it says when suspend is missing is this:

iwlwifi 0000:03:00.0: Failed to register hw (error -22)
iwlwifi 0000:03:00.0: PCI INT A disabled

Most kernels have suspend support, but maybe somebody thinks he doesn't
need it and leaves it out.

Here's the diff between non-working and working kernel configuration
(kernel 3.2.17):

--- doesnt-work.txt	2012-05-19 16:10:54.000000000 +0200
+++ works.txt	2012-05-19 15:53:56.000000000 +0200
@@ -237,7 +237,7 @@
 CONFIG_INLINE_WRITE_UNLOCK_IRQ=y
 # CONFIG_INLINE_WRITE_UNLOCK_IRQRESTORE is not set
 CONFIG_MUTEX_SPIN_ON_OWNER=y
-# CONFIG_FREEZER is not set
+CONFIG_FREEZER=y
 
 #
 # Processor type and features
@@ -371,12 +371,16 @@
 #
 # Power management and ACPI options
 #
-# CONFIG_SUSPEND is not set
+CONFIG_SUSPEND=y
+CONFIG_SUSPEND_FREEZER=y
 # CONFIG_HIBERNATION is not set
+CONFIG_PM_SLEEP=y
+CONFIG_PM_SLEEP_SMP=y
 CONFIG_PM_RUNTIME=y
 CONFIG_PM=y
 # CONFIG_PM_DEBUG is not set
 CONFIG_ACPI=y
+CONFIG_ACPI_SLEEP=y
 # CONFIG_ACPI_PROCFS is not set
 # CONFIG_ACPI_PROCFS_POWER is not set
 # CONFIG_ACPI_EC_DEBUGFS is not set
@@ -865,6 +869,7 @@
 CONFIG_VT=y
 CONFIG_CONSOLE_TRANSLATIONS=y
 CONFIG_VT_CONSOLE=y
+CONFIG_VT_CONSOLE_SLEEP=y
 CONFIG_HW_CONSOLE=y
 # CONFIG_VT_HW_CONSOLE_BINDING is not set
 CONFIG_UNIX98_PTYS=y

Regards,
Sebastian

Re: IWLWIFI should depend on SUSPEND

[Johannes Berg]

On Sat, 2012-05-19 at 22:19 +0200, Sebastian Kemper wrote:
> Hello everybody,
> 
> iwlwifi doesn't start without suspend support in the kernel. The last
> thing it says when suspend is missing is this:
> 
> iwlwifi 0000:03:00.0: Failed to register hw (error -22)
> iwlwifi 0000:03:00.0: PCI INT A disabled

I was going to say it's a mac80211 bug due to this code:

        if ((hw->wiphy->wowlan.flags || hw->wiphy->wowlan.n_patterns)
#ifdef CONFIG_PM
            && (!local->ops->suspend || !local->ops->resume)
#endif
            )
                return -EINVAL;

But then I thought about it and think it makes sense, if you don't have
suspend then you can't have WoWLAN, so the driver shouldn't advertise it
in that case, which makes it an iwlwifi bug again, we should ifdef the
wowlan support correctly.

OTOH, we could also just make the wowlan stuff not compile if CONFIG_PM
isn't set by adding an ifdef in cfg80211... That might be better so
others don't run into this trap.

Thoughts?

johannes

Re: IWLWIFI should depend on SUSPEND

[Johannes Berg]

On Sun, 2012-05-20 at 21:07 +0200, Johannes Berg wrote:
> On Sat, 2012-05-19 at 22:19 +0200, Sebastian Kemper wrote:
> > Hello everybody,
> > 
> > iwlwifi doesn't start without suspend support in the kernel. The last
> > thing it says when suspend is missing is this:
> > 
> > iwlwifi 0000:03:00.0: Failed to register hw (error -22)
> > iwlwifi 0000:03:00.0: PCI INT A disabled
> 
> I was going to say it's a mac80211 bug due to this code:
> 
>         if ((hw->wiphy->wowlan.flags || hw->wiphy->wowlan.n_patterns)
> #ifdef CONFIG_PM
>             && (!local->ops->suspend || !local->ops->resume)
> #endif
>             )
>                 return -EINVAL;
> 
> But then I thought about it and think it makes sense, if you don't have
> suspend then you can't have WoWLAN, so the driver shouldn't advertise it
> in that case, which makes it an iwlwifi bug again, we should ifdef the
> wowlan support correctly.

Could you try this patch please?

johannes

diff --git a/drivers/net/wireless/iwlwifi/iwl-mac80211.c b/drivers/net/wireless/iwlwifi/iwl-mac80211.c
index ab2f4d7..06546f7 100644
--- a/drivers/net/wireless/iwlwifi/iwl-mac80211.c
+++ b/drivers/net/wireless/iwlwifi/iwl-mac80211.c
@@ -199,6 +199,7 @@ int iwlagn_mac_setup_register(struct iwl_priv *priv,
 			    WIPHY_FLAG_DISABLE_BEACON_HINTS |
 			    WIPHY_FLAG_IBSS_RSN;
 
+#ifdef CONFIG_PM
 	if (priv->fw->img[IWL_UCODE_WOWLAN].sec[0].len &&
 	    priv->trans->ops->wowlan_suspend &&
 	    device_can_wakeup(priv->trans->dev)) {
@@ -217,6 +218,7 @@ int iwlagn_mac_setup_register(struct iwl_priv *priv,
 		hw->wiphy->wowlan.pattern_max_len =
 					IWLAGN_WOWLAN_MAX_PATTERN_LEN;
 	}
+#endif
 
 	if (iwlwifi_mod_params.power_save)
 		hw->wiphy->flags |= WIPHY_FLAG_PS_ON_BY_DEFAULT;

Re: IWLWIFI should depend on SUSPEND

[Sebastian Kemper]

On Wed, May 30, 2012 at 03:29:03PM +0200, Johannes Berg wrote:
> Could you try this patch please?
> 
> johannes
> 
> diff --git a/drivers/net/wireless/iwlwifi/iwl-mac80211.c
> b/drivers/net/wireless/iwlwifi/iwl-mac80211.c index ab2f4d7..06546f7
> 100644 --- a/drivers/net/wireless/iwlwifi/iwl-mac80211.c +++
> b/drivers/net/wireless/iwlwifi/iwl-mac80211.c @@ -199,6 +199,7 @@ int
> iwlagn_mac_setup_register(struct iwl_priv *priv,
> WIPHY_FLAG_DISABLE_BEACON_HINTS | WIPHY_FLAG_IBSS_RSN;
>  
> +#ifdef CONFIG_PM if (priv->fw->img[IWL_UCODE_WOWLAN].sec[0].len &&
> priv->trans->ops->wowlan_suspend &&
> device_can_wakeup(priv->trans->dev)) { @@ -217,6 +218,7 @@ int
> iwlagn_mac_setup_register(struct iwl_priv *priv,
> hw->wiphy->wowlan.pattern_max_len = IWLAGN_WOWLAN_MAX_PATTERN_LEN; }
> +#endif
>  
>  	if (iwlwifi_mod_params.power_save) hw->wiphy->flags |=
>  	WIPHY_FLAG_PS_ON_BY_DEFAULT;

Hello Johannes,

the patch doesn't apply on top of 3.2.18 (kernel I currently use),
neither on 3.3.7.

Regards
Sebastian

Re: IWLWIFI should depend on SUSPEND

[Johannes Berg]

On Sun, 2012-06-03 at 11:16 +0200, Sebastian Kemper wrote:
> On Wed, May 30, 2012 at 03:29:03PM +0200, Johannes Berg wrote:
> > Could you try this patch please?
> > 
> > johannes
> > 
> > diff --git a/drivers/net/wireless/iwlwifi/iwl-mac80211.c
> > b/drivers/net/wireless/iwlwifi/iwl-mac80211.c index ab2f4d7..06546f7
> > 100644 --- a/drivers/net/wireless/iwlwifi/iwl-mac80211.c +++
> > b/drivers/net/wireless/iwlwifi/iwl-mac80211.c @@ -199,6 +199,7 @@ int
> > iwlagn_mac_setup_register(struct iwl_priv *priv,
> > WIPHY_FLAG_DISABLE_BEACON_HINTS | WIPHY_FLAG_IBSS_RSN;
> >  
> > +#ifdef CONFIG_PM if (priv->fw->img[IWL_UCODE_WOWLAN].sec[0].len &&
> > priv->trans->ops->wowlan_suspend &&
> > device_can_wakeup(priv->trans->dev)) { @@ -217,6 +218,7 @@ int
> > iwlagn_mac_setup_register(struct iwl_priv *priv,
> > hw->wiphy->wowlan.pattern_max_len = IWLAGN_WOWLAN_MAX_PATTERN_LEN; }
> > +#endif
> >  
> >  	if (iwlwifi_mod_params.power_save) hw->wiphy->flags |=
> >  	WIPHY_FLAG_PS_ON_BY_DEFAULT;
> 
> Hello Johannes,
> 
> the patch doesn't apply on top of 3.2.18 (kernel I currently use),
> neither on 3.3.7.

Oh, so this goes back much further than I thought.

Well, the easiest is probably if you find the wowlan.pattern_max_len
stuff etc. and put #ifdef around the whole wowlan block manually, like
in my patch.

johannes

Re: IWLWIFI should depend on SUSPEND

[Johannes Berg]

On Sun, 2012-06-03 at 18:49 +0200, Sebastian Kemper wrote:
> On Sun, Jun 03, 2012 at 05:02:16PM +0200, Johannes Berg wrote:
> > Oh, so this goes back much further than I thought.
> > 
> > Well, the easiest is probably if you find the wowlan.pattern_max_len
> > stuff etc. and put #ifdef around the whole wowlan block manually, like
> > in my patch.
> > 
> > johannes
> > 
> 
> Hello again,
> 
> I didn't try it out yet, although I did find the block you're talking
> about. I didn't because I *think* that
> 
> #ifdef CONFIG_PM
> …
> #endif
> 
> wouldn't work.

You're right, it should be CONFIG_PM_SLEEP or something ... I'll have to
sort it out. It looks like in iwlwifi, the suspend/resume methods are
under CONFIG_PM_SLEEP, while in mac80211 they're under CONFIG_PM.

johannes

Re: IWLWIFI should depend on SUSPEND

[Sebastian Kemper]

[-- Attachment #1: Type: text/plain, Size: 528 bytes --]

On Sun, Jun 03, 2012 at 08:33:57PM +0200, Johannes Berg wrote:
> You're right, it should be CONFIG_PM_SLEEP or something ... I'll have to
> sort it out. It looks like in iwlwifi, the suspend/resume methods are
> under CONFIG_PM_SLEEP, while in mac80211 they're under CONFIG_PM.

Hello Johannes,

I changed the file iwl-agn.c according to your instructions (with #ifdef
CONFIG_PM_SLEEP) for kernel 3.2.18 (patch attached). It works, the error
is gone and wireless is available.

Thank you for your work!

Kind regards,
Sebastian

[-- Attachment #2: iwl-agn.c.2.6.18.suspend.diff --]
[-- Type: text/plain, Size: 644 bytes --]

--- drivers/net/wireless/iwlwifi/iwl-agn.c.orig	2012-06-04 18:02:32.996662801 +0200
+++ drivers/net/wireless/iwlwifi/iwl-agn.c	2012-06-04 18:03:10.539993094 +0200
@@ -1696,6 +1696,7 @@
 			    WIPHY_FLAG_DISABLE_BEACON_HINTS |
 			    WIPHY_FLAG_IBSS_RSN;
 
+#ifdef CONFIG_PM_SLEEP
 	if (priv->ucode_wowlan.code.len && device_can_wakeup(bus(priv)->dev)) {
 		hw->wiphy->wowlan.flags = WIPHY_WOWLAN_MAGIC_PKT |
 					  WIPHY_WOWLAN_DISCONNECT |
@@ -1712,6 +1713,7 @@
 		hw->wiphy->wowlan.pattern_max_len =
 					IWLAGN_WOWLAN_MAX_PATTERN_LEN;
 	}
+#endif
 
 	if (iwlagn_mod_params.power_save)
 		hw->wiphy->flags |= WIPHY_FLAG_PS_ON_BY_DEFAULT;