From mboxrd@z Thu Jan  1 00:00:00 1970
From: Thomas Monjalon <thomas.monjalon@6wind.com>
Subject: Re: [PATCHv4 2/5] drivers: Update driver registration
	macro usage
Date: Wed, 25 May 2016 18:20:06 +0200
Message-ID: <19273886.LC4JPNcjGO@xps13>
References: <1463431287-4551-1-git-send-email-nhorman@tuxdriver.com>
 <1464118912-19658-1-git-send-email-nhorman@tuxdriver.com>
 <1464118912-19658-3-git-send-email-nhorman@tuxdriver.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7Bit
Cc: dev@dpdk.org, Bruce Richardson <bruce.richardson@intel.com>,
 Stephen Hemminger <stephen@networkplumber.org>,
 Panu Matilainen <pmatilai@redhat.com>
To: Neil Horman <nhorman@tuxdriver.com>
Return-path: <dev-bounces@dpdk.org>
Received: from mail-wm0-f47.google.com (mail-wm0-f47.google.com [74.125.82.47])
 by dpdk.org (Postfix) with ESMTP id 446E12E81
 for <dev@dpdk.org>; Wed, 25 May 2016 18:20:09 +0200 (CEST)
Received: by mail-wm0-f47.google.com with SMTP id n129so190755306wmn.1
 for <dev@dpdk.org>; Wed, 25 May 2016 09:20:09 -0700 (PDT)
In-Reply-To: <1464118912-19658-3-git-send-email-nhorman@tuxdriver.com>
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>

2016-05-24 15:41, Neil Horman:
> Modify the PMD_REGISTER_DRIVER macro, adding a name argument to it.  The
> addition of a name argument creates a token that can be used for subsequent
> macros in the creation of unique symbol names to export additional bits of
> information for use by the pmdinfogen tool.  For example:
> 
> PMD_REGISTER_DRIVER(ena_driver, ena);
> 
> registers the ena_driver struct as it always did, and creates a symbol
> const char this_pmd_name0[] __attribute__((used)) = "ena";
> 
> which pmdinfogen can search for and extract.

The EAL rework (http://dpdk.org/ml/archives/dev/2016-April/037691.html)
integrates already a name:

+#define RTE_EAL_PCI_REGISTER(name, d) \
+RTE_INIT(pciinitfn_ ##name); \
+static void pciinitfn_ ##name(void) \
+{ \
+	rte_eal_pci_register(&d); \
+}

I think it would be better to rebase on top of it.

> The subsequent macro
> 
> DRIVER_REGISTER_PCI_TABLE(ena, ena_pci_id_map);
> 
> creates a symbol
> const char ena_pci_tbl_export[] __attribute__((used)) = "ena_pci_id_map";
> 
> Which allows pmdinfogen to find the pci table of this driver
> 
> Using this pattern, we can export arbitrary bits of information.
> 
> pmdinfo uses this information to extract hardware support from an object file
> and create a json string to make hardware support info discoverable later.

> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -34,4 +34,6 @@ include $(RTE_SDK)/mk/rte.vars.mk
>  DIRS-y += net
>  DIRS-$(CONFIG_RTE_LIBRTE_CRYPTODEV) += crypto
>  
> +DEPDIRS-y += buildtools/pmdinfo

Why pmdinfo is a build dependency?

> --- a/lib/librte_eal/common/include/rte_dev.h
> +++ b/lib/librte_eal/common/include/rte_dev.h
> @@ -48,7 +48,7 @@ extern "C" {
>  
>  #include <stdio.h>
>  #include <sys/queue.h>
> -
> +#include <rte_pci.h>

Why not keep PCI stuff in rte_pci.h?

> +#define DRV_EXP_TAG(n, t) __##n##_##t
> +
> +#define DRIVER_REGISTER_PCI_TABLE(n, t) \
> +static const char DRV_EXP_TAG(n, pci_tbl_export)[] __attribute__((used)) = RTE_STR(t)

I really dislike one-char variables, especially when there is no comment.
Please choose comments or explicit variables.