| Message ID | 1440548737-7465-1-git-send-email-paul.gortmaker@windriver.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
Hi Paul, On Tue, Aug 25, 2015 at 08:25:37PM -0400, Paul Gortmaker wrote: > In commit f309d4443130bf814e991f836e919dca22df37ae ("platform_device: > better support builtin boilerplate avoidance") we introduced the > builtin_driver macro. > > Here we use that support and extend it to PCI driver registration, > so where a driver is clearly non-modular and builtin-only, we can > register it in a similar fashion. And existing code that is clearly > non-modular can be updated with the simple mapping of > > module_pci_driver(...) ---> builtin_pci_driver(...) > > We've essentially cloned the former to make the latter, and taken > out the remove/module_exit parts since those never get used in a > non-modular build of the code. Do you have any estimate of how many potential users of this there are? I took a quick look at users of module_pci_driver() (I see almost 300 of them in v4.3-rc1), and most of them look like legitimate modules. But the comment mentions replacing device_initcall() as well, so maybe there are more there? If only a couple would be converted to builtin_pci_driver(), I'm not sure it's worth it, because it does add more things to look at (builtin_pci_driver() in addition to module_pci_driver()). > Cc: Bjorn Helgaas <bhelgaas@google.com> > Cc: linux-pci@vger.kernel.org > Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com> > --- > include/linux/pci.h | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/include/linux/pci.h b/include/linux/pci.h > index 88bee285b93d..8da2758e7d0e 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -1187,6 +1187,17 @@ void pci_unregister_driver(struct pci_driver *dev); > module_driver(__pci_driver, pci_register_driver, \ > pci_unregister_driver) > > +/** > + * builtin_pci_driver() - Helper macro for registering a PCI driver > + * @__pci_driver: pci_driver struct > + * > + * Helper macro for PCI drivers which do not do anything special in their > + * init code. This eliminates a lot of boilerplate. Each driver may only > + * use this macro once, and calling it replaces device_initcall(...) The builtin_platform_driver() patches I see are single-line patches, so they don't look like they remove a *lot* of boilerplate. Bjorn > + */ > +#define builtin_pci_driver(__pci_driver) \ > + builtin_driver(__pci_driver, pci_register_driver) > + > struct pci_driver *pci_dev_driver(const struct pci_dev *dev); > int pci_add_dynid(struct pci_driver *drv, > unsigned int vendor, unsigned int device, > -- > 2.5.0 > -- 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
[Re: [PATCH] PCI: create builtin_pci_driver to avoid registration boilerplate] On 15/09/2015 (Tue 11:46) Bjorn Helgaas wrote: > Hi Paul, > > On Tue, Aug 25, 2015 at 08:25:37PM -0400, Paul Gortmaker wrote: > > In commit f309d4443130bf814e991f836e919dca22df37ae ("platform_device: > > better support builtin boilerplate avoidance") we introduced the > > builtin_driver macro. > > > > Here we use that support and extend it to PCI driver registration, > > so where a driver is clearly non-modular and builtin-only, we can > > register it in a similar fashion. And existing code that is clearly > > non-modular can be updated with the simple mapping of > > > > module_pci_driver(...) ---> builtin_pci_driver(...) > > > > We've essentially cloned the former to make the latter, and taken > > out the remove/module_exit parts since those never get used in a > > non-modular build of the code. > > Do you have any estimate of how many potential users of this there > are? I took a quick look at users of module_pci_driver() (I see > almost 300 of them in v4.3-rc1), and most of them look like legitimate > modules. But the comment mentions replacing device_initcall() as > well, so maybe there are more there? It doesn't appear to be a lot; maybe a couple. When I sent this, I had not completed much of the treewide audit, nor strayed outside of x86, but I sent it ahead of time since I was hoping to stage any infrastructure patches like this ahead of conversions to avoid cross subsystem dependencies. > > If only a couple would be converted to builtin_pci_driver(), I'm not > sure it's worth it, because it does add more things to look at > (builtin_pci_driver() in addition to module_pci_driver()). So the real goal I am after here, which may not have been clear from this one stand-alone patch, is to ensure people have what they need to make non-modular drivers clearly non-modular. Currently we have about 300+ drivers/files including module.h and using modular registration functions when they can only be built in. And along with that is about 5k worth of dead code in exit / unregistration functions that never has been used. In which case, the numbers maybe don't matter. If we have a specific modular registration function for subsystem X, then we probably should also have a non-modular/builtin parallel of the same thing. However, since there are so few in PCI, I guess we could replace non-modular users with it open coded, if you don't want to add the new alias/macro, i.e.: module_pci_driver(xyz) --> builtin_driver(xyz, pci_register_driver) [...] > > + * builtin_pci_driver() - Helper macro for registering a PCI driver > > + * @__pci_driver: pci_driver struct > > + * > > + * Helper macro for PCI drivers which do not do anything special in their > > + * init code. This eliminates a lot of boilerplate. Each driver may only > > + * use this macro once, and calling it replaces device_initcall(...) > > The builtin_platform_driver() patches I see are single-line patches, > so they don't look like they remove a *lot* of boilerplate. The comment applies here as much as it does to the same comment for the module_pci_driver macro ; I'm assuming it was in reference to what it would look like if none of the platform driver helper macros were used and the whole thing was open coded. Anyway, I'm open to suggestions here, now that you know the whole story. Paul. -- > > Bjorn > > > + */ > > +#define builtin_pci_driver(__pci_driver) \ > > + builtin_driver(__pci_driver, pci_register_driver) > > + > > struct pci_driver *pci_dev_driver(const struct pci_dev *dev); > > int pci_add_dynid(struct pci_driver *drv, > > unsigned int vendor, unsigned int device, > > -- > > 2.5.0 > > -- 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 Tue, Aug 25, 2015 at 08:25:37PM -0400, Paul Gortmaker wrote: > In commit f309d4443130bf814e991f836e919dca22df37ae ("platform_device: > better support builtin boilerplate avoidance") we introduced the > builtin_driver macro. > > Here we use that support and extend it to PCI driver registration, > so where a driver is clearly non-modular and builtin-only, we can > register it in a similar fashion. And existing code that is clearly > non-modular can be updated with the simple mapping of > > module_pci_driver(...) ---> builtin_pci_driver(...) > > We've essentially cloned the former to make the latter, and taken > out the remove/module_exit parts since those never get used in a > non-modular build of the code. > > Cc: Bjorn Helgaas <bhelgaas@google.com> > Cc: linux-pci@vger.kernel.org > Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com> Applied to pci/misc for v4.4, thanks, Paul! > --- > include/linux/pci.h | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/include/linux/pci.h b/include/linux/pci.h > index 88bee285b93d..8da2758e7d0e 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -1187,6 +1187,17 @@ void pci_unregister_driver(struct pci_driver *dev); > module_driver(__pci_driver, pci_register_driver, \ > pci_unregister_driver) > > +/** > + * builtin_pci_driver() - Helper macro for registering a PCI driver > + * @__pci_driver: pci_driver struct > + * > + * Helper macro for PCI drivers which do not do anything special in their > + * init code. This eliminates a lot of boilerplate. Each driver may only > + * use this macro once, and calling it replaces device_initcall(...) > + */ > +#define builtin_pci_driver(__pci_driver) \ > + builtin_driver(__pci_driver, pci_register_driver) > + > struct pci_driver *pci_dev_driver(const struct pci_dev *dev); > int pci_add_dynid(struct pci_driver *drv, > unsigned int vendor, unsigned int device, > -- > 2.5.0 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- 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/include/linux/pci.h b/include/linux/pci.h index 88bee285b93d..8da2758e7d0e 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -1187,6 +1187,17 @@ void pci_unregister_driver(struct pci_driver *dev); module_driver(__pci_driver, pci_register_driver, \ pci_unregister_driver) +/** + * builtin_pci_driver() - Helper macro for registering a PCI driver + * @__pci_driver: pci_driver struct + * + * Helper macro for PCI drivers which do not do anything special in their + * init code. This eliminates a lot of boilerplate. Each driver may only + * use this macro once, and calling it replaces device_initcall(...) + */ +#define builtin_pci_driver(__pci_driver) \ + builtin_driver(__pci_driver, pci_register_driver) + struct pci_driver *pci_dev_driver(const struct pci_dev *dev); int pci_add_dynid(struct pci_driver *drv, unsigned int vendor, unsigned int device,
In commit f309d4443130bf814e991f836e919dca22df37ae ("platform_device: better support builtin boilerplate avoidance") we introduced the builtin_driver macro. Here we use that support and extend it to PCI driver registration, so where a driver is clearly non-modular and builtin-only, we can register it in a similar fashion. And existing code that is clearly non-modular can be updated with the simple mapping of module_pci_driver(...) ---> builtin_pci_driver(...) We've essentially cloned the former to make the latter, and taken out the remove/module_exit parts since those never get used in a non-modular build of the code. Cc: Bjorn Helgaas <bhelgaas@google.com> Cc: linux-pci@vger.kernel.org Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com> --- include/linux/pci.h | 11 +++++++++++ 1 file changed, 11 insertions(+)