Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1355791 > unrolled thread

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

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/3] infiniband: IB/hns: Hisilicon RoCE support Lijun Ou <oulijun@huawei.com> - 2016-03-11 11:30 +0100
    Re: [PATCH 1/3] infiniband: IB/hns: add Hisilicon RoCE support Jiri Pirko <jiri@resnulli.us> - 2016-03-11 11:50 +0100
      Re: [PATCH 1/3] infiniband: IB/hns: add Hisilicon RoCE support Jiri Pirko <jiri@resnulli.us> - 2016-03-16 11:40 +0100
        Re: [PATCH 1/3] infiniband: IB/hns: add Hisilicon RoCE support Leon Romanovsky <leon@leon.nu> - 2016-03-17 07:50 +0100
      Re: [PATCH 1/3] infiniband: IB/hns: add Hisilicon RoCE support oulijun <oulijun@huawei.com> - 2016-03-16 11:40 +0100
    Re: [PATCH 0/3] infiniband: IB/hns: Hisilicon RoCE support Leon Romanovsky <leon@leon.nu> - 2016-03-12 12:00 +0100
    Re: [PATCH 1/3] infiniband: IB/hns: add Hisilicon RoCE support Leon Romanovsky <leon@leon.nu> - 2016-03-12 12:00 +0100
      Re: [PATCH 1/3] infiniband: IB/hns: add Hisilicon RoCE support oulijun <oulijun@huawei.com> - 2016-03-16 11:40 +0100

#1355791 — [PATCH 0/3] infiniband: IB/hns: Hisilicon RoCE support

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

 .../bindings/infiniband/hisilicon-hns-roce.txt     |   68 +
 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 +++++++
 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 +
 27 files changed, 11670 insertions(+), 11 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/infiniband/hisilicon-hns-roce.txt
 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

-- 
1.9.1

[toc] | [next] | [standalone]

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

Fri, Mar 11, 2016 at 11:37:09AM CET, oulijun@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 <oulijun@huawei.com>
>Signed-off-by: Wei Hu(Xavier) <xavier.huwei@huawei.com>
>Signed-off-by: Znlong <zhaonenglong@huawei.com>

<snip>

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)

<snip>	
	
>+	ret = ib_get_cached_gid(ibpd->device, ah_attr->port_num,
>+			ah_attr->grh.sgid_index, &sgid, &gid_attr);

<snip>

>+int hns_roce_bitmap_alloc_range(
>+					struct hns_roce_bitmap *bitmap,
>+					int cnt, int align, u32 *obj)

<snip>

>+			pages =
>+				kmalloc(sizeof(*pages) * buf->nbufs,
>+					GFP_KERNEL);

<snip>

>+		dev_err(dev,
>+			"CQ alloc.Failed to find cq buf addr.\n");

<snip>

>+	resp.qp_tab_size      = hr_dev->caps.num_qps;

<snip>

>+		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(&hr_dev->ib_dev);
>+
>+	return ret;
>+}

[toc] | [prev] | [next] | [standalone]

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

Wed, Mar 16, 2016 at 11:23:10AM CET, oulijun@huawei.com wrote:
>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, oulijun@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 <oulijun@huawei.com>
>>> Signed-off-by: Wei Hu(Xavier) <xavier.huwei@huawei.com>
>>> Signed-off-by: Znlong <zhaonenglong@huawei.com>
>> 
>> <snip>
>> 
>> 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)
>> 
>> <snip>	
>> 	
>>> +	ret = ib_get_cached_gid(ibpd->device, ah_attr->port_num,
>>> +			ah_attr->grh.sgid_index, &sgid, &gid_attr);
>> 
>> <snip>
>> 
>>> +int hns_roce_bitmap_alloc_range(
>>> +					struct hns_roce_bitmap *bitmap,
>>> +					int cnt, int align, u32 *obj)
>> 
>> <snip>
>> 
>>> +			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);

Why it would be problem for checkpatch? I bet it won't.


>so, I continue to have it.

I will continue to bash on your odd codingstyle. Please fix 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.
>
>> <snip>
>> 
>>> +		dev_err(dev,
>>> +			"CQ alloc.Failed to find cq buf addr.\n");
>> 
>> <snip>
>> 
>>> +	resp.qp_tab_size      = hr_dev->caps.num_qps;
>> 
>> <snip>
>> 
>>> +		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(&hr_dev->ib_dev);
>>> +
>>> +	return ret;
>>> +}
>> 
>> .
>> 
>
>

[toc] | [prev] | [next] | [standalone]

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

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.

[toc] | [prev] | [next] | [standalone]

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

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, oulijun@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 <oulijun@huawei.com>
>> Signed-off-by: Wei Hu(Xavier) <xavier.huwei@huawei.com>
>> Signed-off-by: Znlong <zhaonenglong@huawei.com>
> 
> <snip>
> 
> 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)
> 
> <snip>	
> 	
>> +	ret = ib_get_cached_gid(ibpd->device, ah_attr->port_num,
>> +			ah_attr->grh.sgid_index, &sgid, &gid_attr);
> 
> <snip>
> 
>> +int hns_roce_bitmap_alloc_range(
>> +					struct hns_roce_bitmap *bitmap,
>> +					int cnt, int align, u32 *obj)
> 
> <snip>
> 
>> +			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.

> <snip>
> 
>> +		dev_err(dev,
>> +			"CQ alloc.Failed to find cq buf addr.\n");
> 
> <snip>
> 
>> +	resp.qp_tab_size      = hr_dev->caps.num_qps;
> 
> <snip>
> 
>> +		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(&hr_dev->ib_dev);
>> +
>> +	return ret;
>> +}
> 
> .
> 

[toc] | [prev] | [next] | [standalone]

#1356392

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]

> 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
> 

[toc] | [prev] | [next] | [standalone]

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

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 <oulijun@huawei.com>
> Signed-off-by: Wei Hu(Xavier) <xavier.huwei@huawei.com>
> Signed-off-by: Znlong <zhaonenglong@huawei.com>
> ---
>  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"

<snip>

> +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(&bitmap->lock);
> +
> +	*obj = bitmap_find_next_zero_area(bitmap->table, bitmap->max,
> +					  bitmap->last, cnt, align - 1);
> +	if (*obj >= bitmap->max) {
> +		bitmap->top = (bitmap->top + bitmap->max + bitmap->reserved_top)
> +			       & bitmap->mask;
> +		*obj = bitmap_find_next_zero_area(bitmap->table, bitmap->max, 0,
> +						  cnt, align - 1);
> +	}
> +
> +	if (*obj < bitmap->max) {
> +		for (i = 0; i < cnt; i++)
> +			set_bit(*obj + i, bitmap->table);
> +
> +		if (*obj == bitmap->last) {
> +			bitmap->last = (*obj + cnt);
> +			if (bitmap->last >= bitmap->max)
> +				bitmap->last = 0;
> +		}
> +		*obj |= bitmap->top;
> +	} else {
> +		ret = -1;
> +	}
> +
> +	spin_unlock(&bitmap->lock);
> +
> +	return ret;
> +}
> +
> +void hns_roce_bitmap_free_range(
> +					struct hns_roce_bitmap *bitmap,
> +					u32 obj,
> +					int cnt)
> +{
> +	int i;
> +
> +	obj &= bitmap->max + bitmap->reserved_top - 1;
> +
> +	spin_lock(&bitmap->lock);
> +	for (i = 0; i < cnt; i++)
> +		clear_bit(obj + i, bitmap->table);
> +
> +	bitmap->last = min(bitmap->last, obj);
> +	bitmap->top = (bitmap->top + bitmap->max + bitmap->reserved_top)
> +		       & bitmap->mask;
> +	spin_unlock(&bitmap->lock);
> +}
> +
> +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(&bitmap->lock);
> +	bitmap->table =
> +		kzalloc(BITS_TO_LONGS(bitmap->max) * sizeof(long), GFP_KERNEL);

Here and below, please consider to use kcalloc.

> +	if (!bitmap->table)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < reserved_bot; ++i)

[toc] | [prev] | [next] | [standalone]

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

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 <oulijun@huawei.com>
>> Signed-off-by: Wei Hu(Xavier) <xavier.huwei@huawei.com>
>> Signed-off-by: Znlong <zhaonenglong@huawei.com>
>> ---
>>  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"
> 
> <snip>
> 
>> +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(&bitmap->lock);
>> +
>> +	*obj = bitmap_find_next_zero_area(bitmap->table, bitmap->max,
>> +					  bitmap->last, cnt, align - 1);
>> +	if (*obj >= bitmap->max) {
>> +		bitmap->top = (bitmap->top + bitmap->max + bitmap->reserved_top)
>> +			       & bitmap->mask;
>> +		*obj = bitmap_find_next_zero_area(bitmap->table, bitmap->max, 0,
>> +						  cnt, align - 1);
>> +	}
>> +
>> +	if (*obj < bitmap->max) {
>> +		for (i = 0; i < cnt; i++)
>> +			set_bit(*obj + i, bitmap->table);
>> +
>> +		if (*obj == bitmap->last) {
>> +			bitmap->last = (*obj + cnt);
>> +			if (bitmap->last >= bitmap->max)
>> +				bitmap->last = 0;
>> +		}
>> +		*obj |= bitmap->top;
>> +	} else {
>> +		ret = -1;
>> +	}
>> +
>> +	spin_unlock(&bitmap->lock);
>> +
>> +	return ret;
>> +}
>> +
>> +void hns_roce_bitmap_free_range(
>> +					struct hns_roce_bitmap *bitmap,
>> +					u32 obj,
>> +					int cnt)
>> +{
>> +	int i;
>> +
>> +	obj &= bitmap->max + bitmap->reserved_top - 1;
>> +
>> +	spin_lock(&bitmap->lock);
>> +	for (i = 0; i < cnt; i++)
>> +		clear_bit(obj + i, bitmap->table);
>> +
>> +	bitmap->last = min(bitmap->last, obj);
>> +	bitmap->top = (bitmap->top + bitmap->max + bitmap->reserved_top)
>> +		       & bitmap->mask;
>> +	spin_unlock(&bitmap->lock);
>> +}
>> +
>> +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(&bitmap->lock);
>> +	bitmap->table =
>> +		kzalloc(BITS_TO_LONGS(bitmap->max) * sizeof(long), GFP_KERNEL);
> 
> Here and below, please consider to use kcalloc.
> 
>> +	if (!bitmap->table)
>> +		return -ENOMEM;
>> +
>> +	for (i = 0; i < reserved_bot; ++i)
> 
> .
> 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web