DPDK patches and discussions
 help / color / mirror / Atom feed
From: Thomas Monjalon <thomas.monjalon@6wind.com>
To: Danny Zhou <danny.zhou@intel.com>
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v1 1/3] eal: enable uio_pci_generic support
Date: Wed, 18 Feb 2015 14:39:46 +0100	[thread overview]
Message-ID: <22608095.lA2lToQDvY@xps13> (raw)
In-Reply-To: <1422523699-17181-2-git-send-email-danny.zhou@intel.com>

Hi Danny,

I wanted to apply this patchset which was reviewed. But when having a quick
overview, I've seen some strange additions.

2015-01-29 17:28, Danny Zhou:
> 1) Unify procedure to retrieve BAR resource mapping information.  
> 2) Setup bus master bit in NIC's PCIe configuration space for uio_pci_generic.
> 
> Signed-off-by: Danny Zhou <danny.zhou@intel.com>
> Tested-by: Qun Wan <qun.wan@intel.com>
[...]
> --- a/lib/librte_eal/common/include/rte_pci.h
> +++ b/lib/librte_eal/common/include/rte_pci.h
> @@ -148,6 +148,7 @@ struct rte_pci_device {
>  	struct rte_pci_id id;                   /**< PCI ID. */
>  	struct rte_pci_resource mem_resource[PCI_MAX_RESOURCE];   /**< PCI Memory Resource */
>  	struct rte_intr_handle intr_handle;     /**< Interrupt handle */
> +	char driver_name[BUFSIZ];               /**< driver name */

Why not embedding this field in driver struct?
The name and comment should be more precise.
There is also driver->name and hotplug patchset is adding a kernel driver name.
Please bring the light in all these driver names :)

>  	const struct rte_pci_driver *driver;    /**< Associated driver */
[...]
> --- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> +#define IORESOURCE_MEM        0x00000200

Please comment this value.

> --- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h
> +++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h
> @@ -50,8 +50,14 @@ enum rte_intr_handle_type {
>  
>  /** Handle for interrupts. */
>  struct rte_intr_handle {
> -	int vfio_dev_fd;                 /**< VFIO device file descriptor */
> -	int fd;                          /**< file descriptor */
> +	union {
> +		int vfio_dev_fd;  /**< VFIO device file descriptor */
> +	};
> +        union {
> +		int uio_cfg_fd;  /**< UIO config file descriptor
> +					for uio_pci_generic */
> +	};

Apart the indent, it seems there is a mistake here.
Why 2 unions with 1 field each?

> +	int fd;	 /**< interrupt event file descriptor */
>  	enum rte_intr_handle_type type;  /**< handle type */
>  };

  reply	other threads:[~2015-02-18 13:40 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-29  9:28 [dpdk-dev] [PATCH 0/3] Enable " Danny Zhou
2015-01-29  9:28 ` [dpdk-dev] [PATCH v1 1/3] eal: enable " Danny Zhou
2015-02-18 13:39   ` Thomas Monjalon [this message]
2015-02-19 15:48     ` Zhou, Danny
2015-01-29  9:28 ` [dpdk-dev] [PATCH v1 2/3] eal: add interrupt enable/disable routines for uio_pci_generic Danny Zhou
2015-01-29  9:28 ` [dpdk-dev] [PATCH v1 3/3] tools: enable binding NIC device to uio_pci_generic Danny Zhou
2015-01-29  9:34 ` [dpdk-dev] [PATCH 0/3] Enable uio_pci_generic support Qiu, Michael
2015-02-09 14:30 ` Bruce Richardson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=22608095.lA2lToQDvY@xps13 \
    --to=thomas.monjalon@6wind.com \
    --cc=danny.zhou@intel.com \
    --cc=dev@dpdk.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).