[PATCH 0/3] RDMA/iRDMA: Cleanups and improvements

[PATCH 0/3] RDMA/iRDMA: Cleanups and improvements

[Kamal Heib]

This series includes a few cleanup patches for the iRDMA driver.

Kamal Heib (3):
  RDMA/iRDMA: Return void from irdma_init_iw_device()
  RDMA/iRDMA: Return void from irdma_init_rdma_device()
  RDMA/iRDMA: Move iw device ops initialization

 drivers/infiniband/hw/irdma/verbs.c | 35 +++++++++++------------------
 1 file changed, 13 insertions(+), 22 deletions(-)

-- 
2.40.1

[PATCH 1/3] RDMA/iRDMA: Return void from irdma_init_iw_device()

[Kamal Heib]

The return value from irdma_init_iw_device() is always 0 - change it to be
void.

Signed-off-by: Kamal Heib <kheib@redhat.com>
---
 drivers/infiniband/hw/irdma/verbs.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/infiniband/hw/irdma/verbs.c b/drivers/infiniband/hw/irdma/verbs.c
index ab5cdf782785..b405cc961187 100644
--- a/drivers/infiniband/hw/irdma/verbs.c
+++ b/drivers/infiniband/hw/irdma/verbs.c
@@ -4515,7 +4515,7 @@ static void irdma_init_roce_device(struct irdma_device *iwdev)
  * irdma_init_iw_device - initialization of iwarp rdma device
  * @iwdev: irdma device
  */
-static int irdma_init_iw_device(struct irdma_device *iwdev)
+static void irdma_init_iw_device(struct irdma_device *iwdev)
 {
 	struct net_device *netdev = iwdev->netdev;
 
@@ -4533,8 +4533,6 @@ static int irdma_init_iw_device(struct irdma_device *iwdev)
 	memcpy(iwdev->ibdev.iw_ifname, netdev->name,
 	       sizeof(iwdev->ibdev.iw_ifname));
 	ib_set_device_ops(&iwdev->ibdev, &irdma_iw_dev_ops);
-
-	return 0;
 }
 
 /**
@@ -4544,14 +4542,11 @@ static int irdma_init_iw_device(struct irdma_device *iwdev)
 static int irdma_init_rdma_device(struct irdma_device *iwdev)
 {
 	struct pci_dev *pcidev = iwdev->rf->pcidev;
-	int ret;
 
 	if (iwdev->roce_mode) {
 		irdma_init_roce_device(iwdev);
 	} else {
-		ret = irdma_init_iw_device(iwdev);
-		if (ret)
-			return ret;
+		irdma_init_iw_device(iwdev);
 	}
 	iwdev->ibdev.phys_port_cnt = 1;
 	iwdev->ibdev.num_comp_vectors = iwdev->rf->ceqs_count;
-- 
2.40.1

[PATCH 2/3] RDMA/iRDMA: Return void from irdma_init_rdma_device()

[Kamal Heib]

The return value from irdma_init_rdma_device() is always 0 - change it to
be void.

Signed-off-by: Kamal Heib <kheib@redhat.com>
---
 drivers/infiniband/hw/irdma/verbs.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/infiniband/hw/irdma/verbs.c b/drivers/infiniband/hw/irdma/verbs.c
index b405cc961187..771f3abbed63 100644
--- a/drivers/infiniband/hw/irdma/verbs.c
+++ b/drivers/infiniband/hw/irdma/verbs.c
@@ -4539,7 +4539,7 @@ static void irdma_init_iw_device(struct irdma_device *iwdev)
  * irdma_init_rdma_device - initialization of rdma device
  * @iwdev: irdma device
  */
-static int irdma_init_rdma_device(struct irdma_device *iwdev)
+static void irdma_init_rdma_device(struct irdma_device *iwdev)
 {
 	struct pci_dev *pcidev = iwdev->rf->pcidev;
 
@@ -4552,8 +4552,6 @@ static int irdma_init_rdma_device(struct irdma_device *iwdev)
 	iwdev->ibdev.num_comp_vectors = iwdev->rf->ceqs_count;
 	iwdev->ibdev.dev.parent = &pcidev->dev;
 	ib_set_device_ops(&iwdev->ibdev, &irdma_dev_ops);
-
-	return 0;
 }
 
 /**
@@ -4591,9 +4589,7 @@ int irdma_ib_register_device(struct irdma_device *iwdev)
 {
 	int ret;
 
-	ret = irdma_init_rdma_device(iwdev);
-	if (ret)
-		return ret;
+	irdma_init_rdma_device(iwdev);
 
 	ret = ib_device_set_netdev(&iwdev->ibdev, iwdev->netdev, 1);
 	if (ret)
-- 
2.40.1

[PATCH 3/3] RDMA/iRDMA: Move iw device ops initialization

[Kamal Heib]

Move the initialization of the iw device ops to be under the declaration
of the irdma_iw_dev_ops.

Signed-off-by: Kamal Heib <kheib@redhat.com>
---
 drivers/infiniband/hw/irdma/verbs.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/infiniband/hw/irdma/verbs.c b/drivers/infiniband/hw/irdma/verbs.c
index 771f3abbed63..7f5dd0edd072 100644
--- a/drivers/infiniband/hw/irdma/verbs.c
+++ b/drivers/infiniband/hw/irdma/verbs.c
@@ -4450,8 +4450,16 @@ static const struct ib_device_ops irdma_roce_dev_ops = {
 };
 
 static const struct ib_device_ops irdma_iw_dev_ops = {
-	.modify_qp = irdma_modify_qp,
 	.get_port_immutable = irdma_iw_port_immutable,
+	.iw_accept = irdma_accept,
+	.iw_add_ref = irdma_qp_add_ref,
+	.iw_connect = irdma_connect,
+	.iw_create_listen = irdma_create_listen,
+	.iw_destroy_listen = irdma_destroy_listen,
+	.iw_get_qp = irdma_get_qp,
+	.iw_reject = irdma_reject,
+	.iw_rem_ref = irdma_qp_rem_ref,
+	.modify_qp = irdma_modify_qp,
 	.query_gid = irdma_query_gid,
 };
 
@@ -4522,14 +4530,6 @@ static void irdma_init_iw_device(struct irdma_device *iwdev)
 	iwdev->ibdev.node_type = RDMA_NODE_RNIC;
 	addrconf_addr_eui48((u8 *)&iwdev->ibdev.node_guid,
 			    netdev->dev_addr);
-	iwdev->ibdev.ops.iw_add_ref = irdma_qp_add_ref;
-	iwdev->ibdev.ops.iw_rem_ref = irdma_qp_rem_ref;
-	iwdev->ibdev.ops.iw_get_qp = irdma_get_qp;
-	iwdev->ibdev.ops.iw_connect = irdma_connect;
-	iwdev->ibdev.ops.iw_accept = irdma_accept;
-	iwdev->ibdev.ops.iw_reject = irdma_reject;
-	iwdev->ibdev.ops.iw_create_listen = irdma_create_listen;
-	iwdev->ibdev.ops.iw_destroy_listen = irdma_destroy_listen;
 	memcpy(iwdev->ibdev.iw_ifname, netdev->name,
 	       sizeof(iwdev->ibdev.iw_ifname));
 	ib_set_device_ops(&iwdev->ibdev, &irdma_iw_dev_ops);
-- 
2.40.1

RE: [PATCH 1/3] RDMA/iRDMA: Return void from irdma_init_iw_device()

[Saleem, Shiraz]

> Subject: [PATCH 1/3] RDMA/iRDMA: Return void from irdma_init_iw_device()
> 
> The return value from irdma_init_iw_device() is always 0 - change it to be void.
> 
> Signed-off-by: Kamal Heib <kheib@redhat.com>
> ---
>  drivers/infiniband/hw/irdma/verbs.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/irdma/verbs.c b/drivers/infiniband/hw/irdma/verbs.c
> index ab5cdf782785..b405cc961187 100644
> --- a/drivers/infiniband/hw/irdma/verbs.c
> +++ b/drivers/infiniband/hw/irdma/verbs.c
> @@ -4515,7 +4515,7 @@ static void irdma_init_roce_device(struct irdma_device
> *iwdev)
>   * irdma_init_iw_device - initialization of iwarp rdma device
>   * @iwdev: irdma device
>   */
> -static int irdma_init_iw_device(struct irdma_device *iwdev)
> +static void irdma_init_iw_device(struct irdma_device *iwdev)
>  {
>  	struct net_device *netdev = iwdev->netdev;
> 
> @@ -4533,8 +4533,6 @@ static int irdma_init_iw_device(struct irdma_device
> *iwdev)
>  	memcpy(iwdev->ibdev.iw_ifname, netdev->name,
>  	       sizeof(iwdev->ibdev.iw_ifname));
>  	ib_set_device_ops(&iwdev->ibdev, &irdma_iw_dev_ops);
> -
> -	return 0;
>  }
> 
>  /**
> @@ -4544,14 +4542,11 @@ static int irdma_init_iw_device(struct irdma_device
> *iwdev)  static int irdma_init_rdma_device(struct irdma_device *iwdev)  {
>  	struct pci_dev *pcidev = iwdev->rf->pcidev;
> -	int ret;
> 
>  	if (iwdev->roce_mode) {
>  		irdma_init_roce_device(iwdev);
>  	} else {
> -		ret = irdma_init_iw_device(iwdev);
> -		if (ret)
> -			return ret;
> +		irdma_init_iw_device(iwdev);
>  	}

checkpatch doesn't complain here? This becomes a single statement if/else now. No {} required.

>  	iwdev->ibdev.phys_port_cnt = 1;
>  	iwdev->ibdev.num_comp_vectors = iwdev->rf->ceqs_count;
> --
> 2.40.1

RE: [PATCH 0/3] RDMA/iRDMA: Cleanups and improvements

[Saleem, Shiraz]

> Subject: [PATCH 0/3] RDMA/iRDMA: Cleanups and improvements
> 
> This series includes a few cleanup patches for the iRDMA driver.
> 
> Kamal Heib (3):
>   RDMA/iRDMA: Return void from irdma_init_iw_device()
>   RDMA/iRDMA: Return void from irdma_init_rdma_device()
>   RDMA/iRDMA: Move iw device ops initialization
> 
>  drivers/infiniband/hw/irdma/verbs.c | 35 +++++++++++------------------
>  1 file changed, 13 insertions(+), 22 deletions(-)
> 

Thanks! some minor nits.

RDMA/iRDMA --> RDMA/irdma in subject line for the patches.

Reviewed-by: Shiraz Saleem <shiraz.saleem@intel.com>

Re: [PATCH 1/3] RDMA/iRDMA: Return void from irdma_init_iw_device()

[Kamal Heib]

On 2023-05-10 10:48, Saleem, Shiraz wrote:
>> Subject: [PATCH 1/3] RDMA/iRDMA: Return void from irdma_init_iw_device()
>>
>> The return value from irdma_init_iw_device() is always 0 - change it to be void.
>>
>> Signed-off-by: Kamal Heib <kheib@redhat.com>
>> ---
>>   drivers/infiniband/hw/irdma/verbs.c | 9 ++-------
>>   1 file changed, 2 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/infiniband/hw/irdma/verbs.c b/drivers/infiniband/hw/irdma/verbs.c
>> index ab5cdf782785..b405cc961187 100644
>> --- a/drivers/infiniband/hw/irdma/verbs.c
>> +++ b/drivers/infiniband/hw/irdma/verbs.c
>> @@ -4515,7 +4515,7 @@ static void irdma_init_roce_device(struct irdma_device
>> *iwdev)
>>    * irdma_init_iw_device - initialization of iwarp rdma device
>>    * @iwdev: irdma device
>>    */
>> -static int irdma_init_iw_device(struct irdma_device *iwdev)
>> +static void irdma_init_iw_device(struct irdma_device *iwdev)
>>   {
>>   	struct net_device *netdev = iwdev->netdev;
>>
>> @@ -4533,8 +4533,6 @@ static int irdma_init_iw_device(struct irdma_device
>> *iwdev)
>>   	memcpy(iwdev->ibdev.iw_ifname, netdev->name,
>>   	       sizeof(iwdev->ibdev.iw_ifname));
>>   	ib_set_device_ops(&iwdev->ibdev, &irdma_iw_dev_ops);
>> -
>> -	return 0;
>>   }
>>
>>   /**
>> @@ -4544,14 +4542,11 @@ static int irdma_init_iw_device(struct irdma_device
>> *iwdev)  static int irdma_init_rdma_device(struct irdma_device *iwdev)  {
>>   	struct pci_dev *pcidev = iwdev->rf->pcidev;
>> -	int ret;
>>
>>   	if (iwdev->roce_mode) {
>>   		irdma_init_roce_device(iwdev);
>>   	} else {
>> -		ret = irdma_init_iw_device(iwdev);
>> -		if (ret)
>> -			return ret;
>> +		irdma_init_iw_device(iwdev);
>>   	}
> 
> checkpatch doesn't complain here? This becomes a single statement if/else now. No {} required.
> 
It complains when running it over the file, fixed in v2.

Thanks,
Kamal


>>   	iwdev->ibdev.phys_port_cnt = 1;
>>   	iwdev->ibdev.num_comp_vectors = iwdev->rf->ceqs_count;
>> --
>> 2.40.1
> 

Re: [PATCH 0/3] RDMA/iRDMA: Cleanups and improvements

[Kamal Heib]

On 2023-05-10 10:52, Saleem, Shiraz wrote:
>> Subject: [PATCH 0/3] RDMA/iRDMA: Cleanups and improvements
>>
>> This series includes a few cleanup patches for the iRDMA driver.
>>
>> Kamal Heib (3):
>>    RDMA/iRDMA: Return void from irdma_init_iw_device()
>>    RDMA/iRDMA: Return void from irdma_init_rdma_device()
>>    RDMA/iRDMA: Move iw device ops initialization
>>
>>   drivers/infiniband/hw/irdma/verbs.c | 35 +++++++++++------------------
>>   1 file changed, 13 insertions(+), 22 deletions(-)
>>
> 
> Thanks! some minor nits.
> 
> RDMA/iRDMA --> RDMA/irdma in subject line for the patches.
>
Thank you, The subject is fixed in v2.


> Reviewed-by: Shiraz Saleem <shiraz.saleem@intel.com>
>