Hi Stephen, Thank you for your comments. We will fix your comments at the next patches.
Thanks, Ziyang Xuan -----邮件原件----- 发件人: Stephen Hemminger [mailto:step...@networkplumber.org] 发送时间: 2019年5月29日 12:59 收件人: Xuanziyang (William, Chip Application Design Logic and Hardware Development Dept IT_Products & Solutions) <xuanziya...@huawei.com> 抄送: dev@dpdk.org; ferruh.yi...@intel.com; Wangxiaoyun (Cloud, Network Chip Application Development Dept) <cloud.wangxiao...@huawei.com>; zhouguoyang <zhouguoy...@huawei.com>; Shahar Belkar <shahar.bel...@huawei.com>; Luoxianjun <luoxian...@huawei.com> 主题: Re: [dpdk-dev] [PATCH v2 08/11] net/hinic: add hinic PMD build and doc files On Wed, 29 May 2019 11:50:36 +0800 Ziyang Xuan <xuanziya...@huawei.com> wrote: > +Huawei hinic > +M: Xiaoyun Wang <cloud.wangxiao...@huawei.com> > +M: Ziyang Xuan <xuanziya...@huawei.com> > +M: Guoyang Zhou <zhouguoy...@huawei.com> > +M: Rami Rosen <rami.ro...@huawei.com> You should remove Rami, he will be missed. > diff --git a/drivers/net/hinic/base/hinic_logs.c > b/drivers/net/hinic/base/hinic_logs.c > new file mode 100644 > index 000000000..1e9b5ad92 > --- /dev/null > +++ b/drivers/net/hinic/base/hinic_logs.c > @@ -0,0 +1,16 @@ > +/* SPDX-License-Identifier: BSD-3-Clause > + * Copyright(c) 2017 Huawei Technologies Co., Ltd */ > + > + > +#include "hinic_logs.h" > + > +int hinic_logtype; > + > +RTE_INIT(hinic_init_log) > +{ > + hinic_logtype = rte_log_register("pmd.net.hinic"); > + if (hinic_logtype >= 0) > + rte_log_set_level(hinic_logtype, RTE_LOG_INFO); } > + You could just put this in some other file > diff --git a/drivers/net/hinic/base/hinic_logs.h > b/drivers/net/hinic/base/hinic_logs.h > new file mode 100644 > index 000000000..ed84a37a6 > --- /dev/null > +++ b/drivers/net/hinic/base/hinic_logs.h > @@ -0,0 +1,31 @@ > +/* SPDX-License-Identifier: BSD-3-Clause > + * Copyright(c) 2017 Huawei Technologies Co., Ltd */ > + > +#ifndef _HINIC_LOGS_H_ > +#define _HINIC_LOGS_H_ > + > +#ifdef __cplusplus > +#if __cplusplus > +extern "C"{ > +#endif > +#endif /* __cplusplus */ > +#include <rte_log.h> Why all the C++ guards, this is a private file. > +error_cflags = ['-Wno-unused-value', > + '-Wno-unused-but-set-variable', > + '-Wno-unused-variable', > +] Please fix these errors instead of masking them > +/** > + * Atomically writes the link status information into global > + * struct rte_eth_dev. > + */ > +int hinic_dev_atomic_write_link_status(struct rte_eth_dev *dev, > + struct rte_eth_link *link) { > + struct rte_eth_link *dst = &dev->data->dev_link; > + struct rte_eth_link *src = link; > + > + if (rte_atomic64_cmpset((uint64_t *)dst, *(uint64_t *)dst, > + *(uint64_t *)src) == 0) > + return HINIC_ERROR; > + > + return HINIC_OK; > +} There already is code in rte_ethdev in current versions of DPDK to do this.
