Re: [dpdk-dev] [PATCHv4 2/5] drivers: Update driver registration macro usage

[Thomas Monjalon <thomas.monjalon@6wind.com>]

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.