| Message ID | 20220704132559.2859918-1-trix@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v3] PCI: endpoint: pci-epf-vntb: reduce several globals to statics | expand |
> -----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
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 >
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 > >
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/
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
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, };
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(-)