Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1361086

Re: [PATCH 1/3] infiniband: IB/hns: add Hisilicon RoCE support

From oulijun <oulijun@huawei.com>
Newsgroups linux.kernel
Subject Re: [PATCH 1/3] infiniband: IB/hns: add Hisilicon RoCE support
Date 2016-03-19 12:20 +0100
Message-ID <ren5f-7wu-1@gated-at.bofh.it> (permalink)
References (1 earlier) <rbsuu-4D2-29@gated-at.bofh.it> <rbsNQ-4Mh-9@gated-at.bofh.it> <rdh1U-7IW-13@gated-at.bofh.it> <rdh1T-7IW-11@gated-at.bofh.it> <rdzUS-3Ge-5@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On 2016/3/17 14:43, Leon Romanovsky wrote:
> On Wed, Mar 16, 2016 at 11:36:38AM +0100, Jiri Pirko wrote:
>>> so, I continue to have it.
>>
>> I will continue to bash on your odd codingstyle. Please fix it!
> 
> Jiri,
> 
> Checkpatch errors is an easiest issue with this patch.
> 
> It is full of functions without use, unconnected macros and
> if you replace "hsi" to name of other well known driver, you will get
> same code :).
> 
> They need to redesign the whole driver before resubmission.
> 
> Thanks.
> 
> .
> 
Hi, Leon Romanovsky
    Firstly, thanks for reviewing.
    I have checked the patch v2, surely, some funtions without use and unconnected
macros exist. I have removed it according to analyse. I will leave few marcos as follow:
    #define CQ_STATE_INVALID    0
    #define CQ_STATE_RESERV     1
    #define CQ_STATE_VALID      2
    #define CQ_STATE_ERR        3

I thought that these are defined for hardware information. So, I reserved these macros.

In addtion, I didn't completly understand your review as below:
  if you replace "hsi" to name of other well known driver, you will get > same code :).

thanks
Lijun Ou

Back to linux.kernel | Previous | Next | Find similar | Unroll thread


Thread

Re: [PATCH 1/3] infiniband: IB/hns: add Hisilicon RoCE support oulijun <oulijun@huawei.com> - 2016-03-19 12:20 +0100

csiph-web