On Fri, Mar 2, 2012 at 10:06 AM, Tadeusz Struk <tadeusz.struk@xxxxxxxxx> wrote: > On 02/03/12 16:29, Bjorn Helgaas wrote: >> Where do you plan to add calls to pci_dev_specific_reset_add()? In >> drivers? > > Yes, I'm working on a driver for a device with SRIOV capability. > I'll call it from there. > >> Did you consider adding a "reset" function pointer to struct >> pci_driver? That might be more natural -- the reset function is right >> with all the other code that knows about the device, and there's no >> issue with looking up the correct reset function. >> With this patch, we sort of have two different ways to map >> vendor/device IDs to code: the usual pci_register_driver() approach, >> and this one using reset_list. If pci_driver had a "reset" pointer, >> that would be used most of the time. You might still need the >> reset_list for generic things, e.g., the reset_intel_generic_dev() >> function, but it would be a fallback. It might look something like: >> >> struct pci_driver *drv = dev->driver; >> >> if (drv && drv->reset) { >> drv->reset(dev); >> return; >> } >> >> list_for_each_entry(i, &reset_list, list) { >> ... > > No, I didn't think about it. > This is good idea, but for me the pci_dev_specific_reset() works fine. I know your patch works fine, but I think we should have the discussion about whether adding a struct pci_driver pointer is a better long-term solution. Greg, Jesse, others, chime in if you have any thoughts. Bjorn -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html