Re: [PATCH net-next 1/5] RDMA/cxgb4: Cleanup Filter related macros/register defines

2014-11-21 Thread Hariprasad S
On Thu, Nov 20, 2014 at 23:55:54 -0800, Joe Perches wrote:
> On Fri, 2014-11-21 at 12:52 +0530, Hariprasad Shenai wrote:
> > This patch cleanups all filter related macros/register defines that are 
> > defined
> > in t4fw_api.h and the affected files.
> 
> Is there any real value in the FW_FILTER_WR_ prefix?
> Does it need to be so long?
> 
Hey Joe, we have about 600 of these register defines used internally, and more
added as we add functionality and only a small chunk of these are actually used
in the code in kernel. Without the descriptive prefixes code would be extremely
hard to read without having a reference document open at all times. They are
used in 6 different drivers, and no one is proficient enough to know them all
off hand, hence the explanatory prefixes.

> Perhaps it'd be nicer to read if
> _S was _SHIFT
> _M was _MASK
> _V was whatever it's supposed to represent (_SET?)
> and
> _G was _GET
> 
There prefixes are auto-generated based on hardware logic, as the actual
prefixes generated are not acceptable in the kernel due namespace collison
worries, we are trying to keep it in an format that is _both_ acceptable in the
kernel and can be used to convert in-house patches to upstream kernel patches.
Changing the length of the macro names would significantly increase manual
intervention required to ensure that the 80 char limit is not violated. Even
simply changing existing code would require quite a lot of man-hours manually
fixing all the char length issues that would be better spend making code more
maintainable first.

The current focus of the effort is to get all these macros into a uniform
format. Due to legacy reasons, the current macros are represented in 3 varying
formats (one of which actually is _SHIFT, _MASK etc), making code very hard to
maintain, doubly so when transplanting changesets. As a final phase of this, we
will add an explanation about the suffixes used so that the headers are easier
to read.

> > +#define FW_FILTER_WR_TID_S  12
> > +#define FW_FILTER_WR_TID_M  0xf
> > +#define FW_FILTER_WR_TID_V(x)   ((x) << FW_FILTER_WR_TID_S)
> > +#define FW_FILTER_WR_TID_G(x)   \
> > +   (((x) >> FW_FILTER_WR_TID_S) & FW_FILTER_WR_TID_M)
> 
> Why aren't the _V defines masked then shifted?
> 
> #define FW_FILTER_WR__V(x)   \
>   (((x) & FW_FILTER__M) << FW_FILTER__S)
> 
> 
Good point, until now we just have been careful using _V macros. But this too
is best done once we actually have all the macros used to set values in a
uniform format that can be patched in one go.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: mlx5: don't duplicate kvfree()

2014-11-21 Thread David Miller
From: Al Viro 
Date: Thu, 20 Nov 2014 08:13:57 +

> Signed-off-by: Al Viro 

Applied.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH] iw_cxgb4: Wake up waiters after flushing the qp

2014-11-21 Thread Steve Wise
I've pushed these to:

git://git.openfabrics.org/~swise/linux.git for_roland

Steve

Hariprasad Shenai (2):
  iw_cxgb4: Fixes locking issue in process_mpa_request
  iw_cxgb4: limit MRs to < 8GB for T4/T5 devices

Pramod Kumar (2):
  cxgb4/rdma:Increase epd buff size for debug interface
  cxgb4/rdma: configure 0B MRs to match HW implementation

Steve Wise (1):
  iw_cxgb4: Wake up waiters after flushing the qp


> -Original Message-
> From: Steve Wise [mailto:sw...@opengridcomputing.com]
> Sent: Friday, November 21, 2014 9:26 AM
> To: rol...@purestorage.com
> Cc: linux-rdma@vger.kernel.org
> Subject: Re: [PATCH] iw_cxgb4: Wake up waiters after flushing the qp
> 
> Hey Roland,
> 
> Hope you can get this bug fix into 3.18.  Plus these others that have
> been submitted a while ago.  Here is the entire list:
> 
> https://patchwork.kernel.org/patch/5106541/
> https://patchwork.kernel.org/patch/5249511/
> https://patchwork.kernel.org/patch/5249501/
> https://patchwork.kernel.org/patch/5089511/
> https://patchwork.kernel.org/patch/5089501/
> 
> 
> Thanks,
> 
> Steve.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] iw_cxgb4: Wake up waiters after flushing the qp

2014-11-21 Thread Steve Wise

Hey Roland,

Hope you can get this bug fix into 3.18.  Plus these others that have 
been submitted a while ago.  Here is the entire list:


https://patchwork.kernel.org/patch/5106541/
https://patchwork.kernel.org/patch/5249511/
https://patchwork.kernel.org/patch/5249501/
https://patchwork.kernel.org/patch/5089511/
https://patchwork.kernel.org/patch/5089501/


Thanks,

Steve.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH net-next 1/5] RDMA/cxgb4: Cleanup Filter related macros/register defines

2014-11-21 Thread Joe Perches
On Fri, 2014-11-21 at 12:52 +0530, Hariprasad Shenai wrote:
> This patch cleanups all filter related macros/register defines that are 
> defined
> in t4fw_api.h and the affected files.

Is there any real value in the FW_FILTER_WR_ prefix?
Does it need to be so long?

Perhaps it'd be nicer to read if
_S was _SHIFT
_M was _MASK
_V was whatever it's supposed to represent (_SET?)
and
_G was _GET

> +#define FW_FILTER_WR_TID_S  12
> +#define FW_FILTER_WR_TID_M  0xf
> +#define FW_FILTER_WR_TID_V(x)   ((x) << FW_FILTER_WR_TID_S)
> +#define FW_FILTER_WR_TID_G(x)   \
> + (((x) >> FW_FILTER_WR_TID_S) & FW_FILTER_WR_TID_M)

Why aren't the _V defines masked then shifted?

#define FW_FILTER_WR__V(x) \
(((x) & FW_FILTER__M) << FW_FILTER__S)


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html