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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20220517082558.59991355@kernel.org>
Date:   Tue, 17 May 2022 08:25:58 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Jonathan Lemon <jonathan.lemon@...il.com>
Cc:     netdev@...r.kernel.org, richardcochran@...il.com,
        davem@...emloft.net, pabeni@...hat.com, edumazet@...gle.com,
        kernel-team@...com
Subject: Re: [PATCH net-next v3 02/10] ptp: ocp: add Celestica timecard PCI
 ids

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 :)