* Re: [PATCH 5/7] pci: Export the pci_restore_msi_state() function [not found] ` <20071021.162131.43417026.davem@davemloft.net> @ 2007-10-22 1:49 ` Michael Ellerman 2007-10-22 18:13 ` Linas Vepstas 0 siblings, 1 reply; 6+ messages in thread From: Michael Ellerman @ 2007-10-22 1:49 UTC (permalink / raw) To: David Miller; +Cc: netdev, mcarlson, linuxppc-dev list, mchan, linux-pci [-- Attachment #1: Type: text/plain, Size: 1345 bytes --] On Sun, 2007-10-21 at 16:21 -0700, David Miller wrote: > From: "Matt Carlson" <mcarlson@broadcom.com> > Date: Fri, 19 Oct 2007 14:36:56 -0700 > > > This patch exports the pci_restore_msi_state() function. This function > > is needed to restore the MSI state during PCI error recovery. > > > > Signed-off-by: Linas Vepstas <linas@austin.ibm.com> > > Signed-off-by: Matt Carlson <mcarlson@broadcom.com> > > Signed-off-by: Michael Chan <mchan@broadcom.com> > > I'm not so sure about this. On pseries there's a chance it will work for PCI error recovery, but if so it's just lucky that firmware has left everything configured the same way. For actual suspend/resume it will never work, we need to ask firmware to configure things. > Perhaps, instead, you should do a pci_msi_disable() and > pci_msi_enable() in the error detection and recovery sequence. Yes I think so. That way we can properly reconfigure via the firmware interface. The other option would be to design some new arch hook to do resume, but just doing a disable/enable seems simpler to me. cheers -- Michael Ellerman OzLabs, IBM Australia Development Lab wwweb: http://michael.ellerman.id.au phone: +61 2 6212 1183 (tie line 70 21183) We do not inherit the earth from our ancestors, we borrow it from our children. - S.M.A.R.T Person [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 189 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 5/7] pci: Export the pci_restore_msi_state() function 2007-10-22 1:49 ` [PATCH 5/7] pci: Export the pci_restore_msi_state() function Michael Ellerman @ 2007-10-22 18:13 ` Linas Vepstas 2007-10-22 21:24 ` Benjamin Herrenschmidt 2007-10-23 4:20 ` Michael Ellerman 0 siblings, 2 replies; 6+ messages in thread From: Linas Vepstas @ 2007-10-22 18:13 UTC (permalink / raw) To: Michael Ellerman Cc: netdev, mcarlson, linuxppc-dev list, mchan, linux-pci, David Miller On Mon, Oct 22, 2007 at 11:49:24AM +1000, Michael Ellerman wrote: > > On pseries there's a chance it will work for PCI error recovery, but if > so it's just lucky that firmware has left everything configured the same > way. ? The papr is quite clear that i is up to the OS to restore the msi state after an eeh error. > Yes I think so. That way we can properly reconfigure via the firmware > interface. The other option would be to design some new arch hook to do > resume, but just doing a disable/enable seems simpler to me. Err, If you read the code for suspend/resume, it never actually calls disable/enable (and thus doesn't go to the firmware); it calls restore_msi_state() function! If suspend/resume needs to call firmware to restore the state, then, at the moment, suspend/resume is broken. As I mentioned earlier, I presumed that no powerpc laptops currently use msi-enabled devices, as otherwise, this would have been flushed out. --linas ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 5/7] pci: Export the pci_restore_msi_state() function 2007-10-22 18:13 ` Linas Vepstas @ 2007-10-22 21:24 ` Benjamin Herrenschmidt 2007-10-23 0:13 ` Linas Vepstas 2007-10-23 4:20 ` Michael Ellerman 1 sibling, 1 reply; 6+ messages in thread From: Benjamin Herrenschmidt @ 2007-10-22 21:24 UTC (permalink / raw) To: Linas Vepstas Cc: netdev, mcarlson, linuxppc-dev list, mchan, linux-pci, David Miller On Mon, 2007-10-22 at 13:13 -0500, Linas Vepstas wrote: > On Mon, Oct 22, 2007 at 11:49:24AM +1000, Michael Ellerman wrote: > > > > On pseries there's a chance it will work for PCI error recovery, but if > > so it's just lucky that firmware has left everything configured the same > > way. > > ? The papr is quite clear that i is up to the OS to restore the msi > state after an eeh error. Via direct config space access or via firmware change-msi calls ? > > Yes I think so. That way we can properly reconfigure via the firmware > > interface. The other option would be to design some new arch hook to do > > resume, but just doing a disable/enable seems simpler to me. > > Err, If you read the code for suspend/resume, it never actually calls > disable/enable (and thus doesn't go to the firmware); it calls > restore_msi_state() function! > > If suspend/resume needs to call firmware to restore the state, then, > at the moment, suspend/resume is broken. As I mentioned earlier, > I presumed that no powerpc laptops currently use msi-enabled devices, > as otherwise, this would have been flushed out. I don't know why you keep talking about powerpc laptops here ... Ben. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 5/7] pci: Export the pci_restore_msi_state() function 2007-10-22 21:24 ` Benjamin Herrenschmidt @ 2007-10-23 0:13 ` Linas Vepstas 2007-10-23 0:29 ` Benjamin Herrenschmidt 0 siblings, 1 reply; 6+ messages in thread From: Linas Vepstas @ 2007-10-23 0:13 UTC (permalink / raw) To: Benjamin Herrenschmidt Cc: netdev, mcarlson, linuxppc-dev list, mchan, linux-pci, David Miller On Tue, Oct 23, 2007 at 07:24:27AM +1000, Benjamin Herrenschmidt wrote: > > On Mon, 2007-10-22 at 13:13 -0500, Linas Vepstas wrote: > > On Mon, Oct 22, 2007 at 11:49:24AM +1000, Michael Ellerman wrote: > > > > > > On pseries there's a chance it will work for PCI error recovery, but if > > > so it's just lucky that firmware has left everything configured the same > > > way. > > > > ? The papr is quite clear that i is up to the OS to restore the msi > > state after an eeh error. > > Via direct config space access or via firmware change-msi calls ? Direct config space access. It says that the OS is supposed to read the MSI config (after its been set up), save it, and restore it, (via direct config space writes) if the device is ever reset. > I don't know why you keep talking about powerpc laptops here ... Well, there are Apple laptops, right? Aren't those the "powermac" platform? Now, I don't know if they support MSI, but if they do, I get the impression that they might not restore msi state correctly, after being put into hardware suspend. But perhaps I'm mistaken; I was simply grepping for various msi-related functions in various arch subdirectories, comparing x86 to other arches, and noticed that code that would restore msi state seems to be missing for most arches and most powerpc platforms. --linas ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 5/7] pci: Export the pci_restore_msi_state() function 2007-10-23 0:13 ` Linas Vepstas @ 2007-10-23 0:29 ` Benjamin Herrenschmidt 0 siblings, 0 replies; 6+ messages in thread From: Benjamin Herrenschmidt @ 2007-10-23 0:29 UTC (permalink / raw) To: Linas Vepstas Cc: netdev, mcarlson, linuxppc-dev list, mchan, linux-pci, David Miller > > I don't know why you keep talking about powerpc laptops here ... > > Well, there are Apple laptops, right? Aren't those the "powermac" > platform? Now, I don't know if they support MSI, but if they do, > I get the impression that they might not restore msi state correctly, > after being put into hardware suspend. But perhaps I'm mistaken; > I was simply grepping for various msi-related functions in various > arch subdirectories, comparing x86 to other arches, and noticed > that code that would restore msi state seems to be missing for > most arches and most powerpc platforms. Ah ok, i see. Well, platforms that use write_msi_msg() shouldn't need anything special right ? So only pSeries is an issue here.... PowerBooks don't indeed have MSI support, though G5's do and some people have been toying around with suspend/resume on them (hibernation only at that stage) but it doesn't matter at this stage. We are specifically talking about pSeries which is the "special" case here. Ben. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 5/7] pci: Export the pci_restore_msi_state() function 2007-10-22 18:13 ` Linas Vepstas 2007-10-22 21:24 ` Benjamin Herrenschmidt @ 2007-10-23 4:20 ` Michael Ellerman 1 sibling, 0 replies; 6+ messages in thread From: Michael Ellerman @ 2007-10-23 4:20 UTC (permalink / raw) To: Linas Vepstas Cc: netdev, mcarlson, linuxppc-dev list, mchan, linux-pci, David Miller [-- Attachment #1: Type: text/plain, Size: 1962 bytes --] On Mon, 2007-10-22 at 13:13 -0500, Linas Vepstas wrote: > On Mon, Oct 22, 2007 at 11:49:24AM +1000, Michael Ellerman wrote: > > > > On pseries there's a chance it will work for PCI error recovery, but if > > so it's just lucky that firmware has left everything configured the same > > way. > > ? The papr is quite clear that i is up to the OS to restore the msi > state after an eeh error. Perhaps. I see R1-7.3.10.5.1-10, which says we should restore the config space after a reset operation, but after an isolate/unisolate we must recall RTAS. I thought EEH was doing the isolate/unisolate, but if it's just a reset then just blatting the config space back should work. > > Yes I think so. That way we can properly reconfigure via the firmware > > interface. The other option would be to design some new arch hook to do > > resume, but just doing a disable/enable seems simpler to me. > > Err, If you read the code for suspend/resume, it never actually calls > disable/enable (and thus doesn't go to the firmware); it calls > restore_msi_state() function! I know. That was a proposed solution, to explicitly call disable/enable instead of restore_msi_state(). > If suspend/resume needs to call firmware to restore the state, then, > at the moment, suspend/resume is broken. As I mentioned earlier, > I presumed that no powerpc laptops currently use msi-enabled devices, > as otherwise, this would have been flushed out. On _pseries_ suspend/resume with MSI is broken. The other powerpc platforms that implement MSI should be fine, although I don't think anyone's tested them, because they're not laptops and so suspend/resume is not that interesting. cheers -- Michael Ellerman OzLabs, IBM Australia Development Lab wwweb: http://michael.ellerman.id.au phone: +61 2 6212 1183 (tie line 70 21183) We do not inherit the earth from our ancestors, we borrow it from our children. - S.M.A.R.T Person [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 189 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2007-10-23 4:20 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1192829817.22064.559.camel@teletran1>
[not found] ` <20071021.162131.43417026.davem@davemloft.net>
2007-10-22 1:49 ` [PATCH 5/7] pci: Export the pci_restore_msi_state() function Michael Ellerman
2007-10-22 18:13 ` Linas Vepstas
2007-10-22 21:24 ` Benjamin Herrenschmidt
2007-10-23 0:13 ` Linas Vepstas
2007-10-23 0:29 ` Benjamin Herrenschmidt
2007-10-23 4:20 ` Michael Ellerman
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).