On Thu, Aug 15, 2019 at 10:30:03AM -0700, Kuppuswamy Sathyanarayanan wrote:
> On Wed, Aug 14, 2019 at 11:46:57PM -0500, Bjorn Helgaas wrote:
> > On Thu, Aug 01, 2019 at 05:05:59PM -0700, sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx wrote:
> > > From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx>
> > >
> > > Currently, PRI Capability checks are repeated across all PRI API's.
> > > Instead, cache the capability check result in pci_pri_init() and use it
> > > in other PRI API's. Also, since PRI is a shared resource between PF/VF,
> > > initialize default values for common PRI features in pci_pri_init().
> > >
> > > Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx>
> > > ---
> > > drivers/pci/ats.c | 80 ++++++++++++++++++++++++++++-------------
> > > include/linux/pci-ats.h | 5 +++
> > > include/linux/pci.h | 1 +
> > > 3 files changed, 61 insertions(+), 25 deletions(-)
> > >
> >
> > > diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c
> > > index cdd936d10f68..280be911f190 100644
> > > --- a/drivers/pci/ats.c
> > > +++ b/drivers/pci/ats.c
> >
> > > @@ -28,6 +28,8 @@ void pci_ats_init(struct pci_dev *dev)
> > > return;
> > >
> > > dev->ats_cap = pos;
> > > +
> > > + pci_pri_init(dev);
> > > }
> > >
> > > /**
> > > @@ -170,36 +172,72 @@ int pci_ats_page_aligned(struct pci_dev *pdev)
> > > EXPORT_SYMBOL_GPL(pci_ats_page_aligned);
> > >
> > > #ifdef CONFIG_PCI_PRI
> > > +
> > > +void pci_pri_init(struct pci_dev *pdev)
> > > +{
> > > ...
> > > +}
> >
> > > diff --git a/include/linux/pci-ats.h b/include/linux/pci-ats.h
> > > index 1a0bdaee2f32..33653d4ca94f 100644
> > > --- a/include/linux/pci-ats.h
> > > +++ b/include/linux/pci-ats.h
> > > @@ -6,6 +6,7 @@
> > >
> > > #ifdef CONFIG_PCI_PRI
> > >
> > > +void pci_pri_init(struct pci_dev *pdev);
> >
> > pci_pri_init() is implemented and called in drivers/pci/ats.c. Unless
> > there's a need to call this from outside ats.c, it should be static
> > and should not be declared here.
> >
> > If you can make it static, please also reorder the code so you don't
> > need a forward declaration in ats.c.
> Initially I did implement it as static function in drivers/pci/ats.c
> and protected the calling of pci_pri_init() with #ifdef CONFIG_PCI_PRI.
> But Keith did not like the implementation using #ifdefs and asked me to
> define empty functions. That's the reason for moving it to header file.
Defining empty functions doesn't mean it has to be in a header file.
It's only needed inside ats.c, so the whole thing should be static
there. You can easily #ifdef the implementation, e.g., do the
following in ats.c:
static void pci_pri_init(struct pci_dev *pdev)
{
#ifdef CONFIG_PCI_PRI
...
#endif
}
