On 27-Mar-19 20:25, Jason Gunthorpe wrote:
> On Tue, Mar 26, 2019 at 06:32:32PM +0200, Leon Romanovsky wrote:
>> On Tue, Mar 26, 2019 at 05:41:15PM +0200, Gal Pressman wrote:
>>> On 26-Mar-19 17:10, Leon Romanovsky wrote:
>>>> On Tue, Mar 26, 2019 at 02:55:18PM +0200, Gal Pressman wrote:
>>>>> On 19-Mar-19 11:10, Leon Romanovsky wrote:
>>>>>> From: Leon Romanovsky <leonro@xxxxxxxxxxxx>
>>>>>>
>>>>>> port_pd is treated as le32 in declaration and read, fix assignment to be
>>>>>> in le32 too. This change fixes the following compilation warnings.
>>>>>>
>>>>>> drivers/infiniband/hw/hns/hns_roce_ah.c:67:24: warning: incorrect type
>>>>>> in assignment (different base types)
>>>>>> drivers/infiniband/hw/hns/hns_roce_ah.c:67:24: expected restricted __le32 [usertype] port_pd
>>>>>> drivers/infiniband/hw/hns/hns_roce_ah.c:67:24: got restricted __be32 [usertype]
>>>>>>
>>>>>> Fixes: 9a4435375cd1 ("IB/hns: Add driver files for hns RoCE driver")
>>>>>> Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxxxx>
>>>>>> drivers/infiniband/hw/hns/hns_roce_ah.c | 2 +-
>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/drivers/infiniband/hw/hns/hns_roce_ah.c b/drivers/infiniband/hw/hns/hns_roce_ah.c
>>>>>> index b3c8c45ec1e3..64e0c69b69c5 100644
>>>>>> +++ b/drivers/infiniband/hw/hns/hns_roce_ah.c
>>>>>> @@ -70,7 +70,7 @@ struct ib_ah *hns_roce_create_ah(struct ib_pd *ibpd,
>>>>>> HNS_ROCE_VLAN_SL_BIT_MASK) <<
>>>>>> HNS_ROCE_VLAN_SL_SHIFT;
>>>>>>
>>>>>> - ah->av.port_pd = cpu_to_be32(to_hr_pd(ibpd)->pdn |
>>>>>> + ah->av.port_pd = cpu_to_le32(to_hr_pd(ibpd)->pdn |
>>>>>> (rdma_ah_get_port_num(ah_attr) <<
>>>>>> HNS_ROCE_PORT_NUM_SHIFT));
>>>>>> ah->av.gid_index = grh->sgid_index;
>>>>>>
>>>>>
>>>>> The subject makes it sound like this is a cosmetic change (fix variable
>>>>> alignment), I would consider rephrasing it.
>>>>>
>>>>> Reviewed-by: Gal Pressman <galpress@xxxxxxxxxx>
>>>>
>>>> Thanks Gal,
>>>>
>>>> I had an impression that "alignment" is common term to describe it.
>>>> Any suggestions on how to rephrase it?
>>>>
>>>
>>> You might be right, but personally this change was not what I expected after
>>> reading the subject. It's a nit, feel free to ignore my comment :).
>>>
>>> Maybe "Fix wrong endianness conversion of port_pd" is better?
>
> Yes
>
>> I think so,
>>
>> Jason, do you want me to resend it?
>
> I want someone from hns to say that the changed swap is right.. I
> assume this code works as is ??
I was wondering that as well, my guess is no one actually uses the port_num
returned in query_ah flow. TBH, I don't really see a reason why port_pd should
even have an endianness annotation..
But hns guys might have a better answer.
