From: Harish Patil <harish.patil@qlogic.com>
To: Stephen Hemminger <stephen@networkplumber.org>
Cc: "dev@dpdk.org" <dev@dpdk.org>, Sony Chacko <sony.chacko@qlogic.com>
Subject: Re: [dpdk-dev] [PATCH 5/6] qede: add driver
Date: Tue, 23 Feb 2016 02:28:25 +0000 [thread overview]
Message-ID: <D2F0D889.157D8E%harish.patil@qlogic.com> (raw)
In-Reply-To: <20160220172623.5101c7e4@xeon-e3>
>
>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,NETWORKIN
>G_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))
>
>
All of the checkpatch warnings had been fixed (except one which cannot be
fixed) using the checkpatch script available under DPDK scripts/
directory. The linux checkpatch version is 0.32.
[root@dut4019 dpdk]# ./scripts/checkpatches.sh patches-send/*.patch
### patches-send/0004-qede-add-driver-common-module.patch
WARNING:CAMELCASE: Avoid CamelCase: <inflateEnd>
#251: FILE: drivers/net/qede/ecore/bcm_osal.c:120:
+ inflateEnd(p_hwfn->stream);
total: 0 errors, 1 warnings, 54467 lines checked
Wouldn’t that suffice?
________________________________
This message and any attached documents contain information from the sending company or its parent company(s), subsidiaries, divisions or branch offices that may be confidential. If you are not the intended recipient, you may not read, copy, distribute, or use this information. If you have received this transmission in error, please notify the sender immediately by reply e-mail and then delete this message.
next prev parent reply other threads:[~2016-02-23 2:28 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
2016-02-23 2:28 ` Harish Patil [this message]
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=D2F0D889.157D8E%harish.patil@qlogic.com \
--to=harish.patil@qlogic.com \
--cc=dev@dpdk.org \
--cc=sony.chacko@qlogic.com \
--cc=stephen@networkplumber.org \
/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).