On Tue, 2018-01-30 at 20:20 +0800, Lijun Ou wrote:
> The hip06 and hip08 run on a little endian ARM, it needs to
> revise the annotations to indicate that the HW uses little
> endian data in the various DMA buffers, and flow the necessary
> swaps throughout.
>
> The imm_data use big endian mode. The cpu_to_le32/le32_to_cpu
> swaps are no-op for this, which makes the only substantive
> change the handling of imm_data which is now mandatory swapped.
>
> This also keep match with the userspace hns driver and resolve
> the warning by sparse.
>
> Signed-off-by: Lijun Ou <oulijun@xxxxxxxxxx>
> ---
> drivers/infiniband/hw/hns/hns_roce_common.h | 6 +-
> drivers/infiniband/hw/hns/hns_roce_device.h | 2 +-
> drivers/infiniband/hw/hns/hns_roce_hw_v1.c | 57 ++++--
> drivers/infiniband/hw/hns/hns_roce_hw_v1.h | 258 ++++++++++++-------------
> drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 51 +++--
> drivers/infiniband/hw/hns/hns_roce_hw_v2.h | 283 ++++++++++++++--------------
> drivers/infiniband/hw/hns/hns_roce_main.c | 2 +-
> drivers/infiniband/hw/hns/hns_roce_qp.c | 18 +-
> 8 files changed, 357 insertions(+), 320 deletions(-)
>
> diff --git a/drivers/infiniband/hw/hns/hns_roce_common.h b/drivers/infiniband/hw/hns/hns_roce_common.h
> index dd67faf..319cb74 100644
> --- a/drivers/infiniband/hw/hns/hns_roce_common.h
> +++ b/drivers/infiniband/hw/hns/hns_roce_common.h
> @@ -43,15 +43,15 @@
> __raw_writel((__force u32)cpu_to_le32(value), (addr))
>
> #define roce_get_field(origin, mask, shift) \
> - (((origin) & (mask)) >> (shift))
> + (((le32_to_cpu(origin)) & (mask)) >> (shift))
>
> #define roce_get_bit(origin, shift) \
> roce_get_field((origin), (1ul << (shift)), (shift))
>
> #define roce_set_field(origin, mask, shift, val) \
> do { \
> - (origin) &= (~(mask)); \
> - (origin) |= (((u32)(val) << (shift)) & (mask)); \
> + (origin) &= ~cpu_to_le32(mask); \
> + (origin) |= cpu_to_le32(((u32)(val) << (shift)) & (mask)); \
> } while (0)
>
> #define roce_set_bit(origin, shift, val) \
> diff --git a/drivers/infiniband/hw/hns/hns_roce_device.h b/drivers/infiniband/hw/hns/hns_roce_device.h
> index 42c3b5a..2503d7f 100644
> --- a/drivers/infiniband/hw/hns/hns_roce_device.h
> +++ b/drivers/infiniband/hw/hns/hns_roce_device.h
> @@ -466,7 +466,7 @@ struct hns_roce_qp {
> struct ib_qp ibqp;
> struct hns_roce_buf hr_buf;
> struct hns_roce_wq rq;
> - __le64 doorbell_qpn;
> + u32 doorbell_qpn;
Why the change in size here? Did you mean to go from 64bits down to
32bits?
>
> } else if (ibqp->qp_type == IB_QPT_RC) {
> ctrl = wqe;
> memset(ctrl, 0, sizeof(struct hns_roce_wqe_ctrl_seg));
> for (i = 0; i < wr->num_sge; i++)
> - ctrl->msg_length += wr->sg_list[i].length;
> + ctrl->msg_length =
> + cpu_to_le32(le32_to_cpu(ctrl->msg_length) +
> + wr->sg_list[i].length);
Minor nit:
Doing le32_to_cpu and cpu_to_le32 over and over again in a loop is
horribly inefficient. It would be much better IMO if you had a local
variable to use for the length, used that in the loop, and then only at
the end of the loop do a single cpu_to_le32 of the local variable to
store in msg_length. Same comment applies to the other spot in this
patch that does the same loop.
--
Doug Ledford <dledford@xxxxxxxxxx>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDDAttachment:
signature.asc
Description: This is a digitally signed message part
