Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Fri, 7 Jun 2019 17:15:48 +0300
From:   Dan Carpenter <dan.carpenter@oracle.com>
To:     Nishka Dasgupta <nishkadg.linux@gmail.com>
Cc:     gregkh@linuxfoundation.org, devel@driverdev.osuosl.org,
        linux-kernel@vger.kernel.org, straube.linux@gmail.com,
        larry.finger@lwfinger.net, florian.c.schilhabel@googlemail.com,
        colin.king@canonical.com, valdis.kletnieks@vt.edu,
        tiny.windzz@gmail.com
Subject: Re: [PATCH 1/2] staging: rtl8712: r8712_setdatarate_cmd(): Change
Message-ID: <20190607141548.GP31203@kadam>
References: <20190607140658.11932-1-nishkadg.linux@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20190607140658.11932-1-nishkadg.linux@gmail.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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