On Fri, Oct 19, 2018 at 10:58:00PM -0400, Sinan Kaya wrote: > On 10/19/2018 10:09 PM, Bjorn Helgaas wrote: > > On Fri, Oct 19, 2018 at 02:11:21AM +0000, Sinan Kaya wrote: > > > We need a contract between the reset API users and the PCI core about the > > > types of reset that a user needs vs. what PCI core can do internally. > > > If a platform supports hotplug, we need to do hotplug reset as an example. > > > > Somewhere this needs a description of a bug that's being fixed or some > > other justification, e.g., code simplification. The above is a little > > too abstract for me to grasp it. > > This series was in response to a request from Alex to have external users > some level of control on what PCI core can do internally. > > On the other hand, I have been posting patches to remove direct access from > external users to PCI core internals and hide all reset semantics like > save/restore from the users. I like giving less power to the users :) That's a good goal. Along the way we might need to tweak the users to align their usage to some common cases. > > > Expose the reset types to the drivers and try different reset types based > > > on the new reset_type parameter. > > > > > > Most users are expected to use PCI_RESET_ANY, PCI_RESET_FUNC or > > > PCI_RESET_LINK parameters. > > > > There are fewer than a dozen callers of all these functions and the > > complication of these interfaces doesn't seem commensurate with the > > problem. With six different interfaces and five independent bit > > flags, the possibilities are way more than necessary. > > True, I posted an RFC to reduce 5 function reset API flavors to 1 and > rely on the flags. I don't like the flags. Even one API with five bit flags has 32 possible combinations, most of which are meaningless. > Even these generic reset examples are not really generic today. Some of them > call reset from probe path and use the locked reset API > (pci_reset_function_locked()). > > Others call the reset but prefer to not save/restore context. > (__pci_reset_function_locked()). Some drivers even implemented their own > context save/restore code themselves. > > There are others that do obtain the lock as well as save/restore context. > (pci_reset_function()) > > I really don't like having these many reset API flavors and I thought > we could do something about that. I agree we have too many flavors. I'm not convinced all the variation in save/restore is necessary; I suspect most of that is accidental and drivers could probably all live with a single solution. > We can also drop the series if we think that current API are good enough > and nuances are well understood. I don't think the current API is good enough :) It's just a small matter of sorting out a better one. Bjorn
