Re: [PATCH net-next v3 02/10] ptp: ocp: add Celestica timecard PCI ids

[Jakub Kicinski <kuba@kernel.org>]

On Mon, 16 May 2022 18:46:44 -0700 Jonathan Lemon wrote:
> On Mon, May 16, 2022 at 05:43:03PM -0700, Jakub Kicinski wrote:
> > On Fri, 13 May 2022 15:59:16 -0700 Jonathan Lemon wrote:  
> > > +#ifndef PCI_VENDOR_ID_CELESTICA
> > > +#define PCI_VENDOR_ID_CELESTICA 0x18d4
> > > +#endif
> > > +
> > > +#ifndef PCI_DEVICE_ID_CELESTICA_TIMECARD
> > > +#define PCI_DEVICE_ID_CELESTICA_TIMECARD 0x1008
> > > +#endif  
> > 
> > The ifdefs are unnecessary, these kind of constructs are often used out
> > of tree when one does not control the headers, but not sure what purpose
> > they'd serve upstream?  
> 
> include/linux/pci_ids.h says:
> 
>  *      Do not add new entries to this file unless the definitions
>  *      are shared between multiple drivers.
> 
> Neither FACEBOOK (0x1d9b) nor CELESTICA (0x18d4) are present
> in this file.  This seems to a common idiom in several other
> drivers.  Picking one at random:
> 
>    gve.h:#define PCI_VENDOR_ID_GOOGLE 0x1ae0
> 
> 
> So these #defines are needed.

Indeed, but also I'm not complaining about defines but the ifdefs 
in which they are wrapped :)