Re: [PATCH 3/4] infiniband: hns: add Hisilicon RoCE support(driver code)

[Parav Pandit]

On Sat, Mar 19, 2016 at 4:58 PM, oulijun  wrote:
> On 2016/3/15 2:20, Parav Pandit wrote:

 Since SRQ is not supported in this driver version, can you keep
 remaining code base also to not bother about SRQ specifically
 poll_cq_one, modify_qp, destroy_qp etc?
 SRQ support can come as complete additional patch along with cmd_mask,
 callbacks and rest of the code.

 .
>>> Sorry, I see your review in time.
>>> Sure, SRQ is not supported in current roce driver. I have verified the 
>>> function
>>> for RDMA. It is not influence. For your question, we need to analyse it 
>>> scientific.
>>> after that, i will reply your doubt, is that ok?
>>
>> Yes. No problem.
>>
>> .
>>
> Hi, Parav Pandit
> I have analyse and discuss with your reviewing. I considered that the srq 
> is only the
> condition branch in verbs and without independent function, so reserved it.I 
> have delete the relative
> function with srq independently.
> if delete the branch operation with srq, it feel be inconvenient to 
> understand
>

o.k. If I understand correctly, new patch will be without srq functionality.
If thats the case, that makes review and maintainability easier.
Thanks.
Parav

Re: [PATCH 3/4] infiniband: hns: add Hisilicon RoCE support(driver code)

[Parav Pandit]

On Sat, Mar 19, 2016 at 4:58 PM, oulijun  wrote:
> On 2016/3/15 2:20, Parav Pandit wrote:

 Since SRQ is not supported in this driver version, can you keep
 remaining code base also to not bother about SRQ specifically
 poll_cq_one, modify_qp, destroy_qp etc?
 SRQ support can come as complete additional patch along with cmd_mask,
 callbacks and rest of the code.

 .
>>> Sorry, I see your review in time.
>>> Sure, SRQ is not supported in current roce driver. I have verified the 
>>> function
>>> for RDMA. It is not influence. For your question, we need to analyse it 
>>> scientific.
>>> after that, i will reply your doubt, is that ok?
>>
>> Yes. No problem.
>>
>> .
>>
> Hi, Parav Pandit
> I have analyse and discuss with your reviewing. I considered that the srq 
> is only the
> condition branch in verbs and without independent function, so reserved it.I 
> have delete the relative
> function with srq independently.
> if delete the branch operation with srq, it feel be inconvenient to 
> understand
>

o.k. If I understand correctly, new patch will be without srq functionality.
If thats the case, that makes review and maintainability easier.
Thanks.
Parav

Re: [PATCH 3/4] infiniband: hns: add Hisilicon RoCE support(driver code)

[oulijun]

On 2016/3/15 2:20, Parav Pandit wrote:
>>>
>>> Since SRQ is not supported in this driver version, can you keep
>>> remaining code base also to not bother about SRQ specifically
>>> poll_cq_one, modify_qp, destroy_qp etc?
>>> SRQ support can come as complete additional patch along with cmd_mask,
>>> callbacks and rest of the code.
>>>
>>> .
>> Sorry, I see your review in time.
>> Sure, SRQ is not supported in current roce driver. I have verified the 
>> function
>> for RDMA. It is not influence. For your question, we need to analyse it 
>> scientific.
>> after that, i will reply your doubt, is that ok?
> 
> Yes. No problem.
> 
> .
> 
Hi, Parav Pandit
I have analyse and discuss with your reviewing. I considered that the srq 
is only the
condition branch in verbs and without independent function, so reserved it.I 
have delete the relative
function with srq independently.
if delete the branch operation with srq, it feel be inconvenient to 
understand

thanks
Lijun Ou

Re: [PATCH 3/4] infiniband: hns: add Hisilicon RoCE support(driver code)

[oulijun]

On 2016/3/15 2:20, Parav Pandit wrote:
>>>
>>> Since SRQ is not supported in this driver version, can you keep
>>> remaining code base also to not bother about SRQ specifically
>>> poll_cq_one, modify_qp, destroy_qp etc?
>>> SRQ support can come as complete additional patch along with cmd_mask,
>>> callbacks and rest of the code.
>>>
>>> .
>> Sorry, I see your review in time.
>> Sure, SRQ is not supported in current roce driver. I have verified the 
>> function
>> for RDMA. It is not influence. For your question, we need to analyse it 
>> scientific.
>> after that, i will reply your doubt, is that ok?
> 
> Yes. No problem.
> 
> .
> 
Hi, Parav Pandit
I have analyse and discuss with your reviewing. I considered that the srq 
is only the
condition branch in verbs and without independent function, so reserved it.I 
have delete the relative
function with srq independently.
if delete the branch operation with srq, it feel be inconvenient to 
understand

thanks
Lijun Ou

Re: [PATCH 3/4] infiniband: hns: add Hisilicon RoCE support(driver code)

[Parav Pandit]

>>
>> Since SRQ is not supported in this driver version, can you keep
>> remaining code base also to not bother about SRQ specifically
>> poll_cq_one, modify_qp, destroy_qp etc?
>> SRQ support can come as complete additional patch along with cmd_mask,
>> callbacks and rest of the code.
>>
>> .
> Sorry, I see your review in time.
> Sure, SRQ is not supported in current roce driver. I have verified the 
> function
> for RDMA. It is not influence. For your question, we need to analyse it 
> scientific.
> after that, i will reply your doubt, is that ok?

Yes. No problem.

Re: [PATCH 3/4] infiniband: hns: add Hisilicon RoCE support(driver code)

[Parav Pandit]

>>
>> Since SRQ is not supported in this driver version, can you keep
>> remaining code base also to not bother about SRQ specifically
>> poll_cq_one, modify_qp, destroy_qp etc?
>> SRQ support can come as complete additional patch along with cmd_mask,
>> callbacks and rest of the code.
>>
>> .
> Sorry, I see your review in time.
> Sure, SRQ is not supported in current roce driver. I have verified the 
> function
> for RDMA. It is not influence. For your question, we need to analyse it 
> scientific.
> after that, i will reply your doubt, is that ok?

Yes. No problem.

Re: [PATCH 3/4] infiniband: hns: add Hisilicon RoCE support(driver code)

[oulijun]

Hi Parav Pandit, thanks your reviewing.
On 2016/3/4 17:37, Parav Pandit wrote:
> On Fri, Mar 4, 2016 at 2:11 PM, Wei Hu(Xavier)  
> wrote:
>> +
>> +int hns_roce_register_device(struct hns_roce_dev *hr_dev)
>> +{
>> +   int ret;
>> +   struct hns_roce_ib_iboe *iboe = NULL;
>> +   struct ib_device *ib_dev = NULL;
>> +   struct device *dev = _dev->pdev->dev;
>> +
>> +   iboe = _dev->iboe;
>> +
>> +   ib_dev = _dev->ib_dev;
>> +   strlcpy(ib_dev->name, "hisi_%d", IB_DEVICE_NAME_MAX);
>> +
>> +   ib_dev->owner   = THIS_MODULE;
>> +   ib_dev->node_type   = RDMA_NODE_IB_CA;
>> +   ib_dev->dma_device  = dev;
>> +
>> +   ib_dev->phys_port_cnt   = hr_dev->caps.num_ports;
>> +   ib_dev->local_dma_lkey  = hr_dev->caps.reserved_lkey;
>> +   ib_dev->num_comp_vectors= hr_dev->caps.num_comp_vectors;
>> +   ib_dev->uverbs_abi_ver  = 1;
>> +   ib_dev->uverbs_cmd_mask =
>> +   (1ULL << IB_USER_VERBS_CMD_GET_CONTEXT) |
>> +   (1ULL << IB_USER_VERBS_CMD_QUERY_DEVICE) |
>> +   (1ULL << IB_USER_VERBS_CMD_QUERY_PORT) |
>> +   (1ULL << IB_USER_VERBS_CMD_ALLOC_PD) |
>> +   (1ULL << IB_USER_VERBS_CMD_DEALLOC_PD) |
>> +   (1ULL << IB_USER_VERBS_CMD_REG_MR) |
>> +   (1ULL << IB_USER_VERBS_CMD_DEREG_MR) |
>> +   (1ULL << IB_USER_VERBS_CMD_CREATE_COMP_CHANNEL) |
>> +   (1ULL << IB_USER_VERBS_CMD_CREATE_CQ) |
>> +   (1ULL << IB_USER_VERBS_CMD_DESTROY_CQ) |
>> +   (1ULL << IB_USER_VERBS_CMD_CREATE_QP) |
>> +   (1ULL << IB_USER_VERBS_CMD_MODIFY_QP) |
>> +   (1ULL << IB_USER_VERBS_CMD_QUERY_QP) |
>> +   (1ULL << IB_USER_VERBS_CMD_DESTROY_QP);
>> +
> 
> Since SRQ is not supported in this driver version, can you keep
> remaining code base also to not bother about SRQ specifically
> poll_cq_one, modify_qp, destroy_qp etc?
> SRQ support can come as complete additional patch along with cmd_mask,
> callbacks and rest of the code.
> 
> .
Sorry, I see your review in time.
Sure, SRQ is not supported in current roce driver. I have verified the function
for RDMA. It is not influence. For your question, we need to analyse it 
scientific.
after that, i will reply your doubt, is that ok?

thanks
Lijun Ou

> 

Re: [PATCH 3/4] infiniband: hns: add Hisilicon RoCE support(driver code)

[oulijun]

Hi Parav Pandit, thanks your reviewing.
On 2016/3/4 17:37, Parav Pandit wrote:
> On Fri, Mar 4, 2016 at 2:11 PM, Wei Hu(Xavier)  
> wrote:
>> +
>> +int hns_roce_register_device(struct hns_roce_dev *hr_dev)
>> +{
>> +   int ret;
>> +   struct hns_roce_ib_iboe *iboe = NULL;
>> +   struct ib_device *ib_dev = NULL;
>> +   struct device *dev = _dev->pdev->dev;
>> +
>> +   iboe = _dev->iboe;
>> +
>> +   ib_dev = _dev->ib_dev;
>> +   strlcpy(ib_dev->name, "hisi_%d", IB_DEVICE_NAME_MAX);
>> +
>> +   ib_dev->owner   = THIS_MODULE;
>> +   ib_dev->node_type   = RDMA_NODE_IB_CA;
>> +   ib_dev->dma_device  = dev;
>> +
>> +   ib_dev->phys_port_cnt   = hr_dev->caps.num_ports;
>> +   ib_dev->local_dma_lkey  = hr_dev->caps.reserved_lkey;
>> +   ib_dev->num_comp_vectors= hr_dev->caps.num_comp_vectors;
>> +   ib_dev->uverbs_abi_ver  = 1;
>> +   ib_dev->uverbs_cmd_mask =
>> +   (1ULL << IB_USER_VERBS_CMD_GET_CONTEXT) |
>> +   (1ULL << IB_USER_VERBS_CMD_QUERY_DEVICE) |
>> +   (1ULL << IB_USER_VERBS_CMD_QUERY_PORT) |
>> +   (1ULL << IB_USER_VERBS_CMD_ALLOC_PD) |
>> +   (1ULL << IB_USER_VERBS_CMD_DEALLOC_PD) |
>> +   (1ULL << IB_USER_VERBS_CMD_REG_MR) |
>> +   (1ULL << IB_USER_VERBS_CMD_DEREG_MR) |
>> +   (1ULL << IB_USER_VERBS_CMD_CREATE_COMP_CHANNEL) |
>> +   (1ULL << IB_USER_VERBS_CMD_CREATE_CQ) |
>> +   (1ULL << IB_USER_VERBS_CMD_DESTROY_CQ) |
>> +   (1ULL << IB_USER_VERBS_CMD_CREATE_QP) |
>> +   (1ULL << IB_USER_VERBS_CMD_MODIFY_QP) |
>> +   (1ULL << IB_USER_VERBS_CMD_QUERY_QP) |
>> +   (1ULL << IB_USER_VERBS_CMD_DESTROY_QP);
>> +
> 
> Since SRQ is not supported in this driver version, can you keep
> remaining code base also to not bother about SRQ specifically
> poll_cq_one, modify_qp, destroy_qp etc?
> SRQ support can come as complete additional patch along with cmd_mask,
> callbacks and rest of the code.
> 
> .
Sorry, I see your review in time.
Sure, SRQ is not supported in current roce driver. I have verified the function
for RDMA. It is not influence. For your question, we need to analyse it 
scientific.
after that, i will reply your doubt, is that ok?

thanks
Lijun Ou

> 

Re: [PATCH 3/4] infiniband: hns: add Hisilicon RoCE support(driver code)

[Jiri Pirko]

Wed, Mar 09, 2016 at 12:18:06PM CET, ouli...@huawei.com wrote:
>Hi Jiri Pirko, thanks for reviewing
>On 2016/3/4 17:16, Jiri Pirko wrote:
>> Fri, Mar 04, 2016 at 09:41:16AM CET, xavier.hu...@huawei.com wrote:
>> 
>> 
>> 
>>> +int hns_roce_buf_alloc(
>>> +   struct hns_roce_dev *hr_dev,
>>> +   int size, int max_direct,
>>> +   struct hns_roce_buf *buf)
>> 
>>
>>  
>>> +
>>> +   pages =
>>> +   kmalloc(sizeof(*pages) * buf->nbufs,
>>> +   GFP_KERNEL);
>> 
>> 
>> 
>>> +
>>> +   buf->direct.buf = vmap(
>>> +   pages, buf->nbufs, VM_MAP,
>>> +   PAGE_KERNEL);
>> 
>> 
>> 
>>> +   if (
>>> +   event_type != HNS_ROCE_EVENT_TYPE_CQ_ID_INVALID &&
>>> +   event_type != HNS_ROCE_EVENT_TYPE_CQ_ACCESS_ERROR &&
>>> +   event_type != HNS_ROCE_EVENT_TYPE_CQ_OVERFLOW) {
>>> +   dev_err(_dev->pdev->dev,
>>> +   "hns_roce_ib: Unexpected event type 0x%x on CQ %06x\n",
>>> +   event_type, hr_cq->cqn);
>>> +   return;
>>> +   }
>> 
>> Although checkpatch does not complain, I find this semi-random adding of
>> newlines quite odd.
>> 
>Really, the question you mentioned exit in many location in currently 
> patch. I done it
>in order to make it complain checkpatch and linux norms. Now, I have checked 
>and adjust it
>properly combined to checkpatch
>I will send a new patch in future. if not modified in some locations, it 
> have to violate
>checkpatch once modified and is unable to adjust it better. About these,  have 
>you best strategy?

I'm not sure what violation you are talking about. I'm just simply
suggesting to change:
buf->direct.buf = vmap(
pages, buf->nbufs, VM_MAP,
PAGE_KERNEL);
to:
buf->direct.buf = vmap(pages, buf->nbufs, VM_MAP,
   PAGE_KERNEL);


and to change:
pages =
kmalloc(sizeof(*pages) * buf->nbufs,
GFP_KERNEL);
to:
pages = kmalloc(sizeof(*pages) * buf->nbufs,
GFP_KERNEL);

Re: [PATCH 3/4] infiniband: hns: add Hisilicon RoCE support(driver code)

[Jiri Pirko]

Wed, Mar 09, 2016 at 12:18:06PM CET, ouli...@huawei.com wrote:
>Hi Jiri Pirko, thanks for reviewing
>On 2016/3/4 17:16, Jiri Pirko wrote:
>> Fri, Mar 04, 2016 at 09:41:16AM CET, xavier.hu...@huawei.com wrote:
>> 
>> 
>> 
>>> +int hns_roce_buf_alloc(
>>> +   struct hns_roce_dev *hr_dev,
>>> +   int size, int max_direct,
>>> +   struct hns_roce_buf *buf)
>> 
>>
>>  
>>> +
>>> +   pages =
>>> +   kmalloc(sizeof(*pages) * buf->nbufs,
>>> +   GFP_KERNEL);
>> 
>> 
>> 
>>> +
>>> +   buf->direct.buf = vmap(
>>> +   pages, buf->nbufs, VM_MAP,
>>> +   PAGE_KERNEL);
>> 
>> 
>> 
>>> +   if (
>>> +   event_type != HNS_ROCE_EVENT_TYPE_CQ_ID_INVALID &&
>>> +   event_type != HNS_ROCE_EVENT_TYPE_CQ_ACCESS_ERROR &&
>>> +   event_type != HNS_ROCE_EVENT_TYPE_CQ_OVERFLOW) {
>>> +   dev_err(_dev->pdev->dev,
>>> +   "hns_roce_ib: Unexpected event type 0x%x on CQ %06x\n",
>>> +   event_type, hr_cq->cqn);
>>> +   return;
>>> +   }
>> 
>> Although checkpatch does not complain, I find this semi-random adding of
>> newlines quite odd.
>> 
>Really, the question you mentioned exit in many location in currently 
> patch. I done it
>in order to make it complain checkpatch and linux norms. Now, I have checked 
>and adjust it
>properly combined to checkpatch
>I will send a new patch in future. if not modified in some locations, it 
> have to violate
>checkpatch once modified and is unable to adjust it better. About these,  have 
>you best strategy?

I'm not sure what violation you are talking about. I'm just simply
suggesting to change:
buf->direct.buf = vmap(
pages, buf->nbufs, VM_MAP,
PAGE_KERNEL);
to:
buf->direct.buf = vmap(pages, buf->nbufs, VM_MAP,
   PAGE_KERNEL);


and to change:
pages =
kmalloc(sizeof(*pages) * buf->nbufs,
GFP_KERNEL);
to:
pages = kmalloc(sizeof(*pages) * buf->nbufs,
GFP_KERNEL);

Re: [PATCH 3/4] infiniband: hns: add Hisilicon RoCE support(driver code)

[oulijun]

Hi Jiri Pirko, thanks for reviewing
On 2016/3/4 17:16, Jiri Pirko wrote:
> Fri, Mar 04, 2016 at 09:41:16AM CET, xavier.hu...@huawei.com wrote:
> 
> 
> 
>> +int hns_roce_buf_alloc(
>> +struct hns_roce_dev *hr_dev,
>> +int size, int max_direct,
>> +struct hns_roce_buf *buf)
> 
> 
>   
>> +
>> +pages =
>> +kmalloc(sizeof(*pages) * buf->nbufs,
>> +GFP_KERNEL);
> 
> 
> 
>> +
>> +buf->direct.buf = vmap(
>> +pages, buf->nbufs, VM_MAP,
>> +PAGE_KERNEL);
> 
> 
> 
>> +if (
>> +event_type != HNS_ROCE_EVENT_TYPE_CQ_ID_INVALID &&
>> +event_type != HNS_ROCE_EVENT_TYPE_CQ_ACCESS_ERROR &&
>> +event_type != HNS_ROCE_EVENT_TYPE_CQ_OVERFLOW) {
>> +dev_err(_dev->pdev->dev,
>> +"hns_roce_ib: Unexpected event type 0x%x on CQ %06x\n",
>> +event_type, hr_cq->cqn);
>> +return;
>> +}
> 
> Although checkpatch does not complain, I find this semi-random adding of
> newlines quite odd.
> 
Really, the question you mentioned exit in many location in currently 
patch. I done it
in order to make it complain checkpatch and linux norms. Now, I have checked 
and adjust it
properly combined to checkpatch
I will send a new patch in future. if not modified in some locations, it 
have to violate
checkpatch once modified and is unable to adjust it better. About these,  have 
you best strategy?

Thanks
Lijun Ou
> .
> 

Re: [PATCH 3/4] infiniband: hns: add Hisilicon RoCE support(driver code)

[oulijun]

Hi Jiri Pirko, thanks for reviewing
On 2016/3/4 17:16, Jiri Pirko wrote:
> Fri, Mar 04, 2016 at 09:41:16AM CET, xavier.hu...@huawei.com wrote:
> 
> 
> 
>> +int hns_roce_buf_alloc(
>> +struct hns_roce_dev *hr_dev,
>> +int size, int max_direct,
>> +struct hns_roce_buf *buf)
> 
> 
>   
>> +
>> +pages =
>> +kmalloc(sizeof(*pages) * buf->nbufs,
>> +GFP_KERNEL);
> 
> 
> 
>> +
>> +buf->direct.buf = vmap(
>> +pages, buf->nbufs, VM_MAP,
>> +PAGE_KERNEL);
> 
> 
> 
>> +if (
>> +event_type != HNS_ROCE_EVENT_TYPE_CQ_ID_INVALID &&
>> +event_type != HNS_ROCE_EVENT_TYPE_CQ_ACCESS_ERROR &&
>> +event_type != HNS_ROCE_EVENT_TYPE_CQ_OVERFLOW) {
>> +dev_err(_dev->pdev->dev,
>> +"hns_roce_ib: Unexpected event type 0x%x on CQ %06x\n",
>> +event_type, hr_cq->cqn);
>> +return;
>> +}
> 
> Although checkpatch does not complain, I find this semi-random adding of
> newlines quite odd.
> 
Really, the question you mentioned exit in many location in currently 
patch. I done it
in order to make it complain checkpatch and linux norms. Now, I have checked 
and adjust it
properly combined to checkpatch
I will send a new patch in future. if not modified in some locations, it 
have to violate
checkpatch once modified and is unable to adjust it better. About these,  have 
you best strategy?

Thanks
Lijun Ou
> .
> 

Re: [PATCH 3/4] infiniband: hns: add Hisilicon RoCE support(driver code)

[Parav Pandit]

On Fri, Mar 4, 2016 at 2:11 PM, Wei Hu(Xavier)  wrote:
> +
> +int hns_roce_register_device(struct hns_roce_dev *hr_dev)
> +{
> +   int ret;
> +   struct hns_roce_ib_iboe *iboe = NULL;
> +   struct ib_device *ib_dev = NULL;
> +   struct device *dev = _dev->pdev->dev;
> +
> +   iboe = _dev->iboe;
> +
> +   ib_dev = _dev->ib_dev;
> +   strlcpy(ib_dev->name, "hisi_%d", IB_DEVICE_NAME_MAX);
> +
> +   ib_dev->owner   = THIS_MODULE;
> +   ib_dev->node_type   = RDMA_NODE_IB_CA;
> +   ib_dev->dma_device  = dev;
> +
> +   ib_dev->phys_port_cnt   = hr_dev->caps.num_ports;
> +   ib_dev->local_dma_lkey  = hr_dev->caps.reserved_lkey;
> +   ib_dev->num_comp_vectors= hr_dev->caps.num_comp_vectors;
> +   ib_dev->uverbs_abi_ver  = 1;
> +   ib_dev->uverbs_cmd_mask =
> +   (1ULL << IB_USER_VERBS_CMD_GET_CONTEXT) |
> +   (1ULL << IB_USER_VERBS_CMD_QUERY_DEVICE) |
> +   (1ULL << IB_USER_VERBS_CMD_QUERY_PORT) |
> +   (1ULL << IB_USER_VERBS_CMD_ALLOC_PD) |
> +   (1ULL << IB_USER_VERBS_CMD_DEALLOC_PD) |
> +   (1ULL << IB_USER_VERBS_CMD_REG_MR) |
> +   (1ULL << IB_USER_VERBS_CMD_DEREG_MR) |
> +   (1ULL << IB_USER_VERBS_CMD_CREATE_COMP_CHANNEL) |
> +   (1ULL << IB_USER_VERBS_CMD_CREATE_CQ) |
> +   (1ULL << IB_USER_VERBS_CMD_DESTROY_CQ) |
> +   (1ULL << IB_USER_VERBS_CMD_CREATE_QP) |
> +   (1ULL << IB_USER_VERBS_CMD_MODIFY_QP) |
> +   (1ULL << IB_USER_VERBS_CMD_QUERY_QP) |
> +   (1ULL << IB_USER_VERBS_CMD_DESTROY_QP);
> +

Since SRQ is not supported in this driver version, can you keep
remaining code base also to not bother about SRQ specifically
poll_cq_one, modify_qp, destroy_qp etc?
SRQ support can come as complete additional patch along with cmd_mask,
callbacks and rest of the code.

Re: [PATCH 3/4] infiniband: hns: add Hisilicon RoCE support(driver code)

[Parav Pandit]

On Fri, Mar 4, 2016 at 2:11 PM, Wei Hu(Xavier)  wrote:
> +
> +int hns_roce_register_device(struct hns_roce_dev *hr_dev)
> +{
> +   int ret;
> +   struct hns_roce_ib_iboe *iboe = NULL;
> +   struct ib_device *ib_dev = NULL;
> +   struct device *dev = _dev->pdev->dev;
> +
> +   iboe = _dev->iboe;
> +
> +   ib_dev = _dev->ib_dev;
> +   strlcpy(ib_dev->name, "hisi_%d", IB_DEVICE_NAME_MAX);
> +
> +   ib_dev->owner   = THIS_MODULE;
> +   ib_dev->node_type   = RDMA_NODE_IB_CA;
> +   ib_dev->dma_device  = dev;
> +
> +   ib_dev->phys_port_cnt   = hr_dev->caps.num_ports;
> +   ib_dev->local_dma_lkey  = hr_dev->caps.reserved_lkey;
> +   ib_dev->num_comp_vectors= hr_dev->caps.num_comp_vectors;
> +   ib_dev->uverbs_abi_ver  = 1;
> +   ib_dev->uverbs_cmd_mask =
> +   (1ULL << IB_USER_VERBS_CMD_GET_CONTEXT) |
> +   (1ULL << IB_USER_VERBS_CMD_QUERY_DEVICE) |
> +   (1ULL << IB_USER_VERBS_CMD_QUERY_PORT) |
> +   (1ULL << IB_USER_VERBS_CMD_ALLOC_PD) |
> +   (1ULL << IB_USER_VERBS_CMD_DEALLOC_PD) |
> +   (1ULL << IB_USER_VERBS_CMD_REG_MR) |
> +   (1ULL << IB_USER_VERBS_CMD_DEREG_MR) |
> +   (1ULL << IB_USER_VERBS_CMD_CREATE_COMP_CHANNEL) |
> +   (1ULL << IB_USER_VERBS_CMD_CREATE_CQ) |
> +   (1ULL << IB_USER_VERBS_CMD_DESTROY_CQ) |
> +   (1ULL << IB_USER_VERBS_CMD_CREATE_QP) |
> +   (1ULL << IB_USER_VERBS_CMD_MODIFY_QP) |
> +   (1ULL << IB_USER_VERBS_CMD_QUERY_QP) |
> +   (1ULL << IB_USER_VERBS_CMD_DESTROY_QP);
> +

Since SRQ is not supported in this driver version, can you keep
remaining code base also to not bother about SRQ specifically
poll_cq_one, modify_qp, destroy_qp etc?
SRQ support can come as complete additional patch along with cmd_mask,
callbacks and rest of the code.

Re: [PATCH 3/4] infiniband: hns: add Hisilicon RoCE support(driver code)

[Jiri Pirko]

Fri, Mar 04, 2016 at 09:41:16AM CET, xavier.hu...@huawei.com wrote:



>+int hns_roce_buf_alloc(
>+  struct hns_roce_dev *hr_dev,
>+  int size, int max_direct,
>+  struct hns_roce_buf *buf)

  

>+
>+  pages =
>+  kmalloc(sizeof(*pages) * buf->nbufs,
>+  GFP_KERNEL);



>+
>+  buf->direct.buf = vmap(
>+  pages, buf->nbufs, VM_MAP,
>+  PAGE_KERNEL);



>+  if (
>+  event_type != HNS_ROCE_EVENT_TYPE_CQ_ID_INVALID &&
>+  event_type != HNS_ROCE_EVENT_TYPE_CQ_ACCESS_ERROR &&
>+  event_type != HNS_ROCE_EVENT_TYPE_CQ_OVERFLOW) {
>+  dev_err(_dev->pdev->dev,
>+  "hns_roce_ib: Unexpected event type 0x%x on CQ %06x\n",
>+  event_type, hr_cq->cqn);
>+  return;
>+  }

Although checkpatch does not complain, I find this semi-random adding of
newlines quite odd.

Re: [PATCH 3/4] infiniband: hns: add Hisilicon RoCE support(driver code)

[Jiri Pirko]

Fri, Mar 04, 2016 at 09:41:16AM CET, xavier.hu...@huawei.com wrote:



>+int hns_roce_buf_alloc(
>+  struct hns_roce_dev *hr_dev,
>+  int size, int max_direct,
>+  struct hns_roce_buf *buf)

  

>+
>+  pages =
>+  kmalloc(sizeof(*pages) * buf->nbufs,
>+  GFP_KERNEL);



>+
>+  buf->direct.buf = vmap(
>+  pages, buf->nbufs, VM_MAP,
>+  PAGE_KERNEL);



>+  if (
>+  event_type != HNS_ROCE_EVENT_TYPE_CQ_ID_INVALID &&
>+  event_type != HNS_ROCE_EVENT_TYPE_CQ_ACCESS_ERROR &&
>+  event_type != HNS_ROCE_EVENT_TYPE_CQ_OVERFLOW) {
>+  dev_err(_dev->pdev->dev,
>+  "hns_roce_ib: Unexpected event type 0x%x on CQ %06x\n",
>+  event_type, hr_cq->cqn);
>+  return;
>+  }

Although checkpatch does not complain, I find this semi-random adding of
newlines quite odd.