Date: Tue, 01 Jun 2004 15:54:43 +0200
Message-ID: <s5hllj7qt7g.wl@alsa2.suse.de>
From: Takashi Iwai <tiwai@suse.de>
To: Arjan van de Ven <arjanv@redhat.com>
Cc: Pavel Machek <pavel@ucw.cz>, greg@kroah.com, linux-kernel@vger.kernel.org
Subject: Re: Resume enhancement: restore pci config space
In-Reply-To: <20040531133834.GA5834@devserv.devel.redhat.com>
References: <20040526203524.GF2057@devserv.devel.redhat.com>
	<20040530184031.GF997@openzaurus.ucw.cz>
	<20040531133834.GA5834@devserv.devel.redhat.com>
User-Agent: Wanderlust/2.10.1 (Watching The Wheels) SEMI/1.14.5 (Awara-Onsen) FLIM/1.14.5 (Demachiyanagi) APEL/10.6 MULE XEmacs/21.4 (patch 13) (Rational FORTRAN) (i386-suse-linux)
MIME-Version: 1.0 (generated by SEMI 1.14.5 - "Awara-Onsen")
Content-Type: text/plain; charset=US-ASCII
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2432
Lines: 66

At Mon, 31 May 2004 15:38:34 +0200,
Arjan van de Ven wrote:
> 
> [1  <text/plain; us-ascii (7bit)>]
> On Sun, May 30, 2004 at 08:40:31PM +0200, Pavel Machek wrote:
> > Hi!
> > 
> > > One can rightfully argue that the driver resume method should do this, and
> > > yes that is right. So the patch only does it for devices that don't have a
> > > resume method. Like the main PCI bridge on my testbox of which the bios so
> > > nicely forgets to restore the bus master bit during resume.. With this patch
> > > my testbox resumes just fine while it, well, wasn't all too happy as you can
> > > imagine without a busmaster pci bridge.
> > ...
> > > +/* 
> > > + * Default resume method for devices that have no driver provided resume,
> > > + * or not even a driver at all.
> > > + */
> > > +static void pci_default_resume(struct pci_dev *pci_dev)
> > > +{
> > 
> > Perhaps this should not be static so that drivers don't
> > need to duplicate this?
> 
> I wonder if that is useful, can you see cases where it would be?
> I mean, all it does is provide a default handler for places that don't have
> one. All this is info drivers already have, if a driver chooses to implement
> it's resume handler I think they can do better than this (and thus don't
> need this helper). But... if you can come up with a reasonable use I don't
> oppose it. I do like to see a sane user first though before adding this to
> the driver API...

well, most drivers need more or less the similar procedure like
pci_default_suspend/resume(): enable/disable the pci device, toggle
busmastering, and store/restore the pci status. 
if default callbacks are exported, the driver callbacks can be
simplified, such as

int xxx_suspend(struct pci_dev *dev, u32 state)
{
	// ... do h/w specific things
	return pci_default_suspend(dev, state);
}

int xxx_resume(struct pci_dev *dev)
{
	int err;
	if ((err = pci_default_resume(dev)) < 0)
		return err;
	// ... do h/w specific
}


but IMO, the jobs of pci_default_suspend/resume() should be applied
always after/before calling driver's suspend/resume callbacks.

can they break anything potentially?


--
Takashi Iwai <tiwai@suse.de>		ALSA Developer - www.alsa-project.org
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/