Re: [PATCH][next] RDMA/hns: remove duplicate assignment to pointer raq

[oulijun]

在 2020/5/28 23:04, Colin King 写道:

From: Colin Ian King 

The pointer raq is being assigned twice. Fix this by removing
one of the redundant assignments.

Fixes: 14ba87304bf9 ("RDMA/hns: Remove redundant type cast for general 
pointers")
Addressses-Coverity: ("Evaluation order violation")
Signed-off-by: Colin Ian King 
---
  drivers/infiniband/hw/hns/hns_roce_hw_v1.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c 
b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
index 8ff6b922b4d7..d02207cd30df 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
@@ -1146,7 +1146,7 @@ static void hns_roce_db_free(struct hns_roce_dev *hr_dev)
  static int hns_roce_raq_init(struct hns_roce_dev *hr_dev)
  {
struct hns_roce_v1_priv *priv = hr_dev->priv;
-   struct hns_roce_raq_table *raq = raq = >raq_table;
+   struct hns_roce_raq_table *raq = >raq_table;
struct device *dev = _dev->pdev->dev;
int raq_shift = 0;
dma_addr_t addr;


thanks

Re: [PATCH 1/2][next] RDMA/hns: fix comparison of unsigned long variable 'end' with less than zero

[oulijun]

在 2019/5/31 17:21, Colin King 写道:
> From: Colin Ian King 
>
> Currently the comparison of end with less than zero is always false
> because end is an unsigned long.  Also, replace checks of end with
> non-zero with end > 0 as it is possible that the #defined decrement
> may be changed in the future causing end to step over zero and go
> negative.
>
> The initialization of end with 0 is also redundant as this value is
> never read and is later set to HW_SYNC_TIMEOUT_MSECS, so fix this by
> initializing it with this value to begin with.
>
> Addresses-Coverity: ("Unsigned compared against 0")
> Fixes: 669cefb654cb ("RDMA/hns: Remove jiffies operation in disable interrupt 
> context")
> Signed-off-by: Colin Ian King 
> ---
>  drivers/infiniband/hw/hns/hns_roce_hem.c   |  4 ++--
>  drivers/infiniband/hw/hns/hns_roce_hw_v1.c | 12 ++--
>  2 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/infiniband/hw/hns/hns_roce_hem.c 
> b/drivers/infiniband/hw/hns/hns_roce_hem.c
> index 157c84a1f55f..b3641aeff27a 100644
> --- a/drivers/infiniband/hw/hns/hns_roce_hem.c
> +++ b/drivers/infiniband/hw/hns/hns_roce_hem.c
> @@ -326,7 +326,7 @@ static int hns_roce_set_hem(struct hns_roce_dev *hr_dev,
>  {
>   spinlock_t *lock = _dev->bt_cmd_lock;
>   struct device *dev = hr_dev->dev;
> - unsigned long end = 0;
> + long end;
>   unsigned long flags;
>   struct hns_roce_hem_iter iter;
>   void __iomem *bt_cmd;
> @@ -377,7 +377,7 @@ static int hns_roce_set_hem(struct hns_roce_dev *hr_dev,
>   bt_cmd = hr_dev->reg_base + ROCEE_BT_CMD_H_REG;
>  
>   end = HW_SYNC_TIMEOUT_MSECS;
> - while (end) {
> + while (end > 0) {
>   if (!readl(bt_cmd) >> BT_CMD_SYNC_SHIFT)
>   break;
>  
> diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c 
> b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> index 1fc77b1f2c6d..e13fea71bcb8 100644
> --- a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> @@ -966,7 +966,7 @@ static int hns_roce_v1_recreate_lp_qp(struct hns_roce_dev 
> *hr_dev)
>   struct hns_roce_free_mr *free_mr;
>   struct hns_roce_v1_priv *priv;
>   struct completion comp;
> - unsigned long end = HNS_ROCE_V1_RECREATE_LP_QP_TIMEOUT_MSECS;
> + long end = HNS_ROCE_V1_RECREATE_LP_QP_TIMEOUT_MSECS;
>  
>   priv = (struct hns_roce_v1_priv *)hr_dev->priv;
>   free_mr = >free_mr;
> @@ -986,7 +986,7 @@ static int hns_roce_v1_recreate_lp_qp(struct hns_roce_dev 
> *hr_dev)
>  
>   queue_work(free_mr->free_mr_wq, &(lp_qp_work->work));
>  
> - while (end) {
> + while (end > 0) {
>   if (try_wait_for_completion())
>   return 0;
>   msleep(HNS_ROCE_V1_RECREATE_LP_QP_WAIT_VALUE);
> @@ -1104,7 +1104,7 @@ static int hns_roce_v1_dereg_mr(struct hns_roce_dev 
> *hr_dev,
>   struct hns_roce_free_mr *free_mr;
>   struct hns_roce_v1_priv *priv;
>   struct completion comp;
> - unsigned long end = HNS_ROCE_V1_FREE_MR_TIMEOUT_MSECS;
> + long end = HNS_ROCE_V1_FREE_MR_TIMEOUT_MSECS;
>   unsigned long start = jiffies;
>   int npages;
>   int ret = 0;
> @@ -1134,7 +1134,7 @@ static int hns_roce_v1_dereg_mr(struct hns_roce_dev 
> *hr_dev,
>  
>   queue_work(free_mr->free_mr_wq, &(mr_work->work));
>  
> - while (end) {
> + while (end > 0) {
>   if (try_wait_for_completion())
>   goto free_mr;
>   msleep(HNS_ROCE_V1_FREE_MR_WAIT_VALUE);
> @@ -2425,7 +2425,8 @@ static int hns_roce_v1_clear_hem(struct hns_roce_dev 
> *hr_dev,
>  {
>   struct device *dev = _dev->pdev->dev;
>   struct hns_roce_v1_priv *priv;
> - unsigned long end = 0, flags = 0;
> + unsigned long flags = 0;
> + long end = HW_SYNC_TIMEOUT_MSECS;
>   __le32 bt_cmd_val[2] = {0};
>   void __iomem *bt_cmd;
>   u64 bt_ba = 0;
> @@ -2463,7 +2464,6 @@ static int hns_roce_v1_clear_hem(struct hns_roce_dev 
> *hr_dev,
>  
>   bt_cmd = hr_dev->reg_base + ROCEE_BT_CMD_H_REG;
>  
> - end = HW_SYNC_TIMEOUT_MSECS;
>   while (1) {
>   if (readl(bt_cmd) >> BT_CMD_SYNC_SHIFT) {
>   if (end < 0) {

Good, thanks.

Re: linux-next: build failure after merge of the rdma tree

[oulijun]

在 2019/7/4 10:04, Jason Gunthorpe 写道:
> On Thu, Jul 04, 2019 at 12:02:35PM +1000, Stephen Rothwell wrote:
>> Hi all,
>>
>> On Mon, 1 Jul 2019 14:14:31 +1000 Stephen Rothwell  
>> wrote:
>>> Hi all,
>>>
>>> After merging the rdma tree, today's linux-next build (x86_64
>>> allmodconfig) failed like this:
>>>
>>> WARNING: modpost: missing MODULE_LICENSE() in 
>>> drivers/infiniband/hw/hns/hns_roce_ah.o
>>> see include/linux/module.h for more information
>>  .
>>  .
>>  .
>>> ERROR: "hns_roce_bitmap_cleanup" 
>>> [drivers/infiniband/hw/hns/hns_roce_srq.ko] undefined!
>>  .
>>  .
>>  .
>>> ERROR: "hns_roce_ib_destroy_cq" 
>>> [drivers/infiniband/hw/hns/hns-roce-hw-v1.ko] undefined!
>>>
>>> Presumably caused by commit
>>>
>>>   e9816ddf2a33 ("RDMA/hns: Cleanup unnecessary exported symbols")
>>>
>>> I have used the rdma tree from next-20190628 for today.
>> I am still getting these errors/warnings.
> I have not got a fixing patch from HNS team.
>
> At this late date I will revert the problematic HNS patch tomorrow.
>
> Jason
Hi, Jason
   We have sent a fixup patch.  This problem only appears when compiled into 
ko. Our self-test is build in.

Thanks.
Lijun Ou
> .
>

Re: linux-next: build failure after merge of the rdma tree

[oulijun]

在 2019/7/4 10:04, Jason Gunthorpe 写道:
> On Thu, Jul 04, 2019 at 12:02:35PM +1000, Stephen Rothwell wrote:
>> Hi all,
>>
>> On Mon, 1 Jul 2019 14:14:31 +1000 Stephen Rothwell  
>> wrote:
>>> Hi all,
>>>
>>> After merging the rdma tree, today's linux-next build (x86_64
>>> allmodconfig) failed like this:
>>>
>>> WARNING: modpost: missing MODULE_LICENSE() in 
>>> drivers/infiniband/hw/hns/hns_roce_ah.o
>>> see include/linux/module.h for more information
>>  .
>>  .
>>  .
>>> ERROR: "hns_roce_bitmap_cleanup" 
>>> [drivers/infiniband/hw/hns/hns_roce_srq.ko] undefined!
>>  .
>>  .
>>  .
>>> ERROR: "hns_roce_ib_destroy_cq" 
>>> [drivers/infiniband/hw/hns/hns-roce-hw-v1.ko] undefined!
>>>
>>> Presumably caused by commit
>>>
>>>   e9816ddf2a33 ("RDMA/hns: Cleanup unnecessary exported symbols")
>>>
>>> I have used the rdma tree from next-20190628 for today.
>> I am still getting these errors/warnings.
> I have not got a fixing patch from HNS team.
>
> At this late date I will revert the problematic HNS patch tomorrow.
>
> Jason
Hi， Jason
  Sorry,  our guys may not see your mail in time. I will fix it and send a 
email in today.

Thanks
Lijun Ou
> .
>

【Application】hns RoCE driver integration

[oulijun]

Hi, Arlin Davis & Jason

I am the maintainer of hns. I understand that hns is still not integrated 
into OFED. A long time ago I received a consultation email about hns 
integration into OFED.

Sorry for the long time of reply.

   I am trying to push the support for the hns RoCE driver in the next version 
of OFED. If we want to land as soon as possible, how do we need to cooperate?

Thanks

Lijun Ou

Re: [PATCH] IB: make INFINIBAND_ADDR_TRANS configurable

[oulijun]

在 2018/4/13 20:47, Bart Van Assche 写道:
> On Fri, 2018-04-13 at 00:06 -0700, Greg Thelen wrote:
>> Allow INFINIBAND without INFINIBAND_ADDR_TRANS.
>>
>> Signed-off-by: Greg Thelen 
>> Cc: Tarick Bedeir 
>> ---
>>  drivers/infiniband/Kconfig | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/infiniband/Kconfig b/drivers/infiniband/Kconfig
>> index ee270e065ba9..f20a3977087c 100644
>> --- a/drivers/infiniband/Kconfig
>> +++ b/drivers/infiniband/Kconfig
>> @@ -61,7 +61,7 @@ config INFINIBAND_ON_DEMAND_PAGING
>>pages on demand instead.
>>  
>>  config INFINIBAND_ADDR_TRANS
>> -bool
>> +bool "InfiniBand address translation"
>>  depends on INFINIBAND
>>  default y
> 
> Hello Greg,
> 
> Please change the short description from "InfiniBand address translation" 
> (which
> is a misleading description) into "RDMA/CM". Please also add a help text that
> explains to people who are not RDMA experts what the RDMA/CM is.
> 
> Thanks,
> 
> Bart.
> 
Hi, Bart
  We need to select the INFINIBAND without INFINIBAND_ADDR_TRANS if use cm for 
others?

Thanks
Lijun Ou
> 
> 
> �{.n�+���+%��lzwm��b�맲��r��zX��ݙ�����ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z��w���?&�)ߢf
> 

Re: [PATCH] IB: make INFINIBAND_ADDR_TRANS configurable

[oulijun]

在 2018/4/13 20:47, Bart Van Assche 写道:
> On Fri, 2018-04-13 at 00:06 -0700, Greg Thelen wrote:
>> Allow INFINIBAND without INFINIBAND_ADDR_TRANS.
>>
>> Signed-off-by: Greg Thelen 
>> Cc: Tarick Bedeir 
>> ---
>>  drivers/infiniband/Kconfig | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/infiniband/Kconfig b/drivers/infiniband/Kconfig
>> index ee270e065ba9..f20a3977087c 100644
>> --- a/drivers/infiniband/Kconfig
>> +++ b/drivers/infiniband/Kconfig
>> @@ -61,7 +61,7 @@ config INFINIBAND_ON_DEMAND_PAGING
>>pages on demand instead.
>>  
>>  config INFINIBAND_ADDR_TRANS
>> -bool
>> +bool "InfiniBand address translation"
>>  depends on INFINIBAND
>>  default y
> 
> Hello Greg,
> 
> Please change the short description from "InfiniBand address translation" 
> (which
> is a misleading description) into "RDMA/CM". Please also add a help text that
> explains to people who are not RDMA experts what the RDMA/CM is.
> 
> Thanks,
> 
> Bart.
> 
Hi, Bart
  We need to select the INFINIBAND without INFINIBAND_ADDR_TRANS if use cm for 
others?

Thanks
Lijun Ou
> 
> 
> �{.n�+���+%��lzwm��b�맲��r��zX��ݙ�����ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z��w���?&�)ߢf
> 

Re: 【build error report for for next branch】

[oulijun]

在 2017/10/23 23:24, Doug Ledford 写道:
> On Wed, 2017-10-18 at 14:21 +0800, oulijun wrote:
>> Hi, Doug Ledford
>>I am use the for-next branch for building and the result is fail.
>> Is the branch a bug or my buidl way incorrectly?
> 
> [ snip ]
> 
>>   CC [M]  drivers/gpu/drm/tegra/trace.o
>> In file included from drivers/gpu/drm/tegra/trace.h:68:0,
>>  from drivers/gpu/drm/tegra/trace.c:2:
>> ./include/trace/define_trace.h:88:43: fatal error: ./trace.h: No such
>> file or directory
>>  #include TRACE_INCLUDE(TRACE_INCLUDE_FILE)
>>^
>> compilation terminated.
>> make[4]: *** [drivers/gpu/drm/tegra/trace.o] Error 1
>> make[3]: *** [drivers/gpu/drm/tegra] Error 2
>> make[3]: *** Waiting for unfinished jobs
>> make[2]: *** [drivers/gpu/drm] Error 2
>> make[1]: *** [drivers/gpu] Error 2
>> make: *** [drivers] Error 2
> 
> You've probably got a bad .config for your cross build.  I doubt it's
> related to my tree since I have done any patches to the tracing
> subsytem and that's what's failing.
> 
Thanks. I am willing to confirm again.

Re: 【build error report for for next branch】

[oulijun]

在 2017/10/23 23:24, Doug Ledford 写道:
> On Wed, 2017-10-18 at 14:21 +0800, oulijun wrote:
>> Hi, Doug Ledford
>>I am use the for-next branch for building and the result is fail.
>> Is the branch a bug or my buidl way incorrectly?
> 
> [ snip ]
> 
>>   CC [M]  drivers/gpu/drm/tegra/trace.o
>> In file included from drivers/gpu/drm/tegra/trace.h:68:0,
>>  from drivers/gpu/drm/tegra/trace.c:2:
>> ./include/trace/define_trace.h:88:43: fatal error: ./trace.h: No such
>> file or directory
>>  #include TRACE_INCLUDE(TRACE_INCLUDE_FILE)
>>^
>> compilation terminated.
>> make[4]: *** [drivers/gpu/drm/tegra/trace.o] Error 1
>> make[3]: *** [drivers/gpu/drm/tegra] Error 2
>> make[3]: *** Waiting for unfinished jobs
>> make[2]: *** [drivers/gpu/drm] Error 2
>> make[1]: *** [drivers/gpu] Error 2
>> make: *** [drivers] Error 2
> 
> You've probably got a bad .config for your cross build.  I doubt it's
> related to my tree since I have done any patches to the tracing
> subsytem and that's what's failing.
> 
Thanks. I am willing to confirm again.

Re: [patch 1/2] IB/hns: Fix a couple pointer math bugs

[oulijun]

在 2016/10/14 15:28, Dan Carpenter 写道:
> "wqe" is a void pointer so adding sizeof() works.  The original code
> adds sizeof() multiplied by sizeof() so it doesn't work at all.
> 
> Fixes: 9a4435375cd1 ('IB/hns: Add driver files for hns RoCE driver')
> Signed-off-by: Dan Carpenter 
> 
> diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c 
> b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> index 399f5de..58b150e 100644
> --- a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> @@ -205,8 +205,7 @@ int hns_roce_v1_post_send(struct ib_qp *ibqp, struct 
> ib_send_wr *wr,
> (wr->send_flags & IB_SEND_FENCE ?
> (cpu_to_le32(HNS_ROCE_WQE_FENCE)) : 0);
>  
> - wqe = (struct hns_roce_wqe_ctrl_seg *)wqe +
> -sizeof(struct hns_roce_wqe_ctrl_seg);
> + wqe = wqe + sizeof(struct hns_roce_wqe_ctrl_seg);
>  
>   switch (wr->opcode) {
>   case IB_WR_RDMA_READ:
> @@ -235,8 +234,7 @@ int hns_roce_v1_post_send(struct ib_qp *ibqp, struct 
> ib_send_wr *wr,
>   break;
>   }
>   ctrl->flag |= cpu_to_le32(ps_opcode);
> - wqe = (struct hns_roce_wqe_raddr_seg *)wqe +
> -sizeof(struct hns_roce_wqe_raddr_seg);
> + wqe = wqe + sizeof(struct hns_roce_wqe_raddr_seg);
>  
>   dseg = wqe;
>   if (wr->send_flags & IB_SEND_INLINE && wr->num_sge) {
> 
> .
> 
Hi, Dan Carpenter
  firstly, thanks your reviewing. This quesiton is checked while i develop and 
test the CM function, and i have fixed it in a patch. the patch
is as follows:
  https://patchwork.kernel.org/patch/9334859/

  the patch is reviewing by community experts.
  thanks your reviewing again.

Thanks
Lijun Ou

Re: [patch 1/2] IB/hns: Fix a couple pointer math bugs

[oulijun]

在 2016/10/14 15:28, Dan Carpenter 写道:
> "wqe" is a void pointer so adding sizeof() works.  The original code
> adds sizeof() multiplied by sizeof() so it doesn't work at all.
> 
> Fixes: 9a4435375cd1 ('IB/hns: Add driver files for hns RoCE driver')
> Signed-off-by: Dan Carpenter 
> 
> diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c 
> b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> index 399f5de..58b150e 100644
> --- a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> @@ -205,8 +205,7 @@ int hns_roce_v1_post_send(struct ib_qp *ibqp, struct 
> ib_send_wr *wr,
> (wr->send_flags & IB_SEND_FENCE ?
> (cpu_to_le32(HNS_ROCE_WQE_FENCE)) : 0);
>  
> - wqe = (struct hns_roce_wqe_ctrl_seg *)wqe +
> -sizeof(struct hns_roce_wqe_ctrl_seg);
> + wqe = wqe + sizeof(struct hns_roce_wqe_ctrl_seg);
>  
>   switch (wr->opcode) {
>   case IB_WR_RDMA_READ:
> @@ -235,8 +234,7 @@ int hns_roce_v1_post_send(struct ib_qp *ibqp, struct 
> ib_send_wr *wr,
>   break;
>   }
>   ctrl->flag |= cpu_to_le32(ps_opcode);
> - wqe = (struct hns_roce_wqe_raddr_seg *)wqe +
> -sizeof(struct hns_roce_wqe_raddr_seg);
> + wqe = wqe + sizeof(struct hns_roce_wqe_raddr_seg);
>  
>   dseg = wqe;
>   if (wr->send_flags & IB_SEND_INLINE && wr->num_sge) {
> 
> .
> 
Hi, Dan Carpenter
  firstly, thanks your reviewing. This quesiton is checked while i develop and 
test the CM function, and i have fixed it in a patch. the patch
is as follows:
  https://patchwork.kernel.org/patch/9334859/

  the patch is reviewing by community experts.
  thanks your reviewing again.

Thanks
Lijun Ou

Re: [PATCH v11 00/22] Add HiSilicon RoCE driver

[oulijun]

在 2016/7/2 17:39, Lijun Ou 写道:
> The HiSilicon Network Substem is a long term evolution IP which is
> supposed to be used in HiSilicon ICT SoCs. HNS (HiSilicon Network
> Sybsystem) also has a hardware support of performing RDMA with
> RoCEE.
> The driver for HiSilicon RoCEE(RoCE Engine) is a platform driver and
> will support mulitple versions of SOCs in future. This version of driver
> is meant to support Hip06 SoC(which confirms to RoCEv1 hardware
> specifications).
> 
> Changes v10 -> v11:
> [1/22]:
> 1. modify the print description of chip don't support roce
> 2. remove explicit values for enums for patch series
> [3/22]:
> 3. remove non-essential headers for patch series
> 4. add judgement for port_cnt is zero
> 5. Keep unified print style for "set mask..." vs. "No usable
>..."
> 6. modify the MODULE_LICENSE
> 7. remove MODULE_ALIAS
> [4/22]:
> 8. Move this line out of if-else and leave "if (enable)" part only
> 9. renaming the meaningful definition to 20 for patch series
> 10. delete extern keyword for hns_dsaf_roce_reset function
> 11. delete void keyword for hr_dev->hw->reset when driver removed
> [5/22]:
> 12. remove few unnecessary variables and some lines.
> 13. remove the function for one line of code which will be called
> once only for patch series
> [6/22]:
> 14. redesign the method for calculating token_mask' value
> [7/22]:
> 15. delete hns_roce_status_to_errno
> 16. modify the one enum only for all patches
> 17. remove the spin_lock in hns_roce_cq_event function
> 18. add comment here that 0x10 and 0x11 in hns_roce_event struct
> 19. refactor hns_roce_aeq_int function and It has switch in switch
> and it is almost 200 LOCs
> 20. simplify the lines for err_out_free_pages branch
> [8/22]:
> 21. remove icm and redesign it for patch series
> 
> Changes v9 -> v10:
> 1. delete redundant lines which it is netdevice.h in hns_roce_main.c
> 2. adjust the indentation for HNS_ROCE_V1_NUM_ASYNC_EQE
> 3. simplify the lines in hns_roce_init_qp_table function
> 4. add static type for hns_roce_unregister_device
> 5. move the call with hns_roce_unregister_device from the tenth patch to
>the eleventh patch in hns_roce_remove function
> 6. readjuest the alphabetic order in MAINTAINERS
> 7. redesigned the way for getting irq names
> 8. avoid the memory leakage because mr->pbl is not free in
>hns_roce_mr function
> 9. avoid the memory leakage because not kfree table->icm when exception
> 10. add the link from LKML as well whose comment in all
> 
> Changes v8 -> v9:
> 1. delete the definition of ADDR_SHIFT_n, use literal 12, 32 and 44 and
>add comments
> 2. use roce_read/roce_write/readl/write instead of roce_readl/roce_writel
> 3. delete the print error/debug messages for memory allocation errors
> 4. use exit instead of uninit, for example hw->uninit -> hw->exit
> 5. use roce_raw_write instead of _raw_writel in eq_set_cons_index
> 6. modify the label with underscore
> 7. adjust the indentation for the macro definitions in hns_roce_hw_v1.c
> 8. simplify some lines in few functions and structures
> 9. adjust the alphabetic order in MAINTAINERS
> 
> Changes v7 -> v8:
> 1. add a verbs operation named get_port_immutable. It is an 
>independent patch
> 2. add a comment for the definition of ADDR_SHIFT_n, n are 12,32
>and 44
> 3. restructures the code to align with naming convention of the Linux
>according to the review of Doug Ledford
> 4. modify the state for all .c and .h files
> 
> Changes v6 -> v7:
> 1. modify some type of parameter, use bool replace the original type
> 2. add the Signed-off-by signatures in the first patch
> 3. delete the improper print sentence in hns_roce_create_eq.
> 
> Changes v5 -> v6:
> 1. modify the type of obj for unsigned long according the reviews, and
>modify the same questions in RoCE module
> 2. fix the spelling error
> 3. fix the Signed-off-by signatures
> 
> Changes v4 -> v5:
> 1. redesign the patchset for RoCE modules in order to split the huge
>patch into small patches
> 2. fix the directory path for RoCE module. Delete the hisilicon level.
> 3. modify the name of roce_v1_hw into roce_hw_v1
> 
> Changes v3 -> v4:
> 1. modify roce.o into hns-roce.o in Makefile and Kconfig file
> 
> Changes v2 -> v3:
> 1. modify the formats of RoCE driver code base v2 by the experts 
>reviewing. also, it used kmalloc_array instead of kmalloc, kcalloc
>instead of kzalloc, when refer to memory allocation for array
> 2. remove some functions without use and unconnected macros
> 3. modify the binding document with RoCE DT base v2 which added
>interrupt-names
> 4. redesign the port_map and si_map in hns_dsaf_roce_reset
> 5. add HiSilicon RoCE driver maintainers introduction in MAINTAINERS
>document
> 
> Changes v1 -> v2:
> 1. modify the formats of roce driver code by the experts reviewing
> 2. modify the bindings file with roce dts. add the attribute named 
>interrput-names.
> 3. modify the way of defining port mode in hns_dsaf_main.c
> 

Re: [PATCH v11 00/22] Add HiSilicon RoCE driver

[oulijun]

在 2016/7/2 17:39, Lijun Ou 写道:
> The HiSilicon Network Substem is a long term evolution IP which is
> supposed to be used in HiSilicon ICT SoCs. HNS (HiSilicon Network
> Sybsystem) also has a hardware support of performing RDMA with
> RoCEE.
> The driver for HiSilicon RoCEE(RoCE Engine) is a platform driver and
> will support mulitple versions of SOCs in future. This version of driver
> is meant to support Hip06 SoC(which confirms to RoCEv1 hardware
> specifications).
> 
> Changes v10 -> v11:
> [1/22]:
> 1. modify the print description of chip don't support roce
> 2. remove explicit values for enums for patch series
> [3/22]:
> 3. remove non-essential headers for patch series
> 4. add judgement for port_cnt is zero
> 5. Keep unified print style for "set mask..." vs. "No usable
>..."
> 6. modify the MODULE_LICENSE
> 7. remove MODULE_ALIAS
> [4/22]:
> 8. Move this line out of if-else and leave "if (enable)" part only
> 9. renaming the meaningful definition to 20 for patch series
> 10. delete extern keyword for hns_dsaf_roce_reset function
> 11. delete void keyword for hr_dev->hw->reset when driver removed
> [5/22]:
> 12. remove few unnecessary variables and some lines.
> 13. remove the function for one line of code which will be called
> once only for patch series
> [6/22]:
> 14. redesign the method for calculating token_mask' value
> [7/22]:
> 15. delete hns_roce_status_to_errno
> 16. modify the one enum only for all patches
> 17. remove the spin_lock in hns_roce_cq_event function
> 18. add comment here that 0x10 and 0x11 in hns_roce_event struct
> 19. refactor hns_roce_aeq_int function and It has switch in switch
> and it is almost 200 LOCs
> 20. simplify the lines for err_out_free_pages branch
> [8/22]:
> 21. remove icm and redesign it for patch series
> 
> Changes v9 -> v10:
> 1. delete redundant lines which it is netdevice.h in hns_roce_main.c
> 2. adjust the indentation for HNS_ROCE_V1_NUM_ASYNC_EQE
> 3. simplify the lines in hns_roce_init_qp_table function
> 4. add static type for hns_roce_unregister_device
> 5. move the call with hns_roce_unregister_device from the tenth patch to
>the eleventh patch in hns_roce_remove function
> 6. readjuest the alphabetic order in MAINTAINERS
> 7. redesigned the way for getting irq names
> 8. avoid the memory leakage because mr->pbl is not free in
>hns_roce_mr function
> 9. avoid the memory leakage because not kfree table->icm when exception
> 10. add the link from LKML as well whose comment in all
> 
> Changes v8 -> v9:
> 1. delete the definition of ADDR_SHIFT_n, use literal 12, 32 and 44 and
>add comments
> 2. use roce_read/roce_write/readl/write instead of roce_readl/roce_writel
> 3. delete the print error/debug messages for memory allocation errors
> 4. use exit instead of uninit, for example hw->uninit -> hw->exit
> 5. use roce_raw_write instead of _raw_writel in eq_set_cons_index
> 6. modify the label with underscore
> 7. adjust the indentation for the macro definitions in hns_roce_hw_v1.c
> 8. simplify some lines in few functions and structures
> 9. adjust the alphabetic order in MAINTAINERS
> 
> Changes v7 -> v8:
> 1. add a verbs operation named get_port_immutable. It is an 
>independent patch
> 2. add a comment for the definition of ADDR_SHIFT_n, n are 12,32
>and 44
> 3. restructures the code to align with naming convention of the Linux
>according to the review of Doug Ledford
> 4. modify the state for all .c and .h files
> 
> Changes v6 -> v7:
> 1. modify some type of parameter, use bool replace the original type
> 2. add the Signed-off-by signatures in the first patch
> 3. delete the improper print sentence in hns_roce_create_eq.
> 
> Changes v5 -> v6:
> 1. modify the type of obj for unsigned long according the reviews, and
>modify the same questions in RoCE module
> 2. fix the spelling error
> 3. fix the Signed-off-by signatures
> 
> Changes v4 -> v5:
> 1. redesign the patchset for RoCE modules in order to split the huge
>patch into small patches
> 2. fix the directory path for RoCE module. Delete the hisilicon level.
> 3. modify the name of roce_v1_hw into roce_hw_v1
> 
> Changes v3 -> v4:
> 1. modify roce.o into hns-roce.o in Makefile and Kconfig file
> 
> Changes v2 -> v3:
> 1. modify the formats of RoCE driver code base v2 by the experts 
>reviewing. also, it used kmalloc_array instead of kmalloc, kcalloc
>instead of kzalloc, when refer to memory allocation for array
> 2. remove some functions without use and unconnected macros
> 3. modify the binding document with RoCE DT base v2 which added
>interrupt-names
> 4. redesign the port_map and si_map in hns_dsaf_roce_reset
> 5. add HiSilicon RoCE driver maintainers introduction in MAINTAINERS
>document
> 
> Changes v1 -> v2:
> 1. modify the formats of roce driver code by the experts reviewing
> 2. modify the bindings file with roce dts. add the attribute named 
>interrput-names.
> 3. modify the way of defining port mode in hns_dsaf_main.c
> 

Re: [PATCH v11 00/22] Add HiSilicon RoCE driver

[oulijun]

在 2016/7/2 17:39, Lijun Ou 写道:
> The HiSilicon Network Substem is a long term evolution IP which is
> supposed to be used in HiSilicon ICT SoCs. HNS (HiSilicon Network
> Sybsystem) also has a hardware support of performing RDMA with
> RoCEE.
> The driver for HiSilicon RoCEE(RoCE Engine) is a platform driver and
> will support mulitple versions of SOCs in future. This version of driver
> is meant to support Hip06 SoC(which confirms to RoCEv1 hardware
> specifications).
> 
> Changes v10 -> v11:
> [1/22]:
> 1. modify the print description of chip don't support roce
> 2. remove explicit values for enums for patch series
> [3/22]:
> 3. remove non-essential headers for patch series
> 4. add judgement for port_cnt is zero
> 5. Keep unified print style for "set mask..." vs. "No usable
>..."
> 6. modify the MODULE_LICENSE
> 7. remove MODULE_ALIAS
> [4/22]:
> 8. Move this line out of if-else and leave "if (enable)" part only
> 9. renaming the meaningful definition to 20 for patch series
> 10. delete extern keyword for hns_dsaf_roce_reset function
> 11. delete void keyword for hr_dev->hw->reset when driver removed
> [5/22]:
> 12. remove few unnecessary variables and some lines.
> 13. remove the function for one line of code which will be called
> once only for patch series
> [6/22]:
> 14. redesign the method for calculating token_mask' value
> [7/22]:
> 15. delete hns_roce_status_to_errno
> 16. modify the one enum only for all patches
> 17. remove the spin_lock in hns_roce_cq_event function
> 18. add comment here that 0x10 and 0x11 in hns_roce_event struct
> 19. refactor hns_roce_aeq_int function and It has switch in switch
> and it is almost 200 LOCs
> 20. simplify the lines for err_out_free_pages branch
> [8/22]:
> 21. remove icm and redesign it for patch series
> 
> Changes v9 -> v10:
> 1. delete redundant lines which it is netdevice.h in hns_roce_main.c
> 2. adjust the indentation for HNS_ROCE_V1_NUM_ASYNC_EQE
> 3. simplify the lines in hns_roce_init_qp_table function
> 4. add static type for hns_roce_unregister_device
> 5. move the call with hns_roce_unregister_device from the tenth patch to
>the eleventh patch in hns_roce_remove function
> 6. readjuest the alphabetic order in MAINTAINERS
> 7. redesigned the way for getting irq names
> 8. avoid the memory leakage because mr->pbl is not free in
>hns_roce_mr function
> 9. avoid the memory leakage because not kfree table->icm when exception
> 10. add the link from LKML as well whose comment in all
> 
> Changes v8 -> v9:
> 1. delete the definition of ADDR_SHIFT_n, use literal 12, 32 and 44 and
>add comments
> 2. use roce_read/roce_write/readl/write instead of roce_readl/roce_writel
> 3. delete the print error/debug messages for memory allocation errors
> 4. use exit instead of uninit, for example hw->uninit -> hw->exit
> 5. use roce_raw_write instead of _raw_writel in eq_set_cons_index
> 6. modify the label with underscore
> 7. adjust the indentation for the macro definitions in hns_roce_hw_v1.c
> 8. simplify some lines in few functions and structures
> 9. adjust the alphabetic order in MAINTAINERS
> 
> Changes v7 -> v8:
> 1. add a verbs operation named get_port_immutable. It is an 
>independent patch
> 2. add a comment for the definition of ADDR_SHIFT_n, n are 12,32
>and 44
> 3. restructures the code to align with naming convention of the Linux
>according to the review of Doug Ledford
> 4. modify the state for all .c and .h files
> 
> Changes v6 -> v7:
> 1. modify some type of parameter, use bool replace the original type
> 2. add the Signed-off-by signatures in the first patch
> 3. delete the improper print sentence in hns_roce_create_eq.
> 
> Changes v5 -> v6:
> 1. modify the type of obj for unsigned long according the reviews, and
>modify the same questions in RoCE module
> 2. fix the spelling error
> 3. fix the Signed-off-by signatures
> 
> Changes v4 -> v5:
> 1. redesign the patchset for RoCE modules in order to split the huge
>patch into small patches
> 2. fix the directory path for RoCE module. Delete the hisilicon level.
> 3. modify the name of roce_v1_hw into roce_hw_v1
> 
> Changes v3 -> v4:
> 1. modify roce.o into hns-roce.o in Makefile and Kconfig file
> 
> Changes v2 -> v3:
> 1. modify the formats of RoCE driver code base v2 by the experts 
>reviewing. also, it used kmalloc_array instead of kmalloc, kcalloc
>instead of kzalloc, when refer to memory allocation for array
> 2. remove some functions without use and unconnected macros
> 3. modify the binding document with RoCE DT base v2 which added
>interrupt-names
> 4. redesign the port_map and si_map in hns_dsaf_roce_reset
> 5. add HiSilicon RoCE driver maintainers introduction in MAINTAINERS
>document
> 
> Changes v1 -> v2:
> 1. modify the formats of roce driver code by the experts reviewing
> 2. modify the bindings file with roce dts. add the attribute named 
>interrput-names.
> 3. modify the way of defining port mode in hns_dsaf_main.c
> 

Re: [PATCH v11 00/22] Add HiSilicon RoCE driver

[oulijun]

在 2016/7/2 17:39, Lijun Ou 写道:
> The HiSilicon Network Substem is a long term evolution IP which is
> supposed to be used in HiSilicon ICT SoCs. HNS (HiSilicon Network
> Sybsystem) also has a hardware support of performing RDMA with
> RoCEE.
> The driver for HiSilicon RoCEE(RoCE Engine) is a platform driver and
> will support mulitple versions of SOCs in future. This version of driver
> is meant to support Hip06 SoC(which confirms to RoCEv1 hardware
> specifications).
> 
> Changes v10 -> v11:
> [1/22]:
> 1. modify the print description of chip don't support roce
> 2. remove explicit values for enums for patch series
> [3/22]:
> 3. remove non-essential headers for patch series
> 4. add judgement for port_cnt is zero
> 5. Keep unified print style for "set mask..." vs. "No usable
>..."
> 6. modify the MODULE_LICENSE
> 7. remove MODULE_ALIAS
> [4/22]:
> 8. Move this line out of if-else and leave "if (enable)" part only
> 9. renaming the meaningful definition to 20 for patch series
> 10. delete extern keyword for hns_dsaf_roce_reset function
> 11. delete void keyword for hr_dev->hw->reset when driver removed
> [5/22]:
> 12. remove few unnecessary variables and some lines.
> 13. remove the function for one line of code which will be called
> once only for patch series
> [6/22]:
> 14. redesign the method for calculating token_mask' value
> [7/22]:
> 15. delete hns_roce_status_to_errno
> 16. modify the one enum only for all patches
> 17. remove the spin_lock in hns_roce_cq_event function
> 18. add comment here that 0x10 and 0x11 in hns_roce_event struct
> 19. refactor hns_roce_aeq_int function and It has switch in switch
> and it is almost 200 LOCs
> 20. simplify the lines for err_out_free_pages branch
> [8/22]:
> 21. remove icm and redesign it for patch series
> 
> Changes v9 -> v10:
> 1. delete redundant lines which it is netdevice.h in hns_roce_main.c
> 2. adjust the indentation for HNS_ROCE_V1_NUM_ASYNC_EQE
> 3. simplify the lines in hns_roce_init_qp_table function
> 4. add static type for hns_roce_unregister_device
> 5. move the call with hns_roce_unregister_device from the tenth patch to
>the eleventh patch in hns_roce_remove function
> 6. readjuest the alphabetic order in MAINTAINERS
> 7. redesigned the way for getting irq names
> 8. avoid the memory leakage because mr->pbl is not free in
>hns_roce_mr function
> 9. avoid the memory leakage because not kfree table->icm when exception
> 10. add the link from LKML as well whose comment in all
> 
> Changes v8 -> v9:
> 1. delete the definition of ADDR_SHIFT_n, use literal 12, 32 and 44 and
>add comments
> 2. use roce_read/roce_write/readl/write instead of roce_readl/roce_writel
> 3. delete the print error/debug messages for memory allocation errors
> 4. use exit instead of uninit, for example hw->uninit -> hw->exit
> 5. use roce_raw_write instead of _raw_writel in eq_set_cons_index
> 6. modify the label with underscore
> 7. adjust the indentation for the macro definitions in hns_roce_hw_v1.c
> 8. simplify some lines in few functions and structures
> 9. adjust the alphabetic order in MAINTAINERS
> 
> Changes v7 -> v8:
> 1. add a verbs operation named get_port_immutable. It is an 
>independent patch
> 2. add a comment for the definition of ADDR_SHIFT_n, n are 12,32
>and 44
> 3. restructures the code to align with naming convention of the Linux
>according to the review of Doug Ledford
> 4. modify the state for all .c and .h files
> 
> Changes v6 -> v7:
> 1. modify some type of parameter, use bool replace the original type
> 2. add the Signed-off-by signatures in the first patch
> 3. delete the improper print sentence in hns_roce_create_eq.
> 
> Changes v5 -> v6:
> 1. modify the type of obj for unsigned long according the reviews, and
>modify the same questions in RoCE module
> 2. fix the spelling error
> 3. fix the Signed-off-by signatures
> 
> Changes v4 -> v5:
> 1. redesign the patchset for RoCE modules in order to split the huge
>patch into small patches
> 2. fix the directory path for RoCE module. Delete the hisilicon level.
> 3. modify the name of roce_v1_hw into roce_hw_v1
> 
> Changes v3 -> v4:
> 1. modify roce.o into hns-roce.o in Makefile and Kconfig file
> 
> Changes v2 -> v3:
> 1. modify the formats of RoCE driver code base v2 by the experts 
>reviewing. also, it used kmalloc_array instead of kmalloc, kcalloc
>instead of kzalloc, when refer to memory allocation for array
> 2. remove some functions without use and unconnected macros
> 3. modify the binding document with RoCE DT base v2 which added
>interrupt-names
> 4. redesign the port_map and si_map in hns_dsaf_roce_reset
> 5. add HiSilicon RoCE driver maintainers introduction in MAINTAINERS
>document
> 
> Changes v1 -> v2:
> 1. modify the formats of roce driver code by the experts reviewing
> 2. modify the bindings file with roce dts. add the attribute named 
>interrput-names.
> 3. modify the way of defining port mode in hns_dsaf_main.c
> 

Re: [PATCH v11 00/22] Add HiSilicon RoCE driver

[oulijun]

在 2016/7/2 20:58, Leon Romanovsky 写道:
> On Sat, Jul 02, 2016 at 05:39:02PM +0800, Lijun Ou wrote:
> 
> This v11
>>  28 files changed, 10626 insertions(+), 1 deletion(-)
> 
> First version
>>  27 files changed, 11670 insertions(+), 11 deletions(-)
> 
> 1K LOC less, we are moving in right direction.
> 
Good, Thank you very much！
Thank you for more suggestions.

Thanks
Lijun Ou

Re: [PATCH v11 00/22] Add HiSilicon RoCE driver

[oulijun]

在 2016/7/2 20:58, Leon Romanovsky 写道:
> On Sat, Jul 02, 2016 at 05:39:02PM +0800, Lijun Ou wrote:
> 
> This v11
>>  28 files changed, 10626 insertions(+), 1 deletion(-)
> 
> First version
>>  27 files changed, 11670 insertions(+), 11 deletions(-)
> 
> 1K LOC less, we are moving in right direction.
> 
Good, Thank you very much！
Thank you for more suggestions.

Thanks
Lijun Ou

Re: [PATCH v10 07/22] IB/hns: Add event queue support

[oulijun]

Hi, Leon
在 2016/6/24 23:46, Leon Romanovsky 写道:
> On Thu, Jun 16, 2016 at 10:35:15PM +0800, Lijun Ou wrote:
>> This patch added event queue support for RoCE driver. It is used
>> for RoCE interrupt. RoCE includes 32 synchronous event irqs, 1
>> asynchronous event irq and 1 common overflow irq.
>>
>> Signed-off-by: Wei Hu 
>> Signed-off-by: Nenglong Zhao 
>> Signed-off-by: Lijun Ou 
>> ---
>> PATCH v9/v8:
>> - No change over the PATCH v7
>>
>> PATCH v7:
>> This fixes the comments given by Doug Ledford over the PATCH v6:
>>   Link: https://lkml.org/lkml/2016/5/13/510
>>
>> PATCH v6:
>> - No change over the PATCH v5
>>
>> PATCH v5:
>> - The initial patch which was redesigned based on the second patch
>>   in PATCH v4
>> ---
>> ---
>>  drivers/infiniband/hw/hns/hns_roce_cmd.c|  22 +
>>  drivers/infiniband/hw/hns/hns_roce_common.h |  70 +++
>>  drivers/infiniband/hw/hns/hns_roce_cq.c |  77 +++
>>  drivers/infiniband/hw/hns/hns_roce_device.h | 135 +
>>  drivers/infiniband/hw/hns/hns_roce_eq.c | 750 
>> 
>>  drivers/infiniband/hw/hns/hns_roce_eq.h | 130 +
>>  drivers/infiniband/hw/hns/hns_roce_main.c   |  24 +
>>  drivers/infiniband/hw/hns/hns_roce_qp.c |  63 +++
>>  8 files changed, 1271 insertions(+)
>>  create mode 100644 drivers/infiniband/hw/hns/hns_roce_cq.c
>>  create mode 100644 drivers/infiniband/hw/hns/hns_roce_eq.c
>>  create mode 100644 drivers/infiniband/hw/hns/hns_roce_eq.h
>>  create mode 100644 drivers/infiniband/hw/hns/hns_roce_qp.c
>>
>> diff --git a/drivers/infiniband/hw/hns/hns_roce_cmd.c 
>> b/drivers/infiniband/hw/hns/hns_roce_cmd.c
>> index 64e84fe..67b3137 100644
>> --- a/drivers/infiniband/hw/hns/hns_roce_cmd.c
>> +++ b/drivers/infiniband/hw/hns/hns_roce_cmd.c
>> @@ -45,6 +45,28 @@
>>  
>>  #define CMD_MAX_NUM 32
>>  
>> +static int hns_roce_status_to_errno(u8 orig_status)
>> +{
>> +if (orig_status == HNS_ROCE_CMD_SUCCESS)
>> +return 0;
>> +else
>> +return -EIO;
>> +}
> 
> 1. Can orig_status be different from SUCCESS? You defined one enum only.
> 2. return (orig_status == HNS_ROCE_CMD_SUCCESS)?0:(-EIO);
> 
Sure, I will modify it in patch v11.
>> +
>> +void hns_roce_cmd_event(struct hns_roce_dev *hr_dev, u16 token, u8 status,
>> +u64 out_param)
>> +{
>> +struct hns_roce_cmd_context
>> +*context = _dev->cmd.context[token & hr_dev->cmd.token_mask];
>> +
>> +if (token != context->token)
>> +return;
>> +
>> +context->result = hns_roce_status_to_errno(status);
>> +context->out_param = out_param;
>> +complete(>done);
>> +}
>> +
>>  int hns_roce_cmd_init(struct hns_roce_dev *hr_dev)
>>  {
>>  struct device *dev = _dev->pdev->dev;
>> diff --git a/drivers/infiniband/hw/hns/hns_roce_common.h 
>> b/drivers/infiniband/hw/hns/hns_roce_common.h
>> index 595cda9..4805852 100644
>> --- a/drivers/infiniband/hw/hns/hns_roce_common.h
>> +++ b/drivers/infiniband/hw/hns/hns_roce_common.h
>> @@ -33,7 +33,56 @@
>>  #ifndef _HNS_ROCE_COMMON_H
>>  #define _HNS_ROCE_COMMON_H
>>  
>> +#define roce_write(dev, reg, val)   writel((val), (dev)->reg_base + (reg))
>>  #define roce_read(dev, reg) readl((dev)->reg_base + (reg))
>> +#define roce_raw_write(value, addr) \
>> +__raw_writel((__force u32)cpu_to_le32(value), (addr))
>> +
>> +#define roce_get_field(origin, mask, shift) \
>> +(((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)); \
>> +} while (0)
>> +
>> +#define roce_set_bit(origin, shift, val) \
>> +roce_set_field((origin), (1ul << (shift)), (shift), (val))
>> +
>> +#define ROCEE_CAEP_AEQC_AEQE_SHIFT_CAEP_AEQC_STATE_S 0
>> +#define ROCEE_CAEP_AEQC_AEQE_SHIFT_CAEP_AEQC_STATE_M   \
>> +(((1UL << 2) - 1) << ROCEE_CAEP_AEQC_AEQE_SHIFT_CAEP_AEQC_STATE_S)
>> +
>> +#define ROCEE_CAEP_AEQC_AEQE_SHIFT_CAEP_AEQC_AEQE_SHIFT_S 8
>> +#define ROCEE_CAEP_AEQC_AEQE_SHIFT_CAEP_AEQC_AEQE_SHIFT_M   \
>> +(((1UL << 4) - 1) << ROCEE_CAEP_AEQC_AEQE_SHIFT_CAEP_AEQC_AEQE_SHIFT_S)
>> +
>> +#define ROCEE_CAEP_AEQC_AEQE_SHIFT_CAEP_AEQ_ALM_OVF_INT_ST_S 17
>> +
>> +#define ROCEE_CAEP_AEQE_CUR_IDX_CAEP_AEQ_BT_H_S 0
>> +#define ROCEE_CAEP_AEQE_CUR_IDX_CAEP_AEQ_BT_H_M   \
>> +(((1UL << 5) - 1) << ROCEE_CAEP_AEQE_CUR_IDX_CAEP_AEQ_BT_H_S)
>> +
>> +#define ROCEE_CAEP_AEQE_CUR_IDX_CAEP_AEQE_CUR_IDX_S 16
>> +#define ROCEE_CAEP_AEQE_CUR_IDX_CAEP_AEQE_CUR_IDX_M   \
>> +(((1UL << 16) - 1) << ROCEE_CAEP_AEQE_CUR_IDX_CAEP_AEQE_CUR_IDX_S)
>> +
>> +#define ROCEE_CAEP_AEQE_CONS_IDX_CAEP_AEQE_CONS_IDX_S 0
>> +#define ROCEE_CAEP_AEQE_CONS_IDX_CAEP_AEQE_CONS_IDX_M   \
>> +(((1UL << 16) - 1) << 

Re: [PATCH v10 07/22] IB/hns: Add event queue support

[oulijun]

Hi, Leon
在 2016/6/24 23:46, Leon Romanovsky 写道:
> On Thu, Jun 16, 2016 at 10:35:15PM +0800, Lijun Ou wrote:
>> This patch added event queue support for RoCE driver. It is used
>> for RoCE interrupt. RoCE includes 32 synchronous event irqs, 1
>> asynchronous event irq and 1 common overflow irq.
>>
>> Signed-off-by: Wei Hu 
>> Signed-off-by: Nenglong Zhao 
>> Signed-off-by: Lijun Ou 
>> ---
>> PATCH v9/v8:
>> - No change over the PATCH v7
>>
>> PATCH v7:
>> This fixes the comments given by Doug Ledford over the PATCH v6:
>>   Link: https://lkml.org/lkml/2016/5/13/510
>>
>> PATCH v6:
>> - No change over the PATCH v5
>>
>> PATCH v5:
>> - The initial patch which was redesigned based on the second patch
>>   in PATCH v4
>> ---
>> ---
>>  drivers/infiniband/hw/hns/hns_roce_cmd.c|  22 +
>>  drivers/infiniband/hw/hns/hns_roce_common.h |  70 +++
>>  drivers/infiniband/hw/hns/hns_roce_cq.c |  77 +++
>>  drivers/infiniband/hw/hns/hns_roce_device.h | 135 +
>>  drivers/infiniband/hw/hns/hns_roce_eq.c | 750 
>> 
>>  drivers/infiniband/hw/hns/hns_roce_eq.h | 130 +
>>  drivers/infiniband/hw/hns/hns_roce_main.c   |  24 +
>>  drivers/infiniband/hw/hns/hns_roce_qp.c |  63 +++
>>  8 files changed, 1271 insertions(+)
>>  create mode 100644 drivers/infiniband/hw/hns/hns_roce_cq.c
>>  create mode 100644 drivers/infiniband/hw/hns/hns_roce_eq.c
>>  create mode 100644 drivers/infiniband/hw/hns/hns_roce_eq.h
>>  create mode 100644 drivers/infiniband/hw/hns/hns_roce_qp.c
>>
>> diff --git a/drivers/infiniband/hw/hns/hns_roce_cmd.c 
>> b/drivers/infiniband/hw/hns/hns_roce_cmd.c
>> index 64e84fe..67b3137 100644
>> --- a/drivers/infiniband/hw/hns/hns_roce_cmd.c
>> +++ b/drivers/infiniband/hw/hns/hns_roce_cmd.c
>> @@ -45,6 +45,28 @@
>>  
>>  #define CMD_MAX_NUM 32
>>  
>> +static int hns_roce_status_to_errno(u8 orig_status)
>> +{
>> +if (orig_status == HNS_ROCE_CMD_SUCCESS)
>> +return 0;
>> +else
>> +return -EIO;
>> +}
> 
> 1. Can orig_status be different from SUCCESS? You defined one enum only.
> 2. return (orig_status == HNS_ROCE_CMD_SUCCESS)?0:(-EIO);
> 
Sure, I will modify it in patch v11.
>> +
>> +void hns_roce_cmd_event(struct hns_roce_dev *hr_dev, u16 token, u8 status,
>> +u64 out_param)
>> +{
>> +struct hns_roce_cmd_context
>> +*context = _dev->cmd.context[token & hr_dev->cmd.token_mask];
>> +
>> +if (token != context->token)
>> +return;
>> +
>> +context->result = hns_roce_status_to_errno(status);
>> +context->out_param = out_param;
>> +complete(>done);
>> +}
>> +
>>  int hns_roce_cmd_init(struct hns_roce_dev *hr_dev)
>>  {
>>  struct device *dev = _dev->pdev->dev;
>> diff --git a/drivers/infiniband/hw/hns/hns_roce_common.h 
>> b/drivers/infiniband/hw/hns/hns_roce_common.h
>> index 595cda9..4805852 100644
>> --- a/drivers/infiniband/hw/hns/hns_roce_common.h
>> +++ b/drivers/infiniband/hw/hns/hns_roce_common.h
>> @@ -33,7 +33,56 @@
>>  #ifndef _HNS_ROCE_COMMON_H
>>  #define _HNS_ROCE_COMMON_H
>>  
>> +#define roce_write(dev, reg, val)   writel((val), (dev)->reg_base + (reg))
>>  #define roce_read(dev, reg) readl((dev)->reg_base + (reg))
>> +#define roce_raw_write(value, addr) \
>> +__raw_writel((__force u32)cpu_to_le32(value), (addr))
>> +
>> +#define roce_get_field(origin, mask, shift) \
>> +(((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)); \
>> +} while (0)
>> +
>> +#define roce_set_bit(origin, shift, val) \
>> +roce_set_field((origin), (1ul << (shift)), (shift), (val))
>> +
>> +#define ROCEE_CAEP_AEQC_AEQE_SHIFT_CAEP_AEQC_STATE_S 0
>> +#define ROCEE_CAEP_AEQC_AEQE_SHIFT_CAEP_AEQC_STATE_M   \
>> +(((1UL << 2) - 1) << ROCEE_CAEP_AEQC_AEQE_SHIFT_CAEP_AEQC_STATE_S)
>> +
>> +#define ROCEE_CAEP_AEQC_AEQE_SHIFT_CAEP_AEQC_AEQE_SHIFT_S 8
>> +#define ROCEE_CAEP_AEQC_AEQE_SHIFT_CAEP_AEQC_AEQE_SHIFT_M   \
>> +(((1UL << 4) - 1) << ROCEE_CAEP_AEQC_AEQE_SHIFT_CAEP_AEQC_AEQE_SHIFT_S)
>> +
>> +#define ROCEE_CAEP_AEQC_AEQE_SHIFT_CAEP_AEQ_ALM_OVF_INT_ST_S 17
>> +
>> +#define ROCEE_CAEP_AEQE_CUR_IDX_CAEP_AEQ_BT_H_S 0
>> +#define ROCEE_CAEP_AEQE_CUR_IDX_CAEP_AEQ_BT_H_M   \
>> +(((1UL << 5) - 1) << ROCEE_CAEP_AEQE_CUR_IDX_CAEP_AEQ_BT_H_S)
>> +
>> +#define ROCEE_CAEP_AEQE_CUR_IDX_CAEP_AEQE_CUR_IDX_S 16
>> +#define ROCEE_CAEP_AEQE_CUR_IDX_CAEP_AEQE_CUR_IDX_M   \
>> +(((1UL << 16) - 1) << ROCEE_CAEP_AEQE_CUR_IDX_CAEP_AEQE_CUR_IDX_S)
>> +
>> +#define ROCEE_CAEP_AEQE_CONS_IDX_CAEP_AEQE_CONS_IDX_S 0
>> +#define ROCEE_CAEP_AEQE_CONS_IDX_CAEP_AEQE_CONS_IDX_M   \
>> +(((1UL << 16) - 1) << ROCEE_CAEP_AEQE_CONS_IDX_CAEP_AEQE_CONS_IDX_S)
>> +
>> +#define 

Re: [PATCH v10 05/22] IB/hns: Add initial profile resource

[oulijun]

Hi, leon
在 2016/6/24 23:10, Leon Romanovsky 写道:
> On Thu, Jun 16, 2016 at 10:35:13PM +0800, Lijun Ou wrote:
>> This patch added the operation for cmd, and added some functions
>> for initializing eq table and selecting cmd mode.
>>
>> Signed-off-by: Wei Hu 
>> Signed-off-by: Nenglong Zhao 
>> Signed-off-by: Lijun Ou 
>> ---
>> PATCH v9:
>> This fixes the comments given by Leon Romanovsky over the PATCH v8:
>>   Link: https://lkml.org/lkml/2016/6/9/65
>>
>> PATCH v8/v7/v6:
>> - No change over the PATCH v5
>>
>> PATCH v5:
>> - The initial patch which was redesigned based on the second patch
>>   in PATCH v4
>> ---
>> ---
>>  drivers/infiniband/hw/hns/hns_roce_common.h | 49 +++
>>  drivers/infiniband/hw/hns/hns_roce_device.h | 55 -
>>  drivers/infiniband/hw/hns/hns_roce_hw_v1.c  | 75 
>> +
>>  drivers/infiniband/hw/hns/hns_roce_hw_v1.h  | 36 ++
>>  drivers/infiniband/hw/hns/hns_roce_main.c   |  7 +++
>>  5 files changed, 221 insertions(+), 1 deletion(-)
>>  create mode 100644 drivers/infiniband/hw/hns/hns_roce_common.h
>>
>> diff --git a/drivers/infiniband/hw/hns/hns_roce_common.h 
>> b/drivers/infiniband/hw/hns/hns_roce_common.h
>> new file mode 100644
>> index 000..4cc4761
>> --- /dev/null
>> +++ b/drivers/infiniband/hw/hns/hns_roce_common.h
>> @@ -0,0 +1,49 @@
>> +/*
>> + * Copyright (c) 2016 Hisilicon Limited.
>> + *
>> + * This software is available to you under a choice of one of two
>> + * licenses.  You may choose to be licensed under the terms of the GNU
>> + * General Public License (GPL) Version 2, available from the file
>> + * COPYING in the main directory of this source tree, or the
>> + * OpenIB.org BSD license below:
>> + *
>> + * Redistribution and use in source and binary forms, with or
>> + * without modification, are permitted provided that the following
>> + * conditions are met:
>> + *
>> + *  - Redistributions of source code must retain the above
>> + *copyright notice, this list of conditions and the following
>> + *disclaimer.
>> + *
>> + *  - Redistributions in binary form must reproduce the above
>> + *copyright notice, this list of conditions and the following
>> + *disclaimer in the documentation and/or other materials
>> + *provided with the distribution.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
>> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
>> + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
>> + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
>> + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
>> + * SOFTWARE.
>> + */
>> +
>> +#ifndef _HNS_ROCE_COMMON_H
>> +#define _HNS_ROCE_COMMON_H
>> +
>> +#define roce_read(dev, reg) readl((dev)->reg_base + (reg))
>> +
>> +/*ROCEE_REG DEFINITION/
>> +#define ROCEE_VENDOR_ID_REG 0x0
>> +#define ROCEE_VENDOR_PART_ID_REG0x4
>> +
>> +#define ROCEE_HW_VERSION_REG0x8
>> +
>> +#define ROCEE_SYS_IMAGE_GUID_L_REG  0xC
>> +#define ROCEE_SYS_IMAGE_GUID_H_REG  0x10
>> +
>> +#define ROCEE_ACK_DELAY_REG 0x14
>> +
>> +#endif /* _HNS_ROCE_COMMON_H */
>> diff --git a/drivers/infiniband/hw/hns/hns_roce_device.h 
>> b/drivers/infiniband/hw/hns/hns_roce_device.h
>> index b857c76..e01ea34 100644
>> --- a/drivers/infiniband/hw/hns/hns_roce_device.h
>> +++ b/drivers/infiniband/hw/hns/hns_roce_device.h
>> @@ -45,6 +45,12 @@
>>  #define DRV_NAME "hns_roce"
>>  
>>  #define HNS_ROCE_MAX_IRQ_NUM34
>> +
>> +#define HNS_ROCE_COMP_VEC_NUM   32
>> +
>> +#define HNS_ROCE_AEQE_VEC_NUM   1
>> +#define HNS_ROCE_AEQE_OF_VEC_NUM1
>> +
>>  #define HNS_ROCE_MAX_PORTS  6
>>  
>>  struct hns_roce_ib_iboe {
>> @@ -53,11 +59,52 @@ struct hns_roce_ib_iboe {
>>  };
>>  
>>  struct hns_roce_caps {
>> -u8  num_ports;
>> +u64 fw_ver;
>> +u8  num_ports;
>> +int gid_table_len[HNS_ROCE_MAX_PORTS];
>> +int pkey_table_len[HNS_ROCE_MAX_PORTS];
>> +int local_ca_ack_delay;
>> +int num_uars;
>> +u32 phy_num_uars;
>> +u32 max_sq_sg;  /* 2 */
>> +u32 max_sq_inline;  /* 32 */
>> +u32 max_rq_sg;  /* 2 */
>> +int num_qps;/* 256k */
>> +u32 max_wqes;   /* 16k */
>> +u32 max_sq_desc_sz; /* 64 */
>> +u32 max_rq_desc_sz; /* 64 */
>> +int 

Re: [PATCH v10 05/22] IB/hns: Add initial profile resource

[oulijun]

Hi, leon
在 2016/6/24 23:10, Leon Romanovsky 写道:
> On Thu, Jun 16, 2016 at 10:35:13PM +0800, Lijun Ou wrote:
>> This patch added the operation for cmd, and added some functions
>> for initializing eq table and selecting cmd mode.
>>
>> Signed-off-by: Wei Hu 
>> Signed-off-by: Nenglong Zhao 
>> Signed-off-by: Lijun Ou 
>> ---
>> PATCH v9:
>> This fixes the comments given by Leon Romanovsky over the PATCH v8:
>>   Link: https://lkml.org/lkml/2016/6/9/65
>>
>> PATCH v8/v7/v6:
>> - No change over the PATCH v5
>>
>> PATCH v5:
>> - The initial patch which was redesigned based on the second patch
>>   in PATCH v4
>> ---
>> ---
>>  drivers/infiniband/hw/hns/hns_roce_common.h | 49 +++
>>  drivers/infiniband/hw/hns/hns_roce_device.h | 55 -
>>  drivers/infiniband/hw/hns/hns_roce_hw_v1.c  | 75 
>> +
>>  drivers/infiniband/hw/hns/hns_roce_hw_v1.h  | 36 ++
>>  drivers/infiniband/hw/hns/hns_roce_main.c   |  7 +++
>>  5 files changed, 221 insertions(+), 1 deletion(-)
>>  create mode 100644 drivers/infiniband/hw/hns/hns_roce_common.h
>>
>> diff --git a/drivers/infiniband/hw/hns/hns_roce_common.h 
>> b/drivers/infiniband/hw/hns/hns_roce_common.h
>> new file mode 100644
>> index 000..4cc4761
>> --- /dev/null
>> +++ b/drivers/infiniband/hw/hns/hns_roce_common.h
>> @@ -0,0 +1,49 @@
>> +/*
>> + * Copyright (c) 2016 Hisilicon Limited.
>> + *
>> + * This software is available to you under a choice of one of two
>> + * licenses.  You may choose to be licensed under the terms of the GNU
>> + * General Public License (GPL) Version 2, available from the file
>> + * COPYING in the main directory of this source tree, or the
>> + * OpenIB.org BSD license below:
>> + *
>> + * Redistribution and use in source and binary forms, with or
>> + * without modification, are permitted provided that the following
>> + * conditions are met:
>> + *
>> + *  - Redistributions of source code must retain the above
>> + *copyright notice, this list of conditions and the following
>> + *disclaimer.
>> + *
>> + *  - Redistributions in binary form must reproduce the above
>> + *copyright notice, this list of conditions and the following
>> + *disclaimer in the documentation and/or other materials
>> + *provided with the distribution.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
>> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
>> + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
>> + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
>> + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
>> + * SOFTWARE.
>> + */
>> +
>> +#ifndef _HNS_ROCE_COMMON_H
>> +#define _HNS_ROCE_COMMON_H
>> +
>> +#define roce_read(dev, reg) readl((dev)->reg_base + (reg))
>> +
>> +/*ROCEE_REG DEFINITION/
>> +#define ROCEE_VENDOR_ID_REG 0x0
>> +#define ROCEE_VENDOR_PART_ID_REG0x4
>> +
>> +#define ROCEE_HW_VERSION_REG0x8
>> +
>> +#define ROCEE_SYS_IMAGE_GUID_L_REG  0xC
>> +#define ROCEE_SYS_IMAGE_GUID_H_REG  0x10
>> +
>> +#define ROCEE_ACK_DELAY_REG 0x14
>> +
>> +#endif /* _HNS_ROCE_COMMON_H */
>> diff --git a/drivers/infiniband/hw/hns/hns_roce_device.h 
>> b/drivers/infiniband/hw/hns/hns_roce_device.h
>> index b857c76..e01ea34 100644
>> --- a/drivers/infiniband/hw/hns/hns_roce_device.h
>> +++ b/drivers/infiniband/hw/hns/hns_roce_device.h
>> @@ -45,6 +45,12 @@
>>  #define DRV_NAME "hns_roce"
>>  
>>  #define HNS_ROCE_MAX_IRQ_NUM34
>> +
>> +#define HNS_ROCE_COMP_VEC_NUM   32
>> +
>> +#define HNS_ROCE_AEQE_VEC_NUM   1
>> +#define HNS_ROCE_AEQE_OF_VEC_NUM1
>> +
>>  #define HNS_ROCE_MAX_PORTS  6
>>  
>>  struct hns_roce_ib_iboe {
>> @@ -53,11 +59,52 @@ struct hns_roce_ib_iboe {
>>  };
>>  
>>  struct hns_roce_caps {
>> -u8  num_ports;
>> +u64 fw_ver;
>> +u8  num_ports;
>> +int gid_table_len[HNS_ROCE_MAX_PORTS];
>> +int pkey_table_len[HNS_ROCE_MAX_PORTS];
>> +int local_ca_ack_delay;
>> +int num_uars;
>> +u32 phy_num_uars;
>> +u32 max_sq_sg;  /* 2 */
>> +u32 max_sq_inline;  /* 32 */
>> +u32 max_rq_sg;  /* 2 */
>> +int num_qps;/* 256k */
>> +u32 max_wqes;   /* 16k */
>> +u32 max_sq_desc_sz; /* 64 */
>> +u32 max_rq_desc_sz; /* 64 */
>> +int max_qp_init_rdma;
>> +int max_qp_dest_rdma;
>> +int 

Re: [PATCH v10 04/22] IB/hns: Add RoCE engine reset function

[oulijun]

Hi, Leon
在 2016/6/27 16:01, Leon Romanovsky 写道:
> On Sat, Jun 25, 2016 at 06:25:37PM +0800, Wei Hu (Xavier) wrote:
>>
>>
>> On 2016/6/24 22:59, Leon Romanovsky wrote:
>>> On Thu, Jun 16, 2016 at 10:35:12PM +0800, Lijun Ou wrote:
 This patch mainly added reset flow of RoCE engine in RoCE
 driver. It is necessary when RoCE is loaded and removed.

 Signed-off-by: Wei Hu 
 Signed-off-by: Nenglong Zhao 
 Signed-off-by: Lijun Ou 
 ---
> 
> ...
> 
 +
 +#define SLEEP_TIME_INTERVAL   20
 +
 +extern int hns_dsaf_roce_reset(struct fwnode_handle *dsaf_fwnode, bool 
 enable);
>>> Why did you add this extern?
>>> You already exported this function.
>>> drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c:EXPORT_SYMBOL(hns_dsaf_roce_reset);
>> Hi, Leon
>>
>> The function named hns_dsaf_roce_reset is defined in hns_dsaf_main.c
>> It exists in hns_dsaf.ko(ethernet driver)
>>
>> RoCE driver will call this function.
>>
>> Your suggestion is that delete "extern" as below:
>> In /drivers/infiniband/hw/hns/hns_roce_hw_v1.h:
>>
>>   int hns_dsaf_roce_reset(struct fwnode_handle *dsaf_fwnode, bool
>> enable);
>>
>> Right? or other soultion?
> 
> You placed it in header file.
> Please move it to your hns_roce_hw_v1.c file.
> 
 You suggest to do as follows, right?
 in hns_roce_hw_v1.c
   int hns_dsaf_roce_reset(struct fwnode_handle *dsaf_fwnode, bool enable);

 and delete the keyword extern

 Bcause reserve the extern in hns_roce_hw_v1.c, the checkpatch is not pass.

>>
>>
>> Regards
>> Wei Hu
 +
 +#endif
 diff --git a/drivers/infiniband/hw/hns/hns_roce_main.c 
 b/drivers/infiniband/hw/hns/hns_roce_main.c
 index 8924ce3..d5ccce2 100644
 --- a/drivers/infiniband/hw/hns/hns_roce_main.c
 +++ b/drivers/infiniband/hw/hns/hns_roce_main.c
 @@ -71,7 +71,9 @@ static int hns_roce_get_cfg(struct hns_roce_dev *hr_dev)
struct platform_device *pdev = NULL;
struct resource *res;
 -  if (!of_device_is_compatible(np, "hisilicon,hns-roce-v1")) {
 +  if (of_device_is_compatible(np, "hisilicon,hns-roce-v1")) {
 +  hr_dev->hw = _roce_hw_v1;
 +  } else {
dev_err(dev, "device no compatible!\n");
return -EINVAL;
}
 @@ -118,6 +120,11 @@ static int hns_roce_get_cfg(struct hns_roce_dev 
 *hr_dev)
return 0;
  }
 +static int hns_roce_engine_reset(struct hns_roce_dev *hr_dev, bool enable)
 +{
 +  return hr_dev->hw->reset(hr_dev, enable);
 +}
 +
  /**
  * hns_roce_probe - RoCE driver entrance
  * @pdev: pointer to platform device
 @@ -156,6 +163,12 @@ static int hns_roce_probe(struct platform_device 
 *pdev)
goto error_failed_get_cfg;
}
 +  ret = hns_roce_engine_reset(hr_dev, true);
>>> Do you have better solution to sense device without doing full reset of
>>> your hardware?
>> Hi, Leon
>>
>> In this place, we need reset RoCEE engine to ensure that RoCE engine can
>> work correctly.
>> Hip06 Soc only support full reset RoCEE engine.
>>
>> Regards
>> Wei Hu
>>
>>>
 +  if (ret) {
 +  dev_err(dev, "Reset roce engine failed!\n");
 +  goto error_failed_get_cfg;
 +  }
 +
  error_failed_get_cfg:
ib_dealloc_device(_dev->ib_dev);
 @@ -170,6 +183,8 @@ static int hns_roce_remove(struct platform_device 
 *pdev)
  {
struct hns_roce_dev *hr_dev = platform_get_drvdata(pdev);
 +  (void)hns_roce_engine_reset(hr_dev, false);
>>> Any reason to do explicit casting?
>> Hi, Leon
>>
>> /**
>>  * hns_roce_engine_reset - reset roce
>>  * @hr_dev: roce device struct pointer
>>  * @enable: true -- drop reset, false -- reset
>>  * return 0 - success , negative --fail
>>  */
>> static int hns_roce_engine_reset(struct hns_roce_dev *hr_dev, bool enable);
>>
>> hns_roce_engine_reset->hns_roce_v1_reset->hns_dsaf_roce_reset
>>
>> The err branch of hns_roce_engine_reset as below：
>> int hns_dsaf_roce_reset(struct fwnode_handle *dsaf_fwnode, bool enable)
>> {
>> <...>
>> if (!is_of_node(dsaf_fwnode)) {
>> pr_err("hisi_dsaf: Only support DT node!\n");
>> return -EINVAL;
>> }
>>
>> pdev = of_find_device_by_node(to_of_node(dsaf_fwnode));
>> dsaf_dev = dev_get_drvdata(>dev);
>> if (AE_IS_VER1(dsaf_dev->dsaf_ver)) {
>> dev_err(dsaf_dev->dev, "%s v1 chip do not support roce!\n",
>> dsaf_dev->ae_dev.name);
>> return -ENODEV;
>> }
>> <...>
>> }
>>
>>When the cpu is processing hns_roce_engine_reset(hr_dev, false),
>> hns_roce_engine_reset(hr_dev, true)  have been alomost processed
>> sucessfully.
>>From the err branch of hns_roce_engine_reset, we found at this time
>> hns_roce_engine_reset(hr_dev, true) could not return err.
>>
>> In hns_roce_remove 

Re: [PATCH v10 04/22] IB/hns: Add RoCE engine reset function

[oulijun]

Hi, Leon
在 2016/6/27 16:01, Leon Romanovsky 写道:
> On Sat, Jun 25, 2016 at 06:25:37PM +0800, Wei Hu (Xavier) wrote:
>>
>>
>> On 2016/6/24 22:59, Leon Romanovsky wrote:
>>> On Thu, Jun 16, 2016 at 10:35:12PM +0800, Lijun Ou wrote:
 This patch mainly added reset flow of RoCE engine in RoCE
 driver. It is necessary when RoCE is loaded and removed.

 Signed-off-by: Wei Hu 
 Signed-off-by: Nenglong Zhao 
 Signed-off-by: Lijun Ou 
 ---
> 
> ...
> 
 +
 +#define SLEEP_TIME_INTERVAL   20
 +
 +extern int hns_dsaf_roce_reset(struct fwnode_handle *dsaf_fwnode, bool 
 enable);
>>> Why did you add this extern?
>>> You already exported this function.
>>> drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c:EXPORT_SYMBOL(hns_dsaf_roce_reset);
>> Hi, Leon
>>
>> The function named hns_dsaf_roce_reset is defined in hns_dsaf_main.c
>> It exists in hns_dsaf.ko(ethernet driver)
>>
>> RoCE driver will call this function.
>>
>> Your suggestion is that delete "extern" as below:
>> In /drivers/infiniband/hw/hns/hns_roce_hw_v1.h:
>>
>>   int hns_dsaf_roce_reset(struct fwnode_handle *dsaf_fwnode, bool
>> enable);
>>
>> Right? or other soultion?
> 
> You placed it in header file.
> Please move it to your hns_roce_hw_v1.c file.
> 
 You suggest to do as follows, right?
 in hns_roce_hw_v1.c
   int hns_dsaf_roce_reset(struct fwnode_handle *dsaf_fwnode, bool enable);

 and delete the keyword extern

 Bcause reserve the extern in hns_roce_hw_v1.c, the checkpatch is not pass.

>>
>>
>> Regards
>> Wei Hu
 +
 +#endif
 diff --git a/drivers/infiniband/hw/hns/hns_roce_main.c 
 b/drivers/infiniband/hw/hns/hns_roce_main.c
 index 8924ce3..d5ccce2 100644
 --- a/drivers/infiniband/hw/hns/hns_roce_main.c
 +++ b/drivers/infiniband/hw/hns/hns_roce_main.c
 @@ -71,7 +71,9 @@ static int hns_roce_get_cfg(struct hns_roce_dev *hr_dev)
struct platform_device *pdev = NULL;
struct resource *res;
 -  if (!of_device_is_compatible(np, "hisilicon,hns-roce-v1")) {
 +  if (of_device_is_compatible(np, "hisilicon,hns-roce-v1")) {
 +  hr_dev->hw = _roce_hw_v1;
 +  } else {
dev_err(dev, "device no compatible!\n");
return -EINVAL;
}
 @@ -118,6 +120,11 @@ static int hns_roce_get_cfg(struct hns_roce_dev 
 *hr_dev)
return 0;
  }
 +static int hns_roce_engine_reset(struct hns_roce_dev *hr_dev, bool enable)
 +{
 +  return hr_dev->hw->reset(hr_dev, enable);
 +}
 +
  /**
  * hns_roce_probe - RoCE driver entrance
  * @pdev: pointer to platform device
 @@ -156,6 +163,12 @@ static int hns_roce_probe(struct platform_device 
 *pdev)
goto error_failed_get_cfg;
}
 +  ret = hns_roce_engine_reset(hr_dev, true);
>>> Do you have better solution to sense device without doing full reset of
>>> your hardware?
>> Hi, Leon
>>
>> In this place, we need reset RoCEE engine to ensure that RoCE engine can
>> work correctly.
>> Hip06 Soc only support full reset RoCEE engine.
>>
>> Regards
>> Wei Hu
>>
>>>
 +  if (ret) {
 +  dev_err(dev, "Reset roce engine failed!\n");
 +  goto error_failed_get_cfg;
 +  }
 +
  error_failed_get_cfg:
ib_dealloc_device(_dev->ib_dev);
 @@ -170,6 +183,8 @@ static int hns_roce_remove(struct platform_device 
 *pdev)
  {
struct hns_roce_dev *hr_dev = platform_get_drvdata(pdev);
 +  (void)hns_roce_engine_reset(hr_dev, false);
>>> Any reason to do explicit casting?
>> Hi, Leon
>>
>> /**
>>  * hns_roce_engine_reset - reset roce
>>  * @hr_dev: roce device struct pointer
>>  * @enable: true -- drop reset, false -- reset
>>  * return 0 - success , negative --fail
>>  */
>> static int hns_roce_engine_reset(struct hns_roce_dev *hr_dev, bool enable);
>>
>> hns_roce_engine_reset->hns_roce_v1_reset->hns_dsaf_roce_reset
>>
>> The err branch of hns_roce_engine_reset as below：
>> int hns_dsaf_roce_reset(struct fwnode_handle *dsaf_fwnode, bool enable)
>> {
>> <...>
>> if (!is_of_node(dsaf_fwnode)) {
>> pr_err("hisi_dsaf: Only support DT node!\n");
>> return -EINVAL;
>> }
>>
>> pdev = of_find_device_by_node(to_of_node(dsaf_fwnode));
>> dsaf_dev = dev_get_drvdata(>dev);
>> if (AE_IS_VER1(dsaf_dev->dsaf_ver)) {
>> dev_err(dsaf_dev->dev, "%s v1 chip do not support roce!\n",
>> dsaf_dev->ae_dev.name);
>> return -ENODEV;
>> }
>> <...>
>> }
>>
>>When the cpu is processing hns_roce_engine_reset(hr_dev, false),
>> hns_roce_engine_reset(hr_dev, true)  have been alomost processed
>> sucessfully.
>>From the err branch of hns_roce_engine_reset, we found at this time
>> hns_roce_engine_reset(hr_dev, true) could not return err.
>>
>> In hns_roce_remove function, we call hns_roce_engine_reset(hr_dev, false),
>> and doesn't need 

Re: [PATCH v10 01/22] net: hns: Add reset function support for RoCE driver

[oulijun]

Hi, Leon

在 2016/6/24 19:49, Leon Romanovsky 写道:
> On Thu, Jun 16, 2016 at 10:35:09PM +0800, Lijun Ou wrote:
>> It added reset function for RoCE driver. RoCE is a feature of hns.
>> In hip06 SoC, in RoCE reset process, it's needed to configure dsaf
>> channel reset, port and sl map info. Reset function of RoCE is
>> located in dsaf module, we only call it in RoCE driver when needed.
>>
>> Signed-off-by: Wei Hu 
>> Signed-off-by: Nenglong Zhao 
>> Signed-off-by: Lijun Ou 
>> Signed-off-by: Sheng Li 
>> ---
>> PATCH v9/v8/v7:
>> - No change over PATCH v6
>>
>> PATCH v6:
>> This fixes the comments given by Leon Romanovsky over the PATCH v5:
>>   Link: https://lkml.org/lkml/2016/5/3/733
>>
>> PATCH v5/v4/v3:
>> - No change over PATCH v2
>>
>> PATCH v2:
>> This fixes the comments given by Leon Romanovsky over the PATCH v1:
>>   Link: https://lkml.org/lkml/2016/3/12/46
>>
>> PATCH v1:
>> - The initial patch
>> ---
>> ---
>>  drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c | 84 
>> ++
>>  drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.h | 30 
>>  drivers/net/ethernet/hisilicon/hns/hns_dsaf_misc.c | 36 ++
>>  drivers/net/ethernet/hisilicon/hns/hns_dsaf_reg.h  | 14 +++-
>>  4 files changed, 163 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c 
>> b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c
>> index 1c2ddb2..0c4a87c 100644
>> --- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c
>> +++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c
>> @@ -14,6 +14,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>  #include 
>>  #include 
>>  #include 
>> @@ -2685,6 +2686,89 @@ static struct platform_driver g_dsaf_driver = {
>>
>>  module_platform_driver(g_dsaf_driver);
>>
>> +/**
>> + * hns_dsaf_roce_reset - reset dsaf and roce
>> + * @dsaf_fwnode: Pointer to framework node for the dasf
>> + * @enable: false - request reset , true - drop reset
>> + * retuen 0 - success , negative -fail
>> + */
>> +int hns_dsaf_roce_reset(struct fwnode_handle *dsaf_fwnode, bool enable)
>> +{
>> + struct dsaf_device *dsaf_dev;
>> + struct platform_device *pdev;
>> + unsigned int mp;
>> + unsigned int sl;
>> + unsigned int credit;
>> + int i;
>> + const u32 port_map[DSAF_ROCE_CREDIT_CHN][DSAF_ROCE_CHAN_MODE_NUM] = {
>> + {DSAF_ROCE_PORT_0, DSAF_ROCE_PORT_0, DSAF_ROCE_PORT_0},
>> + {DSAF_ROCE_PORT_1, DSAF_ROCE_PORT_0, DSAF_ROCE_PORT_0},
>> + {DSAF_ROCE_PORT_2, DSAF_ROCE_PORT_1, DSAF_ROCE_PORT_0},
>> + {DSAF_ROCE_PORT_3, DSAF_ROCE_PORT_1, DSAF_ROCE_PORT_0},
>> + {DSAF_ROCE_PORT_4, DSAF_ROCE_PORT_2, DSAF_ROCE_PORT_1},
>> + {DSAF_ROCE_PORT_4, DSAF_ROCE_PORT_2, DSAF_ROCE_PORT_1},
>> + {DSAF_ROCE_PORT_5, DSAF_ROCE_PORT_3, DSAF_ROCE_PORT_1},
>> + {DSAF_ROCE_PORT_5, DSAF_ROCE_PORT_3, DSAF_ROCE_PORT_1},
>> + };
>> + const u32 sl_map[DSAF_ROCE_CREDIT_CHN][DSAF_ROCE_CHAN_MODE_NUM] = {
>> + {DSAF_ROCE_SL_0, DSAF_ROCE_SL_0, DSAF_ROCE_SL_0},
>> + {DSAF_ROCE_SL_0, DSAF_ROCE_SL_1, DSAF_ROCE_SL_1},
>> + {DSAF_ROCE_SL_0, DSAF_ROCE_SL_0, DSAF_ROCE_SL_2},
>> + {DSAF_ROCE_SL_0, DSAF_ROCE_SL_1, DSAF_ROCE_SL_3},
>> + {DSAF_ROCE_SL_0, DSAF_ROCE_SL_0, DSAF_ROCE_SL_0},
>> + {DSAF_ROCE_SL_1, DSAF_ROCE_SL_1, DSAF_ROCE_SL_1},
>> + {DSAF_ROCE_SL_0, DSAF_ROCE_SL_0, DSAF_ROCE_SL_2},
>> + {DSAF_ROCE_SL_1, DSAF_ROCE_SL_1, DSAF_ROCE_SL_3},
>> + };
>> +
>> + if (!is_of_node(dsaf_fwnode)) {
>> + pr_err("hisi_dsaf: Only support DT node!\n");
>> + return -EINVAL;
>> + }
>> + pdev = of_find_device_by_node(to_of_node(dsaf_fwnode));
>> + dsaf_dev = dev_get_drvdata(>dev);
>> + if (AE_IS_VER1(dsaf_dev->dsaf_ver)) {
>> + dev_err(dsaf_dev->dev, "%s v1 chip do not support roce!\n",
> 
> chip don't support roce -> chip doesn't support RoCE
I have modified it. I will send a new patch at soon.
> 
>> + dsaf_dev->ae_dev.name);
>> + return -ENODEV;
>> + }
>> +
>> + if (!enable) {
>> + /* Reset rocee-channels in dsaf and rocee */
>> + hns_dsaf_srst_chns(dsaf_dev, DSAF_CHNS_MASK, false);
>> + hns_dsaf_roce_srst(dsaf_dev, false);
>> + } else {
>> + /* Configure dsaf tx roce correspond to port map and sl map */
>> + mp = dsaf_read_dev(dsaf_dev, DSAF_ROCE_PORT_MAP_REG);
>> + for (i = 0; i < DSAF_ROCE_CREDIT_CHN; i++)
>> + dsaf_set_field(mp, 7 << i * 3, i * 3,
>> +   port_map[i][DSAF_ROCE_6PORT_MODE]);
>> + dsaf_set_field(mp, 3 << i * 3, i * 3, 0);
>> + dsaf_write_dev(dsaf_dev, DSAF_ROCE_PORT_MAP_REG, mp);
>> +
>> + sl = dsaf_read_dev(dsaf_dev, DSAF_ROCE_SL_MAP_REG);
>> + for (i = 0; i < DSAF_ROCE_CREDIT_CHN; i++)
>> + dsaf_set_field(sl, 3 << i * 2, i * 2,
>> +   sl_map[i][DSAF_ROCE_6PORT_MODE]);
>> + dsaf_write_dev(dsaf_dev, DSAF_ROCE_SL_MAP_REG, sl);
>> +
>> + /* De-reset rocee-channels in dsaf and rocee */
>> + hns_dsaf_srst_chns(dsaf_dev, DSAF_CHNS_MASK, true);
>> + msleep(20);
>> + hns_dsaf_roce_srst(dsaf_dev, true);
>> +
>> + /* Eanble dsaf channel rocee credit */
>> + credit = 

Re: [PATCH v10 01/22] net: hns: Add reset function support for RoCE driver

[oulijun]

Hi, Leon

在 2016/6/24 19:49, Leon Romanovsky 写道:
> On Thu, Jun 16, 2016 at 10:35:09PM +0800, Lijun Ou wrote:
>> It added reset function for RoCE driver. RoCE is a feature of hns.
>> In hip06 SoC, in RoCE reset process, it's needed to configure dsaf
>> channel reset, port and sl map info. Reset function of RoCE is
>> located in dsaf module, we only call it in RoCE driver when needed.
>>
>> Signed-off-by: Wei Hu 
>> Signed-off-by: Nenglong Zhao 
>> Signed-off-by: Lijun Ou 
>> Signed-off-by: Sheng Li 
>> ---
>> PATCH v9/v8/v7:
>> - No change over PATCH v6
>>
>> PATCH v6:
>> This fixes the comments given by Leon Romanovsky over the PATCH v5:
>>   Link: https://lkml.org/lkml/2016/5/3/733
>>
>> PATCH v5/v4/v3:
>> - No change over PATCH v2
>>
>> PATCH v2:
>> This fixes the comments given by Leon Romanovsky over the PATCH v1:
>>   Link: https://lkml.org/lkml/2016/3/12/46
>>
>> PATCH v1:
>> - The initial patch
>> ---
>> ---
>>  drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c | 84 
>> ++
>>  drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.h | 30 
>>  drivers/net/ethernet/hisilicon/hns/hns_dsaf_misc.c | 36 ++
>>  drivers/net/ethernet/hisilicon/hns/hns_dsaf_reg.h  | 14 +++-
>>  4 files changed, 163 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c 
>> b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c
>> index 1c2ddb2..0c4a87c 100644
>> --- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c
>> +++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c
>> @@ -14,6 +14,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>  #include 
>>  #include 
>>  #include 
>> @@ -2685,6 +2686,89 @@ static struct platform_driver g_dsaf_driver = {
>>
>>  module_platform_driver(g_dsaf_driver);
>>
>> +/**
>> + * hns_dsaf_roce_reset - reset dsaf and roce
>> + * @dsaf_fwnode: Pointer to framework node for the dasf
>> + * @enable: false - request reset , true - drop reset
>> + * retuen 0 - success , negative -fail
>> + */
>> +int hns_dsaf_roce_reset(struct fwnode_handle *dsaf_fwnode, bool enable)
>> +{
>> + struct dsaf_device *dsaf_dev;
>> + struct platform_device *pdev;
>> + unsigned int mp;
>> + unsigned int sl;
>> + unsigned int credit;
>> + int i;
>> + const u32 port_map[DSAF_ROCE_CREDIT_CHN][DSAF_ROCE_CHAN_MODE_NUM] = {
>> + {DSAF_ROCE_PORT_0, DSAF_ROCE_PORT_0, DSAF_ROCE_PORT_0},
>> + {DSAF_ROCE_PORT_1, DSAF_ROCE_PORT_0, DSAF_ROCE_PORT_0},
>> + {DSAF_ROCE_PORT_2, DSAF_ROCE_PORT_1, DSAF_ROCE_PORT_0},
>> + {DSAF_ROCE_PORT_3, DSAF_ROCE_PORT_1, DSAF_ROCE_PORT_0},
>> + {DSAF_ROCE_PORT_4, DSAF_ROCE_PORT_2, DSAF_ROCE_PORT_1},
>> + {DSAF_ROCE_PORT_4, DSAF_ROCE_PORT_2, DSAF_ROCE_PORT_1},
>> + {DSAF_ROCE_PORT_5, DSAF_ROCE_PORT_3, DSAF_ROCE_PORT_1},
>> + {DSAF_ROCE_PORT_5, DSAF_ROCE_PORT_3, DSAF_ROCE_PORT_1},
>> + };
>> + const u32 sl_map[DSAF_ROCE_CREDIT_CHN][DSAF_ROCE_CHAN_MODE_NUM] = {
>> + {DSAF_ROCE_SL_0, DSAF_ROCE_SL_0, DSAF_ROCE_SL_0},
>> + {DSAF_ROCE_SL_0, DSAF_ROCE_SL_1, DSAF_ROCE_SL_1},
>> + {DSAF_ROCE_SL_0, DSAF_ROCE_SL_0, DSAF_ROCE_SL_2},
>> + {DSAF_ROCE_SL_0, DSAF_ROCE_SL_1, DSAF_ROCE_SL_3},
>> + {DSAF_ROCE_SL_0, DSAF_ROCE_SL_0, DSAF_ROCE_SL_0},
>> + {DSAF_ROCE_SL_1, DSAF_ROCE_SL_1, DSAF_ROCE_SL_1},
>> + {DSAF_ROCE_SL_0, DSAF_ROCE_SL_0, DSAF_ROCE_SL_2},
>> + {DSAF_ROCE_SL_1, DSAF_ROCE_SL_1, DSAF_ROCE_SL_3},
>> + };
>> +
>> + if (!is_of_node(dsaf_fwnode)) {
>> + pr_err("hisi_dsaf: Only support DT node!\n");
>> + return -EINVAL;
>> + }
>> + pdev = of_find_device_by_node(to_of_node(dsaf_fwnode));
>> + dsaf_dev = dev_get_drvdata(>dev);
>> + if (AE_IS_VER1(dsaf_dev->dsaf_ver)) {
>> + dev_err(dsaf_dev->dev, "%s v1 chip do not support roce!\n",
> 
> chip don't support roce -> chip doesn't support RoCE
I have modified it. I will send a new patch at soon.
> 
>> + dsaf_dev->ae_dev.name);
>> + return -ENODEV;
>> + }
>> +
>> + if (!enable) {
>> + /* Reset rocee-channels in dsaf and rocee */
>> + hns_dsaf_srst_chns(dsaf_dev, DSAF_CHNS_MASK, false);
>> + hns_dsaf_roce_srst(dsaf_dev, false);
>> + } else {
>> + /* Configure dsaf tx roce correspond to port map and sl map */
>> + mp = dsaf_read_dev(dsaf_dev, DSAF_ROCE_PORT_MAP_REG);
>> + for (i = 0; i < DSAF_ROCE_CREDIT_CHN; i++)
>> + dsaf_set_field(mp, 7 << i * 3, i * 3,
>> +   port_map[i][DSAF_ROCE_6PORT_MODE]);
>> + dsaf_set_field(mp, 3 << i * 3, i * 3, 0);
>> + dsaf_write_dev(dsaf_dev, DSAF_ROCE_PORT_MAP_REG, mp);
>> +
>> + sl = dsaf_read_dev(dsaf_dev, DSAF_ROCE_SL_MAP_REG);
>> + for (i = 0; i < DSAF_ROCE_CREDIT_CHN; i++)
>> + dsaf_set_field(sl, 3 << i * 2, i * 2,
>> +   sl_map[i][DSAF_ROCE_6PORT_MODE]);
>> + dsaf_write_dev(dsaf_dev, DSAF_ROCE_SL_MAP_REG, sl);
>> +
>> + /* De-reset rocee-channels in dsaf and rocee */
>> + hns_dsaf_srst_chns(dsaf_dev, DSAF_CHNS_MASK, true);
>> + msleep(20);
>> + hns_dsaf_roce_srst(dsaf_dev, true);
>> +
>> + /* Eanble dsaf channel rocee credit */
>> + credit = dsaf_read_dev(dsaf_dev, DSAF_SBM_ROCEE_CFG_REG_REG);
>> + dsaf_set_bit(credit, 

Re: [RESEND PATCH v9 00/22] Add HiSilicon RoCE driver

[oulijun]

On 2016/6/8 19:55, Doug Ledford wrote:
> On 6/8/2016 2:44 AM, Lijun Ou wrote:
>> The HiSilicon Network Substem is a long term evolution IP which is
>> supposed to be used in HiSilicon ICT SoCs. HNS (HiSilicon Network
>> Sybsystem) also has a hardware support of performing RDMA with
>> RoCEE.
>> The driver for HiSilicon RoCEE(RoCE Engine) is a platform driver and
>> will support mulitple versions of SOCs in future. This version of driver
>> is meant to support Hip06 SoC(which confirms to RoCEEv1 hardware
>> specifications).
> 
> Please don't resend unless you think the patch has gotten lost.  This is
> a large patchset and takes a considerable deal of time to process.
> 
> That said, I pulled the v9 patchset into my repo in a branch called hns
> yesterday and am doing final review before I accept it into my for-next
> area.
> 
Hi, Doug
  I see. Please forgive my impatience.
  Now, I have sent v10 according to Leon Romanovsky's reviews.

Thanks
Lijun Ou

Re: [RESEND PATCH v9 00/22] Add HiSilicon RoCE driver

[oulijun]

On 2016/6/8 19:55, Doug Ledford wrote:
> On 6/8/2016 2:44 AM, Lijun Ou wrote:
>> The HiSilicon Network Substem is a long term evolution IP which is
>> supposed to be used in HiSilicon ICT SoCs. HNS (HiSilicon Network
>> Sybsystem) also has a hardware support of performing RDMA with
>> RoCEE.
>> The driver for HiSilicon RoCEE(RoCE Engine) is a platform driver and
>> will support mulitple versions of SOCs in future. This version of driver
>> is meant to support Hip06 SoC(which confirms to RoCEEv1 hardware
>> specifications).
> 
> Please don't resend unless you think the patch has gotten lost.  This is
> a large patchset and takes a considerable deal of time to process.
> 
> That said, I pulled the v9 patchset into my repo in a branch called hns
> yesterday and am doing final review before I accept it into my for-next
> area.
> 
Hi, Doug
  I see. Please forgive my impatience.
  Now, I have sent v10 according to Leon Romanovsky's reviews.

Thanks
Lijun Ou

Re: [PATCH v9 05/22] IB/hns: Add initial profile resource

[oulijun]

Hi,
On 2016/6/9 15:01, Leon Romanovsky wrote:
> On Wed, Jun 01, 2016 at 11:37:47PM +0800, Lijun Ou wrote:
>> This patch mainly configured some profile resoure. For example,
>> vendor_id, hardware version, and some data structure sizes so on.
>>
>> Signed-off-by: Wei Hu 
>> Signed-off-by: Nenglong Zhao 
>> Signed-off-by: Lijun Ou 
>> ---
> 
> <...>
> 
>> +#define HNS_ROCE_V1_NUM_COMP_EQE0x8000
>> +#define HNS_ROCE_V1_NUM_ASYNC_EQE   0x400
> 
> Something wrong with indentation.
> 
I have checked it in my local code and will send a new patch at soon.

thanks
Lijun Ou

Re: [PATCH v9 05/22] IB/hns: Add initial profile resource

[oulijun]

Hi,
On 2016/6/9 15:01, Leon Romanovsky wrote:
> On Wed, Jun 01, 2016 at 11:37:47PM +0800, Lijun Ou wrote:
>> This patch mainly configured some profile resoure. For example,
>> vendor_id, hardware version, and some data structure sizes so on.
>>
>> Signed-off-by: Wei Hu 
>> Signed-off-by: Nenglong Zhao 
>> Signed-off-by: Lijun Ou 
>> ---
> 
> <...>
> 
>> +#define HNS_ROCE_V1_NUM_COMP_EQE0x8000
>> +#define HNS_ROCE_V1_NUM_ASYNC_EQE   0x400
> 
> Something wrong with indentation.
> 
I have checked it in my local code and will send a new patch at soon.

thanks
Lijun Ou

Re: [PATCH v6 00/21] Add HiSilicon RoCE driver

[oulijun]

On 2016/5/4 2:48, Leon Romanovsky wrote:
> On Thu, Apr 28, 2016 at 08:09:35PM +0800, Lijun Ou wrote:
>> The HiSilicon Network Substem is a long term evolution IP which is
>> supposed to be used in HiSilicon ICT SoCs. HNS (HiSilicon Network
>> Sybsystem) also has a hardware support of performing RDMA with
>> RoCEE.
>> The driver for HiSilicon RoCEE(RoCE Engine) is a platform driver and
>> will support mulitple versions of SOCs in future. This version of driver
>> is meant to support Hip06 SoC(which confirms to RoCEEv1 hardware
>> specifications).
> 
> Please read Dave's comment [1], it is valuable for your code as
> well.
> 
> * Please use 'bool' and "true/false"
> 
Yes, i see. thanks

Thanks
Lijun Ou
> [1] http://marc.info/?l=linux-rdma=146229367301442=2
> 
> Thanks
> 

Re: [PATCH v6 00/21] Add HiSilicon RoCE driver

[oulijun]

On 2016/5/4 2:48, Leon Romanovsky wrote:
> On Thu, Apr 28, 2016 at 08:09:35PM +0800, Lijun Ou wrote:
>> The HiSilicon Network Substem is a long term evolution IP which is
>> supposed to be used in HiSilicon ICT SoCs. HNS (HiSilicon Network
>> Sybsystem) also has a hardware support of performing RDMA with
>> RoCEE.
>> The driver for HiSilicon RoCEE(RoCE Engine) is a platform driver and
>> will support mulitple versions of SOCs in future. This version of driver
>> is meant to support Hip06 SoC(which confirms to RoCEEv1 hardware
>> specifications).
> 
> Please read Dave's comment [1], it is valuable for your code as
> well.
> 
> * Please use 'bool' and "true/false"
> 
Yes, i see. thanks

Thanks
Lijun Ou
> [1] http://marc.info/?l=linux-rdma=146229367301442=2
> 
> Thanks
> 

Re: [PATCH v5 09/21] IB/hns: Add hca support

[oulijun]

On 2016/4/26 22:18, Leon Romanovsky wrote:
> On Tue, Apr 26, 2016 at 02:34:44PM +0800, oulijun wrote:
>> On 2016/4/24 15:54, Leon Romanovsky wrote:
>>> On Sat, Apr 23, 2016 at 06:26:47PM +0800, Lijun Ou wrote:
>>>> This patch mainly setup hca for RoCE. it will do a series of
>>>> initial works as follows:
>>>>   1. init uar table, allocate uar resource
>>>>   2. init pd table
>>>>   3. init cq table
>>>>   4. init mr table
>>>>   5. init qp table
>>>>
>>>> Signed-off-by: Lijun Ou <ouli...@huawei.com>
>>>> Signed-off-by: Wei Hu(Xavier) <xavier.hu...@huawei.com>
>>>> ---
>>>>  drivers/infiniband/hw/hns/hns_roce_alloc.c  | 104 
>>>>  drivers/infiniband/hw/hns/hns_roce_cq.c |  25 
>>>>  drivers/infiniband/hw/hns/hns_roce_device.h |  69 ++
>>>>  drivers/infiniband/hw/hns/hns_roce_eq.c |   1 -
>>>>  drivers/infiniband/hw/hns/hns_roce_icm.c|  88 +
>>>>  drivers/infiniband/hw/hns/hns_roce_icm.h|   9 ++
>>>>  drivers/infiniband/hw/hns/hns_roce_main.c   |  79 
>>>>  drivers/infiniband/hw/hns/hns_roce_mr.c | 187 
>>>> 
>>>>  drivers/infiniband/hw/hns/hns_roce_pd.c |  65 ++
>>>>  drivers/infiniband/hw/hns/hns_roce_qp.c |  30 +
>>>>  10 files changed, 656 insertions(+), 1 deletion(-)
>>>>  create mode 100644 drivers/infiniband/hw/hns/hns_roce_alloc.c
>>>>  create mode 100644 drivers/infiniband/hw/hns/hns_roce_mr.c
>>>>  create mode 100644 drivers/infiniband/hw/hns/hns_roce_pd.c
>>>>
>>>> diff --git a/drivers/infiniband/hw/hns/hns_roce_alloc.c 
>>>> b/drivers/infiniband/hw/hns/hns_roce_alloc.c
>>>> new file mode 100644
>>>> index 000..0c76f1b
>>>> --- /dev/null
>>>> +++ b/drivers/infiniband/hw/hns/hns_roce_alloc.c
>>>> @@ -0,0 +1,104 @@
>>>> +/*
>>>> + * Copyright (c) 2016 Hisilicon Limited.
>>>> + *
>>>> + * This program is free software; you can redistribute it and/or modify
>>>> + * it under the terms of the GNU General Public License as published by
>>>> + * the Free Software Foundation; either version 2 of the License, or
>>>> + * (at your option) any later version.
>>>> + */
>>>> +
>>>> +#include 
>>>> +#include 
>>>> +#include 
>>>> +#include 
>>>> +#include 
>>>> +#include 
>>>> +#include "hns_roce_device.h"
>>>> +
>>>> +int hns_roce_bitmap_alloc(struct hns_roce_bitmap *bitmap, u32 *obj)
>>>> +{
>>>> +  int ret = 0;
>>>> +
>>>> +  spin_lock(>lock);
>>>> +  *obj = find_next_zero_bit(bitmap->table, bitmap->max, bitmap->last);
>>>> +  if (*obj >= bitmap->max) {
>>>> +  bitmap->top = (bitmap->top + bitmap->max + bitmap->reserved_top)
>>>> + & bitmap->mask;
>>>> +  *obj = find_first_zero_bit(bitmap->table, bitmap->max);
>>>
>>> find_first_zero_bit function returns "unsigned long" which may or may
>>> not be equal to u32 on some architectures.
>>>
>> Hi Leon,
>> I appreciate your keen eye. this code is meant for ARM64bit therefore 
>> should run corretly for 64-bit AARCH64.
>> I will consider changing it as part of good partice and better portability "
>> I will give a primary plan to modified it.
>> for example:
>> *obj = (u32)find_next_zero_bit(bitmap->table, bitmap->max, bitmap->last);
>> Beause the max size of bitmap->table is u32 in current version.
>>
>> int hns_roce_bitmap_init(struct hns_roce_bitmap *bitmap, u32 num, u32 mask,
>>   u32 reserved_bot, u32 reserved_top)
>> {
>>  u32 i;
>>
>>  if (num != roundup_pow_of_two(num))
>>  return -EINVAL;
>>
>>  bitmap->last = 0;
>>  bitmap->top = 0;
>>  bitmap->max = num - reserved_top;
>>  bitmap->mask = mask;
>>  bitmap->reserved_top = reserved_top;
>>  spin_lock_init(>lock);
>>  bitmap->table = kcalloc(BITS_TO_LONGS(bitmap->max), sizeof(long),
>>  GFP_KERNEL);
>>
>> Is this plan ok?
> 
> No,
> You are submitting new driver, please do it properly (without casting)
> from the beginning.
> 
oh, i see. I will give a better plan to modify it.

Thanks
Lijun Ou
>>
>> Thanks
>> Lijun Ou
>>
>> --
>> 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 v5 09/21] IB/hns: Add hca support

[oulijun]

On 2016/4/26 22:18, Leon Romanovsky wrote:
> On Tue, Apr 26, 2016 at 02:34:44PM +0800, oulijun wrote:
>> On 2016/4/24 15:54, Leon Romanovsky wrote:
>>> On Sat, Apr 23, 2016 at 06:26:47PM +0800, Lijun Ou wrote:
>>>> This patch mainly setup hca for RoCE. it will do a series of
>>>> initial works as follows:
>>>>   1. init uar table, allocate uar resource
>>>>   2. init pd table
>>>>   3. init cq table
>>>>   4. init mr table
>>>>   5. init qp table
>>>>
>>>> Signed-off-by: Lijun Ou 
>>>> Signed-off-by: Wei Hu(Xavier) 
>>>> ---
>>>>  drivers/infiniband/hw/hns/hns_roce_alloc.c  | 104 
>>>>  drivers/infiniband/hw/hns/hns_roce_cq.c |  25 
>>>>  drivers/infiniband/hw/hns/hns_roce_device.h |  69 ++
>>>>  drivers/infiniband/hw/hns/hns_roce_eq.c |   1 -
>>>>  drivers/infiniband/hw/hns/hns_roce_icm.c|  88 +
>>>>  drivers/infiniband/hw/hns/hns_roce_icm.h|   9 ++
>>>>  drivers/infiniband/hw/hns/hns_roce_main.c   |  79 
>>>>  drivers/infiniband/hw/hns/hns_roce_mr.c | 187 
>>>> 
>>>>  drivers/infiniband/hw/hns/hns_roce_pd.c |  65 ++
>>>>  drivers/infiniband/hw/hns/hns_roce_qp.c |  30 +
>>>>  10 files changed, 656 insertions(+), 1 deletion(-)
>>>>  create mode 100644 drivers/infiniband/hw/hns/hns_roce_alloc.c
>>>>  create mode 100644 drivers/infiniband/hw/hns/hns_roce_mr.c
>>>>  create mode 100644 drivers/infiniband/hw/hns/hns_roce_pd.c
>>>>
>>>> diff --git a/drivers/infiniband/hw/hns/hns_roce_alloc.c 
>>>> b/drivers/infiniband/hw/hns/hns_roce_alloc.c
>>>> new file mode 100644
>>>> index 000..0c76f1b
>>>> --- /dev/null
>>>> +++ b/drivers/infiniband/hw/hns/hns_roce_alloc.c
>>>> @@ -0,0 +1,104 @@
>>>> +/*
>>>> + * Copyright (c) 2016 Hisilicon Limited.
>>>> + *
>>>> + * This program is free software; you can redistribute it and/or modify
>>>> + * it under the terms of the GNU General Public License as published by
>>>> + * the Free Software Foundation; either version 2 of the License, or
>>>> + * (at your option) any later version.
>>>> + */
>>>> +
>>>> +#include 
>>>> +#include 
>>>> +#include 
>>>> +#include 
>>>> +#include 
>>>> +#include 
>>>> +#include "hns_roce_device.h"
>>>> +
>>>> +int hns_roce_bitmap_alloc(struct hns_roce_bitmap *bitmap, u32 *obj)
>>>> +{
>>>> +  int ret = 0;
>>>> +
>>>> +  spin_lock(>lock);
>>>> +  *obj = find_next_zero_bit(bitmap->table, bitmap->max, bitmap->last);
>>>> +  if (*obj >= bitmap->max) {
>>>> +  bitmap->top = (bitmap->top + bitmap->max + bitmap->reserved_top)
>>>> + & bitmap->mask;
>>>> +  *obj = find_first_zero_bit(bitmap->table, bitmap->max);
>>>
>>> find_first_zero_bit function returns "unsigned long" which may or may
>>> not be equal to u32 on some architectures.
>>>
>> Hi Leon,
>> I appreciate your keen eye. this code is meant for ARM64bit therefore 
>> should run corretly for 64-bit AARCH64.
>> I will consider changing it as part of good partice and better portability "
>> I will give a primary plan to modified it.
>> for example:
>> *obj = (u32)find_next_zero_bit(bitmap->table, bitmap->max, bitmap->last);
>> Beause the max size of bitmap->table is u32 in current version.
>>
>> int hns_roce_bitmap_init(struct hns_roce_bitmap *bitmap, u32 num, u32 mask,
>>   u32 reserved_bot, u32 reserved_top)
>> {
>>  u32 i;
>>
>>  if (num != roundup_pow_of_two(num))
>>  return -EINVAL;
>>
>>  bitmap->last = 0;
>>  bitmap->top = 0;
>>  bitmap->max = num - reserved_top;
>>  bitmap->mask = mask;
>>  bitmap->reserved_top = reserved_top;
>>  spin_lock_init(>lock);
>>  bitmap->table = kcalloc(BITS_TO_LONGS(bitmap->max), sizeof(long),
>>  GFP_KERNEL);
>>
>> Is this plan ok?
> 
> No,
> You are submitting new driver, please do it properly (without casting)
> from the beginning.
> 
oh, i see. I will give a better plan to modify it.

Thanks
Lijun Ou
>>
>> Thanks
>> Lijun Ou
>>
>> --
>> 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 v5 09/21] IB/hns: Add hca support

[oulijun]

On 2016/4/26 22:25, Jiri Pirko wrote:
> Tue, Apr 26, 2016 at 04:18:21PM CEST, l...@kernel.org wrote:
>> On Tue, Apr 26, 2016 at 02:34:44PM +0800, oulijun wrote:
>>> On 2016/4/24 15:54, Leon Romanovsky wrote:
> 
> 
> 
>>>>> +int hns_roce_bitmap_alloc(struct hns_roce_bitmap *bitmap, u32 *obj)
>>>>> +{
>>>>> + int ret = 0;
>>>>> +
>>>>> + spin_lock(>lock);
>>>>> + *obj = find_next_zero_bit(bitmap->table, bitmap->max, bitmap->last);
>>>>> + if (*obj >= bitmap->max) {
>>>>> + bitmap->top = (bitmap->top + bitmap->max + bitmap->reserved_top)
>>>>> +& bitmap->mask;
>>>>> + *obj = find_first_zero_bit(bitmap->table, bitmap->max);
>>>>
>>>> find_first_zero_bit function returns "unsigned long" which may or may
>>>> not be equal to u32 on some architectures.
>>>>
>>> Hi Leon,
>>> I appreciate your keen eye. this code is meant for ARM64bit therefore 
>>> should run corretly for 64-bit AARCH64.
> 
> The driver should run correctly on any arch.
> 
 Hi Jiri Pirko,
Our driver run in ARM64 platform by depending on Kconfig. It will be 
configure in Kconfig file.

Thanks
Lijun Ou
> 
>>> I will consider changing it as part of good partice and better portability "
>>> I will give a primary plan to modified it.
>>> for example:
>>> *obj = (u32)find_next_zero_bit(bitmap->table, bitmap->max, 
>>> bitmap->last);
>>> Beause the max size of bitmap->table is u32 in current version.
>>>
>>> int hns_roce_bitmap_init(struct hns_roce_bitmap *bitmap, u32 num, u32 mask,
>>>  u32 reserved_bot, u32 reserved_top)
>>> {
>>> u32 i;
>>>
>>> if (num != roundup_pow_of_two(num))
>>> return -EINVAL;
>>>
>>> bitmap->last = 0;
>>> bitmap->top = 0;
>>> bitmap->max = num - reserved_top;
>>> bitmap->mask = mask;
>>> bitmap->reserved_top = reserved_top;
>>> spin_lock_init(>lock);
>>> bitmap->table = kcalloc(BITS_TO_LONGS(bitmap->max), sizeof(long),
>>> GFP_KERNEL);
>>>
>>> Is this plan ok?
>>
>> No,
>> You are submitting new driver, please do it properly (without casting)
>>from the beginning.
>>
>>>
>>> Thanks
>>> Lijun Ou
>>>
>>> --
>>> 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 v5 09/21] IB/hns: Add hca support

[oulijun]

On 2016/4/26 22:25, Jiri Pirko wrote:
> Tue, Apr 26, 2016 at 04:18:21PM CEST, l...@kernel.org wrote:
>> On Tue, Apr 26, 2016 at 02:34:44PM +0800, oulijun wrote:
>>> On 2016/4/24 15:54, Leon Romanovsky wrote:
> 
> 
> 
>>>>> +int hns_roce_bitmap_alloc(struct hns_roce_bitmap *bitmap, u32 *obj)
>>>>> +{
>>>>> + int ret = 0;
>>>>> +
>>>>> + spin_lock(>lock);
>>>>> + *obj = find_next_zero_bit(bitmap->table, bitmap->max, bitmap->last);
>>>>> + if (*obj >= bitmap->max) {
>>>>> + bitmap->top = (bitmap->top + bitmap->max + bitmap->reserved_top)
>>>>> +& bitmap->mask;
>>>>> + *obj = find_first_zero_bit(bitmap->table, bitmap->max);
>>>>
>>>> find_first_zero_bit function returns "unsigned long" which may or may
>>>> not be equal to u32 on some architectures.
>>>>
>>> Hi Leon,
>>> I appreciate your keen eye. this code is meant for ARM64bit therefore 
>>> should run corretly for 64-bit AARCH64.
> 
> The driver should run correctly on any arch.
> 
 Hi Jiri Pirko,
Our driver run in ARM64 platform by depending on Kconfig. It will be 
configure in Kconfig file.

Thanks
Lijun Ou
> 
>>> I will consider changing it as part of good partice and better portability "
>>> I will give a primary plan to modified it.
>>> for example:
>>> *obj = (u32)find_next_zero_bit(bitmap->table, bitmap->max, 
>>> bitmap->last);
>>> Beause the max size of bitmap->table is u32 in current version.
>>>
>>> int hns_roce_bitmap_init(struct hns_roce_bitmap *bitmap, u32 num, u32 mask,
>>>  u32 reserved_bot, u32 reserved_top)
>>> {
>>> u32 i;
>>>
>>> if (num != roundup_pow_of_two(num))
>>> return -EINVAL;
>>>
>>> bitmap->last = 0;
>>> bitmap->top = 0;
>>> bitmap->max = num - reserved_top;
>>> bitmap->mask = mask;
>>> bitmap->reserved_top = reserved_top;
>>> spin_lock_init(>lock);
>>> bitmap->table = kcalloc(BITS_TO_LONGS(bitmap->max), sizeof(long),
>>> GFP_KERNEL);
>>>
>>> Is this plan ok?
>>
>> No,
>> You are submitting new driver, please do it properly (without casting)
>>from the beginning.
>>
>>>
>>> Thanks
>>> Lijun Ou
>>>
>>> --
>>> 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 v5 09/21] IB/hns: Add hca support

[oulijun]

On 2016/4/24 15:54, Leon Romanovsky wrote:
> On Sat, Apr 23, 2016 at 06:26:47PM +0800, Lijun Ou wrote:
>> This patch mainly setup hca for RoCE. it will do a series of
>> initial works as follows:
>>   1. init uar table, allocate uar resource
>>   2. init pd table
>>   3. init cq table
>>   4. init mr table
>>   5. init qp table
>>
>> Signed-off-by: Lijun Ou 
>> Signed-off-by: Wei Hu(Xavier) 
>> ---
>>  drivers/infiniband/hw/hns/hns_roce_alloc.c  | 104 
>>  drivers/infiniband/hw/hns/hns_roce_cq.c |  25 
>>  drivers/infiniband/hw/hns/hns_roce_device.h |  69 ++
>>  drivers/infiniband/hw/hns/hns_roce_eq.c |   1 -
>>  drivers/infiniband/hw/hns/hns_roce_icm.c|  88 +
>>  drivers/infiniband/hw/hns/hns_roce_icm.h|   9 ++
>>  drivers/infiniband/hw/hns/hns_roce_main.c   |  79 
>>  drivers/infiniband/hw/hns/hns_roce_mr.c | 187 
>> 
>>  drivers/infiniband/hw/hns/hns_roce_pd.c |  65 ++
>>  drivers/infiniband/hw/hns/hns_roce_qp.c |  30 +
>>  10 files changed, 656 insertions(+), 1 deletion(-)
>>  create mode 100644 drivers/infiniband/hw/hns/hns_roce_alloc.c
>>  create mode 100644 drivers/infiniband/hw/hns/hns_roce_mr.c
>>  create mode 100644 drivers/infiniband/hw/hns/hns_roce_pd.c
>>
>> diff --git a/drivers/infiniband/hw/hns/hns_roce_alloc.c 
>> b/drivers/infiniband/hw/hns/hns_roce_alloc.c
>> new file mode 100644
>> index 000..0c76f1b
>> --- /dev/null
>> +++ b/drivers/infiniband/hw/hns/hns_roce_alloc.c
>> @@ -0,0 +1,104 @@
>> +/*
>> + * Copyright (c) 2016 Hisilicon Limited.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include "hns_roce_device.h"
>> +
>> +int hns_roce_bitmap_alloc(struct hns_roce_bitmap *bitmap, u32 *obj)
>> +{
>> +int ret = 0;
>> +
>> +spin_lock(>lock);
>> +*obj = find_next_zero_bit(bitmap->table, bitmap->max, bitmap->last);
>> +if (*obj >= bitmap->max) {
>> +bitmap->top = (bitmap->top + bitmap->max + bitmap->reserved_top)
>> +   & bitmap->mask;
>> +*obj = find_first_zero_bit(bitmap->table, bitmap->max);
> 
> find_first_zero_bit function returns "unsigned long" which may or may
> not be equal to u32 on some architectures.
> 
Hi Leon,
I appreciate your keen eye. this code is meant for ARM64bit therefore 
should run corretly for 64-bit AARCH64.
I will consider changing it as part of good partice and better portability "
I will give a primary plan to modified it.
for example:
*obj = (u32)find_next_zero_bit(bitmap->table, bitmap->max, bitmap->last);
Beause the max size of bitmap->table is u32 in current version.

int hns_roce_bitmap_init(struct hns_roce_bitmap *bitmap, u32 num, u32 mask,
 u32 reserved_bot, u32 reserved_top)
{
u32 i;

if (num != roundup_pow_of_two(num))
return -EINVAL;

bitmap->last = 0;
bitmap->top = 0;
bitmap->max = num - reserved_top;
bitmap->mask = mask;
bitmap->reserved_top = reserved_top;
spin_lock_init(>lock);
bitmap->table = kcalloc(BITS_TO_LONGS(bitmap->max), sizeof(long),
GFP_KERNEL);

Is this plan ok?

Thanks
Lijun Ou

Re: [PATCH v5 09/21] IB/hns: Add hca support

[oulijun]

On 2016/4/24 15:54, Leon Romanovsky wrote:
> On Sat, Apr 23, 2016 at 06:26:47PM +0800, Lijun Ou wrote:
>> This patch mainly setup hca for RoCE. it will do a series of
>> initial works as follows:
>>   1. init uar table, allocate uar resource
>>   2. init pd table
>>   3. init cq table
>>   4. init mr table
>>   5. init qp table
>>
>> Signed-off-by: Lijun Ou 
>> Signed-off-by: Wei Hu(Xavier) 
>> ---
>>  drivers/infiniband/hw/hns/hns_roce_alloc.c  | 104 
>>  drivers/infiniband/hw/hns/hns_roce_cq.c |  25 
>>  drivers/infiniband/hw/hns/hns_roce_device.h |  69 ++
>>  drivers/infiniband/hw/hns/hns_roce_eq.c |   1 -
>>  drivers/infiniband/hw/hns/hns_roce_icm.c|  88 +
>>  drivers/infiniband/hw/hns/hns_roce_icm.h|   9 ++
>>  drivers/infiniband/hw/hns/hns_roce_main.c   |  79 
>>  drivers/infiniband/hw/hns/hns_roce_mr.c | 187 
>> 
>>  drivers/infiniband/hw/hns/hns_roce_pd.c |  65 ++
>>  drivers/infiniband/hw/hns/hns_roce_qp.c |  30 +
>>  10 files changed, 656 insertions(+), 1 deletion(-)
>>  create mode 100644 drivers/infiniband/hw/hns/hns_roce_alloc.c
>>  create mode 100644 drivers/infiniband/hw/hns/hns_roce_mr.c
>>  create mode 100644 drivers/infiniband/hw/hns/hns_roce_pd.c
>>
>> diff --git a/drivers/infiniband/hw/hns/hns_roce_alloc.c 
>> b/drivers/infiniband/hw/hns/hns_roce_alloc.c
>> new file mode 100644
>> index 000..0c76f1b
>> --- /dev/null
>> +++ b/drivers/infiniband/hw/hns/hns_roce_alloc.c
>> @@ -0,0 +1,104 @@
>> +/*
>> + * Copyright (c) 2016 Hisilicon Limited.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include "hns_roce_device.h"
>> +
>> +int hns_roce_bitmap_alloc(struct hns_roce_bitmap *bitmap, u32 *obj)
>> +{
>> +int ret = 0;
>> +
>> +spin_lock(>lock);
>> +*obj = find_next_zero_bit(bitmap->table, bitmap->max, bitmap->last);
>> +if (*obj >= bitmap->max) {
>> +bitmap->top = (bitmap->top + bitmap->max + bitmap->reserved_top)
>> +   & bitmap->mask;
>> +*obj = find_first_zero_bit(bitmap->table, bitmap->max);
> 
> find_first_zero_bit function returns "unsigned long" which may or may
> not be equal to u32 on some architectures.
> 
Hi Leon,
I appreciate your keen eye. this code is meant for ARM64bit therefore 
should run corretly for 64-bit AARCH64.
I will consider changing it as part of good partice and better portability "
I will give a primary plan to modified it.
for example:
*obj = (u32)find_next_zero_bit(bitmap->table, bitmap->max, bitmap->last);
Beause the max size of bitmap->table is u32 in current version.

int hns_roce_bitmap_init(struct hns_roce_bitmap *bitmap, u32 num, u32 mask,
 u32 reserved_bot, u32 reserved_top)
{
u32 i;

if (num != roundup_pow_of_two(num))
return -EINVAL;

bitmap->last = 0;
bitmap->top = 0;
bitmap->max = num - reserved_top;
bitmap->mask = mask;
bitmap->reserved_top = reserved_top;
spin_lock_init(>lock);
bitmap->table = kcalloc(BITS_TO_LONGS(bitmap->max), sizeof(long),
GFP_KERNEL);

Is this plan ok?

Thanks
Lijun Ou

Re: [RESEND PATCH V4 2/3] IB/hns: Add HiSilicon RoCE driver support

[oulijun]

Hi,  Leon Romanovsky
On 2016/4/2 9:58, Leon Romanovsky wrote:
> On Fri, Apr 01, 2016 at 05:21:31PM +0800, Lijun Ou wrote:
>> The driver for HiSilicon RoCE is a platform driver.
>> The driver will support multiple versions of hardware. Currently only "v1"
>> for hip06 SoC is supported.
>> The driver includes two parts: common driver and hardware-specific
>> operations. hns_roce_v1_hw.c and hns_roce_v1_hw.h are files for
>> hardware-specific operations only for v1 engine, and other files(.c and .h)
>> for common algorithm and common hardware operations.
>>
>> Signed-off-by: Lijun Ou 
>> Signed-off-by: Wei Hu(Xavier) 
>> Signed-off-by: Znlong 
>> ---
>>  MAINTAINERS|8 +
>>  drivers/infiniband/Kconfig |1 +
>>  drivers/infiniband/hw/Makefile |1 +
>>  drivers/infiniband/hw/hisilicon/hns/Kconfig|   10 +
>>  drivers/infiniband/hw/hisilicon/hns/Makefile   |9 +
>>  drivers/infiniband/hw/hisilicon/hns/hns_roce_ah.c  |  110 +
> 
> We are not adding name of company (hisilicon) for infiniband HW drivers
> drivers/infiniband/hw/hisilicon/hns/hns_roce_ah.c
> --->
> drivers/infiniband/hw/hns/hns_roce_ah.c
>
Surely, i will modify the location of RoCE driver code after disscussed in next 
patch

> 
>>  .../infiniband/hw/hisilicon/hns/hns_roce_alloc.c   |  239 ++
>  ^^
> Please fix you paths.
> 
>>  drivers/infiniband/hw/hisilicon/hns/hns_roce_cmd.c |  338 +++
>>  drivers/infiniband/hw/hisilicon/hns/hns_roce_cmd.h |   80 +
>>  .../infiniband/hw/hisilicon/hns/hns_roce_common.h  |  308 +++
>>  drivers/infiniband/hw/hisilicon/hns/hns_roce_cq.c  |  436 +++
>>  .../infiniband/hw/hisilicon/hns/hns_roce_device.h  |  794 ++
>>  drivers/infiniband/hw/hisilicon/hns/hns_roce_eq.c  |  758 ++
>>  drivers/infiniband/hw/hisilicon/hns/hns_roce_eq.h  |  132 +
>>  drivers/infiniband/hw/hisilicon/hns/hns_roce_icm.c |  578 
>>  drivers/infiniband/hw/hisilicon/hns/hns_roce_icm.h |  112 +
>>  .../infiniband/hw/hisilicon/hns/hns_roce_main.c| 1097 
>>  drivers/infiniband/hw/hisilicon/hns/hns_roce_mr.c  |  605 +
>>  drivers/infiniband/hw/hisilicon/hns/hns_roce_pd.c  |  124 +
>>  drivers/infiniband/hw/hisilicon/hns/hns_roce_qp.c  |  841 ++
>>  .../infiniband/hw/hisilicon/hns/hns_roce_user.h|   31 +
>>  .../infiniband/hw/hisilicon/hns/hns_roce_v1_hw.c   | 2832 
>> 
>>  .../infiniband/hw/hisilicon/hns/hns_roce_v1_hw.h   |  985 +++
>   ^^
> Do you support v1 of RoCE or v1 of your HW?
> 
Here, v1 stands for hw, that is, we support v1 of our hw.
>>  23 files changed, 10429 insertions(+)
> 
> Please appreciate the effort needed to review such large patch and
> invest time and effort to divide this to number of small easy review patches.
> 
Surely, i have pay attention to the patch, but i consider that it is not 
better to
split the patch into small patch. because it will the base function of RoCE.
For your advice, i will make further efforts to taking a discussion how to 
reslove the question.

thanks
Lijun Ou
> .
> 

Re: [RESEND PATCH V4 2/3] IB/hns: Add HiSilicon RoCE driver support

[oulijun]

Hi,  Leon Romanovsky
On 2016/4/2 9:58, Leon Romanovsky wrote:
> On Fri, Apr 01, 2016 at 05:21:31PM +0800, Lijun Ou wrote:
>> The driver for HiSilicon RoCE is a platform driver.
>> The driver will support multiple versions of hardware. Currently only "v1"
>> for hip06 SoC is supported.
>> The driver includes two parts: common driver and hardware-specific
>> operations. hns_roce_v1_hw.c and hns_roce_v1_hw.h are files for
>> hardware-specific operations only for v1 engine, and other files(.c and .h)
>> for common algorithm and common hardware operations.
>>
>> Signed-off-by: Lijun Ou 
>> Signed-off-by: Wei Hu(Xavier) 
>> Signed-off-by: Znlong 
>> ---
>>  MAINTAINERS|8 +
>>  drivers/infiniband/Kconfig |1 +
>>  drivers/infiniband/hw/Makefile |1 +
>>  drivers/infiniband/hw/hisilicon/hns/Kconfig|   10 +
>>  drivers/infiniband/hw/hisilicon/hns/Makefile   |9 +
>>  drivers/infiniband/hw/hisilicon/hns/hns_roce_ah.c  |  110 +
> 
> We are not adding name of company (hisilicon) for infiniband HW drivers
> drivers/infiniband/hw/hisilicon/hns/hns_roce_ah.c
> --->
> drivers/infiniband/hw/hns/hns_roce_ah.c
>
Surely, i will modify the location of RoCE driver code after disscussed in next 
patch

> 
>>  .../infiniband/hw/hisilicon/hns/hns_roce_alloc.c   |  239 ++
>  ^^
> Please fix you paths.
> 
>>  drivers/infiniband/hw/hisilicon/hns/hns_roce_cmd.c |  338 +++
>>  drivers/infiniband/hw/hisilicon/hns/hns_roce_cmd.h |   80 +
>>  .../infiniband/hw/hisilicon/hns/hns_roce_common.h  |  308 +++
>>  drivers/infiniband/hw/hisilicon/hns/hns_roce_cq.c  |  436 +++
>>  .../infiniband/hw/hisilicon/hns/hns_roce_device.h  |  794 ++
>>  drivers/infiniband/hw/hisilicon/hns/hns_roce_eq.c  |  758 ++
>>  drivers/infiniband/hw/hisilicon/hns/hns_roce_eq.h  |  132 +
>>  drivers/infiniband/hw/hisilicon/hns/hns_roce_icm.c |  578 
>>  drivers/infiniband/hw/hisilicon/hns/hns_roce_icm.h |  112 +
>>  .../infiniband/hw/hisilicon/hns/hns_roce_main.c| 1097 
>>  drivers/infiniband/hw/hisilicon/hns/hns_roce_mr.c  |  605 +
>>  drivers/infiniband/hw/hisilicon/hns/hns_roce_pd.c  |  124 +
>>  drivers/infiniband/hw/hisilicon/hns/hns_roce_qp.c  |  841 ++
>>  .../infiniband/hw/hisilicon/hns/hns_roce_user.h|   31 +
>>  .../infiniband/hw/hisilicon/hns/hns_roce_v1_hw.c   | 2832 
>> 
>>  .../infiniband/hw/hisilicon/hns/hns_roce_v1_hw.h   |  985 +++
>   ^^
> Do you support v1 of RoCE or v1 of your HW?
> 
Here, v1 stands for hw, that is, we support v1 of our hw.
>>  23 files changed, 10429 insertions(+)
> 
> Please appreciate the effort needed to review such large patch and
> invest time and effort to divide this to number of small easy review patches.
> 
Surely, i have pay attention to the patch, but i consider that it is not 
better to
split the patch into small patch. because it will the base function of RoCE.
For your advice, i will make further efforts to taking a discussion how to 
reslove the question.

thanks
Lijun Ou
> .
> 

Re: call attention to review

[oulijun]

On 2016/3/24 14:17, Leon Romanovsky wrote:
> On Thu, Mar 24, 2016 at 01:50:30PM +0800, oulijun wrote:
>> Hi,
>>I am Lijun Ou. I have sent the PATCH v4 of HiSilicon RoCE driver at March 
>> 22, 2016.
>> if you are convenient, please help to review. Welcome to give your reviewing.
> 
> Hi Lijun,
> Please read whole document which describes how to submit patches [1]
> and especially section 9, but please don't limit yourself to that
> section only. The whole document is relevant to your patches.
> 
> [1] https://www.kernel.org/doc/Documentation/SubmittingPatches
> 
>>
>> thanks
>> Lijun Ou
>>
>> --
>> 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
> 
> .
> 
Sorry, please forgive my impatience.  I will pay attention to the whole 
document afterwards.
I am sorry  for my rashness again.

thanks
Lijun Ou

Re: call attention to review

[oulijun]

On 2016/3/24 14:17, Leon Romanovsky wrote:
> On Thu, Mar 24, 2016 at 01:50:30PM +0800, oulijun wrote:
>> Hi,
>>I am Lijun Ou. I have sent the PATCH v4 of HiSilicon RoCE driver at March 
>> 22, 2016.
>> if you are convenient, please help to review. Welcome to give your reviewing.
> 
> Hi Lijun,
> Please read whole document which describes how to submit patches [1]
> and especially section 9, but please don't limit yourself to that
> section only. The whole document is relevant to your patches.
> 
> [1] https://www.kernel.org/doc/Documentation/SubmittingPatches
> 
>>
>> thanks
>> Lijun Ou
>>
>> --
>> 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
> 
> .
> 
Sorry, please forgive my impatience.  I will pay attention to the whole 
document afterwards.
I am sorry  for my rashness again.

thanks
Lijun Ou

call attention to review

[oulijun]

Hi,
   I am Lijun Ou. I have sent the PATCH v4 of HiSilicon RoCE driver at March 
22, 2016.
if you are convenient, please help to review. Welcome to give your reviewing.

thanks
Lijun Ou

call attention to review

[oulijun]

Hi,
   I am Lijun Ou. I have sent the PATCH v4 of HiSilicon RoCE driver at March 
22, 2016.
if you are convenient, please help to review. Welcome to give your reviewing.

thanks
Lijun Ou

Re: [PATCH v4 2/3] IB/hns: Add HiSilicon RoCE driver support

[oulijun]

On 2016/3/23 2:54, Christoph Hellwig wrote:
>>  drivers/infiniband/Kconfig |1 +
>>  drivers/infiniband/hw/Makefile |1 +
>>  drivers/infiniband/hw/hisilicon/hns/Kconfig|   10 +
> 
> To fit in with the other drivers drop the hisilicon level
> of the directory hierarchy.
> 
> Haven't had time to look at the actual driver yet so far, though.
> 
> .
> 
Hi Christoph Hellwig,
   surely, I considered that other drivers will be put into the hisilicon
level of the directory hierarchy. These drivers will support RDMA function which
will be developed for a series of hisilicon chip. Because of the consideration, 
I drop the
level.
   In additon that, i thought that the patch of RoCE driver which supported the 
base function of RDMA is huge. However,
It is not splitted into patches in form of patchset. I don't put a better 
strategy that it neither make mistakes
of building and running nor huge.

thanks
Lijun Ou

Re: [PATCH v4 2/3] IB/hns: Add HiSilicon RoCE driver support

[oulijun]

On 2016/3/23 2:54, Christoph Hellwig wrote:
>>  drivers/infiniband/Kconfig |1 +
>>  drivers/infiniband/hw/Makefile |1 +
>>  drivers/infiniband/hw/hisilicon/hns/Kconfig|   10 +
> 
> To fit in with the other drivers drop the hisilicon level
> of the directory hierarchy.
> 
> Haven't had time to look at the actual driver yet so far, though.
> 
> .
> 
Hi Christoph Hellwig,
   surely, I considered that other drivers will be put into the hisilicon
level of the directory hierarchy. These drivers will support RDMA function which
will be developed for a series of hisilicon chip. Because of the consideration, 
I drop the
level.
   In additon that, i thought that the patch of RoCE driver which supported the 
base function of RDMA is huge. However,
It is not splitted into patches in form of patchset. I don't put a better 
strategy that it neither make mistakes
of building and running nor huge.

thanks
Lijun Ou

Re: [PATCH 3/4] infiniband: hns: add Hisilicon RoCE support(driver code)

[oulijun]

On 2016/3/15 2:20, Parav Pandit wrote:
>>>
>>> Since SRQ is not supported in this driver version, can you keep
>>> remaining code base also to not bother about SRQ specifically
>>> poll_cq_one, modify_qp, destroy_qp etc?
>>> SRQ support can come as complete additional patch along with cmd_mask,
>>> callbacks and rest of the code.
>>>
>>> .
>> Sorry, I see your review in time.
>> Sure, SRQ is not supported in current roce driver. I have verified the 
>> function
>> for RDMA. It is not influence. For your question, we need to analyse it 
>> scientific.
>> after that, i will reply your doubt, is that ok?
> 
> Yes. No problem.
> 
> .
> 
Hi, Parav Pandit
I have analyse and discuss with your reviewing. I considered that the srq 
is only the
condition branch in verbs and without independent function, so reserved it.I 
have delete the relative
function with srq independently.
if delete the branch operation with srq, it feel be inconvenient to 
understand

thanks
Lijun Ou

Re: [PATCH 3/4] infiniband: hns: add Hisilicon RoCE support(driver code)

[oulijun]

On 2016/3/15 2:20, Parav Pandit wrote:
>>>
>>> Since SRQ is not supported in this driver version, can you keep
>>> remaining code base also to not bother about SRQ specifically
>>> poll_cq_one, modify_qp, destroy_qp etc?
>>> SRQ support can come as complete additional patch along with cmd_mask,
>>> callbacks and rest of the code.
>>>
>>> .
>> Sorry, I see your review in time.
>> Sure, SRQ is not supported in current roce driver. I have verified the 
>> function
>> for RDMA. It is not influence. For your question, we need to analyse it 
>> scientific.
>> after that, i will reply your doubt, is that ok?
> 
> Yes. No problem.
> 
> .
> 
Hi, Parav Pandit
I have analyse and discuss with your reviewing. I considered that the srq 
is only the
condition branch in verbs and without independent function, so reserved it.I 
have delete the relative
function with srq independently.
if delete the branch operation with srq, it feel be inconvenient to 
understand

thanks
Lijun Ou

Re: [PATCH 2/3] net: hns: add Hisilicon RoCE support

[oulijun]

On 2016/3/14 14:49, Leon Romanovsky wrote:
> On Mon, Mar 14, 2016 at 09:12:28AM +0800, Yankejian (Hackim Yim) wrote:
>>
>>
>> On 2016/3/12 18:43, Leon Romanovsky wrote:
>>> On Fri, Mar 11, 2016 at 06:37:10PM +0800, Lijun Ou wrote:
 It added hns_dsaf_roce_reset routine for roce driver.
 RoCE is a feature of hns.
 In hip06 SOC, in roce reset process, it's needed to configure
 dsaf channel reset,port and sl map info.

 Signed-off-by: Lijun Ou 
 Signed-off-by: Wei Hu(Xavier) 
 Signed-off-by: Lisheng 
 ---
  drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c | 84 
 ++
  drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.h | 14 
  drivers/net/ethernet/hisilicon/hns/hns_dsaf_misc.c | 62 +---
  drivers/net/ethernet/hisilicon/hns/hns_dsaf_reg.h  | 13 
  4 files changed, 163 insertions(+), 10 deletions(-)

 diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c 
 b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c
 index 38fc5be..a0f0d4f 100644
 --- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c
 +++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c
 @@ -12,6 +12,7 @@
  #include 
  #include 
  #include 
 +#include 
  #include 
  #include 
  #include 
 @@ -2593,6 +2594,89 @@ static struct platform_driver g_dsaf_driver = {
  
  module_platform_driver(g_dsaf_driver);
  
 +/**
 + * hns_dsaf_roce_reset - reset dsaf and roce
 + * @dsaf_fwnode: Pointer to framework node for the dasf
 + * @val: 0 - request reset , 1 - drop reset
 + * retuen 0 - success , negative --fail
 + */
 +int hns_dsaf_roce_reset(struct fwnode_handle *dsaf_fwnode, u32 val)
 +{
 +  struct dsaf_device *dsaf_dev;
 +  struct platform_device *pdev;
 +  unsigned int mp;
 +  unsigned int sl;
 +  unsigned int credit;
 +  int i;
 +  const u32 port_map[DSAF_ROCE_CREDIT_CHN][DSAF_ROCE_CHAN_MODE] = {
 +  {0, 0, 0},
 +  {1, 0, 0},
 +  {2, 1, 0},
 +  {3, 1, 0},
 +  {4, 2, 1},
 +  {4, 2, 1},
 +  {5, 3, 1},
 +  {5, 3, 1},
 +  };
 +  const u32 sl_map[DSAF_ROCE_CREDIT_CHN][DSAF_ROCE_CHAN_MODE] = {
 +  {0, 0, 0},
 +  {0, 1, 1},
 +  {0, 0, 2},
 +  {0, 1, 3},
 +  {0, 0, 0},
 +  {1, 1, 1},
 +  {0, 0, 2},
 +  {1, 1, 3},
 +  };
>>> Do you have a plan to send a version with enums/defines for this
>>> numbers? Especially for _CHAN_MODE.
>>>
>>> .
>>
>> Hi leon,
>>
>> it seems the enums is added in hns_dsaf_main.h, as belows:
>>
>> diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.h 
>> b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.h
>> index 5fea226..c917b9a 100644
>> --- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.h
>> +++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.h
>> @@ -40,6 +40,16 @@ struct hns_mac_cb;
>>  #define DSAF_DUMP_REGS_NUM 504
>>  #define DSAF_STATIC_NUM 28
>>  
>> +#define DSAF_ROCE_CREDIT_CHN 8
>> +#define DSAF_ROCE_CHAN_MODE 3
>> +
>> +enum dsaf_roce_port_port_mode {
>> +DSAF_ROCE_6PORT_MODE,
>> +DSAF_ROCE_4PORT_MODE,
>> +DSAF_ROCE_2PORT_MODE,
>> +DSAF_ROCE_CHAN_MODE_NUM
>> +};
>> +
> 
> These defines are used as an index entry into si_map and port_map arrays
> and seems as not related to the actual data.
> 
> I suggest you to take this code back to drawing table, redesign it,
> clean unused functions and defines and resubmit it.
> 
> Thanks.
>

Hi Leon Romanovsky

   I have redesigned the port_map and sl_map structures. Please refer to the
hns_dsaf_main.c and hns_dsaf_main.h.

thanks
Lijun Ou

>>
>> MBR, Kejian
>>
>>
>>
>>
>>
>>
>> --
>> 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 2/3] net: hns: add Hisilicon RoCE support

[oulijun]

On 2016/3/14 14:49, Leon Romanovsky wrote:
> On Mon, Mar 14, 2016 at 09:12:28AM +0800, Yankejian (Hackim Yim) wrote:
>>
>>
>> On 2016/3/12 18:43, Leon Romanovsky wrote:
>>> On Fri, Mar 11, 2016 at 06:37:10PM +0800, Lijun Ou wrote:
 It added hns_dsaf_roce_reset routine for roce driver.
 RoCE is a feature of hns.
 In hip06 SOC, in roce reset process, it's needed to configure
 dsaf channel reset,port and sl map info.

 Signed-off-by: Lijun Ou 
 Signed-off-by: Wei Hu(Xavier) 
 Signed-off-by: Lisheng 
 ---
  drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c | 84 
 ++
  drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.h | 14 
  drivers/net/ethernet/hisilicon/hns/hns_dsaf_misc.c | 62 +---
  drivers/net/ethernet/hisilicon/hns/hns_dsaf_reg.h  | 13 
  4 files changed, 163 insertions(+), 10 deletions(-)

 diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c 
 b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c
 index 38fc5be..a0f0d4f 100644
 --- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c
 +++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c
 @@ -12,6 +12,7 @@
  #include 
  #include 
  #include 
 +#include 
  #include 
  #include 
  #include 
 @@ -2593,6 +2594,89 @@ static struct platform_driver g_dsaf_driver = {
  
  module_platform_driver(g_dsaf_driver);
  
 +/**
 + * hns_dsaf_roce_reset - reset dsaf and roce
 + * @dsaf_fwnode: Pointer to framework node for the dasf
 + * @val: 0 - request reset , 1 - drop reset
 + * retuen 0 - success , negative --fail
 + */
 +int hns_dsaf_roce_reset(struct fwnode_handle *dsaf_fwnode, u32 val)
 +{
 +  struct dsaf_device *dsaf_dev;
 +  struct platform_device *pdev;
 +  unsigned int mp;
 +  unsigned int sl;
 +  unsigned int credit;
 +  int i;
 +  const u32 port_map[DSAF_ROCE_CREDIT_CHN][DSAF_ROCE_CHAN_MODE] = {
 +  {0, 0, 0},
 +  {1, 0, 0},
 +  {2, 1, 0},
 +  {3, 1, 0},
 +  {4, 2, 1},
 +  {4, 2, 1},
 +  {5, 3, 1},
 +  {5, 3, 1},
 +  };
 +  const u32 sl_map[DSAF_ROCE_CREDIT_CHN][DSAF_ROCE_CHAN_MODE] = {
 +  {0, 0, 0},
 +  {0, 1, 1},
 +  {0, 0, 2},
 +  {0, 1, 3},
 +  {0, 0, 0},
 +  {1, 1, 1},
 +  {0, 0, 2},
 +  {1, 1, 3},
 +  };
>>> Do you have a plan to send a version with enums/defines for this
>>> numbers? Especially for _CHAN_MODE.
>>>
>>> .
>>
>> Hi leon,
>>
>> it seems the enums is added in hns_dsaf_main.h, as belows:
>>
>> diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.h 
>> b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.h
>> index 5fea226..c917b9a 100644
>> --- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.h
>> +++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.h
>> @@ -40,6 +40,16 @@ struct hns_mac_cb;
>>  #define DSAF_DUMP_REGS_NUM 504
>>  #define DSAF_STATIC_NUM 28
>>  
>> +#define DSAF_ROCE_CREDIT_CHN 8
>> +#define DSAF_ROCE_CHAN_MODE 3
>> +
>> +enum dsaf_roce_port_port_mode {
>> +DSAF_ROCE_6PORT_MODE,
>> +DSAF_ROCE_4PORT_MODE,
>> +DSAF_ROCE_2PORT_MODE,
>> +DSAF_ROCE_CHAN_MODE_NUM
>> +};
>> +
> 
> These defines are used as an index entry into si_map and port_map arrays
> and seems as not related to the actual data.
> 
> I suggest you to take this code back to drawing table, redesign it,
> clean unused functions and defines and resubmit it.
> 
> Thanks.
>

Hi Leon Romanovsky

   I have redesigned the port_map and sl_map structures. Please refer to the
hns_dsaf_main.c and hns_dsaf_main.h.

thanks
Lijun Ou

>>
>> MBR, Kejian
>>
>>
>>
>>
>>
>>
>> --
>> 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 3/3] infiniband: IB/hns: add Hisilicon RoCE support with bindings

[oulijun]

On 2016/3/11 21:39, Sergei Shtylyov wrote:
> Hello.
> 
> On 3/11/2016 1:37 PM, Lijun Ou wrote:
> 
>> This submit add binding file and dts file.
> 
>I see no .dts file.
> 
>>
>> Signed-off-by: Lijun Ou 
>> Signed-off-by: Wei Hu(Xavier) 
>> ---
>>   .../bindings/infiniband/hisilicon-hns-roce.txt | 68 
>> ++
>>   1 file changed, 68 insertions(+)
>>   create mode 100644 
>> Documentation/devicetree/bindings/infiniband/hisilicon-hns-roce.txt
>>
>> diff --git 
>> a/Documentation/devicetree/bindings/infiniband/hisilicon-hns-roce.txt 
>> b/Documentation/devicetree/bindings/infiniband/hisilicon-hns-roce.txt
>> new file mode 100644
>> index 000..8004641
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/infiniband/hisilicon-hns-roce.txt
>> @@ -0,0 +1,68 @@
>> +HiSilicon RoCE DT description
>> +
>> +HiSilicon RoCE engine is a part of network subsystem.
>> +It works depending on other part of network wubsytem, such as, gmac and
> 
>Subsystem.
> 
>> +dsa fabric.
>> +
>> +Additional properties are described here:
>> +
>> +Required properties:
>> +- compatible: Should contain "hisilicon,hns-roce-v1".
>> +- reg: Physical base address of the roce driver and
>> +length of memory mapped region.
>> +- eth-handle: phandle, specifies a reference to a node
>> +representing a ethernet device.
>> +- dsaf-handle: phandle, specifies a reference to a node
>> +representing a dsaf device.
>> +- #address-cells: must be 2
>> +- #size-cells: must be 2
>> +Optional properties:
>> +- dma-coherent: Present if DMA operations are coherent.
>> +- interrupt-parent: the interrupt parent of this device.
>> +- interrupts: should contain 32 completion event irq,1 async event irq
>> +and 1 event overflow irq.
> 
>The "interrupt-names" prop is strongly desired with some many IRQs.
> 
>> +Example:
>> +rocee@0xc400 {
> 
>The node names should be generic, not implementation specific.
> 
> [...]
> 
> MBR, Sergei
> 
> 
> .
> 
Hi Sergei Shtylyov , thanks for reviewing

I have modified it, I will send a new patch at soon

thanks
Lijun Ou

Re: [PATCH 3/3] infiniband: IB/hns: add Hisilicon RoCE support with bindings

[oulijun]

On 2016/3/11 21:39, Sergei Shtylyov wrote:
> Hello.
> 
> On 3/11/2016 1:37 PM, Lijun Ou wrote:
> 
>> This submit add binding file and dts file.
> 
>I see no .dts file.
> 
>>
>> Signed-off-by: Lijun Ou 
>> Signed-off-by: Wei Hu(Xavier) 
>> ---
>>   .../bindings/infiniband/hisilicon-hns-roce.txt | 68 
>> ++
>>   1 file changed, 68 insertions(+)
>>   create mode 100644 
>> Documentation/devicetree/bindings/infiniband/hisilicon-hns-roce.txt
>>
>> diff --git 
>> a/Documentation/devicetree/bindings/infiniband/hisilicon-hns-roce.txt 
>> b/Documentation/devicetree/bindings/infiniband/hisilicon-hns-roce.txt
>> new file mode 100644
>> index 000..8004641
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/infiniband/hisilicon-hns-roce.txt
>> @@ -0,0 +1,68 @@
>> +HiSilicon RoCE DT description
>> +
>> +HiSilicon RoCE engine is a part of network subsystem.
>> +It works depending on other part of network wubsytem, such as, gmac and
> 
>Subsystem.
> 
>> +dsa fabric.
>> +
>> +Additional properties are described here:
>> +
>> +Required properties:
>> +- compatible: Should contain "hisilicon,hns-roce-v1".
>> +- reg: Physical base address of the roce driver and
>> +length of memory mapped region.
>> +- eth-handle: phandle, specifies a reference to a node
>> +representing a ethernet device.
>> +- dsaf-handle: phandle, specifies a reference to a node
>> +representing a dsaf device.
>> +- #address-cells: must be 2
>> +- #size-cells: must be 2
>> +Optional properties:
>> +- dma-coherent: Present if DMA operations are coherent.
>> +- interrupt-parent: the interrupt parent of this device.
>> +- interrupts: should contain 32 completion event irq,1 async event irq
>> +and 1 event overflow irq.
> 
>The "interrupt-names" prop is strongly desired with some many IRQs.
> 
>> +Example:
>> +rocee@0xc400 {
> 
>The node names should be generic, not implementation specific.
> 
> [...]
> 
> MBR, Sergei
> 
> 
> .
> 
Hi Sergei Shtylyov , thanks for reviewing

I have modified it, I will send a new patch at soon

thanks
Lijun Ou

Re: [PATCH 0/3] infiniband: IB/hns: Hisilicon RoCE support

[oulijun]

On 2016/3/12 18:20, Leon Romanovsky wrote:
> On Fri, Mar 11, 2016 at 06:37:08PM +0800, Lijun Ou wrote:
> 
> 1) It is redundant to write "infiniband" and "IB" in one title to mention
> relevant subsystem, since it is the same.
> 
> Please take a look on the other submissions here on the list and use
> similar construction.
> 
> 2) Please use version number in the titles [PATCH vXXX]
> 
Hi dledford, thanks for reviewing

I have modified it, I will send a new patch at soon

thanks
Lijun Ou
>> The Hisilicon Network Substem(hns) is a long term evolution IP which is
>> supposed to be used in Hisilicon ICT SoC. RoCE is a feature of hns.
>> The driver for Hisilicon RoCE engine is a platform driver.
>> The driver will support mulitple versions of hns. Currently only "v1"
>> for hip06 SOC is supported.
>>
>>
>> Changes v1 -> v2:
>> 1. adjust the formats of roce driver code by the experts reviewing
>> 2. modify the bindings file with roce dts. add the attribute named 
>> interrput-names.
>> 3. modify the way of defining port mode in hns_dsaf_main.c
>> 4. move the Kconfig file into the hns directory and send it with roce
>> driver code file together.
>>
>>
>> Lijun Ou (3):
>>   infiniband: IB/hns: add Hisilicon RoCE support
>>   net: hns: add Hisilicon RoCE support
>>   infiniband: IB/hns: add Hisilicon RoCE support with bindings
>>
> 
> .
> 

Re: [PATCH 0/3] infiniband: IB/hns: Hisilicon RoCE support

[oulijun]

On 2016/3/12 18:20, Leon Romanovsky wrote:
> On Fri, Mar 11, 2016 at 06:37:08PM +0800, Lijun Ou wrote:
> 
> 1) It is redundant to write "infiniband" and "IB" in one title to mention
> relevant subsystem, since it is the same.
> 
> Please take a look on the other submissions here on the list and use
> similar construction.
> 
> 2) Please use version number in the titles [PATCH vXXX]
> 
Hi dledford, thanks for reviewing

I have modified it, I will send a new patch at soon

thanks
Lijun Ou
>> The Hisilicon Network Substem(hns) is a long term evolution IP which is
>> supposed to be used in Hisilicon ICT SoC. RoCE is a feature of hns.
>> The driver for Hisilicon RoCE engine is a platform driver.
>> The driver will support mulitple versions of hns. Currently only "v1"
>> for hip06 SOC is supported.
>>
>>
>> Changes v1 -> v2:
>> 1. adjust the formats of roce driver code by the experts reviewing
>> 2. modify the bindings file with roce dts. add the attribute named 
>> interrput-names.
>> 3. modify the way of defining port mode in hns_dsaf_main.c
>> 4. move the Kconfig file into the hns directory and send it with roce
>> driver code file together.
>>
>>
>> Lijun Ou (3):
>>   infiniband: IB/hns: add Hisilicon RoCE support
>>   net: hns: add Hisilicon RoCE support
>>   infiniband: IB/hns: add Hisilicon RoCE support with bindings
>>
> 
> .
> 

Re: [PATCH 1/3] infiniband: IB/hns: add Hisilicon RoCE support

[oulijun]

On 2016/3/17 14:43, Leon Romanovsky wrote:
> On Wed, Mar 16, 2016 at 11:36:38AM +0100, Jiri Pirko wrote:
>>> so, I continue to have it.
>>
>> I will continue to bash on your odd codingstyle. Please fix it!
> 
> Jiri,
> 
> Checkpatch errors is an easiest issue with this patch.
> 
> It is full of functions without use, unconnected macros and
> if you replace "hsi" to name of other well known driver, you will get
> same code :).
> 
> They need to redesign the whole driver before resubmission.
> 
> Thanks.
> 
> .
> 
Hi, Leon Romanovsky
Firstly, thanks for reviewing.
I have checked the patch v2, surely, some funtions without use and 
unconnected
macros exist. I have removed it according to analyse. I will leave few marcos 
as follow:
#define CQ_STATE_INVALID0
#define CQ_STATE_RESERV 1
#define CQ_STATE_VALID  2
#define CQ_STATE_ERR3

I thought that these are defined for hardware information. So, I reserved these 
macros.

In addtion, I didn't completly understand your review as below:
  if you replace "hsi" to name of other well known driver, you will get > same 
code :).

thanks
Lijun Ou

Re: [PATCH 1/3] infiniband: IB/hns: add Hisilicon RoCE support

[oulijun]

On 2016/3/17 14:43, Leon Romanovsky wrote:
> On Wed, Mar 16, 2016 at 11:36:38AM +0100, Jiri Pirko wrote:
>>> so, I continue to have it.
>>
>> I will continue to bash on your odd codingstyle. Please fix it!
> 
> Jiri,
> 
> Checkpatch errors is an easiest issue with this patch.
> 
> It is full of functions without use, unconnected macros and
> if you replace "hsi" to name of other well known driver, you will get
> same code :).
> 
> They need to redesign the whole driver before resubmission.
> 
> Thanks.
> 
> .
> 
Hi, Leon Romanovsky
Firstly, thanks for reviewing.
I have checked the patch v2, surely, some funtions without use and 
unconnected
macros exist. I have removed it according to analyse. I will leave few marcos 
as follow:
#define CQ_STATE_INVALID0
#define CQ_STATE_RESERV 1
#define CQ_STATE_VALID  2
#define CQ_STATE_ERR3

I thought that these are defined for hardware information. So, I reserved these 
macros.

In addtion, I didn't completly understand your review as below:
  if you replace "hsi" to name of other well known driver, you will get > same 
code :).

thanks
Lijun Ou

Re: [PATCH 1/3] infiniband: IB/hns: add Hisilicon RoCE support

[oulijun]

Hi dledford, thanks for reviewing.

I have modified according to your reivews. I will send a new patch
at soon.

thanks
Lijun Ou

On 2016/3/12 18:39, Leon Romanovsky wrote:
> On Fri, Mar 11, 2016 at 06:37:09PM +0800, Lijun Ou wrote:
>> The driver for Hisilicon RoCE is a platform driver.
>> The driver will support mulitple versions of hardware. Currently only "v1"
>> for hip06 SOC is supported.
>> The driver includes two parts: common driver and hardware-specific
>> operations. hns_roce_v1_hw.c and hns_roce_v1_hw.h are files for
>> hardware-specific operations only for v1 engine, and other files(.c and .h)
>> for common algorithm and common hardware operations
>>
>> Signed-off-by: Lijun Ou 
>> Signed-off-by: Wei Hu(Xavier) 
>> Signed-off-by: Znlong 
>> ---
>>  drivers/infiniband/Kconfig |2 +-
>>  drivers/infiniband/hw/Makefile |1 +
>>  drivers/infiniband/hw/hisilicon/hns/Kconfig|   10 +
>>  drivers/infiniband/hw/hisilicon/hns/Makefile   |9 +
>>  drivers/infiniband/hw/hisilicon/hns/hns_roce_ah.c  |  114 +
>>  .../infiniband/hw/hisilicon/hns/hns_roce_alloc.c   |  253 ++
>>  drivers/infiniband/hw/hisilicon/hns/hns_roce_cmd.c |  354 +++
>>  drivers/infiniband/hw/hisilicon/hns/hns_roce_cmd.h |  163 ++
>>  .../infiniband/hw/hisilicon/hns/hns_roce_common.h  |  704 +
>>  drivers/infiniband/hw/hisilicon/hns/hns_roce_cq.c  |  454 +++
>>  .../infiniband/hw/hisilicon/hns/hns_roce_device.h  |  840 ++
>>  drivers/infiniband/hw/hisilicon/hns/hns_roce_eq.c  |  798 ++
>>  drivers/infiniband/hw/hisilicon/hns/hns_roce_eq.h  |  138 +
>>  drivers/infiniband/hw/hisilicon/hns/hns_roce_icm.c |  608 
>>  drivers/infiniband/hw/hisilicon/hns/hns_roce_icm.h |  121 +
>>  .../infiniband/hw/hisilicon/hns/hns_roce_main.c| 1124 
>>  drivers/infiniband/hw/hisilicon/hns/hns_roce_mr.c  |  637 +
>>  drivers/infiniband/hw/hisilicon/hns/hns_roce_pd.c  |  129 +
>>  drivers/infiniband/hw/hisilicon/hns/hns_roce_qp.c  |  890 ++
>>  .../infiniband/hw/hisilicon/hns/hns_roce_user.h|   31 +
>>  .../infiniband/hw/hisilicon/hns/hns_roce_v1_hw.c   | 2992 
>> 
>>  .../infiniband/hw/hisilicon/hns/hns_roce_v1_hw.h   | 1068 +++
>>  22 files changed, 11439 insertions(+), 1 deletion(-)
>>  create mode 100644 drivers/infiniband/hw/hisilicon/hns/Kconfig
>>  create mode 100644 drivers/infiniband/hw/hisilicon/hns/Makefile
>>  create mode 100644 drivers/infiniband/hw/hisilicon/hns/hns_roce_ah.c
>>  create mode 100644 drivers/infiniband/hw/hisilicon/hns/hns_roce_alloc.c
>>  create mode 100644 drivers/infiniband/hw/hisilicon/hns/hns_roce_cmd.c
>>  create mode 100644 drivers/infiniband/hw/hisilicon/hns/hns_roce_cmd.h
>>  create mode 100644 drivers/infiniband/hw/hisilicon/hns/hns_roce_common.h
>>  create mode 100644 drivers/infiniband/hw/hisilicon/hns/hns_roce_cq.c
>>  create mode 100644 drivers/infiniband/hw/hisilicon/hns/hns_roce_device.h
>>  create mode 100644 drivers/infiniband/hw/hisilicon/hns/hns_roce_eq.c
>>  create mode 100644 drivers/infiniband/hw/hisilicon/hns/hns_roce_eq.h
>>  create mode 100644 drivers/infiniband/hw/hisilicon/hns/hns_roce_icm.c
>>  create mode 100644 drivers/infiniband/hw/hisilicon/hns/hns_roce_icm.h
>>  create mode 100644 drivers/infiniband/hw/hisilicon/hns/hns_roce_main.c
>>  create mode 100644 drivers/infiniband/hw/hisilicon/hns/hns_roce_mr.c
>>  create mode 100644 drivers/infiniband/hw/hisilicon/hns/hns_roce_pd.c
>>  create mode 100644 drivers/infiniband/hw/hisilicon/hns/hns_roce_qp.c
>>  create mode 100644 drivers/infiniband/hw/hisilicon/hns/hns_roce_user.h
>>  create mode 100644 drivers/infiniband/hw/hisilicon/hns/hns_roce_v1_hw.c
>>  create mode 100644 drivers/infiniband/hw/hisilicon/hns/hns_roce_v1_hw.h
>>
>> diff --git a/drivers/infiniband/Kconfig b/drivers/infiniband/Kconfig
>> index 8a8440c..c7a24bb 100644
>> --- a/drivers/infiniband/Kconfig
>> +++ b/drivers/infiniband/Kconfig
>> @@ -73,7 +73,7 @@ source "drivers/infiniband/hw/mlx5/Kconfig"
>>  source "drivers/infiniband/hw/nes/Kconfig"
>>  source "drivers/infiniband/hw/ocrdma/Kconfig"
>>  source "drivers/infiniband/hw/usnic/Kconfig"
>> -
> 
> No need to remove this line. It separates HW from ULPs.
> 
>> +source "drivers/infiniband/hw/hisilicon/hns/Kconfig"
>>  source "drivers/infiniband/ulp/ipoib/Kconfig"
>>  
>>  source "drivers/infiniband/ulp/srp/Kconfig"
> 
> 
> 
>> +int hns_roce_bitmap_alloc_range(
>> +struct hns_roce_bitmap *bitmap,
>> +int cnt, int align, u32 *obj)
> 
> You have indentation issues.
> 
>> +{
>> +int i;
>> +int ret = 0;
>> +
>> +if (likely(cnt == 1 && align == 1))
>> +return hns_roce_bitmap_alloc(bitmap, obj);
>> +
>> +spin_lock(>lock);
>> +
>> +*obj = bitmap_find_next_zero_area(bitmap->table, bitmap->max,
>> +  

Re: [PATCH 1/3] infiniband: IB/hns: add Hisilicon RoCE support

[oulijun]

Hi dledford, thanks for reviewing.

I have modified according to your reivews. I will send a new patch
at soon.

thanks
Lijun Ou

On 2016/3/12 18:39, Leon Romanovsky wrote:
> On Fri, Mar 11, 2016 at 06:37:09PM +0800, Lijun Ou wrote:
>> The driver for Hisilicon RoCE is a platform driver.
>> The driver will support mulitple versions of hardware. Currently only "v1"
>> for hip06 SOC is supported.
>> The driver includes two parts: common driver and hardware-specific
>> operations. hns_roce_v1_hw.c and hns_roce_v1_hw.h are files for
>> hardware-specific operations only for v1 engine, and other files(.c and .h)
>> for common algorithm and common hardware operations
>>
>> Signed-off-by: Lijun Ou 
>> Signed-off-by: Wei Hu(Xavier) 
>> Signed-off-by: Znlong 
>> ---
>>  drivers/infiniband/Kconfig |2 +-
>>  drivers/infiniband/hw/Makefile |1 +
>>  drivers/infiniband/hw/hisilicon/hns/Kconfig|   10 +
>>  drivers/infiniband/hw/hisilicon/hns/Makefile   |9 +
>>  drivers/infiniband/hw/hisilicon/hns/hns_roce_ah.c  |  114 +
>>  .../infiniband/hw/hisilicon/hns/hns_roce_alloc.c   |  253 ++
>>  drivers/infiniband/hw/hisilicon/hns/hns_roce_cmd.c |  354 +++
>>  drivers/infiniband/hw/hisilicon/hns/hns_roce_cmd.h |  163 ++
>>  .../infiniband/hw/hisilicon/hns/hns_roce_common.h  |  704 +
>>  drivers/infiniband/hw/hisilicon/hns/hns_roce_cq.c  |  454 +++
>>  .../infiniband/hw/hisilicon/hns/hns_roce_device.h  |  840 ++
>>  drivers/infiniband/hw/hisilicon/hns/hns_roce_eq.c  |  798 ++
>>  drivers/infiniband/hw/hisilicon/hns/hns_roce_eq.h  |  138 +
>>  drivers/infiniband/hw/hisilicon/hns/hns_roce_icm.c |  608 
>>  drivers/infiniband/hw/hisilicon/hns/hns_roce_icm.h |  121 +
>>  .../infiniband/hw/hisilicon/hns/hns_roce_main.c| 1124 
>>  drivers/infiniband/hw/hisilicon/hns/hns_roce_mr.c  |  637 +
>>  drivers/infiniband/hw/hisilicon/hns/hns_roce_pd.c  |  129 +
>>  drivers/infiniband/hw/hisilicon/hns/hns_roce_qp.c  |  890 ++
>>  .../infiniband/hw/hisilicon/hns/hns_roce_user.h|   31 +
>>  .../infiniband/hw/hisilicon/hns/hns_roce_v1_hw.c   | 2992 
>> 
>>  .../infiniband/hw/hisilicon/hns/hns_roce_v1_hw.h   | 1068 +++
>>  22 files changed, 11439 insertions(+), 1 deletion(-)
>>  create mode 100644 drivers/infiniband/hw/hisilicon/hns/Kconfig
>>  create mode 100644 drivers/infiniband/hw/hisilicon/hns/Makefile
>>  create mode 100644 drivers/infiniband/hw/hisilicon/hns/hns_roce_ah.c
>>  create mode 100644 drivers/infiniband/hw/hisilicon/hns/hns_roce_alloc.c
>>  create mode 100644 drivers/infiniband/hw/hisilicon/hns/hns_roce_cmd.c
>>  create mode 100644 drivers/infiniband/hw/hisilicon/hns/hns_roce_cmd.h
>>  create mode 100644 drivers/infiniband/hw/hisilicon/hns/hns_roce_common.h
>>  create mode 100644 drivers/infiniband/hw/hisilicon/hns/hns_roce_cq.c
>>  create mode 100644 drivers/infiniband/hw/hisilicon/hns/hns_roce_device.h
>>  create mode 100644 drivers/infiniband/hw/hisilicon/hns/hns_roce_eq.c
>>  create mode 100644 drivers/infiniband/hw/hisilicon/hns/hns_roce_eq.h
>>  create mode 100644 drivers/infiniband/hw/hisilicon/hns/hns_roce_icm.c
>>  create mode 100644 drivers/infiniband/hw/hisilicon/hns/hns_roce_icm.h
>>  create mode 100644 drivers/infiniband/hw/hisilicon/hns/hns_roce_main.c
>>  create mode 100644 drivers/infiniband/hw/hisilicon/hns/hns_roce_mr.c
>>  create mode 100644 drivers/infiniband/hw/hisilicon/hns/hns_roce_pd.c
>>  create mode 100644 drivers/infiniband/hw/hisilicon/hns/hns_roce_qp.c
>>  create mode 100644 drivers/infiniband/hw/hisilicon/hns/hns_roce_user.h
>>  create mode 100644 drivers/infiniband/hw/hisilicon/hns/hns_roce_v1_hw.c
>>  create mode 100644 drivers/infiniband/hw/hisilicon/hns/hns_roce_v1_hw.h
>>
>> diff --git a/drivers/infiniband/Kconfig b/drivers/infiniband/Kconfig
>> index 8a8440c..c7a24bb 100644
>> --- a/drivers/infiniband/Kconfig
>> +++ b/drivers/infiniband/Kconfig
>> @@ -73,7 +73,7 @@ source "drivers/infiniband/hw/mlx5/Kconfig"
>>  source "drivers/infiniband/hw/nes/Kconfig"
>>  source "drivers/infiniband/hw/ocrdma/Kconfig"
>>  source "drivers/infiniband/hw/usnic/Kconfig"
>> -
> 
> No need to remove this line. It separates HW from ULPs.
> 
>> +source "drivers/infiniband/hw/hisilicon/hns/Kconfig"
>>  source "drivers/infiniband/ulp/ipoib/Kconfig"
>>  
>>  source "drivers/infiniband/ulp/srp/Kconfig"
> 
> 
> 
>> +int hns_roce_bitmap_alloc_range(
>> +struct hns_roce_bitmap *bitmap,
>> +int cnt, int align, u32 *obj)
> 
> You have indentation issues.
> 
>> +{
>> +int i;
>> +int ret = 0;
>> +
>> +if (likely(cnt == 1 && align == 1))
>> +return hns_roce_bitmap_alloc(bitmap, obj);
>> +
>> +spin_lock(>lock);
>> +
>> +*obj = bitmap_find_next_zero_area(bitmap->table, bitmap->max,
>> +  bitmap->last, cnt, align - 1);
>> +if (*obj >= bitmap->max) {
>> +

Re: [PATCH 1/3] infiniband: IB/hns: add Hisilicon RoCE support

[oulijun]

Hi Jiri Pirko, thanks your reviewing.
sorry, I will send a new patch according to your reviews.

On 2016/3/11 18:42, Jiri Pirko wrote:
> Fri, Mar 11, 2016 at 11:37:09AM CET, ouli...@huawei.com wrote:
>> The driver for Hisilicon RoCE is a platform driver.
>> The driver will support mulitple versions of hardware. Currently only "v1"
>> for hip06 SOC is supported.
>> The driver includes two parts: common driver and hardware-specific
>> operations. hns_roce_v1_hw.c and hns_roce_v1_hw.h are files for
>> hardware-specific operations only for v1 engine, and other files(.c and .h)
>> for common algorithm and common hardware operations
>>
>> Signed-off-by: Lijun Ou 
>> Signed-off-by: Wei Hu(Xavier) 
>> Signed-off-by: Znlong 
> 
> 
> 
> I'm sorry to be nitpicking, but you still have style issues in your
> code. I believe that for newly submitted code, this should be avoided. I
> already pointed that out as a comment to your last version, but you
> ignored it. So again, couple of examples:
> 
>> +struct ib_ah *hns_roce_create_ah(struct ib_pd *ibpd,
>> +struct ib_ah_attr *ah_attr)
> 
> 
>   
>> +ret = ib_get_cached_gid(ibpd->device, ah_attr->port_num,
>> +ah_attr->grh.sgid_index, , _attr);
> 
> 
> 
>> +int hns_roce_bitmap_alloc_range(
>> +struct hns_roce_bitmap *bitmap,
>> +int cnt, int align, u32 *obj)
> 
> 
> 
>> +pages =
>> +kmalloc(sizeof(*pages) * buf->nbufs,
>> +GFP_KERNEL);
> 
In v2, I consider that it will violate checkpatch if write as follows
pages = kmalloc(sizeof(*pages) * buf->nbufs,
GFP_KERNEL);
so, I continue to have it.
Now, I have used kmalloc_array instead of it. I will send new patch at soon.
Again, i am sorry for my incorrect plan.

> 
> 
>> +dev_err(dev,
>> +"CQ alloc.Failed to find cq buf addr.\n");
> 
> 
> 
>> +resp.qp_tab_size  = hr_dev->caps.num_qps;
> 
> 
> 
>> +buddy->bits[i] =
>> +kmalloc(s * sizeof(long), GFP_KERNEL);
> 
> 
> and many, many others similar to this. Please fix this.
> 
> 
> Also, I don't understand why you have "_" prefix for labels:
> 
>> +
>> +_error_failed_register_device:
>> +hns_roce_engine_uninit(hr_dev);
>> +
>> +_error_failed_engine_init:
>> +hns_roce_cleanup_bitmap(hr_dev);
>> +
>> +_error_failed_setup_hca:
>> +hns_roce_cleanup_icm(hr_dev);
>> +
>> +_error_failed_init_icm:
>> +if (hr_dev->cmd_mod)
>> +hns_roce_cmd_use_polling(hr_dev);
>> +
>> +_error_failed_use_event:
>> +hns_roce_cleanup_eq_table(hr_dev);
>> +
>> +_error_failed_eq_tabel:
>> +hns_roce_cmd_cleanup(hr_dev);
>> +
>> +_error_failed_cmd_init:
>> +(void)hns_roce_engine_reset(hr_dev, 0);
>> +
>> +_error_failed_reset_engine:
>> +hns_roce_free_cfg(hr_dev);
>> +
>> +_error_failed_get_cfg:
>> +ib_dealloc_device(_dev->ib_dev);
>> +
>> +return ret;
>> +}
> 
> .
> 

Re: [PATCH 1/3] infiniband: IB/hns: add Hisilicon RoCE support

[oulijun]

Hi Jiri Pirko, thanks your reviewing.
sorry, I will send a new patch according to your reviews.

On 2016/3/11 18:42, Jiri Pirko wrote:
> Fri, Mar 11, 2016 at 11:37:09AM CET, ouli...@huawei.com wrote:
>> The driver for Hisilicon RoCE is a platform driver.
>> The driver will support mulitple versions of hardware. Currently only "v1"
>> for hip06 SOC is supported.
>> The driver includes two parts: common driver and hardware-specific
>> operations. hns_roce_v1_hw.c and hns_roce_v1_hw.h are files for
>> hardware-specific operations only for v1 engine, and other files(.c and .h)
>> for common algorithm and common hardware operations
>>
>> Signed-off-by: Lijun Ou 
>> Signed-off-by: Wei Hu(Xavier) 
>> Signed-off-by: Znlong 
> 
> 
> 
> I'm sorry to be nitpicking, but you still have style issues in your
> code. I believe that for newly submitted code, this should be avoided. I
> already pointed that out as a comment to your last version, but you
> ignored it. So again, couple of examples:
> 
>> +struct ib_ah *hns_roce_create_ah(struct ib_pd *ibpd,
>> +struct ib_ah_attr *ah_attr)
> 
> 
>   
>> +ret = ib_get_cached_gid(ibpd->device, ah_attr->port_num,
>> +ah_attr->grh.sgid_index, , _attr);
> 
> 
> 
>> +int hns_roce_bitmap_alloc_range(
>> +struct hns_roce_bitmap *bitmap,
>> +int cnt, int align, u32 *obj)
> 
> 
> 
>> +pages =
>> +kmalloc(sizeof(*pages) * buf->nbufs,
>> +GFP_KERNEL);
> 
In v2, I consider that it will violate checkpatch if write as follows
pages = kmalloc(sizeof(*pages) * buf->nbufs,
GFP_KERNEL);
so, I continue to have it.
Now, I have used kmalloc_array instead of it. I will send new patch at soon.
Again, i am sorry for my incorrect plan.

> 
> 
>> +dev_err(dev,
>> +"CQ alloc.Failed to find cq buf addr.\n");
> 
> 
> 
>> +resp.qp_tab_size  = hr_dev->caps.num_qps;
> 
> 
> 
>> +buddy->bits[i] =
>> +kmalloc(s * sizeof(long), GFP_KERNEL);
> 
> 
> and many, many others similar to this. Please fix this.
> 
> 
> Also, I don't understand why you have "_" prefix for labels:
> 
>> +
>> +_error_failed_register_device:
>> +hns_roce_engine_uninit(hr_dev);
>> +
>> +_error_failed_engine_init:
>> +hns_roce_cleanup_bitmap(hr_dev);
>> +
>> +_error_failed_setup_hca:
>> +hns_roce_cleanup_icm(hr_dev);
>> +
>> +_error_failed_init_icm:
>> +if (hr_dev->cmd_mod)
>> +hns_roce_cmd_use_polling(hr_dev);
>> +
>> +_error_failed_use_event:
>> +hns_roce_cleanup_eq_table(hr_dev);
>> +
>> +_error_failed_eq_tabel:
>> +hns_roce_cmd_cleanup(hr_dev);
>> +
>> +_error_failed_cmd_init:
>> +(void)hns_roce_engine_reset(hr_dev, 0);
>> +
>> +_error_failed_reset_engine:
>> +hns_roce_free_cfg(hr_dev);
>> +
>> +_error_failed_get_cfg:
>> +ib_dealloc_device(_dev->ib_dev);
>> +
>> +return ret;
>> +}
> 
> .
> 

Re: [PATCH 3/4] infiniband: hns: add Hisilicon RoCE support(driver code)

[oulijun]

Hi Parav Pandit, thanks your reviewing.
On 2016/3/4 17:37, Parav Pandit wrote:
> On Fri, Mar 4, 2016 at 2:11 PM, Wei Hu(Xavier)  
> wrote:
>> +
>> +int hns_roce_register_device(struct hns_roce_dev *hr_dev)
>> +{
>> +   int ret;
>> +   struct hns_roce_ib_iboe *iboe = NULL;
>> +   struct ib_device *ib_dev = NULL;
>> +   struct device *dev = _dev->pdev->dev;
>> +
>> +   iboe = _dev->iboe;
>> +
>> +   ib_dev = _dev->ib_dev;
>> +   strlcpy(ib_dev->name, "hisi_%d", IB_DEVICE_NAME_MAX);
>> +
>> +   ib_dev->owner   = THIS_MODULE;
>> +   ib_dev->node_type   = RDMA_NODE_IB_CA;
>> +   ib_dev->dma_device  = dev;
>> +
>> +   ib_dev->phys_port_cnt   = hr_dev->caps.num_ports;
>> +   ib_dev->local_dma_lkey  = hr_dev->caps.reserved_lkey;
>> +   ib_dev->num_comp_vectors= hr_dev->caps.num_comp_vectors;
>> +   ib_dev->uverbs_abi_ver  = 1;
>> +   ib_dev->uverbs_cmd_mask =
>> +   (1ULL << IB_USER_VERBS_CMD_GET_CONTEXT) |
>> +   (1ULL << IB_USER_VERBS_CMD_QUERY_DEVICE) |
>> +   (1ULL << IB_USER_VERBS_CMD_QUERY_PORT) |
>> +   (1ULL << IB_USER_VERBS_CMD_ALLOC_PD) |
>> +   (1ULL << IB_USER_VERBS_CMD_DEALLOC_PD) |
>> +   (1ULL << IB_USER_VERBS_CMD_REG_MR) |
>> +   (1ULL << IB_USER_VERBS_CMD_DEREG_MR) |
>> +   (1ULL << IB_USER_VERBS_CMD_CREATE_COMP_CHANNEL) |
>> +   (1ULL << IB_USER_VERBS_CMD_CREATE_CQ) |
>> +   (1ULL << IB_USER_VERBS_CMD_DESTROY_CQ) |
>> +   (1ULL << IB_USER_VERBS_CMD_CREATE_QP) |
>> +   (1ULL << IB_USER_VERBS_CMD_MODIFY_QP) |
>> +   (1ULL << IB_USER_VERBS_CMD_QUERY_QP) |
>> +   (1ULL << IB_USER_VERBS_CMD_DESTROY_QP);
>> +
> 
> Since SRQ is not supported in this driver version, can you keep
> remaining code base also to not bother about SRQ specifically
> poll_cq_one, modify_qp, destroy_qp etc?
> SRQ support can come as complete additional patch along with cmd_mask,
> callbacks and rest of the code.
> 
> .
Sorry, I see your review in time.
Sure, SRQ is not supported in current roce driver. I have verified the function
for RDMA. It is not influence. For your question, we need to analyse it 
scientific.
after that, i will reply your doubt, is that ok?

thanks
Lijun Ou

> 

Re: [PATCH 3/4] infiniband: hns: add Hisilicon RoCE support(driver code)

[oulijun]

Hi Parav Pandit, thanks your reviewing.
On 2016/3/4 17:37, Parav Pandit wrote:
> On Fri, Mar 4, 2016 at 2:11 PM, Wei Hu(Xavier)  
> wrote:
>> +
>> +int hns_roce_register_device(struct hns_roce_dev *hr_dev)
>> +{
>> +   int ret;
>> +   struct hns_roce_ib_iboe *iboe = NULL;
>> +   struct ib_device *ib_dev = NULL;
>> +   struct device *dev = _dev->pdev->dev;
>> +
>> +   iboe = _dev->iboe;
>> +
>> +   ib_dev = _dev->ib_dev;
>> +   strlcpy(ib_dev->name, "hisi_%d", IB_DEVICE_NAME_MAX);
>> +
>> +   ib_dev->owner   = THIS_MODULE;
>> +   ib_dev->node_type   = RDMA_NODE_IB_CA;
>> +   ib_dev->dma_device  = dev;
>> +
>> +   ib_dev->phys_port_cnt   = hr_dev->caps.num_ports;
>> +   ib_dev->local_dma_lkey  = hr_dev->caps.reserved_lkey;
>> +   ib_dev->num_comp_vectors= hr_dev->caps.num_comp_vectors;
>> +   ib_dev->uverbs_abi_ver  = 1;
>> +   ib_dev->uverbs_cmd_mask =
>> +   (1ULL << IB_USER_VERBS_CMD_GET_CONTEXT) |
>> +   (1ULL << IB_USER_VERBS_CMD_QUERY_DEVICE) |
>> +   (1ULL << IB_USER_VERBS_CMD_QUERY_PORT) |
>> +   (1ULL << IB_USER_VERBS_CMD_ALLOC_PD) |
>> +   (1ULL << IB_USER_VERBS_CMD_DEALLOC_PD) |
>> +   (1ULL << IB_USER_VERBS_CMD_REG_MR) |
>> +   (1ULL << IB_USER_VERBS_CMD_DEREG_MR) |
>> +   (1ULL << IB_USER_VERBS_CMD_CREATE_COMP_CHANNEL) |
>> +   (1ULL << IB_USER_VERBS_CMD_CREATE_CQ) |
>> +   (1ULL << IB_USER_VERBS_CMD_DESTROY_CQ) |
>> +   (1ULL << IB_USER_VERBS_CMD_CREATE_QP) |
>> +   (1ULL << IB_USER_VERBS_CMD_MODIFY_QP) |
>> +   (1ULL << IB_USER_VERBS_CMD_QUERY_QP) |
>> +   (1ULL << IB_USER_VERBS_CMD_DESTROY_QP);
>> +
> 
> Since SRQ is not supported in this driver version, can you keep
> remaining code base also to not bother about SRQ specifically
> poll_cq_one, modify_qp, destroy_qp etc?
> SRQ support can come as complete additional patch along with cmd_mask,
> callbacks and rest of the code.
> 
> .
Sorry, I see your review in time.
Sure, SRQ is not supported in current roce driver. I have verified the function
for RDMA. It is not influence. For your question, we need to analyse it 
scientific.
after that, i will reply your doubt, is that ok?

thanks
Lijun Ou

> 

Re: [PATCH 4/4] infiniband: hns: add Hisilicon RoCE support(Kconfig)

[oulijun]

Hi dledford, thanks your reviewing

On 2016/3/6 22:29, Leon Romanovsky wrote:
> On Fri, Mar 04, 2016 at 04:41:17PM +0800, Wei Hu(Xavier) wrote:
>> This submit add Kconfig.
> 
> It needs be part of previous patch.
We were discussed your review. We will accept it in new patchset.
It will be send soon.

thanks
Lijun Ou
> 
>>
>> Signed-off-by: Wei Hu(Xavier) <xavier.hu...@huawei.com>
>> Signed-off-by: oulijun <ouli...@huawei.com>
> 
> ^^^.. -- Is this first name or second name?
> 
>> ---
>>  drivers/infiniband/Kconfig | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/infiniband/Kconfig b/drivers/infiniband/Kconfig
>> index 8a8440c..c7a24bb 100644
>> --- a/drivers/infiniband/Kconfig
>> +++ b/drivers/infiniband/Kconfig
>> @@ -73,7 +73,7 @@ source "drivers/infiniband/hw/mlx5/Kconfig"
>>  source "drivers/infiniband/hw/nes/Kconfig"
>>  source "drivers/infiniband/hw/ocrdma/Kconfig"
>>  source "drivers/infiniband/hw/usnic/Kconfig"
>> -
>> +source "drivers/infiniband/hw/hisilicon/hns/Kconfig"
>>  source "drivers/infiniband/ulp/ipoib/Kconfig"
>>  
>>  source "drivers/infiniband/ulp/srp/Kconfig"
>> -- 
>> 1.9.1
>>
> 
> .
> 

Re: [PATCH 4/4] infiniband: hns: add Hisilicon RoCE support(Kconfig)

[oulijun]

Hi dledford, thanks your reviewing

On 2016/3/6 22:29, Leon Romanovsky wrote:
> On Fri, Mar 04, 2016 at 04:41:17PM +0800, Wei Hu(Xavier) wrote:
>> This submit add Kconfig.
> 
> It needs be part of previous patch.
We were discussed your review. We will accept it in new patchset.
It will be send soon.

thanks
Lijun Ou
> 
>>
>> Signed-off-by: Wei Hu(Xavier) 
>> Signed-off-by: oulijun 
> 
> ^^^.. -- Is this first name or second name?
> 
>> ---
>>  drivers/infiniband/Kconfig | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/infiniband/Kconfig b/drivers/infiniband/Kconfig
>> index 8a8440c..c7a24bb 100644
>> --- a/drivers/infiniband/Kconfig
>> +++ b/drivers/infiniband/Kconfig
>> @@ -73,7 +73,7 @@ source "drivers/infiniband/hw/mlx5/Kconfig"
>>  source "drivers/infiniband/hw/nes/Kconfig"
>>  source "drivers/infiniband/hw/ocrdma/Kconfig"
>>  source "drivers/infiniband/hw/usnic/Kconfig"
>> -
>> +source "drivers/infiniband/hw/hisilicon/hns/Kconfig"
>>  source "drivers/infiniband/ulp/ipoib/Kconfig"
>>  
>>  source "drivers/infiniband/ulp/srp/Kconfig"
>> -- 
>> 1.9.1
>>
> 
> .
> 

Re: [PATCH 1/4] net: hns: add Hisilicon RoCE support(the dependent routine)

[oulijun]

Hi, dledford
thanks your reviewing.
I will send a new patch soon.

thanks
Lijun Ou
On 2016/3/6 22:25, Leon Romanovsky wrote:
> Please rewrite your title to be without (...).
> 
> On Fri, Mar 04, 2016 at 04:41:14PM +0800, Wei Hu(Xavier) wrote:
>> It added hns_dsaf_roce_reset routine for roce driver.
>> RoCE is a feature of hns.
>> In hip06 SOC, in roce reset process, it's needed to configure
>> dsaf channel reset,port and sl map info.
>>
>> Signed-off-by: Wei Hu(Xavier) <xavier.hu...@huawei.com>
>> Signed-off-by: Lisheng <lisheng...@huawei.com>
>> Signed-off-by: oulijun <ouli...@huawei.com>
>> ---
>>  drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c | 82 
>> ++
>>  drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.h |  7 ++
>>  drivers/net/ethernet/hisilicon/hns/hns_dsaf_misc.c | 62 +---
>>  drivers/net/ethernet/hisilicon/hns/hns_dsaf_reg.h  | 14 
>>  4 files changed, 155 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c 
>> b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c
>> index 9439f04..41ba948 100644
>> --- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c
>> +++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c
>> @@ -12,6 +12,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>  #include 
>>  #include 
>>  #include 
>> @@ -2556,6 +2557,87 @@ static struct platform_driver g_dsaf_driver = {
>>  
>>  module_platform_driver(g_dsaf_driver);
>>  
>> +/**
>> + * hns_dsaf_roce_reset - reset dsaf and roce
>> + * @dsaf_fwnode: Pointer to framework node for the dasf
>> + * @val: 0 - request reset , 1 - drop reset
>> + * retuen 0 - success , negative --fail
>> + */
>> +int hns_dsaf_roce_reset(struct fwnode_handle *dsaf_fwnode, u32 val)
>> +{
>> +struct dsaf_device *dsaf_dev;
>> +struct platform_device *pdev;
>> +unsigned int mp;
>> +unsigned int sl;
>> +unsigned int credit;
>> +int i;
>> +const u32 port_map[DSAF_ROCE_CREDIT_CHN][DSAF_ROCE_CHAN_MODE] = {
>> +{0, 0, 0},
>> +{1, 0, 0},
>> +{2, 1, 0},
>> +{3, 1, 0},
>> +{4, 2, 1},
>> +{4, 2, 1},
>> +{5, 3, 1},
>> +{5, 3, 1},
>> +};
>> +const u32 sl_map[DSAF_ROCE_CREDIT_CHN][DSAF_ROCE_CHAN_MODE] = {
>> +{0, 0, 0},
>> +{0, 1, 1},
>> +{0, 0, 2},
>> +{0, 1, 3},
>> +{0, 0, 0},
>> +{1, 1, 1},
>> +{0, 0, 2},
>> +{1, 1, 3},
>> +};
> 
> Please prefer enums/defines instead of hard coded values.
> it is applicable to whole submitted code.
> 
> 
> .
> 

Re: [PATCH 1/4] net: hns: add Hisilicon RoCE support(the dependent routine)

[oulijun]

Hi, dledford
thanks your reviewing.
I will send a new patch soon.

thanks
Lijun Ou
On 2016/3/6 22:25, Leon Romanovsky wrote:
> Please rewrite your title to be without (...).
> 
> On Fri, Mar 04, 2016 at 04:41:14PM +0800, Wei Hu(Xavier) wrote:
>> It added hns_dsaf_roce_reset routine for roce driver.
>> RoCE is a feature of hns.
>> In hip06 SOC, in roce reset process, it's needed to configure
>> dsaf channel reset,port and sl map info.
>>
>> Signed-off-by: Wei Hu(Xavier) 
>> Signed-off-by: Lisheng 
>> Signed-off-by: oulijun 
>> ---
>>  drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c | 82 
>> ++
>>  drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.h |  7 ++
>>  drivers/net/ethernet/hisilicon/hns/hns_dsaf_misc.c | 62 +---
>>  drivers/net/ethernet/hisilicon/hns/hns_dsaf_reg.h  | 14 
>>  4 files changed, 155 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c 
>> b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c
>> index 9439f04..41ba948 100644
>> --- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c
>> +++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c
>> @@ -12,6 +12,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>  #include 
>>  #include 
>>  #include 
>> @@ -2556,6 +2557,87 @@ static struct platform_driver g_dsaf_driver = {
>>  
>>  module_platform_driver(g_dsaf_driver);
>>  
>> +/**
>> + * hns_dsaf_roce_reset - reset dsaf and roce
>> + * @dsaf_fwnode: Pointer to framework node for the dasf
>> + * @val: 0 - request reset , 1 - drop reset
>> + * retuen 0 - success , negative --fail
>> + */
>> +int hns_dsaf_roce_reset(struct fwnode_handle *dsaf_fwnode, u32 val)
>> +{
>> +struct dsaf_device *dsaf_dev;
>> +struct platform_device *pdev;
>> +unsigned int mp;
>> +unsigned int sl;
>> +unsigned int credit;
>> +int i;
>> +const u32 port_map[DSAF_ROCE_CREDIT_CHN][DSAF_ROCE_CHAN_MODE] = {
>> +{0, 0, 0},
>> +{1, 0, 0},
>> +{2, 1, 0},
>> +{3, 1, 0},
>> +{4, 2, 1},
>> +{4, 2, 1},
>> +{5, 3, 1},
>> +{5, 3, 1},
>> +};
>> +const u32 sl_map[DSAF_ROCE_CREDIT_CHN][DSAF_ROCE_CHAN_MODE] = {
>> +{0, 0, 0},
>> +{0, 1, 1},
>> +{0, 0, 2},
>> +{0, 1, 3},
>> +{0, 0, 0},
>> +{1, 1, 1},
>> +{0, 0, 2},
>> +{1, 1, 3},
>> +};
> 
> Please prefer enums/defines instead of hard coded values.
> it is applicable to whole submitted code.
> 
> 
> .
> 

Re: [PATCH 3/4] infiniband: hns: add Hisilicon RoCE support(driver code)

[oulijun]

Hi Jiri Pirko, thanks for reviewing
On 2016/3/4 17:16, Jiri Pirko wrote:
> Fri, Mar 04, 2016 at 09:41:16AM CET, xavier.hu...@huawei.com wrote:
> 
> 
> 
>> +int hns_roce_buf_alloc(
>> +struct hns_roce_dev *hr_dev,
>> +int size, int max_direct,
>> +struct hns_roce_buf *buf)
> 
> 
>   
>> +
>> +pages =
>> +kmalloc(sizeof(*pages) * buf->nbufs,
>> +GFP_KERNEL);
> 
> 
> 
>> +
>> +buf->direct.buf = vmap(
>> +pages, buf->nbufs, VM_MAP,
>> +PAGE_KERNEL);
> 
> 
> 
>> +if (
>> +event_type != HNS_ROCE_EVENT_TYPE_CQ_ID_INVALID &&
>> +event_type != HNS_ROCE_EVENT_TYPE_CQ_ACCESS_ERROR &&
>> +event_type != HNS_ROCE_EVENT_TYPE_CQ_OVERFLOW) {
>> +dev_err(_dev->pdev->dev,
>> +"hns_roce_ib: Unexpected event type 0x%x on CQ %06x\n",
>> +event_type, hr_cq->cqn);
>> +return;
>> +}
> 
> Although checkpatch does not complain, I find this semi-random adding of
> newlines quite odd.
> 
Really, the question you mentioned exit in many location in currently 
patch. I done it
in order to make it complain checkpatch and linux norms. Now, I have checked 
and adjust it
properly combined to checkpatch
I will send a new patch in future. if not modified in some locations, it 
have to violate
checkpatch once modified and is unable to adjust it better. About these,  have 
you best strategy?

Thanks
Lijun Ou
> .
> 

Re: [PATCH 3/4] infiniband: hns: add Hisilicon RoCE support(driver code)

[oulijun]

Hi Jiri Pirko, thanks for reviewing
On 2016/3/4 17:16, Jiri Pirko wrote:
> Fri, Mar 04, 2016 at 09:41:16AM CET, xavier.hu...@huawei.com wrote:
> 
> 
> 
>> +int hns_roce_buf_alloc(
>> +struct hns_roce_dev *hr_dev,
>> +int size, int max_direct,
>> +struct hns_roce_buf *buf)
> 
> 
>   
>> +
>> +pages =
>> +kmalloc(sizeof(*pages) * buf->nbufs,
>> +GFP_KERNEL);
> 
> 
> 
>> +
>> +buf->direct.buf = vmap(
>> +pages, buf->nbufs, VM_MAP,
>> +PAGE_KERNEL);
> 
> 
> 
>> +if (
>> +event_type != HNS_ROCE_EVENT_TYPE_CQ_ID_INVALID &&
>> +event_type != HNS_ROCE_EVENT_TYPE_CQ_ACCESS_ERROR &&
>> +event_type != HNS_ROCE_EVENT_TYPE_CQ_OVERFLOW) {
>> +dev_err(_dev->pdev->dev,
>> +"hns_roce_ib: Unexpected event type 0x%x on CQ %06x\n",
>> +event_type, hr_cq->cqn);
>> +return;
>> +}
> 
> Although checkpatch does not complain, I find this semi-random adding of
> newlines quite odd.
> 
Really, the question you mentioned exit in many location in currently 
patch. I done it
in order to make it complain checkpatch and linux norms. Now, I have checked 
and adjust it
properly combined to checkpatch
I will send a new patch in future. if not modified in some locations, it 
have to violate
checkpatch once modified and is unable to adjust it better. About these,  have 
you best strategy?

Thanks
Lijun Ou
> .
> 

Re: [PATCH 2/4] infiniband: hns: add Hisilicon RoCE support(binding)

[oulijun]

Hi Sergei Shtylyov, thanks for reviewing
I have modified the binding referred to your advice and the bindings of other 
module.
I will send a new patch in future.

Thanks
Lijun Ou

On 2016/3/4 21:51, Sergei Shtylyov wrote:
> Hello.
> 
> On 3/4/2016 11:41 AM, Wei Hu(Xavier) wrote:
> 
>> This submit add binding file and dts file.
>>
>> Signed-off-by: Wei Hu(Xavier) <xavier.hu...@huawei.com>
>> Signed-off-by: oulijun <ouli...@huawei.com>
>> ---
>>   .../bindings/infiniband/hisilicon-hns-roce.txt | 68 
>> ++
>>   1 file changed, 68 insertions(+)
>>   create mode 100644 
>> Documentation/devicetree/bindings/infiniband/hisilicon-hns-roce.txt
>>
>> diff --git 
>> a/Documentation/devicetree/bindings/infiniband/hisilicon-hns-roce.txt 
>> b/Documentation/devicetree/bindings/infiniband/hisilicon-hns-roce.txt
>> new file mode 100644
>> index 000..8004641
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/infiniband/hisilicon-hns-roce.txt
>> @@ -0,0 +1,68 @@
>> +HiSilicon RoCE DT description
>> +
>> +HiSilicon RoCE engine is a part of network subsystem.
>> +It works depending on other part of network wubsytem, such as, gmac and
>> +dsa fabric.
>> +
>> +Additional properties are described here:
>> +
>> +Required properties:
>> +- compatible: Should contain "hisilicon,hns-roce-v1".
>> +- reg: Physical base address of the roce driver and
>> +length of memory mapped region.
>> +- eth-handle: phandle, specifies a reference to a node
>> +representing a ethernet device.
>> +- dsaf-handle: phandle, specifies a reference to a node
>> +representing a dsaf device.
>> +- #address-cells: must be 2
>> +- #size-cells: must be 2
>> +Optional properties:
>> +- dma-coherent: Present if DMA operations are coherent.
>> +- interrupt-parent: the interrupt parent of this device.
>> +- interrupts: should contain 32 completion event irq,1 async event irq
>> +and 1 event overflow irq.
> 
>The "interrupt-names" prop is very desirable for so many interrupts.
> 
>> +Example:
>> +rocee@0xc400 {
> 
>The node names should be generic and "0x" should be omitted, i.e. 
> "infiniband@c400".
> 
> MBR, Sergei
> 
> 
> .
> 

Re: [PATCH 2/4] infiniband: hns: add Hisilicon RoCE support(binding)

[oulijun]

Hi Sergei Shtylyov, thanks for reviewing
I have modified the binding referred to your advice and the bindings of other 
module.
I will send a new patch in future.

Thanks
Lijun Ou

On 2016/3/4 21:51, Sergei Shtylyov wrote:
> Hello.
> 
> On 3/4/2016 11:41 AM, Wei Hu(Xavier) wrote:
> 
>> This submit add binding file and dts file.
>>
>> Signed-off-by: Wei Hu(Xavier) 
>> Signed-off-by: oulijun 
>> ---
>>   .../bindings/infiniband/hisilicon-hns-roce.txt | 68 
>> ++
>>   1 file changed, 68 insertions(+)
>>   create mode 100644 
>> Documentation/devicetree/bindings/infiniband/hisilicon-hns-roce.txt
>>
>> diff --git 
>> a/Documentation/devicetree/bindings/infiniband/hisilicon-hns-roce.txt 
>> b/Documentation/devicetree/bindings/infiniband/hisilicon-hns-roce.txt
>> new file mode 100644
>> index 000..8004641
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/infiniband/hisilicon-hns-roce.txt
>> @@ -0,0 +1,68 @@
>> +HiSilicon RoCE DT description
>> +
>> +HiSilicon RoCE engine is a part of network subsystem.
>> +It works depending on other part of network wubsytem, such as, gmac and
>> +dsa fabric.
>> +
>> +Additional properties are described here:
>> +
>> +Required properties:
>> +- compatible: Should contain "hisilicon,hns-roce-v1".
>> +- reg: Physical base address of the roce driver and
>> +length of memory mapped region.
>> +- eth-handle: phandle, specifies a reference to a node
>> +representing a ethernet device.
>> +- dsaf-handle: phandle, specifies a reference to a node
>> +representing a dsaf device.
>> +- #address-cells: must be 2
>> +- #size-cells: must be 2
>> +Optional properties:
>> +- dma-coherent: Present if DMA operations are coherent.
>> +- interrupt-parent: the interrupt parent of this device.
>> +- interrupts: should contain 32 completion event irq,1 async event irq
>> +and 1 event overflow irq.
> 
>The "interrupt-names" prop is very desirable for so many interrupts.
> 
>> +Example:
>> +rocee@0xc400 {
> 
>The node names should be generic and "0x" should be omitted, i.e. 
> "infiniband@c400".
> 
> MBR, Sergei
> 
> 
> .
>