Re: [PATCH 1/2] PCI: Introduce pci_alloc_dev(struct pci_bus*) to replace alloc_pci_dev()

[Gu Zheng <guz.fnst@cn.fujitsu.com>]

On 04/20/2013 10:58 AM, Mike Qiu wrote:

> On Fri, 2013-04-19 at 11:32 -0600, Bjorn Helgaas wrote:
>> On Fri, Apr 19, 2013 at 3:44 AM, Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:
>>> From 906167d9a09babbe189f62944ecb8c0b198a0f64 Mon Sep 17 00:00:00 2001
>>> From: Gu Zheng <guz.fnst@cn.fujitsu.com>
>>> Date: Fri, 19 Apr 2013 18:12:32 +0900
>>> Subject: [PATCH 1/2] PCI: Introduce  pci_alloc_dev(struct pci_bus*) to replace alloc_pci_dev()
>>>
>>> Now here we introduce a new struct pci_dev *pci_alloc_dev(struct pci_bus *bus) to replace alloc_pci_dev().
>>> It take a "struct pci_bus *" argument, so we can alloc a pci device on a target pci bus, and it acquire
>>> the reference of the pci_bus.
>>> Since the old alloc_pci_dev() is exported, so we still keep it for a while but mark it as __deprecated.
>>>
>>> Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
>>> ---
>>>  drivers/pci/probe.c |   21 ++++++++++++++++++++-
>>>  include/linux/pci.h |    4 +++-
>>>  2 files changed, 23 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>>> index b494066..5233fb6 100644
>>> --- a/drivers/pci/probe.c
>>> +++ b/drivers/pci/probe.c
>>> @@ -1199,7 +1199,7 @@ static void pci_release_bus_bridge_dev(struct device *dev)
>>>         kfree(bridge);
>>>  }
>>>
>>> -struct pci_dev *alloc_pci_dev(void)
>>> +struct pci_dev *pci_alloc_dev(struct pci_bus *bus)
>>>  {
>>>         struct pci_dev *dev;
>>>
>>> @@ -1209,6 +1209,25 @@ struct pci_dev *alloc_pci_dev(void)
>>>
>>>         INIT_LIST_HEAD(&dev->bus_list);
>>>
>>> +       if (bus) {
>>> +               get_device(&bus->dev);
>>> +               dev->bus = bus;
>>> +       }
>>> +
>>> +       return dev;
>>> +}
>>> +EXPORT_SYMBOL(pci_alloc_dev);
>>> +
>>> +__deprecated struct pci_dev *alloc_pci_dev(void)
>>
>> I don't think there's any point in marking the function *definition*
>> as deprecated; it only makes sense for the declaration in the header
>> file, so callers of the function will generate warnings.
>>
>>> +{
>>> +       struct pci_dev *dev;
>>> +       printk(KERN_DEBUG "alloc_pci_dev is deprecated, please use pci_alloc_dev(struct pci_bus *) instead!\n");
>>
>> I don't want to print a message at run-time, because users will see
>> the message and complain about it, but they can't do anything about
>> it.  And neither can we, because it will only be out-of-tree modules
>> that call alloc_pci_dev().  The build-time warning is all we can do.
> Yes, totally agree .
>>
>>> +       dev = kzalloc(sizeof(struct pci_dev), GFP_KERNEL);
>>> +       if (!dev)
>>> +               return NULL;
>>> +
>>> +       INIT_LIST_HEAD(&dev->bus_list);
>>
>> Can't you implement this as simply:
>>
>>     return pci_alloc_dev(NULL);
> Yes, I think it's better to keep the old API, and modify it to call the
> new one, this makes the modules which call the old fashion API feel
> smoothly. and the new modules can use the new API.

Hi Mike,
    You are right! Thanks for your suggestion!

Regards,
Gu

>>
>>> +
>>>         return dev;
>>>  }
>>>  EXPORT_SYMBOL(alloc_pci_dev);
>>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>>> index 710067f..682de2b 100644
>>> --- a/include/linux/pci.h
>>> +++ b/include/linux/pci.h
>>> @@ -348,7 +348,9 @@ static inline struct pci_dev *pci_physfn(struct pci_dev *dev)
>>>         return dev;
>>>  }
>>>
>>> -extern struct pci_dev *alloc_pci_dev(void);
>>> +extern struct pci_dev *pci_alloc_dev(struct pci_bus *bus);
>>> +
>>> +extern __deprecated struct pci_dev *alloc_pci_dev(void);
>>>
>>>  #define        to_pci_dev(n) container_of(n, struct pci_dev, dev)
>>>  #define for_each_pci_dev(d) while ((d = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, d)) != NULL)
>>> --
>>> 1.7.1
>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
> 
> 
>