| Message ID | 51711214.8050606@cn.fujitsu.com |
|---|---|
| State | Rejected |
| Headers | show |
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. > + 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); > + > 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
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. > > > + > > 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 > -- 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
Hi Bjorn, Sorry for the later reply! On 04/20/2013 01:32 AM, 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. Thanks for your explaining, I'll fix it in the right way. > >> +{ >> + 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. Yeah, it seems useless here. I'll remove it. > >> + 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); Agree, it can simplify the code, and make the later removal of the old alloc_pci_dev() smoothly. > >> + >> 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
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 >> > > > -- 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
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) +{ + struct pci_dev *dev; + printk(KERN_DEBUG "alloc_pci_dev is deprecated, please use pci_alloc_dev(struct pci_bus *) instead!\n"); + dev = kzalloc(sizeof(struct pci_dev), GFP_KERNEL); + if (!dev) + return NULL; + + INIT_LIST_HEAD(&dev->bus_list); + 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)