From: "Michael Chan" <mchan@broadcom.com>
To: David Miller <davem@davemloft.net>
Cc: joachim.deguara@amd.com, akpm@linux-foundation.org,
linux-kernel@vger.kernel.org, michal.k.k.piotrowski@gmail.com,
netdev <netdev@vger.kernel.org>,
linux-acpi@vger.kernel.org
Subject: Re: [REGRESSION] tg3 dead after s2ram
Date: Thu, 02 Aug 2007 16:38:00 -0700 [thread overview]
Message-ID: <1186097880.18322.73.camel@dell> (raw)
In-Reply-To: <20070802.150636.77057800.davem@davemloft.net>
On Thu, 2007-08-02 at 15:06 -0700, David Miller wrote:
> From: "Michael Chan" <mchan@broadcom.com>
> Date: Thu, 02 Aug 2007 12:10:29 -0700
>
> > Alternatively, we can also fix it by calling pci_enable_device() again
> > in tg3_open(). But I think it is better to just always save and restore
> > in suspend/resume. bnx2.c will also require the same fix.
>
> We could do it that way. But don't you think it's more reliable to
> save and restore around the event we know will be what clobbers the
> PCI config space on us? :-)
>
Yes for sure when netif state is running and we were already doing that.
> Other things might happen between ->resume() and ->open() that could
> modify PCI config space, and we could overwrite such changes if we do
> the PCI restore in ->open().
I suggested calling pci_enable_device() in ->open(), not calling
pci_restore_state() in ->open(). I ultimately decided against it
because some devices do not enable memory as a workaround and it would
be messy to deal with it again in tg3_open().
I definitely agree that calling PCI restore in ->open() is a bad idea.
We used to save PCI state in ->probe() once and restore PCI state after
every chip reset. This sequence caused many subtle problems.
next prev parent reply other threads:[~2007-08-02 23:38 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-07-31 9:28 [REGRESSION] tg3 dead after s2ram Joachim Deguara
2007-08-01 0:45 ` Andrew Morton
2007-08-01 7:53 ` Michael Chan
2007-08-01 8:01 ` Joachim Deguara
2007-08-01 17:47 ` Michael Chan
2007-08-01 21:00 ` Michael Chan
2007-08-02 8:05 ` Joachim Deguara
2007-08-02 9:15 ` Joachim Deguara
2007-08-02 9:23 ` David Miller
2007-08-02 19:10 ` Michael Chan
2007-08-02 22:06 ` David Miller
2007-08-02 23:38 ` Michael Chan [this message]
2007-08-03 9:47 ` Joachim Deguara
2007-08-03 9:47 ` Joachim Deguara
2007-08-04 3:57 ` David Miller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1186097880.18322.73.camel@dell \
--to=mchan@broadcom.com \
--cc=akpm@linux-foundation.org \
--cc=davem@davemloft.net \
--cc=joachim.deguara@amd.com \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=michal.k.k.piotrowski@gmail.com \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.