Re: Concerns about our pci_{save,restore}_state()

[Greg KH <greg@kroah.com>]

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