Re: [PATCH 14/14] PCI: xgene: make it explicitly non-modular

[Duc Dang]

On Sat, Jul 9, 2016 at 16:15 Paul Gortmaker
<paul.gortmaker@xxxxxxxxxxxxx> wrote:
>
> [Re: [PATCH 14/14] PCI: xgene: make it explicitly non-modular] On 07/07/2016 (Thu 15:42) Duc Dang wrote:
>
> > On Thu, Jul 7, 2016 at 3:35 PM, Tanmay Inamdar <tinamdar@xxxxxxx> wrote:
> > >
> > >
> > > On Sat, Jul 2, 2016 at 4:13 PM, Paul Gortmaker
> > > <paul.gortmaker@xxxxxxxxxxxxx> wrote:
> > >>
> > >> The Kconfig currently controlling compilation of this code is:
> > >>
> > >> drivers/pci/host/Kconfig:config PCI_XGENE
> > >> drivers/pci/host/Kconfig:       bool "X-Gene PCIe controller"
> > >>
> > >> ...meaning that it currently is not being built as a module by anyone.
> > >>
> > >> Lets remove the few trace uses of modular code and macros, so that
> > >> when reading the driver there is no doubt it is builtin-only.
> > >>
> > >> Since module_platform_driver() uses the same init level priority as
> > >> builtin_platform_driver() the init ordering remains unchanged with
> > >> this commit.
> > >>
> > >> We also delete the MODULE_LICENSE tag etc. since all that information
> > >> is already contained at the top of the file in the comments.
> > >>
> > >> Cc: Tanmay Inamdar <tinamdar@xxxxxxx>
> > >> Cc: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
> > >> Cc: linux-pci@xxxxxxxxxxxxxxx
> > >> Signed-off-by: Paul Gortmaker <paul.gortmaker@xxxxxxxxxxxxx>
> >
> > Thanks for taking care of this, Paul.
> >
> > I tested your patch and it worked fine on my X-Gene Mustang board.
> >
> > One minor comment below.
> >
> > >> ---
> > >>  drivers/pci/host/pci-xgene.c | 8 ++------
> > >>  1 file changed, 2 insertions(+), 6 deletions(-)
> > >>
> > >> diff --git a/drivers/pci/host/pci-xgene.c b/drivers/pci/host/pci-xgene.c
> > >> index 7eb20cc76dd3..a81273c23341 100644
> > >> --- a/drivers/pci/host/pci-xgene.c
> > >> +++ b/drivers/pci/host/pci-xgene.c
> > >> @@ -21,7 +21,7 @@
> > >>  #include <linux/io.h>
> > >>  #include <linux/jiffies.h>
> > >>  #include <linux/memblock.h>
> > >> -#include <linux/module.h>
> > >> +#include <linux/init.h>
> >
> > The platform_device.h already has builtin_platform_driver macro
> > defined. So this init.h is not need?
>
> If you look, you will find that platform_device.h does not include the
> init.h even though it references __init; it can do this w/o error since
> all the references themselves are in a macro.  However once code wants
> to be a consumer of those macros, they will need init.h present.  Often
> you can overlook directly calling it out for inclusion since it gets
> sourced by another header, but it is best policy to list what gets used.

Ah, got it.

Thanks, Paul!
>
> Thanks for testing!
>
> Paul.
> --
>
> >
> > >>  #include <linux/of.h>
> > >>  #include <linux/of_address.h>
> > >>  #include <linux/of_irq.h>
> > >> @@ -579,8 +579,4 @@ static struct platform_driver xgene_pcie_driver = {
> > >>         },
> > >>         .probe = xgene_pcie_probe_bridge,
> > >>  };
> > >> -module_platform_driver(xgene_pcie_driver);
> > >> -
> > >> -MODULE_AUTHOR("Tanmay Inamdar <tinamdar@xxxxxxx>");
> > >> -MODULE_DESCRIPTION("APM X-Gene PCIe driver");
> > >> -MODULE_LICENSE("GPL v2");
> > >> +builtin_platform_driver(xgene_pcie_driver);
> > >
> > >
> > > Copying Duc.
> > >>
> > >> --
> > >> 2.8.4
> > >>
> > >
> > Regards,
> > Duc Dang.