[PATCH v3] PCI: endpoint: pci-epf-vntb: reduce several globals to statics

[PATCH v3] PCI: endpoint: pci-epf-vntb: reduce several globals to statics

[Tom Rix]

sparse reports
drivers/pci/endpoint/functions/pci-epf-vntb.c:956:10: warning: symbol 'pci_space' was not declared. Should it be static?
drivers/pci/endpoint/functions/pci-epf-vntb.c:975:5: warning: symbol 'pci_read' was not declared. Should it be static?
drivers/pci/endpoint/functions/pci-epf-vntb.c:984:5: warning: symbol 'pci_write' was not declared. Should it be static?
drivers/pci/endpoint/functions/pci-epf-vntb.c:989:16: warning: symbol 'vpci_ops' was not declared. Should it be static?

These functions and variables are only used in pci-epf-vntb.c, so their storage
class specifiers should be static.

Fixes: ff32fac00d97 ("NTB: EPF: support NTB transfer between PCI RC and EP connection")
Signed-off-by: Tom Rix <trix@redhat.com>
---
v2,3 : Change commit prefix

---
 drivers/pci/endpoint/functions/pci-epf-vntb.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
index ebf7e243eefa..6f0775b1fec3 100644
--- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
+++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
@@ -953,7 +953,7 @@ static struct config_group *epf_ntb_add_cfs(struct pci_epf *epf,
 
 #define VPCI_BUS_NUM 0x10
 
-uint32_t pci_space[] = {
+static uint32_t pci_space[] = {
 	(VNTB_VID | (VNTB_PID << 16)),	//DeviceID, Vendor ID
 	0,		// status, Command
 	0xffffffff,	// Class code, subclass, prog if, revision id
@@ -972,7 +972,7 @@ uint32_t pci_space[] = {
 	0,		//Max Lat, Min Gnt, interrupt pin, interrupt line
 };
 
-int pci_read(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 *val)
+static int pci_read(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 *val)
 {
 	if (devfn == 0) {
 		memcpy(val, ((uint8_t *)pci_space) + where, size);
@@ -981,12 +981,12 @@ int pci_read(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 *
 	return -1;
 }
 
-int pci_write(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 val)
+static int pci_write(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 val)
 {
 	return 0;
 }
 
-struct pci_ops vpci_ops = {
+static struct pci_ops vpci_ops = {
 	.read = pci_read,
 	.write = pci_write,
 };
-- 
2.27.0

RE: [EXT] [PATCH v3] PCI: endpoint: pci-epf-vntb: reduce several globals to statics

[Frank Li]

> -----Original Message-----
> From: Tom Rix <trix@redhat.com>
> Sent: Monday, July 4, 2022 8:26 AM
> To: kishon@ti.com; lpieralisi@kernel.org; kw@linux.com;
> bhelgaas@google.com; jdmason@kudzu.us; Frank Li <frank.li@nxp.com>
> Cc: linux-pci@vger.kernel.org; linux-kernel@vger.kernel.org; Tom Rix
> <trix@redhat.com>
> Subject: [EXT] [PATCH v3] PCI: endpoint: pci-epf-vntb: reduce several globals
> to statics
> 
> Caution: EXT Email
> 
> sparse reports
> drivers/pci/endpoint/functions/pci-epf-vntb.c:956:10: warning: symbol
> 'pci_space' was not declared. Should it be static?
> drivers/pci/endpoint/functions/pci-epf-vntb.c:975:5: warning: symbol
> 'pci_read' was not declared. Should it be static?
> drivers/pci/endpoint/functions/pci-epf-vntb.c:984:5: warning: symbol
> 'pci_write' was not declared. Should it be static?
> drivers/pci/endpoint/functions/pci-epf-vntb.c:989:16: warning: symbol
> 'vpci_ops' was not declared. Should it be static?
> 
> These functions and variables are only used in pci-epf-vntb.c, so their storage
> class specifiers should be static.
> 
> Fixes: ff32fac00d97 ("NTB: EPF: support NTB transfer between PCI RC and EP
> connection")
> Signed-off-by: Tom Rix <trix@redhat.com>

Acked-by: Frank Li <Frank.Li@nxp.com>

> ---
> v2,3 : Change commit prefix
> 
> ---
>  drivers/pci/endpoint/functions/pci-epf-vntb.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c
> b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> index ebf7e243eefa..6f0775b1fec3 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> @@ -953,7 +953,7 @@ static struct config_group *epf_ntb_add_cfs(struct
> pci_epf *epf,
> 
>  #define VPCI_BUS_NUM 0x10
> 
> -uint32_t pci_space[] = {
> +static uint32_t pci_space[] = {
>         (VNTB_VID | (VNTB_PID << 16)),  //DeviceID, Vendor ID
>         0,              // status, Command
>         0xffffffff,     // Class code, subclass, prog if, revision id
> @@ -972,7 +972,7 @@ uint32_t pci_space[] = {
>         0,              //Max Lat, Min Gnt, interrupt pin, interrupt line
>  };
> 
> -int pci_read(struct pci_bus *bus, unsigned int devfn, int where, int size, u32
> *val)
> +static int pci_read(struct pci_bus *bus, unsigned int devfn, int where, int
> size, u32 *val)
>  {
>         if (devfn == 0) {
>                 memcpy(val, ((uint8_t *)pci_space) + where, size);
> @@ -981,12 +981,12 @@ int pci_read(struct pci_bus *bus, unsigned int
> devfn, int where, int size, u32 *
>         return -1;
>  }
> 
> -int pci_write(struct pci_bus *bus, unsigned int devfn, int where, int size, u32
> val)
> +static int pci_write(struct pci_bus *bus, unsigned int devfn, int where, int
> size, u32 val)
>  {
>         return 0;
>  }
> 
> -struct pci_ops vpci_ops = {
> +static struct pci_ops vpci_ops = {
>         .read = pci_read,
>         .write = pci_write,
>  };
> --
> 2.27.0

Re: [PATCH v3] PCI: endpoint: pci-epf-vntb: reduce several globals to statics

[Bjorn Helgaas]

On Mon, Jul 04, 2022 at 09:25:59AM -0400, Tom Rix wrote:
> sparse reports
> drivers/pci/endpoint/functions/pci-epf-vntb.c:956:10: warning: symbol 'pci_space' was not declared. Should it be static?
> drivers/pci/endpoint/functions/pci-epf-vntb.c:975:5: warning: symbol 'pci_read' was not declared. Should it be static?
> drivers/pci/endpoint/functions/pci-epf-vntb.c:984:5: warning: symbol 'pci_write' was not declared. Should it be static?
> drivers/pci/endpoint/functions/pci-epf-vntb.c:989:16: warning: symbol 'vpci_ops' was not declared. Should it be static?
> 
> These functions and variables are only used in pci-epf-vntb.c, so their storage
> class specifiers should be static.
> 
> Fixes: ff32fac00d97 ("NTB: EPF: support NTB transfer between PCI RC and EP connection")
> Signed-off-by: Tom Rix <trix@redhat.com>

Handled via Jon, I guess?

I'm unclear on the future direction of pci-epf-vntb.c.  Jon, are you
signing up to maintain this?  MAINTAINERS doesn't seem to reflect
that, even in next-20220712, so you're not being copied on everything.

If you are planning to merge and maintain this file, it would be
helpful to me if you acknowledge patches you merge so I know to ignore
them.

> ---
> v2,3 : Change commit prefix
> 
> ---
>  drivers/pci/endpoint/functions/pci-epf-vntb.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> index ebf7e243eefa..6f0775b1fec3 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> @@ -953,7 +953,7 @@ static struct config_group *epf_ntb_add_cfs(struct pci_epf *epf,
>  
>  #define VPCI_BUS_NUM 0x10
>  
> -uint32_t pci_space[] = {
> +static uint32_t pci_space[] = {
>  	(VNTB_VID | (VNTB_PID << 16)),	//DeviceID, Vendor ID
>  	0,		// status, Command
>  	0xffffffff,	// Class code, subclass, prog if, revision id
> @@ -972,7 +972,7 @@ uint32_t pci_space[] = {
>  	0,		//Max Lat, Min Gnt, interrupt pin, interrupt line
>  };
>  
> -int pci_read(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 *val)
> +static int pci_read(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 *val)
>  {
>  	if (devfn == 0) {
>  		memcpy(val, ((uint8_t *)pci_space) + where, size);
> @@ -981,12 +981,12 @@ int pci_read(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 *
>  	return -1;
>  }
>  
> -int pci_write(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 val)
> +static int pci_write(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 val)
>  {
>  	return 0;
>  }
>  
> -struct pci_ops vpci_ops = {
> +static struct pci_ops vpci_ops = {
>  	.read = pci_read,
>  	.write = pci_write,
>  };
> -- 
> 2.27.0
> 

Re: [PATCH v3] PCI: endpoint: pci-epf-vntb: reduce several globals to statics

[Jon Mason]

On Tue, Jul 12, 2022 at 03:05:27PM -0500, Bjorn Helgaas wrote:
> On Mon, Jul 04, 2022 at 09:25:59AM -0400, Tom Rix wrote:
> > sparse reports
> > drivers/pci/endpoint/functions/pci-epf-vntb.c:956:10: warning: symbol 'pci_space' was not declared. Should it be static?
> > drivers/pci/endpoint/functions/pci-epf-vntb.c:975:5: warning: symbol 'pci_read' was not declared. Should it be static?
> > drivers/pci/endpoint/functions/pci-epf-vntb.c:984:5: warning: symbol 'pci_write' was not declared. Should it be static?
> > drivers/pci/endpoint/functions/pci-epf-vntb.c:989:16: warning: symbol 'vpci_ops' was not declared. Should it be static?
> > 
> > These functions and variables are only used in pci-epf-vntb.c, so their storage
> > class specifiers should be static.
> > 
> > Fixes: ff32fac00d97 ("NTB: EPF: support NTB transfer between PCI RC and EP connection")
> > Signed-off-by: Tom Rix <trix@redhat.com>
> 
> Handled via Jon, I guess?
> 
> I'm unclear on the future direction of pci-epf-vntb.c.  Jon, are you
> signing up to maintain this?  MAINTAINERS doesn't seem to reflect
> that, even in next-20220712, so you're not being copied on everything.
> 
> If you are planning to merge and maintain this file, it would be
> helpful to me if you acknowledge patches you merge so I know to ignore
> them.

I massively dropped the ball on all the EPF stuff.  I appologize profusely.

I'm pulling it into my ntb tree bcause of the patch dependencies.  If
you want me to own this stuff because it has ntb in it, then I can do
a matainers entry to reflect it.  My assumption is that because it is
under the drivers/pci umbrella it is yours (unless you want me to own
it).  100% defer to your decision.

All that being said...
Sorry for the extremely long delay in response.  This series is in my
ntb branch and will be in my pull request for v5.20 which should be
going out later today.

Thanks,
Jon

> 
> > ---
> > v2,3 : Change commit prefix
> > 
> > ---
> >  drivers/pci/endpoint/functions/pci-epf-vntb.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> > index ebf7e243eefa..6f0775b1fec3 100644
> > --- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
> > +++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> > @@ -953,7 +953,7 @@ static struct config_group *epf_ntb_add_cfs(struct pci_epf *epf,
> >  
> >  #define VPCI_BUS_NUM 0x10
> >  
> > -uint32_t pci_space[] = {
> > +static uint32_t pci_space[] = {
> >  	(VNTB_VID | (VNTB_PID << 16)),	//DeviceID, Vendor ID
> >  	0,		// status, Command
> >  	0xffffffff,	// Class code, subclass, prog if, revision id
> > @@ -972,7 +972,7 @@ uint32_t pci_space[] = {
> >  	0,		//Max Lat, Min Gnt, interrupt pin, interrupt line
> >  };
> >  
> > -int pci_read(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 *val)
> > +static int pci_read(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 *val)
> >  {
> >  	if (devfn == 0) {
> >  		memcpy(val, ((uint8_t *)pci_space) + where, size);
> > @@ -981,12 +981,12 @@ int pci_read(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 *
> >  	return -1;
> >  }
> >  
> > -int pci_write(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 val)
> > +static int pci_write(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 val)
> >  {
> >  	return 0;
> >  }
> >  
> > -struct pci_ops vpci_ops = {
> > +static struct pci_ops vpci_ops = {
> >  	.read = pci_read,
> >  	.write = pci_write,
> >  };
> > -- 
> > 2.27.0
> > 

Re: [PATCH v3] PCI: endpoint: pci-epf-vntb: reduce several globals to statics

[Bjorn Helgaas]

On Fri, Aug 12, 2022 at 10:13:59AM -0400, Jon Mason wrote:
> On Tue, Jul 12, 2022 at 03:05:27PM -0500, Bjorn Helgaas wrote:
> > Handled via Jon, I guess?
> > 
> > I'm unclear on the future direction of pci-epf-vntb.c.  Jon, are you
> > signing up to maintain this?  MAINTAINERS doesn't seem to reflect
> > that, even in next-20220712, so you're not being copied on everything.
> > 
> > If you are planning to merge and maintain this file, it would be
> > helpful to me if you acknowledge patches you merge so I know to ignore
> > them.
> 
> I massively dropped the ball on all the EPF stuff.  I appologize profusely.

No worries, sounds like you're getting everything sorted out :)

> I'm pulling it into my ntb tree bcause of the patch dependencies.  If
> you want me to own this stuff because it has ntb in it, then I can do
> a matainers entry to reflect it.  My assumption is that because it is
> under the drivers/pci umbrella it is yours (unless you want me to own
> it).  100% defer to your decision.

This patch dependency thing feels like a one-time or at least unusual
situation.  Unless it becomes a problem, I think it makes sense to
keep the drivers/pci umbrella instead of carving bits out of the
middle.

Even if I continue to merge everything under drivers/pci, maybe we
should consider an update like this just so you get cc'd on updates to
these files?

diff --git a/MAINTAINERS b/MAINTAINERS
index 08a5d70ceef9..5bafe7e8c2b2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14466,6 +14466,7 @@ W:	https://github.com/jonmason/ntb/wiki
 T:	git git://github.com/jonmason/ntb.git
 F:	drivers/net/ntb_netdev.c
 F:	drivers/ntb/
+F:	drivers/pci/endpoint/functions/pci-epf-*ntb.c
 F:	include/linux/ntb.h
 F:	include/linux/ntb_transport.h
 F:	tools/testing/selftests/ntb/

Re: [PATCH v3] PCI: endpoint: pci-epf-vntb: reduce several globals to statics

[Jon Mason]

On Fri, Aug 12, 2022 at 3:21 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Fri, Aug 12, 2022 at 10:13:59AM -0400, Jon Mason wrote:
> > On Tue, Jul 12, 2022 at 03:05:27PM -0500, Bjorn Helgaas wrote:
> > > Handled via Jon, I guess?
> > >
> > > I'm unclear on the future direction of pci-epf-vntb.c.  Jon, are you
> > > signing up to maintain this?  MAINTAINERS doesn't seem to reflect
> > > that, even in next-20220712, so you're not being copied on everything.
> > >
> > > If you are planning to merge and maintain this file, it would be
> > > helpful to me if you acknowledge patches you merge so I know to ignore
> > > them.
> >
> > I massively dropped the ball on all the EPF stuff.  I appologize profusely.
>
> No worries, sounds like you're getting everything sorted out :)
>
> > I'm pulling it into my ntb tree bcause of the patch dependencies.  If
> > you want me to own this stuff because it has ntb in it, then I can do
> > a matainers entry to reflect it.  My assumption is that because it is
> > under the drivers/pci umbrella it is yours (unless you want me to own
> > it).  100% defer to your decision.
>
> This patch dependency thing feels like a one-time or at least unusual
> situation.  Unless it becomes a problem, I think it makes sense to
> keep the drivers/pci umbrella instead of carving bits out of the
> middle.
>
> Even if I continue to merge everything under drivers/pci, maybe we
> should consider an update like this just so you get cc'd on updates to
> these files?
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 08a5d70ceef9..5bafe7e8c2b2 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -14466,6 +14466,7 @@ W:      https://github.com/jonmason/ntb/wiki
>  T:     git git://github.com/jonmason/ntb.git
>  F:     drivers/net/ntb_netdev.c
>  F:     drivers/ntb/
> +F:     drivers/pci/endpoint/functions/pci-epf-*ntb.c
>  F:     include/linux/ntb.h
>  F:     include/linux/ntb_transport.h
>  F:     tools/testing/selftests/ntb/

Works for me.  I can do a patch to add this line to maintainers, send
it to the people on this thread, and pull it into my pending pull
request.

Thanks,
Jon