* [PATCH] pci: Use designated initialization in PCI_VDEVICE @ 2014-03-31 21:58 Jeff Kirsher 2014-03-31 22:08 ` Sergei Shtylyov 0 siblings, 1 reply; 6+ messages in thread From: Jeff Kirsher @ 2014-03-31 21:58 UTC (permalink / raw) To: bhelgaas; +Cc: Mark Rustad, linux-pci, netdev, Jeff Kirsher From: Mark Rustad <mark.d.rustad@intel.com> By using designated initialization in PCI_VDEVICE, like other similar macros, many "missing initializer" warnings that appear when compiling with W=2 can be silenced. Signed-off-by: Mark Rustad <mark.d.rustad@intel.com> Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com> Tested-by: Aaron Brown <aaron.f.brown@intel.com> Tested-by: Kavindya Deegala <kavindya.s.deegala@intel.com> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com> --- include/linux/pci.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/include/linux/pci.h b/include/linux/pci.h index fb57c89..49455f9 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -680,8 +680,8 @@ struct pci_driver { /** * PCI_VDEVICE - macro used to describe a specific pci device in short form - * @vendor: the vendor name - * @device: the 16 bit PCI Device ID + * @vend: the vendor name + * @dev: the 16 bit PCI Device ID * * This macro is used to create a struct pci_device_id that matches a * specific PCI device. The subvendor, and subdevice fields will be set @@ -689,9 +689,9 @@ struct pci_driver { * private data. */ -#define PCI_VDEVICE(vendor, device) \ - PCI_VENDOR_ID_##vendor, (device), \ - PCI_ANY_ID, PCI_ANY_ID, 0, 0 +#define PCI_VDEVICE(vend, dev) \ + .vendor = PCI_VENDOR_ID_##vend, .device = (dev), \ + .subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID, 0, 0 /* these external functions are only available when PCI support is enabled */ #ifdef CONFIG_PCI -- 1.9.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] pci: Use designated initialization in PCI_VDEVICE 2014-03-31 21:58 [PATCH] pci: Use designated initialization in PCI_VDEVICE Jeff Kirsher @ 2014-03-31 22:08 ` Sergei Shtylyov 2014-04-24 21:22 ` Bjorn Helgaas 0 siblings, 1 reply; 6+ messages in thread From: Sergei Shtylyov @ 2014-03-31 22:08 UTC (permalink / raw) To: Jeff Kirsher, bhelgaas; +Cc: Mark Rustad, linux-pci, netdev Hello. On 04/01/2014 01:58 AM, Jeff Kirsher wrote: > From: Mark Rustad <mark.d.rustad@intel.com> > By using designated initialization in PCI_VDEVICE, like other > similar macros, many "missing initializer" warnings that appear > when compiling with W=2 can be silenced. > Signed-off-by: Mark Rustad <mark.d.rustad@intel.com> > Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com> > Tested-by: Aaron Brown <aaron.f.brown@intel.com> > Tested-by: Kavindya Deegala <kavindya.s.deegala@intel.com> > Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com> > --- > include/linux/pci.h | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > diff --git a/include/linux/pci.h b/include/linux/pci.h > index fb57c89..49455f9 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h [...] > @@ -689,9 +689,9 @@ struct pci_driver { > * private data. > */ > > -#define PCI_VDEVICE(vendor, device) \ > - PCI_VENDOR_ID_##vendor, (device), \ > - PCI_ANY_ID, PCI_ANY_ID, 0, 0 > +#define PCI_VDEVICE(vend, dev) \ > + .vendor = PCI_VENDOR_ID_##vend, .device = (dev), \ > + .subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID, 0, 0 Initializing the fields to 0 is pointless, as 0 is what should be put into them anyway by the compiler. Also, it doesn't look right when you mix designated and anonymous initializers. WBR, Sergei ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] pci: Use designated initialization in PCI_VDEVICE 2014-03-31 22:08 ` Sergei Shtylyov @ 2014-04-24 21:22 ` Bjorn Helgaas 2014-04-24 23:15 ` Rustad, Mark D 0 siblings, 1 reply; 6+ messages in thread From: Bjorn Helgaas @ 2014-04-24 21:22 UTC (permalink / raw) To: Sergei Shtylyov; +Cc: Jeff Kirsher, Mark Rustad, linux-pci, netdev On Tue, Apr 01, 2014 at 02:08:06AM +0400, Sergei Shtylyov wrote: > Hello. > > On 04/01/2014 01:58 AM, Jeff Kirsher wrote: > > >From: Mark Rustad <mark.d.rustad@intel.com> > > >By using designated initialization in PCI_VDEVICE, like other > >similar macros, many "missing initializer" warnings that appear > >when compiling with W=2 can be silenced. > > >Signed-off-by: Mark Rustad <mark.d.rustad@intel.com> > >Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com> > >Tested-by: Aaron Brown <aaron.f.brown@intel.com> > >Tested-by: Kavindya Deegala <kavindya.s.deegala@intel.com> > >Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com> > >--- > > include/linux/pci.h | 10 +++++----- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > >diff --git a/include/linux/pci.h b/include/linux/pci.h > >index fb57c89..49455f9 100644 > >--- a/include/linux/pci.h > >+++ b/include/linux/pci.h > [...] > >@@ -689,9 +689,9 @@ struct pci_driver { > > * private data. > > */ > > > >-#define PCI_VDEVICE(vendor, device) \ > >- PCI_VENDOR_ID_##vendor, (device), \ > >- PCI_ANY_ID, PCI_ANY_ID, 0, 0 > >+#define PCI_VDEVICE(vend, dev) \ > >+ .vendor = PCI_VENDOR_ID_##vend, .device = (dev), \ > >+ .subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID, 0, 0 > > Initializing the fields to 0 is pointless, as 0 is what should be > put into them anyway by the compiler. Also, it doesn't look right > when you mix designated and anonymous initializers. I'm certainly willing to apply this, but I can't reproduce the benefit. I tried "make W=2 drivers/net/ethernet/intel/e1000e/netdev.o" and didn't see any change before and after this patch (I'm using Ubuntu gcc 4.6.3 if it matters). What am I missing? And do we need the zeroes? Bjorn ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] pci: Use designated initialization in PCI_VDEVICE 2014-04-24 21:22 ` Bjorn Helgaas @ 2014-04-24 23:15 ` Rustad, Mark D 2014-04-24 23:43 ` Rustad, Mark D 2014-04-25 0:10 ` Bjorn Helgaas 0 siblings, 2 replies; 6+ messages in thread From: Rustad, Mark D @ 2014-04-24 23:15 UTC (permalink / raw) To: Bjorn Helgaas, Sergei Shtylyov Cc: Kirsher, Jeffrey T, linux-pci@vger.kernel.org, Netdev Bjorn, Sergei, On Apr 24, 2014, at 2:22 PM, Bjorn Helgaas <bhelgaas@google.com> wrote: > On Tue, Apr 01, 2014 at 02:08:06AM +0400, Sergei Shtylyov wrote: >> Hello. >> >> On 04/01/2014 01:58 AM, Jeff Kirsher wrote: >> >>> From: Mark Rustad <mark.d.rustad@intel.com> >> >>> By using designated initialization in PCI_VDEVICE, like other >>> similar macros, many "missing initializer" warnings that appear >>> when compiling with W=2 can be silenced. >> >>> Signed-off-by: Mark Rustad <mark.d.rustad@intel.com> >>> Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com> >>> Tested-by: Aaron Brown <aaron.f.brown@intel.com> >>> Tested-by: Kavindya Deegala <kavindya.s.deegala@intel.com> >>> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com> >>> --- >>> include/linux/pci.h | 10 +++++----- >>> 1 file changed, 5 insertions(+), 5 deletions(-) >> >>> diff --git a/include/linux/pci.h b/include/linux/pci.h >>> index fb57c89..49455f9 100644 >>> --- a/include/linux/pci.h >>> +++ b/include/linux/pci.h >> [...] >>> @@ -689,9 +689,9 @@ struct pci_driver { >>> * private data. >>> */ >>> >>> -#define PCI_VDEVICE(vendor, device) \ >>> - PCI_VENDOR_ID_##vendor, (device), \ >>> - PCI_ANY_ID, PCI_ANY_ID, 0, 0 >>> +#define PCI_VDEVICE(vend, dev) \ >>> + .vendor = PCI_VENDOR_ID_##vend, .device = (dev), \ >>> + .subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID, 0, 0 >> >> Initializing the fields to 0 is pointless, as 0 is what should be >> put into them anyway by the compiler. Also, it doesn't look right >> when you mix designated and anonymous initializers. The zeros can't be dropped because the macro needs to initialize the same number of fields as the previous macro. Callers of this macro do use undesignated initialization and will rely on how that aligns. This macro does that. > I'm certainly willing to apply this, but I can't reproduce the benefit. I > tried "make W=2 drivers/net/ethernet/intel/e1000e/netdev.o" and didn't see > any change before and after this patch (I'm using Ubuntu gcc 4.6.3 if it > matters). The ixgbe driver throws a lot of warnings because it does not provide the final field after the macro call, allowing it default to zero. I think I saw a few other instances around the kernel, but the largest number are in ixgbe. > What am I missing? And do we need the zeroes? If you build ixgbe with W=2, I think you'll see the noise. And the zeros are needed for the reason given above. The zeros could also be moved to designated initialization, but the C standard is pretty clear on how it works, and many callers of the macro will be using undesignated initialization for the final field anyway. That is, with the new macro, many initializations will effectively have a mixed form. If that really bothers you, then I guess you should drop the patch. I was just trying to silence a bunch of noise in a simple, direct way. Adding the missing field in all of those places would also silence the message, but it seemed reasonable to have require that. -- Mark Rustad, Networking Division, Intel Corporation ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] pci: Use designated initialization in PCI_VDEVICE 2014-04-24 23:15 ` Rustad, Mark D @ 2014-04-24 23:43 ` Rustad, Mark D 2014-04-25 0:10 ` Bjorn Helgaas 1 sibling, 0 replies; 6+ messages in thread From: Rustad, Mark D @ 2014-04-24 23:43 UTC (permalink / raw) To: Bjorn Helgaas, Sergei Shtylyov Cc: Kirsher, Jeffrey T, linux-pci@vger.kernel.org, Netdev On Apr 24, 2014, at 4:15 PM, Rustad, Mark D <mark.d.rustad@intel.com> wrote: <snip> > Adding the missing field in all of those places would also silence the message, but it seemed reasonable to have require that. I meant "it seemed reasonable to *not* have to require that". Hopefully that helps it make some sense. -- Mark Rustad, Networking Division, Intel Corporation ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] pci: Use designated initialization in PCI_VDEVICE 2014-04-24 23:15 ` Rustad, Mark D 2014-04-24 23:43 ` Rustad, Mark D @ 2014-04-25 0:10 ` Bjorn Helgaas 1 sibling, 0 replies; 6+ messages in thread From: Bjorn Helgaas @ 2014-04-25 0:10 UTC (permalink / raw) To: Rustad, Mark D Cc: Sergei Shtylyov, Kirsher, Jeffrey T, linux-pci@vger.kernel.org, Netdev On Thu, Apr 24, 2014 at 5:15 PM, Rustad, Mark D <mark.d.rustad@intel.com> wrote: > Bjorn, Sergei, > > On Apr 24, 2014, at 2:22 PM, Bjorn Helgaas <bhelgaas@google.com> wrote: > >> On Tue, Apr 01, 2014 at 02:08:06AM +0400, Sergei Shtylyov wrote: >>> Hello. >>> >>> On 04/01/2014 01:58 AM, Jeff Kirsher wrote: >>> >>>> From: Mark Rustad <mark.d.rustad@intel.com> >>> >>>> By using designated initialization in PCI_VDEVICE, like other >>>> similar macros, many "missing initializer" warnings that appear >>>> when compiling with W=2 can be silenced. >>> >>>> Signed-off-by: Mark Rustad <mark.d.rustad@intel.com> >>>> Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com> >>>> Tested-by: Aaron Brown <aaron.f.brown@intel.com> >>>> Tested-by: Kavindya Deegala <kavindya.s.deegala@intel.com> >>>> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com> >>>> --- >>>> include/linux/pci.h | 10 +++++----- >>>> 1 file changed, 5 insertions(+), 5 deletions(-) >>> >>>> diff --git a/include/linux/pci.h b/include/linux/pci.h >>>> index fb57c89..49455f9 100644 >>>> --- a/include/linux/pci.h >>>> +++ b/include/linux/pci.h >>> [...] >>>> @@ -689,9 +689,9 @@ struct pci_driver { >>>> * private data. >>>> */ >>>> >>>> -#define PCI_VDEVICE(vendor, device) \ >>>> - PCI_VENDOR_ID_##vendor, (device), \ >>>> - PCI_ANY_ID, PCI_ANY_ID, 0, 0 >>>> +#define PCI_VDEVICE(vend, dev) \ >>>> + .vendor = PCI_VENDOR_ID_##vend, .device = (dev), \ >>>> + .subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID, 0, 0 >>> >>> Initializing the fields to 0 is pointless, as 0 is what should be >>> put into them anyway by the compiler. Also, it doesn't look right >>> when you mix designated and anonymous initializers. > > The zeros can't be dropped because the macro needs to initialize the same number of fields as the previous macro. Callers of this macro do use undesignated initialization and will rely on how that aligns. This macro does that. > >> I'm certainly willing to apply this, but I can't reproduce the benefit. I >> tried "make W=2 drivers/net/ethernet/intel/e1000e/netdev.o" and didn't see >> any change before and after this patch (I'm using Ubuntu gcc 4.6.3 if it >> matters). > > The ixgbe driver throws a lot of warnings because it does not provide the final field after the macro call, allowing it default to zero. I think I saw a few other instances around the kernel, but the largest number are in ixgbe. > >> What am I missing? And do we need the zeroes? > > If you build ixgbe with W=2, I think you'll see the noise. And the zeros are needed for the reason given above. The zeros could also be moved to designated initialization, but the C standard is pretty clear on how it works, and many callers of the macro will be using undesignated initialization for the final field anyway. That is, with the new macro, many initializations will effectively have a mixed form. If that really bothers you, then I guess you should drop the patch. I was just trying to silence a bunch of noise in a simple, direct way. Adding the missing field in all of those places would also silence the message, but it seemed reasonable to have require that. OK, I see why we need the zeros, that makes sense. I tried ixgbe, too, and it still doesn't seem to make a difference for me. But obviously it does for you, and it seems perfectly sensible, so I applied it to pci/misc for v3.16. Thanks! Bjorn ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-04-25 0:10 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-03-31 21:58 [PATCH] pci: Use designated initialization in PCI_VDEVICE Jeff Kirsher 2014-03-31 22:08 ` Sergei Shtylyov 2014-04-24 21:22 ` Bjorn Helgaas 2014-04-24 23:15 ` Rustad, Mark D 2014-04-24 23:43 ` Rustad, Mark D 2014-04-25 0:10 ` Bjorn Helgaas
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).