DPDK patches and discussions
 help / color / mirror / Atom feed
From: Stephen Hemminger <stephen@networkplumber.org>
To: Harish Patil <harish.patil@qlogic.com>
Cc: dev@dpdk.org, Sony Chacko <sony.chacko@qlogic.com>
Subject: Re: [dpdk-dev] [PATCH 5/6] qede: add driver
Date: Sat, 20 Feb 2016 17:26:23 -0800	[thread overview]
Message-ID: <20160220172623.5101c7e4@xeon-e3> (raw)
In-Reply-To: <1455982831-21682-6-git-send-email-harish.patil@qlogic.com>

On Sat, 20 Feb 2016 07:40:30 -0800
Harish Patil <harish.patil@qlogic.com> wrote:

> Signed-off-by: Harish Patil <harish.patil@qlogic.com>
> Signed-off-by: Rasesh Mody <rasesh.mody@qlogic.com>
> Signed-off-by: Sony Chacko <sony.chacko@qlogic.com>
> ---
>  drivers/net/qede/LICENSE.qede_pmd         |   28 +
>  drivers/net/qede/Makefile                 |   95 ++
>  drivers/net/qede/qede_eth_if.c            |  461 ++++++++++
>  drivers/net/qede/qede_eth_if.h            |  180 ++++
>  drivers/net/qede/qede_ethdev.c            |  815 +++++++++++++++++
>  drivers/net/qede/qede_ethdev.h            |  138 +++
>  drivers/net/qede/qede_if.h                |  167 ++++
>  drivers/net/qede/qede_logs.h              |   93 ++
>  drivers/net/qede/qede_main.c              |  587 +++++++++++++
>  drivers/net/qede/qede_rxtx.c              | 1346 +++++++++++++++++++++++++++++
>  drivers/net/qede/qede_rxtx.h              |  183 ++++
>  drivers/net/qede/rte_pmd_qede_version.map |    4 +
>  12 files changed, 4097 insertions(+)
>  create mode 100644 drivers/net/qede/LICENSE.qede_pmd
>  create mode 100644 drivers/net/qede/Makefile
>  create mode 100644 drivers/net/qede/qede_eth_if.c
>  create mode 100644 drivers/net/qede/qede_eth_if.h
>  create mode 100644 drivers/net/qede/qede_ethdev.c
>  create mode 100644 drivers/net/qede/qede_ethdev.h
>  create mode 100644 drivers/net/qede/qede_if.h
>  create mode 100644 drivers/net/qede/qede_logs.h
>  create mode 100644 drivers/net/qede/qede_main.c
>  create mode 100644 drivers/net/qede/qede_rxtx.c
>  create mode 100644 drivers/net/qede/qede_rxtx.h
>  create mode 100644 drivers/net/qede/rte_pmd_qede_version.map

DPDK doesn't follow all the kernel style rules, but lots of them.
The biggest catch was the ; in the LOG macros.
Minor stuff about spacing around casts etc.


Running checkpatch with --ignore  PREFER_KERNEL_TYPES,LINE_SPACING,PARENTHESIS_ALIGNMENT,BIT_MACRO,NETWORKING_BLOCK_COMMENT_STYLE,SPLIT_STRING,COMPARISON_TO_NULL,CONCATENATED_STRING 

Produces:


CHECK:SPACING: spaces preferred around that '*' (ctx:VxO)
#410: FILE: drivers/net/qede/qede_eth_if.c:175:
+	      uint16_t cqe_pbl_size, void OSAL_IOMEM**pp_prod)
 	                                            ^

CHECK:SPACING: spaces preferred around that '*' (ctx:VxO)
#467: FILE: drivers/net/qede/qede_eth_if.c:232:
+	      uint16_t pbl_size, void OSAL_IOMEM**pp_doorbell)
 	                                        ^

CHECK:BRACES: Blank lines aren't necessary after an open brace '{'
#519: FILE: drivers/net/qede/qede_eth_if.c:284:
+{
+

WARNING:SPACING: Unnecessary space before function pointer arguments
#823: FILE: drivers/net/qede/qede_eth_if.h:121:
+	int (*fill_dev_info) (struct ecore_dev *edev,

WARNING:SPACING: Unnecessary space before function pointer arguments
#826: FILE: drivers/net/qede/qede_eth_if.h:124:
+	int (*vport_start) (struct ecore_dev *edev,

WARNING:SPACING: Unnecessary space before function pointer arguments
#829: FILE: drivers/net/qede/qede_eth_if.h:127:
+	int (*vport_stop) (struct ecore_dev *edev, uint8_t vport_id);

WARNING:SPACING: Unnecessary space before function pointer arguments
#831: FILE: drivers/net/qede/qede_eth_if.h:129:
+	int (*vport_update) (struct ecore_dev *edev,

WARNING:SPACING: Unnecessary space before function pointer arguments
#834: FILE: drivers/net/qede/qede_eth_if.h:132:
+	int (*q_rx_start) (struct ecore_dev *cdev,

CHECK:SPACING: spaces preferred around that '*' (ctx:VxO)
#840: FILE: drivers/net/qede/qede_eth_if.h:138:
+			   uint16_t cqe_pbl_size, void OSAL_IOMEM**pp_prod);
 			                                         ^

WARNING:SPACING: Unnecessary space before function pointer arguments
#842: FILE: drivers/net/qede/qede_eth_if.h:140:
+	int (*q_rx_stop) (struct ecore_dev *edev,

WARNING:SPACING: Unnecessary space before function pointer arguments
#845: FILE: drivers/net/qede/qede_eth_if.h:143:
+	int (*q_tx_start) (struct ecore_dev *edev,

CHECK:SPACING: spaces preferred around that '*' (ctx:VxO)
#850: FILE: drivers/net/qede/qede_eth_if.h:148:
+			   uint16_t pbl_size, void OSAL_IOMEM**pp_doorbell);
 			                                     ^

WARNING:SPACING: Unnecessary space before function pointer arguments
#852: FILE: drivers/net/qede/qede_eth_if.h:150:
+	int (*q_tx_stop) (struct ecore_dev *edev,

WARNING:SPACING: Unnecessary space before function pointer arguments
#855: FILE: drivers/net/qede/qede_eth_if.h:153:
+	int (*eth_cqe_completion) (struct ecore_dev *edev,

WARNING:SPACING: Unnecessary space before function pointer arguments
#859: FILE: drivers/net/qede/qede_eth_if.h:157:
+	int (*fastpath_stop) (struct ecore_dev *edev);

WARNING:SPACING: Unnecessary space before function pointer arguments
#861: FILE: drivers/net/qede/qede_eth_if.h:159:
+	void (*get_vport_stats) (struct ecore_dev *edev,

WARNING:SPACING: Unnecessary space before function pointer arguments
#864: FILE: drivers/net/qede/qede_eth_if.h:162:
+	int (*filter_config) (struct ecore_dev *edev,

CHECK:AVOID_EXTERNS: extern prototypes should be avoided in .h files
#872: FILE: drivers/net/qede/qede_eth_if.h:170:
+extern int qed_fill_eth_dev_info(struct ecore_dev *edev,

CHECK:SPACING: No space is necessary after a cast
#905: FILE: drivers/net/qede/qede_ethdev.c:17:
+	ecore_int_sp_dpc((osal_int_ptr_t) (p_hwfn));

CHECK:UNNECESSARY_PARENTHESES: Unnecessary parentheses around eth_dev->pci_dev->intr_handle
#916: FILE: drivers/net/qede/qede_ethdev.c:28:
+	if (rte_intr_enable(&(eth_dev->pci_dev->intr_handle)))

CHECK:BRACES: braces {} should be used on all arms of this statement
#1071: FILE: drivers/net/qede/qede_ethdev.c:183:
+	if (rte_eth_promiscuous_get(eth_dev->data->port_id) == 1)
[...]
+	else if (!qdev->non_configured_vlans) {
[...]

WARNING:MISSING_SPACE: break quoted strings at a space character
#1097: FILE: drivers/net/qede/qede_ethdev.c:209:
+			  "unequal number of rx/tx queues"
+			  "is not supported RX=%u TX=%u\n",

CHECK:SPACING: No space is necessary after a cast
#1151: FILE: drivers/net/qede/qede_ethdev.c:263:
+	dev_info->min_rx_bufsize = (uint32_t) (ETHER_MIN_MTU +

CHECK:SPACING: No space is necessary after a cast
#1153: FILE: drivers/net/qede/qede_ethdev.c:265:
+	dev_info->max_rx_pktlen = (uint32_t) ETH_TX_MAX_NON_LSO_PKT_LEN;

CHECK:SPACING: No space is necessary after a cast
#1156: FILE: drivers/net/qede/qede_ethdev.c:268:
+	dev_info->max_rx_queues = (uint16_t) QEDE_MAX_RSS_CNT(qdev);

CHECK:SPACING: No space is necessary after a cast
#1157: FILE: drivers/net/qede/qede_ethdev.c:269:
+	dev_info->max_tx_queues = (uint16_t) QEDE_MAX_TSS_CNT(qdev);

CHECK:SPACING: No space is necessary after a cast
#1158: FILE: drivers/net/qede/qede_ethdev.c:270:
+	dev_info->max_mac_addrs = (uint32_t) (RESC_NUM(&edev->hwfns[0],

CHECK:SPACING: No space is necessary after a cast
#1160: FILE: drivers/net/qede/qede_ethdev.c:272:
+	dev_info->max_vfs = (uint16_t) NUM_OF_VFS(&qdev->edev);

CHECK:SPACING: No space is necessary after a cast
#1163: FILE: drivers/net/qede/qede_ethdev.c:275:
+	dev_info->flow_type_rss_offloads = (uint64_t) QEDE_RSS_OFFLOAD_ALL;

CHECK:UNNECESSARY_PARENTHESES: Unnecessary parentheses around eth_dev->pci_dev->intr_handle
#1299: FILE: drivers/net/qede/qede_ethdev.c:411:
+	rte_intr_disable(&(eth_dev->pci_dev->intr_handle));

CHECK:UNNECESSARY_PARENTHESES: Unnecessary parentheses around eth_dev->pci_dev->intr_handle
#1301: FILE: drivers/net/qede/qede_ethdev.c:413:
+	rte_intr_callback_unregister(&(eth_dev->pci_dev->intr_handle),

CHECK:UNNECESSARY_PARENTHESES: Unnecessary parentheses around eth_dev->pci_dev->intr_handle
#1511: FILE: drivers/net/qede/qede_ethdev.c:623:
+	rte_intr_callback_register(&(eth_dev->pci_dev->intr_handle),

CHECK:UNNECESSARY_PARENTHESES: Unnecessary parentheses around eth_dev->pci_dev->intr_handle
#1514: FILE: drivers/net/qede/qede_ethdev.c:626:
+	if (rte_intr_enable(&(eth_dev->pci_dev->intr_handle))) {

WARNING:SPACING: Unnecessary space before function pointer arguments
#1948: FILE: drivers/net/qede/qede_if.h:95:
+	void (*link_update) (void *dev, struct qed_link_output *link);

WARNING:SPACING: Unnecessary space before function pointer arguments
#1959: FILE: drivers/net/qede/qede_if.h:106:
+	int (*registers) (struct ecore_dev *edev);

WARNING:SPACING: Unnecessary space before function pointer arguments
#1963: FILE: drivers/net/qede/qede_if.h:110:
+	int (*probe) (struct ecore_dev *edev,

WARNING:SPACING: Unnecessary space before function pointer arguments
#1967: FILE: drivers/net/qede/qede_if.h:114:
+	void (*set_id) (struct ecore_dev *edev,

WARNING:SPACING: Unnecessary space before function pointer arguments
#1969: FILE: drivers/net/qede/qede_if.h:116:
+	enum _ecore_status_t (*chain_alloc) (struct ecore_dev *edev,

WARNING:SPACING: Unnecessary space before function pointer arguments
#1978: FILE: drivers/net/qede/qede_if.h:125:
+	void (*chain_free) (struct ecore_dev *edev,

WARNING:SPACING: Unnecessary space before function pointer arguments
#1981: FILE: drivers/net/qede/qede_if.h:128:
+	void (*get_link) (struct ecore_dev *edev,

WARNING:SPACING: Unnecessary space before function pointer arguments
#1983: FILE: drivers/net/qede/qede_if.h:130:
+	int (*set_link) (struct ecore_dev *edev,

WARNING:SPACING: Unnecessary space before function pointer arguments
#1986: FILE: drivers/net/qede/qede_if.h:133:
+	int (*drain) (struct ecore_dev *edev);

WARNING:SPACING: Unnecessary space before function pointer arguments
#1988: FILE: drivers/net/qede/qede_if.h:135:
+	void (*remove) (struct ecore_dev *edev);

WARNING:SPACING: Unnecessary space before function pointer arguments
#1990: FILE: drivers/net/qede/qede_if.h:137:
+	int (*slowpath_stop) (struct ecore_dev *edev);

WARNING:SPACING: Unnecessary space before function pointer arguments
#1992: FILE: drivers/net/qede/qede_if.h:139:
+	void (*update_pf_params) (struct ecore_dev *edev,

WARNING:SPACING: Unnecessary space before function pointer arguments
#1995: FILE: drivers/net/qede/qede_if.h:142:
+	int (*slowpath_start) (struct ecore_dev *edev,

WARNING:SPACING: Unnecessary space before function pointer arguments
#1998: FILE: drivers/net/qede/qede_if.h:145:
+	int (*set_fp_int) (struct ecore_dev *edev, uint16_t cnt);

WARNING:SPACING: missing space after return type
#2000: FILE: drivers/net/qede/qede_if.h:147:
+	 uint32_t(*sb_init) (struct ecore_dev *edev,

WARNING:SPACING: Unnecessary space before function pointer arguments
#2000: FILE: drivers/net/qede/qede_if.h:147:
+	 uint32_t(*sb_init) (struct ecore_dev *edev,

WARNING:SPACING: missing space after return type
#2006: FILE: drivers/net/qede/qede_if.h:153:
+	 bool(*can_link_change) (struct ecore_dev *edev);

WARNING:SPACING: Unnecessary space before function pointer arguments
#2006: FILE: drivers/net/qede/qede_if.h:153:
+	 bool(*can_link_change) (struct ecore_dev *edev);

WARNING:SPACING: Unnecessary space before function pointer arguments
#2007: FILE: drivers/net/qede/qede_if.h:154:
+	void (*update_msglvl) (struct ecore_dev *edev,

WARNING:TRAILING_SEMICOLON: macros should not use a trailing semicolon
#2038: FILE: drivers/net/qede/qede_logs.h:12:
+#define DP_ERR(p_dev, fmt, ...) \
+	rte_log(RTE_LOG_ERR, RTE_LOGTYPE_PMD, \
+		"[%s:%d(%s)]" fmt, \
+		  __func__, __LINE__, \
+		(p_dev)->name ? (p_dev)->name : "", \
+		##__VA_ARGS__);

WARNING:TRAILING_SEMICOLON: macros should not use a trailing semicolon
#2102: FILE: drivers/net/qede/qede_logs.h:76:
+#define PMD_RX_LOG(level, q, fmt, args...) \
+	RTE_LOG(level, PMD, "%s(): port=%u queue=%u " fmt "\n",	\
+		__func__, q->port_id, q->queue_id, ## args);

WARNING:SUSPECT_CODE_INDENT: suspect code indent for conditional statements (16, 20)
#2469: FILE: drivers/net/qede/qede_main.c:344:
+		for_each_hwfn(edev, i)
+		    info->num_queues +=

CHECK:SPACING: No space is necessary after a cast
#2484: FILE: drivers/net/qede/qede_main.c:359:
+				      (uint8_t *) &info->port_mac);

WARNING:SUSPECT_CODE_INDENT: suspect code indent for conditional statements (8, 12)
#2582: FILE: drivers/net/qede/qede_main.c:457:
+	for_each_hwfn(cdev, i)
+	    qed_inform_vf_link_state(&cdev->hwfns[i]);

CHECK:BRACES: Blank lines aren't necessary before a close brace '}'
#2586: FILE: drivers/net/qede/qede_main.c:461:
+
+}

WARNING:MISSING_SPACE: break quoted strings at a space character
#2753: FILE: drivers/net/qede/qede_rxtx.c:35:
+			   "Failed to allocate rx buffer"
+			   "sw_rx_prod %u sw_rx_cons %u mp entries %u free %u",

WARNING:CONSTANT_COMPARISON: Comparisons should place the constant on the right side of the test
#2776: FILE: drivers/net/qede/qede_rxtx.c:58:
+			if (NULL != rxq->sw_rx_ring[i].mbuf) {

CHECK:SPACING: No space is necessary after a cast
#2817: FILE: drivers/net/qede/qede_rxtx.c:99:
+	uint16_t pkt_len = (uint16_t) dev->data->dev_conf.rxmode.max_rx_pkt_len;

WARNING:MISSING_SPACE: break quoted strings at a space character
#3199: FILE: drivers/net/qede/qede_rxtx.c:481:
+				  "Failed to allocate memory for all of"
+				  "RSS queues\n"

CHECK:SPACING: No space is necessary after a cast
#3228: FILE: drivers/net/qede/qede_rxtx.c:510:
+			(uint32_t *) &rx_prods);

WARNING:UNNECESSARY_ELSE: else is not generally useful after a break or return
#3275: FILE: drivers/net/qede/qede_rxtx.c:557:
+		return -EINVAL;
+	} else {

WARNING:MISSING_SPACE: break quoted strings at a space character
#3325: FILE: drivers/net/qede/qede_rxtx.c:607:
+		       "Cannot update V-VPORT as active as"
+		       "there are no Rx queues\n");

CHECK:SPACING: No space is necessary after a cast
#3587: FILE: drivers/net/qede/qede_rxtx.c:869:
+				(struct eth_slow_path_rx_cqe *) cqe);

CHECK:REDUNDANT_CODE: if this code is redundant consider removing it
#3601: FILE: drivers/net/qede/qede_rxtx.c:883:
+#if 0

WARNING:MISSING_SPACE: break quoted strings at a space character
#3617: FILE: drivers/net/qede/qede_rxtx.c:899:
+				   "CQE in CONS = %u has error, flags = 0x%x"
+				   "dropping incoming packet\n",

WARNING:MISSING_SPACE: break quoted strings at a space character
#3688: FILE: drivers/net/qede/qede_rxtx.c:970:
+			   "null mbuf nb_tx_desc %u nb_tx_avail %u"
+			   "sw_tx_cons %u sw_tx_prod %u\n",

WARNING:UNNECESSARY_ELSE: else is not generally useful after a break or return
#3893: FILE: drivers/net/qede/qede_rxtx.c:1175:
+				return qede_drain_txq(qdev, txq, false);
+			} else {

CHECK:SPACING: No space is necessary after a cast
#4113: FILE: drivers/net/qede/qede_rxtx.h:43:
+	((uint64_t) ((mb)->buf_physaddr + (mb)->data_off))

  reply	other threads:[~2016-02-21  5:40 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-20 15:40 [dpdk-dev] [PATCH 0/6] DPDK PMD for new QLogic FastLinQ QL4xxxx 25G/40G CNAs Harish Patil
2016-02-20 15:40 ` [dpdk-dev] [PATCH 1/6] qede: add maintainers Harish Patil
2016-02-20 15:40 ` [dpdk-dev] [PATCH 2/6] qede: add documentation Harish Patil
2016-02-22 16:52   ` Mcnamara, John
2016-02-24  7:17     ` Harish Patil
2016-02-24  9:26       ` Mcnamara, John
2016-02-22 17:38   ` Mcnamara, John
2016-02-20 15:40 ` [dpdk-dev] [PATCH 3/6] qede: add QLogic PCI ids Harish Patil
2016-02-21  1:17   ` Stephen Hemminger
2016-02-22 23:23     ` Harish Patil
2016-02-20 15:40 ` [dpdk-dev] [PATCH 5/6] qede: add driver Harish Patil
2016-02-21  1:26   ` Stephen Hemminger [this message]
2016-02-23  2:28     ` Harish Patil
2016-02-23  5:30       ` Stephen Hemminger
2016-02-23  5:33       ` Stephen Hemminger
2016-02-23 19:04         ` Harish Patil
2016-02-23 19:06           ` Harish Patil
2016-02-20 15:40 ` [dpdk-dev] [PATCH 6/6] qede: enable PMD build Harish Patil
2016-02-20 20:49 ` [dpdk-dev] [PATCH 0/6] DPDK PMD for new QLogic FastLinQ QL4xxxx 25G/40G CNAs Thomas Monjalon
2016-02-22 16:47   ` Harish Patil
2016-02-22 17:35     ` Thomas Monjalon
2016-02-23 18:13       ` Harish Patil
2016-03-08 14:01 ` Bruce Richardson
2016-03-08 14:24   ` Harish Patil
2016-03-10 17:22     ` Harish Patil

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=20160220172623.5101c7e4@xeon-e3 \
    --to=stephen@networkplumber.org \
    --cc=dev@dpdk.org \
    --cc=harish.patil@qlogic.com \
    --cc=sony.chacko@qlogic.com \
    /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).