[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CADaLND=cY=OcUy8Z5Q_LeijyobifFJy7uACOmvX5eKqGhSSmdw@mail.gmail.com>
Date: Mon, 11 Jul 2016 10:12:15 -0700
From: Duc Dang <dhdang@....com>
To: Paul Gortmaker <paul.gortmaker@...driver.com>
Cc: Tanmay Inamdar <tinamdar@....com>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Bjorn Helgaas <bhelgaas@...gle.com>,
"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>
Subject: Re: [PATCH 14/14] PCI: xgene: make it explicitly non-modular
On Sat, Jul 9, 2016 at 16:15 Paul Gortmaker
<paul.gortmaker@...driver.com> 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@....com> wrote:
> > >
> > >
> > > On Sat, Jul 2, 2016 at 4:13 PM, Paul Gortmaker
> > > <paul.gortmaker@...driver.com> 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@....com>
> > >> Cc: Bjorn Helgaas <bhelgaas@...gle.com>
> > >> Cc: linux-pci@...r.kernel.org
> > >> Signed-off-by: Paul Gortmaker <paul.gortmaker@...driver.com>
> >
> > 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@....com>");
> > >> -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.
Powered by blists - more mailing lists