Commit Message

The firmware agent is not available during resume. Loading the firmware
during open() (see eee3a96c6368f47df8df5bd4ed1843600652b337) is not
enough.
close() is run during resume through rtl8169_reset_task(), whence the
mildly natural release of firmware in the driver removal method instead.
It will help with http://bugs.debian.org/609538. It will not avoid
the 60 seconds delay when:
- there is no firmware
- the driver is loaded and the device is not up before a suspend/resume
Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>Tested-by: Jarek Kamiński <jarek@vilo.eu.org>
Cc: Hayes <hayeswang@realtek.com>
Cc: Ben Hutchings <benh@debian.org>
---
drivers/net/r8169.c | 43 +++++++++++++++++++++++++++++++------------
1 files changed, 31 insertions(+), 12 deletions(-)

Comments

From: Francois Romieu <romieu@fr.zoreil.com>
Date: Fri, 14 Jan 2011 00:07:53 +0100
> The firmware agent is not available during resume. Loading the firmware> during open() (see eee3a96c6368f47df8df5bd4ed1843600652b337) is not> enough.> > close() is run during resume through rtl8169_reset_task(), whence the> mildly natural release of firmware in the driver removal method instead.> > It will help with http://bugs.debian.org/609538. It will not avoid> the 60 seconds delay when:> - there is no firmware> - the driver is loaded and the device is not up before a suspend/resume> > Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>> Tested-by: Jarek Kamiński <jarek@vilo.eu.org>> Cc: Hayes <hayeswang@realtek.com>> Cc: Ben Hutchings <benh@debian.org>
Applied.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

14.01.2011 02:07, Francois Romieu wrote:
> The firmware agent is not available during resume. Loading the firmware> during open() (see eee3a96c6368f47df8df5bd4ed1843600652b337) is not> enough.> > close() is run during resume through rtl8169_reset_task(), whence the> mildly natural release of firmware in the driver removal method instead.> > It will help with http://bugs.debian.org/609538. It will not avoid> the 60 seconds delay when:> - there is no firmware> - the driver is loaded and the device is not up before a suspend/resume
Given all this I think this is somewhat clumsy still. How
does other NIC drivers handles this situation - e.g. tg3?
Maybe this needs to be a generic solution instead of per-driver?
/mjt
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[ background for new people in discussion: once more, a driver resume
didn't get a working firmware load. ]
On Thu, Jan 13, 2011 at 10:52 PM, Michael Tokarev <mjt@tls.msk.ru> wrote:
>> Given all this I think this is somewhat clumsy still. How> does other NIC drivers handles this situation - e.g. tg3?> Maybe this needs to be a generic solution instead of per-driver?
We've had this bug several times - and not just for network drivers
either. It's almost universal that drivers just load their firmware at
initialization, and then don't realize that they need to do it at
resume too and have no filesystems accessible. I think the firmware
access interface is partly to blame, because I suspect both the
documentation and the code (and probably any examples: as mentioned,
this is very common) tends to be about that initial one-shot "bring
the device up" rather than thinking about "oh, resume is a bootup too,
and unlike initial boot, it needs to come up in a configured state".
Maybe the firmware loader could be made more useful to automatically
handle the caching (it already knows about built-in firmware) for the
case where CONFIG_PM is enabled. So that drivers _could_ just
basically do "request_firmware()" in their resume functions, and it
would get satisfied from RAM for the re-request case.
Or maybe we could have some honking big comments, at least ;)
Added some device core/fw-class people to the discussion for comments.
Clearly the fw loading _works_, but it does end up being a common bug
that it's messed up at resume. This time it was the "oh, I moved the
firmware loading around so now it's done at a different stage", but
quite often it's literally just "oh, it never worked, I had just
forgotten about resume".
Linus
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

On Fri, Jan 14, 2011 at 08:05:22AM -0800, Linus Torvalds wrote:
> [ background for new people in discussion: once more, a driver resume> didn't get a working firmware load. ]> > On Thu, Jan 13, 2011 at 10:52 PM, Michael Tokarev <mjt@tls.msk.ru> wrote:> >> > Given all this I think this is somewhat clumsy still. How> > does other NIC drivers handles this situation - e.g. tg3?> > Maybe this needs to be a generic solution instead of per-driver?> > We've had this bug several times - and not just for network drivers> either.
[...]
> Maybe the firmware loader could be made more useful to automatically> handle the caching (it already knows about built-in firmware) for the> case where CONFIG_PM is enabled. So that drivers _could_ just> basically do "request_firmware()" in their resume functions, and it> would get satisfied from RAM for the re-request case.
[...]
This is something I started to implement, but never got finished. I
don't think it can be done without an API change, though, as we need
to know when to drop firmware from the cache. But perhaps this could
be done with a hook in the device-driver binding code.
Ben.

On Fri, Jan 14, 2011 at 8:30 AM, Ben Hutchings <benh@debian.org> wrote:
>> This is something I started to implement, but never got finished. I> don't think it can be done without an API change, though, as we need> to know when to drop firmware from the cache. But perhaps this could> be done with a hook in the device-driver binding code.
Or just associate the firmware with a module?
So if the firmware gets loaded, it stays in memory until the module is unloaded?
And this all would only be the case if CONFIG_PM is set, so you'd not
waste memory unnecessarily.
Linus
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

On Friday, January 14, 2011, Linus Torvalds wrote:
> On Fri, Jan 14, 2011 at 8:30 AM, Ben Hutchings <benh@debian.org> wrote:> >> > This is something I started to implement, but never got finished. I> > don't think it can be done without an API change, though, as we need> > to know when to drop firmware from the cache. But perhaps this could> > be done with a hook in the device-driver binding code.> > Or just associate the firmware with a module?> > So if the firmware gets loaded, it stays in memory until the module is unloaded?> > And this all would only be the case if CONFIG_PM is set, so you'd not> waste memory unnecessarily.
Alternatively, a suspend/hibernate notifier can be used for that I think.
They are called before the freezing and after the thawing of user space, so the
the PM_POST_SUSPEND or PM_POST_RESTORE notification can easily cause the
firmare(s) to be dropped from memory.
Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html