[dpdk-dev] [dpdk-dev, PATCHv2, 2/4] drivers: Update driver registration macro usage

[Jan Viktorin]

Hello Neil,

just few notes...

(sorry if you've recevied this twice, importing mbox files from patchwork
always changes my default From: field)

On Wed, 18 May 2016 17:08:05 -0400
Neil Horman <nhorman at tuxdriver.com> wrote:

> Modify the PMD_REGISTER_DRIVER macro, bifurcating it into two
> (PMD_REGISTER_DRIVER_PDEV and PMD_REGISTER_DRIVER_VDEV.  Both of these do the
> same thing the origional macro did, but both add the definition of a string

I could not find any of those: PMD_REGISTER_DRIVER_PDEV, PMD_REGISTER_DRIVER_VDEV
in this patch. I think the message is misleading...

I am interested as this may lead to merge conflicts when generalizing
rte_pci_device/driver and stuff around it.

> variable that informs interested parties of the name of the pmd, and the former
> also defines an second string that holds the symbol name of the pci table that
> is registered by this pmd.

> 
> pmdinfo uses this information to extract hardware support from an object file
> and create a json string to make hardware support info discoverable later.
> 
> Signed-off-by: Neil Horman <nhorman at tuxdriver.com>
> CC: Bruce Richardson <bruce.richardson at intel.com>
> CC: Thomas Monjalon <thomas.monjalon at 6wind.com>
> CC: Stephen Hemminger <stephen at networkplumber.org>
> CC: Panu Matilainen <pmatilai at redhat.com>
> 
> ---
> drivers/Makefile                           |  2 ++
>  drivers/crypto/aesni_gcm/aesni_gcm_pmd.c   |  4 +++-
>  drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c |  4 +++-
>  drivers/crypto/null/null_crypto_pmd.c      |  4 +++-
>  drivers/crypto/qat/rte_qat_cryptodev.c     |  4 +++-
>  drivers/crypto/snow3g/rte_snow3g_pmd.c     |  4 +++-
>  drivers/net/af_packet/rte_eth_af_packet.c  |  4 +++-
>  drivers/net/bnx2x/bnx2x_ethdev.c           |  6 ++++--
>  drivers/net/bonding/rte_eth_bond_pmd.c     |  7 ++++++-
>  drivers/net/cxgbe/cxgbe_ethdev.c           |  4 +++-
>  drivers/net/e1000/em_ethdev.c              |  3 ++-
>  drivers/net/e1000/igb_ethdev.c             |  6 ++++--
>  drivers/net/ena/ena_ethdev.c               |  3 ++-
>  drivers/net/enic/enic_ethdev.c             |  3 ++-
>  drivers/net/fm10k/fm10k_ethdev.c           |  3 ++-
>  drivers/net/i40e/i40e_ethdev.c             |  3 ++-
>  drivers/net/i40e/i40e_ethdev_vf.c          |  3 ++-
>  drivers/net/ixgbe/ixgbe_ethdev.c           |  6 ++++--
>  drivers/net/mlx4/mlx4.c                    |  3 ++-
>  drivers/net/mlx5/mlx5.c                    |  3 ++-
>  drivers/net/mpipe/mpipe_tilegx.c           |  4 ++--
>  drivers/net/nfp/nfp_net.c                  |  3 ++-
>  drivers/net/null/rte_eth_null.c            |  3 ++-
>  drivers/net/pcap/rte_eth_pcap.c            |  4 +++-
>  drivers/net/ring/rte_eth_ring.c            |  3 ++-
>  drivers/net/szedata2/rte_eth_szedata2.c    |  3 ++-
>  drivers/net/vhost/rte_eth_vhost.c          |  3 ++-
>  drivers/net/virtio/virtio_ethdev.c         |  3 ++-
>  drivers/net/vmxnet3/vmxnet3_ethdev.c       |  3 ++-
>  drivers/net/xenvirt/rte_eth_xenvirt.c      |  2 +-
>  lib/librte_eal/common/include/rte_dev.h    | 20 ++++++++++++++++----
>  31 files changed, 93 insertions(+), 37 deletions(-)
> 

[...]

>  
> -PMD_REGISTER_DRIVER(pmd_xenvirt_drv);
> +PMD_REGISTER_DRIVER(pmd_xenvirt_drv, xenvirt);
> diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
> index f1b5507..871089a 100644
> --- 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>

This should be done in an opposite way. The rte_pci.h should include rte_dev.h
(more specific includes a generic one). I've introduced this in the following patch:

 [PATCH v1 01/28] eal: make enum rte_kernel_driver non-PCI specific
 http://dpdk.org/dev/patchwork/patch/12488/

Regards
Jan

>  #include <rte_log.h>
>  
>  __attribute__((format(printf, 2, 0)))
> @@ -178,12 +178,24 @@ int rte_eal_vdev_init(const char *name, const char *args);
>   */
>  int rte_eal_vdev_uninit(const char *name);
>  
> -#define PMD_REGISTER_DRIVER(d)\
> +#define DRIVER_EXPORT_NAME_ARRAY(n, idx) n##idx[] __attribute__((used))
> +
> +#define DRIVER_EXPORT_NAME(d, idx) \
> +static const char DRIVER_EXPORT_NAME_ARRAY(this_pmd_name, idx) = RTE_STR(d);\
> + 
> +#define PMD_REGISTER_DRIVER(d, n)\
>  void devinitfn_ ##d(void);\
>  void __attribute__((constructor, used)) devinitfn_ ##d(void)\
>  {\
> -	rte_eal_driver_register(&d);\
> -}
> +        rte_eal_driver_register(&d);\
> +}\
> +DRIVER_EXPORT_NAME(n, __COUNTER__)
> +
> +#define DRIVER_REGISTER_PCI_TABLE(n, t) \
> +static const char n##_pci_tbl_export[] __attribute__((used)) = RTE_STR(t)
> +
> +#define DRIVER_REGISTER_PARAM_STRING(n, s) \
> +static const char n##_param_string_export[] __attribute__((used)) = s
>  
>  #ifdef __cplusplus
>  }

-- 
   Jan Viktorin                  E-mail: Viktorin at RehiveTech.com
   System Architect              Web:    www.RehiveTech.com
   RehiveTech
   Brno, Czech Republic