[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAErSpo6QzCy7cgN8boXu4zhGhTbjEG7RyJ6avaVKL_u=vGMOUw@mail.gmail.com>
Date: Fri, 2 Mar 2012 10:17:53 -0700
From: Bjorn Helgaas <bhelgaas@...gle.com>
To: Tadeusz Struk <tadeusz.struk@...el.com>
Cc: jbarnes@...tuousgeek.org, linux-kernel@...r.kernel.org,
kvm@...r.kernel.org, linux-pci@...r.kernel.org,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Subject: Re: [PATCH v2] Dynamically add and remove device specific reset functions
On Fri, Mar 2, 2012 at 10:06 AM, Tadeusz Struk <tadeusz.struk@...el.com> 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 linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists