lkml.org 
[lkml]   [2016]   [Jun]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v10 06/22] IB/hns: Add initial cmd operation
From
Date


On 2016/6/21 19:28, Leon Romanovsky wrote:
> On Tue, Jun 21, 2016 at 06:50:51PM +0800, Wei Hu (Xavier) wrote:
>>
>> On 2016/6/20 21:33, Leon Romanovsky wrote:
>>> On Thu, Jun 16, 2016 at 10:35:14PM +0800, Lijun Ou wrote:
>>>> This patch added the operation for cmd, and added some functions
>>>> for initializing eq table and selecting cmd mode.
>>>>
>>>> Signed-off-by: Wei Hu <xavier.huwei@huawei.com>
>>>> Signed-off-by: Nenglong Zhao <zhaonenglong@hisilicon.com>
>>>> Signed-off-by: Lijun Ou <oulijun@huawei.com>
>>>> ---
>>>> PATCH v9/v8/v7/v6:
>>>> - No change over the PATCH v5
>>>>
>>>> PATCH v5:
>>>> - The initial patch which was redesigned based on the second patch
>>>> in PATCH v4
>>>> ---
>>> <...>
>>>
>>>> +#define CMD_MAX_NUM 32
>>>> +
>>>> +int hns_roce_cmd_init(struct hns_roce_dev *hr_dev)
>>>> +{
>>>> + struct device *dev = &hr_dev->pdev->dev;
>>>> +
>>>> + mutex_init(&hr_dev->cmd.hcr_mutex);
>>>> + sema_init(&hr_dev->cmd.poll_sem, 1);
>>>> + hr_dev->cmd.use_events = 0;
>>>> + hr_dev->cmd.toggle = 1;
>>>> + hr_dev->cmd.max_cmds = CMD_MAX_NUM;
>>> <...>
>>>
>>>> + for (hr_cmd->token_mask = 1; hr_cmd->token_mask < hr_cmd->max_cmds;
>>>> + hr_cmd->token_mask <<= 1)
>>>> + ;
>>>> + --hr_cmd->token_mask;
>>> It doesn't look that you dynamically change max_cmds supported.
>>> Why do you need to calculate token_mask dynamically?
>> Hi, Leon
>>
>> 1. The four lines above are in the function named
>> hns_roce_cmd_use_events.
>> and now this function is only called once in hns_roce_probe.
>> 2. In hns_roce_cmd_use_events,
>> we use these 4 lines to achieve the value of hr_cmd->token_mask
>> according to hr_cmd->max_cmds dynamically,
>> then we only define one marco for hr_cmd->max_cmds as below:
>>
>> #define CMD_MAX_NUM 32
>>
>> And it looks more flexible.
> It is called over engineering.
> I would recommend to you to remove it.
>
> We don't need over complicated code which is executed
> once with need to maintain with zero benefit.
>
> The other places need such simplification too.
Hi, Leon

We will modify this place as below:
In hns_roce_hw_v1.c(for hip06 soc) file:

void hns_roce_v1_profile(struct hns_roce_dev *hr_dev)
{
<snip>
caps->max_cmds = 32;
<snip>
}

In hns_roce_cmd.c file:

int hns_roce_cmd_init(struct hns_roce_dev *hr_dev)
{
<snip>
hr_dev->cmd.max_cmds = hr_dev->caps->max_cmds;
<snip>
}

Can you give more suggestions?


Regards
Wei Hu
>> Regards
>> Wei Hu
>>
>>
>>


\
 
 \ /
  Last update: 2016-06-21 15:41    [W:0.082 / U:1.212 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site