Re: [PATCH] PCI: build Broadcom PAXC quirks unconditionally

From: Bjorn Helgaas <helgaas@kernel.org>
To: Ray Jui <ray.jui@broadcom.com>
Cc: Wei Liu <wei.liu@kernel.org>,
	linux-pci@vger.kernel.org, rjui@broadcom.com
Subject: Re: [PATCH] PCI: build Broadcom PAXC quirks unconditionally
Date: Tue, 10 Dec 2019 18:34:33 -0600	[thread overview]
Message-ID: <20191211003433.GA182827@google.com> (raw)
In-Reply-To: <8d3b9565-1426-20a8-4982-c680001bbbbf@broadcom.com>

On Tue, Dec 10, 2019 at 04:28:47PM -0800, Ray Jui wrote:
> 
> 
> On 12/10/19 4:09 PM, Bjorn Helgaas wrote:
> > On Fri, Nov 15, 2019 at 01:58:42PM +0000, Wei Liu wrote:
> > > CONFIG_PCIE_IPROC_PLATFORM only gets defined when the driver is built
> > > in.  Removing the ifdef will allow us to build the driver as a module.
> > > 
> > > Signed-off-by: Wei Liu <wei.liu@kernel.org>
> > > ---
> > > Alternatively, we can change the condition to:
> > > 
> > >    #ifdef CONFIG_PCIE_IPROC_PLATFORM || CONFIG_PCIE_IPROC_PLATFORM_MODULE
> > > .
> > > 
> > > I chose to remove the ifdef because that's what other quirks looked like
> > > in this file.
> > > ---
> > >   drivers/pci/quirks.c | 2 --
> > >   1 file changed, 2 deletions(-)
> > > 
> > > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> > > index 320255e5e8f8..cd0e7c18e717 100644
> > > --- a/drivers/pci/quirks.c
> > > +++ b/drivers/pci/quirks.c
> > > @@ -2381,7 +2381,6 @@ DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_BROADCOM,
> > >   			 PCI_DEVICE_ID_TIGON3_5719,
> > >   			 quirk_brcm_5719_limit_mrrs);
> > > -#ifdef CONFIG_PCIE_IPROC_PLATFORM
> > >   static void quirk_paxc_bridge(struct pci_dev *pdev)
> > >   {
> > >   	/*
> > > @@ -2405,7 +2404,6 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_BROADCOM, 0x16f0, quirk_paxc_bridge);
> > >   DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_BROADCOM, 0xd750, quirk_paxc_bridge);
> > >   DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_BROADCOM, 0xd802, quirk_paxc_bridge);
> > >   DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_BROADCOM, 0xd804, quirk_paxc_bridge);
> > > -#endif
> > 
> > Is there a reason this quirk can't be moved to
> > drivers/pci/controller/pcie-iproc-platform.c?  That would make it much
> > less subtle because it would be compiled if and only if the driver
> > itself is compiled.
> > 
> > If it needs to be here in quirks.c, please include a note about the
> > reason.
> 
> There's no particular reason and yes it could be moved to pcie-iproc.c.
> 
> If that's preferred (and it sounds like it is) then we can do that.

Yes, please, that would be great!  No #ifdefs, plus the code won't be
compiled into x86 and other arches that never use that driver.

Bjorn