On Tue, Nov 14, 2017 at 05:26:15PM +0800, Yixian Liu wrote:
> This patch-set refactor eq code for hip06 and add eq
> support for hip08.
>
> Yixian Liu (2):
> RDMA/hns: Refactor eq code for hip06
> RDMA/hns: Add eq support of hip08
I read this this whole giant patch and will apply it to for-next.
I did notice some existing bothersome things:
1) Bad commenting around all memory barriers:
/* Memory barrier */
rmb();
Everyone knows that rmb/wmb/mb are memory barriers.
If you use these calls you *MUST* have a comment explaining
what the locking situation is. Explain where the *OTHER SIDE* of
the barrier is.
2) This construct
static inline void hns_roce_write64_k(__be32 val[2], void __iomem *dest)
{
__raw_writeq(*(u64 *) val, dest);
}
static void set_eq_cons_index_v2(struct hns_roce_eq *eq)
{
u32 doorbell[2];
[..]
hns_roce_write64_k(doorbell, eq->doorbell);
Is not OK. doorbell is technically not guarenteed to be 64 bit
aligned, and the deref *(u64 *) requires 64 bit alignment in the
kernel.
I purged this stupid array thing out of the mellanox drivers in
userspace some time ago, you might do the same, or minimually a
union is needed:
union doorbell {
u64 doorbell64;
u32 doorbell[2];
}
Plese consider sending followup patches.
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
