Cc: Ivan Kokshaysky <ink@...>, Greg KH <greg@...>, Matthew Wilcox <matthew@...>, Linus Torvalds <torvalds@...>, Greg KH <gregkh@...>, <linux-kernel@...>, Jeff Garzik <jeff@...>, <linux-pci@...>, Benjamin Herrenschmidt <benh@...>, Martin Mares <mj@...>, Tony Camuso <tcamuso@...>
conf1 has been a hardcoded dependencies of mmconfig for years. Ivan's
patch does not make it worse (in fact it considerably simplifies that
code, making it easier to untangle later).
IMHO, either your patch or Ivan's can be a good base, but:
1) For your remark above to be given any consideration, your patch
should be modified to remove the hardcoded conf1 from the *current*
mmconfig code, otherwise we end up with 3 set of ops (mmconfig + conf1+
a possible third set of operations) intertwined in a confusing manner.
And removing that dependency is not a straightforward operation unless
you also do 2):
2) the pci_enable_ext_config() function and dev->ext_cfg_space field,
sysfs interface should be removed from the patch. There has never been
a problem reporting crashes or any undefined behaviour while trying to
access ext-conf-space, all the problems where *using mmconfig to access
legacy-conf-space*. The "if (dev->cfg_space_ext > 0)" checks can instead
be replaced by "if (reg >= 256)".
Otherwise when using per-device explicit enabling, just *checking*
whether ext-conf-space is available by calling pci_enable_ext_config(),
will make some of the old problems of *loosing legacy conf-space* come
back: you would have introduced a new user-space and kernel API while
only solving half the problems, not a good deal.
if you do 1) and 2), then you really support the good properties you
claimed:
- You can use mmconfig for ext-space and something else for legacy-space.
- You can use mmconfig for everything (for instance if conf1 is not
implemented).
Of course it is as straightforward to modify Ivan's patch to also have
the same properties.
Loic
Loic
--
