DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] i40e: prefer base driver naming
@ 2014-06-24  8:56 Thomas Monjalon
  2014-06-24 15:32 ` Zhang, Helin
  0 siblings, 1 reply; 3+ messages in thread
From: Thomas Monjalon @ 2014-06-24  8:56 UTC (permalink / raw)
  To: dev

The PMD is built on top of the base driver which is provided by Intel
and shouldn't be modified to allow easy batch upgrade from Intel.

The base driver is a "shared code" between many projects. But in DPDK,
the "base driver" naming makes more sense.

Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
---
 lib/librte_pmd_i40e/Makefile      | 33 ++++++++++++++++-----------------
 lib/librte_pmd_i40e/i40e_ethdev.c | 12 ++++++------
 2 files changed, 22 insertions(+), 23 deletions(-)

diff --git a/lib/librte_pmd_i40e/Makefile b/lib/librte_pmd_i40e/Makefile
index 09f2087..3bf2957 100644
--- a/lib/librte_pmd_i40e/Makefile
+++ b/lib/librte_pmd_i40e/Makefile
@@ -39,26 +39,25 @@ LIB = librte_pmd_i40e.a
 CFLAGS += -O3
 CFLAGS += $(WERROR_FLAGS)
 
-ifeq ($(CC), icc)
-CFLAGS_SHARED_DRIVERS = -wd593
-else
-CFLAGS_SHARED_DRIVERS =  -Wno-unused-but-set-variable
-CFLAGS_SHARED_DRIVERS += -Wno-sign-compare
-CFLAGS_SHARED_DRIVERS += -Wno-unused-value
-CFLAGS_SHARED_DRIVERS += -Wno-unused-parameter
-CFLAGS_SHARED_DRIVERS += -Wno-strict-aliasing
-CFLAGS_SHARED_DRIVERS += -Wno-format
-CFLAGS_SHARED_DRIVERS += -Wno-missing-field-initializers
-CFLAGS_SHARED_DRIVERS += -Wno-pointer-to-int-cast
-CFLAGS_SHARED_DRIVERS += -Wno-format-nonliteral
-CFLAGS_SHARED_DRIVERS += -Wno-format-security
-endif
-
 #
 # Add extra flags for ND source files to disable warnings
 #
-SHARED_DRIVERS_OBJS=$(patsubst %.c,%.o,$(notdir $(wildcard $(RTE_SDK)/lib/librte_pmd_i40e/i40e/*.c)))
-$(foreach obj, $(SHARED_DRIVERS_OBJS), $(eval CFLAGS_$(obj)+=$(CFLAGS_SHARED_DRIVERS)))
+ifeq ($(CC), icc)
+BASE_DRIVER_CFLAGS = -wd593
+else
+BASE_DRIVER_CFLAGS =  -Wno-unused-but-set-variable
+BASE_DRIVER_CFLAGS += -Wno-sign-compare
+BASE_DRIVER_CFLAGS += -Wno-unused-value
+BASE_DRIVER_CFLAGS += -Wno-unused-parameter
+BASE_DRIVER_CFLAGS += -Wno-strict-aliasing
+BASE_DRIVER_CFLAGS += -Wno-format
+BASE_DRIVER_CFLAGS += -Wno-missing-field-initializers
+BASE_DRIVER_CFLAGS += -Wno-pointer-to-int-cast
+BASE_DRIVER_CFLAGS += -Wno-format-nonliteral
+BASE_DRIVER_CFLAGS += -Wno-format-security
+endif
+BASE_DRIVER_OBJS=$(patsubst %.c,%.o,$(notdir $(wildcard $(RTE_SDK)/lib/librte_pmd_i40e/i40e/*.c)))
+$(foreach obj, $(BASE_DRIVER_OBJS), $(eval CFLAGS_$(obj)+=$(BASE_DRIVER_CFLAGS)))
 
 VPATH += $(RTE_SDK)/lib/librte_pmd_i40e/i40e
 
diff --git a/lib/librte_pmd_i40e/i40e_ethdev.c b/lib/librte_pmd_i40e/i40e_ethdev.c
index 1b4e822..9cad2d1 100644
--- a/lib/librte_pmd_i40e/i40e_ethdev.c
+++ b/lib/librte_pmd_i40e/i40e_ethdev.c
@@ -387,10 +387,10 @@ eth_i40e_dev_init(__rte_unused struct eth_driver *eth_drv,
 		return ret;
 	}
 
-	/* Initialize the shared code */
+	/* Initialize the base driver (shared code) */
 	ret = i40e_init_shared_code(hw);
 	if (ret) {
-		PMD_INIT_LOG(ERR, "Failed to init shared code: %d", ret);
+		PMD_INIT_LOG(ERR, "Failed to init base driver: %d", ret);
 		return ret;
 	}
 
@@ -1493,7 +1493,7 @@ i40e_dev_rss_reta_query(struct rte_eth_dev *dev,
 }
 
 /**
- * i40e_allocate_dma_mem_d - specific memory alloc for shared code
+ * i40e_allocate_dma_mem_d - specific memory alloc for base driver
  * @hw:   pointer to the HW structure
  * @mem:  pointer to mem struct to fill out
  * @size: size of memory requested
@@ -1527,7 +1527,7 @@ i40e_allocate_dma_mem_d(__attribute__((unused)) struct i40e_hw *hw,
 }
 
 /**
- * i40e_free_dma_mem_d - specific memory free for shared code
+ * i40e_free_dma_mem_d - specific memory free for base driver
  * @hw:   pointer to the HW structure
  * @mem:  ptr to mem struct to free
  **/
@@ -1545,7 +1545,7 @@ i40e_free_dma_mem_d(__attribute__((unused)) struct i40e_hw *hw,
 }
 
 /**
- * i40e_allocate_virt_mem_d - specific memory alloc for shared code
+ * i40e_allocate_virt_mem_d - specific memory alloc for base driver
  * @hw:   pointer to the HW structure
  * @mem:  pointer to mem struct to fill out
  * @size: size of memory requested
@@ -1568,7 +1568,7 @@ i40e_allocate_virt_mem_d(__attribute__((unused)) struct i40e_hw *hw,
 }
 
 /**
- * i40e_free_virt_mem_d - specific memory free for shared code
+ * i40e_free_virt_mem_d - specific memory free for base driver
  * @hw:   pointer to the HW structure
  * @mem:  pointer to mem struct to free
  **/
-- 
2.0.0

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [dpdk-dev] [PATCH] i40e: prefer base driver naming
  2014-06-24  8:56 [dpdk-dev] [PATCH] i40e: prefer base driver naming Thomas Monjalon
@ 2014-06-24 15:32 ` Zhang, Helin
  2014-06-24 21:43   ` Thomas Monjalon
  0 siblings, 1 reply; 3+ messages in thread
From: Zhang, Helin @ 2014-06-24 15:32 UTC (permalink / raw)
  To: Thomas Monjalon, dev



-----Original Message-----
From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] 
Sent: Tuesday, June 24, 2014 4:57 PM
To: dev@dpdk.org
Cc: Zhang, Helin
Subject: [PATCH] i40e: prefer base driver naming

The PMD is built on top of the base driver which is provided by Intel and shouldn't be modified to allow easy batch upgrade from Intel.

The base driver is a "shared code" between many projects. But in DPDK, the "base driver" naming makes more sense.

Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
---
 lib/librte_pmd_i40e/Makefile      | 33 ++++++++++++++++-----------------
 lib/librte_pmd_i40e/i40e_ethdev.c | 12 ++++++------
 2 files changed, 22 insertions(+), 23 deletions(-)

diff --git a/lib/librte_pmd_i40e/Makefile b/lib/librte_pmd_i40e/Makefile index 09f2087..3bf2957 100644
--- a/lib/librte_pmd_i40e/Makefile
+++ b/lib/librte_pmd_i40e/Makefile
@@ -39,26 +39,25 @@ LIB = librte_pmd_i40e.a  CFLAGS += -O3  CFLAGS += $(WERROR_FLAGS)
 
-ifeq ($(CC), icc)
-CFLAGS_SHARED_DRIVERS = -wd593
-else
-CFLAGS_SHARED_DRIVERS =  -Wno-unused-but-set-variable -CFLAGS_SHARED_DRIVERS += -Wno-sign-compare -CFLAGS_SHARED_DRIVERS += -Wno-unused-value -CFLAGS_SHARED_DRIVERS += -Wno-unused-parameter -CFLAGS_SHARED_DRIVERS += -Wno-strict-aliasing -CFLAGS_SHARED_DRIVERS += -Wno-format -CFLAGS_SHARED_DRIVERS += -Wno-missing-field-initializers -CFLAGS_SHARED_DRIVERS += -Wno-pointer-to-int-cast -CFLAGS_SHARED_DRIVERS += -Wno-format-nonliteral -CFLAGS_SHARED_DRIVERS += -Wno-format-security -endif
-
 #
 # Add extra flags for ND source files to disable warnings  # -SHARED_DRIVERS_OBJS=$(patsubst %.c,%.o,$(notdir $(wildcard $(RTE_SDK)/lib/librte_pmd_i40e/i40e/*.c)))
-$(foreach obj, $(SHARED_DRIVERS_OBJS), $(eval CFLAGS_$(obj)+=$(CFLAGS_SHARED_DRIVERS)))
+ifeq ($(CC), icc)
+BASE_DRIVER_CFLAGS = -wd593
+else
+BASE_DRIVER_CFLAGS =  -Wno-unused-but-set-variable BASE_DRIVER_CFLAGS 
++= -Wno-sign-compare BASE_DRIVER_CFLAGS += -Wno-unused-value 
+BASE_DRIVER_CFLAGS += -Wno-unused-parameter BASE_DRIVER_CFLAGS += 
+-Wno-strict-aliasing BASE_DRIVER_CFLAGS += -Wno-format 
+BASE_DRIVER_CFLAGS += -Wno-missing-field-initializers 
+BASE_DRIVER_CFLAGS += -Wno-pointer-to-int-cast BASE_DRIVER_CFLAGS += 
+-Wno-format-nonliteral BASE_DRIVER_CFLAGS += -Wno-format-security endif 
+BASE_DRIVER_OBJS=$(patsubst %.c,%.o,$(notdir $(wildcard 
+$(RTE_SDK)/lib/librte_pmd_i40e/i40e/*.c)))
+$(foreach obj, $(BASE_DRIVER_OBJS), $(eval 
+CFLAGS_$(obj)+=$(BASE_DRIVER_CFLAGS)))
 
 VPATH += $(RTE_SDK)/lib/librte_pmd_i40e/i40e
 
diff --git a/lib/librte_pmd_i40e/i40e_ethdev.c b/lib/librte_pmd_i40e/i40e_ethdev.c
index 1b4e822..9cad2d1 100644
--- a/lib/librte_pmd_i40e/i40e_ethdev.c
+++ b/lib/librte_pmd_i40e/i40e_ethdev.c
@@ -387,10 +387,10 @@ eth_i40e_dev_init(__rte_unused struct eth_driver *eth_drv,
 		return ret;
 	}
 
-	/* Initialize the shared code */
+	/* Initialize the base driver (shared code) */
 	ret = i40e_init_shared_code(hw);
 	if (ret) {
-		PMD_INIT_LOG(ERR, "Failed to init shared code: %d", ret);
+		PMD_INIT_LOG(ERR, "Failed to init base driver: %d", ret);
 		return ret;
 	}
 
@@ -1493,7 +1493,7 @@ i40e_dev_rss_reta_query(struct rte_eth_dev *dev,  }
 
 /**
- * i40e_allocate_dma_mem_d - specific memory alloc for shared code
+ * i40e_allocate_dma_mem_d - specific memory alloc for base driver
  * @hw:   pointer to the HW structure
  * @mem:  pointer to mem struct to fill out
  * @size: size of memory requested
@@ -1527,7 +1527,7 @@ i40e_allocate_dma_mem_d(__attribute__((unused)) struct i40e_hw *hw,  }
 
 /**
- * i40e_free_dma_mem_d - specific memory free for shared code
+ * i40e_free_dma_mem_d - specific memory free for base driver
  * @hw:   pointer to the HW structure
  * @mem:  ptr to mem struct to free
  **/
@@ -1545,7 +1545,7 @@ i40e_free_dma_mem_d(__attribute__((unused)) struct i40e_hw *hw,  }
 
 /**
- * i40e_allocate_virt_mem_d - specific memory alloc for shared code
+ * i40e_allocate_virt_mem_d - specific memory alloc for base driver
  * @hw:   pointer to the HW structure
  * @mem:  pointer to mem struct to fill out
  * @size: size of memory requested
@@ -1568,7 +1568,7 @@ i40e_allocate_virt_mem_d(__attribute__((unused)) struct i40e_hw *hw,  }
 
 /**
- * i40e_free_virt_mem_d - specific memory free for shared code
+ * i40e_free_virt_mem_d - specific memory free for base driver
  * @hw:   pointer to the HW structure
  * @mem:  pointer to mem struct to free
  **/
--
2.0.0

--------------------------------------------------------------------------------------------------------------------------------------------------------------

Hi Thomas

I prefer to keep the name of "shared code", and do not use "base driver". As "base driver" is used to indicate the top level of Linux/Windows/FreeBSD driver by our Network guys who provides us the shared code.
If we do this change, that will get us confused during following development.

BTW, the names of "shared code" and "base driver" have been used for specific code part for a long time by our Network guys.

Regards,
Helin

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [dpdk-dev] [PATCH] i40e: prefer base driver naming
  2014-06-24 15:32 ` Zhang, Helin
@ 2014-06-24 21:43   ` Thomas Monjalon
  0 siblings, 0 replies; 3+ messages in thread
From: Thomas Monjalon @ 2014-06-24 21:43 UTC (permalink / raw)
  To: Zhang, Helin; +Cc: dev

2014-06-24 15:32, Zhang, Helin:
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> 
> > The PMD is built on top of the base driver which is provided by Intel and
> > shouldn't be modified to allow easy batch upgrade from Intel.
> > 
> > The base driver is a "shared code" between many projects. But in DPDK, the
> > "base driver" naming makes more sense.
> 
> I prefer to keep the name of "shared code", and do not use "base driver". As
> "base driver" is used to indicate the top level of Linux/Windows/FreeBSD
> driver by our Network guys who provides us the shared code. If we do this
> change, that will get us confused during following development.

You are speaking about a code that we don't know and that doesn't matter in 
DPDK project.
You say using "base driver" is confusing you but it's what it is: a base 
driver for PMD one.
Furthermore, the name "shared code" is really confusing as it is shared with 
nothing else in DPDK. And it can be misunderstood as "code for shared 
libraries".

> BTW, the names of "shared code" and "base driver" have been used for
> specific code part for a long time by our Network guys.

Wrong argument. We don't care about your Network guys here. We care about how 
contributors will dive into your code.

I can be wrong so I'd like to have opinions from other people (preferably 
outside of Intel ;).

Thanks
-- 
Thomas

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2014-06-24 21:43 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-24  8:56 [dpdk-dev] [PATCH] i40e: prefer base driver naming Thomas Monjalon
2014-06-24 15:32 ` Zhang, Helin
2014-06-24 21:43   ` Thomas Monjalon

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).