On 19/02/2020 12:53, Leon Romanovsky wrote:
> On Wed, Feb 19, 2020 at 12:21:10PM +0200, Kamal Heib wrote:
>> On Wed, Feb 19, 2020 at 11:33:21AM +0200, Leon Romanovsky wrote:
>>> On Wed, Feb 19, 2020 at 10:43:59AM +0200, Kamal Heib wrote:
>>>> On Tue, Feb 18, 2020 at 06:58:47PM +0200, Leon Romanovsky wrote:
>>>>> On Tue, Feb 18, 2020 at 11:59:11AM +0200, Kamal Heib wrote:
>>>>>> Make sure to set the active_{speed, width} attributes to avoid reporting
>>>>>> the same values regardless of the underlying device.
>>>>>>
>>>>>> Fixes: 303ae1cdfdf7 ("rdma/siw: application interface")
>>>>>> Signed-off-by: Kamal Heib <kamalheib1@xxxxxxxxx>
>>>>>> ---
>>>>>> V2: Change rc to rv.
>>>>>> ---
>>>>>> drivers/infiniband/sw/siw/siw_verbs.c | 7 ++++---
>>>>>> 1 file changed, 4 insertions(+), 3 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/infiniband/sw/siw/siw_verbs.c b/drivers/infiniband/sw/siw/siw_verbs.c
>>>>>> index 73485d0da907..d5390d498c61 100644
>>>>>> --- a/drivers/infiniband/sw/siw/siw_verbs.c
>>>>>> +++ b/drivers/infiniband/sw/siw/siw_verbs.c
>>>>>> @@ -165,11 +165,12 @@ int siw_query_port(struct ib_device *base_dev, u8 port,
>>>>>> struct ib_port_attr *attr)
>>>>>> {
>>>>>> struct siw_device *sdev = to_siw_dev(base_dev);
>>>>>> + int rv;
>>>>>>
>>>>>> memset(attr, 0, sizeof(*attr));
>>>>>
>>>>> This line should go too. IB/core clears attr prior to call driver.
>>>>>
>>>>> Thanks
>>>>>
>>>>
>>>> This can be done in a separate patch as this patch fixes a specific issue.
>>>
>>> Whatever works for you, if you don't value your own time, go for it,
>>> do separate patch for every line you are changing. It just looks crazy
>>> to see changes like this:
>>> * changed line
>>> * line to be changed, but not changed
>>> * another changed line
>>>
>>> Thanks
>>>
>>
>> Leon, With all my respect, This isn't your decision what I do and when.
>
> Please carefully reread my answer, I didn't say what and when you should
> do, simply gave to you an explanation why request remove useless memeset
> makes sense in the context of proposed patch.
I tend to agree with Kamal..
If the patch contained the memset removal I would argue it should be dropped as
it has nothing to do with the purpose of this commit. Just adds noise to the review.
