From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.tuxdriver.com (charlotte.tuxdriver.com [70.61.120.58]) by dpdk.org (Postfix) with ESMTP id 275A5A6A for ; Wed, 20 Apr 2016 20:15:25 +0200 (CEST) Received: from [107.15.76.160] (helo=localhost) by smtp.tuxdriver.com with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.63) (envelope-from ) id 1aswef-0003TA-0P; Wed, 20 Apr 2016 14:15:19 -0400 Date: Wed, 20 Apr 2016 14:15:11 -0400 From: Neil Horman To: David Marchand Cc: "dev@dpdk.org" , Thomas Monjalon , Stephen Hemminger , "Richardson, Bruce" , Panu Matilainen , christian.ehrhardt@canonical.com, Wenzhuo Lu Message-ID: <20160420181511.GB21072@hmsreliant.think-freely.org> References: <1453120248-28274-1-git-send-email-david.marchand@6wind.com> <1461156236-25349-1-git-send-email-david.marchand@6wind.com> <1461156236-25349-2-git-send-email-david.marchand@6wind.com> <20160420132908.GA21072@hmsreliant.think-freely.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) X-Spam-Score: -1.0 (-) X-Spam-Status: No Subject: Re: [dpdk-dev] [PATCH v3 01/13] e1000: move pci device ids to driver X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 20 Apr 2016 18:15:25 -0000 On Wed, Apr 20, 2016 at 03:39:59PM +0200, David Marchand wrote: > On Wed, Apr 20, 2016 at 3:29 PM, Neil Horman wrote: > >> +#ifndef RTE_PCI_DEV_ID_DECL_EM > >> +#define RTE_PCI_DEV_ID_DECL_EM(vend, dev) > >> +#endif > >> + > >> +#ifndef PCI_VENDOR_ID_INTEL > >> +/** Vendor ID used by Intel devices */ > >> +#define PCI_VENDOR_ID_INTEL 0x8086 > >> +#endif > >> + > > This is broken, PCI_VENDOR_ID_INTEL should be defined in a central location for > > all pci drivers, not redefined in every compilation unit. And you can likely > > Well we can keep the vendors in a common header, but I don't see the benefit. > ? The fact that you won't have to do this #ifndef PCI_VENDOR_ID_INTEL #define PCI_VENDOR_ID_INTEL ... #endif in every pmd > > just remvoe the RTE_PCI_DEV_ID_DECL_* macros from each patch and use the > > RTE_PCI_DEV macro, as all the DECL macros just evaluate to that anyway. > > app/test/test_pci.c:#define RTE_PCI_DEV_ID_DECL_IGB(vend, dev) > {RTE_PCI_DEVICE(vend, dev)}, > lib/librte_eal/linuxapp/kni/kni_misc.c: #define > RTE_PCI_DEV_ID_DECL_IGB(vend, dev) case (dev): > > All this stuff is because of pci tests and kni code. > You're going to have to explain a bit further than that. Why does the kni code and pci testing code require that each driver have their own macro that just maps to the same macro underneath? Looking at the test_pci code, it appears its done because we used to contain all our pci device ids in a single file, and the device specific macros were used to selectively enable devices that were there for testing. But the point of this series is in part to separate out the device ids to their own locations, so it seems like you will have to fix up the pci tests anyway (and additionally it would be nice to include more than just EM, IGB, and IXGBE in thsoe tests anyway, though I understand thats outside the scope of this series) > >> > > > >> + > >> +#undef RTE_PCI_DEV_ID_DECL_EM > > Why are you undefining this here? Is it because its in a header file and you're > > concerned about multiple inclusion? You should use an ifdef guard instead, or > > just define all the device id's in the header and ennumerate the pci table in > > one of the C files for the driver. That should apply to all the patches in this > > series I think > > Just moved the code. > Ok, do you plan on cleaning that up? Neil > > -- > David Marchand >