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.hu...@huawei.com> > >>>>>Signed-off-by: Nenglong Zhao <zhaonengl...@hisilicon.com> > >>>>>Signed-off-by: Lijun Ou <ouli...@huawei.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.
signature.asc
Description: Digital signature
