[dpdk-dev,RFC,01/18] librte_eal: remove type field from rte_driver structure.
diff mbox

Message ID 1441364514-24905-2-git-send-email-bernard.iremonger@intel.com
State Rejected, archived
Headers show

Commit Message

Bernard Iremonger Sept. 4, 2015, 11:01 a.m. UTC 
Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 lib/librte_eal/common/eal_common_dev.c  | 22 +++++++++++++---------
 lib/librte_eal/common/include/rte_dev.h | 11 +----------
 2 files changed, 14 insertions(+), 19 deletions(-)

Comments

Thomas Monjalon Sept. 4, 2015, 1:08 p.m. UTC | #1 
2015-09-04 12:01, Bernard Iremonger:
> Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>

There is no explanation in this patch.

> -		if (driver->type != PMD_PDEV)
> -			continue;
> -		/* PDEV drivers don't get passed any parameters */
> -		driver->init(NULL, NULL);
> +
> +		/* PCI drivers don't get passed any parameters */
> +		/*
> +		 * Search a virtual driver prefix in device name.
> +		 * It should not be found for PCI devices.
> +		 * Use strncmp to compare.
> +		 */
> +
> +		if ((driver->name) &&
> +			(strncmp(driver->name, "eth_", strlen("eth_")) != 0)) {
> +			driver->init(NULL, NULL);
> +		}

You don't need to submit a full patchset with changes in every drivers
for a RFC. Having just this patch is enough to have an opinion.
Here it is a nack.
We need to have a common init path instead of the current VDEV/PDEV branches.
And instead of "pmd_type", a bus information would be more meaningful.
So just replacing a type by a magical string is worst.

Please don't try to fix wrong problems and focus on your goal.
We had some discussions about possible PCI EAL refactoring but
it probably needs to be done step by step with a clear cleaning motivation
at each step. I think other people involved in EAL will have other ideas.

Patch
diff mbox 

diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
index 4089d66..ccfbb8c 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -1,7 +1,7 @@ 
 /*-
  *   BSD LICENSE
  *
- *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
+ *   Copyright(c) 2010-2015 Intel Corporation. All rights reserved.
  *   Copyright(c) 2014 6WIND S.A.
  *   All rights reserved.
  *
@@ -72,8 +72,6 @@  rte_eal_vdev_init(const char *name, const char *args)
 		return -EINVAL;
 
 	TAILQ_FOREACH(driver, &dev_driver_list, next) {
-		if (driver->type != PMD_VDEV)
-			continue;
 
 		/*
 		 * search a driver prefix in virtual device name.
@@ -117,10 +115,18 @@  rte_eal_dev_init(void)
 
 	/* Once the vdevs are initalized, start calling all the pdev drivers */
 	TAILQ_FOREACH(driver, &dev_driver_list, next) {
-		if (driver->type != PMD_PDEV)
-			continue;
-		/* PDEV drivers don't get passed any parameters */
-		driver->init(NULL, NULL);
+
+		/* PCI drivers don't get passed any parameters */
+		/*
+		 * Search a virtual driver prefix in device name.
+		 * It should not be found for PCI devices.
+		 * Use strncmp to compare.
+		 */
+
+		if ((driver->name) &&
+			(strncmp(driver->name, "eth_", strlen("eth_")) != 0)) {
+			driver->init(NULL, NULL);
+		}
 	}
 	return 0;
 }
@@ -134,8 +140,6 @@  rte_eal_vdev_uninit(const char *name)
 		return -EINVAL;
 
 	TAILQ_FOREACH(driver, &dev_driver_list, next) {
-		if (driver->type != PMD_VDEV)
-			continue;
 
 		/*
 		 * search a driver prefix in virtual device name.
diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
index f601d21..6253185 100644
--- a/lib/librte_eal/common/include/rte_dev.h
+++ b/lib/librte_eal/common/include/rte_dev.h
@@ -62,20 +62,11 @@  typedef int (rte_dev_init_t)(const char *name, const char *args);
 typedef int (rte_dev_uninit_t)(const char *name);
 
 /**
- * Driver type enumeration
- */
-enum pmd_type {
-	PMD_VDEV = 0,
-	PMD_PDEV = 1,
-};
-
-/**
  * A structure describing a device driver.
  */
 struct rte_driver {
 	TAILQ_ENTRY(rte_driver) next;  /**< Next in list. */
-	enum pmd_type type;		   /**< PMD Driver type */
-	const char *name;                   /**< Driver name. */
+	const char *name;                  /**< Driver name. */
 	rte_dev_init_t *init;              /**< Device init. function. */
 	rte_dev_uninit_t *uninit;          /**< Device uninit. function. */
 };