Wed, Mar 09, 2016 at 12:18:06PM CET, [email protected] 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, [email protected] wrote:
>>
>> <snip>
>>
>>> +int hns_roce_buf_alloc(
>>> + struct hns_roce_dev *hr_dev,
>>> + int size, int max_direct,
>>> + struct hns_roce_buf *buf)
>>
>> <snip>
>>
>>> +
>>> + pages =
>>> + kmalloc(sizeof(*pages) * buf->nbufs,
>>> + GFP_KERNEL);
>>
>> <snip>
>>
>>> +
>>> + buf->direct.buf = vmap(
>>> + pages, buf->nbufs, VM_MAP,
>>> + PAGE_KERNEL);
>>
>> <snip>
>>
>>> + 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(&hr_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);