On Mon, Oct 25, 2004 at 02:06:22PM +1000, Benjamin Herrenschmidt wrote: > Hi Greg ! > > I was looking at our "generic" pci_save_state() and pci_restore_state() > and I have various concerns with them, I was wondering what you though > about them... Note, these concerns are the same before the last pci_save_state() changes, right? I didn't break anything new did I? :) > - We should always write the command register after all the BARs, > typically that mean write it back _last_ Hm, probably. I'm away from my PCI book, so I don't really know about that one. For some reason we've been ok so far... > - We shouldn't write to the BIST register, it is defined as having > side effects and writing to it any value may trigger a BIST on the > card, with all the possible bad consequences that has yeah, good point. I guess most of these cards don't have BIST stuff in them. Or just writing back the read value is sane. I'll dig through the PCI book next week. > - What about saving/restoring more registers ? I'm not sure wether it > should be the responsibility of the driver to save and restore things > above dword 15, but we should at least deal with the case of P2P bridges > who have more "standard" registers We need to have a "bridge" driver for something like that. I think lots of things would benifit if we had that. But if we had that, we need a way to overload (or weight) different drivers that might bind to the same device. This is something that we've talked about for a while now, and it's on my list of things to do in the near future. I think once we get that, a "generic" bridge driver will be ok to have. Any hardware specific quirks needed can also just be their own driver (I think Red Hat ran into odd issues when they tried to add a pci bridge driver to one of their older kernel trees.) Oh, and yeah, we should probably save and restore pci express config space too. I need to go look to see if pci x 2.0 also has a expanded config or not... thanks, greg k-h