On Saturday 20 October 2007 07:04:35 Benjamin Herrenschmidt wrote: > > > 1) some drivers use pci_disable_device(), and pci_enable_device(). > > should I use it too? > > I generally don't do the former, and I would expect the late to be done > by pci_restore_state() for you. pci_disable_device(), last I looked, > only cleared the bus master bit though, which might be a good idea to > do. > > People in ACPI/x86 land, are there more good reasons to do one or the > other ? > > That reminds me that I volunteered to write a documentation on how > drivers should do all that stuff at KS and didn't get to actually doing > it yet. shame ... I'll try to start something asap. > > > 2) I accidentally did this: > > pci_set_power_state(pci_dev, pci_choose_state(pci_dev, state)); > > pci_save_state(pci_dev); > > > > I somehow thought that this is correct, that I should save the pci config > > state > > after the power-down, but now I know that it isn't correct. > > Right, you need to do the save_state before the power down. You need to > avoid pretty much any access to the device after the save state other > than the pending set_power_state on resume. > > > I now need to send a patch for dmfe.c network driver that has the same > > commands written by me. > > (but it works perfectly anyway) > > On x86 desktop... might have surprises on others. > > > Is it possible to access pci configuration space in D3? > > It's only really safe to access the PM register itself, though I suppose > you should be able to walk the capability chain to do that. But I > wouldnt recommend doing anything else. > > > And lastly speaking of network drivers, one issue came to my mind: > > most network drivars has a packet queue and in case of dmfe it is located > > in main memory, > > and card does dma from it. > > Note that the network stack nowadays does a fair bit of cleaning up for > you before your suspend routine is called.... > > > > in .suspend I ignore that some packets may be in that queue, and I want > > to ask, whenever there are better ways to do that. > > > > > > this is my dmfe .suspend routine. > > > > /* Disable upper layer interface */ > > netif_device_detach(dev); >
> > Looks allright on a quick glance appart from the bits we already > discussed. > > > I guess, everybody makes mistakes... :-) > > > > Other network drivers has a bit more complicated .suspend/.resume routines, > > but I didn't see a driver waiting for output queue to finish > > I think the network stack does that nowadays but we'll have to double > check, that's based on what DaveM told me at KS. > > Ben. > > Hi, Thanks a lot. I fix the order of calls in dmfe.c and in saa7134-core.c. I probably need to add this synchronize_irq() logic in dmfe.c too, but I probably do it later, I think I am overestimating this race, since most drivers don't do dev->insuspend checks in IRQ handler. Maybe even just use free_irq() after all.... Best regards, Maxim Levitsky - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/