linux-kernel - Re: [PATCH v10 04/22] IB/hns: Add RoCE engine reset function

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20160628080926.GC3584@leon.nu>
Date:	Tue, 28 Jun 2016 11:09:26 +0300
From:	Leon Romanovsky <leon@...nel.org>
To:	"Wei Hu (Xavier)" <xavier.huwei@...wei.com>
Cc:	oulijun <oulijun@...wei.com>, dledford@...hat.com,
	sean.hefty@...el.com, hal.rosenstock@...il.com,
	davem@...emloft.net, jeffrey.t.kirsher@...el.com,
	jiri@...lanox.com, ogerlitz@...lanox.com,
	linux-rdma@...r.kernel.org, linux-kernel@...r.kernel.org,
	netdev@...r.kernel.org, gongyangming@...wei.com,
	xiaokun@...wei.com, tangchaofei@...wei.com, haifeng.wei@...wei.com,
	yisen.zhuang@...wei.com, yankejian@...wei.com,
	charles.chenxin@...wei.com, linuxarm@...wei.com
Subject: Re: [PATCH v10 04/22] IB/hns: Add RoCE engine reset function

On Tue, Jun 28, 2016 at 02:31:41PM +0800, Wei Hu (Xavier) wrote:
> 
> 
> On 2016/6/27 16:31, oulijun wrote:
> >Hi, Leon
> >在 2016/6/27 16:01, Leon Romanovsky 写道:
> >>On Sat, Jun 25, 2016 at 06:25:37PM +0800, Wei Hu (Xavier) wrote:
> >>>
> >>>On 2016/6/24 22:59, Leon Romanovsky wrote:
> >>>>On Thu, Jun 16, 2016 at 10:35:12PM +0800, Lijun Ou wrote:
> >>>>>This patch mainly added reset flow of RoCE engine in RoCE
> >>>>>driver. It is necessary when RoCE is loaded and removed.
> >>>>>
> >>>>>Signed-off-by: Wei Hu <xavier.huwei@...wei.com>
> >>>>>Signed-off-by: Nenglong Zhao <zhaonenglong@...ilicon.com>
> >>>>>Signed-off-by: Lijun Ou <oulijun@...wei.com>
> >>>>>---
> >>...
> >>
> >>>>>+
> >>>>>+#define SLEEP_TIME_INTERVAL				20
> >>>>>+
> >>>>>+extern int hns_dsaf_roce_reset(struct fwnode_handle *dsaf_fwnode, bool enable);
> >>>>Why did you add this extern?
> >>>>You already exported this function.
> >>>>drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c:EXPORT_SYMBOL(hns_dsaf_roce_reset);
> >>>Hi, Leon
> >>>
> >>>         The function named hns_dsaf_roce_reset is defined in hns_dsaf_main.c
> >>>         It exists in hns_dsaf.ko(ethernet driver)
> >>>
> >>>         RoCE driver will call this function.
> >>>
> >>>         Your suggestion is that delete "extern" as below:
> >>>             In /drivers/infiniband/hw/hns/hns_roce_hw_v1.h:
> >>>
> >>>           int hns_dsaf_roce_reset(struct fwnode_handle *dsaf_fwnode, bool
> >>>enable);
> >>>
> >>>Right? or other soultion?
> >>You placed it in header file.
> >>Please move it to your hns_roce_hw_v1.c file.
> >>
> >  You suggest to do as follows, right?
> >  in hns_roce_hw_v1.c
> >    int hns_dsaf_roce_reset(struct fwnode_handle *dsaf_fwnode, bool enable);
> >
> >  and delete the keyword extern
> >
> >  Bcause reserve the extern in hns_roce_hw_v1.c, the checkpatch is not pass.
> Hi, Leon & Doug Ledford
> 
>     If we move it to hns_roce_hw_v1.c file as below:
>         int hns_dsaf_roce_reset(struct fwnode_handle *dsaf_fwnode, bool
> enable);
>     The result of checkpatch is warning.
> 
>     We prepare to add a head file for this function as below:
>         In the directory of include\linux,  mkdir hns.
>         add hns_driver.h in include\linux\hns.
>         In the file of hns_driver.h, the declaration:
>                int hns_dsaf_roce_reset(struct fwnode_handle *dsaf_fwnode,
> bool enable);
>     What do you think about?
> 
>

Please avoid creating new directories/files under include/linux,
especially for one function only.

Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)