* Re: PCI Power management (was: Re: [PATCH 4/13]: PCI Err: e100 ethernet driver recovery
2005-06-30 20:39 ` PCI Power management (was: Re: [PATCH 4/13]: PCI Err: e100 ethernet driver recovery Linas Vepstas
@ 2005-06-30 21:07 ` Linas Vepstas
2005-06-30 23:32 ` Benjamin Herrenschmidt
1 sibling, 0 replies; 3+ messages in thread
From: Linas Vepstas @ 2005-06-30 21:07 UTC (permalink / raw)
To: Andi Kleen, sfr
Cc: Hidetoshi Seto, long, linux-laptop, linux-kernel, pavel,
Paul Mackerras, Greg KH, linuxppc64-dev, linux-pci, johnrose,
mochel
Hm,
Scratch the idea I outline below, seems like its not a good idea.
I'm reading the e100, e1000 and the ixgb power management code, and they
go through all sorts of steps I don't need to do for PCI device reset.
There's no clear abstraction that would serve both needs.
On Thu, Jun 30, 2005 at 03:39:31PM -0500, Linas Vepstas was heard to remark:
> On Wed, Jun 29, 2005 at 06:58:29PM +0200, Andi Kleen was heard to remark:
> > > Yep, OK. Pushig the timer would in fact break if the device was marked
> > > perm disabled.
> >
> > I think for network drivers you should just write a generic error handler
> > (perhaps in net/core/dev.c) that calls the watchdog handler.
> > Then all drivers could be easily converted without much code duplication.
>
> Well, there's no watchdog per-se in "struct net_device" -- are you
> suggesting I add one?
>
> It looks like I can almost create generic handlers for net devices;
> looks like calling netdev->stop() is enough to handle the error
> detection.
>
> However, a generic bringup would need to call pci_enable_device(),
> and net/core/dev.c does not include pci.h so I can't really do it
> there. Other than that, a generic recovry routine looks like it might
> be possible; I'll have to experiment; its hard to tell by reading code.
>
> This might be the wrong paradigm, though. The pci error recovery
> routines are *almost identical* to the power-management suspend/resume
> routines. From what I can tell, the only real difference is that
> I want to not actually turn off/on the power.
>
> Thus, the right thing to do might be to split up the
> struct pci_dev->suspend() and pci_dev->resume() calls into
>
> suspend()
> poweroff()
> poweron()
> resume()
>
> and then have the generic pci error recovery routines call
> suspend/resume only, skipping the poweroff-on calls. Does that
> sound good?
>
> I'm not sure I can pull this off without having someone from
> the power-management world throw a brick at me.
>
> --linas
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: PCI Power management (was: Re: [PATCH 4/13]: PCI Err: e100 ethernet driver recovery
2005-06-30 20:39 ` PCI Power management (was: Re: [PATCH 4/13]: PCI Err: e100 ethernet driver recovery Linas Vepstas
2005-06-30 21:07 ` Linas Vepstas
@ 2005-06-30 23:32 ` Benjamin Herrenschmidt
1 sibling, 0 replies; 3+ messages in thread
From: Benjamin Herrenschmidt @ 2005-06-30 23:32 UTC (permalink / raw)
To: Linas Vepstas
Cc: sfr, Hidetoshi Seto, long, Greg KH, linux-laptop, linux-kernel,
Andi Kleen, pavel, Paul Mackerras, linuxppc64-dev, linux-pci,
johnrose, mochel
On Thu, 2005-06-30 at 15:39 -0500, Linas Vepstas wrote:
> Thus, the right thing to do might be to split up the
> struct pci_dev->suspend() and pci_dev->resume() calls into
>
> suspend()
> poweroff()
> poweron()
> resume()
No. There are very good reasons not to do that split at the pci_dev
level.
> and then have the generic pci error recovery routines call
> suspend/resume only, skipping the poweroff-on calls. Does that
> sound good?
>
> I'm not sure I can pull this off without having someone from
> the power-management world throw a brick at me.
Just keep the error recovery callbacks for now, and we might be able to
provide a generic "helper" doing the watchdog thing (yes, there is a
watchdog in the net core)
Ben.
^ permalink raw reply [flat|nested] 3+ messages in thread