From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Viktorin Subject: Re: [PATCH v6 12/17] pci: add a helper for device name Date: Thu, 14 Jul 2016 18:55:46 +0200 Message-ID: <20160714185546.6e483b40@jvn> References: <1466510566-9240-1-git-send-email-shreyansh.jain@nxp.com> <1468303282-2806-1-git-send-email-shreyansh.jain@nxp.com> <1468303282-2806-13-git-send-email-shreyansh.jain@nxp.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: , , To: Shreyansh Jain Return-path: Received: from wes1-so2.wedos.net (wes1-so2.wedos.net [46.28.106.16]) by dpdk.org (Postfix) with ESMTP id 724753237 for ; Thu, 14 Jul 2016 18:55:40 +0200 (CEST) In-Reply-To: <1468303282-2806-13-git-send-email-shreyansh.jain@nxp.com> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Tue, 12 Jul 2016 11:31:17 +0530 Shreyansh Jain wrote: > eal is a better place than crypto / ethdev for naming resources. s/for naming/to name/ What is meant by "resources" here? > Add a helper in eal and make use of it in crypto / ethdev. > > Signed-off-by: David Marchand > Signed-off-by: Shreyansh Jain > --- > lib/librte_cryptodev/rte_cryptodev.c | 27 ++++----------------------- > lib/librte_eal/common/include/rte_pci.h | 25 +++++++++++++++++++++++++ > lib/librte_ether/rte_ethdev.c | 24 ++++-------------------- > 3 files changed, 33 insertions(+), 43 deletions(-) > > diff --git a/lib/librte_cryptodev/rte_cryptodev.c b/lib/librte_cryptodev/rte_cryptodev.c > index d7be111..60c6384 100644 > --- a/lib/librte_cryptodev/rte_cryptodev.c > +++ b/lib/librte_cryptodev/rte_cryptodev.c > @@ -367,23 +367,6 @@ rte_cryptodev_pmd_allocate(const char *name, int socket_id) > return cryptodev; > } > > -static inline int > -rte_cryptodev_create_unique_device_name(char *name, size_t size, > - struct rte_pci_device *pci_dev) > -{ > - int ret; > - > - if ((name == NULL) || (pci_dev == NULL)) > - return -EINVAL; > - > - ret = snprintf(name, size, "%d:%d.%d", > - pci_dev->addr.bus, pci_dev->addr.devid, > - pci_dev->addr.function); > - if (ret < 0) > - return ret; > - return 0; > -} > - > int > rte_cryptodev_pmd_release_device(struct rte_cryptodev *cryptodev) > { > @@ -446,9 +429,8 @@ rte_cryptodev_pci_probe(struct rte_pci_driver *pci_drv, > if (cryptodrv == NULL) > return -ENODEV; > > - /* Create unique Crypto device name using PCI address */ > - rte_cryptodev_create_unique_device_name(cryptodev_name, > - sizeof(cryptodev_name), pci_dev); > + rte_eal_pci_device_name(&pci_dev->addr, cryptodev_name, > + sizeof(cryptodev_name)); > > cryptodev = rte_cryptodev_pmd_allocate(cryptodev_name, rte_socket_id()); > if (cryptodev == NULL) > @@ -503,9 +485,8 @@ rte_cryptodev_pci_remove(struct rte_pci_device *pci_dev) > if (pci_dev == NULL) > return -EINVAL; > > - /* Create unique device name using PCI address */ > - rte_cryptodev_create_unique_device_name(cryptodev_name, > - sizeof(cryptodev_name), pci_dev); > + rte_eal_pci_device_name(&pci_dev->addr, cryptodev_name, > + sizeof(cryptodev_name)); > > cryptodev = rte_cryptodev_pmd_get_named_dev(cryptodev_name); > if (cryptodev == NULL) > diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h > index 3027adf..06508fa 100644 > --- a/lib/librte_eal/common/include/rte_pci.h > +++ b/lib/librte_eal/common/include/rte_pci.h > @@ -82,6 +82,7 @@ extern "C" { > #include > #include > > +#include > #include > > TAILQ_HEAD(pci_device_list, rte_pci_device); /**< PCI devices in D-linked Q. */ > @@ -95,6 +96,7 @@ const char *pci_get_sysfs_path(void); > > /** Formatting string for PCI device identifier: Ex: 0000:00:01.0 */ > #define PCI_PRI_FMT "%.4" PRIx16 ":%.2" PRIx8 ":%.2" PRIx8 ".%" PRIx8 > +#define PCI_PRI_STR_SIZE sizeof("XXXX:XX:XX.X") > > /** Short formatting string, without domain, for PCI device: Ex: 00:01.0 */ > #define PCI_SHORT_PRI_FMT "%.2" PRIx8 ":%.2" PRIx8 ".%" PRIx8 > @@ -308,6 +310,29 @@ eal_parse_pci_DomBDF(const char *input, struct rte_pci_addr *dev_addr) > } > #undef GET_PCIADDR_FIELD > > +/** > + * Utility function to write a pci device name, this device name can later be > + * used to retrieve the corresponding rte_pci_addr using above functions. What about saying "using functions eal_parse_pci_*BDF"? The specification "above" is quite uncertain... > + * > + * @param addr > + * The PCI Bus-Device-Function address > + * @param output > + * The output buffer string > + * @param size > + * The output buffer size > + * @return > + * 0 on success, negative on error. > + */ > +static inline void > +rte_eal_pci_device_name(const struct rte_pci_addr *addr, > + char *output, size_t size) > +{ > + RTE_VERIFY(size >= PCI_PRI_STR_SIZE); > + RTE_VERIFY(snprintf(output, size, PCI_PRI_FMT, > + addr->domain, addr->bus, > + addr->devid, addr->function) >= 0); > +} > + > /* Compare two PCI device addresses. */ > /** [...]