public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] staging: rtl8712: r8712_setdatarate_cmd(): Change
@ 2019-06-07 14:06 Nishka Dasgupta
  2019-06-07 14:06 ` [PATCH 2/2] staging: rtl8712: r8712_createbss_cmd(): Change Nishka Dasgupta
  2019-06-07 14:15 ` [PATCH 1/2] staging: rtl8712: r8712_setdatarate_cmd(): Change Dan Carpenter
  0 siblings, 2 replies; 6+ messages in thread
From: Nishka Dasgupta @ 2019-06-07 14:06 UTC (permalink / raw)
  To: gregkh, devel, linux-kernel, straube.linux, larry.finger,
	florian.c.schilhabel, colin.king, valdis.kletnieks, tiny.windzz
  Cc: Nishka Dasgupta

Change the return values of function r8712_setdatarate_cmd from _SUCCESS
and _FAIL to 0 and -ENOMEM respectively.
Change the return type of the function from u8 to int to reflect this.
Change the call site of the function to check for 0 instead of _SUCCESS.
(Checking that the return value != 0 is not necessary; the return value
itself can simply be passed into the conditional.)

Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
---
 drivers/staging/rtl8712/rtl871x_cmd.c         | 8 ++++----
 drivers/staging/rtl8712/rtl871x_cmd.h         | 2 +-
 drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/rtl8712/rtl871x_cmd.c b/drivers/staging/rtl8712/rtl871x_cmd.c
index 05a78ac24987..e478c031f95f 100644
--- a/drivers/staging/rtl8712/rtl871x_cmd.c
+++ b/drivers/staging/rtl8712/rtl871x_cmd.c
@@ -242,7 +242,7 @@ u8 r8712_sitesurvey_cmd(struct _adapter *padapter,
 	return _SUCCESS;
 }
 
-u8 r8712_setdatarate_cmd(struct _adapter *padapter, u8 *rateset)
+int r8712_setdatarate_cmd(struct _adapter *padapter, u8 *rateset)
 {
 	struct cmd_obj		*ph2c;
 	struct setdatarate_parm	*pbsetdataratepara;
@@ -250,18 +250,18 @@ u8 r8712_setdatarate_cmd(struct _adapter *padapter, u8 *rateset)
 
 	ph2c = kmalloc(sizeof(*ph2c), GFP_ATOMIC);
 	if (!ph2c)
-		return _FAIL;
+		return -ENOMEM;
 	pbsetdataratepara = kmalloc(sizeof(*pbsetdataratepara), GFP_ATOMIC);
 	if (!pbsetdataratepara) {
 		kfree(ph2c);
-		return _FAIL;
+		return -ENOMEM;
 	}
 	init_h2fwcmd_w_parm_no_rsp(ph2c, pbsetdataratepara,
 				   GEN_CMD_CODE(_SetDataRate));
 	pbsetdataratepara->mac_id = 5;
 	memcpy(pbsetdataratepara->datarates, rateset, NumRates);
 	r8712_enqueue_cmd(pcmdpriv, ph2c);
-	return _SUCCESS;
+	return 0;
 }
 
 u8 r8712_set_chplan_cmd(struct _adapter *padapter, int chplan)
diff --git a/drivers/staging/rtl8712/rtl871x_cmd.h b/drivers/staging/rtl8712/rtl871x_cmd.h
index 262984c58efb..800216cca2f6 100644
--- a/drivers/staging/rtl8712/rtl871x_cmd.h
+++ b/drivers/staging/rtl8712/rtl871x_cmd.h
@@ -719,7 +719,7 @@ u8 r8712_joinbss_cmd(struct _adapter *padapter,
 u8 r8712_disassoc_cmd(struct _adapter *padapter);
 u8 r8712_setopmode_cmd(struct _adapter *padapter,
 		 enum NDIS_802_11_NETWORK_INFRASTRUCTURE networktype);
-u8 r8712_setdatarate_cmd(struct _adapter *padapter, u8 *rateset);
+int r8712_setdatarate_cmd(struct _adapter *padapter, u8 *rateset);
 u8 r8712_set_chplan_cmd(struct _adapter  *padapter, int chplan);
 u8 r8712_setbasicrate_cmd(struct _adapter *padapter, u8 *rateset);
 u8 r8712_getrfreg_cmd(struct _adapter *padapter, u8 offset, u8 *pval);
diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
index b424b8436fcf..761e2ba68a42 100644
--- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
+++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
@@ -1367,7 +1367,7 @@ static int r8711_wx_set_rate(struct net_device *dev,
 			datarates[i] = 0xff;
 		}
 	}
-	if (r8712_setdatarate_cmd(padapter, datarates) != _SUCCESS)
+	if (r8712_setdatarate_cmd(padapter, datarates))
 		ret = -ENOMEM;
 	return ret;
 }
-- 
2.19.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] staging: rtl8712: r8712_createbss_cmd(): Change
  2019-06-07 14:06 [PATCH 1/2] staging: rtl8712: r8712_setdatarate_cmd(): Change Nishka Dasgupta
@ 2019-06-07 14:06 ` Nishka Dasgupta
  2019-06-07 14:18   ` Dan Carpenter
  2019-06-07 14:15 ` [PATCH 1/2] staging: rtl8712: r8712_setdatarate_cmd(): Change Dan Carpenter
  1 sibling, 1 reply; 6+ messages in thread
From: Nishka Dasgupta @ 2019-06-07 14:06 UTC (permalink / raw)
  To: gregkh, devel, linux-kernel, straube.linux, larry.finger,
	florian.c.schilhabel, colin.king, valdis.kletnieks, tiny.windzz
  Cc: Nishka Dasgupta

Change return values of r8712_createbss_cmd from _SUCCESS and _FAIL to 0
and -ENOMEM respectively.
Change return type of the function from unsigned to int to reflect this.
Change call site to check for 0 instead of _SUCCESS.
(Instead of !=0, simply passing the function output to the conditional
will do.)

Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
---
 drivers/staging/rtl8712/rtl871x_cmd.c       | 6 +++---
 drivers/staging/rtl8712/rtl871x_cmd.h       | 2 +-
 drivers/staging/rtl8712/rtl871x_ioctl_set.c | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/rtl8712/rtl871x_cmd.c b/drivers/staging/rtl8712/rtl871x_cmd.c
index e478c031f95f..94ff875d9025 100644
--- a/drivers/staging/rtl8712/rtl871x_cmd.c
+++ b/drivers/staging/rtl8712/rtl871x_cmd.c
@@ -409,7 +409,7 @@ void r8712_readtssi_cmdrsp_callback(struct _adapter *padapter,
 	padapter->mppriv.workparam.bcompleted = true;
 }
 
-u8 r8712_createbss_cmd(struct _adapter *padapter)
+int r8712_createbss_cmd(struct _adapter *padapter)
 {
 	struct cmd_obj *pcmd;
 	struct cmd_priv *pcmdpriv = &padapter->cmdpriv;
@@ -419,7 +419,7 @@ u8 r8712_createbss_cmd(struct _adapter *padapter)
 	padapter->ledpriv.LedControlHandler(padapter, LED_CTL_START_TO_LINK);
 	pcmd = kmalloc(sizeof(*pcmd), GFP_ATOMIC);
 	if (!pcmd)
-		return _FAIL;
+		return -ENOMEM;
 	INIT_LIST_HEAD(&pcmd->list);
 	pcmd->cmdcode = _CreateBss_CMD_;
 	pcmd->parmbuf = (unsigned char *)pdev_network;
@@ -431,7 +431,7 @@ u8 r8712_createbss_cmd(struct _adapter *padapter)
 	pdev_network->IELength = pdev_network->IELength;
 	pdev_network->Ssid.SsidLength =	pdev_network->Ssid.SsidLength;
 	r8712_enqueue_cmd(pcmdpriv, pcmd);
-	return _SUCCESS;
+	return 0;
 }
 
 u8 r8712_joinbss_cmd(struct _adapter  *padapter, struct wlan_network *pnetwork)
diff --git a/drivers/staging/rtl8712/rtl871x_cmd.h b/drivers/staging/rtl8712/rtl871x_cmd.h
index 800216cca2f6..6ea1bafd8acc 100644
--- a/drivers/staging/rtl8712/rtl871x_cmd.h
+++ b/drivers/staging/rtl8712/rtl871x_cmd.h
@@ -712,7 +712,7 @@ u8 r8712_setMacAddr_cmd(struct _adapter *padapter, u8 *mac_addr);
 u8 r8712_setassocsta_cmd(struct _adapter *padapter, u8 *mac_addr);
 u8 r8712_sitesurvey_cmd(struct _adapter *padapter,
 			struct ndis_802_11_ssid *pssid);
-u8 r8712_createbss_cmd(struct _adapter *padapter);
+int r8712_createbss_cmd(struct _adapter *padapter);
 u8 r8712_setstakey_cmd(struct _adapter *padapter, u8 *psta, u8 unicast_key);
 u8 r8712_joinbss_cmd(struct _adapter *padapter,
 		     struct wlan_network *pnetwork);
diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_set.c b/drivers/staging/rtl8712/rtl871x_ioctl_set.c
index 2622d5e3bff9..d0274c65d17e 100644
--- a/drivers/staging/rtl8712/rtl871x_ioctl_set.c
+++ b/drivers/staging/rtl8712/rtl871x_ioctl_set.c
@@ -84,7 +84,7 @@ static u8 do_join(struct _adapter *padapter)
 			       sizeof(struct ndis_802_11_ssid));
 			r8712_update_registrypriv_dev_network(padapter);
 			r8712_generate_random_ibss(pibss);
-			if (r8712_createbss_cmd(padapter) != _SUCCESS)
+			if (r8712_createbss_cmd(padapter))
 				return false;
 			pmlmepriv->to_join = false;
 		} else {
-- 
2.19.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] staging: rtl8712: r8712_setdatarate_cmd(): Change
  2019-06-07 14:06 [PATCH 1/2] staging: rtl8712: r8712_setdatarate_cmd(): Change Nishka Dasgupta
  2019-06-07 14:06 ` [PATCH 2/2] staging: rtl8712: r8712_createbss_cmd(): Change Nishka Dasgupta
@ 2019-06-07 14:15 ` Dan Carpenter
  2019-06-10  4:32   ` Nishka Dasgupta
  1 sibling, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2019-06-07 14:15 UTC (permalink / raw)
  To: Nishka Dasgupta
  Cc: gregkh, devel, linux-kernel, straube.linux, larry.finger,
	florian.c.schilhabel, colin.king, valdis.kletnieks, tiny.windzz

Probably you sent this patch unintentionally.  The subject doesn't make
any sort of sense.  :P

On Fri, Jun 07, 2019 at 07:36:57PM +0530, Nishka Dasgupta wrote:
> Change the return values of function r8712_setdatarate_cmd from _SUCCESS
> and _FAIL to 0 and -ENOMEM respectively.
> Change the return type of the function from u8 to int to reflect this.
> Change the call site of the function to check for 0 instead of _SUCCESS.
> (Checking that the return value != 0 is not necessary; the return value
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> itself can simply be passed into the conditional.)
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

This is obvious.  No need to mention it in the commit message.

> diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> index b424b8436fcf..761e2ba68a42 100644
> --- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> +++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> @@ -1367,7 +1367,7 @@ static int r8711_wx_set_rate(struct net_device *dev,
>  			datarates[i] = 0xff;
>  		}
>  	}
> -	if (r8712_setdatarate_cmd(padapter, datarates) != _SUCCESS)
> +	if (r8712_setdatarate_cmd(padapter, datarates))
>  		ret = -ENOMEM;
>
>  	return ret;


It would be better to write this like so:

	ret = r8712_setdatarate_cmd(padapter, datarates);
	if (ret)
		return ret;

	return 0;

Or you could write it like:

	return r8712_setdatarate_cmd(padapter, datarates);

Which ever one you prefer is fine.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] staging: rtl8712: r8712_createbss_cmd(): Change
  2019-06-07 14:06 ` [PATCH 2/2] staging: rtl8712: r8712_createbss_cmd(): Change Nishka Dasgupta
@ 2019-06-07 14:18   ` Dan Carpenter
  0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2019-06-07 14:18 UTC (permalink / raw)
  To: Nishka Dasgupta
  Cc: gregkh, devel, linux-kernel, straube.linux, larry.finger,
	florian.c.schilhabel, colin.king, valdis.kletnieks, tiny.windzz

Fix the subject.

On Fri, Jun 07, 2019 at 07:36:58PM +0530, Nishka Dasgupta wrote:
> Change return values of r8712_createbss_cmd from _SUCCESS and _FAIL to 0
> and -ENOMEM respectively.
> Change return type of the function from unsigned to int to reflect this.
> Change call site to check for 0 instead of _SUCCESS.
> (Instead of !=0, simply passing the function output to the conditional
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> will do.)
  ^^^^^^^^^

Remove this line.

Otherwise it looks ok.  Please resend.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] staging: rtl8712: r8712_setdatarate_cmd(): Change
  2019-06-07 14:15 ` [PATCH 1/2] staging: rtl8712: r8712_setdatarate_cmd(): Change Dan Carpenter
@ 2019-06-10  4:32   ` Nishka Dasgupta
  2019-06-10 14:33     ` Dan Carpenter
  0 siblings, 1 reply; 6+ messages in thread
From: Nishka Dasgupta @ 2019-06-10  4:32 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: gregkh, devel, linux-kernel, straube.linux, larry.finger,
	florian.c.schilhabel, colin.king, valdis.kletnieks, tiny.windzz

On 07/06/19 7:45 PM, Dan Carpenter wrote:
> Probably you sent this patch unintentionally.  The subject doesn't make
> any sort of sense.  :P

So the problem with the subject line is that git send-email and vim (as 
configured on my laptop) tend to line-wrap even the subject line. Since 
I have two patches that do the same thing for different functions, I 
felt I should have the driver and the function name in the subject line 
(to avoid confusion between the patches and to allow for easy searching 
later). But that doesn't leave enough space in the subject line for 
"Change return values/type" or any other descriptive message. What 
should I do?


> On Fri, Jun 07, 2019 at 07:36:57PM +0530, Nishka Dasgupta wrote:
>> Change the return values of function r8712_setdatarate_cmd from _SUCCESS
>> and _FAIL to 0 and -ENOMEM respectively.
>> Change the return type of the function from u8 to int to reflect this.
>> Change the call site of the function to check for 0 instead of _SUCCESS.
>> (Checking that the return value != 0 is not necessary; the return value
>    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>> itself can simply be passed into the conditional.)
>    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^



> This is obvious.  No need to mention it in the commit message.

Okay, I'll amend that.

>> diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
>> index b424b8436fcf..761e2ba68a42 100644
>> --- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
>> +++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
>> @@ -1367,7 +1367,7 @@ static int r8711_wx_set_rate(struct net_device *dev,
>>   			datarates[i] = 0xff;
>>   		}
>>   	}
>> -	if (r8712_setdatarate_cmd(padapter, datarates) != _SUCCESS)
>> +	if (r8712_setdatarate_cmd(padapter, datarates))
>>   		ret = -ENOMEM;
>>
>>   	return ret;
> 
> 
> It would be better to write this like so:
> 
> 	ret = r8712_setdatarate_cmd(padapter, datarates);
> 	if (ret)
> 		return ret;
> 
> 	return 0;
> 
> Or you could write it like:
> 
> 	return r8712_setdatarate_cmd(padapter, datarates);

Okay, since this is the only point at which a return happens in this 
function, I can do this.

> Which ever one you prefer is fine
Thanking you,
Nishka

> regards,
> dan carpenter
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] staging: rtl8712: r8712_setdatarate_cmd(): Change
  2019-06-10  4:32   ` Nishka Dasgupta
@ 2019-06-10 14:33     ` Dan Carpenter
  0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2019-06-10 14:33 UTC (permalink / raw)
  To: Nishka Dasgupta
  Cc: devel, valdis.kletnieks, florian.c.schilhabel, tiny.windzz,
	gregkh, linux-kernel, colin.king, larry.finger

On Mon, Jun 10, 2019 at 10:02:27AM +0530, Nishka Dasgupta wrote:
> On 07/06/19 7:45 PM, Dan Carpenter wrote:
> > Probably you sent this patch unintentionally.  The subject doesn't make
> > any sort of sense.  :P
> 
> So the problem with the subject line is that git send-email and vim (as
> configured on my laptop) tend to line-wrap even the subject line. Since I
> have two patches that do the same thing for different functions, I felt I
> should have the driver and the function name in the subject line (to avoid
> confusion between the patches and to allow for easy searching later). But
> that doesn't leave enough space in the subject line for "Change return
> values/type" or any other descriptive message. What should I do?
> 

I don't really care.

[PATCH] staging: rtl8712: clean up r8712_setdatarate_cmd() return type

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-06-10 14:35 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-06-07 14:06 [PATCH 1/2] staging: rtl8712: r8712_setdatarate_cmd(): Change Nishka Dasgupta
2019-06-07 14:06 ` [PATCH 2/2] staging: rtl8712: r8712_createbss_cmd(): Change Nishka Dasgupta
2019-06-07 14:18   ` Dan Carpenter
2019-06-07 14:15 ` [PATCH 1/2] staging: rtl8712: r8712_setdatarate_cmd(): Change Dan Carpenter
2019-06-10  4:32   ` Nishka Dasgupta
2019-06-10 14:33     ` Dan Carpenter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox