From mboxrd@z Thu Jan  1 00:00:00 1970
From: David Marchand <david.marchand@6wind.com>
Subject: Re: [PATCH v3 01/13] e1000: move pci device ids to driver
Date: Wed, 20 Apr 2016 15:39:59 +0200
Message-ID: <CALwxeUtRfvOfZSrgSNHoD1apFiLb+gREyNsAoztstE4fZda-Fg@mail.gmail.com>
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=UTF-8
Cc: "dev@dpdk.org" <dev@dpdk.org>, Thomas Monjalon <thomas.monjalon@6wind.com>,
 Stephen Hemminger <stephen@networkplumber.org>,
 "Richardson, Bruce" <bruce.richardson@intel.com>,
 Panu Matilainen <pmatilai@redhat.com>,
 christian.ehrhardt@canonical.com, Wenzhuo Lu <wenzhuo.lu@intel.com>
To: Neil Horman <nhorman@tuxdriver.com>
Return-path: <dev-bounces@dpdk.org>
Received: from mail-wm0-f46.google.com (mail-wm0-f46.google.com [74.125.82.46])
 by dpdk.org (Postfix) with ESMTP id 211852BF9
 for <dev@dpdk.org>; Wed, 20 Apr 2016 15:40:19 +0200 (CEST)
Received: by mail-wm0-f46.google.com with SMTP id 127so11626957wmz.0
 for <dev@dpdk.org>; Wed, 20 Apr 2016 06:40:19 -0700 (PDT)
In-Reply-To: <20160420132908.GA21072@hmsreliant.think-freely.org>
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

On Wed, Apr 20, 2016 at 3:29 PM, Neil Horman <nhorman@tuxdriver.com> 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.

> 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.

>><snip>
>
>> +
>> +#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.


-- 
David Marchand