From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wi0-f176.google.com (mail-wi0-f176.google.com [209.85.212.176]) by dpdk.org (Postfix) with ESMTP id DC69DB47E for ; Wed, 18 Feb 2015 14:40:19 +0100 (CET) Received: by mail-wi0-f176.google.com with SMTP id h11so41034455wiw.3 for ; Wed, 18 Feb 2015 05:40:19 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:organization :user-agent:in-reply-to:references:mime-version :content-transfer-encoding:content-type; bh=7zKInGNIbNb7Tfcn2e4hNCZj9Tp3+3VTfhW7jGJ0gl8=; b=YKgniyALoNqR5bGXg5himwQJV5t0pY70oCM0c0dvD58XmeqnqfhFttYcyX9ha0wpgI rdkYgWWw7Q26RvpPEIFhcE/Q8rnFPhYfBp6y+B/kaQ7bm08jbsqk3VUADj1qXPv5fyWt or23jq6zcE5VBdwdLC+A/9pRpbLIKMJEw/blMkhG4foOqdOG5AfAOO4ivhJjYvdaw847 tLWQp4oRR6zCtui0zvSrVI+i5OWQ0XFyCPjeMPPX+w4OBa8BSBMmpuCu2IpBCCTY1ui2 np5Vty9c6IkMvaFyarmtnNWDLqYzuaSsVhs02NoMR59fHmkS4Mn+8j4YQl/4Y/rSuf8c Rmyg== X-Gm-Message-State: ALoCoQnDZmLvtPF03vVKQIz742Jb8oioPbACzP3zfs2AYYVwm6PR0J84T+YBANGWySIcBTM3pvhu X-Received: by 10.194.80.193 with SMTP id t1mr74612126wjx.8.1424266819667; Wed, 18 Feb 2015 05:40:19 -0800 (PST) Received: from xps13.localnet (6wind.net2.nerim.net. [213.41.180.237]) by mx.google.com with ESMTPSA id df8sm25528739wib.2.2015.02.18.05.40.18 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 18 Feb 2015 05:40:18 -0800 (PST) From: Thomas Monjalon To: Danny Zhou Date: Wed, 18 Feb 2015 14:39:46 +0100 Message-ID: <22608095.lA2lToQDvY@xps13> Organization: 6WIND User-Agent: KMail/4.14.4 (Linux/3.18.4-1-ARCH; KDE/4.14.4; x86_64; ; ) In-Reply-To: <1422523699-17181-2-git-send-email-danny.zhou@intel.com> References: <1422523699-17181-1-git-send-email-danny.zhou@intel.com> <1422523699-17181-2-git-send-email-danny.zhou@intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH v1 1/3] eal: enable uio_pci_generic support X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 18 Feb 2015 13:40:20 -0000 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 > Tested-by: Qun Wan [...] > --- 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 */ > };